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.

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

Allow to use a custom LDAP attribute as group name #261

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 15 additions & 10 deletions src/main/java/hudson/security/LDAPSecurityRealm.java
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
/*
* The MIT License
*
*
* Copyright (c) 2004-2010, Sun Microsystems, Inc., Kohsuke Kawaguchi, Seiji Sogabe,
* Olivier Lamy
* Copyright (c) 2017 CloudBees, Inc.
*
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
Expand Down Expand Up @@ -293,7 +293,7 @@ public class LDAPSecurityRealm extends AbstractPasswordBasedSecurityRealm {
justification = "This public field is exposed to the plugin's API")
@Deprecated @Restricted(NoExternalUse.class)
public transient String userSearch;

/**
* This defines the organizational unit that contains groups.
*
Expand Down Expand Up @@ -326,7 +326,7 @@ public class LDAPSecurityRealm extends AbstractPasswordBasedSecurityRealm {
* @deprecated use {@link #groupMembershipStrategy}
*/
@Deprecated @Restricted(NoExternalUse.class)
@SuppressFBWarnings(value = "UUF_UNUSED_PUBLIC_OR_PROTECTED_FIELD",
@SuppressFBWarnings(value = "UUF_UNUSED_PUBLIC_OR_PROTECTED_FIELD",
justification = "This public field is exposed to the plugin's API")
public transient String groupMembershipFilter;

Expand Down Expand Up @@ -363,7 +363,7 @@ group target (CN is a reasonable default)
public transient String managerDN;

@Deprecated @Restricted(NoExternalUse.class)
@SuppressFBWarnings(value = "UUF_UNUSED_PUBLIC_OR_PROTECTED_FIELD",
@SuppressFBWarnings(value = "UUF_UNUSED_PUBLIC_OR_PROTECTED_FIELD",
justification = "This public field is exposed to the plugin's API")
private transient String managerPassword;

Expand Down Expand Up @@ -1391,10 +1391,15 @@ public static final class AuthoritiesPopulatorImpl extends DefaultLdapAuthoritie
boolean convertToUpperCase = true;
private GrantedAuthority defaultRole = null;

public AuthoritiesPopulatorImpl(ContextSource contextSource, String groupSearchBase) {
public AuthoritiesPopulatorImpl(ContextSource contextSource, String groupSearchBase, LDAPGroupMembershipStrategy ldapGroupMembershipStrategy) {
super(contextSource, fixNull(groupSearchBase));

super.setRolePrefix("");
if (ldapGroupMembershipStrategy instanceof FromGroupSearchLDAPGroupMembershipStrategy) {
FromGroupSearchLDAPGroupMembershipStrategy fromGroupSearchLDAPGroupMembershipStrategy = (FromGroupSearchLDAPGroupMembershipStrategy) ldapGroupMembershipStrategy;
if (StringUtils.isNotBlank(fromGroupSearchLDAPGroupMembershipStrategy.getAttribute())) {
super.setGroupRoleAttribute(fromGroupSearchLDAPGroupMembershipStrategy.getAttribute());
}
}
super.setConvertToUpperCase(false);
}

Expand Down Expand Up @@ -1524,7 +1529,7 @@ public FormValidation doValidate(StaplerRequest req) throws Exception {
String user = json.getString("testUser");
String password = json.getString("testPassword");
JSONObject realmCfg = json.getJSONObject("securityRealm");

// instantiate the realm
LDAPSecurityRealm realm = req.bindJSON(LDAPSecurityRealm.class, realmCfg);
return validate(realm, user, password);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,19 +59,39 @@
*/
private final String filter;

@DataBoundConstructor
/**
* The LDAP attribute name which contains the group name (default = "cn").
*/
private final String attribute;

public FromGroupSearchLDAPGroupMembershipStrategy(String filter) {
this.filter = filter;
this.attribute = "";
}

@DataBoundConstructor
public FromGroupSearchLDAPGroupMembershipStrategy(String filter, String attribute) {
this.filter = filter;
this.attribute = attribute;
}

public String getFilter() {
return filter;
}

public String getAttribute() {
return attribute;
}

@Override
public void setAuthoritiesPopulator(LdapAuthoritiesPopulator authoritiesPopulator) {
if (authoritiesPopulator instanceof LDAPSecurityRealm.AuthoritiesPopulatorImpl && StringUtils.isNotBlank(filter)) {
((LDAPSecurityRealm.AuthoritiesPopulatorImpl) authoritiesPopulator).setGroupSearchFilter(filter);
if (authoritiesPopulator instanceof LDAPSecurityRealm.AuthoritiesPopulatorImpl) {

Check warning on line 88 in src/main/java/jenkins/security/plugins/ldap/FromGroupSearchLDAPGroupMembershipStrategy.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 88 is only partially covered, one branch is missing
if (StringUtils.isNotBlank(filter)) {
((LDAPSecurityRealm.AuthoritiesPopulatorImpl) authoritiesPopulator).setGroupSearchFilter(filter);
}
if (StringUtils.isNotBlank(attribute)) {
((LDAPSecurityRealm.AuthoritiesPopulatorImpl) authoritiesPopulator).setGroupRoleAttribute(attribute);
}
}
super.setAuthoritiesPopulator(authoritiesPopulator);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ public ApplicationContext createApplicationContext(LDAPSecurityRealm realm) {
// this is when we need to find it.
bindAuthenticator.setUserSearch(ldapUserSearch);

LDAPSecurityRealm.AuthoritiesPopulatorImpl ldapAuthoritiesPopulator = new LDAPSecurityRealm.AuthoritiesPopulatorImpl(contextSource, getGroupSearchBase());
LDAPSecurityRealm.AuthoritiesPopulatorImpl ldapAuthoritiesPopulator = new LDAPSecurityRealm.AuthoritiesPopulatorImpl(contextSource, getGroupSearchBase(), getGroupMembershipStrategy());
ldapAuthoritiesPopulator.setSearchSubtree(true);
ldapAuthoritiesPopulator.setGroupSearchFilter("(| (member={0}) (uniqueMember={0}) (memberUid={1}))");
if (realm.isDisableRolePrefixing()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,7 @@ THE SOFTWARE.
<f:entry field="filter" title="${%Group membership filter}">
<f:textbox/>
</f:entry>
<f:entry field="attribute" title="${%Group name attribute}">
<f:textbox/>
</f:entry>
</j:jelly>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<div>
The LDAP attribute name which contains the group name (default = "cn").
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<div>
Le nom de l'attribut LDAP contenant le nom du groupe (valeur par défaut = "cn")
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,4 @@
<f:checkbox />
</f:entry>
</f:advanced>
</j:jelly>
</j:jelly>
37 changes: 37 additions & 0 deletions src/test/java/hudson/security/LDAPEmbeddedTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,43 @@ public void userLookup_rolesFromUserRecord_modern() throws Exception {
assertThat(userGetAuthorities(details), containsInAnyOrder("HMS_Victory"));
}

@Test
@LDAPSchema(ldif = "sevenSeas", id = "sevenSeas", dn = "o=sevenSeas")
public void userLookup_rolesFromGroupSearchWithGroupAttribute() throws Exception {
LDAPSecurityRealm realm = new LDAPSecurityRealm(
ads.getUrl(),
null,
null,
null,
null,
null,
new FromGroupSearchLDAPGroupMembershipStrategy(null, "description"),
"uid=admin,ou=system",
Secret.fromString("pass"),
false,
false,
new LDAPSecurityRealm.CacheConfiguration(100, 1000),
new LDAPSecurityRealm.EnvironmentProperty[0],
"cn",
null,
IdStrategy.CASE_INSENSITIVE,
IdStrategy.CASE_INSENSITIVE);
r.jenkins.setSecurityRealm(realm);
User user = User.get("hhornblo", true, Collections.emptyMap());
List<String> authorities = user.getAuthorities();
assertThat(user.getAuthorities(), containsInAnyOrder("HMS_Lydia", "ROLE_HMS_LYDIA"));
assertThat(user.getDisplayName(), is("Horatio Hornblower"));
assertThat(user.getProperty(Mailer.UserProperty.class).getAddress(), is("[email protected]"));
UserDetails details = realm.authenticate2("hhornblo", "pass");
assertThat(userGetAuthorities(details), containsInAnyOrder("HMS_Lydia", "ROLE_HMS_LYDIA"));
user = User.get("hnelson", true, Collections.emptyMap());
assertThat(user.getAuthorities(), containsInAnyOrder("HMS_Victory", "ROLE_HMS_VICTORY"));
assertThat(user.getDisplayName(), is("Horatio Nelson"));
assertThat(user.getProperty(Mailer.UserProperty.class).getAddress(), is("[email protected]"));
details = realm.authenticate2("hnelson", "pass");
assertThat(userGetAuthorities(details), containsInAnyOrder("HMS_Victory", "ROLE_HMS_VICTORY"));
}

private Set<String> userGetAuthorities(UserDetails details) {
Set<String> authorities = new LinkedHashSet<>();
for (GrantedAuthority a : details.getAuthorities()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,6 @@ public void configure_ldap() {
assertEquals("(&(cn={0})(objectclass=group))", configuration.getGroupSearchFilter());
final FromGroupSearchLDAPGroupMembershipStrategy strategy = ((FromGroupSearchLDAPGroupMembershipStrategy) configuration.getGroupMembershipStrategy());
assertEquals("(&(objectClass=group)(|(cn=GROUP_1)(cn=GROUP_2)))", strategy.getFilter());
assertEquals("description", strategy.getAttribute());
}
}
3 changes: 3 additions & 0 deletions src/test/resources/hudson/security/sevenSeas.ldif
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ dn: cn=HMS Lydia,ou=crews,ou=groups,o=sevenSeas
objectclass: groupOfUniqueNames
objectclass: top
cn: HMS Lydia
description: HMS_Lydia
uniquemember: cn=Horatio Hornblower,ou=people,o=sevenSeas
uniquemember: cn=William Bush,ou=people,o=sevenSeas
uniquemember: cn=Thomas Quist,ou=people,o=sevenSeas
Expand Down Expand Up @@ -236,6 +237,7 @@ dn: cn=HMS Victory,ou=crews,ou=groups,o=sevenSeas
objectclass: groupOfUniqueNames
objectclass: top
cn: HMS Victory
description: HMS_Victory
uniquemember: cn=Horatio Nelson,ou=people,o=sevenSeas
uniquemember: cn=Thomas Masterman Hardy,ou=people,o=sevenSeas
uniquemember: cn=Cornelius Buckley,ou=people,o=sevenSeas
Expand Down Expand Up @@ -320,6 +322,7 @@ dn: cn=HMS Bounty,ou=crews,ou=groups,o=sevenSeas
objectclass: groupOfUniqueNames
objectclass: top
cn: HMS Bounty
description: HMS_Bounty
uniquemember: cn=William Bligh,ou=people,o=sevenSeas
uniquemember: cn=Fletcher Christian,ou=people,o=sevenSeas
uniquemember: cn=John Fryer,ou=people,o=sevenSeas
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ jenkins:
groupMembershipStrategy:
fromGroupSearch:
filter: "(&(objectClass=group)(|(cn=GROUP_1)(cn=GROUP_2)))"
attribute: "description"
cache:
size: 100
ttl: 10
Expand Down