fix(auth): lock accounts after failed attempts, one-time TOTP codes, stronger MFA changes
- Per-account lockout: five failed attempts (password, MFA code or a step-up confirmation) lock the account for 15 minutes, on top of the per-IP throttle - guesses spread over many addresses no longer help. - A TOTP code is accepted once: the time step is stored and that step or any earlier one is refused (also between two concurrent requests). - Login runs bcrypt on every path and answers unknown email, wrong password, deactivated and locked accounts with the same message; failed logins for unknown emails no longer write attacker-chosen text into the audit log. - Turning MFA off needs password and a current code; turning it on needs the password. Both end all other sessions. - New audit action ACCOUNT_LOCKED. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
1 parent
79d20ee64e
commit
5c666fe49a
22 files changed
+494
-91
No files matched your search
@@ -5,6 +5,7 @@ import { PrismaService } from '../prisma/prisma.service';
|
||||
import { UsersService } from '../users/users.service';
|
||||
import { AuditLogService } from '../audit/audit-log.service';
|
||||
import { MfaService } from './mfa/mfa.service';
|
||||
import { AccountGuardService } from './mfa/account-guard.service';
|
||||
|
||||
describe('AuthService', () => {
|
||||
let authService: AuthService;
|
||||
@@ -26,7 +27,12 @@ describe('AuthService', () => {
|
||||
let mfaService: {
|
||||
signChallengeToken: jest.Mock;
|
||||
verifyChallengeToken: jest.Mock;
|
||||
verifyCode: jest.Mock;
|
||||
};
|
||||
let accountGuard: {
|
||||
isLocked: jest.Mock;
|
||||
recordFailure: jest.Mock;
|
||||
recordSuccess: jest.Mock;
|
||||
consumeTotp: jest.Mock;
|
||||
};
|
||||
|
||||
const activeUser = {
|
||||
@@ -60,7 +66,12 @@ describe('AuthService', () => {
|
||||
mfaService = {
|
||||
signChallengeToken: jest.fn().mockReturnValue('mfa-challenge-token'),
|
||||
verifyChallengeToken: jest.fn(),
|
||||
verifyCode: jest.fn(),
|
||||
};
|
||||
accountGuard = {
|
||||
isLocked: jest.fn().mockReturnValue(false),
|
||||
recordFailure: jest.fn().mockResolvedValue(undefined),
|
||||
recordSuccess: jest.fn().mockResolvedValue(undefined),
|
||||
consumeTotp: jest.fn(),
|
||||
};
|
||||
|
||||
authService = new AuthService(
|
||||
@@ -70,6 +81,7 @@ describe('AuthService', () => {
|
||||
auditLog as unknown as AuditLogService,
|
||||
permissionResolver as never,
|
||||
mfaService as unknown as MfaService,
|
||||
accountGuard as unknown as AccountGuardService,
|
||||
);
|
||||
});
|
||||
|
||||
@@ -157,21 +169,40 @@ describe('AuthService', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('records a LOGIN_FAILED audit entry keyed by email when the user is unknown', async () => {
|
||||
it('writes nothing attacker-chosen to the audit log for an unknown email', async () => {
|
||||
usersService.findByEmail.mockResolvedValue(null);
|
||||
await expect(
|
||||
authService.login('[email protected]', 'x'),
|
||||
).rejects.toBeInstanceOf(UnauthorizedException);
|
||||
expect(auditLog.record).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
action: 'LOGIN_FAILED',
|
||||
entityId: '[email protected]',
|
||||
}),
|
||||
);
|
||||
const [[recordArg]] = auditLog.record.mock.calls as [
|
||||
[{ userId?: string }],
|
||||
];
|
||||
expect(recordArg).not.toHaveProperty('userId');
|
||||
expect(auditLog.record).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('counts a wrong password against the account', async () => {
|
||||
usersService.findByEmail.mockResolvedValue(activeUser);
|
||||
await expect(
|
||||
authService.login(activeUser.email, 'wrong-password'),
|
||||
).rejects.toBeInstanceOf(UnauthorizedException);
|
||||
expect(accountGuard.recordFailure).toHaveBeenCalledWith(activeUser.id);
|
||||
});
|
||||
|
||||
it('refuses a locked account even with the right password, with the same message', async () => {
|
||||
usersService.findByEmail.mockResolvedValue(activeUser);
|
||||
accountGuard.isLocked.mockReturnValue(true);
|
||||
|
||||
const lockedMessage = await authService
|
||||
.login(activeUser.email, 'correct-password')
|
||||
.then(() => 'no error')
|
||||
.catch((error: Error) => error.message);
|
||||
|
||||
accountGuard.isLocked.mockReturnValue(false);
|
||||
const wrongMessage = await authService
|
||||
.login(activeUser.email, 'wrong-password')
|
||||
.then(() => 'no error')
|
||||
.catch((error: Error) => error.message);
|
||||
|
||||
expect(lockedMessage).not.toBe('no error');
|
||||
expect(lockedMessage).toBe(wrongMessage);
|
||||
expect(prisma.refreshToken.create).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -185,7 +216,7 @@ describe('AuthService', () => {
|
||||
it('issues tokens and records LOGIN when the code is valid', async () => {
|
||||
mfaService.verifyChallengeToken.mockReturnValue({ userId: mfaUser.id });
|
||||
usersService.findById.mockResolvedValue(mfaUser);
|
||||
mfaService.verifyCode.mockResolvedValue(true);
|
||||
accountGuard.consumeTotp.mockResolvedValue(true);
|
||||
prisma.refreshToken.create.mockResolvedValue({});
|
||||
|
||||
const result = await authService.verifyMfaAndLogin(
|
||||
@@ -194,11 +225,8 @@ describe('AuthService', () => {
|
||||
);
|
||||
|
||||
expect(result.accessToken).toBe('signed.jwt.token');
|
||||
expect(mfaService.verifyCode).toHaveBeenCalledWith(
|
||||
mfaUser.mfaSecret,
|
||||
mfaUser.email,
|
||||
'123456',
|
||||
);
|
||||
expect(accountGuard.consumeTotp).toHaveBeenCalledWith(mfaUser, '123456');
|
||||
expect(accountGuard.recordSuccess).toHaveBeenCalledWith(mfaUser.id);
|
||||
expect(auditLog.record).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ action: 'LOGIN', userId: mfaUser.id }),
|
||||
);
|
||||
@@ -207,7 +235,7 @@ describe('AuthService', () => {
|
||||
it('throws and records LOGIN_FAILED when the code is invalid', async () => {
|
||||
mfaService.verifyChallengeToken.mockReturnValue({ userId: mfaUser.id });
|
||||
usersService.findById.mockResolvedValue(mfaUser);
|
||||
mfaService.verifyCode.mockResolvedValue(false);
|
||||
accountGuard.consumeTotp.mockResolvedValue(false);
|
||||
|
||||
await expect(
|
||||
authService.verifyMfaAndLogin('mfa-challenge-token', '000000'),
|
||||
|
||||
@@ -7,6 +7,7 @@ import { UsersService } from '../users/users.service';
|
||||
import { AuditLogService } from '../audit/audit-log.service';
|
||||
import { PermissionResolverService } from './permissions/permission-resolver.service';
|
||||
import { MfaService } from './mfa/mfa.service';
|
||||
import { AccountGuardService } from './mfa/account-guard.service';
|
||||
import { parseDurationToMs } from './util/duration';
|
||||
import { TOKEN_AUDIENCE, jwtSignOptions } from './util/jwt-claims';
|
||||
|
||||
@@ -32,6 +33,19 @@ export interface CurrentUserProfile {
|
||||
|
||||
const REFRESH_TOKEN_BYTES = 40;
|
||||
|
||||
// Same answer for unknown email, wrong password, deactivated or locked
|
||||
// account - nothing in the response tells which one it was.
|
||||
const INVALID_CREDENTIALS =
|
||||
'E-Mail oder Passwort falsch. Nach mehreren Fehlversuchen ist die Anmeldung für 15 Minuten gesperrt.';
|
||||
|
||||
// Hash of a random value nobody knows, at the same cost as real password
|
||||
// hashes; compared against when there is no real hash, so every login
|
||||
// costs the same time.
|
||||
const DUMMY_PASSWORD_HASH = bcrypt.hashSync(
|
||||
crypto.randomBytes(32).toString('hex'),
|
||||
12,
|
||||
);
|
||||
|
||||
@Injectable()
|
||||
export class AuthService {
|
||||
private readonly logger = new Logger(AuthService.name);
|
||||
@@ -43,6 +57,7 @@ export class AuthService {
|
||||
private readonly auditLog: AuditLogService,
|
||||
private readonly permissionResolver: PermissionResolverService,
|
||||
private readonly mfaService: MfaService,
|
||||
private readonly accountGuard: AccountGuardService,
|
||||
) {}
|
||||
|
||||
async getCurrentUserProfile(userId: string): Promise<CurrentUserProfile> {
|
||||
@@ -91,35 +106,36 @@ export class AuthService {
|
||||
|
||||
async login(email: string, password: string): Promise<LoginResult> {
|
||||
const user = await this.usersService.findByEmail(email);
|
||||
// bcrypt runs on every path - unknown, deactivated, locked or real - so
|
||||
// the response time does not reveal which accounts exist.
|
||||
const passwordMatches = await bcrypt.compare(
|
||||
password,
|
||||
user?.passwordHash ?? DUMMY_PASSWORD_HASH,
|
||||
);
|
||||
if (!user) {
|
||||
this.logger.warn(`Login failed: no user with email ${email}`);
|
||||
await this.auditLog.record({
|
||||
entityType: 'AuthSession',
|
||||
entityId: email,
|
||||
action: 'LOGIN_FAILED',
|
||||
});
|
||||
throw new UnauthorizedException('Invalid credentials');
|
||||
this.logger.warn('Login failed: unknown email');
|
||||
throw new UnauthorizedException(INVALID_CREDENTIALS);
|
||||
}
|
||||
if (!user.isActive) {
|
||||
this.logger.warn(`Login failed: user ${user.id} is deactivated`);
|
||||
if (!user.isActive || this.accountGuard.isLocked(user)) {
|
||||
this.logger.warn(`Login refused: user ${user.id} inactive or locked`);
|
||||
await this.auditLog.record({
|
||||
entityType: 'AuthSession',
|
||||
entityId: user.id,
|
||||
action: 'LOGIN_FAILED',
|
||||
userId: user.id,
|
||||
});
|
||||
throw new UnauthorizedException('Invalid credentials');
|
||||
throw new UnauthorizedException(INVALID_CREDENTIALS);
|
||||
}
|
||||
const passwordMatches = await bcrypt.compare(password, user.passwordHash);
|
||||
if (!passwordMatches) {
|
||||
this.logger.warn(`Login failed: wrong password for user ${user.id}`);
|
||||
await this.accountGuard.recordFailure(user.id);
|
||||
await this.auditLog.record({
|
||||
entityType: 'AuthSession',
|
||||
entityId: user.id,
|
||||
action: 'LOGIN_FAILED',
|
||||
userId: user.id,
|
||||
});
|
||||
throw new UnauthorizedException('Invalid credentials');
|
||||
throw new UnauthorizedException(INVALID_CREDENTIALS);
|
||||
}
|
||||
if (user.mfaEnabled) {
|
||||
return {
|
||||
@@ -133,6 +149,7 @@ export class AuthService {
|
||||
};
|
||||
const accessToken = this.signAccessToken(authenticatedUser);
|
||||
const refreshToken = await this.issueRefreshToken(user.id);
|
||||
await this.accountGuard.recordSuccess(user.id);
|
||||
await this.auditLog.record({
|
||||
entityType: 'AuthSession',
|
||||
entityId: user.id,
|
||||
@@ -180,15 +197,15 @@ export class AuthService {
|
||||
if (!user.isActive || !user.mfaEnabled || !user.mfaSecret) {
|
||||
throw new UnauthorizedException('MFA nicht verfügbar');
|
||||
}
|
||||
const isValid = await this.mfaService.verifyCode(
|
||||
user.mfaSecret,
|
||||
user.email,
|
||||
code,
|
||||
);
|
||||
if (this.accountGuard.isLocked(user)) {
|
||||
throw new UnauthorizedException(INVALID_CREDENTIALS);
|
||||
}
|
||||
const isValid = await this.accountGuard.consumeTotp(user, code);
|
||||
if (!isValid) {
|
||||
this.logger.warn(
|
||||
`MFA verification failed: wrong code for user ${user.id}`,
|
||||
`MFA verification failed: wrong or reused code for user ${user.id}`,
|
||||
);
|
||||
await this.accountGuard.recordFailure(user.id);
|
||||
await this.auditLog.record({
|
||||
entityType: 'AuthSession',
|
||||
entityId: user.id,
|
||||
@@ -197,6 +214,7 @@ export class AuthService {
|
||||
});
|
||||
throw new UnauthorizedException('Code ungültig');
|
||||
}
|
||||
await this.accountGuard.recordSuccess(user.id);
|
||||
const authenticatedUser = {
|
||||
id: user.id,
|
||||
email: user.email,
|
||||
|
||||
@@ -0,0 +1,97 @@
|
||||
import { Injectable, Logger } from '@nestjs/common';
|
||||
import { PrismaService } from '../../prisma/prisma.service';
|
||||
import { AuditLogService } from '../../audit/audit-log.service';
|
||||
import { MfaService } from './mfa.service';
|
||||
|
||||
/** After this many failed attempts in a row the account is locked ... */
|
||||
export const MAX_FAILED_AUTH_ATTEMPTS = 5;
|
||||
/** ... for this long. */
|
||||
export const AUTH_LOCK_MS = 15 * 60 * 1000;
|
||||
|
||||
export interface GuardedUser {
|
||||
id: string;
|
||||
email: string;
|
||||
mfaSecret: string | null;
|
||||
mfaLastTimeStep: number | null;
|
||||
authLockedUntil: Date | null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Account-level brute-force protection, on top of the per-IP throttle:
|
||||
* an attacker spreading guesses over many addresses still only gets five
|
||||
* tries per account per 15 minutes - for the password, the MFA code and
|
||||
* step-up confirmations alike. A TOTP code is accepted once: its time step
|
||||
* is recorded, and that step or any earlier one is refused afterwards.
|
||||
*/
|
||||
@Injectable()
|
||||
export class AccountGuardService {
|
||||
private readonly logger = new Logger(AccountGuardService.name);
|
||||
|
||||
constructor(
|
||||
private readonly prisma: PrismaService,
|
||||
private readonly mfaService: MfaService,
|
||||
private readonly auditLog: AuditLogService,
|
||||
) {}
|
||||
|
||||
isLocked(user: Pick<GuardedUser, 'authLockedUntil'>): boolean {
|
||||
return Boolean(user.authLockedUntil && user.authLockedUntil > new Date());
|
||||
}
|
||||
|
||||
async recordFailure(userId: string): Promise<void> {
|
||||
const updated = await this.prisma.user.update({
|
||||
where: { id: userId },
|
||||
data: { failedAuthAttempts: { increment: 1 } },
|
||||
select: { failedAuthAttempts: true },
|
||||
});
|
||||
if (updated.failedAuthAttempts >= MAX_FAILED_AUTH_ATTEMPTS) {
|
||||
await this.prisma.user.update({
|
||||
where: { id: userId },
|
||||
data: {
|
||||
authLockedUntil: new Date(Date.now() + AUTH_LOCK_MS),
|
||||
failedAuthAttempts: 0,
|
||||
},
|
||||
});
|
||||
this.logger.warn(`Account ${userId} locked after repeated failures`);
|
||||
await this.auditLog.record({
|
||||
entityType: 'AuthSession',
|
||||
entityId: userId,
|
||||
action: 'ACCOUNT_LOCKED',
|
||||
userId,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
async recordSuccess(userId: string): Promise<void> {
|
||||
await this.prisma.user.update({
|
||||
where: { id: userId },
|
||||
data: { failedAuthAttempts: 0, authLockedUntil: null },
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* True only for a valid code whose time step is newer than the last one
|
||||
* used - and only for one of two concurrent requests with the same code.
|
||||
*/
|
||||
async consumeTotp(user: GuardedUser, code: string): Promise<boolean> {
|
||||
if (!user.mfaSecret) {
|
||||
return false;
|
||||
}
|
||||
const timeStep = await this.mfaService.verifyCodeStep(
|
||||
user.mfaSecret,
|
||||
user.email,
|
||||
code,
|
||||
user.mfaLastTimeStep ?? undefined,
|
||||
);
|
||||
if (timeStep === null) {
|
||||
return false;
|
||||
}
|
||||
const claimed = await this.prisma.user.updateMany({
|
||||
where: {
|
||||
id: user.id,
|
||||
OR: [{ mfaLastTimeStep: null }, { mfaLastTimeStep: { lt: timeStep } }],
|
||||
},
|
||||
data: { mfaLastTimeStep: timeStep },
|
||||
});
|
||||
return claimed.count === 1;
|
||||
}
|
||||
}
|
||||
@@ -1,10 +1,11 @@
|
||||
import { Module } from '@nestjs/common';
|
||||
import { JwtModule } from '@nestjs/jwt';
|
||||
import { MfaService } from './mfa.service';
|
||||
import { AccountGuardService } from './account-guard.service';
|
||||
|
||||
@Module({
|
||||
imports: [JwtModule.register({})],
|
||||
providers: [MfaService],
|
||||
exports: [MfaService],
|
||||
providers: [MfaService, AccountGuardService],
|
||||
exports: [MfaService, AccountGuardService],
|
||||
})
|
||||
export class MfaModule {}
|
||||
@@ -2,7 +2,12 @@ import { Injectable, UnauthorizedException } from '@nestjs/common';
|
||||
import { JwtService } from '@nestjs/jwt';
|
||||
import * as QRCode from 'qrcode';
|
||||
import { decryptMfaSecret, encryptMfaSecret } from './mfa-crypto.util';
|
||||
import { buildTotpUri, generateTotpSecret, verifyTotpCode } from './totp.util';
|
||||
import {
|
||||
buildTotpUri,
|
||||
generateTotpSecret,
|
||||
verifyTotpCode,
|
||||
verifyTotpStep,
|
||||
} from './totp.util';
|
||||
import {
|
||||
TOKEN_AUDIENCE,
|
||||
jwtSignOptions,
|
||||
@@ -104,6 +109,21 @@ export class MfaService {
|
||||
return { userId: payload.sub };
|
||||
}
|
||||
|
||||
/** See verifyTotpStep; use AccountGuardService.consumeTotp for logins. */
|
||||
async verifyCodeStep(
|
||||
encryptedSecret: string,
|
||||
email: string,
|
||||
code: string,
|
||||
afterTimeStep?: number,
|
||||
): Promise<number | null> {
|
||||
return verifyTotpStep(
|
||||
decryptMfaSecret(encryptedSecret),
|
||||
email,
|
||||
code,
|
||||
afterTimeStep,
|
||||
);
|
||||
}
|
||||
|
||||
async verifyCode(
|
||||
encryptedSecret: string,
|
||||
email: string,
|
||||
|
||||
@@ -20,6 +20,23 @@ export function buildTotpUri(secret: string, label: string): string {
|
||||
return createTotp(secret, label).toURI();
|
||||
}
|
||||
|
||||
/**
|
||||
* The time step a code belongs to, or null if it is wrong - or not newer
|
||||
* than afterTimeStep (a code that was already used, or an older one).
|
||||
*/
|
||||
export async function verifyTotpStep(
|
||||
secret: string,
|
||||
label: string,
|
||||
code: string,
|
||||
afterTimeStep?: number,
|
||||
): Promise<number | null> {
|
||||
const result = await createTotp(secret, label).verify(code, {
|
||||
epochTolerance: EPOCH_TOLERANCE_SECONDS,
|
||||
afterTimeStep,
|
||||
});
|
||||
return result.valid ? result.timeStep : null;
|
||||
}
|
||||
|
||||
export async function verifyTotpCode(
|
||||
secret: string,
|
||||
label: string,
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { Injectable, UnauthorizedException } from '@nestjs/common';
|
||||
import * as bcrypt from 'bcryptjs';
|
||||
import { PrismaService } from '../../prisma/prisma.service';
|
||||
import { MfaService } from '../mfa/mfa.service';
|
||||
import { AccountGuardService } from '../mfa/account-guard.service';
|
||||
|
||||
export interface ReauthProof {
|
||||
password?: string;
|
||||
@@ -18,25 +18,33 @@ export interface ReauthProof {
|
||||
export class ReauthService {
|
||||
constructor(
|
||||
private readonly prisma: PrismaService,
|
||||
private readonly mfaService: MfaService,
|
||||
private readonly accountGuard: AccountGuardService,
|
||||
) {}
|
||||
|
||||
async verify(userId: string, proof: ReauthProof): Promise<void> {
|
||||
const user = await this.prisma.user.findUnique({
|
||||
where: { id: userId },
|
||||
select: {
|
||||
id: true,
|
||||
email: true,
|
||||
passwordHash: true,
|
||||
mfaEnabled: true,
|
||||
mfaSecret: true,
|
||||
mfaLastTimeStep: true,
|
||||
authLockedUntil: true,
|
||||
isActive: true,
|
||||
},
|
||||
});
|
||||
if (!user || !user.isActive || this.accountGuard.isLocked(user)) {
|
||||
throw new UnauthorizedException(
|
||||
'Bestätigung derzeit nicht möglich. Nach mehreren Fehlversuchen ist sie für 15 Minuten gesperrt.',
|
||||
);
|
||||
}
|
||||
const passwordMatches =
|
||||
Boolean(user?.isActive) &&
|
||||
typeof proof.password === 'string' &&
|
||||
(await bcrypt.compare(proof.password, user!.passwordHash));
|
||||
if (!user || !passwordMatches) {
|
||||
(await bcrypt.compare(proof.password, user.passwordHash));
|
||||
if (!passwordMatches) {
|
||||
await this.accountGuard.recordFailure(user.id);
|
||||
throw new UnauthorizedException(
|
||||
'Bitte zur Bestätigung Ihr aktuelles Passwort eingeben',
|
||||
);
|
||||
@@ -44,17 +52,14 @@ export class ReauthService {
|
||||
if (user.mfaEnabled) {
|
||||
const codeMatches =
|
||||
typeof proof.mfaCode === 'string' &&
|
||||
user.mfaSecret !== null &&
|
||||
(await this.mfaService.verifyCode(
|
||||
user.mfaSecret,
|
||||
user.email,
|
||||
proof.mfaCode,
|
||||
));
|
||||
(await this.accountGuard.consumeTotp(user, proof.mfaCode));
|
||||
if (!codeMatches) {
|
||||
await this.accountGuard.recordFailure(user.id);
|
||||
throw new UnauthorizedException(
|
||||
'Bitte zur Bestätigung den aktuellen Code aus Ihrer Authenticator-App eingeben',
|
||||
);
|
||||
}
|
||||
}
|
||||
await this.accountGuard.recordSuccess(user.id);
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user