Wire up toggle for (dis)allowing printer access from Plugin VM

This CL wires up the toggle in settings for controlling printer access
from Plugin VM to a new pref plugin_vm.printers_allowed. When this or the
PrintingEnabled policy is set to false, the CUPS proxy will return a 401
forbidden error to every request.

Bug: 950431
Change-Id: I14c395b53ea11bcb9c438266e36aa21e6fcd85b7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2120110
Commit-Queue: Timothy Loh <timloh@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Sean Kau <skau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756051}

chrome/browser/chromeos/plugin_vm/plugin_vm_pref_names.cc

@@ -21,6 +21,8 @@ const char kPluginVmImage[] = "plugin_vm.image";
 // A boolean preference representing whether there is a PluginVm image for
 // this user on this device.
 const char kPluginVmImageExists[] = "plugin_vm.image_exists";
+// A boolean preference indicating whether Plugin VM is allowed to use printers.
+const char kPluginVmPrintersAllowed[] = "plugin_vm.printers_allowed";
 // Preferences for storing engagement time data, as per
 // GuestOsEngagementMetrics.
 const char kEngagementPrefsPrefix[] = "plugin_vm.metrics";
@@ -28,6 +30,9 @@ const char kEngagementPrefsPrefix[] = "plugin_vm.metrics";
 void RegisterProfilePrefs(PrefRegistrySimple* registry) {
   registry->RegisterDictionaryPref(kPluginVmImage);
   registry->RegisterBooleanPref(kPluginVmImageExists, false);
+  // TODO(crbug.com/1066760): For convenience this currently defaults to true,
+  // but we'll need to revisit before launch.
+  registry->RegisterBooleanPref(kPluginVmPrintersAllowed, true);
 
   guest_os::prefs::RegisterEngagementProfilePrefs(registry,
                                                   kEngagementPrefsPrefix);

chrome/browser/chromeos/plugin_vm/plugin_vm_pref_names.h

@@ -12,6 +12,7 @@ namespace prefs {
 
 extern const char kPluginVmImage[];
 extern const char kPluginVmImageExists[];
+extern const char kPluginVmPrintersAllowed[];
 extern const char kEngagementPrefsPrefix[];
 
 void RegisterProfilePrefs(PrefRegistrySimple* registry);

chrome/browser/chromeos/plugin_vm/plugin_vm_util.cc

@@ -17,6 +17,7 @@
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
 #include "chrome/common/chrome_features.h"
 #include "chromeos/dbus/dlcservice/dlcservice_client.h"

chrome/browser/chromeos/printing/cups_proxy_service_delegate_impl.cc

@@ -7,10 +7,13 @@
 #include <utility>
 
 #include "base/task/post_task.h"
+#include "chrome/browser/chromeos/plugin_vm/plugin_vm_pref_names.h"
 #include "chrome/browser/chromeos/printing/cups_printers_manager.h"
 #include "chrome/browser/chromeos/printing/cups_printers_manager_factory.h"
 #include "chrome/browser/chromeos/printing/printer_configurer.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/common/pref_names.h"
+#include "components/prefs/pref_service.h"
 #include "content/public/browser/browser_task_traits.h"
 
 namespace chromeos {
@@ -25,6 +28,12 @@ CupsProxyServiceDelegateImpl::CupsProxyServiceDelegateImpl()
 
 CupsProxyServiceDelegateImpl::~CupsProxyServiceDelegateImpl() = default;
 
+bool CupsProxyServiceDelegateImpl::IsPrinterAccessAllowed() const {
+  const PrefService* prefs = profile_->GetPrefs();
+  return prefs->GetBoolean(prefs::kPrintingEnabled) &&
+         prefs->GetBoolean(plugin_vm::prefs::kPluginVmPrintersAllowed);
+}
+
 base::Optional<Printer> CupsProxyServiceDelegateImpl::GetPrinter(
     const std::string& id) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

chrome/browser/chromeos/printing/cups_proxy_service_delegate_impl.h

@@ -33,6 +33,8 @@ class CupsProxyServiceDelegateImpl
   CupsProxyServiceDelegateImpl();
   ~CupsProxyServiceDelegateImpl() override;
 
+  bool IsPrinterAccessAllowed() const override;
+
   // Look for a printer with the given id in any class.  Returns a copy of the
   // printer if found, nullptr otherwise.
   base::Optional<Printer> GetPrinter(const std::string& id) override;

chrome/browser/extensions/api/settings_private/prefs_util.cc

@@ -471,6 +471,8 @@ const PrefsUtil::TypedPrefMap& PrefsUtil::GetWhitelistedKeys() {
   // Plugin Vm
   (*s_whitelist)[plugin_vm::prefs::kPluginVmImageExists] =
       settings_api::PrefType::PREF_TYPE_BOOLEAN;
+  (*s_whitelist)[plugin_vm::prefs::kPluginVmPrintersAllowed] =
+      settings_api::PrefType::PREF_TYPE_BOOLEAN;
 
   // Android Apps.
   (*s_whitelist)[arc::prefs::kArcEnabled] =

chrome/browser/resources/settings/chromeos/plugin_vm_page/plugin_vm_subpage.html

@@ -19,8 +19,9 @@
           aria-roledescription="$i18n{subpageArrowRoleDescription}">
       </cr-icon-button>
     </div>
-    <!-- TODO(timloh): Wire this up to a pref. -->
-    <settings-toggle-button label="$i18n{pluginVmPrinterAccess}">
+    <settings-toggle-button id="plugin-vm-printer-access"
+         label="$i18n{pluginVmPrinterAccess}"
+         pref="{{prefs.plugin_vm.printers_allowed}}">
     </settings-toggle-button>
     <div id="plugin-vm-remove" class="settings-box">
       <div id="pluginVmRemoveLabel" class="start">$i18n{pluginVmRemove}</div>

chrome/services/cups_proxy/cups_proxy_service_delegate.h

@@ -31,6 +31,9 @@ class CupsProxyServiceDelegate {
   // CupsProxyService internal managers.
   base::WeakPtr<CupsProxyServiceDelegate> GetWeakPtr();
 
+  // Printer access can be disabled by either policy or settings.
+  virtual bool IsPrinterAccessAllowed() const = 0;
+
   virtual std::vector<chromeos::Printer> GetPrinters(
       chromeos::PrinterClass printer_class) = 0;
   virtual base::Optional<chromeos::Printer> GetPrinter(

chrome/services/cups_proxy/fake_cups_proxy_service_delegate.cc

@@ -6,6 +6,10 @@
 
 namespace cups_proxy {
 
+bool FakeCupsProxyServiceDelegate::IsPrinterAccessAllowed() const {
+  return true;
+}
+
 std::vector<chromeos::Printer> FakeCupsProxyServiceDelegate::GetPrinters(
     chromeos::PrinterClass printer_class) {
   return {};

chrome/services/cups_proxy/fake_cups_proxy_service_delegate.h

@@ -21,6 +21,7 @@ class FakeCupsProxyServiceDelegate : public CupsProxyServiceDelegate {
   ~FakeCupsProxyServiceDelegate() override = default;
 
   // CupsProxyServiceDelegate overrides.
+  bool IsPrinterAccessAllowed() const override;
   std::vector<chromeos::Printer> GetPrinters(
       chromeos::PrinterClass printer_class) override;
   base::Optional<chromeos::Printer> GetPrinter(const std::string& id) override;

chrome/services/cups_proxy/proxy_manager.cc

@@ -152,6 +152,13 @@ void ProxyManagerImpl::ProxyRequest(
     ProxyRequestCallback cb) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
+  if (!delegate_->IsPrinterAccessAllowed()) {
+    DVLOG(1) << "Printer access not allowed";
+    std::move(cb).Run(/*headers=*/{}, /*ipp_message=*/{},
+                      HTTP_STATUS_FORBIDDEN);
+    return;
+  }
+
   // If we already have an in-flight request, we fail this incoming one
   // directly.
   if (in_flight_) {
@@ -236,10 +243,11 @@ void ProxyManagerImpl::OnParseIpp(
 }
 
 void ProxyManagerImpl::SpoofGetPrinters() {
-  auto printers = FilterPrintersForPluginVm(
+  std::vector<chromeos::Printer> printers = FilterPrintersForPluginVm(
       delegate_->GetPrinters(chromeos::PrinterClass::kSaved),
       delegate_->GetPrinters(chromeos::PrinterClass::kEnterprise));
-  auto response = BuildGetDestsResponse(in_flight_->request, printers);
+  base::Optional<IppResponse> response =
+      BuildGetDestsResponse(in_flight_->request, printers);
   if (!response.has_value()) {
     return Fail("Failed to spoof CUPS-Get-Printers response",
                 HTTP_STATUS_SERVER_ERROR);

chrome/test/data/webui/settings/chromeos/plugin_vm_page_test.js

@@ -131,6 +131,12 @@ suite('Details', function() {
     settings.PluginVmBrowserProxyImpl.instance_ = pluginVmBrowserProxy;
     PolymerTest.clearBody();
     page = document.createElement('settings-plugin-vm-subpage');
+    page.prefs = {
+      plugin_vm: {
+        image_exists: {value: true},
+        printers_allowed: {value: false},
+      }
+    };
     document.body.appendChild(page);
   });
 
@@ -141,6 +147,17 @@ suite('Details', function() {
   test('Sanity', function() {
     assertTrue(!!page.$$('#plugin-vm-shared-paths'));
   });
+
+  test('PrintingToggle', async function() {
+    const toggle = page.$$('#plugin-vm-printer-access');
+    assertTrue(!!toggle);
+    assertTrue(!!toggle);
+    assertFalse(toggle.checked);
+    assertFalse(toggle.pref.value);
+    toggle.click();
+    assertTrue(toggle.checked);
+    assertTrue(toggle.pref.value);
+  });
 });
 
 suite('SharedPaths', function() {