From f103b7819d45d1f06c8b2a8ba0bf1c8a1913014d Mon Sep 17 00:00:00 2001
From: Tim Jacomb <timjacomb1+github@gmail.com>
Date: Thu, 14 Nov 2019 12:40:53 +0000
Subject: [PATCH] JENKINS-56553 Make proxy config compatible with JCasC without
 workarounds on the plugin side (#3935)

* JENKINS-56553 Make proxy config compatible with JCASC

* Use DataBoundSetters as other fields are optional

* Fix Jelly for ProxyConfiguration

* Use default field name now that field matches

* Fix validate proxy

* Change field type

* After review

* Update core/src/main/java/jenkins/model/Jenkins.java

Co-Authored-By: timja <t.jacomb@kainos.com>

* Update core/src/main/java/jenkins/model/Jenkins.java

Co-Authored-By: timja <t.jacomb@kainos.com>

* Update core/src/main/java/hudson/ProxyConfiguration.java

Co-Authored-By: timja <t.jacomb@kainos.com>

* Update core/src/main/java/jenkins/model/Jenkins.java

Co-Authored-By: timja <t.jacomb@kainos.com>

* Update core/src/main/java/jenkins/model/Jenkins.java

Co-Authored-By: timja <t.jacomb@kainos.com>

* Update core/src/main/java/hudson/ProxyConfiguration.java

Co-Authored-By: Oleg Nenashev <o.v.nenashev@gmail.com>

* Tweak javadoc

* Align fields

* Update core/src/main/java/hudson/ProxyConfiguration.java

Co-Authored-By: Adrien Lecharpentier <adrien.lecharpentier@gmail.com>
---
 .../main/java/hudson/ProxyConfiguration.java  | 69 ++++++++++++++++---
 core/src/main/java/jenkins/model/Jenkins.java | 23 ++++++-
 .../hudson/ProxyConfiguration/config.groovy   |  4 +-
 3 files changed, 83 insertions(+), 13 deletions(-)

diff --git a/core/src/main/java/hudson/ProxyConfiguration.java b/core/src/main/java/hudson/ProxyConfiguration.java
index cecd2f6843..7eeb718b57 100644
--- a/core/src/main/java/hudson/ProxyConfiguration.java
+++ b/core/src/main/java/hudson/ProxyConfiguration.java
@@ -62,7 +62,10 @@ import org.apache.commons.httpclient.auth.AuthScope;
 import org.apache.commons.httpclient.methods.GetMethod;
 import org.jenkinsci.Symbol;
 import org.jvnet.robust_http_client.RetryableHttpStream;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.DataBoundSetter;
 import org.kohsuke.stapler.QueryParameter;
 import org.kohsuke.stapler.interceptor.RequirePOST;
 
@@ -94,14 +97,15 @@ public final class ProxyConfiguration extends AbstractDescribableImpl<ProxyConfi
     /**
      * Possibly null proxy user name.
      */
-    private final String userName;
+    private String userName;
 
     /**
      * List of host names that shouldn't use proxy, as typed by users.
      *
      * @see #getNoProxyHostPatterns()
      */
-    public final String noProxyHost;
+    @Restricted(NoExternalUse.class)
+    public String noProxyHost;
 
     @Deprecated
     private String password;
@@ -117,6 +121,7 @@ public final class ProxyConfiguration extends AbstractDescribableImpl<ProxyConfi
 
     private transient boolean authCacheSeeded;
 
+    @DataBoundConstructor
     public ProxyConfiguration(String name, int port) {
         this(name,port,null,null);
     }
@@ -129,12 +134,12 @@ public final class ProxyConfiguration extends AbstractDescribableImpl<ProxyConfi
         this(name,port,userName,password,noProxyHost,null);
     }
 
-    @DataBoundConstructor
     public ProxyConfiguration(String name, int port, String userName, String password, String noProxyHost, String testUrl) {
         this.name = Util.fixEmptyAndTrim(name);
         this.port = port;
         this.userName = Util.fixEmptyAndTrim(userName);
-        this.secretPassword = Secret.fromString(password);
+        String tempPassword = Util.fixEmptyAndTrim(password);
+        this.secretPassword = tempPassword != null ? Secret.fromString(tempPassword) : null;
         this.noProxyHost = Util.fixEmptyAndTrim(noProxyHost);
         this.testUrl = Util.fixEmptyAndTrim(testUrl);
         this.authenticator = newAuthenticator();
@@ -146,7 +151,7 @@ public final class ProxyConfiguration extends AbstractDescribableImpl<ProxyConfi
             public PasswordAuthentication getPasswordAuthentication() {
                 String userName = getUserName();
                 if (getRequestorType() == RequestorType.PROXY && userName != null) {
-                    return new PasswordAuthentication(userName, getPassword().toCharArray());
+                    return new PasswordAuthentication(userName, Secret.toString(secretPassword).toCharArray());
                 }
                 return null;
             }
@@ -157,15 +162,26 @@ public final class ProxyConfiguration extends AbstractDescribableImpl<ProxyConfi
         return userName;
     }
 
-//    This method is public, if it was public only for jelly, then should make it private (or inline contents)
-//    Have left public, as can't tell if anyone else is using from plugins
+    public Secret getSecretPassword() {
+        return secretPassword;
+    }
+
     /**
-     * @return the password in plain text
+     * @deprecated
+     *      Use {@link #getSecretPassword()}
      */
+    @Deprecated
     public String getPassword() {
         return Secret.toString(secretPassword);
     }
 
+    /**
+     * @deprecated
+     *      Use {@link #getSecretPassword()}
+     *
+     * @return the encrypted proxy password
+     */
+    @Deprecated
     public String getEncryptedPassword() {
         return (secretPassword == null) ? null : secretPassword.getEncryptedValue();
     }
@@ -174,6 +190,14 @@ public final class ProxyConfiguration extends AbstractDescribableImpl<ProxyConfi
         return testUrl;
     }
 
+    public int getPort() {
+        return port;
+    }
+
+    public String getName() {
+        return name;
+    }
+
     /**
      * Returns the list of properly formatted no proxy host names.
      */
@@ -181,6 +205,10 @@ public final class ProxyConfiguration extends AbstractDescribableImpl<ProxyConfi
         return getNoProxyHostPatterns(noProxyHost);
     }
 
+    public String getNoProxyHost() {
+        return noProxyHost;
+    }
+
     /**
      * Returns the list of properly formatted no proxy host names.
      */
@@ -195,6 +223,26 @@ public final class ProxyConfiguration extends AbstractDescribableImpl<ProxyConfi
         return r;
     }
 
+    @DataBoundSetter
+    public void setSecretPassword(Secret secretPassword) {
+        this.secretPassword = secretPassword;
+    }
+
+    @DataBoundSetter
+    public void setTestUrl(String testUrl) {
+        this.testUrl = testUrl;
+    }
+
+    @DataBoundSetter
+    public void setUserName(String userName) {
+        this.userName = userName;
+    }
+
+    @DataBoundSetter
+    public void setNoProxyHost(String noProxyHost) {
+        this.noProxyHost = noProxyHost;
+    }
+
     /**
      * @deprecated
      *      Use {@link #createProxy(String)}
@@ -374,9 +422,10 @@ public final class ProxyConfiguration extends AbstractDescribableImpl<ProxyConfi
         }
 
         @RequirePOST
+        @Restricted(NoExternalUse.class)
         public FormValidation doValidateProxy(
                 @QueryParameter("testUrl") String testUrl, @QueryParameter("name") String name, @QueryParameter("port") int port,
-                @QueryParameter("userName") String userName, @QueryParameter("password") String password,
+                @QueryParameter("userName") String userName, @QueryParameter("secretPassword") Secret password,
                 @QueryParameter("noProxyHost") String noProxyHost) {
 
             Jenkins.get().checkPermission(Jenkins.ADMINISTER);
@@ -401,7 +450,7 @@ public final class ProxyConfiguration extends AbstractDescribableImpl<ProxyConfi
                 HttpClient client = new HttpClient();
                 if (Util.fixEmptyAndTrim(name) != null && !isNoProxyHost(host, noProxyHost)) {
                     client.getHostConfiguration().setProxy(name, port);
-                    Credentials credentials = createCredentials(userName, password);
+                    Credentials credentials = createCredentials(userName, password != null ? password.getPlainText() : null);
                     AuthScope scope = new AuthScope(AuthScope.ANY_HOST, AuthScope.ANY_PORT);
                     client.getState().setProxyCredentials(scope, credentials);
                 }
diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java
index 765e80a8e6..45878b2720 100644
--- a/core/src/main/java/jenkins/model/Jenkins.java
+++ b/core/src/main/java/jenkins/model/Jenkins.java
@@ -1065,7 +1065,28 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve
         agentProtocols = null;
         return this;
     }
-    
+
+    /**
+     * Retrieve the proxy configuration.
+     *
+     * @return the proxy configuration
+     * @since TODO
+     */
+    @CheckForNull
+    public ProxyConfiguration getProxy() {
+        return proxy;
+    }
+
+    /**
+     * Set the proxy configuration.
+     * 
+     * @param proxy the proxy to set
+     * @since TODO
+     */
+    public void setProxy(@CheckForNull ProxyConfiguration proxy) {
+        this.proxy = proxy;
+    }
+
     /**
      * Get the Jenkins {@link jenkins.install.InstallState install state}.
      * @return The Jenkins {@link jenkins.install.InstallState install state}.
diff --git a/core/src/main/resources/hudson/ProxyConfiguration/config.groovy b/core/src/main/resources/hudson/ProxyConfiguration/config.groovy
index 8ecce1ee51..0be2a539b6 100644
--- a/core/src/main/resources/hudson/ProxyConfiguration/config.groovy
+++ b/core/src/main/resources/hudson/ProxyConfiguration/config.groovy
@@ -11,8 +11,8 @@ f.entry(title:_("Port"),field:"port") {
 f.entry(title:_("User name"),field:"userName") {
     f.textbox()
 }
-f.entry(title:_("Password"),field:"password") {
-    f.password(value:instance?.encryptedPassword)
+f.entry(title:_("Password"),field:"secretPassword") {
+    f.password()
 }
 f.entry(title:_("No Proxy Host"),field:"noProxyHost") {
     f.textarea()