PKCE support#3492
Open
camohob wants to merge 2 commits into
Open
Conversation
|
Someone is attempting to deploy a commit to the Temporal Team on Vercel. A member of the Team first needs to authorize it. |
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.
Description & motivation 💭
Add PKCE (Proof Key for Code Exchange, RFC 7636) support to the OAuth2 authorization code flow, gated behind
TEMPORAL_AUTH_PKCE_ENABLEDenvironment variable (defaultfalse).Closes #2519.
PKCE provides protection against authorization code interception attacks. When enabled:
code_verifieris generated and stored in a secure HttpOnly cookiecode_challenge_method=S256andcode_challengeare included in the auth redirect URLcode_verifieris sent to the token endpoint during code exchangeDisabled by default to maintain backward compatibility — existing deployments are unaffected until they opt in.
Screenshots (if applicable) 📸
N/A
Design Considerations 🎨
code_verifiercookie cleanup is unconditional even when PKCE is disabled — settingMaxAge=-1for a non-existent cookie is harmlessTesting 🧪
How was this tested 👻
All existing tests pass with
pkceEnabledparameter added to function signaturesNew tests added for PKCE-disabled path in both
authandroutepackagesgo build ./...andgo vet ./...pass (excluding pre-existingui/embed.gofailure without pre-built UI assets)Manual testing
E2E tests added
Unit tests added
Steps for others to test: 🚶🏽♂️🚶🏽♀️
cd server && go test ./auth/... ./route/... -vcode_challenge_method,code_challenge) must be absent from auth URL, andcode_verifiercookie must not be setcode_challenge_method=S256andcode_challenge,code_verifiercookie is set HttpOnly, token exchange sendscode_verifierChecklists
Draft Checklist
go vetcleanAUTHENTICATION.mdupdated with PKCE documentationMerge Checklist
Issue(s) closed
Closes #2519
Docs
Any docs updates needed?
Yes —
AUTHENTICATION.mdhas been updated with:pkceEnabledfield in the Auth Settings table