Skip to content

Commit

Permalink
[SECURITY-1961] add permission check for creating servers
Browse files Browse the repository at this point in the history
Signed-off-by: olivier lamy <olamy@apache.org>
  • Loading branch information
olamy committed Sep 11, 2020
1 parent 966da5a commit 659a66a
Show file tree
Hide file tree
Showing 9 changed files with 247 additions and 15 deletions.
6 changes: 4 additions & 2 deletions acceptance-tests/pom.xml
Expand Up @@ -12,6 +12,8 @@
<maven.compiler.target>1.8</maven.compiler.target>
<phantomjs.binary.path>./target/phantomjs-maven-plugin/phantomjs-2.1.1-linux-x86_64/bin/phantomjs</phantomjs.binary.path>
<argLine>-Djdk.net.URLClassPath.disableClassPathURLCheck=true</argLine>
<slf4j.version>1.7.25</slf4j.version>
<selenium.version>3.141.59</selenium.version> <!-- 3.141.59 4.0.0-alpha-6 -->
</properties>


Expand Down Expand Up @@ -45,12 +47,12 @@
<dependency>
<groupId>org.seleniumhq.selenium</groupId>
<artifactId>selenium-java</artifactId>
<version>3.141.59</version>
<version>${selenium.version}</version>
</dependency>
<dependency>
<groupId>org.seleniumhq.selenium</groupId>
<artifactId>selenium-firefox-driver</artifactId>
<version>3.141.59</version>
<version>${selenium.version}</version>
</dependency>
<!-- begin HttpRequest helper -->
<dependency>
Expand Down
6 changes: 6 additions & 0 deletions blueocean-bitbucket-pipeline/pom.xml
Expand Up @@ -70,6 +70,12 @@
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>matrix-auth</artifactId>
<scope>test</scope>
</dependency>

<!-- begin HttpRequest helper -->
<dependency>
<groupId>io.jenkins.blueocean</groupId>
Expand Down
Expand Up @@ -4,7 +4,8 @@
import com.cloudbees.jenkins.plugins.bitbucket.endpoints.BitbucketEndpointConfiguration;
import com.google.common.base.Function;
import com.google.common.collect.Iterators;
import com.google.common.collect.Lists;
import hudson.model.Item;
import hudson.model.User;
import hudson.security.ACL;
import io.jenkins.blueocean.blueocean_bitbucket_pipeline.Messages;
import io.jenkins.blueocean.commons.ErrorMessage;
Expand All @@ -13,6 +14,7 @@
import io.jenkins.blueocean.rest.hal.Link;
import io.jenkins.blueocean.rest.impl.pipeline.scm.ScmServerEndpoint;
import io.jenkins.blueocean.rest.impl.pipeline.scm.ScmServerEndpointContainer;
import jenkins.model.Jenkins;
import net.sf.json.JSONObject;
import org.acegisecurity.context.SecurityContext;
import org.acegisecurity.context.SecurityContextHolder;
Expand All @@ -22,6 +24,7 @@
import org.slf4j.LoggerFactory;

import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;

/**
Expand All @@ -42,7 +45,14 @@ public Link getLink() {

@Override
public ScmServerEndpoint create(JSONObject request) {
List<ErrorMessage.Error> errors = Lists.newLinkedList();

try {
Jenkins.get().checkPermission(Item.CREATE);
} catch (Exception e) {
throw new ServiceException.ForbiddenException("User does not have permission to create repository", e);
}

List<ErrorMessage.Error> errors = new LinkedList<>();

// Validate name
final String name = (String) request.get(ScmServerEndpoint.NAME);
Expand Down
@@ -0,0 +1,96 @@
package io.jenkins.blueocean.blueocean_bitbucket_pipeline.server;

import com.google.common.collect.ImmutableMap;
import com.mashape.unirest.http.HttpResponse;
import hudson.model.Item;
import hudson.model.User;
import hudson.security.FullControlOnceLoggedInAuthorizationStrategy;
import hudson.security.GlobalMatrixAuthorizationStrategy;
import hudson.security.HudsonPrivateSecurityRealm;
import io.jenkins.blueocean.rest.impl.pipeline.PipelineBaseTest;
import io.jenkins.blueocean.rest.impl.pipeline.scm.ScmServerEndpoint;
import jenkins.model.Jenkins;
import org.junit.Before;
import org.junit.Test;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

/**
* @author Vivek Pandey
*/
public class BitbucketServerEndpointSecuredTest
extends PipelineBaseTest {
private static final String URL = "/organizations/jenkins/scm/bitbucket-server/servers/";
User readUser, writeUser;

@Before
public void setupSecurity() throws Exception {
HudsonPrivateSecurityRealm realm = new HudsonPrivateSecurityRealm(true);
readUser = realm.createAccount("read_user", "pacific_ale");
writeUser = realm.createAccount("write_user", "pale_ale");
j.jenkins.setSecurityRealm(realm);
GlobalMatrixAuthorizationStrategy as = new GlobalMatrixAuthorizationStrategy();
j.jenkins.setAuthorizationStrategy(as);

as.add( Jenkins.READ, (String)Jenkins.ANONYMOUS.getPrincipal());

{
as.add(Jenkins.READ, readUser.getId());
}
{
as.add( Item.BUILD, writeUser.getId());
as.add(Item.CREATE, writeUser.getId());
as.add(Item.CONFIGURE, writeUser.getId());
}
this.crumb = getCrumb(j.jenkins );
}

@Test
public void createAndListFailAnonymous() throws Exception {
HttpResponse<String> response = request()
.crumb( crumb )
.data(ImmutableMap.of(
"name", "My Server",
"apiUrl", "https://foo.com/git/"
))
.post(URL)
.build().asString();
assertEquals(403, response.getStatus());
}

@Test
public void createPermissionFail() throws Exception {

HttpResponse<String> response = request()
.crumb( crumb )
.jwtToken( getJwtToken( j.jenkins, readUser.getId(), "pacific_ale") )
.data(ImmutableMap.of(
"name", "My Server",
"apiUrl", "https://foo.com/git/"
))
.post(URL)
.build().asString();
assertEquals(403, response.getStatus());
assertEquals("Forbidden", response.getStatusText());
assertTrue(response.getBody().contains( "User does not have permission to create repository"));
}

@Test
public void createPermissionSuccessButMissingValue() throws Exception {
// we only test we passed authorisation check and get bad request response
HttpResponse<String> response = request()
.crumb( crumb )
.jwtToken( getJwtToken( j.jenkins, writeUser.getId(), "pale_ale") )
//.auth("read_user", "readonlymate")
.data(ImmutableMap.of(
"name", "",
"apiUrl", "https://foo.com/git/"
))
.post(URL)
.build().asString();
assertEquals(400, response.getStatus());
assertEquals("Bad Request", response.getStatusText());
assertTrue(response.getBody().contains(ScmServerEndpoint.NAME + " is required"));
}
}
6 changes: 6 additions & 0 deletions blueocean-github-pipeline/pom.xml
Expand Up @@ -62,6 +62,12 @@
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>matrix-auth</artifactId>
<scope>test</scope>
</dependency>

</dependencies>

</project>
Expand Up @@ -6,15 +6,17 @@
import com.google.common.base.Predicate;
import com.google.common.collect.Iterables;
import com.google.common.collect.Iterators;
import com.google.common.collect.Lists;
import com.google.common.collect.Ordering;
import com.google.common.hash.Hashing;
import hudson.model.Item;
import hudson.model.User;
import hudson.security.ACL;
import io.jenkins.blueocean.commons.ErrorMessage;
import io.jenkins.blueocean.commons.ServiceException;
import io.jenkins.blueocean.rest.hal.Link;
import io.jenkins.blueocean.rest.impl.pipeline.scm.ScmServerEndpoint;
import io.jenkins.blueocean.rest.impl.pipeline.scm.ScmServerEndpointContainer;
import jenkins.model.Jenkins;
import net.sf.json.JSONObject;
import org.acegisecurity.context.SecurityContext;
import org.acegisecurity.context.SecurityContextHolder;
Expand All @@ -35,6 +37,7 @@
import java.util.Comparator;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.logging.Level;
Expand All @@ -55,7 +58,13 @@ public class GithubServerContainer extends ScmServerEndpointContainer {

public @CheckForNull ScmServerEndpoint create(@JsonBody JSONObject request) {

List<ErrorMessage.Error> errors = Lists.newLinkedList();
try {
Jenkins.get().checkPermission(Item.CREATE);
} catch (Exception e) {
throw new ServiceException.ForbiddenException("User does not have permission to create repository.", e);
}

List<ErrorMessage.Error> errors = new LinkedList();

// Validate name
final String name = (String) request.get(GithubServer.NAME);
Expand Down
@@ -0,0 +1,91 @@
package io.jenkins.blueocean.blueocean_github_pipeline;

import com.google.common.collect.ImmutableMap;
import com.mashape.unirest.http.HttpResponse;
import hudson.model.Item;
import hudson.model.User;
import hudson.security.GlobalMatrixAuthorizationStrategy;
import hudson.security.HudsonPrivateSecurityRealm;
import io.jenkins.blueocean.rest.impl.pipeline.PipelineBaseTest;
import jenkins.model.Jenkins;
import org.junit.Before;
import org.junit.Test;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;

public class GithubServerSecuredTest
extends PipelineBaseTest {

private static final String URL = "/organizations/jenkins/scm/github-enterprise/servers/";
User readUser, writeUser;

@Before
public void setupSecurity() throws Exception {
HudsonPrivateSecurityRealm realm = new HudsonPrivateSecurityRealm(true);
readUser = realm.createAccount("read_user", "pacific_ale");
writeUser = realm.createAccount("write_user", "pale_ale");
j.jenkins.setSecurityRealm(realm);
GlobalMatrixAuthorizationStrategy as = new GlobalMatrixAuthorizationStrategy();
j.jenkins.setAuthorizationStrategy(as);

as.add(Jenkins.READ, (String)Jenkins.ANONYMOUS.getPrincipal());

{
as.add(Jenkins.READ, readUser.getId());
}
{
as.add(Item.BUILD, writeUser.getId());
as.add(Item.CREATE, writeUser.getId());
as.add(Item.CONFIGURE, writeUser.getId());
}
this.crumb = getCrumb(j.jenkins );
}

@Test
public void createAndListFailAnonymous() throws Exception {
HttpResponse<String> response = request()
.crumb( crumb )
.data(ImmutableMap.of(
"name", "My Server",
"apiUrl", "https://foo.com/git/"
))
.post(URL)
.build().asString();
assertEquals(403, response.getStatus());
}

@Test
public void createPermissionFail() throws Exception {

HttpResponse<String> response = request()
.crumb( crumb )
.jwtToken( getJwtToken( j.jenkins, readUser.getId(), "pacific_ale") )
.data(ImmutableMap.of(
"name", "My Server",
"apiUrl", "https://foo.com/git/"
))
.post(URL)
.build().asString();
assertEquals(403, response.getStatus());
assertEquals("Forbidden", response.getStatusText());
assertTrue(response.getBody().contains( "User does not have permission to create repository"));
}

@Test
public void createPermissionSuccessButMissingValue() throws Exception {
// we only test we passed authorisation check and get bad request response
HttpResponse<String> response = request()
.crumb( crumb )
.jwtToken( getJwtToken( j.jenkins, writeUser.getId(), "pale_ale") )
.data(ImmutableMap.of(
"name", "",
"apiUrl", "https://foo.com/git/"
))
.post(URL)
.build().asString();
assertEquals(400, response.getStatus());
assertEquals("Bad Request", response.getStatusText());
assertTrue(response.getBody().contains(GithubServer.NAME + " is required"));
}
}
Expand Up @@ -6,10 +6,13 @@
import com.google.common.base.Charsets;
import com.google.common.collect.ImmutableMap;
import com.google.common.hash.Hashing;
import com.mashape.unirest.http.exceptions.UnirestException;
import hudson.Util;
import hudson.model.Item;
import hudson.model.User;
import hudson.security.GlobalMatrixAuthorizationStrategy;
import hudson.security.HudsonPrivateSecurityRealm;
import hudson.security.Permission;
import io.jenkins.blueocean.rest.impl.pipeline.PipelineBaseTest;
import jenkins.model.Jenkins;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Rule;
Expand All @@ -35,10 +38,19 @@ public class GithubServerTest extends PipelineBaseTest {

@Before
public void createUser() throws Exception {
hudson.model.User user = User.get("alice");
user.setFullName("Alice Cooper");
token = getJwtToken(j.jenkins, "alice", "alice");
this.crumb = getCrumb( j.jenkins );
HudsonPrivateSecurityRealm realm = new HudsonPrivateSecurityRealm( true);
User writeUser = realm.createAccount("write_user", "pale_ale");
j.jenkins.setSecurityRealm(realm);
GlobalMatrixAuthorizationStrategy as = new GlobalMatrixAuthorizationStrategy();
j.jenkins.setAuthorizationStrategy(as);
as.add( Jenkins.READ, (String)Jenkins.ANONYMOUS.getPrincipal());
{
as.add( Item.BUILD, writeUser.getId());
as.add(Item.CREATE, writeUser.getId());
as.add(Item.CONFIGURE, writeUser.getId());
}
token = getJwtToken(j.jenkins, "write_user", "pale_ale");
this.crumb = getCrumb(j.jenkins);
}

@Test
Expand All @@ -47,7 +59,7 @@ public void testServerNotGithub() throws Exception {
Map resp = request()
.status(400)
.jwtToken(token)
.crumb( crumb )
.crumb(crumb)
.data(ImmutableMap.of(
"name", "My Server",
"apiUrl", getApiUrlCustomPath("/notgithub")
Expand Down Expand Up @@ -209,7 +221,7 @@ public void avoidDuplicateByUrl() throws Exception {
@Test
public void avoidDuplicateByName() throws Exception {
// Create a server
Map server = request()
request()
.status(200)
.jwtToken(token)
.crumb( crumb )
Expand Down
Expand Up @@ -92,7 +92,7 @@ public void setup() throws Exception {
}
this.baseUrl = j.jenkins.getRootUrl() + getContextPath();
this.jwtToken = getJwtToken(j.jenkins);
this.crumb = getCrumb( j.jenkins );
this.crumb = getCrumb(j.jenkins );

Unirest.setObjectMapper(new ObjectMapper() {
public <T> T readValue(String value, Class<T> valueType) {
Expand Down

0 comments on commit 659a66a

Please sign in to comment.