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
69 changes: 53 additions & 16 deletions pkg/authserver/storage/cimd_decorator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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).
Expand Down
72 changes: 68 additions & 4 deletions pkg/authserver/storage/cimd_decorator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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()),
Expand Down Expand Up @@ -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 {
Expand Down
13 changes: 11 additions & 2 deletions pkg/oauthproto/cimd/fetch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading