[CrOS Multidevice] Retrieve and persist SoftwareFeatures to and from ExternalDeviceInfo.

Bug: 824568, 752273
Change-Id: Ia5a814ef83e1869e4d3065f5c0afa402f1ec7d61
Reviewed-on: https://chromium-review.googlesource.com/1041222
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555838}

components/cryptauth/cryptauth_device_manager_impl.cc

@@ -50,6 +50,12 @@ const char kExternalDeviceKeyDeviceType[] = "device_type";
 const char kExternalDeviceKeyBeaconSeeds[] = "beacon_seeds";
 const char kExternalDeviceKeyArcPlusPlus[] = "arc_plus_plus";
 const char kExternalDeviceKeyPixelPhone[] = "pixel_phone";
+const char kExternalDeviceKeySupportedSoftwareFeatures[] =
+    "supported_software_features";
+const char kExternalDeviceKeyEnabledSoftwareFeatures[] =
+    "enabled_software_features";
+
+// Keys for ExternalDeviceInfo's BeaconSeed.
 const char kExternalDeviceKeyBeaconSeedData[] = "beacon_seed_data";
 const char kExternalDeviceKeyBeaconSeedStartMs[] = "beacon_seed_start_ms";
 const char kExternalDeviceKeyBeaconSeedEndMs[] = "beacon_seed_end_ms";
@@ -94,6 +100,16 @@ std::unique_ptr<base::ListValue> BeaconSeedsToListValue(
   return list;
 }
 
+// Converts SoftwareFeature protos to a list value that can be stored in user
+// prefs.
+std::unique_ptr<base::ListValue> SoftwareFeaturesToListValue(
+    const google::protobuf::RepeatedField<int>& software_features) {
+  std::unique_ptr<base::ListValue> list = std::make_unique<base::ListValue>();
+  for (auto software_feature : software_features)
+    list->AppendInteger(software_feature);
+  return list;
+}
+
 // Converts an unlock key proto to a dictionary that can be stored in user
 // prefs.
 std::unique_ptr<base::DictionaryValue> UnlockKeyToDictionary(
@@ -153,6 +169,9 @@ std::unique_ptr<base::DictionaryValue> UnlockKeyToDictionary(
     dictionary->SetInteger(kExternalDeviceKeyDeviceType, device.device_type());
   }
 
+  dictionary->Set(kExternalDeviceKeyBeaconSeeds,
+                  BeaconSeedsToListValue(device.beacon_seeds()));
+
   if (device.has_arc_plus_plus()) {
     dictionary->SetBoolean(kExternalDeviceKeyArcPlusPlus,
                            device.arc_plus_plus());
@@ -162,15 +181,18 @@ std::unique_ptr<base::DictionaryValue> UnlockKeyToDictionary(
     dictionary->SetBoolean(kExternalDeviceKeyPixelPhone, device.pixel_phone());
   }
 
-  std::unique_ptr<base::ListValue> beacon_seed_list =
-      BeaconSeedsToListValue(device.beacon_seeds());
-  dictionary->Set(kExternalDeviceKeyBeaconSeeds, std::move(beacon_seed_list));
+  dictionary->Set(
+      kExternalDeviceKeySupportedSoftwareFeatures,
+      SoftwareFeaturesToListValue(device.supported_software_features()));
+  dictionary->Set(
+      kExternalDeviceKeyEnabledSoftwareFeatures,
+      SoftwareFeaturesToListValue(device.enabled_software_features()));
 
   return dictionary;
 }
 
 void AddBeaconSeedsToExternalDevice(const base::ListValue& beacon_seeds,
-                                    ExternalDeviceInfo& external_device) {
+                                    ExternalDeviceInfo* external_device) {
   for (size_t i = 0; i < beacon_seeds.GetSize(); i++) {
     const base::DictionaryValue* seed_dictionary = nullptr;
     if (!beacon_seeds.GetDictionary(i, &seed_dictionary)) {
@@ -208,13 +230,46 @@ void AddBeaconSeedsToExternalDevice(const base::ListValue& beacon_seeds,
       continue;
     }
 
-    BeaconSeed* seed = external_device.add_beacon_seeds();
+    BeaconSeed* seed = external_device->add_beacon_seeds();
     seed->set_data(seed_data);
     seed->set_start_time_millis(start_time_millis);
     seed->set_end_time_millis(end_time_millis);
   }
 }
 
+void AddSoftwareFeaturesToExternalDevice(
+    const base::DictionaryValue& dictionary,
+    const std::string& software_feature_dictionary_key,
+    ExternalDeviceInfo* external_device) {
+  DCHECK(software_feature_dictionary_key ==
+             kExternalDeviceKeySupportedSoftwareFeatures ||
+         software_feature_dictionary_key ==
+             kExternalDeviceKeyEnabledSoftwareFeatures);
+
+  const base::ListValue* software_features;
+  if (!dictionary.GetList(software_feature_dictionary_key, &software_features))
+    return;
+
+  for (size_t i = 0; i < software_features->GetSize(); i++) {
+    int software_feature;
+    if (!software_features->GetInteger(i, &software_feature) ||
+        !SoftwareFeature_IsValid(software_feature)) {
+      PA_LOG(WARNING) << "Unable to retrieve SoftwareFeature; skipping.";
+      continue;
+    }
+
+    if (software_feature_dictionary_key ==
+        kExternalDeviceKeySupportedSoftwareFeatures) {
+      external_device->add_supported_software_features(
+          static_cast<SoftwareFeature>(software_feature));
+    } else if (software_feature_dictionary_key ==
+               kExternalDeviceKeyEnabledSoftwareFeatures) {
+      external_device->add_enabled_software_features(
+          static_cast<SoftwareFeature>(software_feature));
+    }
+  }
+}
+
 // Converts an unlock key dictionary stored in user prefs to an
 // ExternalDeviceInfo proto. Returns true if the dictionary is valid, and the
 // parsed proto is written to |external_device|.
@@ -294,7 +349,7 @@ bool DictionaryToUnlockKey(const base::DictionaryValue& dictionary,
   const base::ListValue* beacon_seeds = nullptr;
   dictionary.GetList(kExternalDeviceKeyBeaconSeeds, &beacon_seeds);
   if (beacon_seeds)
-    AddBeaconSeedsToExternalDevice(*beacon_seeds, *external_device);
+    AddBeaconSeedsToExternalDevice(*beacon_seeds, external_device);
 
   bool arc_plus_plus;
   if (dictionary.GetBoolean(kExternalDeviceKeyArcPlusPlus, &arc_plus_plus))
@@ -304,6 +359,11 @@ bool DictionaryToUnlockKey(const base::DictionaryValue& dictionary,
   if (dictionary.GetBoolean(kExternalDeviceKeyPixelPhone, &pixel_phone))
     external_device->set_pixel_phone(pixel_phone);
 
+  AddSoftwareFeaturesToExternalDevice(
+      dictionary, kExternalDeviceKeySupportedSoftwareFeatures, external_device);
+  AddSoftwareFeaturesToExternalDevice(
+      dictionary, kExternalDeviceKeyEnabledSoftwareFeatures, external_device);
+
   return true;
 }
 

components/cryptauth/cryptauth_device_manager_impl_unittest.cc

@@ -141,6 +141,27 @@ void ExpectSyncedDevicesAreEqual(
       EXPECT_TRUE(seed.has_end_time_millis());
       EXPECT_EQ(expected_seed.end_time_millis(), seed.end_time_millis());
     }
+
+    EXPECT_EQ(expected_device.has_arc_plus_plus(), device.has_arc_plus_plus());
+    EXPECT_EQ(expected_device.arc_plus_plus(), device.arc_plus_plus());
+
+    EXPECT_EQ(expected_device.has_pixel_phone(), device.has_pixel_phone());
+    EXPECT_EQ(expected_device.pixel_phone(), device.pixel_phone());
+
+    ASSERT_EQ(expected_device.supported_software_features_size(),
+              device.supported_software_features_size());
+    for (int i = 0; i < expected_device.supported_software_features_size();
+         i++) {
+      EXPECT_EQ(expected_device.supported_software_features(i),
+                device.supported_software_features(i));
+    }
+
+    ASSERT_EQ(expected_device.enabled_software_features_size(),
+              device.enabled_software_features_size());
+    for (int i = 0; i < expected_device.enabled_software_features_size(); i++) {
+      EXPECT_EQ(expected_device.enabled_software_features(i),
+                device.enabled_software_features(i));
+    }
   }
 }
 
@@ -243,7 +264,7 @@ void ExpectSyncedDevicesAndPrefAreEqual(
 
     const base::ListValue* beacon_seeds_from_prefs;
     if (device_dictionary->GetList("beacon_seeds", &beacon_seeds_from_prefs)) {
-      ASSERT_EQ((size_t)expected_device.beacon_seeds_size(),
+      ASSERT_EQ(static_cast<size_t>(expected_device.beacon_seeds_size()),
                 beacon_seeds_from_prefs->GetSize());
       for (size_t i = 0; i < beacon_seeds_from_prefs->GetSize(); i++) {
         const base::DictionaryValue* seed;
@@ -271,6 +292,58 @@ void ExpectSyncedDevicesAndPrefAreEqual(
     } else {
       EXPECT_FALSE(expected_device.beacon_seeds_size());
     }
+
+    bool arc_plus_plus;
+    if (device_dictionary->GetBoolean("arc_plus_plus", &arc_plus_plus)) {
+      EXPECT_TRUE(expected_device.has_arc_plus_plus());
+      EXPECT_EQ(expected_device.arc_plus_plus(), arc_plus_plus);
+    } else {
+      EXPECT_FALSE(expected_device.has_arc_plus_plus());
+    }
+
+    bool pixel_phone;
+    if (device_dictionary->GetBoolean("pixel_phone", &pixel_phone)) {
+      EXPECT_TRUE(expected_device.has_pixel_phone());
+      EXPECT_EQ(expected_device.pixel_phone(), pixel_phone);
+    } else {
+      EXPECT_FALSE(expected_device.has_pixel_phone());
+    }
+
+    const base::ListValue* supported_software_features_from_prefs;
+    if (device_dictionary->GetList("supported_software_features",
+                                   &supported_software_features_from_prefs)) {
+      ASSERT_EQ(static_cast<size_t>(
+                    expected_device.supported_software_features_size()),
+                supported_software_features_from_prefs->GetSize());
+      for (size_t i = 0; i < supported_software_features_from_prefs->GetSize();
+           i++) {
+        int supported_software_feature;
+        ASSERT_TRUE(supported_software_features_from_prefs->GetInteger(
+            i, &supported_software_feature));
+        EXPECT_EQ(expected_device.supported_software_features(i),
+                  supported_software_feature);
+      }
+    } else {
+      EXPECT_FALSE(expected_device.supported_software_features_size());
+    }
+
+    const base::ListValue* enabled_software_features_from_prefs;
+    if (device_dictionary->GetList("enabled_software_features",
+                                   &enabled_software_features_from_prefs)) {
+      ASSERT_EQ(
+          static_cast<size_t>(expected_device.enabled_software_features_size()),
+          enabled_software_features_from_prefs->GetSize());
+      for (size_t i = 0; i < enabled_software_features_from_prefs->GetSize();
+           i++) {
+        int enabled_software_feature;
+        ASSERT_TRUE(enabled_software_features_from_prefs->GetInteger(
+            i, &enabled_software_feature));
+        EXPECT_EQ(expected_device.enabled_software_features(i),
+                  enabled_software_feature);
+      }
+    } else {
+      EXPECT_FALSE(expected_device.enabled_software_features_size());
+    }
   }
 }
 
@@ -340,6 +413,12 @@ class CryptAuthDeviceManagerImplTest
     seed2->set_end_time_millis(kBeaconSeed2EndTime);
     unlock_key.set_arc_plus_plus(kArcPlusPlus1);
     unlock_key.set_pixel_phone(kPixelPhone1);
+    unlock_key.add_supported_software_features(
+        SoftwareFeature::BETTER_TOGETHER_HOST);
+    unlock_key.add_supported_software_features(
+        SoftwareFeature::BETTER_TOGETHER_CLIENT);
+    unlock_key.add_enabled_software_features(
+        SoftwareFeature::BETTER_TOGETHER_HOST);
     devices_in_response_.push_back(unlock_key);
 
     ExternalDeviceInfo unlockable_device;
@@ -358,6 +437,12 @@ class CryptAuthDeviceManagerImplTest
     seed4->set_end_time_millis(kBeaconSeed4EndTime);
     unlockable_device.set_arc_plus_plus(kArcPlusPlus2);
     unlockable_device.set_pixel_phone(kPixelPhone2);
+    unlock_key.add_supported_software_features(
+        SoftwareFeature::MAGIC_TETHER_HOST);
+    unlock_key.add_supported_software_features(
+        SoftwareFeature::MAGIC_TETHER_CLIENT);
+    unlock_key.add_enabled_software_features(
+        SoftwareFeature::MAGIC_TETHER_HOST);
     devices_in_response_.push_back(unlockable_device);
   }
 
@@ -397,9 +482,13 @@ class CryptAuthDeviceManagerImplTest
     device_dictionary->SetString("bluetooth_address", bluetooth_address_b64);
     device_dictionary->SetBoolean("unlock_key", kStoredUnlockKey);
     device_dictionary->SetBoolean("unlockable", kStoredUnlockable);
-    device_dictionary->Set("beacon_seeds", std::make_unique<base::ListValue>());
     device_dictionary->SetBoolean("mobile_hotspot_supported",
                                   kStoredMobileHotspotSupported);
+    device_dictionary->Set("beacon_seeds", std::make_unique<base::ListValue>());
+    device_dictionary->Set("supported_software_features",
+                           std::make_unique<base::ListValue>());
+    device_dictionary->Set("enabled_software_features",
+                           std::make_unique<base::ListValue>());
     {
       ListPrefUpdate update(&pref_service_,
                             prefs::kCryptAuthDeviceSyncUnlockKeys);
@@ -797,8 +886,6 @@ TEST_F(CryptAuthDeviceManagerImplTest, SyncDeviceWithNoContents) {
 }
 
 TEST_F(CryptAuthDeviceManagerImplTest, SyncFullyDetailedExternalDeviceInfos) {
-  GetMyDevicesResponse response;
-
   // First, use a device with only a public key (a public key is the only
   // required field). This ensures devices work properly when they do not have
   // all fields filled out.
@@ -809,7 +896,6 @@ TEST_F(CryptAuthDeviceManagerImplTest, SyncFullyDetailedExternalDeviceInfos) {
   // TODO(khorimoto): Remove this when support for storing all types of devices
   // is added.
   device_with_only_public_key.set_unlock_key(true);
-  response.add_devices()->CopyFrom(device_with_only_public_key);
 
   // Second, use a device with all fields filled out. This ensures that all
   // device details are properly saved.
@@ -822,17 +908,29 @@ TEST_F(CryptAuthDeviceManagerImplTest, SyncFullyDetailedExternalDeviceInfos) {
   device_with_all_fields.set_last_update_time_millis(123456789L);
   device_with_all_fields.set_mobile_hotspot_supported(true);
   device_with_all_fields.set_device_type(DeviceType::ANDROIDOS);
+
   BeaconSeed seed1;
   seed1.set_data(kBeaconSeed1Data);
   seed1.set_start_time_millis(kBeaconSeed1StartTime);
   seed1.set_end_time_millis(kBeaconSeed1EndTime);
   device_with_all_fields.add_beacon_seeds()->CopyFrom(seed1);
+
   BeaconSeed seed2;
   seed2.set_data(kBeaconSeed2Data);
   seed2.set_start_time_millis(kBeaconSeed2StartTime);
   seed2.set_end_time_millis(kBeaconSeed2EndTime);
   device_with_all_fields.add_beacon_seeds()->CopyFrom(seed2);
-  response.add_devices()->CopyFrom(device_with_all_fields);
+
+  device_with_all_fields.set_arc_plus_plus(true);
+  device_with_all_fields.set_pixel_phone(true);
+
+  device_with_all_fields.add_supported_software_features(
+      SoftwareFeature::EASY_UNLOCK_HOST);
+  device_with_all_fields.add_supported_software_features(
+      SoftwareFeature::MAGIC_TETHER_HOST);
+
+  device_with_all_fields.add_enabled_software_features(
+      SoftwareFeature::MAGIC_TETHER_HOST);
 
   std::vector<ExternalDeviceInfo> expected_devices;
   expected_devices.push_back(device_with_only_public_key);
@@ -844,6 +942,10 @@ TEST_F(CryptAuthDeviceManagerImplTest, SyncFullyDetailedExternalDeviceInfos) {
   EXPECT_CALL(*this, OnSyncFinishedProxy(
                          CryptAuthDeviceManager::SyncResult::SUCCESS,
                          CryptAuthDeviceManager::DeviceChangeResult::CHANGED));
+
+  GetMyDevicesResponse response;
+  response.add_devices()->CopyFrom(device_with_only_public_key);
+  response.add_devices()->CopyFrom(device_with_all_fields);
   success_callback_.Run(response);
 
   ExpectSyncedDevicesAndPrefAreEqual(