From 5349c0ee9cba4c22f8ef72f3a804ce083e25286d Mon Sep 17 00:00:00 2001 From: Marc Date: Wed, 15 Jan 2025 12:30:28 +0100 Subject: [PATCH 1/7] fix: Allow temp access only when user in search scope [DHIS2-18784] --- .../acl/DefaultTrackerOwnershipManager.java | 59 ++------ .../tracker/acl/TrackerOwnershipManager.java | 15 +- .../TrackerAccessManagerTest.java | 9 +- .../TrackerOwnershipManagerTest.java | 134 ++++++++++-------- .../relationship/RelationshipServiceTest.java | 7 +- .../TrackedEntityServiceTest.java | 29 ++-- .../EventSecurityImportValidationTest.java | 3 +- .../TrackerOwnershipControllerTest.java | 4 +- .../ownership/TrackerOwnershipController.java | 6 +- 9 files changed, 124 insertions(+), 142 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerOwnershipManager.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerOwnershipManager.java index 068d619ce5d0..e0f246758cda 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerOwnershipManager.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerOwnershipManager.java @@ -53,7 +53,6 @@ import org.hisp.dhis.user.CurrentUserUtil; import org.hisp.dhis.user.UserDetails; import org.hisp.dhis.user.UserService; -import org.springframework.security.access.AccessDeniedException; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; @@ -147,63 +146,26 @@ public void transferOwnership( } } - @Override - @Transactional - public void assignOwnership( - TrackedEntity trackedEntity, - Program program, - OrganisationUnit organisationUnit, - boolean skipAccessValidation, - boolean overwriteIfExists) { - if (trackedEntity == null || program == null || organisationUnit == null) { - return; - } - - UserDetails currentUser = CurrentUserUtil.getCurrentUserDetails(); - - if (hasAccess(currentUser, trackedEntity, program) || skipAccessValidation) { - TrackedEntityProgramOwner teProgramOwner = - trackedEntityProgramOwnerService.getTrackedEntityProgramOwner(trackedEntity, program); - - if (teProgramOwner != null) { - if (overwriteIfExists && !teProgramOwner.getOrganisationUnit().equals(organisationUnit)) { - ProgramOwnershipHistory programOwnershipHistory = - new ProgramOwnershipHistory( - program, - trackedEntity, - teProgramOwner.getOrganisationUnit(), - teProgramOwner.getLastUpdated(), - teProgramOwner.getCreatedBy()); - programOwnershipHistoryService.addProgramOwnershipHistory(programOwnershipHistory); - trackedEntityProgramOwnerService.updateTrackedEntityProgramOwner( - trackedEntity, program, organisationUnit); - } - } else { - trackedEntityProgramOwnerService.createTrackedEntityProgramOwner( - trackedEntity, program, organisationUnit); - } - - ownerCache.invalidate(getOwnershipCacheKey(trackedEntity::getId, program)); - } else { - log.error("Unauthorized attempt to assign ownership"); - throw new AccessDeniedException( - "User does not have access to assign ownership for the entity-program combination"); - } - } - @Override @Transactional public void grantTemporaryOwnership( - TrackedEntity trackedEntity, Program program, UserDetails user, String reason) { + TrackedEntity trackedEntity, Program program, UserDetails user, String reason) + throws ForbiddenException { if (canSkipOwnershipCheck(user, program) || trackedEntity == null) { return; } if (program.isProtected()) { + if (!isOwnerInUserSearchScope(user, trackedEntity, program)) { + throw new ForbiddenException( + "The owner of the entity-program combination is not in the user's search scope"); + } + if (config.isEnabled(CHANGELOG_TRACKER)) { programTempOwnershipAuditService.addProgramTempOwnershipAudit( new ProgramTempOwnershipAudit(program, trackedEntity, reason, user.getUsername())); } + ProgramTempOwner programTempOwner = new ProgramTempOwner( program, @@ -214,6 +176,11 @@ public void grantTemporaryOwnership( programTempOwnerService.addProgramTempOwner(programTempOwner); tempOwnerCache.invalidate( getTempOwnershipCacheKey(trackedEntity.getUid(), program.getUid(), user.getUid())); + } else { + throw new ForbiddenException( + String.format( + "It's only possible to grant temporary ownership to protected programs, and %s access level is %s", + program.getUid(), program.getAccessLevel().name())); } } diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/acl/TrackerOwnershipManager.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/acl/TrackerOwnershipManager.java index bd1a930bbfb7..ac0a7ac65c16 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/acl/TrackerOwnershipManager.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/acl/TrackerOwnershipManager.java @@ -52,18 +52,6 @@ public interface TrackerOwnershipManager { void transferOwnership(TrackedEntity trackedEntity, Program program, OrganisationUnit orgUnit) throws ForbiddenException; - /** - * @param trackedEntity The tracked entity object - * @param program The program object - * @param organisationUnit The org unit that has to become the owner - */ - void assignOwnership( - TrackedEntity trackedEntity, - Program program, - OrganisationUnit organisationUnit, - boolean skipAccessValidation, - boolean overwriteIfExists); - /** * Check whether the user has access (as owner or has temporarily broken the glass) to the tracked * entity - program combination. @@ -87,7 +75,8 @@ boolean hasAccess( * @param reason The reason for requesting temporary ownership */ void grantTemporaryOwnership( - TrackedEntity trackedEntity, Program program, UserDetails user, String reason); + TrackedEntity trackedEntity, Program program, UserDetails user, String reason) + throws ForbiddenException; /** * Ownership check can be skipped if the user is superuser or if the program type is without diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/trackedentity/TrackerAccessManagerTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/trackedentity/TrackerAccessManagerTest.java index 4fb09bcbeb84..ba633bdbfbd7 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/trackedentity/TrackerAccessManagerTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/trackedentity/TrackerAccessManagerTest.java @@ -62,6 +62,7 @@ import org.hisp.dhis.program.ProgramType; import org.hisp.dhis.security.acl.AccessStringHelper; import org.hisp.dhis.test.integration.PostgresIntegrationTestBase; +import org.hisp.dhis.tracker.acl.TrackedEntityProgramOwnerService; import org.hisp.dhis.tracker.acl.TrackerAccessManager; import org.hisp.dhis.tracker.acl.TrackerOwnershipManager; import org.hisp.dhis.tracker.export.trackedentity.TrackedEntityService; @@ -82,6 +83,8 @@ class TrackerAccessManagerTest extends PostgresIntegrationTestBase { @Autowired private TrackerOwnershipManager trackerOwnershipManager; + @Autowired private TrackedEntityProgramOwnerService trackedEntityProgramOwnerService; + @Autowired private TrackedEntityTypeService trackedEntityTypeService; @Autowired private TrackedEntityService trackedEntityService; @@ -271,7 +274,8 @@ void checkAccessPermissionForEnrollmentInClosedProgram() throws ForbiddenExcepti "User has no create access to organisation unit:"); enrollment.setOrganisationUnit(orgUnitA); // Transferring ownership to orgUnitB. user is no longer owner - trackerOwnershipManager.assignOwnership(trackedEntity, programA, orgUnitA, false, true); + trackedEntityProgramOwnerService.createTrackedEntityProgramOwner( + trackedEntity, programA, orgUnitA); trackerOwnershipManager.transferOwnership(trackedEntity, programA, orgUnitB); // Cannot create enrollment if not owner assertHasError( @@ -388,7 +392,8 @@ void checkAccessPermissionsForEventInClosedProgram() throws ForbiddenException { assertNoErrors(trackerAccessManager.canUpdate(userDetails, eventB, false)); // Can delete events if user is owner irrespective of eventOU assertNoErrors(trackerAccessManager.canDelete(userDetails, eventB, false)); - trackerOwnershipManager.assignOwnership(trackedEntityA, programA, orgUnitA, false, true); + trackedEntityProgramOwnerService.createTrackedEntityProgramOwner( + trackedEntityA, programA, orgUnitA); trackerOwnershipManager.transferOwnership(trackedEntityA, programA, orgUnitB); // Cannot create events anywhere if user is not owner assertHasErrors(2, trackerAccessManager.canCreate(userDetails, eventB, false)); 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 c85e2dcacd0b..7b6a0aa0b0d7 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 @@ -27,7 +27,11 @@ */ package org.hisp.dhis.trackedentity; +import static org.hisp.dhis.common.AccessLevel.AUDITED; +import static org.hisp.dhis.common.AccessLevel.CLOSED; +import static org.hisp.dhis.common.AccessLevel.OPEN; import static org.hisp.dhis.common.OrganisationUnitSelectionMode.ACCESSIBLE; +import static org.hisp.dhis.test.utils.Assertions.assertContains; import static org.hisp.dhis.test.utils.Assertions.assertContainsOnly; import static org.hisp.dhis.test.utils.Assertions.assertIsEmpty; import static org.hisp.dhis.tracker.TrackerTestUtils.uids; @@ -39,6 +43,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.Stream; import org.hisp.dhis.common.AccessLevel; import org.hisp.dhis.common.IdentifiableObjectManager; import org.hisp.dhis.common.UID; @@ -53,6 +58,7 @@ import org.hisp.dhis.security.Authorities; import org.hisp.dhis.security.acl.AccessStringHelper; import org.hisp.dhis.test.integration.PostgresIntegrationTestBase; +import org.hisp.dhis.tracker.acl.TrackedEntityProgramOwnerService; import org.hisp.dhis.tracker.acl.TrackerOwnershipManager; import org.hisp.dhis.tracker.export.trackedentity.TrackedEntityEnrollmentParams; import org.hisp.dhis.tracker.export.trackedentity.TrackedEntityOperationParams; @@ -61,8 +67,11 @@ import org.hisp.dhis.user.UserDetails; import org.hisp.dhis.user.sharing.Sharing; import org.hisp.dhis.user.sharing.UserAccess; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; import org.springframework.beans.factory.annotation.Autowired; /** @@ -83,6 +92,8 @@ class TrackerOwnershipManagerTest extends PostgresIntegrationTestBase { @Autowired private TrackedEntityTypeService trackedEntityTypeService; + @Autowired private TrackedEntityProgramOwnerService trackedEntityProgramOwnerService; + private TrackedEntity trackedEntityA1; private TrackedEntity trackedEntityB1; @@ -162,25 +173,19 @@ void setUp() { } @Test - void testAssignOwnership() { + void shouldFailWhenGrantingTemporaryOwnershipAndUserNotInSearchScope() { assertTrue(trackerOwnershipAccessManager.hasAccess(userDetailsA, trackedEntityA1, programA)); assertFalse(trackerOwnershipAccessManager.hasAccess(userDetailsB, trackedEntityA1, programA)); - assertTrue(trackerOwnershipAccessManager.hasAccess(userDetailsB, trackedEntityB1, programA)); - trackerOwnershipAccessManager.assignOwnership( - trackedEntityA1, programA, organisationUnitB, false, true); - assertFalse(trackerOwnershipAccessManager.hasAccess(userDetailsA, trackedEntityA1, programA)); - assertTrue(trackerOwnershipAccessManager.hasAccess(userDetailsB, trackedEntityA1, programA)); - } + Exception exception = + Assertions.assertThrows( + ForbiddenException.class, + () -> + trackerOwnershipAccessManager.grantTemporaryOwnership( + trackedEntityA1, programA, userDetailsB, "testing reason")); - @Test - void testGrantTemporaryOwnershipWithAudit() { - assertTrue(trackerOwnershipAccessManager.hasAccess(userDetailsA, trackedEntityA1, programA)); - assertFalse(trackerOwnershipAccessManager.hasAccess(userDetailsB, trackedEntityA1, programA)); - trackerOwnershipAccessManager.grantTemporaryOwnership( - trackedEntityA1, programA, userDetailsB, "testing reason"); - assertTrue(trackerOwnershipAccessManager.hasAccess(userDetailsA, trackedEntityA1, programA)); - assertTrue(trackerOwnershipAccessManager.hasAccess(userDetailsA, trackedEntityA1, programA)); - assertTrue(trackerOwnershipAccessManager.hasAccess(userDetailsB, trackedEntityA1, programA)); + assertEquals( + "The owner of the entity-program combination is not in the user's search scope", + exception.getMessage()); } @Test @@ -188,8 +193,8 @@ void shouldNotHaveAccessToEnrollmentWithUserAWhenTransferredToAnotherOrgUnit() throws ForbiddenException { userA.setTeiSearchOrganisationUnits(Set.of(organisationUnitB)); userService.updateUser(userA); - trackerOwnershipAccessManager.assignOwnership( - trackedEntityA1, programA, organisationUnitA, false, true); + trackedEntityProgramOwnerService.createTrackedEntityProgramOwner( + trackedEntityA1, programA, organisationUnitA); trackerOwnershipAccessManager.transferOwnership(trackedEntityA1, programA, organisationUnitB); injectSecurityContextUser(userA); @@ -205,8 +210,8 @@ void shouldNotHaveAccessToEnrollmentWithUserAWhenTransferredToAnotherOrgUnit() @Test void shouldHaveAccessToEnrollmentWithUserBWhenTransferredToOwnOrgUnit() throws ForbiddenException, NotFoundException, BadRequestException { - trackerOwnershipAccessManager.assignOwnership( - trackedEntityA1, programA, organisationUnitA, false, true); + trackedEntityProgramOwnerService.createTrackedEntityProgramOwner( + trackedEntityA1, programA, organisationUnitA); trackerOwnershipAccessManager.transferOwnership(trackedEntityA1, programA, organisationUnitB); injectSecurityContextUser(userB); @@ -219,8 +224,8 @@ void shouldHaveAccessToEnrollmentWithUserBWhenTransferredToOwnOrgUnit() @Test void shouldHaveAccessToEnrollmentWithSuperUserWhenTransferredToOwnOrgUnit() throws ForbiddenException, NotFoundException, BadRequestException { - trackerOwnershipAccessManager.assignOwnership( - trackedEntityA1, programA, organisationUnitA, false, true); + trackedEntityProgramOwnerService.createTrackedEntityProgramOwner( + trackedEntityA1, programA, organisationUnitA); trackerOwnershipAccessManager.transferOwnership(trackedEntityA1, programA, organisationUnitB); superUser.setOrganisationUnits(Set.of(organisationUnitB)); userService.updateUser(superUser); @@ -245,8 +250,8 @@ void shouldHaveAccessToTEWhenProgramNotProvidedButUserHasAccessToAtLeastOneProgr @Test void shouldNotHaveAccessToTEWhenProgramNotProvidedAndUserHasNoAccessToAnyProgram() { injectSecurityContextUser(userA); - trackerOwnershipAccessManager.assignOwnership( - trackedEntityA1, programA, organisationUnitB, false, true); + trackedEntityProgramOwnerService.createTrackedEntityProgramOwner( + trackedEntityA1, programA, organisationUnitB); ForbiddenException exception = assertThrows( @@ -272,9 +277,14 @@ void shouldHaveAccessWhenProgramProtectedAndUserInCaptureScope() { } @Test - void shouldHaveAccessWhenProgramProtectedAndHasTemporaryAccess() { + void shouldHaveAccessWhenProgramProtectedAndHasTemporaryAccess() throws ForbiddenException { + userB.setTeiSearchOrganisationUnits(Set.of(organisationUnitA)); + userService.updateUser(userB); + userDetailsB = UserDetails.fromUser(userB); + trackerOwnershipAccessManager.grantTemporaryOwnership( trackedEntityA1, programA, userDetailsB, "test protected program"); + assertTrue(trackerOwnershipAccessManager.hasAccess(userDetailsB, trackedEntityA1, programA)); assertTrue( trackerOwnershipAccessManager.hasAccess( @@ -338,32 +348,30 @@ void shouldHaveAccessWhenProgramClosedAndUserInCaptureScope() { programB)); } - @Test - void shouldNotHaveAccessWhenProgramClosedAndUserHasTemporaryAccess() { - trackerOwnershipAccessManager.grantTemporaryOwnership( - trackedEntityA1, programB, userDetailsB, "test closed program"); - assertFalse(trackerOwnershipAccessManager.hasAccess(userDetailsB, trackedEntityA1, programB)); - assertFalse( - trackerOwnershipAccessManager.hasAccess( - UserDetails.fromUser(userB), - trackedEntityA1.getUid(), - trackedEntityA1.getOrganisationUnit(), - programB)); + private static Stream providePrograms() { + return Stream.of(createProgram(OPEN), createProgram(AUDITED), createProgram(CLOSED)); + } - injectSecurityContextUser(userB); - ForbiddenException exception = - assertThrows( + @ParameterizedTest + @MethodSource("providePrograms") + void shouldFailWhenGrantingTemporaryOwnershipToProgramWithAccessLevelOtherThanProtected( + Program program) { + Exception exception = + Assertions.assertThrows( ForbiddenException.class, () -> - trackedEntityService.getTrackedEntity( - UID.of(trackedEntityA1), UID.of(programB), defaultParams)); - assertEquals(TrackerOwnershipManager.PROGRAM_ACCESS_CLOSED, exception.getMessage()); + trackerOwnershipAccessManager.grantTemporaryOwnership( + trackedEntityA1, program, userDetailsB, "test temporary ownership")); + + assertContains( + "It's only possible to grant temporary ownership to protected programs", + exception.getMessage()); } @Test void shouldHaveAccessWhenProgramOpenAndUserInScope() throws ForbiddenException, NotFoundException, BadRequestException { - programA.setAccessLevel(AccessLevel.OPEN); + programA.setAccessLevel(OPEN); programService.updateProgram(programA); assertEquals( @@ -374,11 +382,11 @@ void shouldHaveAccessWhenProgramOpenAndUserInScope() @Test void shouldNotHaveAccessWhenProgramOpenAndUserNotInSearchScope() throws ForbiddenException { - programA.setAccessLevel(AccessLevel.OPEN); + programA.setAccessLevel(OPEN); programService.updateProgram(programA); - trackerOwnershipAccessManager.assignOwnership( - trackedEntityA1, programA, organisationUnitA, false, true); + trackedEntityProgramOwnerService.createTrackedEntityProgramOwner( + trackedEntityA1, programA, organisationUnitA); trackerOwnershipAccessManager.transferOwnership(trackedEntityA1, programA, organisationUnitB); injectSecurityContextUser(userA); @@ -394,8 +402,8 @@ void shouldNotHaveAccessWhenProgramOpenAndUserNotInSearchScope() throws Forbidde @Test void shouldHaveAccessWhenProgramNotProvidedAndTEEnrolledButHaveAccessToTEOwner() throws ForbiddenException, NotFoundException, BadRequestException { - trackerOwnershipAccessManager.assignOwnership( - trackedEntityA1, programA, organisationUnitA, false, true); + trackedEntityProgramOwnerService.createTrackedEntityProgramOwner( + trackedEntityA1, programA, organisationUnitA); trackerOwnershipAccessManager.transferOwnership(trackedEntityA1, programA, organisationUnitB); injectSecurityContextUser(userB); @@ -447,8 +455,8 @@ void shouldNotHaveAccessWhenProgramNotProvidedAndTENotEnrolledAndNoAccessToTeReg @Test void shouldFindTrackedEntityWhenTransferredToAccessibleOrgUnit() throws ForbiddenException, BadRequestException, NotFoundException { - trackerOwnershipAccessManager.assignOwnership( - trackedEntityA1, programA, organisationUnitA, false, true); + trackedEntityProgramOwnerService.createTrackedEntityProgramOwner( + trackedEntityA1, programA, organisationUnitA); transferOwnership(trackedEntityA1, programA, organisationUnitB); TrackedEntityOperationParams operationParams = createOperationParams(null); injectSecurityContext(userDetailsB); @@ -461,8 +469,8 @@ void shouldFindTrackedEntityWhenTransferredToAccessibleOrgUnit() @Test void shouldFindTrackedEntityWhenTransferredToAccessibleOrgUnitAndSuperUser() throws ForbiddenException, BadRequestException, NotFoundException { - trackerOwnershipAccessManager.assignOwnership( - trackedEntityA1, programA, organisationUnitA, false, true); + trackedEntityProgramOwnerService.createTrackedEntityProgramOwner( + trackedEntityA1, programA, organisationUnitA); transferOwnership(trackedEntityA1, programA, organisationUnitB); superUser.setOrganisationUnits(Set.of(organisationUnitB)); userService.updateUser(superUser); @@ -477,8 +485,8 @@ void shouldFindTrackedEntityWhenTransferredToAccessibleOrgUnitAndSuperUser() @Test void shouldNotFindTrackedEntityWhenTransferredToInaccessibleOrgUnit() throws ForbiddenException, BadRequestException, NotFoundException { - trackerOwnershipAccessManager.assignOwnership( - trackedEntityA1, programA, organisationUnitA, false, true); + trackedEntityProgramOwnerService.createTrackedEntityProgramOwner( + trackedEntityA1, programA, organisationUnitA); transferOwnership(trackedEntityA1, programA, organisationUnitB); TrackedEntityOperationParams operationParams = createOperationParams(null); @@ -531,7 +539,8 @@ void shouldNotTransferOwnershipWhenOrgUnitNotAssociatedToProgram() { @Test void shouldFindTrackedEntityWhenProgramSuppliedAndUserIsOwner() throws ForbiddenException, BadRequestException, NotFoundException { - assignOwnership(trackedEntityA1, programA, organisationUnitA); + trackedEntityProgramOwnerService.createTrackedEntityProgramOwner( + trackedEntityA1, programA, organisationUnitA); TrackedEntityOperationParams operationParams = createOperationParams(UID.of(programA)); injectSecurityContext(userDetailsA); @@ -543,7 +552,8 @@ void shouldFindTrackedEntityWhenProgramSuppliedAndUserIsOwner() @Test void shouldNotFindTrackedEntityWhenProgramSuppliedAndUserIsNotOwner() throws ForbiddenException, BadRequestException, NotFoundException { - assignOwnership(trackedEntityA1, programA, organisationUnitA); + trackedEntityProgramOwnerService.createTrackedEntityProgramOwner( + trackedEntityA1, programA, organisationUnitA); TrackedEntityOperationParams operationParams = createOperationParams(UID.of(programA)); injectSecurityContext(userDetailsB); @@ -556,11 +566,6 @@ private void transferOwnership( trackerOwnershipAccessManager.transferOwnership(trackedEntity, program, orgUnit); } - private void assignOwnership( - TrackedEntity trackedEntity, Program program, OrganisationUnit orgUnit) { - trackerOwnershipAccessManager.assignOwnership(trackedEntity, program, orgUnit, false, true); - } - private TrackedEntityOperationParams createOperationParams(UID programUid) { return TrackedEntityOperationParams.builder() .trackedEntityType(trackedEntityA1.getTrackedEntityType()) @@ -573,4 +578,11 @@ private List getTrackedEntities(TrackedEntityOperationParams params) throws ForbiddenException, BadRequestException, NotFoundException { return uids(trackedEntityService.getTrackedEntities(params)); } + + private static Program createProgram(AccessLevel accessLevel) { + Program program = new Program(); + program.setAccessLevel(accessLevel); + + return program; + } } diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/relationship/RelationshipServiceTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/relationship/RelationshipServiceTest.java index 07af5aace287..7c470ced7901 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/relationship/RelationshipServiceTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/relationship/RelationshipServiceTest.java @@ -58,6 +58,7 @@ import org.hisp.dhis.test.utils.RelationshipUtils; import org.hisp.dhis.trackedentity.TrackedEntity; import org.hisp.dhis.trackedentity.TrackedEntityType; +import org.hisp.dhis.tracker.acl.TrackedEntityProgramOwnerService; import org.hisp.dhis.tracker.acl.TrackerOwnershipManager; import org.hisp.dhis.user.User; import org.hisp.dhis.user.UserService; @@ -79,6 +80,8 @@ class RelationshipServiceTest extends PostgresIntegrationTestBase { @Autowired private TrackerOwnershipManager trackerOwnershipAccessManager; + @Autowired private TrackedEntityProgramOwnerService trackedEntityProgramOwnerService; + private Date enrollmentDate; private TrackedEntity teA; @@ -301,8 +304,8 @@ void shouldNotReturnRelationshipWhenTeIsTransferredAndUserHasNoAccessToAtLeastOn manager.save(createEnrollment(program, trackedEntityFrom, orgUnitA)); - trackerOwnershipAccessManager.assignOwnership( - trackedEntityFrom, program, orgUnitA, false, true); + trackedEntityProgramOwnerService.createTrackedEntityProgramOwner( + trackedEntityFrom, program, orgUnitA); trackerOwnershipAccessManager.transferOwnership(trackedEntityFrom, program, orgUnitB); diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityServiceTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityServiceTest.java index 160bb6c6281c..3a3455de7266 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityServiceTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/export/trackedentity/TrackedEntityServiceTest.java @@ -100,7 +100,7 @@ import org.hisp.dhis.trackedentity.TrackedEntityType; import org.hisp.dhis.trackedentity.TrackedEntityTypeAttribute; import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValue; -import org.hisp.dhis.tracker.acl.TrackerOwnershipManager; +import org.hisp.dhis.tracker.acl.TrackedEntityProgramOwnerService; import org.hisp.dhis.tracker.export.trackedentity.TrackedEntityOperationParams.TrackedEntityOperationParamsBuilder; import org.hisp.dhis.tracker.trackedentityattributevalue.TrackedEntityAttributeValueService; import org.hisp.dhis.user.User; @@ -123,7 +123,7 @@ class TrackedEntityServiceTest extends PostgresIntegrationTestBase { @Autowired private TrackedEntityAttributeValueService attributeValueService; - @Autowired private TrackerOwnershipManager trackerOwnershipManager; + @Autowired private TrackedEntityProgramOwnerService trackedEntityProgramOwnerService; private User user; @@ -402,8 +402,10 @@ void setUp() { trackedEntityC.setTrackedEntityType(trackedEntityTypeA); manager.save(trackedEntityC, false); - trackerOwnershipManager.assignOwnership(trackedEntityA, programA, orgUnitA, true, true); - trackerOwnershipManager.assignOwnership(trackedEntityA, programB, orgUnitA, true, true); + trackedEntityProgramOwnerService.createTrackedEntityProgramOwner( + trackedEntityA, programA, orgUnitA); + trackedEntityProgramOwnerService.createTrackedEntityProgramOwner( + trackedEntityA, programB, orgUnitA); attributeValueService.addTrackedEntityAttributeValue( new TrackedEntityAttributeValue(teaA, trackedEntityA, "A")); @@ -595,7 +597,8 @@ void shouldReturnTrackedEntitiesGivenUserHasDataReadAccessToTrackedEntityType() void shouldReturnTrackedEntityIncludingAllAttributesEnrollmentsEventsRelationshipsOwners() throws ForbiddenException, NotFoundException, BadRequestException { // this was declared as "remove ownership"; unclear to me how this is removing ownership - trackerOwnershipManager.assignOwnership(trackedEntityA, programB, orgUnitB, true, true); + trackedEntityProgramOwnerService.updateTrackedEntityProgramOwner( + trackedEntityA, programB, orgUnitB); TrackedEntityOperationParams operationParams = TrackedEntityOperationParams.builder() @@ -1530,8 +1533,8 @@ void shouldNotReturnTrackedEntityRelationshipWhenTEFromItemNotAccessible() manager.save(inaccessibleOrgUnit); makeProgramMetadataInaccessible(programB); makeProgramMetadataInaccessible(programC); - trackerOwnershipManager.assignOwnership( - trackedEntityA, programA, inaccessibleOrgUnit, true, true); + trackedEntityProgramOwnerService.updateTrackedEntityProgramOwner( + trackedEntityA, programA, inaccessibleOrgUnit); TrackedEntityOperationParams operationParams = createOperationParams(orgUnitB, trackedEntityB); injectSecurityContextUser(user); @@ -1553,8 +1556,8 @@ void shouldNotReturnTrackedEntityRelationshipWhenTEToItemNotAccessible() manager.save(inaccessibleOrgUnit); makeProgramMetadataInaccessible(programB); makeProgramMetadataInaccessible(programC); - trackerOwnershipManager.assignOwnership( - trackedEntityB, programA, inaccessibleOrgUnit, true, true); + trackedEntityProgramOwnerService.createTrackedEntityProgramOwner( + trackedEntityB, programA, inaccessibleOrgUnit); TrackedEntityOperationParams operationParams = createOperationParams(orgUnitA, trackedEntityA); injectSecurityContextUser(user); @@ -1593,8 +1596,8 @@ void shouldNotReturnTrackedEntityRelationshipWhenEnrollmentItemNotAccessible() OrganisationUnit inaccessibleOrgUnit = createOrganisationUnit('D'); manager.save(inaccessibleOrgUnit); makeProgramMetadataInaccessible(programC); - trackerOwnershipManager.assignOwnership( - trackedEntityB, programB, inaccessibleOrgUnit, true, true); + trackedEntityProgramOwnerService.createTrackedEntityProgramOwner( + trackedEntityB, programB, inaccessibleOrgUnit); TrackedEntityOperationParams operationParams = createOperationParams(orgUnitA, trackedEntityA); injectSecurityContextUser(user); @@ -1633,8 +1636,8 @@ void shouldReturnTrackedEntityRelationshipWhenEventItemNotAccessible() OrganisationUnit inaccessibleOrgUnit = createOrganisationUnit('D'); manager.save(inaccessibleOrgUnit); makeProgramMetadataInaccessible(programC); - trackerOwnershipManager.assignOwnership( - trackedEntityB, programB, inaccessibleOrgUnit, true, true); + trackedEntityProgramOwnerService.createTrackedEntityProgramOwner( + trackedEntityB, programB, inaccessibleOrgUnit); TrackedEntityOperationParams operationParams = createOperationParams(orgUnitA, trackedEntityA); injectSecurityContextUser(user); diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/EventSecurityImportValidationTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/EventSecurityImportValidationTest.java index 76695043d1dd..e649f3633fb1 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/EventSecurityImportValidationTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/EventSecurityImportValidationTest.java @@ -204,7 +204,8 @@ void setUp() throws IOException { manager.save(enrollmentA); maleA.getEnrollments().add(enrollmentA); manager.update(maleA); - trackerOwnershipAccessManager.assignOwnership(maleA, programA, organisationUnitA, false, false); + trackedEntityProgramOwnerService.updateTrackedEntityProgramOwner( + maleA, programA, organisationUnitA); trackedEntityProgramOwnerService.updateTrackedEntityProgramOwner( maleA, programA, organisationUnitA); diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/ownership/TrackerOwnershipControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/ownership/TrackerOwnershipControllerTest.java index ad99714e7c4b..9ff9426e9a51 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/ownership/TrackerOwnershipControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/tracker/ownership/TrackerOwnershipControllerTest.java @@ -170,7 +170,7 @@ void shouldFailToUpdateWhenNoTrackedEntityOrTrackedEntityInstanceParametersArePr } @Test - void shouldOverrideOwnershipAccessWhenUsingDeprecateTrackedEntityInstanceParam() { + void shouldGrantTemporaryAccessWhenUsingDeprecateTrackedEntityInstanceParam() { assertWebMessage( "OK", 200, @@ -184,7 +184,7 @@ void shouldOverrideOwnershipAccessWhenUsingDeprecateTrackedEntityInstanceParam() } @Test - void shouldOverrideOwnershipAccess() { + void shouldGrantTemporaryAccess() { assertWebMessage( "OK", 200, diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/ownership/TrackerOwnershipController.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/ownership/TrackerOwnershipController.java index 1a4d3d5fdd2d..840b0bcf4501 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/ownership/TrackerOwnershipController.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/ownership/TrackerOwnershipController.java @@ -43,6 +43,7 @@ import org.hisp.dhis.program.Program; import org.hisp.dhis.program.ProgramService; import org.hisp.dhis.tracker.acl.TrackerOwnershipManager; +import org.hisp.dhis.tracker.export.trackedentity.TrackedEntityParams; import org.hisp.dhis.tracker.export.trackedentity.TrackedEntityService; import org.hisp.dhis.user.CurrentUserUtil; import org.hisp.dhis.user.UserDetails; @@ -97,7 +98,8 @@ public WebMessage updateTrackerProgramOwner( "trackedEntityInstance", trackedEntityInstance, "trackedEntity", trackedEntity); trackerOwnershipAccessManager.transferOwnership( - trackedEntityService.getTrackedEntity(trackedEntityUid), + trackedEntityService.getTrackedEntity( + trackedEntityUid, UID.of(program), TrackedEntityParams.TRUE), programService.getProgram(program), organisationUnitService.getOrganisationUnit(ou)); return ok("Ownership transferred"); @@ -105,7 +107,7 @@ public WebMessage updateTrackerProgramOwner( @PostMapping(value = "/override", produces = APPLICATION_JSON_VALUE) @ResponseBody - public WebMessage overrideOwnershipAccess( + public WebMessage grantTemporaryAccess( @Deprecated(since = "2.41") @RequestParam(required = false) UID trackedEntityInstance, @RequestParam(required = false) UID trackedEntity, @RequestParam String reason, From 3ffa1cb5469ab7a8ae7a77ed7e326b4e10293c99 Mon Sep 17 00:00:00 2001 From: Marc Date: Wed, 15 Jan 2025 12:42:59 +0100 Subject: [PATCH 2/7] fix: Rephrase message in exception [DHIS2-18784] --- .../hisp/dhis/tracker/acl/DefaultTrackerOwnershipManager.java | 4 ++-- .../hisp/dhis/trackedentity/TrackerOwnershipManagerTest.java | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerOwnershipManager.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerOwnershipManager.java index e0f246758cda..d0d475f4cc96 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerOwnershipManager.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerOwnershipManager.java @@ -158,7 +158,7 @@ public void grantTemporaryOwnership( if (program.isProtected()) { if (!isOwnerInUserSearchScope(user, trackedEntity, program)) { throw new ForbiddenException( - "The owner of the entity-program combination is not in the user's search scope"); + "The owner of the entity-program combination is not in the user's search scope."); } if (config.isEnabled(CHANGELOG_TRACKER)) { @@ -179,7 +179,7 @@ public void grantTemporaryOwnership( } else { throw new ForbiddenException( String.format( - "It's only possible to grant temporary ownership to protected programs, and %s access level is %s", + "Temporary ownership can only be granted to protected programs. %s access level is %s.", program.getUid(), program.getAccessLevel().name())); } } 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 7b6a0aa0b0d7..b530ca892afb 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 @@ -364,8 +364,7 @@ void shouldFailWhenGrantingTemporaryOwnershipToProgramWithAccessLevelOtherThanPr trackedEntityA1, program, userDetailsB, "test temporary ownership")); assertContains( - "It's only possible to grant temporary ownership to protected programs", - exception.getMessage()); + "Temporary ownership can only be granted to protected programs.", exception.getMessage()); } @Test From 7c64434dd86af0aef2f1cc7d74d195011b961435 Mon Sep 17 00:00:00 2001 From: Marc Date: Wed, 15 Jan 2025 13:03:03 +0100 Subject: [PATCH 3/7] fix: Remove unused spring dependency [DHIS2-18784] --- dhis-2/dhis-services/dhis-service-tracker/pom.xml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-tracker/pom.xml b/dhis-2/dhis-services/dhis-service-tracker/pom.xml index e4067be79ef4..80f3cf952c24 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/pom.xml +++ b/dhis-2/dhis-services/dhis-service-tracker/pom.xml @@ -167,10 +167,6 @@ org.geotools gt-main - - org.springframework.security - spring-security-core - From 8b43c72f5e93357f87626e1de3dd0c2d1edba48e Mon Sep 17 00:00:00 2001 From: Marc Date: Wed, 15 Jan 2025 13:26:36 +0100 Subject: [PATCH 4/7] fix: Add dot at the end of the sentence [DHIS2-18784] --- .../hisp/dhis/trackedentity/TrackerOwnershipManagerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 b530ca892afb..0f6ba531329c 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 @@ -184,7 +184,7 @@ void shouldFailWhenGrantingTemporaryOwnershipAndUserNotInSearchScope() { trackedEntityA1, programA, userDetailsB, "testing reason")); assertEquals( - "The owner of the entity-program combination is not in the user's search scope", + "The owner of the entity-program combination is not in the user's search scope.", exception.getMessage()); } From 34b19739fbda9ef7b7a9ae2e31e1c4f50a5fe8b7 Mon Sep 17 00:00:00 2001 From: Marc Date: Wed, 15 Jan 2025 16:44:40 +0100 Subject: [PATCH 5/7] fix: Refactor to improve readability [DHIS2-18784] --- .../acl/DefaultTrackerOwnershipManager.java | 52 ++++++++++--------- .../TrackerOwnershipManagerTest.java | 52 ++++++++++++++++++- 2 files changed, 78 insertions(+), 26 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerOwnershipManager.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerOwnershipManager.java index d0d475f4cc96..0a1115a924fe 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerOwnershipManager.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerOwnershipManager.java @@ -32,6 +32,7 @@ import java.util.Optional; import java.util.function.LongSupplier; import java.util.function.Supplier; +import javax.annotation.Nonnull; import lombok.extern.slf4j.Slf4j; import org.hibernate.Hibernate; import org.hisp.dhis.cache.Cache; @@ -149,39 +150,40 @@ public void transferOwnership( @Override @Transactional public void grantTemporaryOwnership( - TrackedEntity trackedEntity, Program program, UserDetails user, String reason) + @Nonnull TrackedEntity trackedEntity, Program program, UserDetails user, String reason) throws ForbiddenException { - if (canSkipOwnershipCheck(user, program) || trackedEntity == null) { - return; + if (canSkipOwnershipCheck(user, program)) { + throw new ForbiddenException( + "Temporary ownership not created. Either current user is a superuser, program supplied is does not exist or program supplied is not a tracker program."); } - if (program.isProtected()) { - if (!isOwnerInUserSearchScope(user, trackedEntity, program)) { - throw new ForbiddenException( - "The owner of the entity-program combination is not in the user's search scope."); - } - - if (config.isEnabled(CHANGELOG_TRACKER)) { - programTempOwnershipAuditService.addProgramTempOwnershipAudit( - new ProgramTempOwnershipAudit(program, trackedEntity, reason, user.getUsername())); - } - - ProgramTempOwner programTempOwner = - new ProgramTempOwner( - program, - trackedEntity, - reason, - userService.getUser(user.getUid()), - TEMPORARY_OWNERSHIP_VALIDITY_IN_HOURS); - programTempOwnerService.addProgramTempOwner(programTempOwner); - tempOwnerCache.invalidate( - getTempOwnershipCacheKey(trackedEntity.getUid(), program.getUid(), user.getUid())); - } else { + if (!program.isProtected()) { throw new ForbiddenException( String.format( "Temporary ownership can only be granted to protected programs. %s access level is %s.", program.getUid(), program.getAccessLevel().name())); } + + if (!isOwnerInUserSearchScope(user, trackedEntity, program)) { + throw new ForbiddenException( + "The owner of the entity-program combination is not in the user's search scope."); + } + + if (config.isEnabled(CHANGELOG_TRACKER)) { + programTempOwnershipAuditService.addProgramTempOwnershipAudit( + new ProgramTempOwnershipAudit(program, trackedEntity, reason, user.getUsername())); + } + + ProgramTempOwner programTempOwner = + new ProgramTempOwner( + program, + trackedEntity, + reason, + userService.getUser(user.getUid()), + TEMPORARY_OWNERSHIP_VALIDITY_IN_HOURS); + programTempOwnerService.addProgramTempOwner(programTempOwner); + tempOwnerCache.invalidate( + getTempOwnershipCacheKey(trackedEntity.getUid(), program.getUid(), user.getUid())); } @Override 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 0f6ba531329c..f1d0c6880189 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 @@ -30,6 +30,7 @@ import static org.hisp.dhis.common.AccessLevel.AUDITED; import static org.hisp.dhis.common.AccessLevel.CLOSED; import static org.hisp.dhis.common.AccessLevel.OPEN; +import static org.hisp.dhis.common.AccessLevel.PROTECTED; import static org.hisp.dhis.common.OrganisationUnitSelectionMode.ACCESSIBLE; import static org.hisp.dhis.test.utils.Assertions.assertContains; import static org.hisp.dhis.test.utils.Assertions.assertContainsOnly; @@ -55,6 +56,7 @@ import org.hisp.dhis.program.Enrollment; import org.hisp.dhis.program.Program; import org.hisp.dhis.program.ProgramService; +import org.hisp.dhis.program.ProgramType; import org.hisp.dhis.security.Authorities; import org.hisp.dhis.security.acl.AccessStringHelper; import org.hisp.dhis.test.integration.PostgresIntegrationTestBase; @@ -144,7 +146,7 @@ void setUp() { createAndAddUserWithAuth("trackertestownership", organisationUnitA, Authorities.ALL); programA = createProgram('A'); - programA.setAccessLevel(AccessLevel.PROTECTED); + programA.setAccessLevel(PROTECTED); programA.setTrackedEntityType(trackedEntityType); programA.setOrganisationUnits(Set.of(organisationUnitA, organisationUnitB)); programService.addProgram(programA); @@ -367,6 +369,54 @@ void shouldFailWhenGrantingTemporaryOwnershipToProgramWithAccessLevelOtherThanPr "Temporary ownership can only be granted to protected programs.", exception.getMessage()); } + @Test + void shouldFailWhenGrantingTemporaryAccessIfUserIsSuperuser() { + Exception exception = + Assertions.assertThrows( + ForbiddenException.class, + () -> + trackerOwnershipAccessManager.grantTemporaryOwnership( + trackedEntityA1, + programA, + UserDetails.fromUser(superUser), + "test temporary ownership")); + + assertEquals( + "Temporary ownership not created. Either current user is a superuser, program supplied is does not exist or program supplied is not a tracker program.", + exception.getMessage()); + } + + @Test + void shouldFailWhenGrantingTemporaryAccessIfProgramIsNull() { + Exception exception = + Assertions.assertThrows( + ForbiddenException.class, + () -> + trackerOwnershipAccessManager.grantTemporaryOwnership( + trackedEntityA1, null, userDetailsB, "test temporary ownership")); + + assertEquals( + "Temporary ownership not created. Either current user is a superuser, program supplied is does not exist or program supplied is not a tracker program.", + exception.getMessage()); + } + + @Test + void shouldFailWhenGrantingTemporaryAccessIfProgramIsNotTrackerProgram() { + Program eventProgram = createProgram(PROTECTED); + eventProgram.setProgramType(ProgramType.WITHOUT_REGISTRATION); + + Exception exception = + Assertions.assertThrows( + ForbiddenException.class, + () -> + trackerOwnershipAccessManager.grantTemporaryOwnership( + trackedEntityA1, eventProgram, userDetailsB, "test temporary ownership")); + + assertEquals( + "Temporary ownership not created. Either current user is a superuser, program supplied is does not exist or program supplied is not a tracker program.", + exception.getMessage()); + } + @Test void shouldHaveAccessWhenProgramOpenAndUserInScope() throws ForbiddenException, NotFoundException, BadRequestException { From 97cd930840d6f39e2e9e2e6410b51da11c41102b Mon Sep 17 00:00:00 2001 From: Marc Date: Wed, 15 Jan 2025 17:05:55 +0100 Subject: [PATCH 6/7] fix: Skip getting TE enrollments [DHIS2-18784] --- .../tracker/ownership/TrackerOwnershipController.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/ownership/TrackerOwnershipController.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/ownership/TrackerOwnershipController.java index 840b0bcf4501..8fea04d2b93a 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/ownership/TrackerOwnershipController.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/tracker/ownership/TrackerOwnershipController.java @@ -99,7 +99,7 @@ public WebMessage updateTrackerProgramOwner( trackerOwnershipAccessManager.transferOwnership( trackedEntityService.getTrackedEntity( - trackedEntityUid, UID.of(program), TrackedEntityParams.TRUE), + trackedEntityUid, UID.of(program), TrackedEntityParams.FALSE), programService.getProgram(program), organisationUnitService.getOrganisationUnit(ou)); return ok("Ownership transferred"); From fb78c431049360acddb7f7bcd76b357244888b7c Mon Sep 17 00:00:00 2001 From: marc Date: Wed, 15 Jan 2025 18:33:19 +0100 Subject: [PATCH 7/7] Update dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerOwnershipManager.java Co-authored-by: Enrico Colasante --- .../hisp/dhis/tracker/acl/DefaultTrackerOwnershipManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerOwnershipManager.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerOwnershipManager.java index 0a1115a924fe..d63231eb9224 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerOwnershipManager.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerOwnershipManager.java @@ -154,7 +154,7 @@ public void grantTemporaryOwnership( throws ForbiddenException { if (canSkipOwnershipCheck(user, program)) { throw new ForbiddenException( - "Temporary ownership not created. Either current user is a superuser, program supplied is does not exist or program supplied is not a tracker program."); + "Temporary ownership not created. Either current user is a superuser, program supplied does not exist or program supplied is not a tracker program."); } if (!program.isProtected()) {