New transitional function identity::LegacyIsUsernameAllowedByPatternFromPrefs()

As explained in crbug.com/908121, we can't fix SigninManager::Initialize() to
require a non-null local_state just yet as that would require manually fixing
thousands of tests, so we're adding this transitional function to unblock work
on crbug.com/906085 allowing to migrate from SigninManager::IsAllowedUsername
right away without depending on those other changes.

So, this patch implements a new helper function that will behave more similar
to SigninManager::IsAllowedUsername on that it will only check the username
against the specified pattern if (1) there's a non-null local state passed
and (2) the pattern is registered and a value for it has been set, returning
true otherwise.

We also include an unit test for this helper function, to help with future
refactorings if they happen.

Bug: 906085
Change-Id: Idef663d07354fb086a5bc00b1ab447f1811c344d
Reviewed-on: https://chromium-review.googlesource.com/c/1350895
Commit-Queue: Mario Sanchez Prada <mario@igalia.com>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611190}

components/signin/core/browser/BUILD.gn

@@ -36,6 +36,7 @@ static_library("shared") {
   deps = [
     ":signin_buildflags",
     "//components/account_id",
+    "//components/prefs:prefs",
     "//third_party/icu:icui18n",
   ]
   public_deps = [

components/signin/core/browser/identity_utils.cc

@@ -8,6 +8,7 @@
 
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "components/prefs/pref_service.h"
 #include "third_party/icu/source/i18n/unicode/regex.h"
 
 namespace identity {
@@ -46,4 +47,18 @@ bool IsUsernameAllowedByPattern(base::StringPiece username,
   return !!match;  // !! == convert from UBool to bool.
 }
 
+bool LegacyIsUsernameAllowedByPatternFromPrefs(
+    PrefService* prefs,
+    const std::string& username,
+    const std::string& pattern_pref_name) {
+  // TODO(crbug.com/908121): We need to deal for now with the fact that most
+  // unit tests don't register a local state with the browser process, in which
+  // case all usernames are considered 'allowed'.
+  if (!prefs)
+    return true;
+
+  return IsUsernameAllowedByPattern(username,
+                                    prefs->GetString(pattern_pref_name));
+}
+
 }  // namespace identity

components/signin/core/browser/identity_utils.h

@@ -14,6 +14,8 @@
 
 #include "base/strings/string_piece.h"
 
+class PrefService;
+
 namespace identity {
 
 // Returns true if the username is allowed based on the pattern string.
@@ -22,6 +24,17 @@ namespace identity {
 // moved to //services/identity.
 bool IsUsernameAllowedByPattern(base::StringPiece username,
                                 base::StringPiece pattern);
+
+// Returns true if the username is either allowed based on a pattern registered
+// as |pattern_pref_name| with the preferences service referenced by |prefs|,
+// or if such pattern can't be retrieved from |prefs|. This is a legacy
+// method intended to be used to migrate from SigninManager::IsAllowedUsername()
+// only while SigninManager::Initialize() can still accept a null PrefService*,
+// and can be removed once that's no longer the case (see crbug.com/908121).
+bool LegacyIsUsernameAllowedByPatternFromPrefs(
+    PrefService* prefs,
+    const std::string& username,
+    const std::string& pattern_pref_name);
 }  // namespace identity
 
 #endif  // COMPONENTS_SIGNIN_CORE_BROWSER_IDENTITY_UTILS_H_

components/signin/core/browser/identity_utils_unittest.cc

@@ -3,10 +3,13 @@
 // found in the LICENSE file.
 
 #include "components/signin/core/browser/identity_utils.h"
-#include "testing/gtest/include/gtest/gtest.h"
 
-class IdentityUtilsTest : public testing::Test {};
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/pref_service.h"
+#include "components/prefs/testing_pref_service.h"
+#include "testing/gtest/include/gtest/gtest.h"
 
+namespace {
 const char kUsername[] = "test@test.com";
 
 const char kValidWildcardPattern[] = ".*@test.com";
@@ -23,6 +26,21 @@ const char kNonMatchingPattern[] = ".*foo.*";
 const char kNonMatchingUsernamePattern[] = "foo@test.com";
 const char kNonMatchingDomainPattern[] = "test@foo.com";
 
+const char kRegisteredPattern[] = "test.registered.username_pattern";
+}  // namespace
+
+class IdentityUtilsTest : public testing::Test {
+ public:
+  IdentityUtilsTest() {
+    prefs_.registry()->RegisterStringPref(kRegisteredPattern, std::string());
+  }
+
+  TestingPrefServiceSimple* prefs() { return &prefs_; }
+
+ private:
+  TestingPrefServiceSimple prefs_;
+};
+
 TEST_F(IdentityUtilsTest, IsUsernameAllowedByPattern_EmptyPatterns) {
   EXPECT_TRUE(identity::IsUsernameAllowedByPattern(kUsername, ""));
   EXPECT_FALSE(identity::IsUsernameAllowedByPattern(kUsername, "   "));
@@ -58,3 +76,26 @@ TEST_F(IdentityUtilsTest, IsUsernameAllowedByPattern_MatchingWildcardPatterns) {
   EXPECT_FALSE(identity::IsUsernameAllowedByPattern(kUsername,
                                                     kNonMatchingDomainPattern));
 }
+
+TEST_F(IdentityUtilsTest, LegacyIsUsernameAllowedByPatternFromPrefs) {
+  // Passing a null local state should return 'allowed'.
+  EXPECT_TRUE(identity::LegacyIsUsernameAllowedByPatternFromPrefs(
+      nullptr, kUsername, kRegisteredPattern));
+
+  // Now set different values for the named pattern and check against them.
+  prefs()->SetString(kRegisteredPattern, kMatchingPattern1);
+  EXPECT_TRUE(identity::LegacyIsUsernameAllowedByPatternFromPrefs(
+      prefs(), kUsername, kRegisteredPattern));
+
+  prefs()->SetString(kRegisteredPattern, kNonMatchingUsernamePattern);
+  EXPECT_FALSE(identity::LegacyIsUsernameAllowedByPatternFromPrefs(
+      prefs(), kUsername, kRegisteredPattern));
+
+  prefs()->SetString(kRegisteredPattern, kMatchingPattern2);
+  EXPECT_TRUE(identity::LegacyIsUsernameAllowedByPatternFromPrefs(
+      prefs(), kUsername, kRegisteredPattern));
+
+  prefs()->SetString(kRegisteredPattern, kNonMatchingDomainPattern);
+  EXPECT_FALSE(identity::LegacyIsUsernameAllowedByPatternFromPrefs(
+      prefs(), kUsername, kRegisteredPattern));
+}