Adding browser test for MachineLevelUserCloudPolicy enrollment process.

Verify DM token fetch and store status.
Also add an observer for the enrollment process. This will be used
by upcoming browser launch block during enrollment feature.


Bug: 812641
Change-Id: I8d9df9ecc6cdbb3f6616253d597a82d3d9414810
Reviewed-on: https://chromium-review.googlesource.com/1020417
Commit-Queue: Owen Min <zmin@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Roger Tawa <rogerta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553250}

chrome/browser/policy/browser_dm_token_storage.h

@@ -39,11 +39,19 @@ class BrowserDMTokenStorage {
   // an error is encountered.
   virtual std::string RetrieveDMToken() = 0;
 
+  // Set the mock BrowserDMTokenStorage for testing. The caller owns the
+  // instance of the storage.
+  static void SetForTesting(BrowserDMTokenStorage* storage) {
+    storage_for_testing_ = storage;
+  }
+
  protected:
   BrowserDMTokenStorage() = default;
   virtual ~BrowserDMTokenStorage() = default;
 
  private:
+  static BrowserDMTokenStorage* storage_for_testing_;
+
   DISALLOW_COPY_AND_ASSIGN(BrowserDMTokenStorage);
 };
 

chrome/browser/policy/browser_dm_token_storage_stub.cc

@@ -8,7 +8,7 @@
 #include <utility>
 
 #include "base/callback.h"
-#include "base/task_scheduler/post_task.h"
+#include "base/threading/thread_task_runner_handle.h"
 
 namespace policy {
 
@@ -16,7 +16,13 @@ namespace {
 BrowserDMTokenStorage* g_browser_dm_token_storage = nullptr;
 }  // namespace
 
+// static
+BrowserDMTokenStorage* BrowserDMTokenStorage::storage_for_testing_ = nullptr;
+
+// static
 BrowserDMTokenStorage* BrowserDMTokenStorage::Get() {
+  if (storage_for_testing_)
+    return storage_for_testing_;
   if (g_browser_dm_token_storage == nullptr)
     g_browser_dm_token_storage = new BrowserDMTokenStorageStub();
   return g_browser_dm_token_storage;
@@ -32,7 +38,8 @@ std::string BrowserDMTokenStorageStub::RetrieveEnrollmentToken() {
 
 void BrowserDMTokenStorageStub::StoreDMToken(const std::string& dm_token,
                                              StoreCallback callback) {
-  base::PostTask(FROM_HERE, base::BindOnce(std::move(callback), false));
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::BindOnce(std::move(callback), false));
 }
 
 std::string BrowserDMTokenStorageStub::RetrieveDMToken() {

chrome/browser/policy/browser_dm_token_storage_win.cc

@@ -187,7 +187,14 @@ bool StoreDMTokenInRegistry(const std::string& token) {
 
 }  // namespace
 
+// static
+BrowserDMTokenStorage* BrowserDMTokenStorage::storage_for_testing_ = nullptr;
+
+// static
 BrowserDMTokenStorage* BrowserDMTokenStorage::Get() {
+  if (storage_for_testing_)
+    return storage_for_testing_;
+
   static base::NoDestructor<BrowserDMTokenStorageWin> storage;
   return storage.get();
 }

chrome/browser/policy/chrome_browser_policy_connector.cc

@@ -24,6 +24,7 @@
 #include "components/policy/core/common/cloud/cloud_policy_client_registration_helper.h"
 #include "components/policy/core/common/cloud/device_management_service.h"
 #include "components/policy/core/common/cloud/machine_level_user_cloud_policy_manager.h"
+#include "components/policy/core/common/cloud/machine_level_user_cloud_policy_metrics.h"
 #include "components/policy/core/common/cloud/machine_level_user_cloud_policy_store.h"
 #include "components/policy/core/common/cloud/user_cloud_policy_manager.h"
 #include "components/policy/core/common/configuration_policy_provider.h"
@@ -58,16 +59,6 @@ namespace {
 
 #if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
 
-// This enum is used for recording the metrics. It must match the
-// MachineLevelUserCloudPolicyEnrollmentResult in enums.xml and should not be
-// reordered. |kMaxValue| must be assigned to the last entry of the enum.
-enum MachineLevelUserCloudPolicyEnrollmentResult {
-  kSuccess = 0,
-  kFailedToFetch = 1,
-  kFailedToStore = 2,
-  kMaxValue = kFailedToStore,
-};
-
 std::unique_ptr<MachineLevelUserCloudPolicyManager>
 CreateMachineLevelUserCloudPolicyManager() {
   base::FilePath user_data_dir;
@@ -151,6 +142,14 @@ ChromeBrowserPolicyConnector::GetPlatformProvider() {
   return provider ? provider : platform_provider_;
 }
 
+void ChromeBrowserPolicyConnector::AddObserver(Observer* observer) {
+  observers_.AddObserver(observer);
+}
+
+void ChromeBrowserPolicyConnector::RemoveObserver(Observer* observer) {
+  observers_.RemoveObserver(observer);
+}
+
 #if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
 MachineLevelUserCloudPolicyManager*
 ChromeBrowserPolicyConnector::GetMachineLevelUserCloudPolicyManager() {
@@ -303,6 +302,7 @@ void ChromeBrowserPolicyConnector::RegisterForPolicyWithEnrollmentTokenCallback(
     DVLOG(1) << "No DM token returned from browser registration";
     RecordEnrollmentResult(
         MachineLevelUserCloudPolicyEnrollmentResult::kFailedToFetch);
+    NotifyMachineLevelUserCloudPolicyRegisterFinished(false);
     return;
   }
 
@@ -325,6 +325,14 @@ void ChromeBrowserPolicyConnector::RegisterForPolicyWithEnrollmentTokenCallback(
   // Start fetching policies.
   machine_level_user_cloud_policy_fetcher_->SetupRegistrationAndFetchPolicy(
       dm_token, client_id);
+  NotifyMachineLevelUserCloudPolicyRegisterFinished(true);
+}
+
+void ChromeBrowserPolicyConnector::
+    NotifyMachineLevelUserCloudPolicyRegisterFinished(bool succeeded) {
+  for (auto& observer : observers_) {
+    observer.OnMachineLevelUserCloudPolicyRegisterFinished(succeeded);
+  }
 }
 
 #endif  // !defined(OS_ANDROID) && !defined(OS_CHROMEOS)

chrome/browser/policy/chrome_browser_policy_connector.h

@@ -59,6 +59,19 @@ class ChromeBrowserPolicyConnector : public BrowserPolicyConnector {
 
   ConfigurationPolicyProvider* GetPlatformProvider();
 
+  class Observer {
+   public:
+    virtual ~Observer() {}
+
+    // Called when machine level user cloud policy enrollment is finished.
+    // |succeeded| is true if |dm_token| is returned from the server.
+    virtual void OnMachineLevelUserCloudPolicyRegisterFinished(bool succeeded) {
+    }
+  };
+
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
 #if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
   MachineLevelUserCloudPolicyManager* GetMachineLevelUserCloudPolicyManager();
 #endif
@@ -81,6 +94,8 @@ class ChromeBrowserPolicyConnector : public BrowserPolicyConnector {
       const std::string& dm_token,
       const std::string& client_id);
 
+  void NotifyMachineLevelUserCloudPolicyRegisterFinished(bool succeeded);
+
   // Owned by base class.
   MachineLevelUserCloudPolicyManager* machine_level_user_cloud_policy_manager_ =
       nullptr;
@@ -89,7 +104,9 @@ class ChromeBrowserPolicyConnector : public BrowserPolicyConnector {
       machine_level_user_cloud_policy_registrar_;
   std::unique_ptr<MachineLevelUserCloudPolicyFetcher>
       machine_level_user_cloud_policy_fetcher_;
+
 #endif
+  base::ObserverList<Observer, true> observers_;
 
   // Owned by base class.
   ConfigurationPolicyProvider* platform_provider_ = nullptr;

chrome/browser/policy/cloud/machine_level_user_cloud_policy_browsertest.cc

-// Copyright (c) 2018 The Chromium Authors. All rights reserved.
+// Copyright 2018 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <stdint.h>
 
 #include <memory>
+#include <tuple>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ -12,9 +13,15 @@
 #include "base/path_service.h"
 #include "base/run_loop.h"
 #include "base/stl_util.h"
+#include "base/strings/utf_string_conversions.h"
 #include "base/task_scheduler/post_task.h"
+#include "base/test/histogram_tester.h"
 #include "base/threading/thread_restrictions.h"
+#include "base/threading/thread_task_runner_handle.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/chrome_browser_main.h"
+#include "chrome/browser/chrome_browser_main_extra_parts.h"
+#include "chrome/browser/policy/browser_dm_token_storage.h"
 #include "chrome/browser/policy/chrome_browser_policy_connector.h"
 #include "chrome/browser/policy/test/local_policy_test_server.h"
 #include "chrome/common/chrome_paths.h"
@@ -22,9 +29,11 @@
 #include "components/policy/core/common/cloud/cloud_policy_constants.h"
 #include "components/policy/core/common/cloud/device_management_service.h"
 #include "components/policy/core/common/cloud/machine_level_user_cloud_policy_manager.h"
+#include "components/policy/core/common/cloud/machine_level_user_cloud_policy_metrics.h"
 #include "components/policy/core/common/cloud/machine_level_user_cloud_policy_store.h"
 #include "components/policy/core/common/cloud/mock_cloud_external_data_manager.h"
 #include "components/policy/core/common/cloud/mock_device_management_service.h"
+#include "components/policy/core/common/policy_switches.h"
 #include "content/public/browser/browser_thread.h"
 #include "net/base/upload_bytes_element_reader.h"
 #include "net/base/upload_data_stream.h"
@@ -43,14 +52,92 @@ using testing::_;
 
 namespace em = enterprise_management;
 
+namespace policy {
 namespace {
+
 const char kEnrollmentToken[] = "enrollment_token";
+const char kInvalidEnrollmentToken[] = "invalid_enrollment_token";
 const char kMachineName[] = "foo";
 const char kClientID[] = "fake-client-id";
 const char kDMToken[] = "fake-dm-token";
-}  // namespace
+const char kEnrollmentResultMetrics[] =
+    "Enterprise.MachineLevelUserCloudPolicyEnrollment.Result";
 
-namespace policy {
+class ChromeBrowserPolicyConnectorObserver
+    : public ChromeBrowserPolicyConnector::Observer {
+ public:
+  void OnMachineLevelUserCloudPolicyRegisterFinished(bool succeeded) override {
+    EXPECT_EQ(should_succeed_, succeeded);
+    is_finished_ = true;
+    if (run_loop_)
+      run_loop_->Quit();
+  }
+
+  void SetRunLoop(base::RunLoop* run_loop) { run_loop_ = run_loop; }
+  void SetShouldSucceed(bool should_succeed) {
+    should_succeed_ = should_succeed;
+  }
+
+  bool IsFinished() { return is_finished_; }
+
+ private:
+  base::RunLoop* run_loop_ = nullptr;
+  bool is_finished_ = false;
+  bool should_succeed_ = false;
+};
+
+class FakeBrowserDMTokenStorage : public BrowserDMTokenStorage {
+ public:
+  FakeBrowserDMTokenStorage() = default;
+
+  std::string RetrieveClientId() override { return client_id_; }
+  std::string RetrieveEnrollmentToken() override { return enrollment_token_; }
+  void StoreDMToken(const std::string& dm_token,
+                    StoreCallback callback) override {
+    // Store the dm token in memory even if storage gonna failed. This is the
+    // same behavior of production code.
+    dm_token_ = dm_token;
+    // Run the callback synchronously to make sure the metrics is recorded
+    // before verfication.
+    std::move(callback).Run(storage_enabled_);
+  }
+  std::string RetrieveDMToken() override { return dm_token_; }
+
+  void SetEnrollmentToken(const std::string& enrollment_token) {
+    enrollment_token_ = enrollment_token;
+  }
+
+  void SetClientId(std::string client_id) { client_id_ = client_id; }
+
+  void EnableStorage(bool storage_enabled) {
+    storage_enabled_ = storage_enabled;
+  }
+
+ private:
+  std::string enrollment_token_;
+  std::string client_id_;
+  std::string dm_token_;
+  bool storage_enabled_ = true;
+
+  DISALLOW_COPY_AND_ASSIGN(FakeBrowserDMTokenStorage);
+};
+
+class ChromeBrowserExtraSetUp : public ChromeBrowserMainExtraParts {
+ public:
+  explicit ChromeBrowserExtraSetUp(
+      ChromeBrowserPolicyConnectorObserver* observer)
+      : observer_(observer) {}
+  void PreMainMessageLoopStart() override {
+    g_browser_process->browser_policy_connector()->AddObserver(observer_);
+  }
+
+ private:
+  ChromeBrowserPolicyConnectorObserver* observer_;
+
+  DISALLOW_COPY_AND_ASSIGN(ChromeBrowserExtraSetUp);
+};
+
+}  // namespace
 
 MATCHER_P(MatchProto, expected, "matches protobuf") {
   return arg.SerializePartialAsString() == expected.SerializePartialAsString();
@@ -254,4 +341,86 @@ IN_PROC_BROWSER_TEST_F(MachineLevelUserCloudPolicyManagerTest, WithDmToken) {
   EXPECT_TRUE(CreateAndInitManager("dummy_dm_token"));
 }
 
+class MachineLevelUserCloudPolicyEnrollmentTest
+    : public InProcessBrowserTest,
+      public ::testing::WithParamInterface<std::tuple<bool, bool>> {
+ public:
+  MachineLevelUserCloudPolicyEnrollmentTest() {
+    BrowserDMTokenStorage::SetForTesting(&storage_);
+    storage_.SetEnrollmentToken(is_enrollment_token_valid()
+                                    ? kEnrollmentToken
+                                    : kInvalidEnrollmentToken);
+    storage_.SetClientId("client_id");
+    storage_.EnableStorage(storage_enabled());
+    observer_.SetShouldSucceed(is_enrollment_token_valid());
+  }
+
+  void SetUpInProcessBrowserTestFixture() override {
+    ASSERT_TRUE(test_server_.Start());
+
+    base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
+    command_line->AppendSwitchASCII(switches::kDeviceManagementUrl,
+                                    test_server_.GetServiceURL().spec());
+
+    histogram_tester_.ExpectTotalCount(kEnrollmentResultMetrics, 0);
+  }
+
+  void CreatedBrowserMainParts(content::BrowserMainParts* parts) override {
+    static_cast<ChromeBrowserMainParts*>(parts)->AddParts(
+        new ChromeBrowserExtraSetUp(&observer_));
+  }
+
+  void TearDownOnMainThread() override {
+    g_browser_process->browser_policy_connector()->RemoveObserver(&observer_);
+  }
+
+  void WaitForPolicyRegisterFinished() {
+    if (!observer_.IsFinished()) {
+      base::RunLoop run_loop;
+      observer_.SetRunLoop(&run_loop);
+      run_loop.Run();
+    }
+  }
+
+ protected:
+  bool is_enrollment_token_valid() const { return std::get<0>(GetParam()); }
+  bool storage_enabled() const { return std::get<1>(GetParam()); }
+
+  base::HistogramTester histogram_tester_;
+
+ private:
+  LocalPolicyTestServer test_server_;
+  FakeBrowserDMTokenStorage storage_;
+  ChromeBrowserPolicyConnectorObserver observer_;
+
+  DISALLOW_COPY_AND_ASSIGN(MachineLevelUserCloudPolicyEnrollmentTest);
+};
+
+IN_PROC_BROWSER_TEST_P(MachineLevelUserCloudPolicyEnrollmentTest, Test) {
+  WaitForPolicyRegisterFinished();
+
+  EXPECT_EQ(is_enrollment_token_valid() ? "fake_device_management_token"
+                                        : std::string(),
+            BrowserDMTokenStorage::Get()->RetrieveDMToken());
+
+  MachineLevelUserCloudPolicyEnrollmentResult expected_result;
+  if (is_enrollment_token_valid() && storage_enabled()) {
+    expected_result = MachineLevelUserCloudPolicyEnrollmentResult::kSuccess;
+  } else if (is_enrollment_token_valid() && !storage_enabled()) {
+    expected_result =
+        MachineLevelUserCloudPolicyEnrollmentResult::kFailedToStore;
+  } else {
+    expected_result =
+        MachineLevelUserCloudPolicyEnrollmentResult::kFailedToFetch;
+  }
+  histogram_tester_.ExpectBucketCount(kEnrollmentResultMetrics, expected_result,
+                                      1);
+  histogram_tester_.ExpectTotalCount(kEnrollmentResultMetrics, 1);
+}
+
+INSTANTIATE_TEST_CASE_P(,
+                        MachineLevelUserCloudPolicyEnrollmentTest,
+                        ::testing::Combine(::testing::Bool(),
+                                           ::testing::Bool()));
+
 }  // namespace policy

chrome/browser/policy/test/policy_testserver.py

@@ -191,6 +191,8 @@ LICENSE_TYPES = {
   'kiosk': dm.LicenseType.KIOSK,
 }
 
+INVALID_ENROLLMENT_TOKEN = 'invalid_enrollment_token'
+
 class PolicyRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
   """Decodes and handles device management requests from clients.
 
@@ -767,6 +769,9 @@ class PolicyRequestHandler(BaseHTTPServer.BaseHTTPRequestHandler):
     if not msg.machine_name:
       return (400, 'Invalid machine name: ')
 
+    if enrollment_token == INVALID_ENROLLMENT_TOKEN:
+      return (401, 'Invalid enrollment token')
+
     response = dm.DeviceManagementResponse()
     response.register_response.device_management_token = (
         'fake_device_management_token')

components/policy/core/common/BUILD.gn

@@ -66,6 +66,7 @@ source_set("internal") {
     "cloud/external_policy_data_updater.h",
     "cloud/machine_level_user_cloud_policy_manager.cc",
     "cloud/machine_level_user_cloud_policy_manager.h",
+    "cloud/machine_level_user_cloud_policy_metrics.h",
     "cloud/machine_level_user_cloud_policy_store.cc",
     "cloud/machine_level_user_cloud_policy_store.h",
     "cloud/policy_header_io_helper.cc",
@@ -210,6 +211,7 @@ source_set("internal") {
       "cloud/cloud_policy_client_registration_helper.h",
       "cloud/machine_level_user_cloud_policy_manager.cc",
       "cloud/machine_level_user_cloud_policy_manager.h",
+      "cloud/machine_level_user_cloud_policy_metrics.h",
       "cloud/machine_level_user_cloud_policy_store.cc",
       "cloud/machine_level_user_cloud_policy_store.h",
       "cloud/user_cloud_policy_manager.cc",

components/policy/core/common/cloud/machine_level_user_cloud_policy_metrics.h

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_POLICY_CORE_COMMON_CLOUD_MACHINE_LEVEL_USER_CLOUD_POLICY_METRICS_H_
+#define COMPONENTS_POLICY_CORE_COMMON_CLOUD_MACHINE_LEVEL_USER_CLOUD_POLICY_METRICS_H_
+
+namespace policy {
+
+// This enum is used for recording the metrics. It must match the
+// MachineLevelUserCloudPolicyEnrollmentResult in enums.xml and should not be
+// reordered. |kMaxValue| must be assigned to the last entry of the enum.
+enum MachineLevelUserCloudPolicyEnrollmentResult {
+  kSuccess = 0,
+  kFailedToFetch = 1,
+  kFailedToStore = 2,
+  kMaxValue = kFailedToStore,
+};
+
+}  // namespace policy
+
+#endif  // COMPONENTS_POLICY_CORE_COMMON_CLOUD_MACHINE_LEVEL_USER_CLOUD_POLICY_METRICS_H_