diff --git a/src/lib/auth.ts b/src/lib/auth.ts index 66b24a92a..d5ed599a1 100644 --- a/src/lib/auth.ts +++ b/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; diff --git a/src/lib/github-fetch.ts b/src/lib/github-fetch.ts index bc83b1093..aed4197d5 100644 --- a/src/lib/github-fetch.ts +++ b/src/lib/github-fetch.ts @@ -1,4 +1,4 @@ -/** +/** * Typed GitHub API fetch helper. * Centralises Authorization headers, Accept header, ok-check, * and rate-limit error handling so metric routes don't @@ -6,6 +6,7 @@ */ 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 { 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( url: string, token: string, - options: RequestInit = {} + options: RequestInit = {}, + githubId?: string, ): Promise { const res = await fetch(url, { @@ -141,7 +130,7 @@ export async function githubFetch( }); if (!res.ok) { - throw await buildGitHubError(res); + throw await buildGitHubError(res, githubId); } return res.json() as Promise; @@ -153,7 +142,8 @@ export async function githubFetch( export async function githubGraphQL( query: string, token: string, - variables?: Record + variables?: Record, + githubId?: string, ): Promise { const MAX_RETRIES = 2; @@ -169,14 +159,13 @@ export async function githubGraphQL( 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(); diff --git a/src/lib/token-revocation-flag.ts b/src/lib/token-revocation-flag.ts new file mode 100644 index 000000000..682c899ad --- /dev/null +++ b/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 { + 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 { + const redis = getRedisClient(); + if (!redis) return false; + try { + const value = await redis.get(revokedFlagKey(githubId)); + return value === "1"; + } catch { + return false; + } +} diff --git a/test/token-revocation-flag.test.ts b/test/token-revocation-flag.test.ts new file mode 100644 index 000000000..7c65c329d --- /dev/null +++ b/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(); + }); +});