Skip to content

Commit eae4c89

Browse files
Merge pull request #1460 from demilade18-git/demilade18-git/authapi-1339-1342
feat(auth): OpenAPI docs, input sanitization, password refactor, retry with backoff
2 parents c48022d + 1d1c073 commit eae4c89

9 files changed

Lines changed: 226 additions & 82 deletions

File tree

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
# Auth API input sanitization
2+
3+
`RegisterDto` and `LoginDto` normalise input through class-transformer (the global `ValidationPipe` already runs with `transform: true`):
4+
5+
- `email`: Unicode NFKC normalisation, control characters removed, surrounding whitespace trimmed. The case is **not** changed, so existing accounts keep matching.
6+
- `firstName` / `lastName`: additionally strip HTML tags and collapse whitespace.
7+
- Passwords and tokens are never altered.
8+
9+
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`.

‎docs/api/auth-openapi.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
# Auth API OpenAPI documentation
2+
3+
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.
4+
5+
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.

‎docs/api/auth-refactor.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
# Auth API refactor: password handling
2+
3+
`changePassword` and `resetPassword` duplicated three blocks. They now share private helpers in `AuthService`:
4+
5+
- `assertPasswordMeetsPolicy` - complexity check, 400 listing the broken rules,
6+
- `assertPasswordNotReused` - compares against recent hashes, 400 if reused,
7+
- `recordPasswordHistory` - stores the new hash and trims history beyond the retention limit.
8+
9+
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.

‎docs/api/auth-retry.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
# Auth API retry with backoff
2+
3+
`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.
4+
5+
Used for:
6+
7+
- 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).
8+
- 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.
9+
10+
Login and token operations are not retried: they are not idempotent from the caller's side and failures there are authoritative.

‎src/auth/auth.controller.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,11 @@ export class AuthController {
7575
return this.authService.googleOAuthLogin(req.user);
7676
}
7777

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

9499
@ApiBearerAuth('access-token')
95100
@UseGuards(JwtAuthGuard)
101+
@ApiOperation({ summary: 'Log out the current session' })
102+
@ApiResponse({ status: 201, description: 'Tokens revoked' })
103+
@ApiResponse({ status: 401, description: 'Not authenticated' })
96104
@Post('logout')
97105
logout(
98106
@CurrentUser() user: AuthUserPayload,
@@ -104,20 +112,31 @@ export class AuthController {
104112

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

112123
@ApiBearerAuth('access-token')
113124
@UseGuards(JwtAuthGuard)
125+
@ApiOperation({ summary: 'Get the authenticated user's profile' })
126+
@ApiResponse({ status: 200, description: 'Sanitised user profile' })
127+
@ApiResponse({ status: 401, description: 'Not authenticated' })
128+
@ApiResponse({ status: 404, description: 'User not found' })
114129
@Get('me')
115130
me(@CurrentUser() user: AuthUserPayload) {
116131
return this.authService.me(user);
117132
}
118133

119134
@ApiBearerAuth('access-token')
120135
@UseGuards(JwtAuthGuard, FreshTwoFactorGuard)
136+
@ApiOperation({ summary: 'Change the account password' })
137+
@ApiResponse({ status: 201, description: 'Password updated; other sessions revoked' })
138+
@ApiResponse({ status: 400, description: 'Weak or recently used password' })
139+
@ApiResponse({ status: 401, description: 'Current password incorrect' })
121140
@Post('change-password')
122141
changePassword(
123142
@CurrentUser() user: AuthUserPayload,

‎src/auth/auth.service.ts‎

Lines changed: 80 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import {
99
} from '@nestjs/common';
1010
import { AUTH_DEFAULT_PAGE_SIZE, AuthPaginationQueryDto, buildPageMeta } from './dto/pagination-query.dto';
1111
import { ConfigService } from '@nestjs/config';
12-
import { Prisma, BlacklistedToken, Document, ApiKey, PasswordHistory } from '@prisma/client';
12+
import { Prisma, BlacklistedToken, Document, ApiKey } from '@prisma/client';
1313
import { randomUUID } from 'crypto';
1414
import * as jwt from 'jsonwebtoken';
1515
import { PrismaService } from '../database/prisma.service';
@@ -894,6 +894,51 @@ export class AuthService {
894894
};
895895
}
896896

897+
/** Throws a 400 listing every complexity rule the password breaks. */
898+
private assertPasswordMeetsPolicy(newPassword: string): void {
899+
const passwordErrors = validatePassword(newPassword, this.configService);
900+
if (passwordErrors.length > 0) {
901+
throw new BadRequestException(
902+
`Password does not meet complexity requirements: ${passwordErrors.join('; ')}`,
903+
);
904+
}
905+
}
906+
907+
/** Throws a 400 if `newPassword` matches any of the recent password hashes. */
908+
private async assertPasswordNotReused(
909+
newPassword: string,
910+
recentHashes: string[],
911+
historyLimit: number,
912+
): Promise<void> {
913+
const reused = await Promise.all(recentHashes.map((hash) => comparePassword(newPassword, hash)));
914+
if (reused.some(Boolean)) {
915+
throw new BadRequestException(
916+
`Password reuse is not allowed for the last ${historyLimit} passwords`,
917+
);
918+
}
919+
}
920+
921+
/** Stores the new hash and deletes history entries beyond the retention limit. */
922+
private async recordPasswordHistory(
923+
tx: Prisma.TransactionClient,
924+
userId: string,
925+
passwordHash: string,
926+
historyLimit: number,
927+
): Promise<void> {
928+
await tx.passwordHistory.create({ data: { userId, passwordHash } });
929+
930+
const expired = await tx.passwordHistory.findMany({
931+
where: { userId },
932+
orderBy: { createdAt: 'desc' },
933+
skip: historyLimit,
934+
});
935+
if (expired.length > 0) {
936+
await tx.passwordHistory.deleteMany({
937+
where: { id: { in: expired.map((entry: { id: string }) => entry.id) } },
938+
});
939+
}
940+
}
941+
897942
async changePassword(user: AuthUserPayload, data: ChangePasswordDto) {
898943
const passwordHistoryLimit = this.getPasswordHistoryLimit();
899944
const existingUser = await this.prisma.user.findUnique({
@@ -917,27 +962,15 @@ export class AuthService {
917962
throw new UnauthorizedException('Current password is incorrect');
918963
}
919964

920-
const passwordErrors = validatePassword(data.newPassword, this.configService);
921-
if (passwordErrors.length > 0) {
922-
throw new BadRequestException(
923-
`Password does not meet complexity requirements: ${passwordErrors.join('; ')}`,
924-
);
925-
}
926-
927-
const passwordReused = await Promise.all(
965+
this.assertPasswordMeetsPolicy(data.newPassword);
966+
await this.assertPasswordNotReused(
967+
data.newPassword,
928968
existingUser.passwordHistory
929969
.slice(0, passwordHistoryLimit)
930-
.map((entry: { passwordHash: string }) =>
931-
comparePassword(data.newPassword, entry.passwordHash),
932-
),
970+
.map((entry: { passwordHash: string }) => entry.passwordHash),
971+
passwordHistoryLimit,
933972
);
934973

935-
if (passwordReused.some(Boolean)) {
936-
throw new BadRequestException(
937-
`Password reuse is not allowed for the last ${passwordHistoryLimit} passwords`,
938-
);
939-
}
940-
941974
const newPasswordHash = await hashPassword(data.newPassword, this.bcryptRounds);
942975

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

951-
await tx.passwordHistory.create({
952-
data: {
953-
userId: existingUser.id,
954-
passwordHash: newPasswordHash,
955-
},
956-
});
957-
958-
const historyEntries = await tx.passwordHistory.findMany({
959-
where: { userId: existingUser.id },
960-
orderBy: { createdAt: 'desc' },
961-
skip: passwordHistoryLimit,
962-
});
963-
964-
if (historyEntries.length > 0) {
965-
await tx.passwordHistory.deleteMany({
966-
where: {
967-
id: {
968-
in: historyEntries.map((entry: { id: string }) => entry.id),
969-
},
970-
},
971-
});
972-
}
984+
await this.recordPasswordHistory(tx, existingUser.id, newPasswordHash, passwordHistoryLimit);
973985
});
974986

975987
// Audit log password change (#886)
@@ -1696,7 +1708,12 @@ export class AuthService {
16961708
});
16971709

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

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

17251742
const passwordHistoryLimit = this.getPasswordHistoryLimit();
17261743

1727-
const passwordErrors = validatePassword(data.newPassword, this.configService);
1728-
if (passwordErrors.length > 0) {
1729-
throw new BadRequestException(
1730-
`Password does not meet complexity requirements: ${passwordErrors.join('; ')}`,
1731-
);
1732-
}
1744+
this.assertPasswordMeetsPolicy(data.newPassword);
17331745

17341746
// Check if new password was used recently
17351747
const recentPasswords = await this.prisma.passwordHistory.findMany({
17361748
where: { userId: resetToken.userId },
17371749
orderBy: { createdAt: 'desc' },
17381750
take: passwordHistoryLimit,
17391751
});
1740-
1741-
const reuseResults = await Promise.all(
1742-
recentPasswords.map((entry) => comparePassword(data.newPassword, entry.passwordHash)),
1752+
await this.assertPasswordNotReused(
1753+
data.newPassword,
1754+
recentPasswords.map((entry) => entry.passwordHash),
1755+
passwordHistoryLimit,
17431756
);
1744-
if (reuseResults.some(Boolean)) {
1745-
throw new BadRequestException(
1746-
`Password reuse is not allowed for the last ${passwordHistoryLimit} passwords`,
1747-
);
1748-
}
17491757

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

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

1764-
await tx.passwordHistory.create({
1765-
data: {
1766-
userId: resetToken.userId,
1767-
passwordHash: newPasswordHash,
1768-
},
1769-
});
1770-
1771-
// Clean up old password history entries
1772-
const historyEntries = await tx.passwordHistory.findMany({
1773-
where: { userId: resetToken.userId },
1774-
orderBy: { createdAt: 'desc' },
1775-
skip: passwordHistoryLimit,
1776-
});
1777-
1778-
if (historyEntries.length > 0) {
1779-
await tx.passwordHistory.deleteMany({
1780-
where: {
1781-
id: { in: historyEntries.map((entry: PasswordHistory) => entry.id) },
1782-
},
1783-
});
1784-
}
1772+
await this.recordPasswordHistory(tx, resetToken.userId, newPasswordHash, passwordHistoryLimit);
17851773
});
17861774

17871775
await this.sessionsService.revokeAllSessions(resetToken.userId);
@@ -1851,13 +1839,23 @@ export class AuthService {
18511839
}
18521840

18531841
try {
1854-
const response = await fetch('https://www.google.com/recaptcha/api/siteverify', {
1855-
method: 'POST',
1856-
headers: {
1857-
'Content-Type': 'application/x-www-form-urlencoded',
1842+
// Retried with backoff on network errors, 429 and 5xx; other responses are final.
1843+
const response = await retryWithBackoff(
1844+
async () => {
1845+
const res = await fetch('https://www.google.com/recaptcha/api/siteverify', {
1846+
method: 'POST',
1847+
headers: {
1848+
'Content-Type': 'application/x-www-form-urlencoded',
1849+
},
1850+
body: `secret=${secret}&response=${token}`,
1851+
});
1852+
if (res.status === 429 || res.status >= 500) {
1853+
throw new TransientError(`reCAPTCHA responded ${res.status}`);
1854+
}
1855+
return res;
18581856
},
1859-
body: `secret=${secret}&response=${token}`,
1860-
});
1857+
{ attempts: 3, baseDelayMs: 200, maxDelayMs: 1_500 },
1858+
);
18611859

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

‎src/auth/dto/auth.dto.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,11 @@ import {
1010
MinLength,
1111
Matches,
1212
} from 'class-validator';
13+
import { Transform } from 'class-transformer';
14+
import { CleanText, TrimIdentifier } from '../sanitize.util';
1315

1416
export class RegisterDto {
17+
@Transform(TrimIdentifier)
1518
@IsEmail()
1619
email: string;
1720

@@ -23,10 +26,12 @@ export class RegisterDto {
2326
})
2427
password: string;
2528

29+
@Transform(CleanText)
2630
@IsString()
2731
@IsNotEmpty()
2832
firstName: string;
2933

34+
@Transform(CleanText)
3035
@IsString()
3136
@IsNotEmpty()
3237
lastName: string;
@@ -37,6 +42,7 @@ export class RegisterDto {
3742
}
3843

3944
export class LoginDto {
45+
@Transform(TrimIdentifier)
4046
@IsEmail()
4147
email: string;
4248

0 commit comments

Comments
 (0)