diff --git a/cmd/conformance-as/errors.go b/cmd/conformance-as/errors.go index d03db87..8498a0f 100644 --- a/cmd/conformance-as/errors.go +++ b/cmd/conformance-as/errors.go @@ -18,6 +18,13 @@ func writeOAuthJSONError(w http.ResponseWriter, err error) { http.Error(w, "internal error", http.StatusInternalServerError) return } + // Set before writeRawOAuthError'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 srvErr.Nonce() != "" { + w.Header().Set("DPoP-Nonce", srvErr.Nonce()) + } writeRawOAuthError(w, srvErr.HTTPStatus(), string(srvErr.Code()), srvErr.PublicDescription()) } diff --git a/cmd/conformance-as/main.go b/cmd/conformance-as/main.go index 09883a8..06357f8 100644 --- a/cmd/conformance-as/main.go +++ b/cmd/conformance-as/main.go @@ -51,7 +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") + dpopNonceChallenge := flag.Bool("dpop-nonce-challenge", false, "require and rotate a DPoP nonce on /par, /token, /accounts and /userinfo (RFC 9449 §8/§9) — off by default, since the OIDF suite's own driver may not retry on the challenge") flag.Parse() if *configPath == "" { diff --git a/cmd/conformance-as/par.go b/cmd/conformance-as/par.go index 7add462..1f74106 100644 --- a/cmd/conformance-as/par.go +++ b/cmd/conformance-as/par.go @@ -32,6 +32,9 @@ func parHandler(srv *server.Server) http.HandlerFunc { http.Error(w, "internal error", http.StatusInternalServerError) return } + if result.NextDPoPNonce != "" { + w.Header().Set("DPoP-Nonce", result.NextDPoPNonce) + } w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusCreated) _, _ = w.Write(body) diff --git a/cmd/conformance-as/smoke_test.go b/cmd/conformance-as/smoke_test.go index 789dbba..76b96bc 100644 --- a/cmd/conformance-as/smoke_test.go +++ b/cmd/conformance-as/smoke_test.go @@ -397,11 +397,17 @@ 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. +// package's own nonce-retry logic, not just each side's own unit tests +// in isolation. The flag now gates nonce-challenge on every DPoP proof +// this binary verifies — PAR and the token endpoint (server package, +// RFC 9449 §8) as well as /accounts and /userinfo (resource package, +// §9) — so CompleteAuthorization's own internal PAR/token exchange +// already exercises client.ExchangeCode's token-endpoint nonce retry +// (client/exchange_code.go's sendTokenRequest) before FetchUserInfo +// separately exercises ResourceClient.Do's. 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() diff --git a/cmd/conformance-as/token.go b/cmd/conformance-as/token.go index c93f2c6..6845842 100644 --- a/cmd/conformance-as/token.go +++ b/cmd/conformance-as/token.go @@ -50,6 +50,9 @@ func tokenHandler(srv *server.Server) http.HandlerFunc { resp["refresh_token"] = result.RefreshToken.Reveal() } + if result.NextDPoPNonce != "" { + w.Header().Set("DPoP-Nonce", result.NextDPoPNonce) + } // RFC 6749 §5.1 requires this on every token response — checked by // conformance suites, and worth doing correctly here even though // fapitest's own blueprint (which tests wire content, not header diff --git a/cmd/conformance-as/wiring.go b/cmd/conformance-as/wiring.go index 7dbc3b8..45e4013 100644 --- a/cmd/conformance-as/wiring.go +++ b/cmd/conformance-as/wiring.go @@ -141,6 +141,18 @@ func newServerMux(resolved ResolvedConfig, allowLoopbackHTTP bool, dpopNonceChal Random: rand.Reader, IdentityClaims: identityClaims, } + // Off by default (main.go's -dpop-nonce-challenge flag) — same + // reasoning as the resource-side block below: client.ExchangeCode + // already retries a use_dpop_nonce challenge, but the OIDF suite's + // own driver isn't guaranteed to, so this stays opt-in. A separate + // nonce store from the resource side's: PAR/token (this server's + // own role, RFC 9449 §8) and /accounts, /userinfo (the resource + // role, §9) are logically distinct nonce spaces, even though this + // one demo binary happens to host both. + if dpopNonceChallenge { + srvCfg.Limits.DPoPNonceLifetime = dpopNonceLifetime + srvDeps.Nonces = memstore.NewNonceStore() + } srv, err := server.New(srvCfg, srvDeps) if err != nil { return nil, err diff --git a/server/config.go b/server/config.go index bf5467f..6229af0 100644 --- a/server/config.go +++ b/server/config.go @@ -155,6 +155,12 @@ type Limits struct { // and extends how long past exp an artifact is 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; New rejects a zero value in that case, but + // leaves it unvalidated when nonce-challenge support is disabled. + DPoPNonceLifetime time.Duration } // Config is this server's immutable configuration. It is copied by New; diff --git a/server/dependencies.go b/server/dependencies.go index 8c1e2ed..950e1f1 100644 --- a/server/dependencies.go +++ b/server/dependencies.go @@ -66,7 +66,7 @@ type Dependencies struct { Clock Clock // Random is the source of randomness for request_uri, interaction - // handle and authorization code generation. + // handle, authorization code and DPoP nonce generation. Random io.Reader // IdentityClaims resolves identity claim values (e.g. "name", @@ -75,4 +75,19 @@ type Dependencies struct { // which is a permitted response to the "claims" request parameter // (see IdentityClaimsSource), not an error. IdentityClaims IdentityClaimsSource + + // Nonces persists DPoP nonces this server issues and consumes for + // requests to the PAR and token endpoints. Nil disables DPoP + // nonce-challenge support entirely (RFC 9449 §8) — like + // IdentityClaims, this is a genuinely optional field, not a + // security check this module treats as non-negotiable the way + // Revocation is: declining it is the normal, fully spec-compliant + // default. One shared store covers both endpoints — a nonce issued + // from a PAR response is valid at the token endpoint and vice + // versa, the same way resource.Dependencies.Nonces covers every + // protected-resource endpoint uniformly rather than one store per + // endpoint. Set it (and Config.Limits.DPoPNonceLifetime) to have + // this server challenge a DPoP proof carrying no valid nonce, and + // proactively reissue a fresh one on every successful call. + Nonces storage.NonceStore } diff --git a/server/errors.go b/server/errors.go index c3ea941..92a204d 100644 --- a/server/errors.go +++ b/server/errors.go @@ -23,6 +23,16 @@ const ( // wrong with an authorization request (missing client_id, a // malformed request_uri reference that doesn't even parse, etc.). ErrorInvalidRequestURI ErrorCode = "invalid_request_uri" + + // ErrorUseDPoPNonce indicates a DPoP proof presented to the PAR or + // token endpoint was otherwise valid but carried no nonce, or one + // this server didn't just issue (unknown, already consumed, or + // expired) — RFC 9449 §8's own error value, distinct from + // ErrorInvalidGrant/ErrorInvalidRequest (the request 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 every public Server method returns. Code and @@ -35,6 +45,7 @@ type Error struct { httpStatus int description string cause error + nonce string } func newError(code ErrorCode, httpStatus int, description string, cause error) *Error { @@ -47,6 +58,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/server/nonce.go b/server/nonce.go new file mode 100644 index 0000000..124d9a4 --- /dev/null +++ b/server/nonce.go @@ -0,0 +1,80 @@ +package server + +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 +// generateInteractionHandle/par.GenerateRequestURI). +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 s.cfg.Limits.DPoPNonceLifetime. Only ever called once +// s.deps.Nonces is known to be non-nil. +func (s *Server) issueDPoPNonce(ctx context.Context, now time.Time) (string, error) { + nonce, err := generateDPoPNonce(s.deps.Random) + if err != nil { + return "", fmt.Errorf("server: generate dpop nonce: %w", err) + } + if err := s.deps.Nonces.Issue(ctx, storage.NonceIssuance{ + Nonce: nonce, ExpiresAt: now.Add(s.cfg.Limits.DPoPNonceLifetime), + }); err != nil { + return "", fmt.Errorf("server: issue dpop nonce: %w", err) + } + return nonce, nil +} + +// checkDPoPNonce enforces the nonce challenge (RFC 9449 §8) when +// s.deps.Nonces is configured: presented is the DPoP proof's own +// "nonce" claim (empty if absent) — shared by both the token endpoint +// and PAR, since one nonce store covers everything this server +// verifies (see Dependencies.Nonces's own doc comment). 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 the caller proceed. +// +// Called only when s.deps.Nonces != nil — every call site 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 (s *Server) checkDPoPNonce(ctx context.Context, presented string, now time.Time) *Error { + valid := false + if presented != "" { + record, err := s.deps.Nonces.Consume(ctx, storage.NonceConsumption{Nonce: presented}) + valid = err == nil && !now.After(record.ExpiresAt) + } + if valid { + return nil + } + + fresh, err := s.issueDPoPNonce(ctx, now) + if err != nil { + return newError(ErrorServerError, 500, "failed to issue dpop nonce", err) + } + // RFC 9449 §8 token-endpoint nonce errors are ordinary RFC 6749 + // §5.2 OAuth errors — 400, not the 401 WWW-Authenticate challenge + // resource.ErrorUseDPoPNonce uses for a protected-resource request. + challenge := newError(ErrorUseDPoPNonce, 400, "DPoP proof must carry a current nonce", nil) + challenge.nonce = fresh + return challenge +} diff --git a/server/nonce_test.go b/server/nonce_test.go new file mode 100644 index 0000000..600aacd --- /dev/null +++ b/server/nonce_test.go @@ -0,0 +1,501 @@ +package server_test + +import ( + "context" + "crypto/ecdsa" + "crypto/rand" + "net/url" + "testing" + "time" + + fapi "github.com/idfoundry/fapigo" + "github.com/idfoundry/fapigo/internal/clientassertion" + "github.com/idfoundry/fapigo/internal/dpop" + "github.com/idfoundry/fapigo/keys" + "github.com/idfoundry/fapigo/server" + "github.com/idfoundry/fapigo/storage" + "github.com/idfoundry/fapigo/storage/memstore" +) + +// --- Config/Dependencies validation --------------------------------- + +func TestNewAcceptsZeroNonceLifetimeWhenNoncesUnset(t *testing.T) { + cfg := validConfig(t) + cfg.Limits.DPoPNonceLifetime = 0 + if _, err := server.New(cfg, validDependencies()); err != nil { + t.Fatalf("New: %v", err) + } +} + +func TestNewRejectsInvalidNonceConfig(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() + deps.Nonces = nonces + if _, err := server.New(cfg, deps); err == nil { + t.Fatalf("New(zero nonce lifetime, nonces set) = nil error, want error") + } + }) + + t.Run("nonces and lifetime both set", func(t *testing.T) { + cfg := validConfig(t) + cfg.Limits.DPoPNonceLifetime = time.Minute + deps := validDependencies() + deps.Nonces = nonces + if _, err := server.New(cfg, deps); err != nil { + t.Fatalf("New: %v", err) + } + }) +} + +// --- harness wiring ------------------------------------------------ + +// newHarnessWithNonces mirrors newHarnessWithIdentityClaims's shape +// (identity_claims_test.go), but wires a memstore.NonceStore and +// Limits.DPoPNonceLifetime so token/PAR nonce-challenge tests don't have +// to duplicate every other piece of a valid harness. +func newHarnessWithNonces(t *testing.T) (harness, *memstore.NonceStore) { + t.Helper() + now := time.Now() + key := generateKey(t) + serverKey := generateKey(t) + + client, err := storage.NewRegisteredClient(storage.RegisteredClientConfig{ + ID: testClientID, + RedirectURIs: []fapi.RegisteredRedirectURI{testRedirectURI}, + ClientAssertionAlgorithm: fapi.ES256, + AllowedScopes: []string{"openid", "accounts", "offline_access"}, + }) + if err != nil { + t.Fatalf("NewRegisteredClient: %v", err) + } + issuer, err := fapi.ParseIssuerURL(testIssuer) + if err != nil { + t.Fatalf("ParseIssuerURL: %v", err) + } + + cfg := server.Config{ + Issuer: issuer, + Endpoints: testEndpoints(t), + Profile: server.ProfileFAPISecurity, + Algorithms: server.AlgorithmPolicy{ + ClientAssertion: server.AlgorithmSet{fapi.ES256}, + RequestObject: server.AlgorithmSet{fapi.ES256}, + JARM: fapi.ES256, + IDToken: fapi.ES256, + }, + Limits: server.Limits{ + PushedRequestLifetime: 90 * time.Second, + MaxClientAssertionLifetime: time.Minute, + MaxRequestObjectLifetime: time.Minute, + InteractionLifetime: 5 * time.Minute, + AuthorizationCodeLifetime: time.Minute, + JARMResponseLifetime: time.Minute, + AccessTokenLifetime: 5 * time.Minute, + IDTokenLifetime: 5 * time.Minute, + RefreshTokenLifetime: 5 * time.Minute, + MaxDPoPProofAge: time.Minute, + MaxClockSkew: 5 * time.Second, + DPoPNonceLifetime: time.Minute, + }, + Assurance: server.AssuranceDevelopment, + } + serverKeyManager := &fakeKeyManager{key: serverKey, keyID: "as-key-1"} + nonces := memstore.NewNonceStore() + deps := server.Dependencies{ + Clients: &fakeClientRepository{clients: map[fapi.ClientID]storage.RegisteredClient{testClientID: client}}, + Transactions: &fakeTransactionStore{}, + Grants: &fakeGrantStore{}, + Replay: &fakeReplayStore{}, + ClientKeys: &fakeClientKeySource{keysByClient: map[fapi.ClientID][]keys.VerificationKey{ + testClientID: {{Algorithm: fapi.ES256, PublicKey: &key.PublicKey}}, + }}, + Keys: serverKeyManager, + AccessTokens: server.JWTAccessTokens{Keys: serverKeyManager, Algorithm: fapi.ES256}, + Revocation: server.NoRevocation{}, + Clock: fixedClock{now: now}, + Random: rand.Reader, + Nonces: nonces, + } + srv, err := server.New(cfg, deps) + if err != nil { + t.Fatalf("server.New: %v", err) + } + return harness{server: srv, key: key, serverKey: serverKey, now: now}, nonces +} + +// exchangeWithDPoPNonce runs a full ExchangeAuthorizationCode attempt +// (a fresh authorization code each call, since codes are single-use) +// with a DPoP proof carrying nonce ("" for none). +func exchangeWithDPoPNonce(t *testing.T, h harness, dpopKey *ecdsa.PrivateKey, nonce string) (server.TokenResult, error) { + t.Helper() + code := completeSuccessfulAuthorization(t, h, []string{"openid", "accounts"}) + tokenURL, err := url.Parse(testTokenEndpoint) + if err != nil { + t.Fatalf("url.Parse: %v", err) + } + proof, err := dpop.CreateProof(dpop.ProofRequest{ + Signer: dpopKey, Algorithm: fapi.ES256, Method: "POST", URL: tokenURL, Now: h.now, Nonce: nonce, + }) + if err != nil { + t.Fatalf("dpop.CreateProof: %v", err) + } + return h.server.ExchangeAuthorizationCode(context.Background(), server.AuthorizationCodeExchangeRequest{ + HTTP: server.FormRequest{Parameters: exchangeFormParams(h.clientAssertion(t), code, testRedirectURI, testCodeVerifier)}, + DPoPProof: proof, + }) +} + +// --- Token endpoint (ExchangeAuthorizationCode / RefreshAccessToken) --- + +func TestExchangeAuthorizationCodeNonceDisabledByDefault(t *testing.T) { + h := newHarness(t, server.ProfileFAPISecurity, true) + code := completeSuccessfulAuthorization(t, h, []string{"openid", "accounts"}) + dpopKey := generateKey(t) + + result, err := h.server.ExchangeAuthorizationCode(context.Background(), server.AuthorizationCodeExchangeRequest{ + HTTP: server.FormRequest{Parameters: exchangeFormParams(h.clientAssertion(t), code, testRedirectURI, testCodeVerifier)}, + DPoPProof: createDPoPProof(t, dpopKey, h.now), + }) + if err != nil { + t.Fatalf("ExchangeAuthorizationCode: %v", err) + } + if result.NextDPoPNonce != "" { + t.Errorf("NextDPoPNonce = %q, want empty when nonces disabled", result.NextDPoPNonce) + } +} + +func TestExchangeAuthorizationCodeChallengesMissingNonce(t *testing.T) { + h, _ := newHarnessWithNonces(t) + dpopKey := generateKey(t) + + _, err := exchangeWithDPoPNonce(t, h, dpopKey, "") + if err == nil { + t.Fatalf("ExchangeAuthorizationCode(no nonce) = nil error, want error") + } + serr, ok := err.(*server.Error) + if !ok { + t.Fatalf("error type = %T, want *server.Error", err) + } + if serr.Code() != server.ErrorUseDPoPNonce { + t.Errorf("Code() = %v, want %v", serr.Code(), server.ErrorUseDPoPNonce) + } + if serr.Nonce() == "" { + t.Errorf("Nonce() is empty, want a freshly issued nonce") + } +} + +func TestExchangeAuthorizationCodeChallengesUnknownNonce(t *testing.T) { + h, _ := newHarnessWithNonces(t) + dpopKey := generateKey(t) + + _, err := exchangeWithDPoPNonce(t, h, dpopKey, "never-issued") + serr, ok := err.(*server.Error) + if !ok { + t.Fatalf("error type = %T, want *server.Error", err) + } + if serr.Code() != server.ErrorUseDPoPNonce { + t.Errorf("Code() = %v, want %v", serr.Code(), server.ErrorUseDPoPNonce) + } +} + +func TestExchangeAuthorizationCodeChallengesExpiredNonce(t *testing.T) { + h, nonces := newHarnessWithNonces(t) + if err := nonces.Issue(context.Background(), storage.NonceIssuance{ + Nonce: "stale-nonce", ExpiresAt: h.now.Add(-time.Minute), + }); err != nil { + t.Fatalf("Issue: %v", err) + } + dpopKey := generateKey(t) + + _, err := exchangeWithDPoPNonce(t, h, dpopKey, "stale-nonce") + serr, ok := err.(*server.Error) + if !ok { + t.Fatalf("error type = %T, want *server.Error", err) + } + if serr.Code() != server.ErrorUseDPoPNonce { + t.Errorf("Code() = %v, want %v", serr.Code(), server.ErrorUseDPoPNonce) + } +} + +func TestExchangeAuthorizationCodeAcceptsValidNonceAndIssuesNext(t *testing.T) { + h, _ := newHarnessWithNonces(t) + dpopKey := generateKey(t) + + _, err := exchangeWithDPoPNonce(t, h, dpopKey, "") + serr, ok := err.(*server.Error) + if !ok { + t.Fatalf("error type = %T, want *server.Error", err) + } + issued := serr.Nonce() + + result, err := exchangeWithDPoPNonce(t, h, dpopKey, issued) + if err != nil { + t.Fatalf("ExchangeAuthorizationCode(valid nonce): %v", err) + } + if result.NextDPoPNonce == "" { + t.Fatalf("NextDPoPNonce is empty, want a freshly issued nonce") + } + if result.NextDPoPNonce == issued { + t.Fatalf("NextDPoPNonce = %q, want different from the just-consumed nonce %q", result.NextDPoPNonce, issued) + } + + // The consumed nonce is single-use: presenting it again must fail + // (with a fresh authorization code, since the previous one is + // already spent by the successful exchange above). + if _, err := exchangeWithDPoPNonce(t, h, dpopKey, issued); err == nil { + t.Fatalf("ExchangeAuthorizationCode(reused nonce) = nil error, want error") + } +} + +// TestExchangeAuthorizationCodeNonceCheckedBeforeCodeRedemption confirms +// a challenged attempt never actually redeems the authorization code — +// the same code must still work once retried with a valid nonce, +// exactly the shape a real client's single retry produces (RFC 9449 +// §8: the same request, replayed with a fresh proof carrying the +// nonce). +func TestExchangeAuthorizationCodeNonceCheckedBeforeCodeRedemption(t *testing.T) { + h, _ := newHarnessWithNonces(t) + dpopKey := generateKey(t) + code := completeSuccessfulAuthorization(t, h, []string{"openid", "accounts"}) + tokenURL, err := url.Parse(testTokenEndpoint) + if err != nil { + t.Fatalf("url.Parse: %v", err) + } + proofWithoutNonce, err := dpop.CreateProof(dpop.ProofRequest{ + Signer: dpopKey, Algorithm: fapi.ES256, Method: "POST", URL: tokenURL, Now: h.now, + }) + if err != nil { + t.Fatalf("dpop.CreateProof: %v", err) + } + + _, err = h.server.ExchangeAuthorizationCode(context.Background(), server.AuthorizationCodeExchangeRequest{ + HTTP: server.FormRequest{Parameters: exchangeFormParams(h.clientAssertion(t), code, testRedirectURI, testCodeVerifier)}, + DPoPProof: proofWithoutNonce, + }) + serr, ok := err.(*server.Error) + if !ok { + t.Fatalf("error type = %T, want *server.Error", err) + } + if serr.Code() != server.ErrorUseDPoPNonce { + t.Fatalf("Code() = %v, want %v", serr.Code(), server.ErrorUseDPoPNonce) + } + + // The retry reuses the same authorization code — proving the first, + // challenged attempt never redeemed it — but needs a fresh client + // assertion and DPoP proof, exactly like a real client's retry + // (client/exchange_code.go's sendTokenRequest rebuilds the whole + // form for the same reason: a client assertion is exactly as + // single-use as a DPoP proof). + proofWithNonce, err := dpop.CreateProof(dpop.ProofRequest{ + Signer: dpopKey, Algorithm: fapi.ES256, Method: "POST", URL: tokenURL, Now: h.now, Nonce: serr.Nonce(), + }) + if err != nil { + t.Fatalf("dpop.CreateProof: %v", err) + } + if _, err := h.server.ExchangeAuthorizationCode(context.Background(), server.AuthorizationCodeExchangeRequest{ + HTTP: server.FormRequest{Parameters: exchangeFormParams(h.clientAssertion(t), code, testRedirectURI, testCodeVerifier)}, + DPoPProof: proofWithNonce, + }); err != nil { + t.Fatalf("retry with the same authorization code and a valid nonce: %v", err) + } +} + +func TestRefreshAccessTokenReissuesNonceOnSuccess(t *testing.T) { + h, _ := newHarnessWithNonces(t) + dpopKey := generateKey(t) + + code := completeSuccessfulAuthorization(t, h, []string{"openid", "accounts", "offline_access"}) + tokenURL, err := url.Parse(testTokenEndpoint) + if err != nil { + t.Fatalf("url.Parse: %v", err) + } + firstProof, err := dpop.CreateProof(dpop.ProofRequest{ + Signer: dpopKey, Algorithm: fapi.ES256, Method: "POST", URL: tokenURL, Now: h.now, + }) + if err != nil { + t.Fatalf("dpop.CreateProof: %v", err) + } + _, err = h.server.ExchangeAuthorizationCode(context.Background(), server.AuthorizationCodeExchangeRequest{ + HTTP: server.FormRequest{Parameters: exchangeFormParams(h.clientAssertion(t), code, testRedirectURI, testCodeVerifier)}, + DPoPProof: firstProof, + }) + serr, ok := err.(*server.Error) + if !ok { + t.Fatalf("expected the first, nonce-less exchange to be challenged; error type = %T", err) + } + + code = completeSuccessfulAuthorization(t, h, []string{"openid", "accounts", "offline_access"}) + proof, err := dpop.CreateProof(dpop.ProofRequest{ + Signer: dpopKey, Algorithm: fapi.ES256, Method: "POST", URL: tokenURL, Now: h.now, Nonce: serr.Nonce(), + }) + if err != nil { + t.Fatalf("dpop.CreateProof: %v", err) + } + result, err := h.server.ExchangeAuthorizationCode(context.Background(), server.AuthorizationCodeExchangeRequest{ + HTTP: server.FormRequest{Parameters: exchangeFormParams(h.clientAssertion(t), code, testRedirectURI, testCodeVerifier)}, + DPoPProof: proof, + }) + if err != nil { + t.Fatalf("ExchangeAuthorizationCode: %v", err) + } + if !result.HasRefreshToken { + t.Fatalf("expected a refresh token since scope included offline_access") + } + + refreshProof, err := dpop.CreateProof(dpop.ProofRequest{ + Signer: dpopKey, Algorithm: fapi.ES256, Method: "POST", URL: tokenURL, Now: h.now, Nonce: result.NextDPoPNonce, + }) + if err != nil { + t.Fatalf("dpop.CreateProof: %v", err) + } + refreshed, err := h.server.RefreshAccessToken(context.Background(), server.RefreshTokenRequest{ + HTTP: server.FormRequest{Parameters: []server.FormParameter{ + formParam("client_assertion", h.clientAssertion(t)), + formParam("client_assertion_type", clientassertion.AssertionType), + formParam("grant_type", "refresh_token"), + formParam("refresh_token", result.RefreshToken.Reveal()), + }}, + DPoPProof: refreshProof, + }) + if err != nil { + t.Fatalf("RefreshAccessToken: %v", err) + } + if refreshed.NextDPoPNonce == "" { + t.Fatalf("NextDPoPNonce is empty, want a freshly issued nonce") + } +} + +// --- PAR --------------------------------------------------------------- + +func parFormParams(t *testing.T, h harness) []server.FormParameter { + t.Helper() + return []server.FormParameter{ + formParam("client_assertion", h.clientAssertion(t)), + formParam("client_assertion_type", clientassertion.AssertionType), + formParam("response_type", "code"), + formParam("redirect_uri", testRedirectURI), + formParam("scope", "openid accounts"), + formParam("code_challenge", "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM"), + formParam("code_challenge_method", "S256"), + formParam("state", "opaque-state"), + } +} + +// TestPushAuthorizationRequestWithoutDPoPUnaffectedByNonceChallenge is +// the key regression case: PAR's own DPoP proof stays entirely +// optional even with nonce-challenge enabled — a client that never +// sends one there is unaffected. +func TestPushAuthorizationRequestWithoutDPoPUnaffectedByNonceChallenge(t *testing.T) { + h, _ := newHarnessWithNonces(t) + result, err := h.server.PushAuthorizationRequest(context.Background(), server.PushAuthorizationRequest{ + HTTP: server.FormRequest{Parameters: parFormParams(t, h)}, + }) + if err != nil { + t.Fatalf("PushAuthorizationRequest (no DPoP proof) = %v, want success even with nonce-challenge enabled", err) + } + if result.NextDPoPNonce == "" { + t.Fatalf("NextDPoPNonce is empty, want a freshly issued nonce even without a proof presented") + } +} + +func TestPushAuthorizationRequestChallengesMissingNonceWhenDPoPPresented(t *testing.T) { + h, _ := newHarnessWithNonces(t) + dpopKey := generateKey(t) + parURL, err := url.Parse(testPAREndpoint) + if err != nil { + t.Fatalf("url.Parse: %v", err) + } + proof, err := dpop.CreateProof(dpop.ProofRequest{ + Signer: dpopKey, Algorithm: fapi.ES256, Method: "POST", URL: parURL, Now: h.now, + }) + if err != nil { + t.Fatalf("dpop.CreateProof: %v", err) + } + + _, err = h.server.PushAuthorizationRequest(context.Background(), server.PushAuthorizationRequest{ + HTTP: server.FormRequest{Parameters: parFormParams(t, h)}, DPoPProof: proof, + }) + serr, ok := err.(*server.Error) + if !ok { + t.Fatalf("error type = %T, want *server.Error", err) + } + if serr.Code() != server.ErrorUseDPoPNonce { + t.Errorf("Code() = %v, want %v", serr.Code(), server.ErrorUseDPoPNonce) + } + if serr.Nonce() == "" { + t.Errorf("Nonce() is empty, want a freshly issued nonce") + } +} + +func TestPushAuthorizationRequestAcceptsValidNonceAndIssuesNext(t *testing.T) { + h, _ := newHarnessWithNonces(t) + dpopKey := generateKey(t) + parURL, err := url.Parse(testPAREndpoint) + if err != nil { + t.Fatalf("url.Parse: %v", err) + } + firstProof, err := dpop.CreateProof(dpop.ProofRequest{ + Signer: dpopKey, Algorithm: fapi.ES256, Method: "POST", URL: parURL, Now: h.now, + }) + if err != nil { + t.Fatalf("dpop.CreateProof: %v", err) + } + _, err = h.server.PushAuthorizationRequest(context.Background(), server.PushAuthorizationRequest{ + HTTP: server.FormRequest{Parameters: parFormParams(t, h)}, DPoPProof: firstProof, + }) + serr, ok := err.(*server.Error) + if !ok { + t.Fatalf("error type = %T, want *server.Error", err) + } + issued := serr.Nonce() + + retryProof, err := dpop.CreateProof(dpop.ProofRequest{ + Signer: dpopKey, Algorithm: fapi.ES256, Method: "POST", URL: parURL, Now: h.now, Nonce: issued, + }) + if err != nil { + t.Fatalf("dpop.CreateProof: %v", err) + } + result, err := h.server.PushAuthorizationRequest(context.Background(), server.PushAuthorizationRequest{ + HTTP: server.FormRequest{Parameters: parFormParams(t, h)}, DPoPProof: retryProof, + }) + if err != nil { + t.Fatalf("PushAuthorizationRequest(valid nonce): %v", err) + } + if result.NextDPoPNonce == "" || result.NextDPoPNonce == issued { + t.Fatalf("NextDPoPNonce = %q, want a fresh value different from %q", result.NextDPoPNonce, issued) + } +} + +// TestNonceIssuedAtPARIsValidAtTokenEndpoint confirms one shared nonce +// store covers everything this server verifies (Dependencies.Nonces's +// own doc comment) — a nonce issued from a PAR challenge must also be +// accepted at the token endpoint. +func TestNonceIssuedAtPARIsValidAtTokenEndpoint(t *testing.T) { + h, _ := newHarnessWithNonces(t) + dpopKey := generateKey(t) + parURL, err := url.Parse(testPAREndpoint) + if err != nil { + t.Fatalf("url.Parse: %v", err) + } + parProof, err := dpop.CreateProof(dpop.ProofRequest{ + Signer: dpopKey, Algorithm: fapi.ES256, Method: "POST", URL: parURL, Now: h.now, + }) + if err != nil { + t.Fatalf("dpop.CreateProof: %v", err) + } + _, err = h.server.PushAuthorizationRequest(context.Background(), server.PushAuthorizationRequest{ + HTTP: server.FormRequest{Parameters: parFormParams(t, h)}, DPoPProof: parProof, + }) + serr, ok := err.(*server.Error) + if !ok { + t.Fatalf("expected the first, nonce-less PAR call to be challenged; error type = %T", err) + } + + if _, err := exchangeWithDPoPNonce(t, h, dpopKey, serr.Nonce()); err != nil { + t.Fatalf("ExchangeAuthorizationCode(nonce issued by PAR): %v", err) + } +} diff --git a/server/par.go b/server/par.go index 67e5bd8..03e2ea7 100644 --- a/server/par.go +++ b/server/par.go @@ -99,6 +99,15 @@ type PushAuthorizationRequest struct { type PushAuthorizationResult struct { RequestURI RequestURI ExpiresIn time.Duration + + // NextDPoPNonce is a freshly issued DPoP nonce the caller should set + // as this response's own DPoP-Nonce header, so a subsequent PAR or + // token request already carries a valid one — issued unconditionally + // once Dependencies.Nonces is configured, regardless of whether this + // particular PAR call itself presented a DPoP proof, so a client can + // pick one up as early as PAR. Always "" when Dependencies.Nonces is + // nil. + NextDPoPNonce string } // PushAuthorizationRequest authenticates the client, verifies either its @@ -151,10 +160,19 @@ func (s *Server) PushAuthorizationRequest(ctx context.Context, req PushAuthoriza return s.parFail(ctx, client.ID(), newError(ErrorServerError, 500, "failed to persist pushed authorization request", err)) } + var nextNonce string + if s.deps.Nonces != nil { + nextNonce, err = s.issueDPoPNonce(ctx, now) + if err != nil { + return s.parFail(ctx, client.ID(), newError(ErrorServerError, 500, "failed to issue dpop nonce", err)) + } + } + s.audit(ctx, AuditEventPushAuthorizationRequest, client.ID(), AuditOutcomeSuccess, "") return PushAuthorizationResult{ - RequestURI: RequestURI{value: requestURI}, - ExpiresIn: s.cfg.Limits.PushedRequestLifetime, + RequestURI: RequestURI{value: requestURI}, + ExpiresIn: s.cfg.Limits.PushedRequestLifetime, + NextDPoPNonce: nextNonce, }, nil } @@ -403,6 +421,11 @@ func (s *Server) reconcileParDPoPBinding(ctx context.Context, proof string, para if err != nil { return nil, newError(ErrorInvalidRequest, 400, "DPoP proof verification failed", err) } + if s.deps.Nonces != nil { + if challenge := s.checkDPoPNonce(ctx, verified.Nonce, s.deps.Clock.Now()); challenge != nil { + return nil, challenge + } + } thumbprint := verified.Thumbprint.String() if declared, jsonErr := jsonString(params, "dpop_jkt"); jsonErr == nil && declared != "" { diff --git a/server/refresh.go b/server/refresh.go index b22e38e..c7e08df 100644 --- a/server/refresh.go +++ b/server/refresh.go @@ -144,6 +144,14 @@ func (s *Server) RefreshAccessToken(ctx context.Context, req RefreshTokenRequest result.RefreshToken = fapi.NewSecret(rawToken) result.HasRefreshToken = true + if s.deps.Nonces != nil { + nextNonce, err := s.issueDPoPNonce(ctx, now) + if err != nil { + return s.tokenFail(ctx, AuditEventRefreshAccessToken, client.ID(), newError(ErrorServerError, 500, "failed to issue dpop nonce", err)) + } + result.NextDPoPNonce = nextNonce + } + s.audit(ctx, AuditEventRefreshAccessToken, client.ID(), AuditOutcomeSuccess, "") return result, nil } diff --git a/server/server.go b/server/server.go index da81a84..f2a8403 100644 --- a/server/server.go +++ b/server/server.go @@ -204,6 +204,9 @@ func validateDependencies(cfg Config, deps Dependencies) error { if idTokenEncEnabled && deps.ClientEncryptionKeys == nil { return fmt.Errorf("server: dependencies: client encryption keys is required when algorithms.id_token_encryption_key_management/content_encryption are configured") } + if deps.Nonces != nil && cfg.Limits.DPoPNonceLifetime <= 0 { + return fmt.Errorf("server: config: limits.dpop_nonce_lifetime must be positive when dependencies.nonces is set") + } if cfg.Assurance == AssuranceProduction { if deps.Audit == nil { return fmt.Errorf("server: dependencies: audit is required under AssuranceProduction") diff --git a/server/token.go b/server/token.go index 25ac900..0872a62 100644 --- a/server/token.go +++ b/server/token.go @@ -44,6 +44,15 @@ type TokenResult struct { // successful RefreshAccessToken call, which always rotates it. RefreshToken fapi.Secret HasRefreshToken bool + + // NextDPoPNonce is a freshly issued DPoP nonce the caller should set + // as this response's own DPoP-Nonce header, so its next PAR or + // token request 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 success. + NextDPoPNonce string } // ExchangeAuthorizationCode authenticates the client, verifies its DPoP @@ -195,6 +204,14 @@ func (s *Server) ExchangeAuthorizationCode(ctx context.Context, req Authorizatio result.HasRefreshToken = true } + if s.deps.Nonces != nil { + nextNonce, err := s.issueDPoPNonce(ctx, now) + if err != nil { + return s.tokenFail(ctx, AuditEventExchangeAuthorizationCode, client.ID(), newError(ErrorServerError, 500, "failed to issue dpop nonce", err)) + } + result.NextDPoPNonce = nextNonce + } + s.audit(ctx, AuditEventExchangeAuthorizationCode, client.ID(), AuditOutcomeSuccess, "") return result, nil } @@ -216,6 +233,11 @@ func (s *Server) verifyTokenRequestDPoP(ctx context.Context, proof string) (dpop if err != nil { return dpop.VerifiedProof{}, newError(ErrorInvalidRequest, 400, "DPoP proof verification failed", err) } + if s.deps.Nonces != nil { + if challenge := s.checkDPoPNonce(ctx, verified.Nonce, s.deps.Clock.Now()); challenge != nil { + return dpop.VerifiedProof{}, challenge + } + } return verified, nil }