Skip to content

tsidp appears to require client secrets #174

Description

@thomwiggers

I was trying to set up Incus OIDC with this project, but it appears that Incus can't use a client secret. This appears to still be in spec for OIDC. It's fine if that doesn't work for tsidp somehow, but then tsidp should reject this client on self-registration (setting "token_endpoint_auth_method": "none" is accepted).

In particular, whatever is set through token_endpoint_auth_method is completely ignored, because in token.go it will just always attempt Basic auth first before seeing if there are any form values set.

https://github.com/tailscale/tsidp/blob/main/server/token.go#L907-L912

AI slop diagnosis below.


Details

Public clients (token_endpoint_auth_method: none + PKCE) are accepted at registration but rejected at /token

Summary

tsidp's Dynamic Client Registration accepts token_endpoint_auth_method: none, echoes it back, and persists it — but the /token authorization-code handler ignores that field and unconditionally requires a client_secret. A spec-compliant public client using PKCE (no secret) can therefore never complete the authorization-code flow.

This blocks any public/PKCE relying party. Concretely it makes tsidp unusable as the IdP for Incus (and other native/CLI apps), because Incus is hardcoded as a public client with PKCE and no secret:

Environment

  • tsidp: ghcr.io/tailscale/tsidp:latest (issue confirmed against main @ 6359a18)
  • Relying party: Incus 6.23 web console

Reproduction

  1. Register a public client via DCR:

    curl -X POST https://idp.example.ts.net/register \
      -H 'Content-Type: application/json' \
      -d '{
            "client_name": "incus",
            "redirect_uris": ["https://host.example.ts.net/oidc/callback"],
            "token_endpoint_auth_method": "none"
          }'

    Response includes "token_endpoint_auth_method": "none", and it is persisted to oidc-funnel-clients.json.

  2. Run authorization-code + PKCE, sending no client_secret at /token (only client_id + code_verifier, per RFC 6749 §4.1.3 / RFC 7636):

    # GET /authorize -> 302 ...?code=...
    curl -G https://idp.example.ts.net/authorize \
      --data-urlencode response_type=code \
      --data-urlencode client_id=$CID \
      --data-urlencode redirect_uri=$REDIRECT \
      --data-urlencode 'scope=openid email profile' \
      --data-urlencode code_challenge=$CHALLENGE \
      --data-urlencode code_challenge_method=S256 \
      --data-urlencode state=xyz
    
    # POST /token (public client: client_id + PKCE verifier, no secret)
    curl -X POST https://idp.example.ts.net/token \
      --data-urlencode grant_type=authorization_code \
      --data-urlencode code=$CODE \
      --data-urlencode redirect_uri=$REDIRECT \
      --data-urlencode code_verifier=$VERIFIER \
      --data-urlencode client_id=$CID

Expected

Per OIDC Core §9, none means "the Client is a Public Client with no Client Secret." With PKCE validating the exchange, /token should issue tokens without a client_secret.

Actual

HTTP 401
{"error":"invalid_client","error_description":"client authentication failed"}

allowRelyingPartyserver/token.go#L902-L924 requires both client_id and client_secret regardless of the client's registered auth method:

FunnelClient.TokenEndpointAuthMethod is stored (server/clients.go#L26) but never consulted here.

Secondary issue: misleading error on retry

handleAuthorizationCodeGrantserver/token.go#L181-L197 deletes the code from s.code before any validation:

So the first attempt fails with the true cause (invalid_client), but the code is already consumed; any retry of the same code returns the misleading invalid_grant - code not found, hiding the underlying problem. (Consuming a single-use code on a failed exchange is also arguably too aggressive.)

Inconsistency

  • DCR accepts and persists token_endpoint_auth_method: none — and even defaults missing values to client_secret_basic rather than rejecting none: server/clients.go#L341-L359.
  • Discovery advertises only ["client_secret_post","client_secret_basic"] (not none): server/oauth-metadata.go#L76.
  • /token then requires a secret.

tsidp simultaneously hands out public-client registrations, advertises that it doesn't support them, and rejects them at token time.

Proposed fix

In allowRelyingParty, honor the registered auth method:

if ar.FunnelRP.TokenEndpointAuthMethod == "none" {
    // Public client: authentication is provided by PKCE (validated below).
    if ar.CodeChallenge == "" {
        return http.StatusUnauthorized, fmt.Errorf("tsidp: public client requires PKCE")
    }
    return http.StatusOK, nil
}
// else: existing confidential-client client_secret check

Advertise it in discovery (server/oauth-metadata.go#L76):

oauthSupportedTokenEndpointAuthMethods = views.SliceOf([]string{"client_secret_post", "client_secret_basic", "none"})

Optionally, only delete(s.code, code) after a successful exchange so failed attempts surface the real error instead of code not found.

References

  • RFC 6749 §2.1 (public clients), §3.2.1 (only credentialed clients authenticate), §4.1.3 (client_id required when not authenticating)
  • RFC 7636 (PKCE) — secures auth-code for secretless public clients
  • OpenID Connect Core 1.0 §9 — token_endpoint_auth_method: none
  • RFC 9700 / OAuth 2.1 — PKCE recommended for all clients

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions