TRUNK-6203: Global properties access should be privileged

api/src/main/java/org/openmrs/api/AdministrationService.java

@@ -56,6 +56,7 @@ public interface AdministrationService extends OpenmrsService {
 	 * <strong>Should</strong> find object given valid uuid
 	 * <strong>Should</strong> return null if no object found with given uuid
 	 */
+	@Authorized(PrivilegeConstants.GET_GLOBAL_PROPERTIES)
 	public GlobalProperty getGlobalPropertyByUuid(String uuid);
 
 	/**
@@ -90,6 +91,7 @@ public interface AdministrationService extends OpenmrsService {
 	 * <strong>Should</strong> get property value given valid property name
 	 * <strong>Should</strong> get property in case insensitive way
 	 */
+	@Authorized(PrivilegeConstants.GET_GLOBAL_PROPERTIES)
 	public String getGlobalProperty(String propertyName);
 
 	/**
@@ -106,6 +108,7 @@ public interface AdministrationService extends OpenmrsService {
 	 * <strong>Should</strong> return default value if property name does not exist
 	 * <strong>Should</strong> not fail with null default value
 	 */
+	@Authorized(PrivilegeConstants.GET_GLOBAL_PROPERTIES)
 	public String getGlobalProperty(String propertyName, String defaultValue);
 
 	/**
@@ -115,6 +118,7 @@ public interface AdministrationService extends OpenmrsService {
 	 * @return the global property that matches the given <code>propertyName</code>
 	 * <strong>Should</strong> return null when no global property match given property name
 	 */
+	@Authorized(PrivilegeConstants.GET_GLOBAL_PROPERTIES)
 	public GlobalProperty getGlobalPropertyObject(String propertyName);
 
 	/**
@@ -125,6 +129,7 @@ public interface AdministrationService extends OpenmrsService {
 	 * @since 1.5
 	 * <strong>Should</strong> return all relevant global properties in the database
 	 */
+	@Authorized(PrivilegeConstants.GET_GLOBAL_PROPERTIES)
 	public List<GlobalProperty> getGlobalPropertiesByPrefix(String prefix);
 
 	/**
@@ -135,6 +140,7 @@ public interface AdministrationService extends OpenmrsService {
 	 * @since 1.6
 	 * <strong>Should</strong> return all relevant global properties in the database
 	 */
+	@Authorized(PrivilegeConstants.GET_GLOBAL_PROPERTIES)
 	public List<GlobalProperty> getGlobalPropertiesBySuffix(String suffix);
 
 	/**
@@ -189,6 +195,7 @@ public interface AdministrationService extends OpenmrsService {
 	 * <strong>Should</strong> overwrite global property if exists
 	 * <strong>Should</strong> save a global property whose typed value is handled by a custom datatype
 	 */
+	@Authorized(PrivilegeConstants.MANAGE_GLOBAL_PROPERTIES)
 	public void setGlobalProperty(String propertyName, String propertyValue);
 
 	/**
@@ -202,6 +209,7 @@ public interface AdministrationService extends OpenmrsService {
 	 * <strong>Should</strong> fail if global property being updated does not already exist
 	 * <strong>Should</strong> update a global property whose typed value is handled by a custom datatype
 	 */
+	@Authorized(PrivilegeConstants.MANAGE_GLOBAL_PROPERTIES)
 	public void updateGlobalProperty(String propertyName, String propertyValue);
 
 	/**
@@ -313,6 +321,7 @@ public interface AdministrationService extends OpenmrsService {
 	 * @return property value in the type of the default value
 	 * @since 1.7
 	 */
+	@Authorized(PrivilegeConstants.GET_GLOBAL_PROPERTIES)
 	public <T> T getGlobalPropertyValue(String propertyName, T defaultValue);
 
 	/**

api/src/test/java/org/openmrs/annotation/StartModuleAnnotationTest.java

@@ -9,12 +9,14 @@
  */
 package org.openmrs.annotation;
 
-import static org.junit.jupiter.api.Assertions.assertNotNull;
-
 import org.junit.jupiter.api.Test;
+import org.openmrs.Role;
 import org.openmrs.api.context.Context;
-import org.openmrs.test.jupiter.BaseContextSensitiveTest;
 import org.openmrs.test.StartModule;
+import org.openmrs.test.jupiter.BaseContextSensitiveTest;
+import org.openmrs.util.RoleConstants;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 
 @StartModule({ "org/openmrs/module/include/test1-1.0-SNAPSHOT.omod", "org/openmrs/module/include/test2-1.0-SNAPSHOT.omod" })
 public class StartModuleAnnotationTest extends BaseContextSensitiveTest {

api/src/test/java/org/openmrs/aop/AuthorizationAdviceTest.java

@@ -65,7 +65,7 @@ public void before_shouldNotifyListenersAboutCheckedPrivileges() {
 		Context.getConceptService().saveConcept(concept);
 
 		String[] privileges = { PrivilegeConstants.MANAGE_CONCEPTS, PrivilegeConstants.GET_OBS,
-		        PrivilegeConstants.GET_CONCEPT_ATTRIBUTE_TYPES };
+		        PrivilegeConstants.GET_CONCEPT_ATTRIBUTE_TYPES, PrivilegeConstants.GET_GLOBAL_PROPERTIES };
 		assertThat("listener1", listener1.hasPrivileges, containsInAnyOrder(privileges));
 		assertThat("listener2", listener2.hasPrivileges, containsInAnyOrder(privileges));
 		assertThat(listener1.lacksPrivileges, empty());

api/src/test/java/org/openmrs/api/AdministrationServiceTest.java

@@ -554,7 +554,7 @@ public void filterGlobalPropertiesByViewPrivilege_shouldFilterGlobalPropertiesIf
 		GlobalProperty property = new GlobalProperty();
 		property.setProperty("test_property");
 		property.setPropertyValue("test_property_value");
-		property.setViewPrivilege(Context.getUserService().getPrivilege("Some Privilege For View Global Properties"));
+		property.setViewPrivilege(Context.getUserService().getPrivilege(PrivilegeConstants.GET_GLOBAL_PROPERTIES));
 		adminService.saveGlobalProperty(property);
 		// assert new test global property is saved properly
 		List<GlobalProperty> properties = adminService.getAllGlobalProperties();
@@ -588,9 +588,8 @@ public void getGlobalProperty_shouldFailIfUserHasNoPrivileges() {
 		Context.logout();
 		Context.authenticate(getTestUserCredentials());
 
-		APIException exception = assertThrows(APIException.class, () -> adminService.getGlobalProperty(property.getProperty()));
-		assertEquals(exception.getMessage(), String.format("Privilege: %s, required to view globalProperty: %s",
-			property.getViewPrivilege(), property.getProperty()));
+		APIAuthenticationException exception = assertThrows(APIAuthenticationException.class, () -> adminService.getGlobalProperty(property.getProperty()));
+		assertEquals(exception.getMessage(), String.format("Privileges required: %s", property.getViewPrivilege()));
 	}
 
 	/**
@@ -608,6 +607,7 @@ public void getGlobalProperty_shouldReturnGlobalPropertyIfUserIsAllowedToView()
 		Role role = Context.getUserService().getRole("Provider");
 		role.addPrivilege(property.getViewPrivilege());
 		Context.getAuthenticatedUser().addRole(role);
+		Context.addProxyPrivilege(PrivilegeConstants.GET_GLOBAL_PROPERTIES);
 
 		assertNotNull(adminService.getGlobalProperty(property.getProperty()));
 	}
@@ -625,8 +625,8 @@ public void getGlobalPropertyObject_shouldFailIfUserHasNoPrivileges() {
 		Context.authenticate(getTestUserCredentials());
 
 		APIException exception = assertThrows(APIException.class, () -> adminService.getGlobalPropertyObject(property.getProperty()));
-		assertEquals(exception.getMessage(), String.format("Privilege: %s, required to view globalProperty: %s",
-			property.getViewPrivilege(), property.getProperty()));
+		assertEquals(exception.getMessage(), String.format("Privileges required: %s",
+			property.getViewPrivilege()));
 	}
 
 	/**
@@ -644,6 +644,7 @@ public void getGlobalPropertyObject_shouldReturnGlobalPropertyIfUserIsAllowedToV
 		Role role = Context.getUserService().getRole("Provider");
 		role.addPrivilege(property.getViewPrivilege());
 		Context.getAuthenticatedUser().addRole(role);
+		Context.addProxyPrivilege(PrivilegeConstants.GET_GLOBAL_PROPERTIES);
 
 		assertNotNull(adminService.getGlobalPropertyObject(property.getProperty()));
 	}
@@ -662,8 +663,8 @@ public void updateGlobalProperty_shouldFailIfUserIsNotAllowedToEditGlobalPropert
 		Context.authenticate(getTestUserCredentials());
 
 		APIException exception = assertThrows(APIException.class, () -> adminService.updateGlobalProperty(property.getProperty(), "new-value"));
-		assertEquals(exception.getMessage(), String.format("Privilege: %s, required to edit globalProperty: %s",
-			property.getEditPrivilege(), property.getProperty()));
+		assertEquals(exception.getMessage(), String.format("Privileges required: %s",
+			property.getEditPrivilege()));
 	}
 
 	/**
@@ -682,6 +683,7 @@ public void updateGlobalProperty_shouldUpdateIfUserIsAllowedToEditGlobalProperty
 		Role role = Context.getUserService().getRole("Provider");
 		role.addPrivilege(property.getEditPrivilege());
 		Context.getAuthenticatedUser().addRole(role);
+		Context.addProxyPrivilege(PrivilegeConstants.GET_GLOBAL_PROPERTIES);
 
 		adminService.updateGlobalProperty(property.getProperty(), "new-value");
 		String newValue = adminService.getGlobalProperty(property.getProperty());
@@ -735,7 +737,7 @@ private GlobalProperty getGlobalPropertyWithViewPrivilege() {
 		GlobalProperty property = adminService.getGlobalPropertyObject("another-global-property");
 		assertNotNull(property);
 
-		Privilege viewPrivilege = Context.getUserService().getPrivilege("Some Privilege For View Global Properties");
+		Privilege viewPrivilege = Context.getUserService().getPrivilege(PrivilegeConstants.GET_GLOBAL_PROPERTIES);
 		property.setViewPrivilege(viewPrivilege);
 		property = adminService.saveGlobalProperty(property);
 		assertNotNull(property.getViewPrivilege());
@@ -752,7 +754,7 @@ private GlobalProperty getGlobalPropertyWithEditPrivilege() {
 		GlobalProperty property = adminService.getGlobalPropertyObject("another-global-property");
 		assertNotNull(property);
 
-		Privilege editPrivilege = Context.getUserService().getPrivilege("Some Privilege For Edit Global Properties");
+		Privilege editPrivilege = Context.getUserService().getPrivilege(PrivilegeConstants.MANAGE_GLOBAL_PROPERTIES);
 		property.setEditPrivilege(editPrivilege);
 		property = adminService.saveGlobalProperty(property);
 		assertNotNull(property.getEditPrivilege());

api/src/test/java/org/openmrs/api/UserServiceTest.java

@@ -261,6 +261,7 @@ public void createUser_shouldNotAllowCreatingUserWithPrivilegeCurrentUserDoesNot
 		userRole.setRole(RoleConstants.AUTHENTICATED);
 		userRole.addPrivilege(new Privilege("Add Users"));
 		currentUser.addRole(userRole);
+		Context.addProxyPrivilege(PrivilegeConstants.GET_GLOBAL_PROPERTIES);
 		// setup our expected exception
 		// we expect this to fail because the currently logged-in user lacks a privilege to be
 		// assigned to the new user
@@ -294,6 +295,7 @@ public void createUser_shouldNotAllowCreatingUserWithPrivilegesCurrentUserDoesNo
 		userRole.setRole(RoleConstants.AUTHENTICATED);
 		userRole.addPrivilege(new Privilege("Add Users"));
 		currentUser.addRole(userRole);
+		Context.addProxyPrivilege(PrivilegeConstants.GET_GLOBAL_PROPERTIES);
 		// setup our expected exception
 		// we expect this to fail because the currently logged-in user lacks a privilege to be
 		// assigned to the new user
@@ -328,7 +330,7 @@ public void createUser_shouldNotAllowAssigningSuperUserRoleIfCurrentUserDoesNotH
 		userRole.setRole(RoleConstants.AUTHENTICATED);
 		userRole.addPrivilege(new Privilege("Add Users"));
 		currentUser.addRole(userRole);
-
+		Context.addProxyPrivilege(PrivilegeConstants.GET_GLOBAL_PROPERTIES);
 		// setup our expected exception
 		// we expect this to fail because the currently logged-in user lacks a privilege to be
 		// assigned to the new user
@@ -473,7 +475,7 @@ public void changePassword_shouldMatchOnIncorrectlyHashedSha1StoredPassword() {
 		executeDataSet(XML_FILENAME);
 		Context.logout();
 		Context.authenticate("incorrectlyhashedSha1", "test");
-
+		Context.addProxyPrivilege(PrivilegeConstants.GET_GLOBAL_PROPERTIES);
 		userService.changePassword("test", "Tester12");
 
 		Context.logout(); // so that the next test reauthenticates
@@ -515,6 +517,7 @@ public void changePassword_shouldMatchOnCorrectlyHashedSha1StoredPassword() {
 		executeDataSet(XML_FILENAME);
 		Context.logout();
 		Context.authenticate("correctlyhashedSha1", "test");
+		Context.addProxyPrivilege(PrivilegeConstants.GET_GLOBAL_PROPERTIES);
 
 		userService.changePassword("test", "Tester12");
 
@@ -543,6 +546,7 @@ public void changePassword_shouldMatchOnSha512HashedPassword() {
 		executeDataSet(XML_FILENAME);
 		Context.logout();
 		Context.authenticate("userWithSha512Hash", "test");
+		Context.addProxyPrivilege(PrivilegeConstants.GET_GLOBAL_PROPERTIES);
 
 		userService.changePassword("test", "Tester12");
 
@@ -1491,6 +1495,7 @@ public void changePasswordUsingSecretAnswer_shouldUpdatePasswordIfSecretIsCorrec
 		User user = userService.getUser(6001);
 		assertFalse(user.hasPrivilege(PrivilegeConstants.EDIT_USER_PASSWORDS));
 		Context.authenticate(user.getUsername(), "userServiceTest");
+		Context.addProxyPrivilege(PrivilegeConstants.GET_GLOBAL_PROPERTIES);
 
 		userService.changePasswordUsingSecretAnswer("answer", "userServiceTest2");
 
@@ -1585,6 +1590,7 @@ public void changePasswordUsingActivationKey_shouldUpdatePasswordIfActivationKey
 
 		final String PASSWORD = "Admin123";
 		Context.authenticate(createdUser.getUsername(), "Openmr5xy");
+		Context.addProxyPrivilege(PrivilegeConstants.GET_GLOBAL_PROPERTIES);
 		userService.changePasswordUsingActivationKey(key, PASSWORD);
 		Context.authenticate(createdUser.getUsername(), PASSWORD);
 

api/src/test/java/org/openmrs/api/db/ContextDAOTest.java

@@ -32,6 +32,7 @@
 import org.openmrs.api.context.ContextAuthenticationException;
 import org.openmrs.api.db.hibernate.HibernateContextDAO;
 import org.openmrs.test.jupiter.BaseContextSensitiveTest;
+import org.openmrs.util.PrivilegeConstants;
 import org.springframework.stereotype.Component;
 
 /**
@@ -277,6 +278,8 @@ public void authenticate_shouldAuthenticateWithIncorrectHashedPassword() {
 	public void authenticate_shouldPassRegressionTestFor1580() {
 		// logout after the base setup
 		Context.logout();
+		Context.addProxyPrivilege(PrivilegeConstants.GET_GLOBAL_PROPERTIES);
+
 
 		// first we fail a login attempt
 		try {
@@ -309,7 +312,10 @@ public void authenticate_shouldPassRegressionTestFor1580() {
 
 		// those were the first eight, now the ninth request 
 		// (with the same user and right pw) should fail
-		assertThrows(ContextAuthenticationException.class, () -> dao.authenticate("admin", "test"));
+		assertThrows(ContextAuthenticationException.class, () -> {
+			Context.addProxyPrivilege(PrivilegeConstants.GET_GLOBAL_PROPERTIES);
+			dao.authenticate("admin", "test");
+		});
 	}
 
 	@Test

api/src/test/java/org/openmrs/test/StartModuleExecutionListener.java

@@ -27,6 +27,7 @@
 import org.openmrs.module.ModuleInteroperabilityTest;
 import org.openmrs.module.ModuleUtil;
 import org.openmrs.util.OpenmrsClassLoader;
+import org.openmrs.util.PrivilegeConstants;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.config.BeanDefinition;
@@ -80,6 +81,7 @@ public void prepareTestInstance(TestContext testContext) throws Exception {
 
 				if (!Context.isSessionOpen())
 					Context.openSession();
+				Context.addProxyPrivilege(PrivilegeConstants.GET_GLOBAL_PROPERTIES);
 
 				ModuleUtil.shutdown();
 
@@ -160,6 +162,7 @@ public void afterTestClass(TestContext testContext) throws Exception {
 			if (!Context.isSessionOpen()) {
 				Context.openSession();
 			}
+			Context.addProxyPrivilege(PrivilegeConstants.GET_GLOBAL_PROPERTIES);
 
 			// re-registering the bean definitions that we may have removed
 			for (String beanName : filteredDefinitions.keySet()) {

api/src/test/java/org/openmrs/test/jupiter/StartModuleExecutionListener.java

@@ -28,6 +28,7 @@
 import org.openmrs.module.ModuleUtil;
 import org.openmrs.test.StartModule;
 import org.openmrs.util.OpenmrsClassLoader;
+import org.openmrs.util.PrivilegeConstants;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.config.BeanDefinition;
@@ -78,6 +79,8 @@ public void prepareTestInstance(TestContext testContext) throws Exception {
 				if (!Context.isSessionOpen())
 					Context.openSession();
 
+				Context.addProxyPrivilege(PrivilegeConstants.GET_GLOBAL_PROPERTIES);
+
 				ModuleUtil.shutdown();
 
 				// load the omods that the dev defined for this class

api/src/test/resources/org/openmrs/api/include/AdministrationServiceTest-globalproperties.xml

@@ -21,8 +21,8 @@
   <global_property property="valid.integer" property_value="1234" uuid="b7225607-d0c4-4e44-8be5-31e1ac7e1fda"/>
   <global_property property="valid.double" property_value="1234.54" uuid="b7225607-d0c4-4e44-8be6-31e1ac7e1fda"/>
 
-	<privilege privilege="Some Privilege For View Global Properties" description="This is a test privilege for view global properties" uuid="d979d066-15e6-467c-9d4b-cb575ef12345"/>
-	<privilege privilege="Some Privilege For Edit Global Properties" description="This is a test privilege for edit global properties" uuid="d979d066-15e6-467c-9d4b-cb575ef54321"/>
+	<privilege privilege="Get Global Properties" description="This is a test privilege for view global properties" uuid="d979d066-15e6-467c-9d4b-cb575ef12345"/>
+	<privilege privilege="Manage Global Properties" description="This is a test privilege for edit global properties" uuid="d979d066-15e6-467c-9d4b-cb575ef54321"/>
 	<privilege privilege="Some Privilege For Delete Global Properties" description="This is a test privilege for delete global properties" uuid="d979d066-15e6-467c-9d4b-cb575ef67890"/>
 	<users user_id="5506" person_id="501" system_id="7-5" username="test_user" password="4a1750c8607d0fa237de36c6305715c223415189" salt="c788c6ad82a157b712392ca695dfcf2eed193d7f" secret_question="" creator="1" date_created="2008-08-15 15:57:09.0" changed_by="1" date_changed="2008-08-18 11:51:56.0" retired="false" retire_reason="" uuid="06d05314-e132-11de-babe-001e37123456"/>
 </dataset>

web/src/main/java/org/openmrs/module/web/filter/ModuleFilterChain.java

@@ -9,6 +9,9 @@
  */
 package org.openmrs.module.web.filter;
 
+import org.openmrs.api.context.Context;
+import org.openmrs.util.PrivilegeConstants;
+
 import java.io.IOException;
 import java.util.Collection;
 import java.util.Iterator;

web/src/main/java/org/openmrs/web/filter/GZIPFilter.java

@@ -19,6 +19,7 @@
 import org.openmrs.api.APIException;
 import org.openmrs.api.context.Context;
 import org.openmrs.util.OpenmrsConstants;
+import org.openmrs.util.PrivilegeConstants;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.web.filter.OncePerRequestFilter;
@@ -124,8 +125,10 @@ private boolean isGZIPEnabled() {
 		}
 
 		try {
+			Context.addProxyPrivilege(PrivilegeConstants.GET_GLOBAL_PROPERTIES);
 			String gzipEnabled = Context.getAdministrationService().getGlobalProperty(
 			    OpenmrsConstants.GLOBAL_PROPERTY_GZIP_ENABLED, "");
+			Context.removeProxyPrivilege(PrivilegeConstants.GET_GLOBAL_PROPERTIES);
 
 			cachedGZipEnabledFlag = Boolean.valueOf(gzipEnabled);
 			return cachedGZipEnabledFlag;

web/src/main/java/org/openmrs/web/filter/StartupFilter.java

@@ -55,6 +55,7 @@
 import org.openmrs.logging.OpenmrsLoggingUtil;
 import org.openmrs.util.LocaleUtility;
 import org.openmrs.util.OpenmrsUtil;
+import org.openmrs.util.PrivilegeConstants;
 import org.openmrs.web.Listener;
 import org.openmrs.web.WebConstants;
 import org.openmrs.web.filter.initialization.InitializationFilter;