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
14 changes: 11 additions & 3 deletions server/src/controllers/auth.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ import { getClientInfo } from '@/utils/getClientInfo';
import { LoginUserSchema } from '@/validations/login.schema';
import { RegisterUserSchema } from '@/validations/register.schema';

/**
* Fixed bcrypt hash used to equalize timing when login is attempted for an
* unknown email (#83): one bcrypt compare runs in both failure paths.
*/
const DUMMY_PASSWORD_HASH = '$2b$10$lZKU2EGQLmnz9Fi65/t3GO/coz9zBl6zMMvDyd0EOBgeU1Y28ESHG';

export const registerUser = asyncErrorWrapper(async (req: Request, res: Response) => {
const clientInfo = getClientInfo(req);

Expand Down Expand Up @@ -48,7 +54,7 @@ export const registerUser = asyncErrorWrapper(async (req: Request, res: Response
username,
existingField: existing.email === email ? 'email' : 'username',
});
res.status(StatusCodes.CONFLICT).json({ error: 'Username or email already exists' });
res.status(StatusCodes.CONFLICT).json({ error: 'Registration failed' });
return;
}

Expand Down Expand Up @@ -112,13 +118,15 @@ export const loginUser = asyncErrorWrapper(async (req: Request, res: Response) =
const user = await prisma.user.findUnique({ where: { email } });

if (!user) {
await bcrypt.compare(password, DUMMY_PASSWORD_HASH);

logger.warn('Login failed - user not found', {
action: 'LOGIN_USER_NOT_FOUND',
...clientInfo,
email,
});

res.status(StatusCodes.UNAUTHORIZED).json({ error: result.error });
res.status(StatusCodes.UNAUTHORIZED).json({ error: 'Invalid email or password' });
return;
}

Expand All @@ -133,7 +141,7 @@ export const loginUser = asyncErrorWrapper(async (req: Request, res: Response) =
username: user.username,
});

res.status(StatusCodes.UNAUTHORIZED).json({ error: result.error });
res.status(StatusCodes.UNAUTHORIZED).json({ error: 'Invalid email or password' });
return;
}

Expand Down
55 changes: 55 additions & 0 deletions server/test/auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,35 @@ describe('Auth Routes', () => {
});
});

it('should not leak which field collided when registration fails (#83)', async () => {
await request(app).post('/api/auth/register').send({
email: 'enum-email@test.dev',
username: 'enumuser',
password: 'secure123',
});

const dupEmail = await request(app).post('/api/auth/register').send({
email: 'enum-email@test.dev',
username: 'unusedname',
password: 'secure123',
});

const dupUsername = await request(app).post('/api/auth/register').send({
email: 'unused@test.dev',
username: 'enumuser',
password: 'secure123',
});

expect(dupEmail.status).toBe(StatusCodes.CONFLICT);
expect(dupUsername.status).toBe(StatusCodes.CONFLICT);
// Uniform response regardless of which field collided
expect(dupEmail.body).toEqual(dupUsername.body);
expect(typeof dupEmail.body.error).toBe('string');
expect(dupEmail.body.error).not.toMatch(/email/i);
expect(dupEmail.body.error).not.toMatch(/username/i);
expect(dupEmail.body.error).not.toMatch(/exists/i);
});

it('should reject login with invalid password', async () => {
await request(app).post('/api/auth/register').send({
email: 'test@test.dev',
Expand All @@ -104,6 +133,32 @@ describe('Auth Routes', () => {
expect(res.status).toBe(StatusCodes.UNAUTHORIZED);
});

it('should not distinguish unknown user from invalid password on login (#83)', async () => {
await request(app).post('/api/auth/register').send({
email: 'loginenum@test.dev',
username: 'loginenum',
password: 'secure123',
});

const badPassword = await request(app).post('/api/auth/login').send({
email: 'loginenum@test.dev',
password: 'wrongpass',
});

const unknownUser = await request(app).post('/api/auth/login').send({
email: 'ghost@test.dev',
password: 'anypassword',
});

expect(badPassword.status).toBe(StatusCodes.UNAUTHORIZED);
expect(unknownUser.status).toBe(StatusCodes.UNAUTHORIZED);
// Identical responses so probing cannot tell whether the account exists
expect(unknownUser.body).toEqual(badPassword.body);
// And the response carries an actual message (not the legacy empty `{}`)
expect(typeof unknownUser.body.error).toBe('string');
expect(unknownUser.body.error.length).toBeGreaterThan(0);
});

it('should reject registration with invalid email format', async () => {
const res = await request(app).post('/api/auth/register').send({
email: 'invalid-email',
Expand Down
Loading