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.
This commit is contained in:
Firstyear 2025-01-20 21:28:22 +10:00 committed by GitHub
parent b03f842728
commit b3be758b74
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 320 additions and 96 deletions

View file

@ -1,4 +1,5 @@
//! These are types that a client will send to the server. //! These are types that a client will send to the server.
use super::ScimEntryGetQuery;
use super::ScimOauth2ClaimMapJoinChar; use super::ScimOauth2ClaimMapJoinChar;
use crate::attribute::Attribute; use crate::attribute::Attribute;
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
@ -89,10 +90,17 @@ pub struct ScimEntryPutKanidm {
#[derive(Deserialize, Serialize, Debug, Clone)] #[derive(Deserialize, Serialize, Debug, Clone)]
pub struct ScimStrings(#[serde_as(as = "OneOrMany<_, PreferMany>")] pub Vec<String>); pub struct ScimStrings(#[serde_as(as = "OneOrMany<_, PreferMany>")] pub Vec<String>);
#[derive(Debug, Clone, Deserialize)] #[derive(Debug, Clone, Deserialize, Default)]
pub struct ScimEntryPutGeneric { pub struct ScimEntryPutGeneric {
// id is only used to target the entry in question // id is only used to target the entry in question
pub id: Uuid, 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 // external_id can't be set by put
// meta is skipped on put // meta is skipped on put
// Schemas are decoded as part of "attrs". // Schemas are decoded as part of "attrs".
@ -119,6 +127,10 @@ impl TryFrom<ScimEntryPutKanidm> for ScimEntryPutGeneric {
}) })
.collect::<Result<_, _>>()?; .collect::<Result<_, _>>()?;
Ok(ScimEntryPutGeneric { id, attrs }) Ok(ScimEntryPutGeneric {
id,
attrs,
query: Default::default(),
})
} }
} }

View file

@ -20,6 +20,7 @@ use crate::attribute::Attribute;
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use sshkey_attest::proto::PublicKey as SshPublicKey; use sshkey_attest::proto::PublicKey as SshPublicKey;
use std::collections::BTreeMap; use std::collections::BTreeMap;
use std::ops::Not;
use utoipa::ToSchema; use utoipa::ToSchema;
use serde_with::formats::CommaSeparator; use serde_with::formats::CommaSeparator;
@ -47,10 +48,12 @@ pub struct ScimEntryGeneric {
/// SCIM Query Parameters used during the get of a single entry /// SCIM Query Parameters used during the get of a single entry
#[serde_as] #[serde_as]
#[skip_serializing_none] #[skip_serializing_none]
#[derive(Serialize, Deserialize, Debug, Default)] #[derive(Serialize, Deserialize, Clone, Debug, Default)]
pub struct ScimEntryGetQuery { pub struct ScimEntryGetQuery {
#[serde_as(as = "Option<StringWithSeparator::<CommaSeparator, Attribute>>")] #[serde_as(as = "Option<StringWithSeparator::<CommaSeparator, Attribute>>")]
pub attributes: Option<Vec<Attribute>>, pub attributes: Option<Vec<Attribute>>,
#[serde(default, skip_serializing_if = "<&bool>::not")]
pub ext_access_check: bool,
} }
#[derive(Serialize, Deserialize, Debug, Clone, ToSchema)] #[derive(Serialize, Deserialize, Debug, Clone, ToSchema)]
@ -178,7 +181,10 @@ mod tests {
fn scim_entry_get_query() { fn scim_entry_get_query() {
use super::*; use super::*;
let q = ScimEntryGetQuery { attributes: None }; let q = ScimEntryGetQuery {
attributes: None,
..Default::default()
};
let txt = serde_urlencoded::to_string(&q).unwrap(); let txt = serde_urlencoded::to_string(&q).unwrap();
@ -186,6 +192,7 @@ mod tests {
let q = ScimEntryGetQuery { let q = ScimEntryGetQuery {
attributes: Some(vec![Attribute::Name]), attributes: Some(vec![Attribute::Name]),
ext_access_check: false,
}; };
let txt = serde_urlencoded::to_string(&q).unwrap(); let txt = serde_urlencoded::to_string(&q).unwrap();
@ -193,9 +200,10 @@ mod tests {
let q = ScimEntryGetQuery { let q = ScimEntryGetQuery {
attributes: Some(vec![Attribute::Name, Attribute::Spn]), attributes: Some(vec![Attribute::Name, Attribute::Spn]),
ext_access_check: true,
}; };
let txt = serde_urlencoded::to_string(&q).unwrap(); let txt = serde_urlencoded::to_string(&q).unwrap();
assert_eq!(txt, "attributes=name%2Cspn"); assert_eq!(txt, "attributes=name%2Cspn&ext_access_check=true");
} }
} }

View file

@ -16,14 +16,54 @@ use uuid::Uuid;
/// A strongly typed ScimEntry that is for transmission to clients. This uses /// A strongly typed ScimEntry that is for transmission to clients. This uses
/// Kanidm internal strong types for values allowing direct serialisation and /// Kanidm internal strong types for values allowing direct serialisation and
/// transmission. /// transmission.
#[serde_as]
#[skip_serializing_none]
#[derive(Serialize, Debug, Clone, ToSchema)] #[derive(Serialize, Debug, Clone, ToSchema)]
pub struct ScimEntryKanidm { pub struct ScimEntryKanidm {
#[serde(flatten)] #[serde(flatten)]
pub header: ScimEntryHeader, pub header: ScimEntryHeader,
pub ext_access_check: Option<ScimEffectiveAccess>,
#[serde(flatten)] #[serde(flatten)]
pub attrs: BTreeMap<Attribute, ScimValueKanidm>, pub attrs: BTreeMap<Attribute, ScimValueKanidm>,
} }
#[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<Attribute>),
}
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)] #[derive(Serialize, Debug, Clone, ToSchema)]
#[serde(rename_all = "camelCase")] #[serde(rename_all = "camelCase")]
pub struct ScimAddress { pub struct ScimAddress {

View file

@ -20,10 +20,7 @@ fn parse_attributes(
input: &syn::ItemFn, input: &syn::ItemFn,
) -> Result<(proc_macro2::TokenStream, Flags), syn::Error> { ) -> Result<(proc_macro2::TokenStream, Flags), syn::Error> {
let args: Punctuated<ExprAssign, syn::token::Comma> = let args: Punctuated<ExprAssign, syn::token::Comma> =
match Punctuated::<ExprAssign, Token![,]>::parse_terminated.parse(args.clone()) { Punctuated::<ExprAssign, Token![,]>::parse_terminated.parse(args.clone())?;
Ok(it) => it,
Err(e) => return Err(e),
};
let args_are_allowed = args.pairs().all(|p| { let args_are_allowed = args.pairs().all(|p| {
ALLOWED_ATTRIBUTES.to_vec().contains( ALLOWED_ATTRIBUTES.to_vec().contains(

View file

@ -41,12 +41,14 @@ use crate::prelude::*;
use crate::repl::cid::Cid; use crate::repl::cid::Cid;
use crate::repl::entry::EntryChangeState; use crate::repl::entry::EntryChangeState;
use crate::repl::proto::{ReplEntryV1, ReplIncrementalEntryV1}; use crate::repl::proto::{ReplEntryV1, ReplIncrementalEntryV1};
use crate::server::access::AccessEffectivePermission;
use compact_jwt::JwsEs256Signer; use compact_jwt::JwsEs256Signer;
use hashbrown::{HashMap, HashSet}; use hashbrown::{HashMap, HashSet};
use kanidm_proto::internal::ImageValue; use kanidm_proto::internal::ImageValue;
use kanidm_proto::internal::{ use kanidm_proto::internal::{
ConsistencyError, Filter as ProtoFilter, OperationError, SchemaError, UiHint, ConsistencyError, Filter as ProtoFilter, OperationError, SchemaError, UiHint,
}; };
use kanidm_proto::scim_v1::server::ScimEffectiveAccess;
use kanidm_proto::v1::Entry as ProtoEntry; use kanidm_proto::v1::Entry as ProtoEntry;
use ldap3_proto::simple::{LdapPartialAttribute, LdapSearchResultEntry}; use ldap3_proto::simple::{LdapPartialAttribute, LdapSearchResultEntry};
use openssl::ec::EcKey; use openssl::ec::EcKey;
@ -160,6 +162,7 @@ pub struct EntrySealed {
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
pub struct EntryReduced { pub struct EntryReduced {
uuid: Uuid, uuid: Uuid,
effective_access: Option<Box<AccessEffectivePermission>>,
} }
// One day this is going to be Map<Attribute, ValueSet> - @yaleman // One day this is going to be Map<Attribute, ValueSet> - @yaleman
@ -1782,6 +1785,7 @@ impl Entry<EntrySealed, EntryCommitted> {
Entry { Entry {
valid: EntryReduced { valid: EntryReduced {
uuid: self.valid.uuid, uuid: self.valid.uuid,
effective_access: None,
}, },
state: self.state, state: self.state,
attrs: self.attrs, attrs: self.attrs,
@ -1793,6 +1797,7 @@ impl Entry<EntrySealed, EntryCommitted> {
pub fn reduce_attributes( pub fn reduce_attributes(
&self, &self,
allowed_attrs: &BTreeSet<Attribute>, allowed_attrs: &BTreeSet<Attribute>,
effective_access: Option<Box<AccessEffectivePermission>>,
) -> Entry<EntryReduced, EntryCommitted> { ) -> Entry<EntryReduced, EntryCommitted> {
// Remove all attrs from our tree that are NOT in the allowed set. // Remove all attrs from our tree that are NOT in the allowed set.
let f_attrs: Map<_, _> = self let f_attrs: Map<_, _> = self
@ -1809,6 +1814,7 @@ impl Entry<EntrySealed, EntryCommitted> {
let valid = EntryReduced { let valid = EntryReduced {
uuid: self.valid.uuid, uuid: self.valid.uuid,
effective_access,
}; };
let state = self.state.clone(); let state = self.state.clone();
@ -2293,6 +2299,22 @@ impl Entry<EntryReduced, EntryCommitted> {
let attrs = result?; 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(); let id = self.get_uuid();
// Not sure how I want to handle this yet, I think we need some schema changes // Not sure how I want to handle this yet, I think we need some schema changes
@ -2309,6 +2331,7 @@ impl Entry<EntryReduced, EntryCommitted> {
// entry to store some extra metadata. // entry to store some extra metadata.
meta: None, meta: None,
}, },
ext_access_check,
attrs, attrs,
}) })
} }

View file

@ -77,6 +77,7 @@ pub struct SearchEvent {
// This is the original filter, for the purpose of ACI checking. // This is the original filter, for the purpose of ACI checking.
pub filter_orig: Filter<FilterValid>, pub filter_orig: Filter<FilterValid>,
pub attrs: Option<BTreeSet<Attribute>>, pub attrs: Option<BTreeSet<Attribute>>,
pub effective_access_check: bool,
} }
impl SearchEvent { impl SearchEvent {
@ -99,6 +100,7 @@ impl SearchEvent {
// We can't get this from the SearchMessage because it's annoying with the // We can't get this from the SearchMessage because it's annoying with the
// current macro design. // current macro design.
attrs: None, attrs: None,
effective_access_check: false,
}) })
} }
@ -132,6 +134,7 @@ impl SearchEvent {
filter, filter,
filter_orig, filter_orig,
attrs: r_attrs, attrs: r_attrs,
effective_access_check: false,
}) })
} }
@ -168,6 +171,7 @@ impl SearchEvent {
filter, filter,
filter_orig, filter_orig,
attrs: r_attrs, attrs: r_attrs,
effective_access_check: false,
}) })
} }
@ -185,6 +189,7 @@ impl SearchEvent {
filter, filter,
filter_orig, filter_orig,
attrs: None, attrs: None,
effective_access_check: false,
}) })
} }
@ -202,6 +207,7 @@ impl SearchEvent {
filter, filter,
filter_orig, filter_orig,
attrs: None, attrs: None,
effective_access_check: false,
}) })
} }
@ -217,6 +223,7 @@ impl SearchEvent {
filter: filter.clone().into_valid(), filter: filter.clone().into_valid(),
filter_orig: filter.into_valid(), filter_orig: filter.into_valid(),
attrs: None, attrs: None,
effective_access_check: false,
} }
} }
@ -229,6 +236,7 @@ impl SearchEvent {
filter: filter.clone().into_valid(), filter: filter.clone().into_valid(),
filter_orig: filter.into_valid(), filter_orig: filter.into_valid(),
attrs: None, attrs: None,
effective_access_check: false,
} }
} }
@ -242,6 +250,7 @@ impl SearchEvent {
filter, filter,
filter_orig, filter_orig,
attrs: None, attrs: None,
effective_access_check: false,
} }
} }
@ -260,6 +269,7 @@ impl SearchEvent {
filter, filter,
filter_orig, filter_orig,
attrs: None, attrs: None,
effective_access_check: false,
} }
} }
@ -276,6 +286,7 @@ impl SearchEvent {
filter: filter.clone().into_valid().into_ignore_hidden(), filter: filter.clone().into_valid().into_ignore_hidden(),
filter_orig: filter.into_valid(), filter_orig: filter.into_valid(),
attrs: None, attrs: None,
effective_access_check: false,
} }
} }
@ -296,6 +307,7 @@ impl SearchEvent {
filter, filter,
filter_orig, filter_orig,
attrs, attrs,
effective_access_check: false,
}) })
} }
@ -308,6 +320,7 @@ impl SearchEvent {
filter: filter.clone().into_valid(), filter: filter.clone().into_valid(),
filter_orig: filter.into_valid(), filter_orig: filter.into_valid(),
attrs: None, attrs: None,
effective_access_check: false,
} }
} }
@ -317,6 +330,7 @@ impl SearchEvent {
filter: filter.clone(), filter: filter.clone(),
filter_orig: filter, filter_orig: filter,
attrs: None, attrs: None,
effective_access_check: false,
} }
} }
} }

View file

@ -23,7 +23,7 @@ use concread::arcache::ARCacheBuilder;
use concread::cowcell::*; use concread::cowcell::*;
use uuid::Uuid; 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::event::{CreateEvent, DeleteEvent, ModifyEvent, SearchEvent};
use crate::filter::{Filter, FilterValid, ResolveFilterCache, ResolveFilterCacheReadTxn}; use crate::filter::{Filter, FilterValid, ResolveFilterCache, ResolveFilterCacheReadTxn};
use crate::modify::Modify; use crate::modify::Modify;
@ -36,6 +36,8 @@ use self::profiles::{
AccessControlSearchResolved, AccessControlTarget, AccessControlTargetCondition, AccessControlSearchResolved, AccessControlTarget, AccessControlTargetCondition,
}; };
use kanidm_proto::scim_v1::server::ScimAttributeEffectiveAccess;
use self::create::{apply_create_access, CreateResult}; use self::create::{apply_create_access, CreateResult};
use self::delete::{apply_delete_access, DeleteResult}; use self::delete::{apply_delete_access, DeleteResult};
use self::modify::{apply_modify_access, ModifyResult}; use self::modify::{apply_modify_access, ModifyResult};
@ -57,6 +59,16 @@ pub enum Access {
Allow(BTreeSet<Attribute>), Allow(BTreeSet<Attribute>),
} }
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)] #[derive(Debug, Clone, PartialEq, Eq)]
pub enum AccessClass { pub enum AccessClass {
Grant, Grant,
@ -66,8 +78,9 @@ pub enum AccessClass {
#[derive(Debug, Clone, PartialEq, Eq)] #[derive(Debug, Clone, PartialEq, Eq)]
pub struct AccessEffectivePermission { pub struct AccessEffectivePermission {
// I don't think we need this? The ident is implied by the requester. /// Who the access applies to
// ident: Uuid, pub ident: Uuid,
/// The target the access affects
pub target: Uuid, pub target: Uuid,
pub delete: bool, pub delete: bool,
pub search: Access, pub search: Access,
@ -79,12 +92,13 @@ pub struct AccessEffectivePermission {
pub enum AccessResult { pub enum AccessResult {
// Deny this operation unconditionally. // Deny this operation unconditionally.
Denied, Denied,
// Unbounded allow, provided no denied exists. // Unbounded allow, provided no deny state exists.
Grant, Grant,
// This module makes no decisions about this entry. // This module makes no decisions about this entry.
Ignore, Ignore,
// Limit the allowed attr set to this - this doesn't // 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<Attribute>), Constrain(BTreeSet<Attribute>),
// Allow these attributes within constraints. // Allow these attributes within constraints.
Allow(BTreeSet<Attribute>), Allow(BTreeSet<Attribute>),
@ -181,7 +195,11 @@ pub trait AccessControlsTransaction<'a> {
fn get_acp_resolve_filter_cache(&self) -> &mut ResolveFilterCacheReadTxn<'a>; fn get_acp_resolve_filter_cache(&self) -> &mut ResolveFilterCacheReadTxn<'a>;
#[instrument(level = "trace", name = "access::search_related_acp", skip_all)] #[instrument(level = "trace", name = "access::search_related_acp", skip_all)]
fn search_related_acp<'b>(&'b self, ident: &Identity) -> Vec<AccessControlSearchResolved<'b>> { fn search_related_acp<'b>(
&'b self,
ident: &Identity,
attrs: Option<&BTreeSet<Attribute>>,
) -> Vec<AccessControlSearchResolved<'b>> {
let search_state = self.get_search(); let search_state = self.get_search();
let acp_resolve_filter_cache = self.get_acp_resolve_filter_cache(); let acp_resolve_filter_cache = self.get_acp_resolve_filter_cache();
@ -249,8 +267,18 @@ pub trait AccessControlsTransaction<'a> {
}) })
.collect(); .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 related_acp
// }
} }
#[instrument(level = "debug", name = "access::filter_entries", skip_all)] #[instrument(level = "debug", name = "access::filter_entries", skip_all)]
@ -267,7 +295,7 @@ pub trait AccessControlsTransaction<'a> {
let requested_attrs: BTreeSet<Attribute> = filter_orig.get_attr_set(); let requested_attrs: BTreeSet<Attribute> = filter_orig.get_attr_set();
// First get the set of acps that apply to this receiver // 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. // For each entry.
let entries_is_empty = entries.is_empty(); let entries_is_empty = entries.is_empty();
@ -318,33 +346,61 @@ pub trait AccessControlsTransaction<'a> {
name = "access::search_filter_entry_attributes", name = "access::search_filter_entry_attributes",
skip_all skip_all
)] )]
fn search_filter_entry_attributes( fn search_filter_entry_attributes<'b>(
&self, &'b self,
se: &SearchEvent, se: &SearchEvent,
entries: Vec<Arc<EntrySealedCommitted>>, entries: Vec<Arc<EntrySealedCommitted>>,
) -> Result<Vec<Entry<EntryReduced, EntryCommitted>>, OperationError> { ) -> Result<Vec<EntryReducedCommitted>, OperationError> {
struct DoEffectiveCheck<'b> {
modify_related_acp: Vec<AccessControlModifyResolved<'b>>,
delete_related_acp: Vec<AccessControlDeleteResolved<'b>>,
sync_agmts: &'b HashMap<Uuid, BTreeSet<Attribute>>,
}
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 // Build a reference set from the req_attrs. This is what we test against
// to see if the attribute is something we currently want. // 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. // Get the relevant acps for this receiver.
let related_acp = self.search_related_acp(&se.ident); let search_related_acp = self.search_related_acp(&se.ident, se.attrs.as_ref());
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
};
// For each entry. // For each entry.
let entries_is_empty = entries.is_empty(); let entries_is_empty = entries.is_empty();
let allowed_entries: Vec<_> = entries let allowed_entries: Vec<_> = entries
.into_iter() .into_iter()
.filter_map(|e| { .filter_map(|entry| {
match apply_search_access(&se.ident, related_acp.as_slice(), &e) { match apply_search_access(&se.ident, &search_related_acp, &entry) {
SearchResult::Denied => { SearchResult::Denied => {
None None
} }
@ -369,11 +425,20 @@ pub trait AccessControlsTransaction<'a> {
allowed_attrs allowed_attrs
}; };
if reduced_attrs.is_empty() { let effective_permissions = do_effective_check.as_ref().map(|do_check| {
None self.entry_effective_permission_check(
} else { &se.ident,
Some(e.reduce_attributes(&reduced_attrs)) 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 // have an entry template. I think james was right about the create being
// a template copy op ... // a template copy op ...
match &ident.origin { let ident_uuid = match &ident.origin {
IdentType::Internal => { IdentType::Internal => {
// In production we can't risk leaking data here, so we return // In production we can't risk leaking data here, so we return
// empty sets. // empty sets.
@ -810,7 +875,7 @@ pub trait AccessControlsTransaction<'a> {
security_critical!("Blocking sync check"); security_critical!("Blocking sync check");
return Err(OperationError::InvalidState); return Err(OperationError::InvalidState);
} }
IdentType::User(_) => {} IdentType::User(u) => u.entry.get_uuid(),
}; };
trace!(ident = %ident, "Effective permission check"); trace!(ident = %ident, "Effective permission check");
@ -818,31 +883,48 @@ pub trait AccessControlsTransaction<'a> {
// == search == // == search ==
// Get the relevant acps for this receiver. // Get the relevant acps for this receiver.
let search_related_acp = self.search_related_acp(ident); let search_related_acp = self.search_related_acp(ident, attrs.as_ref());
// 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
};
// == modify == // == modify ==
let modify_related_acp = self.modify_related_acp(ident); let modify_related_acp = self.modify_related_acp(ident);
// == delete ==
let delete_related_acp = self.delete_related_acp(ident); let delete_related_acp = self.delete_related_acp(ident);
let sync_agmts = self.get_sync_agreements(); let sync_agmts = self.get_sync_agreements();
let effective_permissions: Vec<_> = entries let effective_permissions: Vec<_> = entries
.iter() .iter()
.map(|e| { .map(|entry| {
self.entry_effective_permission_check(
ident,
ident_uuid,
entry,
&search_related_acp,
&modify_related_acp,
&delete_related_acp,
sync_agmts,
)
})
.collect();
effective_permissions.iter().for_each(|ep| {
trace!(?ep);
});
Ok(effective_permissions)
}
fn entry_effective_permission_check<'b>(
&'b self,
ident: &Identity,
ident_uuid: Uuid,
entry: &Arc<EntrySealedCommitted>,
search_related_acp: &[AccessControlSearchResolved<'b>],
modify_related_acp: &[AccessControlModifyResolved<'b>],
delete_related_acp: &[AccessControlDeleteResolved<'b>],
sync_agmts: &HashMap<Uuid, BTreeSet<Attribute>>,
) -> AccessEffectivePermission {
// == search == // == search ==
let search_effective = let search_effective = match apply_search_access(ident, search_related_acp, entry) {
match apply_search_access(ident, search_related_acp.as_slice(), e) {
SearchResult::Denied => Access::Denied, SearchResult::Denied => Access::Denied,
SearchResult::Grant => Access::Grant, SearchResult::Grant => Access::Grant,
SearchResult::Allow(allowed_attrs) => { SearchResult::Allow(allowed_attrs) => {
@ -852,12 +934,8 @@ pub trait AccessControlsTransaction<'a> {
}; };
// == modify == // == modify ==
let (modify_pres, modify_rem, modify_class) = match apply_modify_access( let (modify_pres, modify_rem, modify_class) =
ident, match apply_modify_access(ident, modify_related_acp, sync_agmts, entry) {
modify_related_acp.as_slice(),
sync_agmts,
e,
) {
ModifyResult::Denied => (Access::Denied, Access::Denied, AccessClass::Denied), ModifyResult::Denied => (Access::Denied, Access::Denied, AccessClass::Denied),
ModifyResult::Grant => (Access::Grant, Access::Grant, AccessClass::Grant), ModifyResult::Grant => (Access::Grant, Access::Grant, AccessClass::Grant),
ModifyResult::Allow { pres, rem, cls } => ( ModifyResult::Allow { pres, rem, cls } => (
@ -868,7 +946,7 @@ pub trait AccessControlsTransaction<'a> {
}; };
// == delete == // == delete ==
let delete_status = apply_delete_access(ident, delete_related_acp.as_slice(), e); let delete_status = apply_delete_access(ident, delete_related_acp, entry);
let delete = match delete_status { let delete = match delete_status {
DeleteResult::Denied => false, DeleteResult::Denied => false,
@ -876,21 +954,14 @@ pub trait AccessControlsTransaction<'a> {
}; };
AccessEffectivePermission { AccessEffectivePermission {
target: e.get_uuid(), ident: ident_uuid,
target: entry.get_uuid(),
delete, delete,
search: search_effective, search: search_effective,
modify_pres, modify_pres,
modify_rem, modify_rem,
modify_class, modify_class,
} }
})
.collect();
effective_permissions.iter().for_each(|ep| {
trace!(?ep);
});
Ok(effective_permissions)
} }
} }
@ -2535,6 +2606,7 @@ mod tests {
vec![], vec![],
&r_set, &r_set,
vec![AccessEffectivePermission { vec![AccessEffectivePermission {
ident: UUID_TEST_ACCOUNT_1,
delete: false, delete: false,
target: uuid!("cc8e95b4-c24f-4d68-ba54-8bed76f63930"), target: uuid!("cc8e95b4-c24f-4d68-ba54-8bed76f63930"),
search: Access::Allow(btreeset![Attribute::Name]), search: Access::Allow(btreeset![Attribute::Name]),
@ -2576,6 +2648,7 @@ mod tests {
)], )],
&r_set, &r_set,
vec![AccessEffectivePermission { vec![AccessEffectivePermission {
ident: UUID_TEST_ACCOUNT_1,
delete: false, delete: false,
target: uuid!("cc8e95b4-c24f-4d68-ba54-8bed76f63930"), target: uuid!("cc8e95b4-c24f-4d68-ba54-8bed76f63930"),
search: Access::Allow(BTreeSet::new()), search: Access::Allow(BTreeSet::new()),

View file

@ -284,7 +284,7 @@ pub trait QueryServerTransaction<'a> {
fn search_ext( fn search_ext(
&mut self, &mut self,
se: &SearchEvent, se: &SearchEvent,
) -> Result<Vec<Entry<EntryReduced, EntryCommitted>>, OperationError> { ) -> Result<Vec<EntryReducedCommitted>, OperationError> {
/* /*
* This just wraps search, but it's for the external interface * This just wraps search, but it's for the external interface
* so as a result it also reduces the entry set's attributes at * so as a result it also reduces the entry set's attributes at
@ -1546,6 +1546,7 @@ impl QueryServerReadTransaction<'_> {
filter: f_valid, filter: f_valid,
filter_orig: f_intent_valid, filter_orig: f_intent_valid,
attrs: r_attrs, attrs: r_attrs,
effective_access_check: query.ext_access_check,
}; };
let mut vs = self.search_ext(&se)?; let mut vs = self.search_ext(&se)?;
@ -2639,6 +2640,7 @@ impl<'a> QueryServerWriteTransaction<'a> {
mod tests { mod tests {
use crate::prelude::*; use crate::prelude::*;
use kanidm_proto::scim_v1::server::ScimReference; use kanidm_proto::scim_v1::server::ScimReference;
use kanidm_proto::scim_v1::ScimEntryGetQuery;
#[qs_test] #[qs_test]
async fn test_name_to_uuid(server: &QueryServer) { 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));
}
} }

View file

@ -14,6 +14,10 @@ pub struct ScimEntryPutEvent {
/// Update an attribute to contain the following value state. /// Update an attribute to contain the following value state.
/// If the attribute is None, it is removed. /// If the attribute is None, it is removed.
pub attrs: BTreeMap<Attribute, Option<ValueSet>>, pub attrs: BTreeMap<Attribute, Option<ValueSet>>,
/// If an effective access check should be carried out post modification
/// of the entries
pub effective_access_check: bool,
} }
impl ScimEntryPutEvent { impl ScimEntryPutEvent {
@ -33,10 +37,13 @@ impl ScimEntryPutEvent {
}) })
.collect::<Result<_, _>>()?; .collect::<Result<_, _>>()?;
let query = entry.query;
Ok(ScimEntryPutEvent { Ok(ScimEntryPutEvent {
ident, ident,
target, target,
attrs, attrs,
effective_access_check: query.ext_access_check,
}) })
} }
} }
@ -54,6 +61,7 @@ impl QueryServerWriteTransaction<'_> {
ident, ident,
target, target,
attrs, attrs,
effective_access_check,
} = scim_entry_put; } = scim_entry_put;
// This function transforms the put event into a modify event. // This function transforms the put event into a modify event.
@ -91,6 +99,7 @@ impl QueryServerWriteTransaction<'_> {
filter_orig: f_intent_valid, filter_orig: f_intent_valid,
// Return all attributes, even ones we didn't affect // Return all attributes, even ones we didn't affect
attrs: None, attrs: None,
effective_access_check,
}; };
let mut vs = self.search_ext(&se)?; let mut vs = self.search_ext(&se)?;

View file

@ -202,6 +202,7 @@ async fn test_scim_sync_entry_get(rsclient: KanidmClient) {
// Limit the attributes we want. // Limit the attributes we want.
let query = ScimEntryGetQuery { let query = ScimEntryGetQuery {
attributes: Some(vec![Attribute::Name]), attributes: Some(vec![Attribute::Name]),
..Default::default()
}; };
let scim_entry = rsclient let scim_entry = rsclient
@ -238,6 +239,7 @@ async fn test_scim_sync_entry_get(rsclient: KanidmClient) {
// Limit the attributes we want. // Limit the attributes we want.
let query = ScimEntryGetQuery { let query = ScimEntryGetQuery {
attributes: Some(vec![Attribute::Name]), attributes: Some(vec![Attribute::Name]),
..Default::default()
}; };
let scim_entry = rsclient let scim_entry = rsclient

View file

@ -238,6 +238,7 @@ impl PersonOpt {
aopt.aopts.account_id.as_str(), aopt.aopts.account_id.as_str(),
Some(ScimEntryGetQuery { Some(ScimEntryGetQuery {
attributes: Some(vec![Attribute::SshPublicKey]), attributes: Some(vec![Attribute::SshPublicKey]),
..Default::default()
}), }),
) )
.await .await