Merge 3077d86aa1 into b113262357

It was possible that a token could be updated in a way that caused
existing cached information to be lost if an event was delayed
in it's write to the user token.

To prevent this, the writes to user tokens now require the HsmLock
to be held, and refresh the token just ahead of writing to ensure
that these data can't be lost. The benefit to this approach is that
readers remain unblocked by a writer.

Cargo.lock

@@ -391,6 +391,7 @@ dependencies = [
  "multer",
  "pin-project-lite",
  "serde",
+ "serde_html_form",
  "tower 0.5.2",
  "tower-layer",
  "tower-service",
@@ -5138,6 +5139,19 @@ dependencies = [
  "syn 2.0.100",
 ]
 
+[[package]]
+name = "serde_html_form"
+version = "0.2.7"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "9d2de91cf02bbc07cde38891769ccd5d4f073d22a40683aa4bc7a95781aaa2c4"
+dependencies = [
+ "form_urlencoded",
+ "indexmap 2.8.0",
+ "itoa",
+ "ryu",
+ "serde",
+]
+
 [[package]]
 name = "serde_json"
 version = "1.0.140"
@@ -5658,9 +5672,9 @@ dependencies = [
 
 [[package]]
 name = "tokio"
-version = "1.44.1"
+version = "1.44.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f382da615b842244d4b8738c82ed1275e6c5dd90c459a30941cd07080b06c91a"
+checksum = "e6b88822cbe49de4185e3a4cbf8321dd487cf5fe0c5c65695fef6346371e9c48"
 dependencies = [
  "backtrace",
  "bytes",

Cargo.toml

@@ -268,7 +268,7 @@ tempfile = "3.15.0"
 testkit-macros = { path = "./server/testkit-macros" }
 time = { version = "^0.3.36", features = ["formatting", "local-offset"] }
 
-tokio = "^1.43.0"
+tokio = "^1.44.2"
 tokio-openssl = "^0.6.5"
 tokio-util = "^0.7.13"
 

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)]

server/core/Cargo.toml

@@ -24,7 +24,7 @@ askama = { workspace = true, features = ["with-axum"] }
 askama_axum = { workspace = true }
 axum = { workspace = true }
 axum-htmx = { workspace = true }
-axum-extra = { version = "0.9.6", features = ["cookie"] }
+axum-extra = { version = "0.9.6", features = ["cookie", "form"] }
 axum-macros = "0.4.2"
 axum-server = { version = "0.7.1", default-features = false }
 bytes = { workspace = true }

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()

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

server/core/src/https/views/constants.rs

@@ -9,17 +9,17 @@ pub(crate) enum ProfileMenuItems {
     UnixPassword,
 }
 
-pub(crate) enum UiMessage {
-    UnlockEdit,
-}
-
-impl std::fmt::Display for UiMessage {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
-            UiMessage::UnlockEdit => write!(f, "Unlock Edit 🔒"),
-        }
-    }
-}
+// pub(crate) enum UiMessage {
+//     UnlockEdit,
+// }
+//
+// impl std::fmt::Display for UiMessage {
+//     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+//         match self {
+//             UiMessage::UnlockEdit => write!(f, "Unlock Edit 🔒"),
+//         }
+//     }
+// }
 
 pub(crate) enum Urls {
     Apps,

server/core/src/https/views/mod.rs

@@ -56,6 +56,7 @@ pub fn view_router() -> Router<ServerState> {
         .route("/reset", get(reset::view_reset_get))
         .route("/update_credentials", get(reset::view_self_reset_get))
         .route("/profile", get(profile::view_profile_get))
+        .route("/profile/diff", get(profile::view_profile_get))
         .route("/profile/unlock", get(profile::view_profile_unlock_get))
         .route("/logout", get(login::view_logout_get))
         .route("/oauth2", get(oauth2::view_index_get));
@@ -129,6 +130,18 @@ pub fn view_router() -> Router<ServerState> {
         )
         .route("/api/cu_cancel", post(reset::cancel_cred_update))
         .route("/api/cu_commit", post(reset::commit))
+        .route(
+            "/api/user_settings/add_email",
+            get(profile::view_new_email_entry_partial),
+        )
+        .route(
+            "/api/user_settings/edit_profile",
+            post(profile::view_profile_diff_start_save_post),
+        )
+        .route(
+            "/api/user_settings/confirm_profile",
+            post(profile::view_profile_diff_confirm_save_post),
+        )
         .layer(HxRequestGuardLayer::new("/ui"));
 
     let admin_router = admin_router();

server/core/src/https/views/profile.rs

@@ -1,18 +1,34 @@
+use super::constants::{ProfileMenuItems, Urls};
+use super::errors::HtmxError;
+use super::login::{LoginDisplayCtx, Reauth, ReauthPurpose};
+use super::navbar::NavbarCtx;
 use crate::https::errors::WebError;
 use crate::https::extractors::{DomainInfo, VerifiedClientInformation};
 use crate::https::middleware::KOpId;
 use crate::https::ServerState;
 use askama::Template;
-use axum::extract::State;
-use axum::response::Response;
+use askama_axum::IntoResponse;
+use axum::extract::{Query, State};
+use axum::http::Uri;
+use axum::response::{Redirect, Response};
 use axum::Extension;
 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_MAIL, ATTR_NAME};
 use kanidm_proto::internal::UserAuthToken;
-
-use super::constants::{ProfileMenuItems, UiMessage, Urls};
-use super::errors::HtmxError;
-use super::login::{LoginDisplayCtx, Reauth, ReauthPurpose};
-use super::navbar::NavbarCtx;
+use kanidm_proto::scim_v1::server::{ScimEffectiveAccess, ScimPerson};
+use kanidm_proto::scim_v1::ScimMail;
+use kanidmd_lib::filter::{f_id, Filter};
+use kanidmd_lib::prelude::f_and;
+use kanidmd_lib::prelude::FC;
+use serde::Deserialize;
+use serde::Serialize;
+use std::fmt;
+use std::fmt::Display;
+use std::fmt::Formatter;
 
 #[derive(Template)]
 #[template(path = "user_settings.html")]
@@ -26,9 +42,68 @@ pub(crate) struct ProfileView {
 struct ProfilePartialView {
     menu_active_item: ProfileMenuItems,
     can_rw: bool,
+    person: ScimPerson,
+    scim_effective_access: ScimEffectiveAccess,
+}
+
+#[derive(Clone, Debug, Serialize, Deserialize)]
+pub(crate) struct SaveProfileQuery {
+    #[serde(rename = "name")]
+    account_name: String,
+    #[serde(rename = "displayname")]
+    display_name: String,
+    #[serde(rename = "email_index")]
+    emails_indexes: Vec<u16>,
+    #[serde(rename = "emails[]")]
+    emails: Vec<String>,
+    // radio buttons are used to pick a primary index, remove causes holes, map back into [emails] using [emails_indexes]
+    primary_email_index: u16,
+}
+
+#[derive(Clone, Debug, Serialize, Deserialize)]
+pub(crate) struct CommitSaveProfileQuery {
+    #[serde(rename = "account_name")]
+    account_name: Option<String>,
+    #[serde(rename = "display_name")]
+    display_name: Option<String>,
+    #[serde(rename = "emails[]")]
+    emails: Vec<String>,
+    #[serde(rename = "new_primary_mail")]
+    new_primary_mail: Option<String>,
+}
+
+#[derive(Clone, Debug, Serialize, Deserialize)]
+pub(crate) struct ProfileAttributes {
     account_name: String,
     display_name: String,
-    email: Option<String>,
+    emails: Vec<ScimMail>,
+}
+
+#[derive(Template, Clone)]
+#[template(path = "user_settings/profile_changes_partial.html")]
+struct ProfileChangesPartialView {
+    menu_active_item: ProfileMenuItems,
+    can_rw: bool,
+    person: ScimPerson,
+    primary_mail: Option<String>,
+    new_attrs: ProfileAttributes,
+    new_primary_mail: Option<String>,
+    emails_are_same: bool,
+}
+
+#[derive(Template, Clone)]
+#[template(path = "user_settings/form_email_entry_partial.html")]
+pub(crate) struct FormEmailEntryListPartial {
+    can_edit: bool,
+    value: String,
+    primary: bool,
+    index: u16,
+}
+
+impl Display for ProfileAttributes {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        write!(f, "{self:?}")
+    }
 }
 
 pub(crate) async fn view_profile_get(
@@ -39,7 +114,16 @@ pub(crate) async fn view_profile_get(
 ) -> Result<ProfileView, WebError> {
     let uat: UserAuthToken = state
         .qe_r_ref
-        .handle_whoami_uat(client_auth_info, kopid.eventid)
+        .handle_whoami_uat(client_auth_info.clone(), 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 time = time::OffsetDateTime::now_utc() + time::Duration::new(60, 0);
@@ -52,13 +136,210 @@ pub(crate) async fn view_profile_get(
         profile_partial: ProfilePartialView {
             menu_active_item: ProfileMenuItems::UserProfile,
             can_rw,
-            account_name: uat.name().to_string(),
-            display_name: uat.displayname.clone(),
-            email: uat.mail_primary.clone(),
+            person: scim_person,
+            scim_effective_access,
         },
     })
 }
 
+pub(crate) async fn view_profile_diff_start_save_post(
+    State(state): State<ServerState>,
+    Extension(kopid): Extension<KOpId>,
+    VerifiedClientInformation(client_auth_info): VerifiedClientInformation,
+    DomainInfo(domain_info): DomainInfo,
+    // Form must be the last parameter because it consumes the request body
+    Form(query): Form<SaveProfileQuery>,
+) -> axum::response::Result<Response> {
+    let uat: UserAuthToken = state
+        .qe_r_ref
+        .handle_whoami_uat(client_auth_info.clone(), kopid.eventid)
+        .map_err(|op_err| HtmxError::new(&kopid, op_err, domain_info.clone()))
+        .await?;
+
+    let time = time::OffsetDateTime::now_utc() + time::Duration::new(60, 0);
+    let can_rw = uat.purpose_readwrite_active(time);
+    // 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 primary_index = query
+        .emails_indexes
+        .iter()
+        .position(|ei| ei == &query.primary_email_index)
+        .unwrap_or(0);
+    let new_mails = query
+        .emails
+        .iter()
+        .enumerate()
+        .map(|(ei, email)| ScimMail {
+            primary: ei == primary_index,
+            value: email.to_string(),
+        })
+        .collect();
+    let old_primary_mail = scim_person
+        .mails
+        .iter()
+        .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,
+        person: scim_person,
+        primary_mail: old_primary_mail,
+        new_attrs: ProfileAttributes {
+            account_name: query.account_name,
+            display_name: query.display_name,
+            emails: new_mails,
+        },
+        emails_are_same,
+        new_primary_mail: query.emails.get(primary_index).cloned(),
+    };
+
+    Ok((
+        HxPushUrl(Uri::from_static("/ui/profile/diff")),
+        profile_view,
+    )
+        .into_response())
+}
+
+pub(crate) async fn view_profile_diff_confirm_save_post(
+    State(state): State<ServerState>,
+    Extension(kopid): Extension<KOpId>,
+    VerifiedClientInformation(client_auth_info): VerifiedClientInformation,
+    DomainInfo(domain_info): DomainInfo,
+    // Form must be the last parameter because it consumes the request body
+    Form(query): Form<CommitSaveProfileQuery>,
+) -> axum::response::Result<Response> {
+    let uat: UserAuthToken = state
+        .qe_r_ref
+        .handle_whoami_uat(client_auth_info.clone(), kopid.eventid)
+        .map_err(|op_err| HtmxError::new(&kopid, op_err, domain_info.clone()))
+        .await?;
+
+    let filter = filter_all!(f_and!([f_id(uat.uuid.to_string().as_str())]));
+
+    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);
+    }
+
+    state
+        .qe_w_ref
+        .handle_setattribute(
+            client_auth_info.clone(),
+            uat.uuid.to_string(),
+            ATTR_MAIL.to_string(),
+            emails,
+            filter.clone(),
+            kopid.eventid,
+        )
+        .map_err(|op_err| HtmxError::new(&kopid, op_err, domain_info.clone()))
+        .await?;
+
+    // TODO: These are normally not permitted, user should be prevented from changing non modifiable fields in the UI though
+    // state
+    //     .qe_w_ref
+    //     .handle_setattribute(
+    //         client_auth_info.clone(),
+    //         uat.uuid.to_string(),
+    //         ATTR_EMAIL.to_string(),
+    //         vec![new_attrs.email.unwrap_or("".to_string())],
+    //         filter.clone(),
+    //         kopid.eventid,
+    //     )
+    //     .map_err(|op_err| HtmxError::new(&kopid, op_err))
+    //     .await?;
+    //
+    // state
+    //     .qe_w_ref
+    //     .handle_setattribute(
+    //         client_auth_info.clone(),
+    //         uat.uuid.to_string(),
+    //         ATTR_NAME.to_string(),
+    //         vec![new_attrs.account_name],
+    //         filter.clone(),
+    //         kopid.eventid,
+    //     )
+    //     .map_err(|op_err| HtmxError::new(&kopid, op_err))
+    //     .await?;
+
+    // TODO: Calling this here returns the old attributes
+    match view_profile_get(
+        State(state),
+        Extension(kopid),
+        VerifiedClientInformation(client_auth_info),
+        DomainInfo(domain_info),
+    )
+    .await
+    {
+        Ok(_) => Ok(Redirect::to(Urls::Profile.as_ref()).into_response()),
+        Err(e) => Ok(e.into_response()),
+    }
+}
+
+#[derive(Deserialize)]
+pub(crate) struct AddEmailQuery {
+    // the last email index is passed so we can return an incremented id
+    email_index: Option<u16>,
+}
+
+// Sends the user a new email input to fill in :)
+pub(crate) async fn view_new_email_entry_partial(
+    State(_state): State<ServerState>,
+    VerifiedClientInformation(_client_auth_info): VerifiedClientInformation,
+    Extension(_kopid): Extension<KOpId>,
+    Query(email_query): Query<AddEmailQuery>,
+) -> axum::response::Result<Response> {
+    let add_email_trigger =
+        HxResponseTrigger::after_swap([HxEvent::new("addEmailSwapped".to_string())]);
+    Ok((
+        add_email_trigger,
+        FormEmailEntryListPartial {
+            can_edit: true,
+            value: "".to_string(),
+            primary: email_query.email_index.is_none(),
+            index: email_query.email_index.map(|i| i + 1).unwrap_or(0),
+        },
+    )
+        .into_response())
+}
+
 pub(crate) async fn view_profile_unlock_get(
     State(state): State<ServerState>,
     VerifiedClientInformation(client_auth_info): VerifiedClientInformation,

server/core/static/external/forms.js

@@ -0,0 +1,25 @@
+// This file will contain js helpers to have some interactivity on forms that we can't achieve with pure html.
+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) continue
+
+        button.addEventListener("click", (e) => {
+            // Expected html nesting: div.email-entry > div.input-group > button.kanidm-remove-list-entry
+            let li = button.parentElement?.parentElement;
+            if (li && li.classList.contains("email-entry")) {
+                li.remove();
+            }
+        })
+        button.setAttribute("kanidm_hooked", "")
+    }
+}
+
+window.onload = function () {
+    rehook_string_list_removers();
+    document.body.addEventListener("addEmailSwapped", () => {
+        rehook_string_list_removers();
+    })
+};
+

server/core/static/external/htmx_bs_validation.js

@@ -0,0 +1,34 @@
+htmx.defineExtension("bs-validation", {
+    onEvent: function (name, evt) {
+        let form = evt.detail.elt;
+        // Htmx propagates attributes onto children like button, which would break those buttons, so we return if not a form.
+        if (form.tagName !== "FORM") return;
+
+        // check if trigger attribute and submit event exists
+        // for the form
+        if (!form.hasAttribute("hx-trigger")) {
+            // set trigger for custom event bs-send
+            form.setAttribute("hx-trigger", "bs-send");
+
+            // and attach the event only once
+            form.addEventListener("submit", function (event) {
+                if (form.checkValidity()) {
+                    // trigger custom event hx-trigger="bs-send"
+                    htmx.trigger(form, "bsSend");
+                }
+
+                // focus the first :invalid field
+                let invalidField = form.querySelector(":invalid");
+                if (invalidField) {
+                    invalidField.focus();
+                }
+
+                console.log("prevented htmx send, form was invalid")
+                event.preventDefault()
+                event.stopPropagation()
+
+                form.classList.add("was-validated")
+            }, false)
+        }
+    }
+});

server/core/static/style.css

@@ -29,6 +29,7 @@ body {
   display: block !important;
 }
 
+
 /*
  * Sidebar
  */
@@ -206,3 +207,7 @@ footer {
   height: var(--icon-size);
   transform: rotate(35deg);
 }
+
+.cursor-pointer:hover {
+  cursor: pointer;
+}

server/core/templates/user_settings.html

@@ -3,6 +3,8 @@
 (% block title %)Profile(% endblock %)
 
 (% block head %)
+<script src="/pkg/external/forms.js"></script>
+<script src="/pkg/external/htmx_bs_validation.js"></script>
 (% endblock %)
 
 (% block main %)

server/core/templates/user_settings/form_email_entry_partial.html

@@ -0,0 +1,17 @@
+<div class="email-entry">
+    <input hidden class="email-index-state" type="text" name="email_index" value="((index))">
+    <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="((index))" aria-label="Primary email radio button for following text input" (% if primary %) checked (% endif %)(% if !can_edit %) disabled (% endif %)>
+        </div>
+
+        (% if can_edit %)
+        <input type="email" aria-label="Email address input ((index))" class="form-control" name="emails[]" value="(( value ))" hx-validate="true" required>
+        <button type="button" class="btn btn-secondary kanidm-remove-list-entry">Remove</button>
+        (% else %)
+        <input type="email" aria-label="Email address input ((index))" class="form-control" name="emails[]" value="(( value ))" hx-validate="true" required disabled>
+        (% endif %)
+    </div>
+    <div class="invalid-feedback">Please enter a valid email address.</div>
+</div>

server/core/templates/user_settings/form_modifiable_entry_modifiable_list_partial.html

@@ -0,0 +1,7 @@
+<li class="mb-2">
+    <div class="input-group">
+        <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>
+</li>

server/core/templates/user_settings/profile_changes_partial.html

@@ -0,0 +1,104 @@
+(% extends "user_settings_partial_base.html" %)
+
+(% block selected_setting_group %)
+Profile Difference
+(% endblock %)
+
+(% block settings_vertical_point %)lg(% endblock %)
+
+(% 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 ))"/>
+    (% endif %)
+
+    (% for email in new_attrs.emails %)
+    (% 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 ))"/>
+    (% 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>
+        </thead>
+        (% 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 %)
+
+        (% 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 %)
+        (% 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>
+
+    <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>
+        (% 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. -->
+            <button class="btn btn-primary" type="button">Unlock Confirm 🔓</button>
+        </a>
+        (% endif %)
+    </div>
+
+</form>
+
+(% endblock %)
+

server/core/templates/user_settings_profile_partial.html

@@ -4,40 +4,78 @@
 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>
-    <div class="mb-2 row">
-        <label for="profileUserName" class="col-12 col-md-3 col-xl-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="(( account_name ))">
-        </div>
-    </div>
+<form id="user_settings_container" class="needs-validation"
+      hx-post="/ui/api/user_settings/edit_profile"
+      hx-target="#main"
+      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="profileDisplayName" class="col-12 col-md-3 col-xl-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="(( display_name ))" disabled>
+    (% call string_attr("Displayname", "displayname", person.displayname, true, Attribute::DisplayName) %)
+
+    <div class="mb-2">
+        <div class="mt-3 mb-2 col-12 col-md-11 col-lg-8">
+            <label for="profileEmail" class="fw-bold">Email addresses (% if can_rw %)(selected => primary)(% else %)(select primary)(% endif %)</label>
+            (% if can_rw %)
+            <a class="cursor-pointer float-end"
+               hx-boost="true"
+               hx-get="/ui/api/user_settings/add_email"
+               hx-target="#emailAddresses"
+               hx-include="#emailAddresses :last-child .email-index-state"
+               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>
+            (% endif %)
         </div>
-    </div>
-    
-    <div class="mb-2 row">
-        <label for="profileEmail" class="col-12 col-md-3 col-xl-2 col-form-label">Email</label>
-        <div class="col-12 col-md-6 col-lg-5">
-            <input type="email" disabled class="form-control-plaintext" id="profileEmail" value="(( email.clone().unwrap_or("None configured".to_string())))">
+        <div>
+            <div class="row g-0">
+                <div class="col-12 col-md-11 col-lg-8" id="emailAddresses">
+                    (% for (index, email) in person.mails.iter().enumerate() %)
+                    (% let value = email.value.clone() %)
+                    (% let primary = email.primary %)
+                    (% let can_edit = scim_effective_access.modify_present.check(Attribute::Mail|as_ref) %)
+
+                    (% include "user_settings/form_email_entry_partial.html" %)
+
+                    (% endfor %)
+                </div>
+            </div>
         </div>
     </div>
 
     <!-- Edit button -->
-    <!-- <div class="pt-4">
+    <div class="pt-4">
         (% if can_rw %)
-            <button class="btn btn-primary" type="button" hx-post="/ui/api/user_settings/edit_profile" disabled>Edit (Currently Not Working!)</button>
+        <button class="btn btn-primary" type="submit">Save</button>
         (% else %)
-            <a href=(Urls::ProfileUnlock) hx-boost="false">
-                <button class="btn btn-primary" type="button">((UiMessage::UnlockEdit))</button>
-            </a>
+        <a href="/ui/profile/unlock" hx-boost="false">
+            <button class="btn btn-primary" type="button">Unlock Edit 🔒</button>
+        </a>
         (% endif %)
-    </div> -->
+    </div>
 </form>
 
 (% endblock %)

unix_integration/resolver/src/idprovider/interface.rs

@@ -194,7 +194,8 @@ impl Into<PamAuthResponse> for AuthRequest {
 }
 
 pub enum AuthResult {
-    Success { token: UserToken },
+    Success,
+    SuccessUpdate { new_token: UserToken },
     Denied,
     Next(AuthRequest),
 }
@@ -251,6 +252,7 @@ pub trait IdProvider {
     async fn unix_user_online_auth_step(
         &self,
         _account_id: &str,
+        _current_token: Option<&UserToken>,
         _cred_handler: &mut AuthCredHandler,
         _pam_next_req: PamAuthRequest,
         _tpm: &mut tpm::BoxedDynTpm,
@@ -290,7 +292,8 @@ pub trait IdProvider {
     // TPM key.
     async fn unix_user_offline_auth_step(
         &self,
-        _token: &UserToken,
+        _current_token: Option<&UserToken>,
+        _session_token: &UserToken,
         _cred_handler: &mut AuthCredHandler,
         _pam_next_req: PamAuthRequest,
         _tpm: &mut tpm::BoxedDynTpm,

unix_integration/resolver/src/idprovider/kanidm.rs

@@ -55,8 +55,6 @@ impl KanidmProvider {
         tpm: &mut tpm::BoxedDynTpm,
         machine_key: &tpm::MachineKey,
     ) -> Result<Self, IdpError> {
-        // FUTURE: Randomised jitter on next check at startup.
-
         // Initially retrieve our HMAC key.
         let loadable_hmac_key: Option<tpm::LoadableHmacKey> = keystore
             .get_tagged_hsm_key(KANIDM_HMAC_KEY)
@@ -248,13 +246,25 @@ impl KanidmProviderInternal {
             // Proceed
             CacheState::Online => true,
             CacheState::OfflineNextCheck(at_time) if now >= at_time => {
-                // Attempt online. If fails, return token.
                 self.attempt_online(tpm, now).await
             }
             CacheState::OfflineNextCheck(_) | CacheState::Offline => false,
         }
     }
 
+    #[instrument(level = "debug", skip_all)]
+    async fn check_online_right_meow(
+        &mut self,
+        tpm: &mut tpm::BoxedDynTpm,
+        now: SystemTime,
+    ) -> bool {
+        match self.state {
+            CacheState::Online => true,
+            CacheState::OfflineNextCheck(_) => self.attempt_online(tpm, now).await,
+            CacheState::Offline => false,
+        }
+    }
+
     #[instrument(level = "debug", skip_all)]
     async fn attempt_online(&mut self, _tpm: &mut tpm::BoxedDynTpm, now: SystemTime) -> bool {
         let mut max_attempts = 3;
@@ -295,7 +305,7 @@ impl IdProvider for KanidmProvider {
 
     async fn attempt_online(&self, tpm: &mut tpm::BoxedDynTpm, now: SystemTime) -> bool {
         let mut inner = self.inner.lock().await;
-        inner.check_online(tpm, now).await
+        inner.check_online_right_meow(tpm, now).await
     }
 
     async fn mark_next_check(&self, now: SystemTime) {
@@ -431,6 +441,7 @@ impl IdProvider for KanidmProvider {
     async fn unix_user_online_auth_step(
         &self,
         account_id: &str,
+        current_token: Option<&UserToken>,
         cred_handler: &mut AuthCredHandler,
         pam_next_req: PamAuthRequest,
         tpm: &mut tpm::BoxedDynTpm,
@@ -449,15 +460,23 @@ impl IdProvider for KanidmProvider {
 
                 match auth_result {
                     Ok(Some(n_tok)) => {
-                        let mut token = UserToken::from(n_tok);
-                        token.kanidm_update_cached_password(
+                        let mut new_token = UserToken::from(n_tok);
+
+                        // Update any keys that may have been in the db in the current
+                        // token.
+                        if let Some(previous_token) = current_token {
+                            new_token.extra_keys = previous_token.extra_keys.clone();
+                        }
+
+                        // Set any new keys that are relevant from this authentication
+                        new_token.kanidm_update_cached_password(
                             &inner.crypto_policy,
                             cred.as_str(),
                             tpm,
                             &inner.hmac_key,
                         );
 
-                        Ok(AuthResult::Success { token })
+                        Ok(AuthResult::SuccessUpdate { new_token })
                     }
                     Ok(None) => {
                         // TODO: i'm not a huge fan of this rn, but currently the way we handle
@@ -552,7 +571,8 @@ impl IdProvider for KanidmProvider {
 
     async fn unix_user_offline_auth_step(
         &self,
-        token: &UserToken,
+        current_token: Option<&UserToken>,
+        session_token: &UserToken,
         cred_handler: &mut AuthCredHandler,
         pam_next_req: PamAuthRequest,
         tpm: &mut tpm::BoxedDynTpm,
@@ -561,11 +581,13 @@ impl IdProvider for KanidmProvider {
             (AuthCredHandler::Password, PamAuthRequest::Password { cred }) => {
                 let inner = self.inner.lock().await;
 
-                if token.kanidm_check_cached_password(cred.as_str(), tpm, &inner.hmac_key) {
+                if session_token.kanidm_check_cached_password(cred.as_str(), tpm, &inner.hmac_key) {
+                    // Ensure we have either the latest token, or if none, at least the session token.
+                    let new_token = current_token.unwrap_or(session_token).clone();
+
                     // TODO: We can update the token here and then do lockouts.
-                    Ok(AuthResult::Success {
-                        token: token.clone(),
-                    })
+
+                    Ok(AuthResult::SuccessUpdate { new_token })
                 } else {
                     Ok(AuthResult::Denied)
                 }

unix_integration/resolver/src/resolver.rs

@@ -47,7 +47,6 @@ pub enum AuthSession {
         client: Arc<dyn IdProvider + Sync + Send>,
         account_id: String,
         id: Id,
-        token: Option<Box<UserToken>>,
         cred_handler: AuthCredHandler,
         /// Some authentication operations may need to spawn background tasks. These tasks need
         /// to know when to stop as the caller has disconnected. This receiver allows that, so
@@ -59,7 +58,7 @@ pub enum AuthSession {
         account_id: String,
         id: Id,
         client: Arc<dyn IdProvider + Sync + Send>,
-        token: Box<UserToken>,
+        session_token: Box<UserToken>,
         cred_handler: AuthCredHandler,
     },
     System {
@@ -225,7 +224,7 @@ impl Resolver {
         //  Attempt to search these in the db.
         let mut dbtxn = self.db.write().await;
         let r = dbtxn.get_account(account_id).map_err(|err| {
-            debug!("get_cached_usertoken {:?}", err);
+            debug!(?err, "get_cached_usertoken");
         })?;
 
         drop(dbtxn);
@@ -318,7 +317,12 @@ impl Resolver {
         }
     }
 
-    async fn set_cache_usertoken(&self, token: &mut UserToken) -> Result<(), ()> {
+    async fn set_cache_usertoken(
+        &self,
+        token: &mut UserToken,
+        // This is just for proof that only one write can occur at a time.
+        _tpm: &mut BoxedDynTpm,
+    ) -> Result<(), ()> {
         // Set an expiry
         let ex_time = SystemTime::now() + Duration::from_secs(self.timeout_seconds);
         let offset = ex_time
@@ -451,6 +455,22 @@ impl Resolver {
 
         let mut hsm_lock = self.hsm.lock().await;
 
+        // We need to re-acquire the token now behind the hsmlock - this is so that
+        // we know that as we write the updated token, we know that no one else has
+        // written to this token, since we are now the only task that is allowed
+        // to be in a write phase.
+        let token = if token.is_some() {
+            self.get_cached_usertoken(account_id)
+                .await
+                .map(|(_expired, option_token)| option_token)
+                .map_err(|err| {
+                    debug!(?err, "get_usertoken error");
+                })?
+        } else {
+            // Was already none, leave it that way.
+            None
+        };
+
         let user_get_result = if let Some(tok) = token.as_ref() {
             // Re-use the provider that the token is from.
             match self.client_ids.get(&tok.provider) {
@@ -486,12 +506,11 @@ impl Resolver {
             }
         };
 
-        drop(hsm_lock);
-
         match user_get_result {
             Ok(UserTokenState::Update(mut n_tok)) => {
                 // We have the token!
-                self.set_cache_usertoken(&mut n_tok).await?;
+                self.set_cache_usertoken(&mut n_tok, hsm_lock.deref_mut())
+                    .await?;
                 Ok(Some(n_tok))
             }
             Ok(UserTokenState::NotFound) => {
@@ -958,7 +977,6 @@ impl Resolver {
                             client,
                             account_id: account_id.to_string(),
                             id,
-                            token: Some(Box::new(token)),
                             cred_handler,
                             shutdown_rx,
                         };
@@ -979,7 +997,7 @@ impl Resolver {
                             account_id: account_id.to_string(),
                             id,
                             client,
-                            token: Box::new(token),
+                            session_token: Box::new(token),
                             cred_handler,
                         };
                         Ok((auth_session, next_req.into()))
@@ -1022,7 +1040,6 @@ impl Resolver {
                             client: client.clone(),
                             account_id: account_id.to_string(),
                             id,
-                            token: None,
                             cred_handler,
                             shutdown_rx,
                         };
@@ -1050,19 +1067,32 @@ impl Resolver {
         auth_session: &mut AuthSession,
         pam_next_req: PamAuthRequest,
     ) -> Result<PamAuthResponse, ()> {
+        let mut hsm_lock = self.hsm.lock().await;
+
         let maybe_err = match &mut *auth_session {
             &mut AuthSession::Online {
                 ref client,
                 ref account_id,
-                id: _,
-                token: _,
+                ref id,
                 ref mut cred_handler,
                 ref shutdown_rx,
             } => {
-                let mut hsm_lock = self.hsm.lock().await;
+                // This is not used in the authentication, but is so that any new
+                // extra keys or data on the token are updated correctly if the authentication
+                // requests an update. Since we hold the hsm_lock, no other task can
+                // update this token between now and completion of the fn.
+                let current_token = self
+                    .get_cached_usertoken(id)
+                    .await
+                    .map(|(_expired, option_token)| option_token)
+                    .map_err(|err| {
+                        debug!(?err, "get_usertoken error");
+                    })?;
+
                 let result = client
                     .unix_user_online_auth_step(
                         account_id,
+                        current_token.as_ref(),
                         cred_handler,
                         pam_next_req,
                         hsm_lock.deref_mut(),
@@ -1071,7 +1101,7 @@ impl Resolver {
                     .await;
 
                 match result {
-                    Ok(AuthResult::Success { .. }) => {
+                    Ok(AuthResult::SuccessUpdate { .. } | AuthResult::Success) => {
                         info!(?account_id, "Authentication Success");
                     }
                     Ok(AuthResult::Denied) => {
@@ -1087,17 +1117,29 @@ impl Resolver {
             }
             &mut AuthSession::Offline {
                 ref account_id,
-                id: _,
+                ref id,
                 ref client,
-                ref token,
+                ref session_token,
                 ref mut cred_handler,
             } => {
+                // This is not used in the authentication, but is so that any new
+                // extra keys or data on the token are updated correctly if the authentication
+                // requests an update. Since we hold the hsm_lock, no other task can
+                // update this token between now and completion of the fn.
+                let current_token = self
+                    .get_cached_usertoken(id)
+                    .await
+                    .map(|(_expired, option_token)| option_token)
+                    .map_err(|err| {
+                        debug!(?err, "get_usertoken error");
+                    })?;
+
                 // We are offline, continue. Remember, authsession should have
                 // *everything you need* to proceed here!
-                let mut hsm_lock = self.hsm.lock().await;
                 let result = client
                     .unix_user_offline_auth_step(
-                        token,
+                        current_token.as_ref(),
+                        session_token,
                         cred_handler,
                         pam_next_req,
                         hsm_lock.deref_mut(),
@@ -1105,7 +1147,7 @@ impl Resolver {
                     .await;
 
                 match result {
-                    Ok(AuthResult::Success { .. }) => {
+                    Ok(AuthResult::SuccessUpdate { .. } | AuthResult::Success) => {
                         info!(?account_id, "Authentication Success");
                     }
                     Ok(AuthResult::Denied) => {
@@ -1156,8 +1198,13 @@ impl Resolver {
 
         match maybe_err {
             // What did the provider direct us to do next?
-            Ok(AuthResult::Success { mut token }) => {
-                self.set_cache_usertoken(&mut token).await?;
+            Ok(AuthResult::Success) => {
+                *auth_session = AuthSession::Success;
+                Ok(PamAuthResponse::Success)
+            }
+            Ok(AuthResult::SuccessUpdate { mut new_token }) => {
+                self.set_cache_usertoken(&mut new_token, hsm_lock.deref_mut())
+                    .await?;
                 *auth_session = AuthSession::Success;
 
                 Ok(PamAuthResponse::Success)