From 43b7f805351ef41fcfff4abea07acc5044f9e70e Mon Sep 17 00:00:00 2001
From: Firstyear <william@blackhats.net.au>
Date: Thu, 6 Feb 2025 08:50:45 +1000
Subject: [PATCH] Correctly return that uuid2spn changed on domain rename
 (#3402)

Due to a missing equality check in value, when a domain
rename occured, the uuid2spn index differential function
did not correctly detect that the domain name had updated
which meant that the uuid2spn index was not updated. Only
this index was affected, and a manual reindex would
resolve.
---
 server/lib/src/plugins/spn.rs | 53 +++++++++++++++++++++++++++++------
 server/lib/src/value.rs       |  3 +-
 2 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/server/lib/src/plugins/spn.rs b/server/lib/src/plugins/spn.rs
index 6a6e759d6..1ba210991 100644
--- a/server/lib/src/plugins/spn.rs
+++ b/server/lib/src/plugins/spn.rs
@@ -208,10 +208,7 @@ impl Spn {
         // All we do is purge spn, and allow the plugin to recreate. Neat! It's also all still
         // within the transaction, just in case!
         qs.internal_modify(
-            &filter!(f_or!([
-                f_eq(Attribute::Class, EntryClass::Group.into()),
-                f_eq(Attribute::Class, EntryClass::Account.into()),
-            ])),
+            &filter!(f_pres(Attribute::Spn)),
             &modlist!([m_purge(Attribute::Spn)]),
         )
     }
@@ -332,12 +329,40 @@ mod tests {
     async fn test_spn_regen_domain_rename(server: &QueryServer) {
         let mut server_txn = server.write(duration_from_epoch_now()).await.unwrap();
 
-        let ex1 = Value::new_spn_str("admin", "example.com");
-        let ex2 = Value::new_spn_str("admin", "new.example.com");
+        let ex1 = Value::new_spn_str("a_testperson1", "example.com");
+        let ex2 = Value::new_spn_str("a_testperson1", "new.example.com");
+
+        let t_uuid = Uuid::new_v4();
+        let g_uuid = Uuid::new_v4();
+
+        assert!(server_txn
+            .internal_create(vec![
+                entry_init!(
+                    (Attribute::Class, EntryClass::Object.to_value()),
+                    (Attribute::Class, EntryClass::Account.to_value()),
+                    (Attribute::Class, EntryClass::Person.to_value()),
+                    (Attribute::Name, Value::new_iname("a_testperson1")),
+                    (Attribute::Uuid, Value::Uuid(t_uuid)),
+                    (Attribute::Description, Value::new_utf8s("testperson1")),
+                    (Attribute::DisplayName, Value::new_utf8s("testperson1"))
+                ),
+                entry_init!(
+                    (Attribute::Class, EntryClass::Object.to_value()),
+                    (Attribute::Class, EntryClass::Group.to_value()),
+                    (Attribute::Name, Value::new_iname("testgroup")),
+                    (Attribute::Uuid, Value::Uuid(g_uuid)),
+                    (Attribute::Member, Value::Refer(t_uuid))
+                ),
+            ])
+            .is_ok());
+
+        assert!(server_txn.commit().is_ok());
+        let mut server_txn = server.write(duration_from_epoch_now()).await.unwrap();
+
         // get the current domain name
         // check the spn on admin is admin@<initial domain>
         let e_pre = server_txn
-            .internal_search_uuid(UUID_ADMIN)
+            .internal_search_uuid(t_uuid)
             .expect("must not fail");
 
         let e_pre_spn = e_pre.get_ava_single(Attribute::Spn).expect("must not fail");
@@ -350,9 +375,12 @@ mod tests {
             .danger_domain_rename("new.example.com")
             .expect("should not fail!");
 
+        assert!(server_txn.commit().is_ok());
+        let mut server_txn = server.write(duration_from_epoch_now()).await.unwrap();
+
         // check the spn on admin is admin@<new domain>
         let e_post = server_txn
-            .internal_search_uuid(UUID_ADMIN)
+            .internal_search_uuid(t_uuid)
             .expect("must not fail");
 
         let e_post_spn = e_post
@@ -362,6 +390,13 @@ mod tests {
         debug!("{:?}", ex2);
         assert_eq!(e_post_spn, ex2);
 
-        server_txn.commit().expect("Must not fail");
+        // Assert that the uuid2spn index updated.
+        let testuser_spn = server_txn
+            .uuid_to_spn(t_uuid)
+            .expect("Must be able to retrieve the spn")
+            .expect("Value must not be none");
+        assert_eq!(testuser_spn, ex2);
+
+        assert!(server_txn.commit().is_ok());
     }
 }
diff --git a/server/lib/src/value.rs b/server/lib/src/value.rs
index 08f6148de..0bab23b8a 100644
--- a/server/lib/src/value.rs
+++ b/server/lib/src/value.rs
@@ -1311,13 +1311,14 @@ impl PartialEq for Value {
             | (Value::Iname(a), Value::Iname(b))
             | (Value::Cred(a, _), Value::Cred(b, _))
             | (Value::SshKey(a, _), Value::SshKey(b, _))
-            | (Value::Spn(a, _), Value::Spn(b, _))
             | (Value::Nsuniqueid(a), Value::Nsuniqueid(b))
             | (Value::EmailAddress(a, _), Value::EmailAddress(b, _))
             | (Value::PhoneNumber(a, _), Value::PhoneNumber(b, _))
             | (Value::OauthScope(a), Value::OauthScope(b))
             | (Value::PublicBinary(a, _), Value::PublicBinary(b, _))
             | (Value::RestrictedString(a), Value::RestrictedString(b)) => a.eq(b),
+            // Spn - need to check both name and domain.
+            (Value::Spn(a, c), Value::Spn(b, d)) => a.eq(b) && c.eq(d),
             // Uuid, Refer
             (Value::Uuid(a), Value::Uuid(b)) | (Value::Refer(a), Value::Refer(b)) => a.eq(b),
             // Bool