docs(oauth2): document shared PKCE redirect for Login and Link#335
Merged
Conversation
PR #325 refactored OAuth2 Login and Link to share a common redirectToProvider helper. Both endpoints now set identical oauth2_state and oauth2_verifier HttpOnly cookies before redirecting the browser to the provider. Update docs/handler/oauth2.md to: - Add a Routes-section note explaining the shared redirect mechanism and that Callback validates the cookies for both flows - Update the Link response-types row to mention the cookies it sets - Update the Account linking section to reference PKCE alongside the HMAC-signed state, and cross-reference Login's cookie behaviour Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the OAuth2 handler documentation to reflect that both OAuth2Handler.Login and OAuth2Handler.Link use the same PKCE+state cookie redirect mechanism, and that Callback validates the same cookies for either flow.
Changes:
- Documented that both
LoginandLinksetoauth2_stateandoauth2_verifierHttpOnlycookies withSameSite=Laxand a 5-minute TTL before redirecting to the provider. - Updated the response types table to note that
Linksets the same state+PKCE verifier cookies asLogin. - Expanded the account-linking section to explicitly mention PKCE alongside the HMAC-signed state, and that
Callbackvalidates the cookies identically for both flows.
Show a summary per file
| File | Description |
|---|---|
| docs/handler/oauth2.md | Clarifies shared Login/Link redirect behavior (state + PKCE verifier cookies) and aligns the Link documentation with Callback validation behavior. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 0
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Resolves conflict by adopting the more detailed Login flow section from main, which adds a cookie attribute table and a debugging tip callout.
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.
PR #325 refactored
OAuth2Handler.LoginandOAuth2Handler.Linkto share a singleredirectToProviderhelper. Both endpoints now set identicaloauth2_stateandoauth2_verifierHttpOnly cookies (SameSite=Lax, 5-minute TTL) before redirecting the browser to the provider.The existing docs described PKCE protection for
Loginbut were silent onLink. This PR closes that gap.Changes
docs/handler/oauth2.md— Routes section: Added a note explaining thatLoginandLinkshare the same redirect mechanism (state + PKCE verifier cookies) and thatCallbackvalidates both cookies for either flow.Linkrow to mention that it sets state and PKCE verifier cookies (consistent withLogin).Login's cookie behaviour so readers understandCallbackvalidates them identically.Testing
Documentation-only change; no code logic altered. Verify by reading
handler/oauth2.go(Login,Link,redirectToProvider) against the updated doc.Add this agentic workflows to your repo
To install this agentic workflow, run
Greptile Summary
This documentation-only PR updates
docs/handler/oauth2.mdto accurately reflect thatLoginandLinkshare a singleredirectToProviderhelper and therefore both set identicaloauth2_stateandoauth2_verifierHttpOnly cookies before redirecting to the provider.LoginandLinkrows mention that they set state and PKCE verifier cookies, eliminating the prior inconsistency where onlyLinkmentioned them.Callbackvalidates the cookies identically for both flows.Confidence Score: 5/5
Documentation-only change with no code modifications; all updated prose accurately mirrors the implementation in handler/oauth2.go.
Every changed passage was verified against the actual Go source: both Login and Link call the shared redirectToProvider helper that sets oauth2_state and oauth2_verifier cookies, Callback validates them identically before diverging on link vs login logic, and the HMAC-signed state distinction is already captured in the pre-existing cookie table. No logic, configuration, or behaviour was altered.
No files require special attention.
Important Files Changed
Sequence Diagram
sequenceDiagram participant Browser participant Handler as OAuth2Handler participant Provider as OAuth2 Provider Note over Browser,Provider: Login flow Browser->>Handler: GET /auth/github/login Handler-->>Browser: 302 + Set-Cookie: oauth2_state, oauth2_verifier Browser->>Provider: Redirect to provider AuthCodeURL Provider-->>Browser: "302 back to /auth/github/callback?code=&state=" Browser->>Handler: "GET /callback (with state & verifier cookies)" Handler->>Handler: "validate state cookie == query param state" Handler->>Handler: PKCE exchange with verifier cookie Handler->>Handler: clear oauth2_state and oauth2_verifier cookies Handler-->>Browser: "302 /?oauth2_login=1 + JWT cookie" Note over Browser,Provider: Link flow (same redirect mechanism) Browser->>Handler: "GET /auth/github/link?nonce=<nonce>" Handler->>Handler: consume nonce, look up user, sign state (HMAC) Handler-->>Browser: 302 + Set-Cookie: oauth2_state (HMAC-signed), oauth2_verifier Browser->>Provider: Redirect to provider AuthCodeURL Provider-->>Browser: "302 back to /auth/github/callback?code=&state=" Browser->>Handler: GET /callback (with cookies) Handler->>Handler: validate state cookie, PKCE exchange (identical to Login) Handler->>Handler: "parseLinkState -> extract userID from HMAC state" Handler->>Handler: "handleLinkCallback -> link subject" Handler-->>Browser: "302 /?oauth2_linked=true"Reviews (3): Last reviewed commit: "Merge origin/main: expand PKCE/CSRF cook..." | Re-trigger Greptile