Skip to content

Commit 335d81a

Browse files
flown4qqqqflown4qqqq
authored
ALTER USER use LOGIN resets failed attempt count (#14444)
1 parent 3d90dec commit 335d81a

File tree

5 files changed

+114
-3
lines changed

5 files changed

+114
-3
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,8 @@ class TAlterLogin: public TSubOperationBase {
8888
auto& sid = context.SS->LoginProvider.Sids[modifyUser.GetUser()];
8989
db.Table<Schema::LoginSids>().Key(sid.Name).Update<Schema::LoginSids::SidType,
9090
Schema::LoginSids::SidHash,
91-
Schema::LoginSids::IsEnabled>(sid.Type, sid.PasswordHash, sid.IsEnabled);
91+
Schema::LoginSids::IsEnabled,
92+
Schema::LoginSids::FailedAttemptCount>(sid.Type, sid.PasswordHash, sid.IsEnabled, sid.FailedLoginAttemptCount);
9293
result->SetStatus(NKikimrScheme::StatusSuccess);
9394

9495
AddIsUserAdmin(modifyUser.GetUser(), context.SS->LoginProvider, additionalParts);

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1092,6 +1092,58 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) {
10921092
CheckToken(resultLogin.token(), describe, "user1");
10931093
}
10941094
}
1095+
1096+
Y_UNIT_TEST(ResetFailedAttemptCountAfterModifyUser) {
1097+
TTestBasicRuntime runtime;
1098+
runtime.AddAppDataInit([] (ui32 nodeIdx, NKikimr::TAppData& appData) {
1099+
Y_UNUSED(nodeIdx);
1100+
auto accountLockout = appData.AuthConfig.MutableAccountLockout();
1101+
accountLockout->SetAttemptThreshold(4);
1102+
accountLockout->SetAttemptResetDuration("3s");
1103+
});
1104+
1105+
TTestEnv env(runtime);
1106+
auto accountLockoutConfig = runtime.GetAppData().AuthConfig.GetAccountLockout();
1107+
ui64 txId = 100;
1108+
{
1109+
auto describe = DescribePath(runtime, TTestTxConfig::SchemeShard, "/MyRoot");
1110+
CheckSecurityState(describe, {.PublicKeysSize = 0, .SidsSize = 0});
1111+
}
1112+
1113+
TString userName = "user1";
1114+
TString userPassword = "password1";
1115+
1116+
auto blockUser = [&]() {
1117+
for (size_t attempt = 0; attempt < accountLockoutConfig.GetAttemptThreshold(); attempt++) {
1118+
auto resultLogin = Login(runtime, userName, TStringBuilder() << "wrongpassword" << attempt);
1119+
UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), "Invalid password");
1120+
}
1121+
};
1122+
1123+
auto loginUser = [&](TString error) {
1124+
auto resultLogin = Login(runtime, userName, userPassword);
1125+
UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), error);
1126+
};
1127+
1128+
auto reboot = [&]() {
1129+
TActorId sender = runtime.AllocateEdgeActor();
1130+
RebootTablet(runtime, TTestTxConfig::SchemeShard, sender);
1131+
};
1132+
1133+
CreateAlterLoginCreateUser(runtime, ++txId, "/MyRoot", userName, userPassword);
1134+
1135+
blockUser();
1136+
loginUser(TStringBuilder() << "User " << userName << " is not permitted to log in");
1137+
reboot();
1138+
loginUser(TStringBuilder() << "User " << userName << " is not permitted to log in");
1139+
ChangeIsEnabledUser(runtime, ++txId, "/MyRoot", userName, true);
1140+
loginUser("");
1141+
1142+
blockUser();
1143+
ChangeIsEnabledUser(runtime, ++txId, "/MyRoot", userName, true);
1144+
reboot();
1145+
loginUser("");
1146+
}
10951147
}
10961148

10971149
namespace NSchemeShardUT_Private {

‎ydb/library/login/login.cpp

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,10 @@ TLoginProvider::TBasicResponse TLoginProvider::ModifyUser(const TModifyUserReque
141141

142142
if (request.CanLogin.has_value()) {
143143
user.IsEnabled = request.CanLogin.value();
144+
145+
if (user.IsEnabled && CheckLockoutByAttemptCount(user)) {
146+
ResetFailedLoginAttemptCount(&user);
147+
}
144148
}
145149

146150
return response;
@@ -347,9 +351,12 @@ std::vector<TString> TLoginProvider::GetGroupsMembership(const TString& member)
347351
return groups;
348352
}
349353

354+
bool TLoginProvider::CheckLockoutByAttemptCount(const TSidRecord& sid) const {
355+
return AccountLockout.AttemptThreshold != 0 && sid.FailedLoginAttemptCount >= AccountLockout.AttemptThreshold;
356+
}
357+
350358
bool TLoginProvider::CheckLockout(const TSidRecord& sid) const {
351-
return !sid.IsEnabled
352-
|| AccountLockout.AttemptThreshold != 0 && sid.FailedLoginAttemptCount >= AccountLockout.AttemptThreshold;
359+
return !sid.IsEnabled || CheckLockoutByAttemptCount(sid);
353360
}
354361

355362
void TLoginProvider::ResetFailedLoginAttemptCount(TSidRecord* sid) {
@@ -364,9 +371,11 @@ bool TLoginProvider::ShouldResetFailedAttemptCount(const TSidRecord& sid) const
364371
if (sid.FailedLoginAttemptCount == 0) {
365372
return false;
366373
}
374+
367375
if (AccountLockout.AttemptResetDuration == std::chrono::system_clock::duration::zero()) {
368376
return false;
369377
}
378+
370379
return sid.LastFailedLogin + AccountLockout.AttemptResetDuration < std::chrono::system_clock::now();
371380
}
372381

‎ydb/library/login/login.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ class TLoginProvider {
221221
bool CheckSubjectExists(const TString& name, const ESidType::SidType& type);
222222
static bool CheckAllowedName(const TString& name);
223223

224+
bool CheckLockoutByAttemptCount(const TSidRecord& sid) const;
224225
bool CheckLockout(const TSidRecord& sid) const;
225226
static void ResetFailedLoginAttemptCount(TSidRecord* sid);
226227
static void UnlockAccount(TSidRecord* sid);

‎ydb/library/login/login_ut.cpp

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,54 @@ Y_UNIT_TEST_SUITE(Login) {
534534
}
535535
}
536536

537+
Y_UNIT_TEST(ResetFailedAttemptCountWithAlterUserLogin) {
538+
TAccountLockout::TInitializer accountLockoutInitializer {.AttemptThreshold = 4, .AttemptResetDuration = "3s"};
539+
TLoginProvider provider(accountLockoutInitializer);
540+
provider.Audience = "test_audience1";
541+
provider.RotateKeys();
542+
543+
TString userName = "user1";
544+
TString userPassword = "password1";
545+
546+
TLoginProvider::TCreateUserRequest createUserRequest {
547+
.User = userName,
548+
.Password = userPassword
549+
};
550+
551+
auto createUserResponse = provider.CreateUser(createUserRequest);
552+
UNIT_ASSERT(!createUserResponse.Error);
553+
554+
for (size_t attempt = 0; attempt < accountLockoutInitializer.AttemptThreshold; attempt++) {
555+
UNIT_ASSERT_VALUES_EQUAL(provider.IsLockedOut(provider.Sids[userName]), false);
556+
auto checkLockoutResponse = provider.CheckLockOutUser({.User = userName});
557+
UNIT_ASSERT_EQUAL(checkLockoutResponse.Status, TLoginProvider::TCheckLockOutResponse::EStatus::UNLOCKED);
558+
auto loginUserResponse = provider.LoginUser({.User = userName, .Password = TStringBuilder() << "wrongpassword" << attempt});
559+
UNIT_ASSERT_EQUAL(loginUserResponse.Status, TLoginProvider::TLoginUserResponse::EStatus::INVALID_PASSWORD);
560+
UNIT_ASSERT_VALUES_EQUAL(loginUserResponse.Error, "Invalid password");
561+
}
562+
563+
{
564+
UNIT_ASSERT_VALUES_EQUAL(provider.IsLockedOut(provider.Sids[userName]), true);
565+
auto checkLockoutResponse = provider.CheckLockOutUser({.User = userName});
566+
UNIT_ASSERT_EQUAL(checkLockoutResponse.Status, TLoginProvider::TCheckLockOutResponse::EStatus::SUCCESS);
567+
UNIT_ASSERT_STRING_CONTAINS(checkLockoutResponse.Error, TStringBuilder() << "User " << userName << " is not permitted to log in");
568+
}
569+
570+
{
571+
TLoginProvider::TModifyUserRequest alterRequest;
572+
alterRequest.User = userName;
573+
alterRequest.CanLogin = true;
574+
auto alterResponse = provider.ModifyUser(alterRequest);
575+
UNIT_ASSERT(!alterResponse.Error);
576+
}
577+
578+
{
579+
UNIT_ASSERT_VALUES_EQUAL(provider.IsLockedOut(provider.Sids[userName]), false);
580+
auto checkLockoutResponse = provider.CheckLockOutUser({.User = userName});
581+
UNIT_ASSERT_EQUAL(checkLockoutResponse.Status, TLoginProvider::TCheckLockOutResponse::EStatus::UNLOCKED);
582+
}
583+
}
584+
537585
Y_UNIT_TEST(CreateAlterUserWithHash) {
538586
TLoginProvider provider;
539587
provider.RotateKeys();

0 commit comments

Comments
 (0)