diff --git a/src/app/api/auth/discord/__tests__/route.test.ts b/src/app/api/auth/discord/__tests__/route.test.ts index 47307eb9..2a1bfb00 100644 --- a/src/app/api/auth/discord/__tests__/route.test.ts +++ b/src/app/api/auth/discord/__tests__/route.test.ts @@ -32,7 +32,7 @@ describe('Discord OAuth API Route', () => { const response = await GET(mockRequest); - expect(response.status).toBe(302); // Redirect status + expect(response.status).toBe(307); // Temporary redirect status expect(response.headers.get('location')).toBe( 'https://discord.com/oauth2/authorize?test=param', ); diff --git a/src/app/api/auth/discord/callback/__tests__/route.test.ts b/src/app/api/auth/discord/callback/__tests__/route.test.ts index e37dd037..e48bb485 100644 --- a/src/app/api/auth/discord/callback/__tests__/route.test.ts +++ b/src/app/api/auth/discord/callback/__tests__/route.test.ts @@ -107,7 +107,7 @@ describe('Discord OAuth Callback API Route', () => { expect(json.message).toBe('Authorization code is required'); }); - it('should validate state parameter', async () => { + it('should validate state parameter using constant-time comparison', async () => { const mockRequest = { nextUrl: new URL( 'http://localhost:3000/api/auth/discord/callback?code=test_code&state=wrong_state', @@ -129,6 +129,47 @@ describe('Discord OAuth Callback API Route', () => { expect(json.message).toBe('Invalid state parameter'); }); + it('should reject when state parameter is missing', async () => { + const mockRequest = { + nextUrl: new URL( + 'http://localhost:3000/api/auth/discord/callback?code=test_code', + ), + headers: new Headers(), + cookies: { + get: vi.fn((name: string) => { + if (name === 'discord_oauth_state') return { value: 'correct_state' }; + return undefined; + }), + delete: vi.fn(), + }, + } as any; + + const response = await GET(mockRequest); + + expect(response.status).toBe(400); + const json = await response.json(); + expect(json.message).toBe('Invalid state parameter'); + }); + + it('should reject when stored state cookie is missing', async () => { + const mockRequest = { + nextUrl: new URL( + 'http://localhost:3000/api/auth/discord/callback?code=test_code&state=test_state', + ), + headers: new Headers(), + cookies: { + get: vi.fn(() => undefined), + delete: vi.fn(), + }, + } as any; + + const response = await GET(mockRequest); + + expect(response.status).toBe(400); + const json = await response.json(); + expect(json.message).toBe('Invalid state parameter'); + }); + it('should reject unverified Discord email', async () => { const { exchangeCodeForToken, getDiscordUser } = await import('@/lib/discord/oauth'); diff --git a/src/app/api/auth/discord/callback/route.ts b/src/app/api/auth/discord/callback/route.ts index 9d2ed3fc..419230d8 100644 --- a/src/app/api/auth/discord/callback/route.ts +++ b/src/app/api/auth/discord/callback/route.ts @@ -4,6 +4,7 @@ import { exchangeCodeForToken, getDiscordUser, getDiscordAvatarUrl } from '@/lib import type { AuthResponseDTO, AuthErrorDTO } from '@/types/api/auth.dto'; import { edgeLog } from '@/../infra/edge-config'; import { createLogger } from '@/lib/logging'; +import { validateOAuthState } from '@/middleware/security'; const logger = createLogger('api-auth-discord-callback'); @@ -41,9 +42,9 @@ export async function GET( ) as NextResponse; } - // Verify state parameter to prevent CSRF attacks + // Verify state parameter to prevent CSRF attacks using constant-time comparison const storedState = request.cookies.get('discord_oauth_state')?.value; - if (!state || state !== storedState) { + if (!validateOAuthState(state, storedState)) { edgeLog('error', '/api/auth/discord/callback', 'Invalid state parameter'); return addHeaders( NextResponse.json({ message: 'Invalid state parameter' }, { status: 400 }), diff --git a/src/lib/discord/__tests__/oauth.test.ts b/src/lib/discord/__tests__/oauth.test.ts index 246639b9..f91c57de 100644 --- a/src/lib/discord/__tests__/oauth.test.ts +++ b/src/lib/discord/__tests__/oauth.test.ts @@ -15,14 +15,28 @@ describe('Discord OAuth Utilities', () => { }); describe('generateState', () => { - it('should generate a random state string', () => { + it('should generate a cryptographically secure 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); + // Secure state should be 64 hex characters (32 bytes) + expect(state1.length).toBe(64); + expect(state2.length).toBe(64); + // Should be valid hex string + expect(state1).toMatch(/^[a-f0-9]{64}$/); + expect(state2).toMatch(/^[a-f0-9]{64}$/); + }); + + it('should generate unique states across multiple calls', () => { + const states = new Set(); + for (let i = 0; i < 100; i++) { + states.add(generateState()); + } + // All 100 states should be unique + expect(states.size).toBe(100); }); }); diff --git a/src/lib/discord/oauth.ts b/src/lib/discord/oauth.ts index 4be066f9..2de52319 100644 --- a/src/lib/discord/oauth.ts +++ b/src/lib/discord/oauth.ts @@ -3,6 +3,8 @@ * Handles Discord OAuth2 flow for authentication */ +import { generateOAuthState as generateSecureOAuthState } from '@/middleware/security'; + export interface DiscordUser { id: string; username: string; @@ -102,10 +104,10 @@ export async function getDiscordUser(accessToken: string): Promise } /** - * Generate a random state parameter for OAuth + * Generate a cryptographically secure random state parameter for OAuth */ export function generateState(): string { - return Math.random().toString(36).substring(2, 15) + Math.random().toString(36).substring(2, 15); + return generateSecureOAuthState(); } /**