Skip to content

Commit

Permalink
DO NOT MERGE: Add support for JCasC using SecretBytes (#48)
Browse files Browse the repository at this point in the history 
* Use DataboundSetter throughout except for final field emailAddress for p12 keys.

* Update dependencies required for using JCasC plugin in tests.

* Remove unneeded imports.

* Add test to verify Configuration as Code behavior, along with associated resources.

* Temporarily use old format for license header for checkStyle, because this is based on master and not develop.

* Preserve backwards compatibility of json keys.

* Preverse compatibility of P12 keys.

* Avoid using real keys in any form during JCasC tests.

* Address checkstyle failures.

* Fix indentation misalignment. Mark dependencies only required for CasC tests as optional so that they are not included when using hpi to package or run.

* Remove findbugs dependency. Uses spotbugs from the parent pom since switching to the version that uses spotbugs instead of findbugs is required to use CasC.

* Add eof new line to test-key.json

* Add missing javadoc on JsonServiceAccountConfig. Use correct capitalization and punctuation in javadoc.

* Add TODO tracking issue #50 for deduplication.

* Fix secret bytes to account for newline in file.

* Incorporate PR feedback.

* Fix typos in yml comments.
  • Loading branch information
Stephen Shank authored Apr 15, 2019
1 parent 5af04a6 commit d69e66f
Show file tree
Hide file tree
Showing 16 changed files with 428 additions and 140 deletions.
2 changes: 1 addition & 1 deletion checkstyleJavaHeader
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
^/\*$
^ \* Copyright 201(3|4|5|6|7|8) .*$
^ \* Copyright 201(3|4|5|6|7|8|9) .*$
^ \*$
^ \* Licensed under the Apache License, Version 2\.0 \(the \"License\"\);$
^ \* you may not use this file except in compliance with the License\.$
Expand Down
72 changes: 53 additions & 19 deletions pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>3.8</version>
<version>3.36</version>
</parent>

<!--
Expand Down Expand Up @@ -101,15 +101,16 @@
<artifactId>cobertura-maven-plugin</artifactId>
<version>2.5.2</version>
<configuration>
<instrumentation>
<excludes>
<exclude>com/google/jenkins/plugins/**/Messages.class</exclude>
</excludes>
</instrumentation>
<formats>
<instrumentation>
<excludes>
<exclude>com/google/jenkins/plugins/**/Messages.class</exclude>
</excludes>
</instrumentation>
<formats>
<format>html</format>
<format>xml</format>
</formats>
<format>xml</format>
</formats>
<check/>
</configuration>
</plugin>
</plugins>
Expand All @@ -120,27 +121,24 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-javadoc-plugin</artifactId>
<version>2.9</version>
<version>2.10.4</version>
</plugin>
</plugins>
</reporting>

<!-- Bring some sanity to version numbering... -->
<properties>
<jenkins.version>1.653</jenkins.version>
<jenkins.version>2.60.3</jenkins.version>
<google.api.version>1.24.1</google.api.version>
<java.level>7</java.level>
<configuration-as-code.version>1.9</configuration-as-code.version>
<credentials.version>2.1.16</credentials.version>
<java.level>8</java.level>
<findbugs.excludeFilterFile>findbugs-exclude.xml</findbugs.excludeFilterFile>
<findbugs.effort>Max</findbugs.effort>
<findbugs.threshold>Medium</findbugs.threshold>
</properties>

<dependencies>
<dependency>
<groupId>net.sourceforge.findbugs</groupId>
<artifactId>jsr305</artifactId>
<version>1.3.2</version>
</dependency>
<!-- com.google.guava -->
<dependency>
<groupId>com.google.guava</groupId>
Expand Down Expand Up @@ -176,6 +174,43 @@
<version>1.8.4</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.jenkins</groupId>
<artifactId>configuration-as-code</artifactId>
<version>${configuration-as-code.version}</version>
<scope>test</scope>
<!--Marked as optional so hpi:run does not include it. -->
<optional>true</optional>
</dependency>
<dependency>
<groupId>io.jenkins</groupId>
<artifactId>configuration-as-code</artifactId>
<version>${configuration-as-code.version}</version>
<classifier>tests</classifier>
<scope>test</scope>
<optional>true</optional>
</dependency>
<dependency>
<groupId>io.jenkins.configuration-as-code</groupId>
<artifactId>configuration-as-code-support</artifactId>
<version>${configuration-as-code.version}</version>
<scope>test</scope>
<optional>true</optional>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plain-credentials</artifactId>
<version>1.5</version>
<scope>test</scope>
<optional>true</optional>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>ssh-credentials</artifactId>
<version>1.13</version>
<scope>test</scope>
<optional>true</optional>
</dependency>

<!-- OAuth Credentials dependency -->
<dependency>
Expand Down Expand Up @@ -215,8 +250,7 @@
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>credentials</artifactId>
<version>2.1.12</version>
<scope>compile</scope>
<version>${credentials.version}</version>
</dependency>
<dependency>
<!-- Required to run P12ServiceAccountConfigTestUtil-dependent tests against newer Jenkins core versions.
Expand Down
4 changes: 2 additions & 2 deletions ...n/java/com/google/jenkins/plugins/credentials/oauth/GoogleRobotPrivateKeyCredentials.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ public GoogleRobotPrivateKeyCredentials(String projectId,
public Object readResolve() {
if (serviceAccountConfig == null) {
String clientEmail = getClientEmailFromSecretsFileAndLogErrors();
serviceAccountConfig = new P12ServiceAccountConfig(clientEmail, null,
p12File);
serviceAccountConfig =
new P12ServiceAccountConfig(clientEmail, null, p12File);
}
return this;
}
Expand Down
126 changes: 88 additions & 38 deletions src/main/java/com/google/jenkins/plugins/credentials/oauth/JsonServiceAccountConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,20 @@
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.DoNotUse;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.DataBoundSetter;

import com.cloudbees.plugins.credentials.SecretBytes;
import com.google.api.client.json.jackson.JacksonFactory;
import com.google.api.client.util.PemReader;
import com.google.api.client.util.Strings;

import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import hudson.Extension;
import jenkins.model.Jenkins;

/**
* Provides authentication mechanism for a service account by setting a .json
* private key file. The .json file structure needs to be:
* Provides authentication mechanism for a service account by setting a JSON
* private key file. The JSON file structure needs to be:
* <p>
* <code>
* {
Expand All @@ -58,6 +60,11 @@
* </code>
*/
public class JsonServiceAccountConfig extends ServiceAccountConfig {
/*
* TODO(jenkinsci/google-oauth-plugin#50): Dedupe shared functionality in
* google-auth-library.
*/

private static final long serialVersionUID = 6818111194672325387L;
private static final Logger LOGGER =
Logger.getLogger(JsonServiceAccountConfig.class.getSimpleName());
Expand All @@ -70,41 +77,61 @@ public class JsonServiceAccountConfig extends ServiceAccountConfig {
private transient String jsonKeyFile;
private transient JsonKey jsonKey;

/** @since 0.8 */
@DataBoundConstructor
public JsonServiceAccountConfig() {}

/**
* @param jsonKeyFile uploaded json key file
* @param filename
* previous json key file name.
* used if jsonKeyFile is not provided.
* @param secretJsonKey
* previous json key file content.
* used if jsonKeyFile is not provided.
* @since 0.7
* For being able to load credentials created with versions < 0.8
* and backwards compatibility with external callers.
*
* @param jsonKeyFile The uploaded JSON key file.
* @param prevJsonKeyFile The path of the previous JSON key file.
* @since 0.3
*/
@DataBoundConstructor
public JsonServiceAccountConfig(FileItem jsonKeyFile,
String filename, SecretBytes secretJsonKey) {
if (jsonKeyFile != null && jsonKeyFile.getSize() > 0) {
@Deprecated
public JsonServiceAccountConfig(
FileItem jsonKeyFile, String prevJsonKeyFile) {
this.setJsonKeyFileUpload(jsonKeyFile);
if (filename == null && prevJsonKeyFile != null) {
this.filename = extractFilename(prevJsonKeyFile);
this.secretJsonKey = getSecretBytesFromFile(prevJsonKeyFile);
}
}

/** @param jsonKeyFileUpload The uploaded JSON key file. */
@DataBoundSetter // Called on form submit, only used when key file is uploaded
public void setJsonKeyFileUpload(FileItem jsonKeyFileUpload) {
if (jsonKeyFileUpload != null && jsonKeyFileUpload.getSize() > 0) {
try {
JsonKey jsonKey = JsonKey.load(new JacksonFactory(),
jsonKeyFile.getInputStream());
jsonKeyFileUpload.getInputStream());
if (jsonKey.getClientEmail() != null &&
jsonKey.getPrivateKey() != null) {
this.filename = extractFilename(jsonKeyFile.getName());
this.secretJsonKey = SecretBytes.fromBytes(jsonKeyFile.get());
jsonKey.getPrivateKey() != null) {
this.filename = extractFilename(jsonKeyFileUpload.getName());
this.secretJsonKey = SecretBytes.fromBytes(jsonKeyFileUpload.get());
}
} catch (IOException e) {
LOGGER.log(Level.SEVERE, "Failed to read json key from file", e);
LOGGER.log(Level.SEVERE, "Failed to read JSON key from file", e);
}
} else {
this.filename = extractFilename(filename);
this.secretJsonKey = secretJsonKey;
}
}

@Deprecated
public JsonServiceAccountConfig(FileItem jsonKeyFile,
String prevJsonKeyFile) {
this(null, prevJsonKeyFile, getSecretBytesFromFile(prevJsonKeyFile));
/** @param filename The JSON key file name. */
@DataBoundSetter
public void setFilename(String filename) {
String newFilename = extractFilename(filename);
if (!Strings.isNullOrEmpty(newFilename)) {
this.filename = newFilename;
}
}

/** @param secretJsonKey The JSON key file content. */
@DataBoundSetter
public void setSecretJsonKey(SecretBytes secretJsonKey) {
if (secretJsonKey != null && secretJsonKey.getPlainData().length > 0) {
this.secretJsonKey = secretJsonKey;
}
}

@Deprecated // used only for compatibility purpose
Expand Down Expand Up @@ -139,7 +166,7 @@ private Object readResolve() {
if (secretJsonKey == null) {
// google-oauth-plugin < 0.7
return new JsonServiceAccountConfig(
null,
null,
getJsonKeyFile()
);
}
Expand All @@ -153,15 +180,15 @@ public DescriptorImpl getDescriptor() {
}

/**
* @return Original uploaded file name
* @return Original uploaded file name.
* @since 0.7
*/
@CheckForNull
public String getFilename() {
return filename;
}

@Restricted(DoNotUse.class) // for UI purpose only
@Restricted(DoNotUse.class) // UI: Required for stapler call of setter.
@CheckForNull
public SecretBytes getSecretJsonKey() {
return secretJsonKey;
Expand All @@ -172,6 +199,23 @@ public String getJsonKeyFile() {
return jsonKeyFile;
}

/**
* For use in UI, do not use.
* @return The uploaded JSON key file.
*/
@Deprecated
@Restricted(DoNotUse.class) // UI: Required for stapler call of setter.
public FileItem getJsonKeyFileUpload() {
return null;
}

/**
* In this context the service account id is represented by the email address
* for that service account, which should be contained in the JSON key.
*
* @return The service account identifier. Null if no JSON key has been
* provided.
*/
@Override
public String getAccountId() {
JsonKey jsonKey = getJsonKey();
Expand All @@ -181,6 +225,10 @@ public String getAccountId() {
return null;
}

/**
* @return The {@link PrivateKey} that comes from the secret JSON key. Null if
* this service account config contains no key or if the key is malformed.
*/
@Override
public PrivateKey getPrivateKey() {
JsonKey jsonKey = getJsonKey();
Expand All @@ -190,14 +238,16 @@ public PrivateKey getPrivateKey() {
PemReader pemReader = new PemReader(new StringReader(privateKey));
try {
PemReader.Section section = pemReader.readNextSection();
PKCS8EncodedKeySpec keySpec =
new PKCS8EncodedKeySpec(section.getBase64DecodedBytes());
return KeyFactory.getInstance("RSA").generatePrivate(keySpec);
} catch (IOException e) {
LOGGER.log(Level.SEVERE, "Failed to read private key", e);
} catch (InvalidKeySpecException e) {
LOGGER.log(Level.SEVERE, "Failed to read private key", e);
} catch (NoSuchAlgorithmException e) {
if (section != null) {
PKCS8EncodedKeySpec keySpec =
new PKCS8EncodedKeySpec(section.getBase64DecodedBytes());
return KeyFactory.getInstance("RSA").generatePrivate(keySpec);
} else {
LOGGER.severe("The provided private key is malformed.");
}
} catch (IOException
| InvalidKeySpecException
| NoSuchAlgorithmException e) {
LOGGER.log(Level.SEVERE, "Failed to read private key", e);
}
}
Expand All @@ -218,7 +268,7 @@ private JsonKey getJsonKey() {
}

/**
* descriptor for .json service account authentication
* Descriptor for JSON service account authentication.
*/
@Extension
public static final class DescriptorImpl extends Descriptor {
Expand Down
Loading

0 comments on commit d69e66f

Please sign in to comment.