Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-20474

getACL methods are too expensive when current ACL is SYSTEM

      There are many occasions when a long block of code is running as ACL.SYSTEM (generally anything that is not handling a HTTP or CLI request), yet permission checks (done as part of e.g. Jenkins.getAllItems) call many getACL methods on model objects, which in turn ask the AuthorizationStrategy to make a new ACL instance, which can be rather expensive in some cases, and then ask that implementation about SYSTEM, which may actually be a shortcut in the strategy but by that point a lot of work has already been doneā€”all wasted, since SYSTEM must have full permissions regardless of strategy.

      It would be better for core should ensure that Jenkins.getACL and other getACL methods calling Jenkins.getInstance().getAuthorizationStrategy().getACL(this) (AbstractItem, Computer, Job, Node, User, Cloud, View) return a proxy ACL whose hasPermission checks for SYSTEM immediately (returning true in this case), only consulting the AuthorizationStrategy for another Authentication. (The proxy ACL could even be a cached part of the model object, avoiding all object construction in this case.)

          [JENKINS-20474] getACL methods are too expensive when current ACL is SYSTEM

          Jesse Glick created issue -

          Jesse Glick added a comment -

          Would solve issues like JENKINS-19623 without the need for plugin changes.

          Jesse Glick added a comment - Would solve issues like JENKINS-19623 without the need for plugin changes.
          Jesse Glick made changes -
          Link New: This issue is blocking JENKINS-19623 [ JENKINS-19623 ]
          Oleg Nenashev made changes -
          Link New: This issue is related to JENKINS-20475 [ JENKINS-20475 ]

          Oleg Nenashev added a comment -

          Could be there any use-case, when the ACL.SYSTEM is explicitly used in security settings?

          I've used such hasPermissions() to protect several Groovy scripts from improper usage from a System, but it is a dirty hack in any case.

          Oleg Nenashev added a comment - Could be there any use-case, when the ACL.SYSTEM is explicitly used in security settings? I've used such hasPermissions() to protect several Groovy scripts from improper usage from a System, but it is a dirty hack in any case.

          Jesse Glick added a comment -

          I think there is no valid use case for ACL.hasPermission(SYSTEM, Permission) to return false.

          Jesse Glick added a comment - I think there is no valid use case for ACL.hasPermission(SYSTEM, Permission) to return false.
          Jesse Glick made changes -
          Link New: This issue is related to JENKINS-22769 [ JENKINS-22769 ]

          Oleg Nenashev added a comment -

          jglick, AFAIK the patch has been applied to the core.
          Could you confirm it?

          Oleg Nenashev added a comment - jglick , AFAIK the patch has been applied to the core. Could you confirm it?

          Jesse Glick added a comment -

          I know of no change to core in this area.

          Jesse Glick added a comment - I know of no change to core in this area.
          Jesse Glick made changes -
          Link New: This issue is related to JENKINS-17122 [ JENKINS-17122 ]

            jglick Jesse Glick
            jglick Jesse Glick
            Votes:
            1 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated:
              Resolved: