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

          Added a pull request with a fix for this bug on github:

          https://github.com/jenkinsci/jenkins/pull/1427

          Christof Schoell added a comment - Added a pull request with a fix for this bug on github: https://github.com/jenkinsci/jenkins/pull/1427

          Christof Schoell added a comment - - edited

          FYI: I rolled out the builded war file with the fix (PR-1427) for jenkins on our test infrastructure and the authentication problems are gone.

          Christof Schoell added a comment - - edited FYI: I rolled out the builded war file with the fix (PR-1427) for jenkins on our test infrastructure and the authentication problems are gone.

          Stopped progress: Waiting for Pull Request to be merged (Dunno wether this fits into the usual workflow...?)

          Christof Schoell added a comment - Stopped progress: Waiting for Pull Request to be merged (Dunno wether this fits into the usual workflow...?)

          uncletall added a comment -

          uncletall added a comment - Related to: https://issues.jenkins-ci.org/browse/JENKINS-25180

          Andreas Mandel added a comment - - edited

          Might be obvious, this affects and blocks also the LTS Jenkins ver. 1.580.1-SNAPSHOT (rc-10/19/2014 06:20 GMT-kohsuke).

          I applied https://github.com/jenkinsci/jenkins/pull/1427 to the LTS RC which fixed the described issue for me.

          Andreas Mandel added a comment - - edited Might be obvious, this affects and blocks also the LTS Jenkins ver. 1.580.1-SNAPSHOT (rc-10/19/2014 06:20 GMT-kohsuke). I applied https://github.com/jenkinsci/jenkins/pull/1427 to the LTS RC which fixed the described issue for me.

          So do I get this right? This change has been reported in 1.584, pulled to LTS, we are in 1.587-SNAPSHOT and it still hasn't been merged to master or fixed otherwise??

          I don't mean to be impatient but...

          Christof Schoell added a comment - So do I get this right? This change has been reported in 1.584, pulled to LTS, we are in 1.587-SNAPSHOT and it still hasn't been merged to master or fixed otherwise?? I don't mean to be impatient but...

          Daniel Beck added a comment -

          This week is JUC SF (see banner in header) so most of the people who could do something about this were/are busy preparing talks and travelling.

          Daniel Beck added a comment - This week is JUC SF (see banner in header) so most of the people who could do something about this were/are busy preparing talks and travelling.

          Ok thanks, that's understandable

          Christof Schoell added a comment - Ok thanks, that's understandable

          @Christof, no sorry, my comment was misleading. I did this locally an my machine and I just wanted to emphasis that your patch works and also can / should / must be applied to the LTS. (+ this is really a blocker for all reverse proxy setups)

          Andreas Mandel added a comment - @Christof, no sorry, my comment was misleading. I did this locally an my machine and I just wanted to emphasis that your patch works and also can / should / must be applied to the LTS. (+ this is really a blocker for all reverse proxy setups)

          @andreasmandel oh ok - thanks for correcting that.

          Christof Schoell added a comment - @andreasmandel oh ok - thanks for correcting that.

          I can not take care for this. I'm not a core committer.

          Andreas Mandel added a comment - I can not take care for this. I'm not a core committer.

          If you look at the history, it seems that you assigned yourself
          Or there are other strange things happening with this bug lol

          Christof Schoell added a comment - If you look at the history, it seems that you assigned yourself Or there are other strange things happening with this bug lol

          Please have a look at this - would be great if this bug would be fixed any time soon, so anyone with a proxy solution can use a recent version of jenkins. Thanks!

          Christof Schoell added a comment - Please have a look at this - would be great if this bug would be fixed any time soon, so anyone with a proxy solution can use a recent version of jenkins. Thanks!

          uncletall added a comment -

          Failing to fix this, there should be at least a WARNING on the main page telling people not to install the latest version if the are running behind a reverse proxy.

          uncletall added a comment - Failing to fix this, there should be at least a WARNING on the main page telling people not to install the latest version if the are running behind a reverse proxy.

          Daniel Beck added a comment -

          Please have a look at this

          I'm sure he'll get right to it after taking care of the other 385 issues assigned to him. (IOW, it's usually more effective to bring issues to someone's attention via other channels.)

          WARNING on the main page telling people not to install the latest version

          You can vote on the changelog ("Community rating") and tell people about this bug.


          TBH I think you're overestimating the impact of this issue. Isn't it easily circumvented by sending User/Password with every request instead of relying on Session ID?

          Daniel Beck added a comment - Please have a look at this I'm sure he'll get right to it after taking care of the other 385 issues assigned to him. (IOW, it's usually more effective to bring issues to someone's attention via other channels.) WARNING on the main page telling people not to install the latest version You can vote on the changelog ("Community rating") and tell people about this bug. TBH I think you're overestimating the impact of this issue. Isn't it easily circumvented by sending User/Password with every request instead of relying on Session ID?

          A Session is created automatically and the User/Password is sent with every request through the proxy.
          That's when the problem occurs. Without any Session Id (which you would have to filter out inside the proxy) the functionality of jenkins is reduced as well...

          Christof Schoell added a comment - A Session is created automatically and the User/Password is sent with every request through the proxy. That's when the problem occurs. Without any Session Id (which you would have to filter out inside the proxy) the functionality of jenkins is reduced as well...

          uncletall added a comment -

          This is my server config. Is there anything I can change to workaround this bug? I am not doing anything with a Session ID, at least not that I am aware of.

          <Location "/">
          AuthBasicProvider ldap
          AuthType Basic
          AuthName "example.com"
          AuthzLDAPAuthoritative off
          AuthLDAPURL "ldap://example.com/ou=users,dc=example,dc=com?uid?one"
          Require valid-user
          RequestHeader set REMOTE_USER %

          {REMOTE_USER}

          s
          </Location>

          AllowEncodedSlashes On
          RewriteEngine On

          <Proxy *>
          Order deny,allow
          Allow from all
          </Proxy>

          1. Jenkins
            <Location /ci>
            ProxyPass http://192.168.1.2:8080/ci/
            ProxyPassReverse /
            Order deny,allow
            Allow from all
            </Location>
            Header edit Location ^http://example.com/ci/ https://example.com/ci/
          1. Gerrit
            ProxyPass /r/ http://192.168.1.1:8081/r/ nocanon
          2. Wiki
            ProxyPass /wiki/ https://192.168.1.3/wiki/
            ProxyPassReverse /wiki/ https://192.168.1.3/wiki/
            ProxyPass /w/ https://192.168.1.3/w/
            ProxyPassReverse /w/ https://192.168.1.3/w/

          uncletall added a comment - This is my server config. Is there anything I can change to workaround this bug? I am not doing anything with a Session ID, at least not that I am aware of. <Location "/"> AuthBasicProvider ldap AuthType Basic AuthName "example.com" AuthzLDAPAuthoritative off AuthLDAPURL "ldap://example.com/ou=users,dc=example,dc=com?uid?one" Require valid-user RequestHeader set REMOTE_USER % {REMOTE_USER} s </Location> AllowEncodedSlashes On RewriteEngine On <Proxy *> Order deny,allow Allow from all </Proxy> Jenkins <Location /ci> ProxyPass http://192.168.1.2:8080/ci/ ProxyPassReverse / Order deny,allow Allow from all </Location> Header edit Location ^ http://example.com/ci/ https://example.com/ci/ Gerrit ProxyPass /r/ http://192.168.1.1:8081/r/ nocanon Wiki ProxyPass /wiki/ https://192.168.1.3/wiki/ ProxyPassReverse /wiki/ https://192.168.1.3/wiki/ ProxyPass /w/ https://192.168.1.3/w/ ProxyPassReverse /w/ https://192.168.1.3/w/

          Christof Schoell added a comment - - edited

          You would have to remove the JSESSIONID set by Jenkins (or tomcat really) from the cookie header so you do not have a session any more.
          This isn't really practicable but I guess it could be possible...

          On the other hand this would remove any functionality from Jenkins which requires a session (e.g. icon sizes in the summary view)

          Christof Schoell added a comment - - edited You would have to remove the JSESSIONID set by Jenkins (or tomcat really) from the cookie header so you do not have a session any more. This isn't really practicable but I guess it could be possible... On the other hand this would remove any functionality from Jenkins which requires a session (e.g. icon sizes in the summary view)

          Daniel Beck added a comment -

          Right, proxy-based auth. I thought this was basic auth as used for API access. Sorry about the confusion.

          Note that it's well possible to use Jenkins based auth behind a free-for-all reverse proxy, so

          this is really a blocker for all reverse proxy setups

          is a bit too much.

          Daniel Beck added a comment - Right, proxy-based auth. I thought this was basic auth as used for API access. Sorry about the confusion. Note that it's well possible to use Jenkins based auth behind a free-for-all reverse proxy, so this is really a blocker for all reverse proxy setups is a bit too much.

          uncletall added a comment -

          Yes, you are right. Maybe this is not as common as I thought it was. Just that I accidentally upgraded Jenkins while upgrading Ubuntu using apt-get upgrade and ended up spending two days fixing my mistake. I would like other people to be spared the trouble.

          uncletall added a comment - Yes, you are right. Maybe this is not as common as I thought it was. Just that I accidentally upgraded Jenkins while upgrading Ubuntu using apt-get upgrade and ended up spending two days fixing my mistake. I would like other people to be spared the trouble.

          Is there an alternative way? I need a 2nd component to perform authentication (against an LDAP server) and single sign on for other tools accessible on this server. All is nicely done by using a Apache with mod proxy and mod_authnz_ldap. I guess this is quite common, at least for larger setups?

          Andreas Mandel added a comment - Is there an alternative way? I need a 2nd component to perform authentication (against an LDAP server) and single sign on for other tools accessible on this server. All is nicely done by using a Apache with mod proxy and mod_authnz_ldap. I guess this is quite common, at least for larger setups?

          uncletall added a comment -

          There were 89K+ downloads for the LDAP plugin in Sep-2014. I guess indeed that this should be quite common.

          uncletall added a comment - There were 89K+ downloads for the LDAP plugin in Sep-2014. I guess indeed that this should be quite common.

          Daniel Beck added a comment -

          No single sign-on though. Everything needs to authenticate.

          Daniel Beck added a comment - No single sign-on though. Everything needs to authenticate.

          uncletall added a comment -

          Actually, the Apache config I posted allows for something like a single sign-on. Apache authenticates the user and passes the credentials on to the other servers. This way I only need to log in once and have automatic access to Jenkins, Gerrit, MediaWiki and MantisBT. This way I do not need to log in for every single site and the users are manage centrally using LDAP. Very convenient.

          uncletall added a comment - Actually, the Apache config I posted allows for something like a single sign-on. Apache authenticates the user and passes the credentials on to the other servers. This way I only need to log in once and have automatic access to Jenkins, Gerrit, MediaWiki and MantisBT. This way I do not need to log in for every single site and the users are manage centrally using LDAP. Very convenient.

          Joachim Knoke added a comment -

          I ran into this and the fix of Christof Schoell works like a charm. Please merge it, if there are no side effects that may have been overseen, so we don´t need to fix this manually all the time. Thanks in advance.

          Joachim Knoke added a comment - I ran into this and the fix of Christof Schoell works like a charm. Please merge it, if there are no side effects that may have been overseen, so we don´t need to fix this manually all the time. Thanks in advance.

          Yep definitely convenient - we have a proxy application which adds a menu for all our tools on top aside from doing the registration and login into jenkins .... (SSO)

          This bug makes it impossible to use though...

          Christof Schoell added a comment - Yep definitely convenient - we have a proxy application which adds a menu for all our tools on top aside from doing the registration and login into jenkins .... (SSO) This bug makes it impossible to use though...

          So is anyone going to pull the fix - it only needs to be merged after all?

          This is really annoying to us, since we would like to update our jenkins to a recent version and can't ...

          Christof Schoell added a comment - So is anyone going to pull the fix - it only needs to be merged after all? This is really annoying to us, since we would like to update our jenkins to a recent version and can't ...

          Florian Hug added a comment -

          For us this is a blocker since we are using the RTC Jazz (Ration Team Concert) plugin to trigger builds directly via the version control system. Since we have to use authentication (company policy) the version control system authenticates against Jenkins (using basic auth) and starts the build using its individual parameters.

          Since basic auth is broken, no build can be triggered via the version control system and this LTS version is completely useless for us.
          We would really appreciate a quick fix within Jenkins LTS.

          Florian Hug added a comment - For us this is a blocker since we are using the RTC Jazz (Ration Team Concert) plugin to trigger builds directly via the version control system. Since we have to use authentication (company policy) the version control system authenticates against Jenkins (using basic auth) and starts the build using its individual parameters. Since basic auth is broken, no build can be triggered via the version control system and this LTS version is completely useless for us. We would really appreciate a quick fix within Jenkins LTS.

          So do we have to create a company internal fork of jenkins now, because nobody fixes this bug? LTS is broken as well after all...
          I really would prefer not to though.

          Christof Schoell added a comment - So do we have to create a company internal fork of jenkins now, because nobody fixes this bug? LTS is broken as well after all... I really would prefer not to though.

          Oleg Nenashev added a comment -

          AFAIK we're waiting for Kohsuke's review.
          I'll ping him in IRC

          Oleg Nenashev added a comment - AFAIK we're waiting for Kohsuke's review. I'll ping him in IRC

          Ok thanks!

          Christof Schoell added a comment - Ok thanks!

          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: