diff --git a/src/app/api/auth/github/callback/__tests__/route.test.ts b/src/app/api/auth/github/callback/__tests__/route.test.ts new file mode 100644 index 00000000..4441e487 --- /dev/null +++ b/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(); + 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'); + }); + }); +}); diff --git a/src/app/api/auth/github/callback/route.ts b/src/app/api/auth/github/callback/route.ts index 77fcf01e..26c16c68 100644 --- a/src/app/api/auth/github/callback/route.ts +++ b/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 }), diff --git a/src/lib/github/__tests__/oauth.test.ts b/src/lib/github/__tests__/oauth.test.ts new file mode 100644 index 00000000..cc4e69a5 --- /dev/null +++ b/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(''); + }); + }); +}); diff --git a/src/lib/github/oauth.ts b/src/lib/github/oauth.ts index 05de6569..505a8a47 100644 --- a/src/lib/github/oauth.ts +++ b/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 { /** * 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); } /** diff --git a/src/middleware/__tests__/security.test.ts b/src/middleware/__tests__/security.test.ts index 04be0b5e..7b74b0b6 100644 --- a/src/middleware/__tests__/security.test.ts +++ b/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); + }); +}); diff --git a/src/middleware/security.ts b/src/middleware/security.ts index c9261ebc..1a4149a7 100644 --- a/src/middleware/security.ts +++ b/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; +}