Skip to content

Commit 3827426

Browse files
nshestakovnshestakov
committed
Checking the rights to write to the table when creating and altering … (#18309)
1 parent 7fc930f commit 3827426

File tree

3 files changed

+93
-9
lines changed

3 files changed

+93
-9
lines changed

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8126,8 +8126,8 @@ Y_UNIT_TEST_SUITE(KqpScheme) {
81268126
)", kikimr.GetEndpoint().c_str());
81278127

81288128
const auto result = session.ExecuteSchemeQuery(query).GetValueSync();
8129-
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::GENERIC_ERROR, result.GetIssues().ToString());
8130-
UNIT_ASSERT_STRING_CONTAINS(result.GetIssues().ToOneLineString(), "The transfer destination path '/Root/table_not_exists' not found");
8129+
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SCHEME_ERROR, result.GetIssues().ToString());
8130+
UNIT_ASSERT_STRING_CONTAINS(result.GetIssues().ToOneLineString(), "Path does not exist");
81318131
}
81328132

81338133
// positive
@@ -8400,8 +8400,8 @@ Y_UNIT_TEST_SUITE(KqpScheme) {
84008400
)", kikimr.GetEndpoint().c_str());
84018401

84028402
const auto result = session.ExecuteQuery(query, NYdb::NQuery::TTxControl::NoTx()).ExtractValueSync();
8403-
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::GENERIC_ERROR, result.GetIssues().ToString());
8404-
UNIT_ASSERT_STRING_CONTAINS(result.GetIssues().ToOneLineString(), "The transfer destination path '/Root/table_not_exists' not found");
8403+
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SCHEME_ERROR, result.GetIssues().ToString());
8404+
UNIT_ASSERT_STRING_CONTAINS(result.GetIssues().ToOneLineString(), "Path does not exist");
84058405
}
84068406

84078407
// positive
@@ -8507,7 +8507,7 @@ Y_UNIT_TEST_SUITE(KqpScheme) {
85078507

85088508
const auto result = session.ExecuteSchemeQuery(query).GetValueSync();
85098509
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SCHEME_ERROR, result.GetIssues().ToString());
8510-
UNIT_ASSERT_STRING_CONTAINS(result.GetIssues().ToOneLineString(), "Check failed: path: '/Root/transfer', error: path hasn't been resolved, nearest resolved path: '/Root'");
8510+
UNIT_ASSERT_STRING_CONTAINS(result.GetIssues().ToOneLineString(), "path hasn't been resolved");
85118511
}
85128512

85138513
{
@@ -8865,7 +8865,7 @@ Y_UNIT_TEST_SUITE(KqpScheme) {
88658865

88668866
const auto result = session.ExecuteQuery(query, NYdb::NQuery::TTxControl::NoTx()).ExtractValueSync();
88678867
UNIT_ASSERT_VALUES_EQUAL_C(result.GetStatus(), EStatus::SCHEME_ERROR, result.GetIssues().ToString());
8868-
UNIT_ASSERT_STRING_CONTAINS(result.GetIssues().ToOneLineString(), "Check failed: path: '/Root/transfer', error: path hasn't been resolved, nearest resolved path: '/Root'");
8868+
UNIT_ASSERT_STRING_CONTAINS(result.GetIssues().ToOneLineString(), "path hasn't been resolved");
88698869
}
88708870

88718871
{

‎ydb/core/tx/tx_proxy/schemereq.cpp

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <ydb/core/docapi/traits.h>
1010
#include <ydb/core/protos/flat_scheme_op.pb.h>
1111
#include <ydb/core/protos/schemeshard/operations.pb.h>
12+
#include <ydb/core/protos/replication.pb.h>
1213
#include <ydb/core/tx/schemeshard/schemeshard.h>
1314

1415
#include <ydb/library/login/login.h>
@@ -713,7 +714,6 @@ struct TBaseSchemeReq: public TActorBootstrapped<TDerived> {
713714
case NKikimrSchemeOp::ESchemeOpAlterColumnTable:
714715
case NKikimrSchemeOp::ESchemeOpAlterSequence:
715716
case NKikimrSchemeOp::ESchemeOpAlterReplication:
716-
case NKikimrSchemeOp::ESchemeOpAlterTransfer:
717717
case NKikimrSchemeOp::ESchemeOpAlterBlobDepot:
718718
case NKikimrSchemeOp::ESchemeOpAlterExternalTable:
719719
case NKikimrSchemeOp::ESchemeOpAlterExternalDataSource:
@@ -729,6 +729,24 @@ struct TBaseSchemeReq: public TActorBootstrapped<TDerived> {
729729
ResolveForACL.push_back(toResolve);
730730
break;
731731
}
732+
case NKikimrSchemeOp::ESchemeOpAlterTransfer:
733+
{
734+
auto toResolve = TPathToResolve(pbModifyScheme);
735+
toResolve.Path = Merge(workingDir, SplitPath(GetPathNameForScheme(pbModifyScheme)));
736+
toResolve.RequireAccess = NACLib::EAccessRights::AlterSchema | accessToUserAttrs;
737+
ResolveForACL.push_back(toResolve);
738+
739+
auto& config = pbModifyScheme.GetReplication().GetConfig();
740+
auto& target = config.GetTransferSpecific().GetTarget();
741+
if (target.HasDstPath()) {
742+
auto toWriteTable = TPathToResolve(pbModifyScheme);
743+
toWriteTable.Path = SplitPath(target.GetDstPath());
744+
toWriteTable.RequireAccess = NACLib::EAccessRights::UpdateRow;
745+
ResolveForACL.push_back(toWriteTable);
746+
}
747+
748+
break;
749+
}
732750
case NKikimrSchemeOp::ESchemeOpRestoreMultipleIncrementalBackups:
733751
{
734752
auto toResolve = TPathToResolve(pbModifyScheme);
@@ -813,7 +831,6 @@ struct TBaseSchemeReq: public TActorBootstrapped<TDerived> {
813831
case NKikimrSchemeOp::ESchemeOpCreateSolomonVolume:
814832
case NKikimrSchemeOp::ESchemeOpCreateSequence:
815833
case NKikimrSchemeOp::ESchemeOpCreateReplication:
816-
case NKikimrSchemeOp::ESchemeOpCreateTransfer:
817834
case NKikimrSchemeOp::ESchemeOpCreateBlobDepot:
818835
case NKikimrSchemeOp::ESchemeOpCreateExternalTable:
819836
case NKikimrSchemeOp::ESchemeOpCreateExternalDataSource:
@@ -827,6 +844,22 @@ struct TBaseSchemeReq: public TActorBootstrapped<TDerived> {
827844
ResolveForACL.push_back(toResolve);
828845
break;
829846
}
847+
case NKikimrSchemeOp::ESchemeOpCreateTransfer:
848+
{
849+
auto toResolve = TPathToResolve(pbModifyScheme);
850+
toResolve.Path = workingDir;
851+
toResolve.RequireAccess = NACLib::EAccessRights::CreateTable | accessToUserAttrs;
852+
ResolveForACL.push_back(toResolve);
853+
854+
auto& config = pbModifyScheme.GetReplication().GetConfig();
855+
auto& target = config.GetTransferSpecific().GetTarget();
856+
auto toWriteTable = TPathToResolve(pbModifyScheme);
857+
toWriteTable.Path = SplitPath(target.GetDstPath());
858+
toWriteTable.RequireAccess = NACLib::EAccessRights::UpdateRow;
859+
ResolveForACL.push_back(toWriteTable);
860+
861+
break;
862+
}
830863
case NKikimrSchemeOp::ESchemeOpCreateConsistentCopyTables: {
831864
for (auto& item: pbModifyScheme.GetCreateConsistentCopyTables().GetCopyTableDescriptions()) {
832865
{

‎ydb/tests/functional/replication/transfer.cpp

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,60 @@ Y_UNIT_TEST_SUITE(Transfer)
4545
|>
4646
];
4747
};
48-
)", MainTestCase::CreateTransferSettings::WithExpectedError(TStringBuilder() << "The transfer destination path '/local/" << testCase.TableName << "' not found"));
48+
)", MainTestCase::CreateTransferSettings::WithExpectedError(TStringBuilder() << "Path does not exist"));
4949

5050
testCase.DropTopic();
5151
}
52+
53+
void CheckPermissionOnCreateCase(const std::vector<std::string> permissions, bool success) {
54+
auto id = RandomNumber<ui16>();
55+
auto username = TStringBuilder() << "u" << id;
56+
57+
MainTestCase permissionSetup;
58+
permissionSetup.ExecuteDDL(Sprintf(R"(
59+
CREATE USER %s
60+
)", username.data()));
61+
permissionSetup.Grant("", username, {"ydb.granular.create_table", "ydb.granular.create_queue"});
62+
63+
MainTestCase testCase(username);
64+
permissionSetup.ExecuteDDL(Sprintf(R"(
65+
CREATE TABLE `%s` (
66+
Key Uint64 NOT NULL,
67+
Message Utf8,
68+
PRIMARY KEY (Key)
69+
) WITH (
70+
STORE = COLUMN
71+
);
72+
)", testCase.TableName.data()));
73+
if (!permissions.empty()) {
74+
permissionSetup.Grant(testCase.TableName, username, permissions);
75+
}
76+
77+
testCase.CreateTopic(1);
78+
permissionSetup.Grant(testCase.TopicName, username, {"ALL"});
79+
80+
testCase.CreateTransfer(R"(
81+
$l = ($x) -> {
82+
return [
83+
<|
84+
Key:CAST($x._offset AS Uint64),
85+
Message:CAST($x._data AS Utf8)
86+
|>
87+
];
88+
};
89+
)", MainTestCase::CreateTransferSettings::WithExpectedError(success ? "The transfer is only available in the Enterprise version" : "Access denied for scheme request"));
90+
91+
testCase.DropTopic();
92+
}
93+
94+
Y_UNIT_TEST(Create_WithPermission)
95+
{
96+
CheckPermissionOnCreateCase({"ydb.generic.write", "ydb.generic.read"}, true);
97+
}
98+
99+
Y_UNIT_TEST(Create_WithoutTablePermission)
100+
{
101+
CheckPermissionOnCreateCase({"ydb.generic.read"}, false);
102+
}
52103
}
53104

0 commit comments

Comments
 (0)