[johnnyhuirilef]

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Other... Please describe:

What is the current behavior?

Issue Number: N/A

Hi! 👋 This is the follow-up I offered in #42. MagicLinkService.consume() answers a request from another origin with a 403, but it has already spent the link.

On master (85fd249), consume() uses the link with consumeMagicLink() and settles the browser cookie (lib/magic-link/magic-link.service.ts:196-199). Only then does it call SignInService.signIn() (:224), which refuses the request with ForbiddenException('Cross-origin sign-in refused') (lib/session/sign-in.service.ts:47 and :195). When the user opens the same link again from the app, it does not sign in: the link is spent, and the refused response also took it out of the browser's cookie.

With bindToBrowser on, someone else can't burn the link, because they don't have the browser's cookie. The case I have in mind is an app whose SPA origin is missing from session.trustedOrigins: the first click fails every time, and the link is gone before anyone fixes the configuration.

I ran the repro on 0.1.1:

=== the link is opened from the app (control) ===
  open from http://app.test:         200 signed in
  expected:                          200 signed in

=== the link is opened from an untrusted origin first ===
  open from http://admin.app.test:   403 Cross-origin sign-in refused
  then open from http://app.test:    401 Unauthorized
  expected:                          403 first, then 200 signed in

RESULT: BUG: the refused request spent the link.

As in #42, the refusal itself is correct. Only its timing is wrong.

What is the new behavior?

• consume() refuses a request from another origin first, with the same SignInService.refuseCrossOrigin() call as create() and PasswordResetService.reset(), on the request of the current call.
• The check runs before the token is read. A refused request leaves the link and the browser cookie as they were, and the same link works from an allowed origin.
• The 403 does not depend on the token, so it does not tell which tokens are valid.
• Outside an HTTP handler there is no request in scope, so nothing changes there: signIn() doesn't check the origin in that case either.

Does this PR introduce a breaking change?

• Yes
• No

A request from another origin now gets the 403 before the other checks. Before, a token that is not valid got null, and a link opened without the browser's cookie got the 401 not_this_browser with a magic-link-refused event. Same-origin requests give the same answer as before.

Other information

The new specs are in tests/magic-links.spec.ts. They cover these cases:

• A request from another origin is refused. consumeMagicLink() is not called, the cookie is not changed, no magic-link-refused event is emitted, and the same link signs in afterwards from the app's own origin.
• The same 403 for an unknown token, a malformed token, and a link opened without the browser's cookie.
• A request from the app's own origin signs in.

The old tests only checked the status of the refusal and that the cookie was cleared. They never opened the same link again, so they did not see that it was spent.

The existing login CSRF test in tests/authentication.e2e.spec.ts pinned the old order: a 401 without the cookie, and a 403 that cleared the cookie. It now expects a 403 in both cases with the cookie kept, and checks that the same link then signs in, on Express and Fastify.

The docs say that consume() "burns the token, calls resolveUser(), and signs the user in". I can add a line in docs.nestjs.com that a request from another origin is refused before the link is used. 🙂