From 848ae3aacf11cc2f1e612fbc873501b39110f780 Mon Sep 17 00:00:00 2001 From: Oliver Eyton-Williams Date: Thu, 22 May 2025 11:28:56 +0200 Subject: [PATCH] fix(api): handle users without email addresses (#60467) --- api/jest.utils.ts | 4 ++++ api/prisma/schema.prisma | 2 +- api/src/routes/protected/certificate.ts | 6 ++--- api/src/routes/protected/donate.test.ts | 23 ++++++++++++++---- api/src/routes/protected/donate.ts | 11 +++++++++ api/src/routes/protected/settings.ts | 2 +- api/src/routes/protected/user.test.ts | 24 +++++++++++++++++-- api/src/routes/protected/user.ts | 16 +++++++++++-- .../routes/public/email-subscription.test.ts | 4 ++-- api/src/schemas/donate/charge-stripe-card.ts | 6 +++++ api/src/utils/create-user.ts | 3 +-- 11 files changed, 84 insertions(+), 17 deletions(-) diff --git a/api/jest.utils.ts b/api/jest.utils.ts index 1b47b0f04c2..ffb18c193ca 100644 --- a/api/jest.utils.ts +++ b/api/jest.utils.ts @@ -206,6 +206,10 @@ export const resetDefaultUser = async (): Promise => { where: { userId: defaultUserId } } ); + await fastifyTestInstance.prisma.user.deleteMany({ + where: { id: defaultUserId } + }); + await fastifyTestInstance.prisma.user.deleteMany({ where: { email: defaultUserEmail } }); diff --git a/api/prisma/schema.prisma b/api/prisma/schema.prisma index 72d8bf41bf8..e748beef93c 100644 --- a/api/prisma/schema.prisma +++ b/api/prisma/schema.prisma @@ -99,7 +99,7 @@ model user { quizAttempts QuizAttempt[] // Undefined currentChallengeId String? donationEmails String[] // Undefined | String[] (only possible for built in Types like String) - email String + email String? emailAuthLinkTTL DateTime? // Null | Undefined emailVerified Boolean? emailVerifyTTL DateTime? // Null | Undefined diff --git a/api/src/routes/protected/certificate.ts b/api/src/routes/protected/certificate.ts index 0e1ea1d2c1a..1f81998bdb0 100644 --- a/api/src/routes/protected/certificate.ts +++ b/api/src/routes/protected/certificate.ts @@ -394,8 +394,8 @@ export const protectedCertificateRoutes: FastifyPluginCallbackTypebox = ( } }); + const email = updatedUser.email; const updatedUserSansNull = removeNulls(updatedUser); - const updatedIsCertMap = getUserIsCertMap(updatedUserSansNull); // TODO(POST-MVP): Consider sending email based on `user.isEmailVerified` as well @@ -403,11 +403,11 @@ export const protectedCertificateRoutes: FastifyPluginCallbackTypebox = ( .map(x => certSlugTypeMap[x]) .every(certType => updatedIsCertMap[certType]); const shouldSendCertifiedEmailToCamper = - isEmail(updatedUser.email) && hasCompletedAllCerts; + email && isEmail(email) && hasCompletedAllCerts; if (shouldSendCertifiedEmailToCamper) { const notifyUser = { - to: updatedUser.email, + to: email, from: 'quincy@freecodecamp.org', subject: 'Congratulations on completing all of the freeCodeCamp certifications!', diff --git a/api/src/routes/protected/donate.test.ts b/api/src/routes/protected/donate.test.ts index 7dafd19c30c..d938f561c6c 100644 --- a/api/src/routes/protected/donate.test.ts +++ b/api/src/routes/protected/donate.test.ts @@ -1,4 +1,3 @@ -import type { Prisma } from '@prisma/client'; import { createSuperRequest, devLogin, @@ -11,9 +10,8 @@ import { createUserInput } from '../../utils/create-user'; const testEWalletEmail = 'baz@bar.com'; const testSubscriptionId = 'sub_test_id'; const testCustomerId = 'cust_test_id'; -const userWithoutProgress: Prisma.userCreateInput = - createUserInput(defaultUserEmail); -const userWithProgress: Prisma.userCreateInput = { +const userWithoutProgress = createUserInput(defaultUserEmail); +const userWithProgress = { ...createUserInput(defaultUserEmail), completedChallenges: [ { @@ -271,6 +269,23 @@ describe('Donate', () => { expect(failResponse.status).toBe(400); }); + it('should return 403 if the user has no email', async () => { + await fastifyTestInstance.prisma.user.updateMany({ + where: { email: userWithProgress.email }, + data: { email: null } + }); + const response = await superPost('/donate/charge-stripe-card').send( + chargeStripeCardReqBody + ); + expect(response.body).toEqual({ + error: { + type: 'EmailRequiredError', + message: 'User has not provided an email address' + } + }); + expect(response.status).toBe(403); + }); + it('should return 500 if Stripe encountes an error', async () => { mockSubCreate.mockImplementationOnce(defaultError); const response = await superPost('/donate/charge-stripe-card').send( diff --git a/api/src/routes/protected/donate.ts b/api/src/routes/protected/donate.ts index 3533d69e6f5..c64c7aa6fbb 100644 --- a/api/src/routes/protected/donate.ts +++ b/api/src/routes/protected/donate.ts @@ -117,6 +117,17 @@ export const donateRoutes: FastifyPluginCallbackTypebox = ( }); const { email, name } = user; + + if (!email) { + logger.warn(`User ${id} has no email`); + void reply.code(403); + return reply.send({ + error: { + type: 'EmailRequiredError', + message: 'User has not provided an email address' + } + }); + } const threeChallengesCompleted = user.completedChallenges.length >= 3; if (!threeChallengesCompleted) { diff --git a/api/src/routes/protected/settings.ts b/api/src/routes/protected/settings.ts index d2428b3ec36..ef7c4ac911b 100644 --- a/api/src/routes/protected/settings.ts +++ b/api/src/routes/protected/settings.ts @@ -186,7 +186,7 @@ Happy coding! } }); const newEmail = req.body.email.toLowerCase(); - const currentEmailFormatted = user.email.toLowerCase(); + const currentEmailFormatted = user.email ? user.email.toLowerCase() : ''; const isVerifiedEmail = user.emailVerified; const isOwnEmail = newEmail === currentEmailFormatted; if (isOwnEmail && isVerifiedEmail) { diff --git a/api/src/routes/protected/user.test.ts b/api/src/routes/protected/user.test.ts index 45058569b4d..32807f41bd3 100644 --- a/api/src/routes/protected/user.test.ts +++ b/api/src/routes/protected/user.test.ts @@ -14,7 +14,8 @@ import { setupServer, superRequest, createSuperRequest, - defaultUsername + defaultUsername, + resetDefaultUser } from '../../../jest.utils'; import { JWT_SECRET } from '../../utils/env'; import { @@ -864,7 +865,8 @@ describe('userRoutes', () => { .mockImplementation(jest.fn()); }); - afterEach(() => { + afterEach(async () => { + await resetDefaultUser(); jest.clearAllMocks(); }); @@ -890,6 +892,24 @@ describe('userRoutes', () => { expect(response.statusCode).toBe(400); }); + test('POST returns 403 for users with no email', async () => { + await fastifyTestInstance.prisma.user.updateMany({ + where: { email: testUserData.email }, + data: { email: null } + }); + + const response = await superPost('/user/report-user').send({ + username: testUserData.username, + reportDescription: 'Test Report' + }); + + expect(response.statusCode).toBe(403); + expect(response.body).toStrictEqual({ + type: 'danger', + message: 'flash.report-error' + }); + }); + test('POST sanitises report description', async () => { await superPost('/user/report-user').send({ username: defaultUsername, diff --git a/api/src/routes/protected/user.ts b/api/src/routes/protected/user.ts index 10ac8a4e8c4..0d651acea20 100644 --- a/api/src/routes/protected/user.ts +++ b/api/src/routes/protected/user.ts @@ -199,6 +199,16 @@ export const userRoutes: FastifyPluginCallbackTypebox = ( const user = await fastify.prisma.user.findUniqueOrThrow({ where: { id: req.user?.id } }); + + if (!user.email) { + logger.warn('User has no email'); + void reply.code(403); + return reply.send({ + type: 'danger', + message: 'flash.report-error' + }); + } + const { username, reportDescription: report } = req.body; // TODO: `findUnique` once db migration forces unique usernames @@ -242,11 +252,11 @@ export const userRoutes: FastifyPluginCallbackTypebox = ( text: generateReportEmail(user, reportedUser, report) }); - return { + reply.send({ type: 'info', message: 'flash.report-sent', variables: { email: user.email } - } as const; + }); } ); @@ -628,6 +638,7 @@ export const userGetRoutes: FastifyPluginCallbackTypebox = ( const [flags, rest] = splitUser(user); const { + email, emailVerified, username, usernameDisplay, @@ -649,6 +660,7 @@ export const userGetRoutes: FastifyPluginCallbackTypebox = ( ...removeNulls(publicUser), ...normalizeFlags(flags), picture: publicUser.picture ?? '', + email: email ?? '', currentChallengeId: currentChallengeId ?? '', completedChallenges: normalizeChallenges(completedChallenges), completedChallengeCount: completedChallenges.length, diff --git a/api/src/routes/public/email-subscription.test.ts b/api/src/routes/public/email-subscription.test.ts index cd833befc3b..20183d2b04d 100644 --- a/api/src/routes/public/email-subscription.test.ts +++ b/api/src/routes/public/email-subscription.test.ts @@ -101,7 +101,7 @@ describe('Email Subscription endpoints', () => { expect(users).toHaveLength(4); users.forEach(user => { - if (['user1@freecodecamp.org'].includes(user.email)) { + if (['user1@freecodecamp.org'].includes(user.email!)) { expect(user.sendQuincyEmail).toBe(false); } else { expect(user.sendQuincyEmail).toBe(true); @@ -148,7 +148,7 @@ describe('Email Subscription endpoints', () => { users.forEach(user => { if ( ['user1@freecodecamp.org', 'user2@freecodecamp.org'].includes( - user.email + user.email! ) ) { expect(user.sendQuincyEmail).toBe(false); diff --git a/api/src/schemas/donate/charge-stripe-card.ts b/api/src/schemas/donate/charge-stripe-card.ts index ec820456eba..ab4f193922a 100644 --- a/api/src/schemas/donate/charge-stripe-card.ts +++ b/api/src/schemas/donate/charge-stripe-card.ts @@ -30,6 +30,12 @@ export const chargeStripeCard = { client_secret: Type.Optional(Type.String()) }) }), + 403: Type.Object({ + error: Type.Object({ + message: Type.String(), + type: Type.Literal('EmailRequiredError') + }) + }), 500: Type.Object({ error: Type.Literal('Donation failed due to a server error.') }) diff --git a/api/src/utils/create-user.ts b/api/src/utils/create-user.ts index 0c5fb20008b..7d25c378a10 100644 --- a/api/src/utils/create-user.ts +++ b/api/src/utils/create-user.ts @@ -1,6 +1,5 @@ import crypto from 'node:crypto'; -import { type Prisma } from '@prisma/client'; import { customAlphabet } from 'nanoid'; export const nanoidCharSet = @@ -46,7 +45,7 @@ export const createResetProperties = () => ({ * @param email The email address of the new user. * @returns Default data for a new user. */ -export function createUserInput(email: string): Prisma.userCreateInput { +export function createUserInput(email: string) { const username = 'fcc-' + crypto.randomUUID(); const externalId = crypto.randomUUID(); // This explicitly includes all array fields. This is not strictly necessary -