From 3723abb25d1a3bcb806ad81c7e7ca346ebcd0f75 Mon Sep 17 00:00:00 2001 From: Firstyear Date: Thu, 23 May 2024 15:58:49 +1000 Subject: [PATCH] Allow name write privileges to be withheld (#2773) --- book/src/access_control/intro.md | 8 ++-- server/lib/src/constants/acp.rs | 65 ++++++++++++++++++++++++++++- server/lib/src/constants/groups.rs | 23 +++++++++- server/lib/src/constants/uuids.rs | 3 +- server/lib/src/entry.rs | 4 ++ server/lib/src/server/create.rs | 5 +++ server/lib/src/server/migrations.rs | 54 ++++++++++++++++++++++-- 7 files changed, 152 insertions(+), 10 deletions(-) diff --git a/book/src/access_control/intro.md b/book/src/access_control/intro.md index 936bd64cb..7bab02e67 100644 --- a/book/src/access_control/intro.md +++ b/book/src/access_control/intro.md @@ -55,8 +55,9 @@ services. ## Default Permission Groups -Kanidm ships with default permission groups. You can use these to enable accounts to perform certain -tasks within Kanidm as required. +Kanidm ships with default permission groups. You can use these to enable (or disable) accounts to +performing certain actions or tasks within Kanidm as required by modifying the memberships of these +groups. | group name | description | | ---------------------------- | ----------------------------------------------------------------------- | @@ -68,7 +69,8 @@ tasks within Kanidm as required. | `idm_people_admins` | create and modify persons | | `idm_people_on_boarding` | create (but not modify) persons. Intended for use with service accounts | | `idm_people_pii_read` | allow read to personally identifying information | -| `idm_people_self_write_mail` | allow self-modification of the mail attribute | +| `idm_people_self_mail_write` | allow self-modification of the mail attribute | +| `idm_people_self_name_write` | allow self-modification of the name related attributes | | `idm_radius_servers` | read user radius secrets. Intended for use with service accounts | | `idm_radius_service_admins` | create and reset user radius secrets, and allow users to access radius | | `idm_recycle_bin_admins` | modify and restore entries from the recycle bin | diff --git a/server/lib/src/constants/acp.rs b/server/lib/src/constants/acp.rs index bc52bf7bc..a497b4067 100644 --- a/server/lib/src/constants/acp.rs +++ b/server/lib/src/constants/acp.rs @@ -1143,7 +1143,7 @@ lazy_static! { name: "idm_people_self_acp_write_mail", uuid: UUID_IDM_ACP_PEOPLE_SELF_WRITE_MAIL, description: "Builtin IDM Control for self write of mail for people accounts.", - receiver: BuiltinAcpReceiver::Group(vec![UUID_IDM_PEOPLE_SELF_WRITE_MAIL]), + receiver: BuiltinAcpReceiver::Group(vec![UUID_IDM_PEOPLE_SELF_MAIL_WRITE]), target: BuiltinAcpTarget::Filter(ProtoFilter::And(vec![ match_class_filter!(EntryClass::Person).clone(), match_class_filter!(EntryClass::Account).clone(), @@ -1230,6 +1230,39 @@ lazy_static! { }; } +lazy_static! { + pub static ref IDM_ACP_SELF_WRITE_DL7: BuiltinAcp = BuiltinAcp{ + name: "idm_acp_self_write", + uuid: UUID_IDM_ACP_SELF_WRITE_V1, + classes: vec![ + EntryClass::Object, + EntryClass::AccessControlProfile, + EntryClass::AccessControlModify, + ], + description: "Builtin IDM Control for self write - required for people to update their own credentials in line with best practices.", + receiver: BuiltinAcpReceiver::Group ( vec![UUID_IDM_ALL_PERSONS] ), + target: BuiltinAcpTarget::Filter(ProtoFilter::SelfUuid), + modify_removed_attrs: vec![ + Attribute::RadiusSecret, + Attribute::PrimaryCredential, + Attribute::SshPublicKey, + Attribute::UnixPassword, + Attribute::PassKeys, + Attribute::AttestedPasskeys, + Attribute::UserAuthTokenSession, + ], + modify_present_attrs: vec![ + Attribute::RadiusSecret, + Attribute::PrimaryCredential, + Attribute::SshPublicKey, + Attribute::UnixPassword, + Attribute::PassKeys, + Attribute::AttestedPasskeys, + ], + ..Default::default() + }; +} + lazy_static! { pub static ref IDM_ACP_SELF_NAME_WRITE_V1: BuiltinAcp = BuiltinAcp{ name: "idm_acp_self_name_write", @@ -1252,6 +1285,36 @@ lazy_static! { }; } +lazy_static! { + pub static ref IDM_ACP_SELF_NAME_WRITE_DL7: BuiltinAcp = BuiltinAcp{ + name: "idm_acp_self_name_write", + uuid: UUID_IDM_ACP_SELF_NAME_WRITE_V1, + classes: vec![ + EntryClass::Object, + EntryClass::AccessControlProfile, + EntryClass::AccessControlModify, + ], + description: "Builtin IDM Control for self write of name - required for people to update their own identities in line with best practices.", + receiver: BuiltinAcpReceiver::Group ( vec![UUID_IDM_PEOPLE_SELF_NAME_WRITE] ), + target: BuiltinAcpTarget::Filter(ProtoFilter::And(vec![ + ProtoFilter::SelfUuid, + match_class_filter!(EntryClass::Person).clone(), + FILTER_ANDNOT_TOMBSTONE_OR_RECYCLED.clone(), + ])), + modify_removed_attrs: vec![ + Attribute::Name, + Attribute::DisplayName, + Attribute::LegalName, + ], + modify_present_attrs: vec![ + Attribute::Name, + Attribute::DisplayName, + Attribute::LegalName, + ], + ..Default::default() + }; +} + lazy_static! { pub static ref IDM_ACP_ACCOUNT_SELF_WRITE_V1: BuiltinAcp = BuiltinAcp { name: "idm_acp_self_account_write", diff --git a/server/lib/src/constants/groups.rs b/server/lib/src/constants/groups.rs index 17e8b7207..c33274665 100644 --- a/server/lib/src/constants/groups.rs +++ b/server/lib/src/constants/groups.rs @@ -178,6 +178,18 @@ lazy_static! { ..Default::default() }; + /// Builtin IDM Group for granting people the ability to write to their own name attributes. + pub static ref BUILTIN_GROUP_PEOPLE_SELF_NAME_WRITE_DL7: BuiltinGroup = BuiltinGroup { + name: "idm_people_self_name_write", + description: "Builtin IDM Group denoting users that can write to their own name attributes.", + uuid: UUID_IDM_PEOPLE_SELF_NAME_WRITE, + entry_managed_by: Some(UUID_IDM_ADMINS), + members: vec![ + UUID_IDM_ALL_PERSONS + ], + ..Default::default() + }; + pub static ref BUILTIN_GROUP_SERVICE_ACCOUNT_ADMINS: BuiltinGroup = BuiltinGroup { name: "idm_service_account_admins", description: "Builtin Service Account Administration Group.", @@ -250,7 +262,16 @@ lazy_static! { pub static ref IDM_PEOPLE_SELF_WRITE_MAIL_V1: BuiltinGroup = BuiltinGroup { name: "idm_people_self_write_mail", description: "Builtin IDM Group for people accounts to update their own mail.", - uuid: UUID_IDM_PEOPLE_SELF_WRITE_MAIL, + uuid: UUID_IDM_PEOPLE_SELF_MAIL_WRITE, + members: Vec::new(), + ..Default::default() + }; + + /// Self-write of mail + pub static ref IDM_PEOPLE_SELF_MAIL_WRITE_DL7: BuiltinGroup = BuiltinGroup { + name: "idm_people_self_mail_write", + description: "Builtin IDM Group for people accounts to update their own mail.", + uuid: UUID_IDM_PEOPLE_SELF_MAIL_WRITE, members: Vec::new(), ..Default::default() }; diff --git a/server/lib/src/constants/uuids.rs b/server/lib/src/constants/uuids.rs index 4a9108025..6ce78360d 100644 --- a/server/lib/src/constants/uuids.rs +++ b/server/lib/src/constants/uuids.rs @@ -43,7 +43,7 @@ pub const UUID_IDM_HP_PEOPLE_EXTEND_PRIV: Uuid = uuid!("00000000-0000-0000-0000- pub const UUID_IDM_RADIUS_SECRET_READ_PRIV_V1: Uuid = uuid!("00000000-0000-0000-0000-000000000032"); pub const UUID_IDM_RADIUS_SECRET_WRITE_PRIV_V1: Uuid = uuid!("00000000-0000-0000-0000-000000000031"); -pub const UUID_IDM_PEOPLE_SELF_WRITE_MAIL: Uuid = uuid!("00000000-0000-0000-0000-000000000033"); +pub const UUID_IDM_PEOPLE_SELF_MAIL_WRITE: Uuid = uuid!("00000000-0000-0000-0000-000000000033"); pub const UUID_IDM_HP_SERVICE_ACCOUNT_INTO_PERSON_MIGRATE_PRIV: Uuid = uuid!("00000000-0000-0000-0000-000000000034"); @@ -66,6 +66,7 @@ pub const UUID_IDM_UNIX_ADMINS: Uuid = uuid!("00000000-0000-0000-0000-0000000000 pub const UUID_IDM_PEOPLE_ON_BOARDING: Uuid = uuid!("00000000-0000-0000-0000-000000000045"); pub const UUID_IDM_SERVICE_ACCOUNT_ADMINS: Uuid = uuid!("00000000-0000-0000-0000-000000000046"); pub const UUID_IDM_ACCOUNT_POLICY_ADMINS: Uuid = uuid!("00000000-0000-0000-0000-000000000047"); +pub const UUID_IDM_PEOPLE_SELF_NAME_WRITE: Uuid = uuid!("00000000-0000-0000-0000-000000000048"); // pub const UUID_IDM_HIGH_PRIVILEGE: Uuid = uuid!("00000000-0000-0000-0000-000000001000"); diff --git a/server/lib/src/entry.rs b/server/lib/src/entry.rs index 1e36ad14c..a9db8425e 100644 --- a/server/lib/src/entry.rs +++ b/server/lib/src/entry.rs @@ -635,6 +635,10 @@ impl Entry { self.add_ava_int(attr, value); } + pub fn remove_ava(&mut self, attr: Attribute) { + self.attrs.remove(attr.as_ref()); + } + /// Replace the existing content of an attribute set of this Entry, with a new set of Values. pub fn set_ava(&mut self, attr: Attribute, iter: T) where diff --git a/server/lib/src/server/create.rs b/server/lib/src/server/create.rs index 8b797188d..a6bdc9ba1 100644 --- a/server/lib/src/server/create.rs +++ b/server/lib/src/server/create.rs @@ -231,6 +231,11 @@ mod tests { Attribute::DirectMemberOf, Value::Refer(UUID_IDM_ALL_ACCOUNTS), ); + // Indirectly via all persons + e.add_ava( + Attribute::MemberOf, + Value::Refer(UUID_IDM_PEOPLE_SELF_NAME_WRITE), + ); // we also add the name_history ava! e.add_ava( Attribute::NameHistory, diff --git a/server/lib/src/server/migrations.rs b/server/lib/src/server/migrations.rs index e063df095..e530f488f 100644 --- a/server/lib/src/server/migrations.rs +++ b/server/lib/src/server/migrations.rs @@ -190,13 +190,25 @@ impl<'a> QueryServerWriteTransaction<'a> { &mut self, e: Entry, ) -> Result<(), OperationError> { - trace!("internal_migrate_or_create operating on {:?}", e.get_uuid()); + self.internal_migrate_or_create_ignore_attrs(e, &[]) + } + + /// This is the same as [internal_migrate_or_create] but it will ignore the specified + /// list of attributes, so that if an admin has modified those values then we don't + /// stomp them. + #[instrument(level = "trace", skip_all)] + pub fn internal_migrate_or_create_ignore_attrs( + &mut self, + mut e: Entry, + attrs: &[Attribute], + ) -> Result<(), OperationError> { + trace!("operating on {:?}", e.get_uuid()); let Some(filt) = e.filter_from_attrs(&[Attribute::Uuid.into()]) else { return Err(OperationError::FilterGeneration); }; - trace!("internal_migrate_or_create search {:?}", filt); + trace!("search {:?}", filt); let results = self.internal_search(filt.clone())?; @@ -204,11 +216,16 @@ impl<'a> QueryServerWriteTransaction<'a> { // It does not exist. Create it. self.internal_create(vec![e]) } else if results.len() == 1 { + // For each ignored attr, we remove it from entry. + for attr in attrs.iter() { + e.remove_ava(*attr); + } + // If the thing is subset, pass match e.gen_modlist_assert(&self.schema) { Ok(modlist) => { // Apply to &results[0] - trace!("Generated modlist -> {:?}", modlist); + trace!(?modlist); self.internal_modify(&filt, &modlist) } Err(e) => Err(OperationError::SchemaViolation(e)), @@ -622,7 +639,36 @@ impl<'a> QueryServerWriteTransaction<'a> { self.reload()?; - // Post schema changes. + // Update access controls + let idm_data = [ + BUILTIN_GROUP_PEOPLE_SELF_NAME_WRITE_DL7 + .clone() + .try_into()?, + IDM_PEOPLE_SELF_MAIL_WRITE_DL7.clone().try_into()?, + ]; + + idm_data + .into_iter() + .try_for_each(|entry| { + self.internal_migrate_or_create_ignore_attrs(entry, &[Attribute::Member]) + }) + .map_err(|err| { + error!(?err, "migrate_domain_6_to_7 -> Error"); + err + })?; + + let idm_data = [ + IDM_ACP_SELF_WRITE_DL7.clone().into(), + IDM_ACP_SELF_NAME_WRITE_DL7.clone().into(), + ]; + + idm_data + .into_iter() + .try_for_each(|entry| self.internal_migrate_or_create(entry)) + .map_err(|err| { + error!(?err, "migrate_domain_6_to_7 -> Error"); + err + })?; Ok(()) }