stacklok / stacklok/toolhive

OIDC upstream clients cannot use client_secret_basic (AuthStyle hardcoded)

Open
#6,542 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug needs-triage
Dominant language
Go
Stars
2.2k
Forks
300
Avg merge
1d 15h
Merged PRs (30d)
184

Description

Bug description

Confidential OIDC upstream clients have no way to authenticate at the token endpoint with client_secret_basic. Unlike the pure-OAuth2 path (fixed in #6536), this isn't a lossy config translation — it's an unconditional literal in the provider constructor itself.

In pkg/authserver/upstream/oidc.go, NewOIDCProvider builds its oauth2.Config with:

// Create the oauth2.Config for use with golang.org/x/oauth2 library
// Use go-oidc's endpoint which handles discovery, but explicitly set AuthStyle
// to ensure client credentials are sent in the request body (not Basic auth header)
// for consistent behavior across different IDP implementations.
providerEndpoint := oidcProvider.Endpoint()
p.oauth2Config = &oauth2.Config{
    ...
    Endpoint: oauth2.Endpoint{
        AuthURL:   providerEndpoint.AuthURL,
        TokenURL:  providerEndpoint.TokenURL,
        AuthStyle: oauth2.AuthStyleInParams,
    },
}

upstream.OIDCConfig has no TokenEndpointAuthMethod field, and neither does OIDCUpstreamRunConfig (pkg/authserver/config.go) or buildOIDCConfig (pkg/authserver/runner/embeddedauthserver.go). There is no config surface to override this, even in principle.

Why this is a separate issue from #6536

#6536's buildPureOAuth2Config fix added a field and default-filled it before handing off to the existing authStyleFromMethod helper — a translation-layer fix. Here there's no translation layer to fix: the literal lives inside NewOIDCProvider itself, with a comment asserting the choice is deliberate. Closing this gap means adding the field and replacing the hardcoded literal with a call to the same authStyleFromMethod helper the OAuth2 path already uses.

Expected behavior

A confidential OIDC upstream client with a configured secret should default to client_secret_basic (matching RFC 7591 §2's default, and OIDC Discovery 1.0 §3's default when a provider's discovery document omits token_endpoint_auth_methods_supported), while still allowing an explicit override to client_secret_post for providers that require it.

Proposed fix shape

  • Add TokenEndpointAuthMethod to CommonOAuthConfig (which upstream.OIDCConfig already embeds), rather than duplicating a parallel field onto OIDCConfig directly — keeps one field/one mapping function shared by both providers instead of two copies that can drift.
  • Add the mirrored field to OIDCUpstreamRunConfig; propagate it in buildOIDCConfig the same way buildPureOAuth2Config does today.
  • Replace the hardcoded AuthStyle: oauth2.AuthStyleInParams in NewOIDCProvider with authStyleFromMethod(config.TokenEndpointAuthMethod), keeping AuthURL/TokenURL from oidcProvider.Endpoint() (go-oidc's discovery) but letting the configured/defaulted method — not go-oidc's own endpoint guess — decide AuthStyle.

Additional context

  • OIDC discovery uniquely publishes token_endpoint_auth_methods_supported, which bare OAuth2 endpoints don't have. A stretch goal beyond "default to Basic" would be respecting that list when present rather than a static default — go-oidc's Provider.Endpoint() doesn't currently expose it, so this would need to read the raw discovery claims separately. Filing this issue scoped to the static default first; the discovery-aware version can be a follow-up if wanted.
  • No known-broken real-world IdP identified that would regress from this default, but flagging as an open risk given some non-conformant providers have historically rejected Basic auth.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with NewOIDCProvider in pkg/authserver/upstream/oidc.go, then compare the existing field propagation in buildPureOAuth2Config and authStyleFromMethod. Update the shared OIDC configuration path through pkg/authserver/config.go and pkg/authserver/runner/embeddedauthserver.go. Done means OIDC defaults to client_secret_basic while allowing an explicit client_secret_post override.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
authentication
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.