Skip to content

Commit dc78601

Browse files
varshavaradarajanvarshavaradarajan
committed
Added validation of the encryptedPassword attribute of password aware materials - svn, p4, tfs.
1 parent 6643ba7 commit dc78601

File tree

7 files changed

+111
-11
lines changed

7 files changed

+111
-11
lines changed

‎common/test/unit/com/thoughtworks/go/config/materials/tfs/TfsMaterialConfigUpdateTest.java‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2015 ThoughtWorks, Inc.
2+
* Copyright 2016 ThoughtWorks, Inc.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -144,6 +144,21 @@ public void validate_shouldEnsureDestFilePathIsValid() {
144144
assertThat(tfsMaterialConfig.errors().on(TfsMaterialConfig.FOLDER), is("Dest folder '../a' is not valid. It must be a sub-directory of the working folder."));
145145
}
146146

147+
@Test
148+
public void shouldThrowErrorsIfBothPasswordAndEncryptedPasswordAreProvided() {
149+
TfsMaterialConfig materialConfig = new TfsMaterialConfig(new UrlArgument("foo/bar"), "password", "encryptedPassword", new GoCipher());
150+
materialConfig.validate(new ConfigSaveValidationContext(null));
151+
assertThat(materialConfig.errors().on("password"), is("You may only specify `password` or `encrypted_password`, not both!"));
152+
assertThat(materialConfig.errors().on("encryptedPassword"), is("You may only specify `password` or `encrypted_password`, not both!"));
153+
}
154+
155+
@Test
156+
public void shouldValidateWhetherTheEncryptedPasswordIsCorrect() {
157+
TfsMaterialConfig materialConfig = new TfsMaterialConfig(new UrlArgument("foo/bar"), "", "encryptedPassword", new GoCipher());
158+
materialConfig.validate(new ConfigSaveValidationContext(null));
159+
assertThat(materialConfig.errors().on("encryptedPassword"), is("Encrypted password value for TFS material with url 'foo/bar' is invalid. This usually happens when the cipher text is modified to have an invalid value."));
160+
}
161+
147162
@Test
148163
public void shouldEncryptTfsPasswordAndMarkPasswordAsNull() throws Exception {
149164
GoCipher mockGoCipher = mock(GoCipher.class);

‎config/config-api/src/com/thoughtworks/go/config/materials/perforce/P4MaterialConfig.java‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232

3333
import static com.thoughtworks.go.util.ExceptionUtils.bomb;
3434
import static com.thoughtworks.go.util.ExceptionUtils.bombIfNull;
35+
import static java.lang.String.format;
3536
import static org.apache.commons.lang.StringUtils.isNotEmpty;
3637

3738
@ConfigTag(value = "p4", label = "Perforce")
@@ -98,6 +99,14 @@ public P4MaterialConfig(String serverAndPort, String userName, String password,
9899
setView(viewStr);
99100
}
100101

102+
//for tests only
103+
protected P4MaterialConfig(String serverAndPort, String password, String encryptedPassword, GoCipher goCipher) {
104+
this(goCipher);
105+
this.password = password;
106+
this.encryptedPassword = encryptedPassword;
107+
this.serverAndPort = serverAndPort;
108+
}
109+
101110
public GoCipher getGoCipher() {
102111
return goCipher;
103112
}
@@ -238,6 +247,14 @@ public void validateConcreteScmMaterial() {
238247
addError("password", "You may only specify `password` or `encrypted_password`, not both!");
239248
addError("encryptedPassword", "You may only specify `password` or `encrypted_password`, not both!");
240249
}
250+
if(isNotEmpty(this.encryptedPassword)) {
251+
try{
252+
currentPassword();
253+
}catch (Exception e) {
254+
addError("encryptedPassword", format("Encrypted password value for P4 material with serverAndPort '%s' is invalid. This usually happens when the cipher text is modified to have an invalid value.",
255+
this.getServerAndPort()));
256+
}
257+
}
241258
}
242259

243260
@Override
@@ -312,7 +329,7 @@ public void ensureEncrypted() {
312329
public String currentPassword() {
313330
try {
314331
return StringUtil.isBlank(encryptedPassword) ? null : this.goCipher.decrypt(encryptedPassword);
315-
} catch (InvalidCipherTextException e) {
332+
} catch (Exception e) {
316333
throw new RuntimeException("Could not decrypt the password to get the real password", e);
317334
}
318335
}

‎config/config-api/src/com/thoughtworks/go/config/materials/svn/SvnMaterialConfig.java‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232

3333
import static com.thoughtworks.go.util.ExceptionUtils.bomb;
3434
import static com.thoughtworks.go.util.ExceptionUtils.bombIfNull;
35+
import static java.lang.String.format;
3536
import static org.apache.commons.lang.StringUtils.isNotEmpty;
3637

3738
@ConfigTag(value = "svn", label = "Subversion")
@@ -105,6 +106,15 @@ public SvnMaterialConfig(UrlArgument url, String userName, String password, bool
105106
this.setPassword(password);
106107
}
107108

109+
//for tests
110+
protected SvnMaterialConfig(UrlArgument url, String password, String encryptedPassword, GoCipher goCipher, Filter filter, boolean invertFilter, String folder) {
111+
super(new CaseInsensitiveString("test"), filter, invertFilter, folder, true, TYPE, new ConfigErrors());
112+
this.url = url;
113+
this.password = password;
114+
this.encryptedPassword = encryptedPassword;
115+
this.goCipher = goCipher;
116+
}
117+
108118
public GoCipher getGoCipher() {
109119
return goCipher;
110120
}
@@ -155,7 +165,7 @@ public boolean isCheckExternals() {
155165
public String currentPassword() {
156166
try {
157167
return StringUtil.isBlank(encryptedPassword) ? null : this.goCipher.decrypt(encryptedPassword);
158-
} catch (InvalidCipherTextException e) {
168+
} catch (Exception e) {
159169
throw new RuntimeException("Could not decrypt the password to get the real password", e);
160170
}
161171
}
@@ -166,7 +176,7 @@ public String getEncryptedPassword() {
166176
}
167177

168178
public void setEncryptedPassword(String encryptedPassword) {
169-
this.encryptedPassword=encryptedPassword;
179+
this.encryptedPassword = encryptedPassword;
170180
}
171181

172182
@PostConstruct
@@ -207,10 +217,18 @@ public void validateConcreteScmMaterial() {
207217
if (StringUtil.isBlank(url.forDisplay())) {
208218
errors().add(URL, "URL cannot be blank");
209219
}
210-
if (isNotEmpty(this.password) && isNotEmpty(this.encryptedPassword)){
220+
if (isNotEmpty(this.password) && isNotEmpty(this.encryptedPassword)) {
211221
addError("password", "You may only specify `password` or `encrypted_password`, not both!");
212222
addError("encryptedPassword", "You may only specify `password` or `encrypted_password`, not both!");
213223
}
224+
if (isNotEmpty(this.encryptedPassword)) {
225+
try {
226+
currentPassword();
227+
} catch (Exception e) {
228+
addError("encryptedPassword", format("Encrypted password value for svn material with url '%s' is invalid. This usually happens when the cipher text is modified to have an invalid value.",
229+
this.getUriForDisplay()));
230+
}
231+
}
214232
}
215233

216234
@Override
@@ -300,11 +318,11 @@ public void setConfigAttributes(Object attributes) {
300318
this.checkExternals = "true".equals(map.get(CHECK_EXTERNALS));
301319
}
302320

303-
public void setCheckExternals(boolean checkExternals){
321+
public void setCheckExternals(boolean checkExternals) {
304322
this.checkExternals = checkExternals;
305323
}
306324

307-
public void setUserName(String userName){
325+
public void setUserName(String userName) {
308326
this.userName = userName;
309327
}
310328

@@ -316,4 +334,4 @@ public String toString() {
316334
", checkExternals=" + checkExternals +
317335
'}';
318336
}
319-
}
337+
}

‎config/config-api/src/com/thoughtworks/go/config/materials/tfs/TfsMaterialConfig.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import java.util.Map;
3434

3535
import static com.thoughtworks.go.util.ExceptionUtils.bomb;
36+
import static java.lang.String.format;
3637
import static org.apache.commons.lang.StringUtils.isNotEmpty;
3738

3839
@ConfigTag(value = "tfs", label = "TFS")
@@ -99,6 +100,14 @@ public TfsMaterialConfig(UrlArgument url, String userName, String domain, String
99100
this.projectPath = projectPath;
100101
}
101102

103+
//for tests only
104+
protected TfsMaterialConfig(UrlArgument urlArgument, String password, String encryptedPassword, GoCipher goCipher) {
105+
this(goCipher);
106+
this.url = urlArgument;
107+
this.password = password;
108+
this.encryptedPassword = encryptedPassword;
109+
}
110+
102111
public GoCipher getGoCipher() {
103112
return goCipher;
104113
}
@@ -181,6 +190,15 @@ public void validateConcreteScmMaterial() {
181190
addError("password", "You may only specify `password` or `encrypted_password`, not both!");
182191
addError("encryptedPassword", "You may only specify `password` or `encrypted_password`, not both!");
183192
}
193+
194+
if(isNotEmpty(this.encryptedPassword)) {
195+
try{
196+
goCipher.decrypt(encryptedPassword);
197+
}catch (Exception e) {
198+
addError("encryptedPassword", format("Encrypted password value for TFS material with url '%s' is invalid. This usually happens when the cipher text is modified to have an invalid value.",
199+
this.getUriForDisplay()));
200+
}
201+
}
184202
}
185203

186204
@Override

‎config/config-api/test/com/thoughtworks/go/config/materials/perforce/P4MaterialConfigTest.java‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2015 ThoughtWorks, Inc.
2+
* Copyright 2016 ThoughtWorks, Inc.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -25,6 +25,7 @@
2525
import com.thoughtworks.go.config.materials.svn.SvnMaterialConfig;
2626
import com.thoughtworks.go.security.GoCipher;
2727
import com.thoughtworks.go.util.ReflectionUtil;
28+
import com.thoughtworks.go.util.command.UrlArgument;
2829
import org.junit.Test;
2930

3031
import java.util.HashMap;
@@ -134,6 +135,21 @@ public void shouldNotSetUseTicketsIfNotInConfigAttributesMap() {
134135
assertThat(p4MaterialConfig.getUseTickets(), is(false));
135136
}
136137

138+
@Test
139+
public void shouldThrowErrorsIfBothPasswordAndEncryptedPasswordAreProvided() {
140+
P4MaterialConfig materialConfig = new P4MaterialConfig("foo/bar, 80", "password", "encryptedPassword", new GoCipher());
141+
materialConfig.validate(new ConfigSaveValidationContext(null));
142+
assertThat(materialConfig.errors().on("password"), is("You may only specify `password` or `encrypted_password`, not both!"));
143+
assertThat(materialConfig.errors().on("encryptedPassword"), is("You may only specify `password` or `encrypted_password`, not both!"));
144+
}
145+
146+
@Test
147+
public void shouldValidateWhetherTheEncryptedPasswordIsCorrect() {
148+
P4MaterialConfig materialConfig = new P4MaterialConfig("foo/bar, 80", "", "encryptedPassword", new GoCipher());
149+
materialConfig.validate(new ConfigSaveValidationContext(null));
150+
assertThat(materialConfig.errors().on("encryptedPassword"), is("Encrypted password value for P4 material with serverAndPort 'foo/bar, 80' is invalid. This usually happens when the cipher text is modified to have an invalid value."));
151+
}
152+
137153
private void assertNoError(String port, String view, String expectedKeyForError) {
138154
P4MaterialConfig p4MaterialConfig = new P4MaterialConfig(port, view);
139155
p4MaterialConfig.validate(new ConfigSaveValidationContext(null));

‎config/config-api/test/com/thoughtworks/go/config/materials/svn/SvnMaterialConfigTest.java‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2015 ThoughtWorks, Inc.
2+
* Copyright 2016 ThoughtWorks, Inc.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -24,6 +24,7 @@
2424
import com.thoughtworks.go.config.materials.ScmMaterialConfig;
2525
import com.thoughtworks.go.security.GoCipher;
2626
import com.thoughtworks.go.util.ReflectionUtil;
27+
import com.thoughtworks.go.util.command.UrlArgument;
2728
import org.hamcrest.core.Is;
2829
import org.junit.Before;
2930
import org.junit.Test;
@@ -94,6 +95,21 @@ public void validate_shouldEnsureDestFilePathIsValid() {
9495
assertThat(svnMaterialConfig.errors().on(SvnMaterialConfig.FOLDER), is("Dest folder '../a' is not valid. It must be a sub-directory of the working folder."));
9596
}
9697

98+
@Test
99+
public void shouldThrowErrorsIfBothPasswordAndEncryptedPasswordAreProvided() {
100+
SvnMaterialConfig svnMaterialConfig = new SvnMaterialConfig(new UrlArgument("foo/bar"), "password", "encryptedPassword", new GoCipher(), null, false, "folder");
101+
svnMaterialConfig.validate(new ConfigSaveValidationContext(null));
102+
assertThat(svnMaterialConfig.errors().on("password"), is("You may only specify `password` or `encrypted_password`, not both!"));
103+
assertThat(svnMaterialConfig.errors().on("encryptedPassword"), is("You may only specify `password` or `encrypted_password`, not both!"));
104+
}
105+
106+
@Test
107+
public void shouldValidateWhetherTheEncryptedPasswordIsCorrect() {
108+
SvnMaterialConfig svnMaterialConfig = new SvnMaterialConfig(new UrlArgument("foo/bar"), "", "encryptedPassword", new GoCipher(), null, false, "folder");
109+
svnMaterialConfig.validate(new ConfigSaveValidationContext(null));
110+
assertThat(svnMaterialConfig.errors().on("encryptedPassword"), is("Encrypted password value for svn material with url 'foo/bar' is invalid. This usually happens when the cipher text is modified to have an invalid value."));
111+
}
112+
97113
@Test
98114
public void setConfigAttributes_shouldUpdatePasswordWhenPasswordChangedBooleanChanged() throws Exception {
99115
SvnMaterialConfig svnMaterial = new SvnMaterialConfig("", "", "notSoSecret", false);

‎config/config-server/test-resources/data/big-cruise-config.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12628,7 +12628,7 @@
1262812628
<pipeline name="cleanArtifacts">
1262912629
<timer>59 59 23 ? * *</timer>
1263012630
<materials>
12631-
<svn url="http://10.18.3.171:8080/svn/artifactsmanagement/trunk" username="cce" encryptedPassword="pVyuW5ny9I6YT4Ou+KLZhQ==" dest="keptfolder" autoUpdate="false" />
12631+
<svn url="http://10.18.3.171:8080/svn/artifactsmanagement/trunk" dest="keptfolder" autoUpdate="false" />
1263212632
<svn url="http://cruisetools.googlecode.com/svn/trunk/artifactsmanagement/lib" dest="app" autoUpdate="false" />
1263312633
</materials>
1263412634
<stage name="defaultStage">

0 commit comments

Comments
 (0)