From b3be758b74cb03276e1f55d4db66deacc3812435 Mon Sep 17 00:00:00 2001 From: Firstyear Date: Mon, 20 Jan 2025 21:28:22 +1000 Subject: [PATCH] 20250114 3325 SCIM access control (#3359) Add an extended query operation to return effective access controls so that UI's can dynamically display what is or is not editable on an entry. --- proto/src/scim_v1/client.rs | 16 +- proto/src/scim_v1/mod.rs | 14 +- proto/src/scim_v1/server.rs | 40 +++++ server/lib-macros/src/entry.rs | 5 +- server/lib/src/entry.rs | 23 +++ server/lib/src/event.rs | 14 ++ server/lib/src/server/access/mod.rs | 245 ++++++++++++++++++---------- server/lib/src/server/mod.rs | 47 +++++- server/lib/src/server/scim.rs | 9 + server/testkit/tests/scim_test.rs | 2 + tools/cli/src/cli/person.rs | 1 + 11 files changed, 320 insertions(+), 96 deletions(-) diff --git a/proto/src/scim_v1/client.rs b/proto/src/scim_v1/client.rs index 07108b7b1..ae5a8a330 100644 --- a/proto/src/scim_v1/client.rs +++ b/proto/src/scim_v1/client.rs @@ -1,4 +1,5 @@ //! These are types that a client will send to the server. +use super::ScimEntryGetQuery; use super::ScimOauth2ClaimMapJoinChar; use crate::attribute::Attribute; use serde::{Deserialize, Serialize}; @@ -89,10 +90,17 @@ pub struct ScimEntryPutKanidm { #[derive(Deserialize, Serialize, Debug, Clone)] pub struct ScimStrings(#[serde_as(as = "OneOrMany<_, PreferMany>")] pub Vec); -#[derive(Debug, Clone, Deserialize)] +#[derive(Debug, Clone, Deserialize, Default)] pub struct ScimEntryPutGeneric { // id is only used to target the entry in question pub id: Uuid, + + #[serde(flatten)] + /// Non-standard extension - allow query options to be set in a put request. This + /// is because a put request also returns the entry state post put, so we want + /// to allow putters to adjust and control what is returned here. + pub query: ScimEntryGetQuery, + // external_id can't be set by put // meta is skipped on put // Schemas are decoded as part of "attrs". @@ -119,6 +127,10 @@ impl TryFrom for ScimEntryPutGeneric { }) .collect::>()?; - Ok(ScimEntryPutGeneric { id, attrs }) + Ok(ScimEntryPutGeneric { + id, + attrs, + query: Default::default(), + }) } } diff --git a/proto/src/scim_v1/mod.rs b/proto/src/scim_v1/mod.rs index 76b6c807d..db9c7a8aa 100644 --- a/proto/src/scim_v1/mod.rs +++ b/proto/src/scim_v1/mod.rs @@ -20,6 +20,7 @@ use crate::attribute::Attribute; use serde::{Deserialize, Serialize}; use sshkey_attest::proto::PublicKey as SshPublicKey; use std::collections::BTreeMap; +use std::ops::Not; use utoipa::ToSchema; use serde_with::formats::CommaSeparator; @@ -47,10 +48,12 @@ pub struct ScimEntryGeneric { /// SCIM Query Parameters used during the get of a single entry #[serde_as] #[skip_serializing_none] -#[derive(Serialize, Deserialize, Debug, Default)] +#[derive(Serialize, Deserialize, Clone, Debug, Default)] pub struct ScimEntryGetQuery { #[serde_as(as = "Option>")] pub attributes: Option>, + #[serde(default, skip_serializing_if = "<&bool>::not")] + pub ext_access_check: bool, } #[derive(Serialize, Deserialize, Debug, Clone, ToSchema)] @@ -178,7 +181,10 @@ mod tests { fn scim_entry_get_query() { use super::*; - let q = ScimEntryGetQuery { attributes: None }; + let q = ScimEntryGetQuery { + attributes: None, + ..Default::default() + }; let txt = serde_urlencoded::to_string(&q).unwrap(); @@ -186,6 +192,7 @@ mod tests { let q = ScimEntryGetQuery { attributes: Some(vec![Attribute::Name]), + ext_access_check: false, }; let txt = serde_urlencoded::to_string(&q).unwrap(); @@ -193,9 +200,10 @@ mod tests { let q = ScimEntryGetQuery { attributes: Some(vec![Attribute::Name, Attribute::Spn]), + ext_access_check: true, }; let txt = serde_urlencoded::to_string(&q).unwrap(); - assert_eq!(txt, "attributes=name%2Cspn"); + assert_eq!(txt, "attributes=name%2Cspn&ext_access_check=true"); } } diff --git a/proto/src/scim_v1/server.rs b/proto/src/scim_v1/server.rs index 4c61cdff7..555071f16 100644 --- a/proto/src/scim_v1/server.rs +++ b/proto/src/scim_v1/server.rs @@ -16,14 +16,54 @@ use uuid::Uuid; /// A strongly typed ScimEntry that is for transmission to clients. This uses /// Kanidm internal strong types for values allowing direct serialisation and /// transmission. +#[serde_as] +#[skip_serializing_none] #[derive(Serialize, Debug, Clone, ToSchema)] pub struct ScimEntryKanidm { #[serde(flatten)] pub header: ScimEntryHeader, + + pub ext_access_check: Option, #[serde(flatten)] pub attrs: BTreeMap, } +#[derive(Serialize, Debug, Clone, ToSchema)] +pub enum ScimAttributeEffectiveAccess { + /// All attributes on the entry have this permission granted + Grant, + /// All attributes on the entry have this permission denied + Denied, + /// The following attributes on the entry have this permission granted + Allow(BTreeSet), +} + +impl ScimAttributeEffectiveAccess { + /// Check if the effective access allows or denies this attribute + pub fn check(&self, attr: &Attribute) -> bool { + match self { + Self::Grant => true, + Self::Denied => false, + Self::Allow(set) => set.contains(attr), + } + } +} + +#[derive(Serialize, Debug, Clone, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct ScimEffectiveAccess { + /// The identity that inherits the effective permission + pub ident: Uuid, + /// If the ident may delete the target entry + pub delete: bool, + /// The set of effective access over search events + pub search: ScimAttributeEffectiveAccess, + /// The set of effective access over modify present events + pub modify_present: ScimAttributeEffectiveAccess, + /// The set of effective access over modify remove events + pub modify_remove: ScimAttributeEffectiveAccess, +} + #[derive(Serialize, Debug, Clone, ToSchema)] #[serde(rename_all = "camelCase")] pub struct ScimAddress { diff --git a/server/lib-macros/src/entry.rs b/server/lib-macros/src/entry.rs index 6301eb182..aad516042 100644 --- a/server/lib-macros/src/entry.rs +++ b/server/lib-macros/src/entry.rs @@ -20,10 +20,7 @@ fn parse_attributes( input: &syn::ItemFn, ) -> Result<(proc_macro2::TokenStream, Flags), syn::Error> { let args: Punctuated = - match Punctuated::::parse_terminated.parse(args.clone()) { - Ok(it) => it, - Err(e) => return Err(e), - }; + Punctuated::::parse_terminated.parse(args.clone())?; let args_are_allowed = args.pairs().all(|p| { ALLOWED_ATTRIBUTES.to_vec().contains( diff --git a/server/lib/src/entry.rs b/server/lib/src/entry.rs index a9004da15..5ad112a07 100644 --- a/server/lib/src/entry.rs +++ b/server/lib/src/entry.rs @@ -41,12 +41,14 @@ use crate::prelude::*; use crate::repl::cid::Cid; use crate::repl::entry::EntryChangeState; use crate::repl::proto::{ReplEntryV1, ReplIncrementalEntryV1}; +use crate::server::access::AccessEffectivePermission; use compact_jwt::JwsEs256Signer; use hashbrown::{HashMap, HashSet}; use kanidm_proto::internal::ImageValue; use kanidm_proto::internal::{ ConsistencyError, Filter as ProtoFilter, OperationError, SchemaError, UiHint, }; +use kanidm_proto::scim_v1::server::ScimEffectiveAccess; use kanidm_proto::v1::Entry as ProtoEntry; use ldap3_proto::simple::{LdapPartialAttribute, LdapSearchResultEntry}; use openssl::ec::EcKey; @@ -160,6 +162,7 @@ pub struct EntrySealed { #[derive(Clone, Debug)] pub struct EntryReduced { uuid: Uuid, + effective_access: Option>, } // One day this is going to be Map - @yaleman @@ -1782,6 +1785,7 @@ impl Entry { Entry { valid: EntryReduced { uuid: self.valid.uuid, + effective_access: None, }, state: self.state, attrs: self.attrs, @@ -1793,6 +1797,7 @@ impl Entry { pub fn reduce_attributes( &self, allowed_attrs: &BTreeSet, + effective_access: Option>, ) -> Entry { // Remove all attrs from our tree that are NOT in the allowed set. let f_attrs: Map<_, _> = self @@ -1809,6 +1814,7 @@ impl Entry { let valid = EntryReduced { uuid: self.valid.uuid, + effective_access, }; let state = self.state.clone(); @@ -2293,6 +2299,22 @@ impl Entry { let attrs = result?; + let ext_access_check = self.valid.effective_access.as_ref().map(|eff_acc| { + let ident = eff_acc.ident; + let delete = eff_acc.delete; + let search = (&eff_acc.search).into(); + let modify_present = (&eff_acc.modify_pres).into(); + let modify_remove = (&eff_acc.modify_rem).into(); + + ScimEffectiveAccess { + ident, + delete, + search, + modify_present, + modify_remove, + } + }); + let id = self.get_uuid(); // Not sure how I want to handle this yet, I think we need some schema changes @@ -2309,6 +2331,7 @@ impl Entry { // entry to store some extra metadata. meta: None, }, + ext_access_check, attrs, }) } diff --git a/server/lib/src/event.rs b/server/lib/src/event.rs index b80f95336..6b98f97f6 100644 --- a/server/lib/src/event.rs +++ b/server/lib/src/event.rs @@ -77,6 +77,7 @@ pub struct SearchEvent { // This is the original filter, for the purpose of ACI checking. pub filter_orig: Filter, pub attrs: Option>, + pub effective_access_check: bool, } impl SearchEvent { @@ -99,6 +100,7 @@ impl SearchEvent { // We can't get this from the SearchMessage because it's annoying with the // current macro design. attrs: None, + effective_access_check: false, }) } @@ -132,6 +134,7 @@ impl SearchEvent { filter, filter_orig, attrs: r_attrs, + effective_access_check: false, }) } @@ -168,6 +171,7 @@ impl SearchEvent { filter, filter_orig, attrs: r_attrs, + effective_access_check: false, }) } @@ -185,6 +189,7 @@ impl SearchEvent { filter, filter_orig, attrs: None, + effective_access_check: false, }) } @@ -202,6 +207,7 @@ impl SearchEvent { filter, filter_orig, attrs: None, + effective_access_check: false, }) } @@ -217,6 +223,7 @@ impl SearchEvent { filter: filter.clone().into_valid(), filter_orig: filter.into_valid(), attrs: None, + effective_access_check: false, } } @@ -229,6 +236,7 @@ impl SearchEvent { filter: filter.clone().into_valid(), filter_orig: filter.into_valid(), attrs: None, + effective_access_check: false, } } @@ -242,6 +250,7 @@ impl SearchEvent { filter, filter_orig, attrs: None, + effective_access_check: false, } } @@ -260,6 +269,7 @@ impl SearchEvent { filter, filter_orig, attrs: None, + effective_access_check: false, } } @@ -276,6 +286,7 @@ impl SearchEvent { filter: filter.clone().into_valid().into_ignore_hidden(), filter_orig: filter.into_valid(), attrs: None, + effective_access_check: false, } } @@ -296,6 +307,7 @@ impl SearchEvent { filter, filter_orig, attrs, + effective_access_check: false, }) } @@ -308,6 +320,7 @@ impl SearchEvent { filter: filter.clone().into_valid(), filter_orig: filter.into_valid(), attrs: None, + effective_access_check: false, } } @@ -317,6 +330,7 @@ impl SearchEvent { filter: filter.clone(), filter_orig: filter, attrs: None, + effective_access_check: false, } } } diff --git a/server/lib/src/server/access/mod.rs b/server/lib/src/server/access/mod.rs index 358992127..4847bfa37 100644 --- a/server/lib/src/server/access/mod.rs +++ b/server/lib/src/server/access/mod.rs @@ -23,7 +23,7 @@ use concread::arcache::ARCacheBuilder; use concread::cowcell::*; use uuid::Uuid; -use crate::entry::{Entry, EntryCommitted, EntryInit, EntryNew, EntryReduced}; +use crate::entry::{Entry, EntryInit, EntryNew}; use crate::event::{CreateEvent, DeleteEvent, ModifyEvent, SearchEvent}; use crate::filter::{Filter, FilterValid, ResolveFilterCache, ResolveFilterCacheReadTxn}; use crate::modify::Modify; @@ -36,6 +36,8 @@ use self::profiles::{ AccessControlSearchResolved, AccessControlTarget, AccessControlTargetCondition, }; +use kanidm_proto::scim_v1::server::ScimAttributeEffectiveAccess; + use self::create::{apply_create_access, CreateResult}; use self::delete::{apply_delete_access, DeleteResult}; use self::modify::{apply_modify_access, ModifyResult}; @@ -57,6 +59,16 @@ pub enum Access { Allow(BTreeSet), } +impl From<&Access> for ScimAttributeEffectiveAccess { + fn from(value: &Access) -> Self { + match value { + Access::Grant => Self::Grant, + Access::Denied => Self::Denied, + Access::Allow(set) => Self::Allow(set.clone()), + } + } +} + #[derive(Debug, Clone, PartialEq, Eq)] pub enum AccessClass { Grant, @@ -66,8 +78,9 @@ pub enum AccessClass { #[derive(Debug, Clone, PartialEq, Eq)] pub struct AccessEffectivePermission { - // I don't think we need this? The ident is implied by the requester. - // ident: Uuid, + /// Who the access applies to + pub ident: Uuid, + /// The target the access affects pub target: Uuid, pub delete: bool, pub search: Access, @@ -79,12 +92,13 @@ pub struct AccessEffectivePermission { pub enum AccessResult { // Deny this operation unconditionally. Denied, - // Unbounded allow, provided no denied exists. + // 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. + // allow anything, it constrains what might be allowed + // by a later module. Constrain(BTreeSet), // Allow these attributes within constraints. Allow(BTreeSet), @@ -181,7 +195,11 @@ pub trait AccessControlsTransaction<'a> { fn get_acp_resolve_filter_cache(&self) -> &mut ResolveFilterCacheReadTxn<'a>; #[instrument(level = "trace", name = "access::search_related_acp", skip_all)] - fn search_related_acp<'b>(&'b self, ident: &Identity) -> Vec> { + fn search_related_acp<'b>( + &'b self, + ident: &Identity, + attrs: Option<&BTreeSet>, + ) -> Vec> { let search_state = self.get_search(); let acp_resolve_filter_cache = self.get_acp_resolve_filter_cache(); @@ -249,8 +267,18 @@ pub trait AccessControlsTransaction<'a> { }) .collect(); + // Trim any search rule that doesn't provide attributes related to the request. + let related_acp = if let Some(r_attrs) = attrs.as_ref() { + related_acp + .into_iter() + .filter(|acs| !acs.acp.attrs.is_disjoint(r_attrs)) + .collect() + } else { + // None here means all attrs requested. + related_acp + }; + related_acp - // } } #[instrument(level = "debug", name = "access::filter_entries", skip_all)] @@ -267,7 +295,7 @@ pub trait AccessControlsTransaction<'a> { let requested_attrs: BTreeSet = filter_orig.get_attr_set(); // First get the set of acps that apply to this receiver - let related_acp = self.search_related_acp(ident); + let related_acp = self.search_related_acp(ident, None); // For each entry. let entries_is_empty = entries.is_empty(); @@ -318,33 +346,61 @@ pub trait AccessControlsTransaction<'a> { name = "access::search_filter_entry_attributes", skip_all )] - fn search_filter_entry_attributes( - &self, + fn search_filter_entry_attributes<'b>( + &'b self, se: &SearchEvent, entries: Vec>, - ) -> Result>, OperationError> { + ) -> Result, OperationError> { + struct DoEffectiveCheck<'b> { + modify_related_acp: Vec>, + delete_related_acp: Vec>, + sync_agmts: &'b HashMap>, + } + + let ident_uuid = match &se.ident.origin { + IdentType::Internal => { + // In production we can't risk leaking data here, so we return + // empty sets. + security_critical!("IMPOSSIBLE STATE: Internal search in external interface?! Returning empty for safety."); + // No need to check ACS + return Err(OperationError::InvalidState); + } + IdentType::Synch(_) => { + security_critical!("Blocking sync check"); + return Err(OperationError::InvalidState); + } + IdentType::User(u) => u.entry.get_uuid(), + }; + // Build a reference set from the req_attrs. This is what we test against // to see if the attribute is something we currently want. + let do_effective_check = se.effective_access_check.then(|| { + debug!("effective permission check requested during reduction phase"); + + // == modify == + let modify_related_acp = self.modify_related_acp(&se.ident); + // == delete == + let delete_related_acp = self.delete_related_acp(&se.ident); + + let sync_agmts = self.get_sync_agreements(); + + DoEffectiveCheck { + modify_related_acp, + delete_related_acp, + sync_agmts, + } + }); + // Get the relevant acps for this receiver. - let related_acp = self.search_related_acp(&se.ident); - let related_acp: Vec<_> = if let Some(r_attrs) = se.attrs.as_ref() { - // If the acp doesn't overlap with our requested attrs, there is no point in - // testing it! - related_acp - .into_iter() - .filter(|acs| !acs.acp.attrs.is_disjoint(r_attrs)) - .collect() - } else { - related_acp - }; + let search_related_acp = self.search_related_acp(&se.ident, se.attrs.as_ref()); // For each entry. let entries_is_empty = entries.is_empty(); let allowed_entries: Vec<_> = entries .into_iter() - .filter_map(|e| { - match apply_search_access(&se.ident, related_acp.as_slice(), &e) { + .filter_map(|entry| { + match apply_search_access(&se.ident, &search_related_acp, &entry) { SearchResult::Denied => { None } @@ -369,11 +425,20 @@ pub trait AccessControlsTransaction<'a> { allowed_attrs }; - if reduced_attrs.is_empty() { - None - } else { - Some(e.reduce_attributes(&reduced_attrs)) - } + let effective_permissions = do_effective_check.as_ref().map(|do_check| { + self.entry_effective_permission_check( + &se.ident, + ident_uuid, + &entry, + &search_related_acp, + &do_check.modify_related_acp, + &do_check.delete_related_acp, + do_check.sync_agmts, + ) + }) + .map(Box::new); + + Some(entry.reduce_attributes(&reduced_attrs, effective_permissions)) } } @@ -798,7 +863,7 @@ pub trait AccessControlsTransaction<'a> { // have an entry template. I think james was right about the create being // a template copy op ... - match &ident.origin { + let ident_uuid = match &ident.origin { IdentType::Internal => { // In production we can't risk leaking data here, so we return // empty sets. @@ -810,7 +875,7 @@ pub trait AccessControlsTransaction<'a> { security_critical!("Blocking sync check"); return Err(OperationError::InvalidState); } - IdentType::User(_) => {} + IdentType::User(u) => u.entry.get_uuid(), }; trace!(ident = %ident, "Effective permission check"); @@ -818,71 +883,26 @@ pub trait AccessControlsTransaction<'a> { // == search == // Get the relevant acps for this receiver. - let search_related_acp = self.search_related_acp(ident); - // Trim any search rule that doesn't provide attributes related to the request. - let search_related_acp = if let Some(r_attrs) = attrs.as_ref() { - search_related_acp - .into_iter() - .filter(|acs| !acs.acp.attrs.is_disjoint(r_attrs)) - .collect() - } else { - // None here means all attrs requested. - search_related_acp - }; - + let search_related_acp = self.search_related_acp(ident, attrs.as_ref()); // == modify == - let modify_related_acp = self.modify_related_acp(ident); + // == delete == let delete_related_acp = self.delete_related_acp(ident); let sync_agmts = self.get_sync_agreements(); let effective_permissions: Vec<_> = entries .iter() - .map(|e| { - // == search == - let search_effective = - match apply_search_access(ident, search_related_acp.as_slice(), e) { - SearchResult::Denied => Access::Denied, - SearchResult::Grant => Access::Grant, - SearchResult::Allow(allowed_attrs) => { - // Bound by requested attrs? - Access::Allow(allowed_attrs.into_iter().collect()) - } - }; - - // == modify == - let (modify_pres, modify_rem, modify_class) = match apply_modify_access( + .map(|entry| { + self.entry_effective_permission_check( ident, - modify_related_acp.as_slice(), + ident_uuid, + entry, + &search_related_acp, + &modify_related_acp, + &delete_related_acp, sync_agmts, - e, - ) { - ModifyResult::Denied => (Access::Denied, Access::Denied, AccessClass::Denied), - ModifyResult::Grant => (Access::Grant, Access::Grant, AccessClass::Grant), - ModifyResult::Allow { pres, rem, cls } => ( - Access::Allow(pres.into_iter().collect()), - Access::Allow(rem.into_iter().collect()), - AccessClass::Allow(cls.into_iter().map(|s| s.into()).collect()), - ), - }; - - // == delete == - let delete_status = apply_delete_access(ident, delete_related_acp.as_slice(), e); - - let delete = match delete_status { - DeleteResult::Denied => false, - DeleteResult::Grant => true, - }; - - AccessEffectivePermission { - target: e.get_uuid(), - delete, - search: search_effective, - modify_pres, - modify_rem, - modify_class, - } + ) }) .collect(); @@ -892,6 +912,57 @@ pub trait AccessControlsTransaction<'a> { Ok(effective_permissions) } + + fn entry_effective_permission_check<'b>( + &'b self, + ident: &Identity, + ident_uuid: Uuid, + entry: &Arc, + search_related_acp: &[AccessControlSearchResolved<'b>], + modify_related_acp: &[AccessControlModifyResolved<'b>], + delete_related_acp: &[AccessControlDeleteResolved<'b>], + sync_agmts: &HashMap>, + ) -> AccessEffectivePermission { + // == search == + let search_effective = match apply_search_access(ident, search_related_acp, entry) { + SearchResult::Denied => Access::Denied, + SearchResult::Grant => Access::Grant, + SearchResult::Allow(allowed_attrs) => { + // Bound by requested attrs? + Access::Allow(allowed_attrs.into_iter().collect()) + } + }; + + // == modify == + let (modify_pres, modify_rem, modify_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 } => ( + Access::Allow(pres.into_iter().collect()), + Access::Allow(rem.into_iter().collect()), + AccessClass::Allow(cls.into_iter().map(|s| s.into()).collect()), + ), + }; + + // == delete == + let delete_status = apply_delete_access(ident, delete_related_acp, entry); + + let delete = match delete_status { + DeleteResult::Denied => false, + DeleteResult::Grant => true, + }; + + AccessEffectivePermission { + ident: ident_uuid, + target: entry.get_uuid(), + delete, + search: search_effective, + modify_pres, + modify_rem, + modify_class, + } + } } pub struct AccessControlsWriteTransaction<'a> { @@ -2535,6 +2606,7 @@ mod tests { vec![], &r_set, vec![AccessEffectivePermission { + ident: UUID_TEST_ACCOUNT_1, delete: false, target: uuid!("cc8e95b4-c24f-4d68-ba54-8bed76f63930"), search: Access::Allow(btreeset![Attribute::Name]), @@ -2576,6 +2648,7 @@ mod tests { )], &r_set, vec![AccessEffectivePermission { + ident: UUID_TEST_ACCOUNT_1, delete: false, target: uuid!("cc8e95b4-c24f-4d68-ba54-8bed76f63930"), search: Access::Allow(BTreeSet::new()), diff --git a/server/lib/src/server/mod.rs b/server/lib/src/server/mod.rs index 5230138e0..f9b2e2012 100644 --- a/server/lib/src/server/mod.rs +++ b/server/lib/src/server/mod.rs @@ -284,7 +284,7 @@ pub trait QueryServerTransaction<'a> { fn search_ext( &mut self, se: &SearchEvent, - ) -> Result>, OperationError> { + ) -> Result, OperationError> { /* * This just wraps search, but it's for the external interface * so as a result it also reduces the entry set's attributes at @@ -1546,6 +1546,7 @@ impl QueryServerReadTransaction<'_> { filter: f_valid, filter_orig: f_intent_valid, attrs: r_attrs, + effective_access_check: query.ext_access_check, }; let mut vs = self.search_ext(&se)?; @@ -2639,6 +2640,7 @@ impl<'a> QueryServerWriteTransaction<'a> { mod tests { use crate::prelude::*; use kanidm_proto::scim_v1::server::ScimReference; + use kanidm_proto::scim_v1::ScimEntryGetQuery; #[qs_test] async fn test_name_to_uuid(server: &QueryServer) { @@ -3046,4 +3048,47 @@ mod tests { } } } + + #[qs_test] + async fn test_scim_effective_access_query(server: &QueryServer) { + let mut server_txn = server.write(duration_from_epoch_now()).await.unwrap(); + + let group_uuid = Uuid::new_v4(); + let e1 = entry_init!( + (Attribute::Class, EntryClass::Object.to_value()), + (Attribute::Class, EntryClass::Group.to_value()), + (Attribute::Name, Value::new_iname("testgroup")), + (Attribute::Uuid, Value::Uuid(group_uuid)) + ); + + assert!(server_txn.internal_create(vec![e1]).is_ok()); + assert!(server_txn.commit().is_ok()); + + // Now read that entry. + + let mut server_txn = server.read().await.unwrap(); + + let idm_admin_entry = server_txn.internal_search_uuid(UUID_IDM_ADMIN).unwrap(); + let idm_admin_ident = Identity::from_impersonate_entry_readwrite(idm_admin_entry); + + let query = ScimEntryGetQuery { + ext_access_check: true, + ..Default::default() + }; + + let scim_entry = server_txn + .scim_entry_id_get_ext(group_uuid, EntryClass::Group, query, idm_admin_ident) + .unwrap(); + + let ext_access_check = scim_entry.ext_access_check.unwrap(); + + trace!(?ext_access_check); + + assert!(ext_access_check.delete); + assert!(ext_access_check.search.check(&Attribute::DirectMemberOf)); + assert!(ext_access_check.search.check(&Attribute::MemberOf)); + assert!(ext_access_check.search.check(&Attribute::Name)); + assert!(ext_access_check.modify_present.check(&Attribute::Name)); + assert!(ext_access_check.modify_remove.check(&Attribute::Name)); + } } diff --git a/server/lib/src/server/scim.rs b/server/lib/src/server/scim.rs index a6a190df4..be529e8be 100644 --- a/server/lib/src/server/scim.rs +++ b/server/lib/src/server/scim.rs @@ -14,6 +14,10 @@ pub struct ScimEntryPutEvent { /// Update an attribute to contain the following value state. /// If the attribute is None, it is removed. pub attrs: BTreeMap>, + + /// If an effective access check should be carried out post modification + /// of the entries + pub effective_access_check: bool, } impl ScimEntryPutEvent { @@ -33,10 +37,13 @@ impl ScimEntryPutEvent { }) .collect::>()?; + let query = entry.query; + Ok(ScimEntryPutEvent { ident, target, attrs, + effective_access_check: query.ext_access_check, }) } } @@ -54,6 +61,7 @@ impl QueryServerWriteTransaction<'_> { ident, target, attrs, + effective_access_check, } = scim_entry_put; // This function transforms the put event into a modify event. @@ -91,6 +99,7 @@ impl QueryServerWriteTransaction<'_> { filter_orig: f_intent_valid, // Return all attributes, even ones we didn't affect attrs: None, + effective_access_check, }; let mut vs = self.search_ext(&se)?; diff --git a/server/testkit/tests/scim_test.rs b/server/testkit/tests/scim_test.rs index 9ee7c2d98..b760406eb 100644 --- a/server/testkit/tests/scim_test.rs +++ b/server/testkit/tests/scim_test.rs @@ -202,6 +202,7 @@ async fn test_scim_sync_entry_get(rsclient: KanidmClient) { // Limit the attributes we want. let query = ScimEntryGetQuery { attributes: Some(vec![Attribute::Name]), + ..Default::default() }; let scim_entry = rsclient @@ -238,6 +239,7 @@ async fn test_scim_sync_entry_get(rsclient: KanidmClient) { // Limit the attributes we want. let query = ScimEntryGetQuery { attributes: Some(vec![Attribute::Name]), + ..Default::default() }; let scim_entry = rsclient diff --git a/tools/cli/src/cli/person.rs b/tools/cli/src/cli/person.rs index 0626af6c5..2469fb85b 100644 --- a/tools/cli/src/cli/person.rs +++ b/tools/cli/src/cli/person.rs @@ -238,6 +238,7 @@ impl PersonOpt { aopt.aopts.account_id.as_str(), Some(ScimEntryGetQuery { attributes: Some(vec![Attribute::SshPublicKey]), + ..Default::default() }), ) .await