Skip to content

Commit

Permalink
ensure login test models correct behavior (#217)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
maxcountryman committed Apr 10, 2024
1 parent e959752 commit ac5ee47
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 4 deletions.
4 changes: 2 additions & 2 deletions axum-login/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ thiserror = "1.0.49"
tower-cookies = "0.10.0"
tower-layer = "0.3.2"
tower-service = "0.3.2"
tower-sessions = "0.12.0"
tower-sessions = "0.12.1"
tracing = { version = "0.1.40", features = ["log"] }
urlencoding = "2.1.3"
form_urlencoded = "1.2.1"
Expand All @@ -37,7 +37,7 @@ axum = "0.7.0"
mockall = "0.12"
reqwest = { version = "0.11.22", features = ["cookies"] }
serial_test = "3.0.0"
time = "0.3.31"
time = "=0.3.34"
tokio = { version = "1.32.0", features = ["full"] }
tokio-test = "0.4.3"
tower = "0.4.13"
Expand Down
12 changes: 10 additions & 2 deletions axum-login/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,14 +311,22 @@ mod tests {
user: None,
backend: mock_backend,
data: Data::default(),
session,
session: session.clone(),
data_key: "auth_data",
};

auth_session.login(&mock_user).await.unwrap();
assert!(auth_session.user.is_some());
assert_eq!(auth_session.user.unwrap().id(), 42);
assert_ne!(original_session_id, auth_session.session.id());

// Simulate request persisting session.
session.save().await.unwrap();

// We were provided no session initially.
assert!(original_session_id.is_none());

// We have a session ID after saving.
assert!(session.id().is_some());
}

#[tokio::test]
Expand Down

0 comments on commit ac5ee47

Please sign in to comment.