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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): use NEXTAUTH_URL when getting the url during initialization #10571

Open
wants to merge 1 commit into
base: v4
Choose a base branch
from

Conversation

ammmze
Copy link
Contributor

@ammmze ammmze commented Apr 13, 2024

☕️ Reasoning

When using an oauth based authentication process and the app is configured with a basePath, we need to set the NEXTAUTH_URL environment variable to tell next-auth / auth.js the base authentication path. However, it does not seem to respect it in all scenarios. For example, I am running next-auth v4.24.7 with a basePath configured as /sso and a NEXTAUTH_URL=http://localhost:3000/sso/api/auth. When I perform a signIn('keycloak'), it does send me to my auth provider (keycloak) BUT the redirect_uri gets set to http://localhost:3000/api/auth/callback (note the missing /sso base path).

🧢 Checklist

  • Documentation
  • Tests
  • Ready to be merged

🎫 Affected issues

I think this fixes the following issues (at least for users using v4...sounds like the v5 beta stuff has this issue too):

📌 Resources

Signed-off-by: Branden Cash <branden.cash@parchment.com>
@ammmze ammmze requested a review from ThangHuuVu as a code owner April 13, 2024 00:10
Copy link

vercel bot commented Apr 13, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
auth-docs ❌ Failed (Inspect) Apr 13, 2024 0:12am
next-auth-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 13, 2024 0:12am

@ndom91
Copy link
Member

ndom91 commented Apr 13, 2024

Hey thanks for your interest! However, we'd like to avoid making changes like this to v4 atm.

Does setting NEXTAUTH_URL to include only the basePath (i.e. http://localhost:3000/sso) work for yuo?

@ammmze
Copy link
Contributor Author

ammmze commented Apr 14, 2024

Does setting NEXTAUTH_URL to include only the basePath (i.e. http://localhost:3000/sso) work for yuo?

No...that just makes things worse. Doing that makes it so that all the other calls that it makes (i.e. /session, /providers, etc) no longer have the /api/auth base path. For example, it tries to load /sso/session, but should be /sso/api/auth/session.

It is also explained the documentation that if you are using a custom base path that you set this to the full path to the next auth endpoints.

@ndom91
Copy link
Member

ndom91 commented Apr 14, 2024

@ammmze okay gotcha, yeah we changed the default behaviour of that in v5 but I couldn't remember how v4 behaved in detail. Okay let me have a think abuot this, because changing this now will almost certainly break other peoples existing setups.. 🤔

@ammmze
Copy link
Contributor Author

ammmze commented Apr 15, 2024

@ndom91 I can make the change a little more involved if we want by changing the behavior of parse-url so that it still uses the protocol and hostname of the given url (the origin) but then also parses the NEXTAUTH_URL. If the given url has a pathname other than /, then we use that as the default path instead of /api/auth. If the pathname of the given url has a pathname of /, then it would preserve the existing behavior.

The current proposed fix would always result in the NEXTAUTH_URL being used as the protocol and hostname. But the change I described above would preserve the origin information, while only taking the path from the NEXTAUTH_URL if it is something with a base path.

@ammmze
Copy link
Contributor Author

ammmze commented Apr 15, 2024

For clarity in the proposed alternative solution, i've implemented it on a separate branch found here. Effectively it just makes it so that the parseUrl function uses the NEXTAUTH_URL to get the default path instead of hard coding it to /api/auth

@ammmze
Copy link
Contributor Author

ammmze commented Apr 19, 2024

Oye...as I continue to dig at this I have made another discovery...

Previously I had assumed the request "origin" would never have a path*. However, apparently we override the origin of the request when making it an "InternalRequest". And then while digging into that detectOrigin method I discovered that having ANY value set for AUTH_TRUST_HOST will actually enable AUTH_TRUST_HOST. So setting it to 'false' (string because its an env var) will enable that. And so then it falls into that condition which just uses the forwarded host from the request instead of the NEXTAUTH_URL.

So I see 2 issues here:

  1. AUTH_TRUST_HOST doesn't allow you to set it to typical falsy value to disable it. Side note: this env var doesn't seem to be documented anymore.
  2. detectOrigin effectively does not respect the base path configured via setting NEXTAUTH_URL (and neither does parseUrl, which may be fixed with something like this.

But I guess the good news is I think I am now un-blocked from this issue by removing the AUTH_TRUST_HOST environment variable.

Note

*origin typically just contains protocol, host, and sometimes the port. Example:

> new URL('https://example.com/foo').origin
'https://example.com'

@smultar
Copy link
Contributor

smultar commented Apr 22, 2024

I have a similar issue, that leads me to be constantly redirected back to my sign-in page or my local host.

My production environment is a docker swarm, with caddy infront of it. It works locally, but not in my prod.

@TomWonder
Copy link

I have the very same issue. Cannot use either signIn() or signOut(). I need signOut() to implement the logout

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legacy Refers to `next-auth` v4. Minimal maintenance.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants