-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Dynamic NEXTAUTH_URL #969
Comments
@balazsorban44 I think this should be easy to implement. Can I get your blessing to take this on? |
Hi there! Just jumping into say we shouldn't use This was actually how v1 worked, as it simplified configuration greatly. There problem is little obtuse (and apologies for not having time to dive into it) but it can be used, in some fairly particular circumstances, to facilitate a Denial of Service attack. It's a bit long winded to explain, but there should be some background on this in old closed issues if you feel like digging around. HOWEVER! Using the Host header in combination with something like |
The exact point I was going to make @iaincollins! Your comment popped up while I was writing almost exactly the same thing. 😅 |
I believe #1123 is also trying to solve this in a different way? cc @gergelyke Corresponsing issue: Allow server-side calls to use a different URL other than NEXTAUTH_URL #1121 |
@balazsorban44 Thanks! Was just writing a comment on that issue :D I think there is some overlap, if folks could check out that PR and the issues linked from it and share thoughts on where they intersect that would be great. We may not want to solve them all in one PR, but they do seem to overlap. For consideration: Having an option like Beyond that, we also might want something like an explicit list of domains - |
Thanks for the feedback @iaincollins & @balazsorban44! |
Hey guys, thank you for the support on this issue and hope you are well! I forked the latest main and started building on top of it by adding parts from this discussion and issue #600 for adding Let me know what you think about it and if I am on a right path. Cheers main...Robert-OP:feature/multi-tenant-ropo UPDATE: posted a multi-tenant version (incomplete, work in progress) on npm that seems to work for my use case to some extent, still working on it and will improve. With this you need to set
(if you guys have any idea how to trigger an event that would SignIn or SignOut users from all domains) |
To continue on my last comment and coming back with some questions, any thoughts about:
Cheers |
Thanks @Robert-OP. I have a similar issue. We currently use NextJS for multiple domains + caching + SSR. Currently, on the main branch and updating my callback redirect, this will not work as for the four requests I will receive something similar to... mydomain.com //NEXTAUTH_URL. <---- Providers are set here and do not update on subsequent requests callbacks: {
async redirect(url, baseUrl) {
return `${hostname}`;
},
}, Is there a possible short term / incremental solution to this And also, should this be a PR on the repository? Edit: Apologies, noticed it is a PR / forked repo already! |
I think the domain whitelist is not a good solution because in vercel the dynamic urls are in the form "XYZ.vercel.app" so the only common part is vercel.app which is in common with any other application of all users on vercel. A different proposal can be: setup an env variable that allow to use req['host'], so everything will work in preview on vercel but not set it in production where the normal VERCEL_URL or the domain whitelist would work |
The Vercel urls are using the format |
Yes, indeed, unless that domain whitelist should be used for the cookie as well, in that case it can be something like ".vercel.app". But probably it's not the case |
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! |
@jasonkuhrt I try implementing your solution .. it works to a point. I keep getting ERR_JWS_VERIFICATION_FAILED after successful sign-in at a provider. How did you resolve the error? Thx |
Hey @thachp We don't use next-auth in the app this came from anymore so haven't stayed on top of this. Good luck! |
Having a domain is a reasonable baseline assumption here.
Using host in this way is potential vector for DDOS attacks. There is some previous discussion of this in older issues.
Technically yes but I don't think it makes sense for us to get into domain specific regexes. I think we should avoid esoteric solutions (which can introduce new factors) when simple solutions like having a domain are a cheap and secure option. |
Custom domain multi-tenancy is a current use case for us, curious if any progress has been made to support cross-domain sessions? |
@jasonkuhrt would you be able to comment on which auth library you used for supporting your use cases? and how well has that worked? |
@viperfx We built our own solution on top of PassportJS. |
jasonkuhrt any way can you share the passport solution? I was not able to make it work end to end. |
Company has moved on yet again now to Auth0 (we also looked at Clerk but top down decision that it wasn't enterprise ready enough). |
Oh okay. Let me give a try for auth0 too. According to you which is the best Library that support multi tenancy ? |
Not sure which is better for that. Clerk was simpler we felt. |
@iaincollins I came to the same issue now and wanted to check if it's indeed possible to use header's host value and list of trusted domains proposed here: #969 (comment) So basically it seems like the main issue now that both |
My another idea would be.. what if we still keep NEXTAUTH_URL as a default url, but each provider would provide a list of trusted URLs can be used in case of making calls to app itself. It will kinda similar to the oauth's redirectionUrl where each client can define the url, server must redirect to after success/failure |
Hey everyone, what is the status of this feature ? |
Hey everybody, This works for me.
Full example:
Thanks, |
People from SvelteKit ecosystem, here is how we share session on preview URLs - https://blog.aakashgoplani.in/managing-shared-sessions-across-multiple-applications-in-sveltekitauth |
Summary of proposed feature
Allow callback URL (
NEXTAUTH_URL
) to be set dynamicallyPurpose of proposed feature
A clear and concise description description of why this feature is necessary and what problems it solves.
A static
NEXTAUTH_URL
is problematic for preview deployments like that offered by Vercel or any other platform/setup with similar concept.It is problematic because a preview deployment URL will be, by definition, dynamic.
There are actually two sides to the problem:
This issue is only about Problem 1 above.
To give more context, I tried mapping
VERCEL_URL
toNEXTAUTH_URL
but that failed because it used the unique preview domains, not the ones we registered in our GitHub app callback configuration. Similar report of this problem can be found here vercel/vercel#5230.In our case, our solution is to have extra domains like
<project name>-branch-<#>.<org>.vercal.app
. All of these have been registered in our GitHub app. When a team member is working on a branch of the app, they one of those domains in Vercel to point to their branch (yes, potentially requires some coordination with team, minor though).Hope that makes sense :)
Detail about proposed feature
Our solution currently looks like as follows.
Before invoking
NextAuth
we dynamically set theNEXTAUTH_URL
based on the request host. The protocol is https in production, otherwise http (e.g. on developer's machine).What I would like to propose is that NextAuth allow
callbackURL
to be passed as an optional configuration so that:Like so:
Another proposal I would like to suggest is a new default when no environment variable has been set AND no config has been passed. Then the default should be:
In many cases I think this default will be right, which is what a good default should be.
Then we can reduce our implementation back to just this:
Potential problems
The behaviour becomes a bit more variable so good docs would be needed (website and IMO JSDoc too).
Some kind of logging/debug logging might be worth considering.
Describe any alternatives you've considered
None
Please indicate if you are willing and able to help implement the proposed feature.
I am willing to submit a complete PR for all of this if agreed to.
The text was updated successfully, but these errors were encountered: