feat(oauth2): send PKCE code challenge for OIDC login sources#38202
Open
yisding wants to merge 5 commits into
Open
feat(oauth2): send PKCE code challenge for OIDC login sources#38202yisding wants to merge 5 commits into
yisding wants to merge 5 commits into
Conversation
When Gitea acts as an OIDC client (login source), it never sent a PKCE code_challenge on the authorization request, so identity providers that require PKCE rejected the sign-in. Generate a per-request S256 verifier in Callout, stash it in the user session, and append code_challenge to the authorization redirect. On the callback, inject the stored code_verifier into the request so goth forwards it to the token exchange. Gated to the openidConnect provider; no settings, schema, or dependency changes (golang.org/x/oauth2 already ships the helpers). Assisted-by: Claude Code:claude-opus-4-8[1m]
GiteaBot
added
the
lgtm/need 2
… cleanup Replace the hardcoded `source.Provider == "openidConnect"` check with a SupportsPKCE() method on the Provider interface, mirroring the existing SupportSSHPublicKey() capability. This drops a duplicated provider-name literal and a string special-case on the shared Callout/Callback path. Because AwsCognitoProvider embeds OpenIDProvider, Cognito login sources now also send PKCE -- correct, since Cognito is OIDC-based and runs the token exchange through the same goth openidConnect Session that reads code_verifier. Also log (instead of silently dropping) the Delete/Release errors during the single-use verifier cleanup in injectPKCEVerifier; the cleanup stays best-effort so a failure never breaks the login. Assisted-by: Claude Code:claude-opus-4-8[1m]
github-actions
Bot
added
the
type/feature
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds PKCE (S256) support to Gitea’s OAuth2/OpenID Connect login source flow (Gitea as the OAuth client), ensuring compatibility with OIDC identity providers that require PKCE during the authorization code flow.
Changes:
- Append
code_challenge+code_challenge_method=S256to the OIDC authorization redirect URL and store the per-request verifier in the Gitea session. - Inject the stored
code_verifierinto the OIDC callback request so goth forwards it to the token exchange. - Extend the OAuth2 provider abstraction with
SupportsPKCE()and add unit tests for PKCE URL mutation and verifier injection behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| services/auth/source/oauth2/source_callout.go | Hooks PKCE into the Callout (challenge) and Callback (verifier) flow. |
| services/auth/source/oauth2/providers.go | Extends the Provider interface with a SupportsPKCE() capability flag. |
| services/auth/source/oauth2/providers_openid.go | Marks the OIDC provider as PKCE-capable. |
| services/auth/source/oauth2/providers_base.go | Provides a default SupportsPKCE() == false implementation for providers embedding BaseProvider. |
| services/auth/source/oauth2/pkce.go | Implements session-backed PKCE verifier storage, URL challenge appending, and callback query injection. |
| services/auth/source/oauth2/pkce_test.go | Adds unit tests for PKCE challenge construction and verifier injection / single-use cleanup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+30
to
+32
| // Append a PKCE code_challenge for OIDC login sources. Remove if upstream resolves gitea#21376. | ||
| if url, err = source.beginPKCE(request, url); err != nil { | ||
| return err |
Comment on lines
+48
to
+49
| // Inject the stashed PKCE code_verifier for OIDC login sources. Remove if upstream resolves gitea#21376. | ||
| source.injectPKCEVerifier(request) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
When Gitea is configured as an OAuth2/OIDC login source — i.e. Gitea acts as the client, logging users in via an external identity provider — it never sent a PKCE
code_challengeon the authorization request. Identity providers that require PKCE (RFC 7636) therefore reject the sign-in.This implements PKCE (S256) for the OpenID Connect login flow:
Callout, after goth builds the authorization URL, generate a per-request verifier withoauth2.GenerateVerifier(), stash it in the user session, and appendcode_challenge+code_challenge_method=S256to the redirect URL.Callback, read the stashed verifier and injectcode_verifierinto the callback request so goth'sopenidConnectprovider forwards it to the token exchange.Notes:
openidConnectprovider, and purely additive: spec-compliant IdPs that don't require PKCE ignorecode_challenge.golang.org/x/oauth2already providesGenerateVerifier/S256ChallengeFromVerifier.authCodeOptionsare shared/static and cannot carry a per-request verifier, which is why the challenge is appended to the redirect URL and the verifier is kept in Gitea's own session rather than routed through goth.Usage
No configuration required. Existing and new OpenID Connect authentication sources automatically send a PKCE
code_challenge, so IdPs that require PKCE now work. Sources whose IdP does not require PKCE are unaffected. (No UI changes.)Testing
Automated (
services/auth/source/oauth2/pkce_test.go):TestPKCEChallenge— the S256 challenge equalsbase64url(sha256(verifier)),code_challenge_method=S256, and existing authorization params (state,client_id,scope,redirect_uri) are preserved.TestInjectPKCEVerifier— pins the seam with goth: the stored verifier lands inrequest.URL.Query()(exactly wheregothic.CompleteUserAuthreads it), callbackcode/stateare preserved, the verifier is single-use, and non-OIDC providers are untouched.make fmt,golangci-lint run,go vet, and the package test suite all pass.Manual verification against a PKCE-requiring IdP (the previously failing "code_challenge is missing" flow) is recommended before marking this ready for review.
Fixes #34747.
Resolves #21376.
AI assistance disclosure
This PR was prepared with AI assistance (Claude Code, Opus 4.8). I have reviewed the change, run the linters and tests, and take responsibility for its correctness and intent.