From c638bdaf76d330ef393de74df2821f94df8245b6 Mon Sep 17 00:00:00 2001 From: Johnny Huirilef Date: Wed, 21 Oct 2026 15:56:53 -0300 Subject: [PATCH] fix(magic-link): refuse a cross-origin sign-in before the link is spent --- lib/magic-link/magic-link.service.ts | 8 ++++- tests/authentication.e2e.spec.ts | 12 +++++--- tests/magic-links.spec.ts | 46 ++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 5 deletions(-) diff --git a/lib/magic-link/magic-link.service.ts b/lib/magic-link/magic-link.service.ts index e70f055..1ec3c0f 100644 --- a/lib/magic-link/magic-link.service.ts +++ b/lib/magic-link/magic-link.service.ts @@ -163,13 +163,19 @@ export class MagicLinkService { * instead, before anything is looked up or burned: the module answers it * with a 401 that tells the customer where to open the link. Each * refusal is a `magic-link-refused` event with its reason. Outside an - * HTTP handler, pass the `request` that carries the cookie. + * HTTP handler, pass the `request` that carries the cookie. A request from + * another origin than the app's own and `session.trustedOrigins` gets a + * `ForbiddenException` first, as `SignInService.signIn()` does, so the + * link and the cookie stay as they were. */ async consume( token: string, { request }: { request?: MagicLinkRequest } = {}, ): Promise<(IssuedSession & { redirectTo?: string }) | null> { const { handler } = this.feature(); + // Signing in would refuse this request at the end, after the link is spent and the cookie + // settled: refuse it before, whatever the token is. + this.signIn.refuseCrossOrigin(this.scope.exchange()?.request); if (typeof token !== 'string' || !TOKEN_PATTERN.test(token)) { return this.refuse('unknown'); } diff --git a/tests/authentication.e2e.spec.ts b/tests/authentication.e2e.spec.ts index 8304658..af280cc 100644 --- a/tests/authentication.e2e.spec.ts +++ b/tests/authentication.e2e.spec.ts @@ -186,12 +186,12 @@ describe.each(adapters.map((a) => a.name))('authentication e2e (%s)', (adapter) } // A magic link of the attacker's, posted from the attacker's page, signs no one in either: - // not without the transaction cookie of the browser that requested it (401), and not with - // it (403, from SignInService). + // the origin is refused first (403), with the transaction cookie or without it, and the link + // and the cookie stay as they were. const requested = await http().post('/auth/magic').send({ email: 'bob@example.com' }).expect(202); const tokenParam = new URL(outbox.at(-1)!.url).searchParams.get('token')!; - await http().post('/auth/magic/consume').set('Origin', 'https://evil.test').send({ token: tokenParam }).expect(401); + await http().post('/auth/magic/consume').set('Origin', 'https://evil.test').send({ token: tokenParam }).expect(403); const res = await http() .post('/auth/magic/consume') @@ -199,7 +199,11 @@ describe.each(adapters.map((a) => a.name))('authentication e2e (%s)', (adapter) .set('Cookie', cookieOf(requested, '__Host-magic_link_tx')) .send({ token: tokenParam }); expect(res.status).toBe(403); - expect(res.headers['set-cookie']).toEqual(['__Host-magic_link_tx=; Max-Age=0; Path=/; HttpOnly; Secure; SameSite=Lax']); + expect(res.headers['set-cookie']).toBeUndefined(); + + // The same link, from the browser that requested it, still signs in. + await http().post('/auth/magic/consume').set('Cookie', cookieOf(requested, '__Host-magic_link_tx')).send({ token: tokenParam }).expect(200); + // Same-origin pages and non-browser clients sign in as before. const host = `127.0.0.1:${app.getHttpServer().address().port}`; await http().post('/auth/login').set('Origin', `http://${host}`).send(bob).expect(200); diff --git a/tests/magic-links.spec.ts b/tests/magic-links.spec.ts index cfe178e..ff322fb 100644 --- a/tests/magic-links.spec.ts +++ b/tests/magic-links.spec.ts @@ -205,6 +205,52 @@ describe('MagicLinkService in an HTTP exchange', () => { expect(own.cookies).toHaveLength(1); }); + describe('consume from a request of another origin', () => { + const crossSite = { host: 'app.test', origin: 'https://evil.test', 'sec-fetch-site': 'cross-site' }; + const sameOrigin = { host: 'app.test', origin: 'https://app.test', 'sec-fetch-site': 'same-origin' }; + + it('refuses before it spends the link or changes the transaction cookie, so the same link signs in afterwards', async () => { + const { service, inRequest, mailer, tokenOf, cookieHeader, links, seen } = setup(); + const created = await inRequest({}, () => service.create('ada@example.com')); + const token = tokenOf(mailer.sent[0]); + const consume = vi.spyOn(links, 'consumeMagicLink'); + const cookie = cookieHeader(created.cookies[0]); + + const refused = await inRequest({ ...cookie, ...crossSite }, () => service.consume(token)); + + expect(refused.error).toEqual(new ForbiddenException('Cross-origin sign-in refused')); + expect(refused.cookies).toEqual([]); + expect(consume).not.toHaveBeenCalled(); + expect(seen.filter((event) => event.type === 'magic-link-refused')).toEqual([]); + + const opened = await inRequest({ ...cookie, ...sameOrigin }, () => service.consume(token)); + expect(opened.value).toMatchObject({ session: { userId: 'u1' } }); + }); + + it('answers the same refusal for a token that is not valid, or from a browser that did not ask, so it tells nothing', async () => { + const { service, inRequest, mailer, tokenOf } = setup(); + await inRequest({}, () => service.create('ada@example.com')); + + const unknown = await inRequest(crossSite, () => service.consume('X'.repeat(43))); + const malformed = await inRequest(crossSite, () => service.consume('not a token')); + + expect(unknown.error).toBeInstanceOf(ForbiddenException); + expect(malformed.error).toBeInstanceOf(ForbiddenException); + // A real link, opened without the cookie of the browser that asked: the origin answers first. + const foreign = await inRequest(crossSite, () => service.consume(tokenOf(mailer.sent[0]))); + expect(foreign.error).toBeInstanceOf(ForbiddenException); + }); + + it('signs in a request of the app’s own origin', async () => { + const { service, inRequest, mailer, tokenOf, cookieHeader } = setup(); + const created = await inRequest({}, () => service.create('ada@example.com')); + + const opened = await inRequest({ ...cookieHeader(created.cookies[0]), ...sameOrigin }, () => service.consume(tokenOf(mailer.sent[0]))); + + expect(opened.value).toMatchObject({ session: { userId: 'u1' } }); + }); + }); + it('finds nothing for a link read from a log or forwarded, whatever cookie is forged for it', async () => { const { service, inRequest, mailer, tokenOf, cookieHeader, seen } = setup(); const created = await inRequest({}, () => service.create('ada@example.com'));