68 20230912 session consistency (#2110)

This adds support for special-casing sessions in replication to allow them to internally trim and merge so that session revocations and creations are not lost between replicas.

proto/src/v1.rs

@@ -342,13 +342,31 @@ pub enum UatPurposeStatus {
     PrivilegeCapable,
 }
 
+#[derive(Debug, Serialize, Deserialize, Clone)]
+#[serde(rename_all = "lowercase")]
+pub enum UatStatusState {
+    #[serde(with = "time::serde::timestamp")]
+    ExpiresAt(time::OffsetDateTime),
+    NeverExpires,
+    Revoked,
+}
+
+impl fmt::Display for UatStatusState {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            UatStatusState::ExpiresAt(odt) => write!(f, "expires at {}", odt),
+            UatStatusState::NeverExpires => write!(f, "never expires"),
+            UatStatusState::Revoked => write!(f, "revoked"),
+        }
+    }
+}
+
 #[derive(Debug, Serialize, Deserialize, Clone)]
 #[serde(rename_all = "lowercase")]
 pub struct UatStatus {
     pub account_id: Uuid,
     pub session_id: Uuid,
-    #[serde(with = "time::serde::timestamp::option")]
-    pub expiry: Option<time::OffsetDateTime>,
+    pub state: UatStatusState,
     #[serde(with = "time::serde::timestamp")]
     pub issued_at: time::OffsetDateTime,
     pub purpose: UatPurposeStatus,
@@ -358,11 +376,7 @@ impl fmt::Display for UatStatus {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         writeln!(f, "account_id: {}", self.account_id)?;
         writeln!(f, "session_id: {}", self.session_id)?;
-        if let Some(exp) = self.expiry {
-            writeln!(f, "expiry: {}", exp)?;
-        } else {
-            writeln!(f, "expiry: -")?;
-        }
+        writeln!(f, "state: {}", self.state)?;
         writeln!(f, "issued_at: {}", self.issued_at)?;
         match &self.purpose {
             UatPurposeStatus::ReadOnly => writeln!(f, "purpose: read only")?,

server/lib/src/be/dbvalue.rs

@@ -389,6 +389,16 @@ pub enum DbValueIdentityId {
     V1Sync(Uuid),
 }
 
+#[derive(Serialize, Deserialize, Debug)]
+pub enum DbValueSessionStateV1 {
+    #[serde(rename = "ea")]
+    ExpiresAt(String),
+    #[serde(rename = "nv")]
+    Never,
+    #[serde(rename = "ra")]
+    RevokedAt(DbCidV1),
+}
+
 #[derive(Serialize, Deserialize, Debug)]
 pub enum DbValueSession {
     V1 {
@@ -421,6 +431,22 @@ pub enum DbValueSession {
         #[serde(rename = "s", default)]
         scope: DbValueAccessScopeV1,
     },
+    V3 {
+        #[serde(rename = "u")]
+        refer: Uuid,
+        #[serde(rename = "l")]
+        label: String,
+        #[serde(rename = "e")]
+        state: DbValueSessionStateV1,
+        #[serde(rename = "i")]
+        issued_at: String,
+        #[serde(rename = "b")]
+        issued_by: DbValueIdentityId,
+        #[serde(rename = "c")]
+        cred_id: Uuid,
+        #[serde(rename = "s", default)]
+        scope: DbValueAccessScopeV1,
+    },
 }
 
 #[derive(Serialize, Deserialize, Debug, Default)]
@@ -466,6 +492,18 @@ pub enum DbValueOauth2Session {
         #[serde(rename = "r")]
         rs_uuid: Uuid,
     },
+    V2 {
+        #[serde(rename = "u")]
+        refer: Uuid,
+        #[serde(rename = "p")]
+        parent: Uuid,
+        #[serde(rename = "e")]
+        state: DbValueSessionStateV1,
+        #[serde(rename = "i")]
+        issued_at: String,
+        #[serde(rename = "r")]
+        rs_uuid: Uuid,
+    },
 }
 
 #[derive(Serialize, Deserialize, Debug)]

server/lib/src/constants/mod.rs

@@ -77,7 +77,8 @@ pub const MFAREG_SESSION_TIMEOUT: u64 = 300;
 pub const PW_MIN_LENGTH: usize = 10;
 
 // Default - sessions last for 1 hour.
-pub const DEFAULT_AUTH_SESSION_EXPIRY: u32 = 3600;
+pub const DEFAULT_AUTH_SESSION_EXPIRY: u32 = 86400;
+pub const DEFAULT_AUTH_SESSION_LIMITED_EXPIRY: u32 = 3600;
 // Default - privileges last for 10 minutes.
 pub const DEFAULT_AUTH_PRIVILEGE_EXPIRY: u32 = 600;
 // Default - oauth refresh tokens last for 16 hours.

server/lib/src/entry.rs

@@ -629,7 +629,7 @@ impl Entry<EntryInit, EntryNew> {
     where
         T: IntoIterator<Item = Value>,
     {
-        self.set_ava_int(attr.as_ref(), iter)
+        self.set_ava_iter_int(attr.as_ref(), iter)
     }
 
     pub fn get_ava_mut(&mut self, attr: &str) -> Option<&mut ValueSet> {
@@ -846,6 +846,7 @@ impl Entry<EntryIncremental, EntryNew> {
         &self,
         db_ent: &EntrySealedCommitted,
         _schema: &dyn SchemaTransaction,
+        trim_cid: &Cid,
     ) -> EntryIncrementalCommitted {
         use crate::repl::entry::State;
 
@@ -897,7 +898,8 @@ impl Entry<EntryIncremental, EntryNew> {
                             match (self.attrs.get(attr_name), db_ent.attrs.get(attr_name)) {
                                 (Some(vs_left), Some(vs_right)) if take_left => {
                                     #[allow(clippy::todo)]
-                                    if let Some(_attr_state) = vs_left.repl_merge_valueset(vs_right)
+                                    if let Some(_attr_state) =
+                                        vs_left.repl_merge_valueset(vs_right, trim_cid)
                                     {
                                         // TODO note: This is for special attr types that need to merge
                                         // rather than choose content.
@@ -909,7 +911,8 @@ impl Entry<EntryIncremental, EntryNew> {
                                 }
                                 (Some(vs_left), Some(vs_right)) => {
                                     #[allow(clippy::todo)]
-                                    if let Some(_attr_state) = vs_right.repl_merge_valueset(vs_left)
+                                    if let Some(_attr_state) =
+                                        vs_right.repl_merge_valueset(vs_left, trim_cid)
                                     {
                                         // TODO note: This is for special attr types that need to merge
                                         // rather than choose content.
@@ -2181,14 +2184,6 @@ impl<STATE> Entry<EntryValid, STATE> {
         Ok(())
     }
 
-    pub fn invalidate(self, cid: Cid, ecstate: EntryChangeState) -> Entry<EntryInvalid, STATE> {
-        Entry {
-            valid: EntryInvalid { cid, ecstate },
-            state: self.state,
-            attrs: self.attrs,
-        }
-    }
-
     pub fn seal(self, schema: &dyn SchemaTransaction) -> Entry<EntrySealed, STATE> {
         let EntryValid { uuid, mut ecstate } = self.valid;
 
@@ -2213,7 +2208,12 @@ impl<STATE> Entry<EntrySealed, STATE>
 where
     STATE: Clone,
 {
-    pub fn invalidate(self, cid: Cid) -> Entry<EntryInvalid, STATE> {
+    pub fn invalidate(mut self, cid: Cid, trim_cid: &Cid) -> Entry<EntryInvalid, STATE> {
+        // Trim attributes that require it. For most this is a no-op.
+        for vs in self.attrs.values_mut() {
+            vs.trim(trim_cid);
+        }
+
         Entry {
             valid: EntryInvalid {
                 cid,
@@ -2390,24 +2390,21 @@ impl<VALID, STATE> Entry<VALID, STATE> {
     }
 
     /// Overwrite the current set of values for an attribute, with this new set.
-    fn set_ava_int<T>(&mut self, attr: &str, iter: T)
+    fn set_ava_iter_int<T>(&mut self, attr: &str, iter: T)
     where
         T: IntoIterator<Item = Value>,
     {
-        // Overwrite the existing value, build a tree from the list.
-        let values = valueset::from_value_iter(iter.into_iter());
-        match values {
-            Ok(vs) => {
-                let _ = self.attrs.insert(AttrString::from(attr), vs);
-            }
-            Err(e) => {
-                admin_warn!(
-                    "dropping content of {} due to invalid valueset {:?}",
-                    attr,
-                    e
-                );
-                self.attrs.remove(attr);
-            }
+        let Ok(vs) = valueset::from_value_iter(iter.into_iter()) else {
+            trace!("set_ava_iter_int - empty from_value_iter, skipping");
+            return;
+        };
+
+        if let Some(existing_vs) = self.attrs.get_mut(attr) {
+            // This is the suboptimal path. This can only exist in rare cases.
+            let _ = existing_vs.merge(&vs);
+        } else {
+            // Normally this is what's taken.
+            self.attrs.insert(AttrString::from(attr), vs);
         }
     }
 
@@ -2979,11 +2976,7 @@ where
 
     fn assert_ava(&mut self, attr: &str, value: &PartialValue) -> Result<(), OperationError> {
         self.valid.ecstate.change_ava(&self.valid.cid, attr);
-        /*
-        self.valid
-            .eclog
-            .assert_ava(&self.valid.cid, attr, value.clone());
-        */
+
         if self.attribute_equality(attr, value) {
             Ok(())
         } else {
@@ -2995,14 +2988,9 @@ where
     /// don't do anything else since we are asserting the absence of a value.
     pub(crate) fn remove_ava(&mut self, attr: &str, value: &PartialValue) {
         self.valid.ecstate.change_ava(&self.valid.cid, attr);
-        /*
-        self.valid
-            .eclog
-            .remove_ava_iter(&self.valid.cid, attr, std::iter::once(value.clone()));
-        */
 
         let rm = if let Some(vs) = self.attrs.get_mut(attr) {
-            vs.remove(value);
+            vs.remove(value, &self.valid.cid);
             vs.is_empty()
         } else {
             false
@@ -3014,15 +3002,10 @@ where
 
     pub(crate) fn remove_avas(&mut self, attr: &str, values: &BTreeSet<PartialValue>) {
         self.valid.ecstate.change_ava(&self.valid.cid, attr);
-        /*
-        self.valid
-            .eclog
-            .remove_ava_iter(&self.valid.cid, attr, values.iter().cloned());
-        */
 
         let rm = if let Some(vs) = self.attrs.get_mut(attr) {
             values.iter().for_each(|k| {
-                vs.remove(k);
+                vs.remove(k, &self.valid.cid);
             });
             vs.is_empty()
         } else {
@@ -3037,42 +3020,66 @@ where
     /// asserts that no content of that attribute exist.
     pub(crate) fn purge_ava(&mut self, attr: &str) {
         self.valid.ecstate.change_ava(&self.valid.cid, attr);
-        // self.valid.eclog.purge_ava(&self.valid.cid, attr);
-        self.attrs.remove(attr);
+        let can_remove = self
+            .attrs
+            .get_mut(attr)
+            .map(|vs| vs.purge(&self.valid.cid))
+            // Default to false since a missing attr can't be removed!
+            .unwrap_or_default();
+        if can_remove {
+            self.attrs.remove(attr);
+        }
     }
 
-    /// Remove all values of this attribute from the entry, and return their content.
+    /// Remove this value set from the entry, returning the value set at the time of removal.
     pub fn pop_ava(&mut self, attr: &str) -> Option<ValueSet> {
         self.valid.ecstate.change_ava(&self.valid.cid, attr);
-        // self.valid.eclog.purge_ava(&self.valid.cid, attr);
+
+        let mut vs = self.attrs.remove(attr)?;
+        if vs.purge(&self.valid.cid) {
+            // Can return as is.
+            Some(vs)
+        } else {
+            // This type may need special handling. Clone and reinsert.
+            let r_vs = vs.clone();
+            self.attrs.insert(attr.into(), vs);
+            Some(r_vs)
+        }
+    }
+
+    /// Unlike pop or purge, this does NOT respect the attributes purge settings, meaning
+    /// that this can break replication by force clearing the state of an attribute. It's
+    /// useful for things like "session" to test the grace window by removing the revoked
+    /// sessions from the value set that you otherwise, could not.
+    #[cfg(test)]
+    pub(crate) fn force_trim_ava(&mut self, attr: &str) -> Option<ValueSet> {
+        self.valid.ecstate.change_ava(&self.valid.cid, attr);
         self.attrs.remove(attr)
     }
 
-    /// Replace the content of this attribute with a new value set. Effectively this is
-    /// a a "purge and set".
+    /// Replace the content of this attribute with the values from this
+    /// iterator. If the iterator is empty, the attribute is purged.
     pub fn set_ava<T>(&mut self, attr: &str, iter: T)
     where
         T: Clone + IntoIterator<Item = Value>,
     {
-        self.valid.ecstate.change_ava(&self.valid.cid, attr);
-        /*
-        self.valid.eclog.purge_ava(&self.valid.cid, attr);
-        self.valid
-            .eclog
-            .add_ava_iter(&self.valid.cid, attr, iter.clone());
-        */
-        self.set_ava_int(attr, iter)
+        // self.valid.ecstate.change_ava(&self.valid.cid, attr);
+        // purge_ava triggers ecstate for us.
+        self.purge_ava(attr);
+        self.set_ava_iter_int(attr, iter)
     }
 
+    /// Replace the content of this attribute with a new value set. Effectively this is
+    /// a a "purge and set".
     pub fn set_ava_set(&mut self, attr: &str, vs: ValueSet) {
-        self.valid.ecstate.change_ava(&self.valid.cid, attr);
-        /*
-        self.valid.eclog.purge_ava(&self.valid.cid, attr);
-        self.valid
-            .eclog
-            .add_ava_iter(&self.valid.cid, attr, vs.to_value_iter());
-        */
-        self.attrs.insert(AttrString::from(attr), vs);
+        // self.valid.ecstate.change_ava(&self.valid.cid, attr);
+        // purge_ava triggers ecstate for us.
+        self.purge_ava(attr);
+        if let Some(existing_vs) = self.attrs.get_mut(attr) {
+            let _ = existing_vs.merge(&vs);
+        } else {
+            self.attrs.insert(AttrString::from(attr), vs);
+        }
     }
 
     /// Apply the content of this modlist to this entry, enforcing the expressed state.

server/lib/src/idm/account.rs

@@ -2,7 +2,8 @@ use std::collections::{BTreeMap, BTreeSet};
 use std::time::Duration;
 
 use kanidm_proto::v1::{
-    BackupCodesView, CredentialStatus, OperationError, UatPurpose, UatStatus, UiHint, UserAuthToken,
+    BackupCodesView, CredentialStatus, OperationError, UatPurpose, UatStatus, UatStatusState,
+    UiHint, UserAuthToken,
 };
 use time::OffsetDateTime;
 use uuid::Uuid;
@@ -20,7 +21,7 @@ use crate::idm::server::{IdmServerProxyReadTransaction, IdmServerProxyWriteTrans
 use crate::modify::{ModifyInvalid, ModifyList};
 use crate::prelude::*;
 use crate::schema::SchemaTransaction;
-use crate::value::{IntentTokenState, PartialValue, Value};
+use crate::value::{IntentTokenState, PartialValue, SessionState, Value};
 use kanidm_lib_crypto::CryptoPolicy;
 
 macro_rules! try_from_entry {
@@ -229,8 +230,15 @@ impl Account {
         // ns value which breaks some checks.
         let ct = ct - Duration::from_nanos(ct.subsec_nanos() as u64);
         let issued_at = OffsetDateTime::UNIX_EPOCH + ct;
+        // Note that currently the auth_session time comes from policy, but the already-privileged
+        // session bound is hardcoded.
         let expiry =
             Some(OffsetDateTime::UNIX_EPOCH + ct + Duration::from_secs(auth_session_expiry as u64));
+        let limited_expiry = Some(
+            OffsetDateTime::UNIX_EPOCH
+                + ct
+                + Duration::from_secs(DEFAULT_AUTH_SESSION_LIMITED_EXPIRY as u64),
+        );
 
         let (purpose, expiry) = match scope {
             // Issue an invalid/expired session.
@@ -243,18 +251,9 @@ impl Account {
             SessionScope::ReadOnly => (UatPurpose::ReadOnly, expiry),
             SessionScope::ReadWrite => {
                 // These sessions are always rw, and so have limited life.
-                (UatPurpose::ReadWrite { expiry }, expiry)
-            }
-            SessionScope::PrivilegeCapable =>
-            // Return a rw capable session with the expiry currently invalid.
-            // These sessions COULD live forever since they can re-auth properly.
-            // Today I'm setting this to 24hr though.
-            {
-                (
-                    UatPurpose::ReadWrite { expiry: None },
-                    Some(OffsetDateTime::UNIX_EPOCH + ct + Duration::from_secs(86400)),
-                )
+                (UatPurpose::ReadWrite { expiry }, limited_expiry)
             }
+            SessionScope::PrivilegeCapable => (UatPurpose::ReadWrite { expiry: None }, expiry),
         };
 
         Some(UserAuthToken {
@@ -571,15 +570,28 @@ impl Account {
                 .get_ava_as_session_map("user_auth_token_session")
                 .and_then(|session_map| session_map.get(&uat.session_id));
 
+            // Important - we don't have to check the expiry time against ct here since it was
+            // already checked in token_to_token. Here we just need to check it's consistent
+            // to our internal session knowledge.
             if let Some(session) = session_present {
-                security_info!("A valid session value exists for this token");
-
-                if session.expiry == uat.expiry {
-                    true
-                } else {
-                    security_info!("Session and uat expiry are not consistent, rejecting.");
-                    debug!(ses_exp = ?session.expiry, uat_exp = ?uat.expiry);
-                    false
+                match (&session.state, &uat.expiry) {
+                    (SessionState::ExpiresAt(s_exp), Some(u_exp)) if s_exp == u_exp => {
+                        security_info!("A valid limited session value exists for this token");
+                        true
+                    }
+                    (SessionState::NeverExpires, None) => {
+                        security_info!("A valid unbound session value exists for this token");
+                        true
+                    }
+                    (SessionState::RevokedAt(_), _) => {
+                        security_info!("Session has been revoked");
+                        false
+                    }
+                    _ => {
+                        security_info!("Session and uat expiry are not consistent, rejecting.");
+                        debug!(ses_st = ?session.state, uat_exp = ?uat.expiry);
+                        false
+                    }
                 }
             } else {
                 let grace = uat.issued_at + GRACE_WINDOW;
@@ -780,12 +792,22 @@ impl<'a> IdmServerProxyReadTransaction<'a> {
                             .map(|smap| {
                                 smap.iter()
                                     .map(|(u, s)| {
+                                        let state = match s.state {
+                                            SessionState::ExpiresAt(odt) => {
+                                                UatStatusState::ExpiresAt(odt)
+                                            }
+                                            SessionState::NeverExpires => {
+                                                UatStatusState::NeverExpires
+                                            }
+                                            SessionState::RevokedAt(_) => UatStatusState::Revoked,
+                                        };
+
                                         s.scope
                                             .try_into()
                                             .map(|purpose| UatStatus {
                                                 account_id,
                                                 session_id: *u,
-                                                expiry: s.expiry,
+                                                state,
                                                 issued_at: s.issued_at,
                                                 purpose,
                                             })

server/lib/src/idm/authsession.rs

@@ -33,7 +33,7 @@ use crate::idm::delayed::{
 };
 use crate::idm::AuthState;
 use crate::prelude::*;
-use crate::value::Session;
+use crate::value::{Session, SessionState};
 use time::OffsetDateTime;
 
 use super::server::AccountPolicy;
@@ -893,6 +893,17 @@ impl AuthSession {
             State::Expired
         };
 
+        let session_expiry = match session.state {
+            SessionState::ExpiresAt(odt) => Some(odt),
+            SessionState::NeverExpires => None,
+            SessionState::RevokedAt(_) => {
+                security_error!(
+                    "Invalid State - Should not be possible to trigger re-auth on revoked session."
+                );
+                return (None, AuthState::Denied(ACCOUNT_EXPIRED.to_string()));
+            }
+        };
+
         match state {
             State::Proceed(handler) => {
                 let allow = handler.next_auth_allowed();
@@ -902,7 +913,7 @@ impl AuthSession {
                     issue,
                     intent: AuthIntent::Reauth {
                         session_id,
-                        session_expiry: session.expiry,
+                        session_expiry,
                     },
                     source,
                 };

server/lib/src/idm/oauth2.rs

@@ -40,7 +40,7 @@ use crate::idm::server::{
     IdmServerProxyReadTransaction, IdmServerProxyWriteTransaction, IdmServerTransaction,
 };
 use crate::prelude::*;
-use crate::value::{Oauth2Session, OAUTHSCOPE_RE};
+use crate::value::{Oauth2Session, SessionState, OAUTHSCOPE_RE};
 
 #[derive(Serialize, Deserialize, Debug, PartialEq)]
 #[serde(rename_all = "snake_case")]
@@ -484,6 +484,7 @@ impl<'a> Oauth2ResourceServersWriteTransaction<'a> {
 }
 
 impl<'a> IdmServerProxyWriteTransaction<'a> {
+    #[instrument(level = "debug", skip_all)]
     pub fn oauth2_token_revoke(
         &mut self,
         client_authz: &str,
@@ -575,6 +576,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
         }
     }
 
+    #[instrument(level = "debug", skip_all)]
     pub fn check_oauth2_token_exchange(
         &mut self,
         client_authz: Option<&str>,
@@ -659,6 +661,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
         }
     }
 
+    #[instrument(level = "debug", skip_all)]
     pub fn check_oauth2_authorise_permit(
         &mut self,
         ident: &Identity,
@@ -750,6 +753,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
         })
     }
 
+    #[instrument(level = "debug", skip_all)]
     fn check_oauth2_token_exchange_authorization_code(
         &mut self,
         o2rs: &Oauth2RS,
@@ -849,6 +853,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
         )
     }
 
+    #[instrument(level = "debug", skip_all)]
     fn check_oauth2_token_refresh(
         &mut self,
         o2rs: &Oauth2RS,
@@ -1131,7 +1136,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
             session_id,
             Oauth2Session {
                 parent: parent_session_id,
-                expiry: Some(refresh_expiry),
+                state: SessionState::ExpiresAt(refresh_expiry),
                 issued_at: odt_ct,
                 rs_uuid: o2rs.uuid,
             },
@@ -1139,7 +1144,9 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
 
         // We need to update (replace) this session id if present.
         let modlist = ModifyList::new_list(vec![
-            Modify::Removed("oauth2_session".into(), PartialValue::Refer(session_id)),
+            // NOTE: Oauth2_session has special handling that allows update in place without
+            // the remove step needing to be carried out.
+            // Modify::Removed("oauth2_session".into(), PartialValue::Refer(session_id)),
             Modify::Present("oauth2_session".into(), session),
         ]);
 
@@ -1208,6 +1215,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
 }
 
 impl<'a> IdmServerProxyReadTransaction<'a> {
+    #[instrument(level = "debug", skip_all)]
     pub fn check_oauth2_authorisation(
         &self,
         ident: &Identity,
@@ -1499,6 +1507,7 @@ impl<'a> IdmServerProxyReadTransaction<'a> {
         }
     }
 
+    #[instrument(level = "debug", skip_all)]
     pub fn check_oauth2_authorise_reject(
         &self,
         ident: &Identity,
@@ -1550,6 +1559,7 @@ impl<'a> IdmServerProxyReadTransaction<'a> {
         Ok(consent_req.redirect_uri)
     }
 
+    #[instrument(level = "debug", skip_all)]
     pub fn check_oauth2_token_introspect(
         &mut self,
         client_authz: &str,
@@ -1659,6 +1669,7 @@ impl<'a> IdmServerProxyReadTransaction<'a> {
         }
     }
 
+    #[instrument(level = "debug", skip_all)]
     pub fn oauth2_openid_userinfo(
         &mut self,
         client_id: &str,
@@ -1765,6 +1776,7 @@ impl<'a> IdmServerProxyReadTransaction<'a> {
         }
     }
 
+    #[instrument(level = "debug", skip_all)]
     pub fn oauth2_openid_discovery(
         &self,
         client_id: &str,
@@ -1848,6 +1860,7 @@ impl<'a> IdmServerProxyReadTransaction<'a> {
         })
     }
 
+    #[instrument(level = "debug", skip_all)]
     pub fn oauth2_openid_publickey(&self, client_id: &str) -> Result<JwkKeySet, OperationError> {
         let o2rs = self.oauth2rs.inner.rs_set.get(client_id).ok_or_else(|| {
             admin_warn!(
@@ -1974,6 +1987,7 @@ mod tests {
     use crate::idm::oauth2::{AuthoriseResponse, Oauth2Error};
     use crate::idm::server::{IdmServer, IdmServerTransaction};
     use crate::prelude::*;
+    use crate::value::SessionState;
 
     use crate::credential::Credential;
     use kanidm_lib_crypto::CryptoPolicy;
@@ -2127,7 +2141,10 @@ mod tests {
             .expect("Unable to create uat");
 
         // Need the uat first for expiry.
-        let expiry = uat.expiry;
+        let state = uat
+            .expiry
+            .map(SessionState::ExpiresAt)
+            .unwrap_or(SessionState::NeverExpires);
 
         let p = CryptoPolicy::minimum();
         let cred = Credential::new_password_only(&p, "test_password").unwrap();
@@ -2137,7 +2154,7 @@ mod tests {
             session_id,
             crate::value::Session {
                 label: "label".to_string(),
-                expiry,
+                state,
                 issued_at: time::OffsetDateTime::UNIX_EPOCH + ct,
                 issued_by: IdentityId::Internal,
                 cred_id,
@@ -2246,7 +2263,10 @@ mod tests {
             .expect("Unable to create uat");
 
         // Need the uat first for expiry.
-        let expiry = uat.expiry;
+        let state = uat
+            .expiry
+            .map(SessionState::ExpiresAt)
+            .unwrap_or(SessionState::NeverExpires);
 
         let p = CryptoPolicy::minimum();
         let cred = Credential::new_password_only(&p, "test_password").unwrap();
@@ -2256,7 +2276,7 @@ mod tests {
             session_id,
             crate::value::Session {
                 label: "label".to_string(),
-                expiry,
+                state,
                 issued_at: time::OffsetDateTime::UNIX_EPOCH + ct,
                 issued_by: IdentityId::Internal,
                 cred_id,
@@ -3083,18 +3103,6 @@ mod tests {
             .is_ok());
         assert!(idms_prox_write.commit().is_ok());
 
-        // Check it is still valid - this is because we are still in the GRACE window.
-        let mut idms_prox_read = idms.proxy_read().await;
-        let intr_response = idms_prox_read
-            .check_oauth2_token_introspect(client_authz.as_deref().unwrap(), &intr_request, ct)
-            .expect("Failed to inspect token");
-
-        assert!(intr_response.active);
-        drop(idms_prox_read);
-
-        // Check after the grace window, it will be invalid.
-        let ct = ct + GRACE_WINDOW;
-
         // Assert it is now invalid.
         let mut idms_prox_read = idms.proxy_read().await;
         let intr_response = idms_prox_read
@@ -3104,6 +3112,39 @@ mod tests {
         assert!(!intr_response.active);
         drop(idms_prox_read);
 
+        // Force trim the session and wait for the grace window to pass. The token will be invalidated
+        let mut idms_prox_write = idms.proxy_write(ct).await;
+        let filt = filter!(f_eq(Attribute::Uuid, PartialValue::Uuid(uat.uuid)));
+        let mut work_set = idms_prox_write
+            .qs_write
+            .internal_search_writeable(&filt)
+            .expect("Failed to perform internal search writeable");
+        for (_, entry) in work_set.iter_mut() {
+            let _ = entry.force_trim_ava(Attribute::OAuth2Session.into());
+        }
+        assert!(idms_prox_write
+            .qs_write
+            .internal_apply_writable(work_set)
+            .is_ok());
+
+        assert!(idms_prox_write.commit().is_ok());
+
+        let mut idms_prox_read = idms.proxy_read().await;
+        // Grace window in effect.
+        let intr_response = idms_prox_read
+            .check_oauth2_token_introspect(client_authz.as_deref().unwrap(), &intr_request, ct)
+            .expect("Failed to inspect token");
+        assert!(intr_response.active);
+
+        // Grace window passed, it will now be invalid.
+        let ct = ct + GRACE_WINDOW;
+        let intr_response = idms_prox_read
+            .check_oauth2_token_introspect(client_authz.as_deref().unwrap(), &intr_request, ct)
+            .expect("Failed to inspect token");
+        assert!(!intr_response.active);
+
+        drop(idms_prox_read);
+
         // A second invalidation of the token "does nothing".
         let mut idms_prox_write = idms.proxy_write(ct).await;
         let revoke_request = TokenRevokeRequest {
@@ -3201,17 +3242,19 @@ mod tests {
 
         assert!(idms_prox_write.qs_write.delete(&de).is_ok());
 
-        // Assert the session is gone. This is cleaned up as an artifact of the referential
-        // integrity plugin.
+        // Assert the session is revoked. This is cleaned up as an artifact of the referential
+        // integrity plugin. Remember, refint doesn't consider revoked sessions once they are
+        // revoked.
         let entry = idms_prox_write
             .qs_write
             .internal_search_uuid(UUID_ADMIN)
             .expect("failed");
-        let valid = entry
+        let revoked = entry
             .get_ava_as_oauth2session_map("oauth2_session")
-            .map(|map| map.get(&session_id).is_some())
+            .and_then(|sessions| sessions.get(&session_id))
+            .map(|session| matches!(session.state, SessionState::RevokedAt(_)))
             .unwrap_or(false);
-        assert!(!valid);
+        assert!(revoked);
 
         assert!(idms_prox_write.commit().is_ok());
     }
@@ -4763,13 +4806,12 @@ mod tests {
             .expect("failed");
         let valid = entry
             .get_ava_as_oauth2session_map("oauth2_session")
-            .map(|map| {
-                trace!(?map);
-                map.is_empty()
-            })
-            // If there is no map, it must be empty
-            .unwrap_or(true);
-        assert!(valid);
+            .and_then(|sessions| sessions.first_key_value())
+            .map(|(_, session)| !matches!(session.state, SessionState::RevokedAt(_)))
+            // If there is no map, then something is wrong.
+            .unwrap();
+        // The session should be invalid at this point.
+        assert!(!valid);
 
         assert!(idms_prox_write.commit().is_ok());
     }
@@ -4840,8 +4882,10 @@ mod tests {
 
         // Success!
     }
-    #[test] // I know this looks kinda dumb but at some point someone pointed out that our scope syntax wasn't compliant with rfc6749
-            //(https://datatracker.ietf.org/doc/html/rfc6749#section-3.3), so I'm just making sure that we don't break it again.
+
+    #[test]
+    // I know this looks kinda dumb but at some point someone pointed out that our scope syntax wasn't compliant with rfc6749
+    //(https://datatracker.ietf.org/doc/html/rfc6749#section-3.3), so I'm just making sure that we don't break it again.
     fn compliant_serialization_test() {
         let token_req: Result<AccessTokenRequest, serde_json::Error> = serde_json::from_str(
             r#"

server/lib/src/idm/server.rs

@@ -56,7 +56,7 @@ use crate::idm::unix::{UnixGroup, UnixUserAccount};
 use crate::idm::AuthState;
 use crate::prelude::*;
 use crate::utils::{password_from_random, readable_password_from_random, uuid_from_duration, Sid};
-use crate::value::Session;
+use crate::value::{Session, SessionState};
 
 pub(crate) type AuthSessionMutex = Arc<Mutex<AuthSession>>;
 pub(crate) type CredSoftLockMutex = Arc<Mutex<CredSoftLock>>;
@@ -536,10 +536,12 @@ pub trait IdmServerTransaction<'a> {
                 .map(|t: Jws<UserAuthToken>| t.into_inner())?;
 
             if let Some(exp) = uat.expiry {
-                if time::OffsetDateTime::UNIX_EPOCH + ct >= exp {
-                    security_info!("Session expired");
+                let ct_odt = time::OffsetDateTime::UNIX_EPOCH + ct;
+                if ct_odt >= exp {
+                    security_info!(?ct_odt, ?exp, "Session expired");
                     Err(OperationError::SessionExpired)
                 } else {
+                    trace!(?ct_odt, ?exp, "Session not yet expired");
                     Ok(Token::UserAuthToken(uat))
                 }
             } else {
@@ -667,28 +669,54 @@ pub trait IdmServerTransaction<'a> {
             return Ok(None);
         }
 
-        if ct >= Duration::from_secs(iat as u64) + GRACE_WINDOW {
-            // We are past the grace window. Enforce session presence.
-            // We enforce both sessions are present in case of inconsistency
-            // that may occur with replication.
-            let oauth2_session_valid = entry
-                .get_ava_as_oauth2session_map(Attribute::OAuth2Session.as_ref())
-                .map(|map| map.get(&session_id).is_some())
-                .unwrap_or(false);
-            let uat_session_valid = entry
-                .get_ava_as_session_map(Attribute::UserAuthTokenSession.as_ref())
-                .map(|map| map.get(&parent_session_id).is_some())
-                .unwrap_or(false);
+        // We are past the grace window. Enforce session presence.
+        // We enforce both sessions are present in case of inconsistency
+        // that may occur with replication.
 
-            if oauth2_session_valid && uat_session_valid {
-                security_info!("A valid session value exists for this token");
-            } else {
-                security_info!(%uat_session_valid, %oauth2_session_valid, "The token grace window has passed and no sessions exist. Assuming invalid.");
+        let grace_valid = ct < (Duration::from_secs(iat as u64) + GRACE_WINDOW);
+
+        let oauth2_session = entry
+            .get_ava_as_oauth2session_map(Attribute::OAuth2Session.as_ref())
+            .and_then(|sessions| sessions.get(&session_id));
+        let uat_session = entry
+            .get_ava_as_session_map(Attribute::UserAuthTokenSession.as_ref())
+            .and_then(|sessions| sessions.get(&parent_session_id));
+
+        if let Some(oauth2_session) = oauth2_session {
+            // We have the oauth2 session, lets check it.
+            let oauth2_session_valid = !matches!(oauth2_session.state, SessionState::RevokedAt(_));
+
+            if !oauth2_session_valid {
+                security_info!("The oauth2 session associated to this token is revoked.");
                 return Ok(None);
             }
-        } else {
+
+            if let Some(uat_session) = uat_session {
+                let parent_session_valid = !matches!(uat_session.state, SessionState::RevokedAt(_));
+                if parent_session_valid {
+                    security_info!("A valid parent and oauth2 session value exists for this token");
+                } else {
+                    security_info!(
+                        "The parent oauth2 session associated to this token is revoked."
+                    );
+                    return Ok(None);
+                }
+            } else if grace_valid {
+                security_info!(
+                    "The token grace window is in effect. Assuming parent session valid."
+                );
+            } else {
+                security_info!("The token grace window has passed and no entry parent sessions exist. Assuming invalid.");
+                return Ok(None);
+            }
+        } else if grace_valid {
             security_info!("The token grace window is in effect. Assuming valid.");
-        };
+        } else {
+            security_info!(
+                "The token grace window has passed and no entry sessions exist. Assuming invalid."
+            );
+            return Ok(None);
+        }
 
         Ok(Some(entry))
     }
@@ -1792,6 +1820,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
         Ok(())
     }
 
+    #[instrument(level = "debug", skip_all)]
     pub fn recover_account(
         &mut self,
         name: &str,
@@ -1836,6 +1865,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
         Ok(cleartext)
     }
 
+    #[instrument(level = "debug", skip_all)]
     pub fn regenerate_radius_secret(
         &mut self,
         rrse: &RegenerateRadiusSecretEvent,
@@ -1874,6 +1904,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
     }
 
     // -- delayed action processing --
+    #[instrument(level = "debug", skip_all)]
     fn process_pwupgrade(&mut self, pwu: &PasswordUpgrade) -> Result<(), OperationError> {
         // get the account
         let account = self.target_to_account(pwu.target_uuid)?;
@@ -1898,6 +1929,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
         }
     }
 
+    #[instrument(level = "debug", skip_all)]
     fn process_unixpwupgrade(&mut self, pwu: &UnixPasswordUpgrade) -> Result<(), OperationError> {
         info!(session_id = %pwu.target_uuid, "Processing unix password hash upgrade");
 
@@ -1926,6 +1958,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
         }
     }
 
+    #[instrument(level = "debug", skip_all)]
     pub(crate) fn process_webauthncounterinc(
         &mut self,
         wci: &WebauthnCounterIncrement,
@@ -1954,6 +1987,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
         }
     }
 
+    #[instrument(level = "debug", skip_all)]
     pub(crate) fn process_backupcoderemoval(
         &mut self,
         bcr: &BackupCodeRemoval,
@@ -1975,17 +2009,22 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
         )
     }
 
+    #[instrument(level = "debug", skip_all)]
     pub(crate) fn process_authsessionrecord(
         &mut self,
         asr: &AuthSessionRecord,
     ) -> Result<(), OperationError> {
         // We have to get the entry so we can work out if we need to expire any of it's sessions.
+        let state = match asr.expiry {
+            Some(e) => SessionState::ExpiresAt(e),
+            None => SessionState::NeverExpires,
+        };
 
         let session = Value::Session(
             asr.session_id,
             Session {
                 label: asr.label.clone(),
-                expiry: asr.expiry,
+                state,
                 // Need the other inner bits?
                 // for the gracewindow.
                 issued_at: asr.issued_at,
@@ -2016,6 +2055,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
         // Done!
     }
 
+    #[instrument(level = "debug", skip_all)]
     pub fn process_delayedaction(
         &mut self,
         da: DelayedAction,
@@ -2135,6 +2175,7 @@ mod tests {
     use crate::modify::{Modify, ModifyList};
     use crate::prelude::*;
     use crate::utils::duration_from_epoch_now;
+    use crate::value::SessionState;
     use kanidm_lib_crypto::CryptoPolicy;
 
     const TEST_PASSWORD: &str = "ntaoeuntnaoeuhraohuercahu😍";
@@ -3467,13 +3508,13 @@ mod tests {
         let da = idms_delayed.try_recv().expect("invalid");
         assert!(matches!(da, DelayedAction::AuthSessionRecord(_)));
         // Persist it.
-        let r = idms.delayed_action(duration_from_epoch_now(), da).await;
+        let r = idms.delayed_action(ct, da).await;
         assert!(Ok(true) == r);
         idms_delayed.check_is_empty_or_panic();
 
         let mut idms_prox_read = idms.proxy_read().await;
 
-        // Check it's valid.
+        // Check it's valid - This is within the time window so will pass.
         idms_prox_read
             .validate_and_parse_token_to_ident(Some(token.as_str()), ct)
             .expect("Failed to validate");
@@ -3536,16 +3577,12 @@ mod tests {
             .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);
+        let session_data_a = sessions.get(&session_a).expect("Session A is missing!");
+        assert!(matches!(session_data_a.state, SessionState::ExpiresAt(_)));
 
         drop(idms_prox_read);
 
-        // When we re-auth, this is what triggers the session cleanup via the delayed action.
+        // When we re-auth, this is what triggers the session revoke via the delayed action.
 
         let da = DelayedAction::AuthSessionRecord(AuthSessionRecord {
             target_uuid: UUID_ADMIN,
@@ -3570,15 +3607,14 @@ mod tests {
             .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!(sessions.len() == 2);
 
-        assert!(session_id_a != session_id_b);
+        let session_data_a = sessions.get(&session_a).expect("Session A is missing!");
+        assert!(matches!(session_data_a.state, SessionState::RevokedAt(_)));
+
+        let session_data_b = sessions.get(&session_b).expect("Session B is missing!");
+        assert!(matches!(session_data_b.state, SessionState::ExpiresAt(_)));
+        // Now show that sessions trim!
     }
 
     #[idm_test]
@@ -3640,7 +3676,32 @@ mod tests {
         // Now check again with the session destroyed.
         let mut idms_prox_read = idms.proxy_read().await;
 
-        // Now, within gracewindow, it's still valid.
+        // Now, within gracewindow, it's NOT valid because the session entry exists and is in
+        // the revoked state!
+        match idms_prox_read.validate_and_parse_token_to_ident(Some(token.as_str()), post_grace) {
+            Err(OperationError::SessionExpired) => {}
+            _ => assert!(false),
+        }
+        drop(idms_prox_read);
+
+        // Force trim the session out so that we can check the grate handling.
+        let mut idms_prox_write = idms.proxy_write(ct).await;
+        let filt = filter!(f_eq(Attribute::Uuid, PartialValue::Uuid(uat_inner.uuid)));
+        let mut work_set = idms_prox_write
+            .qs_write
+            .internal_search_writeable(&filt)
+            .expect("Failed to perform internal search writeable");
+        for (_, entry) in work_set.iter_mut() {
+            let _ = entry.force_trim_ava(Attribute::UserAuthTokenSession.into());
+        }
+        assert!(idms_prox_write
+            .qs_write
+            .internal_apply_writable(work_set)
+            .is_ok());
+
+        assert!(idms_prox_write.commit().is_ok());
+
+        let mut idms_prox_read = idms.proxy_read().await;
         idms_prox_read
             .validate_and_parse_token_to_ident(Some(token.as_str()), ct)
             .expect("Failed to validate");

server/lib/src/lib.rs

@@ -92,6 +92,7 @@ pub mod prelude {
     pub use crate::modify::{
         m_assert, m_pres, m_purge, m_remove, Modify, ModifyInvalid, ModifyList, ModifyValid,
     };
+    pub use crate::repl::cid::Cid;
     pub use crate::server::access::AccessControlsTransaction;
     pub use crate::server::batch_modify::BatchModifyEvent;
     pub use crate::server::identity::{

server/lib/src/macros.rs

@@ -486,9 +486,11 @@ macro_rules! mergemaps {
         $b:expr
     ) => {{
         $b.iter().for_each(|(k, v)| {
-            if !$a.contains_key(k) {
-                $a.insert(k.clone(), v.clone());
-            }
+            // I think to be consistent, we need the content of b to always
+            // the content of a
+            // if !$a.contains_key(k) {
+            $a.insert(k.clone(), v.clone());
+            // }
         });
         Ok(())
     }};

server/lib/src/plugins/namehistory.rs

@@ -128,6 +128,7 @@ mod tests {
     use crate::prelude::{uuid, EntryClass};
     use crate::repl::cid::Cid;
     use crate::value::Value;
+    use crate::valueset::AUDIT_LOG_STRING_CAPACITY;
 
     #[test]
     fn name_purge_and_set() {
@@ -216,10 +217,10 @@ mod tests {
     #[test]
     fn name_purge_and_set_with_filled_history() {
         let mut cids: Vec<Cid> = Vec::new();
-        for i in 1..8 {
+        for i in 1..AUDIT_LOG_STRING_CAPACITY {
             cids.push(Cid::new(
                 uuid!("d2b496bd-8493-47b7-8142-f568b5cf47e1"),
-                Duration::new(20 + i, 0),
+                Duration::new(20 + i as u64, 0),
             ))
         }
         // Add another uuid to a type
@@ -257,10 +258,10 @@ mod tests {
                 let e = qs
                     .internal_search_uuid(uuid!("d2b496bd-8493-47b7-8142-f568b5cf47ee"))
                     .expect("failed to get entry");
-                trace!("{:?}", e.get_ava());
                 let c = e
                     .get_ava_set(Attribute::NameHistory.as_ref())
                     .expect("failed to get name_history ava :/");
+                trace!(?c);
                 assert!(
                     !c.contains(&PartialValue::new_utf8s("old_name1"))
                         && c.contains(&PartialValue::new_utf8s("new_name"))

server/lib/src/plugins/refint.rs

@@ -435,7 +435,7 @@ mod tests {
 
     use crate::event::CreateEvent;
     use crate::prelude::*;
-    use crate::value::{Oauth2Session, Session};
+    use crate::value::{Oauth2Session, Session, SessionState};
     use time::OffsetDateTime;
     use uuid::uuid;
 
@@ -1051,8 +1051,8 @@ mod tests {
         let session_id = Uuid::new_v4();
         let pv_session_id = PartialValue::Refer(session_id);
 
-        let parent = Uuid::new_v4();
-        let pv_parent_id = PartialValue::Refer(parent);
+        let parent_id = Uuid::new_v4();
+        let pv_parent_id = PartialValue::Refer(parent_id);
         let issued_at = curtime_odt;
         let issued_by = IdentityId::User(tuuid);
         let scope = SessionScope::ReadOnly;
@@ -1064,9 +1064,9 @@ mod tests {
                 Value::Oauth2Session(
                     session_id,
                     Oauth2Session {
-                        parent,
+                        parent: parent_id,
                         // Note we set the exp to None so we are not removing based on exp
-                        expiry: None,
+                        state: SessionState::NeverExpires,
                         issued_at,
                         rs_uuid,
                     },
@@ -1075,11 +1075,11 @@ mod tests {
             Modify::Present(
                 Attribute::UserAuthTokenSession.into(),
                 Value::Session(
-                    parent,
+                    parent_id,
                     Session {
                         label: "label".to_string(),
                         // Note we set the exp to None so we are not removing based on removal of the parent.
-                        expiry: None,
+                        state: SessionState::NeverExpires,
                         // Need the other inner bits?
                         // for the gracewindow.
                         issued_at,
@@ -1114,11 +1114,18 @@ mod tests {
         let entry = server_txn.internal_search_uuid(tuuid).expect("failed");
 
         // Note the uat is present still.
-        assert!(entry.attribute_equality(Attribute::UserAuthTokenSession.as_ref(), &pv_parent_id));
-        // The oauth2 session is removed.
-        dbg!(&entry);
-        dbg!(&pv_session_id);
-        assert!(!entry.attribute_equality(Attribute::OAuth2Session.as_ref(), &pv_session_id));
+        let session = entry
+            .get_ava_as_session_map(Attribute::UserAuthTokenSession.into())
+            .and_then(|sessions| sessions.get(&parent_id))
+            .expect("No session map found");
+        assert!(matches!(session.state, SessionState::NeverExpires));
+
+        // The oauth2 session is revoked.
+        let session = entry
+            .get_ava_as_oauth2session_map(Attribute::OAuth2Session.into())
+            .and_then(|sessions| sessions.get(&session_id))
+            .expect("No session map found");
+        assert!(matches!(session.state, SessionState::RevokedAt(_)));
 
         assert!(server_txn.commit().is_ok());
     }

server/lib/src/plugins/session.rs

@@ -10,6 +10,7 @@
 use crate::event::ModifyEvent;
 use crate::plugins::Plugin;
 use crate::prelude::*;
+use crate::value::SessionState;
 use std::collections::BTreeSet;
 use std::sync::Arc;
 use time::OffsetDateTime;
@@ -49,6 +50,7 @@ impl SessionConsistency {
     ) -> Result<(), OperationError> {
         let curtime = qs.get_curtime();
         let curtime_odt = OffsetDateTime::UNIX_EPOCH + curtime;
+        trace!(%curtime_odt);
 
         // We need to assert a number of properties. We must do these *in order*.
         cand.iter_mut().try_for_each(|entry| {
@@ -87,8 +89,9 @@ impl SessionConsistency {
             let expired: Option<BTreeSet<_>> = entry.get_ava_as_session_map(Attribute::UserAuthTokenSession.into())
                 .map(|sessions| {
                     sessions.iter().filter_map(|(session_id, session)| {
-                        match &session.expiry {
-                            Some(exp) if exp <= &curtime_odt => {
+                        trace!(?session_id, ?session);
+                        match &session.state {
+                            SessionState::ExpiresAt(exp) if exp <= &curtime_odt => {
                                 info!(%session_id, "Removing expired auth session");
                                 Some(PartialValue::Refer(*session_id))
                             }
@@ -109,26 +112,43 @@ impl SessionConsistency {
                 let sessions = entry.get_ava_as_session_map(Attribute::UserAuthTokenSession.into());
 
                 oauth2_sessions.iter().filter_map(|(o2_session_id, session)| {
-                    match &session.expiry {
-                        Some(exp) if exp <= &curtime_odt => {
+                    trace!(?o2_session_id, ?session);
+                    match &session.state {
+                        SessionState::ExpiresAt(exp) if exp <= &curtime_odt => {
                             info!(%o2_session_id, "Removing expired oauth2 session");
                             Some(PartialValue::Refer(*o2_session_id))
                         }
+                        SessionState::RevokedAt(_) => {
+                            // no-op, it's already revoked.
+                            trace!("Skip already revoked session");
+                            None
+                        }
                         _ => {
                             // Okay, now check the issued / grace time for parent enforcement.
-                            if session.issued_at + GRACE_WINDOW <= curtime_odt {
-                                if sessions.map(|s| s.contains_key(&session.parent)).unwrap_or(false) {
-                                    // The parent exists, go ahead
+                                if sessions.map(|session_map| {
+                                    if let Some(parent_session) = session_map.get(&session.parent) {
+                                        // Only match non-revoked sessions
+                                        !matches!(parent_session.state, SessionState::RevokedAt(_))
+                                    } else {
+                                        // not found
+                                        false
+                                    }
+                                }).unwrap_or(false) {
+                                    // The parent exists and is still valid, go ahead
+                                    debug!("Parent session remains valid.");
                                     None
                                 } else {
-                                    info!(%o2_session_id, parent_id = %session.parent, "Removing unbound oauth2 session");
-                                    Some(PartialValue::Refer(*o2_session_id))
-                                }
-                            } else {
-                                // Grace window is still in effect
-                                None
-                            }
+                                    // Can't find the parent. Are we within grace window
+                                    if session.issued_at + GRACE_WINDOW <= curtime_odt {
+                                        info!(%o2_session_id, parent_id = %session.parent, "Removing orphaned oauth2 session");
+                                        Some(PartialValue::Refer(*o2_session_id))
+                                    } else {
+                                        // Grace window is still in effect
+                                        debug!("Not enforcing parent session consistency on session within grace window");
+                                        None
+                                    }
 
+                                }
                         }
                     }
 
@@ -151,7 +171,7 @@ mod tests {
     use crate::prelude::*;
 
     use crate::event::CreateEvent;
-    use crate::value::{Oauth2Session, Session};
+    use crate::value::{Oauth2Session, Session, SessionState};
     use kanidm_proto::constants::OAUTH2_SCOPE_OPENID;
     use std::time::Duration;
     use time::OffsetDateTime;
@@ -198,8 +218,7 @@ mod tests {
 
         // Create a fake session.
         let session_id = Uuid::new_v4();
-        let pv_session_id = PartialValue::Refer(session_id);
-        let expiry = Some(exp_curtime_odt);
+        let state = SessionState::ExpiresAt(exp_curtime_odt);
         let issued_at = curtime_odt;
         let issued_by = IdentityId::User(tuuid);
         let scope = SessionScope::ReadOnly;
@@ -208,7 +227,7 @@ mod tests {
             session_id,
             Session {
                 label: "label".to_string(),
-                expiry,
+                state,
                 // Need the other inner bits?
                 // for the gracewindow.
                 issued_at,
@@ -235,7 +254,11 @@ mod tests {
 
         let entry = server_txn.internal_search_uuid(tuuid).expect("failed");
 
-        assert!(entry.attribute_equality(Attribute::UserAuthTokenSession.into(), &pv_session_id));
+        let session = entry
+            .get_ava_as_session_map(Attribute::UserAuthTokenSession.into())
+            .and_then(|sessions| sessions.get(&session_id))
+            .expect("No session map found");
+        assert!(matches!(session.state, SessionState::ExpiresAt(_)));
 
         assert!(server_txn.commit().is_ok());
         let mut server_txn = server.write(exp_curtime).await;
@@ -256,8 +279,12 @@ mod tests {
         // Session gone.
         let entry = server_txn.internal_search_uuid(tuuid).expect("failed");
 
-        // Note it's a not condition now.
-        assert!(!entry.attribute_equality(Attribute::UserAuthTokenSession.into(), &pv_session_id));
+        // We get the attribute and have to check it's now in a revoked state.
+        let session = entry
+            .get_ava_as_session_map(Attribute::UserAuthTokenSession.into())
+            .and_then(|sessions| sessions.get(&session_id))
+            .expect("No session map found");
+        assert!(matches!(session.state, SessionState::RevokedAt(_)));
 
         assert!(server_txn.commit().is_ok());
     }
@@ -336,11 +363,8 @@ mod tests {
         // Create a fake session and oauth2 session.
 
         let session_id = Uuid::new_v4();
-        let pv_session_id = PartialValue::Refer(session_id);
-
-        let parent = Uuid::new_v4();
-        let pv_parent_id = PartialValue::Refer(parent);
-        let expiry = Some(exp_curtime_odt);
+        let parent_id = Uuid::new_v4();
+        let state = SessionState::ExpiresAt(exp_curtime_odt);
         let issued_at = curtime_odt;
         let issued_by = IdentityId::User(tuuid);
         let scope = SessionScope::ReadOnly;
@@ -352,9 +376,9 @@ mod tests {
                 Value::Oauth2Session(
                     session_id,
                     Oauth2Session {
-                        parent,
+                        parent: parent_id,
                         // Set to the exp window.
-                        expiry,
+                        state,
                         issued_at,
                         rs_uuid,
                     },
@@ -363,11 +387,11 @@ mod tests {
             Modify::Present(
                 Attribute::UserAuthTokenSession.into(),
                 Value::Session(
-                    parent,
+                    parent_id,
                     Session {
                         label: "label".to_string(),
                         // Note we set the exp to None so we are not removing based on removal of the parent.
-                        expiry: None,
+                        state: SessionState::NeverExpires,
                         // Need the other inner bits?
                         // for the gracewindow.
                         issued_at,
@@ -393,8 +417,17 @@ mod tests {
 
         let entry = server_txn.internal_search_uuid(tuuid).expect("failed");
 
-        assert!(entry.attribute_equality(Attribute::UserAuthTokenSession.into(), &pv_parent_id));
-        assert!(entry.attribute_equality(Attribute::OAuth2Session.into(), &pv_session_id));
+        let session = entry
+            .get_ava_as_session_map(Attribute::UserAuthTokenSession.into())
+            .and_then(|sessions| sessions.get(&parent_id))
+            .expect("No session map found");
+        assert!(matches!(session.state, SessionState::NeverExpires));
+
+        let session = entry
+            .get_ava_as_oauth2session_map(Attribute::OAuth2Session.into())
+            .and_then(|sessions| sessions.get(&session_id))
+            .expect("No session map found");
+        assert!(matches!(session.state, SessionState::ExpiresAt(_)));
 
         assert!(server_txn.commit().is_ok());
 
@@ -419,9 +452,17 @@ mod tests {
         let entry = server_txn.internal_search_uuid(tuuid).expect("failed");
 
         // Note the uat is still present
-        assert!(entry.attribute_equality(Attribute::UserAuthTokenSession.into(), &pv_parent_id));
-        // Note it's a not condition now.
-        assert!(!entry.attribute_equality(Attribute::OAuth2Session.into(), &pv_session_id));
+        let session = entry
+            .get_ava_as_session_map(Attribute::UserAuthTokenSession.into())
+            .and_then(|sessions| sessions.get(&parent_id))
+            .expect("No session map found");
+        assert!(matches!(session.state, SessionState::NeverExpires));
+
+        let session = entry
+            .get_ava_as_oauth2session_map(Attribute::OAuth2Session.into())
+            .and_then(|sessions| sessions.get(&session_id))
+            .expect("No session map found");
+        assert!(matches!(session.state, SessionState::RevokedAt(_)));
 
         assert!(server_txn.commit().is_ok());
     }
@@ -497,10 +538,7 @@ mod tests {
         // Create a fake session and oauth2 session.
 
         let session_id = Uuid::new_v4();
-        let pv_session_id = PartialValue::Refer(session_id);
-
-        let parent = Uuid::new_v4();
-        let pv_parent_id = PartialValue::Refer(parent);
+        let parent_id = Uuid::new_v4();
         let issued_at = curtime_odt;
         let issued_by = IdentityId::User(tuuid);
         let scope = SessionScope::ReadOnly;
@@ -512,9 +550,9 @@ mod tests {
                 Value::Oauth2Session(
                     session_id,
                     Oauth2Session {
-                        parent,
+                        parent: parent_id,
                         // Note we set the exp to None so we are not removing based on exp
-                        expiry: None,
+                        state: SessionState::NeverExpires,
                         issued_at,
                         rs_uuid,
                     },
@@ -523,11 +561,11 @@ mod tests {
             Modify::Present(
                 Attribute::UserAuthTokenSession.into(),
                 Value::Session(
-                    parent,
+                    parent_id,
                     Session {
                         label: "label".to_string(),
                         // Note we set the exp to None so we are not removing based on removal of the parent.
-                        expiry: None,
+                        state: SessionState::NeverExpires,
                         // Need the other inner bits?
                         // for the gracewindow.
                         issued_at,
@@ -553,16 +591,27 @@ mod tests {
 
         let entry = server_txn.internal_search_uuid(tuuid).expect("failed");
 
-        assert!(entry.attribute_equality(Attribute::UserAuthTokenSession.into(), &pv_parent_id));
-        assert!(entry.attribute_equality(Attribute::OAuth2Session.into(), &pv_session_id));
+        let session = entry
+            .get_ava_as_session_map(Attribute::UserAuthTokenSession.into())
+            .and_then(|sessions| sessions.get(&parent_id))
+            .expect("No session map found");
+        assert!(matches!(session.state, SessionState::NeverExpires));
+
+        let session = entry
+            .get_ava_as_oauth2session_map(Attribute::OAuth2Session.into())
+            .and_then(|sessions| sessions.get(&session_id))
+            .expect("No session map found");
+        assert!(matches!(session.state, SessionState::NeverExpires));
 
         // We need the time to be past grace_window.
         assert!(server_txn.commit().is_ok());
         let mut server_txn = server.write(exp_curtime).await;
 
         // Mod again - remove the parent session.
-        let modlist =
-            ModifyList::new_remove(Attribute::UserAuthTokenSession.into(), pv_parent_id.clone());
+        let modlist = ModifyList::new_remove(
+            Attribute::UserAuthTokenSession.into(),
+            PartialValue::Refer(parent_id),
+        );
 
         server_txn
             .internal_modify(
@@ -575,9 +624,18 @@ mod tests {
         let entry = server_txn.internal_search_uuid(tuuid).expect("failed");
 
         // Note the uat is removed
-        assert!(!entry.attribute_equality(Attribute::UserAuthTokenSession.into(), &pv_parent_id));
+        let session = entry
+            .get_ava_as_session_map(Attribute::UserAuthTokenSession.into())
+            .and_then(|sessions| sessions.get(&parent_id))
+            .expect("No session map found");
+        assert!(matches!(session.state, SessionState::RevokedAt(_)));
+
         // The oauth2 session is also removed.
-        assert!(!entry.attribute_equality(Attribute::OAuth2Session.into(), &pv_session_id));
+        let session = entry
+            .get_ava_as_oauth2session_map(Attribute::OAuth2Session.into())
+            .and_then(|sessions| sessions.get(&session_id))
+            .expect("No session map found");
+        assert!(matches!(session.state, SessionState::RevokedAt(_)));
 
         assert!(server_txn.commit().is_ok());
     }
@@ -647,8 +705,6 @@ mod tests {
 
         // Create a fake session.
         let session_id = Uuid::new_v4();
-        let pv_session_id = PartialValue::Refer(session_id);
-
         let parent = Uuid::new_v4();
         let issued_at = curtime_odt;
 
@@ -658,7 +714,7 @@ mod tests {
                 parent,
                 // Note we set the exp to None so we are asserting the removal is due to the lack
                 // of the parent session.
-                expiry: None,
+                state: SessionState::NeverExpires,
                 issued_at,
                 rs_uuid,
             },
@@ -678,7 +734,11 @@ mod tests {
 
         let entry = server_txn.internal_search_uuid(tuuid).expect("failed");
 
-        assert!(entry.attribute_equality(Attribute::OAuth2Session.into(), &pv_session_id));
+        let session = entry
+            .get_ava_as_oauth2session_map(Attribute::OAuth2Session.into())
+            .and_then(|sessions| sessions.get(&session_id))
+            .expect("No session map found");
+        assert!(matches!(session.state, SessionState::NeverExpires));
 
         assert!(server_txn.commit().is_ok());
 
@@ -703,7 +763,11 @@ mod tests {
         let entry = server_txn.internal_search_uuid(tuuid).expect("failed");
 
         // Note it's a not condition now.
-        assert!(!entry.attribute_equality(Attribute::OAuth2Session.as_ref(), &pv_session_id));
+        let session = entry
+            .get_ava_as_oauth2session_map(Attribute::OAuth2Session.into())
+            .and_then(|sessions| sessions.get(&session_id))
+            .expect("No session map found");
+        assert!(matches!(session.state, SessionState::RevokedAt(_)));
 
         assert!(server_txn.commit().is_ok());
     }
@@ -741,9 +805,7 @@ mod tests {
 
         // Create a fake session.
         let session_id = Uuid::new_v4();
-        let pv_session_id = PartialValue::Refer(session_id);
         // No expiry!
-        let expiry = None;
         let issued_at = curtime_odt;
         let issued_by = IdentityId::User(tuuid);
         let scope = SessionScope::ReadOnly;
@@ -752,7 +814,7 @@ mod tests {
             session_id,
             Session {
                 label: "label".to_string(),
-                expiry,
+                state: SessionState::NeverExpires,
                 // Need the other inner bits?
                 // for the gracewindow.
                 issued_at,
@@ -779,7 +841,11 @@ mod tests {
 
         let entry = server_txn.internal_search_uuid(tuuid).expect("failed");
 
-        assert!(entry.attribute_equality(Attribute::UserAuthTokenSession.into(), &pv_session_id));
+        let session = entry
+            .get_ava_as_session_map(Attribute::UserAuthTokenSession.into())
+            .and_then(|sessions| sessions.get(&session_id))
+            .expect("No session map found");
+        assert!(matches!(session.state, SessionState::NeverExpires));
 
         assert!(server_txn.commit().is_ok());
 
@@ -800,7 +866,11 @@ mod tests {
         let entry = server_txn.internal_search_uuid(tuuid).expect("failed");
 
         // Note it's a not condition now.
-        assert!(!entry.attribute_equality(Attribute::UserAuthTokenSession.into(), &pv_session_id));
+        let session = entry
+            .get_ava_as_session_map(Attribute::UserAuthTokenSession.into())
+            .and_then(|sessions| sessions.get(&session_id))
+            .expect("No session map found");
+        assert!(matches!(session.state, SessionState::RevokedAt(_)));
 
         assert!(server_txn.commit().is_ok());
     }

server/lib/src/repl/consumer.rs

@@ -104,7 +104,7 @@ impl<'a> QueryServerWriteTransaction<'a> {
                 // their attribute sets/states per the change state rules.
 
                 // This must create an EntryInvalidCommitted
-                let merge_ent = ctx_ent.merge_state(db_ent.as_ref(), &self.schema);
+                let merge_ent = ctx_ent.merge_state(db_ent.as_ref(), &self.schema, self.trim_cid());
                 (merge_ent, db_ent)
             })
             .collect();

server/lib/src/repl/proto.rs

@@ -229,7 +229,8 @@ pub struct ReplOauthScopeMapV1 {
 pub struct ReplOauth2SessionV1 {
     pub refer: Uuid,
     pub parent: Uuid,
-    pub expiry: Option<String>,
+    pub state: ReplSessionStateV1,
+    // pub expiry: Option<String>,
     pub issued_at: String,
     pub rs_uuid: Uuid,
 }
@@ -262,13 +263,21 @@ pub enum ReplIdentityIdV1 {
 pub struct ReplSessionV1 {
     pub refer: Uuid,
     pub label: String,
-    pub expiry: Option<String>,
+    pub state: ReplSessionStateV1,
+    // pub expiry: Option<String>,
     pub issued_at: String,
     pub issued_by: ReplIdentityIdV1,
     pub cred_id: Uuid,
     pub scope: ReplSessionScopeV1,
 }
 
+#[derive(Serialize, Deserialize, Debug, PartialEq, Eq)]
+pub enum ReplSessionStateV1 {
+    ExpiresAt(String),
+    Never,
+    RevokedAt(ReplCidV1),
+}
+
 #[derive(Serialize, Deserialize, Debug, PartialEq, Eq)]
 pub struct ReplApiTokenV1 {
     pub refer: Uuid,
@@ -388,7 +397,7 @@ pub enum ReplAttrV1 {
         set: Vec<(String, ReplTotpV1)>,
     },
     AuditLogString {
-        set: Vec<(Cid, String)>,
+        map: Vec<(Cid, String)>,
     },
     EcKeyPrivate {
         key: Vec<u8>,

server/lib/src/server/batch_modify.rs

@@ -106,7 +106,10 @@ impl<'a> QueryServerWriteTransaction<'a> {
             .iter()
             .map(|er| {
                 let u = er.get_uuid();
-                let mut ent_mut = er.as_ref().clone().invalidate(self.cid.clone());
+                let mut ent_mut = er
+                    .as_ref()
+                    .clone()
+                    .invalidate(self.cid.clone(), &self.trim_cid);
 
                 me.modset
                     .get(&u)

server/lib/src/server/delete.rs

@@ -51,7 +51,11 @@ impl<'a> QueryServerWriteTransaction<'a> {
         let mut candidates: Vec<Entry<EntryInvalid, EntryCommitted>> = pre_candidates
             .iter()
             // Invalidate and assign change id's
-            .map(|er| er.as_ref().clone().invalidate(self.cid.clone()))
+            .map(|er| {
+                er.as_ref()
+                    .clone()
+                    .invalidate(self.cid.clone(), &self.trim_cid)
+            })
             .collect();
 
         trace!(?candidates, "delete: candidates");

server/lib/src/server/migrations.rs

@@ -223,7 +223,11 @@ impl<'a> QueryServerWriteTransaction<'a> {
         // Change the value type.
         let mut candidates: Vec<Entry<EntryInvalid, EntryCommitted>> = pre_candidates
             .iter()
-            .map(|er| er.as_ref().clone().invalidate(self.cid.clone()))
+            .map(|er| {
+                er.as_ref()
+                    .clone()
+                    .invalidate(self.cid.clone(), &self.trim_cid)
+            })
             .collect();
 
         candidates.iter_mut().try_for_each(|er| {

server/lib/src/server/mod.rs

@@ -104,6 +104,7 @@ pub struct QueryServerWriteTransaction<'a> {
     d_info: CowCellWriteTxn<'a, DomainInfo>,
     curtime: Duration,
     cid: Cid,
+    trim_cid: Cid,
     pub(crate) be_txn: BackendWriteTransaction<'a>,
     pub(crate) schema: SchemaWriteTransaction<'a>,
     accesscontrols: AccessControlsWriteTransaction<'a>,
@@ -124,6 +125,12 @@ pub struct QueryServerWriteTransaction<'a> {
     dyngroup_cache: CowCellWriteTxn<'a, DynGroupCache>,
 }
 
+impl<'a> QueryServerWriteTransaction<'a> {
+    pub(crate) fn trim_cid(&self) -> &Cid {
+        &self.trim_cid
+    }
+}
+
 /// The `QueryServerTransaction` trait provides a set of common read only operations to be
 /// shared between [`QueryServerReadTransaction`] and [`QueryServerWriteTransaction`]s.
 ///
@@ -1191,6 +1198,10 @@ impl QueryServer {
             .get_db_ts_max(curtime)
             .expect("Unable to get db_ts_max");
         let cid = Cid::new_lamport(self.s_uuid, curtime, &ts_max);
+        #[allow(clippy::expect_used)]
+        let trim_cid = cid
+            .sub_secs(CHANGELOG_MAX_AGE)
+            .expect("unable to generate trim cid");
 
         QueryServerWriteTransaction {
             // I think this is *not* needed, because commit is mut self which should
@@ -1204,6 +1215,7 @@ impl QueryServer {
             d_info,
             curtime,
             cid,
+            trim_cid,
             be_txn,
             schema: schema_write,
             accesscontrols: self.accesscontrols.write(),

server/lib/src/server/modify.rs

@@ -29,6 +29,8 @@ impl<'a> QueryServerWriteTransaction<'a> {
         &mut self,
         me: &'x ModifyEvent,
     ) -> Result<Option<ModifyPartial<'x>>, OperationError> {
+        trace!(?me);
+
         // Get the candidates.
         // Modify applies a modlist to a filter, so we need to internal search
         // then apply.
@@ -95,7 +97,11 @@ impl<'a> QueryServerWriteTransaction<'a> {
         // and the new modified ents.
         let mut candidates: Vec<Entry<EntryInvalid, EntryCommitted>> = pre_candidates
             .iter()
-            .map(|er| er.as_ref().clone().invalidate(self.cid.clone()))
+            .map(|er| {
+                er.as_ref()
+                    .clone()
+                    .invalidate(self.cid.clone(), &self.trim_cid)
+            })
             .collect();
 
         candidates.iter_mut().try_for_each(|er| {
@@ -273,7 +279,10 @@ impl<'a> QueryServerWriteTransaction<'a> {
         self.search(&se).map(|vs| {
             vs.into_iter()
                 .map(|e| {
-                    let writeable = e.as_ref().clone().invalidate(self.cid.clone());
+                    let writeable = e
+                        .as_ref()
+                        .clone()
+                        .invalidate(self.cid.clone(), &self.trim_cid);
                     (e, writeable)
                 })
                 .collect()

server/lib/src/server/recycle.rs

@@ -8,14 +8,11 @@ impl<'a> QueryServerWriteTransaction<'a> {
     #[instrument(level = "debug", skip_all)]
     pub fn purge_tombstones(&mut self) -> Result<(), OperationError> {
         // purge everything that is a tombstone.
-        let cid = self.cid.sub_secs(CHANGELOG_MAX_AGE).map_err(|e| {
-            admin_error!("Unable to generate search cid {:?}", e);
-            e
-        })?;
+        let trim_cid = self.trim_cid().clone();
 
         // Delete them - this is a TRUE delete, no going back now!
         self.be_txn
-            .reap_tombstones(&cid)
+            .reap_tombstones(&trim_cid)
             .map_err(|e| {
                 admin_error!(err = ?e, "Tombstone purge operation failed (backend)");
                 e
@@ -164,7 +161,11 @@ impl<'a> QueryServerWriteTransaction<'a> {
         // clone the writeable entries.
         let mut candidates: Vec<Entry<EntryInvalid, EntryCommitted>> = pre_candidates
             .iter()
-            .map(|er| er.as_ref().clone().invalidate(self.cid.clone()))
+            .map(|er| {
+                er.as_ref()
+                    .clone()
+                    .invalidate(self.cid.clone(), &self.trim_cid)
+            })
             // Mutate to apply the revive.
             .map(|er| er.to_revived())
             .collect();

server/lib/src/value.rs

@@ -5,6 +5,7 @@
 
 #![allow(non_upper_case_globals)]
 
+use std::cmp::Ordering;
 use std::collections::BTreeSet;
 use std::convert::TryFrom;
 use std::fmt;
@@ -817,10 +818,54 @@ impl TryInto<UatPurposeStatus> for SessionScope {
     }
 }
 
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub enum SessionState {
+    // IMPORTANT - this order allows sorting by
+    // lowest to highest, we always want to take
+    // the lowest value!
+    RevokedAt(Cid),
+    ExpiresAt(OffsetDateTime),
+    NeverExpires,
+}
+
+impl PartialOrd for SessionState {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl Ord for SessionState {
+    fn cmp(&self, other: &Self) -> Ordering {
+        // We need this to order by highest -> least which represents
+        // priority amongst these elements.
+        match (self, other) {
+            // RevokedAt with the earliest time = highest
+            (SessionState::RevokedAt(c_self), SessionState::RevokedAt(c_other)) => {
+                // We need to reverse this - we need the "lower value" to take priority.
+                // This is similar to tombstones where the earliest CID must be persisted
+                c_other.cmp(c_self)
+            }
+            (SessionState::RevokedAt(_), _) => Ordering::Greater,
+            (_, SessionState::RevokedAt(_)) => Ordering::Less,
+            // ExpiresAt with a greater time = higher
+            (SessionState::ExpiresAt(e_self), SessionState::ExpiresAt(e_other)) => {
+                // Keep the "newer" expiry. This can be because a session was extended
+                // by some mechanism, generally in oauth2.
+                e_self.cmp(e_other)
+            }
+            (SessionState::ExpiresAt(_), _) => Ordering::Greater,
+            (_, SessionState::ExpiresAt(_)) => Ordering::Less,
+            // NeverExpires = least.
+            (SessionState::NeverExpires, SessionState::NeverExpires) => Ordering::Equal,
+        }
+    }
+}
+
 #[derive(Clone, PartialEq, Eq)]
 pub struct Session {
     pub label: String,
-    pub expiry: Option<OffsetDateTime>,
+    // pub expiry: Option<OffsetDateTime>,
+    pub state: SessionState,
     pub issued_at: OffsetDateTime,
     pub issued_by: IdentityId,
     pub cred_id: Uuid,
@@ -834,13 +879,14 @@ impl fmt::Debug for Session {
             IdentityId::Synch(u) => format!("Synch - {}", uuid_to_proto_string(u)),
             IdentityId::Internal => "Internal".to_string(),
         };
-        let expiry = match self.expiry {
-            Some(e) => e.to_string(),
-            None => "never".to_string(),
+        let expiry = match self.state {
+            SessionState::ExpiresAt(e) => e.to_string(),
+            SessionState::NeverExpires => "never".to_string(),
+            SessionState::RevokedAt(_) => "revoked".to_string(),
         };
         write!(
             f,
-            "expiry: {}, issued at: {}, issued by: {}, credential id: {}, scope: {:?}",
+            "state: {}, issued at: {}, issued by: {}, credential id: {}, scope: {:?}",
             expiry, self.issued_at, issuer, self.cred_id, self.scope
         )
     }
@@ -849,7 +895,8 @@ impl fmt::Debug for Session {
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct Oauth2Session {
     pub parent: Uuid,
-    pub expiry: Option<OffsetDateTime>,
+    // pub expiry: Option<OffsetDateTime>,
+    pub state: SessionState,
     pub issued_at: OffsetDateTime,
     pub rs_uuid: Uuid,
 }

server/lib/src/valueset/address.rs

@@ -104,7 +104,7 @@ impl ValueSetT for ValueSetAddress {
         self.set.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::Address(_) => {
                 unreachable!()
@@ -325,7 +325,7 @@ impl ValueSetT for ValueSetEmailAddress {
         self.set.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::EmailAddress(a) => {
                 let r = self.set.remove(a);
@@ -463,6 +463,7 @@ pub struct ValueSetPhoneNumber {
 #[cfg(test)]
 mod tests {
     use super::ValueSetEmailAddress;
+    use crate::repl::cid::Cid;
     use crate::value::{PartialValue, Value};
     use crate::valueset::{self, ValueSet};
 
@@ -501,7 +502,10 @@ mod tests {
         assert!(vs.to_email_address_primary_str() == vs2.to_email_address_primary_str());
 
         // Remove primary, assert it's gone and that the "first" address is assigned.
-        assert!(vs.remove(&PartialValue::new_email_address_s("primary@example.com")));
+        assert!(vs.remove(
+            &PartialValue::new_email_address_s("primary@example.com"),
+            &Cid::new_zero()
+        ));
         assert!(vs.len() == 2);
         assert!(vs.to_email_address_primary_str() == Some("alice@example.com"));
 

server/lib/src/valueset/auditlogstring.rs

@@ -1,44 +1,47 @@
-use smolset::SmolSet;
-
 use crate::prelude::*;
 use crate::repl::cid::Cid;
 use crate::repl::proto::ReplAttrV1;
 use crate::schema::SchemaAttribute;
 use crate::valueset::{DbValueSetV2, ValueSet};
+use std::collections::BTreeMap;
 
 type AuditLogStringType = (Cid, String);
 
+pub const AUDIT_LOG_STRING_CAPACITY: usize = 9;
+
 #[derive(Debug, Clone)]
 pub struct ValueSetAuditLogString {
-    set: SmolSet<[AuditLogStringType; 8]>,
+    map: BTreeMap<Cid, String>,
 }
 
 impl ValueSetAuditLogString {
     fn remove_oldest(&mut self) {
-        let oldest = self.set.iter().min().cloned();
-        if let Some(oldest_value) = oldest {
-            self.set.remove(&oldest_value);
+        // pop to size.
+        while self.map.len() > AUDIT_LOG_STRING_CAPACITY {
+            self.map.pop_first();
         }
     }
 
-    pub fn new(s: AuditLogStringType) -> Box<Self> {
-        let mut set = SmolSet::new();
-        set.insert(s);
-        Box::new(ValueSetAuditLogString { set })
+    pub fn new((c, s): AuditLogStringType) -> Box<Self> {
+        let mut map = BTreeMap::new();
+        map.insert(c, s);
+        Box::new(ValueSetAuditLogString { map })
     }
 
-    pub fn push(&mut self, s: AuditLogStringType) -> bool {
-        self.set.insert(s)
+    /*
+    pub fn push(&mut self, (c, s): AuditLogStringType) -> bool {
+        self.map.insert(c, s).is_none()
     }
+    */
 
     pub fn from_dbvs2(data: Vec<AuditLogStringType>) -> Result<ValueSet, OperationError> {
-        let set = data.into_iter().collect();
-        Ok(Box::new(ValueSetAuditLogString { set }))
+        let map = data.into_iter().collect();
+        Ok(Box::new(ValueSetAuditLogString { map }))
     }
 
     pub fn from_repl_v1(data: &[AuditLogStringType]) -> Result<ValueSet, OperationError> {
-        let set = data.iter().map(|e| (e.0.clone(), e.1.clone())).collect();
-        Ok(Box::new(ValueSetAuditLogString { set }))
+        let map = data.iter().map(|e| (e.0.clone(), e.1.clone())).collect();
+        Ok(Box::new(ValueSetAuditLogString { map }))
     }
 }
 
@@ -46,10 +49,10 @@ impl ValueSetT for ValueSetAuditLogString {
     fn insert_checked(&mut self, value: Value) -> Result<bool, OperationError> {
         match value {
             Value::AuditLogString(c, s) => {
-                if self.set.len() >= 8 {
-                    self.remove_oldest();
-                }
-                Ok(self.push((c, s)))
+                let r = self.map.insert(c, s);
+                self.remove_oldest();
+                // true if insert was a new value.
+                Ok(r.is_none())
             }
             _ => {
                 debug_assert!(false);
@@ -59,16 +62,17 @@ impl ValueSetT for ValueSetAuditLogString {
     }
 
     fn clear(&mut self) {
-        self.set.clear();
+        self.map.clear();
     }
 
-    fn remove(&mut self, _pv: &PartialValue) -> bool {
+    fn remove(&mut self, _pv: &PartialValue, _cid: &Cid) -> bool {
         false
     }
 
     fn contains(&self, pv: &PartialValue) -> bool {
         match pv {
-            PartialValue::Utf8(s) => self.set.iter().any(|(_, current)| s.eq(current)),
+            PartialValue::Utf8(s) => self.map.values().any(|current| s.eq(current)),
+            PartialValue::Cid(c) => self.map.contains_key(c),
             _ => {
                 debug_assert!(false);
                 true
@@ -85,11 +89,11 @@ impl ValueSetT for ValueSetAuditLogString {
     }
 
     fn len(&self) -> usize {
-        self.set.len()
+        self.map.len()
     }
 
     fn generate_idx_eq_keys(&self) -> Vec<String> {
-        self.set.iter().map(|(d, s)| format!("{d}-{s}")).collect()
+        self.map.iter().map(|(d, s)| format!("{d}-{s}")).collect()
     }
 
     fn syntax(&self) -> SyntaxType {
@@ -97,33 +101,42 @@ impl ValueSetT for ValueSetAuditLogString {
     }
 
     fn validate(&self, _schema_attr: &SchemaAttribute) -> bool {
-        self.set
+        self.map
             .iter()
             .all(|(_, s)| Value::validate_str_escapes(s) && Value::validate_singleline(s))
-            && self.set.len() <= 8
+            && self.map.len() <= AUDIT_LOG_STRING_CAPACITY
     }
 
     fn to_proto_string_clone_iter(&self) -> Box<dyn Iterator<Item = String> + '_> {
-        Box::new(self.set.iter().map(|(d, s)| format!("{d}-{s}")))
+        Box::new(self.map.iter().map(|(d, s)| format!("{d}-{s}")))
     }
 
     fn to_db_valueset_v2(&self) -> DbValueSetV2 {
-        DbValueSetV2::AuditLogString(self.set.iter().cloned().collect())
+        DbValueSetV2::AuditLogString(
+            self.map
+                .iter()
+                .map(|(c, s)| (c.clone(), s.clone()))
+                .collect(),
+        )
     }
 
     fn to_repl_v1(&self) -> ReplAttrV1 {
         ReplAttrV1::AuditLogString {
-            set: self.set.iter().cloned().collect(),
+            map: self
+                .map
+                .iter()
+                .map(|(c, s)| (c.clone(), s.clone()))
+                .collect(),
         }
     }
 
     fn to_partialvalue_iter(&self) -> Box<dyn Iterator<Item = PartialValue> + '_> {
-        Box::new(self.set.iter().map(|(_, s)| PartialValue::Utf8(s.clone())))
+        Box::new(self.map.keys().map(|c| PartialValue::Cid(c.clone())))
     }
 
     fn to_value_iter(&self) -> Box<dyn Iterator<Item = Value> + '_> {
         Box::new(
-            self.set
+            self.map
                 .iter()
                 .map(|(c, s)| Value::AuditLogString(c.clone(), s.clone())),
         )
@@ -131,7 +144,7 @@ impl ValueSetT for ValueSetAuditLogString {
 
     fn equal(&self, other: &ValueSet) -> bool {
         if let Some(other) = other.as_audit_log_string() {
-            &self.set == other
+            &self.map == other
         } else {
             debug_assert!(false);
             false
@@ -140,13 +153,187 @@ impl ValueSetT for ValueSetAuditLogString {
 
     fn merge(&mut self, other: &ValueSet) -> Result<(), OperationError> {
         if let Some(b) = other.as_audit_log_string() {
-            mergesets!(self.set, b)
+            mergemaps!(self.map, b)?;
+            self.remove_oldest();
+            Ok(())
         } else {
             debug_assert!(false);
             Err(OperationError::InvalidValueState)
         }
     }
-    fn as_audit_log_string(&self) -> Option<&SmolSet<[(Cid, String); 8]>> {
-        Some(&self.set)
+
+    #[allow(clippy::todo)]
+    fn repl_merge_valueset(&self, older: &ValueSet, _trim_cid: &Cid) -> Option<ValueSet> {
+        if let Some(mut map) = older.as_audit_log_string().cloned() {
+            // Merge maps is right-preferencing, so this means that
+            // newer content always wins over.
+            mergemaps!(map, self.map)
+                .map_err(|_: OperationError| ())
+                .ok()?;
+            let mut new_vs = Box::new(ValueSetAuditLogString { map });
+            new_vs.remove_oldest();
+            Some(new_vs)
+        } else {
+            debug_assert!(false);
+            None
+        }
+    }
+
+    fn as_audit_log_string(&self) -> Option<&BTreeMap<Cid, String>> {
+        Some(&self.map)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::{ValueSetAuditLogString, AUDIT_LOG_STRING_CAPACITY};
+    use crate::repl::cid::Cid;
+    use crate::value::Value;
+    use crate::valueset::ValueSet;
+    use std::time::Duration;
+
+    #[test]
+    fn test_valueset_auditlogstring_merge() {
+        let mut vs: ValueSet = ValueSetAuditLogString::new((Cid::new_count(0), "A".to_string()));
+        assert!(vs.len() == 1);
+
+        for i in 1..AUDIT_LOG_STRING_CAPACITY {
+            vs.insert_checked(Value::AuditLogString(
+                Cid::new_count(i as u64),
+                "A".to_string(),
+            ))
+            .unwrap();
+        }
+
+        assert!(vs.len() == AUDIT_LOG_STRING_CAPACITY);
+
+        // Add one extra
+        vs.insert_checked(Value::AuditLogString(
+            Cid::new_count(AUDIT_LOG_STRING_CAPACITY as u64),
+            "A".to_string(),
+        ))
+        .unwrap();
+
+        assert!(vs.len() == AUDIT_LOG_STRING_CAPACITY);
+
+        let mut v_iter = vs.to_value_iter();
+        let Some(Value::AuditLogString(c, _s)) = v_iter.next() else {
+            unreachable!();
+        };
+        // Should always be '1' since the set merge would have pushed '0' (ring-buffer);
+        assert!(c.ts == Duration::from_secs(1));
+        println!("{:?}", c);
+        drop(v_iter);
+
+        // Make a second set.
+        let other_vs: ValueSet = ValueSetAuditLogString::new(
+            // Notice that 0 here is older than our other set items.
+            (Cid::new_count(0), "A".to_string()),
+        );
+        assert!(other_vs.len() == 1);
+
+        // Merge. The content of other_vs should be dropped.
+        vs.merge(&other_vs)
+            .expect("Failed to merge, incorrect types");
+
+        // No change in the state of the set.
+        assert!(vs.len() == AUDIT_LOG_STRING_CAPACITY);
+        let mut v_iter = vs.to_value_iter();
+        let Some(Value::AuditLogString(c, _s)) = v_iter.next() else {
+            unreachable!();
+        };
+        // Should always be '1' since the set merge would have pushed '0' (ring-buffer);
+        assert!(c.ts == Duration::from_secs(1));
+        println!("{:?}", c);
+        drop(v_iter);
+
+        // Now merge in with a set that has a value that is newer.
+
+        assert!(100 > AUDIT_LOG_STRING_CAPACITY);
+
+        let other_vs: ValueSet = ValueSetAuditLogString::new(
+            // Notice that 0 here is older than our other set items.
+            (Cid::new_count(100), "A".to_string()),
+        );
+        assert!(other_vs.len() == 1);
+
+        vs.merge(&other_vs)
+            .expect("Failed to merge, incorrect types");
+
+        // New value has pushed out the next oldest.
+        assert!(vs.len() == AUDIT_LOG_STRING_CAPACITY);
+        let mut v_iter = vs.to_value_iter();
+        let Some(Value::AuditLogString(c, _s)) = v_iter.next() else {
+            unreachable!();
+        };
+        // Should always be '1' since the set merge would have pushed '0' (ring-buffer);
+        println!("{:?}", c);
+        assert!(c.ts == Duration::from_secs(2));
+        drop(v_iter);
+    }
+
+    #[test]
+    fn test_valueset_auditlogstring_repl_merge() {
+        let zero_cid = Cid::new_zero();
+        let mut vs: ValueSet = ValueSetAuditLogString::new((Cid::new_count(1), "A".to_string()));
+        assert!(vs.len() == 1);
+
+        for i in 2..(AUDIT_LOG_STRING_CAPACITY + 1) {
+            vs.insert_checked(Value::AuditLogString(
+                Cid::new_count(i as u64),
+                "A".to_string(),
+            ))
+            .unwrap();
+        }
+
+        assert!(vs.len() == AUDIT_LOG_STRING_CAPACITY);
+
+        // Make a second set.
+        let other_vs: ValueSet = ValueSetAuditLogString::new(
+            // Notice that 0 here is older than our other set items.
+            (Cid::new_count(0), "A".to_string()),
+        );
+        assert!(other_vs.len() == 1);
+
+        // Merge. The content of other_vs should be dropped.
+        let r_vs = vs
+            .repl_merge_valueset(&other_vs, &zero_cid)
+            .expect("merge did not occur");
+
+        // No change in the state of the set.
+        assert!(r_vs.len() == AUDIT_LOG_STRING_CAPACITY);
+        let mut v_iter = r_vs.to_value_iter();
+        let Some(Value::AuditLogString(c, _s)) = v_iter.next() else {
+            unreachable!();
+        };
+        // Should always be '1' since the set merge would have pushed '0' (ring-buffer);
+        assert!(c.ts == Duration::from_secs(1));
+        println!("{:?}", c);
+        drop(v_iter);
+
+        // Now merge in with a set that has a value that is newer.
+
+        assert!(100 > AUDIT_LOG_STRING_CAPACITY);
+
+        let other_vs: ValueSet = ValueSetAuditLogString::new(
+            // Notice that 0 here is older than our other set items.
+            (Cid::new_count(100), "A".to_string()),
+        );
+        assert!(other_vs.len() == 1);
+
+        let r_vs = vs
+            .repl_merge_valueset(&other_vs, &zero_cid)
+            .expect("merge did not occur");
+
+        // New value has pushed out the next oldest.
+        assert!(r_vs.len() == AUDIT_LOG_STRING_CAPACITY);
+        let mut v_iter = r_vs.to_value_iter();
+        let Some(Value::AuditLogString(c, _s)) = v_iter.next() else {
+            unreachable!();
+        };
+        // Should always be '1' since the set merge would have pushed '0' (ring-buffer);
+        println!("{:?}", c);
+        assert!(c.ts == Duration::from_secs(2));
+        drop(v_iter);
     }
 }

server/lib/src/valueset/binary.rs

@@ -62,7 +62,7 @@ impl ValueSetT for ValueSetPrivateBinary {
         self.set.clear();
     }
 
-    fn remove(&mut self, _pv: &PartialValue) -> bool {
+    fn remove(&mut self, _pv: &PartialValue, _cid: &Cid) -> bool {
         true
     }
 
@@ -209,7 +209,7 @@ impl ValueSetT for ValueSetPublicBinary {
         self.map.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::PublicBinary(t) => self.map.remove(t.as_str()).is_some(),
             _ => false,

server/lib/src/valueset/bool.rs

@@ -58,7 +58,7 @@ impl ValueSetT for ValueSetBool {
         self.set.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::Bool(u) => self.set.remove(u),
             _ => {

server/lib/src/valueset/cid.rs

@@ -65,7 +65,7 @@ impl ValueSetT for ValueSetCid {
         self.set.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::Cid(u) => self.set.remove(u),
             _ => {

server/lib/src/valueset/cred.rs

@@ -83,7 +83,7 @@ impl ValueSetT for ValueSetCredential {
         self.map.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::Cred(t) => self.map.remove(t.as_str()).is_some(),
             _ => false,
@@ -338,13 +338,18 @@ impl ValueSetT for ValueSetIntentToken {
         self.map.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::IntentToken(u) => self.map.remove(u).is_some(),
             _ => false,
         }
     }
 
+    fn purge(&mut self, _cid: &Cid) -> bool {
+        // Could consider making this a TS capable entry.
+        true
+    }
+
     fn contains(&self, pv: &PartialValue) -> bool {
         match pv {
             PartialValue::IntentToken(u) => self.map.contains_key(u),
@@ -511,6 +516,11 @@ impl ValueSetT for ValueSetIntentToken {
         }
     }
 
+    fn repl_merge_valueset(&self, _older: &ValueSet, _trim_cid: &Cid) -> Option<ValueSet> {
+        // Im not sure this actually needs repl handling ...
+        None
+    }
+
     fn as_intenttoken_map(&self) -> Option<&BTreeMap<String, IntentTokenState>> {
         Some(&self.map)
     }
@@ -582,7 +592,7 @@ impl ValueSetT for ValueSetPasskey {
         self.map.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::Passkey(u) => self.map.remove(u).is_some(),
             _ => false,
@@ -766,7 +776,7 @@ impl ValueSetT for ValueSetDeviceKey {
         self.map.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::DeviceKey(u) => self.map.remove(u).is_some(),
             _ => false,

server/lib/src/valueset/datetime.rs

@@ -73,7 +73,7 @@ impl ValueSetT for ValueSetDateTime {
         self.set.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::DateTime(u) => self.set.remove(u),
             _ => false,

server/lib/src/valueset/eckey.rs

@@ -1,7 +1,7 @@
 use std::iter::{self};
 
 use crate::be::dbvalue::DbValueSetV2;
-use crate::prelude::ValueSetT;
+use crate::prelude::*;
 use crate::repl::proto::ReplAttrV1;
 use crate::value::{PartialValue, SyntaxType, Value};
 use kanidm_proto::v1::OperationError;
@@ -90,7 +90,7 @@ impl ValueSetT for ValueSetEcKeyPrivate {
         self.set = None;
     }
 
-    fn remove(&mut self, _pv: &crate::value::PartialValue) -> bool {
+    fn remove(&mut self, _pv: &crate::value::PartialValue, _cid: &Cid) -> bool {
         false
     }
 

server/lib/src/valueset/iname.rs

@@ -58,7 +58,7 @@ impl ValueSetT for ValueSetIname {
         self.set.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::Iname(s) => self.set.remove(s),
             _ => {

server/lib/src/valueset/index.rs

@@ -59,7 +59,7 @@ impl ValueSetT for ValueSetIndex {
         self.set.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::Index(u) => self.set.remove(u),
             _ => {

server/lib/src/valueset/iutf8.rs

@@ -59,7 +59,7 @@ impl ValueSetT for ValueSetIutf8 {
         self.set.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::Iutf8(s) => self.set.remove(s),
             _ => {

server/lib/src/valueset/json.rs

@@ -65,7 +65,7 @@ impl ValueSetT for ValueSetJsonFilter {
         self.set.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::JsonFilt(u) => self.set.remove(u),
             _ => {

server/lib/src/valueset/jws.rs

@@ -79,7 +79,7 @@ impl ValueSetT for ValueSetJwsKeyEs256 {
         self.set.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::Iutf8(kid) => {
                 let x = self.set.len();
@@ -263,7 +263,7 @@ impl ValueSetT for ValueSetJwsKeyRs256 {
         self.set.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::Iutf8(kid) => {
                 let x = self.set.len();

server/lib/src/valueset/mod.rs

@@ -21,9 +21,9 @@ use crate::prelude::*;
 use crate::repl::{cid::Cid, proto::ReplAttrV1};
 use crate::schema::SchemaAttribute;
 use crate::value::{Address, ApiToken, IntentTokenState, Oauth2Session, Session};
-use crate::valueset::auditlogstring::ValueSetAuditLogString;
 
 pub use self::address::{ValueSetAddress, ValueSetEmailAddress};
+pub use self::auditlogstring::{ValueSetAuditLogString, AUDIT_LOG_STRING_CAPACITY};
 pub use self::binary::{ValueSetPrivateBinary, ValueSetPublicBinary};
 pub use self::bool::ValueSetBool;
 pub use self::cid::ValueSetCid;
@@ -87,7 +87,16 @@ pub trait ValueSetT: std::fmt::Debug + DynClone {
 
     fn clear(&mut self);
 
-    fn remove(&mut self, pv: &PartialValue) -> bool;
+    fn remove(&mut self, pv: &PartialValue, cid: &Cid) -> bool;
+
+    fn purge(&mut self, _cid: &Cid) -> bool {
+        // Default handling is true.
+        true
+    }
+
+    fn trim(&mut self, _trim_cid: &Cid) {
+        // default to a no-op
+    }
 
     fn contains(&self, pv: &PartialValue) -> bool;
 
@@ -543,7 +552,7 @@ pub trait ValueSetT: std::fmt::Debug + DynClone {
         None
     }
 
-    fn as_audit_log_string(&self) -> Option<&SmolSet<[(Cid, String); 8]>> {
+    fn as_audit_log_string(&self) -> Option<&BTreeMap<Cid, String>> {
         debug_assert!(false);
         None
     }
@@ -556,7 +565,7 @@ pub trait ValueSetT: std::fmt::Debug + DynClone {
     fn repl_merge_valueset(
         &self,
         _older: &ValueSet,
-        // schema_attr: &SchemaAttribute
+        _trim_cid: &Cid, // schema_attr: &SchemaAttribute
     ) -> Option<ValueSet> {
         // Self is the "latest" content. Older contains the earlier
         // state of the attribute.
@@ -790,7 +799,7 @@ pub fn from_repl_v1(rv1: &ReplAttrV1) -> Result<ValueSet, OperationError> {
         ReplAttrV1::Session { set } => ValueSetSession::from_repl_v1(set),
         ReplAttrV1::ApiToken { set } => ValueSetApiToken::from_repl_v1(set),
         ReplAttrV1::TotpSecret { set } => ValueSetTotpSecret::from_repl_v1(set),
-        ReplAttrV1::AuditLogString { set } => ValueSetAuditLogString::from_repl_v1(set),
+        ReplAttrV1::AuditLogString { map } => ValueSetAuditLogString::from_repl_v1(map),
         ReplAttrV1::EcKeyPrivate { key } => ValueSetEcKeyPrivate::from_repl_v1(key),
     }
 }

server/lib/src/valueset/nsuniqueid.rs

@@ -59,7 +59,7 @@ impl ValueSetT for ValueSetNsUniqueId {
         self.set.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::Nsuniqueid(u) => self.set.remove(u),
             _ => {

server/lib/src/valueset/oauth.rs

@@ -61,7 +61,7 @@ impl ValueSetT for ValueSetOauthScope {
         self.set.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::OauthScope(s) => self.set.remove(s.as_str()),
             _ => {
@@ -233,7 +233,7 @@ impl ValueSetT for ValueSetOauthScopeMap {
         self.map.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::Refer(u) => self.map.remove(u).is_some(),
             _ => false,

server/lib/src/valueset/restricted.rs

@@ -58,7 +58,7 @@ impl ValueSetT for ValueSetRestricted {
         self.set.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::RestrictedString(s) => self.set.remove(s),
             _ => {

server/lib/src/valueset/secret.rs

@@ -58,7 +58,7 @@ impl ValueSetT for ValueSetSecret {
         self.set.clear();
     }
 
-    fn remove(&mut self, _pv: &PartialValue) -> bool {
+    fn remove(&mut self, _pv: &PartialValue, _cid: &Cid) -> bool {
         false
     }
 

server/lib/src/valueset/spn.rs

@@ -58,7 +58,7 @@ impl ValueSetT for ValueSetSpn {
         self.set.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::Spn(n, d) => self.set.remove(&(n.clone(), d.clone())),
             _ => {

server/lib/src/valueset/ssh.rs

@@ -69,7 +69,7 @@ impl ValueSetT for ValueSetSshKey {
         self.map.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::SshKey(t) => self.map.remove(t.as_str()).is_some(),
             _ => false,

server/lib/src/valueset/syntax.rs

@@ -59,7 +59,7 @@ impl ValueSetT for ValueSetSyntax {
         self.set.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::Syntax(u) => self.set.remove(u),
             _ => {

server/lib/src/valueset/totp.rs

@@ -80,7 +80,7 @@ impl ValueSetT for ValueSetTotpSecret {
         self.map.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::Utf8(l) => self.map.remove(l.as_str()).is_some(),
             _ => false,

server/lib/src/valueset/uihint.rs

@@ -48,7 +48,7 @@ impl ValueSetT for ValueSetUiHint {
         self.set.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::UiHint(s) => self.set.remove(s),
             _ => {

server/lib/src/valueset/uint32.rs

@@ -58,7 +58,7 @@ impl ValueSetT for ValueSetUint32 {
         self.set.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::Uint32(u) => self.set.remove(u),
             _ => {

server/lib/src/valueset/url.rs

@@ -58,7 +58,7 @@ impl ValueSetT for ValueSetUrl {
         self.set.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::Url(u) => self.set.remove(u),
             _ => false,

server/lib/src/valueset/utf8.rs

@@ -44,7 +44,7 @@ impl ValueSetT for ValueSetUtf8 {
         self.set.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::Utf8(s) => self.set.remove(s),
             _ => {

server/lib/src/valueset/uuid.rs

@@ -60,7 +60,7 @@ impl ValueSetT for ValueSetUuid {
         self.set.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::Uuid(u) => self.set.remove(u),
             _ => {
@@ -220,7 +220,7 @@ impl ValueSetT for ValueSetRefer {
         self.set.clear();
     }
 
-    fn remove(&mut self, pv: &PartialValue) -> bool {
+    fn remove(&mut self, pv: &PartialValue, _cid: &Cid) -> bool {
         match pv {
             PartialValue::Refer(u) => self.set.remove(u),
             _ => {

server/testkit/tests/default_entries.rs

@@ -28,9 +28,10 @@ static SELF_WRITEABLE_ATTRS: [&str; 7] = [
     "displayname",
     "legalname",
     "radius_secret",
-    "primary_credential",
     ATTR_LDAP_SSH_PUBLICKEY,
     "unix_password",
+    // Must be last - changing credential invalidates auth sessions!
+    "primary_credential",
 ];
 static DEFAULT_HP_GROUP_NAMES: [&str; 24] = [
     "idm_admins",
@@ -117,9 +118,10 @@ async fn test_default_entries_rbac_account_managers(rsclient: KanidmClient) {
     static ACCOUNT_MANAGER_ATTRS: [&str; 5] = [
         "name",
         "displayname",
-        "primary_credential",
         ATTR_LDAP_SSH_PUBLICKEY,
         "mail",
+        // Must be last, writing to this invalidates sessions.
+        "primary_credential",
     ];
     test_write_attrs(
         &rsclient,
@@ -420,7 +422,7 @@ async fn test_default_entries_rbac_people_managers(rsclient: KanidmClient) {
 
     static PEOPLE_MANAGER_ATTRS: [&str; 2] = ["legalname", "mail"];
 
-    static TECHNICAL_ATTRS: [&str; 3] = ["primary_credential", "radius_secret", "unix_password"];
+    static TECHNICAL_ATTRS: [&str; 3] = ["radius_secret", "unix_password", "primary_credential"];
     test_read_attrs(
         &rsclient,
         NOT_ADMIN_TEST_USERNAME,

server/testkit/tests/proto_v1_test.rs

@@ -1447,6 +1447,12 @@ async fn test_server_user_auth_token_lifecycle(rsclient: KanidmClient) {
         .await
         .expect("Failed to destroy user auth token");
 
+    // Since the session is revoked, check with the admin.
+    let res = rsclient
+        .auth_simple_password("admin", ADMIN_TEST_PASSWORD)
+        .await;
+    assert!(res.is_ok());
+
     let tokens = rsclient
         .idm_service_account_list_api_token("demo_account")
         .await