From a3a052a0773beabd0f08220ced3637e216d4b5ed Mon Sep 17 00:00:00 2001 From: Oscar Sanderson Date: Fri, 28 Aug 2026 01:39:30 +0800 Subject: [PATCH] =?UTF-8?q?feat:=20send=20a=20DPoP=20proof=20at=20PAR=20by?= =?UTF-8?q?=20default,=20per=20RFC=209449=20=C2=A710.1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit client always committed the authorization code to its DPoP key at PAR via the plain dpop_jkt parameter (Option A) and never the alternative RFC 9449 §10.1 recognizes: presenting an actual DPoP proof at PAR (Option B). §10.1 itself recommends Option B — it reuses the same proof-building this client already does at the token and resource endpoints, and unlike dpop_jkt, it's real proof of possession at PAR time, not just a key identifier. Adds Config.PARDPoPBinding, defaulting (zero value) to the recommended PARDPoPBindingProof; PARDPoPBindingJKT keeps today's exact behavior for interop. Every authorization server supporting DPoP at PAR must accept both mechanisms (§10.1's own MUST) — server/par.go's reconcileParDPoPBinding already does, unchanged by this commit. Sending a proof at PAR means PAR can now be nonce-challenged the same way the token endpoint already can, so PARDPoPBindingProof gets the identical retry BeginAuthorization's ExchangeCode already has — rebuild the form (a client assertion is exactly as single-use as a DPoP proof) and resubmit once with the challenged nonce. Verified against the live OIDF conformance suite: the RP suites (which exercise this new default against a real, spec-compliant AS, not just this module's own) pass unchanged. Co-Authored-By: Claude Sonnet 5 --- client/begin_authorization.go | 133 ++++++++++++++++++++++++++++------ client/client.go | 3 + client/client_test.go | 15 ++++ client/config.go | 39 ++++++++++ client/flow_test.go | 120 ++++++++++++++++++++++++++++-- 5 files changed, 283 insertions(+), 27 deletions(-) diff --git a/client/begin_authorization.go b/client/begin_authorization.go index c11121b..c817b76 100644 --- a/client/begin_authorization.go +++ b/client/begin_authorization.go @@ -2,6 +2,7 @@ package client import ( "context" + "crypto" "encoding/json" "fmt" "net/http" @@ -12,6 +13,7 @@ import ( "github.com/idfoundry/fapigo/extension" "github.com/idfoundry/fapigo/internal/clientassertion" + "github.com/idfoundry/fapigo/internal/dpop" "github.com/idfoundry/fapigo/internal/jose" "github.com/idfoundry/fapigo/internal/par" "github.com/idfoundry/fapigo/internal/pkce" @@ -78,11 +80,6 @@ func (c *Client) BeginAuthorization(ctx context.Context, req BeginAuthorizationR return AuthorizationSession{}, newError(ErrorInternal, "failed to derive PKCE challenge", err) } - dpopThumbprint, err := c.dpopKeyThumbprint(ctx) - if err != nil { - return AuthorizationSession{}, newError(ErrorInternal, "failed to compute DPoP key thumbprint", err) - } - now := c.deps.Clock.Now() params := map[string]string{ // client_id (RFC 6749 §4.1.1) is a required authorization-request @@ -98,29 +95,22 @@ func (c *Client) BeginAuthorization(ctx context.Context, req BeginAuthorizationR "nonce": nonce, "code_challenge": challenge, "code_challenge_method": "S256", - // dpop_jkt (RFC 9449 §10) commits the authorization code to this - // client's DPoP key at PAR time, rather than leaving that - // binding to whichever key first shows up at the token endpoint - // — closing an authorization-code-injection window and matching - // the key this client will actually present with in - // ExchangeCode. - "dpop_jkt": dpopThumbprint, } if len(req.ACRValues) > 0 { params["acr_values"] = strings.Join(req.ACRValues, " ") } - formParams, buildErr := c.buildPushedRequestForm(ctx, now, params, req.Extensions) - if buildErr != nil { - return AuthorizationSession{}, buildErr + var ( + body []byte + parErr *Error + ) + if c.cfg.PARDPoPBinding == PARDPoPBindingJKT { + body, parErr = c.pushAuthorizationRequestWithJKT(ctx, params, req.Extensions) + } else { + body, parErr = c.pushAuthorizationRequestWithDPoPProof(ctx, params, req.Extensions) } - - body, status, _, err := c.postForm(ctx, c.cfg.Endpoints.PushedAuthorizationRequest.String(), par.EncodeForm(formParams), nil) - if err != nil { - return AuthorizationSession{}, newError(ErrorInternal, "pushed authorization request failed", err) - } - if status != http.StatusCreated && status != http.StatusOK { - return AuthorizationSession{}, parErrorFromResponse(body) + if parErr != nil { + return AuthorizationSession{}, parErr } result, err := par.DecodeResult(body) @@ -174,6 +164,105 @@ func (c *Client) dpopKeyThumbprint(ctx context.Context) (string, error) { return thumbprint.String(), nil } +// pushAuthorizationRequestWithJKT implements PARDPoPBindingJKT: declares +// this client's DPoP key via the plain "dpop_jkt" parameter (RFC 9449 +// §10) — committing the authorization code to it, rather than leaving +// that binding to whichever key first shows up at the token endpoint — +// without proving possession of it until ExchangeCode presents a proof +// with the matching key. +func (c *Client) pushAuthorizationRequestWithJKT(ctx context.Context, params map[string]string, extensions extension.Values) ([]byte, *Error) { + dpopThumbprint, err := c.dpopKeyThumbprint(ctx) + if err != nil { + return nil, newError(ErrorInternal, "failed to compute DPoP key thumbprint", err) + } + params["dpop_jkt"] = dpopThumbprint + + formParams, buildErr := c.buildPushedRequestForm(ctx, c.deps.Clock.Now(), params, extensions) + if buildErr != nil { + return nil, buildErr + } + body, status, _, err := c.postForm(ctx, c.cfg.Endpoints.PushedAuthorizationRequest.String(), par.EncodeForm(formParams), nil) + if err != nil { + return nil, newError(ErrorInternal, "pushed authorization request failed", err) + } + if status != http.StatusCreated && status != http.StatusOK { + return nil, parErrorFromResponse(body) + } + return body, nil +} + +// pushAuthorizationRequestWithDPoPProof implements PARDPoPBindingProof +// (the default): binds the authorization code to this client's DPoP key +// by presenting an actual proof at PAR — RFC 9449 §10.1's recommended +// mechanism — instead of the plain dpop_jkt parameter, retrying once on +// a use_dpop_nonce challenge. This mirrors sendTokenRequest's identical +// mechanic for the token endpoint: an authorization server that +// nonce-challenges DPoP proofs it verifies can now challenge this one +// too, since PAR is presenting one for the first time. A client +// assertion is exactly as single-use as a DPoP proof, so the retry +// rebuilds the whole form, not just the proof — same reasoning +// sendTokenRequest's own doc comment gives for the token endpoint. +func (c *Client) pushAuthorizationRequestWithDPoPProof(ctx context.Context, params map[string]string, extensions extension.Values) ([]byte, *Error) { + dpopSigner, _, err := c.newSigner(ctx, keys.DPoPProofSigning, c.cfg.Algorithms.DPoP) + if err != nil { + return nil, newError(ErrorInternal, "failed to resolve DPoP signing key", err) + } + parURL := c.cfg.Endpoints.PushedAuthorizationRequest.URL() + + buildParForm := func() ([]byte, error) { + formParams, buildErr := c.buildPushedRequestForm(ctx, c.deps.Clock.Now(), params, extensions) + if buildErr != nil { + return nil, buildErr + } + return par.EncodeForm(formParams), nil + } + form, buildErr := buildParForm() + if buildErr != nil { + return nil, newError(ErrorInternal, "failed to build pushed authorization request", buildErr) + } + + body, status, header, err := c.postParRequestWithDPoP(ctx, dpopSigner, &parURL, form, "") + if err != nil { + return nil, newError(ErrorInternal, "pushed authorization request failed", err) + } + if status == http.StatusCreated || status == http.StatusOK { + return body, nil + } + + nonce := header.Get("DPoP-Nonce") + if nonce == "" || !isDPoPNonceError(body) { + return nil, parErrorFromResponse(body) + } + retryForm, buildErr := buildParForm() + if buildErr != nil { + return nil, newError(ErrorInternal, "failed to build pushed authorization request", buildErr) + } + body, status, _, err = c.postParRequestWithDPoP(ctx, dpopSigner, &parURL, retryForm, nonce) + if err != nil { + return nil, newError(ErrorInternal, "pushed authorization request failed", err) + } + if status != http.StatusCreated && status != http.StatusOK { + return nil, parErrorFromResponse(body) + } + return body, nil +} + +// postParRequestWithDPoP signs a fresh DPoP proof (new iat and jti) for +// the pushed authorization request — no AccessToken/ath, since none +// exists yet at PAR time, matching how server/par.go's own dpop.Verify +// call never expects one either — and posts form to it. +func (c *Client) postParRequestWithDPoP(ctx context.Context, dpopSigner crypto.Signer, parURL *url.URL, form []byte, nonce string) ([]byte, int, http.Header, error) { + proof, err := dpop.CreateProof(dpop.ProofRequest{ + Signer: dpopSigner, Algorithm: c.cfg.Algorithms.DPoP, + Method: http.MethodPost, URL: parURL, Now: c.deps.Clock.Now(), + Random: c.deps.Random, Nonce: nonce, + }) + if err != nil { + return nil, 0, nil, fmt.Errorf("build DPoP proof: %w", err) + } + return c.postForm(ctx, parURL.String(), form, map[string]string{"DPoP": proof}) +} + // buildPushedRequestForm builds the PAR endpoint's form body: a client // assertion for authentication, plus either a signed request object // (ProfileFAPISecurityWithMessageSigning) or the plain authorization diff --git a/client/client.go b/client/client.go index 7d450bc..cf65393 100644 --- a/client/client.go +++ b/client/client.go @@ -44,6 +44,9 @@ func validateConfig(cfg Config) error { if cfg.Profile != ProfileFAPISecurity && cfg.Profile != ProfileFAPISecurityWithMessageSigning { return fmt.Errorf("client: config: profile is invalid") } + if cfg.PARDPoPBinding != PARDPoPBindingProof && cfg.PARDPoPBinding != PARDPoPBindingJKT { + return fmt.Errorf("client: config: par_dpop_binding is invalid") + } if !cfg.Algorithms.ClientAuthentication.IsValid() { return fmt.Errorf("client: config: algorithms.client_authentication is required") diff --git a/client/client_test.go b/client/client_test.go index 1f62efc..4fb6447 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -210,6 +210,20 @@ func TestNewAcceptsValidConfig(t *testing.T) { } } +// A Config that never mentions PARDPoPBinding at all must build +// successfully and behave as PARDPoPBindingProof — RFC 9449 §10.1's +// recommended mechanism is the zero-value default, not a validation +// error, unlike Profile/Algorithms/Limits. +func TestNewAcceptsZeroValuePARDPoPBinding(t *testing.T) { + cfg := validConfig(t) + if cfg.PARDPoPBinding != client.PARDPoPBindingProof { + t.Fatalf("validConfig's zero-value PARDPoPBinding = %v, want PARDPoPBindingProof", cfg.PARDPoPBinding) + } + if _, err := client.New(cfg, validDependencies(t)); err != nil { + t.Fatalf("New: %v", err) + } +} + func TestNewRejectsInvalidConfig(t *testing.T) { cases := map[string]func(*client.Config){ "zero issuer": func(c *client.Config) { c.Issuer = fapi.URL{} }, @@ -229,6 +243,7 @@ func TestNewRejectsInvalidConfig(t *testing.T) { "zero http timeout": func(c *client.Config) { c.Limits.HTTPTimeout = 0 }, "zero max response bytes": func(c *client.Config) { c.Limits.MaxHTTPResponseBytes = 0 }, "zero max jose compact bytes": func(c *client.Config) { c.Limits.MaxJOSECompactBytes = 0 }, + "invalid par dpop binding": func(c *client.Config) { c.PARDPoPBinding = client.PARDPoPBinding(99) }, "id_token key management set without content encryption": func(c *client.Config) { c.Algorithms.IDTokenKeyManagement = fapi.RSAOAEP256 }, diff --git a/client/config.go b/client/config.go index 4d2f0f5..6538ab1 100644 --- a/client/config.go +++ b/client/config.go @@ -26,6 +26,39 @@ const ( ProfileFAPISecurityWithMessageSigning ) +// PARDPoPBinding selects how BeginAuthorization commits the eventual +// authorization code to this client's DPoP key at PAR time (RFC 9449 +// §10.1 recognizes two mechanisms; an authorization server supporting +// both PAR and DPoP must accept either). Unlike Profile/Algorithms/ +// Limits, whose zero value is deliberately invalid because there's no +// universally-preferable choice, this type's zero value is a real, +// meaningful default: RFC 9449 §10.1 itself recommends +// PARDPoPBindingProof over PARDPoPBindingJKT, so a Config that never +// mentions this field gets the recommended behavior for free rather +// than a validation error. +type PARDPoPBinding uint8 + +const ( + // PARDPoPBindingProof (the default, zero value) sends an actual DPoP + // proof — not just its key's thumbprint — as this pushed + // authorization request's own "DPoP" header, binding the eventual + // authorization code to whichever key demonstrated possession here. + // RFC 9449 §10.1 recommends this over PARDPoPBindingJKT: it reuses + // the same proof-building this client already does at the token and + // resource endpoints (no separate thumbprint computation or + // parameter), and unlike a bare dpop_jkt claim, it's actual proof of + // possession at PAR time, not just a key identifier. + PARDPoPBindingProof PARDPoPBinding = iota + + // PARDPoPBindingJKT instead declares the key via the plain + // "dpop_jkt" request parameter (RFC 9449 §10), without proving + // possession of it until the token endpoint. Every authorization + // server supporting DPoP at PAR must accept this mechanism too (RFC + // 9449 §10.1's own MUST) — kept for interop with a deployment that + // has a specific reason to prefer it. + PARDPoPBindingJKT +) + // Algorithms are the single algorithm this client uses for each signing // operation it performs, and the single algorithm it expects the // authorization server to use for each of its own. A closed @@ -234,4 +267,10 @@ type Config struct { // OIDC Core §5.3.2 exists to catch. Defaults to false, which // preserves the exact-match behavior this package has always had. TolerateUserInfoSubjectEqualsClientID bool + + // PARDPoPBinding selects how BeginAuthorization commits this + // client's DPoP key at PAR time — see PARDPoPBinding's own doc + // comment. Defaults to PARDPoPBindingProof, RFC 9449 §10.1's own + // recommended mechanism. + PARDPoPBinding PARDPoPBinding } diff --git a/client/flow_test.go b/client/flow_test.go index e0703d3..dc5ba11 100644 --- a/client/flow_test.go +++ b/client/flow_test.go @@ -42,6 +42,7 @@ type fakeAS struct { messageSigned bool lastPARForm url.Values + lastPARDPoPProof string lastTokenForm url.Values lastTokenDPoPProof string lastNonce string @@ -54,6 +55,13 @@ type fakeAS struct { tokenCallCount int seenClientAssertions map[string]bool + // challengeParDPoPNonce mirrors challengeDPoPNonce, but for + // handlePAR — RFC 9449 §8's nonce challenge applies to any DPoP + // proof the authorization server verifies, not just the token + // endpoint's, and PARDPoPBindingProof means PAR now presents one. + challengeParDPoPNonce string + parCallCount int + // tokenTypeOverride, if non-empty, replaces the token response's // token_type value (e.g. "dpop" lowercase, to test RFC 6749 §7.1's // "Values are case insensitive"). @@ -111,9 +119,23 @@ func (a *fakeAS) handlePAR(w http.ResponseWriter, r *http.Request) { a.t.Fatalf("PAR: parse form: %v", err) } a.lastPARForm = r.PostForm + a.parCallCount++ + proof := r.Header.Get("DPoP") + a.lastPARDPoPProof = proof if r.PostForm.Get("client_assertion") == "" { a.t.Errorf("PAR: missing client_assertion") } + + if a.challengeParDPoPNonce != "" && dpopProofNonce(a.t, proof) != a.challengeParDPoPNonce { + w.Header().Set("DPoP-Nonce", a.challengeParDPoPNonce) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusBadRequest) + json.NewEncoder(w).Encode(map[string]any{ + "error": "use_dpop_nonce", + "error_description": "resubmit with the DPoP-Nonce value", + }) + return + } if a.messageSigned { requestJWT := r.PostForm.Get("request") if requestJWT == "" { @@ -316,6 +338,42 @@ func newTestClient(t *testing.T, messageSigned bool) (*client.Client, *fakeAS, * return c, as, ts } +// newTestClientWithPARBinding is newTestClient (baseline profile only) +// with an explicit Config.PARDPoPBinding, for tests that care which of +// the two RFC 9449 §10.1 mechanisms BeginAuthorization uses at PAR. +func newTestClientWithPARBinding(t *testing.T, binding client.PARDPoPBinding) (*client.Client, *fakeAS, *httptest.Server) { + t.Helper() + as := newFakeAS(t, testIssuer, false) + ts := httptest.NewServer(as.handler()) + t.Cleanup(ts.Close) + + cfg := validConfig(t) + cfg.PARDPoPBinding = binding + parURL, err := fapi.ParseEndpointURL(ts.URL+"/par", fapi.AllowLoopbackHTTP()) + if err != nil { + t.Fatalf("ParseEndpointURL(par): %v", err) + } + tokenURL, err := fapi.ParseEndpointURL(ts.URL+"/token", fapi.AllowLoopbackHTTP()) + if err != nil { + t.Fatalf("ParseEndpointURL(token): %v", err) + } + cfg.Endpoints.PushedAuthorizationRequest = parURL + cfg.Endpoints.Token = tokenURL + + deps := validDependencies(t) + deps.HTTP = ts.Client() + deps.IssuerKeys = &fakeIssuerKeySource{keys: map[keys.IssuerVerificationPurpose]crypto.PublicKey{ + keys.JARMVerification: &as.jarmKey.PublicKey, + keys.IDTokenVerification: &as.idTokenKey.PublicKey, + }} + + c, err := client.New(cfg, deps) + if err != nil { + t.Fatalf("client.New: %v", err) + } + return c, as, ts +} + // newTestClientWithEncryptedIDToken is newTestClient (baseline profile // only) plus a configured Decryption dependency and a fakeAS that // encrypts every ID token it issues to that same key under alg — for @@ -672,11 +730,11 @@ func TestBeginAuthorizationSendsClientIDInSignedRequestObject(t *testing.T) { // RFC 9449 §10: committing the DPoP key binding at PAR time (rather // than leaving it to whichever key first shows up at the token // endpoint) closes an authorization-code-injection window. This is the -// full round trip: the "dpop_jkt" sent at PAR must be the thumbprint of -// the exact key this client later presents a DPoP proof with at the -// token endpoint for the same authorization. -func TestBeginAuthorizationCommitsDPoPKeyAtPARMatchingTokenEndpointProof(t *testing.T) { - c, as, _ := newTestClient(t, false) +// full round trip under PARDPoPBindingJKT: the "dpop_jkt" sent at PAR +// must be the thumbprint of the exact key this client later presents a +// DPoP proof with at the token endpoint for the same authorization. +func TestBeginAuthorizationCommitsDPoPKeyAtPARMatchingTokenEndpointProofJKT(t *testing.T) { + c, as, _ := newTestClientWithPARBinding(t, client.PARDPoPBindingJKT) ctx := context.Background() session, err := c.BeginAuthorization(ctx, client.BeginAuthorizationRequest{Scope: []string{"openid"}}) @@ -687,6 +745,9 @@ func TestBeginAuthorizationCommitsDPoPKeyAtPARMatchingTokenEndpointProof(t *test if parJKT == "" { t.Fatalf("PAR form is missing dpop_jkt") } + if as.lastPARDPoPProof != "" { + t.Errorf("PAR request carried a DPoP header %q, want none under PARDPoPBindingJKT", as.lastPARDPoPProof) + } rawQuery := as.callbackFor(t, session.Handle().String(), "auth-code-dpop-jkt", "") if _, err := c.CompleteAuthorization(ctx, client.AuthorizationCallback{RawQuery: rawQuery}); err != nil { @@ -699,6 +760,55 @@ func TestBeginAuthorizationCommitsDPoPKeyAtPARMatchingTokenEndpointProof(t *test } } +// The same commitment property as above, but for PARDPoPBindingProof +// (the default, RFC 9449 §10.1's recommended mechanism): PAR presents +// an actual DPoP proof instead of dpop_jkt, and that proof's own key +// must match the one later presented at the token endpoint. +func TestBeginAuthorizationCommitsDPoPKeyAtPARMatchingTokenEndpointProofDefault(t *testing.T) { + c, as, _ := newTestClientWithPARBinding(t, client.PARDPoPBindingProof) + ctx := context.Background() + + session, err := c.BeginAuthorization(ctx, client.BeginAuthorizationRequest{Scope: []string{"openid"}}) + if err != nil { + t.Fatalf("BeginAuthorization: %v", err) + } + if as.lastPARForm.Get("dpop_jkt") != "" { + t.Errorf("PAR form carried dpop_jkt %q, want none under the default PARDPoPBindingProof", as.lastPARForm.Get("dpop_jkt")) + } + if as.lastPARDPoPProof == "" { + t.Fatalf("PAR request is missing a DPoP header") + } + parJKT := dpopProofJKT(t, as.lastPARDPoPProof) + + rawQuery := as.callbackFor(t, session.Handle().String(), "auth-code-dpop-proof", "") + if _, err := c.CompleteAuthorization(ctx, client.AuthorizationCallback{RawQuery: rawQuery}); err != nil { + t.Fatalf("CompleteAuthorization: %v", err) + } + + tokenJKT := dpopProofJKT(t, as.lastTokenDPoPProof) + if tokenJKT != parJKT { + t.Errorf("token endpoint DPoP proof key thumbprint = %q, want it to match PAR proof's key %q", tokenJKT, parJKT) + } +} + +// RFC 9449 §8: an authorization server that requires a DPoP nonce can +// challenge any DPoP proof it verifies, not just the token endpoint's — +// PARDPoPBindingProof means PAR now presents one for the first time, so +// it needs the same retry BeginAuthorization already gets from +// ExchangeCode at the token endpoint. +func TestBeginAuthorizationRetriesOnPARDPoPNonceChallenge(t *testing.T) { + c, as, _ := newTestClientWithPARBinding(t, client.PARDPoPBindingProof) + as.challengeParDPoPNonce = "server-issued-par-nonce-1" + ctx := context.Background() + + if _, err := c.BeginAuthorization(ctx, client.BeginAuthorizationRequest{Scope: []string{"openid"}}); err != nil { + t.Fatalf("BeginAuthorization: %v", err) + } + if as.parCallCount != 2 { + t.Errorf("PAR endpoint called %d times, want 2 (initial + nonce retry)", as.parCallCount) + } +} + // A JARM response carries no top-level "iss" query parameter — iss // lives inside the signed JWT claims instead, verified as part of // jarm.Verify — so RequireAuthorizationResponseIss must not reject a