Conversation
| <groupId>org.openmrs.module</groupId> | ||
| <artifactId>legacyui-omod</artifactId> | ||
| <version>1.0</version> | ||
| <version>2.1.0</version> |
There was a problem hiding this comment.
Compiling against 2.1.0 is right (it's the first release with RequirePrivilege), but the runtime half of this fix is openmrs/openmrs-module-legacyui#268, which isn't in any legacyui release yet. The 2.1.0 DwrAuthorizationFilter builds its allowlist from legacyui's own config.xml only. I initialized the filter straight from the published 2.1.0 jar to check: 100 entries, all legacyui's own scripts, nothing from this module. So under legacyui 2.1.0 every DWRCohortBuilderService.* call keeps returning the RCM-124 403 even with this merged, and the annotations added here are never read. Nothing to change in the code. I'd just record the minimum legacyui version in the ticket and in the release notes of whatever module release carries this, and hold that release until the legacyui side ships, so nobody upgrades against legacyui 2.1.0 expecting the cohort builder back.
There was a problem hiding this comment.
I'm actually bumping the required version up to 2.2.0-SNAPSHOT @dkayiwa , just to ensure the fix is in and these endpoints work
There was a problem hiding this comment.
Sounds good @mseaton. 2.2.0-SNAPSHOT does include the LUI-212 fix (openmrs/openmrs-module-legacyui#268 merged earlier today), so compiling against it guarantees the annotations and the filter line up. Two things worth keeping on the radar for release time:
-
The maven-release-plugin will refuse to release this module while
legacyuiVersionpoints at a SNAPSHOT, so a legacyui 2.2.0 release needs to be cut first and the property bumped to it. Conveniently, that also enforces the "hold this release until the legacyui side ships" part automatically. -
The pom bump only covers compile time. In config.xml, legacyui is still just an
<aware_of_module>with no version constraint, so nothing at runtime stops the module from starting against legacyui 2.1.0, where the DWR endpoints keep returning the 403. Since legacyui is deliberately optional here, I'd still record legacyui >= 2.2.0 in the RCM-124 ticket and the release notes.
| </create> | ||
| <create creator="new" javascript="DWRCohortBuilderService"> | ||
| <param name="class" value="org.openmrs.web.dwr.DWRCohortBuilderService"/> | ||
| <include method="getPatients"/> |
There was a problem hiding this comment.
Once the LUI-212 filter is in play it fails open, so a method added to this list later without @RequirePrivilege ships silently unenforced at the request boundary (nothing throws, the only trace is a startup WARN in legacyui's log). The enforcement lives in legacyui, but the parity is testable in this repo: a small unit test could parse config.xml and assert that every <include>d method resolves to an annotated method on the declared class, and that the file keeps exactly one <dwr> block (WebModuleUtil merges only the first one into dwr-modules.xml, which is how the old second block here got silently dropped for years). Optional, but it would keep the next addition honest.
There was a problem hiding this comment.
Added DwrConfigXmlTest (00fdc0c): asserts config.xml has exactly one block, and that every d method resolves to a @RequirePrivilege-annotated method, mirroring DwrAuthorizationFilter's own scanDwrCreates/findMethodAnnotation logic. Verified it fails when either bug is reintroduced (duplicate block, or an unannotated method). Needed Java 8 (diamond operator, try-with-resources), so bumped the module's min compiler source/target from 1.5 to 1.8 in the same commit.
…to 8 Per PR review feedback: a DwrConfigXmlTest guards against the two bugs this ticket fixed - a second <dwr> block silently dropped by WebModuleUtil, and DWR methods exposed without @RequirePrivilege, which DwrAuthorizationFilter fails open on rather than rejecting. Requires Java 8 (diamond operator, try-with-resources), so the module's minimum compiler source/target is bumped from 1.5.
2 participants
No description provided.