Skip to content

Commit

Permalink
[FIXED JENKINS-39423] Make site independent of each domain (#54)
Browse files Browse the repository at this point in the history 
[JENKINS-39423] Make site independent of each domain
  • Loading branch information
@fbelzunc
fbelzunc committed Nov 9, 2016
1 parent 6bb270d commit 8675b69
Show file tree
Hide file tree
Showing 13 changed files with 77 additions and 12 deletions.
10 changes: 10 additions & 0 deletions pom.xml
View file
Expand Up @@ -91,6 +91,16 @@
<compatibleSinceVersion>2.0</compatibleSinceVersion>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>2.19.1</version>
<configuration>
<forkCount>2C</forkCount>
<reuseForks>true</reuseForks>
<argLine>-Xms2048m -Xmx2048m</argLine>
</configuration>
</plugin>
</plugins>
</build>

Expand Down
20 changes: 18 additions & 2 deletions src/main/java/hudson/plugins/active_directory/ActiveDirectoryDomain.java
View file
Expand Up @@ -63,6 +63,16 @@ public class ActiveDirectoryDomain extends AbstractDescribableImpl<ActiveDirecto
*/
public String servers;

/**
* Active directory site (which specifies the physical concentration of the
* servers), if any. If the value is non-null, we'll only contact servers in
* this site.
*
* <p>
* On Windows, I'm assuming ADSI takes care of everything automatically.
*/
public 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,
Expand All @@ -75,11 +85,11 @@ public class ActiveDirectoryDomain extends AbstractDescribableImpl<ActiveDirecto
public Secret bindPassword;

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

@DataBoundConstructor
public ActiveDirectoryDomain(String name, String servers, String bindName, String bindPassword) {
public ActiveDirectoryDomain(String name, String servers, String site, String bindName, String bindPassword) {
this.name = name;
// Append default port if not specified
servers = fixEmpty(servers);
Expand All @@ -93,6 +103,7 @@ public ActiveDirectoryDomain(String name, String servers, String bindName, Strin
servers = StringUtils.join(serversArray, ",");
}
this.servers = servers;
this.site = fixEmpty(site);
this.bindName = fixEmpty(bindName);
this.bindPassword = Secret.fromString(fixEmpty(bindPassword));
}
Expand All @@ -117,6 +128,11 @@ public Secret getBindPassword() {
return bindPassword;
}

@Restricted(NoExternalUse.class)
public String getSite() {
return site;
}

/**
* Convert empty string to null.
*/
Expand Down
34 changes: 28 additions & 6 deletions src/main/java/hudson/plugins/active_directory/ActiveDirectorySecurityRealm.java
View file
Expand Up @@ -138,8 +138,12 @@ public class ActiveDirectorySecurityRealm extends AbstractPasswordBasedSecurityR
*
* <p>
* On Windows, I'm assuming ADSI takes care of everything automatically.
*
* <p>
* We need to keep this as transient in order to be able to use readResolve
* to migrate the old descriptor to the newone.
*/
public final String site;
public transient final String site;

/**
* Represent the old bindName
Expand Down Expand Up @@ -189,6 +193,8 @@ public class ActiveDirectorySecurityRealm extends AbstractPasswordBasedSecurityR

public transient String testDomainControllers;

public transient String testSite;

public ActiveDirectorySecurityRealm(String domain, String site, String bindName, String bindPassword, String server) {
this(domain, site, bindName, bindPassword, server, GroupLookupStrategy.AUTO, false);
}
Expand Down Expand Up @@ -304,6 +310,11 @@ public String getTestDomainControllers() {
return testDomainControllers;
}

@Restricted(NoExternalUse.class)
public String getTestSite() {
return testSite;
}

public Object readResolve() throws ObjectStreamException {
if (domain != null) {
this.domains = new ArrayList<ActiveDirectoryDomain>();
Expand All @@ -321,6 +332,12 @@ public Object readResolve() throws ObjectStreamException {
activeDirectoryDomain.bindPassword = bindPassword;
}
}
// JENKINS-39423 Make site independent of each domain
if (site != null) {
for (ActiveDirectoryDomain activeDirectoryDomain : this.getDomains()) {
activeDirectoryDomain.site = site;
}
}
return this;
}

Expand Down Expand Up @@ -349,8 +366,8 @@ public void doAuthTest(StaplerRequest req, StaplerResponse rsp, @QueryParameter

for (ActiveDirectoryDomain domain : domains) {
try {
pw.println("Domain= " + domain.getName() + " site= "+ site);
List<SocketInfo> ldapServers = descriptor.obtainLDAPServer(domain.getName(), site, domain.getServers());
pw.println("Domain= " + domain.getName() + " site= "+ domain.getSite());
List<SocketInfo> ldapServers = descriptor.obtainLDAPServer(domain);
pw.println("List of domain controllers: "+ldapServers);

for (SocketInfo ldapServer : ldapServers) {
Expand Down Expand Up @@ -447,7 +464,7 @@ public ListBoxModel doFillGroupLookupStrategyItems() {

private static boolean WARNED = false;

public FormValidation doValidateTest(@QueryParameter(fixEmpty = true) String testDomain, @QueryParameter(fixEmpty = true) String testDomainControllers, @QueryParameter(fixEmpty = true) String site, @QueryParameter(fixEmpty = true) String bindName,
public FormValidation doValidateTest(@QueryParameter(fixEmpty = true) String testDomain, @QueryParameter(fixEmpty = true) String testDomainControllers, @QueryParameter(fixEmpty = true) String testSite, @QueryParameter(fixEmpty = true) String bindName,
@QueryParameter(fixEmpty = true) String bindPassword) throws IOException, ServletException, NamingException {
ClassLoader ccl = Thread.currentThread().getContextClassLoader();
Thread.currentThread().setContextClassLoader(getClass().getClassLoader());
Expand Down Expand Up @@ -503,9 +520,9 @@ public FormValidation doValidateTest(@QueryParameter(fixEmpty = true) String tes
// Then look for the LDAP server
List<SocketInfo> servers;
try {
servers = obtainLDAPServer(ictx, testDomain, site, testDomainControllers);
servers = obtainLDAPServer(ictx, testDomain, testSite, testDomainControllers);
} catch (NamingException e) {
String msg = site == null ? "No LDAP server was found in " + testDomain : "No LDAP server was found in the " + site + " site of " + testDomain;
String msg = testSite == null ? "No LDAP server was found in " + testDomain : "No LDAP server was found in the " + testSite + " site of " + testDomain;
LOGGER.log(Level.WARNING, msg, e);
return FormValidation.error(e, msg);
}
Expand Down Expand Up @@ -697,10 +714,15 @@ public DirContext createDNSLookupContext() throws NamingException {
return new InitialDirContext(env);
}

@Deprecated
public List<SocketInfo> obtainLDAPServer(String domainName, String site, String preferredServer) throws NamingException {
return obtainLDAPServer(createDNSLookupContext(), domainName, site, preferredServer);
}

public List<SocketInfo> obtainLDAPServer(ActiveDirectoryDomain activeDirectoryDomain) throws NamingException {
return obtainLDAPServer(createDNSLookupContext(), activeDirectoryDomain.getName(), activeDirectoryDomain.getSite(), activeDirectoryDomain.getServers());
}

// domain name prefixes
// see http://technet.microsoft.com/en-us/library/cc759550(WS.10).aspx
private static final List<String> CANDIDATES = Arrays.asList("_gc._tcp.", "_ldap._tcp.");
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/hudson/plugins/active_directory/ActiveDirectoryUnixAuthenticationProvider.java
View file
Expand Up @@ -235,7 +235,7 @@ private UserDetails retrieveUser(String username, UsernamePasswordAuthentication
*/
private List<SocketInfo> obtainLDAPServers(ActiveDirectoryDomain domain) throws AuthenticationServiceException {
try {
return descriptor.obtainLDAPServer(domain.getName(), site, domain.getServers());
return descriptor.obtainLDAPServer(domain);
} catch (NamingException e) {
LOGGER.log(Level.WARNING, "Failed to find the LDAP service", e);
throw new AuthenticationServiceException("Failed to find the LDAP service for the domain "+ domain.getName(), e);
Expand Down
6 changes: 6 additions & 0 deletions src/main/resources/hudson/plugins/active_directory/ActiveDirectorySecurityRealm/config.jelly
View file
Expand Up @@ -18,6 +18,9 @@
<f:entry title="${%Domain controller}" field="servers">
<f:textbox />
</f:entry>
<f:entry field="site" title="${%Site}">
<f:textbox />
</f:entry>
<f:entry field="bindName" title="${%Bind DN}">
<f:textbox />
</f:entry>
Expand Down Expand Up @@ -47,6 +50,9 @@
<f:entry title="${%Domain controller}" field="servers">
<f:textbox />
</f:entry>
<f:entry field="site" title="${%Site}" help="/plugin/active-directory/help/help-site.html">
<f:textbox />
</f:entry>
<f:entry field="bindName" title="${%Bind DN}" help="/plugin/active-directory/help/help-bindName.html">
<f:textbox />
</f:entry>
Expand Down
6 changes: 3 additions & 3 deletions ...sources/hudson/plugins/active_directory/ActiveDirectorySecurityRealm/configAdvanced.jelly
View file
@@ -1,9 +1,6 @@
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
<f:advanced>
<f:entry field="site" title="${%Site}">
<f:textbox />
</f:entry>
<f:entry field="groupLookupStrategy" title="${%Group Membership Lookup Strategy}">
<f:select />
</f:entry>
Expand All @@ -27,6 +24,9 @@
<f:entry field="testDomainControllers" title="${%Test Domain Controllers}">
<f:textbox />
</f:entry>
<f:entry field="testSite" title="${%Test Site}">
<f:textbox />
</f:entry>
</f:advanced>
<f:nested>
<f:validateButton with="testDomain,testDomainControllers,domains,site,bindName,bindPassword" title="${%Test a test Domain}" method="validateTest"/>
Expand Down
0 ...tiveDirectorySecurityRealm/help-site.html → src/main/webapp/help/help-site.html
View file
File renamed without changes.
11 changes: 11 additions & 0 deletions src/test/java/hudson/plugins/active_directory/ActiveDirectorySecurityRealmTest.java
View file
Expand Up @@ -29,6 +29,8 @@ public void testReadResolveSingleDomain() throws Exception {
// JENKINS-39375 Support a different bindUser per domain
assertEquals("bindUser", activeDirectorySecurityRealm.getDomains().get(0).getBindName());
assertNotNull(activeDirectorySecurityRealm.getDomains().get(0).getBindPassword());
// JENKINS-39423 Make Site independent of each domain
assertEquals("site", activeDirectorySecurityRealm.getDomains().get(0).getSite());
}
}

Expand All @@ -46,6 +48,8 @@ public void testReadResolveSingleDomainSingleServer() throws Exception {
// JENKINS-39375 Support a different bindUser per domain
assertEquals("bindUser", activeDirectorySecurityRealm.getDomains().get(0).getBindName());
assertNotNull(activeDirectorySecurityRealm.getDomains().get(0).getBindPassword());
// JENKINS-39423 Make Site independent of each domain
assertEquals("site", activeDirectorySecurityRealm.getDomains().get(0).getSite());
}
}

Expand All @@ -63,6 +67,8 @@ public void testReadResolveSingleDomainWithTwoServers() throws Exception {
// JENKINS-39375 Support a different bindUser per domain
assertEquals("bindUser", activeDirectorySecurityRealm.getDomains().get(0).getBindName());
assertNotNull(activeDirectorySecurityRealm.getDomains().get(0).getBindPassword());
// JENKINS-39423 Make Site independent of each domain
assertEquals("site", activeDirectorySecurityRealm.getDomains().get(0).getSite());
}
}

Expand Down Expand Up @@ -206,6 +212,8 @@ public void testReadResolveMultiDomainSingleDomainOneDisplayName() throws Except
// JENKINS-39375 Support a different bindUser per domain
assertEquals("bindUser", activeDirectorySecurityRealm.getDomains().get(0).getBindName());
assertNotNull(activeDirectorySecurityRealm.getDomains().get(0).getBindPassword());
// JENKINS-39423 Make Site independent of each domain
assertEquals("site", activeDirectorySecurityRealm.getDomains().get(0).getSite());
}
}

Expand All @@ -227,6 +235,9 @@ public void testReadResolveMultiDomainTwoDomainsOneDisplayName() throws Exceptio
assertNotNull(activeDirectorySecurityRealm.getDomains().get(0).getBindPassword());
assertEquals("bindUser", activeDirectorySecurityRealm.getDomains().get(1).getBindName());
assertNotNull(activeDirectorySecurityRealm.getDomains().get(1).getBindPassword());
// JENKINS-39423 Make Site independent of each domain
assertEquals("site", activeDirectorySecurityRealm.getDomains().get(0).getSite());
assertEquals("site", activeDirectorySecurityRealm.getDomains().get(1).getSite());
}
}
}
Binary file modified ...ActiveDirectorySecurityRealmTest/testReadResolveMultiDomainSingleDomainOneDisplayName.zip
View file
Binary file not shown.
Binary file modified ...y/ActiveDirectorySecurityRealmTest/testReadResolveMultiDomainTwoDomainsOneDisplayName.zip
View file
Binary file not shown.
Binary file modified ...plugins/active_directory/ActiveDirectorySecurityRealmTest/testReadResolveSingleDomain.zip
View file
Binary file not shown.
Binary file modified ...ve_directory/ActiveDirectorySecurityRealmTest/testReadResolveSingleDomainSingleServer.zip
View file
Binary file not shown.
Binary file modified ..._directory/ActiveDirectorySecurityRealmTest/testReadResolveSingleDomainWithTwoServers.zip
View file
Binary file not shown.

0 comments on commit 8675b69

Please sign in to comment.