[FIXED JENKINS-39423] Make site independent of each domain (#54)

[JENKINS-39423] Make site independent of each domain

pom.xml

@@ -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>
 

src/main/java/hudson/plugins/active_directory/ActiveDirectoryDomain.java

@@ -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,
@@ -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);
@@ -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));
     }
@@ -117,6 +128,11 @@ public Secret getBindPassword() {
         return bindPassword;
     }
 
+    @Restricted(NoExternalUse.class)
+    public String getSite() {
+        return site;
+    }
+
     /**
      * Convert empty string to null.
      */

src/main/java/hudson/plugins/active_directory/ActiveDirectorySecurityRealm.java

@@ -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
@@ -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);
     }
@@ -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>();
@@ -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;
     }
 
@@ -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) {
@@ -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());
@@ -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);
                 }
@@ -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.");

src/main/java/hudson/plugins/active_directory/ActiveDirectoryUnixAuthenticationProvider.java

@@ -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);

src/main/resources/hudson/plugins/active_directory/ActiveDirectorySecurityRealm/config.jelly

@@ -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>
@@ -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>

src/main/resources/hudson/plugins/active_directory/ActiveDirectorySecurityRealm/configAdvanced.jelly

@@ -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>
@@ -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"/>

src/test/java/hudson/plugins/active_directory/ActiveDirectorySecurityRealmTest.java

@@ -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());
         }
     }
 
@@ -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());
         }
     }
 
@@ -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());
         }
     }
 
@@ -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());
         }
     }
 
@@ -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());
         }
     }
 }