Skip to content

Negotiate CIMD token endpoint auth method - #6400

Open
alex-feel wants to merge 1 commit into
stacklok:mainfrom
alex-feel:cimd-auth-methods-fallback
Open

Negotiate CIMD token endpoint auth method#6400
alex-feel wants to merge 1 commit into
stacklok:mainfrom
alex-feel:cimd-auth-methods-fallback

Conversation

@alex-feel

Copy link
Copy Markdown

Summary

CIMD documents from live clients (observed with ChatGPT) can declare a preferred token_endpoint_auth_method this server does not support while also publishing a plural token_endpoint_auth_methods_supported list (per OpenID Connect Relying Party Metadata Choices 1.0) that includes a method the server does support (none). The server rejected the whole document without ever consulting that list.

  • Add TokenEndpointAuthMethodsSupported to ClientMetadataDocument.
  • Negotiate the effective auth method: accept the declared singular method when supported, otherwise fall back to a mutually supported method from the plural list, otherwise reject with the existing error, which now names both fields.
  • This follows the same describes-capability-across-every-AS reading already applied to grant_types/response_types filtering for CIMD documents.

Fixes #6278

Type of change

  • Bug fix

Test plan

  • Manual testing (describe below)

Ran gofmt -l on the changed files, go build, and go vet across pkg/authserver/... and pkg/oauthproto/... (all clean), and go test -count=1 ./pkg/authserver/... ./pkg/oauthproto/... (all packages green).

  • Added TestFetch_TokenEndpointAuthMethodNegotiation covering three cases: an unsupported singular method rescued by none in the supported list, an unsupported singular method with no mutually supported method still rejected, and an omitted singular method with a plural list present accepted exactly as before.
  • The pre-existing TestFetch_RejectsUnsupportedTokenEndpointAuthMethod is unchanged and still green, so the previously pinned contract for singular-only documents holds.

Changes

File Change
pkg/oauthproto/cimd/fetch.go Add the TokenEndpointAuthMethodsSupported []string field to ClientMetadataDocument
pkg/authserver/storage/cimd_decorator.go Replace the outright-rejection guard with negotiateTokenEndpointAuthMethod; pass the negotiated method into buildFositeClient as a new parameter, removing its internal empty-to-none fallback
pkg/authserver/storage/cimd_decorator_test.go New negotiation test table; existing rejection test untouched; build-helper calls updated for the new parameter

Does this introduce a user-facing change?

Yes: CIMD clients previously rejected outright for declaring an unsupported preferred token_endpoint_auth_method are now accepted when their token_endpoint_auth_methods_supported list names a method this server supports (none). This is a pure widening: no document accepted today becomes rejected.

Special notes for reviewers

  • Two deliberate boundaries: (1) this only widens acceptance, it introduces no new rejections; (2) the server's supported-method set stays the constant none rather than the value the discovery endpoint advertises, because discovery may legitimately advertise client_secret_basic/client_secret_post for other client types, and CIMD documents are never allowed to declare symmetric methods.
  • The existing TestFetch_RejectsUnsupportedTokenEndpointAuthMethod test is unchanged and still passes, so the previously pinned contract for singular-only documents holds.

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CIMD rejects clients that advertise a supported fallback auth method

1 participant