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

Improve AD/LDAP attribute analysis for locked accounts

    Details

    • Similar Issues:

      Description

      In the current situation, there is no check about the accounts that are disabled, locked or expired, or having their credentials expired in active-directory.

      This ticket has the goal to improve the situation by reading as much as possible from the attributes returned by the server.

      Relevant docs:

        Attachments

          Activity

          wfollonier Wadeck Follonier created issue -
          spinus1 Alessio Moscatello made changes -
          Field Original Value New Value
          Assignee Wadeck Follonier [ wfollonier ] Alessio Moscatello [ spinus1 ]
          wfollonier Wadeck Follonier made changes -
          Remote Link This issue links to "#89 in active-directory (Web Link)" [ 22316 ]
          wfollonier Wadeck Follonier made changes -
          Remote Link This issue links to "#34 in ldap (Web Link)" [ 22317 ]
          wfollonier Wadeck Follonier made changes -
          Remote Link This issue links to "#3866 in core (Web Link)" [ 22318 ]
          Hide
          wfollonier Wadeck Follonier added a comment -

          The PRs in ldap and active-directory uses the Microsoft's standard for the attribute names/values. I am not sure that's sufficient to cover most of the usage.

          Show
          wfollonier Wadeck Follonier added a comment - The PRs in ldap and active-directory uses the Microsoft's standard for the attribute names/values. I am not sure that's sufficient to cover most of the usage.
          spinus1 Alessio Moscatello made changes -
          Assignee Alessio Moscatello [ spinus1 ] Wadeck Follonier [ wfollonier ]
          danielbeck Daniel Beck made changes -
          Link This issue is duplicated by SECURITY-900 [ SECURITY-900 ]
          Hide
          jvz Matt Sicker added a comment -

          Wadeck Follonier what do you mean by cover most of the usage? The usage within Jenkins plugins that may wish to impersonate a user? Or other LDAP servers? I've been starting to investigate this and have gotten somewhat confused around the current goal.

          Show
          jvz Matt Sicker added a comment - Wadeck Follonier what do you mean by cover most of the usage? The usage within Jenkins plugins that may wish to impersonate a user? Or other LDAP servers? I've been starting to investigate this and have gotten somewhat confused around the current goal.
          Hide
          wfollonier Wadeck Follonier added a comment -

          Matt Sicker In the core, I covered only the cast of the API Token, but didn't investigate further, it was just a PoC at that time. We need to ensure that every use of the Security realm check methods are consistent, i.e. checking the attribute of the UserDetails before using them.

          Show
          wfollonier Wadeck Follonier added a comment - Matt Sicker In the core, I covered only the cast of the API Token, but didn't investigate further, it was just a PoC at that time. We need to ensure that every use of the Security realm check methods are consistent, i.e. checking the attribute of the UserDetails before using them.
          fbelzunc Félix Belzunce Arcos made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          fbelzunc Félix Belzunce Arcos made changes -
          Status In Progress [ 3 ] Open [ 1 ]
          Hide
          jschlessel James Schlesselman added a comment -

          FYI . . I was not able to login after upgading to 2.15.  I downgraded back to 2.14 and was able to login again.

          Show
          jschlessel James Schlesselman added a comment - FYI . . I was not able to login after upgading to 2.15.  I downgraded back to 2.14 and was able to login again.
          Hide
          nsleigh Neil Sleightholm added a comment -

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

          Show
          nsleigh Neil Sleightholm added a comment - Same issue for me, 2.15 stops me logging in had to revert to 2.14.
          Hide
          jvz 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.

          Show
          jvz 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.
          Hide
          jvz Matt Sicker added a comment -

          Adding link to updated AD PR as a draft.

          Show
          jvz Matt Sicker added a comment - Adding link to updated AD PR as a draft.
          jvz Matt Sicker made changes -
          Remote Link This issue links to "#96 in active-directory (Web Link)" [ 23013 ]
          jvz Matt Sicker made changes -
          Remote Link This issue links to "#89 in active-directory (Web Link)" [ 22316 ]
          Hide
          wfollonier Wadeck Follonier added a comment -

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

          Show
          wfollonier Wadeck Follonier added a comment - The work on this ticket is "on-hold" for the moment, to be resumed soon-ish.
          Hide
          jvz 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.

          Show
          jvz 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.
          jvz Matt Sicker made changes -
          Description In the current situation, there is no check about the accounts that are disabled, locked or expired, or having their credentials expired in active-directory.

          This ticket has the goal to improve the situation by reading as much as possible from the attributes returned by the server.
          In the current situation, there is no check about the accounts that are disabled, locked or expired, or having their credentials expired in active-directory.

          This ticket has the goal to improve the situation by reading as much as possible from the attributes returned by the server.

          Relevant docs:
           * [https://ldapwiki.com/wiki/Administratively%20Disabled]
           ** [https://ldapwiki.com/wiki/ACCOUNTDISABLE]
           * [https://ldapwiki.com/wiki/Account%20Expiration]
           ** [https://ldapwiki.com/wiki/AccountExpires]
           * [https://ldapwiki.com/wiki/Password%20Expiration]
           ** [https://ldapwiki.com/wiki/AD%20Determining%20Password%20Expiration]
           * [https://ldapwiki.com/wiki/Account%20Lockout] and [https://ldapwiki.com/wiki/Intruder%20Detection]
           ** [https://ldapwiki.com/wiki/Active%20Directory%20Account%20Lockout]
          Hide
          jvz 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.

          Show
          jvz 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.
          jvz Matt Sicker made changes -
          Remote Link This issue links to "LDAP PR second attempt (Web Link)" [ 25811 ]
          jvz Matt Sicker made changes -
          Remote Link This issue links to "Core PR second attempt (Web Link)" [ 25812 ]
          Hide
          jvz 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.

          Show
          jvz 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.

            People

            • Assignee:
              wfollonier Wadeck Follonier
              Reporter:
              wfollonier Wadeck Follonier
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated: