fix: Validate transfer program/orgUnit combo [DHIS2-17880][2.40]

dhis2 · Aug 15, 2024 · 21e93e3 · 21e93e3
1 parent 924e1b4
commit 21e93e3
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 4 deletions.
diff --git a/...services/dhis-service-core/src/main/java/org/hisp/dhis/program/DefaultProgramService.java b/...services/dhis-service-core/src/main/java/org/hisp/dhis/program/DefaultProgramService.java
@@ -167,7 +167,7 @@ public List<ProgramDataElementDimensionItem> getGeneratedProgramDataElements(Str
 
   @Override
   public boolean hasOrgUnit(Program program, OrganisationUnit organisationUnit) {
-    return this.programStore.hasOrgUnit(program, organisationUnit);
+    return programStore.hasOrgUnit(program, organisationUnit);
   }
 
   @Override

diff --git a/...ervice-core/src/main/java/org/hisp/dhis/trackedentity/DefaultTrackerOwnershipManager.java b/...ervice-core/src/main/java/org/hisp/dhis/trackedentity/DefaultTrackerOwnershipManager.java
@@ -45,6 +45,7 @@
 import org.hisp.dhis.program.Program;
 import org.hisp.dhis.program.ProgramOwnershipHistory;
 import org.hisp.dhis.program.ProgramOwnershipHistoryService;
+import org.hisp.dhis.program.ProgramService;
 import org.hisp.dhis.program.ProgramTempOwner;
 import org.hisp.dhis.program.ProgramTempOwnerService;
 import org.hisp.dhis.program.ProgramTempOwnershipAudit;
@@ -81,6 +82,8 @@ public class DefaultTrackerOwnershipManager implements TrackerOwnershipManager {
 
   private final OrganisationUnitService organisationUnitService;
 
+  private final ProgramService programService;
+
   private final TrackedEntityInstanceService trackedEntityInstanceService;
 
   private final DhisConfigurationProvider config;
@@ -94,6 +97,7 @@ public DefaultTrackerOwnershipManager(
       ProgramOwnershipHistoryService programOwnershipHistoryService,
       TrackedEntityInstanceService trackedEntityInstanceService,
       OrganisationUnitService organisationUnitService,
+      ProgramService programService,
       DhisConfigurationProvider config,
       Environment env) {
     checkNotNull(currentUserService);
@@ -113,6 +117,7 @@ public DefaultTrackerOwnershipManager(
     this.programTempOwnerService = programTempOwnerService;
     this.organisationUnitService = organisationUnitService;
     this.trackedEntityInstanceService = trackedEntityInstanceService;
+    this.programService = programService;
     this.config = config;
     this.ownerCache = cacheProvider.createProgramOwnerCache();
     this.tempOwnerCache = cacheProvider.createProgramTempOwnerCache();
@@ -140,8 +145,9 @@ public void transferOwnership(
       return;
     }
 
-    if (hasAccess(currentUserService.getCurrentUser(), entityInstance, program)
-        || skipAccessValidation) {
+    if ((hasAccess(currentUserService.getCurrentUser(), entityInstance, program)
+            || skipAccessValidation)
+        && programService.hasOrgUnit(program, orgUnit)) {
       TrackedEntityProgramOwner teProgramOwner =
           trackedEntityProgramOwnerService.getTrackedEntityProgramOwner(
               entityInstance.getId(), program.getId());

diff --git a/...st-integration/src/test/java/org/hisp/dhis/trackedentity/TrackerOwnershipManagerTest.java b/...st-integration/src/test/java/org/hisp/dhis/trackedentity/TrackerOwnershipManagerTest.java
@@ -35,6 +35,7 @@
 import static org.hisp.dhis.utils.Assertions.assertIsEmpty;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.time.Instant;
@@ -61,6 +62,7 @@
 import org.hisp.dhis.user.sharing.UserAccess;
 import org.junit.jupiter.api.Test;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.security.access.AccessDeniedException;
 
 /**
  * @author Ameen Mohamed <[email protected]>
@@ -141,6 +143,7 @@ protected void setUpTest() throws Exception {
     programA = createProgram('A');
     programA.setAccessLevel(AccessLevel.PROTECTED);
     programA.setTrackedEntityType(trackedEntityType);
+    programA.setOrganisationUnits(Set.of(organisationUnitA, organisationUnitB));
     programService.addProgram(programA);
     UserAccess userAccess = new UserAccess(userA.getUid(), FULL);
     programA.setSharing(new Sharing(FULL, userAccess));
@@ -293,6 +296,19 @@ void shouldFindTrackedEntityWhenTransferredToInaccessibleOrgUnitIfSuperUser() {
             .collect(Collectors.toList()));
   }
 
+  @Test
+  void shouldNotTransferOwnershipWhenOrgUnitNotAssociatedToProgram() {
+    OrganisationUnit notAssociatedOrgUnit = createOrganisationUnit('C');
+    organisationUnitService.addOrganisationUnit(notAssociatedOrgUnit);
+    Exception exception =
+        assertThrows(
+            AccessDeniedException.class,
+            () -> transferOwnership(entityInstanceA1, programA, notAssociatedOrgUnit));
+    assertEquals(
+        "User does not have access to change ownership for the entity-program combination",
+        exception.getMessage());
+  }
+
   @Test
   void shouldFindTrackedEntityWhenProgramSuppliedAndUserIsOwner() {
     assignOwnership(entityInstanceA1, programA, organisationUnitA);

diff --git a/...web-api/src/test/java/org/hisp/dhis/webapi/controller/TrackerOwnershipControllerTest.java b/...web-api/src/test/java/org/hisp/dhis/webapi/controller/TrackerOwnershipControllerTest.java
@@ -69,7 +69,9 @@ void setUp() {
             HttpStatus.CREATED,
             POST(
                 "/programs/",
-                "{'name':'P1', 'shortName':'P1', 'programType':'WITHOUT_REGISTRATION'}"));
+                "{'name':'P1', 'shortName':'P1', 'programType':'WITHOUT_REGISTRATION', 'organisationUnits': [{'id':'"
+                    + ouId
+                    + "'}]}"));
   }
 
   @Test