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 - 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..12f50fbf7687 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; @@ -53,7 +54,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; @@ -149,71 +149,54 @@ 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; + public void grantTemporaryOwnership( + @Nonnull TrackedEntity trackedEntity, Program program, UserDetails user, String reason) + throws ForbiddenException { + validateGrantTemporaryOwnershipInputs(trackedEntity, program, user); + + if (config.isEnabled(CHANGELOG_TRACKER)) { + programTempOwnershipAuditService.addProgramTempOwnershipAudit( + new ProgramTempOwnershipAudit(program, trackedEntity, reason, user.getUsername())); } - UserDetails currentUser = CurrentUserUtil.getCurrentUserDetails(); + 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())); + } - if (hasAccess(currentUser, trackedEntity, program) || skipAccessValidation) { - TrackedEntityProgramOwner teProgramOwner = - trackedEntityProgramOwnerService.getTrackedEntityProgramOwner(trackedEntity, program); + private void validateGrantTemporaryOwnershipInputs( + TrackedEntity trackedEntity, Program program, UserDetails user) throws ForbiddenException { + if (program == null) { + throw new ForbiddenException( + "Temporary ownership not created. Program supplied does not exist."); + } - 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); - } + if (user.isSuper()) { + throw new ForbiddenException("Temporary ownership not created. Current user is a superuser."); + } - 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"); + if (ProgramType.WITHOUT_REGISTRATION == program.getProgramType()) { + throw new ForbiddenException( + "Temporary ownership not created. Program supplied is not a tracker program."); } - } - @Override - @Transactional - public void grantTemporaryOwnership( - TrackedEntity trackedEntity, Program program, UserDetails user, String reason) { - if (canSkipOwnershipCheck(user, program) || trackedEntity == null) { - return; + 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 (program.isProtected()) { - 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())); + if (!isOwnerInUserSearchScope(user, trackedEntity, program)) { + throw new ForbiddenException( + "The owner of the entity-program combination is not in the user's search scope."); } } 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..6440cb149a0d 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,12 @@ */ 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.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; import static org.hisp.dhis.test.utils.Assertions.assertIsEmpty; import static org.hisp.dhis.tracker.TrackerTestUtils.uids; @@ -39,6 +44,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; @@ -50,9 +56,11 @@ 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; +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 +69,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 +94,8 @@ class TrackerOwnershipManagerTest extends PostgresIntegrationTestBase { @Autowired private TrackedEntityTypeService trackedEntityTypeService; + @Autowired private TrackedEntityProgramOwnerService trackedEntityProgramOwnerService; + private TrackedEntity trackedEntityA1; private TrackedEntity trackedEntityB1; @@ -133,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); @@ -162,25 +175,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 +195,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 +212,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 +226,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 +252,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 +279,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 +350,76 @@ void shouldHaveAccessWhenProgramClosedAndUserInCaptureScope() { programB)); } + private static Stream providePrograms() { + return Stream.of(createProgram(OPEN), createProgram(AUDITED), createProgram(CLOSED)); + } + + @ParameterizedTest + @MethodSource("providePrograms") + void shouldFailWhenGrantingTemporaryOwnershipToProgramWithAccessLevelOtherThanProtected( + Program program) { + Exception exception = + Assertions.assertThrows( + ForbiddenException.class, + () -> + trackerOwnershipAccessManager.grantTemporaryOwnership( + trackedEntityA1, program, userDetailsB, "test temporary ownership")); + + assertContains( + "Temporary ownership can only be granted to protected programs.", exception.getMessage()); + } + @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)); + void shouldFailWhenGrantingTemporaryAccessIfUserIsSuperuser() { + Exception exception = + Assertions.assertThrows( + ForbiddenException.class, + () -> + trackerOwnershipAccessManager.grantTemporaryOwnership( + trackedEntityA1, + programA, + UserDetails.fromUser(superUser), + "test temporary ownership")); - injectSecurityContextUser(userB); - ForbiddenException exception = - assertThrows( + assertEquals( + "Temporary ownership not created. Current user is a superuser.", exception.getMessage()); + } + + @Test + void shouldFailWhenGrantingTemporaryAccessIfProgramIsNull() { + 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, null, userDetailsB, "test temporary ownership")); + + assertEquals( + "Temporary ownership not created. Program supplied does not exist.", + 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. Program supplied is not a tracker program.", + exception.getMessage()); } @Test void shouldHaveAccessWhenProgramOpenAndUserInScope() throws ForbiddenException, NotFoundException, BadRequestException { - programA.setAccessLevel(AccessLevel.OPEN); + programA.setAccessLevel(OPEN); programService.updateProgram(programA); assertEquals( @@ -374,11 +430,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 +450,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 +503,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 +517,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 +533,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 +587,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 +600,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 +614,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 +626,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 6e99d46da1c8..c02670ca455e 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() @@ -1532,8 +1535,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); @@ -1555,8 +1558,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); @@ -1595,8 +1598,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); @@ -1635,8 +1638,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..db48f507bc2c 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 @@ -57,6 +57,8 @@ class TrackerOwnershipControllerTest extends PostgresControllerIntegrationTestBa private String pId; + private User regularUser; + @BeforeEach void setUp() { orgUnitAUid = @@ -74,11 +76,19 @@ void setUp() { OrganisationUnit orgUnitA = manager.get(OrganisationUnit.class, orgUnitAUid); OrganisationUnit orgUnitB = manager.get(OrganisationUnit.class, orgUnitBUid); - User user = - createAndAddUser(true, "user", Set.of(orgUnitA, orgUnitB), Set.of(orgUnitA, orgUnitB)); - injectSecurityContextUser(user); - - String tetId = assertStatus(HttpStatus.CREATED, POST("/trackedEntityTypes/", "{'name': 'A'}")); + regularUser = + createAndAddUser( + false, "regular-user", Set.of(orgUnitA, orgUnitB), Set.of(orgUnitA, orgUnitB)); + User superuser = + createAndAddUser(true, "superuser", Set.of(orgUnitA, orgUnitB), Set.of(orgUnitA, orgUnitB)); + injectSecurityContextUser(superuser); + + String tetId = + assertStatus( + HttpStatus.CREATED, + POST( + "/trackedEntityTypes/", + "{'name': 'A', 'sharing':{'external':false,'public':'rwrw----'}}")); teUid = CodeGenerator.generateUid(); assertStatus( @@ -107,11 +117,14 @@ void setUp() { { 'name':'P1', 'shortName':'P1', - 'programType':'WITHOUT_REGISTRATION', - 'organisationUnits': [{'id':'%s'},{'id':'%s'}] + 'programType':'WITH_REGISTRATION', + 'accessLevel':'PROTECTED', + 'trackedEntityType': {'id': '%s'}, + 'organisationUnits': [{'id':'%s'},{'id':'%s'}], + 'sharing':{'external':false,'public':'rwrw----'} } """ - .formatted(orgUnitAUid, orgUnitBUid))); + .formatted(tetId, orgUnitAUid, orgUnitBUid))); } @Test @@ -170,7 +183,8 @@ void shouldFailToUpdateWhenNoTrackedEntityOrTrackedEntityInstanceParametersArePr } @Test - void shouldOverrideOwnershipAccessWhenUsingDeprecateTrackedEntityInstanceParam() { + void shouldGrantTemporaryAccessWhenUsingDeprecateTrackedEntityInstanceParam() { + injectSecurityContextUser(regularUser); assertWebMessage( "OK", 200, @@ -184,7 +198,8 @@ void shouldOverrideOwnershipAccessWhenUsingDeprecateTrackedEntityInstanceParam() } @Test - void shouldOverrideOwnershipAccess() { + void shouldGrantTemporaryAccess() { + injectSecurityContextUser(regularUser); 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..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 @@ -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.FALSE), 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,