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"); + }); +});