From 9f0179619f7d96f971f979d73d7e32cedd896e6e Mon Sep 17 00:00:00 2001 From: holgerhagen <19706592+holgerhagen@users.noreply.github.com> Date: Wed, 1 Jul 2020 14:19:09 +0200 Subject: [PATCH] TSK-1253: Prevent LDAP injection. --- .../taskana/common/rest/ldap/LdapClient.java | 15 ++++++++-- .../rest/AccessIdControllerIntTest.java | 28 +++++++++++++++++++ 2 files changed, 41 insertions(+), 2 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 82363f882..d5d591b77 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 @@ -17,9 +17,9 @@ import org.springframework.ldap.core.LdapTemplate; import org.springframework.ldap.core.support.AbstractContextMapper; import org.springframework.ldap.filter.AndFilter; import org.springframework.ldap.filter.EqualsFilter; -import org.springframework.ldap.filter.LikeFilter; import org.springframework.ldap.filter.OrFilter; import org.springframework.ldap.filter.WhitespaceWildcardsFilter; +import org.springframework.ldap.support.LdapNameBuilder; import org.springframework.stereotype.Component; import pro.taskana.common.api.exceptions.InvalidArgumentException; @@ -191,7 +191,18 @@ public class LdapClient { final AndFilter andFilter = new AndFilter(); andFilter.and(new EqualsFilter(getGroupSearchFilterName(), getGroupSearchFilterValue())); - andFilter.and(new LikeFilter(getGroupsOfUser(), "*" + accessId + "*")); + final OrFilter orFilter = new OrFilter(); + orFilter.or(new EqualsFilter(getGroupsOfUser(), accessId)); + orFilter.or( + new EqualsFilter( + getGroupsOfUser(), + LdapNameBuilder.newInstance() + .add(getBaseDn()) + .add(getUserSearchBase()) + .add("uid", accessId) + .build() + .toString())); + andFilter.and(orFilter); String[] userAttributesToReturn = {getUserIdAttribute(), getGroupNameAttribute()}; diff --git a/rest/taskana-rest-spring/src/test/java/pro/taskana/common/rest/AccessIdControllerIntTest.java b/rest/taskana-rest-spring/src/test/java/pro/taskana/common/rest/AccessIdControllerIntTest.java index 82bbfc85b..48154acb1 100644 --- a/rest/taskana-rest-spring/src/test/java/pro/taskana/common/rest/AccessIdControllerIntTest.java +++ b/rest/taskana-rest-spring/src/test/java/pro/taskana/common/rest/AccessIdControllerIntTest.java @@ -61,6 +61,17 @@ class AccessIdControllerIntTest { .containsExactly("cn=ksc-users,cn=groups,OU=Test,O=TASKANA"); } + @Test + void should_returnEmptyResults_ifInvalidCharacterIsUsedInCondition() { + ResponseEntity response = + TEMPLATE.exchange( + restHelper.toUrl(Mapping.URL_ACCESSID) + "?search-for=ksc-teamleads,cn=groups", + HttpMethod.GET, + restHelper.defaultRequest(), + ParameterizedTypeReference.forType(AccessIdListResource.class)); + assertThat(response.getBody()).isNotNull().isEmpty(); + } + @Test void testGetMatches() { ResponseEntity> response = @@ -128,6 +139,23 @@ class AccessIdControllerIntTest { + "cn=Organisationseinheit KSC,cn=organisation,OU=Test,O=TASKANA"); } + @Test + void should_returnBadRequest_ifAccessIdOfUserContainsInvalidCharacter() { + ThrowingCallable call = + () -> + TEMPLATE.exchange( + restHelper.toUrl(Mapping.URL_ACCESSID_GROUPS) + "?access-id=teamlead-2,cn=users", + HttpMethod.GET, + restHelper.defaultRequest(), + ParameterizedTypeReference.forType(AccessIdListResource.class)); + + assertThatThrownBy(call) + .isInstanceOf(HttpClientErrorException.class) + .hasMessageContaining("The accessId is invalid") + .extracting(ex -> ((HttpClientErrorException) ex).getStatusCode()) + .isEqualTo(HttpStatus.BAD_REQUEST); + } + @Test void should_returnAccessIdsOfGroupsTheAccessIdIsMemberOf_ifAccessIdOfGroupIsGiven() { ResponseEntity> response =