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

Improve AD/LDAP attribute analysis for locked accounts

      In Spring Security, a user account has three relevant flags related to the account's validity:

      • Disabled (used by admins to disable accounts rather than deleting them)
      • Locked (used by intrusion detection systems typically)
      • Expired (used to set a specific lifetime for an account; this can be handy for time-bound contracts and such)

      As it exists now, these attributes are not actually specified by any of Jenkins' security realms. In Spring Security, these attributes are typically only used by a JDBC-backed user database (or other first-party Spring Security user databases), but Jenkins does not expose that functionality. Spring Security's servlet filters and such all check these flags on the user details loaded for an authenticated user, and performing a normal password login will also respect flags provided the AD or LDAP server itself denies the login due to these states (which is how they normally work). These flags are not validated when authenticating with alternative methods than the user's password such as via an API token or SSH key. This may allow an account that hasn't logged in since having their validity state change may still be able to use their API token, SSH key, or any other automated actions that work on behalf of that user (e.g., authorize project running a pipeline as the user if they still have commit access).

      The goal of this issue to introduce additional security hardening to attempt to detect when user accounts have gone invalid without needing to re-authenticate them (which would have necessitated storing their password long term which is a no go). When user details are loaded from an Active Directory or LDAP security realm, the user's operational attributes should be checked according to some commonly used standards such as the draft LDAP password policy and Active Directory's attributes.

      Relevant docs:

          [JENKINS-55813] Improve AD/LDAP attribute analysis for locked accounts

          Same issue for me, 2.15 stops me logging in had to revert to 2.14.

          Neil Sleightholm added a comment - Same issue for me, 2.15 stops me logging in had to revert to 2.14.

          Matt Sicker added a comment -

          I believe this PR was merged prematurely in the AD plugin. I'll submit a revert PR and refile the original as a draft PR.

          Matt Sicker added a comment - I believe this PR was merged prematurely in the AD plugin. I'll submit a revert PR and refile the original as a draft PR.

          Matt Sicker added a comment -

          Adding link to updated AD PR as a draft.

          Matt Sicker added a comment - Adding link to updated AD PR as a draft.

          The work on this ticket is "on-hold" for the moment, to be resumed soon-ish.

          Wadeck Follonier added a comment - The work on this ticket is "on-hold" for the moment, to be resumed soon-ish.

          Matt Sicker added a comment -

          I started looking back into this issue and found some interesting info to share:

          1. Acegi has a UserDetailsChecker API which we can re-use rather than the UserDetailsHelper class. This is what's done by AbstractUserDetailsAuthenticationProvider which is used for LDAP and AD.
          2. The LDAP and AD PRs need some proper integration tests added considering they're somewhat complex scenarios.
          3. It would be great if we could make an automated test using a Windows Docker container for the AD scenario, though that might be too complex to set up.

          Basically, we can deliver this in three pieces since the Jenkins core updates will be to ensure user details are checked at all (and not just most) of the necessary call sites while the plugin updates will be to fill in real values other than true.

          Matt Sicker added a comment - I started looking back into this issue and found some interesting info to share: Acegi has a UserDetailsChecker API which we can re-use rather than the UserDetailsHelper class. This is what's done by AbstractUserDetailsAuthenticationProvider which is used for LDAP and AD. The LDAP and AD PRs need some proper integration tests added considering they're somewhat complex scenarios. It would be great if we could make an automated test using a Windows Docker container for the AD scenario, though that might be too complex to set up. Basically, we can deliver this in three pieces since the Jenkins core updates will be to ensure user details are checked at all (and not just most) of the necessary call sites while the plugin updates will be to fill in real values other than true.

          Matt Sicker added a comment -

          Turns out there were more scenarios to support in the original PR. The included Docker tests don't seem to work properly on macOS, so I ended up testing out my changes here using ApacheDS and OpenLDAP locally with the Planet Express data set (along with changing various account attributes to test out account disabled scenarios). All seems to work, though the caching in place would be best served by listening for LDAP events when supported, but that seems like a separate feature.

          Matt Sicker added a comment - Turns out there were more scenarios to support in the original PR. The included Docker tests don't seem to work properly on macOS, so I ended up testing out my changes here using ApacheDS and OpenLDAP locally with the Planet Express data set (along with changing various account attributes to test out account disabled scenarios). All seems to work, though the caching in place would be best served by listening for LDAP events when supported, but that seems like a separate feature.

          Matt Sicker added a comment -

          I've filed some PRs to get this moving:

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

          https://github.com/jenkinsci/ldap-plugin/pull/50

          I haven't had a chance to test out the equivalent patch for active-directory-plugin yet.

          Matt Sicker added a comment - I've filed some PRs to get this moving: https://github.com/jenkinsci/jenkins/pull/4925 https://github.com/jenkinsci/ldap-plugin/pull/50 I haven't had a chance to test out the equivalent patch for active-directory-plugin yet.

          Matt Sicker added a comment -

          Matt Sicker added a comment - Add https://github.com/jenkinsci/sshd-module/pull/39 and https://github.com/jenkins-infra/jenkins.io/pull/3953 as more PRs in action. I need to manually test it, but I've also got https://github.com/jenkinsci/active-directory-plugin/pull/96 for the AD variant.

          Matt Sicker added a comment -

          Matt Sicker added a comment - I've finished manual testing of both plugins with Active Directory. The following list of pull requests completes this feature: https://github.com/jenkinsci/jenkins/pull/4925 https://github.com/jenkinsci/sshd-module/pull/39 https://github.com/jenkins-infra/jenkins.io/pull/3953 https://github.com/jenkinsci/ldap-plugin/pull/59 https://github.com/jenkinsci/active-directory-plugin/pull/96

          These changes quite probably cause some issues in other plugins, pls see JENKINS-64850

          Istvan Szekeres added a comment - These changes quite probably cause some issues in other plugins, pls see JENKINS-64850

            jvz Matt Sicker
            wfollonier Wadeck Follonier
            Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

              Created:
              Updated:
              Resolved: