feat(auth): 12-character minimum, 72-byte maximum and breach check for new passwords
- New passwords (create user, admin reset, own change) need at least 12 characters (ASVS 2.1.1) and may not exceed 72 bytes, which bcrypt would silently cut off. - They are checked against Have I Been Pwned with k-anonymity (only the first 5 SHA-1 characters leave the server, padded responses); an unreachable service is logged and does not block. PASSWORD_BREACH_CHECK =false disables it for offline deployments. - Login accepts existing passwords as before, capped at 256 characters to bound anonymous bcrypt work. - Password fields in the UI say the rule up front. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
1 parent
61e32474e4
commit
0ba8996e3a
13 files changed
+212
-17
No files matched your search
@@ -11,6 +11,10 @@ JWT_ACCESS_EXPIRES_IN="15m"
|
||||
JWT_REFRESH_EXPIRES_IN="7d"
|
||||
# Hard limit from login, however often the session is refreshed.
|
||||
SESSION_MAX_AGE="7d"
|
||||
# New passwords are checked against known breaches (Have I Been Pwned,
|
||||
# k-anonymity: only 5 hash characters leave the server). "false" turns the
|
||||
# check off for deployments without internet access.
|
||||
# PASSWORD_BREACH_CHECK=true
|
||||
|
||||
PORT=3001
|
||||
FRONTEND_ORIGIN="http://localhost:3000"
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { ApiProperty } from '@nestjs/swagger';
|
||||
import { IsEmail, IsString, MinLength } from 'class-validator';
|
||||
import { IsEmail, IsString, MaxLength, MinLength } from 'class-validator';
|
||||
import { NormalizeEmail } from '../../common/validation/normalize-email';
|
||||
|
||||
export class LoginDto {
|
||||
@@ -11,5 +11,8 @@ export class LoginDto {
|
||||
@ApiProperty()
|
||||
@IsString()
|
||||
@MinLength(8)
|
||||
// Long enough for any real password; bounds the bcrypt work an
|
||||
// anonymous request can cause.
|
||||
@MaxLength(256)
|
||||
password!: string;
|
||||
}
|
||||
@@ -8,7 +8,12 @@ describe('requestLoggerMiddleware', () => {
|
||||
});
|
||||
|
||||
function fakeRequest(): Request {
|
||||
return { method: 'GET', originalUrl: '/api/v1/dashboard' } as Request;
|
||||
return {
|
||||
method: 'GET',
|
||||
baseUrl: '',
|
||||
path: '/api/v1/dashboard',
|
||||
originalUrl: '/api/v1/dashboard?q=secret',
|
||||
} as Request;
|
||||
}
|
||||
|
||||
function fakeResponse(statusCode: number): {
|
||||
|
||||
@@ -0,0 +1,76 @@
|
||||
import { BadRequestException } from '@nestjs/common';
|
||||
import { plainToInstance } from 'class-transformer';
|
||||
import { validateSync } from 'class-validator';
|
||||
import * as crypto from 'node:crypto';
|
||||
import { IsNewPassword, assertPasswordNotBreached } from './password-policy';
|
||||
|
||||
class Dto {
|
||||
@IsNewPassword()
|
||||
password!: string;
|
||||
}
|
||||
|
||||
function errorsFor(password: string): number {
|
||||
return validateSync(plainToInstance(Dto, { password })).length;
|
||||
}
|
||||
|
||||
describe('password policy', () => {
|
||||
it('requires 12 characters and at most 72 bytes', () => {
|
||||
expect(errorsFor('kurz123!')).toBe(1);
|
||||
expect(errorsFor('lang-genug-12')).toBe(0);
|
||||
expect(errorsFor('a'.repeat(72))).toBe(0);
|
||||
expect(errorsFor('a'.repeat(73))).toBe(1);
|
||||
expect(errorsFor('ä'.repeat(37))).toBe(1); // 74 bytes
|
||||
});
|
||||
|
||||
describe('assertPasswordNotBreached', () => {
|
||||
const originalEnv = process.env.NODE_ENV;
|
||||
let fetchMock: jest.SpiedFunction<typeof fetch>;
|
||||
|
||||
beforeEach(() => {
|
||||
process.env.NODE_ENV = 'production';
|
||||
fetchMock = jest.spyOn(global, 'fetch');
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
process.env.NODE_ENV = originalEnv;
|
||||
fetchMock.mockRestore();
|
||||
});
|
||||
|
||||
function rangeFor(password: string, count: number): string {
|
||||
const sha1 = crypto
|
||||
.createHash('sha1')
|
||||
.update(password)
|
||||
.digest('hex')
|
||||
.toUpperCase();
|
||||
return `0000000000000000000000000000000000A:0\r\n${sha1.slice(5)}:${count}\r\n`;
|
||||
}
|
||||
|
||||
it('sends only the first five hash characters', async () => {
|
||||
fetchMock.mockResolvedValue(new Response(rangeFor('other', 3)));
|
||||
await assertPasswordNotBreached('Pferdebatterie-Heftklammer');
|
||||
const [url] = fetchMock.mock.calls[0] as [string];
|
||||
expect(url).toMatch(/\/range\/[0-9A-F]{5}$/);
|
||||
});
|
||||
|
||||
it('refuses a breached password', async () => {
|
||||
fetchMock.mockResolvedValue(new Response(rangeFor('Passwort1234', 4242)));
|
||||
await expect(
|
||||
assertPasswordNotBreached('Passwort1234'),
|
||||
).rejects.toBeInstanceOf(BadRequestException);
|
||||
});
|
||||
|
||||
it('ignores padding entries with a count of 0', async () => {
|
||||
fetchMock.mockResolvedValue(new Response(rangeFor('Padding-Only-1', 0)));
|
||||
await expect(
|
||||
assertPasswordNotBreached('Padding-Only-1'),
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it('accepts the password when the service is unreachable', async () => {
|
||||
fetchMock.mockRejectedValue(new Error('offline'));
|
||||
await expect(
|
||||
assertPasswordNotBreached('Irgendwas-123'),
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,95 @@
|
||||
import { BadRequestException, Logger } from '@nestjs/common';
|
||||
import { applyDecorators } from '@nestjs/common';
|
||||
import { ApiProperty } from '@nestjs/swagger';
|
||||
import { IsString, MinLength, ValidateBy, buildMessage } from 'class-validator';
|
||||
import * as crypto from 'node:crypto';
|
||||
|
||||
/** OWASP ASVS 2.1.1: at least 12 characters for a new password. */
|
||||
export const MIN_PASSWORD_LENGTH = 12;
|
||||
/** bcrypt ignores everything after 72 bytes - refuse instead of truncating. */
|
||||
export const MAX_PASSWORD_BYTES = 72;
|
||||
|
||||
const PWNED_RANGE_URL = 'https://api.pwnedpasswords.com/range/';
|
||||
const BREACH_CHECK_TIMEOUT_MS = 3_000;
|
||||
const logger = new Logger('PasswordPolicy');
|
||||
|
||||
/** Validation for every NEW password (not for login, where old ones still count). */
|
||||
export function IsNewPassword(): PropertyDecorator {
|
||||
return applyDecorators(
|
||||
ApiProperty({
|
||||
minLength: MIN_PASSWORD_LENGTH,
|
||||
description: `Mindestens ${MIN_PASSWORD_LENGTH} Zeichen, höchstens ${MAX_PASSWORD_BYTES} Byte`,
|
||||
}),
|
||||
IsString(),
|
||||
MinLength(MIN_PASSWORD_LENGTH, {
|
||||
message: `Passwort muss mindestens ${MIN_PASSWORD_LENGTH} Zeichen lang sein`,
|
||||
}),
|
||||
ValidateBy({
|
||||
name: 'maxPasswordBytes',
|
||||
validator: {
|
||||
validate: (value: unknown) =>
|
||||
typeof value === 'string' &&
|
||||
Buffer.byteLength(value, 'utf8') <= MAX_PASSWORD_BYTES,
|
||||
defaultMessage: buildMessage(
|
||||
() =>
|
||||
`Passwort darf höchstens ${MAX_PASSWORD_BYTES} Byte lang sein (bei Umlauten weniger Zeichen)`,
|
||||
),
|
||||
},
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
function breachCheckEnabled(): boolean {
|
||||
return (
|
||||
process.env.PASSWORD_BREACH_CHECK !== 'false' &&
|
||||
process.env.NODE_ENV !== 'test'
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Refuses a password that appears in known data breaches (Have I Been
|
||||
* Pwned, k-anonymity: only the first 5 hex characters of its SHA-1 leave
|
||||
* the server, padded responses). If the service cannot be reached the
|
||||
* password is accepted and a warning logged - a third-party outage must not
|
||||
* lock everyone out of changing passwords. PASSWORD_BREACH_CHECK=false turns
|
||||
* it off for offline deployments.
|
||||
*/
|
||||
export async function assertPasswordNotBreached(
|
||||
password: string,
|
||||
): Promise<void> {
|
||||
if (!breachCheckEnabled()) {
|
||||
return;
|
||||
}
|
||||
const sha1 = crypto
|
||||
.createHash('sha1')
|
||||
.update(password)
|
||||
.digest('hex')
|
||||
.toUpperCase();
|
||||
const prefix = sha1.slice(0, 5);
|
||||
const suffix = sha1.slice(5);
|
||||
let body: string;
|
||||
try {
|
||||
const response = await fetch(`${PWNED_RANGE_URL}${prefix}`, {
|
||||
headers: { 'Add-Padding': 'true', 'User-Agent': 'ApiCore' },
|
||||
signal: AbortSignal.timeout(BREACH_CHECK_TIMEOUT_MS),
|
||||
});
|
||||
if (!response.ok) {
|
||||
throw new Error(`HTTP ${response.status}`);
|
||||
}
|
||||
body = await response.text();
|
||||
} catch (error: unknown) {
|
||||
logger.warn(
|
||||
`Breach check unavailable, password accepted: ${error instanceof Error ? error.message : String(error)}`,
|
||||
);
|
||||
return;
|
||||
}
|
||||
const breached = body.split('\n').some((line) => {
|
||||
const [hashSuffix, count] = line.trim().split(':');
|
||||
return hashSuffix === suffix && Number(count) > 0;
|
||||
});
|
||||
if (breached) {
|
||||
throw new BadRequestException(
|
||||
'Dieses Passwort ist aus Datenlecks bekannt und daher unsicher. Bitte ein anderes wählen.',
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -1,13 +1,12 @@
|
||||
import { ApiProperty } from '@nestjs/swagger';
|
||||
import { IsString, MinLength } from 'class-validator';
|
||||
import { IsNewPassword } from '../../common/password/password-policy';
|
||||
import { IsString } from 'class-validator';
|
||||
|
||||
export class ChangePasswordDto {
|
||||
@ApiProperty()
|
||||
@IsString()
|
||||
currentPassword!: string;
|
||||
|
||||
@ApiProperty()
|
||||
@IsString()
|
||||
@MinLength(8)
|
||||
@IsNewPassword()
|
||||
newPassword!: string;
|
||||
}
|
||||
@@ -14,6 +14,7 @@ import { AuditLogService } from '../audit/audit-log.service';
|
||||
import { matchesFileSignature } from '../common/file-signature';
|
||||
import { MfaService, MfaEnrollment } from '../auth/mfa/mfa.service';
|
||||
import { ReauthService } from '../auth/reauth/reauth.service';
|
||||
import { assertPasswordNotBreached } from '../common/password/password-policy';
|
||||
import { PermissionResolverService } from '../auth/permissions/permission-resolver.service';
|
||||
import { UsersService } from '../users/users.service';
|
||||
import {
|
||||
@@ -114,6 +115,7 @@ export class ProfileService {
|
||||
if (!currentMatches) {
|
||||
throw new UnauthorizedException('Aktuelles Passwort ist falsch');
|
||||
}
|
||||
await assertPasswordNotBreached(newPassword);
|
||||
const passwordHash = await bcrypt.hash(newPassword, BCRYPT_SALT_ROUNDS);
|
||||
await this.prisma.user.update({
|
||||
where: { id: userId },
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { ApiProperty } from '@nestjs/swagger';
|
||||
import { IsNewPassword } from '../../common/password/password-policy';
|
||||
import { IsEmail, IsString, MinLength } from 'class-validator';
|
||||
import { NormalizeEmail } from '../../common/validation/normalize-email';
|
||||
|
||||
@@ -8,9 +9,7 @@ export class CreateUserDto {
|
||||
@IsEmail()
|
||||
email!: string;
|
||||
|
||||
@ApiProperty()
|
||||
@IsString()
|
||||
@MinLength(8)
|
||||
@IsNewPassword()
|
||||
password!: string;
|
||||
|
||||
@ApiProperty()
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
import { ApiProperty } from '@nestjs/swagger';
|
||||
import { IsString, MinLength } from 'class-validator';
|
||||
import { IsNewPassword } from '../../common/password/password-policy';
|
||||
|
||||
/**
|
||||
* An administrator setting a new password for someone else. There is no
|
||||
@@ -8,8 +7,6 @@ import { IsString, MinLength } from 'class-validator';
|
||||
* generic user update cannot make, since the password is never readable.
|
||||
*/
|
||||
export class ResetUserPasswordDto {
|
||||
@ApiProperty({ minLength: 8 })
|
||||
@IsString()
|
||||
@MinLength(8)
|
||||
@IsNewPassword()
|
||||
password!: string;
|
||||
}
|
||||
@@ -7,6 +7,7 @@ import {
|
||||
import * as bcrypt from 'bcryptjs';
|
||||
import { PrismaService } from '../prisma/prisma.service';
|
||||
import { AuditLogService } from '../audit/audit-log.service';
|
||||
import { assertPasswordNotBreached } from '../common/password/password-policy';
|
||||
import { PermissionResolverService } from '../auth/permissions/permission-resolver.service';
|
||||
import { Prisma } from '../../generated/prisma';
|
||||
import { CreateUserDto } from './dto/create-user.dto';
|
||||
@@ -209,6 +210,7 @@ export class UsersService {
|
||||
await this.assertActiveRole(dto.roleId);
|
||||
await this.assertCanAssignRole(actingUserId);
|
||||
await this.assertWithinCallerPermissions(dto.roleId, actingUserId);
|
||||
await assertPasswordNotBreached(dto.password);
|
||||
const passwordHash = await bcrypt.hash(dto.password, BCRYPT_SALT_ROUNDS);
|
||||
const user = await this.prisma.user.create({
|
||||
data: {
|
||||
@@ -337,6 +339,7 @@ export class UsersService {
|
||||
}
|
||||
const target = await this.findById(id);
|
||||
await this.assertWithinCallerPermissions(target.roleId, actingUserId);
|
||||
await assertPasswordNotBreached(dto.password);
|
||||
const passwordHash = await bcrypt.hash(dto.password, BCRYPT_SALT_ROUNDS);
|
||||
const user = await this.prisma.user.update({
|
||||
where: { id },
|
||||
|
||||
Reference in new issue
Block a user