Prevent clear auth data when getSession error
Describe the feature
If getSession throws an exception, the backend is most likely at fault: If you clear auth data, all users using the website will have to log in again.
https://github.com/sidebase/nuxt-auth/blob/faa039b72da2c64d9c214f0502dea053c7a4ff84/src/runtime/composables/local/useAuth.ts#L99
How would you implement this?
Only clear the session when the http error code returned from the backend is 401
Additional information
- [X] Would you be willing to help implement this feature?
Provider
- [ ] AuthJS
- [X] Local
- [ ] Refresh
- [ ] New Provider
I'm having the same problem with refresh method.
For example, when the user disconnects their Wifi and periodic refresh is turned on, with the first failed request, the user is logged out because the token data is removed from the state.
The tokens should only be removed in three cases imo:
- the user explicitly wants them removed (by signing out)
- the tokens are expired... A refresh token that is still valid should not be nullified.
- a request to refresh (and get a valid access token) fails on SSR
Hi @mbellamyy and @nvthuong1996 👋
I added the rfc label to this issue. I would pull in @phoenix-ru, and we can revisit the current behavior and discuss potential modifications to it!
hello @phoenix-ru @zoey-kaiser ! has any update for this issue ?
Hi @nvthuong1996 👋
We are currently focusing on finishing up the documentation rewrites, which is why we are currently less actively pushing forward discussions on new issues. Once we have the docs deployed we will revisit this issue (should be pretty soon).
Some updates for this error ?
It would be nice if the getSession method throws an error instead of just clearing the session data and returning null: https://github.com/sidebase/nuxt-auth/blob/52d4b9a5b48431eb9d522865410d209c6d66a613/src/runtime/composables/local/useAuth.ts#L147
I made a workaround using the callGetSession option set to false of the signIn method for the local provider, but I am not sure if you can do the same with refresh calls. And I still need to do a manual getSession like call which is for us this:
public async loginWith(...): Promise<void> {
const { getSession, signIn } = useAuth();
await signIn(
{...},
{
// Don't redirect and don't call get session, because login must catch OTP/2FA errors from getSession.
callGetSession: false,
// The callback URL is only set to prevent the sidebase signIn method from doing extra work.
// Improvement issue opened: https://github.com/sidebase/nuxt-auth/issues/978
callbackUrl: '/',
redirect: false,
},
);
// NOTE: This extra fetch is needed because of how getSession works. It does not return error response data.
// See https://github.com/sidebase/nuxt-auth/blob/main/src/runtime/composables/local/useAuth.ts#L142
// This is a custom API call to check if we get an error. If it returns an error it is caught upstream.
await useNuxtApp().$backendFetch('/user/me');
// Second is a call to really get the user session so the Sidebase Auth module is filled. Redirect after.
await getSession();
const { redirectedFrom } = useRoute();
navigateTo(redirectedFrom ? redirectedFrom.fullPath : '/');
}
@bitfactory-frank-spee
I have a similar issue (described in detail in #998). The workaround you mentioned doesn't appear to fix the issue I've described, does it? Do you know any solution for my issue?
As a suggestion: It would be nice if we could pass a custom errorHandler function to override the catch block of the getSession function. This way the developer could decide when and how to logout the user; it could be a 401 error or any other custom condition. And if not passed, the default behavior takes precedence.
https://github.com/sidebase/nuxt-auth/blob/956b0fa1b5aed3fb94a47563a9e47dc811940a6a/src/runtime/composables/local/useAuth.ts#L143-L151
I created this PR (https://github.com/sidebase/nuxt-auth/pull/999). I'm generally new to contributing to open-source projects, so forgive me if I've made any mistakes in the PR. I deliberately left the PR as minimal as possible to get feedback and of course, to see if the feature is actually a valid feature in your opinion.
Update: Closed the PR for now because it doesn't work. Will reopen it when it works.
It looks like this issue is going stale for some time now. I think @sadeghi-aa has a good point on how to solve it with a hook without breaking existing functionality. I don't know if this is still relevant for the latest nuxt-auth version. Shame the #999 PR is not finished. It looks like a good idea, maybe not do a return in the catch though. Just call the hook.
I have a similar issue (described in detail in #998). The workaround you mentioned doesn't appear to fix the issue I've described, does it? Do you know any solution for my issue?
Issue https://github.com/sidebase/nuxt-auth/issues/998 is indeed the same as this one, so looks duplicate to me. But I see in that issue that it is actually called a as duplicate of https://github.com/sidebase/nuxt-auth/issues/964 which links to https://github.com/sidebase/nuxt-auth/pull/1062. @phoenix-ru it would be a good idea to consolidate all these issues into 1 main one and close the others IMO.
@sadeghi-aa I don't know why my workaround would not work anymore or for you, we are still using it. Although I am not sure which version of nuxt-auth we are currently using with it in our projects. You do need to check if callGetSession is available in your used version.
Shame the https://github.com/sidebase/nuxt-auth/pull/999 PR is not finished
Actually you'll get a way for customizing anything in the local provider (which will be a new provider) when #1062 lands 🙂 That PR is not so far from being ready for testing, after which it'll land as an experimental option.
You'll be able to have a way better flexibility with it, so please don't hesitate to look into the PR and give early feedback. You can also read the docs (either in .md or if you checkout the PR and run pnpm docs:dev)
would be a good idea to consolidate all these issues into 1 main one and close the others IMO.
Agreed, will do the next time I am working on NuxtAuth
Shame the #999 PR is not finished
Actually you'll get a way for customizing anything in the
localprovider (which will be a new provider) when #1062 lands 🙂 That PR is not so far from being ready for testing, after which it'll land as an experimental option.You'll be able to have a way better flexibility with it, so please don't hesitate to look into the PR and give early feedback. You can also read the docs (either in
.mdor if you checkout the PR and runpnpm docs:dev)
That sounds really nice, excellent work 👍 👍 I will look into that.
would be a good idea to consolidate all these issues into 1 main one and close the others IMO.
Agreed, will do the next time I am working on NuxtAuth
👍 🙌