Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-38048] Permit credentialsId dropdowns to be used from a global configuration screen #437

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions src/main/java/hudson/plugins/git/UserRemoteConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import com.cloudbees.plugins.credentials.common.StandardCredentials;
import com.cloudbees.plugins.credentials.common.StandardListBoxModel;
import com.cloudbees.plugins.credentials.common.StandardUsernameCredentials;
import com.cloudbees.plugins.credentials.domains.URIRequirementBuilder;
import hudson.EnvVars;
import hudson.Extension;
import hudson.Util;
Expand Down Expand Up @@ -36,6 +35,7 @@

import static hudson.Util.fixEmpty;
import static hudson.Util.fixEmptyAndTrim;
import javax.annotation.CheckForNull;

@ExportedBean
public class UserRemoteConfig extends AbstractDescribableImpl<UserRemoteConfig> implements Serializable {
Expand Down Expand Up @@ -85,7 +85,8 @@ public static class DescriptorImpl extends Descriptor<UserRemoteConfig> {
public ListBoxModel doFillCredentialsIdItems(@AncestorInPath Item project,
@QueryParameter String url,
@QueryParameter String credentialsId) {
if (project == null || !project.hasPermission(Item.EXTENDED_READ)) {
if (project == null && !Jenkins.getActiveInstance().hasPermission(Jenkins.ADMINISTER) ||
project != null && !project.hasPermission(Item.EXTENDED_READ)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the diff hunk currently covered by the functional test—the one most important for the reported bug.

return new StandardListBoxModel().includeCurrentValue(credentialsId);
}
return new StandardListBoxModel()
Expand All @@ -104,7 +105,8 @@ public ListBoxModel doFillCredentialsIdItems(@AncestorInPath Item project,
public FormValidation doCheckCredentialsId(@AncestorInPath Item project,
@QueryParameter String url,
@QueryParameter String value) {
if (project == null || !project.hasPermission(Item.EXTENDED_READ)) {
if (project == null && !Jenkins.getActiveInstance().hasPermission(Jenkins.ADMINISTER) ||
project != null && !project.hasPermission(Item.EXTENDED_READ)) {
return FormValidation.ok();
}

Expand Down Expand Up @@ -149,7 +151,8 @@ public FormValidation doCheckUrl(@AncestorInPath Item item,

// Normally this permission is hidden and implied by Item.CONFIGURE, so from a view-only form you will not be able to use this check.
// (TODO under certain circumstances being granted only USE_OWN might suffice, though this presumes a fix of JENKINS-31870.)
if (item == null || !item.hasPermission(CredentialsProvider.USE_ITEM)) {
if (item == null && !Jenkins.getActiveInstance().hasPermission(Jenkins.ADMINISTER) ||
item != null && !item.hasPermission(CredentialsProvider.USE_ITEM)) {
return FormValidation.ok();
}

Expand Down Expand Up @@ -185,7 +188,7 @@ public FormValidation doCheckUrl(@AncestorInPath Item item,
return FormValidation.ok();
}

private static StandardCredentials lookupCredentials(Item project, String credentialId, String uri) {
private static StandardCredentials lookupCredentials(@CheckForNull Item project, String credentialId, String uri) {
return (credentialId == null) ? null : CredentialsMatchers.firstOrNull(
CredentialsProvider.lookupCredentials(StandardCredentials.class, project, ACL.SYSTEM,
Copy link
Member

@stephenc stephenc Sep 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather be explicit in plugins about what to fall back to if the project is null... perhaps we should discuss this and jenkinsci/credentials-plugin#68

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lookupCredentials already accepted null. Perhaps you meant to comment on a different line?

GitURIRequirementsBuilder.fromUri(uri).build()),
Expand Down
6 changes: 4 additions & 2 deletions src/main/java/jenkins/plugins/git/GitSCMSource.java
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ public String getDisplayName() {
public ListBoxModel doFillCredentialsIdItems(@AncestorInPath SCMSourceOwner context,
@QueryParameter String remote,
@QueryParameter String credentialsId) {
if (context == null || !context.hasPermission(Item.EXTENDED_READ)) {
if (context == null && !Jenkins.getActiveInstance().hasPermission(Jenkins.ADMINISTER) ||
context != null && !context.hasPermission(Item.EXTENDED_READ)) {
return new StandardListBoxModel().includeCurrentValue(credentialsId);
}
return new StandardListBoxModel()
Expand All @@ -209,7 +210,8 @@ public ListBoxModel doFillCredentialsIdItems(@AncestorInPath SCMSourceOwner cont
public FormValidation doCheckCredentialsId(@AncestorInPath SCMSourceOwner context,
@QueryParameter String url,
@QueryParameter String value) {
if (context == null || !context.hasPermission(Item.EXTENDED_READ)) {
if (context == null && !Jenkins.getActiveInstance().hasPermission(Jenkins.ADMINISTER) ||
context != null && !context.hasPermission(Item.EXTENDED_READ)) {
return FormValidation.ok();
}

Expand Down