1125 expired session cleanup (#1162)

kanidm_proto/src/v1.rs

@@ -366,8 +366,10 @@ pub enum UatPurpose {
     IdentityOnly,
     ReadOnly,
     ReadWrite {
-        #[serde(with = "time::serde::timestamp")]
-        expiry: time::OffsetDateTime,
+        /// If none, there is no expiry, and this is always rw. If there is
+        /// an expiry, check that the current time < expiry.
+        #[serde(with = "time::serde::timestamp::option")]
+        expiry: Option<time::OffsetDateTime>,
     },
 }
 
@@ -386,6 +388,8 @@ pub struct UserAuthToken {
     pub auth_type: AuthType,
     #[serde(with = "time::serde::timestamp")]
     pub issued_at: time::OffsetDateTime,
+    /// If none, there is no expiry, and this is always valid. If there is
+    /// an expiry, check that the current time < expiry.
     #[serde(with = "time::serde::timestamp::option")]
     pub expiry: Option<time::OffsetDateTime>,
     pub purpose: UatPurpose,
@@ -410,8 +414,11 @@ impl fmt::Display for UserAuthToken {
         match &self.purpose {
             UatPurpose::IdentityOnly => writeln!(f, "purpose: identity only")?,
             UatPurpose::ReadOnly => writeln!(f, "purpose: read only")?,
-            UatPurpose::ReadWrite { expiry } => {
-                writeln!(f, "purpose: read write (expiry: {})", expiry)?
+            UatPurpose::ReadWrite {
+                expiry: Some(expiry),
+            } => writeln!(f, "purpose: read write (expiry: {})", expiry)?,
+            UatPurpose::ReadWrite { expiry: None } => {
+                writeln!(f, "purpose: read write (expiry: none)")?
             }
         }
         /*

kanidmd/core/src/actors/v1_write.rs

@@ -1437,7 +1437,7 @@ impl QueryServerWriteV1 {
         let ct = duration_from_epoch_now();
         let mut idms_prox_write = self.idms.proxy_write(ct).await;
         if let Err(res) = idms_prox_write
-            .process_delayedaction(da)
+            .process_delayedaction(da, ct)
             .and_then(|_| idms_prox_write.commit())
         {
             admin_info!(?res, "delayed action error");

kanidmd/lib/src/idm/account.rs

@@ -189,6 +189,7 @@ impl Account {
         session_id: Uuid,
         ct: Duration,
         auth_type: AuthType,
+        expiry_secs: Option<u64>,
     ) -> Option<UserAuthToken> {
         // This could consume self?
         // The cred handler provided is what authenticated this user, so we can use it to
@@ -196,15 +197,16 @@ impl Account {
         // Get the claims from the cred_h
 
         // TODO: Apply policy to this expiry time.
-        let expiry = OffsetDateTime::unix_epoch() + ct + Duration::from_secs(AUTH_SESSION_EXPIRY);
+        let expiry = expiry_secs
+            .map(|offset| OffsetDateTime::unix_epoch() + ct + Duration::from_secs(offset));
         let issued_at = OffsetDateTime::unix_epoch() + ct;
-        // TODO: Apply priv expiry.
-        let purpose = UatPurpose::ReadWrite { expiry: expiry };
+        // TODO: Apply priv expiry, and what type of token this is (ident, ro, rw).
+        let purpose = UatPurpose::ReadWrite { expiry };
 
         Some(UserAuthToken {
             session_id,
             auth_type,
-            expiry: Some(expiry),
+            expiry,
             issued_at,
             purpose,
             uuid: self.uuid,

kanidmd/lib/src/idm/authsession.rs

@@ -768,7 +768,12 @@ impl AuthSession {
 
                         let uat = self
                             .account
-                            .to_userauthtoken(session_id, *time, auth_type.clone())
+                            .to_userauthtoken(
+                                session_id,
+                                *time,
+                                auth_type.clone(),
+                                Some(AUTH_SESSION_EXPIRY),
+                            )
                             .ok_or(OperationError::InvalidState)?;
 
                         // Queue the session info write.

kanidmd/lib/src/idm/oauth2.rs

@@ -1529,7 +1529,12 @@ mod tests {
             .expect("account must exist");
         let session_id = uuid::Uuid::new_v4();
         let uat = account
-            .to_userauthtoken(session_id, ct, AuthType::PasswordMfa)
+            .to_userauthtoken(
+                session_id,
+                ct,
+                AuthType::PasswordMfa,
+                Some(AUTH_SESSION_EXPIRY),
+            )
             .expect("Unable to create uat");
         let ident = idms_prox_write
             .process_uat_to_identity(&uat, ct)
@@ -1551,7 +1556,7 @@ mod tests {
             .expect("account must exist");
         let session_id = uuid::Uuid::new_v4();
         let uat = account
-            .to_userauthtoken(session_id, ct, authtype)
+            .to_userauthtoken(session_id, ct, authtype, Some(AUTH_SESSION_EXPIRY))
             .expect("Unable to create uat");
         let ident = idms_prox_write
             .process_uat_to_identity(&uat, ct)
@@ -1805,7 +1810,12 @@ mod tests {
                     .expect("account must exist");
                 let session_id = uuid::Uuid::new_v4();
                 let uat2 = account
-                    .to_userauthtoken(session_id, ct, AuthType::PasswordMfa)
+                    .to_userauthtoken(
+                        session_id,
+                        ct,
+                        AuthType::PasswordMfa,
+                        Some(AUTH_SESSION_EXPIRY),
+                    )
                     .expect("Unable to create uat");
                 let ident2 = idms_prox_write
                     .process_uat_to_identity(&uat2, ct)
@@ -2155,7 +2165,12 @@ mod tests {
                     .expect("account must exist");
                 let session_id = uuid::Uuid::new_v4();
                 let uat2 = account
-                    .to_userauthtoken(session_id, ct, AuthType::PasswordMfa)
+                    .to_userauthtoken(
+                        session_id,
+                        ct,
+                        AuthType::PasswordMfa,
+                        Some(AUTH_SESSION_EXPIRY),
+                    )
                     .expect("Unable to create uat");
                 let ident2 = idms_prox_write
                     .process_uat_to_identity(&uat2, ct)

kanidmd/lib/src/idm/server.rs

@@ -19,6 +19,7 @@ use kanidm_proto::v1::{
     UnixGroupToken, UnixUserToken, UserAuthToken,
 };
 use rand::prelude::*;
+use time::OffsetDateTime;
 use tokio::sync::mpsc::{
     unbounded_channel as unbounded, UnboundedReceiver as Receiver, UnboundedSender as Sender,
 };
@@ -331,11 +332,11 @@ impl IdmServer {
     #[cfg(test)]
     pub(crate) async fn delayed_action(
         &self,
-        ts: Duration,
+        ct: Duration,
         da: DelayedAction,
     ) -> Result<bool, OperationError> {
-        let mut pw = self.proxy_write(ts).await;
-        pw.process_delayedaction(da)
+        let mut pw = self.proxy_write(ct).await;
+        pw.process_delayedaction(da, ct)
             .and_then(|_| pw.commit())
             .map(|()| true)
     }
@@ -628,7 +629,10 @@ pub trait IdmServerTransaction<'a> {
         let scope = match uat.purpose {
             UatPurpose::IdentityOnly => AccessScope::IdentityOnly,
             UatPurpose::ReadOnly => AccessScope::ReadOnly,
-            UatPurpose::ReadWrite { expiry } => {
+            UatPurpose::ReadWrite { expiry: None } => AccessScope::ReadWrite,
+            UatPurpose::ReadWrite {
+                expiry: Some(expiry),
+            } => {
                 let cot = time::OffsetDateTime::unix_epoch() + ct;
                 if cot < expiry {
                     AccessScope::ReadWrite
@@ -2094,7 +2098,13 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
     pub(crate) fn process_authsessionrecord(
         &mut self,
         asr: &AuthSessionRecord,
+        ct: Duration,
     ) -> Result<(), OperationError> {
+        // We have to get the entry so we can work out if we need to expire any of it's sessions.
+
+        let entry = self.qs_write.internal_search_uuid(&asr.target_uuid)?;
+        let sessions = entry.get_ava_as_session_map("user_auth_token_session");
+
         let session = Value::Session(
             asr.session_id,
             Session {
@@ -2113,11 +2123,34 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
 
         info!(session_id = %asr.session_id, "Persisting auth session");
 
+        let offset_ct = OffsetDateTime::unix_epoch() + ct;
+
+        let mlist: Vec<_> = sessions
+            .iter()
+            .map(|item| item.iter())
+            .flatten()
+            .filter_map(|(k, v)| {
+                // We only check if an expiry is present
+                v.expiry.and_then(|exp| {
+                    if exp <= offset_ct {
+                        info!(session_id = %k, "Removing expired auth session");
+                        Some(Modify::Removed(
+                            AttrString::from("user_auth_token_session"),
+                            PartialValue::Refer(*k),
+                        ))
+                    } else {
+                        None
+                    }
+                })
+            })
+            .chain(std::iter::once(Modify::Present(
+                AttrString::from("user_auth_token_session"),
+                session,
+            )))
+            .collect();
+
         // modify the account to put the session onto it.
-        let modlist = ModifyList::new_list(vec![Modify::Present(
-            AttrString::from("user_auth_token_session"),
-            session,
-        )]);
+        let modlist = ModifyList::new_list(mlist);
 
         self.qs_write
             .internal_modify(
@@ -2152,14 +2185,18 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
         )
     }
 
-    pub fn process_delayedaction(&mut self, da: DelayedAction) -> Result<(), OperationError> {
+    pub fn process_delayedaction(
+        &mut self,
+        da: DelayedAction,
+        ct: Duration,
+    ) -> Result<(), OperationError> {
         match da {
             DelayedAction::PwUpgrade(pwu) => self.process_pwupgrade(&pwu),
             DelayedAction::UnixPwUpgrade(upwu) => self.process_unixpwupgrade(&upwu),
             DelayedAction::WebauthnCounterIncrement(wci) => self.process_webauthncounterinc(&wci),
             DelayedAction::BackupCodeRemoval(bcr) => self.process_backupcoderemoval(&bcr),
             DelayedAction::Oauth2ConsentGrant(o2cg) => self.process_oauth2consentgrant(&o2cg),
-            DelayedAction::AuthSessionRecord(asr) => self.process_authsessionrecord(&asr),
+            DelayedAction::AuthSessionRecord(asr) => self.process_authsessionrecord(&asr, ct),
         }
     }
 
@@ -2244,13 +2281,14 @@ mod tests {
     use async_std::task;
     use kanidm_proto::v1::{AuthAllowed, AuthIssueSession, AuthMech, AuthType, OperationError};
     use smartstring::alias::String as AttrString;
+    use time::OffsetDateTime;
     use uuid::Uuid;
 
     use crate::credential::policy::CryptoPolicy;
     use crate::credential::{Credential, Password};
     use crate::event::{CreateEvent, ModifyEvent};
     use crate::idm::account::DestroySessionTokenEvent;
-    use crate::idm::delayed::DelayedAction;
+    use crate::idm::delayed::{AuthSessionRecord, DelayedAction};
     use crate::idm::event::{AuthEvent, AuthResult};
     use crate::idm::event::{
         PasswordChangeEvent, RadiusAuthTokenEvent, RegenerateRadiusSecretEvent,
@@ -3655,9 +3693,10 @@ mod tests {
                     .expect("Failed to setup admin account");
                 let token = check_admin_password(idms, TEST_PASSWORD);
 
-                // Clear our the session record
+                // Clear out the queued session record
                 let da = idms_delayed.try_recv().expect("invalid");
                 assert!(matches!(da, DelayedAction::AuthSessionRecord(_)));
+                // Persist it.
                 let r = task::block_on(idms.delayed_action(duration_from_epoch_now(), da));
                 assert!(Ok(true) == r);
                 idms_delayed.check_is_empty_or_panic();
@@ -3679,6 +3718,95 @@ mod tests {
         )
     }
 
+    #[test]
+    fn test_idm_expired_auth_session_cleanup() {
+        run_idm_test!(|_qs: &QueryServer,
+                       idms: &IdmServer,
+                       _idms_delayed: &mut IdmServerDelayed| {
+            let ct = Duration::from_secs(TEST_CURRENT_TIME);
+            let expiry = ct + Duration::from_secs(AUTH_SESSION_EXPIRY + 1);
+
+            let session_a = Uuid::new_v4();
+            let session_b = Uuid::new_v4();
+
+            // Assert no sessions present
+            let idms_prox_read = task::block_on(idms.proxy_read());
+            let admin = idms_prox_read
+                .qs_read
+                .internal_search_uuid(&UUID_ADMIN)
+                .expect("failed");
+            let sessions = admin.get_ava_as_session_map("user_auth_token_session");
+            assert!(sessions.is_none());
+            drop(idms_prox_read);
+
+            let da = DelayedAction::AuthSessionRecord(AuthSessionRecord {
+                target_uuid: UUID_ADMIN,
+                session_id: session_a,
+                label: "Test Session A".to_string(),
+                expiry: Some(OffsetDateTime::unix_epoch() + expiry),
+                issued_at: OffsetDateTime::unix_epoch() + ct,
+                issued_by: IdentityId::User(UUID_ADMIN),
+                scope: AccessScope::IdentityOnly,
+            });
+            // Persist it.
+            let r = task::block_on(idms.delayed_action(ct, da));
+            assert!(Ok(true) == r);
+
+            // Check it was written, and check
+            let idms_prox_read = task::block_on(idms.proxy_read());
+            let admin = idms_prox_read
+                .qs_read
+                .internal_search_uuid(&UUID_ADMIN)
+                .expect("failed");
+            let sessions = admin
+                .get_ava_as_session_map("user_auth_token_session")
+                .expect("Sessions must be present!");
+            assert!(sessions.len() == 1);
+            let session_id_a = sessions
+                .keys()
+                .copied()
+                .next()
+                .expect("Could not access session id");
+            assert!(session_id_a == session_a);
+
+            drop(idms_prox_read);
+
+            // When we re-auth, this is what triggers the session cleanup via the delayed action.
+
+            let da = DelayedAction::AuthSessionRecord(AuthSessionRecord {
+                target_uuid: UUID_ADMIN,
+                session_id: session_b,
+                label: "Test Session B".to_string(),
+                expiry: Some(OffsetDateTime::unix_epoch() + expiry),
+                issued_at: OffsetDateTime::unix_epoch() + ct,
+                issued_by: IdentityId::User(UUID_ADMIN),
+                scope: AccessScope::IdentityOnly,
+            });
+            // Persist it.
+            let r = task::block_on(idms.delayed_action(expiry, da));
+            assert!(Ok(true) == r);
+
+            let idms_prox_read = task::block_on(idms.proxy_read());
+            let admin = idms_prox_read
+                .qs_read
+                .internal_search_uuid(&UUID_ADMIN)
+                .expect("failed");
+            let sessions = admin
+                .get_ava_as_session_map("user_auth_token_session")
+                .expect("Sessions must be present!");
+            trace!(?sessions);
+            assert!(sessions.len() == 1);
+            let session_id_b = sessions
+                .keys()
+                .copied()
+                .next()
+                .expect("Could not access session id");
+            assert!(session_id_b == session_b);
+
+            assert!(session_id_a != session_id_b);
+        })
+    }
+
     #[test]
     fn test_idm_account_session_validation() {
         run_idm_test!(
@@ -3774,7 +3902,7 @@ mod tests {
 
             // == anonymous
             let uat = account
-                .to_userauthtoken(session_id, ct, AuthType::Anonymous)
+                .to_userauthtoken(session_id, ct, AuthType::Anonymous, None)
                 .expect("Unable to create uat");
             let ident = idms_prox_write
                 .process_uat_to_identity(&uat, ct)
@@ -3788,7 +3916,7 @@ mod tests {
 
             // == unixpassword
             let uat = account
-                .to_userauthtoken(session_id, ct, AuthType::UnixPassword)
+                .to_userauthtoken(session_id, ct, AuthType::UnixPassword, None)
                 .expect("Unable to create uat");
             let ident = idms_prox_write
                 .process_uat_to_identity(&uat, ct)
@@ -3802,7 +3930,7 @@ mod tests {
 
             // == password
             let uat = account
-                .to_userauthtoken(session_id, ct, AuthType::Password)
+                .to_userauthtoken(session_id, ct, AuthType::Password, None)
                 .expect("Unable to create uat");
             let ident = idms_prox_write
                 .process_uat_to_identity(&uat, ct)
@@ -3816,7 +3944,7 @@ mod tests {
 
             // == generatedpassword
             let uat = account
-                .to_userauthtoken(session_id, ct, AuthType::GeneratedPassword)
+                .to_userauthtoken(session_id, ct, AuthType::GeneratedPassword, None)
                 .expect("Unable to create uat");
             let ident = idms_prox_write
                 .process_uat_to_identity(&uat, ct)
@@ -3830,7 +3958,7 @@ mod tests {
 
             // == webauthn
             let uat = account
-                .to_userauthtoken(session_id, ct, AuthType::Passkey)
+                .to_userauthtoken(session_id, ct, AuthType::Passkey, None)
                 .expect("Unable to create uat");
             let ident = idms_prox_write
                 .process_uat_to_identity(&uat, ct)
@@ -3844,7 +3972,7 @@ mod tests {
 
             // == passwordmfa
             let uat = account
-                .to_userauthtoken(session_id, ct, AuthType::PasswordMfa)
+                .to_userauthtoken(session_id, ct, AuthType::PasswordMfa, None)
                 .expect("Unable to create uat");
             let ident = idms_prox_write
                 .process_uat_to_identity(&uat, ct)

kanidmd/lib/src/lib.rs

@@ -78,7 +78,7 @@ pub mod prelude {
         f_and, f_andnot, f_eq, f_id, f_inc, f_lt, f_or, f_pres, f_self, f_spn_name, f_sub, Filter,
         FilterInvalid, FC,
     };
-    pub use crate::identity::{AccessScope, IdentType, Identity};
+    pub use crate::identity::{AccessScope, IdentType, Identity, IdentityId};
     pub use crate::modify::{m_pres, m_purge, m_remove, Modify, ModifyInvalid, ModifyList};
     pub use crate::server::{
         QueryServer, QueryServerReadTransaction, QueryServerTransaction,