ALTER USER use LOGIN resets failed attempt count

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<Schema::LoginSids>().Key(sid.Name).Update<Schema::LoginSids::SidType,
                                                                            Schema::LoginSids::SidHash,
-                                                                           Schema::LoginSids::IsEnabled>(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);

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 {

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<TString> 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();
 }
 

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);

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();