Don't reprompt for login when no session exists in cli (#3082)

* Fix handling when session is already removed so that we don't re-prompt for login.
* Remove async recursion

Cargo.lock

@@ -259,17 +259,6 @@ dependencies = [
  "tokio",
 ]
 
-[[package]]
-name = "async-recursion"
-version = "1.1.1"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "3b43422f69d8ff38f95f1b2bb76517c91589a924d1559a0e935d7c8ce0274c11"
-dependencies = [
- "proc-macro2",
- "quote",
- "syn 2.0.79",
-]
-
 [[package]]
 name = "async-stream"
 version = "0.3.5"
@@ -3322,7 +3311,6 @@ name = "kanidm_tools"
 version = "1.4.0-dev"
 dependencies = [
  "anyhow",
- "async-recursion",
  "clap",
  "clap_complete",
  "compact_jwt 0.4.2",

Cargo.toml

@@ -142,7 +142,6 @@ sketching = { path = "./libs/sketching", version = "=1.4.0-dev" }
 anyhow = { version = "1.0.89" }
 argon2 = { version = "0.5.3", features = ["alloc"] }
 askama = { version = "0.12.1", features = ["serde"] }
-async-recursion = "1.1.0"
 async-trait = "^0.1.83"
 axum = { version = "0.7.7", features = [
     "form",

tools/cli/Cargo.toml

@@ -36,7 +36,6 @@ doctest = false
 
 [dependencies]
 anyhow = { workspace = true }
-async-recursion = { workspace = true }
 clap = { workspace = true, features = ["derive", "env"] }
 compact_jwt = { workspace = true, features = ["openssl"] }
 dialoguer = { workspace = true }

tools/cli/src/cli/common.rs

@@ -1,6 +1,5 @@
 use std::env;
 
-use async_recursion::async_recursion;
 use compact_jwt::{traits::JwsVerifiable, JwsCompact, JwsEs256Verifier, JwsVerifier, JwtError};
 use dialoguer::theme::ColorfulTheme;
 use dialoguer::{Confirm, Select};
@@ -22,7 +21,7 @@ pub enum OpType {
 #[derive(Debug)]
 pub enum ToClientError {
     NeedLogin(String),
-    NeedReauth(String),
+    NeedReauth(String, KanidmClient),
     Other,
 }
 
@@ -98,7 +97,10 @@ impl CommonOpt {
         })
     }
 
-    async fn try_to_client(&self, optype: OpType) -> Result<KanidmClient, ToClientError> {
+    pub(crate) async fn try_to_client(
+        &self,
+        optype: OpType,
+    ) -> Result<KanidmClient, ToClientError> {
         let client = self.to_unauth_client();
 
         // Read the token file.
@@ -247,6 +249,9 @@ impl CommonOpt {
                     }
                 }
 
+                // It's probably valid, set into the client
+                client.set_token(jwsc.to_string()).await;
+
                 // Check what we are doing based on op.
                 match optype {
                     OpType::Read => {}
@@ -256,7 +261,7 @@ impl CommonOpt {
                                 "Privileges have expired for {} - you need to re-authenticate again.",
                                 uat.spn
                             );
-                            return Err(ToClientError::NeedReauth(spn));
+                            return Err(ToClientError::NeedReauth(spn, client));
                         }
                     }
                 }
@@ -268,19 +273,15 @@ impl CommonOpt {
             }
         };
 
-        // Set it into the client
-        client.set_token(jwsc.to_string()).await;
-
         Ok(client)
     }
 
-    #[async_recursion]
     pub async fn to_client(&self, optype: OpType) -> KanidmClient {
+        let mut copt_mut = self.clone();
+        loop {
             match self.try_to_client(optype.clone()).await {
-            Ok(c) => c,
-            Err(e) => {
-                match e {
-                    ToClientError::NeedLogin(username) => {
+                Ok(c) => break c,
+                Err(ToClientError::NeedLogin(username)) => {
                     if !Confirm::new()
                         .with_prompt("Would you like to login again?")
                         .default(true)
@@ -289,19 +290,19 @@ impl CommonOpt {
                     {
                         std::process::exit(1);
                     }
-                        let mut copt = self.clone();
-                        copt.username = Some(username);
+
+                    copt_mut.username = Some(username);
+                    let copt = copt_mut.clone();
                     let login_opt = LoginOpt {
                         copt,
                         password: env::var("KANIDM_PASSWORD").ok(),
                     };
+
                     login_opt.exec().await;
-                        // we still use `to_client` instead of `try_to_client` because we may need to prompt user to re-auth again.
-                        // since reauth_opt will call `to_client`, this function is recursive anyway.
-                        // we use copt since it's username is updated.
-                        return login_opt.copt.to_client(optype).await;
+                    // Okay, try again ...
+                    continue;
                 }
-                    ToClientError::NeedReauth(username) => {
+                Err(ToClientError::NeedReauth(username, client)) => {
                     if !Confirm::new()
                         .with_prompt("Would you like to re-authenticate?")
                         .default(true)
@@ -310,22 +311,18 @@ impl CommonOpt {
                     {
                         std::process::exit(1);
                     }
-                        let mut copt = self.clone();
-                        copt.username = Some(username);
+                    copt_mut.username = Some(username);
+                    let copt = copt_mut.clone();
                     let reauth_opt = ReauthOpt { copt };
-                        // calls `to_client` recursively
-                        // but should not goes into `NeedLogin` branch again
-                        reauth_opt.exec().await;
-                        if let Ok(c) = reauth_opt.copt.try_to_client(optype).await {
-                            return c;
+                    reauth_opt.inner(client).await;
+
+                    // Okay, re-auth should have passed, lets loop
+                    continue;
                 }
-                    }
-                    ToClientError::Other => {
+                Err(ToClientError::Other) => {
                     std::process::exit(1);
                 }
             }
-                std::process::exit(1);
-            }
         }
     }
 }

tools/cli/src/cli/session.rs

@@ -1,4 +1,4 @@
-use crate::common::OpType;
+use crate::common::{OpType, ToClientError};
 use std::cmp::Reverse;
 use std::collections::BTreeMap;
 use std::fs::{create_dir, File};
@@ -569,9 +569,7 @@ impl ReauthOpt {
         self.copt.debug
     }
 
-    pub async fn exec(&self) {
-        let client = self.copt.to_client(OpType::Read).await;
-
+    pub(crate) async fn inner(&self, client: KanidmClient) {
         let instance_name = &self.copt.instance;
 
         let allowed = client.reauth_begin().await.unwrap_or_else(|e| {
@@ -581,6 +579,12 @@ impl ReauthOpt {
 
         process_auth_state(allowed, client, &None, instance_name).await;
     }
+
+    pub async fn exec(&self) {
+        let client = self.copt.to_client(OpType::Read).await;
+        // This is to break a recursion loop in re-auth with to_client
+        self.inner(client).await
+    }
 }
 
 impl LogoutOpt {
@@ -626,7 +630,18 @@ impl LogoutOpt {
                 }
             }
         } else {
-            let client = self.copt.to_client(OpType::Read).await;
+            let client = match self.copt.try_to_client(OpType::Read).await {
+                Ok(c) => c,
+                Err(ToClientError::NeedLogin(_)) => {
+                    // There are no session tokens, so return a success.
+                    std::process::exit(0);
+                }
+                Err(ToClientError::NeedReauth(_, _)) | Err(ToClientError::Other) => {
+                    // This can only occur in bad cases, so fail.
+                    std::process::exit(1);
+                }
+            };
+
             let token = match client.get_token().await {
                 Some(t) => t,
                 None => {