Skip to content

Fix nonce signing when auth_required is omitted#1109

Merged
mtmk merged 1 commit into
mainfrom
fix/nkey-sign-without-auth-required
Apr 16, 2026
Merged

Fix nonce signing when auth_required is omitted#1109
mtmk merged 1 commit into
mainfrom
fix/nkey-sign-without-auth-required

Conversation

@mtmk

@mtmk mtmk commented Apr 15, 2026

Copy link
Copy Markdown
Member

When the server is configured with no_auth_user, it omits auth_required from the INFO message but still sends a nonce. The client was gating signature generation on AuthRequired: true, so an NKey-configured client would send nkey without sig and the server would reject the connection with Authorization Violation. Other clients (Go, Python) sign whenever a nonce is present; this aligns the .NET client with that behaviour.

Fixes #1087

Servers with no_auth_user defined omit auth_required from INFO but
still send a nonce. NKey-configured clients must respond with a
signature or the server rejects the connection.
@synadia-claude-reviewer

synadia-claude-reviewer Bot commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

Claude finished @mtmk's task in 2m 3s —— View job


PR Review: Fix nonce signing when auth_required is omitted

  • Gather context (CLAUDE.md, PR diff)
  • Review changed files
  • Post final review summary

Summary

The fix is correct, minimal, and well-targeted. No significant issues found.

Analysis

Root cause confirmed

AuthRequired is a non-nullable bool in ServerInfo (defaults to false when the field is absent from JSON). When a server is configured with no_auth_user, it omits auth_required from the INFO payload, so AuthRequired deserialises as false. The old guard info is { AuthRequired: true, Nonce: { } } therefore never fires, causing the client to omit sig while still sending nkey.

Change is safe

Sign() already returns null when seed == null || nonce == null (UserCredentials.cs:53–54), so:

  • Token-only clients: this.Seed == nullSign() returns nullopts.Sig = null. No accidental signing.
  • NKey clients (direct opts path): seed stays null in the local variable but Sign() falls back to this.Seed. If this.Seed is configured, it signs; otherwise it doesn't. Correct either way.
  • Creds (JWT+seed) clients: will now sign when a nonce is present even if auth_required is absent. This is consistent with Go/Python behaviour and is actually more correct — a creds client should authenticate as its configured user, not silently fall through to the no_auth_user account.

Test

The new NKeyAuthWithNoAuthUserTest covers the exact failure scenario (anon connection via no_auth_user + NKey-authenticated connection) using the same key material already established in the existing nkey test suite. No concerns.

Config file

nkey-no-auth-user.conf correctly omits authorization.required at the top level (letting no_auth_user govern access) while still requiring NKey auth for the AUTH account users. The public key in the conf matches the NKey used in the test.

No issues found

This is a clean, one-line logic fix backed by a focused integration test, consistent with how the Go and Python clients handle the same case.

@github-actions

Copy link
Copy Markdown

@mtmk
mtmk requested a review from scottf April 15, 2026 13:17

@Jarema Jarema left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@mtmk
mtmk merged commit cae0ff6 into main Apr 16, 2026
32 of 34 checks passed
@mtmk
mtmk deleted the fix/nkey-sign-without-auth-required branch April 16, 2026 08:10
This was referenced Apr 16, 2026
@mtmk mtmk mentioned this pull request May 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing signature in connect message when server no_auth_user is defined

2 participants