From 97360f63a005b860c3c67c5067be71b212fc4044 Mon Sep 17 00:00:00 2001 From: rkorytkowski Date: Thu, 25 Apr 2024 15:07:19 +0200 Subject: [PATCH] TRUNK-6228 Protect admin credentials not working if username not set to admin fix for tests --- .../org/openmrs/api/impl/UserServiceImpl.java | 2 +- .../java/org/openmrs/api/UserServiceTest.java | 23 ++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/api/src/main/java/org/openmrs/api/impl/UserServiceImpl.java b/api/src/main/java/org/openmrs/api/impl/UserServiceImpl.java index c63b1a3dde73..6f51cdf0f0c9 100644 --- a/api/src/main/java/org/openmrs/api/impl/UserServiceImpl.java +++ b/api/src/main/java/org/openmrs/api/impl/UserServiceImpl.java @@ -652,7 +652,7 @@ public void changePassword(User user, String oldPassword, String newPassword) th throw new APIException("new.password.equal.to.old", (Object[]) null); } - if ("admin".equals(user.getSystemId()) && Boolean.parseBoolean( + if (("admin".equals(user.getSystemId()) || "admin".equals(user.getUsername())) && Boolean.parseBoolean( Context.getRuntimeProperties().getProperty(ADMIN_PASSWORD_LOCKED_PROPERTY, "false"))) { throw new APIException("admin.password.is.locked"); } diff --git a/api/src/test/java/org/openmrs/api/UserServiceTest.java b/api/src/test/java/org/openmrs/api/UserServiceTest.java index f876fc3b80ed..af2f6b6324dd 100644 --- a/api/src/test/java/org/openmrs/api/UserServiceTest.java +++ b/api/src/test/java/org/openmrs/api/UserServiceTest.java @@ -397,9 +397,11 @@ public void changePassword_shouldBeAbleToUpdatePasswordMultipleTimes() { @Test public void changePassword_shouldRespectLockingViaRuntimeProperty() { assertThat("admin", is(Context.getAuthenticatedUser().getUsername())); + assertTrue(Context.getAuthenticatedUser().isSuperUser()); + User u = userService.getUserByUsername(ADMIN_USERNAME); - assertThat(u.getSystemId(), is("admin")); + assertThat(u.getUsername(), is("admin")); Properties props = Context.getRuntimeProperties(); props.setProperty(UserService.ADMIN_PASSWORD_LOCKED_PROPERTY, "true"); @@ -421,6 +423,25 @@ public void changePassword_shouldRespectLockingViaRuntimeProperty() { userService.changePassword(u,"test", "SuperAdmin123"); } + @Test + public void changePassword_shouldRespectLockingViaRuntimePropertyForSystemIdAdminAndNoUsername() { + assertThat("admin", is(Context.getAuthenticatedUser().getUsername())); + assertTrue(Context.getAuthenticatedUser().isSuperUser()); + + User u = userService.getUserByUsername(ADMIN_USERNAME); + + u.setSystemId("admin"); + u.setUsername(null); + + Properties props = Context.getRuntimeProperties(); + props.setProperty(UserService.ADMIN_PASSWORD_LOCKED_PROPERTY, "true"); + Context.setRuntimeProperties(props); + + APIException apiException = assertThrows(APIException.class, () -> userService.changePassword(u, "test", "SuperAdmin123")); + + assertThat(apiException.getMessage(), is("admin.password.is.locked")); + } + @Test public void saveUser_shouldGrantNewRolesInRolesListToUser() { // add in some basic properties