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

Access to check for unprotected/never secured paths

    XMLWordPrintable

    Details

    • Type: Improvement
    • Status: Resolved (View Workflow)
    • Priority: Minor
    • Resolution: Fixed
    • Component/s: core
    • Labels:
      None
    • Similar Issues:

      Description

      Currently no method exists (that I could find) to allow authentication plugins working on top of Jenkins to test a path to see if it does not require authentication; Jenkins does it (in part, at least) using Jenkins.getTarget() (https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/jenkins/model/Jenkins.java#L3875). This function does too much for some plugins, e.g. NegotiateSSO, because it first checks for read permissions.

      I propose breaking the functionality of Jenkins.getTarget() into several functions, allowing a plugin to access the checks for paths that shouldn't be protected. The attached diff file gives an example (diff base is commit e014700, on jenkinsci/jenkins).

      The other solutions that have been given to me so far involve maintaining a list of exceptions, which seems unwise to me.

        Attachments

          Activity

          Hide
          olivergondza Oliver Gondža added a comment - - edited

          It makes sense to provide a way for plugins to tell of path should not be protected or not. Commenting on the state of the path as attached (please move this to PR), please consolidate it to one method that accept restOfPath as an argument and performs all of the checks. Provided you do not need the two for some reason.

          Show
          olivergondza Oliver Gondža added a comment - - edited It makes sense to provide a way for plugins to tell of path should not be protected or not. Commenting on the state of the path as attached (please move this to PR), please consolidate it to one method that accept restOfPath as an argument and performs all of the checks. Provided you do not need the two for some reason.
          Hide
          farmgeek4life Bryson Gibbons added a comment -

          Will do as one function; after looking at my duplicated code, I was unable to use "Stapler.getCurrentRequest().getRestOfPath()" because at the point I am checking permission requirements the request is still in the filters, and stapler doesn't know about it yet.

          Show
          farmgeek4life Bryson Gibbons added a comment - Will do as one function; after looking at my duplicated code, I was unable to use "Stapler.getCurrentRequest().getRestOfPath()" because at the point I am checking permission requirements the request is still in the filters, and stapler doesn't know about it yet.
          Hide
          farmgeek4life Bryson Gibbons added a comment -
          Show
          farmgeek4life Bryson Gibbons added a comment - Pull Request: https://github.com/jenkinsci/jenkins/pull/2652
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Bryson Gibbons
          Path:
          core/src/main/java/jenkins/model/Jenkins.java
          http://jenkins-ci.org/commit/jenkins/0060335b8cf6d36641bd610817bae98873c32746
          Log:
          JENKINS-32797 Break the catch clause contents of Jenkins.getTarget(… (#2652)

          • JENKINS-32797 Break the catch clause contents of Jenkins.getTarget() out into a separate, publicly accessible function.

          This will allow plugins (particularly authentication plugins that override the normal authentication process) to determine if authentication is not required for a particular path by calling isPathUnprotected(restOfPath).

          • Add @since TODO to comment
          • Change name of function to something that is accurate and clear

          isPathUnprotected is misleading, and the Javadoc was worse. isSubjectToMandatoryReadPermissionCheck is a much better name, and the return value is reversed to match the name,

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Bryson Gibbons Path: core/src/main/java/jenkins/model/Jenkins.java http://jenkins-ci.org/commit/jenkins/0060335b8cf6d36641bd610817bae98873c32746 Log: JENKINS-32797 Break the catch clause contents of Jenkins.getTarget(… (#2652) JENKINS-32797 Break the catch clause contents of Jenkins.getTarget() out into a separate, publicly accessible function. This will allow plugins (particularly authentication plugins that override the normal authentication process) to determine if authentication is not required for a particular path by calling isPathUnprotected(restOfPath). Add @since TODO to comment Change name of function to something that is accurate and clear isPathUnprotected is misleading, and the Javadoc was worse. isSubjectToMandatoryReadPermissionCheck is a much better name, and the return value is reversed to match the name,
          Hide
          oleg_nenashev Oleg Nenashev added a comment -

          Released in jenkins-2.37

          Show
          oleg_nenashev Oleg Nenashev added a comment - Released in jenkins-2.37

            People

            • Assignee:
              Unassigned
              Reporter:
              farmgeek4life Bryson Gibbons
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: