applied with followup patch (see below) On Thu, Sep 21, 2017 at 11:09:14AM +0200, Philip Abernethy wrote: > Die with a helpful error message instead of silently ignoring the user > when trying to delete a special role. > Also add a property to the API answer for possible later use by the > WebUI. > --- > PVE/API2/Role.pm | 6 +++++- > PVE/AccessControl.pm | 5 +++++ > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/PVE/API2/Role.pm b/PVE/API2/Role.pm > index 6392e13..0216c8d 100644 > --- a/PVE/API2/Role.pm > +++ b/PVE/API2/Role.pm > @@ -44,7 +44,8 @@ __PACKAGE__->register_method ({ > > foreach my $role (keys %{$usercfg->{roles}}) { > my $privs = join(',', sort keys %{$usercfg->{roles}->{$role}}); > - push @$res, { roleid => $role, privs => $privs }; > + push @$res, { roleid => $role, privs => $privs, > + special => PVE::AccessControl::role_is_special($role) }; > } > > return $res; > @@ -195,6 +196,9 @@ __PACKAGE__->register_method ({ > die "role '$role' does not exist\n" > if !$usercfg->{roles}->{$role}; > > + die "auto-generated role '$role' can not be deleted\n" > + if PVE::AccessControl::role_is_special($role);
This block here holds a file lock, but the check only needs to know the role parameter and does not depend on anything else, so I moved it up to before acquiring the lock. > + > delete ($usercfg->{roles}->{$role}); > > # fixme: delete role from acl? > diff --git a/PVE/AccessControl.pm b/PVE/AccessControl.pm > index 7d02cdf..98e2fd6 100644 > --- a/PVE/AccessControl.pm > +++ b/PVE/AccessControl.pm > @@ -502,6 +502,11 @@ sub create_roles { > > create_roles(); > > +sub role_is_special { > + my ($role) = @_; > + return exists $special_roles->{$role}; > +} > + > sub add_role_privs { > my ($role, $usercfg, $privs) = @_; > > -- > 2.11.0 _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel