fix(api): handle users without email addresses (#60467)

This commit is contained in:
Oliver Eyton-Williams
2025-05-22 11:28:56 +02:00
committed by GitHub
parent 1311b4e10f
commit 848ae3aacf
11 changed files with 84 additions and 17 deletions
+4
View File
@@ -206,6 +206,10 @@ export const resetDefaultUser = async (): Promise<void> => {
where: { userId: defaultUserId } where: { userId: defaultUserId }
} }
); );
await fastifyTestInstance.prisma.user.deleteMany({
where: { id: defaultUserId }
});
await fastifyTestInstance.prisma.user.deleteMany({ await fastifyTestInstance.prisma.user.deleteMany({
where: { email: defaultUserEmail } where: { email: defaultUserEmail }
}); });
+1 -1
View File
@@ -99,7 +99,7 @@ model user {
quizAttempts QuizAttempt[] // Undefined quizAttempts QuizAttempt[] // Undefined
currentChallengeId String? currentChallengeId String?
donationEmails String[] // Undefined | String[] (only possible for built in Types like String) donationEmails String[] // Undefined | String[] (only possible for built in Types like String)
email String email String?
emailAuthLinkTTL DateTime? // Null | Undefined emailAuthLinkTTL DateTime? // Null | Undefined
emailVerified Boolean? emailVerified Boolean?
emailVerifyTTL DateTime? // Null | Undefined emailVerifyTTL DateTime? // Null | Undefined
+3 -3
View File
@@ -394,8 +394,8 @@ export const protectedCertificateRoutes: FastifyPluginCallbackTypebox = (
} }
}); });
const email = updatedUser.email;
const updatedUserSansNull = removeNulls(updatedUser); const updatedUserSansNull = removeNulls(updatedUser);
const updatedIsCertMap = getUserIsCertMap(updatedUserSansNull); const updatedIsCertMap = getUserIsCertMap(updatedUserSansNull);
// TODO(POST-MVP): Consider sending email based on `user.isEmailVerified` as well // TODO(POST-MVP): Consider sending email based on `user.isEmailVerified` as well
@@ -403,11 +403,11 @@ export const protectedCertificateRoutes: FastifyPluginCallbackTypebox = (
.map(x => certSlugTypeMap[x]) .map(x => certSlugTypeMap[x])
.every(certType => updatedIsCertMap[certType]); .every(certType => updatedIsCertMap[certType]);
const shouldSendCertifiedEmailToCamper = const shouldSendCertifiedEmailToCamper =
isEmail(updatedUser.email) && hasCompletedAllCerts; email && isEmail(email) && hasCompletedAllCerts;
if (shouldSendCertifiedEmailToCamper) { if (shouldSendCertifiedEmailToCamper) {
const notifyUser = { const notifyUser = {
to: updatedUser.email, to: email,
from: 'quincy@freecodecamp.org', from: 'quincy@freecodecamp.org',
subject: subject:
'Congratulations on completing all of the freeCodeCamp certifications!', 'Congratulations on completing all of the freeCodeCamp certifications!',
+19 -4
View File
@@ -1,4 +1,3 @@
import type { Prisma } from '@prisma/client';
import { import {
createSuperRequest, createSuperRequest,
devLogin, devLogin,
@@ -11,9 +10,8 @@ import { createUserInput } from '../../utils/create-user';
const testEWalletEmail = 'baz@bar.com'; const testEWalletEmail = 'baz@bar.com';
const testSubscriptionId = 'sub_test_id'; const testSubscriptionId = 'sub_test_id';
const testCustomerId = 'cust_test_id'; const testCustomerId = 'cust_test_id';
const userWithoutProgress: Prisma.userCreateInput = const userWithoutProgress = createUserInput(defaultUserEmail);
createUserInput(defaultUserEmail); const userWithProgress = {
const userWithProgress: Prisma.userCreateInput = {
...createUserInput(defaultUserEmail), ...createUserInput(defaultUserEmail),
completedChallenges: [ completedChallenges: [
{ {
@@ -271,6 +269,23 @@ describe('Donate', () => {
expect(failResponse.status).toBe(400); 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 () => { it('should return 500 if Stripe encountes an error', async () => {
mockSubCreate.mockImplementationOnce(defaultError); mockSubCreate.mockImplementationOnce(defaultError);
const response = await superPost('/donate/charge-stripe-card').send( const response = await superPost('/donate/charge-stripe-card').send(
+11
View File
@@ -117,6 +117,17 @@ export const donateRoutes: FastifyPluginCallbackTypebox = (
}); });
const { email, name } = user; 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; const threeChallengesCompleted = user.completedChallenges.length >= 3;
if (!threeChallengesCompleted) { if (!threeChallengesCompleted) {
+1 -1
View File
@@ -186,7 +186,7 @@ Happy coding!
} }
}); });
const newEmail = req.body.email.toLowerCase(); const newEmail = req.body.email.toLowerCase();
const currentEmailFormatted = user.email.toLowerCase(); const currentEmailFormatted = user.email ? user.email.toLowerCase() : '';
const isVerifiedEmail = user.emailVerified; const isVerifiedEmail = user.emailVerified;
const isOwnEmail = newEmail === currentEmailFormatted; const isOwnEmail = newEmail === currentEmailFormatted;
if (isOwnEmail && isVerifiedEmail) { if (isOwnEmail && isVerifiedEmail) {
+22 -2
View File
@@ -14,7 +14,8 @@ import {
setupServer, setupServer,
superRequest, superRequest,
createSuperRequest, createSuperRequest,
defaultUsername defaultUsername,
resetDefaultUser
} from '../../../jest.utils'; } from '../../../jest.utils';
import { JWT_SECRET } from '../../utils/env'; import { JWT_SECRET } from '../../utils/env';
import { import {
@@ -864,7 +865,8 @@ describe('userRoutes', () => {
.mockImplementation(jest.fn()); .mockImplementation(jest.fn());
}); });
afterEach(() => { afterEach(async () => {
await resetDefaultUser();
jest.clearAllMocks(); jest.clearAllMocks();
}); });
@@ -890,6 +892,24 @@ describe('userRoutes', () => {
expect(response.statusCode).toBe(400); 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 () => { test('POST sanitises report description', async () => {
await superPost('/user/report-user').send({ await superPost('/user/report-user').send({
username: defaultUsername, username: defaultUsername,
+14 -2
View File
@@ -199,6 +199,16 @@ export const userRoutes: FastifyPluginCallbackTypebox = (
const user = await fastify.prisma.user.findUniqueOrThrow({ const user = await fastify.prisma.user.findUniqueOrThrow({
where: { id: req.user?.id } 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; const { username, reportDescription: report } = req.body;
// TODO: `findUnique` once db migration forces unique usernames // TODO: `findUnique` once db migration forces unique usernames
@@ -242,11 +252,11 @@ export const userRoutes: FastifyPluginCallbackTypebox = (
text: generateReportEmail(user, reportedUser, report) text: generateReportEmail(user, reportedUser, report)
}); });
return { reply.send({
type: 'info', type: 'info',
message: 'flash.report-sent', message: 'flash.report-sent',
variables: { email: user.email } variables: { email: user.email }
} as const; });
} }
); );
@@ -628,6 +638,7 @@ export const userGetRoutes: FastifyPluginCallbackTypebox = (
const [flags, rest] = splitUser(user); const [flags, rest] = splitUser(user);
const { const {
email,
emailVerified, emailVerified,
username, username,
usernameDisplay, usernameDisplay,
@@ -649,6 +660,7 @@ export const userGetRoutes: FastifyPluginCallbackTypebox = (
...removeNulls(publicUser), ...removeNulls(publicUser),
...normalizeFlags(flags), ...normalizeFlags(flags),
picture: publicUser.picture ?? '', picture: publicUser.picture ?? '',
email: email ?? '',
currentChallengeId: currentChallengeId ?? '', currentChallengeId: currentChallengeId ?? '',
completedChallenges: normalizeChallenges(completedChallenges), completedChallenges: normalizeChallenges(completedChallenges),
completedChallengeCount: completedChallenges.length, completedChallengeCount: completedChallenges.length,
@@ -101,7 +101,7 @@ describe('Email Subscription endpoints', () => {
expect(users).toHaveLength(4); expect(users).toHaveLength(4);
users.forEach(user => { users.forEach(user => {
if (['user1@freecodecamp.org'].includes(user.email)) { if (['user1@freecodecamp.org'].includes(user.email!)) {
expect(user.sendQuincyEmail).toBe(false); expect(user.sendQuincyEmail).toBe(false);
} else { } else {
expect(user.sendQuincyEmail).toBe(true); expect(user.sendQuincyEmail).toBe(true);
@@ -148,7 +148,7 @@ describe('Email Subscription endpoints', () => {
users.forEach(user => { users.forEach(user => {
if ( if (
['user1@freecodecamp.org', 'user2@freecodecamp.org'].includes( ['user1@freecodecamp.org', 'user2@freecodecamp.org'].includes(
user.email user.email!
) )
) { ) {
expect(user.sendQuincyEmail).toBe(false); expect(user.sendQuincyEmail).toBe(false);
@@ -30,6 +30,12 @@ export const chargeStripeCard = {
client_secret: Type.Optional(Type.String()) client_secret: Type.Optional(Type.String())
}) })
}), }),
403: Type.Object({
error: Type.Object({
message: Type.String(),
type: Type.Literal('EmailRequiredError')
})
}),
500: Type.Object({ 500: Type.Object({
error: Type.Literal('Donation failed due to a server error.') error: Type.Literal('Donation failed due to a server error.')
}) })
+1 -2
View File
@@ -1,6 +1,5 @@
import crypto from 'node:crypto'; import crypto from 'node:crypto';
import { type Prisma } from '@prisma/client';
import { customAlphabet } from 'nanoid'; import { customAlphabet } from 'nanoid';
export const nanoidCharSet = export const nanoidCharSet =
@@ -46,7 +45,7 @@ export const createResetProperties = () => ({
* @param email The email address of the new user. * @param email The email address of the new user.
* @returns Default data for a 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 username = 'fcc-' + crypto.randomUUID();
const externalId = crypto.randomUUID(); const externalId = crypto.randomUUID();
// This explicitly includes all array fields. This is not strictly necessary - // This explicitly includes all array fields. This is not strictly necessary -