From 5d75202cc65f83f6ed868eaad6c00ca9f12bb791 Mon Sep 17 00:00:00 2001 From: flown4qqqq Date: Thu, 10 Apr 2025 16:51:57 +0300 Subject: [PATCH 1/2] Clarifying the cause of the error after an authentication attempt (#17012) --- ydb/core/tx/schemeshard/ut_login/ut_login.cpp | 26 +++++++++---------- ydb/library/login/login.cpp | 7 ++++- ydb/library/login/login_ut.cpp | 2 +- 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/ydb/core/tx/schemeshard/ut_login/ut_login.cpp b/ydb/core/tx/schemeshard/ut_login/ut_login.cpp index e904bcb126ef..600b340a54ef 100644 --- a/ydb/core/tx/schemeshard/ut_login/ut_login.cpp +++ b/ydb/core/tx/schemeshard/ut_login/ut_login.cpp @@ -574,7 +574,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) { UNIT_ASSERT_VALUES_EQUAL(resultLogin1.error(), ""); ChangeIsEnabledUser(runtime, ++txId, "/MyRoot", "user1", false); auto resultLogin2 = Login(runtime, "user1", "123"); - UNIT_ASSERT_VALUES_EQUAL(resultLogin2.error(), "User user1 is not permitted to log in"); + UNIT_ASSERT_VALUES_EQUAL(resultLogin2.error(), "User user1 login denied: account is blocked"); ChangeIsEnabledUser(runtime, ++txId, "/MyRoot", "user1", true); auto resultLogin3 = Login(runtime, "user1", "123"); UNIT_ASSERT_VALUES_EQUAL(resultLogin3.error(), ""); @@ -610,7 +610,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) { Sleep(TDuration::Seconds(4)); auto resultLogin = Login(runtime, "user1", "123"); - UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), "User user1 is not permitted to log in"); + UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), "User user1 login denied: account is blocked"); } Y_UNIT_TEST(ChangeAcceptablePasswordParameters) { @@ -854,11 +854,11 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) { UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), "Invalid password"); } resultLogin = Login(runtime, "user1", TStringBuilder() << "wrongpassword" << accountLockoutConfig.GetAttemptThreshold()); - UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), TStringBuilder() << "User user1 is not permitted to log in"); + UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), TStringBuilder() << "User user1 login denied: too many failed password attempts"); // Also do not accept correct password resultLogin = Login(runtime, "user1", "password1"); - UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), TStringBuilder() << "User user1 is not permitted to log in"); + UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), TStringBuilder() << "User user1 login denied: too many failed password attempts"); { auto describe = DescribePath(runtime, TTestTxConfig::SchemeShard, "/MyRoot"); @@ -951,7 +951,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) { UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), "Invalid password"); } resultLogin = Login(runtime, "user1", TStringBuilder() << "wrongpassword" << accountLockoutConfig.GetAttemptThreshold()); - UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), TStringBuilder() << "User user1 is not permitted to log in"); + UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), TStringBuilder() << "User user1 login denied: too many failed password attempts"); { auto describe = DescribePath(runtime, TTestTxConfig::SchemeShard, "/MyRoot"); @@ -982,7 +982,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) { } resultLogin = Login(runtime, "user2", TStringBuilder() << "wrongpassword2" << newAttemptThreshold); // User is not permitted to log in after 6 attempts - UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), TStringBuilder() << "User user2 is not permitted to log in"); + UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), TStringBuilder() << "User user2 login denied: too many failed password attempts"); { auto describe = DescribePath(runtime, TTestTxConfig::SchemeShard, "/MyRoot"); @@ -993,7 +993,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) { // After 4 seconds user2 must be locked out Sleep(TDuration::Seconds(4)); resultLogin = Login(runtime, "user2", "wrongpassword28"); - UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), TStringBuilder() << "User user2 is not permitted to log in"); + UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), TStringBuilder() << "User user2 login denied: too many failed password attempts"); // After 7 seconds user2 must be unlocked Sleep(TDuration::Seconds(8)); @@ -1026,11 +1026,11 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) { UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), "Invalid password"); } resultLogin = Login(runtime, "user1", TStringBuilder() << "wrongpassword" << accountLockoutConfig.GetAttemptThreshold()); - UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), TStringBuilder() << "User user1 is not permitted to log in"); + UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), TStringBuilder() << "User user1 login denied: too many failed password attempts"); // Also do not accept correct password resultLogin = Login(runtime, "user1", "password1"); - UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), TStringBuilder() << "User user1 is not permitted to log in"); + UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), TStringBuilder() << "User user1 login denied: too many failed password attempts"); } Y_UNIT_TEST(CheckThatLockedOutParametersIsRestoredFromLocalDb) { @@ -1067,7 +1067,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) { UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), "Invalid password"); } resultLogin = Login(runtime, "user1", TStringBuilder() << "wrongpassword" << accountLockoutConfig.GetAttemptThreshold()); - UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), TStringBuilder() << "User user1 is not permitted to log in"); + UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), TStringBuilder() << "User user1 login denied: too many failed password attempts"); { auto describe = DescribePath(runtime, TTestTxConfig::SchemeShard, "/MyRoot"); @@ -1079,7 +1079,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) { // After reboot schemeshard user1 must be locked out resultLogin = Login(runtime, "user1", TStringBuilder() << "wrongpassword" << accountLockoutConfig.GetAttemptThreshold()); - UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), TStringBuilder() << "User user1 is not permitted to log in"); + UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), TStringBuilder() << "User user1 login denied: too many failed password attempts"); // User1 must be unlocked in 1 second after reboot schemeshard Sleep(TDuration::Seconds(2)); @@ -1133,9 +1133,9 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) { CreateAlterLoginCreateUser(runtime, ++txId, "/MyRoot", userName, userPassword); blockUser(); - loginUser(TStringBuilder() << "User " << userName << " is not permitted to log in"); + loginUser(TStringBuilder() << "User " << userName << " login denied: too many failed password attempts"); reboot(); - loginUser(TStringBuilder() << "User " << userName << " is not permitted to log in"); + loginUser(TStringBuilder() << "User " << userName << " login denied: too many failed password attempts"); ChangeIsEnabledUser(runtime, ++txId, "/MyRoot", userName, true); loginUser(""); diff --git a/ydb/library/login/login.cpp b/ydb/library/login/login.cpp index 9a6c8ecb46d6..199201b8705d 100644 --- a/ydb/library/login/login.cpp +++ b/ydb/library/login/login.cpp @@ -420,7 +420,12 @@ TLoginProvider::TCheckLockOutResponse TLoginProvider::CheckLockOutUser(const TCh response.Status = TCheckLockOutResponse::EStatus::RESET; } else { response.Status = TCheckLockOutResponse::EStatus::SUCCESS; - response.Error = TStringBuilder() << "User " << request.User << " is not permitted to log in"; + + if (!sid.IsEnabled) { + response.Error = TStringBuilder() << "User " << request.User << " login denied: account is blocked"; + } else { + response.Error = TStringBuilder() << "User " << request.User << " login denied: too many failed password attempts"; + } } return response; } else if (ShouldResetFailedAttemptCount(sid)) { diff --git a/ydb/library/login/login_ut.cpp b/ydb/library/login/login_ut.cpp index d4a1f572c626..5aa016bae864 100644 --- a/ydb/library/login/login_ut.cpp +++ b/ydb/library/login/login_ut.cpp @@ -563,7 +563,7 @@ Y_UNIT_TEST_SUITE(Login) { UNIT_ASSERT_VALUES_EQUAL(provider.IsLockedOut(provider.Sids[userName]), true); auto checkLockoutResponse = provider.CheckLockOutUser({.User = userName}); UNIT_ASSERT_EQUAL(checkLockoutResponse.Status, TLoginProvider::TCheckLockOutResponse::EStatus::SUCCESS); - UNIT_ASSERT_STRING_CONTAINS(checkLockoutResponse.Error, TStringBuilder() << "User " << userName << " is not permitted to log in"); + UNIT_ASSERT_STRING_CONTAINS(checkLockoutResponse.Error, TStringBuilder() << "User " << userName << " login denied: too many failed password attempts"); } { From 9dfe69c0fabe7902a0aa1a5940da214b44253138 Mon Sep 17 00:00:00 2001 From: flown4qqqq Date: Thu, 10 Apr 2025 16:38:43 +0300 Subject: [PATCH 2/2] Add operation details in audit logging of operation 'MODIFY USER' (#16989) --- .../tx/schemeshard/schemeshard_audit_log.cpp | 24 ++--- .../schemeshard_audit_log_fragment.cpp | 75 ++++++++++---- .../schemeshard_audit_log_fragment.h | 1 + .../tx/schemeshard/ut_helpers/helpers.cpp | 29 +++++- ydb/core/tx/schemeshard/ut_helpers/helpers.h | 8 ++ ydb/core/tx/schemeshard/ut_login/ut_login.cpp | 98 +++++++++++++++++-- 6 files changed, 193 insertions(+), 42 deletions(-) diff --git a/ydb/core/tx/schemeshard/schemeshard_audit_log.cpp b/ydb/core/tx/schemeshard/schemeshard_audit_log.cpp index aa66cfb98da4..a3608413dc19 100644 --- a/ydb/core/tx/schemeshard/schemeshard_audit_log.cpp +++ b/ydb/core/tx/schemeshard/schemeshard_audit_log.cpp @@ -128,9 +128,9 @@ void AuditLogModifySchemeOperation(const NKikimrSchemeOp::TModifyScheme& operati AUDIT_PART(name, (!value.empty() ? value : EmptyValue)) } - AUDIT_PART("cloud_id", cloud_id, !cloud_id.empty()); - AUDIT_PART("folder_id", folder_id, !folder_id.empty()); - AUDIT_PART("resource_id", database_id, !database_id.empty()); + AUDIT_PART("cloud_id", cloud_id, !cloud_id.empty()) + AUDIT_PART("folder_id", folder_id, !folder_id.empty()) + AUDIT_PART("resource_id", database_id, !database_id.empty()) // Additionally: @@ -140,21 +140,23 @@ void AuditLogModifySchemeOperation(const NKikimrSchemeOp::TModifyScheme& operati // 1. explicit operation ESchemeOpModifyACL -- to modify ACL on a path // 2. ESchemeOpMkDir or ESchemeOpCreate* operations -- to set rights to newly created paths/entities // 3. ESchemeOpCopyTable -- to be checked against acl size limit, not to be applied in any way - AUDIT_PART("new_owner", logEntry.NewOwner, !logEntry.NewOwner.empty()); - AUDIT_PART("acl_add", RenderList(logEntry.ACLAdd), !logEntry.ACLAdd.empty()); - AUDIT_PART("acl_remove", RenderList(logEntry.ACLRemove), !logEntry.ACLRemove.empty()); + AUDIT_PART("new_owner", logEntry.NewOwner, !logEntry.NewOwner.empty()) + AUDIT_PART("acl_add", RenderList(logEntry.ACLAdd), !logEntry.ACLAdd.empty()) + AUDIT_PART("acl_remove", RenderList(logEntry.ACLRemove), !logEntry.ACLRemove.empty()) // AlterUserAttributes. // 1. explicit operation ESchemeOpAlterUserAttributes -- to modify user attributes on a path // 2. ESchemeOpMkDir or some ESchemeOpCreate* operations -- to set user attributes for newly created paths/entities - AUDIT_PART("user_attrs_add", RenderList(logEntry.UserAttrsAdd), !logEntry.UserAttrsAdd.empty()); - AUDIT_PART("user_attrs_remove", RenderList(logEntry.UserAttrsRemove), !logEntry.UserAttrsRemove.empty()); + AUDIT_PART("user_attrs_add", RenderList(logEntry.UserAttrsAdd), !logEntry.UserAttrsAdd.empty()) + AUDIT_PART("user_attrs_remove", RenderList(logEntry.UserAttrsRemove), !logEntry.UserAttrsRemove.empty()) // AlterLogin. // explicit operation ESchemeOpAlterLogin -- to modify user and groups - AUDIT_PART("login_user", logEntry.LoginUser); - AUDIT_PART("login_group", logEntry.LoginGroup); - AUDIT_PART("login_member", logEntry.LoginMember); + AUDIT_PART("login_user", logEntry.LoginUser) + AUDIT_PART("login_group", logEntry.LoginGroup) + AUDIT_PART("login_member", logEntry.LoginMember) + + AUDIT_PART("login_user_change", RenderList(logEntry.LoginUserChange), logEntry.LoginUserChange) ); } diff --git a/ydb/core/tx/schemeshard/schemeshard_audit_log_fragment.cpp b/ydb/core/tx/schemeshard/schemeshard_audit_log_fragment.cpp index 97506b73eb95..7920ee2de4e2 100644 --- a/ydb/core/tx/schemeshard/schemeshard_audit_log_fragment.cpp +++ b/ydb/core/tx/schemeshard/schemeshard_audit_log_fragment.cpp @@ -678,40 +678,74 @@ struct TChangeLogin { TString LoginUser; TString LoginGroup; TString LoginMember; + TVector LoginUserChange; }; TChangeLogin ExtractLoginChange(const NKikimrSchemeOp::TModifyScheme& tx) { if (tx.HasAlterLogin()) { + const auto& alter = tx.GetAlterLogin(); + TChangeLogin result; switch (tx.GetAlterLogin().GetAlterCase()) { - case NKikimrSchemeOp::TAlterLogin::kCreateUser: - result.LoginUser = tx.GetAlterLogin().GetCreateUser().GetUser(); + case NKikimrSchemeOp::TAlterLogin::kCreateUser: { + result.LoginUser = alter.GetCreateUser().GetUser(); break; - case NKikimrSchemeOp::TAlterLogin::kModifyUser: - result.LoginUser = tx.GetAlterLogin().GetModifyUser().GetUser(); + } + + case NKikimrSchemeOp::TAlterLogin::kModifyUser: { + const auto& modify = alter.GetModifyUser(); + result.LoginUser = modify.GetUser(); + + if (modify.HasPassword()) { // there is no difference beetwen password and password's hash + result.LoginUserChange.push_back("password"); + } + + if (modify.HasCanLogin() && modify.GetCanLogin()) { + result.LoginUserChange.push_back("unblocking"); + } + + if (modify.HasCanLogin() && !modify.GetCanLogin()) { + result.LoginUserChange.push_back("blocking"); + } + break; - case NKikimrSchemeOp::TAlterLogin::kRemoveUser: - result.LoginUser = tx.GetAlterLogin().GetRemoveUser().GetUser(); + } + + case NKikimrSchemeOp::TAlterLogin::kRemoveUser: { + result.LoginUser = alter.GetRemoveUser().GetUser(); break; - case NKikimrSchemeOp::TAlterLogin::kCreateGroup: - result.LoginGroup = tx.GetAlterLogin().GetCreateGroup().GetGroup(); + } + + case NKikimrSchemeOp::TAlterLogin::kCreateGroup: { + result.LoginGroup = alter.GetCreateGroup().GetGroup(); break; - case NKikimrSchemeOp::TAlterLogin::kAddGroupMembership: - result.LoginGroup = tx.GetAlterLogin().GetAddGroupMembership().GetGroup(); - result.LoginMember = tx.GetAlterLogin().GetAddGroupMembership().GetMember(); + } + + case NKikimrSchemeOp::TAlterLogin::kAddGroupMembership: { + result.LoginGroup = alter.GetAddGroupMembership().GetGroup(); + result.LoginMember = alter.GetAddGroupMembership().GetMember(); break; - case NKikimrSchemeOp::TAlterLogin::kRemoveGroupMembership: - result.LoginGroup = tx.GetAlterLogin().GetRemoveGroupMembership().GetGroup(); - result.LoginMember = tx.GetAlterLogin().GetRemoveGroupMembership().GetMember(); + } + + case NKikimrSchemeOp::TAlterLogin::kRemoveGroupMembership: { + result.LoginGroup = alter.GetRemoveGroupMembership().GetGroup(); + result.LoginMember = alter.GetRemoveGroupMembership().GetMember(); break; - case NKikimrSchemeOp::TAlterLogin::kRenameGroup: - result.LoginGroup = tx.GetAlterLogin().GetRenameGroup().GetGroup(); + } + + case NKikimrSchemeOp::TAlterLogin::kRenameGroup: { + result.LoginGroup = alter.GetRenameGroup().GetGroup(); break; - case NKikimrSchemeOp::TAlterLogin::kRemoveGroup: - result.LoginGroup = tx.GetAlterLogin().GetRemoveGroup().GetGroup(); + } + + case NKikimrSchemeOp::TAlterLogin::kRemoveGroup: { + result.LoginGroup = alter.GetRemoveGroup().GetGroup(); break; - default: + } + + default: { Y_ABORT("switch should cover all operation types"); + } } return result; } @@ -725,7 +759,7 @@ namespace NKikimr::NSchemeShard { TAuditLogFragment MakeAuditLogFragment(const NKikimrSchemeOp::TModifyScheme& tx) { auto [aclAdd, aclRemove] = ExtractACLChange(tx); auto [userAttrsAdd, userAttrsRemove] = ExtractUserAttrChange(tx); - auto [loginUser, loginGroup, loginMember] = ExtractLoginChange(tx); + auto [loginUser, loginGroup, loginMember, loginUserChange] = ExtractLoginChange(tx); return { .Operation = DefineUserOperationName(tx), @@ -738,6 +772,7 @@ TAuditLogFragment MakeAuditLogFragment(const NKikimrSchemeOp::TModifyScheme& tx) .LoginUser = loginUser, .LoginGroup = loginGroup, .LoginMember = loginMember, + .LoginUserChange = loginUserChange }; } diff --git a/ydb/core/tx/schemeshard/schemeshard_audit_log_fragment.h b/ydb/core/tx/schemeshard/schemeshard_audit_log_fragment.h index e6347fb73255..52258901427d 100644 --- a/ydb/core/tx/schemeshard/schemeshard_audit_log_fragment.h +++ b/ydb/core/tx/schemeshard/schemeshard_audit_log_fragment.h @@ -21,6 +21,7 @@ struct TAuditLogFragment { TString LoginUser; TString LoginGroup; TString LoginMember; + TVector LoginUserChange; }; TAuditLogFragment MakeAuditLogFragment(const NKikimrSchemeOp::TModifyScheme& tx); diff --git a/ydb/core/tx/schemeshard/ut_helpers/helpers.cpp b/ydb/core/tx/schemeshard/ut_helpers/helpers.cpp index af0e9352663d..8e8e0aa364f7 100644 --- a/ydb/core/tx/schemeshard/ut_helpers/helpers.cpp +++ b/ydb/core/tx/schemeshard/ut_helpers/helpers.cpp @@ -2059,7 +2059,7 @@ namespace NSchemeShardUT_Private { return event->Record; } - void ChangeIsEnabledUser(TTestActorRuntime& runtime, ui64 txId, const TString& database, const TString& user, bool isEnabled) { + void ModifyUser(TTestActorRuntime& runtime, ui64 txId, const TString& database, std::function&& initiator) { auto modifyTx = std::make_unique(txId, TTestTxConfig::SchemeShard); auto transaction = modifyTx->Record.AddTransaction(); transaction->SetWorkingDir(database); @@ -2067,12 +2067,33 @@ namespace NSchemeShardUT_Private { auto alterUser = transaction->MutableAlterLogin()->MutableModifyUser(); - alterUser->SetUser(user); - alterUser->SetCanLogin(isEnabled); + initiator(alterUser); AsyncSend(runtime, TTestTxConfig::SchemeShard, modifyTx.release()); TAutoPtr handle; - [[maybe_unused]]auto event = runtime.GrabEdgeEvent(handle); // wait() + [[maybe_unused]]auto event = runtime.GrabEdgeEvent(handle); // wait() + } + + void ChangeIsEnabledUser(TTestActorRuntime& runtime, ui64 txId, const TString& database, const TString& user, bool isEnabled) { + ModifyUser(runtime, txId, database, [user, isEnabled](auto* alterUser) { + alterUser->SetUser(std::move(user)); + alterUser->SetCanLogin(isEnabled); + }); + } + + void ChangePasswordUser(TTestActorRuntime& runtime, ui64 txId, const TString& database, const TString& user, const TString& password) { + ModifyUser(runtime, txId, database, [user, password](auto* alterUser) { + alterUser->SetUser(std::move(user)); + alterUser->SetPassword(std::move(password)); + }); + } + + void ChangePasswordHashUser(TTestActorRuntime& runtime, ui64 txId, const TString& database, const TString& user, const TString& hash) { + ModifyUser(runtime, txId, database, [user, hash](auto* alterUser) { + alterUser->SetUser(std::move(user)); + alterUser->SetPassword(std::move(hash)); + alterUser->SetIsHashedPassword(true); + }); } // class TFakeDataReq { diff --git a/ydb/core/tx/schemeshard/ut_helpers/helpers.h b/ydb/core/tx/schemeshard/ut_helpers/helpers.h index 3b04a896ad75..ba5babd48e26 100644 --- a/ydb/core/tx/schemeshard/ut_helpers/helpers.h +++ b/ydb/core/tx/schemeshard/ut_helpers/helpers.h @@ -551,9 +551,17 @@ namespace NSchemeShardUT_Private { NKikimrScheme::TEvLoginResult Login(TTestActorRuntime& runtime, const TString& user, const TString& password); + void ModifyUser(TTestActorRuntime& runtime, ui64 txId, const TString& database, std::function&& initiator); + void ChangeIsEnabledUser(TTestActorRuntime& runtime, ui64 txId, const TString& database, const TString& user, bool isEnabled); + void ChangePasswordUser(TTestActorRuntime& runtime, ui64 txId, const TString& database, + const TString& user, const TString& password); + + void ChangePasswordHashUser(TTestActorRuntime& runtime, ui64 txId, const TString& database, + const TString& user, const TString& hash); + // Mimics data query to a single table with multiple partitions class TFakeDataReq { public: diff --git a/ydb/core/tx/schemeshard/ut_login/ut_login.cpp b/ydb/core/tx/schemeshard/ut_login/ut_login.cpp index 600b340a54ef..348a572e2f6d 100644 --- a/ydb/core/tx/schemeshard/ut_login/ut_login.cpp +++ b/ydb/core/tx/schemeshard/ut_login/ut_login.cpp @@ -1197,7 +1197,7 @@ NHttp::THttpIncomingRequestPtr MakeLogoutRequest(const TString& cookieName, cons Y_UNIT_TEST_SUITE(TWebLoginService) { void AuditLogLoginTest(TTestBasicRuntime& runtime, bool isUserAdmin) { std::vector lines; - runtime.AuditLogBackends = std::move(CreateTestAuditLogBackends(lines)); + runtime.AuditLogBackends = CreateTestAuditLogBackends(lines); TTestEnv env(runtime); UNIT_ASSERT_VALUES_EQUAL(lines.size(), 1); // alter root subdomain @@ -1267,7 +1267,7 @@ Y_UNIT_TEST_SUITE(TWebLoginService) { Y_UNIT_TEST(AuditLogLoginBadPassword) { TTestBasicRuntime runtime; std::vector lines; - runtime.AuditLogBackends = std::move(CreateTestAuditLogBackends(lines)); + runtime.AuditLogBackends = CreateTestAuditLogBackends(lines); TTestEnv env(runtime); UNIT_ASSERT_VALUES_EQUAL(lines.size(), 1); // alter root subdomain @@ -1309,7 +1309,7 @@ Y_UNIT_TEST_SUITE(TWebLoginService) { Y_UNIT_TEST(AuditLogLdapLoginSuccess) { TTestBasicRuntime runtime; std::vector lines; - runtime.AuditLogBackends = std::move(CreateTestAuditLogBackends(lines)); + runtime.AuditLogBackends = CreateTestAuditLogBackends(lines); // Enable and configure ldap auth runtime.SetLogPriority(NKikimrServices::LDAP_AUTH_PROVIDER, NActors::NLog::PRI_DEBUG); TTestEnv env(runtime); @@ -1405,7 +1405,7 @@ Y_UNIT_TEST_SUITE(TWebLoginService) { Y_UNIT_TEST(AuditLogLdapLoginBadPassword) { TTestBasicRuntime runtime; std::vector lines; - runtime.AuditLogBackends = std::move(CreateTestAuditLogBackends(lines)); + runtime.AuditLogBackends =CreateTestAuditLogBackends(lines); // Enable and configure ldap auth runtime.SetLogPriority(NKikimrServices::LDAP_AUTH_PROVIDER, NActors::NLog::PRI_DEBUG); TTestEnv env(runtime); @@ -1500,7 +1500,7 @@ Y_UNIT_TEST_SUITE(TWebLoginService) { Y_UNIT_TEST(AuditLogLdapLoginBadUser) { TTestBasicRuntime runtime; std::vector lines; - runtime.AuditLogBackends = std::move(CreateTestAuditLogBackends(lines)); + runtime.AuditLogBackends =CreateTestAuditLogBackends(lines); // Enable and configure ldap auth runtime.SetLogPriority(NKikimrServices::LDAP_AUTH_PROVIDER, NActors::NLog::PRI_DEBUG); TTestEnv env(runtime); @@ -1596,7 +1596,7 @@ Y_UNIT_TEST_SUITE(TWebLoginService) { Y_UNIT_TEST(AuditLogLdapLoginBadBind) { TTestBasicRuntime runtime; std::vector lines; - runtime.AuditLogBackends = std::move(CreateTestAuditLogBackends(lines)); + runtime.AuditLogBackends =CreateTestAuditLogBackends(lines); // Enable and configure ldap auth runtime.SetLogPriority(NKikimrServices::LDAP_AUTH_PROVIDER, NActors::NLog::PRI_DEBUG); TTestEnv env(runtime); @@ -1691,7 +1691,7 @@ Y_UNIT_TEST_SUITE(TWebLoginService) { Y_UNIT_TEST(AuditLogLogout) { TTestBasicRuntime runtime; std::vector lines; - runtime.AuditLogBackends = std::move(CreateTestAuditLogBackends(lines)); + runtime.AuditLogBackends =CreateTestAuditLogBackends(lines); TTestEnv env(runtime); // Add ticket parser to the mix @@ -1792,4 +1792,88 @@ Y_UNIT_TEST_SUITE(TWebLoginService) { UNIT_ASSERT(last.find("sanitized_token={none}") == std::string::npos); } } + + Y_UNIT_TEST(AuditLogCreateModifyUser) { + TTestBasicRuntime runtime; + std::vector lines; + runtime.AuditLogBackends = CreateTestAuditLogBackends(lines); + TTestEnv env(runtime); + UNIT_ASSERT_VALUES_EQUAL(lines.size(), 1); + ui64 txId = 100; + + TString database = "/MyRoot"; + TString user = "user1"; + TString password = "password1"; + TString newPassword = "password2"; + TString hash = R"( + { + "hash": "p4ffeMugohqyBwyckYCK1TjJfz3LIHbKiGL+t+oEhzw=", + "salt": "U+tzBtgo06EBQCjlARA6Jg==", + "type": "argon2id" + } + )"; + + auto check = [&](const TString& operation, const TVector& details = {}) { + auto render = [](const TVector& list) { + auto result = TStringBuilder(); + result << "[" << JoinStrings(list.begin(), list.end(), ", ") << "]"; + return result; + }; + + auto last = FindAuditLine(lines, Sprintf("tx_id=%u", txId)); + + UNIT_ASSERT_STRING_CONTAINS(last, Sprintf("operation=%s", operation.c_str())); + + if (!details.empty()) { + UNIT_ASSERT_STRING_CONTAINS(last, Sprintf("login_user_change=%s", render(details).c_str())); + } + + UNIT_ASSERT_STRING_CONTAINS(last, "component=schemeshard"); + UNIT_ASSERT_STRING_CONTAINS(last, Sprintf("database=%s", database.c_str())); + UNIT_ASSERT_STRING_CONTAINS(last, "status=SUCCESS"); + UNIT_ASSERT_STRING_CONTAINS(last, "detailed_status=StatusSuccess"); + UNIT_ASSERT_STRING_CONTAINS(last, "login_user_level=admin"); + UNIT_ASSERT_STRING_CONTAINS(last, Sprintf("login_user=%s", user.c_str())); + }; + + CreateAlterLoginCreateUser(runtime, ++txId, database, user, password); + { + UNIT_ASSERT_VALUES_EQUAL(lines.size(), 2); + check("CREATE USER"); + } + + ChangePasswordUser(runtime, ++txId, database, user, newPassword); + { + UNIT_ASSERT_VALUES_EQUAL(lines.size(), 3); + check("MODIFY USER", {"password"}); + } + + ChangeIsEnabledUser(runtime, ++txId, database, user, false); + { + UNIT_ASSERT_VALUES_EQUAL(lines.size(), 4); + check("MODIFY USER", {"blocking"}); + } + + ChangeIsEnabledUser(runtime, ++txId, database, user, true); + { + UNIT_ASSERT_VALUES_EQUAL(lines.size(), 5); + check("MODIFY USER", {"unblocking"}); + } + + ChangePasswordHashUser(runtime, ++txId, database, user, hash); + { + UNIT_ASSERT_VALUES_EQUAL(lines.size(), 6); + check("MODIFY USER", {"password"}); + } + + ModifyUser(runtime, ++txId, database, [user, password, isEnabled = false](auto* alterUser) { + alterUser->SetUser(std::move(user)); + alterUser->SetCanLogin(isEnabled); + alterUser->SetPassword(std::move(password)); + }); + { + UNIT_ASSERT_VALUES_EQUAL(lines.size(), 7); + check("MODIFY USER", {"password", "blocking"}); + } + } }