From 491c70d226754266b2217ac2482e26ccdedefbfe Mon Sep 17 00:00:00 2001 From: muilpp Date: Fri, 3 Nov 2023 08:47:12 +0100 Subject: [PATCH 1/9] fix: Remove unnecessary capture mode check [TECH-1589] --- .../TrackedEntityOperationParamsMapper.java | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) 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..454573019f8b 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 @@ -33,7 +33,6 @@ import java.util.Set; import javax.annotation.Nonnull; import lombok.RequiredArgsConstructor; -import org.hisp.dhis.common.OrganisationUnitSelectionMode; import org.hisp.dhis.common.QueryFilter; import org.hisp.dhis.common.UID; import org.hisp.dhis.feedback.BadRequestException; @@ -76,9 +75,7 @@ public TrackedEntityQueryParams map(TrackedEntityOperationParams operationParams validateTrackedEntityType(operationParams.getTrackedEntityTypeUid()); User user = operationParams.getUser(); - Set orgUnits = - validateOrgUnits( - user, operationParams.getOrganisationUnits(), operationParams.getOrgUnitMode()); + Set orgUnits = validateOrgUnits(user, operationParams.getOrganisationUnits()); TrackedEntityQueryParams params = new TrackedEntityQueryParams(); mapAttributeFilters(params, operationParams.getFilters()); @@ -135,8 +132,7 @@ private void mapAttributeFilters( } } - private Set validateOrgUnits( - User user, Set orgUnitIds, OrganisationUnitSelectionMode orgUnitMode) + private Set validateOrgUnits(User user, Set orgUnitIds) throws BadRequestException, ForbiddenException { Set orgUnits = new HashSet<>(); for (String orgUnitUid : orgUnitIds) { @@ -156,10 +152,6 @@ private Set validateOrgUnits( orgUnits.add(orgUnit); } - if (orgUnitMode == OrganisationUnitSelectionMode.CAPTURE && user != null) { - orgUnits.addAll(user.getOrganisationUnits()); - } - return orgUnits; } From 14f1816a4c056e204978edb754f4d2b23fc25f87 Mon Sep 17 00:00:00 2001 From: muilpp Date: Fri, 3 Nov 2023 15:59:58 +0100 Subject: [PATCH 2/9] fix: Add test to validate mode ALL behavior [TECH-1589] --- .../TrackedEntityServiceTest.java | 70 ++++++++++++++++++- 1 file changed, 69 insertions(+), 1 deletion(-) diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityServiceTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityServiceTest.java index 1bd60004f83f..400fb1cb5406 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityServiceTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityServiceTest.java @@ -27,6 +27,7 @@ */ package org.hisp.dhis.tracker.export.trackedentity; +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.tracker.TrackerTestUtils.oneHourAfter; @@ -37,6 +38,7 @@ import static org.hisp.dhis.utils.Assertions.assertContains; import static org.hisp.dhis.utils.Assertions.assertContainsOnly; import static org.hisp.dhis.utils.Assertions.assertIsEmpty; +import static org.hisp.dhis.utils.Assertions.assertNotEmpty; import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -65,6 +67,7 @@ import org.hisp.dhis.common.BaseIdentifiableObject; import org.hisp.dhis.common.CodeGenerator; import org.hisp.dhis.common.IdentifiableObjectManager; +import org.hisp.dhis.common.IllegalQueryException; import org.hisp.dhis.common.QueryFilter; import org.hisp.dhis.common.QueryOperator; import org.hisp.dhis.common.ValueType; @@ -127,6 +130,8 @@ class TrackedEntityServiceTest extends IntegrationTestBase { private User user; + private User userWithSearchInAllAuthority; + private User admin; private OrganisationUnit orgUnitA; @@ -141,6 +146,8 @@ class TrackedEntityServiceTest extends IntegrationTestBase { private Program programB; + private Program programC; + private Enrollment enrollmentA; private Enrollment enrollmentB; @@ -173,7 +180,6 @@ private static List uids( @Override protected void setUpTest() throws Exception { userService = _userService; - admin = preCreateInjectAdminUser(); orgUnitA = createOrganisationUnit('A'); manager.save(orgUnitA, false); @@ -182,9 +188,23 @@ protected void setUpTest() throws Exception { OrganisationUnit orgUnitC = createOrganisationUnit('C'); manager.save(orgUnitC, false); + admin = preCreateInjectAdminUser(); + admin.setOrganisationUnits(Set.of(orgUnitA, orgUnitB)); + manager.save(admin); + user = createAndAddUser(false, "user", Set.of(orgUnitA), Set.of(orgUnitA), "F_EXPORT_DATA"); user.setTeiSearchOrganisationUnits(Set.of(orgUnitA, orgUnitB, orgUnitC)); + userWithSearchInAllAuthority = + createAndAddUser( + false, + "userSearchInAll", + Set.of(orgUnitA), + Set.of(orgUnitA), + "F_TRACKED_ENTITY_INSTANCE_SEARCH_IN_ALL_ORGUNITS"); + userWithSearchInAllAuthority.setTeiSearchOrganisationUnits( + Set.of(orgUnitA, orgUnitB, orgUnitC)); + teaA = createTrackedEntityAttribute('A', ValueType.TEXT); manager.save(teaA, false); TrackedEntityAttribute teaB = createTrackedEntityAttribute('B', ValueType.TEXT); @@ -216,6 +236,7 @@ protected void setUpTest() throws Exception { programA.setProgramType(ProgramType.WITH_REGISTRATION); programA.setTrackedEntityType(trackedEntityTypeA); programA.setCategoryCombo(defaultCategoryCombo); + programA.setMinAttributesRequiredToSearch(0); manager.save(programA, false); ProgramStage programStageA1 = createProgramStage(programA); programStageA1.setPublicAccess(AccessStringHelper.FULL); @@ -254,6 +275,14 @@ protected void setUpTest() throws Exception { programB.getSharing().setPublicAccess(AccessStringHelper.FULL); manager.update(programB); + programC = createProgram('C', new HashSet<>(), orgUnitC); + programC.setProgramType(ProgramType.WITH_REGISTRATION); + programC.setTrackedEntityType(trackedEntityTypeA); + programC.setCategoryCombo(defaultCategoryCombo); + programC.setAccessLevel(AccessLevel.PROTECTED); + programC.getSharing().setPublicAccess(AccessStringHelper.READ); + manager.save(programC, false); + programB .getProgramAttributes() .addAll( @@ -1361,6 +1390,45 @@ void shouldReturnTrackedEntityRelationshipsWithTei2Event() () -> assertEquals(eventA.getUid(), actual.getTo().getEvent().getUid())); } + @Test + void shouldFailWhenModeAllUserCanSearchEverywhereButNotSuperuserAndNoAccessToProgram() { + injectSecurityContext(userWithSearchInAllAuthority); + TrackedEntityOperationParams operationParams = + TrackedEntityOperationParams.builder() + .orgUnitMode(ALL) + .programUid(programC.getUid()) + .user(userWithSearchInAllAuthority) + .build(); + + IllegalQueryException ex = + assertThrows( + IllegalQueryException.class, + () -> trackedEntityService.getTrackedEntities(operationParams)); + + assertContains( + String.format( + "Current user is not authorized to read data from selected program: %s", + programC.getUid()), + ex.getMessage()); + } + + @Test + void shouldReturnAllEntitiesWhenSuperuserAndModeAll() + throws ForbiddenException, BadRequestException, NotFoundException { + injectSecurityContext(admin); + TrackedEntityParams params = + new TrackedEntityParams(true, TrackedEntityEnrollmentParams.TRUE, false, false); + TrackedEntityOperationParams operationParams = + TrackedEntityOperationParams.builder() + .orgUnitMode(ALL) + .programUid(programA.getUid()) + .user(admin) + .build(); + + List trackedEntities = trackedEntityService.getTrackedEntities(operationParams); + assertNotEmpty(trackedEntities); + } + private Set attributeNames(final Collection attributes) { // depends on createTrackedEntityAttribute() prefixing with "Attribute" return attributes.stream() From c44b92c65c00c14af89f383482d61e6789d320d9 Mon Sep 17 00:00:00 2001 From: muilpp Date: Fri, 3 Nov 2023 16:10:56 +0100 Subject: [PATCH 3/9] fix: Remove unnecessary test param [TECH-1589] --- .../tracker/export/trackedentity/TrackedEntityServiceTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityServiceTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityServiceTest.java index 400fb1cb5406..325b251cda0e 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityServiceTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityServiceTest.java @@ -1416,8 +1416,6 @@ void shouldFailWhenModeAllUserCanSearchEverywhereButNotSuperuserAndNoAccessToPro void shouldReturnAllEntitiesWhenSuperuserAndModeAll() throws ForbiddenException, BadRequestException, NotFoundException { injectSecurityContext(admin); - TrackedEntityParams params = - new TrackedEntityParams(true, TrackedEntityEnrollmentParams.TRUE, false, false); TrackedEntityOperationParams operationParams = TrackedEntityOperationParams.builder() .orgUnitMode(ALL) From 2207140069defd3205dcd83d8c0150e51b6abb5c Mon Sep 17 00:00:00 2001 From: muilpp Date: Fri, 3 Nov 2023 16:20:18 +0100 Subject: [PATCH 4/9] fix: Validate tracked entity returned in test [TECH-1589] --- .../tracker/export/trackedentity/TrackedEntityServiceTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityServiceTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityServiceTest.java index 325b251cda0e..25ee1f3eb0dc 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityServiceTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityServiceTest.java @@ -38,7 +38,6 @@ import static org.hisp.dhis.utils.Assertions.assertContains; import static org.hisp.dhis.utils.Assertions.assertContainsOnly; import static org.hisp.dhis.utils.Assertions.assertIsEmpty; -import static org.hisp.dhis.utils.Assertions.assertNotEmpty; import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -1424,7 +1423,7 @@ void shouldReturnAllEntitiesWhenSuperuserAndModeAll() .build(); List trackedEntities = trackedEntityService.getTrackedEntities(operationParams); - assertNotEmpty(trackedEntities); + assertContainsOnly(Set.of(trackedEntityA.getUid()), uids(trackedEntities)); } private Set attributeNames(final Collection attributes) { From 38822ab331170e1d6133d785aaf554d59c248969 Mon Sep 17 00:00:00 2001 From: muilpp Date: Mon, 6 Nov 2023 09:37:03 +0100 Subject: [PATCH 5/9] fix: Validate user authorized when mode ALL in enrollments [TECH-1589] --- .../enrollment/DefaultEnrollmentService.java | 9 ---- .../EnrollmentOperationParamsMapper.java | 3 ++ .../enrollment/EnrollmentServiceTest.java | 43 ++++++++++++++++++- 3 files changed, 45 insertions(+), 10 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/DefaultEnrollmentService.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/DefaultEnrollmentService.java index 6ecdf046ec00..cc21908c3b6d 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/DefaultEnrollmentService.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/DefaultEnrollmentService.java @@ -27,7 +27,6 @@ */ package org.hisp.dhis.tracker.export.enrollment; -import static org.hisp.dhis.common.OrganisationUnitSelectionMode.ACCESSIBLE; import static org.hisp.dhis.common.OrganisationUnitSelectionMode.CAPTURE; import static org.hisp.dhis.common.OrganisationUnitSelectionMode.CHILDREN; import static org.hisp.dhis.common.OrganisationUnitSelectionMode.DESCENDANTS; @@ -292,14 +291,6 @@ public void validate(EnrollmentQueryParams params) throws IllegalQueryException throw new IllegalQueryException("Params cannot be null"); } - User user = params.getUser(); - - if (params.isOrganisationUnitMode(ACCESSIBLE) - && (user == null || !user.hasDataViewOrganisationUnitWithFallback())) { - violation = - "Current user must be associated with at least one organisation unit when selection mode is ACCESSIBLE"; - } - if (params.hasProgram() && params.hasTrackedEntityType()) { violation = "Program and tracked entity cannot be specified simultaneously"; } diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentOperationParamsMapper.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentOperationParamsMapper.java index afeedfbd83a1..84ee815ac6a7 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentOperationParamsMapper.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentOperationParamsMapper.java @@ -27,6 +27,8 @@ */ package org.hisp.dhis.tracker.export.enrollment; +import static org.hisp.dhis.tracker.export.OperationsParamsValidator.validateOrgUnitMode; + import java.util.HashSet; import java.util.Set; import lombok.RequiredArgsConstructor; @@ -72,6 +74,7 @@ public EnrollmentQueryParams map(EnrollmentOperationParams operationParams) User user = currentUserService.getCurrentUser(); Set orgUnits = validateOrgUnits(operationParams.getOrgUnitUids(), user); + validateOrgUnitMode(operationParams.getOrgUnitMode(), user, program); EnrollmentQueryParams params = new EnrollmentQueryParams(); params.setProgram(program); diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentServiceTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentServiceTest.java index e8fbf65b1783..a9b1dbfa1a8a 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentServiceTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentServiceTest.java @@ -27,7 +27,10 @@ */ package org.hisp.dhis.tracker.export.enrollment; +import static org.hisp.dhis.common.OrganisationUnitSelectionMode.ACCESSIBLE; +import static org.hisp.dhis.common.OrganisationUnitSelectionMode.ALL; import static org.hisp.dhis.common.OrganisationUnitSelectionMode.CAPTURE; +import static org.hisp.dhis.common.OrganisationUnitSelectionMode.SELECTED; import static org.hisp.dhis.tracker.TrackerTestUtils.oneHourAfter; import static org.hisp.dhis.tracker.TrackerTestUtils.oneHourBefore; import static org.hisp.dhis.tracker.TrackerTestUtils.uids; @@ -69,6 +72,7 @@ import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValue; import org.hisp.dhis.user.User; import org.hisp.dhis.user.UserService; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -410,7 +414,10 @@ void shouldGetEnrollmentsWhenUserHasReadAccessToProgramAndNoOrgUnitNorOrgUnitMod manager.updateNoAcl(programA); EnrollmentOperationParams params = - EnrollmentOperationParams.builder().programUid(programA.getUid()).build(); + EnrollmentOperationParams.builder() + .programUid(programA.getUid()) + .orgUnitMode(ACCESSIBLE) + .build(); List enrollments = enrollmentService.getEnrollments(params); @@ -447,6 +454,7 @@ void shouldGetEnrollmentWhenEnrollmentsAndOtherParamsAreSpecified() EnrollmentOperationParams.builder() .programUid(programA.getUid()) .enrollmentUids(Set.of(enrollmentA.getUid())) + .orgUnitMode(ACCESSIBLE) .build(); List enrollments = enrollmentService.getEnrollments(params); @@ -464,6 +472,7 @@ void shouldGetEnrollmentsByTrackedEntityWhenUserHasAccessToTrackedEntityType() EnrollmentOperationParams params = EnrollmentOperationParams.builder() .orgUnitUids(Set.of(trackedEntityA.getOrganisationUnit().getUid())) + .orgUnitMode(SELECTED) .trackedEntityUid(trackedEntityA.getUid()) .build(); @@ -485,6 +494,7 @@ void shouldFailGettingEnrollmentsByTrackedEntityWhenUserHasNoAccessToTrackedEnti EnrollmentOperationParams params = EnrollmentOperationParams.builder() .orgUnitUids(Set.of(trackedEntityA.getOrganisationUnit().getUid())) + .orgUnitMode(SELECTED) .trackedEntityUid(trackedEntityA.getUid()) .build(); @@ -501,6 +511,7 @@ void shouldReturnEnrollmentIfEnrollmentWasUpdatedBeforePassedDateAndTime() EnrollmentOperationParams operationParams = EnrollmentOperationParams.builder() .orgUnitUids(Set.of(orgUnitA.getUid())) + .orgUnitMode(SELECTED) .lastUpdated(oneHourBeforeLastUpdated) .build(); @@ -517,6 +528,7 @@ void shouldReturnEmptyIfEnrollmentWasUpdatedAfterPassedDateAndTime() EnrollmentOperationParams operationParams = EnrollmentOperationParams.builder() .orgUnitUids(Set.of(orgUnitA.getUid())) + .orgUnitMode(SELECTED) .lastUpdated(oneHourAfterLastUpdated) .build(); @@ -534,6 +546,7 @@ void shouldReturnEnrollmentIfEnrollmentStartedBeforePassedDateAndTime() EnrollmentOperationParams operationParams = EnrollmentOperationParams.builder() .orgUnitUids(Set.of(orgUnitA.getUid())) + .orgUnitMode(SELECTED) .programUid(programA.getUid()) .programStartDate(oneHourBeforeEnrollmentDate) .build(); @@ -552,6 +565,7 @@ void shouldReturnEmptyIfEnrollmentStartedAfterPassedDateAndTime() EnrollmentOperationParams operationParams = EnrollmentOperationParams.builder() .orgUnitUids(Set.of(orgUnitA.getUid())) + .orgUnitMode(SELECTED) .programUid(programA.getUid()) .programStartDate(oneHourAfterEnrollmentDate) .build(); @@ -570,6 +584,7 @@ void shouldReturnEnrollmentIfEnrollmentEndedAfterPassedDateAndTime() EnrollmentOperationParams operationParams = EnrollmentOperationParams.builder() .orgUnitUids(Set.of(orgUnitA.getUid())) + .orgUnitMode(SELECTED) .programUid(programA.getUid()) .programEndDate(oneHourAfterEnrollmentDate) .build(); @@ -588,6 +603,7 @@ void shouldReturnEmptyIfEnrollmentEndedBeforePassedDateAndTime() EnrollmentOperationParams operationParams = EnrollmentOperationParams.builder() .orgUnitUids(Set.of(orgUnitA.getUid())) + .orgUnitMode(SELECTED) .programUid(programA.getUid()) .programEndDate(oneHourBeforeEnrollmentDate) .build(); @@ -597,6 +613,31 @@ void shouldReturnEmptyIfEnrollmentEndedBeforePassedDateAndTime() assertIsEmpty(enrollments); } + @Test + void shouldFailWhenOrgUnitModeAllAndUserNotAuthorized() { + EnrollmentOperationParams operationParams = + EnrollmentOperationParams.builder().orgUnitMode(ALL).build(); + + BadRequestException exception = + Assertions.assertThrows( + BadRequestException.class, () -> enrollmentService.getEnrollments(operationParams)); + Assertions.assertEquals( + "Current user is not authorized to query across all organisation units", + exception.getMessage()); + } + + @Test + void shouldReturnAllEnrollmentsWhenOrgUnitModeAllAndUserAuthorized() + throws ForbiddenException, BadRequestException, NotFoundException { + injectSecurityContext(admin); + + EnrollmentOperationParams operationParams = + EnrollmentOperationParams.builder().orgUnitMode(ALL).build(); + + List enrollments = enrollmentService.getEnrollments(operationParams); + assertContainsOnly(List.of(enrollmentA, enrollmentB, enrollmentChildA), enrollments); + } + private static List attributeUids(Enrollment enrollment) { return enrollment.getTrackedEntity().getTrackedEntityAttributeValues().stream() .map(v -> v.getAttribute().getUid()) From f8a82b833a80e2859092c6c1d4077d037e084fde Mon Sep 17 00:00:00 2001 From: muilpp Date: Mon, 6 Nov 2023 10:24:50 +0100 Subject: [PATCH 6/9] fix: Add org unit mode in enrollment service tests [TECH-1589] --- .../EnrollmentOperationParamsMapperTest.java | 32 +++++++++++++++---- .../OrderAndPaginationExporterTest.java | 9 +++++- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentOperationParamsMapperTest.java b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentOperationParamsMapperTest.java index 403db5e52553..447fa72bbaa1 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentOperationParamsMapperTest.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentOperationParamsMapperTest.java @@ -27,6 +27,8 @@ */ package org.hisp.dhis.tracker.export.enrollment; +import static org.hisp.dhis.common.OrganisationUnitSelectionMode.ACCESSIBLE; +import static org.hisp.dhis.common.OrganisationUnitSelectionMode.SELECTED; import static org.hisp.dhis.utils.Assertions.assertContainsOnly; import static org.hisp.dhis.utils.Assertions.assertIsEmpty; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -118,6 +120,8 @@ void setUp() { orgUnit2.getUid(), user.getTeiSearchOrganisationUnitsWithFallback())) .thenReturn(true); + user.setTeiSearchOrganisationUnits(Set.of(orgUnit1, orgUnit2)); + program = new Program(); program.setUid(PROGRAM_UID); when(programService.getProgram(PROGRAM_UID)).thenReturn(program); @@ -135,7 +139,8 @@ void setUp() { @Test void shouldMapWithoutFetchingNullParamsWhenParamsAreNotSpecified() throws BadRequestException, ForbiddenException { - EnrollmentOperationParams operationParams = EnrollmentOperationParams.EMPTY; + EnrollmentOperationParams operationParams = + EnrollmentOperationParams.builder().orgUnitMode(ACCESSIBLE).build(); mapper.map(operationParams); @@ -151,10 +156,17 @@ void shouldMapOrgUnitsWhenOrgUnitUidsAreSpecified() EnrollmentOperationParams operationParams = EnrollmentOperationParams.builder() .orgUnitUids(Set.of(ORG_UNIT_1_UID, ORG_UNIT_2_UID)) + .orgUnitMode(SELECTED) .programUid(program.getUid()) .build(); when(trackerAccessManager.canAccess(user, program, orgUnit1)).thenReturn(true); when(trackerAccessManager.canAccess(user, program, orgUnit2)).thenReturn(true); + when(organisationUnitService.isInUserHierarchy( + orgUnit1.getUid(), user.getTeiSearchOrganisationUnitsWithFallback())) + .thenReturn(true); + when(organisationUnitService.isInUserHierarchy( + orgUnit2.getUid(), user.getTeiSearchOrganisationUnitsWithFallback())) + .thenReturn(true); EnrollmentQueryParams params = mapper.map(operationParams); @@ -193,7 +205,7 @@ void shouldThrowExceptionWhenOrgUnitNotInScope() { @Test void shouldMapProgramWhenProgramUidIsSpecified() throws BadRequestException, ForbiddenException { EnrollmentOperationParams requestParams = - EnrollmentOperationParams.builder().programUid(PROGRAM_UID).build(); + EnrollmentOperationParams.builder().programUid(PROGRAM_UID).orgUnitMode(ACCESSIBLE).build(); EnrollmentQueryParams params = mapper.map(requestParams); @@ -214,7 +226,10 @@ void shouldThrowExceptionWhenProgramNotFound() { void shouldMapTrackedEntityTypeWhenTrackedEntityTypeUidIsSpecified() throws BadRequestException, ForbiddenException { EnrollmentOperationParams operationParams = - EnrollmentOperationParams.builder().trackedEntityTypeUid(TRACKED_ENTITY_TYPE_UID).build(); + EnrollmentOperationParams.builder() + .trackedEntityTypeUid(TRACKED_ENTITY_TYPE_UID) + .orgUnitMode(ACCESSIBLE) + .build(); EnrollmentQueryParams params = mapper.map(operationParams); @@ -235,7 +250,10 @@ void shouldThrowExceptionWhenTrackedEntityTypeNotFound() { void shouldMapTrackedEntityWhenTrackedEntityUidIsSpecified() throws BadRequestException, ForbiddenException { EnrollmentOperationParams operationParams = - EnrollmentOperationParams.builder().trackedEntityUid(TRACKED_ENTITY_UID).build(); + EnrollmentOperationParams.builder() + .trackedEntityUid(TRACKED_ENTITY_UID) + .orgUnitMode(ACCESSIBLE) + .build(); EnrollmentQueryParams params = mapper.map(operationParams); @@ -259,6 +277,7 @@ void shouldMapOrderInGivenOrder() throws BadRequestException, ForbiddenException EnrollmentOperationParams.builder() .orderBy("enrollmentDate", SortDirection.ASC) .orderBy("created", SortDirection.DESC) + .orgUnitMode(ACCESSIBLE) .build(); EnrollmentQueryParams params = mapper.map(operationParams); @@ -273,9 +292,10 @@ void shouldMapOrderInGivenOrder() throws BadRequestException, ForbiddenException @Test void shouldMapNullOrderingParamsWhenNoOrderingParamsAreSpecified() throws BadRequestException, ForbiddenException { - EnrollmentOperationParams requestParams = EnrollmentOperationParams.EMPTY; + EnrollmentOperationParams operationParams = + EnrollmentOperationParams.builder().orgUnitMode(ACCESSIBLE).build(); - EnrollmentQueryParams params = mapper.map(requestParams); + EnrollmentQueryParams params = mapper.map(operationParams); assertIsEmpty(params.getOrder()); } 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..cfca6375f2df 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 @@ -522,6 +522,7 @@ void shouldReturnPaginatedEnrollmentsGivenNonDefaultPageSize() EnrollmentOperationParams operationParams = EnrollmentOperationParams.builder() .orgUnitUids(Set.of(orgUnit.getUid())) + .orgUnitMode(SELECTED) .orderBy("enrollmentDate", SortDirection.ASC) .build(); @@ -553,6 +554,7 @@ void shouldReturnPaginatedEnrollmentsGivenNonDefaultPageSizeAndTotalPages() EnrollmentOperationParams operationParams = EnrollmentOperationParams.builder() .orgUnitUids(Set.of(orgUnit.getUid())) + .orgUnitMode(SELECTED) .orderBy("enrollmentDate", SortDirection.ASC) .build(); @@ -590,7 +592,10 @@ void shouldOrderEnrollmentsByPrimaryKeyDescByDefault() .toList(); EnrollmentOperationParams params = - EnrollmentOperationParams.builder().orgUnitUids(Set.of(orgUnit.getUid())).build(); + EnrollmentOperationParams.builder() + .orgUnitUids(Set.of(orgUnit.getUid())) + .orgUnitMode(SELECTED) + .build(); List enrollments = getEnrollments(params); @@ -603,6 +608,7 @@ void shouldOrderEnrollmentsByEnrolledAtAsc() EnrollmentOperationParams params = EnrollmentOperationParams.builder() .orgUnitUids(Set.of(orgUnit.getUid())) + .orgUnitMode(SELECTED) .orderBy("enrollmentDate", SortDirection.ASC) .build(); @@ -617,6 +623,7 @@ void shouldOrderEnrollmentsByEnrolledAtDesc() EnrollmentOperationParams params = EnrollmentOperationParams.builder() .orgUnitUids(Set.of(orgUnit.getUid())) + .orgUnitMode(SELECTED) .orderBy("enrollmentDate", SortDirection.DESC) .build(); From 06e1abb6b030b22a795b42581234a2bb1ff16ee9 Mon Sep 17 00:00:00 2001 From: muilpp Date: Mon, 6 Nov 2023 10:33:08 +0100 Subject: [PATCH 7/9] fix: Add org unit mode in enrollment service tests [TECH-1589] --- .../export/enrollment/EnrollmentOperationParamsMapperTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentOperationParamsMapperTest.java b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentOperationParamsMapperTest.java index 447fa72bbaa1..57b2ad9b24f2 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentOperationParamsMapperTest.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentOperationParamsMapperTest.java @@ -178,6 +178,7 @@ void shouldThrowExceptionWhenOrgUnitNotFound() { EnrollmentOperationParams operationParams = EnrollmentOperationParams.builder() .orgUnitUids(Set.of("JW6BrFd0HLu", ORG_UNIT_2_UID)) + .orgUnitMode(SELECTED) .programUid(PROGRAM_UID) .build(); From b2ae88972323f974113fddedbc856c3cb53a530a Mon Sep 17 00:00:00 2001 From: muilpp Date: Mon, 6 Nov 2023 11:55:46 +0100 Subject: [PATCH 8/9] fix: Add org unit mode in enrollment service tests [TECH-1589] --- .../controller/ProgramControllerIntegrationTest.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/ProgramControllerIntegrationTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/ProgramControllerIntegrationTest.java index 05eddbea937c..ca7de97a65fb 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/ProgramControllerIntegrationTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/ProgramControllerIntegrationTest.java @@ -37,6 +37,8 @@ import org.hisp.dhis.association.jdbc.JdbcOrgUnitAssociationsStore; import org.hisp.dhis.dataelement.DataElement; import org.hisp.dhis.jsontree.JsonList; +import org.hisp.dhis.organisationunit.OrganisationUnit; +import org.hisp.dhis.organisationunit.OrganisationUnitService; import org.hisp.dhis.trackedentity.TrackedEntityAttribute; import org.hisp.dhis.user.User; import org.hisp.dhis.web.HttpStatus; @@ -60,6 +62,8 @@ class ProgramControllerIntegrationTest extends DhisControllerIntegrationTest { @Autowired private ObjectMapper jsonMapper; + @Autowired private OrganisationUnitService orgUnitService; + public static final String PROGRAM_UID = "PrZMWi7rBga"; public static final String ORG_UNIT_UID = "Orgunit1000"; @@ -86,6 +90,10 @@ public void testSetup() throws JsonProcessingException { @Test void testCopyProgramEnrollments() { + OrganisationUnit orgUnit = orgUnitService.getOrganisationUnit(ORG_UNIT_UID); + User user = createAndAddUser(true, "user", Set.of(orgUnit), Set.of(orgUnit)); + injectSecurityContext(user); + assertStatus( HttpStatus.CREATED, POST( From c61c595c1049aca21d54bb5c55f134af11870031 Mon Sep 17 00:00:00 2001 From: muilpp Date: Mon, 6 Nov 2023 14:16:32 +0100 Subject: [PATCH 9/9] fix: Validate mode ALL on enrollments old API [TECH-1589] --- .../export/OperationsParamsValidator.java | 2 +- .../tracker/EnrollmentCriteriaMapper.java | 10 ++ .../tracker/EnrollmentCriteriaMapperTest.java | 128 +++++++++++++++++- 3 files changed, 134 insertions(+), 6 deletions(-) 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..95b044b91786 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 @@ -64,7 +64,7 @@ public static void validateOrgUnitMode( private static void validateUserCanSearchOrgUnitModeALL(User user) throws BadRequestException { if (user != null && !(user.isSuper() - || user.isAuthorized(F_TRACKED_ENTITY_INSTANCE_SEARCH_IN_ALL_ORGUNITS.name()))) { + || user.isAuthorized(F_TRACKED_ENTITY_INSTANCE_SEARCH_IN_ALL_ORGUNITS))) { throw new BadRequestException( "Current user is not authorized to query across all organisation units"); } diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/deprecated/tracker/EnrollmentCriteriaMapper.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/deprecated/tracker/EnrollmentCriteriaMapper.java index 46f412461dd5..bebbdec6bac1 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/deprecated/tracker/EnrollmentCriteriaMapper.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/deprecated/tracker/EnrollmentCriteriaMapper.java @@ -28,7 +28,9 @@ package org.hisp.dhis.webapi.controller.deprecated.tracker; import static org.hisp.dhis.common.OrganisationUnitSelectionMode.ACCESSIBLE; +import static org.hisp.dhis.common.OrganisationUnitSelectionMode.ALL; import static org.hisp.dhis.common.OrganisationUnitSelectionMode.CAPTURE; +import static org.hisp.dhis.security.Authorities.F_TRACKED_ENTITY_INSTANCE_SEARCH_IN_ALL_ORGUNITS; import static org.hisp.dhis.webapi.controller.event.mapper.OrderParamsHelper.toOrderParams; import java.util.Date; @@ -145,6 +147,14 @@ public EnrollmentQueryParams getFromUrl( } } + if (ouMode == ALL + && (user != null + && !user.isSuper() + && !user.isAuthorized(F_TRACKED_ENTITY_INSTANCE_SEARCH_IN_ALL_ORGUNITS))) { + throw new IllegalQueryException( + "Current user is not authorized to query across all organisation units"); + } + Program pr = programUid != null ? programService.getProgram(programUid) : null; if (programUid != null && pr == null) { diff --git a/dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/controller/deprecated/tracker/EnrollmentCriteriaMapperTest.java b/dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/controller/deprecated/tracker/EnrollmentCriteriaMapperTest.java index df118624a617..d2164f1e1f7b 100644 --- a/dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/controller/deprecated/tracker/EnrollmentCriteriaMapperTest.java +++ b/dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/controller/deprecated/tracker/EnrollmentCriteriaMapperTest.java @@ -28,19 +28,21 @@ package org.hisp.dhis.webapi.controller.deprecated.tracker; import static org.hisp.dhis.common.OrganisationUnitSelectionMode.ACCESSIBLE; +import static org.hisp.dhis.common.OrganisationUnitSelectionMode.ALL; import static org.hisp.dhis.common.OrganisationUnitSelectionMode.CAPTURE; import static org.hisp.dhis.common.OrganisationUnitSelectionMode.CHILDREN; 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.utils.Assertions.assertNotEmpty; import static org.hisp.dhis.utils.Assertions.assertStartsWith; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.when; +import com.google.common.collect.Sets; import java.util.Set; import org.hisp.dhis.common.IllegalQueryException; -import org.hisp.dhis.feedback.ForbiddenException; import org.hisp.dhis.organisationunit.OrganisationUnit; import org.hisp.dhis.organisationunit.OrganisationUnitService; import org.hisp.dhis.program.EnrollmentQueryParams; @@ -53,6 +55,8 @@ import org.hisp.dhis.trackedentity.TrackedEntityTypeService; import org.hisp.dhis.user.CurrentUserService; import org.hisp.dhis.user.User; +import org.hisp.dhis.user.UserRole; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -265,7 +269,7 @@ void shouldFailWhenOrgUnitSuppliedAndOrgUnitModeCapture() { } @Test - void shouldMapOrgUnitModeWhenOrgUnitSuppliedAndOrgUnitModeSelected() throws ForbiddenException { + void shouldMapOrgUnitModeWhenOrgUnitSuppliedAndOrgUnitModeSelected() { when(programService.getProgram(PROGRAM_UID)).thenReturn(program); when(organisationUnitService.getOrganisationUnit(ORG_UNIT1)).thenReturn(organisationUnit); when(organisationUnitService.isInUserHierarchy( @@ -298,8 +302,7 @@ void shouldMapOrgUnitModeWhenOrgUnitSuppliedAndOrgUnitModeSelected() throws Forb } @Test - void shouldMapOrgUnitModeWhenOrgUnitSuppliedAndOrgUnitModeDescendants() - throws ForbiddenException { + void shouldMapOrgUnitModeWhenOrgUnitSuppliedAndOrgUnitModeDescendants() { when(programService.getProgram(PROGRAM_UID)).thenReturn(program); when(organisationUnitService.getOrganisationUnit(ORG_UNIT1)).thenReturn(organisationUnit); when(organisationUnitService.isInUserHierarchy( @@ -332,7 +335,7 @@ void shouldMapOrgUnitModeWhenOrgUnitSuppliedAndOrgUnitModeDescendants() } @Test - void shouldMapOrgUnitModeWhenOrgUnitSuppliedAndOrgUnitModeChildren() throws ForbiddenException { + void shouldMapOrgUnitModeWhenOrgUnitSuppliedAndOrgUnitModeChildren() { when(programService.getProgram(PROGRAM_UID)).thenReturn(program); when(organisationUnitService.getOrganisationUnit(ORG_UNIT1)).thenReturn(organisationUnit); when(organisationUnitService.isInUserHierarchy( @@ -363,4 +366,119 @@ void shouldMapOrgUnitModeWhenOrgUnitSuppliedAndOrgUnitModeChildren() throws Forb assertEquals(CHILDREN, enrollmentQueryParams.getOrganisationUnitMode()); } + + @Test + void shouldMapParamsWhenOrgUnitModeAllAndUserIsSuperuser() { + when(programService.getProgram(PROGRAM_UID)).thenReturn(program); + when(organisationUnitService.getOrganisationUnit(ORG_UNIT1)).thenReturn(organisationUnit); + when(trackedEntityTypeService.getTrackedEntityType(ENTITY_TYPE)).thenReturn(trackedEntityType); + when(trackedEntityService.getTrackedEntity(TRACKED_ENTITY)).thenReturn(trackedEntity); + + User superuser = new User(); + UserRole userRole = new UserRole(); + userRole.setAuthorities(Sets.newHashSet("ALL")); + superuser.setUserRoles(Set.of(userRole)); + + when(currentUserService.getCurrentUser()).thenReturn(superuser); + + EnrollmentQueryParams enrollmentQueryParams = + mapper.getFromUrl( + ORG_UNITS, + ALL, + null, + "lastUpdated", + PROGRAM_UID, + ProgramStatus.ACTIVE, + null, + null, + ENTITY_TYPE, + TRACKED_ENTITY, + false, + 1, + 1, + false, + false, + false, + null); + + assertEquals(ALL, enrollmentQueryParams.getOrganisationUnitMode()); + } + + @Test + void shouldMapParamsWhenOrgUnitModeAllAndUserIsAuthorized() { + when(programService.getProgram(PROGRAM_UID)).thenReturn(program); + when(organisationUnitService.getOrganisationUnit(ORG_UNIT1)).thenReturn(organisationUnit); + when(trackedEntityTypeService.getTrackedEntityType(ENTITY_TYPE)).thenReturn(trackedEntityType); + when(trackedEntityService.getTrackedEntity(TRACKED_ENTITY)).thenReturn(trackedEntity); + + User superuser = new User(); + UserRole userRole = new UserRole(); + userRole.setAuthorities(Sets.newHashSet("ALL")); + superuser.setUserRoles(Set.of(userRole)); + + when(currentUserService.getCurrentUser()).thenReturn(superuser); + + EnrollmentQueryParams enrollmentQueryParams = + mapper.getFromUrl( + ORG_UNITS, + ALL, + null, + "lastUpdated", + PROGRAM_UID, + ProgramStatus.ACTIVE, + null, + null, + ENTITY_TYPE, + TRACKED_ENTITY, + false, + 1, + 1, + false, + false, + false, + null); + + assertEquals(ALL, enrollmentQueryParams.getOrganisationUnitMode()); + } + + @Test + void shouldFailWhenOrgUnitModeAllAndUserNotAuthorized() { + when(organisationUnitService.getOrganisationUnit(ORG_UNIT1)).thenReturn(organisationUnit); + when(organisationUnitService.isInUserHierarchy( + organisationUnit.getUid(), user.getTeiSearchOrganisationUnitsWithFallback())) + .thenReturn(true); + + User superuser = new User(); + UserRole userRole = new UserRole(); + userRole.setAuthorities( + Sets.newHashSet(F_TRACKED_ENTITY_INSTANCE_SEARCH_IN_ALL_ORGUNITS.name())); + superuser.setUserRoles(Set.of(userRole)); + + IllegalQueryException exception = + Assertions.assertThrows( + IllegalQueryException.class, + () -> + mapper.getFromUrl( + ORG_UNITS, + ALL, + null, + "lastUpdated", + PROGRAM_UID, + ProgramStatus.ACTIVE, + null, + null, + ENTITY_TYPE, + TRACKED_ENTITY, + false, + 1, + 1, + false, + false, + false, + null)); + + Assertions.assertEquals( + "Current user is not authorized to query across all organisation units", + exception.getMessage()); + } }