Skip to content

Add error logging for OAuth2 code exchange failures in callback#293

Merged
veverkap merged 1 commit into
mainfrom
copilot/fix-oauth2-http-401-error
May 23, 2026
Merged

Add error logging for OAuth2 code exchange failures in callback#293
veverkap merged 1 commit into
mainfrom
copilot/fix-oauth2-http-401-error

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 20, 2026

OAuth2 callback currently returns 401 failed to exchange code when OAuthConfig.Exchange() fails, but does not emit structured error logs. This hides provider/network outage signals and makes operational diagnosis difficult.

  • Problem

    • handler/oauth2.go callback path swallowed Exchange() failures behind a client-facing 401 without server-side error context.
  • Change

    • Added slog.ErrorContext immediately before writeError(...) in the OAuthConfig.Exchange() error branch.
    • Preserved existing response contract (401 + "failed to exchange code"), adding observability only.
  • Code snippet

    token, err := h.OAuthConfig.Exchange(r.Context(), code, oauth2.VerifierOption(verifierCookie.Value))
    if err != nil {
        slog.ErrorContext(r.Context(), "OAuth2 code exchange failed", slog.Any("error", err))
        writeError(r.Context(), w, http.StatusUnauthorized, "failed to exchange code")
        return
    }

Greptile Summary

This PR adds a single slog.ErrorContext call in handler/oauth2.go to emit a structured error log when OAuthConfig.Exchange() fails during the OAuth2 callback, improving observability without changing the existing 401 response contract.

  • Adds slog.ErrorContext(r.Context(), \"OAuth2 code exchange failed\", slog.Any(\"error\", err)) immediately before the writeError call in the Exchange failure branch, consistent with the logging pattern used throughout the file for other error paths (e.g., FetchUserInfo, findOrCreateUser).
  • The new log call correctly passes r.Context() as the first argument, satisfying the project's convention of including a context.Context in all slog calls.

Confidence Score: 5/5

This is a safe, additive change that introduces a single structured log line with no effect on request handling or response contracts.

The change is minimal and mechanical: one slog.ErrorContext call mirroring the same pattern already used for FetchUserInfo and findOrCreateUser failures elsewhere in the same function. Context is passed correctly, the error key follows project convention, and the HTTP response path is untouched.

No files require special attention.

Important Files Changed

Filename Overview
handler/oauth2.go Adds slog.ErrorContext(r.Context(), ...) before the existing writeError call in the Exchange failure branch — follows the established logging pattern throughout the file and correctly passes context.

Sequence Diagram

sequenceDiagram
    participant Browser
    participant Callback as OAuth2Handler.Callback
    participant Provider as OAuth2 Provider
    participant slog as slog (structured log)
    participant Response as HTTP Response

    Browser->>Callback: "GET /callback?code=...&state=..."
    Callback->>Callback: "Validate state cookie & PKCE verifier"
    Callback->>Provider: OAuthConfig.Exchange(ctx, code, verifier)
    alt Exchange succeeds
        Provider-->>Callback: "*oauth2.Token"
        Callback->>Provider: Provider.FetchUserInfo(ctx, token)
        Provider-->>Callback: "*OAuth2UserInfo"
        Callback->>Response: Set JWT/session cookies
        Callback->>Browser: "302 Redirect /?oauth2_login=1"
    else Exchange fails (NEW: now logged)
        Provider-->>Callback: error
        Callback->>slog: ErrorContext(ctx, "OAuth2 code exchange failed", error)
        Callback->>Response: 401 "failed to exchange code"
        Response-->>Browser: 401 JSON error
    end
Loading

Reviews (1): Last reviewed commit: "Log OAuth2 exchange failures in callback" | Re-trigger Greptile

Context used:

  • Context used - For any Go slog calls, make sure that they include... (source)

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

Adds structured server-side error logging to the OAuth2 callback when the authorization code exchange fails, improving operational diagnosability while preserving the existing client-facing behavior.

Changes:

  • Log OAuthConfig.Exchange() failures via slog.ErrorContext immediately before returning the existing 401 failed to exchange code response.
Show a summary per file
File Description
handler/oauth2.go Adds structured error logging for OAuth2 code exchange failures in the callback handler.

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

@veverkap veverkap merged commit b8e743a into main May 23, 2026
24 checks passed
@veverkap veverkap deleted the copilot/fix-oauth2-http-401-error branch May 23, 2026 15:32
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.

3 participants