From 9b0ff497b012f038b09f44e7f2c18d22b2fbe77a Mon Sep 17 00:00:00 2001 From: Ameen Mohamed Date: Wed, 4 Dec 2024 20:55:19 +0100 Subject: [PATCH] fix: [DHIS2-18551] (2.41) Fix access check when registering TEI --- .../trackedentity/TrackerAccessManager.java | 2 + .../DefaultTrackerAccessManager.java | 29 +++++++++++ .../AbstractTrackedEntityInstanceService.java | 3 +- .../tracker/TrackedEntityServiceTest.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 bd035c8ddd0e..742072d7949e 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, TrackedEntity trackedEntity); + List canCreate(User user, TrackedEntity trackedEntity); + /** * Check if a user has data access to the supplied program and tracked entity type. It also * validates user ownership to the TE/program pair. 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 c7fcbaf1e387..2bfd6c108a2f 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, TrackedEntity 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, TrackedEntity 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, TrackedEntity trackedEntity, List programs) { TrackedEntityType trackedEntityType = trackedEntity.getTrackedEntityType(); diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/deprecated/tracker/trackedentity/AbstractTrackedEntityInstanceService.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/deprecated/tracker/trackedentity/AbstractTrackedEntityInstanceService.java index 00bb02efcf85..e4ff24c72ad7 100644 --- a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/deprecated/tracker/trackedentity/AbstractTrackedEntityInstanceService.java +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/deprecated/tracker/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/deprecated/tracker/TrackedEntityServiceTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/dxf2/deprecated/tracker/TrackedEntityServiceTest.java index 38a39d2be8f2..b800eff188b2 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/dxf2/deprecated/tracker/TrackedEntityServiceTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/dxf2/deprecated/tracker/TrackedEntityServiceTest.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; @@ -156,6 +157,7 @@ class TrackedEntityServiceTest extends TransactionalIntegrationTest { private TrackedEntityType trackedEntityType; private FileResource fileResource; + private User user; @Override @@ -572,13 +574,19 @@ void testUpdateTeiByDeletingExistingEventAndAddNewEventForSameProgramStage() { } @Test - void testSavePerson() { + void shouldPassWhenRegisteringPerson() { + trackedEntityType.setSharing(Sharing.builder().publicAccess("rwrw----").build()); + + injectSecurityContextUser(createAndAddUser(Set.of(organisationUnitA), Set.of())); + 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()); } @@ -609,6 +617,46 @@ void shouldCreateTeiWhenValidateUniqueAttribute() { assertEquals(ImportStatus.SUCCESS, importSummary.getStatus()); } + @Test + void shouldFailWhenRegisteringPersonOutsideCaptureScope() { + trackedEntityType.setSharing(Sharing.builder().publicAccess("rwrw----").build()); + injectSecurityContextUser(createAndAddUser(Set.of(organisationUnitB), Set.of())); + 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() { + injectSecurityContextUser(createAndAddUser(Set.of(organisationUnitB), Set.of())); + 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());