From 84a41a4da80ed1ce08becdd9256aeb84a0e17203 Mon Sep 17 00:00:00 2001 From: Oliver Eyton-Williams Date: Mon, 3 Jun 2024 09:08:42 +0200 Subject: [PATCH] feat(api): finish update-my-email endpoint (#54921) Co-authored-by: Shaun Hamilton --- api/src/plugins/code-flow-auth.test.ts | 4 +- api/src/plugins/code-flow-auth.ts | 36 +++-- api/src/routes/settings.test.ts | 142 ++++++++++++++------ api/src/routes/settings.ts | 60 +++++++-- api/src/schemas/settings/update-my-email.ts | 6 +- api/src/utils/tokens.test.ts | 28 +++- api/src/utils/tokens.ts | 22 ++- 7 files changed, 217 insertions(+), 81 deletions(-) diff --git a/api/src/plugins/code-flow-auth.test.ts b/api/src/plugins/code-flow-auth.test.ts index 22e2fb3cc0c..c2f0683ba9b 100644 --- a/api/src/plugins/code-flow-auth.test.ts +++ b/api/src/plugins/code-flow-auth.test.ts @@ -2,7 +2,7 @@ import Fastify, { FastifyInstance } from 'fastify'; import jwt from 'jsonwebtoken'; import { COOKIE_DOMAIN, JWT_SECRET } from '../utils/env'; -import { AccessToken, createAccessToken } from '../utils/tokens'; +import { type Token, createAccessToken } from '../utils/tokens'; import cookies, { sign as signCookie, unsign as unsignCookie } from './cookies'; import codeFlowAuth from './code-flow-auth'; @@ -38,7 +38,7 @@ describe('auth', () => { const { value, ...rest } = res.cookies[0]!; const unsignedOnce = unsignCookie(value); const unsignedTwice = jwt.verify(unsignedOnce.value!, JWT_SECRET) as { - accessToken: AccessToken; + accessToken: Token; }; expect(unsignedTwice.accessToken).toEqual(token); expect(rest).toEqual({ diff --git a/api/src/plugins/code-flow-auth.ts b/api/src/plugins/code-flow-auth.ts index ef0def1710b..6b5c2dc1632 100644 --- a/api/src/plugins/code-flow-auth.ts +++ b/api/src/plugins/code-flow-auth.ts @@ -5,14 +5,11 @@ import { isBefore } from 'date-fns'; import { type user } from '@prisma/client'; import { COOKIE_DOMAIN, JWT_SECRET } from '../utils/env'; -import { AccessToken } from '../utils/tokens'; +import { type Token } from '../utils/tokens'; declare module 'fastify' { interface FastifyReply { - setAccessTokenCookie: ( - this: FastifyReply, - accessToken: AccessToken - ) => void; + setAccessTokenCookie: (this: FastifyReply, accessToken: Token) => void; } interface FastifyRequest { @@ -26,21 +23,18 @@ declare module 'fastify' { } const codeFlowAuth: FastifyPluginCallback = (fastify, _options, done) => { - fastify.decorateReply( - 'setAccessTokenCookie', - function (accessToken: AccessToken) { - const signedToken = jwt.sign({ accessToken }, JWT_SECRET); - void this.setCookie('jwt_access_token', signedToken, { - path: '/', - httpOnly: false, - secure: false, - sameSite: 'lax', - domain: COOKIE_DOMAIN, - signed: true, - maxAge: accessToken.ttl - }); - } - ); + fastify.decorateReply('setAccessTokenCookie', function (accessToken: Token) { + const signedToken = jwt.sign({ accessToken }, JWT_SECRET); + void this.setCookie('jwt_access_token', signedToken, { + path: '/', + httpOnly: false, + secure: false, + sameSite: 'lax', + domain: COOKIE_DOMAIN, + signed: true, + maxAge: accessToken.ttl + }); + }); const TOKEN_REQUIRED = 'Access token is required for this request'; const TOKEN_INVALID = 'Your access token is invalid'; @@ -68,7 +62,7 @@ const codeFlowAuth: FastifyPluginCallback = (fastify, _options, done) => { const { accessToken: { created, ttl, userId } - } = jwt.decode(jwtAccessToken!) as { accessToken: AccessToken }; + } = jwt.decode(jwtAccessToken!) as { accessToken: Token }; const valid = isBefore(Date.now(), Date.parse(created) + ttl); if (!valid) return send401(reply, TOKEN_EXPIRED); diff --git a/api/src/routes/settings.test.ts b/api/src/routes/settings.test.ts index 4234aac421c..90b49f4ab18 100644 --- a/api/src/routes/settings.test.ts +++ b/api/src/routes/settings.test.ts @@ -1,11 +1,13 @@ +/* eslint-disable @typescript-eslint/no-unsafe-assignment */ import { devLogin, setupServer, superRequest, - createSuperRequest + createSuperRequest, + defaultUserId } from '../../jest.utils'; import { createUserInput } from '../utils/create-user'; - +import { API_LOCATION } from '../utils/env'; import { isPictureWithProtocol, getWaitMessage } from './settings'; const baseProfileUI = { @@ -171,6 +173,7 @@ describe('settingRoutes', () => { }); describe('/update-my-email', () => { + let sendEmailSpy: jest.SpyInstance; beforeEach(async () => { await fastifyTestInstance.prisma.user.updateMany({ where: { email: developerUserEmail }, @@ -181,47 +184,56 @@ describe('settingRoutes', () => { emailAuthLinkTTL: null } }); + + sendEmailSpy = jest + .spyOn(fastifyTestInstance, 'sendEmail') + .mockImplementationOnce(jest.fn()); }); - test('PUT returns 200 status code with "success" message', async () => { + + afterEach(async () => { + jest.clearAllMocks(); + await fastifyTestInstance.prisma.authToken.deleteMany({ + where: { userId: defaultUserId } + }); + }); + test('PUT returns 200 status code with "info" message', async () => { const response = await superPut('/update-my-email').send({ email: 'foo@foo.com' }); - expect(response?.body).toEqual({ - message: 'flash.email-valid', - type: 'success' + expect(response.body).toEqual({ + message: + 'Check your email and click the link we sent you to confirm your new email address.', + type: 'info' }); - expect(response?.statusCode).toEqual(200); + expect(response.statusCode).toEqual(200); }); test("PUT updates the user's record in preparation for receiving auth email", async () => { + const timeBefore = Date.now(); const response = await superPut('/update-my-email').send({ email: unusedEmailOne }); const user = await fastifyTestInstance.prisma.user.findFirstOrThrow({ where: { email: developerUserEmail }, - select: { emailVerifyTTL: true, emailVerified: true, newEmail: true } + select: { + emailAuthLinkTTL: true, + emailVerifyTTL: true, + emailVerified: true, + newEmail: true + } }); - const emailVerifyTTL = user?.emailVerifyTTL; - expect(emailVerifyTTL).toBeTruthy(); - // This throw is to mollify TS (if this is necessary a lot, create a - // helper) - if (!emailVerifyTTL) { - throw new Error('emailVerifyTTL is not defined'); - } - expect(response?.statusCode).toEqual(200); + // expect the emailVerifyTTL and emailAuthLinkTTL to be set to the current time + expect(user.emailVerifyTTL!.getTime()).toBeGreaterThan(timeBefore); + expect(user.emailVerifyTTL!.getTime()).toBeLessThan(Date.now()); + expect(user.emailAuthLinkTTL!.getTime()).toBeGreaterThan(timeBefore); + expect(user.emailAuthLinkTTL!.getTime()).toBeLessThan(Date.now()); - // expect the emailVerifyTTL to be within 10 seconds of the current time - const tenSeconds = 10 * 1000; - expect(emailVerifyTTL.getTime()).toBeGreaterThan( - Date.now() - tenSeconds - ); - expect(emailVerifyTTL.getTime()).toBeLessThan(Date.now() + tenSeconds); - - expect(user?.emailVerified).toEqual(false); - expect(user?.newEmail).toEqual(unusedEmailOne); + expect(user.emailVerified).toEqual(false); + expect(user.newEmail).toEqual(unusedEmailOne); + expect(response.statusCode).toEqual(200); }); test('PUT rejects invalid email addresses', async () => { @@ -231,11 +243,11 @@ describe('settingRoutes', () => { // We cannot use fastify's default validation failure response here // because the client consumes the response and displays it to the user. - expect(response?.body).toEqual({ + expect(response.body).toEqual({ type: 'danger', message: 'Email format is invalid' }); - expect(response?.statusCode).toEqual(400); + expect(response.statusCode).toEqual(400); }); test('PUT accepts requests to update to the current email address (ignoring case) if it is not verified', async () => { @@ -247,10 +259,11 @@ describe('settingRoutes', () => { email: developerUserEmail.toUpperCase() }); - expect(response?.statusCode).toEqual(200); - expect(response?.body).toEqual({ - message: 'flash.email-valid', - type: 'success' + expect(response.statusCode).toEqual(200); + expect(response.body).toEqual({ + message: + 'Check your email and click the link we sent you to confirm your new email address.', + type: 'info' }); }); @@ -259,12 +272,12 @@ describe('settingRoutes', () => { email: developerUserEmail.toUpperCase() }); - expect(response?.body).toEqual({ + expect(response.body).toEqual({ type: 'info', message: `${developerUserEmail} is already associated with this account. You can update a new email address instead.` }); - expect(response?.statusCode).toEqual(400); + expect(response.statusCode).toEqual(400); }); test('PUT rejects a request to update to the same email (ignoring case) twice', async () => { @@ -272,7 +285,7 @@ You can update a new email address instead.` email: unusedEmailOne }); - expect(successResponse?.statusCode).toEqual(200); + expect(successResponse.statusCode).toEqual(200); const failResponse = await superPut('/update-my-email').send({ email: unusedEmailOne.toUpperCase() @@ -291,11 +304,11 @@ Please wait 5 minutes to resend an authentication link.` email: otherDeveloperUserEmail }); - expect(response?.body).toEqual({ + expect(response.body).toEqual({ type: 'info', message: `${otherDeveloperUserEmail} is already associated with another account.` }); - expect(response?.statusCode).toEqual(400); + expect(response.statusCode).toEqual(400); }); test('PUT rejects the second request if is immediately after the first', async () => { @@ -303,7 +316,7 @@ Please wait 5 minutes to resend an authentication link.` email: unusedEmailOne }); - expect(successResponse?.statusCode).toEqual(200); + expect(successResponse.statusCode).toEqual(200); const failResponse = await superPut('/update-my-email').send({ email: unusedEmailTwo @@ -317,7 +330,60 @@ Please wait 5 minutes to resend an authentication link.` }); }); - // TODO: test that the correct email gets sent + test('PUT sends an email to the new email address', async () => { + jest + .spyOn(fastifyTestInstance.prisma.authToken, 'create') + .mockImplementationOnce(() => + // @ts-expect-error This is a mock implementation, all we're + // interested in is the id. + Promise.resolve({ + id: '123' + }) + ); + await superPut('/update-my-email').send({ + email: unusedEmailOne + }); + + const expectedLink = `${API_LOCATION}/confirm-email?email=${Buffer.from(unusedEmailOne).toString('base64')}&token=123&emailChange=true`; + expect(sendEmailSpy).toHaveBeenCalledWith({ + from: 'team@freecodecamp.org', + to: unusedEmailOne, + subject: + 'Please confirm your updated email address for freeCodeCamp.org', + text: `Please confirm this email address for freeCodeCamp.org: + +${expectedLink} + +Happy coding! + +- The freeCodeCamp.org Team +` + }); + }); + + test('PUT creates an auth token record for the requesting user', async () => { + const noToken = await fastifyTestInstance.prisma.authToken.findFirst({ + where: { userId: defaultUserId } + }); + expect(noToken).toBeNull(); + + await superPut('/update-my-email').send({ + email: unusedEmailOne + }); + + const token = await fastifyTestInstance.prisma.authToken.findFirst({ + where: { userId: defaultUserId } + }); + + expect(token).toEqual({ + ttl: 15 * 60 * 1000, + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + created: expect.any(Date), + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + id: expect.any(String), + userId: defaultUserId + }); + }); }); describe('/update-my-theme', () => { diff --git a/api/src/routes/settings.ts b/api/src/routes/settings.ts index c80f7f4eddb..1a0290e0285 100644 --- a/api/src/routes/settings.ts +++ b/api/src/routes/settings.ts @@ -19,6 +19,8 @@ import { isProfane } from 'no-profanity'; import { blocklistedUsernames } from '../../../shared/config/constants'; import { isValidUsername } from '../../../shared/utils/validate'; import * as schemas from '../schemas'; +import { createAuthToken } from '../utils/tokens'; +import { API_LOCATION } from '../utils/env'; type WaitMesssageArgs = { sentAt: Date | null; @@ -154,6 +156,18 @@ export const settingRoutes: FastifyPluginCallbackTypebox = ( } ); + function createUpdateEmailText({ email, id }: { email: string; id: string }) { + const encodedEmail = Buffer.from(email).toString('base64'); + return `Please confirm this email address for freeCodeCamp.org: + +${API_LOCATION}/confirm-email?email=${encodedEmail}&token=${id}&emailChange=true + +Happy coding! + +- The freeCodeCamp.org Team +`; + } + fastify.put( '/update-my-email', { @@ -170,6 +184,7 @@ export const settingRoutes: FastifyPluginCallbackTypebox = ( const user = await fastify.prisma.user.findUniqueOrThrow({ where: { id: req.user?.id }, select: { + id: true, email: true, emailVerifyTTL: true, newEmail: true, @@ -183,11 +198,11 @@ export const settingRoutes: FastifyPluginCallbackTypebox = ( const isOwnEmail = newEmail === currentEmailFormatted; if (isOwnEmail && isVerifiedEmail) { void reply.code(400); - return { + return reply.send({ type: 'info', message: `${newEmail} is already associated with this account. You can update a new email address instead.` - } as const; + }); } const isResendUpdateToSameEmail = @@ -198,11 +213,11 @@ You can update a new email address instead.` if (isResendUpdateToSameEmail && isLinkSentWithinLimitTTL) { void reply.code(429); - return { + return reply.send({ type: 'info', message: `We have already sent an email confirmation request to ${newEmail}. ${isLinkSentWithinLimitTTL}` - } as const; + }); } const isEmailAlreadyTaken = @@ -210,16 +225,16 @@ ${isLinkSentWithinLimitTTL}` if (isEmailAlreadyTaken && !isOwnEmail) { void reply.code(400); - return { + return reply.send({ type: 'info', message: `${newEmail} is already associated with another account.` - } as const; + }); } // ToDo(MVP): email the new email and wait user to confirm it, before we update the user schema. try { await fastify.prisma.user.update({ - where: { id: req.user?.id }, + where: { id: user.id }, data: { newEmail, emailVerified: false, @@ -237,24 +252,45 @@ ${isLinkSentWithinLimitTTL}` if (tooManyRequestsMessage) { void reply.code(429); - return { + return reply.send({ type: 'info', message: tooManyRequestsMessage - } as const; + }); } + // Update the emailAuthLinkTTL to ensure we don't send too many emails. await fastify.prisma.user.update({ - where: { id: req.user?.id }, + where: { id: user.id }, data: { emailAuthLinkTTL: new Date() } }); - return { message: 'flash.email-valid', type: 'success' } as const; + // The auth token is used to confirm that the user owns the email. If + // the user provides the correct id (by following the link we send + // them), then we can update the email. + const { id } = await fastify.prisma.authToken.create({ + data: createAuthToken(user.id), + select: { id: true } + }); + + await fastify.sendEmail({ + from: 'team@freecodecamp.org', + to: newEmail, + subject: + 'Please confirm your updated email address for freeCodeCamp.org', + text: createUpdateEmailText({ email: newEmail, id }) + }); + + await reply.send({ + message: + 'Check your email and click the link we sent you to confirm your new email address.', + type: 'info' + }); } catch (err) { fastify.log.error(err); void reply.code(500); - return { message: 'flash.wrong-updating', type: 'danger' } as const; + await reply.send({ message: 'flash.wrong-updating', type: 'danger' }); } } ); diff --git a/api/src/schemas/settings/update-my-email.ts b/api/src/schemas/settings/update-my-email.ts index 610e3b15572..ed76cc9e2a8 100644 --- a/api/src/schemas/settings/update-my-email.ts +++ b/api/src/schemas/settings/update-my-email.ts @@ -6,8 +6,10 @@ export const updateMyEmail = { }), response: { 200: Type.Object({ - message: Type.Literal('flash.email-valid'), - type: Type.Literal('success') + message: Type.Literal( + 'Check your email and click the link we sent you to confirm your new email address.' + ), + type: Type.Literal('info') }), '4xx': Type.Object({ message: Type.String(), diff --git a/api/src/utils/tokens.test.ts b/api/src/utils/tokens.test.ts index b79877c36b5..10576cb1781 100644 --- a/api/src/utils/tokens.test.ts +++ b/api/src/utils/tokens.test.ts @@ -1,5 +1,5 @@ /* eslint-disable @typescript-eslint/no-unsafe-assignment */ -import { createAccessToken } from './tokens'; +import { createAccessToken, createAuthToken } from './tokens'; describe('createAccessToken', () => { it('creates an object with id, ttl, created and userId', () => { @@ -26,3 +26,29 @@ describe('createAccessToken', () => { expect(createAccessToken(userId).ttl).toBe(77760000000); }); }); + +describe('createAuthToken', () => { + it('creates an object with id, ttl, created and userId', () => { + const userId = 'abc'; + + const actual = createAuthToken(userId); + + expect(actual).toStrictEqual({ + id: expect.stringMatching(/[a-zA-Z0-9]{64}/), + ttl: 900000, + created: expect.stringMatching( + /\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z/ + ), + userId + }); + }); + + it('sets the ttl, defaulting to 900000 ms', () => { + const userId = 'abc'; + const ttl = 123; + const actual = createAuthToken(userId, ttl); + + expect(actual.ttl).toBe(ttl); + expect(createAuthToken(userId).ttl).toBe(900000); + }); +}); diff --git a/api/src/utils/tokens.ts b/api/src/utils/tokens.ts index 553ea61b854..543a3c612a4 100644 --- a/api/src/utils/tokens.ts +++ b/api/src/utils/tokens.ts @@ -13,7 +13,7 @@ export function encodeUserToken(userToken: string): string { return jwt.sign({ userToken }, JWT_SECRET); } -export type AccessToken = { +export type Token = { userId: string; id: string; ttl: number; @@ -26,10 +26,7 @@ export type AccessToken = { * @param ttl The time to live for the token in milliseconds (default: 77760000000). * @returns The access token. */ -export const createAccessToken = ( - userId: string, - ttl?: number -): AccessToken => { +export const createAccessToken = (userId: string, ttl?: number): Token => { return { userId, id: customNanoid(), @@ -37,3 +34,18 @@ export const createAccessToken = ( created: new Date().toISOString() }; }; + +/** + * Creates an auth token. + * @param userId The user ID as a string (yes, it's an ObjectID, but it will be serialized to a string anyway). + * @param ttl The time to live for the token in milliseconds (default: 900000 aka 15 minutes). + * @returns The access token. + */ +export const createAuthToken = (userId: string, ttl?: number): Token => { + return { + userId, + id: customNanoid(), + ttl: ttl ?? 900000, + created: new Date().toISOString() + }; +};