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

fixed cookie refresh bug #876

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

Conversation

Arcane-Ryn
Copy link

Issue #824. Before, if a user was logged in with the login_user function when the remember parameter was set to false, their cookies would still be refreshed if the "REMEMBER_COOKIE_REFRESH_EACH_REQUEST" configuration option was set to true. This happens because if the login_user function has the remember parameter be false, it doesn't assign session["_rememeber"] any value. When session["_rememeber"] doesn't have any value and the "REMEMBER_COOKIE_REFRESH_EACH_REQUEST" configuration option is set to true, the _update_remember_cookie function sets the session["_rememeber"] value to "set". This fix makes it so if the login_user function is given false for the remember parameter, instead of leaving session["_remember"] empty, it sets the value to "unset".

Issue maxcountryman#824. Before, if a user was logged in with the login_user function when the remember parameter was set to false, their cookies would still be refreshed if the "REMEMBER_COOKIE_REFRESH_EACH_REQUEST" configuration option was set to true. This happens because if the login_user function has the remember parameter be false, it doesn't assign session["_rememeber"] any value. When session["_rememeber"] doesn't have any value and the "REMEMBER_COOKIE_REFRESH_EACH_REQUEST" configuration option is set to true, the _update_remember_cookie function sets the session["_rememeber"] value to "set". This fix makes it so if the login_user function is given false for the remember parameter, instead of leaving session["_remember"] empty, it  sets the value to "unset".
@coveralls
Copy link

coveralls commented Aug 10, 2024

Coverage Status

coverage: 95.976% (+0.03%) from 95.951%
when pulling 5638c48 on Arcane-Ryn:824-bugfix-fixcookierefresh
into 26d12ea on maxcountryman:main.

@davidism
Copy link
Collaborator

Can you add a test that demonstrates how this fixes the issue?

Added unit tests for each different combination of values for the "remember" and "REMEMBER_COOKIE_REFRESH_EACH_REQUEST" values.
@Arcane-Ryn
Copy link
Author

I've added the tests. Please forgive the mountain of commits, I still haven't gotten used to the proper code formatting.

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.

3 participants