Skip to content

Commit

Permalink
Fix credentials bugs as noted in #117
Browse files Browse the repository at this point in the history 
- Actually use the new path generated as a fix in
  #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
  • Loading branch information
@ixdy
ixdy committed Jun 26, 2015
1 parent 10a5d92 commit de6083a
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 67 deletions.
4 changes: 2 additions & 2 deletions src/main/java/org/jenkinsci/plugins/ghprb/Ghprb.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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();
Expand Down
104 changes: 40 additions & 64 deletions src/main/java/org/jenkinsci/plugins/ghprb/GhprbGitHubAuth.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -164,7 +177,7 @@ public String getDisplayName() {
* @throws URISyntaxException
*/
public ListBoxModel doFillCredentialsIdItems(@AncestorInPath Item context, @QueryParameter String serverAPIUrl) throws URISyntaxException {
List<DomainRequirement> domainRequirements = getDomainReqs(serverAPIUrl);
List<DomainRequirement> domainRequirements = URIRequirementBuilder.fromUri(serverAPIUrl).build();

return new StandardListBoxModel()
.withEmptySelection()
Expand Down Expand Up @@ -222,24 +235,6 @@ public FormValidation doCreateApiToken(
}
}

private List<DomainRequirement> getDomainReqs(String serverAPIUrl) throws URISyntaxException {
List<DomainRequirement> requirements = new ArrayList<DomainRequirement>(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();
Expand All @@ -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);
Expand All @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jenkinsci/plugins/ghprb/GhprbTrigger.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down

0 comments on commit de6083a

Please sign in to comment.