From d0a7e11880432da2c669d0f7b79fd4c893a18db3 Mon Sep 17 00:00:00 2001 From: Tobias Metzke-Bernstein Date: Wed, 5 Jul 2023 13:19:04 +0200 Subject: [PATCH] test(engine+webapps): simplify revoke ALWAYS tests (#3540) * Removes repeated authorization test cases (~1600) for revoke mode ALWAYS. Instead, ensures that mode ALWAYS leads equivalent queries compared to when AUTO mode triggers. The latter is well-tested, ensuring ALWAYS works as well. related to https://github.com/camunda/camunda-bpm-platform/issues/3530 --- .../AuthorizationRevokeModeAlwaysTest.java | 110 ++++++++++++++++++ .../api/authorization/AuthorizationTest.java | 26 ----- .../SetTaskPropertyAuthorizationTest.java | 29 +++-- .../base/authorization/AuthorizationTest.java | 27 ----- 4 files changed, 123 insertions(+), 69 deletions(-) create mode 100644 engine/src/test/java/org/camunda/bpm/engine/test/api/authorization/AuthorizationRevokeModeAlwaysTest.java diff --git a/engine/src/test/java/org/camunda/bpm/engine/test/api/authorization/AuthorizationRevokeModeAlwaysTest.java b/engine/src/test/java/org/camunda/bpm/engine/test/api/authorization/AuthorizationRevokeModeAlwaysTest.java new file mode 100644 index 00000000000..f712bec2efe --- /dev/null +++ b/engine/src/test/java/org/camunda/bpm/engine/test/api/authorization/AuthorizationRevokeModeAlwaysTest.java @@ -0,0 +1,110 @@ +/* + * Copyright Camunda Services GmbH and/or licensed to Camunda Services GmbH + * under one or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information regarding copyright + * ownership. Camunda licenses this file to you under the Apache License, + * Version 2.0; you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.camunda.bpm.engine.test.api.authorization; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.camunda.bpm.engine.authorization.Authorization.ANY; +import static org.camunda.bpm.engine.authorization.Permissions.READ; +import static org.camunda.bpm.engine.authorization.Permissions.UPDATE; +import static org.camunda.bpm.engine.authorization.Resources.TASK; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.spi.ILoggingEvent; +import java.util.List; +import org.camunda.bpm.engine.impl.cfg.ProcessEngineConfigurationImpl; +import org.camunda.commons.testing.ProcessEngineLoggingRule; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; + +public class AuthorizationRevokeModeAlwaysTest extends AuthorizationTest { + + protected static final String LOGGING_CONTEXT = "org.camunda.bpm.engine.impl.persistence.entity.TaskEntity"; + + protected String defaultRevokeMode; + + @Rule + public ProcessEngineLoggingRule loggingRule = new ProcessEngineLoggingRule() + .watch(LOGGING_CONTEXT, Level.DEBUG); + + @Before + public void storeRevokeMode() { + defaultRevokeMode = processEngineConfiguration.getAuthorizationCheckRevokes(); + } + + @After + public void resetRevokeMode() { + processEngineConfiguration.setAuthorizationCheckRevokes(defaultRevokeMode); + } + + @Test + public void shouldCreateEqualQueriesForModesAlwaysAndAutoWhenRevokeExists() { + // given + disableAuthorization(); + testRule.deploy("org/camunda/bpm/engine/test/api/oneTaskProcess.bpmn20.xml"); + runtimeService.startProcessInstanceByKey("oneTaskProcess"); + createGrantAuthorization(TASK, ANY, userId, READ); + createRevokeAuthorization(TASK, ANY, userId, UPDATE); + enableAuthorization(); + + processEngineConfiguration.setAuthorizationCheckRevokes(ProcessEngineConfigurationImpl.AUTHORIZATION_CHECK_REVOKE_ALWAYS); + int taskLogOffset = loggingRule.getLog(LOGGING_CONTEXT).size(); + + taskService.createTaskQuery().list(); + List taskLog = loggingRule.getLog(LOGGING_CONTEXT); + String modeAlwaysQuery = taskLog.get(taskLogOffset).getFormattedMessage(); + taskLogOffset = taskLog.size(); + + processEngineConfiguration.setAuthorizationCheckRevokes(ProcessEngineConfigurationImpl.AUTHORIZATION_CHECK_REVOKE_AUTO); + + // when + taskService.createTaskQuery().list(); + + // then + String modeAutoQuery = loggingRule.getLog(LOGGING_CONTEXT).get(taskLogOffset).getFormattedMessage(); + assertThat(modeAutoQuery).containsIgnoringCase("Preparing: select").isEqualTo(modeAlwaysQuery); + } + + @Test + public void shouldCreateUnequalQueriesForModesAlwaysAndAutoWhenNoRevokeExists() { + // given + disableAuthorization(); + testRule.deploy("org/camunda/bpm/engine/test/api/oneTaskProcess.bpmn20.xml"); + runtimeService.startProcessInstanceByKey("oneTaskProcess"); + createGrantAuthorization(TASK, ANY, userId, READ); + enableAuthorization(); + + processEngineConfiguration.setAuthorizationCheckRevokes(ProcessEngineConfigurationImpl.AUTHORIZATION_CHECK_REVOKE_ALWAYS); + int taskLogOffset = loggingRule.getLog(LOGGING_CONTEXT).size(); + + taskService.createTaskQuery().list(); + List taskLog = loggingRule.getLog(LOGGING_CONTEXT); + String modeAlwaysQuery = taskLog.get(taskLogOffset).getFormattedMessage(); + taskLogOffset = taskLog.size(); + + processEngineConfiguration.setAuthorizationCheckRevokes(ProcessEngineConfigurationImpl.AUTHORIZATION_CHECK_REVOKE_AUTO); + + // when + taskService.createTaskQuery().list(); + + // then + String modeAutoQuery = loggingRule.getLog(LOGGING_CONTEXT).get(taskLogOffset).getFormattedMessage(); + assertThat(modeAutoQuery).containsIgnoringCase("Preparing: select").isNotEqualTo(modeAlwaysQuery); + } + +} diff --git a/engine/src/test/java/org/camunda/bpm/engine/test/api/authorization/AuthorizationTest.java b/engine/src/test/java/org/camunda/bpm/engine/test/api/authorization/AuthorizationTest.java index 9d9a74134ba..b3c16f8bbde 100644 --- a/engine/src/test/java/org/camunda/bpm/engine/test/api/authorization/AuthorizationTest.java +++ b/engine/src/test/java/org/camunda/bpm/engine/test/api/authorization/AuthorizationTest.java @@ -33,7 +33,6 @@ import java.util.List; import java.util.Map; import java.util.concurrent.Callable; -import org.camunda.bpm.engine.ProcessEngineConfiguration; import org.camunda.bpm.engine.ProcessEngineException; import org.camunda.bpm.engine.authorization.Authorization; import org.camunda.bpm.engine.authorization.Permission; @@ -57,15 +56,10 @@ import org.camunda.bpm.engine.variable.Variables; import org.junit.After; import org.junit.Before; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; -import org.junit.runners.Parameterized.Parameter; -import org.junit.runners.Parameterized.Parameters; /** * @author Roman Smirnov */ -@RunWith(Parameterized.class) public abstract class AuthorizationTest extends PluggableProcessEngineTest { protected String userId = "test"; @@ -77,22 +71,6 @@ public abstract class AuthorizationTest extends PluggableProcessEngineTest { protected static final String VARIABLE_VALUE = "aVariableValue"; protected List deploymentIds = new ArrayList<>(); - @Parameter(0) - public String authorizationRevokeMode; - protected String defaultAuthorizationRevokeMode; - - /** - * Parameters: - * authorizationRevokeMode: The revoke authorization mode to use in engine configuration - */ - @Parameters(name = "revoke check mode {0}") - public static List data() { - return Arrays.asList(new Object[][] { - { ProcessEngineConfiguration.AUTHORIZATION_CHECK_REVOKE_ALWAYS }, - { ProcessEngineConfiguration.AUTHORIZATION_CHECK_REVOKE_AUTO } - }); - } - @Before public void setUp() throws Exception { user = createUser(userId); @@ -102,15 +80,11 @@ public void setUp() throws Exception { identityService.setAuthentication(userId, Arrays.asList(groupId)); processEngineConfiguration.setAuthorizationEnabled(true); - - defaultAuthorizationRevokeMode = processEngineConfiguration.getAuthorizationCheckRevokes(); - processEngineConfiguration.setAuthorizationCheckRevokes(authorizationRevokeMode); } @After public void tearDown() { processEngineConfiguration.setAuthorizationEnabled(false); - processEngineConfiguration.setAuthorizationCheckRevokes(defaultAuthorizationRevokeMode); for (User user : identityService.createUserQuery().list()) { identityService.deleteUser(user.getId()); } diff --git a/engine/src/test/java/org/camunda/bpm/engine/test/api/authorization/SetTaskPropertyAuthorizationTest.java b/engine/src/test/java/org/camunda/bpm/engine/test/api/authorization/SetTaskPropertyAuthorizationTest.java index 1b9af19b1f1..26ec5779003 100644 --- a/engine/src/test/java/org/camunda/bpm/engine/test/api/authorization/SetTaskPropertyAuthorizationTest.java +++ b/engine/src/test/java/org/camunda/bpm/engine/test/api/authorization/SetTaskPropertyAuthorizationTest.java @@ -30,7 +30,6 @@ import java.util.Date; import java.util.List; import org.camunda.bpm.engine.AuthorizationException; -import org.camunda.bpm.engine.ProcessEngineConfiguration; import org.camunda.bpm.engine.TaskService; import org.camunda.bpm.engine.task.Task; import org.camunda.bpm.engine.test.util.ClockTestUtil; @@ -41,9 +40,12 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameter; import org.junit.runners.Parameterized.Parameters; +@RunWith(Parameterized.class) public class SetTaskPropertyAuthorizationTest extends AuthorizationTest { protected static final String PROCESS_KEY = "oneTaskProcess"; @@ -51,13 +53,13 @@ public class SetTaskPropertyAuthorizationTest extends AuthorizationTest { @Rule public EntityRemoveRule entityRemoveRule = EntityRemoveRule.of(testRule); - @Parameter(1) + @Parameter(0) public String operationName; - @Parameter(2) + @Parameter(1) public TriConsumer operation; - @Parameter(3) + @Parameter(2) public String taskId; - @Parameter(4) + @Parameter(3) public Object value; protected boolean deleteTask; @@ -70,7 +72,7 @@ public class SetTaskPropertyAuthorizationTest extends AuthorizationTest { * setValue: The value to use to set property to * taskQueryBuilderMethodName: The corresponding taskQuery builder method name to use for assertion purposes */ - @Parameters(name = "{1} (mode {0})") + @Parameters(name = "{0}") public static List data() { TriConsumer setPriority = (taskService, taskId, value) -> taskService.setPriority(taskId, (int) value); TriConsumer setName = (taskService, taskId, value) -> taskService.setName(taskId, (String) value); @@ -79,16 +81,11 @@ public static List data() { TriConsumer setFollowUpDate = (taskService, taskId, value) -> taskService.setFollowUpDate(taskId, (Date) value); return Arrays.asList(new Object[][] { - { ProcessEngineConfiguration.AUTHORIZATION_CHECK_REVOKE_ALWAYS, "setPriority", setPriority, "taskId", 80 }, - { ProcessEngineConfiguration.AUTHORIZATION_CHECK_REVOKE_ALWAYS, "setName", setName, "taskId", "name" }, - { ProcessEngineConfiguration.AUTHORIZATION_CHECK_REVOKE_ALWAYS, "setDescription", setDescription, "taskId", "description" }, - { ProcessEngineConfiguration.AUTHORIZATION_CHECK_REVOKE_ALWAYS, "setDueDate", setDueDate, "taskId", ClockTestUtil.setClockToDateWithoutMilliseconds() }, - { ProcessEngineConfiguration.AUTHORIZATION_CHECK_REVOKE_ALWAYS, "setFollowUpDate", setFollowUpDate, "taskId", ClockTestUtil.setClockToDateWithoutMilliseconds() }, - { ProcessEngineConfiguration.AUTHORIZATION_CHECK_REVOKE_AUTO, "setPriority", setPriority, "taskId", 80 }, - { ProcessEngineConfiguration.AUTHORIZATION_CHECK_REVOKE_AUTO, "setName", setName, "taskId", "name" }, - { ProcessEngineConfiguration.AUTHORIZATION_CHECK_REVOKE_AUTO, "setDescription", setDescription, "taskId", "description" }, - { ProcessEngineConfiguration.AUTHORIZATION_CHECK_REVOKE_AUTO, "setDueDate", setDueDate, "taskId", ClockTestUtil.setClockToDateWithoutMilliseconds()}, - { ProcessEngineConfiguration.AUTHORIZATION_CHECK_REVOKE_AUTO, "setFollowUpDate", setFollowUpDate, "taskId", ClockTestUtil.setClockToDateWithoutMilliseconds() } + {"setPriority", setPriority, "taskId", 80 }, + {"setName", setName, "taskId", "name" }, + {"setDescription", setDescription, "taskId", "description" }, + {"setDueDate", setDueDate, "taskId", ClockTestUtil.setClockToDateWithoutMilliseconds() }, + {"setFollowUpDate", setFollowUpDate, "taskId", ClockTestUtil.setClockToDateWithoutMilliseconds() } }); } diff --git a/webapps/assembly/src/test/java/org/camunda/bpm/cockpit/plugin/base/authorization/AuthorizationTest.java b/webapps/assembly/src/test/java/org/camunda/bpm/cockpit/plugin/base/authorization/AuthorizationTest.java index b337616a53d..bbefcbbfbb8 100644 --- a/webapps/assembly/src/test/java/org/camunda/bpm/cockpit/plugin/base/authorization/AuthorizationTest.java +++ b/webapps/assembly/src/test/java/org/camunda/bpm/cockpit/plugin/base/authorization/AuthorizationTest.java @@ -23,12 +23,10 @@ import static org.camunda.bpm.engine.authorization.Resources.USER; import java.util.Arrays; -import java.util.List; import org.camunda.bpm.cockpit.plugin.test.AbstractCockpitPluginTest; import org.camunda.bpm.engine.AuthorizationService; import org.camunda.bpm.engine.IdentityService; import org.camunda.bpm.engine.ProcessEngine; -import org.camunda.bpm.engine.ProcessEngineConfiguration; import org.camunda.bpm.engine.RepositoryService; import org.camunda.bpm.engine.RuntimeService; import org.camunda.bpm.engine.authorization.Authorization; @@ -44,16 +42,11 @@ import org.camunda.bpm.engine.runtime.ProcessInstance; import org.junit.After; import org.junit.Before; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; -import org.junit.runners.Parameterized.Parameter; -import org.junit.runners.Parameterized.Parameters; /** * @author Roman Smirnov * */ -@RunWith(Parameterized.class) public abstract class AuthorizationTest extends AbstractCockpitPluginTest { protected ProcessEngine processEngine; @@ -70,22 +63,6 @@ public abstract class AuthorizationTest extends AbstractCockpitPluginTest { protected User user; protected Group group; - @Parameter(0) - public String authorizationRevokeMode; - protected String defaultAuthorizationRevokeMode; - - /** - * Parameters: - * authorizationRevokeMode: The revoke authorization mode to use in engine configuration - */ - @Parameters(name = "revoke check mode {0}") - public static List data() { - return Arrays.asList(new Object[][] { - { ProcessEngineConfiguration.AUTHORIZATION_CHECK_REVOKE_ALWAYS }, - { ProcessEngineConfiguration.AUTHORIZATION_CHECK_REVOKE_AUTO } - }); - } - @Before public void setUp() throws Exception { super.before(); @@ -106,15 +83,11 @@ public void setUp() throws Exception { identityService.setAuthentication(userId, Arrays.asList(groupId)); enableAuthorization(); - - defaultAuthorizationRevokeMode = processEngineConfiguration.getAuthorizationCheckRevokes(); - processEngineConfiguration.setAuthorizationCheckRevokes(authorizationRevokeMode); } @After public void tearDown() { disableAuthorization(); - processEngineConfiguration.setAuthorizationCheckRevokes(defaultAuthorizationRevokeMode); for (User user : identityService.createUserQuery().list()) { identityService.deleteUser(user.getId()); }