diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/OperationsParamsValidator.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/OperationsParamsValidator.java index 2f290a910b59..6c416b479933 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/OperationsParamsValidator.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/OperationsParamsValidator.java @@ -30,16 +30,26 @@ import static org.hisp.dhis.common.OrganisationUnitSelectionMode.CAPTURE; import static org.hisp.dhis.security.Authorities.F_TRACKED_ENTITY_INSTANCE_SEARCH_IN_ALL_ORGUNITS; -import lombok.AccessLevel; -import lombok.NoArgsConstructor; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; +import lombok.RequiredArgsConstructor; import org.hisp.dhis.common.OrganisationUnitSelectionMode; +import org.hisp.dhis.dataelement.DataElement; import org.hisp.dhis.feedback.BadRequestException; +import org.hisp.dhis.feedback.ForbiddenException; import org.hisp.dhis.program.Program; +import org.hisp.dhis.security.acl.AclService; +import org.hisp.dhis.trackedentity.TrackedEntityAttribute; import org.hisp.dhis.user.User; +import org.springframework.stereotype.Component; -@NoArgsConstructor(access = AccessLevel.PRIVATE) +@Component +@RequiredArgsConstructor public class OperationsParamsValidator { + private final AclService aclService; + /** * Validates the user is authorized and/or has the necessary configuration set up in case the org * unit mode is ALL, ACCESSIBLE or CAPTURE. If the mode used is none of these three, no validation @@ -50,7 +60,7 @@ public class OperationsParamsValidator { * @throws BadRequestException if a validation error occurs for any of the three aforementioned * modes */ - public static void validateOrgUnitMode( + public void validateOrgUnitMode( OrganisationUnitSelectionMode orgUnitMode, User user, Program program) throws BadRequestException { switch (orgUnitMode) { @@ -61,7 +71,7 @@ public static void validateOrgUnitMode( } } - private static void validateUserCanSearchOrgUnitModeALL(User user) throws BadRequestException { + private void validateUserCanSearchOrgUnitModeALL(User user) throws BadRequestException { if (user != null && !(user.isSuper() || user.isAuthorized(F_TRACKED_ENTITY_INSTANCE_SEARCH_IN_ALL_ORGUNITS.name()))) { @@ -70,7 +80,7 @@ private static void validateUserCanSearchOrgUnitModeALL(User user) throws BadReq } } - private static void validateUserScope( + private void validateUserScope( User user, Program program, OrganisationUnitSelectionMode orgUnitMode) throws BadRequestException { @@ -89,11 +99,41 @@ private static void validateUserScope( } } - private static void validateCaptureScope(User user) throws BadRequestException { + private void validateCaptureScope(User user) throws BadRequestException { if (user == null) { throw new BadRequestException("User is required for orgUnitMode: " + CAPTURE); } else if (user.getOrganisationUnits().isEmpty()) { throw new BadRequestException("User needs to be assigned data capture orgunits"); } } + + public void validateOrderableAttributes(List order, User user) throws ForbiddenException { + Set orderableTeas = + order.stream() + .filter(o -> o.getField() instanceof TrackedEntityAttribute) + .map(o -> (TrackedEntityAttribute) o.getField()) + .collect(Collectors.toSet()); + + for (TrackedEntityAttribute tea : orderableTeas) { + if (!aclService.canDataRead(user, tea)) { + throw new ForbiddenException( + "User has no access to tracked entity attribute: " + tea.getUid()); + } + } + } + + public void validateOrderableDataElements(List order, User user) + throws ForbiddenException { + Set orderableDes = + order.stream() + .filter(o -> o.getField() instanceof DataElement) + .map(o -> (DataElement) o.getField()) + .collect(Collectors.toSet()); + + for (DataElement de : orderableDes) { + if (!aclService.canDataRead(user, de)) { + throw new ForbiddenException("User has no access to data element: " + de.getUid()); + } + } + } } diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventOperationParamsMapper.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventOperationParamsMapper.java index 26bbadebbfe8..c0cec02d0313 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventOperationParamsMapper.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventOperationParamsMapper.java @@ -28,7 +28,6 @@ package org.hisp.dhis.tracker.export.event; import static org.hisp.dhis.security.Authorities.F_TRACKED_ENTITY_INSTANCE_SEARCH_IN_ALL_ORGUNITS; -import static org.hisp.dhis.tracker.export.OperationsParamsValidator.validateOrgUnitMode; import java.util.List; import java.util.Map; @@ -53,6 +52,7 @@ import org.hisp.dhis.trackedentity.TrackedEntityAttribute; import org.hisp.dhis.trackedentity.TrackedEntityAttributeService; import org.hisp.dhis.trackedentity.TrackedEntityService; +import org.hisp.dhis.tracker.export.OperationsParamsValidator; import org.hisp.dhis.tracker.export.Order; import org.hisp.dhis.user.CurrentUserService; import org.hisp.dhis.user.User; @@ -85,6 +85,8 @@ class EventOperationParamsMapper { private final DataElementService dataElementService; + private final OperationsParamsValidator operationsParamsValidator; + @Transactional(readOnly = true) public EventQueryParams map(EventOperationParams operationParams) throws BadRequestException, ForbiddenException { @@ -96,7 +98,7 @@ public EventQueryParams map(EventOperationParams operationParams) OrganisationUnit orgUnit = validateRequestedOrgUnit(operationParams.getOrgUnitUid()); validateUser(user, program, programStage, orgUnit); - validateOrgUnitMode(operationParams.getOrgUnitMode(), user, program); + operationsParamsValidator.validateOrgUnitMode(operationParams.getOrgUnitMode(), user, program); TrackedEntity trackedEntity = validateTrackedEntity(operationParams.getTrackedEntityUid()); @@ -115,6 +117,8 @@ public EventQueryParams map(EventOperationParams operationParams) mapDataElementFilters(queryParams, operationParams.getDataElementFilters()); mapAttributeFilters(queryParams, operationParams.getAttributeFilters()); mapOrderParam(queryParams, operationParams.getOrder()); + operationsParamsValidator.validateOrderableAttributes(queryParams.getOrder(), user); + operationsParamsValidator.validateOrderableDataElements(queryParams.getOrder(), user); return queryParams .setProgram(program) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityOperationParamsMapper.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityOperationParamsMapper.java index f7f15b66324e..bb667d8cecbb 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityOperationParamsMapper.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityOperationParamsMapper.java @@ -47,6 +47,7 @@ import org.hisp.dhis.trackedentity.TrackedEntityAttributeService; import org.hisp.dhis.trackedentity.TrackedEntityType; import org.hisp.dhis.trackedentity.TrackedEntityTypeService; +import org.hisp.dhis.tracker.export.OperationsParamsValidator; import org.hisp.dhis.tracker.export.Order; import org.hisp.dhis.user.User; import org.springframework.stereotype.Component; @@ -67,6 +68,8 @@ class TrackedEntityOperationParamsMapper { @Nonnull private final TrackedEntityAttributeService attributeService; + @Nonnull private final OperationsParamsValidator operationsParamsValidator; + @Transactional(readOnly = true) public TrackedEntityQueryParams map(TrackedEntityOperationParams operationParams) throws BadRequestException, ForbiddenException { @@ -84,6 +87,7 @@ public TrackedEntityQueryParams map(TrackedEntityOperationParams operationParams mapAttributeFilters(params, operationParams.getFilters()); mapOrderParam(params, operationParams.getOrder()); + operationsParamsValidator.validateOrderableAttributes(params.getOrder(), user); params .setProgram(program) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/OperationsParamsValidatorTest.java b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/OperationsParamsValidatorTest.java index c1ccc30961d8..4d791bfd20b1 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/OperationsParamsValidatorTest.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/OperationsParamsValidatorTest.java @@ -29,7 +29,6 @@ import static org.hisp.dhis.common.OrganisationUnitSelectionMode.ALL; import static org.hisp.dhis.common.OrganisationUnitSelectionMode.CAPTURE; -import static org.hisp.dhis.tracker.export.OperationsParamsValidator.validateOrgUnitMode; import static org.junit.jupiter.api.Assertions.assertEquals; import java.util.Set; @@ -44,6 +43,7 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.EnumSource; +import org.mockito.InjectMocks; import org.mockito.junit.jupiter.MockitoExtension; @ExtendWith(MockitoExtension.class) @@ -57,6 +57,8 @@ class OperationsParamsValidatorTest { private final Program program = new Program("program"); + @InjectMocks private OperationsParamsValidator operationsParamsValidator; + @BeforeEach public void setUp() { OrganisationUnit organisationUnit = createOrgUnit("orgUnit", PARENT_ORG_UNIT_UID); @@ -67,7 +69,8 @@ public void setUp() { void shouldFailWhenOuModeCaptureAndUserHasNoOrgUnitsAssigned() { Exception exception = Assertions.assertThrows( - BadRequestException.class, () -> validateOrgUnitMode(CAPTURE, new User(), program)); + BadRequestException.class, + () -> operationsParamsValidator.validateOrgUnitMode(CAPTURE, new User(), program)); assertEquals("User needs to be assigned data capture orgunits", exception.getMessage()); } @@ -80,7 +83,8 @@ void shouldFailWhenOuModeRequiresUserScopeOrgUnitAndUserHasNoOrgUnitsAssigned( OrganisationUnitSelectionMode orgUnitMode) { Exception exception = Assertions.assertThrows( - BadRequestException.class, () -> validateOrgUnitMode(orgUnitMode, new User(), program)); + BadRequestException.class, + () -> operationsParamsValidator.validateOrgUnitMode(orgUnitMode, new User(), program)); assertEquals( "User needs to be assigned either search or data capture org units", @@ -91,7 +95,8 @@ void shouldFailWhenOuModeRequiresUserScopeOrgUnitAndUserHasNoOrgUnitsAssigned( void shouldFailWhenOuModeAllAndNotSuperuser() { Exception exception = Assertions.assertThrows( - BadRequestException.class, () -> validateOrgUnitMode(ALL, new User(), program)); + BadRequestException.class, + () -> operationsParamsValidator.validateOrgUnitMode(ALL, new User(), program)); assertEquals( "Current user is not authorized to query across all organisation units", diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/event/EventOperationParamsMapperTest.java b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/event/EventOperationParamsMapperTest.java index 6d2ac17ac751..62d5a5cdbae9 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/event/EventOperationParamsMapperTest.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/event/EventOperationParamsMapperTest.java @@ -358,6 +358,8 @@ void shouldMapOrderInGivenOrder() throws BadRequestException, ForbiddenException de1.setUid(DE_1_UID); when(dataElementService.getDataElement(DE_1_UID)).thenReturn(de1); when(dataElementService.getDataElement(TEA_1_UID)).thenReturn(null); + when(aclService.canDataRead(user, de1)).thenReturn(true); + when(aclService.canDataRead(user, tea1)).thenReturn(true); EventOperationParams operationParams = eventBuilder @@ -574,6 +576,65 @@ void shouldMapOrgUnitAndModeWhenModeAllAndUserIsAuthorized(String userName) assertEquals(ALL, params.getOrgUnitMode()); } + @Test + void shouldThrowWhenUserHasNoAccessToDataElementOrder() { + User user = new User(); + UserRole userRole = new UserRole(); + userRole.setAuthorities(Set.of(F_TRACKED_ENTITY_INSTANCE_SEARCH_IN_ALL_ORGUNITS.name())); + user.setUserRoles(Set.of(userRole)); + + when(currentUserService.getCurrentUser()).thenReturn(user); + when(organisationUnitService.getOrganisationUnit(orgUnitId)).thenReturn(orgUnit); + + String deUid = CodeGenerator.generateUid(); + DataElement de = new DataElement(); + de.setUid(deUid); + when(dataElementService.getDataElement(deUid)).thenReturn(de); + when(aclService.canDataRead(user, de)).thenReturn(false); + + EventOperationParams operationParams = + eventBuilder + .orgUnitMode(ALL) + .orgUnitUid(orgUnit.getUid()) + .orderBy(UID.of(deUid), SortDirection.DESC) + .build(); + + ForbiddenException exception = + assertThrows(ForbiddenException.class, () -> mapper.map(operationParams)); + + assertEquals("User has no access to data element: " + deUid, exception.getMessage()); + } + + @Test + void shouldThrowWhenUserHasNoAccessToTrackedEntityAttributeOrder() { + User user = new User(); + UserRole userRole = new UserRole(); + userRole.setAuthorities(Set.of(F_TRACKED_ENTITY_INSTANCE_SEARCH_IN_ALL_ORGUNITS.name())); + user.setUserRoles(Set.of(userRole)); + + when(currentUserService.getCurrentUser()).thenReturn(user); + when(organisationUnitService.getOrganisationUnit(orgUnitId)).thenReturn(orgUnit); + + String teaUid = CodeGenerator.generateUid(); + TrackedEntityAttribute tea = new TrackedEntityAttribute(); + tea.setUid(teaUid); + when(trackedEntityAttributeService.getTrackedEntityAttribute(teaUid)).thenReturn(tea); + when(aclService.canDataRead(user, tea)).thenReturn(false); + + EventOperationParams operationParams = + eventBuilder + .orgUnitMode(ALL) + .orgUnitUid(orgUnit.getUid()) + .orderBy(UID.of(teaUid), SortDirection.DESC) + .build(); + + ForbiddenException exception = + assertThrows(ForbiddenException.class, () -> mapper.map(operationParams)); + + assertEquals( + "User has no access to tracked entity attribute: " + teaUid, exception.getMessage()); + } + private User createUserWithAuthority(Authorities authority) { User user = new User(); UserRole userRole = new UserRole(); diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityOperationParamsMapperTest.java b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityOperationParamsMapperTest.java index 330ad78f4f97..c1a7862fee8e 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityOperationParamsMapperTest.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityOperationParamsMapperTest.java @@ -65,6 +65,7 @@ import org.hisp.dhis.program.ProgramService; import org.hisp.dhis.program.ProgramStage; import org.hisp.dhis.program.ProgramStatus; +import org.hisp.dhis.security.acl.AclService; import org.hisp.dhis.trackedentity.TrackedEntityAttribute; import org.hisp.dhis.trackedentity.TrackedEntityAttributeService; import org.hisp.dhis.trackedentity.TrackedEntityType; @@ -110,6 +111,8 @@ class TrackedEntityOperationParamsMapperTest { @Mock private TrackedEntityTypeService trackedEntityTypeService; + @Mock private AclService aclService; + @InjectMocks private TrackedEntityOperationParamsMapper mapper; private User user; @@ -553,4 +556,26 @@ void shouldFailToMapGivenInvalidOrderNameWhichIsAValidUID() { assertThrows(BadRequestException.class, () -> mapper.map(operationParams)); assertStartsWith("Cannot order by 'lastUpdated'", exception.getMessage()); } + + @Test + void shouldThrowWhenUserHasNoAccessToTrackedEntityAttributeOrder() { + TrackedEntityAttribute tea = new TrackedEntityAttribute(); + tea.setUid(TEA_1_UID); + when(attributeService.getTrackedEntityAttribute(TEA_1_UID)).thenReturn(tea); + + when(aclService.canDataRead(user, tea)).thenReturn(false); + + TrackedEntityOperationParams operationParams = + TrackedEntityOperationParams.builder() + .orgUnitMode(ACCESSIBLE) + .user(user) + .orderBy(UID.of(TEA_1_UID), SortDirection.ASC) + .build(); + + ForbiddenException exception = + assertThrows(ForbiddenException.class, () -> mapper.map(operationParams)); + + assertEquals( + "User has no access to tracked entity attribute: " + TEA_1_UID, exception.getMessage()); + } } diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/OrderAndPaginationExporterTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/OrderAndPaginationExporterTest.java index 7be8f923fa6b..a0a7b1905645 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/OrderAndPaginationExporterTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/OrderAndPaginationExporterTest.java @@ -28,14 +28,17 @@ package org.hisp.dhis.tracker.export; import static org.hisp.dhis.common.OrganisationUnitSelectionMode.ACCESSIBLE; +import static org.hisp.dhis.common.OrganisationUnitSelectionMode.ALL; import static org.hisp.dhis.common.OrganisationUnitSelectionMode.DESCENDANTS; import static org.hisp.dhis.common.OrganisationUnitSelectionMode.SELECTED; +import static org.hisp.dhis.security.Authorities.F_TRACKED_ENTITY_INSTANCE_SEARCH_IN_ALL_ORGUNITS; import static org.hisp.dhis.tracker.Assertions.assertNoErrors; import static org.hisp.dhis.utils.Assertions.assertContainsOnly; import static org.hisp.dhis.utils.Assertions.assertIsEmpty; import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import java.io.IOException; import java.util.Comparator; @@ -107,6 +110,8 @@ class OrderAndPaginationExporterTest extends TrackerTest { @Override protected void initTest() throws IOException { + + userService = _userService; setUpMetadata("tracker/simple_metadata.json"); importUser = userService.getUser("M5zQapPyTZI"); assertNoErrors( @@ -419,6 +424,24 @@ void shouldOrderTrackedEntitiesByAttributeDesc() assertEquals(List.of("QS6w44flWAf", "dUE514NMOlo"), trackedEntities); } + @Test + void shouldThrowWhenGetTrackedEntityAndUserHasNoAccessToTrackedEntityOrder() { + User user = createUserWithAuth("user_no_access_tea"); + user.setOrganisationUnits(Set.of(orgUnit)); + + TrackedEntityOperationParams params = + TrackedEntityOperationParams.builder() + .organisationUnits(Set.of(orgUnit.getUid())) + .orgUnitMode(SELECTED) + .trackedEntityUids(Set.of("QS6w44flWAf", "dUE514NMOlo")) + .trackedEntityTypeUid(trackedEntityType.getUid()) + .user(user) + .orderBy(UID.of("toDelete000"), SortDirection.DESC) + .build(); + + assertThrows(ForbiddenException.class, () -> getTrackedEntities(params)); + } + @Test void shouldOrderTrackedEntitiesByAttributeWhenFiltered() throws ForbiddenException, BadRequestException, NotFoundException { @@ -1252,6 +1275,40 @@ void shouldOrderRelationshipsByCreatedDesc() throws ForbiddenException, NotFound } } + @Test + void shouldThrowWhenGetEventsAndUserHasNoAccessToDataElementOrder() { + User user = + createUserWithAuth( + "de_user_search_all_ou", F_TRACKED_ENTITY_INSTANCE_SEARCH_IN_ALL_ORGUNITS.name()); + injectSecurityContext(user); + + EventOperationParams params = + eventParamsBuilder + .orgUnitUid(orgUnit.getUid()) + .orgUnitMode(ALL) + .orderBy(UID.of("DATAEL00001"), SortDirection.DESC) + .build(); + + assertThrows(ForbiddenException.class, () -> getEvents(params)); + } + + @Test + void shouldThrowWhenGetEventsAndUserHasNoAccessToTrackedEntityOrder() { + User user = + createUserWithAuth( + "tea_user_search_all_ou", F_TRACKED_ENTITY_INSTANCE_SEARCH_IN_ALL_ORGUNITS.name()); + injectSecurityContext(user); + + EventOperationParams params = + eventParamsBuilder + .orgUnitUid(orgUnit.getUid()) + .orgUnitMode(ALL) + .orderBy(UID.of("toDelete000"), SortDirection.DESC) + .build(); + + assertThrows(ForbiddenException.class, () -> getEvents(params)); + } + private T get(Class type, String uid) { T t = manager.get(type, uid); assertNotNull( diff --git a/dhis-2/dhis-test-integration/src/test/resources/tracker/simple_metadata.json b/dhis-2/dhis-test-integration/src/test/resources/tracker/simple_metadata.json index ebe7a300b8ae..0938771d4b21 100644 --- a/dhis-2/dhis-test-integration/src/test/resources/tracker/simple_metadata.json +++ b/dhis-2/dhis-test-integration/src/test/resources/tracker/simple_metadata.json @@ -462,7 +462,7 @@ "shortName": "test-dataelement1", "aggregationType": "SUM", "domainType": "TRACKER", - "publicAccess": "rw------", + "publicAccess": "--------", "valueType": "TEXT", "zeroIsSignificant": false, "categoryCombo": { @@ -2443,7 +2443,7 @@ "shortName": "to-delete-tei-attribute", "aggregationType": "NONE", "displayInListNoProgram": false, - "publicAccess": "rw------", + "publicAccess": "--------", "pattern": "", "skipSynchronization": false, "generated": false,