Improve token readability, Fix issue with spn format (#773)

kanidm_book/src/pam_and_nsswitch.md

@@ -164,7 +164,7 @@ The content should look like:
     account    sufficient    pam_unix.so
     account    [default=1 ignore=ignore success=ok]  pam_succeed_if.so uid >= 1000 quiet_success quiet_fail
     account    sufficient    pam_kanidm.so ignore_unknown_user
-    account    pam_deny.so
+    account    required      pam_deny.so
 
     # /etc/pam.d/common-password-pc
     # Controls flow of what happens when a user invokes the passwd command. Currently does NOT

kanidmd/Dockerfile

@@ -48,7 +48,7 @@ RUN if [ "${SCCACHE_REDIS}" != "" ]; \
 FROM ${BASE_IMAGE}
 LABEL maintainer william@blackhats.net.au
 
-RUN zypper ref
+RUN zypper refresh
 RUN zypper dup -y
 RUN zypper install -y \
         timezone \

kanidmd/idm/src/actors/v1_write.rs

@@ -750,7 +750,7 @@ impl QueryServerWriteV1 {
                     e
                 })
                 .map(|tok| CUIntentToken {
-                    intent_token: tok.token_enc,
+                    intent_token: tok.intent_id,
                 })
         });
         res
@@ -771,7 +771,7 @@ impl QueryServerWriteV1 {
         let mut idms_prox_write = self.idms.proxy_write_async(ct).await;
         let res = spanned!("actors::v1_write::handle<IdmCredentialExchangeIntent>", {
             let intent_token = CredentialUpdateIntentToken {
-                token_enc: intent_token.intent_token,
+                intent_id: intent_token.intent_token,
             };
 
             idms_prox_write

kanidmd/idm/src/be/dbentry.rs

@@ -25,6 +25,18 @@ pub enum DbEntryVers {
     V2(DbEntryV2),
 }
 
+#[derive(Serialize, Deserialize, Debug)]
+// This doesn't need a version since uuid2spn is reindexed - remember if you change this
+// though, to change the index version!
+pub enum DbIdentSpn {
+    #[serde(rename = "SP")]
+    Spn(String, String),
+    #[serde(rename = "N8")]
+    Iname(String),
+    #[serde(rename = "UU")]
+    Uuid(Uuid),
+}
+
 // This is actually what we store into the DB.
 #[derive(Serialize, Deserialize)]
 pub struct DbEntry {
@@ -375,7 +387,7 @@ fn from_vec_dbval1(attr_val: Vec<DbValueV1>) -> Result<DbValueSetV2, OperationEr
             let vs: Result<Vec<_>, _> = viter
                 .map(|dbv| {
                     if let DbValueV1::IntentToken { u, s } = dbv {
-                        Ok((u, s))
+                        Ok((u.as_hyphenated().to_string(), s))
                     } else {
                         Err(OperationError::InvalidValueState)
                     }

kanidmd/idm/src/be/dbvalue.rs

@@ -33,11 +33,15 @@ impl std::fmt::Debug for DbPasswordV1 {
 #[derive(Serialize, Deserialize, Debug)]
 pub enum DbValueIntentTokenStateV1 {
     #[serde(rename = "v")]
-    Valid,
+    Valid { max_ttl: Duration },
     #[serde(rename = "p")]
-    InProgress(Uuid, Duration),
+    InProgress {
+        max_ttl: Duration,
+        session_id: Uuid,
+        session_ttl: Duration,
+    },
     #[serde(rename = "c")]
-    Consumed,
+    Consumed { max_ttl: Duration },
 }
 
 #[derive(Serialize, Deserialize, Debug)]
@@ -307,7 +311,7 @@ pub enum DbValueSetV2 {
     #[serde(rename = "RS")]
     RestrictedString(Vec<String>),
     #[serde(rename = "IT")]
-    IntentToken(Vec<(Uuid, DbValueIntentTokenStateV1)>),
+    IntentToken(Vec<(String, DbValueIntentTokenStateV1)>),
     #[serde(rename = "TE")]
     TrustedDeviceEnrollment(Vec<Uuid>),
     #[serde(rename = "AS")]

kanidmd/idm/src/be/idl_sqlite.rs

@@ -1,10 +1,10 @@
 use crate::be::dbentry::DbEntry;
-use crate::be::dbvalue::DbValueSetV2;
+use crate::be::dbentry::DbIdentSpn;
 use crate::be::{BackendConfig, IdList, IdRawEntry, IdxKey, IdxSlope};
 use crate::entry::{Entry, EntryCommitted, EntrySealed};
 use crate::prelude::*;
 use crate::value::{IndexType, Value};
-use crate::valueset;
+// use crate::valueset;
 use hashbrown::HashMap;
 use idlset::v2::IDLBitRange;
 use kanidm_proto::v1::{ConsistencyError, OperationError};
@@ -290,12 +290,10 @@ pub trait IdlSqliteTransaction {
 
             let spn: Option<Value> = match spn_raw {
                 Some(d) => {
-                    let dbv: DbValueSetV2 =
+                    let dbv: DbIdentSpn =
                         serde_json::from_slice(d.as_slice()).map_err(serde_json_error)?;
 
-                    valueset::from_db_valueset_v2(dbv)
+                    Some(Value::from(dbv))
-                        .map_err(|_| OperationError::CorruptedIndex("uuid2spn".to_string()))
-                        .map(|vs| vs.to_value_single())?
                 }
                 None => None,
             };
@@ -396,7 +394,6 @@ pub trait IdlSqliteTransaction {
                 row.get(0)
             })
             .optional()
-            // this whole `map` call is useless
             .map(|e_opt| {
                 // If we have a row, we try to make it a sid
                 e_opt.map(|e| {
@@ -883,7 +880,7 @@ impl IdlSqliteWriteTransaction {
         let uuids = uuid.as_hyphenated().to_string();
         match k {
             Some(k) => {
-                let dbv1 = k.to_supplementary_db_valuev1();
+                let dbv1: DbIdentSpn = k.to_db_ident_spn();
                 let data = serde_json::to_vec(&dbv1).map_err(serde_json_error)?;
                 self.conn
                     .prepare("INSERT OR REPLACE INTO idx_uuid2spn (uuid, spn) VALUES(:uuid, :spn)")

kanidmd/idm/src/be/mod.rs

@@ -2232,17 +2232,17 @@ mod tests {
             // Test that on entry create, the indexes are made correctly.
             // this is a similar case to reindex.
             let mut e1: Entry<EntryInit, EntryNew> = Entry::new();
-            e1.add_ava("name", Value::from("william"));
+            e1.add_ava("name", Value::new_iname("william"));
             e1.add_ava("uuid", Value::from("db237e8a-0079-4b8c-8a56-593b22aa44d1"));
             let e1 = unsafe { e1.into_sealed_new() };
 
             let mut e2: Entry<EntryInit, EntryNew> = Entry::new();
-            e2.add_ava("name", Value::from("claire"));
+            e2.add_ava("name", Value::new_iname("claire"));
             e2.add_ava("uuid", Value::from("bd651620-00dd-426b-aaa0-4494f7b7906f"));
             let e2 = unsafe { e2.into_sealed_new() };
 
             let mut e3: Entry<EntryInit, EntryNew> = Entry::new();
-            e3.add_ava("userid", Value::from("lucy"));
+            e3.add_ava("userid", Value::new_iname("lucy"));
             e3.add_ava("uuid", Value::from("7b23c99d-c06b-4a9a-a958-3afa56383e1d"));
             let e3 = unsafe { e3.into_sealed_new() };
 
@@ -2274,7 +2274,7 @@ mod tests {
             assert!(be.name2uuid("claire") == Ok(Some(claire_uuid)));
             let x = be.uuid2spn(claire_uuid);
             trace!(?x);
-            assert!(be.uuid2spn(claire_uuid) == Ok(Some(Value::from("claire"))));
+            assert!(be.uuid2spn(claire_uuid) == Ok(Some(Value::new_iname("claire"))));
             assert!(be.uuid2rdn(claire_uuid) == Ok(Some("name=claire".to_string())));
 
             assert!(be.name2uuid("william") == Ok(None));
@@ -2295,7 +2295,7 @@ mod tests {
             // us. For the test to be "accurate" we must add one attr, remove one attr
             // and change one attr.
             let mut e1: Entry<EntryInit, EntryNew> = Entry::new();
-            e1.add_ava("name", Value::from("william"));
+            e1.add_ava("name", Value::new_iname("william"));
             e1.add_ava("uuid", Value::from("db237e8a-0079-4b8c-8a56-593b22aa44d1"));
             e1.add_ava("ta", Value::from("test"));
             let e1 = unsafe { e1.into_sealed_new() };
@@ -2310,7 +2310,7 @@ mod tests {
             ce1.purge_ava("ta");
             // mod something.
             ce1.purge_ava("name");
-            ce1.add_ava("name", Value::from("claire"));
+            ce1.add_ava("name", Value::new_iname("claire"));
 
             let ce1 = unsafe { ce1.into_sealed_committed() };
 
@@ -2329,7 +2329,7 @@ mod tests {
             let william_uuid = Uuid::parse_str("db237e8a-0079-4b8c-8a56-593b22aa44d1").unwrap();
             assert!(be.name2uuid("william") == Ok(None));
             assert!(be.name2uuid("claire") == Ok(Some(william_uuid)));
-            assert!(be.uuid2spn(william_uuid) == Ok(Some(Value::from("claire"))));
+            assert!(be.uuid2spn(william_uuid) == Ok(Some(Value::new_iname("claire"))));
             assert!(be.uuid2rdn(william_uuid) == Ok(Some("name=claire".to_string())));
         })
     }
@@ -2342,7 +2342,7 @@ mod tests {
             // This will be needing to be correct for conflicts when we add
             // replication support!
             let mut e1: Entry<EntryInit, EntryNew> = Entry::new();
-            e1.add_ava("name", Value::from("william"));
+            e1.add_ava("name", Value::new_iname("william"));
             e1.add_ava("uuid", Value::from("db237e8a-0079-4b8c-8a56-593b22aa44d1"));
             let e1 = unsafe { e1.into_sealed_new() };
 
@@ -2352,7 +2352,7 @@ mod tests {
             let mut ce1 = unsafe { rset[0].as_ref().clone().into_invalid() };
             ce1.purge_ava("name");
             ce1.purge_ava("uuid");
-            ce1.add_ava("name", Value::from("claire"));
+            ce1.add_ava("name", Value::new_iname("claire"));
             ce1.add_ava("uuid", Value::from("04091a7a-6ce4-42d2-abf5-c2ce244ac9e8"));
             let ce1 = unsafe { ce1.into_sealed_committed() };
 
@@ -2386,7 +2386,7 @@ mod tests {
             assert!(be.name2uuid("claire") == Ok(Some(claire_uuid)));
             assert!(be.uuid2spn(william_uuid) == Ok(None));
             assert!(be.uuid2rdn(william_uuid) == Ok(None));
-            assert!(be.uuid2spn(claire_uuid) == Ok(Some(Value::from("claire"))));
+            assert!(be.uuid2spn(claire_uuid) == Ok(Some(Value::new_iname("claire"))));
             assert!(be.uuid2rdn(claire_uuid) == Ok(Some("name=claire".to_string())));
         })
     }
@@ -2398,14 +2398,14 @@ mod tests {
 
             // Create a test entry with some indexed / unindexed values.
             let mut e1: Entry<EntryInit, EntryNew> = Entry::new();
-            e1.add_ava("name", Value::from("william"));
+            e1.add_ava("name", Value::new_iname("william"));
             e1.add_ava("uuid", Value::from("db237e8a-0079-4b8c-8a56-593b22aa44d1"));
             e1.add_ava("no-index", Value::from("william"));
             e1.add_ava("other-no-index", Value::from("william"));
             let e1 = unsafe { e1.into_sealed_new() };
 
             let mut e2: Entry<EntryInit, EntryNew> = Entry::new();
-            e2.add_ava("name", Value::from("claire"));
+            e2.add_ava("name", Value::new_iname("claire"));
             e2.add_ava("uuid", Value::from("db237e8a-0079-4b8c-8a56-593b22aa44d2"));
             let e2 = unsafe { e2.into_sealed_new() };
 
@@ -2706,21 +2706,21 @@ mod tests {
         run_test!(|be: &mut BackendWriteTransaction| {
             // Create some test entry with some indexed / unindexed values.
             let mut e1: Entry<EntryInit, EntryNew> = Entry::new();
-            e1.add_ava("name", Value::from("william"));
+            e1.add_ava("name", Value::new_iname("william"));
             e1.add_ava("uuid", Value::from("db237e8a-0079-4b8c-8a56-593b22aa44d1"));
             e1.add_ava("ta", Value::from("dupe"));
             e1.add_ava("tb", Value::from("1"));
             let e1 = unsafe { e1.into_sealed_new() };
 
             let mut e2: Entry<EntryInit, EntryNew> = Entry::new();
-            e2.add_ava("name", Value::from("claire"));
+            e2.add_ava("name", Value::new_iname("claire"));
             e2.add_ava("uuid", Value::from("db237e8a-0079-4b8c-8a56-593b22aa44d2"));
             e2.add_ava("ta", Value::from("dupe"));
             e2.add_ava("tb", Value::from("1"));
             let e2 = unsafe { e2.into_sealed_new() };
 
             let mut e3: Entry<EntryInit, EntryNew> = Entry::new();
-            e3.add_ava("name", Value::from("benny"));
+            e3.add_ava("name", Value::new_iname("benny"));
             e3.add_ava("uuid", Value::from("db237e8a-0079-4b8c-8a56-593b22aa44d3"));
             e3.add_ava("ta", Value::from("dupe"));
             e3.add_ava("tb", Value::from("2"));
@@ -2899,7 +2899,7 @@ mod tests {
             lim_deny.search_max_filter_test = 0;
 
             let mut e: Entry<EntryInit, EntryNew> = Entry::new();
-            e.add_ava("name", Value::from("william"));
+            e.add_ava("name", Value::new_iname("william"));
             e.add_ava("uuid", Value::from("db237e8a-0079-4b8c-8a56-593b22aa44d1"));
             e.add_ava("nonexist", Value::from("x"));
             e.add_ava("nonexist", Value::from("y"));

kanidmd/idm/src/constants/mod.rs

@@ -13,7 +13,7 @@ pub use crate::constants::system_config::*;
 pub use crate::constants::uuids::*;
 
 // Increment this as we add new schema types and values!!!
-pub const SYSTEM_INDEX_VERSION: i64 = 22;
+pub const SYSTEM_INDEX_VERSION: i64 = 23;
 // On test builds, define to 60 seconds
 #[cfg(test)]
 pub const PURGE_FREQUENCY: u64 = 60;

kanidmd/idm/src/constants/schema.rs

@@ -844,7 +844,9 @@ pub const JSON_SCHEMA_ATTR_CREDENTIAL_UPDATE_INTENT_TOKEN: &str = r#"{
       "description": [
         "The status of a credential update intent token"
       ],
-      "index": [],
+      "index": [
+        "EQUALITY"
+      ],
       "unique": [
         "false"
       ],

kanidmd/idm/src/entry.rs

@@ -1675,7 +1675,7 @@ impl<VALID, STATE> Entry<VALID, STATE> {
     pub fn get_ava_as_intenttokens(
         &self,
         attr: &str,
-    ) -> Option<&std::collections::BTreeMap<Uuid, IntentTokenState>> {
+    ) -> Option<&std::collections::BTreeMap<String, IntentTokenState>> {
         self.attrs.get(attr).and_then(|vs| vs.as_intenttoken_map())
     }
 

kanidmd/idm/src/idm/account.rs

@@ -122,7 +122,7 @@ pub(crate) struct Account {
     // to include these.
     pub mail_primary: Option<String>,
     pub mail: Vec<String>,
-    pub credential_update_intent_tokens: BTreeMap<Uuid, IntentTokenState>,
+    pub credential_update_intent_tokens: BTreeMap<String, IntentTokenState>,
 }
 
 impl Account {

kanidmd/idm/src/idm/credupdatesession.rs

@@ -11,7 +11,7 @@ use crate::credential::totp::{Totp, TOTP_DEFAULT_STEP};
 
 use kanidm_proto::v1::{CURegState, CUStatus, CredentialDetail, PasswordFeedback, TotpSecret};
 
-use crate::utils::{backup_code_from_random, uuid_from_duration};
+use crate::utils::{backup_code_from_random, readable_password_from_random, uuid_from_duration};
 
 use serde::{Deserialize, Serialize};
 
@@ -36,20 +36,9 @@ pub enum PasswordQuality {
     Feedback(Vec<PasswordFeedback>),
 }
 
-#[derive(Serialize, Deserialize, Debug)]
-struct CredentialUpdateIntentTokenInner {
-    pub sessionid: Uuid,
-    // Who is it targeting?
-    pub target: Uuid,
-    // Id of the intent, for checking if it's already been used against this user.
-    pub uuid: Uuid,
-    // How long is it valid for?
-    pub max_ttl: Duration,
-}
-
 #[derive(Clone, Debug)]
 pub struct CredentialUpdateIntentToken {
-    pub token_enc: String,
+    pub intent_id: String,
 }
 
 #[derive(Serialize, Deserialize, Debug)]
@@ -87,7 +76,7 @@ pub(crate) struct CredentialUpdateSession {
     // Current credentials - these are on the Account!
     account: Account,
     //
-    intent_token_id: Option<Uuid>,
+    intent_token_id: Option<String>,
     // Acc policy
     // The credentials as they are being updated
     primary: Option<Credential>,
@@ -292,7 +281,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
     fn create_credupdate_session(
         &mut self,
         sessionid: Uuid,
-        intent_token_id: Option<Uuid>,
+        intent_token_id: Option<String>,
         account: Account,
         ct: Duration,
     ) -> Result<CredentialUpdateSessionToken, OperationError> {
@@ -344,15 +333,14 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
             // Build the intent token.
             let mttl = event.max_ttl.unwrap_or_else(|| Duration::new(0, 0));
             let max_ttl = ct + mttl.clamp(MINIMUM_INTENT_TTL, MAXIMUM_INTENT_TTL);
-            let sessionid = uuid_from_duration(max_ttl, self.sid);
+            // let sessionid = uuid_from_duration(max_ttl, self.sid);
-            let uuid = Uuid::new_v4();
+            let intent_id = readable_password_from_random();
-
-            let target = event.target;
 
+            /*
             let token = CredentialUpdateIntentTokenInner {
                 sessionid,
                 target,
-                uuid,
+                intent_id,
                 max_ttl,
             };
 
@@ -364,11 +352,38 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
             let token_enc = self
                 .token_enc_key
                 .encrypt_at_time(&token_data, ct.as_secs());
+            */
 
             // Mark that we have created an intent token on the user.
-            let modlist = ModifyList::new_append(
+            // ⚠️   -- remember, there is a risk, very low, but still a risk of collision of the intent_id.
+            //        instead of enforcing unique, which would divulge that the collision occured, we
+            //        write anyway, and instead on the intent access path we invalidate IF the collision
+            //        occurs.
+            let mut modlist = ModifyList::new_append(
                 "credential_update_intent_token",
-                Value::IntentToken(token.sessionid, IntentTokenState::Valid),
+                Value::IntentToken(intent_id.clone(), IntentTokenState::Valid { max_ttl }),
+            );
+
+            // Remove any old credential update intents
+            account.credential_update_intent_tokens.iter().for_each(
+                |(existing_intent_id, state)| {
+                    let max_ttl = match state {
+                        IntentTokenState::Valid { max_ttl }
+                        | IntentTokenState::InProgress {
+                            max_ttl,
+                            session_id: _,
+                            session_ttl: _,
+                        }
+                        | IntentTokenState::Consumed { max_ttl } => *max_ttl,
+                    };
+
+                    if ct >= max_ttl {
+                        modlist.push_mod(Modify::Removed(
+                            AttrString::from("credential_update_intent_token"),
+                            PartialValue::IntentToken(existing_intent_id.clone()),
+                        ));
+                    }
+                },
             );
 
             self.qs_write
@@ -382,7 +397,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
                     e
                 })?;
 
-            Ok(CredentialUpdateIntentToken { token_enc })
+            Ok(CredentialUpdateIntentToken { intent_id })
         })
     }
 
@@ -391,28 +406,71 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
         token: CredentialUpdateIntentToken,
         ct: Duration,
     ) -> Result<CredentialUpdateSessionToken, OperationError> {
-        let token: CredentialUpdateIntentTokenInner = self
+        let CredentialUpdateIntentToken { intent_id } = token;
-            .token_enc_key
-            .decrypt(&token.token_enc)
-            .map_err(|e| {
-                admin_error!(?e, "Failed to decrypt intent request");
-                OperationError::SessionExpired
-            })
-            .and_then(|data| {
-                serde_json::from_slice(&data).map_err(|e| {
-                    admin_error!(err = ?e, "Failed to deserialise intent request");
-                    OperationError::SerdeJsonError
-                })
-            })?;
-
-        // Check the TTL
-        if ct >= token.max_ttl {
-            trace!(?ct, ?token.max_ttl);
-            security_info!(%token.sessionid, "session expired");
-            return Err(OperationError::SessionExpired);
-        }
 
+        /*
             let entry = self.qs_write.internal_search_uuid(&token.target)?;
+        */
+        // ⚠️  due to a low, but possible risk of intent_id collision, if there are multiple
+        // entries, we will reject the intent.
+        // DO we need to force both to "Consumed" in this step?
+        //
+        // ⚠️  If not present, it may be due to replication delay. We can report this.
+
+        let mut vs = self.qs_write.internal_search(filter!(f_eq(
+            "credential_update_intent_token",
+            PartialValue::IntentToken(intent_id.clone())
+        )))?;
+
+        let entry = match vs.pop() {
+            Some(entry) => {
+                if vs.is_empty() {
+                    // Happy Path!
+                    entry
+                } else {
+                    // Multiple entries matched! This is bad!
+                    let matched_uuids = std::iter::once(entry.get_uuid())
+                        .chain(vs.iter().map(|e| e.get_uuid()))
+                        .collect::<Vec<_>>();
+
+                    security_error!("Multiple entries had identical intent_id - for safety, rejecting the use of this intent_id! {:?}", matched_uuids);
+
+                    /*
+                    let mut modlist = ModifyList::new();
+
+                    modlist.push_mod(Modify::Removed(
+                        AttrString::from("credential_update_intent_token"),
+                        PartialValue::IntentToken(intent_id.clone()),
+                    ));
+
+                    let filter_or = matched_uuids.into_iter()
+                        .map(|u| f_eq("uuid", PartialValue::new_uuid(u)))
+                        .collect();
+
+                    self.qs_write
+                        .internal_modify(
+                            // Filter as executed
+                            &filter!(f_or(filter_or)),
+                            &modlist,
+                        )
+                        .map_err(|e| {
+                            request_error!(error = ?e);
+                            e
+                        })?;
+                    */
+
+                    return Err(OperationError::InvalidState);
+                }
+            }
+            None => {
+                security_info!(
+                    "Rejecting Update Session - Intent Token does not exist (replication delay?)",
+                );
+                return Err(OperationError::Wait(
+                    OffsetDateTime::unix_epoch() + (ct + Duration::from_secs(150)),
+                ));
+            }
+        };
 
         // Is target an account? This checks for us.
         let account = Account::try_from_entry_rw(entry.as_ref(), &mut self.qs_write)?;
@@ -420,41 +478,57 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
         // Check there is not already a user session in progress with this intent token.
         // Is there a need to revoke intent tokens?
 
-        match account
+        let max_ttl = match account.credential_update_intent_tokens.get(&intent_id) {
-            .credential_update_intent_tokens
+            Some(IntentTokenState::Consumed { max_ttl: _ }) => {
-            .get(&token.sessionid)
-        {
-            Some(IntentTokenState::Consumed) => {
                 security_info!(
                     ?entry,
-                    %token.target,
+                    %account.uuid,
                     "Rejecting Update Session - Intent Token has already been exchanged",
                 );
                 return Err(OperationError::SessionExpired);
             }
-            Some(IntentTokenState::InProgress(si, sd)) => {
+            Some(IntentTokenState::InProgress {
-                if ct > *sd {
+                max_ttl,
+                session_id,
+                session_ttl,
+            }) => {
+                if ct > *session_ttl {
                     // The former session has expired, continue.
                     security_info!(
                         ?entry,
-                        %token.target,
+                        %account.uuid,
-                        "Initiating Credential Update Session - Previous session {} has expired", si
+                        "Initiating Credential Update Session - Previous session {} has expired", session_id
                     );
+                    *max_ttl
                 } else {
                     security_info!(
                         ?entry,
-                        %token.target,
+                        %account.uuid,
-                        "Rejecting Update Session - Intent Token is in use {}. Try again later", si
+                        "Rejecting Update Session - Intent Token is in use {}. Try again later", session_id
                     );
-                    return Err(OperationError::Wait(OffsetDateTime::unix_epoch() + *sd));
+                    return Err(OperationError::Wait(
+                        OffsetDateTime::unix_epoch() + *session_ttl,
+                    ));
                 }
             }
-            Some(IntentTokenState::Valid) | None => {
+            Some(IntentTokenState::Valid { max_ttl }) => {
+                // Check the TTL
+                if ct >= *max_ttl {
+                    trace!(?ct, ?max_ttl);
+                    security_info!(%account.uuid, "intent has expired");
+                    return Err(OperationError::SessionExpired);
+                } else {
                     security_info!(
                         ?entry,
-                    %token.target,
+                        %account.uuid,
                         "Initiating Credential Update Session",
                     );
+                    *max_ttl
+                }
+            }
+            None => {
+                admin_error!("Corruption may have occured - index yielded an entry for intent_id, but the entry does not contain that intent_id");
+                return Err(OperationError::InvalidState);
             }
         };
 
@@ -467,19 +541,23 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
         // We need to pin the id from the intent token into the credential to ensure it's not re-used
 
         // Need to change this to the expiry time, so we can purge up to.
-        let sessionid = uuid_from_duration(ct + MAXIMUM_CRED_UPDATE_TTL, self.sid);
+        let session_id = uuid_from_duration(ct + MAXIMUM_CRED_UPDATE_TTL, self.sid);
 
         let mut modlist = ModifyList::new();
 
         modlist.push_mod(Modify::Removed(
             AttrString::from("credential_update_intent_token"),
-            PartialValue::IntentToken(token.sessionid),
+            PartialValue::IntentToken(intent_id.clone()),
         ));
         modlist.push_mod(Modify::Present(
             AttrString::from("credential_update_intent_token"),
             Value::IntentToken(
-                token.sessionid,
+                intent_id.clone(),
-                IntentTokenState::InProgress(sessionid, ct + MAXIMUM_CRED_UPDATE_TTL),
+                IntentTokenState::InProgress {
+                    max_ttl,
+                    session_id,
+                    session_ttl: ct + MAXIMUM_CRED_UPDATE_TTL,
+                },
             ),
         ));
 
@@ -497,7 +575,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
         // ==========
         // Okay, good to exchange.
 
-        self.create_credupdate_session(sessionid, Some(token.sessionid), account, ct)
+        self.create_credupdate_session(session_id, Some(intent_id), account, ct)
     }
 
     pub fn init_credential_update(
@@ -571,6 +649,53 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
 
         // Setup mods for the various bits. We always assert an *exact* state.
 
+        // IF an intent was used on this session, AND that intent is not in our
+        // session state as an exact match, FAIL the commit. Move the intent to "Consumed".
+        //
+        // Should we mark the credential as suspect (lock the account?)
+        //
+        // If the credential has changed, reject? Do we need "asserts" in the modlist?
+        // that would allow better expression of this, and will allow resolving via replication
+
+        // If an intent token was used, remove it's former value, and add it as consumed.
+        if let Some(intent_token_id) = &session.intent_token_id {
+            let entry = self.qs_write.internal_search_uuid(&session.account.uuid)?;
+            let account = Account::try_from_entry_rw(entry.as_ref(), &mut self.qs_write)?;
+
+            let max_ttl = match account.credential_update_intent_tokens.get(intent_token_id) {
+                Some(IntentTokenState::InProgress {
+                    max_ttl,
+                    session_id,
+                    session_ttl: _,
+                }) => {
+                    if *session_id != session_token.sessionid {
+                        security_info!("Session originated from an intent token, but the intent token has initiated a conflicting second update session. Refusing to commit changes.");
+                        return Err(OperationError::InvalidState);
+                    } else {
+                        *max_ttl
+                    }
+                }
+                Some(IntentTokenState::Consumed { max_ttl: _ })
+                | Some(IntentTokenState::Valid { max_ttl: _ })
+                | None => {
+                    security_info!("Session originated from an intent token, but the intent token has transitioned to an invalid state. Refusing to commit changes.");
+                    return Err(OperationError::InvalidState);
+                }
+            };
+
+            modlist.push_mod(Modify::Removed(
+                AttrString::from("credential_update_intent_token"),
+                PartialValue::IntentToken(intent_token_id.clone()),
+            ));
+            modlist.push_mod(Modify::Present(
+                AttrString::from("credential_update_intent_token"),
+                Value::IntentToken(
+                    intent_token_id.clone(),
+                    IntentTokenState::Consumed { max_ttl },
+                ),
+            ));
+        };
+
         match &session.primary {
             Some(ncred) => {
                 modlist.push_mod(Modify::Purged(AttrString::from("primary_credential")));
@@ -585,18 +710,6 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
             }
         };
 
-        // If an intent token was used, remove it's former value, and add it as consumed.
-        if let Some(intent_token_id) = session.intent_token_id {
-            modlist.push_mod(Modify::Removed(
-                AttrString::from("credential_update_intent_token"),
-                PartialValue::IntentToken(intent_token_id),
-            ));
-            modlist.push_mod(Modify::Present(
-                AttrString::from("credential_update_intent_token"),
-                Value::IntentToken(intent_token_id, IntentTokenState::Consumed),
-            ));
-        };
-
         // Are any other checks needed?
 
         // Apply to the account!

kanidmd/idm/src/server.rs

@@ -640,13 +640,12 @@ pub trait QueryServerTransaction<'a> {
                     }),
                     SyntaxType::OauthScope => Ok(PartialValue::new_oauthscope(value)),
                     SyntaxType::PrivateBinary => Ok(PartialValue::PrivateBinary),
-                    SyntaxType::IntentToken => {
+                    SyntaxType::IntentToken => PartialValue::new_intenttoken_s(value.to_string())
-                        PartialValue::new_intenttoken_s(value).ok_or_else(|| {
+                        .ok_or_else(|| {
                             OperationError::InvalidAttribute(
                                 "Invalid Intent Token ID (uuid) syntax".to_string(),
                             )
-                        })
+                        }),
-                    }
                 }
             }
             None => {
@@ -2678,7 +2677,7 @@ impl<'a> QueryServerWriteTransaction<'a> {
                     mut_d_info.d_name,
                 );
                 admin_warn!(
-                    "If you think this is an error, see https://kanidm.github.io/kanidm/administrivia.html#rename-the-domain"
+                    "If you think this is an error, see https://kanidm.github.io/kanidm/stable/administrivia.html#rename-the-domain"
                 );
                 mut_d_info.d_name = domain_name;
             }

kanidmd/idm/src/value.rs

@@ -3,7 +3,7 @@
 //! typed values, allows their comparison, filtering and more. It also has the code for serialising
 //! these into a form for the backend that can be persistent into the [`Backend`](crate::be::Backend).
 
-use crate::be::dbvalue::DbValueV1;
+use crate::be::dbentry::DbIdentSpn;
 use crate::credential::Credential;
 use crate::repl::cid::Cid;
 use kanidm_proto::v1::Filter as ProtoFilter;
@@ -70,9 +70,17 @@ pub enum IndexType {
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum IntentTokenState {
-    Valid,
+    Valid {
-    InProgress(Uuid, Duration),
+        max_ttl: Duration,
-    Consumed,
+    },
+    InProgress {
+        max_ttl: Duration,
+        session_id: Uuid,
+        session_ttl: Duration,
+    },
+    Consumed {
+        max_ttl: Duration,
+    },
 }
 
 impl TryFrom<&str> for IndexType {
@@ -332,7 +340,7 @@ pub enum PartialValue {
     // Enumeration(String),
     // Float64(f64),
     RestrictedString(String),
-    IntentToken(Uuid),
+    IntentToken(String),
     TrustedDeviceEnrollment(Uuid),
     AuthSession(Uuid),
 }
@@ -647,11 +655,8 @@ impl PartialValue {
         PartialValue::RestrictedString(s.to_string())
     }
 
-    pub fn new_intenttoken_s(us: &str) -> Option<Self> {
+    pub fn new_intenttoken_s(s: String) -> Option<Self> {
-        match Uuid::parse_str(us) {
+        Some(PartialValue::IntentToken(s))
-            Ok(u) => Some(PartialValue::IntentToken(u)),
-            Err(_) => None,
-        }
     }
 
     pub fn to_str(&self) -> Option<&str> {
@@ -705,7 +710,7 @@ impl PartialValue {
             PartialValue::OauthScopeMap(u) => u.as_hyphenated().to_string(),
             PartialValue::Address(a) => a.to_string(),
             PartialValue::PhoneNumber(a) => a.to_string(),
-            PartialValue::IntentToken(u) => u.as_hyphenated().to_string(),
+            PartialValue::IntentToken(u) => u.clone(),
             PartialValue::TrustedDeviceEnrollment(u) => u.as_hyphenated().to_string(),
             PartialValue::AuthSession(u) => u.as_hyphenated().to_string(),
         }
@@ -752,7 +757,7 @@ pub enum Value {
     // Enumeration(String),
     // Float64(f64),
     RestrictedString(String),
-    IntentToken(Uuid, IntentTokenState),
+    IntentToken(String, IntentTokenState),
     TrustedDeviceEnrollment(Uuid),
     AuthSession(Uuid),
 }
@@ -881,6 +886,16 @@ impl From<Uuid> for Value {
     }
 }
 
+impl From<DbIdentSpn> for Value {
+    fn from(dis: DbIdentSpn) -> Self {
+        match dis {
+            DbIdentSpn::Spn(n, r) => Value::Spn(n, r),
+            DbIdentSpn::Iname(n) => Value::Iname(n),
+            DbIdentSpn::Uuid(u) => Value::Uuid(u),
+        }
+    }
+}
+
 impl Value {
     // I get the feeling this will have a lot of matching ... sigh.
     pub fn new_utf8(s: String) -> Self {
@@ -1234,15 +1249,15 @@ impl Value {
     }
 
     #[allow(clippy::unreachable)]
-    pub(crate) fn to_supplementary_db_valuev1(&self) -> DbValueV1 {
+    pub(crate) fn to_db_ident_spn(&self) -> DbIdentSpn {
         // This has to clone due to how the backend works.
         match &self {
-            Value::Iname(s) => DbValueV1::Iname(s.clone()),
+            Value::Spn(n, r) => DbIdentSpn::Spn(n.clone(), r.clone()),
-            Value::Utf8(s) => DbValueV1::Utf8(s.clone()),
+            Value::Iname(s) => DbIdentSpn::Iname(s.clone()),
-            Value::Iutf8(s) => DbValueV1::Iutf8(s.clone()),
+            Value::Uuid(u) => DbIdentSpn::Uuid(*u),
-            Value::Uuid(u) => DbValueV1::Uuid(*u),
+            // Value::Iutf8(s) => DbValueV1::Iutf8(s.clone()),
-            Value::Spn(n, r) => DbValueV1::Spn(n.clone(), r.clone()),
+            // Value::Utf8(s) => DbValueV1::Utf8(s.clone()),
-            Value::Nsuniqueid(s) => DbValueV1::NsUniqueId(s.clone()),
+            // Value::Nsuniqueid(s) => DbValueV1::NsUniqueId(s.clone()),
             v => unreachable!("-> {:?}", v),
         }
     }
@@ -1436,7 +1451,7 @@ impl Value {
         }
     }
 
-    pub fn to_intenttoken(self) -> Option<(Uuid, IntentTokenState)> {
+    pub fn to_intenttoken(self) -> Option<(String, IntentTokenState)> {
         match self {
             Value::IntentToken(u, s) => Some((u, s)),
             _ => None,

kanidmd/idm/src/valueset/cred.rs

@@ -1,6 +1,5 @@
 use crate::prelude::*;
 use crate::schema::SchemaAttribute;
-use crate::valueset::uuid_to_proto_string;
 use crate::valueset::DbValueSetV2;
 use crate::valueset::ValueSet;
 use std::collections::btree_map::Entry as BTreeEntry;
@@ -170,34 +169,44 @@ impl ValueSetT for ValueSetCredential {
 
 #[derive(Debug, Clone)]
 pub struct ValueSetIntentToken {
-    map: BTreeMap<Uuid, IntentTokenState>,
+    map: BTreeMap<String, IntentTokenState>,
 }
 
 impl ValueSetIntentToken {
-    pub fn new(t: Uuid, s: IntentTokenState) -> Box<Self> {
+    pub fn new(t: String, s: IntentTokenState) -> Box<Self> {
         let mut map = BTreeMap::new();
         map.insert(t, s);
         Box::new(ValueSetIntentToken { map })
     }
 
-    pub fn push(&mut self, t: Uuid, s: IntentTokenState) -> bool {
+    pub fn push(&mut self, t: String, s: IntentTokenState) -> bool {
         self.map.insert(t, s).is_none()
     }
 
     pub fn from_dbvs2(
-        data: Vec<(Uuid, DbValueIntentTokenStateV1)>,
+        data: Vec<(String, DbValueIntentTokenStateV1)>,
     ) -> Result<ValueSet, OperationError> {
         let map = data
             .into_iter()
-            .map(|(u, dits)| {
+            .map(|(s, dits)| {
                 let ts = match dits {
-                    DbValueIntentTokenStateV1::Valid => IntentTokenState::Valid,
+                    DbValueIntentTokenStateV1::Valid { max_ttl } => {
-                    DbValueIntentTokenStateV1::InProgress(pu, pd) => {
+                        IntentTokenState::Valid { max_ttl }
-                        IntentTokenState::InProgress(pu, pd)
+                    }
+                    DbValueIntentTokenStateV1::InProgress {
+                        max_ttl,
+                        session_id,
+                        session_ttl,
+                    } => IntentTokenState::InProgress {
+                        max_ttl,
+                        session_id,
+                        session_ttl,
+                    },
+                    DbValueIntentTokenStateV1::Consumed { max_ttl } => {
+                        IntentTokenState::Consumed { max_ttl }
                     }
-                    DbValueIntentTokenStateV1::Consumed => IntentTokenState::Consumed,
                 };
-                (u, ts)
+                (s, ts)
             })
             .collect();
         Ok(Box::new(ValueSetIntentToken { map }))
@@ -205,7 +214,7 @@ impl ValueSetIntentToken {
 
     pub fn from_iter<T>(iter: T) -> Option<Box<Self>>
     where
-        T: IntoIterator<Item = (Uuid, IntentTokenState)>,
+        T: IntoIterator<Item = (String, IntentTokenState)>,
     {
         let map = iter.into_iter().collect();
         Some(Box::new(ValueSetIntentToken { map }))
@@ -258,10 +267,7 @@ impl ValueSetT for ValueSetIntentToken {
     }
 
     fn generate_idx_eq_keys(&self) -> Vec<String> {
-        self.map
+        self.map.keys().cloned().collect()
-            .keys()
-            .map(|u| u.as_hyphenated().to_string())
-            .collect()
     }
 
     fn syntax(&self) -> SyntaxType {
@@ -273,11 +279,7 @@ impl ValueSetT for ValueSetIntentToken {
     }
 
     fn to_proto_string_clone_iter(&self) -> Box<dyn Iterator<Item = String> + '_> {
-        Box::new(
+        Box::new(self.map.keys().cloned())
-            self.map
-                .iter()
-                .map(|(u, m)| format!("{}: {:?}", uuid_to_proto_string(*u), m)),
-        )
     }
 
     fn to_db_valueset_v2(&self) -> DbValueSetV2 {
@@ -286,13 +288,23 @@ impl ValueSetT for ValueSetIntentToken {
                 .iter()
                 .map(|(u, s)| {
                     (
-                        *u,
+                        u.clone(),
                         match s {
-                            IntentTokenState::Valid => DbValueIntentTokenStateV1::Valid,
+                            IntentTokenState::Valid { max_ttl } => {
-                            IntentTokenState::InProgress(i, d) => {
+                                DbValueIntentTokenStateV1::Valid { max_ttl: *max_ttl }
-                                DbValueIntentTokenStateV1::InProgress(*i, *d)
+                            }
+                            IntentTokenState::InProgress {
+                                max_ttl,
+                                session_id,
+                                session_ttl,
+                            } => DbValueIntentTokenStateV1::InProgress {
+                                max_ttl: *max_ttl,
+                                session_id: *session_id,
+                                session_ttl: *session_ttl,
+                            },
+                            IntentTokenState::Consumed { max_ttl } => {
+                                DbValueIntentTokenStateV1::Consumed { max_ttl: *max_ttl }
                             }
-                            IntentTokenState::Consumed => DbValueIntentTokenStateV1::Consumed,
                         },
                     )
                 })
@@ -330,7 +342,7 @@ impl ValueSetT for ValueSetIntentToken {
         }
     }
 
-    fn as_intenttoken_map(&self) -> Option<&BTreeMap<Uuid, IntentTokenState>> {
+    fn as_intenttoken_map(&self) -> Option<&BTreeMap<String, IntentTokenState>> {
         Some(&self.map)
     }
 }

kanidmd/idm/src/valueset/mod.rs

@@ -287,10 +287,12 @@ pub trait ValueSetT: std::fmt::Debug + DynClone {
     }
 
     fn as_oauthscopemap(&self) -> Option<&BTreeMap<Uuid, BTreeSet<String>>> {
+        /*
         error!(
             "as_oauthscopemap should not be called on {:?}",
             self.syntax()
         );
+        */
         None
     }
 
@@ -299,7 +301,7 @@ pub trait ValueSetT: std::fmt::Debug + DynClone {
         None
     }
 
-    fn as_intenttoken_map(&self) -> Option<&BTreeMap<Uuid, IntentTokenState>> {
+    fn as_intenttoken_map(&self) -> Option<&BTreeMap<String, IntentTokenState>> {
         debug_assert!(false);
         None
     }