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: correct cookie domain on logout #646
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This influences the global cookie management, which effectively changes e.g. the continuity module behavior. Let's either have a dedicated store for session cookies, or fix this in the logout logic :)
Also, please add at least one failing test case to prevent future regression!
@aeneasr I've just created a seperate CookieManager for the continuity module. If you'd rather have it the other way around I can refactor it but I had no idea of a good name (sessionSessionStore?) As for the test perhaps you can guide me in the right direction. As far as I can see it is hard to test cookies with a different domain setting as the server is running on |
@aeneasr I know you're probably busy with the other projects but please let me know if there is anything I can do to get this issue fixed and merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the long delay on this one, I was very deep into #624 and then we had a company retreat to plan the next milestones.
Awesome 🎉 Thank you for your contribution! |
Related issue
#645
Proposed changes
Checklist
(Not sure what my commit message should've had as prefix?)
vulnerability. If this pull request addresses a security. vulnerability, I
confirm that I got green light (please contact
security@ory.sh) from the maintainers to push
the changes.
works.
Further comments