From 8760bdceb366e1d078c4d4244b18352e3d328450 Mon Sep 17 00:00:00 2001 From: holgerhagen <19706592+holgerhagen@users.noreply.github.com> Date: Wed, 24 Jun 2020 11:59:42 +0200 Subject: [PATCH] TSK-1196: improved accessId check before deleting all user access items --- .../taskana/common/rest/ldap/LdapClient.java | 39 ++++++++++++++---- .../rest/WorkbasketAccessItemController.java | 5 +-- ...WorkbasketAccessItemControllerIntTest.java | 40 +++++++++++++++++-- 3 files changed, 69 insertions(+), 15 deletions(-) diff --git a/rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/ldap/LdapClient.java b/rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/ldap/LdapClient.java index ba2b90ae1..8436adf91 100644 --- a/rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/ldap/LdapClient.java +++ b/rest/taskana-rest-spring/src/main/java/pro/taskana/common/rest/ldap/LdapClient.java @@ -75,7 +75,7 @@ public class LdapClient { accessIds.add(groupByDn); } } else { - accessIds.addAll(searchUsersByName(name)); + accessIds.addAll(searchUsersByNameOrAccessId(name)); accessIds.addAll(searchGroupsByName(name)); } sortListOfAccessIdResources(accessIds); @@ -90,9 +90,9 @@ public class LdapClient { return result; } - public List searchUsersByName(final String name) + public List searchUsersByNameOrAccessId(final String name) throws InvalidArgumentException { - LOGGER.debug("entry to searchUsersByName(name = {}).", name); + LOGGER.debug("entry to searchUsersByNameOrAccessId(name = {}).", name); isInitOrFail(); testMinSearchForLength(name); @@ -116,7 +116,31 @@ public class LdapClient { SearchControls.SUBTREE_SCOPE, userAttributesToReturn, new UserContextMapper()); - LOGGER.debug("exit from searchUsersByName. Retrieved the following users: {}.", accessIds); + LOGGER.debug( + "exit from searchUsersByNameOrAccessId. Retrieved the following users: {}.", accessIds); + return accessIds; + } + + public List getUsersByAccessId(final String accessId) { + LOGGER.debug("entry to searchUsersByAccessId(name = {}).", accessId); + isInitOrFail(); + + final AndFilter andFilter = new AndFilter(); + andFilter.and(new EqualsFilter(getUserSearchFilterName(), getUserSearchFilterValue())); + andFilter.and(new EqualsFilter(getUserIdAttribute(), accessId)); + + String[] userAttributesToReturn = { + getUserFirstnameAttribute(), getUserLastnameAttribute(), getUserIdAttribute() + }; + + final List accessIds = + ldapTemplate.search( + getUserSearchBase(), + andFilter.encode(), + SearchControls.SUBTREE_SCOPE, + userAttributesToReturn, + new UserContextMapper()); + LOGGER.debug("exit from searchUsersByAccessId. Retrieved the following users: {}.", accessIds); return accessIds; } @@ -260,8 +284,8 @@ public class LdapClient { return LdapSettings.TASKANA_LDAP_GROUPS_OF_USER.getValueFromEnv(env); } - public boolean isGroup(String accessId) { - return accessId.contains(getGroupSearchBase()); + public boolean isUser(String accessId) { + return !getUsersByAccessId(accessId).isEmpty(); } boolean nameIsDn(String name) { @@ -293,9 +317,8 @@ public class LdapClient { String[] getLookUpGoupAttributesToReturn() { if (CN.equals(getGroupNameAttribute())) { return new String[] {CN}; - } else { - return new String[] {getGroupNameAttribute(), CN}; } + return new String[] {getGroupNameAttribute(), CN}; } @PostConstruct diff --git a/rest/taskana-rest-spring/src/main/java/pro/taskana/workbasket/rest/WorkbasketAccessItemController.java b/rest/taskana-rest-spring/src/main/java/pro/taskana/workbasket/rest/WorkbasketAccessItemController.java index 86e09a632..becd7792b 100644 --- a/rest/taskana-rest-spring/src/main/java/pro/taskana/workbasket/rest/WorkbasketAccessItemController.java +++ b/rest/taskana-rest-spring/src/main/java/pro/taskana/workbasket/rest/WorkbasketAccessItemController.java @@ -115,7 +115,7 @@ public class WorkbasketAccessItemController extends AbstractPagingController { @RequestParam("access-id") String accessId) throws NotAuthorizedException, InvalidArgumentException { LOGGER.debug("Entry to removeWorkbasketAccessItems(access-id= {})", accessId); - if (!ldapClient.isGroup(accessId)) { + if (ldapClient.isUser(accessId)) { List workbasketAccessItemList = workbasketService.createWorkbasketAccessItemQuery().accessIdIn(accessId).list(); @@ -125,8 +125,7 @@ public class WorkbasketAccessItemController extends AbstractPagingController { } else { throw new InvalidArgumentException( String.format( - "%s corresponding to a group, not a user. " - + "You just can remove access items for a user", + "AccessId '%s' is not a user. " + "You can remove all access items for users only.", accessId)); } diff --git a/rest/taskana-rest-spring/src/test/java/pro/taskana/workbasket/rest/WorkbasketAccessItemControllerIntTest.java b/rest/taskana-rest-spring/src/test/java/pro/taskana/workbasket/rest/WorkbasketAccessItemControllerIntTest.java index 1b1326eb0..d0e6cca1c 100644 --- a/rest/taskana-rest-spring/src/test/java/pro/taskana/workbasket/rest/WorkbasketAccessItemControllerIntTest.java +++ b/rest/taskana-rest-spring/src/test/java/pro/taskana/workbasket/rest/WorkbasketAccessItemControllerIntTest.java @@ -120,9 +120,9 @@ class WorkbasketAccessItemControllerIntTest { } @Test - void testRemoveWorkbasketAccessItemsOfUser() { + void should_deleteAllAccessItemForUser_ifValidAccessIdOfUserIsSupplied() { - String parameters = "?access-id=user-1-1"; + String parameters = "?access-id=teamlead-2"; ResponseEntity response = template.exchange( restHelper.toUrl(Mapping.URL_WORKBASKET_ACCESS_ITEMS) + parameters, @@ -134,8 +134,40 @@ class WorkbasketAccessItemControllerIntTest { } @Test - void testGetBadRequestIfTryingToDeleteAccessItemsForGroup() { - String parameters = "?access-id=cn=monitor-users,cn=groups,OU=Test,O=TASKANA"; + void should_returnBadRequest_ifAccessIdIsSubStringOfUser() { + String parameters = "?access-id=user-1"; + ThrowingCallable httpCall = + () -> + template.exchange( + restHelper.toUrl(Mapping.URL_WORKBASKET_ACCESS_ITEMS) + parameters, + HttpMethod.DELETE, + restHelper.defaultRequest(), + ParameterizedTypeReference.forType(Void.class)); + assertThatThrownBy(httpCall) + .isInstanceOf(HttpClientErrorException.class) + .extracting(ex -> ((HttpClientErrorException) ex).getStatusCode()) + .isEqualTo(HttpStatus.BAD_REQUEST); + } + + @Test + void should_returnBadRequest_ifAccessIdIsGroup() { + String parameters = "?access-id=cn=monitor-users,cn=groups,ou=test,o=taskana"; + ThrowingCallable httpCall = + () -> + template.exchange( + restHelper.toUrl(Mapping.URL_WORKBASKET_ACCESS_ITEMS) + parameters, + HttpMethod.DELETE, + restHelper.defaultRequest(), + ParameterizedTypeReference.forType(Void.class)); + assertThatThrownBy(httpCall) + .isInstanceOf(HttpClientErrorException.class) + .extracting(ex -> ((HttpClientErrorException) ex).getStatusCode()) + .isEqualTo(HttpStatus.BAD_REQUEST); + } + + @Test + void should_returnBadRequest_ifAccessIdIsOrganizationalGroup() { + String parameters = "?access-id=cn=organisationseinheit ksc,cn=organisation,ou=test,o=taskana"; ThrowingCallable httpCall = () -> template.exchange(