Bug chasing (#2257)

* service-account validity expire-at doesn't accept all time nouns as defined by docs
Fixes #2153
* realised a logic bug
* making clippy happy while I'm here
* returning an empty set from the creds if the creds attribute is not found, which is then handled downstream

server/core/src/https/v1.rs

@@ -1336,12 +1336,22 @@ pub async fn service_account_id_credential_status_get(
     Extension(kopid): Extension<KOpId>,
     Path(id): Path<String>,
 ) -> Result<Json<CredentialStatus>, WebError> {
-    state
+    match state
         .qe_r_ref
-        .handle_idmcredentialstatus(kopid.uat, id, kopid.eventid)
+        .handle_idmcredentialstatus(kopid.uat, id.clone(), kopid.eventid)
         .await
         .map(Json::from)
-        .map_err(WebError::from)
+    {
+        Ok(val) => Ok(val),
+        Err(err) => {
+            if let OperationError::NoMatchingAttributes = err {
+                debug!("No credentials set on account {}, returning empty list", id);
+                Ok(Json(CredentialStatus { creds: Vec::new() }))
+            } else {
+                Err(WebError::from(err))
+            }
+        }
+    }
 }
 
 #[utoipa::path(
@@ -1362,12 +1372,22 @@ pub async fn person_get_id_credential_status(
     Extension(kopid): Extension<KOpId>,
     Path(id): Path<String>,
 ) -> Result<Json<CredentialStatus>, WebError> {
-    state
+    match state
         .qe_r_ref
-        .handle_idmcredentialstatus(kopid.uat, id, kopid.eventid)
+        .handle_idmcredentialstatus(kopid.uat, id.clone(), kopid.eventid)
         .await
         .map(Json::from)
-        .map_err(WebError::from)
+    {
+        Ok(val) => Ok(val),
+        Err(err) => {
+            if let OperationError::NoMatchingAttributes = err {
+                debug!("No credentials set on person {}, returning empty list", id);
+                Ok(Json(CredentialStatus { creds: Vec::new() }))
+            } else {
+                Err(WebError::from(err))
+            }
+        }
+    }
 }
 
 #[utoipa::path(

tools/cli/src/cli/common.rs

@@ -8,6 +8,8 @@ use dialoguer::{Confirm, Select};
 use kanidm_client::{KanidmClient, KanidmClientBuilder};
 use kanidm_proto::constants::{DEFAULT_CLIENT_CONFIG_PATH, DEFAULT_CLIENT_CONFIG_PATH_HOME};
 use kanidm_proto::v1::UserAuthToken;
+use time::format_description::well_known::Rfc3339;
+use time::OffsetDateTime;
 
 use crate::session::read_tokens;
 use crate::{CommonOpt, LoginOpt, ReauthOpt};
@@ -375,3 +377,37 @@ pub fn prompt_for_username_get_token() -> Result<String, String> {
     }
 }
 */
+
+/// This parses the input for the person/service-account expire-at CLI commands
+///
+/// If it fails, return error, if it needs to *clear* the result, return Ok(None),
+/// otherwise return Ok(Some(String)) which is the new value to set.
+pub(crate) fn try_expire_at_from_string(input: &str) -> Result<Option<String>, ()> {
+    match input {
+        "any" | "never" | "clear" => Ok(None),
+        "now" => match OffsetDateTime::now_utc().format(&Rfc3339) {
+            Ok(s) => Ok(Some(s)),
+            Err(e) => {
+                error!(err = ?e, "Unable to format current time to rfc3339");
+                Err(())
+            }
+        },
+        "epoch" => match OffsetDateTime::UNIX_EPOCH.format(&Rfc3339) {
+            Ok(val) => Ok(Some(val)),
+            Err(err) => {
+                error!("Failed to format epoch timestamp as RFC3339: {:?}", err);
+                Err(())
+            }
+        },
+        _ => {
+            // fall back to parsing it as a date
+            match OffsetDateTime::parse(input, &Rfc3339) {
+                Ok(_) => Ok(Some(input.to_string())),
+                Err(err) => {
+                    error!("Failed to parse supplied timestamp: {:?}", err);
+                    Err(())
+                }
+            }
+        }
+    }
+}

tools/cli/src/cli/person.rs

@@ -1,4 +1,4 @@
-use crate::common::OpType;
+use crate::common::{try_expire_at_from_string, OpType};
 use std::fmt::{self, Debug};
 use std::str::FromStr;
 
@@ -410,78 +410,33 @@ impl PersonOpt {
                 }
                 AccountValidity::ExpireAt(ano) => {
                     let client = ano.copt.to_client(OpType::Write).await;
-                    if matches!(ano.datetime.as_str(), "never" | "clear") {
+                    let validity = match try_expire_at_from_string(ano.datetime.as_str()) {
-                        // Unset the value
+                        Ok(val) => val,
-                        match client
+                        Err(()) => return,
+                    };
+                    let res = match validity {
+                        None => {
+                            client
                                 .idm_person_account_purge_attr(
                                     ano.aopts.account_id.as_str(),
                                     ATTR_ACCOUNT_EXPIRE,
                                 )
                                 .await
-                        {
-                            Err(e) => handle_client_error(e, &ano.copt.output_mode),
-                            _ => println!("Success"),
                         }
-                    } else if matches!(ano.datetime.as_str(), "now") {
+                        Some(new_expiry) => {
-                        // set the expiry to *now*
+                            client
-                        let now = match OffsetDateTime::now_utc().format(&Rfc3339) {
+                                .idm_person_account_set_attr(
-                            Ok(s) => s,
+                                    ano.aopts.account_id.as_str(),
-                            Err(e) => {
+                                    ATTR_ACCOUNT_EXPIRE,
-                                error!(err = ?e, "Unable to format current time to rfc3339");
+                                    &[&new_expiry],
-                                return;
+                                )
+                                .await
                         }
                     };
-                        debug!("Setting expiry to {}", now);
+                    match res {
-                        match client
-                            .idm_person_account_set_attr(
-                                ano.aopts.account_id.as_str(),
-                                ATTR_ACCOUNT_EXPIRE,
-                                &[&now],
-                            )
-                            .await
-                        {
-                            Err(e) => error!("Error setting expiry to 'now' -> {:?}", e),
-                            _ => println!("Success"),
-                        }
-                    } else if matches!(ano.datetime.as_str(), "epoch") {
-                        // set the expiry to the epoch
-                        let epoch_str = match OffsetDateTime::UNIX_EPOCH.format(&Rfc3339) {
-                            Ok(s) => s,
-                            Err(e) => {
-                                error!(err = ?e, "Unable to format unix epoch to rfc3339");
-                                return;
-                            }
-                        };
-                        debug!("Setting expiry to {}", epoch_str);
-                        match client
-                            .idm_person_account_set_attr(
-                                ano.aopts.account_id.as_str(),
-                                ATTR_ACCOUNT_EXPIRE,
-                                &[&epoch_str],
-                            )
-                            .await
-                        {
-                            Err(e) => error!("Error setting expiry to 'epoch' -> {:?}", e),
-                            _ => println!("Success"),
-                        }
-                    } else {
-                        if let Err(e) = OffsetDateTime::parse(ano.datetime.as_str(), &Rfc3339) {
-                            error!("Error -> {:?}", e);
-                            return;
-                        }
-
-                        match client
-                            .idm_person_account_set_attr(
-                                ano.aopts.account_id.as_str(),
-                                ATTR_ACCOUNT_EXPIRE,
-                                &[ano.datetime.as_str()],
-                            )
-                            .await
-                        {
                         Err(e) => handle_client_error(e, &ano.copt.output_mode),
                         _ => println!("Success"),
-                        }
+                    };
-                    }
                 }
                 AccountValidity::BeginFrom(ano) => {
                     let client = ano.copt.to_client(OpType::Write).await;

tools/cli/src/cli/serviceaccount.rs

@@ -1,4 +1,4 @@
-use crate::common::OpType;
+use crate::common::{try_expire_at_from_string, OpType};
 use kanidm_proto::constants::{ATTR_ACCOUNT_EXPIRE, ATTR_ACCOUNT_VALID_FROM};
 use kanidm_proto::messages::{AccountChangeMessage, ConsoleOutputMode, MessageStatus};
 use time::OffsetDateTime;
@@ -423,36 +423,33 @@ impl ServiceAccountOpt {
                 }
                 AccountValidity::ExpireAt(ano) => {
                     let client = ano.copt.to_client(OpType::Write).await;
-                    if matches!(ano.datetime.as_str(), "never" | "clear") {
+                    let validity = match try_expire_at_from_string(ano.datetime.as_str()) {
-                        // Unset the value
+                        Ok(val) => val,
-                        match client
+                        Err(()) => return,
+                    };
+                    let res = match validity {
+                        None => {
+                            client
                                 .idm_service_account_purge_attr(
                                     ano.aopts.account_id.as_str(),
                                     ATTR_ACCOUNT_EXPIRE,
                                 )
                                 .await
-                        {
-                            Err(e) => error!("Error -> {:?}", e),
-                            _ => println!("Success"),
                         }
-                    } else {
+                        Some(new_expiry) => {
-                        if let Err(e) = OffsetDateTime::parse(ano.datetime.as_str(), &Rfc3339) {
+                            client
-                            error!("Error -> {:?}", e);
-                            return;
-                        }
-
-                        match client
                                 .idm_service_account_set_attr(
                                     ano.aopts.account_id.as_str(),
                                     ATTR_ACCOUNT_EXPIRE,
-                                &[ano.datetime.as_str()],
+                                    &[&new_expiry],
                                 )
                                 .await
-                        {
+                        }
+                    };
+                    match res {
                         Err(e) => handle_client_error(e, &ano.copt.output_mode),
                         _ => println!("Success"),
-                        }
+                    };
-                    }
                 }
                 AccountValidity::BeginFrom(ano) => {
                     let client = ano.copt.to_client(OpType::Write).await;