Reland again: Stop supporting Plugin VM device license key

This has been superseded by ByteBot licensing, and has caused some
confusion by users who had previously added such a key, as the policy
is no longer available in the admin console (excl managedchrome).

This is a reland of https://crrev.com/c/2789091. Running multiple
tast tests for plugin vm in the same tast run was failing as the
app service was not being notified that Plugin VM was installed.

BUG=b:178895204

Change-Id: Ife647d7741ca5c66f70a225504693e8832542a40
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2967487
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Jason Lin <lxj@google.com>
Commit-Queue: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#894553}

chrome/browser/ash/dbus/plugin_vm_service_provider.cc

@@ -79,14 +79,15 @@ void PluginVmServiceProvider::GetLicenseData(
   std::unique_ptr<dbus::Response> response =
       dbus::Response::FromMethodCall(method_call);
   plugin_vm_service::GetLicenseDataResponse payload;
+
   if (plugin_vm::FakeLicenseKeyIsSet()) {
     payload.set_device_id(kFakeUUID);
+    payload.set_license_key(plugin_vm::GetFakeLicenseKey());
   } else {
     payload.set_device_id(g_browser_process->platform_part()
                               ->browser_policy_connector_chromeos()
                               ->GetDirectoryApiID());
   }
-  payload.set_license_key(plugin_vm::GetPluginVmLicenseKey());
   dbus::MessageWriter writer(response.get());
   writer.AppendProtoAsArrayOfBytes(payload);
   std::move(response_sender).Run(std::move(response));

chrome/browser/ash/plugin_vm/plugin_vm_features.cc

@@ -97,8 +97,7 @@ PolicyConfigured CheckPolicyConfigured(const Profile* profile) {
     return PolicyConfigured::kErrorNotAllowedByUserPolicy;
   }
 
-  if (GetPluginVmLicenseKey().empty() &&
-      GetPluginVmUserIdForProfile(profile).empty()) {
+  if (GetPluginVmUserIdForProfile(profile).empty()) {
     return PolicyConfigured::kErrorLicenseNotSetUp;
   }
 

chrome/browser/ash/plugin_vm/plugin_vm_installer.cc

@@ -331,9 +331,7 @@ PluginVmInstaller::~PluginVmInstaller() = default;
 void PluginVmInstaller::CheckLicense() {
   UpdateInstallingState(InstallingState::kCheckingLicense);
 
-  // If the server has provided a license key, responsibility of validating is
-  // passed to the Plugin VM application.
-  if (!GetPluginVmLicenseKey().empty()) {
+  if (skip_license_check_for_testing_) {
     OnLicenseChecked(true);
     return;
   }

chrome/browser/ash/plugin_vm/plugin_vm_installer.h

@@ -155,6 +155,7 @@ class PluginVmInstaller : public KeyedService,
   // Returns free disk space required to install Plugin VM in bytes.
   int64_t RequiredFreeDiskSpace();
 
+  void SkipLicenseCheckForTesting() { skip_license_check_for_testing_ = true; }
   void SetFreeDiskSpaceForTesting(int64_t bytes) {
     free_disk_space_for_testing_ = bytes;
   }
@@ -280,6 +281,7 @@ class PluginVmInstaller : public KeyedService,
   std::unique_ptr<PluginVmLicenseChecker> license_checker_;
   bool using_drive_download_service_ = false;
 
+  bool skip_license_check_for_testing_ = false;
   // -1 indicates not set
   int64_t free_disk_space_for_testing_ = -1;
   absl::optional<base::FilePath> downloaded_image_for_testing_;

chrome/browser/ash/plugin_vm/plugin_vm_installer_unittest.cc

@@ -173,6 +173,7 @@ class PluginVmInstallerTestBase : public testing::Test {
     installer_ = PluginVmInstallerFactory::GetForProfile(profile_.get());
     observer_ = std::make_unique<StrictMock<MockObserver>>();
     installer_->SetObserver(observer_.get());
+    installer_->SkipLicenseCheckForTesting();
     installer_->SetFreeDiskSpaceForTesting(std::numeric_limits<int64_t>::max());
     installer_->SetDownloadedImageForTesting(CreateZipFile());
 

chrome/browser/ash/plugin_vm/plugin_vm_test_helper.cc

@@ -28,7 +28,6 @@ namespace plugin_vm {
 namespace {
 
 const char kDiskImageImportCommandUuid[] = "3922722bd7394acf85bf4d5a330d4a47";
-const char kPluginVmLicenseKey[] = "LICENSE_KEY";
 const char kDomain[] = "example.com";
 const char kDeviceId[] = "device_id";
 
@@ -138,10 +137,10 @@ PluginVmTestHelper::~PluginVmTestHelper() = default;
 void PluginVmTestHelper::SetPolicyRequirementsToAllowPluginVm() {
   testing_profile_->GetPrefs()->SetBoolean(plugin_vm::prefs::kPluginVmAllowed,
                                            true);
+  testing_profile_->GetPrefs()->SetString(plugin_vm::prefs::kPluginVmUserId,
+                                          "fake-id");
   testing_profile_->ScopedCrosSettingsTestHelper()->SetBoolean(
       chromeos::kPluginVmAllowed, true);
-  testing_profile_->ScopedCrosSettingsTestHelper()->SetString(
-      chromeos::kPluginVmLicenseKey, kPluginVmLicenseKey);
 }
 
 void PluginVmTestHelper::SetUserRequirementsToAllowPluginVm() {

chrome/browser/ash/plugin_vm/plugin_vm_util.cc

@@ -16,6 +16,7 @@
 #include "base/task/thread_pool.h"
 #include "chrome/browser/ash/plugin_vm/plugin_vm_drive_image_download_service.h"
 #include "chrome/browser/ash/plugin_vm/plugin_vm_features.h"
+#include "chrome/browser/ash/plugin_vm/plugin_vm_installer_factory.h"
 #include "chrome/browser/ash/plugin_vm/plugin_vm_manager.h"
 #include "chrome/browser/ash/plugin_vm/plugin_vm_manager_factory.h"
 #include "chrome/browser/ash/plugin_vm/plugin_vm_pref_names.h"
@@ -38,21 +39,16 @@ const char kChromeOSBaseDirectoryDisplayText[] = "Network \u203a ChromeOS";
 
 namespace {
 
-std::string& GetFakeLicenseKey() {
+static std::string& MutableFakeLicenseKey() {
   static base::NoDestructor<std::string> license_key;
   return *license_key;
 }
 
-base::RepeatingClosureList& GetFakeLicenceKeyListeners() {
+base::RepeatingClosureList& GetFakeLicenseKeyListeners() {
   static base::NoDestructor<base::RepeatingClosureList> instance;
   return *instance;
 }
 
-std::string& GetFakeUserId() {
-  static base::NoDestructor<std::string> user_id;
-  return *user_id;
-}
-
 }  // namespace
 
 bool IsPluginVmRunning(Profile* profile) {
@@ -70,17 +66,6 @@ bool IsPluginVmAppWindow(const aura::Window* window) {
   return *app_id == "org.chromium.plugin_vm_ui";
 }
 
-std::string GetPluginVmLicenseKey() {
-  if (FakeLicenseKeyIsSet())
-    return GetFakeLicenseKey();
-  std::string plugin_vm_license_key;
-  if (!ash::CrosSettings::Get()->GetString(chromeos::kPluginVmLicenseKey,
-                                           &plugin_vm_license_key)) {
-    return std::string();
-  }
-  return plugin_vm_license_key;
-}
-
 std::string GetPluginVmUserIdForProfile(const Profile* profile) {
   DCHECK(profile);
   return profile->GetPrefs()->GetString(plugin_vm::prefs::kPluginVmUserId);
@@ -95,19 +80,18 @@ void SetFakePluginVmPolicy(Profile* profile,
   base::DictionaryValue* dict = update.Get();
   dict->SetPath(prefs::kPluginVmImageUrlKeyName, base::Value(image_url));
   dict->SetPath(prefs::kPluginVmImageHashKeyName, base::Value(image_hash));
-
-  GetFakeLicenseKey() = license_key;
-
-  GetFakeLicenceKeyListeners().Notify();
-  GetFakeUserId() = "FAKE_USER_ID";
+  plugin_vm::PluginVmInstallerFactory::GetForProfile(profile)
+      ->SkipLicenseCheckForTesting();  // IN-TEST
+  MutableFakeLicenseKey() = license_key;
+  GetFakeLicenseKeyListeners().Notify();
 }
 
-bool FakeLicenseKeyIsSet() {
-  return !GetFakeLicenseKey().empty();
+std::string GetFakeLicenseKey() {
+  return MutableFakeLicenseKey();
 }
 
-bool FakeUserIdIsSet() {
-  return !GetFakeUserId().empty();
+bool FakeLicenseKeyIsSet() {
+  return !MutableFakeLicenseKey().empty();
 }
 
 void RemoveDriveDownloadDirectoryIfExists() {
@@ -175,11 +159,7 @@ PluginVmPolicySubscription::PluginVmPolicySubscription(
       chromeos::kPluginVmAllowed,
       base::BindRepeating(&PluginVmPolicySubscription::OnPolicyChanged,
                           base::Unretained(this)));
-  license_subscription_ = cros_settings->AddSettingsObserver(
-      chromeos::kPluginVmLicenseKey,
-      base::BindRepeating(&PluginVmPolicySubscription::OnPolicyChanged,
-                          base::Unretained(this)));
-  fake_license_subscription_ = GetFakeLicenceKeyListeners().Add(
+  fake_license_subscription_ = GetFakeLicenseKeyListeners().Add(
       base::BindRepeating(&PluginVmPolicySubscription::OnPolicyChanged,
                           base::Unretained(this)));
 

chrome/browser/ash/plugin_vm/plugin_vm_util.h

@@ -68,24 +68,23 @@ void ShowPluginVmInstallerView(Profile* profile);
 // the Plugin VM installer.
 bool IsPluginVmAppWindow(const aura::Window* window);
 
-// Retrieves the license key to be used for Plugin VM. If
-// none is set this will return an empty string.
-std::string GetPluginVmLicenseKey();
-
 // Retrieves the User Id to be used for Plugin VM. If none is set this will
 // return an empty string.
 std::string GetPluginVmUserIdForProfile(const Profile* profile);
 
-// Sets fake policy values and enables Plugin VM for testing. These set global
-// state so this should be called with empty strings on tear down.
-// TODO(crbug.com/1025136): Remove this once Tast supports setting test
-// policies.
+// Sets fake policy values and enables Plugin VM for tast tests. Device license
+// keys are no longer supported in policy, but for testing purposes this key
+// will still be used by the PluginVmService. This function also makes the
+// installer skip its license check.
+// This sets global state, not per-profile state.
+// TODO(crbug.com/1025136): Set policy directly from tast instead of using a
+// test helper function.
 void SetFakePluginVmPolicy(Profile* profile,
                            const std::string& image_path,
                            const std::string& image_hash,
                            const std::string& license_key);
 bool FakeLicenseKeyIsSet();
-bool FakeUserIdIsSet();
+std::string GetFakeLicenseKey();
 
 // Used to clean up the Plugin VM Drive download directory if it did not get
 // removed when it should have, perhaps due to a crash.
@@ -120,7 +119,6 @@ class PluginVmPolicySubscription {
 
   std::unique_ptr<PrefChangeRegistrar> pref_change_registrar_;
   base::CallbackListSubscription device_allowed_subscription_;
-  base::CallbackListSubscription license_subscription_;
   base::CallbackListSubscription fake_license_subscription_;
 };
 

chrome/browser/ash/plugin_vm/plugin_vm_util_unittest.cc

@@ -84,16 +84,6 @@ TEST_F(PluginVmUtilTest, PluginVmShouldBeConfiguredOnceAllConditionsAreMet) {
   EXPECT_TRUE(PluginVmFeatures::Get()->IsConfigured(testing_profile_.get()));
 }
 
-TEST_F(PluginVmUtilTest, GetPluginVmLicenseKey) {
-  // If no license key is set, the method should return the empty string.
-  EXPECT_EQ(std::string(), GetPluginVmLicenseKey());
-
-  const std::string kLicenseKey = "LICENSE_KEY";
-  testing_profile_->ScopedCrosSettingsTestHelper()->SetString(
-      chromeos::kPluginVmLicenseKey, kLicenseKey);
-  EXPECT_EQ(kLicenseKey, GetPluginVmLicenseKey());
-}
-
 TEST_F(PluginVmUtilTest, AddPluginVmPolicyObserver) {
   const std::unique_ptr<PluginVmPolicySubscription> subscription =
       std::make_unique<plugin_vm::PluginVmPolicySubscription>(
@@ -107,17 +97,6 @@ TEST_F(PluginVmUtilTest, AddPluginVmPolicyObserver) {
   test_helper_->AllowPluginVm();
   testing::Mock::VerifyAndClearExpectations(this);
 
-  EXPECT_CALL(*this, OnPolicyChanged(false));
-  testing_profile_->ScopedCrosSettingsTestHelper()->SetString(
-      chromeos::kPluginVmLicenseKey, "");
-  testing::Mock::VerifyAndClearExpectations(this);
-
-  EXPECT_CALL(*this, OnPolicyChanged(true));
-  const std::string kLicenseKey = "LICENSE_KEY";
-  testing_profile_->ScopedCrosSettingsTestHelper()->SetString(
-      chromeos::kPluginVmLicenseKey, kLicenseKey);
-  testing::Mock::VerifyAndClearExpectations(this);
-
   EXPECT_CALL(*this, OnPolicyChanged(false));
   testing_profile_->ScopedCrosSettingsTestHelper()->SetBoolean(
       chromeos::kPluginVmAllowed, false);
@@ -131,6 +110,23 @@ TEST_F(PluginVmUtilTest, AddPluginVmPolicyObserver) {
   EXPECT_CALL(*this, OnPolicyChanged(false));
   testing_profile_->GetPrefs()->SetBoolean(plugin_vm::prefs::kPluginVmAllowed,
                                            false);
+  testing::Mock::VerifyAndClearExpectations(this);
+
+  EXPECT_CALL(*this, OnPolicyChanged(true));
+  testing_profile_->GetPrefs()->SetBoolean(plugin_vm::prefs::kPluginVmAllowed,
+                                           true);
+  testing::Mock::VerifyAndClearExpectations(this);
+
+  EXPECT_CALL(*this, OnPolicyChanged(false));
+  testing_profile_->GetPrefs()->SetString(plugin_vm::prefs::kPluginVmUserId,
+                                          "");
+  testing::Mock::VerifyAndClearExpectations(this);
+
+  EXPECT_CALL(*this, OnPolicyChanged(true));
+  const std::string kPluginVmUserId = "fancy-user-id";
+  testing_profile_->GetPrefs()->SetString(plugin_vm::prefs::kPluginVmUserId,
+                                          kPluginVmUserId);
+  testing::Mock::VerifyAndClearExpectations(this);
 }
 
 TEST_F(PluginVmUtilTest, DriveUrlNonMatches) {

chrome/browser/ash/settings/device_settings_provider.cc

@@ -112,7 +112,6 @@ const char* const kKnownSettings[] = {
     kLoginAuthenticationBehavior,
     kLoginVideoCaptureAllowedUrls,
     kPluginVmAllowed,
-    kPluginVmLicenseKey,
     kPolicyMissingMitigationMode,
     kRebootOnShutdown,
     kReleaseChannel,
@@ -905,15 +904,6 @@ void DecodeGenericPolicies(const em::ChromeDeviceSettingsProto& policy,
     }
   }
 
-  if (policy.has_plugin_vm_license_key()) {
-    const em::PluginVmLicenseKeyProto& container(
-        policy.plugin_vm_license_key());
-    if (container.has_plugin_vm_license_key()) {
-      new_values_cache->SetValue(
-          kPluginVmLicenseKey, base::Value(container.plugin_vm_license_key()));
-    }
-  }
-
   // Default value of the policy in case it's missing.
   int access_mode = em::DevicePrintersAccessModeProto::ACCESS_MODE_ALL;
   // Use DevicePrintersAccessMode policy if present, otherwise Native version.

chrome/browser/ash/settings/device_settings_provider_unittest.cc

@@ -281,13 +281,6 @@ class DeviceSettingsProviderTest : public DeviceSettingsTestBase {
     BuildAndInstallDevicePolicy();
   }
 
-  void SetPluginVmLicenseKeySetting(const std::string& plugin_vm_license_key) {
-    em::PluginVmLicenseKeyProto* proto =
-        device_policy_->payload().mutable_plugin_vm_license_key();
-    proto->set_plugin_vm_license_key(plugin_vm_license_key);
-    BuildAndInstallDevicePolicy();
-  }
-
   void SetDeviceRebootOnUserSignout(
       em::DeviceRebootOnUserSignoutProto::RebootOnSignoutMode value) {
     EXPECT_CALL(*this, SettingChanged(_)).Times(AtLeast(1));
@@ -936,11 +929,6 @@ TEST_F(DeviceSettingsProviderTest, DecodePluginVmAllowedSetting) {
   EXPECT_EQ(base::Value(false), *provider_->Get(kPluginVmAllowed));
 }
 
-TEST_F(DeviceSettingsProviderTest, DecodePluginVmLicenseKeySetting) {
-  SetPluginVmLicenseKeySetting("LICENSE_KEY");
-  EXPECT_EQ(base::Value("LICENSE_KEY"), *provider_->Get(kPluginVmLicenseKey));
-}
-
 TEST_F(DeviceSettingsProviderTest, DeviceRebootAfterUserSignout) {
   using PolicyProto = em::DeviceRebootOnUserSignoutProto;
 

chrome/browser/chromeos/policy/core/device_policy_decoder_chromeos.cc

@@ -1733,16 +1733,6 @@ void DecodeGenericPolicies(const em::ChromeDeviceSettingsProto& policy,
     }
   }
 
-  if (policy.has_plugin_vm_license_key()) {
-    const em::PluginVmLicenseKeyProto& container(
-        policy.plugin_vm_license_key());
-    if (container.has_plugin_vm_license_key()) {
-      policies->Set(key::kPluginVmLicenseKey, POLICY_LEVEL_MANDATORY,
-                    POLICY_SCOPE_MACHINE, POLICY_SOURCE_CLOUD,
-                    base::Value(container.plugin_vm_license_key()), nullptr);
-    }
-  }
-
   if (policy.has_device_wilco_dtc_allowed() &&
       policy.device_wilco_dtc_allowed().has_device_wilco_dtc_allowed()) {
     VLOG(2) << "Set Wilco DTC allowed to "

chrome/browser/ui/views/plugin_vm/plugin_vm_installer_view_browsertest.cc

@@ -101,6 +101,7 @@ class PluginVmInstallerViewBrowserTest : public DialogBrowserTest {
     auto* installer = plugin_vm::PluginVmInstallerFactory::GetForProfile(
         browser()->profile());
     installer->SetFreeDiskSpaceForTesting(installer->RequiredFreeDiskSpace());
+    installer->SkipLicenseCheckForTesting();
   }
 
   void SetPluginVmImagePref(std::string url, std::string hash) {
@@ -164,8 +165,6 @@ class PluginVmInstallerViewBrowserTest : public DialogBrowserTest {
     // Device policies.
     scoped_testing_cros_settings_.device_settings()->Set(
         chromeos::kPluginVmAllowed, base::Value(true));
-    scoped_testing_cros_settings_.device_settings()->Set(
-        chromeos::kPluginVmLicenseKey, base::Value("LICENSE_KEY"));
   }
 
   void SetUserWithAffiliation() {

chrome/test/data/policy/policy_test_cases.json

@@ -6613,7 +6613,7 @@
   },
 
   "PluginVmLicenseKey": {
-    "reason_for_missing_test": "Maps into CrosSettings"
+    "note": "This policy has been removed since Chrome 94."
   },
 
   "ParentAccessCodeConfig": {

chromeos/settings/cros_settings_names.cc

@@ -438,8 +438,6 @@ const char kDeviceUnaffiliatedCrostiniAllowed[] =
 // A boolean pref that indicates whether PluginVm is allowed to run on this
 // device.
 const char kPluginVmAllowed[] = "cros.device.plugin_vm_allowed";
-// A string pref that specifies PluginVm license key for this device.
-const char kPluginVmLicenseKey[] = "cros.device.plugin_vm_license_key";
 
 // A boolean pref that indicates whether Borealis is allowed to run on this
 // device.

chromeos/settings/cros_settings_names.h

@@ -241,7 +241,6 @@ COMPONENT_EXPORT(CHROMEOS_SETTINGS)
 extern const char kDeviceUnaffiliatedCrostiniAllowed[];
 
 COMPONENT_EXPORT(CHROMEOS_SETTINGS) extern const char kPluginVmAllowed[];
-COMPONENT_EXPORT(CHROMEOS_SETTINGS) extern const char kPluginVmLicenseKey[];
 
 COMPONENT_EXPORT(CHROMEOS_SETTINGS)
 extern const char kBorealisAllowedForDevice[];
@@ -373,7 +372,6 @@ using ::chromeos::kHeartbeatFrequency;
 using ::chromeos::kLoginAuthenticationBehavior;
 using ::chromeos::kLoginVideoCaptureAllowedUrls;
 using ::chromeos::kPluginVmAllowed;
-using ::chromeos::kPluginVmLicenseKey;
 using ::chromeos::kPolicyMissingMitigationMode;
 using ::chromeos::kRebootOnShutdown;
 using ::chromeos::kReleaseChannel;