Skip to content

Commit 719c8c9

Browse files
carderneTrigger.dev RepoOps
authored andcommitted
feat(webapp): require confirmation before changing account email
Mono-RevId: 3cc98d225d8b132958976a5e504ec29b6d1097ba
1 parent 82f1095 commit 719c8c9

14 files changed

Lines changed: 723 additions & 77 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: improvement
4+
---
5+
6+
Changing your account email address now sends a confirmation link to the new address; the change only takes effect once that link is opened. Requesting a magic link no longer creates an account until the link is used.

‎apps/webapp/app/models/user.server.ts‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -368,14 +368,14 @@ export async function getUserById(id: User["id"]) {
368368
};
369369
}
370370

371+
/** The onboarding write. Email changes go through `emailChange.server.ts` and need confirmation. */
371372
export function updateUser({
372373
id,
373374
name,
374-
email,
375375
marketingEmails,
376376
referralSource,
377377
onboardingData,
378-
}: Pick<User, "id" | "name" | "email"> & {
378+
}: Pick<User, "id" | "name"> & {
379379
marketingEmails?: boolean;
380380
referralSource?: string;
381381
onboardingData?: Prisma.InputJsonValue;
@@ -384,7 +384,6 @@ export function updateUser({
384384
where: { id },
385385
data: {
386386
name,
387-
email,
388387
marketingEmails,
389388
referralSource,
390389
onboardingData,
@@ -404,13 +403,6 @@ export function updateUserName({ id, name }: Pick<User, "id" | "name">) {
404403
});
405404
}
406405

407-
export function updateUserEmail({ id, email }: Pick<User, "id" | "email">) {
408-
return prisma.user.update({
409-
where: { id },
410-
data: { email },
411-
});
412-
}
413-
414406
/**
415407
* `updateMany` so the WHERE does the comparing: a redundant request updates zero
416408
* rows rather than churning the row and its updatedAt.

‎apps/webapp/app/routes/account._index/route.tsx‎

Lines changed: 63 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@ import {
6262
THEME_OPTIONS_BY_VALUE,
6363
themeOptionIcon,
6464
} from "~/components/themeOptions";
65-
import { prisma } from "~/db.server";
6665
import { SelectBestEnvironmentPresenter } from "~/presenters/SelectBestEnvironmentPresenter.server";
6766
import {
6867
applyThemeContrast,
@@ -72,9 +71,14 @@ import {
7271
} from "~/hooks/useSystemThemeSync";
7372
import { useFeatures } from "~/hooks/useFeatures";
7473
import { useHasAdminAccess, useUser } from "~/hooks/useUser";
75-
import { updateUserEmail, updateUserMarketingEmails, updateUserName } from "~/models/user.server";
74+
import { updateUserMarketingEmails, updateUserName } from "~/models/user.server";
75+
import {
76+
cancelEmailChange,
77+
requestEmailChange,
78+
resendEmailChange,
79+
} from "~/services/emailChange.server";
7680
import { logger } from "~/services/logger.server";
77-
import { type EmailOwnership, getEmailOwnership } from "~/services/ssoManagedIdentity.server";
81+
import { type EmailOwnership } from "~/services/ssoManagedIdentity.server";
7882
import {
7983
updateContrastPreference,
8084
updateIconContrastPreference,
@@ -370,27 +374,25 @@ export const action: ActionFunction = async ({ request }) => {
370374
);
371375
}
372376

373-
const { email } = submission.data;
377+
const result = await requestEmailChange(gate.user, submission.data.email);
378+
if (!result.ok) return profileUpdateError(result.error, result.status);
379+
return json({ success: true as const });
380+
}
374381

375-
const ownership = await getEmailOwnership(gate.user, email);
376-
if (ownership === "idp") {
377-
return profileUpdateError(
378-
"Your email address is managed by your organization's identity provider.",
379-
403
380-
);
381-
}
382-
if (ownership === "unknown") {
383-
return profileUpdateError(
384-
"We couldn't check your single sign-on settings just now. Please try again shortly.",
385-
503
386-
);
387-
}
388-
const existingUser = await prisma.user.findFirst({ where: { email } });
389-
if (existingUser && existingUser.id !== gate.user.id) {
390-
return profileUpdateError("Email is already being used by a different account", 400);
391-
}
382+
if (formData.get("action") === "resend-email-change") {
383+
const gate = await requireOwnAccountWrite(request);
384+
if ("error" in gate) return gate.error;
392385

393-
await updateUserEmail({ id: gate.user.id, email });
386+
const result = await resendEmailChange(gate.user);
387+
if (!result.ok) return profileUpdateError(result.error, result.status);
388+
return json({ success: true as const });
389+
}
390+
391+
if (formData.get("action") === "cancel-email-change") {
392+
const gate = await requireOwnAccountWrite(request);
393+
if ("error" in gate) return gate.error;
394+
395+
await cancelEmailChange(gate.user.id);
394396
return json({ success: true as const });
395397
}
396398

@@ -528,7 +530,7 @@ function EditEmailButton() {
528530
const user = useUser();
529531
const [isOpen, setIsOpen] = useState(false);
530532
const { fetcher, error, setError, isSubmitting } = useProfileFieldUpdate({
531-
successMessage: "Your email address has been updated.",
533+
successMessage: "Check your inbox for a link to confirm your new email address.",
532534
onSuccess: () => setIsOpen(false),
533535
});
534536
const ownershipFetcher = useFetcher<{ ownership: EmailOwnership }>();
@@ -601,7 +603,7 @@ function EditEmailButton() {
601603
Cancel
602604
</Button>
603605
<Button type="submit" variant="primary/medium" disabled={isSubmitting}>
604-
{isSubmitting ? "Saving…" : "Update"}
606+
{isSubmitting ? "Sending…" : "Send confirmation link"}
605607
</Button>
606608
</DialogFooter>
607609
</fetcher.Form>
@@ -612,6 +614,40 @@ function EditEmailButton() {
612614
);
613615
}
614616

617+
/** Shown under the email row while a new address is waiting to be confirmed. */
618+
function PendingEmailChange({ pendingEmail }: { pendingEmail: string }) {
619+
const resend = useProfileFieldUpdate({
620+
successMessage: `We've sent another confirmation link to ${pendingEmail}.`,
621+
onSuccess: () => {},
622+
});
623+
const cancel = useProfileFieldUpdate({
624+
successMessage: "Email change cancelled.",
625+
onSuccess: () => {},
626+
});
627+
628+
return (
629+
<div className="flex w-full items-center justify-between gap-4 pb-3">
630+
<Paragraph variant="extra-small" className="min-w-0 break-all text-text-dimmed">
631+
Confirm {pendingEmail} using the link we emailed to it.
632+
</Paragraph>
633+
<div className="flex flex-none items-center gap-2">
634+
<resend.fetcher.Form method="post">
635+
<input type="hidden" name="action" value="resend-email-change" />
636+
<Button type="submit" variant="tertiary/small" disabled={resend.isSubmitting}>
637+
Resend
638+
</Button>
639+
</resend.fetcher.Form>
640+
<cancel.fetcher.Form method="post">
641+
<input type="hidden" name="action" value="cancel-email-change" />
642+
<Button type="submit" variant="tertiary/small" disabled={cancel.isSubmitting}>
643+
Cancel
644+
</Button>
645+
</cancel.fetcher.Form>
646+
</div>
647+
</div>
648+
);
649+
}
650+
615651
const MARKETING_EMAILS_DEBOUNCE_MS = 600;
616652

617653
/** Debounced, one write at a time; the action's rate limit backs it up. */
@@ -988,8 +1024,8 @@ export default function Page() {
9881024
</div>
9891025
</div>
9901026
</div>
991-
<div className="flex min-h-16 w-full items-center border-b border-grid-dimmed">
992-
<div className="flex w-full items-center justify-between gap-4">
1027+
<div className="flex min-h-16 w-full flex-col justify-center border-b border-grid-dimmed">
1028+
<div className="flex w-full items-center justify-between gap-4 py-3">
9931029
<Label>Email address</Label>
9941030
<div className="flex min-w-0 items-center gap-3">
9951031
{/* break-all: an address has no spaces to wrap at, so a long one
@@ -1000,6 +1036,7 @@ export default function Page() {
10001036
<EditEmailButton />
10011037
</div>
10021038
</div>
1039+
{user.pendingEmail && <PendingEmailChange pendingEmail={user.pendingEmail} />}
10031040
</div>
10041041
<div className="flex min-h-16 w-full items-center border-b border-grid-dimmed">
10051042
<div className="flex w-full items-center justify-between gap-4">
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import type { LoaderFunctionArgs } from "@remix-run/node";
2+
import { redirectWithErrorMessage, redirectWithSuccessMessage } from "~/models/message.server";
3+
import { confirmEmailChange } from "~/services/emailChange.server";
4+
import { getUserId } from "~/services/session.server";
5+
import { accountPath } from "~/utils/pathBuilder";
6+
7+
/**
8+
* Target of the link emailed to a pending address. Works logged out: sending
9+
* the visitor through /login first would invite them to "log in" with the new
10+
* address, which would register it as a separate account.
11+
*/
12+
export async function loader({ request }: LoaderFunctionArgs) {
13+
const token = new URL(request.url).searchParams.get("token") ?? "";
14+
const result = await confirmEmailChange(token);
15+
16+
const sessionUserId = await getUserId(request);
17+
const destination = sessionUserId ? accountPath() : "/login";
18+
19+
if (!result.ok) {
20+
return redirectWithErrorMessage(destination, request, result.error);
21+
}
22+
23+
return redirectWithSuccessMessage(
24+
destination,
25+
request,
26+
sessionUserId === result.userId
27+
? "Your email address has been updated."
28+
: `Your email address is now ${result.email}. Use it to log in.`
29+
);
30+
}

‎apps/webapp/app/routes/confirm-basic-details.tsx‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ import { useFeatures } from "~/hooks/useFeatures";
2626
import { useUser } from "~/hooks/useUser";
2727
import { redirectWithSuccessMessage } from "~/models/message.server";
2828
import { updateUser } from "~/models/user.server";
29-
import { requireUserId } from "~/services/session.server";
29+
import { normalizeEmail, requestEmailChange } from "~/services/emailChange.server";
30+
import { requireUser } from "~/services/session.server";
3031
import { telemetry } from "~/services/telemetry.server";
3132
import { emailSchema, MAX_EMAIL_LENGTH } from "~/utils/emailValidation";
3233
import { rootPath } from "~/utils/pathBuilder";
@@ -113,7 +114,8 @@ function createSchema(
113114
}
114115

115116
export const action: ActionFunction = async ({ request }) => {
116-
const userId = await requireUserId(request);
117+
const user = await requireUser(request);
118+
const userId = user.id;
117119
const formData = await request.formData();
118120

119121
const formSchema = createSchema({
@@ -170,10 +172,21 @@ export const action: ActionFunction = async ({ request }) => {
170172
? `Other: ${submission.value.referralSourceOther}`
171173
: submission.value.referralSource;
172174

175+
// The email is not written here: the account keeps its current address
176+
// until the user opens the link sent to the new one. Requested first so a
177+
// refused address leaves onboarding unconfirmed and the form comes back.
178+
const newEmail = normalizeEmail(submission.value.email);
179+
const emailChanged = newEmail !== normalizeEmail(user.email);
180+
if (emailChanged) {
181+
const result = await requestEmailChange(user, newEmail);
182+
if (!result.ok) {
183+
return json(submission.reply({ formErrors: [result.error] }), { status: result.status });
184+
}
185+
}
186+
173187
await updateUser({
174188
id: userId,
175189
name: submission.value.name,
176-
email: submission.value.email,
177190
referralSource: referralSourceForLegacy,
178191
onboardingData: Object.keys(onboardingData).length > 0 ? onboardingData : undefined,
179192
});
@@ -200,7 +213,13 @@ export const action: ActionFunction = async ({ request }) => {
200213
redirectUrl = `/orgs/new?${params.toString()}`;
201214
}
202215

203-
return redirectWithSuccessMessage(redirectUrl, request, "Your details have been updated.");
216+
return redirectWithSuccessMessage(
217+
redirectUrl,
218+
request,
219+
emailChanged
220+
? `Your details have been updated. Check ${newEmail} for a link to confirm your new email address.`
221+
: "Your details have been updated."
222+
);
204223
} catch (error: any) {
205224
return json({ errors: { body: error.message } }, { status: 400 });
206225
}
@@ -428,6 +447,7 @@ export default function Page() {
428447
</>
429448
)}
430449

450+
<FormError id={form.errorId}>{form.errors}</FormError>
431451
<FormButtons
432452
confirmButton={
433453
<Button

‎apps/webapp/app/services/emailAuth.server.tsx‎

Lines changed: 45 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -12,48 +12,58 @@ import { SsoRequiredError, ssoRedirectForEmail } from "./ssoAutoDiscovery.server
1212
let secret = env.MAGIC_LINK_SECRET;
1313
if (!secret) throw new Error("Missing MAGIC_LINK_SECRET env variable.");
1414

15+
/**
16+
* The strategy's verify callback. It runs twice per login: once when SENDING
17+
* the link (`magicLinkVerify: false`, only to hand a user to `sendEmail`, which
18+
* ignores it; the throw is swallowed) and once when the link is opened.
19+
*/
20+
export async function verifyMagicLink({
21+
email,
22+
magicLinkVerify,
23+
}: {
24+
email: string;
25+
magicLinkVerify: boolean;
26+
}): Promise<AuthUser> {
27+
// Nothing may be created on the send path: an account must not exist for an
28+
// address until a link delivered to it has been opened, or anyone could
29+
// register someone else's email and have mail sent to it.
30+
if (!magicLinkVerify) {
31+
throw new Error("Magic link not yet verified");
32+
}
33+
34+
logger.info("Magic link user authenticated");
35+
36+
// Gate the link CLICK: a magic link issued before SSO enforcement flipped
37+
// on (or replayed within its validity window) must not mint a session for
38+
// an enforced domain.
39+
const ssoRedirect = await ssoRedirectForEmail(email, "domain_policy");
40+
if (ssoRedirect) {
41+
throw new SsoRequiredError(ssoRedirect);
42+
}
43+
44+
try {
45+
const { user, isNewUser } = await findOrCreateUser({
46+
email,
47+
authenticationMethod: "MAGIC_LINK",
48+
});
49+
50+
await postAuthentication({ user, isNewUser, loginMethod: "MAGIC_LINK" });
51+
52+
return { userId: user.id };
53+
} catch (error) {
54+
logger.debug("Magic link user failed to authenticate", { error: JSON.stringify(error) });
55+
throw error;
56+
}
57+
}
58+
1559
const emailStrategy = new EmailLinkStrategy(
1660
{
1761
sendEmail: sendMagicLinkEmail,
1862
secret,
1963
callbackURL: "/magic",
2064
sessionMagicLinkKey: "triggerdotdev:magiclink",
2165
},
22-
async ({
23-
email,
24-
form,
25-
magicLinkVerify,
26-
}: {
27-
email: string;
28-
form: FormData;
29-
magicLinkVerify: boolean;
30-
}) => {
31-
logger.info("Magic link user authenticated", { magicLinkVerify });
32-
33-
// Gate the link CLICK, not just the send: a magic link issued before
34-
// SSO enforcement flipped on (or replayed within its validity
35-
// window) must not mint a session for an enforced domain.
36-
if (magicLinkVerify) {
37-
const ssoRedirect = await ssoRedirectForEmail(email, "domain_policy");
38-
if (ssoRedirect) {
39-
throw new SsoRequiredError(ssoRedirect);
40-
}
41-
}
42-
43-
try {
44-
const { user, isNewUser } = await findOrCreateUser({
45-
email,
46-
authenticationMethod: "MAGIC_LINK",
47-
});
48-
49-
await postAuthentication({ user, isNewUser, loginMethod: "MAGIC_LINK" });
50-
51-
return { userId: user.id };
52-
} catch (error) {
53-
logger.debug("Magic link user failed to authenticate", { error: JSON.stringify(error) });
54-
throw error;
55-
}
56-
}
66+
verifyMagicLink
5767
);
5868

5969
export function addEmailLinkStrategy(authenticator: Authenticator<AuthUser>) {

0 commit comments

Comments
 (0)