From bdfea652c10e6702963e5856c2e0c606baf08fa7 Mon Sep 17 00:00:00 2001 From: Denis Glazachev Date: Thu, 20 Aug 2020 11:39:27 +0400 Subject: [PATCH] Change user_template to roles Change top_enclosing_storage to access_control_manager Simplify the lookup in peer storages --- programs/server/config.xml | 13 +++-- src/Access/AccessControlManager.cpp | 1 - src/Access/LDAPAccessStorage.cpp | 81 +++++++++++++++-------------- src/Access/LDAPAccessStorage.h | 10 ++-- 4 files changed, 57 insertions(+), 48 deletions(-) diff --git a/programs/server/config.xml b/programs/server/config.xml index ba3ec90affb..cdae1c5409e 100644 --- a/programs/server/config.xml +++ b/programs/server/config.xml @@ -262,14 +262,17 @@ with the following parameters: server - one of LDAP server names defined in 'ldap_servers' config section above. This parameter is mandatory and cannot be empty. - user_template - name of a locally defined user that will be used as a template for all users retrieved from the LDAP server. - Each LDAP user will inherit all parameters from 'user_template' user, except for the authentication method, - which will be set to 'ldap' and the 'server' defined above will be used as a LDAP server for authentication. - This parameter is optional, if not specified, 'default' user will be used as a template. + roles - section with a list of locally defined roles that will be granted to each user retrieved from the LDAP server. + If no roles are specified, user will not be able to perform any actions after authentication. + If any of the listed roles is not defined locally at the time of authentication, the authenthication attept + will fail as if the provided password was incorrect. Example: my_ldap_server - my_user + + + + --> diff --git a/src/Access/AccessControlManager.cpp b/src/Access/AccessControlManager.cpp index 06347412da7..9dcb1cdd4f9 100644 --- a/src/Access/AccessControlManager.cpp +++ b/src/Access/AccessControlManager.cpp @@ -157,7 +157,6 @@ void AccessControlManager::addUsersConfigStorage(const Poco::Util::AbstractConfi addUsersConfigStorage(UsersConfigAccessStorage::STORAGE_TYPE, users_config_); } - void AccessControlManager::addUsersConfigStorage(const String & storage_name_, const Poco::Util::AbstractConfiguration & users_config_) { auto check_setting_name_function = [this](const std::string_view & setting_name) { checkSettingNameIsAllowed(setting_name); }; diff --git a/src/Access/LDAPAccessStorage.cpp b/src/Access/LDAPAccessStorage.cpp index a98eab5a2b0..8285239abb0 100644 --- a/src/Access/LDAPAccessStorage.cpp +++ b/src/Access/LDAPAccessStorage.cpp @@ -1,5 +1,7 @@ #include +#include #include +#include #include #include #include @@ -20,39 +22,42 @@ LDAPAccessStorage::LDAPAccessStorage(const String & storage_name_) } -void LDAPAccessStorage::setConfiguration(IAccessStorage * top_enclosing_storage_, const Poco::Util::AbstractConfiguration & config, const String & prefix) +void LDAPAccessStorage::setConfiguration(AccessControlManager * access_control_manager_, const Poco::Util::AbstractConfiguration & config, const String & prefix) { + // TODO: switch to passing config as a ConfigurationView and remove this extra prefix once a version of Poco with proper implementation is available. const String prefix_str = (prefix.empty() ? "" : prefix + "."); std::scoped_lock lock(mutex); const bool has_server = config.has(prefix_str + "server"); - const bool has_user_template = config.has(prefix_str + "user_template"); + const bool has_roles = config.has(prefix_str + "roles"); if (!has_server) throw Exception("Missing 'server' field for LDAP user directory.", ErrorCodes::BAD_ARGUMENTS); const auto ldap_server_cfg = config.getString(prefix_str + "server"); - String user_template_cfg; - if (ldap_server_cfg.empty()) throw Exception("Empty 'server' field for LDAP user directory.", ErrorCodes::BAD_ARGUMENTS); - if (has_user_template) - user_template_cfg = config.getString(prefix_str + "user_template"); + std::set roles_cfg; + if (has_roles) + { + Poco::Util::AbstractConfiguration::Keys role_names; + config.keys(prefix_str + "roles", role_names); - if (user_template_cfg.empty()) - user_template_cfg = "default"; + // Currently, we only extract names of roles from the section names and assign them directly and unconditionally. + roles_cfg.insert(role_names.begin(), role_names.end()); + } ldap_server = ldap_server_cfg; - user_template = user_template_cfg; - top_enclosing_storage = top_enclosing_storage_; + roles.swap(roles_cfg); + access_control_manager = access_control_manager_; } bool LDAPAccessStorage::isConfiguredNoLock() const { - return !ldap_server.empty() && !user_template.empty() && top_enclosing_storage; + return !ldap_server.empty() &&/* !roles.empty() &&*/ access_control_manager; } @@ -74,13 +79,6 @@ std::optional LDAPAccessStorage::findImpl(EntityType type, const String & { std::scoped_lock lock(mutex); - // Detect and avoid loops/duplicate creations. - if (helper_lookup_in_progress) - return {}; - - helper_lookup_in_progress = true; - SCOPE_EXIT({ helper_lookup_in_progress = false; }); - // Return the id immediately if we already have it. const auto id = memory_storage.find(type, name); if (id.has_value()) @@ -89,32 +87,39 @@ std::optional LDAPAccessStorage::findImpl(EntityType type, const String & if (!isConfiguredNoLock()) return {}; - // Stop if entity exists anywhere else, to avoid duplicates. - if (top_enclosing_storage->find(type, name)) - return {}; + // Stop if entity exists anywhere else, to avoid generating duplicates. + auto * this_base = dynamic_cast(this); + const auto storages = access_control_manager->getStoragesPtr(); + for (const auto & storage : *storages) + { + if (storage.get() != this_base && storage->find(type, name)) + return {}; + } // Entity doesn't exist. We are going to create one. - - // Retrieve the template first. - std::shared_ptr user_tmp; - try - { - user_tmp = top_enclosing_storage->read(user_template); - if (!user_tmp) - throw Exception("Retrieved user is empty", IAccessEntity::TypeInfo::get(IAccessEntity::Type::USER).not_found_error_code); - } - catch (...) - { - tryLogCurrentException(getLogger(), "Unable to retrieve user template '" + user_template + "' from access storage '" + top_enclosing_storage->getStorageName() + "'"); - return {}; - } - - // Build the new entity based on the existing template. - const auto user = std::make_shared(*user_tmp); + const auto user = std::make_shared(); user->setName(name); user->authentication = Authentication(Authentication::Type::LDAP_SERVER); user->authentication.setServerName(ldap_server); + for (const auto& role_name : roles) { + std::optional role_id; + + try + { + role_id = access_control_manager->find(role_name); + if (!role_id) + throw Exception("Retrieved role info is empty", IAccessEntity::TypeInfo::get(IAccessEntity::Type::ROLE).not_found_error_code); + } + catch (...) + { + tryLogCurrentException(getLogger(), "Unable to retrieve role '" + role_name + "' info from access storage '" + access_control_manager->getStorageName() + "'"); + return {}; + } + + user->granted_roles.grant(role_id.value()); + } + return memory_storage.insert(user); } diff --git a/src/Access/LDAPAccessStorage.h b/src/Access/LDAPAccessStorage.h index 90a87eca292..eb8b5cac1bc 100644 --- a/src/Access/LDAPAccessStorage.h +++ b/src/Access/LDAPAccessStorage.h @@ -3,6 +3,7 @@ #include #include #include +#include namespace Poco @@ -16,6 +17,8 @@ namespace Poco namespace DB { +class AccessControlManager; + /// Implementation of IAccessStorage which allows attaching users from a remote LDAP server. /// Currently, any user name will be treated as a name of an existing remote user, /// a user info entity will be created, with LDAP_SERVER authentication type. @@ -27,7 +30,7 @@ public: explicit LDAPAccessStorage(const String & storage_name_ = STORAGE_TYPE); virtual ~LDAPAccessStorage() override = default; - void setConfiguration(IAccessStorage * top_enclosing_storage_, const Poco::Util::AbstractConfiguration & config, const String & prefix = ""); + void setConfiguration(AccessControlManager * access_control_manager_, const Poco::Util::AbstractConfiguration & config, const String & prefix = ""); public: // IAccessStorage implementations. virtual const char * getStorageType() const override; @@ -53,9 +56,8 @@ private: mutable std::recursive_mutex mutex; String ldap_server; - String user_template; - IAccessStorage * top_enclosing_storage = nullptr; - mutable bool helper_lookup_in_progress = false; + std::set roles; + AccessControlManager * access_control_manager = nullptr; mutable MemoryAccessStorage memory_storage; }; }