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 6e912e995a7f..74a9c9ba6281 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 @@ -76,29 +76,68 @@ class DefaultEnrollmentService implements EnrollmentService { private final EnrollmentOperationParamsMapper paramsMapper; + @Nonnull @Override public Enrollment getEnrollment(@Nonnull UID uid) throws ForbiddenException, NotFoundException { return getEnrollment(uid, EnrollmentParams.FALSE, false); } + @Nonnull @Override public Enrollment getEnrollment( @Nonnull UID uid, @Nonnull EnrollmentParams params, boolean includeDeleted) throws NotFoundException, ForbiddenException { - UserDetails currentUser = getCurrentUserDetails(); - Enrollment enrollment = enrollmentStore.getByUid(uid.getValue()); + Page enrollments; + try { + EnrollmentOperationParams operationParams = + EnrollmentOperationParams.builder() + .enrollments(Set.of(uid)) + .enrollmentParams(params) + .includeDeleted(includeDeleted) + .build(); + enrollments = getEnrollments(operationParams, new PageParams(1, 1, false)); + } catch (BadRequestException e) { + throw new IllegalArgumentException( + "this must be a bug in how the EnrollmentOperationParams are built"); + } - if (enrollment == null) { + if (enrollments.getItems().isEmpty()) { throw new NotFoundException(Enrollment.class, uid); } - List errors = trackerAccessManager.canRead(currentUser, enrollment, false); + return enrollments.getItems().get(0); + } - if (!errors.isEmpty()) { - throw new ForbiddenException(errors.toString()); + @Override + public RelationshipItem getEnrollmentInRelationshipItem( + @Nonnull UID uid, boolean includeDeleted) { + Enrollment enrollment; + try { + enrollment = getEnrollment(uid); + } catch (NotFoundException | ForbiddenException e) { + // enrollments are not shown in relationships if the user has no access to them + return null; } - return getEnrollment(enrollment, params, includeDeleted, currentUser); + RelationshipItem relationshipItem = new RelationshipItem(); + relationshipItem.setEnrollment(enrollment); + return relationshipItem; + } + + private Set getEvents( + Enrollment enrollment, EventParams eventParams, boolean includeDeleted) { + EventOperationParams eventOperationParams = + EventOperationParams.builder() + .enrollments(Set.of(UID.of(enrollment))) + .eventParams(eventParams) + .includeDeleted(includeDeleted) + .build(); + try { + return Set.copyOf(eventService.getEvents(eventOperationParams)); + } catch (BadRequestException | ForbiddenException e) { + throw new IllegalArgumentException( + "this must be a bug in how the EventOperationParams are built"); + } } private Enrollment getEnrollment( @@ -151,48 +190,6 @@ private Enrollment getEnrollment( return result; } - @Override - public RelationshipItem getEnrollmentInRelationshipItem(@Nonnull UID uid, boolean includeDeleted) - throws NotFoundException { - - RelationshipItem relationshipItem = new RelationshipItem(); - Enrollment enrollment = enrollmentStore.getByUid(uid.getValue()); - - if (enrollment == null) { - throw new NotFoundException(Enrollment.class, uid); - } - - UserDetails currentUser = getCurrentUserDetails(); - List errors = trackerAccessManager.canRead(currentUser, enrollment, false); - if (!errors.isEmpty()) { - return null; - } - - relationshipItem.setEnrollment( - getEnrollment( - enrollment, - EnrollmentParams.FALSE.withIncludeAttributes(true), - includeDeleted, - currentUser)); - return relationshipItem; - } - - private Set getEvents( - Enrollment enrollment, EventParams eventParams, boolean includeDeleted) { - EventOperationParams eventOperationParams = - EventOperationParams.builder() - .enrollments(Set.of(UID.of(enrollment))) - .eventParams(eventParams) - .includeDeleted(includeDeleted) - .build(); - try { - return Set.copyOf(eventService.getEvents(eventOperationParams)); - } catch (BadRequestException | ForbiddenException e) { - throw new IllegalArgumentException( - "this must be a bug in how the EventOperationParams are built"); - } - } - private Set getRelationshipItems( UserDetails user, Enrollment enrollment, boolean includeDeleted) { Set relationshipItems = new HashSet<>(); @@ -225,24 +222,28 @@ private Set getTrackedEntityAttributeValues( return attributeValues; } + @Nonnull @Override public List getEnrollments(@Nonnull Set uids) throws ForbiddenException { - List enrollments = enrollmentStore.getByUid(UID.toValueList(uids)); - UserDetails user = getCurrentUserDetails(); - List errors = - enrollments.stream() - .flatMap(e -> trackerAccessManager.canRead(user, e, false).stream()) - .toList(); - - if (!errors.isEmpty()) { - throw new ForbiddenException(errors.toString()); + EnrollmentQueryParams queryParams; + try { + queryParams = + paramsMapper.map( + EnrollmentOperationParams.builder().enrollments(uids).build(), + getCurrentUserDetails()); + } catch (BadRequestException e) { + throw new IllegalArgumentException( + "this must be a bug in how the EventOperationParams are built"); } - return enrollments.stream() - .map(e -> getEnrollment(e, EnrollmentParams.FALSE, false, user)) - .toList(); + return getEnrollments( + new ArrayList<>(enrollmentStore.getEnrollments(queryParams)), + EnrollmentParams.FALSE, + false, + queryParams.getOrganisationUnitMode()); } + @Nonnull @Override public List getEnrollments(@Nonnull EnrollmentOperationParams params) throws ForbiddenException, BadRequestException { @@ -255,6 +256,7 @@ public List getEnrollments(@Nonnull EnrollmentOperationParams params queryParams.getOrganisationUnitMode()); } + @Nonnull @Override public Page getEnrollments( @Nonnull EnrollmentOperationParams params, PageParams pageParams) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentOperationParams.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentOperationParams.java index 92d80e5dd037..b56f99ae085c 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentOperationParams.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentOperationParams.java @@ -71,7 +71,9 @@ public class EnrollmentOperationParams { @Builder.Default private final Set orgUnits = new HashSet<>(); /** Selection mode for the specified organisation units. */ - private final OrganisationUnitSelectionMode orgUnitMode; + @Builder.Default + private final OrganisationUnitSelectionMode orgUnitMode = + OrganisationUnitSelectionMode.ACCESSIBLE; /** Enrollments must be enrolled into this program. */ private final UID program; diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentService.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentService.java index 06051c96f20c..75bf70a6af4c 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentService.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentService.java @@ -40,8 +40,10 @@ import org.hisp.dhis.tracker.export.PageParams; public interface EnrollmentService { + @Nonnull Enrollment getEnrollment(UID uid) throws ForbiddenException, NotFoundException; + @Nonnull Enrollment getEnrollment(UID uid, EnrollmentParams params, boolean includeDeleted) throws NotFoundException, ForbiddenException; @@ -62,6 +64,7 @@ Page getEnrollments(EnrollmentOperationParams params, PageParams pag * Get event matching given {@code UID} under the privileges the user in the context. This method * does not get the events relationships. */ + @Nonnull List getEnrollments(@Nonnull Set uids) throws ForbiddenException; /** diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventOperationParams.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventOperationParams.java index 131700f08e61..842e15ac1393 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventOperationParams.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/export/event/EventOperationParams.java @@ -125,10 +125,6 @@ public class EventOperationParams { */ private List order; - private boolean includeAttributes; - - private boolean includeAllDataElements; - @Builder.Default private Set events = new HashSet<>(); /** Data element filters per data element UID. */ diff --git a/dhis-2/dhis-support/dhis-support-test/src/main/java/org/hisp/dhis/test/TestBase.java b/dhis-2/dhis-support/dhis-support-test/src/main/java/org/hisp/dhis/test/TestBase.java index f5e549b9218a..670b2f3eac2b 100644 --- a/dhis-2/dhis-support/dhis-support-test/src/main/java/org/hisp/dhis/test/TestBase.java +++ b/dhis-2/dhis-support/dhis-support-test/src/main/java/org/hisp/dhis/test/TestBase.java @@ -2683,7 +2683,7 @@ protected void injectSecurityContextUser(User user) { } user = userService.getUser(user.getUid()); - UserDetails userDetails = userService.createUserDetails(user); + UserDetails userDetails = UserDetails.fromUser(user); injectSecurityContext(userDetails); } diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/deduplication/PotentialDuplicateRemoveTrackedEntityTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/deduplication/PotentialDuplicateRemoveTrackedEntityTest.java index 8e3dbb5de456..34aadcfc8251 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/deduplication/PotentialDuplicateRemoveTrackedEntityTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/deduplication/PotentialDuplicateRemoveTrackedEntityTest.java @@ -27,11 +27,13 @@ */ package org.hisp.dhis.tracker.deduplication; +import static org.hisp.dhis.security.Authorities.ALL; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import java.util.List; +import java.util.Set; import org.hisp.dhis.common.IdentifiableObjectManager; import org.hisp.dhis.common.UID; import org.hisp.dhis.dbms.DbmsManager; @@ -52,6 +54,7 @@ import org.hisp.dhis.tracker.export.enrollment.EnrollmentService; import org.hisp.dhis.tracker.imports.bundle.persister.TrackerObjectDeletionService; import org.hisp.dhis.tracker.trackedentityattributevalue.TrackedEntityAttributeValueService; +import org.hisp.dhis.user.User; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.transaction.annotation.Transactional; @@ -145,6 +148,8 @@ void shouldDeleteRelationShips() throws NotFoundException { void shouldDeleteEnrollments() throws ForbiddenException, NotFoundException { OrganisationUnit ou = createOrganisationUnit("OU_A"); organisationUnitService.addOrganisationUnit(ou); + User user = createAndAddUser(false, "user", Set.of(ou), Set.of(ou), ALL.toString()); + injectSecurityContextUser(user); TrackedEntity original = createTrackedEntity(ou); TrackedEntity duplicate = createTrackedEntity(ou); TrackedEntity control1 = createTrackedEntity(ou); 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 55f9eafe3141..a35b460c4623 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,6 @@ */ package org.hisp.dhis.tracker.export.enrollment; -import static java.util.Collections.emptySet; import static org.hisp.dhis.common.OrganisationUnitSelectionMode.ACCESSIBLE; import static org.hisp.dhis.common.OrganisationUnitSelectionMode.ALL; import static org.hisp.dhis.common.OrganisationUnitSelectionMode.CAPTURE; @@ -95,7 +94,7 @@ class EnrollmentServiceTest extends PostgresIntegrationTestBase { private User admin; - private User userWithoutOrgUnit; + private User userWithOrgUnitZ; private User authorizedUser; @@ -141,17 +140,20 @@ void setUp() { manager.save(orgUnitChildA, false); OrganisationUnit orgUnitGrandchildA = createOrganisationUnit('E', orgUnitChildA); manager.save(orgUnitGrandchildA, false); + OrganisationUnit orgUnitZ = createOrganisationUnit('Z'); + manager.save(orgUnitZ, false); User user = createAndAddUser(false, "user", Set.of(orgUnitA), Set.of(orgUnitA), "F_EXPORT_DATA"); user.setTeiSearchOrganisationUnits(Set.of(orgUnitA, orgUnitB, orgUnitC)); - userWithoutOrgUnit = createUserWithAuth("userWithoutOrgUnit"); + userWithOrgUnitZ = createUserWithAuth("userWithoutOrgUnit"); + userWithOrgUnitZ.setTeiSearchOrganisationUnits(Set.of(orgUnitZ)); authorizedUser = createAndAddUser( false, "test user", - emptySet(), - emptySet(), + Set.of(orgUnitA), + Set.of(orgUnitA), "F_TRACKED_ENTITY_INSTANCE_SEARCH_IN_ALL_ORGUNITS"); trackedEntityTypeA = createTrackedEntityType('A'); @@ -373,13 +375,13 @@ void shouldFailGettingEnrollmentWhenUserHasNoAccessToProgramsTrackedEntityType() trackedEntityTypeA.getSharing().setPublicAccess(AccessStringHelper.DEFAULT); manager.updateNoAcl(trackedEntityTypeA); - ForbiddenException exception = + NotFoundException exception = assertThrows( - ForbiddenException.class, + NotFoundException.class, () -> enrollmentService.getEnrollment( UID.of(enrollmentA), EnrollmentParams.FALSE, false)); - assertContains("access to tracked entity type", exception.getMessage()); + assertContains("could not be found", exception.getMessage()); } @Test @@ -405,15 +407,15 @@ void shouldFailGettingEnrollmentWhenUserHasReadAccessToProgramButNoAccessToOrgUn programA.getSharing().setPublicAccess(AccessStringHelper.DATA_READ); manager.updateNoAcl(programA); - injectSecurityContextUser(userWithoutOrgUnit); + injectSecurityContextUser(userWithOrgUnitZ); - ForbiddenException exception = + NotFoundException exception = assertThrows( - ForbiddenException.class, + NotFoundException.class, () -> enrollmentService.getEnrollment( UID.of(enrollmentA), EnrollmentParams.FALSE, false)); - assertContains("OWNERSHIP_ACCESS_DENIED", exception.getMessage()); + assertContains("could not be found", exception.getMessage()); } @Test @@ -421,15 +423,15 @@ void shouldFailGettingEnrollmentWhenUserHasReadWriteAccessToProgramButNoAccessTo programA.getSharing().setPublicAccess(AccessStringHelper.DATA_READ_WRITE); manager.updateNoAcl(programA); - injectSecurityContextUser(userWithoutOrgUnit); + injectSecurityContextUser(userWithOrgUnitZ); - ForbiddenException exception = + NotFoundException exception = assertThrows( - ForbiddenException.class, + NotFoundException.class, () -> enrollmentService.getEnrollment( UID.of(enrollmentA), EnrollmentParams.FALSE, false)); - assertContains("OWNERSHIP_ACCESS_DENIED", exception.getMessage()); + assertContains("could not be found", exception.getMessage()); } @Test @@ -437,13 +439,13 @@ void shouldFailGettingEnrollmentWhenUserHasNoAccessToProgramButAccessToOrgUnit() programA.getSharing().setPublicAccess(AccessStringHelper.DEFAULT); manager.updateNoAcl(programA); - ForbiddenException exception = + NotFoundException exception = assertThrows( - ForbiddenException.class, + NotFoundException.class, () -> enrollmentService.getEnrollment( UID.of(enrollmentA), EnrollmentParams.FALSE, false)); - assertContains("access to program", exception.getMessage()); + assertContains("could not be found", exception.getMessage()); } @Test @@ -680,7 +682,7 @@ void shouldFailWhenOrgUnitModeAllAndUserNotAuthorized() { @Test void shouldFailWhenUserCanSearchEverywhereModeDescendantsAndOrgUnitNotInSearchScope() { - injectSecurityContextUser(authorizedUser); + injectSecurityContextUser(userWithOrgUnitZ); EnrollmentOperationParams operationParams = EnrollmentOperationParams.builder().orgUnitMode(DESCENDANTS).orgUnits(orgUnitA).build(); @@ -760,19 +762,16 @@ void shouldReturnChildrenOfRequestedOrgUnitWhenOrgUnitModeChildren() } @Test - void shouldFailWhenRequestingListOfEnrollmentsAndAtLeastOneNotAccessible() { + void shouldReturnEmptyListFailWhenRequestingEnrollmentsAndTheyAreNotAccessible() + throws ForbiddenException { injectSecurityContextUser(admin); programA.getSharing().setPublicAccess("rw------"); manager.update(programA); injectSecurityContextUser(authorizedUser); - ForbiddenException exception = - assertThrows( - ForbiddenException.class, - () -> enrollmentService.getEnrollments(UID.of(enrollmentA, enrollmentB))); - assertContains( - String.format("User has no data read access to program: %s", programA.getUid()), - exception.getMessage()); + List enrollments = + enrollmentService.getEnrollments(UID.of(enrollmentA, enrollmentB)); + assertIsEmpty(enrollments); } @Test diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/note/NoteServiceTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/note/NoteServiceTest.java index 9772a78ae9dc..af37587931b4 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/note/NoteServiceTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/note/NoteServiceTest.java @@ -131,7 +131,7 @@ void shouldFailToCreateEnrollmentNoteIfUserHasNoAccessToEnrollment() { Note note = note(); assertThrows( - ForbiddenException.class, + NotFoundException.class, () -> noteService.addNoteForEnrollment(note, UID.of("nxP7UnKhomJ"))); } diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/relationship/RelationshipsExportControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/relationship/RelationshipsExportControllerTest.java index 9261a496081b..fbcdd131e6d1 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/relationship/RelationshipsExportControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/export/relationship/RelationshipsExportControllerTest.java @@ -493,6 +493,7 @@ void getRelationshipsByEnrollmentWithNotes() { @Test void getRelationshipsByEnrollmentNotFound() { + switchContextToUser(user); assertStartsWith( "Enrollment with id Hq3Kc6HK4OZ", GET("/tracker/relationships?enrollment=Hq3Kc6HK4OZ") diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/export/event/EventRequestParamsMapper.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/export/event/EventRequestParamsMapper.java index 90c5e4809dd7..c50bfa86201d 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/export/event/EventRequestParamsMapper.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/export/event/EventRequestParamsMapper.java @@ -163,8 +163,6 @@ public EventOperationParams map( .eventStatus(eventRequestParams.getStatus()) .attributeCategoryCombo(attributeCategoryCombo) .attributeCategoryOptions(attributeCategoryOptions) - .includeAttributes(false) - .includeAllDataElements(false) .dataElementFilters(dataElementFilters) .attributeFilters(attributeFilters) .events(eventUids)