Skip to content

feat(authserver): support additional token-request params (RFC 8707 resource indicators) - #6430

Open
aron-muon wants to merge 1 commit into
stacklok:mainfrom
aron-muon:feat/additional-token-params
Open

feat(authserver): support additional token-request params (RFC 8707 resource indicators)#6430
aron-muon wants to merge 1 commit into
stacklok:mainfrom
aron-muon:feat/additional-token-params

Conversation

@aron-muon

Copy link
Copy Markdown
Contributor

Problem

Some authorization servers enforce RFC 8707 resource indicators on token requests, not just authorization requests: the code exchange and refresh are rejected with invalid_target unless resource is present in the POST form body (query-string placement is ignored).

additionalAuthorizationParams only reaches the authorize URL, so against such an AS the embedded auth server passes authorization and then fails at the code exchange. Live example: Nominal's MCP authorization server (api.gov.nominal.io) — probing its token endpoint with a bogus code returns {"error":"invalid_target"} without resource in the body, and proceeds to code validation (reauthorization_required) with it.

Change

Adds AdditionalTokenParams alongside AdditionalAuthorizationParams:

  • upstream.CommonOAuthConfig gains AdditionalTokenParams, applied in BaseOAuth2Provider.exchangeCodeForTokens and RefreshTokens via oauth2.SetAuthURLParam options, which land in the POST form body on Exchange. OIDC providers inherit both paths through the embedded base provider.
  • Reserved-parameter validation mirrors the authorization-side list with token-request semantics: grant_type, code, redirect_uri, client_id, client_secret, code_verifier, refresh_token, and scope are rejected.
  • CRD: additionalTokenParams on both oidcConfig and oauth2Config upstream provider types, plumbed through the operator run-config builders and validated at reconcile time (MCPExternalAuthConfig and VirtualMCPServer), matching the additionalAuthorizationParams treatment.
  • Regenerated deepcopy, CRD manifests, and CRD API docs.

Testing

  • New unit tests: reserved-param validation table for token params, and an httptest-backed test asserting the configured params appear in the token endpoint's POST form body on both the authorization-code exchange and the refresh grant.
  • Extended the operator's upstream-provider validation table with valid/reserved additionalTokenParams cases for both provider types.
  • go build ./..., go test ./pkg/authserver/... ./cmd/thv-operator/api/v1beta1/ ./cmd/thv-operator/pkg/controllerutil/ ./cmd/thv-operator/controllers/ all pass.

🤖 Generated with Claude Code

Some authorization servers enforce RFC 8707 resource indicators on token
requests as well as authorization requests: the code exchange and refresh
are rejected with invalid_target unless the resource parameter is present
in the POST form body (query-string placement is ignored). Nominal's MCP
authorization server (api.gov.nominal.io) is a live example — with only
additionalAuthorizationParams, the flow passes authorization and then
fails at the code exchange.

Add AdditionalTokenParams alongside AdditionalAuthorizationParams:

- upstream.CommonOAuthConfig gains AdditionalTokenParams, applied in
  BaseOAuth2Provider.exchangeCodeForTokens and RefreshTokens via
  oauth2.SetAuthURLParam options (which land in the POST form body on
  Exchange). OIDC providers inherit both paths through the embedded
  base provider.
- Reserved-parameter validation mirrors the authorization-side list with
  token-request semantics: grant_type, code, redirect_uri, client_id,
  client_secret, code_verifier, refresh_token, and scope are rejected.
- CRD: additionalTokenParams on both oidcConfig and oauth2Config upstream
  provider types, plumbed through the operator run-config builders and
  validated at reconcile time (MCPExternalAuthConfig and VirtualMCPServer),
  matching the additionalAuthorizationParams treatment.
- Regenerated deepcopy, CRD manifests, and CRD API docs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Aron Gates <aron@muonspace.com>
@codecov

codecov Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.81%. Comparing base (7c9c55e) to head (46eb005).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6430      +/-   ##
==========================================
- Coverage   77.81%   77.81%   -0.01%     
==========================================
  Files         760      760              
  Lines       73133    73160      +27     
==========================================
+ Hits        56908    56926      +18     
- Misses      16220    16229       +9     
  Partials        5        5              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant