29867099098

Committed 21 Jul 2026 08:37PM UTC coverage: 66.218% (+0.1%) from 66.106%

Build # 29867099098

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

24392 of 36836 relevant lines covered (66.22%)

0.66 hits per line

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

96.55

/api/src/main/java/org/openmrs/validator/AllergyValidator.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.validator;

import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.math.NumberUtils;
import org.openmrs.Allergen;
import org.openmrs.Allergies;
import org.openmrs.Allergy;
import org.openmrs.annotation.Handler;
import org.openmrs.api.PatientService;
import org.openmrs.api.context.Context;
import org.openmrs.messagesource.MessageSourceService;
import org.openmrs.util.PrivilegeConstants;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;
import org.springframework.validation.Errors;
import org.springframework.validation.ValidationUtils;
import org.springframework.validation.Validator;

@Component("allergyValidator")
@Handler(supports = { Allergy.class }, order = 50)
public class AllergyValidator implements Validator {
        
        @Autowired
        private MessageSourceService messageSourceService;
        
        @Autowired
        private PatientService patientService;
        
        @Override
        public boolean supports(Class<?> clazz) {
                return Allergy.class.isAssignableFrom(clazz);
        }
        
        /**
         * @see Validator#validate(Object, org.springframework.validation.Errors)
         * @param target
         * @param errors
         * <strong>Should</strong> fail for a null value
         * <strong>Should</strong> fail if patient is null
         * <strong>Should</strong> fail id allergenType is null
         * <strong>Should</strong> fail if allergen is null
         * <strong>Should</strong> fail if codedAllergen is null
         * <strong>Should</strong> fail if nonCodedAllergen is null and allergen is set to other non coded
         * <strong>Should</strong> reject a duplicate allergen
         * <strong>Should</strong> reject a duplicate non coded allergen
         * <strong>Should</strong> pass for a valid allergy
         * <strong>Should</strong> reject numeric values and symbols on reactionNonCoded
         */
        @Override
        public void validate(Object target, Errors errors) {
                
                if (target == null) {
                        throw new IllegalArgumentException("Allergy should not be null");
                }
                
                ValidationUtils.rejectIfEmpty(errors, "patient", "allergyapi.patient.required");
                
                Allergy allergy = (Allergy) target;
                
                if (allergy.getReactionNonCoded() != null) {
                        if (NumberUtils.isParsable(allergy.getReactionNonCoded())) {
                                errors.rejectValue("reactionNonCoded", "error.allergyapi.allergy.ReactionNonCoded.cannotBeNumeric");
                        }
                }
                if (allergy.getAllergen() == null) {
                        errors.rejectValue("allergen", "allergyapi.allergen.required");
                } else {
                        Allergen allergen = allergy.getAllergen();
                        if (allergen.getAllergenType() == null) {
                                errors.rejectValue("allergen", "allergyapi.allergenType.required");
                        }
                        
                        if (allergen.getCodedAllergen() == null && StringUtils.isBlank(allergen.getNonCodedAllergen())) {
                                errors.rejectValue("allergen", "allergyapi.allergen.codedOrNonCodedAllergen.required");
                        } else if (!allergen.isCoded() && StringUtils.isBlank(allergen.getNonCodedAllergen())) {
                                errors.rejectValue("allergen", "allergyapi.allergen.nonCodedAllergen.required");
                        }
                        
                        if (allergy.getAllergyId() == null && allergy.getPatient() != null) {
                                // this duplicate-allergen check is an internal read done while saving the allergy; a
                                // user with an allergy write privilege should not additionally need Get Allergies for
                                // it, so acquire the read privilege on their behalf
                                Allergies existingAllergies;
                                try {
                                        Context.addProxyPrivilege(PrivilegeConstants.GET_ALLERGIES);
                                        existingAllergies = patientService.getAllergies(allergy.getPatient());
                                } finally {
                                        Context.removeProxyPrivilege(PrivilegeConstants.GET_ALLERGIES);
                                }
                                if (existingAllergies.containsAllergen(allergy)) {
                                        String key = "ui.i18n.Concept.name." + allergen.getCodedAllergen().getUuid();
                                        String name = messageSourceService.getMessage(key);
                                        if (key.equals(name)) {
                                                name = allergen.toString();
                                        }
                                        errors.rejectValue("allergen", "allergyapi.message.duplicateAllergen", new Object[] { name }, null);
                                }
                        }
                }
        }
}

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.validator;
11
12	import org.apache.commons.lang3.StringUtils;
13	import org.apache.commons.lang3.math.NumberUtils;
14	import org.openmrs.Allergen;
15	import org.openmrs.Allergies;
16	import org.openmrs.Allergy;
17	import org.openmrs.annotation.Handler;
18	import org.openmrs.api.PatientService;
19	import org.openmrs.api.context.Context;
20	import org.openmrs.messagesource.MessageSourceService;
21	import org.openmrs.util.PrivilegeConstants;
22	import org.springframework.beans.factory.annotation.Autowired;
23	import org.springframework.stereotype.Component;
24	import org.springframework.validation.Errors;
25	import org.springframework.validation.ValidationUtils;
26	import org.springframework.validation.Validator;
27
28	@Component("allergyValidator")
29	@Handler(supports = { Allergy.class }, order = 50)
30	public class AllergyValidator implements Validator {	1✔
31
32	@Autowired
33	private MessageSourceService messageSourceService;
34
35	@Autowired
36	private PatientService patientService;
37
38	@Override
39	public boolean supports(Class<?> clazz) {
40	return Allergy.class.isAssignableFrom(clazz);	1✔
41	}
42
43	/**
44	* @see Validator#validate(Object, org.springframework.validation.Errors)
45	* @param target
46	* @param errors
47	* <strong>Should</strong> fail for a null value
48	* <strong>Should</strong> fail if patient is null
49	* <strong>Should</strong> fail id allergenType is null
50	* <strong>Should</strong> fail if allergen is null
51	* <strong>Should</strong> fail if codedAllergen is null
52	* <strong>Should</strong> fail if nonCodedAllergen is null and allergen is set to other non coded
53	* <strong>Should</strong> reject a duplicate allergen
54	* <strong>Should</strong> reject a duplicate non coded allergen
55	* <strong>Should</strong> pass for a valid allergy
56	* <strong>Should</strong> reject numeric values and symbols on reactionNonCoded
57	*/
58	@Override
59	public void validate(Object target, Errors errors) {
60
61	if (target == null) {	1✔
62	throw new IllegalArgumentException("Allergy should not be null");	1✔
63	}
64
65	ValidationUtils.rejectIfEmpty(errors, "patient", "allergyapi.patient.required");	1✔
66
67	Allergy allergy = (Allergy) target;	1✔
68
69	if (allergy.getReactionNonCoded() != null) {	1✔
70	if (NumberUtils.isParsable(allergy.getReactionNonCoded())) {	1✔
71	errors.rejectValue("reactionNonCoded", "error.allergyapi.allergy.ReactionNonCoded.cannotBeNumeric");	1✔
72	}
73	}
74	if (allergy.getAllergen() == null) {	1✔
75	errors.rejectValue("allergen", "allergyapi.allergen.required");	1✔
76	} else {
77	Allergen allergen = allergy.getAllergen();	1✔
78	if (allergen.getAllergenType() == null) {	1✔
79	errors.rejectValue("allergen", "allergyapi.allergenType.required");	1✔
80	}
81
82	if (allergen.getCodedAllergen() == null && StringUtils.isBlank(allergen.getNonCodedAllergen())) {	1✔
83	errors.rejectValue("allergen", "allergyapi.allergen.codedOrNonCodedAllergen.required");	1✔
84	} else if (!allergen.isCoded() && StringUtils.isBlank(allergen.getNonCodedAllergen())) {	1✔
85	errors.rejectValue("allergen", "allergyapi.allergen.nonCodedAllergen.required");	1✔
86	}
87
88	if (allergy.getAllergyId() == null && allergy.getPatient() != null) {	1✔
89	// this duplicate-allergen check is an internal read done while saving the allergy; a
90	// user with an allergy write privilege should not additionally need Get Allergies for
91	// it, so acquire the read privilege on their behalf
92	Allergies existingAllergies;
93	try {
94	Context.addProxyPrivilege(PrivilegeConstants.GET_ALLERGIES);	1✔
95	existingAllergies = patientService.getAllergies(allergy.getPatient());	1✔
96	} finally {
97	Context.removeProxyPrivilege(PrivilegeConstants.GET_ALLERGIES);	1✔
98	}
99	if (existingAllergies.containsAllergen(allergy)) {	1✔
100	String key = "ui.i18n.Concept.name." + allergen.getCodedAllergen().getUuid();	1✔
101	String name = messageSourceService.getMessage(key);	1✔
102	if (key.equals(name)) {	1✔
103	name = allergen.toString();	×
104	}
105	errors.rejectValue("allergen", "allergyapi.message.duplicateAllergen", new Object[] { name }, null);	1✔
106	}
107	}
108	}
109	}	1✔
110	}

openmrs / openmrs-core / 29867099098

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