From 7653e1a4c31d904de918b5bc7453a8248bb56b82 Mon Sep 17 00:00:00 2001
From: mdecimus <mauro@stalw.art>
Date: Mon, 8 Sep 2025 10:27:52 +0200
Subject: [PATCH] OIDC: Do not overwrite local aliases (fixes #2065)

---
 crates/directory/src/backend/ldap/lookup.rs |  2 +-
 crates/directory/src/backend/oidc/lookup.rs |  2 +-
 crates/directory/src/backend/sql/lookup.rs  |  3 +-
 crates/directory/src/core/principal.rs      | 40 ++++++++++++++++-----
 4 files changed, 34 insertions(+), 13 deletions(-)

diff --git a/crates/directory/src/backend/ldap/lookup.rs b/crates/directory/src/backend/ldap/lookup.rs
index 8c0760eb..2f0acdc7 100644
--- a/crates/directory/src/backend/ldap/lookup.rs
+++ b/crates/directory/src/backend/ldap/lookup.rs
@@ -270,7 +270,7 @@ impl LdapDirectory {
         };
 
         // Keep the internal store up to date with the LDAP server
-        let changes = principal.update_external(external_principal);
+        let changes = principal.update_external(external_principal, true);
         if !changes.is_empty() {
             self.data_store
                 .update_principal(
diff --git a/crates/directory/src/backend/oidc/lookup.rs b/crates/directory/src/backend/oidc/lookup.rs
index 105cf840..02b6bf67 100644
--- a/crates/directory/src/backend/oidc/lookup.rs
+++ b/crates/directory/src/backend/oidc/lookup.rs
@@ -104,7 +104,7 @@ impl OpenIdDirectory {
                             .ok_or_else(|| manage::not_found(id).caused_by(trc::location!()))?;
 
                         // Keep the internal store up to date with the OIDC server
-                        let changes = principal.update_external(external_principal);
+                        let changes = principal.update_external(external_principal, false);
                         if !changes.is_empty() {
                             self.data_store
                                 .update_principal(
diff --git a/crates/directory/src/backend/sql/lookup.rs b/crates/directory/src/backend/sql/lookup.rs
index 17b8915a..494b515c 100644
--- a/crates/directory/src/backend/sql/lookup.rs
+++ b/crates/directory/src/backend/sql/lookup.rs
@@ -15,7 +15,6 @@ use crate::{
         },
     },
 };
-
 use mail_send::Credentials;
 use store::{NamedRows, Rows, Value};
 use trc::AddContext;
@@ -188,7 +187,7 @@ impl SqlDirectory {
         };
 
         // Keep the internal store up to date with the SQL server
-        let changes = principal.update_external(external_principal);
+        let changes = principal.update_external(external_principal, true);
         if !changes.is_empty() {
             self.data_store
                 .update_principal(
diff --git a/crates/directory/src/core/principal.rs b/crates/directory/src/core/principal.rs
index 9ffb9661..6320e4ef 100644
--- a/crates/directory/src/core/principal.rs
+++ b/crates/directory/src/core/principal.rs
@@ -258,7 +258,11 @@ impl Principal {
         }
     }
 
-    pub fn update_external(&mut self, mut external: Principal) -> Vec<PrincipalUpdate> {
+    pub fn update_external(
+        &mut self,
+        mut external: Principal,
+        overwrite_emails: bool,
+    ) -> Vec<PrincipalUpdate> {
         let mut updates = Vec::new();
 
         // Add external members
@@ -284,16 +288,34 @@ impl Principal {
             ));
         }
 
-        for (name, field, external_field) in [
-            (PrincipalField::Secrets, &mut self.secrets, external.secrets),
-            (PrincipalField::Emails, &mut self.emails, external.emails),
-        ] {
-            if !external_field.is_empty() && &external_field != field {
-                *field = external_field;
+        if !external.secrets.is_empty() && external.secrets != self.secrets {
+            self.secrets = external.secrets;
+            updates.push(PrincipalUpdate::set(
+                PrincipalField::Secrets,
+                PrincipalValue::StringList(self.secrets.clone()),
+            ));
+        }
+
+        if !external.emails.is_empty() && external.emails != self.emails {
+            if overwrite_emails {
+                self.emails = external.emails;
                 updates.push(PrincipalUpdate::set(
-                    name,
-                    PrincipalValue::StringList(field.clone()),
+                    PrincipalField::Emails,
+                    PrincipalValue::StringList(self.emails.clone()),
                 ));
+            } else {
+                // Missing emails are appended to avoid overwriting locally defined aliases
+                // This means that old email addresses need to be deleted either manually or using the API
+                for email in external.emails {
+                    let email = email.to_lowercase();
+                    if !self.emails.contains(&email) {
+                        updates.push(PrincipalUpdate::add_item(
+                            PrincipalField::Emails,
+                            PrincipalValue::String(email.clone()),
+                        ));
+                        self.emails.push(email);
+                    }
+                }
             }
         }