From 0bac027cd4789f5288d3581d11452e5b58e12cfd Mon Sep 17 00:00:00 2001 From: Tassilo Weidner Date: Thu, 23 Apr 2020 10:16:06 +0200 Subject: [PATCH] feat(engine): enforce hist task auth for op log query related to CAM-11628 --- .../camunda/bpm/engine/HistoryService.java | 3 + .../entity/AuthorizationManager.java | 7 +- .../mapping/entity/UserOperationLogEntry.xml | 1 + .../UserOperationLogAuthorizationTest.java | 226 ++++++++++++++++++ 4 files changed, 235 insertions(+), 2 deletions(-) diff --git a/engine/src/main/java/org/camunda/bpm/engine/HistoryService.java b/engine/src/main/java/org/camunda/bpm/engine/HistoryService.java index cbe4baf8569..d707cba288d 100644 --- a/engine/src/main/java/org/camunda/bpm/engine/HistoryService.java +++ b/engine/src/main/java/org/camunda/bpm/engine/HistoryService.java @@ -207,6 +207,9 @@ public interface HistoryService { * {@link Resources#PROCESS_DEFINITION} OR *
  • The user has no {@link HistoricProcessInstancePermissions#READ} permission on * {@link Resources#HISTORIC_PROCESS_INSTANCE} ({@code enableHistoricInstancePermissions} in + * {@link ProcessEngineConfigurationImpl} must be set to {@code true}) OR + *
  • The user has no {@link HistoricTaskPermissions#READ} permission on + * {@link Resources#HISTORIC_TASK} ({@code enableHistoricInstancePermissions} in * {@link ProcessEngineConfigurationImpl} must be set to {@code true}) * * */ diff --git a/engine/src/main/java/org/camunda/bpm/engine/impl/persistence/entity/AuthorizationManager.java b/engine/src/main/java/org/camunda/bpm/engine/impl/persistence/entity/AuthorizationManager.java index 8ae8dc89f31..6b156e182c0 100644 --- a/engine/src/main/java/org/camunda/bpm/engine/impl/persistence/entity/AuthorizationManager.java +++ b/engine/src/main/java/org/camunda/bpm/engine/impl/persistence/entity/AuthorizationManager.java @@ -936,8 +936,11 @@ public void configureUserOperationLogQuery(UserOperationLogQueryImpl query) { authCheck.setHistoricInstancePermissionsEnabled(isHistoricInstancePermissionsEnabled); if (isHistoricInstancePermissionsEnabled) { - permissionCheckBuilder.atomicCheck(HISTORIC_PROCESS_INSTANCE, "RES.PROC_INST_ID_", - HistoricProcessInstancePermissions.READ); + permissionCheckBuilder + .atomicCheck(HISTORIC_PROCESS_INSTANCE, "RES.PROC_INST_ID_", + HistoricProcessInstancePermissions.READ) + .atomicCheck(HISTORIC_TASK, "RES.TASK_ID_", + HistoricTaskPermissions.READ); } CompositePermissionCheck permissionCheck = permissionCheckBuilder.build(); diff --git a/engine/src/main/resources/org/camunda/bpm/engine/impl/mapping/entity/UserOperationLogEntry.xml b/engine/src/main/resources/org/camunda/bpm/engine/impl/mapping/entity/UserOperationLogEntry.xml index 2e135e859ff..f47e8b04cb1 100644 --- a/engine/src/main/resources/org/camunda/bpm/engine/impl/mapping/entity/UserOperationLogEntry.xml +++ b/engine/src/main/resources/org/camunda/bpm/engine/impl/mapping/entity/UserOperationLogEntry.xml @@ -247,6 +247,7 @@ AUTH ON (AUTH.RESOURCE_ID_ in ( RES.PROC_INST_ID_, + RES.TASK_ID_, RES.PROC_DEF_KEY_, RES.CATEGORY_, '*')) diff --git a/engine/src/test/java/org/camunda/bpm/engine/test/api/authorization/history/UserOperationLogAuthorizationTest.java b/engine/src/test/java/org/camunda/bpm/engine/test/api/authorization/history/UserOperationLogAuthorizationTest.java index 1500b7b4687..fd1734ba670 100644 --- a/engine/src/test/java/org/camunda/bpm/engine/test/api/authorization/history/UserOperationLogAuthorizationTest.java +++ b/engine/src/test/java/org/camunda/bpm/engine/test/api/authorization/history/UserOperationLogAuthorizationTest.java @@ -22,6 +22,7 @@ import static org.camunda.bpm.engine.authorization.Permissions.READ_HISTORY; import static org.camunda.bpm.engine.authorization.Permissions.UPDATE; import static org.camunda.bpm.engine.authorization.ProcessDefinitionPermissions.UPDATE_HISTORY; +import static org.camunda.bpm.engine.authorization.Resources.HISTORIC_TASK; import static org.camunda.bpm.engine.authorization.Resources.OPERATION_LOG_CATEGORY; import static org.camunda.bpm.engine.authorization.Resources.PROCESS_DEFINITION; import static org.camunda.bpm.engine.authorization.UserOperationLogCategoryPermissions.DELETE; @@ -35,6 +36,7 @@ import org.camunda.bpm.engine.AuthorizationException; import org.camunda.bpm.engine.ProcessEngineConfiguration; import org.camunda.bpm.engine.authorization.HistoricProcessInstancePermissions; +import org.camunda.bpm.engine.authorization.HistoricTaskPermissions; import org.camunda.bpm.engine.authorization.ProcessDefinitionPermissions; import org.camunda.bpm.engine.authorization.Resources; import org.camunda.bpm.engine.authorization.UserOperationLogCategoryPermissions; @@ -66,6 +68,7 @@ public class UserOperationLogAuthorizationTest extends AuthorizationTest { protected static final String TIMER_BOUNDARY_PROCESS_KEY = "timerBoundaryProcess"; protected String deploymentId; + protected String taskId; @Override public void setUp() throws Exception { @@ -81,6 +84,12 @@ public void tearDown() { super.tearDown(); deleteDeployment(deploymentId); processEngineConfiguration.setEnableHistoricInstancePermissions(false); + + if (taskId != null) { + deleteTask(taskId, true); + taskId = null; + + } } // standalone task /////////////////////////////// @@ -504,6 +513,223 @@ public void testHistoricProcessInstancePermissionsAuthorizationDisabled() { .containsExactly(processInstanceId, processInstanceId); } + public void testCheckNonePermissionOnHistoricTask() { + // given + processEngineConfiguration.setEnableHistoricInstancePermissions(true); + + startProcessInstanceByKey(ONE_TASK_PROCESS_KEY); + + String taskId = selectSingleTask().getId(); + setAssignee(taskId, "demo"); + + createGrantAuthorizationWithoutAuthentication(HISTORIC_TASK, taskId, userId, + HistoricTaskPermissions.NONE); + + // when + UserOperationLogQuery query = historyService.createUserOperationLogQuery(); + + // then + assertThat(query.list()).isEmpty(); + } + + public void testCheckReadPermissionOnHistoricTask() { + // given + processEngineConfiguration.setEnableHistoricInstancePermissions(true); + + startProcessInstanceByKey(ONE_TASK_PROCESS_KEY); + String taskId = selectSingleTask().getId(); + setAssignee(taskId, "demo"); + + createGrantAuthorizationWithoutAuthentication(HISTORIC_TASK, taskId, userId, + HistoricTaskPermissions.READ); + + // when + UserOperationLogQuery query = historyService.createUserOperationLogQuery(); + + // then + assertThat(query.list()) + .extracting("taskId") + .containsExactly(taskId); + } + + public void testCheckReadPermissionOnStandaloneHistoricTask() { + // given + processEngineConfiguration.setEnableHistoricInstancePermissions(true); + + taskId = "aTaskId"; + createTask(taskId); + + disableAuthorization(); + taskService.setAssignee(taskId, userId); + enableAuthorization(); + + createGrantAuthorizationWithoutAuthentication(HISTORIC_TASK, taskId, userId, + HistoricTaskPermissions.READ); + + // when + UserOperationLogQuery query = historyService.createUserOperationLogQuery(); + + // then + assertThat(query.list()) + .extracting("taskId") + .containsExactly(taskId, taskId); + } + + public void testCheckNonePermissionOnStandaloneHistoricTask() { + // given + processEngineConfiguration.setEnableHistoricInstancePermissions(true); + + taskId = "aTaskId"; + createTask(taskId); + disableAuthorization(); + taskService.setAssignee(taskId, userId); + enableAuthorization(); + + createGrantAuthorizationWithoutAuthentication(HISTORIC_TASK, taskId, userId, + HistoricTaskPermissions.NONE); + + // when + UserOperationLogQuery query = historyService.createUserOperationLogQuery(); + + // then + assertThat(query.list()).isEmpty(); + } + + public void testCheckReadPermissionOnCompletedHistoricTask() { + // given + processEngineConfiguration.setEnableHistoricInstancePermissions(true); + + startProcessInstanceByKey(ONE_TASK_PROCESS_KEY); + String taskId = selectSingleTask().getId(); + disableAuthorization(); + taskService.setAssignee(taskId, userId); + taskService.complete(taskId); + enableAuthorization(); + + createGrantAuthorizationWithoutAuthentication(HISTORIC_TASK, taskId, userId, + HistoricTaskPermissions.READ); + + // when + UserOperationLogQuery query = historyService.createUserOperationLogQuery(); + + // then + assertThat(query.list()) + .extracting("taskId") + .containsExactly(taskId, taskId); + } + + public void testCheckNonePermissionOnHistoricTaskAndReadHistoryPermissionOnProcessDefinition() { + // given + processEngineConfiguration.setEnableHistoricInstancePermissions(true); + + startProcessInstanceByKey(ONE_TASK_PROCESS_KEY); + String taskId = selectSingleTask().getId(); + disableAuthorization(); + taskService.complete(taskId); + enableAuthorization(); + + createGrantAuthorizationWithoutAuthentication(HISTORIC_TASK, taskId, userId, + HistoricTaskPermissions.NONE); + createGrantAuthorizationWithoutAuthentication(PROCESS_DEFINITION, ONE_TASK_PROCESS_KEY, + userId, ProcessDefinitionPermissions.READ_HISTORY); + + // when + UserOperationLogQuery query = historyService.createUserOperationLogQuery(); + + // then + assertThat(query.list()) + .extracting("taskId") + .containsExactlyInAnyOrder(taskId, null); + } + + public void testCheckReadPermissionOnHistoricTaskAndNonePermissionOnProcessDefinition() { + // given + processEngineConfiguration.setEnableHistoricInstancePermissions(true); + + startProcessInstanceByKey(ONE_TASK_PROCESS_KEY); + String taskId = selectSingleTask().getId(); + disableAuthorization(); + taskService.complete(taskId); + enableAuthorization(); + + createGrantAuthorizationWithoutAuthentication(HISTORIC_TASK, taskId, userId, + HistoricTaskPermissions.READ); + createGrantAuthorizationWithoutAuthentication(PROCESS_DEFINITION, ONE_TASK_PROCESS_KEY, userId, + ProcessDefinitionPermissions.NONE); + + // when + UserOperationLogQuery query = historyService.createUserOperationLogQuery(); + + // then + assertThat(query.list()) + .extracting("taskId") + .containsExactly(taskId); + } + + public void testCheckNoneOnHistoricTaskAndTaskWorkerCategory() { + // given + processEngineConfiguration.setEnableHistoricInstancePermissions(true); + + startProcessInstanceByKey(ONE_TASK_PROCESS_KEY); + + String taskId = selectSingleTask().getId(); + setAssignee(taskId, "demo"); + + createGrantAuthorizationWithoutAuthentication(Resources.HISTORIC_TASK, + taskId, userId, HistoricTaskPermissions.NONE); + createGrantAuthorizationWithoutAuthentication(OPERATION_LOG_CATEGORY, CATEGORY_TASK_WORKER, + userId, UserOperationLogCategoryPermissions.READ); + + // when + UserOperationLogQuery query = historyService.createUserOperationLogQuery(); + + // then + assertThat(query.list()) + .extracting("taskId") + .containsExactly(taskId); + } + + public void testCheckReadOnHistoricTaskAndAdminCategory() { + // given + processEngineConfiguration.setEnableHistoricInstancePermissions(true); + + startProcessInstanceByKey(ONE_TASK_PROCESS_KEY); + + String taskId = selectSingleTask().getId(); + setAssignee(taskId, "demo"); + + createGrantAuthorizationWithoutAuthentication(Resources.HISTORIC_TASK, taskId, userId, + HistoricTaskPermissions.READ); + createGrantAuthorizationWithoutAuthentication(OPERATION_LOG_CATEGORY, + CATEGORY_ADMIN, userId, UserOperationLogCategoryPermissions.READ); + + // when + UserOperationLogQuery query = historyService.createUserOperationLogQuery(); + + // then + assertThat(query.list()) + .extracting("taskId") + .containsExactly(taskId); + } + + public void testHistoricTaskPermissionsAuthorizationDisabled() { + // given + processEngineConfiguration.setEnableHistoricInstancePermissions(true); + + startProcessInstanceByKey(ONE_TASK_PROCESS_KEY); + String taskId = selectSingleTask().getId(); + setAssignee(taskId, "demo"); + disableAuthorization(); + + // when + UserOperationLogQuery query = historyService.createUserOperationLogQuery(); + + // then + assertThat(query.list()) + .extracting("taskId") + .containsExactlyInAnyOrder(taskId, null); + } + public void testQuerySetAssigneeTaskUserOperationLogWithReadPermissionOnCategory() { // given startProcessInstanceByKey(ONE_TASK_PROCESS_KEY);