From b2bd261fa1d959b6a3b497c2b2acd17cc998fd9d Mon Sep 17 00:00:00 2001 From: kungasc Date: Wed, 29 Jan 2025 17:24:58 +0300 Subject: [PATCH 1/7] navigate uses table range filter --- ydb/core/sys_view/auth/auth_scan_base.h | 72 ++++++++++++++++++++---- ydb/core/sys_view/auth/group_members.cpp | 2 +- ydb/core/sys_view/auth/groups.cpp | 2 +- ydb/core/sys_view/auth/owners.cpp | 2 +- ydb/core/sys_view/auth/permissions.cpp | 2 +- ydb/core/sys_view/ut_kqp.cpp | 42 ++++++++++++++ 6 files changed, 106 insertions(+), 16 deletions(-) diff --git a/ydb/core/sys_view/auth/auth_scan_base.h b/ydb/core/sys_view/auth/auth_scan_base.h index a0363601360e..87a7d1d33d62 100644 --- a/ydb/core/sys_view/auth/auth_scan_base.h +++ b/ydb/core/sys_view/auth/auth_scan_base.h @@ -51,11 +51,19 @@ class TAuthScanBase : public TScanActorBase { TAuthScanBase(const NActors::TActorId& ownerId, ui32 scanId, const TTableId& tableId, const TTableRange& tableRange, const TArrayRef& columns, TIntrusiveConstPtr userToken, - bool requireUserAdministratorAccess) + bool requireUserAdministratorAccess, bool applyPathTableRange) : TBase(ownerId, scanId, tableId, tableRange, columns) , UserToken(std::move(userToken)) , RequireUserAdministratorAccess(requireUserAdministratorAccess) { + if (applyPathTableRange) { + if (auto cellsFrom = TBase::TableRange.From.GetCells(); cellsFrom.size() > 0 && !cellsFrom[0].IsNull()) { + PathFrom = cellsFrom[0].AsBuf(); + } + if (auto cellsTo = TBase::TableRange.To.GetCells(); cellsTo.size() > 0 && !cellsTo[0].IsNull()) { + PathTo = cellsTo[0].AsBuf(); + } + } } STFUNC(StateScan) { @@ -81,16 +89,6 @@ class TAuthScanBase : public TScanActorBase { return; } - // TODO: support TableRange filter - if (auto cellsFrom = TBase::TableRange.From.GetCells(); cellsFrom.size() > 0 && !cellsFrom[0].IsNull()) { - TBase::ReplyErrorAndDie(Ydb::StatusIds::INTERNAL_ERROR, TStringBuilder() << "TableRange.From filter is not supported"); - return; - } - if (auto cellsTo = TBase::TableRange.To.GetCells(); cellsTo.size() > 0 && !cellsTo[0].IsNull()) { - TBase::ReplyErrorAndDie(Ydb::StatusIds::INTERNAL_ERROR, TStringBuilder() << "TableRange.To filter is not supported"); - return; - } - auto& last = DeepFirstSearchStack.emplace_back(); last.Index = Max(); // tenant root @@ -108,6 +106,10 @@ class TAuthScanBase : public TScanActorBase { auto& last = DeepFirstSearchStack.back(); if (last.Index == Max()) { // tenant root + if ((PathFrom || PathTo) && ShouldSkipPath(TBase::TenantName)) { + DeepFirstSearchStack.pop_back(); + continue; + } NavigatePath(SplitPath(TBase::TenantName)); DeepFirstSearchStack.pop_back(); return; @@ -120,8 +122,13 @@ class TAuthScanBase : public TScanActorBase { if (child.Kind == TSchemeCacheNavigate::KindExtSubdomain || child.Kind == TSchemeCacheNavigate::KindSubdomain) { continue; } - + last.Entry.Path.push_back(child.Name); + if ((PathFrom || PathTo) && ShouldSkipPath(CanonizePath(last.Entry.Path))) { + last.Entry.Path.pop_back(); + continue; + } + NavigatePath(last.Entry.Path); last.Entry.Path.pop_back(); return; @@ -189,6 +196,46 @@ class TAuthScanBase : public TScanActorBase { TBase::Send(MakeSchemeCacheID(), new TEvTxProxySchemeCache::TEvNavigateKeySet(request.Release())); } + + // this method only skip foolproof useless paths + // ignores from/to inclusive flags for simplicity + // ignores some boundary cases for simplicity + // precise check will be performed later on batch rows filtering + bool ShouldSkipPath(const TString& path) { + Y_DEBUG_ABORT_UNLESS(PathFrom || PathTo); + + if (PathFrom) { + // example: + // PathFrom = "Dir2/SubDir2" + // skip: + // - "Dir1" + // - "Dir2/SubDir1" + // do not skip: + // - "Dir2" + // - "Dir3" + + if (PathFrom > path && !PathFrom->StartsWith(path)) { + return true; + } + } + + if (PathTo) { + // example: + // PathTo = "Dir2/SubDir2" + // skip: + // - "Dir3" + // - "Dir2/SubDir3" + // do not skip: + // - "Dir1" + // - "Dir2" + + if (PathTo < path) { + return true; + } + } + + return false; + } virtual void FillBatch(NKqp::TEvKqpCompute::TEvScanData& batch, const TNavigate::TEntry& entry) = 0; @@ -197,6 +244,7 @@ class TAuthScanBase : public TScanActorBase { private: bool RequireUserAdministratorAccess; + std::optional PathFrom, PathTo; TVector DeepFirstSearchStack; }; diff --git a/ydb/core/sys_view/auth/group_members.cpp b/ydb/core/sys_view/auth/group_members.cpp index da290a40ec7f..7858c4aea4d7 100644 --- a/ydb/core/sys_view/auth/group_members.cpp +++ b/ydb/core/sys_view/auth/group_members.cpp @@ -22,7 +22,7 @@ class TGroupMembersScan : public TAuthScanBase { TGroupMembersScan(const NActors::TActorId& ownerId, ui32 scanId, const TTableId& tableId, const TTableRange& tableRange, const TArrayRef& columns, TIntrusiveConstPtr userToken) - : TAuthBase(ownerId, scanId, tableId, tableRange, columns, std::move(userToken), true) + : TAuthBase(ownerId, scanId, tableId, tableRange, columns, std::move(userToken), true, false) { } diff --git a/ydb/core/sys_view/auth/groups.cpp b/ydb/core/sys_view/auth/groups.cpp index d5add729896f..61abea6d01cb 100644 --- a/ydb/core/sys_view/auth/groups.cpp +++ b/ydb/core/sys_view/auth/groups.cpp @@ -22,7 +22,7 @@ class TGroupsScan : public TAuthScanBase { TGroupsScan(const NActors::TActorId& ownerId, ui32 scanId, const TTableId& tableId, const TTableRange& tableRange, const TArrayRef& columns, TIntrusiveConstPtr userToken) - : TAuthBase(ownerId, scanId, tableId, tableRange, columns, std::move(userToken), true) + : TAuthBase(ownerId, scanId, tableId, tableRange, columns, std::move(userToken), true, false) { } diff --git a/ydb/core/sys_view/auth/owners.cpp b/ydb/core/sys_view/auth/owners.cpp index 7d9b1182fea2..293e89e431e7 100644 --- a/ydb/core/sys_view/auth/owners.cpp +++ b/ydb/core/sys_view/auth/owners.cpp @@ -22,7 +22,7 @@ class TOwnersScan : public TAuthScanBase { TOwnersScan(const NActors::TActorId& ownerId, ui32 scanId, const TTableId& tableId, const TTableRange& tableRange, const TArrayRef& columns, TIntrusiveConstPtr userToken) - : TAuthBase(ownerId, scanId, tableId, tableRange, columns, std::move(userToken), false) + : TAuthBase(ownerId, scanId, tableId, tableRange, columns, std::move(userToken), false, true) { } diff --git a/ydb/core/sys_view/auth/permissions.cpp b/ydb/core/sys_view/auth/permissions.cpp index 30fb45653f67..7b9f75ce0faf 100644 --- a/ydb/core/sys_view/auth/permissions.cpp +++ b/ydb/core/sys_view/auth/permissions.cpp @@ -23,7 +23,7 @@ class TPermissionsScan : public TAuthScanBase { TPermissionsScan(bool effective, const NActors::TActorId& ownerId, ui32 scanId, const TTableId& tableId, const TTableRange& tableRange, const TArrayRef& columns, TIntrusiveConstPtr userToken) - : TAuthBase(ownerId, scanId, tableId, tableRange, columns, std::move(userToken), false) + : TAuthBase(ownerId, scanId, tableId, tableRange, columns, std::move(userToken), false, true) , Effective(effective) { } diff --git a/ydb/core/sys_view/ut_kqp.cpp b/ydb/core/sys_view/ut_kqp.cpp index 434adf075113..b3924eadef00 100644 --- a/ydb/core/sys_view/ut_kqp.cpp +++ b/ydb/core/sys_view/ut_kqp.cpp @@ -3038,6 +3038,48 @@ Y_UNIT_TEST_SUITE(SystemView) { NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); } + Y_UNIT_TEST(AuthOwners_TableRange) { + TTestEnv env; + SetupAuthEnvironment(env); + TTableClient client(env.GetDriver()); + + for (auto path : { + "Dir0/SubDir0", + "Dir0/SubDir1", + "Dir0/SubDir2", + "Dir1/SubDir0", + "Dir1/SubDir1", + "Dir1/SubDir2", + "Dir2/SubDir0", + "Dir2/SubDir1", + "Dir2/SubDir2", + "Dir3/SubDir0", + "Dir3/SubDir1", + "Dir3/SubDir2", + }) { + env.GetClient().MkDir("/Root", path); + } + env.GetClient().CreateUser("/Root", "user1", "password1"); + env.GetClient().ModifyOwner("/Root/Dir1", "SubDir1", "user1"); + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_owners` + WHERE Path >= "/Root/Dir1/SubDir1" AND Path < "/Root/Dir2/SubDir1" + )").GetValueSync(); + + auto expected = R"([ + [["/Root/Dir1/SubDir1"];["user1"]]; + [["/Root/Dir1/SubDir2"];["root@builtin"]]; + [["/Root/Dir2"];["root@builtin"]]; + [["/Root/Dir2/SubDir0"];["root@builtin"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + } + Y_UNIT_TEST(AuthPermissions) { TTestEnv env; SetupAuthEnvironment(env); From ff8d9c08e0e20e80d6af370c38d2c7a1295c7fab Mon Sep 17 00:00:00 2001 From: kungasc Date: Thu, 30 Jan 2025 16:46:32 +0300 Subject: [PATCH 2/7] print message --- ydb/core/kqp/ut/common/kqp_ut_common.cpp | 8 ++++---- ydb/core/kqp/ut/common/kqp_ut_common.h | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ydb/core/kqp/ut/common/kqp_ut_common.cpp b/ydb/core/kqp/ut/common/kqp_ut_common.cpp index 4313192b6b88..86ab759c81c0 100644 --- a/ydb/core/kqp/ut/common/kqp_ut_common.cpp +++ b/ydb/core/kqp/ut/common/kqp_ut_common.cpp @@ -552,11 +552,11 @@ TString ReformatYson(const TString& yson) { return output.Str(); } -void CompareYson(const TString& expected, const TString& actual) { - UNIT_ASSERT_VALUES_EQUAL(ReformatYson(expected), ReformatYson(actual)); +void CompareYson(const TString& expected, const TString& actual, const TString& message) { + UNIT_ASSERT_VALUES_EQUAL_C(ReformatYson(expected), ReformatYson(actual), message); } -void CompareYson(const TString& expected, const NKikimrMiniKQL::TResult& actual) { +void CompareYson(const TString& expected, const NKikimrMiniKQL::TResult& actual, const TString& message) { TStringStream ysonStream; NYson::TYsonWriter writer(&ysonStream, NYson::EYsonFormat::Text); NYql::IDataProvider::TFillSettings fillSettings; @@ -564,7 +564,7 @@ void CompareYson(const TString& expected, const NKikimrMiniKQL::TResult& actual) KikimrResultToYson(ysonStream, writer, actual, {}, fillSettings, truncated); UNIT_ASSERT(!truncated); - CompareYson(expected, ysonStream.Str()); + CompareYson(expected, ysonStream.Str(), message); } bool HasIssue(const NYql::TIssues& issues, ui32 code, diff --git a/ydb/core/kqp/ut/common/kqp_ut_common.h b/ydb/core/kqp/ut/common/kqp_ut_common.h index f9b5703fdeaf..00d32391c7ed 100644 --- a/ydb/core/kqp/ut/common/kqp_ut_common.h +++ b/ydb/core/kqp/ut/common/kqp_ut_common.h @@ -296,8 +296,8 @@ inline constexpr TStringBuf IndexWithSqlString(EIndexTypeSql type) { } TString ReformatYson(const TString& yson); -void CompareYson(const TString& expected, const TString& actual); -void CompareYson(const TString& expected, const NKikimrMiniKQL::TResult& actual); +void CompareYson(const TString& expected, const TString& actual, const TString& message = {}); +void CompareYson(const TString& expected, const NKikimrMiniKQL::TResult& actual, const TString& message = {}); void CreateLargeTable(TKikimrRunner& kikimr, ui32 rowsPerShard, ui32 keyTextSize, ui32 dataTextSize, ui32 batchSizeRows = 100, ui32 fillShardsCount = 8, ui32 largeTableKeysPerShard = 1000000); From 4ba28de4039dc5c1913bb4e78b5d256b2653d59b Mon Sep 17 00:00:00 2001 From: kungasc Date: Thu, 30 Jan 2025 16:48:09 +0300 Subject: [PATCH 3/7] + tests --- ydb/core/sys_view/ut_kqp.cpp | 408 ++++++++++++++++++++++++++++++++--- 1 file changed, 383 insertions(+), 25 deletions(-) diff --git a/ydb/core/sys_view/ut_kqp.cpp b/ydb/core/sys_view/ut_kqp.cpp index b3924eadef00..9827b73e0762 100644 --- a/ydb/core/sys_view/ut_kqp.cpp +++ b/ydb/core/sys_view/ut_kqp.cpp @@ -2376,6 +2376,128 @@ Y_UNIT_TEST_SUITE(SystemView) { NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); } + Y_UNIT_TEST(AuthUsers_TableRange) { + TTestEnv env; + SetupAuthEnvironment(env); + TTableClient client(env.GetDriver()); + + for (auto user : { + "user1", + "user2", + "user3", + "user4" + }) { + env.GetClient().CreateUser("/Root", user, "password"); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT Sid + FROM `Root/.sys/auth_users` + )").GetValueSync(); + + auto expected = R"([ + [["user1"]]; + [["user2"]]; + [["user3"]]; + [["user4"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT Sid + FROM `Root/.sys/auth_users` + WHERE Sid >= "user2" + )").GetValueSync(); + + auto expected = R"([ + [["user2"]]; + [["user3"]]; + [["user4"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT Sid + FROM `Root/.sys/auth_users` + WHERE Sid > "user2" + )").GetValueSync(); + + auto expected = R"([ + [["user3"]]; + [["user4"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT Sid + FROM `Root/.sys/auth_users` + WHERE Sid <= "user3" + )").GetValueSync(); + + auto expected = R"([ + [["user1"]]; + [["user2"]]; + [["user3"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT Sid + FROM `Root/.sys/auth_users` + WHERE Sid < "user3" + )").GetValueSync(); + + auto expected = R"([ + [["user1"]]; + [["user2"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT Sid + FROM `Root/.sys/auth_users` + WHERE Sid > "user1" AND Sid <= "user3" + )").GetValueSync(); + + auto expected = R"([ + [["user2"]]; + [["user3"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT Sid + FROM `Root/.sys/auth_users` + WHERE Sid >= "user2" AND Sid < "user3" + )").GetValueSync(); + + auto expected = R"([ + [["user2"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + } + Y_UNIT_TEST(AuthGroups) { TTestEnv env; SetupAuthEnvironment(env); @@ -3059,25 +3181,277 @@ Y_UNIT_TEST_SUITE(SystemView) { }) { env.GetClient().MkDir("/Root", path); } + env.GetClient().CreateUser("/Root", "user0", "password0"); env.GetClient().CreateUser("/Root", "user1", "password1"); + env.GetClient().CreateUser("/Root", "user2", "password2"); + env.GetClient().ModifyOwner("/Root/Dir1", "SubDir0", "user0"); env.GetClient().ModifyOwner("/Root/Dir1", "SubDir1", "user1"); + env.GetClient().ModifyOwner("/Root/Dir1", "SubDir2", "user2"); { auto it = client.StreamExecuteScanQuery(R"( SELECT * FROM `Root/.sys/auth_owners` - WHERE Path >= "/Root/Dir1/SubDir1" AND Path < "/Root/Dir2/SubDir1" )").GetValueSync(); auto expected = R"([ + [["/Root"];["root@builtin"]]; + [["/Root/.metadata"];["metadata@system"]]; + [["/Root/.metadata/workload_manager"];["metadata@system"]]; + [["/Root/.metadata/workload_manager/pools"];["metadata@system"]]; + [["/Root/.metadata/workload_manager/pools/default"];["metadata@system"]]; + [["/Root/Dir0"];["root@builtin"]]; + [["/Root/Dir0/SubDir0"];["root@builtin"]]; + [["/Root/Dir0/SubDir1"];["root@builtin"]]; + [["/Root/Dir0/SubDir2"];["root@builtin"]]; + [["/Root/Dir1"];["root@builtin"]]; + [["/Root/Dir1/SubDir0"];["user0"]]; [["/Root/Dir1/SubDir1"];["user1"]]; - [["/Root/Dir1/SubDir2"];["root@builtin"]]; + [["/Root/Dir1/SubDir2"];["user2"]]; + [["/Root/Dir2"];["root@builtin"]]; + [["/Root/Dir2/SubDir0"];["root@builtin"]]; + [["/Root/Dir2/SubDir1"];["root@builtin"]]; + [["/Root/Dir2/SubDir2"];["root@builtin"]]; + [["/Root/Dir3"];["root@builtin"]]; + [["/Root/Dir3/SubDir0"];["root@builtin"]]; + [["/Root/Dir3/SubDir1"];["root@builtin"]]; + [["/Root/Dir3/SubDir2"];["root@builtin"]]; + [["/Root/Table0"];["root@builtin"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_owners` + WHERE Path >= "/A" AND Path <= "/Z" + )").GetValueSync(); + + auto expected = R"([ + [["/Root"];["root@builtin"]]; + [["/Root/.metadata"];["metadata@system"]]; + [["/Root/.metadata/workload_manager"];["metadata@system"]]; + [["/Root/.metadata/workload_manager/pools"];["metadata@system"]]; + [["/Root/.metadata/workload_manager/pools/default"];["metadata@system"]]; + [["/Root/Dir0"];["root@builtin"]]; + [["/Root/Dir0/SubDir0"];["root@builtin"]]; + [["/Root/Dir0/SubDir1"];["root@builtin"]]; + [["/Root/Dir0/SubDir2"];["root@builtin"]]; + [["/Root/Dir1"];["root@builtin"]]; + [["/Root/Dir1/SubDir0"];["user0"]]; + [["/Root/Dir1/SubDir1"];["user1"]]; + [["/Root/Dir1/SubDir2"];["user2"]]; + [["/Root/Dir2"];["root@builtin"]]; + [["/Root/Dir2/SubDir0"];["root@builtin"]]; + [["/Root/Dir2/SubDir1"];["root@builtin"]]; + [["/Root/Dir2/SubDir2"];["root@builtin"]]; + [["/Root/Dir3"];["root@builtin"]]; + [["/Root/Dir3/SubDir0"];["root@builtin"]]; + [["/Root/Dir3/SubDir1"];["root@builtin"]]; + [["/Root/Dir3/SubDir2"];["root@builtin"]]; + [["/Root/Table0"];["root@builtin"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_owners` + WHERE Path >= "/Root/Dir1" AND Path < "/Root/Dir3" + )").GetValueSync(); + + auto expected = R"([ + [["/Root/Dir1"];["root@builtin"]]; + [["/Root/Dir1/SubDir0"];["user0"]]; + [["/Root/Dir1/SubDir1"];["user1"]]; + [["/Root/Dir1/SubDir2"];["user2"]]; + [["/Root/Dir2"];["root@builtin"]]; + [["/Root/Dir2/SubDir0"];["root@builtin"]]; + [["/Root/Dir2/SubDir1"];["root@builtin"]]; + [["/Root/Dir2/SubDir2"];["root@builtin"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_owners` + WHERE Path >= "/Root/Dir1/SubDir1" AND Path <= "/Root/Dir2/SubDir1" + )").GetValueSync(); + + auto expected = R"([ + [["/Root/Dir1/SubDir1"];["user1"]]; + [["/Root/Dir1/SubDir2"];["user2"]]; + [["/Root/Dir2"];["root@builtin"]]; + [["/Root/Dir2/SubDir0"];["root@builtin"]]; + [["/Root/Dir2/SubDir1"];["root@builtin"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_owners` + WHERE Path > "/Root/Dir1/SubDir1" AND Path < "/Root/Dir2/SubDir1" + )").GetValueSync(); + + auto expected = R"([ + [["/Root/Dir1/SubDir2"];["user2"]]; [["/Root/Dir2"];["root@builtin"]]; [["/Root/Dir2/SubDir0"];["root@builtin"]]; ])"; NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_owners` + WHERE Path = "/Root/Dir1/SubDir1" + )").GetValueSync(); + + auto expected = R"([ + [["/Root/Dir1/SubDir1"];["user1"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_owners` + WHERE Path >= "/Root/Dir1/SubDir0" AND Sid >= "user1" AND Path < "/Root/Dir2" + )").GetValueSync(); + + auto expected = R"([ + [["/Root/Dir1/SubDir1"];["user1"]]; + [["/Root/Dir1/SubDir2"];["user2"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_owners` + WHERE Path = "/Root/Dir1/SubDir1" AND Sid > "user0" + )").GetValueSync(); + + auto expected = R"([ + [["/Root/Dir1/SubDir1"];["user1"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_owners` + WHERE Path = "/Root/Dir1/SubDir1" AND Sid < "user2" + )").GetValueSync(); + + auto expected = R"([ + [["/Root/Dir1/SubDir1"];["user1"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_owners` + WHERE Path = "/Root/Dir1/SubDir1" AND Sid >= "user1" + )").GetValueSync(); + + auto expected = R"([ + [["/Root/Dir1/SubDir1"];["user1"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_owners` + WHERE Path = "/Root/Dir1/SubDir1" AND Sid <= "user1" + )").GetValueSync(); + + auto expected = R"([ + [["/Root/Dir1/SubDir1"];["user1"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_owners` + WHERE Path = "/Root/Dir1/SubDir1" AND Sid > "user1" + )").GetValueSync(); + + auto expected = R"([ + + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_owners` + WHERE Path = "/Root/Dir1/SubDir1" AND Sid < "user1" + )").GetValueSync(); + + auto expected = R"([ + + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_owners` + WHERE Path = "/Root/Dir1/SubDir1" AND Sid = "user1" + )").GetValueSync(); + + auto expected = R"([ + [["/Root/Dir1/SubDir1"];["user1"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT Sid, Path + FROM `Root/.sys/auth_owners` + WHERE Path = "/Root/Dir1/SubDir1" AND Sid >= "user1" + )").GetValueSync(); + + auto expected = R"([ + [["user1"];["/Root/Dir1/SubDir1"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } } Y_UNIT_TEST(AuthPermissions) { @@ -3494,26 +3868,12 @@ Y_UNIT_TEST_SUITE(SystemView) { WHERE Path = "/Root/Dir1" )").GetValueSync(); - // TODO: - NKqp::StreamResultToYson(it, false, EStatus::INTERNAL_ERROR, "TableRange.From filter is not supported"); - - // auto expected = R"([ - // [["/Root/Dir1"];["ydb.generic.use"];["user1"]]; - // [["/Root/Dir1"];["ydb.granular.select_row"];["user2"]]; - // ])"; - - // NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); - } - - { - auto it = client.StreamExecuteScanQuery(R"( - SELECT * - FROM `Root/.sys/auth_effective_permissions` - WHERE Path = "/Root/Dir1" - )").GetValueSync(); + auto expected = R"([ + [["/Root/Dir1"];["ydb.generic.use"];["user1"]]; + [["/Root/Dir1"];["ydb.granular.select_row"];["user2"]]; + ])"; - // TODO: - NKqp::StreamResultToYson(it, false, EStatus::INTERNAL_ERROR, "TableRange.From filter is not supported"); + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); } { @@ -3535,15 +3895,13 @@ Y_UNIT_TEST_SUITE(SystemView) { { auto it = client.StreamExecuteScanQuery(R"( SELECT * - FROM `Root/.sys/auth_effective_permissions` - WHERE Sid = "user2" + FROM `Root/.sys/auth_permissions` + WHERE Path = "/Root/Dir1/SubDir1" AND Sid >= "user2" )").GetValueSync(); auto expected = R"([ - [["/Root/Dir1"];["ydb.granular.select_row"];["user2"]]; [["/Root/Dir1/SubDir1"];["ydb.granular.erase_row"];["user2"]]; [["/Root/Dir1/SubDir1"];["ydb.granular.select_row"];["user2"]]; - [["/Root/Dir1/SubDir2"];["ydb.granular.select_row"];["user2"]]; ])"; NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); From a255f5ce2d22560ced9a8dc5285010719cccd234 Mon Sep 17 00:00:00 2001 From: kungasc Date: Thu, 30 Jan 2025 17:22:23 +0300 Subject: [PATCH 4/7] owners & users table range --- ydb/core/sys_view/auth/auth_scan_base.h | 14 +++--- ydb/core/sys_view/auth/owners.cpp | 44 ++++++++++++------- ydb/core/sys_view/auth/users.cpp | 27 ++++++------ .../sys_view/common/scan_actor_base_impl.h | 15 +++++++ 4 files changed, 64 insertions(+), 36 deletions(-) diff --git a/ydb/core/sys_view/auth/auth_scan_base.h b/ydb/core/sys_view/auth/auth_scan_base.h index 87a7d1d33d62..997fe50978ac 100644 --- a/ydb/core/sys_view/auth/auth_scan_base.h +++ b/ydb/core/sys_view/auth/auth_scan_base.h @@ -57,11 +57,11 @@ class TAuthScanBase : public TScanActorBase { , RequireUserAdministratorAccess(requireUserAdministratorAccess) { if (applyPathTableRange) { - if (auto cellsFrom = TBase::TableRange.From.GetCells(); cellsFrom.size() > 0 && !cellsFrom[0].IsNull()) { - PathFrom = cellsFrom[0].AsBuf(); + if (auto cell = TBase::GetCellFrom(0)) { + PathFrom = cell->AsBuf(); } - if (auto cellsTo = TBase::TableRange.To.GetCells(); cellsTo.size() > 0 && !cellsTo[0].IsNull()) { - PathTo = cellsTo[0].AsBuf(); + if (auto cell = TBase::GetCellTo(0)) { + PathTo = cell->AsBuf(); } } } @@ -106,7 +106,7 @@ class TAuthScanBase : public TScanActorBase { auto& last = DeepFirstSearchStack.back(); if (last.Index == Max()) { // tenant root - if ((PathFrom || PathTo) && ShouldSkipPath(TBase::TenantName)) { + if ((PathFrom || PathTo) && ShouldSkipSubTree(TBase::TenantName)) { DeepFirstSearchStack.pop_back(); continue; } @@ -124,7 +124,7 @@ class TAuthScanBase : public TScanActorBase { } last.Entry.Path.push_back(child.Name); - if ((PathFrom || PathTo) && ShouldSkipPath(CanonizePath(last.Entry.Path))) { + if ((PathFrom || PathTo) && ShouldSkipSubTree(CanonizePath(last.Entry.Path))) { last.Entry.Path.pop_back(); continue; } @@ -201,7 +201,7 @@ class TAuthScanBase : public TScanActorBase { // ignores from/to inclusive flags for simplicity // ignores some boundary cases for simplicity // precise check will be performed later on batch rows filtering - bool ShouldSkipPath(const TString& path) { + bool ShouldSkipSubTree(const TString& path) { Y_DEBUG_ABORT_UNLESS(PathFrom || PathTo); if (PathFrom) { diff --git a/ydb/core/sys_view/auth/owners.cpp b/ydb/core/sys_view/auth/owners.cpp index 293e89e431e7..2a8d7f875254 100644 --- a/ydb/core/sys_view/auth/owners.cpp +++ b/ydb/core/sys_view/auth/owners.cpp @@ -34,26 +34,38 @@ class TOwnersScan : public TAuthScanBase { auto entryPath = CanonizePath(entry.Path); - for (auto& column : Columns) { - switch (column.Tag) { - case Schema::AuthOwners::Path::ColumnId: - cells.push_back(TCell(entryPath.data(), entryPath.size())); - break; - case Schema::AuthOwners::Sid::ColumnId: - if (entry.SecurityObject && entry.SecurityObject->HasOwnerSID()) { - cells.push_back(TCell(entry.SecurityObject->GetOwnerSID().data(), entry.SecurityObject->GetOwnerSID().size())); - } else { + bool isInRange = true; + if (auto pathFrom = GetCellFrom(0); pathFrom) { + int cmp = pathFrom->AsBuf().compare(entryPath); + isInRange &= cmp < 0 || cmp == 0 && TableRange.FromInclusive; + } + if (auto pathTo = GetCellTo(0); pathTo) { + int cmp = pathTo->AsBuf().compare(entryPath); + isInRange &= cmp > 0 || cmp == 0 && TableRange.ToInclusive; + } + + if (isInRange) { + for (auto& column : Columns) { + switch (column.Tag) { + case Schema::AuthOwners::Path::ColumnId: + cells.push_back(TCell(entryPath.data(), entryPath.size())); + break; + case Schema::AuthOwners::Sid::ColumnId: + if (entry.SecurityObject && entry.SecurityObject->HasOwnerSID()) { + cells.push_back(TCell(entry.SecurityObject->GetOwnerSID().data(), entry.SecurityObject->GetOwnerSID().size())); + } else { + cells.emplace_back(); + } + break; + default: cells.emplace_back(); } - break; - default: - cells.emplace_back(); } - } - TArrayRef ref(cells); - batch.Rows.emplace_back(TOwnedCellVec::Make(ref)); - cells.clear(); + TArrayRef ref(cells); + batch.Rows.emplace_back(TOwnedCellVec::Make(ref)); + cells.clear(); + } batch.Finished = false; } diff --git a/ydb/core/sys_view/auth/users.cpp b/ydb/core/sys_view/auth/users.cpp index 263ac0bd52e6..a20a9e35999c 100644 --- a/ydb/core/sys_view/auth/users.cpp +++ b/ydb/core/sys_view/auth/users.cpp @@ -54,16 +54,6 @@ class TUsersScan : public TScanActorBase { } void StartScan() { - // TODO: support TableRange filter - if (auto cellsFrom = TBase::TableRange.From.GetCells(); cellsFrom.size() > 0 && !cellsFrom[0].IsNull()) { - TBase::ReplyErrorAndDie(Ydb::StatusIds::INTERNAL_ERROR, TStringBuilder() << "TableRange.From filter is not supported"); - return; - } - if (auto cellsTo = TBase::TableRange.To.GetCells(); cellsTo.size() > 0 && !cellsTo[0].IsNull()) { - TBase::ReplyErrorAndDie(Ydb::StatusIds::INTERNAL_ERROR, TStringBuilder() << "TableRange.To filter is not supported"); - return; - } - auto request = MakeHolder(); LOG_TRACE_S(TlsActivationContext->AsActorContext(), NKikimrServices::SYSTEM_VIEWS, @@ -108,12 +98,23 @@ class TUsersScan : public TScanActorBase { TVector cells(::Reserve(Columns.size())); for (const auto* user : users) { + bool isInRange = true; + if (auto pathFrom = GetCellFrom(0); pathFrom) { + int cmp = pathFrom->AsBuf().compare(user->GetName()); + isInRange &= cmp < 0 || cmp == 0 && TableRange.FromInclusive; + } + if (auto pathTo = GetCellTo(0); pathTo) { + int cmp = pathTo->AsBuf().compare(user->GetName()); + isInRange &= cmp > 0 || cmp == 0 && TableRange.ToInclusive; + } + if (!isInRange) { + continue; + } + for (auto& column : Columns) { switch (column.Tag) { case Schema::AuthUsers::Sid::ColumnId: - cells.push_back(user->HasName() - ? TCell(user->GetName().data(), user->GetName().size()) - : TCell()); + cells.push_back(TCell(user->GetName().data(), user->GetName().size())); break; case Schema::AuthUsers::IsEnabled::ColumnId: cells.push_back(user->HasIsEnabled() diff --git a/ydb/core/sys_view/common/scan_actor_base_impl.h b/ydb/core/sys_view/common/scan_actor_base_impl.h index c126bdbb8bd9..fdfff0c6d39d 100644 --- a/ydb/core/sys_view/common/scan_actor_base_impl.h +++ b/ydb/core/sys_view/common/scan_actor_base_impl.h @@ -162,6 +162,14 @@ class TScanActorBase : public TActorBootstrapped { SendBatch(std::move(batch)); } + std::optional GetCellFrom(size_t index) const { + return GetCell(TableRange.From.GetCells(), index); + } + + std::optional GetCellTo(size_t index) const { + return GetCell(TableRange.To.GetCells(), index); + } + private: virtual void ProceedToScan() = 0; @@ -309,6 +317,13 @@ class TScanActorBase : public TActorBootstrapped { } } + std::optional GetCell(TConstArrayRef cells, size_t index) const { + if (index < cells.size() && !cells[index].IsNull()) { + return cells[index]; + } + return {}; + } + protected: static constexpr TDuration Timeout = TDuration::Seconds(60); From 3e4b81cc6c1d23db659482b65f5aefcd9744c8d3 Mon Sep 17 00:00:00 2001 From: kungasc Date: Thu, 30 Jan 2025 17:59:17 +0300 Subject: [PATCH 5/7] groups table range --- ydb/core/sys_view/auth/groups.cpp | 3 ++ ydb/core/sys_view/auth/owners.cpp | 12 +------- ydb/core/sys_view/auth/users.cpp | 16 ++-------- .../sys_view/common/scan_actor_base_impl.h | 14 +++++++++ ydb/core/sys_view/ut_kqp.cpp | 30 +++++++++++++++++++ 5 files changed, 51 insertions(+), 24 deletions(-) diff --git a/ydb/core/sys_view/auth/groups.cpp b/ydb/core/sys_view/auth/groups.cpp index 61abea6d01cb..f1ace2ec9121 100644 --- a/ydb/core/sys_view/auth/groups.cpp +++ b/ydb/core/sys_view/auth/groups.cpp @@ -33,6 +33,9 @@ class TGroupsScan : public TAuthScanBase { TVector groups(::Reserve(entry.DomainInfo->Groups.size())); for (const auto& group : entry.DomainInfo->Groups) { + if (!OneCellStringKeyIsInTableRange(group.Sid)) { + continue; + } groups.push_back(&group); } SortBatch(groups, [](const auto* left, const auto* right) { diff --git a/ydb/core/sys_view/auth/owners.cpp b/ydb/core/sys_view/auth/owners.cpp index 2a8d7f875254..afa0090e3fd2 100644 --- a/ydb/core/sys_view/auth/owners.cpp +++ b/ydb/core/sys_view/auth/owners.cpp @@ -34,17 +34,7 @@ class TOwnersScan : public TAuthScanBase { auto entryPath = CanonizePath(entry.Path); - bool isInRange = true; - if (auto pathFrom = GetCellFrom(0); pathFrom) { - int cmp = pathFrom->AsBuf().compare(entryPath); - isInRange &= cmp < 0 || cmp == 0 && TableRange.FromInclusive; - } - if (auto pathTo = GetCellTo(0); pathTo) { - int cmp = pathTo->AsBuf().compare(entryPath); - isInRange &= cmp > 0 || cmp == 0 && TableRange.ToInclusive; - } - - if (isInRange) { + if (OneCellStringKeyIsInTableRange(entryPath)) { for (auto& column : Columns) { switch (column.Tag) { case Schema::AuthOwners::Path::ColumnId: diff --git a/ydb/core/sys_view/auth/users.cpp b/ydb/core/sys_view/auth/users.cpp index a20a9e35999c..1be26dd4e5c5 100644 --- a/ydb/core/sys_view/auth/users.cpp +++ b/ydb/core/sys_view/auth/users.cpp @@ -89,6 +89,9 @@ class TUsersScan : public TScanActorBase { if (!user.HasName() || !CanAccessUser(user.GetName())) { continue; } + if (!OneCellStringKeyIsInTableRange(user.GetName())) { + continue; + } users.push_back(&user); } SortBatch(users, [](const auto* left, const auto* right) { @@ -98,19 +101,6 @@ class TUsersScan : public TScanActorBase { TVector cells(::Reserve(Columns.size())); for (const auto* user : users) { - bool isInRange = true; - if (auto pathFrom = GetCellFrom(0); pathFrom) { - int cmp = pathFrom->AsBuf().compare(user->GetName()); - isInRange &= cmp < 0 || cmp == 0 && TableRange.FromInclusive; - } - if (auto pathTo = GetCellTo(0); pathTo) { - int cmp = pathTo->AsBuf().compare(user->GetName()); - isInRange &= cmp > 0 || cmp == 0 && TableRange.ToInclusive; - } - if (!isInRange) { - continue; - } - for (auto& column : Columns) { switch (column.Tag) { case Schema::AuthUsers::Sid::ColumnId: diff --git a/ydb/core/sys_view/common/scan_actor_base_impl.h b/ydb/core/sys_view/common/scan_actor_base_impl.h index fdfff0c6d39d..e24a9336d1a0 100644 --- a/ydb/core/sys_view/common/scan_actor_base_impl.h +++ b/ydb/core/sys_view/common/scan_actor_base_impl.h @@ -170,6 +170,20 @@ class TScanActorBase : public TActorBootstrapped { return GetCell(TableRange.To.GetCells(), index); } + bool OneCellStringKeyIsInTableRange(const TString value) const { + if (auto pathFrom = GetCellFrom(0); pathFrom) { + if (int cmp = pathFrom->AsBuf().compare(value); cmp > 0 || cmp == 0 && !TableRange.FromInclusive) { + return false; + } + } + if (auto pathTo = GetCellTo(0); pathTo) { + if (int cmp = pathTo->AsBuf().compare(value); cmp < 0 || cmp == 0 && !TableRange.ToInclusive) { + return false; + } + } + return true; + } + private: virtual void ProceedToScan() = 0; diff --git a/ydb/core/sys_view/ut_kqp.cpp b/ydb/core/sys_view/ut_kqp.cpp index 9827b73e0762..37c6376614ca 100644 --- a/ydb/core/sys_view/ut_kqp.cpp +++ b/ydb/core/sys_view/ut_kqp.cpp @@ -2690,6 +2690,36 @@ Y_UNIT_TEST_SUITE(SystemView) { NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); } + Y_UNIT_TEST(AuthGroups_TableRange) { + TTestEnv env; + SetupAuthEnvironment(env); + TTableClient client(env.GetDriver()); + + for (auto group : { + "group1", + "group2", + "group3", + "group4", + }) { + env.GetClient().CreateGroup("/Root", group); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT Sid + FROM `Root/.sys/auth_groups` + WHERE Sid > "group1" AND Sid <= "group3" + )").GetValueSync(); + + auto expected = R"([ + [["group2"]]; + [["group3"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + } + Y_UNIT_TEST(AuthGroupMembers) { TTestEnv env; SetupAuthEnvironment(env); From 7da0fc7908cab5155e9644d43fa3f3cc1e5f2491 Mon Sep 17 00:00:00 2001 From: kungasc Date: Thu, 30 Jan 2025 19:30:04 +0300 Subject: [PATCH 6/7] loop filter apply --- ydb/core/sys_view/auth/auth_scan_base.h | 8 +- ydb/core/sys_view/auth/group_members.cpp | 4 +- ydb/core/sys_view/auth/groups.cpp | 5 +- ydb/core/sys_view/auth/owners.cpp | 2 +- ydb/core/sys_view/auth/permissions.cpp | 11 +- ydb/core/sys_view/auth/users.cpp | 2 +- .../sys_view/common/scan_actor_base_impl.h | 60 +++-- ydb/core/sys_view/ut_kqp.cpp | 239 ++++++++++++++++++ 8 files changed, 298 insertions(+), 33 deletions(-) diff --git a/ydb/core/sys_view/auth/auth_scan_base.h b/ydb/core/sys_view/auth/auth_scan_base.h index 997fe50978ac..5fdd92f2bf27 100644 --- a/ydb/core/sys_view/auth/auth_scan_base.h +++ b/ydb/core/sys_view/auth/auth_scan_base.h @@ -57,11 +57,11 @@ class TAuthScanBase : public TScanActorBase { , RequireUserAdministratorAccess(requireUserAdministratorAccess) { if (applyPathTableRange) { - if (auto cell = TBase::GetCellFrom(0)) { - PathFrom = cell->AsBuf(); + if (auto cellsFrom = TBase::TableRange.From.GetCells(); cellsFrom.size() > 0 && !cellsFrom[0].IsNull()) { + PathFrom = cellsFrom[0].AsBuf(); } - if (auto cell = TBase::GetCellTo(0)) { - PathTo = cell->AsBuf(); + if (auto cellsTo = TBase::TableRange.To.GetCells(); cellsTo.size() > 0 && !cellsTo[0].IsNull()) { + PathTo = cellsTo[0].AsBuf(); } } } diff --git a/ydb/core/sys_view/auth/group_members.cpp b/ydb/core/sys_view/auth/group_members.cpp index 7858c4aea4d7..253b9bcb63cd 100644 --- a/ydb/core/sys_view/auth/group_members.cpp +++ b/ydb/core/sys_view/auth/group_members.cpp @@ -34,7 +34,9 @@ class TGroupMembersScan : public TAuthScanBase { TVector> memberships; for (const auto& group : entry.DomainInfo->Groups) { for (const auto& member : group.Members) { - memberships.emplace_back(&group, &member); + if (StringKeyIsInTableRange({group.Sid, member})) { + memberships.emplace_back(&group, &member); + } } } SortBatch(memberships, [](const auto& left, const auto& right) { diff --git a/ydb/core/sys_view/auth/groups.cpp b/ydb/core/sys_view/auth/groups.cpp index f1ace2ec9121..ae1d2263627a 100644 --- a/ydb/core/sys_view/auth/groups.cpp +++ b/ydb/core/sys_view/auth/groups.cpp @@ -33,10 +33,9 @@ class TGroupsScan : public TAuthScanBase { TVector groups(::Reserve(entry.DomainInfo->Groups.size())); for (const auto& group : entry.DomainInfo->Groups) { - if (!OneCellStringKeyIsInTableRange(group.Sid)) { - continue; + if (StringKeyIsInTableRange({group.Sid})) { + groups.push_back(&group); } - groups.push_back(&group); } SortBatch(groups, [](const auto* left, const auto* right) { return left->Sid < right->Sid; diff --git a/ydb/core/sys_view/auth/owners.cpp b/ydb/core/sys_view/auth/owners.cpp index afa0090e3fd2..02f325c1f5d8 100644 --- a/ydb/core/sys_view/auth/owners.cpp +++ b/ydb/core/sys_view/auth/owners.cpp @@ -34,7 +34,7 @@ class TOwnersScan : public TAuthScanBase { auto entryPath = CanonizePath(entry.Path); - if (OneCellStringKeyIsInTableRange(entryPath)) { + if (StringKeyIsInTableRange({entryPath})) { for (auto& column : Columns) { switch (column.Tag) { case Schema::AuthOwners::Path::ColumnId: diff --git a/ydb/core/sys_view/auth/permissions.cpp b/ydb/core/sys_view/auth/permissions.cpp index 7b9f75ce0faf..c09def50ab0c 100644 --- a/ydb/core/sys_view/auth/permissions.cpp +++ b/ydb/core/sys_view/auth/permissions.cpp @@ -36,6 +36,8 @@ class TPermissionsScan : public TAuthScanBase { batch.Finished = false; return; } + + auto entryPath = CanonizePath(entry.Path); TVector> permissions; for (const NACLibProto::TACE& ace : entry.SecurityObject->GetACL().GetACE()) { @@ -45,10 +47,15 @@ class TPermissionsScan : public TAuthScanBase { if (!Effective && ace.GetInherited()) { continue; } + if (!ace.HasSID()) { + continue; + } auto acePermissions = ConvertACLMaskToYdbPermissionNames(ace.GetAccessRight()); for (const auto& permission : acePermissions) { - permissions.emplace_back(ace.HasSID() ? ace.GetSID() : TString{}, std::move(permission)); + if (StringKeyIsInTableRange({entryPath, ace.GetSID(), permission})) { + permissions.emplace_back(ace.GetSID(), std::move(permission)); + } } } // Note: due to rights inheritance permissions may be duplicated @@ -59,8 +66,6 @@ class TPermissionsScan : public TAuthScanBase { TVector cells(::Reserve(Columns.size())); - auto entryPath = CanonizePath(entry.Path); - for (const auto& [sid, permission] : permissions) { for (auto& column : Columns) { switch (column.Tag) { diff --git a/ydb/core/sys_view/auth/users.cpp b/ydb/core/sys_view/auth/users.cpp index 1be26dd4e5c5..5aa8c3b337c9 100644 --- a/ydb/core/sys_view/auth/users.cpp +++ b/ydb/core/sys_view/auth/users.cpp @@ -89,7 +89,7 @@ class TUsersScan : public TScanActorBase { if (!user.HasName() || !CanAccessUser(user.GetName())) { continue; } - if (!OneCellStringKeyIsInTableRange(user.GetName())) { + if (!StringKeyIsInTableRange({user.GetName()})) { continue; } users.push_back(&user); diff --git a/ydb/core/sys_view/common/scan_actor_base_impl.h b/ydb/core/sys_view/common/scan_actor_base_impl.h index e24a9336d1a0..c3a72b3ac84c 100644 --- a/ydb/core/sys_view/common/scan_actor_base_impl.h +++ b/ydb/core/sys_view/common/scan_actor_base_impl.h @@ -162,25 +162,52 @@ class TScanActorBase : public TActorBootstrapped { SendBatch(std::move(batch)); } - std::optional GetCellFrom(size_t index) const { - return GetCell(TableRange.From.GetCells(), index); - } - - std::optional GetCellTo(size_t index) const { - return GetCell(TableRange.To.GetCells(), index); - } - - bool OneCellStringKeyIsInTableRange(const TString value) const { - if (auto pathFrom = GetCellFrom(0); pathFrom) { - if (int cmp = pathFrom->AsBuf().compare(value); cmp > 0 || cmp == 0 && !TableRange.FromInclusive) { + bool StringKeyIsInTableRange(const TVector& key) const { + { + bool equalPrefixes = true; + for (size_t index : xrange(Min(TableRange.From.GetCells().size(), key.size()))) { + if (auto cellFrom = TableRange.From.GetCells()[index]; !cellFrom.IsNull()) { + int cmp = cellFrom.AsBuf().compare(key[index]); + if (cmp < 0) { + equalPrefixes = false; + break; + } + if (cmp > 0) { + return false; + } + // cmp == 0, prefixes are equal, go further + } else { + equalPrefixes = false; + break; + } + } + if (equalPrefixes && !TableRange.FromInclusive) { return false; } } - if (auto pathTo = GetCellTo(0); pathTo) { - if (int cmp = pathTo->AsBuf().compare(value); cmp < 0 || cmp == 0 && !TableRange.ToInclusive) { + + if (TableRange.To.GetCells().size()) { + bool equalPrefixes = true; + for (size_t index : xrange(Min(TableRange.To.GetCells().size(), key.size()))) { + if (auto cellTo = TableRange.To.GetCells()[index]; !cellTo.IsNull()) { + int cmp = cellTo.AsBuf().compare(key[index]); + if (cmp > 0) { + equalPrefixes = false; + break; + } + if (cmp < 0) { + return false; + } + // cmp == 0, prefixes are equal, go further + } else { + break; + } + } + if (equalPrefixes && !TableRange.ToInclusive) { return false; } } + return true; } @@ -331,13 +358,6 @@ class TScanActorBase : public TActorBootstrapped { } } - std::optional GetCell(TConstArrayRef cells, size_t index) const { - if (index < cells.size() && !cells[index].IsNull()) { - return cells[index]; - } - return {}; - } - protected: static constexpr TDuration Timeout = TDuration::Seconds(60); diff --git a/ydb/core/sys_view/ut_kqp.cpp b/ydb/core/sys_view/ut_kqp.cpp index 37c6376614ca..c35f501add9b 100644 --- a/ydb/core/sys_view/ut_kqp.cpp +++ b/ydb/core/sys_view/ut_kqp.cpp @@ -2939,6 +2939,201 @@ Y_UNIT_TEST_SUITE(SystemView) { NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); } + Y_UNIT_TEST(AuthGroupMembers_TableRange) { + TTestEnv env; + SetupAuthEnvironment(env); + TTableClient client(env.GetDriver()); + + for (auto group : { + "group1", + "group2", + "group3", + }) { + env.GetClient().CreateGroup("/Root", group); + } + + for (auto user : { + "user1", + "user2", + "user3" + }) { + env.GetClient().CreateUser("/Root", user, "password"); + } + + for (auto membership : TVector>{ + {"group1", "user1"}, + {"group1", "user2"}, + {"group2", "user1"}, + {"group2", "user2"}, + {"group2", "user3"}, + {"group3", "user1"}, + {"group3", "user2"}, + }) { + env.GetClient().AddGroupMembership("/Root", membership.first, membership.second); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_group_members` + )").GetValueSync(); + + auto expected = R"([ + [["group1"];["user1"]]; + [["group1"];["user2"]]; + [["group2"];["user1"]]; + [["group2"];["user2"]]; + [["group2"];["user3"]]; + [["group3"];["user1"]]; + [["group3"];["user2"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_group_members` + WHERE GroupSid > "group1" AND GroupSid <= "group3" + )").GetValueSync(); + + auto expected = R"([ + [["group2"];["user1"]]; + [["group2"];["user2"]]; + [["group2"];["user3"]]; + [["group3"];["user1"]]; + [["group3"];["user2"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_group_members` + WHERE GroupSid >= "group2" + )").GetValueSync(); + + auto expected = R"([ + [["group2"];["user1"]]; + [["group2"];["user2"]]; + [["group2"];["user3"]]; + [["group3"];["user1"]]; + [["group3"];["user2"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_group_members` + WHERE GroupSid > "group2" + )").GetValueSync(); + + auto expected = R"([ + [["group3"];["user1"]]; + [["group3"];["user2"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_group_members` + WHERE GroupSid <= "group2" + )").GetValueSync(); + + auto expected = R"([ + [["group1"];["user1"]]; + [["group1"];["user2"]]; + [["group2"];["user1"]]; + [["group2"];["user2"]]; + [["group2"];["user3"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_group_members` + WHERE GroupSid < "group2" + )").GetValueSync(); + + auto expected = R"([ + [["group1"];["user1"]]; + [["group1"];["user2"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_group_members` + WHERE GroupSid = "group2" AND MemberSid >= "user2" + )").GetValueSync(); + + auto expected = R"([ + [["group2"];["user2"]]; + [["group2"];["user3"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_group_members` + WHERE GroupSid = "group2" AND MemberSid > "user2" + )").GetValueSync(); + + auto expected = R"([ + [["group2"];["user3"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_group_members` + WHERE GroupSid = "group2" AND MemberSid <= "user2" + )").GetValueSync(); + + auto expected = R"([ + [["group2"];["user1"]]; + [["group2"];["user2"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_group_members` + WHERE GroupSid = "group2" AND MemberSid < "user2" + )").GetValueSync(); + + auto expected = R"([ + [["group2"];["user1"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + } + Y_UNIT_TEST(AuthOwners) { TTestEnv env; SetupAuthEnvironment(env); @@ -3936,6 +4131,50 @@ Y_UNIT_TEST_SUITE(SystemView) { NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_permissions` + WHERE Path = "/Root/Dir1/SubDir1" AND Sid = "user2" + )").GetValueSync(); + + auto expected = R"([ + [["/Root/Dir1/SubDir1"];["ydb.granular.erase_row"];["user2"]]; + [["/Root/Dir1/SubDir1"];["ydb.granular.select_row"];["user2"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_permissions` + WHERE Path = "/Root/Dir1/SubDir1" AND Sid = "user2" AND Permission >= "ydb.granular.erase_row" + )").GetValueSync(); + + auto expected = R"([ + [["/Root/Dir1/SubDir1"];["ydb.granular.erase_row"];["user2"]]; + [["/Root/Dir1/SubDir1"];["ydb.granular.select_row"];["user2"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } + + { + auto it = client.StreamExecuteScanQuery(R"( + SELECT * + FROM `Root/.sys/auth_permissions` + WHERE Path = "/Root/Dir1/SubDir1" AND Sid = "user2" AND Permission > "ydb.granular.erase_row" + )").GetValueSync(); + + auto expected = R"([ + [["/Root/Dir1/SubDir1"];["ydb.granular.select_row"];["user2"]]; + ])"; + + NKqp::CompareYson(expected, NKqp::StreamResultToYson(it)); + } } } From 866ea7a0de0b24d84a3038a88b6527716a59ef3e Mon Sep 17 00:00:00 2001 From: kungasc Date: Thu, 30 Jan 2025 21:06:54 +0300 Subject: [PATCH 7/7] fix style --- ydb/core/sys_view/auth/auth_scan_base.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ydb/core/sys_view/auth/auth_scan_base.h b/ydb/core/sys_view/auth/auth_scan_base.h index 5fdd92f2bf27..b6ce0a4a9852 100644 --- a/ydb/core/sys_view/auth/auth_scan_base.h +++ b/ydb/core/sys_view/auth/auth_scan_base.h @@ -122,7 +122,7 @@ class TAuthScanBase : public TScanActorBase { if (child.Kind == TSchemeCacheNavigate::KindExtSubdomain || child.Kind == TSchemeCacheNavigate::KindSubdomain) { continue; } - + last.Entry.Path.push_back(child.Name); if ((PathFrom || PathTo) && ShouldSkipSubTree(CanonizePath(last.Entry.Path))) { last.Entry.Path.pop_back();