From 349803d033bd0e5faa1bf3f5eb5506bdb99eee98 Mon Sep 17 00:00:00 2001 From: Mohamed Ameen Date: Wed, 4 Dec 2024 18:42:01 +0100 Subject: [PATCH] fix: [DHIS2-18551] (2.40) Fix access check when registering TEI (#19384) --- .../trackedentity/TrackerAccessManager.java | 2 + .../DefaultTrackerAccessManager.java | 29 +++++++++++ .../AbstractTrackedEntityInstanceService.java | 3 +- .../TrackedEntityInstanceServiceTest.java | 50 ++++++++++++++++++- 4 files changed, 82 insertions(+), 2 deletions(-) diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/trackedentity/TrackerAccessManager.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/trackedentity/TrackerAccessManager.java index 2923d36828ee..c0ea607bca25 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/trackedentity/TrackerAccessManager.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/trackedentity/TrackerAccessManager.java @@ -46,6 +46,8 @@ public interface TrackerAccessManager { List canWrite(User user, TrackedEntityInstance trackedEntityInstance); + List canCreate(User user, TrackedEntityInstance trackedEntity); + List canRead( User user, TrackedEntityInstance trackedEntityInstance, diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/trackedentity/DefaultTrackerAccessManager.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/trackedentity/DefaultTrackerAccessManager.java index ee71d1372b90..55f299bd82b3 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/trackedentity/DefaultTrackerAccessManager.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/trackedentity/DefaultTrackerAccessManager.java @@ -159,6 +159,35 @@ public List canWrite(User user, TrackedEntityInstance trackedEntity) { return canWrite(user, trackedEntity, programService.getAllPrograms()); } + /** + * Check if the user can create the TE. For a regular user, they must have data write permissions + * to tracked entity type metadata as well as Capture Access to the Org Unit. + * + * @return No errors if a user has write access to the TrackedEntityType and to the OrgUnit + */ + @Override + @Transactional(readOnly = true) + public List canCreate(User user, TrackedEntityInstance trackedEntity) { + if (user == null || user.isSuper() || trackedEntity == null) { + return List.of(); + } + List errors = new ArrayList<>(); + if (!aclService.canDataWrite(user, trackedEntity.getTrackedEntityType())) { + errors.add( + "User has no data write access to tracked entity type: " + + trackedEntity.getTrackedEntityType().getUid()); + } + + if (trackedEntity.getOrganisationUnit() != null + && !organisationUnitService.isInUserHierarchy(user, trackedEntity.getOrganisationUnit())) { + errors.add( + "User has no write access to organisation unit: " + + trackedEntity.getOrganisationUnit().getUid()); + } + + return errors; + } + private List canWrite( User user, TrackedEntityInstance trackedEntity, List programs) { diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/events/trackedentity/AbstractTrackedEntityInstanceService.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/events/trackedentity/AbstractTrackedEntityInstanceService.java index 0642d03522bf..3be429d9003b 100644 --- a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/events/trackedentity/AbstractTrackedEntityInstanceService.java +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/events/trackedentity/AbstractTrackedEntityInstanceService.java @@ -671,7 +671,8 @@ private ImportSummary addTrackedEntityInstance( return importSummary; } - List errors = trackerAccessManager.canWrite(importOptions.getUser(), daoEntityInstance); + List errors = + trackerAccessManager.canCreate(importOptions.getUser(), daoEntityInstance); if (!errors.isEmpty()) { return new ImportSummary(ImportStatus.ERROR, errors.toString()).incrementIgnored(); diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/dxf2/events/TrackedEntityInstanceServiceTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/dxf2/events/TrackedEntityInstanceServiceTest.java index 8eb91274c8cc..26f89110d8b8 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/dxf2/events/TrackedEntityInstanceServiceTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/dxf2/events/TrackedEntityInstanceServiceTest.java @@ -90,6 +90,7 @@ import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValueService; import org.hisp.dhis.user.User; import org.hisp.dhis.user.UserService; +import org.hisp.dhis.user.sharing.Sharing; import org.joda.time.DateTime; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -593,16 +594,63 @@ void testUpdateTeiByDeletingExistingEventAndAddNewEventForSameProgramStage() { } @Test - void testSavePerson() { + void shouldPassWhenRegisteringPerson() { + trackedEntityType.setSharing(Sharing.builder().publicAccess("rwrw----").build()); + regularUser.setOrganisationUnits(Set.of(organisationUnitA)); + injectSecurityContext(regularUser); TrackedEntityInstance trackedEntityInstance = new TrackedEntityInstance(); trackedEntityInstance.setTrackedEntityInstance(CodeGenerator.generateUid()); trackedEntityInstance.setOrgUnit(organisationUnitA.getUid()); trackedEntityInstance.setTrackedEntityType(trackedEntityType.getUid()); + ImportSummary importSummary = trackedEntityInstanceService.addTrackedEntityInstance(trackedEntityInstance, null); + assertEquals(ImportStatus.SUCCESS, importSummary.getStatus()); } + @Test + void shouldFailWhenRegisteringPersonOutsideCaptureScope() { + trackedEntityType.setSharing(Sharing.builder().publicAccess("rwrw----").build()); + regularUser.setOrganisationUnits(Set.of(organisationUnitB)); + injectSecurityContext(regularUser); + TrackedEntityInstance trackedEntityInstance = new TrackedEntityInstance(); + trackedEntityInstance.setTrackedEntityInstance(CodeGenerator.generateUid()); + trackedEntityInstance.setOrgUnit(organisationUnitA.getUid()); + trackedEntityInstance.setTrackedEntityType(trackedEntityType.getUid()); + + ImportSummary importSummary = + trackedEntityInstanceService.addTrackedEntityInstance(trackedEntityInstance, null); + + assertEquals(ImportStatus.ERROR, importSummary.getStatus()); + assertEquals(1, importSummary.getImportCount().getIgnored()); + assertEquals( + String.format( + "[User has no write access to organisation unit: %s]", organisationUnitA.getUid()), + importSummary.getDescription()); + } + + @Test + void shouldFailWhenRegisteringPersonWithoutTETypeAccess() { + regularUser.setOrganisationUnits(Set.of(organisationUnitB)); + injectSecurityContext(regularUser); + TrackedEntityInstance trackedEntityInstance = new TrackedEntityInstance(); + trackedEntityInstance.setTrackedEntityInstance(CodeGenerator.generateUid()); + trackedEntityInstance.setOrgUnit(organisationUnitB.getUid()); + trackedEntityInstance.setTrackedEntityType(trackedEntityType.getUid()); + + ImportSummary importSummary = + trackedEntityInstanceService.addTrackedEntityInstance(trackedEntityInstance, null); + + assertEquals(ImportStatus.ERROR, importSummary.getStatus()); + assertEquals(1, importSummary.getImportCount().getIgnored()); + assertEquals( + String.format( + "[User has no data write access to tracked entity type: %s]", + trackedEntityType.getUid()), + importSummary.getDescription()); + } + @Test void testDeletePerson() { trackedEntityInstanceService.deleteTrackedEntityInstance(maleA.getUid());