Build against Spring Security #49
Conversation
| DefaultInitialDirContextFactory initialDirContextFactory = new DefaultInitialDirContextFactory(getLDAPURL()); | ||
| DefaultSpringSecurityContextSource contextSource = new DefaultSpringSecurityContextSource(getLDAPURL()); |
| initialDirContextFactory.setManagerDn(getManagerDN()); | ||
| initialDirContextFactory.setManagerPassword(getManagerPassword()); | ||
| contextSource.setUserDn(getManagerDN()); | ||
| contextSource.setPassword(getManagerPassword()); |
There was a problem hiding this comment.
Inferred from LdapAuthenticationProviderConfigurer that this is the equivalent.
src/main/java/jenkins/security/plugins/ldap/LDAPConfiguration.java
Outdated
Show resolved
Hide resolved
| import org.springframework.ldap.core.support.AbstractContextMapper; | ||
|
|
||
| /** | ||
| * Sort of like {@link AttributesMapper} but with a DN; also sort of like {@link AbstractContextMapper}. |
There was a problem hiding this comment.
Could find no exact equivalent of this Acegi Security type, but anyway it is only being used internally here.
…se segregated in DirContextOperations; InetOrgPerson would be more natural but LDAPConfiguration is too, well, configurable
…tiesPopulatorImpl so its constructor should not do so redundantly; unclear how tests pass in jenkinsci#46 without this
|
Since this will be a major version change with long sought after upgrades, I think removing some of the small obscure features is OK. |
Probably so, but I do not know enough about this plugin’s usage to even know which features are “small” or “obscure”.
|
…on, so adjusting LdapMultiEmbeddedTest to match
…vironmentProperties still fails
…through various places to populate DelegatedLdapUserDetails.attributes more reliably
Can try to use ACI for the Windows build but a VM for Linux builds.
| @@ -1765,7 +1770,7 @@ public FormValidation validate(LDAPSecurityRealm realm, String user, String pass | |||
| .LDAPSecurityRealm_UserLookupManagerDnPermissions(), | |||
| jenkins.security.plugins.ldap.Messages.LDAPSecurityRealm_UserLookupSettingsCorrect()); | |||
| // we do not flag these errors as could be probing user accounts | |||
| } catch (LdapDataAccessException e) { | |||
| } catch (AuthenticationException e) { | |||
There was a problem hiding this comment.
The old LdapDataAccessException meant that there was problems communicating with the ldap server, or your query was wrong or similar. Is the new AuthenticationException equivalent? By the name it sounds to me like it's a different exception.
There was a problem hiding this comment.
There is no more org.springframework.dao.DataAccessException or its subtypes, so in the core PR as well as here I have converted these to the more generic AuthenticationException. Whether this is the most appropriate type or not, I do not know, and I am not sure we really care.
| * Creates security components. | ||
| * @return Created {@link SecurityComponents} | ||
| * @throws IllegalStateException Execution error | ||
| */ | ||
| @Override @Nonnull | ||
| public SecurityComponents createSecurityComponents() { | ||
| if (configurations.size() > 1) { |
There was a problem hiding this comment.
History lesson for context;
When we implemented the multiple ldap server support we were a bit paranoid of changing the behavior in any way for configurations with only one server configured. That's why you see all of these if (configurations.size() > 1) all over the place. It was also the reason for the DelegatedLdapUserDetails class that I see you have changed to always be used instead of before; only when multiple configurations are in effect.
Since the behavior (both user wise but also in binary compatibility) is changing now and DelegatedLdapUserDetails is used all the time, I believe it's now possible to remove all this semi redundant code that has separate treatments for the single server configuration scenario.
But I won't block this PR if you don't want to include that cleanup work here ;).
…nsulated from a release of jenkinsci/ldap-plugin#49
Following up #48 but now trying to build against jenkinsci/jenkins#4848 and use only Spring Security.
Managed to make everything compile & pass tests. @rwinch I will likely need your help here. There seem to have been quite substantial changes between Acegi Security and Spring Security in the LDAP support, and I have not managed to find a migration guide. Most (though not all) of the problems center around some change from using
LdapUserDetails, which used to have agetAttributesmethod, toDirContextOperations(which has this method) plus aString usernamebut which is not anAuthenticationorUserDetails. I am feeling my way around in the dark, knowing next to nothing about LDAP, JNDI, etc.