diff options
7 files changed, 104 insertions, 105 deletions
diff --git a/chrome/browser/ui/views/website_settings/website_settings_popup_view_unittest.cc b/chrome/browser/ui/views/website_settings/website_settings_popup_view_unittest.cc index e14fd92..716c001 100644 --- a/chrome/browser/ui/views/website_settings/website_settings_popup_view_unittest.cc +++ b/chrome/browser/ui/views/website_settings/website_settings_popup_view_unittest.cc @@ -16,6 +16,8 @@ #include "device/core/device_client.h" #include "device/usb/mock_usb_device.h" #include "device/usb/mock_usb_service.h" +#include "device/usb/mojo/type_converters.h" +#include "device/usb/public/interfaces/device.mojom.h" #include "testing/gtest/include/gtest/gtest.h" #include "ui/events/event_utils.h" #include "ui/views/controls/button/menu_button.h" @@ -244,5 +246,6 @@ TEST_F(WebsiteSettingsPopupViewTest, SetPermissionInfoWithUsbDevice) { button_listener->ButtonPressed(button, event); api_->SetPermissionInfo(list); EXPECT_EQ(0, api_->permissions_content()->child_count()); - EXPECT_FALSE(store->HasDevicePermission(origin, origin, device->guid())); + EXPECT_FALSE(store->HasDevicePermission( + origin, origin, *device::usb::DeviceInfo::From(*device))); } diff --git a/chrome/browser/ui/website_settings/website_settings_unittest.cc b/chrome/browser/ui/website_settings/website_settings_unittest.cc index 52c033d..09aa64f 100644 --- a/chrome/browser/ui/website_settings/website_settings_unittest.cc +++ b/chrome/browser/ui/website_settings/website_settings_unittest.cc @@ -28,6 +28,8 @@ #include "device/core/device_client.h" #include "device/usb/mock_usb_device.h" #include "device/usb/mock_usb_service.h" +#include "device/usb/mojo/type_converters.h" +#include "device/usb/public/interfaces/device.mojom.h" #include "grit/theme_resources.h" #include "net/cert/cert_status_flags.h" #include "net/cert/x509_certificate.h" @@ -343,7 +345,8 @@ TEST_F(WebsiteSettingsTest, OnChosenObjectDeleted) { last_chosen_object_info()[0].get(); website_settings()->OnSiteChosenObjectDeleted(info->ui_info, *info->object); - EXPECT_FALSE(store->HasDevicePermission(url(), url(), device->guid())); + EXPECT_FALSE(store->HasDevicePermission( + url(), url(), *device::usb::DeviceInfo::From(*device))); EXPECT_EQ(0u, last_chosen_object_info().size()); } diff --git a/chrome/browser/usb/usb_chooser_context.cc b/chrome/browser/usb/usb_chooser_context.cc index 4aeb569..2fb3bfc 100644 --- a/chrome/browser/usb/usb_chooser_context.cc +++ b/chrome/browser/usb/usb_chooser_context.cc @@ -13,6 +13,7 @@ #include "base/values.h" #include "chrome/browser/profiles/profile.h" #include "device/core/device_client.h" +#include "device/usb/public/interfaces/device.mojom.h" #include "device/usb/usb_device.h" using device::UsbDevice; @@ -47,28 +48,6 @@ bool CanStorePersistentEntry(const scoped_refptr<const UsbDevice>& device) { return !device->serial_number().empty(); } -const base::DictionaryValue* FindForDevice( - const std::vector<scoped_ptr<base::DictionaryValue>>& device_list, - const scoped_refptr<const UsbDevice>& device) { - const std::string utf8_serial_number = - base::UTF16ToUTF8(device->serial_number()); - - for (const scoped_ptr<base::DictionaryValue>& device_dict : device_list) { - int vendor_id; - int product_id; - std::string serial_number; - if (device_dict->GetInteger(kVendorIdKey, &vendor_id) && - device->vendor_id() == vendor_id && - device_dict->GetInteger(kProductIdKey, &product_id) && - device->product_id() == product_id && - device_dict->GetString(kSerialNumberKey, &serial_number) && - utf8_serial_number == serial_number) { - return device_dict.get(); - } - } - return nullptr; -} - } // namespace UsbChooserContext::UsbChooserContext(Profile* profile) @@ -134,7 +113,13 @@ void UsbChooserContext::RevokeObjectPermission( const base::DictionaryValue& object) { std::string guid; if (object.GetString(kGuidKey, &guid)) { - RevokeDevicePermission(requesting_origin, embedding_origin, guid); + auto it = ephemeral_devices_.find( + std::make_pair(requesting_origin, embedding_origin)); + if (it != ephemeral_devices_.end()) { + it->second.erase(guid); + if (it->second.empty()) + ephemeral_devices_.erase(it); + } RecordPermissionRevocation(WEBUSB_PERMISSION_REVOKED_EPHEMERAL); } else { ChooserContextBase::RevokeObjectPermission(requesting_origin, @@ -164,43 +149,34 @@ void UsbChooserContext::GrantDevicePermission(const GURL& requesting_origin, } } -void UsbChooserContext::RevokeDevicePermission(const GURL& requesting_origin, - const GURL& embedding_origin, - const std::string& guid) { +bool UsbChooserContext::HasDevicePermission( + const GURL& requesting_origin, + const GURL& embedding_origin, + const device::usb::DeviceInfo& device_info) { auto it = ephemeral_devices_.find( std::make_pair(requesting_origin, embedding_origin)); - if (it != ephemeral_devices_.end()) { - it->second.erase(guid); - if (it->second.empty()) - ephemeral_devices_.erase(it); + if (it != ephemeral_devices_.end() && + ContainsValue(it->second, device_info.guid)) { + return true; } - scoped_refptr<UsbDevice> device = usb_service_->GetDevice(guid); - if (!device) - return; - std::vector<scoped_ptr<base::DictionaryValue>> device_list = GetGrantedObjects(requesting_origin, embedding_origin); - const base::DictionaryValue* entry = FindForDevice(device_list, device); - if (entry) - RevokeObjectPermission(requesting_origin, embedding_origin, *entry); -} - -bool UsbChooserContext::HasDevicePermission(const GURL& requesting_origin, - const GURL& embedding_origin, - const std::string& guid) { - auto it = ephemeral_devices_.find( - std::make_pair(requesting_origin, embedding_origin)); - if (it != ephemeral_devices_.end()) - return ContainsValue(it->second, guid); - - scoped_refptr<UsbDevice> device = usb_service_->GetDevice(guid); - if (!device) - return false; + for (const scoped_ptr<base::DictionaryValue>& device_dict : device_list) { + int vendor_id; + int product_id; + std::string serial_number; + if (device_dict->GetInteger(kVendorIdKey, &vendor_id) && + device_info.vendor_id == vendor_id && + device_dict->GetInteger(kProductIdKey, &product_id) && + device_info.product_id == product_id && + device_dict->GetString(kSerialNumberKey, &serial_number) && + device_info.serial_number == serial_number) { + return true; + } + } - std::vector<scoped_ptr<base::DictionaryValue>> device_list = - GetGrantedObjects(requesting_origin, embedding_origin); - return FindForDevice(device_list, device) != nullptr; + return false; } bool UsbChooserContext::IsValidObject(const base::DictionaryValue& object) { diff --git a/chrome/browser/usb/usb_chooser_context.h b/chrome/browser/usb/usb_chooser_context.h index 9b26b8b..3fa604f 100644 --- a/chrome/browser/usb/usb_chooser_context.h +++ b/chrome/browser/usb/usb_chooser_context.h @@ -16,6 +16,12 @@ #include "chrome/browser/permissions/chooser_context_base.h" #include "device/usb/usb_service.h" +namespace device { +namespace usb { +class DeviceInfo; +} +} + class UsbChooserContext : public ChooserContextBase, public device::UsbService::Observer { public: @@ -39,17 +45,11 @@ class UsbChooserContext : public ChooserContextBase, const GURL& embedding_origin, const std::string& guid); - // Revokes |requesting_origin|'s access to the USB device known to - // device::UsbService as |guid|. - void RevokeDevicePermission(const GURL& requesting_origin, - const GURL& embedding_origin, - const std::string& guid); - // Checks if |requesting_origin| (when embedded within |embedding_origin| has - // access to the USB device known to device::UsbService as |guid|. + // access to a device with |device_info|. bool HasDevicePermission(const GURL& requesting_origin, const GURL& embedding_origin, - const std::string& guid); + const device::usb::DeviceInfo& device_info); private: // ChooserContextBase implementation. diff --git a/chrome/browser/usb/usb_chooser_context_unittest.cc b/chrome/browser/usb/usb_chooser_context_unittest.cc index cb5985e..f2ccc8d 100644 --- a/chrome/browser/usb/usb_chooser_context_unittest.cc +++ b/chrome/browser/usb/usb_chooser_context_unittest.cc @@ -11,8 +11,11 @@ #include "device/core/mock_device_client.h" #include "device/usb/mock_usb_device.h" #include "device/usb/mock_usb_service.h" +#include "device/usb/mojo/type_converters.h" +#include "device/usb/public/interfaces/device.mojom.h" using device::MockUsbDevice; +using device::UsbDevice; class UsbChooserContextTest : public testing::Test { public: @@ -31,8 +34,10 @@ class UsbChooserContextTest : public testing::Test { TEST_F(UsbChooserContextTest, CheckGrantAndRevokePermission) { GURL origin("https://www.google.com"); - scoped_refptr<MockUsbDevice> device = + scoped_refptr<UsbDevice> device = new MockUsbDevice(0, 0, "Google", "Gizmo", "123ABC"); + device::usb::DeviceInfoPtr device_info = + device::usb::DeviceInfo::From(*device); device_client_.usb_service()->AddDevice(device); UsbChooserContext* store = UsbChooserContextFactory::GetForProfile(profile()); @@ -42,23 +47,23 @@ TEST_F(UsbChooserContextTest, CheckGrantAndRevokePermission) { object_dict.SetInteger("product-id", 0); object_dict.SetString("serial-number", "123ABC"); - EXPECT_FALSE(store->HasDevicePermission(origin, origin, device->guid())); + EXPECT_FALSE(store->HasDevicePermission(origin, origin, *device_info)); store->GrantDevicePermission(origin, origin, device->guid()); - EXPECT_TRUE(store->HasDevicePermission(origin, origin, device->guid())); + EXPECT_TRUE(store->HasDevicePermission(origin, origin, *device_info)); std::vector<scoped_ptr<base::DictionaryValue>> objects = store->GetGrantedObjects(origin, origin); - EXPECT_EQ(1u, objects.size()); + ASSERT_EQ(1u, objects.size()); EXPECT_TRUE(object_dict.Equals(objects[0].get())); std::vector<scoped_ptr<ChooserContextBase::Object>> all_origin_objects = store->GetAllGrantedObjects(); - EXPECT_EQ(1u, all_origin_objects.size()); + ASSERT_EQ(1u, all_origin_objects.size()); EXPECT_EQ(origin, all_origin_objects[0]->requesting_origin); EXPECT_EQ(origin, all_origin_objects[0]->embedding_origin); EXPECT_TRUE(object_dict.Equals(&all_origin_objects[0]->object)); EXPECT_FALSE(all_origin_objects[0]->incognito); - store->RevokeDevicePermission(origin, origin, device->guid()); - EXPECT_FALSE(store->HasDevicePermission(origin, origin, device->guid())); + store->RevokeObjectPermission(origin, origin, *objects[0]); + EXPECT_FALSE(store->HasDevicePermission(origin, origin, *device_info)); objects = store->GetGrantedObjects(origin, origin); EXPECT_EQ(0u, objects.size()); all_origin_objects = store->GetAllGrantedObjects(); @@ -67,10 +72,14 @@ TEST_F(UsbChooserContextTest, CheckGrantAndRevokePermission) { TEST_F(UsbChooserContextTest, CheckGrantAndRevokeEphemeralPermission) { GURL origin("https://www.google.com"); - scoped_refptr<MockUsbDevice> device = + scoped_refptr<UsbDevice> device = new MockUsbDevice(0, 0, "Google", "Gizmo", ""); - scoped_refptr<MockUsbDevice> other_device = + device::usb::DeviceInfoPtr device_info = + device::usb::DeviceInfo::From(*device); + scoped_refptr<UsbDevice> other_device = new MockUsbDevice(0, 0, "Google", "Gizmo", ""); + device::usb::DeviceInfoPtr other_device_info = + device::usb::DeviceInfo::From(*other_device); device_client_.usb_service()->AddDevice(device); UsbChooserContext* store = UsbChooserContextFactory::GetForProfile(profile()); @@ -78,11 +87,10 @@ TEST_F(UsbChooserContextTest, CheckGrantAndRevokeEphemeralPermission) { object_dict.SetString("name", "Gizmo"); object_dict.SetString("ephemeral-guid", device->guid()); - EXPECT_FALSE(store->HasDevicePermission(origin, origin, device->guid())); + EXPECT_FALSE(store->HasDevicePermission(origin, origin, *device_info)); store->GrantDevicePermission(origin, origin, device->guid()); - EXPECT_TRUE(store->HasDevicePermission(origin, origin, device->guid())); - EXPECT_FALSE( - store->HasDevicePermission(origin, origin, other_device->guid())); + EXPECT_TRUE(store->HasDevicePermission(origin, origin, *device_info)); + EXPECT_FALSE(store->HasDevicePermission(origin, origin, *other_device_info)); std::vector<scoped_ptr<base::DictionaryValue>> objects = store->GetGrantedObjects(origin, origin); EXPECT_EQ(1u, objects.size()); @@ -95,8 +103,8 @@ TEST_F(UsbChooserContextTest, CheckGrantAndRevokeEphemeralPermission) { EXPECT_TRUE(object_dict.Equals(&all_origin_objects[0]->object)); EXPECT_FALSE(all_origin_objects[0]->incognito); - store->RevokeDevicePermission(origin, origin, device->guid()); - EXPECT_FALSE(store->HasDevicePermission(origin, origin, device->guid())); + store->RevokeObjectPermission(origin, origin, *objects[0]); + EXPECT_FALSE(store->HasDevicePermission(origin, origin, *device_info)); objects = store->GetGrantedObjects(origin, origin); EXPECT_EQ(0u, objects.size()); all_origin_objects = store->GetAllGrantedObjects(); @@ -105,14 +113,16 @@ TEST_F(UsbChooserContextTest, CheckGrantAndRevokeEphemeralPermission) { TEST_F(UsbChooserContextTest, DisconnectDeviceWithPermission) { GURL origin("https://www.google.com"); - scoped_refptr<MockUsbDevice> device = + scoped_refptr<UsbDevice> device = new MockUsbDevice(0, 0, "Google", "Gizmo", "123ABC"); + device::usb::DeviceInfoPtr device_info = + device::usb::DeviceInfo::From(*device); device_client_.usb_service()->AddDevice(device); UsbChooserContext* store = UsbChooserContextFactory::GetForProfile(profile()); - EXPECT_FALSE(store->HasDevicePermission(origin, origin, device->guid())); + EXPECT_FALSE(store->HasDevicePermission(origin, origin, *device_info)); store->GrantDevicePermission(origin, origin, device->guid()); - EXPECT_TRUE(store->HasDevicePermission(origin, origin, device->guid())); + EXPECT_TRUE(store->HasDevicePermission(origin, origin, *device_info)); std::vector<scoped_ptr<base::DictionaryValue>> objects = store->GetGrantedObjects(origin, origin); EXPECT_EQ(1u, objects.size()); @@ -121,17 +131,17 @@ TEST_F(UsbChooserContextTest, DisconnectDeviceWithPermission) { EXPECT_EQ(1u, all_origin_objects.size()); device_client_.usb_service()->RemoveDevice(device); - EXPECT_FALSE(store->HasDevicePermission(origin, origin, device->guid())); + EXPECT_TRUE(store->HasDevicePermission(origin, origin, *device_info)); objects = store->GetGrantedObjects(origin, origin); EXPECT_EQ(1u, objects.size()); all_origin_objects = store->GetAllGrantedObjects(); EXPECT_EQ(1u, all_origin_objects.size()); - scoped_refptr<MockUsbDevice> reconnected_device = + scoped_refptr<UsbDevice> reconnected_device = new MockUsbDevice(0, 0, "Google", "Gizmo", "123ABC"); device_client_.usb_service()->AddDevice(reconnected_device); - EXPECT_TRUE( - store->HasDevicePermission(origin, origin, reconnected_device->guid())); + device_info = device::usb::DeviceInfo::From(*reconnected_device); + EXPECT_TRUE(store->HasDevicePermission(origin, origin, *device_info)); objects = store->GetGrantedObjects(origin, origin); EXPECT_EQ(1u, objects.size()); all_origin_objects = store->GetAllGrantedObjects(); @@ -140,14 +150,16 @@ TEST_F(UsbChooserContextTest, DisconnectDeviceWithPermission) { TEST_F(UsbChooserContextTest, DisconnectDeviceWithEphemeralPermission) { GURL origin("https://www.google.com"); - scoped_refptr<MockUsbDevice> device = + scoped_refptr<UsbDevice> device = new MockUsbDevice(0, 0, "Google", "Gizmo", ""); + device::usb::DeviceInfoPtr device_info = + device::usb::DeviceInfo::From(*device); device_client_.usb_service()->AddDevice(device); UsbChooserContext* store = UsbChooserContextFactory::GetForProfile(profile()); - EXPECT_FALSE(store->HasDevicePermission(origin, origin, device->guid())); + EXPECT_FALSE(store->HasDevicePermission(origin, origin, *device_info)); store->GrantDevicePermission(origin, origin, device->guid()); - EXPECT_TRUE(store->HasDevicePermission(origin, origin, device->guid())); + EXPECT_TRUE(store->HasDevicePermission(origin, origin, *device_info)); std::vector<scoped_ptr<base::DictionaryValue>> objects = store->GetGrantedObjects(origin, origin); EXPECT_EQ(1u, objects.size()); @@ -156,17 +168,17 @@ TEST_F(UsbChooserContextTest, DisconnectDeviceWithEphemeralPermission) { EXPECT_EQ(1u, all_origin_objects.size()); device_client_.usb_service()->RemoveDevice(device); - EXPECT_FALSE(store->HasDevicePermission(origin, origin, device->guid())); + EXPECT_FALSE(store->HasDevicePermission(origin, origin, *device_info)); objects = store->GetGrantedObjects(origin, origin); EXPECT_EQ(0u, objects.size()); all_origin_objects = store->GetAllGrantedObjects(); EXPECT_EQ(0u, all_origin_objects.size()); - scoped_refptr<MockUsbDevice> reconnected_device = + scoped_refptr<UsbDevice> reconnected_device = new MockUsbDevice(0, 0, "Google", "Gizmo", ""); + device_info = device::usb::DeviceInfo::From(*reconnected_device); device_client_.usb_service()->AddDevice(reconnected_device); - EXPECT_FALSE( - store->HasDevicePermission(origin, origin, reconnected_device->guid())); + EXPECT_FALSE(store->HasDevicePermission(origin, origin, *device_info)); objects = store->GetGrantedObjects(origin, origin); EXPECT_EQ(0u, objects.size()); all_origin_objects = store->GetAllGrantedObjects(); @@ -179,25 +191,29 @@ TEST_F(UsbChooserContextTest, GrantPermissionInIncognito) { UsbChooserContext* incognito_store = UsbChooserContextFactory::GetForProfile( profile()->GetOffTheRecordProfile()); - scoped_refptr<MockUsbDevice> device1 = + scoped_refptr<UsbDevice> device1 = new MockUsbDevice(0, 0, "Google", "Gizmo", ""); - scoped_refptr<MockUsbDevice> device2 = + device::usb::DeviceInfoPtr device1_info = + device::usb::DeviceInfo::From(*device1); + scoped_refptr<UsbDevice> device2 = new MockUsbDevice(0, 0, "Google", "Gizmo", ""); + device::usb::DeviceInfoPtr device2_info = + device::usb::DeviceInfo::From(*device2); device_client_.usb_service()->AddDevice(device1); device_client_.usb_service()->AddDevice(device2); store->GrantDevicePermission(origin, origin, device1->guid()); - EXPECT_TRUE(store->HasDevicePermission(origin, origin, device1->guid())); + EXPECT_TRUE(store->HasDevicePermission(origin, origin, *device1_info)); EXPECT_FALSE( - incognito_store->HasDevicePermission(origin, origin, device1->guid())); + incognito_store->HasDevicePermission(origin, origin, *device1_info)); incognito_store->GrantDevicePermission(origin, origin, device2->guid()); - EXPECT_TRUE(store->HasDevicePermission(origin, origin, device1->guid())); - EXPECT_FALSE(store->HasDevicePermission(origin, origin, device2->guid())); + EXPECT_TRUE(store->HasDevicePermission(origin, origin, *device1_info)); + EXPECT_FALSE(store->HasDevicePermission(origin, origin, *device2_info)); EXPECT_FALSE( - incognito_store->HasDevicePermission(origin, origin, device1->guid())); + incognito_store->HasDevicePermission(origin, origin, *device1_info)); EXPECT_TRUE( - incognito_store->HasDevicePermission(origin, origin, device2->guid())); + incognito_store->HasDevicePermission(origin, origin, *device2_info)); { std::vector<scoped_ptr<base::DictionaryValue>> objects = diff --git a/chrome/browser/usb/web_usb_permission_provider.cc b/chrome/browser/usb/web_usb_permission_provider.cc index 01de4bf..a037432 100644 --- a/chrome/browser/usb/web_usb_permission_provider.cc +++ b/chrome/browser/usb/web_usb_permission_provider.cc @@ -86,12 +86,13 @@ void WebUSBPermissionProvider::HasDevicePermission( mojo::Array<mojo::String> allowed_guids; for (size_t i = 0; i < requested_devices.size(); ++i) { - const device::usb::DeviceInfoPtr& device = requested_devices[i]; - if (FindOriginInDescriptorSet(device->webusb_allowed_origins.get(), + const device::usb::DeviceInfo& device = *requested_devices[i]; + if (FindOriginInDescriptorSet(device.webusb_allowed_origins.get(), requesting_origin, nullptr, nullptr) && chooser_context->HasDevicePermission(requesting_origin, - embedding_origin, device->guid)) - allowed_guids.push_back(device->guid); + embedding_origin, device)) { + allowed_guids.push_back(device.guid); + } } callback.Run(std::move(allowed_guids)); } diff --git a/device/usb/mojo/type_converters.h b/device/usb/mojo/type_converters.h index 4984c6e..002bb79 100644 --- a/device/usb/mojo/type_converters.h +++ b/device/usb/mojo/type_converters.h @@ -75,7 +75,7 @@ template <> struct TypeConverter<device::usb::AlternateInterfaceInfoPtr, device::UsbInterfaceDescriptor> { static device::usb::AlternateInterfaceInfoPtr Convert( - const device::UsbInterfaceDescriptor& interface); + const device::UsbInterfaceDescriptor& iface); }; // Note that this is an explicit vector-to-array conversion, as |