kinde-oss / kinde-auth-nextjs

Kinde NextJS SDK - authentication for server rendered apps
https://kinde.com/docs/developer-tools/nextjs-sdk/
MIT License
132 stars 17 forks source link

Bug: `/api/auth/setup` is failing with 401 Unauthorized #170

Open AntoLepejian opened 1 month ago

AntoLepejian commented 1 month ago

Prerequisites

Describe the issue

Following the NextJS guide, I constantly see the following error in my console

Screenshot 2024-05-27 at 1 33 55 AM

This setup fetch call in this effect is always failing with 401 error.

The response just says "Login with Kinde".

At the moment it appears to be a red herring since I'm able to login anyways, but thought i'd report, since either the guide is missing a step, there's a bug with the SDK or I'm doing something wrong.

Thanks!

Library URL

https://github.com/kinde-oss/kinde-auth-nextjs

Library version

^2.2.10

Operating system(s)

macOS

Operating system version(s)

Sonoma 14.5

Further environment details

No response

Reproducible test case URL

No response

Additional information

No response

AntoLepejian commented 1 month ago

I just realised that this is 401ing only when there's no active user session, and once I login the request to /api/auth/setup succeed.

Sounds like a bug in the useEffect, which shouldn't fire if there's no active user session.

For reference, this is the problematic code

  const { user } = useKindeBrowserClient();

IIUC that's how I'm meant to check if a user is logged in or not, and hence there should be no call to /setup if it requires an active user session.

DanielRivers commented 1 month ago

This is current intended behaviour. Due to security measures the access token cookie is not readable by javascript so the browser knows nothing about the user. This /setup endpoint returns the data about the user based on cookie access token to the browser can setup the browser client.

The 401 happens when the user is not signed in as no token is present it doesn't have access to the route.

This is something we want to improve in the future.

AntoLepejian commented 1 month ago

I see thank you!

@DanielRivers should we handle the request rejection and swallow it then to avoid spamming the console?

If it's intended behaviour, there's a low-hanging fix to wrap it in a try/catch, and swallow the 401 errors? If it's any other status code we can rethrow.

Either way I'll leave it with you and the team 🙂

DanielRivers commented 1 month ago

Sadly, 401 errors cannot be omitted from the console. They are always outputted there is no way to prevent this.

AntoLepejian commented 1 month ago

TIL. I guess you'd have to return 200 { isLoggedIn: boolean } and reserve 401 for real unauthorised access attempts. Which is a slightly bigger job.

No worries then thanks for looking into this!

davidf9265 commented 2 weeks ago

I totally agree with @AntoLepejian , i think this could be handled better, or at least being well documented. In my team, we spent a considerable time figuring out this wasnt an error on our side. Please improve this behavior, to allow us to really handle unauthorized states when they are really intended...

DanielRivers commented 1 week ago

Thanks for the feedback here, this is under discussion internally on improved flows here, care has to be taken as this could result in a breaking change.