From 599aff810d3d52867c97f7608b2ed59b6cfc2722 Mon Sep 17 00:00:00 2001 From: Jeff Grafton Date: Thu, 25 Jun 2015 20:46:17 -0700 Subject: [PATCH] Fix credentials bugs as noted in jenkinsci/ghprb-plugin#117 - Actually use the new path generated as a fix in jenkinsci/ghprb-plugin#122 - Use URIRequirementBuilder instead of manually building a requirements list - Add logging around accessing credentials to debug failures, and give better error messages when something is wrong - General refactoring of a few methods to reduce code duplication --- .../org/jenkinsci/plugins/ghprb/Ghprb.java | 4 +- .../plugins/ghprb/GhprbGitHubAuth.java | 104 +++++++----------- .../jenkinsci/plugins/ghprb/GhprbTrigger.java | 2 +- 3 files changed, 43 insertions(+), 67 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/Ghprb.java b/src/main/java/org/jenkinsci/plugins/ghprb/Ghprb.java index 4de9b4b8f..0c5af83cf 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/Ghprb.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/Ghprb.java @@ -8,11 +8,11 @@ import com.cloudbees.plugins.credentials.common.StandardCredentials; import com.cloudbees.plugins.credentials.domains.Domain; import com.cloudbees.plugins.credentials.domains.DomainSpecification; -import com.cloudbees.plugins.credentials.domains.URIRequirementBuilder; import com.cloudbees.plugins.credentials.domains.HostnamePortSpecification; import com.cloudbees.plugins.credentials.domains.HostnameSpecification; import com.cloudbees.plugins.credentials.domains.PathSpecification; import com.cloudbees.plugins.credentials.domains.SchemeSpecification; +import com.cloudbees.plugins.credentials.domains.URIRequirementBuilder; import com.cloudbees.plugins.credentials.impl.UsernamePasswordCredentialsImpl; import com.coravy.hudson.plugins.github.GithubProjectProperty; @@ -411,7 +411,7 @@ private static String createCredentials(String serverAPIUrl, StandardCredentials if (StringUtils.isEmpty(path)) { path = "/"; } - specifications.add(new PathSpecification(serverUri.getPath(), null, false)); + specifications.add(new PathSpecification(path, null, false)); Domain domain = new Domain(serverUri.getHost(), "Auto generated credentials domain", specifications); CredentialsStore provider = new SystemCredentialsProvider.StoreImpl(); diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHubAuth.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHubAuth.java index cf3d0395f..6ee5e6d4b 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHubAuth.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHubAuth.java @@ -29,7 +29,6 @@ import com.cloudbees.plugins.credentials.common.StandardCredentials; import com.cloudbees.plugins.credentials.common.StandardListBoxModel; import com.cloudbees.plugins.credentials.common.StandardUsernamePasswordCredentials; -import com.cloudbees.plugins.credentials.common.UsernamePasswordCredentials; import com.cloudbees.plugins.credentials.domains.DomainRequirement; import com.cloudbees.plugins.credentials.domains.HostnamePortRequirement; import com.cloudbees.plugins.credentials.domains.HostnameRequirement; @@ -108,29 +107,43 @@ public String getId() { public String getSecret() { return secret; } - + + private static GitHubBuilder getBuilder(Item context, String serverAPIUrl, String credentialsId) { + GitHubBuilder builder = new GitHubBuilder() + .withEndpoint(serverAPIUrl) + .withConnector(new HttpConnectorWithJenkinsProxy()); + String contextName = context == null ? "(null)" : context.getFullDisplayName(); + if (StringUtils.isEmpty(credentialsId)) { + logger.log(Level.WARNING, "credentialsId not set for context {0}, using anonymous connection", contextName); + return builder; + } + + StandardCredentials credentials = Ghprb.lookupCredentials(context, credentialsId, serverAPIUrl); + if (credentials == null) { + logger.log(Level.SEVERE, "Failed to look up credentials for context {0} using id: {1}", + new Object[] { contextName, credentialsId }); + } else if (credentials instanceof StandardUsernamePasswordCredentials) { + logger.log(Level.FINEST, "Using username/password for context {0}", contextName); + StandardUsernamePasswordCredentials upCredentials = (StandardUsernamePasswordCredentials) credentials; + builder.withPassword(upCredentials.getUsername(), upCredentials.getPassword().getPlainText()); + } else if (credentials instanceof StringCredentials) { + logger.log(Level.FINEST, "Using OAuth token for context {0}", contextName); + StringCredentials tokenCredentials = (StringCredentials) credentials; + builder.withOAuthToken(tokenCredentials.getSecret().getPlainText()); + } else { + logger.log(Level.SEVERE, "Unknown credential type for context {0} using id: {1}: {2}", + new Object[] { contextName, credentialsId, credentials.getClass().getName() }); + return null; + } + return builder; + } + public GitHub getConnection(Item context) throws IOException { GitHub gh = null; - GitHubBuilder builder = new GitHubBuilder() - .withEndpoint(serverAPIUrl) - .withConnector(new HttpConnectorWithJenkinsProxy()); - - if (!StringUtils.isEmpty(credentialsId)) { - StandardCredentials credentials = CredentialsMatchers - .firstOrNull( - CredentialsProvider.lookupCredentials(StandardCredentials.class, context, - ACL.SYSTEM, URIRequirementBuilder.fromUri(serverAPIUrl).build()), - CredentialsMatchers.allOf(CredentialsMatchers.withId(credentialsId))); - - if (credentials instanceof StringCredentials) { - String accessToken = ((StringCredentials) credentials).getSecret().getPlainText(); - builder.withOAuthToken(accessToken); - } else if (credentials instanceof UsernamePasswordCredentials){ - UsernamePasswordCredentials creds = (UsernamePasswordCredentials) credentials; - String username = creds.getUsername(); - String password = creds.getPassword().getPlainText(); - builder.withPassword(username, password); - } + GitHubBuilder builder = getBuilder(context, serverAPIUrl, credentialsId); + if (builder == null) { + logger.log(Level.SEVERE, "Unable to get builder using credentials: {0}", credentialsId); + return null; } try { gh = builder.build(); @@ -164,7 +177,7 @@ public String getDisplayName() { * @throws URISyntaxException */ public ListBoxModel doFillCredentialsIdItems(@AncestorInPath Item context, @QueryParameter String serverAPIUrl) throws URISyntaxException { - List domainRequirements = getDomainReqs(serverAPIUrl); + List domainRequirements = URIRequirementBuilder.fromUri(serverAPIUrl).build(); return new StandardListBoxModel() .withEmptySelection() @@ -222,24 +235,6 @@ public FormValidation doCreateApiToken( } } - private List getDomainReqs(String serverAPIUrl) throws URISyntaxException { - List requirements = new ArrayList(2); - - URI serverUri = new URI(serverAPIUrl); - - if (serverUri.getPort() > 0) { - requirements.add(new HostnamePortRequirement(serverUri.getHost(), serverUri.getPort())); - } else { - requirements.add(new HostnameRequirement(serverUri.getHost())); - } - - requirements.add(new SchemeRequirement(serverUri.getScheme())); - if (!StringUtils.isEmpty(serverUri.getPath())) { - requirements.add(new PathRequirement(serverUri.getPath())); - } - return requirements; - } - public FormValidation doCheckServerAPIUrl(@QueryParameter String value) { if ("https://api.github.com".equals(value)) { return FormValidation.ok(); @@ -255,9 +250,9 @@ public FormValidation doCheckRepoAccess( @QueryParameter("credentialsId") final String credentialsId, @QueryParameter("repo") final String repo) { try { - GitHubBuilder builder = getBuilder(serverAPIUrl, credentialsId); + GitHubBuilder builder = getBuilder(null, serverAPIUrl, credentialsId); if (builder == null) { - return FormValidation.error("No credentials ID provided!!"); + return FormValidation.error("Unable to look up GitHub credentials using ID: " + credentialsId + "!!"); } GitHub gh = builder.build(); GHRepository repository = gh.getRepository(repo); @@ -281,32 +276,13 @@ public FormValidation doCheckRepoAccess( } } - private GitHubBuilder getBuilder(String serverAPIUrl, String credentialsId) { - GitHubBuilder builder = new GitHubBuilder() - .withEndpoint(serverAPIUrl) - .withConnector(new HttpConnectorWithJenkinsProxy()); - - StandardCredentials credentials = Ghprb.lookupCredentials(null, credentialsId, serverAPIUrl); - if (credentials instanceof StandardUsernamePasswordCredentials) { - StandardUsernamePasswordCredentials upCredentials = (StandardUsernamePasswordCredentials) credentials; - builder.withPassword(upCredentials.getUsername(), upCredentials.getPassword().getPlainText()); - - } else if (credentials instanceof StringCredentials) { - StringCredentials tokenCredentials = (StringCredentials) credentials; - builder.withOAuthToken(tokenCredentials.getSecret().getPlainText()); - } else { - return null; - } - return builder; - } - public FormValidation doTestGithubAccess( @QueryParameter("serverAPIUrl") final String serverAPIUrl, @QueryParameter("credentialsId") final String credentialsId) { try { - GitHubBuilder builder = getBuilder(serverAPIUrl, credentialsId); + GitHubBuilder builder = getBuilder(null, serverAPIUrl, credentialsId); if (builder == null) { - return FormValidation.error("No credentials ID provided!!"); + return FormValidation.error("Unable to look up GitHub credentials using ID: " + credentialsId + "!!"); } GitHub gh = builder.build(); GHMyself me = gh.getMyself(); diff --git a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java index 02a040a80..7d430ccf2 100644 --- a/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java +++ b/src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java @@ -193,7 +193,7 @@ public void run() { } if ((helper != null && helper.isProjectDisabled()) || (_project != null && _project.isDisabled())) { - logger.log(Level.FINE, "Project is disabled, ignoring trigger run call"); + logger.log(Level.FINE, "Project is disabled, ignoring trigger run call for job {0}", this.project); return; }