Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build against Spring Security #49

Merged
merged 52 commits into from
Nov 17, 2020
Merged

Build against Spring Security #49

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
9bf71eb
Trying to compile against Spring Security
jglick Jul 31, 2020
bc478a4
Some more progress
jglick Jul 31, 2020
c2cab64
Merge branch 'spring-5303' into spring-5303-new
jglick Aug 24, 2020
6463be1
Using an incremental jenkins.version
jglick Aug 24, 2020
d600c62
Got LDAPUserDetailsService to be compilable
jglick Aug 24, 2020
5b3de91
Reusing DelegatedLdapUserDetails to keep track of Attributes, otherwi…
jglick Aug 24, 2020
7b876e8
More strongly type some stuff as DelegatedLdapUserDetails
jglick Aug 24, 2020
f345adf
Making tests compilable
jglick Aug 24, 2020
ffe7187
Missing call to AbstractContextSource.afterPropertiesSet
jglick Aug 25, 2020
7dcddf3
Missing call to LdapUserDetailsImpl.Essence.setUsername
jglick Aug 25, 2020
03a4aaf
Various uses of Acegi Security in tests
jglick Aug 25, 2020
d62654c
Cargo cult programming
jglick Aug 25, 2020
a1f1f4d
LDAPConfiguration.createApplicationContext already configures Authori…
jglick Aug 25, 2020
d145884
Stray reference to DataAccessException
jglick Aug 26, 2020
17feb1a
DataAccessException has been replaced by AuthenticationServiceExcepti…
jglick Aug 26, 2020
8d576b9
Merge branch 'spring-5303' into spring-5303-new
jglick Sep 3, 2020
0f8b360
Core bump
jglick Sep 3, 2020
9993b9b
Found something to do with extraEnvVars, but LDAPEmbeddedTest#usingEn…
jglick Sep 3, 2020
a0943f4
Got LDAPEmbeddedTest#validate to pass by threading an LdapUserSearch …
jglick Sep 3, 2020
270cffc
Merge branch 'spring-5303' into spring-5303-new
jglick Sep 14, 2020
4b99b0b
Merge branch 'master' of github.com:jenkinsci/ldap-plugin into spring…
jglick Sep 15, 2020
c1e4287
Merge branch 'master' of github.com:jenkinsci/ldap-plugin into spring…
jglick Sep 15, 2020
8159c83
Picking up some core fixes
jglick Sep 15, 2020
a9dc4e4
AuthoritiesPopulatorImpl handling of rolePrefix and convertToUpperCas…
jglick Sep 15, 2020
68f6252
Less intrusive import rearrangement
jglick Sep 15, 2020
c08f374
Javadoc errors
jglick Sep 15, 2020
e7e4064
Failures in LdapMultiEmbeddedTest related to AuthenticationServiceExc…
jglick Sep 15, 2020
8ce42d8
Updated parent
jglick Sep 15, 2020
b876e54
Fixing some more failures in LdapMultiEmbeddedTest.
jglick Sep 15, 2020
fdc1ec0
Fixing LdapMultiEmbedded2Test.login by returning DelegatedLdapAuthent…
jglick Sep 15, 2020
7c00268
Found DefaultSpringSecurityContextSource.setBaseEnvironmentProperties…
jglick Sep 15, 2020
87b0e14
LDAPSecurityRealmTest was passing invalid rootDN values; syntax is no…
jglick Sep 15, 2020
53f75aa
SpotBugs
jglick Sep 15, 2020
74a81d6
No need to bundle all of Spring & Spring Security, just the LDAP-spec…
jglick Sep 16, 2020
f95183c
Noting that MailAddressResolver already works, and indeed still works…
jglick Sep 16, 2020
e15835f
Got LDAPSecurityRealmTest.sessionStressTest to compile, and figured o…
jglick Sep 16, 2020
474aa6d
Reimplemented LDAPUserDetailsService.attributesCache
jglick Sep 16, 2020
4a3a73d
Despite the name, LdapUserDetailsImpl.Essence.dn should really be Dir…
jglick Sep 16, 2020
92b0950
No need to clone ldapUser if it is the first one using a given NameAw…
jglick Sep 16, 2020
321a357
Was doing too much work—no need to clone the DirContextAdapter.
jglick Sep 16, 2020
e530174
There is no real reason to enforce use of NameAwareAttributes.
jglick Sep 16, 2020
e8a2a6a
Introduced SetContextClassLoader to fix class loading errors (see JEN…
jglick Sep 17, 2020
2935db9
Reordering some imports
jglick Sep 17, 2020
b994caf
Non-ACI agents not working out
jglick Sep 17, 2020
9577fe6
Forgot that ACI agents are not going to be able to run Dockerized tests.
jglick Sep 17, 2020
69fa05a
https://github.com/jenkinsci/ldap-plugin/pull/49#discussion_r495065425
jglick Sep 25, 2020
f779469
Merge branch 'master' of github.com:jenkinsci/ldap-plugin into spring…
jglick Sep 28, 2020
5ae8750
Finally found a way to make LDAPEmbeddedTest.usingEnvironmentProperti…
jglick Sep 28, 2020
03d10b1
https://github.com/jenkinsci/ldap-plugin/pull/49#discussion_r496120420
jglick Sep 28, 2020
93d8489
https://docs.oracle.com/javase/8/docs/technotes/guides/jndi/jndi-ldap…
jglick Oct 7, 2020
475a03b
Merge branch 'master' of https://github.com/jenkinsci/ldap-plugin int…
jglick Nov 10, 2020
d46b981
Can now use 2.266
jglick Nov 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 3 additions & 53 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

<properties>
<apacheds.version>2.0.0.AM25</apacheds.version>
<jenkins.version>2.235.3</jenkins.version>
<jenkins.version>2.251-SNAPSHOT</jenkins.version> <!-- TODO https://github.com/jenkinsci/jenkins/pull/4848 -->
<java.level>8</java.level>
<configuration-as-code.version>1.36</configuration-as-code.version>
</properties>
Expand All @@ -50,59 +50,9 @@
</pluginRepositories>

<dependencies>
<dependency> <!-- for compatibility with https://github.com/jenkinsci/jenkins/pull/4848 -->
<groupId>org.acegisecurity</groupId>
<artifactId>acegi-security</artifactId>
<version>1.0.7</version>
<exclusions>
<exclusion>
<groupId>commons-codec</groupId>
<artifactId>commons-codec</artifactId>
</exclusion>
<exclusion>
<groupId>commons-collections</groupId>
<artifactId>commons-collections</artifactId>
</exclusion>
<exclusion>
<groupId>commons-lang</groupId>
<artifactId>commons-lang</artifactId>
</exclusion>
<exclusion>
<groupId>org.springframework</groupId>
<artifactId>spring-core</artifactId>
</exclusion>
<exclusion>
<groupId>org.springframework</groupId>
<artifactId>spring-jdbc</artifactId>
</exclusion>
<exclusion>
<groupId>org.springframework</groupId>
<artifactId>spring-remoting</artifactId>
</exclusion>
<exclusion>
<groupId>org.springframework</groupId>
<artifactId>spring-support</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.springframework</groupId>
<artifactId>spring-dao</artifactId>
<version>1.2.9</version>
<exclusions>
<exclusion>
<groupId>org.springframework</groupId>
<artifactId>spring-beans</artifactId>
</exclusion>
<exclusion>
<groupId>org.springframework</groupId>
<artifactId>spring-context</artifactId>
</exclusion>
<exclusion>
<groupId>org.springframework</groupId>
<artifactId>spring-core</artifactId>
</exclusion>
</exclusions>
<groupId>org.springframework.security</groupId>
<artifactId>spring-security-ldap</artifactId>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
Expand Down
219 changes: 99 additions & 120 deletions src/main/java/hudson/security/LDAPSecurityRealm.java
View file
Open in desktop

Large diffs are not rendered by default.

21 changes: 11 additions & 10 deletions ...dap/authenticator/BindAuthenticator2.java → ...rity/plugins/ldap/BindAuthenticator2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,14 @@
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package org.acegisecurity.providers.ldap.authenticator;

import org.acegisecurity.ldap.InitialDirContextFactory;
import org.acegisecurity.userdetails.ldap.LdapUserDetails;
package jenkins.security.plugins.ldap;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

handleBindException is now protected so no more need to align packages.


import java.util.logging.Logger;
import java.util.logging.Level;
import org.springframework.ldap.core.DirContextOperations;
import org.springframework.ldap.core.support.BaseLdapPathContextSource;
import org.springframework.security.core.Authentication;
import org.springframework.security.ldap.authentication.BindAuthenticator;

/**
* {@link BindAuthenticator} with improved diagnostics.
Expand All @@ -40,19 +41,19 @@ public class BindAuthenticator2 extends BindAuthenticator {
*/
private boolean hadSuccessfulAuthentication;

public BindAuthenticator2(InitialDirContextFactory initialDirContextFactory) {
super(initialDirContextFactory);
public BindAuthenticator2(BaseLdapPathContextSource contextSource) {
super(contextSource);
jglick marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
public LdapUserDetails authenticate(String username, String password) {
LdapUserDetails user = super.authenticate(username, password);
public DirContextOperations authenticate(Authentication authentication) {
DirContextOperations operations = super.authenticate(authentication);
hadSuccessfulAuthentication = true;
return user;
return operations;
}

@Override
void handleBindException(String userDn, String username, Throwable cause) {
protected void handleBindException(String userDn, String username, Throwable cause) {
LOGGER.log(hadSuccessfulAuthentication? Level.FINE : Level.WARNING,
"Failed to bind to LDAP: userDn"+userDn+" username="+username,cause);
super.handleBindException(userDn, username, cause);
Expand Down
20 changes: 10 additions & 10 deletions src/main/java/jenkins/security/plugins/ldap/FromGroupSearchLDAPGroupMembershipStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

import hudson.Extension;
import hudson.security.LDAPSecurityRealm;
import java.util.Collection;
import java.util.Collections;
import java.util.Set;
import java.util.TreeSet;
Expand All @@ -35,13 +36,12 @@
import javax.naming.NamingException;
import javax.naming.directory.Attributes;
import javax.naming.ldap.LdapName;
import org.acegisecurity.GrantedAuthority;
import org.acegisecurity.ldap.LdapEntryMapper;
import org.acegisecurity.providers.ldap.LdapAuthoritiesPopulator;
import org.acegisecurity.userdetails.ldap.LdapUserDetails;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.stapler.DataBoundConstructor;
import org.springframework.dao.DataAccessException;
import org.springframework.ldap.core.DirContextOperations;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.ldap.LdapUtils;
import org.springframework.security.ldap.userdetails.LdapAuthoritiesPopulator;

/**
* Traditional strategy.
Expand Down Expand Up @@ -77,15 +77,15 @@ public void setAuthoritiesPopulator(LdapAuthoritiesPopulator authoritiesPopulato
}

@Override
public GrantedAuthority[] getGrantedAuthorities(LdapUserDetails ldapUser) {
return getAuthoritiesPopulator().getGrantedAuthorities(ldapUser);
public Collection<? extends GrantedAuthority> getGrantedAuthorities(DirContextOperations userData, String username) {
return getAuthoritiesPopulator().getGrantedAuthorities(userData, username);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just trying to make something work.

}

@Override
public Set<String> getGroupMembers(String groupDn, LDAPConfiguration conf) throws DataAccessException {
public Set<String> getGroupMembers(String groupDn, LDAPConfiguration conf) {
LDAPExtendedTemplate template = conf.getLdapTemplate();
String[] memberAttributes = { "member", "uniqueMember", "memberUid" };
return (Set<String>) template.retrieveEntry(groupDn, new GroupMembersMapper(), memberAttributes);
return template.executeReadOnly(ctx -> new GroupMembersMapper().mapAttributes(groupDn, ctx.getAttributes(LdapUtils.getRelativeName(groupDn, ctx), memberAttributes)));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just guessing here. Read only? Read write? retrieveEntry is nowhere to be found in Spring Security so I tried to inline the Acegi Security behavior to the extent I understood it. Note that this means GroupMembersMapper.mapAttributes could just be inlined here too, but I am trying to keep the diff small where possible.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the calls should be read-only. The ldap plugin never writes in the remote ldap afair.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could just be inlined here too

⇒ #52

}

@Extension
Expand All @@ -100,7 +100,7 @@ public String getDisplayName() {
/**
* Maps member attributes in groups to a set of member names.
*/
private static class GroupMembersMapper implements LdapEntryMapper {
private static class GroupMembersMapper implements LdapEntryMapper<Set<String>> {
@Override
public Set<String> mapAttributes(String dn, Attributes attributes) throws NamingException {
NamingEnumeration<?> enumeration;
Expand Down
31 changes: 15 additions & 16 deletions src/main/java/jenkins/security/plugins/ldap/FromUserRecordLDAPGroupMembershipStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,26 +26,25 @@
import hudson.Extension;
import hudson.security.LDAPSecurityRealm;
import java.util.Set;
import org.acegisecurity.GrantedAuthority;
import org.acegisecurity.GrantedAuthorityImpl;
import org.acegisecurity.ldap.LdapEntryMapper;
import org.acegisecurity.userdetails.ldap.LdapUserDetails;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.stapler.DataBoundConstructor;
import org.springframework.dao.DataAccessException;

import javax.naming.InvalidNameException;
import javax.naming.NamingException;
import javax.naming.directory.Attribute;
import javax.naming.directory.Attributes;
import javax.naming.ldap.LdapName;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.logging.Level;
import java.util.logging.LogRecord;
import java.util.logging.Logger;
import org.springframework.ldap.core.DirContextOperations;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.core.authority.SimpleGrantedAuthority;

/**
* This strategy is rumoured to work for Active Directory!
Expand All @@ -67,9 +66,9 @@ public String getAttributeName() {
}

@Override
public GrantedAuthority[] getGrantedAuthorities(LdapUserDetails ldapUser) {
public Collection<? extends GrantedAuthority> getGrantedAuthorities(DirContextOperations userData, String username) {
List<GrantedAuthority> result = new ArrayList<GrantedAuthority>();
Attributes attributes = ldapUser.getAttributes();
Attributes attributes = userData.getAttributes();
final String attributeName = getAttributeName();
Attribute attribute = attributes == null ? null : attributes.get(attributeName);
if (attribute != null) {
Expand All @@ -82,7 +81,7 @@ public GrantedAuthority[] getGrantedAuthorities(LdapUserDetails ldapUser) {
} catch (InvalidNameException e) {
LOGGER.log(Level.FINEST, "Expected a Group DN but found: {0}", groupName);
}
result.add(new GrantedAuthorityImpl(groupName));
result.add(new SimpleGrantedAuthority(groupName));
}
} catch (NamingException e) {
LogRecord lr = new LogRecord(Level.FINE,
Expand All @@ -102,37 +101,37 @@ public GrantedAuthority[] getGrantedAuthorities(LdapUserDetails ldapUser) {
String role = ga.getAuthority();

// backward compatible name mangling
if (authoritiesPopulatorImpl.isConvertToUpperCase()) {
if (authoritiesPopulatorImpl._isConvertToUpperCase()) {
role = role.toUpperCase();
}
GrantedAuthorityImpl extraAuthority = new GrantedAuthorityImpl(
authoritiesPopulatorImpl.getRolePrefix() + role);
GrantedAuthority extraAuthority = new SimpleGrantedAuthority(
authoritiesPopulatorImpl._getRolePrefix() + role);
result.add(extraAuthority);
}
}
result.addAll(authoritiesPopulatorImpl.getAdditionalRoles(ldapUser));
result.addAll(authoritiesPopulatorImpl.getAdditionalRoles(userData, /* TODO currently ignored anyway, but where does username come from? */null));
jglick marked this conversation as resolved.
Show resolved Hide resolved
GrantedAuthority defaultRole = authoritiesPopulatorImpl.getDefaultRole();
if (defaultRole != null) {
result.add(defaultRole);
}
}

return result.toArray(new GrantedAuthority[result.size()]);
return result;
}

@Override
public Set<String> getGroupMembers(String groupDn, LDAPConfiguration conf) throws DataAccessException {
public Set<String> getGroupMembers(String groupDn, LDAPConfiguration conf) {
LDAPExtendedTemplate template = conf.getLdapTemplate();
String searchBase = conf.getUserSearchBase() != null ? conf.getUserSearchBase() : "";
String[] filterArgs = { getAttributeName(), groupDn };
return new HashSet<>((List<String>)template.searchForAllEntries(searchBase, USER_SEARCH_FILTER,
return new HashSet<>(template.searchForAllEntries(searchBase, USER_SEARCH_FILTER,
filterArgs, new String[]{}, new UserRecordMapper()));
}

/**
* Maps users records to names.
*/
private static class UserRecordMapper implements LdapEntryMapper {
private static class UserRecordMapper implements LdapEntryMapper<String> {
@Override
public String mapAttributes(String dn, Attributes attributes) throws NamingException {
LdapName name = new LdapName(dn);
Expand Down
Loading