Skip to content

Commit 6c1f64b

Browse files
ijonijon
authored
Merge ed332cb into 01359b9 
2 parents 01359b9 + ed332cb commit 6c1f64b

File tree

18 files changed

+1336
-176
lines changed

18 files changed

+1336
-176
lines changed

‎ydb/core/base/auth.cpp

Lines changed: 56 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,83 @@
1+
#include <ydb/library/aclib/aclib.h>
2+
13
#include "auth.h"
24

35
namespace NKikimr {
46

57
namespace {
68

7-
bool HasToken(const TAppData* appData, const NACLib::TUserToken& userToken) {
8-
for (const auto& sid : appData->AdministrationAllowedSIDs) {
9-
if (userToken.IsExist(sid)) {
9+
template <class Iterable>
10+
bool IsTokenAllowedImpl(const NACLib::TUserToken* userToken, const Iterable& allowedSIDs) {
11+
// empty set contains any element
12+
if (allowedSIDs.empty()) {
13+
return true;
14+
}
15+
16+
// empty element does not belong to any non-empty set
17+
if (!userToken || userToken->GetUserSID().empty()) {
18+
return false;
19+
}
20+
21+
// its enough to a single sid (user or group) from the token to be in the set
22+
for (const auto& sid : allowedSIDs) {
23+
if (userToken->IsExist(sid)) {
1024
return true;
1125
}
1226
}
1327

1428
return false;
1529
}
1630

31+
NACLib::TUserToken ParseUserToken(const TString& userTokenSerialized) {
32+
NACLibProto::TUserToken tokenPb;
33+
if (!tokenPb.ParseFromString(userTokenSerialized)) {
34+
// we want to treat invalid token as empty,
35+
// so then result object must be recreated (or cleared)
36+
// because failed parsing makes result object dirty
37+
tokenPb = NACLibProto::TUserToken();
38+
}
39+
return NACLib::TUserToken(tokenPb);
1740
}
1841

19-
bool IsAdministrator(const TAppData* appData, const TString& userToken) {
20-
if (appData->AdministrationAllowedSIDs.empty()) {
21-
return true;
22-
}
42+
}
2343

24-
if (!userToken) {
25-
return false;
26-
}
44+
bool IsTokenAllowed(const NACLib::TUserToken* userToken, const TVector<TString>& allowedSIDs) {
45+
return IsTokenAllowedImpl(userToken, allowedSIDs);
46+
}
2747

28-
NACLibProto::TUserToken tokenPb;
29-
if (!tokenPb.ParseFromString(userToken)) {
30-
return false;
31-
}
48+
bool IsTokenAllowed(const NACLib::TUserToken* userToken, const NProtoBuf::RepeatedPtrField<TString>& allowedSIDs) {
49+
return IsTokenAllowedImpl(userToken, allowedSIDs);
50+
}
51+
52+
bool IsTokenAllowed(const TString& userTokenSerialized, const TVector<TString>& allowedSIDs) {
53+
NACLib::TUserToken userToken = ParseUserToken(userTokenSerialized);
54+
return IsTokenAllowed(&userToken, allowedSIDs);
55+
}
56+
57+
bool IsTokenAllowed(const TString& userTokenSerialized, const NProtoBuf::RepeatedPtrField<TString>& allowedSIDs) {
58+
NACLib::TUserToken userToken = ParseUserToken(userTokenSerialized);
59+
return IsTokenAllowed(&userToken, allowedSIDs);
60+
}
3261

33-
return HasToken(appData, NACLib::TUserToken(std::move(tokenPb)));
62+
bool IsAdministrator(const TAppData* appData, const TString& userTokenSerialized) {
63+
return IsTokenAllowed(userTokenSerialized, appData->AdministrationAllowedSIDs);
3464
}
3565

3666
bool IsAdministrator(const TAppData* appData, const NACLib::TUserToken* userToken) {
37-
if (appData->AdministrationAllowedSIDs.empty()) {
38-
return true;
39-
}
67+
return IsTokenAllowed(userToken, appData->AdministrationAllowedSIDs);
68+
}
69+
4070

41-
if (!userToken || userToken->GetSerializedToken().empty()) {
71+
bool IsDatabaseAdministrator(const NACLib::TUserToken* userToken, const NACLib::TSID& databaseOwner) {
72+
// no database, no access
73+
if (databaseOwner.empty()) {
4274
return false;
4375
}
44-
45-
return HasToken(appData, *userToken);
76+
// empty token can't have raised access level
77+
if (!userToken || userToken->GetUserSID().empty()) {
78+
return false;
79+
}
80+
return userToken->IsExist(databaseOwner);
4681
}
4782

4883
}

‎ydb/core/base/auth.h

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,17 @@
55

66
namespace NKikimr {
77

8-
bool IsAdministrator(const TAppData* appData, const TString& userToken);
8+
// Check token against given list of allowed sids
9+
bool IsTokenAllowed(const NACLib::TUserToken* userToken, const TVector<TString>& allowedSIDs);
10+
bool IsTokenAllowed(const NACLib::TUserToken* userToken, const NProtoBuf::RepeatedPtrField<TString>& allowedSIDs);
11+
bool IsTokenAllowed(const TString& userTokenSerialized, const TVector<TString>& allowedSIDs);
12+
bool IsTokenAllowed(const TString& userTokenSerialized, const NProtoBuf::RepeatedPtrField<TString>& allowedSIDs);
913

14+
// Check token against AdministrationAllowedSIDs
15+
bool IsAdministrator(const TAppData* appData, const TString& userTokenSerialized);
1016
bool IsAdministrator(const TAppData* appData, const NACLib::TUserToken* userToken);
1117

18+
// Check token against database owner
19+
bool IsDatabaseAdministrator(const NACLib::TUserToken* userToken, const NACLib::TSID& databaseOwner);
20+
1221
}

‎ydb/core/base/auth_ut.cpp

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
#include <library/cpp/testing/unittest/registar.h>
2+
3+
#include "auth.h"
4+
5+
using namespace NKikimr;
6+
7+
Y_UNIT_TEST_SUITE(AuthTokenAllowed) {
8+
9+
const TVector<TString> EmptyList;
10+
11+
// Empty list allows empty token (regardless of its kind)
12+
Y_UNIT_TEST(PassOnEmptyListAndEmptyToken) {
13+
NACLib::TUserToken token(NACLib::TUserToken::TUserTokenInitFields{});
14+
UNIT_ASSERT_EQUAL(IsTokenAllowed(&token, EmptyList), true);
15+
UNIT_ASSERT_EQUAL(IsTokenAllowed(token.SerializeAsString(), EmptyList), true);
16+
}
17+
Y_UNIT_TEST(PassOnEmptyListAndTokenWithEmptyUserSid) {
18+
NACLib::TUserToken token({ .UserSID = "" });
19+
UNIT_ASSERT_EQUAL(IsTokenAllowed(&token, EmptyList), true);
20+
UNIT_ASSERT_EQUAL(IsTokenAllowed(token.SerializeAsString(), EmptyList), true);
21+
}
22+
Y_UNIT_TEST(PassOnEmptyListAndTokenWithEmptyUserSidAndGroups) {
23+
NACLib::TUserToken token({ .UserSID = "", .GroupSIDs = {"group1"} });
24+
UNIT_ASSERT_EQUAL(IsTokenAllowed(&token, EmptyList), true);
25+
UNIT_ASSERT_EQUAL(IsTokenAllowed(token.SerializeAsString(), EmptyList), true);
26+
}
27+
Y_UNIT_TEST(PassOnEmptyListAndNoToken) {
28+
UNIT_ASSERT_EQUAL(IsTokenAllowed(nullptr, EmptyList), true);
29+
}
30+
Y_UNIT_TEST(PassOnEmptyListAndToken) {
31+
NACLib::TUserToken token({ .UserSID = "user1" });
32+
UNIT_ASSERT_EQUAL(IsTokenAllowed(&token, EmptyList), true);
33+
UNIT_ASSERT_EQUAL(IsTokenAllowed(token.SerializeAsString(), EmptyList), true);
34+
}
35+
Y_UNIT_TEST(PassOnEmptyListAndInvalidTokenSerialized) {
36+
UNIT_ASSERT_EQUAL(IsTokenAllowed("invalid-serialized-protobuf", EmptyList), true);
37+
}
38+
39+
// Non empty list forbids empty token (regardless of its kind)
40+
Y_UNIT_TEST(FailOnListAndEmptyToken) {
41+
NACLib::TUserToken token(NACLib::TUserToken::TUserTokenInitFields{});
42+
UNIT_ASSERT_EQUAL(IsTokenAllowed(&token, {"entry"}), false);
43+
UNIT_ASSERT_EQUAL(IsTokenAllowed(token.SerializeAsString(), {"entry"}), false);
44+
}
45+
Y_UNIT_TEST(FailOnListAndTokenWithEmptyUserSid) {
46+
NACLib::TUserToken token({ .UserSID = "" });
47+
UNIT_ASSERT_EQUAL(IsTokenAllowed(&token, {"entry"}), false);
48+
UNIT_ASSERT_EQUAL(IsTokenAllowed(token.SerializeAsString(), {"entry"}), false);
49+
}
50+
Y_UNIT_TEST(FailOnListAndTokenWithEmptyUserSidAndGroups) {
51+
NACLib::TUserToken token({ .UserSID = "", .GroupSIDs = {"group1"} });
52+
UNIT_ASSERT_EQUAL(IsTokenAllowed(&token, {"entry"}), false);
53+
UNIT_ASSERT_EQUAL(IsTokenAllowed(token.SerializeAsString(), {"entry"}), false);
54+
}
55+
Y_UNIT_TEST(FailOnListAndNoToken) {
56+
UNIT_ASSERT_EQUAL(IsTokenAllowed(nullptr, {"entry"}), false);
57+
}
58+
59+
// List matches token
60+
Y_UNIT_TEST(PassOnListMatchUserSid) {
61+
NACLib::TUserToken token({ .UserSID = "user1" });
62+
UNIT_ASSERT_EQUAL(IsTokenAllowed(&token, {"group1", "group2", "user1", "user2"}), true);
63+
UNIT_ASSERT_EQUAL(IsTokenAllowed(token.SerializeAsString(), {"group1", "group2", "user1", "user2"}), true);
64+
}
65+
Y_UNIT_TEST(PassOnListMatchUserSidWithGroup) {
66+
NACLib::TUserToken token({ .UserSID = "user1", .GroupSIDs = {"no-match-group"} });
67+
UNIT_ASSERT_EQUAL(IsTokenAllowed(&token, {"group1", "group2", "user1", "user2"}), true);
68+
UNIT_ASSERT_EQUAL(IsTokenAllowed(token.SerializeAsString(), {"group1", "group2", "user1", "user2"}), true);
69+
}
70+
Y_UNIT_TEST(PassOnListMatchGroupSid) {
71+
NACLib::TUserToken token({ .UserSID = "no-match-user", .GroupSIDs = {"group2"} });
72+
UNIT_ASSERT_EQUAL(IsTokenAllowed(&token, {"group1", "group2", "user1", "user2"}), true);
73+
UNIT_ASSERT_EQUAL(IsTokenAllowed(token.SerializeAsString(), {"group1", "group2", "user1", "user2"}), true);
74+
}
75+
76+
// List does not matchs token
77+
Y_UNIT_TEST(FailOnListMatchGroupSid) {
78+
NACLib::TUserToken token({ .UserSID = "no-match-user", .GroupSIDs = {"no-match-group"} });
79+
UNIT_ASSERT_EQUAL(IsTokenAllowed(&token, {"group1", "group2", "user1", "user2"}), false);
80+
UNIT_ASSERT_EQUAL(IsTokenAllowed(token.SerializeAsString(), {"group1", "group2", "user1", "user2"}), false);
81+
}
82+
83+
}
84+
85+
Y_UNIT_TEST_SUITE(AuthDatabaseAdmin) {
86+
87+
// Empty owner forbids empty token (regardless of its kind)
88+
Y_UNIT_TEST(FailOnEmptyOwnerAndEmptyToken) {
89+
NACLib::TUserToken token(NACLib::TUserToken::TUserTokenInitFields{});
90+
UNIT_ASSERT_EQUAL(IsDatabaseAdministrator(&token, ""), false);
91+
}
92+
Y_UNIT_TEST(FailOnEmptyOwnerAndTokenWithEmptyUserSid) {
93+
NACLib::TUserToken token({ .UserSID = "" });
94+
UNIT_ASSERT_EQUAL(IsDatabaseAdministrator(&token, ""), false);
95+
}
96+
Y_UNIT_TEST(FailOnEmptyOwnerAndTokenWithEmptyUserSidAndGroups) {
97+
NACLib::TUserToken token({ .UserSID = "", .GroupSIDs = {"group1"} });
98+
UNIT_ASSERT_EQUAL(IsDatabaseAdministrator(&token, ""), false);
99+
}
100+
Y_UNIT_TEST(FailOnEmptyOwnerAndNoToken) {
101+
UNIT_ASSERT_EQUAL(IsDatabaseAdministrator(nullptr, ""), false);
102+
}
103+
104+
// Non empty owner forbids empty token (regardless of its kind)
105+
Y_UNIT_TEST(FailOnOwnerAndEmptyToken) {
106+
NACLib::TUserToken token(NACLib::TUserToken::TUserTokenInitFields{});
107+
UNIT_ASSERT_EQUAL(IsDatabaseAdministrator(&token, "owner"), false);
108+
}
109+
Y_UNIT_TEST(FailOnOwnerAndTokenWithEmptyUserSid) {
110+
NACLib::TUserToken token({ .UserSID = "" });
111+
UNIT_ASSERT_EQUAL(IsDatabaseAdministrator(&token, "owner"), false);
112+
}
113+
Y_UNIT_TEST(FailOnOwnerAndTokenWithEmptyUserSidAndGroups) {
114+
NACLib::TUserToken token({ .UserSID = "", .GroupSIDs = {"group1"} });
115+
UNIT_ASSERT_EQUAL(IsDatabaseAdministrator(&token, "owner"), false);
116+
}
117+
Y_UNIT_TEST(FailOnOwnerAndNoToken) {
118+
UNIT_ASSERT_EQUAL(IsDatabaseAdministrator(nullptr, "owner"), false);
119+
}
120+
121+
// Owner matches token
122+
Y_UNIT_TEST(PassOnOwnerMatchUserSid) {
123+
NACLib::TUserToken token({ .UserSID = "user1" });
124+
UNIT_ASSERT_EQUAL(IsDatabaseAdministrator(&token, "user1"), true);
125+
}
126+
Y_UNIT_TEST(PassOnOwnerMatchUserSidWithGroup) {
127+
NACLib::TUserToken token({ .UserSID = "user1", .GroupSIDs = {"group1"} });
128+
UNIT_ASSERT_EQUAL(IsDatabaseAdministrator(&token, "user1"), true);
129+
}
130+
Y_UNIT_TEST(PassOnOwnerMatchGroupSid) {
131+
NACLib::TUserToken token({ .UserSID = "user1", .GroupSIDs = {"group1"} });
132+
UNIT_ASSERT_EQUAL(IsDatabaseAdministrator(&token, "group1"), true);
133+
}
134+
135+
}

‎ydb/core/base/ut_auth/ya.make

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
UNITTEST_FOR(ydb/core/base)
2+
3+
FORK_SUBTESTS()
4+
5+
SIZE(MEDIUM)
6+
7+
PEERDIR(
8+
library/cpp/testing/unittest
9+
ydb/library/aclib
10+
)
11+
12+
YQL_LAST_ABI_VERSION()
13+
14+
SRCS(
15+
auth_ut.cpp
16+
)
17+
18+
END()

‎ydb/core/base/ya.make

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,5 +125,6 @@ RECURSE(
125125

126126
RECURSE_FOR_TESTS(
127127
ut
128+
ut_auth
128129
ut_board_subscriber
129130
)

‎ydb/core/kqp/ut/scheme/kqp_scheme_ut.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2823,7 +2823,7 @@ Y_UNIT_TEST_SUITE(KqpScheme) {
28232823
auto result = userSession.AlterTable("/Root/SecondaryKeys/Index/indexImplTable", tableSettings).ExtractValueSync();
28242824
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::UNAUTHORIZED, result.GetIssues().ToString());
28252825
UNIT_ASSERT_STRING_CONTAINS(result.GetIssues().ToString(),
2826-
"Error: Access denied for user@builtin to path Root/SecondaryKeys/Index/indexImplTable"
2826+
"Error: Access denied for user@builtin on path Root/SecondaryKeys/Index/indexImplTable"
28272827
);
28282828
}
28292829
// grant necessary permission

‎ydb/core/kqp/workload_service/actors/scheme_actors.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,16 +325,19 @@ class TPoolCreatorActor : public TSchemeActorBase<TPoolCreatorActor> {
325325
protected:
326326
void StartRequest() override {
327327
LOG_D("Start pool creating");
328+
const auto& database = DatabaseIdToDatabase(DatabaseId);
329+
328330
auto event = std::make_unique<TEvTxUserProxy::TEvProposeTransaction>();
329331

330332
auto& schemeTx = *event->Record.MutableTransaction()->MutableModifyScheme();
331-
schemeTx.SetWorkingDir(JoinPath({DatabaseIdToDatabase(DatabaseId), ".metadata/workload_manager/pools"}));
333+
schemeTx.SetWorkingDir(JoinPath({database, ".metadata/workload_manager/pools"}));
332334
schemeTx.SetOperationType(NKikimrSchemeOp::ESchemeOpCreateResourcePool);
333335
schemeTx.SetInternal(true);
334336

335337
BuildCreatePoolRequest(*schemeTx.MutableCreateResourcePool());
336338
BuildModifyAclRequest(*schemeTx.MutableModifyACL());
337339

340+
event->Record.SetDatabaseName(database);
338341
if (UserToken) {
339342
event->Record.SetUserToken(UserToken->SerializeAsString());
340343
}

‎ydb/core/protos/feature_flags.proto

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,4 +190,7 @@ message TFeatureFlags {
190190
optional bool EnableColumnStore = 165 [default = false];
191191
optional bool EnableStrictAclCheck = 166 [default = false];
192192
optional bool DatabaseYamlConfigAllowed = 167 [default = false];
193+
// deny non-administrators the privilege of administering local users and groups
194+
optional bool EnableStrictUserManagement = 168 [default = false];
195+
optional bool EnableDatabaseAdmin = 169 [default = false];
193196
}

‎ydb/core/testlib/basics/feature_flags.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,8 @@ class TTestFeatureFlagsHolder {
7373
FEATURE_FLAG_SETTER(EnableFollowerStats)
7474
FEATURE_FLAG_SETTER(EnableExportChecksums)
7575
FEATURE_FLAG_SETTER(EnableTopicTransfer)
76+
FEATURE_FLAG_SETTER(EnableStrictUserManagement)
77+
FEATURE_FLAG_SETTER(EnableDatabaseAdmin)
7678

7779
#undef FEATURE_FLAG_SETTER
7880
};

‎ydb/core/testlib/test_client.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2671,6 +2671,9 @@ namespace Tests {
26712671
TAutoPtr<NMsgBusProxy::TBusBlobStorageConfigRequest> request(new NMsgBusProxy::TBusBlobStorageConfigRequest());
26722672
request->Record.MutableRequest()->AddCommand()->MutableDefineStoragePool()->CopyFrom(storagePool);
26732673
request->Record.SetDomain(Domain);
2674+
if (SecurityToken) {
2675+
request->Record.SetSecurityToken(SecurityToken);
2676+
}
26742677

26752678
TAutoPtr<NBus::TBusMessage> reply;
26762679
NBus::EMessageStatus msgStatus = SendWhenReady(request, reply);

‎ydb/core/testlib/test_client.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,7 @@ namespace Tests {
291291
FeatureFlags.SetEnableColumnStore(true);
292292
}
293293

294+
TServerSettings() = default;
294295
TServerSettings(const TServerSettings& settings) = default;
295296
TServerSettings& operator=(const TServerSettings& settings) = default;
296297
private:

‎ydb/core/tx/schemeshard/ut_helpers/test_env.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,8 @@ namespace NSchemeShardUT_Private {
7373
OPTION(std::optional<bool>, EnableTopicTransfer, std::nullopt);
7474
OPTION(bool, SetupKqpProxy, false);
7575
OPTION(bool, EnableStrictAclCheck, false);
76+
OPTION(std::optional<bool>, EnableStrictUserManagement, std::nullopt);
77+
OPTION(std::optional<bool>, EnableDatabaseAdmin, std::nullopt);
7678

7779
#undef OPTION
7880
};

‎ydb/core/tx/tx_proxy/proxy_ut_helpers.h

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,20 @@
2323
template<bool OPT> \
2424
void Test##N(NUnitTest::TTestContext&)
2525

26+
#define Y_UNIT_TEST_FLAGS(N, OPT1, OPT2) \
27+
template<bool OPT1, bool OPT2> void N(NUnitTest::TTestContext&); \
28+
struct TTestRegistration##N { \
29+
TTestRegistration##N() { \
30+
TCurrentTest::AddTest(#N, static_cast<void (*)(NUnitTest::TTestContext&)>(&N<false, false>), false); \
31+
TCurrentTest::AddTest(#N "-" #OPT2, static_cast<void (*)(NUnitTest::TTestContext&)>(&N<false, true>), false); \
32+
TCurrentTest::AddTest(#N "-" #OPT1, static_cast<void (*)(NUnitTest::TTestContext&)>(&N<true, false>), false); \
33+
TCurrentTest::AddTest(#N "-" #OPT1 "-" #OPT2, static_cast<void (*)(NUnitTest::TTestContext&)>(&N<true, true>), false); \
34+
} \
35+
}; \
36+
static TTestRegistration##N testRegistration##N; \
37+
template<bool OPT1, bool OPT2> \
38+
void N(NUnitTest::TTestContext&)
39+
2640

2741
namespace NKikimr {
2842
namespace NTxProxyUT {
@@ -112,10 +126,14 @@ class TBaseTestEnv {
112126

113127
class TTestEnv: public TBaseTestEnv {
114128
public:
115-
TTestEnv(ui32 staticNodes = 1, ui32 dynamicNodes = 0)
129+
TTestEnv(ui32 staticNodes = 1, ui32 dynamicNodes = 0, const std::optional<NKikimrConfig::TFeatureFlags>& featureFlags = std::nullopt)
116130
{
117131
Settings = new Tests::TServerSettings(PortManager.GetPort(3534));
118132

133+
if (featureFlags) {
134+
GetSettings().SetFeatureFlags(*featureFlags);
135+
}
136+
119137
GetSettings().SetNodeCount(staticNodes);
120138
GetSettings().SetDynamicNodeCount(dynamicNodes);
121139

0 commit comments

Comments
 (0)