more consistent ISecurityManager interface: return shared pointer to User to...

more consistent ISecurityManager interface: return shared pointer to User to prevent race conditions with loadFromConfig

dbms/src/Interpreters/Context.cpp

@@ -543,7 +543,7 @@ void Context::calculateUserSettings()
 {
     auto lock = getLock();
 
-    String profile = shared->security_manager->getUser(client_info.current_user).profile;
+    String profile = shared->security_manager->getUser(client_info.current_user)->profile;
 
     /// 1) Set default settings (hardcoded values)
     /// NOTE: we ignore global_context settings (from which it is usually copied)
@@ -564,7 +564,7 @@ void Context::setUser(const String & name, const String & password, const Poco::
 {
     auto lock = getLock();
 
-    const User & user_props = shared->security_manager->authorizeAndGetUser(name, password, address.host());
+    auto user_props = shared->security_manager->authorizeAndGetUser(name, password, address.host());
 
     client_info.current_user = name;
     client_info.current_address = address;
@@ -574,7 +574,7 @@ void Context::setUser(const String & name, const String & password, const Poco::
 
     calculateUserSettings();
 
-    setQuota(user_props.quota, quota_key, name, address.host());
+    setQuota(user_props->quota, quota_key, name, address.host());
 }
 
 

dbms/src/Interpreters/ISecurityManager.h

@@ -13,16 +13,18 @@ namespace DB
 class ISecurityManager
 {
 public:
+    using UserPtr = std::shared_ptr<const User>;
+
     virtual void loadFromConfig(Poco::Util::AbstractConfiguration & config) = 0;
 
     /// Find user and make authorize checks
-    virtual const User & authorizeAndGetUser(
+    virtual UserPtr authorizeAndGetUser(
         const String & user_name,
         const String & password,
         const Poco::Net::IPAddress & address) const = 0;
 
     /// Just find user
-    virtual const User & getUser(const String & user_name) const = 0;
+    virtual UserPtr getUser(const String & user_name) const = 0;
 
     /// Check if the user has access to the database.
     virtual bool hasAccessToDatabase(const String & user_name, const String & database_name) const = 0;

dbms/src/Interpreters/SecurityManager.cpp

@@ -27,6 +27,8 @@ namespace ErrorCodes
     extern const int BAD_ARGUMENTS;
 }
 
+using UserPtr = SecurityManager::UserPtr;
+
 void SecurityManager::loadFromConfig(Poco::Util::AbstractConfiguration & config)
 {
     Container new_users;
@@ -35,12 +37,15 @@ void SecurityManager::loadFromConfig(Poco::Util::AbstractConfiguration & config)
     config.keys("users", config_keys);
 
     for (const std::string & key : config_keys)
-        new_users.emplace(std::piecewise_construct, std::forward_as_tuple(key), std::forward_as_tuple(key, "users." + key, config));
+    {
+        auto user = std::make_shared<const User>(key, "users." + key, config);
+        new_users.emplace(key, std::move(user));
+    }
 
     users = std::move(new_users);
 }
 
-const User & SecurityManager::authorizeAndGetUser(
+UserPtr SecurityManager::authorizeAndGetUser(
     const String & user_name,
     const String & password,
     const Poco::Net::IPAddress & address) const
@@ -50,7 +55,7 @@ const User & SecurityManager::authorizeAndGetUser(
     if (users.end() == it)
         throw Exception("Unknown user " + user_name, ErrorCodes::UNKNOWN_USER);
 
-    if (!it->second.addresses.contains(address))
+    if (!it->second->addresses.contains(address))
         throw Exception("User " + user_name + " is not allowed to connect from address " + address.toString(), ErrorCodes::IP_ADDRESS_NOT_ALLOWED);
 
     auto on_wrong_password = [&]()
@@ -61,7 +66,7 @@ const User & SecurityManager::authorizeAndGetUser(
             throw Exception("Wrong password for user " + user_name, ErrorCodes::WRONG_PASSWORD);
     };
 
-    if (!it->second.password_sha256_hex.empty())
+    if (!it->second->password_sha256_hex.empty())
     {
         unsigned char hash[32];
 
@@ -79,10 +84,10 @@ const User & SecurityManager::authorizeAndGetUser(
 
         Poco::toLowerInPlace(hash_hex);
 
-        if (hash_hex != it->second.password_sha256_hex)
+        if (hash_hex != it->second->password_sha256_hex)
             on_wrong_password();
     }
-    else if (password != it->second.password)
+    else if (password != it->second->password)
     {
         on_wrong_password();
     }
@@ -90,7 +95,7 @@ const User & SecurityManager::authorizeAndGetUser(
     return it->second;
 }
 
-const User & SecurityManager::getUser(const String & user_name) const
+UserPtr SecurityManager::getUser(const String & user_name) const
 {
     auto it = users.find(user_name);
 
@@ -107,8 +112,8 @@ bool SecurityManager::hasAccessToDatabase(const std::string & user_name, const s
     if (users.end() == it)
         throw Exception("Unknown user " + user_name, ErrorCodes::UNKNOWN_USER);
 
-    const auto & user = it->second;
-    return user.databases.empty() || user.databases.count(database_name);
+    auto user = it->second;
+    return user->databases.empty() || user->databases.count(database_name);
 }
 
 }

dbms/src/Interpreters/SecurityManager.h

@@ -13,18 +13,18 @@ namespace DB
 class SecurityManager : public ISecurityManager
 {
 private:
-    using Container = std::map<String, User>;
+    using Container = std::map<String, UserPtr>;
     Container users;
 
 public:
     void loadFromConfig(Poco::Util::AbstractConfiguration & config) override;
 
-    const User & authorizeAndGetUser(
+    UserPtr authorizeAndGetUser(
         const String & user_name,
         const String & password,
         const Poco::Net::IPAddress & address) const override;
 
-    const User & getUser(const String & user_name) const override;
+    UserPtr getUser(const String & user_name) const override;
 
     bool hasAccessToDatabase(const String & user_name, const String & database_name) const override;
 };