Skip to content

Commit

Permalink
fix: [DHIS2-18551] (2.40) Fix access check when registering TEI (#19384)
Browse files Browse the repository at this point in the history
  • Loading branch information
ameenhere authored Dec 4, 2024
1 parent e5823c0 commit 349803d
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ public interface TrackerAccessManager {

List<String> canWrite(User user, TrackedEntityInstance trackedEntityInstance);

List<String> canCreate(User user, TrackedEntityInstance trackedEntity);

List<String> canRead(
User user,
TrackedEntityInstance trackedEntityInstance,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,35 @@ public List<String> canWrite(User user, TrackedEntityInstance trackedEntity) {
return canWrite(user, trackedEntity, programService.getAllPrograms());
}

/**
* Check if the user can create the TE. For a regular user, they must have data write permissions
* to tracked entity type metadata as well as Capture Access to the Org Unit.
*
* @return No errors if a user has write access to the TrackedEntityType and to the OrgUnit
*/
@Override
@Transactional(readOnly = true)
public List<String> canCreate(User user, TrackedEntityInstance trackedEntity) {
if (user == null || user.isSuper() || trackedEntity == null) {
return List.of();
}
List<String> errors = new ArrayList<>();
if (!aclService.canDataWrite(user, trackedEntity.getTrackedEntityType())) {
errors.add(
"User has no data write access to tracked entity type: "
+ trackedEntity.getTrackedEntityType().getUid());
}

if (trackedEntity.getOrganisationUnit() != null
&& !organisationUnitService.isInUserHierarchy(user, trackedEntity.getOrganisationUnit())) {
errors.add(
"User has no write access to organisation unit: "
+ trackedEntity.getOrganisationUnit().getUid());
}

return errors;
}

private List<String> canWrite(
User user, TrackedEntityInstance trackedEntity, List<Program> programs) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,8 @@ private ImportSummary addTrackedEntityInstance(
return importSummary;
}

List<String> errors = trackerAccessManager.canWrite(importOptions.getUser(), daoEntityInstance);
List<String> errors =
trackerAccessManager.canCreate(importOptions.getUser(), daoEntityInstance);

if (!errors.isEmpty()) {
return new ImportSummary(ImportStatus.ERROR, errors.toString()).incrementIgnored();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@
import org.hisp.dhis.trackedentityattributevalue.TrackedEntityAttributeValueService;
import org.hisp.dhis.user.User;
import org.hisp.dhis.user.UserService;
import org.hisp.dhis.user.sharing.Sharing;
import org.joda.time.DateTime;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
Expand Down Expand Up @@ -593,16 +594,63 @@ void testUpdateTeiByDeletingExistingEventAndAddNewEventForSameProgramStage() {
}

@Test
void testSavePerson() {
void shouldPassWhenRegisteringPerson() {
trackedEntityType.setSharing(Sharing.builder().publicAccess("rwrw----").build());
regularUser.setOrganisationUnits(Set.of(organisationUnitA));
injectSecurityContext(regularUser);
TrackedEntityInstance trackedEntityInstance = new TrackedEntityInstance();
trackedEntityInstance.setTrackedEntityInstance(CodeGenerator.generateUid());
trackedEntityInstance.setOrgUnit(organisationUnitA.getUid());
trackedEntityInstance.setTrackedEntityType(trackedEntityType.getUid());

ImportSummary importSummary =
trackedEntityInstanceService.addTrackedEntityInstance(trackedEntityInstance, null);

assertEquals(ImportStatus.SUCCESS, importSummary.getStatus());
}

@Test
void shouldFailWhenRegisteringPersonOutsideCaptureScope() {
trackedEntityType.setSharing(Sharing.builder().publicAccess("rwrw----").build());
regularUser.setOrganisationUnits(Set.of(organisationUnitB));
injectSecurityContext(regularUser);
TrackedEntityInstance trackedEntityInstance = new TrackedEntityInstance();
trackedEntityInstance.setTrackedEntityInstance(CodeGenerator.generateUid());
trackedEntityInstance.setOrgUnit(organisationUnitA.getUid());
trackedEntityInstance.setTrackedEntityType(trackedEntityType.getUid());

ImportSummary importSummary =
trackedEntityInstanceService.addTrackedEntityInstance(trackedEntityInstance, null);

assertEquals(ImportStatus.ERROR, importSummary.getStatus());
assertEquals(1, importSummary.getImportCount().getIgnored());
assertEquals(
String.format(
"[User has no write access to organisation unit: %s]", organisationUnitA.getUid()),
importSummary.getDescription());
}

@Test
void shouldFailWhenRegisteringPersonWithoutTETypeAccess() {
regularUser.setOrganisationUnits(Set.of(organisationUnitB));
injectSecurityContext(regularUser);
TrackedEntityInstance trackedEntityInstance = new TrackedEntityInstance();
trackedEntityInstance.setTrackedEntityInstance(CodeGenerator.generateUid());
trackedEntityInstance.setOrgUnit(organisationUnitB.getUid());
trackedEntityInstance.setTrackedEntityType(trackedEntityType.getUid());

ImportSummary importSummary =
trackedEntityInstanceService.addTrackedEntityInstance(trackedEntityInstance, null);

assertEquals(ImportStatus.ERROR, importSummary.getStatus());
assertEquals(1, importSummary.getImportCount().getIgnored());
assertEquals(
String.format(
"[User has no data write access to tracked entity type: %s]",
trackedEntityType.getUid()),
importSummary.getDescription());
}

@Test
void testDeletePerson() {
trackedEntityInstanceService.deleteTrackedEntityInstance(maleA.getUid());
Expand Down

0 comments on commit 349803d

Please sign in to comment.