[FIXED JENKINS-23033] don’t use String but Secret to store password

UI binding will use a cipher and avoid it to be exposed as plain text in HTML form.
jenkinsci · Sep 9, 2014 · bd98b91 · bd98b91 · jglick · Nov 20, 2014
1 parent 9298264
commit bd98b91
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 7 deletions.
diff --git a/src/main/java/hudson/plugins/tfs/TeamFoundationServerScm.java b/src/main/java/hudson/plugins/tfs/TeamFoundationServerScm.java
@@ -12,6 +12,7 @@
 import java.util.logging.Logger;
 import java.util.regex.Pattern;
 
+import hudson.util.Secret;
 import net.sf.json.JSONObject;
 
 import org.apache.commons.lang.StringUtils;
@@ -75,7 +76,8 @@ public class TeamFoundationServerScm extends SCM {
     private final String projectPath;
     private final String localPath;
     private final String workspaceName;
-    private final String userPassword;
+    private transient @Deprecated String userPassword;
+    private /* almost final */ Secret password;
     private final String userName;
     private final boolean useUpdate;
 
@@ -84,17 +86,29 @@ public class TeamFoundationServerScm extends SCM {
     private transient String normalizedWorkspaceName;
     private transient String workspaceChangesetVersion;
 
-    private static final Logger logger = Logger.getLogger(TeamFoundationServerScm.class.getName()); 
+    private static final Logger logger = Logger.getLogger(TeamFoundationServerScm.class.getName());
+
+    @Deprecated
+    public TeamFoundationServerScm(String serverUrl, String projectPath, String localPath, boolean useUpdate, String workspaceName, String userName, String password) {
+        this(serverUrl, projectPath, localPath, useUpdate, workspaceName, userName, Secret.fromString(password));
+    }
 
     @DataBoundConstructor
-    public TeamFoundationServerScm(String serverUrl, String projectPath, String localPath, boolean useUpdate, String workspaceName, String userName, String userPassword) {
+    public TeamFoundationServerScm(String serverUrl, String projectPath, String localPath, boolean useUpdate, String workspaceName, String userName, Secret password) {
         this.serverUrl = serverUrl;
         this.projectPath = projectPath;
         this.useUpdate = useUpdate;
         this.localPath = (Util.fixEmptyAndTrim(localPath) == null ? "." : localPath);
         this.workspaceName = (Util.fixEmptyAndTrim(workspaceName) == null ? "Hudson-${JOB_NAME}-${NODE_NAME}" : workspaceName);
         this.userName = userName;
-        this.userPassword = Scrambler.scramble(userPassword);
+        this.password = password;
+    }
+
+    /* Migrate legacy data */
+    private Object readResolve() {
+        if (password == null && userPassword != null)
+            password = Secret.fromString(Scrambler.scramble(userPassword));
+        return this;
     }
 
     // Bean properties need for job configuration
@@ -119,7 +133,11 @@ public boolean isUseUpdate() {
     }
 
     public String getUserPassword() {
-        return Scrambler.descramble(userPassword);
+        return Secret.toString(password);
+    }
+
+    public Secret getPassword() {
+        return password;
     }
 
     public String getUserName() {

diff --git a/src/main/resources/hudson/plugins/tfs/TeamFoundationServerScm/config.jelly b/src/main/resources/hudson/plugins/tfs/TeamFoundationServerScm/config.jelly
@@ -16,7 +16,7 @@
     </f:entry>
 
     <f:entry title="User password">
-        <input type="password" class="setting-input" name="tfs.userPassword" value="${scm.userPassword}"/>
+        <input type="password" class="setting-input" name="tfs.password" value="${scm.password}"/>
     </f:entry>
 
     <f:advanced>

diff --git a/src/test/java/hudson/plugins/tfs/browsers/TeamSystemWebAccessBrowserTest.java b/src/test/java/hudson/plugins/tfs/browsers/TeamSystemWebAccessBrowserTest.java
@@ -11,6 +11,7 @@
 
 import java.net.URL;
 
+import hudson.util.Secret;
 import org.junit.Test;
 import org.jvnet.hudson.test.Bug;
 
@@ -49,7 +50,7 @@ public void assertChangeSetLinkWithOnlyServerUrlWithTrailingSlash() throws Excep
         AbstractBuild build = mock(AbstractBuild.class);
         AbstractProject<?,?> project = mock(AbstractProject.class);
         when(build.getProject()).thenReturn(project);
-        when(project.getScm()).thenReturn(new TeamFoundationServerScm("http://server:80", null, null, false, null, null, null));
+        when(project.getScm()).thenReturn(new TeamFoundationServerScm("http://server:80", null, null, false, null, null, (Secret) null));
 
         ChangeSet changeset = new ChangeSet("62643", null, "user", "comment");
         new ChangeLogSet(build, new ChangeSet[]{ changeset});