From 173fcb2ab1927e320e36abbba3f3b10fb73d3df2 Mon Sep 17 00:00:00 2001 From: kungasc Date: Tue, 28 Jan 2025 16:55:48 +0300 Subject: [PATCH 1/3] Check group remove allowance similarly user --- .../schemeshard__operation_alter_login.cpp | 73 ++++++---- .../tx/schemeshard/ut_helpers/helpers.cpp | 12 ++ ydb/core/tx/schemeshard/ut_helpers/helpers.h | 3 + .../tx/schemeshard/ut_helpers/ls_checks.cpp | 12 +- .../tx/schemeshard/ut_helpers/ls_checks.h | 1 + ydb/core/tx/schemeshard/ut_login/ut_login.cpp | 125 +++++++++++++++++- ydb/library/login/login.cpp | 18 +-- ydb/library/login/login.h | 8 +- 8 files changed, 207 insertions(+), 45 deletions(-) diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_alter_login.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_alter_login.cpp index 00b51f0a743a..7c2e43e0e03b 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_alter_login.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_alter_login.cpp @@ -182,18 +182,10 @@ class TAlterLogin: public TSubOperationBase { } case NKikimrSchemeOp::TAlterLogin::kRemoveGroup: { const auto& removeGroup = alterLogin.GetRemoveGroup(); - const TString& group = removeGroup.GetGroup(); - auto response = context.SS->LoginProvider.RemoveGroup({ - .Group = group, - .MissingOk = removeGroup.GetMissingOk() - }); + auto response = RemoveGroup(context, removeGroup, db); if (response.Error) { result->SetStatus(NKikimrScheme::StatusPreconditionFailed, response.Error); } else { - db.Table().Key(group).Delete(); - for (const TString& parent : response.TouchedGroups) { - db.Table().Key(parent, group).Delete(); - } result->SetStatus(NKikimrScheme::StatusSuccess); } break; @@ -249,20 +241,8 @@ class TAlterLogin: public TSubOperationBase { return {.Error = "User not found"}; } - auto subTree = context.SS->ListSubTree(context.SS->RootPathId(), context.Ctx); - for (auto pathId : subTree) { - TPathElement::TPtr path = context.SS->PathsById.at(pathId); - if (path->Owner == user) { - auto pathStr = TPath::Init(pathId, context.SS).PathString(); - return {.Error = TStringBuilder() << - "User " << user << " owns " << pathStr << " and can't be removed"}; - } - NACLib::TACL acl(path->ACL); - if (acl.HasAccess(user)) { - auto pathStr = TPath::Init(pathId, context.SS).PathString(); - return {.Error = TStringBuilder() << - "User " << user << " has an ACL record on " << pathStr << " and can't be removed"}; - } + if (auto canRemove = CanRemoveSid(context, user, "User"); canRemove.Error) { + return canRemove; } auto removeUserResponse = context.SS->LoginProvider.RemoveUser(user); @@ -278,6 +258,53 @@ class TAlterLogin: public TSubOperationBase { return {}; // success } + NLogin::TLoginProvider::TBasicResponse RemoveGroup(TOperationContext& context, const NKikimrSchemeOp::TLoginRemoveGroup& removeGroup, NIceDb::TNiceDb& db) { + const TString& group = removeGroup.GetGroup(); + + if (!context.SS->LoginProvider.CheckGroupExists(group)) { + if (removeGroup.GetMissingOk()) { + return {}; // success + } + return {.Error = "Group not found"}; + } + + if (auto canRemove = CanRemoveSid(context, group, "Group"); canRemove.Error) { + return canRemove; + } + + auto removeGroupResponse = context.SS->LoginProvider.RemoveGroup(group); + if (removeGroupResponse.Error) { + return removeGroupResponse; + } + + db.Table().Key(group).Delete(); + for (const TString& parent : removeGroupResponse.TouchedGroups) { + db.Table().Key(parent, group).Delete(); + } + + return {}; // success + } + + NLogin::TLoginProvider::TBasicResponse CanRemoveSid(TOperationContext& context, const TString sid, const TString& sidType) { + auto subTree = context.SS->ListSubTree(context.SS->RootPathId(), context.Ctx); + for (auto pathId : subTree) { + TPathElement::TPtr path = context.SS->PathsById.at(pathId); + if (path->Owner == sid) { + auto pathStr = TPath::Init(pathId, context.SS).PathString(); + return {.Error = TStringBuilder() << + sidType << " " << sid << " owns " << pathStr << " and can't be removed"}; + } + NACLib::TACL acl(path->ACL); + if (acl.HasAccess(sid)) { + auto pathStr = TPath::Init(pathId, context.SS).PathString(); + return {.Error = TStringBuilder() << + sidType << " " << sid << " has an ACL record on " << pathStr << " and can't be removed"}; + } + } + + return {}; // success + } + void AddIsUserAdmin(const TString& user, NLogin::TLoginProvider& loginProvider, TParts& additionalParts) { const auto& adminSids = AppData()->AdministrationAllowedSIDs; bool isAdmin = adminSids.empty(); diff --git a/ydb/core/tx/schemeshard/ut_helpers/helpers.cpp b/ydb/core/tx/schemeshard/ut_helpers/helpers.cpp index ac12c4ccb7a8..f7b04092c3e3 100644 --- a/ydb/core/tx/schemeshard/ut_helpers/helpers.cpp +++ b/ydb/core/tx/schemeshard/ut_helpers/helpers.cpp @@ -1993,6 +1993,18 @@ namespace NSchemeShardUT_Private { TestModificationResults(runtime, txId, expectedResults); } + void CreateAlterLoginRemoveGroup(TTestActorRuntime& runtime, ui64 txId, const TString& database, const TString& group, const TVector& expectedResults) { + auto modifyTx = std::make_unique(txId, TTestTxConfig::SchemeShard); + auto transaction = modifyTx->Record.AddTransaction(); + transaction->SetWorkingDir(database); + transaction->SetOperationType(NKikimrSchemeOp::EOperationType::ESchemeOpAlterLogin); + auto createGroup = transaction->MutableAlterLogin()->MutableRemoveGroup(); + createGroup->SetGroup(group); + + AsyncSend(runtime, TTestTxConfig::SchemeShard, modifyTx.release()); + TestModificationResults(runtime, txId, expectedResults); + } + void AlterLoginAddGroupMembership(TTestActorRuntime& runtime, ui64 txId, const TString& database, const TString& member, const TString& group, const TVector& expectedResults) { auto modifyTx = std::make_unique(txId, TTestTxConfig::SchemeShard); auto transaction = modifyTx->Record.AddTransaction(); diff --git a/ydb/core/tx/schemeshard/ut_helpers/helpers.h b/ydb/core/tx/schemeshard/ut_helpers/helpers.h index a9f4e9bce29b..bcca433cc1cc 100644 --- a/ydb/core/tx/schemeshard/ut_helpers/helpers.h +++ b/ydb/core/tx/schemeshard/ut_helpers/helpers.h @@ -534,6 +534,9 @@ namespace NSchemeShardUT_Private { void CreateAlterLoginCreateGroup(TTestActorRuntime& runtime, ui64 txId, const TString& database, const TString& group, const TVector& expectedResults = {{NKikimrScheme::StatusSuccess}}); + + void CreateAlterLoginRemoveGroup(TTestActorRuntime& runtime, ui64 txId, const TString& database, + const TString& group, const TVector& expectedResults = {{NKikimrScheme::StatusSuccess}}); void AlterLoginAddGroupMembership(TTestActorRuntime& runtime, ui64 txId, const TString& database, const TString& member, const TString& group, diff --git a/ydb/core/tx/schemeshard/ut_helpers/ls_checks.cpp b/ydb/core/tx/schemeshard/ut_helpers/ls_checks.cpp index 94eeb6975d51..25804068e46c 100644 --- a/ydb/core/tx/schemeshard/ut_helpers/ls_checks.cpp +++ b/ydb/core/tx/schemeshard/ut_helpers/ls_checks.cpp @@ -1248,7 +1248,7 @@ TCheckFunc HasGroup(const TString& group, const TSet members) { return [=](const NKikimrScheme::TEvDescribeSchemeResult& record) { std::optional> actualMembers; for (const auto& sid : record.GetPathDescription().GetDomainDescription().GetSecurityState().GetSids()) { - if (sid.GetName() == group) { + if (sid.GetName() == group && sid.GetType() == NLoginProto::ESidType::GROUP) { actualMembers.emplace(); for (const auto& member : sid.GetMembers()) { actualMembers->insert(member); @@ -1260,6 +1260,16 @@ TCheckFunc HasGroup(const TString& group, const TSet members) { }; } +TCheckFunc HasNoGroup(const TString& group) { + return [=](const NKikimrScheme::TEvDescribeSchemeResult& record) { + for (const auto& sid : record.GetPathDescription().GetDomainDescription().GetSecurityState().GetSids()) { + if (sid.GetName() == group && sid.GetType() == NLoginProto::ESidType::GROUP) { + UNIT_FAIL("Group " + group + " exists"); + } + } + }; +} + void CheckRight(const NKikimrScheme::TEvDescribeSchemeResult& record, const TString& right, bool mustHave, bool isEffective) { const auto& self = record.GetPathDescription().GetSelf(); TSecurityObject src(self.GetOwner(), isEffective ? self.GetEffectiveACL() : self.GetACL(), false); diff --git a/ydb/core/tx/schemeshard/ut_helpers/ls_checks.h b/ydb/core/tx/schemeshard/ut_helpers/ls_checks.h index 1ed97ba6ca3c..1813edca62b5 100644 --- a/ydb/core/tx/schemeshard/ut_helpers/ls_checks.h +++ b/ydb/core/tx/schemeshard/ut_helpers/ls_checks.h @@ -175,6 +175,7 @@ namespace NLs { TCheckFunc BackupHistoryCount(ui64 count); TCheckFunc HasGroup(const TString& group, const TSet members); + TCheckFunc HasNoGroup(const TString& group); TCheckFunc HasOwner(const TString& owner); TCheckFunc HasRight(const TString& right); TCheckFunc HasNoRight(const TString& right); diff --git a/ydb/core/tx/schemeshard/ut_login/ut_login.cpp b/ydb/core/tx/schemeshard/ut_login/ut_login.cpp index bf6fb1aaf0fe..bce1aa86ab36 100644 --- a/ydb/core/tx/schemeshard/ut_login/ut_login.cpp +++ b/ydb/core/tx/schemeshard/ut_login/ut_login.cpp @@ -73,7 +73,7 @@ void CheckToken(const TString& token, const NKikimrScheme::TEvDescribeSchemeResu Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) { - Y_UNIT_TEST(Login) { + Y_UNIT_TEST(UserLogin) { TTestBasicRuntime runtime; TTestEnv env(runtime); ui64 txId = 100; @@ -107,7 +107,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) { } } - Y_UNIT_TEST(RemoveLogin) { + Y_UNIT_TEST(RemoveUser) { TTestBasicRuntime runtime; TTestEnv env(runtime); ui64 txId = 100; @@ -125,12 +125,12 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) { } } - Y_UNIT_TEST(RemoveLogin_NonExisting) { + Y_UNIT_TEST(RemoveUser_NonExisting) { TTestBasicRuntime runtime; TTestEnv env(runtime); ui64 txId = 100; - for (bool missingOk : { false, true}) { + for (bool missingOk : {false, true}) { auto modifyTx = std::make_unique(txId, TTestTxConfig::SchemeShard); auto transaction = modifyTx->Record.AddTransaction(); transaction->SetWorkingDir("/MyRoot"); @@ -148,7 +148,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) { } } - Y_UNIT_TEST(RemoveLogin_Groups) { + Y_UNIT_TEST(RemoveUser_Groups) { TTestBasicRuntime runtime; TTestEnv env(runtime); ui64 txId = 100; @@ -193,7 +193,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) { } } - Y_UNIT_TEST(RemoveLogin_Owner) { + Y_UNIT_TEST(RemoveUser_Owner) { TTestBasicRuntime runtime; TTestEnv env(runtime); ui64 txId = 100; @@ -237,7 +237,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) { } } - Y_UNIT_TEST(RemoveLogin_Acl) { + Y_UNIT_TEST(RemoveUser_Acl) { TTestBasicRuntime runtime; TTestEnv env(runtime); ui64 txId = 100; @@ -299,6 +299,117 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) { } } + Y_UNIT_TEST(RemoveGroup) { + TTestBasicRuntime runtime; + TTestEnv env(runtime); + ui64 txId = 100; + + CreateAlterLoginCreateUser(runtime, ++txId, "/MyRoot", "user1", "password1"); + CreateAlterLoginCreateGroup(runtime, ++txId, "/MyRoot", "group1"); + AlterLoginAddGroupMembership(runtime, ++txId, "/MyRoot", "user1", "group1"); + TestDescribeResult(DescribePath(runtime, "/MyRoot"), + {NLs::HasGroup("group1", {"user1"})}); + + CreateAlterLoginRemoveGroup(runtime, ++txId, "/MyRoot", "group1"); + TestDescribeResult(DescribePath(runtime, "/MyRoot"), + {NLs::HasNoGroup("group1")}); + } + + Y_UNIT_TEST(RemoveGroup_NonExisting) { + TTestBasicRuntime runtime; + TTestEnv env(runtime); + ui64 txId = 100; + + for (bool missingOk : {false, true}) { + auto modifyTx = std::make_unique(txId, TTestTxConfig::SchemeShard); + auto transaction = modifyTx->Record.AddTransaction(); + transaction->SetWorkingDir("/MyRoot"); + transaction->SetOperationType(NKikimrSchemeOp::EOperationType::ESchemeOpAlterLogin); + + auto removeUser = transaction->MutableAlterLogin()->MutableRemoveGroup(); + removeUser->SetGroup("group1"); + removeUser->SetMissingOk(missingOk); + + AsyncSend(runtime, TTestTxConfig::SchemeShard, modifyTx.release()); + TestModificationResults(runtime, txId, TVector{{ + missingOk ? NKikimrScheme::StatusSuccess : NKikimrScheme::StatusPreconditionFailed, + missingOk ? "" : "Group not found" + }}); + } + } + + Y_UNIT_TEST(RemoveGroup_Owner) { + TTestBasicRuntime runtime; + TTestEnv env(runtime); + ui64 txId = 100; + + CreateAlterLoginCreateGroup(runtime, ++txId, "/MyRoot", "group1"); + TestDescribeResult(DescribePath(runtime, "/MyRoot"), + {NLs::HasGroup("group1", {})}); + + AsyncMkDir(runtime, ++txId, "/MyRoot", "Dir1"); + AsyncModifyACL(runtime, ++txId, "/MyRoot", "Dir1", NACLib::TDiffACL{}.SerializeAsString(), "group1"); + TestModificationResult(runtime, txId, NKikimrScheme::StatusSuccess); + TestDescribeResult(DescribePath(runtime, "/MyRoot/Dir1"), + {NLs::HasOwner("group1")}); + + CreateAlterLoginRemoveGroup(runtime, ++txId, "/MyRoot", "group1", + TVector{{NKikimrScheme::StatusPreconditionFailed, "Group group1 owns /MyRoot/Dir1 and can't be removed"}}); + TestDescribeResult(DescribePath(runtime, "/MyRoot"), + {NLs::HasGroup("group1", {})}); + TestDescribeResult(DescribePath(runtime, "/MyRoot/Dir1"), + {NLs::HasOwner("group1")}); + + AsyncModifyACL(runtime, ++txId, "/MyRoot", "Dir1", NACLib::TDiffACL{}.SerializeAsString(), "root@builtin"); + TestModificationResult(runtime, txId, NKikimrScheme::StatusSuccess); + TestDescribeResult(DescribePath(runtime, "/MyRoot/Dir1"), + {NLs::HasOwner("root@builtin")}); + + CreateAlterLoginRemoveGroup(runtime, ++txId, "/MyRoot", "group1"); + TestDescribeResult(DescribePath(runtime, "/MyRoot"), + {NLs::HasNoGroup("group1")}); + } + + Y_UNIT_TEST(RemoveGroup_Acl) { + TTestBasicRuntime runtime; + TTestEnv env(runtime); + ui64 txId = 100; + + CreateAlterLoginCreateGroup(runtime, ++txId, "/MyRoot", "group1"); + TestDescribeResult(DescribePath(runtime, "/MyRoot"), + {NLs::HasGroup("group1", {})}); + + AsyncMkDir(runtime, ++txId, "/MyRoot", "Dir1"); + { + NACLib::TDiffACL diffACL; + diffACL.AddAccess(NACLib::EAccessType::Allow, NACLib::GenericUse, "group1"); + AsyncModifyACL(runtime, ++txId, "/MyRoot", "Dir1", diffACL.SerializeAsString(), ""); + } + TestModificationResult(runtime, txId, NKikimrScheme::StatusSuccess); + TestDescribeResult(DescribePath(runtime, "/MyRoot/Dir1"), + {NLs::HasRight("+U:group1")}); + + CreateAlterLoginRemoveGroup(runtime, ++txId, "/MyRoot", "group1", + TVector{{NKikimrScheme::StatusPreconditionFailed, "Group group1 has an ACL record on /MyRoot/Dir1 and can't be removed"}}); + TestDescribeResult(DescribePath(runtime, "/MyRoot"), + {NLs::HasGroup("group1", {})}); + TestDescribeResult(DescribePath(runtime, "/MyRoot/Dir1"), + {NLs::HasRight("+U:group1")}); + + { + NACLib::TDiffACL diffACL; + diffACL.RemoveAccess(NACLib::EAccessType::Allow, NACLib::GenericUse, "group1"); + AsyncModifyACL(runtime, ++txId, "/MyRoot", "Dir1", diffACL.SerializeAsString(), ""); + } + TestModificationResult(runtime, txId, NKikimrScheme::StatusSuccess); + TestDescribeResult(DescribePath(runtime, "/MyRoot/Dir1"), + {NLs::HasNoRight("+U:group1")}); + + CreateAlterLoginRemoveGroup(runtime, ++txId, "/MyRoot", "group1"); + TestDescribeResult(DescribePath(runtime, "/MyRoot"), + {NLs::HasNoGroup("group1")}); + } + Y_UNIT_TEST(TestExternalLogin) { TTestBasicRuntime runtime; TTestEnv env(runtime); diff --git a/ydb/library/login/login.cpp b/ydb/library/login/login.cpp index dd779cdac5b8..e0236e1be3cd 100644 --- a/ydb/library/login/login.cpp +++ b/ydb/library/login/login.cpp @@ -100,6 +100,10 @@ bool TLoginProvider::CheckUserExists(const TString& user) { return CheckSubjectExists(user, ESidType::USER); } +bool TLoginProvider::CheckGroupExists(const TString& group) { + return CheckSubjectExists(group, ESidType::GROUP); +} + TLoginProvider::TBasicResponse TLoginProvider::ModifyUser(const TModifyUserRequest& request) { TBasicResponse response; @@ -278,31 +282,29 @@ TLoginProvider::TRenameGroupResponse TLoginProvider::RenameGroup(const TRenameGr return response; } -TLoginProvider::TRemoveGroupResponse TLoginProvider::RemoveGroup(const TRemoveGroupRequest& request) { +TLoginProvider::TRemoveGroupResponse TLoginProvider::RemoveGroup(const TString& group) { TRemoveGroupResponse response; - auto itGroupModify = Sids.find(request.Group); + auto itGroupModify = Sids.find(group); if (itGroupModify == Sids.end() || itGroupModify->second.Type != ESidType::GROUP) { - if (!request.MissingOk) { - response.Error = "Group not found"; - } + response.Error = "Group not found"; return response; } - auto itChildToParentIndex = ChildToParentIndex.find(request.Group); + auto itChildToParentIndex = ChildToParentIndex.find(group); if (itChildToParentIndex != ChildToParentIndex.end()) { for (const TString& parent : itChildToParentIndex->second) { auto itGroup = Sids.find(parent); if (itGroup != Sids.end()) { response.TouchedGroups.emplace_back(itGroup->first); - itGroup->second.Members.erase(request.Group); + itGroup->second.Members.erase(group); } } ChildToParentIndex.erase(itChildToParentIndex); } for (const TString& member : itGroupModify->second.Members) { - ChildToParentIndex[member].erase(request.Group); + ChildToParentIndex[member].erase(group); } Sids.erase(itGroupModify); diff --git a/ydb/library/login/login.h b/ydb/library/login/login.h index a40b509b9064..1ec45941bfaf 100644 --- a/ydb/library/login/login.h +++ b/ydb/library/login/login.h @@ -140,11 +140,6 @@ class TLoginProvider { std::vector TouchedGroups; }; - struct TRemoveGroupRequest : TBasicRequest { - TString Group; - bool MissingOk; - }; - struct TRemoveGroupResponse : TBasicResponse { std::vector TouchedGroups; }; @@ -202,7 +197,8 @@ class TLoginProvider { TBasicResponse AddGroupMembership(const TAddGroupMembershipRequest& request); TBasicResponse RemoveGroupMembership(const TRemoveGroupMembershipRequest& request); TRenameGroupResponse RenameGroup(const TRenameGroupRequest& request); - TRemoveGroupResponse RemoveGroup(const TRemoveGroupRequest& request); + TRemoveGroupResponse RemoveGroup(const TString& group); + bool CheckGroupExists(const TString& group); void UpdatePasswordCheckParameters(const TPasswordComplexity& passwordComplexity); void UpdateAccountLockout(const TAccountLockout::TInitializer& accountLockoutInitializer); From 5755dbb5bd7d07c4cc598cbe2e77a58eb149e5b7 Mon Sep 17 00:00:00 2001 From: kungasc Date: Tue, 28 Jan 2025 17:38:04 +0300 Subject: [PATCH 2/3] better audit log naming --- .../schemeshard__operation_alter_login.cpp | 22 +++++++++---------- .../tx/schemeshard/schemeshard_audit_log.cpp | 10 ++++----- .../tx/schemeshard/schemeshard_audit_log.h | 4 ++-- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_alter_login.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_alter_login.cpp index 7c2e43e0e03b..dbe181f69055 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_alter_login.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_alter_login.cpp @@ -27,7 +27,7 @@ class TAlterLogin: public TSubOperationBase { const NKikimrConfig::TDomainsConfig::TSecurityConfig& securityConfig = context.SS->GetDomainsConfig().GetSecurityConfig(); const NKikimrSchemeOp::TAlterLogin& alterLogin = Transaction.GetAlterLogin(); - TParts additionalParts; + TAuditLogParts auditLogParts; switch (alterLogin.GetAlterCase()) { case NKikimrSchemeOp::TAlterLogin::kCreateUser: { @@ -60,7 +60,7 @@ class TAlterLogin: public TSubOperationBase { } result->SetStatus(NKikimrScheme::StatusSuccess); - AddIsUserAdmin(createUser.GetUser(), context.SS->LoginProvider, additionalParts); + AddAuditLogIsUserAdminPart(createUser.GetUser(), context.SS->LoginProvider, auditLogParts); } break; } @@ -89,8 +89,8 @@ class TAlterLogin: public TSubOperationBase { Schema::LoginSids::IsEnabled>(sid.Type, sid.PasswordHash, sid.IsEnabled); result->SetStatus(NKikimrScheme::StatusSuccess); - AddIsUserAdmin(modifyUser.GetUser(), context.SS->LoginProvider, additionalParts); - AddLastSuccessfulLogin(sid, additionalParts); + AddAuditLogIsUserAdminPart(modifyUser.GetUser(), context.SS->LoginProvider, auditLogParts); + AddAuditLogLastSuccessfulLoginPart(sid, auditLogParts); } break; } @@ -99,7 +99,7 @@ class TAlterLogin: public TSubOperationBase { auto sid = context.SS->LoginProvider.Sids.find(removeUser.GetUser()); if (context.SS->LoginProvider.Sids.end() != sid) { - AddLastSuccessfulLogin(sid->second, additionalParts); + AddAuditLogLastSuccessfulLoginPart(sid->second, auditLogParts); } auto response = RemoveUser(context, removeUser, db); @@ -108,7 +108,7 @@ class TAlterLogin: public TSubOperationBase { } else { result->SetStatus(NKikimrScheme::StatusSuccess); - AddIsUserAdmin(removeUser.GetUser(), context.SS->LoginProvider, additionalParts); + AddAuditLogIsUserAdminPart(removeUser.GetUser(), context.SS->LoginProvider, auditLogParts); } break; } @@ -203,7 +203,7 @@ class TAlterLogin: public TSubOperationBase { } const auto status = result->Record.GetStatus(); const auto reason = result->Record.HasReason() ? result->Record.GetReason() : TString(); - AuditLogModifySchemeOperation(Transaction, status, reason, context.SS, context.PeerName, userSID, sanitizedToken, ui64(txId), additionalParts); + AuditLogModifySchemeOperation(Transaction, status, reason, context.SS, context.PeerName, userSID, sanitizedToken, ui64(txId), auditLogParts); } if (result->Record.GetStatus() == NKikimrScheme::StatusSuccess) { @@ -305,7 +305,7 @@ class TAlterLogin: public TSubOperationBase { return {}; // success } - void AddIsUserAdmin(const TString& user, NLogin::TLoginProvider& loginProvider, TParts& additionalParts) { + void AddAuditLogIsUserAdminPart(const TString& user, NLogin::TLoginProvider& loginProvider, TAuditLogParts& auditLogParts) { const auto& adminSids = AppData()->AdministrationAllowedSIDs; bool isAdmin = adminSids.empty(); if (!isAdmin) { @@ -319,15 +319,15 @@ class TAlterLogin: public TSubOperationBase { } if (isAdmin) { - additionalParts.emplace_back("login_user_level", "admin"); + auditLogParts.emplace_back("login_user_level", "admin"); } } - void AddLastSuccessfulLogin(NLogin::TLoginProvider::TSidRecord& sid, TParts& additionalParts) { + void AddAuditLogLastSuccessfulLoginPart(NLogin::TLoginProvider::TSidRecord& sid, TAuditLogParts& auditLogParts) { const auto duration = sid.LastSuccessfulLogin.time_since_epoch(); const auto time = std::chrono::duration_cast(duration).count(); if (time) { - additionalParts.emplace_back("last_login", TInstant::MicroSeconds(time).ToString()); + auditLogParts.emplace_back("last_login", TInstant::MicroSeconds(time).ToString()); } } }; diff --git a/ydb/core/tx/schemeshard/schemeshard_audit_log.cpp b/ydb/core/tx/schemeshard/schemeshard_audit_log.cpp index 9141da0ab692..27a5a84806c1 100644 --- a/ydb/core/tx/schemeshard/schemeshard_audit_log.cpp +++ b/ydb/core/tx/schemeshard/schemeshard_audit_log.cpp @@ -84,7 +84,7 @@ TPath DatabasePathFromWorkingDir(TSchemeShard* SS, const TString &opWorkingDir) void AuditLogModifySchemeOperation(const NKikimrSchemeOp::TModifyScheme& operation, NKikimrScheme::EStatus status, const TString& reason, TSchemeShard* SS, const TString& peerName, const TString& userSID, const TString& sanitizedToken, - ui64 txId, const TParts& additionalParts) { + ui64 txId, const TAuditLogParts& additionalParts) { auto logEntry = MakeAuditLogFragment(operation); TPath databasePath = DatabasePathFromWorkingDir(SS, operation.GetWorkingDir()); @@ -151,7 +151,7 @@ void AuditLogModifySchemeTransaction(const NKikimrScheme::TEvModifySchemeTransac if (NKikimrSchemeOp::EOperationType::ESchemeOpAlterLogin == type) { continue; } - AuditLogModifySchemeOperation(operation, status, reason, SS, peerName, userSID, sanitizedToken, txId, TParts()); + AuditLogModifySchemeOperation(operation, status, reason, SS, peerName, userSID, sanitizedToken, txId, TAuditLogParts()); } } @@ -214,7 +214,7 @@ struct TXxportRecord { TString Status; Ydb::StatusIds::StatusCode DetailedStatus; TString Reason; - TParts AdditionalParts; + TAuditLogParts AdditionalParts; TString StartTime; TString EndTime; TString CloudId; @@ -309,7 +309,7 @@ template <> TParts ImportKindSpecificParts(const Ydb::Import::ImportFromS3Settin } // anonymous namespace template -void _AuditLogXxportStart(const Request& request, const Response& response, const TString& operationName, TParts&& additionalParts, TSchemeShard* SS) { +void _AuditLogXxportStart(const Request& request, const Response& response, const TString& operationName, TAuditLogParts&& additionalParts, TSchemeShard* SS) { TPath databasePath = DatabasePathFromWorkingDir(SS, request.GetDatabaseName()); auto [cloud_id, folder_id, database_id] = GetDatabaseCloudIds(databasePath); auto peerName = NKikimr::NAddressClassifier::ExtractAddress(request.GetPeerName()); @@ -348,7 +348,7 @@ void AuditLogImportStart(const NKikimrImport::TEvCreateImportRequest& request, c } template -void _AuditLogXxportEnd(const Info& info, const TString& operationName, TParts&& additionalParts, TSchemeShard* SS) { +void _AuditLogXxportEnd(const Info& info, const TString& operationName, TAuditLogParts&& additionalParts, TSchemeShard* SS) { const TPath databasePath = TPath::Init(info.DomainPathId, SS); auto [cloud_id, folder_id, database_id] = GetDatabaseCloudIds(databasePath); auto peerName = NKikimr::NAddressClassifier::ExtractAddress(info.PeerName); diff --git a/ydb/core/tx/schemeshard/schemeshard_audit_log.h b/ydb/core/tx/schemeshard/schemeshard_audit_log.h index f476ffecec86..b6a8b1bd6851 100644 --- a/ydb/core/tx/schemeshard/schemeshard_audit_log.h +++ b/ydb/core/tx/schemeshard/schemeshard_audit_log.h @@ -36,12 +36,12 @@ class TSchemeShard; struct TExportInfo; struct TImportInfo; -using TParts = TVector>; +using TAuditLogParts = TVector>; void AuditLogModifySchemeOperation(const NKikimrSchemeOp::TModifyScheme& operation, NKikimrScheme::EStatus status, const TString& reason, TSchemeShard* SS, const TString& peerName, const TString& userSID, const TString& sanitizedToken, - ui64 txId, const TParts& additionalParts); + ui64 txId, const TAuditLogParts& additionalParts); void AuditLogModifySchemeTransaction(const NKikimrScheme::TEvModifySchemeTransaction& request, const NKikimrScheme::TEvModifySchemeTransactionResult& response, TSchemeShard* SS, From 501eaf8c4a8b35f4b6f454feba9b4ba2ffcadea9 Mon Sep 17 00:00:00 2001 From: kungasc Date: Tue, 28 Jan 2025 17:57:29 +0300 Subject: [PATCH 3/3] fix build --- ydb/core/security/ticket_parser_ut.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ydb/core/security/ticket_parser_ut.cpp b/ydb/core/security/ticket_parser_ut.cpp index 442c8628e21b..72d72ff7bd7f 100644 --- a/ydb/core/security/ticket_parser_ut.cpp +++ b/ydb/core/security/ticket_parser_ut.cpp @@ -258,7 +258,7 @@ Y_UNIT_TEST_SUITE(TTicketParserTest) { UNIT_ASSERT(result->Token->IsExist("group2")); UNIT_ASSERT_VALUES_EQUAL(result->Token->GetGroupSIDs().size(), 3); - provider.RemoveGroup({.Group = "group2"}); + provider.RemoveGroup("group2"); provider.CreateGroup({.Group = "group3"}); provider.AddGroupMembership({.Group = "group3", .Member = "user1"}); runtime->Send(new IEventHandle(MakeTicketParserID(), sender, new TEvTicketParser::TEvUpdateLoginSecurityState(provider.GetSecurityState())), 0);