Skip to content

Commit 33cb454

Browse files
committed
require 2fa code when changing email
1 parent 83dcab5 commit 33cb454

File tree

5 files changed

+114
-35
lines changed

5 files changed

+114
-35
lines changed

app/root.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,11 @@ function App() {
236236
</div>
237237
<Confetti confetti={data.flash?.confetti} />
238238
<Toaster />
239-
{RemixDevTools && <Suspense><RemixDevTools showRouteBoundaries /></Suspense>}
239+
{RemixDevTools && (
240+
<Suspense>
241+
<RemixDevTools showRouteBoundaries />
242+
</Suspense>
243+
)}
240244
</Document>
241245
)
242246
}

app/routes/resources+/login.tsx

Lines changed: 63 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,27 @@ import { safeRedirect } from 'remix-utils'
1212
import { z } from 'zod'
1313
import { CheckboxField, ErrorList, Field } from '~/components/forms.tsx'
1414
import { StatusButton } from '~/components/ui/status-button.tsx'
15-
import { authenticator } from '~/utils/auth.server.ts'
15+
import { authenticator, getUserId } from '~/utils/auth.server.ts'
1616
import { prisma } from '~/utils/db.server.ts'
1717
import { redirectWithToast } from '~/utils/flash-session.server.ts'
18-
import { EnsurePE, ensurePE, getReferrerRoute } from '~/utils/misc.tsx'
18+
import {
19+
EnsurePE,
20+
ensurePE,
21+
getReferrerRoute,
22+
invariant,
23+
} from '~/utils/misc.tsx'
1924
import {
2025
commitSession,
2126
destroySession,
2227
getSession,
2328
} from '~/utils/session.server.ts'
2429
import { passwordSchema, usernameSchema } from '~/utils/user-validation.ts'
2530
import { checkboxSchema } from '~/utils/zod-extensions.ts'
26-
import { isCodeValid } from './verify.tsx'
31+
import {
32+
type VerificationTypes,
33+
isCodeValid,
34+
type VerifyFunctionArgs,
35+
} from './verify.tsx'
2736

2837
const ROUTE_PATH = '/resources/login'
2938

@@ -51,19 +60,58 @@ const LoginFormSchema = z.object({
5160
remember: checkboxSchema(),
5261
})
5362

63+
const verifiedTimeKey = 'verified-time'
5464
const unverifiedSessionIdKey = 'unverified-session-id'
5565
const loginSubmissionKey = 'login-submission'
66+
const inlineLoginFormId = 'inline-login'
67+
const inlineTwoFAFormId = 'inline-two-fa'
68+
const verificationType = '2fa' satisfies VerificationTypes
5669

57-
async function shouldRequestTwoFA(request: Request) {
58-
const session = await getSession(request.headers.get('cookie'))
59-
return session.has(unverifiedSessionIdKey)
70+
export async function handleVerification({
71+
request,
72+
body,
73+
submission,
74+
}: VerifyFunctionArgs) {
75+
invariant(submission.value, 'Submission should have a value by this point')
76+
const cookieSession = await getSession(request.headers.get('cookie'))
77+
const { redirectTo } = submission.value
78+
cookieSession.set(verifiedTimeKey, Date.now())
79+
const responseInit = {
80+
headers: { 'Set-Cookie': await commitSession(cookieSession) },
81+
}
82+
83+
if (redirectTo) {
84+
return redirect(safeRedirect(redirectTo), responseInit)
85+
} else {
86+
ensurePE(body, request, responseInit)
87+
return json({ status: 'success', submission } as const, responseInit)
88+
}
89+
}
90+
91+
export async function shouldRequestTwoFA(request: Request) {
92+
const cookieSession = await getSession(request.headers.get('cookie'))
93+
if (cookieSession.has(unverifiedSessionIdKey)) return true
94+
const userId = await getUserId(request)
95+
if (!userId) return false
96+
// if it's over two hours since they last verified, we should request 2FA again
97+
const userHasTwoFA = await prisma.verification.findUnique({
98+
select: { id: true },
99+
where: { target_type: { target: userId, type: verificationType } },
100+
})
101+
if (!userHasTwoFA) return false
102+
const verifiedTime = cookieSession.get(verifiedTimeKey) ?? new Date(0)
103+
const twoHours = 1000 * 60 * 60 * 2
104+
return Date.now() - verifiedTime > twoHours
60105
}
61106

62107
export async function action(args: DataFunctionArgs) {
63-
if (await shouldRequestTwoFA(args.request)) {
108+
const form = (await args.request.clone().formData()).get('form')
109+
if (form === inlineTwoFAFormId) {
64110
return inlineTwoFAAction(args)
65-
} else {
111+
} else if (form === inlineLoginFormId) {
66112
return inlineLoginAction(args)
113+
} else {
114+
throw new Response('Invalid form', { status: 400 })
67115
}
68116
}
69117

@@ -134,7 +182,7 @@ async function inlineLoginAction({ request }: DataFunctionArgs) {
134182
const { remember, redirectTo, session } = submission.value
135183

136184
const verification = await prisma.verification.findUnique({
137-
where: { target_type: { target: session.userId, type: '2fa' } },
185+
where: { target_type: { target: session.userId, type: verificationType } },
138186
select: { id: true },
139187
})
140188
const userHasTwoFactor = Boolean(verification)
@@ -192,7 +240,7 @@ function InlineLoginForm({
192240
const loginFetcher = useFetcher<typeof inlineLoginAction>()
193241

194242
const [form, fields] = useForm({
195-
id: 'inline-login',
243+
id: inlineLoginFormId,
196244
defaultValue: { redirectTo },
197245
constraint: getFieldsetConstraint(LoginFormSchema),
198246
lastSubmission: loginFetcher.data?.submission ?? submission,
@@ -211,6 +259,7 @@ function InlineLoginForm({
211259
name="login"
212260
{...form.props}
213261
>
262+
<input type="hidden" name="form" value={form.id} />
214263
<EnsurePE />
215264
<Field
216265
labelProps={{ children: 'Username' }}
@@ -316,7 +365,7 @@ async function inlineTwoFAAction({ request }: DataFunctionArgs) {
316365
schema: TwoFAFormSchema.superRefine(async (data, ctx) => {
317366
const codeIsValid = await isCodeValid({
318367
code: data.code,
319-
type: '2fa',
368+
type: verificationType,
320369
target: session.userId,
321370
})
322371
if (!codeIsValid) {
@@ -351,6 +400,7 @@ async function inlineTwoFAAction({ request }: DataFunctionArgs) {
351400
}
352401

353402
const { redirectTo } = submission.value
403+
cookieSession.set(verifiedTimeKey, Date.now())
354404
cookieSession.unset(unverifiedSessionIdKey)
355405
cookieSession.set(authenticator.sessionKey, session.id)
356406
const responseInit = {
@@ -377,7 +427,7 @@ function InlineTwoFA({
377427
const twoFAFetcher = useFetcher<typeof inlineTwoFAAction>()
378428

379429
const [form, fields] = useForm({
380-
id: 'inline-two-fa',
430+
id: inlineTwoFAFormId,
381431
defaultValue: { redirectTo },
382432
constraint: getFieldsetConstraint(TwoFAFormSchema),
383433
lastSubmission: twoFAFetcher.data?.submission ?? submission,
@@ -389,6 +439,7 @@ function InlineTwoFA({
389439

390440
return (
391441
<twoFAFetcher.Form method="POST" action={ROUTE_PATH} {...form.props}>
442+
<input type="hidden" name="form" value={form.id} />
392443
{/* Putting this at the top so we can have the tab order of cancel first,
393444
but have "enter" submit the confirmation. */}
394445
<button type="submit" className="hidden" name="intent" value="confirm" />

app/routes/resources+/verify.tsx

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,13 @@ import { z } from 'zod'
66
import { ErrorList, Field } from '~/components/forms.tsx'
77
import { StatusButton } from '~/components/ui/status-button.tsx'
88
import { prisma } from '~/utils/db.server.ts'
9-
import { getDomainUrl, useIsSubmitting } from '~/utils/misc.tsx'
9+
import { EnsurePE, getDomainUrl, useIsSubmitting } from '~/utils/misc.tsx'
1010
import { generateTOTP, verifyTOTP } from '~/utils/totp.server.ts'
1111
import { handleVerification as handleForgotPasswordVerification } from '../_auth+/forgot-password/index.tsx'
1212
import { handleVerification as handleOnboardingVerification } from '../_auth+/onboarding.tsx'
1313
import { handleVerification as handleChangeEmailVerification } from '../settings+/profile.change-email.index/index.tsx'
1414
import { handleVerification as handleEnableTwoFactorVerification } from '../settings+/profile.two-factor.verify.tsx'
15+
import { handleVerification as handleReverifyVerification } from './login.tsx'
1516

1617
export const ROUTE_PATH = '/resources/verify'
1718

@@ -100,6 +101,7 @@ export type VerifySubmission = Submission<z.infer<typeof VerifySchema>>
100101
export type VerifyFunctionArgs = {
101102
request: Request
102103
submission: Submission<z.infer<typeof VerifySchema>>
104+
body: FormData | URLSearchParams
103105
}
104106

105107
export async function isCodeValid({
@@ -181,18 +183,21 @@ export async function validateRequest(
181183
switch (submissionValue[typeQueryParam]) {
182184
case 'forgot-password': {
183185
await deleteVerification()
184-
return handleForgotPasswordVerification({ request, submission })
186+
return handleForgotPasswordVerification({ request, body, submission })
185187
}
186188
case 'onboarding': {
187189
await deleteVerification()
188-
return handleOnboardingVerification({ request, submission })
190+
return handleOnboardingVerification({ request, body, submission })
189191
}
190192
case '2fa-verify': {
191-
return handleEnableTwoFactorVerification({ request, submission })
193+
return handleEnableTwoFactorVerification({ request, body, submission })
194+
}
195+
case '2fa': {
196+
return handleReverifyVerification({ request, body, submission })
192197
}
193198
case 'change-email': {
194199
await deleteVerification()
195-
return await handleChangeEmailVerification({ request, submission })
200+
return await handleChangeEmailVerification({ request, body, submission })
196201
}
197202
default: {
198203
submission.error[''] = ['Invalid verification type']
@@ -254,6 +259,7 @@ export function Verify({
254259
{...form.props}
255260
className="flex-1"
256261
>
262+
<EnsurePE />
257263
<input
258264
{...conform.input(fields[typeQueryParam], { type: 'hidden' })}
259265
/>

app/routes/settings+/profile.change-email.index/index.tsx

Lines changed: 34 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,12 @@ import { commitSession, getSession } from '~/utils/session.server.ts'
1414
import { emailSchema } from '~/utils/user-validation.ts'
1515
import {
1616
prepareVerification,
17+
Verify,
1718
type VerifyFunctionArgs,
1819
} from '../../resources+/verify.tsx'
1920
import { EmailChangeEmail, EmailChangeNoticeEmail } from './email.server.tsx'
21+
import { shouldRequestTwoFA } from '~/routes/resources+/login.tsx'
22+
import { useUser } from '~/utils/user.ts'
2023

2124
export const newEmailAddressSessionKey = 'new-email-address'
2225

@@ -37,10 +40,15 @@ export async function loader({ request }: DataFunctionArgs) {
3740
description: 'You must login first to change your email',
3841
})
3942
}
40-
return json({ user })
43+
const shouldReverify = await shouldRequestTwoFA(request)
44+
return json({ user, shouldReverify })
4145
}
4246

4347
export async function action({ request }: DataFunctionArgs) {
48+
if (await shouldRequestTwoFA(request)) {
49+
// looks like they waited too long enter the email
50+
return redirect(request.url)
51+
}
4452
const userId = await requireUserId(request)
4553
const formData = await request.formData()
4654
const submission = await parse(formData, {
@@ -134,6 +142,7 @@ export async function handleVerification({
134142

135143
export default function ChangeEmailIndex() {
136144
const data = useLoaderData<typeof loader>()
145+
const user = useUser()
137146
const actionData = useActionData<typeof action>()
138147

139148
const [form, fields] = useForm({
@@ -153,21 +162,30 @@ export default function ChangeEmailIndex() {
153162
<p>
154163
An email notice will also be sent to your old address {data.user.email}.
155164
</p>
156-
<Form method="POST" {...form.props}>
157-
<Field
158-
labelProps={{ children: 'New Email' }}
159-
inputProps={conform.input(fields.email)}
160-
errors={fields.email.errors}
161-
/>
162-
<ErrorList id={form.errorId} errors={form.errors} />
163-
<div>
164-
<StatusButton
165-
status={isSubmitting ? 'pending' : actionData?.status ?? 'idle'}
166-
>
167-
Send Confirmation
168-
</StatusButton>
169-
</div>
170-
</Form>
165+
<div className="mx-auto mt-5 max-w-sm">
166+
{data.shouldReverify ? (
167+
<>
168+
<p>Please reverify your account by submitting your 2FA code</p>
169+
<Verify target={user.id} type="2fa" />
170+
</>
171+
) : (
172+
<Form method="POST" {...form.props}>
173+
<Field
174+
labelProps={{ children: 'New Email' }}
175+
inputProps={conform.input(fields.email)}
176+
errors={fields.email.errors}
177+
/>
178+
<ErrorList id={form.errorId} errors={form.errors} />
179+
<div>
180+
<StatusButton
181+
status={isSubmitting ? 'pending' : actionData?.status ?? 'idle'}
182+
>
183+
Send Confirmation
184+
</StatusButton>
185+
</div>
186+
</Form>
187+
)}
188+
</div>
171189
</div>
172190
)
173191
}

tests/e2e/settings-profile.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ test('Users can change their email address', async ({ page, login }) => {
9393
await page.getByRole('link', { name: /change email/i }).click()
9494
await page.getByRole('textbox', { name: /new email/i }).fill(newEmailAddress)
9595
await page.getByRole('button', { name: /send confirmation/i }).click()
96-
// await expect(page.getByText(/check your email/i)).toBeVisible()
96+
await expect(page.getByText(/check your email/i)).toBeVisible()
9797
const email = await waitFor(() => readEmail(newEmailAddress), {
9898
errorMessage: 'Confirmation email was not sent',
9999
})

0 commit comments

Comments
 (0)