diff --git a/at_client/src/main/java/org/atsign/client/api/AtCommandExecutorContext.java b/at_client/src/main/java/org/atsign/client/api/AtCommandExecutorContext.java new file mode 100644 index 00000000..3db7cfa9 --- /dev/null +++ b/at_client/src/main/java/org/atsign/client/api/AtCommandExecutorContext.java @@ -0,0 +1,62 @@ +package org.atsign.client.api; + +import lombok.*; + +import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; + +/** + * The identity a connection authenticates as (its {@code atSign}, {@code keys} and {@code config}) + * together with the single-use challenge from the {@code from:} that is issued as the first command + * once the connection is ready. + * + *

+ * The context is created by the builder (see + * {@code AtCommandExecutors#createCommandExecutor}) and closed over by the {@code onReady} + * consumers it wires, so the {@code from:} sender can retain the challenge and the authentication + * that follows on the same connection can reuse it rather than issuing a second {@code from:}. + * The command executor itself is pure transport and knows nothing about this context. + * + *

+ * The identity fields are fixed for the life of the context. The challenge is per-connection state: + * {@link #setChallenge(String) retained} when the initial {@code from:} completes and + * {@link #consumeChallenge() consumed} at most once (the server's {@code from:} challenge is + * single-use). On reconnect the ready sequence re-runs, so a fresh challenge overwrites any + * previous + * one before it is consumed. + */ +@Value +public class AtCommandExecutorContext { + + AtSign atSign; + + AtKeys keys; + + Map config; + + @Getter(AccessLevel.NONE) + @EqualsAndHashCode.Exclude + @ToString.Exclude + AtomicReference challenge = new AtomicReference<>(); + + /** + * Retains the challenge returned by the initial {@code from:}, so the authentication that follows + * on the same connection can reuse it. + * + * @param challenge the challenge from the server's {@code from:} response + */ + public void setChallenge(String challenge) { + this.challenge.set(challenge); + } + + /** + * Returns the retained {@code from:} challenge and clears it, so it is consumed at most once. + * Whichever authentication (CRAM or PKAM) sends its digest first consumes it; anything else on the + * same connection gets {@code null} and must issue its own {@code from:}. + * + * @return the retained challenge, or {@code null} if none is available + */ + public String consumeChallenge() { + return challenge.getAndSet(null); + } +} diff --git a/at_client/src/main/java/org/atsign/client/impl/AtCommandExecutors.java b/at_client/src/main/java/org/atsign/client/impl/AtCommandExecutors.java index 4b7d1f49..70773ada 100644 --- a/at_client/src/main/java/org/atsign/client/impl/AtCommandExecutors.java +++ b/at_client/src/main/java/org/atsign/client/impl/AtCommandExecutors.java @@ -1,7 +1,17 @@ package org.atsign.client.impl; -import static org.atsign.client.impl.common.Preconditions.checkNotNull; +import lombok.Builder; +import lombok.extern.slf4j.Slf4j; +import org.atsign.client.api.AtCommandExecutor; +import org.atsign.client.api.AtCommandExecutorContext; +import org.atsign.client.api.AtKeys; +import org.atsign.client.api.AtSign; +import org.atsign.client.impl.commands.AuthenticationCommands; +import org.atsign.client.impl.common.ReconnectStrategy; +import org.atsign.client.impl.common.SimpleReconnectStrategy; +import org.atsign.client.impl.exceptions.AtException; +import org.atsign.client.impl.netty.NettyAtCommandExecutor; import java.io.InputStream; import java.net.URL; @@ -12,17 +22,7 @@ import java.util.concurrent.TimeUnit; import java.util.function.Consumer; -import org.atsign.client.api.AtCommandExecutor; -import org.atsign.client.api.AtKeys; -import org.atsign.client.api.AtSign; -import org.atsign.client.impl.commands.AuthenticationCommands; -import org.atsign.client.impl.common.ReconnectStrategy; -import org.atsign.client.impl.common.SimpleReconnectStrategy; -import org.atsign.client.impl.exceptions.AtException; -import org.atsign.client.impl.netty.NettyAtCommandExecutor; - -import lombok.Builder; -import lombok.extern.slf4j.Slf4j; +import static org.atsign.client.impl.common.Preconditions.checkNotNull; /** * Utility methods / builders for instantiating {@link AtCommandExecutor} implementations @@ -42,8 +42,10 @@ * * NOTE: If the url is prefixed with proxy (e.g. proxy:host:port) then the builder * will automatically attempt to connect to an At Server at host:port. - * NOTE: If atSign and keys are provided then the builder - * will automatically configure the {@link AtCommandExecutor} to authenticate with PKAM. + * NOTE: If an atSign is provided then the builder issues {@code from:@atSign} as the first + * command once connected (so proxies / gateways can route the connection); if keys are also + * provided it then authenticates the {@link AtCommandExecutor} with PKAM, reusing that + * {@code from:}'s challenge. * NOTE: If reconnect is not set then the builder will default to a * {@link SimpleReconnectStrategy} */ @@ -69,6 +71,10 @@ public static AtCommandExecutor createCommandExecutor(String url, checkNotNull(atSign, "atSign not set"); } + // the context is closed over by the onReady consumers the builder wires (see createOnReady); the + // command executor itself stays pure transport and knows nothing about it + AtCommandExecutorContext context = new AtCommandExecutorContext(atSign, keys, createClientConfig(config)); + return NettyAtCommandExecutor.builder() .endpoint(AtEndpointSuppliers.builder().url(url).atSign(atSign).build()) .isVerbose(isVerbose) @@ -76,7 +82,7 @@ public static AtCommandExecutor createCommandExecutor(String url, .awaitReadyMillis(defaultIfNotSet(awaitReadyMillis, DEFAULT_TIMEOUT_MILLIS)) .reconnect(defaultIfNotSet(reconnect, SimpleReconnectStrategy.builder().build())) .queueLimit(queueLimit) - .onReady(defaultIfNotSet(onReady, createOnReady(atSign, keys, createClientConfig(config)))) + .onReady(defaultIfNotSet(onReady, createOnReady(context))) .build(); } @@ -128,14 +134,21 @@ public static Map createClientConfig(Map config) return result; } - private static Consumer createOnReady(AtSign atSign, AtKeys keys, Map config) { - Consumer onReady; - if (atSign != null && keys != null) { - onReady = AuthenticationCommands.pkamAuthenticator(atSign, keys, config); - } else { - onReady = c -> { + /** + * The default protocol for a newly-ready connection. A connection with no atSign (e.g. one talking + * to the atDirectory / root server) sends nothing. Every connection that has an atSign issues + * {@code from:@atSign} first so that proxies / gateways can route it; if keys are also present it + * then authenticates with PKAM, reusing the challenge from that initial {@code from:}. + */ + private static Consumer createOnReady(AtCommandExecutorContext context) { + if (context.getAtSign() == null) { + return c -> { }; } + Consumer onReady = AuthenticationCommands.sendFrom(context); + if (context.getKeys() != null) { + onReady = onReady.andThen(AuthenticationCommands.pkamAuthenticator(context)); + } return onReady; } diff --git a/at_client/src/main/java/org/atsign/client/impl/cli/AbstractCli.java b/at_client/src/main/java/org/atsign/client/impl/cli/AbstractCli.java index 41ffe904..73a85e85 100644 --- a/at_client/src/main/java/org/atsign/client/impl/cli/AbstractCli.java +++ b/at_client/src/main/java/org/atsign/client/impl/cli/AbstractCli.java @@ -4,16 +4,16 @@ import java.util.concurrent.TimeUnit; import org.atsign.client.api.AtCommandExecutor; +import org.atsign.client.api.AtCommandExecutorContext; import org.atsign.client.api.AtKeys; import org.atsign.client.api.AtSign; -import org.atsign.client.impl.AtEndpointSupplier; -import org.atsign.client.impl.AtEndpointSuppliers; +import org.atsign.client.impl.AtCommandExecutors; +import org.atsign.client.impl.AtCommandExecutors.AtCommandExecutorBuilder; import org.atsign.client.impl.commands.AuthenticationCommands; +import org.atsign.client.impl.common.ReconnectStrategy; import org.atsign.client.impl.common.SimpleReconnectStrategy; import org.atsign.client.impl.exceptions.AtClientConfigException; import org.atsign.client.impl.exceptions.AtException; -import org.atsign.client.impl.netty.NettyAtCommandExecutor; -import org.atsign.client.impl.netty.NettyAtCommandExecutor.NettyAtCommandExecutorBuilder; import org.atsign.client.impl.util.KeysUtils; import picocli.CommandLine.Option; @@ -87,25 +87,69 @@ protected static String ensureNotNull(String value, String defaultValue) { } protected AtCommandExecutor createConnection(String rootUrl, AtSign atSign, int retries) throws AtException { - return createCommandExecutorBuilder(rootUrl, atSign, retries, verbose).build(); + // no keys: the builder wires an onReady that issues from:@atSign only (so proxies can route it) + return connectionBuilder(rootUrl, retries, verbose).atSign(atSign).build(); } protected AtCommandExecutor createAuthenticatedConnection(String rootUrl, AtSign atSign, int retries) throws AtException { - return createCommandExecutorBuilder(rootUrl, atSign, retries, verbose) - .onReady(AuthenticationCommands.pkamAuthenticator(atSign, getKeys(), null)) + // keys present: the builder wires from:@atSign followed by PKAM (reusing the from: challenge) + return connectionBuilder(rootUrl, retries, verbose).atSign(atSign).keys(getKeys()).build(); + } + + /** + * A connection that issues from:@atSign on connect (so proxies / gateways can route it), wiring the + * {@code from:} over the given {@code context} so the challenge it retains is the one an imperative + * flow driving the connection (e.g. onboarding: scan, then CRAM, then PKAM) later consumes. No + * on-ready authentication is wired — the caller authenticates imperatively. + * + *

+ * Reconnect is disabled: because the imperative auth reuses the connect-time {@code from:} + * challenge, a mid-flow drop must abort (the one-shot flow is then rerun) rather than reconnect and + * authenticate on the new connection with a challenge the previous connection issued — the new + * connection's server would reject it. (The challenge is not cleared on close, so reconnect-and- + * reuse would be stale.) + */ + protected AtCommandExecutor createConnectionSendingFrom(AtCommandExecutorContext context) throws AtException { + return AtCommandExecutors.builder() + .url(rootUrl) + .atSign(context.getAtSign()) + .onReady(AuthenticationCommands.sendFrom(context)) + .reconnect(ReconnectStrategy.NONE) + .isVerbose(verbose) .build(); } - private static NettyAtCommandExecutorBuilder createCommandExecutorBuilder(String rootUrl, AtSign atSign, int retries, - boolean verbose) { - AtEndpointSupplier endpoint = AtEndpointSuppliers.builder().url(rootUrl).atSign(atSign).build(); + /** + * A connection that does NOT issue {@code from:} on connect — its initial {@code from:} is issued + * by the caller's own first command — for flows that authenticate imperatively and must control the + * timing (e.g. a pending-retry loop), where a connect-time {@code from:} challenge could go stale + * before it is used. No onReady {@code from:} is wired, so the caller's first command (its + * authentication) establishes the atSign for proxies / gateways. + */ + protected AtCommandExecutor createConnectionDeferringFrom(String rootUrl, AtSign atSign, int retries) + throws AtException { + return connectionBuilder(rootUrl, retries, verbose).atSign(atSign).onReady(executor -> { + }).build(); + } + + /** + * Creates an {@link AtCommandExecutorContext} carrying this CLI's atSign and the given {@code keys} + * (the identity being onboarded), to be passed to + * {@link #createConnectionSendingFrom(AtCommandExecutorContext)} and threaded into the + * imperative onboarding flow, which reuses the connection's {@code from:} challenge. + */ + protected AtCommandExecutorContext newConnectionContext(AtKeys keys) { + return new AtCommandExecutorContext(atSign, keys, AtCommandExecutors.createClientConfig(null)); + } + + private static AtCommandExecutorBuilder connectionBuilder(String rootUrl, int retries, boolean verbose) { SimpleReconnectStrategy reconnect = SimpleReconnectStrategy.builder() .maxReconnectRetries(retries) .reconnectPauseMillis(TimeUnit.SECONDS.toMillis(2)) .build(); - return NettyAtCommandExecutor.builder() - .endpoint(endpoint) + return AtCommandExecutors.builder() + .url(rootUrl) .reconnect(reconnect) .isVerbose(verbose); } diff --git a/at_client/src/main/java/org/atsign/client/impl/cli/Activate.java b/at_client/src/main/java/org/atsign/client/impl/cli/Activate.java index 5bf2cc40..e7685bd7 100644 --- a/at_client/src/main/java/org/atsign/client/impl/cli/Activate.java +++ b/at_client/src/main/java/org/atsign/client/impl/cli/Activate.java @@ -14,6 +14,7 @@ import org.atsign.client.api.AtKeys; import org.atsign.client.api.AtCommandExecutor; +import org.atsign.client.api.AtCommandExecutorContext; import org.atsign.client.impl.commands.EnrollCommands; import org.atsign.client.impl.commands.ScanCommands; import org.atsign.client.impl.common.EnrollmentId; @@ -180,24 +181,23 @@ public Activate addNamespace(String namespace, String accessControl) { } public EnrollmentId onboard() throws Exception { - try (AtCommandExecutor executor = createConnection(rootUrl, atSign, connectionRetries)) { - return onboard(executor); + AtCommandExecutorContext context = newConnectionContext(generateAtKeys(true)); + try (AtCommandExecutor executor = createConnectionSendingFrom(context)) { + return onboard(executor, context); } } - public EnrollmentId onboard(AtCommandExecutor executor) throws Exception { + public EnrollmentId onboard(AtCommandExecutor executor, AtCommandExecutorContext context) throws Exception { File file = getAtKeysFile(keysFile, atSign); if (!overwriteKeysFile) { checkNotExists(file); } - AtKeys keys = generateAtKeys(true); - keys = EnrollCommands.onboard(executor, - atSign, - keys, - cramSecret, - ensureNotNull(appName, DEFAULT_FIRST_APP), - ensureNotNull(deviceName, DEFAULT_FIRST_DEVICE), - deleteCramKey); + AtKeys keys = EnrollCommands.onboard(executor, + context, + cramSecret, + ensureNotNull(appName, DEFAULT_FIRST_APP), + ensureNotNull(deviceName, DEFAULT_FIRST_DEVICE), + deleteCramKey); saveKeys(keys, file); return enrollmentId; } @@ -292,7 +292,9 @@ public EnrollmentId enroll(AtCommandExecutor executor) throws Exception { } public void complete() throws Exception { - try (AtCommandExecutor executor = createConnection(rootUrl, atSign, connectionRetries)) { + // complete authenticates imperatively (and retries on "pending"), so it issues its own from: + // rather than reusing a connect-time challenge that could go stale before the retried PKAM + try (AtCommandExecutor executor = createConnectionDeferringFrom(rootUrl, atSign, connectionRetries)) { complete(executor); } } @@ -304,7 +306,8 @@ public void complete(AtCommandExecutor executor) throws Exception { } public void complete(int retries, long sleepDuration, TimeUnit sleepUnit) throws Exception { - try (AtCommandExecutor executor = createConnection(rootUrl, atSign, connectionRetries)) { + // see complete(): imperative, retried PKAM issues its own from: + try (AtCommandExecutor executor = createConnectionDeferringFrom(rootUrl, atSign, connectionRetries)) { complete(executor, retries, sleepDuration, sleepUnit); } } diff --git a/at_client/src/main/java/org/atsign/client/impl/commands/AuthenticationCommands.java b/at_client/src/main/java/org/atsign/client/impl/commands/AuthenticationCommands.java index b7d3e702..f4ac9b54 100644 --- a/at_client/src/main/java/org/atsign/client/impl/commands/AuthenticationCommands.java +++ b/at_client/src/main/java/org/atsign/client/impl/commands/AuthenticationCommands.java @@ -15,6 +15,7 @@ import org.atsign.client.api.AtKeys; import org.atsign.client.api.AtCommandExecutor; +import org.atsign.client.api.AtCommandExecutorContext; import org.atsign.client.impl.util.EncryptionUtils; import org.atsign.client.impl.exceptions.AtException; import org.atsign.client.api.AtSign; @@ -26,6 +27,42 @@ */ public class AuthenticationCommands { + /** + * Returns an {@code onReady} consumer that issues {@code from:@atSign} as the first command on the + * connection and retains the challenge from the response in the given {@code context}. This + * establishes the connection's atSign up front so that proxies / gateways can route it, and lets + * any authentication that follows on the same connection reuse the challenge (see + * {@link #pkamAuthenticator(AtCommandExecutorContext)}) rather than issuing a second {@code from:}. + * + * @param context the connection context; supplies the atSign / config and receives the challenge + * @return an onReady consumer that sends the initial {@code from:} + */ + public static Consumer sendFrom(AtCommandExecutorContext context) { + return throwOnReadyException(executor -> { + String fromCommand = CommandBuilders.fromCommandBuilder() + .atSign(context.getAtSign()) + .config(context.getConfig()) + .build(); + String fromResponse = executor.sendSync(fromCommand); + context.setChallenge(matchDataStringNoWhitespace(throwExceptionIfError(fromResponse))); + }); + } + + /** + * Returns an {@code onReady} consumer that authenticates with PKAM using the identity in the given + * {@code context}, reusing the challenge from an initial {@code from:} (as issued by + * {@link #sendFrom(AtCommandExecutorContext)}) when one is available and otherwise issuing its own. + * This is the standard client path, where the identity is fixed for the connection. + * + * @param context the connection context; supplies the atSign / keys / config and the challenge + * @return an onReady consumer that performs PKAM authentication + */ + public static Consumer pkamAuthenticator(AtCommandExecutorContext context) { + return throwOnReadyException(executor -> authenticateWithPkam(executor, context.getAtSign(), + context.getKeys(), context.getConfig(), + context.consumeChallenge())); + } + public static Consumer pkamAuthenticator(AtSign atSign, AtKeys keys, Map config) { return throwOnReadyException(executor -> authenticateWithPkam(executor, atSign, keys, config)); } @@ -57,12 +94,37 @@ public static void authenticateWithPkam(AtCommandExecutor executor, AtKeys keys, Map config) throws AtException { + authenticateWithPkam(executor, atSign, keys, config, null); + } + + /** + * Implements the protocol workflow / sequence for PKAM authentication, reusing an already-issued + * {@code from:} challenge when one is supplied. + * + * @param executor The executor with which to send the commands. + * @param atSign The asign to authenticate. + * @param keys The keys to use to authenticate. + * @param config The map of configuration values to send in the from command. + * @param reusableChallenge The challenge from an initial {@code from:} to reuse, or {@code null} to + * issue a fresh {@code from:}. + * @throws AtException If authentication fails. + */ + private static void authenticateWithPkam(AtCommandExecutor executor, + AtSign atSign, + AtKeys keys, + Map config, + String reusableChallenge) + throws AtException { try { + // reuse the challenge from the initial from: if one was issued on this connection, otherwise // send a from command and expect to receive a challenge - String fromCommand = CommandBuilders.fromCommandBuilder().atSign(atSign).config(config).build(); - String fromResponse = executor.sendSync(fromCommand); - String challenge = matchDataStringNoWhitespace(throwExceptionIfError(fromResponse)); + String challenge = reusableChallenge; + if (challenge == null) { + String fromCommand = CommandBuilders.fromCommandBuilder().atSign(atSign).config(config).build(); + String fromResponse = executor.sendSync(fromCommand); + challenge = matchDataStringNoWhitespace(throwExceptionIfError(fromResponse)); + } // send a pkam command with the signed challenge String signature = EncryptionUtils.signSHA256RSA(challenge, keys.getApkamPrivateKey()); @@ -84,21 +146,30 @@ public static void authenticateWithPkam(AtCommandExecutor executor, } /** - * Implements the protocol workflow / sequence for CRAM authentication. + * Implements the protocol workflow / sequence for CRAM authentication, reusing the challenge from + * an initial {@code from:} (as issued by {@link #sendFrom(AtCommandExecutorContext)}) held in the + * given {@code context} when one is available and otherwise issuing its own. Used by onboarding, + * which issues a {@code from:} on connect, runs a connectivity scan, then authenticates. * * @param executor The executor with which to send the commands. - * @param atSign The asign to authenticate. + * @param context The connection context; supplies the atSign and the retained challenge. * @param cramSecret The cramSecret that was assigned during At Server provisioning. * @throws AtException If authentication fails. */ - public static void authenticateWithCram(AtCommandExecutor executor, AtSign atSign, String cramSecret) + public static void authenticateWithCram(AtCommandExecutor executor, + AtCommandExecutorContext context, + String cramSecret) throws AtException { try { + // reuse the challenge from the initial from: if one was issued on this connection, otherwise // send a from command and expect to receive a challenge - String fromCommand = CommandBuilders.fromCommandBuilder().atSign(atSign).build(); - String fromResponse = executor.sendSync(fromCommand); - String challenge = matchDataStringNoWhitespace(throwExceptionIfError(fromResponse)); + String challenge = context.consumeChallenge(); + if (challenge == null) { + String fromCommand = CommandBuilders.fromCommandBuilder().atSign(context.getAtSign()).build(); + String fromResponse = executor.sendSync(fromCommand); + challenge = matchDataStringNoWhitespace(throwExceptionIfError(fromResponse)); + } // send a cram command String cramDigest = createDigest(cramSecret, challenge); @@ -123,6 +194,4 @@ private static String createDigest(String cramSecret, String challenge) throws A throw new AtEncryptionException("failed to generate cramDigest", e); } } - - } diff --git a/at_client/src/main/java/org/atsign/client/impl/commands/EnrollCommands.java b/at_client/src/main/java/org/atsign/client/impl/commands/EnrollCommands.java index 9b1694cf..963eaa88 100644 --- a/at_client/src/main/java/org/atsign/client/impl/commands/EnrollCommands.java +++ b/at_client/src/main/java/org/atsign/client/impl/commands/EnrollCommands.java @@ -14,6 +14,7 @@ import java.util.stream.Collectors; import org.atsign.client.api.AtCommandExecutor; +import org.atsign.client.api.AtCommandExecutorContext; import org.atsign.client.api.AtKeyNames; import org.atsign.client.api.AtKeys; import org.atsign.client.api.AtSign; @@ -32,8 +33,10 @@ public class EnrollCommands { * app/device keys that can approve subsequent enrollments. * * @param executor The {@link AtCommandExecutor} to use. - * @param atSign The AtSign that corresponds to the executor. - * @param keys The {@link AtKeys} for the {@link AtSign}, these should already be populated. + * @param context The connection context for the executor; the single source of identity for this + * onboarding — its atSign, the {@link AtKeys} being onboarded (already populated), and the + * {@code from:} challenge that CRAM reuses (the connection issues {@code from:} on connect, + * before the connectivity scan below). * @param cramSecret The CRAM secret. * @param appName The app name for this first enrollment. * @param deviceName The device name for this first enrollment. @@ -43,13 +46,14 @@ public class EnrollCommands { * @throws AtException If any of the commands fail. */ public static AtKeys onboard(AtCommandExecutor executor, - AtSign atSign, - AtKeys keys, + AtCommandExecutorContext context, String cramSecret, String appName, String deviceName, boolean deleteCramKey) throws AtException { + AtSign atSign = context.getAtSign(); + AtKeys keys = context.getKeys(); try { // verify that the executor is connected to the atsigns atserver @@ -60,8 +64,8 @@ public static AtKeys onboard(AtCommandExecutor executor, throw new IllegalStateException("not connected to the atsign's at server"); } - // authenticate with CRAM - AuthenticationCommands.authenticateWithCram(executor, atSign, cramSecret); + // authenticate with CRAM, reusing the from: challenge the connection issued on connect + AuthenticationCommands.authenticateWithCram(executor, context, cramSecret); // send an enroll request which should automatically be approved after CRAM authentication String requestCommand = CommandBuilders.enrollCommandBuilder() @@ -79,7 +83,9 @@ public static AtKeys onboard(AtCommandExecutor executor, .enrollmentId(createEnrollmentId(response.get("enrollmentId"))) .build(); - // authenticate with PKAM + // authenticate with PKAM — issues its own from: (the connection's challenge was single-use and + // is spent by CRAM above; PKAM also authenticates with the freshly-enrolled keys, not the + // connection identity) AuthenticationCommands.authenticateWithPkam(executor, atSign, keys); // explicitly store the public encryption key in the atserver diff --git a/at_client/src/test/java/org/atsign/client/impl/commands/AuthenticationCommandsTest.java b/at_client/src/test/java/org/atsign/client/impl/commands/AuthenticationCommandsTest.java index c52d60dd..65c11cb4 100644 --- a/at_client/src/test/java/org/atsign/client/impl/commands/AuthenticationCommandsTest.java +++ b/at_client/src/test/java/org/atsign/client/impl/commands/AuthenticationCommandsTest.java @@ -1,14 +1,8 @@ package org.atsign.client.impl.commands; -import static org.atsign.client.impl.util.EncryptionUtils.generateRSAKeyPair; -import static org.atsign.client.impl.common.EnrollmentId.createEnrollmentId; -import static org.atsign.client.api.AtSign.createAtSign; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.*; -import static org.junit.jupiter.api.Assertions.assertThrows; - -import org.atsign.client.api.AtKeys; import org.atsign.client.api.AtCommandExecutor; +import org.atsign.client.api.AtCommandExecutorContext; +import org.atsign.client.api.AtKeys; import org.atsign.client.impl.exceptions.AtOnReadyException; import org.atsign.client.impl.exceptions.AtUnauthenticatedException; import org.junit.jupiter.api.Test; @@ -16,6 +10,16 @@ import java.util.Collections; import java.util.Map; +import static org.atsign.client.api.AtSign.createAtSign; +import static org.atsign.client.impl.common.EnrollmentId.createEnrollmentId; +import static org.atsign.client.impl.util.EncryptionUtils.generateRSAKeyPair; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.*; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.matches; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + public class AuthenticationCommandsTest { @Test @@ -25,7 +29,9 @@ public void testAuthenticateWithCramDoesNotThrowException() throws Exception { .stub("cram:7e91508d5.+", "data:success") .build(); - AuthenticationCommands.authenticateWithCram(executor, createAtSign("@alice"), "secret"); + AuthenticationCommands.authenticateWithCram(executor, + new AtCommandExecutorContext(createAtSign("@alice"), null, null), + "secret"); } @Test @@ -36,7 +42,11 @@ public void testAuthenticateWithCramFailThrowsExpectedException() throws Excepti .build(); Exception ex = assertThrows(AtUnauthenticatedException.class, - () -> AuthenticationCommands.authenticateWithCram(executor, createAtSign("@alice"), + () -> AuthenticationCommands.authenticateWithCram( + executor, + new AtCommandExecutorContext( + createAtSign("@alice"), null, + null), "secret")); assertThat(ex.getMessage(), containsString("deliberate")); } @@ -103,4 +113,63 @@ public void testPkamAuthenticatorThrowsOnReadyException() throws Exception { assertThat(ex, instanceOf(AtOnReadyException.class)); assertThat(ex.getMessage(), containsString("deliberate")); } + + @Test + public void testSendFromIssuesFromAndRetainsChallenge() throws Exception { + AtCommandExecutor executor = TestExecutorBuilder.builder() + .stub("from:@alice", "data:challenge") + .build(); + AtCommandExecutorContext context = new AtCommandExecutorContext(createAtSign("@alice"), null, null); + + AuthenticationCommands.sendFrom(context).accept(executor); + + verify(executor, times(1)).sendSync(matches("from:.*")); + assertThat(context.consumeChallenge(), is("challenge")); + } + + @Test + public void testPkamAuthenticatorReusesTheInitialFromChallenge() throws Exception { + AtKeys keys = AtKeys.builder().apkamKeyPair(generateRSAKeyPair()).build(); + AtCommandExecutor executor = TestExecutorBuilder.builder() + .stub("from:@alice", "data:challenge") + .stub("pkam:[^{].+", "data:success") + .build(); + AtCommandExecutorContext context = new AtCommandExecutorContext(createAtSign("@alice"), keys, null); + + // the from: sender runs first (as wired by createOnReady), then PKAM reuses its challenge + AuthenticationCommands.sendFrom(context).accept(executor); + AuthenticationCommands.pkamAuthenticator(context).accept(executor); + + // exactly one from: for the whole connection — PKAM did not issue a second one + verify(executor, times(1)).sendSync(matches("from:.*")); + verify(executor, times(1)).sendSync(matches("pkam:.*")); + assertThat(context.consumeChallenge(), is(nullValue())); + } + + @Test + public void testPkamAuthenticatorIssuesItsOwnFromWhenNoChallengeRetained() throws Exception { + AtKeys keys = AtKeys.builder().apkamKeyPair(generateRSAKeyPair()).build(); + AtCommandExecutor executor = TestExecutorBuilder.builder() + .stub("from:@alice", "data:challenge") + .stub("pkam:[^{].+", "data:success") + .build(); + AtCommandExecutorContext context = new AtCommandExecutorContext(createAtSign("@alice"), keys, null); + + // no prior sendFrom: the authenticator must issue its own from: to obtain a challenge + AuthenticationCommands.pkamAuthenticator(context).accept(executor); + + verify(executor, times(1)).sendSync(matches("from:.*")); + verify(executor, times(1)).sendSync(matches("pkam:.*")); + } + + @Test + public void testRetainedChallengeIsConsumedAtMostOnce() { + AtCommandExecutorContext context = new AtCommandExecutorContext(createAtSign("@alice"), null, null); + context.setChallenge("challenge"); + + // single-use: the first consumer gets it, a second (e.g. a further auth on the same connection) + // gets null and falls back to issuing its own from: + assertThat(context.consumeChallenge(), is("challenge")); + assertThat(context.consumeChallenge(), is(nullValue())); + } } diff --git a/at_client/src/test/java/org/atsign/client/impl/commands/EnrollCommandsTest.java b/at_client/src/test/java/org/atsign/client/impl/commands/EnrollCommandsTest.java index fe07ee02..f3f4a8d6 100644 --- a/at_client/src/test/java/org/atsign/client/impl/commands/EnrollCommandsTest.java +++ b/at_client/src/test/java/org/atsign/client/impl/commands/EnrollCommandsTest.java @@ -1,23 +1,27 @@ package org.atsign.client.impl.commands; +import org.atsign.client.api.AtCommandExecutor; +import org.atsign.client.api.AtCommandExecutorContext; +import org.atsign.client.api.AtKeys; +import org.atsign.client.api.AtSign; +import org.atsign.client.impl.common.EnrollmentId; +import org.atsign.client.impl.exceptions.AtException; +import org.atsign.client.impl.exceptions.AtServerRuntimeException; +import org.junit.jupiter.api.Test; + +import java.util.List; + import static java.lang.String.format; import static java.util.Collections.singletonMap; -import static org.atsign.client.impl.util.EncryptionUtils.*; -import static org.atsign.client.impl.common.EnrollmentId.createEnrollmentId; import static org.atsign.client.api.AtSign.createAtSign; +import static org.atsign.client.impl.common.EnrollmentId.createEnrollmentId; +import static org.atsign.client.impl.util.EncryptionUtils.*; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.*; import static org.junit.jupiter.api.Assertions.assertThrows; - -import java.util.List; - -import org.atsign.client.api.AtKeys; -import org.atsign.client.api.AtCommandExecutor; -import org.atsign.client.impl.common.EnrollmentId; -import org.atsign.client.impl.exceptions.AtException; -import org.atsign.client.api.AtSign; -import org.atsign.client.impl.exceptions.AtServerRuntimeException; -import org.junit.jupiter.api.Test; +import static org.mockito.ArgumentMatchers.matches; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; public class EnrollCommandsTest { @@ -63,7 +67,8 @@ public void testOnboardThrowsExceptionIfSigningPublicKeyIsMissing() throws Excep .build(); Exception ex = assertThrows(Exception.class, - () -> EnrollCommands.onboard(executor, atSign, keys, "secret", "app", "device", false)); + () -> EnrollCommands.onboard(executor, new AtCommandExecutorContext(atSign, keys, null), + "secret", "app", "device", false)); assertThat(ex.getMessage(), containsString("not connected to the atsign's at server")); } @@ -102,12 +107,43 @@ public void testOnboard() throws Exception { .stub("update:public:publickey@alice .+", "data:1") .build(); - AtKeys newKeys = EnrollCommands.onboard(executor, atSign, keys, "secret", "app", "device", false); + AtKeys newKeys = EnrollCommands.onboard(executor, new AtCommandExecutorContext(atSign, keys, null), "secret", + "app", "device", false); assertThat(newKeys, is(not(sameInstance(keys)))); assertThat(newKeys.getEnrollmentId(), equalTo(createEnrollmentId("904dcbf7"))); } + @Test + public void testOnboardReusesTheConnectionFromChallengeForCram() throws Exception { + AtSign atSign = createAtSign("@alice"); + AtKeys keys = AtKeys.builder() + .apkamKeyPair(generateRSAKeyPair()) + .encryptKeyPair(generateRSAKeyPair()) + .build(); + + // simulate the connection having issued from: on connect (sendFrom) and retained the challenge + AtCommandExecutorContext context = new AtCommandExecutorContext(atSign, keys, null); + context.setChallenge("challenge"); + + AtCommandExecutor executor = TestExecutorBuilder.builder() + .stub("scan", "data:[\"signing_publickey@alice\"]") + .stub("cram:7e91508d5.+", "data:success") + .stub("enroll:request.+", "data:{\"enrollmentId\":\"904dcbf7\",\"status\":\"approved\"}") + .stub("from:@alice", "data:challenge2") + .stub("pkam:[^{].+", "data:success") + .stub("update:public:publickey@alice .+", "data:1") + .build(); + + EnrollCommands.onboard(executor, context, "secret", "app", "device", false); + + // CRAM reused the retained challenge, so exactly ONE from: reaches the wire — PKAM's own (the + // challenge is single-use and PKAM authenticates with the freshly-enrolled keys) + verify(executor, times(1)).sendSync(matches("from:.*")); + verify(executor, times(1)).sendSync(matches("cram:.*")); + assertThat(context.consumeChallenge(), is(nullValue())); + } + @Test public void testEnroll() throws Exception { AtSign atSign = createAtSign("@alice");