Not inheriting Overall/* permissions
70 views
Skip to first unread message
Daniel Beck
Apr 24, 2019, 7:17:17 PM4/24/19
to Jenkins Developers
Hi everyone,
When I investigated the cause for a bug involving Blue Ocean and Matrix Auth plugins today (JENKINS-46540), I noticed that it seems undefined whether "Overall" permissions would be granted on non root objects.
This matters because Matrix Auth plugin allows to not inherit permissions from higher level ACLs, so that something like "Access to all jobs except X" can more easily be implemented through the few jobs that are exceptions, rather than granting the permission individually on all other jobs.
As a side effect, checking for Overall/Read on non root level, for example l:task's `permission` attribute, will now fail (which AFAICT is ultimately the cause of JENKINS-46540). Obviously, checking Overall/Read is generally not useful outside of UnprotectedRootActions with nontrivial behavior, as users are expected to have this permission to even see the UI that would check that permission.
Perhaps a better example: I expect the same problem applies to an agent level permission check for Overall/RunScripts to display the agent script console link in Computer/sidepanel.jelly. Of course, we don't allow these 'dangerous' permissions to be granted independent of Administer without jumping through hoops for a few years now, so it's not a great example either.
To not prevent accidental locking out of administrators, Overall/Administer is an exception to permissions not being inherited in Matrix Auth, so cannot serve as an example here.
What is the correct behavior for authorization realms here? Should Overall/* permissions be inherited by all ACLs?
When I investigated the cause for a bug involving Blue Ocean and Matrix Auth plugins today (JENKINS-46540), I noticed that it seems undefined whether "Overall" permissions would be granted on non root objects.
This matters because Matrix Auth plugin allows to not inherit permissions from higher level ACLs, so that something like "Access to all jobs except X" can more easily be implemented through the few jobs that are exceptions, rather than granting the permission individually on all other jobs.
As a side effect, checking for Overall/Read on non root level, for example l:task's `permission` attribute, will now fail (which AFAICT is ultimately the cause of JENKINS-46540). Obviously, checking Overall/Read is generally not useful outside of UnprotectedRootActions with nontrivial behavior, as users are expected to have this permission to even see the UI that would check that permission.
Perhaps a better example: I expect the same problem applies to an agent level permission check for Overall/RunScripts to display the agent script console link in Computer/sidepanel.jelly. Of course, we don't allow these 'dangerous' permissions to be granted independent of Administer without jumping through hoops for a few years now, so it's not a great example either.
To not prevent accidental locking out of administrators, Overall/Administer is an exception to permissions not being inherited in Matrix Auth, so cannot serve as an example here.
What is the correct behavior for authorization realms here? Should Overall/* permissions be inherited by all ACLs?
Jesse Glick
Apr 25, 2019, 8:38:05 AM4/25/19
to Jenkins Dev
On Wed, Apr 24, 2019 at 7:17 PM Daniel Beck <m...@beckweb.net> wrote:
> What is the correct behavior for authorization realms here? Should Overall/* permissions be inherited by all ACLs?
It should not really matter since any code even calling
> What is the correct behavior for authorization realms here? Should Overall/* permissions be inherited by all ACLs?
`hasPermission` / `checkPermission` against a permission using
`PermissionScope.JENKINS` on something other than `Jenkins.instance`
is a bug. Ideally we could enforce `PermissionScope`s at runtime to
catch mistakes like that. (There are some weird cases, some of which I
have tried to fix in the past, if you look up my PRs: `RUN` and `ITEM`
sometimes get confused, and `COMPUTER` is confusingly applied either
to `Computer` or `Node`.)
Reply all
Reply to author
Forward
0 new messages