Skip to content

Commit f020c80

Browse files
committed
Better error ux
1 parent 71aa702 commit f020c80

File tree

1 file changed

+47
-38
lines changed

1 file changed

+47
-38
lines changed

apps/webapp/app/routes/login.mfa/route.tsx

Lines changed: 47 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { ActionFunctionArgs, LoaderFunctionArgs, MetaFunction } from "@remix-run/node";
22
import { redirect } from "@remix-run/node";
33
import { Form, useNavigation } from "@remix-run/react";
4-
import { useState } from "react";
4+
import React, { useState } from "react";
55
import { typedjson, useTypedLoaderData } from "remix-typedjson";
66
import { z } from "zod";
77
import { LoginPageLayout } from "~/components/LoginPageLayout";
@@ -16,8 +16,9 @@ import { Paragraph } from "~/components/primitives/Paragraph";
1616
import { Spinner } from "~/components/primitives/Spinner";
1717
import { authenticator } from "~/services/auth.server";
1818
import { commitSession, getUserSession, sessionStorage } from "~/services/sessionStorage.server";
19+
import { getSession as getMessageSession } from "~/models/message.server";
1920
import { MultiFactorAuthenticationService } from "~/services/mfa/multiFactorAuthentication.server";
20-
import { redirectWithErrorMessage } from "~/models/message.server";
21+
import { redirectWithErrorMessage, redirectBackWithErrorMessage } from "~/models/message.server";
2122
import { ServiceValidationError } from "~/v3/services/baseService.server";
2223
import { checkMfaRateLimit, MfaRateLimitError } from "~/services/mfa/mfaRateLimiter.server";
2324

@@ -55,15 +56,13 @@ export async function loader({ request }: LoaderFunctionArgs) {
5556
return redirect("/login");
5657
}
5758

58-
const error = session.get("auth:error");
59-
59+
// Get flash message for MFA errors
60+
const messageSession = await getMessageSession(request.headers.get("cookie"));
61+
const toastMessage = messageSession.get("toastMessage");
62+
6063
let mfaError: string | undefined;
61-
if (error) {
62-
if ("message" in error) {
63-
mfaError = error.message;
64-
} else {
65-
mfaError = JSON.stringify(error, null, 2);
66-
}
64+
if (toastMessage?.type === "error") {
65+
mfaError = toastMessage.message;
6766
}
6867

6968
return typedjson(
@@ -99,10 +98,7 @@ export async function action({ request }: ActionFunctionArgs) {
9998
const recoveryCode = payload.recoveryCode as string;
10099

101100
if (!recoveryCode) {
102-
session.set("auth:error", { message: "Recovery code is required" });
103-
return redirect("/login/mfa", {
104-
headers: { "Set-Cookie": await commitSession(session) },
105-
});
101+
return redirectBackWithErrorMessage(request, "Recovery code is required");
106102
}
107103

108104
// Rate limit MFA verification attempts
@@ -111,10 +107,7 @@ export async function action({ request }: ActionFunctionArgs) {
111107
const result = await mfaService.verifyRecoveryCodeForLogin(pendingUserId, recoveryCode);
112108

113109
if (!result.success) {
114-
session.set("auth:error", { message: result.error });
115-
return redirect("/login/mfa", {
116-
headers: { "Set-Cookie": await commitSession(session) },
117-
});
110+
return redirectBackWithErrorMessage(request, result.error || "Invalid authentication code");
118111
}
119112
// Recovery code verified - complete the login
120113
return await completeLogin(request, session, pendingUserId);
@@ -123,10 +116,7 @@ export async function action({ request }: ActionFunctionArgs) {
123116
const mfaCode = payload.mfaCode as string;
124117

125118
if (!mfaCode || mfaCode.length !== 6) {
126-
session.set("auth:error", { message: "Valid 6-digit code is required" });
127-
return redirect("/login/mfa", {
128-
headers: { "Set-Cookie": await commitSession(session) },
129-
});
119+
return redirectBackWithErrorMessage(request, "Valid 6-digit code is required");
130120
}
131121

132122
// Rate limit MFA verification attempts
@@ -135,10 +125,7 @@ export async function action({ request }: ActionFunctionArgs) {
135125
const result = await mfaService.verifyTotpForLogin(pendingUserId, mfaCode);
136126

137127
if (!result.success) {
138-
session.set("auth:error", { message: result.error });
139-
return redirect("/login/mfa", {
140-
headers: { "Set-Cookie": await commitSession(session) },
141-
});
128+
return redirectBackWithErrorMessage(request, result.error || "Invalid authentication code");
142129
}
143130

144131
// TOTP code verified - complete the login
@@ -153,11 +140,7 @@ export async function action({ request }: ActionFunctionArgs) {
153140
}
154141

155142
if (error instanceof MfaRateLimitError) {
156-
const session = await getUserSession(request);
157-
session.set("auth:error", { message: error.message });
158-
return redirect("/login/mfa", {
159-
headers: { "Set-Cookie": await commitSession(session) },
160-
});
143+
return redirectBackWithErrorMessage(request, error.message);
161144
}
162145

163146
throw error;
@@ -173,7 +156,6 @@ async function completeLogin(request: Request, session: any, userId: string) {
173156
const redirectTo = session.get("pending-mfa-redirect-to") ?? "/";
174157
session.unset("pending-mfa-user-id");
175158
session.unset("pending-mfa-redirect-to");
176-
session.unset("auth:error");
177159

178160
return redirect(redirectTo, {
179161
headers: {
@@ -184,16 +166,43 @@ async function completeLogin(request: Request, session: any, userId: string) {
184166

185167
export default function LoginMfaPage() {
186168
const data = useTypedLoaderData<typeof loader>();
187-
const mfaError = 'mfaError' in data ? data.mfaError : undefined;
169+
const rawMfaError = 'mfaError' in data ? data.mfaError : undefined;
188170
const navigate = useNavigation();
189171
const [showRecoveryCode, setShowRecoveryCode] = useState(false);
190172
const [mfaCode, setMfaCode] = useState("");
173+
const [hideError, setHideError] = useState(false);
174+
175+
// Clear the MFA code when form submission completes (success or failure)
176+
const prevNavigationState = React.useRef(navigate.state);
177+
React.useEffect(() => {
178+
if (prevNavigationState.current === "submitting" && navigate.state === "idle") {
179+
setMfaCode("");
180+
}
181+
prevNavigationState.current = navigate.state;
182+
}, [navigate.state]);
183+
184+
// Reset hideError when a new error appears
185+
React.useEffect(() => {
186+
if (rawMfaError) {
187+
setHideError(false);
188+
}
189+
}, [rawMfaError]);
190+
191+
// Clear error and MFA code when switching between modes
192+
const handleShowRecoveryCode = (show: boolean) => {
193+
setShowRecoveryCode(show);
194+
setHideError(true);
195+
if (!show) {
196+
setMfaCode("");
197+
}
198+
};
199+
200+
// Only show error if not explicitly hidden and we have an error
201+
const mfaError = hideError ? undefined : rawMfaError;
191202

192203
const isLoading =
193204
(navigate.state === "loading" || navigate.state === "submitting") &&
194-
navigate.formAction !== undefined &&
195-
(navigate.formData?.get("action") === "verify-mfa" ||
196-
navigate.formData?.get("action") === "verify-recovery");
205+
navigate.formAction !== undefined;
197206

198207
return (
199208
<LoginPageLayout>
@@ -240,7 +249,7 @@ export default function LoginMfaPage() {
240249
</Fieldset>
241250
<Button
242251
type="button"
243-
onClick={() => setShowRecoveryCode(false)}
252+
onClick={() => handleShowRecoveryCode(false)}
244253
variant="minimal/small"
245254
data-action="use authenticator app"
246255
className="mt-4"
@@ -292,7 +301,7 @@ export default function LoginMfaPage() {
292301
</Fieldset>
293302
<Button
294303
type="button"
295-
onClick={() => setShowRecoveryCode(true)}
304+
onClick={() => handleShowRecoveryCode(true)}
296305
variant="minimal/small"
297306
data-action="use recovery code"
298307
className="mt-4"

0 commit comments

Comments
 (0)