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

[aron-muon]

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.93%. Comparing base (e532cf0) to head (fa2ba08).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6430      +/-   ##
==========================================
- Coverage   78.98%   78.93%   -0.05%     
==========================================
  Files         782      782              
  Lines       78065    78107      +42     
==========================================
- Hits        61658    61656       -2     
- Misses      16402    16446      +44     
  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.

[jhrozek]

Sorry this took us so long to review, and thank you for the contribution — the RFC 8707 use case and the end-to-end token-form coverage are both very helpful.

blocker: Please reserve client_assertion and client_assertion_type in ReservedTokenParams. They are RFC 7523 client-authentication credentials; allowing them through additionalTokenParams can combine them with ToolHive's configured client_secret or HTTP Basic authentication, producing an invalid multi-method client-auth request. It also allows an assertion credential to live in CRD configuration instead of a secret-backed credential path.

suggestion: When resource is configured, validate it as an absolute URI with no fragment. RFC 8707 requires that shape, while the current name-only validation accepts an empty, relative, malformed, or fragment-bearing value that will fail later at the authorization server.

suggestion: Please add BuildAuthServerRunConfig tests that assert AdditionalTokenParams propagates for both OIDC and OAuth2 upstreams. The direct HTTP-form tests are good, but the new CRD-to-runtime mappings can otherwise regress silently; the adjacent AdditionalAuthorizationParams test cases are a natural template.

[aron-muon]

Thanks, all three are in as of fa2ba08, on top of a merge with main.

blocker, client_assertion / client_assertion_type: both reserved. Your reasoning holds and I had not considered the multi-method client-auth case. I put them in ReservedTokenParams with a comment recording why, so the next person adding a key does not undo it.

resource shape: validated as an absolute URI with no fragment. The check lives in ValidateTokenParams, which is the single choke point for the CRD path (ValidateAdditionalTokenParams) and the runtime path (upstream/oauth2.go), so one implementation covers both. Empty, relative, fragment-bearing and unparseable values now fail at config time. Note the fragment check tests the raw string as well as url.URL.Fragment, because https://api.example.com/v1# parses to an empty fragment but is still fragment-bearing.

BuildAuthServerRunConfig tests: added for both OIDC and OAuth2, modelled on the adjacent AdditionalAuthorizationParams cases. New pkg/authserver/oauthparams table test covers the reserved keys and every rejected resource shape.

Verified with task lint, task test, task operator-test, task sdk-verify, task sdk-lint, task sdk-test and task build. The only local failure is pkg/runner/retriever/TestResolveMCPServer_WithoutGroup, which needs registry network access and passes in CI.

One thing worth flagging separately, since it cost me a round trip here. task docs is not reproducible locally: swag init --parseDependencyLevel 1 intermittently emits fully-qualified schema names (github_com_stacklok_toolhive_pkg_authserver.OIDCUpstreamRunConfig rather than authserver.OIDCUpstreamRunConfig). I reproduced it in a clean shallow clone of main at e532cf0, so it is not my tree. It also breaks task sdk-generate, which rejects the hyphen in thv-operator with schema name ... contains a source-name artifact. The pin comment in verify-docgen.yml already hints at this. Happy to open a separate issue if it is not already tracked.

[jhrozek]

Thanks for addressing the feedback — client_assertion/client_assertion_type are now reserved, the resource indicator is validated per RFC 8707, and the BuildAuthServerRunConfig tests cover both OIDC and OAuth2 propagation. LGTM.