From 9db610662a61e6443189204731cc898dcb35c775 Mon Sep 17 00:00:00 2001
From: Jesse Glick <jglick@cloudbees.com>
Date: Fri, 25 Mar 2022 20:28:49 -0400
Subject: [PATCH 1/3] Use `CredentialsProvider.findCredentialById` whenever
 possible

---
 src/main/java/hudson/plugins/git/GitSCM.java | 23 +++++++++++++-------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/src/main/java/hudson/plugins/git/GitSCM.java b/src/main/java/hudson/plugins/git/GitSCM.java
index 085c0f5670..d70c74539f 100644
--- a/src/main/java/hudson/plugins/git/GitSCM.java
+++ b/src/main/java/hudson/plugins/git/GitSCM.java
@@ -712,7 +712,7 @@ private PollingResult compareRemoteRevisionWithImpl(Job<?, ?> project, Launcher
 
             final EnvVars environment = project instanceof AbstractProject ? GitUtils.getPollEnvironment((AbstractProject) project, workspace, launcher, listener, false) : new EnvVars();
 
-            GitClient git = createClient(listener, environment, project, Jenkins.get(), null);
+            GitClient git = createClient(listener, environment, project, project.getLastBuild(), Jenkins.get(), null);
 
             for (RemoteConfig remoteConfig : getParamExpandedRepos(lastBuild, listener)) {
                 String remote = remoteConfig.getName();
@@ -788,7 +788,7 @@ private PollingResult compareRemoteRevisionWithImpl(Job<?, ?> project, Launcher
             return BUILD_NOW;
         }
 
-        GitClient git = createClient(listener, environment, project, node, workingDirectory);
+        GitClient git = createClient(listener, environment, project, project.getLastBuild(), node, workingDirectory);
 
         if (git.hasGitRepo(false)) {
             // Repo is there - do a fetch
@@ -835,7 +835,7 @@ public GitClient createClient(TaskListener listener, EnvVars environment, Run<?,
         if (ws != null) {
             ws.mkdirs(); // ensure it exists
         }
-        return createClient(listener,environment, build.getParent(), GitUtils.workspaceToNode(workspace), ws, null);
+        return createClient(listener,environment, build.getParent(), build, GitUtils.workspaceToNode(workspace), ws, null);
     }
 
     /**
@@ -858,17 +858,17 @@ public GitClient createClient(TaskListener listener, EnvVars environment, Run<?,
         if (ws != null) {
             ws.mkdirs(); // ensure it exists
         }
-        return createClient(listener,environment, build.getParent(), GitUtils.workspaceToNode(workspace), ws, postBuildUnsupportedCommand);
+        return createClient(listener,environment, build.getParent(), build, GitUtils.workspaceToNode(workspace), ws, postBuildUnsupportedCommand);
 
     }
 
     @NonNull
-    /*package*/ GitClient createClient(TaskListener listener, EnvVars environment, Job project, Node n, FilePath ws) throws IOException, InterruptedException {
-        return createClient(listener, environment, project, n, ws, null);
+    /*package*/ GitClient createClient(TaskListener listener, EnvVars environment, Job project, Run<?, ?> build, Node n, FilePath ws) throws IOException, InterruptedException {
+        return createClient(listener, environment, project, build, n, ws, null);
     }
 
     @NonNull
-    /*package*/ GitClient createClient(TaskListener listener, EnvVars environment, Job project, Node n, FilePath ws, UnsupportedCommand postBuildUnsupportedCommand) throws IOException, InterruptedException {
+    /*package*/ GitClient createClient(TaskListener listener, EnvVars environment, Job project, Run<?, ?> build, Node n, FilePath ws, UnsupportedCommand postBuildUnsupportedCommand) throws IOException, InterruptedException {
 
         if (postBuildUnsupportedCommand == null) {
             /* UnsupportedCommand supports JGit by default */
@@ -915,7 +915,7 @@ public GitClient createClient(TaskListener listener, EnvVars environment, Run<?,
                 listener.getLogger().println("No credentials specified");
             } else {
                 String url = getParameterString(uc.getUrl(), environment);
-                StandardUsernameCredentials credentials = lookupScanCredentials(project, url, ucCredentialsId);
+                StandardUsernameCredentials credentials = lookupScanCredentials(project, build, url, ucCredentialsId);
                 if (credentials != null) {
                     c.addCredentials(url, credentials);
                     if(!isHideCredentials()) {
@@ -937,10 +937,17 @@ public GitClient createClient(TaskListener listener, EnvVars environment, Run<?,
     }
 
     private static StandardUsernameCredentials lookupScanCredentials(@CheckForNull Item project,
+                                                              @CheckForNull Run<?, ?> build,
                                                               @CheckForNull String url,
                                                               @CheckForNull String ucCredentialsId) {
         if (Util.fixEmpty(ucCredentialsId) == null) {
             return null;
+        } else if (build != null) { // preferred mode as it can call Credentials.forRun
+            return CredentialsProvider.findCredentialById(
+                    ucCredentialsId,
+                    StandardUsernameCredentials.class,
+                    build,
+                    URIRequirementBuilder.fromUri(url).build());
         } else {
             return CredentialsMatchers.firstOrNull(
                     CredentialsProvider.lookupCredentials(

From 92d3fb821aeaec5df088fac0fa80b9fd91d7e858 Mon Sep 17 00:00:00 2001
From: Jesse Glick <jglick@cloudbees.com>
Date: Sat, 26 Mar 2022 10:00:23 -0400
Subject: [PATCH 2/3] Turns out we have a `@NonNull Run` context for all of the
 `createClient` overloads. This allows methods to be simplified by dropping
 the `Job` / `Item` parameter; and allows us to drop the
 `CredentialsProvider.lookupCredentials` code branch, so that
 `CredentialsProvider.findCredentialById` can be called whenever there are
 credentials.

---
 src/main/java/hudson/plugins/git/GitSCM.java | 47 +++++++-------------
 1 file changed, 16 insertions(+), 31 deletions(-)

diff --git a/src/main/java/hudson/plugins/git/GitSCM.java b/src/main/java/hudson/plugins/git/GitSCM.java
index d70c74539f..39cf3e7f6c 100644
--- a/src/main/java/hudson/plugins/git/GitSCM.java
+++ b/src/main/java/hudson/plugins/git/GitSCM.java
@@ -42,7 +42,6 @@
 import hudson.scm.RepositoryBrowser;
 import hudson.scm.SCMDescriptor;
 import hudson.scm.SCMRevisionState;
-import hudson.security.ACL;
 import hudson.security.Permission;
 import hudson.tasks.Builder;
 import hudson.tasks.Publisher;
@@ -712,7 +711,7 @@ private PollingResult compareRemoteRevisionWithImpl(Job<?, ?> project, Launcher
 
             final EnvVars environment = project instanceof AbstractProject ? GitUtils.getPollEnvironment((AbstractProject) project, workspace, launcher, listener, false) : new EnvVars();
 
-            GitClient git = createClient(listener, environment, project, project.getLastBuild(), Jenkins.get(), null);
+            GitClient git = createClient(listener, environment, lastBuild, Jenkins.get(), null);
 
             for (RemoteConfig remoteConfig : getParamExpandedRepos(lastBuild, listener)) {
                 String remote = remoteConfig.getName();
@@ -788,7 +787,7 @@ private PollingResult compareRemoteRevisionWithImpl(Job<?, ?> project, Launcher
             return BUILD_NOW;
         }
 
-        GitClient git = createClient(listener, environment, project, project.getLastBuild(), node, workingDirectory);
+        GitClient git = createClient(listener, environment, lastBuild, node, workingDirectory);
 
         if (git.hasGitRepo(false)) {
             // Repo is there - do a fetch
@@ -829,13 +828,13 @@ private PollingResult compareRemoteRevisionWithImpl(Job<?, ?> project, Launcher
      * @throws InterruptedException when interrupted
      */
     @NonNull
-    public GitClient createClient(TaskListener listener, EnvVars environment, Run<?,?> build, FilePath workspace) throws IOException, InterruptedException {
+    public GitClient createClient(TaskListener listener, EnvVars environment, @NonNull Run<?,?> build, FilePath workspace) throws IOException, InterruptedException {
         FilePath ws = workingDirectory(build.getParent(), workspace, environment, listener);
         /* ws will be null if the node which ran the build is offline */
         if (ws != null) {
             ws.mkdirs(); // ensure it exists
         }
-        return createClient(listener,environment, build.getParent(), build, GitUtils.workspaceToNode(workspace), ws, null);
+        return createClient(listener,environment, build, GitUtils.workspaceToNode(workspace), ws, null);
     }
 
     /**
@@ -852,23 +851,23 @@ public GitClient createClient(TaskListener listener, EnvVars environment, Run<?,
      * @throws InterruptedException when interrupted
      */
     @NonNull
-    public GitClient createClient(TaskListener listener, EnvVars environment, Run<?,?> build, FilePath workspace, UnsupportedCommand postBuildUnsupportedCommand) throws IOException, InterruptedException {
+    public GitClient createClient(TaskListener listener, EnvVars environment, @NonNull Run<?,?> build, FilePath workspace, UnsupportedCommand postBuildUnsupportedCommand) throws IOException, InterruptedException {
         FilePath ws = workingDirectory(build.getParent(), workspace, environment, listener);
         /* ws will be null if the node which ran the build is offline */
         if (ws != null) {
             ws.mkdirs(); // ensure it exists
         }
-        return createClient(listener,environment, build.getParent(), build, GitUtils.workspaceToNode(workspace), ws, postBuildUnsupportedCommand);
+        return createClient(listener,environment, build, GitUtils.workspaceToNode(workspace), ws, postBuildUnsupportedCommand);
 
     }
 
     @NonNull
-    /*package*/ GitClient createClient(TaskListener listener, EnvVars environment, Job project, Run<?, ?> build, Node n, FilePath ws) throws IOException, InterruptedException {
-        return createClient(listener, environment, project, build, n, ws, null);
+    private GitClient createClient(TaskListener listener, EnvVars environment, @NonNull Run<?, ?> build, Node n, FilePath ws) throws IOException, InterruptedException {
+        return createClient(listener, environment, build, n, ws, null);
     }
 
     @NonNull
-    /*package*/ GitClient createClient(TaskListener listener, EnvVars environment, Job project, Run<?, ?> build, Node n, FilePath ws, UnsupportedCommand postBuildUnsupportedCommand) throws IOException, InterruptedException {
+    private GitClient createClient(TaskListener listener, EnvVars environment, @NonNull Run<?, ?> build, Node n, FilePath ws, UnsupportedCommand postBuildUnsupportedCommand) throws IOException, InterruptedException {
 
         if (postBuildUnsupportedCommand == null) {
             /* UnsupportedCommand supports JGit by default */
@@ -890,7 +889,7 @@ public GitClient createClient(TaskListener listener, EnvVars environment, Run<?,
                 String url = getParameterString(uc.getUrl(), environment);
                 /* If any of the extensions do not support JGit, it should not be suggested */
                 /* If the post build action does not support JGit, it should not be suggested */
-                chooser = new GitToolChooser(url, project, ucCredentialsId, gitTool, n, listener,
+                chooser = new GitToolChooser(url, build.getParent(), ucCredentialsId, gitTool, n, listener,
                                              unsupportedCommand.determineSupportForJGit() && postBuildUnsupportedCommand.determineSupportForJGit());
             }
             if (chooser != null) {
@@ -915,15 +914,13 @@ public GitClient createClient(TaskListener listener, EnvVars environment, Run<?,
                 listener.getLogger().println("No credentials specified");
             } else {
                 String url = getParameterString(uc.getUrl(), environment);
-                StandardUsernameCredentials credentials = lookupScanCredentials(project, build, url, ucCredentialsId);
+                StandardUsernameCredentials credentials = lookupScanCredentials(build, url, ucCredentialsId);
                 if (credentials != null) {
                     c.addCredentials(url, credentials);
                     if(!isHideCredentials()) {
                         listener.getLogger().printf("using credential %s%n", credentials.getId());
                     }
-                    if (project != null && project.getLastBuild() != null) {
-                        CredentialsProvider.track(project.getLastBuild(), credentials);
-                    }
+                    CredentialsProvider.track(build, credentials); // TODO unclear if findCredentialById was meant to do this in all cases
                 } else {
                     if(!isHideCredentials()) {
                         listener.getLogger().printf("Warning: CredentialId \"%s\" could not be found.%n", ucCredentialsId);
@@ -936,30 +933,18 @@ public GitClient createClient(TaskListener listener, EnvVars environment, Run<?,
         return c;
     }
 
-    private static StandardUsernameCredentials lookupScanCredentials(@CheckForNull Item project,
-                                                              @CheckForNull Run<?, ?> build,
+    private static StandardUsernameCredentials lookupScanCredentials(@NonNull Run<?, ?> build,
                                                               @CheckForNull String url,
                                                               @CheckForNull String ucCredentialsId) {
         if (Util.fixEmpty(ucCredentialsId) == null) {
             return null;
-        } else if (build != null) { // preferred mode as it can call Credentials.forRun
-            return CredentialsProvider.findCredentialById(
+        } else {
+            StandardUsernameCredentials c = CredentialsProvider.findCredentialById(
                     ucCredentialsId,
                     StandardUsernameCredentials.class,
                     build,
                     URIRequirementBuilder.fromUri(url).build());
-        } else {
-            return CredentialsMatchers.firstOrNull(
-                    CredentialsProvider.lookupCredentials(
-                            StandardUsernameCredentials.class,
-                            project,
-                            project instanceof Queue.Task
-                                    ? ((Queue.Task) project).getDefaultAuthentication()
-                                    : ACL.SYSTEM,
-                            URIRequirementBuilder.fromUri(url).build()
-                    ),
-                    CredentialsMatchers.allOf(CredentialsMatchers.withId(ucCredentialsId), GitClient.CREDENTIALS_MATCHER)
-            );
+            return GitClient.CREDENTIALS_MATCHER.matches(c) ? c : null;
         }
     }
 

From e153ba3e88b41f75a114e1c73edec3a0c28e0fe3 Mon Sep 17 00:00:00 2001
From: Jesse Glick <jglick@cloudbees.com>
Date: Sat, 26 Mar 2022 10:24:02 -0400
Subject: [PATCH 3/3] SpotBugs

---
 src/main/java/hudson/plugins/git/GitSCM.java | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/main/java/hudson/plugins/git/GitSCM.java b/src/main/java/hudson/plugins/git/GitSCM.java
index 39cf3e7f6c..aafbf2c6af 100644
--- a/src/main/java/hudson/plugins/git/GitSCM.java
+++ b/src/main/java/hudson/plugins/git/GitSCM.java
@@ -944,7 +944,7 @@ private static StandardUsernameCredentials lookupScanCredentials(@NonNull Run<?,
                     StandardUsernameCredentials.class,
                     build,
                     URIRequirementBuilder.fromUri(url).build());
-            return GitClient.CREDENTIALS_MATCHER.matches(c) ? c : null;
+            return c != null && GitClient.CREDENTIALS_MATCHER.matches(c) ? c : null;
         }
     }