TSK-1029: Fix deletion of workbasket as business admin not possible

This commit is contained in:
Benjamin Eckstein 2020-01-20 15:39:32 +01:00 committed by Holger Hagen
parent 5a08fd4b03
commit fae9d1fb9b
5 changed files with 52 additions and 38 deletions

View File

@ -1,5 +1,7 @@
package pro.taskana.impl;
import static pro.taskana.security.CurrentUserContext.runAsAdmin;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Arrays;
@ -660,13 +662,15 @@ public class WorkbasketServiceImpl implements WorkbasketService {
this.getWorkbasket(workbasketId);
long numTasksNotCompletedInWorkbasket =
taskanaEngine
.getEngine()
.getTaskService()
.createTaskQuery()
.workbasketIdIn(workbasketId)
.stateNotIn(TaskState.COMPLETED)
.count();
runAsAdmin(
() ->
taskanaEngine
.getEngine()
.getTaskService()
.createTaskQuery()
.workbasketIdIn(workbasketId)
.stateNotIn(TaskState.COMPLETED)
.count());
if (numTasksNotCompletedInWorkbasket > 0) {
throw new WorkbasketInUseException(
@ -676,12 +680,14 @@ public class WorkbasketServiceImpl implements WorkbasketService {
}
long numTasksInWorkbasket =
taskanaEngine
.getEngine()
.getTaskService()
.createTaskQuery()
.workbasketIdIn(workbasketId)
.count();
runAsAdmin(
() ->
taskanaEngine
.getEngine()
.getTaskService()
.createTaskQuery()
.workbasketIdIn(workbasketId)
.count());
if (numTasksInWorkbasket == 0) {
workbasketMapper.delete(workbasketId);

View File

@ -5,10 +5,12 @@ 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.List;
import java.util.Set;
import java.util.function.Supplier;
import javax.security.auth.Subject;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -77,6 +79,31 @@ 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> principals = subject.getPrincipals();
Set<Object> privateCredentials = subject.getPrivateCredentials();
Set<Object> publicCredentials = subject.getPublicCredentials();
principals.add(new GroupPrincipal("admin"));
Subject subject1 = new Subject(true, principals, privateCredentials, publicCredentials);
return Subject.doAs(subject1, (PrivilegedAction<T>) supplier::get);
}
/**
* Returns the unique security name of the first public credentials found in the WSSubject as
* userid.

View File

@ -17,12 +17,10 @@ import pro.taskana.TaskService;
import pro.taskana.Workbasket;
import pro.taskana.WorkbasketAccessItem;
import pro.taskana.WorkbasketService;
import pro.taskana.WorkbasketType;
import pro.taskana.exceptions.InvalidArgumentException;
import pro.taskana.exceptions.InvalidOwnerException;
import pro.taskana.exceptions.InvalidStateException;
import pro.taskana.exceptions.NotAuthorizedException;
import pro.taskana.exceptions.NotAuthorizedToQueryWorkbasketException;
import pro.taskana.exceptions.TaskNotFoundException;
import pro.taskana.exceptions.WorkbasketInUseException;
import pro.taskana.exceptions.WorkbasketNotFoundException;
@ -148,23 +146,6 @@ class DeleteWorkbasketAccTest extends AbstractAccTest {
WorkbasketInUseException.class, () -> workbasketService.deleteWorkbasket(wb.getId()));
}
@WithAccessId(
userName = "user_1_2",
groupNames = {"businessadmin"})
@Test
void testCreateAndDeleteWorkbasket() throws Exception {
WorkbasketService workbasketService = taskanaEngine.getWorkbasketService();
Workbasket workbasket = workbasketService.newWorkbasket("NT1234", "DOMAIN_A");
workbasket.setName("TheUltimate");
workbasket.setType(WorkbasketType.GROUP);
workbasket.setOrgLevel1("company");
Workbasket workbasket2 = workbasketService.createWorkbasket(workbasket);
Assertions.assertThrows(
NotAuthorizedToQueryWorkbasketException.class,
() -> workbasketService.deleteWorkbasket(workbasket2.getId()));
}
@WithAccessId(
userName = "teamlead_2",
groupNames = {"businessadmin"})

View File

@ -142,6 +142,7 @@ public class WorkbasketController extends AbstractPagingController {
throws NotAuthorizedException, InvalidArgumentException, WorkbasketNotFoundException,
WorkbasketInUseException {
LOGGER.debug("Entry to markWorkbasketForDeletion(workbasketId= {})", workbasketId);
//http status code accepted because workbaskets will not be deleted immediately
ResponseEntity<?> response =
ResponseEntity.accepted().body(workbasketService.deleteWorkbasket(workbasketId));
LOGGER.debug("Exit from markWorkbasketForDeletion(), returning {}", response);

View File

@ -109,13 +109,12 @@ class WorkbasketControllerIntTest {
}
/**
* Bug Ticket TSK-1029
*
* <p>Businessadmin is allowed to delete any workbasket ticket without user related access
* restrictions
* Bug Ticket TSK-1029.
* Businessadmin is allowed to delete any workbasket ticket without user related access
* restrictions.
*/
@Test
void testWorkbasketDeletePermission() {
void testDeleteWorkbasketPermissionWithBusinessAdmin() {
String workbasketID = "WBI:100000000000000000000000000000000005";
@ -125,7 +124,7 @@ class WorkbasketControllerIntTest {
HttpMethod.DELETE,
new HttpEntity<>(restHelper.getHeadersBusinessAdmin()),
Void.class);
assertEquals(HttpStatus.NO_CONTENT, response.getStatusCode());
assertEquals(HttpStatus.ACCEPTED, response.getStatusCode());
}
@Test