state-handler check doesn't stop callback invocation

[syumai]

Description 🐜

• state-handler returns res.redirect() when state check fails.

  • next-auth/src/server/lib/oauth/state-handler.js

    Line 33 in 46b62d7

    return res.redirect(`${baseUrl}${basePath}/error?error=OAuthCallback`)

• res.redirect() returns undefined as result value (res.end() returns no value), therefore if (await state.handleCallback(req, res)) return doesn't work and routes.callback is invoked.
  • res.redirect:

    next-auth/src/server/lib/extend-res.js

    Line 33 in e8a58a0

    return res.end()

  • server/index.js:

    next-auth/src/server/index.js

    Line 192 in e8a58a0

    if (await state.handleCallback(req, res)) return

I've confirmed that in the beta branch, state validation logic above has been removed, but I think res.redirect() should return res (NextAuthResponse) as same as Next.js does. Current implementation may cause other problems in the future.

• Next.js's implementation: https://github.com/vercel/next.js/blob/6f3c338745d9d574e7a9a64ca5e38bada0fd0362/packages/next/server/api-utils.ts#L235

Is this a bug in your own project?

No

How to reproduce ☕️

1. start OAuth authorization request to IdP
2. remove or rewrite csrfToken while request in the other tab or window
3. complete authorization request

Screenshots / Logs 📽

[next-auth][debug][oauth_callback_protection] Comparing received and expected state {
  state: '4930bc690f39ef9ce3e78517f19e05fd77b092e063048f989095d5ed218f08f6',
  expectedState: 'ac1e2095ea4e731e44d206bfdc51557b961f9f78ca92139026bd5001a53678f0'
}
[next-auth][error][state_error]
https://next-auth.js.org/errors#state_error OAuthCallbackError: Invalid state returned from OAuth provider
    at Object.handleCallback (/.../node_modules/next-auth/dist/server/lib/oauth/state-handler.js:42:13)
    at /.../node_modules/next-auth/dist/server/index.js:189:42
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:94:5)
event - build page: /error
wait  - compiling...
event - compiled successfully
[next-auth][debug][profile_data] { ... }
[next-auth][debug][oauth_callback_response] { ... }
[next-auth][warn][jwt_auto_generated_signing_key]
https://next-auth.js.org/warnings#jwt_auto_generated_signing_key
[next-auth][error][oauth_callback_handler_error]
https://next-auth.js.org/errors#oauth_callback_handler_error Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client
    at new NodeError (node:internal/errors:329:5)
    at ServerResponse.setHeader (node:_http_outgoing:573:11)
    at Object.set (/.../node_modules/next-auth/dist/server/lib/cookie.js:24:7)
    at Object.callback (/.../node_modules/next-auth/dist/server/routes/callback.js:117:18)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:94:5) {
  code: 'ERR_HTTP_HEADERS_SENT'
}
[next-auth][error][oauth_callback_error]
https://next-auth.js.org/errors#oauth_callback_error Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client
    at new NodeError (node:internal/errors:329:5)
    at ServerResponse.setHeader (node:_http_outgoing:573:11)
    at ServerResponse.res.redirect (/.../node_modules/next-auth/dist/server/lib/extend-res.js:39:21)
    at Object.callback (/.../node_modules/next-auth/dist/server/routes/callback.js:147:20)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:94:5) {
  code: 'ERR_HTTP_HEADERS_SENT'
}
error - unhandledRejection: Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client
    at new NodeError (node:internal/errors:329:5)
    at ServerResponse.setHeader (node:_http_outgoing:573:11)
    at ServerResponse.res.redirect (/.../node_modules/next-auth/dist/server/lib/extend-res.js:39:21)
    at Object.callback (/.../node_modules/next-auth/dist/server/routes/callback.js:156:18)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:94:5) {
  code: 'ERR_HTTP_HEADERS_SENT'
}

Environment 🖥

• macOS 11.6
• Chrome 94.0.4606.81
• Node.js v15.10.0
• Next.js v11.1.2
• Next Auth v3.29.0

Contributing 🙌🏽

No, I am afraid I cannot help regarding this

[balazsorban44]

The core is being refactored in #2857, so I think this goes away totally.

In any case, why would you remove the cookies forcefully? it's not an intended thing to do, so I cannot consider this as a bug

[syumai]

The core is being refactored in #2857, so I think this goes away totally.

That's right. 2857 looks great.

why would you remove the cookies forcefully?

I just wanted to test the pattern of failing state checks. My concerns will be resolved by next-auth v4.

Thank you for your reply.