[pbs-devel] [PATCH pbs 2/2] fix #4734: manager: add tfa {list, delete, add} commands

[Maximiliano Sandoval <m.sandoval@proxmox.com>]

Adds the commands

    proxmox-backup-manager tfa list <userid>
    proxmox-backup-manager tfa delete <userid> <id>
    proxmox-backup-manager tfa add <userid> <type>

The rationale for not listing these commands under
proxmox-backup-manager user is that in this way we match the API better.

The command `tfa add` will present the user with a scannable QR code and
a prompt to enter the associated 6 digit token for validation.

The function api2::access::list_user_tfa needed an additional `returns`
annotation, otherwise one gets the following error

    unable to format result: got unexpected data (expected null).

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 pbs-datastore/src/paperkey.rs         |   2 +-
 src/api2/access/tfa.rs                |  11 +-
 src/bin/proxmox-backup-manager.rs     |   1 +
 src/bin/proxmox_backup_manager/mod.rs |   2 +
 src/bin/proxmox_backup_manager/tfa.rs | 181 ++++++++++++++++++++++++++
 src/config/tfa.rs                     |  48 +++++++
 6 files changed, 241 insertions(+), 4 deletions(-)
 create mode 100644 src/bin/proxmox_backup_manager/tfa.rs

diff --git a/pbs-datastore/src/paperkey.rs b/pbs-datastore/src/paperkey.rs
index 6d8cd914..11dc8fe3 100644
--- a/pbs-datastore/src/paperkey.rs
+++ b/pbs-datastore/src/paperkey.rs
@@ -225,7 +225,7 @@ fn paperkey_text<W: Write>(
     Ok(())
 }
 
-fn generate_qr_code(output_type: &str, lines: &[String]) -> Result<Vec<u8>, Error> {
+pub fn generate_qr_code(output_type: &str, lines: &[String]) -> Result<Vec<u8>, Error> {
     let padding = match output_type {
         "utf8" | "utf8i" => "-m2",
         _ => "-m0",
diff --git a/src/api2/access/tfa.rs b/src/api2/access/tfa.rs
index 82816d21..595c9433 100644
--- a/src/api2/access/tfa.rs
+++ b/src/api2/access/tfa.rs
@@ -54,6 +54,11 @@ async fn tfa_update_auth(
     input: {
         properties: { userid: { type: Userid } },
     },
+    returns: {
+        description: "The list of TFA entries.",
+        type: Array,
+        items: { type: methods::TypedTfaInfo }
+    },
     access: {
         permission: &Permission::Or(&[
             &Permission::Privilege(&["access", "users"], PRIV_PERMISSIONS_MODIFY, false),
@@ -62,7 +67,7 @@ async fn tfa_update_auth(
     },
 )]
 /// Add a TOTP secret to the user.
-fn list_user_tfa(userid: Userid) -> Result<Vec<methods::TypedTfaInfo>, Error> {
+pub fn list_user_tfa(userid: Userid) -> Result<Vec<methods::TypedTfaInfo>, Error> {
     let _lock = crate::config::tfa::read_lock()?;
 
     methods::list_user_tfa(&crate::config::tfa::read()?, userid.as_str())
@@ -115,7 +120,7 @@ fn get_tfa_entry(userid: Userid, id: String) -> Result<methods::TypedTfaInfo, Er
     },
 )]
 /// Delete a single TFA entry.
-async fn delete_tfa(
+pub async fn delete_tfa(
     userid: Userid,
     id: String,
     password: Option<String>,
@@ -208,7 +213,7 @@ fn list_tfa(rpcenv: &mut dyn RpcEnvironment) -> Result<Vec<methods::TfaUser>, Er
 )]
 /// Add a TFA entry to the user.
 #[allow(clippy::too_many_arguments)]
-async fn add_tfa_entry(
+pub async fn add_tfa_entry(
     userid: Userid,
     description: Option<String>,
     totp: Option<String>,
diff --git a/src/bin/proxmox-backup-manager.rs b/src/bin/proxmox-backup-manager.rs
index b4cb6cb3..a8c1046c 100644
--- a/src/bin/proxmox-backup-manager.rs
+++ b/src/bin/proxmox-backup-manager.rs
@@ -440,6 +440,7 @@ async fn run() -> Result<(), Error> {
         .insert("network", network_commands())
         .insert("node", node_commands())
         .insert("user", user_commands())
+        .insert("tfa", tfa_commands())
         .insert("openid", openid_commands())
         .insert("remote", remote_commands())
         .insert("traffic-control", traffic_control_commands())
diff --git a/src/bin/proxmox_backup_manager/mod.rs b/src/bin/proxmox_backup_manager/mod.rs
index 8a1c140c..4a2e6189 100644
--- a/src/bin/proxmox_backup_manager/mod.rs
+++ b/src/bin/proxmox_backup_manager/mod.rs
@@ -32,3 +32,5 @@ mod openid;
 pub use openid::*;
 mod traffic_control;
 pub use traffic_control::*;
+mod tfa;
+pub use tfa::*;
diff --git a/src/bin/proxmox_backup_manager/tfa.rs b/src/bin/proxmox_backup_manager/tfa.rs
new file mode 100644
index 00000000..9a1bd073
--- /dev/null
+++ b/src/bin/proxmox_backup_manager/tfa.rs
@@ -0,0 +1,181 @@
+use std::io::Write;
+use std::str::FromStr;
+
+use anyhow::{bail, Error};
+use serde_json::{json, Value};
+
+use pbs_api_types::Userid;
+use proxmox_backup::api2;
+use proxmox_router::{cli::*, ApiHandler, RpcEnvironment};
+use proxmox_schema::{api, api_types::PASSWORD_SCHEMA};
+use proxmox_sys::linux::tty;
+use proxmox_tfa::{api::methods, totp::Totp};
+
+fn generate_totp_uri(issuer: &str, account_name: &str) -> Result<String, Error> {
+    // Generate a CSPRNG binary value of 160 bits, the recomended size
+    // from [rfc-4226](https://www.rfc-editor.org/rfc/rfc4226#section-4)
+    //
+    // > The length of the shared secret MUST be at least 128 bits.
+    // > This document RECOMMENDs a shared secret length of 160 bits.
+    //
+    // The generated secret is not guaranteed to be a valid UTF-8
+    // sequence
+    //
+    // See https://github.com/constantoine/totp-rs/blob/e4e055dedddc012cd691dc7156ee707843dd8105/src/secret.rs#L166
+    let secret = proxmox_sys::linux::random_data(20)?;
+
+    let totp = Totp::builder()
+        .secret(secret)
+        .issuer(issuer.to_string())
+        .account_name(account_name.to_string())
+        .build();
+
+    Ok(totp.to_uri()?)
+}
+
+#[api(
+    input: {
+        properties: {
+            "output-format": {
+                schema: OUTPUT_FORMAT,
+                optional: true,
+            },
+            userid: {
+                type: Userid,
+            }
+        },
+    }
+)]
+/// List configured users.
+fn list_user_tfa(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
+    let output_format = get_output_format(&param);
+
+    let info = &api2::access::tfa::API_METHOD_LIST_USER_TFA;
+    let mut data = match info.handler {
+        ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
+        _ => unreachable!(),
+    };
+
+    let options = default_table_format_options()
+        .column(ColumnConfig::new("id"))
+        .column(ColumnConfig::new("type"))
+        .column(ColumnConfig::new("description"))
+        .column(ColumnConfig::new("created").renderer(pbs_tools::format::render_epoch));
+
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
+
+    Ok(Value::Null)
+}
+
+#[api(
+    input: {
+        properties: {
+            "output-format": {
+                schema: OUTPUT_FORMAT,
+                optional: true,
+            },
+            userid: {
+                type: Userid,
+            },
+            "type": {
+                type: methods::TfaType,
+            },
+            description: {
+                description: "A description to distinguish multiple entries from one another",
+                type: String,
+                max_length: 255,
+                optional: true,
+            },
+            password: {
+                schema: PASSWORD_SCHEMA,
+                optional: true,
+            },
+        },
+    },
+    returns: { type: methods::TfaUpdateInfo },
+)]
+/// Add a tfa entry for a user
+async fn add_tfa_entry(param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
+    let output_format = get_output_format(&param);
+
+    let info = &api2::access::tfa::API_METHOD_ADD_TFA_ENTRY;
+
+    let mut param = param;
+
+    match param
+        .get("type")
+        .and_then(|v| v.as_str())
+        .and_then(|s| methods::TfaType::from_str(s).ok())
+    {
+        Some(methods::TfaType::Totp) => {
+            if !tty::stdin_isatty() {
+                bail!("unable to generate qr code - no tty");
+            }
+            if param.get("description").is_none() {
+                bail!("Error: 'description' is required for new totp entries");
+            }
+
+            let userid = param
+                .get("userid")
+                .and_then(|v| v.as_str())
+                .unwrap_or("User");
+
+            let uri = generate_totp_uri("Proxmox", userid)?;
+            param["totp"] = json!(&uri);
+
+            // We use utf8 as it displays the codes as black on white which is
+            // readed by most qr scanners.
+            let qrcode = pbs_datastore::paperkey::generate_qr_code("utf8", &[uri])?;
+            let stdout = std::io::stdout();
+
+            let mut writer = stdout.lock();
+
+            writer.write(&qrcode)?;
+            writer.write(b"Scan QR code in a TOTP app and enter an authentication code here\n")?;
+
+            let value = String::from_utf8(tty::read_password("Verify Code: ")?)?;
+
+            param["value"] = json!(value);
+        }
+        Some(methods::TfaType::Recovery) => (),
+        Some(methods::TfaType::Webauthn) => bail!("WebAuthn is not supported from cli"),
+        Some(methods::TfaType::U2f) => bail!("U2F is not supported from cli"),
+        Some(methods::TfaType::Yubico) => bail!("Yubico is not supported from cli"),
+        None => unreachable!(),
+    }
+    let mut data = match info.handler {
+        ApiHandler::Async(handler) => (handler)(param, info, rpcenv).await?,
+        _ => unreachable!(),
+    };
+
+    let options = default_table_format_options().column(ColumnConfig::new("id"));
+
+    format_and_print_result_full(&mut data, &info.returns, &output_format, &options);
+
+    Ok(Value::Null)
+}
+
+pub fn tfa_commands() -> CommandLineInterface {
+    let cmd_def = CliCommandMap::new()
+        .insert(
+            "list",
+            CliCommand::new(&API_METHOD_LIST_USER_TFA)
+                .arg_param(&["userid"])
+                .completion_cb("userid", pbs_config::user::complete_userid),
+        )
+        .insert(
+            "add",
+            CliCommand::new(&API_METHOD_ADD_TFA_ENTRY)
+                .arg_param(&["userid", "type"])
+                .completion_cb("userid", pbs_config::user::complete_userid),
+        )
+        .insert(
+            "delete",
+            CliCommand::new(&api2::access::tfa::API_METHOD_DELETE_TFA)
+                .arg_param(&["userid", "id"])
+                .completion_cb("userid", pbs_config::user::complete_userid)
+                .completion_cb("id", proxmox_backup::config::tfa::complete_tfa_id),
+        );
+
+    cmd_def.into()
+}
diff --git a/src/config/tfa.rs b/src/config/tfa.rs
index 6b1312bb..bc5a2f85 100644
--- a/src/config/tfa.rs
+++ b/src/config/tfa.rs
@@ -1,3 +1,4 @@
+use std::collections::HashMap;
 use std::fs::File;
 use std::io::{self, Read, Seek, SeekFrom};
 use std::os::unix::fs::OpenOptionsExt;
@@ -302,3 +303,50 @@ impl proxmox_tfa::api::UserChallengeAccess for TfaUserChallengeData {
         TfaUserChallengeData::save(self)
     }
 }
+
+// shell completion helper
+pub fn complete_tfa_id(_arg: &str, param: &HashMap<String, String>) -> Vec<String> {
+    let mut results = Vec::new();
+
+    let json = match read() {
+        Ok(json) => json,
+        Err(_err) => return results,
+    };
+    let user = match param
+        .get("userid")
+        .and_then(|user_name| json.users.get(user_name))
+    {
+        Some(user) => user,
+        None => return results,
+    };
+
+    results.extend(
+        user.totp
+            .iter()
+            .map(|token| token.info.id.clone())
+            .collect::<Vec<_>>(),
+    );
+    results.extend(
+        user.u2f
+            .iter()
+            .map(|token| token.info.id.clone())
+            .collect::<Vec<_>>(),
+    );
+    results.extend(
+        user.webauthn
+            .iter()
+            .map(|token| token.info.id.clone())
+            .collect::<Vec<_>>(),
+    );
+    results.extend(
+        user.yubico
+            .iter()
+            .map(|token| token.info.id.clone())
+            .collect::<Vec<_>>(),
+    );
+    if user.recovery.is_some() {
+        results.push("recovery".to_string());
+    };
+
+    results
+}
-- 
2.39.2