feat(auth): send from: on connect and reuse its challenge

[gkc]

- What I did
For a new connection, the first command sent should be from: - this is so that the client can communicate successfully via proxies / gateways etc.

- How I did it
The executor now issues from:@atsign as its first command once the connection is ready, so the connection's atSign is established up front and the server's challenge is retained. CRAM/PKAM authentication reuses that challenge instead of sending a second from:, saving a round trip.

The challenge is single-use — getFromChallenge() consumes it atomically, so a second authentication on the same connection (onboarding does CRAM then PKAM) gets null and falls back to issuing its own from:. It is also cleared on disconnect, since a from: challenge is only valid for the server session that issued it. Adds NettyAtCommandExecutorTest coverage for the challenge lifecycle: retained and consumed once, absent with no atSign, cleared on disconnect, and refreshed on reconnect.

- How to verify it
Tests pass

[akafredperry]

I can see what the objective is and what you did clearly works but I wonder if there is a simpler way.

Namely leaving the executor as is. But just adding a default onReady consumer when an atsign is specified (and no onReady consumer has been set).

This default onReady consumer would send the from command and store the challenge.

The changes to the CRAM and PKAM commands would then check for a stored challenge in their implementation.

We would need to decide where the appropriate place to cache the challenge is, I don't think it's the command executor.

One option would be AtKeys as an instance is already provided to the auth command implementations.
AtKeys already supports a transient map for caching keys, using that would be the least amount of changes.
BUT
If we feel that is an abuse of the design we could add an explicit authChallenge field to AtKeys (perhaps still yuck).
OR
Create a new context class that "gathers together" all the current (and future) contexty stuff (e.g. AtSign, AtKeys, connection config and now authChallenge).

The latter also has the advantage of simplifying and homogenizing the existing command method signatures.

This context instance would "live in" AtClientImpl.

The place to wire this default in is AtCommandExecutors.java#createOnReady()

This already adds a default onReady if no onReady is provided to the builder.

At the moment that is a pkamAuthenticator if atSign and keys are set or "do nothing" (talking to root server).

I think we just need a middle case which is atSign is set but keys are not set in which case, it should create this new "send from" onReady.

Happy to discuss further / help if you want

[gkc]

Thanks Fred I will noodle on that today

[gkc]

@akafredperry i made an attempt along the lines you suggested here and in chat ... ptal

[akafredperry]

Since we are using Lombok, I would recommend for this class too (as will remove boiler plate). Something like this...

@Value
public class AtCommandExecutorContext {

   AtSign atSign;
   AtKeys keys;
   Map<String, Object> config;

    @Getter(AccessLevel.NONE)
    @EqualsAndHashCode.Exclude
    @ToString.Exclude
    AtomicReference<String> challenge = new AtomicReference<>();

    public void setChallenge(String challenge)...
    public String consumeChallenge()...
    public void clearChallenge()...
}

[akafredperry]

This change still "lives on" the command executor rather than within the AtClientImpl as I was suggesting.

The intention was that the AtCommandExecutor(s) are solely transport specific implementations that are capable of sending commands but with no protocol knowledge.

The protocol for initial connection is currently the onReady impl.

What we currently do is almost compliant with the requirement (I think), just need to cover the case where AtCommandExecutorBuilder (in AtCommandExecutors.java) has atSign field set but not the AtKeys field.

Having said that we need to ensure that everything is using AtCommandExecutorBuilder.

[gkc]

OK .... maybe I should scrap this branch and start again