nextauthjs / next-auth

Authentication for the Web.
https://authjs.dev
ISC License
22.49k stars 2.98k forks source link

Provider configs `token` and `userinfo` ignore `url` and `request` properties #10732

Closed joonhyungshin closed 2 weeks ago

joonhyungshin commented 3 weeks ago

Environment

  System:
    OS: macOS 14.4.1
    CPU: (8) arm64 Apple M1 Pro
    Memory: 2.24 GB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.12.2 - /usr/local/bin/node
    npm: 10.5.0 - /usr/local/bin/npm
  Browsers:
    Chrome: 124.0.6367.92
    Safari: 17.4.1
  npmPackages:
    next: 14.2.3 => 14.2.3
    next-auth: ^5.0.0-beta.17 => 5.0.0-beta.17
    react: ^18 => 18.3.0

Reproduction URL

https://github.com/joonhyungshin/next-auth-mre

Describe the issue

I am working on a personal project with social account. I did not want Auth.js to call userinfo/ endpoint, because in my case token verification and user info fetch are done in a separate backend server. So I only wanted Auth.js to receive an access token using the standard OAuth2 flow, so in my root auth.ts file I replaced userinfo.request with a no-op function.

export const { handlers, signIn, signOut, auth } = NextAuth({
  providers: [
    Twitter({
      userinfo: { request: () => {} }
    })
  ],
  // callbacks and events omitted
})

However, I realized that Auth.js still calls the userinfo/ endpoint. It seems like the url and request properties are all ignored, since the following config worked with no error.

export const { handlers, signIn, signOut, auth } = NextAuth({
  providers: [
    Twitter({
      userinfo: { url: "asdf", request: () => {} }
    })
  ],
  // callbacks and events omitted
})

On the other hand, the following code errors as expected.

export const { handlers, signIn, signOut, auth } = NextAuth({
  providers: [
    Twitter({
      userinfo: "asdf",
    })
  ],
  // callbacks and events omitted
})

So I suspect that Auth.js just falls back to the default config if userinfo is not a string. The code doesn't seem to do so, so I don't understand why.

I also noticed that the token property has the same issue.

How to reproduce

  1. Start a new Next.js app with npx create-next-app@latest.
  2. Install Auth.js with npm i next-auth@beta.
  3. Create auth.ts at the root, token.request and userinfo.request replaced with no-op functions.
    
    import NextAuth from "next-auth"
    import Twitter from "next-auth/providers/twitter"

export const { handlers, signIn, signOut, auth } = NextAuth({ providers: [ Twitter({ token: { request: () => {} }, userinfo: { request: () => {} } }) ], })

4. Create `app/api/auth/[...nextauth]/route.ts`.
```ts
import { handlers } from "@/auth"
export const { GET, POST } = handlers
  1. Add environment variables to .env.local.
    AUTH_SECRET=***
    AUTH_TWITTER_ID=***
    AUTH_TWITTER_SECRET=***
  2. Replace app/page.tsx with the following code.
    
    import { signIn } from "@/auth"

export default function Home() { return (

{ "use server" await signIn("twitter") }} >

); }


7. Run `npm run dev`. The Signin with Twitter button still works.

### Expected behavior

Error, because Auth.js must not be able to fetch access token or user info.
balazsorban44 commented 2 weeks ago

userinfo.request should work: https://github.com/nextauthjs/next-auth/blob/main/packages/core/src/lib/actions/callback/oauth/callback.ts/#L160

token.request is replaced by conform, which is an internal API (so the type is hidden for it). We don't really want people to patch the standard OAuth token request.

joonhyungshin commented 2 weeks ago

userinfo.request should work: https://github.com/nextauthjs/next-auth/blob/main/packages/core/src/lib/actions/callback/oauth/callback.ts/#L160

That's exactly why I filed this issue. I read that code, but userinfo.request does not work as expected.

bigbigbo commented 1 week ago

userinfo.request should work: https://github.com/nextauthjs/next-auth/blob/main/packages/core/src/lib/actions/callback/oauth/callback.ts/#L160

token.request is replaced by conform, which is an internal API (so the type is hidden for it). We don't really want people to patch the standard OAuth token request.

In version 4, the custom provider worked well, but I don't understand why the token.request was removed, and the corresponding TypeScript type definitions were not removed either, nor was there any documentation explaining this. Our OAuth service provider will not change their service in the short term, what should I do to make token.request work?

I've spent an entire night troubleshooting why the custom provider failed. I looked through the source code and found the issue, but I still don't know how to resolve it. Additionally, there are some minor issues with version 4, such as the fact that returning null in the JWT callback throws an error, preventing the session from being cleared.

Awaiting your response, thank you.

Nik-Novak commented 1 week ago

userinfo.request should work: https://github.com/nextauthjs/next-auth/blob/main/packages/core/src/lib/actions/callback/oauth/callback.ts/#L160 token.request is replaced by conform, which is an internal API (so the type is hidden for it). We don't really want people to patch the standard OAuth token request.

In version 4, the custom provider worked well, but I don't understand why the token.request was removed, and the corresponding TypeScript type definitions were not removed either, nor was there any documentation explaining this. Our OAuth service provider will not change their service in the short term, what should I do to make token.request work?

I've spent an entire night troubleshooting why the custom provider failed. I looked through the source code and found the issue, but I still don't know how to resolve it. Additionally, there are some minor issues with version 4, such as the fact that returning null in the JWT callback throws an error, preventing the session from being cleared.

Awaiting your response, thank you.

Yeah please help, this has been a nightmare. Thank you so much for tracking this down @bigbigbo and for clarifying @balazsorban44

Nik-Novak commented 1 week ago

userinfo.request should work: https://github.com/nextauthjs/next-auth/blob/main/packages/core/src/lib/actions/callback/oauth/callback.ts/#L160

token.request is replaced by conform, which is an internal API (so the type is hidden for it). We don't really want people to patch the standard OAuth token request.

If I'm understanding this correctly you're enforcing a very strict pattern on auth that does not consider edge cases.

k0l11 commented 6 days ago

@balazsorban44 You are aware that being unable to overwrite token.request directly breaks the existing Azure DevOps provider in v5 (in fact, it is broken because of this right now), right? Not allowing this to be overwritten prevents people dealing with weird OAuth providers (like Microsoft...).

joonhyungshin commented 6 days ago

Personally, I have decided to step away from Auth.js and placed all the auth logic to my separate backend server. I am very happy with it.

I hope that maintainers take a closer look at the issues before closing them. I understand that their time is limited there are many more important issues to handle, but people are also taking their time to debug the library, read the code, create minimal reproducible examples, and describe the issues.

userinfo.request should work: https://github.com/nextauthjs/next-auth/blob/main/packages/core/src/lib/actions/callback/oauth/callback.ts/#L160

To me, simply saying something like 'this should work' and closing the issue counts as ignoring it, because this means the maintainers didn't bother to look at the reproducible example, despite my effort trying to make it as minimal as possible to save their time. I believe they should either tell me what I did wrong or keep the issue open (if this is indeed a bug). For the future issues, I wish the maintainers take them more seriously and provide better response to the authors.