From a2eae533280179809faf65dd02e440b8dd2df192 Mon Sep 17 00:00:00 2001 From: Firstyear <william@blackhats.net.au> Date: Tue, 1 Apr 2025 11:00:56 +1000 Subject: [PATCH] 20250314 remove protected plugin (#3504) Removes the protected plugin into an access control module so that it's outputs can be properly represented in effective access checks. --- Cargo.lock | 49 +- proto/src/attribute.rs | 6 + proto/src/constants.rs | 2 + proto/src/scim_v1/server.rs | 4 +- server/lib/src/constants/uuids.rs | 4 + server/lib/src/idm/credupdatesession.rs | 32 +- server/lib/src/migration_data/dl10/access.rs | 15 +- server/lib/src/plugins/base.rs | 2 - server/lib/src/plugins/mod.rs | 15 +- server/lib/src/plugins/protected.rs | 690 ------------------- server/lib/src/schema.rs | 32 + server/lib/src/server/access/create.rs | 39 +- server/lib/src/server/access/delete.rs | 49 +- server/lib/src/server/access/mod.rs | 475 ++++++++++--- server/lib/src/server/access/modify.rs | 304 +++++--- server/lib/src/server/access/profiles.rs | 42 +- server/lib/src/server/access/protected.rs | 83 +++ server/lib/src/server/access/search.rs | 97 +-- 18 files changed, 933 insertions(+), 1007 deletions(-) delete mode 100644 server/lib/src/plugins/protected.rs create mode 100644 server/lib/src/server/access/protected.rs diff --git a/Cargo.lock b/Cargo.lock index 2a3cd4d4f..fe306e47e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -232,9 +232,9 @@ dependencies = [ [[package]] name = "async-compression" -version = "0.4.21" +version = "0.4.22" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c0cf008e5e1a9e9e22a7d3c9a4992e21a350290069e36d8fb72304ed17e8f2d2" +checksum = "59a194f9d963d8099596278594b3107448656ba73831c9d8c783e613ce86da64" dependencies = [ "flate2", "futures-core", @@ -1167,9 +1167,9 @@ dependencies = [ [[package]] name = "deranged" -version = "0.4.0" +version = "0.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9c9e6a11ca8224451684bc0d7d5a7adbf8f2fd6887261a1cfc3c0432f9d4068e" +checksum = "28cfac68e08048ae1883171632c2aef3ebc555621ae56fbccce1cbf22dd7f058" dependencies = [ "powerfmt", "serde", @@ -2532,14 +2532,15 @@ dependencies = [ [[package]] name = "iana-time-zone" -version = "0.1.61" +version = "0.1.62" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "235e081f3925a06703c2d0117ea8b91f042756fd6e7a6e5d901e8ca1a996b220" +checksum = "b2fd658b06e56721792c5df4475705b6cda790e9298d19d2f8af083457bcd127" dependencies = [ "android_system_properties", "core-foundation-sys", "iana-time-zone-haiku", "js-sys", + "log", "wasm-bindgen", "windows-core", ] @@ -2594,9 +2595,9 @@ dependencies = [ [[package]] name = "icu_locid_transform_data" -version = "1.5.0" +version = "1.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fdc8ff3388f852bede6b579ad4e978ab004f139284d7b28715f773507b946f6e" +checksum = "7515e6d781098bf9f7205ab3fc7e9709d34554ae0b21ddbcb5febfa4bc7df11d" [[package]] name = "icu_normalizer" @@ -2618,9 +2619,9 @@ dependencies = [ [[package]] name = "icu_normalizer_data" -version = "1.5.0" +version = "1.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f8cafbf7aa791e9b22bec55a167906f9e1215fd475cd22adfcf660e03e989516" +checksum = "c5e8338228bdc8ab83303f16b797e177953730f601a96c25d10cb3ab0daa0cb7" [[package]] name = "icu_properties" @@ -2639,9 +2640,9 @@ dependencies = [ [[package]] name = "icu_properties_data" -version = "1.5.0" +version = "1.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "67a8effbc3dd3e4ba1afa8ad918d5684b8868b3b26500753effea8d2eed19569" +checksum = "85fb8799753b75aee8d2a21d7c14d9f38921b54b3dbda10f5a3c7a7b82dba5e2" [[package]] name = "icu_provider" @@ -3483,9 +3484,9 @@ dependencies = [ [[package]] name = "log" -version = "0.4.26" +version = "0.4.27" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "30bde2b3dc3671ae49d8e2e9f044c7c005836e7a023ee57cffa25ab82764bb9e" +checksum = "13dc2df351e3202783a1fe0d44375f7295ffb4049267b0f3018346dc122a1d94" [[package]] name = "lru" @@ -3948,9 +3949,9 @@ dependencies = [ [[package]] name = "once_cell" -version = "1.21.1" +version = "1.21.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d75b0bedcc4fe52caa0e03d9f1151a323e4aa5e2d78ba3580400cd3c9e2bc4bc" +checksum = "c2806eaa3524762875e21c3dcd057bc4b7bfa01ce4da8d46be1cd43649e1cc6b" [[package]] name = "openssl" @@ -4483,9 +4484,9 @@ dependencies = [ [[package]] name = "quinn-udp" -version = "0.5.10" +version = "0.5.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e46f3055866785f6b92bc6164b76be02ca8f2eb4b002c0354b28cf4c119e5944" +checksum = "541d0f57c6ec747a90738a52741d3221f7960e8ac2f0ff4b1a63680e033b4ab5" dependencies = [ "cfg_aliases", "libc", @@ -4947,9 +4948,9 @@ dependencies = [ [[package]] name = "rustls-webpki" -version = "0.103.0" +version = "0.103.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0aa4eeac2588ffff23e9d7a7e9b3f971c5fb5b7ebc9452745e0c232c64f83b2f" +checksum = "fef8b8769aaccf73098557a87cd1816b4f9c7c16811c9c77142aa695c16f2c03" dependencies = [ "ring", "rustls-pki-types", @@ -5576,9 +5577,9 @@ dependencies = [ [[package]] name = "time" -version = "0.3.40" +version = "0.3.41" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9d9c75b47bdff86fa3334a3db91356b8d7d86a9b839dab7d0bdc5c3d3a077618" +checksum = "8a7619e19bc266e0f9c5e6686659d394bc57973859340060a69221e57dbc0c40" dependencies = [ "deranged", "itoa", @@ -5599,9 +5600,9 @@ checksum = "c9e9a38711f559d9e3ce1cdb06dd7c5b8ea546bc90052da6d06bb76da74bb07c" [[package]] name = "time-macros" -version = "0.2.21" +version = "0.2.22" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "29aa485584182073ed57fd5004aa09c371f021325014694e432313345865fd04" +checksum = "3526739392ec93fd8b359c8e98514cb3e8e021beb4e5f597b00a0221f8ed8a49" dependencies = [ "num-conv", "time-core", diff --git a/proto/src/attribute.rs b/proto/src/attribute.rs index d26600aa4..493509944 100644 --- a/proto/src/attribute.rs +++ b/proto/src/attribute.rs @@ -22,6 +22,8 @@ pub enum Attribute { AcpCreateClass, AcpEnable, AcpModifyClass, + AcpModifyPresentClass, + AcpModifyRemoveClass, AcpModifyPresentAttr, AcpModifyRemovedAttr, AcpReceiver, @@ -255,6 +257,8 @@ impl Attribute { Attribute::AcpCreateClass => ATTR_ACP_CREATE_CLASS, Attribute::AcpEnable => ATTR_ACP_ENABLE, Attribute::AcpModifyClass => ATTR_ACP_MODIFY_CLASS, + Attribute::AcpModifyPresentClass => ATTR_ACP_MODIFY_PRESENT_CLASS, + Attribute::AcpModifyRemoveClass => ATTR_ACP_MODIFY_REMOVE_CLASS, Attribute::AcpModifyPresentAttr => ATTR_ACP_MODIFY_PRESENTATTR, Attribute::AcpModifyRemovedAttr => ATTR_ACP_MODIFY_REMOVEDATTR, Attribute::AcpReceiver => ATTR_ACP_RECEIVER, @@ -440,6 +444,8 @@ impl Attribute { ATTR_ACP_CREATE_CLASS => Attribute::AcpCreateClass, ATTR_ACP_ENABLE => Attribute::AcpEnable, ATTR_ACP_MODIFY_CLASS => Attribute::AcpModifyClass, + ATTR_ACP_MODIFY_PRESENT_CLASS => Attribute::AcpModifyPresentClass, + ATTR_ACP_MODIFY_REMOVE_CLASS => Attribute::AcpModifyRemoveClass, ATTR_ACP_MODIFY_PRESENTATTR => Attribute::AcpModifyPresentAttr, ATTR_ACP_MODIFY_REMOVEDATTR => Attribute::AcpModifyRemovedAttr, ATTR_ACP_RECEIVER => Attribute::AcpReceiver, diff --git a/proto/src/constants.rs b/proto/src/constants.rs index 7be28d9ac..414c51791 100644 --- a/proto/src/constants.rs +++ b/proto/src/constants.rs @@ -62,6 +62,8 @@ pub const ATTR_ACP_CREATE_ATTR: &str = "acp_create_attr"; pub const ATTR_ACP_CREATE_CLASS: &str = "acp_create_class"; pub const ATTR_ACP_ENABLE: &str = "acp_enable"; pub const ATTR_ACP_MODIFY_CLASS: &str = "acp_modify_class"; +pub const ATTR_ACP_MODIFY_PRESENT_CLASS: &str = "acp_modify_present_class"; +pub const ATTR_ACP_MODIFY_REMOVE_CLASS: &str = "acp_modify_remove_class"; pub const ATTR_ACP_MODIFY_PRESENTATTR: &str = "acp_modify_presentattr"; pub const ATTR_ACP_MODIFY_REMOVEDATTR: &str = "acp_modify_removedattr"; pub const ATTR_ACP_RECEIVER_GROUP: &str = "acp_receiver_group"; diff --git a/proto/src/scim_v1/server.rs b/proto/src/scim_v1/server.rs index f99784fdc..18f640539 100644 --- a/proto/src/scim_v1/server.rs +++ b/proto/src/scim_v1/server.rs @@ -33,7 +33,7 @@ pub enum ScimAttributeEffectiveAccess { /// All attributes on the entry have this permission granted Grant, /// All attributes on the entry have this permission denied - Denied, + Deny, /// The following attributes on the entry have this permission granted Allow(BTreeSet<Attribute>), } @@ -43,7 +43,7 @@ impl ScimAttributeEffectiveAccess { pub fn check(&self, attr: &Attribute) -> bool { match self { Self::Grant => true, - Self::Denied => false, + Self::Deny => false, Self::Allow(set) => set.contains(attr), } } diff --git a/server/lib/src/constants/uuids.rs b/server/lib/src/constants/uuids.rs index 021364b4a..fc6c2f286 100644 --- a/server/lib/src/constants/uuids.rs +++ b/server/lib/src/constants/uuids.rs @@ -330,6 +330,10 @@ pub const UUID_SCHEMA_ATTR_DOMAIN_ALLOW_EASTER_EGGS: Uuid = pub const UUID_SCHEMA_ATTR_LDAP_MAXIMUM_QUERYABLE_ATTRIBUTES: Uuid = uuid!("00000000-0000-0000-0000-ffff00000187"); pub const UUID_SCHEMA_ATTR_INDEXED: Uuid = uuid!("00000000-0000-0000-0000-ffff00000188"); +pub const UUID_SCHEMA_ATTR_ACP_MODIFY_PRESENT_CLASS: Uuid = + uuid!("00000000-0000-0000-0000-ffff00000189"); +pub const UUID_SCHEMA_ATTR_ACP_MODIFY_REMOVE_CLASS: Uuid = + uuid!("00000000-0000-0000-0000-ffff00000190"); // System and domain infos // I'd like to strongly criticise william of the past for making poor choices about these allocations. diff --git a/server/lib/src/idm/credupdatesession.rs b/server/lib/src/idm/credupdatesession.rs index 9d4ce0299..0453fdbdc 100644 --- a/server/lib/src/idm/credupdatesession.rs +++ b/server/lib/src/idm/credupdatesession.rs @@ -599,19 +599,19 @@ impl IdmServerProxyWriteTransaction<'_> { } let eperm_search_primary_cred = match &eperm.search { - Access::Denied => false, + Access::Deny => false, Access::Grant => true, Access::Allow(attrs) => attrs.contains(&Attribute::PrimaryCredential), }; let eperm_mod_primary_cred = match &eperm.modify_pres { - Access::Denied => false, + Access::Deny => false, Access::Grant => true, Access::Allow(attrs) => attrs.contains(&Attribute::PrimaryCredential), }; let eperm_rem_primary_cred = match &eperm.modify_rem { - Access::Denied => false, + Access::Deny => false, Access::Grant => true, Access::Allow(attrs) => attrs.contains(&Attribute::PrimaryCredential), }; @@ -620,19 +620,19 @@ impl IdmServerProxyWriteTransaction<'_> { eperm_search_primary_cred && eperm_mod_primary_cred && eperm_rem_primary_cred; let eperm_search_passkeys = match &eperm.search { - Access::Denied => false, + Access::Deny => false, Access::Grant => true, Access::Allow(attrs) => attrs.contains(&Attribute::PassKeys), }; let eperm_mod_passkeys = match &eperm.modify_pres { - Access::Denied => false, + Access::Deny => false, Access::Grant => true, Access::Allow(attrs) => attrs.contains(&Attribute::PassKeys), }; let eperm_rem_passkeys = match &eperm.modify_rem { - Access::Denied => false, + Access::Deny => false, Access::Grant => true, Access::Allow(attrs) => attrs.contains(&Attribute::PassKeys), }; @@ -640,19 +640,19 @@ impl IdmServerProxyWriteTransaction<'_> { let passkeys_can_edit = eperm_search_passkeys && eperm_mod_passkeys && eperm_rem_passkeys; let eperm_search_attested_passkeys = match &eperm.search { - Access::Denied => false, + Access::Deny => false, Access::Grant => true, Access::Allow(attrs) => attrs.contains(&Attribute::AttestedPasskeys), }; let eperm_mod_attested_passkeys = match &eperm.modify_pres { - Access::Denied => false, + Access::Deny => false, Access::Grant => true, Access::Allow(attrs) => attrs.contains(&Attribute::AttestedPasskeys), }; let eperm_rem_attested_passkeys = match &eperm.modify_rem { - Access::Denied => false, + Access::Deny => false, Access::Grant => true, Access::Allow(attrs) => attrs.contains(&Attribute::AttestedPasskeys), }; @@ -662,19 +662,19 @@ impl IdmServerProxyWriteTransaction<'_> { && eperm_rem_attested_passkeys; let eperm_search_unixcred = match &eperm.search { - Access::Denied => false, + Access::Deny => false, Access::Grant => true, Access::Allow(attrs) => attrs.contains(&Attribute::UnixPassword), }; let eperm_mod_unixcred = match &eperm.modify_pres { - Access::Denied => false, + Access::Deny => false, Access::Grant => true, Access::Allow(attrs) => attrs.contains(&Attribute::UnixPassword), }; let eperm_rem_unixcred = match &eperm.modify_rem { - Access::Denied => false, + Access::Deny => false, Access::Grant => true, Access::Allow(attrs) => attrs.contains(&Attribute::UnixPassword), }; @@ -685,19 +685,19 @@ impl IdmServerProxyWriteTransaction<'_> { && eperm_rem_unixcred; let eperm_search_sshpubkey = match &eperm.search { - Access::Denied => false, + Access::Deny => false, Access::Grant => true, Access::Allow(attrs) => attrs.contains(&Attribute::SshPublicKey), }; let eperm_mod_sshpubkey = match &eperm.modify_pres { - Access::Denied => false, + Access::Deny => false, Access::Grant => true, Access::Allow(attrs) => attrs.contains(&Attribute::SshPublicKey), }; let eperm_rem_sshpubkey = match &eperm.modify_rem { - Access::Denied => false, + Access::Deny => false, Access::Grant => true, Access::Allow(attrs) => attrs.contains(&Attribute::SshPublicKey), }; @@ -726,7 +726,7 @@ impl IdmServerProxyWriteTransaction<'_> { })?; match &eperm.search { - Access::Denied => false, + Access::Deny => false, Access::Grant => true, Access::Allow(attrs) => attrs.contains(&Attribute::SyncCredentialPortal), } diff --git a/server/lib/src/migration_data/dl10/access.rs b/server/lib/src/migration_data/dl10/access.rs index ab43bc6fd..3e56613fc 100644 --- a/server/lib/src/migration_data/dl10/access.rs +++ b/server/lib/src/migration_data/dl10/access.rs @@ -72,6 +72,8 @@ pub struct BuiltinAcp { modify_present_attrs: Vec<Attribute>, modify_removed_attrs: Vec<Attribute>, modify_classes: Vec<EntryClass>, + modify_present_classes: Vec<EntryClass>, + modify_remove_classes: Vec<EntryClass>, create_classes: Vec<EntryClass>, create_attrs: Vec<Attribute>, } @@ -159,9 +161,19 @@ impl From<BuiltinAcp> for EntryInitNew { value.modify_removed_attrs.into_iter().for_each(|attr| { entry.add_ava(Attribute::AcpModifyRemovedAttr, Value::from(attr)); }); + value.modify_classes.into_iter().for_each(|class| { entry.add_ava(Attribute::AcpModifyClass, Value::from(class)); }); + + value.modify_present_classes.into_iter().for_each(|class| { + entry.add_ava(Attribute::AcpModifyPresentClass, Value::from(class)); + }); + + value.modify_remove_classes.into_iter().for_each(|class| { + entry.add_ava(Attribute::AcpModifyRemoveClass, Value::from(class)); + }); + value.create_classes.into_iter().for_each(|class| { entry.add_ava(Attribute::AcpCreateClass, Value::from(class)); }); @@ -214,7 +226,7 @@ lazy_static! { ATTR_RECYCLED.to_string() )), modify_removed_attrs: vec![Attribute::Class], - modify_classes: vec![EntryClass::Recycled], + modify_remove_classes: vec![EntryClass::Recycled], ..Default::default() }; } @@ -425,6 +437,7 @@ lazy_static! { EntryClass::AccessControlCreate, EntryClass::AccessControlDelete, ], + ..Default::default() }; } diff --git a/server/lib/src/plugins/base.rs b/server/lib/src/plugins/base.rs index 48d8dcd15..a143c984a 100644 --- a/server/lib/src/plugins/base.rs +++ b/server/lib/src/plugins/base.rs @@ -695,7 +695,6 @@ mod tests { let e = entry_init!( (Attribute::Class, EntryClass::Person.to_value()), - (Attribute::Class, EntryClass::System.to_value()), (Attribute::Name, Value::new_iname("testperson")), (Attribute::DisplayName, Value::new_iname("testperson")), ( @@ -726,7 +725,6 @@ mod tests { let e = entry_init!( (Attribute::Class, EntryClass::Person.to_value()), - (Attribute::Class, EntryClass::System.to_value()), (Attribute::Name, Value::new_iname("testperson")), (Attribute::DisplayName, Value::new_iname("testperson")), ( diff --git a/server/lib/src/plugins/mod.rs b/server/lib/src/plugins/mod.rs index 84dea658b..87668467c 100644 --- a/server/lib/src/plugins/mod.rs +++ b/server/lib/src/plugins/mod.rs @@ -22,7 +22,6 @@ mod jwskeygen; mod keyobject; mod memberof; mod namehistory; -mod protected; mod refint; mod session; mod spn; @@ -44,6 +43,7 @@ trait Plugin { Err(OperationError::InvalidState) } + #[allow(dead_code)] fn pre_create( _qs: &mut QueryServerWriteTransaction, // List of what we will commit that is valid? @@ -243,13 +243,13 @@ impl Plugins { attrunique::AttrUnique::pre_create_transform(qs, cand, ce) } - #[instrument(level = "debug", name = "plugins::run_pre_create", skip_all)] + #[instrument(level = "trace", name = "plugins::run_pre_create", skip_all)] pub fn run_pre_create( - qs: &mut QueryServerWriteTransaction, - cand: &[Entry<EntrySealed, EntryNew>], - ce: &CreateEvent, + _qs: &mut QueryServerWriteTransaction, + _cand: &[Entry<EntrySealed, EntryNew>], + _ce: &CreateEvent, ) -> Result<(), OperationError> { - protected::Protected::pre_create(qs, cand, ce) + Ok(()) } #[instrument(level = "debug", name = "plugins::run_post_create", skip_all)] @@ -269,7 +269,6 @@ impl Plugins { cand: &mut Vec<Entry<EntryInvalid, EntryCommitted>>, me: &ModifyEvent, ) -> Result<(), OperationError> { - protected::Protected::pre_modify(qs, pre_cand, cand, me)?; base::Base::pre_modify(qs, pre_cand, cand, me)?; valuedeny::ValueDeny::pre_modify(qs, pre_cand, cand, me)?; cred_import::CredImport::pre_modify(qs, pre_cand, cand, me)?; @@ -305,7 +304,6 @@ impl Plugins { cand: &mut Vec<Entry<EntryInvalid, EntryCommitted>>, me: &BatchModifyEvent, ) -> Result<(), OperationError> { - protected::Protected::pre_batch_modify(qs, pre_cand, cand, me)?; base::Base::pre_batch_modify(qs, pre_cand, cand, me)?; valuedeny::ValueDeny::pre_batch_modify(qs, pre_cand, cand, me)?; cred_import::CredImport::pre_batch_modify(qs, pre_cand, cand, me)?; @@ -340,7 +338,6 @@ impl Plugins { cand: &mut Vec<Entry<EntryInvalid, EntryCommitted>>, de: &DeleteEvent, ) -> Result<(), OperationError> { - protected::Protected::pre_delete(qs, cand, de)?; memberof::MemberOf::pre_delete(qs, cand, de) } diff --git a/server/lib/src/plugins/protected.rs b/server/lib/src/plugins/protected.rs deleted file mode 100644 index ae58217ae..000000000 --- a/server/lib/src/plugins/protected.rs +++ /dev/null @@ -1,690 +0,0 @@ -// System protected objects. Items matching specific requirements -// may only have certain modifications performed. - -use hashbrown::HashSet; -use std::sync::Arc; - -use crate::event::{CreateEvent, DeleteEvent, ModifyEvent}; -use crate::modify::Modify; -use crate::plugins::Plugin; -use crate::prelude::*; - -pub struct Protected {} - -// Here is the declaration of all the attrs that can be altered by -// a call on a system object. We trust they are allowed because -// schema will have checked this, and we don't allow class changes! - -lazy_static! { - static ref ALLOWED_ATTRS: HashSet<Attribute> = { - let attrs = vec![ - // Allow modification of some schema class types to allow local extension - // of schema types. - Attribute::Must, - Attribute::May, - // modification of some domain info types for local configuratiomn. - Attribute::DomainSsid, - Attribute::DomainLdapBasedn, - Attribute::LdapMaxQueryableAttrs, - Attribute::LdapAllowUnixPwBind, - Attribute::FernetPrivateKeyStr, - Attribute::Es256PrivateKeyDer, - Attribute::KeyActionRevoke, - Attribute::KeyActionRotate, - Attribute::IdVerificationEcKey, - Attribute::BadlistPassword, - Attribute::DeniedName, - Attribute::DomainDisplayName, - Attribute::Image, - // modification of account policy values for dyngroup. - Attribute::AuthSessionExpiry, - Attribute::AuthPasswordMinimumLength, - Attribute::CredentialTypeMinimum, - Attribute::PrivilegeExpiry, - Attribute::WebauthnAttestationCaList, - Attribute::LimitSearchMaxResults, - Attribute::LimitSearchMaxFilterTest, - Attribute::AllowPrimaryCredFallback, - ]; - - let mut m = HashSet::with_capacity(attrs.len()); - m.extend(attrs); - - m - }; - - static ref PROTECTED_ENTRYCLASSES: Vec<EntryClass> = - vec![ - EntryClass::System, - EntryClass::DomainInfo, - EntryClass::SystemInfo, - EntryClass::SystemConfig, - EntryClass::DynGroup, - EntryClass::SyncObject, - EntryClass::Tombstone, - EntryClass::Recycled, - ]; -} - -impl Plugin for Protected { - fn id() -> &'static str { - "plugin_protected" - } - - #[instrument(level = "debug", name = "protected_pre_create", skip_all)] - fn pre_create( - _qs: &mut QueryServerWriteTransaction, - // List of what we will commit that is valid? - cand: &[Entry<EntrySealed, EntryNew>], - ce: &CreateEvent, - ) -> Result<(), OperationError> { - if ce.ident.is_internal() { - trace!("Internal operation, not enforcing system object protection"); - return Ok(()); - } - - cand.iter().try_fold((), |(), cand| { - if PROTECTED_ENTRYCLASSES - .iter() - .any(|c| cand.attribute_equality(Attribute::Class, &c.to_partialvalue())) - { - trace!("Rejecting operation during pre_create check"); - Err(OperationError::SystemProtectedObject) - } else { - Ok(()) - } - }) - } - - #[instrument(level = "debug", name = "protected_pre_modify", skip_all)] - fn pre_modify( - _qs: &mut QueryServerWriteTransaction, - _pre_cand: &[Arc<EntrySealedCommitted>], - cand: &mut Vec<EntryInvalidCommitted>, - me: &ModifyEvent, - ) -> Result<(), OperationError> { - if me.ident.is_internal() { - trace!("Internal operation, not enforcing system object protection"); - return Ok(()); - } - // Prevent adding class: system, domain_info, tombstone, or recycled. - me.modlist.iter().try_fold((), |(), m| match m { - Modify::Present(a, v) => { - if a == Attribute::Class.as_ref() - && PROTECTED_ENTRYCLASSES.iter().any(|c| v == &c.to_value()) - { - trace!("Rejecting operation during pre_modify check"); - Err(OperationError::SystemProtectedObject) - } else { - Ok(()) - } - } - _ => Ok(()), - })?; - - // HARD block mods on tombstone or recycle. We soft block on the rest as they may - // have some allowed attrs. - cand.iter().try_fold((), |(), cand| { - if cand.attribute_equality(Attribute::Class, &EntryClass::Tombstone.into()) - || cand.attribute_equality(Attribute::Class, &EntryClass::Recycled.into()) - { - Err(OperationError::SystemProtectedObject) - } else { - Ok(()) - } - })?; - - // if class: system, check the mods are "allowed" - let system_pres = cand.iter().any(|c| { - // We don't need to check for domain info here because domain_info has a class - // system also. We just need to block it from being created. - c.attribute_equality(Attribute::Class, &EntryClass::System.into()) - }); - - trace!("class: system -> {}", system_pres); - // No system types being altered, return. - if !system_pres { - return Ok(()); - } - - // Something altered is system, check if it's allowed. - me.modlist.into_iter().try_fold((), |(), m| { - // Already hit an error, move on. - let a = match m { - Modify::Present(a, _) - | Modify::Removed(a, _) - | Modify::Set(a, _) - | Modify::Purged(a) => Some(a), - Modify::Assert(_, _) => None, - }; - if let Some(attr) = a { - match ALLOWED_ATTRS.contains(attr) { - true => Ok(()), - false => { - trace!("If you're getting this, you need to modify the ALLOWED_ATTRS list"); - Err(OperationError::SystemProtectedObject) - } - } - } else { - // Was not a mod needing checking - Ok(()) - } - }) - } - - #[instrument(level = "debug", name = "protected_pre_batch_modify", skip_all)] - fn pre_batch_modify( - _qs: &mut QueryServerWriteTransaction, - _pre_cand: &[Arc<EntrySealedCommitted>], - cand: &mut Vec<EntryInvalidCommitted>, - me: &BatchModifyEvent, - ) -> Result<(), OperationError> { - if me.ident.is_internal() { - trace!("Internal operation, not enforcing system object protection"); - return Ok(()); - } - - me.modset - .values() - .flat_map(|ml| ml.iter()) - .try_fold((), |(), m| match m { - Modify::Present(a, v) => { - if a == Attribute::Class.as_ref() - && PROTECTED_ENTRYCLASSES.iter().any(|c| v == &c.to_value()) - { - trace!("Rejecting operation during pre_batch_modify check"); - Err(OperationError::SystemProtectedObject) - } else { - Ok(()) - } - } - _ => Ok(()), - })?; - - // HARD block mods on tombstone or recycle. We soft block on the rest as they may - // have some allowed attrs. - cand.iter().try_fold((), |(), cand| { - if cand.attribute_equality(Attribute::Class, &EntryClass::Tombstone.into()) - || cand.attribute_equality(Attribute::Class, &EntryClass::Recycled.into()) - { - Err(OperationError::SystemProtectedObject) - } else { - Ok(()) - } - })?; - - // if class: system, check the mods are "allowed" - let system_pres = cand.iter().any(|c| { - // We don't need to check for domain info here because domain_info has a class - // system also. We just need to block it from being created. - c.attribute_equality(Attribute::Class, &EntryClass::System.into()) - }); - - trace!("{}: system -> {}", Attribute::Class, system_pres); - // No system types being altered, return. - if !system_pres { - return Ok(()); - } - - // Something altered is system, check if it's allowed. - me.modset - .values() - .flat_map(|ml| ml.iter()) - .try_fold((), |(), m| { - // Already hit an error, move on. - let a = match m { - Modify::Present(a, _) | Modify::Removed(a, _) | Modify::Set(a, _) | Modify::Purged(a) => Some(a), - Modify::Assert(_, _) => None, - }; - if let Some(attr) = a { - match ALLOWED_ATTRS.contains(attr) { - true => Ok(()), - false => { - - trace!("Rejecting operation during pre_batch_modify check, if you're getting this check ALLOWED_ATTRS"); - Err(OperationError::SystemProtectedObject) - }, - } - } else { - // Was not a mod needing checking - Ok(()) - } - }) - } - - #[instrument(level = "debug", name = "protected_pre_delete", skip_all)] - fn pre_delete( - _qs: &mut QueryServerWriteTransaction, - // Should these be EntrySealed - cand: &mut Vec<Entry<EntryInvalid, EntryCommitted>>, - de: &DeleteEvent, - ) -> Result<(), OperationError> { - if de.ident.is_internal() { - trace!("Internal operation, not enforcing system object protection"); - return Ok(()); - } - - cand.iter().try_fold((), |(), cand| { - if PROTECTED_ENTRYCLASSES - .iter() - .any(|c| cand.attribute_equality(Attribute::Class, &c.to_partialvalue())) - { - trace!("Rejecting operation during pre_delete check"); - Err(OperationError::SystemProtectedObject) - } else { - Ok(()) - } - }) - } -} - -#[cfg(test)] -mod tests { - use crate::prelude::*; - use std::sync::Arc; - - const UUID_TEST_ACCOUNT: Uuid = uuid::uuid!("cc8e95b4-c24f-4d68-ba54-8bed76f63930"); - const UUID_TEST_GROUP: Uuid = uuid::uuid!("81ec1640-3637-4a2f-8a52-874fa3c3c92f"); - const UUID_TEST_ACP: Uuid = uuid::uuid!("acae81d6-5ea7-4bd8-8f7f-fcec4c0dd647"); - - lazy_static! { - pub static ref TEST_ACCOUNT: EntryInitNew = entry_init!( - (Attribute::Class, EntryClass::Account.to_value()), - (Attribute::Class, EntryClass::ServiceAccount.to_value()), - (Attribute::Class, EntryClass::MemberOf.to_value()), - (Attribute::Name, Value::new_iname("test_account_1")), - (Attribute::DisplayName, Value::new_utf8s("test_account_1")), - (Attribute::Uuid, Value::Uuid(UUID_TEST_ACCOUNT)), - (Attribute::MemberOf, Value::Refer(UUID_TEST_GROUP)) - ); - pub static ref TEST_GROUP: EntryInitNew = entry_init!( - (Attribute::Class, EntryClass::Group.to_value()), - (Attribute::Name, Value::new_iname("test_group_a")), - (Attribute::Uuid, Value::Uuid(UUID_TEST_GROUP)), - (Attribute::Member, Value::Refer(UUID_TEST_ACCOUNT)) - ); - pub static ref ALLOW_ALL: EntryInitNew = entry_init!( - (Attribute::Class, EntryClass::Object.to_value()), - ( - Attribute::Class, - EntryClass::AccessControlProfile.to_value() - ), - ( - Attribute::Class, - EntryClass::AccessControlTargetScope.to_value() - ), - ( - Attribute::Class, - EntryClass::AccessControlReceiverGroup.to_value() - ), - (Attribute::Class, EntryClass::AccessControlModify.to_value()), - (Attribute::Class, EntryClass::AccessControlCreate.to_value()), - (Attribute::Class, EntryClass::AccessControlDelete.to_value()), - (Attribute::Class, EntryClass::AccessControlSearch.to_value()), - ( - Attribute::Name, - Value::new_iname("idm_admins_acp_allow_all_test") - ), - (Attribute::Uuid, Value::Uuid(UUID_TEST_ACP)), - (Attribute::AcpReceiverGroup, Value::Refer(UUID_TEST_GROUP)), - ( - Attribute::AcpTargetScope, - Value::new_json_filter_s("{\"pres\":\"class\"}").expect("filter") - ), - (Attribute::AcpSearchAttr, Value::from(Attribute::Name)), - (Attribute::AcpSearchAttr, Value::from(Attribute::Class)), - (Attribute::AcpSearchAttr, Value::from(Attribute::Uuid)), - (Attribute::AcpSearchAttr, Value::new_iutf8("classname")), - ( - Attribute::AcpSearchAttr, - Value::new_iutf8(Attribute::AttributeName.as_ref()) - ), - (Attribute::AcpModifyClass, EntryClass::System.to_value()), - (Attribute::AcpModifyClass, Value::new_iutf8("domain_info")), - ( - Attribute::AcpModifyRemovedAttr, - Value::from(Attribute::Class) - ), - ( - Attribute::AcpModifyRemovedAttr, - Value::from(Attribute::DisplayName) - ), - (Attribute::AcpModifyRemovedAttr, Value::from(Attribute::May)), - ( - Attribute::AcpModifyRemovedAttr, - Value::from(Attribute::Must) - ), - ( - Attribute::AcpModifyRemovedAttr, - Value::from(Attribute::DomainName) - ), - ( - Attribute::AcpModifyRemovedAttr, - Value::from(Attribute::DomainDisplayName) - ), - ( - Attribute::AcpModifyRemovedAttr, - Value::from(Attribute::DomainUuid) - ), - ( - Attribute::AcpModifyRemovedAttr, - Value::from(Attribute::DomainSsid) - ), - ( - Attribute::AcpModifyRemovedAttr, - Value::from(Attribute::FernetPrivateKeyStr) - ), - ( - Attribute::AcpModifyRemovedAttr, - Value::from(Attribute::Es256PrivateKeyDer) - ), - ( - Attribute::AcpModifyRemovedAttr, - Value::from(Attribute::PrivateCookieKey) - ), - ( - Attribute::AcpModifyPresentAttr, - Value::from(Attribute::Class) - ), - ( - Attribute::AcpModifyPresentAttr, - Value::from(Attribute::DisplayName) - ), - (Attribute::AcpModifyPresentAttr, Value::from(Attribute::May)), - ( - Attribute::AcpModifyPresentAttr, - Value::from(Attribute::Must) - ), - ( - Attribute::AcpModifyPresentAttr, - Value::from(Attribute::DomainName) - ), - ( - Attribute::AcpModifyPresentAttr, - Value::from(Attribute::DomainDisplayName) - ), - ( - Attribute::AcpModifyPresentAttr, - Value::from(Attribute::DomainUuid) - ), - ( - Attribute::AcpModifyPresentAttr, - Value::from(Attribute::DomainSsid) - ), - ( - Attribute::AcpModifyPresentAttr, - Value::from(Attribute::FernetPrivateKeyStr) - ), - ( - Attribute::AcpModifyPresentAttr, - Value::from(Attribute::Es256PrivateKeyDer) - ), - ( - Attribute::AcpModifyPresentAttr, - Value::from(Attribute::PrivateCookieKey) - ), - (Attribute::AcpCreateClass, EntryClass::Object.to_value()), - (Attribute::AcpCreateClass, EntryClass::Account.to_value()), - (Attribute::AcpCreateClass, EntryClass::Person.to_value()), - (Attribute::AcpCreateClass, EntryClass::System.to_value()), - (Attribute::AcpCreateClass, EntryClass::DomainInfo.to_value()), - (Attribute::AcpCreateAttr, Value::from(Attribute::Name)), - (Attribute::AcpCreateAttr, EntryClass::Class.to_value(),), - ( - Attribute::AcpCreateAttr, - Value::from(Attribute::Description), - ), - ( - Attribute::AcpCreateAttr, - Value::from(Attribute::DisplayName), - ), - (Attribute::AcpCreateAttr, Value::from(Attribute::DomainName),), - ( - Attribute::AcpCreateAttr, - Value::from(Attribute::DomainDisplayName) - ), - (Attribute::AcpCreateAttr, Value::from(Attribute::DomainUuid)), - (Attribute::AcpCreateAttr, Value::from(Attribute::DomainSsid)), - (Attribute::AcpCreateAttr, Value::from(Attribute::Uuid)), - ( - Attribute::AcpCreateAttr, - Value::from(Attribute::FernetPrivateKeyStr) - ), - ( - Attribute::AcpCreateAttr, - Value::from(Attribute::Es256PrivateKeyDer) - ), - ( - Attribute::AcpCreateAttr, - Value::from(Attribute::PrivateCookieKey) - ), - (Attribute::AcpCreateAttr, Value::from(Attribute::Version)) - ); - pub static ref PRELOAD: Vec<EntryInitNew> = - vec![TEST_ACCOUNT.clone(), TEST_GROUP.clone(), ALLOW_ALL.clone()]; - pub static ref E_TEST_ACCOUNT: Arc<EntrySealedCommitted> = - Arc::new(TEST_ACCOUNT.clone().into_sealed_committed()); - } - - #[test] - fn test_pre_create_deny() { - // Test creating with class: system is rejected. - let e = entry_init!( - (Attribute::Class, EntryClass::Account.to_value()), - (Attribute::Class, EntryClass::Person.to_value()), - (Attribute::Class, EntryClass::System.to_value()), - (Attribute::Name, Value::new_iname("testperson")), - ( - Attribute::DisplayName, - Value::Utf8("testperson".to_string()) - ) - ); - - let create = vec![e]; - let preload = PRELOAD.clone(); - - run_create_test!( - Err(OperationError::SystemProtectedObject), - preload, - create, - Some(E_TEST_ACCOUNT.clone()), - |_| {} - ); - } - - #[test] - fn test_pre_modify_system_deny() { - // Test modify of class to a system is denied - let e = entry_init!( - (Attribute::Class, EntryClass::Account.to_value()), - (Attribute::Class, EntryClass::Person.to_value()), - (Attribute::Class, EntryClass::System.to_value()), - (Attribute::Name, Value::new_iname("testperson")), - ( - Attribute::DisplayName, - Value::Utf8("testperson".to_string()) - ) - ); - - let mut preload = PRELOAD.clone(); - preload.push(e); - - run_modify_test!( - Err(OperationError::SystemProtectedObject), - preload, - filter!(f_eq(Attribute::Name, PartialValue::new_iname("testperson"))), - modlist!([ - m_purge(Attribute::DisplayName), - m_pres(Attribute::DisplayName, &Value::new_utf8s("system test")), - ]), - Some(E_TEST_ACCOUNT.clone()), - |_| {}, - |_| {} - ); - } - - #[test] - fn test_pre_modify_class_add_deny() { - // Show that adding a system class is denied - // TODO: replace this with a `SchemaClass` object - let e = entry_init!( - (Attribute::Class, EntryClass::Object.to_value()), - (Attribute::Class, EntryClass::ClassType.to_value()), - (Attribute::ClassName, Value::new_iutf8("testclass")), - ( - Attribute::Uuid, - Value::Uuid(uuid::uuid!("66c68b2f-d02c-4243-8013-7946e40fe321")) - ), - ( - Attribute::Description, - Value::Utf8("class test".to_string()) - ) - ); - let mut preload = PRELOAD.clone(); - preload.push(e); - - run_modify_test!( - Ok(()), - preload, - filter!(f_eq( - Attribute::ClassName, - PartialValue::new_iutf8("testclass") - )), - modlist!([ - m_pres(Attribute::May, &Value::from(Attribute::Name)), - m_pres(Attribute::Must, &Value::from(Attribute::Name)), - ]), - Some(E_TEST_ACCOUNT.clone()), - |_| {}, - |_| {} - ); - } - - #[test] - fn test_pre_delete_deny() { - // Test deleting with class: system is rejected. - let e = entry_init!( - (Attribute::Class, EntryClass::Account.to_value()), - (Attribute::Class, EntryClass::Person.to_value()), - (Attribute::Class, EntryClass::System.to_value()), - (Attribute::Name, Value::new_iname("testperson")), - ( - Attribute::DisplayName, - Value::Utf8("testperson".to_string()) - ) - ); - - let mut preload = PRELOAD.clone(); - preload.push(e); - - run_delete_test!( - Err(OperationError::SystemProtectedObject), - preload, - filter!(f_eq(Attribute::Name, PartialValue::new_iname("testperson"))), - Some(E_TEST_ACCOUNT.clone()), - |_| {} - ); - } - - #[test] - fn test_modify_domain() { - // Can edit *my* domain_ssid and domain_name - // Show that adding a system class is denied - let e = entry_init!( - (Attribute::Class, EntryClass::DomainInfo.to_value()), - (Attribute::Name, Value::new_iname("domain_example.net.au")), - (Attribute::Uuid, Value::Uuid(uuid::uuid!("96fd1112-28bc-48ae-9dda-5acb4719aaba"))), - ( - Attribute::Description, - Value::new_utf8s("Demonstration of a remote domain's info being created for uuid generation in test_modify_domain") - ), - (Attribute::DomainUuid, Value::Uuid(uuid::uuid!("96fd1112-28bc-48ae-9dda-5acb4719aaba"))), - (Attribute::DomainName, Value::new_iname("example.net.au")), - (Attribute::DomainDisplayName, Value::Utf8("example.net.au".to_string())), - (Attribute::DomainSsid, Value::Utf8("Example_Wifi".to_string())), - (Attribute::Version, Value::Uint32(1)) - ); - - let mut preload = PRELOAD.clone(); - preload.push(e); - - run_modify_test!( - Ok(()), - preload, - filter!(f_eq( - Attribute::Name, - PartialValue::new_iname("domain_example.net.au") - )), - modlist!([ - m_purge(Attribute::DomainSsid), - m_pres(Attribute::DomainSsid, &Value::new_utf8s("NewExampleWifi")), - ]), - Some(E_TEST_ACCOUNT.clone()), - |_| {}, - |_| {} - ); - } - - #[test] - fn test_ext_create_domain() { - // can not add a domain_info type - note the lack of class: system - let e = entry_init!( - (Attribute::Class, EntryClass::DomainInfo.to_value()), - (Attribute::Name, Value::new_iname("domain_example.net.au")), - (Attribute::Uuid, Value::Uuid(uuid::uuid!("96fd1112-28bc-48ae-9dda-5acb4719aaba"))), - ( - Attribute::Description, - Value::new_utf8s("Demonstration of a remote domain's info being created for uuid generation in test_modify_domain") - ), - (Attribute::DomainUuid, Value::Uuid(uuid::uuid!("96fd1112-28bc-48ae-9dda-5acb4719aaba"))), - (Attribute::DomainName, Value::new_iname("example.net.au")), - (Attribute::DomainDisplayName, Value::Utf8("example.net.au".to_string())), - (Attribute::DomainSsid, Value::Utf8("Example_Wifi".to_string())), - (Attribute::Version, Value::Uint32(1)) - ); - - let create = vec![e]; - let preload = PRELOAD.clone(); - - run_create_test!( - Err(OperationError::SystemProtectedObject), - preload, - create, - Some(E_TEST_ACCOUNT.clone()), - |_| {} - ); - } - - #[test] - fn test_delete_domain() { - // On the real thing we have a class: system, but to prove the point ... - let e = entry_init!( - (Attribute::Class, EntryClass::DomainInfo.to_value()), - (Attribute::Name, Value::new_iname("domain_example.net.au")), - (Attribute::Uuid, Value::Uuid(uuid::uuid!("96fd1112-28bc-48ae-9dda-5acb4719aaba"))), - ( - Attribute::Description, - Value::new_utf8s("Demonstration of a remote domain's info being created for uuid generation in test_modify_domain") - ), - (Attribute::DomainUuid, Value::Uuid(uuid::uuid!("96fd1112-28bc-48ae-9dda-5acb4719aaba"))), - (Attribute::DomainName, Value::new_iname("example.net.au")), - (Attribute::DomainDisplayName, Value::Utf8("example.net.au".to_string())), - (Attribute::DomainSsid, Value::Utf8("Example_Wifi".to_string())), - (Attribute::Version, Value::Uint32(1)) - ); - - let mut preload = PRELOAD.clone(); - preload.push(e); - - run_delete_test!( - Err(OperationError::SystemProtectedObject), - preload, - filter!(f_eq( - Attribute::Name, - PartialValue::new_iname("domain_example.net.au") - )), - Some(E_TEST_ACCOUNT.clone()), - |_| {} - ); - } -} diff --git a/server/lib/src/schema.rs b/server/lib/src/schema.rs index 4f7c55a34..d87170bce 100644 --- a/server/lib/src/schema.rs +++ b/server/lib/src/schema.rs @@ -1366,6 +1366,36 @@ impl SchemaWriteTransaction<'_> { syntax: SyntaxType::Utf8StringInsensitive, }, ); + self.attributes.insert( + Attribute::AcpModifyPresentClass, + SchemaAttribute { + name: Attribute::AcpModifyPresentClass, + uuid: UUID_SCHEMA_ATTR_ACP_MODIFY_PRESENT_CLASS, + description: String::from("The set of class values that could be asserted or added to an entry. Only applies to modify::present operations on class."), + multivalue: true, + unique: false, + phantom: false, + sync_allowed: false, + replicated: Replicated::True, + indexed: false, + syntax: SyntaxType::Utf8StringInsensitive, + }, + ); + self.attributes.insert( + Attribute::AcpModifyRemoveClass, + SchemaAttribute { + name: Attribute::AcpModifyRemoveClass, + uuid: UUID_SCHEMA_ATTR_ACP_MODIFY_REMOVE_CLASS, + description: String::from("The set of class values that could be asserted or added to an entry. Only applies to modify::remove operations on class."), + multivalue: true, + unique: false, + phantom: false, + sync_allowed: false, + replicated: Replicated::True, + indexed: false, + syntax: SyntaxType::Utf8StringInsensitive, + }, + ); self.attributes.insert( Attribute::EntryManagedBy, SchemaAttribute { @@ -2069,6 +2099,8 @@ impl SchemaWriteTransaction<'_> { Attribute::AcpModifyRemovedAttr, Attribute::AcpModifyPresentAttr, Attribute::AcpModifyClass, + Attribute::AcpModifyPresentClass, + Attribute::AcpModifyRemoveClass, ], ..Default::default() }, diff --git a/server/lib/src/server/access/create.rs b/server/lib/src/server/access/create.rs index 5f6fec3ba..8c0888bbc 100644 --- a/server/lib/src/server/access/create.rs +++ b/server/lib/src/server/access/create.rs @@ -1,16 +1,17 @@ use super::profiles::{ AccessControlCreateResolved, AccessControlReceiverCondition, AccessControlTargetCondition, }; +use super::protected::PROTECTED_ENTRY_CLASSES; use crate::prelude::*; use std::collections::BTreeSet; pub(super) enum CreateResult { - Denied, + Deny, Grant, } enum IResult { - Denied, + Deny, Grant, Ignore, } @@ -25,25 +26,25 @@ pub(super) fn apply_create_access<'a>( // This module can never yield a grant. match protected_filter_entry(ident, entry) { - IResult::Denied => denied = true, + IResult::Deny => denied = true, IResult::Grant | IResult::Ignore => {} } match create_filter_entry(ident, related_acp, entry) { - IResult::Denied => denied = true, + IResult::Deny => denied = true, IResult::Grant => grant = true, IResult::Ignore => {} } if denied { // Something explicitly said no. - CreateResult::Denied + CreateResult::Deny } else if grant { // Something said yes CreateResult::Grant } else { // Nothing said yes. - CreateResult::Denied + CreateResult::Deny } } @@ -60,7 +61,7 @@ fn create_filter_entry<'a>( } IdentType::Synch(_) => { security_critical!("Blocking sync check"); - return IResult::Denied; + return IResult::Deny; } IdentType::User(_) => {} }; @@ -69,7 +70,7 @@ fn create_filter_entry<'a>( match ident.access_scope() { AccessScope::ReadOnly | AccessScope::Synchronise => { security_access!("denied ❌ - identity access scope is not permitted to create"); - return IResult::Denied; + return IResult::Deny; } AccessScope::ReadWrite => { // As you were @@ -96,7 +97,7 @@ fn create_filter_entry<'a>( Some(s) => s.collect(), None => { admin_error!("Class set failed to build - corrupted entry?"); - return IResult::Denied; + return IResult::Deny; } }; @@ -173,22 +174,22 @@ fn protected_filter_entry(ident: &Identity, entry: &Entry<EntryInit, EntryNew>) } IdentType::Synch(_) => { security_access!("sync agreements may not directly create entities"); - IResult::Denied + IResult::Deny } IdentType::User(_) => { // Now check things ... - - // For now we just block create on sync object - if let Some(classes) = entry.get_ava_set(Attribute::Class) { - if classes.contains(&EntryClass::SyncObject.into()) { - // Block the mod - security_access!("attempt to create with protected class type"); - IResult::Denied - } else { + if let Some(classes) = entry.get_ava_as_iutf8(Attribute::Class) { + if classes.is_disjoint(&PROTECTED_ENTRY_CLASSES) { + // It's different, go ahead IResult::Ignore + } else { + // Block the mod, something is present + security_access!("attempt to create with protected class type"); + IResult::Deny } } else { - // Nothing to check. + // Nothing to check - this entry will fail to create anyway because it has + // no classes IResult::Ignore } } diff --git a/server/lib/src/server/access/delete.rs b/server/lib/src/server/access/delete.rs index 8b49a22b6..465224fef 100644 --- a/server/lib/src/server/access/delete.rs +++ b/server/lib/src/server/access/delete.rs @@ -1,16 +1,17 @@ use super::profiles::{ AccessControlDeleteResolved, AccessControlReceiverCondition, AccessControlTargetCondition, }; +use super::protected::PROTECTED_ENTRY_CLASSES; use crate::prelude::*; use std::sync::Arc; pub(super) enum DeleteResult { - Denied, + Deny, Grant, } enum IResult { - Denied, + Deny, Grant, Ignore, } @@ -24,25 +25,25 @@ pub(super) fn apply_delete_access<'a>( let mut grant = false; match protected_filter_entry(ident, entry) { - IResult::Denied => denied = true, + IResult::Deny => denied = true, IResult::Grant | IResult::Ignore => {} } match delete_filter_entry(ident, related_acp, entry) { - IResult::Denied => denied = true, + IResult::Deny => denied = true, IResult::Grant => grant = true, IResult::Ignore => {} } if denied { // Something explicitly said no. - DeleteResult::Denied + DeleteResult::Deny } else if grant { // Something said yes DeleteResult::Grant } else { // Nothing said yes. - DeleteResult::Denied + DeleteResult::Deny } } @@ -59,7 +60,7 @@ fn delete_filter_entry<'a>( } IdentType::Synch(_) => { security_critical!("Blocking sync check"); - return IResult::Denied; + return IResult::Deny; } IdentType::User(_) => {} }; @@ -68,7 +69,7 @@ fn delete_filter_entry<'a>( match ident.access_scope() { AccessScope::ReadOnly | AccessScope::Synchronise => { security_access!("denied ❌ - identity access scope is not permitted to delete"); - return IResult::Denied; + return IResult::Deny; } AccessScope::ReadWrite => { // As you were @@ -152,28 +153,30 @@ fn protected_filter_entry(ident: &Identity, entry: &Arc<EntrySealedCommitted>) - } IdentType::Synch(_) => { security_access!("sync agreements may not directly delete entities"); - IResult::Denied + IResult::Deny } IdentType::User(_) => { - // Now check things ... - - // For now we just block create on sync object - if let Some(classes) = entry.get_ava_set(Attribute::Class) { - if classes.contains(&EntryClass::SyncObject.into()) { - // Block the mod - security_access!("attempt to delete with protected class type"); - return IResult::Denied; - } - }; - // Prevent deletion of entries that exist in the system controlled entry range. if entry.get_uuid() <= UUID_ANONYMOUS { security_access!("attempt to delete system builtin entry"); - return IResult::Denied; + return IResult::Deny; } - // Checks exhausted, no more input from us - IResult::Ignore + // Prevent deleting some protected types. + if let Some(classes) = entry.get_ava_as_iutf8(Attribute::Class) { + if classes.is_disjoint(&PROTECTED_ENTRY_CLASSES) { + // It's different, go ahead + IResult::Ignore + } else { + // Block the mod, something is present + security_access!("attempt to create with protected class type"); + IResult::Deny + } + } else { + // Nothing to check - this entry will fail to create anyway because it has + // no classes + IResult::Ignore + } } } } diff --git a/server/lib/src/server/access/mod.rs b/server/lib/src/server/access/mod.rs index 956bcb512..9c13ca1b6 100644 --- a/server/lib/src/server/access/mod.rs +++ b/server/lib/src/server/access/mod.rs @@ -50,12 +50,13 @@ mod create; mod delete; mod modify; pub mod profiles; +mod protected; mod search; #[derive(Debug, Clone, PartialEq, Eq)] pub enum Access { Grant, - Denied, + Deny, Allow(BTreeSet<Attribute>), } @@ -63,7 +64,7 @@ impl From<&Access> for ScimAttributeEffectiveAccess { fn from(value: &Access) -> Self { match value { Access::Grant => Self::Grant, - Access::Denied => Self::Denied, + Access::Deny => Self::Deny, Access::Allow(set) => Self::Allow(set.clone()), } } @@ -72,7 +73,7 @@ impl From<&Access> for ScimAttributeEffectiveAccess { #[derive(Debug, Clone, PartialEq, Eq)] pub enum AccessClass { Grant, - Denied, + Deny, Allow(BTreeSet<AttrString>), } @@ -86,12 +87,22 @@ pub struct AccessEffectivePermission { pub search: Access, pub modify_pres: Access, pub modify_rem: Access, - pub modify_class: AccessClass, + pub modify_pres_class: AccessClass, + pub modify_rem_class: AccessClass, } -pub enum AccessResult { +pub enum AccessBasicResult { // Deny this operation unconditionally. - Denied, + Deny, + // Unbounded allow, provided no deny state exists. + Grant, + // This module makes no decisions about this entry. + Ignore, +} + +pub enum AccessSrchResult { + // Deny this operation unconditionally. + Deny, // Unbounded allow, provided no deny state exists. Grant, // This module makes no decisions about this entry. @@ -99,24 +110,37 @@ pub enum AccessResult { // Limit the allowed attr set to this - this doesn't // allow anything, it constrains what might be allowed // by a later module. - Constrain(BTreeSet<Attribute>), - // Allow these attributes within constraints. - Allow(BTreeSet<Attribute>), + /* + Constrain { + attr: BTreeSet<Attribute>, + }, + */ + Allow { attr: BTreeSet<Attribute> }, } -#[allow(dead_code)] -pub enum AccessResultClass<'a> { +pub enum AccessModResult<'a> { // Deny this operation unconditionally. - Denied, - // Unbounded allow, provided no denied exists. - Grant, + Deny, + // Unbounded allow, provided no deny state exists. + // Grant, // This module makes no decisions about this entry. Ignore, // Limit the allowed attr set to this - this doesn't - // allow anything, it constrains what might be allowed. - Constrain(BTreeSet<&'a str>), - // Allow these attributes within constraints. - Allow(BTreeSet<&'a str>), + // allow anything, it constrains what might be allowed + // by a later module. + Constrain { + pres_attr: BTreeSet<Attribute>, + rem_attr: BTreeSet<Attribute>, + pres_cls: Option<BTreeSet<&'a str>>, + rem_cls: Option<BTreeSet<&'a str>>, + }, + // Allow these modifications within constraints. + Allow { + pres_attr: BTreeSet<Attribute>, + rem_attr: BTreeSet<Attribute>, + pres_class: BTreeSet<&'a str>, + rem_class: BTreeSet<&'a str>, + }, } // ========================================================================= @@ -303,7 +327,7 @@ pub trait AccessControlsTransaction<'a> { .into_iter() .filter(|e| { match apply_search_access(ident, related_acp.as_slice(), e) { - SearchResult::Denied => false, + SearchResult::Deny => false, SearchResult::Grant => true, SearchResult::Allow(allowed_attrs) => { // The allow set constrained. @@ -401,7 +425,7 @@ pub trait AccessControlsTransaction<'a> { .into_iter() .filter_map(|entry| { match apply_search_access(&se.ident, &search_related_acp, &entry) { - SearchResult::Denied => { + SearchResult::Deny => { None } SearchResult::Grant => { @@ -536,7 +560,8 @@ pub trait AccessControlsTransaction<'a> { // Build the set of classes that we to work on, only in terms of "addition". To remove // I think we have no limit, but ... william of the future may find a problem with this // policy. - let mut requested_classes: BTreeSet<&str> = Default::default(); + let mut requested_pres_classes: BTreeSet<&str> = Default::default(); + let mut requested_rem_classes: BTreeSet<&str> = Default::default(); for modify in me.modlist.iter() { match modify { @@ -548,27 +573,33 @@ pub trait AccessControlsTransaction<'a> { // existence, and second, we would have failed the mod at schema checking // earlier in the process as these were not correctly type. As a result // we can trust these to be correct here and not to be "None". - requested_classes.extend(v.to_str()) + requested_pres_classes.extend(v.to_str()) } } Modify::Removed(a, v) => { if a == Attribute::Class.as_ref() { - requested_classes.extend(v.to_str()) + requested_rem_classes.extend(v.to_str()) } } Modify::Set(a, v) => { if a == Attribute::Class.as_ref() { - // flatten to remove the option down to an iterator - requested_classes.extend(v.as_iutf8_iter().into_iter().flatten()) + // This is a reasonably complex case - we actually have to contemplate + // the difference between what exists and what doesn't, but that's per-entry. + // + // for now, we treat this as both pres and rem, but I think that ultimately + // to fix this we need to make all modifies apply in terms of "batch mod" + requested_pres_classes.extend(v.as_iutf8_iter().into_iter().flatten()); + requested_rem_classes.extend(v.as_iutf8_iter().into_iter().flatten()); } } _ => {} } } - debug!(?requested_pres, "Requested present set"); - debug!(?requested_rem, "Requested remove set"); - debug!(?requested_classes, "Requested class set"); + debug!(?requested_pres, "Requested present attribute set"); + debug!(?requested_rem, "Requested remove attribute set"); + debug!(?requested_pres_classes, "Requested present class set"); + debug!(?requested_rem_classes, "Requested remove class set"); let sync_agmts = self.get_sync_agreements(); @@ -576,9 +607,16 @@ pub trait AccessControlsTransaction<'a> { debug!(entry_id = %e.get_display_id()); match apply_modify_access(&me.ident, related_acp.as_slice(), sync_agmts, e) { - ModifyResult::Denied => false, + ModifyResult::Deny => false, ModifyResult::Grant => true, - ModifyResult::Allow { pres, rem, cls } => { + ModifyResult::Allow { + pres, + rem, + pres_cls, + rem_cls, + } => { + let mut decision = true; + if !requested_pres.is_subset(&pres) { security_error!("requested_pres is not a subset of allowed"); security_error!( @@ -586,23 +624,41 @@ pub trait AccessControlsTransaction<'a> { requested_pres, pres ); - false - } else if !requested_rem.is_subset(&rem) { + decision = false + }; + + if !requested_rem.is_subset(&rem) { security_error!("requested_rem is not a subset of allowed"); security_error!("requested_rem: {:?} !⊆ allowed: {:?}", requested_rem, rem); - false - } else if !requested_classes.is_subset(&cls) { - security_error!("requested_classes is not a subset of allowed"); + decision = false; + }; + + if !requested_pres_classes.is_subset(&pres_cls) { + security_error!("requested_pres_classes is not a subset of allowed"); security_error!( - "requested_classes: {:?} !⊆ allowed: {:?}", - requested_classes, - cls + "requested_pres_classes: {:?} !⊆ allowed: {:?}", + requested_pres_classes, + pres_cls ); - false - } else { + decision = false; + }; + + if !requested_rem_classes.is_subset(&rem_cls) { + security_error!("requested_rem_classes is not a subset of allowed"); + security_error!( + "requested_rem_classes: {:?} !⊆ allowed: {:?}", + requested_rem_classes, + rem_cls + ); + decision = false; + } + + if decision { debug!("passed pres, rem, classes check."); - true - } // if acc == false + } + + // Yield the result + decision } } }); @@ -668,47 +724,55 @@ pub trait AccessControlsTransaction<'a> { }) .collect(); - // Build the set of classes that we to work on, only in terms of "addition". To remove - // I think we have no limit, but ... william of the future may find a problem with this - // policy. - let requested_classes: BTreeSet<&str> = modlist - .iter() - .filter_map(|m| match m { + let mut requested_pres_classes: BTreeSet<&str> = Default::default(); + let mut requested_rem_classes: BTreeSet<&str> = Default::default(); + + for modify in modlist.iter() { + match modify { Modify::Present(a, v) => { if a == Attribute::Class.as_ref() { - // Here we have an option<&str> which could mean there is a risk of - // a malicious entity attempting to trick us by masking class mods - // in non-iutf8 types. However, the server first won't respect their - // existence, and second, we would have failed the mod at schema checking - // earlier in the process as these were not correctly type. As a result - // we can trust these to be correct here and not to be "None". - v.to_str() - } else { - None + requested_pres_classes.extend(v.to_str()) } } Modify::Removed(a, v) => { if a == Attribute::Class.as_ref() { - v.to_str() - } else { - None + requested_rem_classes.extend(v.to_str()) } } - _ => None, - }) - .collect(); + Modify::Set(a, v) => { + if a == Attribute::Class.as_ref() { + // This is a reasonably complex case - we actually have to contemplate + // the difference between what exists and what doesn't, but that's per-entry. + // + // for now, we treat this as both pres and rem, but I think that ultimately + // to fix this we need to make all modifies apply in terms of "batch mod" + requested_pres_classes.extend(v.as_iutf8_iter().into_iter().flatten()); + requested_rem_classes.extend(v.as_iutf8_iter().into_iter().flatten()); + } + } + _ => {} + } + } debug!(?requested_pres, "Requested present set"); debug!(?requested_rem, "Requested remove set"); - debug!(?requested_classes, "Requested class set"); + debug!(?requested_pres_classes, "Requested present class set"); + debug!(?requested_rem_classes, "Requested remove class set"); debug!(entry_id = %e.get_display_id()); let sync_agmts = self.get_sync_agreements(); match apply_modify_access(&me.ident, related_acp.as_slice(), sync_agmts, e) { - ModifyResult::Denied => false, + ModifyResult::Deny => false, ModifyResult::Grant => true, - ModifyResult::Allow { pres, rem, cls } => { + ModifyResult::Allow { + pres, + rem, + pres_cls, + rem_cls, + } => { + let mut decision = true; + if !requested_pres.is_subset(&pres) { security_error!("requested_pres is not a subset of allowed"); security_error!( @@ -716,23 +780,41 @@ pub trait AccessControlsTransaction<'a> { requested_pres, pres ); - false - } else if !requested_rem.is_subset(&rem) { + decision = false + }; + + if !requested_rem.is_subset(&rem) { security_error!("requested_rem is not a subset of allowed"); security_error!("requested_rem: {:?} !⊆ allowed: {:?}", requested_rem, rem); - false - } else if !requested_classes.is_subset(&cls) { - security_error!("requested_classes is not a subset of allowed"); + decision = false; + }; + + if !requested_pres_classes.is_subset(&pres_cls) { + security_error!("requested_pres_classes is not a subset of allowed"); security_error!( "requested_classes: {:?} !⊆ allowed: {:?}", - requested_classes, - cls + requested_pres_classes, + pres_cls ); - false - } else { - security_access!("passed pres, rem, classes check."); - true - } // if acc == false + decision = false; + }; + + if !requested_rem_classes.is_subset(&rem_cls) { + security_error!("requested_rem_classes is not a subset of allowed"); + security_error!( + "requested_classes: {:?} !⊆ allowed: {:?}", + requested_rem_classes, + rem_cls + ); + decision = false; + } + + if decision { + debug!("passed pres, rem, classes check."); + } + + // Yield the result + decision } } }); @@ -780,7 +862,7 @@ pub trait AccessControlsTransaction<'a> { // For each entry let r = entries.iter().all(|e| { match apply_create_access(&ce.ident, related_acp.as_slice(), e) { - CreateResult::Denied => false, + CreateResult::Deny => false, CreateResult::Grant => true, } }); @@ -836,7 +918,7 @@ pub trait AccessControlsTransaction<'a> { // For each entry let r = entries.iter().all(|e| { match apply_delete_access(&de.ident, related_acp.as_slice(), e) { - DeleteResult::Denied => false, + DeleteResult::Deny => false, DeleteResult::Grant => true, } }); @@ -925,7 +1007,7 @@ pub trait AccessControlsTransaction<'a> { ) -> AccessEffectivePermission { // == search == let search_effective = match apply_search_access(ident, search_related_acp, entry) { - SearchResult::Denied => Access::Denied, + SearchResult::Deny => Access::Deny, SearchResult::Grant => Access::Grant, SearchResult::Allow(allowed_attrs) => { // Bound by requested attrs? @@ -934,14 +1016,30 @@ pub trait AccessControlsTransaction<'a> { }; // == modify == - let (modify_pres, modify_rem, modify_class) = + let (modify_pres, modify_rem, modify_pres_class, modify_rem_class) = match apply_modify_access(ident, modify_related_acp, sync_agmts, entry) { - ModifyResult::Denied => (Access::Denied, Access::Denied, AccessClass::Denied), - ModifyResult::Grant => (Access::Grant, Access::Grant, AccessClass::Grant), - ModifyResult::Allow { pres, rem, cls } => ( + ModifyResult::Deny => ( + Access::Deny, + Access::Deny, + AccessClass::Deny, + AccessClass::Deny, + ), + ModifyResult::Grant => ( + Access::Grant, + Access::Grant, + AccessClass::Grant, + AccessClass::Grant, + ), + ModifyResult::Allow { + pres, + rem, + pres_cls, + rem_cls, + } => ( Access::Allow(pres.into_iter().collect()), Access::Allow(rem.into_iter().collect()), - AccessClass::Allow(cls.into_iter().map(|s| s.into()).collect()), + AccessClass::Allow(pres_cls.into_iter().map(|s| s.into()).collect()), + AccessClass::Allow(rem_cls.into_iter().map(|s| s.into()).collect()), ), }; @@ -949,7 +1047,7 @@ pub trait AccessControlsTransaction<'a> { let delete_status = apply_delete_access(ident, delete_related_acp, entry); let delete = match delete_status { - DeleteResult::Denied => false, + DeleteResult::Deny => false, DeleteResult::Grant => true, }; @@ -960,7 +1058,8 @@ pub trait AccessControlsTransaction<'a> { search: search_effective, modify_pres, modify_rem, - modify_class, + modify_pres_class, + modify_rem_class, } } } @@ -2166,6 +2265,8 @@ mod tests { "name class", // And the class allowed is account EntryClass::Account.into(), + // And the class allowed is account + EntryClass::Account.into(), ); // Allow member, class is group. IE not account let acp_deny = AccessControlModify::from_raw( @@ -2182,8 +2283,8 @@ mod tests { "member class", // Allow rem name and class "member class", - // And the class allowed is account - "group", + EntryClass::Group.into(), + EntryClass::Group.into(), ); // Does not have a pres or rem class in attrs let acp_no_class = AccessControlModify::from_raw( @@ -2201,7 +2302,8 @@ mod tests { // Allow rem name and class "name class", // And the class allowed is NOT an account ... - "group", + EntryClass::Group.into(), + EntryClass::Group.into(), ); // Test allowed pres @@ -2287,6 +2389,7 @@ mod tests { "name class", // And the class allowed is account EntryClass::Account.into(), + EntryClass::Account.into(), ); test_acp_modify!(&me_pres_ro, vec![acp_allow.clone()], &r_set, false); @@ -2614,7 +2717,8 @@ mod tests { search: Access::Allow(btreeset![Attribute::Name]), modify_pres: Access::Allow(BTreeSet::new()), modify_rem: Access::Allow(BTreeSet::new()), - modify_class: AccessClass::Allow(BTreeSet::new()), + modify_pres_class: AccessClass::Allow(BTreeSet::new()), + modify_rem_class: AccessClass::Allow(BTreeSet::new()), }] ) } @@ -2647,6 +2751,7 @@ mod tests { Attribute::Name.as_ref(), Attribute::Name.as_ref(), EntryClass::Object.into(), + EntryClass::Object.into(), )], &r_set, vec![AccessEffectivePermission { @@ -2656,7 +2761,8 @@ mod tests { search: Access::Allow(BTreeSet::new()), modify_pres: Access::Allow(btreeset![Attribute::Name]), modify_rem: Access::Allow(btreeset![Attribute::Name]), - modify_class: AccessClass::Allow(btreeset![EntryClass::Object.into()]), + modify_pres_class: AccessClass::Allow(btreeset![EntryClass::Object.into()]), + modify_rem_class: AccessClass::Allow(btreeset![EntryClass::Object.into()]), }] ) } @@ -2796,6 +2902,7 @@ mod tests { &format!("{} {}", Attribute::UserAuthTokenSession, Attribute::Name), // And the class allowed is account, we don't use it though. EntryClass::Account.into(), + EntryClass::Account.into(), ); // NOTE! Syntax doesn't matter here, we just need to assert if the attr exists @@ -3296,6 +3403,7 @@ mod tests { "name class", // And the class allowed is account EntryClass::Account.into(), + EntryClass::Account.into(), ); // Test allowed pres @@ -3424,4 +3532,185 @@ mod tests { // Finally test it! test_acp_search_reduce!(&se_anon_ro, vec![acp], r_set, ex_anon_some); } + + #[test] + fn test_access_protected_deny_create() { + sketching::test_init(); + + let ev1 = entry_init!( + (Attribute::Class, EntryClass::Account.to_value()), + (Attribute::Name, Value::new_iname("testperson1")), + (Attribute::Uuid, Value::Uuid(UUID_TEST_ACCOUNT_1)) + ); + let r1_set = vec![ev1]; + + let ev2 = entry_init!( + (Attribute::Class, EntryClass::Account.to_value()), + (Attribute::Class, EntryClass::System.to_value()), + (Attribute::Name, Value::new_iname("testperson1")), + (Attribute::Uuid, Value::Uuid(UUID_TEST_ACCOUNT_1)) + ); + + let r2_set = vec![ev2]; + + let ce_admin = CreateEvent::new_impersonate_identity( + Identity::from_impersonate_entry_readwrite(E_TEST_ACCOUNT_1.clone()), + vec![], + ); + + let acp = 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( + Attribute::Name, + PartialValue::new_iname("testperson1") + )), + // classes + EntryClass::Account.into(), + // attrs + "class name uuid", + ); + + // Test allowed to create + test_acp_create!(&ce_admin, vec![acp.clone()], &r1_set, true); + // Test reject create (not allowed attr) + test_acp_create!(&ce_admin, vec![acp.clone()], &r2_set, false); + } + + #[test] + fn test_access_protected_deny_delete() { + sketching::test_init(); + + let ev1 = entry_init!( + (Attribute::Class, EntryClass::Account.to_value()), + (Attribute::Name, Value::new_iname("testperson1")), + (Attribute::Uuid, Value::Uuid(UUID_TEST_ACCOUNT_1)) + ) + .into_sealed_committed(); + let r1_set = vec![Arc::new(ev1)]; + + let ev2 = entry_init!( + (Attribute::Class, EntryClass::Account.to_value()), + (Attribute::Class, EntryClass::System.to_value()), + (Attribute::Name, Value::new_iname("testperson1")), + (Attribute::Uuid, Value::Uuid(UUID_TEST_ACCOUNT_1)) + ) + .into_sealed_committed(); + + let r2_set = vec![Arc::new(ev2)]; + + let de = DeleteEvent::new_impersonate_entry( + E_TEST_ACCOUNT_1.clone(), + filter_all!(f_eq( + Attribute::Name, + PartialValue::new_iname("testperson1") + )), + ); + + let acp = AccessControlDelete::from_raw( + "test_delete", + Uuid::new_v4(), + // Apply to admin + UUID_TEST_GROUP_1, + // To delete testperson + filter_valid!(f_eq( + Attribute::Name, + PartialValue::new_iname("testperson1") + )), + ); + + // Test allowed to delete + test_acp_delete!(&de, vec![acp.clone()], &r1_set, true); + // Test not allowed to delete + test_acp_delete!(&de, vec![acp.clone()], &r2_set, false); + } + + #[test] + fn test_access_protected_deny_modify() { + sketching::test_init(); + + let ev1 = entry_init!( + (Attribute::Class, EntryClass::Account.to_value()), + (Attribute::Name, Value::new_iname("testperson1")), + (Attribute::Uuid, Value::Uuid(UUID_TEST_ACCOUNT_1)) + ) + .into_sealed_committed(); + let r1_set = vec![Arc::new(ev1)]; + + let ev2 = entry_init!( + (Attribute::Class, EntryClass::Account.to_value()), + (Attribute::Class, EntryClass::System.to_value()), + (Attribute::Name, Value::new_iname("testperson1")), + (Attribute::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 = AccessControlModify::from_raw( + "test_modify_allow", + Uuid::new_v4(), + // Apply to admin + UUID_TEST_GROUP_1, + // To modify testperson + filter_valid!(f_eq( + Attribute::Name, + PartialValue::new_iname("testperson1") + )), + // Allow pres disp name and class + "displayname class", + // Allow rem disp name and class + "displayname class", + // And the classes allowed to add/rem are as such + "system recycled", + "system recycled", + ); + + let me_pres = ModifyEvent::new_impersonate_entry( + E_TEST_ACCOUNT_1.clone(), + filter_all!(f_eq( + Attribute::Name, + PartialValue::new_iname("testperson1") + )), + modlist!([m_pres(Attribute::DisplayName, &Value::new_utf8s("value"))]), + ); + + // Test allowed pres + test_acp_modify!(&me_pres, vec![acp_allow.clone()], &r1_set, true); + + // Test not allowed pres (due to system class) + test_acp_modify!(&me_pres, vec![acp_allow.clone()], &r2_set, false); + + // Test that we can not remove class::system + let me_rem_sys = ModifyEvent::new_impersonate_entry( + E_TEST_ACCOUNT_1.clone(), + filter_all!(f_eq( + Attribute::Class, + PartialValue::new_iname("testperson1") + )), + modlist!([m_remove( + Attribute::Class, + &EntryClass::System.to_partialvalue() + )]), + ); + + test_acp_modify!(&me_rem_sys, vec![acp_allow.clone()], &r2_set, false); + + // Ensure that we can't add recycled. + let me_pres = ModifyEvent::new_impersonate_entry( + E_TEST_ACCOUNT_1.clone(), + filter_all!(f_eq( + Attribute::Name, + PartialValue::new_iname("testperson1") + )), + modlist!([m_pres(Attribute::Class, &EntryClass::Recycled.to_value())]), + ); + + test_acp_modify!(&me_pres, vec![acp_allow.clone()], &r1_set, false); + } } diff --git a/server/lib/src/server/access/modify.rs b/server/lib/src/server/access/modify.rs index 853ad4088..d0025c71c 100644 --- a/server/lib/src/server/access/modify.rs +++ b/server/lib/src/server/access/modify.rs @@ -1,21 +1,25 @@ -use crate::prelude::*; -use hashbrown::HashMap; -use std::collections::BTreeSet; - use super::profiles::{ AccessControlModify, AccessControlModifyResolved, AccessControlReceiverCondition, AccessControlTargetCondition, }; -use super::{AccessResult, AccessResultClass}; +use super::protected::{ + LOCKED_ENTRY_CLASSES, PROTECTED_MOD_ENTRY_CLASSES, PROTECTED_MOD_PRES_ENTRY_CLASSES, + PROTECTED_MOD_REM_ENTRY_CLASSES, +}; +use super::{AccessBasicResult, AccessModResult}; +use crate::prelude::*; +use hashbrown::HashMap; +use std::collections::BTreeSet; use std::sync::Arc; pub(super) enum ModifyResult<'a> { - Denied, + Deny, Grant, Allow { pres: BTreeSet<Attribute>, rem: BTreeSet<Attribute>, - cls: BTreeSet<&'a str>, + pres_cls: BTreeSet<&'a str>, + rem_cls: BTreeSet<&'a str>, }, } @@ -27,12 +31,17 @@ pub(super) fn apply_modify_access<'a>( ) -> ModifyResult<'a> { let mut denied = false; let mut grant = false; + let mut constrain_pres = BTreeSet::default(); let mut allow_pres = BTreeSet::default(); let mut constrain_rem = BTreeSet::default(); let mut allow_rem = BTreeSet::default(); - let mut constrain_cls = BTreeSet::default(); - let mut allow_cls = BTreeSet::default(); + + let mut constrain_pres_cls = BTreeSet::default(); + let mut allow_pres_cls = BTreeSet::default(); + + let mut constrain_rem_cls = BTreeSet::default(); + let mut allow_rem_cls = BTreeSet::default(); // Some useful references. // - needed for checking entry manager conditions. @@ -43,28 +52,53 @@ pub(super) fn apply_modify_access<'a>( // kind of being three operations all in one. match modify_ident_test(ident) { - AccessResult::Denied => denied = true, - AccessResult::Grant => grant = true, - AccessResult::Ignore => {} - AccessResult::Constrain(mut set) => constrain_pres.append(&mut set), - AccessResult::Allow(mut set) => allow_pres.append(&mut set), + AccessBasicResult::Deny => denied = true, + AccessBasicResult::Grant => grant = true, + AccessBasicResult::Ignore => {} + } + + // Check with protected if we should proceed. + match modify_protected_attrs(ident, entry) { + AccessModResult::Deny => denied = true, + AccessModResult::Constrain { + mut pres_attr, + mut rem_attr, + pres_cls, + rem_cls, + } => { + constrain_rem.append(&mut rem_attr); + constrain_pres.append(&mut pres_attr); + + if let Some(mut pres_cls) = pres_cls { + constrain_pres_cls.append(&mut pres_cls); + } + + if let Some(mut rem_cls) = rem_cls { + constrain_rem_cls.append(&mut rem_cls); + } + } + // Can't grant. + // AccessModResult::Grant | + // Can't allow + AccessModResult::Allow { .. } | AccessModResult::Ignore => {} } if !grant && !denied { - // Check with protected if we should proceed. - // If it's a sync entry, constrain it. match modify_sync_constrain(ident, entry, sync_agreements) { - AccessResult::Denied => denied = true, - AccessResult::Constrain(mut set) => { - constrain_rem.extend(set.iter().cloned()); - constrain_pres.append(&mut set) + AccessModResult::Deny => denied = true, + AccessModResult::Constrain { + mut pres_attr, + mut rem_attr, + .. + } => { + constrain_rem.append(&mut rem_attr); + constrain_pres.append(&mut pres_attr); } // Can't grant. - AccessResult::Grant | + // AccessModResult::Grant | // Can't allow - AccessResult::Allow(_) | - AccessResult::Ignore => {} + AccessModResult::Allow { .. } | AccessModResult::Ignore => {} } // Setup the acp's here @@ -122,35 +156,27 @@ pub(super) fn apply_modify_access<'a>( .collect(); match modify_pres_test(scoped_acp.as_slice()) { - AccessResult::Denied => denied = true, + AccessModResult::Deny => denied = true, // Can never return a unilateral grant. - AccessResult::Grant => {} - AccessResult::Ignore => {} - AccessResult::Constrain(mut set) => constrain_pres.append(&mut set), - AccessResult::Allow(mut set) => allow_pres.append(&mut set), - } - - match modify_rem_test(scoped_acp.as_slice()) { - AccessResult::Denied => denied = true, - // Can never return a unilateral grant. - AccessResult::Grant => {} - AccessResult::Ignore => {} - AccessResult::Constrain(mut set) => constrain_rem.append(&mut set), - AccessResult::Allow(mut set) => allow_rem.append(&mut set), - } - - match modify_cls_test(scoped_acp.as_slice()) { - AccessResultClass::Denied => denied = true, - // Can never return a unilateral grant. - AccessResultClass::Grant => {} - AccessResultClass::Ignore => {} - AccessResultClass::Constrain(mut set) => constrain_cls.append(&mut set), - AccessResultClass::Allow(mut set) => allow_cls.append(&mut set), + // AccessModResult::Grant => {} + AccessModResult::Ignore => {} + AccessModResult::Constrain { .. } => {} + AccessModResult::Allow { + mut pres_attr, + mut rem_attr, + mut pres_class, + mut rem_class, + } => { + allow_pres.append(&mut pres_attr); + allow_rem.append(&mut rem_attr); + allow_pres_cls.append(&mut pres_class); + allow_rem_cls.append(&mut rem_class); + } } } if denied { - ModifyResult::Denied + ModifyResult::Deny } else if grant { ModifyResult::Grant } else { @@ -168,31 +194,48 @@ pub(super) fn apply_modify_access<'a>( allow_rem }; - let allowed_cls = if !constrain_cls.is_empty() { + let mut allowed_pres_cls = if !constrain_pres_cls.is_empty() { // bit_and - &constrain_cls & &allow_cls + &constrain_pres_cls & &allow_pres_cls } else { - allow_cls + allow_pres_cls }; + let mut allowed_rem_cls = if !constrain_rem_cls.is_empty() { + // bit_and + &constrain_rem_cls & &allow_rem_cls + } else { + allow_rem_cls + }; + + // Deny these classes from being part of any addition or removal to an entry + for protected_cls in PROTECTED_MOD_PRES_ENTRY_CLASSES.iter() { + allowed_pres_cls.remove(protected_cls.as_str()); + } + + for protected_cls in PROTECTED_MOD_REM_ENTRY_CLASSES.iter() { + allowed_rem_cls.remove(protected_cls.as_str()); + } + ModifyResult::Allow { pres: allowed_pres, rem: allowed_rem, - cls: allowed_cls, + pres_cls: allowed_pres_cls, + rem_cls: allowed_rem_cls, } } } -fn modify_ident_test(ident: &Identity) -> AccessResult { +fn modify_ident_test(ident: &Identity) -> AccessBasicResult { match &ident.origin { IdentType::Internal => { trace!("Internal operation, bypassing access check"); // No need to check ACS - return AccessResult::Grant; + return AccessBasicResult::Grant; } IdentType::Synch(_) => { security_critical!("Blocking sync check"); - return AccessResult::Denied; + return AccessBasicResult::Deny; } IdentType::User(_) => {} }; @@ -201,53 +244,56 @@ fn modify_ident_test(ident: &Identity) -> AccessResult { match ident.access_scope() { AccessScope::ReadOnly | AccessScope::Synchronise => { security_access!("denied ❌ - identity access scope is not permitted to modify"); - return AccessResult::Denied; + return AccessBasicResult::Deny; } AccessScope::ReadWrite => { // As you were } }; - AccessResult::Ignore + AccessBasicResult::Ignore } -fn modify_pres_test(scoped_acp: &[&AccessControlModify]) -> AccessResult { - let allowed_pres: BTreeSet<Attribute> = scoped_acp +fn modify_pres_test<'a>(scoped_acp: &[&'a AccessControlModify]) -> AccessModResult<'a> { + let pres_attr: BTreeSet<Attribute> = scoped_acp .iter() .flat_map(|acp| acp.presattrs.iter().cloned()) .collect(); - AccessResult::Allow(allowed_pres) -} -fn modify_rem_test(scoped_acp: &[&AccessControlModify]) -> AccessResult { - let allowed_rem: BTreeSet<Attribute> = scoped_acp + let rem_attr: BTreeSet<Attribute> = scoped_acp .iter() .flat_map(|acp| acp.remattrs.iter().cloned()) .collect(); - AccessResult::Allow(allowed_rem) -} -// TODO: Should this be reverted to the Str borrow method? Or do we try to change -// to EntryClass? -fn modify_cls_test<'a>(scoped_acp: &[&'a AccessControlModify]) -> AccessResultClass<'a> { - let allowed_classes: BTreeSet<&'a str> = scoped_acp + let pres_class: BTreeSet<&'a str> = scoped_acp .iter() - .flat_map(|acp| acp.classes.iter().map(|s| s.as_str())) + .flat_map(|acp| acp.pres_classes.iter().map(|s| s.as_str())) .collect(); - AccessResultClass::Allow(allowed_classes) + + let rem_class: BTreeSet<&'a str> = scoped_acp + .iter() + .flat_map(|acp| acp.rem_classes.iter().map(|s| s.as_str())) + .collect(); + + AccessModResult::Allow { + pres_attr, + rem_attr, + pres_class, + rem_class, + } } -fn modify_sync_constrain( +fn modify_sync_constrain<'a>( ident: &Identity, entry: &Arc<EntrySealedCommitted>, sync_agreements: &HashMap<Uuid, BTreeSet<Attribute>>, -) -> AccessResult { +) -> AccessModResult<'a> { match &ident.origin { - IdentType::Internal => AccessResult::Ignore, + IdentType::Internal => AccessModResult::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 + AccessModResult::Ignore } IdentType::User(_) => { // We need to meet these conditions. @@ -259,7 +305,7 @@ fn modify_sync_constrain( .unwrap_or(false); if !is_sync { - return AccessResult::Ignore; + return AccessModResult::Ignore; } if let Some(sync_uuid) = entry.get_ava_single_refer(Attribute::SyncParentUuid) { @@ -274,11 +320,115 @@ fn modify_sync_constrain( set.extend(sync_yield_authority.iter().cloned()) } - AccessResult::Constrain(set) + AccessModResult::Constrain { + pres_attr: set.clone(), + rem_attr: set, + pres_cls: None, + rem_cls: None, + } } else { warn!(entry = ?entry.get_uuid(), "sync_parent_uuid not found on sync object, preventing all access"); - AccessResult::Denied + AccessModResult::Deny } } } } + +/// Verify if the modification runs into limits that are defined by our protection rules. +fn modify_protected_attrs<'a>( + ident: &Identity, + entry: &Arc<EntrySealedCommitted>, +) -> AccessModResult<'a> { + match &ident.origin { + IdentType::Internal | IdentType::Synch(_) => { + // We don't constraint or influence these. + AccessModResult::Ignore + } + IdentType::User(_) => { + if let Some(classes) = entry.get_ava_as_iutf8(Attribute::Class) { + if classes.is_disjoint(&PROTECTED_MOD_ENTRY_CLASSES) { + // Not protected, go ahead + AccessModResult::Ignore + } else { + // Okay, the entry is protected, apply the full ruleset. + modify_protected_entry_attrs(classes) + } + } else { + // Nothing to check - this entry will fail to modify anyway because it has + // no classes + AccessModResult::Ignore + } + } + } +} + +fn modify_protected_entry_attrs<'a>(classes: &BTreeSet<String>) -> AccessModResult<'a> { + // This is where the majority of the logic is - this contains the modification + // rules as they apply. + + // First check for the hard-deny rules. + if !classes.is_disjoint(&LOCKED_ENTRY_CLASSES) { + // Hard deny attribute modifications to these types. + return AccessModResult::Deny; + } + + let mut constrain_attrs = BTreeSet::default(); + + // Allows removal of the recycled class specifically on recycled entries. + if classes.contains(EntryClass::Recycled.into()) { + constrain_attrs.extend([Attribute::Class]); + } + + if classes.contains(EntryClass::ClassType.into()) { + constrain_attrs.extend([Attribute::May, Attribute::Must]); + } + + if classes.contains(EntryClass::SystemConfig.into()) { + constrain_attrs.extend([Attribute::BadlistPassword]); + } + + // Allow domain settings. + if classes.contains(EntryClass::DomainInfo.into()) { + constrain_attrs.extend([ + Attribute::DomainSsid, + Attribute::DomainLdapBasedn, + Attribute::LdapMaxQueryableAttrs, + Attribute::LdapAllowUnixPwBind, + Attribute::FernetPrivateKeyStr, + Attribute::Es256PrivateKeyDer, + Attribute::KeyActionRevoke, + Attribute::KeyActionRotate, + Attribute::IdVerificationEcKey, + Attribute::DeniedName, + Attribute::DomainDisplayName, + Attribute::Image, + ]); + } + + // Allow account policy related attributes to be changed on dyngroup + if classes.contains(EntryClass::DynGroup.into()) { + constrain_attrs.extend([ + Attribute::AuthSessionExpiry, + Attribute::AuthPasswordMinimumLength, + Attribute::CredentialTypeMinimum, + Attribute::PrivilegeExpiry, + Attribute::WebauthnAttestationCaList, + Attribute::LimitSearchMaxResults, + Attribute::LimitSearchMaxFilterTest, + Attribute::AllowPrimaryCredFallback, + ]); + } + + // If we don't constrain the attributes at all, we have to deny the change + // from proceeding. + if constrain_attrs.is_empty() { + AccessModResult::Deny + } else { + AccessModResult::Constrain { + pres_attr: constrain_attrs.clone(), + rem_attr: constrain_attrs, + pres_cls: None, + rem_cls: None, + } + } +} diff --git a/server/lib/src/server/access/profiles.rs b/server/lib/src/server/access/profiles.rs index b3ab09d1f..63fea6b4b 100644 --- a/server/lib/src/server/access/profiles.rs +++ b/server/lib/src/server/access/profiles.rs @@ -266,9 +266,10 @@ pub struct AccessControlModifyResolved<'a> { #[derive(Debug, Clone)] pub struct AccessControlModify { pub acp: AccessControlProfile, - pub classes: Vec<AttrString>, pub presattrs: Vec<Attribute>, pub remattrs: Vec<Attribute>, + pub pres_classes: Vec<AttrString>, + pub rem_classes: Vec<AttrString>, } impl AccessControlModify { @@ -293,14 +294,25 @@ impl AccessControlModify { .map(|i| i.map(Attribute::from).collect()) .unwrap_or_default(); - let classes = value + let classes: Vec<AttrString> = value .get_ava_iter_iutf8(Attribute::AcpModifyClass) .map(|i| i.map(AttrString::from).collect()) .unwrap_or_default(); + let pres_classes = value + .get_ava_iter_iutf8(Attribute::AcpModifyPresentClass) + .map(|i| i.map(AttrString::from).collect()) + .unwrap_or_else(|| classes.clone()); + + let rem_classes = value + .get_ava_iter_iutf8(Attribute::AcpModifyRemoveClass) + .map(|i| i.map(AttrString::from).collect()) + .unwrap_or_else(|| classes); + Ok(AccessControlModify { acp: AccessControlProfile::try_from(qs, value)?, - classes, + pres_classes, + rem_classes, presattrs, remattrs, }) @@ -316,7 +328,8 @@ impl AccessControlModify { targetscope: Filter<FilterValid>, presattrs: &str, remattrs: &str, - classes: &str, + pres_classes: &str, + rem_classes: &str, ) -> Self { AccessControlModify { acp: AccessControlProfile { @@ -325,7 +338,14 @@ impl AccessControlModify { receiver: AccessControlReceiver::Group(btreeset!(receiver)), target: AccessControlTarget::Scope(targetscope), }, - classes: classes.split_whitespace().map(AttrString::from).collect(), + pres_classes: pres_classes + .split_whitespace() + .map(AttrString::from) + .collect(), + rem_classes: rem_classes + .split_whitespace() + .map(AttrString::from) + .collect(), presattrs: presattrs.split_whitespace().map(Attribute::from).collect(), remattrs: remattrs.split_whitespace().map(Attribute::from).collect(), } @@ -340,7 +360,8 @@ impl AccessControlModify { target: AccessControlTarget, presattrs: &str, remattrs: &str, - classes: &str, + pres_classes: &str, + rem_classes: &str, ) -> Self { AccessControlModify { acp: AccessControlProfile { @@ -349,7 +370,14 @@ impl AccessControlModify { receiver: AccessControlReceiver::EntryManager, target, }, - classes: classes.split_whitespace().map(AttrString::from).collect(), + pres_classes: pres_classes + .split_whitespace() + .map(AttrString::from) + .collect(), + rem_classes: rem_classes + .split_whitespace() + .map(AttrString::from) + .collect(), presattrs: presattrs.split_whitespace().map(Attribute::from).collect(), remattrs: remattrs.split_whitespace().map(Attribute::from).collect(), } diff --git a/server/lib/src/server/access/protected.rs b/server/lib/src/server/access/protected.rs new file mode 100644 index 000000000..9c048db08 --- /dev/null +++ b/server/lib/src/server/access/protected.rs @@ -0,0 +1,83 @@ +use crate::prelude::EntryClass; +use std::collections::BTreeSet; +use std::sync::LazyLock; + +/// These entry classes may not be created or deleted, and may invoke some protection rules +/// if on an entry. +pub static PROTECTED_ENTRY_CLASSES: LazyLock<BTreeSet<String>> = LazyLock::new(|| { + let classes = vec![ + EntryClass::System, + EntryClass::DomainInfo, + EntryClass::SystemInfo, + EntryClass::SystemConfig, + EntryClass::DynGroup, + EntryClass::SyncObject, + EntryClass::Tombstone, + EntryClass::Recycled, + ]; + + BTreeSet::from_iter(classes.into_iter().map(|ec| ec.into())) +}); + +/// Entries with these classes are protected from modifications - not that +/// sync object is not present here as there are separate rules for that in +/// the modification access module. +/// +/// Recycled is also not protected here as it needs to be able to be removed +/// by a recycle bin admin. +pub static PROTECTED_MOD_ENTRY_CLASSES: LazyLock<BTreeSet<String>> = LazyLock::new(|| { + let classes = vec![ + EntryClass::System, + EntryClass::DomainInfo, + EntryClass::SystemInfo, + EntryClass::SystemConfig, + EntryClass::DynGroup, + // EntryClass::SyncObject, + EntryClass::Tombstone, + EntryClass::Recycled, + ]; + + BTreeSet::from_iter(classes.into_iter().map(|ec| ec.into())) +}); + +/// These classes may NOT be added to ANY ENTRY +pub static PROTECTED_MOD_PRES_ENTRY_CLASSES: LazyLock<BTreeSet<String>> = LazyLock::new(|| { + let classes = vec![ + EntryClass::System, + EntryClass::DomainInfo, + EntryClass::SystemInfo, + EntryClass::SystemConfig, + EntryClass::DynGroup, + EntryClass::SyncObject, + EntryClass::Tombstone, + EntryClass::Recycled, + ]; + + BTreeSet::from_iter(classes.into_iter().map(|ec| ec.into())) +}); + +/// These classes may NOT be removed from ANY ENTRY +pub static PROTECTED_MOD_REM_ENTRY_CLASSES: LazyLock<BTreeSet<String>> = LazyLock::new(|| { + let classes = vec![ + EntryClass::System, + EntryClass::DomainInfo, + EntryClass::SystemInfo, + EntryClass::SystemConfig, + EntryClass::DynGroup, + EntryClass::SyncObject, + EntryClass::Tombstone, + // EntryClass::Recycled, + ]; + + BTreeSet::from_iter(classes.into_iter().map(|ec| ec.into())) +}); + +/// Entries with these classes may not be modified under any circumstance. +pub static LOCKED_ENTRY_CLASSES: LazyLock<BTreeSet<String>> = LazyLock::new(|| { + let classes = vec![ + EntryClass::Tombstone, + // EntryClass::Recycled, + ]; + + BTreeSet::from_iter(classes.into_iter().map(|ec| ec.into())) +}); diff --git a/server/lib/src/server/access/search.rs b/server/lib/src/server/access/search.rs index 96e30a86b..c539a20c0 100644 --- a/server/lib/src/server/access/search.rs +++ b/server/lib/src/server/access/search.rs @@ -4,11 +4,11 @@ use std::collections::BTreeSet; use super::profiles::{ AccessControlReceiverCondition, AccessControlSearchResolved, AccessControlTargetCondition, }; -use super::AccessResult; +use super::AccessSrchResult; use std::sync::Arc; pub(super) enum SearchResult { - Denied, + Deny, Grant, Allow(BTreeSet<Attribute>), } @@ -23,32 +23,32 @@ pub(super) fn apply_search_access( // that. let mut denied = false; let mut grant = false; - let mut constrain = BTreeSet::default(); + let constrain = BTreeSet::default(); let mut allow = BTreeSet::default(); // The access control profile match search_filter_entry(ident, related_acp, entry) { - AccessResult::Denied => denied = true, - AccessResult::Grant => grant = true, - AccessResult::Ignore => {} - AccessResult::Constrain(mut set) => constrain.append(&mut set), - AccessResult::Allow(mut set) => allow.append(&mut set), + AccessSrchResult::Deny => denied = true, + AccessSrchResult::Grant => grant = true, + AccessSrchResult::Ignore => {} + // AccessSrchResult::Constrain { mut attr } => constrain.append(&mut attr), + AccessSrchResult::Allow { mut attr } => allow.append(&mut attr), }; match search_oauth2_filter_entry(ident, entry) { - AccessResult::Denied => denied = true, - AccessResult::Grant => grant = true, - AccessResult::Ignore => {} - AccessResult::Constrain(mut set) => constrain.append(&mut set), - AccessResult::Allow(mut set) => allow.append(&mut set), + AccessSrchResult::Deny => denied = true, + AccessSrchResult::Grant => grant = true, + AccessSrchResult::Ignore => {} + // AccessSrchResult::Constrain { mut attr } => constrain.append(&mut attr), + AccessSrchResult::Allow { mut attr } => allow.append(&mut attr), }; match search_sync_account_filter_entry(ident, entry) { - AccessResult::Denied => denied = true, - AccessResult::Grant => grant = true, - AccessResult::Ignore => {} - AccessResult::Constrain(mut set) => constrain.append(&mut set), - AccessResult::Allow(mut set) => allow.append(&mut set), + AccessSrchResult::Deny => denied = true, + AccessSrchResult::Grant => grant = true, + AccessSrchResult::Ignore => {} + // AccessSrchResult::Constrain{ mut attr } => constrain.append(&mut attr), + AccessSrchResult::Allow { mut attr } => allow.append(&mut attr), }; // We'll add more modules later. @@ -56,7 +56,7 @@ pub(super) fn apply_search_access( // Now finalise the decision. if denied { - SearchResult::Denied + SearchResult::Deny } else if grant { SearchResult::Grant } else { @@ -74,17 +74,17 @@ fn search_filter_entry( ident: &Identity, related_acp: &[AccessControlSearchResolved], entry: &Arc<EntrySealedCommitted>, -) -> AccessResult { +) -> AccessSrchResult { // If this is an internal search, return our working set. match &ident.origin { IdentType::Internal => { trace!(uuid = ?entry.get_display_id(), "Internal operation, bypassing access check"); // No need to check ACS - return AccessResult::Grant; + return AccessSrchResult::Grant; } IdentType::Synch(_) => { security_debug!(uuid = ?entry.get_display_id(), "Blocking sync check"); - return AccessResult::Denied; + return AccessSrchResult::Deny; } IdentType::User(_) => {} }; @@ -95,7 +95,7 @@ fn search_filter_entry( security_debug!( "denied ❌ - identity access scope 'Synchronise' is not permitted to search" ); - return AccessResult::Denied; + return AccessSrchResult::Deny; } AccessScope::ReadOnly | AccessScope::ReadWrite => { // As you were @@ -161,16 +161,21 @@ fn search_filter_entry( .flatten() .collect(); - AccessResult::Allow(allowed_attrs) + AccessSrchResult::Allow { + attr: allowed_attrs, + } } -fn search_oauth2_filter_entry(ident: &Identity, entry: &Arc<EntrySealedCommitted>) -> AccessResult { +fn search_oauth2_filter_entry( + ident: &Identity, + entry: &Arc<EntrySealedCommitted>, +) -> AccessSrchResult { match &ident.origin { - IdentType::Internal | IdentType::Synch(_) => AccessResult::Ignore, + IdentType::Internal | IdentType::Synch(_) => AccessSrchResult::Ignore, IdentType::User(iuser) => { if iuser.entry.get_uuid() == UUID_ANONYMOUS { debug!("Anonymous can't access OAuth2 entries, ignoring"); - return AccessResult::Ignore; + return AccessSrchResult::Ignore; } let contains_o2_rs = entry @@ -190,16 +195,18 @@ fn search_oauth2_filter_entry(ident: &Identity, entry: &Arc<EntrySealedCommitted if contains_o2_rs && contains_o2_scope_member { security_debug!(entry = ?entry.get_uuid(), ident = ?iuser.entry.get_uuid2rdn(), "ident is a memberof a group granted an oauth2 scope by this entry"); - return AccessResult::Allow(btreeset!( - Attribute::Class, - Attribute::DisplayName, - Attribute::Uuid, - Attribute::Name, - Attribute::OAuth2RsOriginLanding, - Attribute::Image - )); + return AccessSrchResult::Allow { + attr: btreeset!( + Attribute::Class, + Attribute::DisplayName, + Attribute::Uuid, + Attribute::Name, + Attribute::OAuth2RsOriginLanding, + Attribute::Image + ), + }; } - AccessResult::Ignore + AccessSrchResult::Ignore } } } @@ -207,9 +214,9 @@ fn search_oauth2_filter_entry(ident: &Identity, entry: &Arc<EntrySealedCommitted fn search_sync_account_filter_entry( ident: &Identity, entry: &Arc<EntrySealedCommitted>, -) -> AccessResult { +) -> AccessSrchResult { match &ident.origin { - IdentType::Internal | IdentType::Synch(_) => AccessResult::Ignore, + IdentType::Internal | IdentType::Synch(_) => AccessSrchResult::Ignore, IdentType::User(iuser) => { // Is the user a synced object? let is_user_sync_account = iuser @@ -244,16 +251,18 @@ fn search_sync_account_filter_entry( // We finally got here! security_debug!(entry = ?entry.get_uuid(), ident = ?iuser.entry.get_uuid2rdn(), "ident is a synchronised account from this sync account"); - return AccessResult::Allow(btreeset!( - Attribute::Class, - Attribute::Uuid, - Attribute::SyncCredentialPortal - )); + return AccessSrchResult::Allow { + attr: btreeset!( + Attribute::Class, + Attribute::Uuid, + Attribute::SyncCredentialPortal + ), + }; } } } // Fall through - AccessResult::Ignore + AccessSrchResult::Ignore } } }