diff --git a/resources/coverage-badge.svg b/resources/coverage-badge.svg index edfcbfb..4396890 100644 --- a/resources/coverage-badge.svg +++ b/resources/coverage-badge.svg @@ -1,5 +1,5 @@ - - coverage: 83% + + coverage: 82.9% @@ -7,17 +7,17 @@ - + - - + + coverage coverage - 83% - 83% + 82.9% + 82.9% diff --git a/src/app.ts b/src/app.ts index 4d02498..ef7fc7e 100644 --- a/src/app.ts +++ b/src/app.ts @@ -15,8 +15,8 @@ import { dynamicRateLimit } from './middleware/rateLimit.js'; import { logRoute } from './middleware/routeLogger.js'; import { dynamicSlowDown } from './middleware/slowDown.js'; import { applyTrustedClientIp } from './middleware/trustedClientIp.js'; -import { AuthEvent } from './models/authEvents.js'; import { generateOpenApiDocument } from './openapi/document.js'; +import { AuthEventService } from './services/authEventService.js'; import getLogger from './utils/logger.js'; const logger = getLogger('app'); @@ -35,13 +35,13 @@ const corsOptions: CorsOptions = { } logger.warn(`Unknown CORS origin: ${origin}`); - AuthEvent.create({ - user_id: null, - type: 'request_suspicious', - ip_address: origin, - user_agent: 'unknown', - metadata: { reason: 'Unknown origin request' }, - }); + void AuthEventService.requestSuspiciousContext( + { + ipAddress: origin, + userAgent: 'unknown', + }, + { reason: 'Unknown origin request' }, + ); return callback(null, false); }, credentials: true, @@ -92,11 +92,8 @@ export async function createApp() { await loadRoutes(app); app.use((err: Error, req: Request, res: Response, next: NextFunction) => { if (err.message === 'Not allowed by CORS') { - AuthEvent.create({ - type: 'request_suspicous', - ip_address: req.ip, - user_agent: req.headers['user-agent'], - metadata: { reason: 'Request from an unexpected origin' }, + void AuthEventService.requestSuspicious(req, { + reason: 'Request from an unexpected origin', }); res.setHeader('Access-Control-Allow-Origin', rawOrigin[0]); return res.status(403).json({ message: 'CORS policy does not allow this origin.' }); @@ -120,15 +117,10 @@ export async function createApp() { logger.warn( `[${req.ip}] ${req.method} ${req.originalUrl} did not match any route. Tracking suspicious behavior`, ); - AuthEvent.create({ - type: 'request_suspicous', - ip_address: req.ip, - user_agent: req.headers['user-agent'], - metadata: { - reason: 'Request to an unknown route.', - method: req.method, - path: req.originalUrl, - }, + void AuthEventService.requestSuspicious(req, { + reason: 'Request to an unknown route.', + method: req.method, + path: req.originalUrl, }); return res.status(404).json({ error: 'Not Found' }); }); diff --git a/src/controllers/authentication.ts b/src/controllers/authentication.ts index 6ed8c99..f4e7c69 100644 --- a/src/controllers/authentication.ts +++ b/src/controllers/authentication.ts @@ -245,12 +245,7 @@ export const refreshSession = async (req: Request, res: Response) => { if (!refreshToken) { logger.error('Refresh token provided is not of expected type for auth server configurations'); - await AuthEventService.log({ - userId: null, - type: 'refresh_token_failed', - req, - metadata: { reason: 'Missing refresh token' }, - }); + await AuthEventService.refreshTokenFailed(req, { reason: 'Missing refresh token' }); res.status(401).json({ error: 'Not allowed' }); return; } @@ -274,7 +269,11 @@ export const refreshSession = async (req: Request, res: Response) => { ); } - await AuthEventService.serviceTokenInvalid(req); + await AuthEventService.refreshTokenFailed(req, { + reason: 'No refresh session found for refresh token', + legacyFallbackCandidates, + tokenFormat: looksLikeJwt ? 'jwt_like' : 'opaque', + }); return res.status(401).json({ error: 'invalid_refresh_token' }); } @@ -291,7 +290,17 @@ export const refreshSession = async (req: Request, res: Response) => { ); // Reuse -> revoke session chain await revokeSessionChain(session); - // Log security event + await AuthEventService.log({ + userId: session.userId, + type: 'refresh_token_suspicious', + req, + metadata: { + reason: 'Refresh token reuse detected', + sessionId: session.id, + replacedBySessionId: session.replacedBySessionId, + revokedAt: session.revokedAt?.toISOString() ?? null, + }, + }); return res.status(401).json({ error: 'refresh_token_reused' }); } @@ -301,6 +310,15 @@ export const refreshSession = async (req: Request, res: Response) => { const user = await User.findByPk(session.userId); if (!user) { + await AuthEventService.log({ + userId: session.userId, + type: 'refresh_token_suspicious', + req, + metadata: { + reason: 'Refresh session user not found', + sessionId: session.id, + }, + }); await hardRevokeSession(session, 'user_not_found'); return res.status(401).json({ error: 'invalid_session' }); } diff --git a/src/controllers/webauthn.ts b/src/controllers/webauthn.ts index 66b551e..2e9c638 100644 --- a/src/controllers/webauthn.ts +++ b/src/controllers/webauthn.ts @@ -156,11 +156,10 @@ const verifyWebAuthnRegistration = async (req: Request, res: Response) => { const expectedChallenge = user.challenge; if (!expectedChallenge) { logger.error('Unexpected user challegnge supplied.'); - await AuthEvent.create({ - user_id: user.id, - type: 'registration_suspicous', - ip_address: req.ip, - user_agent: req.headers['user-agent'], + await AuthEventService.log({ + userId: user.id, + type: 'registration_suspicious', + req, metadata: { reason: 'Missing challenge for registration' }, }); return res.status(403).json({ message: 'Missing challenge' }); diff --git a/src/middleware/verifyCookieAuth.ts b/src/middleware/verifyCookieAuth.ts index 0b60830..a92f37e 100644 --- a/src/middleware/verifyCookieAuth.ts +++ b/src/middleware/verifyCookieAuth.ts @@ -128,7 +128,10 @@ async function performSilentRefresh(req: Request, res: Response): Promise; diff --git a/src/services/authEventService.ts b/src/services/authEventService.ts index 6db59c3..3960791 100644 --- a/src/services/authEventService.ts +++ b/src/services/authEventService.ts @@ -7,86 +7,54 @@ import { Request } from 'express'; import { AuthEvent } from '../models/authEvents.js'; +import type { AuthEventType } from '../schemas/authEvent.types.js'; import getLogger from '../utils/logger.js'; const logger = getLogger('authEventService'); -export type AuthEventType = - | 'auth_action_incremented' - | 'bearer_token_failed' - | 'bearer_token_success' - | 'bearer_token_suspicious' - | 'bootstrap_admin_granted' - | 'bootstrap_admin_check_skipped' - | 'cookie_token_failed' - | 'cookie_token_success' - | 'cookie_token_suspicious' - | 'informational' - | 'internal_user_updated_by_owner' - | 'jwks_failed' - | 'jwks_success' - | 'jwks_suspicious' - | 'login_failed' - | 'login_success' - | 'login_suspicious' - | 'logout_failed' - | 'logout_success' - | 'logout_suspicious' - | 'magic_link_poll_completed_successfully' - | 'magic_link_requested' - | 'magic_link_success' - | 'mfa_otp_failed' - | 'mfa_otp_success' - | 'mfa_otp_suspicious' - | 'notication_sent' - | 'otp_failed' - | 'otp_success' - | 'otp_suspicious' - | 'recovery_otp_failed' - | 'recovery_otp_success' - | 'recovery_otp_suspicious' - | 'refresh_token_failed' - | 'refresh_token_success' - | 'refresh_token_suspicious' - | 'registration_failed' - | 'registration_success' - | 'registration_suspicious' - | 'service_token_failed' - | 'service_token_rotated' - | 'service_token_success' - | 'service_token_suspicious' - | 'system_config_error' - | 'system_config_read' - | 'system_config_updated' - | 'user_created' - | 'user_data_failed' - | 'user_data_success' - | 'user_data_suspicious' - | 'verify_otp_failed' - | 'verify_otp_success' - | 'verify_otp_suspicious' - | 'webauthn_login_failed' - | 'webauthn_login_success' - | 'webauthn_login_suspicious' - | 'webauthn_registration_failed' - | 'webauthn_registration_success' - | 'webauthn_registration_suspicious'; +type DeprecatedAuthEventType = 'notication_sent' | 'registration_suspicous' | 'request_suspicous'; + +type LoggableAuthEventType = AuthEventType | DeprecatedAuthEventType; + +const AUTH_EVENT_TYPE_ALIASES: Record = { + notication_sent: 'notification_sent', + registration_suspicous: 'registration_suspicious', + request_suspicous: 'request_suspicious', +}; + +function normalizeAuthEventType(type: LoggableAuthEventType): AuthEventType { + return AUTH_EVENT_TYPE_ALIASES[type as DeprecatedAuthEventType] ?? type; +} export interface AuthEventOptions { userId?: string | null; - type: AuthEventType; + type: LoggableAuthEventType; req: Request; metadata?: Record | null; } +interface AuthEventContextOptions { + userId?: string | null; + type: LoggableAuthEventType; + ipAddress?: string | null; + userAgent?: string | null; + metadata?: Record | null; +} + export class AuthEventService { - static async log({ userId = null, type, req, metadata = null }: AuthEventOptions) { + static async logContext({ + userId = null, + type, + ipAddress = 'unknown', + userAgent = 'unknown', + metadata = null, + }: AuthEventContextOptions) { try { await AuthEvent.create({ user_id: userId, - type, - ip_address: req.ip || 'unknown', - user_agent: req.headers['user-agent'] || 'unknown', + type: normalizeAuthEventType(type), + ip_address: ipAddress || 'unknown', + user_agent: userAgent || 'unknown', metadata, }); } catch (err) { @@ -94,6 +62,16 @@ export class AuthEventService { } } + static async log({ userId = null, type, req, metadata = null }: AuthEventOptions) { + return this.logContext({ + userId, + type, + ipAddress: req.ip, + userAgent: req.headers['user-agent'], + metadata, + }); + } + static loginSuccess(userId: string, req: Request) { return this.log({ userId, type: 'login_success', req }); } @@ -121,7 +99,7 @@ export class AuthEventService { } static notificationSent(by: string, req: Request, metadata?: Record) { - return this.log({ userId: by, type: 'notication_sent', req, metadata }); + return this.log({ userId: by, type: 'notification_sent', req, metadata }); } static serviceTokenUsed(clientId: string, req: Request) { @@ -139,4 +117,32 @@ export class AuthEventService { req, }); } + + static refreshTokenFailed(req: Request, metadata?: Record | null) { + return this.log({ + type: 'refresh_token_failed', + metadata: metadata ?? null, + req, + }); + } + + static requestSuspicious(req: Request, metadata?: Record | null) { + return this.log({ + type: 'request_suspicious', + metadata: metadata ?? null, + req, + }); + } + + static requestSuspiciousContext( + context: { ipAddress?: string | null; userAgent?: string | null }, + metadata?: Record | null, + ) { + return this.logContext({ + type: 'request_suspicious', + ipAddress: context.ipAddress, + userAgent: context.userAgent, + metadata: metadata ?? null, + }); + } } diff --git a/tests/e2e/authFlow.spec.ts b/tests/e2e/authFlow.spec.ts index cd55f1a..a6af42c 100644 --- a/tests/e2e/authFlow.spec.ts +++ b/tests/e2e/authFlow.spec.ts @@ -11,6 +11,9 @@ vi.mock('../../../src/services/authEventService.js', () => ({ AuthEventService: { log: vi.fn(), notificationSent: vi.fn(), + refreshTokenFailed: vi.fn(), + requestSuspicious: vi.fn(), + requestSuspiciousContext: vi.fn(), }, })); diff --git a/tests/integration/auth/cookieAuth.security.spec.ts b/tests/integration/auth/cookieAuth.security.spec.ts index 5acfb7d..a101858 100644 --- a/tests/integration/auth/cookieAuth.security.spec.ts +++ b/tests/integration/auth/cookieAuth.security.spec.ts @@ -141,7 +141,7 @@ describe('verifyCookieAuth security - silent refresh', () => { legacyFallbackCandidates: 0, usedLegacyFallback: false, }); - (AuthEventService.serviceTokenInvalid as any).mockResolvedValue(undefined); + (AuthEventService.refreshTokenFailed as any).mockResolvedValue(undefined); const middleware = verifyCookieAuth('access'); const { req, res, next } = mockReqRes({ @@ -150,7 +150,13 @@ describe('verifyCookieAuth security - silent refresh', () => { await middleware(req, res, next); - expect(AuthEventService.serviceTokenInvalid).toHaveBeenCalledWith(req); + expect(AuthEventService.refreshTokenFailed).toHaveBeenCalledWith( + req, + expect.objectContaining({ + reason: 'No matching session found for refresh token', + legacyFallbackCandidates: 0, + }), + ); expect(res.status).toHaveBeenCalledWith(401); expect(next).not.toHaveBeenCalled(); }); @@ -168,7 +174,7 @@ describe('verifyCookieAuth security - silent refresh', () => { usedLegacyFallback: false, }); (revokeSessionChain as any).mockResolvedValue(undefined); - (AuthEventService.serviceTokenInvalid as any).mockResolvedValue(undefined); + (AuthEventService.log as any).mockResolvedValue(undefined); const middleware = verifyCookieAuth('access'); const { req, res, next } = mockReqRes({ @@ -178,7 +184,12 @@ describe('verifyCookieAuth security - silent refresh', () => { await middleware(req, res, next); expect(revokeSessionChain).toHaveBeenCalledWith(reusedSession); - expect(AuthEventService.serviceTokenInvalid).toHaveBeenCalledWith(req); + expect(AuthEventService.log).toHaveBeenCalledWith( + expect.objectContaining({ + userId: 'user-1', + type: 'refresh_token_suspicious', + }), + ); expect(res.status).toHaveBeenCalledWith(401); expect(next).not.toHaveBeenCalled(); }); @@ -196,7 +207,7 @@ describe('verifyCookieAuth security - silent refresh', () => { usedLegacyFallback: false, }); (revokeSessionChain as any).mockResolvedValue(undefined); - (AuthEventService.serviceTokenInvalid as any).mockResolvedValue(undefined); + (AuthEventService.log as any).mockResolvedValue(undefined); const middleware = verifyCookieAuth('access'); const { req, res, next } = mockReqRes({ diff --git a/tests/integration/health/health.spec.ts b/tests/integration/health/health.spec.ts index 6b2eaca..193f681 100644 --- a/tests/integration/health/health.spec.ts +++ b/tests/integration/health/health.spec.ts @@ -1,14 +1,20 @@ import request from 'supertest'; import { createApp } from '../../../src/app'; -import { beforeAll, describe, expect, it } from 'vitest'; +import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; import { Application } from 'express'; +import { AuthEventService } from '../../../src/services/authEventService.js'; + let app: Application; beforeAll(async () => { app = await createApp(); }); +beforeEach(() => { + vi.clearAllMocks(); +}); + describe('Health Routes', () => { it('returns system status', async () => { const res = await request(app).get('/health/status'); @@ -21,5 +27,14 @@ describe('Health Routes', () => { const res = await request(app).get('/health/unknown'); expect(res.status).toBe(404); + expect(AuthEventService.requestSuspicious).toHaveBeenCalledWith( + expect.objectContaining({ + originalUrl: '/health/unknown', + }), + expect.objectContaining({ + reason: 'Request to an unknown route.', + path: '/health/unknown', + }), + ); }); }); diff --git a/tests/integration/registration/register.spec.ts b/tests/integration/registration/register.spec.ts index 0c8aac3..61d729a 100644 --- a/tests/integration/registration/register.spec.ts +++ b/tests/integration/registration/register.spec.ts @@ -25,6 +25,9 @@ vi.mock('../../../src/services/authEventService.js', () => ({ AuthEventService: { log: vi.fn(), notificationSent: vi.fn(), + refreshTokenFailed: vi.fn(), + requestSuspicious: vi.fn(), + requestSuspiciousContext: vi.fn(), }, })); diff --git a/tests/setup/mocks.ts b/tests/setup/mocks.ts index efac91d..371a66b 100644 --- a/tests/setup/mocks.ts +++ b/tests/setup/mocks.ts @@ -153,6 +153,9 @@ vi.mock('../../src/services/authEventService.js', () => ({ AuthEventService: { log: vi.fn(), notificationSent: vi.fn(), + refreshTokenFailed: vi.fn(), + requestSuspicious: vi.fn(), + requestSuspiciousContext: vi.fn(), serviceTokenInvalid: vi.fn(), loginSuccess: vi.fn(), }, diff --git a/tests/unit/controllers/authentication.spec.ts b/tests/unit/controllers/authentication.spec.ts index 39c5543..d00acd3 100644 --- a/tests/unit/controllers/authentication.spec.ts +++ b/tests/unit/controllers/authentication.spec.ts @@ -75,11 +75,10 @@ describe('refreshSession', () => { await refreshSession(req, res); - expect(AuthEventService.log).toHaveBeenCalledWith( + expect(AuthEventService.refreshTokenFailed).toHaveBeenCalledWith( + req, expect.objectContaining({ - userId: null, - type: 'refresh_token_failed', - metadata: { reason: 'Missing refresh token' }, + reason: 'Missing refresh token', }), ); expect(res.status).toHaveBeenCalledWith(401); @@ -96,12 +95,19 @@ describe('refreshSession', () => { legacyFallbackCandidates: 2, usedLegacyFallback: true, }); - (AuthEventService.serviceTokenInvalid as any).mockResolvedValue(undefined); + (AuthEventService.refreshTokenFailed as any).mockResolvedValue(undefined); await refreshSession(req, res); expect(findRefreshSessionByToken).toHaveBeenCalledWith('raw-refresh-token', expect.any(Date)); - expect(AuthEventService.serviceTokenInvalid).toHaveBeenCalledWith(req); + expect(AuthEventService.refreshTokenFailed).toHaveBeenCalledWith( + req, + expect.objectContaining({ + reason: 'No refresh session found for refresh token', + legacyFallbackCandidates: 2, + tokenFormat: 'opaque', + }), + ); expect(res.status).toHaveBeenCalledWith(401); expect(res.json).toHaveBeenCalledWith({ error: 'invalid_refresh_token' }); }); diff --git a/tests/unit/controllers/otp.spec.ts b/tests/unit/controllers/otp.spec.ts index 2bf2cb9..ed59952 100644 --- a/tests/unit/controllers/otp.spec.ts +++ b/tests/unit/controllers/otp.spec.ts @@ -29,6 +29,9 @@ vi.mock('../../../src/lib/token.js', () => ({ vi.mock('../../../src/services/authEventService.js', () => ({ AuthEventService: { log: authEventLogMock, + refreshTokenFailed: vi.fn(), + requestSuspicious: vi.fn(), + requestSuspiciousContext: vi.fn(), }, })); diff --git a/tests/unit/services/authEventService.spec.ts b/tests/unit/services/authEventService.spec.ts index b75b765..16c2a72 100644 --- a/tests/unit/services/authEventService.spec.ts +++ b/tests/unit/services/authEventService.spec.ts @@ -168,7 +168,12 @@ describe('AuthEventService', () => { await AuthEventService.notificationSent('user-1', req); - expect(spy).toHaveBeenCalled(); + expect(spy).toHaveBeenCalledWith({ + userId: 'user-1', + type: 'notification_sent', + req, + metadata: undefined, + }); }); it('serviceTokenUsed logs correct metadata', async () => { @@ -202,4 +207,41 @@ describe('AuthEventService', () => { req, }); }); + + it('refreshTokenFailed logs refresh failures', async () => { + const { AuthEventService } = await import('../../../src/services/authEventService.js'); + + const spy = vi.spyOn(AuthEventService, 'log'); + + const req = buildReq(); + + await AuthEventService.refreshTokenFailed(req, { reason: 'Missing refresh token' }); + + expect(spy).toHaveBeenCalledWith({ + type: 'refresh_token_failed', + metadata: { reason: 'Missing refresh token' }, + req, + }); + }); + + it('normalizes legacy event type aliases', async () => { + const { AuthEvent } = await import('../../../src/models/authEvents.js'); + const { AuthEventService } = await import('../../../src/services/authEventService.js'); + + const req = buildReq(); + + await AuthEventService.log({ + type: 'request_suspicous', + req, + metadata: { reason: 'legacy typo' }, + }); + + expect(AuthEvent.create).toHaveBeenCalledWith({ + user_id: null, + type: 'request_suspicious', + ip_address: '127.0.0.1', + user_agent: 'agent', + metadata: { reason: 'legacy typo' }, + }); + }); }); diff --git a/tests/unit/services/bootstrapPromotionService.spec.ts b/tests/unit/services/bootstrapPromotionService.spec.ts index b9a8b9a..a3ec442 100644 --- a/tests/unit/services/bootstrapPromotionService.spec.ts +++ b/tests/unit/services/bootstrapPromotionService.spec.ts @@ -28,6 +28,9 @@ vi.mock('../../../src/models/index.js', () => ({ vi.mock('../../../src/services/authEventService.js', () => ({ AuthEventService: { log: vi.fn(), + refreshTokenFailed: vi.fn(), + requestSuspicious: vi.fn(), + requestSuspiciousContext: vi.fn(), }, }));