Make good on some TechDebt (#3084)

adds MissingClass & MissingAttribute OperationError kinds to more strongly type our error messages.

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";

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>),
-    ConsistencyError(Vec<Result<(), ConsistencyError>>),
     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<String> {
         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::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."),
+            Self::MG0007Oauth2StrictConstraintsNotMet => Some("Migration Constraints Not Met - All OAuth2 clients must have strict-redirect-uri mode enabled.".into()),
-            Self::MG0008SkipUpgradeAttempted => Some("Skip Upgrade Attempted."),
+            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::UI0001ChallengeSerialisation => Some("The WebAuthn challenge was unable to be serialised.".into()),
-            Self::UI0002InvalidState => Some("The credential update process returned an invalid state transition."),
+            Self::UI0002InvalidState => Some("The credential update process returned an invalid state transition.".into()),
         }
     }
 }

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 let Err(OperationError::MissingClass(class)) = &res {
+        if class == ENTRYCLASS_POSIX_ACCOUNT {
             return Err(OperationError::NoMatchingEntries.into());
         }
     };

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(),
+            ))
         }
     }
 

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<EntryClass> for &'static str {
     fn from(val: EntryClass) -> Self {
         match val {
-            EntryClass::AccessControlCreate => "access_control_create",
+            EntryClass::AccessControlCreate => ACCESS_CONTROL_CREATE,
-            EntryClass::AccessControlDelete => "access_control_delete",
+            EntryClass::AccessControlDelete => ACCESS_CONTROL_DELETE,
-            EntryClass::AccessControlModify => "access_control_modify",
+            EntryClass::AccessControlModify => ACCESS_CONTROL_MODIFY,
-            EntryClass::AccessControlProfile => "access_control_profile",
+            EntryClass::AccessControlProfile => ACCESS_CONTROL_PROFILE,
-            EntryClass::AccessControlReceiverEntryManager => {
+            EntryClass::AccessControlReceiverEntryManager => ACCESS_CONTROL_RECEIVER_ENTRY_MANAGER,
-                "access_control_receiver_entry_manager"
+            EntryClass::AccessControlReceiverGroup => ACCESS_CONTROL_RECEIVER_GROUP,
-            }
+            EntryClass::AccessControlSearch => ACCESS_CONTROL_SEARCH,
-            EntryClass::AccessControlReceiverGroup => "access_control_receiver_group",
+            EntryClass::AccessControlTargetScope => ACCESS_CONTROL_TARGET_SCOPE,
-            EntryClass::AccessControlSearch => "access_control_search",
+            EntryClass::Account => ENTRYCLASS_ACCOUNT,
-            EntryClass::AccessControlTargetScope => "access_control_target_scope",
+            EntryClass::AccountPolicy => ENTRYCLASS_ACCOUNT_POLICY,
-            EntryClass::Account => "account",
+            EntryClass::Application => ENTRYCLASS_APPLICATION,
-            EntryClass::AccountPolicy => "account_policy",
+            EntryClass::AttributeType => ENTRYCLASS_ATTRIBUTE_TYPE,
-            EntryClass::Application => "application",
-            EntryClass::AttributeType => "attributetype",
             EntryClass::Builtin => ENTRYCLASS_BUILTIN,
-            EntryClass::Class => ATTR_CLASS,
+            EntryClass::Class => ENTRYCLASS_CLASS,
-            EntryClass::ClassType => "classtype",
+            EntryClass::ClassType => ENTRYCLASS_CLASS_TYPE,
-            EntryClass::ClientCertificate => "client_certificate",
+            EntryClass::ClientCertificate => ENTRYCLASS_CLIENT_CERTIFICATE,
-            EntryClass::Conflict => "conflict",
+            EntryClass::Conflict => ENTRYCLASS_CONFLICT,
-            EntryClass::DomainInfo => "domain_info",
+            EntryClass::DomainInfo => ENTRYCLASS_DOMAIN_INFO,
-            EntryClass::DynGroup => ATTR_DYNGROUP,
+            EntryClass::DynGroup => ENTRYCLASS_DYN_GROUP,
-            EntryClass::ExtensibleObject => "extensibleobject",
+            EntryClass::ExtensibleObject => ENTRYCLASS_EXTENSIBLE_OBJECT,
-            EntryClass::Group => ATTR_GROUP,
+            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::MemberOf => ENTRYCLASS_MEMBER_OF,
-            EntryClass::OAuth2ResourceServer => "oauth2_resource_server",
+            EntryClass::OAuth2ResourceServer => OAUTH2_RESOURCE_SERVER,
-            EntryClass::OAuth2ResourceServerBasic => "oauth2_resource_server_basic",
+            EntryClass::OAuth2ResourceServerBasic => OAUTH2_RESOURCE_SERVER_BASIC,
-            EntryClass::OAuth2ResourceServerPublic => "oauth2_resource_server_public",
+            EntryClass::OAuth2ResourceServerPublic => OAUTH2_RESOURCE_SERVER_PUBLIC,
-            EntryClass::Object => "object",
+            EntryClass::Object => ENTRYCLASS_OBJECT,
-            EntryClass::OrgPerson => "orgperson",
+            EntryClass::OrgPerson => ENTRYCLASS_ORG_PERSON,
-            EntryClass::Person => "person",
+            EntryClass::Person => ENTRYCLASS_PERSON,
-            EntryClass::PosixAccount => "posixaccount",
+            EntryClass::PosixAccount => ENTRYCLASS_POSIX_ACCOUNT,
-            EntryClass::PosixGroup => "posixgroup",
+            EntryClass::PosixGroup => ENTRYCLASS_POSIX_GROUP,
-            EntryClass::Recycled => "recycled",
+            EntryClass::Recycled => ENTRYCLASS_RECYCLED,
-            EntryClass::Service => "service",
+            EntryClass::Service => ENTRYCLASS_SERVICE,
-            EntryClass::ServiceAccount => "service_account",
+            EntryClass::ServiceAccount => ENTRYCLASS_SERVICE_ACCOUNT,
-            EntryClass::SyncAccount => "sync_account",
+            EntryClass::SyncAccount => ENTRYCLASS_SYNC_ACCOUNT,
-            EntryClass::SyncObject => "sync_object",
+            EntryClass::SyncObject => ENTRYCLASS_SYNC_OBJECT,
-            EntryClass::System => "system",
+            EntryClass::System => ENTRYCLASS_SYSTEM,
-            EntryClass::SystemConfig => "system_config",
+            EntryClass::SystemConfig => ENTRYCLASS_SYSTEM_CONFIG,
-            EntryClass::SystemInfo => "system_info",
+            EntryClass::SystemInfo => ENTRYCLASS_SYSTEM_INFO,
-            EntryClass::Tombstone => "tombstone",
+            EntryClass::Tombstone => ENTRYCLASS_TOMBSTONE,
             #[cfg(any(test, debug_assertions))]
-            EntryClass::TestClass => "testclass",
+            EntryClass::TestClass => TEST_ENTRYCLASS_TEST_CLASS,
-            EntryClass::User => "user",
+            EntryClass::User => ENTRYCLASS_USER,
         }
     }
 }

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!(
+            return Err(OperationError::MissingClass(ENTRYCLASS_ACCOUNT.into()));
-                "Missing class: {}",
-                EntryClass::Account
-            )));
         }
 
         // Now extract our needed attributes
         let name = $value
             .get_ava_single_iname(Attribute::Name)
             .map(|s| s.to_string())
-            .ok_or(OperationError::InvalidAccountState(format!(
+            .ok_or(OperationError::MissingAttribute(Attribute::Name))?;
-                "Missing attribute: {}",
-                Attribute::Name
-            )))?;
 
         let displayname = $value
             .get_ava_single_utf8(Attribute::DisplayName)
             .map(|s| s.to_string())
-            .ok_or(OperationError::InvalidAccountState(format!(
+            .ok_or(OperationError::MissingAttribute(Attribute::DisplayName))?;
-                "Missing attribute: {}",
-                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(
+        let spn = $value
-            OperationError::InvalidAccountState(format!("Missing attribute: {}", Attribute::Spn)),
+            .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(|| {
+                .ok_or_else(|| OperationError::MissingAttribute(Attribute::GidNumber))?;
-                    OperationError::InvalidAccountState(format!(
-                        "Missing attribute: {}",
-                        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!(
+                return Err(OperationError::MissingClass(
-                    "Missing class: {}",
+                    ENTRYCLASS_POSIX_ACCOUNT.into(),
-                    EntryClass::PosixAccount
+                ));
-                )));
             }
         };
 
@@ -929,10 +914,7 @@ impl<'a> IdmServerProxyWriteTransaction<'a> {
                     "Invalid entry, {} attribute is not present or not iutf8",
                     Attribute::Class
                 );
-                OperationError::InvalidAccountState(format!(
+                OperationError::MissingAttribute(Attribute::Class)
-                    "Missing attribute: {}",
-                    Attribute::Class
-                ))
             })?
             .collect();
 

server/lib/src/idm/application.rs

@@ -23,9 +23,7 @@ impl Application {
         _qs: &mut QueryServerReadTransaction,
     ) -> Result<Self, OperationError> {
         if !value.attribute_equality(Attribute::Class, &EntryClass::Application.to_partialvalue()) {
-            return Err(OperationError::InvalidAccountState(
+            return Err(OperationError::MissingClass(ENTRYCLASS_APPLICATION.into()));
-                "Missing class: application".to_string(),
-            ));
         }
 
         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(|| {
+            .ok_or_else(|| OperationError::MissingAttribute(Attribute::Name))?;
-                OperationError::InvalidAccountState(format!(
-                    "Missing attribute: {}",
-                    Attribute::Name
-                ))
-            })?;
 
         let linked_group = value
             .get_ava_single_refer(Attribute::LinkedGroup)
-            .ok_or_else(|| {
+            .ok_or_else(|| OperationError::MissingAttribute(Attribute::LinkedGroup))?;
-                OperationError::InvalidAccountState(format!(
-                    "Missing attribute: {}",
-                    Attribute::LinkedGroup
-                ))
-            })?;
 
         Ok(Application {
             name,

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(
+        let spn = $value
-            OperationError::InvalidAccountState(format!("Missing attribute: {}", Attribute::Spn)),
+            .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(
+            let name = $value
-                OperationError::InvalidAccountState(format!(
+                .get_ava_single_proto_string(Attribute::Name)
-                    "Missing attribute: {}",
+                .ok_or(OperationError::MissingAttribute(Attribute::Name))?;
-                    Attribute::Name
-                )),
-            )?;
 
-            let gidnumber = $value.get_ava_single_uint32(Attribute::GidNumber).ok_or(
+            let gidnumber = $value
-                OperationError::InvalidAccountState(format!(
+                .get_ava_single_uint32(Attribute::GidNumber)
-                    "Missing attribute: {}",
+                .ok_or(OperationError::MissingAttribute(Attribute::GidNumber))?;
-                    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(
+        let spn = $value
-            OperationError::InvalidAccountState(format!("Missing attribute: {}", Attribute::Spn)),
+            .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<EntrySealed, EntryCommitted>,
     ) -> Result<Self, OperationError> {
         if !value.attribute_equality(Attribute::Class, &EntryClass::Group.into()) {
-            return Err(OperationError::InvalidAccountState(
+            return Err(OperationError::MissingAttribute(Attribute::Group));
-                "Missing class: group".to_string(),
-            ));
         }
 
         // 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(|| {
+            .ok_or_else(|| OperationError::MissingAttribute(Attribute::Spn))?;
-                OperationError::InvalidAccountState("Missing attribute: spn".to_string())
-            })?;
 
         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(|| {
+            .ok_or_else(|| OperationError::MissingAttribute(Attribute::Name))?;
-                OperationError::InvalidAccountState(format!(
-                    "Missing attribute: {}",
-                    Attribute::Name
-                ))
-            })?;
 
         let spn = $value
             .get_ava_single_proto_string(Attribute::Spn)
-            .ok_or_else(|| {
+            .ok_or_else(|| OperationError::MissingAttribute(Attribute::Spn))?;
-                OperationError::InvalidAccountState(format!(
-                    "Missing attribute: {}",
-                    Attribute::Spn
-                ))
-            })?;
 
         let uuid = $value.get_uuid();
 
         let gidnumber = $value
             .get_ava_single_uint32(Attribute::GidNumber)
-            .ok_or_else(|| {
+            .ok_or_else(|| OperationError::MissingAttribute(Attribute::GidNumber))?;
-                OperationError::InvalidAccountState(format!(
-                    "Missing attribute: {}",
-                    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!(
+            return Err(OperationError::MissingClass(ENTRYCLASS_ACCOUNT.into()));
-                "Missing class: {}",
-                EntryClass::Account
-            )));
         }
 
         if !$value.attribute_equality(
             Attribute::Class,
             &EntryClass::PosixAccount.to_partialvalue(),
         ) {
-            return Err(OperationError::InvalidAccountState(format!(
+            return Err(OperationError::MissingClass(
-                "Missing class: {}",
+                ENTRYCLASS_POSIX_ACCOUNT.into(),
-                EntryClass::PosixAccount
+            ));
-            )));
         }
 
         let name = $value
             .get_ava_single_iname(Attribute::Name)
             .map(|s| s.to_string())
-            .ok_or_else(|| {
+            .ok_or_else(|| OperationError::MissingAttribute(Attribute::Name))?;
-                OperationError::InvalidAccountState(format!(
-                    "Missing attribute: {}",
-                    Attribute::Name
-                ))
-            })?;
 
         let spn = $value
             .get_ava_single_proto_string(Attribute::Spn)
-            .ok_or_else(|| {
+            .ok_or_else(|| OperationError::MissingAttribute(Attribute::Spn))?;
-                OperationError::InvalidAccountState(format!(
-                    "Missing attribute: {}",
-                    Attribute::Spn
-                ))
-            })?;
 
         let uuid = $value.get_uuid();
 
         let gidnumber = $value
             .get_ava_single_uint32(Attribute::GidNumber)
-            .ok_or_else(|| {
+            .ok_or_else(|| OperationError::MissingAttribute(Attribute::GidNumber))?;
-                OperationError::InvalidAccountState(format!(
-                    "Missing attribute: {}",
-                    Attribute::GidNumber
-                ))
-            })?;
 
         // This is the user private group.
         let upg = UnixGroup {

server/lib/src/idm/radius.rs

@@ -25,42 +25,25 @@ impl RadiusAccount {
         qs: &mut QueryServerReadTransaction,
     ) -> Result<Self, OperationError> {
         if !value.attribute_equality(Attribute::Class, &EntryClass::Account.into()) {
-            return Err(OperationError::InvalidAccountState(
+            return Err(OperationError::MissingClass(ENTRYCLASS_ACCOUNT.into()));
-                "Missing class: account".to_string(),
-            ));
         }
 
         let radius_secret = value
             .get_ava_single_secret(Attribute::RadiusSecret)
-            .ok_or_else(|| {
+            .ok_or_else(|| OperationError::MissingAttribute(Attribute::RadiusSecret))?
-                OperationError::InvalidAccountState(format!(
-                    "Missing attribute: {}",
-                    Attribute::RadiusSecret
-                ))
-            })?
             .to_string();
 
         let name = value
             .get_ava_single_iname(Attribute::Name)
             .map(|s| s.to_string())
-            .ok_or_else(|| {
+            .ok_or_else(|| OperationError::MissingAttribute(Attribute::Name))?;
-                OperationError::InvalidAccountState(format!(
-                    "Missing attribute: {}",
-                    Attribute::Name
-                ))
-            })?;
 
         let uuid = value.get_uuid();
 
         let displayname = value
             .get_ava_single_utf8(Attribute::DisplayName)
             .map(|s| s.to_string())
-            .ok_or_else(|| {
+            .ok_or_else(|| OperationError::MissingAttribute(Attribute::DisplayName))?;
-                OperationError::InvalidAccountState(format!(
-                    "Missing attribute: {}",
-                    Attribute::DisplayName
-                ))
-            })?;
 
         let groups = Group::try_from_account_entry_reduced(value, qs)?;
 

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(
+            return Err(OperationError::MissingClass(ENTRYCLASS_SYNC_ACCOUNT.into()));
-                "Missing class: sync account".to_string(),
-            ));
         }
 
         let name = $value
             .get_ava_single_iname(Attribute::Name)
             .map(|s| s.to_string())
-            .ok_or(OperationError::InvalidAccountState(
+            .ok_or(OperationError::MissingAttribute(Attribute::Name))?;
-                "Missing attribute: name".to_string(),
-            ))?;
 
         let jws_key = $value
             .get_ava_single_jws_key_es256(Attribute::JwsEs256PrivateKey)

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(|| {
+                    .ok_or_else(|| OperationError::MissingAttribute(Attribute::Spn))?;
-                        OperationError::InvalidAccountState(format!(
-                            "Missing attribute: {}",
-                            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!(
+            return Err(OperationError::MissingClass(
-                "Missing class: {}",
+                ENTRYCLASS_POSIX_ACCOUNT.into(),
-                EntryClass::PosixAccount
+            ));
-            )));
         }
 
         // 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!(
+                return Err(OperationError::MissingClass(
-                    "Missing class: {}",
+                    ENTRYCLASS_POSIX_ACCOUNT.into(),
-                    EntryClass::PosixAccount
+                ));
-                )));
             }
         };
 

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(
+            return Err(OperationError::MissingClass(
-                "Missing class: service account".to_string(),
+                ENTRYCLASS_SERVICE_ACCOUNT.into(),
             ));
         }
 

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(),
+            ))
         }
     }
 }

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.

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(_)),