fix: TOTPHandler.Generate returns 404 for deleted users instead of 500

[Copilot]

TOTPHandler.Generate was the only authenticated handler calling FindByID without checking auth.ErrNotFound, returning HTTP 500 when an account was deleted mid-session. Every other FindByID call in an authenticated context returns 404 in this case.

Changes

• handler/totp.go: Add errors.Is(err, auth.ErrNotFound) → 404 branch and slog.ErrorContext log for unexpected store errors before the 500 fallback:

user, 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")
        return
    }
    slog.ErrorContext(r.Context(), "failed to fetch user", slog.Any("error", err))
    writeError(r.Context(), w, http.StatusInternalServerError, "failed to fetch user")
    return
}

• handler/totp_test.go: Rename existing TestTOTP_generate_userNotFound → TestTOTP_generate_userStoreError (generic DB error → 500); add TestTOTP_generate_userNotFound covering the auth.ErrNotFound → 404 path.

Greptile Summary

This PR fixes TOTPHandler.Generate to return HTTP 404 instead of 500 when a user cannot be found (deleted mid-session), aligning it with every other authenticated FindByID call in the codebase. The implementation correctly uses errors.Is(err, auth.ErrNotFound), includes slog.ErrorContext with context for unexpected store errors, and adds both the missing 404 test and renames the existing 500 test for clarity.

Confidence Score: 5/5

This PR is safe to merge — it is a minimal, well-tested bug fix with no regressions.

The change is small and targeted: one new error branch, one slog call with context, and two well-scoped tests. All imports were already present, the slog call correctly passes r.Context() per the project rule, and the fix aligns with the established pattern in the rest of the codebase. No P0 or P1 issues found.

No files require special attention.

Important Files Changed

Filename	Overview
handler/totp.go	Adds ErrNotFound → 404 branch and slog.ErrorContext for unexpected store errors before the existing 500 fallback; imports are already in place.
handler/totp_test.go	Adds TestTOTP_generate_userNotFound covering the new 404 path and renames the old test to TestTOTP_generate_userStoreError for the 500 path; both cases are exercised correctly.

Sequence Diagram

sequenceDiagram
    participant Client
    participant TOTPHandler
    participant UserStore

    Client->>TOTPHandler: POST /totp/generate
    TOTPHandler->>TOTPHandler: auth.GenerateTOTPSecret()
    TOTPHandler->>UserStore: FindByID(ctx, userID)

    alt user not found (ErrNotFound)
        UserStore-->>TOTPHandler: nil, auth.ErrNotFound
        TOTPHandler-->>Client: 404 user not found
    else store error
        UserStore-->>TOTPHandler: nil, err
        TOTPHandler->>TOTPHandler: slog.ErrorContext(ctx, "failed to fetch user")
        TOTPHandler-->>Client: 500 failed to fetch user
    else success
        UserStore-->>TOTPHandler: *auth.User, nil
        TOTPHandler->>TOTPHandler: TOTPProvisioningURI(secret, email, issuer)
        TOTPHandler-->>Client: 200 {secret, provisioningURI}
    end

Loading

_{Reviews (1): Last reviewed commit: "fix: TOTPHandler.Generate checks ErrNotF..." | Re-trigger Greptile}

[Copilot]

Pull request overview

This PR aligns TOTPHandler.Generate with the rest of the authenticated handlers by treating a deleted user record (auth.ErrNotFound) as a client-visible 404 instead of an internal 500, improving correctness when accounts are removed mid-session.

Changes:

• Update TOTPHandler.Generate to return HTTP 404 on errors.Is(err, auth.ErrNotFound) and log unexpected user-store errors before returning 500.
• Extend/adjust tests to cover both the “user not found” (404) path and the generic user store failure (500) path.

Show a summary per file

File	Description
handler/totp.go	Adds an ErrNotFound → 404 branch and logs unexpected FindByID failures before returning 500.
handler/totp_test.go	Adds a dedicated 404 test case and renames the prior test to cover generic store errors (500).

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