Closed jasonkuhrt closed 3 years ago
@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 req.headers["host"]
as it's insecure unless it's being checked against a whitelist.
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 domain
option (which could be a string, or an array) and to check it is a whitelisted option is secure and could be relevant to @grikomsn RE: #1168.
The exact point I was going to make @iaincollins! Your comment popped up while I was writing almost exactly the same thing. 😅
I believe https://github.com/nextauthjs/next-auth/pull/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 domain: "example.com"
would also be potentially really interesting for the future as we could leverage for cookie policy and/or CORS too, to make it easier to sign in and create a session that works on subdomains at the same domain, which I think a lot of folk would find helpful.
Beyond that, we also might want something like an explicit list of domains - domains: ["example.com", "www.example.com", "www.example.org"]
- to support cross domain sign in. Given that's a larger bit of work it's seems reasonable to not worry about multiple top level domains the short term, but just mention it in case it helps folks coming up with ideas.
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 https://github.com/nextauthjs/next-auth/issues/600 for adding multi-tenancy
to the library (also related with https://github.com/nextauthjs/next-auth/issues/1156). Still need to add the actual whitelisting of domains
as @iaincollins suggested.
Let me know what you think about it and if I am on a right path. Cheers
https://github.com/nextauthjs/next-auth/compare/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 MULTI_TENANT="true"
in environment variables (.env
) such that it's picked up by the library and goes into multi-tenant
mode. NOTE that the domains whitelisting
still need to be included. Also, the following behaviors are not as intended and need more work:
(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:
How to share the session across multiple domains after a user has SignedIn
into one domain?
(linked to this stack overflow question which @iaincollins responded to)
How to SignOut
and delete the session for all domains?
Where in the next-auth
codebase could 1. and 2. be solved? 🤔
Cheers
Thanks @Robert-OP.
I have a similar issue. We currently use NextJS for multiple domains + caching + SSR. Credentials for each provider are fetched based on the request headers and then added as provider ids and secrets.
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 otherDomain.com otherDomain.com otherDomain.com
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 [app-id]-[random-id]-[team-id].vercel.app
.
So wouldn't it be possible to create a domain whitelist like this https://myapp-*-myteam.vercel.app
?
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!
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.
Having a domain is a reasonable baseline assumption here.
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
Using host in this way is potential vector for DDOS attacks. There is some previous discussion of this in older issues.
The Vercel urls are using the format [app-id]-[random-id]-[team-id].vercel.app. So wouldn't it be possible to create a domain whitelist like this https://myapp-*-myteam.vercel.app?
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.
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).
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: https://github.com/nextauthjs/next-auth/issues/969#issuecomment-770887523 I also checked the changed proposed here, though they are too old: https://github.com/nextauthjs/next-auth/compare/main...Robert-OP:feature/multi-tenant-ropo
So basically it seems like the main issue now that both signIn
and signOut
should have a baseUrl to perform requests to, but both of the methods have no access to request context meaning we can't really get the header's value. Any ideas how can we workaround it?
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.
I've set the NEXTAUTH_URL
env variable from the inside NextJS API route before NextAuth init.
const { host } = req.headers;
const protocol = !host.includes('local') ? 'https' : 'http';
process.env.NEXTAUTH_URL = `${protocol}://${host}/api/auth/`;
Full example:
import NextAuth from 'next-auth';
import KeycloakProvider from 'next-auth/providers/keycloak';
export default async function auth(req, res) {
const { host } = req.headers;
const protocol = !host.includes('local') ? 'https' : 'http';
process.env.NEXTAUTH_URL = `${protocol}://${host}/api/auth/`;
async function createProviders() {
const keycloakConfig = {
clientId: process.env.KEYCLOAK_CLIENT_ID,
clientSecret: process.env.KEYCLOAK_CLIENT_SECRET,
issuer: KEYCLOAK_ISSUER,
realm: REALM,
};
return [KeycloakProvider(keycloakConfig)];
}
const providers = await createProviders();
const authOptions = {
providers,
callbacks: {
async session({ session, token }) {
...
},
async jwt({ token, user, account }) {
...
},
},
};
return NextAuth(req, res, authOptions);
}
Thanks, Denys
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 https://github.com/vercel/vercel/discussions/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.