diff --git a/lib/taskana-core/src/main/java/pro/taskana/impl/InternalTaskanaEngine.java b/lib/taskana-core/src/main/java/pro/taskana/impl/InternalTaskanaEngine.java index 4300e1fe3..3aa4f509b 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/impl/InternalTaskanaEngine.java +++ b/lib/taskana-core/src/main/java/pro/taskana/impl/InternalTaskanaEngine.java @@ -75,4 +75,15 @@ public interface InternalTaskanaEngine { * @return the TaskRoutingProducer instance. */ TaskRoutingManager getTaskRoutingManager(); + + + /** + * This method is supposed to skip further permission checks if we are already in a secured + * environment. With great power comes great responsibility. + * + * @param supplier will be executed with admin privileges + * @param defined with the supplier return value + * @return output from supplier + */ + T runAsAdmin(Supplier supplier); } diff --git a/lib/taskana-core/src/main/java/pro/taskana/impl/TaskanaEngineImpl.java b/lib/taskana-core/src/main/java/pro/taskana/impl/TaskanaEngineImpl.java index cc63f9062..f64abbb4b 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/impl/TaskanaEngineImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/impl/TaskanaEngineImpl.java @@ -1,5 +1,8 @@ package pro.taskana.impl; +import java.security.AccessController; +import java.security.Principal; +import java.security.PrivilegedAction; import java.sql.Connection; import java.sql.SQLException; import java.time.Instant; @@ -10,6 +13,7 @@ import java.util.HashSet; import java.util.List; import java.util.Set; import java.util.function.Supplier; +import javax.security.auth.Subject; import org.apache.ibatis.mapping.Environment; import org.apache.ibatis.session.Configuration; import org.apache.ibatis.session.SqlSession; @@ -35,6 +39,7 @@ import pro.taskana.exceptions.AutocommitFailedException; import pro.taskana.exceptions.ConnectionNotSetException; import pro.taskana.exceptions.NotAuthorizedException; import pro.taskana.exceptions.SystemException; +import pro.taskana.exceptions.TaskanaRuntimeException; import pro.taskana.history.HistoryEventProducer; import pro.taskana.impl.persistence.InstantTypeHandler; import pro.taskana.impl.persistence.MapTypeHandler; @@ -50,6 +55,7 @@ import pro.taskana.mappings.TaskMonitorMapper; import pro.taskana.mappings.WorkbasketAccessMapper; import pro.taskana.mappings.WorkbasketMapper; import pro.taskana.security.CurrentUserContext; +import pro.taskana.security.GroupPrincipal; import pro.taskana.taskrouting.TaskRoutingManager; /** This is the implementation of TaskanaEngine. */ @@ -326,6 +332,31 @@ public class TaskanaEngineImpl implements TaskanaEngine { } } + @Override + public T runAsAdmin(Supplier supplier) { + + Subject subject = Subject.getSubject(AccessController.getContext()); + if (subject == null) { + // dont add authorisation if none is available. + return supplier.get(); + } + + Set principalsCopy = new HashSet<>(subject.getPrincipals()); + Set privateCredentialsCopy = new HashSet<>(subject.getPrivateCredentials()); + Set publicCredentialsCopy = new HashSet<>(subject.getPublicCredentials()); + + String adminName = + this.getEngine().getConfiguration().getRoleMap().get(TaskanaRole.ADMIN).stream() + .findFirst() + .orElseThrow(() -> new TaskanaRuntimeException("There is no admin configured")); + + principalsCopy.add(new GroupPrincipal(adminName)); + Subject subject1 = + new Subject(true, principalsCopy, privateCredentialsCopy, publicCredentialsCopy); + + return Subject.doAs(subject1, (PrivilegedAction) supplier::get); + } + @Override public void returnConnection() { if (mode != ConnectionManagementMode.EXPLICIT) { diff --git a/lib/taskana-core/src/main/java/pro/taskana/impl/WorkbasketServiceImpl.java b/lib/taskana-core/src/main/java/pro/taskana/impl/WorkbasketServiceImpl.java index 0f0d31db3..29b10116b 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/impl/WorkbasketServiceImpl.java +++ b/lib/taskana-core/src/main/java/pro/taskana/impl/WorkbasketServiceImpl.java @@ -1,7 +1,5 @@ package pro.taskana.impl; -import static pro.taskana.security.CurrentUserContext.runAsAdmin; - import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; @@ -651,58 +649,78 @@ public class WorkbasketServiceImpl implements WorkbasketService { LOGGER.debug("entry to deleteWorkbasket(workbasketId = {})", workbasketId); taskanaEngine.getEngine().checkRoleMembership(TaskanaRole.BUSINESS_ADMIN, TaskanaRole.ADMIN); + validateWorkbasketId(workbasketId); + try { taskanaEngine.openConnection(); - if (workbasketId == null || workbasketId.isEmpty()) { - throw new InvalidArgumentException( - "The WorkbasketId can´t be NULL or EMPTY for deleteWorkbasket()"); + + try { + this.getWorkbasket(workbasketId); + } catch (WorkbasketNotFoundException ex) { + LOGGER.debug("Workbasket with workbasketId = {} is already deleted?", workbasketId); + throw ex; } - // check if the workbasket does exist and is empty (Task) - this.getWorkbasket(workbasketId); + long countTasksNotCompletedInWorkbasket = + taskanaEngine.runAsAdmin(() -> getCountTasksNotCompletedByWorkbasketId(workbasketId)); - long numTasksNotCompletedInWorkbasket = - runAsAdmin( - () -> - taskanaEngine - .getEngine() - .getTaskService() - .createTaskQuery() - .workbasketIdIn(workbasketId) - .stateNotIn(TaskState.COMPLETED) - .count()); - - if (numTasksNotCompletedInWorkbasket > 0) { - throw new WorkbasketInUseException( - "Workbasket " - + workbasketId - + " contains non-completed tasks and can´t be marked for deletion."); + if (countTasksNotCompletedInWorkbasket > 0) { + String errorMessage = + String.format( + "Workbasket %s contains %s non-completed tasks and can´t be marked for deletion.", + workbasketId, countTasksNotCompletedInWorkbasket); + throw new WorkbasketInUseException(errorMessage); } - long numTasksInWorkbasket = - runAsAdmin( - () -> - taskanaEngine - .getEngine() - .getTaskService() - .createTaskQuery() - .workbasketIdIn(workbasketId) - .count()); + long countTasksInWorkbasket = + taskanaEngine.runAsAdmin(() -> getCountTasksByWorkbasketId(workbasketId)); - if (numTasksInWorkbasket == 0) { + boolean canBeDeletedNow = countTasksInWorkbasket == 0; + + if (canBeDeletedNow) { workbasketMapper.delete(workbasketId); deleteReferencesToWorkbasket(workbasketId); - return true; } else { markWorkbasketForDeletion(workbasketId); - return false; } + return canBeDeletedNow; } finally { taskanaEngine.returnConnection(); LOGGER.debug("exit from deleteWorkbasket(workbasketId = {})", workbasketId); } } + private void validateWorkbasketId(String workbasketId) throws InvalidArgumentException { + if (workbasketId == null) { + throw new InvalidArgumentException( + "The WorkbasketId can´t be NULL"); + } + + if (workbasketId.isEmpty()) { + throw new InvalidArgumentException( + "The WorkbasketId can´t be EMPTY for deleteWorkbasket()"); + } + } + + private long getCountTasksByWorkbasketId(String workbasketId) { + return taskanaEngine + .getEngine() + .getTaskService() + .createTaskQuery() + .workbasketIdIn(workbasketId) + .count(); + } + + private long getCountTasksNotCompletedByWorkbasketId(String workbasketId) { + return taskanaEngine + .getEngine() + .getTaskService() + .createTaskQuery() + .workbasketIdIn(workbasketId) + .stateNotIn(TaskState.COMPLETED) + .count(); + } + public BulkOperationResults deleteWorkbaskets( List workbasketsIds) throws NotAuthorizedException, InvalidArgumentException { if (LOGGER.isDebugEnabled()) { @@ -933,11 +951,7 @@ public class WorkbasketServiceImpl implements WorkbasketService { taskanaEngine.getEngine().checkRoleMembership(TaskanaRole.BUSINESS_ADMIN, TaskanaRole.ADMIN); try { taskanaEngine.openConnection(); - - if (workbasketId == null || workbasketId.isEmpty()) { - throw new InvalidArgumentException( - "The WorkbasketId can´t be NULL or EMPTY for markWorkbasketForDeletion()"); - } + validateWorkbasketId(workbasketId); WorkbasketImpl workbasket = workbasketMapper.findById(workbasketId); workbasket.setMarkedForDeletion(true); workbasketMapper.update(workbasket); diff --git a/lib/taskana-core/src/main/java/pro/taskana/security/CurrentUserContext.java b/lib/taskana-core/src/main/java/pro/taskana/security/CurrentUserContext.java index af70379ba..f2af833ff 100644 --- a/lib/taskana-core/src/main/java/pro/taskana/security/CurrentUserContext.java +++ b/lib/taskana-core/src/main/java/pro/taskana/security/CurrentUserContext.java @@ -5,13 +5,10 @@ import static pro.taskana.configuration.TaskanaEngineConfiguration.shouldUseLowe import java.lang.reflect.Method; import java.security.AccessController; import java.security.Principal; -import java.security.PrivilegedAction; import java.security.acl.Group; import java.util.ArrayList; -import java.util.HashSet; import java.util.List; import java.util.Set; -import java.util.function.Supplier; import javax.security.auth.Subject; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -80,33 +77,6 @@ public final class CurrentUserContext { return accessIds; } - /** - * This method is supposed to skip further permission checks if we are already in a secured - * environment. With great power comes great responsibility. - * - * @param supplier will be executed with admin privileges - * @param defined with the supplier return value - * @return output from supplier - */ - public static T runAsAdmin(Supplier supplier) { - - Subject subject = Subject.getSubject(AccessController.getContext()); - if (subject == null) { - // dont add authorisation if none is available. - return supplier.get(); - } - - Set principalsCopy = new HashSet<>(subject.getPrincipals()); - Set privateCredentialsCopy = new HashSet<>(subject.getPrivateCredentials()); - Set publicCredentialsCopy = new HashSet<>(subject.getPublicCredentials()); - - principalsCopy.add(new GroupPrincipal("admin")); - Subject subject1 = - new Subject(true, principalsCopy, privateCredentialsCopy, publicCredentialsCopy); - - return Subject.doAs(subject1, (PrivilegedAction) supplier::get); - } - /** * Returns the unique security name of the first public credentials found in the WSSubject as * userid. diff --git a/lib/taskana-core/src/test/java/acceptance/security/TaskEngineAccTest.java b/lib/taskana-core/src/test/java/acceptance/security/TaskEngineAccTest.java index fc0af4cda..0572f7858 100644 --- a/lib/taskana-core/src/test/java/acceptance/security/TaskEngineAccTest.java +++ b/lib/taskana-core/src/test/java/acceptance/security/TaskEngineAccTest.java @@ -10,7 +10,7 @@ import org.junit.jupiter.api.extension.ExtendWith; import pro.taskana.TaskanaRole; import pro.taskana.exceptions.NotAuthorizedException; -import pro.taskana.security.CurrentUserContext; +import pro.taskana.impl.TaskanaEngineProxyForTest; import pro.taskana.security.JaasExtension; import pro.taskana.security.WithAccessId; @@ -35,13 +35,17 @@ class TaskEngineAccTest extends AbstractAccTest { userName = "user_1_1", groupNames = {"businessadmin"}) @Test - void testRunAsAdminIsOnlyTemporary() { + void testRunAsAdminIsOnlyTemporary() throws NoSuchFieldException, IllegalAccessException { assertTrue(taskanaEngine.isUserInRole(TaskanaRole.BUSINESS_ADMIN)); assertFalse(taskanaEngine.isUserInRole(TaskanaRole.ADMIN)); - CurrentUserContext.runAsAdmin(() -> { - assertTrue(taskanaEngine.isUserInRole(TaskanaRole.ADMIN)); - return true; - }); + + new TaskanaEngineProxyForTest(taskanaEngine) + .getEngine() + .runAsAdmin( + () -> { + assertTrue(taskanaEngine.isUserInRole(TaskanaRole.ADMIN)); + return true; + }); assertFalse(taskanaEngine.isUserInRole(TaskanaRole.ADMIN)); } diff --git a/lib/taskana-core/src/test/java/pro/taskana/impl/TaskanaEngineProxyForTest.java b/lib/taskana-core/src/test/java/pro/taskana/impl/TaskanaEngineProxyForTest.java index bf3888df4..cc5b54d2e 100644 --- a/lib/taskana-core/src/test/java/pro/taskana/impl/TaskanaEngineProxyForTest.java +++ b/lib/taskana-core/src/test/java/pro/taskana/impl/TaskanaEngineProxyForTest.java @@ -21,6 +21,10 @@ public class TaskanaEngineProxyForTest { engine = (InternalTaskanaEngine) internal.get(taskanaEngine); } + public InternalTaskanaEngine getEngine() { + return engine; + } + public SqlSession getSqlSession() { return engine.getSqlSession(); }