Skip to content

Commit b113e5a

Browse files
ericallamericallam
authored
Fix non-MFA login (#2246)
* Fix non-MFA login * fixed typecheck errors
1 parent b119a52 commit b113e5a

File tree

5 files changed

+100
-110
lines changed

5 files changed

+100
-110
lines changed

‎apps/webapp/app/routes/auth.github.callback.tsx

Lines changed: 46 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,53 +1,58 @@
11
import type { LoaderFunction } from "@remix-run/node";
22
import { redirect } from "@remix-run/node";
3+
import { prisma } from "~/db.server";
4+
import { getSession, redirectWithErrorMessage } from "~/models/message.server";
35
import { authenticator } from "~/services/auth.server";
6+
import { commitSession } from "~/services/sessionStorage.server";
47
import { redirectCookie } from "./auth.github";
5-
import { getUserSession, commitSession } from "~/services/sessionStorage.server";
6-
import { logger } from "~/services/logger.server";
7-
import { MfaRequiredError } from "~/services/mfa/multiFactorAuthentication.server";
88

99
export let loader: LoaderFunction = async ({ request }) => {
10-
try {
11-
const cookie = request.headers.get("Cookie");
12-
const redirectValue = await redirectCookie.parse(cookie);
13-
const redirectTo = redirectValue ?? "/";
10+
const cookie = request.headers.get("Cookie");
11+
const redirectValue = await redirectCookie.parse(cookie);
12+
const redirectTo = redirectValue ?? "/";
13+
14+
const auth = await authenticator.authenticate("github", request, {
15+
failureRedirect: "/login", // If auth fails, the failureRedirect will be thrown as a Response
16+
});
17+
18+
// manually get the session
19+
const session = await getSession(request.headers.get("cookie"));
20+
21+
const userRecord = await prisma.user.findFirst({
22+
where: {
23+
id: auth.userId,
24+
},
25+
select: {
26+
id: true,
27+
mfaEnabledAt: true,
28+
},
29+
});
30+
31+
if (!userRecord) {
32+
return redirectWithErrorMessage(
33+
"/login",
34+
request,
35+
"Could not find your account. Please contact support."
36+
);
37+
}
1438

15-
logger.debug("auth.github.callback loader", {
16-
redirectTo,
17-
});
39+
if (userRecord.mfaEnabledAt) {
40+
session.set("pending-mfa-user-id", userRecord.id);
41+
session.set("pending-mfa-redirect-to", redirectTo);
1842

19-
const authuser = await authenticator.authenticate("github", request, {
20-
successRedirect: undefined, // Don't auto-redirect, we'll handle it
21-
failureRedirect: undefined, // Don't auto-redirect on failure either
43+
return redirect("/login/mfa", {
44+
headers: {
45+
"Set-Cookie": await commitSession(session),
46+
},
2247
});
48+
}
2349

24-
logger.debug("auth.github.callback authuser", {
25-
authuser,
26-
});
50+
// and store the user data
51+
session.set(authenticator.sessionKey, auth);
2752

28-
// If we get here, user doesn't have MFA - complete login normally
29-
return redirect(redirectTo);
30-
} catch (error) {
31-
// Check if this is an MFA_REQUIRED error
32-
if (error instanceof MfaRequiredError) {
33-
// User has MFA enabled - store pending user ID and redirect to MFA page
34-
const session = await getUserSession(request);
35-
session.set("pending-mfa-user-id", error.userId);
36-
37-
const cookie = request.headers.get("Cookie");
38-
const redirectValue = await redirectCookie.parse(cookie);
39-
const redirectTo = redirectValue ?? "/";
40-
session.set("pending-mfa-redirect-to", redirectTo);
41-
42-
return redirect("/login/mfa", {
43-
headers: {
44-
"Set-Cookie": await commitSession(session),
45-
},
46-
});
47-
}
48-
49-
// Regular authentication failure, redirect to login page
50-
logger.debug("auth.github.callback error", { error });
51-
return redirect("/login");
52-
}
53+
return redirect(redirectTo, {
54+
headers: {
55+
"Set-Cookie": await commitSession(session),
56+
},
57+
});
5358
};

‎apps/webapp/app/routes/magic.tsx

Lines changed: 47 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,56 @@
11
import type { LoaderFunctionArgs } from "@remix-run/server-runtime";
22
import { redirect } from "@remix-run/server-runtime";
3+
import { prisma } from "~/db.server";
4+
import { redirectWithErrorMessage } from "~/models/message.server";
35
import { authenticator } from "~/services/auth.server";
4-
import { MfaRequiredError } from "~/services/mfa/multiFactorAuthentication.server";
56
import { getRedirectTo } from "~/services/redirectTo.server";
6-
import { getUserSession, commitSession } from "~/services/sessionStorage.server";
7+
import { commitSession, getSession } from "~/services/sessionStorage.server";
78

89
export async function loader({ request }: LoaderFunctionArgs) {
9-
try {
10-
// Attempt to authenticate the user with email-link
11-
const authUser = await authenticator.authenticate("email-link", request, {
12-
successRedirect: undefined, // Don't auto-redirect, we'll handle it
13-
failureRedirect: undefined, // Don't auto-redirect on failure either
14-
});
10+
const redirectTo = await getRedirectTo(request);
11+
12+
const auth = await authenticator.authenticate("email-link", request, {
13+
failureRedirect: "/login/magic", // If auth fails, the failureRedirect will be thrown as a Response
14+
});
15+
16+
// manually get the session
17+
const session = await getSession(request.headers.get("cookie"));
18+
19+
const userRecord = await prisma.user.findFirst({
20+
where: {
21+
id: auth.userId,
22+
},
23+
select: {
24+
id: true,
25+
mfaEnabledAt: true,
26+
},
27+
});
28+
29+
if (!userRecord) {
30+
return redirectWithErrorMessage(
31+
"/login/magic",
32+
request,
33+
"Could not find your account. Please contact support."
34+
);
35+
}
36+
37+
if (userRecord.mfaEnabledAt) {
38+
session.set("pending-mfa-user-id", userRecord.id);
39+
session.set("pending-mfa-redirect-to", redirectTo ?? "/");
1540

16-
// If we get here, user doesn't have MFA - complete login normally
17-
const redirectTo = await getRedirectTo(request);
18-
return redirect(redirectTo ?? "/");
19-
} catch (error) {
20-
// Check if this is an MFA_REQUIRED error
21-
if (error instanceof MfaRequiredError) {
22-
// User has MFA enabled - store pending user ID and redirect to MFA page
23-
const session = await getUserSession(request);
24-
session.set("pending-mfa-user-id", error.userId);
25-
26-
const redirectTo = await getRedirectTo(request);
27-
session.set("pending-mfa-redirect-to", redirectTo ?? "/");
28-
29-
return redirect("/login/mfa", {
30-
headers: {
31-
"Set-Cookie": await commitSession(session),
32-
},
33-
});
34-
}
35-
36-
// Regular authentication failure, redirect to magic link page
37-
return redirect("/login/magic");
41+
return redirect("/login/mfa", {
42+
headers: {
43+
"Set-Cookie": await commitSession(session),
44+
},
45+
});
3846
}
47+
48+
// and store the user data
49+
session.set(authenticator.sessionKey, auth);
50+
51+
return redirect(redirectTo ?? "/", {
52+
headers: {
53+
"Set-Cookie": await commitSession(session),
54+
},
55+
});
3956
}

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

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
import { EmailLinkStrategy } from "remix-auth-email-link";
21
import type { Authenticator } from "remix-auth";
3-
import type { AuthUser } from "./authUser";
4-
import { findOrCreateUser } from "~/models/user.server";
2+
import { EmailLinkStrategy } from "remix-auth-email-link";
53
import { env } from "~/env.server";
4+
import { findOrCreateUser } from "~/models/user.server";
65
import { sendMagicLinkEmail } from "~/services/email.server";
7-
import { postAuthentication } from "./postAuth.server";
6+
import type { AuthUser } from "./authUser";
87
import { logger } from "./logger.server";
9-
import { MfaRequiredError } from "./mfa/multiFactorAuthentication.server";
8+
9+
import { postAuthentication } from "./postAuth.server";
1010

1111
let secret = env.MAGIC_LINK_SECRET;
1212
if (!secret) throw new Error("Missing MAGIC_LINK_SECRET env variable.");
@@ -37,19 +37,8 @@ const emailStrategy = new EmailLinkStrategy(
3737

3838
await postAuthentication({ user, isNewUser, loginMethod: "MAGIC_LINK" });
3939

40-
// Check if user has MFA enabled
41-
if (user.mfaEnabledAt) {
42-
// Throw a special error that will be caught by the magic route
43-
throw new MfaRequiredError(user.id);
44-
}
45-
4640
return { userId: user.id };
4741
} catch (error) {
48-
// Skip logging the error if it's a MfaRequiredError
49-
if (error instanceof MfaRequiredError) {
50-
throw error;
51-
}
52-
5342
logger.debug("Magic link user failed to authenticate", { error: JSON.stringify(error) });
5443
throw error;
5544
}

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

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,8 @@ import { GitHubStrategy } from "remix-auth-github";
33
import { env } from "~/env.server";
44
import { findOrCreateUser } from "~/models/user.server";
55
import type { AuthUser } from "./authUser";
6-
import { postAuthentication } from "./postAuth.server";
76
import { logger } from "./logger.server";
8-
import { MfaRequiredError } from "./mfa/multiFactorAuthentication.server";
7+
import { postAuthentication } from "./postAuth.server";
98

109
export function addGitHubStrategy(
1110
authenticator: Authenticator<AuthUser>,
@@ -41,22 +40,11 @@ export function addGitHubStrategy(
4140

4241
await postAuthentication({ user, isNewUser, loginMethod: "GITHUB" });
4342

44-
// Check if user has MFA enabled
45-
if (user.mfaEnabledAt) {
46-
// Throw a special error that will be caught by the callback route
47-
throw new MfaRequiredError(user.id);
48-
}
49-
5043
return {
5144
userId: user.id,
5245
};
5346
} catch (error) {
54-
// Skip logging the error if it's a MfaRequiredError
55-
if (error instanceof MfaRequiredError) {
56-
throw error;
57-
}
58-
59-
console.error(error);
47+
logger.error("GitHub login failed", { error: JSON.stringify(error) });
6048
throw error;
6149
}
6250
}

‎apps/webapp/app/services/mfa/multiFactorAuthentication.server.ts

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,6 @@ const SecretSchema = z.object({
1515
secret: z.string(),
1616
});
1717

18-
export class MfaRequiredError extends Error {
19-
public readonly userId: string;
20-
21-
constructor(userId: string) {
22-
super(`MFA is required for user ${userId}`);
23-
this.userId = userId;
24-
}
25-
}
26-
2718
export class MultiFactorAuthenticationService {
2819
#prismaClient: PrismaClient;
2920

0 commit comments

Comments
 (0)