From af88ecc18112c9309e9754492b49c332cc8548d1 Mon Sep 17 00:00:00 2001
From: Firstyear <william@blackhats.net.au>
Date: Wed, 16 Aug 2023 15:33:28 +1000
Subject: [PATCH] 1982 service account access (#1985)

* Fix issue with incorrect filter class preventing service account delete
---
 server/core/src/https/v1.rs     |  2 +-
 server/lib/src/entry.rs         | 12 +++++++++++-
 server/lib/src/server/delete.rs |  2 +-
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/server/core/src/https/v1.rs b/server/core/src/https/v1.rs
index e987cd8a6..01087e4bd 100644
--- a/server/core/src/https/v1.rs
+++ b/server/core/src/https/v1.rs
@@ -477,7 +477,7 @@ pub async fn service_account_id_delete(
     Path(id): Path<String>,
     Extension(kopid): Extension<KOpId>,
 ) -> impl IntoResponse {
-    let filter = filter_all!(f_eq("class", PartialValue::new_class("service_accont")));
+    let filter = filter_all!(f_eq("class", PartialValue::new_class("service_account")));
     json_rest_event_delete_id(state, id, filter, kopid).await
 }
 
diff --git a/server/lib/src/entry.rs b/server/lib/src/entry.rs
index e6f419c3e..9a721054e 100644
--- a/server/lib/src/entry.rs
+++ b/server/lib/src/entry.rs
@@ -1902,6 +1902,9 @@ impl<STATE> Entry<EntryValid, STATE> {
             return Ok(());
         };
 
+        // Are we in the recycle bin? We soften some checks if we are.
+        let recycled = self.attribute_equality("class", &PVCLASS_RECYCLED);
+
         // Do we have extensible? We still validate syntax of attrs but don't
         // check for valid object structures.
         let extensible = self.attribute_equality("class", &PVCLASS_EXTENSIBLE);
@@ -2023,7 +2026,14 @@ impl<STATE> Entry<EntryValid, STATE> {
                 "Validation error, the following required (must) attributes are missing - {:?}",
                 missing_must
             );
-            return Err(SchemaError::MissingMustAttribute(missing_must));
+            // We if are in the recycle bin, we don't hard error here. This can occur when
+            // a migration occurs and we delete an acp, and then the related group. Because
+            // this would trigger refint which purges the acp_receiver_group, then this
+            // must value becomes unsatisfiable. So here we soften the check for recycled
+            // entries because they are in a "nebulous" state anyway.
+            if !recycled {
+                return Err(SchemaError::MissingMustAttribute(missing_must));
+            }
         }
 
         if extensible {
diff --git a/server/lib/src/server/delete.rs b/server/lib/src/server/delete.rs
index 39d73197a..6b5ddd934 100644
--- a/server/lib/src/server/delete.rs
+++ b/server/lib/src/server/delete.rs
@@ -163,7 +163,7 @@ impl<'a> QueryServerWriteTransaction<'a> {
         self.delete(&de)
     }
 
-    #[instrument(level = "debug", skip_all)]
+    #[instrument(level = "debug", skip(self))]
     pub fn internal_delete_uuid_if_exists(
         &mut self,
         target_uuid: Uuid,