supertokens-node icon indicating copy to clipboard operation
supertokens-node copied to clipboard

Changing cookieDomain can lead to a session refresh loop

Open anku255 opened this issue 1 year ago • 1 comments

Issue

There is an edge case where changing the cookieDomain config on the server can lead to session integrity issues. For instance, if the API server URL is 'api.example.com' with a cookie domain of '.example.com', and the server updates the cookie domain to 'api.example.com' the client may retain cookies with both '.example.com' and 'api.example.com' domains.

Consequently, if the server chooses the older cookie, session invalidation occurs, potentially resulting in an infinite refresh loop.

Solution

To fix this, users are asked to specify "olderCookieDomain" in the config. After this the flow would like -

  • apiDomain: 'api.example.com'
  • cookieDomain: 'api.example.com'

Flow:

  1. After authentication, the frontend has cookies set with domain=api.example.com, but the access token has expired.
  2. The server updates cookieDomain to .example.com.
  3. An API call requiring session with an expired access token (cookie with domain=api.example.com) results in a 401 response.
  4. The frontend attempts to refresh the session, generating a new access token saved with domain=.example.com.
  5. The original API call is retried, but because it sends both the old and new cookies, it again results in a 401 response.
  6. The frontend tries to refresh the session with multiple access tokens:
    • If olderCookieDomain is not set, the refresh fails with a 500 error.
      • The user remains stuck until they clear cookies manually or olderCookieDomain is set.
    • If olderCookieDomain is set, the refresh clears the older cookie, returning a 200 response.
      • The frontend retries the original API call, sending only the new cookie (domain=.example.com), resulting in a successful request.

TODOs

  • [x] Fix the errorHandler override in all the backend SDKs
  • [x] The CLI PR needs a comment + undo version changes
  • [ ] The docs PR is also back into in review
  • [ ] Need to also make frontend SDK changes to ignore leading dot when setting frontendScope.
  • [ ] Need to add end to end tests with supertokens-website repo.
    • [ ] Simulate this issue in a e2e test and make sure it doesn't get stuck
  • [x] we are now checking in a bunch of places that the header mode is any or cookies and that the access token cookie is present. But maybe, we should add another condition to it that the access token cookie is present AND that access token is one that was created by supertokens?
    • [x] I don't think we need to do that, we generally only need to check this in headers, because using the Authorization header is a common thing, while using the sAccessToken cookie is not.

anku255 avatar May 02 '24 05:05 anku255

In Server Side Rendering (SSR), we use the TryRefreshComponent to refresh sessions.

If olderCookieDomain is set when changing the cookieDomain, clients with session cookies under the old domain will require multiple refresh cycles to update their expired session cookies. The useEffect in TryRefreshComponent doesn't run after the first cycle, causing the UI to stall at "Loading...". To resolve this, add a key prop to TryRefreshComponent to force re-rendering, ensuring useEffect runs again.

anku255 avatar May 03 '24 04:05 anku255