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
184 changes: 87 additions & 97 deletions src/controllers/UsersControllers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ describe('UsersController.register', () => {
expect(res.cookie).toHaveBeenCalledWith('token', 'jwt-reg-tok');
expect(res.status).toHaveBeenCalledWith(200);
expect(res.json).toHaveBeenCalledWith(
expect.objectContaining({ token: 'jwt-reg-tok', verificationPending: true })
expect.objectContaining({ token: 'jwt-reg-tok' })
);
expect(next).not.toHaveBeenCalled();
});
Expand Down Expand Up @@ -241,7 +241,7 @@ describe('UsersController.verifyEmail', () => {
return { redirect } as unknown as express.Response & { redirect: jest.Mock };
};

it('redirects to /uploads?verified=1 when the verify_email token is valid', async () => {
it('redirects to /login?verified=1 when the verify_email token is valid and user is unauthenticated', async () => {
const verifyMagicToken = jest
.fn()
.mockResolvedValue({ userId: 7, purpose: 'verify_email' });
Expand All @@ -254,7 +254,31 @@ describe('UsersController.verifyEmail', () => {
await controller.verifyEmail(req, res, next);

expect(markEmailVerified).toHaveBeenCalledWith('7');
expect(res.redirect).toHaveBeenCalledWith('/downloads?verified=1');
expect(res.redirect).toHaveBeenCalledWith('/login?verified=1');
});

it('redirects to /account?verified=1 when the token is valid and user is authenticated', async () => {
const verifyMagicToken = jest
.fn()
.mockResolvedValue({ userId: 7, purpose: 'verify_email' });
const markEmailVerified = jest.fn().mockResolvedValue(1);
const authGetUserFrom = jest.fn().mockResolvedValue({ id: 7 });
const { controller } = buildVerifyEmailController({
verifyMagicToken,
markEmailVerified,
authGetUserFrom,
});
const req = {
params: { token: 'valid-verify-tok' },
cookies: { token: 'session' },
} as unknown as express.Request;
const res = buildVerifyEmailRes();
const next = jest.fn();

await controller.verifyEmail(req, res, next);

expect(markEmailVerified).toHaveBeenCalledWith('7');
expect(res.redirect).toHaveBeenCalledWith('/account?verified=1');
});

it('redirects to /login?verify_error=expired when token is invalid and user is unauthenticated', async () => {
Expand Down Expand Up @@ -405,6 +429,7 @@ describe('UsersController.verifyMagicLink', () => {
newJWTToken?: jest.Mock;
persistToken?: jest.Mock;
updateLastLoginAt?: jest.Mock;
markEmailVerified?: jest.Mock;
}) => {
const userService = {
verifyMagicToken:
Expand All @@ -414,6 +439,8 @@ describe('UsersController.verifyMagicLink', () => {
jest.fn().mockResolvedValue({ id: 1, email: 'al@example.com' }),
updateLastLoginAt:
overrides?.updateLastLoginAt ?? jest.fn().mockResolvedValue(undefined),
markEmailVerified:
overrides?.markEmailVerified ?? jest.fn().mockResolvedValue(1),
} as unknown as UsersService;
const authService = {
newJWTToken:
Expand Down Expand Up @@ -498,6 +525,7 @@ describe('UsersController.verifyMagicLink', () => {
getUserById,
updateResetToken,
updateLastLoginAt: jest.fn().mockResolvedValue(undefined),
markEmailVerified: jest.fn().mockResolvedValue(1),
} as unknown as UsersService;
const authService = {
newJWTToken: jest.fn().mockResolvedValue('jwt-tok'),
Expand All @@ -521,6 +549,62 @@ describe('UsersController.verifyMagicLink', () => {
expect(jsonCall.reset_token.length).toBeGreaterThan(0);
expect(updateResetToken).toHaveBeenCalledWith('8', jsonCall.reset_token);
});

it('marks email verified after a successful login magic link', async () => {
const verifyMagicToken = jest
.fn()
.mockResolvedValue({ userId: 5, purpose: 'login' });
const getUserById = jest
.fn()
.mockResolvedValue({ id: 5, email: 'al@example.com' });
const markEmailVerified = jest.fn().mockResolvedValue(1);
const { controller } = buildVerifyController({
verifyMagicToken,
getUserById,
markEmailVerified,
});
const req = { params: { token: 'valid-tok' } } as unknown as express.Request;
const res = buildVerifyRes();
const next = jest.fn();

await controller.verifyMagicLink(req, res, next);

expect(markEmailVerified).toHaveBeenCalledWith('5');
});

it('marks email verified after a successful password_reset magic link', async () => {
const verifyMagicToken = jest
.fn()
.mockResolvedValue({ userId: 8, purpose: 'password_reset' });
const getUserById = jest
.fn()
.mockResolvedValue({ id: 8, email: 'reset@example.com' });
const markEmailVerified = jest.fn().mockResolvedValue(1);
const updateResetToken = jest.fn().mockResolvedValue(undefined);
const userService = {
verifyMagicToken,
getUserById,
updateResetToken,
updateLastLoginAt: jest.fn().mockResolvedValue(undefined),
markEmailVerified,
} as unknown as UsersService;
const authService = {
newJWTToken: jest.fn().mockResolvedValue('jwt-tok'),
persistToken: jest.fn().mockResolvedValue(undefined),
} as unknown as AuthenticationService;
const controller = new UsersController(
userService,
authService,
{} as ReturnType<typeof import('../data_layer').getDatabase>
);
const req = { params: { token: 'reset-tok' } } as unknown as express.Request;
const res = buildVerifyRes();
const next = jest.fn();

await controller.verifyMagicLink(req, res, next);

expect(markEmailVerified).toHaveBeenCalledWith('8');
});
});

describe('UsersController.getLocals', () => {
Expand Down Expand Up @@ -593,97 +677,3 @@ describe('UsersController.getLocals', () => {
});
});

describe('UsersController.resendVerificationEmail', () => {
it('returns 200 with ok:true on success', async () => {
const resendVerificationEmail = jest.fn().mockResolvedValue({ ok: true });
const userService = { resendVerificationEmail } as unknown as UsersService;
const authService = {} as unknown as AuthenticationService;
const controller = new UsersController(
userService,
authService,
{} as ReturnType<typeof import('../data_layer').getDatabase>
);
const req = {} as express.Request;
const res = {
locals: { owner: 5 },
json: jest.fn(),
status: jest.fn().mockReturnThis(),
} as unknown as express.Response & { json: jest.Mock; status: jest.Mock };
const next = jest.fn();

await controller.resendVerificationEmail(req, res, next);

expect(res.json).toHaveBeenCalledWith({ ok: true });
});

it('returns 200 with alreadyVerified:true when already verified', async () => {
const resendVerificationEmail = jest
.fn()
.mockResolvedValue({ ok: true, alreadyVerified: true });
const userService = { resendVerificationEmail } as unknown as UsersService;
const authService = {} as unknown as AuthenticationService;
const controller = new UsersController(
userService,
authService,
{} as ReturnType<typeof import('../data_layer').getDatabase>
);
const req = {} as express.Request;
const res = {
locals: { owner: 5 },
json: jest.fn(),
status: jest.fn().mockReturnThis(),
} as unknown as express.Response & { json: jest.Mock; status: jest.Mock };
const next = jest.fn();

await controller.resendVerificationEmail(req, res, next);

expect(res.json).toHaveBeenCalledWith({ ok: true, alreadyVerified: true });
});

it('returns 429 when rate limited', async () => {
const resendVerificationEmail = jest
.fn()
.mockRejectedValue(new MagicLinkRateLimitError());
const userService = { resendVerificationEmail } as unknown as UsersService;
const authService = {} as unknown as AuthenticationService;
const controller = new UsersController(
userService,
authService,
{} as ReturnType<typeof import('../data_layer').getDatabase>
);
const req = {} as express.Request;
const json = jest.fn();
const res = {
locals: { owner: 5 },
json,
status: jest.fn().mockReturnValue({ json }),
} as unknown as express.Response & { json: jest.Mock; status: jest.Mock };
const next = jest.fn();

await controller.resendVerificationEmail(req, res, next);

expect(res.status).toHaveBeenCalledWith(429);
});

it('returns 401 when not authenticated', async () => {
const userService = {} as unknown as UsersService;
const authService = {} as unknown as AuthenticationService;
const controller = new UsersController(
userService,
authService,
{} as ReturnType<typeof import('../data_layer').getDatabase>
);
const req = {} as express.Request;
const json = jest.fn();
const res = {
locals: {},
json,
status: jest.fn().mockReturnValue({ json }),
} as unknown as express.Response & { json: jest.Mock; status: jest.Mock };
const next = jest.fn();

await controller.resendVerificationEmail(req, res, next);

expect(res.status).toHaveBeenCalledWith(401);
});
});
43 changes: 9 additions & 34 deletions src/controllers/UsersControllers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ class UsersController {
await this.authService.persistToken(token, newUser.id.toString());
await this.userService.updateLastLoginAt(newUser.id.toString());
res.cookie('token', token);
return res.status(200).json({ token, verificationPending: true });
return res.status(200).json({ token });
}
}
res.status(200).json({ message: 'ok' });
Expand Down Expand Up @@ -547,7 +547,7 @@ class UsersController {
const isNewUser = !user;
if (!user) {
const hashedPassword = this.authService.getHashPassword(getRandomUUID());
await this.userService.register(name ?? email, hashedPassword, email, null, true);
await this.userService.register(name ?? email, hashedPassword, email, null);
user = await this.userService.getUserFrom(email);
}
if (isNewUser && user) {
Expand Down Expand Up @@ -665,54 +665,27 @@ class UsersController {
return res.status(200).json({ message: 'ok' });
}

async resendVerificationEmail(
_req: express.Request,
res: express.Response,
next: express.NextFunction
) {
const { owner } = res.locals;
if (owner == null) {
return res.status(401).json({ message: 'Authentication required' });
}

try {
const result = await this.userService.resendVerificationEmail(owner.toString());
return res.json(result);
} catch (error) {
if (error instanceof MagicLinkRateLimitError) {
return res.status(429).json({ message: 'Too many requests. Try again in a minute.' });
}
next(error);
}
}

async verifyEmail(
req: express.Request,
res: express.Response,
next: express.NextFunction
) {
const { token } = req.params;

const expiredRedirect = async () => {
const sessionUser = await this.authService.getUserFrom(req.cookies?.token);
return sessionUser != null
? res.redirect('/account?verify_error=expired')
: res.redirect('/login?verify_error=expired');
};
const sessionUser = await this.authService.getUserFrom(req.cookies?.token);
const base = sessionUser ? '/account' : '/login';

if (token == null || token.length === 0) {
return expiredRedirect();
return res.redirect(`${base}?verify_error=expired`);
}

try {
const result = await this.userService.verifyMagicToken(token);
if (result?.purpose !== 'verify_email') {
return expiredRedirect();
return res.redirect(`${base}?verify_error=expired`);
}
await this.userService.markEmailVerified(result.userId.toString());
return res.redirect('/downloads?verified=1');
return res.redirect(`${base}?verified=1`);
} catch (error) {
console.error('Email verification failed:', error);
next(error);
}
}
Expand Down Expand Up @@ -747,6 +720,7 @@ class UsersController {
const jwtToken = await this.authService.newJWTToken(user.id);
await this.authService.persistToken(jwtToken, user.id.toString());
await this.userService.updateLastLoginAt(user.id.toString());
await this.userService.markEmailVerified(user.id.toString());
res.cookie('token', jwtToken);
return res.status(200).json({ token: jwtToken });
}
Expand All @@ -760,6 +734,7 @@ class UsersController {
}
const resetToken = crypto.randomUUID();
await this.userService.updateResetToken(user.id.toString(), resetToken);
await this.userService.markEmailVerified(user.id.toString());
return res.status(200).json({ purpose: 'password_reset', reset_token: resetToken });
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ function buildEmailService(): jest.Mocked<IEmailService> {
sendMagicLinkEmail: jest.fn(),
sendReEngagementEmail: jest.fn().mockResolvedValue(undefined),
sendInactivityWarningEmail: jest.fn().mockResolvedValue(undefined),
sendVerificationEmail: jest.fn().mockResolvedValue(undefined),
sendAbandonedCheckoutRecoveryEmail: jest.fn().mockResolvedValue(undefined),
};
}
Expand Down
12 changes: 3 additions & 9 deletions src/routes/UserRouter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,8 @@ const UserRouter = () => {
* @swagger
* /api/users/verify/{token}:
* get:
* summary: Verify email address
* description: Validates an email verification token, marks the user's email as verified, and redirects to the app.
* summary: Honor in-flight email verification links
* description: Kept alive for verify_email tokens issued before email verification was removed. New signups no longer receive these.
* tags: [Authentication]
* parameters:
* - in: path
Expand All @@ -203,7 +203,7 @@ const UserRouter = () => {
* type: string
* responses:
* 302:
* description: Redirects to /uploads on success, or /login?error=verification-expired on failure
* description: Redirects to /login?verified=1 (or /account?verified=1 if signed in) on success; same paths with verify_error=expired on failure
*/
router.get('/api/users/verify/:token', (req, res, next) =>
controller.verifyEmail(req, res, next)
Expand Down Expand Up @@ -545,12 +545,6 @@ const UserRouter = () => {
(req, res) => controller.startTrial(req, res)
);

router.post(
'/api/users/resend-verification',
RequireAuthentication,
(req, res, next) => controller.resendVerificationEmail(req, res, next)
);

/**
* @swagger
* /login:
Expand Down
Loading