Merge pull request #70680 from ClickHouse/backport/24.8/70644

Backport #70644 to 24.8: Don't do validation when synchronizing user_directories from keeper
This commit is contained in:
robot-ch-test-poll4 2024-10-15 19:47:13 +02:00 committed by GitHub
commit d2acaba718
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 49 additions and 39 deletions

View File

@ -117,20 +117,20 @@ bool operator ==(const AuthenticationData & lhs, const AuthenticationData & rhs)
}
void AuthenticationData::setPassword(const String & password_)
void AuthenticationData::setPassword(const String & password_, bool validate)
{
switch (type)
{
case AuthenticationType::PLAINTEXT_PASSWORD:
setPasswordHashBinary(Util::stringToDigest(password_));
setPasswordHashBinary(Util::stringToDigest(password_), validate);
return;
case AuthenticationType::SHA256_PASSWORD:
setPasswordHashBinary(Util::encodeSHA256(password_));
setPasswordHashBinary(Util::encodeSHA256(password_), validate);
return;
case AuthenticationType::DOUBLE_SHA1_PASSWORD:
setPasswordHashBinary(Util::encodeDoubleSHA1(password_));
setPasswordHashBinary(Util::encodeDoubleSHA1(password_), validate);
return;
case AuthenticationType::BCRYPT_PASSWORD:
@ -149,12 +149,12 @@ void AuthenticationData::setPassword(const String & password_)
throw Exception(ErrorCodes::NOT_IMPLEMENTED, "setPassword(): authentication type {} not supported", toString(type));
}
void AuthenticationData::setPasswordBcrypt(const String & password_, int workfactor_)
void AuthenticationData::setPasswordBcrypt(const String & password_, int workfactor_, bool validate)
{
if (type != AuthenticationType::BCRYPT_PASSWORD)
throw Exception(ErrorCodes::LOGICAL_ERROR, "Cannot specify bcrypt password for authentication type {}", toString(type));
setPasswordHashBinary(Util::encodeBcrypt(password_, workfactor_));
setPasswordHashBinary(Util::encodeBcrypt(password_, workfactor_), validate);
}
String AuthenticationData::getPassword() const
@ -165,7 +165,7 @@ String AuthenticationData::getPassword() const
}
void AuthenticationData::setPasswordHashHex(const String & hash)
void AuthenticationData::setPasswordHashHex(const String & hash, bool validate)
{
Digest digest;
digest.resize(hash.size() / 2);
@ -179,7 +179,7 @@ void AuthenticationData::setPasswordHashHex(const String & hash)
throw Exception(ErrorCodes::BAD_ARGUMENTS, "Cannot read password hash in hex, check for valid characters [0-9a-fA-F] and length");
}
setPasswordHashBinary(digest);
setPasswordHashBinary(digest, validate);
}
@ -195,7 +195,7 @@ String AuthenticationData::getPasswordHashHex() const
}
void AuthenticationData::setPasswordHashBinary(const Digest & hash)
void AuthenticationData::setPasswordHashBinary(const Digest & hash, bool validate)
{
switch (type)
{
@ -217,7 +217,7 @@ void AuthenticationData::setPasswordHashBinary(const Digest & hash)
case AuthenticationType::DOUBLE_SHA1_PASSWORD:
{
if (hash.size() != 20)
if (validate && hash.size() != 20)
throw Exception(ErrorCodes::BAD_ARGUMENTS,
"Password hash for the 'DOUBLE_SHA1_PASSWORD' authentication type has length {} "
"but must be exactly 20 bytes.", hash.size());
@ -231,7 +231,7 @@ void AuthenticationData::setPasswordHashBinary(const Digest & hash)
/// However the library we use to encode it requires hash string to be 64 characters long,
/// so we also allow the hash of this length.
if (hash.size() != 59 && hash.size() != 60 && hash.size() != 64)
if (validate && hash.size() != 59 && hash.size() != 60 && hash.size() != 64)
throw Exception(ErrorCodes::BAD_ARGUMENTS,
"Password hash for the 'BCRYPT_PASSWORD' authentication type has length {} "
"but must be 59 or 60 bytes.", hash.size());
@ -240,10 +240,13 @@ void AuthenticationData::setPasswordHashBinary(const Digest & hash)
resized.resize(64);
#if USE_BCRYPT
/// Verify that it is a valid hash
int ret = bcrypt_checkpw("", reinterpret_cast<const char *>(resized.data()));
if (ret == -1)
throw Exception(ErrorCodes::BAD_ARGUMENTS, "Could not decode the provided hash with 'bcrypt_hash'");
if (validate)
{
/// Verify that it is a valid hash
int ret = bcrypt_checkpw("", reinterpret_cast<const char *>(resized.data()));
if (ret == -1)
throw Exception(ErrorCodes::BAD_ARGUMENTS, "Could not decode the provided hash with 'bcrypt_hash'");
}
#endif
password_hash = hash;
@ -384,7 +387,7 @@ std::shared_ptr<ASTAuthenticationData> AuthenticationData::toAST() const
}
AuthenticationData AuthenticationData::fromAST(const ASTAuthenticationData & query, ContextPtr context, bool check_password_rules)
AuthenticationData AuthenticationData::fromAST(const ASTAuthenticationData & query, ContextPtr context, bool validate)
{
if (query.type && query.type == AuthenticationType::NO_PASSWORD)
return AuthenticationData();
@ -430,7 +433,7 @@ AuthenticationData AuthenticationData::fromAST(const ASTAuthenticationData & que
if (!query.type && !context)
throw Exception(ErrorCodes::LOGICAL_ERROR, "Cannot get default password type without context");
if (check_password_rules && !context)
if (validate && !context)
throw Exception(ErrorCodes::LOGICAL_ERROR, "Cannot check password complexity rules without context");
if (query.type == AuthenticationType::BCRYPT_PASSWORD && !context)
@ -447,13 +450,13 @@ AuthenticationData AuthenticationData::fromAST(const ASTAuthenticationData & que
AuthenticationData auth_data(current_type);
if (check_password_rules)
if (validate)
context->getAccessControl().checkPasswordComplexityRules(value);
if (query.type == AuthenticationType::BCRYPT_PASSWORD)
{
int workfactor = context->getAccessControl().getBcryptWorkfactor();
auth_data.setPasswordBcrypt(value, workfactor);
auth_data.setPasswordBcrypt(value, workfactor, validate);
return auth_data;
}
@ -485,7 +488,7 @@ AuthenticationData AuthenticationData::fromAST(const ASTAuthenticationData & que
#endif
}
auth_data.setPassword(value);
auth_data.setPassword(value, validate);
return auth_data;
}
@ -497,12 +500,12 @@ AuthenticationData AuthenticationData::fromAST(const ASTAuthenticationData & que
if (query.type == AuthenticationType::BCRYPT_PASSWORD)
{
auth_data.setPasswordHashBinary(AuthenticationData::Util::stringToDigest(value));
auth_data.setPasswordHashBinary(AuthenticationData::Util::stringToDigest(value), validate);
return auth_data;
}
else
{
auth_data.setPasswordHashHex(value);
auth_data.setPasswordHashHex(value, validate);
}
if (query.type == AuthenticationType::SHA256_PASSWORD && args_size == 2)

View File

@ -31,17 +31,17 @@ public:
AuthenticationType getType() const { return type; }
/// Sets the password and encrypt it using the authentication type set in the constructor.
void setPassword(const String & password_);
void setPassword(const String & password_, bool validate);
/// Returns the password. Allowed to use only for Type::PLAINTEXT_PASSWORD.
String getPassword() const;
/// Sets the password as a string of hexadecimal digits.
void setPasswordHashHex(const String & hash);
void setPasswordHashHex(const String & hash, bool validate);
String getPasswordHashHex() const;
/// Sets the password in binary form.
void setPasswordHashBinary(const Digest & hash);
void setPasswordHashBinary(const Digest & hash, bool validate);
const Digest & getPasswordHashBinary() const { return password_hash; }
/// Sets the salt in String form.
@ -49,7 +49,7 @@ public:
String getSalt() const;
/// Sets the password using bcrypt hash with specified workfactor
void setPasswordBcrypt(const String & password_, int workfactor_);
void setPasswordBcrypt(const String & password_, int workfactor_, bool validate);
/// Sets the server name for authentication type LDAP.
const String & getLDAPServerName() const { return ldap_server_name; }
@ -77,7 +77,7 @@ public:
friend bool operator ==(const AuthenticationData & lhs, const AuthenticationData & rhs);
friend bool operator !=(const AuthenticationData & lhs, const AuthenticationData & rhs) { return !(lhs == rhs); }
static AuthenticationData fromAST(const ASTAuthenticationData & query, ContextPtr context, bool check_password_rules);
static AuthenticationData fromAST(const ASTAuthenticationData & query, ContextPtr context, bool validate);
std::shared_ptr<ASTAuthenticationData> toAST() const;
struct Util

View File

@ -120,6 +120,7 @@ namespace
bool allow_no_password,
bool allow_plaintext_password)
{
const bool validate = true;
auto user = std::make_shared<User>();
user->setName(user_name);
String user_config = "users." + user_name;
@ -156,17 +157,17 @@ namespace
if (has_password_plaintext)
{
user->auth_data = AuthenticationData{AuthenticationType::PLAINTEXT_PASSWORD};
user->auth_data.setPassword(config.getString(user_config + ".password"));
user->auth_data.setPassword(config.getString(user_config + ".password"), validate);
}
else if (has_password_sha256_hex)
{
user->auth_data = AuthenticationData{AuthenticationType::SHA256_PASSWORD};
user->auth_data.setPasswordHashHex(config.getString(user_config + ".password_sha256_hex"));
user->auth_data.setPasswordHashHex(config.getString(user_config + ".password_sha256_hex"), validate);
}
else if (has_password_double_sha1_hex)
{
user->auth_data = AuthenticationData{AuthenticationType::DOUBLE_SHA1_PASSWORD};
user->auth_data.setPasswordHashHex(config.getString(user_config + ".password_double_sha1_hex"));
user->auth_data.setPasswordHashHex(config.getString(user_config + ".password_double_sha1_hex"), validate);
}
else if (has_ldap)
{

View File

@ -1,5 +1,6 @@
import psycopg2
import time
import psycopg2
from psycopg2.extensions import ISOLATION_LEVEL_AUTOCOMMIT
postgres_table_template = """
@ -245,9 +246,9 @@ class PostgresManager:
):
postgres_database = self.database_or_default(postgres_database)
self.created_materialized_postgres_db_list.add(materialized_database)
self.instance.query(f"DROP DATABASE IF EXISTS {materialized_database}")
self.instance.query(f"DROP DATABASE IF EXISTS `{materialized_database}`")
create_query = f"CREATE DATABASE {materialized_database} ENGINE = MaterializedPostgreSQL('{ip}:{port}', '{postgres_database}', '{user}', '{password}')"
create_query = f"CREATE DATABASE `{materialized_database}` ENGINE = MaterializedPostgreSQL('{ip}:{port}', '{postgres_database}', '{user}', '{password}')"
if len(settings) > 0:
create_query += " SETTINGS "
for i in range(len(settings)):
@ -259,7 +260,7 @@ class PostgresManager:
assert materialized_database in self.instance.query("SHOW DATABASES")
def drop_materialized_db(self, materialized_database="test_database"):
self.instance.query(f"DROP DATABASE IF EXISTS {materialized_database} SYNC")
self.instance.query(f"DROP DATABASE IF EXISTS `{materialized_database}` SYNC")
if materialized_database in self.created_materialized_postgres_db_list:
self.created_materialized_postgres_db_list.remove(materialized_database)
@ -342,11 +343,15 @@ def assert_nested_table_is_created(
table = schema_name + "." + table_name
print(f"Checking table {table} exists in {materialized_database}")
database_tables = instance.query(f"SHOW TABLES FROM {materialized_database}")
database_tables = instance.query(
f"SHOW TABLES FROM `{materialized_database}` WHERE name = '{table}'"
)
while table not in database_tables:
time.sleep(0.2)
database_tables = instance.query(f"SHOW TABLES FROM {materialized_database}")
database_tables = instance.query(
f"SHOW TABLES FROM `{materialized_database}` WHERE name = '{table}'"
)
assert table in database_tables
@ -372,6 +377,7 @@ def check_tables_are_synchronized(
postgres_database="postgres_database",
materialized_database="test_database",
schema_name="",
columns=["*"],
):
assert_nested_table_is_created(
instance, table_name, materialized_database, schema_name
@ -379,15 +385,15 @@ def check_tables_are_synchronized(
table_path = ""
if len(schema_name) == 0:
table_path = f"{materialized_database}.{table_name}"
table_path = f"`{materialized_database}`.`{table_name}`"
else:
table_path = f"{materialized_database}.`{schema_name}.{table_name}`"
table_path = f"`{materialized_database}`.`{schema_name}.{table_name}`"
print(f"Checking table is synchronized: {table_path}")
result_query = f"select * from {table_path} order by {order_by};"
expected = instance.query(
f"select * from `{postgres_database}`.`{table_name}` order by {order_by};"
f"select {','.join(columns)} from `{postgres_database}`.`{table_name}` order by {order_by};"
)
result = instance.query(result_query)