Merge 91307dc62e into 848af4cecd

* Adding TOTP Label verification (for both empty and duplicate)

Cargo.lock

@@ -391,6 +391,7 @@ dependencies = [
  "multer",
  "pin-project-lite",
  "serde",
+ "serde_html_form",
  "tower 0.5.2",
  "tower-layer",
  "tower-service",
@@ -5064,6 +5065,19 @@ dependencies = [
  "syn 2.0.98",
 ]
 
+[[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.7.1",
+ "itoa",
+ "ryu",
+ "serde",
+]
+
 [[package]]
 name = "serde_json"
 version = "1.0.138"

book/src/integrations/oauth2.md

@@ -70,36 +70,10 @@ anything special for Kanidm (or another provider).
 **Note:** some apps automatically append `/.well-known/openid-configuration` to
 the end of an OIDC Discovery URL, so you may need to omit that.
 
-
-<dl>
-<dt>[Webfinger](https://datatracker.ietf.org/doc/html/rfc7033) URL</dt>
-
-<dd>
-
-`https://idm.example.com/oauth2/openid/:client_id:/.well-known/webfinger`
-
-The webfinger URL is implemented for each OpenID client, under its specific endpoint, giving full control to the administrator regarding which to use.
-
-To make this compliant with the standard, it must be made available under the correct [well-known endpoint](https://datatracker.ietf.org/doc/html/rfc7033#section-10.1) (e.g `example.com/.well-known/webfinger`), typically via a reverse proxy or similar. Kanidm doesn't currently provide a mechanism for this URI rewrite.
-
-One example would be dedicating one client as the "primary" or "default" and redirecting all requests to that. Alternatively, source IP or other request metadata could be used to decide which client to forward the request to.
-
-### Caddy
-`Caddyfile`
-```caddy
-# assuming a kanidm service with domain "example.com"
-example.com {
-  redir /.well-known/webfinger https://idm.example.com/oauth2/openid/:client_id:{uri} 307
-}
-```
-**Note:** the `{uri}` is important as it preserves the original request past the redirect.
-
-
-</dd>
-
 <dt>
 
-[RFC 8414 OAuth 2.0 Authorisation Server Metadata](https://datatracker.ietf.org/doc/html/rfc8414) URL
+[RFC 8414 OAuth 2.0 Authorisation Server Metadata](https://datatracker.ietf.org/doc/html/rfc8414)
+URL **(recommended)**
 
 </dt>
 
@@ -111,6 +85,21 @@ example.com {
 
 <dt>
 
+[WebFinger URL **(discouraged)**](#webfinger)
+
+</dt>
+
+<dd>
+
+`https://idm.example.com/oauth2/openid/:client_id:/.well-known/webfinger`
+
+See [the WebFinger section](#webfinger) for more details, as there a number of
+caveats for WebFinger clients.
+
+</dd>
+
+<dt>
+
 User auth
 
 </dt>
@@ -466,3 +455,61 @@ kanidm system oauth2 reset-secrets
 ```
 
 Each client has unique signing keys and access secrets, so this is limited to each service.
+
+## WebFinger
+
+[WebFinger](https://datatracker.ietf.org/doc/html/rfc7033) provides a mechanism
+for discovering information about people or other entities. It can be used by an
+identity provider to supply OpenID Connect discovery information.
+
+Kanidm provides
+[an Identity Provider Discovery for OIDC URL](https://datatracker.ietf.org/doc/html/rfc7033#section-3.1)
+response to all incoming WebFinger requests, using a user's SPN as their account
+ID. This does not match on email addresses as they are not guaranteed to be
+unique.
+
+However, WebFinger has a number of flaws which make it difficult to use with
+Kanidm:
+
+* WebFinger assumes that the identity provider will give the same `iss`
+  (Issuer) for every OAuth 2.0/OIDC client, and there is no standard way for a
+  WebFinger client to report its client ID.
+
+  Kanidm uses a *different* `iss` (Issuer) value for each client.
+
+* WebFinger requires that this be served at the *root* of the domain of a user's
+  SPN (ie: information about the user with SPN `user@idm.example.com` is at
+  `https://idm.example.com/.well-known/webfinger`).
+
+  Kanidm *does not* provide a WebFinger endpoint at its root URL, because it has
+  no way to know *which* OAuth 2.0/OIDC client a WebFinger request is associated
+  with, so could report an incorrect `iss` (Issuer).
+
+  You will need a load balancer in front of Kanidm's HTTPS server to redirect
+  requests to the appropriate `/oauth2/openid/:client_id:/.well-known/webfinger`
+  URL. If the client does not follow redirects, you may need to rewrite the
+  request in the load balancer instead.
+
+  If you have *multiple* WebFinger clients, it will need to map some other
+  property of the request (such as a source IP address or `User-Agent` header)
+  to a client ID, and redirect to the appropriate WebFinger URL for that client.
+
+* Kanidm responds to *all* WebFinger queries with
+  [an Identity Provider Discovery for OIDC URL](https://datatracker.ietf.org/doc/html/rfc7033#section-3.1),
+  **regardless** of what
+  [`rel` parameter](https://datatracker.ietf.org/doc/html/rfc7033#section-4.4.4.1)
+  was specified.
+
+  This is to work around
+  [a broken client](https://tailscale.com/kb/1240/sso-custom-oidc) which doesn't
+  send a `rel` parameter, but expects an Identity Provider Discovery issuer URL
+  in response.
+
+  If you want to use WebFinger in any *other* context on Kanidm's hostname,
+  you'll need a load balancer in front of Kanidm which matches on some property
+  of the request.
+
+Because of the flaws of the WebFinger specification and the deployment
+difficulties they introduce, we recommend that applications use OpenID Connect
+Discovery or OAuth 2.0 Authorisation Server Metadata for client configuration
+instead of WebFinger.

proto/src/internal/credupdate.rs

@@ -130,6 +130,7 @@ pub enum CURegState {
     None,
     TotpCheck(TotpSecret),
     TotpTryAgain,
+    TotpNameTryAgain(String),
     TotpInvalidSha1,
     BackupCodes(Vec<String>),
     Passkey(CreationChallengeResponse),

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/v1.rs

@@ -1396,6 +1396,7 @@ pub async fn credential_update_update(
             return Err(WebError::InternalServerError(errmsg));
         }
     };
+
     let session_token = match serde_json::from_value(cubody[1].clone()) {
         Ok(val) => val,
         Err(err) => {
@@ -1406,6 +1407,7 @@ pub async fn credential_update_update(
     };
     debug!("session_token: {:?}", session_token);
     debug!("scr: {:?}", scr);
+
     state
         .qe_r_ref
         .handle_idmcredentialupdate(session_token, scr, kopid.eventid)

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

@@ -47,6 +47,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));
@@ -120,6 +121,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",
+            post(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"));
 
     Router::new().merge(unguarded_router).merge(guarded_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 askama_axum::IntoResponse;
 use axum::extract::State;
+use axum::http::Uri;
 use axum::response::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_LEGALNAME, ATTR_MAIL};
 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::v1::Entry;
+use kanidmd_lib::filter::{f_eq, 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;
+use std::fmt;
+use std::fmt::Display;
+use std::fmt::Formatter;
 
 #[derive(Template)]
 #[template(path = "user_settings.html")]
@@ -26,9 +42,43 @@ pub(crate) struct ProfileView {
 struct ProfilePartialView {
     menu_active_item: ProfileMenuItems,
     can_rw: bool,
+    attrs: ProfileAttributes
+}
+
+#[derive(Clone, Debug, Serialize, Deserialize)]
+pub(crate) struct ProfileAttributes {
     account_name: String,
     display_name: String,
-    email: Option<String>,
+    legal_name: String,
+    #[serde(rename = "emails[]")]
+    emails: Vec<String>,
+    primary_email: Option<String>,
+}
+
+#[derive(Template, Clone)]
+#[template(path = "user_settings/profile_changes_partial.html")]
+struct ProfileChangesPartialView {
+    menu_active_item: ProfileMenuItems,
+    can_rw: bool,
+    attrs: ProfileAttributes,
+    new_attrs: ProfileAttributes,
+}
+
+#[derive(Template, Clone)]
+#[template(path = "user_settings/form_modifiable_entry_modifiable_list_partial.html")]
+// Modifiable entry in a modifiable list partial
+pub(crate) struct FormModEntryModListPartial {
+    can_rw: bool,
+    r#type: String,
+    name: String,
+    value: String,
+    invalid_feedback: String,
+}
+
+impl Display for ProfileAttributes {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        write!(f, "{self:?}")
+    }
 }
 
 pub(crate) async fn view_profile_get(
@@ -39,9 +89,23 @@ 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 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 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);
 
     let can_rw = uat.purpose_readwrite_active(time);
@@ -51,13 +115,185 @@ 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(),
+            attrs: ProfileAttributes {
+                account_name: uat.name().to_string(),
+                display_name: uat.displayname.clone(),
+                legal_name: "hardcoded".to_string(),
+                emails,
+                primary_email,
+            },
         },
     })
 }
 
+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(new_attrs): Form<ProfileAttributes>,
+) -> 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);
+
+    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();
+
+    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,
+        },
+        new_attrs
+    };
+
+    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(new_attrs): Form<ProfileAttributes>,
+) -> 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?;
+    dbg!(&new_attrs);
+
+    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(
+            client_auth_info.clone(),
+            uat.uuid.to_string(),
+            ATTR_DISPLAYNAME.to_string(),
+            vec![new_attrs.display_name],
+            filter.clone(),
+            kopid.eventid,
+        )
+        .map_err(|op_err| HtmxError::new(&kopid, op_err, domain_info.clone()))
+        .await?;
+
+    state
+        .qe_w_ref
+        .handle_setattribute(
+            client_auth_info.clone(),
+            uat.uuid.to_string(),
+            ATTR_MAIL.to_string(),
+            new_attrs.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(pv) => Ok(pv.into_response()),
+        Err(e) => Ok(e.into_response()),
+    }
+}
+
+// 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>,
+) -> axum::response::Result<Response> {
+    let passkey_init_trigger =
+        HxResponseTrigger::after_swap([HxEvent::new("addEmailSwapped".to_string())]);
+    Ok((
+        passkey_init_trigger,
+        FormModEntryModListPartial {
+            can_rw: true,
+            r#type: "email".to_string(),
+            name: "emails[]".to_string(),
+            value: "".to_string(),
+            invalid_feedback: "Please enter a valid email address.".to_string(),
+        },
+    )
+        .into_response())
+}
+
 pub(crate) async fn view_profile_unlock_get(
     State(state): State<ServerState>,
     VerifiedClientInformation(client_auth_info): VerifiedClientInformation,

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

@@ -210,6 +210,8 @@ pub(crate) struct TotpInit {
 pub(crate) struct TotpCheck {
     wrong_code: bool,
     broken_app: bool,
+    bad_name: bool,
+    taken_name: Option<String>,
 }
 
 #[derive(Template)]
@@ -599,6 +601,25 @@ pub(crate) async fn add_totp(
     let cu_session_token = get_cu_session(&jar).await?;
 
     let check_totpcode = u32::from_str(&new_totp_form.check_totpcode).unwrap_or_default();
+    let swapped_handler_trigger =
+        HxResponseTrigger::after_swap([HxEvent::new("addTotpSwapped".to_string())]);
+
+    // If the user has not provided a name or added only spaces we exit early
+    if new_totp_form.name.trim().is_empty() {
+        return Ok((
+            swapped_handler_trigger,
+            AddTotpPartial {
+                totp_init: None,
+                totp_name: "".into(),
+                totp_value: new_totp_form.check_totpcode.clone(),
+                check: TotpCheck {
+                    bad_name: true,
+                    ..Default::default()
+                },
+            },
+        )
+            .into_response());
+    }
 
     let cu_status = if new_totp_form.ignore_broken_app {
         // Cope with SHA1 apps because the user has intended to do so, their totp code was already verified
@@ -624,6 +645,10 @@ pub(crate) async fn add_totp(
             wrong_code: true,
             ..Default::default()
         },
+        CURegState::TotpNameTryAgain(val) => TotpCheck {
+            taken_name: Some(val.clone()),
+            ..Default::default()
+        },
         CURegState::TotpInvalidSha1 => TotpCheck {
             broken_app: true,
             ..Default::default()
@@ -646,9 +671,6 @@ pub(crate) async fn add_totp(
         new_totp_form.check_totpcode.clone()
     };
 
-    let swapped_handler_trigger =
-        HxResponseTrigger::after_swap([HxEvent::new("addTotpSwapped".to_string())]);
-
     Ok((
         swapped_handler_trigger,
         AddTotpPartial {

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) return
+
+        button.addEventListener("click", (e) => {
+            // Expected html nesting: li > div.input-group > button.kanidm-remove-list-entry
+            let li = button.parentElement?.parentElement;
+            if (li && li.tagName === "LI") {
+                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

@@ -20,6 +20,16 @@ body {
   max-width: 680px;
 }
 
+/*
+* Bootstrap 5.3 fix for input-group validation
+* :has checks that a child can be selected with the selector
+* + selects the next sibling.
+*/
+.was-validated .input-group:has(.form-control:invalid) + .invalid-feedback {
+  display: block !important;
+}
+
+
 /*
  * Sidebar
  */

server/core/templates/credential_update_add_totp_partial.html

@@ -19,7 +19,8 @@
             <label for="new-totp-name" class="form-label">Enter a name for your TOTP</label>
             <input
                     aria-describedby="totp-name-validation-feedback"
-                    class="form-control"
+                    class="form-control (%- if let Some(_) = check.taken_name -%)is-invalid(%- endif -%)
+                           (%- if check.bad_name -%)is-invalid(%- endif -%)"
                     name="name"
                     id="new-totp-name"
                     value="(( totp_name ))"
@@ -51,6 +52,18 @@
                         <li>Incorrect TOTP code - Please try again</li>
                     </ul>
                 </div>
+            (% else if check.bad_name %)
+                <div id="neq-totp-validation-feedback">
+                    <ul>
+                        <li>The name you provided was empty or blank. Please provide a proper name</li>
+                    </ul>
+                </div>
+            (% else if let Some(name) = check.taken_name %)
+                <div id="neq-totp-validation-feedback">
+                    <ul>
+                        <li>The name "((name))" is either invalid or already taken, Please pick a different one</li>
+                    </ul>
+                </div>
             (% endif %)
 
         </form>

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_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,89 @@
+(% extends "user_settings_partial_base.html" %)
+
+(% block selected_setting_group %)
+Profile Difference
+(% endblock %)
+
+(% block settings_vertical_point %)lg(% endblock %)
+
+(% block settings_window %)
+
+<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 ))"/>
+    (% for email in new_attrs.emails %)
+        <input type="hidden" name="emails[]" value="(( email ))"/>
+    (% endfor %)
+
+    <table class="table table-bordered table-responsive">
+        <thead>
+            <tr>
+                <th scope="col">Attribute</th>
+                <th scope="col">Old value</th>
+                <th scope="col">New value</th>
+            </tr>
+        </thead>
+    (% if attrs.account_name != new_attrs.account_name %)
+        <tr>
+            <th scope="row">User name</th>
+            <td class="text-break">(( attrs.account_name ))</td>
+            <td class="text-break">(( new_attrs.account_name ))</td>
+        </tr>
+    (% endif %)
+
+    (% if attrs.display_name != new_attrs.display_name %)
+        <tr>
+            <th scope="row">Display name</th>
+            <td class="text-break">(( attrs.display_name ))</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>
+                (% endfor %)
+                </ul>
+            </td>
+            <td class="text-break">
+                <ul>
+                (% for email in new_attrs.emails %)
+                    <li>(( email ))</li>
+                (% 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="#user_settings_container" hx-swap="outerHTML">Discard Changes</button>
+        <button class="btn btn-primary" type="button" hx-post="/ui/api/user_settings/confirm_profile" hx-target="#user_settings_container" 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

@@ -6,38 +6,56 @@ Profile
 
 (% block settings_window %)
 
-<form>
+<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-xl-2 col-form-label">User name</label>
+        <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="(( account_name ))">
-        </div>
-    </div>
-
-    <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>
+            <input type="text" readonly class="form-control-plaintext" id="profileUserName" value="(( attrs.account_name ))">
         </div>
     </div>
     
     <div class="mb-2 row">
-        <label for="profileEmail" class="col-12 col-md-3 col-xl-2 col-form-label">Email</label>
+        <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="email" disabled class="form-control-plaintext" id="profileEmail" value="(( email.clone().unwrap_or("None configured".to_string())))">
+            <input type="text" class="form-control-plaintext" id="profileDisplayName" value="(( attrs.display_name ))">
+        </div>
+    </div>
+
+    <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>
+
+    <div class="mb-2">
+        <label for="profileEmail" class="mb-2">Emails</label>
+        <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" %)
+                    (% endfor %)
+                </ul>
+            </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">
+    <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">Edit</button>
         (% else %)
-            <a href=(Urls::ProfileUnlock) hx-boost="false">
-                <button class="btn btn-primary" type="button">((UiMessage::UnlockEdit))</button>
+            <a href="/ui/profile/unlock" hx-boost="false">
+                <button class="btn btn-primary" type="button">Unlock Edit 🔒</button>
             </a>
         (% endif %)
-    </div> -->
+    </div>
 </form>
 
 (% endblock %)

server/lib/src/credential/mod.rs

@@ -702,6 +702,13 @@ impl Credential {
         }
     }
 
+    pub(crate) fn has_totp_by_name(&self, label: &str) -> bool {
+        match &self.type_ {
+            CredentialType::PasswordMfa(_, totp, _, _) => totp.contains_key(label),
+            _ => false,
+        }
+    }
+
     pub(crate) fn new_from_generatedpassword(pw: Password) -> Self {
         Credential {
             type_: CredentialType::GeneratedPassword(pw),

server/lib/src/idm/credupdatesession.rs

@@ -86,6 +86,7 @@ enum MfaRegState {
     None,
     TotpInit(Totp),
     TotpTryAgain(Totp),
+    TotpNameTryAgain(Totp, String),
     TotpInvalidSha1(Totp, Totp, String),
     Passkey(Box<CreationChallengeResponse>, PasskeyRegistration),
     #[allow(dead_code)]
@@ -98,6 +99,7 @@ impl fmt::Debug for MfaRegState {
             MfaRegState::None => "MfaRegState::None",
             MfaRegState::TotpInit(_) => "MfaRegState::TotpInit",
             MfaRegState::TotpTryAgain(_) => "MfaRegState::TotpTryAgain",
+            MfaRegState::TotpNameTryAgain(_, _) => "MfaRegState::TotpNameTryAgain",
             MfaRegState::TotpInvalidSha1(_, _, _) => "MfaRegState::TotpInvalidSha1",
             MfaRegState::Passkey(_, _) => "MfaRegState::Passkey",
             MfaRegState::AttestedPasskey(_, _) => "MfaRegState::AttestedPasskey",
@@ -273,6 +275,7 @@ pub enum MfaRegStateStatus {
     None,
     TotpCheck(TotpSecret),
     TotpTryAgain,
+    TotpNameTryAgain(String),
     TotpInvalidSha1,
     BackupCodes(HashSet<String>),
     Passkey(CreationChallengeResponse),
@@ -283,8 +286,9 @@ impl fmt::Debug for MfaRegStateStatus {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let t = match self {
             MfaRegStateStatus::None => "MfaRegStateStatus::None",
-            MfaRegStateStatus::TotpCheck(_) => "MfaRegStateStatus::TotpCheck(_)",
+            MfaRegStateStatus::TotpCheck(_) => "MfaRegStateStatus::TotpCheck",
             MfaRegStateStatus::TotpTryAgain => "MfaRegStateStatus::TotpTryAgain",
+            MfaRegStateStatus::TotpNameTryAgain(_) => "MfaRegStateStatus::TotpNameTryAgain",
             MfaRegStateStatus::TotpInvalidSha1 => "MfaRegStateStatus::TotpInvalidSha1",
             MfaRegStateStatus::BackupCodes(_) => "MfaRegStateStatus::BackupCodes",
             MfaRegStateStatus::Passkey(_) => "MfaRegStateStatus::Passkey",
@@ -389,6 +393,7 @@ impl Into<CUStatus> for CredentialUpdateSessionStatus {
                 MfaRegStateStatus::None => CURegState::None,
                 MfaRegStateStatus::TotpCheck(c) => CURegState::TotpCheck(c),
                 MfaRegStateStatus::TotpTryAgain => CURegState::TotpTryAgain,
+                MfaRegStateStatus::TotpNameTryAgain(label) => CURegState::TotpNameTryAgain(label),
                 MfaRegStateStatus::TotpInvalidSha1 => CURegState::TotpInvalidSha1,
                 MfaRegStateStatus::BackupCodes(s) => {
                     CURegState::BackupCodes(s.into_iter().collect())
@@ -469,6 +474,9 @@ impl From<&CredentialUpdateSession> for CredentialUpdateSessionStatus {
                 MfaRegState::TotpInit(token) => MfaRegStateStatus::TotpCheck(
                     token.to_proto(session.account.name.as_str(), session.issuer.as_str()),
                 ),
+                MfaRegState::TotpNameTryAgain(_, name) => {
+                    MfaRegStateStatus::TotpNameTryAgain(name.clone())
+                }
                 MfaRegState::TotpTryAgain(_) => MfaRegStateStatus::TotpTryAgain,
                 MfaRegState::TotpInvalidSha1(_, _, _) => MfaRegStateStatus::TotpInvalidSha1,
                 MfaRegState::Passkey(r, _) => MfaRegStateStatus::Passkey(r.as_ref().clone()),
@@ -1899,7 +1907,22 @@ impl IdmServerCredUpdateTransaction<'_> {
         match &session.mfaregstate {
             MfaRegState::TotpInit(totp_token)
             | MfaRegState::TotpTryAgain(totp_token)
+            | MfaRegState::TotpNameTryAgain(totp_token, _)
             | MfaRegState::TotpInvalidSha1(totp_token, _, _) => {
+                if session
+                    .primary
+                    .as_ref()
+                    .map(|cred| cred.has_totp_by_name(label))
+                    .unwrap_or_default()
+                    || label.trim().is_empty()
+                    || !Value::validate_str_escapes(label)
+                {
+                    // The user is trying to add a second TOTP under the same name. Lets save them from themselves
+                    session.mfaregstate =
+                        MfaRegState::TotpNameTryAgain(totp_token.clone(), label.into());
+                    return Ok(session.deref().into());
+                }
+
                 if totp_token.verify(totp_chal, ct) {
                     // It was valid. Update the credential.
                     let ncred = session
@@ -3368,10 +3391,39 @@ mod tests {
             .credential_primary_check_totp(&cust, ct, chal + 1, "totp")
             .expect("Failed to update the primary cred totp");
 
-        assert!(matches!(
-            c_status.mfaregstate,
-            MfaRegStateStatus::TotpTryAgain
-        ));
+        assert!(
+            matches!(c_status.mfaregstate, MfaRegStateStatus::TotpTryAgain),
+            "{:?}",
+            c_status.mfaregstate
+        );
+
+        // Check that the user actually put something into the label
+        let c_status = cutxn
+            .credential_primary_check_totp(&cust, ct, chal, "")
+            .expect("Failed to update the primary cred totp");
+
+        assert!(
+            matches!(
+                c_status.mfaregstate,
+                MfaRegStateStatus::TotpNameTryAgain(ref val) if val == ""
+            ),
+            "{:?}",
+            c_status.mfaregstate
+        );
+
+        // Okay, Now they are trying to be smart...
+        let c_status = cutxn
+            .credential_primary_check_totp(&cust, ct, chal, "           ")
+            .expect("Failed to update the primary cred totp");
+
+        assert!(
+            matches!(
+                c_status.mfaregstate,
+                MfaRegStateStatus::TotpNameTryAgain(ref val) if val == "           "
+            ),
+            "{:?}",
+            c_status.mfaregstate
+        );
 
         let c_status = cutxn
             .credential_primary_check_totp(&cust, ct, chal, "totp")
@@ -3383,6 +3435,40 @@ mod tests {
             _ => false,
         });
 
+        {
+            let c_status = cutxn
+                .credential_primary_init_totp(&cust, ct)
+                .expect("Failed to update the primary cred password");
+
+            // Check the status has the token.
+            let totp_token: Totp = match c_status.mfaregstate {
+                MfaRegStateStatus::TotpCheck(secret) => Some(secret.try_into().unwrap()),
+                _ => None,
+            }
+            .expect("Unable to retrieve totp token, invalid state.");
+
+            trace!(?totp_token);
+            let chal = totp_token
+                .do_totp_duration_from_epoch(&ct)
+                .expect("Failed to perform totp step");
+
+            // They tried to add a second totp under the same name
+            let c_status = cutxn
+                .credential_primary_check_totp(&cust, ct, chal, "totp")
+                .expect("Failed to update the primary cred totp");
+
+            assert!(
+                matches!(
+                    c_status.mfaregstate,
+                    MfaRegStateStatus::TotpNameTryAgain(ref val) if val == "totp"
+                ),
+                "{:?}",
+                c_status.mfaregstate
+            );
+
+            assert!(cutxn.credential_update_cancel_mfareg(&cust, ct).is_ok())
+        }
+
         // Should be okay now!
 
         drop(cutxn);

tools/cli/src/cli/person.rs

@@ -831,6 +831,13 @@ async fn totp_enroll_prompt(session_token: &CUSessionToken, client: &KanidmClien
 
     let label: String = Input::new()
         .with_prompt("TOTP Label")
+        .validate_with(|input: &String| -> Result<(), &str> {
+            if input.trim().is_empty() {
+                Err("Label cannot be empty")
+            } else {
+                Ok(())
+            }
+        })
         .interact_text()
         .expect("Failed to interact with interactive session");
 
@@ -919,6 +926,13 @@ async fn totp_enroll_prompt(session_token: &CUSessionToken, client: &KanidmClien
                 eprintln!("Incorrect TOTP code entered. Please try again.");
                 continue;
             }
+            Ok(CUStatus {
+                mfaregstate: CURegState::TotpNameTryAgain(label),
+                ..
+            }) => {
+                eprintln!("{label} is either invalid or already taken. Please try again.");
+                continue;
+            }
             Ok(CUStatus {
                 mfaregstate: CURegState::TotpInvalidSha1,
                 ..