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.

Sign up for GitHub

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-60116: Allow 'EXTENDED_READ' permission to select credentials #165

Merged
merged 1 commit into from
Jul 12, 2020
Merged

JENKINS-60116: Allow 'EXTENDED_READ' permission to select credentials #165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 5 additions & 2 deletions src/main/java/com/atlassian/bitbucket/jenkins/internal/scm/BitbucketSCM.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import hudson.Extension;
import hudson.FilePath;
import hudson.Launcher;
import hudson.model.Item;
import hudson.model.Job;
import hudson.model.Run;
import hudson.model.TaskListener;
Expand All @@ -27,6 +28,7 @@
import hudson.util.FormValidation;
import hudson.util.ListBoxModel;
import org.jenkinsci.Symbol;
import org.kohsuke.stapler.AncestorInPath;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.HttpResponse;
import org.kohsuke.stapler.QueryParameter;
Expand Down Expand Up @@ -413,9 +415,10 @@ public FormValidation doCheckServerId(@QueryParameter String serverId) {

@Override
@POST
public ListBoxModel doFillCredentialsIdItems(@QueryParameter String baseUrl,
public ListBoxModel doFillCredentialsIdItems(@AncestorInPath Item context,
@QueryParameter String baseUrl,
@QueryParameter String credentialsId) {
return formFill.doFillCredentialsIdItems(baseUrl, credentialsId);
return formFill.doFillCredentialsIdItems(context, baseUrl, credentialsId);
}

@Override
Expand Down
14 changes: 11 additions & 3 deletions src/main/java/com/atlassian/bitbucket/jenkins/internal/scm/BitbucketSCMSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,19 @@
import com.atlassian.bitbucket.jenkins.internal.model.BitbucketProject;
import com.atlassian.bitbucket.jenkins.internal.model.BitbucketRepository;
import com.atlassian.bitbucket.jenkins.internal.trigger.RetryingWebhookHandler;
import com.cloudbees.plugins.credentials.common.StandardListBoxModel;
import com.cloudbees.plugins.credentials.common.StandardUsernameCredentials;
import com.cloudbees.plugins.credentials.domains.URIRequirementBuilder;
import hudson.Extension;
import hudson.model.Item;
import hudson.model.TaskListener;
import hudson.model.queue.Tasks;
import hudson.plugins.git.GitTool;
import hudson.plugins.git.UserRemoteConfig;
import hudson.plugins.git.extensions.GitSCMExtension;
import hudson.plugins.git.extensions.GitSCMExtensionDescriptor;
import hudson.scm.SCM;
import hudson.security.ACL;
import hudson.util.FormValidation;
import hudson.util.ListBoxModel;
import jenkins.model.Jenkins;
Expand All @@ -27,6 +33,8 @@
import jenkins.scm.impl.UncategorizedSCMHeadCategory;
import jenkins.scm.impl.form.NamedArrayList;
import org.jenkinsci.Symbol;
import org.jenkinsci.plugins.gitclient.GitClient;
import org.kohsuke.stapler.AncestorInPath;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.HttpResponse;
import org.kohsuke.stapler.QueryParameter;
Expand Down Expand Up @@ -311,10 +319,10 @@ public FormValidation doCheckServerId(@QueryParameter String serverId) {

@Override
@POST
public ListBoxModel doFillCredentialsIdItems(@QueryParameter String baseUrl,
public ListBoxModel doFillCredentialsIdItems(@AncestorInPath Item context,
@QueryParameter String baseUrl,
@QueryParameter String credentialsId) {
Jenkins.get().checkPermission(CONFIGURE);
return formFill.doFillCredentialsIdItems(baseUrl, credentialsId);
return formFill.doFillCredentialsIdItems(context, baseUrl, credentialsId);
}

@Override
Expand Down
7 changes: 5 additions & 2 deletions src/main/java/com/atlassian/bitbucket/jenkins/internal/scm/BitbucketSCMStep.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import com.atlassian.bitbucket.jenkins.internal.model.RepositoryState;
import com.google.inject.Guice;
import hudson.Extension;
import hudson.model.Item;
import hudson.plugins.git.BranchSpec;
import hudson.plugins.git.GitTool;
import hudson.plugins.git.extensions.GitSCMExtensionDescriptor;
Expand All @@ -20,6 +21,7 @@
import hudson.util.ListBoxModel;
import org.jenkinsci.Symbol;
import org.jenkinsci.plugins.workflow.steps.scm.SCMStep;
import org.kohsuke.stapler.AncestorInPath;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.HttpResponse;
import org.kohsuke.stapler.QueryParameter;
Expand Down Expand Up @@ -261,9 +263,10 @@ public FormValidation doTestConnection(@QueryParameter String serverId,

@Override
@POST
public ListBoxModel doFillCredentialsIdItems(@QueryParameter String baseUrl,
public ListBoxModel doFillCredentialsIdItems(@AncestorInPath Item context,
@QueryParameter String baseUrl,
@QueryParameter String credentialsId) {
return formFill.doFillCredentialsIdItems(baseUrl, credentialsId);
return formFill.doFillCredentialsIdItems(context, baseUrl, credentialsId);
}

@Override
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/com/atlassian/bitbucket/jenkins/internal/scm/BitbucketScmFormFill.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
package com.atlassian.bitbucket.jenkins.internal.scm;

import hudson.model.Item;
import hudson.plugins.git.GitTool;
import hudson.plugins.git.extensions.GitSCMExtensionDescriptor;
import hudson.util.ListBoxModel;
import org.kohsuke.stapler.HttpResponse;

import javax.annotation.Nullable;
import java.util.List;

public interface BitbucketScmFormFill {

ListBoxModel doFillCredentialsIdItems(String baseUrl, String credentialsId);
ListBoxModel doFillCredentialsIdItems(@Nullable Item context, String baseUrl, String credentialsId);

HttpResponse doFillProjectNameItems(String serverId, String credentialsId, String projectName);

Expand Down
8 changes: 6 additions & 2 deletions src/main/java/com/atlassian/bitbucket/jenkins/internal/scm/BitbucketScmFormFillDelegate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import com.cloudbees.plugins.credentials.common.StandardListBoxModel;
import com.cloudbees.plugins.credentials.common.StandardUsernamePasswordCredentials;
import com.cloudbees.plugins.credentials.domains.URIRequirementBuilder;
import hudson.model.Item;
import hudson.plugins.git.GitTool;
import hudson.plugins.git.extensions.GitSCMExtensionDescriptor;
import hudson.security.ACL;
Expand All @@ -24,6 +25,7 @@
import org.jenkinsci.plugins.plaincredentials.StringCredentials;
import org.kohsuke.stapler.HttpResponse;

import javax.annotation.Nullable;
import javax.inject.Inject;
import javax.inject.Singleton;
import java.util.Collection;
Expand Down Expand Up @@ -64,9 +66,11 @@ public BitbucketScmFormFillDelegate(BitbucketClientFactoryProvider bitbucketClie
}

@Override
public ListBoxModel doFillCredentialsIdItems(String baseUrl, String credentialsId) {
public ListBoxModel doFillCredentialsIdItems(@Nullable Item context, String baseUrl, String credentialsId) {
Jenkins instance = Jenkins.get();
if (!instance.hasPermission(Jenkins.ADMINISTER)) {

if (context == null && !instance.hasPermission(Jenkins.ADMINISTER) ||
context != null && !context.hasPermission(Item.EXTENDED_READ)) {
return new StandardListBoxModel().includeCurrentValue(credentialsId);
}

Expand Down
7 changes: 5 additions & 2 deletions src/test/java/com/atlassian/bitbucket/jenkins/internal/scm/BitbucketSCMDescriptorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.atlassian.bitbucket.jenkins.internal.config.BitbucketPluginConfiguration;
import com.atlassian.bitbucket.jenkins.internal.credentials.JenkinsToBitbucketCredentials;
import com.atlassian.bitbucket.jenkins.internal.fixture.BitbucketMockJenkinsRule;
import hudson.model.Item;
import org.junit.ClassRule;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand Down Expand Up @@ -34,6 +35,8 @@ public class BitbucketSCMDescriptorTest {
private BitbucketPluginConfiguration bitbucketPluginConfiguration;
@Mock
private JenkinsToBitbucketCredentials jenkinsToBitbucketCredentials;
@Mock
private Item item;

@Test
public void testDoCheckCredentialsId() {
Expand Down Expand Up @@ -61,8 +64,8 @@ public void testDoCheckServerId() {

@Test
public void testDoFillCredentialsIdItems() {
descriptor.doFillCredentialsIdItems("myBaseUrl", "myCredentialsId");
verify(formFill).doFillCredentialsIdItems("myBaseUrl", "myCredentialsId");
descriptor.doFillCredentialsIdItems(item, "myBaseUrl", "myCredentialsId");
verify(formFill).doFillCredentialsIdItems(item, "myBaseUrl", "myCredentialsId");
}

@Test
Expand Down
9 changes: 6 additions & 3 deletions ...est/java/com/atlassian/bitbucket/jenkins/internal/scm/BitbucketSCMStepDescriptorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.atlassian.bitbucket.jenkins.internal.scm;

import hudson.model.Item;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.InjectMocks;
Expand All @@ -18,6 +19,8 @@ public class BitbucketSCMStepDescriptorTest {
private BitbucketScmFormFillDelegate formFill;
@Mock
private BitbucketScmFormValidationDelegate formValidation;
@Mock
private Item item;

@Test
public void testDoCheckCredentialsId() {
Expand Down Expand Up @@ -45,8 +48,8 @@ public void testDoCheckServerId() {

@Test
public void testDoFillCredentialsIdItems() {
descriptor.doFillCredentialsIdItems("myBaseUrl", "myCredentialsId");
verify(formFill).doFillCredentialsIdItems("myBaseUrl", "myCredentialsId");
descriptor.doFillCredentialsIdItems(item, "myBaseUrl", "myCredentialsId");
Copy link

Choose a reason for hiding this comment

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

I think here (and other POST methods we're testing) we should be mocking item privileges to ensure they're insufficient privileges are being rejected. That's a blind spot in the code base in general though, and I don't think it's needed to merge this PR.

verify(formFill).doFillCredentialsIdItems(item, "myBaseUrl", "myCredentialsId");
}

@Test
Expand All @@ -66,4 +69,4 @@ public void testDoFillServerIdItems() {
descriptor.doFillServerIdItems("myServerId");
verify(formFill).doFillServerIdItems("myServerId");
}
}
}