[pbs-devel] [PATCH proxmox-backup] fix #3015: allow user self-service

[pbs-devel] [PATCH proxmox-backup] fix #3015: allow user self-service

[Fabian Grünbichler]

listing, updating or deleting a user is now possible for the user
itself, in addition to higher-privileged users that have appropriate
privileges on '/access/users'.

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

Notes:
    requires bumped proxmox crate with Permission::UserParam support

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

diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs
index ad50f2d9..432a48e1 100644
--- a/src/api2/access/user.rs
+++ b/src/api2/access/user.rs
@@ -8,6 +8,7 @@ use proxmox::tools::fs::open_file_locked;
 use crate::api2::types::*;
 use crate::config::user;
 use crate::config::acl::{PRIV_SYS_AUDIT, PRIV_PERMISSIONS_MODIFY};
+use crate::config::cached_user_info::CachedUserInfo;
 
 pub const PBS_PASSWORD_SCHEMA: Schema = StringSchema::new("User Password.")
     .format(&PASSWORD_FORMAT)
@@ -25,10 +26,11 @@ pub const PBS_PASSWORD_SCHEMA: Schema = StringSchema::new("User Password.")
         items: { type: user::User },
     },
     access: {
-        permission: &Permission::Privilege(&["access", "users"], PRIV_SYS_AUDIT, false),
+        permission: &Permission::Anybody,
+        description: "Returns all or just the logged-in user, depending on privileges.",
     },
 )]
-/// List all users
+/// List users
 pub fn list_users(
     _param: Value,
     _info: &ApiMethod,
@@ -37,11 +39,21 @@ pub fn list_users(
 
     let (config, digest) = user::config()?;
 
-    let list = config.convert_to_typed_array("user")?;
+    let userid: Userid = rpcenv.get_user().unwrap().parse()?;
+    let user_info = CachedUserInfo::new()?;
+
+    let top_level_privs = user_info.lookup_privs(&userid, &["access", "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
+    };
+
+    let list:Vec<user::User> = config.convert_to_typed_array("user")?;
 
     rpcenv["digest"] = proxmox::tools::digest_to_hex(&digest).into();
 
-    Ok(list)
+    Ok(list.into_iter().filter(filter_by_privs).collect())
 }
 
 #[api(
@@ -124,7 +136,10 @@ pub fn create_user(password: Option<String>, param: Value) -> Result<(), Error>
         type: user::User,
     },
     access: {
-        permission: &Permission::Privilege(&["access", "users"], PRIV_SYS_AUDIT, false),
+        permission: &Permission::Or(&[
+            &Permission::Privilege(&["access", "users"], PRIV_SYS_AUDIT, false),
+            &Permission::UserParam("userid"),
+        ]),
     },
 )]
 /// Read user configuration data.
@@ -177,7 +192,10 @@ pub fn read_user(userid: Userid, mut rpcenv: &mut dyn RpcEnvironment) -> Result<
         },
     },
     access: {
-        permission: &Permission::Privilege(&["access", "users"], PRIV_PERMISSIONS_MODIFY, false),
+        permission: &Permission::Or(&[
+            &Permission::Privilege(&["access", "users"], PRIV_PERMISSIONS_MODIFY, false),
+            &Permission::UserParam("userid"),
+        ]),
     },
 )]
 /// Update user configuration.
@@ -258,7 +276,10 @@ pub fn update_user(
         },
     },
     access: {
-        permission: &Permission::Privilege(&["access", "users"], PRIV_PERMISSIONS_MODIFY, false),
+        permission: &Permission::Or(&[
+            &Permission::Privilege(&["access", "users"], PRIV_PERMISSIONS_MODIFY, false),
+            &Permission::UserParam("userid"),
+        ]),
     },
 )]
 /// Remove a user from the configuration file.
-- 
2.20.1

[pbs-devel] [PATCH proxmox] permissions: introduce UserParam permission

[Fabian Grünbichler]

to safely differentiate between checking
- the current user matches some static string
- the current user matches the value in some (path) parameter.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
---
 proxmox/src/api/permission.rs | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/proxmox/src/api/permission.rs b/proxmox/src/api/permission.rs
index bbaf52f..a35e581 100644
--- a/proxmox/src/api/permission.rs
+++ b/proxmox/src/api/permission.rs
@@ -16,6 +16,8 @@ pub enum Permission {
     Anybody,
     /// Allow access for the specified user
     User(&'static str),
+    /// Allow access if specified param matches logged in user
+    UserParam(&'static str),
     /// Allow access for the specified group of users
     Group(&'static str),
     /// Use a parameter value as userid to run sub-permission tests.
@@ -45,6 +47,9 @@ impl fmt::Debug for Permission {
             Permission::User(ref userid) => {
                 write!(f, "User({})", userid)
             }
+            Permission::UserParam(param_name) => {
+                write!(f, "UserParam({})", param_name)
+            }
             Permission::Group(ref group) => {
                 write!(f, "Group({})", group)
             }
@@ -123,6 +128,13 @@ fn check_api_permission_tail(
                 Some(ref userid) => return userid == expected_userid,
             }
         }
+        Permission::UserParam(param_name) => {
+            match (userid, param.get(&param_name.to_string())) {
+                (None, _) => return false,
+                (_, None) => return false,
+                (Some(ref userid), Some(ref expected)) => return userid == expected,
+            }
+        }
         Permission::Group(expected_group) => {
             match userid {
                 None => return false,
-- 
2.20.1

Re: [pbs-devel] [PATCH proxmox] permissions: introduce UserParam permission

[Dietmar Maurer]

I am unable to apply that - Please rebase and send again.

[pbs-devel] applied: [PATCH proxmox-backup] fix #3015: allow user self-service

[Dietmar Maurer]

applied

Re: [pbs-devel] [PATCH proxmox] permissions: introduce UserParam permission

[Thomas Lamprecht]

On 9/18/20 3:51 PM, Dietmar Maurer wrote:
> I am unable to apply that - Please rebase and send again.

works fine here.

note that this is in "proxmox" not "proxmox-backup"

[pbs-devel] applied: Re: [PATCH proxmox] permissions: introduce UserParam permission

[Thomas Lamprecht]

On 9/18/20 3:01 PM, Fabian Grünbichler wrote:
> to safely differentiate between checking
> - the current user matches some static string
> - the current user matches the value in some (path) parameter.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> ---
>  proxmox/src/api/permission.rs | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
>

applied, thanks!

Re: [pbs-devel] [PATCH proxmox] permissions: introduce UserParam permission

[Dietmar Maurer]

> On 9/18/20 3:51 PM, Dietmar Maurer wrote:
> > I am unable to apply that - Please rebase and send again.
> 
> works fine here.
> 
> note that this is in "proxmox" not "proxmox-backup"

Oh, that makes sense now! Thanks for the help. I just build and uploaded new
packages.