Use Basic auth for OAuth2 upstreams

[jhrozek]

Summary

Some upstream identity providers require client_secret_basic for confidential clients and reject credentials sent in the token request body. Pre-registered (non-DCR) OAuth2 upstream clients had no way to select this: OAuth2UpstreamRunConfig had no token_endpoint_auth_method field, so buildPureOAuth2Config always left it empty, which downstream authStyleFromMethod maps to POST-body credentials.

• Add TokenEndpointAuthMethod to OAuth2UpstreamRunConfig, defaulting to client_secret_basic when a secret is configured and the field is left empty (matches the RFC 7591 §2 default already applied on the DCR-negotiated path).
• Propagate it through buildPureOAuth2Config into upstream.OAuth2Config, where the existing authStyleFromMethod selects the HTTP Basic auth style.
• Reject unsupported values and the invalid none-with-a-secret combination in OAuth2UpstreamRunConfig.Validate().
• Explicit client_secret_post and DCR-negotiated methods continue to work unchanged.

Fixes #6536

Type of change

• Bug fix

Test plan

• Unit tests (task test)

Added cases to TestBuildPureOAuth2Config: defaults to client_secret_basic when a secret is configured, stays empty for a public client without a secret, and preserves an explicit client_secret_post override.

API Compatibility

Not applicable — this PR does not touch operator CRD/API surface.

Does this introduce a user-facing change?

Yes: pre-registered OAuth2 upstream clients with a configured secret now authenticate at the upstream token endpoint using HTTP Basic auth by default, instead of always sending the secret in the POST body. Operators who need the previous behavior can set token_endpoint_auth_method: client_secret_post explicitly on the upstream config.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.69%. Comparing base (c4f6cb4) to head (3f7c057).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6543      +/-   ##
==========================================
+ Coverage   78.67%   78.69%   +0.02%     
==========================================
  Files         777      777              
  Lines       76686    76820     +134     
==========================================
+ Hits        60334    60457     +123     
- Misses      16347    16358      +11     
  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.

[JAORMX]

Blocking — operator-managed OAuth2 upstreams cannot retain client_secret_post

The new runtime field is not exposed by or copied by (, ). Thus an with a static client secret always arrives with an empty method and is forced to ; operators using an upstream that requires now have no compatible configuration path. Add the field to the CRD with the supported-value validation, propagate it into the RunConfig, regenerate generated CRD/docs artifacts, and cover that conversion.

Blocking — explicit confidential authentication accepts a missing secret

accepts and even if neither secret reference is configured (). The runner subsequently configures the selected confidential auth style with an empty secret (), deferring the failure to a token exchange and potentially emitting an empty credential. Reject confidential methods without a secret source (and reject an empty resolved secret), with tests for both methods.

CI is reported separately: the exact head has failing Swagger verification and Go linting checks; those failures are not the basis for this review decision.,

[JAORMX]

Correction to my prior review: shell quoting stripped code identifiers from that body. This is the complete review.

Blocking — operator-managed OAuth2 upstreams cannot retain client_secret_post

The new runtime field is not exposed by OAuth2UpstreamConfig or copied by buildOAuth2UpstreamRunConfig (cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go:1624; cmd/thv-operator/pkg/controllerutil/authserver.go:1368-1407). Thus an MCPExternalAuthConfig with a static client secret always arrives with an empty method and is forced to client_secret_basic; operators using an upstream that requires client_secret_post now have no compatible configuration path. Add the field to the CRD with supported-value validation, propagate it into RunConfig, regenerate generated CRD/docs artifacts, and cover that conversion.

Blocking — explicit confidential authentication accepts a missing secret

OAuth2UpstreamRunConfig.Validate accepts client_secret_basic and client_secret_post even if neither secret reference is configured (pkg/authserver/config.go:1427-1439). The runner subsequently configures the selected confidential auth style with an empty secret (pkg/authserver/runner/embeddedauthserver.go:749-755), deferring failure to a token exchange and potentially emitting an empty credential. Reject confidential methods without a secret source, and reject an empty resolved secret, with tests for both methods.

CI is reported separately: the exact head has failing Swagger verification and Go linting checks; those failures are not the basis for this review decision.

[JAORMX]

Fresh review of exact head 3f7c057 against main base 2533388.

Both prior blockers are resolved:

1. OAuth2UpstreamConfig exposes the constrained token_endpoint_auth_method, conversion copies it, and CRD/chart/API-doc generated artifacts include it (cmd/thv-operator/api/v1beta1/mcpexternalauthconfig_types.go:1657-1664; cmd/thv-operator/pkg/controllerutil/authserver.go:1368-1380; docs/operator/crd-api.md:3715).
2. client_secret_basic and client_secret_post require a configured source, while the conversion path rejects an empty resolved secret (pkg/authserver/config.go:1437-1457; pkg/authserver/runner/embeddedauthserver.go:747-760), with coverage for both absent-source and empty-file cases.

No blocking or non-blocking findings in the current range. Exact-head CI is green; CI is reported separately from this approval.