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

Basic Authentication in combination with Session is broken

    XMLWordPrintable

    Details

    • Similar Issues:

      Description

      BasicAuthentication in combination with a sessionId is broken - after the first login following page refreshs fail with bad credentials.

      Here my analysis (I commented this on the corresponding commit on github as well):
      The BasicHeaderProcessor expects a not null Authentication Object

      From BasicHeaderProcessor:

      Authentication auth = a.authenticate(req, rsp, username, password);
      if (auth!=null) {
      LOGGER.log(FINE, "Request authenticated as

      {0}

      by

      {1}

      ", new Object[]

      {auth,a}

      );
      success(req, rsp, chain, auth);
      return;
      }
      From BasicHeaderRealPasswordAuthenticator:

      if (!authenticationIsRequired(username))
      return null;
      It seems that you need to return the existing authentication Object from BasicHeaderRealPasswordAuthenticator and not null if the current authentication is already valid...?

      Anyway since we are running jenkins through a proxy with basicAuth the current version is completely broken for us...

      Corresponding Github commit: https://github.com/jenkinsci/jenkins/commit/b2a98f6bc6924d1fd25f7da583888c2f4f36d83c

        Attachments

          Issue Links

            Activity

            Hide
            danielbeck Daniel Beck added a comment -

            1.580.2 RC is due this week, so this won't be soaked enough. Inclusion in .3 is more likely.

            Show
            danielbeck Daniel Beck added a comment - 1.580.2 RC is due this week, so this won't be soaked enough. Inclusion in .3 is more likely.
            Hide
            oleg_nenashev Oleg Nenashev added a comment -

            It makes sense to discuss the exception on the project meeting

            Show
            oleg_nenashev Oleg Nenashev added a comment - It makes sense to discuss the exception on the project meeting
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Christof Schoell
            Path:
            core/src/main/java/jenkins/security/BasicHeaderRealPasswordAuthenticator.java
            test/src/test/java/jenkins/security/BasicHeaderProcessorTest.java
            http://jenkins-ci.org/commit/jenkins/7169a3916528ac95eada2cf13e3fbd7e50ae6387
            Log:
            JENKINS-25144

            return authentication object instead of null if authentication is not
            required - otherwise valid login fails with basic authentication

            (cherry picked from commit 0176b6d902faeec7bff63eb34ce16e2f70062035)

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Christof Schoell Path: core/src/main/java/jenkins/security/BasicHeaderRealPasswordAuthenticator.java test/src/test/java/jenkins/security/BasicHeaderProcessorTest.java http://jenkins-ci.org/commit/jenkins/7169a3916528ac95eada2cf13e3fbd7e50ae6387 Log: JENKINS-25144 return authentication object instead of null if authentication is not required - otherwise valid login fails with basic authentication (cherry picked from commit 0176b6d902faeec7bff63eb34ce16e2f70062035)
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Kohsuke Kawaguchi
            Path:
            core/src/main/java/jenkins/security/BasicHeaderProcessor.java
            core/src/main/java/jenkins/security/BasicHeaderRealPasswordAuthenticator.java
            http://jenkins-ci.org/commit/jenkins/be34b675f0f9cb59a67c09dbe42364d34c3eaff1
            Log:
            [FIXED JENKINS-25144] Revisiting the attempted fix in the previous commit.

            IIUC, the issue here is that the request in question contains both a
            valid session cookie AND basic authentication header, and that path
            results in a failure because BasicHeaderProcessor expects one of
            BasicHeaderAuthenticators to validate the basic authentication header
            (without knowing that there's already a valid Authentication object that
            came from the HTTP session, yet no BasicHeaderAuthenticator actually
            processes this because BasicHeaderRealPasswordAuthenticator backs away
            from doing that.

            I think the corect fix is for BasicHeaderRealPasswordAuthenticator to
            get rid of authenticationIsRequired check. This check instead belongs to
            BasicHeaderProcessor, where it should be used to check if any
            BasicHeaderAuthenticator should be consulted or not.

            The problem with having this logic in
            BasicHeaderRealPasswordAuthenticator is that this is just an
            implementation of an extension point, and thus it needs to be removable.
            As it stands right now in this fix, if this impl is removed,
            JENKINS-25144 will be back again.

            (cherry picked from commit 9e81b8e4feebceef94d117b757952c965bf91c61)

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Kohsuke Kawaguchi Path: core/src/main/java/jenkins/security/BasicHeaderProcessor.java core/src/main/java/jenkins/security/BasicHeaderRealPasswordAuthenticator.java http://jenkins-ci.org/commit/jenkins/be34b675f0f9cb59a67c09dbe42364d34c3eaff1 Log: [FIXED JENKINS-25144] Revisiting the attempted fix in the previous commit. IIUC, the issue here is that the request in question contains both a valid session cookie AND basic authentication header, and that path results in a failure because BasicHeaderProcessor expects one of BasicHeaderAuthenticators to validate the basic authentication header (without knowing that there's already a valid Authentication object that came from the HTTP session, yet no BasicHeaderAuthenticator actually processes this because BasicHeaderRealPasswordAuthenticator backs away from doing that. I think the corect fix is for BasicHeaderRealPasswordAuthenticator to get rid of authenticationIsRequired check. This check instead belongs to BasicHeaderProcessor, where it should be used to check if any BasicHeaderAuthenticator should be consulted or not. The problem with having this logic in BasicHeaderRealPasswordAuthenticator is that this is just an implementation of an extension point, and thus it needs to be removable. As it stands right now in this fix, if this impl is removed, JENKINS-25144 will be back again. (cherry picked from commit 9e81b8e4feebceef94d117b757952c965bf91c61)
            Hide
            dogfood dogfood added a comment -

            Integrated in jenkins_main_trunk #4292
            JENKINS-25144 (Revision 7169a3916528ac95eada2cf13e3fbd7e50ae6387)
            [FIXED JENKINS-25144] Revisiting the attempted fix in the previous commit. (Revision be34b675f0f9cb59a67c09dbe42364d34c3eaff1)

            Result = UNSTABLE
            ogondza : 7169a3916528ac95eada2cf13e3fbd7e50ae6387
            Files :

            • core/src/main/java/jenkins/security/BasicHeaderRealPasswordAuthenticator.java
            • test/src/test/java/jenkins/security/BasicHeaderProcessorTest.java

            ogondza : be34b675f0f9cb59a67c09dbe42364d34c3eaff1
            Files :

            • core/src/main/java/jenkins/security/BasicHeaderProcessor.java
            • core/src/main/java/jenkins/security/BasicHeaderRealPasswordAuthenticator.java
            Show
            dogfood dogfood added a comment - Integrated in jenkins_main_trunk #4292 JENKINS-25144 (Revision 7169a3916528ac95eada2cf13e3fbd7e50ae6387) [FIXED JENKINS-25144] Revisiting the attempted fix in the previous commit. (Revision be34b675f0f9cb59a67c09dbe42364d34c3eaff1) Result = UNSTABLE ogondza : 7169a3916528ac95eada2cf13e3fbd7e50ae6387 Files : core/src/main/java/jenkins/security/BasicHeaderRealPasswordAuthenticator.java test/src/test/java/jenkins/security/BasicHeaderProcessorTest.java ogondza : be34b675f0f9cb59a67c09dbe42364d34c3eaff1 Files : core/src/main/java/jenkins/security/BasicHeaderProcessor.java core/src/main/java/jenkins/security/BasicHeaderRealPasswordAuthenticator.java

              People

              • Assignee:
                oleg_nenashev Oleg Nenashev
                Reporter:
                cschoell Christof Schoell
              • Votes:
                8 Vote for this issue
                Watchers:
                14 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: