From 3e68368b59f9380b160242e819d5e4a5e14ad8e6 Mon Sep 17 00:00:00 2001 From: Denis Glazachev Date: Sat, 11 Jul 2020 21:06:01 +0400 Subject: [PATCH] Refactor ExternalAuthenticators configuration process --- src/Access/AccessControlManager.cpp | 9 +++++---- src/Access/AccessControlManager.h | 2 +- src/Access/Authentication.cpp | 7 ++----- src/Access/Authentication.h | 2 +- src/Access/ExternalAuthenticators.cpp | 9 ++++++++- src/Access/ExternalAuthenticators.h | 5 +++-- 6 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/Access/AccessControlManager.cpp b/src/Access/AccessControlManager.cpp index 2f900374a46..5966c1aff75 100644 --- a/src/Access/AccessControlManager.cpp +++ b/src/Access/AccessControlManager.cpp @@ -65,7 +65,8 @@ AccessControlManager::AccessControlManager() role_cache(std::make_unique(*this)), row_policy_cache(std::make_unique(*this)), quota_cache(std::make_unique(*this)), - settings_profiles_cache(std::make_unique(*this)) + settings_profiles_cache(std::make_unique(*this)), + external_authenticators(std::make_unique()) { } @@ -82,7 +83,7 @@ void AccessControlManager::setLocalDirectory(const String & directory_path) void AccessControlManager::setExternalAuthenticatorsConfig(const Poco::Util::AbstractConfiguration & config) { - external_authenticators = std::make_unique(config, getLogger()); + external_authenticators->setConfig(config, getLogger()); } @@ -170,9 +171,9 @@ std::shared_ptr AccessControlManager::getProfileSettings( return settings_profiles_cache->getProfileSettings(profile_name); } -const ExternalAuthenticators * AccessControlManager::getExternalAuthenticators() const +const ExternalAuthenticators & AccessControlManager::getExternalAuthenticators() const { - return external_authenticators.get(); + return *external_authenticators; } } diff --git a/src/Access/AccessControlManager.h b/src/Access/AccessControlManager.h index 7089ee50a85..467b7471423 100644 --- a/src/Access/AccessControlManager.h +++ b/src/Access/AccessControlManager.h @@ -87,7 +87,7 @@ public: std::shared_ptr getProfileSettings(const String & profile_name) const; - const ExternalAuthenticators * getExternalAuthenticators() const; + const ExternalAuthenticators & getExternalAuthenticators() const; private: class ContextAccessCache; diff --git a/src/Access/Authentication.cpp b/src/Access/Authentication.cpp index 4338e53c08a..d29e2f897e8 100644 --- a/src/Access/Authentication.cpp +++ b/src/Access/Authentication.cpp @@ -49,7 +49,7 @@ Authentication::Digest Authentication::getPasswordDoubleSHA1() const } -bool Authentication::isCorrectPassword(const String & password_, const String & user_, const ExternalAuthenticators * external_authenticators) const +bool Authentication::isCorrectPassword(const String & password_, const String & user_, const ExternalAuthenticators & external_authenticators) const { switch (type) { @@ -82,10 +82,7 @@ bool Authentication::isCorrectPassword(const String & password_, const String & case LDAP_SERVER: { - if (!external_authenticators) - throw Exception("External authenticators are not configured", ErrorCodes::BAD_ARGUMENTS); - - auto ldap_server_params = external_authenticators->getLDAPServerParams(server_name); + auto ldap_server_params = external_authenticators.getLDAPServerParams(server_name); ldap_server_params.user = user_; ldap_server_params.password = password_; diff --git a/src/Access/Authentication.h b/src/Access/Authentication.h index 6faf5a369eb..35ff0fa1d32 100644 --- a/src/Access/Authentication.h +++ b/src/Access/Authentication.h @@ -89,7 +89,7 @@ public: /// Checks if the provided password is correct. Returns false if not. /// User name and external authenticators' info are used only by some specific authentication type (e.g., LDAP_SERVER). - bool isCorrectPassword(const String & password_, const String & user_, const ExternalAuthenticators * external_authenticators) const; + bool isCorrectPassword(const String & password_, const String & user_, const ExternalAuthenticators & external_authenticators) const; friend bool operator ==(const Authentication & lhs, const Authentication & rhs) { return (lhs.type == rhs.type) && (lhs.password_hash == rhs.password_hash); } friend bool operator !=(const Authentication & lhs, const Authentication & rhs) { return !(lhs == rhs); } diff --git a/src/Access/ExternalAuthenticators.cpp b/src/Access/ExternalAuthenticators.cpp index fcb7317a52d..4f4104214c3 100644 --- a/src/Access/ExternalAuthenticators.cpp +++ b/src/Access/ExternalAuthenticators.cpp @@ -150,8 +150,15 @@ void parseAndAddLDAPServers(ExternalAuthenticators & external_authenticators, co } -ExternalAuthenticators::ExternalAuthenticators(const Poco::Util::AbstractConfiguration & config, Poco::Logger * log) +void ExternalAuthenticators::reset() { + std::scoped_lock lock(mutex); + ldap_server_params.clear(); +} + +void ExternalAuthenticators::setConfig(const Poco::Util::AbstractConfiguration & config, Poco::Logger * log) { + std::scoped_lock lock(mutex); + reset(); parseAndAddLDAPServers(*this, config, log); } diff --git a/src/Access/ExternalAuthenticators.h b/src/Access/ExternalAuthenticators.h index 50d9e68f91f..54af87604a6 100644 --- a/src/Access/ExternalAuthenticators.h +++ b/src/Access/ExternalAuthenticators.h @@ -25,13 +25,14 @@ namespace DB class ExternalAuthenticators { public: - explicit ExternalAuthenticators(const Poco::Util::AbstractConfiguration & config, Poco::Logger * log); + void reset(); + void setConfig(const Poco::Util::AbstractConfiguration & config, Poco::Logger * log); void setLDAPServerParams(const String & server, const LDAPServerParams & params); LDAPServerParams getLDAPServerParams(const String & server) const; private: - mutable std::mutex mutex; + mutable std::recursive_mutex mutex; std::map ldap_server_params; };