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

API returning a 401 error can lead to Axios infinite request loop #113

Open
fa-sharp opened this issue Mar 26, 2022 · 2 comments
Open

API returning a 401 error can lead to Axios infinite request loop #113

fa-sharp opened this issue Mar 26, 2022 · 2 comments

Comments

@fa-sharp
Copy link

fa-sharp commented Mar 26, 2022

Hi there 👋,

First, I want to say I'm loving the library so far - as someone who is relatively new to coding, this makes it easy (and even fun!) to implement authentication, and I really appreciate it. Thanks so much for all your hard work on this!

Bug description

I'm using the Axios interceptor in a client-side React app to make requests to my Fastify server (where Supertokens is set up).

So, I've identified a bug where if a user is already signed in and has a current session, and then they make a request to my API and receive a 401 error, the Axios interceptor assumes that the session is expired and automatically sends a request to refresh the session. But in my case, the user already has a current session, and my API is returning a 401 error for an entirely different reason. So the client keeps trying the /session/refresh route (which is successful) and then re-trying my API route (which returns 401), and this becomes an infinite loop. (I can see this in the Network tab of Chrome Dev tools)

There are several ways for me to work around this, the easiest of which is to just have my API route return a different error code than 401. But I'm pretty sure this is a bug that needs to be addressed, so as to avoid the loop!

Steps to reproduce the issue

  1. Set up an API route protected by the verifySession middleware (i'm using Fastify, but it can be any Node server). The route returns a 401 error even after the session is verified through verifySession
  2. On the client side, set up Supertokens and Axios interceptor. Sign in a user, and then make a request to the API route through Axios

I'd be happy to put together a more concrete example if needed. Let me know!

Code that may be the issue

I have a hunch that this happens because of one of the status code checks below (or a similar one somewhere else), where it's only checking the response.status code of the response, and so it automatically assumes that the session needs to be refreshed if the API returns a 401 error. But if the user already has a current session at this point, and it keeps getting a 401 error from an API route - this results in the loop. Perhaps some code could be added here to make sure that the 401 error is actually happening because of an expired session?

if (response.status === AuthHttpRequestFetch.config.sessionExpiredStatusCode) {
let config = response.config;
return AuthHttpRequest.doRequest(
(config: AxiosRequestConfig) => {
// we create an instance since we don't want to intercept this.
// const instance = axios.create();
// return instance(config);
return axiosInstance(config);
},
config,
url,
response,
true
);

if (response.status === AuthHttpRequestFetch.config.sessionExpiredStatusCode) {
const refreshResult = await onUnauthorisedResponse(preRequestIdToken);
if (refreshResult.result !== "RETRY") {
// Returning refreshResult.error as an Axios Error if we attempted a refresh
// Returning the response to the original response as an error if we did not attempt refreshing
returnObj = refreshResult.error
? await createAxiosErrorFromFetchResp(refreshResult.error)
: await createAxiosErrorFromAxiosResp(response);
break;
}

@rishabhpoddar
Copy link
Contributor

Thanks for the detailed description @fa-sharp. This is a good point. There is no easy way for the frontend (on its own) to know if the 401 is cause of an expired access token vs some other reason.

The only reliable solution is for the backend to send a message along with the 401 indicating to the frontend that the 401 is cause of an expired access token.

Right now, from our SDKs, the backend sends a message like "try refresh token" which the frontend could check for, however, we do have several users who have made their own backend SDK and there is no guarantee that they will return the same message to the frontend.

Due to this, if we have to enforce such a message (which I think is a good idea), then this would be a breaking change. As a result, we will want to release this coupled with other breaking changes. So it's not going to happen very soon.

As a work around, you can:

Keeping this issue open until it's resolved.

@fa-sharp
Copy link
Author

Sounds good 👍, thank you! I didn't realize you can set a custom status code for session expiry, that's an interesting solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants