Skip to content

Commit 32e5edb

Browse files
authored
fix(webapp): restore magic link login on the login page (#4220)
## Summary Magic link login could appear completely broken: submitting your email on the login page showed a stale "This email is unauthorized" error instead of the "we've sent you a magic link" confirmation, even when the address was fine. This PR reverts [#4215](#4215) (whose diagnosis and fix turned out to be wrong) and fixes the actual bug, which was in how login errors are stored and consumed. ## Root cause Two session bugs compounded on the login page: - The `/login` loader read the flashed `auth:error` without committing the session. A Remix flash is only consumed when the session is committed after the read, so once any attempt flashed an error (for example an address rejected on an instance with `WHITELISTED_EMAILS` set), it stayed in the session cookie and reappeared on every later `/login` visit, making successful attempts look like failures. - The `/login/magic` action stored its validation and rate limit errors with `session.set`, which survives every later read and commit, so those errors stuck permanently. [#4215](#4215) had instead diagnosed a server-only module leaking into the client bundle and crashing navigation. Checking the shipped images' client bundles via their sourcemaps shows `.server` modules were always stubbed out, so that change fixed nothing and is reverted here. ## Fix - `/login` reads the flashed error and commits the session when one was present, so an error renders once and clears. The `redirectTo` branch now surfaces the error too instead of leaving it in the cookie. - `/login/magic` flashes its errors instead of `set`ting them. Verified end-to-end on a live preview environment: a rejected address shows the error once and a reload clears it; a valid address lands on the confirmation screen with the address named; GitHub, Google, and SSO login paths are untouched by this diff.
1 parent 9f76c92 commit 32e5edb

7 files changed

Lines changed: 105 additions & 51 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+
Clearer login error when an email address is blocked by the WHITELISTED_EMAILS setting: the message now explains the address isn't allowed on this instance instead of the ambiguous "This email is unauthorized".
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
area: webapp
3+
type: fix
4+
---
5+
6+
Fixed stale login errors: an error from a previous login attempt (for example a rejected email address) no longer keeps reappearing on the login page and no longer makes later, successful attempts look like they failed.

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

Lines changed: 47 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,10 @@ import { isGithubAuthSupported, isGoogleAuthSupported } from "~/services/auth.se
2323
import { getLastAuthMethod } from "~/services/lastAuthMethod.server";
2424
import { commitSession, setRedirectTo } from "~/services/redirectTo.server";
2525
import { getUserId } from "~/services/session.server";
26-
import { getUserSession } from "~/services/sessionStorage.server";
26+
import {
27+
commitSession as commitUserSession,
28+
getUserSession,
29+
} from "~/services/sessionStorage.server";
2730
import { ssoController } from "~/services/sso.server";
2831
import { flags as getGlobalFlags } from "~/v3/featureFlags.server";
2932
import { requestUrl } from "~/utils/requestUrl.server";
@@ -111,8 +114,36 @@ export async function loader({ request }: LoaderFunctionArgs) {
111114
showSsoAuth = (globalFlags as Record<string, unknown>).hasSso === true;
112115
}
113116

117+
// Read the flashed auth error and, when one exists, commit the session so
118+
// the flash is consumed. Reading without committing leaves the flash in the
119+
// cookie, so a stale error (e.g. a once-rejected email address) would
120+
// resurface on every future /login visit and make later, successful login
121+
// attempts look like they failed.
122+
const userSession = await getUserSession(request);
123+
const error = userSession.get("auth:error");
124+
// get() only auto-clears flash-stored values. Sessions written before this
125+
// fix stored the error with set(), which survives get() + commit — unset it
126+
// explicitly so those sessions heal too instead of showing the stale error
127+
// until the cookie's one-year maxAge runs out.
128+
userSession.unset("auth:error");
129+
130+
let authError: string | undefined;
131+
if (error) {
132+
if ("message" in error) {
133+
authError = error.message;
134+
} else {
135+
authError = JSON.stringify(error, null, 2);
136+
}
137+
}
138+
139+
const headers = new Headers();
140+
if (error) {
141+
headers.append("Set-Cookie", await commitUserSession(userSession));
142+
}
143+
114144
if (redirectTo) {
115145
const session = await setRedirectTo(request, redirectTo);
146+
headers.append("Set-Cookie", await commitSession(session));
116147

117148
return typedjson(
118149
{
@@ -121,39 +152,26 @@ export async function loader({ request }: LoaderFunctionArgs) {
121152
showGoogleAuth: isGoogleAuthSupported,
122153
showSsoAuth,
123154
lastAuthMethod,
124-
authError: null,
155+
authError,
125156
notice,
126157
isVercelMarketplace: redirectTo.startsWith("/vercel/callback"),
127158
},
128-
{
129-
headers: {
130-
"Set-Cookie": await commitSession(session),
131-
},
132-
}
159+
{ headers }
133160
);
134161
} else {
135-
const session = await getUserSession(request);
136-
const error = session.get("auth:error");
137-
138-
let authError: string | undefined;
139-
if (error) {
140-
if ("message" in error) {
141-
authError = error.message;
142-
} else {
143-
authError = JSON.stringify(error, null, 2);
144-
}
145-
}
146-
147-
return typedjson({
148-
redirectTo: null,
149-
showGithubAuth: isGithubAuthSupported,
150-
showGoogleAuth: isGoogleAuthSupported,
151-
showSsoAuth,
152-
lastAuthMethod,
153-
authError,
154-
notice,
155-
isVercelMarketplace: false,
156-
});
162+
return typedjson(
163+
{
164+
redirectTo: null,
165+
showGithubAuth: isGithubAuthSupported,
166+
showGoogleAuth: isGoogleAuthSupported,
167+
showSsoAuth,
168+
lastAuthMethod,
169+
authError,
170+
notice,
171+
isVercelMarketplace: false,
172+
},
173+
{ headers }
174+
);
157175
}
158176
}
159177

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import { createCookie } from "@remix-run/node";
2+
import { env } from "~/env.server";
3+
4+
// Carries the submitted email to the confirmation screen in a short-lived,
5+
// httpOnly cookie rather than the URL, so the address never lands in access
6+
// logs, browser history, or error-tracker breadcrumbs. Lives in a .server
7+
// module: it calls createCookie at import time using server-only env, which
8+
// throws if it ever evaluates in the client bundle.
9+
export const magicLinkEmailCookie = createCookie("magiclink-email", {
10+
maxAge: 60 * 10,
11+
httpOnly: true,
12+
sameSite: "lax",
13+
secure: env.NODE_ENV === "production",
14+
path: "/",
15+
});

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

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import { ssoRedirectForEmail } from "~/services/ssoAutoDiscovery.server";
3434
import { logger, tryCatch } from "@trigger.dev/core/v3";
3535
import { env } from "~/env.server";
3636
import { extractClientIp } from "~/utils/extractClientIp.server";
37+
import { magicLinkEmailCookie } from "./magicLinkEmailCookie.server";
3738

3839
export const meta: MetaFunction = ({ matches }) => {
3940
const parentMeta = matches
@@ -73,14 +74,12 @@ export async function loader({ request }: LoaderFunctionArgs) {
7374
// confirmation renders the flashed error as magicLinkError below.
7475
const url = new URL(request.url);
7576
const sanitized = sanitizeRedirectPath(url.searchParams.get("redirectTo"));
76-
// The email-link strategy stores the submitted address in the session
77-
// (`auth:email`) alongside the magic-link key, so read it from there to name
78-
// the confirmation — no address in the URL, and no separate cookie to leak
79-
// into the client bundle. Validate before echoing it back.
80-
const emailValue = session.get("auth:email");
77+
// The submitted address is carried in a short-lived cookie (not the URL) so
78+
// the confirmation can name it. Validate before echoing it back.
79+
const emailCookie = await magicLinkEmailCookie.parse(request.headers.get("Cookie"));
8180
const email =
82-
typeof emailValue === "string" && z.string().email().safeParse(emailValue).success
83-
? emailValue
81+
typeof emailCookie === "string" && z.string().email().safeParse(emailCookie).success
82+
? emailCookie
8483
: null;
8584
if (!session.has("triggerdotdev:magiclink")) {
8685
// Throw (not return) so the redirect doesn't widen the loader's return
@@ -92,6 +91,9 @@ export async function loader({ request }: LoaderFunctionArgs) {
9291
}
9392

9493
const error = session.get("auth:error");
94+
// Same migration hygiene as /login: pre-fix sessions stored this with
95+
// set(), which get() doesn't clear — unset so the commit below removes it.
96+
session.unset("auth:error");
9597

9698
const redirectTo = sanitized === "/" ? null : sanitized;
9799
const headers = new Headers();
@@ -142,7 +144,9 @@ export async function action({ request }: ActionFunctionArgs) {
142144

143145
if (!result.success) {
144146
const session = await getUserSession(request);
145-
session.set("auth:error", {
147+
// flash, not set: a set() key survives every later read/commit, so the
148+
// error would reappear on each /login visit long after the failed attempt.
149+
session.flash("auth:error", {
146150
message: "Please enter a valid email address.",
147151
});
148152

@@ -192,7 +196,8 @@ export async function action({ request }: ActionFunctionArgs) {
192196
: "Failed sending magic link. Please try again shortly.";
193197

194198
const session = await getUserSession(request);
195-
session.set("auth:error", {
199+
// flash, not set — same one-shot semantics as the validation error above.
200+
session.flash("auth:error", {
196201
message: errorMessage,
197202
});
198203

@@ -216,13 +221,20 @@ export async function action({ request }: ActionFunctionArgs) {
216221
return redirect(ssoRedirect);
217222
}
218223

219-
// The email-link strategy stores the address in the session (`auth:email`)
220-
// and throws its own redirect Response (with the committed session cookie),
221-
// so return it directly — the confirmation reads the email from the session.
222-
return await authenticator.authenticate("email-link", request, {
223-
successRedirect: "/login/magic",
224-
failureRedirect: "/login",
225-
});
224+
// authenticator.authenticate throws its redirect Response; attach the
225+
// sent-to email as a short-lived cookie so the confirmation can name it
226+
// without putting the address in the URL.
227+
try {
228+
return await authenticator.authenticate("email-link", request, {
229+
successRedirect: "/login/magic",
230+
failureRedirect: "/login",
231+
});
232+
} catch (thrown) {
233+
if (thrown instanceof Response) {
234+
thrown.headers.append("Set-Cookie", await magicLinkEmailCookie.serialize(email));
235+
}
236+
throw thrown;
237+
}
226238
}
227239
case "reset":
228240
default: {

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,6 @@ const emailStrategy = new EmailLinkStrategy(
1818
secret,
1919
callbackURL: "/magic",
2020
sessionMagicLinkKey: "triggerdotdev:magiclink",
21-
// Pin explicitly to the library default rather than relying on it: the
22-
// /login/magic loader reads the submitted address via
23-
// session.get("auth:email") to name it on the confirmation screen, so a
24-
// future remix-auth-email-link default change can't silently break that.
25-
sessionEmailKey: "auth:email",
2621
},
2722
async ({
2823
email,

apps/webapp/app/utils/email.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ export function assertEmailAllowed(email: string) {
77
}
88

99
if (!emailMatchesPattern(env.WHITELISTED_EMAILS, email)) {
10-
throw new Error("This email is unauthorized");
10+
// Surfaced verbatim on the login page. Name the actual policy so a
11+
// rejection on a restricted instance reads as configuration, not a bug.
12+
throw new Error("This email address isn't allowed to sign in on this instance.");
1113
}
1214
}

0 commit comments

Comments
 (0)