fix: detect revoked GitHub token immediately instead of waiting up to 24 hours

src/lib/auth.ts

@@ -1,23 +1,17 @@
-import { type NextAuthOptions } from "next-auth";
+import { type NextAuthOptions } from "next-auth";
 import GitHubProvider from "next-auth/providers/github";
 import { syncGitHubAchievementsForUser } from "./github-achievements";
 import { supabaseAdmin } from "./supabase";
+import { wasTokenRevokedNow } from "./token-revocation-flag";
 
 const SESSION_MAX_AGE = 30 * 24 * 60 * 60;
 const SESSION_UPDATE_AGE = 24 * 60 * 60;
 const isPlaywrightServer = process.env.PLAYWRIGHT_SERVER_MODE === "start";
 
 const GITHUB_API = "https://api.github.com";
-// Re-validate the stored GitHub token at most once every 24 hours per session.
-// Catches revocations within a reasonable window without adding per-request latency.
-// Without this check a revoked token silently continues for up to 30 days (JWT lifetime).
 const TOKEN_VALIDATION_INTERVAL_MS = 24 * 60 * 60 * 1000;
 
 export const authOptions: NextAuthOptions = {
-  // Playwright runs on plain HTTP (127.0.0.1) and relies on the default
-  // `next-auth.session-token` cookie name. If NextAuth infers HTTPS via
-  // forwarded headers, it may switch to secure cookie prefixes and the E2E
-  // session cookie won't be read. Force non-secure cookies in this mode.
   ...(isPlaywrightServer ? { useSecureCookies: false } : {}),
   providers: [
     GitHubProvider({
@@ -31,9 +25,6 @@ export const authOptions: NextAuthOptions = {
   pages: {
     signIn: "/auth/signin",
   },
-  // Use NextAuth's default cookie behavior (secure cookies on HTTPS deployments),
-  // which keeps Playwright E2E (http://127.0.0.1) aligned with the default
-  // `next-auth.session-token` cookie name.
   session: {
     strategy: "jwt",
     maxAge: SESSION_MAX_AGE,
@@ -47,11 +38,6 @@ export const authOptions: NextAuthOptions = {
       if (account?.provider === "github" && profile) {
         const p = profile as { id: number; login: string; email?: string };
 
-        // Guard: supabaseAdmin is null when Supabase env vars are missing or
-        // contain placeholder values (see src/lib/supabase.ts). Calling .from()
-        // on null throws a TypeError which NextAuth silently converts to
-        // return false → error=github redirect. Skip the upsert gracefully
-        // so authentication can still succeed with degraded functionality.
         if (!supabaseAdmin) {
           console.warn(
             "signIn: supabaseAdmin is not configured; skipping DB upsert. " +
@@ -75,8 +61,6 @@ export const authOptions: NextAuthOptions = {
             .select("id")
             .single();
 
-          // If the email column does not exist yet (migration pending),
-          // PostgREST returns a 42703 error. Fallback to upsert without email.
           if (upsertError && (upsertError as { code?: string }).code === "42703") {
             const fallback = await supabaseAdmin
               .from("users")
@@ -111,21 +95,16 @@ export const authOptions: NextAuthOptions = {
             }
           }
         } catch (error) {
-          // Database failures must not block sign-in — the user is authenticated
-          // by GitHub; local sync is best-effort.
           console.error("[auth] signIn callback error (non-fatal):", error);
         }
       }
       return true;
     },
     async jwt({ token, account, profile, user }) {
-      // account is only populated on the initial sign-in; all subsequent JWT
-      // refreshes arrive here with account === undefined.
       if (account?.access_token) {
         token.accessToken = account.access_token;
-        // Record when we first obtained and validated this token so we know
-        // when the next liveness check is due.
         token.accessTokenValidatedAt = Date.now();
+        token.error = undefined;
       } else if (user && (user as any).accessToken) {
         token.accessToken = (user as any).accessToken;
         token.accessTokenValidatedAt = Date.now();
@@ -139,10 +118,18 @@ export const authOptions: NextAuthOptions = {
         token.githubLogin = (user as any).login || "mock-user";
       }
 
-      // Periodic token liveness check: if more than TOKEN_VALIDATION_INTERVAL_MS
-      // has elapsed since the last successful validation, hit GET /user with the
-      // stored token. A 401 response means the user has revoked access via GitHub
-      // Settings — flag the token so the dashboard can force re-authentication.
+      // Fast path: if a route just got a live 401 from GitHub for this user,
+      // it marks a short-lived flag (see token-revocation-flag.ts). Checking
+      // it here means the next session read reflects "TokenRevoked" right
+      // away instead of waiting for the 24h periodic check below.
+      if (
+        !token.error &&
+        typeof token.githubId === "string" &&
+        (await wasTokenRevokedNow(token.githubId))
+      ) {
+        token.error = "TokenRevoked";
+      }
+
       if (
         !account &&
         typeof token.accessToken === "string" &&
@@ -156,19 +143,12 @@ export const authOptions: NextAuthOptions = {
             cache: "no-store",
           });
           if (res.status === 401) {
-            // Explicit revocation: mark the session for forced sign-out.
             token.error = "TokenRevoked";
           } else if (res.ok) {
-            // Only advance the timestamp on a confirmed-good response; transient
-            // errors (429, 5xx) should be retried on the next request, not cached
-            // as a successful validation.
             token.accessTokenValidatedAt = Date.now();
           }
-          // Non-401 non-ok responses (rate limit, server error) are intentionally
-          // left without updating accessTokenValidatedAt so the next request retries.
         } catch (e) {
           // Network failures during validation are not treated as revocation.
-          // The check will be retried on the next request.
         }
       }
 
@@ -181,7 +161,6 @@ export const authOptions: NextAuthOptions = {
         session.githubId = token.githubId;
       if (typeof token.githubLogin === "string")
         session.githubLogin = token.githubLogin;
-      // Surface the revocation flag so pages can redirect to re-authentication.
       if (token.error === "TokenRevoked")
         session.error = "TokenRevoked";
       return session;

src/lib/github-fetch.ts

@@ -1,11 +1,12 @@
-/**
+/**
  * Typed GitHub API fetch helper.
  * Centralises Authorization headers, Accept header, ok-check,
  * and rate-limit error handling so metric routes don't
  * repeat the same ~10-line pattern.
  */
 
 import { GITHUB_API } from "@/lib/github";
+import { markTokenRevokedNow } from "@/lib/token-revocation-flag";
 
 export { GITHUB_API };
 
@@ -26,10 +27,6 @@ export class GitHubApiError extends Error {
   }
 }
 
-/**
- * Thrown when the GitHub API responds with 401, indicating the stored OAuth
- * token has been revoked or has expired.
- */
 export class GitHubAuthError extends Error {
   readonly status = 401;
   constructor() {
@@ -38,11 +35,6 @@ export class GitHubAuthError extends Error {
   }
 }
 
-/**
- * Returns a standardised 401 JSON response for metric routes that detect a
- * revoked token.  Client-side code checks for the `token_expired` error code
- * to show a reconnect prompt instead of a generic error state.
- */
 export function githubAuthErrorResponse(): Response {
   return Response.json(
     { error: "token_expired" },
@@ -78,26 +70,21 @@ function isSecondaryRateLimitBody(body: unknown): boolean {
 
 async function buildGitHubError(
   res: Response,
+  githubId?: string,
 ): Promise<GitHubRateLimitError | GitHubApiError> {
   const { resetAt, retryAfter, remaining } = extractRateLimitInfo(res.headers);
 
-  // 429: always a rate limit
   if (res.status === 429) {
     return new GitHubRateLimitError(resetAt, retryAfter);
   }
 
   if (res.status === 403) {
-    // Primary rate limit: quota exhausted
     if (remaining === 0) {
       return new GitHubRateLimitError(resetAt, retryAfter);
     }
-
-    // Secondary rate limit: Retry-After header signals required backoff
     if (retryAfter !== null) {
       return new GitHubRateLimitError(resetAt, retryAfter);
     }
-
-    // Secondary rate limit: body message indicates rate limiting
     let body: unknown = null;
     try {
       body = await res.json();
@@ -107,27 +94,29 @@ async function buildGitHubError(
     if (isSecondaryRateLimitBody(body)) {
       return new GitHubRateLimitError(resetAt, retryAfter);
     }
-
-    // Authorization failure: invalid token, insufficient scope, permissions
     return new GitHubApiError(res.status);
   }
 
+  if (res.status === 401 && githubId) {
+    // Live signal: this token just failed against GitHub right now. Flag it
+    // immediately so the next session check surfaces "TokenRevoked" without
+    // waiting for the 24h periodic validation in auth.ts.
+    await markTokenRevokedNow(githubId);
+  }
+
   return new GitHubApiError(res.status);
 }
 
 /**
  * Fetch a GitHub API endpoint with standard headers.
- * Throws GitHubRateLimitError when response headers or body indicate actual rate limiting:
- * - 429 responses
- * - 403 with X-RateLimit-Remaining: 0 (primary rate limit)
- * - 403 with Retry-After header (secondary rate limit)
- * - 403 with rate-limit message in response body (secondary rate limit)
- * Authorization failures (invalid token, insufficient scope, permissions) throw GitHubApiError.
+ * Pass githubId when available so a live 401 can immediately flag the
+ * token as revoked (see lib/token-revocation-flag.ts).
  */
 export async function githubFetch<T>(
   url: string,
   token: string,
-  options: RequestInit = {}
+  options: RequestInit = {},
+  githubId?: string,
 ): Promise<T> {
 
   const res = await fetch(url, {
@@ -141,7 +130,7 @@ export async function githubFetch<T>(
   });
 
   if (!res.ok) {
-    throw await buildGitHubError(res);
+    throw await buildGitHubError(res, githubId);
   }
 
   return res.json() as Promise<T>;
@@ -153,7 +142,8 @@ export async function githubFetch<T>(
 export async function githubGraphQL<T>(
   query: string,
   token: string,
-  variables?: Record<string, unknown>
+  variables?: Record<string, unknown>,
+  githubId?: string,
 ): Promise<T> {
 
   const MAX_RETRIES = 2;
@@ -169,14 +159,13 @@ export async function githubGraphQL<T>(
       cache: "no-store",
     });
 
-    // Retry on transient server errors (502/503) before error classification.
     if ((res.status === 502 || res.status === 503) && attempt < MAX_RETRIES) {
       await new Promise((r) => setTimeout(r, 1000 * (attempt + 1)));
       continue;
     }
 
     if (!res.ok) {
-      throw await buildGitHubError(res);
+      throw await buildGitHubError(res, githubId);
     }
 
     const json = await res.json();

src/lib/token-revocation-flag.ts

@@ -0,0 +1,28 @@
+import { getRedisClient } from "@/lib/metrics-cache";
+
+const REVOKED_FLAG_TTL_SECONDS = 5 * 60;
+
+function revokedFlagKey(githubId: string): string {
+  return `auth:token-revoked:${githubId}`;
+}
+
+export async function markTokenRevokedNow(githubId: string): Promise<void> {
+  const redis = getRedisClient();
+  if (!redis) return;
+  try {
+    await redis.set(revokedFlagKey(githubId), "1", { ex: REVOKED_FLAG_TTL_SECONDS });
+  } catch {
+    // Best-effort only; the 24h periodic check in auth.ts is the fallback.
+  }
+}
+
+export async function wasTokenRevokedNow(githubId: string): Promise<boolean> {
+  const redis = getRedisClient();
+  if (!redis) return false;
+  try {
+    const value = await redis.get<string>(revokedFlagKey(githubId));
+    return value === "1";
+  } catch {
+    return false;
+  }
+}

test/token-revocation-flag.test.ts

@@ -0,0 +1,71 @@
+import { describe, it, expect, beforeEach, vi } from "vitest";
+
+const mockRedisGet = vi.fn();
+const mockRedisSet = vi.fn();
+
+vi.mock("@upstash/redis", () => {
+  return {
+    Redis: vi.fn(function RedisMock() {
+      return {
+        get: mockRedisGet,
+        set: mockRedisSet,
+      };
+    }),
+  };
+});
+
+import {
+  markTokenRevokedNow,
+  wasTokenRevokedNow,
+} from "../src/lib/token-revocation-flag";
+
+describe("token-revocation-flag", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    process.env.UPSTASH_REDIS_REST_URL = "https://test-redis.example.com";
+    process.env.UPSTASH_REDIS_REST_TOKEN = "test-token";
+  });
+
+  it("markTokenRevokedNow writes a flag with a TTL", async () => {
+    mockRedisSet.mockResolvedValue("OK");
+
+    await markTokenRevokedNow("12345");
+
+    expect(mockRedisSet).toHaveBeenCalledWith(
+      "auth:token-revoked:12345",
+      "1",
+      { ex: 5 * 60 }
+    );
+  });
+
+  it("wasTokenRevokedNow returns true when the flag is set", async () => {
+    mockRedisGet.mockResolvedValue("1");
+
+    const result = await wasTokenRevokedNow("12345");
+
+    expect(result).toBe(true);
+    expect(mockRedisGet).toHaveBeenCalledWith("auth:token-revoked:12345");
+  });
+
+  it("wasTokenRevokedNow returns false when no flag was set", async () => {
+    mockRedisGet.mockResolvedValue(null);
+
+    const result = await wasTokenRevokedNow("12345");
+
+    expect(result).toBe(false);
+  });
+
+  it("wasTokenRevokedNow returns false if Redis throws", async () => {
+    mockRedisGet.mockRejectedValue(new Error("network error"));
+
+    const result = await wasTokenRevokedNow("12345");
+
+    expect(result).toBe(false);
+  });
+
+  it("markTokenRevokedNow does not throw if Redis throws", async () => {
+    mockRedisSet.mockRejectedValue(new Error("network error"));
+
+    await expect(markTokenRevokedNow("12345")).resolves.toBeUndefined();
+  });
+});