You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by wu...@apache.org on 2022/11/08 18:37:05 UTC
[ambari] branch trunk updated: AMBARI-25201: check acting users password on change password request (#3444)
This is an automated email from the ASF dual-hosted git repository.
wuzhiguo pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ambari.git
The following commit(s) were added to refs/heads/trunk by this push:
new a4d579c67a AMBARI-25201: check acting users password on change password request (#3444)
a4d579c67a is described below
commit a4d579c67af031092873e07c31c5970c04a6cc2d
Author: Zhiguo Wu <wu...@apache.org>
AuthorDate: Wed Nov 9 02:36:59 2022 +0800
AMBARI-25201: check acting users password on change password request (#3444)
---
.../server/security/authorization/Users.java | 23 ++++--
.../server/security/authorization/TestUsers.java | 24 +++++-
.../server/security/authorization/UsersTest.java | 92 +++++++++++++++++++++-
3 files changed, 126 insertions(+), 13 deletions(-)
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java b/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java
index fc32d62818..d8d36c3e0a 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/Users.java
@@ -26,6 +26,7 @@ import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
import java.util.regex.Pattern;
@@ -1248,14 +1249,13 @@ public class Users {
if (userAuthenticationEntity != null) {
if (userAuthenticationEntity.getAuthenticationType() == UserAuthenticationType.LOCAL) {
- // If the authentication record represents a local password and the authenticated user is
- // changing the password for himself, ensure the old key value matches the current key value
- // If the authenticated user is can manager users and is not changing his own password, there
- // is no need to check that the authenticated user knows the current password - just update it.
- if (isSelf &&
- (StringUtils.isEmpty(currentKey) || !passwordEncoder.matches(currentKey, userAuthenticationEntity.getAuthenticationKey()))) {
- // The authenticated user is the same user as subject user and the correct current password
- // was not supplied.
+
+ String expectedCurrentKey = isSelf
+ ? userAuthenticationEntity.getAuthenticationKey()
+ : getAuthenticatedUserLocalAuthenticationMethod().
+ orElseThrow(() -> new AmbariException("Authentication error")).getAuthenticationKey();
+
+ if (StringUtils.isEmpty(currentKey) || !passwordEncoder.matches(currentKey, expectedCurrentKey)) {
throw new AmbariException("Wrong current password provided");
}
@@ -1273,6 +1273,13 @@ public class Users {
}
}
+ private Optional<AuthenticationMethod> getAuthenticatedUserLocalAuthenticationMethod() {
+ User authenticatedUser = getUser(AuthorizationHelper.getAuthenticatedId());
+ return authenticatedUser.getAuthenticationMethods().stream()
+ .filter(am -> UserAuthenticationType.LOCAL.equals(am.getAuthenticationType()))
+ .findAny();
+ }
+
public void removeAuthentication(String username, Long authenticationId) {
removeAuthentication(getUserEntity(username), authenticationId);
}
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/TestUsers.java b/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/TestUsers.java
index 2d065d3b82..da47027efa 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/TestUsers.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/TestUsers.java
@@ -17,6 +17,10 @@
*/
package org.apache.ambari.server.security.authorization;
+import static java.util.Collections.emptySet;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.mock;
+import static org.easymock.EasyMock.replay;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNotSame;
@@ -50,6 +54,7 @@ import org.apache.ambari.server.orm.entities.ResourceEntity;
import org.apache.ambari.server.orm.entities.ResourceTypeEntity;
import org.apache.ambari.server.orm.entities.UserAuthenticationEntity;
import org.apache.ambari.server.orm.entities.UserEntity;
+import org.apache.ambari.server.security.authentication.AmbariUserDetailsImpl;
import org.apache.ambari.server.security.ldap.LdapBatchDto;
import org.apache.ambari.server.security.ldap.LdapGroupDto;
import org.apache.ambari.server.security.ldap.LdapUserDto;
@@ -58,6 +63,9 @@ import org.easymock.EasyMock;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.core.context.SecurityContext;
+import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.crypto.password.PasswordEncoder;
import com.google.inject.Guice;
@@ -164,6 +172,8 @@ public class TestUsers {
users.grantAdminPrivilege(userEntity);
users.addLocalAuthentication(userEntity, "admin");
+ setAuthenticatedUser(userEntity);
+
userEntity = users.createUser("user", "user", "user");
users.addLocalAuthentication(userEntity, "user");
@@ -178,7 +188,7 @@ public class TestUsers {
foundUserEntity = userDAO.findUserByName("admin");
assertNotNull(foundUserEntity);
- users.modifyAuthentication(foundLocalAuthenticationEntity, "user", "user_new_password", false);
+ users.modifyAuthentication(foundLocalAuthenticationEntity, "admin", "user_new_password", false);
foundUserEntity = userDAO.findUserByName("user");
assertNotNull(foundUserEntity);
@@ -204,7 +214,7 @@ public class TestUsers {
assertTrue(passwordEncoder.matches("user", foundLocalAuthenticationEntity.getAuthenticationKey()));
try {
- users.modifyAuthentication(foundLocalAuthenticationEntity, "user", null, false);
+ users.modifyAuthentication(foundLocalAuthenticationEntity, "user", null, true);
fail("Null password should not be allowed");
} catch (IllegalArgumentException e) {
assertEquals("The password does not meet the password policy requirements", e.getLocalizedMessage());
@@ -616,4 +626,14 @@ public class TestUsers {
return null;
}
+
+ private void setAuthenticatedUser(UserEntity userEntity) {
+ AmbariUserDetailsImpl principal = new AmbariUserDetailsImpl(new User(userEntity), "", emptySet());
+ Authentication auth = mock(Authentication.class);
+ expect(auth.getPrincipal()).andReturn(principal).anyTimes();
+ SecurityContext securityContext = mock(SecurityContext.class);
+ expect(securityContext.getAuthentication()).andReturn(auth).anyTimes();
+ replay(auth, securityContext);
+ SecurityContextHolder.setContext(securityContext);
+ }
}
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/UsersTest.java b/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/UsersTest.java
index b32f114a21..9a3d237a4d 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/UsersTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/UsersTest.java
@@ -18,6 +18,7 @@
package org.apache.ambari.server.security.authorization;
+import static org.easymock.EasyMock.anyInt;
import static org.easymock.EasyMock.anyObject;
import static org.easymock.EasyMock.anyString;
import static org.easymock.EasyMock.capture;
@@ -25,11 +26,13 @@ import static org.easymock.EasyMock.eq;
import static org.easymock.EasyMock.expect;
import static org.easymock.EasyMock.expectLastCall;
import static org.easymock.EasyMock.newCapture;
+import static org.junit.Assert.assertEquals;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
+import java.util.Objects;
import javax.annotation.Nullable;
import javax.persistence.EntityManager;
@@ -50,23 +53,25 @@ import org.apache.ambari.server.orm.entities.MemberEntity;
import org.apache.ambari.server.orm.entities.PermissionEntity;
import org.apache.ambari.server.orm.entities.PrincipalEntity;
import org.apache.ambari.server.orm.entities.PrivilegeEntity;
+import org.apache.ambari.server.orm.entities.UserAuthenticationEntity;
import org.apache.ambari.server.orm.entities.UserEntity;
import org.apache.ambari.server.security.encryption.Encryptor;
import org.apache.ambari.server.state.stack.OsFamily;
import org.easymock.Capture;
import org.easymock.EasyMock;
import org.easymock.EasyMockSupport;
+import org.junit.Assert;
import org.junit.Test;
import org.springframework.security.crypto.password.PasswordEncoder;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.inject.AbstractModule;
import com.google.inject.Guice;
import com.google.inject.Injector;
import com.google.inject.TypeLiteral;
import com.google.inject.name.Names;
-import junit.framework.Assert;
-
public class UsersTest extends EasyMockSupport {
private static final String SERVICEOP_USER_NAME = "serviceopuser";
@@ -179,6 +184,52 @@ public class UsersTest extends EasyMockSupport {
users.createUser(SERVICEOP_USER_NAME, SERVICEOP_USER_NAME, SERVICEOP_USER_NAME);
}
+ @Test
+ public void modifyAuthentication_local_bySameUser() throws AmbariException {
+ // given
+ UserAuthenticationEntity entity = initForModifyAuthentication();
+
+ // when
+ Users users = injector.getInstance(Users.class);
+ users.modifyAuthentication(entity, "hello", "world", true);
+
+ // then
+ assertEquals("world", entity.getAuthenticationKey());
+ }
+
+ @Test(expected = AmbariException.class)
+ public void modifyAuthentication_local_bySameUser_wrongPassword() throws AmbariException {
+ // given
+ UserAuthenticationEntity entity = initForModifyAuthentication();
+
+ // when
+ Users users = injector.getInstance(Users.class);
+ users.modifyAuthentication(entity, "12345", "world", true);
+ }
+
+ @Test
+ public void modifyAuthentication_local_byAdminUser() throws AmbariException {
+ // given
+ UserAuthenticationEntity entity = initForModifyAuthentication();
+
+ // when
+ Users users = injector.getInstance(Users.class);
+ users.modifyAuthentication(entity, "admin1234", "world", false);
+
+ // then
+ assertEquals("world", entity.getAuthenticationKey());
+ }
+
+ @Test(expected = AmbariException.class)
+ public void modifyAuthentication_local_byAdminUser_wrongPassword() throws AmbariException {
+ // given
+ UserAuthenticationEntity entity = initForModifyAuthentication();
+
+ // when
+ Users users = injector.getInstance(Users.class);
+ users.modifyAuthentication(entity, "wrong password", "world", false);
+ }
+
private void initForCreateUser(@Nullable UserEntity existingUser) {
UserDAO userDao = createStrictMock(UserDAO.class);
expect(userDao.findUserByName(anyString())).andReturn(existingUser);
@@ -190,6 +241,35 @@ public class UsersTest extends EasyMockSupport {
createInjector(userDao, entityManager);
}
+ private UserAuthenticationEntity initForModifyAuthentication() {
+ UserAuthenticationEntity userEntity = new UserAuthenticationEntity();
+ userEntity.setAuthenticationKey("hello");
+ userEntity.setAuthenticationType(UserAuthenticationType.LOCAL);
+
+ EntityManager manager = mock(EntityManager.class);
+ expect(manager.merge(userEntity)).andReturn(userEntity).once();
+
+ UserDAO dao = createMock(UserDAO.class);
+ UserEntity admin = new UserEntity();
+ admin.setUserId(-1);
+ admin.setUserName("admin");
+ PrincipalEntity principalEntity = new PrincipalEntity();
+ admin.setPrincipal(principalEntity);
+ PrivilegeEntity privilegeEntity = new PrivilegeEntity();
+ principalEntity.setPrivileges(ImmutableSet.of(privilegeEntity));
+ PermissionEntity permissionEntity = new PermissionEntity();
+ privilegeEntity.setPermission(permissionEntity);
+ permissionEntity.setPermissionName(PermissionEntity.AMBARI_ADMINISTRATOR_PERMISSION_NAME);
+ UserAuthenticationEntity adminAuthentication = new UserAuthenticationEntity();
+ admin.setAuthenticationEntities(ImmutableList.of(adminAuthentication));
+ adminAuthentication.setAuthenticationKey("admin1234");
+ expect(dao.findByPK(anyInt())).andReturn(admin).anyTimes();
+
+ createInjector(dao, manager);
+ replayAll();
+ return userEntity;
+ }
+
private void createInjector() {
createInjector(createMock(UserDAO.class), createMock(EntityManager.class));
}
@@ -204,13 +284,19 @@ public class UsersTest extends EasyMockSupport {
bind(UserDAO.class).toInstance(mockUserDao);
bind(MemberDAO.class).toInstance(createMock(MemberDAO.class));
bind(PrivilegeDAO.class).toInstance(createMock(PrivilegeDAO.class));
- bind(PasswordEncoder.class).toInstance(createMock(PasswordEncoder.class));
bind(HookService.class).toInstance(createMock(HookService.class));
bind(HookContextFactory.class).toInstance(createMock(HookContextFactory.class));
bind(PrincipalDAO.class).toInstance(createMock(PrincipalDAO.class));
bind(Configuration.class).toInstance(createNiceMock(Configuration.class));
bind(new TypeLiteral<Encryptor<AmbariServerConfiguration>>() {}).annotatedWith(Names.named("AmbariServerConfigurationEncryptor")).toInstance(Encryptor.NONE);
bind(AmbariLdapConfigurationProvider.class).toInstance(createMock(AmbariLdapConfigurationProvider.class));
+
+ PasswordEncoder nopEncoder = createMock(PasswordEncoder.class);
+ expect(nopEncoder.matches(anyString(), anyString())).andAnswer(
+ () -> Objects.equals(EasyMock.getCurrentArguments()[0], EasyMock.getCurrentArguments()[1])).anyTimes();
+ expect(nopEncoder.encode(anyString())).andAnswer(
+ () -> (String)EasyMock.getCurrentArguments()[0]).anyTimes();
+ bind(PasswordEncoder.class).toInstance(nopEncoder);
}
});
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@ambari.apache.org
For additional commands, e-mail: commits-help@ambari.apache.org