Skip to content

Commit fabd719

Browse files
author
Vitaly Baranov
committed
Code cleanups and improvements.
1 parent 51ffc33 commit fabd719

27 files changed

+659
-664
lines changed

Diff for: programs/local/LocalServer.cpp

+5-4
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
#include <Databases/DatabaseMemory.h>
1010
#include <Storages/System/attachSystemTables.h>
1111
#include <Interpreters/ProcessList.h>
12-
#include <Interpreters/Session.h>
1312
#include <Interpreters/executeQuery.h>
1413
#include <Interpreters/loadMetadata.h>
1514
#include <Interpreters/DatabaseCatalog.h>
@@ -377,11 +376,13 @@ void LocalServer::processQueries()
377376

378377
/// we can't mutate global global_context (can lead to races, as it was already passed to some background threads)
379378
/// so we can't reuse it safely as a query context and need a copy here
380-
Session session(global_context, ClientInfo::Interface::TCP);
381-
session.setUser("default", "", Poco::Net::SocketAddress{});
379+
auto context = Context::createCopy(global_context);
382380

383-
auto context = session.makeQueryContext("");
381+
context->makeSessionContext();
382+
context->makeQueryContext();
384383

384+
context->authenticate("default", "", Poco::Net::SocketAddress{});
385+
context->setCurrentQueryId("");
385386
applyCmdSettings(context);
386387

387388
/// Use the same query_id (and thread group) for all queries

Diff for: programs/server/Server.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -47,13 +47,13 @@
4747
#include <Interpreters/ExternalDictionariesLoader.h>
4848
#include <Interpreters/ExternalModelsLoader.h>
4949
#include <Interpreters/ProcessList.h>
50-
#include <Interpreters/Session.h>
5150
#include <Interpreters/loadMetadata.h>
5251
#include <Interpreters/DatabaseCatalog.h>
5352
#include <Interpreters/DNSCacheUpdater.h>
5453
#include <Interpreters/ExternalLoaderXMLConfigRepository.h>
5554
#include <Interpreters/InterserverCredentials.h>
5655
#include <Interpreters/JIT/CompiledExpressionCache.h>
56+
#include <Interpreters/Session.h>
5757
#include <Access/AccessControlManager.h>
5858
#include <Storages/StorageReplicatedMergeTree.h>
5959
#include <Storages/System/attachSystemTables.h>
@@ -1429,7 +1429,7 @@ if (ThreadFuzzer::instance().isEffective())
14291429

14301430
/// Must be done after initialization of `servers`, because async_metrics will access `servers` variable from its thread.
14311431
async_metrics.start();
1432-
Session::enableNamedSessions();
1432+
Session::startupNamedSessions();
14331433

14341434
{
14351435
String level_str = config().getString("text_log.level", "");

Diff for: src/Access/ContextAccess.h

+1
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ class ContextAccess
7070
/// Returns the current user. The function can return nullptr.
7171
UserPtr getUser() const;
7272
String getUserName() const;
73+
std::optional<UUID> getUserID() const { return getParams().user_id; }
7374

7475
/// Returns information about current and enabled roles.
7576
std::shared_ptr<const EnabledRolesInfo> getRolesInfo() const;

Diff for: src/Access/Credentials.h

+2
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ class Credentials
2626
String user_name;
2727
};
2828

29+
/// Does not check the password/credentials and that the specified host is allowed.
30+
/// (Used only internally in cluster, if the secret matches)
2931
class AlwaysAllowCredentials
3032
: public Credentials
3133
{

Diff for: src/Bridge/IBridgeHelper.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include <Poco/Net/HTTPRequest.h>
66
#include <Poco/URI.h>
77
#include <filesystem>
8+
#include <thread>
89

910
namespace fs = std::filesystem;
1011

Diff for: src/Core/MySQL/Authentication.cpp

+6-10
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22
#include <Core/MySQL/PacketsConnection.h>
33
#include <Poco/RandomStream.h>
44
#include <Poco/SHA1Engine.h>
5-
#include <Access/User.h>
6-
#include <Access/AccessControlManager.h>
75
#include <Interpreters/Session.h>
86

97
#include <common/logger_useful.h>
@@ -74,7 +72,7 @@ Native41::Native41(const String & password, const String & auth_plugin_data)
7472
}
7573

7674
void Native41::authenticate(
77-
const String & user_name, std::optional<String> auth_response, Session & session,
75+
const String & user_name, Session & session, std::optional<String> auth_response,
7876
std::shared_ptr<PacketEndpoint> packet_endpoint, bool, const Poco::Net::SocketAddress & address)
7977
{
8078
if (!auth_response)
@@ -87,7 +85,7 @@ void Native41::authenticate(
8785

8886
if (auth_response->empty())
8987
{
90-
session.setUser(user_name, "", address);
88+
session.authenticate(user_name, "", address);
9189
return;
9290
}
9391

@@ -97,9 +95,7 @@ void Native41::authenticate(
9795
+ " bytes, received: " + std::to_string(auth_response->size()) + " bytes.",
9896
ErrorCodes::UNKNOWN_EXCEPTION);
9997

100-
const auto user_authentication = session.getUserAuthentication(user_name);
101-
102-
Poco::SHA1Engine::Digest double_sha1_value = user_authentication.getPasswordDoubleSHA1();
98+
Poco::SHA1Engine::Digest double_sha1_value = session.getPasswordDoubleSHA1(user_name);
10399
assert(double_sha1_value.size() == Poco::SHA1Engine::DIGEST_SIZE);
104100

105101
Poco::SHA1Engine engine;
@@ -112,7 +108,7 @@ void Native41::authenticate(
112108
{
113109
password_sha1[i] = digest[i] ^ static_cast<unsigned char>((*auth_response)[i]);
114110
}
115-
session.setUser(user_name, password_sha1, address);
111+
session.authenticate(user_name, password_sha1, address);
116112
}
117113

118114
#if USE_SSL
@@ -137,7 +133,7 @@ Sha256Password::Sha256Password(RSA & public_key_, RSA & private_key_, Poco::Logg
137133
}
138134

139135
void Sha256Password::authenticate(
140-
const String & user_name, std::optional<String> auth_response, Session & session,
136+
const String & user_name, Session & session, std::optional<String> auth_response,
141137
std::shared_ptr<PacketEndpoint> packet_endpoint, bool is_secure_connection, const Poco::Net::SocketAddress & address)
142138
{
143139
if (!auth_response)
@@ -232,7 +228,7 @@ void Sha256Password::authenticate(
232228
password.pop_back();
233229
}
234230

235-
session.setUser(user_name, password, address);
231+
session.authenticate(user_name, password, address);
236232
}
237233

238234
#endif

Diff for: src/Core/MySQL/Authentication.h

+4-3
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
namespace DB
1717
{
18+
class Session;
1819

1920
namespace MySQLProtocol
2021
{
@@ -32,7 +33,7 @@ class IPlugin
3233
virtual String getAuthPluginData() = 0;
3334

3435
virtual void authenticate(
35-
const String & user_name, std::optional<String> auth_response, Session & session,
36+
const String & user_name, Session & session, std::optional<String> auth_response,
3637
std::shared_ptr<PacketEndpoint> packet_endpoint, bool is_secure_connection, const Poco::Net::SocketAddress & address) = 0;
3738
};
3839

@@ -49,7 +50,7 @@ class Native41 : public IPlugin
4950
String getAuthPluginData() override { return scramble; }
5051

5152
void authenticate(
52-
const String & user_name, std::optional<String> auth_response, Session & session,
53+
const String & user_name, Session & session, std::optional<String> auth_response,
5354
std::shared_ptr<PacketEndpoint> packet_endpoint, bool /* is_secure_connection */, const Poco::Net::SocketAddress & address) override;
5455

5556
private:
@@ -69,7 +70,7 @@ class Sha256Password : public IPlugin
6970
String getAuthPluginData() override { return scramble; }
7071

7172
void authenticate(
72-
const String & user_name, std::optional<String> auth_response, Session & session,
73+
const String & user_name, Session & session, std::optional<String> auth_response,
7374
std::shared_ptr<PacketEndpoint> packet_endpoint, bool is_secure_connection, const Poco::Net::SocketAddress & address) override;
7475

7576
private:

Diff for: src/Core/PostgreSQLProtocol.h

+7-18
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,11 @@
11
#pragma once
22

3-
#include <Access/AccessControlManager.h>
4-
#include <Access/User.h>
53
#include <functional>
6-
#include <Interpreters/Session.h>
7-
#include <Interpreters/Context.h>
84
#include <IO/ReadBuffer.h>
95
#include <IO/ReadHelpers.h>
106
#include <IO/WriteBuffer.h>
117
#include <IO/WriteHelpers.h>
8+
#include <Interpreters/Session.h>
129
#include <common/logger_useful.h>
1310
#include <Poco/Format.h>
1411
#include <Poco/RegularExpression.h>
@@ -808,8 +805,9 @@ class AuthenticationMethod
808805
Messaging::MessageTransport & mt,
809806
const Poco::Net::SocketAddress & address)
810807
{
811-
try {
812-
session.setUser(user_name, password, address);
808+
try
809+
{
810+
session.authenticate(user_name, password, address);
813811
}
814812
catch (const Exception &)
815813
{
@@ -841,7 +839,7 @@ class NoPasswordAuth : public AuthenticationMethod
841839
Messaging::MessageTransport & mt,
842840
const Poco::Net::SocketAddress & address) override
843841
{
844-
setPassword(user_name, "", session, mt, address);
842+
return setPassword(user_name, "", session, mt, address);
845843
}
846844

847845
Authentication::Type getType() const override
@@ -865,7 +863,7 @@ class CleartextPasswordAuth : public AuthenticationMethod
865863
if (type == Messaging::FrontMessageType::PASSWORD_MESSAGE)
866864
{
867865
std::unique_ptr<Messaging::PasswordMessage> password = mt.receive<Messaging::PasswordMessage>();
868-
setPassword(user_name, password->password, session, mt, address);
866+
return setPassword(user_name, password->password, session, mt, address);
869867
}
870868
else
871869
throw Exception(
@@ -902,16 +900,7 @@ class AuthenticationManager
902900
Messaging::MessageTransport & mt,
903901
const Poco::Net::SocketAddress & address)
904902
{
905-
Authentication::Type user_auth_type;
906-
try
907-
{
908-
user_auth_type = session.getUserAuthentication(user_name).getType();
909-
}
910-
catch (const std::exception & e)
911-
{
912-
session.onLogInFailure(user_name, e);
913-
throw;
914-
}
903+
Authentication::Type user_auth_type = session.getAuthenticationType(user_name);
915904

916905
if (type_to_method.find(user_auth_type) != type_to_method.end())
917906
{

Diff for: src/Dictionaries/ClickHouseDictionarySource.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ void registerDictionarySourceClickHouse(DictionarySourceFactory & factory)
255255
/// We should set user info even for the case when the dictionary is loaded in-process (without TCP communication).
256256
if (configuration.is_local)
257257
{
258-
context_copy->setUser(configuration.user, configuration.password, Poco::Net::SocketAddress("127.0.0.1", 0));
258+
context_copy->authenticate(configuration.user, configuration.password, Poco::Net::SocketAddress("127.0.0.1", 0));
259259
context_copy = copyContextAndApplySettings(config_prefix, context_copy, config);
260260
}
261261

Diff for: src/IO/ReadBufferFromFileDescriptor.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include <Common/UnicodeBar.h>
1313
#include <Common/TerminalSize.h>
1414
#include <IO/Operators.h>
15+
#include <IO/Progress.h>
1516

1617

1718
namespace ProfileEvents

Diff for: src/Interpreters/Context.cpp

+35-31
Original file line numberDiff line numberDiff line change
@@ -588,61 +588,52 @@ ConfigurationPtr Context::getUsersConfig()
588588
}
589589

590590

591-
void Context::setUser(const Credentials & credentials, const Poco::Net::SocketAddress & address)
591+
void Context::authenticate(const String & name, const String & password, const Poco::Net::SocketAddress & address)
592592
{
593-
auto lock = getLock();
593+
authenticate(BasicCredentials(name, password), address);
594+
}
595+
596+
void Context::authenticate(const Credentials & credentials, const Poco::Net::SocketAddress & address)
597+
{
598+
auto authenticated_user_id = getAccessControlManager().login(credentials, address.host());
594599

595600
client_info.current_user = credentials.getUserName();
596601
client_info.current_address = address;
597602

598603
#if defined(ARCADIA_BUILD)
599604
/// This is harmful field that is used only in foreign "Arcadia" build.
600-
client_info.current_password.clear();
601605
if (const auto * basic_credentials = dynamic_cast<const BasicCredentials *>(&credentials))
602606
client_info.current_password = basic_credentials->getPassword();
603607
#endif
604608

605-
/// Find a user with such name and check the credentials.
606-
auto new_user_id = getAccessControlManager().login(credentials, address.host());
607-
auto new_access = getAccessControlManager().getContextAccess(
608-
new_user_id, /* current_roles = */ {}, /* use_default_roles = */ true,
609-
settings, current_database, client_info);
609+
setUser(authenticated_user_id);
610+
}
611+
612+
void Context::setUser(const UUID & user_id_)
613+
{
614+
auto lock = getLock();
610615

611-
user_id = new_user_id;
612-
access = std::move(new_access);
616+
user_id = user_id_;
617+
618+
access = getAccessControlManager().getContextAccess(
619+
user_id_, /* current_roles = */ {}, /* use_default_roles = */ true, settings, current_database, client_info);
613620

614621
auto user = access->getUser();
615622
current_roles = std::make_shared<std::vector<UUID>>(user->granted_roles.findGranted(user->default_roles));
616623

617-
if (!user->default_database.empty())
618-
setCurrentDatabase(user->default_database);
619-
620624
auto default_profile_info = access->getDefaultProfileInfo();
621625
settings_constraints_and_current_profiles = default_profile_info->getConstraintsAndProfileIDs();
622626
applySettingsChanges(default_profile_info->settings);
623-
}
624627

625-
void Context::setUser(const String & name, const String & password, const Poco::Net::SocketAddress & address)
626-
{
627-
setUser(BasicCredentials(name, password), address);
628-
}
629-
630-
void Context::setUserWithoutCheckingPassword(const String & name, const Poco::Net::SocketAddress & address)
631-
{
632-
setUser(AlwaysAllowCredentials(name), address);
628+
if (!user->default_database.empty())
629+
setCurrentDatabase(user->default_database);
633630
}
634631

635632
std::shared_ptr<const User> Context::getUser() const
636633
{
637634
return getAccess()->getUser();
638635
}
639636

640-
void Context::setQuotaKey(String quota_key_)
641-
{
642-
auto lock = getLock();
643-
client_info.quota_key = std::move(quota_key_);
644-
}
645-
646637
String Context::getUserName() const
647638
{
648639
return getAccess()->getUserName();
@@ -655,6 +646,13 @@ std::optional<UUID> Context::getUserID() const
655646
}
656647

657648

649+
void Context::setQuotaKey(String quota_key_)
650+
{
651+
auto lock = getLock();
652+
client_info.quota_key = std::move(quota_key_);
653+
}
654+
655+
658656
void Context::setCurrentRoles(const std::vector<UUID> & current_roles_)
659657
{
660658
auto lock = getLock();
@@ -736,10 +734,13 @@ ASTPtr Context::getRowPolicyCondition(const String & database, const String & ta
736734
void Context::setInitialRowPolicy()
737735
{
738736
auto lock = getLock();
739-
auto initial_user_id = getAccessControlManager().find<User>(client_info.initial_user);
740737
initial_row_policy = nullptr;
741-
if (initial_user_id)
742-
initial_row_policy = getAccessControlManager().getEnabledRowPolicies(*initial_user_id, {});
738+
if (client_info.initial_user == client_info.current_user)
739+
return;
740+
auto initial_user_id = getAccessControlManager().find<User>(client_info.initial_user);
741+
if (!initial_user_id)
742+
return;
743+
initial_row_policy = getAccessControlManager().getEnabledRowPolicies(*initial_user_id, {});
743744
}
744745

745746

@@ -1180,6 +1181,9 @@ void Context::setCurrentQueryId(const String & query_id)
11801181
}
11811182

11821183
client_info.current_query_id = query_id_to_set;
1184+
1185+
if (client_info.query_kind == ClientInfo::QueryKind::INITIAL_QUERY)
1186+
client_info.initial_query_id = client_info.current_query_id;
11831187
}
11841188

11851189
void Context::killCurrentQuery()

0 commit comments

Comments
 (0)