From 6318f2912f64f0cc5582725cd59e2b7d9c4adaa9 Mon Sep 17 00:00:00 2001 From: Marc Date: Thu, 12 Dec 2024 13:21:47 +0100 Subject: [PATCH 1/3] fix: Validate search scope when scheduled event created [DHIS2-17335] --- .../event/SecurityOwnershipValidator.java | 14 ++-- .../event/SecurityOwnershipValidatorTest.java | 36 ++++++++++ .../EventSecurityImportValidationTest.java | 34 ++++++++-- .../events-scheduled-with-registration.json | 68 +++++++++++++++++++ 4 files changed, 142 insertions(+), 10 deletions(-) create mode 100644 dhis-2/dhis-test-integration/src/test/resources/tracker/validations/events-scheduled-with-registration.json diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecurityOwnershipValidator.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecurityOwnershipValidator.java index 505ad94c96ba..c39ef77be89c 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecurityOwnershipValidator.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecurityOwnershipValidator.java @@ -66,9 +66,6 @@ @Slf4j class SecurityOwnershipValidator implements Validator { - private static final String ORG_UNIT_NO_USER_ASSIGNED = - "Event {} has no organisation unit assigned, so we skip user validation"; - @Nonnull private final AclService aclService; @Nonnull private final TrackerOwnershipManager ownershipAccessManager; @@ -93,10 +90,15 @@ public void validate( organisationUnit = bundle.getPreheat().getOrganisationUnit(event.getOrgUnit()); } - // If event is newly created, or going to be deleted, capture scope - // has to be checked if (program.isWithoutRegistration() || strategy.isCreate() || strategy.isDelete()) { - checkOrgUnitInCaptureScope(reporter, event, organisationUnit, bundle.getUser()); + checkEventOrgUnitWriteAccess( + reporter, + event, + organisationUnit, + strategy.isCreate() + ? event.isCreatableInSearchScope() + : preheatEvent.isCreatableInSearchScope(), + bundle.getUser()); } UID teUid = getTeUidFromEvent(bundle, event, program); diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecurityOwnershipValidatorTest.java b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecurityOwnershipValidatorTest.java index 552ecdc2797d..9366679f2154 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecurityOwnershipValidatorTest.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecurityOwnershipValidatorTest.java @@ -485,6 +485,42 @@ void verifySuccessEventValidationWhenEventHasNoOrgUnitAssigned() { assertIsEmpty(reporter.getErrors()); } + @Test + void shouldSucceedWhenScheduledEventIsCreatedFromSearchOrgUnit() { + UID enrollmentUid = UID.generate(); + org.hisp.dhis.tracker.imports.domain.Event event = + org.hisp.dhis.tracker.imports.domain.Event.builder() + .event(UID.generate()) + .enrollment(enrollmentUid) + .orgUnit(MetadataIdentifier.ofUid(ORG_UNIT_ID)) + .programStage(MetadataIdentifier.ofUid(PS_ID)) + .program(MetadataIdentifier.ofUid(PROGRAM_ID)) + .status(EventStatus.SCHEDULE) + .build(); + + when(bundle.getPreheat()).thenReturn(preheat); + when(bundle.getStrategy(event)).thenReturn(TrackerImportStrategy.CREATE); + Enrollment enrollment = getEnrollment(enrollmentUid); + + Event preheatEvent = getEvent(); + preheatEvent.setEnrollment(enrollment); + preheatEvent.setOrganisationUnit(null); + + when(preheat.getEvent(event.getEvent())).thenReturn(preheatEvent); + when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_ID))).thenReturn(program); + when(preheat.getProgramStage(MetadataIdentifier.ofUid(PS_ID))).thenReturn(programStage); + when(preheat.getOrganisationUnit(MetadataIdentifier.ofUid(ORG_UNIT_ID))) + .thenReturn(organisationUnit); + + when(aclService.canDataRead(user, program.getTrackedEntityType())).thenReturn(true); + when(aclService.canDataRead(user, program)).thenReturn(true); + when(aclService.canDataWrite(user, programStage)).thenReturn(true); + + validator.validate(reporter, bundle, event); + + assertIsEmpty(reporter.getErrors()); + } + private TrackedEntity teWithNoEnrollments() { TrackedEntity trackedEntity = createTrackedEntity(organisationUnit); trackedEntity.setUid(TE_ID.getValue()); diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/EventSecurityImportValidationTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/EventSecurityImportValidationTest.java index 7785acafad44..76695043d1dd 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/EventSecurityImportValidationTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/imports/validation/EventSecurityImportValidationTest.java @@ -27,6 +27,7 @@ */ package org.hisp.dhis.tracker.imports.validation; +import static org.hisp.dhis.tracker.Assertions.assertHasError; import static org.hisp.dhis.tracker.Assertions.assertHasOnlyErrors; import static org.hisp.dhis.tracker.Assertions.assertNoErrors; import static org.hisp.dhis.tracker.imports.validation.Users.USER_3; @@ -37,6 +38,7 @@ import java.util.Calendar; import java.util.Date; import java.util.HashSet; +import java.util.Set; import org.hisp.dhis.common.IdentifiableObjectManager; import org.hisp.dhis.common.ValueType; import org.hisp.dhis.dataelement.DataElement; @@ -207,15 +209,11 @@ void setUp() throws IOException { trackedEntityProgramOwnerService.updateTrackedEntityProgramOwner( maleA, programA, organisationUnitA); manager.update(programA); - User user = userService.getUser(USER_5); OrganisationUnit qfUVllTs6cS = organisationUnitService.getOrganisationUnit("QfUVllTs6cS"); - user.addOrganisationUnit(qfUVllTs6cS); - user.addOrganisationUnit(organisationUnitA); importUser.addOrganisationUnit(organisationUnitA); Program p = programService.getProgram("prabcdefghA"); p.addOrganisationUnit(qfUVllTs6cS); programService.updateProgram(p); - manager.update(user); manager.update(importUser); } @@ -269,4 +267,32 @@ void testNoUncompleteEventAuth() throws IOException { importReport = trackerImportService.importTracker(params, trackerObjects); assertHasOnlyErrors(importReport, ValidationCode.E1083); } + + @Test + void shouldSucceedWhenCreatingScheduledEventFromInsideSearchOrgUnit() throws IOException { + TrackerObjects trackerObjects = + fromJson("tracker/validations/events-scheduled-with-registration.json"); + TrackerImportParams params = TrackerImportParams.builder().build(); + params.setImportStrategy(TrackerImportStrategy.CREATE); + OrganisationUnit orgUnit = organisationUnitService.getOrganisationUnit("QfUVllTs6cS"); + User user = userService.getUser(USER_5); + user.setTeiSearchOrganisationUnits(Set.of(orgUnit)); + manager.update(user); + injectSecurityContextUser(user); + ImportReport importReport = trackerImportService.importTracker(params, trackerObjects); + + assertNoErrors(importReport); + } + + @Test + void shouldFailWhenCreatingScheduledEventFromOutsideSearchOrgUnit() throws IOException { + TrackerObjects trackerObjects = + fromJson("tracker/validations/events-scheduled-with-registration.json"); + TrackerImportParams params = TrackerImportParams.builder().build(); + params.setImportStrategy(TrackerImportStrategy.CREATE); + injectSecurityContextUser(userService.getUser(USER_5)); + ImportReport importReport = trackerImportService.importTracker(params, trackerObjects); + + assertHasError(importReport, ValidationCode.E1000); + } } diff --git a/dhis-2/dhis-test-integration/src/test/resources/tracker/validations/events-scheduled-with-registration.json b/dhis-2/dhis-test-integration/src/test/resources/tracker/validations/events-scheduled-with-registration.json new file mode 100644 index 000000000000..640943e95cca --- /dev/null +++ b/dhis-2/dhis-test-integration/src/test/resources/tracker/validations/events-scheduled-with-registration.json @@ -0,0 +1,68 @@ +{ + "importMode": "COMMIT", + "idSchemes": { + "dataElementIdScheme": { + "idScheme": "UID" + }, + "orgUnitIdScheme": { + "idScheme": "UID" + }, + "programIdScheme": { + "idScheme": "UID" + }, + "programStageIdScheme": { + "idScheme": "UID" + }, + "idScheme": { + "idScheme": "UID" + }, + "categoryOptionComboIdScheme": { + "idScheme": "UID" + }, + "categoryOptionIdScheme": { + "idScheme": "UID" + } + }, + "importStrategy": "CREATE", + "atomicMode": "ALL", + "flushMode": "AUTO", + "validationMode": "FULL", + "skipPatternValidation": false, + "skipSideEffects": false, + "skipRuleEngine": false, + "trackedEntities": [], + "enrollments": [], + "events": [ + { + "event": "ZwwuwNp6gVd", + "status": "SCHEDULE", + "program": { + "idScheme": "UID", + "identifier": "E8o1E9tAppy" + }, + "programStage": { + "idScheme": "UID", + "identifier": "Qmqxq907VNz" + }, + "enrollment": "MNWZ6hnuhSw", + "orgUnit": { + "idScheme": "UID", + "identifier": "QfUVllTs6cS" + }, + "orgUnitName": "TA org_unit lvl2", + "relationships": [], + "scheduledAt": "2019-08-19T13:59:13.688", + "storedBy": "admin", + "deleted": false, + "attributeOptionCombo": { + "idScheme": "UID", + "identifier": "HllvX50cXC0" + }, + "attributeCategoryOptions": [], + "dataValues": [], + "notes": [] + } + ], + "relationships": [], + "username": "system-process" +} \ No newline at end of file From c44e58871848d9139bf0ae113d56a4ac33da5e52 Mon Sep 17 00:00:00 2001 From: Marc Date: Thu, 12 Dec 2024 13:37:39 +0100 Subject: [PATCH 2/3] fix: Remove unused method & test [DHIS2-17335] --- .../event/SecurityOwnershipValidator.java | 7 ---- .../event/SecurityOwnershipValidatorTest.java | 36 ------------------- 2 files changed, 43 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecurityOwnershipValidator.java b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecurityOwnershipValidator.java index c39ef77be89c..69e94d859633 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecurityOwnershipValidator.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecurityOwnershipValidator.java @@ -230,13 +230,6 @@ public boolean needsToRun(TrackerImportStrategy strategy) { return true; } - private void checkOrgUnitInCaptureScope( - Reporter reporter, TrackerDto dto, OrganisationUnit orgUnit, UserDetails user) { - if (!user.isInUserHierarchy(orgUnit.getPath())) { - reporter.addError(dto, ValidationCode.E1000, user, orgUnit); - } - } - private void checkTeTypeAndTeProgramAccess( Reporter reporter, TrackerDto dto, diff --git a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecurityOwnershipValidatorTest.java b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecurityOwnershipValidatorTest.java index 9366679f2154..552ecdc2797d 100644 --- a/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecurityOwnershipValidatorTest.java +++ b/dhis-2/dhis-services/dhis-service-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecurityOwnershipValidatorTest.java @@ -485,42 +485,6 @@ void verifySuccessEventValidationWhenEventHasNoOrgUnitAssigned() { assertIsEmpty(reporter.getErrors()); } - @Test - void shouldSucceedWhenScheduledEventIsCreatedFromSearchOrgUnit() { - UID enrollmentUid = UID.generate(); - org.hisp.dhis.tracker.imports.domain.Event event = - org.hisp.dhis.tracker.imports.domain.Event.builder() - .event(UID.generate()) - .enrollment(enrollmentUid) - .orgUnit(MetadataIdentifier.ofUid(ORG_UNIT_ID)) - .programStage(MetadataIdentifier.ofUid(PS_ID)) - .program(MetadataIdentifier.ofUid(PROGRAM_ID)) - .status(EventStatus.SCHEDULE) - .build(); - - when(bundle.getPreheat()).thenReturn(preheat); - when(bundle.getStrategy(event)).thenReturn(TrackerImportStrategy.CREATE); - Enrollment enrollment = getEnrollment(enrollmentUid); - - Event preheatEvent = getEvent(); - preheatEvent.setEnrollment(enrollment); - preheatEvent.setOrganisationUnit(null); - - when(preheat.getEvent(event.getEvent())).thenReturn(preheatEvent); - when(preheat.getProgram(MetadataIdentifier.ofUid(PROGRAM_ID))).thenReturn(program); - when(preheat.getProgramStage(MetadataIdentifier.ofUid(PS_ID))).thenReturn(programStage); - when(preheat.getOrganisationUnit(MetadataIdentifier.ofUid(ORG_UNIT_ID))) - .thenReturn(organisationUnit); - - when(aclService.canDataRead(user, program.getTrackedEntityType())).thenReturn(true); - when(aclService.canDataRead(user, program)).thenReturn(true); - when(aclService.canDataWrite(user, programStage)).thenReturn(true); - - validator.validate(reporter, bundle, event); - - assertIsEmpty(reporter.getErrors()); - } - private TrackedEntity teWithNoEnrollments() { TrackedEntity trackedEntity = createTrackedEntity(organisationUnit); trackedEntity.setUid(TE_ID.getValue()); From 5bcd17adf671beb9351d018498acf94630eb5192 Mon Sep 17 00:00:00 2001 From: Marc Date: Thu, 12 Dec 2024 15:09:43 +0100 Subject: [PATCH 3/3] fix: Add PR suggestions [DHIS2-17335] --- .../events-scheduled-with-registration.json | 45 ++----------------- 1 file changed, 3 insertions(+), 42 deletions(-) diff --git a/dhis-2/dhis-test-integration/src/test/resources/tracker/validations/events-scheduled-with-registration.json b/dhis-2/dhis-test-integration/src/test/resources/tracker/validations/events-scheduled-with-registration.json index 640943e95cca..a4baaa2cc558 100644 --- a/dhis-2/dhis-test-integration/src/test/resources/tracker/validations/events-scheduled-with-registration.json +++ b/dhis-2/dhis-test-integration/src/test/resources/tracker/validations/events-scheduled-with-registration.json @@ -1,38 +1,5 @@ { - "importMode": "COMMIT", - "idSchemes": { - "dataElementIdScheme": { - "idScheme": "UID" - }, - "orgUnitIdScheme": { - "idScheme": "UID" - }, - "programIdScheme": { - "idScheme": "UID" - }, - "programStageIdScheme": { - "idScheme": "UID" - }, - "idScheme": { - "idScheme": "UID" - }, - "categoryOptionComboIdScheme": { - "idScheme": "UID" - }, - "categoryOptionIdScheme": { - "idScheme": "UID" - } - }, - "importStrategy": "CREATE", - "atomicMode": "ALL", - "flushMode": "AUTO", - "validationMode": "FULL", - "skipPatternValidation": false, - "skipSideEffects": false, - "skipRuleEngine": false, - "trackedEntities": [], - "enrollments": [], - "events": [ + "events": [ { "event": "ZwwuwNp6gVd", "status": "SCHEDULE", @@ -50,19 +17,13 @@ "identifier": "QfUVllTs6cS" }, "orgUnitName": "TA org_unit lvl2", - "relationships": [], "scheduledAt": "2019-08-19T13:59:13.688", "storedBy": "admin", "deleted": false, "attributeOptionCombo": { "idScheme": "UID", "identifier": "HllvX50cXC0" - }, - "attributeCategoryOptions": [], - "dataValues": [], - "notes": [] + } } - ], - "relationships": [], - "username": "system-process" + ] } \ No newline at end of file