Skip to content

Authorization-boundary hardening follow-ups (from security review) #6334

Description

@dkayiwa

While hardening the recent batch of authorization fixes, a few authorization-boundary observations came up that are broader than any single advisory and worth tracking as hardening work. None is a regression from the recent fixes; all are pre-existing. Filing as a tracking issue (no exploit detail); happy to split these or convert any item to a private advisory if preferred.

1. Service-layer @Authorized does not cover domain-object getters

AuthorizationAdvice enforces @Authorized only on service beans (AOP proxies). Navigating a loaded domain object reaches related data without the corresponding service privilege — e.g. Encounter.getAllergies() exposes an encounter's allergies to a caller holding only encounter privileges, without Get Allergies; the same pattern applies to other lazy relations. This is inherent to the current model. Worth deciding which navigations, if any, should be guarded and documenting the boundary.

2. Generic getRefByUuid path does not go through per-service @Authorized

RefByUuid.getRefByUuid / DomainServiceImpl.fetch (used by ~20 services) resolve an entity by uuid by delegating to that service's own getXByUuid, but via an internal this. call that does not re-enter the AOP proxy — so the @Authorized on those getXByUuid methods is not applied when an entity is reached through getRefByUuid. This is externally reachable through JSON deserialization: UuidReferenceModule resolves every OpenmrsObject-typed field via DomainServiceImpl.fetchByUuid, so deserializing a payload that references such an object by uuid resolves it without the per-service read privilege. Confirmed for PatientProgram / PatientState / PatientProgramAttribute while verifying #6318 (whose direct-invocation @Authorized is correct, but does not cover this path); the same applies to every RefByUuid implementation (Patient, Person, Order, Obs, Encounter, …). Consider enforcing authorization on the generic path (or restructuring so the delegated call goes through the proxy) so it matches the per-service read guards.

3. Minor: DB-update wizard progress endpoint

UpdateFilter's progress AJAX endpoint returns update status/warnings/log path without an auth check during the DB-upgrade window, and the filter does not rotate the session id on successful authentication. Low severity (transient, pre-startup); noted for completeness.

Context: surfaced during review of the recent authorization-annotation fixes.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions