Skip to content

Commit

Permalink
fix: Harmonize user in tracker exporters [DHIS2-18001] (#18690)
Browse files Browse the repository at this point in the history 
* fix: Harmonize user in tracker exporters [DHIS2-18001]

* Fix tests

* Fix code review comments

* Fix code review comments

* Fix code review comments
  • Loading branch information
@enricocolasante
enricocolasante authored Sep 30, 2024
1 parent 14ee84c commit f775e1a
Show file tree
Hide file tree
Showing 35 changed files with 742 additions and 1,002 deletions.
9 changes: 4 additions & 5 deletions dhis-2/dhis-api/src/main/java/org/hisp/dhis/common/AssignedUserQueryParam.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@
*/
package org.hisp.dhis.common;

import static org.hisp.dhis.user.CurrentUserUtil.getCurrentUserDetails;

import java.util.Collections;
import java.util.Set;
import lombok.Value;
Expand All @@ -41,7 +39,7 @@
public class AssignedUserQueryParam {

public static final AssignedUserQueryParam ALL =
new AssignedUserQueryParam(AssignedUserSelectionMode.ALL, Collections.emptySet());
new AssignedUserQueryParam(AssignedUserSelectionMode.ALL, Collections.emptySet(), null);

private final AssignedUserSelectionMode mode;

Expand All @@ -53,7 +51,8 @@ public class AssignedUserQueryParam {
* @param mode assigned user mode
* @param assignedUsers assigned user uids
*/
public AssignedUserQueryParam(AssignedUserSelectionMode mode, Set<String> assignedUsers) {
public AssignedUserQueryParam(
AssignedUserSelectionMode mode, Set<String> assignedUsers, String userUid) {
if (mode == AssignedUserSelectionMode.PROVIDED
&& (assignedUsers == null || assignedUsers.isEmpty())) {
throw new IllegalQueryException(
Expand All @@ -70,7 +69,7 @@ public AssignedUserQueryParam(AssignedUserSelectionMode mode, Set<String> assign

if (mode == AssignedUserSelectionMode.CURRENT) {
this.mode = AssignedUserSelectionMode.PROVIDED;
this.assignedUsers = Collections.singleton(getCurrentUserDetails().getUid());
this.assignedUsers = Collections.singleton(userUid);
} else if ((mode == null || mode == AssignedUserSelectionMode.PROVIDED)
&& (assignedUsers != null && !assignedUsers.isEmpty())) {
this.mode = AssignedUserSelectionMode.PROVIDED;
Expand Down
39 changes: 12 additions & 27 deletions dhis-2/dhis-api/src/test/java/org/hisp/dhis/common/AssignedUserQueryParamTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,10 @@

import java.util.Collection;
import java.util.Set;
import org.hisp.dhis.user.User;
import org.hisp.dhis.user.UserDetails;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.EnumSource;
import org.junit.jupiter.params.provider.NullAndEmptySource;
import org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContext;
import org.springframework.security.core.context.SecurityContextHolder;

class AssignedUserQueryParamTest {
public static final String CURRENT_USER_UID = "Kj6vYde4LHh";
Expand All @@ -59,7 +53,8 @@ class AssignedUserQueryParamTest {
@Test
void testUserWithAssignedUsersGivenUsersAndModeProvided() {

AssignedUserQueryParam param = new AssignedUserQueryParam(PROVIDED, NON_CURRENT_USER_UIDS);
AssignedUserQueryParam param =
new AssignedUserQueryParam(PROVIDED, NON_CURRENT_USER_UIDS, CURRENT_USER_UID);

assertEquals(PROVIDED, param.getMode());
assertEquals(NON_CURRENT_USER_UIDS, param.getAssignedUsers());
Expand All @@ -69,7 +64,8 @@ void testUserWithAssignedUsersGivenUsersAndModeProvided() {
@Test
void testUserWithAssignedUsersGivenUsersAndNoMode() {

AssignedUserQueryParam param = new AssignedUserQueryParam(null, NON_CURRENT_USER_UIDS);
AssignedUserQueryParam param =
new AssignedUserQueryParam(null, NON_CURRENT_USER_UIDS, CURRENT_USER_UID);

assertEquals(PROVIDED, param.getMode());
assertEquals(NON_CURRENT_USER_UIDS, param.getAssignedUsers());
Expand All @@ -80,7 +76,7 @@ void testUserWithAssignedUsersGivenUsersAndNoMode() {
@NullAndEmptySource
void testUserWithAssignedUsersGivenNoModeAndNoUsers(Set<String> users) {

AssignedUserQueryParam param = new AssignedUserQueryParam(null, users);
AssignedUserQueryParam param = new AssignedUserQueryParam(null, users, CURRENT_USER_UID);

assertEquals(ALL, param.getMode());
assertIsEmpty(param.getAssignedUsers());
Expand All @@ -91,18 +87,15 @@ void testUserWithAssignedUsersGivenNoModeAndNoUsers(Set<String> users) {
@NullAndEmptySource
void testUserWithAssignedUsersFailsGivenNoUsersAndProvided(Set<String> users) {

assertThrows(IllegalQueryException.class, () -> new AssignedUserQueryParam(PROVIDED, users));
assertThrows(
IllegalQueryException.class,
() -> new AssignedUserQueryParam(PROVIDED, users, CURRENT_USER_UID));
}

@ParameterizedTest
@NullAndEmptySource
void testUserWithAssignedUsersGivenCurrentUserAndModeCurrentAndUsersNull(Set<String> users) {
User user = new User();
user.setUid(CURRENT_USER_UID);

injectSecurityContext(UserDetails.fromUser(user));

AssignedUserQueryParam param = new AssignedUserQueryParam(CURRENT, users);
AssignedUserQueryParam param = new AssignedUserQueryParam(CURRENT, users, CURRENT_USER_UID);

assertEquals(PROVIDED, param.getMode());
assertEquals(Set.of(CURRENT_USER_UID), param.getAssignedUsers());
Expand All @@ -118,7 +111,8 @@ void testUserWithAssignedUsersFailsGivenUsersAndModeOtherThanProvided(
AssignedUserSelectionMode mode) {

assertThrows(
IllegalQueryException.class, () -> new AssignedUserQueryParam(mode, NON_CURRENT_USER_UIDS));
IllegalQueryException.class,
() -> new AssignedUserQueryParam(mode, NON_CURRENT_USER_UIDS, CURRENT_USER_UID));
}

@ParameterizedTest
Expand All @@ -128,7 +122,7 @@ void testUserWithAssignedUsersFailsGivenUsersAndModeOtherThanProvided(
names = {"PROVIDED", "CURRENT"})
void testUserWithAssignedUsersGivenNullUsersAndModeOtherThanProvided(
AssignedUserSelectionMode mode) {
AssignedUserQueryParam param = new AssignedUserQueryParam(mode, null);
AssignedUserQueryParam param = new AssignedUserQueryParam(mode, null, CURRENT_USER_UID);

assertEquals(mode, param.getMode());
assertIsEmpty(param.getAssignedUsers());
Expand All @@ -139,13 +133,4 @@ private static void assertIsEmpty(Collection<?> actual) {
assertNotNull(actual);
assertTrue(actual.isEmpty(), actual.toString());
}

private void injectSecurityContext(UserDetails currentUserDetails) {
Authentication authentication =
new UsernamePasswordAuthenticationToken(
currentUserDetails, "", currentUserDetails.getAuthorities());
SecurityContext context = SecurityContextHolder.createEmptyContext();
context.setAuthentication(authentication);
SecurityContextHolder.setContext(context);
}
}
62 changes: 30 additions & 32 deletions ...service-tracker/src/main/java/org/hisp/dhis/tracker/export/OperationsParamsValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@

import static org.hisp.dhis.changelog.ChangeLogType.READ;
import static org.hisp.dhis.security.Authorities.F_TRACKED_ENTITY_INSTANCE_SEARCH_IN_ALL_ORGUNITS;
import static org.hisp.dhis.user.CurrentUserUtil.getCurrentUserDetails;
import static org.hisp.dhis.user.CurrentUserUtil.getCurrentUsername;

import java.util.HashSet;
import java.util.Set;
Expand All @@ -48,8 +46,7 @@
import org.hisp.dhis.trackedentity.TrackedEntityType;
import org.hisp.dhis.trackedentity.TrackedEntityTypeService;
import org.hisp.dhis.tracker.deprecated.audit.TrackedEntityAuditService;
import org.hisp.dhis.user.CurrentUserUtil;
import org.hisp.dhis.user.User;
import org.hisp.dhis.user.UserDetails;
import org.springframework.stereotype.Component;

@Component
Expand Down Expand Up @@ -78,38 +75,40 @@ public class OperationsParamsValidator {
* @throws BadRequestException if a validation error occurs for any of the three aforementioned
* modes
*/
public static void validateOrgUnitMode(OrganisationUnitSelectionMode orgUnitMode, Program program)
public static void validateOrgUnitMode(
OrganisationUnitSelectionMode orgUnitMode, Program program, UserDetails user)
throws BadRequestException {
switch (orgUnitMode) {
case ALL -> validateUserCanSearchOrgUnitModeALL();
case SELECTED, ACCESSIBLE, DESCENDANTS, CHILDREN -> validateUserScope(program);
case CAPTURE -> validateCaptureScope();
case ALL -> validateUserCanSearchOrgUnitModeALL(user);
case SELECTED, ACCESSIBLE, DESCENDANTS, CHILDREN -> validateUserScope(program, user);
case CAPTURE -> validateCaptureScope(user);
}
}

private static void validateUserCanSearchOrgUnitModeALL() throws BadRequestException {
if (!CurrentUserUtil.getCurrentUserDetails()
.isAuthorized(F_TRACKED_ENTITY_INSTANCE_SEARCH_IN_ALL_ORGUNITS)) {
private static void validateUserCanSearchOrgUnitModeALL(UserDetails user)
throws BadRequestException {
if (!user.isAuthorized(F_TRACKED_ENTITY_INSTANCE_SEARCH_IN_ALL_ORGUNITS)) {
throw new BadRequestException(
"Current user is not authorized to query across all organisation units");
"User is not authorized to query across all organisation units");
}
}

private static void validateUserScope(Program program) throws BadRequestException {
private static void validateUserScope(Program program, UserDetails user)
throws BadRequestException {

if (program != null && (program.isClosed() || program.isProtected())) {
if (getCurrentUserDetails().getUserOrgUnitIds().isEmpty()) {
if (user.getUserOrgUnitIds().isEmpty()) {
throw new BadRequestException("User needs to be assigned data capture org units");
}

} else if (getCurrentUserDetails().getUserEffectiveSearchOrgUnitIds().isEmpty()) {
} else if (user.getUserEffectiveSearchOrgUnitIds().isEmpty()) {
throw new BadRequestException(
"User needs to be assigned either search or data capture org units");
}
}

private static void validateCaptureScope() throws BadRequestException {
if (getCurrentUserDetails().getUserOrgUnitIds().isEmpty()) {
private static void validateCaptureScope(UserDetails user) throws BadRequestException {
if (user.getUserOrgUnitIds().isEmpty()) {
throw new BadRequestException("User needs to be assigned data capture org units");
}
}
Expand All @@ -122,9 +121,9 @@ private static void validateCaptureScope() throws BadRequestException {
* @throws ForbiddenException if the user has no data read access to the program or its tracked
* entity type
*/
public Program validateTrackerProgram(String programUid)
public Program validateTrackerProgram(String programUid, UserDetails user)
throws BadRequestException, ForbiddenException {
Program program = validateProgramAccess(programUid);
Program program = validateProgramAccess(programUid, user);

if (program == null) {
return null;
Expand All @@ -135,9 +134,9 @@ public Program validateTrackerProgram(String programUid)
}

if (program.getTrackedEntityType() != null
&& !aclService.canDataRead(getCurrentUserDetails(), program.getTrackedEntityType())) {
&& !aclService.canDataRead(user, program.getTrackedEntityType())) {
throw new ForbiddenException(
"Current user is not authorized to read data from selected program's tracked entity type: "
"User is not authorized to read data from selected program's tracked entity type: "
+ program.getTrackedEntityType().getUid());
}

Expand All @@ -152,7 +151,7 @@ public Program validateTrackerProgram(String programUid)
* @throws BadRequestException if the program uid does not exist
* @throws ForbiddenException if the user has no data read access to the program
*/
public Program validateProgramAccess(String programUid)
public Program validateProgramAccess(String programUid, UserDetails user)
throws BadRequestException, ForbiddenException {
if (programUid == null) {
return null;
Expand All @@ -163,7 +162,7 @@ public Program validateProgramAccess(String programUid)
throw new BadRequestException("Program is specified but does not exist: " + programUid);
}

if (!aclService.canDataRead(getCurrentUserDetails(), program)) {
if (!aclService.canDataRead(user, program)) {
throw new ForbiddenException("User has no access to program: " + program.getUid());
}

Expand All @@ -177,7 +176,7 @@ public Program validateProgramAccess(String programUid)
* @throws BadRequestException if the tracked entity uid does not exist
* @throws ForbiddenException if the user has no data read access to type of the tracked entity
*/
public TrackedEntity validateTrackedEntity(String trackedEntityUid, User user)
public TrackedEntity validateTrackedEntity(String trackedEntityUid, UserDetails user)
throws BadRequestException, ForbiddenException {
if (trackedEntityUid == null) {
return null;
Expand All @@ -189,12 +188,12 @@ public TrackedEntity validateTrackedEntity(String trackedEntityUid, User user)
throw new BadRequestException(
"Tracked entity is specified but does not exist: " + trackedEntityUid);
}
trackedEntityAuditService.addTrackedEntityAudit(trackedEntity, getCurrentUsername(), READ);
trackedEntityAuditService.addTrackedEntityAudit(trackedEntity, user.getUsername(), READ);

if (trackedEntity.getTrackedEntityType() != null
&& !aclService.canDataRead(user, trackedEntity.getTrackedEntityType())) {
throw new ForbiddenException(
"Current user is not authorized to read data from type of selected tracked entity: "
"User is not authorized to read data from type of selected tracked entity: "
+ trackedEntity.getTrackedEntityType().getUid());
}

Expand All @@ -208,7 +207,7 @@ public TrackedEntity validateTrackedEntity(String trackedEntityUid, User user)
* @throws BadRequestException if the tracked entity type uid does not exist
* @throws ForbiddenException if the user has no data read access to the tracked entity type
*/
public TrackedEntityType validateTrackedEntityType(String uid)
public TrackedEntityType validateTrackedEntityType(String uid, UserDetails user)
throws BadRequestException, ForbiddenException {
if (uid == null) {
return null;
Expand All @@ -219,9 +218,9 @@ public TrackedEntityType validateTrackedEntityType(String uid)
throw new BadRequestException("Tracked entity type is specified but does not exist: " + uid);
}

if (!aclService.canDataRead(getCurrentUserDetails(), trackedEntityType)) {
if (!aclService.canDataRead(user, trackedEntityType)) {
throw new ForbiddenException(
"Current user is not authorized to read data from selected tracked entity type: "
"User is not authorized to read data from selected tracked entity type: "
+ trackedEntityType.getUid());
}

Expand All @@ -235,7 +234,7 @@ public TrackedEntityType validateTrackedEntityType(String uid)
* @throws BadRequestException if the org unit uid does not exist
* @throws ForbiddenException if the org unit is not part of the user scope
*/
public Set<OrganisationUnit> validateOrgUnits(Set<String> orgUnitIds)
public Set<OrganisationUnit> validateOrgUnits(Set<String> orgUnitIds, UserDetails user)
throws BadRequestException, ForbiddenException {
Set<OrganisationUnit> orgUnits = new HashSet<>();
for (String orgUnitUid : orgUnitIds) {
Expand All @@ -244,8 +243,7 @@ public Set<OrganisationUnit> validateOrgUnits(Set<String> orgUnitIds)
throw new BadRequestException("Organisation unit does not exist: " + orgUnitUid);
}

if (!getCurrentUserDetails().isSuper()
&& !getCurrentUserDetails().isInUserEffectiveSearchOrgUnitHierarchy(orgUnit.getPath())) {
if (!user.isSuper() && !user.isInUserEffectiveSearchOrgUnitHierarchy(orgUnit.getPath())) {
throw new ForbiddenException(
"Organisation unit is not part of the search scope: " + orgUnit.getUid());
}
Expand Down
Loading

0 comments on commit f775e1a

Please sign in to comment.