supertokens / supertokens-website

Frontend SDK for SuperTokens - for session management + automatically refreshing sessions
https://supertokens.com
Other
54 stars 13 forks source link

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

Open fa-sharp opened 2 years ago

fa-sharp commented 2 years ago

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? https://github.com/supertokens/supertokens-website/blob/1daef7b907c397747ed2ee18721225fc9ade1472/lib/ts/axios.ts#L157-L170 https://github.com/supertokens/supertokens-website/blob/1daef7b907c397747ed2ee18721225fc9ade1472/lib/ts/axios.ts#L344-L353

rishabhpoddar commented 2 years ago

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 commented 2 years ago

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