Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[FIXED JENKINS-39375] Support a different bindUser per domain (#51)
[FIXED JENKINS-39375] Support a different bindUser per domain
  • Loading branch information
fbelzunc committed Oct 31, 2016
1 parent 2feb4e4 commit 16bc35c
Show file tree
Hide file tree
Showing 11 changed files with 157 additions and 38 deletions.
Expand Up @@ -48,7 +48,8 @@ protected AbstractActiveDirectoryAuthenticationProvider() {
/**
* Returns true if we can retrieve user just from the name without supplying any credential.
*/
protected abstract boolean canRetrieveUserByName();
@Deprecated
protected abstract boolean canRetrieveUserByName(ActiveDirectoryDomain domain);

public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException, DataAccessException {
return retrieveUser(username,null);
Expand Down
Expand Up @@ -216,7 +216,7 @@ public UserDetails call() {
}

@Override
protected boolean canRetrieveUserByName() {
protected boolean canRetrieveUserByName(ActiveDirectoryDomain domain) {
return true;
}

Expand Down
Expand Up @@ -27,6 +27,7 @@
import hudson.Extension;
import hudson.model.AbstractDescribableImpl;
import hudson.model.Descriptor;
import hudson.util.Secret;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
Expand Down Expand Up @@ -62,8 +63,23 @@ public class ActiveDirectoryDomain extends AbstractDescribableImpl<ActiveDirecto
*/
public String servers;

@DataBoundConstructor
/**
* If non-null, use this name and password to bind to LDAP to obtain the DN
* of the user trying to login. This is unnecessary in a single-domain mode,
* where we can just bind with the user name and password provided during
* the login, but in a forest mode, without some known credential, we cannot
* figure out which domain in the forest the user belongs to.
*/
public String bindName;

public Secret bindPassword;

public ActiveDirectoryDomain(String name, String servers) {
this(name, servers, null, null);
}

@DataBoundConstructor
public ActiveDirectoryDomain(String name, String servers, String bindName, String bindPassword) {
this.name = name;
// Append default port if not specified
servers = fixEmpty(servers);
Expand All @@ -77,6 +93,8 @@ public ActiveDirectoryDomain(String name, String servers) {
servers = StringUtils.join(serversArray, ",");
}
this.servers = servers;
this.bindName = fixEmpty(bindName);
this.bindPassword = Secret.fromString(fixEmpty(bindPassword));
}

@Restricted(NoExternalUse.class)
Expand All @@ -89,6 +107,16 @@ public String getServers() {
return servers;
}

@Restricted(NoExternalUse.class)
public String getBindName() {
return bindName;
}

@Restricted(NoExternalUse.class)
public Secret getBindPassword() {
return bindPassword;
}

/**
* Convert empty string to null.
*/
Expand Down
Expand Up @@ -142,15 +142,28 @@ public class ActiveDirectorySecurityRealm extends AbstractPasswordBasedSecurityR
public final String site;

/**
* If non-null, use this name and password to bind to LDAP to obtain the DN
* of the user trying to login. This is unnecessary in a single-domain mode,
* where we can just bind with the user name and password provided during
* the login, but in a forest mode, without some known credential, we cannot
* figure out which domain in the forest the user belongs to.
* Represent the old bindName
*
* <p>
* We need to keep this as transient in order to be able to use readResolve
* to migrate the old descriptor to the new one.
*
* <p>
* This has been deprecated @since Jenkins 2.1
*/
public final String bindName;
public transient String bindName;

public final Secret bindPassword;
/**
* Represent the old bindPassword
*
* <p>
* We need to keep this as transient in order to be able to use readResolve
* to migrate the old descriptor to the new one.
*
* <p>
* This has been deprecated @since Jenkins 2.1
*/
public transient Secret bindPassword;

private GroupLookupStrategy groupLookupStrategy;

Expand Down Expand Up @@ -192,7 +205,6 @@ public ActiveDirectorySecurityRealm(String domain, String site, String bindName,
public ActiveDirectorySecurityRealm(String domain, String site, String bindName,
String bindPassword, String server, GroupLookupStrategy groupLookupStrategy, boolean removeIrrelevantGroups, CacheConfiguration cache) {
this(domain, Lists.newArrayList(new ActiveDirectoryDomain(domain, null)), site, bindName, bindPassword, server, groupLookupStrategy, removeIrrelevantGroups, domain!=null, cache);

}

@DataBoundConstructor
Expand Down Expand Up @@ -302,6 +314,13 @@ public Object readResolve() throws ObjectStreamException {
this.domains.add(new ActiveDirectoryDomain(oldDomain, server));
}
}
// JENKINS-39375 Support a different bindUser per domain
if (bindName != null && bindPassword != null) {
for (ActiveDirectoryDomain activeDirectoryDomain : this.getDomains()) {
activeDirectoryDomain.bindName = bindName;
activeDirectoryDomain.bindPassword = bindPassword;
}
}
return this;
}

Expand Down Expand Up @@ -361,16 +380,6 @@ public void doAuthTest(StaplerRequest req, StaplerResponse rsp, @QueryParameter
req.getView(this, "test.jelly").forward(req, rsp);
}

public FormValidation doCheckBindName(@QueryParameter String bindName) {
String[] DnItems = {"CN=", "DC=", "OU="};
for (String dnItem : DnItems) {
if (bindName.contains(dnItem)) {
return FormValidation.warning("If you are using multiple domains the display name must be used");
}
}
return FormValidation.ok();
}

@Extension
public static final class DescriptorImpl extends Descriptor<SecurityRealm> {
public String getDisplayName() {
Expand Down
Expand Up @@ -79,8 +79,6 @@ public class ActiveDirectoryUnixAuthenticationProvider extends AbstractActiveDir

private final String site;

private final String bindName, bindPassword;

private final ActiveDirectorySecurityRealm.DescriptorImpl descriptor;

private GroupLookupStrategy groupLookupStrategy;
Expand Down Expand Up @@ -132,9 +130,7 @@ public ActiveDirectoryUnixAuthenticationProvider(ActiveDirectorySecurityRealm re
throw new IllegalArgumentException("An Active Directory domain name is required but it is not set");
}
this.site = realm.site;
this.bindName = realm.bindName;
this.domains = realm.domains;
this.bindPassword = Secret.toString(realm.bindPassword);
this.groupLookupStrategy = realm.getGroupLookupStrategy();
this.descriptor = realm.getDescriptor();
this.cache = realm.cache;
Expand Down Expand Up @@ -208,8 +204,8 @@ protected UserDetails retrieveUser(final String username, final UsernamePassword
}

@Override
protected boolean canRetrieveUserByName() {
return bindName!=null;
protected boolean canRetrieveUserByName(ActiveDirectoryDomain domain) {
return domain.getBindName()!=null;
}

/**
Expand Down Expand Up @@ -260,6 +256,8 @@ private List<SocketInfo> obtainLDAPServers(ActiveDirectoryDomain domain) throws
public UserDetails retrieveUser(final String username, final String password, final ActiveDirectoryDomain domain, final List<SocketInfo> ldapServers) {
UserDetails userDetails;
String hashKey = username + "@@" + DigestUtils.sha1Hex(password);
final String bindName = domain.getBindName();
final String bindPassword = domain.getBindPassword().getPlainText();
try {
final ActiveDirectoryUserDetail[] cacheMiss = new ActiveDirectoryUserDetail[1];
userDetails = userCache.get(hashKey, new Callable<UserDetails>() {
Expand Down Expand Up @@ -389,20 +387,19 @@ public UserDetails call() throws AuthenticationException {
}

public GroupDetails loadGroupByGroupname(final String groupname) {
if (bindName==null) {
throw new UserMayOrMayNotExistException("Unable to retrieve group information without bind DN/password configured");
}

try {
return groupCache.get(groupname, new Callable<ActiveDirectoryGroupDetails>() {
public ActiveDirectoryGroupDetails call() {
for (ActiveDirectoryDomain domain : domains) {
if (domain==null) {
throw new UserMayOrMayNotExistException("Unable to retrieve group information without bind DN/password configured");
}
// when we use custom socket factory below, every LDAP operations result
// in a classloading via context classloader, so we need it to resolve.
ClassLoader ccl = Thread.currentThread().getContextClassLoader();
Thread.currentThread().setContextClassLoader(getClass().getClassLoader());
try {
DirContext context = descriptor.bind(bindName, bindPassword, obtainLDAPServers(domain));
DirContext context = descriptor.bind(domain.getName(), domain.getBindPassword().getPlainText(), obtainLDAPServers(domain));

try {
final String domainDN = toDC(domain.getName());
Expand Down
Expand Up @@ -18,6 +18,12 @@
<f:entry title="${%Domain controller}" field="servers">
<f:textbox />
</f:entry>
<f:entry field="bindName" title="${%Bind DN}">
<f:textbox />
</f:entry>
<f:entry field="bindPassword" title="${%Bind Password}">
<f:password />
</f:entry>
</table>
<div align="right">
<input type="button" value="Delete Domain" class="repeatable-delete" style="margin-left: 1em;" />
Expand All @@ -41,6 +47,12 @@
<f:entry title="${%Domain controller}" field="servers">
<f:textbox />
</f:entry>
<f:entry field="bindName" title="${%Bind DN}" help="/plugin/active-directory/help/help-bindName.html">
<f:textbox />
</f:entry>
<f:entry field="bindPassword" title="${%Bind Password}">
<f:password />
</f:entry>
</table>
<div align="right">
<input type="button" value="Delete Domain" class="repeatable-delete" style="margin-left: 1em;" />
Expand Down
Expand Up @@ -4,12 +4,6 @@
<f:entry field="site" title="${%Site}">
<f:textbox />
</f:entry>
<f:entry field="bindName" title="${%Bind DN}">
<f:textbox />
</f:entry>
<f:entry field="bindPassword" title="${%Bind Password}">
<f:password />
</f:entry>
<f:entry field="groupLookupStrategy" title="${%Group Membership Lookup Strategy}">
<f:select />
</f:entry>
Expand Down
File renamed without changes.

0 comments on commit 16bc35c

Please sign in to comment.