Skip to content

Commit 467ed6c

Browse files
Pldi23Pldi23
committed
SECURITY-2033
2 parents 9fa69fd + a596f65 commit 467ed6c

File tree

6 files changed

+253
-3
lines changed

6 files changed

+253
-3
lines changed

‎src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketCredentials.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,11 @@
3535
import hudson.model.Queue;
3636
import hudson.model.queue.Tasks;
3737
import hudson.security.ACL;
38+
import hudson.security.AccessControlled;
3839
import hudson.util.FormValidation;
3940
import hudson.util.ListBoxModel;
4041
import jenkins.authentication.tokens.api.AuthenticationTokens;
42+
import jenkins.model.Jenkins;
4143
import jenkins.scm.api.SCMSourceOwner;
4244
import org.apache.commons.lang.StringUtils;
4345
import org.kohsuke.stapler.AncestorInPath;
@@ -80,6 +82,10 @@ static ListBoxModel fillCredentialsIdItems(
8082
@QueryParameter String serverUrl) {
8183
StandardListBoxModel result = new StandardListBoxModel();
8284
result.includeEmptyValue();
85+
AccessControlled contextToCheck = context == null ? Jenkins.get() : context;
86+
if (!contextToCheck.hasPermission(CredentialsProvider.VIEW)) {
87+
return result;
88+
}
8389
result.includeMatchingAs(
8490
context instanceof Queue.Task
8591
? Tasks.getDefaultAuthenticationOf((Queue.Task) context)
@@ -97,6 +103,8 @@ static FormValidation checkCredentialsId(
97103
@QueryParameter String value,
98104
@QueryParameter String serverUrl) {
99105
if (!value.isEmpty()) {
106+
AccessControlled contextToCheck = context == null ? Jenkins.get() : context;
107+
contextToCheck.checkPermission(CredentialsProvider.VIEW);
100108
if (CredentialsMatchers.firstOrNull(
101109
CredentialsProvider.lookupCredentials(
102110
StandardCertificateCredentials.class,

‎src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMNavigator.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,10 @@
4343
import hudson.Util;
4444
import hudson.console.HyperlinkNote;
4545
import hudson.model.Action;
46+
import hudson.model.Item;
4647
import hudson.model.TaskListener;
4748
import hudson.plugins.git.GitSCM;
49+
import hudson.security.AccessControlled;
4850
import hudson.util.FormValidation;
4951
import hudson.util.ListBoxModel;
5052
import java.io.IOException;
@@ -607,7 +609,11 @@ public boolean isServerUrlSelectable() {
607609
}
608610

609611
@SuppressWarnings("unused") // used By stapler
610-
public ListBoxModel doFillServerUrlItems() {
612+
public ListBoxModel doFillServerUrlItems(@AncestorInPath SCMSourceOwner context) {
613+
AccessControlled contextToCheck = context == null ? Jenkins.get() : context;
614+
if (!contextToCheck.hasPermission(Item.CONFIGURE)) {
615+
return new ListBoxModel();
616+
}
611617
return BitbucketEndpointConfiguration.get().getEndpointItems();
612618
}
613619

‎src/main/java/com/cloudbees/jenkins/plugins/bitbucket/BitbucketSCMSource.java‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
import hudson.model.TaskListener;
6262
import hudson.plugins.git.GitSCM;
6363
import hudson.scm.SCM;
64+
import hudson.security.AccessControlled;
6465
import hudson.util.FormFillFailure;
6566
import hudson.util.FormValidation;
6667
import hudson.util.ListBoxModel;
@@ -122,6 +123,7 @@
122123
import org.kohsuke.stapler.DataBoundConstructor;
123124
import org.kohsuke.stapler.DataBoundSetter;
124125
import org.kohsuke.stapler.QueryParameter;
126+
import org.kohsuke.stapler.interceptor.RequirePOST;
125127

126128
/**
127129
* SCM source implementation for Bitbucket.
@@ -1203,7 +1205,9 @@ public FormValidation doCheckCredentialsId(@CheckForNull @AncestorInPath SCMSour
12031205
}
12041206

12051207
@SuppressWarnings("unused") // used By stapler
1206-
public static FormValidation doCheckServerUrl(@QueryParameter String value) {
1208+
public static FormValidation doCheckServerUrl(@AncestorInPath SCMSourceOwner context, @QueryParameter String value) {
1209+
AccessControlled contextToCheck = context == null ? Jenkins.get() : context;
1210+
contextToCheck.checkPermission(Item.CONFIGURE);
12071211
if (BitbucketEndpointConfiguration.get().findEndpoint(value) == null) {
12081212
return FormValidation.error("Unregistered Server: " + value);
12091213
}
@@ -1216,7 +1220,11 @@ public boolean isServerUrlSelectable() {
12161220
}
12171221

12181222
@SuppressWarnings("unused") // used By stapler
1219-
public ListBoxModel doFillServerUrlItems() {
1223+
public ListBoxModel doFillServerUrlItems(@AncestorInPath SCMSourceOwner context) {
1224+
AccessControlled contextToCheck = context == null ? Jenkins.get() : context;
1225+
if (!contextToCheck.hasPermission(Item.CONFIGURE)) {
1226+
return new ListBoxModel();
1227+
}
12201228
return BitbucketEndpointConfiguration.get().getEndpointItems();
12211229
}
12221230

@@ -1226,6 +1234,7 @@ public ListBoxModel doFillCredentialsIdItems(@AncestorInPath SCMSourceOwner cont
12261234
}
12271235

12281236
@SuppressWarnings("unused") // used By stapler
1237+
@RequirePOST
12291238
public ListBoxModel doFillRepositoryItems(@AncestorInPath SCMSourceOwner context,
12301239
@QueryParameter String serverUrl,
12311240
@QueryParameter String credentialsId,

‎src/main/java/com/cloudbees/jenkins/plugins/bitbucket/endpoints/BitbucketCloudEndpoint.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,11 @@
3131
import hudson.Extension;
3232
import hudson.util.FormValidation;
3333
import java.util.List;
34+
import jenkins.model.Jenkins;
3435
import org.kohsuke.accmod.Restricted;
3536
import org.kohsuke.accmod.restrictions.NoExternalUse;
3637
import org.kohsuke.stapler.DataBoundConstructor;
38+
import org.kohsuke.stapler.verb.POST;
3739

3840
/**
3941
* Represents <a href="https://bitbucket.org">Bitbucket Cloud</a>.
@@ -152,6 +154,7 @@ public String getDisplayName() {
152154
}
153155

154156
public FormValidation doShowStats() {
157+
Jenkins.get().checkPermission(Jenkins.MANAGE);
155158
List<String> stats = BitbucketCloudApiClient.stats();
156159
StringBuilder builder = new StringBuilder();
157160
for (String stat : stats) {
@@ -160,7 +163,9 @@ public FormValidation doShowStats() {
160163
return FormValidation.okWithMarkup(builder.toString());
161164
}
162165

166+
@POST
163167
public FormValidation doClear() {
168+
Jenkins.get().checkPermission(Jenkins.MANAGE);
164169
BitbucketCloudApiClient.clearCaches();
165170
return FormValidation.ok("Caches cleared");
166171
}

‎src/test/java/com/cloudbees/jenkins/plugins/bitbucket/Security2033Test.java‎

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
package com.cloudbees.jenkins.plugins.bitbucket;
2+
3+
import com.cloudbees.jenkins.plugins.bitbucket.endpoints.BitbucketCloudEndpoint;
4+
import com.cloudbees.plugins.credentials.CredentialsScope;
5+
import com.cloudbees.plugins.credentials.SystemCredentialsProvider;
6+
import com.cloudbees.plugins.credentials.common.StandardUsernamePasswordCredentials;
7+
import com.cloudbees.plugins.credentials.impl.UsernamePasswordCredentialsImpl;
8+
import com.gargoylesoftware.htmlunit.Page;
9+
import hudson.model.Item;
10+
import hudson.model.User;
11+
import hudson.security.ACL;
12+
import hudson.security.ACLContext;
13+
import hudson.util.ListBoxModel;
14+
import java.io.IOException;
15+
import java.net.HttpURLConnection;
16+
import jenkins.model.Jenkins;
17+
import org.hamcrest.CoreMatchers;
18+
import org.jenkinsci.plugins.workflow.multibranch.WorkflowMultiBranchProject;
19+
import org.junit.Before;
20+
import org.junit.Rule;
21+
import org.junit.Test;
22+
import org.jvnet.hudson.test.Issue;
23+
import org.jvnet.hudson.test.JenkinsRule;
24+
import org.jvnet.hudson.test.MockAuthorizationStrategy;
25+
26+
import static org.hamcrest.CoreMatchers.not;
27+
import static org.hamcrest.MatcherAssert.assertThat;
28+
import static org.hamcrest.Matchers.containsString;
29+
import static org.hamcrest.Matchers.empty;
30+
import static org.hamcrest.Matchers.hasSize;
31+
import static org.hamcrest.Matchers.is;
32+
import static org.junit.Assert.fail;
33+
34+
public class Security2033Test {
35+
36+
private static final String PROJECT_NAME = "p";
37+
private static final String NOT_AUTHORIZED_USER = "userNoPermission";
38+
private static final String SERVER_URL = "server.url";
39+
40+
@Rule
41+
public JenkinsRule j = new JenkinsRule();
42+
43+
private WorkflowMultiBranchProject pr;
44+
45+
@Before
46+
public void setup() throws Exception {
47+
pr = j.jenkins.createProject(WorkflowMultiBranchProject.class, PROJECT_NAME);
48+
setUpAuthorization();
49+
initCredentials();
50+
}
51+
52+
@Issue("SECURITY-2033")
53+
@Test
54+
public void doFillCredentialsIdItemsSCMSourceWhenUserWithoutCredentialsViewPermissionThenListNotPopulated() {
55+
BitbucketSCMSource.DescriptorImpl descriptor = (BitbucketSCMSource.DescriptorImpl) Jenkins.get().getDescriptorOrDie(BitbucketSCMSource.class);
56+
try (ACLContext aclContext = ACL.as(User.getOrCreateByIdOrFullName(NOT_AUTHORIZED_USER))) {
57+
ListBoxModel actual = descriptor.doFillCredentialsIdItems(pr, SERVER_URL);
58+
ListBoxModel expected = new ListBoxModel(new ListBoxModel.Option("- none -", ""));
59+
assertListBoxModel(actual, expected);
60+
}
61+
}
62+
63+
@Issue("SECURITY-2033")
64+
@Test
65+
public void doFillCredentialsIdItemsSCMNavigatorWhenUserWithoutCredentialsViewPermissionThenListNotPopulated() {
66+
BitbucketSCMNavigator.DescriptorImpl descriptor = (BitbucketSCMNavigator.DescriptorImpl) Jenkins.get().getDescriptorOrDie(BitbucketSCMNavigator.class);
67+
try (ACLContext aclContext = ACL.as(User.getOrCreateByIdOrFullName(NOT_AUTHORIZED_USER))) {
68+
ListBoxModel actual = descriptor.doFillCredentialsIdItems(pr, SERVER_URL);
69+
ListBoxModel expected = new ListBoxModel(new ListBoxModel.Option("- none -", ""));
70+
assertListBoxModel(actual, expected);
71+
}
72+
}
73+
74+
@Issue("SECURITY-2033")
75+
@Test
76+
public void doCheckCredentialsIdSCMNavigatorWhenUserWithoutCredentialsViewPermissionThenReturnForbiddenStatus() {
77+
BitbucketSCMNavigator.DescriptorImpl descriptor = (BitbucketSCMNavigator.DescriptorImpl) Jenkins.get().getDescriptorOrDie(BitbucketSCMNavigator.class);
78+
try (ACLContext aclContext = ACL.as(User.getOrCreateByIdOrFullName(NOT_AUTHORIZED_USER))) {
79+
descriptor.doCheckCredentialsId(pr, SERVER_URL, "nonEmpty");
80+
fail("Should fail with AccessDeniedException2");
81+
} catch (Exception accessDeniedException2) {
82+
assertThat(accessDeniedException2.getMessage(), is(NOT_AUTHORIZED_USER + " is missing the Credentials/View permission"));
83+
}
84+
}
85+
86+
@Issue("SECURITY-2033")
87+
@Test
88+
public void doCheckCredentialsIdSCMSourceWhenUserWithoutCredentialsViewPermissionThenReturnForbiddenStatus() {
89+
BitbucketSCMSource.DescriptorImpl descriptor = (BitbucketSCMSource.DescriptorImpl) Jenkins.get().getDescriptorOrDie(BitbucketSCMSource.class);
90+
try (ACLContext aclContext = ACL.as(User.getOrCreateByIdOrFullName(NOT_AUTHORIZED_USER))) {
91+
descriptor.doCheckCredentialsId(pr, SERVER_URL, "nonEmpty");
92+
fail("Should fail with AccessDeniedException2 but not");
93+
} catch (Exception accessDeniedException2) {
94+
assertThat(accessDeniedException2.getMessage(), is(NOT_AUTHORIZED_USER + " is missing the Credentials/View permission"));
95+
}
96+
}
97+
98+
@Issue("SECURITY-2033")
99+
@Test
100+
public void doFillServerUrlItemsSCMNavigatorWhenUserWithoutPermissionThenReturnEmptyList() {
101+
BitbucketSCMNavigator.DescriptorImpl descriptor = (BitbucketSCMNavigator.DescriptorImpl) Jenkins.get().getDescriptorOrDie(BitbucketSCMNavigator.class);
102+
try (ACLContext aclContext = ACL.as(User.getOrCreateByIdOrFullName(NOT_AUTHORIZED_USER))) {
103+
ListBoxModel actual = descriptor.doFillServerUrlItems(pr);
104+
assertThat(actual, is(empty()));
105+
}
106+
}
107+
108+
@Issue("SECURITY-2033")
109+
@Test
110+
public void doFillServerUrlItemsSCMSourceWhenUserWithoutPermissionThenReturnEmptyList() {
111+
BitbucketSCMSource.DescriptorImpl descriptor = (BitbucketSCMSource.DescriptorImpl) Jenkins.get().getDescriptorOrDie(BitbucketSCMSource.class);
112+
try (ACLContext aclContext = ACL.as(User.getOrCreateByIdOrFullName(NOT_AUTHORIZED_USER))) {
113+
ListBoxModel actual = descriptor.doFillServerUrlItems(pr);
114+
assertThat(actual, is(empty()));
115+
}
116+
}
117+
118+
@Issue("SECURITY-2033")
119+
@Test
120+
public void doCheckServerUrlWhenUserWithoutPermissionThenReturnForbiddenStatus() {
121+
try (ACLContext aclContext = ACL.as(User.getOrCreateByIdOrFullName(NOT_AUTHORIZED_USER))) {
122+
BitbucketSCMSource.DescriptorImpl.doCheckServerUrl(pr, SERVER_URL);
123+
fail("Should fail with AccessDeniedException2");
124+
} catch (Exception accessDeniedException2) {
125+
assertThat(accessDeniedException2.getMessage(), is(NOT_AUTHORIZED_USER + " is missing the Job/Configure permission"));
126+
}
127+
}
128+
129+
@Issue("SECURITY-2033")
130+
@Test
131+
public void doShowStatsWhenUserWithoutAdminPermissionThenReturnForbiddenStatus() {
132+
BitbucketCloudEndpoint.DescriptorImpl descriptor = (BitbucketCloudEndpoint.DescriptorImpl) Jenkins.get().getDescriptorOrDie(BitbucketCloudEndpoint.class);
133+
try (ACLContext aclContext = ACL.as(User.getOrCreateByIdOrFullName(NOT_AUTHORIZED_USER))) {
134+
descriptor.doShowStats();
135+
fail("Should fail with AccessDeniedException2");
136+
} catch (Exception accessDeniedException2) {
137+
assertThat(accessDeniedException2.getMessage(), is(NOT_AUTHORIZED_USER + " is missing the Overall/Administer permission"));
138+
}
139+
}
140+
141+
@Issue("SECURITY-2033")
142+
@Test
143+
public void doClearWhenUserWithoutAdminPermissionThenReturnForbiddenStatus() {
144+
BitbucketCloudEndpoint.DescriptorImpl descriptor = (BitbucketCloudEndpoint.DescriptorImpl) Jenkins.get().getDescriptorOrDie(BitbucketCloudEndpoint.class);
145+
try (ACLContext aclContext = ACL.as(User.getOrCreateByIdOrFullName(NOT_AUTHORIZED_USER))) {
146+
descriptor.doClear();
147+
fail("Should fail with AccessDeniedException2");
148+
} catch (Exception accessDeniedException2) {
149+
assertThat(accessDeniedException2.getMessage(), is(NOT_AUTHORIZED_USER + " is missing the Overall/Administer permission"));
150+
}
151+
}
152+
153+
@Issue("SECURITY-2033")
154+
@Test
155+
public void doClearWhenInvokedUsingGetMethodThenResourceNotFound() throws Exception {
156+
JenkinsRule.WebClient webClient = j .createWebClient().withThrowExceptionOnFailingStatusCode(false);
157+
webClient.login(NOT_AUTHORIZED_USER);
158+
Page page = webClient.goTo("job/" + PROJECT_NAME +"/descriptorByName/com.cloudbees.jenkins.plugins.bitbucket.endpoints.BitbucketCloudEndpoint/clear");
159+
160+
assertThat(page.getWebResponse().getStatusCode(), is(HttpURLConnection.HTTP_NOT_FOUND));
161+
assertThat(page.getWebResponse().getContentAsString(), containsString("Stapler processed this HTTP request as follows, but couldn't find the resource to consume the request"));
162+
}
163+
164+
private void initCredentials() throws IOException {
165+
StandardUsernamePasswordCredentials key = new UsernamePasswordCredentialsImpl(CredentialsScope.GLOBAL, "id", "desc", "username", "pass");
166+
SystemCredentialsProvider.getInstance().getCredentials().add(key);
167+
168+
SystemCredentialsProvider.getInstance().save();
169+
}
170+
171+
private void setUpAuthorization() {
172+
j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
173+
j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy()
174+
.grant(Jenkins.READ, Item.READ).everywhere().to(NOT_AUTHORIZED_USER));
175+
}
176+
177+
private static void assertListBoxModel(ListBoxModel actual, ListBoxModel expected) {
178+
assertThat(actual, CoreMatchers.is(not(empty())));
179+
assertThat(actual, hasSize(expected.size()));
180+
assertThat(actual.get(0).name, CoreMatchers.is(expected.get(0).name));
181+
assertThat(actual.get(0).value, CoreMatchers.is(expected.get(0).value));
182+
}
183+
}

‎src/test/java/com/cloudbees/jenkins/plugins/bitbucket/Security2467Test.java‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
package com.cloudbees.jenkins.plugins.bitbucket;
2+
3+
import com.gargoylesoftware.htmlunit.html.HtmlPage;
4+
import java.net.HttpURLConnection;
5+
import jenkins.model.Jenkins;
6+
import org.jenkinsci.plugins.workflow.multibranch.WorkflowMultiBranchProject;
7+
import org.junit.Rule;
8+
import org.junit.Test;
9+
import org.jvnet.hudson.test.Issue;
10+
import org.jvnet.hudson.test.JenkinsRule;
11+
import org.jvnet.hudson.test.MockAuthorizationStrategy;
12+
13+
import static org.hamcrest.MatcherAssert.assertThat;
14+
import static org.hamcrest.Matchers.containsString;
15+
import static org.hamcrest.Matchers.is;
16+
17+
public class Security2467Test {
18+
19+
@Rule
20+
public JenkinsRule j = new JenkinsRule();
21+
22+
@Issue("SECURITY-2467")
23+
@Test
24+
public void doFillRepositoryItemsWhenInvokedUsingGetMethodThenReturnMethodNotAllowed() throws Exception {
25+
String admin = "Admin";
26+
String projectName = "p";
27+
WorkflowMultiBranchProject pr = j.jenkins.createProject(WorkflowMultiBranchProject.class, projectName);
28+
j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
29+
j.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy().
30+
grant(Jenkins.ADMINISTER).everywhere().to(admin));
31+
32+
JenkinsRule.WebClient webClient = j.createWebClient().withThrowExceptionOnFailingStatusCode(false);
33+
webClient.login(admin);
34+
HtmlPage htmlPage = webClient.goTo("job/" + projectName +"/descriptorByName/com.cloudbees.jenkins.plugins.bitbucket.BitbucketSCMSource/fillRepositoryItems?serverUrl=http://hacker:9000&credentialsId=ID_Admin&repoOwner=admin");
35+
36+
assertThat(htmlPage.getWebResponse().getStatusCode(), is(HttpURLConnection.HTTP_BAD_METHOD));
37+
assertThat(htmlPage.getWebResponse().getContentAsString(), containsString("This URL requires POST"));
38+
}
39+
}

0 commit comments

Comments
 (0)