diff --git a/ydb/core/tx/schemeshard/schemeshard__operation_alter_login.cpp b/ydb/core/tx/schemeshard/schemeshard__operation_alter_login.cpp index 1c37bddba91d..4a2e4f4bd320 100644 --- a/ydb/core/tx/schemeshard/schemeshard__operation_alter_login.cpp +++ b/ydb/core/tx/schemeshard/schemeshard__operation_alter_login.cpp @@ -88,7 +88,8 @@ class TAlterLogin: public TSubOperationBase { auto& sid = context.SS->LoginProvider.Sids[modifyUser.GetUser()]; db.Table().Key(sid.Name).Update(sid.Type, sid.PasswordHash, sid.IsEnabled); + Schema::LoginSids::IsEnabled, + Schema::LoginSids::FailedAttemptCount>(sid.Type, sid.PasswordHash, sid.IsEnabled, sid.FailedLoginAttemptCount); result->SetStatus(NKikimrScheme::StatusSuccess); AddIsUserAdmin(modifyUser.GetUser(), context.SS->LoginProvider, additionalParts); diff --git a/ydb/core/tx/schemeshard/ut_login/ut_login.cpp b/ydb/core/tx/schemeshard/ut_login/ut_login.cpp index ded24b5b6167..3957bff01f85 100644 --- a/ydb/core/tx/schemeshard/ut_login/ut_login.cpp +++ b/ydb/core/tx/schemeshard/ut_login/ut_login.cpp @@ -1092,6 +1092,58 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) { CheckToken(resultLogin.token(), describe, "user1"); } } + + Y_UNIT_TEST(ResetFailedAttemptCountAfterModifyUser) { + TTestBasicRuntime runtime; + runtime.AddAppDataInit([] (ui32 nodeIdx, NKikimr::TAppData& appData) { + Y_UNUSED(nodeIdx); + auto accountLockout = appData.AuthConfig.MutableAccountLockout(); + accountLockout->SetAttemptThreshold(4); + accountLockout->SetAttemptResetDuration("3s"); + }); + + TTestEnv env(runtime); + auto accountLockoutConfig = runtime.GetAppData().AuthConfig.GetAccountLockout(); + ui64 txId = 100; + { + auto describe = DescribePath(runtime, TTestTxConfig::SchemeShard, "/MyRoot"); + CheckSecurityState(describe, {.PublicKeysSize = 0, .SidsSize = 0}); + } + + TString userName = "user1"; + TString userPassword = "password1"; + + auto blockUser = [&]() { + for (size_t attempt = 0; attempt < accountLockoutConfig.GetAttemptThreshold(); attempt++) { + auto resultLogin = Login(runtime, userName, TStringBuilder() << "wrongpassword" << attempt); + UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), "Invalid password"); + } + }; + + auto loginUser = [&](TString error) { + auto resultLogin = Login(runtime, userName, userPassword); + UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), error); + }; + + auto reboot = [&]() { + TActorId sender = runtime.AllocateEdgeActor(); + RebootTablet(runtime, TTestTxConfig::SchemeShard, sender); + }; + + CreateAlterLoginCreateUser(runtime, ++txId, "/MyRoot", userName, userPassword); + + blockUser(); + loginUser(TStringBuilder() << "User " << userName << " is not permitted to log in"); + reboot(); + loginUser(TStringBuilder() << "User " << userName << " is not permitted to log in"); + ChangeIsEnabledUser(runtime, ++txId, "/MyRoot", userName, true); + loginUser(""); + + blockUser(); + ChangeIsEnabledUser(runtime, ++txId, "/MyRoot", userName, true); + reboot(); + loginUser(""); + } } namespace NSchemeShardUT_Private { diff --git a/ydb/library/login/login.cpp b/ydb/library/login/login.cpp index b04a7837701f..508bf11e0a00 100644 --- a/ydb/library/login/login.cpp +++ b/ydb/library/login/login.cpp @@ -141,6 +141,10 @@ TLoginProvider::TBasicResponse TLoginProvider::ModifyUser(const TModifyUserReque if (request.CanLogin.has_value()) { user.IsEnabled = request.CanLogin.value(); + + if (user.IsEnabled && CheckLockoutByAttemptCount(user)) { + ResetFailedLoginAttemptCount(&user); + } } return response; @@ -347,9 +351,12 @@ std::vector TLoginProvider::GetGroupsMembership(const TString& member) return groups; } +bool TLoginProvider::CheckLockoutByAttemptCount(const TSidRecord& sid) const { + return AccountLockout.AttemptThreshold != 0 && sid.FailedLoginAttemptCount >= AccountLockout.AttemptThreshold; +} + bool TLoginProvider::CheckLockout(const TSidRecord& sid) const { - return !sid.IsEnabled - || AccountLockout.AttemptThreshold != 0 && sid.FailedLoginAttemptCount >= AccountLockout.AttemptThreshold; + return !sid.IsEnabled || CheckLockoutByAttemptCount(sid); } void TLoginProvider::ResetFailedLoginAttemptCount(TSidRecord* sid) { @@ -364,9 +371,11 @@ bool TLoginProvider::ShouldResetFailedAttemptCount(const TSidRecord& sid) const if (sid.FailedLoginAttemptCount == 0) { return false; } + if (AccountLockout.AttemptResetDuration == std::chrono::system_clock::duration::zero()) { return false; } + return sid.LastFailedLogin + AccountLockout.AttemptResetDuration < std::chrono::system_clock::now(); } diff --git a/ydb/library/login/login.h b/ydb/library/login/login.h index 42fa25c16255..acee71321961 100644 --- a/ydb/library/login/login.h +++ b/ydb/library/login/login.h @@ -221,6 +221,7 @@ class TLoginProvider { bool CheckSubjectExists(const TString& name, const ESidType::SidType& type); static bool CheckAllowedName(const TString& name); + bool CheckLockoutByAttemptCount(const TSidRecord& sid) const; bool CheckLockout(const TSidRecord& sid) const; static void ResetFailedLoginAttemptCount(TSidRecord* sid); static void UnlockAccount(TSidRecord* sid); diff --git a/ydb/library/login/login_ut.cpp b/ydb/library/login/login_ut.cpp index 310e1a8458b4..2c158e17c61d 100644 --- a/ydb/library/login/login_ut.cpp +++ b/ydb/library/login/login_ut.cpp @@ -534,6 +534,54 @@ Y_UNIT_TEST_SUITE(Login) { } } + Y_UNIT_TEST(ResetFailedAttemptCountWithAlterUserLogin) { + TAccountLockout::TInitializer accountLockoutInitializer {.AttemptThreshold = 4, .AttemptResetDuration = "3s"}; + TLoginProvider provider(accountLockoutInitializer); + provider.Audience = "test_audience1"; + provider.RotateKeys(); + + TString userName = "user1"; + TString userPassword = "password1"; + + TLoginProvider::TCreateUserRequest createUserRequest { + .User = userName, + .Password = userPassword + }; + + auto createUserResponse = provider.CreateUser(createUserRequest); + UNIT_ASSERT(!createUserResponse.Error); + + for (size_t attempt = 0; attempt < accountLockoutInitializer.AttemptThreshold; attempt++) { + UNIT_ASSERT_VALUES_EQUAL(provider.IsLockedOut(provider.Sids[userName]), false); + auto checkLockoutResponse = provider.CheckLockOutUser({.User = userName}); + UNIT_ASSERT_EQUAL(checkLockoutResponse.Status, TLoginProvider::TCheckLockOutResponse::EStatus::UNLOCKED); + auto loginUserResponse = provider.LoginUser({.User = userName, .Password = TStringBuilder() << "wrongpassword" << attempt}); + UNIT_ASSERT_EQUAL(loginUserResponse.Status, TLoginProvider::TLoginUserResponse::EStatus::INVALID_PASSWORD); + UNIT_ASSERT_VALUES_EQUAL(loginUserResponse.Error, "Invalid password"); + } + + { + 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"); + } + + { + TLoginProvider::TModifyUserRequest alterRequest; + alterRequest.User = userName; + alterRequest.CanLogin = true; + auto alterResponse = provider.ModifyUser(alterRequest); + UNIT_ASSERT(!alterResponse.Error); + } + + { + UNIT_ASSERT_VALUES_EQUAL(provider.IsLockedOut(provider.Sids[userName]), false); + auto checkLockoutResponse = provider.CheckLockOutUser({.User = userName}); + UNIT_ASSERT_EQUAL(checkLockoutResponse.Status, TLoginProvider::TCheckLockOutResponse::EStatus::UNLOCKED); + } + } + Y_UNIT_TEST(CreateAlterUserWithHash) { TLoginProvider provider; provider.RotateKeys();