diff --git a/src/controllers/oauthSsoController.ts b/src/controllers/oauthSsoController.ts index a417f6d..4cf381e 100644 --- a/src/controllers/oauthSsoController.ts +++ b/src/controllers/oauthSsoController.ts @@ -12,12 +12,40 @@ import { getPublicProviderInfo, isLocalAuthAllowed, isOAuthSsoEnabled, + getOAuthSsoConfig as getSsoConfigFromService, } from '../services/oauthSsoService.js'; import { JWT_SECRET } from '../config/jwt.js'; import config from '../config/index.js'; const TOKEN_EXPIRY = '24h'; +/** + * Get the base URL for OAuth callbacks + * Uses configured callbackBaseUrl if available, otherwise derives from request + * This approach is more secure than blindly trusting forwarded headers + */ +async function getCallbackBaseUrl(req: Request): Promise { + // First, check if a callback base URL is configured (most secure option) + const ssoConfig = await getSsoConfigFromService(); + if (ssoConfig?.callbackBaseUrl) { + return ssoConfig.callbackBaseUrl; + } + + // Fall back to deriving from request (less secure, but works in simpler setups) + // Only trust forwarded headers if app is configured to trust proxy + if (req.app.get('trust proxy') && req.headers['x-forwarded-proto'] && req.headers['x-forwarded-host']) { + const proto = Array.isArray(req.headers['x-forwarded-proto']) + ? req.headers['x-forwarded-proto'][0] + : req.headers['x-forwarded-proto']; + const host = Array.isArray(req.headers['x-forwarded-host']) + ? req.headers['x-forwarded-host'][0] + : req.headers['x-forwarded-host']; + return `${proto}://${host}`; + } + + return `${req.protocol}://${req.get('host')}`; +} + /** * Get OAuth SSO configuration for frontend * Returns enabled providers and whether local auth is allowed @@ -65,10 +93,9 @@ export const initiateOAuthLogin = async (req: Request, res: Response): Promise(); const STATE_TTL_MS = 10 * 60 * 1000; // 10 minutes // Cleanup old state entries periodically -setInterval(() => { - const now = Date.now(); - for (const [state, entry] of stateStore.entries()) { - if (now - entry.createdAt > STATE_TTL_MS) { - stateStore.delete(state); +let cleanupInterval: ReturnType | null = null; + +function startStateCleanup(): void { + if (cleanupInterval) return; + cleanupInterval = setInterval(() => { + const now = Date.now(); + for (const [state, entry] of stateStore.entries()) { + if (now - entry.createdAt > STATE_TTL_MS) { + stateStore.delete(state); + } } + }, 60 * 1000); // Cleanup every minute +} + +// Start cleanup on module load +startStateCleanup(); + +/** + * Stop the state cleanup interval (useful for tests and graceful shutdown) + */ +export function stopStateCleanup(): void { + if (cleanupInterval) { + clearInterval(cleanupInterval); + cleanupInterval = null; } -}, 60 * 1000); // Cleanup every minute +} + +// GitHub API response types for type safety +interface GitHubUserResponse { + id: number; + login: string; + name?: string; + email?: string; + avatar_url?: string; +} + +interface GitHubEmailResponse { + email: string; + primary: boolean; + verified: boolean; + visibility?: string; +} // Provider configurations cache const providerConfigsCache = new Map< @@ -326,7 +362,7 @@ async function getUserInfo( throw new Error(`Failed to fetch GitHub user info: ${response.statusText}`); } - const data = await response.json(); + const data = (await response.json()) as GitHubUserResponse; // Fetch email separately if not public let email = data.email; @@ -339,8 +375,8 @@ async function getUserInfo( }); if (emailResponse.ok) { - const emails = await emailResponse.json(); - const primaryEmail = emails.find((e: any) => e.primary); + const emails = (await emailResponse.json()) as GitHubEmailResponse[]; + const primaryEmail = emails.find((e) => e.primary); email = primaryEmail?.email || emails[0]?.email; } } diff --git a/tests/services/oauthSsoService.test.ts b/tests/services/oauthSsoService.test.ts index 7db846e..592d81f 100644 --- a/tests/services/oauthSsoService.test.ts +++ b/tests/services/oauthSsoService.test.ts @@ -24,6 +24,7 @@ import { isLocalAuthAllowed, getPublicProviderInfo, clearProviderCache, + stopStateCleanup, } from '../../src/services/oauthSsoService.js'; describe('OAuth SSO Service', () => { @@ -32,6 +33,11 @@ describe('OAuth SSO Service', () => { >; const mockGetUserDao = daoModule.getUserDao as jest.MockedFunction; + // Stop the cleanup interval to prevent Jest from hanging + afterAll(() => { + stopStateCleanup(); + }); + const defaultSsoConfig = { enabled: true, allowLocalAuth: true,