Skip to content

feat(client): choose the callback binding in Config, not on the session store - #535

Merged
osanderson merged 3 commits into
mainfrom
feat/client-callback-binding
Oct 3, 2026
Merged

osanderson merged 3 commits into
mainfrom
feat/client-callback-binding

Conversation

@osanderson

Copy link
Copy Markdown
Collaborator

Summary

These are the fixes from the security and DevX review of v0.45.0..fa120ab, before v0.46.0. There are three commits, for release-please.

feat(client): the callback binding moves to client.Config (both reviews' main point).

#532 let a session store declare storage.Capabilities.SingleUserAgent, which let a callback complete with no SessionHandle. But "only one user agent writes this store" is a fact about where a store is deployed, not about its type:

  • A SQLite store written for a mobile app and reused in a server-side web app (or wrapped by embedding) would carry the declaration with it, and silently bring login CSRF back (RFC 9700 §4.7).
  • It was also the only capability that relaxed a check rather than adding a production requirement.

It hasn't been released, so it moves now:

  • client.Config.CallbackBinding:

    • CallbackBindingSessionHandle is the zero value, unchanged behaviour: AuthorizationCallback.Session is required and compared.
    • CallbackBindingDeviceLocalStore is for a native app whose session store is its own on-device storage. The session is the callback's own state, read from the verified signed response under Message Signing.

    It's an enum (per the bool→enum rule, feat!: require explicit AuthorizationResponseIssPolicy #358), chosen per deployment by whoever wires the client. New refuses an unknown value. Its doc says never to use it with a store a server shares between users, to keep SessionLifetime short (any pending session in the store can complete), and that saving Handle().String() on the device works too.

  • storage.Capabilities.SingleUserAgent is removed.

  • The missing-Session error said "required unless … declares SingleUserAgent", which told a web developer who forgot the cookie how to switch off their CSRF protection. It now points at the SessionHandle and client/sessioncookie.

fix(client): bounded error text.

  • A malformed error code (not RFC 6749 error text) is cut to 64 bytes before being quoted into Error(), so only bounded server text reaches it.
  • The client.Error type doc no longer calls PublicDescription safe to show a user, since it can be the server's own error_description.

docs:

  • A "Native apps" section in client's package doc ties together the native story from v0.44–v0.46:

    • registration as native, and RedirectPort;
    • platform-keystore keys via keys.NewKeyManagerFromSigners + DeclareCustody(Durable);
    • the on-device session store and its locking;
    • completing after relaunch;
    • TokenSetSealer;
    • mapping errors to Code()/ServerResponse() before gomobile turns them into an NSError.

    GETTING_STARTED links to it.

  • Stale statements fixed:

    • client/doc.go and ARCHITECTURE no longer say the SessionHandle is always required.
    • Dependencies.Sessions states its production requirements.
    • KeyCustody.Durable says a native key is kept until the tokens bound to it are discarded, not until "the flow ends", which could be read as deleting the DPoP key at code exchange.

Tests

  • TestCallbackWithoutSessionForADeviceLocalStore: completion with no Session works under CallbackBindingDeviceLocalStore, both plain and JARM.
  • TestCallbackWithoutSessionStillRefused:
    • the default binding requires Session;
    • a state this app never began is refused;
    • a mismatching Session is refused.
  • TestNewRefusesAnUnknownCallbackBinding.
  • TestMissingSessionErrorPointsAtTheHandle: the error names sessioncookie and doesn't mention the device-local binding.
  • TestPARErrorBoundsAMalformedCode: a 500-byte malformed code shows only its first 64 bytes.
  • Mutation checks: each of these fails a test:
    • ignoring the binding;
    • deriving the session by default;
    • overriding a given handle;
    • accepting an unknown binding;
    • dropping the truncation.
  • Other checks:
    • go test -race across client, storage, keys, fapitest and server passes.
    • go test ./cmd/... passes.
    • Every demo's tests pass.
    • golangci-lint is clean.
    • The package doc renders the new heading.

🤖 Generated with Claude Code

osanderson and others added 3 commits October 3, 2026 22:07
…on store

#532 let a session store declare storage.Capabilities.SingleUserAgent,
which allowed a callback to complete with no SessionHandle. But "only one
user agent writes this store" is a fact about where a store is deployed,
not about its type. A store written for a mobile app and reused in a
server-side web app would carry the declaration with it and silently
bring login CSRF back. The declaration was also the only capability that
relaxed a check rather than adding a requirement. It hasn't been
released, so it moves now:

- client.Config.CallbackBinding: CallbackBindingSessionHandle (the zero
  value, requiring AuthorizationCallback.Session as before) or
  CallbackBindingDeviceLocalStore (for a native app whose session store
  is its own on-device storage: the session is the callback's own state,
  taken from the verified signed response under Message Signing). It's
  an enum, set by whoever wires the client for a deployment, and New
  refuses an unknown value.
- storage.Capabilities.SingleUserAgent is removed.
- The missing-Session error no longer advertises turning the binding
  off. Its usual cause is a web client that forgot its cookie, so it
  points at the SessionHandle and client/sessioncookie.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An authorization server's error code that isn't RFC 6749 error text was
quoted into Error() at any length, up to the response size limit. It's
now cut to 64 bytes, so only bounded text from the server reaches
Error(). The client.Error type doc no longer calls PublicDescription
safe to show a user: it can be the server's own error_description.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
client's package doc gains a Native apps section: registration as a
native app, RedirectPort, platform-keystore keys declared Durable,
the on-device session store and its locking, completing after
relaunch (CallbackBindingDeviceLocalStore, or a saved handle),
TokenSetSealer, and mapping errors to Code() and ServerResponse()
before they reach platform code. GETTING_STARTED points to it.
client/doc.go and ARCHITECTURE no longer say the SessionHandle is
always required. Dependencies.Sessions states its production
requirements. KeyCustody.Durable now says a native key is kept until
the tokens bound to it are discarded, not until the flow ends.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@osanderson
osanderson force-pushed the feat/client-callback-binding branch from f0a718c to e7720a6 Compare October 3, 2026 14:07
@codecov

codecov Bot commented Oct 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@sonarqubecloud

sonarqubecloud Bot commented Oct 3, 2026

Copy link
Copy Markdown

@osanderson
osanderson merged commit ef9726e into main Oct 3, 2026
17 checks passed
@osanderson
osanderson deleted the feat/client-callback-binding branch October 3, 2026 14:12
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.

1 participant