fix: [DHIS2-18551] (2.41) Fix access check when registering TEI

dhis2 · Dec 4, 2024 · 9b0ff49 · 9b0ff49
1 parent 872e314
commit 9b0ff49
Show file tree

Hide file tree

Showing 4 changed files with 82 additions and 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
@@ -46,6 +46,8 @@ public interface TrackerAccessManager {
 
   List<String> canWrite(User user, TrackedEntity trackedEntity);
 
+  List<String> 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/...s-service-core/src/main/java/org/hisp/dhis/trackedentity/DefaultTrackerAccessManager.java b/...s-service-core/src/main/java/org/hisp/dhis/trackedentity/DefaultTrackerAccessManager.java
@@ -159,6 +159,35 @@ public List<String> 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<String> canCreate(User user, TrackedEntity trackedEntity) {
+    if (user == null || user.isSuper() || trackedEntity == null) {
+      return List.of();
+    }
+    List<String> 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<String> canWrite(User user, TrackedEntity trackedEntity, List<Program> programs) {
 
     TrackedEntityType trackedEntityType = trackedEntity.getTrackedEntityType();

diff --git a/...hisp/dhis/dxf2/deprecated/tracker/trackedentity/AbstractTrackedEntityInstanceService.java b/...hisp/dhis/dxf2/deprecated/tracker/trackedentity/AbstractTrackedEntityInstanceService.java
@@ -671,7 +671,8 @@ private ImportSummary addTrackedEntityInstance(
       return importSummary;
     }
 
-    List<String> errors = trackerAccessManager.canWrite(importOptions.getUser(), daoEntityInstance);
+    List<String> errors =
+        trackerAccessManager.canCreate(importOptions.getUser(), daoEntityInstance);
 
     if (!errors.isEmpty()) {
       return new ImportSummary(ImportStatus.ERROR, errors.toString()).incrementIgnored();

diff --git a/...gration/src/test/java/org/hisp/dhis/dxf2/deprecated/tracker/TrackedEntityServiceTest.java b/...gration/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());