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

fix(api): clean session cookies only for invalid refresh token #2049

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

smalluban
Copy link
Collaborator

@smalluban smalluban commented Nov 12, 2024

I've checked how the v8 and account-web process rejected requests for refreshing the token with the following code (almost identical code). The code only checks if the response status code is higher or equal to 400.

if (status >= 400) {
  res.json()
    .then((data2) =>
      this._errorHandler(data2.errors)
        .then(() => resolve(data2))
        .catch((err) => reject(err)),
    )
    .catch((err) => reject(err));
}

Moreover, I have not found any code, which cleans up the cookies when refreshing the token fails. We added this because we had a bug in Safari with setting the correct cookie expiration date (some time ago). We can still keep the cleanup, but it should be done only if the server explicitly responds with 400 code - so we are sure that something is wrong with the cookie itself.

Without the fix, if a user loses internet connection on that call, the catch will clean up the session.

@smalluban smalluban requested a review from chrmod November 12, 2024 15:27
@smalluban smalluban merged commit 9e50578 into main Nov 12, 2024
2 checks passed
@smalluban smalluban deleted the fix-api-session branch November 12, 2024 16:34
@smalluban smalluban mentioned this pull request Nov 13, 2024
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

Successfully merging this pull request may close these issues.

2 participants