Upgrade StoreDMToken and RetrieveDMToken.

Changes the DM Token Retrieve/Store from returning only a string to
returning the token string and a status. The statuses are:
Valid: The DM token is non-empty and its string value can be used.
Invalidated: The DM token has been explicitly invalidated. Should be
             treated as if the device was not enrolled.
Empty: The DM token is empty. Enrollment might occur if the enrollment
       token is non-empty.

Bug: 1020294
Change-Id: I1418615edc82e65f15325da2fca008c8cdab5ca0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1879340Reviewed-by: Owen Min <zmin@chromium.org>
Reviewed-by: Tien Mai <tienmai@chromium.org>
Commit-Queue: Dominique Fauteux-Chapleau <domfc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713129}

chrome/browser/policy/browser_dm_token_storage.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/policy/browser_dm_token_storage.h"
 
 #include <memory>
+#include <string>
 #include <utility>
 #include <vector>
 
@@ -28,6 +29,9 @@
 namespace policy {
 
 namespace {
+
+constexpr char kInvalidTokenValue[] = "INVALID_DM_TOKEN";
+
 void OnHardwarePlatformInfo(base::OnceClosure quit_closure,
                             std::string* out,
                             base::SysInfo::HardwareInfo info) {
@@ -36,10 +40,48 @@ void OnHardwarePlatformInfo(base::OnceClosure quit_closure,
 }
 }  // namespace
 
+using BrowserDMToken = BrowserDMTokenStorage::BrowserDMToken;
+
 // static
 BrowserDMTokenStorage* BrowserDMTokenStorage::storage_for_testing_ = nullptr;
 
-BrowserDMTokenStorage::BrowserDMTokenStorage() : is_initialized_(false) {
+BrowserDMToken BrowserDMToken::CreateValidToken(const std::string& dm_token) {
+  DCHECK_NE(dm_token, kInvalidTokenValue);
+  DCHECK(!dm_token.empty());
+  return BrowserDMToken(Status::kValid, dm_token);
+}
+
+BrowserDMToken BrowserDMToken::CreateInvalidToken() {
+  return BrowserDMToken(Status::kInvalid, "");
+}
+
+BrowserDMToken BrowserDMToken::CreateEmptyToken() {
+  return BrowserDMToken(Status::kEmpty, "");
+}
+
+const std::string& BrowserDMToken::value() const {
+  // TODO(domfc): Uncomment DCHECK(is_valid()) after migrating code.
+  // DCHECK(is_valid());
+  return value_;
+}
+
+bool BrowserDMToken::is_valid() const {
+  return status_ == Status::kValid;
+}
+
+bool BrowserDMToken::is_invalid() const {
+  return status_ == Status::kInvalid;
+}
+
+bool BrowserDMToken::is_empty() const {
+  return status_ == Status::kEmpty;
+}
+
+BrowserDMToken::BrowserDMToken(Status status, const base::StringPiece value)
+    : status_(status), value_(value) {}
+
+BrowserDMTokenStorage::BrowserDMTokenStorage()
+    : is_initialized_(false), dm_token_(BrowserDMToken::CreateEmptyToken()) {
   DETACH_FROM_SEQUENCE(sequence_checker_);
 
   // We don't call InitIfNeeded() here so that the global instance can be
@@ -78,6 +120,17 @@ std::string BrowserDMTokenStorage::RetrieveEnrollmentToken() {
 
 void BrowserDMTokenStorage::StoreDMToken(const std::string& dm_token,
                                          StoreCallback callback) {
+  if (dm_token.empty())
+    StoreDMToken(BrowserDMToken::CreateEmptyToken(), std::move(callback));
+  else if (dm_token == kInvalidTokenValue)
+    StoreDMToken(BrowserDMToken::CreateInvalidToken(), std::move(callback));
+  else
+    StoreDMToken(BrowserDMToken::CreateValidToken(dm_token),
+                 std::move(callback));
+}
+
+void BrowserDMTokenStorage::StoreDMToken(const BrowserDMToken& dm_token,
+                                         StoreCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(!store_callback_);
   InitIfNeeded();
@@ -86,10 +139,17 @@ void BrowserDMTokenStorage::StoreDMToken(const std::string& dm_token,
 
   store_callback_ = std::move(callback);
 
-  SaveDMToken(dm_token);
+  if (dm_token_.is_invalid())
+    SaveDMToken(kInvalidTokenValue);
+  else
+    SaveDMToken(dm_token_.value());
 }
 
 std::string BrowserDMTokenStorage::RetrieveDMToken() {
+  return RetrieveBrowserDMToken().value();
+}
+
+BrowserDMToken BrowserDMTokenStorage::RetrieveBrowserDMToken() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(!store_callback_);
 
@@ -129,8 +189,22 @@ void BrowserDMTokenStorage::InitIfNeeded() {
   enrollment_token_ = InitEnrollmentToken();
   DVLOG(1) << "Enrollment token = " << enrollment_token_;
 
-  dm_token_ = InitDMToken();
-  DVLOG(1) << "DM Token = " << dm_token_;
+  std::string init_dm_token = InitDMToken();
+  if (init_dm_token.empty())
+    dm_token_ = BrowserDMToken::CreateEmptyToken();
+  else if (init_dm_token == kInvalidTokenValue)
+    dm_token_ = BrowserDMToken::CreateInvalidToken();
+  else
+    dm_token_ = BrowserDMToken::CreateValidToken(init_dm_token);
+
+  if (dm_token_.is_valid())
+    DVLOG(1) << "DM Token = " << dm_token_.value();
+  else if (dm_token_.is_empty())
+    DVLOG(1) << "DM Token = empty";
+  else if (dm_token_.is_invalid())
+    DVLOG(1) << "DM Token = invalid";
+  else
+    DVLOG(1) << "DM Token = unknown status";
 
   should_display_error_message_on_failure_ = InitEnrollmentErrorOption();
 }

chrome/browser/policy/browser_dm_token_storage.h

@@ -17,6 +17,7 @@
 #include "base/run_loop.h"
 #include "base/sequence_checker.h"
 #include "base/single_thread_task_runner.h"
+#include "base/strings/string_piece_forward.h"
 #include "base/system/sys_info.h"
 
 namespace policy {
@@ -30,6 +31,38 @@ namespace policy {
 // called.
 class BrowserDMTokenStorage {
  public:
+  // Represents the browser's DM token with its status, which can be:
+  // kValid:
+  //    A valid token to be used to make requests. Its value cannot be empty or
+  //    equal to |kInvalidTokenValue|.
+  // kInvalid:
+  //    The token explicitly marks this browser as unenrolled. The browser
+  //    should not sync policies or try to get a new DM token if it is set to
+  //    this value.
+  // kEmpty:
+  //    The token is empty. The browser will try to get a valid token if an
+  //    enrollment token is present.
+  class BrowserDMToken {
+   public:
+    const std::string& value() const;
+
+    bool is_valid() const;
+    bool is_invalid() const;
+    bool is_empty() const;
+
+    static BrowserDMToken CreateValidToken(const std::string& value);
+    static BrowserDMToken CreateInvalidToken();
+    static BrowserDMToken CreateEmptyToken();
+
+   private:
+    enum class Status { kValid, kInvalid, kEmpty };
+
+    BrowserDMToken();
+    BrowserDMToken(Status status, const base::StringPiece value);
+
+    Status status_;
+    std::string value_;
+  };
   using StoreCallback = base::OnceCallback<void(bool success)>;
 
   // Returns the global singleton object. Must be called from the UI thread.
@@ -44,10 +77,16 @@ class BrowserDMTokenStorage {
   // Asynchronously stores |dm_token| and calls |callback| with a boolean to
   // indicate success or failure. It is an error to attempt concurrent store
   // operations.
+  // TODO(domfc): Remove overload after updating callers.
   void StoreDMToken(const std::string& dm_token, StoreCallback callback);
+  void StoreDMToken(const BrowserDMToken& dm_token, StoreCallback callback);
   // Returns an already stored DM token. An empty token is returned if no DM
   // token exists on the system or an error is encountered.
+  // TODO(domfc): Remove overload after updating callers. Note that the names
+  //              are different because you cannot overload functions that only
+  //              differ in their return type.
   std::string RetrieveDMToken();
+  BrowserDMToken RetrieveBrowserDMToken();
   // Must be called after the DM token is saved, to ensure that the callback is
   // invoked.
   void OnDMTokenStored(bool success);
@@ -102,7 +141,7 @@ class BrowserDMTokenStorage {
   std::string client_id_;
   base::Optional<std::string> serial_number_;
   std::string enrollment_token_;
-  std::string dm_token_;
+  BrowserDMToken dm_token_;
   bool should_display_error_message_on_failure_;
 
   SEQUENCE_CHECKER(sequence_checker_);

chrome/browser/policy/browser_dm_token_storage_unittest.cc

@@ -27,18 +27,74 @@ constexpr char kEnrollmentToken2[] = "fake-enrollment-token-2";
 constexpr char kDMToken1[] = "fake-dm-token-1";
 constexpr char kDMToken2[] = "fake-dm-token-2";
 
-}  // namespace
-
-class BrowserDMTokenStorageTest : public testing::Test {
+class BrowserDMTokenStorageTestBase {
  public:
-  BrowserDMTokenStorageTest()
-      : storage_(kClientId1, kEnrollmentToken1, kDMToken1, false) {}
+  BrowserDMTokenStorageTestBase(const std::string& client_id,
+                                const std::string& enrollment_token,
+                                const std::string& dm_token,
+                                const bool enrollment_error_option)
+      : storage_(client_id,
+                 enrollment_token,
+                 dm_token,
+                 enrollment_error_option) {}
   FakeBrowserDMTokenStorage storage_;
 
  private:
   content::BrowserTaskEnvironment task_environment_;
 };
 
+class BrowserDMTokenStorageTest : public BrowserDMTokenStorageTestBase,
+                                  public testing::Test {
+ public:
+  BrowserDMTokenStorageTest()
+      : BrowserDMTokenStorageTestBase(kClientId1,
+                                      kEnrollmentToken1,
+                                      kDMToken1,
+                                      false) {}
+};
+
+struct StoreAndRetrieveTestParams {
+ public:
+  StoreAndRetrieveTestParams(const std::string& dm_token_to_store,
+                             const std::string& expected_retrieved_dm_token,
+                             bool expect_valid,
+                             bool expect_invalid,
+                             bool expect_empty)
+      : dm_token_to_store(dm_token_to_store),
+        expected_retrieved_dm_token(expected_retrieved_dm_token),
+        expect_valid(expect_valid),
+        expect_invalid(expect_invalid),
+        expect_empty(expect_empty) {}
+
+  std::string dm_token_to_store;
+  std::string expected_retrieved_dm_token;
+  bool expect_valid;
+  bool expect_invalid;
+  bool expect_empty;
+};
+
+class BrowserDMTokenStorageStoreAndRetrieveTest
+    : public BrowserDMTokenStorageTestBase,
+      public testing::TestWithParam<StoreAndRetrieveTestParams> {
+ public:
+  BrowserDMTokenStorageStoreAndRetrieveTest()
+      : BrowserDMTokenStorageTestBase(kClientId1,
+                                      kEnrollmentToken1,
+                                      GetParam().dm_token_to_store,
+                                      false) {}
+};
+
+}  // namespace
+
+INSTANTIATE_TEST_SUITE_P(
+    BrowserDMTokenStorageStoreAndRetrieveTest,
+    BrowserDMTokenStorageStoreAndRetrieveTest,
+    testing::Values(
+        StoreAndRetrieveTestParams(kDMToken1, kDMToken1, true, false, false),
+        StoreAndRetrieveTestParams(kDMToken2, kDMToken2, true, false, false),
+        StoreAndRetrieveTestParams("INVALID_DM_TOKEN", "", false, true, false),
+        StoreAndRetrieveTestParams("", "", false, false, true)));
+
 TEST_F(BrowserDMTokenStorageTest, RetrieveClientId) {
   EXPECT_EQ(kClientId1, storage_.RetrieveClientId());
   // The client ID value should be cached in memory and not read from the system
@@ -56,12 +112,31 @@ TEST_F(BrowserDMTokenStorageTest, RetrieveEnrollmentToken) {
   EXPECT_EQ(kEnrollmentToken1, storage_.RetrieveEnrollmentToken());
 }
 
-TEST_F(BrowserDMTokenStorageTest, RetrieveDMToken) {
-  EXPECT_EQ(kDMToken1, storage_.RetrieveDMToken());
+TEST_P(BrowserDMTokenStorageStoreAndRetrieveTest, StoreDMToken) {
+  storage_.SetDMToken(GetParam().dm_token_to_store);
+  auto dm_token = storage_.RetrieveBrowserDMToken();
+  if (GetParam().expect_valid || GetParam().expect_empty) {
+    EXPECT_EQ(GetParam().expected_retrieved_dm_token, dm_token.value());
+  }
+  EXPECT_EQ(GetParam().expect_valid, dm_token.is_valid());
+  EXPECT_EQ(GetParam().expect_invalid, dm_token.is_invalid());
+  EXPECT_EQ(GetParam().expect_empty, dm_token.is_empty());
 
   // The DM token should be cached in memory and not read from the system again.
-  storage_.SetDMToken(kDMToken2);
-  EXPECT_EQ(kDMToken1, storage_.RetrieveDMToken());
+  storage_.SetDMToken("not_saved");
+  if (GetParam().expect_valid || GetParam().expect_empty) {
+    EXPECT_EQ(GetParam().expected_retrieved_dm_token, dm_token.value());
+  }
+}
+
+TEST_P(BrowserDMTokenStorageStoreAndRetrieveTest, RetrieveDMToken) {
+  auto dm_token = storage_.RetrieveBrowserDMToken();
+  if (GetParam().expect_valid || GetParam().expect_empty) {
+    EXPECT_EQ(GetParam().expected_retrieved_dm_token, dm_token.value());
+  }
+  EXPECT_EQ(GetParam().expect_valid, dm_token.is_valid());
+  EXPECT_EQ(GetParam().expect_invalid, dm_token.is_invalid());
+  EXPECT_EQ(GetParam().expect_empty, dm_token.is_empty());
 }
 
 TEST_F(BrowserDMTokenStorageTest, ShouldDisplayErrorMessageOnFailure) {