Skip to content

Refactor shared account-link nonce creation for OIDC and OAuth2#324

Merged
veverkap merged 2 commits into
mainfrom
copilot/fix-duplicate-code-create-link-nonce
May 23, 2026
Merged

Refactor shared account-link nonce creation for OIDC and OAuth2#324
veverkap merged 2 commits into
mainfrom
copilot/fix-duplicate-code-create-link-nonce

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 23, 2026

OIDC and OAuth2 account-link flows still shared nearly identical CreateLinkNonce logic, with only a superficial difference in how the nonce string was generated. This change finishes the deduplication by moving nonce generation into the shared helper so both handlers follow the same issuance path.

  • What changed

    • Simplified createLinkNonce in handler/oauth2_common.go so it owns nonce generation, hashing, storage, and response writing.
    • Removed the generator callback parameter from the helper.
    • Updated both OIDCHandler.CreateLinkNonce and OAuth2Handler.CreateLinkNonce to delegate using only the nonce store and TTL.
  • Result

    • OIDC and OAuth2 link-nonce issuance now use one implementation end-to-end.
    • Eliminates the remaining drift risk between generateOIDCState() and auth.GenerateRandomBase64(32) for account-link nonces.
    • Keeps TTL-specific behavior at the call sites while centralizing the shared flow.
  • Representative change

    func createLinkNonce(w http.ResponseWriter, r *http.Request, store auth.OIDCLinkNonceStore, ttl time.Duration) {
    	userID := auth.UserIDFromContext(r.Context())
    	nonce, err := auth.GenerateRandomBase64(32)
    	// hash, persist, and return nonce
    }
    
    func (h *OIDCHandler) CreateLinkNonce(w http.ResponseWriter, r *http.Request) {
    	createLinkNonce(w, r, h.LinkNonces, oidcStateCookieTTL)
    }
    
    func (h *OAuth2Handler) CreateLinkNonce(w http.ResponseWriter, r *http.Request) {
    	createLinkNonce(w, r, h.LinkNonces, oauth2StateCookieTTL)
    }

Greptile Summary

This PR removes the generateNonce callback parameter from the shared createLinkNonce helper and inlines auth.GenerateRandomBase64(32) directly, consolidating nonce issuance for both OIDC and OAuth2 account-linking flows into one implementation. Both handlers now call createLinkNonce with only a store and TTL.

  • handler/oauth2_common.go: Drops the generateNonce func() (string, error) parameter and calls auth.GenerateRandomBase64(32) directly; all downstream logic (hash, persist, respond) is unchanged.
  • handler/oidc.go / handler/oauth2.go: Call sites updated to omit the now-removed callback — since generateOIDCState was itself just a thin wrapper around auth.GenerateRandomBase64(32), the behavioral difference is zero.

Confidence Score: 5/5

Safe to merge — the refactoring removes indirection without changing any observable behavior.

Both the old OIDC callback (generateOIDCState) and the old OAuth2 callback were thin wrappers around the same auth.GenerateRandomBase64(32) call, so the unified implementation is byte-for-byte equivalent at runtime. Hash, storage, and response logic are untouched. The slog calls in the modified function already pass r.Context() via ErrorContext, satisfying the team's context-logging requirement.

No files require special attention.

Important Files Changed

Filename Overview
handler/oauth2_common.go Callback parameter removed; nonce generation inlined as auth.GenerateRandomBase64(32). Hash, store, and response logic unchanged. Both slog calls already use ErrorContext with r.Context(), satisfying the context logging rule.
handler/oauth2.go Call site updated to remove the anonymous generateNonce wrapper; no other logic changed.
handler/oidc.go Call site updated to remove generateOIDCState callback; generateOIDCState was itself just auth.GenerateRandomBase64(32), so behavior is identical.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Handler as OIDC/OAuth2 Handler
    participant Helper as createLinkNonce
    participant Store as OIDCLinkNonceStore
    participant AuthPkg as auth package

    Client->>Handler: POST /link-nonce
    Handler->>Helper: store, ttl
    Helper->>AuthPkg: GenerateRandomBase64(32)
    AuthPkg-->>Helper: nonce string
    Helper->>AuthPkg: HashHighEntropyToken(nonce)
    AuthPkg-->>Helper: nonceHash
    Helper->>Store: CreateLinkNonce(ctx, userID, nonceHash, expiry)
    Store-->>Helper: ok
    Helper-->>Client: "200 JSON {nonce}"
Loading

Reviews (1): Last reviewed commit: "refactor: unify link nonce generation" | Re-trigger Greptile

Copilot AI changed the title [WIP] Refactor duplicated CreateLinkNonce logic in OIDCHandler and OAuth2Handler Refactor shared account-link nonce creation for OIDC and OAuth2 May 23, 2026
Copilot AI requested a review from veverkap May 23, 2026 15:39
@veverkap veverkap marked this pull request as ready for review May 23, 2026 16:15
@veverkap veverkap requested review from a team and Copilot May 23, 2026 16:15
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR completes deduplication of the OIDC/OAuth2 account-link nonce issuance path by centralizing nonce generation inside the shared createLinkNonce helper, so both handlers follow the same end-to-end implementation.

Changes:

  • Refactored createLinkNonce (shared helper) to generate the nonce internally and removed the generator callback parameter.
  • Updated OIDCHandler.CreateLinkNonce to call the new helper signature.
  • Updated OAuth2Handler.CreateLinkNonce to call the new helper signature.
Show a summary per file
File Description
handler/oidc.go Updates OIDC link-nonce issuance to use the shared helper without a custom generator callback.
handler/oauth2.go Updates OAuth2 link-nonce issuance to use the shared helper without an inline nonce generator.
handler/oauth2_common.go Centralizes nonce generation within createLinkNonce, simplifying the shared issuance flow.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 3/3 changed files
  • Comments generated: 0

@veverkap veverkap merged commit d07d68b into main May 23, 2026
33 checks passed
@veverkap veverkap deleted the copilot/fix-duplicate-code-create-link-nonce branch May 23, 2026 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🔍 Duplicate Code Detected: CreateLinkNonce Logic in OIDCHandler and OAuth2Handler

3 participants