Plan: Harden OAuth2 Provider — Security Fixes + OAuth 2.1 Compliance

Context & Why

Security issue coder/security#121 reports a critical session takeover via CSRF on the OAuth2 provider. This plan covers all remaining security fixes from that issue plus convergence on OAuth 2.1 requirements. The goal is a single PR that closes all actionable gaps.

Current State (already committed on branch csrf-sjx1)

Remaining Work

Fix 2 — Require state parameter (DROPPED)

Decision: Do not enforce state as required. The state parameter is still hashed and stored when provided (via hashOAuth2State / state_hash column from prior commits), but clients are not forced to supply it. This avoids breaking existing integrations that omit state.

Rollback: Remove "state" from the RequiredNotEmpty call in coderd/oauth2provider/authorize.go:42:

// BEFORE (current on branch)
p.RequiredNotEmpty("response_type", "client_id", "state", "code_challenge")

// AFTER
p.RequiredNotEmpty("response_type", "client_id", "code_challenge")

No test changes needed — tests already pass state voluntarily.

Fix 4 — Exact redirect URI matching

Currently coderd/httpapi/queryparams.go:233 uses prefix matching:

// CURRENT — prefix match
if v.Host != base.Host || !strings.HasPrefix(v.Path, base.Path) {

OAuth 2.1 requires exact string matching. Change to:

// AFTER — exact match (OAuth 2.1 §4.1.2.1)
if v.Host != base.Host || v.Path != base.Path {

File: coderd/httpapi/queryparams.go — RedirectURL method

Also update the error message from "must be a subset of" to "must exactly match".

Additionally, store redirect_uri with the auth code and verify at the token endpoint (RFC 6749 §4.1.3):

1. New migration (same migration file or a new 000421): Add redirect_uri text column to oauth2_provider_app_codes
2. Update INSERT query in coderd/database/queries/oauth2.sql to include redirect_uri
3. coderd/oauth2provider/authorize.go: Store params.redirectURL.String() when inserting the code
4. coderd/oauth2provider/tokens.go: After retrieving the code from DB, verify that redirect_uri from the token request matches the stored value exactly. Currently tokens.go:103 calls p.RedirectURL(vals, callbackURL, "redirect_uri") for prefix validation only — it must compare against the stored redirect_uri from the code, not just the app's callback URL.

Fix 7 — frame-ancestors CSP on consent page

The consent page can be iframed by a workspace app (same-site), which is the attack vector. Add a Content-Security-Policy header to prevent framing.

File: site/site.go — RenderOAuthAllowPage function (~line 731)

Before writing the response, add:

func RenderOAuthAllowPage(rw http.ResponseWriter, r *http.Request, data RenderOAuthAllowData) {
    rw.Header().Set("Content-Type", "text/html; charset=utf-8")
    // Prevent the consent page from being framed to mitigate
    // clickjacking attacks (coder/security#121).
    rw.Header().Set("Content-Security-Policy", "frame-ancestors 'none'")
    rw.Header().Set("X-Frame-Options", "DENY")
    ...

Both headers for defense-in-depth (CSP for modern browsers, X-Frame-Options for legacy).

OAuth 2.1 — Mandatory PKCE

Currently PKCE is checked only when code_challenge was provided during authorization (tokens.go:258):

// CURRENT — conditional check
if dbCode.CodeChallenge.Valid && dbCode.CodeChallenge.String != "" {
    // verify PKCE
}

OAuth 2.1 requires PKCE for ALL authorization code flows. Change to:

File: coderd/oauth2provider/authorize.go — Add "code_challenge" to required params:

p.RequiredNotEmpty("response_type", "client_id", "code_challenge")

File: coderd/oauth2provider/tokens.go:257-265 — Make PKCE verification unconditional:

// AFTER — PKCE always required (OAuth 2.1)
if req.CodeVerifier == "" {
    return codersdk.OAuth2TokenResponse{}, errInvalidPKCE
}
if !dbCode.CodeChallenge.Valid || dbCode.CodeChallenge.String == "" {
    // Code was issued without a challenge — should not happen
    // with the authorize endpoint enforcement, but defend in
    // depth.
    return codersdk.OAuth2TokenResponse{}, errInvalidPKCE
}
if !VerifyPKCE(dbCode.CodeChallenge.String, req.CodeVerifier) {
    return codersdk.OAuth2TokenResponse{}, errInvalidPKCE
}

File: codersdk/oauth2.go — Remove OAuth2ProviderResponseTypeToken from the enum or reject it explicitly in the authorize handler. Currently it's defined at line 216 but the handler ignores response_type and always issues a code. We should either:

• (a) Remove the "token" variant from the enum and reject it with unsupported_response_type, OR
• (b) Add an explicit check in ProcessAuthorize that rejects response_type=token

Option (b) is simpler and more backwards-compatible:

// In ProcessAuthorize, after extracting params:
if params.responseType != codersdk.OAuth2ProviderResponseTypeCode {
    httpapi.WriteOAuth2Error(ctx, rw, http.StatusBadRequest,
        codersdk.OAuth2ErrorCodeUnsupportedResponseType,
        "Only response_type=code is supported")
    return
}

OAuth 2.1 — Bearer tokens in query strings

coderd/httpmw/apikey.go:743 accepts access_token from URL query parameters. OAuth 2.1 prohibits this. However, this may be used internally (e.g., workspace apps, DERP). Need to audit callers before removing.

Approach: This is a larger change with potential breakage. Mark as a separate follow-up issue rather than including in this PR. Document the finding.

OAuth 2.1 — Removed flows

✅ Already compliant. tokens.go only supports authorization_code and refresh_token grant types. The implicit grant (response_type=token) will be explicitly rejected per the PKCE section above.

OAuth 2.1 — Refresh token rotation

✅ Already compliant. tokens.go:442 deletes the old API key when a refresh token is used.

Migration Plan

All DB changes can go in a single new migration (or extend 000420 if the branch is rebased before merge). Columns to add:

• redirect_uri text on oauth2_provider_app_codes

The state_hash column is already added by migration 000420.

Implementation Order

1. Fix 7 — CSP headers on consent page (isolated, no deps)
2. Fix 2 — Require state parameter (DROPPED — state stays optional)
3. Fix 4 — Exact redirect URI matching + store/verify redirect_uri
4. PKCE mandatory — Require code_challenge + reject response_type=token
5. Rollback — Remove "state" from RequiredNotEmpty in authorize.go
6. Tests — Update/add tests for all changes
7. make gen after DB changes

Out of Scope (separate PRs)

• Bearer tokens in query strings (needs internal caller audit)
• Scope enforcement on OAuth2 tokens
• Rate limiting / quota on dynamic client registration