Skip to content

Commit 982f3dc

Browse files
andrewstalinandrewstalin
committed
fix review remarks
1 parent 0dc5da9 commit 982f3dc

File tree

3 files changed

+36
-29
lines changed

3 files changed

+36
-29
lines changed

‎ydb/core/tx/schemeshard/schemeshard__operation_alter_login.cpp‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,6 @@ class TAlterLogin: public TSubOperationBase {
3535
auto response = context.SS->LoginProvider.CreateUser(
3636
{.User = createUser.GetUser(), .Password = createUser.GetPassword()});
3737

38-
AddIsUserAdmin(createUser.GetUser(), context.SS->LoginProvider, additionalParts);
39-
4038
if (response.Error) {
4139
result->SetStatus(NKikimrScheme::StatusPreconditionFailed, response.Error);
4240
} else {
@@ -54,34 +52,41 @@ class TAlterLogin: public TSubOperationBase {
5452
}
5553
}
5654
result->SetStatus(NKikimrScheme::StatusSuccess);
55+
56+
AddIsUserAdmin(createUser.GetUser(), context.SS->LoginProvider, additionalParts);
5757
}
5858
break;
5959
}
6060
case NKikimrSchemeOp::TAlterLogin::kModifyUser: {
6161
const auto& modifyUser = alterLogin.GetModifyUser();
62-
63-
AddIsUserAdmin(modifyUser.GetUser(), context.SS->LoginProvider, additionalParts);
64-
6562
auto response = context.SS->LoginProvider.ModifyUser({.User = modifyUser.GetUser(), .Password = modifyUser.GetPassword()});
6663
if (response.Error) {
6764
result->SetStatus(NKikimrScheme::StatusPreconditionFailed, response.Error);
6865
} else {
6966
auto& sid = context.SS->LoginProvider.Sids[modifyUser.GetUser()];
7067
db.Table<Schema::LoginSids>().Key(sid.Name).Update<Schema::LoginSids::SidType, Schema::LoginSids::SidHash>(sid.Type, sid.Hash);
7168
result->SetStatus(NKikimrScheme::StatusSuccess);
69+
70+
AddIsUserAdmin(modifyUser.GetUser(), context.SS->LoginProvider, additionalParts);
71+
AddLastSuccessfulLogin(sid, additionalParts);
7272
}
7373
break;
7474
}
7575
case NKikimrSchemeOp::TAlterLogin::kRemoveUser: {
7676
const auto& removeUser = alterLogin.GetRemoveUser();
7777

78-
AddIsUserAdmin(removeUser.GetUser(), context.SS->LoginProvider, additionalParts);
78+
auto sid = context.SS->LoginProvider.Sids.find(removeUser.GetUser());
79+
if (context.SS->LoginProvider.Sids.end() != sid) {
80+
AddLastSuccessfulLogin(sid->second, additionalParts);
81+
}
7982

8083
auto response = RemoveUser(context, removeUser, db);
8184
if (response.Error) {
8285
result->SetStatus(NKikimrScheme::StatusPreconditionFailed, response.Error);
8386
} else {
8487
result->SetStatus(NKikimrScheme::StatusSuccess);
88+
89+
AddIsUserAdmin(removeUser.GetUser(), context.SS->LoginProvider, additionalParts);
8590
}
8691
break;
8792
}
@@ -182,7 +187,9 @@ class TAlterLogin: public TSubOperationBase {
182187
userSID = context.UserToken->GetUserSID();
183188
sanitizedToken = context.UserToken->GetSanitizedToken();
184189
}
185-
AuditLogModifySchemeTransaction(Transaction, result->Record, context.SS, context.PeerName, userSID, sanitizedToken, ui64(txId), additionalParts);
190+
const auto status = result->Record.GetStatus();
191+
const auto reason = result->Record.HasReason() ? result->Record.GetReason() : TString();
192+
AuditLogModifySchemeOperation(Transaction, status, reason, context.SS, context.PeerName, userSID, sanitizedToken, ui64(txId), additionalParts);
186193
}
187194

188195
if (result->Record.GetStatus() == NKikimrScheme::StatusSuccess) {
@@ -282,7 +289,13 @@ class TAlterLogin: public TSubOperationBase {
282289
}
283290

284291
if (isAdmin) {
285-
additionalParts.emplace_back("account_type", "admin");
292+
additionalParts.emplace_back("login_user_level", "admin");
293+
}
294+
}
295+
296+
void AddLastSuccessfulLogin(NLogin::TLoginProvider::TSidRecord& sid, TParts& additionalParts) {
297+
if (sid.LastSuccessfulLogin) {
298+
additionalParts.emplace_back("last_login", TInstant::FromValue(sid.LastSuccessfulLogin).ToString());
286299
}
287300
}
288301
};

‎ydb/core/tx/schemeshard/schemeshard_audit_log.cpp‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55

66
#include <ydb/library/actors/http/http.h>
77

8-
#include <ydb/core/protos/flat_tx_scheme.pb.h>
98
#include <ydb/core/protos/export.pb.h>
109
#include <ydb/core/protos/import.pb.h>
1110

@@ -81,10 +80,10 @@ TPath DatabasePathFromWorkingDir(TSchemeShard* SS, const TString &opWorkingDir)
8180

8281
} // anonymous namespace
8382

84-
void AuditLogModifySchemeTransaction(const NKikimrSchemeOp::TModifyScheme& operation,
85-
const NKikimrScheme::TEvModifySchemeTransactionResult& response, TSchemeShard* SS,
86-
const TString& peerName, const TString& userSID, const TString& sanitizedToken,
87-
ui64 txId, const TParts& additionalParts) {
83+
void AuditLogModifySchemeOperation(const NKikimrSchemeOp::TModifyScheme& operation,
84+
NKikimrScheme::EStatus status, const TString& reason, TSchemeShard* SS,
85+
const TString& peerName, const TString& userSID, const TString& sanitizedToken,
86+
ui64 txId, const TParts& additionalParts) {
8887
auto logEntry = MakeAuditLogFragment(operation);
8988

9089
TPath databasePath = DatabasePathFromWorkingDir(SS, operation.GetWorkingDir());
@@ -100,9 +99,9 @@ void AuditLogModifySchemeTransaction(const NKikimrSchemeOp::TModifyScheme& opera
10099
AUDIT_PART("database", (!databasePath.IsEmpty() ? databasePath.GetDomainPathString() : EmptyValue))
101100
AUDIT_PART("operation", logEntry.Operation)
102101
AUDIT_PART("paths", RenderList(logEntry.Paths), !logEntry.Paths.empty())
103-
AUDIT_PART("status", GeneralStatus(response.GetStatus()))
104-
AUDIT_PART("detailed_status", NKikimrScheme::EStatus_Name(response.GetStatus()))
105-
AUDIT_PART("reason", response.GetReason(), response.HasReason())
102+
AUDIT_PART("status", GeneralStatus(status))
103+
AUDIT_PART("detailed_status", NKikimrScheme::EStatus_Name(status))
104+
AUDIT_PART("reason", reason, !reason.empty())
106105

107106
for (const auto& [name, value] : additionalParts) {
108107
AUDIT_PART(name, (!value.empty() ? value : EmptyValue))
@@ -144,12 +143,14 @@ void AuditLogModifySchemeTransaction(const NKikimrScheme::TEvModifySchemeTransac
144143
// Each TEvModifySchemeTransaction.Transaction is a self sufficient operation and should be logged independently
145144
// (even if it was packed into a single TxProxy transaction with some other operations).
146145
const auto txId = request.GetTxId();
146+
const auto status = response.GetStatus();
147+
const auto reason = response.HasReason() ? response.GetReason() : TString();
147148
for (const auto& operation : request.GetTransaction()) {
148149
const auto type = operation.GetOperationType();
149150
if (NKikimrSchemeOp::EOperationType::ESchemeOpAlterLogin == type) {
150151
continue;
151152
}
152-
AuditLogModifySchemeTransaction(operation, response, SS, peerName, userSID, sanitizedToken, txId, TParts());
153+
AuditLogModifySchemeOperation(operation, status, reason, SS, peerName, userSID, sanitizedToken, txId, TParts());
153154
}
154155
}
155156

‎ydb/core/tx/schemeshard/schemeshard_audit_log.h‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,7 @@
11
#pragma once
22

33
#include <util/generic/string.h>
4-
5-
namespace NKikimrScheme {
6-
class TEvModifySchemeTransaction;
7-
class TEvModifySchemeTransactionResult;
8-
9-
class TEvLogin;
10-
class TEvLoginResult;
11-
}
4+
#include <ydb/core/protos/flat_tx_scheme.pb.h>
125

136
namespace NKikimrExport {
147
class TEvCreateExportRequest;
@@ -36,10 +29,10 @@ struct TImportInfo;
3629

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

39-
void AuditLogModifySchemeTransaction(const NKikimrSchemeOp::TModifyScheme& operation,
40-
const NKikimrScheme::TEvModifySchemeTransactionResult& response, TSchemeShard* SS,
41-
const TString& peerName, const TString& userSID, const TString& sanitizedToken,
42-
ui64 txId, const TParts& additionalParts);
32+
void AuditLogModifySchemeOperation(const NKikimrSchemeOp::TModifyScheme& operation,
33+
NKikimrScheme::EStatus status, const TString& reason, TSchemeShard* SS,
34+
const TString& peerName, const TString& userSID, const TString& sanitizedToken,
35+
ui64 txId, const TParts& additionalParts);
4336

4437
void AuditLogModifySchemeTransaction(const NKikimrScheme::TEvModifySchemeTransaction& request,
4538
const NKikimrScheme::TEvModifySchemeTransactionResult& response, TSchemeShard* SS,

0 commit comments

Comments
 (0)