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

ARCHITECTURE.md

@@ -171,7 +171,11 @@ belongs to the user agent that began the flow — the caller passes back
 the `SessionHandle` it bound to that browser (an HttpOnly cookie;
 `client/sessioncookie` sets and reads one), and a
 callback whose `state` doesn't match it is rejected before anything is
-consumed, closing login CSRF ([RFC 9700 §4.7][bcp]) — then correlation
+consumed, closing login CSRF ([RFC 9700 §4.7][bcp]); a native app whose
+session store is its own on-device storage has that binding already,
+and with `Config.CallbackBinding` set to `CallbackBindingDeviceLocalStore`
+completes from the callback alone, `state` taken from the verified
+signed response under Message Signing — then correlation
 state, issuer, JARM signature and claims, audience, expiry, response
 mode, authorization-code presence, error-response integrity, and replay.
 

GETTING_STARTED.md

@@ -124,7 +124,10 @@ that listens on a port the operating system picks for each flow sets
 `Config.RedirectURI` to the port-less loopback URI and passes the port
 as `client.BeginAuthorizationRequest.RedirectPort`. A native client still authenticates like any
 other: give each app instance its own credentials, as attestation-based
-client authentication does.
+client authentication does. The rest of the native app's side — keys in
+the platform key store, an on-device session store, completing after
+the app is relaunched, token storage — is in `client`'s package doc,
+under "Native apps".
 
 ## 4. Wire `Dependencies` and construct the server
 

client/authorization_response.go

@@ -24,19 +24,12 @@ type AuthorizationCallback struct {
 	// Session is the SessionHandle BeginAuthorization returned for this
 	// attempt, recovered from wherever the caller bound it to the user
 	// agent (see SessionHandle) — never from the callback itself.
-	// Required, unless Dependencies.Sessions declares
-	// storage.Capabilities.SingleUserAgent: a callback whose "state"
-	// doesn't match it is rejected before its session is consumed, so a
-	// callback URL delivered to a different browser can't complete
-	// someone else's flow.
-	//
-	// A native app whose session store is its own on-device storage —
-	// declaring SingleUserAgent — may leave it empty: the session is
-	// then the callback's own state, taken from the verified signed
-	// response under Message Signing. Nothing else writes that store, so
-	// a session found there was begun by this app. That is what lets an
-	// app the operating system stopped mid-authorization complete it
-	// after relaunching, with no handle kept in memory.
+	// Required — a callback whose "state" doesn't match it is rejected
+	// before its session is consumed, so a callback URL delivered to a
+	// different browser can't complete someone else's flow — unless
+	// Config.CallbackBinding is CallbackBindingDeviceLocalStore, for a
+	// native app whose session store is its own on-device storage: see
+	// that constant.
 	Session SessionHandle
 }
 
@@ -178,13 +171,13 @@ func (c *Client) consumeCallbackSession(ctx context.Context, cb AuthorizationCal
 	// §4.7) before consuming anything: a mismatch leaves the session
 	// intact for its rightful browser.
 	session := cb.Session.value
-	if session == "" && c.sessionsSingleUserAgent() {
-		// The store holds only this user agent's sessions: one found by
-		// the callback's state is this user agent's.
+	if session == "" && c.cfg.CallbackBinding == CallbackBindingDeviceLocalStore {
+		// The store holds only this app's sessions: one found by the
+		// callback's state is this app's.
 		session = state
 	}
 	if session == "" {
-		return sessionRecord{}, newError(ErrorInvalidRequest, "callback is not bound to a session: AuthorizationCallback.Session is required unless Dependencies.Sessions declares storage.Capabilities.SingleUserAgent", nil)
+		return sessionRecord{}, newError(ErrorInvalidRequest, "callback is not bound to a session: pass the SessionHandle kept with this user agent as AuthorizationCallback.Session (client/sessioncookie keeps it for a browser)", nil)
 	}
 	if subtle.ConstantTimeCompare([]byte(state), []byte(session)) != 1 {
 		return sessionRecord{}, newError(ErrorInvalidRequest, "callback state does not match this user agent's session", nil)
@@ -306,10 +299,3 @@ func paramString(params map[string]json.RawMessage, key string) (string, bool) {
 	}
 	return s, true
 }
-
-// sessionsSingleUserAgent reports whether Dependencies.Sessions declares
-// storage.Capabilities.SingleUserAgent.
-func (c *Client) sessionsSingleUserAgent() bool {
-	a, ok := c.deps.Sessions.(storage.StoreAssurance)
-	return ok && a.Capabilities().SingleUserAgent
-}

client/single_user_agent_test.go → client/callback_binding_test.go

@@ -2,31 +2,22 @@ package client_test
 
 import (
 	"context"
+	"strings"
 	"testing"
 
 	"github.com/idfoundry/fapigo/client"
-	"github.com/idfoundry/fapigo/storage"
-	"github.com/idfoundry/fapigo/storage/memstore"
 )
 
-// appSessionStore is a native app's own session store: nothing but
-// this app writes it, which it declares.
-type appSessionStore struct{ storage.SessionStore }
-
-func (appSessionStore) Capabilities() storage.Capabilities {
-	return storage.Capabilities{SingleUserAgent: true}
-}
-
-// TestCallbackWithoutSessionForASingleUserAgentStore covers an app the
+// TestCallbackWithoutSessionForADeviceLocalStore covers an app the
 // operating system stopped mid-authorization: relaunched, it holds no
-// SessionHandle, and completes from the callback alone, its store
-// declaring it holds only this app's sessions. Under Message Signing the
-// state comes from the verified signed response.
-func TestCallbackWithoutSessionForASingleUserAgentStore(t *testing.T) {
+// SessionHandle, and completes from the callback alone, configured with
+// CallbackBindingDeviceLocalStore. Under Message Signing the state comes
+// from the verified signed response.
+func TestCallbackWithoutSessionForADeviceLocalStore(t *testing.T) {
 	for name, messageSigned := range map[string]bool{"plain": false, "JARM": true} {
 		t.Run(name, func(t *testing.T) {
-			c, as, _ := newTestClientWith(t, messageSigned, func(_ *client.Config, d *client.Dependencies) {
-				d.Sessions = appSessionStore{memstore.NewSessionStore()}
+			c, as, _ := newTestClientWith(t, messageSigned, func(cfg *client.Config, _ *client.Dependencies) {
+				cfg.CallbackBinding = client.CallbackBindingDeviceLocalStore
 			})
 			ctx := context.Background()
 			session, err := c.BeginAuthorization(ctx, client.BeginAuthorizationRequest{Scope: []string{"openid", "accounts"}})
@@ -45,13 +36,13 @@ func TestCallbackWithoutSessionForASingleUserAgentStore(t *testing.T) {
 	}
 }
 
-// TestCallbackWithoutSessionStillRefused covers what the declaration
-// doesn't change: a store that doesn't declare it still needs the
-// handle, a callback whose state no session in the store has is still
-// refused, and a handle given is still compared.
+// TestCallbackWithoutSessionStillRefused covers what the binding
+// doesn't change: the default still needs the handle, a callback whose
+// state no session in the store has is still refused, and a handle
+// given is still compared.
 func TestCallbackWithoutSessionStillRefused(t *testing.T) {
 	ctx := context.Background()
-	t.Run("store not declaring it", func(t *testing.T) {
+	t.Run("the default binding", func(t *testing.T) {
 		c, as, _ := newTestClient(t, false)
 		session, err := c.BeginAuthorization(ctx, client.BeginAuthorizationRequest{Scope: []string{"openid"}})
 		if err != nil {
@@ -63,8 +54,8 @@ func TestCallbackWithoutSessionStillRefused(t *testing.T) {
 		}
 	})
 	declaring := func(t *testing.T) (*client.Client, *fakeAS) {
-		c, as, _ := newTestClientWith(t, false, func(_ *client.Config, d *client.Dependencies) {
-			d.Sessions = appSessionStore{memstore.NewSessionStore()}
+		c, as, _ := newTestClientWith(t, false, func(cfg *client.Config, _ *client.Dependencies) {
+			cfg.CallbackBinding = client.CallbackBindingDeviceLocalStore
 		})
 		return c, as
 	}
@@ -93,3 +84,26 @@ func TestCallbackWithoutSessionStillRefused(t *testing.T) {
 		}
 	})
 }
+
+func TestNewRefusesAnUnknownCallbackBinding(t *testing.T) {
+	cfg, deps := validConfig(t), validDependencies(t)
+	cfg.CallbackBinding = 9
+	if _, err := client.New(cfg, deps); err == nil {
+		t.Error("New accepted an unknown CallbackBinding")
+	}
+}
+
+// TestMissingSessionErrorPointsAtTheHandle covers the refusal a web
+// client that forgot its cookie meets: it says to pass the handle, and
+// doesn't advertise turning the binding off.
+func TestMissingSessionErrorPointsAtTheHandle(t *testing.T) {
+	c, as, _ := newTestClient(t, false)
+	session, err := c.BeginAuthorization(context.Background(), client.BeginAuthorizationRequest{Scope: []string{"openid"}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = c.CompleteAuthorization(context.Background(), client.AuthorizationCallback{RawQuery: as.callbackFor(t, session.Handle().String(), "c", "")})
+	if err == nil || !strings.Contains(err.Error(), "sessioncookie") || strings.Contains(err.Error(), "DeviceLocal") {
+		t.Errorf("error = %v, want it to point at the handle and sessioncookie, not the device-local binding", err)
+	}
+}

client/client.go

@@ -218,6 +218,9 @@ func validateAuthMethodAlgorithms(cfg Config) error {
 	// Algorithms.DPoP is required only under SenderConstrainDPoP (the
 	// default) — an mTLS-sender-constrained client never builds a DPoP
 	// proof at all, so it never needs a DPoP signing algorithm.
+	if !cfg.CallbackBinding.IsValid() {
+		return fmt.Errorf("client: config: callback_binding %d is not a CallbackBinding", cfg.CallbackBinding)
+	}
 	if cfg.SenderConstrain == storage.SenderConstrainDPoP && !cfg.Algorithms.DPoP.IsValid() {
 		return fmt.Errorf("client: config: algorithms.dpop is required when sender_constrain is SenderConstrainDPoP")
 	}

client/config.go

@@ -381,6 +381,12 @@ type Config struct {
 	// names.
 	RedirectURI string
 
+	// CallbackBinding is how an authorization callback is tied to the
+	// user agent that began it (RFC 9700 §4.7) — see CallbackBinding.
+	// The zero value, CallbackBindingSessionHandle, is right for every
+	// browser-based client.
+	CallbackBinding CallbackBinding
+
 	Endpoints  Endpoints
 	Profile    Profile
 	Algorithms Algorithms
@@ -549,3 +555,42 @@ type FederationConfig struct {
 	// key. Required when EntityID is set.
 	Algorithm fapi.SignatureAlgorithm
 }
+
+// CallbackBinding is how HandleAuthorizationResponse ties a callback to
+// the user agent that began its authorization, against login CSRF (RFC
+// 9700 §4.7): the attacker starts a flow as themselves and delivers its
+// callback to a victim, whose user agent would complete it.
+type CallbackBinding uint8
+
+const (
+	// CallbackBindingSessionHandle requires AuthorizationCallback.Session:
+	// the SessionHandle the application kept with the user agent that
+	// began the flow (a cookie; client/sessioncookie). A callback whose
+	// state doesn't match it is refused. The zero value, and the only
+	// right choice wherever Dependencies.Sessions holds more than one
+	// user agent's sessions — any server-side store — since an attacker's
+	// own session is found there by its state too.
+	CallbackBindingSessionHandle CallbackBinding = iota
+
+	// CallbackBindingDeviceLocalStore is for a native app whose
+	// Dependencies.Sessions is its own storage on the device, which
+	// nothing but this app writes. A session found there by a callback's
+	// state was begun by this app, so AuthorizationCallback.Session may
+	// be left empty: the session is the callback's own state, taken from
+	// the verified signed response under Message Signing. That lets an
+	// app the operating system stopped mid-authorization complete it
+	// after relaunching, with no handle kept. (An app may instead save
+	// Handle().String() on the device and pass it back, as any client
+	// does.) A Session that is given is still compared. Any authorization
+	// still pending in the store can be completed this way, not only the
+	// latest, so keep Limits.SessionLifetime short. Never set it for a
+	// store a server shares between users: it would bring login CSRF
+	// back.
+	CallbackBindingDeviceLocalStore
+)
+
+// IsValid reports whether b is one of this package's CallbackBinding
+// values.
+func (b CallbackBinding) IsValid() bool {
+	return b == CallbackBindingSessionHandle || b == CallbackBindingDeviceLocalStore
+}

client/dependencies.go

@@ -12,7 +12,11 @@ import (
 // nil value for any field — there is no implicit fallback (no default
 // clock, no silently-installed in-memory session store).
 type Dependencies struct {
-	// Sessions persists in-progress authorization-flow state.
+	// Sessions persists in-progress authorization-flow state, until
+	// Limits.SessionLifetime. Under AssuranceProduction it must declare
+	// Durable and AtomicConsume (storage.StoreAssurance). A native app's
+	// own on-device store: see "Native apps" in the package doc, and
+	// Config.CallbackBinding.
 	Sessions storage.SessionStore
 
 	// Keys performs this client's own signing operations: client

client/doc.go

@@ -73,7 +73,9 @@
 // its own String form (ParseSessionHandle) — the caller stores it with
 // the user agent that began the flow (package client/sessioncookie does
 // this), and HandleAuthorizationResponse
-// rejects a callback that doesn't carry the matching one;
+// rejects a callback that doesn't carry the matching one — unless
+// Config.CallbackBinding is CallbackBindingDeviceLocalStore, for a
+// native app whose session store is its own (see "Native apps" below);
 // HandleAuthorizationResponse returns a closed sum type
 // rather than one struct with optional fields, so a caller can't assume
 // every callback carries a code; every DPoP proof, request-object
@@ -87,4 +89,43 @@
 // error response, when there was one, available through
 // Error.ServerResponse — not a bare error the caller has to
 // string-match.
+//
+// # Native apps
+//
+// A mobile or desktop app — a wallet, say — uses this package as any
+// client does, with these differences:
+//
+//   - Registration. The authorization server registers the client as a
+//     native app (storage.ApplicationTypeNative), so it may use a
+//     private-use scheme redirect, written with a single slash
+//     (com.example.wallet:/callback), or loopback http to 127.0.0.1 or
+//     [::1]. For loopback on a port the operating system picks per flow,
+//     set Config.RedirectURI without a port and pass the port as
+//     BeginAuthorizationRequest.RedirectPort. The app still
+//     authenticates: give each installation its own credentials, as
+//     attestation-based client authentication does.
+//   - Keys. Hold the DPoP and attestation keys in the platform's key
+//     store (the iOS Keychain or Secure Enclave, the Android Keystore)
+//     behind crypto.Signer, through keys.NewKeyManagerFromSigners with
+//     keys.DeclareCustody(keys.KeyCustody{Durable: true}) — see
+//     KeyCustody.Durable. Keep each key until the tokens bound to it are
+//     discarded. The Secure Enclave signs ES256 only.
+//   - Sessions. Dependencies.Sessions is the app's own durable storage
+//     (a file, SQLite) in its data container, declaring Durable and
+//     AtomicConsume (storage.Capabilities). A mutex makes Consume
+//     atomic for one process; an extension or widget sharing the
+//     container needs a file lock or a transaction.
+//   - Relaunch. If the operating system stops the app while the user is
+//     at the authorization server, the callback reaches a fresh process.
+//     Build the Client again from the same Config, keys and session
+//     store, and call CompleteAuthorization with the callback's
+//     RawQuery. Set Config.CallbackBinding to
+//     CallbackBindingDeviceLocalStore so it needs no SessionHandle, or
+//     save Handle().String() on the device when the flow begins and
+//     pass it back.
+//   - Tokens. Keep them between launches sealed, with a TokenSetSealer.
+//   - Errors. Error() may include text the authorization server wrote.
+//     An app that hands errors to platform code — gomobile turns a Go
+//     error into an NSError with Error() as its message — should map an
+//     *Error to Code() and ServerResponse() in its own Go layer instead.
 package client

client/errors.go

@@ -21,10 +21,13 @@ const (
 	ErrorResponseTooLarge ErrorCode = "response_too_large"
 )
 
-// Error is the error type every public Client method returns. Code and
-// PublicDescription are safe to surface to the embedding application
-// (e.g. in a UI message); the underlying cause (available via Unwrap,
-// and included in Error's own message) is for logs only.
+// Error is the error type every public Client method returns. Code is
+// safe to surface to the embedding application. PublicDescription is
+// this package's own text, or, when the error came from a server's error
+// response, that server's error_description (see ServerResponse): meant
+// for an operator, not shown to a user as is. The underlying cause
+// (available via Unwrap, and included in Error's own message) is for
+// logs only.
 type Error struct {
 	code        ErrorCode
 	description string
@@ -50,7 +53,8 @@ func (e *Error) PublicDescription() string { return e.description }
 // It never includes a response body. It may include text the remote
 // server wrote: an authorization server error response's
 // error_description (PublicDescription), limited to RFC 6749's printable
-// ASCII, and its error code — quoted, when it isn't RFC 6749 error text.
+// ASCII, and its error code — quoted, and cut to 64 bytes, when it isn't
+// RFC 6749 error text.
 // Where only this client's own words may be logged, log Code() and
 // ServerResponse()'s Code and HTTPStatus instead.
 func (e *Error) Error() string {

client/server_error.go

@@ -88,6 +88,12 @@ func parErrorFromResponse(status int, body []byte) *Error {
 	resp := newServerErrorResponse(status, errResp.Code, errResp.Description, errResp.URI)
 	code := errResp.Code
 	if resp.Code == "" {
+		// Quoted, and cut short: only bounded text from the server
+		// reaches Error().
+		const maxShown = 64
+		if len(code) > maxShown {
+			code = code[:maxShown] + "…"
+		}
 		code = strconv.Quote(code)
 	}
 	return newError(ErrorInvalidResponse, resp.Description, fmt.Errorf("authorization server error: %s", code)).

client/server_error_test.go

@@ -301,3 +301,17 @@ func TestCallbackDeniedEnforcesErrorCharacterSet(t *testing.T) {
 		})
 	}
 }
+
+// TestPARErrorBoundsAMalformedCode covers an error code that isn't RFC
+// 6749 error text and is long: Error() shows only its first 64 bytes.
+func TestPARErrorBoundsAMalformedCode(t *testing.T) {
+	long := "bad\n" + strings.Repeat("x", 500)
+	c := newPARErrorTestClient(t, func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusBadRequest)
+		_ = json.NewEncoder(w).Encode(map[string]string{"error": long})
+	})
+	if msg := beginAuthorizationError(t, c).Error(); strings.Contains(msg, strings.Repeat("x", 100)) || !strings.Contains(msg, "xxx") {
+		t.Errorf("Error() = %q, want the malformed code cut to 64 bytes", msg)
+	}
+}

client/session.go

@@ -41,9 +41,8 @@ func generateRandomToken(random io.Reader) (string, error) {
 // HandleAuthorizationResponse and CompleteAuthorization reject a
 // callback whose Session doesn't match its "state".
 //
-// A native app whose session store is its own on-device storage, and
-// declares storage.Capabilities.SingleUserAgent, has that binding
-// already: see AuthorizationCallback.Session.
+// A native app whose session store is its own on-device storage has that
+// binding already: see CallbackBindingDeviceLocalStore.
 type SessionHandle struct {
 	value string
 }

keys/custody.go

@@ -17,10 +17,10 @@ type KeyCustody struct {
 	// For a client, it means a key outlives the process that made it
 	// until the client is done with it: an authorization in progress can
 	// be completed after a restart, and a DPoP-bound access or refresh
-	// token stays usable. A native app's keys created for one flow and
-	// deleted when it ends, but kept meanwhile in the platform's key
-	// store (the iOS Keychain or Secure Enclave, the Android Keystore),
-	// are Durable in this sense.
+	// token stays usable. A native app's keys created for one issuance,
+	// kept in the platform's key store (the iOS Keychain or Secure
+	// Enclave, the Android Keystore) and deleted only once the tokens
+	// bound to them are discarded, are Durable in this sense.
 	Durable bool
 
 	// CrossInstanceConsistent is true iff every instance of a