diff --git a/src/Interpreters/Access/InterpreterCreateUserQuery.cpp b/src/Interpreters/Access/InterpreterCreateUserQuery.cpp index b5d5c13d9e4..75b0aed24c1 100644 --- a/src/Interpreters/Access/InterpreterCreateUserQuery.cpp +++ b/src/Interpreters/Access/InterpreterCreateUserQuery.cpp @@ -65,8 +65,17 @@ namespace user.authentication_methods.emplace_back(); } - // a leading IDENTIFIED WITH will drop existing authentication methods in favor of new ones - if (replace_authentication_methods) + bool has_no_password_authentication_method = std::find_if( + user.authentication_methods.begin(), + user.authentication_methods.end(), + [](const AuthenticationData & authentication_method) + { + return authentication_method.getType() == AuthenticationType::NO_PASSWORD; + }) != user.authentication_methods.end(); + + // 1. a leading IDENTIFIED WITH will drop existing authentication methods in favor of new ones. + // 2. if the user contains an auth method of type NO_PASSWORD and another one is being added, NO_PASSWORD must be dropped + if (replace_authentication_methods || (has_no_password_authentication_method && !authentication_methods.empty())) { user.authentication_methods.clear(); } diff --git a/src/Parsers/Access/ParserCreateUserQuery.cpp b/src/Parsers/Access/ParserCreateUserQuery.cpp index 11f79acd979..96294c380a0 100644 --- a/src/Parsers/Access/ParserCreateUserQuery.cpp +++ b/src/Parsers/Access/ParserCreateUserQuery.cpp @@ -596,6 +596,19 @@ bool ParserCreateUserQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expec break; } + bool has_no_password_authentication_method = std::find_if( + auth_data.begin(), + auth_data.end(), + [](const std::shared_ptr & ast_authentication_data) + { + return ast_authentication_data->type == AuthenticationType::NO_PASSWORD; + }) != auth_data.end(); + + if (has_no_password_authentication_method && auth_data.size() > 1) + { + throw Exception(ErrorCodes::BAD_ARGUMENTS, "NO_PASSWORD Authentication method cannot co-exist with other authentication methods"); + } + if (!alter && !hosts) { String common_host_pattern; @@ -630,7 +643,7 @@ bool ParserCreateUserQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expec query->valid_until = std::move(valid_until); query->storage_name = std::move(storage_name); query->reset_authentication_methods_to_new = reset_authentication_methods_to_new.value_or(false); - query->replace_authentication_methods = parsed_identified_with; + query->replace_authentication_methods = parsed_identified_with || has_no_password_authentication_method; for (const auto & authentication_method : query->authentication_methods) { diff --git a/tests/queries/0_stateless/03174_add_identified_with.reference b/tests/queries/0_stateless/03174_add_identified_with.reference index e8e1ee9d294..ae330212941 100644 --- a/tests/queries/0_stateless/03174_add_identified_with.reference +++ b/tests/queries/0_stateless/03174_add_identified_with.reference @@ -37,4 +37,10 @@ CREATE Identified with must precede all add identified with, not allowed BAD_ARGUMENTS Create user with no identification Add identified with -CREATE USER u01_03174 IDENTIFIED WITH no_password ADD IDENTIFIED WITH plaintext_password +CREATE USER u01_03174 IDENTIFIED WITH plaintext_password +Try to provide no_password mixed with other authentication methods, should not be allowed +BAD_ARGUMENTS +Adding no_password, should drop existing auth method +CREATE USER u01_03174 IDENTIFIED WITH no_password +Trying to auth with no pwd, should succeed +1 diff --git a/tests/queries/0_stateless/03174_add_identified_with.sh b/tests/queries/0_stateless/03174_add_identified_with.sh index 0884f31dd73..c3c42047bcf 100755 --- a/tests/queries/0_stateless/03174_add_identified_with.sh +++ b/tests/queries/0_stateless/03174_add_identified_with.sh @@ -13,6 +13,11 @@ ssh_key="-----BEGIN OPENSSH PRIVATE KEY----- Ux7i7d3xPoseFrwnhY4YAAAADWFydGh1ckBhcnRodXI= -----END OPENSSH PRIVATE KEY-----" +function test_login_no_pwd +{ + ${CLICKHOUSE_CLIENT} --user $1 --query "select 1" +} + function test_login_pwd { ${CLICKHOUSE_CLIENT} --user $1 --password $2 --query "select 1" @@ -107,4 +112,14 @@ ${CLICKHOUSE_CLIENT} --query "ALTER USER ${user} ADD IDENTIFIED WITH plaintext_p ${CLICKHOUSE_CLIENT} --query "SHOW CREATE USER ${user}" +echo "Try to provide no_password mixed with other authentication methods, should not be allowed" +${CLICKHOUSE_CLIENT} --query "ALTER USER ${user} ADD IDENTIFIED WITH plaintext_password by '8' ADD IDENTIFIED WITH no_password" 2>&1 | grep -m1 -o "BAD_ARGUMENTS" + +echo "Adding no_password, should drop existing auth method" +${CLICKHOUSE_CLIENT} --query "ALTER USER ${user} ADD IDENTIFIED WITH no_password" +${CLICKHOUSE_CLIENT} --query "SHOW CREATE USER ${user}" + +echo "Trying to auth with no pwd, should succeed" +test_login_no_pwd ${user} + ${CLICKHOUSE_CLIENT} --query "DROP USER IF EXISTS ${user}"