WIP: serialization and domain info setting wonkiness (#2791)

libs/client/src/lib.rs

@@ -25,9 +25,12 @@ use std::path::Path;
 use std::time::Duration;
 
 use compact_jwt::Jwk;
+
 use kanidm_proto::constants::uri::V1_AUTH_VALID;
 use kanidm_proto::constants::{
-    APPLICATION_JSON, ATTR_ENTRY_MANAGED_BY, ATTR_NAME, CLIENT_TOKEN_CACHE, KOPID, KVERSION,
+    APPLICATION_JSON, ATTR_DOMAIN_DISPLAY_NAME, ATTR_DOMAIN_LDAP_BASEDN, ATTR_DOMAIN_SSID,
+    ATTR_ENTRY_MANAGED_BY, ATTR_KEY_ACTION_REVOKE, ATTR_LDAP_ALLOW_UNIX_PW_BIND, ATTR_NAME,
+    CLIENT_TOKEN_CACHE, KOPID, KVERSION,
 };
 use kanidm_proto::internal::*;
 use kanidm_proto::v1::*;
@@ -71,6 +74,7 @@ pub enum ClientError {
     ConfigParseIssue(String),
     CertParseIssue(String),
     UntrustedCertificate(String),
+    InvalidRequest(String),
 }
 
 /// Settings describing a single instance.
@@ -864,6 +868,10 @@ impl KanidmClient {
 
         match response.status() {
             reqwest::StatusCode::OK => {}
+            reqwest::StatusCode::UNPROCESSABLE_ENTITY => {
+                return Err(ClientError::InvalidRequest(format!("Something about the request content was invalid, check the server logs for further information. Operation ID: {} Error: {:?}",opid, response.text().await.ok() )))
+            }
+
             unexpect => {
                 return Err(ClientError::Http(
                     unexpect,
@@ -1927,16 +1935,16 @@ impl KanidmClient {
         new_display_name: &str,
     ) -> Result<(), ClientError> {
         self.perform_put_request(
-            "/v1/domain/_attr/domain_display_name",
+            &format!("/v1/domain/_attr/{}", ATTR_DOMAIN_DISPLAY_NAME),
-            vec![new_display_name.to_string()],
+            vec![new_display_name],
         )
         .await
     }
 
     pub async fn idm_domain_set_ldap_basedn(&self, new_basedn: &str) -> Result<(), ClientError> {
         self.perform_put_request(
-            "/v1/domain/_attr/domain_ldap_basedn",
+            &format!("/v1/domain/_attr/{}", ATTR_DOMAIN_LDAP_BASEDN),
-            vec![new_basedn.to_string()],
+            vec![new_basedn],
         )
         .await
     }
@@ -1945,12 +1953,15 @@ impl KanidmClient {
         &self,
         enable: bool,
     ) -> Result<(), ClientError> {
-        self.perform_put_request("/v1/domain/_attr/ldap_allow_unix_pw_bind", vec![enable])
+        self.perform_put_request(
-            .await
+            &format!("{}{}", "/v1/domain/_attr/", ATTR_LDAP_ALLOW_UNIX_PW_BIND),
+            vec![enable.to_string()],
+        )
+        .await
     }
 
     pub async fn idm_domain_get_ssid(&self) -> Result<String, ClientError> {
-        self.perform_get_request("/v1/domain/_attr/domain_ssid")
+        self.perform_get_request(&format!("/v1/domain/_attr/{}", ATTR_DOMAIN_SSID))
             .await
             .and_then(|mut r: Vec<String>|
                 // Get the first result
@@ -1961,13 +1972,16 @@ impl KanidmClient {
     }
 
     pub async fn idm_domain_set_ssid(&self, ssid: &str) -> Result<(), ClientError> {
-        self.perform_put_request("/v1/domain/_attr/domain_ssid", vec![ssid.to_string()])
+        self.perform_put_request(
-            .await
+            &format!("/v1/domain/_attr/{}", ATTR_DOMAIN_SSID),
+            vec![ssid.to_string()],
+        )
+        .await
     }
 
     pub async fn idm_domain_revoke_key(&self, key_id: &str) -> Result<(), ClientError> {
         self.perform_put_request(
-            "/v1/domain/_attr/key_action_revoke",
+            &format!("/v1/domain/_attr/{}", ATTR_KEY_ACTION_REVOKE),
             vec![key_id.to_string()],
         )
         .await

server/core/src/config.rs

@@ -454,6 +454,8 @@ impl FromStr for ServerRole {
 pub struct IntegrationTestConfig {
     pub admin_user: String,
     pub admin_password: String,
+    pub idm_admin_user: String,
+    pub idm_admin_password: String,
 }
 
 #[derive(Debug, Clone)]

server/core/src/https/v1.rs

@@ -344,6 +344,15 @@ pub async fn json_rest_event_post_id_attr(
         .map_err(WebError::from)
 }
 
+// Okay, so a put normally needs
+///  * filter of what we are working on (id + class)
+///  * a `Map<String, Vec<String>>` that we turn into a modlist.
+///
+/// OR
+///  * filter of what we are working on (id + class)
+///  * a `Vec<String>` that we are changing
+///  * the attr name  (as a param to this in path)
+///
 pub async fn json_rest_event_put_attr(
     state: ServerState,
     id: String,
@@ -378,27 +387,6 @@ pub async fn json_rest_event_post_attr(
         .map_err(WebError::from)
 }
 
-// Okay, so a put normally needs
-///  * filter of what we are working on (id + class)
-///  * a `Map<String, Vec<String>>` that we turn into a modlist.
-///
-/// OR
-///  * filter of what we are working on (id + class)
-///  * a `Vec<String>` that we are changing
-///  * the attr name  (as a param to this in path)
-///
-pub async fn json_rest_event_put_id_attr(
-    state: ServerState,
-    id: String,
-    attr: String,
-    filter: Filter<FilterInvalid>,
-    values: Vec<String>,
-    kopid: KOpId,
-    client_auth_info: ClientAuthInfo,
-) -> Result<Json<()>, WebError> {
-    json_rest_event_put_attr(state, id, attr, filter, values, kopid, client_auth_info).await
-}
-
 pub async fn json_rest_event_delete_id_attr(
     state: ServerState,
     id: String,
@@ -2304,7 +2292,7 @@ pub async fn group_id_attr_put(
     Json(values): Json<Vec<String>>,
 ) -> Result<Json<()>, WebError> {
     let filter = filter_all!(f_eq(Attribute::Class, EntryClass::Group.into()));
-    json_rest_event_put_id_attr(state, id, attr, filter, values, kopid, client_auth_info).await
+    json_rest_event_put_attr(state, id, attr, filter, values, kopid, client_auth_info).await
 }
 
 #[utoipa::path(
@@ -2426,6 +2414,7 @@ pub async fn domain_attr_put(
     Json(values): Json<Vec<String>>,
 ) -> Result<Json<()>, WebError> {
     let filter = filter_all!(f_eq(Attribute::Class, EntryClass::DomainInfo.into()));
+
     json_rest_event_put_attr(
         state,
         STR_UUID_DOMAIN_INFO.to_string(),

server/core/src/https/v1_scim.rs

@@ -3,7 +3,7 @@ use super::errors::WebError;
 use super::middleware::KOpId;
 use super::v1::{
     json_rest_event_get, json_rest_event_get_id, json_rest_event_get_id_attr, json_rest_event_post,
-    json_rest_event_put_id_attr,
+    json_rest_event_put_attr,
 };
 use super::ServerState;
 use crate::https::extractors::VerifiedClientInformation;
@@ -298,7 +298,7 @@ pub async fn sync_account_id_attr_put(
     Json(values): Json<Vec<String>>,
 ) -> Result<Json<()>, WebError> {
     let filter = filter_all!(f_eq(Attribute::Class, EntryClass::SyncAccount.into()));
-    json_rest_event_put_id_attr(state, id, attr, filter, values, kopid, client_auth_info).await
+    json_rest_event_put_attr(state, id, attr, filter, values, kopid, client_auth_info).await
 }
 
 /// When you want the kitchen Sink

server/core/src/lib.rs

@@ -864,13 +864,26 @@ pub async fn create_server_core(
     match &config.integration_test_config {
         Some(itc) => {
             let mut idms_prox_write = idms.proxy_write(duration_from_epoch_now()).await;
-            // We need to get the admin pw.
+            // We need to set the admin pw.
-            match idms_prox_write.recover_account("admin", Some(&itc.admin_password)) {
+            match idms_prox_write.recover_account(&itc.admin_user, Some(&itc.admin_password)) {
                 Ok(_) => {}
                 Err(e) => {
                     error!(
-                        "Unable to configure INTEGRATION TEST admin account -> {:?}",
+                        "Unable to configure INTEGRATION TEST {} account -> {:?}",
-                        e
+                        &itc.admin_user, e
+                    );
+                    return Err(());
+                }
+            };
+            // set the idm_admin account password
+            match idms_prox_write
+                .recover_account(&itc.idm_admin_user, Some(&itc.idm_admin_password))
+            {
+                Ok(_) => {}
+                Err(e) => {
+                    error!(
+                        "Unable to configure INTEGRATION TEST {} account -> {:?}",
+                        &itc.idm_admin_user, e
                     );
                     return Err(());
                 }

server/lib/src/idm/server.rs

@@ -1604,6 +1604,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
 
         // Deny the change if the account is anonymous!
         if account.is_anonymous() {
+            trace!("Unable to use anonymous to change UNIX account password");
             return Err(OperationError::SystemProtectedObject);
         }
 

server/lib/src/plugins/domain.rs

@@ -84,7 +84,7 @@ impl Domain {
                     .get_ava_single_iutf8(Attribute::DomainLdapBasedn) {
 
                     if !DOMAIN_LDAP_BASEDN_RE.is_match(basedn) {
-                        error!("Invalid {}. Must pass regex \"{}\"", Attribute::DomainLdapBasedn, *DOMAIN_LDAP_BASEDN_RE);
+                        error!("Invalid {} '{}'. Must pass regex \"{}\"", Attribute::DomainLdapBasedn,basedn, *DOMAIN_LDAP_BASEDN_RE);
                         return Err(OperationError::InvalidState);
                     }
                 }

server/lib/src/plugins/gidnumber.rs

@@ -13,7 +13,7 @@ use crate::utils::uuid_to_gid_u32;
 /// system uids from 0 - 1000, and many others give user ids between 1000 to
 /// 2000. This whole numberspace is cursed, lets assume it's not ours. :(
 ///
-/// Per https://systemd.io/UIDS-GIDS/, systemd claims a huge chunk of this
+/// Per <https://systemd.io/UIDS-GIDS/>, systemd claims a huge chunk of this
 /// space to itself. As a result we can't allocate between 65536 and u32 max
 /// because systemd takes most of the usable range for its own containers,
 /// and half the range is probably going to trigger linux kernel issues.

server/lib/src/plugins/protected.rs

@@ -17,30 +17,47 @@ pub struct Protected {}
 
 lazy_static! {
     static ref ALLOWED_ATTRS: HashSet<Attribute> = {
-        let mut m = HashSet::with_capacity(32);
+        let attrs = vec![
         // Allow modification of some schema class types to allow local extension
         // of schema types.
-        //
+        Attribute::Must,
-        m.insert(Attribute::Must);
+        Attribute::May,
-        m.insert(Attribute::May);
+        // modification of some domain info types for local configuratiomn.
-        // Allow modification of some domain info types for local configuration.
+        Attribute::DomainSsid,
-        m.insert(Attribute::DomainSsid);
+        Attribute::DomainLdapBasedn,
-        m.insert(Attribute::DomainLdapBasedn);
+        Attribute::LdapAllowUnixPwBind,
-        m.insert(Attribute::FernetPrivateKeyStr);
+        Attribute::FernetPrivateKeyStr,
-        m.insert(Attribute::Es256PrivateKeyDer);
+        Attribute::Es256PrivateKeyDer,
-        m.insert(Attribute::KeyActionRevoke);
+        Attribute::KeyActionRevoke,
-        m.insert(Attribute::KeyActionRotate);
+        Attribute::KeyActionRotate,
-        m.insert(Attribute::IdVerificationEcKey);
+        Attribute::IdVerificationEcKey,
-        m.insert(Attribute::BadlistPassword);
+        Attribute::BadlistPassword,
-        m.insert(Attribute::DeniedName);
+        Attribute::DeniedName,
-        m.insert(Attribute::DomainDisplayName);
+        Attribute::DomainDisplayName,
-        // Allow modification of account policy values for dyngroups
+        // modification of account policy values for dyngroup.
-        m.insert(Attribute::AuthSessionExpiry);
+        Attribute::AuthSessionExpiry,
-        m.insert(Attribute::PrivilegeExpiry);
+        Attribute::PrivilegeExpiry,
-        m.insert(Attribute::CredentialTypeMinimum);
+        Attribute::CredentialTypeMinimum,
-        m.insert(Attribute::WebauthnAttestationCaList);
+        Attribute::WebauthnAttestationCaList,
+        ];
+
+        let mut m = HashSet::with_capacity(attrs.len());
+        m.extend(attrs);
+
         m
     };
+
+    static ref PROTECTED_ENTRYCLASSES: Vec<EntryClass> =
+        vec![
+            EntryClass::System,
+            EntryClass::DomainInfo,
+            EntryClass::SystemInfo,
+            EntryClass::SystemConfig,
+            EntryClass::DynGroup,
+            EntryClass::SyncObject,
+            EntryClass::Tombstone,
+            EntryClass::Recycled,
+        ];
 }
 
 impl Plugin for Protected {
@@ -61,14 +78,11 @@ impl Plugin for Protected {
         }
 
         cand.iter().try_fold((), |(), cand| {
-            if cand.attribute_equality(Attribute::Class, &EntryClass::System.into())
+            if PROTECTED_ENTRYCLASSES
-                || cand.attribute_equality(Attribute::Class, &EntryClass::DomainInfo.into())
+                .iter()
-                || cand.attribute_equality(Attribute::Class, &EntryClass::SystemInfo.into())
+                .any(|c| cand.attribute_equality(Attribute::Class, &c.to_partialvalue()))
-                || cand.attribute_equality(Attribute::Class, &EntryClass::SystemConfig.into())
-                || cand.attribute_equality(Attribute::Class, &EntryClass::Tombstone.into())
-                || cand.attribute_equality(Attribute::Class, &EntryClass::Recycled.into())
-                || cand.attribute_equality(Attribute::Class, &EntryClass::DynGroup.into())
             {
+                trace!("Rejecting operation during pre_create check");
                 Err(OperationError::SystemProtectedObject)
             } else {
                 Ok(())
@@ -91,15 +105,9 @@ impl Plugin for Protected {
         me.modlist.iter().try_fold((), |(), m| match m {
             Modify::Present(a, v) => {
                 if a == Attribute::Class.as_ref()
-                    && (v == &EntryClass::System.to_value()
+                    && PROTECTED_ENTRYCLASSES.iter().any(|c| v == &c.to_value())
-                        || v == &EntryClass::DomainInfo.to_value()
-                        || v == &EntryClass::SystemInfo.into()
-                        || v == &EntryClass::SystemConfig.to_value()
-                        || v == &EntryClass::DynGroup.to_value()
-                        || v == &EntryClass::SyncObject.to_value()
-                        || v == &EntryClass::Tombstone.to_value()
-                        || v == &EntryClass::Recycled.to_value())
                 {
+                    trace!("Rejecting operation during pre_modify check");
                     Err(OperationError::SystemProtectedObject)
                 } else {
                     Ok(())
@@ -144,7 +152,10 @@ impl Plugin for Protected {
                 let attr: Attribute = a.try_into()?;
                 match ALLOWED_ATTRS.contains(&attr) {
                     true => Ok(()),
-                    false => Err(OperationError::SystemProtectedObject),
+                    false => {
+                        trace!("If you're getting this, you need to modify the ALLOWED_ATTRS list");
+                        Err(OperationError::SystemProtectedObject)
+                    }
                 }
             } else {
                 // Was not a mod needing checking
@@ -171,15 +182,9 @@ impl Plugin for Protected {
             .try_fold((), |(), m| match m {
                 Modify::Present(a, v) => {
                     if a == Attribute::Class.as_ref()
-                        && (v == &EntryClass::System.to_value()
+                        && PROTECTED_ENTRYCLASSES.iter().any(|c| v == &c.to_value())
-                            || v == &EntryClass::DomainInfo.to_value()
-                            || v == &EntryClass::SystemInfo.to_value()
-                            || v == &EntryClass::SystemConfig.to_value()
-                            || v == &EntryClass::DynGroup.to_value()
-                            || v == &EntryClass::SyncObject.to_value()
-                            || v == &EntryClass::Tombstone.to_value()
-                            || v == &EntryClass::Recycled.to_value())
                     {
+                        trace!("Rejecting operation during pre_batch_modify check");
                         Err(OperationError::SystemProtectedObject)
                     } else {
                         Ok(())
@@ -227,7 +232,11 @@ impl Plugin for Protected {
                     let attr: Attribute = a.try_into()?;
                     match ALLOWED_ATTRS.contains(&attr) {
                         true => Ok(()),
-                        false => Err(OperationError::SystemProtectedObject),
+                        false => {
+
+                        trace!("Rejecting operation during pre_batch_modify check, if you're getting this check ALLOWED_ATTRS");
+                            Err(OperationError::SystemProtectedObject)
+                        },
                     }
                 } else {
                     // Was not a mod needing checking
@@ -249,14 +258,11 @@ impl Plugin for Protected {
         }
 
         cand.iter().try_fold((), |(), cand| {
-            if cand.attribute_equality(Attribute::Class, &EntryClass::System.into())
+            if PROTECTED_ENTRYCLASSES
-                || cand.attribute_equality(Attribute::Class, &EntryClass::DomainInfo.into())
+                .iter()
-                || cand.attribute_equality(Attribute::Class, &EntryClass::SystemInfo.into())
+                .any(|c| cand.attribute_equality(Attribute::Class, &c.to_partialvalue()))
-                || cand.attribute_equality(Attribute::Class, &EntryClass::SystemConfig.into())
-                || cand.attribute_equality(Attribute::Class, &EntryClass::Tombstone.into())
-                || cand.attribute_equality(Attribute::Class, &EntryClass::Recycled.into())
-                || cand.attribute_equality(Attribute::Class, &EntryClass::DynGroup.into())
             {
+                trace!("Rejecting operation during pre_delete check");
                 Err(OperationError::SystemProtectedObject)
             } else {
                 Ok(())

server/lib/src/server/migrations.rs

@@ -193,7 +193,7 @@ impl<'a> QueryServerWriteTransaction<'a> {
         self.internal_migrate_or_create_ignore_attrs(e, &[])
     }
 
-    /// This is the same as [internal_migrate_or_create] but it will ignore the specified
+    /// This is the same as [QueryServerWriteTransaction::internal_migrate_or_create] but it will ignore the specified
     /// list of attributes, so that if an admin has modified those values then we don't
     /// stomp them.
     #[instrument(level = "trace", skip_all)]

server/testkit/src/lib.rs

@@ -62,6 +62,8 @@ pub async fn setup_async_test(mut config: Configuration) -> (KanidmClient, CoreH
     let int_config = Box::new(IntegrationTestConfig {
         admin_user: ADMIN_TEST_USER.to_string(),
         admin_password: ADMIN_TEST_PASSWORD.to_string(),
+        idm_admin_user: IDM_ADMIN_TEST_USER.to_string(),
+        idm_admin_password: IDM_ADMIN_TEST_PASSWORD.to_string(),
     });
 
     let addr = format!("http://localhost:{}", port);

server/testkit/tests/domain.rs

@@ -0,0 +1,52 @@
+use kanidm_client::KanidmClient;
+use kanidm_proto::constants::ATTR_DOMAIN_DISPLAY_NAME;
+use kanidmd_testkit::{ADMIN_TEST_PASSWORD, ADMIN_TEST_USER};
+
+#[kanidmd_testkit::test]
+async fn test_idm_set_ldap_allow_unix_password_bind(rsclient: KanidmClient) {
+    rsclient
+        .auth_simple_password(ADMIN_TEST_USER, ADMIN_TEST_PASSWORD)
+        .await
+        .expect("Failed to login as admin");
+    rsclient
+        .idm_set_ldap_allow_unix_password_bind(true)
+        .await
+        .expect("Failed to set LDAP allow unix password bind to true");
+}
+#[kanidmd_testkit::test]
+async fn test_idm_domain_set_ldap_basedn(rsclient: KanidmClient) {
+    rsclient
+        .auth_simple_password(ADMIN_TEST_USER, ADMIN_TEST_PASSWORD)
+        .await
+        .expect("Failed to login as admin");
+
+    rsclient
+        .idm_domain_set_ldap_basedn("dc=example,dc=com")
+        .await
+        .expect("Failed to set idm_domain_set_ldap_basedn");
+}
+
+#[kanidmd_testkit::test]
+async fn test_idm_domain_set_display_name(rsclient: KanidmClient) {
+    rsclient
+        .auth_simple_password(ADMIN_TEST_USER, ADMIN_TEST_PASSWORD)
+        .await
+        .expect("Failed to login as admin");
+
+    let new_domain_display_name = "hello kanidm 12345667";
+
+    rsclient
+        .idm_domain_set_display_name(new_domain_display_name)
+        .await
+        .expect("Failed to set idm_domain_set_display_name");
+
+    let domain_after = rsclient
+        .idm_domain_get()
+        .await
+        .expect("Failed to idm_domain_get");
+
+    assert_eq!(
+        domain_after.attrs.get(ATTR_DOMAIN_DISPLAY_NAME),
+        Some(&vec![new_domain_display_name.to_string()])
+    );
+}

unix_integration/tests/cache_layer_test.rs

@@ -24,6 +24,8 @@ use kanidm_hsm_crypto::{soft::SoftTpm, AuthValue, BoxedDynTpm, Tpm};
 
 const ADMIN_TEST_USER: &str = "admin";
 const ADMIN_TEST_PASSWORD: &str = "integration test admin password";
+const IDM_ADMIN_TEST_USER: &str = "idm_admin";
+const IDM_ADMIN_TEST_PASSWORD: &str = "integration test idm_admin password";
 const TESTACCOUNT1_PASSWORD_A: &str = "password a for account1 test";
 const TESTACCOUNT1_PASSWORD_B: &str = "password b for account1 test";
 const TESTACCOUNT1_PASSWORD_INC: &str = "never going to work";
@@ -58,6 +60,8 @@ async fn setup_test(fix_fn: Fixture) -> (Resolver<KanidmProvider>, KanidmClient)
     let int_config = Box::new(IntegrationTestConfig {
         admin_user: ADMIN_TEST_USER.to_string(),
         admin_password: ADMIN_TEST_PASSWORD.to_string(),
+        idm_admin_user: IDM_ADMIN_TEST_USER.to_string(),
+        idm_admin_password: IDM_ADMIN_TEST_PASSWORD.to_string(),
     });
 
     // Setup the config ...