diff --git a/designs/system_protected_objects.rst b/designs/system_protected_objects.rst new file mode 100644 index 000000000..0273bfec1 --- /dev/null +++ b/designs/system_protected_objects.rst @@ -0,0 +1,60 @@ +System Protected Objects +------------------------ + +There are a number of system provided objects and functions that are important +for correct operation of the server. It's important we protect these from certain +types of modifications so that the system has a baseline of correct functionality. + +Some of this is protected by the migrations system built into the server, which +will correct and "set" certain items to a known state on startup. However, like +all IDM systems, downtime/restarts are rare, so we have to account for human +error and provide constraints to maintain a healthy running system. + +What needs protecting? +---------------------- + +The current list (july 2019) is: + +* Core schema (enough to make attributetypes/classtypes/memberof/acp operate) +* Anonymous +* Admin +* systeminfo + +Additionally, the scope of protection is limited - or rather, the scope of what is +allowed. + +* New system protected items can only be created via internal operations, preventing +accidental creation of these. +* schema classtypes can have must/may altered on classes +* schema attributetypes can have index altered on classes +* anonymous can be locked +* admin can be locked, and password changed (or other credentials changed). + +The plugin design +----------------- + +This should be a plugin, as the hooks exist in the correct places to intercept and block +the required operations, saving custom coding. The plugin will: + +* Block any create on "class": "system". +* Block any delete on "class": "system". +* Block modify to "class": "system" where the affect attr is NOT in the allowed mod set. + +The modify block will not be class aware, because schema protects us from other odd behaviours. +An example - addition of account lock to a schema element. This would be "allowed" by this plugin +because account lock will be in the allowed set for system, but schema would not allow addition +of accountlock to a schema element thus protecting us. Because class will NEVER be in the +allowed set for a system protected type, we can trust that these bounds won't be exceeded. + +Why not ACP? +------------ + +It was considered to provide default ACP's that would protect system items. This was rejected because: + +* it would require a "deny" acp type, and I do not wish to create this, as people could then create their own deny rules (always incorrect!) +* There would be a lot of acp's involved in this protection (but acp's are expressive enough to provide it!) +* The acp's would need a self-referencing acp to protect themselves from modification. +* Having a seperate plugin to protect this will be faster than acp processing because we check less filters (But this is not a strong argument) +* the plugin can provide targeted error messages about why they were denied, rather than a generic acp denied message. +* the plugin can provide detailed testing of edge cases in a confined manner + diff --git a/src/lib/access.rs b/src/lib/access.rs index 33fe11dec..4dfb2ed29 100644 --- a/src/lib/access.rs +++ b/src/lib/access.rs @@ -15,6 +15,8 @@ // requirements (also search). // +// TODO: Purge all reference to acp_allow/deny + use concread::cowcell::{CowCell, CowCellReadTxn, CowCellWriteTxn}; use std::collections::{BTreeMap, BTreeSet}; diff --git a/src/lib/constants.rs b/src/lib/constants.rs index a0f3e47d1..7ac718cfe 100644 --- a/src/lib/constants.rs +++ b/src/lib/constants.rs @@ -138,7 +138,6 @@ pub static UUID_SCHEMA_ATTR_UUID: &'static str = "642a893b-fe1a-4fe1-805d-fb78e7 pub static UUID_SCHEMA_ATTR_NAME: &'static str = "27be9127-5ba1-4c06-bce9-7250f2c7f630"; pub static UUID_SCHEMA_ATTR_PRINCIPAL_NAME: &'static str = "64dda3ac-12cb-4000-9b30-97a92767ccab"; pub static UUID_SCHEMA_ATTR_DESCRIPTION: &'static str = "a4da35a2-c5fb-4f8f-a341-72cd39ec9eee"; -pub static UUID_SCHEMA_ATTR_SYSTEM: &'static str = "ee28df1e-cf02-49ca-80b5-8310fb619377"; pub static UUID_SCHEMA_ATTR_SECRET: &'static str = "0231c61a-0a43-4987-9293-8732ed9459fa"; pub static UUID_SCHEMA_ATTR_MULTIVALUE: &'static str = "8a6a8bf3-7053-42e2-8cda-15af7a197513"; pub static UUID_SCHEMA_ATTR_INDEX: &'static str = "2c5ff455-0709-4f67-a37c-35ff7e67bfff"; @@ -187,6 +186,7 @@ pub static UUID_SCHEMA_CLASS_ACCESS_CONTROL_MODIFY: &'static str = "fd860561-9d0a-4f12-be30-406834292d46"; pub static UUID_SCHEMA_CLASS_ACCESS_CONTROL_CREATE: &'static str = "58c5c197-51d8-4c30-9a8e-b8a0bb0eaacd"; +pub static UUID_SCHEMA_CLASS_SYSTEM: &'static str = "ee28df1e-cf02-49ca-80b5-8310fb619377"; // system supplementary pub static UUID_SCHEMA_ATTR_DISPLAYNAME: &'static str = "201bc966-954b-48f5-bf25-99ffed759861"; @@ -198,6 +198,7 @@ pub static JSON_SCHEMA_ATTR_DISPLAYNAME: &'static str = r#"{ "attrs": { "class": [ "object", + "system", "attributetype" ], "description": [ @@ -218,9 +219,6 @@ pub static JSON_SCHEMA_ATTR_DISPLAYNAME: &'static str = r#"{ "syntax": [ "UTF8STRING" ], - "system": [ - "true" - ], "uuid": [ "201bc966-954b-48f5-bf25-99ffed759861" ] @@ -236,6 +234,7 @@ pub static JSON_SCHEMA_ATTR_MAIL: &'static str = r#" "attrs": { "class": [ "object", + "system", "attributetype" ], "description": [ @@ -256,9 +255,6 @@ pub static JSON_SCHEMA_ATTR_MAIL: &'static str = r#" "syntax": [ "UTF8STRING" ], - "system": [ - "true" - ], "uuid": [ "fae94676-720b-461b-9438-bfe8cfd7e6cd" ] @@ -275,6 +271,7 @@ pub static JSON_SCHEMA_ATTR_SSH_PUBLICKEY: &'static str = r#" "attrs": { "class": [ "object", + "system", "attributetype" ], "description": [ @@ -293,9 +290,6 @@ pub static JSON_SCHEMA_ATTR_SSH_PUBLICKEY: &'static str = r#" "syntax": [ "UTF8STRING" ], - "system": [ - "true" - ], "uuid": [ "52f2f13f-d35c-4cca-9f43-90a12c968f72" ] @@ -312,6 +306,7 @@ pub static JSON_SCHEMA_ATTR_PASSWORD: &'static str = r#" "attrs": { "class": [ "object", + "system", "attributetype" ], "description": [ @@ -330,9 +325,6 @@ pub static JSON_SCHEMA_ATTR_PASSWORD: &'static str = r#" "syntax": [ "UTF8STRING" ], - "system": [ - "true" - ], "uuid": [ "a5121082-be54-4624-a307-383839b0366b" ] @@ -350,6 +342,7 @@ pub static JSON_SCHEMA_CLASS_PERSON: &'static str = r#" "attrs": { "class": [ "object", + "system", "classtype" ], "description": [ @@ -383,6 +376,7 @@ pub static JSON_SCHEMA_CLASS_GROUP: &'static str = r#" "attrs": { "class": [ "object", + "system", "classtype" ], "description": [ @@ -413,6 +407,7 @@ pub static JSON_SCHEMA_CLASS_ACCOUNT: &'static str = r#" "attrs": { "class": [ "object", + "system", "classtype" ], "description": [ diff --git a/src/lib/entry.rs b/src/lib/entry.rs index c89f3ceea..3e001b7c9 100644 --- a/src/lib/entry.rs +++ b/src/lib/entry.rs @@ -1104,12 +1104,6 @@ impl From<&SchemaAttribute> for Entry { let name_v = vec![s.name.clone()]; let desc_v = vec![s.description.clone()]; - let system_v = vec![if s.system { - "true".to_string() - } else { - "false".to_string() - }]; - let secret_v = vec![if s.secret { "true".to_string() } else { @@ -1131,14 +1125,17 @@ impl From<&SchemaAttribute> for Entry { attrs.insert("name".to_string(), name_v); attrs.insert("description".to_string(), desc_v); attrs.insert("uuid".to_string(), uuid_v); - attrs.insert("system".to_string(), system_v); attrs.insert("secret".to_string(), secret_v); attrs.insert("multivalue".to_string(), multivalue_v); attrs.insert("index".to_string(), index_v); attrs.insert("syntax".to_string(), syntax_v); attrs.insert( "class".to_string(), - vec!["object".to_string(), "attributetype".to_string()], + vec![ + "object".to_string(), + "system".to_string(), + "attributetype".to_string(), + ], ); // Insert stuff. @@ -1167,7 +1164,11 @@ impl From<&SchemaClass> for Entry { attrs.insert("uuid".to_string(), uuid_v); attrs.insert( "class".to_string(), - vec!["object".to_string(), "classtype".to_string()], + vec![ + "object".to_string(), + "system".to_string(), + "classtype".to_string(), + ], ); attrs.insert("systemmay".to_string(), s.systemmay.clone()); attrs.insert("systemmust".to_string(), s.systemmust.clone()); diff --git a/src/lib/error.rs b/src/lib/error.rs index 24f398d78..90c0eefde 100644 --- a/src/lib/error.rs +++ b/src/lib/error.rs @@ -40,6 +40,7 @@ pub enum OperationError { NotAuthenticated, InvalidAuthState(&'static str), InvalidSessionState, + SystemProtectedObject, } #[derive(Serialize, Deserialize, Debug, PartialEq)] diff --git a/src/lib/event.rs b/src/lib/event.rs index 2b0f9dc0c..f148e8852 100644 --- a/src/lib/event.rs +++ b/src/lib/event.rs @@ -170,6 +170,13 @@ impl Event { // to audits and logs to determine what happened. event.clone() } + + pub fn is_internal(&self) -> bool { + match self.origin { + EventOrigin::Internal => true, + _ => false, + } + } } #[derive(Debug)] diff --git a/src/lib/plugins/mod.rs b/src/lib/plugins/mod.rs index d60d6683c..f0e1c8d96 100644 --- a/src/lib/plugins/mod.rs +++ b/src/lib/plugins/mod.rs @@ -17,18 +17,19 @@ mod refint; trait Plugin { fn id() -> &'static str; - // TODO: These should all return OperationError::Unimplemented - fn pre_create_transform( _au: &mut AuditScope, _qs: &QueryServerWriteTransaction, _cand: &mut Vec>, _ce: &CreateEvent, ) -> Result<(), OperationError> { - Ok(()) + debug!( + "plugin {} has an unimplemented pre_create_transform!", + Self::id() + ); + Err(OperationError::Plugin) } - /* fn pre_create( _au: &mut AuditScope, _qs: &QueryServerWriteTransaction, @@ -36,9 +37,9 @@ trait Plugin { _cand: &Vec>, _ce: &CreateEvent, ) -> Result<(), OperationError> { - Ok(()) + debug!("plugin {} has an unimplemented pre_create!", Self::id()); + Err(OperationError::Plugin) } - */ fn post_create( _au: &mut AuditScope, @@ -47,7 +48,8 @@ trait Plugin { _cand: &Vec>, _ce: &CreateEvent, ) -> Result<(), OperationError> { - Ok(()) + debug!("plugin {} has an unimplemented post_create!", Self::id()); + Err(OperationError::Plugin) } fn pre_modify( @@ -56,7 +58,8 @@ trait Plugin { _cand: &mut Vec>, _me: &ModifyEvent, ) -> Result<(), OperationError> { - Ok(()) + debug!("plugin {} has an unimplemented pre_modify!", Self::id()); + Err(OperationError::Plugin) } fn post_modify( @@ -67,7 +70,8 @@ trait Plugin { _cand: &Vec>, _ce: &ModifyEvent, ) -> Result<(), OperationError> { - Ok(()) + debug!("plugin {} has an unimplemented post_modify!", Self::id()); + Err(OperationError::Plugin) } fn pre_delete( @@ -76,7 +80,8 @@ trait Plugin { _cand: &mut Vec>, _de: &DeleteEvent, ) -> Result<(), OperationError> { - Ok(()) + debug!("plugin {} has an unimplemented pre_delete!", Self::id()); + Err(OperationError::Plugin) } fn post_delete( @@ -86,14 +91,16 @@ trait Plugin { _cand: &Vec>, _ce: &DeleteEvent, ) -> Result<(), OperationError> { - Ok(()) + debug!("plugin {} has an unimplemented post_delete!", Self::id()); + Err(OperationError::Plugin) } fn verify( _au: &mut AuditScope, _qs: &QueryServerReadTransaction, ) -> Vec> { - Vec::new() + debug!("plugin {} has an unimplemented verify!", Self::id()); + vec![Err(ConsistencyError::Unknown)] } } @@ -121,7 +128,6 @@ macro_rules! run_pre_create_transform_plugin { }}; } -/* macro_rules! run_pre_create_plugin { ( $au:ident, @@ -141,7 +147,6 @@ macro_rules! run_pre_create_plugin { r }}; } -*/ macro_rules! run_post_create_plugin { ( @@ -270,16 +275,12 @@ impl Plugins { ce: &CreateEvent, ) -> Result<(), OperationError> { audit_segment!(au, || { - let res = - run_pre_create_transform_plugin!(au, qs, cand, ce, base::Base).and_then(|_| { - run_pre_create_transform_plugin!(au, qs, cand, ce, refint::ReferentialIntegrity) - }); + let res = run_pre_create_transform_plugin!(au, qs, cand, ce, base::Base); res }) } - /* pub fn run_pre_create( au: &mut AuditScope, qs: &QueryServerWriteTransaction, @@ -287,14 +288,11 @@ impl Plugins { ce: &CreateEvent, ) -> Result<(), OperationError> { audit_segment!(au, || { - let res = run_pre_create_plugin!(au, qs, cand, ce, base::Base).and_then(|_| { - run_pre_create_plugin!(au, qs, cand, ce, refint::ReferentialIntegrity) - }); + let res = run_pre_create_plugin!(au, qs, cand, ce, protected::Protected); res }) } - */ pub fn run_post_create( au: &mut AuditScope, @@ -303,10 +301,8 @@ impl Plugins { ce: &CreateEvent, ) -> Result<(), OperationError> { audit_segment!(au, || { - let res = run_post_create_plugin!(au, qs, cand, ce, base::Base).and_then(|_| { - run_post_create_plugin!(au, qs, cand, ce, refint::ReferentialIntegrity) - .and_then(|_| run_post_create_plugin!(au, qs, cand, ce, memberof::MemberOf)) - }); + let res = run_post_create_plugin!(au, qs, cand, ce, refint::ReferentialIntegrity) + .and_then(|_| run_post_create_plugin!(au, qs, cand, ce, memberof::MemberOf)); res }) @@ -319,9 +315,8 @@ impl Plugins { me: &ModifyEvent, ) -> Result<(), OperationError> { audit_segment!(au, || { - let res = run_pre_modify_plugin!(au, qs, cand, me, base::Base).and_then(|_| { - run_pre_modify_plugin!(au, qs, cand, me, refint::ReferentialIntegrity) - }); + let res = run_pre_modify_plugin!(au, qs, cand, me, protected::Protected) + .and_then(|_| run_pre_modify_plugin!(au, qs, cand, me, base::Base)); res }) @@ -336,19 +331,10 @@ impl Plugins { ) -> Result<(), OperationError> { audit_segment!(au, || { let res = - run_post_modify_plugin!(au, qs, pre_cand, cand, me, base::Base).and_then(|_| { - run_post_modify_plugin!( - au, - qs, - pre_cand, - cand, - me, - refint::ReferentialIntegrity - ) + run_post_modify_plugin!(au, qs, pre_cand, cand, me, refint::ReferentialIntegrity) .and_then(|_| { run_post_modify_plugin!(au, qs, pre_cand, cand, me, memberof::MemberOf) - }) - }); + }); res }) @@ -361,10 +347,7 @@ impl Plugins { de: &DeleteEvent, ) -> Result<(), OperationError> { audit_segment!(au, || { - let res = run_pre_delete_plugin!(au, qs, cand, de, base::Base).and_then(|_| { - run_pre_delete_plugin!(au, qs, cand, de, refint::ReferentialIntegrity) - }); - + let res = run_pre_delete_plugin!(au, qs, cand, de, protected::Protected); res }) } @@ -376,10 +359,8 @@ impl Plugins { de: &DeleteEvent, ) -> Result<(), OperationError> { audit_segment!(au, || { - let res = run_post_delete_plugin!(au, qs, cand, de, base::Base).and_then(|_| { - run_post_delete_plugin!(au, qs, cand, de, refint::ReferentialIntegrity) - .and_then(|_| run_post_delete_plugin!(au, qs, cand, de, memberof::MemberOf)) - }); + let res = run_post_delete_plugin!(au, qs, cand, de, refint::ReferentialIntegrity) + .and_then(|_| run_post_delete_plugin!(au, qs, cand, de, memberof::MemberOf)); res }) diff --git a/src/lib/plugins/protected.rs b/src/lib/plugins/protected.rs index 02d8fbdbd..29b7b559c 100644 --- a/src/lib/plugins/protected.rs +++ b/src/lib/plugins/protected.rs @@ -1,2 +1,339 @@ -// Objects matching some filter condition should -// be protected from modification / deletion +// System protected objects. Items matching specific requirements +// may only have certain modifications performed. +use crate::plugins::Plugin; + +use crate::audit::AuditScope; +use crate::entry::{Entry, EntryCommitted, EntryInvalid, EntryNew, EntryValid}; +use crate::error::OperationError; +use crate::event::{CreateEvent, DeleteEvent, ModifyEvent}; +use crate::modify::Modify; +use crate::server::{QueryServerTransaction, QueryServerWriteTransaction}; +use std::collections::HashSet; + +pub struct Protected {} + +impl Plugin for Protected { + fn id() -> &'static str { + "plugin_protected" + } + + fn pre_create( + au: &mut AuditScope, + _qs: &QueryServerWriteTransaction, + // List of what we will commit that is valid? + cand: &Vec>, + ce: &CreateEvent, + ) -> Result<(), OperationError> { + if ce.event.is_internal() { + audit_log!( + au, + "Internal operation, not enforcing system object protection" + ); + return Ok(()); + } + + cand.iter().fold(Ok(()), |acc, cand| match acc { + Err(_) => acc, + Ok(_) => { + if cand.attribute_value_pres("class", "system") { + Err(OperationError::SystemProtectedObject) + } else { + acc + } + } + }) + } + + fn pre_modify( + au: &mut AuditScope, + _qs: &QueryServerWriteTransaction, + // Should these be EntryValid? + cand: &mut Vec>, + me: &ModifyEvent, + ) -> Result<(), OperationError> { + if me.event.is_internal() { + audit_log!( + au, + "Internal operation, not enforcing system object protection" + ); + return Ok(()); + } + // Prevent adding class: system + me.modlist.iter().fold(Ok(()), |acc, m| { + if acc.is_err() { + acc + } else { + match m { + Modify::Present(a, v) => { + if a == "class" && v == "system" { + Err(OperationError::SystemProtectedObject) + } else { + Ok(()) + } + } + _ => Ok(()), + } + } + })?; + // if class: system, check the mods are "allowed" + + let system_pres = cand.iter().fold(false, |acc, c| { + if acc { + acc + } else { + c.attribute_value_pres("class", "system") + } + }); + + audit_log!(au, "class: system -> {}", system_pres); + // No system types being altered, return. + if system_pres == false { + return Ok(()); + } + + // Something altered is system, check if it's allowed. + me.modlist.iter().fold(Ok(()), |acc, m| { + // Already hit an error, move on. + if acc.is_err() { + acc + } else { + let a = match m { + Modify::Present(a, _) => a, + Modify::Removed(a, _) => a, + Modify::Purged(a) => a, + }; + // TODO: This should be a set or some kind of structure? + // I had issues statically allocating this though ... alternately + // we could make it a "configurable" type? + if a == "must" { + Ok(()) + } else if a == "may" { + Ok(()) + } else { + Err(OperationError::SystemProtectedObject) + } + } + }) + } + + fn pre_delete( + au: &mut AuditScope, + _qs: &QueryServerWriteTransaction, + // Should these be EntryValid + cand: &mut Vec>, + de: &DeleteEvent, + ) -> Result<(), OperationError> { + if de.event.is_internal() { + audit_log!( + au, + "Internal operation, not enforcing system object protection" + ); + return Ok(()); + } + + cand.iter().fold(Ok(()), |acc, cand| match acc { + Err(_) => acc, + Ok(_) => { + if cand.attribute_value_pres("class", "system") { + Err(OperationError::SystemProtectedObject) + } else { + acc + } + } + }) + } +} + +#[cfg(test)] +mod tests { + use crate::constants::JSON_ADMIN_V1; + use crate::entry::{Entry, EntryInvalid, EntryNew}; + use crate::error::OperationError; + + static JSON_ADMIN_ALLOW_ALL: &'static str = r#"{ + "valid": null, + "state": null, + "attrs": { + "class": [ + "object", + "access_control_profile", + "access_control_modify", + "access_control_create", + "access_control_delete", + "access_control_search" + ], + "name": ["idm_admins_acp_allow_all_test"], + "uuid": ["bb18f746-a409-497d-928c-5455d4aef4f7"], + "description": ["Builtin IDM Administrators Access Controls."], + "acp_enable": ["true"], + "acp_receiver": [ + "{\"Eq\":[\"uuid\",\"00000000-0000-0000-0000-000000000000\"]}" + ], + "acp_targetscope": [ + "{\"Pres\":\"class\"}" + ], + "acp_search_attr": ["name", "class", "uuid"], + "acp_modify_class": ["system"], + "acp_modify_removedattr": ["class", "displayname", "may", "must"], + "acp_modify_presentattr": ["class", "displayname", "may", "must"], + "acp_create_class": ["object", "person", "system"], + "acp_create_attr": ["name", "class", "description", "displayname"] + } + }"#; + + #[test] + fn test_pre_create_deny() { + // Test creating with class: system is rejected. + let acp: Entry = + serde_json::from_str(JSON_ADMIN_ALLOW_ALL).expect("json parse failure"); + + let preload = vec![acp]; + + let e: Entry = serde_json::from_str( + r#"{ + "valid": null, + "state": null, + "attrs": { + "class": ["person", "system"], + "name": ["testperson"], + "description": ["testperson"], + "displayname": ["testperson"] + } + }"#, + ) + .expect("json parse failure"); + + let create = vec![e.clone()]; + + run_create_test!( + Err(OperationError::SystemProtectedObject), + preload, + create, + Some(JSON_ADMIN_V1), + |_, _| {} + ); + } + + #[test] + fn test_pre_modify_system_deny() { + let acp: Entry = + serde_json::from_str(JSON_ADMIN_ALLOW_ALL).expect("json parse failure"); + // Test modify of class to a system is denied + let e: Entry = serde_json::from_str( + r#"{ + "valid": null, + "state": null, + "attrs": { + "class": ["person", "system"], + "name": ["testperson"], + "description": ["testperson"], + "displayname": ["testperson"] + } + }"#, + ) + .expect("json parse failure"); + + let preload = vec![acp, e.clone()]; + + run_modify_test!( + Err(OperationError::SystemProtectedObject), + preload, + filter!(f_eq("name", "testperson")), + modlist!([m_purge("displayname"), m_pres("displayname", "system test"),]), + Some(JSON_ADMIN_V1), + |_, _| {} + ); + } + + #[test] + fn test_pre_modify_class_add_deny() { + let acp: Entry = + serde_json::from_str(JSON_ADMIN_ALLOW_ALL).expect("json parse failure"); + // Show that adding a system class is denied + let e: Entry = serde_json::from_str( + r#"{ + "valid": null, + "state": null, + "attrs": { + "class": ["person"], + "name": ["testperson"], + "description": ["testperson"], + "displayname": ["testperson"] + } + }"#, + ) + .expect("json parse failure"); + + let preload = vec![acp, e.clone()]; + + run_modify_test!( + Err(OperationError::SystemProtectedObject), + preload, + filter!(f_eq("name", "testperson")), + modlist!([m_pres("class", "system"),]), + Some(JSON_ADMIN_V1), + |_, _| {} + ); + } + + #[test] + fn test_pre_modify_attr_must_may_allow() { + let acp: Entry = + serde_json::from_str(JSON_ADMIN_ALLOW_ALL).expect("json parse failure"); + // Show that adding a system class is denied + let e: Entry = serde_json::from_str( + r#"{ + "valid": null, + "state": null, + "attrs": { + "class": ["object", "classtype"], + "name": ["testclass"], + "uuid": ["cfcae205-31c3-484b-8ced-667d1709c5e3"], + "description": ["Test Class"] + } + }"#, + ) + .expect("json parse failure"); + + let preload = vec![acp, e.clone()]; + + run_modify_test!( + Ok(()), + preload, + filter!(f_eq("name", "testclass")), + modlist!([m_pres("may", "name"), m_pres("must", "name"),]), + Some(JSON_ADMIN_V1), + |_, _| {} + ); + } + + #[test] + fn test_pre_delete_deny() { + let acp: Entry = + serde_json::from_str(JSON_ADMIN_ALLOW_ALL).expect("json parse failure"); + // Test deleting with class: system is rejected. + let e: Entry = serde_json::from_str( + r#"{ + "valid": null, + "state": null, + "attrs": { + "class": ["person", "system"], + "name": ["testperson"], + "description": ["testperson"], + "displayname": ["testperson"] + } + }"#, + ) + .expect("json parse failure"); + + let preload = vec![acp, e.clone()]; + + run_delete_test!( + Err(OperationError::SystemProtectedObject), + preload, + filter!(f_eq("name", "testperson")), + Some(JSON_ADMIN_V1), + |_, _| {} + ); + } +} diff --git a/src/lib/schema.rs b/src/lib/schema.rs index 361b99cff..422425043 100644 --- a/src/lib/schema.rs +++ b/src/lib/schema.rs @@ -19,15 +19,6 @@ use concread::cowcell::{CowCell, CowCellReadTxn, CowCellWriteTxn}; // In the future this will parse/read it's schema from the db // but we have to bootstrap with some core types. -// TODO: system_info metadata object schema - -// TODO: system class to indicate the type is a system object? -// just a class? Does the class imply protections? -// probably just protection from delete and modify, except systemmay/systemmust/index? - -// TODO: Schema types -> Entry conversion -// TODO: Entry -> Schema given class. This is for loading from the db. - // TODO: prefix on all schema types that are system? #[allow(non_camel_case_types)] @@ -132,7 +123,8 @@ pub struct SchemaAttribute { pub uuid: Uuid, // Perhaps later add aliases? pub description: String, - pub system: bool, + // TODO: Remove this field, it does nothing + // and will be replaced by a different specific schema element. pub secret: bool, pub multivalue: bool, pub index: Vec, @@ -174,16 +166,6 @@ impl SchemaAttribute { .ok_or(OperationError::InvalidSchemaState("missing description")) ); - // system - let system = try_audit!( - audit, - value - .get_ava_single_bool("system") - .ok_or(OperationError::InvalidSchemaState( - "missing or invalid system" - )) - ); - // secret let secret = try_audit!( audit, @@ -219,7 +201,6 @@ impl SchemaAttribute { name: name.clone(), uuid: uuid.clone(), description: description.clone(), - system: system, secret: secret, multivalue: multivalue, index: index, @@ -548,7 +529,6 @@ impl SchemaInner { uuid: Uuid::parse_str(UUID_SCHEMA_ATTR_CLASS) .expect("unable to parse static uuid"), description: String::from("The set of classes defining an object"), - system: true, secret: false, multivalue: true, index: vec![IndexType::EQUALITY], @@ -562,7 +542,6 @@ impl SchemaInner { uuid: Uuid::parse_str(UUID_SCHEMA_ATTR_UUID) .expect("unable to parse static uuid"), description: String::from("The universal unique id of the object"), - system: true, secret: false, multivalue: false, index: vec![IndexType::EQUALITY], @@ -576,7 +555,6 @@ impl SchemaInner { uuid: Uuid::parse_str(UUID_SCHEMA_ATTR_NAME) .expect("unable to parse static uuid"), description: String::from("The shortform name of an object"), - system: true, secret: false, multivalue: false, index: vec![IndexType::EQUALITY], @@ -585,17 +563,16 @@ impl SchemaInner { ); s.attributes.insert( String::from("principal_name"), - SchemaAttribute { - name: String::from("principal_name"), - uuid: Uuid::parse_str(UUID_SCHEMA_ATTR_PRINCIPAL_NAME).expect("unable to parse static uuid"), - description: String::from("The longform name of an object, derived from name and domain. Example: alice@project.org"), - system: true, - secret: false, - multivalue: false, - index: vec![IndexType::EQUALITY], - syntax: SyntaxType::UTF8STRING_PRINCIPAL, - }, - ); + SchemaAttribute { + name: String::from("principal_name"), + uuid: Uuid::parse_str(UUID_SCHEMA_ATTR_PRINCIPAL_NAME).expect("unable to parse static uuid"), + description: String::from("The longform name of an object, derived from name and domain. Example: alice@project.org"), + secret: false, + multivalue: false, + index: vec![IndexType::EQUALITY], + syntax: SyntaxType::UTF8STRING_PRINCIPAL, + }, + ); s.attributes.insert( String::from("description"), SchemaAttribute { @@ -603,53 +580,31 @@ impl SchemaInner { uuid: Uuid::parse_str(UUID_SCHEMA_ATTR_DESCRIPTION) .expect("unable to parse static uuid"), description: String::from("A description of an attribute, object or class"), - system: true, secret: false, multivalue: true, index: vec![], syntax: SyntaxType::UTF8STRING, }, ); - s.attributes.insert( - // FIXME: Rename to system_provided? Or should we eschew this in favour of class? - // system_provided attr seems easier to provide access controls on, and can be - // part of object ... - String::from("system"), - SchemaAttribute { - name: String::from("system"), - uuid: Uuid::parse_str(UUID_SCHEMA_ATTR_SYSTEM) - .expect("unable to parse static uuid"), - description: String::from( - "Is this object or attribute provided from the core system?", - ), - system: true, - secret: false, - multivalue: false, - index: vec![], - syntax: SyntaxType::BOOLEAN, - }, - ); s.attributes.insert(String::from("secret"), SchemaAttribute { - // FIXME: Rename from system to schema_private? system_private? attr_private? private_attr? - name: String::from("secret"), - uuid: Uuid::parse_str(UUID_SCHEMA_ATTR_SECRET).expect("unable to parse static uuid"), - description: String::from("If true, this value is always hidden internally to the server, even beyond access controls."), - system: true, - secret: false, - multivalue: false, - index: vec![], - syntax: SyntaxType::BOOLEAN, - }); + // FIXME: Rename from system to schema_private? system_private? attr_private? private_attr? + name: String::from("secret"), + uuid: Uuid::parse_str(UUID_SCHEMA_ATTR_SECRET).expect("unable to parse static uuid"), + description: String::from("If true, this value is always hidden internally to the server, even beyond access controls."), + secret: false, + multivalue: false, + index: vec![], + syntax: SyntaxType::BOOLEAN, + }); s.attributes.insert(String::from("multivalue"), SchemaAttribute { - name: String::from("multivalue"), - uuid: Uuid::parse_str(UUID_SCHEMA_ATTR_MULTIVALUE).expect("unable to parse static uuid"), - description: String::from("If true, this attribute is able to store multiple values rather than just a single value."), - system: true, - secret: false, - multivalue: false, - index: vec![], - syntax: SyntaxType::BOOLEAN, - }); + name: String::from("multivalue"), + uuid: Uuid::parse_str(UUID_SCHEMA_ATTR_MULTIVALUE).expect("unable to parse static uuid"), + description: String::from("If true, this attribute is able to store multiple values rather than just a single value."), + secret: false, + multivalue: false, + index: vec![], + syntax: SyntaxType::BOOLEAN, + }); s.attributes.insert( // FIXME: Rename to index_attribute? attr_index? String::from("index"), @@ -660,7 +615,6 @@ impl SchemaInner { description: String::from( "Describe the indexes to apply to instances of this attribute.", ), - system: true, secret: false, multivalue: true, index: vec![], @@ -677,7 +631,6 @@ impl SchemaInner { description: String::from( "Describe the syntax of this attribute. This affects indexing and sorting.", ), - system: true, secret: false, multivalue: false, index: vec![IndexType::EQUALITY], @@ -694,7 +647,6 @@ impl SchemaInner { description: String::from( "A list of system provided optional attributes this class can store.", ), - system: true, secret: false, multivalue: true, index: vec![], @@ -711,7 +663,6 @@ impl SchemaInner { description: String::from( "A user modifiable list of optional attributes this class can store.", ), - system: true, secret: false, multivalue: true, index: vec![], @@ -728,7 +679,6 @@ impl SchemaInner { description: String::from( "A list of system provided required attributes this class must store.", ), - system: true, secret: false, multivalue: true, index: vec![], @@ -745,7 +695,6 @@ impl SchemaInner { description: String::from( "A user modifiable list of required attributes this class must store.", ), - system: true, secret: false, multivalue: true, index: vec![], @@ -761,7 +710,6 @@ impl SchemaInner { uuid: Uuid::parse_str(UUID_SCHEMA_ATTR_ACP_ENABLE) .expect("unable to parse static uuid"), description: String::from("A flag to determine if this ACP is active for application. True is enabled, and enforce. False is checked but not enforced."), - system: true, secret: false, multivalue: false, index: vec![IndexType::EQUALITY], @@ -778,7 +726,6 @@ impl SchemaInner { description: String::from( "Who the ACP applies to, constraining or allowing operations.", ), - system: true, secret: false, multivalue: false, index: vec![IndexType::EQUALITY, IndexType::SUBSTRING], @@ -794,7 +741,6 @@ impl SchemaInner { description: String::from( "The effective targets of the ACP, IE what will be acted upon.", ), - system: true, secret: false, multivalue: false, index: vec![IndexType::EQUALITY, IndexType::SUBSTRING], @@ -808,7 +754,6 @@ impl SchemaInner { uuid: Uuid::parse_str(UUID_SCHEMA_ATTR_ACP_SEARCH_ATTR) .expect("unable to parse static uuid"), description: String::from("The attributes that may be viewed or searched by the reciever on targetscope."), - system: true, secret: false, multivalue: true, index: vec![IndexType::EQUALITY], @@ -824,7 +769,6 @@ impl SchemaInner { description: String::from( "The set of classes that can be created on a new entry.", ), - system: true, secret: false, multivalue: true, index: vec![IndexType::EQUALITY], @@ -840,7 +784,6 @@ impl SchemaInner { description: String::from( "The set of attribute types that can be created on an entry.", ), - system: true, secret: false, multivalue: true, index: vec![IndexType::EQUALITY], @@ -855,7 +798,6 @@ impl SchemaInner { uuid: Uuid::parse_str(UUID_SCHEMA_ATTR_ACP_MODIFY_REMOVEDATTR) .expect("unable to parse static uuid"), description: String::from("The set of attribute types that could be removed or purged in a modification."), - system: true, secret: false, multivalue: true, index: vec![IndexType::EQUALITY], @@ -869,7 +811,6 @@ impl SchemaInner { uuid: Uuid::parse_str(UUID_SCHEMA_ATTR_ACP_MODIFY_PRESENTATTR) .expect("unable to parse static uuid"), description: String::from("The set of attribute types that could be added or asserted in a modification."), - system: true, secret: false, multivalue: true, index: vec![IndexType::EQUALITY], @@ -883,7 +824,6 @@ impl SchemaInner { uuid: Uuid::parse_str(UUID_SCHEMA_ATTR_ACP_MODIFY_CLASS) .expect("unable to parse static uuid"), 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."), - system: true, secret: false, multivalue: true, index: vec![IndexType::EQUALITY], @@ -898,7 +838,6 @@ impl SchemaInner { uuid: Uuid::parse_str(UUID_SCHEMA_ATTR_MEMBEROF) .expect("unable to parse static uuid"), description: String::from("reverse group membership of the object"), - system: true, secret: false, multivalue: true, index: vec![IndexType::EQUALITY], @@ -912,7 +851,6 @@ impl SchemaInner { uuid: Uuid::parse_str(UUID_SCHEMA_ATTR_DIRECTMEMBEROF) .expect("unable to parse static uuid"), description: String::from("reverse direct group membership of the object"), - system: true, secret: false, multivalue: true, index: vec![IndexType::EQUALITY], @@ -926,7 +864,6 @@ impl SchemaInner { uuid: Uuid::parse_str(UUID_SCHEMA_ATTR_MEMBER) .expect("unable to parse static uuid"), description: String::from("List of members of the group"), - system: true, secret: false, multivalue: true, index: vec![IndexType::EQUALITY], @@ -943,7 +880,6 @@ impl SchemaInner { description: String::from( "The systems internal migration version for provided objects", ), - system: true, secret: true, multivalue: false, index: vec![IndexType::EQUALITY], @@ -958,7 +894,6 @@ impl SchemaInner { uuid: Uuid::parse_str(UUID_SCHEMA_ATTR_DOMAIN) .expect("unable to parse static uuid"), description: String::from("A DNS Domain name entry."), - system: true, secret: false, multivalue: true, index: vec![IndexType::EQUALITY], @@ -978,7 +913,6 @@ impl SchemaInner { systemmust: vec![ String::from("class"), String::from("name"), - String::from("system"), String::from("secret"), String::from("multivalue"), String::from("syntax"), @@ -1187,6 +1121,19 @@ impl SchemaInner { must: vec![], }, ); + s.classes.insert( + String::from("system"), + SchemaClass { + name: String::from("system"), + uuid: Uuid::parse_str(UUID_SCHEMA_CLASS_SYSTEM) + .expect("unable to parse static uuid"), + description: String::from("A class denoting that a type is system generated and protected. It has special internal behaviour."), + systemmay: vec![], + may: vec![], + systemmust: vec![], + must: vec![], + }, + ); // TODO: Probably needs to do a collect. let r = s.validate(&mut au); @@ -1468,7 +1415,6 @@ mod tests { "class": ["object", "attributetype"], "name": ["schema_attr_test"], "uuid": ["66c68b2f-d02c-4243-8013-7946e40fe321"], - "system": ["false"], "secret": ["false"], "multivalue": ["false"], "index": ["EQUALITY"], @@ -1488,9 +1434,8 @@ mod tests { "name": ["schema_attr_test"], "uuid": ["66c68b2f-d02c-4243-8013-7946e40fe321"], "description": ["Test attr parsing"], - "system": ["thoeunaouhnt"], "secret": ["false"], - "multivalue": ["false"], + "multivalue": ["htouaoeu"], "index": ["EQUALITY"], "syntax": ["UTF8STRING"] } @@ -1508,7 +1453,6 @@ mod tests { "name": ["schema_attr_test"], "uuid": ["66c68b2f-d02c-4243-8013-7946e40fe321"], "description": ["Test attr parsing"], - "system": ["true"], "secret": ["false"], "multivalue": ["false"], "index": ["NTEHNOU"], @@ -1528,7 +1472,6 @@ mod tests { "name": ["schema_attr_test"], "uuid": ["66c68b2f-d02c-4243-8013-7946e40fe321"], "description": ["Test attr parsing"], - "system": ["true"], "secret": ["false"], "multivalue": ["false"], "index": ["EQUALITY"], @@ -1549,7 +1492,6 @@ mod tests { "name": ["schema_attr_test"], "uuid": ["66c68b2f-d02c-4243-8013-7946e40fe321"], "description": ["Test attr parsing"], - "system": ["false"], "secret": ["false"], "multivalue": ["false"], "syntax": ["UTF8STRING"] @@ -1569,7 +1511,6 @@ mod tests { "name": ["schema_attr_test"], "uuid": ["66c68b2f-d02c-4243-8013-7946e40fe321"], "description": ["Test attr parsing"], - "system": ["false"], "secret": ["false"], "multivalue": ["false"], "index": ["EQUALITY"], @@ -1742,7 +1683,6 @@ mod tests { name: String::from("principal_name"), uuid: Uuid::parse_str(UUID_SCHEMA_ATTR_PRINCIPAL_NAME).expect("unable to parse static uuid"), description: String::from("The longform name of an object, derived from name and domain. Example: alice@project.org"), - system: true, secret: false, multivalue: false, index: vec![IndexType::EQUALITY], @@ -1774,7 +1714,6 @@ mod tests { description: String::from( "Who the ACP applies to, constraining or allowing operations.", ), - system: true, secret: false, multivalue: false, index: vec![IndexType::EQUALITY, IndexType::SUBSTRING], @@ -1809,7 +1748,6 @@ mod tests { name: String::from("uuid"), uuid: Uuid::parse_str(UUID_SCHEMA_ATTR_UUID).expect("unable to parse static uuid"), description: String::from("The universal unique id of the object"), - system: true, secret: false, multivalue: false, index: vec![IndexType::EQUALITY], @@ -1831,7 +1769,6 @@ mod tests { name: String::from("single_value"), uuid: Uuid::new_v4(), description: String::from(""), - system: true, secret: false, multivalue: false, index: vec![IndexType::EQUALITY], @@ -1852,7 +1789,6 @@ mod tests { name: String::from("mv_string"), uuid: Uuid::new_v4(), description: String::from(""), - system: true, secret: false, multivalue: true, index: vec![IndexType::EQUALITY], @@ -1868,7 +1804,6 @@ mod tests { name: String::from("mv_bool"), uuid: Uuid::new_v4(), description: String::from(""), - system: true, secret: false, multivalue: true, index: vec![IndexType::EQUALITY], @@ -1889,7 +1824,6 @@ mod tests { name: String::from("sv_syntax"), uuid: Uuid::new_v4(), description: String::from(""), - system: true, secret: false, multivalue: false, index: vec![IndexType::EQUALITY], @@ -1907,7 +1841,6 @@ mod tests { name: String::from("sv_index"), uuid: Uuid::new_v4(), description: String::from(""), - system: true, secret: false, multivalue: false, index: vec![IndexType::EQUALITY], @@ -2018,7 +1951,6 @@ mod tests { "class": ["object", "attributetype"], "name": ["testattr"], "description": ["testattr"], - "system": ["false"], "secret": ["false"], "multivalue": ["false"], "syntax": ["UTF8STRING"], @@ -2042,7 +1974,6 @@ mod tests { "class": ["object", "attributetype"], "name": ["testattr"], "description": ["testattr"], - "system": ["false"], "secret": ["false"], "multivalue": ["zzzzz"], "uuid": ["db237e8a-0079-4b8c-8a56-593b22aa44d1"], @@ -2065,7 +1996,6 @@ mod tests { "class": ["object", "attributetype"], "name": ["testattr"], "description": ["testattr"], - "system": ["false"], "secret": ["false"], "multivalue": ["true"], "uuid": ["db237e8a-0079-4b8c-8a56-593b22aa44d1"], diff --git a/src/lib/server.rs b/src/lib/server.rs index eb6b43e2a..15e98000f 100644 --- a/src/lib/server.rs +++ b/src/lib/server.rs @@ -698,14 +698,14 @@ impl<'a> QueryServerWriteTransaction<'a> { let norm_cand: Vec> = try_audit!(au, res); - /* + // Run any pre-create plugins now with schema validated entries. + // This is important for normalisation of certain types IE class + // or attributes for these checks. let mut audit_plugin_pre = AuditScope::new("plugin_pre_create"); - let plug_pre_res = - Plugins::run_pre_create(&mut audit_plugin_pre, &self, &norm_cand, ce); + let plug_pre_res = Plugins::run_pre_create(&mut audit_plugin_pre, &self, &norm_cand, ce); au.append_scope(audit_plugin_pre); let _ = try_audit!(au, plug_pre_res, "Create operation failed (plugin), {:?}"); - */ let mut audit_be = AuditScope::new("backend_create"); // We may change from ce.entries later to something else? @@ -2445,8 +2445,7 @@ mod tests { "description": ["Test Attribute"], "multivalue": ["false"], "secret": ["false"], - "syntax": ["UTF8STRING"], - "system": ["false"] + "syntax": ["UTF8STRING"] } }"#, )