use comma separated instead of multiple add id add id

This commit is contained in:
Arthur Passos 2024-07-11 17:04:28 -03:00
parent 91e8ef6776
commit 29e0d5c1e3
5 changed files with 108 additions and 116 deletions

View File

@ -44,7 +44,7 @@ void ASTAuthenticationData::formatImpl(const FormatSettings & settings, FormatSt
{
if (type && *type == AuthenticationType::NO_PASSWORD)
{
settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << " IDENTIFIED WITH no_password"
settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << " no_password"
<< (settings.hilite ? IAST::hilite_none : "");
return;
}
@ -160,12 +160,9 @@ void ASTAuthenticationData::formatImpl(const FormatSettings & settings, FormatSt
auth_type_name = AuthenticationTypeInfo::get(*type).name;
}
settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << " IDENTIFIED" << (settings.hilite ? IAST::hilite_none : "");
if (!auth_type_name.empty())
{
settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << " WITH " << auth_type_name
<< (settings.hilite ? IAST::hilite_none : "");
settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << " " << auth_type_name << (settings.hilite ? IAST::hilite_none : "");
}
if (!prefix.empty())

View File

@ -21,16 +21,24 @@ namespace
void formatAuthenticationData(const std::vector<std::shared_ptr<ASTAuthenticationData>> & authentication_methods, const IAST::FormatSettings & settings)
{
/*
* The first authentication method must be formatted in the form of: `IDENTIFIED WITH xyz..`
* The remaining ones shall contain the `ADD`: `ADD IDENTIFIED WITH`.
* */
authentication_methods[0]->format(settings);
settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << " IDENTIFIED" << (settings.hilite ? IAST::hilite_none : "");
for (std::size_t i = 1; i < authentication_methods.size(); i++)
// safe because this method is only called if authentication_methods.size > 1
// if the first type is present, include the `WITH` keyword
if (authentication_methods[0]->type)
{
settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << " WITH" << (settings.hilite ? IAST::hilite_none : "");
}
for (std::size_t i = 0; i < authentication_methods.size(); i++)
{
settings.ostr << (settings.hilite ? IAST::hilite_keyword : "") << " ADD" << (settings.hilite ? IAST::hilite_none : "");
authentication_methods[i]->format(settings);
bool is_last = i < authentication_methods.size() - 1;
if (is_last)
{
settings.ostr << (settings.hilite ? IAST::hilite_keyword : ",");
}
}
}

View File

@ -48,21 +48,10 @@ namespace
});
}
bool parseAuthenticationData(IParserBase::Pos & pos, Expected & expected, std::shared_ptr<ASTAuthenticationData> & auth_data)
bool parseAuthenticationData(IParserBase::Pos & pos, Expected & expected, std::shared_ptr<ASTAuthenticationData> & auth_data, bool is_type_specifier_mandatory)
{
return IParserBase::wrapParseImpl(pos, [&]
{
if (ParserKeyword{Keyword::NOT_IDENTIFIED}.ignore(pos, expected))
{
auth_data = std::make_shared<ASTAuthenticationData>();
auth_data->type = AuthenticationType::NO_PASSWORD;
return true;
}
if (!ParserKeyword{Keyword::IDENTIFIED}.ignore(pos, expected))
return false;
std::optional<AuthenticationType> type;
bool expect_password = false;
@ -73,51 +62,48 @@ namespace
bool expect_public_ssh_key = false;
bool expect_http_auth_server = false;
if (ParserKeyword{Keyword::WITH}.ignore(pos, expected))
for (auto check_type : collections::range(AuthenticationType::MAX))
{
for (auto check_type : collections::range(AuthenticationType::MAX))
if (ParserKeyword{AuthenticationTypeInfo::get(check_type).keyword}.ignore(pos, expected))
{
if (ParserKeyword{AuthenticationTypeInfo::get(check_type).keyword}.ignore(pos, expected))
{
type = check_type;
type = check_type;
if (check_type == AuthenticationType::LDAP)
expect_ldap_server_name = true;
else if (check_type == AuthenticationType::KERBEROS)
expect_kerberos_realm = true;
else if (check_type == AuthenticationType::SSL_CERTIFICATE)
expect_ssl_cert_subjects = true;
else if (check_type == AuthenticationType::SSH_KEY)
expect_public_ssh_key = true;
else if (check_type == AuthenticationType::HTTP)
expect_http_auth_server = true;
else if (check_type != AuthenticationType::NO_PASSWORD)
expect_password = true;
if (check_type == AuthenticationType::LDAP)
expect_ldap_server_name = true;
else if (check_type == AuthenticationType::KERBEROS)
expect_kerberos_realm = true;
else if (check_type == AuthenticationType::SSL_CERTIFICATE)
expect_ssl_cert_subjects = true;
else if (check_type == AuthenticationType::SSH_KEY)
expect_public_ssh_key = true;
else if (check_type == AuthenticationType::HTTP)
expect_http_auth_server = true;
else if (check_type != AuthenticationType::NO_PASSWORD)
expect_password = true;
break;
}
break;
}
}
if (!type)
if (!type)
{
if (ParserKeyword{Keyword::SHA256_HASH}.ignore(pos, expected))
{
if (ParserKeyword{Keyword::SHA256_HASH}.ignore(pos, expected))
{
type = AuthenticationType::SHA256_PASSWORD;
expect_hash = true;
}
else if (ParserKeyword{Keyword::DOUBLE_SHA1_HASH}.ignore(pos, expected))
{
type = AuthenticationType::DOUBLE_SHA1_PASSWORD;
expect_hash = true;
}
else if (ParserKeyword{Keyword::BCRYPT_HASH}.ignore(pos, expected))
{
type = AuthenticationType::BCRYPT_PASSWORD;
expect_hash = true;
}
else
return false;
type = AuthenticationType::SHA256_PASSWORD;
expect_hash = true;
}
else if (ParserKeyword{Keyword::DOUBLE_SHA1_HASH}.ignore(pos, expected))
{
type = AuthenticationType::DOUBLE_SHA1_PASSWORD;
expect_hash = true;
}
else if (ParserKeyword{Keyword::BCRYPT_HASH}.ignore(pos, expected))
{
type = AuthenticationType::BCRYPT_PASSWORD;
expect_hash = true;
}
else if (is_type_specifier_mandatory)
return false;
}
/// If authentication type is not specified, then the default password type is used
@ -224,6 +210,43 @@ namespace
}
bool parseIdentifiedWith(IParserBase::Pos & pos, Expected & expected, std::vector<std::shared_ptr<ASTAuthenticationData>> & authentication_methods)
{
return IParserBase::wrapParseImpl(pos, [&]
{
if (ParserKeyword{Keyword::NOT_IDENTIFIED}.ignore(pos, expected))
{
authentication_methods.emplace_back(std::make_shared<ASTAuthenticationData>());
authentication_methods.back()->type = AuthenticationType::NO_PASSWORD;
return true;
}
if (!ParserKeyword{Keyword::IDENTIFIED}.ignore(pos, expected))
return false;
bool is_type_specifier_mandatory = ParserKeyword{Keyword::WITH}.ignore(pos, expected);
std::shared_ptr<ASTAuthenticationData> ast_authentication_data;
while (parseAuthenticationData(pos, expected, ast_authentication_data, is_type_specifier_mandatory))
{
authentication_methods.push_back(ast_authentication_data);
if (!ParserToken{TokenType::Comma}.ignore(pos, expected))
{
break;
}
ParserToken{TokenType::Whitespace}.ignore(pos, expected);
is_type_specifier_mandatory = false;
}
return !authentication_methods.empty();
});
}
bool parseHostsWithoutPrefix(IParserBase::Pos & pos, Expected & expected, AllowedClientHosts & hosts)
{
AllowedClientHosts res_hosts;
@ -417,7 +440,7 @@ namespace
});
}
bool parseAddNewAuthenticationMethod(IParserBase::Pos & pos, Expected & expected, std::shared_ptr<ASTAuthenticationData> & auth_data)
bool parseAddIdentifiedWith(IParserBase::Pos & pos, Expected & expected, std::vector<std::shared_ptr<ASTAuthenticationData>> & auth_data)
{
return IParserBase::wrapParseImpl(pos, [&]
{
@ -426,7 +449,7 @@ namespace
return false;
}
return parseAuthenticationData(pos, expected, auth_data);
return parseIdentifiedWith(pos, expected, auth_data);
});
}
@ -497,41 +520,14 @@ bool ParserCreateUserQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expec
while (true)
{
std::shared_ptr<ASTAuthenticationData> identified_with_auth_data;
bool parse_auth_data = parseAuthenticationData(pos, expected, identified_with_auth_data);
bool found_multiple_identified_with = parsed_identified_with && parse_auth_data;
if (found_multiple_identified_with)
if (auth_data.empty())
{
throw Exception(ErrorCodes::BAD_ARGUMENTS, "Only one identified with is permitted");
}
parsed_identified_with = parseIdentifiedWith(pos, expected, auth_data);
if (parse_auth_data)
{
if (parsed_add_new_method)
if (!parsed_identified_with)
{
throw Exception(ErrorCodes::BAD_ARGUMENTS, "Identified with must precede add new auth");
parsed_add_new_method = parseAddIdentifiedWith(pos, expected, auth_data);
}
auth_data.push_back(identified_with_auth_data);
parsed_identified_with = true;
continue;
}
std::shared_ptr<ASTAuthenticationData> add_new_auth_method;
parsed_add_new_method = parseAddNewAuthenticationMethod(pos, expected, add_new_auth_method);
if (parsed_add_new_method)
{
if (add_new_auth_method->type == AuthenticationType::NO_PASSWORD)
{
throw Exception(ErrorCodes::BAD_ARGUMENTS, "The authentication method 'no_password' cannot be used with the ADD keyword. "
"Use 'ALTER USER xyz IDENTIFIED WITH no_password' to replace existing authentication methods");
}
auth_data.push_back(add_new_auth_method);
continue;
}
if (!reset_authentication_methods_to_new.has_value())
@ -616,6 +612,13 @@ bool ParserCreateUserQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expec
throw Exception(ErrorCodes::BAD_ARGUMENTS, "NO_PASSWORD Authentication method cannot co-exist with other authentication methods");
}
if (has_no_password_authentication_method && parsed_add_new_method)
{
throw Exception(ErrorCodes::BAD_ARGUMENTS, "The authentication method 'no_password' cannot be used with the ADD keyword. "
"Use 'ALTER USER xyz IDENTIFIED WITH no_password' to replace existing authentication methods");
}
if (!alter && !hosts)
{
String common_host_pattern;

View File

@ -24,17 +24,9 @@ AUTHENTICATION_FAILED
Should work
1
Multiple identified with, not allowed
BAD_ARGUMENTS
Multiple identified with, not allowed, even if mixed
BAD_ARGUMENTS
Identified with must precede all add identified with, not allowed
BAD_ARGUMENTS
Syntax error
CREATE Multiple identified with, not allowed
BAD_ARGUMENTS
CREATE Multiple identified with, not allowed, even if mixed
BAD_ARGUMENTS
CREATE Identified with must precede all add identified with, not allowed
BAD_ARGUMENTS
Syntax error
Create user with no identification
Add identified with
CREATE USER u01_03174 IDENTIFIED WITH plaintext_password

View File

@ -45,7 +45,7 @@ test_login_pwd_expect_error ${user} '1'
echo "New password should work"
test_login_pwd ${user} '2'
${CLICKHOUSE_CLIENT} --query "ALTER USER ${user} ADD IDENTIFIED WITH plaintext_password BY '3' ADD IDENTIFIED WITH plaintext_password BY '4'"
${CLICKHOUSE_CLIENT} --query "ALTER USER ${user} ADD IDENTIFIED WITH plaintext_password BY '3', plaintext_password BY '4'"
echo "Two new passwords were added, should both work"
test_login_pwd ${user} '3'
@ -87,20 +87,12 @@ echo "Should work"
test_login_pwd ${user} '6'
echo "Multiple identified with, not allowed"
${CLICKHOUSE_CLIENT} --query "ALTER USER ${user} IDENTIFIED WITH plaintext_password by '7' IDENTIFIED WITH plaintext_password by '8'" 2>&1 | grep -m1 -o "BAD_ARGUMENTS"
echo "Multiple identified with, not allowed, even if mixed"
${CLICKHOUSE_CLIENT} --query "ALTER USER ${user} IDENTIFIED WITH plaintext_password by '7' ADD IDENTIFIED WITH plaintext_password by '8' IDENTIFIED WITH plaintext_password by '9'" 2>&1 | grep -m1 -o "BAD_ARGUMENTS"
echo "Identified with must precede all add identified with, not allowed"
${CLICKHOUSE_CLIENT} --query "ALTER USER ${user} ADD IDENTIFIED WITH plaintext_password by '7' IDENTIFIED WITH plaintext_password by '8'" 2>&1 | grep -m1 -o "BAD_ARGUMENTS"
${CLICKHOUSE_CLIENT} --query "ALTER USER ${user} IDENTIFIED WITH plaintext_password by '7', IDENTIFIED plaintext_password by '8'" 2>&1 | grep -m1 -o "Syntax error"
${CLICKHOUSE_CLIENT} --query "DROP USER ${user}"
echo "CREATE Multiple identified with, not allowed"
${CLICKHOUSE_CLIENT} --query "CREATE USER ${user} IDENTIFIED WITH plaintext_password by '7' IDENTIFIED WITH plaintext_password by '8'" 2>&1 | grep -m1 -o "BAD_ARGUMENTS"
echo "CREATE Multiple identified with, not allowed, even if mixed"
${CLICKHOUSE_CLIENT} --query "CREATE USER ${user} IDENTIFIED WITH plaintext_password by '7' ADD IDENTIFIED WITH plaintext_password by '8' IDENTIFIED WITH plaintext_password by '9'" 2>&1 | grep -m1 -o "BAD_ARGUMENTS"
echo "CREATE Identified with must precede all add identified with, not allowed"
${CLICKHOUSE_CLIENT} --query "CREATE USER ${user} ADD IDENTIFIED WITH plaintext_password by '7' IDENTIFIED WITH plaintext_password by '8'" 2>&1 | grep -m1 -o "BAD_ARGUMENTS"
${CLICKHOUSE_CLIENT} --query "CREATE USER ${user} IDENTIFIED WITH plaintext_password by '7', IDENTIFIED WITH plaintext_password by '8'" 2>&1 | grep -m1 -o "Syntax error"
${CLICKHOUSE_CLIENT} --query "DROP USER IF EXISTS ${user}"
@ -113,7 +105,7 @@ ${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"
${CLICKHOUSE_CLIENT} --query "ALTER USER ${user} ADD IDENTIFIED WITH plaintext_password by '8', no_password" 2>&1 | grep -m1 -o "BAD_ARGUMENTS"
echo "Adding no_password, should fail"
${CLICKHOUSE_CLIENT} --query "ALTER USER ${user} ADD IDENTIFIED WITH no_password" 2>&1 | grep -m1 -o "BAD_ARGUMENTS"