Removing raw API Token and forcing credential

In a pipeline you can no longer do `apiToken: 'asd'` and will have to do `apiTokenCredentialsId: 'id'`. SECURITY-1577
jenkinsci · Sep 13, 2019 · e8237a8 · e8237a8
1 parent fbf1796
commit e8237a8
Show file tree

Hide file tree

Showing 9 changed files with 35 additions and 89 deletions.
diff --git a/README.md b/README.md
@@ -292,7 +292,6 @@ job('GitLab_MR_Builder') {
     enableLogging(true)
     
  
-    apiToken("")
     apiTokenCredentialsId("gitlabtoken")
     apiTokenPrivate(true)
     authMethodHeader(true)
@@ -485,8 +484,6 @@ node {
   proxyUser: '',
   proxyPassword: '',
  
-  // Specify one of these
-  apiToken: '6xRcmSzPzzEXeS2qqr7R',
   apiTokenCredentialsId: 'id',
  
   apiTokenPrivate: true,

diff --git a/pom.xml b/pom.xml
@@ -13,9 +13,9 @@
 		<jenkins.version>2.1</jenkins.version>
 		<findbugs.failOnError>false</findbugs.failOnError>
 		<maven.javadoc.skip>true</maven.javadoc.skip>
-		<violations-maven>1.20</violations-maven>
+		<violations-maven>1.21</violations-maven>
 		<violation-comments-lib>1.78</violation-comments-lib>
-		<violations-lib>1.97</violations-lib>
+		<violations-lib>1.101</violations-lib>
 		<changelog>1.60</changelog>
 		<fmt>2.9</fmt>
 	</properties>

diff --git a/sandbox/gitlab.com.testpipeline.jenkinsfile b/sandbox/gitlab.com.testpipeline.jenkinsfile
@@ -26,9 +26,6 @@ node {
     commentOnlyChangedFiles: true,
     createCommentWithAllSingleFileComments: true, 
     minSeverity: 'INFO',
-    useApiToken: true,
-    apiToken: 'asdasdasdasd',
-    useApiTokenCredentials: false,
     apiTokenCredentialsId: 'id',
     apiTokenPrivate: true,
     authMethodHeader: true,

diff --git a/src/main/java/org/jenkinsci/plugins/jvctgl/ViolationsToGitLabGlobalConfiguration.java b/src/main/java/org/jenkinsci/plugins/jvctgl/ViolationsToGitLabGlobalConfiguration.java
@@ -35,7 +35,6 @@ public static ViolationsToGitLabGlobalConfiguration get() {
 
   private boolean ignoreCertificateErrors;
 
-  private String apiToken;
   private boolean apiTokenPrivate;
   private boolean authMethodHeader;
   private String apiTokenCredentialsId;
@@ -63,16 +62,18 @@ public ListBoxModel doFillMinSeverityItems() {
 
   @SuppressWarnings("unused") // Used by stapler
   public ListBoxModel doFillApiTokenCredentialsIdItems(
-      @AncestorInPath Item item,
-      @QueryParameter String apiTokenCredentialsId,
-      @QueryParameter String gitLabUrl) {
+      @AncestorInPath final Item item,
+      @QueryParameter final String apiTokenCredentialsId,
+      @QueryParameter final String gitLabUrl) {
     return CredentialsHelper.doFillApiTokenCredentialsIdItems(
         item, apiTokenCredentialsId, gitLabUrl);
   }
 
   @SuppressWarnings("unused") // Used by stapler
   public FormValidation doCheckApiTokenCredentialsId(
-      @AncestorInPath Item item, @QueryParameter String value, @QueryParameter String gitLabUrl) {
+      @AncestorInPath final Item item,
+      @QueryParameter final String value,
+      @QueryParameter final String gitLabUrl) {
     return CredentialsHelper.doCheckApiTokenCredentialsId(item, value, gitLabUrl);
   }
 
@@ -89,13 +90,11 @@ public void setIgnoreCertificateErrors(final boolean ignoreCertificateErrors) {
     this.ignoreCertificateErrors = ignoreCertificateErrors;
   }
 
-  public String getApiToken() {
-    return apiToken;
-  }
-
   @DataBoundSetter
+  @Deprecated
   public void setApiToken(final String apiToken) {
-    this.apiToken = apiToken;
+    throw new RuntimeException(
+        "Setting raw API token is removed, set the apiTokenCredentialsId with a string credential instead!");
   }
 
   public String getApiTokenCredentialsId() {
@@ -147,7 +146,6 @@ public void setAuthMethodHeader(final boolean authMethodHeader) {
   public int hashCode() {
     final int prime = 31;
     int result = 1;
-    result = prime * result + (apiToken == null ? 0 : apiToken.hashCode());
     result =
         prime * result + (apiTokenCredentialsId == null ? 0 : apiTokenCredentialsId.hashCode());
     result = prime * result + (apiTokenPrivate ? 1231 : 1237);
@@ -159,7 +157,7 @@ public int hashCode() {
   }
 
   @Override
-  public boolean equals(Object obj) {
+  public boolean equals(final Object obj) {
     if (this == obj) {
       return true;
     }
@@ -170,13 +168,6 @@ public boolean equals(Object obj) {
       return false;
     }
     final ViolationsToGitLabGlobalConfiguration other = (ViolationsToGitLabGlobalConfiguration) obj;
-    if (apiToken == null) {
-      if (other.apiToken != null) {
-        return false;
-      }
-    } else if (!apiToken.equals(other.apiToken)) {
-      return false;
-    }
     if (apiTokenCredentialsId == null) {
       if (other.apiTokenCredentialsId != null) {
         return false;
@@ -212,8 +203,6 @@ public String toString() {
         + gitLabUrl
         + ", ignoreCertificateErrors="
         + ignoreCertificateErrors
-        + ", apiToken="
-        + apiToken
         + ", apiTokenPrivate="
         + apiTokenPrivate
         + ", authMethodHeader="

diff --git a/src/main/java/org/jenkinsci/plugins/jvctgl/config/ViolationsToGitLabConfig.java b/src/main/java/org/jenkinsci/plugins/jvctgl/config/ViolationsToGitLabConfig.java
@@ -33,7 +33,6 @@ public class ViolationsToGitLabConfig extends AbstractDescribableImpl<Violations
   private boolean createSingleFileComments;
   private List<ViolationConfig> violationConfigs;
   private String gitLabUrl;
-  private String apiToken;
   private String projectId;
   private String mergeRequestIid;
   @Deprecated private transient String mergeRequestId;
@@ -53,14 +52,18 @@ public class ViolationsToGitLabConfig extends AbstractDescribableImpl<Violations
 
   @DataBoundConstructor
   public ViolationsToGitLabConfig(
-      final String gitLabUrl, final String projectId, final String mergeRequestIid) {
+      final String gitLabUrl,
+      final String projectId,
+      final String mergeRequestIid,
+      final String apiTokenCredentialsId) {
     this.gitLabUrl = gitLabUrl;
     this.projectId = projectId;
     this.mergeRequestIid = mergeRequestIid;
     this.keepOldComments = true;
     this.shouldSetWip = false;
     this.enableLogging = false;
     this.maxNumberOfViolations = null;
+    this.apiTokenCredentialsId = apiTokenCredentialsId;
   }
 
   public ViolationsToGitLabConfig(final ViolationsToGitLabConfig rhs) {
@@ -70,7 +73,6 @@ public ViolationsToGitLabConfig(final ViolationsToGitLabConfig rhs) {
     this.commentOnlyChangedContent = rhs.commentOnlyChangedContent;
     this.commentOnlyChangedFiles = rhs.commentOnlyChangedFiles;
     this.gitLabUrl = rhs.gitLabUrl;
-    this.apiToken = rhs.apiToken;
     this.projectId = rhs.projectId;
     this.mergeRequestIid = rhs.mergeRequestIid;
     this.apiTokenCredentialsId = rhs.apiTokenCredentialsId;
@@ -116,9 +118,6 @@ public void applyDefaults(final ViolationsToGitLabGlobalConfiguration defaults)
     if (isNullOrEmpty(this.gitLabUrl)) {
       this.gitLabUrl = defaults.getGitLabUrl();
     }
-    if (isNullOrEmpty(this.apiToken)) {
-      this.apiToken = defaults.getApiToken();
-    }
     if (isNullOrEmpty(this.apiTokenCredentialsId)) {
       this.apiTokenCredentialsId = defaults.getApiTokenCredentialsId();
     }
@@ -174,20 +173,11 @@ public void setProjectId(final String projectId) {
     this.projectId = projectId;
   }
 
-  @DataBoundSetter
-  public void setApiTokenCredentialsId(final String apiTokenCredentialsId) {
-    this.apiTokenCredentialsId = apiTokenCredentialsId;
-  }
-
   @DataBoundSetter
   public void setAuthMethodHeader(final Boolean authMethodHeader) {
     this.authMethodHeader = authMethodHeader;
   }
 
-  public String getApiToken() {
-    return apiToken;
-  }
-
   public String getProjectId() {
     return projectId;
   }
@@ -255,9 +245,15 @@ public void setGitLabUrl(final String gitLabUrl) {
     this.gitLabUrl = gitLabUrl;
   }
 
+  public void setApiTokenCredentialsId(final String apiTokenCredentialsId) {
+    this.apiTokenCredentialsId = apiTokenCredentialsId;
+  }
+
   @DataBoundSetter
+  @Deprecated
   public void setApiToken(final String apiToken) {
-    this.apiToken = apiToken;
+    throw new RuntimeException(
+        "Setting raw API token is removed, set the apiTokenCredentialsId with a string credential instead!");
   }
 
   @Override
@@ -275,7 +271,6 @@ public boolean equals(final Object o) {
         && createSingleFileComments == that.createSingleFileComments
         && Objects.equals(violationConfigs, that.violationConfigs)
         && Objects.equals(gitLabUrl, that.gitLabUrl)
-        && Objects.equals(apiToken, that.apiToken)
         && Objects.equals(projectId, that.projectId)
         && Objects.equals(mergeRequestIid, that.mergeRequestIid)
         && Objects.equals(mergeRequestId, that.mergeRequestId)
@@ -302,7 +297,6 @@ public int hashCode() {
         createSingleFileComments,
         violationConfigs,
         gitLabUrl,
-        apiToken,
         projectId,
         mergeRequestIid,
         mergeRequestId,

diff --git a/src/main/java/org/jenkinsci/plugins/jvctgl/config/ViolationsToGitLabConfigHelper.java b/src/main/java/org/jenkinsci/plugins/jvctgl/config/ViolationsToGitLabConfigHelper.java
@@ -11,9 +11,6 @@ public class ViolationsToGitLabConfigHelper {
   public static final String FIELD_PARSER = "parser";
   public static final String FIELD_PROJECTID = "projectId";
   public static final String FIELD_MERGEREQUESTIID = "mergeRequestIid";
-  public static final String FIELD_APITOKEN = "apiToken";
-  public static final String FIELD_USEAPITOKENCREDENTIALS = "useApiTokenCredentials";
-  public static final String FIELD_USEAPITOKEN = "useApiToken";
   public static final String FIELD_APITOKENCREDENTIALSID = "apiTokenCredentialsId";
   public static final String FIELD_IGNORECERTIFICATEERRORS = "ignoreCertificateErrors";
   public static final String FIELD_APITOKENPRIVATE = "apiTokenPrivate";

diff --git a/src/main/java/org/jenkinsci/plugins/jvctgl/perform/JvctglPerformer.java b/src/main/java/org/jenkinsci/plugins/jvctgl/perform/JvctglPerformer.java
@@ -6,7 +6,6 @@
 import static com.google.common.base.Throwables.propagate;
 import static com.google.common.collect.Lists.newArrayList;
 import static java.util.logging.Level.SEVERE;
-import static org.jenkinsci.plugins.jvctgl.config.ViolationsToGitLabConfigHelper.FIELD_APITOKEN;
 import static org.jenkinsci.plugins.jvctgl.config.ViolationsToGitLabConfigHelper.FIELD_APITOKENCREDENTIALSID;
 import static org.jenkinsci.plugins.jvctgl.config.ViolationsToGitLabConfigHelper.FIELD_APITOKENPRIVATE;
 import static org.jenkinsci.plugins.jvctgl.config.ViolationsToGitLabConfigHelper.FIELD_AUTHMETHODHEADER;
@@ -60,7 +59,7 @@ public class JvctglPerformer {
   @VisibleForTesting
   public static void doPerform(
       final ViolationsToGitLabConfig config,
-      final Optional<StringCredentials> apiTokenCredentials,
+      final String apiToken,
       final File workspace,
       final TaskListener listener)
       throws MalformedURLException {
@@ -99,14 +98,6 @@ public static void doPerform(
       }
     }
 
-    String apiToken = config.getApiToken();
-    if (apiTokenCredentials.isPresent()) {
-      apiToken = apiTokenCredentials.get().getSecret().getPlainText();
-    }
-    if (isNullOrEmpty(apiToken)) {
-      throw new IllegalStateException("No credentials found!");
-    }
-
     final String hostUrl = config.getGitLabUrl();
     final String projectId = config.getProjectId();
     final String mergeRequestIid = config.getMergeRequestIid();
@@ -181,8 +172,6 @@ static ViolationsToGitLabConfig expand(
     expanded.setProjectId(environment.expand(config.getProjectId()));
     expanded.setMergeRequestIid(environment.expand(config.getMergeRequestIid()));
 
-    expanded.setApiToken(config.getApiToken());
-
     expanded.setApiTokenCredentialsId(config.getApiTokenCredentialsId());
 
     expanded.setAuthMethodHeader(config.getAuthMethodHeader());
@@ -259,7 +248,11 @@ public Void invoke(final File workspace, final VirtualChannel channel)
                 throws IOException, InterruptedException {
               setupFindBugsMessages();
               listener.getLogger().println("Workspace: " + workspace.getAbsolutePath());
-              doPerform(configExpanded, apiTokenCredentials, workspace, listener);
+              doPerform(
+                  configExpanded,
+                  apiTokenCredentials.get().getSecret().getPlainText(),
+                  workspace,
+                  listener);
               return null;
             }
           });
@@ -279,8 +272,6 @@ private static void logConfiguration(
     logger.println(FIELD_PROJECTID + ": " + config.getProjectId());
     logger.println(FIELD_MERGEREQUESTIID + ": " + config.getMergeRequestIid());
 
-    logger.println(FIELD_APITOKEN + ": " + !isNullOrEmpty(config.getApiToken()));
-
     logger.println(
         FIELD_APITOKENCREDENTIALSID + ": " + !isNullOrEmpty(config.getApiTokenCredentialsId()));
     logger.println(FIELD_IGNORECERTIFICATEERRORS + ": " + config.getIgnoreCertificateErrors());

diff --git a/...resources/org/jenkinsci/plugins/jvctgl/ViolationsToGitLabGlobalConfiguration/config.jelly b/...resources/org/jenkinsci/plugins/jvctgl/ViolationsToGitLabGlobalConfiguration/config.jelly
@@ -9,17 +9,9 @@
          xmlns:c="/lib/credentials">
  <f:section title="GitLab Violations Server Defaults">
 
-  <f:optionalBlock checked="${!empty instance.apiTokenCredentialsId}" title="Specify API token from credentials" inline="true">
-    <f:entry title="API token Crendential" field="apiTokenCredentialsId">
-        <c:select />
-    </f:entry>
-  </f:optionalBlock>
-
-  <f:optionalBlock checked="${!empty instance.apiToken}" title="Specify API token here" inline="true">
-    <f:entry title="OAuth2 token" field="apiToken">
-        <f:password />
-    </f:entry>
-  </f:optionalBlock>
+  <f:entry title="API token Crendential" field="apiTokenCredentialsId">
+      <c:select />
+  </f:entry>
 
   <f:entry title="Private token" field="apiTokenPrivate">
     <f:checkbox/>

diff --git a/src/main/resources/org/jenkinsci/plugins/jvctgl/config/ViolationsToGitLabConfig/config.jelly b/src/main/resources/org/jenkinsci/plugins/jvctgl/config/ViolationsToGitLabConfig/config.jelly
@@ -8,23 +8,12 @@
          xmlns:t="/lib/hudson"
          xmlns:c="/lib/credentials">
 
-  <f:optionalBlock checked="${!empty instance.apiTokenCredentialsId}" title="Specify API token from credentials" inline="true">
-    <f:entry title="API token Crendential" field="apiTokenCredentialsId">
+  <f:entry title="API token Crendential" field="apiTokenCredentialsId">
         <c:select />
         <f:description>
             Will default to global config.
         </f:description>
-    </f:entry>
-  </f:optionalBlock>
-
-  <f:optionalBlock checked="${!empty instance.apiToken}" title="Specify API token here" inline="true">
-    <f:entry title="OAuth2 token" field="apiToken">
-        <f:password />
-        <f:description>
-          Will default to global config.
-        </f:description>
-    </f:entry>
-  </f:optionalBlock>
+  </f:entry>
 
   <f:optionalBlock checked="${!empty instance.proxyUri}" title="Use proxy" inline="true">
     <f:entry title="URI" field="proxyUri">