-
Type:
Improvement
-
Status: Open (View Workflow)
-
Priority:
Major
-
Resolution: Unresolved
-
Component/s: active-directory-plugin, core, ldap-plugin
-
Labels:None
-
Similar Issues:
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:
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.
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.
FYI . . I was not able to login after upgading to 2.15. I downgraded back to 2.14 and was able to login again.
Same issue for me, 2.15 stops me logging in had to revert to 2.14.
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.
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.
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.
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.
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.
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.