From 1df985e7368be7b01ccf73329403c6e2a613bdc8 Mon Sep 17 00:00:00 2001 From: Marc Date: Mon, 27 Apr 2026 16:24:23 +0200 Subject: [PATCH 1/5] chore: Refactor ACL validation of single events [DHIS2-20158] --- .../acl/DefaultTrackerAccessManager.java | 154 ++++----- .../tracker/acl/TrackerAccessManager.java | 21 +- .../event/CompletedSingleEventValidator.java | 67 ++++ ...va => CompletedTrackerEventValidator.java} | 2 +- .../validator/event/EventValidator.java | 3 +- .../event/SecuritySingleEventValidator.java | 121 +++---- .../acl/DefaultTrackerAccessManagerTest.java | 141 -------- .../CompletedSingleEventValidatorTest.java | 183 ++++++++++ ...> CompletedTrackerEventValidatorTest.java} | 6 +- .../SecuritySingleEventValidatorTest.java | 317 +++++------------- 10 files changed, 463 insertions(+), 552 deletions(-) create mode 100644 dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/CompletedSingleEventValidator.java rename dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/{CompletedEventValidator.java => CompletedTrackerEventValidator.java} (97%) delete mode 100644 dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/acl/DefaultTrackerAccessManagerTest.java create mode 100644 dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/CompletedSingleEventValidatorTest.java rename dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/{CompletedEventValidatorTest.java => CompletedTrackerEventValidatorTest.java} (97%) diff --git a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerAccessManager.java b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerAccessManager.java index c4b261d62f21..54bb6993c0ac 100644 --- a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerAccessManager.java +++ b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerAccessManager.java @@ -29,7 +29,6 @@ */ package org.hisp.dhis.tracker.acl; -import static org.hisp.dhis.tracker.acl.TrackerOwnershipManager.NO_READ_ACCESS_TO_ORG_UNIT; import static org.hisp.dhis.tracker.imports.validation.ValidationCode.E1000; import static org.hisp.dhis.tracker.imports.validation.ValidationCode.E1096; import static org.hisp.dhis.tracker.imports.validation.ValidationCode.E1097; @@ -46,7 +45,6 @@ import lombok.RequiredArgsConstructor; import org.hisp.dhis.category.CategoryOption; import org.hisp.dhis.category.CategoryOptionCombo; -import org.hisp.dhis.dataelement.DataElement; import org.hisp.dhis.organisationunit.OrganisationUnit; import org.hisp.dhis.program.Program; import org.hisp.dhis.program.ProgramStage; @@ -319,57 +317,66 @@ private List validateTrackerEventAccess( } @Override - public List canRead(@Nonnull UserDetails user, SingleEvent event) { - if (user.isSuper() || event == null) { + public List canRead(@Nonnull UserDetails user, @Nonnull SingleEvent event) { + if (user.isSuper()) { return List.of(); } - ProgramStage programStage = event.getProgramStage(); + ProgramStage programStage = event.getProgramStage(); Program program = programStage.getProgram(); - List errors = new ArrayList<>(); - if (!aclService.canDataRead(user, program)) { - errors.add("User has no data read access to program: " + program.getUid()); - } - OrganisationUnit ou = event.getOrganisationUnit(); - if (!canAccess(user, program, ou)) { - errors.add(NO_READ_ACCESS_TO_ORG_UNIT + ": " + ou.getUid()); - } - errors.addAll(canRead(user, event.getAttributeOptionCombo())); + + List errors = new ArrayList<>(); + checkOrgUnitInScope( + errors, user, event.getProgramStage().getProgram(), event.getOrganisationUnit()); + checkDataReadAccessToProgram(errors, user, program); + checkDataReadAccessToCategoryOptionCombo(errors, user, event.getAttributeOptionCombo()); return errors; } @Override - public List canRead( - @Nonnull UserDetails user, SingleEvent event, DataElement dataElement) { + public List canCreate(@Nonnull UserDetails user, @Nonnull SingleEvent event) { if (user.isSuper()) { return List.of(); } - List errors = new ArrayList<>(canRead(user, event)); + return new ArrayList<>(validateSingleEventAccess(user, event)); + } + + @Override + public List canUpdate( + @Nonnull UserDetails user, @Nonnull SingleEvent event, @Nonnull OrganisationUnit orgUnit) { + if (user.isSuper()) { + return List.of(); + } - if (!aclService.canRead(user, dataElement)) { - errors.add("User has no read access to data element: " + dataElement.getUid()); + List errors = new ArrayList<>(validateSingleEventAccess(user, event)); + if (!orgUnit.getUid().equals(event.getOrganisationUnit().getUid())) { + checkOrgUnitInCaptureScope(errors, user, orgUnit); } return errors; } @Override - public List canCreate(@Nonnull UserDetails user, SingleEvent event) { - if (user.isSuper() || event == null) { + public List canDelete(@Nonnull UserDetails user, @Nonnull SingleEvent event) { + if (user.isSuper()) { return List.of(); } - ProgramStage programStage = event.getProgramStage(); + return canCreate(user, event); + } - Program program = programStage.getProgram(); - List errors = new ArrayList<>(); - if (!aclService.canDataWrite(user, program)) { - errors.add("User has no data write access to program: " + program.getUid()); + private List validateSingleEventAccess( + @Nonnull UserDetails user, @Nonnull SingleEvent event) { + if (user.isSuper()) { + return List.of(); } - errors.addAll(canWrite(user, event.getAttributeOptionCombo())); + List errors = new ArrayList<>(); + checkOrgUnitInCaptureScope(errors, user, event.getOrganisationUnit()); + checkDataWriteAccessToProgram(errors, user, event.getProgramStage().getProgram()); + checkDataWriteAccessToCategoryOptionCombo(errors, user, event.getAttributeOptionCombo()); return errors; } @@ -426,32 +433,6 @@ private List canWriteRelationship(UserDetails user, Relationship relatio return errors; } - /** - * Checks if the user has access to organisation unit under defined tracker program protection - * level - * - * @param user the user to check access for - * @param program program to check against protection level - * @param orgUnit the org unit to be checked under user's scope and program protection - * @return true if the user has access to the org unit under the mentioned program context, - * otherwise return false - */ - boolean canAccess(@Nonnull UserDetails user, Program program, OrganisationUnit orgUnit) { - if (orgUnit == null) { - return false; - } - - if (user.isSuper()) { - return true; - } - - if (program != null && (program.isClosed() || program.isProtected())) { - return user.isInUserHierarchy(orgUnit.getStoredPath()); - } - - return user.isInUserEffectiveSearchOrgUnitHierarchy(orgUnit.getStoredPath()); - } - private List canRead(@Nonnull UserDetails user, RelationshipItem item) { if (item.getTrackedEntity() != null) return canRead(user, item.getTrackedEntity()).stream() @@ -465,7 +446,10 @@ private List canRead(@Nonnull UserDetails user, RelationshipItem item) { return canRead(user, item.getTrackerEvent()).stream() .map(em -> em.validationCode().getMessage()) .toList(); - if (item.getSingleEvent() != null) return canRead(user, item.getSingleEvent()); + if (item.getSingleEvent() != null) + return canRead(user, item.getSingleEvent()).stream() + .map(em -> em.validationCode().getMessage()) + .toList(); return List.of(); } @@ -488,7 +472,10 @@ private List canWrite(@Nonnull UserDetails user, RelationshipItem item) .map(em -> em.validationCode().getMessage()) .toList(); } - if (item.getSingleEvent() != null) return canCreate(user, item.getSingleEvent()); + if (item.getSingleEvent() != null) + return canCreate(user, item.getSingleEvent()).stream() + .map(em -> em.validationCode().getMessage()) + .toList(); return List.of(); } @@ -573,23 +560,6 @@ private void checkDataReadAccessToCategoryOptionCombo( } } - // TODO(tracker) Remove this method and use #checkDataReadAccessToCategoryOptionCombo when - // refactoring single events - private List canRead(@Nonnull UserDetails user, CategoryOptionCombo categoryOptionCombo) { - if (user.isSuper() || categoryOptionCombo == null) { - return List.of(); - } - - List errors = new ArrayList<>(); - for (CategoryOption categoryOption : categoryOptionCombo.getCategoryOptions()) { - if (!aclService.canDataRead(user, categoryOption)) { - errors.add("User has no read access to category option: " + categoryOption.getUid()); - } - } - - return errors; - } - private void checkDataWriteAccessToCategoryOptionCombo( List errors, UserDetails user, CategoryOptionCombo categoryOptionCombo) { if (categoryOptionCombo == null) { @@ -605,24 +575,6 @@ private void checkDataWriteAccessToCategoryOptionCombo( } } - // TODO(tracker) Remove this method and use #checkDataWriteAccessToCategoryOptionCombo when - // refactoring single events - private List canWrite( - @Nonnull UserDetails user, CategoryOptionCombo categoryOptionCombo) { - if (user.isSuper() || categoryOptionCombo == null) { - return List.of(); - } - - List errors = new ArrayList<>(); - for (CategoryOption categoryOption : categoryOptionCombo.getCategoryOptions()) { - if (!aclService.canDataWrite(user, categoryOption)) { - errors.add("User has no write access to category option: " + categoryOption.getUid()); - } - } - - return errors; - } - private void checkOwnershipAccess( List errors, UserDetails user, TrackedEntity trackedEntity, Program program) { if (!ownershipAccessManager.hasAccess(user, trackedEntity, program)) { @@ -648,6 +600,28 @@ private void checkOrgUnitInCaptureScope( } } + private void checkOrgUnitInScope( + List errors, + UserDetails user, + @Nonnull Program program, + @Nonnull OrganisationUnit orgUnit) { + if (user.isSuper()) { + return; + } + + if (program.isClosed() || program.isProtected()) { + if (!user.isInUserHierarchy(orgUnit.getStoredPath())) { + errors.add( + new ErrorMessage(E1000, user.getUid(), List.of(user.getUid(), orgUnit.getUid()))); + } + } else { + if (!user.isInUserEffectiveSearchOrgUnitHierarchy(orgUnit.getStoredPath())) { + errors.add( + new ErrorMessage(E1105, user.getUid(), List.of(user.getUid(), orgUnit.getUid()))); + } + } + } + private void checkTrackedEntityProgramAccess( List errors, UserDetails user, diff --git a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/acl/TrackerAccessManager.java b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/acl/TrackerAccessManager.java index 5e81ef0779d7..7a6fb8e40025 100644 --- a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/acl/TrackerAccessManager.java +++ b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/acl/TrackerAccessManager.java @@ -31,7 +31,6 @@ import java.util.List; import javax.annotation.Nonnull; -import org.hisp.dhis.dataelement.DataElement; import org.hisp.dhis.organisationunit.OrganisationUnit; import org.hisp.dhis.tracker.model.Enrollment; import org.hisp.dhis.tracker.model.Relationship; @@ -44,6 +43,7 @@ * @author Morten Olav Hansen */ public interface TrackerAccessManager { + /** * Checks data read access to the TET and ownership of a tracked entity across programs for which * the user has data read access. @@ -147,23 +147,18 @@ List canUpdate( /** Like {@link #canCreate(UserDetails, TrackerEvent)}. */ List canDelete(UserDetails user, TrackerEvent event); - List canRead(UserDetails user, SingleEvent event); + List canRead(UserDetails user, SingleEvent event); + + List canCreate(UserDetails user, SingleEvent event); + + List canUpdate( + @Nonnull UserDetails user, SingleEvent event, @Nonnull OrganisationUnit orgUnit); - List canCreate(UserDetails user, SingleEvent event); + List canDelete(@Nonnull UserDetails user, SingleEvent event); List canRead(UserDetails user, Relationship relationship); List canCreate(UserDetails user, Relationship relationship); List canDelete(UserDetails user, @Nonnull Relationship relationship); - - /** - * Checks the sharing read access to EventDataValue - * - * @param user User validated for write access - * @param event SingleEvent under which the EventDataValue belongs - * @param dataElement DataElement of EventDataValue - * @return Empty list if read access allowed, list of errors otherwise. - */ - List canRead(UserDetails user, SingleEvent event, DataElement dataElement); } diff --git a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/CompletedSingleEventValidator.java b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/CompletedSingleEventValidator.java new file mode 100644 index 000000000000..4dfae67a7e1f --- /dev/null +++ b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/CompletedSingleEventValidator.java @@ -0,0 +1,67 @@ +/* + * Copyright (c) 2004-2026, University of Oslo + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, this + * list of conditions and the following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * 3. Neither the name of the copyright holder nor the names of its contributors + * may be used to endorse or promote products derived from this software without + * specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR + * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package org.hisp.dhis.tracker.imports.validation.validator.event; + +import static org.hisp.dhis.security.Authorities.F_UNCOMPLETE_EVENT; +import static org.hisp.dhis.tracker.imports.validation.ValidationCode.E1083; + +import org.hisp.dhis.event.EventStatus; +import org.hisp.dhis.tracker.imports.TrackerImportStrategy; +import org.hisp.dhis.tracker.imports.bundle.TrackerBundle; +import org.hisp.dhis.tracker.imports.domain.Event; +import org.hisp.dhis.tracker.imports.validation.Reporter; +import org.hisp.dhis.tracker.imports.validation.Validator; +import org.hisp.dhis.user.UserDetails; + +class CompletedSingleEventValidator implements Validator { + + @Override + public void validate(Reporter reporter, TrackerBundle bundle, Event event) { + if (!(event instanceof org.hisp.dhis.tracker.imports.domain.SingleEvent singleEvent)) { + return; + } + + UserDetails user = bundle.getUser(); + org.hisp.dhis.tracker.model.SingleEvent databaseSingleEvent = + bundle.getPreheat().getSingleEvent(singleEvent.getUID()); + + if (EventStatus.COMPLETED == databaseSingleEvent.getStatus() + && singleEvent.getStatus() != null + && singleEvent.getStatus() != databaseSingleEvent.getStatus() + && !user.isAuthorized(F_UNCOMPLETE_EVENT)) { + reporter.addError(singleEvent, E1083, user.getUid()); + } + } + + @Override + public boolean needsToRun(TrackerImportStrategy strategy) { + return strategy.isUpdate(); + } +} diff --git a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/CompletedEventValidator.java b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/CompletedTrackerEventValidator.java similarity index 97% rename from dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/CompletedEventValidator.java rename to dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/CompletedTrackerEventValidator.java index 07e5b3b3c0c9..cddfd476836c 100644 --- a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/CompletedEventValidator.java +++ b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/CompletedTrackerEventValidator.java @@ -40,7 +40,7 @@ import org.hisp.dhis.tracker.imports.validation.Validator; import org.hisp.dhis.user.UserDetails; -class CompletedEventValidator implements Validator { +class CompletedTrackerEventValidator implements Validator { @Override public void validate(Reporter reporter, TrackerBundle bundle, Event event) { diff --git a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/EventValidator.java b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/EventValidator.java index 80d690f39224..ad0a1325208a 100644 --- a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/EventValidator.java +++ b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/EventValidator.java @@ -65,7 +65,8 @@ public EventValidator( all( securityTrackerEventValidator, securitySingleEventValidator, - new CompletedEventValidator()), + new CompletedTrackerEventValidator(), + new CompletedSingleEventValidator()), all( categoryOptValidator, new DateValidator(), diff --git a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecuritySingleEventValidator.java b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecuritySingleEventValidator.java index 24eda582e976..32d419afeb6b 100644 --- a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecuritySingleEventValidator.java +++ b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecuritySingleEventValidator.java @@ -29,26 +29,18 @@ */ package org.hisp.dhis.tracker.imports.validation.validator.event; -import static org.hisp.dhis.security.Authorities.F_UNCOMPLETE_EVENT; -import static org.hisp.dhis.tracker.imports.validation.ValidationCode.E1083; +import static org.hisp.dhis.tracker.imports.bundle.TrackerObjectsMapper.mapSingleEvent; import javax.annotation.Nonnull; import lombok.RequiredArgsConstructor; -import org.hisp.dhis.category.CategoryOption; import org.hisp.dhis.category.CategoryOptionCombo; -import org.hisp.dhis.event.EventStatus; import org.hisp.dhis.organisationunit.OrganisationUnit; -import org.hisp.dhis.program.Program; -import org.hisp.dhis.program.ProgramStage; -import org.hisp.dhis.security.acl.AclService; +import org.hisp.dhis.tracker.acl.TrackerAccessManager; import org.hisp.dhis.tracker.imports.TrackerImportStrategy; import org.hisp.dhis.tracker.imports.bundle.TrackerBundle; import org.hisp.dhis.tracker.imports.domain.SingleEvent; -import org.hisp.dhis.tracker.imports.domain.TrackerDto; import org.hisp.dhis.tracker.imports.validation.Reporter; -import org.hisp.dhis.tracker.imports.validation.ValidationCode; import org.hisp.dhis.tracker.imports.validation.Validator; -import org.hisp.dhis.user.UserDetails; import org.springframework.stereotype.Component; @Component("org.hisp.dhis.tracker.imports.validation.validator.event.SecuritySingleEventValidator") @@ -56,89 +48,70 @@ class SecuritySingleEventValidator implements Validator { - @Nonnull private final AclService aclService; + @Nonnull private final TrackerAccessManager trackerAccessManager; @Override public void validate( Reporter reporter, TrackerBundle bundle, org.hisp.dhis.tracker.imports.domain.Event event) { - if (!(event instanceof SingleEvent)) { + if (!(event instanceof SingleEvent singleEvent)) { return; } TrackerImportStrategy strategy = bundle.getStrategy(event); - org.hisp.dhis.tracker.model.SingleEvent preheatEvent = - bundle.getPreheat().getSingleEvent(event.getEvent()); - OrganisationUnit organisationUnit = - strategy.isUpdateOrDelete() - ? preheatEvent.getOrganisationUnit() - : bundle.getPreheat().getOrganisationUnit(event.getOrgUnit()); - ProgramStage programStage = - strategy.isUpdateOrDelete() - ? preheatEvent.getProgramStage() - : bundle.getPreheat().getProgramStage(event.getProgramStage()); - - CategoryOptionCombo categoryOptionCombo = - bundle.getPreheat().getCategoryOptionCombo(event.getAttributeOptionCombo()); - - checkOrgUnitInCaptureScope(reporter, event, organisationUnit, bundle.getUser()); - checkProgramWriteAccess(reporter, event, programStage.getProgram(), bundle.getUser()); - checkWriteCategoryOptionComboAccess(reporter, event, categoryOptionCombo, bundle.getUser()); - - if (strategy.isUpdate()) { - OrganisationUnit payloadOrgUnit = bundle.getPreheat().getOrganisationUnit(event.getOrgUnit()); - if (!preheatEvent.getOrganisationUnit().getUid().equals(payloadOrgUnit.getUid())) { - checkOrgUnitInCaptureScope(reporter, event, payloadOrgUnit, bundle.getUser()); + if (strategy.isCreate()) { + handleCreate(reporter, bundle, singleEvent); + } else { + org.hisp.dhis.tracker.model.SingleEvent databaseSingleEvent = + bundle.getPreheat().getSingleEvent(singleEvent.getUID()); + CategoryOptionCombo aoc = + bundle.getPreheat().getCategoryOptionCombo(singleEvent.getAttributeOptionCombo()); + databaseSingleEvent.setAttributeOptionCombo(aoc); + + if (strategy.isUpdate()) { + handleUpdate(reporter, bundle, databaseSingleEvent, singleEvent); + } else if (strategy.isDelete()) { + handleDelete(reporter, bundle, databaseSingleEvent, singleEvent); } - - checkCompletablePermission(reporter, event, preheatEvent, bundle.getUser()); } } - private void checkCompletablePermission( - Reporter reporter, - org.hisp.dhis.tracker.imports.domain.Event event, - org.hisp.dhis.tracker.model.SingleEvent preheatEvent, - UserDetails user) { - if (EventStatus.COMPLETED == preheatEvent.getStatus() - && event.getStatus() != preheatEvent.getStatus() - && (!user.isSuper() && !user.isAuthorized(F_UNCOMPLETE_EVENT))) { - reporter.addError(event, E1083, user); - } - } + private void handleCreate(Reporter reporter, TrackerBundle bundle, SingleEvent singleEvent) { + org.hisp.dhis.tracker.model.SingleEvent mappedEvent = + mapSingleEvent(bundle.getPreheat(), singleEvent, bundle.getUser()); - @Override - public boolean needsToRun(TrackerImportStrategy strategy) { - return true; + trackerAccessManager + .canCreate(bundle.getUser(), mappedEvent) + .forEach(em -> reporter.addError(singleEvent, em.validationCode(), em.args().toArray())); } - private void checkOrgUnitInCaptureScope( - Reporter reporter, TrackerDto dto, OrganisationUnit eventOrgUnit, UserDetails user) { - if (!user.isInUserHierarchy(eventOrgUnit.getStoredPath())) { - reporter.addError(dto, ValidationCode.E1000, user, eventOrgUnit); - } - } - - private void checkProgramWriteAccess( - Reporter reporter, TrackerDto dto, Program program, UserDetails user) { - if (!aclService.canDataWrite(user, program)) { - reporter.addError(dto, ValidationCode.E1091, user, program); - } + private void handleUpdate( + Reporter reporter, + TrackerBundle bundle, + org.hisp.dhis.tracker.model.SingleEvent databaseSingleEvent, + SingleEvent singleEvent) { + OrganisationUnit payloadOrgUnit = + bundle.getPreheat().getOrganisationUnit(singleEvent.getOrgUnit()); + OrganisationUnit orgUnit = + payloadOrgUnit != null ? payloadOrgUnit : databaseSingleEvent.getOrganisationUnit(); + + trackerAccessManager + .canUpdate(bundle.getUser(), databaseSingleEvent, orgUnit) + .forEach(em -> reporter.addError(singleEvent, em.validationCode(), em.args().toArray())); } - public void checkWriteCategoryOptionComboAccess( + private void handleDelete( Reporter reporter, - TrackerDto dto, - CategoryOptionCombo categoryOptionCombo, - UserDetails user) { - if (categoryOptionCombo == null) { - return; - } + TrackerBundle bundle, + org.hisp.dhis.tracker.model.SingleEvent databaseSingleEvent, + SingleEvent singleEvent) { + trackerAccessManager + .canDelete(bundle.getUser(), databaseSingleEvent) + .forEach(em -> reporter.addError(singleEvent, em.validationCode(), em.args().toArray())); + } - for (CategoryOption categoryOption : categoryOptionCombo.getCategoryOptions()) { - if (!aclService.canDataWrite(user, categoryOption)) { - reporter.addError(dto, ValidationCode.E1099, user, categoryOption); - } - } + @Override + public boolean needsToRun(TrackerImportStrategy strategy) { + return true; } } diff --git a/dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/acl/DefaultTrackerAccessManagerTest.java b/dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/acl/DefaultTrackerAccessManagerTest.java deleted file mode 100644 index f7c367d3a94c..000000000000 --- a/dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/acl/DefaultTrackerAccessManagerTest.java +++ /dev/null @@ -1,141 +0,0 @@ -/* - * Copyright (c) 2004-2023, University of Oslo - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * - * 1. Redistributions of source code must retain the above copyright notice, this - * list of conditions and the following disclaimer. - * - * 2. Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following disclaimer in the documentation - * and/or other materials provided with the distribution. - * - * 3. Neither the name of the copyright holder nor the names of its contributors - * may be used to endorse or promote products derived from this software without - * specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND - * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED - * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE - * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR - * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES - * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; - * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON - * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS - * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - */ -package org.hisp.dhis.tracker.acl; - -import static org.hisp.dhis.common.AccessLevel.CLOSED; -import static org.hisp.dhis.common.AccessLevel.OPEN; -import static org.hisp.dhis.common.AccessLevel.PROTECTED; -import static org.hisp.dhis.test.TestBase.createOrganisationUnit; -import static org.hisp.dhis.test.TestBase.createProgram; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; - -import java.util.Set; -import org.hisp.dhis.organisationunit.OrganisationUnit; -import org.hisp.dhis.program.Program; -import org.hisp.dhis.user.User; -import org.hisp.dhis.user.UserDetails; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.InjectMocks; -import org.mockito.junit.jupiter.MockitoExtension; - -@ExtendWith(MockitoExtension.class) -class DefaultTrackerAccessManagerTest { - - @InjectMocks private DefaultTrackerAccessManager trackerAccessManager; - - private Program program; - - private OrganisationUnit orgUnit; - - private User user; - - @BeforeEach - void before() { - program = createProgram('A'); - orgUnit = createOrganisationUnit('A'); - user = new User(); - } - - @Test - void shouldHaveAccessWhenProgramOpenAndSearchAccessAvailable() { - program.setAccessLevel(OPEN); - user.setTeiSearchOrganisationUnits(Set.of(orgUnit)); - - assertTrue( - trackerAccessManager.canAccess(UserDetails.fromUser(user), program, orgUnit), - "User should have access to open program"); - } - - @Test - void shouldNotHaveAccessWhenProgramOpenAndSearchAccessNotAvailable() { - program.setAccessLevel(OPEN); - - assertFalse( - trackerAccessManager.canAccess(UserDetails.fromUser(user), program, orgUnit), - "User should not have access to open program"); - } - - @Test - void shouldHaveAccessWhenProgramNullAndSearchAccessAvailable() { - user.setTeiSearchOrganisationUnits(Set.of(orgUnit)); - - assertTrue( - trackerAccessManager.canAccess(UserDetails.fromUser(user), null, orgUnit), - "User should have access to unspecified program"); - } - - @Test - void shouldNotHaveAccessWhenProgramNullAndSearchAccessNotAvailable() { - assertFalse( - trackerAccessManager.canAccess(UserDetails.fromUser(user), null, orgUnit), - "User should not have access to unspecified program"); - } - - @Test - void shouldHaveAccessWhenProgramClosedAndCaptureAccessAvailable() { - program.setAccessLevel(CLOSED); - user.setOrganisationUnits(Set.of(orgUnit)); - - assertTrue( - trackerAccessManager.canAccess(UserDetails.fromUser(user), program, orgUnit), - "User should have access to closed program"); - } - - @Test - void shouldNotHaveAccessWhenProgramClosedAndCaptureAccessNotAvailable() { - program.setAccessLevel(CLOSED); - - assertFalse( - trackerAccessManager.canAccess(UserDetails.fromUser(user), program, orgUnit), - "User should not have access to closed program"); - } - - @Test - void shouldHaveAccessWhenProgramProtectedAndCaptureAccessAvailable() { - program.setAccessLevel(PROTECTED); - user.setOrganisationUnits(Set.of(orgUnit)); - - assertTrue( - trackerAccessManager.canAccess(UserDetails.fromUser(user), program, orgUnit), - "User should have access to protected program"); - } - - @Test - void shouldNotHaveAccessWhenProgramProtectedAndCaptureAccessNotAvailable() { - program.setAccessLevel(PROTECTED); - - assertFalse( - trackerAccessManager.canAccess(UserDetails.fromUser(user), program, orgUnit), - "User should not have access to protected program"); - } -} diff --git a/dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/CompletedSingleEventValidatorTest.java b/dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/CompletedSingleEventValidatorTest.java new file mode 100644 index 000000000000..c4d32146de9f --- /dev/null +++ b/dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/CompletedSingleEventValidatorTest.java @@ -0,0 +1,183 @@ +/* + * Copyright (c) 2004-2026, University of Oslo + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, this + * list of conditions and the following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * 3. Neither the name of the copyright holder nor the names of its contributors + * may be used to endorse or promote products derived from this software without + * specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR + * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON + * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ +package org.hisp.dhis.tracker.imports.validation.validator.event; + +import static org.hisp.dhis.security.Authorities.F_UNCOMPLETE_EVENT; +import static org.hisp.dhis.test.utils.Assertions.assertIsEmpty; +import static org.hisp.dhis.tracker.imports.validation.ValidationCode.E1083; +import static org.hisp.dhis.tracker.imports.validation.validator.AssertValidations.assertHasError; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import org.hisp.dhis.common.UID; +import org.hisp.dhis.event.EventStatus; +import org.hisp.dhis.tracker.TrackerIdSchemeParams; +import org.hisp.dhis.tracker.imports.bundle.TrackerBundle; +import org.hisp.dhis.tracker.imports.preheat.TrackerPreheat; +import org.hisp.dhis.tracker.imports.validation.Reporter; +import org.hisp.dhis.user.UserDetails; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class CompletedSingleEventValidatorTest { + private static final UID EVENT_UID = UID.generate(); + + @Mock private TrackerBundle bundle; + + @Mock private TrackerPreheat preheat; + + @Mock private UserDetails user; + + private CompletedSingleEventValidator validator; + + private Reporter reporter; + + @BeforeEach + void setUp() { + reporter = new Reporter(TrackerIdSchemeParams.builder().build()); + validator = new CompletedSingleEventValidator(); + } + + @Test + void shouldPassWhenUserHasAuthorityToReopenCompletedEvent() { + when(bundle.getPreheat()).thenReturn(preheat); + when(bundle.getUser()).thenReturn(user); + org.hisp.dhis.tracker.model.SingleEvent databaseEvent = + mock(org.hisp.dhis.tracker.model.SingleEvent.class); + when(databaseEvent.getStatus()).thenReturn(EventStatus.COMPLETED); + when(preheat.getSingleEvent(EVENT_UID)).thenReturn(databaseEvent); + when(user.isAuthorized(F_UNCOMPLETE_EVENT)).thenReturn(true); + + org.hisp.dhis.tracker.imports.domain.SingleEvent event = + org.hisp.dhis.tracker.imports.domain.SingleEvent.builder() + .event(EVENT_UID) + .status(EventStatus.ACTIVE) + .build(); + + validator.validate(reporter, bundle, event); + + assertIsEmpty(reporter.getErrors()); + } + + @Test + void shouldPassWhenEventNotCompleted() { + when(bundle.getPreheat()).thenReturn(preheat); + org.hisp.dhis.tracker.model.SingleEvent databaseEvent = + mock(org.hisp.dhis.tracker.model.SingleEvent.class); + when(databaseEvent.getStatus()).thenReturn(EventStatus.ACTIVE); + when(preheat.getSingleEvent(EVENT_UID)).thenReturn(databaseEvent); + + org.hisp.dhis.tracker.imports.domain.SingleEvent event = + org.hisp.dhis.tracker.imports.domain.SingleEvent.builder() + .event(EVENT_UID) + .status(EventStatus.COMPLETED) + .build(); + + validator.validate(reporter, bundle, event); + + assertIsEmpty(reporter.getErrors()); + } + + @Test + void shouldPassWhenPayloadEventStatusIsNull() { + when(bundle.getPreheat()).thenReturn(preheat); + org.hisp.dhis.tracker.model.SingleEvent databaseEvent = + mock(org.hisp.dhis.tracker.model.SingleEvent.class); + when(databaseEvent.getStatus()).thenReturn(EventStatus.COMPLETED); + when(preheat.getSingleEvent(EVENT_UID)).thenReturn(databaseEvent); + + org.hisp.dhis.tracker.imports.domain.SingleEvent event = + org.hisp.dhis.tracker.imports.domain.SingleEvent.builder() + .event(EVENT_UID) + .status(null) + .build(); + + validator.validate(reporter, bundle, event); + + assertIsEmpty(reporter.getErrors()); + } + + @Test + void shouldPassWhenPayloadEventStatusSameAsDatabaseEventStatus() { + when(bundle.getPreheat()).thenReturn(preheat); + org.hisp.dhis.tracker.model.SingleEvent databaseEvent = + mock(org.hisp.dhis.tracker.model.SingleEvent.class); + when(databaseEvent.getStatus()).thenReturn(EventStatus.COMPLETED); + when(preheat.getSingleEvent(EVENT_UID)).thenReturn(databaseEvent); + + org.hisp.dhis.tracker.imports.domain.SingleEvent event = + org.hisp.dhis.tracker.imports.domain.SingleEvent.builder() + .event(EVENT_UID) + .status(EventStatus.COMPLETED) + .build(); + + validator.validate(reporter, bundle, event); + + assertIsEmpty(reporter.getErrors()); + } + + @Test + void shouldFailWhenUserNotAllowedToReopenCompletedEvent() { + when(bundle.getPreheat()).thenReturn(preheat); + when(bundle.getUser()).thenReturn(user); + org.hisp.dhis.tracker.model.SingleEvent databaseEvent = + mock(org.hisp.dhis.tracker.model.SingleEvent.class); + when(databaseEvent.getStatus()).thenReturn(EventStatus.COMPLETED); + when(preheat.getSingleEvent(EVENT_UID)).thenReturn(databaseEvent); + when(user.isAuthorized(F_UNCOMPLETE_EVENT)).thenReturn(false); + + org.hisp.dhis.tracker.imports.domain.SingleEvent event = + org.hisp.dhis.tracker.imports.domain.SingleEvent.builder() + .event(EVENT_UID) + .status(EventStatus.ACTIVE) + .build(); + + validator.validate(reporter, bundle, event); + + assertHasError(reporter, event, E1083); + } + + @Test + void shouldPassWhenEventIsTrackerEvent() { + org.hisp.dhis.tracker.imports.domain.TrackerEvent event = + org.hisp.dhis.tracker.imports.domain.TrackerEvent.builder() + .event(EVENT_UID) + .status(EventStatus.ACTIVE) + .build(); + + validator.validate(reporter, bundle, event); + + assertIsEmpty(reporter.getErrors()); + } +} diff --git a/dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/CompletedEventValidatorTest.java b/dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/CompletedTrackerEventValidatorTest.java similarity index 97% rename from dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/CompletedEventValidatorTest.java rename to dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/CompletedTrackerEventValidatorTest.java index 07bed400c4f6..b6179750cdc8 100644 --- a/dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/CompletedEventValidatorTest.java +++ b/dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/CompletedTrackerEventValidatorTest.java @@ -50,7 +50,7 @@ import org.mockito.junit.jupiter.MockitoExtension; @ExtendWith(MockitoExtension.class) -class CompletedEventValidatorTest { +class CompletedTrackerEventValidatorTest { private static final UID EVENT_UID = UID.generate(); @Mock private TrackerBundle bundle; @@ -59,7 +59,7 @@ class CompletedEventValidatorTest { @Mock private UserDetails user; - private CompletedEventValidator validator; + private CompletedTrackerEventValidator validator; private Reporter reporter; @@ -68,7 +68,7 @@ void setUp() { TrackerIdSchemeParams idSchemes = TrackerIdSchemeParams.builder().build(); reporter = new Reporter(idSchemes); - validator = new CompletedEventValidator(); + validator = new CompletedTrackerEventValidator(); } @Test diff --git a/dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecuritySingleEventValidatorTest.java b/dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecuritySingleEventValidatorTest.java index 097128e44c3e..f61362072114 100644 --- a/dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecuritySingleEventValidatorTest.java +++ b/dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecuritySingleEventValidatorTest.java @@ -31,24 +31,22 @@ import static org.hisp.dhis.test.utils.Assertions.assertIsEmpty; import static org.hisp.dhis.tracker.imports.validation.ValidationCode.E1000; -import static org.hisp.dhis.tracker.imports.validation.ValidationCode.E1083; import static org.hisp.dhis.tracker.imports.validation.ValidationCode.E1091; import static org.hisp.dhis.tracker.imports.validation.ValidationCode.E1099; import static org.hisp.dhis.tracker.imports.validation.validator.AssertValidations.assertHasError; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.when; -import java.util.Set; -import org.hisp.dhis.category.CategoryOption; -import org.hisp.dhis.category.CategoryOptionCombo; +import java.util.List; import org.hisp.dhis.common.UID; -import org.hisp.dhis.event.EventStatus; import org.hisp.dhis.organisationunit.OrganisationUnit; import org.hisp.dhis.program.Program; import org.hisp.dhis.program.ProgramStage; import org.hisp.dhis.program.ProgramType; -import org.hisp.dhis.security.acl.AclService; import org.hisp.dhis.tracker.TrackerIdSchemeParams; +import org.hisp.dhis.tracker.acl.ErrorMessage; +import org.hisp.dhis.tracker.acl.TrackerAccessManager; import org.hisp.dhis.tracker.imports.TrackerImportStrategy; import org.hisp.dhis.tracker.imports.bundle.TrackerBundle; import org.hisp.dhis.tracker.imports.domain.MetadataIdentifier; @@ -56,10 +54,8 @@ import org.hisp.dhis.tracker.imports.validation.Reporter; import org.hisp.dhis.tracker.model.SingleEvent; import org.hisp.dhis.tracker.test.TrackerTestBase; -import org.hisp.dhis.user.User; import org.hisp.dhis.user.UserDetails; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.EnumSource; @@ -83,7 +79,7 @@ class SecuritySingleEventValidatorTest extends TrackerTestBase { @Mock private TrackerPreheat preheat; - @Mock private AclService aclService; + @Mock private TrackerAccessManager trackerAccessManager; private final UserDetails user = UserDetails.fromUser(makeUser("A")); @@ -91,47 +87,29 @@ class SecuritySingleEventValidatorTest extends TrackerTestBase { private OrganisationUnit organisationUnit; - private Program program; - private ProgramStage programStage; - private CategoryOptionCombo categoryOptionCombo; - - private CategoryOption categoryOption; - @BeforeEach void setUp() { when(bundle.getPreheat()).thenReturn(preheat); when(bundle.getUser()).thenReturn(user); + organisationUnit = createOrganisationUnit('A'); organisationUnit.setUid(ORG_UNIT_ID); organisationUnit.updatePath(); - program = createProgram('A'); + Program program = createProgram('A'); program.setUid(PROGRAM_ID); program.setProgramType(ProgramType.WITHOUT_REGISTRATION); - programStage = createProgramStage('A', program); programStage.setUid(PS_ID); - categoryOption = createCategoryOption('A'); - categoryOptionCombo = createCategoryOptionCombo('A'); - categoryOptionCombo.setCategoryOptions(Set.of(categoryOption)); - - TrackerIdSchemeParams idSchemes = TrackerIdSchemeParams.builder().build(); - reporter = new Reporter(idSchemes); - - validator = new SecuritySingleEventValidator(aclService); - - when(bundle.getPreheat()).thenReturn(preheat); - } + reporter = new Reporter(TrackerIdSchemeParams.builder().build()); + validator = new SecuritySingleEventValidator(trackerAccessManager); - private UserDetails setUpUserWithOrgUnit() { - User userWithOrgUnit = makeUser("B"); - userWithOrgUnit.setOrganisationUnits(Set.of(organisationUnit)); - UserDetails currentUserDetails = UserDetails.fromUser(userWithOrgUnit); - when(bundle.getUser()).thenReturn(currentUserDetails); - return currentUserDetails; + lenient() + .when(preheat.getProgramStage(MetadataIdentifier.ofUid(PS_ID))) + .thenReturn(programStage); } @ParameterizedTest @@ -141,20 +119,11 @@ private UserDetails setUpUserWithOrgUnit() { names = {"CREATE"}) void shouldFailValidationWhenUserDoNotHaveOrgUnitInCaptureScoreForCreateStrategy( TrackerImportStrategy strategy) { - UID enrollmentUid = UID.generate(); - org.hisp.dhis.tracker.imports.domain.Event event = - org.hisp.dhis.tracker.imports.domain.SingleEvent.builder() - .event(UID.generate()) - .enrollment(enrollmentUid) - .orgUnit(MetadataIdentifier.ofUid(ORG_UNIT_ID)) - .programStage(MetadataIdentifier.ofUid(PS_ID)) - .program(MetadataIdentifier.ofUid(PROGRAM_ID)) - .build(); - + org.hisp.dhis.tracker.imports.domain.Event event = singleEvent(); when(bundle.getStrategy(event)).thenReturn(strategy); - when(preheat.getProgramStage(event.getProgramStage())).thenReturn(programStage); - when(preheat.getOrganisationUnit(MetadataIdentifier.ofUid(ORG_UNIT_ID))) - .thenReturn(organisationUnit); + when(trackerAccessManager.canCreate(any(UserDetails.class), any(SingleEvent.class))) + .thenReturn( + List.of(new ErrorMessage(E1000, user.getUid(), List.of(user.getUid(), ORG_UNIT_ID)))); validator.validate(reporter, bundle, event); @@ -166,25 +135,21 @@ void shouldFailValidationWhenUserDoNotHaveOrgUnitInCaptureScoreForCreateStrategy value = TrackerImportStrategy.class, mode = EnumSource.Mode.INCLUDE, names = {"UPDATE", "DELETE"}) - void - shouldFailValidationWhenUserDoesNotHaveDatabaseOrgUnitInCaptureScopeForUpdateAndDeleteStrategy( - TrackerImportStrategy strategy) { - UID enrollmentUid = UID.generate(); - org.hisp.dhis.tracker.imports.domain.Event event = - org.hisp.dhis.tracker.imports.domain.SingleEvent.builder() - .event(UID.generate()) - .enrollment(enrollmentUid) - .orgUnit(MetadataIdentifier.ofUid(ORG_UNIT_ID)) - .programStage(MetadataIdentifier.ofUid(PS_ID)) - .program(MetadataIdentifier.ofUid(PROGRAM_ID)) - .build(); - + void shouldFailValidationWhenUserDoesNotHaveOrgUnitInCaptureScopeForUpdateAndDeleteStrategy( + TrackerImportStrategy strategy) { + org.hisp.dhis.tracker.imports.domain.Event event = singleEvent(); when(bundle.getStrategy(event)).thenReturn(strategy); - SingleEvent preheatEvent = getEvent(); - when(preheat.getSingleEvent(event.getEvent())).thenReturn(preheatEvent); + when(preheat.getSingleEvent(event.getEvent())).thenReturn(dbSingleEvent()); lenient() - .when(bundle.getPreheat().getOrganisationUnit(event.getOrgUnit())) - .thenReturn(organisationUnit); + .when( + trackerAccessManager.canUpdate( + any(UserDetails.class), any(SingleEvent.class), any(OrganisationUnit.class))) + .thenReturn( + List.of(new ErrorMessage(E1000, user.getUid(), List.of(user.getUid(), ORG_UNIT_ID)))); + lenient() + .when(trackerAccessManager.canDelete(any(UserDetails.class), any(SingleEvent.class))) + .thenReturn( + List.of(new ErrorMessage(E1000, user.getUid(), List.of(user.getUid(), ORG_UNIT_ID)))); validator.validate(reporter, bundle, event); @@ -198,29 +163,14 @@ void shouldFailValidationWhenUserDoNotHaveOrgUnitInCaptureScoreForCreateStrategy names = {"UPDATE"}) void shouldFailValidationWhenUserDoesNotHavePayloadOrgUnitInCaptureScopeForUpdateStrategy( TrackerImportStrategy strategy) { - UID enrollmentUid = UID.generate(); - org.hisp.dhis.tracker.imports.domain.Event event = - org.hisp.dhis.tracker.imports.domain.SingleEvent.builder() - .event(UID.generate()) - .enrollment(enrollmentUid) - .orgUnit(MetadataIdentifier.ofUid(ORG_UNIT_ID)) - .programStage(MetadataIdentifier.ofUid(PS_ID)) - .program(MetadataIdentifier.ofUid(PROGRAM_ID)) - .build(); - - User user = makeUser("B"); - user.setOrganisationUnits(Set.of(organisationUnit)); - UserDetails userDetails = UserDetails.fromUser(user); - when(bundle.getUser()).thenReturn(userDetails); - - OrganisationUnit outOfScopeOrgUnit = createOrganisationUnit('B'); - outOfScopeOrgUnit.setUid("ORG_UNIT_UID"); - outOfScopeOrgUnit.updatePath(); - + org.hisp.dhis.tracker.imports.domain.Event event = singleEvent(); when(bundle.getStrategy(event)).thenReturn(strategy); - SingleEvent preheatEvent = getEvent(); - when(preheat.getSingleEvent(event.getEvent())).thenReturn(preheatEvent); - when(bundle.getPreheat().getOrganisationUnit(event.getOrgUnit())).thenReturn(outOfScopeOrgUnit); + when(preheat.getSingleEvent(event.getEvent())).thenReturn(dbSingleEvent()); + when(preheat.getOrganisationUnit(event.getOrgUnit())).thenReturn(organisationUnit); + when(trackerAccessManager.canUpdate( + any(UserDetails.class), any(SingleEvent.class), any(OrganisationUnit.class))) + .thenReturn( + List.of(new ErrorMessage(E1000, user.getUid(), List.of(user.getUid(), ORG_UNIT_ID)))); validator.validate(reporter, bundle, event); @@ -234,23 +184,11 @@ void shouldFailValidationWhenUserDoesNotHavePayloadOrgUnitInCaptureScopeForUpdat names = {"CREATE"}) void shouldFailValidationWhenUserDoNotHaveWriteAccessToProgramForCreateStrategy( TrackerImportStrategy strategy) { - UID enrollmentUid = UID.generate(); - org.hisp.dhis.tracker.imports.domain.Event event = - org.hisp.dhis.tracker.imports.domain.SingleEvent.builder() - .event(UID.generate()) - .enrollment(enrollmentUid) - .orgUnit(MetadataIdentifier.ofUid(ORG_UNIT_ID)) - .programStage(MetadataIdentifier.ofUid(PS_ID)) - .program(MetadataIdentifier.ofUid(PROGRAM_ID)) - .build(); - + org.hisp.dhis.tracker.imports.domain.Event event = singleEvent(); when(bundle.getStrategy(event)).thenReturn(strategy); - when(preheat.getProgramStage(event.getProgramStage())).thenReturn(programStage); - when(preheat.getOrganisationUnit(MetadataIdentifier.ofUid(ORG_UNIT_ID))) - .thenReturn(organisationUnit); - - UserDetails userDetails = setUpUserWithOrgUnit(); - when(aclService.canDataWrite(userDetails, program)).thenReturn(false); + when(trackerAccessManager.canCreate(any(UserDetails.class), any(SingleEvent.class))) + .thenReturn( + List.of(new ErrorMessage(E1091, user.getUid(), List.of(user.getUid(), PROGRAM_ID)))); validator.validate(reporter, bundle, event); @@ -264,24 +202,19 @@ void shouldFailValidationWhenUserDoNotHaveWriteAccessToProgramForCreateStrategy( names = {"UPDATE", "DELETE"}) void shouldFailValidationWhenUserDoNotHaveWriteAccessToProgramForUpdateAndDeleteStrategy( TrackerImportStrategy strategy) { - UID enrollmentUid = UID.generate(); - org.hisp.dhis.tracker.imports.domain.Event event = - org.hisp.dhis.tracker.imports.domain.SingleEvent.builder() - .event(UID.generate()) - .enrollment(enrollmentUid) - .orgUnit(MetadataIdentifier.ofUid(ORG_UNIT_ID)) - .programStage(MetadataIdentifier.ofUid(PS_ID)) - .program(MetadataIdentifier.ofUid(PROGRAM_ID)) - .build(); - + org.hisp.dhis.tracker.imports.domain.Event event = singleEvent(); when(bundle.getStrategy(event)).thenReturn(strategy); - SingleEvent preheatEvent = getEvent(); - when(preheat.getSingleEvent(event.getEvent())).thenReturn(preheatEvent); - UserDetails userDetails = setUpUserWithOrgUnit(); - when(aclService.canDataWrite(userDetails, program)).thenReturn(false); + when(preheat.getSingleEvent(event.getEvent())).thenReturn(dbSingleEvent()); + lenient() + .when( + trackerAccessManager.canUpdate( + any(UserDetails.class), any(SingleEvent.class), any(OrganisationUnit.class))) + .thenReturn( + List.of(new ErrorMessage(E1091, user.getUid(), List.of(user.getUid(), PROGRAM_ID)))); lenient() - .when(bundle.getPreheat().getOrganisationUnit(event.getOrgUnit())) - .thenReturn(organisationUnit); + .when(trackerAccessManager.canDelete(any(UserDetails.class), any(SingleEvent.class))) + .thenReturn( + List.of(new ErrorMessage(E1091, user.getUid(), List.of(user.getUid(), PROGRAM_ID)))); validator.validate(reporter, bundle, event); @@ -295,28 +228,11 @@ void shouldFailValidationWhenUserDoNotHaveWriteAccessToProgramForUpdateAndDelete names = {"CREATE"}) void shouldFailValidationWhenUserDoNotHaveWriteAccessToCategoryOptionForCreateStrategy( TrackerImportStrategy strategy) { - UID enrollmentUid = UID.generate(); - MetadataIdentifier attributeOptionComboUid = - MetadataIdentifier.ofUid(categoryOptionCombo.getUid()); - org.hisp.dhis.tracker.imports.domain.Event event = - org.hisp.dhis.tracker.imports.domain.SingleEvent.builder() - .event(UID.generate()) - .enrollment(enrollmentUid) - .orgUnit(MetadataIdentifier.ofUid(ORG_UNIT_ID)) - .programStage(MetadataIdentifier.ofUid(PS_ID)) - .program(MetadataIdentifier.ofUid(PROGRAM_ID)) - .attributeOptionCombo(attributeOptionComboUid) - .build(); - - UserDetails userDetails = setUpUserWithOrgUnit(); + org.hisp.dhis.tracker.imports.domain.Event event = singleEvent(); when(bundle.getStrategy(event)).thenReturn(strategy); - when(preheat.getProgramStage(event.getProgramStage())).thenReturn(programStage); - when(preheat.getCategoryOptionCombo(MetadataIdentifier.ofUid(categoryOptionCombo))) - .thenReturn(categoryOptionCombo); - when(preheat.getOrganisationUnit(MetadataIdentifier.ofUid(ORG_UNIT_ID))) - .thenReturn(organisationUnit); - when(aclService.canDataWrite(userDetails, program)).thenReturn(true); - when(aclService.canDataWrite(userDetails, categoryOption)).thenReturn(false); + when(trackerAccessManager.canCreate(any(UserDetails.class), any(SingleEvent.class))) + .thenReturn( + List.of(new ErrorMessage(E1099, user.getUid(), List.of(user.getUid(), "catOptUid")))); validator.validate(reporter, bundle, event); @@ -330,29 +246,20 @@ void shouldFailValidationWhenUserDoNotHaveWriteAccessToCategoryOptionForCreateSt names = {"UPDATE", "DELETE"}) void shouldFailValidationWhenUserDoNotHaveWriteAccessToCategoryOptionForUpdateAndDeleteStrategy( TrackerImportStrategy strategy) { - UID enrollmentUid = UID.generate(); - org.hisp.dhis.tracker.imports.domain.Event event = - org.hisp.dhis.tracker.imports.domain.SingleEvent.builder() - .event(UID.generate()) - .enrollment(enrollmentUid) - .orgUnit(MetadataIdentifier.ofUid(ORG_UNIT_ID)) - .programStage(MetadataIdentifier.ofUid(PS_ID)) - .program(MetadataIdentifier.ofUid(PROGRAM_ID)) - .attributeOptionCombo(MetadataIdentifier.ofUid(categoryOptionCombo)) - .build(); - + org.hisp.dhis.tracker.imports.domain.Event event = singleEvent(); when(bundle.getStrategy(event)).thenReturn(strategy); - SingleEvent preheatEvent = getEvent(); - when(preheat.getSingleEvent(event.getEvent())).thenReturn(preheatEvent); - when(preheat.getCategoryOptionCombo(MetadataIdentifier.ofUid(categoryOptionCombo))) - .thenReturn(categoryOptionCombo); - - UserDetails userDetails = setUpUserWithOrgUnit(); - when(aclService.canDataWrite(userDetails, program)).thenReturn(true); - when(aclService.canDataWrite(userDetails, categoryOption)).thenReturn(false); + when(preheat.getSingleEvent(event.getEvent())).thenReturn(dbSingleEvent()); + lenient() + .when( + trackerAccessManager.canUpdate( + any(UserDetails.class), any(SingleEvent.class), any(OrganisationUnit.class))) + .thenReturn( + List.of(new ErrorMessage(E1099, user.getUid(), List.of(user.getUid(), "catOptUid")))); lenient() - .when(bundle.getPreheat().getOrganisationUnit(event.getOrgUnit())) - .thenReturn(organisationUnit); + .when(trackerAccessManager.canDelete(any(UserDetails.class), any(SingleEvent.class))) + .thenReturn( + List.of(new ErrorMessage(E1099, user.getUid(), List.of(user.getUid(), "catOptUid")))); + validator.validate(reporter, bundle, event); assertHasError(reporter, event, E1099); @@ -364,31 +271,17 @@ void shouldFailValidationWhenUserDoNotHaveWriteAccessToCategoryOptionForUpdateAn mode = EnumSource.Mode.INCLUDE, names = {"UPDATE", "DELETE"}) void shouldPassValidationWhenDeletingOrUpdatingSingleEvent(TrackerImportStrategy strategy) { - UID enrollmentUid = UID.generate(); - org.hisp.dhis.tracker.imports.domain.Event event = - org.hisp.dhis.tracker.imports.domain.SingleEvent.builder() - .event(UID.generate()) - .enrollment(enrollmentUid) - .orgUnit(MetadataIdentifier.ofUid(ORG_UNIT_ID)) - .programStage(MetadataIdentifier.ofUid(PS_ID)) - .program(MetadataIdentifier.ofUid(PROGRAM_ID)) - .status(EventStatus.COMPLETED) - .attributeOptionCombo(MetadataIdentifier.ofUid(categoryOptionCombo)) - .build(); - + org.hisp.dhis.tracker.imports.domain.Event event = singleEvent(); when(bundle.getStrategy(event)).thenReturn(strategy); - SingleEvent preheatEvent = getEvent(); - - when(preheat.getSingleEvent(event.getEvent())).thenReturn(preheatEvent); - when(preheat.getCategoryOptionCombo(MetadataIdentifier.ofUid(categoryOptionCombo))) - .thenReturn(categoryOptionCombo); - - UserDetails userDetails = setUpUserWithOrgUnit(); - when(aclService.canDataWrite(userDetails, program)).thenReturn(true); - when(aclService.canDataWrite(userDetails, categoryOption)).thenReturn(true); + when(preheat.getSingleEvent(event.getEvent())).thenReturn(dbSingleEvent()); lenient() - .when(bundle.getPreheat().getOrganisationUnit(event.getOrgUnit())) - .thenReturn(organisationUnit); + .when( + trackerAccessManager.canUpdate( + any(UserDetails.class), any(SingleEvent.class), any(OrganisationUnit.class))) + .thenReturn(List.of()); + lenient() + .when(trackerAccessManager.canDelete(any(UserDetails.class), any(SingleEvent.class))) + .thenReturn(List.of()); validator.validate(reporter, bundle, event); @@ -401,63 +294,29 @@ void shouldPassValidationWhenDeletingOrUpdatingSingleEvent(TrackerImportStrategy mode = EnumSource.Mode.INCLUDE, names = {"CREATE"}) void shouldPassValidationWhenCreatingSingleEvent(TrackerImportStrategy strategy) { - org.hisp.dhis.tracker.imports.domain.Event event = - org.hisp.dhis.tracker.imports.domain.SingleEvent.builder() - .event(UID.generate()) - .orgUnit(MetadataIdentifier.ofUid(ORG_UNIT_ID)) - .programStage(MetadataIdentifier.ofUid(PS_ID)) - .program(MetadataIdentifier.ofUid(PROGRAM_ID)) - .attributeOptionCombo(MetadataIdentifier.ofUid(categoryOptionCombo)) - .build(); - + org.hisp.dhis.tracker.imports.domain.Event event = singleEvent(); when(bundle.getStrategy(event)).thenReturn(strategy); - when(preheat.getProgramStage(event.getProgramStage())).thenReturn(programStage); - SingleEvent preheatEvent = getEvent(); - when(preheat.getSingleEvent(event.getEvent())).thenReturn(preheatEvent); - when(preheat.getOrganisationUnit(MetadataIdentifier.ofUid(ORG_UNIT_ID))) - .thenReturn(organisationUnit); - when(preheat.getCategoryOptionCombo(MetadataIdentifier.ofUid(categoryOptionCombo))) - .thenReturn(categoryOptionCombo); - - UserDetails userDetails = setUpUserWithOrgUnit(); - when(aclService.canDataWrite(userDetails, program)).thenReturn(true); - when(aclService.canDataWrite(userDetails, categoryOption)).thenReturn(true); + when(trackerAccessManager.canCreate(any(UserDetails.class), any(SingleEvent.class))) + .thenReturn(List.of()); validator.validate(reporter, bundle, event); assertIsEmpty(reporter.getErrors()); } - @Test - void shouldFailValidationWhenUpdatingCompletedEventAndUserHasNoAuthorityToUncompleteEvent() { - UID enrollmentUid = UID.generate(); - org.hisp.dhis.tracker.imports.domain.Event event = - org.hisp.dhis.tracker.imports.domain.SingleEvent.builder() - .event(UID.generate()) - .enrollment(enrollmentUid) - .orgUnit(MetadataIdentifier.ofUid(ORG_UNIT_ID)) - .programStage(MetadataIdentifier.ofUid(PS_ID)) - .program(MetadataIdentifier.ofUid(PROGRAM_ID)) - .status(EventStatus.ACTIVE) - .build(); - - when(bundle.getStrategy(event)).thenReturn(TrackerImportStrategy.UPDATE); - SingleEvent preheatEvent = getEvent(); - when(preheat.getSingleEvent(event.getEvent())).thenReturn(preheatEvent); - - when(aclService.canDataWrite(user, program)).thenReturn(true); - when(bundle.getPreheat().getOrganisationUnit(event.getOrgUnit())).thenReturn(organisationUnit); - - validator.validate(reporter, bundle, event); - - assertHasError(reporter, event, E1083); + private org.hisp.dhis.tracker.imports.domain.Event singleEvent() { + return org.hisp.dhis.tracker.imports.domain.SingleEvent.builder() + .event(UID.generate()) + .orgUnit(MetadataIdentifier.ofUid(ORG_UNIT_ID)) + .programStage(MetadataIdentifier.ofUid(PS_ID)) + .program(MetadataIdentifier.ofUid(PROGRAM_ID)) + .attributeOptionCombo(MetadataIdentifier.EMPTY_UID) + .build(); } - private SingleEvent getEvent() { + private SingleEvent dbSingleEvent() { SingleEvent event = new SingleEvent(); - event.setProgramStage(programStage); event.setOrganisationUnit(organisationUnit); - event.setStatus(EventStatus.COMPLETED); return event; } } From ba9c648be62b00ec390c41b8c6d34fd55fb51e0f Mon Sep 17 00:00:00 2001 From: Marc Date: Tue, 28 Apr 2026 15:59:45 +0200 Subject: [PATCH 2/5] chore: Reuse user scope validation methods [DHIS2-20158] --- .../dhis/tracker/acl/DefaultTrackerAccessManager.java | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerAccessManager.java b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerAccessManager.java index 54bb6993c0ac..7ca7489ca45d 100644 --- a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerAccessManager.java +++ b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerAccessManager.java @@ -610,15 +610,9 @@ private void checkOrgUnitInScope( } if (program.isClosed() || program.isProtected()) { - if (!user.isInUserHierarchy(orgUnit.getStoredPath())) { - errors.add( - new ErrorMessage(E1000, user.getUid(), List.of(user.getUid(), orgUnit.getUid()))); - } + checkOrgUnitInCaptureScope(errors, user, orgUnit); } else { - if (!user.isInUserEffectiveSearchOrgUnitHierarchy(orgUnit.getStoredPath())) { - errors.add( - new ErrorMessage(E1105, user.getUid(), List.of(user.getUid(), orgUnit.getUid()))); - } + checkOrgUnitInSearchScope(errors, user, orgUnit); } } From a947c4032c4d463bcedfac4375fbc0b67a8bfa71 Mon Sep 17 00:00:00 2001 From: Marc Date: Wed, 6 May 2026 12:02:59 +0200 Subject: [PATCH 3/5] chore: Validate single event AOC on update and delete [DHIS2-20158] --- .../event/SecuritySingleEventValidator.java | 10 ++- .../SecuritySingleEventValidatorTest.java | 82 +++++++++++++++++++ 2 files changed, 89 insertions(+), 3 deletions(-) diff --git a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecuritySingleEventValidator.java b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecuritySingleEventValidator.java index 32d419afeb6b..426f6656fdc3 100644 --- a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecuritySingleEventValidator.java +++ b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecuritySingleEventValidator.java @@ -38,6 +38,7 @@ import org.hisp.dhis.tracker.acl.TrackerAccessManager; import org.hisp.dhis.tracker.imports.TrackerImportStrategy; import org.hisp.dhis.tracker.imports.bundle.TrackerBundle; +import org.hisp.dhis.tracker.imports.domain.MetadataIdentifier; import org.hisp.dhis.tracker.imports.domain.SingleEvent; import org.hisp.dhis.tracker.imports.validation.Reporter; import org.hisp.dhis.tracker.imports.validation.Validator; @@ -64,11 +65,14 @@ public void validate( } else { org.hisp.dhis.tracker.model.SingleEvent databaseSingleEvent = bundle.getPreheat().getSingleEvent(singleEvent.getUID()); - CategoryOptionCombo aoc = - bundle.getPreheat().getCategoryOptionCombo(singleEvent.getAttributeOptionCombo()); - databaseSingleEvent.setAttributeOptionCombo(aoc); if (strategy.isUpdate()) { + MetadataIdentifier singleEventAoc = singleEvent.getAttributeOptionCombo(); + if (singleEventAoc != null && singleEventAoc.isNotBlank()) { + CategoryOptionCombo aoc = bundle.getPreheat().getCategoryOptionCombo(singleEventAoc); + databaseSingleEvent.setAttributeOptionCombo(aoc); + } + handleUpdate(reporter, bundle, databaseSingleEvent, singleEvent); } else if (strategy.isDelete()) { handleDelete(reporter, bundle, databaseSingleEvent, singleEvent); diff --git a/dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecuritySingleEventValidatorTest.java b/dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecuritySingleEventValidatorTest.java index f61362072114..881d90962cc0 100644 --- a/dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecuritySingleEventValidatorTest.java +++ b/dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecuritySingleEventValidatorTest.java @@ -34,11 +34,14 @@ import static org.hisp.dhis.tracker.imports.validation.ValidationCode.E1091; import static org.hisp.dhis.tracker.imports.validation.ValidationCode.E1099; import static org.hisp.dhis.tracker.imports.validation.validator.AssertValidations.assertHasError; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.util.List; +import org.hisp.dhis.category.CategoryOptionCombo; import org.hisp.dhis.common.UID; import org.hisp.dhis.organisationunit.OrganisationUnit; import org.hisp.dhis.program.Program; @@ -56,9 +59,11 @@ import org.hisp.dhis.tracker.test.TrackerTestBase; import org.hisp.dhis.user.UserDetails; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.EnumSource; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; @@ -304,6 +309,83 @@ void shouldPassValidationWhenCreatingSingleEvent(TrackerImportStrategy strategy) assertIsEmpty(reporter.getErrors()); } + @Test + void shouldPassDatabaseAocToCanUpdateWhenPayloadOmitsAoc() { + org.hisp.dhis.tracker.imports.domain.Event event = singleEvent(); + when(bundle.getStrategy(event)).thenReturn(TrackerImportStrategy.UPDATE); + CategoryOptionCombo dbAoc = createCategoryOptionCombo('A'); + SingleEvent dbEvent = dbSingleEvent(); + dbEvent.setAttributeOptionCombo(dbAoc); + when(preheat.getSingleEvent(event.getEvent())).thenReturn(dbEvent); + when(trackerAccessManager.canUpdate( + any(UserDetails.class), any(SingleEvent.class), any(OrganisationUnit.class))) + .thenReturn(List.of()); + + validator.validate(reporter, bundle, event); + + ArgumentCaptor captor = ArgumentCaptor.forClass(SingleEvent.class); + verify(trackerAccessManager) + .canUpdate(any(UserDetails.class), captor.capture(), any(OrganisationUnit.class)); + assertEquals(dbAoc, captor.getValue().getAttributeOptionCombo()); + } + + @Test + void shouldPassPayloadAocToCanUpdateWhenPayloadSpecifiesAoc() { + CategoryOptionCombo payloadAoc = createCategoryOptionCombo('B'); + MetadataIdentifier aocId = MetadataIdentifier.ofUid(payloadAoc.getUid()); + org.hisp.dhis.tracker.imports.domain.Event event = + org.hisp.dhis.tracker.imports.domain.SingleEvent.builder() + .event(UID.generate()) + .orgUnit(MetadataIdentifier.ofUid(ORG_UNIT_ID)) + .programStage(MetadataIdentifier.ofUid(PS_ID)) + .program(MetadataIdentifier.ofUid(PROGRAM_ID)) + .attributeOptionCombo(aocId) + .build(); + when(bundle.getStrategy(event)).thenReturn(TrackerImportStrategy.UPDATE); + CategoryOptionCombo dbAoc = createCategoryOptionCombo('A'); + SingleEvent dbEvent = dbSingleEvent(); + dbEvent.setAttributeOptionCombo(dbAoc); + when(preheat.getSingleEvent(event.getEvent())).thenReturn(dbEvent); + when(preheat.getCategoryOptionCombo(aocId)).thenReturn(payloadAoc); + when(trackerAccessManager.canUpdate( + any(UserDetails.class), any(SingleEvent.class), any(OrganisationUnit.class))) + .thenReturn(List.of()); + + validator.validate(reporter, bundle, event); + + ArgumentCaptor captor = ArgumentCaptor.forClass(SingleEvent.class); + verify(trackerAccessManager) + .canUpdate(any(UserDetails.class), captor.capture(), any(OrganisationUnit.class)); + assertEquals(payloadAoc, captor.getValue().getAttributeOptionCombo()); + } + + @Test + void shouldPassDatabaseAocToCanDeleteRegardlessOfPayloadAoc() { + CategoryOptionCombo payloadAoc = createCategoryOptionCombo('B'); + MetadataIdentifier aocId = MetadataIdentifier.ofUid(payloadAoc.getUid()); + org.hisp.dhis.tracker.imports.domain.Event event = + org.hisp.dhis.tracker.imports.domain.SingleEvent.builder() + .event(UID.generate()) + .orgUnit(MetadataIdentifier.ofUid(ORG_UNIT_ID)) + .programStage(MetadataIdentifier.ofUid(PS_ID)) + .program(MetadataIdentifier.ofUid(PROGRAM_ID)) + .attributeOptionCombo(aocId) + .build(); + when(bundle.getStrategy(event)).thenReturn(TrackerImportStrategy.DELETE); + CategoryOptionCombo dbAoc = createCategoryOptionCombo('A'); + SingleEvent dbEvent = dbSingleEvent(); + dbEvent.setAttributeOptionCombo(dbAoc); + when(preheat.getSingleEvent(event.getEvent())).thenReturn(dbEvent); + when(trackerAccessManager.canDelete(any(UserDetails.class), any(SingleEvent.class))) + .thenReturn(List.of()); + + validator.validate(reporter, bundle, event); + + ArgumentCaptor captor = ArgumentCaptor.forClass(SingleEvent.class); + verify(trackerAccessManager).canDelete(any(UserDetails.class), captor.capture()); + assertEquals(dbAoc, captor.getValue().getAttributeOptionCombo()); + } + private org.hisp.dhis.tracker.imports.domain.Event singleEvent() { return org.hisp.dhis.tracker.imports.domain.SingleEvent.builder() .event(UID.generate()) From d1685a0518f2f4c4957d84d2835a9bcffada8825 Mon Sep 17 00:00:00 2001 From: Marc Date: Wed, 6 May 2026 17:22:19 +0200 Subject: [PATCH 4/5] chore: Validate both AOC on single event update [DHIS2-20158] --- .../tracker/acl/TrackerAccessManagerTest.java | 108 ++++++++++++++++++ .../acl/DefaultTrackerAccessManager.java | 9 +- .../tracker/acl/TrackerAccessManager.java | 35 +++++- .../event/SecuritySingleEventValidator.java | 14 +-- .../SecuritySingleEventValidatorTest.java | 58 +++++++--- 5 files changed, 201 insertions(+), 23 deletions(-) diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/acl/TrackerAccessManagerTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/acl/TrackerAccessManagerTest.java index bc71f537aebe..b9a9f479d9c5 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/acl/TrackerAccessManagerTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/tracker/acl/TrackerAccessManagerTest.java @@ -30,6 +30,7 @@ package org.hisp.dhis.tracker.acl; import static org.hisp.dhis.security.acl.AccessStringHelper.CATEGORY_NO_DATA_SHARING_DEFAULT; +import static org.hisp.dhis.security.acl.AccessStringHelper.CATEGORY_OPTION_DEFAULT; import static org.hisp.dhis.test.utils.Assertions.assertIsEmpty; import static org.hisp.dhis.tracker.imports.validation.ValidationCode.E1000; import static org.hisp.dhis.tracker.imports.validation.ValidationCode.E1099; @@ -51,6 +52,8 @@ import java.util.Set; import java.util.function.BiFunction; import org.apache.commons.lang3.time.DateUtils; +import org.hisp.dhis.category.Category; +import org.hisp.dhis.category.CategoryCombo; import org.hisp.dhis.category.CategoryOption; import org.hisp.dhis.category.CategoryOptionCombo; import org.hisp.dhis.category.CategoryService; @@ -75,6 +78,7 @@ import org.hisp.dhis.tracker.export.trackedentity.TrackedEntityService; import org.hisp.dhis.tracker.imports.validation.ValidationCode; import org.hisp.dhis.tracker.model.Enrollment; +import org.hisp.dhis.tracker.model.SingleEvent; import org.hisp.dhis.tracker.model.TrackedEntity; import org.hisp.dhis.tracker.model.TrackerEvent; import org.hisp.dhis.user.User; @@ -122,6 +126,16 @@ class TrackerAccessManagerTest extends PostgresIntegrationTestBase { private TrackerEvent eventB; + private SingleEvent singleEvent; + + private CategoryOption oldCatOption; + + private CategoryOption newCatOption; + + private CategoryOptionCombo oldAoc; + + private CategoryOptionCombo newAoc; + @BeforeEach void setUp() { CategoryOptionCombo coA = categoryService.getDefaultCategoryOptionCombo(); @@ -203,6 +217,51 @@ void setUp() { eventB.setAttributeOptionCombo(coA); manager.save(eventB, false); + oldCatOption = createCategoryOption('M'); + categoryService.addCategoryOption(oldCatOption); + oldCatOption.getSharing().setPublicAccess(CATEGORY_OPTION_DEFAULT); + categoryService.updateCategoryOption(oldCatOption); + + newCatOption = createCategoryOption('N'); + categoryService.addCategoryOption(newCatOption); + newCatOption.getSharing().setPublicAccess(CATEGORY_OPTION_DEFAULT); + categoryService.updateCategoryOption(newCatOption); + + Category singleEventCategory = createCategory('M', oldCatOption, newCatOption); + categoryService.addCategory(singleEventCategory); + + CategoryCombo singleEventCategoryCombo = createCategoryCombo('M', singleEventCategory); + categoryService.addCategoryCombo(singleEventCategoryCombo); + + oldAoc = createCategoryOptionCombo(singleEventCategoryCombo, oldCatOption); + categoryService.addCategoryOptionCombo(oldAoc); + oldCatOption.getCategoryOptionCombos().add(oldAoc); + categoryService.updateCategoryOption(oldCatOption); + + newAoc = createCategoryOptionCombo(singleEventCategoryCombo, newCatOption); + categoryService.addCategoryOptionCombo(newAoc); + newCatOption.getCategoryOptionCombos().add(newAoc); + categoryService.updateCategoryOption(newCatOption); + + ProgramStage singleEventProgramStage = createProgramStage('C', 0); + manager.save(singleEventProgramStage); + + Program singleEventProgram = createProgram('C', new HashSet<>(), orgUnitA); + singleEventProgram.setProgramType(ProgramType.WITHOUT_REGISTRATION); + singleEventProgramStage.setProgram(singleEventProgram); + singleEventProgram.getProgramStages().add(singleEventProgramStage); + manager.save(singleEventProgram); + singleEventProgram.setPublicAccess(AccessStringHelper.DATA_READ_WRITE); + manager.update(singleEventProgram); + manager.update(singleEventProgramStage); + + singleEvent = new SingleEvent(); + singleEvent.setProgramStage(singleEventProgramStage); + singleEvent.setOrganisationUnit(orgUnitA); + singleEvent.setAttributeOptionCombo(oldAoc); + singleEvent.setOccurredDate(new Date()); + manager.save(singleEvent); + User adminUser = getAdminUser(); adminUser.setTeiSearchOrganisationUnits(Set.of(orgUnitA, orgUnitB)); adminUser.setOrganisationUnits(Set.of(orgUnitA)); @@ -511,6 +570,55 @@ void shouldFailToDeleteEnrollmentWhenUserLacksAccess() { (userDetails, enrollment) -> trackerAccessManager.canDelete(userDetails, enrollment)); } + @Test + void shouldPassWhenUpdatingSingleEventWithAccessToBothOldAndNewAoc() { + User user = createUserWithAuth("user1").setOrganisationUnits(Sets.newHashSet(orgUnitA)); + + assertNoErrors(trackerAccessManager.canUpdate(fromUser(user), singleEvent, orgUnitA, newAoc)); + } + + @Test + void shouldFailWhenUpdatingSingleEventWithAccessOnlyToNewAoc() { + oldCatOption.getSharing().setPublicAccess(CATEGORY_NO_DATA_SHARING_DEFAULT); + manager.update(oldCatOption); + + User user = createUserWithAuth("user1").setOrganisationUnits(Sets.newHashSet(orgUnitA)); + + assertHasErrorMessage( + trackerAccessManager.canUpdate(fromUser(user), singleEvent, orgUnitA, newAoc), E1099); + } + + @Test + void shouldFailWhenUpdatingSingleEventWithAccessOnlyToOldAoc() { + newCatOption.getSharing().setPublicAccess(CATEGORY_NO_DATA_SHARING_DEFAULT); + manager.update(newCatOption); + + User user = createUserWithAuth("user1").setOrganisationUnits(Sets.newHashSet(orgUnitA)); + + assertHasErrorMessage( + trackerAccessManager.canUpdate(fromUser(user), singleEvent, orgUnitA, newAoc), E1099); + } + + @Test + void shouldPassWhenUpdatingSingleEventWithNoAocChangeAndUserHasAccess() { + User user = createUserWithAuth("user1").setOrganisationUnits(Sets.newHashSet(orgUnitA)); + + assertNoErrors(trackerAccessManager.canUpdate(fromUser(user), singleEvent, orgUnitA, oldAoc)); + } + + @Test + void shouldFailWhenUpdatingSingleEventWithNoAccessToEitherAoc() { + oldCatOption.getSharing().setPublicAccess(CATEGORY_NO_DATA_SHARING_DEFAULT); + manager.update(oldCatOption); + newCatOption.getSharing().setPublicAccess(CATEGORY_NO_DATA_SHARING_DEFAULT); + manager.update(newCatOption); + + User user = createUserWithAuth("user1").setOrganisationUnits(Sets.newHashSet(orgUnitA)); + + assertHasErrors( + 2, trackerAccessManager.canUpdate(fromUser(user), singleEvent, orgUnitA, newAoc)); + } + private void assertEnrollmentCategoryOptionAccessFails( BiFunction> accessCheck) { TrackedEntity trackedEntity = manager.get(TrackedEntity.class, trackedEntityA.getUid()); diff --git a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerAccessManager.java b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerAccessManager.java index 7ca7489ca45d..69dafbabcda6 100644 --- a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerAccessManager.java +++ b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/acl/DefaultTrackerAccessManager.java @@ -345,7 +345,10 @@ public List canCreate(@Nonnull UserDetails user, @Nonnull SingleEv @Override public List canUpdate( - @Nonnull UserDetails user, @Nonnull SingleEvent event, @Nonnull OrganisationUnit orgUnit) { + @Nonnull UserDetails user, + @Nonnull SingleEvent event, + @Nonnull OrganisationUnit orgUnit, + @Nonnull CategoryOptionCombo categoryOptionCombo) { if (user.isSuper()) { return List.of(); } @@ -355,6 +358,10 @@ public List canUpdate( checkOrgUnitInCaptureScope(errors, user, orgUnit); } + if (!categoryOptionCombo.getUid().equals(event.getAttributeOptionCombo().getUid())) { + checkDataWriteAccessToCategoryOptionCombo(errors, user, categoryOptionCombo); + } + return errors; } diff --git a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/acl/TrackerAccessManager.java b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/acl/TrackerAccessManager.java index 7a6fb8e40025..81711dc05ecf 100644 --- a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/acl/TrackerAccessManager.java +++ b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/acl/TrackerAccessManager.java @@ -31,6 +31,7 @@ import java.util.List; import javax.annotation.Nonnull; +import org.hisp.dhis.category.CategoryOptionCombo; import org.hisp.dhis.organisationunit.OrganisationUnit; import org.hisp.dhis.tracker.model.Enrollment; import org.hisp.dhis.tracker.model.Relationship; @@ -147,13 +148,45 @@ List canUpdate( /** Like {@link #canCreate(UserDetails, TrackerEvent)}. */ List canDelete(UserDetails user, TrackerEvent event); + /** + * Checks org unit scope access, data read access to the program, and data read access to the + * category option combo. + * + * @return No errors if the user has org unit scope access, data read access to the program, and + * data read access to the category option combo. + */ List canRead(UserDetails user, SingleEvent event); + /** + * Checks capture scope access, data write access to the program, and data write access to the + * category option combo. + * + * @return No errors if the user has all required access rights to create the event. + */ List canCreate(UserDetails user, SingleEvent event); + /** + * Checks capture scope access, data write access to the program, and data write access to the + * stored event's category option combo. When {@code orgUnit} differs from the stored event's org + * unit, capture scope access to it is also required. When {@code categoryOptionCombo} differs + * from the stored event's category option combo, data write access to it is also required. + * + * @param user the user whose access is being validated. + * @param event the stored event to update. + * @param orgUnit the org unit the caller intends to move the entity to; if no org unit change is + * intended, pass the entity's existing org unit. + * @param categoryOptionCombo the category option combo the caller intends to set on the event; if + * no category option combo change is intended, pass the entity's existing category option + * combo. + * @return No errors if the user has all required access rights to update the event. + */ List canUpdate( - @Nonnull UserDetails user, SingleEvent event, @Nonnull OrganisationUnit orgUnit); + @Nonnull UserDetails user, + SingleEvent event, + @Nonnull OrganisationUnit orgUnit, + @Nonnull CategoryOptionCombo categoryOptionCombo); + /** Like {@link #canCreate(UserDetails, SingleEvent)}. */ List canDelete(@Nonnull UserDetails user, SingleEvent event); List canRead(UserDetails user, Relationship relationship); diff --git a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecuritySingleEventValidator.java b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecuritySingleEventValidator.java index 426f6656fdc3..06f5343f0061 100644 --- a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecuritySingleEventValidator.java +++ b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecuritySingleEventValidator.java @@ -38,7 +38,6 @@ import org.hisp.dhis.tracker.acl.TrackerAccessManager; import org.hisp.dhis.tracker.imports.TrackerImportStrategy; import org.hisp.dhis.tracker.imports.bundle.TrackerBundle; -import org.hisp.dhis.tracker.imports.domain.MetadataIdentifier; import org.hisp.dhis.tracker.imports.domain.SingleEvent; import org.hisp.dhis.tracker.imports.validation.Reporter; import org.hisp.dhis.tracker.imports.validation.Validator; @@ -67,12 +66,6 @@ public void validate( bundle.getPreheat().getSingleEvent(singleEvent.getUID()); if (strategy.isUpdate()) { - MetadataIdentifier singleEventAoc = singleEvent.getAttributeOptionCombo(); - if (singleEventAoc != null && singleEventAoc.isNotBlank()) { - CategoryOptionCombo aoc = bundle.getPreheat().getCategoryOptionCombo(singleEventAoc); - databaseSingleEvent.setAttributeOptionCombo(aoc); - } - handleUpdate(reporter, bundle, databaseSingleEvent, singleEvent); } else if (strategy.isDelete()) { handleDelete(reporter, bundle, databaseSingleEvent, singleEvent); @@ -99,8 +92,13 @@ private void handleUpdate( OrganisationUnit orgUnit = payloadOrgUnit != null ? payloadOrgUnit : databaseSingleEvent.getOrganisationUnit(); + CategoryOptionCombo payloadAoc = + bundle.getPreheat().getCategoryOptionCombo(singleEvent.getAttributeOptionCombo()); + CategoryOptionCombo aoc = + payloadAoc != null ? payloadAoc : databaseSingleEvent.getAttributeOptionCombo(); + trackerAccessManager - .canUpdate(bundle.getUser(), databaseSingleEvent, orgUnit) + .canUpdate(bundle.getUser(), databaseSingleEvent, orgUnit, aoc) .forEach(em -> reporter.addError(singleEvent, em.validationCode(), em.args().toArray())); } diff --git a/dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecuritySingleEventValidatorTest.java b/dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecuritySingleEventValidatorTest.java index 881d90962cc0..2d9aae648b0a 100644 --- a/dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecuritySingleEventValidatorTest.java +++ b/dhis-2/dhis-tracker/src/test/java/org/hisp/dhis/tracker/imports/validation/validator/event/SecuritySingleEventValidatorTest.java @@ -148,7 +148,10 @@ void shouldFailValidationWhenUserDoesNotHaveOrgUnitInCaptureScopeForUpdateAndDel lenient() .when( trackerAccessManager.canUpdate( - any(UserDetails.class), any(SingleEvent.class), any(OrganisationUnit.class))) + any(UserDetails.class), + any(SingleEvent.class), + any(OrganisationUnit.class), + any(CategoryOptionCombo.class))) .thenReturn( List.of(new ErrorMessage(E1000, user.getUid(), List.of(user.getUid(), ORG_UNIT_ID)))); lenient() @@ -173,7 +176,10 @@ void shouldFailValidationWhenUserDoesNotHavePayloadOrgUnitInCaptureScopeForUpdat when(preheat.getSingleEvent(event.getEvent())).thenReturn(dbSingleEvent()); when(preheat.getOrganisationUnit(event.getOrgUnit())).thenReturn(organisationUnit); when(trackerAccessManager.canUpdate( - any(UserDetails.class), any(SingleEvent.class), any(OrganisationUnit.class))) + any(UserDetails.class), + any(SingleEvent.class), + any(OrganisationUnit.class), + any(CategoryOptionCombo.class))) .thenReturn( List.of(new ErrorMessage(E1000, user.getUid(), List.of(user.getUid(), ORG_UNIT_ID)))); @@ -213,7 +219,10 @@ void shouldFailValidationWhenUserDoNotHaveWriteAccessToProgramForUpdateAndDelete lenient() .when( trackerAccessManager.canUpdate( - any(UserDetails.class), any(SingleEvent.class), any(OrganisationUnit.class))) + any(UserDetails.class), + any(SingleEvent.class), + any(OrganisationUnit.class), + any(CategoryOptionCombo.class))) .thenReturn( List.of(new ErrorMessage(E1091, user.getUid(), List.of(user.getUid(), PROGRAM_ID)))); lenient() @@ -257,7 +266,10 @@ void shouldFailValidationWhenUserDoNotHaveWriteAccessToCategoryOptionForUpdateAn lenient() .when( trackerAccessManager.canUpdate( - any(UserDetails.class), any(SingleEvent.class), any(OrganisationUnit.class))) + any(UserDetails.class), + any(SingleEvent.class), + any(OrganisationUnit.class), + any(CategoryOptionCombo.class))) .thenReturn( List.of(new ErrorMessage(E1099, user.getUid(), List.of(user.getUid(), "catOptUid")))); lenient() @@ -282,7 +294,10 @@ void shouldPassValidationWhenDeletingOrUpdatingSingleEvent(TrackerImportStrategy lenient() .when( trackerAccessManager.canUpdate( - any(UserDetails.class), any(SingleEvent.class), any(OrganisationUnit.class))) + any(UserDetails.class), + any(SingleEvent.class), + any(OrganisationUnit.class), + any(CategoryOptionCombo.class))) .thenReturn(List.of()); lenient() .when(trackerAccessManager.canDelete(any(UserDetails.class), any(SingleEvent.class))) @@ -318,15 +333,23 @@ void shouldPassDatabaseAocToCanUpdateWhenPayloadOmitsAoc() { dbEvent.setAttributeOptionCombo(dbAoc); when(preheat.getSingleEvent(event.getEvent())).thenReturn(dbEvent); when(trackerAccessManager.canUpdate( - any(UserDetails.class), any(SingleEvent.class), any(OrganisationUnit.class))) + any(UserDetails.class), + any(SingleEvent.class), + any(OrganisationUnit.class), + any(CategoryOptionCombo.class))) .thenReturn(List.of()); validator.validate(reporter, bundle, event); - ArgumentCaptor captor = ArgumentCaptor.forClass(SingleEvent.class); + ArgumentCaptor aocCaptor = + ArgumentCaptor.forClass(CategoryOptionCombo.class); verify(trackerAccessManager) - .canUpdate(any(UserDetails.class), captor.capture(), any(OrganisationUnit.class)); - assertEquals(dbAoc, captor.getValue().getAttributeOptionCombo()); + .canUpdate( + any(UserDetails.class), + any(SingleEvent.class), + any(OrganisationUnit.class), + aocCaptor.capture()); + assertEquals(dbAoc, aocCaptor.getValue()); } @Test @@ -348,15 +371,23 @@ void shouldPassPayloadAocToCanUpdateWhenPayloadSpecifiesAoc() { when(preheat.getSingleEvent(event.getEvent())).thenReturn(dbEvent); when(preheat.getCategoryOptionCombo(aocId)).thenReturn(payloadAoc); when(trackerAccessManager.canUpdate( - any(UserDetails.class), any(SingleEvent.class), any(OrganisationUnit.class))) + any(UserDetails.class), + any(SingleEvent.class), + any(OrganisationUnit.class), + any(CategoryOptionCombo.class))) .thenReturn(List.of()); validator.validate(reporter, bundle, event); - ArgumentCaptor captor = ArgumentCaptor.forClass(SingleEvent.class); + ArgumentCaptor aocCaptor = + ArgumentCaptor.forClass(CategoryOptionCombo.class); verify(trackerAccessManager) - .canUpdate(any(UserDetails.class), captor.capture(), any(OrganisationUnit.class)); - assertEquals(payloadAoc, captor.getValue().getAttributeOptionCombo()); + .canUpdate( + any(UserDetails.class), + any(SingleEvent.class), + any(OrganisationUnit.class), + aocCaptor.capture()); + assertEquals(payloadAoc, aocCaptor.getValue()); } @Test @@ -399,6 +430,7 @@ private org.hisp.dhis.tracker.imports.domain.Event singleEvent() { private SingleEvent dbSingleEvent() { SingleEvent event = new SingleEvent(); event.setOrganisationUnit(organisationUnit); + event.setAttributeOptionCombo(createCategoryOptionCombo('Z')); return event; } } From 710a0525b53c77dcbdf8db25f2f1f6299adff373 Mon Sep 17 00:00:00 2001 From: Marc Date: Wed, 6 May 2026 18:30:13 +0200 Subject: [PATCH 5/5] chore: Validate both AOC on single event update [DHIS2-20158] --- .../org/hisp/dhis/tracker/acl/TrackerAccessManager.java | 8 ++++---- .../imports/preheat/mappers/SingleEventMapper.java | 9 ++++++++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/acl/TrackerAccessManager.java b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/acl/TrackerAccessManager.java index 81711dc05ecf..574b5f324861 100644 --- a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/acl/TrackerAccessManager.java +++ b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/acl/TrackerAccessManager.java @@ -181,13 +181,13 @@ List canUpdate( * @return No errors if the user has all required access rights to update the event. */ List canUpdate( - @Nonnull UserDetails user, + UserDetails user, SingleEvent event, - @Nonnull OrganisationUnit orgUnit, - @Nonnull CategoryOptionCombo categoryOptionCombo); + OrganisationUnit orgUnit, + CategoryOptionCombo categoryOptionCombo); /** Like {@link #canCreate(UserDetails, SingleEvent)}. */ - List canDelete(@Nonnull UserDetails user, SingleEvent event); + List canDelete(UserDetails user, SingleEvent event); List canRead(UserDetails user, Relationship relationship); diff --git a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/preheat/mappers/SingleEventMapper.java b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/preheat/mappers/SingleEventMapper.java index bbe64251af5d..0b01a3ba694a 100644 --- a/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/preheat/mappers/SingleEventMapper.java +++ b/dhis-2/dhis-tracker/src/main/java/org/hisp/dhis/tracker/imports/preheat/mappers/SingleEventMapper.java @@ -35,7 +35,13 @@ import org.mapstruct.Mapping; import org.mapstruct.factory.Mappers; -@Mapper(uses = {ProgramStageMapper.class, OrganisationUnitMapper.class, EnrollmentMapper.class}) +@Mapper( + uses = { + ProgramStageMapper.class, + OrganisationUnitMapper.class, + EnrollmentMapper.class, + CategoryOptionComboMapper.class + }) public interface SingleEventMapper extends PreheatMapper { SingleEventMapper INSTANCE = Mappers.getMapper(SingleEventMapper.class); @@ -48,6 +54,7 @@ public interface SingleEventMapper extends PreheatMapper { @Mapping(target = "programStage") @Mapping(target = "status") @Mapping(target = "organisationUnit") + @Mapping(target = "attributeOptionCombo") @Mapping(target = "created") @Mapping(target = "eventDataValues") @Mapping(target = "notes")