Merge pull request #64 from fbelzunc/JENKINS-42641

[FIXED JENKINS-42641] Configure startTls on the UI
jenkinsci · Mar 10, 2017 · 943701d · 943701d
2 parents 9dc0b0f + 94f88fa
commit 943701d
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 3 deletions.
diff --git a/src/main/java/hudson/plugins/active_directory/ActiveDirectorySecurityRealm.java b/src/main/java/hudson/plugins/active_directory/ActiveDirectorySecurityRealm.java
@@ -43,6 +43,7 @@
 import hudson.util.ListBoxModel;
 import hudson.util.Secret;
 import hudson.util.spring.BeanBuilder;
+import jenkins.model.Jenkins;
 import org.acegisecurity.Authentication;
 import org.acegisecurity.AuthenticationException;
 import org.acegisecurity.AuthenticationManager;
@@ -172,6 +173,12 @@ public class ActiveDirectorySecurityRealm extends AbstractPasswordBasedSecurityR
      */
     public transient Secret bindPassword;
 
+    /**
+     * If true enable startTls in case plain communication is used. In case the plugin
+     * is configured to use TLS then this option will not have any impact.
+     */
+    public Boolean startTls;
+
     private GroupLookupStrategy groupLookupStrategy;
 
     /**
@@ -213,13 +220,13 @@ 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, server)), site, bindName, bindPassword, server, groupLookupStrategy, removeIrrelevantGroups, domain!=null, cache);
+        this(domain, Lists.newArrayList(new ActiveDirectoryDomain(domain, server)), site, bindName, bindPassword, server, groupLookupStrategy, removeIrrelevantGroups, domain!=null, cache, true);
     }
 
     @DataBoundConstructor
     // as Java signature, this binding doesn't make sense, so please don't use this constructor
     public ActiveDirectorySecurityRealm(String domain, List<ActiveDirectoryDomain> domains, String site, String bindName,
-                                        String bindPassword, String server, GroupLookupStrategy groupLookupStrategy, boolean removeIrrelevantGroups, Boolean customDomain, CacheConfiguration cache) {
+                                        String bindPassword, String server, GroupLookupStrategy groupLookupStrategy, boolean removeIrrelevantGroups, Boolean customDomain, CacheConfiguration cache, Boolean startTls) {
         if (customDomain!=null && !customDomain)
             domains = null;
         this.domain = fixEmpty(domain);
@@ -231,6 +238,7 @@ public ActiveDirectorySecurityRealm(String domain, List<ActiveDirectoryDomain> d
         this.groupLookupStrategy = groupLookupStrategy;
         this.removeIrrelevantGroups = removeIrrelevantGroups;
         this.cache = cache;
+        this.startTls = startTls;
     }
 
     @DataBoundSetter
@@ -245,6 +253,10 @@ public CacheConfiguration getCache() {
         }
         return cache;
     }
+    @Restricted(NoExternalUse.class)
+    public Boolean isStartTls() {
+        return startTls;
+    }
 
     public Integer getSize() {
         return cache == null ? null : cache.getSize();
@@ -326,6 +338,10 @@ public Object readResolve() throws ObjectStreamException {
                 activeDirectoryDomain.site = site;
             }
         }
+        if (startTls == null) {
+            this.startTls = true;
+        }
+
         return this;
     }
 
@@ -552,7 +568,14 @@ private LdapContext bind(String principalName, String password, SocketInfo serve
 
                 LdapContext context = (LdapContext)LdapCtxFactory.getLdapCtxInstance(ldapUrl, props);
 
-                if (!FORCE_LDAPS) {
+                boolean isStartTls = true;
+                SecurityRealm securityRealm = Jenkins.getInstance().getSecurityRealm();
+                if (securityRealm instanceof ActiveDirectorySecurityRealm) {
+                    ActiveDirectorySecurityRealm activeDirectorySecurityRealm = (ActiveDirectorySecurityRealm) securityRealm;
+                     isStartTls= activeDirectorySecurityRealm.isStartTls();
+                }
+
+                if (!FORCE_LDAPS && isStartTls) {
                     // try to upgrade to TLS if we can, but failing to do so isn't fatal
                     // see http://download.oracle.com/javase/jndi/tutorial/ldap/ext/starttls.html
                     try {

diff --git a/...sources/hudson/plugins/active_directory/ActiveDirectorySecurityRealm/configAdvanced.jelly b/...sources/hudson/plugins/active_directory/ActiveDirectorySecurityRealm/configAdvanced.jelly
@@ -1,6 +1,9 @@
 <?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="startTls" title="${%Enable StartTls}">
+      <f:checkbox checked="true" />
+    </f:entry>
     <f:entry field="groupLookupStrategy" title="${%Group Membership Lookup Strategy}">
       <f:select />
     </f:entry>

diff --git a/...resources/hudson/plugins/active_directory/ActiveDirectorySecurityRealm/help-startTls.html b/...resources/hudson/plugins/active_directory/ActiveDirectorySecurityRealm/help-startTls.html
@@ -0,0 +1,5 @@
+<div>
+    This property allows you to enable/disable StartTLS. In case the Active Directory plugin
+    is set-up to use TLS, then StartTLS will not try to start.
+    StartTLS will only tries to start in case the communication is started on plain.
+</div>