Adding a builtin class for all built-in things (#2603)

* adding builtin class to builtin objects
* Resolve issues with builtin PR

---------

Co-authored-by: William Brown <william@blackhats.net.au>

book/src/monitoring.md

@@ -22,6 +22,7 @@ Configure OTLP trace exports by setting a `otel_grpc_url` in the server configur
 enable [OpenTelemetry traces](https://opentelemetry.io) to be sent for observability use cases.
 
 Example:
+
 ```toml
 otel_grpc_url = "http://my-otel-host:4317"
 ```

proto/src/constants.rs

@@ -228,4 +228,8 @@ pub const KOPID: &str = "X-KANIDM-OPID";
 /// HTTP Header containing the Kanidm server version
 pub const KVERSION: &str = "X-KANIDM-VERSION";
 
+/// X-Forwarded-For header
 pub const X_FORWARDED_FOR: &str = "x-forwarded-for";
+
+/// Builtin object
+pub const ENTRYCLASS_BUILTIN: &str = "builtin";

server/lib-macros/src/entry.rs

@@ -205,7 +205,7 @@ pub(crate) fn qs_pair_test(args: &TokenStream, item: TokenStream) -> TokenStream
     };
 
     // Setup the config filling the remaining fields with the default values
-    let (default_config_struct, _flags) = match parse_attributes(&args, &input) {
+    let (default_config_struct, _flags) = match parse_attributes(args, &input) {
         Ok(dc) => dc,
         Err(e) => return token_stream_with_error(args.clone(), e),
     };
@@ -286,7 +286,7 @@ pub(crate) fn idm_test(args: &TokenStream, item: TokenStream) -> TokenStream {
     };
 
     // Setup the config filling the remaining fields with the default values
-    let (default_config_struct, flags) = match parse_attributes(&args, &input) {
+    let (default_config_struct, flags) = match parse_attributes(args, &input) {
         Ok(dc) => dc,
         Err(e) => return token_stream_with_error(args.clone(), e),
     };

server/lib/benches/scaling_10k.rs

@@ -7,6 +7,7 @@ use criterion::{
 use kanidmd_lib::entry::{Entry, EntryInit, EntryNew};
 use kanidmd_lib::entry_init;
 use kanidmd_lib::prelude::{Attribute, EntryClass};
+use kanidmd_lib::testkit::{setup_idm_test, TestConfiguration};
 use kanidmd_lib::value::Value;
 
 pub fn duration_from_epoch_now() -> Duration {
@@ -38,7 +39,7 @@ pub fn scaling_user_create_single(c: &mut Criterion) {
                         .expect("Failed building the Runtime")
                         .block_on(async {
                             let (idms, _idms_delayed, _idms_audit) =
-                                kanidmd_lib::testkit::setup_idm_test().await;
+                                setup_idm_test(TestConfiguration::default()).await;
 
                             let ct = duration_from_epoch_now();
                             let start = Instant::now();
@@ -105,7 +106,7 @@ pub fn scaling_user_create_batched(c: &mut Criterion) {
                         .expect("Failed building the Runtime")
                         .block_on(async {
                             let (idms, _idms_delayed, _idms_audit) =
-                                kanidmd_lib::testkit::setup_idm_test().await;
+                                setup_idm_test(TestConfiguration::default()).await;
 
                             let ct = duration_from_epoch_now();
                             let start = Instant::now();

server/lib/src/constants/acp.rs

@@ -77,6 +77,7 @@ pub struct BuiltinAcp {
 }
 
 impl From<BuiltinAcp> for EntryInitNew {
+    #[allow(clippy::panic)]
     fn from(value: BuiltinAcp) -> Self {
         let mut entry = EntryInitNew::default();
 
@@ -94,6 +95,11 @@ impl From<BuiltinAcp> for EntryInitNew {
         });
 
         entry.set_ava(Attribute::Name, [Value::new_iname(value.name)]);
+
+        if value.uuid >= DYNAMIC_RANGE_MINIMUM_UUID {
+            panic!("Builtin ACP has invalid UUID! {:?}", value);
+        }
+
         entry.set_ava(Attribute::Uuid, [Value::Uuid(value.uuid)]);
         entry.set_ava(
             Attribute::Description,

server/lib/src/constants/entries.rs

@@ -600,6 +600,7 @@ pub enum EntryClass {
     Account,
     AccountPolicy,
     AttributeType,
+    Builtin,
     Class,
     ClassType,
     Conflict,
@@ -646,6 +647,7 @@ impl From<EntryClass> for &'static str {
             EntryClass::Account => "account",
             EntryClass::AccountPolicy => "account_policy",
             EntryClass::AttributeType => "attributetype",
+            EntryClass::Builtin => ENTRYCLASS_BUILTIN,
             EntryClass::Class => ATTR_CLASS,
             EntryClass::ClassType => "classtype",
             EntryClass::Conflict => "conflict",
@@ -801,6 +803,10 @@ impl Default for BuiltinAccount {
 
 impl From<BuiltinAccount> for Account {
     fn from(value: BuiltinAccount) -> Self {
+        #[allow(clippy::panic)]
+        if value.uuid >= DYNAMIC_RANGE_MINIMUM_UUID {
+            panic!("Builtin ACP has invalid UUID! {:?}", value);
+        }
         Account {
             name: value.name.to_string(),
             uuid: value.uuid,
@@ -817,6 +823,10 @@ impl From<BuiltinAccount> for EntryInitNew {
     fn from(value: BuiltinAccount) -> Self {
         let mut entry = EntryInitNew::new();
         entry.add_ava(Attribute::Name, Value::new_iname(value.name));
+        #[allow(clippy::panic)]
+        if value.uuid >= DYNAMIC_RANGE_MINIMUM_UUID {
+            panic!("Builtin ACP has invalid UUID! {:?}", value);
+        }
         entry.add_ava(Attribute::Uuid, Value::Uuid(value.uuid));
         entry.add_ava(Attribute::Description, Value::new_utf8s(value.description));
         entry.add_ava(Attribute::DisplayName, Value::new_utf8s(value.displayname));
@@ -912,6 +922,11 @@ lazy_static! {
     );
 }
 
+// ⚠️  DOMAIN LEVEL 1 ENTRIES ⚠️
+// Future entries need to be added via migrations.
+//
+// DO NOT MODIFY THIS DEFINITION
+
 /// Build a list of internal admin entries
 pub fn idm_builtin_admin_entries() -> Result<Vec<EntryInitNew>, OperationError> {
     let mut res: Vec<EntryInitNew> = vec![

server/lib/src/constants/groups.rs

@@ -22,6 +22,11 @@ impl TryFrom<BuiltinGroup> for EntryInitNew {
     fn try_from(val: BuiltinGroup) -> Result<Self, OperationError> {
         let mut entry = EntryInitNew::new();
 
+        if val.uuid >= DYNAMIC_RANGE_MINIMUM_UUID {
+            error!("Builtin ACP has invalid UUID! {:?}", val);
+            return Err(OperationError::InvalidUuid);
+        }
+
         entry.add_ava(Attribute::Name, Value::new_iname(val.name));
         entry.add_ava(Attribute::Description, Value::new_utf8s(val.description));
         // classes for groups

server/lib/src/constants/uuids.rs

@@ -280,6 +280,7 @@ pub const UUID_SCHEMA_ATTR_LIMIT_SEARCH_MAX_RESULTS: Uuid =
     uuid!("00000000-0000-0000-0000-ffff00000161");
 pub const UUID_SCHEMA_ATTR_LIMIT_SEARCH_MAX_FILTER_TEST: Uuid =
     uuid!("00000000-0000-0000-0000-ffff00000162");
+pub const UUID_SCHEMA_CLASS_BUILTIN: Uuid = uuid!("00000000-0000-0000-0000-ffff00000163");
 
 // System and domain infos
 // I'd like to strongly criticise william of the past for making poor choices about these allocations.
@@ -396,3 +397,5 @@ pub const UUID_IDM_ACP_ACCOUNT_UNIX_EXTEND_V1: Uuid = uuid!("00000000-0000-0000-
 // End of system ranges
 pub const UUID_DOES_NOT_EXIST: Uuid = uuid!("00000000-0000-0000-0000-fffffffffffe");
 pub const UUID_ANONYMOUS: Uuid = uuid!("00000000-0000-0000-0000-ffffffffffff");
+
+pub const DYNAMIC_RANGE_MINIMUM_UUID: Uuid = uuid!("00000000-0000-0000-0001-000000000000");

server/lib/src/plugins/base.rs

@@ -67,15 +67,34 @@ impl Plugin for Base {
         // Now, every cand has a UUID - create a cand uuid set from it.
         let mut cand_uuid: BTreeSet<Uuid> = BTreeSet::new();
 
+        let mut system_range_invalid = false;
+
         // As we insert into the set, if a duplicate is found, return an error
         // that a duplicate exists.
         //
         // Remember, we have to use the ava here, not the get_uuid types because
         // we may not have filled in the uuid field yet.
-        for entry in cand.iter() {
+        for entry in cand.iter_mut() {
             let uuid_ref: Uuid = entry
                 .get_ava_single_uuid(Attribute::Uuid)
                 .ok_or_else(|| OperationError::InvalidAttribute(Attribute::Uuid.to_string()))?;
+
+            // Check that the system-protected range is not in the cand_uuid, unless we are
+            // an internal operation.
+            if uuid_ref < DYNAMIC_RANGE_MINIMUM_UUID {
+                if ce.ident.is_internal() {
+                    // it's a builtin entry, lets add the class.
+                    entry.add_ava(Attribute::Class, EntryClass::Builtin.to_value());
+                } else {
+                    // Don't do that!
+                    error!(
+                        "uuid from protected system UUID range found in create set! {:?}",
+                        uuid_ref
+                    );
+                    system_range_invalid = true;
+                }
+            };
+
             if !cand_uuid.insert(uuid_ref) {
                 trace!("uuid duplicate found in create set! {:?}", uuid_ref);
                 return Err(OperationError::Plugin(PluginError::Base(
@@ -84,33 +103,18 @@ impl Plugin for Base {
             }
         }
 
-        // Check that the system-protected range is not in the cand_uuid, unless we are
-        // an internal operation.
-        if !ce.ident.is_internal() {
-            // TODO: We can't lazy const this as you can't borrow the type down to what
-            // range and contains on btreeset need, but can we possibly make these
-            // part of the struct at init. rather than needing to parse a lot?
-            // The internal set is bounded by: UUID_ADMIN -> UUID_ANONYMOUS
-            // Sadly we need to allocate these to strings to make references, sigh.
-            let overlap: usize = cand_uuid.range(UUID_ADMIN..UUID_ANONYMOUS).count();
-            if overlap != 0 {
-                admin_error!(
-                    "uuid from protected system UUID range found in create set! {:?}",
-                    overlap
-                );
+        if system_range_invalid {
             return Err(OperationError::Plugin(PluginError::Base(
                 "Uuid must not be in protected range".to_string(),
             )));
         }
-        }
 
         if cand_uuid.contains(&UUID_DOES_NOT_EXIST) {
-            admin_error!(
-                "uuid \"does not exist\" found in create set! {:?}",
-                UUID_DOES_NOT_EXIST
+            error!(
+                "uuid \"does not exist\" found in create set! THIS IS A BUG. PLEASE REPORT IT IMMEDIATELY."
             );
             return Err(OperationError::Plugin(PluginError::Base(
-                "UUID_DOES_NOT_EXIST may not exist!".to_string(),
+                "Attempt to create UUID_DOES_NOT_EXIST".to_string(),
             )));
         }
 

server/lib/src/schema.rs

@@ -1962,6 +1962,15 @@ impl<'a> SchemaWriteTransaction<'a> {
                 ..Default::default()
             },
         );
+        self.classes.insert(
+            EntryClass::Builtin.into(),
+            SchemaClass {
+                name: EntryClass::Builtin.into(),
+                uuid: UUID_SCHEMA_CLASS_BUILTIN,
+                description: String::from("A marker class denoting builtin entries"),
+                ..Default::default()
+            },
+        );
         self.classes.insert(
             EntryClass::MemberOf.into(),
             SchemaClass {

server/lib/src/server/migrations.rs

@@ -888,6 +888,7 @@ impl<'a> QueryServerWriteTransaction<'a> {
             // Update anonymous with the correct entry manager,
             BUILTIN_ACCOUNT_ANONYMOUS_DL6.clone().into(),
         ];
+        self.reload()?;
 
         idm_access_controls
             .into_iter()
@@ -897,6 +898,15 @@ impl<'a> QueryServerWriteTransaction<'a> {
                 err
             })?;
 
+        // all the built-in objects get a builtin class
+        let filter = f_lt(
+            Attribute::Uuid,
+            PartialValue::Uuid(DYNAMIC_RANGE_MINIMUM_UUID),
+        );
+        let modlist = modlist!([m_pres(Attribute::Class, &EntryClass::Builtin.into())]);
+
+        self.internal_modify(&filter!(filter), &modlist)?;
+
         Ok(())
     }
 
@@ -927,6 +937,10 @@ impl<'a> QueryServerWriteTransaction<'a> {
     pub fn initialise_schema_idm(&mut self) -> Result<(), OperationError> {
         admin_debug!("initialise_schema_idm -> start ...");
 
+        // ⚠️  DOMAIN LEVEL 1 SCHEMA ATTRIBUTES ⚠️
+        // Future schema attributes need to be added via migrations.
+        //
+        // DO NOT MODIFY THIS DEFINITION
         let idm_schema_attrs = [
             SCHEMA_ATTR_SYNC_CREDENTIAL_PORTAL.clone().into(),
             SCHEMA_ATTR_SYNC_YIELD_AUTHORITY.clone().into(),
@@ -941,7 +955,10 @@ impl<'a> QueryServerWriteTransaction<'a> {
         }
         debug_assert!(r.is_ok());
 
-        // List of IDM schemas to init.
+        // ⚠️  DOMAIN LEVEL 1 SCHEMA ATTRIBUTES ⚠️
+        // Future schema classes need to be added via migrations.
+        //
+        // DO NOT MODIFY THIS DEFINITION
         let idm_schema: Vec<EntryInitNew> = vec![
             SCHEMA_ATTR_MAIL.clone().into(),
             SCHEMA_ATTR_ACCOUNT_EXPIRE.clone().into(),
@@ -1013,7 +1030,11 @@ impl<'a> QueryServerWriteTransaction<'a> {
 
         debug_assert!(r.is_ok());
 
-        let idm_schema_classes: Vec<EntryInitNew> = vec![
+        // ⚠️  DOMAIN LEVEL 1 SCHEMA CLASSES ⚠️
+        // Future schema classes need to be added via migrations.
+        //
+        // DO NOT MODIFY THIS DEFINITION
+        let idm_schema_classes_dl1: Vec<EntryInitNew> = vec![
             SCHEMA_CLASS_ACCOUNT.clone().into(),
             SCHEMA_CLASS_ACCOUNT_POLICY.clone().into(),
             SCHEMA_CLASS_DOMAIN_INFO.clone().into(),
@@ -1031,7 +1052,7 @@ impl<'a> QueryServerWriteTransaction<'a> {
             SCHEMA_CLASS_OAUTH2_RS_PUBLIC.clone().into(),
         ];
 
-        let r: Result<(), _> = idm_schema_classes
+        let r: Result<(), _> = idm_schema_classes_dl1
             .into_iter()
             .try_for_each(|entry| self.internal_migrate_or_create(entry));
 
@@ -1096,6 +1117,10 @@ impl<'a> QueryServerWriteTransaction<'a> {
         debug_assert!(res.is_ok());
         res?;
 
+        // ⚠️  DOMAIN LEVEL 1 ENTRIES ⚠️
+        // Future entries need to be added via migrations.
+        //
+        // DO NOT MODIFY THIS DEFINITION
         let idm_entries: Vec<BuiltinAcp> = vec![
             // Built in access controls.
             IDM_ACP_RECYCLE_BIN_SEARCH_V1.clone(),

server/lib/src/value.rs

@@ -999,7 +999,7 @@ pub enum OauthClaimMapJoin {
 }
 
 impl OauthClaimMapJoin {
-    pub(crate) fn to_char(&self) -> char {
+    pub(crate) fn to_char(self) -> char {
         match self {
             OauthClaimMapJoin::CommaSeparatedValue => ',',
             OauthClaimMapJoin::SpaceSeparatedValue => ' ',

server/lib/src/valueset/uuid.rs

@@ -89,8 +89,11 @@ impl ValueSetT for ValueSetUuid {
         false
     }
 
-    fn lessthan(&self, _pv: &PartialValue) -> bool {
-        false
+    fn lessthan(&self, pv: &PartialValue) -> bool {
+        match pv {
+            PartialValue::Uuid(u) => self.set.iter().any(|v| v < u),
+            _ => false,
+        }
     }
 
     fn len(&self) -> usize {
@@ -257,8 +260,11 @@ impl ValueSetT for ValueSetRefer {
         false
     }
 
-    fn lessthan(&self, _pv: &PartialValue) -> bool {
-        false
+    fn lessthan(&self, pv: &PartialValue) -> bool {
+        match pv {
+            PartialValue::Refer(u) => self.set.iter().any(|v| v < u),
+            _ => false,
+        }
     }
 
     fn len(&self) -> usize {

server/testkit/tests/integration.rs

@@ -5,6 +5,7 @@
 // use tempfile::tempdir;
 
 use kanidm_client::KanidmClient;
+use kanidmd_lib::constants::EntryClass;
 use kanidmd_testkit::login_put_admin_idm_admins;
 // use testkit_macros::cli_kanidm;
 
@@ -96,7 +97,7 @@ async fn test_webdriver_user_login(rsclient: kanidm_client::KanidmClient) {
 
     println!("Waiting for page to load");
     let mut wait_attempts = 0;
-    while wait_attempts < 10 {
+    loop {
         tokio::time::sleep(tokio::time::Duration::from_micros(200)).await;
         c.wait();
 
@@ -225,6 +226,24 @@ async fn test_idm_domain_set_ldap_basedn(rsclient: KanidmClient) {
         .is_err());
 }
 
+#[kanidmd_testkit::test]
+/// Checks that a built-in group idm_all_persons has the "builtin" class as expected.
+async fn test_all_persons_has_builtin_class(rsclient: KanidmClient) {
+    login_put_admin_idm_admins(&rsclient).await;
+    let res = rsclient
+        .idm_group_get("idm_all_persons")
+        .await
+        .expect("Failed to get idm_all_persons");
+    eprintln!("res: {:?}", res);
+
+    assert!(res
+        .unwrap()
+        .attrs
+        .get("class")
+        .unwrap()
+        .contains(&EntryClass::Builtin.as_ref().into()));
+}
+
 // /// run a test command as the admin user
 // fn test_cmd_admin(token_cache_path: &str, rsclient: &KanidmClient, cmd: &str) -> Output {
 //     let split_cmd: Vec<&str> = cmd.split_ascii_whitespace().collect();