[PasswordGeneration] Conditionally added '0' and '1' to numeric alphabet.

Currently, the '0'/'1' digits are not used in generated password to avoid
visual confusion with letters like 'l','I' and 'O'. With this change, the
digits are added to the numeric alphabet if the generated password does
not contain lower and upper case letters.

Bug: 1038659
Change-Id: I25552b85592b8a3f2e286b9c30652db6664eff00
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1985768
Commit-Queue: Matthias Körber <koerber@google.com>
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Dominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732775}

components/password_manager/core/browser/generation/password_generator.cc

@@ -7,6 +7,7 @@
 #include <algorithm>
 #include <limits>
 #include <map>
+#include <utility>
 #include <vector>
 
 #include "base/logging.h"
@@ -203,6 +204,12 @@ base::string16 GenerateMaxEntropyPassword(PasswordRequirementsSpec spec) {
 
 }  // namespace
 
+void ConditionallyAddNumericDigitsToAlphabet(PasswordRequirementsSpec* spec) {
+  DCHECK(spec);
+  if (spec->lower_case().max() == 0 && spec->upper_case().max() == 0)
+    spec->mutable_numeric()->mutable_character_set()->append("01");
+}
+
 base::string16 GeneratePassword(const PasswordRequirementsSpec& spec) {
   PasswordRequirementsSpec actual_spec = BuildDefaultSpec();
 
@@ -210,6 +217,9 @@ base::string16 GeneratePassword(const PasswordRequirementsSpec& spec) {
   // recursively.
   actual_spec.MergeFrom(spec);
 
+  // For passwords without letters, add the '0' and '1' to the numeric alphabet.
+  ConditionallyAddNumericDigitsToAlphabet(&actual_spec);
+
   base::string16 password = GenerateMaxEntropyPassword(std::move(actual_spec));
 
   // Catch cases where supplied spec is infeasible.

components/password_manager/core/browser/generation/password_generator.h

@@ -11,6 +11,11 @@ namespace autofill {
 
 class PasswordRequirementsSpec;
 
+// If neither lower nor upper case letters are included in the password
+// generation alphabet, add the '0' and '1' digit to the numeric alphabet
+// to increase the entropy of numeric-only passwords.
+void ConditionallyAddNumericDigitsToAlphabet(PasswordRequirementsSpec* spec);
+
 extern const uint32_t kDefaultPasswordLength;
 
 // Returns a password that follows the |spec| as well as possible. If this is

components/password_manager/core/browser/generation/password_generator_unittest.cc

@@ -4,6 +4,8 @@
 
 #include "components/password_manager/core/browser/generation/password_generator.h"
 
+#include <string>
+
 #include "base/logging.h"
 #include "components/autofill/core/browser/proto/password_requirements.pb.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -81,6 +83,42 @@ TEST_F(PasswordGeneratorTest, PasswordLengthDefault) {
   EXPECT_EQ(kDefaultPasswordLength, GeneratePassword(spec_).length());
 }
 
+TEST_F(PasswordGeneratorTest, ConditionallyAddNumericDigitsToAlphabet) {
+  // Check if '0' and '1' is added to the numeric alphabet if the password
+  // does not contain letters.
+  spec_.mutable_lower_case()->set_max(0);
+  spec_.mutable_upper_case()->set_max(0);
+  ConditionallyAddNumericDigitsToAlphabet(&spec_);
+  EXPECT_TRUE(spec_.numeric().character_set().find('0') != std::string::npos);
+  EXPECT_TRUE(spec_.numeric().character_set().find('1') != std::string::npos);
+}
+
+TEST_F(PasswordGeneratorTest, ConditionallyDoNotAddNumericDigitsToAlphabet) {
+  // Check if '0' and '1' is not added to the numeric alphabet if the password
+  // does contain letters lower and/or upper case letters.
+
+  // Check for only lower case letters.
+  spec_.mutable_lower_case()->set_max(1);
+  spec_.mutable_upper_case()->set_max(0);
+  ConditionallyAddNumericDigitsToAlphabet(&spec_);
+  EXPECT_TRUE(spec_.numeric().character_set().find('0') == std::string::npos);
+  EXPECT_TRUE(spec_.numeric().character_set().find('1') == std::string::npos);
+
+  // Check for only upper case letters.
+  spec_.mutable_lower_case()->set_max(0);
+  spec_.mutable_upper_case()->set_max(1);
+  ConditionallyAddNumericDigitsToAlphabet(&spec_);
+  EXPECT_TRUE(spec_.numeric().character_set().find('0') == std::string::npos);
+  EXPECT_TRUE(spec_.numeric().character_set().find('1') == std::string::npos);
+
+  // Mixed case with lower and upper case letters.
+  spec_.mutable_lower_case()->set_max(1);
+  spec_.mutable_upper_case()->set_max(1);
+  ConditionallyAddNumericDigitsToAlphabet(&spec_);
+  EXPECT_TRUE(spec_.numeric().character_set().find('0') == std::string::npos);
+  EXPECT_TRUE(spec_.numeric().character_set().find('1') == std::string::npos);
+}
+
 TEST_F(PasswordGeneratorTest, PasswordLengthMaxLength) {
   // Limit length according to requirement.
   spec_.set_max_length(kDefaultPasswordLength - 5u);