* [pbs-devel] [PATCH proxmox-backup] fix: config: remove duplicate privilege lookup in cached_user_info
@ 2022-06-10 8:13 Stefan Sterz
2022-06-10 8:52 ` Wolfgang Bumiller
2022-06-10 9:31 ` [pbs-devel] applied: " Wolfgang Bumiller
0 siblings, 2 replies; 4+ messages in thread
From: Stefan Sterz @ 2022-06-10 8:13 UTC (permalink / raw)
To: pbs-devel
`lookup_privs` just uses `lookup_privs_details` but ignores the
propagated privileges it returns. thus, the lookup here is redundant
as it is immediately followed by a call to `lookup_privs_details` with
the same parameters.
Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
---
pbs-config/src/cached_user_info.rs | 1 -
1 file changed, 1 deletion(-)
diff --git a/pbs-config/src/cached_user_info.rs b/pbs-config/src/cached_user_info.rs
index 8dd2375a..b9534b80 100644
--- a/pbs-config/src/cached_user_info.rs
+++ b/pbs-config/src/cached_user_info.rs
@@ -170,7 +170,6 @@ impl CachedUserInfo {
if auth_id.is_token() {
// limit privs to that of owning user
let user_auth_id = Authid::from(auth_id.user().clone());
- privs &= self.lookup_privs(&user_auth_id, path);
let (owner_privs, owner_propagated_privs) =
self.lookup_privs_details(&user_auth_id, path);
privs &= owner_privs;
--
2.30.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup] fix: config: remove duplicate privilege lookup in cached_user_info
2022-06-10 8:13 [pbs-devel] [PATCH proxmox-backup] fix: config: remove duplicate privilege lookup in cached_user_info Stefan Sterz
@ 2022-06-10 8:52 ` Wolfgang Bumiller
2022-06-10 9:00 ` Stefan Sterz
2022-06-10 9:31 ` [pbs-devel] applied: " Wolfgang Bumiller
1 sibling, 1 reply; 4+ messages in thread
From: Wolfgang Bumiller @ 2022-06-10 8:52 UTC (permalink / raw)
To: Stefan Sterz; +Cc: pbs-devel
Any reason for the "fix: " prefix in the commit message, though? This
just seems to remove something redundant and not actually fix an issue?
Or am I missing something?
Code-wise it seems fine, so I'd apply it, but I'd drop the 'fix' prefix?
On Fri, Jun 10, 2022 at 10:13:25AM +0200, Stefan Sterz wrote:
> `lookup_privs` just uses `lookup_privs_details` but ignores the
> propagated privileges it returns. thus, the lookup here is redundant
> as it is immediately followed by a call to `lookup_privs_details` with
> the same parameters.
>
> Signed-off-by: Stefan Sterz <s.sterz@proxmox.com>
> ---
> pbs-config/src/cached_user_info.rs | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/pbs-config/src/cached_user_info.rs b/pbs-config/src/cached_user_info.rs
> index 8dd2375a..b9534b80 100644
> --- a/pbs-config/src/cached_user_info.rs
> +++ b/pbs-config/src/cached_user_info.rs
> @@ -170,7 +170,6 @@ impl CachedUserInfo {
> if auth_id.is_token() {
> // limit privs to that of owning user
> let user_auth_id = Authid::from(auth_id.user().clone());
> - privs &= self.lookup_privs(&user_auth_id, path);
> let (owner_privs, owner_propagated_privs) =
> self.lookup_privs_details(&user_auth_id, path);
> privs &= owner_privs;
> --
> 2.30.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup] fix: config: remove duplicate privilege lookup in cached_user_info
2022-06-10 8:52 ` Wolfgang Bumiller
@ 2022-06-10 9:00 ` Stefan Sterz
0 siblings, 0 replies; 4+ messages in thread
From: Stefan Sterz @ 2022-06-10 9:00 UTC (permalink / raw)
To: Wolfgang Bumiller; +Cc: pbs-devel
On 6/10/22 10:52, Wolfgang Bumiller wrote:
> Any reason for the "fix: " prefix in the commit message, though? This
> just seems to remove something redundant and not actually fix an issue?
> Or am I missing something?
>
> Code-wise it seems fine, so I'd apply it, but I'd drop the 'fix' prefix?
>
i can see your point in that this technically doesn't fix a bug. you
can remove it.
my reasoning was that it removes a performance loss (albeit a rather
minor one) and also potential confusion when reviewing this code in
the future. it tripped me up when i looked at it. at the very least it
makes the code here more concise.
-- snip --
^ permalink raw reply [flat|nested] 4+ messages in thread
* [pbs-devel] applied: [PATCH proxmox-backup] fix: config: remove duplicate privilege lookup in cached_user_info
2022-06-10 8:13 [pbs-devel] [PATCH proxmox-backup] fix: config: remove duplicate privilege lookup in cached_user_info Stefan Sterz
2022-06-10 8:52 ` Wolfgang Bumiller
@ 2022-06-10 9:31 ` Wolfgang Bumiller
1 sibling, 0 replies; 4+ messages in thread
From: Wolfgang Bumiller @ 2022-06-10 9:31 UTC (permalink / raw)
To: Stefan Sterz; +Cc: pbs-devel
applied w/o the 'fix:' prefix in the commit message
thanks
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-06-10 9:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-10 8:13 [pbs-devel] [PATCH proxmox-backup] fix: config: remove duplicate privilege lookup in cached_user_info Stefan Sterz
2022-06-10 8:52 ` Wolfgang Bumiller
2022-06-10 9:00 ` Stefan Sterz
2022-06-10 9:31 ` [pbs-devel] applied: " Wolfgang Bumiller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal