Skip to content

Commit

Permalink
Nicer JCasC syntax for defining permissions (#145)
Browse files Browse the repository at this point in the history
* Nicer JCasC syntax for defining permissions

* Oops, wrong Converter! Also, clean up test and note PR dependency

* Sort entries in the JCasC YAML, add proper assertions

* Implement nested type to prevent weird sort order

* Adapt link in readme to future preferred syntax

* Improve error message

* Add test for v3 format

---------

Co-authored-by: Daniel Beck <[email protected]>
  • Loading branch information
daniel-beck and daniel-beck authored Aug 22, 2023
1 parent 5f2fae6 commit 47a3280
Show file tree
Hide file tree
Showing 17 changed files with 588 additions and 151 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Depending on the entity being configured, all or a subset of the following _inhe

Matrix Authorization Strategy Plugin has full support for use in Configuration as Code and Job DSL.

For an example combining the two, see [this `configuration-as-code.yml` test resource](https://github.com/jenkinsci/matrix-auth-plugin/blob/master/src/test/resources/org/jenkinsci/plugins/matrixauth/integrations/casc/configuration-as-code.yml).
For an example combining the two, see [this `configuration-as-code.yml` test resource](https://github.com/jenkinsci/matrix-auth-plugin/blob/master/src/test/resources/org/jenkinsci/plugins/matrixauth/integrations/casc/configuration-as-code-v3.yml).


## Caveats
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,14 +211,7 @@ default void add(String shortForm) {
"Processing a permission assignment in the legacy format (without explicit TYPE prefix): "
+ shortForm);
}
Permission p = Permission.fromId(permissionString);
if (p == null) {
// attempt to find the permission based on the 'nice' name, e.g. Overall/Administer
p = PermissionFinder.findPermission(permissionString);
}
if (p == null) {
throw new IllegalArgumentException("Failed to parse '" + shortForm + "' --- no such permission");
}
Permission p = parsePermission(permissionString);
if (!p.isContainedBy(((AuthorizationContainerDescriptor) getDescriptor()).getPermissionScope())) {
LOGGER.log(
Level.WARNING,
Expand All @@ -228,6 +221,19 @@ default void add(String shortForm) {
add(p, new PermissionEntry(type, sid));
}

@Restricted(NoExternalUse.class)
static Permission parsePermission(String permission) {
Permission p = Permission.fromId(permission);
if (p == null) {
// attempt to find the permission based on the 'nice' name, e.g. Overall/Administer
p = PermissionFinder.findPermission(permission);
}
if (p == null) {
throw new IllegalArgumentException("Failed to parse '" + permission + "' --- no such permission");
}
return p;
}

@Restricted(NoExternalUse.class)
@SuppressWarnings("unused") // used from Jelly
Permission getEditingPermission();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
*
* @since 3.0
*/
public class PermissionEntry {
public class PermissionEntry implements Comparable<PermissionEntry> {
private final AuthorizationType type;
private final String sid;

Expand Down Expand Up @@ -110,4 +110,13 @@ public int hashCode() {
public String toString() {
return "PermissionEntry{" + "type=" + type + ", sid='" + sid + "'" + '}';
}

@Override
public int compareTo(@NonNull PermissionEntry o) {
final int type = getType().compareTo(o.getType());
if (type != 0) {
return type;
}
return this.getSid().compareTo(o.getSid());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,12 @@ protected AuthorizationMatrixNodeProperty instance(Mapping mapping, Configuratio
public Set<Attribute<AuthorizationMatrixNodeProperty, ?>> describe() {
return new HashSet<>(Arrays.asList(
new MultivaluedAttribute<AuthorizationMatrixNodeProperty, String>("permissions", String.class)
.getter(MatrixAuthorizationStrategyConfigurator::getPermissions)
.setter(MatrixAuthorizationStrategyConfigurator::setPermissions),
.getter(unused -> null)
.setter(MatrixAuthorizationStrategyConfigurator::setLegacyPermissions),
new MultivaluedAttribute<AuthorizationMatrixNodeProperty, DefinitionEntry>(
"entries", DefinitionEntry.class)
.getter(MatrixAuthorizationStrategyConfigurator::getEntries)
.setter(MatrixAuthorizationStrategyConfigurator::setEntries),
new DescribableAttribute<AuthorizationMatrixNodeProperty, InheritanceStrategy>(
"inheritanceStrategy", InheritanceStrategy.class)));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
package org.jenkinsci.plugins.matrixauth.integrations.casc;

import java.util.List;
import java.util.stream.Collectors;
import org.jenkinsci.plugins.matrixauth.AuthorizationType;
import org.jenkinsci.plugins.matrixauth.PermissionEntry;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.DataBoundSetter;

/**
* Entry for type-safe permission definitions in JCasC YAML.
*/
@Restricted(NoExternalUse.class)
public class DefinitionEntry {
private AuthorizationType type;
private Child child;

@DataBoundConstructor
public DefinitionEntry() {}

public DefinitionEntry(AuthorizationType type, Child child) {
this.child = child;
this.type = type;
}

@DataBoundSetter
public void setUserOrGroup(Child child) {
setNew(AuthorizationType.EITHER, child);
}

public Child getUserOrGroup() {
return type == AuthorizationType.EITHER ? child : null;
}

@DataBoundSetter
public void setUser(Child child) {
setNew(AuthorizationType.USER, child);
}

public Child getUser() {
return type == AuthorizationType.USER ? child : null;
}

@DataBoundSetter
public void setGroup(Child child) {
setNew(AuthorizationType.GROUP, child);
}

private void setNew(AuthorizationType type, Child child) {
if (this.type != null) {
throw new IllegalStateException(
"Can only configure one of: 'user', 'group', 'userOrGroup', but attempted to redefine to '"
+ authorizationTypeToKey(type) + "' with name '" + child.name + "' after '"
+ authorizationTypeToKey(this.type) + "' was already set to '"
+ this.child.name + "'");
}
this.type = type;
this.child = child;
}

public Child getGroup() {
return type == AuthorizationType.GROUP ? child : null;
}

public Child child() {
return child;
}

public PermissionEntry permissionEntry() {
return new PermissionEntry(type, child.name);
}

private static String authorizationTypeToKey(AuthorizationType type) {
if (type == null) {
throw new NullPointerException("Received null 'type'");
}
if (type == AuthorizationType.USER) {
return "user";
}
if (type == AuthorizationType.GROUP) {
return "group";
}
if (type == AuthorizationType.EITHER) {
return "userOrGroup";
}
throw new IllegalStateException("Unexpected 'type': " + type);
}

@Restricted(NoExternalUse.class)
public static class Child {
final List<PermissionDefinition> permissions;
final String name;

@DataBoundConstructor
public Child(String name, List<PermissionDefinition> permissions) {
this.name = name;
this.permissions = permissions;
}

public List<PermissionDefinition> getPermissions() {
return permissions.stream().sorted().collect(Collectors.toList());
}

public String getName() {
return name;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,24 @@

import edu.umd.cs.findbugs.annotations.NonNull;
import hudson.security.AuthorizationStrategy;
import hudson.security.Permission;
import io.jenkins.plugins.casc.Attribute;
import io.jenkins.plugins.casc.BaseConfigurator;
import io.jenkins.plugins.casc.impl.attributes.MultivaluedAttribute;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.stream.Collectors;
import org.jenkinsci.plugins.matrixauth.AuthorizationContainer;
import org.jenkinsci.plugins.matrixauth.PermissionEntry;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;

Expand All @@ -30,20 +37,80 @@ public Class<?> getImplementedAPI() {
@NonNull
public Set<Attribute<T, ?>> describe() {
return new HashSet<>(Arrays.asList(
new MultivaluedAttribute<T, String>("permissions", String.class)
.getter(MatrixAuthorizationStrategyConfigurator::getPermissions)
.setter(MatrixAuthorizationStrategyConfigurator::setPermissions),
new MultivaluedAttribute<T, DefinitionEntry>("entries", DefinitionEntry.class)
.getter(MatrixAuthorizationStrategyConfigurator::getEntries)
.setter(MatrixAuthorizationStrategyConfigurator::setEntries),

// support old style configuration options
new MultivaluedAttribute<T, String>("permissions", String.class)
.getter(unused -> null)
.setter(MatrixAuthorizationStrategyConfigurator::setLegacyPermissions),
new MultivaluedAttribute<T, String>("grantedPermissions", String.class)
.getter(unused -> null)
.setter(MatrixAuthorizationStrategyConfigurator::setPermissionsDeprecated)));
}

private static class PermissionAssignment {
private final PermissionDefinition permission;
private final PermissionEntry entry;

private PermissionAssignment(PermissionDefinition permission, PermissionEntry entry) {
this.permission = permission;
this.entry = entry;
}

public PermissionDefinition getPermission() {
return permission;
}

public PermissionEntry getEntry() {
return entry;
}
}

/**
* Maps an {@link AuthorizationContainer} to a collection (list) of {@link DefinitionEntry}, its serialized form.
*
* @param container the container
* @return
*/
public static Collection<DefinitionEntry> getEntries(AuthorizationContainer container) {
// Contain has: Map from Permission to List of PermissionEntries (sid and type)
final Map<Permission, Set<PermissionEntry>> entries = container.getGrantedPermissionEntries();

final HashMap<PermissionEntry, List<PermissionDefinition>> intermediate = entries.entrySet().stream()
.map(entry -> Map.entry(PermissionDefinition.forPermission(entry.getKey()), entry.getValue()))
.flatMap(entry -> entry.getValue().stream().map(p -> new PermissionAssignment(entry.getKey(), p)))
.collect(
HashMap::new,
(c, e) -> c.computeIfAbsent(e.getEntry(), f -> new ArrayList<>())
.add(e.getPermission()),
(c1, c2) -> {
/* unused */
});
final List<DefinitionEntry> result = intermediate.entrySet().stream()
.map(entry -> new DefinitionEntry(
entry.getKey().getType(),
new DefinitionEntry.Child(entry.getKey().getSid(), entry.getValue())))
.sorted(Comparator.comparing(DefinitionEntry::permissionEntry))
.collect(Collectors.toList());
return result;
}

public static void setEntries(AuthorizationContainer container, Collection<DefinitionEntry> entries) {
entries.forEach(e -> {
e.child().getPermissions().stream()
.map(PermissionDefinition::getPermission)
.forEach(p -> {
container.add(p, e.permissionEntry());
});
});
}

/**
* Extract container's permissions as a List of "TYPE:PERMISSION:sid"
*/
public static Collection<String> getPermissions(AuthorizationContainer container) {
public static Collection<String> getLegacyPermissions(AuthorizationContainer container) {
return container.getGrantedPermissionEntries().entrySet().stream()
.flatMap(e -> e.getValue().stream()
.map(v -> v.getType().toPrefix() + e.getKey().group.getId() + "/" + e.getKey().name + ":"
Expand All @@ -55,19 +122,23 @@ public static Collection<String> getPermissions(AuthorizationContainer container
/**
* Configure container's permissions from a List of "PERMISSION:sid" or "TYPE:PERMISSION:sid"
*/
public static void setPermissions(AuthorizationContainer container, Collection<String> permissions) {
public static void setLegacyPermissions(AuthorizationContainer container, Collection<String> permissions) {
LOGGER.log(
Level.WARNING,
"Loading deprecated attribute 'permissions' for instance of '"
+ container.getClass().getName() + "'. Use 'entries' instead.");
permissions.forEach(container::add);
}

/**
* Like {@link #setPermissions(AuthorizationContainer, Collection)} but logs a deprecation warning
* Like {@link #setLegacyPermissions(AuthorizationContainer, Collection)} but logs a deprecation warning
*/
public static void setPermissionsDeprecated(AuthorizationContainer container, Collection<String> permissions) {
LOGGER.log(
Level.WARNING,
"Loading deprecated attribute 'grantedPermissions' for instance of '"
+ container.getClass().getName() + "'. Use 'permissions' instead.");
setPermissions(container, permissions);
setLegacyPermissions(container, permissions);
}

private static final Logger LOGGER = Logger.getLogger(MatrixAuthorizationStrategyConfigurator.class.getName());
Expand Down
Loading

0 comments on commit 47a3280

Please sign in to comment.