Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .server-changes/clearer-whitelist-login-error.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
area: webapp
type: improvement
---

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".
6 changes: 6 additions & 0 deletions .server-changes/consume-stale-login-errors.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
area: webapp
type: fix
---

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.
76 changes: 47 additions & 29 deletions apps/webapp/app/routes/login._index/route.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ import { isGithubAuthSupported, isGoogleAuthSupported } from "~/services/auth.se
import { getLastAuthMethod } from "~/services/lastAuthMethod.server";
import { commitSession, setRedirectTo } from "~/services/redirectTo.server";
import { getUserId } from "~/services/session.server";
import { getUserSession } from "~/services/sessionStorage.server";
import {
commitSession as commitUserSession,
getUserSession,
} from "~/services/sessionStorage.server";
import { ssoController } from "~/services/sso.server";
import { flags as getGlobalFlags } from "~/v3/featureFlags.server";
import { requestUrl } from "~/utils/requestUrl.server";
Expand Down Expand Up @@ -111,8 +114,36 @@ export async function loader({ request }: LoaderFunctionArgs) {
showSsoAuth = (globalFlags as Record<string, unknown>).hasSso === true;
}

// Read the flashed auth error and, when one exists, commit the session so
// the flash is consumed. Reading without committing leaves the flash in the
// cookie, so a stale error (e.g. a once-rejected email address) would
// resurface on every future /login visit and make later, successful login
// attempts look like they failed.
const userSession = await getUserSession(request);
const error = userSession.get("auth:error");
// get() only auto-clears flash-stored values. Sessions written before this
// fix stored the error with set(), which survives get() + commit — unset it
// explicitly so those sessions heal too instead of showing the stale error
// until the cookie's one-year maxAge runs out.
userSession.unset("auth:error");

let authError: string | undefined;
if (error) {
if ("message" in error) {
authError = error.message;
} else {
authError = JSON.stringify(error, null, 2);
}
}

const headers = new Headers();
if (error) {
headers.append("Set-Cookie", await commitUserSession(userSession));
}
Comment thread
samejr marked this conversation as resolved.

if (redirectTo) {
const session = await setRedirectTo(request, redirectTo);
headers.append("Set-Cookie", await commitSession(session));

return typedjson(
{
Expand All @@ -121,39 +152,26 @@ export async function loader({ request }: LoaderFunctionArgs) {
showGoogleAuth: isGoogleAuthSupported,
showSsoAuth,
lastAuthMethod,
authError: null,
authError,
Comment thread
matt-aitken marked this conversation as resolved.
notice,
isVercelMarketplace: redirectTo.startsWith("/vercel/callback"),
},
{
headers: {
"Set-Cookie": await commitSession(session),
},
}
{ headers }
);
} else {
const session = await getUserSession(request);
const error = session.get("auth:error");

let authError: string | undefined;
if (error) {
if ("message" in error) {
authError = error.message;
} else {
authError = JSON.stringify(error, null, 2);
}
}

return typedjson({
redirectTo: null,
showGithubAuth: isGithubAuthSupported,
showGoogleAuth: isGoogleAuthSupported,
showSsoAuth,
lastAuthMethod,
authError,
notice,
isVercelMarketplace: false,
});
return typedjson(
{
redirectTo: null,
showGithubAuth: isGithubAuthSupported,
showGoogleAuth: isGoogleAuthSupported,
showSsoAuth,
lastAuthMethod,
authError,
notice,
isVercelMarketplace: false,
},
{ headers }
);
}
}

Expand Down
15 changes: 15 additions & 0 deletions apps/webapp/app/routes/login.magic/magicLinkEmailCookie.server.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { createCookie } from "@remix-run/node";
import { env } from "~/env.server";

// Carries the submitted email to the confirmation screen in a short-lived,
// httpOnly cookie rather than the URL, so the address never lands in access
// logs, browser history, or error-tracker breadcrumbs. Lives in a .server
// module: it calls createCookie at import time using server-only env, which
// throws if it ever evaluates in the client bundle.
export const magicLinkEmailCookie = createCookie("magiclink-email", {
maxAge: 60 * 10,
httpOnly: true,
sameSite: "lax",
secure: env.NODE_ENV === "production",
path: "/",
});
44 changes: 28 additions & 16 deletions apps/webapp/app/routes/login.magic/route.tsx
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import { ssoRedirectForEmail } from "~/services/ssoAutoDiscovery.server";
import { logger, tryCatch } from "@trigger.dev/core/v3";
import { env } from "~/env.server";
import { extractClientIp } from "~/utils/extractClientIp.server";
import { magicLinkEmailCookie } from "./magicLinkEmailCookie.server";

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

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

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

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

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

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

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

// The email-link strategy stores the address in the session (`auth:email`)
// and throws its own redirect Response (with the committed session cookie),
// so return it directly — the confirmation reads the email from the session.
return await authenticator.authenticate("email-link", request, {
successRedirect: "/login/magic",
failureRedirect: "/login",
});
// authenticator.authenticate throws its redirect Response; attach the
// sent-to email as a short-lived cookie so the confirmation can name it
// without putting the address in the URL.
try {
return await authenticator.authenticate("email-link", request, {
successRedirect: "/login/magic",
failureRedirect: "/login",
});
} catch (thrown) {
if (thrown instanceof Response) {
thrown.headers.append("Set-Cookie", await magicLinkEmailCookie.serialize(email));
}
throw thrown;
}
}
case "reset":
default: {
Expand Down
5 changes: 0 additions & 5 deletions apps/webapp/app/services/emailAuth.server.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,6 @@ const emailStrategy = new EmailLinkStrategy(
secret,
callbackURL: "/magic",
sessionMagicLinkKey: "triggerdotdev:magiclink",
// Pin explicitly to the library default rather than relying on it: the
// /login/magic loader reads the submitted address via
// session.get("auth:email") to name it on the confirmation screen, so a
// future remix-auth-email-link default change can't silently break that.
sessionEmailKey: "auth:email",
},
async ({
email,
Expand Down
4 changes: 3 additions & 1 deletion apps/webapp/app/utils/email.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ export function assertEmailAllowed(email: string) {
}

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