Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 84 additions & 1 deletion src/rbac/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,17 @@ impl Sessions {
let Some(sessions) = self.user_sessions.get_mut(user) else {
return;
};
sessions.retain(|(_, expiry)| expiry < &now);
let initial_count = sessions.len();
sessions.retain(|(_, expiry)| expiry > &now);
let removed_count = initial_count - sessions.len();
if removed_count > 0 {
tracing::debug!(
user = user,
removed = removed_count,
remaining = sessions.len(),
"Removed expired sessions for user"
);
}
}

// get permission related to this session
Expand Down Expand Up @@ -353,3 +363,76 @@ impl From<Vec<UserGroup>> for UserGroups {
map
}
}

#[cfg(test)]
mod tests {
use super::*;
use chrono::Days;

#[test]
fn test_remove_expired_session_keeps_valid_sessions() {
let mut sessions = Sessions::default();
let user = "test_user".to_string();
let key = SessionKey::SessionId(ulid::Ulid::new());

// Add a session that expires 7 days from now (valid session)
let future_expiry = Utc::now() + Days::new(7);
sessions.track_new(user.clone(), key.clone(), future_expiry, vec![]);

// Verify session exists
assert!(sessions.get(&key).is_some());
assert_eq!(sessions.user_sessions.get(&user).unwrap().len(), 1);

// Trigger expired session cleanup by adding another session
let key2 = SessionKey::SessionId(ulid::Ulid::new());
sessions.track_new(user.clone(), key2.clone(), future_expiry, vec![]);

// Both valid sessions should still exist
assert!(sessions.get(&key).is_some());
assert!(sessions.get(&key2).is_some());
assert_eq!(sessions.user_sessions.get(&user).unwrap().len(), 2);
}

#[test]
fn test_remove_expired_session_removes_expired_sessions() {
let mut sessions = Sessions::default();
let user = "test_user".to_string();
let key = SessionKey::SessionId(ulid::Ulid::new());

// Add a session that expired 1 day ago
let past_expiry = Utc::now() - Days::new(1);
sessions.user_sessions.insert(user.clone(), vec![(key.clone(), past_expiry)]);
sessions.active_sessions.insert(key.clone(), (user.clone(), vec![]));

// Verify expired session exists before cleanup
assert!(sessions.get(&key).is_some());

// Trigger cleanup by adding a new session
let key2 = SessionKey::SessionId(ulid::Ulid::new());
let future_expiry = Utc::now() + Days::new(7);
sessions.track_new(user.clone(), key2.clone(), future_expiry, vec![]);

// After cleanup, the expired session should be removed from user_sessions.
// Note: remove_expired_session only cleans up user_sessions, not active_sessions.
// The active_sessions cleanup happens separately when the session is accessed.
let user_sess = sessions.user_sessions.get(&user).unwrap();
// Only the new valid session should remain in user_sessions
assert_eq!(user_sess.len(), 1);
// The new session should be the one with future expiry
assert!(user_sess.iter().any(|(k, _)| k == &key2));
}

#[test]
fn test_session_expiry_logic_correctness() {
// This test verifies the fix for the bug where expired sessions
// were being kept and valid sessions were being removed
let now = Utc::now();
let past = now - Days::new(1);
let future = now + Days::new(1);

// The retain predicate should keep sessions where expiry > now
// i.e., sessions that have NOT yet expired
assert!(future > now); // Future sessions should be kept
assert!(!(past > now)); // Past sessions should be removed
}
}
Loading