Handle GitHub OAuth failures in /auth/callback without a 500 (#54)

tarekziade · claude · web-flow · commit 8813ab85ad66 · 2026-07-07T15:31:23.000+02:00
## What A user hit **Internal Server Error** on `https://serge.huggingface.tech/login` today. The OAuth roundtrip got through the token exchange but GitHub rejected the freshly-issued token on `GET /user`: ``` POST github.com/login/oauth/access_token → 200 OK GET api.github.com/user → 401 Unauthorized ("Bad credentials") GET /auth/callback?... → 500 Internal Server Error ``` `auth_callback` called `user_resp.raise_for_status()`, so the 401 bubbled up as an unhandled `httpx.HTTPStatusError` → bare **500** with no logging of *why*. ## Change Each GitHub step in the callback (token exchange, `/user`, and the org-membership-derived allow-list check) now: - logs GitHub's own status + response body (so we can diagnose the *next* failure), and - redirects to `/login?error=<code>` instead of raising. `login.html` maps the code to a readable message: - `github_auth_failed` — "GitHub couldn't verify your sign-in. This is usually temporary — please try again." - `invalid_oauth_state` — expired/reused sign-in link - `not_allowed` — account not on the allow-list ## Testing Drove the real 401 path with a FastAPI `TestClient` + mocked GitHub responses: the `/user` 401 now yields `302 → /login?error=github_auth_failed` (no 500), and the log line reads `oauth /user lookup failed: github returned 401: {"message":"Bad credentials"}`. `ruff format`/`check` clean; no tests referenced the old error strings. Note: the underlying 401 on a just-minted token is most likely transient (or an OAuth App `client_secret` drift) — this PR is the UX/robustness fix so it surfaces cleanly instead of a 500. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
diff --git a/reviewbot/static/login.html b/reviewbot/static/login.html
@@ -6,6 +6,16 @@
   <meta name="viewport" content="width=device-width, initial-scale=1">
   <link rel="icon" type="image/png" href="/static/serge-logo.png">
   <link rel="stylesheet" href="/static/styles.css">
+  <style>
+    .login-error {
+      margin: 0 0 1rem;
+      padding: .625rem .875rem;
+      border: 1px solid var(--danger);
+      border-radius: 6px;
+      background: rgba(220, 38, 38, .08);
+      color: var(--danger);
+    }
+  </style>
 </head>
 <body>
   <header class="bar">
@@ -17,11 +27,29 @@ <h1><img class="brand-logo" src="/static/serge-logo.png" alt=""> Serge</h1>
   <main>
     <section class="card">
       <h2>Sign in</h2>
+      <p id="login-error" class="login-error" hidden></p>
       <p>You need to be on the allow-list to use this app. Sign in with GitHub to continue.</p>
       <p class="actions">
         <a href="/auth/login"><button class="primary">Sign in with GitHub</button></a>
       </p>
     </section>
   </main>
+  <script>
+    (function () {
+      var code = new URLSearchParams(window.location.search).get("error");
+      if (!code) return;
+      var messages = {
+        github_auth_failed:
+          "GitHub couldn't verify your sign-in. This is usually temporary — please try again in a moment.",
+        invalid_oauth_state:
+          "Your sign-in link expired or was already used. Please start again.",
+        not_allowed:
+          "Your GitHub account isn't on the allow-list for this app.",
+      };
+      var el = document.getElementById("login-error");
+      el.textContent = messages[code] || "Sign-in failed. Please try again.";
+      el.hidden = false;
+    })();
+  </script>
 </body>
 </html>
diff --git a/reviewbot/webapp.py b/reviewbot/webapp.py
@@ -2154,6 +2154,13 @@ def journal_page(request: Request) -> Response:
     return HTMLResponse(html)
 
 
+def _login_error(code: str) -> RedirectResponse:
+    """Bounce a failed sign-in back to the login page with a machine code
+    the static page turns into a human message. Beats a bare 500/JSON body
+    when GitHub rejects the OAuth roundtrip."""
+    return RedirectResponse(f"/login?error={code}", status_code=302)
+
+
 @app.get("/login")
 def login_page(request: Request) -> Response:
     if _current_user(request):
@@ -2199,7 +2206,7 @@ async def auth_callback(request: Request) -> Response:
     sess = _load_session(request)
     expected_state = sess.pop("oauth_state", None)
     if not code or not state or state != expected_state:
-        raise HTTPException(status_code=400, detail="invalid_oauth_state")
+        return _login_error("invalid_oauth_state")
 
     async with httpx.AsyncClient(timeout=30) as client:
         token_resp = await client.post(
@@ -2211,10 +2218,25 @@ async def auth_callback(request: Request) -> Response:
             },
             headers={"Accept": "application/json"},
         )
-        token_resp.raise_for_status()
-        token = token_resp.json().get("access_token")
+        if not token_resp.is_success:
+            log.warning(
+                "oauth token exchange failed: github returned %s: %s",
+                token_resp.status_code,
+                token_resp.text[:500],
+            )
+            return _login_error("github_auth_failed")
+        token_body = token_resp.json()
+        token = token_body.get("access_token")
         if not token:
-            raise HTTPException(status_code=400, detail="oauth_token_exchange_failed")
+            # GitHub answers 200 with an error payload for an expired/reused
+            # code or mismatched client credentials — the token is just absent.
+            log.warning(
+                "oauth token exchange returned no token: %s",
+                token_body.get("error_description")
+                or token_body.get("error")
+                or token_body,
+            )
+            return _login_error("github_auth_failed")
 
         user_resp = await client.get(
             "https://api.github.com/user",
@@ -2223,10 +2245,21 @@ async def auth_callback(request: Request) -> Response:
                 "Accept": "application/vnd.github+json",
             },
         )
-        user_resp.raise_for_status()
+        if not user_resp.is_success:
+            # A token GitHub just minted getting rejected here (typically
+            # 401 "Bad credentials") points at an OAuth App credential
+            # problem or a transient GitHub blip. Surface it cleanly and log
+            # GitHub's own message instead of throwing an unhandled 500.
+            log.warning(
+                "oauth /user lookup failed: github returned %s: %s",
+                user_resp.status_code,
+                user_resp.text[:500],
+            )
+            return _login_error("github_auth_failed")
         login = user_resp.json().get("login")
         if not isinstance(login, str) or not login:
-            raise HTTPException(status_code=400, detail="oauth_no_login")
+            log.warning("oauth /user succeeded but returned no login field")
+            return _login_error("github_auth_failed")
 
         # Always fetch orgs at login: even when web_allowed_orgs is
         # empty, the provider_configs table uses orgs to gate which API
@@ -2263,7 +2296,7 @@ async def auth_callback(request: Request) -> Response:
                     break
         if not verified_via_app:
             log.warning("denied login attempt by %s (orgs=%s)", login, orgs)
-            raise HTTPException(status_code=403, detail="user_not_allowed")
+            return _login_error("not_allowed")
         log.info(
             "user %s authorized via App membership lookup (orgs=%s)",
             login,
