diff --git a/api/pom.xml b/api/pom.xml index 9b1aae6..cd2ea54 100644 --- a/api/pom.xml +++ b/api/pom.xml @@ -11,57 +11,6 @@ API project for Reporting Compatibility - - - - org.openmrs.api - openmrs-api - jar - - - org.openmrs.api - openmrs-api - test-jar - - - org.openmrs.web - openmrs-web - jar - - - org.openmrs.web - openmrs-web - test-jar - - - org.openmrs.test - openmrs-test - pom - - - org.apache.tomcat - jasper - 6.0.47 - provided - - - org.openmrs.module - legacyui-api - 1.0 - provided - - - org.openmrs.module - legacyui-omod - 1.0 - provided - org.openmrs.simpleframework simple-xml diff --git a/omod/pom.xml b/omod/pom.xml index f978877..84f64d2 100644 --- a/omod/pom.xml +++ b/omod/pom.xml @@ -24,42 +24,25 @@ test-jar test - - - org.openmrs.api - openmrs-api - jar - - - org.openmrs.api - openmrs-api - test-jar - + org.openmrs.web openmrs-web + ${openMRSVersion} jar + provided org.openmrs.web openmrs-web + ${openMRSVersion} test-jar - - - org.openmrs.test - openmrs-test - pom - - - org.apache.tomcat - jasper - 6.0.47 - provided + test org.openmrs.module legacyui-omod - 1.0 + ${legacyuiVersion} provided diff --git a/api/src/main/java/org/openmrs/module/reportingcompatibility/extension/html/AdminList.java b/omod/src/main/java/org/openmrs/module/reportingcompatibility/extension/html/AdminList.java similarity index 97% rename from api/src/main/java/org/openmrs/module/reportingcompatibility/extension/html/AdminList.java rename to omod/src/main/java/org/openmrs/module/reportingcompatibility/extension/html/AdminList.java index 36f0410..916dbb3 100644 --- a/api/src/main/java/org/openmrs/module/reportingcompatibility/extension/html/AdminList.java +++ b/omod/src/main/java/org/openmrs/module/reportingcompatibility/extension/html/AdminList.java @@ -1,97 +1,97 @@ -/** - * The contents of this file are subject to the OpenMRS Public License - * Version 1.0 (the "License"); you may not use this file except in - * compliance with the License. You may obtain a copy of the License at - * http://license.openmrs.org - * - * Software distributed under the License is distributed on an "AS IS" - * basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See the - * License for the specific language governing rights and limitations - * under the License. - * - * Copyright (C) OpenMRS, LLC. All Rights Reserved. - */ -package org.openmrs.module.reportingcompatibility.extension.html; - -import java.util.LinkedHashMap; -import java.util.Map; - -import org.openmrs.api.context.Context; -import org.openmrs.module.Extension; -import org.openmrs.module.web.extension.AdministrationSectionExt; - -/** - * This class defines the links that will appear on the administration page - * - * This extension is enabled by defining (uncommenting) it in the - * /metadata/config.xml file. - */ -public class AdminList extends AdministrationSectionExt { - - /** - * @see org.openmrs.module.web.extension.AdministrationSectionExt#getMediaType() - */ - public Extension.MEDIA_TYPE getMediaType() { - return Extension.MEDIA_TYPE.html; - } - - /** - * @see AdministrationSectionExt#getRequiredPrivilege() - */ - @Override - public String getRequiredPrivilege() { - return "Run Reports,Manage Reports,View Reports,View Data Exports"; - } - - /** - * @see org.openmrs.module.web.extension.AdministrationSectionExt#getTitle() - */ - public String getTitle() { - return "reportingcompatibility.Report.header"; - } - - /** - * @see org.openmrs.module.web.extension.AdministrationSectionExt#getLinks() - */ - public Map getLinks() { - - Map map = new LinkedHashMap(); - - addLink(map, "Run Reports", "/admin/reports/runReport.list", "reportingcompatibility.Report.list.title"); - addLink(map, "Manage Reports", "/admin/reports/reportSchemaXml.list", "reportingcompatibility.Report.manage.title"); - addLink(map, "Manage Reports", "/admin/reports/reportMacros.form", "reportingcompatibility.Report.macros.title"); - addLink(map, "Add Report Objects,Edit Report Objects,Delete Report Objects,View Report Objects", - "/admin/reports/dataExport.list", "reportingcompatibility.DataExport.manage"); - addLink(map, "Add Report Objects,Edit Report Objects,Delete Report Objects,View Report Objects", - "/admin/reports/rowPerObsDataExport.list", "reportingcompatibility.RowPerObsDataExport.manage"); - addLink(map, "Add Cohorts,Edit Cohorts,Delete Cohorts,View Cohorts", "/admin/reports/cohorts.list", "Cohort.manage"); - addLink(map, "Add Patient Searches,Edit Patient Searches,Delete Patient Searches,View Patient Searches", - "/admin/reports/patientSearch.list", "reportingcompatibility.PatientSearch.manage"); - addLink(map, "Add Report Objects,Edit Report Objects,Delete Report Objects,View Report Objects", - "/admin/reports/reportObject.list", "reportingcompatibility.ReportObject.manage"); - - return map; - } - - /** - * Private method to add a link to the AdminList if there are sufficient privileges - * @param m - * @param privileges - * @param messageKey - * @param link - */ - private void addLink(Map m, String privileges, String link, String messageKey) { - boolean hasPrivilege = false; - if (privileges != null) { - for (String p : privileges.split(",")) { - if (Context.hasPrivilege(p)) { - hasPrivilege = true; - break; - } - } - } - if (hasPrivilege) { - m.put(link, messageKey); - } - } -} +/** + * The contents of this file are subject to the OpenMRS Public License + * Version 1.0 (the "License"); you may not use this file except in + * compliance with the License. You may obtain a copy of the License at + * http://license.openmrs.org + * + * Software distributed under the License is distributed on an "AS IS" + * basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See the + * License for the specific language governing rights and limitations + * under the License. + * + * Copyright (C) OpenMRS, LLC. All Rights Reserved. + */ +package org.openmrs.module.reportingcompatibility.extension.html; + +import java.util.LinkedHashMap; +import java.util.Map; + +import org.openmrs.api.context.Context; +import org.openmrs.module.Extension; +import org.openmrs.module.web.extension.AdministrationSectionExt; + +/** + * This class defines the links that will appear on the administration page + * + * This extension is enabled by defining (uncommenting) it in the + * /metadata/config.xml file. + */ +public class AdminList extends AdministrationSectionExt { + + /** + * @see org.openmrs.module.web.extension.AdministrationSectionExt#getMediaType() + */ + public Extension.MEDIA_TYPE getMediaType() { + return Extension.MEDIA_TYPE.html; + } + + /** + * @see AdministrationSectionExt#getRequiredPrivilege() + */ + @Override + public String getRequiredPrivilege() { + return "Run Reports,Manage Reports,View Reports,View Data Exports"; + } + + /** + * @see org.openmrs.module.web.extension.AdministrationSectionExt#getTitle() + */ + public String getTitle() { + return "reportingcompatibility.Report.header"; + } + + /** + * @see org.openmrs.module.web.extension.AdministrationSectionExt#getLinks() + */ + public Map getLinks() { + + Map map = new LinkedHashMap(); + + addLink(map, "Run Reports", "/admin/reports/runReport.list", "reportingcompatibility.Report.list.title"); + addLink(map, "Manage Reports", "/admin/reports/reportSchemaXml.list", "reportingcompatibility.Report.manage.title"); + addLink(map, "Manage Reports", "/admin/reports/reportMacros.form", "reportingcompatibility.Report.macros.title"); + addLink(map, "Add Report Objects,Edit Report Objects,Delete Report Objects,View Report Objects", + "/admin/reports/dataExport.list", "reportingcompatibility.DataExport.manage"); + addLink(map, "Add Report Objects,Edit Report Objects,Delete Report Objects,View Report Objects", + "/admin/reports/rowPerObsDataExport.list", "reportingcompatibility.RowPerObsDataExport.manage"); + addLink(map, "Add Cohorts,Edit Cohorts,Delete Cohorts,View Cohorts", "/admin/reports/cohorts.list", "Cohort.manage"); + addLink(map, "Add Patient Searches,Edit Patient Searches,Delete Patient Searches,View Patient Searches", + "/admin/reports/patientSearch.list", "reportingcompatibility.PatientSearch.manage"); + addLink(map, "Add Report Objects,Edit Report Objects,Delete Report Objects,View Report Objects", + "/admin/reports/reportObject.list", "reportingcompatibility.ReportObject.manage"); + + return map; + } + + /** + * Private method to add a link to the AdminList if there are sufficient privileges + * @param m + * @param privileges + * @param messageKey + * @param link + */ + private void addLink(Map m, String privileges, String link, String messageKey) { + boolean hasPrivilege = false; + if (privileges != null) { + for (String p : privileges.split(",")) { + if (Context.hasPrivilege(p)) { + hasPrivilege = true; + break; + } + } + } + if (hasPrivilege) { + m.put(link, messageKey); + } + } +} diff --git a/api/src/main/java/org/openmrs/module/reportingcompatibility/extension/html/CohortBuilderHeader.java b/omod/src/main/java/org/openmrs/module/reportingcompatibility/extension/html/CohortBuilderHeader.java similarity index 96% rename from api/src/main/java/org/openmrs/module/reportingcompatibility/extension/html/CohortBuilderHeader.java rename to omod/src/main/java/org/openmrs/module/reportingcompatibility/extension/html/CohortBuilderHeader.java index aa48754..46b56a0 100644 --- a/api/src/main/java/org/openmrs/module/reportingcompatibility/extension/html/CohortBuilderHeader.java +++ b/omod/src/main/java/org/openmrs/module/reportingcompatibility/extension/html/CohortBuilderHeader.java @@ -1,58 +1,58 @@ -/** - * The contents of this file are subject to the OpenMRS Public License - * Version 1.0 (the "License"); you may not use this file except in - * compliance with the License. You may obtain a copy of the License at - * http://license.openmrs.org - * - * Software distributed under the License is distributed on an "AS IS" - * basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See the - * License for the specific language governing rights and limitations - * under the License. - * - * Copyright (C) OpenMRS, LLC. All Rights Reserved. - */ -package org.openmrs.module.reportingcompatibility.extension.html; - -import org.openmrs.module.Extension; -import org.openmrs.module.web.extension.AdministrationSectionExt; -import org.openmrs.module.web.extension.LinkExt; - -/** - * This class defines the links that will appear on the administration page - * - * This extension is enabled by defining (uncommenting) it in the - * /metadata/config.xml file. - */ -public class CohortBuilderHeader extends LinkExt { - - /** - * @see org.openmrs.module.web.extension.AdministrationSectionExt#getMediaType() - */ - public Extension.MEDIA_TYPE getMediaType() { - return Extension.MEDIA_TYPE.html; - } - - /** - * @see AdministrationSectionExt#getRequiredPrivilege() - */ - @Override - public String getRequiredPrivilege() { - return "View Patient Cohorts"; - } - - /** - * @see org.openmrs.module.web.extension.LinkExt#getLabel() - */ - @Override - public String getLabel() { - return "reportingcompatibility.Navigation.analysis"; - } - - /** - * @see org.openmrs.module.web.extension.LinkExt#getUrl() - */ - @Override - public String getUrl() { - return "/cohortBuilder.list"; - } -} +/** + * The contents of this file are subject to the OpenMRS Public License + * Version 1.0 (the "License"); you may not use this file except in + * compliance with the License. You may obtain a copy of the License at + * http://license.openmrs.org + * + * Software distributed under the License is distributed on an "AS IS" + * basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See the + * License for the specific language governing rights and limitations + * under the License. + * + * Copyright (C) OpenMRS, LLC. All Rights Reserved. + */ +package org.openmrs.module.reportingcompatibility.extension.html; + +import org.openmrs.module.Extension; +import org.openmrs.module.web.extension.AdministrationSectionExt; +import org.openmrs.module.web.extension.LinkExt; + +/** + * This class defines the links that will appear on the administration page + * + * This extension is enabled by defining (uncommenting) it in the + * /metadata/config.xml file. + */ +public class CohortBuilderHeader extends LinkExt { + + /** + * @see org.openmrs.module.web.extension.AdministrationSectionExt#getMediaType() + */ + public Extension.MEDIA_TYPE getMediaType() { + return Extension.MEDIA_TYPE.html; + } + + /** + * @see AdministrationSectionExt#getRequiredPrivilege() + */ + @Override + public String getRequiredPrivilege() { + return "View Patient Cohorts"; + } + + /** + * @see org.openmrs.module.web.extension.LinkExt#getLabel() + */ + @Override + public String getLabel() { + return "reportingcompatibility.Navigation.analysis"; + } + + /** + * @see org.openmrs.module.web.extension.LinkExt#getUrl() + */ + @Override + public String getUrl() { + return "/cohortBuilder.list"; + } +} diff --git a/omod/src/main/java/org/openmrs/web/dwr/DWRCohortBuilderService.java b/omod/src/main/java/org/openmrs/web/dwr/DWRCohortBuilderService.java index 58100ce..0f2ef5a 100644 --- a/omod/src/main/java/org/openmrs/web/dwr/DWRCohortBuilderService.java +++ b/omod/src/main/java/org/openmrs/web/dwr/DWRCohortBuilderService.java @@ -39,13 +39,19 @@ import org.openmrs.reporting.ReportObject; import org.openmrs.reporting.ReportObjectService; import org.openmrs.util.OpenmrsUtil; +import org.openmrs.util.PrivilegeConstants; import org.openmrs.util.ReportingcompatibilityUtil; import org.openmrs.web.controller.analysis.CohortBuilderController; +import org.openmrs.web.security.RequirePrivilege; public class DWRCohortBuilderService { - + + // matches the privilege cohortBuilder.jsp itself requires via + private static final String VIEW_PATIENT_COHORTS = "View Patient Cohorts"; + protected final Log log = LogFactory.getLog(getClass()); - + + @RequirePrivilege(VIEW_PATIENT_COHORTS) public Integer getResultCountForFilterId(Integer filterId) { ReportObjectService rs = (ReportObjectService) Context.getService(ReportObjectService.class); PatientFilter pf = rs.getPatientFilterById(filterId); @@ -64,30 +70,35 @@ private CohortSearchHistory getMySearchHistory() { * @param index * @return the number of patients in the resulting PatientSet */ + @RequirePrivilege(VIEW_PATIENT_COHORTS) public Integer getResultCountForSearch(int index) { CohortSearchHistory history = getMySearchHistory(); Cohort ps = history.getPatientSet(index, null); return ps.size(); } + @RequirePrivilege(VIEW_PATIENT_COHORTS) public Cohort getResultForSearch(int index) { CohortSearchHistory history = getMySearchHistory(); Cohort ps = history.getPatientSet(index, null); return ps; } + @RequirePrivilege(VIEW_PATIENT_COHORTS) public Cohort getResultCombineWithAnd() { CohortSearchHistory history = getMySearchHistory(); Cohort ps = history.getPatientSetCombineWithAnd(new EvaluationContext()); return ps; } + @RequirePrivilege(VIEW_PATIENT_COHORTS) public Cohort getResultCombineWithOr() { CohortSearchHistory history = getMySearchHistory(); Cohort ps = history.getPatientSetCombineWithOr(new EvaluationContext()); return ps; } + @RequirePrivilege(VIEW_PATIENT_COHORTS) public Cohort getLastResult() { CohortSearchHistory history = getMySearchHistory(); if (history != null) { @@ -98,6 +109,7 @@ public Cohort getLastResult() { new Cohort(); } + @RequirePrivilege(VIEW_PATIENT_COHORTS) public List getSavedSearches(boolean includeParameterized) { List ret = new ArrayList(); ReportObjectService rs = (ReportObjectService) Context.getService(ReportObjectService.class); @@ -115,6 +127,7 @@ public List getSavedSearches(boolean includeParameterized) { return ret; } + @RequirePrivilege(VIEW_PATIENT_COHORTS) public List getSavedFilters() { List ret = new ArrayList(); ReportObjectService rs = (ReportObjectService) Context.getService(ReportObjectService.class); @@ -134,6 +147,7 @@ public List getSavedFilters() { * * @return */ + @RequirePrivilege(VIEW_PATIENT_COHORTS) public List getSavedCohorts() { List ret = new ArrayList(); List cohorts = Context.getCohortService().getAllCohorts(); @@ -154,6 +168,7 @@ public List getSavedCohorts() { * @param filterId * @return */ + @RequirePrivilege(VIEW_PATIENT_COHORTS) public String getFilterResultAsCommaSeparatedIds(Integer filterId) { ReportObjectService rs = (ReportObjectService) Context.getService(ReportObjectService.class); PatientFilter pf = rs.getPatientFilterById(filterId); @@ -169,6 +184,7 @@ public String getFilterResultAsCommaSeparatedIds(Integer filterId) { * @param cohortId * @return */ + @RequirePrivilege(VIEW_PATIENT_COHORTS) public String getCohortAsCommaSeparatedIds(Integer cohortId) { Cohort c = ReportingcompatibilityUtil.convert(Context.getCohortService().getCohort(cohortId)); if (c == null) @@ -182,6 +198,7 @@ public String getCohortAsCommaSeparatedIds(Integer cohortId) { * * @return */ + @RequirePrivilege(VIEW_PATIENT_COHORTS) public List getSearchHistories() { List ret = new ArrayList(); ReportObjectService rs = (ReportObjectService) Context.getService(ReportObjectService.class); @@ -202,6 +219,7 @@ public List getSearchHistories() { * @param name * @param description */ + @RequirePrivilege(VIEW_PATIENT_COHORTS) public void saveSearchHistory(String name, String description) { CohortSearchHistory history = getMySearchHistory(); if (history.getReportObjectId() != null) @@ -217,6 +235,7 @@ public void saveSearchHistory(String name, String description) { * * @param id */ + @RequirePrivilege(VIEW_PATIENT_COHORTS) public void loadSearchHistory(Integer id) { ReportObjectService rs = (ReportObjectService) Context.getService(ReportObjectService.class); CohortBuilderController.setVolatileUserData("CohortBuilderSearchHistory", rs.getSearchHistory(id)); @@ -229,6 +248,7 @@ public void loadSearchHistory(Integer id) { * @param description The description to give the saved filter * @param indexInHistory The index into the authenticated user's search history */ + @RequirePrivilege(VIEW_PATIENT_COHORTS) public Boolean saveHistoryElement(String name, String description, Integer indexInHistory) { CohortSearchHistory history = getMySearchHistory(); try { @@ -259,6 +279,8 @@ public Boolean saveHistoryElement(String name, String description, Integer index * @param description * @param commaSeparatedIds */ + // matches CohortService#saveCohort's own @Authorized requirement + @RequirePrivilege({ PrivilegeConstants.ADD_COHORTS, PrivilegeConstants.EDIT_COHORTS }) public void saveCohort(String name, String description, String commaSeparatedIds) { Set ids = new HashSet(OpenmrsUtil.delimitedStringToIntegerList(commaSeparatedIds, ",")); org.openmrs.Cohort cohort = new org.openmrs.Cohort(); @@ -272,6 +294,7 @@ public void saveCohort(String name, String description, String commaSeparatedIds * This isn't really useful because most of the properties don't have DWR converters. I'm * leaving it here in case I get to work on it later. */ + @RequirePrivilege(VIEW_PATIENT_COHORTS) public CohortSearchHistory getUserSearchHistory() { return getMySearchHistory(); } @@ -289,6 +312,7 @@ public CohortSearchHistory getUserSearchHistory() { * @return Vector containing all Parameters that need to be provided to evaluate the * input cohortSpecification */ + @RequirePrivilege(VIEW_PATIENT_COHORTS) public Vector getMissingParameters(String cohortSpecification) { Vector ret = new Vector(); CohortDefinition def = CohortUtil.parse(cohortSpecification); @@ -306,6 +330,7 @@ public Vector getMissingParameters(String cohortSpecification) { * @param cohortSpecification - This input String represents the Cohort Definition to evaluate * @return Cohort - The Cohort of patients that are returned */ + @RequirePrivilege(VIEW_PATIENT_COHORTS) public Cohort evaluateCohortDefinition(String cohortSpecification, Map parameterValues) { CohortDefinition def = CohortUtil.parse(cohortSpecification); EvaluationContext evalContext = new EvaluationContext(); diff --git a/omod/src/main/java/org/openmrs/web/dwr/DWRPatientSetService.java b/omod/src/main/java/org/openmrs/web/dwr/DWRPatientSetService.java index 5a13c0e..7fadcc0 100644 --- a/omod/src/main/java/org/openmrs/web/dwr/DWRPatientSetService.java +++ b/omod/src/main/java/org/openmrs/web/dwr/DWRPatientSetService.java @@ -18,11 +18,15 @@ import org.openmrs.Patient; import org.openmrs.api.PatientService; import org.openmrs.api.context.Context; +import org.openmrs.util.PrivilegeConstants; +import org.openmrs.web.security.RequirePrivilege; public class DWRPatientSetService { - + protected final Log log = LogFactory.getLog(getClass()); - + + // matches PatientService#getPatient's own @Authorized requirement + @RequirePrivilege(PrivilegeConstants.GET_PATIENTS) public Vector getPatients(String patientIds) { Vector ret = new Vector(); List ptIds = new ArrayList(); diff --git a/omod/src/main/resources/config.xml b/omod/src/main/resources/config.xml index deb0d65..df39862 100644 --- a/omod/src/main/resources/config.xml +++ b/omod/src/main/resources/config.xml @@ -33,31 +33,12 @@ - - - + - - - - - reportingcompatibility.patientLinkUrl - patientDashboard.form - The link url for a particular patient to view from the cohort builder - - - - reportingcompatibility.data_export_batch_size - 7500 - The number of patients to export at a time in a data export. The larger this number the faster and more memory that is used. The smaller this number the slower and less memory is used. - - - - - + @@ -82,7 +63,19 @@ - + + + reportingcompatibility.patientLinkUrl + patientDashboard.form + The link url for a particular patient to view from the cohort builder + + + + reportingcompatibility.data_export_batch_size + 7500 + The number of patients to export at a time in a data export. The larger this number the faster and more memory that is used. The smaller this number the slower and less memory is used. + + dataExportServlet org.openmrs.web.servlet.DataExportServlet diff --git a/omod/src/test/java/org/openmrs/web/dwr/DwrConfigXmlTest.java b/omod/src/test/java/org/openmrs/web/dwr/DwrConfigXmlTest.java new file mode 100644 index 0000000..878f985 --- /dev/null +++ b/omod/src/test/java/org/openmrs/web/dwr/DwrConfigXmlTest.java @@ -0,0 +1,128 @@ +/** + * This Source Code Form is subject to the terms of the Mozilla Public License, + * v. 2.0. If a copy of the MPL was not distributed with this file, You can + * obtain one at http://mozilla.org/MPL/2.0/. OpenMRS is also distributed under + * the terms of the Healthcare Disclaimer located at http://openmrs.org/license. + * + * Copyright (C) OpenMRS Inc. OpenMRS is a registered trademark and the OpenMRS + * graphic logo is a trademark of OpenMRS Inc. + */ +package org.openmrs.web.dwr; + +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +import java.io.InputStream; +import java.lang.reflect.Method; +import java.util.ArrayList; +import java.util.List; + +import javax.xml.parsers.DocumentBuilderFactory; + +import org.junit.Test; +import org.openmrs.web.security.RequirePrivilege; +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.Node; +import org.w3c.dom.NodeList; + +/** + * Regression guard for the two bugs fixed in RCM-124. {@code WebModuleUtil} only merges the + * first {@code } block per module into {@code dwr-modules.xml}, so a second block added by + * mistake is silently dropped (this is exactly how {@code DWRCohortBuilderService}'s real + * registration went missing for years). And {@code DwrAuthorizationFilter} (legacyui) fails open + * on a DWR method with no {@link RequirePrivilege} rather than rejecting it, so a future method + * added here without the annotation would ship silently unenforced - the only trace is a startup + * WARN in legacyui's log, easy to miss. Mirrors {@code DwrAuthorizationFilter#scanDwrCreates} and + * {@code #findMethodAnnotation} so this test fails whenever that filter's real allow-list would + * actually be missing an entry. + */ +public class DwrConfigXmlTest { + + @Test + public void configXml_shouldDeclareExactlyOneDwrBlock() throws Exception { + NodeList dwrBlocks = loadConfigXml().getElementsByTagName("dwr"); + assertTrue("config.xml should declare exactly one block - WebModuleUtil only merges " + + "the first one per module into dwr-modules.xml, so a second block is silently " + + "dropped rather than merged; found " + dwrBlocks.getLength(), dwrBlocks.getLength() == 1); + } + + @Test + public void configXml_everyIncludedDwrMethodShouldBeAnnotated() throws Exception { + List unannotated = new ArrayList<>(); + + Element dwrBlock = (Element) loadConfigXml().getElementsByTagName("dwr").item(0); + NodeList creates = dwrBlock.getElementsByTagName("create"); + for (int i = 0; i < creates.getLength(); i++) { + Element create = (Element) creates.item(i); + String script = create.getAttribute("javascript"); + String className = findClassParam(create); + assertNotNull("DWR script '" + script + "' has no ", className); + + Class dwrClass = Class.forName(className); + + NodeList includes = create.getElementsByTagName("include"); + for (int j = 0; j < includes.getLength(); j++) { + String methodName = ((Element) includes.item(j)).getAttribute("method"); + if (findMethodAnnotation(dwrClass, methodName) == null) { + unannotated.add(script + "." + methodName); + } + } + } + + assertTrue("DWR methods exposed without @RequirePrivilege ship silently unenforced " + + "(DwrAuthorizationFilter fails open on them rather than rejecting): " + unannotated, + unannotated.isEmpty()); + } + + /** + * Mirrors {@code DwrAuthorizationFilter#findMethodAnnotation}: first declared method (walking + * up the class hierarchy) whose name matches, DWR resolves overloads by argument count at + * runtime but the filter treats all overloads of one name as sharing one privilege + * requirement. + */ + private RequirePrivilege findMethodAnnotation(Class dwrClass, String methodName) { + Class cursor = dwrClass; + while (cursor != null && cursor != Object.class) { + for (Method method : cursor.getDeclaredMethods()) { + if (method.getName().equals(methodName)) { + RequirePrivilege annotation = method.getAnnotation(RequirePrivilege.class); + if (annotation != null) { + return annotation; + } + } + } + cursor = cursor.getSuperclass(); + } + return null; + } + + private String findClassParam(Element create) { + NodeList params = create.getElementsByTagName("param"); + for (int i = 0; i < params.getLength(); i++) { + Node node = params.item(i); + if (!(node instanceof Element)) { + continue; + } + Element param = (Element) node; + if ("class".equals(param.getAttribute("name"))) { + String value = param.getAttribute("value"); + if (value != null && !value.isEmpty()) { + return value; + } + } + } + return null; + } + + private Document loadConfigXml() throws Exception { + DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); + dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + dbf.setFeature("http://xml.org/sax/features/external-general-entities", false); + dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + try (InputStream in = getClass().getClassLoader().getResourceAsStream("config.xml")) { + assertNotNull("config.xml not found on test classpath", in); + return dbf.newDocumentBuilder().parse(in); + } + } +} diff --git a/pom.xml b/pom.xml index 29acae1..c65fd20 100644 --- a/pom.xml +++ b/pom.xml @@ -36,61 +36,39 @@ omod - - - - - - org.openmrs.api - openmrs-api - ${openMRSVersion} - jar - provided - - - org.openmrs.api - openmrs-api - ${openMRSVersion} - test-jar - test - - - org.openmrs.web - openmrs-web - ${openMRSVersion} - jar - provided - - - org.openmrs.web - openmrs-web - ${openMRSVersion} - test-jar - test - - - org.openmrs.test - openmrs-test - ${openMRSVersion} - pom - test - - - + + + org.openmrs.api + openmrs-api + ${openMRSVersion} + jar + provided + + + org.openmrs.api + openmrs-api + ${openMRSVersion} + test-jar + test + + + org.openmrs.test + openmrs-test + ${openMRSVersion} + pom + test + + + org.openmrs.module + legacyui-api + ${legacyuiVersion} + provided + + 2.0.0 + 2.2.0-SNAPSHOT UTF-8 @@ -107,8 +85,8 @@ org.apache.maven.plugins maven-compiler-plugin - 1.5 - 1.5 + 1.8 + 1.8