TSK-1029: Refactor runAsAdmin method and usage

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 <T> defined with the supplier return value
+   * @return output from supplier
+   */
+  <T> T runAsAdmin(Supplier<T> supplier);
 }

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> T runAsAdmin(Supplier<T> supplier) {
+
+      Subject subject = Subject.getSubject(AccessController.getContext());
+      if (subject == null) {
+        // dont add authorisation if none is available.
+        return supplier.get();
+      }
+
+      Set<Principal> principalsCopy = new HashSet<>(subject.getPrincipals());
+      Set<Object> privateCredentialsCopy = new HashSet<>(subject.getPrivateCredentials());
+      Set<Object> 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<T>) supplier::get);
+    }
+
     @Override
     public void returnConnection() {
       if (mode != ConnectionManagementMode.EXPLICIT) {

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(
+      try {
-            "The WorkbasketId can´t be NULL or EMPTY for deleteWorkbasket()");
+        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)
+      long countTasksNotCompletedInWorkbasket =
-      this.getWorkbasket(workbasketId);
+          taskanaEngine.runAsAdmin(() -> getCountTasksNotCompletedByWorkbasketId(workbasketId));
 
-      long numTasksNotCompletedInWorkbasket =
+      if (countTasksNotCompletedInWorkbasket > 0) {
-          runAsAdmin(
+        String errorMessage =
-              () ->
+            String.format(
-                  taskanaEngine
+                "Workbasket %s contains %s non-completed tasks and can´t be marked for deletion.",
-                      .getEngine()
+                workbasketId, countTasksNotCompletedInWorkbasket);
-                      .getTaskService()
+        throw new WorkbasketInUseException(errorMessage);
-                      .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.");
       }
 
-      long numTasksInWorkbasket =
+      long countTasksInWorkbasket =
-          runAsAdmin(
+          taskanaEngine.runAsAdmin(() -> getCountTasksByWorkbasketId(workbasketId));
-              () ->
-                  taskanaEngine
-                      .getEngine()
-                      .getTaskService()
-                      .createTaskQuery()
-                      .workbasketIdIn(workbasketId)
-                      .count());
 
-      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<String, TaskanaException> deleteWorkbaskets(
       List<String> 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();
-
+      validateWorkbasketId(workbasketId);
-      if (workbasketId == null || workbasketId.isEmpty()) {
-        throw new InvalidArgumentException(
-            "The WorkbasketId can´t be NULL or EMPTY for markWorkbasketForDeletion()");
-      }
       WorkbasketImpl workbasket = workbasketMapper.findById(workbasketId);
       workbasket.setMarkedForDeletion(true);
       workbasketMapper.update(workbasket);

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 <T> defined with the supplier return value
-   * @return output from supplier
-   */
-  public static <T> T runAsAdmin(Supplier<T> supplier) {
-
-    Subject subject = Subject.getSubject(AccessController.getContext());
-    if (subject == null) {
-      // dont add authorisation if none is available.
-      return supplier.get();
-    }
-
-    Set<Principal> principalsCopy = new HashSet<>(subject.getPrincipals());
-    Set<Object> privateCredentialsCopy = new HashSet<>(subject.getPrivateCredentials());
-    Set<Object> publicCredentialsCopy = new HashSet<>(subject.getPublicCredentials());
-
-    principalsCopy.add(new GroupPrincipal("admin"));
-    Subject subject1 =
-        new Subject(true, principalsCopy, privateCredentialsCopy, publicCredentialsCopy);
-
-    return Subject.doAs(subject1, (PrivilegedAction<T>) supplier::get);
-  }
-
   /**
    * Returns the unique security name of the first public credentials found in the WSSubject as
    * userid.

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));
+    new TaskanaEngineProxyForTest(taskanaEngine)
-      return true;
+        .getEngine()
-    });
+        .runAsAdmin(
+            () -> {
+              assertTrue(taskanaEngine.isUserInRole(TaskanaRole.ADMIN));
+              return true;
+            });
     assertFalse(taskanaEngine.isUserInRole(TaskanaRole.ADMIN));
   }
 

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();
   }