From 7ee7e49c6c117adf35eebfd27698a4b7f9a9a214 Mon Sep 17 00:00:00 2001 From: Marc Date: Tue, 10 Dec 2024 15:00:16 +0100 Subject: [PATCH 1/2] fix: Validate data sharing on program stage [DHIS2-17594][2.41] --- .../DefaultTrackedEntityService.java | 5 +- .../TrackedEntityServiceTest.java | 64 ++++++++++++++++--- 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/DefaultTrackedEntityService.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/DefaultTrackedEntityService.java index f54bef19b27b..a4efd437ad8c 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/DefaultTrackedEntityService.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/DefaultTrackedEntityService.java @@ -375,7 +375,10 @@ private Set getEnrollments( e -> { Set filteredEvents = e.getEvents().stream() - .filter(event -> includeDeleted || !event.isDeleted()) + .filter( + event -> + trackerAccessManager.canRead(user, event, false).isEmpty() + && (includeDeleted || !event.isDeleted())) .collect(Collectors.toSet()); e.setEvents(filteredEvents); return e; 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 58f5b0985f85..9ca23fde411c 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 @@ -103,6 +103,7 @@ import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValue; import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValueService; import org.hisp.dhis.user.User; +import org.hisp.dhis.user.UserDetails; import org.hisp.dhis.user.UserService; import org.hisp.dhis.user.sharing.Sharing; import org.junit.jupiter.api.Disabled; @@ -164,14 +165,12 @@ class TrackedEntityServiceTest extends IntegrationTestBase { private Enrollment enrollmentB; - private Enrollment enrollmentC; + private ProgramStage programStageA1; private Event eventA; private Event eventB; - private Event eventC; - private TrackedEntity trackedEntityA; private TrackedEntity trackedEntityB; @@ -180,8 +179,6 @@ class TrackedEntityServiceTest extends IntegrationTestBase { private TrackedEntity trackedEntityGrandchildA; - private Note note; - private CategoryOptionCombo defaultCategoryOptionCombo; private Relationship relationshipA; @@ -194,6 +191,8 @@ class TrackedEntityServiceTest extends IntegrationTestBase { private Relationship relationshipE; + private Note note; + private static List uids( Collection identifiableObjects) { return identifiableObjects.stream() @@ -282,7 +281,7 @@ protected void setUpTest() throws Exception { programA.setCategoryCombo(defaultCategoryCombo); programA.setMinAttributesRequiredToSearch(0); manager.save(programA, false); - ProgramStage programStageA1 = createProgramStage(programA); + programStageA1 = createProgramStage(programA); programStageA1.setPublicAccess(AccessStringHelper.FULL); manager.save(programStageA1, false); ProgramStage programStageA2 = createProgramStage(programA); @@ -399,10 +398,10 @@ protected void setUpTest() throws Exception { trackedEntityB.setTrackedEntityType(trackedEntityTypeA); manager.save(trackedEntityB, false); - enrollmentC = + Enrollment enrollmentC = enrollmentService.enrollTrackedEntity( trackedEntityB, programB, new Date(), new Date(), orgUnitB); - eventC = new Event(); + Event eventC = new Event(); eventC.setEnrollment(enrollmentC); eventC.setProgramStage(programStageB1); eventC.setOrganisationUnit(orgUnitB); @@ -2071,6 +2070,55 @@ void shouldReturnTrackedEntityTypeAttributesWhenSingleTERequestedAndNoProgramSpe trackedEntity.getTrackedEntityAttributeValues()); } + @Test + void shouldFindTrackedEntityWithEventsWhenEventRequestedAndAccessible() + throws ForbiddenException, NotFoundException, BadRequestException { + injectSecurityContextUser(getAdminUser()); + User testUser = createAndAddUser(false, "testUser", emptySet(), emptySet(), "F_EXPORT_DATA"); + testUser.setOrganisationUnits(Set.of(orgUnitA)); + manager.update(testUser); + injectSecurityContext(UserDetails.fromUser(testUser)); + + TrackedEntity trackedEntity = + trackedEntityService.getTrackedEntity( + trackedEntityA.getUid(), programA.getUid(), TrackedEntityParams.TRUE, false); + + assertEquals(trackedEntityA.getUid(), trackedEntity.getUid()); + assertContainsOnly(Set.of(enrollmentA.getUid()), uids(trackedEntity.getEnrollments())); + List enrollments = new ArrayList<>(trackedEntity.getEnrollments()); + Optional enrollmentA = + enrollments.stream() + .filter(enrollment -> enrollment.getUid().equals(this.enrollmentA.getUid())) + .findFirst(); + Set events = enrollmentA.get().getEvents(); + assertContainsOnly(Set.of(eventA.getUid()), uids(events)); + } + + @Test + void shouldFindTrackedEntityWithoutEventsWhenEventRequestedButNotAccessible() + throws ForbiddenException, NotFoundException, BadRequestException { + injectSecurityContextUser(getAdminUser()); + programStageA1.setSharing(Sharing.builder().publicAccess("--------").build()); + manager.update(programStageA1); + User testUser = createAndAddUser(false, "testUser", emptySet(), emptySet(), "F_EXPORT_DATA"); + testUser.setOrganisationUnits(Set.of(orgUnitA)); + manager.update(testUser); + injectSecurityContext(UserDetails.fromUser(testUser)); + + TrackedEntity trackedEntity = + trackedEntityService.getTrackedEntity( + trackedEntityA.getUid(), programA.getUid(), TrackedEntityParams.TRUE, false); + + assertEquals(trackedEntityA.getUid(), trackedEntity.getUid()); + assertContainsOnly(Set.of(enrollmentA.getUid()), uids(trackedEntity.getEnrollments())); + List enrollments = new ArrayList<>(trackedEntity.getEnrollments()); + Optional enrollmentA = + enrollments.stream() + .filter(enrollment -> enrollment.getUid().equals(this.enrollmentA.getUid())) + .findFirst(); + assertIsEmpty(enrollmentA.get().getEvents()); + } + private Set attributeNames(final Collection attributes) { // depends on createTrackedEntityAttribute() prefixing with "Attribute" return attributes.stream() From 749af6c8c36aa94e498f523eb10c3806c750ace2 Mon Sep 17 00:00:00 2001 From: Marc Date: Tue, 10 Dec 2024 15:51:15 +0100 Subject: [PATCH 2/2] fix: Validate data sharing on program stage [DHIS2-17594][2.41] --- .../DefaultTrackedEntityService.java | 4 ++-- .../trackedentity/TrackedEntityServiceTest.java | 16 ++++++---------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/DefaultTrackedEntityService.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/DefaultTrackedEntityService.java index a4efd437ad8c..ee7006010327 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/DefaultTrackedEntityService.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/trackedentity/DefaultTrackedEntityService.java @@ -377,8 +377,8 @@ private Set getEnrollments( e.getEvents().stream() .filter( event -> - trackerAccessManager.canRead(user, event, false).isEmpty() - && (includeDeleted || !event.isDeleted())) + (includeDeleted || !event.isDeleted()) + && trackerAccessManager.canRead(user, event, false).isEmpty()) .collect(Collectors.toSet()); e.setEvents(filteredEvents); return e; 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 9ca23fde411c..b6e88fb14672 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 @@ -2086,11 +2086,9 @@ void shouldFindTrackedEntityWithEventsWhenEventRequestedAndAccessible() assertEquals(trackedEntityA.getUid(), trackedEntity.getUid()); assertContainsOnly(Set.of(enrollmentA.getUid()), uids(trackedEntity.getEnrollments())); List enrollments = new ArrayList<>(trackedEntity.getEnrollments()); - Optional enrollmentA = - enrollments.stream() - .filter(enrollment -> enrollment.getUid().equals(this.enrollmentA.getUid())) - .findFirst(); - Set events = enrollmentA.get().getEvents(); + Optional enrollment = + enrollments.stream().filter(e -> e.getUid().equals(this.enrollmentA.getUid())).findFirst(); + Set events = enrollment.get().getEvents(); assertContainsOnly(Set.of(eventA.getUid()), uids(events)); } @@ -2112,11 +2110,9 @@ void shouldFindTrackedEntityWithoutEventsWhenEventRequestedButNotAccessible() assertEquals(trackedEntityA.getUid(), trackedEntity.getUid()); assertContainsOnly(Set.of(enrollmentA.getUid()), uids(trackedEntity.getEnrollments())); List enrollments = new ArrayList<>(trackedEntity.getEnrollments()); - Optional enrollmentA = - enrollments.stream() - .filter(enrollment -> enrollment.getUid().equals(this.enrollmentA.getUid())) - .findFirst(); - assertIsEmpty(enrollmentA.get().getEvents()); + Optional enrollment = + enrollments.stream().filter(e -> e.getUid().equals(this.enrollmentA.getUid())).findFirst(); + assertIsEmpty(enrollment.get().getEvents()); } private Set attributeNames(final Collection attributes) {