diff --git a/libs/client/src/lib.rs b/libs/client/src/lib.rs index b4a2fb71b..b18d09db8 100644 --- a/libs/client/src/lib.rs +++ b/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", - vec![new_display_name.to_string()], + &format!("/v1/domain/_attr/{}", ATTR_DOMAIN_DISPLAY_NAME), + 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", - vec![new_basedn.to_string()], + &format!("/v1/domain/_attr/{}", ATTR_DOMAIN_LDAP_BASEDN), + 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]) - .await + self.perform_put_request( + &format!("{}{}", "/v1/domain/_attr/", ATTR_LDAP_ALLOW_UNIX_PW_BIND), + vec![enable.to_string()], + ) + .await } pub async fn idm_domain_get_ssid(&self) -> Result { - 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| // 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()]) - .await + self.perform_put_request( + &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 diff --git a/server/core/src/config.rs b/server/core/src/config.rs index 030526c9f..f5322f8c0 100644 --- a/server/core/src/config.rs +++ b/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)] diff --git a/server/core/src/https/v1.rs b/server/core/src/https/v1.rs index d0f268649..c94bee6cf 100644 --- a/server/core/src/https/v1.rs +++ b/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>` that we turn into a modlist. +/// +/// OR +/// * filter of what we are working on (id + class) +/// * a `Vec` 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>` that we turn into a modlist. -/// -/// OR -/// * filter of what we are working on (id + class) -/// * a `Vec` 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, - values: Vec, - kopid: KOpId, - client_auth_info: ClientAuthInfo, -) -> Result, 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>, ) -> Result, 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>, ) -> Result, WebError> { let filter = filter_all!(f_eq(Attribute::Class, EntryClass::DomainInfo.into())); + json_rest_event_put_attr( state, STR_UUID_DOMAIN_INFO.to_string(), diff --git a/server/core/src/https/v1_scim.rs b/server/core/src/https/v1_scim.rs index 2b932aa5b..10389168b 100644 --- a/server/core/src/https/v1_scim.rs +++ b/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>, ) -> Result, 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 diff --git a/server/core/src/lib.rs b/server/core/src/lib.rs index f9b1df24b..80faa43c9 100644 --- a/server/core/src/lib.rs +++ b/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. - match idms_prox_write.recover_account("admin", Some(&itc.admin_password)) { + // We need to set the admin pw. + match idms_prox_write.recover_account(&itc.admin_user, Some(&itc.admin_password)) { Ok(_) => {} Err(e) => { error!( - "Unable to configure INTEGRATION TEST admin account -> {:?}", - e + "Unable to configure INTEGRATION TEST {} account -> {:?}", + &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(()); } diff --git a/server/lib/src/idm/server.rs b/server/lib/src/idm/server.rs index 232743c88..8219145df 100644 --- a/server/lib/src/idm/server.rs +++ b/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); } diff --git a/server/lib/src/plugins/domain.rs b/server/lib/src/plugins/domain.rs index c27074d87..3cfda35ac 100644 --- a/server/lib/src/plugins/domain.rs +++ b/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); } } diff --git a/server/lib/src/plugins/gidnumber.rs b/server/lib/src/plugins/gidnumber.rs index fb8b91890..afc8caf97 100644 --- a/server/lib/src/plugins/gidnumber.rs +++ b/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 , 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. diff --git a/server/lib/src/plugins/protected.rs b/server/lib/src/plugins/protected.rs index 31872eb18..d2ab18ec9 100644 --- a/server/lib/src/plugins/protected.rs +++ b/server/lib/src/plugins/protected.rs @@ -17,30 +17,47 @@ pub struct Protected {} lazy_static! { static ref ALLOWED_ATTRS: HashSet = { - let mut m = HashSet::with_capacity(32); + let attrs = vec![ // Allow modification of some schema class types to allow local extension // of schema types. - // - m.insert(Attribute::Must); - m.insert(Attribute::May); - // Allow modification of some domain info types for local configuration. - m.insert(Attribute::DomainSsid); - m.insert(Attribute::DomainLdapBasedn); - m.insert(Attribute::FernetPrivateKeyStr); - m.insert(Attribute::Es256PrivateKeyDer); - m.insert(Attribute::KeyActionRevoke); - m.insert(Attribute::KeyActionRotate); - m.insert(Attribute::IdVerificationEcKey); - m.insert(Attribute::BadlistPassword); - m.insert(Attribute::DeniedName); - m.insert(Attribute::DomainDisplayName); - // Allow modification of account policy values for dyngroups - m.insert(Attribute::AuthSessionExpiry); - m.insert(Attribute::PrivilegeExpiry); - m.insert(Attribute::CredentialTypeMinimum); - m.insert(Attribute::WebauthnAttestationCaList); + Attribute::Must, + Attribute::May, + // modification of some domain info types for local configuratiomn. + Attribute::DomainSsid, + Attribute::DomainLdapBasedn, + Attribute::LdapAllowUnixPwBind, + Attribute::FernetPrivateKeyStr, + Attribute::Es256PrivateKeyDer, + Attribute::KeyActionRevoke, + Attribute::KeyActionRotate, + Attribute::IdVerificationEcKey, + Attribute::BadlistPassword, + Attribute::DeniedName, + Attribute::DomainDisplayName, + // modification of account policy values for dyngroup. + Attribute::AuthSessionExpiry, + Attribute::PrivilegeExpiry, + Attribute::CredentialTypeMinimum, + Attribute::WebauthnAttestationCaList, + ]; + + let mut m = HashSet::with_capacity(attrs.len()); + m.extend(attrs); + m }; + + static ref PROTECTED_ENTRYCLASSES: Vec = + 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()) - || cand.attribute_equality(Attribute::Class, &EntryClass::DomainInfo.into()) - || cand.attribute_equality(Attribute::Class, &EntryClass::SystemInfo.into()) - || 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()) + if PROTECTED_ENTRYCLASSES + .iter() + .any(|c| cand.attribute_equality(Attribute::Class, &c.to_partialvalue())) { + 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() - || 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()) + && PROTECTED_ENTRYCLASSES.iter().any(|c| v == &c.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() - || 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()) + && PROTECTED_ENTRYCLASSES.iter().any(|c| v == &c.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()) - || cand.attribute_equality(Attribute::Class, &EntryClass::DomainInfo.into()) - || cand.attribute_equality(Attribute::Class, &EntryClass::SystemInfo.into()) - || 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()) + if PROTECTED_ENTRYCLASSES + .iter() + .any(|c| cand.attribute_equality(Attribute::Class, &c.to_partialvalue())) { + trace!("Rejecting operation during pre_delete check"); Err(OperationError::SystemProtectedObject) } else { Ok(()) diff --git a/server/lib/src/server/migrations.rs b/server/lib/src/server/migrations.rs index e530f488f..a8ec14740 100644 --- a/server/lib/src/server/migrations.rs +++ b/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)] diff --git a/server/testkit/src/lib.rs b/server/testkit/src/lib.rs index 9d475a048..d3633e70f 100644 --- a/server/testkit/src/lib.rs +++ b/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); diff --git a/server/testkit/tests/domain.rs b/server/testkit/tests/domain.rs new file mode 100644 index 000000000..baacff0e2 --- /dev/null +++ b/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()]) + ); +} diff --git a/unix_integration/tests/cache_layer_test.rs b/unix_integration/tests/cache_layer_test.rs index 96f4ffcda..7496ddde8 100644 --- a/unix_integration/tests/cache_layer_test.rs +++ b/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, 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 ...