Store/restore persistent USB permissions.

Stores and restores persistent USB permission with Chrome prefs.

Bug: 776476
Bug: b:24572867
Change-Id: I008166a32d47cba2edbdc54858b807491add6858
Reviewed-on: https://chromium-review.googlesource.com/944249
Commit-Queue: Long Cheng <lgcheng@google.com>
Reviewed-by: Yury Khmel <khmel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541679}

chrome/browser/ui/app_list/arc/arc_app_list_prefs.cc

@@ -832,6 +832,27 @@ void ArcAppListPrefs::RegisterDefaultApps() {
   }
 }
 
+base::Value* ArcAppListPrefs::GetPackagePrefs(const std::string& package_name,
+                                              const std::string& key) {
+  if (!GetPackage(package_name)) {
+    LOG(ERROR) << package_name << " can not be found.";
+    return nullptr;
+  }
+  ScopedArcPrefUpdate update(prefs_, package_name, arc::prefs::kArcPackages);
+  return update.Get()->FindKey(key);
+}
+
+void ArcAppListPrefs::SetPackagePrefs(const std::string& package_name,
+                                      const std::string& key,
+                                      base::Value value) {
+  if (!GetPackage(package_name)) {
+    LOG(ERROR) << package_name << " can not be found.";
+    return;
+  }
+  ScopedArcPrefUpdate update(prefs_, package_name, arc::prefs::kArcPackages);
+  update.Get()->SetKey(key, std::move(value));
+}
+
 void ArcAppListPrefs::SetDefaltAppsReadyCallback(base::Closure callback) {
   DCHECK(!callback.is_null());
   DCHECK(default_apps_ready_callback_.is_null());

chrome/browser/ui/app_list/arc/arc_app_list_prefs.h

@@ -268,6 +268,14 @@ class ArcAppListPrefs : public KeyedService,
   std::unordered_set<std::string> GetAppsForPackage(
       const std::string& package_name) const;
 
+  // Gets Chrome prefs for given |package_name| and |key|.
+  base::Value* GetPackagePrefs(const std::string& package_name,
+                               const std::string& key);
+  // Sets Chrome prefs for given |package_name| and |key| to |value|.
+  void SetPackagePrefs(const std::string& package_name,
+                       const std::string& key,
+                       base::Value value);
+
   void SetDefaltAppsReadyCallback(base::Closure callback);
   void SimulateDefaultAppAvailabilityTimeoutForTesting();
 

chrome/browser/ui/app_list/arc/arc_usb_host_permission_browsertest.cc

+// 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 "chrome/browser/ui/app_list/arc/arc_usb_host_permission_manager.h"
+
+#include "base/command_line.h"
+#include "base/macros.h"
+#include "base/run_loop.h"
+#include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/chromeos/arc/arc_session_manager.h"
+#include "chrome/browser/chromeos/arc/arc_util.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/app_list/app_list_service.h"
+#include "chrome/browser/ui/app_list/arc/arc_app_list_prefs.h"
+#include "chrome/browser/ui/app_list/arc/arc_app_list_prefs_factory.h"
+#include "chrome/browser/ui/app_list/arc/arc_app_utils.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "components/arc/arc_util.h"
+#include "components/arc/common/app.mojom.h"
+#include "components/arc/test/connection_holder_util.h"
+#include "components/arc/test/fake_app_instance.h"
+#include "content/public/test/test_utils.h"
+
+namespace arc {
+
+namespace {
+
+// Test package strings.
+constexpr char kAppName[] = "test.app.name";
+constexpr char kAppActivity[] = "test.app.activity";
+constexpr char kPackageName[] = "test.app.package.name";
+
+}  // namespace
+
+class ArcUsbHostPermissionTest : public InProcessBrowserTest {
+ public:
+  ArcUsbHostPermissionTest() = default;
+
+  ~ArcUsbHostPermissionTest() override = default;
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    arc::SetArcAvailableCommandLineForTesting(command_line);
+  }
+
+  void SetUpInProcessBrowserTestFixture() override {
+    ArcSessionManager::DisableUIForTesting();
+  }
+
+  void SetUpOnMainThread() override {
+    profile_ = browser()->profile();
+    arc::SetArcPlayStoreEnabledForProfile(profile_, true);
+
+    arc_app_list_pref_ = ArcAppListPrefs::Get(profile_);
+    DCHECK(arc_app_list_pref_);
+
+    base::RunLoop run_loop;
+    arc_app_list_pref_->SetDefaltAppsReadyCallback(run_loop.QuitClosure());
+    run_loop.Run();
+
+    app_instance_.reset(new arc::FakeAppInstance(arc_app_list_pref_));
+    arc_app_list_pref_->app_connection_holder()->SetInstance(
+        app_instance_.get());
+    WaitForInstanceReady(arc_app_list_pref_->app_connection_holder());
+
+    arc_usb_permission_manager_ =
+        ArcUsbHostPermissionManager::GetForBrowserContext(profile_);
+    DCHECK(arc_usb_permission_manager_);
+  }
+
+  void TearDownOnMainThread() override {
+    arc_app_list_pref_->app_connection_holder()->CloseInstance(
+        app_instance_.get());
+    app_instance_.reset();
+    ArcSessionManager::Get()->Shutdown();
+  }
+
+ protected:
+  void AddArcApp(const std::string& app_name,
+                 const std::string& package_name,
+                 const std::string& activity) {
+    arc::mojom::AppInfo app_info;
+    app_info.name = app_name;
+    app_info.package_name = package_name;
+    app_info.activity = activity;
+
+    app_instance_->SendPackageAppListRefreshed(package_name, {app_info});
+  }
+
+  void AddArcPackage(const std::string& package_name) {
+    arc::mojom::ArcPackageInfo package;
+    package.package_name = package_name;
+    package.package_version = 0;
+    package.last_backup_android_id = 0;
+    package.last_backup_time = 0;
+    package.sync = false;
+    app_instance_->SendPackageAdded(package);
+  }
+
+  void RemovePackage(const std::string& package_name) {
+    app_instance_->UninstallPackage(package_name);
+  }
+
+  void DeviceRemoved(const std::string& guid) {
+    arc_usb_permission_manager_->DeviceRemoved(guid);
+  }
+
+  void RestorePermissionFromChromePrefs() {
+    arc_usb_permission_manager_->RestorePermissionFromChromePrefs();
+  }
+
+  void UpdateArcUsbScanDeviceListPermission(const std::string& package_name,
+                                            bool allowed) {
+    arc_usb_permission_manager_->UpdateArcUsbScanDeviceListPermission(
+        package_name, allowed);
+  }
+
+  void UpdateArcUsbAccessPermission(
+      const std::string& package_name,
+      const ArcUsbHostPermissionManager::UsbDeviceEntry& usb_device_entry,
+      bool allowed) {
+    arc_usb_permission_manager_->UpdateArcUsbAccessPermission(
+        package_name, usb_device_entry, allowed);
+  }
+
+  bool HasUsbScanDeviceListPermission(const std::string& package_name) const {
+    return arc_usb_permission_manager_->HasUsbScanDeviceListPermission(
+        package_name);
+  }
+
+  bool HasUsbAccessPermission(
+      const std::string& package_name,
+      const ArcUsbHostPermissionManager::UsbDeviceEntry& usb_device_entry) {
+    return arc_usb_permission_manager_->HasUsbAccessPermission(
+        package_name, usb_device_entry);
+  }
+
+  void ClearPermissions() {
+    arc_usb_permission_manager_->ClearPermissionForTesting();
+  }
+
+ private:
+  ArcAppListPrefs* arc_app_list_pref_;
+  ArcUsbHostPermissionManager* arc_usb_permission_manager_;
+  std::unique_ptr<FakeAppInstance> app_instance_;
+  Profile* profile_;
+
+  DISALLOW_COPY_AND_ASSIGN(ArcUsbHostPermissionTest);
+};
+
+IN_PROC_BROWSER_TEST_F(ArcUsbHostPermissionTest, UsbChromePrefsTest) {
+  AddArcApp(kAppName, kPackageName, kAppActivity);
+  AddArcPackage(kPackageName);
+
+  // Persistent device0.
+  const std::string guid0 = "TestGuidXXXXXX0";
+  const base::string16 device_name0 = base::UTF8ToUTF16("TestDevice0");
+  const base::string16 serial_number0 = base::UTF8ToUTF16("TestSerialNumber0");
+  uint16_t vendor_id0 = 123;
+  uint16_t product_id0 = 456;
+  // Persistent device1.
+  const std::string guid1 = "TestGuidXXXXXX1";
+  const base::string16 device_name1 = base::UTF8ToUTF16("TestDevice1");
+  const base::string16 serial_number1 = base::UTF8ToUTF16("TestSerialNumber1");
+  uint16_t vendor_id1 = 234;
+  uint16_t product_id1 = 567;
+  // Non persistent device2.
+  const std::string guid2 = "TestGuidXXXXXX2";
+  const base::string16 device_name2 = base::UTF8ToUTF16("TestDevice2");
+  uint16_t vendor_id2 = 345;
+  uint16_t product_id2 = 678;
+
+  ArcUsbHostPermissionManager::UsbDeviceEntry testDevice0(
+      guid0, device_name0, serial_number0, vendor_id0, product_id0);
+  ArcUsbHostPermissionManager::UsbDeviceEntry testDevice1(
+      guid1, device_name1, serial_number1, vendor_id1, product_id1);
+  ArcUsbHostPermissionManager::UsbDeviceEntry testDevice2(
+      guid2, device_name2, base::string16() /*serial_number*/, vendor_id2,
+      product_id2);
+
+  EXPECT_FALSE(HasUsbScanDeviceListPermission(kPackageName));
+  EXPECT_FALSE(HasUsbAccessPermission(kPackageName, testDevice0));
+  EXPECT_FALSE(HasUsbAccessPermission(kPackageName, testDevice1));
+  EXPECT_FALSE(HasUsbAccessPermission(kPackageName, testDevice2));
+
+  UpdateArcUsbScanDeviceListPermission(kPackageName, true /*allowed*/);
+  UpdateArcUsbAccessPermission(kPackageName, testDevice0, true /*allowed*/);
+  UpdateArcUsbAccessPermission(kPackageName, testDevice1, true /*allowed*/);
+  UpdateArcUsbAccessPermission(kPackageName, testDevice2, true /*allowed*/);
+
+  EXPECT_TRUE(HasUsbScanDeviceListPermission(kPackageName));
+  EXPECT_TRUE(HasUsbAccessPermission(kPackageName, testDevice0));
+  EXPECT_TRUE(HasUsbAccessPermission(kPackageName, testDevice1));
+  EXPECT_TRUE(HasUsbAccessPermission(kPackageName, testDevice2));
+
+  // Remove all devices. Permission for persistent device should remain.
+  DeviceRemoved(guid0);
+  DeviceRemoved(guid1);
+  DeviceRemoved(guid2);
+  EXPECT_TRUE(HasUsbScanDeviceListPermission(kPackageName));
+  EXPECT_TRUE(HasUsbAccessPermission(kPackageName, testDevice0));
+  EXPECT_TRUE(HasUsbAccessPermission(kPackageName, testDevice1));
+  EXPECT_FALSE(HasUsbAccessPermission(kPackageName, testDevice2));
+
+  ClearPermissions();
+  EXPECT_FALSE(HasUsbScanDeviceListPermission(kPackageName));
+  EXPECT_FALSE(HasUsbAccessPermission(kPackageName, testDevice0));
+  EXPECT_FALSE(HasUsbAccessPermission(kPackageName, testDevice1));
+  EXPECT_FALSE(HasUsbAccessPermission(kPackageName, testDevice2));
+
+  // Resotre permission from Chrome prefs. Permission for persistent devices
+  // should be restored.
+  RestorePermissionFromChromePrefs();
+  EXPECT_TRUE(HasUsbScanDeviceListPermission(kPackageName));
+  EXPECT_TRUE(HasUsbAccessPermission(kPackageName, testDevice0));
+  EXPECT_TRUE(HasUsbAccessPermission(kPackageName, testDevice1));
+  EXPECT_FALSE(HasUsbAccessPermission(kPackageName, testDevice2));
+
+  // Remove the package. All permission are gone.
+  ClearPermissions();
+  RemovePackage(kPackageName);
+  RestorePermissionFromChromePrefs();
+  EXPECT_FALSE(HasUsbScanDeviceListPermission(kPackageName));
+  EXPECT_FALSE(HasUsbAccessPermission(kPackageName, testDevice0));
+  EXPECT_FALSE(HasUsbAccessPermission(kPackageName, testDevice1));
+  EXPECT_FALSE(HasUsbAccessPermission(kPackageName, testDevice2));
+}
+
+}  // namespace arc

chrome/browser/ui/app_list/arc/arc_usb_host_permission_manager.cc

@@ -6,6 +6,7 @@
 
 #include <utility>
 
+#include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/app_list/arc/arc_app_dialog.h"
 #include "chrome/browser/ui/app_list/arc/arc_usb_host_permission_manager_factory.h"
@@ -16,6 +17,14 @@ namespace arc {
 
 namespace {
 
+constexpr char kUsbAccessPermission[] = "usb_access_permission";
+constexpr char kUsbDeviceName[] = "usb_device_name";
+constexpr char kUsbProductId[] = "usb_product_id";
+constexpr char kUsbScanDeviceListPermission[] =
+    "usb_scan_devicelist_permission";
+constexpr char kUsbSerialNumber[] = "usb_serial_number";
+constexpr char kUsbVendorId[] = "usb_vendor_id";
+
 std::string GetAppIdFromPackageName(const std::string& package_name,
                                     const ArcAppListPrefs* arc_app_list_prefs) {
   DCHECK(arc_app_list_prefs);
@@ -29,6 +38,20 @@ std::string GetAppIdFromPackageName(const std::string& package_name,
   return app_ids.empty() ? std::string() : *app_ids.begin();
 }
 
+bool GetUint16FromDict(const base::Value& dict,
+                       const std::string& key,
+                       uint16_t* result) {
+  const base::Value* value =
+      dict.FindKeyOfType(key, base::Value::Type::INTEGER);
+  if (!value)
+    return false;
+  int tmp = value->GetInt();
+  if (tmp < 0 || tmp > UINT16_MAX)
+    return false;
+  *result = static_cast<uint16_t>(tmp);
+  return true;
+}
+
 }  // namespace
 
 // UsbPermissionRequest
@@ -125,9 +148,63 @@ ArcUsbHostPermissionManager::~ArcUsbHostPermissionManager() {
 }
 
 void ArcUsbHostPermissionManager::RestorePermissionFromChromePrefs() {
-  // TODO(lgcheng): Restores scan devicelist permissions.
+  const std::vector<std::string> packages =
+      arc_app_list_prefs_->GetPackagesFromPrefs();
+  for (auto& package : packages) {
+    // Restore scan device list permission.
+    const base::Value* scan_permision_value =
+        arc_app_list_prefs_->GetPackagePrefs(package,
+                                             kUsbScanDeviceListPermission);
+    if (scan_permision_value && scan_permision_value->is_bool() &&
+        scan_permision_value->GetBool()) {
+      usb_scan_devicelist_permission_packages_.emplace(package);
+    }
+
+    // Restore device accesss permission.
+    const base::Value* access_permision_list_value =
+        arc_app_list_prefs_->GetPackagePrefs(package, kUsbAccessPermission);
+    if (!access_permision_list_value)
+      return;
 
-  // TODO(lgcheng): Restores persistent device access permissions.
+    if (!access_permision_list_value->is_list()) {
+      LOG(ERROR) << "Wrong value type found for device access permission list.";
+      return;
+    }
+
+    for (const auto& access_permision :
+         access_permision_list_value->GetList()) {
+      const base::Value* serial_number_value = access_permision.FindKeyOfType(
+          kUsbSerialNumber, base::Value::Type::STRING);
+      if (!serial_number_value) {
+        LOG(WARNING) << "Invalid device access permisison: No serial number.";
+        continue;
+      }
+
+      const base::Value* device_name_value = access_permision.FindKeyOfType(
+          kUsbDeviceName, base::Value::Type::STRING);
+      if (!device_name_value) {
+        LOG(WARNING) << "Invalid device access permisison: No device name.";
+        continue;
+      }
+
+      uint16_t vendor_id;
+      uint16_t product_id;
+
+      if (!GetUint16FromDict(access_permision, kUsbVendorId, &vendor_id) ||
+          !GetUint16FromDict(access_permision, kUsbProductId, &product_id)) {
+        LOG(WARNING) << "Invalid device access permisison: Invalid vendor_id "
+                        "and/or product_id.";
+        continue;
+      }
+
+      usb_access_permission_dict_.emplace(std::make_pair(
+          package,
+          UsbDeviceEntry(std::string() /*guid*/,
+                         base::UTF8ToUTF16(device_name_value->GetString()),
+                         base::UTF8ToUTF16(serial_number_value->GetString()),
+                         vendor_id, product_id)));
+    }
+  }
 }
 
 void ArcUsbHostPermissionManager::RequestUsbScanDeviceListPermission(
@@ -198,7 +275,7 @@ void ArcUsbHostPermissionManager::DeviceRemoved(const std::string& guid) {
   // Remove runtime permissions.
   for (auto iter = usb_access_permission_dict_.begin();
        iter != usb_access_permission_dict_.end();) {
-    auto& usb_device_entry = iter->second;
+    UsbDeviceEntry& usb_device_entry = iter->second;
     if (!usb_device_entry.IsPersistent() && usb_device_entry.guid == guid)
       iter = usb_access_permission_dict_.erase(iter);
     else
@@ -241,7 +318,7 @@ void ArcUsbHostPermissionManager::MaybeProcessNextPermissionRequest() {
   else
     current_requesting_guid_ = current_request.usb_device_entry()->guid;
 
-  auto app_id =
+  const std::string app_id =
       GetAppIdFromPackageName(current_requesting_package_, arc_app_list_prefs_);
   // App is uninstalled during the process.
   if (app_id.empty()) {
@@ -249,7 +326,7 @@ void ArcUsbHostPermissionManager::MaybeProcessNextPermissionRequest() {
     return;
   }
 
-  auto app_name = arc_app_list_prefs_->GetApp(app_id)->name;
+  const std::string app_name = arc_app_list_prefs_->GetApp(app_id)->name;
 
   if (current_request.is_scan_request()) {
     if (HasUsbScanDeviceListPermission(current_requesting_package_)) {
@@ -337,8 +414,14 @@ void ArcUsbHostPermissionManager::UpdateArcUsbScanDeviceListPermission(
   if (!allowed)
     return;
 
-  usb_scan_devicelist_permission_packages_.emplace(package_name);
-  // TODO(lgcheng): Update Chrome prefs based result of emplace.
+  const auto result =
+      usb_scan_devicelist_permission_packages_.emplace(package_name);
+  // Permission is already recored.
+  if (!result.second)
+    return;
+
+  arc_app_list_prefs_->SetPackagePrefs(
+      package_name, kUsbScanDeviceListPermission, base::Value(allowed));
 }
 
 void ArcUsbHostPermissionManager::UpdateArcUsbAccessPermission(
@@ -355,7 +438,40 @@ void ArcUsbHostPermissionManager::UpdateArcUsbAccessPermission(
 
   usb_access_permission_dict_.emplace(
       std::make_pair(package_name, usb_device_entry));
-  // TODO(lgcheng): Update Chrome prefs if the device entry is persistent.
+
+  if (!usb_device_entry.IsPersistent())
+    return;
+
+  base::DictionaryValue new_permission;
+  new_permission.SetKey(kUsbSerialNumber,
+                        base::Value(usb_device_entry.serial_number));
+  new_permission.SetKey(kUsbDeviceName,
+                        base::Value(usb_device_entry.device_name));
+  new_permission.SetKey(kUsbVendorId, base::Value(usb_device_entry.vendor_id));
+  new_permission.SetKey(kUsbProductId,
+                        base::Value(usb_device_entry.product_id));
+
+  base::Value* access_permission_list_value =
+      arc_app_list_prefs_->GetPackagePrefs(package_name, kUsbAccessPermission);
+  if (!access_permission_list_value ||
+      !access_permission_list_value->is_list()) {
+    if (access_permission_list_value &&
+        !access_permission_list_value->is_list()) {
+      LOG(WARNING) << "Wrong base::Value::Type found for device access "
+                      "permission list Chrome prefs. Will overwrite with "
+                      "base::Value::Type::List";
+    }
+    arc_app_list_prefs_->SetPackagePrefs(package_name, kUsbAccessPermission,
+                                         base::Value(base::Value::Type::LIST));
+  }
+  arc_app_list_prefs_->GetPackagePrefs(package_name, kUsbAccessPermission)
+      ->GetList()
+      .emplace_back(std::move(new_permission));
+}
+
+void ArcUsbHostPermissionManager::ClearPermissionForTesting() {
+  usb_scan_devicelist_permission_packages_.clear();
+  usb_access_permission_dict_.clear();
 }
 
 }  // namespace arc

chrome/browser/ui/app_list/arc/arc_usb_host_permission_manager.h

@@ -23,6 +23,7 @@ class Profile;
 namespace arc {
 
 class ArcUsbHostBridge;
+class ArcUsbHostPermissionTest;
 
 class ArcUsbHostPermissionManager : public ArcAppListPrefs::Observer,
                                     public ArcUsbHostUiDelegate,
@@ -129,8 +130,13 @@ class ArcUsbHostPermissionManager : public ArcAppListPrefs::Observer,
     return pending_requests_;
   }
 
+  // Clear |usb_scan_devicelist_permission_packages_| and
+  // |usb_access_permission_dict_| for testing. Will not affect Chrome prefs.
+  void ClearPermissionForTesting();
+
  private:
   friend class ArcUsbHostPermissionManagerFactory;
+  friend class ArcUsbHostPermissionTest;
 
   ArcUsbHostPermissionManager(Profile* profile,
                               ArcAppListPrefs* arc_app_list_prefs,
@@ -160,7 +166,7 @@ class ArcUsbHostPermissionManager : public ArcAppListPrefs::Observer,
 
   void UpdateArcUsbAccessPermission(const std::string& package_name,
                                     const UsbDeviceEntry& usb_device_entry,
-                                    bool accpet);
+                                    bool allowed);
 
   std::vector<UsbPermissionRequest> pending_requests_;
 

chrome/test/BUILD.gn

@@ -1634,6 +1634,7 @@ test("browser_tests") {
         "../browser/extensions/api/vpn_provider/vpn_provider_apitest.cc",
         "../browser/mash_service_registry_browsertest.cc",
         "../browser/signin/chromeos_mirror_account_consistency_browsertest.cc",
+        "../browser/ui/app_list/arc/arc_usb_host_permission_browsertest.cc",
         "../browser/ui/app_list/crostini/crostini_installer_view_browsertest.cc",
         "../browser/ui/ash/accelerator_commands_browsertest.cc",
         "../browser/ui/ash/app_list/app_list_browsertest.cc",