Skip to content

Commit

Permalink
fix: Grant temp access only if user in search scope [DHIS2-18784] [2.…
Browse files Browse the repository at this point in the history 
…41] (#19710)
  • Loading branch information
@muilpp
muilpp authored Jan 20, 2025
1 parent 9e3b7f3 commit a90d7b2
Show file tree
Hide file tree
Showing 5 changed files with 183 additions and 64 deletions.
3 changes: 2 additions & 1 deletion dhis-2/dhis-api/src/main/java/org/hisp/dhis/trackedentity/TrackerOwnershipManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ boolean hasAccess(
* @param reason The reason for requesting temporary ownership
*/
void grantTemporaryOwnership(
TrackedEntity entityInstance, Program program, User user, String reason);
TrackedEntity entityInstance, Program program, User user, String reason)
throws ForbiddenException;

/**
* Ownership check can be skipped if the user is super user or if the program type is without
Expand Down
56 changes: 42 additions & 14 deletions ...ervice-core/src/main/java/org/hisp/dhis/trackedentity/DefaultTrackerOwnershipManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,22 +234,50 @@ public void assignOwnership(
@Override
@Transactional
public void grantTemporaryOwnership(
TrackedEntity entityInstance, Program program, User user, String reason) {
if (canSkipOwnershipCheck(user, program) || entityInstance == null) {
return;
TrackedEntity entityInstance, Program program, User user, String reason)
throws ForbiddenException {

validateGrantTemporaryOwnershipInputs(entityInstance, program, user);

if (config.isEnabled(CHANGELOG_TRACKER)) {
programTempOwnershipAuditService.addProgramTempOwnershipAudit(
new ProgramTempOwnershipAudit(program, entityInstance, reason, user.getUsername()));
}

if (program.isProtected()) {
if (config.isEnabled(CHANGELOG_TRACKER)) {
programTempOwnershipAuditService.addProgramTempOwnershipAudit(
new ProgramTempOwnershipAudit(program, entityInstance, reason, user.getUsername()));
}
ProgramTempOwner programTempOwner =
new ProgramTempOwner(
program, entityInstance, reason, user, TEMPORARY_OWNERSHIP_VALIDITY_IN_HOURS);
programTempOwnerService.addProgramTempOwner(programTempOwner);
tempOwnerCache.invalidate(
getTempOwnershipCacheKey(entityInstance.getUid(), program.getUid(), user.getUid()));
ProgramTempOwner programTempOwner =
new ProgramTempOwner(
program, entityInstance, reason, user, TEMPORARY_OWNERSHIP_VALIDITY_IN_HOURS);
programTempOwnerService.addProgramTempOwner(programTempOwner);
tempOwnerCache.invalidate(
getTempOwnershipCacheKey(entityInstance.getUid(), program.getUid(), user.getUid()));
}

private void validateGrantTemporaryOwnershipInputs(
TrackedEntity entityInstance, Program program, User user) throws ForbiddenException {
if (program == null) {
throw new ForbiddenException(
"Temporary ownership not created. Program supplied does not exist.");
}

if (user.isSuper()) {
throw new ForbiddenException("Temporary ownership not created. Current user is a superuser.");
}

if (ProgramType.WITHOUT_REGISTRATION == program.getProgramType()) {
throw new ForbiddenException(
"Temporary ownership not created. Program supplied is not a tracker program.");
}

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, entityInstance, program)) {
throw new ForbiddenException(
"The owner of the entity-program combination is not in the user's search scope.");
}
}

Expand Down
122 changes: 90 additions & 32 deletions ...st-integration/src/test/java/org/hisp/dhis/trackedentity/TrackerOwnershipManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +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.OrganisationUnitSelectionMode.ACCESSIBLE;
import static org.hisp.dhis.tracker.TrackerTestUtils.uids;
import static org.hisp.dhis.utils.Assertions.assertContains;
import static org.hisp.dhis.utils.Assertions.assertContainsOnly;
import static org.hisp.dhis.utils.Assertions.assertIsEmpty;
import static org.junit.jupiter.api.Assertions.assertEquals;
Expand All @@ -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.feedback.BadRequestException;
import org.hisp.dhis.feedback.ForbiddenException;
Expand All @@ -48,6 +53,7 @@
import org.hisp.dhis.program.EnrollmentService;
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.IntegrationTestBase;
Expand All @@ -61,6 +67,8 @@
import org.hisp.dhis.user.sharing.UserAccess;
import org.hisp.dhis.utils.Assertions;
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;

/**
Expand Down Expand Up @@ -148,7 +156,7 @@ protected void setUpTest() throws Exception {
programA.setSharing(Sharing.builder().publicAccess(AccessStringHelper.FULL).build());
programService.updateProgram(programA);
programB = createProgram('B');
programB.setAccessLevel(AccessLevel.CLOSED);
programB.setAccessLevel(CLOSED);
programB.setTrackedEntityType(trackedEntityType);
programService.addProgram(programB);
programB.setSharing(
Expand All @@ -168,25 +176,20 @@ protected void setUpTest() throws Exception {
}

@Test
void testAssignOwnership() {
void shouldFailWhenGrantingTemporaryOwnershipAndUserNotInSearchScope() {
assertTrue(trackerOwnershipAccessManager.hasAccess(userA, entityInstanceA1, programA));
assertFalse(trackerOwnershipAccessManager.hasAccess(userB, entityInstanceA1, programA));
assertTrue(trackerOwnershipAccessManager.hasAccess(userB, entityInstanceB1, programA));
trackerOwnershipAccessManager.assignOwnership(
entityInstanceA1, programA, organisationUnitB, false, true);
assertFalse(trackerOwnershipAccessManager.hasAccess(userA, entityInstanceA1, programA));
assertTrue(trackerOwnershipAccessManager.hasAccess(userB, entityInstanceA1, programA));
}

@Test
void testGrantTemporaryOwnershipWithAudit() {
assertTrue(trackerOwnershipAccessManager.hasAccess(userA, entityInstanceA1, programA));
assertFalse(trackerOwnershipAccessManager.hasAccess(userB, entityInstanceA1, programA));
trackerOwnershipAccessManager.grantTemporaryOwnership(
entityInstanceA1, programA, userB, "testing reason");
assertTrue(trackerOwnershipAccessManager.hasAccess(userA, entityInstanceA1, programA));
assertTrue(trackerOwnershipAccessManager.hasAccess(userA, entityInstanceA1, programA));
assertTrue(trackerOwnershipAccessManager.hasAccess(userB, entityInstanceA1, programA));
Exception exception =
assertThrows(
ForbiddenException.class,
() ->
trackerOwnershipAccessManager.grantTemporaryOwnership(
entityInstanceA1, programA, userB, "testing reason"));

assertEquals(
"The owner of the entity-program combination is not in the user's search scope.",
exception.getMessage());
}

@Test
Expand Down Expand Up @@ -279,9 +282,13 @@ void shouldHaveAccessWhenProgramProtectedAndUserInCaptureScope() {
}

@Test
void shouldHaveAccessWhenProgramProtectedAndHasTemporaryAccess() {
void shouldHaveAccessWhenProgramProtectedAndHasTemporaryAccess() throws ForbiddenException {
userB.setTeiSearchOrganisationUnits(Set.of(organisationUnitA));
userService.updateUser(userB);

trackerOwnershipAccessManager.grantTemporaryOwnership(
entityInstanceA1, programA, userB, "test protected program");

assertTrue(trackerOwnershipAccessManager.hasAccess(userB, entityInstanceA1, programA));
assertTrue(
trackerOwnershipAccessManager.hasAccess(
Expand Down Expand Up @@ -333,29 +340,73 @@ void shouldHaveAccessWhenProgramClosedAndUserInCaptureScope() {
userB, entityInstanceB1.getUid(), entityInstanceB1.getOrganisationUnit(), programB));
}

private static Stream<Program> providePrograms() {
return Stream.of(createProgram(OPEN), createProgram(AUDITED), createProgram(CLOSED));
}

@ParameterizedTest
@MethodSource("providePrograms")
void shouldFailWhenGrantingTemporaryOwnershipToProgramWithAccessLevelOtherThanProtected(
Program program) {
Exception exception =
assertThrows(
ForbiddenException.class,
() ->
trackerOwnershipAccessManager.grantTemporaryOwnership(
entityInstanceA1, program, userB, "test temporary ownership"));

assertContains(
"Temporary ownership can only be granted to protected programs.", exception.getMessage());
}

@Test
void shouldNotHaveAccessWhenProgramClosedAndUserHasTemporaryAccess() {
trackerOwnershipAccessManager.grantTemporaryOwnership(
entityInstanceA1, programB, userB, "test closed program");
assertFalse(trackerOwnershipAccessManager.hasAccess(userB, entityInstanceA1, programB));
assertFalse(
trackerOwnershipAccessManager.hasAccess(
userB, entityInstanceA1.getUid(), entityInstanceA1.getOrganisationUnit(), programB));
void shouldFailWhenGrantingTemporaryAccessIfUserIsSuperuser() {
Exception exception =
assertThrows(
ForbiddenException.class,
() ->
trackerOwnershipAccessManager.grantTemporaryOwnership(
entityInstanceA1, programA, superUser, "test temporary ownership"));

injectSecurityContextUser(userB);
ForbiddenException exception =
assertEquals(
"Temporary ownership not created. Current user is a superuser.", exception.getMessage());
}

@Test
void shouldFailWhenGrantingTemporaryAccessIfProgramIsNull() {
Exception exception =
assertThrows(
ForbiddenException.class,
() ->
trackedEntityService.getTrackedEntity(
entityInstanceA1.getUid(), programB.getUid(), defaultParams, false));
assertEquals(TrackerOwnershipManager.PROGRAM_ACCESS_CLOSED, exception.getMessage());
trackerOwnershipAccessManager.grantTemporaryOwnership(
entityInstanceA1, null, userB, "test temporary ownership"));

assertEquals(
"Temporary ownership not created. Program supplied does not exist.",
exception.getMessage());
}

@Test
void shouldFailWhenGrantingTemporaryAccessIfProgramIsNotTrackerProgram() {
Program eventProgram = createProgram(AccessLevel.PROTECTED);
eventProgram.setProgramType(ProgramType.WITHOUT_REGISTRATION);

Exception exception =
assertThrows(
ForbiddenException.class,
() ->
trackerOwnershipAccessManager.grantTemporaryOwnership(
entityInstanceA1, eventProgram, userB, "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(
Expand All @@ -366,7 +417,7 @@ void shouldHaveAccessWhenProgramOpenAndUserInScope()

@Test
void shouldNotHaveAccessWhenProgramOpenAndUserNotInSearchScope() throws ForbiddenException {
programA.setAccessLevel(AccessLevel.OPEN);
programA.setAccessLevel(OPEN);
programService.updateProgram(programA);
trackerOwnershipAccessManager.transferOwnership(
entityInstanceA1, programA, organisationUnitB, true, true);
Expand Down Expand Up @@ -557,4 +608,11 @@ private List<String> 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;
}
}
Loading

0 comments on commit a90d7b2

Please sign in to comment.