Skip to content

Deduplicate OAuth2/OIDC link nonce creation#287

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

Deduplicate OAuth2/OIDC link nonce creation#287
veverkap merged 4 commits into
mainfrom
copilot/fix-duplicate-code-create-link-nonce

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 20, 2026

CreateLinkNonce had two near-identical implementations in OAuth2Handler and OIDCHandler, which created avoidable maintenance risk around nonce generation, hashing, persistence, and response handling. This change consolidates that flow into one shared path while preserving each handler’s existing nonce-generation entry point and TTL.

  • Shared link-nonce flow

    • Added createLinkNonce in handler/oauth2_common.go for the common account-linking nonce lifecycle:
      • store availability guard
      • authenticated user lookup from request context
      • nonce generation
      • high-entropy token hashing
      • nonce persistence with expiration
      • JSON response payload
  • Handler delegation

    • Replaced duplicated CreateLinkNonce bodies in:
      • OAuth2Handler
      • OIDCHandler
    • Both handlers now delegate to the shared helper.
  • Behavior preserved

    • The helper accepts a nonce generator, so each handler keeps its original generation path:
      • OAuth2 continues to use auth.GenerateRandomBase64(32)
      • OIDC continues to use generateOIDCState
    • Existing handler-specific TTLs remain unchanged.
func (h *OIDCHandler) CreateLinkNonce(w http.ResponseWriter, r *http.Request) {
	createLinkNonce(w, r, h.LinkNonces, oidcStateCookieTTL, generateOIDCState)
}

func (h *OAuth2Handler) CreateLinkNonce(w http.ResponseWriter, r *http.Request) {
	createLinkNonce(w, r, h.LinkNonces, oauth2StateCookieTTL, func() (string, error) {
		return auth.GenerateRandomBase64(32)
	})
}

Greptile Summary

This PR eliminates duplicated CreateLinkNonce implementations in OAuth2Handler and OIDCHandler by extracting a shared createLinkNonce helper in oauth2_common.go. Both handlers delegate to it while retaining their original nonce generators and TTLs.

  • handler/oauth2_common.go: New createLinkNonce function handles the full lifecycle — store nil-guard, user-ID extraction, nonce generation via injected func, high-entropy hashing, persistence, and JSON response.
  • handler/oauth2.go / handler/oidc.go: Each CreateLinkNonce method is replaced with a single delegation call, with no change to observable behavior.

Confidence Score: 5/5

Safe to merge — the refactor is a straightforward delegation with no behavioral changes to nonce generation, hashing, persistence, or response handling.

Both handlers preserve their original nonce generators and TTLs; the shared helper replicates the exact same control flow that existed in both originals. No logic has been altered.

No files require special attention.

Important Files Changed

Filename Overview
handler/oauth2_common.go New shared createLinkNonce helper correctly consolidates the store-guard, user-ID extraction, nonce generation, hashing, persistence, and JSON response; minor inconsistency in structured log attributes.
handler/oauth2.go Delegates CreateLinkNonce to the shared helper with the original auth.GenerateRandomBase64(32) generator and oauth2StateCookieTTL; behavior preserved.
handler/oidc.go Delegates CreateLinkNonce to the shared helper with generateOIDCState and oidcStateCookieTTL; behavior preserved.

Sequence Diagram

sequenceDiagram
    participant Client
    participant OAuth2Handler/OIDCHandler
    participant createLinkNonce
    participant Store as auth.OIDCLinkNonceStore

    Client->>OAuth2Handler/OIDCHandler: POST /link-nonce
    OAuth2Handler/OIDCHandler->>createLinkNonce: delegate(w, r, store, ttl, generateNonce)
    createLinkNonce->>createLinkNonce: store nil-guard
    createLinkNonce->>createLinkNonce: auth.UserIDFromContext(r.Context())
    createLinkNonce->>createLinkNonce: generateNonce()
    createLinkNonce->>createLinkNonce: auth.HashHighEntropyToken(nonce)
    createLinkNonce->>Store: CreateLinkNonce(ctx, userID, hash, expiry)
    Store-->>createLinkNonce: ok / err
    createLinkNonce-->>Client: "200 {nonce} or error"
Loading

Fix All in Claude Code

Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
handler/oauth2_common.go:85
The `user_id` structured attribute is included in the generate-nonce error log (line 78) but is dropped from the store-nonce error log, even though `userID` is already in scope. Without it, a store failure in the persistence layer loses the user identity that would be needed for incident triage.

```suggestion
		slog.ErrorContext(r.Context(), "failed to store link nonce", slog.Any("error", err), slog.String("user_id", userID))
```

Reviews (2): Last reviewed commit: "Update handler/oauth2_common.go" | Re-trigger Greptile

Copilot AI changed the title [WIP] Refactor CreateLinkNonce method to eliminate duplication Deduplicate OAuth2/OIDC link nonce creation May 20, 2026
Copilot AI requested a review from veverkap May 20, 2026 13:48
@veverkap veverkap marked this pull request as ready for review May 20, 2026 14:48
@veverkap veverkap requested review from a team and Copilot May 20, 2026 14:48
Comment thread handler/oauth2_common.go
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 reduces maintenance risk in the account-linking flow by deduplicating the OAuth2 and OIDC implementations of link-nonce creation into a shared helper while keeping each handler’s existing nonce generation approach and TTL.

Changes:

  • Added a shared createLinkNonce helper in handler/oauth2_common.go to centralize nonce generation, hashing, persistence, and JSON response writing.
  • Updated OIDCHandler.CreateLinkNonce and OAuth2Handler.CreateLinkNonce to delegate to the shared helper.
Show a summary per file
File Description
handler/oidc.go Replaces the inline OIDC link-nonce creation logic with a call to the shared helper.
handler/oauth2.go Replaces the inline OAuth2 link-nonce creation logic with a call to the shared helper, preserving its nonce generation method.
handler/oauth2_common.go Introduces the shared createLinkNonce helper that encapsulates the link-nonce lifecycle.

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: 1

Comment thread handler/oauth2_common.go Outdated
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@veverkap veverkap merged commit 616d2eb into main May 21, 2026
14 checks passed
@veverkap veverkap deleted the copilot/fix-duplicate-code-create-link-nonce branch May 21, 2026 01:18
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 Method Duplicated Across OAuth2Handler and OIDCHandler

3 participants