20230128 protected to access (#1349)

Cargo.toml

@@ -143,7 +143,8 @@ tokio-util = "^0.7.4"
 
 toml = "^0.5.11"
 touch = "^0.0.1"
-tracing = { version = "^0.1.37", features = ["max_level_trace", "release_max_level_debug"] }
+# tracing = { version = "^0.1.37", features = ["max_level_trace", "release_max_level_debug"] }
+tracing = { version = "^0.1.37" }
 tracing-subscriber = { version = "^0.3.16", features = ["env-filter"] }
 
 # tracing-forest = { path = "/Users/william/development/tracing-forest/tracing-forest" }

Makefile

@@ -16,7 +16,7 @@ help:
 .PHONY: buildx/kanidmd/x86_64_v3
 buildx/kanidmd/x86_64_v3: ## build multiarch server images
 buildx/kanidmd/x86_64_v3:
-	@$(CONTAINER_TOOL) buildx build $(CONTAINER_TOOL_ARGS) --pull --push --platform "linux/amd64" \
+	@$(CONTAINER_TOOL) buildx build $(CONTAINER_TOOL_ARGS) --pull --push --platform "linux/amd64/v3" \
 		-f kanidmd/Dockerfile -t $(IMAGE_BASE)/server:x86_64_$(IMAGE_VERSION) \
 		--build-arg "KANIDM_BUILD_PROFILE=container_x86_64_v3" \
 		--build-arg "KANIDM_FEATURES=" \

iam_migrations/freeipa/src/main.rs

@@ -715,10 +715,26 @@ fn ipa_to_scim_entry(
 
         let password_import = entry
             .remove_ava_single("ipanthash")
-            .map(|s| format!("ipaNTHash: {}", s));
+            .map(|s| format!("ipaNTHash: {}", s))
+            // If we don't have this, try one of the other hashes that *might* work
+            // The reason we don't do this by default is there are multiple
+            // pw hash formats in 389-ds we don't support!
+            .or_else(|| entry.remove_ava_single("userpassword"));
 
+        let totp_import = if !totp.is_empty() {
+            if password_import.is_some() {
                 // If there are TOTP's, convert them to something sensible.
-        let totp_import = totp.iter().filter_map(ipa_to_totp).collect();
+                totp.iter().filter_map(ipa_to_totp).collect()
+            } else {
+                warn!(
+                    "Skipping totp for {} as password is not available to import.",
+                    dn
+                );
+                Vec::default()
+            }
+        } else {
+            Vec::default()
+        };
 
         let login_shell = entry.remove_ava_single("loginshell");
         let external_id = Some(entry.dn);

kanidm_unix_int/src/tasks_daemon.rs

@@ -87,10 +87,7 @@ fn create_home_directory(
     use_etc_skel: bool,
 ) -> Result<(), String> {
     // Final sanity check to prevent certain classes of attacks.
-    let name = info
+    let name = info.name.trim_start_matches('.').replace(['/', '\\'], "");
-        .name
-        .trim_start_matches('.')
-        .replace(['/', '\\'], "");
 
     let home_prefix_path = Path::new(home_prefix);
 
@@ -151,9 +148,7 @@ fn create_home_directory(
     for alias in info.aliases.iter() {
         // Sanity check the alias.
         // let alias = alias.replace(".", "").replace("/", "").replace("\\", "");
-        let alias = alias
+        let alias = alias.trim_start_matches('.').replace(['/', '\\'], "");
-            .trim_start_matches('.')
-            .replace(['/', '\\'], "");
         let alias_path_raw = format!("{}{}", home_prefix, alias);
         let alias_path = Path::new(&alias_path_raw);
 

kanidmd/core/src/https/middleware.rs

@@ -113,8 +113,6 @@ impl<State: Clone + Send + Sync + 'static> tide::Middleware<State> for StrictRes
 #[derive(Default)]
 struct StrictRequestMiddleware;
 
-
-
 #[async_trait::async_trait]
 impl<State: Clone + Send + Sync + 'static> tide::Middleware<State> for StrictRequestMiddleware {
     async fn handle(

kanidmd/core/src/https/routemaps.rs

@@ -76,14 +76,11 @@ pub struct RouteInfo {
     pub method: http_types::Method,
 }
 
-#[derive(Clone, Debug, Deserialize, Serialize)]
+#[derive(Clone, Debug, Deserialize, Serialize, Default)]
-#[derive(Default)]
 pub struct RouteMap {
     pub routelist: Vec<RouteInfo>,
 }
 
-
-
 impl RouteMap {
     // Serializes the object out to a pretty JSON blob
     pub fn do_map(&self) -> String {

kanidmd/daemon/src/main.rs

@@ -505,9 +505,7 @@ async fn main() {
                     debug!("Request: {req:?}");
                     println!("OK")
                 }
-                KanidmdOpt::Version(_) => {
+                KanidmdOpt::Version(_) => {}
-                    
-                }
             }
         })
         .await;

kanidmd/lib/src/be/mod.rs

@@ -1998,9 +1998,7 @@ mod tests {
             let vr2 = unsafe { r2.into_sealed_committed() };
 
             // Modify single
-            assert!(be
+            assert!(be.modify(&CID_ZERO, &[pre1], &[vr1.clone()]).is_ok());
-                .modify(&CID_ZERO, &[pre1], &[vr1.clone()])
-                .is_ok());
             // Assert no other changes
             assert!(entry_attr_pres!(be, vr1, "desc"));
             assert!(!entry_attr_pres!(be, vr2, "desc"));
@@ -2064,19 +2062,13 @@ mod tests {
             // This sets up the RUV with the changes.
             let r1_ts = unsafe { r1.to_tombstone(CID_ONE.clone()).into_sealed_committed() };
 
-            assert!(be
+            assert!(be.modify(&CID_ONE, &[r1], &[r1_ts.clone()]).is_ok());
-                .modify(&CID_ONE, &[r1], &[r1_ts.clone()])
-                .is_ok());
 
             let r2_ts = unsafe { r2.to_tombstone(CID_TWO.clone()).into_sealed_committed() };
             let r3_ts = unsafe { r3.to_tombstone(CID_TWO.clone()).into_sealed_committed() };
 
             assert!(be
-                .modify(
+                .modify(&CID_TWO, &[r2, r3], &[r2_ts.clone(), r3_ts.clone()])
-                    &CID_TWO,
-                    &[r2, r3],
-                    &[r2_ts.clone(), r3_ts.clone()]
-                )
                 .is_ok());
 
             // The entry are now tombstones, but is still in the ruv. This is because we
@@ -2405,9 +2397,7 @@ mod tests {
 
             // == Now we reap_tombstones, and assert we removed the items.
             let e1_ts = unsafe { e1.to_tombstone(CID_ONE.clone()).into_sealed_committed() };
-            assert!(be
+            assert!(be.modify(&CID_ONE, &[e1], &[e1_ts]).is_ok());
-                .modify(&CID_ONE, &[e1], &[e1_ts])
-                .is_ok());
             be.reap_tombstones(&CID_TWO).unwrap();
 
             idl_state!(be, "name", IndexType::Equality, "william", Some(Vec::new()));
@@ -2453,9 +2443,7 @@ mod tests {
             e3.add_ava("uuid", Value::from("7b23c99d-c06b-4a9a-a958-3afa56383e1d"));
             let e3 = unsafe { e3.into_sealed_new() };
 
-            let mut rset = be
+            let mut rset = be.create(&CID_ZERO, vec![e1, e2, e3]).unwrap();
-                .create(&CID_ZERO, vec![e1, e2, e3])
-                .unwrap();
             rset.remove(1);
             let mut rset: Vec<_> = rset.into_iter().map(Arc::new).collect();
             let e1 = rset.pop().unwrap();
@@ -2464,13 +2452,7 @@ mod tests {
             // Now remove e1, e3.
             let e1_ts = unsafe { e1.to_tombstone(CID_ONE.clone()).into_sealed_committed() };
             let e3_ts = unsafe { e3.to_tombstone(CID_ONE.clone()).into_sealed_committed() };
-            assert!(be
+            assert!(be.modify(&CID_ONE, &[e1, e3], &[e1_ts, e3_ts]).is_ok());
-                .modify(
-                    &CID_ONE,
-                    &[e1, e3],
-                    &[e1_ts, e3_ts]
-                )
-                .is_ok());
             be.reap_tombstones(&CID_TWO).unwrap();
 
             idl_state!(be, "name", IndexType::Equality, "claire", Some(vec![2]));
@@ -2945,9 +2927,7 @@ mod tests {
             e3.add_ava("tb", Value::from("2"));
             let e3 = unsafe { e3.into_sealed_new() };
 
-            let _rset = be
+            let _rset = be.create(&CID_ZERO, vec![e1, e2, e3]).unwrap();
-                .create(&CID_ZERO, vec![e1, e2, e3])
-                .unwrap();
 
             // If the slopes haven't been generated yet, there are some hardcoded values
             // that we can use instead. They aren't generated until a first re-index.

kanidmd/lib/src/constants/schema.rs

@@ -123,6 +123,9 @@ pub const JSON_SCHEMA_ATTR_PRIMARY_CREDENTIAL: &str = r#"
       "multivalue": [
         "false"
       ],
+      "sync_allowed": [
+        "true"
+      ],
       "attributename": [
         "primary_credential"
       ],

kanidmd/lib/src/credential/totp.rs

@@ -367,12 +367,7 @@ mod tests {
     fn totp_allow_one_previous() {
         let key = vec![0x00, 0xaa, 0xbb, 0xcc];
         let secs = 1585369780;
-        let otp = Totp::new(
+        let otp = Totp::new(key, TOTP_DEFAULT_STEP, TotpAlgo::Sha512, TotpDigits::Six);
-            key,
-            TOTP_DEFAULT_STEP,
-            TotpAlgo::Sha512,
-            TotpDigits::Six,
-        );
         let d = Duration::from_secs(secs);
         // Step
         assert!(otp.verify(952181, &d));

kanidmd/lib/src/entry.rs

@@ -559,9 +559,7 @@ impl Entry<EntryInit, EntryNew> {
         let cid = Cid::new_zero();
         self.set_last_changed(cid.clone());
         let eclog = EntryChangelog::new_without_schema(cid, self.attrs.clone());
-        let uuid = self
+        let uuid = self.get_uuid().unwrap_or_else(Uuid::new_v4);
-            .get_uuid()
-            .unwrap_or_else(Uuid::new_v4);
         Entry {
             valid: EntrySealed { uuid, eclog },
             state: EntryCommitted { id: 0 },
@@ -950,9 +948,7 @@ impl Entry<EntryInvalid, EntryNew> {
 
     #[cfg(test)]
     pub unsafe fn into_sealed_committed(self) -> Entry<EntrySealed, EntryCommitted> {
-        let uuid = self
+        let uuid = self.get_uuid().unwrap_or_else(Uuid::new_v4);
-            .get_uuid()
-            .unwrap_or_else(Uuid::new_v4);
         Entry {
             valid: EntrySealed {
                 uuid,
@@ -983,9 +979,7 @@ impl Entry<EntryInvalid, EntryNew> {
 
     #[cfg(test)]
     pub unsafe fn into_valid_committed(self) -> Entry<EntryValid, EntryCommitted> {
-        let uuid = self
+        let uuid = self.get_uuid().unwrap_or_else(Uuid::new_v4);
-            .get_uuid()
-            .unwrap_or_else(Uuid::new_v4);
         Entry {
             valid: EntryValid {
                 cid: self.valid.cid,
@@ -1001,9 +995,7 @@ impl Entry<EntryInvalid, EntryNew> {
 impl Entry<EntryInvalid, EntryCommitted> {
     #[cfg(test)]
     pub unsafe fn into_sealed_committed(self) -> Entry<EntrySealed, EntryCommitted> {
-        let uuid = self
+        let uuid = self.get_uuid().unwrap_or_else(Uuid::new_v4);
-            .get_uuid()
-            .unwrap_or_else(Uuid::new_v4);
         Entry {
             valid: EntrySealed {
                 uuid,
@@ -1897,6 +1889,11 @@ impl<VALID, STATE> Entry<VALID, STATE> {
         self.attrs.get(attr).and_then(|vs| vs.as_iutf8_iter())
     }
 
+    #[inline(always)]
+    pub fn get_ava_as_iutf8(&self, attr: &str) -> Option<&BTreeSet<String>> {
+        self.attrs.get(attr).and_then(|vs| vs.as_iutf8_set())
+    }
+
     #[inline(always)]
     pub fn get_ava_as_oauthscopes(&self, attr: &str) -> Option<impl Iterator<Item = &str>> {
         self.attrs.get(attr).and_then(|vs| vs.as_oauthscope_iter())

kanidmd/lib/src/plugins/cred_import.rs

@@ -75,7 +75,6 @@ impl CredImport {
                 // does the entry have a primary cred?
                 match e.get_ava_single_credential("primary_credential") {
                     Some(c) => {
-                        // This is the major diff to create, we can update in place!
                         let c = c.update_password(pw);
                         e.set_ava(
                             "primary_credential",
@@ -93,7 +92,8 @@ impl CredImport {
                 }
             };
 
-            // TOTP IMPORT
+            // TOTP IMPORT - Must be subsequent to password import to allow primary cred to
+            // be created.
             if let Some(vs) = e.pop_ava("totp_import") {
                 // Get the map.
                 let totps = vs.as_totp_map().ok_or_else(|| {

kanidmd/lib/src/plugins/protected.rs

@@ -57,7 +57,6 @@ impl Plugin for Protected {
                 || cand.attribute_equality("class", &PVCLASS_TOMBSTONE)
                 || cand.attribute_equality("class", &PVCLASS_RECYCLED)
                 || cand.attribute_equality("class", &PVCLASS_DYNGROUP)
-                || cand.attribute_equality("class", &PVCLASS_SYNC_OBJECT)
             {
                 Err(OperationError::SystemProtectedObject)
             } else {
@@ -103,8 +102,6 @@ impl Plugin for Protected {
             if cand.attribute_equality("class", &PVCLASS_TOMBSTONE)
                 || cand.attribute_equality("class", &PVCLASS_RECYCLED)
                 || cand.attribute_equality("class", &PVCLASS_DYNGROUP)
-                // Temporary until I move this into access.rs
-                || cand.attribute_equality("class", &PVCLASS_SYNC_OBJECT)
             {
                 Err(OperationError::SystemProtectedObject)
             } else {
@@ -183,8 +180,6 @@ impl Plugin for Protected {
             if cand.attribute_equality("class", &PVCLASS_TOMBSTONE)
                 || cand.attribute_equality("class", &PVCLASS_RECYCLED)
                 || cand.attribute_equality("class", &PVCLASS_DYNGROUP)
-                // Temporary until I move this into access.rs
-                || cand.attribute_equality("class", &PVCLASS_SYNC_OBJECT)
             {
                 Err(OperationError::SystemProtectedObject)
             } else {
@@ -247,7 +242,6 @@ impl Plugin for Protected {
                 || cand.attribute_equality("class", &PVCLASS_TOMBSTONE)
                 || cand.attribute_equality("class", &PVCLASS_RECYCLED)
                 || cand.attribute_equality("class", &PVCLASS_DYNGROUP)
-                || cand.attribute_equality("class", &PVCLASS_SYNC_OBJECT)
             {
                 Err(OperationError::SystemProtectedObject)
             } else {

kanidmd/lib/src/server/access/create.rs

@@ -22,6 +22,12 @@ pub(super) fn apply_create_access<'a>(
     let mut denied = false;
     let mut grant = false;
 
+    // This module can never yield a grant.
+    match protected_filter_entry(ident, entry) {
+        IResult::Denied => denied = true,
+        IResult::Grant | IResult::Ignore => {}
+    }
+
     match create_filter_entry(ident, related_acp, entry) {
         IResult::Denied => denied = true,
         IResult::Grant => grant = true,
@@ -136,3 +142,33 @@ fn create_filter_entry<'a>(
         IResult::Ignore
     }
 }
+
+fn protected_filter_entry<'a>(ident: &Identity, entry: &'a Entry<EntryInit, EntryNew>) -> IResult {
+    match &ident.origin {
+        IdentType::Internal => {
+            trace!("Internal operation, protected rules do not apply.");
+            IResult::Ignore
+        }
+        IdentType::Synch(_) => {
+            security_access!("sync agreements may not directly create entities");
+            IResult::Denied
+        }
+        IdentType::User(_) => {
+            // Now check things ...
+
+            // For now we just block create on sync object
+            if let Some(classes) = entry.get_ava_set("class") {
+                if classes.contains(&PVCLASS_SYNC_OBJECT) {
+                    // Block the mod
+                    security_access!("attempt to create with protected class type");
+                    IResult::Denied
+                } else {
+                    IResult::Ignore
+                }
+            } else {
+                // Nothing to check.
+                IResult::Ignore
+            }
+        }
+    }
+}

kanidmd/lib/src/server/access/delete.rs

@@ -22,6 +22,11 @@ pub(super) fn apply_delete_access<'a>(
     let mut denied = false;
     let mut grant = false;
 
+    match protected_filter_entry(ident, entry) {
+        IResult::Denied => denied = true,
+        IResult::Grant | IResult::Ignore => {}
+    }
+
     match delete_filter_entry(ident, related_acp, entry) {
         IResult::Denied => denied = true,
         IResult::Grant => grant = true,
@@ -95,3 +100,33 @@ fn delete_filter_entry<'a>(
         IResult::Ignore
     }
 }
+
+fn protected_filter_entry<'a>(ident: &Identity, entry: &'a Arc<EntrySealedCommitted>) -> IResult {
+    match &ident.origin {
+        IdentType::Internal => {
+            trace!("Internal operation, protected rules do not apply.");
+            IResult::Ignore
+        }
+        IdentType::Synch(_) => {
+            security_access!("sync agreements may not directly delete entities");
+            IResult::Denied
+        }
+        IdentType::User(_) => {
+            // Now check things ...
+
+            // For now we just block create on sync object
+            if let Some(classes) = entry.get_ava_set("class") {
+                if classes.contains(&PVCLASS_SYNC_OBJECT) {
+                    // Block the mod
+                    security_access!("attempt to delete with protected class type");
+                    IResult::Denied
+                } else {
+                    IResult::Ignore
+                }
+            } else {
+                // Nothing to check.
+                IResult::Ignore
+            }
+        }
+    }
+}

kanidmd/lib/src/server/access/mod.rs

@@ -233,7 +233,7 @@ pub trait AccessControlsTransaction<'a> {
             .collect();
 
         if allowed_entries.is_empty() {
-            security_access!("denied ❌");
+            security_access!("denied ❌ - no entries were released");
         } else {
             security_access!("allowed {} entries ✅", allowed_entries.len());
         }
@@ -488,7 +488,7 @@ pub trait AccessControlsTransaction<'a> {
         if r {
             security_access!("allowed ✅");
         } else {
-            security_access!("denied ❌");
+            security_access!("denied ❌ - modifications may not proceed");
         }
         Ok(r)
     }
@@ -621,7 +621,7 @@ pub trait AccessControlsTransaction<'a> {
         if r {
             security_access!("allowed ✅");
         } else {
-            security_access!("denied ❌");
+            security_access!("denied ❌ - modifications may not proceed");
         }
         Ok(r)
     }
@@ -674,7 +674,7 @@ pub trait AccessControlsTransaction<'a> {
         if r {
             security_access!("allowed ✅");
         } else {
-            security_access!("denied ❌");
+            security_access!("denied ❌ - create may not proceed");
         }
 
         Ok(r)
@@ -737,7 +737,7 @@ pub trait AccessControlsTransaction<'a> {
         if r {
             security_access!("allowed ✅");
         } else {
-            security_access!("denied ❌");
+            security_access!("denied ❌ - delete may not proceed");
         }
         Ok(r)
     }
@@ -2384,4 +2384,223 @@ mod tests {
             }]
         )
     }
+
+    #[test]
+    fn test_access_sync_authority_create() {
+        sketching::test_init();
+
+        let ce_admin = CreateEvent::new_impersonate_identity(
+            Identity::from_impersonate_entry_readwrite(E_TEST_ACCOUNT_1.clone()),
+            vec![],
+        );
+
+        // We can create without a sync class.
+        let ev1 = entry_init!(
+            ("class", CLASS_ACCOUNT.clone()),
+            ("name", Value::new_iname("testperson1")),
+            ("uuid", Value::Uuid(UUID_TEST_ACCOUNT_1))
+        );
+        let r1_set = vec![ev1];
+
+        let ev2 = entry_init!(
+            ("class", CLASS_ACCOUNT.clone()),
+            ("class", CLASS_SYNC_OBJECT.clone()),
+            ("name", Value::new_iname("testperson1")),
+            ("uuid", Value::Uuid(UUID_TEST_ACCOUNT_1))
+        );
+        let r2_set = vec![ev2];
+
+        let acp = unsafe {
+            AccessControlCreate::from_raw(
+                "test_create",
+                Uuid::new_v4(),
+                // Apply to admin
+                UUID_TEST_GROUP_1,
+                // To create matching filter testperson
+                // Can this be empty?
+                filter_valid!(f_eq("name", PartialValue::new_iname("testperson1"))),
+                // classes
+                "account sync_object",
+                // attrs
+                "class name uuid",
+            )
+        };
+
+        // Test allowed to create
+        test_acp_create!(&ce_admin, vec![acp.clone()], &r1_set, true);
+        // Test Fails due to protected from sync object
+        test_acp_create!(&ce_admin, vec![acp.clone()], &r2_set, false);
+    }
+
+    #[test]
+    fn test_access_sync_authority_delete() {
+        sketching::test_init();
+
+        let ev1 = unsafe {
+            entry_init!(
+                ("class", CLASS_ACCOUNT.clone()),
+                ("name", Value::new_iname("testperson1")),
+                ("uuid", Value::Uuid(UUID_TEST_ACCOUNT_1))
+            )
+            .into_sealed_committed()
+        };
+        let r1_set = vec![Arc::new(ev1)];
+
+        let ev2 = unsafe {
+            entry_init!(
+                ("class", CLASS_ACCOUNT.clone()),
+                ("class", CLASS_SYNC_OBJECT.clone()),
+                ("name", Value::new_iname("testperson1")),
+                ("uuid", Value::Uuid(UUID_TEST_ACCOUNT_1))
+            )
+            .into_sealed_committed()
+        };
+        let r2_set = vec![Arc::new(ev2)];
+
+        let de_admin = unsafe {
+            DeleteEvent::new_impersonate_entry(
+                E_TEST_ACCOUNT_1.clone(),
+                filter_all!(f_eq("name", PartialValue::new_iname("testperson1"))),
+            )
+        };
+
+        let acp = unsafe {
+            AccessControlDelete::from_raw(
+                "test_delete",
+                Uuid::new_v4(),
+                // Apply to admin
+                UUID_TEST_GROUP_1,
+                // To delete testperson
+                filter_valid!(f_eq("name", PartialValue::new_iname("testperson1"))),
+            )
+        };
+
+        // Test allowed to delete
+        test_acp_delete!(&de_admin, vec![acp.clone()], &r1_set, true);
+        // Test reject delete
+        test_acp_delete!(&de_admin, vec![acp], &r2_set, false);
+    }
+
+    #[test]
+    fn test_access_sync_authority_modify() {
+        sketching::test_init();
+
+        let ev1 = unsafe {
+            entry_init!(
+                ("class", CLASS_ACCOUNT.clone()),
+                ("name", Value::new_iname("testperson1")),
+                ("uuid", Value::Uuid(UUID_TEST_ACCOUNT_1))
+            )
+            .into_sealed_committed()
+        };
+        let r1_set = vec![Arc::new(ev1)];
+
+        let ev2 = unsafe {
+            entry_init!(
+                ("class", CLASS_ACCOUNT.clone()),
+                ("class", CLASS_SYNC_OBJECT.clone()),
+                ("name", Value::new_iname("testperson1")),
+                ("uuid", Value::Uuid(UUID_TEST_ACCOUNT_1))
+            )
+            .into_sealed_committed()
+        };
+        let r2_set = vec![Arc::new(ev2)];
+
+        // Allow name and class, class is account
+        let acp_allow = unsafe {
+            AccessControlModify::from_raw(
+                "test_modify_allow",
+                Uuid::new_v4(),
+                // Apply to admin
+                UUID_TEST_GROUP_1,
+                // To modify testperson
+                filter_valid!(f_eq("name", PartialValue::new_iname("testperson1"))),
+                // Allow pres user_auth_token_session
+                "user_auth_token_session name",
+                // Allow user_auth_token_session
+                "user_auth_token_session name",
+                // And the class allowed is account, we don't use it though.
+                "account",
+            )
+        };
+
+        // NOTE! Syntax doesn't matter here, we just need to assert if the attr exists
+        // and is being modified.
+        // Name present
+        let me_pres = unsafe {
+            ModifyEvent::new_impersonate_entry(
+                E_TEST_ACCOUNT_1.clone(),
+                filter_all!(f_eq("name", PartialValue::new_iname("testperson1"))),
+                modlist!([m_pres(
+                    "user_auth_token_session",
+                    &Value::new_iname("value")
+                )]),
+            )
+        };
+        // Name rem
+        let me_rem = unsafe {
+            ModifyEvent::new_impersonate_entry(
+                E_TEST_ACCOUNT_1.clone(),
+                filter_all!(f_eq("name", PartialValue::new_iname("testperson1"))),
+                modlist!([m_remove(
+                    "user_auth_token_session",
+                    &PartialValue::new_iname("value")
+                )]),
+            )
+        };
+        // Name purge
+        let me_purge = unsafe {
+            ModifyEvent::new_impersonate_entry(
+                E_TEST_ACCOUNT_1.clone(),
+                filter_all!(f_eq("name", PartialValue::new_iname("testperson1"))),
+                modlist!([m_purge("user_auth_token_session")]),
+            )
+        };
+
+        // Test allowed pres
+        test_acp_modify!(&me_pres, vec![acp_allow.clone()], &r1_set, true);
+        // test allowed rem
+        test_acp_modify!(&me_rem, vec![acp_allow.clone()], &r1_set, true);
+        // test allowed purge
+        test_acp_modify!(&me_purge, vec![acp_allow.clone()], &r1_set, true);
+
+        // Test allow pres
+        test_acp_modify!(&me_pres, vec![acp_allow.clone()], &r2_set, true);
+        // Test allow rem
+        test_acp_modify!(&me_rem, vec![acp_allow.clone()], &r2_set, true);
+        // Test allow purge
+        test_acp_modify!(&me_purge, vec![acp_allow.clone()], &r2_set, true);
+
+        // But other attrs are blocked.
+        let me_pres = unsafe {
+            ModifyEvent::new_impersonate_entry(
+                E_TEST_ACCOUNT_1.clone(),
+                filter_all!(f_eq("name", PartialValue::new_iname("testperson1"))),
+                modlist!([m_pres("name", &Value::new_iname("value"))]),
+            )
+        };
+        // Name rem
+        let me_rem = unsafe {
+            ModifyEvent::new_impersonate_entry(
+                E_TEST_ACCOUNT_1.clone(),
+                filter_all!(f_eq("name", PartialValue::new_iname("testperson1"))),
+                modlist!([m_remove("name", &PartialValue::new_iname("value"))]),
+            )
+        };
+        // Name purge
+        let me_purge = unsafe {
+            ModifyEvent::new_impersonate_entry(
+                E_TEST_ACCOUNT_1.clone(),
+                filter_all!(f_eq("name", PartialValue::new_iname("testperson1"))),
+                modlist!([m_purge("name")]),
+            )
+        };
+
+        // Test reject pres
+        test_acp_modify!(&me_pres, vec![acp_allow.clone()], &r2_set, false);
+        // Test reject rem
+        test_acp_modify!(&me_rem, vec![acp_allow.clone()], &r2_set, false);
+        // Test reject purge
+        test_acp_modify!(&me_purge, vec![acp_allow.clone()], &r2_set, false);
+    }
 }

kanidmd/lib/src/server/access/modify.rs

@@ -19,6 +19,7 @@ pub(super) enum ModifyResult<'a> {
 pub(super) fn apply_modify_access<'a>(
     ident: &Identity,
     related_acp: &'a [(&AccessControlModify, Filter<FilterValidResolved>)],
+    // may need sync agreements later.
     entry: &'a Arc<EntrySealedCommitted>,
 ) -> ModifyResult<'a> {
     let mut denied = false;
@@ -42,6 +43,22 @@ pub(super) fn apply_modify_access<'a>(
     }
 
     if !grant && !denied {
+        // Check with protected if we should proceed.
+
+        // If it's a sync entry, constrain it.
+        match modify_sync_constrain(ident, entry) {
+            AccessResult::Denied => denied = true,
+            AccessResult::Constrain(mut set) => {
+                constrain_rem.extend(set.iter().copied());
+                constrain_pres.append(&mut set)
+            }
+            // Can't grant.
+            AccessResult::Grant |
+            // Can't allow
+            AccessResult::Allow(_) |
+            AccessResult::Ignore => {}
+        }
+
         // Setup the acp's here
         let scoped_acp: Vec<&AccessControlModify> = related_acp
             .iter()
@@ -167,3 +184,36 @@ fn modify_cls_test<'a>(scoped_acp: &[&'a AccessControlModify]) -> AccessResult<'
         .collect();
     AccessResult::Allow(allowed_classes)
 }
+
+fn modify_sync_constrain<'a>(
+    ident: &Identity,
+    entry: &'a Arc<EntrySealedCommitted>,
+) -> AccessResult<'a> {
+    match &ident.origin {
+        IdentType::Internal => AccessResult::Ignore,
+        IdentType::Synch(_) => {
+            // Allowed to mod sync objects. Later we'll probably need to check the limits of what
+            // it can do if we go that way.
+            AccessResult::Ignore
+        }
+        IdentType::User(_) => {
+            if let Some(classes) = entry.get_ava_set("class") {
+                // If the entry is sync object.
+                if classes.contains(&PVCLASS_SYNC_OBJECT) {
+                    // Constrain to a limited set of attributes.
+                    AccessResult::Constrain(btreeset![
+                        "user_auth_token_session",
+                        "oauth2_session",
+                        "oauth2_consent_scope_map",
+                        "credential_update_intent_token"
+                    ])
+                } else {
+                    AccessResult::Ignore
+                }
+            } else {
+                // Nothing to check.
+                AccessResult::Ignore
+            }
+        }
+    }
+}

kanidmd/lib/src/server/access/profiles.rs

@@ -56,10 +56,7 @@ impl AccessControlSearch {
                 receiver: Some(receiver),
                 targetscope,
             },
-            attrs: attrs
+            attrs: attrs.split_whitespace().map(AttrString::from).collect(),
-                .split_whitespace()
-                .map(AttrString::from)
-                .collect(),
         }
     }
 }
@@ -222,18 +219,9 @@ impl AccessControlModify {
                 receiver: Some(receiver),
                 targetscope,
             },
-            classes: classes
+            classes: classes.split_whitespace().map(AttrString::from).collect(),
-                .split_whitespace()
+            presattrs: presattrs.split_whitespace().map(AttrString::from).collect(),
-                .map(AttrString::from)
+            remattrs: remattrs.split_whitespace().map(AttrString::from).collect(),
-                .collect(),
-            presattrs: presattrs
-                .split_whitespace()
-                .map(AttrString::from)
-                .collect(),
-            remattrs: remattrs
-                .split_whitespace()
-                .map(AttrString::from)
-                .collect(),
         }
     }
 }

kanidmd/lib/src/server/mod.rs

@@ -1585,10 +1585,7 @@ mod tests {
         assert!(r3 == Ok(Value::Refer(uuid!("cc8e95b4-c24f-4d68-ba54-8bed76f63930"))));
 
         // test attr reference already resolved.
-        let r4 = server_txn.clone_value(
+        let r4 = server_txn.clone_value("member", "cc8e95b4-c24f-4d68-ba54-8bed76f63930");
-            "member",
-            "cc8e95b4-c24f-4d68-ba54-8bed76f63930",
-        );
 
         debug!("{:?}", r4);
         assert!(r4 == Ok(Value::Refer(uuid!("cc8e95b4-c24f-4d68-ba54-8bed76f63930"))));

kanidmd/lib/src/server/recycle.rs

@@ -811,13 +811,11 @@ mod tests {
             )
         };
         assert!(server_txn.revive_recycled(&rev3).is_ok());
-        assert!(
+        assert!(!check_entry_has_mo(
-            !check_entry_has_mo(
             &mut server_txn,
             "u3",
             "36048117-e479-45ed-aeb5-611e8d83d5b1"
-            )
+        ));
-        );
 
         // Revive u4, should NOT have the MO.
         let rev4a = unsafe {
@@ -827,13 +825,11 @@ mod tests {
             )
         };
         assert!(server_txn.revive_recycled(&rev4a).is_ok());
-        assert!(
+        assert!(!check_entry_has_mo(
-            !check_entry_has_mo(
             &mut server_txn,
             "u4",
             "d5c59ac6-c533-4b00-989f-d0e183f07bab"
-            )
+        ));
-        );
 
         // Now revive g4, should allow MO onto u4.
         let rev4b = unsafe {
@@ -843,13 +839,11 @@ mod tests {
             )
         };
         assert!(server_txn.revive_recycled(&rev4b).is_ok());
-        assert!(
+        assert!(!check_entry_has_mo(
-            !check_entry_has_mo(
             &mut server_txn,
             "u4",
             "d5c59ac6-c533-4b00-989f-d0e183f07bab"
-            )
+        ));
-        );
 
         assert!(server_txn.commit().is_ok());
     }

kanidmd/testkit/tests/default_entries.rs

@@ -219,12 +219,7 @@ async fn login_account_via_admin(rsclient: &KanidmClient, id: &str) {
     login_account(rsclient, id).await
 }
 
-async fn test_read_attrs(
+async fn test_read_attrs(rsclient: &KanidmClient, id: &str, attrs: &[&str], is_readable: bool) {
-    rsclient: &KanidmClient,
-    id: &str,
-    attrs: &[&str],
-    is_readable: bool,
-) {
     println!("Test read to {}, is readable: {}", id, is_readable);
     let rset = rsclient
         .search(Filter::Eq("name".to_string(), id.to_string()))
@@ -246,12 +241,7 @@ async fn test_read_attrs(
     }
 }
 
-async fn test_write_attrs(
+async fn test_write_attrs(rsclient: &KanidmClient, id: &str, attrs: &[&str], is_writeable: bool) {
-    rsclient: &KanidmClient,
-    id: &str,
-    attrs: &[&str],
-    is_writeable: bool,
-) {
     println!("Test write to {}, is writeable: {}", id, is_writeable);
     for attr in attrs.iter() {
         println!("Writing to {}", attr);
@@ -260,11 +250,7 @@ async fn test_write_attrs(
     }
 }
 
-async fn test_modify_group(
+async fn test_modify_group(rsclient: &KanidmClient, group_names: &[&str], is_modificable: bool) {
-    rsclient: &KanidmClient,
-    group_names: &[&str],
-    is_modificable: bool,
-) {
     // need user test created to be added as test part
     for group in group_names.iter() {
         println!("Testing group: {}", group);