From d109622d71ccd9b9e70ec269e69a5acb2c7b92e2 Mon Sep 17 00:00:00 2001 From: CEbbinghaus Date: Thu, 3 Oct 2024 10:48:28 +1000 Subject: [PATCH] Make good on some TechDebt (#3084) adds MissingClass & MissingAttribute OperationError kinds to more strongly type our error messages. --- proto/src/constants.rs | 46 ++++++++- proto/src/internal/error.rs | 38 ++++---- server/core/src/https/v1.rs | 6 +- server/lib/src/be/mod.rs | 4 +- server/lib/src/constants/entries.rs | 87 ++++++++--------- server/lib/src/idm/account.rs | 40 +++----- server/lib/src/idm/application.rs | 18 +--- server/lib/src/idm/group.rs | 94 +++++-------------- server/lib/src/idm/radius.rs | 25 +---- server/lib/src/idm/scim.rs | 8 +- server/lib/src/idm/server.rs | 21 ++--- server/lib/src/idm/serviceaccount.rs | 4 +- server/lib/src/schema.rs | 4 +- server/lib/src/server/mod.rs | 4 +- .../resolver/src/idprovider/kanidm.rs | 30 ++++++ 15 files changed, 208 insertions(+), 221 deletions(-) diff --git a/proto/src/constants.rs b/proto/src/constants.rs index 3c78a5dcc..7afc9d9c2 100644 --- a/proto/src/constants.rs +++ b/proto/src/constants.rs @@ -245,6 +245,7 @@ pub const TEST_ATTR_TEST_ATTR: &str = "testattr"; pub const TEST_ATTR_EXTRA: &str = "extra"; pub const TEST_ATTR_NUMBER: &str = "testattrnumber"; pub const TEST_ATTR_NOTALLOWED: &str = "notallowed"; +pub const TEST_ENTRYCLASS_TEST_CLASS: &str = "testclass"; /// HTTP Header containing an auth session ID for when you're going through an auth flow pub const KSESSIONID: &str = "X-KANIDM-AUTH-SESSION-ID"; @@ -256,8 +257,51 @@ pub const KVERSION: &str = "X-KANIDM-VERSION"; /// X-Forwarded-For header pub const X_FORWARDED_FOR: &str = "x-forwarded-for"; -/// Builtin object +// OAuth +pub const OAUTH2_RESOURCE_SERVER: &str = "oauth2_resource_server"; +pub const OAUTH2_RESOURCE_SERVER_BASIC: &str = "oauth2_resource_server_basic"; +pub const OAUTH2_RESOURCE_SERVER_PUBLIC: &str = "oauth2_resource_server_public"; + +// Access Control +pub const ACCESS_CONTROL_CREATE: &str = "access_control_create"; +pub const ACCESS_CONTROL_DELETE: &str = "access_control_delete"; +pub const ACCESS_CONTROL_MODIFY: &str = "access_control_modify"; +pub const ACCESS_CONTROL_PROFILE: &str = "access_control_profile"; +pub const ACCESS_CONTROL_RECEIVER_ENTRY_MANAGER: &str = "access_control_receiver_entry_manager"; +pub const ACCESS_CONTROL_RECEIVER_GROUP: &str = "access_control_receiver_group"; +pub const ACCESS_CONTROL_SEARCH: &str = "access_control_search"; +pub const ACCESS_CONTROL_TARGET_SCOPE: &str = "access_control_target_scope"; + +/// Entryclass pub const ENTRYCLASS_BUILTIN: &str = "builtin"; +pub const ENTRYCLASS_ACCOUNT: &str = "account"; +pub const ENTRYCLASS_ACCOUNT_POLICY: &str = "account_policy"; +pub const ENTRYCLASS_APPLICATION: &str = "application"; +pub const ENTRYCLASS_ATTRIBUTE_TYPE: &str = "attributetype"; +pub const ENTRYCLASS_CLASS: &str = "class"; +pub const ENTRYCLASS_CLASS_TYPE: &str = "classtype"; +pub const ENTRYCLASS_CLIENT_CERTIFICATE: &str = "client_certificate"; +pub const ENTRYCLASS_CONFLICT: &str = "conflict"; +pub const ENTRYCLASS_DOMAIN_INFO: &str = "domain_info"; +pub const ENTRYCLASS_DYN_GROUP: &str = "dyngroup"; +pub const ENTRYCLASS_EXTENSIBLE_OBJECT: &str = "extensibleobject"; +pub const ENTRYCLASS_GROUP: &str = "group"; +pub const ENTRYCLASS_MEMBER_OF: &str = "memberof"; +pub const ENTRYCLASS_OBJECT: &str = "object"; +pub const ENTRYCLASS_ORG_PERSON: &str = "orgperson"; +pub const ENTRYCLASS_PERSON: &str = "person"; +pub const ENTRYCLASS_POSIX_ACCOUNT: &str = "posixaccount"; +pub const ENTRYCLASS_POSIX_GROUP: &str = "posixgroup"; +pub const ENTRYCLASS_RECYCLED: &str = "recycled"; +pub const ENTRYCLASS_SERVICE: &str = "service"; +pub const ENTRYCLASS_SERVICE_ACCOUNT: &str = "service_account"; +pub const ENTRYCLASS_SYNC_ACCOUNT: &str = "sync_account"; +pub const ENTRYCLASS_SYNC_OBJECT: &str = "sync_object"; +pub const ENTRYCLASS_SYSTEM: &str = "system"; +pub const ENTRYCLASS_SYSTEM_CONFIG: &str = "system_config"; +pub const ENTRYCLASS_SYSTEM_INFO: &str = "system_info"; +pub const ENTRYCLASS_TOMBSTONE: &str = "tombstone"; +pub const ENTRYCLASS_USER: &str = "user"; pub const ENTRYCLASS_KEY_PROVIDER: &str = "key_provider"; pub const ENTRYCLASS_KEY_PROVIDER_INTERNAL: &str = "key_provider_internal"; pub const ENTRYCLASS_KEY_OBJECT: &str = "key_object"; diff --git a/proto/src/internal/error.rs b/proto/src/internal/error.rs index b1c327ad4..0b8d1bf36 100644 --- a/proto/src/internal/error.rs +++ b/proto/src/internal/error.rs @@ -76,8 +76,7 @@ pub enum OperationError { UniqueConstraintViolation, CorruptedEntry(u64), CorruptedIndex(String), - // TODO: this should just be a vec of the ConsistencyErrors, surely? - ConsistencyError(Vec>), + ConsistencyError(Vec), SchemaViolation(SchemaError), Plugin(PluginError), FilterGeneration, @@ -98,6 +97,11 @@ pub enum OperationError { InvalidAcpState(String), InvalidSchemaState(String), InvalidAccountState(String), + // This really oughta be EntryClass but its not in proto... + // It should at least be &'static str but we + // Serialize & Deserialize this enum... + MissingClass(String), + MissingAttribute(Attribute), MissingEntries, ModifyAssertionFailed, BackendEngine, @@ -250,14 +254,14 @@ impl Display for OperationError { impl OperationError { /// Return the message associated with the error if there is one. - fn message(&self) -> Option<&'static str> { + fn message(&self) -> Option { match self { Self::SessionExpired => None, Self::EmptyRequest => None, Self::Backend => None, Self::NoMatchingEntries => None, Self::NoMatchingAttributes => None, - Self::UniqueConstraintViolation => Some("A unique constraint was violated resulting in multiple conflicting results."), + Self::UniqueConstraintViolation => Some("A unique constraint was violated resulting in multiple conflicting results.".into()), Self::CorruptedEntry(_) => None, Self::CorruptedIndex(_) => None, Self::ConsistencyError(_) => None, @@ -280,7 +284,9 @@ impl OperationError { Self::InvalidReplChangeId => None, Self::InvalidAcpState(_) => None, Self::InvalidSchemaState(_) => None, - Self::InvalidAccountState(_) => None, + Self::InvalidAccountState(val) => Some(format!("Invalid account state: {}", val)), + Self::MissingClass(val) => Some(format!("Missing class: {}", val)), + Self::MissingAttribute(val) => Some(format!("Missing attribute: {}", val)), Self::MissingEntries => None, Self::ModifyAssertionFailed => None, Self::BackendEngine => None, @@ -301,7 +307,7 @@ impl OperationError { Self::QueueDisconnected => None, Self::Webauthn => None, Self::Wait(_) => None, - Self::CannotStartMFADuringOngoingMFASession => Some("Cannot start a new MFA authentication flow when there already is one active."), + Self::CannotStartMFADuringOngoingMFASession => Some("Cannot start a new MFA authentication flow when there already is one active.".into()), Self::ReplReplayFailure => None, Self::ReplEntryNotChanged => None, Self::ReplInvalidRUVState => None, @@ -310,17 +316,17 @@ impl OperationError { Self::ReplServerUuidSplitDataState => None, Self::TransactionAlreadyCommitted => None, Self::ValueDenyName => None, - Self::DatabaseLockAcquisitionTimeout => Some("Unable to acquire a database lock - the current server may be too busy. Try again later."), + Self::DatabaseLockAcquisitionTimeout => Some("Unable to acquire a database lock - the current server may be too busy. Try again later.".into()), Self::CU0002WebauthnRegistrationError => None, - Self::CU0003WebauthnUserNotVerified => Some("User Verification bit not set while registering credential, you may need to configure a PIN on this device."), + Self::CU0003WebauthnUserNotVerified => Some("User Verification bit not set while registering credential, you may need to configure a PIN on this device.".into()), Self::CU0001WebauthnAttestationNotTrusted => None, Self::VS0001IncomingReplSshPublicKey => None, - Self::VS0003CertificateDerDecode => Some("Decoding the stored certificate from DER failed."), + Self::VS0003CertificateDerDecode => Some("Decoding the stored certificate from DER failed.".into()), Self::VS0002CertificatePublicKeyDigest | Self::VS0004CertificatePublicKeyDigest | - Self::VS0005CertificatePublicKeyDigest => Some("The certificates public key is unabled to be digested."), + Self::VS0005CertificatePublicKeyDigest => Some("The certificates public key is unabled to be digested.".into()), Self::VL0001ValueSshPublicKeyString => None, - Self::LD0001AnonymousNotAllowed => Some("Anonymous is not allowed to access LDAP with this method."), + Self::LD0001AnonymousNotAllowed => Some("Anonymous is not allowed to access LDAP with this method.".into()), Self::SC0001IncomingSshPublicKey => None, Self::MG0001InvalidReMigrationLevel => None, Self::MG0002RaiseDomainLevelExceedsMaximum => None, @@ -330,9 +336,9 @@ impl OperationError { Self::DB0003FilterResolveCacheBuild => None, Self::MG0004DomainLevelInDevelopment => None, Self::MG0005GidConstraintsNotMet => None, - Self::MG0006SKConstraintsNotMet => Some("Migration Constraints Not Met - Security Keys should not be present."), - Self::MG0007Oauth2StrictConstraintsNotMet => Some("Migration Constraints Not Met - All OAuth2 clients must have strict-redirect-uri mode enabled."), - Self::MG0008SkipUpgradeAttempted => Some("Skip Upgrade Attempted."), + Self::MG0006SKConstraintsNotMet => Some("Migration Constraints Not Met - Security Keys should not be present.".into()), + Self::MG0007Oauth2StrictConstraintsNotMet => Some("Migration Constraints Not Met - All OAuth2 clients must have strict-redirect-uri mode enabled.".into()), + Self::MG0008SkipUpgradeAttempted => Some("Skip Upgrade Attempted.".into()), Self::KP0001KeyProviderNotLoaded => None, Self::KP0002KeyProviderInvalidClass => None, Self::KP0003KeyProviderInvalidType => None, @@ -379,8 +385,8 @@ impl OperationError { Self::KP0043KeyObjectJweA128GCMEncryption => None, Self::KP0044KeyObjectJwsPublicJwk => None, Self::PL0001GidOverlapsSystemRange => None, - Self::UI0001ChallengeSerialisation => Some("The WebAuthn challenge was unable to be serialised."), - Self::UI0002InvalidState => Some("The credential update process returned an invalid state transition."), + Self::UI0001ChallengeSerialisation => Some("The WebAuthn challenge was unable to be serialised.".into()), + Self::UI0002InvalidState => Some("The credential update process returned an invalid state transition.".into()), } } } diff --git a/server/core/src/https/v1.rs b/server/core/src/https/v1.rs index f486379ab..cbf6783c1 100644 --- a/server/core/src/https/v1.rs +++ b/server/core/src/https/v1.rs @@ -2061,9 +2061,9 @@ pub async fn account_id_unix_token( .await .map(Json::from); - if let Err(OperationError::InvalidAccountState(val)) = &res { - // if they're not a posix user we should just hide them - if *val == format!("Missing class: {}", "posixaccount") { + // if they're not a posix user we should just hide them + if let Err(OperationError::MissingClass(class)) = &res { + if class == ENTRYCLASS_POSIX_ACCOUNT { return Err(OperationError::NoMatchingEntries.into()); } }; diff --git a/server/lib/src/be/mod.rs b/server/lib/src/be/mod.rs index 6c5721c46..1f444cab2 100644 --- a/server/lib/src/be/mod.rs +++ b/server/lib/src/be/mod.rs @@ -1920,7 +1920,9 @@ impl<'a> BackendWriteTransaction<'a> { if vr.is_empty() { Ok(()) } else { - Err(OperationError::ConsistencyError(vr)) + Err(OperationError::ConsistencyError( + vr.into_iter().filter_map(|v| v.err()).collect(), + )) } } diff --git a/server/lib/src/constants/entries.rs b/server/lib/src/constants/entries.rs index f69216d98..2239ee1b0 100644 --- a/server/lib/src/constants/entries.rs +++ b/server/lib/src/constants/entries.rs @@ -14,6 +14,11 @@ use kanidm_proto::v1::AccountType; use uuid::Uuid; +//TODO: This would do well in the proto lib +// together with all the other definitions. +// That way`OperationError::MissingClass` can +// Directly reference the entryclass rather +// than relying on its string name #[derive(Copy, Clone, Debug)] pub enum EntryClass { AccessControlCreate, @@ -69,56 +74,54 @@ pub enum EntryClass { impl From for &'static str { fn from(val: EntryClass) -> Self { match val { - EntryClass::AccessControlCreate => "access_control_create", - EntryClass::AccessControlDelete => "access_control_delete", - EntryClass::AccessControlModify => "access_control_modify", - EntryClass::AccessControlProfile => "access_control_profile", - EntryClass::AccessControlReceiverEntryManager => { - "access_control_receiver_entry_manager" - } - EntryClass::AccessControlReceiverGroup => "access_control_receiver_group", - EntryClass::AccessControlSearch => "access_control_search", - EntryClass::AccessControlTargetScope => "access_control_target_scope", - EntryClass::Account => "account", - EntryClass::AccountPolicy => "account_policy", - EntryClass::Application => "application", - EntryClass::AttributeType => "attributetype", + EntryClass::AccessControlCreate => ACCESS_CONTROL_CREATE, + EntryClass::AccessControlDelete => ACCESS_CONTROL_DELETE, + EntryClass::AccessControlModify => ACCESS_CONTROL_MODIFY, + EntryClass::AccessControlProfile => ACCESS_CONTROL_PROFILE, + EntryClass::AccessControlReceiverEntryManager => ACCESS_CONTROL_RECEIVER_ENTRY_MANAGER, + EntryClass::AccessControlReceiverGroup => ACCESS_CONTROL_RECEIVER_GROUP, + EntryClass::AccessControlSearch => ACCESS_CONTROL_SEARCH, + EntryClass::AccessControlTargetScope => ACCESS_CONTROL_TARGET_SCOPE, + EntryClass::Account => ENTRYCLASS_ACCOUNT, + EntryClass::AccountPolicy => ENTRYCLASS_ACCOUNT_POLICY, + EntryClass::Application => ENTRYCLASS_APPLICATION, + EntryClass::AttributeType => ENTRYCLASS_ATTRIBUTE_TYPE, EntryClass::Builtin => ENTRYCLASS_BUILTIN, - EntryClass::Class => ATTR_CLASS, - EntryClass::ClassType => "classtype", - EntryClass::ClientCertificate => "client_certificate", - EntryClass::Conflict => "conflict", - EntryClass::DomainInfo => "domain_info", - EntryClass::DynGroup => ATTR_DYNGROUP, - EntryClass::ExtensibleObject => "extensibleobject", - EntryClass::Group => ATTR_GROUP, + EntryClass::Class => ENTRYCLASS_CLASS, + EntryClass::ClassType => ENTRYCLASS_CLASS_TYPE, + EntryClass::ClientCertificate => ENTRYCLASS_CLIENT_CERTIFICATE, + EntryClass::Conflict => ENTRYCLASS_CONFLICT, + EntryClass::DomainInfo => ENTRYCLASS_DOMAIN_INFO, + EntryClass::DynGroup => ENTRYCLASS_DYN_GROUP, + EntryClass::ExtensibleObject => ENTRYCLASS_EXTENSIBLE_OBJECT, + EntryClass::Group => ENTRYCLASS_GROUP, EntryClass::KeyProvider => ENTRYCLASS_KEY_PROVIDER, EntryClass::KeyProviderInternal => ENTRYCLASS_KEY_PROVIDER_INTERNAL, EntryClass::KeyObject => ENTRYCLASS_KEY_OBJECT, EntryClass::KeyObjectJwtEs256 => ENTRYCLASS_KEY_OBJECT_JWT_ES256, EntryClass::KeyObjectJweA128GCM => ENTRYCLASS_KEY_OBJECT_JWE_A128GCM, EntryClass::KeyObjectInternal => ENTRYCLASS_KEY_OBJECT_INTERNAL, - EntryClass::MemberOf => "memberof", - EntryClass::OAuth2ResourceServer => "oauth2_resource_server", - EntryClass::OAuth2ResourceServerBasic => "oauth2_resource_server_basic", - EntryClass::OAuth2ResourceServerPublic => "oauth2_resource_server_public", - EntryClass::Object => "object", - EntryClass::OrgPerson => "orgperson", - EntryClass::Person => "person", - EntryClass::PosixAccount => "posixaccount", - EntryClass::PosixGroup => "posixgroup", - EntryClass::Recycled => "recycled", - EntryClass::Service => "service", - EntryClass::ServiceAccount => "service_account", - EntryClass::SyncAccount => "sync_account", - EntryClass::SyncObject => "sync_object", - EntryClass::System => "system", - EntryClass::SystemConfig => "system_config", - EntryClass::SystemInfo => "system_info", - EntryClass::Tombstone => "tombstone", + EntryClass::MemberOf => ENTRYCLASS_MEMBER_OF, + EntryClass::OAuth2ResourceServer => OAUTH2_RESOURCE_SERVER, + EntryClass::OAuth2ResourceServerBasic => OAUTH2_RESOURCE_SERVER_BASIC, + EntryClass::OAuth2ResourceServerPublic => OAUTH2_RESOURCE_SERVER_PUBLIC, + EntryClass::Object => ENTRYCLASS_OBJECT, + EntryClass::OrgPerson => ENTRYCLASS_ORG_PERSON, + EntryClass::Person => ENTRYCLASS_PERSON, + EntryClass::PosixAccount => ENTRYCLASS_POSIX_ACCOUNT, + EntryClass::PosixGroup => ENTRYCLASS_POSIX_GROUP, + EntryClass::Recycled => ENTRYCLASS_RECYCLED, + EntryClass::Service => ENTRYCLASS_SERVICE, + EntryClass::ServiceAccount => ENTRYCLASS_SERVICE_ACCOUNT, + EntryClass::SyncAccount => ENTRYCLASS_SYNC_ACCOUNT, + EntryClass::SyncObject => ENTRYCLASS_SYNC_OBJECT, + EntryClass::System => ENTRYCLASS_SYSTEM, + EntryClass::SystemConfig => ENTRYCLASS_SYSTEM_CONFIG, + EntryClass::SystemInfo => ENTRYCLASS_SYSTEM_INFO, + EntryClass::Tombstone => ENTRYCLASS_TOMBSTONE, #[cfg(any(test, debug_assertions))] - EntryClass::TestClass => "testclass", - EntryClass::User => "user", + EntryClass::TestClass => TEST_ENTRYCLASS_TEST_CLASS, + EntryClass::User => ENTRYCLASS_USER, } } } diff --git a/server/lib/src/idm/account.rs b/server/lib/src/idm/account.rs index 5fd1d59f4..a4d871402 100644 --- a/server/lib/src/idm/account.rs +++ b/server/lib/src/idm/account.rs @@ -78,28 +78,19 @@ macro_rules! try_from_entry { ($value:expr, $groups:expr, $unix_groups:expr) => {{ // Check the classes if !$value.attribute_equality(Attribute::Class, &EntryClass::Account.to_partialvalue()) { - return Err(OperationError::InvalidAccountState(format!( - "Missing class: {}", - EntryClass::Account - ))); + return Err(OperationError::MissingClass(ENTRYCLASS_ACCOUNT.into())); } // Now extract our needed attributes let name = $value .get_ava_single_iname(Attribute::Name) .map(|s| s.to_string()) - .ok_or(OperationError::InvalidAccountState(format!( - "Missing attribute: {}", - Attribute::Name - )))?; + .ok_or(OperationError::MissingAttribute(Attribute::Name))?; let displayname = $value .get_ava_single_utf8(Attribute::DisplayName) .map(|s| s.to_string()) - .ok_or(OperationError::InvalidAccountState(format!( - "Missing attribute: {}", - Attribute::DisplayName - )))?; + .ok_or(OperationError::MissingAttribute(Attribute::DisplayName))?; let sync_parent_uuid = $value.get_ava_single_refer(Attribute::SyncParentUuid); @@ -117,9 +108,9 @@ macro_rules! try_from_entry { .cloned() .unwrap_or_default(); - let spn = $value.get_ava_single_proto_string(Attribute::Spn).ok_or( - OperationError::InvalidAccountState(format!("Missing attribute: {}", Attribute::Spn)), - )?; + let spn = $value + .get_ava_single_proto_string(Attribute::Spn) + .ok_or(OperationError::MissingAttribute(Attribute::Spn))?; let mail_primary = $value .get_ava_mail_primary(Attribute::Mail) @@ -187,12 +178,7 @@ macro_rules! try_from_entry { let gidnumber = $value .get_ava_single_uint32(Attribute::GidNumber) - .ok_or_else(|| { - OperationError::InvalidAccountState(format!( - "Missing attribute: {}", - Attribute::GidNumber - )) - })?; + .ok_or_else(|| OperationError::MissingAttribute(Attribute::GidNumber))?; let groups = $unix_groups; @@ -817,10 +803,9 @@ impl Account { (ue.gidnumber, ue.shell.clone(), sshkeys, ue.groups.clone()) } None => { - return Err(OperationError::InvalidAccountState(format!( - "Missing class: {}", - EntryClass::PosixAccount - ))); + return Err(OperationError::MissingClass( + ENTRYCLASS_POSIX_ACCOUNT.into(), + )); } }; @@ -929,10 +914,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> { "Invalid entry, {} attribute is not present or not iutf8", Attribute::Class ); - OperationError::InvalidAccountState(format!( - "Missing attribute: {}", - Attribute::Class - )) + OperationError::MissingAttribute(Attribute::Class) })? .collect(); diff --git a/server/lib/src/idm/application.rs b/server/lib/src/idm/application.rs index d32f934e1..ab0c44b7d 100644 --- a/server/lib/src/idm/application.rs +++ b/server/lib/src/idm/application.rs @@ -23,9 +23,7 @@ impl Application { _qs: &mut QueryServerReadTransaction, ) -> Result { if !value.attribute_equality(Attribute::Class, &EntryClass::Application.to_partialvalue()) { - return Err(OperationError::InvalidAccountState( - "Missing class: application".to_string(), - )); + return Err(OperationError::MissingClass(ENTRYCLASS_APPLICATION.into())); } let uuid = value.get_uuid(); @@ -33,21 +31,11 @@ impl Application { let name = value .get_ava_single_iname(Attribute::Name) .map(|s| s.to_string()) - .ok_or_else(|| { - OperationError::InvalidAccountState(format!( - "Missing attribute: {}", - Attribute::Name - )) - })?; + .ok_or_else(|| OperationError::MissingAttribute(Attribute::Name))?; let linked_group = value .get_ava_single_refer(Attribute::LinkedGroup) - .ok_or_else(|| { - OperationError::InvalidAccountState(format!( - "Missing attribute: {}", - Attribute::LinkedGroup - )) - })?; + .ok_or_else(|| OperationError::MissingAttribute(Attribute::LinkedGroup))?; Ok(Application { name, diff --git a/server/lib/src/idm/group.rs b/server/lib/src/idm/group.rs index da90bbd7e..f89b2f1c5 100644 --- a/server/lib/src/idm/group.rs +++ b/server/lib/src/idm/group.rs @@ -45,9 +45,9 @@ macro_rules! load_all_groups_from_iter { ); // Setup the user private group - let spn = $value.get_ava_single_proto_string(Attribute::Spn).ok_or( - OperationError::InvalidAccountState(format!("Missing attribute: {}", Attribute::Spn)), - )?; + let spn = $value + .get_ava_single_proto_string(Attribute::Spn) + .ok_or(OperationError::MissingAttribute(Attribute::Spn))?; let uuid = $value.get_uuid(); @@ -61,19 +61,13 @@ macro_rules! load_all_groups_from_iter { }); if is_unix_account { - let name = $value.get_ava_single_proto_string(Attribute::Name).ok_or( - OperationError::InvalidAccountState(format!( - "Missing attribute: {}", - Attribute::Name - )), - )?; + let name = $value + .get_ava_single_proto_string(Attribute::Name) + .ok_or(OperationError::MissingAttribute(Attribute::Name))?; - let gidnumber = $value.get_ava_single_uint32(Attribute::GidNumber).ok_or( - OperationError::InvalidAccountState(format!( - "Missing attribute: {}", - Attribute::GidNumber - )), - )?; + let gidnumber = $value + .get_ava_single_uint32(Attribute::GidNumber) + .ok_or(OperationError::MissingAttribute(Attribute::GidNumber))?; unix_groups.push(UnixGroup { name, @@ -150,9 +144,9 @@ pub struct Group { macro_rules! upg_from_account_e { ($value:expr, $groups:expr) => {{ // Setup the user private group - let spn = $value.get_ava_single_proto_string(Attribute::Spn).ok_or( - OperationError::InvalidAccountState(format!("Missing attribute: {}", Attribute::Spn)), - )?; + let spn = $value + .get_ava_single_proto_string(Attribute::Spn) + .ok_or(OperationError::MissingAttribute(Attribute::Spn))?; let uuid = $value.get_uuid(); @@ -210,9 +204,7 @@ impl Group { value: &Entry, ) -> Result { if !value.attribute_equality(Attribute::Class, &EntryClass::Group.into()) { - return Err(OperationError::InvalidAccountState( - "Missing class: group".to_string(), - )); + return Err(OperationError::MissingAttribute(Attribute::Group)); } // Now extract our needed attributes @@ -221,14 +213,12 @@ impl Group { .get_ava_single_iname(Attribute::Name) .map(|s| s.to_string()) .ok_or_else(|| { - OperationError::InvalidAccountState("Missing attribute: name".to_string()) + OperationError::MissingAttribute(Attribute::Name) })?; */ let spn = value .get_ava_single_proto_string(Attribute::Spn) - .ok_or_else(|| { - OperationError::InvalidAccountState("Missing attribute: spn".to_string()) - })?; + .ok_or_else(|| OperationError::MissingAttribute(Attribute::Spn))?; let uuid = value.get_uuid(); @@ -288,32 +278,17 @@ macro_rules! try_from_group_e { let name = $value .get_ava_single_iname(Attribute::Name) .map(|s| s.to_string()) - .ok_or_else(|| { - OperationError::InvalidAccountState(format!( - "Missing attribute: {}", - Attribute::Name - )) - })?; + .ok_or_else(|| OperationError::MissingAttribute(Attribute::Name))?; let spn = $value .get_ava_single_proto_string(Attribute::Spn) - .ok_or_else(|| { - OperationError::InvalidAccountState(format!( - "Missing attribute: {}", - Attribute::Spn - )) - })?; + .ok_or_else(|| OperationError::MissingAttribute(Attribute::Spn))?; let uuid = $value.get_uuid(); let gidnumber = $value .get_ava_single_uint32(Attribute::GidNumber) - .ok_or_else(|| { - OperationError::InvalidAccountState(format!( - "Missing attribute: {}", - Attribute::GidNumber - )) - })?; + .ok_or_else(|| OperationError::MissingAttribute(Attribute::GidNumber))?; Ok(UnixGroup { name, @@ -330,51 +305,32 @@ macro_rules! try_from_account_group_e { // We have already checked these, but paranoia is better than // complacency. if !$value.attribute_equality(Attribute::Class, &EntryClass::Account.to_partialvalue()) { - return Err(OperationError::InvalidAccountState(format!( - "Missing class: {}", - EntryClass::Account - ))); + return Err(OperationError::MissingClass(ENTRYCLASS_ACCOUNT.into())); } if !$value.attribute_equality( Attribute::Class, &EntryClass::PosixAccount.to_partialvalue(), ) { - return Err(OperationError::InvalidAccountState(format!( - "Missing class: {}", - EntryClass::PosixAccount - ))); + return Err(OperationError::MissingClass( + ENTRYCLASS_POSIX_ACCOUNT.into(), + )); } let name = $value .get_ava_single_iname(Attribute::Name) .map(|s| s.to_string()) - .ok_or_else(|| { - OperationError::InvalidAccountState(format!( - "Missing attribute: {}", - Attribute::Name - )) - })?; + .ok_or_else(|| OperationError::MissingAttribute(Attribute::Name))?; let spn = $value .get_ava_single_proto_string(Attribute::Spn) - .ok_or_else(|| { - OperationError::InvalidAccountState(format!( - "Missing attribute: {}", - Attribute::Spn - )) - })?; + .ok_or_else(|| OperationError::MissingAttribute(Attribute::Spn))?; let uuid = $value.get_uuid(); let gidnumber = $value .get_ava_single_uint32(Attribute::GidNumber) - .ok_or_else(|| { - OperationError::InvalidAccountState(format!( - "Missing attribute: {}", - Attribute::GidNumber - )) - })?; + .ok_or_else(|| OperationError::MissingAttribute(Attribute::GidNumber))?; // This is the user private group. let upg = UnixGroup { diff --git a/server/lib/src/idm/radius.rs b/server/lib/src/idm/radius.rs index c56f981f6..37c7f6d53 100644 --- a/server/lib/src/idm/radius.rs +++ b/server/lib/src/idm/radius.rs @@ -25,42 +25,25 @@ impl RadiusAccount { qs: &mut QueryServerReadTransaction, ) -> Result { if !value.attribute_equality(Attribute::Class, &EntryClass::Account.into()) { - return Err(OperationError::InvalidAccountState( - "Missing class: account".to_string(), - )); + return Err(OperationError::MissingClass(ENTRYCLASS_ACCOUNT.into())); } let radius_secret = value .get_ava_single_secret(Attribute::RadiusSecret) - .ok_or_else(|| { - OperationError::InvalidAccountState(format!( - "Missing attribute: {}", - Attribute::RadiusSecret - )) - })? + .ok_or_else(|| OperationError::MissingAttribute(Attribute::RadiusSecret))? .to_string(); let name = value .get_ava_single_iname(Attribute::Name) .map(|s| s.to_string()) - .ok_or_else(|| { - OperationError::InvalidAccountState(format!( - "Missing attribute: {}", - Attribute::Name - )) - })?; + .ok_or_else(|| OperationError::MissingAttribute(Attribute::Name))?; let uuid = value.get_uuid(); let displayname = value .get_ava_single_utf8(Attribute::DisplayName) .map(|s| s.to_string()) - .ok_or_else(|| { - OperationError::InvalidAccountState(format!( - "Missing attribute: {}", - Attribute::DisplayName - )) - })?; + .ok_or_else(|| OperationError::MissingAttribute(Attribute::DisplayName))?; let groups = Group::try_from_account_entry_reduced(value, qs)?; diff --git a/server/lib/src/idm/scim.rs b/server/lib/src/idm/scim.rs index 8466069a8..1f35f6b8c 100644 --- a/server/lib/src/idm/scim.rs +++ b/server/lib/src/idm/scim.rs @@ -29,17 +29,13 @@ macro_rules! try_from_entry { ($value:expr) => {{ // Check the classes if !$value.attribute_equality(Attribute::Class, &EntryClass::SyncAccount.into()) { - return Err(OperationError::InvalidAccountState( - "Missing class: sync account".to_string(), - )); + return Err(OperationError::MissingClass(ENTRYCLASS_SYNC_ACCOUNT.into())); } let name = $value .get_ava_single_iname(Attribute::Name) .map(|s| s.to_string()) - .ok_or(OperationError::InvalidAccountState( - "Missing attribute: name".to_string(), - ))?; + .ok_or(OperationError::MissingAttribute(Attribute::Name))?; let jws_key = $value .get_ava_single_jws_key_es256(Attribute::JwsEs256PrivateKey) diff --git a/server/lib/src/idm/server.rs b/server/lib/src/idm/server.rs index 86d619be1..9b18a7e34 100644 --- a/server/lib/src/idm/server.rs +++ b/server/lib/src/idm/server.rs @@ -1468,12 +1468,7 @@ impl<'a> IdmServerAuthTransaction<'a> { Token::ApiToken(apit, entry) => { let spn = entry .get_ava_single_proto_string(Attribute::Spn) - .ok_or_else(|| { - OperationError::InvalidAccountState(format!( - "Missing attribute: {}", - Attribute::Spn - )) - })?; + .ok_or_else(|| OperationError::MissingAttribute(Attribute::Spn))?; Ok(Some(LdapBoundToken { session_id: apit.token_id, @@ -1801,10 +1796,9 @@ impl<'a> IdmServerProxyWriteTransaction<'a> { // Account is not a unix account if account.unix_extn().is_none() { - return Err(OperationError::InvalidAccountState(format!( - "Missing class: {}", - EntryClass::PosixAccount - ))); + return Err(OperationError::MissingClass( + ENTRYCLASS_POSIX_ACCOUNT.into(), + )); } // Deny the change if the account is anonymous! @@ -1991,10 +1985,9 @@ impl<'a> IdmServerProxyWriteTransaction<'a> { let cred = match account.unix_extn() { Some(ue) => ue.ucred(), None => { - return Err(OperationError::InvalidAccountState(format!( - "Missing class: {}", - EntryClass::PosixAccount - ))); + return Err(OperationError::MissingClass( + ENTRYCLASS_POSIX_ACCOUNT.into(), + )); } }; diff --git a/server/lib/src/idm/serviceaccount.rs b/server/lib/src/idm/serviceaccount.rs index c781e5c0e..e432af998 100644 --- a/server/lib/src/idm/serviceaccount.rs +++ b/server/lib/src/idm/serviceaccount.rs @@ -18,8 +18,8 @@ macro_rules! try_from_entry { ($value:expr) => {{ // Check the classes if !$value.attribute_equality(Attribute::Class, &EntryClass::ServiceAccount.into()) { - return Err(OperationError::InvalidAccountState( - "Missing class: service account".to_string(), + return Err(OperationError::MissingClass( + ENTRYCLASS_SERVICE_ACCOUNT.into(), )); } diff --git a/server/lib/src/schema.rs b/server/lib/src/schema.rs index 6e0620248..916af55c5 100644 --- a/server/lib/src/schema.rs +++ b/server/lib/src/schema.rs @@ -2210,7 +2210,9 @@ impl<'a> SchemaWriteTransaction<'a> { Ok(()) } else { admin_error!(err = ?r, "schema validate -> errors"); - Err(OperationError::ConsistencyError(r)) + Err(OperationError::ConsistencyError( + r.into_iter().filter_map(|v| v.err()).collect(), + )) } } } diff --git a/server/lib/src/server/mod.rs b/server/lib/src/server/mod.rs index 82494f5fa..be7fe2c6b 100644 --- a/server/lib/src/server/mod.rs +++ b/server/lib/src/server/mod.rs @@ -1704,7 +1704,9 @@ impl<'a> QueryServerWriteTransaction<'a> { } else { // Log the failures? admin_error!("Schema reload failed -> {:?}", valid_r); - Err(OperationError::ConsistencyError(valid_r)) + Err(OperationError::ConsistencyError( + valid_r.into_iter().filter_map(|v| v.err()).collect(), + )) }?; // TODO: Clear the filter resolve cache. diff --git a/unix_integration/resolver/src/idprovider/kanidm.rs b/unix_integration/resolver/src/idprovider/kanidm.rs index ae7bee893..5dc40666b 100644 --- a/unix_integration/resolver/src/idprovider/kanidm.rs +++ b/unix_integration/resolver/src/idprovider/kanidm.rs @@ -347,6 +347,16 @@ impl IdProvider for KanidmProvider { Some(OperationError::NoMatchingEntries), opid, )) + | Err(ClientError::Http( + StatusCode::NOT_FOUND, + Some(OperationError::MissingAttribute(_)), + opid, + )) + | Err(ClientError::Http( + StatusCode::NOT_FOUND, + Some(OperationError::MissingClass(_)), + opid, + )) | Err(ClientError::Http( StatusCode::BAD_REQUEST, Some(OperationError::InvalidAccountState(_)), @@ -460,6 +470,16 @@ impl IdProvider for KanidmProvider { Some(OperationError::NoMatchingEntries), opid, )) + | Err(ClientError::Http( + StatusCode::NOT_FOUND, + Some(OperationError::MissingAttribute(_)), + opid, + )) + | Err(ClientError::Http( + StatusCode::NOT_FOUND, + Some(OperationError::MissingClass(_)), + opid, + )) | Err(ClientError::Http( StatusCode::BAD_REQUEST, Some(OperationError::InvalidAccountState(_)), @@ -591,6 +611,16 @@ impl IdProvider for KanidmProvider { Some(OperationError::NoMatchingEntries), opid, )) + | Err(ClientError::Http( + StatusCode::NOT_FOUND, + Some(OperationError::MissingAttribute(_)), + opid, + )) + | Err(ClientError::Http( + StatusCode::NOT_FOUND, + Some(OperationError::MissingClass(_)), + opid, + )) | Err(ClientError::Http( StatusCode::BAD_REQUEST, Some(OperationError::InvalidAccountState(_)),