From 26cbb2d8c13449df2a82580eba6dc3aa2491f755 Mon Sep 17 00:00:00 2001
From: ToxicMushroom <32853531+ToxicMushroom@users.noreply.github.com>
Date: Sat, 5 Apr 2025 03:02:22 +0200
Subject: [PATCH] Add primary email selection, update email add button. Use
 scim.

---
 server/core/src/https/views/admin/mod.rs      |  2 +-
 server/core/src/https/views/admin/persons.rs  | 21 ++---
 server/core/src/https/views/profile.rs        | 92 ++++++------------
 server/core/static/external/forms.js          |  2 +-
 server/core/static/style.css                  |  4 +
 .../profile_changes_partial.html              | 26 ++----
 .../user_settings_profile_partial.html        | 93 ++++++++++++-------
 7 files changed, 107 insertions(+), 133 deletions(-)

diff --git a/server/core/src/https/views/admin/mod.rs b/server/core/src/https/views/admin/mod.rs
index e728e33b0..a0c826942 100644
--- a/server/core/src/https/views/admin/mod.rs
+++ b/server/core/src/https/views/admin/mod.rs
@@ -3,7 +3,7 @@ use axum::routing::get;
 use axum::Router;
 use axum_htmx::HxRequestGuardLayer;
 
-mod persons;
+pub(crate) mod persons;
 
 pub fn admin_router() -> Router<ServerState> {
     let unguarded_router = Router::new()
diff --git a/server/core/src/https/views/admin/persons.rs b/server/core/src/https/views/admin/persons.rs
index e1a2d723f..385e7b0f0 100644
--- a/server/core/src/https/views/admin/persons.rs
+++ b/server/core/src/https/views/admin/persons.rs
@@ -1,3 +1,4 @@
+use crate::https::errors::WebError;
 use crate::https::extractors::{DomainInfo, VerifiedClientInformation};
 use crate::https::middleware::KOpId;
 use crate::https::views::errors::HtmxError;
@@ -7,17 +8,15 @@ use crate::https::ServerState;
 use askama::Template;
 use axum::extract::{Path, State};
 use axum::http::Uri;
-use axum::response::{ErrorResponse, IntoResponse, Response};
+use axum::response::{IntoResponse, Response};
 use axum::Extension;
 use axum_htmx::{HxPushUrl, HxRequest};
-use futures_util::TryFutureExt;
 use kanidm_proto::attribute::Attribute;
 use kanidm_proto::internal::OperationError;
 use kanidm_proto::scim_v1::client::ScimFilter;
 use kanidm_proto::scim_v1::server::{ScimEffectiveAccess, ScimEntryKanidm, ScimPerson};
 use kanidm_proto::scim_v1::ScimEntryGetQuery;
 use kanidmd_lib::constants::EntryClass;
-use kanidmd_lib::idm::server::DomainInfoRead;
 use kanidmd_lib::idm::ClientAuthInfo;
 use std::str::FromStr;
 use uuid::Uuid;
@@ -70,7 +69,7 @@ pub(crate) async fn view_person_view_get(
     DomainInfo(domain_info): DomainInfo,
 ) -> axum::response::Result<Response> {
     let (person, scim_effective_access) =
-        get_person_info(uuid, state, &kopid, client_auth_info, domain_info.clone()).await?;
+        get_person_info(uuid, state, &kopid, client_auth_info).await?;
     let person_partial = PersonViewPartial {
         person,
         scim_effective_access,
@@ -101,7 +100,7 @@ pub(crate) async fn view_persons_get(
     DomainInfo(domain_info): DomainInfo,
     VerifiedClientInformation(client_auth_info): VerifiedClientInformation,
 ) -> axum::response::Result<Response> {
-    let persons = get_persons_info(state, &kopid, client_auth_info, domain_info.clone()).await?;
+    let persons = get_persons_info(state, &kopid, client_auth_info).await?;
     let persons_partial = PersonsPartialView { persons };
 
     let push_url = HxPushUrl(Uri::from_static("/ui/admin/persons"));
@@ -119,13 +118,12 @@ pub(crate) async fn view_persons_get(
     })
 }
 
-async fn get_person_info(
+pub async fn get_person_info(
     uuid: Uuid,
     state: ServerState,
     kopid: &KOpId,
     client_auth_info: ClientAuthInfo,
-    domain_info: DomainInfoRead,
-) -> Result<(ScimPerson, ScimEffectiveAccess), ErrorResponse> {
+) -> Result<(ScimPerson, ScimEffectiveAccess), WebError> {
     let scim_entry: ScimEntryKanidm = state
         .qe_r_ref
         .scim_entry_id_get(
@@ -138,13 +136,12 @@ async fn get_person_info(
                 ext_access_check: true,
             },
         )
-        .map_err(|op_err| HtmxError::new(kopid, op_err, domain_info.clone()))
         .await?;
 
     if let Some(personinfo_info) = scimentry_into_personinfo(scim_entry) {
         Ok(personinfo_info)
     } else {
-        Err(HtmxError::new(kopid, OperationError::InvalidState, domain_info.clone()).into())
+        Err(WebError::from(OperationError::InvalidState))
     }
 }
 
@@ -152,8 +149,7 @@ async fn get_persons_info(
     state: ServerState,
     kopid: &KOpId,
     client_auth_info: ClientAuthInfo,
-    domain_info: DomainInfoRead,
-) -> Result<Vec<(ScimPerson, ScimEffectiveAccess)>, ErrorResponse> {
+) -> Result<Vec<(ScimPerson, ScimEffectiveAccess)>, WebError> {
     let filter = ScimFilter::Equal(Attribute::Class.into(), EntryClass::Person.into());
 
     let base: Vec<ScimEntryKanidm> = state
@@ -167,7 +163,6 @@ async fn get_persons_info(
                 ext_access_check: true,
             },
         )
-        .map_err(|op_err| HtmxError::new(kopid, op_err, domain_info.clone()))
         .await?;
 
     // TODO: inefficient to sort here
diff --git a/server/core/src/https/views/profile.rs b/server/core/src/https/views/profile.rs
index 99ff596fc..fbfa0c581 100644
--- a/server/core/src/https/views/profile.rs
+++ b/server/core/src/https/views/profile.rs
@@ -1,3 +1,4 @@
+use kanidm_proto::attribute::Attribute;
 use super::constants::{ProfileMenuItems, Urls};
 use super::errors::HtmxError;
 use super::login::{LoginDisplayCtx, Reauth, ReauthPurpose};
@@ -16,13 +17,11 @@ use axum_extra::extract::cookie::CookieJar;
 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_LEGALNAME, ATTR_MAIL};
+use kanidm_proto::constants::{ATTR_DISPLAYNAME, ATTR_MAIL};
 use kanidm_proto::internal::UserAuthToken;
-use kanidm_proto::v1::Entry;
-use kanidmd_lib::filter::{f_eq, f_id, Filter};
+use kanidm_proto::scim_v1::server::{ScimEffectiveAccess, ScimPerson};
+use kanidmd_lib::filter::{f_id, Filter};
 use kanidmd_lib::prelude::f_and;
-use kanidmd_lib::prelude::PartialValue;
 use kanidmd_lib::prelude::FC;
 use serde::Deserialize;
 use serde::Serialize;
@@ -42,17 +41,20 @@ pub(crate) struct ProfileView {
 struct ProfilePartialView {
     menu_active_item: ProfileMenuItems,
     can_rw: bool,
-    attrs: ProfileAttributes
+    person: ScimPerson,
+    scim_effective_access: ScimEffectiveAccess,
 }
 
 #[derive(Clone, Debug, Serialize, Deserialize)]
 pub(crate) struct ProfileAttributes {
+    #[serde(rename = "name")]
     account_name: String,
+    #[serde(rename = "displayname")]
     display_name: String,
-    legal_name: String,
     #[serde(rename = "emails[]")]
     emails: Vec<String>,
-    primary_email: Option<String>,
+    // radio buttons are used to pick a primary index
+    primary_email_index: u16,
 }
 
 #[derive(Template, Clone)]
@@ -60,7 +62,7 @@ pub(crate) struct ProfileAttributes {
 struct ProfileChangesPartialView {
     menu_active_item: ProfileMenuItems,
     can_rw: bool,
-    attrs: ProfileAttributes,
+    person: ScimPerson,
     new_attrs: ProfileAttributes,
 }
 
@@ -92,19 +94,12 @@ pub(crate) async fn view_profile_get(
         .handle_whoami_uat(client_auth_info.clone(), kopid.eventid)
         .await?;
 
-    let filter = filter_all!(f_and!([f_eq(
-        Attribute::Uuid,
-        PartialValue::Uuid(uat.uuid)
-    )]));
-    let base: Vec<Entry> = state
-        .qe_r_ref
-        .handle_internalsearch(client_auth_info.clone(), filter, None, kopid.eventid)
-        .await?;
+    let (scim_person, scim_effective_access) = crate::https::views::admin::persons::get_person_info(
+        uat.uuid,
+        state,
+        &kopid,
+        client_auth_info.clone()).await?;
 
-    let self_entry = base.first().expect("Self no longer exists");
-    let empty = vec![];
-    let emails = self_entry.attrs.get(ATTR_MAIL).unwrap_or(&empty).clone();
-    let primary_email = emails.first().cloned();
 
     let time = time::OffsetDateTime::now_utc() + time::Duration::new(60, 0);
 
@@ -116,13 +111,8 @@ pub(crate) async fn view_profile_get(
         profile_partial: ProfilePartialView {
             menu_active_item: ProfileMenuItems::UserProfile,
             can_rw,
-            attrs: ProfileAttributes {
-                account_name: uat.name().to_string(),
-                display_name: uat.displayname.clone(),
-                legal_name: "hardcoded".to_string(),
-                emails,
-                primary_email,
-            },
+            person: scim_person,
+            scim_effective_access
         },
     })
 }
@@ -143,32 +133,17 @@ pub(crate) async fn view_profile_diff_start_save_post(
 
     let time = time::OffsetDateTime::now_utc() + time::Duration::new(60, 0);
     let can_rw = uat.purpose_readwrite_active(time);
-
-    let filter = filter_all!(f_and!([f_eq(
-        Attribute::Uuid,
-        PartialValue::Uuid(uat.uuid)
-    )]));
-    let base: Vec<Entry> = state
-        .qe_r_ref
-        .handle_internalsearch(client_auth_info.clone(), filter, None, kopid.eventid)
-        .map_err(|op_err| HtmxError::new(&kopid, op_err, domain_info))
-        .await?;
-
-    let self_entry = base.first().expect("Self no longer exists");
-    let empty = vec![];
-    let emails = self_entry.attrs.get(ATTR_MAIL).unwrap_or(&empty).clone();
-    let primary_email = emails.first().cloned();
+    // TODO: A bit overkill to request scimEffectiveAccess here.
+    let (scim_person, _) = crate::https::views::admin::persons::get_person_info(
+        uat.uuid,
+        state,
+        &kopid,
+        client_auth_info.clone()).await?;
 
     let profile_view = ProfileChangesPartialView {
         menu_active_item: ProfileMenuItems::UserProfile,
         can_rw,
-        attrs: ProfileAttributes {
-            account_name: uat.name().to_string(),
-            display_name: uat.displayname.clone(),
-            legal_name: "hardcoded".to_string(),
-            emails,
-            primary_email,
-        },
+        person: scim_person,
         new_attrs
     };
 
@@ -196,19 +171,6 @@ pub(crate) async fn view_profile_diff_confirm_save_post(
 
     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_LEGALNAME.to_string(),
-            vec![new_attrs.legal_name],
-            filter.clone(),
-            kopid.eventid,
-        )
-        .map_err(|op_err| HtmxError::new(&kopid, op_err, domain_info.clone()))
-        .await?;
-
     state
         .qe_w_ref
         .handle_setattribute(
@@ -280,10 +242,10 @@ pub(crate) async fn view_new_email_entry_partial(
     VerifiedClientInformation(_client_auth_info): VerifiedClientInformation,
     Extension(_kopid): Extension<KOpId>,
 ) -> axum::response::Result<Response> {
-    let passkey_init_trigger =
+    let add_email_trigger =
         HxResponseTrigger::after_swap([HxEvent::new("addEmailSwapped".to_string())]);
     Ok((
-        passkey_init_trigger,
+        add_email_trigger,
         FormModEntryModListPartial {
             can_rw: true,
             r#type: "email".to_string(),
diff --git a/server/core/static/external/forms.js b/server/core/static/external/forms.js
index c60b009d2..7e5bfe74c 100644
--- a/server/core/static/external/forms.js
+++ b/server/core/static/external/forms.js
@@ -3,7 +3,7 @@ function rehook_string_list_removers() {
     const buttons = document.getElementsByClassName("kanidm-remove-list-entry");
     for (let i = 0; i < buttons.length; i++) {
         const button = buttons.item(i)
-        if (button.getAttribute("kanidm_hooked") !== null) return
+        if (button.getAttribute("kanidm_hooked") !== null) continue
 
         button.addEventListener("click", (e) => {
             // Expected html nesting: li > div.input-group > button.kanidm-remove-list-entry
diff --git a/server/core/static/style.css b/server/core/static/style.css
index c90fdaac0..3566d8f3f 100644
--- a/server/core/static/style.css
+++ b/server/core/static/style.css
@@ -207,3 +207,7 @@ footer {
   height: var(--icon-size);
   transform: rotate(35deg);
 }
+
+.cursor-pointer:hover {
+  cursor: pointer;
+}
\ No newline at end of file
diff --git a/server/core/templates/user_settings/profile_changes_partial.html b/server/core/templates/user_settings/profile_changes_partial.html
index bc3451c32..6288868aa 100644
--- a/server/core/templates/user_settings/profile_changes_partial.html
+++ b/server/core/templates/user_settings/profile_changes_partial.html
@@ -11,7 +11,7 @@ Profile Difference
 <form>
     <input type="hidden" name="account_name" value="(( new_attrs.account_name ))"/>
     <input type="hidden" name="display_name" value="(( new_attrs.display_name ))"/>
-    <input type="hidden" name="legal_name" value="(( new_attrs.legal_name ))"/>
+<!--    <input type="hidden" name="legal_name" value=" new_attrs.legal_name "/>-->
     (% for email in new_attrs.emails %)
         <input type="hidden" name="emails[]" value="(( email ))"/>
     (% endfor %)
@@ -24,39 +24,30 @@ Profile Difference
                 <th scope="col">New value</th>
             </tr>
         </thead>
-    (% if attrs.account_name != new_attrs.account_name %)
+    (% if person.name != new_attrs.account_name %)
         <tr>
-            <th scope="row">User name</th>
-            <td class="text-break">(( attrs.account_name ))</td>
+            <th scope="row">Username</th>
+            <td class="text-break">(( person.name ))</td>
             <td class="text-break">(( new_attrs.account_name ))</td>
         </tr>
     (% endif %)
 
-    (% if attrs.display_name != new_attrs.display_name %)
+    (% if person.displayname != new_attrs.display_name %)
         <tr>
             <th scope="row">Display name</th>
-            <td class="text-break">(( attrs.display_name ))</td>
+            <td class="text-break">(( person.displayname ))</td>
             <td class="text-break">(( new_attrs.display_name ))</td>
         </tr>
     (% endif %)
 
-    (% if attrs.legal_name != new_attrs.legal_name %)
-        <tr>
-            <th scope="row">Legal name</th>
-            <td class="text-break">(( attrs.legal_name ))</td>
-            <td class="text-break">(( new_attrs.legal_name ))</td>
-        </tr>
-    (% endif %)
-
-    (% if attrs.emails != new_attrs.emails %)
 
         <!-- TODO: List new items with +, same items with . -->
         <tr>
             <th scope="row">Emails</th>
             <td class="text-break">
                 <ul>
-                (% for email in attrs.emails %)
-                    <li>(( email ))</li>
+                (% for email in person.mails %)
+                    <li>(( email.value ))</li>
                 (% endfor %)
                 </ul>
             </td>
@@ -68,7 +59,6 @@ Profile Difference
                 </ul>
             </td>
         </tr>
-    (% endif %)
     </table>
     <!-- Edit button -->
     <div class="pt-4" hx-target="#user_settings_container" hx-swap="outerHTML">
diff --git a/server/core/templates/user_settings_profile_partial.html b/server/core/templates/user_settings_profile_partial.html
index 193684c2f..2ab46ff69 100644
--- a/server/core/templates/user_settings_profile_partial.html
+++ b/server/core/templates/user_settings_profile_partial.html
@@ -4,56 +4,79 @@
 Profile
 (% endblock %)
 
+(% macro string_attr(dispname, name, value, editable, attribute) %)
+(% if scim_effective_access.search.check(attribute|as_ref) %)
+<div class="row g-0 mt-3">
+    <label for="person(( name ))" class="col-12 col-md-3 col-lg-2 col-form-label fw-bold py-0">(( dispname ))</label>
+    <div class="col-12 col-md-8 col-lg-6">
+        (% if scim_effective_access.modify_present.check(attribute|as_ref) %)
+        <input class="form-control py-0" id="person(( name ))" name="(( name ))" value="(( value ))">
+        (% else %)
+        <input readonly class="form-control-plaintext py-0" id="person(( name ))" name="(( name ))" value="(( value ))">
+        (% endif %)
+    </div>
+</div>
+(% endif %)
+(% endmacro %)
+
 (% block settings_window %)
 
-<form class="needs-validation" hx-post="/ui/api/user_settings/edit_profile" hx-target="#user_settings_container" hx-swap="outerHTML" hx-validate="true" hx-ext="bs-validation" novalidate>
-    <div class="mb-2 row">
-        <label for="profileUserName" class="col-12 col-md-3 col-lg-2 col-form-label">User name</label>
-        <div class="col-12 col-md-6 col-lg-5">
-            <input type="text" readonly class="form-control-plaintext" id="profileUserName" value="(( attrs.account_name ))">
-        </div>
-    </div>
-    
-    <div class="mb-2 row">
-        <label for="profileDisplayName" class="col-12 col-md-3 col-lg-2 col-form-label">Display name</label>
-        <div class="col-12 col-md-6 col-lg-5">
-            <input type="text" class="form-control-plaintext" id="profileDisplayName" value="(( attrs.display_name ))">
-        </div>
-    </div>
+<form id="user_settings_container" class="needs-validation" hx-post="/ui/api/user_settings/edit_profile"
+      hx-target="#user_settings_container" hx-swap="outerHTML" hx-validate="true" hx-ext="bs-validation" novalidate>
+    (% call string_attr("Name", "name", person.name, true, Attribute::Name) %)
 
-    <div class="mb-2 row">
-        <label for="profileLegalName" class="col-12 col-md-3 col-lg-2 col-form-label">Legal name</label>
-        <div class="col-12 col-md-6 col-lg-5">
-            <input type="text" class="form-control-plaintext" id="profileLegalName" value="(( attrs.legal_name ))">
-        </div>
-    </div>
+    (% call string_attr("Displayname", "displayname", person.displayname, true, Attribute::DisplayName) %)
 
     <div class="mb-2">
-        <label for="profileEmail" class="mb-2">Emails</label>
+        <div class="mt-3 mb-2 col-12 col-md-11 col-lg-8">
+            <label for="profileEmail" class="fw-bold">Email addresses (select primary)</label>
+            (% if can_rw %)
+            <a class="cursor-pointer float-end" hx-boost="true" hx-post="/ui/api/user_settings/add_email" hx-target="#emailAddresses"
+               hx-swap="beforeend">
+                <svg xmlns="http://www.w3.org/2000/svg" fill="currentColor" class="bi bi-plus-square"
+                     viewBox="0 0 16 16" width="20" height="20">
+                    <path d="M14 1a1 1 0 0 1 1 1v12a1 1 0 0 1-1 1H2a1 1 0 0 1-1-1V2a1 1 0 0 1 1-1zM2 0a2 2 0 0 0-2 2v12a2 2 0 0 0 2 2h12a2 2 0 0 0 2-2V2a2 2 0 0 0-2-2z"></path>
+                    <path d="M8 4a.5.5 0 0 1 .5.5v3h3a.5.5 0 0 1 0 1h-3v3a.5.5 0 0 1-1 0v-3h-3a.5.5 0 0 1 0-1h3v-3A.5.5 0 0 1 8 4"></path>
+                </svg>
+            </a>
+        </div>
+        (% endif %)
         <div>
-            <div class="row">
-                <ul class="col-12 col-md-11 col-lg-8" id="emailList">
-                    (% for email in attrs.emails %)
-                        (% let type = "email" %)
-                        (% let name = "emails[]" %)
-                        (% let value = email %)
-                        (% let invalid_feedback = "Please enter a valid email address." %)
-                        (% include "user_settings/form_modifiable_entry_modifiable_list_partial.html" %)
+            <div class="row g-0">
+                <div class="col-12 col-md-11 col-lg-8" id="emailAddresses">
+                    (% for (i, email) in person.mails.iter().enumerate() %)
+                    (% let type = "email" %)
+                    (% let name = "emails[]" %)
+                    (% let value = email.value.clone() %)
+                    (% let invalid_feedback = "Please enter a valid email address." %)
+
+                        <div class="input-group mb-1">
+                            <div class="input-group-text">
+                                <input class="form-check-input mt-0" name="primary_email_index" type="radio" value="((i))" aria-label="Primary email radio button for following text input">
+                            </div>
+
+                            <input type="(( type ))" class="form-control" name="(( name ))" value="(( value ))" hx-validate="true" required>
+                            (% if can_rw %)
+                            <button type="button" class="btn btn-secondary kanidm-remove-list-entry">Remove</button>
+                            (% endif %)
+
+                        </div>
+                        <div class="invalid-feedback">(( invalid_feedback ))</div>
+
                     (% endfor %)
-                </ul>
+                </div>
             </div>
-            (% if can_rw %)<button type="button" class="btn btn-primary" hx-post="/ui/api/user_settings/add_email" hx-target="#emailList" hx-swap="beforeend">Add Email</button>(% endif %)
         </div>
     </div>
 
     <!-- Edit button -->
     <div class="pt-4">
         (% if can_rw %)
-            <button class="btn btn-primary" type="submit">Edit</button>
+        <button class="btn btn-primary" type="submit">Save</button>
         (% else %)
-            <a href="/ui/profile/unlock" hx-boost="false">
-                <button class="btn btn-primary" type="button">Unlock Edit 🔒</button>
-            </a>
+        <a href="/ui/profile/unlock" hx-boost="false">
+            <button class="btn btn-primary" type="button">Unlock Edit 🔒</button>
+        </a>
         (% endif %)
     </div>
 </form>