Fix handle password complexity default values (#15314)

Author: molotkov-and

ydb/core/tx/schemeshard/schemeshard_impl.cpp

@@ -7445,7 +7445,6 @@ void TSchemeShard::ConfigureBackgroundCleaningQueue(
                  << ", InflightLimit# " << cleaningConfig.InflightLimit);
 }
 
-// void TScheme
 void TSchemeShard::ConfigureLoginProvider(
         const ::NKikimrProto::TAuthConfig& config,
         const TActorContext &ctx)
@@ -7457,18 +7456,26 @@ void TSchemeShard::ConfigureLoginProvider(
         .MinUpperCaseCount = passwordComplexityConfig.GetMinUpperCaseCount(),
         .MinNumbersCount = passwordComplexityConfig.GetMinNumbersCount(),
         .MinSpecialCharsCount = passwordComplexityConfig.GetMinSpecialCharsCount(),
-        .SpecialChars = (passwordComplexityConfig.GetSpecialChars().empty() ? NLogin::TPasswordComplexity::VALID_SPECIAL_CHARS : passwordComplexityConfig.GetSpecialChars()),
+        .SpecialChars = passwordComplexityConfig.GetSpecialChars(),
         .CanContainUsername = passwordComplexityConfig.GetCanContainUsername()
     });
     LoginProvider.UpdatePasswordCheckParameters(passwordComplexity);
 
+    auto getSpecialChars = [&passwordComplexity] () {
+        TStringBuilder result;
+        for (const auto& ch : passwordComplexity.SpecialChars) {
+            result << ch;
+        }
+        return result;
+    };
+
     LOG_NOTICE_S(ctx, NKikimrServices::FLAT_TX_SCHEMESHARD,
                  "PasswordComplexity for LoginProvider configured: MinLength# " << passwordComplexity.MinLength
                  << ", MinLowerCaseCount# " << passwordComplexity.MinLowerCaseCount
                  << ", MinUpperCaseCount# " << passwordComplexity.MinUpperCaseCount
                  << ", MinNumbersCount# " << passwordComplexity.MinNumbersCount
                  << ", MinSpecialCharsCount# " << passwordComplexity.MinSpecialCharsCount
-                 << ", SpecialChars# " << (passwordComplexityConfig.GetSpecialChars().empty() ? NLogin::TPasswordComplexity::VALID_SPECIAL_CHARS : passwordComplexityConfig.GetSpecialChars())
+                 << ", SpecialChars# " << getSpecialChars()
                  << ", CanContainUsername# " << (passwordComplexity.CanContainUsername ? "true" : "false"));
 }
 

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

@@ -313,7 +313,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) {
         AlterLoginAddGroupMembership(runtime, ++txId, "/MyRoot", "user1", "group1");
         TestDescribeResult(DescribePath(runtime, "/MyRoot"),
             {NLs::HasGroup("group1", {"user1"})});
-        
+
         CreateAlterLoginRemoveGroup(runtime, ++txId, "/MyRoot", "group1");
         TestDescribeResult(DescribePath(runtime, "/MyRoot"),
             {NLs::HasNoGroup("group1")});
@@ -323,7 +323,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) {
         TTestBasicRuntime runtime;
         TTestEnv env(runtime, TTestEnvOptions().EnableStrictAclCheck(StrictAclCheck));
         ui64 txId = 100;
-        
+
         for (bool missingOk : {false, true}) {
             auto modifyTx = std::make_unique<TEvSchemeShard::TEvModifySchemeTransaction>(txId, TTestTxConfig::SchemeShard);
             auto transaction = modifyTx->Record.AddTransaction();
@@ -370,7 +370,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) {
         TestModificationResult(runtime, txId, NKikimrScheme::StatusSuccess);
         TestDescribeResult(DescribePath(runtime, "/MyRoot/Dir1"),
                 {NLs::HasOwner("root@builtin")});
-        
+
         CreateAlterLoginRemoveGroup(runtime, ++txId, "/MyRoot", "group1");
         TestDescribeResult(DescribePath(runtime, "/MyRoot"),
             {NLs::HasNoGroup("group1")});
@@ -412,7 +412,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) {
         TestModificationResult(runtime, txId, NKikimrScheme::StatusSuccess);
         TestDescribeResult(DescribePath(runtime, "/MyRoot/Dir1"),
                 {NLs::HasNoRight("+U:group1")});
-        
+
         CreateAlterLoginRemoveGroup(runtime, ++txId, "/MyRoot", "group1");
         TestDescribeResult(DescribePath(runtime, "/MyRoot"),
             {NLs::HasNoGroup("group1")});
@@ -630,8 +630,8 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) {
         // required: cannot contain username
 
         {
-            CreateAlterLoginCreateUser(runtime, ++txId, "/MyRoot", "user1", "password1");
-            auto resultLogin = Login(runtime, "user1", "password1");
+            CreateAlterLoginCreateUser(runtime, ++txId, "/MyRoot", "user1", "Pass_word1");
+            auto resultLogin = Login(runtime, "user1", "Pass_word1");
             UNIT_ASSERT_VALUES_EQUAL(resultLogin.error(), "");
             auto describe = DescribePath(runtime, TTestTxConfig::SchemeShard, "/MyRoot");
             CheckSecurityState(describe, {.PublicKeysSize = 1, .SidsSize = 1});
@@ -1101,7 +1101,7 @@ Y_UNIT_TEST_SUITE(TSchemeShardLoginTest) {
             accountLockout->SetAttemptThreshold(4);
             accountLockout->SetAttemptResetDuration("3s");
         });
- 
+
         TTestEnv env(runtime);
         auto accountLockoutConfig = runtime.GetAppData().AuthConfig.GetAccountLockout();
         ui64 txId = 100;

ydb/library/login/login_ut.cpp

@@ -109,8 +109,7 @@ Y_UNIT_TEST_SUITE(Login) {
             .MinLowerCaseCount = 2,
             .MinUpperCaseCount = 2,
             .MinNumbersCount = 2,
-            .MinSpecialCharsCount = 2,
-            .SpecialChars = TPasswordComplexity::VALID_SPECIAL_CHARS
+            .MinSpecialCharsCount = 2
         });
 
         provider.UpdatePasswordCheckParameters(passwordComplexity);

ydb/library/login/password_checker/password_checker.cpp

@@ -5,7 +5,7 @@
 namespace NLogin {
 
 TPasswordComplexity::TPasswordComplexity()
-    : SpecialChars(VALID_SPECIAL_CHARS.cbegin(), VALID_SPECIAL_CHARS.cend())
+    : SpecialChars(VALID_SPECIAL_CHARS)
 {}
 
 TPasswordComplexity::TPasswordComplexity(const TInitializer& initializer)
@@ -16,10 +16,13 @@ TPasswordComplexity::TPasswordComplexity(const TInitializer& initializer)
     , MinSpecialCharsCount(initializer.MinSpecialCharsCount)
     , CanContainUsername(initializer.CanContainUsername)
 {
-    static const std::unordered_set<char> validSpecialChars(VALID_SPECIAL_CHARS.cbegin(), VALID_SPECIAL_CHARS.cend());
-    for (const char ch : initializer.SpecialChars) {
-        if (validSpecialChars.contains(ch)) {
-            SpecialChars.insert(ch);
+    if (initializer.SpecialChars.empty()) {
+        SpecialChars.insert(VALID_SPECIAL_CHARS.begin(), VALID_SPECIAL_CHARS.end());
+    } else {
+        for (const char ch : initializer.SpecialChars) {
+            if (VALID_SPECIAL_CHARS.contains(ch)) {
+                SpecialChars.insert(ch);
+            }
         }
     }
 }
@@ -28,7 +31,9 @@ bool TPasswordComplexity::IsSpecialCharValid(char ch) const {
     return SpecialChars.contains(ch);
 }
 
-const TString TPasswordComplexity::VALID_SPECIAL_CHARS = "!@#$%^&*()_+{}|<>?=";
+const std::unordered_set<char> TPasswordComplexity::VALID_SPECIAL_CHARS {'!', '@', '#', '$', '%', '^', '&',
+                                                                         '*', '(', ')', '_', '+', '{',
+                                                                         '}', '|', '<', '>', '?', '='};
 
 TPasswordChecker::TComplexityState::TComplexityState(const TPasswordComplexity& passwordComplexity)
     : PasswordComplexity(passwordComplexity)

ydb/library/login/password_checker/password_checker.h

@@ -14,11 +14,11 @@ class TPasswordComplexity {
         size_t MinUpperCaseCount = 0;
         size_t MinNumbersCount = 0;
         size_t MinSpecialCharsCount = 0;
-        TString SpecialChars = VALID_SPECIAL_CHARS;
+        TString SpecialChars;
         bool CanContainUsername = false;
     };
 
-    static const TString VALID_SPECIAL_CHARS;
+    static const std::unordered_set<char> VALID_SPECIAL_CHARS;
 
     size_t MinLength = 0;
     size_t MinLowerCaseCount = 0;

ydb/library/login/password_checker/password_checker_ut.cpp

@@ -14,7 +14,6 @@ Y_UNIT_TEST_SUITE(PasswordChecker) {
             .MinUpperCaseCount = 2,
             .MinNumbersCount = 2,
             .MinSpecialCharsCount = 2,
-            .SpecialChars = TPasswordComplexity::VALID_SPECIAL_CHARS,
             .CanContainUsername = false
         });
         TPasswordChecker passwordChecker(passwordComplexity);
@@ -169,6 +168,18 @@ Y_UNIT_TEST_SUITE(PasswordChecker) {
         UNIT_ASSERT_STRINGS_EQUAL(result.Error, "Password is too short");
     }
 
+    Y_UNIT_TEST(AcceptPasswordWithSpecialCharsIfSpecialCharsListIsEmpty) {
+        TPasswordComplexity passwordComplexity({
+            .SpecialChars = ""
+        });
+        TPasswordChecker passwordChecker(passwordComplexity);
+        TString username = "testuser";
+        TString password = "user_password";
+        TPasswordChecker::TResult result = passwordChecker.Check(username, password);
+        UNIT_ASSERT_C(result.Success, result.Error);
+        UNIT_ASSERT(result.Error.empty());
+    }
+
     Y_UNIT_TEST(HashChecker) {
         {
             auto hash = R"(