Fix PolicyCertService queried with the wrong key

The PolicyCertService tracks which Profile has used a policy provided
trust anchor by the user's email address.

This CL fixes an issue in which the stored data is queried by the
username hash instead of the email.

Bug: 1106441
Test: unit test
Change-Id: I0e0402f39068fbbc1b218cde0684c4fe6c1a880e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2413489Reviewed-by: Andreea-Elena Costinas <acostinas@google.com>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Commit-Queue: Andreea-Elena Costinas <acostinas@google.com>
Cr-Commit-Position: refs/heads/master@{#809274}

chrome/browser/chrome_content_browser_client.cc

@@ -2775,7 +2775,7 @@ void ChromeContentBrowserClient::OnTrustAnchorUsed(
             Profile::FromBrowserContext(browser_context));
     if (user && !user->username_hash().empty()) {
       policy::PolicyCertServiceFactory::SetUsedPolicyCertificates(
-          user->username_hash());
+          user->GetAccountId().GetUserEmail());
     }
   }
 }

chrome/browser/chrome_content_browser_client_unittest.cc

@@ -73,12 +73,16 @@
 
 #if defined(OS_CHROMEOS)
 #include "base/test/scoped_feature_list.h"
+#include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
+#include "chrome/browser/chromeos/policy/policy_cert_service.h"
+#include "chrome/browser/chromeos/policy/policy_cert_service_factory.h"
 #include "chrome/browser/chromeos/policy/system_features_disable_list_policy_handler.h"
 #include "chrome/test/base/scoped_testing_local_state.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chromeos/components/scanning/url_constants.h"
 #include "chromeos/constants/chromeos_features.h"
 #include "components/policy/core/common/policy_pref_names.h"
+#include "components/user_manager/scoped_user_manager.h"
 #endif  // defined(OS_CHROMEOS)
 
 using content::BrowsingDataFilterBuilder;
@@ -838,6 +842,61 @@ TEST_F(ChromeContentSettingsRedirectTest, RedirectScanningAppURL) {
   test_content_browser_client.HandleWebUI(&dest_url, &profile_);
   EXPECT_EQ(GURL(chrome::kChromeUIAppDisabledURL), dest_url);
 }
+namespace {
+constexpr char kEmail[] = "test@test.com";
+std::unique_ptr<KeyedService> CreateTestPolicyCertService(
+    content::BrowserContext* context) {
+  return policy::PolicyCertService::CreateForTesting(
+      kEmail, user_manager::UserManager::Get());
+}
+}  // namespace
+
+// Test to verify that the PolicyCertService is correctly updated when a policy
+// provided trust anchor is used.
+class ChromeContentSettingsPolicyTrustAnchor
+    : public ChromeContentBrowserClientTest {
+ public:
+  ChromeContentSettingsPolicyTrustAnchor()
+      : testing_local_state_(TestingBrowserProcess::GetGlobal()) {}
+
+  void SetUp() override {
+    // Add a profile
+    auto fake_user_manager =
+        std::make_unique<chromeos::FakeChromeUserManager>();
+    AccountId account_id = AccountId::FromUserEmailGaiaId(kEmail, "gaia_id");
+    user_manager::User* user =
+        fake_user_manager->AddUserWithAffiliationAndTypeAndProfile(
+            account_id, false /*is_affiliated*/,
+            user_manager::USER_TYPE_REGULAR, &profile_);
+    fake_user_manager->UserLoggedIn(account_id, user->username_hash(),
+                                    false /* browser_restart */,
+                                    false /* is_child */);
+    scoped_user_manager_ = std::make_unique<user_manager::ScopedUserManager>(
+        std::move(fake_user_manager));
+    // Create a PolicyCertServiceFactory
+    ASSERT_TRUE(
+        policy::PolicyCertServiceFactory::GetInstance()
+            ->SetTestingFactoryAndUse(
+                &profile_, base::BindRepeating(&CreateTestPolicyCertService)));
+  }
+
+  void TearDown() override { scoped_user_manager_.reset(); }
+
+ protected:
+  content::BrowserTaskEnvironment task_environment_;
+  ScopedTestingLocalState testing_local_state_;
+  std::unique_ptr<user_manager::ScopedUserManager> scoped_user_manager_;
+  TestingProfile profile_;
+};
+
+TEST_F(ChromeContentSettingsPolicyTrustAnchor, PolicyTrustAnchor) {
+  ChromeContentBrowserClient client;
+  EXPECT_FALSE(
+      policy::PolicyCertServiceFactory::UsedPolicyCertificates(kEmail));
+  client.OnTrustAnchorUsed(&profile_);
+  EXPECT_TRUE(policy::PolicyCertServiceFactory::UsedPolicyCertificates(kEmail));
+}
+
 #endif  // defined(OS_CHROMEOS)
 
 class CaptivePortalCheckProcessHost : public content::MockRenderProcessHost {

chrome/browser/chromeos/policy/policy_cert_service_factory.cc

@@ -61,26 +61,26 @@ PolicyCertServiceFactory* PolicyCertServiceFactory::GetInstance() {
 
 // static
 void PolicyCertServiceFactory::SetUsedPolicyCertificates(
-    const std::string& user_id) {
-  if (UsedPolicyCertificates(user_id))
+    const std::string& user_email) {
+  if (UsedPolicyCertificates(user_email))
     return;
   ListPrefUpdate update(g_browser_process->local_state(),
                         prefs::kUsedPolicyCertificates);
-  update->AppendString(user_id);
+  update->AppendString(user_email);
 }
 
 // static
 void PolicyCertServiceFactory::ClearUsedPolicyCertificates(
-    const std::string& user_id) {
+    const std::string& user_email) {
   ListPrefUpdate update(g_browser_process->local_state(),
                         prefs::kUsedPolicyCertificates);
-  update->Remove(base::Value(user_id), nullptr);
+  update->Remove(base::Value(user_email), nullptr);
 }
 
 // static
 bool PolicyCertServiceFactory::UsedPolicyCertificates(
-    const std::string& user_id) {
-  base::Value value(user_id);
+    const std::string& user_email) {
+  base::Value value(user_email);
   const base::ListValue* list =
       g_browser_process->local_state()->GetList(prefs::kUsedPolicyCertificates);
   if (!list) {

chrome/browser/chromeos/policy/policy_cert_service_factory.h

@@ -43,11 +43,11 @@ class PolicyCertServiceFactory : public BrowserContextKeyedServiceFactory {
 
   static PolicyCertServiceFactory* GetInstance();
 
-  // Used to mark or clear |user_id| as having used certificates pushed by
+  // Used to mark or clear |user_email| as having used certificates pushed by
   // policy before.
-  static void SetUsedPolicyCertificates(const std::string& user_id);
-  static void ClearUsedPolicyCertificates(const std::string& user_id);
-  static bool UsedPolicyCertificates(const std::string& user_id);
+  static void SetUsedPolicyCertificates(const std::string& user_email);
+  static void ClearUsedPolicyCertificates(const std::string& user_email);
+  static bool UsedPolicyCertificates(const std::string& user_email);
 
   static void RegisterPrefs(PrefRegistrySimple* local_state);