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

Clear invalid session cookies #22327

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

paescuj
Copy link
Member

@paescuj paescuj commented Apr 26, 2024

Scope

In some cases tokens can become invalid. For example, if the signing secret has changed since the token was issued, which will especially be the case in dev/test environments after merging #22320.

A client with an invalid token won't be able to perform any requests. This is particularly cumbersome in browsers using the session cookie mode (Data Studio) as the cookie would have to be deleted manually, in order to be able to refresh the session (login) and get a valid token again.

This PR ensures such invalid session cookies are cleared from the client's cookie store. For the Data Studio, this means the login page will be shown instead of a reoccurring error message (for open tabs, a page refresh might be necessary).

Potential Risks / Drawbacks

  • This solution feels like the simplest one, but could perhaps present a somewhat unexpected / not that intuitive behavior. That seems negligible to me, as we are dealing with an edge case here respectively a case which is only to be expected in dev/tests environments.
  • The affected request will still fail, the clearing of the cookie will only take effect in subsequent requests.

Review Notes / Questions

None

Copy link

changeset-bot bot commented Apr 26, 2024

🦋 Changeset detected

Latest commit: 3ae3b1c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@directus/api Patch
directus Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@paescuj paescuj marked this pull request as draft May 2, 2024 18:42
@paescuj paescuj marked this pull request as ready for review May 2, 2024 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🙅 Blocked
Development

Successfully merging this pull request may close these issues.

None yet

4 participants