auth0 / nextjs-auth0

Next.js SDK for signing in with Auth0
MIT License
2.01k stars 381 forks source link

Custom store implementation #279

Closed johansundwall closed 1 year ago

johansundwall commented 3 years ago

Describe the problem you'd like to have solved

A cookie implementation isn't sufficient when the amount of claims starts to rise and the browser will emit a 431 Request Header Fields Too Large. For those who'll need a different store strategy allowing for a custom store implementation will unblock without putting the work and maintenance on Auth0, as each user can bring their own store, a bit like express-session.

Describe the ideal solution

Allow to pass an instance of a Store in the params to overwrite the CookieStore.

Alternatives and current work-arounds

No current workaround. Currently patching the library to allow this.

Additional information, if any

adamjmcgrath commented 3 years ago

Hi @johansundwall - thanks for your suggestion

I can't give you a timeline currently, but this is on the roadmap, not for the 1.0.0 release, but later down the line.

johansundwall commented 3 years ago

@adamjmcgrath what's the thoughts on the implementation? Im open to creating a PR.

adamjmcgrath commented 3 years ago

Thanks @johansundwall - we're just going through the process with our express mw as it happens auth0/express-openid-connect/pull/190

This SDKs session middleware is based on express-openid-connect's so once that's implemented, this SDKs will be based on that.

Never want to turn down the generous offer of a PR so very happy to look at one, but it should be based on auth0/express-openid-connect/pull/190 when it's finalised so we're not reinventing the wheel

johansundwall commented 3 years ago

@adamjmcgrath alright thanks. I'll take a look at the express-openid-connect's implementation as well before opening a PR. I've opened a draft https://github.com/auth0/nextjs-auth0/pull/281 with what we're currently using to patch the package up with, I have yet to check the compatibility with https://github.com/auth0/express-openid-connect/pull/190.

adamjmcgrath commented 3 years ago

Thanks for sharing #281 - looks perfectly sensible

I think the main points of design where we'll differ is express-session store compatibility (obviously less of an issue here, but important for us) and being able to handle async store save

PSoltes commented 2 years ago

Is this being worked on in any way? If so couldn't find documentation about this

jhaji2911 commented 2 years ago

Are you guys working on this issue?, I could not find any resources to fix this as my API requests are failing due to 431 error.

adamjmcgrath commented 2 years ago

Hi @jhaji2911 - thanks for your interest

Currently we're not working on this feature, but it's something we're considering for a future iteration

ShawnFumo commented 2 years ago

Just as a note to others, we have a system in place where a user logging in through SSO, may have permissions to certain resources, which is determined on their SSO. So we have a rule which takes this list of resources (unfortunately actions can't access this information) and puts it into user metadata (there's also a size limit of setting user metadata in actions that isn't in rules). Then an action adds this data as claims to the JWT. This works pretty well since it eliminates the API needing to call Auth0 on every request to access the user's metadata (which adds 200-300ms latency from the extra request), or else implement a caching scheme of some sort. The JWT acts as the cache.

But it runs into that headers size issue, if someone is associated with too many resources. If you're like us and most users don't run up against the limit, you can in the action look at the total size of the claims. If it is over your threshold (depends on how many other headers you typically have), put only a special claim in the JWT which basically just says the claims are too big to fit.. Then if the API sees that claim, it can call the auth0 api to get the data directly out of the user's metadata. This again adds latency to each request, but is only for these few specific users that have an unusual amount of claims.

PSoltes commented 1 year ago

@adamjmcgrath is there possibility of this package accepting my PR of this feature? cause it is real pain for us as we need to use redis for this and it blocks us from updating and confuses new developers quite a bit.

adamjmcgrath commented 1 year ago

Hi @PSoltes - thanks for your interest in this.

We've done a lot of work in v2 to enable this feature and I'm planning on working on it next quarter. once v2 is out

I have a branch in flight (https://github.com/auth0/nextjs-auth0/compare/vNext...session-stores) and I hope to get some time to complete it before the end of the year.

PSoltes commented 1 year ago

Hi @PSoltes - thanks for your interest in this.

We've done a lot of work in v2 to enable this feature and I'm planning on working on it next quarter. once v2 is out

I have a branch in flight (vNext...session-stores) and I hope to get some time to complete it before the end of the year.

that is superb news, I/We will be eagerly waiting for any update. Thanks again

adamjmcgrath commented 1 year ago

@johansundwall @PSoltes @ShawnFumo @jhaji2911 @matiascarranza @alechartung

I've released an experimental version of the SDK with support for custom session stores.

To install:

npm i @auth0/nextjs-auth0@experimental-custom-store

See this example for how to implement https://github.com/auth0/nextjs-auth0/blob/session-stores/EXAMPLES.md#use-a-custom-session-store

I've also set up an example app using the SDK with a redis session store (using upstash) here https://github.com/adamjmcgrath/nextjs-auth0-redis-session (The interesting file is /lib/auth0-config.ts)

Please try it out and add any feedback or issues in this thread 👇

jhaji2911 commented 1 year ago

@adamjmcgrath , thanks alot 🙏, I will surely try this and will come back with a feedback on it.

PSoltes commented 1 year ago

beautiful, will try it tomorrow

adamjmcgrath commented 1 year ago

@PSoltes @jhaji2911

FYI am planning on releasing this week - so let me know if you have run into any issues when trying this out

PSoltes commented 1 year ago

@adamjmcgrath just updating from 2.0.0 to 2.1.0, looks real nice, it seems I will be able to completely remove patching of this lib which is stellar. Just wanna ask is there a way to access user data in genId function? cause as a key of redis we want to use user nickname (of course encrypted). If not simple change to call of genId!(req) will fix it, eg. genId!(req, session). Sorry for not adding this feedback before, I was just really busy with one long awaited release. So add/not add when you want and if you want.

adamjmcgrath commented 1 year ago

Hi @PSoltes - sure, happy to accept a PR or separate feature request

PSoltes commented 1 year ago

Hi @PSoltes - sure, happy to accept a PR or separate feature request

I'll create a PR, it is really simple change. To what branch should PR go? main?

adamjmcgrath commented 1 year ago

Thanks - main please 👍