Skip to content

Commit

Permalink
[SECURITY-967]
Browse files Browse the repository at this point in the history
  • Loading branch information
@galmeida @daniel-beck
galmeida authored and daniel-beck committed Jun 19, 2018
1 parent 83a8656 commit a45d3dc
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 12 deletions.
22 changes: 16 additions & 6 deletions src/main/java/com/amazonaws/codepipeline/jenkinsplugin/AWSCodePipelineSCM.java
View file
Expand Up @@ -36,8 +36,10 @@
import java.util.Collections;
import java.util.Random;

import hudson.util.Secret;
import net.sf.json.JSONObject;

import org.apache.commons.lang.StringUtils;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.StaplerRequest;
Expand Down Expand Up @@ -94,7 +96,7 @@ public class AWSCodePipelineSCM extends hudson.scm.SCM {

private final String region;
private final String awsAccessKey;
private final String awsSecretKey;
private final Secret awsSecretKey;
private final String proxyHost;
private final int proxyPort;

Expand Down Expand Up @@ -133,7 +135,7 @@ public AWSCodePipelineSCM(
clearWorkspace = clear;
this.region = Validation.sanitize(region.trim());
this.awsAccessKey = Validation.sanitize(awsAccessKey.trim());
this.awsSecretKey = Validation.sanitize(awsSecretKey.trim());
this.awsSecretKey = Secret.fromString(Validation.sanitize(awsSecretKey.trim()));
this.proxyHost = Validation.sanitize(proxyHost.trim());
this.projectName = null;
actionTypeCategory = Validation.sanitize(category.trim());
Expand Down Expand Up @@ -278,7 +280,10 @@ public String getAwsAccessKey() {
return awsAccessKey;
}

public String getAwsSecretKey() {
public Secret getAwsSecretKey() {
if (StringUtils.isEmpty(Secret.toString(awsSecretKey))) {
return null;
}
return awsSecretKey;
}

Expand Down Expand Up @@ -306,11 +311,16 @@ public String getVersion() {
return actionTypeVersion;
}

// this is required so we can use JenkinsRule.assertEqualDataBoundBeans in unit tests
public String getName() {
return null;
}

public void initializeModel() {
final CodePipelineStateModel model = new CodePipelineStateModel();
model.setActionTypeCategory(actionTypeCategory);
model.setAwsAccessKey(awsAccessKey);
model.setAwsSecretKey(awsSecretKey);
model.setAwsSecretKey(Secret.toString(awsSecretKey));
model.setCompressionType(CompressionType.None);
model.setJob(job);
model.setProxyHost(proxyHost);
Expand All @@ -322,7 +332,7 @@ public void initializeModel() {
private void validate(final String projectName, final TaskListener listener) {
Validation.validatePlugin(
awsAccessKey,
awsSecretKey,
Secret.toString(awsSecretKey),
region,
actionTypeCategory,
actionTypeProvider,
Expand All @@ -334,7 +344,7 @@ private void validate(final String projectName, final TaskListener listener) {
private AWSCodePipeline getCodePipelineClient() {
return awsClientFactory.getAwsClient(
awsAccessKey,
awsSecretKey,
Secret.toString(awsSecretKey),
proxyHost,
proxyPort,
region,
Expand Down
72 changes: 66 additions & 6 deletions src/test/java/com/amazonaws/codepipeline/jenkinsplugin/AWSCodePipelineSCMTest.java
View file
Expand Up @@ -16,6 +16,7 @@

import static com.amazonaws.codepipeline.jenkinsplugin.TestUtils.assertContainsIgnoreCase;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.Matchers.any;
Expand All @@ -29,26 +30,33 @@
import hudson.AbortException;
import hudson.EnvVars;
import hudson.FilePath;
import hudson.model.FreeStyleProject;
import hudson.model.Project;
import hudson.model.TaskListener;
import hudson.model.AbstractBuild;
import hudson.scm.PollingResult;
import hudson.scm.SCM;
import hudson.util.FormValidation;
import hudson.util.Secret;

import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.IOException;
import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.Collections;
import java.util.UUID;

import org.apache.commons.lang.RandomStringUtils;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Suite;
import org.junit.runners.model.InitializationError;
import org.junit.runners.model.RunnerBuilder;
import org.jvnet.hudson.test.JenkinsRule;
import org.mockito.ArgumentCaptor;
import org.mockito.Captor;
import org.mockito.InOrder;
Expand All @@ -73,7 +81,8 @@
@Suite.SuiteClasses({
AWSCodePipelineSCMTest.PollForJobsTests.class,
AWSCodePipelineSCMTest.CheckoutTests.class,
AWSCodePipelineSCMTest.SCMDescriptorTests.class
AWSCodePipelineSCMTest.SCMDescriptorTests.class,
AWSCodePipelineSCMTest.ConfigTest.class
})
public class AWSCodePipelineSCMTest extends Suite {

Expand All @@ -94,10 +103,15 @@ private static class TestBase {
protected String jobId;
protected String jobNonce;

public void setUp() throws IOException, InterruptedException {
public void setUp() throws IOException, InterruptedException, ReflectiveOperationException {
MockitoAnnotations.initMocks(this);
jobId = UUID.randomUUID().toString();
jobNonce = UUID.randomUUID().toString();

// Override the secret key so that we can test this class without {@link jenkins.model.Jenkins}.
Field field = Secret.class.getDeclaredField("SECRET");
field.setAccessible(true);
field.set(null, RandomStringUtils.random(16));
}
}

Expand All @@ -120,7 +134,7 @@ private static class PollingTestBase extends TestBase {
protected ByteArrayOutputStream outContent;

@Before
public void setUp() throws IOException, InterruptedException {
public void setUp() throws IOException, InterruptedException, ReflectiveOperationException {
super.setUp();

when(mockFactory.getAwsClient(anyString(), anyString(), anyString(), anyInt(), anyString(), anyString()))
Expand Down Expand Up @@ -158,7 +172,7 @@ public void setUp() throws IOException, InterruptedException {
public static class PollForJobsTests extends PollingTestBase {

@Before
public void setUp() throws IOException, InterruptedException {
public void setUp() throws IOException, InterruptedException, ReflectiveOperationException {
super.setUp();
}

Expand Down Expand Up @@ -211,7 +225,7 @@ public static class CheckoutTests extends PollingTestBase {
private ArgumentCaptor<AcknowledgeJobRequest> acknowledgeJobRequest;

@Before
public void setUp() throws IOException, InterruptedException {
public void setUp() throws IOException, InterruptedException, ReflectiveOperationException {
super.setUp();

final File tempFile = File.createTempFile("workspacePath", "tmp");
Expand Down Expand Up @@ -311,7 +325,7 @@ public static class SCMDescriptorTests extends TestBase {
private AWSCodePipelineSCM.DescriptorImpl descriptor;

@Before
public void setUp() throws IOException, InterruptedException {
public void setUp() throws IOException, InterruptedException, ReflectiveOperationException {
super.setUp();

descriptor = new AWSCodePipelineSCM.DescriptorImpl(false);
Expand Down Expand Up @@ -470,4 +484,50 @@ private void assertValidationMessage(final FormValidation expected, final FormVa
}
}

public static class ConfigTest extends TestBase {
private static final String PLAIN_SECRET = "PLAIN-SECRET";

@Rule
public JenkinsRule jenkinsRule = new JenkinsRule();

private AWSCodePipelineSCM awsCodePipelineSCM;

@Before
public void setUp() throws InterruptedException, ReflectiveOperationException, IOException {
super.setUp();

awsCodePipelineSCM = new AWSCodePipelineSCM("name",
true,
"us-east-1",
"awsAccessKey",
PLAIN_SECRET,
"proxyhost",
"8080",
"Build",
"Jenkins",
"1");
}

@Test
public void testRoundTripConfiguration() throws Exception {
final AWSCodePipelineSCM before = awsCodePipelineSCM;
final Project project = jenkinsRule.createFreeStyleProject();
project.setScm(before);

jenkinsRule.configRoundtrip(project);
final SCM after = project.getScm();

jenkinsRule.assertEqualDataBoundBeans(before, after);
}

@Test
public void storeAwsSecretKeyAsSecret() throws Exception {
final Project project = jenkinsRule.createFreeStyleProject();
project.setScm(awsCodePipelineSCM);

jenkinsRule.configRoundtrip(project);

assertFalse(project.getConfigFile().asString().contains(PLAIN_SECRET));
}
}
}

0 comments on commit a45d3dc

Please sign in to comment.