Skip to content

Commit f65fa00

Browse files
carderneTrigger.dev RepoOps
authored andcommitted
fix(webapp): require explicit email change confirmation
Changing your email address now requires clicking a confirmation button after opening the emailed link. You can still confirm while logged out or on another device. Mono-RevId: 58b3904fe89df4cc8743eed1fc579c01d93090ce
1 parent a8a8785 commit f65fa00

6 files changed

Lines changed: 209 additions & 10 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
area: webapp
3+
type: fix
4+
---
5+
6+
Changing your email address now requires clicking a confirmation button after opening the emailed link.

‎apps/webapp/app/routes/account.email.confirm.tsx‎

Lines changed: 71 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
1-
import type { LoaderFunctionArgs } from "@remix-run/node";
1+
import { json, redirect, type ActionFunctionArgs, type LoaderFunctionArgs } from "@remix-run/node";
2+
import { Form, useLoaderData, useNavigation } from "@remix-run/react";
3+
import { LoginPageLayout } from "~/components/LoginPageLayout";
4+
import { Button } from "~/components/primitives/Buttons";
5+
import { Header1 } from "~/components/primitives/Headers";
6+
import { Paragraph } from "~/components/primitives/Paragraph";
27
import { redirectWithErrorMessage, redirectWithSuccessMessage } from "~/models/message.server";
3-
import { confirmEmailChange } from "~/services/emailChange.server";
8+
import { confirmEmailChange, previewEmailChange } from "~/services/emailChange.server";
9+
import { emailChangeConfirmationCookie } from "~/services/emailChangeConfirmationCookie.server";
410
import { getUserId } from "~/services/session.server";
511
import { accountPath } from "~/utils/pathBuilder";
612

@@ -10,8 +16,38 @@ import { accountPath } from "~/utils/pathBuilder";
1016
* address, which would register it as a separate account.
1117
*/
1218
export async function loader({ request }: LoaderFunctionArgs) {
13-
const token = new URL(request.url).searchParams.get("token") ?? "";
14-
const result = await confirmEmailChange(token);
19+
const url = new URL(request.url);
20+
if (url.searchParams.has("token")) {
21+
const token = url.searchParams.get("token") ?? "";
22+
const preview = await previewEmailChange(token);
23+
return redirect(url.pathname, {
24+
headers: {
25+
"Cache-Control": "no-store",
26+
"Referrer-Policy": "no-referrer",
27+
"Set-Cookie": await emailChangeConfirmationCookie.serialize(
28+
preview ? token : "",
29+
preview ? {} : { maxAge: 0 }
30+
),
31+
},
32+
});
33+
}
34+
35+
const cookie: unknown = await emailChangeConfirmationCookie.parse(request.headers.get("Cookie"));
36+
const token = typeof cookie === "string" ? cookie : "";
37+
const preview = await previewEmailChange(token);
38+
return json(preview ? { token, email: preview.email } : { token: "", email: null }, {
39+
headers: { "Cache-Control": "no-store" },
40+
});
41+
}
42+
43+
export async function action({ request }: ActionFunctionArgs) {
44+
if (request.method !== "POST") {
45+
throw new Response("Method not allowed", { status: 405, headers: { Allow: "POST" } });
46+
}
47+
48+
const formData = await request.formData();
49+
const token = formData.get("token");
50+
const result = await confirmEmailChange(typeof token === "string" ? token : "");
1551

1652
const sessionUserId = await getUserId(request);
1753
const destination = sessionUserId ? accountPath() : "/login";
@@ -28,3 +64,34 @@ export async function loader({ request }: LoaderFunctionArgs) {
2864
: `Your email address is now ${result.email}. Use it to log in.`
2965
);
3066
}
67+
68+
export default function ConfirmEmailChangePage() {
69+
const { token, email } = useLoaderData<typeof loader>();
70+
const navigation = useNavigation();
71+
72+
return (
73+
<LoginPageLayout>
74+
<div className="space-y-6">
75+
<Header1>Confirm your email change</Header1>
76+
{email ? (
77+
<>
78+
<Paragraph>
79+
Confirm that you want to use <strong>{email}</strong> for your account. If you didn’t
80+
request this change, close this page.
81+
</Paragraph>
82+
<Form method="post">
83+
<input type="hidden" name="token" value={token} />
84+
<Button type="submit" variant="primary/medium" disabled={navigation.state !== "idle"}>
85+
Confirm email change
86+
</Button>
87+
</Form>
88+
</>
89+
) : (
90+
<Paragraph>
91+
This confirmation link is invalid or has expired. Request a new one.
92+
</Paragraph>
93+
)}
94+
</div>
95+
</LoginPageLayout>
96+
);
97+
}

‎apps/webapp/app/services/emailChange.server.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Changing an account's email address requires opening a link sent to the new
2+
* Changing an account's email address requires confirming a link sent to the new
33
* address. Until then the new address sits in `User.pendingEmail` and the live
44
* `User.email` (which invites and logins key on) is untouched.
55
*
@@ -73,6 +73,11 @@ async function verifyEmailChangeToken(
7373
return { userId: sub, email };
7474
}
7575

76+
export async function previewEmailChange(token: string): Promise<{ email: string } | undefined> {
77+
const claims = await verifyEmailChangeToken(env.SESSION_SECRET, token);
78+
return claims ? { email: claims.email } : undefined;
79+
}
80+
7681
type EmailChangeFailure = { ok: false; error: string; status: number };
7782
export type EmailChangeResult = { ok: true } | EmailChangeFailure;
7883

@@ -201,8 +206,7 @@ export async function confirmEmailChange(token: string): Promise<ConfirmEmailCha
201206
return failure("This confirmation link is invalid or has expired.", 400);
202207
}
203208

204-
// Already applied: mail scanners fetch links before the recipient does, and
205-
// people open them twice. Both must read as success, not as a dead link.
209+
// Repeated confirmations read as success without sending another notice.
206210
if (normalizeEmail(user.email) === claims.email) {
207211
return { ok: true, userId: user.id, email: claims.email };
208212
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import { createCookie } from "@remix-run/node";
2+
import { env } from "~/env.server";
3+
4+
// Redirect off the bearer-token URL before rendering a page that runs analytics.
5+
export const emailChangeConfirmationCookie = createCookie("email-change-confirmation", {
6+
maxAge: 60 * 60,
7+
httpOnly: true,
8+
sameSite: "lax",
9+
secure: env.NODE_ENV === "production",
10+
path: "/account/email/confirm",
11+
secrets: [env.SESSION_SECRET],
12+
});

‎apps/webapp/test/emailChange.test.ts‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { randomBytes } from "node:crypto";
22
import type { PrismaClient } from "@trigger.dev/database";
3+
import { generateJWT } from "@trigger.dev/core/v3/jwt";
34
import { describe, expect, vi } from "vitest";
45

56
const prismaHolder = vi.hoisted(() => ({
@@ -74,14 +75,23 @@ async function seedUser(prisma: PrismaClient) {
7475
}
7576

7677
describe("email change", () => {
77-
postgresTest("live email is untouched until the link is opened", async ({ prisma }) => {
78+
postgresTest("live email is untouched until the change is confirmed", async ({ prisma }) => {
7879
prismaHolder.client = prisma;
7980
const svc = await import("~/services/emailChange.server");
8081
const user = await seedUser(prisma);
8182
const newEmail = `new-${suffix()}@example.test`;
8283

8384
expect(await svc.requestEmailChange(user, ` ${newEmail.toUpperCase()} `)).toEqual({ ok: true });
8485

86+
expect(await svc.previewEmailChange(lastToken())).toEqual({ email: newEmail });
87+
expect(await svc.previewEmailChange("invalid")).toBeUndefined();
88+
const expiredJwt = await generateJWT({
89+
secretKey: "test-session-secret",
90+
payload: { kind: "email_change", sub: user.id, email: newEmail },
91+
expirationTime: "-1h",
92+
});
93+
expect(await svc.previewEmailChange(`tr_ecc_${expiredJwt}`)).toBeUndefined();
94+
8595
const pending = await prisma.user.findUniqueOrThrow({ where: { id: user.id } });
8696
expect(pending.email).toBe(user.email);
8797
expect(pending.pendingEmail).toBe(newEmail);
@@ -95,8 +105,7 @@ describe("email change", () => {
95105
expect(confirmed.pendingEmail).toBeNull();
96106
expect(sent.notices.at(-1)).toEqual({ to: user.email, newEmail });
97107

98-
// Opening it again (a mail scanner got there first, or a double click)
99-
// reads as success without sending another notice.
108+
// Confirming again reads as success without sending another notice.
100109
const noticesBefore = sent.notices.length;
101110
expect(await svc.confirmEmailChange(token)).toMatchObject({ ok: true, userId: user.id });
102111
expect(sent.notices.length).toBe(noticesBefore);
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
// @vitest-environment jsdom
2+
import { randomBytes } from "node:crypto";
3+
import { generateJWT } from "@trigger.dev/core/v3/jwt";
4+
import { expect, it } from "vitest";
5+
import { getTestServer } from "./helpers/sharedTestServer";
6+
import { seedTestUser } from "./helpers/seedTestSession";
7+
8+
// Must match the webapp test container's SESSION_SECRET.
9+
const SESSION_SECRET = "test-session-secret-for-e2e-tests";
10+
const CONFIRM_PATH = "/account/email/confirm";
11+
12+
async function pendingChange(expirationTime = "1h") {
13+
const { prisma } = getTestServer();
14+
const user = await seedTestUser(prisma);
15+
const email = `new-${randomBytes(6).toString("hex")}@test.local`;
16+
await prisma.user.update({ where: { id: user.id }, data: { pendingEmail: email } });
17+
const jwt = await generateJWT({
18+
secretKey: SESSION_SECRET,
19+
payload: { kind: "email_change", sub: user.id, email },
20+
expirationTime,
21+
});
22+
return { user, email, token: `tr_ecc_${jwt}` };
23+
}
24+
25+
async function openConfirmation(token: string) {
26+
const { webapp } = getTestServer();
27+
const landing = await webapp.fetch(`${CONFIRM_PATH}?token=${encodeURIComponent(token)}`, {
28+
redirect: "manual",
29+
});
30+
expect(landing.status).toBe(302);
31+
expect(landing.headers.get("location")).toBe(CONFIRM_PATH);
32+
expect(landing.headers.get("referrer-policy")).toBe("no-referrer");
33+
const cookie = landing.headers.get("set-cookie")?.split(";")[0] ?? "";
34+
const page = await webapp.fetch(CONFIRM_PATH, {
35+
headers: { Cookie: cookie },
36+
redirect: "manual",
37+
});
38+
expect(page.status).toBe(200);
39+
return { page, cookie };
40+
}
41+
42+
it("redirects off the token URL and confirms only when the rendered form is submitted", async () => {
43+
const { prisma } = getTestServer();
44+
const { user, email, token } = await pendingChange();
45+
const { page, cookie } = await openConfirmation(token);
46+
const document = new DOMParser().parseFromString(await page.text(), "text/html");
47+
expect(document.body.textContent).toContain(email);
48+
const input = document.querySelector<HTMLInputElement>('input[name="token"]');
49+
const form = input?.form;
50+
if (!form) throw new Error("No confirmation form rendered");
51+
expect(form.method).toBe("post");
52+
const action = new URL(form.getAttribute("action") || page.url, page.url);
53+
expect(action.search).toBe("");
54+
expect(await prisma.user.findFirstOrThrow({ where: { id: user.id } })).toMatchObject({
55+
email: user.email,
56+
pendingEmail: email,
57+
});
58+
59+
const body = new URLSearchParams();
60+
for (const [name, value] of new FormData(form)) {
61+
if (typeof value === "string") body.append(name, value);
62+
}
63+
const response = await fetch(action, {
64+
method: form.method,
65+
headers: { Cookie: cookie },
66+
body,
67+
redirect: "manual",
68+
});
69+
expect(response.status).toBe(302);
70+
expect(response.headers.get("location")).toBe("/login");
71+
expect(await prisma.user.findFirstOrThrow({ where: { id: user.id } })).toMatchObject({
72+
email,
73+
pendingEmail: null,
74+
});
75+
});
76+
77+
it("does not accept a confirmation token from the URL instead of the POST body", async () => {
78+
const { prisma, webapp } = getTestServer();
79+
const { user, email, token } = await pendingChange();
80+
const response = await webapp.fetch(`${CONFIRM_PATH}?token=${encodeURIComponent(token)}`, {
81+
method: "POST",
82+
body: new URLSearchParams(),
83+
redirect: "manual",
84+
});
85+
expect(response.status).toBe(302);
86+
expect(await prisma.user.findFirstOrThrow({ where: { id: user.id } })).toMatchObject({
87+
email: user.email,
88+
pendingEmail: email,
89+
});
90+
});
91+
92+
it("shows an error without a confirmation form for invalid or expired links", async () => {
93+
const { token } = await pendingChange("-1h");
94+
for (const invalidToken of ["invalid", token]) {
95+
const { page } = await openConfirmation(invalidToken);
96+
const document = new DOMParser().parseFromString(await page.text(), "text/html");
97+
expect(document.body.textContent).toContain("invalid or has expired");
98+
expect(document.querySelector('input[name="token"]')).toBeNull();
99+
expect(document.querySelector('button[type="submit"]')).toBeNull();
100+
}
101+
});

0 commit comments

Comments
 (0)