Merge pull request #11 from recena/JENKINS-31574

[JENKINS-31574] Improved validation for Scan Credentials

src/main/java/org/jenkinsci/plugins/github_branch_source/AbstractGitHubSCMSource.java

@@ -197,26 +197,27 @@ public String getRemote() {
 
     @Override protected final void retrieve(SCMHeadObserver observer, final TaskListener listener) throws IOException, InterruptedException {
         StandardCredentials credentials = Connector.lookupScanCredentials(getOwner(), apiUri, scanCredentialsId);
-        if (credentials == null) {
-            listener.getLogger().println("No scan credentials, skipping");
-            return;
-        }
-        listener.getLogger().format("Connecting to %s using %s%n", getDescriptor().getDisplayName(), CredentialsNameProvider.name(credentials));
         GitHub github = Connector.connect(apiUri, credentials);
-        String fullName = repoOwner + "/" + repository;
-        /* TODO call GitHubBuilder withRateLimitHandler to notify listener so we do not get stuck without messages in something like
-        java.lang.Thread.State: TIMED_WAITING (sleeping)
-                at java.lang.Thread.sleep(Native Method)
-                at org.kohsuke.github.RateLimitHandler$1.onError(RateLimitHandler.java:38)
-                at org.kohsuke.github.Requester.handleApiError(Requester.java:486)
-                at org.kohsuke.github.Requester._to(Requester.java:245)
-                at org.kohsuke.github.Requester.to(Requester.java:191)
-                at org.kohsuke.github.GitHub.getRepository(GitHub.java:320)
-        */
-        final GHRepository repo = github.getRepository(fullName);
-        listener.getLogger().format("Looking up %s%n", HyperlinkNote.encodeTo(repo.getHtmlUrl().toString(), fullName));
-        doRetrieve(observer, listener, repo);
-        listener.getLogger().format("%nDone examining %s%n%n", fullName);
+        try {
+            if (credentials != null && !github.isCredentialValid()) {
+                listener.getLogger().format("Invalid scan credentials, skipping%n");
+                return;
+            }
+            if (!github.isAnonymous()) {
+                listener.getLogger().format("Connecting to %s using %s%n", getDescriptor().getDisplayName(),
+                        CredentialsNameProvider.name(credentials));
+            } else {
+                listener.getLogger().format("Connecting to %s using anonymous access%n", getDescriptor().getDisplayName());
+            }
+            String fullName = repoOwner + "/" + repository;
+            final GHRepository repo = github.getRepository(fullName);
+            listener.getLogger().format("Looking up %s%n", HyperlinkNote.encodeTo(repo.getHtmlUrl().toString(), fullName));
+            doRetrieve(observer, listener, repo);
+            listener.getLogger().format("%nDone examining %s%n%n", fullName);
+        } catch (RateLimitExceededException rle) {
+            listener.getLogger().format("%n%s%n%n", rle.getMessage());
+            throw new InterruptedException();
+        }
     }
 
     protected abstract void doRetrieve(SCMHeadObserver observer, TaskListener listener, GHRepository repo) throws IOException, InterruptedException;
@@ -327,21 +328,24 @@ public ListBoxModel doFillRepositoryItems(@AncestorInPath SCMSourceOwner context
                 return result;
             }
             try {
-                GitHub github = Connector.connect(apiUri, Connector.lookupScanCredentials(context, apiUri, scanCredentialsId));
-
-                GHMyself myself = null;
-                try {
-                    myself = github.getMyself();
-                } catch (IllegalStateException e) {
-                    LOGGER.log(Level.WARNING, e.getMessage());
-                } catch (IOException e) {
-                    LOGGER.log(Level.WARNING, e.getMessage());
-                }
-                if (myself != null && repoOwner.equals(myself.getLogin())) {
-                    for (String name : myself.getAllRepositories().keySet()) {
-                        result.add(name);
+                StandardCredentials credentials = Connector.lookupScanCredentials(context, apiUri, scanCredentialsId);
+                GitHub github = Connector.connect(apiUri, credentials);
+
+                if (!github.isAnonymous()) {
+                    GHMyself myself = null;
+                    try {
+                        myself = github.getMyself();
+                    } catch (IllegalStateException e) {
+                        LOGGER.log(Level.WARNING, e.getMessage());
+                    } catch (IOException e) {
+                        LOGGER.log(Level.WARNING, e.getMessage());
+                    }
+                    if (myself != null && repoOwner.equals(myself.getLogin())) {
+                        for (String name : myself.getAllRepositories().keySet()) {
+                            result.add(name);
+                        }
+                        return result;
                     }
-                    return result;
                 }
 
                 GHOrganization org = null;

src/main/java/org/jenkinsci/plugins/github_branch_source/Connector.java

@@ -36,13 +36,16 @@
 import hudson.Util;
 import hudson.security.ACL;
 import java.io.IOException;
+import java.net.HttpURLConnection;
 import java.util.List;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
 import jenkins.scm.api.SCMSourceOwner;
 import org.apache.commons.lang.StringUtils;
 import org.jenkinsci.plugins.gitclient.GitClient;
 import org.kohsuke.github.GitHub;
+import org.kohsuke.github.GitHubBuilder;
+import org.kohsuke.github.RateLimitHandler;
 
 /**
  * Utilities that could perhaps be moved into {@code github-api}.
@@ -68,7 +71,7 @@
     public static @Nonnull GitHub connect(@CheckForNull String apiUri, @CheckForNull StandardCredentials credentials) throws IOException {
         if (Util.fixEmptyAndTrim(apiUri) == null) {
             if (credentials == null) {
-                return GitHub.connectAnonymously();
+                return new GitHubBuilder().withRateLimitHandler(CUSTOMIZED).build();
             } else if (credentials instanceof StandardUsernamePasswordCredentials) {
                 StandardUsernamePasswordCredentials c = (StandardUsernamePasswordCredentials) credentials;
                 return GitHub.connectUsingPassword(c.getUsername(), c.getPassword().getPlainText());
@@ -108,4 +111,25 @@ private static CredentialsMatcher githubScanCredentialsMatcher() {
 
     private Connector() {}
 
+    /**
+     * Fail immediately and throw a customized exception.
+     */
+    public static final RateLimitHandler CUSTOMIZED = new RateLimitHandler() {
+
+        @Override
+        public void onError(IOException e, HttpURLConnection uc) throws IOException {
+            try {
+                long limit = Long.parseLong(uc.getHeaderField("X-RateLimit-Limit"));
+                long remaining = Long.parseLong(uc.getHeaderField("X-RateLimit-Remaining"));
+                long reset = Long.parseLong(uc.getHeaderField("X-RateLimit-Reset"));
+
+                throw new RateLimitExceededException("GitHub API rate limit exceeded", limit, remaining, reset);
+            } catch (NumberFormatException nfe) {
+                // Something wrong happened
+                throw new IOException(nfe);
+            }
+        }
+
+    };
+
 }

src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator.java

@@ -43,6 +43,8 @@
 import jenkins.scm.api.SCMSource;
 import jenkins.scm.api.SCMSourceObserver;
 import jenkins.scm.api.SCMSourceOwner;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 import org.kohsuke.github.GHMyself;
 import org.kohsuke.github.GHOrganization;
 import org.kohsuke.github.GHRepository;
@@ -103,46 +105,63 @@ public String getApiUri() {
             return;
         }
         StandardCredentials credentials = Connector.lookupScanCredentials(observer.getContext(), apiUri, scanCredentialsId);
-        if (credentials == null) {
-            listener.getLogger().format("No scan credentials, skipping%n");
-            return;
-        }
-        listener.getLogger().format("Connecting to GitHub using %s%n", CredentialsNameProvider.name(credentials));
         GitHub github = Connector.connect(apiUri, credentials);
-        GHMyself myself = null;
-        try {
-            myself = github.getMyself();
-        } catch (IllegalStateException e) {
-            // may be anonymous... ok to ignore
-        } catch (IOException e) {
-            // may be anonymous... ok to ignore
+        if (credentials != null && !github.isCredentialValid()) {
+            listener.getLogger().format("Invalid scan credentials, skipping%n");
+            return;
         }
-        if (myself != null && repoOwner.equals(myself.getLogin())) {
-            listener.getLogger().format("Looking up repositories of myself %s%n", repoOwner);
-            for (GHRepository repo : myself.listRepositories()) {
-                if (!repo.getOwnerName().equals(repoOwner)) {
-                    continue; // ignore repos in other orgs when using GHMyself
+
+        if (!github.isAnonymous()) {
+            listener.getLogger().format("Connecting to GitHub using %s%n", CredentialsNameProvider.name(credentials));
+            GHMyself myself = null;
+            try {
+                // Requires an authenticated access
+                myself = github.getMyself();
+            } catch (IOException e) {
+                // Something wrong happened, maybe java.net.ConnectException?
+            }
+            if (myself != null && repoOwner.equals(myself.getLogin())) {
+                listener.getLogger().format("Looking up repositories of myself %s%n%n", repoOwner);
+                for (GHRepository repo : myself.listRepositories()) {
+                    if (!repo.getOwnerName().equals(repoOwner)) {
+                        continue; // ignore repos in other orgs when using GHMyself
+                    }
+                    add(listener, observer, repo);
                 }
-                add(listener, observer, repo);
+                return;
             }
-            return;
+        } else {
+            listener.getLogger().format("Connecting to GitHub using anonymous access%n");
+        }
+
+        GHOrganization org = null;
+        try {
+            org = github.getOrganization(repoOwner);
+        } catch (RateLimitExceededException rle) {
+            listener.getLogger().format("%n%s%n%n", rle.getMessage());
+            throw new InterruptedException();
+        } catch (IOException e) {
+            // may be a user... ok to ignore
         }
-        GHOrganization org = github.getOrganization(repoOwner);
         if (org != null && repoOwner.equals(org.getLogin())) {
-            listener.getLogger().format("Looking up repositories of organization %s%n", repoOwner);
+            listener.getLogger().format("Looking up repositories of organization %s%n%n", repoOwner);
             for (GHRepository repo : org.listRepositories()) {
                 add(listener, observer, repo);
             }
             return;
         }
+
         GHUser user = null;
         try {
             user = github.getUser(repoOwner);
+        } catch (RateLimitExceededException rle) {
+            listener.getLogger().format("%n%s%n%n", rle.getMessage());
+            throw new InterruptedException();
         } catch (IOException e) {
-            // may be organization... ok to ignore
+            // Something wrong happened, maybe java.net.ConnectException?
         }
         if (user != null && repoOwner.equals(user.getLogin())) {
-            listener.getLogger().format("Looking up repositories of user %s%n", repoOwner);
+            listener.getLogger().format("Looking up repositories of user %s%n%n", repoOwner);
             for (GHRepository repo : user.listRepositories()) {
                 add(listener, observer, repo);
             }
@@ -182,11 +201,26 @@ private void add(TaskListener listener, SCMSourceObserver observer, GHRepository
             return new GitHubSCMNavigator("", name, "", AbstractGitHubSCMSource.AbstractGitHubSCMSourceDescriptor.SAME);
         }
 
-        public FormValidation doCheckScanCredentialsId(@QueryParameter String value) {
-            if (!value.isEmpty()) {
-                return FormValidation.ok();
+        @Restricted(NoExternalUse.class)
+        public FormValidation doCheckScanCredentialsId(@AncestorInPath SCMSourceOwner context,
+                @QueryParameter String scanCredentialsId, @QueryParameter String apiUri) {
+            if (!scanCredentialsId.isEmpty()) {
+                StandardCredentials credentials = Connector.lookupScanCredentials(context, apiUri, scanCredentialsId);
+                if (credentials == null) {
+                    FormValidation.error("Invalid credentials");
+                } else {
+                    try {
+                        GitHub connector = Connector.connect(apiUri, credentials);
+                        if (connector.isCredentialValid()) {
+                            return FormValidation.ok();
+                        }
+                    } catch (IOException e) {
+                        // ignore, never thrown
+                    }
+                }
+                return FormValidation.error("Invalid credentials");
             } else {
-                return FormValidation.warning("Credentials are required");
+                return FormValidation.warning("Credentials are recommended");
             }
         }
 

src/main/java/org/jenkinsci/plugins/github_branch_source/RateLimitExceededException.java

@@ -0,0 +1,71 @@
+/*
+ * The MIT License
+ *
+ * Copyright 2015 CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+package org.jenkinsci.plugins.github_branch_source;
+
+import java.io.IOException;
+
+public class RateLimitExceededException extends IOException {
+
+    private static final long serialVersionUID = 1L;
+
+    private long limit;
+
+    private long remaining;
+
+    private long reset;
+
+    public RateLimitExceededException() {
+        super();
+    }
+
+    public RateLimitExceededException(String msg, long limit, long remaining, long reset) {
+        super(msg);
+        this.limit = limit;
+        this.remaining = remaining;
+        this.reset = reset;
+    }
+
+    public RateLimitExceededException(Throwable cause) {
+        initCause(cause);
+    }
+
+    public RateLimitExceededException(String message, Throwable cause) {
+        super(message);
+        initCause(cause);
+    }
+
+    public long getReset() {
+        return reset;
+    }
+
+    public long getRemaining() {
+        return remaining;
+    }
+
+    public long getLimit() {
+        return limit;
+    }
+
+}