Skip to content

Commit

Permalink
fix: Return accessible enrollments instead of exception [TECH-1589] (#…
Browse files Browse the repository at this point in the history 
…15979)

* fix: Return accessible enrollments instead of exception [TECH-1589]

* fix: Return all accessible enrollments [TECH-1589]

* fix: Fail with forbidden exception when no access to program [TECH-1589]

* fix: Make test names more explicit [TECH-1589]
  • Loading branch information
@muilpp
muilpp authored Dec 20, 2023
1 parent 533f996 commit 67baea9
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 88 deletions.
25 changes: 12 additions & 13 deletions ...acker/src/main/java/org/hisp/dhis/tracker/export/enrollment/DefaultEnrollmentService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,23 +84,21 @@ public Enrollment getEnrollment(String uid, EnrollmentParams params, boolean inc
throw new NotFoundException(Enrollment.class, uid);
}

return getEnrollment(enrollment, params, includeDeleted, null);
}

@Override
public Enrollment getEnrollment(
@Nonnull Enrollment enrollment,
EnrollmentParams params,
boolean includeDeleted,
OrganisationUnitSelectionMode orgUnitMode)
throws ForbiddenException {
User user = currentUserService.getCurrentUser();
List<String> errors = trackerAccessManager.canRead(user, enrollment, orgUnitMode == ALL);
List<String> errors = trackerAccessManager.canRead(user, enrollment, false);

if (!errors.isEmpty()) {
throw new ForbiddenException(errors.toString());
}

return getEnrollment(enrollment, params, includeDeleted, user);
}

@Override
public Enrollment getEnrollment(
@Nonnull Enrollment enrollment, EnrollmentParams params, boolean includeDeleted, User user)
throws ForbiddenException {

Enrollment result = new Enrollment();
result.setId(enrollment.getId());
result.setUid(enrollment.getUid());
Expand Down Expand Up @@ -228,8 +226,9 @@ private List<Enrollment> getEnrollments(
if (enrollment != null
&& (orgUnitMode == ALL
|| trackerOwnershipAccessManager.hasAccess(
user, enrollment.getTrackedEntity(), enrollment.getProgram()))) {
enrollmentList.add(getEnrollment(enrollment, params, includeDeleted, orgUnitMode));
user, enrollment.getTrackedEntity(), enrollment.getProgram()))
&& trackerAccessManager.canRead(user, enrollment, orgUnitMode == ALL).isEmpty()) {
enrollmentList.add(getEnrollment(enrollment, params, includeDeleted, user));
}
}

Expand Down
25 changes: 17 additions & 8 deletions ...rc/main/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentOperationParamsMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import java.util.HashSet;
import java.util.Set;
import lombok.RequiredArgsConstructor;
import org.hisp.dhis.common.IllegalQueryException;
import org.hisp.dhis.feedback.BadRequestException;
import org.hisp.dhis.feedback.ForbiddenException;
import org.hisp.dhis.organisationunit.OrganisationUnit;
Expand Down Expand Up @@ -79,7 +78,8 @@ public EnrollmentQueryParams map(EnrollmentOperationParams operationParams)
Program program = validateProgram(operationParams.getProgramUid(), user);
TrackedEntityType trackedEntityType =
validateTrackedEntityType(operationParams.getTrackedEntityTypeUid(), user);
TrackedEntity trackedEntity = validateTrackedEntity(operationParams.getTrackedEntityUid());
TrackedEntity trackedEntity =
validateTrackedEntity(operationParams.getTrackedEntityUid(), user);

Set<OrganisationUnit> orgUnits = validateOrgUnits(operationParams.getOrgUnitUids(), user);
validateOrgUnitMode(operationParams.getOrgUnitMode(), user, program);
Expand Down Expand Up @@ -122,7 +122,8 @@ private void mergeOrgUnitModes(
}
}

private Program validateProgram(String uid, User user) throws BadRequestException {
private Program validateProgram(String uid, User user)
throws BadRequestException, ForbiddenException {
if (uid == null) {
return null;
}
Expand All @@ -133,14 +134,14 @@ private Program validateProgram(String uid, User user) throws BadRequestExceptio
}

if (!aclService.canDataRead(user, program)) {
throw new IllegalQueryException(
throw new ForbiddenException(
"Current user is not authorized to read data from selected program: "
+ program.getUid());
}

if (program.getTrackedEntityType() != null
&& !aclService.canDataRead(user, program.getTrackedEntityType())) {
throw new IllegalQueryException(
throw new ForbiddenException(
"Current user is not authorized to read data from selected program's tracked entity type: "
+ program.getTrackedEntityType().getUid());
}
Expand All @@ -149,7 +150,7 @@ private Program validateProgram(String uid, User user) throws BadRequestExceptio
}

private TrackedEntityType validateTrackedEntityType(String uid, User user)
throws BadRequestException {
throws BadRequestException, ForbiddenException {
if (uid == null) {
return null;
}
Expand All @@ -160,15 +161,16 @@ private TrackedEntityType validateTrackedEntityType(String uid, User user)
}

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

return trackedEntityType;
}

private TrackedEntity validateTrackedEntity(String uid) throws BadRequestException {
private TrackedEntity validateTrackedEntity(String uid, User user)
throws BadRequestException, ForbiddenException {
if (uid == null) {
return null;
}
Expand All @@ -178,6 +180,13 @@ private TrackedEntity validateTrackedEntity(String uid) throws BadRequestExcepti
throw new BadRequestException("Tracked entity is specified but does not exist: " + uid);
}

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: "
+ trackedEntity.getTrackedEntityType().getUid());
}

return trackedEntity;
}

Expand Down
7 changes: 2 additions & 5 deletions ...vice-tracker/src/main/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,23 +29,20 @@

import java.util.List;
import java.util.Set;
import org.hisp.dhis.common.OrganisationUnitSelectionMode;
import org.hisp.dhis.feedback.BadRequestException;
import org.hisp.dhis.feedback.ForbiddenException;
import org.hisp.dhis.feedback.NotFoundException;
import org.hisp.dhis.program.Enrollment;
import org.hisp.dhis.tracker.export.Page;
import org.hisp.dhis.tracker.export.PageParams;
import org.hisp.dhis.user.User;

public interface EnrollmentService {
Enrollment getEnrollment(String uid, EnrollmentParams params, boolean includeDeleted)
throws NotFoundException, ForbiddenException;

Enrollment getEnrollment(
Enrollment enrollment,
EnrollmentParams params,
boolean includeDeleted,
OrganisationUnitSelectionMode orgUnitMode)
Enrollment enrollment, EnrollmentParams params, boolean includeDeleted, User user)
throws ForbiddenException;

/** Get all enrollments matching given params. */
Expand Down
43 changes: 29 additions & 14 deletions ...est/java/org/hisp/dhis/tracker/export/enrollment/EnrollmentOperationParamsMapperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
import java.util.List;
import java.util.Set;
import org.hisp.dhis.common.BaseIdentifiableObject;
import org.hisp.dhis.common.IllegalQueryException;
import org.hisp.dhis.feedback.BadRequestException;
import org.hisp.dhis.feedback.ForbiddenException;
import org.hisp.dhis.organisationunit.OrganisationUnit;
Expand Down Expand Up @@ -142,6 +141,7 @@ void setUp() {

trackedEntity = new TrackedEntity();
trackedEntity.setUid(TRACKED_ENTITY_UID);
trackedEntity.setTrackedEntityType(trackedEntityType);
when(trackedEntityService.getTrackedEntity(TRACKED_ENTITY_UID)).thenReturn(trackedEntity);
}

Expand Down Expand Up @@ -185,7 +185,7 @@ void shouldMapOrgUnitsWhenOrgUnitUidsAreSpecified()
}

@Test
void shouldThrowExceptionWhenOrgUnitNotFound() {
void shouldThrowBadRequestExceptionWhenOrgUnitNotFound() {
EnrollmentOperationParams operationParams =
EnrollmentOperationParams.builder()
.orgUnitUids(Set.of("JW6BrFd0HLu"))
Expand All @@ -203,7 +203,7 @@ void shouldThrowExceptionWhenOrgUnitNotFound() {
}

@Test
void shouldThrowExceptionWhenOrgUnitNotInScope() {
void shouldThrowForbiddenExceptionWhenOrgUnitNotInScope() {
EnrollmentOperationParams operationParams =
EnrollmentOperationParams.builder().orgUnitUids(Set.of(ORG_UNIT_1_UID)).build();
when(organisationUnitService.isInUserHierarchy(
Expand Down Expand Up @@ -239,7 +239,7 @@ void shouldMapParamsWhenOrgUnitNotInScopeButUserIsSuperuser()
}

@Test
void shouldFailWhenOrgUnitNotInScopeAndUserHasSearchInAllAuthority() {
void shouldThrowForbiddenExceptionWhenOrgUnitNotInScopeAndUserHasSearchInAllAuthority() {

User user = createUser(F_TRACKED_ENTITY_INSTANCE_SEARCH_IN_ALL_ORGUNITS.name());
user.setTeiSearchOrganisationUnits(Set.of(orgUnit2));
Expand Down Expand Up @@ -283,7 +283,7 @@ void shouldMapProgramWhenProgramUidIsSpecified() throws BadRequestException, For
}

@Test
void shouldThrowExceptionWhenProgramNotFound() {
void shouldThrowBadRequestExceptionWhenProgramNotFound() {
EnrollmentOperationParams operationParams =
EnrollmentOperationParams.builder().programUid("JW6BrFd0HLu").build();

Expand All @@ -293,32 +293,30 @@ void shouldThrowExceptionWhenProgramNotFound() {
}

@Test
void shouldFailWhenUserCantReadProgramData() {
void shouldThrowForbiddenExceptionWhenUserCantReadProgramData() {
when(programService.getProgram(PROGRAM_UID)).thenReturn(program);
when(aclService.canDataRead(user, program)).thenReturn(false);

EnrollmentOperationParams operationParams =
EnrollmentOperationParams.builder().programUid(PROGRAM_UID).build();

Exception exception =
assertThrows(IllegalQueryException.class, () -> mapper.map(operationParams));
Exception exception = assertThrows(ForbiddenException.class, () -> mapper.map(operationParams));
assertEquals(
String.format(
"Current user is not authorized to read data from selected program: %s", PROGRAM_UID),
exception.getMessage());
}

@Test
void shouldFailWhenUserCantReadProgramTrackedEntityTypeData() {
void shouldThrowForbiddenExceptionWhenUserCantReadProgramTrackedEntityTypeData() {
when(programService.getProgram(PROGRAM_UID)).thenReturn(program);
when(aclService.canDataRead(user, program)).thenReturn(true);
when(aclService.canDataRead(user, program.getTrackedEntityType())).thenReturn(false);

EnrollmentOperationParams operationParams =
EnrollmentOperationParams.builder().programUid(PROGRAM_UID).build();

Exception exception =
assertThrows(IllegalQueryException.class, () -> mapper.map(operationParams));
Exception exception = assertThrows(ForbiddenException.class, () -> mapper.map(operationParams));
assertEquals(
String.format(
"Current user is not authorized to read data from selected program's tracked entity type: %s",
Expand All @@ -342,7 +340,7 @@ void shouldMapTrackedEntityTypeWhenTrackedEntityTypeUidIsSpecified()
}

@Test
void shouldThrowExceptionWhenTrackedEntityTypeNotFound() {
void shouldThrowBadRequestExceptionWhenTrackedEntityTypeNotFound() {
EnrollmentOperationParams requestParams =
EnrollmentOperationParams.builder().trackedEntityTypeUid("JW6BrFd0HLu").build();

Expand All @@ -352,7 +350,7 @@ void shouldThrowExceptionWhenTrackedEntityTypeNotFound() {
}

@Test
void shouldFailWhenUserCantReadTrackedEntityTypeData() {
void shouldThrowBadRequestExceptionWhenUserCantReadTrackedEntityTypeData() {
when(trackedEntityTypeService.getTrackedEntityType(TRACKED_ENTITY_TYPE_UID))
.thenReturn(trackedEntityType);
when(aclService.canDataRead(user, trackedEntityType)).thenReturn(false);
Expand All @@ -376,13 +374,15 @@ void shouldMapTrackedEntityWhenTrackedEntityUidIsSpecified()
.orgUnitMode(ACCESSIBLE)
.build();

when(aclService.canDataRead(user, trackedEntity.getTrackedEntityType())).thenReturn(true);

EnrollmentQueryParams params = mapper.map(operationParams);

assertEquals(trackedEntity, params.getTrackedEntity());
}

@Test
void shouldThrowExceptionTrackedEntityNotFound() {
void shouldThrowBadRequestExceptionTrackedEntityNotFound() {
EnrollmentOperationParams operationParams =
EnrollmentOperationParams.builder().trackedEntityUid("JW6BrFd0HLu").build();

Expand All @@ -392,6 +392,21 @@ void shouldThrowExceptionTrackedEntityNotFound() {
"Tracked entity is specified but does not exist: JW6BrFd0HLu", exception.getMessage());
}

@Test
void shouldThrowForbiddenExceptionWhenTypeOfTrackedEntityNotAccessible() {
EnrollmentOperationParams operationParams =
EnrollmentOperationParams.builder().trackedEntityUid(TRACKED_ENTITY_UID).build();

when(aclService.canDataRead(user, trackedEntity.getTrackedEntityType())).thenReturn(false);

Exception exception = assertThrows(ForbiddenException.class, () -> mapper.map(operationParams));
assertEquals(
String.format(
"Current user is not authorized to read data from type of selected tracked entity: %s",
trackedEntity.getTrackedEntityType().getUid()),
exception.getMessage());
}

@Test
void shouldMapOrderInGivenOrder() throws BadRequestException, ForbiddenException {
EnrollmentOperationParams operationParams =
Expand Down
Loading

0 comments on commit 67baea9

Please sign in to comment.