fix(security): validate GitHub OAuth state nonce to prevent login CSRF

src/app/api/auth/github/callback/__tests__/route.test.ts

@@ -0,0 +1,129 @@
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { GET } from '../route';
+
+vi.mock('@/lib/ratelimit', () => ({
+  withRateLimit: vi.fn(() => ({
+    addHeaders: (response: Response) => response,
+    rateLimitResponse: null,
+  })),
+}));
+
+vi.mock('@/lib/github/oauth', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('@/lib/github/oauth')>();
+  return {
+    ...actual,
+    exchangeCodeForToken: vi.fn(),
+    getGitHubUser: vi.fn(),
+    getGitHubAvatarUrl: vi.fn(() => 'https://example.com/avatar.png'),
+  };
+});
+
+vi.mock('@/../infra/edge-config', () => ({
+  edgeLog: vi.fn(),
+}));
+
+function createRequest(url: string) {
+  return {
+    nextUrl: new URL(url),
+    headers: new Headers(),
+    cookies: {
+      get: vi.fn((name: string) => {
+        if (name === 'github_oauth_state') return { value: 'test_state' };
+        return undefined;
+      }),
+      delete: vi.fn(),
+    },
+  } as any;
+}
+
+const CALLBACK_URL = 'http://localhost:3000/api/auth/github/callback';
+
+describe('GitHub OAuth Callback API Route', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  describe('GET /api/auth/github/callback', () => {
+    it('should handle a successful GitHub OAuth callback', async () => {
+      const { exchangeCodeForToken, getGitHubUser } = await import('@/lib/github/oauth');
+
+      (exchangeCodeForToken as any).mockResolvedValueOnce({
+        access_token: 'test_access_token',
+        token_type: 'Bearer',
+        scope: 'read:user user:email',
+      });
+
+      (getGitHubUser as any).mockResolvedValueOnce({
+        id: 123456789,
+        login: 'testuser',
+        name: 'Test User',
+        email: 'test@example.com',
+      });
+
+      const response = await GET(createRequest(`${CALLBACK_URL}?code=test_code&state=test_state`));
+
+      expect(response.status).toBe(200);
+      const json = await response.json();
+      expect(json.message).toBe('GitHub authentication successful');
+      expect(json.user).toBeTruthy();
+      expect(json.user.email).toBe('test@example.com');
+      expect(json.user.provider).toBe('github');
+      expect(json.token).toBeTruthy();
+    });
+
+    it('should reject a callback with a mismatched state parameter', async () => {
+      const response = await GET(createRequest(`${CALLBACK_URL}?code=test_code&state=wrong_state`));
+
+      expect(response.status).toBe(400);
+      const json = await response.json();
+      expect(json.message).toBe('Invalid state parameter');
+    });
+
+    it('should reject a callback with a missing state parameter', async () => {
+      const response = await GET(createRequest(`${CALLBACK_URL}?code=test_code`));
+
+      expect(response.status).toBe(400);
+      const json = await response.json();
+      expect(json.message).toBe('Invalid state parameter');
+    });
+
+    it('should handle an OAuth error from GitHub', async () => {
+      const response = await GET(createRequest(`${CALLBACK_URL}?error=access_denied`));
+
+      expect(response.status).toBe(400);
+      const json = await response.json();
+      expect(json.message).toContain('GitHub OAuth error');
+    });
+
+    it('should handle a missing authorization code', async () => {
+      const response = await GET(createRequest(`${CALLBACK_URL}?state=test_state`));
+
+      expect(response.status).toBe(400);
+      const json = await response.json();
+      expect(json.message).toBe('Authorization code is required');
+    });
+
+    it('should reject a GitHub account without a verified email', async () => {
+      const { exchangeCodeForToken, getGitHubUser } = await import('@/lib/github/oauth');
+
+      (exchangeCodeForToken as any).mockResolvedValueOnce({
+        access_token: 'test_access_token',
+        token_type: 'Bearer',
+        scope: 'read:user user:email',
+      });
+
+      (getGitHubUser as any).mockResolvedValueOnce({
+        id: 123456789,
+        login: 'testuser',
+        name: 'Test User',
+        email: null,
+      });
+
+      const response = await GET(createRequest(`${CALLBACK_URL}?code=test_code&state=test_state`));
+
+      expect(response.status).toBe(400);
+      const json = await response.json();
+      expect(json.message).toBe('GitHub account must have a verified email');
+    });
+  });
+});

src/app/api/auth/github/callback/route.ts

@@ -1,6 +1,11 @@
 import { NextRequest, NextResponse } from 'next/server';
 import { withRateLimit } from '@/lib/ratelimit';
-import { exchangeCodeForToken, getGitHubUser, getGitHubAvatarUrl } from '@/lib/github/oauth';
+import {
+  exchangeCodeForToken,
+  getGitHubUser,
+  getGitHubAvatarUrl,
+  validateState,
+} from '@/lib/github/oauth';
 import type { AuthResponseDTO, AuthErrorDTO } from '@/types/api/auth.dto';
 import { edgeLog } from '@/../infra/edge-config';
 
@@ -21,7 +26,7 @@ export async function GET(
   try {
     const searchParams = request.nextUrl.searchParams;
     const code = searchParams.get('code');
-    const state = searchParams.get('state');
+    const state = searchParams.get('state') ?? undefined;
     const error = searchParams.get('error');
 
     // Check for OAuth errors
@@ -40,7 +45,7 @@ export async function GET(
 
     // Verify state parameter to prevent CSRF attacks
     const storedState = request.cookies.get('github_oauth_state')?.value;
-    if (!state || state !== storedState) {
+    if (!validateState(state, storedState)) {
       edgeLog('error', '/api/auth/github/callback', 'Invalid state parameter');
       return addHeaders(
         NextResponse.json({ message: 'Invalid state parameter' }, { status: 400 }),

src/lib/github/__tests__/oauth.test.ts

@@ -0,0 +1,108 @@
+import { describe, it, expect, beforeEach } from 'vitest';
+import { getGitHubAuthUrl, generateState, validateState, getGitHubAvatarUrl } from '../oauth';
+
+const mockEnv = {
+  GITHUB_CLIENT_ID: 'test_client_id',
+  GITHUB_REDIRECT_URI: 'http://localhost:3000/api/auth/github/callback',
+};
+
+describe('GitHub OAuth Utilities', () => {
+  beforeEach(() => {
+    process.env.GITHUB_CLIENT_ID = mockEnv.GITHUB_CLIENT_ID;
+    process.env.GITHUB_REDIRECT_URI = mockEnv.GITHUB_REDIRECT_URI;
+  });
+
+  describe('generateState', () => {
+    it('should generate a random state string', () => {
+      const state1 = generateState();
+      const state2 = generateState();
+
+      expect(state1).toBeTruthy();
+      expect(state2).toBeTruthy();
+      expect(state1).not.toBe(state2);
+      expect(state1.length).toBeGreaterThan(10);
+    });
+
+    it('should produce an unpredictable, hex-encoded nonce with 256 bits of entropy', () => {
+      expect(generateState()).toMatch(/^[0-9a-f]{64}$/);
+    });
+  });
+
+  describe('validateState', () => {
+    it('should accept a state matching the stored value', () => {
+      expect(validateState('abc123', 'abc123')).toBe(true);
+    });
+
+    it('should reject a state that does not match the stored value', () => {
+      expect(validateState('wrong', 'right')).toBe(false);
+    });
+
+    it('should reject missing state or stored value', () => {
+      expect(validateState(undefined, 'stored')).toBe(false);
+      expect(validateState('present', undefined)).toBe(false);
+      expect(validateState(undefined, undefined)).toBe(false);
+    });
+  });
+
+  describe('getGitHubAuthUrl', () => {
+    it('should generate correct GitHub authorization URL', () => {
+      const state = 'test_state_123';
+      const url = getGitHubAuthUrl(state);
+      const params = new URL(url).searchParams;
+
+      expect(url).toContain('https://github.com/login/oauth/authorize');
+      expect(params.get('client_id')).toBe(mockEnv.GITHUB_CLIENT_ID);
+      expect(params.get('redirect_uri')).toBe(mockEnv.GITHUB_REDIRECT_URI);
+      expect(params.get('scope')).toBe('read:user user:email');
+      expect(params.get('state')).toBe(state);
+    });
+
+    it('should throw error when GITHUB_CLIENT_ID is missing', () => {
+      process.env.GITHUB_CLIENT_ID = '';
+      expect(() => getGitHubAuthUrl('test_state')).toThrow('GitHub OAuth configuration is missing');
+    });
+
+    it('should throw error when GITHUB_REDIRECT_URI is missing', () => {
+      process.env.GITHUB_REDIRECT_URI = '';
+      expect(() => getGitHubAuthUrl('test_state')).toThrow('GitHub OAuth configuration is missing');
+    });
+  });
+
+  describe('getGitHubAvatarUrl', () => {
+    it('should return the avatar URL when present', () => {
+      const user = {
+        id: 1,
+        login: 'testuser',
+        name: 'Test User',
+        email: 'test@example.com',
+        avatar_url: 'https://avatars.githubusercontent.com/u/1?v=4',
+        html_url: 'https://github.com/testuser',
+        bio: null,
+        location: null,
+        blog: null,
+        twitter_username: null,
+        company: null,
+      };
+
+      expect(getGitHubAvatarUrl(user)).toBe('https://avatars.githubusercontent.com/u/1?v=4');
+    });
+
+    it('should return an empty string when the user has no avatar', () => {
+      const user = {
+        id: 1,
+        login: 'testuser',
+        name: 'Test User',
+        email: 'test@example.com',
+        avatar_url: '',
+        html_url: 'https://github.com/testuser',
+        bio: null,
+        location: null,
+        blog: null,
+        twitter_username: null,
+        company: null,
+      };
+
+      expect(getGitHubAvatarUrl(user)).toBe('');
+    });
+  });
+});

src/lib/github/oauth.ts

@@ -3,6 +3,8 @@
  * Handles GitHub OAuth2 flow for authentication
  */
 
+import { generateOAuthState, validateOAuthState } from '@/middleware/security';
+
 export interface GitHubUser {
   id: number;
   login: string;
@@ -123,9 +125,20 @@ export async function getGitHubUser(accessToken: string): Promise<GitHubUser> {
 
 /**
  * Generate a random state parameter for OAuth
+ *
+ * The value is drawn from the CSPRNG so it cannot be predicted or replayed by
+ * an attacker, which is the guarantee the OAuth `state` parameter exists for.
  */
 export function generateState(): string {
-  return Math.random().toString(36).substring(2, 15) + Math.random().toString(36).substring(2, 15);
+  return generateOAuthState();
+}
+
+/**
+ * Verify an OAuth `state` supplied on the callback against the value stored
+ * when the flow started. Guards the exchange against login CSRF.
+ */
+export function validateState(actual?: string, expected?: string): boolean {
+  return validateOAuthState(actual, expected);
 }
 
 /**

src/middleware/__tests__/security.test.ts

@@ -1,6 +1,11 @@
 import { describe, expect, it } from 'vitest';
 import type { NextRequest, NextResponse } from 'next/server';
-import { applySecurityHeaders, buildSecurityHeaders } from '../security';
+import {
+  applySecurityHeaders,
+  buildSecurityHeaders,
+  generateOAuthState,
+  validateOAuthState,
+} from '../security';
 import { CSP_NONCE_REQUEST_HEADER, CSP_NONCE_RESPONSE_HEADER, containsUnsafeSources } from '../csp';
 
 function createRequest(options: { nonce?: string; protocol?: string } = {}): NextRequest {
@@ -105,3 +110,36 @@ describe('applySecurityHeaders', () => {
     );
   });
 });
+
+describe('generateOAuthState', () => {
+  it('returns distinct, high-entropy hex nonces', () => {
+    const state1 = generateOAuthState();
+    const state2 = generateOAuthState();
+
+    expect(state1).toMatch(/^[0-9a-f]{64}$/);
+    expect(state2).toMatch(/^[0-9a-f]{64}$/);
+    expect(state1).not.toBe(state2);
+  });
+
+  it('derives entropy from the configured byte length', () => {
+    expect(generateOAuthState(16)).toMatch(/^[0-9a-f]{32}$/);
+    expect(generateOAuthState(1)).toMatch(/^[0-9a-f]{2}$/);
+  });
+});
+
+describe('validateOAuthState', () => {
+  it('accepts a matching value', () => {
+    expect(validateOAuthState('deadbeef', 'deadbeef')).toBe(true);
+  });
+
+  it('rejects mismatched values', () => {
+    expect(validateOAuthState('deadbeef', 'deadbeec')).toBe(false);
+    expect(validateOAuthState('deadbeef', 'deadbeef1')).toBe(false);
+  });
+
+  it('rejects missing values', () => {
+    expect(validateOAuthState(undefined, 'deadbeef')).toBe(false);
+    expect(validateOAuthState('deadbeef', undefined)).toBe(false);
+    expect(validateOAuthState(undefined, undefined)).toBe(false);
+  });
+});

src/middleware/security.ts

@@ -85,3 +85,40 @@ export function applySecurityHeaders(
 
   return response;
 }
+
+/** Entropy (in bytes) for a generated OAuth state nonce — 256 bits. */
+const OAUTH_STATE_BYTES = 32;
+
+/**
+ * Generate a cryptographically random OAuth `state` nonce.
+ *
+ * The OAuth `state` parameter must be unpredictable. A guessable value lets an
+ * attacker prefix the flow with a state they control, then submit the victim's
+ * authorization code back to the callback as a login CSRF. `Math.random()` is
+ * seeded by time and is not cryptographically strong, so the value is drawn
+ * from the CSPRNG (`crypto.getRandomValues`) available on the edge runtime.
+ */
+export function generateOAuthState(byteLength: number = OAUTH_STATE_BYTES): string {
+  const bytes = new Uint8Array(byteLength);
+  crypto.getRandomValues(bytes);
+
+  return Array.from(bytes, (byte) => byte.toString(16).padStart(2, '0')).join('');
+}
+
+/**
+ * Constant-time comparison of an OAuth `state` nonce against the stored value.
+ * The value supplied on the callback (`actual`) must match the value stored
+ * when the flow started (`expected`); any mismatch aborts the exchange.
+ */
+export function validateOAuthState(actual?: string, expected?: string): boolean {
+  if (!actual || !expected || actual.length !== expected.length) {
+    return false;
+  }
+
+  let diff = 0;
+  for (let i = 0; i < expected.length; i += 1) {
+    diff |= actual.charCodeAt(i) ^ expected.charCodeAt(i);
+  }
+
+  return diff === 0;
+}