Skip to content

Commit 8222695

Browse files
committed
fix: guard against accidental deletion
1 parent 54aece2 commit 8222695

File tree

5 files changed

+89
-20
lines changed

5 files changed

+89
-20
lines changed

locales/en/settings.ftl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ settings-delete-monitor-free-account-dialog-cta-label = Delete account
5858
settings-delete-monitor-free-account-dialog-cancel-button-label = Never mind, take me back
5959
settings-delete-monitor-account-confirmation-toast-label-2 = Your { -brand-monitor } account is now deleted.
6060
settings-delete-monitor-account-confirmation-toast-dismiss-label = Dismiss
61+
settings-delete-account-recent-auth-required = For your safety, please sign in again (within the last hour) before deleting your account.
62+
settings-delete-account-error-generic = Something went wrong while trying to delete your account. Try again.
6163
6264
## Monthly Monitor Report
6365

src/app/(proper_react)/(redesign)/(authenticated)/user/(dashboard)/settings/DeleteAccountButton.tsx

Lines changed: 49 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { signOut } from "next-auth/react";
99
import { Button } from "../../../../../../components/client/Button";
1010
import { type onDeleteAccount } from "./actions";
1111
import { useTelemetry } from "../../../../../../hooks/useTelemetry";
12+
import { useL10n } from "../../../../../../hooks/l10n";
1213

1314
/**
1415
* Since <Button> is a client component, the `onDeleteAccount` handler can only
@@ -21,29 +22,57 @@ export const DeleteAccountButton = (
2122
onDeleteAccount: typeof onDeleteAccount;
2223
},
2324
) => {
25+
const l10n = useL10n();
2426
const recordTelemetry = useTelemetry();
2527
const [isSubmitting, setIsSubmitting] = useState(false);
28+
const [errorMessage, setErrorMessage] = useState<string | null>(null);
2629
return (
27-
<Button
28-
{...props}
29-
isLoading={isSubmitting}
30-
isDisabled={isSubmitting}
31-
onPress={() => {
32-
recordTelemetry("ctaButton", "click", {
33-
button_id: "confirm_delete_account",
34-
});
35-
setIsSubmitting(true);
36-
// It's currently unclear if and how we should mock our server action:
37-
/* c8 ignore next 8 */
38-
void props
39-
.onDeleteAccount()
40-
.then(() => {
41-
void signOut({ callbackUrl: "/" });
42-
})
43-
.catch(() => {
44-
setIsSubmitting(false);
30+
<>
31+
<Button
32+
{...props}
33+
isLoading={isSubmitting}
34+
isDisabled={isSubmitting}
35+
onPress={() => {
36+
recordTelemetry("ctaButton", "click", {
37+
button_id: "confirm_delete_account",
4538
});
46-
}}
47-
/>
39+
setIsSubmitting(true);
40+
setErrorMessage(null);
41+
// It's currently unclear if and how we should mock our server action:
42+
/* c8 ignore next 13 */
43+
void props
44+
.onDeleteAccount()
45+
.then((result) => {
46+
if (result && typeof result === "object" && "success" in result) {
47+
if (result.success === false) {
48+
setIsSubmitting(false);
49+
setErrorMessage(
50+
result.errorMessage ??
51+
l10n.getString("settings-delete-account-error-generic"),
52+
);
53+
return;
54+
}
55+
}
56+
void signOut({ callbackUrl: "/" });
57+
})
58+
.catch(() => {
59+
setIsSubmitting(false);
60+
setErrorMessage(
61+
l10n.getString("settings-delete-account-error-generic"),
62+
);
63+
});
64+
}}
65+
/>
66+
{errorMessage && (
67+
<p
68+
role="status"
69+
style={{
70+
marginTop: "0.5rem",
71+
}}
72+
>
73+
{errorMessage}
74+
</p>
75+
)}
76+
</>
4877
);
4978
};

src/app/(proper_react)/(redesign)/(authenticated)/user/(dashboard)/settings/actions.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ import { type NormalizedProfileData } from "./panels/SettingsPanelEditProfile/Ed
3737
import { OnerepUsPhoneNumber } from "../../../../../../functions/server/onerep";
3838
import { getEnabledFeatureFlags } from "../../../../../../../db/tables/featureFlags";
3939

40+
const RECENT_AUTH_WINDOW_MS = 60 * 60 * 1000;
41+
4042
export type AddEmailFormState =
4143
| { success?: never }
4244
| { success: true; submittedAddress: string }
@@ -192,6 +194,17 @@ export async function onRemoveEmail(email: SanitizedEmailAddressRow) {
192194
}
193195
}
194196

197+
const createRecentAuthErrorResponse = async () => {
198+
const l10n = getL10n(await getAcceptLangHeaderInServerComponents());
199+
return {
200+
success: false,
201+
error: "delete-account-auth-too-old",
202+
errorMessage: l10n.getString(
203+
"settings-delete-account-recent-auth-required",
204+
),
205+
} as const;
206+
};
207+
195208
export async function onDeleteAccount() {
196209
const session = await getServerSession();
197210
if (!session?.user.subscriber?.fxa_uid) {
@@ -203,6 +216,22 @@ export async function onDeleteAccount() {
203216
};
204217
}
205218

219+
const authenticatedAt = session.authenticatedAt
220+
? Date.parse(session.authenticatedAt)
221+
: NaN;
222+
if (!Number.isFinite(authenticatedAt)) {
223+
logger.warn("delete-account-missing-authenticated-at", {
224+
subscriber_id: session.user.subscriber.id,
225+
});
226+
return createRecentAuthErrorResponse();
227+
}
228+
if (Date.now() - authenticatedAt > RECENT_AUTH_WINDOW_MS) {
229+
logger.warn("delete-account-authenticated-at-too-old", {
230+
subscriber_id: session.user.subscriber.id,
231+
});
232+
return createRecentAuthErrorResponse();
233+
}
234+
206235
const subscriber = await getSubscriberByFxaUid(
207236
session.user.subscriber.fxa_uid,
208237
);
@@ -229,6 +258,7 @@ export async function onDeleteAccount() {
229258
// possibile, so instead the sign-out and redirect needs to happen on the
230259
// client side after this action completes.
231260
// See https://github.com/nextauthjs/next-auth/discussions/5334.
261+
return { success: true } as const;
232262
}
233263

234264
export async function onHandleUpdateProfileData(

src/app/api/utils/auth.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,9 @@ export const authOptions: AuthOptions = {
118118
},
119119
// Unused arguments also listed to show what's available:
120120
async jwt({ token, account, profile, trigger }) {
121+
if (account) {
122+
token.authenticatedAt = Date.now();
123+
}
121124
if (trigger === "update") {
122125
// Refresh the user data from FxA, in case e.g. new subscriptions got added:
123126
const subscriberFromDb = await getSubscriberByFxaUid(
@@ -298,6 +301,9 @@ export const authOptions: AuthOptions = {
298301
}
299302
}
300303
}
304+
if (token.authenticatedAt) {
305+
session.authenticatedAt = new Date(token.authenticatedAt).toISOString();
306+
}
301307

302308
return session;
303309
},

src/next-auth.d.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ declare module "next-auth" {
4646
/** Session data available after deserialising the JWT */
4747
interface Session {
4848
error?: "RefreshAccessTokenError";
49+
authenticatedAt?: ISO8601DateString;
4950
user: {
5051
fxa?: {
5152
/** The value of the Accept-Language header when the user signed up for their Firefox Account */
@@ -76,5 +77,6 @@ declare module "next-auth/jwt" {
7677
subscriptions: Array<string>;
7778
};
7879
subscriber?: SerializedSubscriber;
80+
authenticatedAt?: number;
7981
}
8082
}

0 commit comments

Comments
 (0)