change parsing logic a bit

This commit is contained in:
Arthur Passos 2024-08-27 10:40:04 -03:00
parent cb7ef910e2
commit a65d175a81
4 changed files with 68 additions and 85 deletions

View File

@ -67,17 +67,8 @@ namespace
user.authentication_methods.emplace_back();
}
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. an 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()))
if (replace_authentication_methods)
{
user.authentication_methods.clear();
}
@ -109,6 +100,18 @@ namespace
user.authentication_methods.emplace_back(authentication_method);
}
bool has_no_password_authentication_method = std::find_if(user.authentication_methods.begin(),
user.authentication_methods.end(),
[](const AuthenticationData & authentication_data)
{
return authentication_data.getType() == AuthenticationType::NO_PASSWORD;
}) != user.authentication_methods.end();
if (has_no_password_authentication_method && user.authentication_methods.size() > 1)
{
throw Exception(ErrorCodes::BAD_ARGUMENTS, "Authentication method 'no_password' cannot co-exist with other authentication methods.");
}
if (!query.alter)
{
for (const auto & authentication_method : user.authentication_methods)

View File

@ -53,7 +53,8 @@ namespace
Expected & expected,
std::shared_ptr<ASTAuthenticationData> & auth_data,
bool is_type_specifier_mandatory,
bool is_type_specifier_allowed)
bool is_type_specifier_allowed,
bool should_parse_no_password)
{
return IParserBase::wrapParseImpl(pos, [&]
{
@ -67,7 +68,7 @@ namespace
bool expect_public_ssh_key = false;
bool expect_http_auth_server = false;
for (auto check_type : collections::range(AuthenticationType::MAX))
auto parse_non_password_based_type = [&](auto check_type)
{
if (ParserKeyword{AuthenticationTypeInfo::get(check_type).keyword}.ignore(pos, expected))
{
@ -86,7 +87,20 @@ namespace
else if (check_type != AuthenticationType::NO_PASSWORD)
expect_password = true;
break;
return true;
}
return false;
};
{
const auto first_authentication_type_element_to_check
= should_parse_no_password ? AuthenticationType::NO_PASSWORD : AuthenticationType::PLAINTEXT_PASSWORD;
for (auto check_type : collections::range(first_authentication_type_element_to_check, AuthenticationType::MAX))
{
if (parse_non_password_based_type(check_type))
break;
}
}
@ -219,7 +233,11 @@ namespace
}
bool parseIdentifiedWith(IParserBase::Pos & pos, Expected & expected, std::vector<std::shared_ptr<ASTAuthenticationData>> & authentication_methods)
bool parseIdentifiedWith(
IParserBase::Pos & pos,
Expected & expected,
std::vector<std::shared_ptr<ASTAuthenticationData>> & authentication_methods,
bool should_parse_no_password)
{
return IParserBase::wrapParseImpl(pos, [&]
{
@ -232,7 +250,7 @@ namespace
std::shared_ptr<ASTAuthenticationData> ast_authentication_data;
if (!parseAuthenticationData(pos, expected, ast_authentication_data, is_type_specifier_mandatory, is_type_specifier_mandatory))
if (!parseAuthenticationData(pos, expected, ast_authentication_data, is_type_specifier_mandatory, is_type_specifier_mandatory, should_parse_no_password))
{
return false;
}
@ -248,7 +266,7 @@ namespace
{
std::shared_ptr<ASTAuthenticationData> ast_authentication_data;
if (!parseAuthenticationData(aux_pos, expected, ast_authentication_data, false, true))
if (!parseAuthenticationData(aux_pos, expected, ast_authentication_data, false, true, should_parse_no_password))
{
break;
}
@ -273,7 +291,7 @@ namespace
return true;
}
return parseIdentifiedWith(pos, expected, authentication_methods);
return parseIdentifiedWith(pos, expected, authentication_methods, true);
});
}
@ -480,7 +498,7 @@ namespace
return false;
}
return parseIdentifiedWith(pos, expected, auth_data);
return parseIdentifiedWith(pos, expected, auth_data, false);
});
}
@ -551,29 +569,19 @@ bool ParserCreateUserQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expec
while (true)
{
if (auth_data.empty())
if (auth_data.empty() && !reset_authentication_methods_to_new)
{
parsed_identified_with = parseIdentifiedOrNotIdentified(pos, expected, auth_data);
if (!parsed_identified_with)
if (!parsed_identified_with && alter)
{
parsed_add_identified_with = parseAddIdentifiedWith(pos, expected, auth_data);
if (parsed_add_identified_with && !alter)
{
throw Exception(ErrorCodes::BAD_ARGUMENTS, "Create user query is not allowed to have ADD IDENTIFIED, remove the ADD keyword.");
}
}
}
if (!reset_authentication_methods_to_new)
if (!reset_authentication_methods_to_new && alter && auth_data.empty())
{
reset_authentication_methods_to_new = parseResetAuthenticationMethods(pos, expected);
if (reset_authentication_methods_to_new && !alter)
{
throw Exception(ErrorCodes::BAD_ARGUMENTS, "RESET AUTHENTICATION METHODS TO NEW can only be used on ALTER statement");
}
}
if (!valid_until)
@ -640,31 +648,6 @@ 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<ASTAuthenticationData> & 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, "Authentication method 'no_password' cannot co-exist with other authentication methods.");
}
if (has_no_password_authentication_method && parsed_add_identified_with)
{
throw Exception(ErrorCodes::BAD_ARGUMENTS, "Authentication method 'no_password' cannot co-exist with other authentication methods. "
"Use 'ALTER USER xyz IDENTIFIED WITH no_password' to replace existing authentication methods");
}
if (reset_authentication_methods_to_new && !auth_data.empty())
{
throw Exception(ErrorCodes::BAD_ARGUMENTS, "RESET AUTHENTICATION METHODS TO NEW cannot be used along with [ADD] IDENTIFIED clauses");
}
if (!alter && !hosts)
{
String common_host_pattern;
@ -700,7 +683,7 @@ bool ParserCreateUserQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expec
query->storage_name = std::move(storage_name);
query->reset_authentication_methods_to_new = reset_authentication_methods_to_new;
query->add_identified_with = parsed_add_identified_with;
query->replace_authentication_methods = parsed_identified_with || has_no_password_authentication_method;
query->replace_authentication_methods = parsed_identified_with;
for (const auto & authentication_method : query->authentication_methods)
{

View File

@ -28,13 +28,12 @@ Syntax error
CREATE Multiple identified with, not allowed
Syntax error
Create user with no identification
Add identified with
CREATE USER u01_03174 IDENTIFIED WITH plaintext_password
Try to provide no_password mixed with other authentication methods, should not be allowed
Add identified with, should not be allowed because user is currently identified with no_password and it can not co-exist with other auth types
BAD_ARGUMENTS
Try to add no_password mixed with other authentication methods, should not be allowed
SYNTAX_ERROR
Adding no_password, should fail
BAD_ARGUMENTS
CREATE USER u01_03174 IDENTIFIED WITH plaintext_password
SYNTAX_ERROR
Replacing existing authentication methods in favor of no_password, should succeed
CREATE USER u01_03174 IDENTIFIED WITH no_password
Trying to auth with no pwd, should succeed
@ -46,17 +45,18 @@ CREATE USER u01_03174 IDENTIFIED WITH sha256_password, plaintext_password, bcryp
Use WITH without providing authentication type, should fail
Syntax error
Create user with ADD identification, should fail, add is not allowed for create query
BAD_ARGUMENTS
SYNTAX_ERROR
Trailing comma should result in syntax error
SYNTAX_ERROR
First auth method can't specify type if WITH keyword is not present
SYNTAX_ERROR
RESET AUTHENTICATION METHODS TO NEW can only be used on alter statement
BAD_ARGUMENTS
SYNTAX_ERROR
ADD NOT IDENTIFIED should result in syntax error
SYNTAX_ERROR
RESET AUTHENTICATION METHODS TO NEW cannot be used along with [ADD] IDENTIFIED clauses
BAD_ARGUMENTS
SYNTAX_ERROR
On cluster tests
localhost 9000 0 0 0
localhost 9000 0 0 0
Basic authentication after user creation
@ -98,14 +98,12 @@ Syntax error
localhost 9000 0 0 0
Create user with no identification
localhost 9000 0 0 0
Add identified with
localhost 9000 0 0 0
CREATE USER u01_03174 IDENTIFIED WITH plaintext_password
Try to provide no_password mixed with other authentication methods, should not be allowed
Add identified with, should not be allowed because user is currently identified with no_password and it can not co-exist with other auth types
BAD_ARGUMENTS
Try to add no_password mixed with other authentication methods, should not be allowed
SYNTAX_ERROR
Adding no_password, should fail
BAD_ARGUMENTS
CREATE USER u01_03174 IDENTIFIED WITH plaintext_password
SYNTAX_ERROR
Replacing existing authentication methods in favor of no_password, should succeed
localhost 9000 0 0 0
CREATE USER u01_03174 IDENTIFIED WITH no_password
@ -123,14 +121,14 @@ localhost 9000 0 0 0
Use WITH without providing authentication type, should fail
Syntax error
Create user with ADD identification, should fail, add is not allowed for create query
BAD_ARGUMENTS
SYNTAX_ERROR
Trailing comma should result in syntax error
SYNTAX_ERROR
First auth method can't specify type if WITH keyword is not present
SYNTAX_ERROR
RESET AUTHENTICATION METHODS TO NEW can only be used on alter statement
BAD_ARGUMENTS
SYNTAX_ERROR
ADD NOT IDENTIFIED should result in syntax error
SYNTAX_ERROR
RESET AUTHENTICATION METHODS TO NEW cannot be used along with [ADD] IDENTIFIED clauses
BAD_ARGUMENTS
SYNTAX_ERROR

View File

@ -102,17 +102,14 @@ function test
echo "Create user with no identification"
${CLICKHOUSE_CLIENT} --query "CREATE USER ${user} $1"
echo "Add identified with"
${CLICKHOUSE_CLIENT} --query "ALTER USER ${user} $1 ADD IDENTIFIED WITH plaintext_password by '7'"
echo "Add identified with, should not be allowed because user is currently identified with no_password and it can not co-exist with other auth types"
${CLICKHOUSE_CLIENT} --query "ALTER USER ${user} $1 ADD IDENTIFIED WITH plaintext_password by '7'" 2>&1 | grep -m1 -o "BAD_ARGUMENTS"
${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} $1 ADD IDENTIFIED WITH plaintext_password by '8', no_password" 2>&1 | grep -m1 -o "BAD_ARGUMENTS"
echo "Try to add no_password mixed with other authentication methods, should not be allowed"
${CLICKHOUSE_CLIENT} --query "ALTER USER ${user} $1 ADD IDENTIFIED WITH plaintext_password by '8', no_password" 2>&1 | grep -m1 -o "SYNTAX_ERROR"
echo "Adding no_password, should fail"
${CLICKHOUSE_CLIENT} --query "ALTER USER ${user} $1 ADD IDENTIFIED WITH no_password" 2>&1 | grep -m1 -o "BAD_ARGUMENTS"
${CLICKHOUSE_CLIENT} --query "SHOW CREATE USER ${user}"
${CLICKHOUSE_CLIENT} --query "ALTER USER ${user} $1 ADD IDENTIFIED WITH no_password" 2>&1 | grep -m1 -o "SYNTAX_ERROR"
echo "Replacing existing authentication methods in favor of no_password, should succeed"
${CLICKHOUSE_CLIENT} --query "ALTER USER ${user} $1 IDENTIFIED WITH no_password"
@ -139,7 +136,7 @@ function test
${CLICKHOUSE_CLIENT} --query "CREATE USER ${user} $1 IDENTIFIED WITH BY '1';" 2>&1 | grep -m1 -o "Syntax error"
echo "Create user with ADD identification, should fail, add is not allowed for create query"
${CLICKHOUSE_CLIENT} --query "CREATE USER ${user} $1 ADD IDENTIFIED WITH plaintext_password by '1'" 2>&1 | grep -m1 -o "BAD_ARGUMENTS"
${CLICKHOUSE_CLIENT} --query "CREATE USER ${user} $1 ADD IDENTIFIED WITH plaintext_password by '1'" 2>&1 | grep -m1 -o "SYNTAX_ERROR"
echo "Trailing comma should result in syntax error"
${CLICKHOUSE_CLIENT} --query "ALTER USER ${user} $1 ADD IDENTIFIED WITH plaintext_password by '1'," 2>&1 | grep -m1 -o "SYNTAX_ERROR"
@ -148,13 +145,13 @@ function test
${CLICKHOUSE_CLIENT} --query "CREATE USER ${user} $1 IDENTIFIED plaintext_password by '1'" 2>&1 | grep -m1 -o "SYNTAX_ERROR"
echo "RESET AUTHENTICATION METHODS TO NEW can only be used on alter statement"
${CLICKHOUSE_CLIENT} --query "CREATE USER ${user} $1 RESET AUTHENTICATION METHODS TO NEW" 2>&1 | grep -m1 -o "BAD_ARGUMENTS"
${CLICKHOUSE_CLIENT} --query "CREATE USER ${user} $1 RESET AUTHENTICATION METHODS TO NEW" 2>&1 | grep -m1 -o "SYNTAX_ERROR"
echo "ADD NOT IDENTIFIED should result in syntax error"
${CLICKHOUSE_CLIENT} --query "ALTER USER ${user} $1 ADD NOT IDENTIFIED" 2>&1 | grep -m1 -o "SYNTAX_ERROR"
echo "RESET AUTHENTICATION METHODS TO NEW cannot be used along with [ADD] IDENTIFIED clauses"
${CLICKHOUSE_CLIENT} --query "ALTER USER ${user} $1 IDENTIFIED WITH plaintext_password by '1' RESET AUTHENTICATION METHODS TO NEW" 2>&1 | grep -m1 -o "BAD_ARGUMENTS"
${CLICKHOUSE_CLIENT} --query "ALTER USER ${user} $1 IDENTIFIED WITH plaintext_password by '1' RESET AUTHENTICATION METHODS TO NEW" 2>&1 | grep -m1 -o "SYNTAX_ERROR"
${CLICKHOUSE_CLIENT} --query "DROP USER IF EXISTS ${user}"
@ -162,4 +159,6 @@ function test
test ""
echo "On cluster tests"
test "ON CLUSTER test_shard_localhost"