Skip to content

Commit c2cc5a9

Browse files
ericallamTrigger.dev RepoOps
authored andcommitted
fix(webapp): build magic links from LOGIN_ORIGIN
Magic-link sign-in emails now always link to `LOGIN_ORIGIN`, whatever host the sign-in request came in on. Mono-RevId: 7977b5e15f67095f5bba4912121c4d92d906e9b2
1 parent 74d961a commit c2cc5a9

6 files changed

Lines changed: 131 additions & 1 deletion

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: fix
4+
---
5+
6+
Magic-link sign-in emails now always link to `LOGIN_ORIGIN`, whatever host the request came in on.

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { findOrCreateUser } from "~/models/user.server";
55
import { sendMagicLinkEmail } from "~/services/email.server";
66
import type { AuthUser } from "./authUser";
77
import { logger } from "./logger.server";
8+
import { pinMagicLinkOrigin } from "./magicLinkOrigin.server";
89

910
import { postAuthentication } from "./postAuth.server";
1011
import { SsoRequiredError, ssoRedirectForEmail } from "./ssoAutoDiscovery.server";
@@ -74,6 +75,7 @@ const emailStrategy = new EmailLinkStrategy(
7475
},
7576
verifyMagicLink
7677
);
78+
pinMagicLinkOrigin(emailStrategy, env.LOGIN_ORIGIN);
7779

7880
export function addEmailLinkStrategy(authenticator: Authenticator<AuthUser>) {
7981
authenticator.use(emailStrategy);
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
import { createCookieSessionStorage } from "@remix-run/node";
2+
import { EmailLinkStrategy } from "remix-auth-email-link";
3+
import { describe, expect, it } from "vitest";
4+
import { pinMagicLinkOrigin } from "./magicLinkOrigin.server";
5+
6+
const LOGIN_ORIGIN = "https://cloud.example";
7+
8+
function createStrategy() {
9+
const sentLinks: string[] = [];
10+
const strategy = new EmailLinkStrategy<{ id: string }>(
11+
{
12+
secret: "test-magic-link-secret",
13+
callbackURL: "/magic",
14+
sendEmail: async ({ magicLink }) => {
15+
sentLinks.push(magicLink);
16+
},
17+
},
18+
async () => ({ id: "user" })
19+
);
20+
return { strategy, sentLinks };
21+
}
22+
23+
async function requestLink(strategy: EmailLinkStrategy<{ id: string }>, headers: HeadersInit) {
24+
const sessionStorage = createCookieSessionStorage({
25+
cookie: { name: "__session", secrets: ["test-session-secret"] },
26+
});
27+
const request = new Request("https://cloud.example/login/magic", {
28+
method: "POST",
29+
headers: { "Content-Type": "application/x-www-form-urlencoded", ...headers },
30+
body: new URLSearchParams({ email: "someone@example.com" }),
31+
});
32+
33+
const thrown = await strategy
34+
.authenticate(request, sessionStorage, {
35+
name: "email-link",
36+
sessionKey: "user",
37+
sessionErrorKey: "auth:error",
38+
sessionStrategyKey: "strategy",
39+
successRedirect: "/login/magic",
40+
failureRedirect: "/login",
41+
})
42+
.catch((error: unknown) => error);
43+
44+
expect(thrown).toBeInstanceOf(Response);
45+
expect((thrown as Response).headers.get("Location")).toBe("/login/magic");
46+
}
47+
48+
describe("pinMagicLinkOrigin", () => {
49+
it("builds the link from the request's forwarded host without it", async () => {
50+
const { strategy, sentLinks } = createStrategy();
51+
52+
await requestLink(strategy, { Host: "cloud.example", "X-Forwarded-Host": "attacker.example" });
53+
54+
expect(new URL(sentLinks[0]).host).toBe("attacker.example");
55+
});
56+
57+
it("ignores a forged X-Forwarded-Host", async () => {
58+
const { strategy, sentLinks } = createStrategy();
59+
pinMagicLinkOrigin(strategy, LOGIN_ORIGIN);
60+
61+
await requestLink(strategy, {
62+
Host: "cloud.example",
63+
"X-Forwarded-Host": "attacker.example",
64+
"X-Forwarded-Proto": "http",
65+
});
66+
67+
expect(sentLinks).toHaveLength(1);
68+
expect(sentLinks[0].startsWith(`${LOGIN_ORIGIN}/magic?token=`)).toBe(true);
69+
});
70+
71+
it("ignores a forged Host", async () => {
72+
const { strategy, sentLinks } = createStrategy();
73+
pinMagicLinkOrigin(strategy, LOGIN_ORIGIN);
74+
75+
await requestLink(strategy, { Host: "attacker.example" });
76+
77+
expect(sentLinks[0].startsWith(`${LOGIN_ORIGIN}/magic?token=`)).toBe(true);
78+
});
79+
80+
it("normalizes an origin given with a trailing slash or path", async () => {
81+
const { strategy, sentLinks } = createStrategy();
82+
pinMagicLinkOrigin(strategy, "https://cloud.example/some/path/");
83+
84+
await requestLink(strategy, { Host: "cloud.example" });
85+
86+
expect(sentLinks[0].startsWith("https://cloud.example/magic?token=")).toBe(true);
87+
});
88+
89+
it("throws when the strategy no longer has getDomainURL", () => {
90+
expect(() => pinMagicLinkOrigin({}, LOGIN_ORIGIN)).toThrow(/getDomainURL/);
91+
});
92+
});
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/**
2+
* Makes an email-link strategy build every magic link on `origin`.
3+
*
4+
* `remix-auth-email-link` builds the link from the request's `X-Forwarded-Host`
5+
* (falling back to `Host`). Both are client-controlled, so a request for someone
6+
* else's address could make us email them a genuine link pointing at another
7+
* host, which would then receive the token. Links must only ever point at our
8+
* login origin.
9+
*
10+
* The library keeps `getDomainURL` private, so this replaces it on the instance.
11+
* It throws if the method is gone, so a library upgrade that renames it fails at
12+
* boot instead of silently reverting to header-derived links.
13+
*/
14+
export function pinMagicLinkOrigin(strategy: object, origin: string): void {
15+
if (typeof (strategy as { getDomainURL?: unknown }).getDomainURL !== "function") {
16+
throw new Error(
17+
"remix-auth-email-link no longer builds magic links with getDomainURL; update pinMagicLinkOrigin"
18+
);
19+
}
20+
21+
const pinnedOrigin = new URL(origin).origin;
22+
Object.defineProperty(strategy, "getDomainURL", {
23+
value: () => pinnedOrigin,
24+
configurable: true,
25+
writable: true,
26+
});
27+
}

‎apps/webapp/test/emailAuthVerify.test.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@ const mocks = vi.hoisted(() => ({
77
})),
88
}));
99

10-
vi.mock("~/env.server", () => ({ env: { MAGIC_LINK_SECRET: "test-magic-link-secret" } }));
10+
vi.mock("~/env.server", () => ({
11+
env: { MAGIC_LINK_SECRET: "test-magic-link-secret", LOGIN_ORIGIN: "https://cloud.example" },
12+
}));
1113
vi.mock("~/models/user.server", () => ({ findOrCreateUser: mocks.findOrCreateUser }));
1214
vi.mock("~/services/email.server", () => ({ sendMagicLinkEmail: async () => {} }));
1315
vi.mock("~/services/postAuth.server", () => ({ postAuthentication: async () => {} }));

‎apps/webapp/vitest.config.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ export default defineConfig({
2222
"app/services/sessionAuthorizationTelemetry.server.test.ts",
2323
"app/services/previewAutoArchive.server.test.ts",
2424
"app/services/provisionBillingCustomer.server.test.ts",
25+
"app/services/magicLinkOrigin.server.test.ts",
2526
"app/services/turnstile.server.test.ts",
2627
"app/services/clickhouse/**/*.test.ts",
2728
"app/utils/**/*.test.ts",

0 commit comments

Comments
 (0)