Skip to content

Commit 15be80c

Browse files
muilppmuilpp
authored
chore: Validate AOC on updates and deletes [DHIS2-20158] (#23824)
* chore: Validate AOC on updates and deletes [DHIS2-20158] * chore: Validate AOC on updates and deletes [DHIS2-20158] * chore: Validate AOC on updates and deletes [DHIS2-20158]
1 parent d24bbc8 commit 15be80c

9 files changed

Lines changed: 471 additions & 36 deletions

File tree

‎dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/acl/TrackerAccessManagerTest.java‎

Lines changed: 172 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,10 @@ class TrackerAccessManagerTest extends PostgresIntegrationTestBase {
126126

127127
private TrackerEvent eventB;
128128

129+
private TrackerEvent trackerEvent;
130+
131+
private org.hisp.dhis.tracker.model.Enrollment enrollmentForAoc;
132+
129133
private SingleEvent singleEvent;
130134

131135
private CategoryOption oldCatOption;
@@ -243,6 +247,48 @@ void setUp() {
243247
newCatOption.getCategoryOptionCombos().add(newAoc);
244248
categoryService.updateCategoryOption(newCatOption);
245249

250+
trackedEntityType.setPublicAccess(AccessStringHelper.FULL);
251+
manager.update(trackedEntityType);
252+
253+
ProgramStage trackerEventProgramStage = createProgramStage('M', 0);
254+
manager.save(trackerEventProgramStage);
255+
256+
Program trackerEventProgram = createProgram('M', new HashSet<>(), orgUnitA);
257+
trackerEventProgram.setProgramType(ProgramType.WITH_REGISTRATION);
258+
trackerEventProgram.setAccessLevel(AccessLevel.PROTECTED);
259+
trackerEventProgram.setTrackedEntityType(trackedEntityType);
260+
trackerEventProgramStage.setProgram(trackerEventProgram);
261+
trackerEventProgram.getProgramStages().add(trackerEventProgramStage);
262+
manager.save(trackerEventProgram);
263+
trackerEventProgram.setPublicAccess(AccessStringHelper.DATA_READ_WRITE);
264+
manager.update(trackerEventProgram);
265+
trackerEventProgramStage.setPublicAccess(AccessStringHelper.DATA_READ_WRITE);
266+
manager.update(trackerEventProgramStage);
267+
268+
TrackedEntity trackerEventTe = createTrackedEntity(orgUnitA, trackedEntityType);
269+
manager.save(trackerEventTe);
270+
271+
Enrollment trackerEventEnrollment =
272+
createEnrollment(trackerEventProgram, trackerEventTe, orgUnitA);
273+
manager.save(trackerEventEnrollment);
274+
trackerEventTe.getEnrollments().add(trackerEventEnrollment);
275+
manager.update(trackerEventTe);
276+
277+
trackedEntityProgramOwnerService.createTrackedEntityProgramOwner(
278+
trackerEventTe, trackerEventProgram, orgUnitA);
279+
280+
trackerEvent = new TrackerEvent();
281+
trackerEvent.setEnrollment(trackerEventEnrollment);
282+
trackerEvent.setProgramStage(trackerEventProgramStage);
283+
trackerEvent.setOrganisationUnit(orgUnitA);
284+
trackerEvent.setAttributeOptionCombo(oldAoc);
285+
trackerEvent.setOccurredDate(new Date());
286+
manager.save(trackerEvent, false);
287+
288+
enrollmentForAoc = createEnrollment(trackerEventProgram, trackerEventTe, orgUnitA);
289+
enrollmentForAoc.setAttributeOptionCombo(oldAoc);
290+
manager.save(enrollmentForAoc);
291+
246292
ProgramStage singleEventProgramStage = createProgramStage('C', 0);
247293
manager.save(singleEventProgramStage);
248294

@@ -357,7 +403,11 @@ void checkAccessPermissionForEnrollmentInClosedProgram()
357403
assertNoErrors(trackerAccessManager.canCreate(userDetails, enrollment));
358404
// Can update enrollment
359405
assertNoErrors(
360-
trackerAccessManager.canUpdate(userDetails, enrollment, enrollment.getOrganisationUnit()));
406+
trackerAccessManager.canUpdate(
407+
userDetails,
408+
enrollment,
409+
enrollment.getOrganisationUnit(),
410+
enrollment.getAttributeOptionCombo()));
361411
// Can delete enrollment
362412
assertNoErrors(trackerAccessManager.canDelete(userDetails, enrollment));
363413
// Can read enrollment
@@ -373,7 +423,11 @@ void checkAccessPermissionForEnrollmentInClosedProgram()
373423
assertHasErrorMessage(trackerAccessManager.canCreate(userDetails, enrollment), E1102);
374424
// Cannot update enrollment if not owner
375425
assertHasErrorMessage(
376-
trackerAccessManager.canUpdate(userDetails, enrollment, enrollment.getOrganisationUnit()),
426+
trackerAccessManager.canUpdate(
427+
userDetails,
428+
enrollment,
429+
enrollment.getOrganisationUnit(),
430+
enrollment.getAttributeOptionCombo()),
377431
E1102);
378432
// Cannot delete enrollment if not owner
379433
assertHasErrorMessage(trackerAccessManager.canDelete(userDetails, enrollment), E1102);
@@ -399,7 +453,11 @@ void checkAccessPermissionForEnrollmentInOpenProgram()
399453
assertHasError(trackerAccessManager.canCreate(userDetails, enrollment));
400454
// Can update enrollment if ownerOU falls inside search scope
401455
assertNoErrors(
402-
trackerAccessManager.canUpdate(userDetails, enrollment, enrollment.getOrganisationUnit()));
456+
trackerAccessManager.canUpdate(
457+
userDetails,
458+
enrollment,
459+
enrollment.getOrganisationUnit(),
460+
enrollment.getAttributeOptionCombo()));
403461
// Cannot delete enrollment if enrollmentOU fails outside capture scope
404462
assertHasError(trackerAccessManager.canDelete(userDetails, enrollment));
405463
// Can read enrollment if ownerOU falls inside search scope
@@ -412,7 +470,11 @@ void checkAccessPermissionForEnrollmentInOpenProgram()
412470
assertHasErrorMessage(trackerAccessManager.canCreate(userDetails, enrollment), E1000);
413471
// Can update enrollment
414472
assertNoErrors(
415-
trackerAccessManager.canUpdate(userDetails, enrollment, enrollment.getOrganisationUnit()));
473+
trackerAccessManager.canUpdate(
474+
userDetails,
475+
enrollment,
476+
enrollment.getOrganisationUnit(),
477+
enrollment.getAttributeOptionCombo()));
416478
// Cannot delete enrollment if enrollmentOU falls outside capture scope,
417479
// even if user is owner
418480
assertHasErrorMessage(trackerAccessManager.canDelete(userDetails, enrollment), E1000);
@@ -430,7 +492,11 @@ void checkAccessPermissionForEnrollmentInOpenProgram()
430492
assertHasErrorMessage(trackerAccessManager.canCreate(userDetails, enrollment), E1000);
431493
// Can update enrollment if ownerOU is in search scope
432494
assertNoErrors(
433-
trackerAccessManager.canUpdate(userDetails, enrollment, enrollment.getOrganisationUnit()));
495+
trackerAccessManager.canUpdate(
496+
userDetails,
497+
enrollment,
498+
enrollment.getOrganisationUnit(),
499+
enrollment.getAttributeOptionCombo()));
434500
// Cannot delete enrollment if enrollment OU is outside capture scope
435501
assertHasErrorMessage(trackerAccessManager.canDelete(userDetails, enrollment), E1000);
436502
// Can read enrollment if ownerOU is in search scope
@@ -465,7 +531,8 @@ void checkAccessPermissionsForEventInClosedProgram()
465531
assertNoErrors(trackerAccessManager.canRead(userDetails, eventA));
466532
// Can update events if owner org unit falls into users search scope
467533
assertNoErrors(
468-
trackerAccessManager.canUpdate(userDetails, eventA, eventA.getOrganisationUnit()));
534+
trackerAccessManager.canUpdate(
535+
userDetails, eventA, eventA.getOrganisationUnit(), eventA.getAttributeOptionCombo()));
469536
// Can delete events if event org unit and owner org unit in capture scope
470537
assertNoErrors(trackerAccessManager.canDelete(userDetails, eventA));
471538

@@ -482,7 +549,8 @@ void checkAccessPermissionsForEventInClosedProgram()
482549
assertNoErrors(trackerAccessManager.canRead(userDetails, eventB));
483550
// Can update events if user is owner irrespective of eventOU
484551
assertNoErrors(
485-
trackerAccessManager.canUpdate(userDetails, eventB, eventB.getOrganisationUnit()));
552+
trackerAccessManager.canUpdate(
553+
userDetails, eventB, eventB.getOrganisationUnit(), eventB.getAttributeOptionCombo()));
486554
// Cannot delete events outside capture scope even if user is owner
487555
assertHasErrorMessage(trackerAccessManager.canDelete(userDetails, eventB), E1000);
488556
trackerOwnershipManager.transferOwnership(trackedEntityA, programA.getUID(), orgUnitB.getUID());
@@ -492,7 +560,9 @@ void checkAccessPermissionsForEventInClosedProgram()
492560
assertHasErrorMessage(trackerAccessManager.canRead(userDetails, eventB), E1102);
493561
// Cannot update events if user is not owner (OwnerOU falls into capture scope)
494562
assertHasErrorMessage(
495-
trackerAccessManager.canUpdate(userDetails, eventB, eventB.getOrganisationUnit()), E1102);
563+
trackerAccessManager.canUpdate(
564+
userDetails, eventB, eventB.getOrganisationUnit(), eventB.getAttributeOptionCombo()),
565+
E1102);
496566
// Cannot delete events anywhere if user is not owner and event org unit not in capture scope
497567
assertHasErrors(2, trackerAccessManager.canDelete(userDetails, eventB));
498568
}
@@ -519,7 +589,8 @@ void checkAccessPermissionsForEventInOpenProgram()
519589
assertNoErrors(trackerAccessManager.canRead(userDetails, eventA));
520590
// Can update events if owner org unit falls into users search scope
521591
assertNoErrors(
522-
trackerAccessManager.canUpdate(userDetails, eventA, eventA.getOrganisationUnit()));
592+
trackerAccessManager.canUpdate(
593+
userDetails, eventA, eventA.getOrganisationUnit(), eventA.getAttributeOptionCombo()));
523594
// Can delete events if event org unit and owner org unit in capture scope
524595
assertNoErrors(trackerAccessManager.canDelete(userDetails, eventA));
525596

@@ -532,7 +603,8 @@ void checkAccessPermissionsForEventInOpenProgram()
532603
assertNoErrors(trackerAccessManager.canRead(userDetails, eventA));
533604
// Can update events if ownerOu falls into users search scope
534605
assertNoErrors(
535-
trackerAccessManager.canUpdate(userDetails, eventA, eventA.getOrganisationUnit()));
606+
trackerAccessManager.canUpdate(
607+
userDetails, eventA, eventA.getOrganisationUnit(), eventA.getAttributeOptionCombo()));
536608
// Cannot delete events with event ou outside capture scope
537609
assertHasErrorMessage(trackerAccessManager.canDelete(userDetails, eventA), E1000);
538610
trackerOwnershipManager.transferOwnership(trackedEntityA, programA.getUID(), orgUnitB.getUID());
@@ -544,7 +616,8 @@ void checkAccessPermissionsForEventInOpenProgram()
544616
assertNoErrors(trackerAccessManager.canRead(userDetails, eventA));
545617
// Can update events if ownerOu falls into users capture scope
546618
assertNoErrors(
547-
trackerAccessManager.canUpdate(userDetails, eventA, eventA.getOrganisationUnit()));
619+
trackerAccessManager.canUpdate(
620+
userDetails, eventA, eventA.getOrganisationUnit(), eventA.getAttributeOptionCombo()));
548621
// Cannot delete events with eventOu outside capture scope, even if ownerOu is also in capture
549622
// scope
550623
assertHasErrorMessage(trackerAccessManager.canDelete(userDetails, eventA), E1000);
@@ -561,7 +634,10 @@ void shouldFailToUpdateEnrollmentWhenUserLacksAccess() {
561634
assertEnrollmentCategoryOptionAccessFails(
562635
(userDetails, enrollment) ->
563636
trackerAccessManager.canUpdate(
564-
userDetails, enrollment, enrollment.getOrganisationUnit()));
637+
userDetails,
638+
enrollment,
639+
enrollment.getOrganisationUnit(),
640+
enrollment.getAttributeOptionCombo()));
565641
}
566642

567643
@Test
@@ -677,4 +753,88 @@ private void assertHasError(List<?> errors) {
677753
private void assertHasErrors(int errorNumber, List<?> errors) {
678754
assertEquals(errorNumber, errors.size());
679755
}
756+
757+
@Test
758+
void shouldPassWhenUpdatingEnrollmentWithAccessToBothOldAndNewAoc() {
759+
User user = createUserWithAuth("user1").setOrganisationUnits(Sets.newHashSet(orgUnitA));
760+
assertNoErrors(
761+
trackerAccessManager.canUpdate(fromUser(user), enrollmentForAoc, orgUnitA, newAoc));
762+
}
763+
764+
@Test
765+
void shouldFailWhenUpdatingEnrollmentWithAccessOnlyToNewAoc() {
766+
oldCatOption.getSharing().setPublicAccess(CATEGORY_NO_DATA_SHARING_DEFAULT);
767+
manager.update(oldCatOption);
768+
User user = createUserWithAuth("user1").setOrganisationUnits(Sets.newHashSet(orgUnitA));
769+
assertHasErrorMessage(
770+
trackerAccessManager.canUpdate(fromUser(user), enrollmentForAoc, orgUnitA, newAoc), E1099);
771+
}
772+
773+
@Test
774+
void shouldFailWhenUpdatingEnrollmentWithAccessOnlyToOldAoc() {
775+
newCatOption.getSharing().setPublicAccess(CATEGORY_NO_DATA_SHARING_DEFAULT);
776+
manager.update(newCatOption);
777+
User user = createUserWithAuth("user1").setOrganisationUnits(Sets.newHashSet(orgUnitA));
778+
assertHasErrorMessage(
779+
trackerAccessManager.canUpdate(fromUser(user), enrollmentForAoc, orgUnitA, newAoc), E1099);
780+
}
781+
782+
@Test
783+
void shouldPassWhenUpdatingEnrollmentWithNoAocChangeAndUserHasAccess() {
784+
User user = createUserWithAuth("user1").setOrganisationUnits(Sets.newHashSet(orgUnitA));
785+
assertNoErrors(
786+
trackerAccessManager.canUpdate(fromUser(user), enrollmentForAoc, orgUnitA, oldAoc));
787+
}
788+
789+
@Test
790+
void shouldFailWhenUpdatingEnrollmentWithNoAccessToEitherAoc() {
791+
oldCatOption.getSharing().setPublicAccess(CATEGORY_NO_DATA_SHARING_DEFAULT);
792+
manager.update(oldCatOption);
793+
newCatOption.getSharing().setPublicAccess(CATEGORY_NO_DATA_SHARING_DEFAULT);
794+
manager.update(newCatOption);
795+
User user = createUserWithAuth("user1").setOrganisationUnits(Sets.newHashSet(orgUnitA));
796+
assertHasErrors(
797+
2, trackerAccessManager.canUpdate(fromUser(user), enrollmentForAoc, orgUnitA, newAoc));
798+
}
799+
800+
@Test
801+
void shouldPassWhenUpdatingTrackerEventWithAccessToBothOldAndNewAoc() {
802+
User user = createUserWithAuth("user1").setOrganisationUnits(Sets.newHashSet(orgUnitA));
803+
assertNoErrors(trackerAccessManager.canUpdate(fromUser(user), trackerEvent, orgUnitA, newAoc));
804+
}
805+
806+
@Test
807+
void shouldFailWhenUpdatingTrackerEventWithAccessOnlyToNewAoc() {
808+
oldCatOption.getSharing().setPublicAccess(CATEGORY_NO_DATA_SHARING_DEFAULT);
809+
manager.update(oldCatOption);
810+
User user = createUserWithAuth("user1").setOrganisationUnits(Sets.newHashSet(orgUnitA));
811+
assertHasErrorMessage(
812+
trackerAccessManager.canUpdate(fromUser(user), trackerEvent, orgUnitA, newAoc), E1099);
813+
}
814+
815+
@Test
816+
void shouldFailWhenUpdatingTrackerEventWithAccessOnlyToOldAoc() {
817+
newCatOption.getSharing().setPublicAccess(CATEGORY_NO_DATA_SHARING_DEFAULT);
818+
manager.update(newCatOption);
819+
User user = createUserWithAuth("user1").setOrganisationUnits(Sets.newHashSet(orgUnitA));
820+
assertHasErrorMessage(
821+
trackerAccessManager.canUpdate(fromUser(user), trackerEvent, orgUnitA, newAoc), E1099);
822+
}
823+
824+
@Test
825+
void shouldPassWhenUpdatingTrackerEventWithNoAocChangeAndUserHasAccess() {
826+
User user = createUserWithAuth("user1").setOrganisationUnits(Sets.newHashSet(orgUnitA));
827+
assertNoErrors(trackerAccessManager.canUpdate(fromUser(user), trackerEvent, orgUnitA, oldAoc));
828+
}
829+
830+
@Test
831+
void shouldFailWhenUpdatingTrackerEventWithNoAccessToEitherAoc() {
832+
oldCatOption.getSharing().setPublicAccess(CATEGORY_NO_DATA_SHARING_DEFAULT);
833+
manager.update(oldCatOption);
834+
newCatOption.getSharing().setPublicAccess(CATEGORY_NO_DATA_SHARING_DEFAULT);
835+
manager.update(newCatOption);
836+
User user = createUserWithAuth("user1").setOrganisationUnits(Sets.newHashSet(orgUnitA));
837+
assertHasErrors(
838+
2, trackerAccessManager.canUpdate(fromUser(user), trackerEvent, orgUnitA, newAoc));
839+
}
680840
}

‎dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerAccessManager.java‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,8 @@ public List<ErrorMessage> canCreate(@Nonnull UserDetails user, @Nonnull Enrollme
195195
public List<ErrorMessage> canUpdate(
196196
@Nonnull UserDetails user,
197197
@Nonnull Enrollment enrollment,
198-
@Nonnull OrganisationUnit orgUnit) {
198+
@Nonnull OrganisationUnit orgUnit,
199+
@Nonnull CategoryOptionCombo categoryOptionCombo) {
199200
if (user.isSuper()) {
200201
return List.of();
201202
}
@@ -206,6 +207,10 @@ public List<ErrorMessage> canUpdate(
206207
checkOrgUnitInCaptureScope(errors, user, orgUnit);
207208
}
208209

210+
if (!categoryOptionCombo.getUid().equals(enrollment.getAttributeOptionCombo().getUid())) {
211+
checkDataWriteAccessToCategoryOptionCombo(errors, user, categoryOptionCombo);
212+
}
213+
209214
return errors;
210215
}
211216

@@ -272,7 +277,10 @@ public List<ErrorMessage> canCreate(@Nonnull UserDetails user, @Nonnull TrackerE
272277

273278
@Override
274279
public List<ErrorMessage> canUpdate(
275-
@Nonnull UserDetails user, @Nonnull TrackerEvent event, @Nonnull OrganisationUnit orgUnit) {
280+
@Nonnull UserDetails user,
281+
@Nonnull TrackerEvent event,
282+
@Nonnull OrganisationUnit orgUnit,
283+
@Nonnull CategoryOptionCombo attributeOptionCombo) {
276284
if (user.isSuper()) {
277285
return List.of();
278286
}
@@ -283,6 +291,10 @@ public List<ErrorMessage> canUpdate(
283291
checkOrgUnitInCaptureScope(errors, user, orgUnit);
284292
}
285293

294+
if (!attributeOptionCombo.getUid().equals(event.getAttributeOptionCombo().getUid())) {
295+
checkDataWriteAccessToCategoryOptionCombo(errors, user, attributeOptionCombo);
296+
}
297+
286298
return errors;
287299
}
288300

@@ -469,13 +481,19 @@ private List<String> canWrite(@Nonnull UserDetails user, RelationshipItem item)
469481
}
470482
if (item.getEnrollment() != null) {
471483
Enrollment enrollment = item.getEnrollment();
472-
return canUpdate(user, enrollment, enrollment.getOrganisationUnit()).stream()
484+
return canUpdate(
485+
user,
486+
enrollment,
487+
enrollment.getOrganisationUnit(),
488+
enrollment.getAttributeOptionCombo())
489+
.stream()
473490
.map(em -> em.validationCode().getMessage())
474491
.toList();
475492
}
476493
if (item.getTrackerEvent() != null) {
477494
TrackerEvent event = item.getTrackerEvent();
478-
return canUpdate(user, event, event.getOrganisationUnit()).stream()
495+
return canUpdate(user, event, event.getOrganisationUnit(), event.getAttributeOptionCombo())
496+
.stream()
479497
.map(em -> em.validationCode().getMessage())
480498
.toList();
481499
}

0 commit comments

Comments
 (0)