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

Basic Authentication in combination with Session is broken

      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

          [JENKINS-25144] Basic Authentication in combination with Session is broken

          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/0176b6d902faeec7bff63eb34ce16e2f70062035
          Log:
          JENKINS-25144

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

          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/0176b6d902faeec7bff63eb34ce16e2f70062035 Log: JENKINS-25144 return authentication object instead of null if authentication is not required - otherwise valid login fails with basic authentication

          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/9e81b8e4feebceef94d117b757952c965bf91c61
          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.

          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/9e81b8e4feebceef94d117b757952c965bf91c61 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.

          Code changed in jenkins
          User: Kohsuke Kawaguchi
          Path:
          changelog.html
          core/src/main/java/jenkins/security/BasicHeaderProcessor.java
          core/src/main/java/jenkins/security/BasicHeaderRealPasswordAuthenticator.java
          test/src/test/java/jenkins/security/BasicHeaderProcessorTest.java
          http://jenkins-ci.org/commit/jenkins/ada8432fc57b0293cc7c28777e64f8a72d413409
          Log:
          JENKINS-25144 Merge pull request #1427

          Compare: https://github.com/jenkinsci/jenkins/compare/24c5e5e44038...ada8432fc57b

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Kohsuke Kawaguchi Path: changelog.html core/src/main/java/jenkins/security/BasicHeaderProcessor.java core/src/main/java/jenkins/security/BasicHeaderRealPasswordAuthenticator.java test/src/test/java/jenkins/security/BasicHeaderProcessorTest.java http://jenkins-ci.org/commit/jenkins/ada8432fc57b0293cc7c28777e64f8a72d413409 Log: JENKINS-25144 Merge pull request #1427 Compare: https://github.com/jenkinsci/jenkins/compare/24c5e5e44038...ada8432fc57b

          dogfood added a comment -

          Integrated in jenkins_main_trunk #3800
          JENKINS-25144 (Revision 0176b6d902faeec7bff63eb34ce16e2f70062035)
          [FIXED JENKINS-25144] Revisiting the attempted fix in the previous commit. (Revision 9e81b8e4feebceef94d117b757952c965bf91c61)

          Result = SUCCESS
          kohsuke : 0176b6d902faeec7bff63eb34ce16e2f70062035
          Files :

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

          kohsuke : 9e81b8e4feebceef94d117b757952c965bf91c61
          Files :

          • core/src/main/java/jenkins/security/BasicHeaderRealPasswordAuthenticator.java
          • core/src/main/java/jenkins/security/BasicHeaderProcessor.java

          dogfood added a comment - Integrated in jenkins_main_trunk #3800 JENKINS-25144 (Revision 0176b6d902faeec7bff63eb34ce16e2f70062035) [FIXED JENKINS-25144] Revisiting the attempted fix in the previous commit. (Revision 9e81b8e4feebceef94d117b757952c965bf91c61) Result = SUCCESS kohsuke : 0176b6d902faeec7bff63eb34ce16e2f70062035 Files : test/src/test/java/jenkins/security/BasicHeaderProcessorTest.java core/src/main/java/jenkins/security/BasicHeaderRealPasswordAuthenticator.java kohsuke : 9e81b8e4feebceef94d117b757952c965bf91c61 Files : core/src/main/java/jenkins/security/BasicHeaderRealPasswordAuthenticator.java core/src/main/java/jenkins/security/BasicHeaderProcessor.java

          Florian Hug added a comment -

          Thanks a lot for fixing this issue! Could this fix please be included in the LTS version 1.580.2? The current LTS is completely useless for us... Thanks a lot!

          Florian Hug added a comment - Thanks a lot for fixing this issue! Could this fix please be included in the LTS version 1.580.2? The current LTS is completely useless for us... Thanks a lot!

          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.

          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.

          Oleg Nenashev added a comment -

          It makes sense to discuss the exception on the project meeting

          Oleg Nenashev added a comment - It makes sense to discuss the exception on the project meeting

          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)

          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)

          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)

          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)

          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

          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

            oleg_nenashev Oleg Nenashev
            cschoell Christof Schoell
            Votes:
            8 Vote for this issue
            Watchers:
            13 Start watching this issue

              Created:
              Updated:
              Resolved: