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

ensure login test models correct behavior #217

Merged
merged 2 commits into from
Apr 10, 2024
Merged

ensure login test models correct behavior #217

merged 2 commits into from
Apr 10, 2024

Conversation

maxcountryman
Copy link
Owner

With the release of tower-sessions 0.12.1, we addressed a potential security issue related to cycling session IDs. A side effect of that fix is that we uncovered incorrect modeling of the login behavior within our login test in axum-login.

Our login test is intended to assert, among other things, that a successful login will cycle the session ID. Previously, we checked that the original session ID and the new session ID did not match. However, this assumed behavior of tower-sessions that was incorrect (cycling an ID should in fact set the session ID to None). Instead, the session ID is only updated once the session is saved, e.g. by the tower-sessions middleware, when it resolves a response.

Here we've addressed this by more closely modeling the middleware.

Closes #215.

With the release of tower-sessions `0.12.1`, we addressed a potential
security issue related to cycling session IDs. A side effect of that fix
is that we uncovered incorrect modeling of the login behavior within our
login test in axum-login.

Our login test is intended to assert, among other things, that a
successful login will cycle the session ID. Previously, we checked that
the original session ID and the new session ID did not match. However,
this assumed behavior of tower-sessions that was incorrect (cycling an
ID should in fact set the session ID to `None`). Instead, the session ID
is only updated once the session is saved, e.g. by the tower-sessions
middleware, when it resolves a response.

Here we've addressed this by more closely modeling the middleware.

Closes #215.
@maxcountryman maxcountryman force-pushed the test-login branch 3 times, most recently from 70d4bb3 to 3a94749 Compare April 10, 2024 17:34
@maxcountryman maxcountryman merged commit ac5ee47 into main Apr 10, 2024
5 checks passed
@maxcountryman maxcountryman deleted the test-login branch April 10, 2024 17:43
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.

cargo test failing on session::tests::test_login
1 participant