Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions docs/api/auth-input-sanitization.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Auth API input sanitization

`RegisterDto` and `LoginDto` normalise input through class-transformer (the global `ValidationPipe` already runs with `transform: true`):

- `email`: Unicode NFKC normalisation, control characters removed, surrounding whitespace trimmed. The case is **not** changed, so existing accounts keep matching.
- `firstName` / `lastName`: additionally strip HTML tags and collapse whitespace.
- Passwords and tokens are never altered.

Sanitising happens before validation, so `" user@example.com "` is accepted as `user@example.com` and a name consisting only of tags becomes empty and fails `@IsNotEmpty`. Helpers live in `src/auth/sanitize.util.ts`.
5 changes: 5 additions & 0 deletions docs/api/auth-openapi.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Auth API OpenAPI documentation

The main `AuthController` operations now carry `@ApiOperation` summaries and `@ApiResponse` entries (success plus the documented 400/401/404/429 cases) so they appear described in Swagger: register, login, refresh, logout, logout-all, me, change-password, password-reset request/reset and verify-email.

Open the Swagger UI exposed by the app and look under the **Authentication** tag. Protected operations show the `access-token` bearer scheme through the existing `@ApiBearerAuth` decorators.
9 changes: 9 additions & 0 deletions docs/api/auth-refactor.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Auth API refactor: password handling

`changePassword` and `resetPassword` duplicated three blocks. They now share private helpers in `AuthService`:

- `assertPasswordMeetsPolicy` - complexity check, 400 listing the broken rules,
- `assertPasswordNotReused` - compares against recent hashes, 400 if reused,
- `recordPasswordHistory` - stores the new hash and trims history beyond the retention limit.

Behaviour, error messages and the transaction boundaries are unchanged; each method is roughly 25 lines shorter and the reuse and retention rules now live in one place.
10 changes: 10 additions & 0 deletions docs/api/auth-retry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Auth API retry with backoff

`retryWithBackoff` (`src/auth/retry.util.ts`) retries a failing call with exponential backoff (doubling from the base delay, capped, with up to 50% jitter) and rethrows the last error when attempts run out or `isRetryable` says stop.

Used for:

- the reCAPTCHA verification request: up to 3 attempts, retried on network errors, `429` and `5xx` (other responses are final). If it still fails the existing behaviour applies (verification fails closed).
- sending the password reset email: up to 3 attempts, so a transient mail-provider error does not silently lose the email. Persistent failure still propagates as before.

Login and token operations are not retried: they are not idempotent from the caller's side and failures there are authoritative.
19 changes: 19 additions & 0 deletions src/auth/auth.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,11 @@ export class AuthController {
return this.authService.googleOAuthLogin(req.user);
}

@ApiOperation({ summary: 'Log in with email and password' })
@ApiResponse({ status: 201, description: 'Access and refresh tokens (or a 2FA challenge)' })
@ApiResponse({ status: 400, description: 'Validation failed' })
@ApiResponse({ status: 401, description: 'Invalid credentials or locked account' })
@ApiResponse({ status: 429, description: 'Too many attempts' })
@Post('login')
login(@Body(CredentialInjectionPipe) loginDto: LoginDto, @Req() request: Request) {
const ipAddress = request.ip || request.socket.remoteAddress;
Expand All @@ -93,6 +98,9 @@ export class AuthController {

@ApiBearerAuth('access-token')
@UseGuards(JwtAuthGuard)
@ApiOperation({ summary: 'Log out the current session' })
@ApiResponse({ status: 201, description: 'Tokens revoked' })
@ApiResponse({ status: 401, description: 'Not authenticated' })
@Post('logout')
logout(
@CurrentUser() user: AuthUserPayload,
Expand All @@ -104,20 +112,31 @@ export class AuthController {

@ApiBearerAuth('access-token')
@UseGuards(JwtAuthGuard)
@ApiOperation({ summary: 'Log out of every device' })
@ApiResponse({ status: 201, description: 'All sessions revoked' })
@ApiResponse({ status: 401, description: 'Not authenticated' })
@Post('logout-all')
logoutAllDevices(@CurrentUser() user: AuthUserPayload, @Req() request: { accessToken?: string }) {
return this.authService.logoutAllDevices(user, request.accessToken);
}

@ApiBearerAuth('access-token')
@UseGuards(JwtAuthGuard)
@ApiOperation({ summary: 'Get the authenticated user's profile' })
@ApiResponse({ status: 200, description: 'Sanitised user profile' })
@ApiResponse({ status: 401, description: 'Not authenticated' })
@ApiResponse({ status: 404, description: 'User not found' })
@Get('me')
me(@CurrentUser() user: AuthUserPayload) {
return this.authService.me(user);
}

@ApiBearerAuth('access-token')
@UseGuards(JwtAuthGuard, FreshTwoFactorGuard)
@ApiOperation({ summary: 'Change the account password' })
@ApiResponse({ status: 201, description: 'Password updated; other sessions revoked' })
@ApiResponse({ status: 400, description: 'Weak or recently used password' })
@ApiResponse({ status: 401, description: 'Current password incorrect' })
@Post('change-password')
changePassword(
@CurrentUser() user: AuthUserPayload,
Expand Down
162 changes: 80 additions & 82 deletions src/auth/auth.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
} from '@nestjs/common';
import { AUTH_DEFAULT_PAGE_SIZE, AuthPaginationQueryDto, buildPageMeta } from './dto/pagination-query.dto';
import { ConfigService } from '@nestjs/config';
import { Prisma, BlacklistedToken, Document, ApiKey, PasswordHistory } from '@prisma/client';
import { Prisma, BlacklistedToken, Document, ApiKey } from '@prisma/client';
import { randomUUID } from 'crypto';
import * as jwt from 'jsonwebtoken';
import { PrismaService } from '../database/prisma.service';
Expand Down Expand Up @@ -894,6 +894,51 @@ export class AuthService {
};
}

/** Throws a 400 listing every complexity rule the password breaks. */
private assertPasswordMeetsPolicy(newPassword: string): void {
const passwordErrors = validatePassword(newPassword, this.configService);
if (passwordErrors.length > 0) {
throw new BadRequestException(
`Password does not meet complexity requirements: ${passwordErrors.join('; ')}`,
);
}
}

/** Throws a 400 if `newPassword` matches any of the recent password hashes. */
private async assertPasswordNotReused(
newPassword: string,
recentHashes: string[],
historyLimit: number,
): Promise<void> {
const reused = await Promise.all(recentHashes.map((hash) => comparePassword(newPassword, hash)));
if (reused.some(Boolean)) {
throw new BadRequestException(
`Password reuse is not allowed for the last ${historyLimit} passwords`,
);
}
}

/** Stores the new hash and deletes history entries beyond the retention limit. */
private async recordPasswordHistory(
tx: Prisma.TransactionClient,
userId: string,
passwordHash: string,
historyLimit: number,
): Promise<void> {
await tx.passwordHistory.create({ data: { userId, passwordHash } });

const expired = await tx.passwordHistory.findMany({
where: { userId },
orderBy: { createdAt: 'desc' },
skip: historyLimit,
});
if (expired.length > 0) {
await tx.passwordHistory.deleteMany({
where: { id: { in: expired.map((entry: { id: string }) => entry.id) } },
});
}
}

async changePassword(user: AuthUserPayload, data: ChangePasswordDto) {
const passwordHistoryLimit = this.getPasswordHistoryLimit();
const existingUser = await this.prisma.user.findUnique({
Expand All @@ -917,27 +962,15 @@ export class AuthService {
throw new UnauthorizedException('Current password is incorrect');
}

const passwordErrors = validatePassword(data.newPassword, this.configService);
if (passwordErrors.length > 0) {
throw new BadRequestException(
`Password does not meet complexity requirements: ${passwordErrors.join('; ')}`,
);
}

const passwordReused = await Promise.all(
this.assertPasswordMeetsPolicy(data.newPassword);
await this.assertPasswordNotReused(
data.newPassword,
existingUser.passwordHistory
.slice(0, passwordHistoryLimit)
.map((entry: { passwordHash: string }) =>
comparePassword(data.newPassword, entry.passwordHash),
),
.map((entry: { passwordHash: string }) => entry.passwordHash),
passwordHistoryLimit,
);

if (passwordReused.some(Boolean)) {
throw new BadRequestException(
`Password reuse is not allowed for the last ${passwordHistoryLimit} passwords`,
);
}

const newPasswordHash = await hashPassword(data.newPassword, this.bcryptRounds);

await this.prisma.$transaction(async (tx: Prisma.TransactionClient) => {
Expand All @@ -948,28 +981,7 @@ export class AuthService {
},
});

await tx.passwordHistory.create({
data: {
userId: existingUser.id,
passwordHash: newPasswordHash,
},
});

const historyEntries = await tx.passwordHistory.findMany({
where: { userId: existingUser.id },
orderBy: { createdAt: 'desc' },
skip: passwordHistoryLimit,
});

if (historyEntries.length > 0) {
await tx.passwordHistory.deleteMany({
where: {
id: {
in: historyEntries.map((entry: { id: string }) => entry.id),
},
},
});
}
await this.recordPasswordHistory(tx, existingUser.id, newPasswordHash, passwordHistoryLimit);
});

// Audit log password change (#886)
Expand Down Expand Up @@ -1696,7 +1708,12 @@ export class AuthService {
});

// Send reset email with raw token (not the hash)
await this.emailService.sendPasswordResetEmail(user.email, resetToken);
// Retried with backoff: a transient mail-provider failure should not lose the reset email.
await retryWithBackoff(() => this.emailService.sendPasswordResetEmail(user.email, resetToken), {
attempts: 3,
baseDelayMs: 300,
maxDelayMs: 2_000,
});
}

async resetPassword(data: ResetPasswordDto): Promise<void> {
Expand Down Expand Up @@ -1724,28 +1741,19 @@ export class AuthService {

const passwordHistoryLimit = this.getPasswordHistoryLimit();

const passwordErrors = validatePassword(data.newPassword, this.configService);
if (passwordErrors.length > 0) {
throw new BadRequestException(
`Password does not meet complexity requirements: ${passwordErrors.join('; ')}`,
);
}
this.assertPasswordMeetsPolicy(data.newPassword);

// Check if new password was used recently
const recentPasswords = await this.prisma.passwordHistory.findMany({
where: { userId: resetToken.userId },
orderBy: { createdAt: 'desc' },
take: passwordHistoryLimit,
});

const reuseResults = await Promise.all(
recentPasswords.map((entry) => comparePassword(data.newPassword, entry.passwordHash)),
await this.assertPasswordNotReused(
data.newPassword,
recentPasswords.map((entry) => entry.passwordHash),
passwordHistoryLimit,
);
if (reuseResults.some(Boolean)) {
throw new BadRequestException(
`Password reuse is not allowed for the last ${passwordHistoryLimit} passwords`,
);
}

const newPasswordHash = await hashPassword(data.newPassword, this.bcryptRounds);

Expand All @@ -1761,27 +1769,7 @@ export class AuthService {
data: { usedAt: new Date() },
});

await tx.passwordHistory.create({
data: {
userId: resetToken.userId,
passwordHash: newPasswordHash,
},
});

// Clean up old password history entries
const historyEntries = await tx.passwordHistory.findMany({
where: { userId: resetToken.userId },
orderBy: { createdAt: 'desc' },
skip: passwordHistoryLimit,
});

if (historyEntries.length > 0) {
await tx.passwordHistory.deleteMany({
where: {
id: { in: historyEntries.map((entry: PasswordHistory) => entry.id) },
},
});
}
await this.recordPasswordHistory(tx, resetToken.userId, newPasswordHash, passwordHistoryLimit);
});

await this.sessionsService.revokeAllSessions(resetToken.userId);
Expand Down Expand Up @@ -1851,13 +1839,23 @@ export class AuthService {
}

try {
const response = await fetch('https://www.google.com/recaptcha/api/siteverify', {
method: 'POST',
headers: {
'Content-Type': 'application/x-www-form-urlencoded',
// Retried with backoff on network errors, 429 and 5xx; other responses are final.
const response = await retryWithBackoff(
async () => {
const res = await fetch('https://www.google.com/recaptcha/api/siteverify', {
method: 'POST',
headers: {
'Content-Type': 'application/x-www-form-urlencoded',
},
body: `secret=${secret}&response=${token}`,
});
if (res.status === 429 || res.status >= 500) {
throw new TransientError(`reCAPTCHA responded ${res.status}`);
}
return res;
},
body: `secret=${secret}&response=${token}`,
});
{ attempts: 3, baseDelayMs: 200, maxDelayMs: 1_500 },
);

const data = (await response.json()) as RecaptchaVerifyResponse;

Expand Down
6 changes: 6 additions & 0 deletions src/auth/dto/auth.dto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,11 @@ import {
MinLength,
Matches,
} from 'class-validator';
import { Transform } from 'class-transformer';
import { CleanText, TrimIdentifier } from '../sanitize.util';

export class RegisterDto {
@Transform(TrimIdentifier)
@IsEmail()
email: string;

Expand All @@ -23,10 +26,12 @@ export class RegisterDto {
})
password: string;

@Transform(CleanText)
@IsString()
@IsNotEmpty()
firstName: string;

@Transform(CleanText)
@IsString()
@IsNotEmpty()
lastName: string;
Expand All @@ -37,6 +42,7 @@ export class RegisterDto {
}

export class LoginDto {
@Transform(TrimIdentifier)
@IsEmail()
email: string;

Expand Down
Loading