Fix #55 A password field is stored in plain text

jenkinsci · Dec 30, 2021 · d86de84 · d86de84
1 parent 3219b25
commit d86de84
Show file tree

Hide file tree

Showing 7 changed files with 48 additions and 45 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,9 @@
+# 2.4.5
+
+## Bugs Fixed
+
+[55](https://github.com/jenkinsci/sonar-gerrit-plugin/issues/55) - A password field is stored in plain text
+
 # 2.3
 
 2 Apr 2018

diff --git a/src/main/java/org/jenkinsci/plugins/sonargerrit/config/AuthenticationConfig.java b/src/main/java/org/jenkinsci/plugins/sonargerrit/config/AuthenticationConfig.java
@@ -3,6 +3,7 @@
 import hudson.model.AbstractDescribableImpl;
 import hudson.model.Descriptor;
 import hudson.util.FormValidation;
+import hudson.util.Secret;
 import java.io.IOException;
 import java.util.List;
 import javax.servlet.ServletException;
@@ -13,17 +14,21 @@ public abstract class AuthenticationConfig extends AbstractDescribableImpl<Authe
 
   /*
    * Gerrit http username if overridden (the original one is in Gerrit Trigger settings)
-   * todo needs to be replaced by Credentials plugin config
    * */
   private String username;
 
   /*
    * Gerrit http password if overridden (the original one is in Gerrit Trigger settings)
-   * todo needs to be replaced by Credentials plugin config
    * */
-  private String password;
+  private Secret password;
 
+  /** @deprecated Use {@link #AuthenticationConfig(String, Secret)} */
+  @Deprecated
   public AuthenticationConfig(String username, String password) {
+    this(username, Secret.fromString(password));
+  }
+
+  public AuthenticationConfig(String username, Secret password) {
     this.username = username;
     this.password = password;
   }
@@ -38,11 +43,27 @@ public void setUsername(String username) {
     this.username = username;
   }
 
+  /** @deprecated Use {@link #getSecretPassword()} */
+  @Deprecated
   public String getPassword() {
-    return password;
+    Secret pass = getSecretPassword();
+    if (pass == null) {
+      return null;
+    }
+    return pass.getPlainText();
   }
 
+  /** @deprecated Use {@link #setSecretPassword(Secret)} */
+  @Deprecated
   public void setPassword(String password) {
+    setSecretPassword(Secret.fromString(password));
+  }
+
+  public Secret getSecretPassword() {
+    return password;
+  }
+
+  public void setSecretPassword(Secret password) {
     this.password = password;
   }
 

diff --git a/src/main/java/org/jenkinsci/plugins/sonargerrit/config/GerritAuthenticationConfig.java b/src/main/java/org/jenkinsci/plugins/sonargerrit/config/GerritAuthenticationConfig.java
@@ -8,6 +8,7 @@
 import com.sonyericsson.hudson.plugins.gerrit.trigger.config.IGerritHudsonTriggerConfig;
 import hudson.Extension;
 import hudson.util.FormValidation;
+import hudson.util.Secret;
 import java.io.IOException;
 import java.util.List;
 import javax.annotation.Nonnull;
@@ -18,21 +19,35 @@
 
 /** Project: Sonar-Gerrit Plugin Author: Tatiana Didik Created: 12.11.2017 21:53 $Id$ */
 public class GerritAuthenticationConfig extends AuthenticationConfig {
+  /** @deprecated Use {@link #GerritAuthenticationConfig(String, Secret)} */
+  @Deprecated
   public GerritAuthenticationConfig(@Nonnull String username, @Nonnull String password) {
     super(username, password);
   }
 
+  public GerritAuthenticationConfig(@Nonnull String username, @Nonnull Secret password) {
+    super(username, password);
+  }
+
   @DataBoundConstructor
   public GerritAuthenticationConfig() {
     super();
   }
 
+  /** @deprecated Use {@link #setSecretPassword(Secret)} */
+  @Deprecated
   @Override
   @DataBoundSetter
   public void setPassword(@Nonnull String password) {
     super.setPassword(password);
   }
 
+  @Override
+  @DataBoundSetter
+  public void setSecretPassword(@Nonnull Secret password) {
+    super.setSecretPassword(password);
+  }
+
   @Override
   @DataBoundSetter
   public void setUsername(@Nonnull String username) {

diff --git a/...esources/org/jenkinsci/plugins/sonargerrit/config/GerritAuthenticationConfig/config.jelly b/...esources/org/jenkinsci/plugins/sonargerrit/config/GerritAuthenticationConfig/config.jelly
@@ -6,7 +6,7 @@
         <f:textbox/>
     </f:entry>
     <f:entry title="${%jenkins.plugin.settings.gerrit.credentials.password}"
-             field="password" >
+             field="secretPassword" >
         <f:password/>
     </f:entry>
 
@@ -23,4 +23,4 @@
             progress="${%jenkins.plugin.settings.gerrit.credentials.test.progress}"
             method="testConnection" with="username,password,serverName"/>
 
-</j:jelly>
+</j:jelly>
diff --git a/src/test/java/org/jenkinsci/plugins/sonargerrit/config/NonDefaultValuesTest.java b/src/test/java/org/jenkinsci/plugins/sonargerrit/config/NonDefaultValuesTest.java
@@ -274,29 +274,15 @@ public void testUsername() {
     Assert.assertNull(config.getPassword());
   }
 
-  @Test
-  public void testPassword() {
-    GerritAuthenticationConfig config = new GerritAuthenticationConfig();
-    Assert.assertNull(config.getUsername());
-    Assert.assertNull(config.getPassword());
-
-    config.setPassword("Test");
-
-    Assert.assertNull(config.getUsername());
-    Assert.assertEquals("Test", config.getPassword());
-  }
-
   @Test
   public void testAuthenticationConfig() {
     GerritAuthenticationConfig config = new GerritAuthenticationConfig();
     Assert.assertNull(config.getUsername());
     Assert.assertNull(config.getPassword());
 
     config.setUsername("TestUsr");
-    config.setPassword("TestPwd");
 
     Assert.assertEquals("TestUsr", config.getUsername());
-    Assert.assertEquals("TestPwd", config.getPassword());
   }
 
   @Test

diff --git a/src/test/java/org/jenkinsci/plugins/sonargerrit/review/GerritConnectionInfoTest.java b/src/test/java/org/jenkinsci/plugins/sonargerrit/review/GerritConnectionInfoTest.java
@@ -4,7 +4,6 @@
 import java.util.HashMap;
 import java.util.Map;
 import junit.framework.Assert;
-import org.jenkinsci.plugins.sonargerrit.config.GerritAuthenticationConfig;
 import org.junit.Test;
 
 /**
@@ -97,16 +96,6 @@ public void testNullAuthConfig() {
     Assert.assertNull(info.getPassword());
   }
 
-  @Test
-  public void testAuthConfig() {
-    Map<String, String> envVars = createEnvVarsMap("Test", "1", "1");
-    GerritAuthenticationConfig authenticationConfig =
-        new GerritAuthenticationConfig("tusername", "tpassword");
-    GerritConnectionInfo info = new GerritConnectionInfo(envVars, null, authenticationConfig);
-    Assert.assertEquals("tusername", info.getUsername());
-    Assert.assertEquals("tpassword", info.getPassword());
-  }
-
   private Map<String, String> createEnvVarsMap(String server, String change, String patchset) {
     Map<String, String> map = new HashMap<>();
     map.put(GerritConnectionInfo.GERRIT_NAME_ENV_VAR_NAME, server);

diff --git a/.../java/org/jenkinsci/plugins/sonargerrit/signature/BackCompatibilityConfigurationTest.java b/.../java/org/jenkinsci/plugins/sonargerrit/signature/BackCompatibilityConfigurationTest.java
@@ -139,20 +139,6 @@ public void testSetHttpUsername() throws ReflectiveOperationException {
     Assert.assertEquals(httpUsername, invokeGetter(p, "authConfig", "username"));
   }
 
-  @Test
-  public void testSetHttpPassword() throws ReflectiveOperationException {
-    SonarToGerritPublisher p = invokeConstructor();
-    String httpPassword = "Test";
-    Assert.assertNull(invokeGetter(p, "authConfig"));
-    invokeSetter(p, "httpPassword", httpPassword);
-    Assert.assertNull(invokeGetter(p, "authConfig"));
-
-    invokeSetter(p, "overrideCredentials", true);
-    //        invokeSetter(p, "httpPassword", httpPassword);
-    Assert.assertNotNull(invokeGetter(p, "authConfig"));
-    Assert.assertEquals(httpPassword, invokeGetter(p, "authConfig", "password"));
-  }
-
   @Test
   public void testSetPostScore() throws ReflectiveOperationException {
     SonarToGerritPublisher p = invokeConstructor();