From 3077d86aa1cb77a83bc7c6bdf8c400c0a57f5f65 Mon Sep 17 00:00:00 2001
From: ToxicMushroom <32853531+ToxicMushroom@users.noreply.github.com>
Date: Sun, 6 Apr 2025 04:22:56 +0200
Subject: [PATCH] final fixes

---
 proto/src/scim_v1/mod.rs                      |   2 +-
 server/core/src/https/views/profile.rs        |  52 ++++++---
 .../profile_changes_partial.html              | 106 ++++++++++--------
 3 files changed, 93 insertions(+), 67 deletions(-)

diff --git a/proto/src/scim_v1/mod.rs b/proto/src/scim_v1/mod.rs
index db9c7a8aa..5750e9572 100644
--- a/proto/src/scim_v1/mod.rs
+++ b/proto/src/scim_v1/mod.rs
@@ -71,7 +71,7 @@ pub enum ScimSchema {
 }
 
 #[serde_as]
-#[derive(Deserialize, Serialize, Debug, Clone, ToSchema)]
+#[derive(Deserialize, Serialize, PartialEq, Eq, Debug, Clone, ToSchema)]
 #[serde(deny_unknown_fields, rename_all = "camelCase")]
 pub struct ScimMail {
     #[serde(default)]
diff --git a/server/core/src/https/views/profile.rs b/server/core/src/https/views/profile.rs
index 2c6ab892a..63cbf1603 100644
--- a/server/core/src/https/views/profile.rs
+++ b/server/core/src/https/views/profile.rs
@@ -17,7 +17,7 @@ use axum_extra::extract::Form;
 use axum_htmx::{HxEvent, HxPushUrl, HxResponseTrigger};
 use futures_util::TryFutureExt;
 use kanidm_proto::attribute::Attribute;
-use kanidm_proto::constants::{ATTR_DISPLAYNAME, ATTR_MAIL};
+use kanidm_proto::constants::{ATTR_DISPLAYNAME, ATTR_MAIL, ATTR_NAME};
 use kanidm_proto::internal::UserAuthToken;
 use kanidm_proto::scim_v1::server::{ScimEffectiveAccess, ScimPerson};
 use kanidm_proto::scim_v1::ScimMail;
@@ -63,9 +63,9 @@ pub(crate) struct SaveProfileQuery {
 #[derive(Clone, Debug, Serialize, Deserialize)]
 pub(crate) struct CommitSaveProfileQuery {
     #[serde(rename = "account_name")]
-    account_name: String,
+    account_name: Option<String>,
     #[serde(rename = "display_name")]
-    display_name: String,
+    display_name: Option<String>,
     #[serde(rename = "emails[]")]
     emails: Vec<String>,
     #[serde(rename = "new_primary_mail")]
@@ -88,6 +88,7 @@ struct ProfileChangesPartialView {
     primary_mail: Option<String>,
     new_attrs: ProfileAttributes,
     new_primary_mail: Option<String>,
+    emails_are_same: bool,
 }
 
 #[derive(Template, Clone)]
@@ -186,6 +187,8 @@ pub(crate) async fn view_profile_diff_start_save_post(
         .find(|sm| sm.primary)
         .map(|sm| sm.value.clone());
 
+    let emails_are_same = scim_person.mails == new_mails;
+
     let profile_view = ProfileChangesPartialView {
         menu_active_item: ProfileMenuItems::UserProfile,
         can_rw,
@@ -196,6 +199,7 @@ pub(crate) async fn view_profile_diff_start_save_post(
             display_name: query.display_name,
             emails: new_mails,
         },
+        emails_are_same,
         new_primary_mail: query.emails.get(primary_index).cloned(),
     };
 
@@ -219,23 +223,37 @@ pub(crate) async fn view_profile_diff_confirm_save_post(
         .handle_whoami_uat(client_auth_info.clone(), kopid.eventid)
         .map_err(|op_err| HtmxError::new(&kopid, op_err, domain_info.clone()))
         .await?;
-    dbg!(&query);
 
     let filter = filter_all!(f_and!([f_id(uat.uuid.to_string().as_str())]));
 
-    state
-        .qe_w_ref
-        .handle_setattribute(
-            client_auth_info.clone(),
-            uat.uuid.to_string(),
-            ATTR_DISPLAYNAME.to_string(),
-            vec![query.display_name],
-            filter.clone(),
-            kopid.eventid,
-        )
-        .map_err(|op_err| HtmxError::new(&kopid, op_err, domain_info.clone()))
-        .await?;
-
+    if let Some(account_name) = query.account_name {
+        state
+            .qe_w_ref
+            .handle_setattribute(
+                client_auth_info.clone(),
+                uat.uuid.to_string(),
+                ATTR_NAME.to_string(),
+                vec![account_name],
+                filter.clone(),
+                kopid.eventid,
+            )
+            .map_err(|op_err| HtmxError::new(&kopid, op_err, domain_info.clone()))
+            .await?;
+    }
+    if let Some(display_name) = query.display_name {
+        state
+            .qe_w_ref
+            .handle_setattribute(
+                client_auth_info.clone(),
+                uat.uuid.to_string(),
+                ATTR_DISPLAYNAME.to_string(),
+                vec![display_name],
+                filter.clone(),
+                kopid.eventid,
+            )
+            .map_err(|op_err| HtmxError::new(&kopid, op_err, domain_info.clone()))
+            .await?;
+    }
     let mut emails = query.emails;
     if let Some(primary) = query.new_primary_mail {
         emails.insert(0, primary);
diff --git a/server/core/templates/user_settings/profile_changes_partial.html b/server/core/templates/user_settings/profile_changes_partial.html
index 43229b24e..36b5c59ba 100644
--- a/server/core/templates/user_settings/profile_changes_partial.html
+++ b/server/core/templates/user_settings/profile_changes_partial.html
@@ -9,79 +9,87 @@ Profile Difference
 (% block settings_window %)
 
 <form>
+    (% if person.name != new_attrs.account_name %)
     <input type="hidden" name="account_name" value="(( new_attrs.account_name ))"/>
+    (% endif %)
+    (% if person.displayname != new_attrs.display_name %)
     <input type="hidden" name="display_name" value="(( new_attrs.display_name ))"/>
-<!--    <input type="hidden" name="legal_name" value=" new_attrs.legal_name "/>-->
+    (% endif %)
+
     (% for email in new_attrs.emails %)
-        (% if !email.primary %)
-            <input type="hidden" name="emails[]" value="(( email.value ))"/>
-        (% endif %)
+    (% if !email.primary %)
+    <input type="hidden" name="emails[]" value="(( email.value ))"/>
+    (% endif %)
     (% endfor %)
     (% if let Some(new_primary_mail) = new_primary_mail %)
-        <input type="hidden" name="new_primary_mail" value="(( new_primary_mail ))"/>
+    <input type="hidden" name="new_primary_mail" value="(( new_primary_mail ))"/>
     (% endif %)
 
     <table class="table table-bordered overflow-x-scroll">
         <thead>
-            <tr>
-                <th scope="col">Attribute</th>
-                <th scope="col">Old value</th>
-                <th scope="col">New value</th>
-            </tr>
+        <tr>
+            <th scope="col">Attribute</th>
+            <th scope="col">Old value</th>
+            <th scope="col">New value</th>
+        </tr>
         </thead>
-    (% if person.name != new_attrs.account_name %)
+        (% if person.name != new_attrs.account_name %)
         <tr>
             <th scope="row">Username</th>
             <td class="text-break">(( person.name ))</td>
             <td class="text-break">(( new_attrs.account_name ))</td>
         </tr>
-    (% endif %)
+        (% endif %)
 
-    (% if person.displayname != new_attrs.display_name %)
+        (% if person.displayname != new_attrs.display_name %)
         <tr>
             <th scope="row">Display name</th>
             <td class="text-break">(( person.displayname ))</td>
             <td class="text-break">(( new_attrs.display_name ))</td>
         </tr>
-    (% endif %)
-    <!-- TODO: List new items with +, same items with . -->
-    <tr>
-        <th scope="row">Primary Emails</th>
-        <td class="text-nowrap">
-            (( primary_mail.clone().unwrap_or("none".to_string()) ))
-        </td>
-        <td class="text-nowrap">
-            (( new_primary_mail.clone().unwrap_or("none".to_string()) ))
-        </td>
-    </tr>
-    <tr>
-        <th scope="row">Secondary Emails</th>
-        <td class="text-break">
-            <ul class="ps-3">
-                (% for email in person.mails %)
-                (% if !email.primary %)
-                <li class="text-nowrap">(( email.value ))</li>
-                (% endif %)
-                (% endfor %)
-            </ul>
-        </td>
-        <td class="text-break">
-            <ul class="ps-3">
-                (% for email in new_attrs.emails %)
-                (% if !email.primary %)
-                <li class="text-nowrap">(( email.value ))</li>
-                (% endif %)
-                (% endfor %)
-            </ul>
-        </td>
-    </tr>
-
+        (% endif %)
+        (% if !emails_are_same %)
+        <tr>
+            <th scope="row">Primary Emails</th>
+            <td class="text-nowrap">
+                (( primary_mail.clone().unwrap_or("none".to_string()) ))
+            </td>
+            <td class="text-nowrap">
+                (( new_primary_mail.clone().unwrap_or("none".to_string()) ))
+            </td>
+        </tr>
+        <tr>
+            <th scope="row">Secondary Emails</th>
+            <td class="text-break">
+                <ul class="ps-3">
+                    (% for email in person.mails %)
+                    (% if !email.primary %)
+                    <li class="text-nowrap">(( email.value ))</li>
+                    (% endif %)
+                    (% endfor %)
+                </ul>
+            </td>
+            <td class="text-break">
+                <ul class="ps-3">
+                    (% for email in new_attrs.emails %)
+                    (% if !email.primary %)
+                    <li class="text-nowrap">(( email.value ))</li>
+                    (% endif %)
+                    (% endfor %)
+                </ul>
+            </td>
+        </tr>
+        (% endif %)
     </table>
-    <!-- Edit button -->
+
     <div class="pt-4" hx-target="#user_settings_container" hx-swap="outerHTML">
         (% if can_rw %)
-        <button class="btn btn-danger" type="button" hx-get="/ui/profile" hx-target="body" hx-swap="outerHTML">Discard Changes</button>
-        <button class="btn btn-primary" type="button" hx-post="/ui/api/user_settings/confirm_profile" hx-target="body" hx-swap="outerHTML">Confirm Changes</button>
+        <button class="btn btn-danger" type="button" hx-get="/ui/profile" hx-target="body" hx-swap="outerHTML">Discard
+            Changes
+        </button>
+        <button class="btn btn-primary" type="button" hx-post="/ui/api/user_settings/confirm_profile" hx-target="body"
+                hx-swap="outerHTML">Confirm Changes
+        </button>
         (% else %)
         <a href="/ui/profile/unlock" hx-boost="false">
             <!-- TODO: at the moment, session expiring here means progress is lost. Do we just show an error screen ? We can't pass the update state through the reauth session, and we don't have profile update sessions like cred update. -->