mirror of
https://github.com/vernu/textbee.git
synced 2026-09-03 03:29:58 +03:00
fix(api): actually enforce email and password validation
validateEmail and validatePassword are async and signal failure by throwing (a rejected promise). Three callers invoked them without await: register (both) and changePassword. An unawaited rejected promise is dropped, so execution continued past the check: registration accepted a malformed email or an out-of-range password, and a password change accepted a too-short new password. The validators were effectively dead. Adding await makes the existing rules take effect at their intended point. Legitimate clients are unaffected: the web signup and change-password forms already enforce a valid email and the length bounds, so this only rejects input the service was always meant to reject, with a clean 400 instead of persisting bad data. Guards were written first and seen to fail against the pre-fix code (register with a bad email or short password still created the user; changePassword with a short password still saved), then pass after the await is added. The valid-input paths are asserted to still succeed. This is the dependency-free half of what a global ValidationPipe would have covered. The pipe itself is deferred: class-transformer is not installed, so enabling it now would add a dependency and risk crashing once any DTO gains a decorator. Recommended as a separate follow-up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
b514891cad
commit
6b66160f7f
@@ -17,19 +17,22 @@ const build = () => {
|
||||
const usersService = {
|
||||
findOne: jest.fn(),
|
||||
findOneWithPassword: jest.fn(),
|
||||
create: jest.fn(),
|
||||
}
|
||||
const passwordResetModel = { findOne: jest.fn() }
|
||||
const mailService = { sendEmailFromTemplate: jest.fn().mockResolvedValue(undefined) }
|
||||
const jwtService = { sign: jest.fn().mockReturnValue('signed-jwt') }
|
||||
const turnstileService = { verify: jest.fn().mockResolvedValue(undefined) }
|
||||
|
||||
const service = new AuthService(
|
||||
usersService as any,
|
||||
{} as any, // jwtService
|
||||
jwtService as any,
|
||||
apiKeyModel,
|
||||
passwordResetModel as any,
|
||||
{} as any, // accessLogModel
|
||||
{} as any, // emailVerificationModel
|
||||
mailService as any,
|
||||
{} as any, // turnstileService
|
||||
turnstileService as any,
|
||||
)
|
||||
|
||||
return {
|
||||
@@ -38,6 +41,8 @@ const build = () => {
|
||||
usersService,
|
||||
passwordResetModel,
|
||||
mailService,
|
||||
jwtService,
|
||||
turnstileService,
|
||||
getLastApiKeyDoc: () => lastApiKeyDoc,
|
||||
}
|
||||
}
|
||||
@@ -177,6 +182,83 @@ describe('AuthService', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('register input validation', () => {
|
||||
const registerSetup = () => {
|
||||
const ctx = build()
|
||||
ctx.usersService.findOne.mockResolvedValue(null) // no existing user
|
||||
// If validation is (incorrectly) skipped, register would reach create;
|
||||
// return a usable doc so the pre-fix path resolves rather than erroring.
|
||||
ctx.usersService.create.mockResolvedValue({
|
||||
_id: 'user_1',
|
||||
email: 'x',
|
||||
lastLoginAt: null,
|
||||
save: jest.fn().mockResolvedValue(undefined),
|
||||
toObject: () => ({ _id: 'user_1' }),
|
||||
})
|
||||
return ctx
|
||||
}
|
||||
|
||||
it('rejects a malformed email and does not create the user', async () => {
|
||||
const { service, usersService } = registerSetup()
|
||||
|
||||
await expect(
|
||||
service.register({
|
||||
name: 'Ada',
|
||||
email: 'not-an-email',
|
||||
password: 'a-valid-password',
|
||||
turnstileToken: 'token',
|
||||
}),
|
||||
).rejects.toThrow(HttpException)
|
||||
expect(usersService.create).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('rejects a too-short password and does not create the user', async () => {
|
||||
const { service, usersService } = registerSetup()
|
||||
|
||||
await expect(
|
||||
service.register({
|
||||
name: 'Ada',
|
||||
email: 'a@b.com',
|
||||
password: '123',
|
||||
turnstileToken: 'token',
|
||||
}),
|
||||
).rejects.toThrow(HttpException)
|
||||
expect(usersService.create).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('creates the user when email and password are valid', async () => {
|
||||
const { service, usersService } = registerSetup()
|
||||
|
||||
await service.register({
|
||||
name: 'Ada',
|
||||
email: 'a@b.com',
|
||||
password: 'a-valid-password',
|
||||
turnstileToken: 'token',
|
||||
})
|
||||
expect(usersService.create).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
|
||||
describe('changePassword input validation', () => {
|
||||
it('rejects a too-short new password without saving', async () => {
|
||||
const ctx = build()
|
||||
const stored = {
|
||||
_id: 'user_1',
|
||||
password: bcrypt.hashSync('correct-old', 10),
|
||||
save: jest.fn().mockResolvedValue(undefined),
|
||||
}
|
||||
ctx.usersService.findOneWithPassword.mockResolvedValue(stored)
|
||||
|
||||
await expect(
|
||||
ctx.service.changePassword(
|
||||
{ oldPassword: 'correct-old', newPassword: '123' },
|
||||
{ _id: 'user_1' } as any,
|
||||
),
|
||||
).rejects.toThrow(HttpException)
|
||||
expect(stored.save).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe('resetPassword', () => {
|
||||
const setup = () => {
|
||||
const ctx = build()
|
||||
|
||||
@@ -126,8 +126,8 @@ export class AuthService {
|
||||
)
|
||||
}
|
||||
|
||||
this.validateEmail(userData.email)
|
||||
this.validatePassword(userData.password)
|
||||
await this.validateEmail(userData.email)
|
||||
await this.validatePassword(userData.password)
|
||||
|
||||
const hashedPassword = await bcrypt.hash(userData.password, 10)
|
||||
const { turnstileToken, ...sanitizedUserData } = userData
|
||||
@@ -261,7 +261,7 @@ export class AuthService {
|
||||
)
|
||||
}
|
||||
|
||||
this.validatePassword(input.newPassword)
|
||||
await this.validatePassword(input.newPassword)
|
||||
|
||||
const hashedPassword = await bcrypt.hash(input.newPassword, 10)
|
||||
userToUpdate.password = hashedPassword
|
||||
|
||||
Reference in New Issue
Block a user