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 b400cc79f6 AMBARI-25268: implement configurable password policy for Ambari users (#3450) b400cc79f6 is described below commit b400cc79f60cf89f74ac0ce4160dcf87788c1c11 Author: Zhiguo Wu <wuzhi...@apache.org> AuthorDate: Fri Nov 4 11:24:12 2022 +0800 AMBARI-25268: implement configurable password policy for Ambari users (#3450) --- .../ambari/server/configuration/Configuration.java | 42 ++++++++++++++++++++++ .../ambari/server/controller/AmbariServer.java | 1 + .../controller/internal/UserResourceProvider.java | 8 ++++- .../server/security/authorization/Users.java | 19 +++++----- .../server/security/authorization/TestUsers.java | 22 +++++++++--- 5 files changed, 79 insertions(+), 13 deletions(-) diff --git a/ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java b/ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java index c6e8404d9f..93a76625d7 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java @@ -50,6 +50,7 @@ import java.util.concurrent.BlockingQueue; import java.util.concurrent.Callable; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; +import java.util.regex.Pattern; import org.apache.ambari.annotations.Experimental; import org.apache.ambari.annotations.ExperimentalFeature; @@ -507,6 +508,22 @@ public class Configuration { public static final ConfigurationProperty<String> AGENT_USE_SSL = new ConfigurationProperty<>( "agent.ssl", "true"); + /** + * Configurable password policy for Ambari users + */ + @Markdown( + description = "Determines Ambari user password policy. Passwords should match the regex") + public static final ConfigurationProperty<String> PASSWORD_POLICY_REGEXP = new ConfigurationProperty<>( + "security.password.policy.regexp", ".*"); + + /** + * Configurable password policy for Ambari users + */ + @Markdown( + description = "Password policy description that is shown to users") + public static final ConfigurationProperty<String> PASSWORD_POLICY_DESCRIPTION = new ConfigurationProperty<>( + "security.password.policy.description", ""); + /** * Determines whether the Ambari Agent host names should be validated against * a regular expression to ensure that they are well-formed. @@ -2667,6 +2684,17 @@ public class Configuration { } } + /** + * Validate password policy regexp syntax + * @throws java.util.regex.PatternSyntaxException If the expression's syntax is invalid + */ + public void validatePasswordPolicyRegexp() { + String regexp = getPasswordPolicyRegexp(); + if (!StringUtils.isEmpty(regexp) && !regexp.equalsIgnoreCase(".*")) { + Pattern.compile(regexp); + } + } + /** * Ldap username collision handling behavior. * ADD - append the new LDAP entry to the set of existing authentication methods. @@ -4044,6 +4072,20 @@ public class Configuration { return getProperty(MYSQL_JAR_NAME); } + /** + * @return Configurable password policy for Ambari users + */ + public String getPasswordPolicyRegexp() { + return getProperty(PASSWORD_POLICY_REGEXP); + } + + /** + * @return Password policy explanation according to regexp + */ + public String getPasswordPolicyDescription() { + return getProperty(PASSWORD_POLICY_DESCRIPTION); + } + public JPATableGenerationStrategy getJPATableGenerationStrategy() { return JPATableGenerationStrategy.fromString( System.getProperty(SERVER_JDBC_GENERATE_TABLES.getKey())); diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java index 40fe567922..fbb571caa4 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java @@ -1086,6 +1086,7 @@ public class AmbariServer { // check if this instance is the active instance Configuration config = injector.getInstance(Configuration.class); + config.validatePasswordPolicyRegexp(); if (!config.isActiveInstance()) { String errMsg = "This instance of ambari server is not designated as active. Cannot start ambari server." + "The property active.instance is set to false in ambari.properties"; diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UserResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UserResourceProvider.java index 8c5a8ff35f..2fa99cf2a9 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UserResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UserResourceProvider.java @@ -380,6 +380,12 @@ public class UserResourceProvider extends AbstractControllerResourceProvider imp String username = request.getUsername(); String displayName = StringUtils.defaultIfEmpty(request.getDisplayName(), username); String localUserName = StringUtils.defaultIfEmpty(request.getLocalUserName(), username); + String password = request.getPassword(); + // Setting a user's the password here is to allow for backward compatibility with pre-Ambari-3.0 + // versions so that the contract for REST API V1 is maintained. + if (!StringUtils.isEmpty(password)) { + users.validatePassword(password); + } UserEntity userEntity = users.createUser(username, localUserName, displayName, request.isActive()); if (userEntity != null) { @@ -389,7 +395,7 @@ public class UserResourceProvider extends AbstractControllerResourceProvider imp // Setting a user's the password here is to allow for backward compatibility with pre-Ambari-3.0 // versions so that the contract for REST API V1 is maintained. - if (!StringUtils.isEmpty(request.getPassword())) { + if (!StringUtils.isEmpty(password)) { // This is performed as a user administrator since the authorization check was done prior // to executing #createResourcesAuthorized. addOrUpdateLocalAuthenticationSource(true, userEntity, request.getPassword(), null); 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 b9258605c6..fc32d62818 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 @@ -27,6 +27,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.regex.Pattern; import javax.persistence.EntityManager; import javax.persistence.OptimisticLockException; @@ -1746,20 +1747,22 @@ public class Users { } /** - * Validates the password meets configured requirements. + * Validates the password meets configured requirements according ambari.properties. + * * <p> - * In the future this may be configurable. For now just make sure the password is not empty. * * @param password the password - * @return true if the password is valid; false otherwise + * @throws IllegalArgumentException if password does not meet the password policy requirements */ - public boolean validatePassword(String password) throws AmbariException { - // TODO: validate the new password... + public void validatePassword(String password) { if (StringUtils.isEmpty(password)) { - throw new AmbariException("The new password does not meet the Ambari password requirements"); + throw new IllegalArgumentException("The password does not meet the password policy requirements"); + } + String regexp = configuration.getPasswordPolicyRegexp(); + if (!StringUtils.isEmpty(regexp) && (!Pattern.matches(regexp,password))) { + final String msg = "The password does not meet the Ambari user password policy : " + configuration.getPasswordPolicyDescription(); + throw new IllegalArgumentException(msg); } - - return true; } /** 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 f7174f5bf4..2d065d3b82 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 @@ -31,6 +31,7 @@ import java.util.List; import org.apache.ambari.server.AmbariException; import org.apache.ambari.server.H2DatabaseCleaner; +import org.apache.ambari.server.configuration.Configuration; import org.apache.ambari.server.ldap.domain.AmbariLdapConfiguration; import org.apache.ambari.server.ldap.service.AmbariLdapConfigurationProvider; import org.apache.ambari.server.orm.GuiceJpaInitializer; @@ -86,6 +87,8 @@ public class TestUsers { protected PrincipalDAO principalDAO; @Inject protected PasswordEncoder passwordEncoder; + @Inject + protected Configuration configuration; @Before public void setup() throws AmbariException { @@ -203,16 +206,27 @@ public class TestUsers { try { users.modifyAuthentication(foundLocalAuthenticationEntity, "user", null, false); fail("Null password should not be allowed"); - } catch (AmbariException e) { - assertEquals("The new password does not meet the Ambari password requirements", e.getLocalizedMessage()); + } catch (IllegalArgumentException e) { + assertEquals("The password does not meet the password policy requirements", e.getLocalizedMessage()); } try { users.modifyAuthentication(foundLocalAuthenticationEntity, "user", "", false); fail("Empty password should not be allowed"); - } catch (AmbariException e) { - assertEquals("The new password does not meet the Ambari password requirements", e.getLocalizedMessage()); + } catch (IllegalArgumentException e) { + assertEquals("The password does not meet the password policy requirements", e.getLocalizedMessage()); + } + + //Minimum eight characters, at least one letter and one number: + configuration.setProperty(Configuration.PASSWORD_POLICY_REGEXP, "^(?=.*[A-Za-z])(?=.*\\d)[A-Za-z\\d]{8,}$"); + configuration.setProperty(Configuration.PASSWORD_POLICY_DESCRIPTION, "test description"); + try { + users.modifyAuthentication(foundLocalAuthenticationEntity, "user", "abc123", false); + fail("Should not pass validation"); + } catch (IllegalArgumentException e) { + assertEquals("The password does not meet the Ambari user password policy : test description", e.getLocalizedMessage()); } + users.modifyAuthentication(foundLocalAuthenticationEntity, "user", "abcd1234", false); } @Test --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@ambari.apache.org For additional commands, e-mail: commits-h...@ambari.apache.org