From 75b34f8a0e83bc1ca82d62c6c9f7b8f64aea4017 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Mon, 31 Aug 2026 10:18:19 -0400 Subject: [PATCH] feat(magic-link): forward a redirect target to the auth API seamless-auth-api now takes an optional redirectUri on GET /magic-link deciding where the emailed link lands. The adapters called that route with no query, so the feature was reachable only by a backend calling the API directly and not by a browser or mobile client, which was most of the point. RequestMagicLinkInput gains an optional redirectUri, built onto the URL with the existing buildUpstreamUrl helper. Both adapters read it from the body of their own POST /magic-link. Omit it and the upstream URL is byte for byte what it was. Forwarded rather than checked. The API validates against the configured origins and answers 400, and an allowlist kept in two places is one that eventually disagrees with itself. A non-string is dropped rather than coerced so it cannot become a query parameter meaning something the caller did not send. Covered on both adapters. Only the forwarding underneath is shared, since each reads its own request body, so the fastify side goes in the parity suite next to the express one. --- .changeset/olive-pandas-forward.md | 29 ++++++ .../src/handlers/requestMagicLinkHandler.ts | 16 +++- .../tests/requestMagicLinkHandler.test.js | 94 +++++++++++++++++++ .../express/src/handlers/requestMagicLink.ts | 3 + .../express/tests/magicLinkRedirect.test.js | 93 ++++++++++++++++++ packages/fastify/src/routes/authRoutes.ts | 7 +- packages/fastify/tests/parity.test.js | 49 ++++++++++ 7 files changed, 289 insertions(+), 2 deletions(-) create mode 100644 .changeset/olive-pandas-forward.md create mode 100644 packages/core/tests/requestMagicLinkHandler.test.js create mode 100644 packages/express/tests/magicLinkRedirect.test.js diff --git a/.changeset/olive-pandas-forward.md b/.changeset/olive-pandas-forward.md new file mode 100644 index 0000000..2c487bf --- /dev/null +++ b/.changeset/olive-pandas-forward.md @@ -0,0 +1,29 @@ +--- +'@seamless-auth/core': minor +'@seamless-auth/express': minor +'@seamless-auth/fastify': minor +--- + +Forward a magic link destination to the auth API. + +`seamless-auth-api` now accepts an optional `redirectUri` on `GET /magic-link`, +deciding where the emailed link lands. Until now the adapters called that route with +no query, so the feature was reachable only by a backend calling the API directly. A +browser or mobile client could not use it, which was most of the point: a tenant with +both a web app and a mobile app needs each to receive a link that opens in the right +place. + +`RequestMagicLinkInput` gains an optional `redirectUri`, and both adapters read it +from the request body of their own `POST /magic-link` and pass it through. Omit it and +nothing changes: the upstream URL is exactly what it was, so no adopter has to do +anything. + +The adapters forward the value rather than checking it. The auth API validates it +against the configured origins and answers `400` if it is not allowed, and an +allowlist that lives in two places is one that eventually disagrees with itself. A +value that is not a string is dropped rather than coerced, so it cannot turn into a +query parameter meaning something the caller did not send. + +Requires an auth API that understands the parameter. Against an older one the +parameter is ignored and the link keeps the tenant-wide destination, which is the +behaviour adopters have today. diff --git a/packages/core/src/handlers/requestMagicLinkHandler.ts b/packages/core/src/handlers/requestMagicLinkHandler.ts index 70a1420..2db29a9 100644 --- a/packages/core/src/handlers/requestMagicLinkHandler.ts +++ b/packages/core/src/handlers/requestMagicLinkHandler.ts @@ -1,10 +1,18 @@ import { authFetch } from "../authFetch.js"; import { readPassthroughFailure } from "../upstreamError.js"; import { EXTERNAL_DELIVERY_HEADERS } from "../apiContract.js"; +import { buildUpstreamUrl } from "../proxyRequest.js"; import type { ResultFailure } from "../result.js"; export interface RequestMagicLinkInput { authorization?: string; + /** + * Where the emailed link should land, for a tenant whose web and mobile clients + * need different destinations. Forwarded as-is: the auth API validates it against + * the configured origins and answers 400 if it is not allowed, so the decision + * stays in one place rather than being made again here. + */ + redirectUri?: string; } export interface RequestMagicLinkOptions { @@ -23,7 +31,13 @@ export async function requestMagicLinkHandler( input: RequestMagicLinkInput, opts: RequestMagicLinkOptions, ): Promise { - const up = await authFetch(`${opts.authServerUrl}/magic-link`, { + const url = buildUpstreamUrl( + opts.authServerUrl, + "/magic-link", + input.redirectUri ? { redirectUri: input.redirectUri } : undefined, + ); + + const up = await authFetch(url, { method: "GET", authorization: input.authorization, forwardedClientIp: opts.forwardedClientIp, diff --git a/packages/core/tests/requestMagicLinkHandler.test.js b/packages/core/tests/requestMagicLinkHandler.test.js new file mode 100644 index 0000000..300cc9f --- /dev/null +++ b/packages/core/tests/requestMagicLinkHandler.test.js @@ -0,0 +1,94 @@ +import { jest } from "@jest/globals"; + +function jsonResponse(status, body) { + return new Response(JSON.stringify(body), { + status, + headers: { "Content-Type": "application/json" }, + }); +} + +async function load() { + const { requestMagicLinkHandler } = await import( + "../dist/handlers/requestMagicLinkHandler.js" + ); + + return requestMagicLinkHandler; +} + +function requestedUrl() { + return global.fetch.mock.calls[0][0]; +} + +describe("requestMagicLinkHandler", () => { + const originalFetch = global.fetch; + + beforeEach(() => { + global.fetch = jest + .fn() + .mockResolvedValue(jsonResponse(200, { message: "sent" })); + }); + + afterEach(() => { + global.fetch = originalFetch; + }); + + it("asks for the tenant default when no target is given", async () => { + const requestMagicLinkHandler = await load(); + + await requestMagicLinkHandler( + {}, + { authServerUrl: "https://auth.example.com" }, + ); + + expect(requestedUrl()).toBe("https://auth.example.com/magic-link"); + }); + + it("forwards a requested target as a query parameter", async () => { + const requestMagicLinkHandler = await load(); + + await requestMagicLinkHandler( + { redirectUri: "https://app.example.com/magic" }, + { authServerUrl: "https://auth.example.com" }, + ); + + expect(requestedUrl()).toBe( + "https://auth.example.com/magic-link?redirectUri=https%3A%2F%2Fapp.example.com%2Fmagic", + ); + }); + + it("encodes a target that carries its own query", async () => { + const requestMagicLinkHandler = await load(); + + await requestMagicLinkHandler( + { redirectUri: "https://app.example.com/magic?platform=ios" }, + { authServerUrl: "https://auth.example.com" }, + ); + + const url = new URL(requestedUrl()); + + expect(url.searchParams.get("redirectUri")).toBe( + "https://app.example.com/magic?platform=ios", + ); + }); + + /** + * The API is the only allowlist. Forwarding rather than judging keeps one place + * where a redirect is approved, so this asserts the refusal is passed back rather + * than that the adapter blocked it. + */ + it("passes an upstream refusal back to the caller", async () => { + const requestMagicLinkHandler = await load(); + + global.fetch.mockResolvedValue( + jsonResponse(400, { error: "Redirect URI is not allowed" }), + ); + + const result = await requestMagicLinkHandler( + { redirectUri: "https://evil.example/steal" }, + { authServerUrl: "https://auth.example.com" }, + ); + + expect(result.status).toBe(400); + expect(result.errorBody).toBeDefined(); + }); +}); diff --git a/packages/express/src/handlers/requestMagicLink.ts b/packages/express/src/handlers/requestMagicLink.ts index 79610cd..da87711 100644 --- a/packages/express/src/handlers/requestMagicLink.ts +++ b/packages/express/src/handlers/requestMagicLink.ts @@ -15,9 +15,12 @@ export async function requestMagicLink( res: Response, opts: SeamlessAuthServerOptions, ) { + const { redirectUri } = (req.body ?? {}) as { redirectUri?: unknown }; + const result = await requestMagicLinkHandler( { authorization: buildServiceAuthorization(req, opts), + redirectUri: typeof redirectUri === "string" ? redirectUri : undefined, }, { authServerUrl: opts.authServerUrl, diff --git a/packages/express/tests/magicLinkRedirect.test.js b/packages/express/tests/magicLinkRedirect.test.js new file mode 100644 index 0000000..d13a53e --- /dev/null +++ b/packages/express/tests/magicLinkRedirect.test.js @@ -0,0 +1,93 @@ +// Locks the path a magic link destination takes through the adapter: a browser +// posts it in the body, and the auth API expects it as a query parameter on a GET. +// The adapter forwards without judging it, because the API holds the allowlist. +import { jest } from "@jest/globals"; +import express from "express"; +import jwt from "jsonwebtoken"; +import request from "supertest"; + +const { default: createSeamlessAuthServer } = await import("../dist/index.js"); + +const COOKIE_SECRET = "cookie-secret-cookie-secret-cookie-secret"; + +function createJsonResponse(status, body) { + return { ok: status >= 200 && status < 300, status, json: async () => body }; +} + +function createPreAuthCookie() { + const token = jwt.sign( + { sub: "user-123", token: "ephemeral-token" }, + COOKIE_SECRET, + { algorithm: "HS256", expiresIn: "300s" }, + ); + + return `seamless-ephemeral=${token}`; +} + +function createApp() { + const app = express(); + + app.use( + "/auth", + createSeamlessAuthServer({ + authServerUrl: "https://auth.example.com", + cookieSecret: COOKIE_SECRET, + serviceSecret: "service-secret-service-secret-service-secret", + audience: "https://auth.example.com", + jwksKid: "test-main", + }), + ); + + return app; +} + +describe("magic link redirect target", () => { + const originalFetch = global.fetch; + let requestedUrl; + + beforeEach(() => { + requestedUrl = undefined; + global.fetch = jest.fn(async (url) => { + requestedUrl = String(url); + return createJsonResponse(200, { message: "sent" }); + }); + }); + + afterEach(() => { + global.fetch = originalFetch; + }); + + async function post(body) { + const req = request(createApp()) + .post("/auth/magic-link") + .set("Cookie", createPreAuthCookie()); + + await (body === undefined ? req : req.send(body)); + + return requestedUrl; + } + + it("forwards a target from the request body as a query parameter", async () => { + const url = await post({ redirectUri: "https://app.example.com/magic" }); + + expect(new URL(url).searchParams.get("redirectUri")).toBe( + "https://app.example.com/magic", + ); + }); + + it("asks for the tenant default when the body carries no target", async () => { + expect(await post({})).toBe("https://auth.example.com/magic-link"); + }); + + it("asks for the tenant default when there is no body at all", async () => { + expect(await post()).toBe("https://auth.example.com/magic-link"); + }); + + // Anything the API cannot validate as a URL should reach it and be refused there, + // rather than being turned into a query parameter that means something else. + it("ignores a target that is not a string", async () => { + expect(await post({ redirectUri: { href: "https://evil.example" } })).toBe( + "https://auth.example.com/magic-link", + ); + }); +}); diff --git a/packages/fastify/src/routes/authRoutes.ts b/packages/fastify/src/routes/authRoutes.ts index d637fe2..13086f5 100644 --- a/packages/fastify/src/routes/authRoutes.ts +++ b/packages/fastify/src/routes/authRoutes.ts @@ -235,8 +235,13 @@ export function registerAuthRoutes( }); fastify.post("/magic-link", async (req, reply) => { + const { redirectUri } = (req.body ?? {}) as { redirectUri?: unknown }; + const result = await requestMagicLinkHandler( - { authorization: buildServiceAuthorization(req) }, + { + authorization: buildServiceAuthorization(req), + redirectUri: typeof redirectUri === "string" ? redirectUri : undefined, + }, { ...common(req), externalDelivery: Boolean(opts.messaging), diff --git a/packages/fastify/tests/parity.test.js b/packages/fastify/tests/parity.test.js index 81c8418..8714e15 100644 --- a/packages/fastify/tests/parity.test.js +++ b/packages/fastify/tests/parity.test.js @@ -639,3 +639,52 @@ describe("fastify and express console proxies agree", () => { expect(expressResult.status).toBeGreaterThanOrEqual(400); }); }); + +// A browser sends the magic link destination in the body; the auth API wants it as a +// query parameter on a GET. Asserted on both adapters because each reads its own +// request body, so only the forwarding underneath them is shared. +describe("both adapters forward a magic link destination", () => { + const originalFetch = global.fetch; + + afterEach(() => { + global.fetch = originalFetch; + }); + + async function upstreamUrl(run, payload) { + let requested; + + global.fetch = jest.fn(async (url) => { + requested = String(url); + return upstream(200, { message: "sent" }); + }); + + await run({ + method: "post", + path: "/magic-link", + cookie: preAuthCookie(), + payload, + }); + + return requested; + } + + it("sends a requested target as a query parameter", async () => { + const payload = { redirectUri: "https://app.example.com/magic" }; + + const viaF = await upstreamUrl(viaFastify, payload); + const viaE = await upstreamUrl(viaExpress, payload); + + expect(viaF).toBe(viaE); + expect(new URL(viaF).searchParams.get("redirectUri")).toBe( + "https://app.example.com/magic", + ); + }); + + it("asks for the tenant default when no target is given", async () => { + const viaF = await upstreamUrl(viaFastify, {}); + const viaE = await upstreamUrl(viaExpress, {}); + + expect(viaF).toBe(viaE); + expect(viaF).toBe("https://auth.example.com/magic-link"); + }); +});