Allow caching of successful "bind" attempts to LDAP server for configurable period of time

[traceon]

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Changelog category:

• Improvement

Changelog entry:

• Added verification_cooldown parameter in LDAP server connection configuration to allow caching of successful "bind" attempts for configurable period of time

Detailed description / Documentation draft:

• Documented in a form of in-line comments in the sample config.xml file. See <ldap_servers> tag.

[traceon]

@den-crane Does this look like a good solution for the slow LDAP server operations, that you were pointing out?

verification_cooldown - a period of time, in seconds, after a successful bind attempt, during which a user will be assumed
                         to be successfully authenticated for all consecutive requests without contacting the LDAP server.
                        Specify 0 (the default) to disable caching and force contacting the LDAP server for each authentication request.

[vitlibar]

I suppose it's better to use a ready out-of-the-box solution. I mean the class ExpireLRUCache which Poco provides. It's thread-safe, it's well-tested, and it already can do what you want. Check it out:

class ExternalAuthenticators
{
    ...
private:
    using CacheOfCorrectPasswords = ExpireLRUCache<UUID /* identifier of user */, UInt128 /* hash of password */>>;

    struct LDAPServerInfo
    {
        std::shared_ptr<const LDAPServerParams> params;
        CacheOfCorrectPasswords cache_of_correct_passwords; // clears every time when `params` changes
    };

    std::mutex mutex;
    std::unordered_map<String, LDAPServerInfo> ldap_servers;
};

void ExternalAuthenticators::setLDAPServerParams(const String & server, const std::shared_ptr<const LDAPServerParams> & params)
{
    std::scoped_lock lock(mutex);
    auto it = ldap_servers.find(server);
    if (it == ldap_servers.end())
    {
        ldap_servers[server].params = params;
        return;
    }
    
    const auto & old_params = it->second.params;
    if ((params == old_params) || (*params == *old_params))
        return;
    
    it->second.params = params;
    it->second.cache_of_correct_passwords.reset();
}

bool ExternalAuthenticators::isPasswordInCacheOfCorrectPasswords(const String & ldap_server, const UUID & user_id, const String & password)
{
    std::scoped_lock lock(mutex);
    auto it = ldap_servers.find(ldap_server);
    if (it == ldap_servers.end())
        return false;

    auto & cache_of_correct_passwords = it->second.cache_of_correct_passwords;
    auto hash = cache_of_correct_passwords.get(user_id);
    return hash && (*hash == md5(password));
}

void ExternalAuthenticators::putCorrectPasswordToCache(const String & ldap_server, const UUID & user_id, const String & password)
{
    std::scoped_lock lock(mutex);
    auto it = ldap_servers.find(ldap_server);
    if (it == ldap_servers.end())
        return;

    auto & cache_of_correct_passwords = it->second.cache_of_correct_passwords;
    cache_of_correct_passwords.set(user_id, md5(password));
}

[traceon]

I didn't use ExpireLRUCache because I didn't like the idea of decoupling something that is already perfectly mapped. There is one-to-one correspondence between the cached info and the user info. This decoupling is not necessary, and provides additional orphan cache entry cleanup problems. Also, it looks like ExpireLRUCache doesn't support unlimited cache size.

[vitlibar]

#15988 (comment)

[vitlibar]

I didn't use ExpireLRUCache because I didn't like the idea of decoupling something that is already perfectly mapped.

As you said that's not a tight spot so we're not forced to speed up our code here at any cost. It's better to keep invariants.

[traceon]

@vitlibar ExpireLRUCache seemingly updates the expire time on access. This is not what I need. It is poorly documented (even has bugs in the docs.) It is not used anywhere in ClickHouse. I will move the caching to the ExternalAuthenticators as per your request, but not going to use ExpireLRUCache, it is completely wrong tool for this task.

[vitlibar]

This comment is one of the reasons why password_ used to be passed first: it's used almost for each authentication type unlike user_.