From 4716260e833322cb0d7caf93cad39cacbea8ff97 Mon Sep 17 00:00:00 2001 From: Oscar Sanderson Date: Fri, 28 Aug 2026 00:17:49 +0800 Subject: [PATCH 1/3] feat: add storage.NonceStore and surface a DPoP proof's nonce claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Foundational pieces for DPoP nonce-challenge support (RFC 9449 §8, §9): a single-use NonceStore contract (storage package) plus its in-memory reference implementation, mirroring SessionStore/ReplayStore's own issue-once/consume-once shape exactly. internal/dpop.VerifiedProof now also surfaces the proof's own "nonce" claim unconditionally — unlike RequiredNonce's compare-to-one-known-value check, a caller implementing single-use, issued-per-challenge nonces doesn't know the expected value in advance and has to look up whether the presented one was actually issued, which only that caller can do. Co-Authored-By: Claude Sonnet 5 --- internal/dpop/dpop_test.go | 41 +++++++++++++++++++++ internal/dpop/verify.go | 13 ++++++- storage/contract.go | 61 +++++++++++++++++++++++++++++++ storage/memstore/memstore_test.go | 6 +++ storage/memstore/nonce_store.go | 43 ++++++++++++++++++++++ storage/nonce.go | 46 +++++++++++++++++++++++ 6 files changed, 208 insertions(+), 2 deletions(-) create mode 100644 storage/memstore/nonce_store.go create mode 100644 storage/nonce.go diff --git a/internal/dpop/dpop_test.go b/internal/dpop/dpop_test.go index 31343aa..8341f33 100644 --- a/internal/dpop/dpop_test.go +++ b/internal/dpop/dpop_test.go @@ -283,6 +283,47 @@ func TestVerifyNonceBinding(t *testing.T) { } } +// TestVerifyReturnsProofNonce confirms VerifiedProof.Nonce surfaces the +// proof's own "nonce" claim unconditionally — even with no RequiredNonce +// set — since a caller implementing single-use, issued-per-challenge +// nonces (unlike RequiredNonce's compare-to-one-known-value check) needs +// the presented value itself to look up, not just a pass/fail verdict. +func TestVerifyReturnsProofNonce(t *testing.T) { + key := generateKey(t) + now := time.Now() + target := mustURL(t, "https://as.example/token") + + withNonce, err := CreateProof(ProofRequest{ + Signer: key, Algorithm: fapi.ES256, Method: "POST", URL: target, Now: now, Nonce: "presented-nonce", + }) + if err != nil { + t.Fatalf("CreateProof: %v", err) + } + verified, err := Verify(context.Background(), VerifyRequest{ + Proof: withNonce, Method: "POST", URL: target, Now: now, MaxProofAge: time.Minute, + }) + if err != nil { + t.Fatalf("Verify(with nonce): %v", err) + } + if verified.Nonce != "presented-nonce" { + t.Fatalf("VerifiedProof.Nonce = %q, want %q", verified.Nonce, "presented-nonce") + } + + withoutNonce, err := CreateProof(ProofRequest{Signer: key, Algorithm: fapi.ES256, Method: "POST", URL: target, Now: now}) + if err != nil { + t.Fatalf("CreateProof: %v", err) + } + verified, err = Verify(context.Background(), VerifyRequest{ + Proof: withoutNonce, Method: "POST", URL: target, Now: now, MaxProofAge: time.Minute, + }) + if err != nil { + t.Fatalf("Verify(without nonce): %v", err) + } + if verified.Nonce != "" { + t.Fatalf("VerifiedProof.Nonce = %q, want empty", verified.Nonce) + } +} + func TestVerifyRequiresMaxProofAge(t *testing.T) { key := generateKey(t) now := time.Now() diff --git a/internal/dpop/verify.go b/internal/dpop/verify.go index 86eb1bf..5e42e9a 100644 --- a/internal/dpop/verify.go +++ b/internal/dpop/verify.go @@ -63,10 +63,19 @@ type VerifyRequest struct { // VerifiedProof is everything about a DPoP proof worth retaining once it // has been verified: the thumbprint of the key it was signed with (for -// binding against a token's cnf.jkt) and when it was issued. +// binding against a token's cnf.jkt), when it was issued, and the +// proof's own "nonce" claim (empty if absent). +// +// Nonce is handed back unconditionally, regardless of RequiredNonce — +// unlike RequiredNonce's own compare-to-one-known-value check, a caller +// implementing single-use, issued-per-challenge nonces (RFC 9449 §8, +// §9) doesn't know the expected value in advance; it has to look up +// whether the presented value was one it actually issued, which only it +// (not this package) can do. type VerifiedProof struct { Thumbprint jose.Thumbprint IssuedAt time.Time + Nonce string } // Verify checks a DPoP proof against req. On success, the returned @@ -157,5 +166,5 @@ func Verify(ctx context.Context, req VerifyRequest) (VerifiedProof, error) { if err != nil { return VerifiedProof{}, fmt.Errorf("dpop: %w", err) } - return VerifiedProof{Thumbprint: thumbprint, IssuedAt: iat}, nil + return VerifiedProof{Thumbprint: thumbprint, IssuedAt: iat, Nonce: c.Nonce}, nil } diff --git a/storage/contract.go b/storage/contract.go index 2b37711..d577564 100644 --- a/storage/contract.go +++ b/storage/contract.go @@ -635,6 +635,67 @@ func TestSessionStoreContract(t *testing.T, factory func() SessionStore) { }) } +// TestNonceStoreContract exercises factory()'s behavior against the +// guarantees NonceStore's documentation promises: an issued nonce is +// atomically single-use, an unknown nonce is rejected, and concurrent +// consumption of the same nonce has exactly one winner. factory must +// return a fresh, empty NonceStore each call. +func TestNonceStoreContract(t *testing.T, factory func() NonceStore) { + t.Helper() + + t.Run("IssueThenConsumeRoundTripsExpiry", func(t *testing.T) { + store := factory() + ctx := context.Background() + expiresAt := time.Now().Add(time.Minute).Truncate(time.Second) + if err := store.Issue(ctx, NonceIssuance{Nonce: "nonce-1", ExpiresAt: expiresAt}); err != nil { + t.Fatalf("Issue: %v", err) + } + got, err := store.Consume(ctx, NonceConsumption{Nonce: "nonce-1"}) + if err != nil { + t.Fatalf("Consume: %v", err) + } + if !got.ExpiresAt.Equal(expiresAt) { + t.Fatalf("Consume returned ExpiresAt %v, want %v", got.ExpiresAt, expiresAt) + } + }) + + t.Run("ConsumeIsSingleUse", func(t *testing.T) { + store := factory() + ctx := context.Background() + if err := store.Issue(ctx, NonceIssuance{Nonce: "nonce-2", ExpiresAt: time.Now().Add(time.Minute)}); err != nil { + t.Fatalf("Issue: %v", err) + } + if _, err := store.Consume(ctx, NonceConsumption{Nonce: "nonce-2"}); err != nil { + t.Fatalf("first Consume: %v", err) + } + if _, err := store.Consume(ctx, NonceConsumption{Nonce: "nonce-2"}); err == nil { + t.Fatalf("second Consume = nil error, want error") + } + }) + + t.Run("ConsumeUnknownNonceFails", func(t *testing.T) { + store := factory() + if _, err := store.Consume(context.Background(), NonceConsumption{Nonce: "never-issued"}); err == nil { + t.Fatalf("Consume(unknown nonce) = nil error, want error") + } + }) + + t.Run("ConcurrentConsumeHasExactlyOneWinner", func(t *testing.T) { + store := factory() + ctx := context.Background() + if err := store.Issue(ctx, NonceIssuance{Nonce: "nonce-3", ExpiresAt: time.Now().Add(time.Minute)}); err != nil { + t.Fatalf("Issue: %v", err) + } + successes := runConcurrently(contractConcurrentAttempts, func() bool { + _, err := store.Consume(ctx, NonceConsumption{Nonce: "nonce-3"}) + return err == nil + }) + if successes != 1 { + t.Fatalf("concurrent Consume succeeded %d times, want exactly 1", successes) + } + }) +} + // TestAccessTokenStoreContract exercises factory()'s behavior against // the guarantees AccessTokenStore's documentation promises: a created // token's fields round-trip faithfully through LookupAccessToken, and diff --git a/storage/memstore/memstore_test.go b/storage/memstore/memstore_test.go index 28703bb..a06e0ec 100644 --- a/storage/memstore/memstore_test.go +++ b/storage/memstore/memstore_test.go @@ -33,6 +33,12 @@ func TestAccessTokenStoreContract(t *testing.T) { }) } +func TestNonceStoreContract(t *testing.T) { + storage.TestNonceStoreContract(t, func() storage.NonceStore { + return NewNonceStore() + }) +} + func TestRevocationStoreNotRevokedByDefault(t *testing.T) { s := NewRevocationStore() diff --git a/storage/memstore/nonce_store.go b/storage/memstore/nonce_store.go new file mode 100644 index 0000000..f2eb0fa --- /dev/null +++ b/storage/memstore/nonce_store.go @@ -0,0 +1,43 @@ +package memstore + +import ( + "context" + "fmt" + "sync" + + "github.com/idfoundry/fapigo/storage" +) + +// NonceStore is an in-memory storage.NonceStore. See the package doc +// comment for why this is development/testing only. +type NonceStore struct { + mu sync.Mutex + issued map[string]storage.NonceRecord +} + +// NewNonceStore builds an empty NonceStore. +func NewNonceStore() *NonceStore { + return &NonceStore{issued: make(map[string]storage.NonceRecord)} +} + +// Issue implements storage.NonceStore. +func (s *NonceStore) Issue(_ context.Context, issuance storage.NonceIssuance) error { + s.mu.Lock() + defer s.mu.Unlock() + s.issued[issuance.Nonce] = storage.NonceRecord{ExpiresAt: issuance.ExpiresAt} + return nil +} + +// Consume implements storage.NonceStore. Deleting the map entry on +// every call — whether or not it was present — is what makes this +// single-use: a second Consume of the same nonce always finds nothing. +func (s *NonceStore) Consume(_ context.Context, consumption storage.NonceConsumption) (storage.NonceRecord, error) { + s.mu.Lock() + defer s.mu.Unlock() + record, ok := s.issued[consumption.Nonce] + delete(s.issued, consumption.Nonce) + if !ok { + return storage.NonceRecord{}, fmt.Errorf("memstore: unknown or already-consumed nonce") + } + return record, nil +} diff --git a/storage/nonce.go b/storage/nonce.go new file mode 100644 index 0000000..1dc94b8 --- /dev/null +++ b/storage/nonce.go @@ -0,0 +1,46 @@ +package storage + +import ( + "context" + "time" +) + +// NonceIssuance is what Issue persists for one DPoP nonce a verifier has +// handed out — either as an RFC 9449 §8/§9 challenge, or proactively +// alongside a successful response — keyed by Nonce itself. +type NonceIssuance struct { + Nonce string + ExpiresAt time.Time +} + +// NonceConsumption is the input to NonceStore.Consume. +type NonceConsumption struct { + // Nonce is the value a presented DPoP proof's own "nonce" claim + // carried — the lookup key. + Nonce string +} + +// NonceRecord is what Consume returns for a successfully consumed +// nonce — the expiry Issue persisted, for the caller to compare against +// the time it's verifying at, the same division of responsibility every +// other store in this package uses (the store itself never judges +// expiry). +type NonceRecord struct { + ExpiresAt time.Time +} + +// NonceStore persists DPoP nonces a verifier has issued, keyed by the +// nonce value itself. Like SessionStore, it exposes no generic CRUD — +// Consume is the only way to check a nonce, and it always retires the +// record it returns. +type NonceStore interface { + Issue(ctx context.Context, issuance NonceIssuance) error + + // Consume atomically retrieves and retires the nonce identified by + // consumption.Nonce — a second call with the same value must fail, + // exactly like SessionStore.Consume, so a captured nonce can never + // be presented twice. It returns an error if the nonce is unknown or + // already consumed; the caller checks the returned record's own + // expiry (ExpiresAt) against the time it's verifying at. + Consume(ctx context.Context, consumption NonceConsumption) (NonceRecord, error) +} From 069552d5eacf051ae9d87995039eec0ffcaf9784 Mon Sep 17 00:00:00 2001 From: Oscar Sanderson Date: Fri, 28 Aug 2026 00:18:04 +0800 Subject: [PATCH 2/3] feat: add DPoP nonce-challenge support to resource.Verifier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Neither this module's resource role nor its server role has ever implemented the resource-server half of RFC 9449 §8/§9's nonce challenge — only client consumes one, from a third-party AS. Adds it to resource.Verifier as genuinely optional (Dependencies.Nonces nil disables it entirely, same as today): a request whose DPoP proof carries no current, unconsumed nonce is rejected with the new ErrorUseDPoPNonce code and a freshly issued replacement attached (via Error.Nonce); a successful Verify proactively issues another one (AuthorizationContext.NextDPoPNonce) so steady-state traffic only pays the extra round trip once, at the very first request. Deliberately not a required-with-visible-opt-out dependency like Revocation: RFC 9449 §8 makes this a resource-server MAY, and declining it is the normal, fully spec-compliant default, not a security check this module considers non-negotiable. Co-Authored-By: Claude Sonnet 5 --- resource/config.go | 7 ++ resource/dependencies.go | 27 ++++- resource/errors.go | 16 +++ resource/nonce.go | 74 ++++++++++++ resource/nonce_test.go | 234 ++++++++++++++++++++++++++++++++++++++ resource/verifier.go | 10 +- resource/verifier_test.go | 50 ++++++++ resource/verify.go | 38 ++++++- 8 files changed, 446 insertions(+), 10 deletions(-) create mode 100644 resource/nonce.go create mode 100644 resource/nonce_test.go diff --git a/resource/config.go b/resource/config.go index a43f229..39d05bb 100644 --- a/resource/config.go +++ b/resource/config.go @@ -19,6 +19,13 @@ type Limits struct { // extends how long past a token's own expiry it's still accepted. // Zero means no tolerance. MaxClockSkew time.Duration + + // DPoPNonceLifetime bounds how long an issued DPoP nonce remains + // valid. Required only when Dependencies.Nonces is non-nil — see + // its own doc comment; NewVerifier rejects a zero value in that + // case, exactly like MaxDPoPProofAge above, but leaves it + // unvalidated when nonce-challenge support is disabled. + DPoPNonceLifetime time.Duration } // Config is this verifier's immutable configuration. It is copied by diff --git a/resource/dependencies.go b/resource/dependencies.go index be88a87..d4ab100 100644 --- a/resource/dependencies.go +++ b/resource/dependencies.go @@ -1,12 +1,21 @@ package resource import ( + "io" + "github.com/idfoundry/fapigo/storage" ) // Dependencies are this verifier's injected collaborators. NewVerifier -// rejects a nil value for any field — there is no implicit fallback (no -// default clock, no silently-installed in-memory replay store). +// rejects a nil value for any field required unconditionally — there is +// no implicit fallback (no default clock, no silently-installed +// in-memory replay store). Nonces/Random are the one exception: unlike +// every other field, DPoP nonce-challenge support (RFC 9449 §8, §9) is +// genuinely optional protocol behavior, not a security check this +// module considers non-negotiable — leaving Nonces nil disables it +// entirely, with no visible-opt-out sentinel needed the way +// Revocation's NoRevocation{} is, because declining it is the normal, +// fully spec-compliant default, not a choice that needs to be visible. type Dependencies struct { // AccessTokens resolves a presented access token's claims — see // AccessTokenResolver. Required — pass JWTAccessTokens{...} (the @@ -25,4 +34,18 @@ type Dependencies struct { // Clock supplies the current time. Clock Clock + + // Nonces persists DPoP nonces this verifier issues and consumes. + // Nil disables DPoP nonce-challenge support entirely — Verify never + // requires or checks a nonce, exactly like today. Set it (and + // Random, and Config.Limits.DPoPNonceLifetime) to have Verify + // challenge a request whose proof carries no valid nonce, and + // proactively reissue a fresh one on every successful call. + Nonces storage.NonceStore + + // Random is the source of randomness for DPoP nonce generation. + // Required only when Nonces is non-nil — NewVerifier rejects nil + // Random in that case, matching server.Dependencies.Random's own + // "no implicit fallback" rule. + Random io.Reader } diff --git a/resource/errors.go b/resource/errors.go index ce95af6..7c641d0 100644 --- a/resource/errors.go +++ b/resource/errors.go @@ -11,6 +11,15 @@ const ( ErrorInvalidRequest ErrorCode = "invalid_request" ErrorInvalidToken ErrorCode = "invalid_token" ErrorServerError ErrorCode = "server_error" + + // ErrorUseDPoPNonce indicates the presented DPoP proof was otherwise + // valid but carried no nonce, or one this verifier didn't just issue + // (unknown, already consumed, or expired) — RFC 9449 §8's own error + // value, distinct from ErrorInvalidToken (the token itself is fine; + // the caller just needs to retry with the nonce this error's own + // Nonce method returns). Only ever returned when + // Dependencies.Nonces is configured. + ErrorUseDPoPNonce ErrorCode = "use_dpop_nonce" ) // Error is the error type Verify returns. Code and PublicDescription @@ -23,6 +32,7 @@ type Error struct { httpStatus int description string cause error + nonce string } func newError(code ErrorCode, httpStatus int, description string, cause error) *Error { @@ -35,6 +45,12 @@ func (e *Error) Code() ErrorCode { return e.code } // PublicDescription returns a short, safe-to-expose description. func (e *Error) PublicDescription() string { return e.description } +// Nonce returns the nonce a caller should present on retry, alongside +// this error's own DPoP challenge — non-empty only when Code is +// ErrorUseDPoPNonce, in which case it belongs in the response's own +// DPoP-Nonce header (RFC 9449 §8). +func (e *Error) Nonce() string { return e.nonce } + // HTTPStatus returns the HTTP status code an adapter should respond // with. func (e *Error) HTTPStatus() int { return e.httpStatus } diff --git a/resource/nonce.go b/resource/nonce.go new file mode 100644 index 0000000..89d1cf3 --- /dev/null +++ b/resource/nonce.go @@ -0,0 +1,74 @@ +package resource + +import ( + "context" + "crypto/rand" + "encoding/base64" + "fmt" + "io" + "time" + + "github.com/idfoundry/fapigo/storage" +) + +// dpopNonceSize is the byte length of a generated DPoP nonce — 256 +// bits, matching this module's other generated identifiers (see +// server's own interactionHandleSize/par.referenceSize). +const dpopNonceSize = 32 + +func generateDPoPNonce(random io.Reader) (string, error) { + if random == nil { + random = rand.Reader + } + buf := make([]byte, dpopNonceSize) + if _, err := io.ReadFull(random, buf); err != nil { + return "", err + } + return base64.RawURLEncoding.EncodeToString(buf), nil +} + +// issueDPoPNonce generates and persists a fresh nonce, valid from now +// for v.cfg.Limits.DPoPNonceLifetime. Only ever called once v.deps.Nonces +// is known to be non-nil. +func (v *Verifier) issueDPoPNonce(ctx context.Context, now time.Time) (string, error) { + nonce, err := generateDPoPNonce(v.deps.Random) + if err != nil { + return "", fmt.Errorf("resource: generate dpop nonce: %w", err) + } + if err := v.deps.Nonces.Issue(ctx, storage.NonceIssuance{ + Nonce: nonce, ExpiresAt: now.Add(v.cfg.Limits.DPoPNonceLifetime), + }); err != nil { + return "", fmt.Errorf("resource: issue dpop nonce: %w", err) + } + return nonce, nil +} + +// checkDPoPNonce enforces the nonce challenge (RFC 9449 §8, §9) when +// v.deps.Nonces is configured: presented is the DPoP proof's own +// "nonce" claim (empty if absent). A missing, unknown, already-consumed +// or expired nonce is rejected with a freshly issued replacement +// attached (via *Error's own Nonce method) for the caller to retry +// with; a validly consumed nonce returns nil, letting Verify proceed. +// +// Called only when v.deps.Nonces != nil — Verify itself guards that, +// since this is the one dependency this package treats as genuinely +// optional rather than required-with-visible-opt-out (see +// Dependencies.Nonces's own doc comment). +func (v *Verifier) checkDPoPNonce(ctx context.Context, presented string, now time.Time) *Error { + valid := false + if presented != "" { + record, err := v.deps.Nonces.Consume(ctx, storage.NonceConsumption{Nonce: presented}) + valid = err == nil && !now.After(record.ExpiresAt) + } + if valid { + return nil + } + + fresh, err := v.issueDPoPNonce(ctx, now) + if err != nil { + return newError(ErrorServerError, 500, "failed to issue dpop nonce", err) + } + challenge := newError(ErrorUseDPoPNonce, 401, "DPoP proof must carry a current nonce", nil) + challenge.nonce = fresh + return challenge +} diff --git a/resource/nonce_test.go b/resource/nonce_test.go new file mode 100644 index 0000000..1b84d15 --- /dev/null +++ b/resource/nonce_test.go @@ -0,0 +1,234 @@ +package resource_test + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "net/url" + "testing" + "time" + + fapi "github.com/idfoundry/fapigo" + "github.com/idfoundry/fapigo/internal/dpop" + "github.com/idfoundry/fapigo/internal/jose" + "github.com/idfoundry/fapigo/internal/token" + "github.com/idfoundry/fapigo/keys" + "github.com/idfoundry/fapigo/resource" + "github.com/idfoundry/fapigo/storage" + "github.com/idfoundry/fapigo/storage/memstore" +) + +// nonceFixture mirrors newFixture, but builds its DPoP proof with an +// explicit (possibly empty) nonce claim and wires a memstore.NonceStore +// into the verifier, so nonce-challenge tests don't have to duplicate +// every other piece of a valid request. nonces may be shared across +// fixtures (pass the same store to simulate a client's next request +// after a challenge or a successful response), or left nil to get a +// fresh, empty one. +type nonceFixture struct { + verifier *resource.Verifier + nonces *memstore.NonceStore + accessToken string + dpopProof string + target *url.URL +} + +func newNonceFixture(t *testing.T, proofNonce string, nonces *memstore.NonceStore) nonceFixture { + t.Helper() + + issuerKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("generate issuer key: %v", err) + } + dpopKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("generate dpop key: %v", err) + } + dpopJWK, err := jose.NewJWK(dpopKey.Public(), fapi.ES256) + if err != nil { + t.Fatalf("dpop jwk: %v", err) + } + thumbprint, err := dpopJWK.Thumbprint() + if err != nil { + t.Fatalf("dpop thumbprint: %v", err) + } + + now := time.Now() + target, err := url.Parse("https://rs.example.com/userinfo") + if err != nil { + t.Fatalf("parse target url: %v", err) + } + + accessToken, _, err := token.IssueAccessToken(token.AccessTokenParams{ + Signer: issuerKey, Algorithm: fapi.ES256, KeyID: "as-kid", + Issuer: testIssuer, Subject: "user-1", Audience: testIssuer, + ClientID: "client-1", Scope: "openid", + Confirmation: &token.Confirmation{JKT: thumbprint.String()}, + Now: now, Lifetime: 5 * time.Minute, + Random: rand.Reader, + }) + if err != nil { + t.Fatalf("issue access token: %v", err) + } + + dpopProof, err := dpop.CreateProof(dpop.ProofRequest{ + Signer: dpopKey, Algorithm: fapi.ES256, + Method: "GET", URL: target, + AccessToken: accessToken, + Nonce: proofNonce, + Now: now, + Random: rand.Reader, + }) + if err != nil { + t.Fatalf("create dpop proof: %v", err) + } + + issuerURL, err := fapi.ParseIssuerURL(testIssuer) + if err != nil { + t.Fatalf("ParseIssuerURL: %v", err) + } + jwtAccessTokens, err := resource.NewJWTAccessTokens(&fakeIssuerKeySource{set: keys.IssuerKeySet{Keys: []keys.IssuerKey{ + {KeyID: "as-kid", Algorithm: fapi.ES256, PublicKey: issuerKey.Public()}, + }}}, issuerURL, testIssuer, fapi.ES256, 5*time.Minute) + if err != nil { + t.Fatalf("NewJWTAccessTokens: %v", err) + } + + if nonces == nil { + nonces = memstore.NewNonceStore() + } + cfg := validConfig(t) + cfg.Limits.DPoPNonceLifetime = time.Minute + v, err := resource.NewVerifier(cfg, resource.Dependencies{ + AccessTokens: jwtAccessTokens, + Replay: &fakeReplayStore{}, + Revocation: &fakeRevocationChecker{}, + Clock: fixedClock{now: now}, + Nonces: nonces, + Random: rand.Reader, + }) + if err != nil { + t.Fatalf("NewVerifier: %v", err) + } + + return nonceFixture{verifier: v, nonces: nonces, accessToken: accessToken, dpopProof: dpopProof, target: target} +} + +func (f nonceFixture) verify(t *testing.T) (resource.AuthorizationContext, error) { + t.Helper() + return f.verifier.Verify(context.Background(), resource.VerifyRequest{ + Method: "GET", + URL: f.target, + Authorization: "DPoP " + f.accessToken, + DPoPProof: f.dpopProof, + }) +} + +func TestVerifyNonceDisabledByDefault(t *testing.T) { + // The ordinary newFixture from verify_test.go never sets + // Dependencies.Nonces — confirms nonce-challenge support changes + // nothing for a verifier that never opted in. + f := newFixture(t) + authz, err := f.verifier.Verify(context.Background(), resource.VerifyRequest{ + Method: "GET", + URL: f.target, + Authorization: "DPoP " + f.accessToken, + DPoPProof: f.dpopProof, + }) + if err != nil { + t.Fatalf("Verify: %v", err) + } + if authz.NextDPoPNonce != "" { + t.Errorf("NextDPoPNonce = %q, want empty when nonces disabled", authz.NextDPoPNonce) + } +} + +func TestVerifyChallengesMissingNonce(t *testing.T) { + f := newNonceFixture(t, "", nil) + + _, err := f.verify(t) + if err == nil { + t.Fatalf("Verify(no nonce) = nil error, want error") + } + rerr, ok := err.(*resource.Error) + if !ok { + t.Fatalf("error type = %T, want *resource.Error", err) + } + if rerr.Code() != resource.ErrorUseDPoPNonce { + t.Errorf("Code() = %v, want %v", rerr.Code(), resource.ErrorUseDPoPNonce) + } + if rerr.Nonce() == "" { + t.Errorf("Nonce() is empty, want a freshly issued nonce") + } +} + +func TestVerifyChallengesUnknownNonce(t *testing.T) { + f := newNonceFixture(t, "never-issued", nil) + + _, err := f.verify(t) + rerr, ok := err.(*resource.Error) + if !ok { + t.Fatalf("error type = %T, want *resource.Error", err) + } + if rerr.Code() != resource.ErrorUseDPoPNonce { + t.Errorf("Code() = %v, want %v", rerr.Code(), resource.ErrorUseDPoPNonce) + } +} + +// TestVerifyChallengesExpiredNonce confirms a nonce that was validly +// issued but has since passed its own ExpiresAt is rejected exactly +// like an unknown one — and is still consumed in the process (a stale +// nonce can't be retried indefinitely). +func TestVerifyChallengesExpiredNonce(t *testing.T) { + nonces := memstore.NewNonceStore() + if err := nonces.Issue(context.Background(), storage.NonceIssuance{ + Nonce: "stale-nonce", ExpiresAt: time.Now().Add(-time.Minute), + }); err != nil { + t.Fatalf("Issue: %v", err) + } + + f := newNonceFixture(t, "stale-nonce", nonces) + _, err := f.verify(t) + rerr, ok := err.(*resource.Error) + if !ok { + t.Fatalf("error type = %T, want *resource.Error", err) + } + if rerr.Code() != resource.ErrorUseDPoPNonce { + t.Errorf("Code() = %v, want %v", rerr.Code(), resource.ErrorUseDPoPNonce) + } +} + +func TestVerifyAcceptsValidNonceAndIssuesNext(t *testing.T) { + first := newNonceFixture(t, "", nil) + + // First call: challenged, but the issued nonce is now valid for the + // retry — exactly the flow ResourceClient.Do drives on the client + // side. A real retry is a fresh request (new access token, new DPoP + // proof/jti), sharing only the resource server's own nonce store. + _, err := first.verify(t) + rerr, ok := err.(*resource.Error) + if !ok { + t.Fatalf("error type = %T, want *resource.Error", err) + } + issued := rerr.Nonce() + + retry := newNonceFixture(t, issued, first.nonces) + authz, err := retry.verify(t) + if err != nil { + t.Fatalf("Verify(valid nonce): %v", err) + } + if authz.NextDPoPNonce == "" { + t.Fatalf("NextDPoPNonce is empty, want a freshly issued nonce") + } + if authz.NextDPoPNonce == issued { + t.Fatalf("NextDPoPNonce = %q, want different from the just-consumed nonce %q", authz.NextDPoPNonce, issued) + } + + // The consumed nonce is single-use: a third request presenting it + // again must fail even though it was validly issued once. + reused := newNonceFixture(t, issued, first.nonces) + if _, err := reused.verify(t); err == nil { + t.Fatalf("Verify(reused nonce) = nil error, want error") + } +} diff --git a/resource/verifier.go b/resource/verifier.go index 97787f7..db1e49d 100644 --- a/resource/verifier.go +++ b/resource/verifier.go @@ -15,7 +15,7 @@ type Verifier struct { // present and valid — see Config, Limits and Dependencies for what "no // implicit fallback" means for each field. func NewVerifier(cfg Config, deps Dependencies) (*Verifier, error) { - if err := validateConfig(cfg); err != nil { + if err := validateConfig(cfg, deps); err != nil { return nil, err } if err := validateDependencies(deps); err != nil { @@ -24,13 +24,16 @@ func NewVerifier(cfg Config, deps Dependencies) (*Verifier, error) { return &Verifier{cfg: cfg, deps: deps}, nil } -func validateConfig(cfg Config) error { +func validateConfig(cfg Config, deps Dependencies) error { if cfg.Limits.MaxDPoPProofAge <= 0 { return fmt.Errorf("resource: config: limits.max_dpop_proof_age must be positive") } if cfg.Limits.MaxClockSkew < 0 { return fmt.Errorf("resource: config: limits.max_clock_skew must not be negative") } + if deps.Nonces != nil && cfg.Limits.DPoPNonceLifetime <= 0 { + return fmt.Errorf("resource: config: limits.dpop_nonce_lifetime must be positive when dependencies.nonces is set") + } return nil } @@ -47,5 +50,8 @@ func validateDependencies(deps Dependencies) error { if deps.Clock == nil { return fmt.Errorf("resource: dependencies: clock is required") } + if deps.Nonces != nil && deps.Random == nil { + return fmt.Errorf("resource: dependencies: random is required when nonces is set") + } return nil } diff --git a/resource/verifier_test.go b/resource/verifier_test.go index 846b29a..ee4acf1 100644 --- a/resource/verifier_test.go +++ b/resource/verifier_test.go @@ -2,6 +2,7 @@ package resource_test import ( "context" + "crypto/rand" "testing" "time" @@ -9,6 +10,7 @@ import ( "github.com/idfoundry/fapigo/keys" "github.com/idfoundry/fapigo/resource" "github.com/idfoundry/fapigo/storage" + "github.com/idfoundry/fapigo/storage/memstore" ) const testIssuer = "https://as.example.com" @@ -121,6 +123,54 @@ func TestNewVerifierRejectsInvalidConfig(t *testing.T) { } } +// TestNewVerifierAcceptsZeroNonceLifetimeWhenNoncesUnset confirms +// DPoPNonceLifetime is only validated when Dependencies.Nonces is +// actually set — a verifier that never opts into nonce-challenge +// support shouldn't be forced to set an otherwise-unused limit. +func TestNewVerifierAcceptsZeroNonceLifetimeWhenNoncesUnset(t *testing.T) { + cfg := validConfig(t) + cfg.Limits.DPoPNonceLifetime = 0 + if _, err := resource.NewVerifier(cfg, validDependencies(t)); err != nil { + t.Fatalf("NewVerifier: %v", err) + } +} + +func TestNewVerifierRejectsInvalidNonceConfig(t *testing.T) { + nonces := memstore.NewNonceStore() + + t.Run("zero nonce lifetime with nonces set", func(t *testing.T) { + cfg := validConfig(t) + cfg.Limits.DPoPNonceLifetime = 0 + deps := validDependencies(t) + deps.Nonces = nonces + deps.Random = rand.Reader + if _, err := resource.NewVerifier(cfg, deps); err == nil { + t.Fatalf("NewVerifier(zero nonce lifetime, nonces set) = nil error, want error") + } + }) + + t.Run("nil random with nonces set", func(t *testing.T) { + cfg := validConfig(t) + cfg.Limits.DPoPNonceLifetime = time.Minute + deps := validDependencies(t) + deps.Nonces = nonces + if _, err := resource.NewVerifier(cfg, deps); err == nil { + t.Fatalf("NewVerifier(nil random, nonces set) = nil error, want error") + } + }) + + t.Run("nonces and random both set", func(t *testing.T) { + cfg := validConfig(t) + cfg.Limits.DPoPNonceLifetime = time.Minute + deps := validDependencies(t) + deps.Nonces = nonces + deps.Random = rand.Reader + if _, err := resource.NewVerifier(cfg, deps); err != nil { + t.Fatalf("NewVerifier: %v", err) + } + }) +} + // TestNewJWTAccessTokensRejectsInvalid covers what // TestNewVerifierRejectsInvalidConfig used to check directly against // resource.Config before issuer/audience/algorithm/max-token-lifetime diff --git a/resource/verify.go b/resource/verify.go index 51ec2c1..9f784c3 100644 --- a/resource/verify.go +++ b/resource/verify.go @@ -40,6 +40,14 @@ type AuthorizationContext struct { // logging — it's already available at this point (see // Dependencies.Revocation), so surfacing it costs nothing. Key string + + // NextDPoPNonce is a freshly issued DPoP nonce the caller should set + // as this response's own DPoP-Nonce header, so its next call already + // carries a valid one instead of needing its own challenge/retry + // round trip (RFC 9449 §8's own proactive-refresh recommendation). + // Always "" when Dependencies.Nonces is nil (nonce-challenge support + // disabled); otherwise always populated on a successful Verify. + NextDPoPNonce string } // Verify checks req's Authorization header and DPoP proof together — @@ -84,6 +92,15 @@ func (v *Verifier) Verify(ctx context.Context, req VerifyRequest) (Authorization return AuthorizationContext{}, newError(ErrorInvalidToken, 401, "DPoP proof verification failed", err) } + // Nonce freshness is checked before spending the cost of resolving + // the access token — a request that fails this cheap, early gate + // shouldn't get as far as touching Dependencies.AccessTokens at all. + if v.deps.Nonces != nil { + if challenge := v.checkDPoPNonce(ctx, verifiedProof.Nonce, now); challenge != nil { + return AuthorizationContext{}, challenge + } + } + resolved, err := v.deps.AccessTokens.ResolveAccessToken(ctx, ResolveAccessTokenRequest{Raw: raw, Now: now}) if err != nil { // A *Error carries its own exposure (see AccessTokenResolver's @@ -120,12 +137,21 @@ func (v *Verifier) Verify(ctx context.Context, req VerifyRequest) (Authorization return AuthorizationContext{}, newError(ErrorInvalidToken, 401, "access token has been revoked", nil) } + var nextNonce string + if v.deps.Nonces != nil { + nextNonce, err = v.issueDPoPNonce(ctx, now) + if err != nil { + return AuthorizationContext{}, newError(ErrorServerError, 500, "failed to issue dpop nonce", err) + } + } + return AuthorizationContext{ - Subject: resolved.Subject, - ClientID: resolved.ClientID, - Scopes: resolved.Scopes, - Claims: resolved.Claims, - ExpiresAt: resolved.ExpiresAt, - Key: resolved.Key, + Subject: resolved.Subject, + ClientID: resolved.ClientID, + Scopes: resolved.Scopes, + Claims: resolved.Claims, + ExpiresAt: resolved.ExpiresAt, + Key: resolved.Key, + NextDPoPNonce: nextNonce, }, nil } From 674d927f34e292de3e4e38a500fe08a99cb44ed4 Mon Sep 17 00:00:00 2001 From: Oscar Sanderson Date: Fri, 28 Aug 2026 00:18:17 +0800 Subject: [PATCH 3/3] feat: wire DPoP nonce challenge into the conformance-as example resource MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New -dpop-nonce-challenge flag (off by default) demonstrates resource.Verifier's new nonce-challenge support end to end on the /accounts and /userinfo example endpoints: DPoP-Nonce is set from Error.Nonce on a use_dpop_nonce rejection and from AuthorizationContext.NextDPoPNonce on success. Off by default because the OIDF suite's own AS-plan protected-resource caller isn't guaranteed to implement the client-side retry the way this module's own client package does — flipping it on unconditionally would risk breaking unrelated AS conformance. TestSmokeUserInfoWithDPoPNonceChallenge confirms the two independently built halves actually interoperate: FetchUserInfo succeeds transparently against a nonce-challenging endpoint, entirely through client's existing ResourceClient.Do retry logic, with no special handling in the test itself. Co-Authored-By: Claude Sonnet 5 --- cmd/conformance-as/main.go | 7 +++- cmd/conformance-as/resource.go | 13 ++++++++ cmd/conformance-as/smoke_test.go | 57 +++++++++++++++++++++++++++++++- cmd/conformance-as/wiring.go | 20 ++++++++--- 4 files changed, 91 insertions(+), 6 deletions(-) diff --git a/cmd/conformance-as/main.go b/cmd/conformance-as/main.go index dd4ac01..09883a8 100644 --- a/cmd/conformance-as/main.go +++ b/cmd/conformance-as/main.go @@ -16,6 +16,10 @@ import ( // httpFetchTimeout bounds a single outbound client-JWKS fetch. const httpFetchTimeout = 10 * time.Second +// dpopNonceLifetime bounds how long a DPoP nonce issued under +// -dpop-nonce-challenge remains valid. +const dpopNonceLifetime = time.Minute + // readHeaderTimeout bounds how long the server waits to receive a // request's headers — without it, a client that trickles headers one // byte at a time can hold a connection (and its goroutine) open @@ -47,6 +51,7 @@ func main() { keyOverride := flag.String("key", "", "override tls.key_file from the config file") insecureHTTP := flag.Bool("insecure-http", false, "serve plaintext HTTP instead of TLS (loopback listen_addr only)") accessTokenFormat := flag.String("access-token-format", string(AccessTokenFormatJWT), "access token format to issue/verify: jwt or opaque") + dpopNonceChallenge := flag.Bool("dpop-nonce-challenge", false, "require and rotate a DPoP nonce on /accounts and /userinfo (RFC 9449 §8/§9) — off by default, since the OIDF suite's own protected-resource caller may not retry on the challenge") flag.Parse() if *configPath == "" { @@ -75,7 +80,7 @@ func main() { log.Fatal("conformance-as: -insecure-http requires a loopback listen_addr (e.g. 127.0.0.1:8443)") } - mux, err := newServerMux(resolved, *insecureHTTP) + mux, err := newServerMux(resolved, *insecureHTTP, *dpopNonceChallenge) if err != nil { log.Fatalf("conformance-as: %v", err) } diff --git a/cmd/conformance-as/resource.go b/cmd/conformance-as/resource.go index bc9a7ef..348c496 100644 --- a/cmd/conformance-as/resource.go +++ b/cmd/conformance-as/resource.go @@ -61,6 +61,9 @@ func resourceHandler(verifier *fapires.Verifier, resourceURL *url.URL) http.Hand writeResourceError(w, err) return } + if authCtx.NextDPoPNonce != "" { + w.Header().Set("DPoP-Nonce", authCtx.NextDPoPNonce) + } w.Header().Set("Content-Type", "application/json") _ = json.NewEncoder(w).Encode(map[string]any{ "subject": authCtx.Subject, @@ -138,6 +141,9 @@ func userinfoHandler(verifier *fapires.Verifier, userinfoURL *url.URL, identityC } body["sub"] = authCtx.Subject + if authCtx.NextDPoPNonce != "" { + w.Header().Set("DPoP-Nonce", authCtx.NextDPoPNonce) + } w.Header().Set("Content-Type", "application/json") _ = json.NewEncoder(w).Encode(body) } @@ -149,6 +155,13 @@ func writeResourceError(w http.ResponseWriter, err error) { http.Error(w, "internal error", http.StatusInternalServerError) return } + // Set before writeResourceErrorRaw's own WriteHeader call — a + // header can't be added after that. Only ErrorUseDPoPNonce ever + // carries a nonce (Error.Nonce's own doc comment), so this is a + // no-op for every other rejection. + if resErr.Nonce() != "" { + w.Header().Set("DPoP-Nonce", resErr.Nonce()) + } writeResourceErrorRaw(w, resErr.HTTPStatus(), string(resErr.Code()), resErr.PublicDescription()) } diff --git a/cmd/conformance-as/smoke_test.go b/cmd/conformance-as/smoke_test.go index acb7239..789dbba 100644 --- a/cmd/conformance-as/smoke_test.go +++ b/cmd/conformance-as/smoke_test.go @@ -120,6 +120,10 @@ type smokeHarness struct { } func newSmokeHarness(t *testing.T, format AccessTokenFormat) *smokeHarness { + return newSmokeHarnessWithNonceChallenge(t, format, false) +} + +func newSmokeHarnessWithNonceChallenge(t *testing.T, format AccessTokenFormat, dpopNonceChallenge bool) *smokeHarness { t.Helper() cert, pool := selfSignedCert(t) @@ -140,6 +144,10 @@ func newSmokeHarness(t *testing.T, format AccessTokenFormat) *smokeHarness { if err != nil { t.Fatalf("build endpoints: %v", err) } + userinfoURL, err := buildResourceURL(issuer, false, "/userinfo") + if err != nil { + t.Fatalf("build userinfo url: %v", err) + } clientKeyManager, err := ephemeral.NewKeyManager(map[keys.SigningPurpose]fapi.SignatureAlgorithm{ keys.ClientAuthentication: fapi.ES256, @@ -188,7 +196,7 @@ func newSmokeHarness(t *testing.T, format AccessTokenFormat) *smokeHarness { AdvertisedScopes: []string{"openid", "accounts", "offline_access"}, } - mux, err := newServerMux(resolved, false) + mux, err := newServerMux(resolved, false, dpopNonceChallenge) if err != nil { t.Fatalf("build server mux: %v", err) } @@ -226,6 +234,7 @@ func newSmokeHarness(t *testing.T, format AccessTokenFormat) *smokeHarness { Authorization: endpoints.Authorization, Token: endpoints.Token, PushedAuthorizationRequest: endpoints.PushedAuthorizationRequest, + UserInfo: userinfoURL, }, Profile: client.ProfileFAPISecurity, Algorithms: client.Algorithms{ @@ -386,6 +395,52 @@ func testSmokeAuthorizationCodeFlow(t *testing.T, format AccessTokenFormat) { } } +// TestSmokeUserInfoWithDPoPNonceChallenge confirms this binary's +// -dpop-nonce-challenge wiring actually interoperates with the client +// package's own nonce-retry logic (client/resource.go's +// ResourceClient.Do), not just each side's own unit tests in isolation: +// FetchUserInfo must succeed transparently even though the very first +// call is challenged and retried entirely inside the client, with no +// special handling from this test. +func TestSmokeUserInfoWithDPoPNonceChallenge(t *testing.T) { + h := newSmokeHarnessWithNonceChallenge(t, AccessTokenFormatJWT, true) + ctx := context.Background() + scope := []string{"openid", "accounts"} + + handle := h.runToConsent(ctx, scope) + rawQuery := h.submitDecision(ctx, handle, "approve", scope) + + result, err := h.client.CompleteAuthorization(ctx, client.AuthorizationCallback{RawQuery: rawQuery}) + if err != nil { + t.Fatalf("CompleteAuthorization: %v", err) + } + success, ok := result.(client.CompletionSuccess) + if !ok { + t.Fatalf("CompleteAuthorization result = %T, want client.CompletionSuccess", result) + } + + info, err := h.client.FetchUserInfo(ctx, success.Tokens) + if err != nil { + t.Fatalf("FetchUserInfo (should retry the nonce challenge transparently): %v", err) + } + if info.Subject != smokeSubject { + t.Fatalf("UserInfo.Subject = %q, want %q", info.Subject, smokeSubject) + } + + // A second, independent call must succeed too — confirming the + // server's single-use nonce store doesn't end up in some broken + // state after one full challenge/consume/reissue cycle. (client + // itself has no cross-call nonce cache — ResourceClient.Do only + // reacts to a challenge within the one call it's already making — + // so this still exercises its own full challenge/retry round trip, + // same as the first call above; the server's proactive + // NextDPoPNonce only benefits a caller that tracks and reuses it + // itself, which this client doesn't do.) + if _, err := h.client.FetchUserInfo(ctx, success.Tokens); err != nil { + t.Fatalf("FetchUserInfo (second call): %v", err) + } +} + func TestSmokeAuthorizationDenied(t *testing.T) { h := newSmokeHarness(t, AccessTokenFormatJWT) ctx := context.Background() diff --git a/cmd/conformance-as/wiring.go b/cmd/conformance-as/wiring.go index dfd037d..7dbc3b8 100644 --- a/cmd/conformance-as/wiring.go +++ b/cmd/conformance-as/wiring.go @@ -19,7 +19,7 @@ import ( // Factored out so the end-to-end smoke test can stand up the exact same // wiring main.go uses, against its own TLS listener, without going // through flags or a config file on disk. -func newServerMux(resolved ResolvedConfig, allowLoopbackHTTP bool) (*http.ServeMux, error) { +func newServerMux(resolved ResolvedConfig, allowLoopbackHTTP bool, dpopNonceChallenge bool) (*http.ServeMux, error) { endpoints, err := buildEndpoints(resolved.Issuer, allowLoopbackHTTP) if err != nil { return nil, err @@ -146,17 +146,29 @@ func newServerMux(resolved ResolvedConfig, allowLoopbackHTTP bool) (*http.ServeM return nil, err } - resourceVerifier, err := fapires.NewVerifier(fapires.Config{ + resourceCfg := fapires.Config{ Limits: fapires.Limits{ MaxDPoPProofAge: resolved.Limits.MaxDPoPProofAge, MaxClockSkew: resolved.Limits.MaxClockSkew, }, - }, fapires.Dependencies{ + } + resourceDeps := fapires.Dependencies{ AccessTokens: resourceAccessTokens, Replay: replayStore, Revocation: revocationStore, Clock: fapires.SystemClock{}, - }) + } + // Off by default (main.go's -dpop-nonce-challenge flag): the OIDF + // suite's own AS-plan protected-resource caller isn't guaranteed to + // implement the client-side nonce-challenge retry the way this + // module's own client package does, so turning this on + // unconditionally would risk breaking unrelated AS conformance. + if dpopNonceChallenge { + resourceCfg.Limits.DPoPNonceLifetime = dpopNonceLifetime + resourceDeps.Nonces = memstore.NewNonceStore() + resourceDeps.Random = rand.Reader + } + resourceVerifier, err := fapires.NewVerifier(resourceCfg, resourceDeps) if err != nil { return nil, err }