From 21e93e322bbf3bbe1160f15a9f760a19fbe22f20 Mon Sep 17 00:00:00 2001 From: Marc Date: Thu, 15 Aug 2024 16:30:37 +0200 Subject: [PATCH] fix: Validate transfer program/orgUnit combo [DHIS2-17880][2.40] --- .../hisp/dhis/program/DefaultProgramService.java | 2 +- .../DefaultTrackerOwnershipManager.java | 10 ++++++++-- .../TrackerOwnershipManagerTest.java | 16 ++++++++++++++++ .../TrackerOwnershipControllerTest.java | 4 +++- 4 files changed, 28 insertions(+), 4 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/program/DefaultProgramService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/program/DefaultProgramService.java index 01614f706179..764827d27819 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/program/DefaultProgramService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/program/DefaultProgramService.java @@ -167,7 +167,7 @@ public List 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/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/trackedentity/DefaultTrackerOwnershipManager.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/trackedentity/DefaultTrackerOwnershipManager.java index a044f33da826..b5548f8b6797 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/trackedentity/DefaultTrackerOwnershipManager.java +++ b/dhis-2/dhis-services/dhis-service-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/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/trackedentity/TrackerOwnershipManagerTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/trackedentity/TrackerOwnershipManagerTest.java index 8ccf93f5d012..ae38cfc0d59b 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/trackedentity/TrackerOwnershipManagerTest.java +++ b/dhis-2/dhis-test-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 @@ -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/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/TrackerOwnershipControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/TrackerOwnershipControllerTest.java index e20ad53885d1..3dc15f3d02e5 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/TrackerOwnershipControllerTest.java +++ b/dhis-2/dhis-test-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