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

Session Expired Response Handling #53

Open
rkeshwani opened this issue Dec 14, 2021 · 2 comments
Open

Session Expired Response Handling #53

rkeshwani opened this issue Dec 14, 2021 · 2 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@rkeshwani
Copy link

Right now, if you leave your browser window open too long and then attempt to make a request to your laravel application from your React Application, the token times out and the module doesn't automatically refresh the token, there is an error thrown CSRF Token mismatch.

Possibly include a configuration option that automatically enables an axios response interceptor that runs a function from the configuration when status code 419: Session Expired is returned?

As explained here: https://laracasts.com/discuss/channels/general-discussion/detecting-expired-session-with-sanctumspa
This leaves it open for the user to either write a function to reload the window, or reroute the user to re-authenticate, or if they want to try to refresh the session using the csrf method that is already provided.

@koole koole added the enhancement New feature or request label Dec 14, 2021
@koole
Copy link
Owner

koole commented Dec 14, 2021

I’m not sure how this could would work. This package only deals with requests for sanctum, so it doesn’t know about the request you’re making to the API, which might return a 419.

I feel like adding an interceptor to Axios like the example you linked would be a bad idea in a package like this, as it would completely overwrite the interceptors the users’ own interceptors, which is not desired at all.

I think what might be best is to add a piece of documentation describing how to deal with token refreshing on errors, so people can add it to their own interceptors. That way there is no need for this package to change the global axios behavior.

Would you agree? I might be missing something, so if you have a good solution I’m all ears.

@koole koole added the question Further information is requested label Dec 14, 2021
@rkeshwani
Copy link
Author

Hmm. So my understanding of the official documentation https://axios-http.com/docs/interceptors of the Axios interceptors is that it works like a delegation pattern I think? So every function sent to axios.interceptors.request.use method is called when "intercepting" a request. It won't overwrite the user's own interceptors. Internally it looks like there is an array that behaves like a stack with these functions.
https://github.com/axios/axios/blob/d99d5faac29899eba68ce671e6b3cbc9832e9ad8/lib/core/InterceptorManager.js#L17

As you suggested adding some documentation to address this issue is a good idea in case others run into it. Based on the Axios documentation most will want to handle this at the global level and so the suggested implementation may have to include that the implementation of the interceptors would have to be done to the global instance of Axios .
If the global instance of Axios's interceptors is set AFTER the local instance of Axios is instantiated then the user would still need some form of access to the localAxios instance IF the user doesn't specify an instance of Axios in SanctumConfig.

The suggestion below is not needed but if you were to implement it this could be a potential solution to adding to the interceptors in the localAxios instance:

One suggestion might be to add an optional parameter to the sanctum config called sessionTimeoutHandler:
interface Props { config: { apiUrl: string; csrfCookieRoute: string; signInRoute: string; signOutRoute: string; userObjectRoute: string; twoFactorChallengeRoute?: string; axiosInstance?: AxiosInstance; sessionTimeoutHandler?: Function; }; checkOnInit?: boolean; }

Then after the Axios instance initialization,
if(config.sessionTimeoutHandler) { localAxiosInstance.interceptors.response.use(config.sessionTimeoutHandler) }
This will respect anything the user wants to do and let the user optionally handle any response from the localAxios instance within the Sanctum module. My typescript knowledge is not great so the code above might not work correctly if only copy/pasted.

There are two issues I see with implementation:

  • If what I suggest is implemented, might have to think of a way to eject the interceptor in case a user needs to do that for some reason. in that scenario, some exportable variable might need to be created that wraps around the eject method like checkAuthentication.

  • There is another issue here is that there is an increasing dependency on the Axios library with this implementation which I think you were eluding to above. In this case alot of things in the library would need to be abstracted to remove the dependency on Axios.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants