TRUNK-6187 Fix changing password by superuser if locked

openmrs · Aug 17, 2023 · 8896fe3 · 8896fe3
1 parent dc28384
commit 8896fe3
Show file tree

Hide file tree

Showing 6 changed files with 32 additions and 29 deletions.
diff --git a/api/src/main/java/org/openmrs/api/UserService.java b/api/src/main/java/org/openmrs/api/UserService.java
@@ -328,7 +328,6 @@ public interface UserService extends OpenmrsService {
 	 * <strong>Should</strong> match on sha512 hashed password
 	 * <strong>Should</strong> be able to update password multiple times
 	 * <strong>Should</strong> respect locking via runtime properties
-	 * <strong>Should</strong> respect locking via runtime properties except for startup
 	 */
 	@Logging(ignoredArgumentIndexes = { 0, 1 })
 	public void changePassword(String pw, String pw2) throws APIException;

diff --git a/api/src/main/java/org/openmrs/api/impl/UserServiceImpl.java b/api/src/main/java/org/openmrs/api/impl/UserServiceImpl.java
@@ -651,9 +651,8 @@ public void changePassword(User user, String oldPassword, String newPassword) th
 	}
 
 	private void updatePassword(User user, String newPassword) {
-		if (user.isSuperUser() && Boolean.valueOf(Context.getRuntimeProperties()
-			.getProperty(ADMIN_PASSWORD_LOCKED_PROPERTY, "false")) &&
-		!Context.hasPrivilege(PrivilegeConstants.EDIT_ADMIN_USER_PASSWORD)) {
+		if ("admin".equals(user.getUsername()) && Boolean.valueOf(Context.getRuntimeProperties()
+			.getProperty(ADMIN_PASSWORD_LOCKED_PROPERTY, "false"))) {
 			throw new APIException("admin.password.is.locked");
 		}
 

diff --git a/api/src/main/java/org/openmrs/util/PrivilegeConstants.java b/api/src/main/java/org/openmrs/util/PrivilegeConstants.java
@@ -191,9 +191,6 @@ private PrivilegeConstants() {
 
 	@AddOnStartup(description = "Able to change the passwords of users in OpenMRS")
 	public static final String EDIT_USER_PASSWORDS = "Edit User Passwords";
-
-	@AddOnStartup(description = "Able to change the admin user password even if locked (for startup use only)")
-	public static final String EDIT_ADMIN_USER_PASSWORD = "Edit Admin User Password";
 
 	@AddOnStartup(description = "Able to add patient encounters")
 	public static final String ADD_ENCOUNTERS = "Add Encounters";

diff --git a/api/src/test/java/org/openmrs/api/UserServiceTest.java b/api/src/test/java/org/openmrs/api/UserServiceTest.java
@@ -28,6 +28,7 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Properties;
 import java.util.Set;
 
 import org.apache.commons.lang3.reflect.FieldUtils;
@@ -393,32 +394,29 @@ public void changePassword_shouldBeAbleToUpdatePasswordMultipleTimes() {
 
 	@Test
 	public void changePassword_shouldRespectLockingViaRuntimeProperty() {
+		assertThat("admin", is(Context.getAuthenticatedUser().getUsername()));
 		User u = userService.getUserByUsername(ADMIN_USERNAME);
 
-		Context.getRuntimeProperties().setProperty(UserService.ADMIN_PASSWORD_LOCKED_PROPERTY, "true");
-
-		assertThrows(APIException.class, () -> userService.changePassword("admin", "SuperAdmin123"));
-
-		Context.getRuntimeProperties().setProperty(UserService.ADMIN_PASSWORD_LOCKED_PROPERTY, "True");
-
-		assertThrows(APIException.class, () -> userService.changePassword("admin", "SuperAdmin123"));
+		assertThat(u.isSuperUser(), is(true));
 
-		Context.getRuntimeProperties().remove(UserService.ADMIN_PASSWORD_LOCKED_PROPERTY);
-
-		userService.changePassword(u,"test", "SuperAdmin123");
-	}
+		Properties props = Context.getRuntimeProperties();
+		props.setProperty(UserService.ADMIN_PASSWORD_LOCKED_PROPERTY, "true");
+		Context.setRuntimeProperties(props);
 
-	@Test
-	public void changePassword_shouldRespectLockingViaRuntimePropertyExceptForStartup() {
-		User u = userService.getUserByUsername(ADMIN_USERNAME);
+		APIException apiException = assertThrows(APIException.class, () -> userService.changePassword(u,"test", "SuperAdmin123"));
+		
+		assertThat(apiException.getMessage(), is("admin.password.is.locked"));
 
-		Context.getRuntimeProperties().setProperty(UserService.ADMIN_PASSWORD_LOCKED_PROPERTY, "true");
+		props.setProperty(UserService.ADMIN_PASSWORD_LOCKED_PROPERTY, "True");
+		Context.setRuntimeProperties(props);
 
-		Context.addProxyPrivilege(PrivilegeConstants.EDIT_ADMIN_USER_PASSWORD);
+		apiException = assertThrows(APIException.class, () -> userService.changePassword(u,"test", "SuperAdmin123"));
+		assertThat(apiException.getMessage(), is("admin.password.is.locked"));
+
+		props.remove(UserService.ADMIN_PASSWORD_LOCKED_PROPERTY);
+		Context.setRuntimeProperties(props);
 
 		userService.changePassword(u,"test", "SuperAdmin123");
-
-		Context.removeProxyPrivilege(PrivilegeConstants.EDIT_ADMIN_USER_PASSWORD);
 	}
 
 	@Test

diff --git a/docker-compose.yml b/docker-compose.yml
@@ -32,8 +32,6 @@ services:
             cache_from:
                 - openmrs/openmrs-core:${TAG:-dev}
                 - openmrs/openmrs-core:${TAG:-nightly}
-        links:
-            - db
         ports:
             - "8080:8080"
         environment:

diff --git a/web/src/main/java/org/openmrs/web/filter/initialization/InitializationFilter.java b/web/src/main/java/org/openmrs/web/filter/initialization/InitializationFilter.java
@@ -43,6 +43,7 @@
 import org.openmrs.ImplementationId;
 import org.openmrs.api.APIAuthenticationException;
 import org.openmrs.api.PasswordException;
+import org.openmrs.api.UserService;
 import org.openmrs.api.context.Context;
 import org.openmrs.api.context.ContextAuthenticationException;
 import org.openmrs.liquibase.ChangeLogDetective;
@@ -1782,9 +1783,20 @@ && getRuntimePropertiesFile().setReadable(true)) {
 							if (wizardModel.createTables) {
 								try {
 									Context.authenticate("admin", "test");
-									Context.addProxyPrivilege(PrivilegeConstants.EDIT_ADMIN_USER_PASSWORD);
+
+									Properties props = Context.getRuntimeProperties();
+									String initValue = props.getProperty(UserService.ADMIN_PASSWORD_LOCKED_PROPERTY);
+									props.setProperty(UserService.ADMIN_PASSWORD_LOCKED_PROPERTY, "false");
+									Context.setRuntimeProperties(props);
+
 									Context.getUserService().changePassword("test", wizardModel.adminUserPassword);
-									Context.removeProxyPrivilege(PrivilegeConstants.EDIT_ADMIN_USER_PASSWORD);
+
+									if (initValue == null) {
+										props.remove(UserService.ADMIN_PASSWORD_LOCKED_PROPERTY);
+									} else {
+										props.setProperty(UserService.ADMIN_PASSWORD_LOCKED_PROPERTY, initValue);
+									}
+									Context.setRuntimeProperties(props);
 									Context.logout();
 								}
 								catch (ContextAuthenticationException ex) {