server test to verify auth callout in operator mode & default sentinel still preserves nonce challenge flow#8024
Conversation
Signed-off-by: Borja Etxebarria <borja.bit@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 586265f9f5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e8af2c9a7a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| require_NoError(t, err) | ||
| nkPub, err := nkUsr.PublicKey() | ||
| require_NoError(t, err) | ||
| nc = ac.Connect(nats.Token(nonceChallengeToken), nats.Nkey(nkPub, func(nonce []byte) ([]byte, error) { |
There was a problem hiding this comment.
Remove explicit token to exercise default_sentinel path
This new nonce-challenge assertion does not actually cover the default_sentinel flow it describes, because the connection still sends an explicit token (nats.Token(nonceChallengeToken)). If server-side default-sentinel injection regresses for clients that omit token/JWT, this test would keep passing since it never exercises that code path. The scenario under test should connect with only the NKey option (no explicit token) and validate the callout still receives the expected sentinel and signed nonce.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Token, or user, or password can be included or not, as these are independent from the sentinel logic - default sentinel just requires that the client does not include a JWT.
In this case the token is just included internally by the test to trigger multiple sub-tests.
|
@codex re-review |
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@claude, review. |
This comment was marked as resolved.
This comment was marked as resolved.
MauriceVanVeen
left a comment
There was a problem hiding this comment.
LGTM, but could the commits be squashed down into one?
AuthCallout in Operator mode + nkey challenge
Decentralized authentication (operator mode) uses nkeys + nonce challenge like static nkeys authentication, but ties them to a trust chain:
nkey → user JWT → account JWT → operator
Clients must provide:
This could be done using a creds file or a jwt+seed
https://pkg.go.dev/github.com/nats-io/nats.go#UserCredentials
https://pkg.go.dev/github.com/nats-io/nats.go#UserJWTAndSeed
Those are convenience wrappers on top of:
https://pkg.go.dev/github.com/nats-io/nats.go#UserJWT
Auth Callout in Operator Mode - constraint: Sentinel JWT vs NKey
Clients must normally send a sentinel JWT (a bearer token, typically non-expiring) to select the appropriate auth callout flow.
However, per the client API:
https://pkg.go.dev/github.com/nats-io/nats.go#UserJWT
The UserJWT option and Nkey option are mutually exclusive.
This means we cannot both send a sentinel JWT, and use nkey-based nonce authentication.
The Loophole: default_sentinel
With the introduction of the default_sentinel option:
#6577
The client no longer needs to send the sentinel JWT explicitly. The server applies a default sentinel internally
So the client can use an nkey and still benefit from nonce challenge authentication
This effectively enables an authentication flow with auth callout that still leverages the nonce challenge.
Signed-off-by: Borja Etxebarria borja.bit@gmail.com