Role Strategy Fix + Cleanup + PermissionAssert (#131)

* Move PermissionFinder to an utility package and restrict it

* Fix #99 - Now RoleStrategy uses standartized PermissionFinder

* Add logic for Permission assert

* #99  - Add tests for Role Strategy with proper coverage
This commit is contained in:
Oleg Nenashev 2018-02-26 10:03:34 +01:00 committed by Mads Nielsen
parent c5521fcf0d
commit 9b9804bb58
7 changed files with 254 additions and 47 deletions

View File

@ -256,6 +256,12 @@
<version>1.21</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>cloudbees-folder</artifactId>
<version>6.1.2</version>
<scope>test</scope>
</dependency>
<!-- https://mvnrepository.com/artifact/com.github.stefanbirkner/system-rules -->
<dependency>
<groupId>com.github.stefanbirkner</groupId>

View File

@ -1,6 +1,7 @@
package org.jenkinsci.plugins.casc.integrations.globalmatrixauth;
import hudson.security.Permission;
import org.jenkinsci.plugins.casc.util.PermissionFinder;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.DataBoundConstructor;

View File

@ -1,14 +1,20 @@
package org.jenkinsci.plugins.casc.integrations.rolebasedauth;
import com.michelin.cio.hudson.plugins.rolestrategy.Role;
import hudson.security.AccessControlled;
import hudson.security.Permission;
import org.jenkinsci.plugins.casc.util.PermissionFinder;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.DataBoundConstructor;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import java.util.regex.Pattern;
/**
* Role definition.
@ -19,20 +25,61 @@ import java.util.Set;
@Restricted(NoExternalUse.class)
public class RoleDefinition {
private final Role role;
private transient Role role;
@Nonnull
private final String name;
@CheckForNull
private final String description;
@CheckForNull
private final String pattern;
private final Set<String> permissions;
private final Set<String> assignments;
@DataBoundConstructor
public RoleDefinition(Role role, Collection<String> assignments) {
this.role = role;
public RoleDefinition(String name, String description, String pattern, Collection<String> permissions, Collection<String> assignments) {
this.name = name;
this.description = description;
this.pattern = pattern;
this.permissions = permissions != null ? new HashSet<>(permissions) : Collections.emptySet();
this.assignments = assignments != null ? new HashSet<>(assignments) : Collections.emptySet();
this.role = getRole();
}
public Role getRole() {
public final Role getRole() {
if (role == null) {
HashSet<String> resolvedIds = new HashSet<>();
for (String id : permissions) {
String resolvedId = PermissionFinder.findPermissionId(id);
if (resolvedId != null) {
resolvedIds.add(resolvedId);
} else {
throw new IllegalStateException("Cannot resolve permission for ID: " + id);
}
}
role = new Role(name, pattern, resolvedIds, description);
}
return role;
}
public String getName() {
return name;
}
public String getDescription() {
return description;
}
public String getPattern() {
return pattern;
}
public Set<String> getPermissions() {
return Collections.unmodifiableSet(permissions);
}
public Set<String> getAssignments() {
return Collections.unmodifiableSet(assignments);
}
}

View File

@ -1,8 +1,9 @@
package org.jenkinsci.plugins.casc.integrations.globalmatrixauth;
package org.jenkinsci.plugins.casc.util;
import hudson.security.Permission;
import hudson.security.PermissionGroup;
import jenkins.model.Jenkins;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import javax.annotation.CheckForNull;
import java.util.List;
@ -11,8 +12,10 @@ import java.util.regex.Pattern;
/**
* Implements lookup for {@link Permission}s.
* Created by mads on 2/9/18.
*/
@Restricted(NoExternalUse.class)
public class PermissionFinder {
/** For Matrix Auth - Title/Permission **/
@ -20,12 +23,22 @@ public class PermissionFinder {
/**
* Attempt to match a given permission to what is defined in the UI.
* TODO: Refactor this away when proper permission API is in place for C-as-C
* @param id String of the form "Title/Permission" (Look in the UI) for a particular permission
* @return a matched permission
*/
@CheckForNull
public static Permission findPermission(String id) {
final String resolvedId = findPermissionId(id);
return resolvedId != null ? Permission.fromId(resolvedId) : null;
}
/**
* Attempt to match a given permission to what is defined in the UI.
* @param id String of the form "Title/Permission" (Look in the UI) for a particular permission
* @return a matched permission ID
*/
@CheckForNull
public static String findPermissionId(String id) {
List<PermissionGroup> pgs = PermissionGroup.getAll();
Matcher m = PERMISSION_PATTERN.matcher(id);
if(m.matches()) {
@ -35,11 +48,13 @@ public class PermissionFinder {
if(pg.owner.equals(Permission.class)) {
continue;
}
//TODO: this logic uses a localizable field for resolution. It may blow up at any moment
//How do we do this properly, we want to mimic the UI as best as possible. So the logic conclusion is
//That when you want admin to be Overall/Administer you put that in. Overall being the group title...
//Name being the Permssion you want to set in the matrix.
//Name being the Permission you want to set in the matrix.
if(pg.title.toString().equals(owner)) {
return Permission.fromId(pg.owner.getName()+"."+name);
return pg.owner.getName() + "." + name;
}
}
}

View File

@ -1,12 +1,18 @@
package org.jenkinsci.plugins.casc.integrations.rolebasedauth;
import com.cloudbees.hudson.plugins.folder.Folder;
import com.michelin.cio.hudson.plugins.rolestrategy.Role;
import com.michelin.cio.hudson.plugins.rolestrategy.RoleBasedAuthorizationStrategy;
import hudson.model.Computer;
import hudson.model.FreeStyleProject;
import hudson.model.Item;
import hudson.model.User;
import hudson.security.AuthorizationStrategy;
import jenkins.model.Jenkins;
import org.jenkinsci.plugins.casc.Configurator;
import org.jenkinsci.plugins.casc.misc.ConfiguredWithCode;
import org.jenkinsci.plugins.casc.misc.JenkinsConfiguredWithCodeRule;
import static org.jenkinsci.plugins.casc.misc.PermissionAssert.*;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;
@ -50,9 +56,18 @@ public class RoleStrategyTest {
@Issue("Issue #48")
@ConfiguredWithCode("RoleStrategy1.yml")
public void shouldReadRolesCorrectly() throws Exception {
final Jenkins jenkins = Jenkins.getInstance();
j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
User admin = User.get("admin");
User user1 = User.get("user1");
User user2 = User.get("user2");
Computer agent1 = j.jenkins.getComputer("agent1");
Computer agent2 = j.jenkins.getComputer("agent2");
Folder folderA = j.jenkins.createProject(Folder.class, "A");
FreeStyleProject jobA1 = folderA.createProject(FreeStyleProject.class, "1");
Folder folderB = j.jenkins.createProject(Folder.class, "B");
FreeStyleProject jobB2 = folderB.createProject(FreeStyleProject.class, "2");
AuthorizationStrategy s = jenkins.getAuthorizationStrategy();
AuthorizationStrategy s = j.jenkins.getAuthorizationStrategy();
assertThat("Authorization Strategy has been read incorrectly",
s, instanceOf(RoleBasedAuthorizationStrategy.class));
RoleBasedAuthorizationStrategy rbas = (RoleBasedAuthorizationStrategy) s;
@ -60,6 +75,33 @@ public class RoleStrategyTest {
Map<Role, Set<String>> globalRoles = rbas.getGrantedRoles(RoleBasedAuthorizationStrategy.GLOBAL);
assertThat(globalRoles.size(), equalTo(2));
//TODO: add more checks to the test
// Admin has configuration access
assertHasPermission(admin, j.jenkins, Jenkins.ADMINISTER, Jenkins.READ);
assertHasPermission(user1, j.jenkins, Jenkins.READ);
assertHasNoPermission(user1, j.jenkins, Jenkins.ADMINISTER, Jenkins.RUN_SCRIPTS);
// Folder A is restricted to admin
assertHasPermission(admin, folderA, Item.CONFIGURE);
assertHasPermission(user1, folderA, Item.READ, Item.DISCOVER);
assertHasNoPermission(user1, folderA, Item.CONFIGURE, Item.DELETE, Item.BUILD);
// But they have access to jobs in Folder A
assertHasPermission(admin, folderA, Item.CONFIGURE, Item.CANCEL);
assertHasPermission(user1, jobA1, Item.READ, Item.DISCOVER, Item.CONFIGURE, Item.BUILD, Item.DELETE);
assertHasPermission(user2, jobA1, Item.READ, Item.DISCOVER, Item.CONFIGURE, Item.BUILD, Item.DELETE);
assertHasNoPermission(user1, folderA, Item.CANCEL);
// FolderB is editable by user2, but he cannot delete it
assertHasPermission(user2, folderB, Item.READ, Item.DISCOVER, Item.CONFIGURE, Item.BUILD);
assertHasNoPermission(user2, folderB, Item.DELETE);
assertHasNoPermission(user1, folderB, Item.CONFIGURE, Item.BUILD, Item.DELETE);
// Only user1 can run on agent1, but he still cannot configure it
assertHasPermission(admin, agent1, Computer.CONFIGURE, Computer.DELETE, Computer.BUILD);
assertHasPermission(user1, agent1, Computer.BUILD);
assertHasNoPermission(user1, agent1, Computer.CONFIGURE, Computer.DISCONNECT);
// Same user still cannot build on agent2
assertHasNoPermission(user1, agent2, Computer.BUILD);
}
}

View File

@ -0,0 +1,78 @@
/*
* Copyright (c) 2018 Oleg Nenashev
*
* Permission is hereby granted, free of charge, to any person obtaining a copy of this
* software and associated documentation files (the "Software"), to deal in the Software
* without restriction, including without limitation the rights to use, copy, modify, merge,
* publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to
* whom the Software is furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in all copies or
* substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package org.jenkinsci.plugins.casc.misc;
import hudson.model.User;
import hudson.security.ACL;
import hudson.security.ACLContext;
import hudson.security.AccessControlled;
import hudson.security.Permission;
import static org.junit.Assert.assertThat;
import static org.hamcrest.Matchers.*;
/**
* Provides asserts for {@link hudson.security.Permission} checks.
* @author Oleg Nenashev
*/
public class PermissionAssert {
public static void assertHasPermission(User user, final Permission permission, final AccessControlled ... items) {
for (AccessControlled item : items) {
assertPermission(user, item, permission);
}
}
public static void assertHasPermission(User user, final AccessControlled item, final Permission ... permissions) {
for (Permission permission : permissions) {
assertPermission(user, item, permission);
}
}
public static void assertHasNoPermission(User user, final Permission permission, final AccessControlled ... items) {
for (AccessControlled item : items) {
assertNoPermission(user, item, permission);
}
}
public static void assertHasNoPermission(User user, final AccessControlled item, final Permission ... permissions) {
for (Permission permission : permissions) {
assertNoPermission(user, item, permission);
}
}
private static void assertPermission(User user, final AccessControlled item, final Permission p) {
assertThat("User '" + user + "' has no " + p.getId() + " permission for " + item + ", but it should according to security settings",
hasPermission(user, item, p), equalTo(true));
}
private static void assertNoPermission(User user, final AccessControlled item, final Permission p) {
assertThat("User '" + user + "' has the " + p.getId() + " permission for " + item + ", but it should not according to security settings",
hasPermission(user, item, p), equalTo(false));
}
private static boolean hasPermission(User user, final AccessControlled item, final Permission p) {
try (ACLContext c = ACL.as(user)) {
return item.hasPermission(p);
}
}
}

View File

@ -4,49 +4,67 @@ jenkins:
roleStrategy:
roles:
global:
- role:
name: "admin"
description: "Jenkins administrators"
permissions:
- "Jenkins/Administer"
- name: "admin"
description: "Jenkins administrators"
permissions:
- "Overall/Administer"
assignments:
- "admin"
- role:
name: "readonly"
description: "Read-only users"
permissions:
- "Jenkins/Read"
- "Item/Read"
- name: "readonly"
description: "Read-only users"
permissions:
- "Overall/Read"
- "Job/Read"
assignments:
- "authenticated"
items:
- role:
name: "FolderA"
description: "Jobs in Folder A, but not the folder itself"
pattern: "A/.*"
permissions:
- "Item/Configure"
- "Item/Build"
- "Item/Delete"
assignments:
- "authenticated"
- role:
name: "FolderB"
description: "Jobs in Folder B, but not the folder itself"
pattern: "B/.*"
permissions:
- "Item/Configure"
- "Item/Build"
- "Item/Delete"
- name: "FolderA"
description: "Jobs in Folder A, but not the folder itself"
pattern: "A/.*"
permissions:
- "Job/Configure"
- "Job/Build"
- "Job/Delete"
assignments:
- "user1"
- "user2"
- name: "FolderB"
description: "Jobs in Folder B, but not the folder itself"
pattern: "B.*"
permissions:
- "Job/Configure"
- "Job/Build"
assignments:
- "user2"
agents:
- role:
name: "Agent1"
description: "Agent 1"
pattern: "agent1*"
permissions:
- "Computer/Run"
- name: "Agent1"
description: "Agent 1"
pattern: "agent1"
permissions:
- "Agent/Build"
assignments:
- "user1"
# System for test
securityRealm:
local:
allowsSignup: false
users:
- id: "admin"
password: "1234"
- id: "user1"
password: ""
nodes:
- dumb:
mode: NORMAL
name: "agent1"
remoteFS: "/home/user1"
launcher:
jnlp:
- dumb:
mode: NORMAL
name: "agent2"
remoteFS: "/home/user1"
launcher:
jnlp: