29867170267

Committed 21 Jul 2026 05:53PM UTC coverage: 64.101% (+0.1%) from 63.994%

Build # 29867170267

Build Type

push

github

Committed by

ibacher

Commit Message

Require Get Patient Programs privilege for by-uuid program lookups (#6318)

* Require Get Patient Programs privilege for by-uuid program lookups

ProgramWorkflowService.getPatientProgramByUuid and getPatientStateByUuid
had no @Authorized annotation. Because AuthorizationAdvice only enforces
when a method declares a privilege, both fell through with no check at
all, letting any authenticated user read program-enrollment PHI by uuid.
Every other read on the service, including getPatientProgram(Integer),
already requires Get Patient Programs; annotate both by-uuid methods to
match.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Require Get Patient Programs privilege for patient program attribute lookup

getPatientProgramAttributeByAttributeName returns patient program
attribute values keyed by patient id, yet lacked an @Authorized check.
Its two sibling attribute readers (getPatientProgramAttributeByUuid and
getPatientProgramByAttributeNameAndValue) already guard on Get Patient
Programs, so this was a missed PHI read of the same class addressed by
GHSA-gqf9-38mg-wgqg.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TpKNfxm3QjwUBDePEtFm2H

* Add authorization tests pinning the by-uuid program privilege checks

The existing tests for getPatientProgramByUuid, getPatientStateByUuid and
getPatientProgramAttributeByAttributeName run as the privileged superuser
and assert only find/null behavior, so they pass even if the
@Authorized(Get Patient Programs) checks added by this branch are removed.
getPatientProgramAttributeByAttributeName had no test at all.

Add a test per method that logs out (dropping Get Patient Programs) and
asserts APIAuthenticationException is thrown for otherwise-valid arguments,
mirroring AuthorizationAdviceTest and AdministrationServiceTest. These fail
if any of the three annotations is removed, pinning the fix.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Cl... (continued)

Coverage Stats

24273 of 37867 relevant lines covered (64.1%)

0.64 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

92.41

/api/src/main/java/org/openmrs/api/impl/CohortServiceImpl.java

/**
 * 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.api.impl;

import static org.openmrs.util.DateUtil.truncateToSeconds;

import java.util.Date;
import java.util.List;
import java.util.stream.Collectors;

import org.openmrs.Cohort;
import org.openmrs.CohortMembership;
import org.openmrs.Patient;
import org.openmrs.User;
import org.openmrs.api.APIException;
import org.openmrs.api.CohortService;
import org.openmrs.api.context.Context;
import org.openmrs.api.db.CohortDAO;
import org.openmrs.util.OpenmrsUtil;
import org.openmrs.util.PrivilegeConstants;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.transaction.annotation.Transactional;

/**
 * API functions related to Cohorts
 */
@Transactional
public class CohortServiceImpl extends BaseOpenmrsService implements CohortService {
        
        private static final Logger log = LoggerFactory.getLogger(CohortServiceImpl.class);
        
        private CohortDAO dao;
        
        /**
         * @see org.openmrs.api.CohortService#setCohortDAO(org.openmrs.api.db.CohortDAO)
         */
        @Override
        public void setCohortDAO(CohortDAO dao) {
                this.dao = dao;
        }
        
        /**
         * @see org.openmrs.api.CohortService#saveCohort(org.openmrs.Cohort)
         */
        @Override
        public Cohort saveCohort(Cohort cohort) throws APIException {
                if (cohort.getCohortId() == null) {
                        Context.requirePrivilege(PrivilegeConstants.ADD_COHORTS);
                } else {
                        Context.requirePrivilege(PrivilegeConstants.EDIT_COHORTS);
                }
                if (cohort.getName() == null) {
                        throw new APIException("Cohort.save.nameRequired", (Object[]) null);
                }
                if (cohort.getDescription() == null) {
                        throw new APIException("Cohort.save.descriptionRequired", (Object[]) null);
                }
                if (log.isInfoEnabled()) {
                        log.info("Saving cohort " + cohort);
                }
                
                return dao.saveCohort(cohort);
        }
        
        /**
         * @see org.openmrs.api.CohortService#getCohort(java.lang.Integer)
         */
        @Override
        @Transactional(readOnly = true)
        public Cohort getCohort(Integer id) {
                return dao.getCohort(id);
        }
        
        /**
         * @see org.openmrs.api.CohortService#voidCohort(org.openmrs.Cohort, java.lang.String)
         */
        @Override
        public Cohort voidCohort(Cohort cohort, String reason) {
                // other setters done by the save handlers
                return Context.getCohortService().saveCohort(cohort);
        }
        
        /**
         * @see org.openmrs.api.CohortService#getCohortByUuid(java.lang.String)
         */
        @Override
        @Transactional(readOnly = true)
        public Cohort getCohortByUuid(String uuid) {
                return dao.getCohortByUuid(uuid);
        }
        
        /**
         * @see org.openmrs.api.CohortService#getCohortMembershipByUuid(java.lang.String)
         */
        @Override
        @Transactional(readOnly = true)
        public CohortMembership getCohortMembershipByUuid(String uuid) {
                return dao.getCohortMembershipByUuid(uuid);
        }
        
        /**
         * @see org.openmrs.api.CohortService#addPatientToCohort(org.openmrs.Cohort,
         *      org.openmrs.Patient)
         */
        @Override
        public Cohort addPatientToCohort(Cohort cohort, Patient patient) {
                if (!cohort.contains(patient.getPatientId())) {
                        CohortMembership cohortMembership = new CohortMembership(patient.getPatientId());
                        cohort.addMembership(cohortMembership);
                        Context.getCohortService().saveCohort(cohort);
                }
                return cohort;
        }
        
        /**
         * @see org.openmrs.api.CohortService#removePatientFromCohort(org.openmrs.Cohort,
         *      org.openmrs.Patient)
         */
        @Override
        public Cohort removePatientFromCohort(Cohort cohort, Patient patient) {
                List<CohortMembership> memberships = getCohortMemberships(patient.getPatientId(), null, false);
                List<CohortMembership> toVoid = memberships.stream()
                                .filter(m -> m.getCohort().equals(cohort))
                                .collect(Collectors.toList());
                
                for (CohortMembership membership : toVoid) {
                        Context.getCohortService().voidCohortMembership(membership, "removePatientFromCohort");
                }
                return cohort;
        }
        
        @Override
        @Transactional(readOnly = true)
        public List<Cohort> getCohortsContainingPatient(Patient patient) {
                return getCohortsContainingPatientId(patient.getPatientId());
        }
        
        @Override
        @Transactional(readOnly = true)
        public List<Cohort> getCohortsContainingPatientId(Integer patientId) {
                return dao.getCohortsContainingPatientId(patientId, false, new Date());
        }
        
        /**
         * @see org.openmrs.api.CohortService#getCohorts(java.lang.String)
         */
        @Override
        @Transactional(readOnly = true)
        public List<Cohort> getCohorts(String nameFragment) throws APIException {
                return dao.getCohorts(nameFragment);
        }
        
        /**
         * @see org.openmrs.api.CohortService#getAllCohorts()
         */
        @Override
        @Transactional(readOnly = true)
        public List<Cohort> getAllCohorts() throws APIException {
                return Context.getCohortService().getAllCohorts(false);
        }
        
        /**
         * @see org.openmrs.api.CohortService#getAllCohorts(boolean)
         */
        @Override
        @Transactional(readOnly = true)
        public List<Cohort> getAllCohorts(boolean includeVoided) throws APIException {
                return dao.getAllCohorts(includeVoided);
        }
        
        /**
         * @see org.openmrs.api.CohortService#getCohortByName(java.lang.String)
         */
        @Override
        @Transactional(readOnly = true)
        public Cohort getCohortByName(String name) throws APIException {
                return dao.getCohort(name);
        }
        
        /**
         * @see org.openmrs.api.CohortService#getCohort(java.lang.String)
         */
        @Override
        @Transactional(readOnly = true)
        public Cohort getCohort(String name) throws APIException {
                return getCohortByName(name);
        }
        
        /**
         * @see org.openmrs.api.CohortService#purgeCohort(org.openmrs.Cohort)
         */
        @Override
        public Cohort purgeCohort(Cohort cohort) throws APIException {
                return dao.deleteCohort(cohort);
        }
        
        /**
         * @see CohortService#purgeCohortMembership(CohortMembership)
         */
        @Override
        public void purgeCohortMembership(CohortMembership cohortMembership) throws APIException {
                Cohort cohort = cohortMembership.getCohort();
                boolean removed = cohort.removeMembership(cohortMembership);
                if (removed) {
                        Context.getCohortService().saveCohort(cohort);
                }
        }
        
        /**
         * @see CohortService#voidCohortMembership(CohortMembership, String)
         */
        @Override
        public CohortMembership voidCohortMembership(CohortMembership cohortMembership, String reason) {
                Context.getCohortService().saveCohort(cohortMembership.getCohort());
                return cohortMembership;
        }
        
        /**
         * @see CohortService#endCohortMembership(CohortMembership, Date)
         */
        @Override
        public CohortMembership endCohortMembership(CohortMembership cohortMembership, Date onDate) {
                cohortMembership.setEndDate(onDate == null ? new Date() : onDate);
                Context.getCohortService().saveCohort(cohortMembership.getCohort());
                return cohortMembership;
        }
        
        /**
         * @see org.openmrs.api.CohortService#notifyPatientVoided(org.openmrs.Patient)
         */
        @Override
        public void notifyPatientVoided(Patient patient) throws APIException {
                // this membership read is internal bookkeeping done while a patient is being voided; acquire
                // the read privilege on the voiding user's behalf rather than requiring them to hold it
                List<CohortMembership> memberships;
                try {
                        Context.addProxyPrivilege(PrivilegeConstants.GET_PATIENT_COHORTS);
                        memberships = Context.getCohortService().getCohortMemberships(patient.getPatientId(), null, false);
                } finally {
                        Context.removeProxyPrivilege(PrivilegeConstants.GET_PATIENT_COHORTS);
                }
                memberships.forEach(m -> {
                        m.setVoided(patient.getVoided());
                        m.setDateVoided(patient.getDateVoided());
                        m.setVoidedBy(patient.getVoidedBy());
                        m.setVoidReason(patient.getVoidReason());
                        dao.saveCohortMembership(m);
                });
        }
        
        /**
         * @see org.openmrs.api.CohortService#notifyPatientUnvoided(Patient, User, Date)
         */
        @Override
        public void notifyPatientUnvoided(Patient patient, User originallyVoidedBy, Date originalDateVoided) throws APIException {
                List<CohortMembership> memberships = getCohortMemberships(patient.getPatientId(), null, true);
                List<CohortMembership> toUnvoid = memberships.stream().filter(
                                                m -> m.getVoided()
                                                                && m.getVoidedBy().equals(originallyVoidedBy)
                                                                && OpenmrsUtil.compare(
                                                                                truncateToSeconds(m.getDateVoided()),
                                                                                truncateToSeconds(originalDateVoided)) == 0)
                                .collect(Collectors.toList());
                
                for (CohortMembership member : toUnvoid) {
                        member.setVoided(false);
                        member.setDateVoided(null);
                        member.setVoidedBy(null);
                        member.setVoidReason(null);
                        dao.saveCohortMembership(member);
                }
        }
        
        @Override
        public List<CohortMembership> getCohortMemberships(Integer patientId, Date activeOnDate, boolean includeVoided) {
                if (patientId == null) {
                        throw new IllegalArgumentException("patientId is required");
                }
                return dao.getCohortMemberships(patientId, activeOnDate, includeVoided);
        }
}

1	/**
2	* This Source Code Form is subject to the terms of the Mozilla Public License,
3	* v. 2.0. If a copy of the MPL was not distributed with this file, You can
4	* obtain one at http://mozilla.org/MPL/2.0/. OpenMRS is also distributed under
5	* the terms of the Healthcare Disclaimer located at http://openmrs.org/license.
6	*
7	* Copyright (C) OpenMRS Inc. OpenMRS is a registered trademark and the OpenMRS
8	* graphic logo is a trademark of OpenMRS Inc.
9	*/
10	package org.openmrs.api.impl;
11
12	import static org.openmrs.util.DateUtil.truncateToSeconds;
13
14	import java.util.Date;
15	import java.util.List;
16	import java.util.stream.Collectors;
17
18	import org.openmrs.Cohort;
19	import org.openmrs.CohortMembership;
20	import org.openmrs.Patient;
21	import org.openmrs.User;
22	import org.openmrs.api.APIException;
23	import org.openmrs.api.CohortService;
24	import org.openmrs.api.context.Context;
25	import org.openmrs.api.db.CohortDAO;
26	import org.openmrs.util.OpenmrsUtil;
27	import org.openmrs.util.PrivilegeConstants;
28	import org.slf4j.Logger;
29	import org.slf4j.LoggerFactory;
30	import org.springframework.transaction.annotation.Transactional;
31
32	/**
33	* API functions related to Cohorts
34	*/
35	@Transactional
36	public class CohortServiceImpl extends BaseOpenmrsService implements CohortService {	1✔
37
38	private static final Logger log = LoggerFactory.getLogger(CohortServiceImpl.class);	1✔
39
40	private CohortDAO dao;
41
42	/**
43	* @see org.openmrs.api.CohortService#setCohortDAO(org.openmrs.api.db.CohortDAO)
44	*/
45	@Override
46	public void setCohortDAO(CohortDAO dao) {
47	this.dao = dao;	1✔
48	}	1✔
49
50	/**
51	* @see org.openmrs.api.CohortService#saveCohort(org.openmrs.Cohort)
52	*/
53	@Override
54	public Cohort saveCohort(Cohort cohort) throws APIException {
55	if (cohort.getCohortId() == null) {	1✔
56	Context.requirePrivilege(PrivilegeConstants.ADD_COHORTS);	1✔
57	} else {
58	Context.requirePrivilege(PrivilegeConstants.EDIT_COHORTS);	1✔
59	}
60	if (cohort.getName() == null) {	1✔
61	throw new APIException("Cohort.save.nameRequired", (Object[]) null);	×
62	}
63	if (cohort.getDescription() == null) {	1✔
64	throw new APIException("Cohort.save.descriptionRequired", (Object[]) null);	×
65	}
66	if (log.isInfoEnabled()) {	1✔
67	log.info("Saving cohort " + cohort);	×
68	}
69
70	return dao.saveCohort(cohort);	1✔
71	}
72
73	/**
74	* @see org.openmrs.api.CohortService#getCohort(java.lang.Integer)
75	*/
76	@Override
77	@Transactional(readOnly = true)
78	public Cohort getCohort(Integer id) {
79	return dao.getCohort(id);	1✔
80	}
81
82	/**
83	* @see org.openmrs.api.CohortService#voidCohort(org.openmrs.Cohort, java.lang.String)
84	*/
85	@Override
86	public Cohort voidCohort(Cohort cohort, String reason) {
87	// other setters done by the save handlers
88	return Context.getCohortService().saveCohort(cohort);	1✔
89	}
90
91	/**
92	* @see org.openmrs.api.CohortService#getCohortByUuid(java.lang.String)
93	*/
94	@Override
95	@Transactional(readOnly = true)
96	public Cohort getCohortByUuid(String uuid) {
97	return dao.getCohortByUuid(uuid);	1✔
98	}
99
100	/**
101	* @see org.openmrs.api.CohortService#getCohortMembershipByUuid(java.lang.String)
102	*/
103	@Override
104	@Transactional(readOnly = true)
105	public CohortMembership getCohortMembershipByUuid(String uuid) {
106	return dao.getCohortMembershipByUuid(uuid);	1✔
107	}
108
109	/**
110	* @see org.openmrs.api.CohortService#addPatientToCohort(org.openmrs.Cohort,
111	* org.openmrs.Patient)
112	*/
113	@Override
114	public Cohort addPatientToCohort(Cohort cohort, Patient patient) {
115	if (!cohort.contains(patient.getPatientId())) {	1✔
116	CohortMembership cohortMembership = new CohortMembership(patient.getPatientId());	1✔
117	cohort.addMembership(cohortMembership);	1✔
118	Context.getCohortService().saveCohort(cohort);	1✔
119	}
120	return cohort;	1✔
121	}
122
123	/**
124	* @see org.openmrs.api.CohortService#removePatientFromCohort(org.openmrs.Cohort,
125	* org.openmrs.Patient)
126	*/
127	@Override
128	public Cohort removePatientFromCohort(Cohort cohort, Patient patient) {
129	List<CohortMembership> memberships = getCohortMemberships(patient.getPatientId(), null, false);	1✔
130	List<CohortMembership> toVoid = memberships.stream()	1✔
131	.filter(m -> m.getCohort().equals(cohort))	1✔
132	.collect(Collectors.toList());	1✔
133
134	for (CohortMembership membership : toVoid) {	1✔
135	Context.getCohortService().voidCohortMembership(membership, "removePatientFromCohort");	1✔
136	}	1✔
137	return cohort;	1✔
138	}
139
140	@Override
141	@Transactional(readOnly = true)
142	public List<Cohort> getCohortsContainingPatient(Patient patient) {
143	return getCohortsContainingPatientId(patient.getPatientId());	×
144	}
145
146	@Override
147	@Transactional(readOnly = true)
148	public List<Cohort> getCohortsContainingPatientId(Integer patientId) {
149	return dao.getCohortsContainingPatientId(patientId, false, new Date());	1✔
150	}
151
152	/**
153	* @see org.openmrs.api.CohortService#getCohorts(java.lang.String)
154	*/
155	@Override
156	@Transactional(readOnly = true)
157	public List<Cohort> getCohorts(String nameFragment) throws APIException {
158	return dao.getCohorts(nameFragment);	1✔
159	}
160
161	/**
162	* @see org.openmrs.api.CohortService#getAllCohorts()
163	*/
164	@Override
165	@Transactional(readOnly = true)
166	public List<Cohort> getAllCohorts() throws APIException {
167	return Context.getCohortService().getAllCohorts(false);	1✔
168	}
169
170	/**
171	* @see org.openmrs.api.CohortService#getAllCohorts(boolean)
172	*/
173	@Override
174	@Transactional(readOnly = true)
175	public List<Cohort> getAllCohorts(boolean includeVoided) throws APIException {
176	return dao.getAllCohorts(includeVoided);	1✔
177	}
178
179	/**
180	* @see org.openmrs.api.CohortService#getCohortByName(java.lang.String)
181	*/
182	@Override
183	@Transactional(readOnly = true)
184	public Cohort getCohortByName(String name) throws APIException {
185	return dao.getCohort(name);	1✔
186	}
187
188	/**
189	* @see org.openmrs.api.CohortService#getCohort(java.lang.String)
190	*/
191	@Override
192	@Transactional(readOnly = true)
193	public Cohort getCohort(String name) throws APIException {
194	return getCohortByName(name);	×
195	}
196
197	/**
198	* @see org.openmrs.api.CohortService#purgeCohort(org.openmrs.Cohort)
199	*/
200	@Override
201	public Cohort purgeCohort(Cohort cohort) throws APIException {
202	return dao.deleteCohort(cohort);	1✔
203	}
204
205	/**
206	* @see CohortService#purgeCohortMembership(CohortMembership)
207	*/
208	@Override
209	public void purgeCohortMembership(CohortMembership cohortMembership) throws APIException {
210	Cohort cohort = cohortMembership.getCohort();	1✔
211	boolean removed = cohort.removeMembership(cohortMembership);	1✔
212	if (removed) {	1✔
213	Context.getCohortService().saveCohort(cohort);	1✔
214	}
215	}	1✔
216
217	/**
218	* @see CohortService#voidCohortMembership(CohortMembership, String)
219	*/
220	@Override
221	public CohortMembership voidCohortMembership(CohortMembership cohortMembership, String reason) {
222	Context.getCohortService().saveCohort(cohortMembership.getCohort());	1✔
223	return cohortMembership;	1✔
224	}
225
226	/**
227	* @see CohortService#endCohortMembership(CohortMembership, Date)
228	*/
229	@Override
230	public CohortMembership endCohortMembership(CohortMembership cohortMembership, Date onDate) {
231	cohortMembership.setEndDate(onDate == null ? new Date() : onDate);	1✔
232	Context.getCohortService().saveCohort(cohortMembership.getCohort());	1✔
233	return cohortMembership;	1✔
234	}
235
236	/**
237	* @see org.openmrs.api.CohortService#notifyPatientVoided(org.openmrs.Patient)
238	*/
239	@Override
240	public void notifyPatientVoided(Patient patient) throws APIException {
241	// this membership read is internal bookkeeping done while a patient is being voided; acquire
242	// the read privilege on the voiding user's behalf rather than requiring them to hold it
243	List<CohortMembership> memberships;
244	try {
245	Context.addProxyPrivilege(PrivilegeConstants.GET_PATIENT_COHORTS);	1✔
246	memberships = Context.getCohortService().getCohortMemberships(patient.getPatientId(), null, false);	1✔
247	} finally {
248	Context.removeProxyPrivilege(PrivilegeConstants.GET_PATIENT_COHORTS);	1✔
249	}
250	memberships.forEach(m -> {	1✔
251	m.setVoided(patient.getVoided());	1✔
252	m.setDateVoided(patient.getDateVoided());	1✔
253	m.setVoidedBy(patient.getVoidedBy());	1✔
254	m.setVoidReason(patient.getVoidReason());	1✔
255	dao.saveCohortMembership(m);	1✔
256	});	1✔
257	}	1✔
258
259	/**
260	* @see org.openmrs.api.CohortService#notifyPatientUnvoided(Patient, User, Date)
261	*/
262	@Override
263	public void notifyPatientUnvoided(Patient patient, User originallyVoidedBy, Date originalDateVoided) throws APIException {
264	List<CohortMembership> memberships = getCohortMemberships(patient.getPatientId(), null, true);	1✔
265	List<CohortMembership> toUnvoid = memberships.stream().filter(	1✔
266	m -> m.getVoided()	1✔
267	&& m.getVoidedBy().equals(originallyVoidedBy)	1✔
268	&& OpenmrsUtil.compare(	1✔
269	truncateToSeconds(m.getDateVoided()),	1✔
270	truncateToSeconds(originalDateVoided)) == 0)	1✔
271	.collect(Collectors.toList());	1✔
272
273	for (CohortMembership member : toUnvoid) {	1✔
274	member.setVoided(false);	1✔
275	member.setDateVoided(null);	1✔
276	member.setVoidedBy(null);	1✔
277	member.setVoidReason(null);	1✔
278	dao.saveCohortMembership(member);	1✔
279	}	1✔
280	}	1✔
281
282	@Override
283	public List<CohortMembership> getCohortMemberships(Integer patientId, Date activeOnDate, boolean includeVoided) {
284	if (patientId == null) {	1✔
285	throw new IllegalArgumentException("patientId is required");	×
286	}
287	return dao.getCohortMemberships(patientId, activeOnDate, includeVoided);	1✔
288	}
289	}

openmrs / openmrs-core / 29867170267

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous