Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Credentials provider: request headers undefined in authorize callback on new Nodes #2612

Closed
svkoskin opened this issue Aug 27, 2021 · 5 comments
Labels
enhancement New feature or request good first issue Good issue to take for first time contributors stale Did not receive any activity for 60 days upstream The issue dervies from one of next-auth dependencies

Comments

@svkoskin
Copy link

Description 馃悳

When using Credentials provider, the authorize callback should receive the incoming message object as the second parameter. However, on certain Node versions the incoming message has undefined as headers.

This happens on Node versions 14.15.2, >=15.1 and >=16.0 and is related to this change: nodejs/node#35281 There is an issue on Node but it is closed (nodejs/node#36550)

A developer seems to be advising against using the spread operator on headers (nodejs/node#36550 (comment)) which is exactly what NextAuth is doing before calling authorize callback (

credentials, {...req, options: {}, cookies: {}}
).

Is this a bug in your own project?

No

How to reproduce 鈽曪笍

import Providers from `next-auth/providers`
...
providers: [
  Providers.Credentials({
    name: 'Credentials',
    credentials: {
      username: { label: "Username", type: "text", placeholder: "jsmith" },
      password: { label: "Password", type: "password" }
    },
    async authorize(credentials, req) {
      // The following statement should output a structure with the HTTP request headers.
      // On Node 14.15.2, >=15.1 and >=16.0 it outputs undefined and none of the headers can be accessed.
      console.log(req.headers);
      }
    }
  })
]

Screenshots / Logs 馃摻

No response

Environment 馃枼

  System:
    OS: Linux 4.19 Ubuntu 20.04.2 LTS (Focal Fossa)
    CPU: (4) x64 Intel(R) Core(TM) i5-6500 CPU @ 3.20GHz
    Memory: 16.37 GB / 25.02 GB
    Container: Yes
    Shell: 5.0.17 - /bin/bash
  Binaries:
    Node: 16.2.0 - ~/.nvm/versions/node/v16.2.0/bin/node
    npm: 7.13.0 - ~/.nvm/versions/node/v16.2.0/bin/npm
  npmPackages:
    next: 11.1.0 => 11.1.0
    next-auth: 3.28.0 => 3.28.0
    react: 17.0.2 => 17.0.2

Contributing 馃檶馃徑

No, I am afraid I cannot help regarding this

@svkoskin svkoskin added the bug Something isn't working label Aug 27, 2021
@balazsorban44 balazsorban44 added upstream The issue dervies from one of next-auth dependencies enhancement New feature or request and removed bug Something isn't working labels Aug 27, 2021
@balazsorban44
Copy link
Member

I wouldn't say it's a bug, more of an inconvenience maybe. Feel free to open a PR that strips the internal options and cookies from the request. We want to gate people from messing with those.

You have access to the original request though, just do:

export default function auth(req, res) {
  NextAuth(req, res, {
     ... 
     authorize(credentials) {
       req
    } 
   ... 
  })
}

@balazsorban44 balazsorban44 added the good first issue Good issue to take for first time contributors label Aug 27, 2021
@ilijaNL
Copy link

ilijaNL commented Sep 29, 2021

I wouldn't say it's a bug, more of an inconvenience maybe. Feel free to open a PR that strips the internal options and cookies from the request. We want to gate people from messing with those.

You have access to the original request though, just do:

export default function auth(req, res) {
  NextAuth(req, res, {
     ... 
     authorize(credentials) {
       req
    } 
   ... 
  })
}

Or add additional parameter: rawrequest

@balazsorban44
Copy link
Member

balazsorban44 commented Sep 29, 2021

this will actually change, once #2857 is merged!

we won't pass the whole req anymore, as we won't have it.

my comment from #2612 (comment) will be still valid, and we will continue to pass body, query, headers and method, usually the only ones that would be needed

@stale
Copy link

stale bot commented Nov 29, 2021

Hi there! It looks like this issue hasn't had any activity for a while. It will be closed if no further activity occurs. If you think your issue is still relevant, feel free to comment on it to keep it open. (Read more at #912) Thanks!

@stale stale bot added the stale Did not receive any activity for 60 days label Nov 29, 2021
@stale
Copy link

stale bot commented Dec 6, 2021

Hi there! It looks like this issue hasn't had any activity for a while. To keep things tidy, I am going to close this issue for now. If you think your issue is still relevant, just leave a comment and I will reopen it. (Read more at #912) Thanks!

@stale stale bot closed this as completed Dec 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good issue to take for first time contributors stale Did not receive any activity for 60 days upstream The issue dervies from one of next-auth dependencies
Projects
None yet
Development

No branches or pull requests

3 participants