Skip to content

Commit

Permalink
chore(engine\rest): improve adding new Permissions enum
Browse files Browse the repository at this point in the history
In case of the XxxPermissions enum the following must be update:
* add the enum to ResourceTypeUtil#PERMISSION_ENUMS
* update accordingly ResourceTypeUtil#getPermissionForNameByResourceType
* copy all existing Permissions for this resource in the XxxPermissions

Related to CAM-9786
  • Loading branch information
yanavasileva committed Feb 21, 2019
1 parent 32672be commit b940b86
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 100 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,14 @@
*/
package org.camunda.bpm.engine.rest.dto.authorization;

import java.util.ArrayList;
import java.util.List;

import org.camunda.bpm.engine.authorization.Authorization;
import org.camunda.bpm.engine.authorization.BatchPermissions;
import org.camunda.bpm.engine.authorization.Permission;
import org.camunda.bpm.engine.authorization.Permissions;
import org.camunda.bpm.engine.authorization.ProcessDefinitionPermissions;
import org.camunda.bpm.engine.authorization.ProcessInstancePermissions;
import org.camunda.bpm.engine.authorization.Resources;
import org.camunda.bpm.engine.authorization.TaskPermissions;
import org.camunda.bpm.engine.impl.util.ResourceTypeUtil;
import org.camunda.bpm.engine.rest.dto.converter.PermissionConverter;

import java.util.ArrayList;
import java.util.List;

/**
* @author Daniel Meyer
*
Expand Down Expand Up @@ -136,17 +131,9 @@ public void setResourceId(String resourceId) {

private static Permission[] getPermissions(Authorization dbAuthorization) {
int givenResourceType = dbAuthorization.getResourceType();
if (givenResourceType == Resources.BATCH.resourceType()) {
return dbAuthorization.getPermissions(BatchPermissions.values());
} else if (givenResourceType == Resources.PROCESS_DEFINITION.resourceType()) {
return dbAuthorization.getPermissions(ProcessDefinitionPermissions.values());
} else if (givenResourceType == Resources.PROCESS_INSTANCE.resourceType()) {
return dbAuthorization.getPermissions(ProcessInstancePermissions.values());
} else if (givenResourceType == Resources.TASK.resourceType()) {
return dbAuthorization.getPermissions(TaskPermissions.values());
} else {
return dbAuthorization.getPermissions(Permissions.values());
}
Permission[] permissionsByResourceType = ResourceTypeUtil.getPermissionsByResourceType(givenResourceType);

return dbAuthorization.getPermissions(permissionsByResourceType);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,9 @@
package org.camunda.bpm.engine.rest.dto.converter;

import org.camunda.bpm.engine.authorization.Authorization;
import org.camunda.bpm.engine.authorization.BatchPermissions;
import org.camunda.bpm.engine.authorization.Permission;
import org.camunda.bpm.engine.authorization.Permissions;
import org.camunda.bpm.engine.authorization.ProcessDefinitionPermissions;
import org.camunda.bpm.engine.authorization.ProcessInstancePermissions;
import org.camunda.bpm.engine.authorization.Resources;
import org.camunda.bpm.engine.authorization.TaskPermissions;
import org.camunda.bpm.engine.impl.util.ResourceTypeUtil;

import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -79,17 +75,7 @@ public static String[] getNamesForPermissions(Authorization authorization, Permi

public static Permission getPermissionForName(String name, int resourceType) {
// TODO: make this configurable via SPI
if (resourceType == Resources.BATCH.resourceType()) {
return BatchPermissions.forName(name);
} else if (resourceType == Resources.PROCESS_DEFINITION.resourceType()) {
return ProcessDefinitionPermissions.forName(name);
} else if (resourceType == Resources.PROCESS_INSTANCE.resourceType()) {
return ProcessInstancePermissions.forName(name);
} else if (resourceType == Resources.TASK.resourceType()) {
return TaskPermissions.forName(name);
}else {
return Permissions.forName(name);
}
return ResourceTypeUtil.getPermissionForNameByResourceType(name, resourceType);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,10 @@ public enum Permissions implements Permission {
/** Indicates that MIGRATE_INSTANCE interactions are permitted */
MIGRATE_INSTANCE("MIGRATE_INSTANCE", 65536, EnumSet.of(Resources.PROCESS_DEFINITION));

// Note: Please use *Permissions for new permissions
// NOTE: Please use XxxPermissions for new permissions
// Keep in mind to use unique permissions' ids for the same Resource
// TODO in case a new XxxPermissions enum is created:
// please adjust ResourceTypeUtil#PERMISSION_ENUMS accordingly


// implementation //////////////////////////
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,37 @@
*/
package org.camunda.bpm.engine.impl.util;

import java.util.HashMap;
import java.util.Map;

import org.camunda.bpm.engine.authorization.BatchPermissions;
import org.camunda.bpm.engine.authorization.Permission;
import org.camunda.bpm.engine.authorization.Permissions;
import org.camunda.bpm.engine.authorization.ProcessDefinitionPermissions;
import org.camunda.bpm.engine.authorization.ProcessInstancePermissions;
import org.camunda.bpm.engine.authorization.Resource;
import org.camunda.bpm.engine.authorization.Resources;
import org.camunda.bpm.engine.authorization.TaskPermissions;

public class ResourceTypeUtil {

private static final Map<Integer, Class<? extends Enum<? extends Permission>>> permissionEnums = new HashMap<>();

static {
permissionEnums.put(Resources.BATCH.resourceType(), BatchPermissions.class);
permissionEnums.put(Resources.PROCESS_DEFINITION.resourceType(), ProcessDefinitionPermissions.class);
permissionEnums.put(Resources.PROCESS_INSTANCE.resourceType(), ProcessInstancePermissions.class);
permissionEnums.put(Resources.TASK.resourceType(), TaskPermissions.class);

// the rest
for (Resource resource : Resources.values()) {
int resourceType = resource.resourceType();
if (!permissionEnums.containsKey(resourceType)) {
permissionEnums.put(resourceType, Permissions.class);
}
}
}

public static boolean resourceIsContainedInArray(Integer resourceTypeId, Resource[] list) {
for (Resource resource : list) {
if (resourceTypeId == resource.resourceType()) {
Expand All @@ -28,4 +55,29 @@ public static boolean resourceIsContainedInArray(Integer resourceTypeId, Resourc
return false;
}

public static Map<Integer, Class<? extends Enum<? extends Permission>>> getPermissionEnums() {
return permissionEnums;
}

public static Permission[] getPermissionsByResourceType(int givenResourceType) {
Class<? extends Enum<? extends Permission>> clazz = permissionEnums.get(givenResourceType);
if (clazz == null) {
return Permissions.values();
}
return ((Permission[]) clazz.getEnumConstants());
}

public static Permission getPermissionForNameByResourceType(String name, int resourceType) {
if (resourceType == Resources.BATCH.resourceType()) {
return BatchPermissions.forName(name);
} else if (resourceType == Resources.PROCESS_DEFINITION.resourceType()) {
return ProcessDefinitionPermissions.forName(name);
} else if (resourceType == Resources.PROCESS_INSTANCE.resourceType()) {
return ProcessInstancePermissions.forName(name);
} else if (resourceType == Resources.TASK.resourceType()) {
return TaskPermissions.forName(name);
} else {
return Permissions.forName(name);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,39 +17,24 @@

import static org.assertj.core.api.Assertions.assertThat;

import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;

import org.camunda.bpm.engine.authorization.BatchPermissions;
import org.camunda.bpm.engine.authorization.Permission;
import org.camunda.bpm.engine.authorization.Permissions;
import org.camunda.bpm.engine.authorization.ProcessDefinitionPermissions;
import org.camunda.bpm.engine.authorization.ProcessInstancePermissions;
import org.camunda.bpm.engine.authorization.Resource;
import org.camunda.bpm.engine.authorization.Resources;
import org.camunda.bpm.engine.authorization.TaskPermissions;
import org.camunda.bpm.engine.impl.util.ResourceTypeUtil;
import org.junit.Test;

public class PermissionsTest {

private static final Map<Integer, Class<? extends Enum<? extends Permission>>> PERMISSION_ENUMS = new HashMap<>();

static {
PERMISSION_ENUMS.put(Resources.BATCH.resourceType(), BatchPermissions.class);
PERMISSION_ENUMS.put(Resources.PROCESS_DEFINITION.resourceType(), ProcessDefinitionPermissions.class);
PERMISSION_ENUMS.put(Resources.PROCESS_INSTANCE.resourceType(), ProcessInstancePermissions.class);
PERMISSION_ENUMS.put(Resources.TASK.resourceType(), TaskPermissions.class);
}

@Test
public void testNewPermissionsIntegrityToOld() {
for (Permissions permission : Permissions.values()) {
String permissionName = permission.getName();
for (Resource resource : permission.getTypes()) {
Class<? extends Enum<?>> clazz = PERMISSION_ENUMS.get(resource.resourceType());
if (clazz != null) {
Class<? extends Enum<?>> clazz = ResourceTypeUtil.getPermissionEnums().get(resource.resourceType());
if (clazz != null && !clazz.getSimpleName().equals(Permissions.class.getSimpleName())) {
Permission resolvedPermission = null;
for (Enum<?> enumCandidate : clazz.getEnumConstants()) {
if (enumCandidate.toString().equals(permissionName)) {
Expand All @@ -69,41 +54,30 @@ public void testNewPermissionsIntegrityToOld() {

@Test
public void testPermissionsValues() {
verifyValuesAreUniqueAndPowerOfTwo(Permissions.values());
}

@Test
public void testBatchPermissionsValues() {
verifyValuesAreUniqueAndPowerOfTwo(BatchPermissions.values());
}

@Test
public void testProcessInstancePermissionsValues() {
verifyValuesAreUniqueAndPowerOfTwo(ProcessInstancePermissions.values());
}

@Test
public void testProcessDefinitionPermissionsValues() {
verifyValuesAreUniqueAndPowerOfTwo(ProcessDefinitionPermissions.values());
verifyValuesAreUniqueAndPowerOfTwo(Permissions.values(), Permissions.class.getSimpleName());
}

@Test
public void testTaskPermissionsValues() {
verifyValuesAreUniqueAndPowerOfTwo(TaskPermissions.values());
public void testRestOfPermissionsEnumValues() {
for (Class<? extends Enum<? extends Permission>> permissionsClass : ResourceTypeUtil.getPermissionEnums().values()) {
if(!permissionsClass.getSimpleName().equals(Permissions.class.getSimpleName())) {
verifyValuesAreUniqueAndPowerOfTwo((Permission[])permissionsClass.getEnumConstants(), permissionsClass.getSimpleName());
}
}
}

private void verifyValuesAreUniqueAndPowerOfTwo(Permission[] permissions) {
private void verifyValuesAreUniqueAndPowerOfTwo(Permission[] permissions, String className) {
Set<Integer> values = new HashSet<>();
for (Permission permission : permissions) {
int value = permission.getValue();
// value is unique
assertThat(values.add(value))
.overridingErrorMessage("The value '%s' of '%s' permission is not unique. Another permission already has this value.", value, permission)
.overridingErrorMessage("The value '%s' of '%s' permission is not unique for '%s' permission enum. Another permission already has this value.", value, permission, className)
.isTrue();
if (value != Integer.MAX_VALUE && value != 0) {
// value is power of 2
assertThat(isPowerOfTwo(value))
.overridingErrorMessage("The value '%s' of '%s' permission is invalid. The values must be power of 2.", value, permission)
.overridingErrorMessage("The value '%s' of '%s' permission is invalid for '%s' permission enum. The values must be power of 2.", value, permission, className)
.isTrue();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,43 +21,26 @@
import java.util.Map;

import org.camunda.bpm.engine.authorization.Authorization;
import org.camunda.bpm.engine.authorization.BatchPermissions;
import org.camunda.bpm.engine.authorization.MissingAuthorization;
import org.camunda.bpm.engine.authorization.Permission;
import org.camunda.bpm.engine.authorization.Permissions;
import org.camunda.bpm.engine.authorization.ProcessDefinitionPermissions;
import org.camunda.bpm.engine.authorization.ProcessInstancePermissions;
import org.camunda.bpm.engine.authorization.Resource;
import org.camunda.bpm.engine.authorization.Resources;
import org.camunda.bpm.engine.authorization.TaskPermissions;
import org.camunda.bpm.engine.impl.util.ResourceTypeUtil;

/**
* @author Thorben Lindhauer
*
*/
public class AuthorizationTestUtil {

protected static final int BATCH = Resources.BATCH.resourceType();
protected static final int PROCESS_DEFINITION = Resources.PROCESS_DEFINITION.resourceType();
protected static final int PROCESS_INSTANCE = Resources.PROCESS_INSTANCE.resourceType();
protected static final int TASK = Resources.TASK.resourceType();

protected static Map<Integer, Resource> resourcesByType = new HashMap<Integer, Resource>();
protected static Map<Integer, Permission[]> permissionMap = new HashMap<Integer, Permission[]>();

static {
for (Resource resource : Resources.values()) {
resourcesByType.put(resource.resourceType(), resource);
}
}

static {
permissionMap.put(BATCH, BatchPermissions.values());
permissionMap.put(PROCESS_DEFINITION, ProcessDefinitionPermissions.values());
permissionMap.put(PROCESS_INSTANCE, ProcessInstancePermissions.values());
permissionMap.put(TASK, TaskPermissions.values());
}

public static Resource getResourceByType(int type) {
return resourcesByType.get(type);
}
Expand All @@ -83,11 +66,8 @@ public static void assertExceptionInfo(String expectedPermissionName, String exp
public static Permission[] getPermissions(Authorization authorization)
{
int resourceType = authorization.getResourceType();
if (resourceType == BATCH || resourceType == PROCESS_DEFINITION || resourceType == PROCESS_INSTANCE || resourceType == TASK) {
Permission[] permissionsForType = permissionMap.get(resourceType);
return authorization.getPermissions(permissionsForType);
} else {
return authorization.getPermissions(Permissions.values());
}
Permission[] permissionsByResourceType = ResourceTypeUtil.getPermissionsByResourceType(resourceType);

return authorization.getPermissions(permissionsByResourceType);
}
}

0 comments on commit b940b86

Please sign in to comment.