Avoid caching the credential, instead get the credential value whenev…

…er it is required through the request filter (#1712) * Avoid caching the credential, instead get the credential value whenever it is required through the request filter (ApiHeaderTokenFilter) * Fix tests * spotless * Update src/main/java/com/dabsquared/gitlabjenkins/connection/GitlabCredentialResolver.java Co-authored-by: Kris Stern <[email protected]> * Update src/main/java/com/dabsquared/gitlabjenkins/connection/GitlabCredentialResolver.java Co-authored-by: Kris Stern <[email protected]> * include imports * Update src/main/java/com/dabsquared/gitlabjenkins/connection/GitlabCredentialResolver.java Co-authored-by: Kris Stern <[email protected]> * Update src/main/java/com/dabsquared/gitlabjenkins/connection/GitlabCredentialResolver.java Co-authored-by: Kris Stern <[email protected]> --------- Co-authored-by: Aaron Echavarria <[email protected]> Co-authored-by: Kris Stern <[email protected]>
jenkinsci · Oct 24, 2024 · 0ff4703 · 0ff4703
1 parent 2b3232e
commit 0ff4703
Show file tree

Hide file tree

Showing 9 changed files with 131 additions and 56 deletions.
diff --git a/src/main/java/com/dabsquared/gitlabjenkins/connection/GitLabConnection.java b/src/main/java/com/dabsquared/gitlabjenkins/connection/GitLabConnection.java
@@ -1,10 +1,8 @@
 package com.dabsquared.gitlabjenkins.connection;
 
-import static com.cloudbees.plugins.credentials.CredentialsProvider.lookupCredentials;
 import static com.dabsquared.gitlabjenkins.gitlab.api.GitLabClientBuilder.getAllGitLabClientBuilders;
 import static com.dabsquared.gitlabjenkins.gitlab.api.GitLabClientBuilder.getGitLabClientBuilderById;
 
-import com.cloudbees.plugins.credentials.CredentialsMatchers;
 import com.cloudbees.plugins.credentials.CredentialsProvider;
 import com.cloudbees.plugins.credentials.CredentialsScope;
 import com.cloudbees.plugins.credentials.CredentialsStore;
@@ -22,7 +20,6 @@
 import hudson.model.AbstractDescribableImpl;
 import hudson.model.Descriptor;
 import hudson.model.Item;
-import hudson.model.ItemGroup;
 import hudson.security.ACL;
 import hudson.util.FormValidation;
 import hudson.util.ListBoxModel;
@@ -36,7 +33,6 @@
 import javax.ws.rs.WebApplicationException;
 import jenkins.model.Jenkins;
 import org.eclipse.jgit.util.StringUtils;
-import org.jenkinsci.plugins.plaincredentials.StringCredentials;
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.DoNotUse;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
@@ -145,44 +141,26 @@ public int getReadTimeout() {
     public GitLabClient getClient(Item item, String jobCredentialId) {
         final String clientId;
         final String token;
+        GitlabCredentialResolver credentialResolver = new GitlabCredentialResolver();
         if ((jobCredentialId == null) || jobCredentialId.equals(apiTokenId)) {
             clientId = "global";
-            token = getApiToken(apiTokenId, null);
+            credentialResolver.setCredentialsId(apiTokenId);
         } else {
             // Add prefix to credential ID to avoid collision with "global"
             clientId = "alternative-" + jobCredentialId;
-            token = getApiToken(jobCredentialId, item);
+            credentialResolver.setCredentialsId(jobCredentialId);
+            credentialResolver.setItem(item);
         }
 
         if (!clientCache.containsKey(clientId)) {
             clientCache.put(
                     clientId,
-                    clientBuilder.buildClient(url, token, ignoreCertificateErrors, connectionTimeout, readTimeout));
+                    clientBuilder.buildClient(
+                            url, credentialResolver, ignoreCertificateErrors, connectionTimeout, readTimeout));
         }
         return clientCache.get(clientId);
     }
 
-    @Restricted(NoExternalUse.class)
-    private String getApiToken(String apiTokenId, Item item) {
-        ItemGroup<?> context = item != null ? item.getParent() : Jenkins.get();
-        StandardCredentials credentials = CredentialsMatchers.firstOrNull(
-                lookupCredentials(
-                        StandardCredentials.class,
-                        context,
-                        ACL.SYSTEM,
-                        URIRequirementBuilder.fromUri(url).build()),
-                CredentialsMatchers.withId(apiTokenId));
-        if (credentials != null) {
-            if (credentials instanceof GitLabApiToken) {
-                return ((GitLabApiToken) credentials).getApiToken().getPlainText();
-            }
-            if (credentials instanceof StringCredentials) {
-                return ((StringCredentials) credentials).getSecret().getPlainText();
-            }
-        }
-        throw new IllegalStateException("No credentials found for credentialsId: " + apiTokenId);
-    }
-
     protected GitLabConnection readResolve() {
         if (connectionTimeout == null || readTimeout == null) {
             return new GitLabConnection(

diff --git a/src/main/java/com/dabsquared/gitlabjenkins/connection/GitlabCredentialResolver.java b/src/main/java/com/dabsquared/gitlabjenkins/connection/GitlabCredentialResolver.java
@@ -0,0 +1,38 @@
+package com.dabsquared.gitlabjenkins.connection;
+
+import hudson.model.Item;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
+
+public class GitlabCredentialResolver {
+
+    private Item item;
+    private String credentialsId;
+
+    public GitlabCredentialResolver() {}
+
+    public GitlabCredentialResolver(Item item, String credentialsId) {
+        this.item = item;
+        this.credentialsId = credentialsId;
+    }
+
+    @Restricted(NoExternalUse.class)
+    public Item getItem() {
+        return item;
+    }
+
+    @Restricted(NoExternalUse.class)
+    public void setItem(Item item) {
+        this.item = item;
+    }
+
+    @Restricted(NoExternalUse.class)
+    public String getCredentialsId() {
+        return credentialsId;
+    }
+
+    @Restricted(NoExternalUse.class)
+    public void setCredentialsId(String credentialsId) {
+        this.credentialsId = credentialsId;
+    }
+}
diff --git a/src/main/java/com/dabsquared/gitlabjenkins/gitlab/api/GitLabClientBuilder.java b/src/main/java/com/dabsquared/gitlabjenkins/gitlab/api/GitLabClientBuilder.java
@@ -2,6 +2,7 @@
 
 import static java.util.Collections.sort;
 
+import com.dabsquared.gitlabjenkins.connection.GitlabCredentialResolver;
 import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.ExtensionPoint;
 import java.io.Serializable;
@@ -47,7 +48,11 @@ public final String id() {
 
     @NonNull
     public abstract GitLabClient buildClient(
-            String url, String token, boolean ignoreCertificateErrors, int connectionTimeout, int readTimeout);
+            String url,
+            GitlabCredentialResolver credentialResolver,
+            boolean ignoreCertificateErrors,
+            int connectionTimeout,
+            int readTimeout);
 
     @Override
     public final int compareTo(@NonNull GitLabClientBuilder other) {

diff --git a/...main/java/com/dabsquared/gitlabjenkins/gitlab/api/impl/AutodetectGitLabClientBuilder.java b/...main/java/com/dabsquared/gitlabjenkins/gitlab/api/impl/AutodetectGitLabClientBuilder.java
@@ -1,5 +1,6 @@
 package com.dabsquared.gitlabjenkins.gitlab.api.impl;
 
+import com.dabsquared.gitlabjenkins.connection.GitlabCredentialResolver;
 import com.dabsquared.gitlabjenkins.gitlab.api.GitLabClient;
 import com.dabsquared.gitlabjenkins.gitlab.api.GitLabClientBuilder;
 import edu.umd.cs.findbugs.annotations.NonNull;
@@ -19,10 +20,14 @@ public AutodetectGitLabClientBuilder() {
     @Override
     @NonNull
     public GitLabClient buildClient(
-            String url, String token, boolean ignoreCertificateErrors, int connectionTimeout, int readTimeout) {
+            String url,
+            GitlabCredentialResolver resolver,
+            boolean ignoreCertificateErrors,
+            int connectionTimeout,
+            int readTimeout) {
         Collection<GitLabClientBuilder> candidates = new ArrayList<>(getAllGitLabClientBuilders());
         candidates.remove(this);
         return new AutodetectingGitLabClient(
-                candidates, url, token, ignoreCertificateErrors, connectionTimeout, readTimeout);
+                candidates, url, resolver, ignoreCertificateErrors, connectionTimeout, readTimeout);
     }
 }
diff --git a/src/main/java/com/dabsquared/gitlabjenkins/gitlab/api/impl/AutodetectingGitLabClient.java b/src/main/java/com/dabsquared/gitlabjenkins/gitlab/api/impl/AutodetectingGitLabClient.java
@@ -1,5 +1,6 @@
 package com.dabsquared.gitlabjenkins.gitlab.api.impl;
 
+import com.dabsquared.gitlabjenkins.connection.GitlabCredentialResolver;
 import com.dabsquared.gitlabjenkins.gitlab.api.GitLabClient;
 import com.dabsquared.gitlabjenkins.gitlab.api.GitLabClientBuilder;
 import com.dabsquared.gitlabjenkins.gitlab.api.model.*;
@@ -11,7 +12,7 @@
 final class AutodetectingGitLabClient implements GitLabClient {
     private final Iterable<GitLabClientBuilder> builders;
     private final String url;
-    private final String token;
+    private final GitlabCredentialResolver credentialResolver;
     private final boolean ignoreCertificateErrors;
     private final int connectionTimeout;
     private final int readTimeout;
@@ -20,13 +21,13 @@ final class AutodetectingGitLabClient implements GitLabClient {
     AutodetectingGitLabClient(
             Iterable<GitLabClientBuilder> builders,
             String url,
-            String token,
+            GitlabCredentialResolver credentialResolver,
             boolean ignoreCertificateErrors,
             int connectionTimeout,
             int readTimeout) {
         this.builders = builders;
         this.url = url;
-        this.token = token;
+        this.credentialResolver = credentialResolver;
         this.ignoreCertificateErrors = ignoreCertificateErrors;
         this.connectionTimeout = connectionTimeout;
         this.readTimeout = readTimeout;
@@ -380,8 +381,8 @@ private GitLabClient autodetectOrDie() {
 
     private GitLabClient autodetect() {
         for (GitLabClientBuilder candidate : builders) {
-            GitLabClient client =
-                    candidate.buildClient(url, token, ignoreCertificateErrors, connectionTimeout, readTimeout);
+            GitLabClient client = candidate.buildClient(
+                    url, credentialResolver, ignoreCertificateErrors, connectionTimeout, readTimeout);
             try {
                 client.getCurrentUser();
                 return client;

diff --git a/src/main/java/com/dabsquared/gitlabjenkins/gitlab/api/impl/ResteasyGitLabClientBuilder.java b/src/main/java/com/dabsquared/gitlabjenkins/gitlab/api/impl/ResteasyGitLabClientBuilder.java
@@ -1,7 +1,13 @@
 package com.dabsquared.gitlabjenkins.gitlab.api.impl;
 
+import static com.cloudbees.plugins.credentials.CredentialsProvider.lookupCredentials;
 import static java.net.Proxy.Type.HTTP;
 
+import com.cloudbees.plugins.credentials.CredentialsMatchers;
+import com.cloudbees.plugins.credentials.common.StandardCredentials;
+import com.cloudbees.plugins.credentials.domains.URIRequirementBuilder;
+import com.dabsquared.gitlabjenkins.connection.GitLabApiToken;
+import com.dabsquared.gitlabjenkins.connection.GitlabCredentialResolver;
 import com.dabsquared.gitlabjenkins.gitlab.JacksonConfig;
 import com.dabsquared.gitlabjenkins.gitlab.api.GitLabClient;
 import com.dabsquared.gitlabjenkins.gitlab.api.GitLabClientBuilder;
@@ -13,6 +19,9 @@
 import hudson.ProxyConfiguration;
 import hudson.init.InitMilestone;
 import hudson.init.Initializer;
+import hudson.model.Item;
+import hudson.model.ItemGroup;
+import hudson.security.ACL;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.InetSocketAddress;
@@ -60,6 +69,7 @@
 import org.jboss.resteasy.client.jaxrs.engines.factory.ApacheHttpClient4EngineFactory;
 import org.jboss.resteasy.plugins.providers.JaxrsFormProvider;
 import org.jboss.resteasy.spi.ResteasyProviderFactory;
+import org.jenkinsci.plugins.plaincredentials.StringCredentials;
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
 
@@ -89,10 +99,14 @@ public static void setRuntimeDelegate() {
     @NonNull
     @Override
     public final GitLabClient buildClient(
-            String url, String apiToken, boolean ignoreCertificateErrors, int connectionTimeout, int readTimeout) {
+            String url,
+            GitlabCredentialResolver credentialResolver,
+            boolean ignoreCertificateErrors,
+            int connectionTimeout,
+            int readTimeout) {
         return buildClient(
                 url,
-                apiToken,
+                credentialResolver,
                 Jenkins.getActiveInstance().proxy,
                 ignoreCertificateErrors,
                 connectionTimeout,
@@ -101,7 +115,7 @@ public final GitLabClient buildClient(
 
     private GitLabClient buildClient(
             String url,
-            String apiToken,
+            GitlabCredentialResolver credentialResolver,
             ProxyConfiguration httpProxyConfig,
             boolean ignoreCertificateErrors,
             int connectionTimeout,
@@ -142,7 +156,7 @@ private GitLabClient buildClient(
                 .socketTimeout(readTimeout, TimeUnit.SECONDS)
                 .register(new JacksonFeature())
                 .register(new JacksonConfig())
-                .register(new ApiHeaderTokenFilter(apiToken))
+                .register(new ApiHeaderTokenFilter(credentialResolver, url))
                 .register(new LoggingFilter())
                 .register(new RemoveAcceptEncodingFilter())
                 .register(new JaxrsFormProvider())
@@ -165,14 +179,44 @@ private String getHost(String url) {
 
     @Priority(Priorities.HEADER_DECORATOR)
     private static class ApiHeaderTokenFilter implements ClientRequestFilter {
-        private final String gitlabApiToken;
+        private final GitlabCredentialResolver credentialResolver;
+        private final String url;
+
+        ApiHeaderTokenFilter(GitlabCredentialResolver credentialResolver, String url) {
+            this.credentialResolver = credentialResolver;
+            this.url = url;
+        }
 
-        ApiHeaderTokenFilter(String gitlabApiToken) {
-            this.gitlabApiToken = gitlabApiToken;
+        @Restricted(NoExternalUse.class)
+        private String getApiToken(GitlabCredentialResolver credentialResolver) {
+            Item item = credentialResolver.getItem();
+            ItemGroup<?> context = item != null ? item.getParent() : Jenkins.get();
+            StandardCredentials credentials = CredentialsMatchers.firstOrNull(
+                    lookupCredentials(
+                            StandardCredentials.class,
+                            context,
+                            ACL.SYSTEM,
+                            URIRequirementBuilder.fromUri(url).build()),
+                    CredentialsMatchers.withId(credentialResolver.getCredentialsId()));
+
+            if (item != null) {
+                com.cloudbees.plugins.credentials.CredentialsProvider.track(item, credentials);
+            }
+
+            if (credentials != null) {
+                if (credentials instanceof GitLabApiToken) {
+                    return ((GitLabApiToken) credentials).getApiToken().getPlainText();
+                }
+                if (credentials instanceof StringCredentials) {
+                    return ((StringCredentials) credentials).getSecret().getPlainText();
+                }
+            }
+            throw new IllegalStateException(
+                    "No credentials found for credentialsId: " + credentialResolver.getCredentialsId());
         }
 
         public void filter(ClientRequestContext requestContext) {
-            requestContext.getHeaders().putSingle(PRIVATE_TOKEN, gitlabApiToken);
+            requestContext.getHeaders().putSingle(PRIVATE_TOKEN, getApiToken(credentialResolver));
         }
     }
 

diff --git a/...test/java/com/dabsquared/gitlabjenkins/gitlab/api/impl/AutodetectingGitLabClientTest.java b/...test/java/com/dabsquared/gitlabjenkins/gitlab/api/impl/AutodetectingGitLabClientTest.java
@@ -1,15 +1,11 @@
 package com.dabsquared.gitlabjenkins.gitlab.api.impl;
 
-import static com.dabsquared.gitlabjenkins.gitlab.api.impl.TestUtility.API_TOKEN;
-import static com.dabsquared.gitlabjenkins.gitlab.api.impl.TestUtility.addGitLabApiToken;
-import static com.dabsquared.gitlabjenkins.gitlab.api.impl.TestUtility.assertApiImpl;
-import static com.dabsquared.gitlabjenkins.gitlab.api.impl.TestUtility.responseNotFound;
-import static com.dabsquared.gitlabjenkins.gitlab.api.impl.TestUtility.responseOk;
-import static com.dabsquared.gitlabjenkins.gitlab.api.impl.TestUtility.versionRequest;
+import static com.dabsquared.gitlabjenkins.gitlab.api.impl.TestUtility.*;
 import static org.junit.Assert.fail;
 import static org.mockserver.matchers.Times.exactly;
 import static org.mockserver.matchers.Times.once;
 
+import com.dabsquared.gitlabjenkins.connection.GitlabCredentialResolver;
 import com.dabsquared.gitlabjenkins.gitlab.api.GitLabClientBuilder;
 import java.io.IOException;
 import java.util.Arrays;
@@ -44,7 +40,8 @@ public void setup() throws IOException {
 
         List<GitLabClientBuilder> builders =
                 Arrays.<GitLabClientBuilder>asList(new V3GitLabClientBuilder(), new V4GitLabClientBuilder());
-        api = new AutodetectingGitLabClient(builders, gitLabUrl, API_TOKEN, true, 10, 10);
+        api = new AutodetectingGitLabClient(
+                builders, gitLabUrl, new GitlabCredentialResolver(null, API_TOKEN_ID), true, 10, 10);
 
         v3Request = versionRequest(V3GitLabApiProxy.ID);
         v4Request = versionRequest(V4GitLabApiProxy.ID);

diff --git a/src/test/java/com/dabsquared/gitlabjenkins/gitlab/api/impl/TestUtility.java b/src/test/java/com/dabsquared/gitlabjenkins/gitlab/api/impl/TestUtility.java
@@ -10,6 +10,7 @@
 import com.cloudbees.plugins.credentials.CredentialsStore;
 import com.cloudbees.plugins.credentials.SystemCredentialsProvider;
 import com.cloudbees.plugins.credentials.domains.Domain;
+import com.dabsquared.gitlabjenkins.connection.GitlabCredentialResolver;
 import com.dabsquared.gitlabjenkins.gitlab.api.GitLabClient;
 import com.dabsquared.gitlabjenkins.gitlab.api.GitLabClientBuilder;
 import hudson.util.Secret;
@@ -25,7 +26,7 @@
 
 class TestUtility {
     static final String API_TOKEN = "secret";
-    private static final String API_TOKEN_ID = "apiTokenId";
+    static final String API_TOKEN_ID = "apiTokenId";
     private static final boolean IGNORE_CERTIFICATE_ERRORS = true;
     private static final int CONNECTION_TIMEOUT = 10;
     private static final int READ_TIMEOUT = 10;
@@ -65,7 +66,12 @@ private static HttpResponse responseWithStatus(Response.Status status) {
     }
 
     static GitLabClient buildClientWithDefaults(GitLabClientBuilder clientBuilder, String url) {
-        return clientBuilder.buildClient(url, API_TOKEN, IGNORE_CERTIFICATE_ERRORS, CONNECTION_TIMEOUT, READ_TIMEOUT);
+        return clientBuilder.buildClient(
+                url,
+                new GitlabCredentialResolver(null, API_TOKEN_ID),
+                IGNORE_CERTIFICATE_ERRORS,
+                CONNECTION_TIMEOUT,
+                READ_TIMEOUT);
     }
 
     static void assertApiImpl(GitLabClient client, Class<? extends GitLabApiProxy> apiImplClass) throws Exception {