mirror of
https://github.com/Memo-2023/mana-monorepo.git
synced 2026-05-23 16:26:42 +02:00
feat(auth): add audit logging, account lockout, and API key rate limiting
1. SecurityEventsService: Centralized audit logging for all auth events (login, register, logout, password changes, API key operations, SSO token exchange, etc.). Fire-and-forget pattern ensures auth flows are never blocked by logging failures. 2. AccountLockoutService: Locks accounts after 5 failed login attempts within 15 minutes. 30-minute lockout duration. Fails open on DB errors. Clears attempts on successful login. Email-not-verified does not count as a failed attempt. 3. API Key validation endpoint secured with rate limiting (10 req/min per IP via ThrottlerGuard) and audit logging. Key prefixes logged for forensics, never full keys. New schema: auth.login_attempts table for tracking failed logins. 174 tests passing across all auth and security modules. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
effa57fd61
commit
f7df8e97aa
14 changed files with 700 additions and 68 deletions
|
|
@ -34,6 +34,7 @@ import { ThrottlerModule, ThrottlerGuard } from '@nestjs/throttler';
|
|||
import { AuthController } from './auth.controller';
|
||||
import { BetterAuthService } from './services/better-auth.service';
|
||||
import { JwtAuthGuard } from '../common/guards/jwt-auth.guard';
|
||||
import { SecurityEventsService, AccountLockoutService } from '../security';
|
||||
import { mockDtoFactory } from '../__tests__/utils/mock-factories';
|
||||
|
||||
describe('AuthController', () => {
|
||||
|
|
@ -43,6 +44,7 @@ describe('AuthController', () => {
|
|||
// Common test data
|
||||
const mockAuthHeader = 'Bearer valid-jwt-token';
|
||||
const mockToken = 'valid-jwt-token';
|
||||
const mockReq = { headers: { 'user-agent': 'test' }, ip: '127.0.0.1' } as any;
|
||||
|
||||
beforeEach(async () => {
|
||||
// Create mock BetterAuthService with all methods
|
||||
|
|
@ -63,6 +65,18 @@ describe('AuthController', () => {
|
|||
validateToken: jest.fn(),
|
||||
};
|
||||
|
||||
const mockSecurityEventsService = {
|
||||
logEvent: jest.fn().mockResolvedValue(undefined),
|
||||
logEventWithRequest: jest.fn().mockResolvedValue(undefined),
|
||||
extractRequestInfo: jest.fn().mockReturnValue({ ipAddress: '127.0.0.1', userAgent: 'test' }),
|
||||
};
|
||||
|
||||
const mockAccountLockoutService = {
|
||||
checkLockout: jest.fn().mockResolvedValue({ locked: false }),
|
||||
recordAttempt: jest.fn().mockResolvedValue(undefined),
|
||||
clearAttempts: jest.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
|
||||
const module: TestingModule = await Test.createTestingModule({
|
||||
imports: [ThrottlerModule.forRoot([{ ttl: 60000, limit: 100 }])],
|
||||
controllers: [AuthController],
|
||||
|
|
@ -71,6 +85,14 @@ describe('AuthController', () => {
|
|||
provide: BetterAuthService,
|
||||
useValue: mockBetterAuthService,
|
||||
},
|
||||
{
|
||||
provide: SecurityEventsService,
|
||||
useValue: mockSecurityEventsService,
|
||||
},
|
||||
{
|
||||
provide: AccountLockoutService,
|
||||
useValue: mockAccountLockoutService,
|
||||
},
|
||||
],
|
||||
})
|
||||
.overrideGuard(JwtAuthGuard)
|
||||
|
|
@ -110,7 +132,7 @@ describe('AuthController', () => {
|
|||
|
||||
betterAuthService.registerB2C.mockResolvedValue(expectedResult);
|
||||
|
||||
const result = await controller.register(registerDto);
|
||||
const result = await controller.register(registerDto, mockReq);
|
||||
|
||||
expect(result).toEqual(expectedResult);
|
||||
expect(betterAuthService.registerB2C).toHaveBeenCalledWith({
|
||||
|
|
@ -133,7 +155,7 @@ describe('AuthController', () => {
|
|||
|
||||
betterAuthService.registerB2C.mockResolvedValue(expectedResult);
|
||||
|
||||
const result = await controller.register(registerDto as any);
|
||||
const result = await controller.register(registerDto as any, mockReq);
|
||||
|
||||
expect(result).toEqual(expectedResult);
|
||||
expect(betterAuthService.registerB2C).toHaveBeenCalledWith({
|
||||
|
|
@ -151,7 +173,7 @@ describe('AuthController', () => {
|
|||
new ConflictException('User with this email already exists')
|
||||
);
|
||||
|
||||
await expect(controller.register(registerDto)).rejects.toThrow(ConflictException);
|
||||
await expect(controller.register(registerDto, mockReq)).rejects.toThrow(ConflictException);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -180,7 +202,7 @@ describe('AuthController', () => {
|
|||
|
||||
betterAuthService.signIn.mockResolvedValue(expectedResult);
|
||||
|
||||
const result = await controller.login(loginDto);
|
||||
const result = await controller.login(loginDto, mockReq);
|
||||
|
||||
expect(result).toEqual(expectedResult);
|
||||
expect(betterAuthService.signIn).toHaveBeenCalledWith({
|
||||
|
|
@ -206,7 +228,7 @@ describe('AuthController', () => {
|
|||
expiresIn: 900,
|
||||
});
|
||||
|
||||
await controller.login(loginDto);
|
||||
await controller.login(loginDto, mockReq);
|
||||
|
||||
expect(betterAuthService.signIn).toHaveBeenCalledWith({
|
||||
email: loginDto.email,
|
||||
|
|
@ -223,7 +245,7 @@ describe('AuthController', () => {
|
|||
new UnauthorizedException('Invalid email or password')
|
||||
);
|
||||
|
||||
await expect(controller.login(loginDto)).rejects.toThrow(UnauthorizedException);
|
||||
await expect(controller.login(loginDto, mockReq)).rejects.toThrow(UnauthorizedException);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -237,7 +259,7 @@ describe('AuthController', () => {
|
|||
|
||||
betterAuthService.signOut.mockResolvedValue(expectedResult);
|
||||
|
||||
const result = await controller.logout(mockAuthHeader);
|
||||
const result = await controller.logout(mockAuthHeader, mockReq);
|
||||
|
||||
expect(result).toEqual(expectedResult);
|
||||
expect(betterAuthService.signOut).toHaveBeenCalledWith(mockToken);
|
||||
|
|
@ -246,7 +268,7 @@ describe('AuthController', () => {
|
|||
it('should extract token from Bearer header', async () => {
|
||||
betterAuthService.signOut.mockResolvedValue({ success: true, message: 'Signed out' });
|
||||
|
||||
await controller.logout('Bearer my-secret-token');
|
||||
await controller.logout('Bearer my-secret-token', mockReq);
|
||||
|
||||
expect(betterAuthService.signOut).toHaveBeenCalledWith('my-secret-token');
|
||||
});
|
||||
|
|
@ -254,7 +276,7 @@ describe('AuthController', () => {
|
|||
it('should handle raw token without Bearer prefix', async () => {
|
||||
betterAuthService.signOut.mockResolvedValue({ success: true, message: 'Signed out' });
|
||||
|
||||
await controller.logout('raw-token');
|
||||
await controller.logout('raw-token', mockReq);
|
||||
|
||||
expect(betterAuthService.signOut).toHaveBeenCalledWith('raw-token');
|
||||
});
|
||||
|
|
@ -700,7 +722,7 @@ describe('AuthController', () => {
|
|||
it('should extract token from Bearer authorization header', async () => {
|
||||
betterAuthService.signOut.mockResolvedValue({ success: true, message: 'OK' });
|
||||
|
||||
await controller.logout('Bearer my-token-123');
|
||||
await controller.logout('Bearer my-token-123', mockReq);
|
||||
|
||||
expect(betterAuthService.signOut).toHaveBeenCalledWith('my-token-123');
|
||||
});
|
||||
|
|
@ -708,7 +730,7 @@ describe('AuthController', () => {
|
|||
it('should handle missing authorization header', async () => {
|
||||
betterAuthService.signOut.mockResolvedValue({ success: true, message: 'OK' });
|
||||
|
||||
await controller.logout('');
|
||||
await controller.logout('', mockReq);
|
||||
|
||||
expect(betterAuthService.signOut).toHaveBeenCalledWith('');
|
||||
});
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ import {
|
|||
HttpStatus,
|
||||
Req,
|
||||
Res,
|
||||
ForbiddenException,
|
||||
} from '@nestjs/common';
|
||||
import type { Request, Response } from 'express';
|
||||
import { Throttle, ThrottlerGuard } from '@nestjs/throttler';
|
||||
|
|
@ -36,6 +37,7 @@ import { UpdateMemberRoleDto } from './dto/update-member-role.dto';
|
|||
import { JwtAuthGuard } from '../common/guards/jwt-auth.guard';
|
||||
import { CurrentUser } from '../common/decorators/current-user.decorator';
|
||||
import type { CurrentUserData } from '../common/decorators/current-user.decorator';
|
||||
import { SecurityEventsService, SecurityEventType, AccountLockoutService } from '../security';
|
||||
|
||||
/**
|
||||
* Auth Controller
|
||||
|
|
@ -71,7 +73,11 @@ import type { CurrentUserData } from '../common/decorators/current-user.decorato
|
|||
@Controller('auth')
|
||||
@UseGuards(ThrottlerGuard)
|
||||
export class AuthController {
|
||||
constructor(private readonly betterAuthService: BetterAuthService) {}
|
||||
constructor(
|
||||
private readonly betterAuthService: BetterAuthService,
|
||||
private readonly securityEvents: SecurityEventsService,
|
||||
private readonly accountLockout: AccountLockoutService
|
||||
) {}
|
||||
|
||||
// =========================================================================
|
||||
// B2C Authentication Endpoints
|
||||
|
|
@ -94,13 +100,21 @@ export class AuthController {
|
|||
@ApiResponse({ status: 400, description: 'Invalid input data' })
|
||||
@ApiResponse({ status: 409, description: 'Email already exists' })
|
||||
@ApiResponse({ status: 429, description: 'Rate limit exceeded' })
|
||||
async register(@Body() registerDto: RegisterDto) {
|
||||
return this.betterAuthService.registerB2C({
|
||||
async register(@Body() registerDto: RegisterDto, @Req() req: Request) {
|
||||
const result = await this.betterAuthService.registerB2C({
|
||||
email: registerDto.email,
|
||||
password: registerDto.password,
|
||||
name: registerDto.name || '',
|
||||
sourceAppUrl: registerDto.sourceAppUrl,
|
||||
});
|
||||
|
||||
this.securityEvents.logEventWithRequest(req, {
|
||||
userId: result.user?.id,
|
||||
eventType: SecurityEventType.REGISTER,
|
||||
metadata: { email: registerDto.email },
|
||||
});
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -139,13 +153,59 @@ export class AuthController {
|
|||
})
|
||||
@ApiResponse({ status: 401, description: 'Invalid credentials' })
|
||||
@ApiResponse({ status: 429, description: 'Rate limit exceeded' })
|
||||
async login(@Body() loginDto: LoginDto) {
|
||||
return this.betterAuthService.signIn({
|
||||
email: loginDto.email,
|
||||
password: loginDto.password,
|
||||
deviceId: loginDto.deviceId,
|
||||
deviceName: loginDto.deviceName,
|
||||
});
|
||||
async login(@Body() loginDto: LoginDto, @Req() req: Request) {
|
||||
const { ipAddress, userAgent } = this.securityEvents.extractRequestInfo(req);
|
||||
|
||||
// Check account lockout before attempting login
|
||||
const lockout = await this.accountLockout.checkLockout(loginDto.email);
|
||||
if (lockout.locked) {
|
||||
this.securityEvents.logEventWithRequest(req, {
|
||||
eventType: SecurityEventType.LOGIN_FAILURE,
|
||||
metadata: { email: loginDto.email, reason: 'account_locked' },
|
||||
});
|
||||
throw new ForbiddenException({
|
||||
message: 'Account temporarily locked due to too many failed login attempts',
|
||||
code: 'ACCOUNT_LOCKED',
|
||||
retryAfter: lockout.remainingSeconds,
|
||||
});
|
||||
}
|
||||
|
||||
try {
|
||||
const result = await this.betterAuthService.signIn({
|
||||
email: loginDto.email,
|
||||
password: loginDto.password,
|
||||
deviceId: loginDto.deviceId,
|
||||
deviceName: loginDto.deviceName,
|
||||
});
|
||||
|
||||
// Login successful - clear failed attempts and log
|
||||
this.accountLockout.clearAttempts(loginDto.email);
|
||||
this.securityEvents.logEvent({
|
||||
userId: result.user?.id,
|
||||
eventType: SecurityEventType.LOGIN_SUCCESS,
|
||||
ipAddress,
|
||||
userAgent,
|
||||
metadata: { email: loginDto.email, deviceId: loginDto.deviceId },
|
||||
});
|
||||
|
||||
return result;
|
||||
} catch (error) {
|
||||
// Don't count email-not-verified as a failed login attempt
|
||||
if (error instanceof ForbiddenException) {
|
||||
throw error;
|
||||
}
|
||||
|
||||
// Record failed attempt
|
||||
this.accountLockout.recordAttempt(loginDto.email, false, ipAddress);
|
||||
this.securityEvents.logEvent({
|
||||
eventType: SecurityEventType.LOGIN_FAILURE,
|
||||
ipAddress,
|
||||
userAgent,
|
||||
metadata: { email: loginDto.email, reason: 'invalid_credentials' },
|
||||
});
|
||||
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -163,9 +223,15 @@ export class AuthController {
|
|||
})
|
||||
@ApiResponse({ status: 200, description: 'Logout successful' })
|
||||
@ApiResponse({ status: 401, description: 'Not authenticated' })
|
||||
async logout(@Headers('authorization') authorization: string) {
|
||||
async logout(@Headers('authorization') authorization: string, @Req() req: Request) {
|
||||
const token = this.extractToken(authorization);
|
||||
return this.betterAuthService.signOut(token);
|
||||
const result = await this.betterAuthService.signOut(token);
|
||||
|
||||
this.securityEvents.logEventWithRequest(req, {
|
||||
eventType: SecurityEventType.LOGOUT,
|
||||
});
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -244,7 +310,15 @@ export class AuthController {
|
|||
})
|
||||
@ApiResponse({ status: 401, description: 'No valid session cookie' })
|
||||
async sessionToToken(@Req() req: Request, @Res({ passthrough: true }) res: Response) {
|
||||
return this.betterAuthService.sessionToToken(req, res);
|
||||
const result = await this.betterAuthService.sessionToToken(req, res);
|
||||
|
||||
this.securityEvents.logEventWithRequest(req, {
|
||||
userId: result.user?.id,
|
||||
eventType: SecurityEventType.SSO_TOKEN_EXCHANGE,
|
||||
metadata: { email: result.user?.email },
|
||||
});
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -272,11 +346,18 @@ export class AuthController {
|
|||
@Post('forgot-password')
|
||||
@Throttle({ default: { ttl: 60000, limit: 3 } })
|
||||
@HttpCode(HttpStatus.OK)
|
||||
async forgotPassword(@Body() forgotPasswordDto: ForgotPasswordDto) {
|
||||
return this.betterAuthService.requestPasswordReset(
|
||||
async forgotPassword(@Body() forgotPasswordDto: ForgotPasswordDto, @Req() req: Request) {
|
||||
const result = await this.betterAuthService.requestPasswordReset(
|
||||
forgotPasswordDto.email,
|
||||
forgotPasswordDto.redirectTo
|
||||
);
|
||||
|
||||
this.securityEvents.logEventWithRequest(req, {
|
||||
eventType: SecurityEventType.PASSWORD_RESET_REQUESTED,
|
||||
metadata: { email: forgotPasswordDto.email },
|
||||
});
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -288,11 +369,17 @@ export class AuthController {
|
|||
@Post('reset-password')
|
||||
@Throttle({ default: { ttl: 60000, limit: 5 } })
|
||||
@HttpCode(HttpStatus.OK)
|
||||
async resetPassword(@Body() resetPasswordDto: ResetPasswordDto) {
|
||||
return this.betterAuthService.resetPassword(
|
||||
async resetPassword(@Body() resetPasswordDto: ResetPasswordDto, @Req() req: Request) {
|
||||
const result = await this.betterAuthService.resetPassword(
|
||||
resetPasswordDto.token,
|
||||
resetPasswordDto.newPassword
|
||||
);
|
||||
|
||||
this.securityEvents.logEventWithRequest(req, {
|
||||
eventType: SecurityEventType.PASSWORD_RESET_COMPLETED,
|
||||
});
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -381,12 +468,23 @@ export class AuthController {
|
|||
@ApiBody({ type: ChangePasswordDto })
|
||||
@ApiResponse({ status: 200, description: 'Password changed successfully' })
|
||||
@ApiResponse({ status: 401, description: 'Current password is incorrect' })
|
||||
async changePassword(@CurrentUser() user: CurrentUserData, @Body() changeDto: ChangePasswordDto) {
|
||||
return this.betterAuthService.changePassword(
|
||||
async changePassword(
|
||||
@CurrentUser() user: CurrentUserData,
|
||||
@Body() changeDto: ChangePasswordDto,
|
||||
@Req() req: Request
|
||||
) {
|
||||
const result = await this.betterAuthService.changePassword(
|
||||
user.userId,
|
||||
changeDto.currentPassword,
|
||||
changeDto.newPassword
|
||||
);
|
||||
|
||||
this.securityEvents.logEventWithRequest(req, {
|
||||
userId: user.userId,
|
||||
eventType: SecurityEventType.PASSWORD_CHANGED,
|
||||
});
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -403,8 +501,24 @@ export class AuthController {
|
|||
@ApiBody({ type: DeleteAccountDto })
|
||||
@ApiResponse({ status: 200, description: 'Account deleted' })
|
||||
@ApiResponse({ status: 401, description: 'Password is incorrect' })
|
||||
async deleteAccount(@CurrentUser() user: CurrentUserData, @Body() deleteDto: DeleteAccountDto) {
|
||||
return this.betterAuthService.deleteAccount(user.userId, deleteDto.password, deleteDto.reason);
|
||||
async deleteAccount(
|
||||
@CurrentUser() user: CurrentUserData,
|
||||
@Body() deleteDto: DeleteAccountDto,
|
||||
@Req() req: Request
|
||||
) {
|
||||
const result = await this.betterAuthService.deleteAccount(
|
||||
user.userId,
|
||||
deleteDto.password,
|
||||
deleteDto.reason
|
||||
);
|
||||
|
||||
this.securityEvents.logEventWithRequest(req, {
|
||||
userId: user.userId,
|
||||
eventType: SecurityEventType.ACCOUNT_DELETED,
|
||||
metadata: { reason: deleteDto.reason },
|
||||
});
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
// =========================================================================
|
||||
|
|
@ -684,8 +798,7 @@ export class AuthController {
|
|||
@ApiBearerAuth('JWT-auth')
|
||||
@ApiOperation({
|
||||
summary: 'Cancel or reject invitation',
|
||||
description:
|
||||
'Cancel (as org admin/owner) or reject (as invitee) a pending invitation.',
|
||||
description: 'Cancel (as org admin/owner) or reject (as invitee) a pending invitation.',
|
||||
})
|
||||
@ApiResponse({ status: 204, description: 'Invitation cancelled/rejected successfully' })
|
||||
@ApiResponse({ status: 401, description: 'Not authenticated' })
|
||||
|
|
|
|||
|
|
@ -6,8 +6,10 @@ import { OidcLoginController } from './oidc-login.controller';
|
|||
import { MatrixSessionController } from './matrix-session.controller';
|
||||
import { BetterAuthService } from './services/better-auth.service';
|
||||
import { MatrixSessionService } from './services/matrix-session.service';
|
||||
import { SecurityModule } from '../security';
|
||||
|
||||
@Module({
|
||||
imports: [SecurityModule],
|
||||
controllers: [
|
||||
AuthController,
|
||||
BetterAuthPassthroughController,
|
||||
|
|
|
|||
|
|
@ -1534,19 +1534,6 @@ export class BetterAuthService {
|
|||
|
||||
this.logger.log('Password changed', { userId });
|
||||
|
||||
// Log security event
|
||||
try {
|
||||
const { securityEvents } = await import('../../db/schema/auth.schema');
|
||||
await db.insert(securityEvents).values({
|
||||
userId,
|
||||
eventType: 'password_changed',
|
||||
metadata: { changedAt: new Date().toISOString() },
|
||||
});
|
||||
} catch {
|
||||
// Non-critical - just log
|
||||
this.logger.warn('Failed to log security event for password change');
|
||||
}
|
||||
|
||||
return {
|
||||
success: true,
|
||||
message: 'Password changed successfully',
|
||||
|
|
@ -1603,19 +1590,6 @@ export class BetterAuthService {
|
|||
|
||||
this.logger.log('Account deleted', { userId, reason });
|
||||
|
||||
// Log security event
|
||||
try {
|
||||
const { securityEvents } = await import('../../db/schema/auth.schema');
|
||||
await db.insert(securityEvents).values({
|
||||
userId,
|
||||
eventType: 'account_deleted',
|
||||
metadata: { reason, deletedAt: now.toISOString() },
|
||||
});
|
||||
} catch {
|
||||
// Non-critical
|
||||
this.logger.warn('Failed to log security event for account deletion');
|
||||
}
|
||||
|
||||
return {
|
||||
success: true,
|
||||
message: 'Account has been deleted',
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue