Skip to content
Permalink
Browse files
[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.
  • Loading branch information
ndeloof committed Sep 9, 2014
1 parent 9298264 commit bd98b91ea614c307a6bb1e0af36d9dd2a5646e29
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 7 deletions.
@@ -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() {
@@ -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>
@@ -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});

4 comments on commit bd98b91

@jglick
Copy link
Member

@jglick jglick commented on bd98b91 Nov 20, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@olivierdagenais this is a critical fix. Do you plan to do a new plugin release soon?

@apemberton
Copy link

@apemberton apemberton commented on bd98b91 Nov 20, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@olivierdagenais
Copy link
Member

@olivierdagenais olivierdagenais commented on bd98b91 Nov 29, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'm installing TFS so I can verify everything still works before I release.

@jglick
Copy link
Member

@jglick jglick commented on bd98b91 Dec 16, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, thanks for update.

Please sign in to comment.