Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions client/begin_authorization.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,26 +221,28 @@
return nil, newError(ErrorInternal, "failed to build pushed authorization request", buildErr)
}

body, status, header, err := c.postParRequestWithDPoP(ctx, dpopSigner, &parURL, form, "")
body, status, header, err := c.postParRequestWithDPoP(ctx, dpopSigner, &parURL, form, c.cachedDPoPNonce(ctx, asNonceScope))
if err != nil {
return nil, newError(ErrorInternal, "pushed authorization request failed", err)
}
nextNonce := header.Get("DPoP-Nonce")
c.cacheDPoPNonce(ctx, asNonceScope, nextNonce)
if status == http.StatusCreated || status == http.StatusOK {
return body, nil
}

nonce := header.Get("DPoP-Nonce")
if nonce == "" || !isDPoPNonceError(body) {
if nextNonce == "" || !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)
body, status, header, err = c.postParRequestWithDPoP(ctx, dpopSigner, &parURL, retryForm, nextNonce)
if err != nil {
return nil, newError(ErrorInternal, "pushed authorization request failed", err)
}
c.cacheDPoPNonce(ctx, asNonceScope, header.Get("DPoP-Nonce"))
if status != http.StatusCreated && status != http.StatusOK {
return nil, parErrorFromResponse(body)
}
Expand All @@ -267,7 +269,7 @@
// assertion for authentication, plus either a signed request object
// (ProfileFAPISecurityWithMessageSigning) or the plain authorization
// parameters directly.
func (c *Client) buildPushedRequestForm(ctx context.Context, now time.Time, params map[string]string, extensions extension.Values) (map[string]string, *Error) {

Check failure on line 272 in client/begin_authorization.go

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 16 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=IDFoundry_FAPIgo&issues=AaBEZGRIbIkXIHHZRbe3&open=AaBEZGRIbIkXIHHZRbe3&pullRequest=170
assertionSigner, assertionKID, err := c.newSigner(ctx, keys.ClientAuthentication, c.cfg.Algorithms.ClientAuthentication)
if err != nil {
return nil, newError(ErrorInternal, "failed to resolve client authentication key", err)
Expand Down
8 changes: 8 additions & 0 deletions client/dependencies.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,12 @@ type Dependencies struct {
// this stays an opt-in dependency rather than a mandatory one every
// embedder has to wire up.
Decryption keys.Decrypter

// DPoPNonceCache lets this client proactively reuse a DPoP nonce a
// server already handed it, instead of always paying the challenge
// round trip RFC 9449 §8/§9 otherwise requires on every call — see
// its own doc comment. Nil (the zero value) disables the
// optimization entirely; pass NewInMemoryDPoPNonceCache() to enable
// it, the same deliberate opt-in Clock: SystemClock{} already is.
DPoPNonceCache DPoPNonceCache
}
10 changes: 6 additions & 4 deletions client/exchange_code.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,26 +216,28 @@ func (c *Client) ExchangeCode(ctx context.Context, resp ValidatedAuthorizationRe
// client assertion is exactly as single-use as a DPoP proof is (see
// ExchangeCode's own comment on buildTokenForm).
func (c *Client) sendTokenRequest(ctx context.Context, dpopSigner crypto.Signer, tokenURL *url.URL, buildTokenForm func() ([]byte, error), form []byte) ([]byte, *Error) {
body, status, header, err := c.postTokenRequestWithDPoP(ctx, dpopSigner, tokenURL, form, "")
body, status, header, err := c.postTokenRequestWithDPoP(ctx, dpopSigner, tokenURL, form, c.cachedDPoPNonce(ctx, asNonceScope))
if err != nil {
return nil, newError(ErrorInternal, "token request failed", err)
}
nextNonce := header.Get("DPoP-Nonce")
c.cacheDPoPNonce(ctx, asNonceScope, nextNonce)
if status == http.StatusOK {
return body, nil
}

nonce := header.Get("DPoP-Nonce")
if nonce == "" || !isDPoPNonceError(body) {
if nextNonce == "" || !isDPoPNonceError(body) {
return nil, parErrorFromResponse(body)
}
retryForm, buildErr := buildTokenForm()
if buildErr != nil {
return nil, newError(ErrorInternal, "failed to build client assertion", buildErr)
}
body, status, _, err = c.postTokenRequestWithDPoP(ctx, dpopSigner, tokenURL, retryForm, nonce)
body, status, header, err = c.postTokenRequestWithDPoP(ctx, dpopSigner, tokenURL, retryForm, nextNonce)
if err != nil {
return nil, newError(ErrorInternal, "token request failed", err)
}
c.cacheDPoPNonce(ctx, asNonceScope, header.Get("DPoP-Nonce"))
if status != http.StatusOK {
return nil, parErrorFromResponse(body)
}
Expand Down
136 changes: 136 additions & 0 deletions client/flow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,13 @@ type fakeAS struct {
challengeParDPoPNonce string
parCallCount int

// nextDPoPNonce, if non-empty, makes both handlePAR and handleToken
// set a "DPoP-Nonce" response header on every successful response —
// the proactive-refresh behavior RFC 9449 §8 recommends and
// server's own NextDPoPNonce implements — so a test can check that
// client actually reuses it on its next, independent call.
nextDPoPNonce string

// 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").
Expand Down Expand Up @@ -155,6 +162,9 @@ func (a *fakeAS) handlePAR(w http.ResponseWriter, r *http.Request) {
a.lastNonce = r.PostForm.Get("nonce")
}

if a.nextDPoPNonce != "" {
w.Header().Set("DPoP-Nonce", a.nextDPoPNonce)
}
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusCreated)
json.NewEncoder(w).Encode(map[string]any{
Expand Down Expand Up @@ -294,6 +304,9 @@ func (a *fakeAS) handleToken(w http.ResponseWriter, r *http.Request) {
resp["expires_in"] = 300
}

if a.nextDPoPNonce != "" {
w.Header().Set("DPoP-Nonce", a.nextDPoPNonce)
}
w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(resp)
}
Expand Down Expand Up @@ -374,6 +387,45 @@ func newTestClientWithPARBinding(t *testing.T, binding client.PARDPoPBinding) (*
return c, as, ts
}

// newTestClientWithNonceCache is newTestClientWithPARBinding
// (PARDPoPBindingProof, so PAR presents a DPoP proof and is
// nonce-challengeable) plus a wired-in DPoPNonceCache, for tests
// checking that a cached nonce is proactively reused across
// independent calls.
func newTestClientWithNonceCache(t *testing.T) (*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 = client.PARDPoPBindingProof
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.DPoPNonceCache = client.NewInMemoryDPoPNonceCache()
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
Expand Down Expand Up @@ -796,6 +848,90 @@ func TestBeginAuthorizationCommitsDPoPKeyAtPARMatchingTokenEndpointProofDefault(
// 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.
// RFC 9449 §8's own proactive-refresh recommendation only helps a
// client that actually tracks and reuses the nonce a response handed
// it. With a DPoPNonceCache configured, a second, independent
// authorization flow through the same Client must need no
// challenge/retry round trip at either PAR or the token endpoint,
// since both share one cached nonce (Dependencies.DPoPNonceCache,
// asNonceScope) seeded by the first flow's own successful responses.
func TestBeginAuthorizationAndExchangeCodeReuseCachedNonceOnSecondFlow(t *testing.T) {
c, as, _ := newTestClientWithNonceCache(t)
as.challengeParDPoPNonce = "server-nonce-1"
as.challengeDPoPNonce = "server-nonce-1"
as.nextDPoPNonce = "server-nonce-1"
ctx := context.Background()

// First flow: PAR has no cached nonce yet, so it must be challenged
// once — but that challenge's own successful retry response caches
// a nonce into the *shared* "as" scope before ExchangeCode ever
// calls the token endpoint, so the token endpoint's very first
// attempt already succeeds. PAR priming the token call this way is
// exactly the point of sharing one scope between them.
session, err := c.BeginAuthorization(ctx, client.BeginAuthorizationRequest{Scope: []string{"openid", "accounts"}})
if err != nil {
t.Fatalf("BeginAuthorization (first): %v", err)
}
if as.parCallCount != 2 {
t.Fatalf("first flow PAR call count = %d, want 2 (challenge + retry)", as.parCallCount)
}
rawQuery := as.callbackFor(t, session.Handle().String(), "auth-code-1", "")
if _, err := c.CompleteAuthorization(ctx, client.AuthorizationCallback{RawQuery: rawQuery}); err != nil {
t.Fatalf("CompleteAuthorization (first): %v", err)
}
if as.tokenCallCount != 1 {
t.Fatalf("first flow token call count = %d, want 1 (already primed by PAR's own cached nonce)", as.tokenCallCount)
}

// Second, independent flow: both must succeed on the very first
// attempt, since the client proactively sends the nonce it cached
// from the first flow's own successful responses.
session, err = c.BeginAuthorization(ctx, client.BeginAuthorizationRequest{Scope: []string{"openid", "accounts"}})
if err != nil {
t.Fatalf("BeginAuthorization (second): %v", err)
}
if as.parCallCount != 3 {
t.Fatalf("PAR call count after second BeginAuthorization = %d, want 3 (no retry needed)", as.parCallCount)
}
rawQuery = as.callbackFor(t, session.Handle().String(), "auth-code-2", "")
if _, err := c.CompleteAuthorization(ctx, client.AuthorizationCallback{RawQuery: rawQuery}); err != nil {
t.Fatalf("CompleteAuthorization (second): %v", err)
}
if as.tokenCallCount != 2 {
t.Fatalf("token call count after second exchange = %d, want 2 (no retry needed)", as.tokenCallCount)
}
}

// A cached nonce that turns out to be stale must still recover via the
// existing challenge/retry logic — caching only ever removes a round
// trip, it never introduces a new failure mode.
func TestBeginAuthorizationRecoversFromStaleCachedNonce(t *testing.T) {
c, as, _ := newTestClientWithNonceCache(t)
ctx := context.Background()

// Seed the cache with "first-nonce" via one real flow.
as.challengeParDPoPNonce = "first-nonce"
as.nextDPoPNonce = "first-nonce"
if _, err := c.BeginAuthorization(ctx, client.BeginAuthorizationRequest{Scope: []string{"openid"}}); err != nil {
t.Fatalf("BeginAuthorization (seed): %v", err)
}
if as.parCallCount != 2 {
t.Fatalf("PAR call count after seed flow = %d, want 2 (challenge + retry)", as.parCallCount)
}

// The server has since rotated its nonce — the client's cached
// value ("first-nonce") is now stale, but the flow must still
// succeed via its own challenge/retry.
as.challengeParDPoPNonce = "rotated-nonce"
as.nextDPoPNonce = "rotated-nonce"
if _, err := c.BeginAuthorization(ctx, client.BeginAuthorizationRequest{Scope: []string{"openid"}}); err != nil {
t.Fatalf("BeginAuthorization (stale cache): %v", err)
}
if as.parCallCount != 4 {
t.Fatalf("PAR call count = %d, want 4 (2 from the seed flow + 2 more: the second flow still needed its own challenge/retry)", as.parCallCount)
}
}

func TestBeginAuthorizationRetriesOnPARDPoPNonceChallenge(t *testing.T) {
c, as, _ := newTestClientWithPARBinding(t, client.PARDPoPBindingProof)
as.challengeParDPoPNonce = "server-issued-par-nonce-1"
Expand Down
97 changes: 97 additions & 0 deletions client/noncecache.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
package client

import (
"context"
"net/url"
"sync"
)

// asNonceScope is the DPoPNonceCache key shared by the token endpoint
// and PAR — a Client is already bound to exactly one issuer, and
// server's own Dependencies.Nonces treats PAR and the token endpoint as
// one shared nonce space (RFC 9449 §8), so a nonce obtained from either
// is valid to present to either.
const asNonceScope = "as"

// resourceNonceScope is a ResourceClient.Do call's DPoPNonceCache key:
// per resource-server origin, since RFC 9449 §9 nonces are scoped to
// the resource server that issued them and Do can be pointed at any
// resource URL, not just one fixed endpoint.
func resourceNonceScope(u *url.URL) string {
return u.Scheme + "://" + u.Host
}

// DPoPNonceCache remembers the most recent DPoP nonce a server handed
// this client for a given scope, so the next call can present it
// proactively instead of always paying the initial challenge round
// trip RFC 9449 §8/§9 otherwise requires. It is a hint, not a ledger —
// losing it, or a stale/wrong value, only costs one extra
// challenge/retry round trip on the next call, recovered by the
// existing retry logic exactly as if this cache didn't exist at all.
// Dependencies.DPoPNonceCache being nil disables the optimization
// entirely — every call behaves exactly as it always has.
type DPoPNonceCache interface {
// Get returns the nonce last cached for scope, and whether one was
// found at all.
Get(ctx context.Context, scope string) (nonce string, ok bool)

// Set replaces the cached nonce for scope. Called with whatever a
// response's own "DPoP-Nonce" header carried, regardless of that
// response's status — a server may send one on an unrelated error,
// or on success, purely to pre-seed the caller's next request.
Set(ctx context.Context, scope string, nonce string)
}

// InMemoryDPoPNonceCache is a DPoPNonceCache backed by a plain map, safe
// for concurrent use. It is a ready-made implementation a caller can
// wire in explicitly — supplying it is as deliberate a choice as
// Clock: SystemClock{} is; New never installs one on its own.
type InMemoryDPoPNonceCache struct {
mu sync.Mutex
nonces map[string]string
}

// NewInMemoryDPoPNonceCache builds an empty InMemoryDPoPNonceCache.
func NewInMemoryDPoPNonceCache() *InMemoryDPoPNonceCache {
return &InMemoryDPoPNonceCache{nonces: make(map[string]string)}
}

// Get implements DPoPNonceCache.
func (c *InMemoryDPoPNonceCache) Get(_ context.Context, scope string) (string, bool) {
c.mu.Lock()
defer c.mu.Unlock()
nonce, ok := c.nonces[scope]
return nonce, ok
}

// Set implements DPoPNonceCache.
func (c *InMemoryDPoPNonceCache) Set(_ context.Context, scope string, nonce string) {
c.mu.Lock()
defer c.mu.Unlock()
c.nonces[scope] = nonce
}

// cachedDPoPNonce returns the nonce cached for scope, or "" if
// Dependencies.DPoPNonceCache is nil or has none — the same value every
// nonce-aware call site used unconditionally before this cache existed.
func (c *Client) cachedDPoPNonce(ctx context.Context, scope string) string {
if c.deps.DPoPNonceCache == nil {
return ""
}
nonce, ok := c.deps.DPoPNonceCache.Get(ctx, scope)
if !ok {
return ""
}
return nonce
}

// cacheDPoPNonce records nonce for scope, if both a cache is configured
// and nonce is non-empty — a response with no "DPoP-Nonce" header
// leaves whatever was cached before untouched, rather than overwriting
// it with an empty value.
func (c *Client) cacheDPoPNonce(ctx context.Context, scope, nonce string) {
if c.deps.DPoPNonceCache == nil || nonce == "" {
return
}
c.deps.DPoPNonceCache.Set(ctx, scope, nonce)
}
Loading
Loading