Conversation
psrpinto
force-pushed
the
delete-logout-cookie
branch
from
255eb3a to
c04c8b9
Compare
ashfame
reviewed
Jan 11, 2023
Member
ashfame
left a comment
ashfame
left a comment
There was a problem hiding this comment.
I suspect this would cause logout issues when used on a website which has caching plugin, so might be safer to always load the logout script file and not based on cookie check. To expand on it, imagine a WP site with logout redirection to homepage and has a caching plugin so homepage load never really checks for cookie presence for loading logout script.
Also, we have another problem with the logout. Inside logoutSession() fetch doesn't reject a 403 http request by design, so a failed logout attempt is considered as a success.
In sites with caching, page would not necessarily contain the script. We want to make sure script always runs. It's a very small script, around 500 bytes.
This was referenced Jan 25, 2023
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When the user logs out of WordPress, a
chatrix-logoutcookie is set, which will result in thelogout.tsscript runing, which logs out matrix sessions, and deletes all chatrix data in the browser.However, this cookie wasn't beeing expired after sessions had been logged out, which would result in the script running on all page loads (until the cookie expires). This PR fixes that by expiring the cookie once sessions have been logged out and data deleted.
Addiitionally, as suggested by @ashfame, the logout script is now only being enqueued when thechatrix-logoutcookie exists.