OAuth2 Link: return 404 for missing user; keep 409 for already-linked accounts

[Copilot]

Bug Fix

What was the bug?

OAuth2Handler.Link() treated Users.FindByID(...)=ErrNotFound as 409 cannot link account, mirroring OIDC behavior. This conflated a missing-user condition with the distinct conflict case (u.OIDCSubject != nil).

How did you fix it?

• Handler status-code semantics
  • Changed /handler/oauth2.go link flow so ErrNotFound from FindByID returns 404 user not found.
  • Kept 409 cannot link account only for the explicit already-linked branch (u.OIDCSubject != nil).
• Behavioral contract in tests
  • Updated TestOAuth2Link_userNotFound in /handler/oauth2_test.go to assert 404 instead of 409.

Testing

• Targeted expectation update
  • Adjusted the link-path unit test to lock in the new missing-user behavior.

u, err := h.Users.FindByID(r.Context(), userID)
if err != nil {
    if errors.Is(err, auth.ErrNotFound) {
        writeError(r.Context(), w, http.StatusNotFound, "user not found")
    }
    // ...
}
if u.OIDCSubject != nil {
    writeError(r.Context(), w, http.StatusConflict, "cannot link account")
}

Greptile Summary

This PR fixes a status-code conflation bug in both the OAuth2 and OIDC Link handlers, where a missing user (ErrNotFound from FindByID) was incorrectly reported as 409 Conflict instead of 404 Not Found. The 409 path is now reserved exclusively for the already-linked case (u.OIDCSubject != nil).

• handler/oauth2.go / handler/oidc.go: ErrNotFound now writes 404 user not found; 409 cannot link account is only emitted when the user record has an existing OIDC subject.
• handler/oauth2_test.go / handler/oidc_test.go: userNotFound test cases updated to assert http.StatusNotFound, locking in the corrected semantics for both handlers.

Confidence Score: 5/5

Safe to merge — both handlers now return the correct status codes, return paths are in place, and tests cover the corrected behavior.

The change is a minimal two-line fix applied symmetrically to both Link handlers. The logic is straightforward: a previously-incorrect status code for a missing user is now 404, and the 409 path remains correctly guarded by the explicit already-linked check. Both fixes are covered by updated unit tests. No other code paths are affected.

No files require special attention.

Important Files Changed

Filename	Overview
handler/oauth2.go	Corrected ErrNotFound branch to return 404 instead of 409; 409 is now reserved exclusively for the already-linked case. Return flow and error logging are correct.
handler/oauth2_test.go	TestOAuth2Link_userNotFound updated to assert 404, matching the corrected handler behavior.
handler/oidc.go	Applies the same missing-user fix as oauth2.go: ErrNotFound now returns 404, and 409 is kept only for the OIDCSubject-already-set branch. Previously flagged concern is now resolved.
handler/oidc_test.go	TestOIDCLink_userNotFound updated to assert 404, mirroring the oauth2 test change.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Link request received] --> B[Consume link nonce]
    B -->|nonce invalid/expired| C[401 Unauthorized]
    B -->|nonce error| D[500 Internal Server Error]
    B -->|ok| E[FindByID userID]
    E -->|ErrNotFound| F[404 user not found ✅ FIXED]
    E -->|other error| G[500 Internal Server Error]
    E -->|user found| H{u.OIDCSubject != nil?}
    H -->|yes| I[409 cannot link account]
    H -->|no| J[Generate state & verifier]
    J --> K[Redirect to OAuth2/OIDC provider]

Loading

Comments Outside Diff (1)

1. handler/oidc.go, line 258-259 (link)

   Same missing-user / conflict conflation in the OIDC Link handler

   handler/oidc.go has exactly the same bug this PR fixes in oauth2.go: when FindByID returns ErrNotFound, line 259 responds with 409 Conflict ("cannot link account") instead of 404 Not Found. Callers of the OIDC link endpoint will still receive a misleading conflict error for a user that simply doesn't exist, while the OAuth2 path now returns the correct 404. The fix and the corresponding test update in oidc_test.go (parallel to TestOAuth2Link_userNotFound) appear to have been left out of this PR.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: handler/oidc.go
   Line: 258-259
   
   Comment:
   **Same missing-user / conflict conflation in the OIDC Link handler**
   
   `handler/oidc.go` has exactly the same bug this PR fixes in `oauth2.go`: when `FindByID` returns `ErrNotFound`, line 259 responds with `409 Conflict` ("cannot link account") instead of `404 Not Found`. Callers of the OIDC link endpoint will still receive a misleading conflict error for a user that simply doesn't exist, while the OAuth2 path now returns the correct 404. The fix and the corresponding test update in `oidc_test.go` (parallel to `TestOAuth2Link_userNotFound`) appear to have been left out of this PR.
   
   How can I resolve this? If you propose a fix, please make it concise.

   

_{Reviews (2): Last reviewed commit: "Fix OIDC link handler returning 409 for ..." | Re-trigger Greptile}

[Copilot]

Pull request overview

Adjusts OAuth2 account-linking error semantics so a link nonce that resolves to a missing/deleted user yields a 404 user not found, while preserving 409 cannot link account strictly for the already-linked condition. This makes the OAuth2 link flow’s status codes more precise for clients.

Changes:

• Updated OAuth2Handler.Link to return 404 on Users.FindByID(...)=auth.ErrNotFound.
• Updated TestOAuth2Link_userNotFound to assert 404 instead of 409.

Show a summary per file

File	Description
handler/oauth2.go	Returns 404 user not found when the nonce maps to a missing user during OAuth2 linking.
handler/oauth2_test.go	Updates the link-path unit test to lock in the new 404 behavior.

Copilot's findings

Tip

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

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

[veverkap]

Fixed in f596031. The OIDC link handler (handler/oidc.go:259) now returns 404 with "user not found" when FindByID returns ErrNotFound, matching the OAuth2 handler behavior. TestOIDCLink_userNotFound updated to assert StatusNotFound instead of StatusConflict.