chore: Refactor ACL validation of single events [DHIS2-20158]#23752
Conversation
| org.hisp.dhis.tracker.model.SingleEvent databaseSingleEvent = | ||
| bundle.getPreheat().getSingleEvent(singleEvent.getUID()); | ||
| CategoryOptionCombo aoc = | ||
| bundle.getPreheat().getCategoryOptionCombo(singleEvent.getAttributeOptionCombo()); |
There was a problem hiding this comment.
We cannot assume (and we shouldn't) that a payload for delete has no attributeOptionCombo defined.
I think this logic of replacing the attributeOptionCombo only applies to update strategy.
This is working in most cases but I found it confusing. canDelete in TrackerAccessManager doesn't need to check attributeOptionCombo write access and indeed it doesn't happen but only because it was null in the payload.
It is actually an issue if the client send a payload for delete and it includes the attributeOptionCombo . In this case attributeOptionCombo is going to be validated and is not passing any other validation, so it could be not accesssible for the user.
There was a problem hiding this comment.
We should double check this also on tracker event validator
There was a problem hiding this comment.
Right, I’ll ignore the payload AOC for deletions, but we still need to validate access to it. A user’s access may change between the time the event was created and when they try to delete it.
Tracker events and enrollments I'll do in a different PR.
|
enricocolasante
left a comment
enricocolasante
left a comment
There was a problem hiding this comment.
We will need to re-check tracker event logic for AOC and maybe even enrollments
this is done here |
3 participants
Refactors
SecuritySingleEventValidatorto delegate all access checks toTrackerAccessManager, splits uncomplete event handling into a dedicatedCompletedSingleEventValidator, mirroring the existingCompletedTrackerEventValidator.The old
canCreate(SingleEvent)only checked program and category option write access, it did not perform any org unit unit check. That particular check was previously handled inSecuritySingleEventValidator, but not inTrackerAccessManager. This is now handled in the newvalidateSingleEventAccessmethod.It also migrates the
SingleEventaccess manager methods fromList<String>toList<ErrorMessage>to align with other entity types.Next and last is relationships.