Skip to content

Commit

Permalink
[JENKINS-35081] Use AccessControlled for findAncestorObject. Allows b…
Browse files Browse the repository at this point in the history 
…ypassing permission checks only to system administrators.
  • Loading branch information
@ikedam
ikedam committed Jan 14, 2017
1 parent 6c1e742 commit c7c59a2
Show file tree
Hide file tree
Showing 3 changed files with 218 additions and 22 deletions.
16 changes: 7 additions & 9 deletions src/main/java/org/jenkinsci/plugins/authorizeproject/AuthorizeProjectStrategy.java
View file
Expand Up @@ -186,17 +186,15 @@ private void checkUnsecuredConfiguration() throws ObjectStreamException {
if (!ProjectQueueItemAuthenticator.isConfigured()) {
return;
}
if (Jenkins.getActiveInstance().hasPermission(Jenkins.ADMINISTER)) {
// allows any configurations by system administrators.
// It may not be allowed even if the user is an administrator of the job.
return;
}
StaplerRequest request = Stapler.getCurrentRequest();
AccessControlled context;
if (request == null) {
AccessControlled context = (request != null) ? request.findAncestorObject(AccessControlled.class) : null;
if (context == null) {
context = Jenkins.getActiveInstance();
} else {
Job<?, ?> job = request.findAncestorObject(Job.class);
context = job == null ? Jenkins.getActiveInstance() : job;
}
if (context.hasPermission(Jenkins.ADMINISTER)) {
// allows configuration by administrators
return;
}
try {
checkJobConfigurePermission(context);
Expand Down
25 changes: 19 additions & 6 deletions src/main/java/org/jenkinsci/plugins/authorizeproject/ConfigurationPermissionEnforcer.java
View file
Expand Up @@ -4,8 +4,12 @@
import hudson.model.Job;
import hudson.model.JobProperty;
import hudson.model.JobPropertyDescriptor;
import hudson.security.AccessControlled;
import jenkins.model.Jenkins;
import net.sf.json.JSONObject;

import javax.annotation.CheckForNull;

import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.DataBoundConstructor;
Expand Down Expand Up @@ -47,15 +51,24 @@ public String getDisplayName() {
@Override
public JobProperty<?> newInstance(StaplerRequest req, JSONObject formData) throws FormException {
Job<?,?> job = req.findAncestorObject(Job.class);
if (job != null) {
checkConfigurePermission(job);
}
AccessControlled context = req.findAncestorObject(AccessControlled.class);
checkConfigurePermission(job, context);
// we don't actually return a job property... just want to be called on every form submission.
return null;
}

private void checkConfigurePermission(Job<?, ?> job) {
if (job.hasPermission(Jenkins.ADMINISTER)) {
private void checkConfigurePermission(@CheckForNull Job<?, ?> job, @CheckForNull AccessControlled context) {
if (job == null) {
return;
}
if (context == null) {
// this should not happen.
context = job;
}
if (Jenkins.getActiveInstance().hasPermission(Jenkins.ADMINISTER)) {
// allows any configurations by system administrators.
// It may not be allowed even if the user is an administrator of the job,
//
return;
}
AuthorizeProjectProperty property = job.getProperty(AuthorizeProjectProperty.class);
Expand All @@ -69,7 +82,7 @@ private void checkConfigurePermission(Job<?, ?> job) {
if (strategy == null) {
return;
}
strategy.checkJobConfigurePermission(job);
strategy.checkJobConfigurePermission(context);
}
}
}
199 changes: 192 additions & 7 deletions ...g/jenkinsci/plugins/authorizeproject/strategy/SpecificUsersAuthorizationStrategyTest.java
View file
Expand Up @@ -111,12 +111,6 @@ private void prepareJobBasedSecurity() {

ProjectMatrixAuthorizationStrategy authorization = new ProjectMatrixAuthorizationStrategy();
authorization.add(Jenkins.ADMINISTER, "admin");
authorization.add(Jenkins.READ, "test1");
authorization.add(Item.READ, "test1");
authorization.add(Item.CONFIGURE, "test1");
authorization.add(Jenkins.READ, "test2");
authorization.add(Item.READ, "test2");
authorization.add(Item.CONFIGURE, "test2");

// This is required for CLI, JENKINS-12543.
authorization.add(Jenkins.READ, "anonymous");
Expand Down Expand Up @@ -621,6 +615,172 @@ public void testCliFailure() throws Exception {
}
}

@Test
public void testCliSuccessBySystemAdmin() throws Exception {
prepareSecurity();

FreeStyleProject srcProject = j.createFreeStyleProject();
srcProject.addProperty(new AuthorizeProjectProperty(new SpecificUsersAuthorizationStrategy("test1")));
srcProject.save();

// GET config.xml of srcProject (userid is set to test1)
String configXml = null;
{
CLI cli = new CLI(j.getURL());
ByteArrayOutputStream stdout = new ByteArrayOutputStream();
ByteArrayOutputStream stderr = new ByteArrayOutputStream();
int ret = cli.execute(Arrays.asList(
"get-job",
srcProject.getFullName(),
"--username",
"admin",
"--password",
"admin"
),
new NullInputStream(0),
stdout,
stderr
);
assertEquals(stderr.toString(), 0, ret);
configXml = stdout.toString();
}

// POST config.xml of srcProject (userid is set to test1) to a new project.
// This should success when the user is administrator of the system.
FreeStyleProject destProject = j.createFreeStyleProject();
destProject.save();
String projectName = destProject.getFullName();

{
CLI cli = new CLI(j.getURL());
ByteArrayOutputStream stdout = new ByteArrayOutputStream();
ByteArrayOutputStream stderr = new ByteArrayOutputStream();
int ret = cli.execute(Arrays.asList(
"update-job",
destProject.getFullName(),
"--username",
"admin",
"--password",
"admin"
),
new ByteArrayInputStream(configXml.getBytes()),
stdout,
stderr
);
assertEquals(stderr.toString(), 0, ret);
}

{
FreeStyleProject p = j.jenkins.getItemByFullName(projectName, FreeStyleProject.class);
assertNotNull(p);
AuthorizeProjectProperty prop = p.getProperty(AuthorizeProjectProperty.class);
assertNotNull(prop);
assertEquals(SpecificUsersAuthorizationStrategy.class, prop.getStrategy().getClass());
SpecificUsersAuthorizationStrategy strategy = (SpecificUsersAuthorizationStrategy)prop.getStrategy();
assertEquals("test1", strategy.getUserid());
}

j.jenkins.reload();

{
FreeStyleProject p = j.jenkins.getItemByFullName(projectName, FreeStyleProject.class);
assertNotNull(p);
AuthorizeProjectProperty prop = p.getProperty(AuthorizeProjectProperty.class);
assertNotNull(prop);
assertEquals(SpecificUsersAuthorizationStrategy.class, prop.getStrategy().getClass());
SpecificUsersAuthorizationStrategy strategy = (SpecificUsersAuthorizationStrategy)prop.getStrategy();
assertEquals("test1", strategy.getUserid());
}
}

@Test
public void testCliFailureEvenByJobAdmin() throws Exception {
prepareJobBasedSecurity();

FreeStyleProject srcProject = j.createFreeStyleProject();
srcProject.addProperty(new AuthorizeProjectProperty(new SpecificUsersAuthorizationStrategy("admin")));
{
Map<Permission, Set<String>> authMap = new HashMap<>();
authMap.put(Item.EXTENDED_READ, Sets.newHashSet("test1"));
srcProject.addProperty(new AuthorizationMatrixProperty(authMap));
}
srcProject.save();

// GET config.xml of srcProject (userid is set to admin)
String configXml = null;
{
CLI cli = new CLI(j.getURL());
ByteArrayOutputStream stdout = new ByteArrayOutputStream();
ByteArrayOutputStream stderr = new ByteArrayOutputStream();
int ret = cli.execute(Arrays.asList(
"get-job",
srcProject.getFullName(),
"--username",
"test1",
"--password",
"test1"
),
new NullInputStream(0),
stdout,
stderr
);
assertEquals(stderr.toString(), 0, ret);
configXml = stdout.toString();
}

// POST config.xml of srcProject (userid is set to test1) to a new project.
// This should fail even if test1 is a administrator of the new job.
FreeStyleProject destProject = j.createFreeStyleProject();
{
Map<Permission, Set<String>> authMap = new HashMap<>();
authMap.put(Jenkins.ADMINISTER, Sets.newHashSet("test1"));
destProject.addProperty(new AuthorizationMatrixProperty(authMap));
}
destProject.save();
String projectName = destProject.getFullName();

{
CLI cli = new CLI(j.getURL());
ByteArrayOutputStream stdout = new ByteArrayOutputStream();
ByteArrayOutputStream stderr = new ByteArrayOutputStream();
int ret = cli.execute(Arrays.asList(
"update-job",
destProject.getFullName(),
"--username",
"test1",
"--password",
"test1"
),
new ByteArrayInputStream(configXml.getBytes()),
stdout,
stderr
);
assertNotEquals(0, ret);
}

{
FreeStyleProject p = j.jenkins.getItemByFullName(projectName, FreeStyleProject.class);
assertNotNull(p);
AuthorizeProjectProperty prop = p.getProperty(AuthorizeProjectProperty.class);
assertTrue(
prop == null
|| prop.getStrategy() == null
);
}

j.jenkins.reload();

{
FreeStyleProject p = j.jenkins.getItemByFullName(projectName, FreeStyleProject.class);
assertNotNull(p);
AuthorizeProjectProperty prop = p.getProperty(AuthorizeProjectProperty.class);
assertTrue(
prop == null
|| prop.getStrategy() == null
);
}
}

@Test
public void testConfigurationAuthentication() throws Exception {
prepareSecurity();
Expand Down Expand Up @@ -838,7 +998,7 @@ public void testConfigureJobByAnotherUserIsForbidden() throws Exception {
}

@Test
public void testConfigureJobByAdminIsAllowed() throws Exception {
public void testConfigureJobBySystemAdminIsAllowed() throws Exception {
prepareJobBasedSecurity();

FreeStyleProject p = j.createFreeStyleProject();
Expand All @@ -856,4 +1016,29 @@ public void testConfigureJobByAdminIsAllowed() throws Exception {

j.submit(wc.getPage(p, "configure").getFormByName("config"));
}

@Test
public void testConfigureJobByJobAdminIsNotAllowed() throws Exception {
prepareJobBasedSecurity();

FreeStyleProject p = j.createFreeStyleProject();
p.addProperty(new AuthorizeProjectProperty(new SpecificUsersAuthorizationStrategy("test1")));
{
Map<Permission, Set<String>> authMap = new HashMap<>();
authMap.put(Item.READ, Sets.newHashSet("test1"));
authMap.put(Item.CONFIGURE, Sets.newHashSet("test1"));
authMap.put(Jenkins.ADMINISTER, Sets.newHashSet("test2"));
p.addProperty(new AuthorizationMatrixProperty(authMap));
}
p.save();

WebClient wc = j.createWebClient();
wc.login("test2", "test2");

try {
j.submit(wc.getPage(p, "configure").getFormByName("config"));
} catch (FailingHttpStatusCodeException e) {
assertEquals(403, e.getStatusCode());
}
}
}

0 comments on commit c7c59a2

Please sign in to comment.