From a08f9df52dbdd8e49279451c2ce30571bd7a0de8 Mon Sep 17 00:00:00 2001 From: Christian Fehmer Date: Thu, 10 Jul 2025 15:41:51 +0200 Subject: [PATCH 1/5] impr: store emailVerified status in database (@fehmer) --- .../__tests__/api/controllers/user.spec.ts | 60 +++++++++++++++++++ backend/__tests__/dal/user.spec.ts | 7 ++- backend/__tests__/middlewares/auth.spec.ts | 3 + backend/src/api/controllers/user.ts | 12 ++++ backend/src/dal/user.ts | 10 +++- backend/src/middlewares/auth.ts | 2 + packages/schemas/src/users.ts | 1 + 7 files changed, 92 insertions(+), 3 deletions(-) diff --git a/backend/__tests__/api/controllers/user.spec.ts b/backend/__tests__/api/controllers/user.spec.ts index f32d40dbe733..c948a326aa43 100644 --- a/backend/__tests__/api/controllers/user.spec.ts +++ b/backend/__tests__/api/controllers/user.spec.ts @@ -253,6 +253,66 @@ describe("user controller test", () => { }); }); }); + describe("getUser", () => { + const getUserMock = vi.spyOn(UserDal, "getUser"); + const updateEmailMock = vi.spyOn(UserDal, "updateEmail"); + beforeEach(() => { + getUserMock.mockReset(); + updateEmailMock.mockReset(); + }); + it("should update emailVerified if undefined", async () => { + //GIVEN + getUserMock.mockResolvedValue({ + uid, + email: "old", + } as any); + mockAuth.modifyToken({ email_verified: false, email: "old" }); + + //WHEN + await mockApp + .get("/users") + .set("Authorization", `Bearer ${uid}`) + .expect(200); + + //THEN + expect(updateEmailMock).toHaveBeenCalledWith(uid, "old", false); + }); + it("should update emailVerified if changed", async () => { + //GIVEN + getUserMock.mockResolvedValue({ + uid, + emailVerified: false, + email: "old", + } as any); + mockAuth.modifyToken({ email_verified: true, email: "next" }); + + //WHEN + await mockApp + .get("/users") + .set("Authorization", `Bearer ${uid}`) + .expect(200); + + //THEN + expect(updateEmailMock).toHaveBeenCalledWith(uid, "next", true); + }); + it("should not update emailVerified if not changed", async () => { + //GIVEN + getUserMock.mockResolvedValue({ + uid, + emailVerified: false, + } as any); + mockAuth.modifyToken({ email_verified: false }); + + //WHEN + await mockApp + .get("/users") + .set("Authorization", `Bearer ${uid}`) + .expect(200); + + //THEN + expect(updateEmailMock).not.toHaveBeenCalled(); + }); + }); describe("sendVerificationEmail", () => { const adminGetUserMock = vi.fn(); const adminGenerateVerificationLinkMock = vi.fn(); diff --git a/backend/__tests__/dal/user.spec.ts b/backend/__tests__/dal/user.spec.ts index d763a06efa6d..aa75bdeffdae 100644 --- a/backend/__tests__/dal/user.spec.ts +++ b/backend/__tests__/dal/user.spec.ts @@ -102,6 +102,7 @@ describe("UserDal", () => { expect(insertedUser.email).toBe(newUser.email); expect(insertedUser.uid).toBe(newUser.uid); expect(insertedUser.name).toBe(newUser.name); + expect(insertedUser.emailVerified).toBe(false); }); it("should error if the user already exists", async () => { @@ -1167,7 +1168,10 @@ describe("UserDal", () => { }); it("should update", async () => { //given - const { uid } = await UserTestData.createUser({ email: "init" }); + const { uid } = await UserTestData.createUser({ + email: "init", + emailVerified: true, + }); //when await expect(UserDAL.updateEmail(uid, "next")).resolves.toBe(true); @@ -1175,6 +1179,7 @@ describe("UserDal", () => { //then const read = await UserDAL.getUser(uid, "read"); expect(read.email).toEqual("next"); + expect(read.emailVerified).toEqual(false); }); }); describe("resetPb", () => { diff --git a/backend/__tests__/middlewares/auth.spec.ts b/backend/__tests__/middlewares/auth.spec.ts index 378714ae0097..e7f892b7e1fc 100644 --- a/backend/__tests__/middlewares/auth.spec.ts +++ b/backend/__tests__/middlewares/auth.spec.ts @@ -20,6 +20,7 @@ const mockDecodedToken: DecodedIdToken = { uid: "123456789", email: "newuser@mail.com", iat: 0, + email_verified: true, } as DecodedIdToken; vi.spyOn(AuthUtils, "verifyIdToken").mockResolvedValue(mockDecodedToken); @@ -62,6 +63,7 @@ describe("middlewares/auth", () => { type: "None", uid: "", email: "", + emailVerified: false, }, }, }; @@ -122,6 +124,7 @@ describe("middlewares/auth", () => { expect(decodedToken?.type).toBe("Bearer"); expect(decodedToken?.email).toBe(mockDecodedToken.email); expect(decodedToken?.uid).toBe(mockDecodedToken.uid); + expect(decodedToken?.emailVerified).toBe(mockDecodedToken.email_verified); expect(nextFunction).toHaveBeenCalledOnce(); expect(prometheusIncrementAuthMock).toHaveBeenCalledWith("Bearer"); diff --git a/backend/src/api/controllers/user.ts b/backend/src/api/controllers/user.ts index 348b66724baa..9380c25f30b1 100644 --- a/backend/src/api/controllers/user.ts +++ b/backend/src/api/controllers/user.ts @@ -605,6 +605,18 @@ export async function getUser(req: MonkeyRequest): Promise { ); delete relevantUserInfo.customThemes; + //update users emailVerified status if it changed + const { email, emailVerified } = req.ctx.decodedToken; + if (emailVerified !== undefined && emailVerified !== userInfo.emailVerified) { + void addImportantLog( + "user_verify_email", + `emailVerified changed to ${emailVerified} for email ${email}`, + uid + ); + await UserDAL.updateEmail(uid, email, emailVerified); + userInfo.emailVerified = emailVerified; + } + const userData: User = { ...relevantUserInfo, resultFilterPresets, diff --git a/backend/src/dal/user.ts b/backend/src/dal/user.ts index 17b0615ad30b..a2bb9cc2015f 100644 --- a/backend/src/dal/user.ts +++ b/backend/src/dal/user.ts @@ -87,6 +87,7 @@ export async function addUser( custom: {}, }, testActivity: {}, + emailVerified: false, }; const result = await getUsersCollection().updateOne( @@ -231,9 +232,14 @@ export async function updateQuoteRatings( export async function updateEmail( uid: string, - email: string + email: string, + emailVerified: boolean = true ): Promise { - await updateUser({ uid }, { $set: { email } }, { stack: "update email" }); + await updateUser( + { uid }, + { $set: { email, emailVerified } }, + { stack: "update email" } + ); return true; } diff --git a/backend/src/middlewares/auth.ts b/backend/src/middlewares/auth.ts index 3c04c981c4b3..e3999050cb9e 100644 --- a/backend/src/middlewares/auth.ts +++ b/backend/src/middlewares/auth.ts @@ -26,6 +26,7 @@ export type DecodedToken = { type: "Bearer" | "ApeKey" | "None" | "GithubWebhook"; uid: string; email: string; + emailVerified?: boolean; }; const DEFAULT_OPTIONS: RequestAuthenticationOptions = { @@ -189,6 +190,7 @@ async function authenticateWithBearerToken( type: "Bearer", uid: decodedToken.uid, email: decodedToken.email ?? "", + emailVerified: decodedToken.email_verified, }; } catch (error) { if ( diff --git a/packages/schemas/src/users.ts b/packages/schemas/src/users.ts index a72ecca0f624..9c0baee763b2 100644 --- a/packages/schemas/src/users.ts +++ b/packages/schemas/src/users.ts @@ -271,6 +271,7 @@ export const UserSchema = z.object({ quoteMod: QuoteModSchema.optional(), resultFilterPresets: z.array(ResultFiltersSchema).optional(), testActivity: TestActivitySchema.optional(), + emailVerified: z.boolean().optional(), }); export type User = z.infer; From e04a7e66cf7633fc51a00569429aa6a180c3f2fc Mon Sep 17 00:00:00 2001 From: Christian Fehmer Date: Thu, 10 Jul 2025 19:01:29 +0200 Subject: [PATCH 2/5] fix --- backend/__tests__/dal/user.spec.ts | 16 ++++++++++++++++ backend/src/dal/user.ts | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/backend/__tests__/dal/user.spec.ts b/backend/__tests__/dal/user.spec.ts index aa75bdeffdae..992e9737b490 100644 --- a/backend/__tests__/dal/user.spec.ts +++ b/backend/__tests__/dal/user.spec.ts @@ -1181,6 +1181,22 @@ describe("UserDal", () => { expect(read.email).toEqual("next"); expect(read.emailVerified).toEqual(false); }); + + it("should update email and isVerified", async () => { + //given + const { uid } = await UserTestData.createUser({ + email: "init", + emailVerified: false, + }); + + //when + await expect(UserDAL.updateEmail(uid, "next", true)).resolves.toBe(true); + + //then + const read = await UserDAL.getUser(uid, "read"); + expect(read.email).toEqual("next"); + expect(read.emailVerified).toEqual(true); + }); }); describe("resetPb", () => { it("throws for nonexisting user", async () => { diff --git a/backend/src/dal/user.ts b/backend/src/dal/user.ts index a2bb9cc2015f..3f91417d6549 100644 --- a/backend/src/dal/user.ts +++ b/backend/src/dal/user.ts @@ -233,7 +233,7 @@ export async function updateQuoteRatings( export async function updateEmail( uid: string, email: string, - emailVerified: boolean = true + emailVerified: boolean = false ): Promise { await updateUser( { uid }, From 995af2b1fa74e54c54d76d99a4c3cb5648f5ef2f Mon Sep 17 00:00:00 2001 From: Christian Fehmer Date: Mon, 14 Jul 2025 13:00:26 +0200 Subject: [PATCH 3/5] new endpoint to update emailVerified from email-handler --- backend/src/api/controllers/user.ts | 10 +++++++++- backend/src/api/routes/users.ts | 3 +++ frontend/src/email-handler.html | 17 ++++++++++++++++- packages/contracts/src/rate-limit/index.ts | 5 +++++ packages/contracts/src/users.ts | 13 +++++++++++++ 5 files changed, 46 insertions(+), 2 deletions(-) diff --git a/backend/src/api/controllers/user.ts b/backend/src/api/controllers/user.ts index 9380c25f30b1..1e398ce611ea 100644 --- a/backend/src/api/controllers/user.ts +++ b/backend/src/api/controllers/user.ts @@ -158,6 +158,7 @@ export async function sendVerificationEmail( ); }) ).emailVerified; + if (isVerified) { throw new MonkeyError(400, "Email already verified"); } @@ -240,6 +241,13 @@ export async function sendVerificationEmail( return new MonkeyResponse("Email sent", null); } +export async function verifyEmail(req: MonkeyRequest): Promise { + const { uid, email, emailVerified } = req.ctx.decodedToken; + await UserDAL.updateEmail(uid, email, emailVerified); + + return new MonkeyResponse("emailVerify updated.", null); +} + export async function sendForgotPasswordEmail( req: MonkeyRequest ): Promise { @@ -605,7 +613,7 @@ export async function getUser(req: MonkeyRequest): Promise { ); delete relevantUserInfo.customThemes; - //update users emailVerified status if it changed + // soft-migrate user.emailVerified for existing users, update status if it has changed const { email, emailVerified } = req.ctx.decodedToken; if (emailVerified !== undefined && emailVerified !== userInfo.emailVerified) { void addImportantLog( diff --git a/backend/src/api/routes/users.ts b/backend/src/api/routes/users.ts index 8e34831a8c0d..e78fec04beb9 100644 --- a/backend/src/api/routes/users.ts +++ b/backend/src/api/routes/users.ts @@ -120,6 +120,9 @@ export default s.router(usersContract, { handler: async (r) => callController(UserController.sendVerificationEmail)(r), }, + verifyEmail: { + handler: async (r) => callController(UserController.verifyEmail)(r), + }, forgotPasswordEmail: { handler: async (r) => callController(UserController.sendForgotPasswordEmail)(r), diff --git a/frontend/src/email-handler.html b/frontend/src/email-handler.html index 78a4529da2a5..2dee2a9d3edb 100644 --- a/frontend/src/email-handler.html +++ b/frontend/src/email-handler.html @@ -176,6 +176,8 @@ signInWithEmailAndPassword, } from "firebase/auth"; + import { envConfig } from "./ts/constants/env-config"; + function isPasswordStrong(password) { const hasCapital = !!password.match(/[A-Z]/); const hasNumber = !!password.match(/[\d]/); @@ -189,8 +191,21 @@ function handleVerifyEmail(actionCode, continueUrl) { applyActionCode(Auth, actionCode) - .then((resp) => { + .then(async (resp) => { // Email address has been verified. + const token = + Auth.currentUser !== undefined + ? await Auth.currentUser.getIdToken(true) + : undefined; + const url = envConfig.backendUrl + "/users/verifyEmail"; + + await fetch(url, { + method: "GET", + headers: { + "Content-Type": "application/json", + Authorization: `Bearer ${token}`, + }, + }); $("main .preloader .icon").html(``); $("main .preloader .text").text( diff --git a/packages/contracts/src/rate-limit/index.ts b/packages/contracts/src/rate-limit/index.ts index 205e3115caa8..385284575ae1 100644 --- a/packages/contracts/src/rate-limit/index.ts +++ b/packages/contracts/src/rate-limit/index.ts @@ -301,6 +301,11 @@ export const limits = { max: 1, }, + userVerifyEmail: { + window: 15 * 60 * 1000, //15 minutes + max: 1, + }, + userForgotPasswordEmail: { window: "minute", max: 1, diff --git a/packages/contracts/src/users.ts b/packages/contracts/src/users.ts index 5ec48b8477df..56ed2db0eba4 100644 --- a/packages/contracts/src/users.ts +++ b/packages/contracts/src/users.ts @@ -872,6 +872,19 @@ export const usersContract = c.router( rateLimit: "userRequestVerificationEmail", }), }, + verifyEmail: { + summary: "verify email", + description: "Verify the user email", + method: "GET", + path: "/verifyEmail", + responses: { + 200: MonkeyResponseSchema, + }, + metadata: meta({ + authenticationOptions: { noCache: true }, + rateLimit: "userVerifyEmail", + }), + }, forgotPasswordEmail: { summary: "send forgot password email", description: "Send a forgot password email", From b0a9029e8e0eec67f9f1b5825377b106a5c078ef Mon Sep 17 00:00:00 2001 From: Christian Fehmer Date: Mon, 14 Jul 2025 16:00:17 +0200 Subject: [PATCH 4/5] make verifyEmail public, use firebase admin --- backend/src/api/controllers/user.ts | 34 ++++++++++++++++++++++++----- frontend/src/email-handler.html | 23 ++++++++++--------- packages/contracts/src/users.ts | 10 +++++++-- 3 files changed, 49 insertions(+), 18 deletions(-) diff --git a/backend/src/api/controllers/user.ts b/backend/src/api/controllers/user.ts index 1e398ce611ea..e1c8b46d1425 100644 --- a/backend/src/api/controllers/user.ts +++ b/backend/src/api/controllers/user.ts @@ -85,6 +85,7 @@ import { UpdateUserNameRequest, UpdateUserProfileRequest, UpdateUserProfileResponse, + VerifyEmailRequest, } from "@monkeytype/contracts/users"; import { MILLISECONDS_IN_DAY } from "@monkeytype/util/date-and-time"; import { MonkeyRequest } from "../types"; @@ -241,9 +242,30 @@ export async function sendVerificationEmail( return new MonkeyResponse("Email sent", null); } -export async function verifyEmail(req: MonkeyRequest): Promise { - const { uid, email, emailVerified } = req.ctx.decodedToken; - await UserDAL.updateEmail(uid, email, emailVerified); +export async function verifyEmail( + req: MonkeyRequest +): Promise { + const { email } = req.body; + + const { data: firebaseUser } = await tryCatch( + FirebaseAdmin().auth().getUserByEmail(email) + ); + + if (firebaseUser === undefined || firebaseUser === null) { + throw new MonkeyError(404, "not found", "verify email"); + } + + await UserDAL.updateEmail( + firebaseUser.uid, + email, + firebaseUser.emailVerified + ); + + void addImportantLog( + "user_verify_email", + `emailVerified changed to ${firebaseUser.emailVerified} for email ${email}`, + firebaseUser.uid + ); return new MonkeyResponse("emailVerify updated.", null); } @@ -616,13 +638,13 @@ export async function getUser(req: MonkeyRequest): Promise { // soft-migrate user.emailVerified for existing users, update status if it has changed const { email, emailVerified } = req.ctx.decodedToken; if (emailVerified !== undefined && emailVerified !== userInfo.emailVerified) { + await UserDAL.updateEmail(uid, email, emailVerified); + userInfo.emailVerified = emailVerified; void addImportantLog( "user_verify_email", - `emailVerified changed to ${emailVerified} for email ${email}`, + `soft-migrate emailVerified changed to ${emailVerified} for email ${email}`, uid ); - await UserDAL.updateEmail(uid, email, emailVerified); - userInfo.emailVerified = emailVerified; } const userData: User = { diff --git a/frontend/src/email-handler.html b/frontend/src/email-handler.html index 2dee2a9d3edb..732aabcc99da 100644 --- a/frontend/src/email-handler.html +++ b/frontend/src/email-handler.html @@ -190,20 +190,23 @@ } function handleVerifyEmail(actionCode, continueUrl) { - applyActionCode(Auth, actionCode) + var email = null; + checkActionCode(Auth, actionCode) + .then((info) => { + // Get the email address + email = info["data"]["email"]; + + return applyActionCode(Auth, actionCode); + }) + .then(async (resp) => { // Email address has been verified. - const token = - Auth.currentUser !== undefined - ? await Auth.currentUser.getIdToken(true) - : undefined; - const url = envConfig.backendUrl + "/users/verifyEmail"; - - await fetch(url, { - method: "GET", + + await fetch(envConfig.backendUrl + "/users/verifyEmail", { + method: "POST", + body: JSON.stringify({ email }), headers: { "Content-Type": "application/json", - Authorization: `Bearer ${token}`, }, }); diff --git a/packages/contracts/src/users.ts b/packages/contracts/src/users.ts index 56ed2db0eba4..99b62526445a 100644 --- a/packages/contracts/src/users.ts +++ b/packages/contracts/src/users.ts @@ -308,6 +308,11 @@ export const ReportUserRequestSchema = z.object({ }); export type ReportUserRequest = z.infer; +export const VerifyEmailRequestSchema = z.object({ + email: UserEmailSchema, +}); +export type VerifyEmailRequest = z.infer; + export const ForgotPasswordEmailRequestSchema = z.object({ captcha: z.string(), email: UserEmailSchema, @@ -875,13 +880,14 @@ export const usersContract = c.router( verifyEmail: { summary: "verify email", description: "Verify the user email", - method: "GET", + method: "POST", path: "/verifyEmail", + body: VerifyEmailRequestSchema.strict(), responses: { 200: MonkeyResponseSchema, }, metadata: meta({ - authenticationOptions: { noCache: true }, + authenticationOptions: { isPublic: true }, rateLimit: "userVerifyEmail", }), }, From 30d974b15a7ee1b509af9fc5fc88870bd5d96d2d Mon Sep 17 00:00:00 2001 From: Christian Fehmer Date: Sat, 26 Jul 2025 16:03:22 +0200 Subject: [PATCH 5/5] check user first, tests missing --- backend/src/api/controllers/user.ts | 8 ++++++++ backend/src/dal/user.ts | 14 ++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/backend/src/api/controllers/user.ts b/backend/src/api/controllers/user.ts index e1c8b46d1425..628143ec3ad5 100644 --- a/backend/src/api/controllers/user.ts +++ b/backend/src/api/controllers/user.ts @@ -247,6 +247,14 @@ export async function verifyEmail( ): Promise { const { email } = req.body; + const user = await UserDAL.findPartialByEmail(email, [ + "email", + "emailVerified", + ]); + if (user === undefined || user.emailVerified === true) { + throw new MonkeyError(400, "cannot verify", "verify email"); + } + const { data: firebaseUser } = await tryCatch( FirebaseAdmin().auth().getUserByEmail(email) ); diff --git a/backend/src/dal/user.ts b/backend/src/dal/user.ts index 3f91417d6549..c3e505683e17 100644 --- a/backend/src/dal/user.ts +++ b/backend/src/dal/user.ts @@ -280,6 +280,20 @@ export async function findByName(name: string): Promise { )[0]; } +export async function findPartialByEmail( + email: string, + fields: K[] +): Promise | undefined> { + const projection = new Map(fields.map((it) => [it, 1])); + return ( + await getUsersCollection() + .find({ email }, { projection }) + .collation({ locale: "en", strength: 1 }) + .limit(1) + .toArray() + )[0]; +} + export async function isNameAvailable( name: string, uid: string