timja / jenkins-gh-issues-poc-06-18

0 stars 0 forks source link

[JENKINS-13190] Add ACLPermissionOverride Extension Point to grant additional permissions to an ACL regardless of the AuthorizationStrategy being used #2161

Open timja opened 12 years ago

timja commented 12 years ago

For the github-oauth-plugin people want to use the existing GlobalMatrixAuthorizationStrategy and enable things like the github-webhook callback. Currently I have my own AuthorizationStrategy that supports these extra callback URL's but I want to be able to transparently support them without caring which specific AuthorizationStrategy is being used.

My solution is to add a new extension point into Jenkins that is invoked at the base ACL class that checks if any ACLPermissionOverride extensions want to grant the permission before the ACL checks its own authorization logic.

For the github-oauth-plugin it means that I can add in these extra URL's allow options into my SecurityRealm and then get them applied before the GlobalMatrixAuthorizationStrategy's ACL logic is used.


Originally reported by mocleiri, imported from: Add ACLPermissionOverride Extension Point to grant additional permissions to an ACL regardless of the AuthorizationStrategy being used
  • status: Open
  • priority: Minor
  • resolution: Unresolved
  • imported: 2022/01/10
timja commented 12 years ago

mocleiri:

Added pull request:
https://github.com/jenkinsci/jenkins/pull/412

I have tested this using the github-oauth-plugin and it works.

This is my initial implementation using this extension:
https://github.com/mocleiri/github-oauth-plugin/commit/28d9114bb10fecf6fb1bb33114b45604d0b20487

timja commented 12 years ago

scm_issue_link:

Code changed in jenkins
User: Michael O'Cleirigh
Path:
core/src/main/java/hudson/security/ACL.java
core/src/main/java/hudson/security/ACLPermissionOverride.java
http://jenkins-ci.org/commit/jenkins/7d0380f46f013effb7793622707042a452ff1c66
Log:
JENKINS-13190 Add ACLPermissionOverride extension point

Modifies the base ACL class to first check with ACLPermissionOverride extension point
implementations as to whether the permission should be granted.

If none of the ACLPermissionOverride instances approve the permission the ACL's permission
checking logic will be used instead.

This override provides a mechanism to grant more permissions than the ACL being overridden provides.

timja commented 8 years ago

ikedam:

This feature is also required by authorize-project plugin (JENKINS-35081).

I plan to try an alternate implementation to resolve following problems:

timja commented 8 years ago

ikedam:

I believe there're two ways to introduce overriding ACLs.
As ACL itself doesn't hold what item it is for, overriding should be performed to AuthorizationStrategy.

A: Jenkins#getAuthorizationStrategy returns overridden AuthorizationStrategy. A new method Jenkins#getBaseAuthorizationStrategy is introduced to return the original AuthorizationStrategy.
B: Jenkins#getAuthorizationStrategy returns the original AuthorizationStrategy. A new method Jenkins#getOverriddenAuthorizationStrategy is introduced to return the overridden AuthorizationStrategy.

These two ways result following regressions:

A: Plugins using Jenkins#getAuthorizationStrategy not to resolve ACLs (e.g. to test its own strategy is configured as Jenkins#getAuthorizationStrategy() instanceof XxxxAuthorizationStraetgy) will be broken.
B: Plugins retrieving ACLs via Jenkins#getAuthorizationStrategy will be broken.

I searched github of jenkinsci with getAuthorizationStrategy and list up plugins affected by A and B as followings:
A:

B.

Though this is really rough investigation, I can conclude that A is a really bad idea as it affects too many plugins.

Plugins should use Jenkins.getACL, Item.getACL and so on instread of getAuthorizationStrategy().getACL(item).

timja commented 8 years ago

ikedam:

> Though this is really rough investigation, I can conclude that A is a really bad idea as it affects too many plugins.

I also note that A contains popular plugins like matrix-auth-plugin and role-strategy-plugin.

timja commented 8 years ago

ikedam:

Sent a pull request:
https://github.com/jenkinsci/jenkins/pull/2481

timja commented 7 years ago

jglick:

Related proposals in JENKINS-32596.

timja commented 2 years ago

[Originally related to: JENKINS-35081]