From 4f321f6fea4da0ce602d906b4cef95b649cc3938 Mon Sep 17 00:00:00 2001 From: Aleksandr Filippov <71711753+alex-feel@users.noreply.github.com> Date: Thu, 20 Aug 2026 15:58:45 +0300 Subject: [PATCH] Negotiate CIMD token endpoint auth method CIMD documents from live clients can declare a preferred token_endpoint_auth_method this server does not support while also publishing a plural token_endpoint_auth_methods_supported list (OpenID Connect RP Metadata Choices 1.0) that includes a method the server does support, and the server rejected the whole document without ever looking at that list. Add TokenEndpointAuthMethodsSupported to ClientMetadataDocument and negotiate the effective auth method from it before falling back to the prior outright rejection, mirroring the same describes-capability-across-every-AS reading the grant_types/response_types filtering already applies to CIMD documents. The negotiated set stays the server's own supported constant rather than the AS discovery document's advertised methods, since discovery may legitimately advertise symmetric methods that CIMD documents are never allowed to declare. The change is a pure widening: no document accepted today becomes rejected, and the existing singular-only rejection test is unchanged. Signed-off-by: Aleksandr Filippov <71711753+alex-feel@users.noreply.github.com> --- pkg/authserver/storage/cimd_decorator.go | 69 +++++++++++++----- pkg/authserver/storage/cimd_decorator_test.go | 72 +++++++++++++++++-- pkg/oauthproto/cimd/fetch.go | 13 +++- 3 files changed, 132 insertions(+), 22 deletions(-) diff --git a/pkg/authserver/storage/cimd_decorator.go b/pkg/authserver/storage/cimd_decorator.go index 330f3e521c..89e8ef3b80 100644 --- a/pkg/authserver/storage/cimd_decorator.go +++ b/pkg/authserver/storage/cimd_decorator.go @@ -145,15 +145,22 @@ func (d *CIMDStorageDecorator) fetch(ctx context.Context, id string) (fosite.Cli return nil, fmt.Errorf("%w: %w", fosite.ErrNotFound.WithHint("CIMD fetch failed"), err) } - // Reject documents that declare an auth method this AS does not support. - // ErrInvalidClient: the document was fetched successfully but its declared - // metadata violates AS policy (distinct from ErrNotFound which means the - // document could not be fetched at all). - if m := doc.TokenEndpointAuthMethod; m != "" && m != defaultCIMDTokenEndpointAuthMethod { + // Negotiate the effective token_endpoint_auth_method rather than rejecting + // outright on a declared-but-unsupported singular value. ErrInvalidClient: + // the document was fetched successfully but its declared metadata violates + // AS policy (distinct from ErrNotFound which means the document could not + // be fetched at all). + authMethod, ok := negotiateTokenEndpointAuthMethod(doc) + if !ok { return nil, fmt.Errorf("%w: CIMD document at %s claims token_endpoint_auth_method %q "+ - "but this server only supports %q", + "but this server only supports %q (token_endpoint_auth_methods_supported: %v)", fosite.ErrInvalidClient.WithHint("unsupported token_endpoint_auth_method"), - id, m, defaultCIMDTokenEndpointAuthMethod) + id, doc.TokenEndpointAuthMethod, defaultCIMDTokenEndpointAuthMethod, + doc.TokenEndpointAuthMethodsSupported) + } + if doc.TokenEndpointAuthMethod != "" && authMethod != doc.TokenEndpointAuthMethod { + slog.Debug("CIMD: negotiated token_endpoint_auth_method from the client's supported list", + "client_id", id, "declared", doc.TokenEndpointAuthMethod, "effective", authMethod) } // Filter — not reject — grant_types and response_types this AS does not @@ -224,7 +231,7 @@ func (d *CIMDStorageDecorator) fetch(ctx context.Context, id string) (fosite.Cli resolvedScopes = registration.UnionScopes(resolvedScopes, d.baselineClientScopes) } - client := buildFositeClient(doc, resolvedScopes, grantTypes, responseTypes) + client := buildFositeClient(doc, resolvedScopes, grantTypes, responseTypes, authMethod) d.cache.Add(id, &cimdCacheEntry{ client: client, @@ -235,11 +242,41 @@ func (d *CIMDStorageDecorator) fetch(ctx context.Context, id string) (fosite.Cli } // defaultCIMDTokenEndpointAuthMethod is the token endpoint authentication -// method applied when the CIMD document omits token_endpoint_auth_method. -// Documents that declare any other value are rejected by fetch() before -// buildFositeClient is called. +// method applied when the CIMD document omits token_endpoint_auth_method, and +// the only method this server ever selects when negotiating a fallback for a +// declared-but-unsupported value. Documents whose declared method is neither +// this value nor negotiable to it via negotiateTokenEndpointAuthMethod are +// rejected by fetch() before buildFositeClient is called. const defaultCIMDTokenEndpointAuthMethod = "none" +// negotiateTokenEndpointAuthMethod resolves the effective token endpoint auth +// method for a CIMD document. The declared singular TokenEndpointAuthMethod is +// used when it is empty or already equal to defaultCIMDTokenEndpointAuthMethod. +// When it names anything else, the plural TokenEndpointAuthMethodsSupported +// (OpenID Connect RP Metadata Choices 1.0 — not part of the CIMD draft) is +// consulted: if it contains defaultCIMDTokenEndpointAuthMethod, the document +// is accepted with that as the negotiated method, since the client itself +// declared willingness to use it. Otherwise negotiation fails and ok is false. +// +// The supported set this function negotiates against is deliberately the +// defaultCIMDTokenEndpointAuthMethod constant alone, never the AS discovery +// document's own token_endpoint_auth_methods_supported: discovery legitimately +// advertises client_secret_basic/client_secret_post when confidential DCR or +// static delegate clients are enabled, and the CIMD draft (§4.1) forbids those +// symmetric methods in CIMD documents outright. Deriving the negotiated set +// from discovery would let a CIMD document negotiate its way into a forbidden +// method, so no such config plumbing is introduced. +func negotiateTokenEndpointAuthMethod(doc *cimd.ClientMetadataDocument) (string, bool) { + declared := doc.TokenEndpointAuthMethod + if declared == "" || declared == defaultCIMDTokenEndpointAuthMethod { + return defaultCIMDTokenEndpointAuthMethod, true + } + if slices.Contains(doc.TokenEndpointAuthMethodsSupported, defaultCIMDTokenEndpointAuthMethod) { + return defaultCIMDTokenEndpointAuthMethod, true + } + return "", false +} + // buildFositeClient converts a ClientMetadataDocument into a fosite.Client. // RFC 8252 §7.3 loopback dynamic-port matching for a "http://localhost" redirect // URI is provided generically by registration.RegisteredLoopbackRedirectURI @@ -253,14 +290,14 @@ const defaultCIMDTokenEndpointAuthMethod = "none" // empty (the filters apply defaults and reject empty intersections). The // stored client therefore carries only grant/response types this server can // actually serve, not the document's full declaration. +// tokenEndpointAuthMethod is the already-negotiated value computed by fetch() +// via negotiateTokenEndpointAuthMethod; this function no longer applies its +// own empty-to-default fallback, so the resolver is the single authority over +// which method a CIMD-derived client ends up with. func buildFositeClient( doc *cimd.ClientMetadataDocument, resolvedScopes, grantTypes, responseTypes []string, + tokenEndpointAuthMethod string, ) fosite.Client { - tokenEndpointAuthMethod := doc.TokenEndpointAuthMethod - if tokenEndpointAuthMethod == "" { - tokenEndpointAuthMethod = defaultCIMDTokenEndpointAuthMethod - } - // Scopes were computed and validated by fetch() via registration.ValidateScopes, // consistent with the DCR handler. Fall back to DefaultScopes only when the // decorator has no ScopesSupported restriction (unconstrained AS). diff --git a/pkg/authserver/storage/cimd_decorator_test.go b/pkg/authserver/storage/cimd_decorator_test.go index f2754c2851..0ce4ad196f 100644 --- a/pkg/authserver/storage/cimd_decorator_test.go +++ b/pkg/authserver/storage/cimd_decorator_test.go @@ -357,11 +357,12 @@ func TestCIMDStorageDecorator_GetClient_CIMDURLHitsCacheDirectly(t *testing.T) { // buildFositeClientWithDefaults calls buildFositeClient with the standard // filtered grant/response types (what FilterPublicGrantTypes / -// FilterPublicResponseTypes return for an omitted declaration), for tests -// that don't exercise those fields. +// FilterPublicResponseTypes return for an omitted declaration) and the +// default negotiated auth method, for tests that don't exercise those fields. func buildFositeClientWithDefaults(doc *cimd.ClientMetadataDocument, scopes []string) fosite.Client { return buildFositeClient(doc, scopes, - []string{"authorization_code", "refresh_token"}, []string{"code"}) + []string{"authorization_code", "refresh_token"}, []string{"code"}, + defaultCIMDTokenEndpointAuthMethod) } func TestBuildFositeClient_PassesThroughGrantAndResponseTypes(t *testing.T) { @@ -375,7 +376,8 @@ func TestBuildFositeClient_PassesThroughGrantAndResponseTypes(t *testing.T) { GrantTypes: []string{"authorization_code", "urn:ietf:params:oauth:grant-type:device_code"}, } - got := buildFositeClient(doc, nil, []string{"authorization_code"}, []string{"code"}) + got := buildFositeClient(doc, nil, []string{"authorization_code"}, []string{"code"}, + defaultCIMDTokenEndpointAuthMethod) assert.Equal(t, "https://example.com/meta.json", got.GetID()) assert.True(t, got.IsPublic()) assert.ElementsMatch(t, []string{"authorization_code"}, []string(got.GetGrantTypes()), @@ -467,6 +469,68 @@ func TestFetch_RejectsUnsupportedTokenEndpointAuthMethod(t *testing.T) { assert.NotErrorIs(t, err, fosite.ErrNotFound) } +// TestFetch_TokenEndpointAuthMethodNegotiation exercises the negotiation +// introduced for #6278: a declared-but-unsupported singular +// token_endpoint_auth_method is rescued when the plural +// token_endpoint_auth_methods_supported list names a method this server does +// support, instead of the document being rejected outright. +func TestFetch_TokenEndpointAuthMethodNegotiation(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + declared string + supportedList []string + wantErr bool + wantAuthMethod string + }{ + { + name: "unsupported singular rescued by none in the supported list", + declared: "private_key_jwt", + supportedList: []string{"none", "private_key_jwt"}, + wantErr: false, + wantAuthMethod: "none", + }, + { + name: "unsupported singular with no none in the supported list stays rejected", + declared: "private_key_jwt", + supportedList: []string{"private_key_jwt", "tls_client_auth"}, + wantErr: true, + }, + { + name: "omitted singular with a supported list is unaffected — accepted as none", + declared: "", + supportedList: []string{"private_key_jwt"}, + wantErr: false, + wantAuthMethod: "none", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + srv := serveCIMDDocWithFields(t, func(doc *cimd.ClientMetadataDocument) { + doc.TokenEndpointAuthMethod = tt.declared + doc.TokenEndpointAuthMethodsSupported = tt.supportedList + }) + dec := newEnabledDecorator(t, newTestBase(t), 10, time.Minute) + client, err := dec.fetchOrCached(context.Background(), srv.URL+"/meta.json") + if tt.wantErr { + require.Error(t, err) + assert.ErrorIs(t, err, fosite.ErrInvalidClient, + "CIMD policy rejections must use ErrInvalidClient, not ErrNotFound") + assert.NotErrorIs(t, err, fosite.ErrNotFound) + return + } + require.NoError(t, err) + assert.True(t, client.IsPublic()) + oidc, ok := client.(fosite.OpenIDConnectClient) + require.True(t, ok, "client must implement fosite.OpenIDConnectClient") + assert.Equal(t, tt.wantAuthMethod, oidc.GetTokenEndpointAuthMethod()) + }) + } +} + // serveCIMDDocWithFields starts an httptest.Server that serves a CIMD document // customised by the provided mutator function. Pass nil for a plain valid doc. func serveCIMDDocWithFields(t *testing.T, mutate func(*cimd.ClientMetadataDocument)) *httptest.Server { diff --git a/pkg/oauthproto/cimd/fetch.go b/pkg/oauthproto/cimd/fetch.go index 0ba07bfee2..9f9976cfa0 100644 --- a/pkg/oauthproto/cimd/fetch.go +++ b/pkg/oauthproto/cimd/fetch.go @@ -41,8 +41,17 @@ type ClientMetadataDocument struct { ResponseTypes []string `json:"response_types,omitempty"` Scope string `json:"scope,omitempty"` TokenEndpointAuthMethod string `json:"token_endpoint_auth_method,omitempty"` - ApplicationType string `json:"application_type,omitempty"` - PostLogoutRedirectURIs []string `json:"post_logout_redirect_uris,omitempty"` + // TokenEndpointAuthMethodsSupported is not part of the CIMD draft itself; + // it is defined by OpenID Connect RP Metadata Choices 1.0 as the plural + // counterpart of TokenEndpointAuthMethod, listing every method the client + // supports while TokenEndpointAuthMethod names its preferred one. Live + // clients (e.g. ChatGPT) publish it alongside a stricter singular + // preference the client_id-issuing party does not control per-AS; the + // decorator in pkg/authserver/storage uses it to negotiate a mutually + // supported method instead of rejecting the document outright (#6278). + TokenEndpointAuthMethodsSupported []string `json:"token_endpoint_auth_methods_supported,omitempty"` + ApplicationType string `json:"application_type,omitempty"` + PostLogoutRedirectURIs []string `json:"post_logout_redirect_uris,omitempty"` } // forbiddenAuthMethods lists token_endpoint_auth_method values that MUST NOT