[pbs-devel] [PATCH proxmox-backup 1/2] api: improve error messages for restricted endpoints

[pbs-devel] [PATCH proxmox-backup 1/2] api: improve error messages for restricted endpoints

[Fabian Grünbichler]

the old variant attempted to parse a tokenid as userid and returned the
cryptic parsing error to the client, which is rather confusing.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    not sure whether this is already enough repetition to have a helper somewhere?
    I expect the number of such API endpoints to remain rather small.. the
    change_password one could even be lifted with small changes..

 src/api2/access.rs | 14 +++++++++-----
 src/api2/node.rs   | 22 ++++++++++++++++++----
 2 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/src/api2/access.rs b/src/api2/access.rs
index 2f7fb6ec..22d6ebd2 100644
--- a/src/api2/access.rs
+++ b/src/api2/access.rs
@@ -206,14 +206,18 @@ fn change_password(
     password: String,
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Value, Error> {
-
-    let current_user: Userid = rpcenv
+    let current_auth: Authid = rpcenv
         .get_auth_id()
-        .ok_or_else(|| format_err!("unknown user"))?
+        .ok_or_else(|| format_err!("no authid available"))?
         .parse()?;
-    let current_auth = Authid::from(current_user.clone());
 
-    let mut allowed = userid == current_user;
+    if current_auth.is_token() {
+        bail!("API tokens cannot access this API endpoint");
+    }
+
+    let current_user = current_auth.user();
+
+    let mut allowed = userid == *current_user;
 
     if current_user == "root@pam" { allowed = true; }
 
diff --git a/src/api2/node.rs b/src/api2/node.rs
index dcde83df..b1a25e0e 100644
--- a/src/api2/node.rs
+++ b/src/api2/node.rs
@@ -92,11 +92,16 @@ async fn termproxy(
     rpcenv: &mut dyn RpcEnvironment,
 ) -> Result<Value, Error> {
     // intentionally user only for now
-    let userid: Userid = rpcenv
+    let auth_id: Authid = rpcenv
         .get_auth_id()
-        .ok_or_else(|| format_err!("unknown user"))?
+        .ok_or_else(|| format_err!("no authid available"))?
         .parse()?;
-    let auth_id = Authid::from(userid.clone());
+
+    if auth_id.is_token() {
+        bail!("API tokens cannot access this API endpoint");
+    }
+
+    let userid = auth_id.user();
 
     if userid.realm() != "pam" {
         bail!("only pam users can use the console");
@@ -267,7 +272,16 @@ fn upgrade_to_websocket(
 ) -> ApiResponseFuture {
     async move {
         // intentionally user only for now
-        let userid: Userid = rpcenv.get_auth_id().unwrap().parse()?;
+        let auth_id: Authid = rpcenv
+            .get_auth_id()
+            .ok_or_else(|| format_err!("no authid available"))?
+            .parse()?;
+
+        if auth_id.is_token() {
+            bail!("API tokens cannot access this API endpoint");
+        }
+
+        let userid = auth_id.user();
         let ticket = tools::required_string_param(&param, "vncticket")?;
         let port: u16 = tools::required_integer_param(&param, "port")? as u16;
 
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 2/2] api: allow tokens to list users

[Fabian Grünbichler]

their owner, or all if they have the appropriate privileges.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---

Notes:
    this seems benign enough. an otherwise unprivileged API token can still not
    list any tokens, but that would require more changes..

 src/api2/access/user.rs | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs
index 72c79def..9ac9bbfa 100644
--- a/src/api2/access/user.rs
+++ b/src/api2/access/user.rs
@@ -1,4 +1,4 @@
-use anyhow::{bail, Error};
+use anyhow::{bail, format_err, Error};
 use serde::{Serialize, Deserialize};
 use serde_json::{json, Value};
 use std::collections::HashMap;
@@ -94,7 +94,6 @@ impl UserWithTokens {
     }
 }
 
-
 #[api(
     input: {
         properties: {
@@ -113,7 +112,7 @@ impl UserWithTokens {
     },
     access: {
         permission: &Permission::Anybody,
-        description: "Returns all or just the logged-in user, depending on privileges.",
+        description: "Returns all or just the logged-in user (/API token owner), depending on privileges.",
     },
 )]
 /// List users
@@ -125,9 +124,12 @@ pub fn list_users(
 
     let (config, digest) = user::config()?;
 
-    // intentionally user only for now
-    let userid: Userid = rpcenv.get_auth_id().unwrap().parse()?;
-    let auth_id = Authid::from(userid.clone());
+    let auth_id: Authid = rpcenv
+        .get_auth_id()
+        .ok_or_else(|| format_err!("no authid available"))?
+        .parse()?;
+
+    let userid = auth_id.user();
 
     let user_info = CachedUserInfo::new()?;
 
@@ -135,7 +137,7 @@ pub fn list_users(
     let top_level_allowed = (top_level_privs & PRIV_SYS_AUDIT) != 0;
 
     let filter_by_privs = |user: &user::User| {
-        top_level_allowed || user.userid == userid
+        top_level_allowed || user.userid == *userid
     };
 
 
-- 
2.20.1

[pbs-devel] applied: [PATCH proxmox-backup 1/2] api: improve error messages for restricted endpoints

[Dietmar Maurer]

applied both patches