Skip to content

Commit

Permalink
Merge 501eaf8 into 0d933fd
Browse files Browse the repository at this point in the history
  • Loading branch information
@kunga
kunga authored Jan 28, 2025
2 parents 0d933fd + 501eaf8 commit 294fe34
Show file tree
Hide file tree
Showing 11 changed files with 226 additions and 64 deletions.
2 changes: 1 addition & 1 deletion ydb/core/security/ticket_parser_ut.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
95 changes: 61 additions & 34 deletions ydb/core/tx/schemeshard/schemeshard__operation_alter_login.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -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);
Expand All @@ -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;
}
Expand Down Expand Up @@ -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<Schema::LoginSids>().Key(group).Delete();
for (const TString& parent : response.TouchedGroups) {
db.Table<Schema::LoginSidMembers>().Key(parent, group).Delete();
}
result->SetStatus(NKikimrScheme::StatusSuccess);
}
break;
Expand All @@ -211,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) {
Expand Down Expand Up @@ -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);
Expand All @@ -278,7 +258,54 @@ class TAlterLogin: public TSubOperationBase {
return {}; // success
}

void AddIsUserAdmin(const TString& user, NLogin::TLoginProvider& loginProvider, TParts& additionalParts) {
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<Schema::LoginSids>().Key(group).Delete();
for (const TString& parent : removeGroupResponse.TouchedGroups) {
db.Table<Schema::LoginSidMembers>().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 AddAuditLogIsUserAdminPart(const TString& user, NLogin::TLoginProvider& loginProvider, TAuditLogParts& auditLogParts) {
const auto& adminSids = AppData()->AdministrationAllowedSIDs;
bool isAdmin = adminSids.empty();
if (!isAdmin) {
Expand All @@ -292,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<std::chrono::microseconds>(duration).count();
if (time) {
additionalParts.emplace_back("last_login", TInstant::MicroSeconds(time).ToString());
auditLogParts.emplace_back("last_login", TInstant::MicroSeconds(time).ToString());
}
}
};
Expand Down
10 changes: 5 additions & 5 deletions ydb/core/tx/schemeshard/schemeshard_audit_log.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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());
}
}

Expand Down Expand Up @@ -214,7 +214,7 @@ struct TXxportRecord {
TString Status;
Ydb::StatusIds::StatusCode DetailedStatus;
TString Reason;
TParts AdditionalParts;
TAuditLogParts AdditionalParts;
TString StartTime;
TString EndTime;
TString CloudId;
Expand Down Expand Up @@ -309,7 +309,7 @@ template <> TParts ImportKindSpecificParts(const Ydb::Import::ImportFromS3Settin
} // anonymous namespace

template <class Request, class Response>
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());
Expand Down Expand Up @@ -348,7 +348,7 @@ void AuditLogImportStart(const NKikimrImport::TEvCreateImportRequest& request, c
}

template <class Info>
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);
Expand Down
4 changes: 2 additions & 2 deletions ydb/core/tx/schemeshard/schemeshard_audit_log.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ class TSchemeShard;
struct TExportInfo;
struct TImportInfo;

using TParts = TVector<std::pair<TString, TString>>;
using TAuditLogParts = TVector<std::pair<TString, TString>>;

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,
Expand Down
12 changes: 12 additions & 0 deletions ydb/core/tx/schemeshard/ut_helpers/helpers.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<TExpectedResult>& expectedResults) {
auto modifyTx = std::make_unique<TEvSchemeShard::TEvModifySchemeTransaction>(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<TExpectedResult>& expectedResults) {
auto modifyTx = std::make_unique<TEvSchemeShard::TEvModifySchemeTransaction>(txId, TTestTxConfig::SchemeShard);
auto transaction = modifyTx->Record.AddTransaction();
Expand Down
3 changes: 3 additions & 0 deletions ydb/core/tx/schemeshard/ut_helpers/helpers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,9 @@ namespace NSchemeShardUT_Private {

void CreateAlterLoginCreateGroup(TTestActorRuntime& runtime, ui64 txId, const TString& database,
const TString& group, const TVector<TExpectedResult>& expectedResults = {{NKikimrScheme::StatusSuccess}});

void CreateAlterLoginRemoveGroup(TTestActorRuntime& runtime, ui64 txId, const TString& database,
const TString& group, const TVector<TExpectedResult>& expectedResults = {{NKikimrScheme::StatusSuccess}});

void AlterLoginAddGroupMembership(TTestActorRuntime& runtime, ui64 txId, const TString& database,
const TString& member, const TString& group,
Expand Down
12 changes: 11 additions & 1 deletion ydb/core/tx/schemeshard/ut_helpers/ls_checks.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1248,7 +1248,7 @@ TCheckFunc HasGroup(const TString& group, const TSet<TString> members) {
return [=](const NKikimrScheme::TEvDescribeSchemeResult& record) {
std::optional<TSet<TString>> 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);
Expand All @@ -1260,6 +1260,16 @@ TCheckFunc HasGroup(const TString& group, const TSet<TString> 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);
Expand Down
1 change: 1 addition & 0 deletions ydb/core/tx/schemeshard/ut_helpers/ls_checks.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ namespace NLs {
TCheckFunc BackupHistoryCount(ui64 count);

TCheckFunc HasGroup(const TString& group, const TSet<TString> members);
TCheckFunc HasNoGroup(const TString& group);
TCheckFunc HasOwner(const TString& owner);
TCheckFunc HasRight(const TString& right);
TCheckFunc HasNoRight(const TString& right);
Expand Down
Loading

0 comments on commit 294fe34

Please sign in to comment.