Prevent to send plaintext password to browser

Fix for JENKINS-23165 and pull #157
jenkinsci · May 28, 2014 · d402536 · d402536
1 parent 351b4fb
commit d402536
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 31 deletions.
diff --git a/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/config/Config.java b/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/config/Config.java
@@ -477,11 +477,7 @@ public void setGerritAuthKeyFile(File gerritAuthKeyFile) {
 
     @Override
     public String getGerritAuthKeyFilePassword() {
-        if (gerritAuthKeyFilePassword == null) {
-            return "";
-        } else {
-            return gerritAuthKeyFilePassword.getPlainText();
-        }
+        return Secret.toString(gerritAuthKeyFilePassword);
     }
 
     /**
@@ -494,6 +490,11 @@ public void setGerritAuthKeyFilePassword(String gerritAuthKeyFilePassword) {
         this.gerritAuthKeyFilePassword = Secret.fromString(gerritAuthKeyFilePassword);
     }
 
+    @Override
+    public Secret getGerritAuthKeyFileSecretPassword() {
+        return gerritAuthKeyFilePassword;
+    }
+
     /**
      * GerritBuildCurrentPatchesOnly.
      *
@@ -878,14 +879,7 @@ public void setEnableManualTrigger(boolean enableManualTrigger) {
 
     @Override
     public Authentication getGerritAuthentication() {
-        Authentication authentication;
-        if (gerritAuthKeyFilePassword == null) {
-            authentication = new Authentication(gerritAuthKeyFile, gerritUserName, "");
-        } else {
-            authentication = new Authentication(
-                    gerritAuthKeyFile, gerritUserName, gerritAuthKeyFilePassword.getPlainText());
-        }
-        return authentication;
+        return new Authentication(gerritAuthKeyFile, gerritUserName, Secret.toString(gerritAuthKeyFilePassword));
     }
 
     @Override
@@ -944,11 +938,7 @@ public void setUseRestApi(boolean useRestApi) {
 
     @Override
     public String getGerritHttpPassword() {
-        if (gerritHttpPassword == null) {
-            return "";
-        } else {
-            return gerritHttpPassword.getPlainText();
-        }
+        return Secret.toString(gerritHttpPassword);
     }
 
     /**
@@ -978,11 +968,7 @@ public void setGerritHttpUserName(String gerritHttpUserName) {
 
     @Override
     public Credentials getHttpCredentials() {
-        if (gerritHttpPassword == null) {
-            return new UsernamePasswordCredentials(gerritHttpUserName, "");
-        } else {
-            return new UsernamePasswordCredentials(gerritHttpUserName, gerritHttpPassword.getPlainText());
-        }
+        return new UsernamePasswordCredentials(gerritHttpUserName, Secret.toString(gerritHttpPassword));
     }
 
 }
diff --git a/...ava/com/sonyericsson/hudson/plugins/gerrit/trigger/config/IGerritHudsonTriggerConfig.java b/...ava/com/sonyericsson/hudson/plugins/gerrit/trigger/config/IGerritHudsonTriggerConfig.java
@@ -30,6 +30,7 @@
 import com.sonyericsson.hudson.plugins.gerrit.trigger.VerdictCategory;
 
 import net.sf.json.JSONObject;
+import hudson.util.Secret;
 
 import java.util.List;
 
@@ -262,4 +263,11 @@ public interface IGerritHudsonTriggerConfig extends GerritConnectionConfig2 {
      * @return the notification level value
      */
      Notify getNotificationLevel();
+
+     /**
+      * The instance of {@link Secret} which has a password for the private key, or null if there is none.
+      *
+      * @return the instance of {@link Secret}.
+      */
+     Secret getGerritAuthKeyFileSecretPassword();
 }
diff --git a/src/main/resources/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer/index.jelly b/src/main/resources/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer/index.jelly
@@ -74,7 +74,7 @@
                     <f:entry title="${%SSH Keyfile Password}"
                              help="/plugin/gerrit-trigger/help-GerritKeyFilePassword.html">
                         <f:password name="gerritAuthKeyFilePassword"
-                                    value="${it.config.gerritAuthKeyFilePassword}"
+                                    value="${it.config.gerritAuthKeyFileSecretPassword}"
                                     default="${com.sonyericsson.hudson.plugins.gerrit.gerritevents.GerritDefaultValues.DEFAULT_GERRIT_AUTH_KEY_FILE_PASSWORD}"/>
                     </f:entry>
                     <f:entry title="${%Build Current Patches Only}"
@@ -233,7 +233,7 @@
                         <f:entry title="${%Notification Level}"
                                  field="notificationLevel"
                                  help="/plugin/gerrit-trigger/help-GerritNotificationLevel.html">
-                            <f:select value="${it.config.notificationLevel}" 
+                            <f:select value="${it.config.notificationLevel}"
                                       default="${com.sonyericsson.hudson.plugins.gerrit.trigger.config.Config.DEFAULT_NOTIFICATION_LEVEL}" />
                         </f:entry>
                         <f:entry title="${%Verdict Categories}">

diff --git a/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/config/ConfigTest.java b/src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/config/ConfigTest.java
@@ -261,11 +261,42 @@ public void testGetGerritFrontEndUrlForChangeBasedEventProvider() {
      * With a encrypted string as password.
      */
     @Test
-    public void testGetGerritAuthKeyFilePasswordFromEncryptedString() {
-        Secret pass = Secret.fromString("gerritpass");
-        String formString = "{\"gerritAuthKeyFilePassword\":\"" + pass.getEncryptedValue() + "\"}";
-        JSONObject form = (JSONObject)JSONSerializer.toJSON(formString);
-        Config config = new Config(form);
-        assertEquals("gerritpass", config.getGerritAuthKeyFilePassword());
+    public void testGetGerritAuthKeyFilePassword() {
+        String formString;
+        JSONObject form;
+        Config config;
+
+        // plain text
+        formString = "{\"gerritAuthKeyFilePassword\":\"plainpass\"}";
+        form = (JSONObject)JSONSerializer.toJSON(formString);
+        config = new Config(form);
+        assertEquals("plainpass", config.getGerritAuthKeyFilePassword());
+
+        // encrypted string
+        Secret pass = Secret.fromString("encryptpass");
+        formString = "{\"gerritAuthKeyFilePassword\":\"" + pass.getEncryptedValue() + "\"}";
+        form = (JSONObject)JSONSerializer.toJSON(formString);
+        config = new Config(form);
+        assertEquals("encryptpass", config.getGerritAuthKeyFilePassword());
+
+        // empty
+        formString = "{\"gerritAuthKeyFilePassword\":\"\"}";
+        form = (JSONObject)JSONSerializer.toJSON(formString);
+        config = new Config(form);
+        assertEquals("Empty check", "", config.getGerritAuthKeyFilePassword());
+
+        // null
+        config = new Config();
+        assertEquals("Null check", "", config.getGerritAuthKeyFilePassword());
+    }
+
+    /**
+     * Tests {@link Config#getGerritAuthKeyFileSecretPassword()}.
+     */
+    @Test
+    public void testGetGerritAuthkeyFileSecretPassword() {
+        Config config = new Config();
+        config.setGerritAuthKeyFilePassword("secretpass");
+        assertEquals(Secret.fromString("secretpass"), config.getGerritAuthKeyFileSecretPassword());
     }
 }
diff --git a/...va/com/sonyericsson/hudson/plugins/gerrit/trigger/mock/MockGerritHudsonTriggerConfig.java b/...va/com/sonyericsson/hudson/plugins/gerrit/trigger/mock/MockGerritHudsonTriggerConfig.java
@@ -36,6 +36,8 @@
 
 import org.apache.http.auth.Credentials;
 
+import hudson.util.Secret;
+
 import java.io.File;
 import java.util.LinkedList;
 import java.util.List;
@@ -108,6 +110,11 @@ public String getGerritAuthKeyFilePassword() {
         throw new UnsupportedOperationException("Not supported yet.");
     }
 
+    @Override
+    public Secret getGerritAuthKeyFileSecretPassword() {
+        throw new UnsupportedOperationException("Not supported yet.");
+    }
+
     @Override
     public String getGerritFrontEndUrl() {
         return "http://gerrit/";