Skip to content

Commit

Permalink
manually set path and domain on removal (#154)
Browse files Browse the repository at this point in the history
This ensures that configured paths and domains are provided to the
cookie managed by the underlying cookie jar.

Note that this has no bearing on deletion from stores, which handle
removal regardless of this change.
  • Loading branch information
maxcountryman authored Feb 6, 2024
1 parent 98fd377 commit 9c883d3
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 13 deletions.
12 changes: 11 additions & 1 deletion src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,18 @@ where
tracing::trace!(modified = modified, empty = empty, "session response state");

match session_cookie {
Some(cookie) if empty => {
Some(mut cookie) if empty => {
tracing::debug!("removing session cookie");

// Path and domain must be manually set to ensure a proper removal cookie is
// constructed.
//
// See: https://docs.rs/cookie/latest/cookie/struct.CookieJar.html#method.remove
cookie.set_path(session_config.path);
if let Some(domain) = session_config.domain {
cookie.set_domain(domain);
}

cookies.remove(cookie)
}

Expand Down
52 changes: 42 additions & 10 deletions tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,16 @@ fn routes() -> Router {
pub fn build_app<Store: SessionStore + Clone>(
mut session_manager: SessionManagerLayer<Store>,
max_age: Option<Duration>,
domain: Option<String>,
) -> Router {
if let Some(max_age) = max_age {
session_manager = session_manager.with_expiry(Expiry::OnInactivity(max_age));
}

if let Some(domain) = domain {
session_manager = session_manager.with_domain(domain);
}

routes().layer(session_manager)
}

Expand Down Expand Up @@ -92,7 +97,7 @@ macro_rules! route_tests {
#[tokio::test]
async fn no_session_set() {
let req = Request::builder().uri("/").body(Body::empty()).unwrap();
let res = $create_app(Some(Duration::hours(1)))
let res = $create_app(Some(Duration::hours(1)), None)
.await
.oneshot(req)
.await
Expand All @@ -114,7 +119,7 @@ macro_rules! route_tests {
.header(header::COOKIE, session_cookie.encoded().to_string())
.body(Body::empty())
.unwrap();
let res = $create_app(Some(Duration::hours(1)))
let res = $create_app(Some(Duration::hours(1)), None)
.await
.oneshot(req)
.await
Expand All @@ -133,7 +138,7 @@ macro_rules! route_tests {
.header(header::COOKIE, session_cookie.encoded().to_string())
.body(Body::empty())
.unwrap();
let res = $create_app(Some(Duration::hours(1)))
let res = $create_app(Some(Duration::hours(1)), None)
.await
.oneshot(req)
.await
Expand All @@ -150,7 +155,7 @@ macro_rules! route_tests {
.uri("/insert")
.body(Body::empty())
.unwrap();
let res = $create_app(Some(Duration::hours(1)))
let res = $create_app(Some(Duration::hours(1)), None)
.await
.oneshot(req)
.await
Expand All @@ -173,7 +178,7 @@ macro_rules! route_tests {
.uri("/insert")
.body(Body::empty())
.unwrap();
let res = $create_app(None).await.oneshot(req).await.unwrap();
let res = $create_app(None, None).await.oneshot(req).await.unwrap();
let session_cookie = get_session_cookie(res.headers()).unwrap();

assert_eq!(session_cookie.name(), "id");
Expand All @@ -186,7 +191,7 @@ macro_rules! route_tests {

#[tokio::test]
async fn get_session() {
let app = $create_app(Some(Duration::hours(1))).await;
let app = $create_app(Some(Duration::hours(1)), None).await;

let req = Request::builder()
.uri("/insert")
Expand All @@ -208,7 +213,7 @@ macro_rules! route_tests {

#[tokio::test]
async fn get_no_value() {
let app = $create_app(Some(Duration::hours(1))).await;
let app = $create_app(Some(Duration::hours(1)), None).await;

let req = Request::builder()
.uri("/get_value")
Expand All @@ -221,7 +226,7 @@ macro_rules! route_tests {

#[tokio::test]
async fn remove_last_value() {
let app = $create_app(Some(Duration::hours(1))).await;
let app = $create_app(Some(Duration::hours(1)), None).await;

let req = Request::builder()
.uri("/insert")
Expand Down Expand Up @@ -249,7 +254,7 @@ macro_rules! route_tests {

#[tokio::test]
async fn cycle_session_id() {
let app = $create_app(Some(Duration::hours(1))).await;
let app = $create_app(Some(Duration::hours(1)), None).await;

let req = Request::builder()
.uri("/insert")
Expand Down Expand Up @@ -279,7 +284,32 @@ macro_rules! route_tests {

#[tokio::test]
async fn flush_session() {
let app = $create_app(Some(Duration::hours(1))).await;
let app = $create_app(Some(Duration::hours(1)), None).await;

let req = Request::builder()
.uri("/insert")
.body(Body::empty())
.unwrap();
let res = app.clone().oneshot(req).await.unwrap();
let session_cookie = get_session_cookie(res.headers()).unwrap();

let req = Request::builder()
.uri("/flush")
.header(header::COOKIE, session_cookie.encoded().to_string())
.body(Body::empty())
.unwrap();
let res = app.oneshot(req).await.unwrap();

let session_cookie = get_session_cookie(res.headers()).unwrap();

assert_eq!(session_cookie.value(), "");
assert_eq!(session_cookie.max_age(), Some(Duration::ZERO));
assert_eq!(session_cookie.path(), Some("/"));
}

#[tokio::test]
async fn flush_with_domain() {
let app = $create_app(Some(Duration::hours(1)), Some("localhost".to_string())).await;

let req = Request::builder()
.uri("/insert")
Expand All @@ -299,6 +329,8 @@ macro_rules! route_tests {

assert_eq!(session_cookie.value(), "");
assert_eq!(session_cookie.max_age(), Some(Duration::ZERO));
assert_eq!(session_cookie.domain(), Some("localhost"));
assert_eq!(session_cookie.path(), Some("/"));
}
};
}
4 changes: 2 additions & 2 deletions tests/integration-tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ mod memory_store_tests {

use crate::common::build_app;

async fn app(max_age: Option<Duration>) -> Router {
async fn app(max_age: Option<Duration>, domain: Option<String>) -> Router {
let session_store = MemoryStore::default();
let session_manager = SessionManagerLayer::new(session_store).with_secure(true);
build_app(session_manager, max_age)
build_app(session_manager, max_age, domain)
}

route_tests!(app);
Expand Down

0 comments on commit 9c883d3

Please sign in to comment.