Separate PermissionsForExtension from KeyPermissionsService

This CL removes PermissionsForExtension class from
KeyPermissionsService* and converts it into a service
(ExtensionKeyPermissionsService). With this CL, only service operations
in the context of a profile should be handled by KeyPermissionsService
and only service operations in the context of a (profile, extension) pair
should be handled by ExtensionKeyPermissionsService.

Bug: 1127284
Change-Id: I54acf3af8d234129c070a3342b9e90e447c6982c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2429064
Commit-Queue: Omar Morsi <omorsi@google.com>
Reviewed-by: Edman Anjos <edman@chromium.org>
Reviewed-by: Pavol Marko <pmarko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811217}

chrome/browser/chromeos/BUILD.gn

@@ -1938,6 +1938,10 @@ source_set("chromeos") {
     "platform_keys/extension_platform_keys_service.h",
     "platform_keys/extension_platform_keys_service_factory.cc",
     "platform_keys/extension_platform_keys_service_factory.h",
+    "platform_keys/key_permissions/extension_key_permissions_service.cc",
+    "platform_keys/key_permissions/extension_key_permissions_service.h",
+    "platform_keys/key_permissions/extension_key_permissions_service_factory.cc",
+    "platform_keys/key_permissions/extension_key_permissions_service_factory.h",
     "platform_keys/key_permissions/key_permissions_policy_handler.cc",
     "platform_keys/key_permissions/key_permissions_policy_handler.h",
     "platform_keys/key_permissions/key_permissions_service.cc",

chrome/browser/chromeos/arc/enterprise/cert_store/arc_cert_store_bridge.cc

@@ -17,6 +17,7 @@
 #include "base/containers/queue.h"
 #include "base/logging.h"
 #include "base/memory/singleton.h"
+#include "chrome/browser/chromeos/platform_keys/key_permissions/extension_key_permissions_service.h"
 #include "chrome/browser/chromeos/platform_keys/key_permissions/key_permissions_service_factory.h"
 #include "chrome/browser/chromeos/platform_keys/key_permissions/key_permissions_service_impl.h"
 #include "chrome/browser/chromeos/platform_keys/platform_keys.h"
@@ -401,7 +402,7 @@ void ArcCertStoreBridge::UpdateFromKeyPermissionsPolicy() {
   DVLOG(1) << "ArcCertStoreBridge::UpdateFromKeyPermissionsPolicy";
 
   std::vector<std::string> app_ids =
-      chromeos::platform_keys::KeyPermissionsServiceImpl::
+      chromeos::platform_keys::ExtensionKeyPermissionsService::
           GetCorporateKeyUsageAllowedAppIds(policy_service_);
   std::vector<std::string> permissions;
   for (const auto& app_id : app_ids) {

chrome/browser/chromeos/arc/enterprise/cert_store/arc_cert_store_bridge_browsertest.cc

@@ -15,7 +15,8 @@
 #include "chrome/browser/chromeos/arc/enterprise/cert_store/arc_cert_store_bridge.h"
 #include "chrome/browser/chromeos/arc/session/arc_service_launcher.h"
 #include "chrome/browser/chromeos/login/test/local_policy_test_server_mixin.h"
-#include "chrome/browser/chromeos/platform_keys/key_permissions/key_permissions_service.h"
+#include "chrome/browser/chromeos/platform_keys/key_permissions/extension_key_permissions_service.h"
+#include "chrome/browser/chromeos/platform_keys/key_permissions/extension_key_permissions_service_factory.h"
 #include "chrome/browser/chromeos/platform_keys/key_permissions/key_permissions_service_factory.h"
 #include "chrome/browser/chromeos/platform_keys/platform_keys.h"
 #include "chrome/browser/chromeos/policy/user_policy_test_helper.h"
@@ -213,10 +214,12 @@ class ArcCertStoreBridgeTest : public MixinBasedInProcessBrowserTest {
 
     {
       base::RunLoop run_loop;
-      key_permissions_service->GetPermissionsForExtension(
-          kFakeExtensionId,
-          base::BindOnce(&ArcCertStoreBridgeTest::GotPermissionsForExtension,
-                         base::Unretained(this), run_loop.QuitClosure()));
+      chromeos::platform_keys::ExtensionKeyPermissionsServiceFactory::
+          GetForBrowserContextAndExtension(
+              base::BindOnce(
+                  &ArcCertStoreBridgeTest::GotPermissionsForExtension,
+                  base::Unretained(this), run_loop.QuitClosure()),
+              browser()->profile(), kFakeExtensionId, key_permissions_service);
       run_loop.Run();
     }
   }
@@ -255,31 +258,32 @@ class ArcCertStoreBridgeTest : public MixinBasedInProcessBrowserTest {
   net::ScopedCERTCertificate client_cert2_;
 
  private:
+  void OnKeyRegisteredForCorporateUsage(
+      std::unique_ptr<chromeos::platform_keys::ExtensionKeyPermissionsService>
+          extension_key_permissions_service,
+      const base::Closure& done_callback,
+      chromeos::platform_keys::Status status) {
+    ASSERT_EQ(status, chromeos::platform_keys::Status::kSuccess);
+    done_callback.Run();
+  }
+
   // Register only client_cert1_ for corporate usage to test that
   // client_cert2_ is not allowed.
   void GotPermissionsForExtension(
       const base::Closure& done_callback,
-      std::unique_ptr<chromeos::platform_keys::KeyPermissionsService::
-                          PermissionsForExtension> permissions_for_ext) {
-    auto* permissions_for_ext_unowned = permissions_for_ext.get();
+      std::unique_ptr<chromeos::platform_keys::ExtensionKeyPermissionsService>
+          extension_key_permissions_service) {
+    auto* extension_key_permissions_service_unowned =
+        extension_key_permissions_service.get();
     std::string client_cert1_spki(
         client_cert1_->derPublicKey.data,
         client_cert1_->derPublicKey.data + client_cert1_->derPublicKey.len);
-    permissions_for_ext_unowned->RegisterKeyForCorporateUsage(
+    extension_key_permissions_service_unowned->RegisterKeyForCorporateUsage(
         client_cert1_spki,
         base::BindOnce(
             &ArcCertStoreBridgeTest::OnKeyRegisteredForCorporateUsage,
-            base::Unretained(this), std::move(permissions_for_ext),
-            done_callback));
-  }
-
-  void OnKeyRegisteredForCorporateUsage(
-      std::unique_ptr<chromeos::platform_keys::KeyPermissionsService::
-                          PermissionsForExtension> permissions_for_ext,
-      const base::Closure& done_callback,
-      chromeos::platform_keys::Status status) {
-    ASSERT_EQ(status, chromeos::platform_keys::Status::kSuccess);
-    done_callback.Run();
+            base::Unretained(this),
+            std::move(extension_key_permissions_service), done_callback));
   }
 
   void SetUpTestClientCerts(const base::Closure& done_callback,

chrome/browser/chromeos/platform_keys/extension_platform_keys_service.cc

@@ -16,6 +16,8 @@
 #include "base/optional.h"
 #include "base/stl_util.h"
 #include "base/values.h"
+#include "chrome/browser/chromeos/platform_keys/key_permissions/extension_key_permissions_service.h"
+#include "chrome/browser/chromeos/platform_keys/key_permissions/extension_key_permissions_service_factory.h"
 #include "chrome/browser/chromeos/platform_keys/key_permissions/key_permissions_service.h"
 #include "chrome/browser/chromeos/platform_keys/key_permissions/key_permissions_service_factory.h"
 #include "chrome/browser/chromeos/platform_keys/platform_keys.h"
@@ -103,8 +105,8 @@ class ExtensionPlatformKeysService::GenerateKeyTask : public Task {
   std::string public_key_spki_der_;
   const std::string extension_id_;
   GenerateKeyCallback callback_;
-  std::unique_ptr<platform_keys::KeyPermissionsService::PermissionsForExtension>
-      extension_permissions_;
+  std::unique_ptr<platform_keys::ExtensionKeyPermissionsService>
+      extension_key_permissions_service_;
   ExtensionPlatformKeysService* const service_;
 
  private:
@@ -146,9 +148,12 @@ class ExtensionPlatformKeysService::GenerateKeyTask : public Task {
 
   // Gets the permissions for the extension with id |extension_id|.
   void GetExtensionPermissions() {
-    service_->key_permissions_service_->GetPermissionsForExtension(
-        extension_id_, base::BindOnce(&GenerateKeyTask::GotPermissions,
-                                      base::Unretained(this)));
+    platform_keys::ExtensionKeyPermissionsServiceFactory::
+        GetForBrowserContextAndExtension(
+            base::BindOnce(&GenerateKeyTask::GotPermissions,
+                           base::Unretained(this)),
+            service_->browser_context_, extension_id_,
+            service_->key_permissions_service_);
   }
 
   void OnKeyRegisteredForCorporateUsage(platform_keys::Status status) {
@@ -168,17 +173,17 @@ class ExtensionPlatformKeysService::GenerateKeyTask : public Task {
   }
 
   void UpdatePermissionsAndCallBack() {
-    extension_permissions_->RegisterKeyForCorporateUsage(
+    extension_key_permissions_service_->RegisterKeyForCorporateUsage(
         public_key_spki_der_,
         base::BindOnce(&GenerateKeyTask::OnKeyRegisteredForCorporateUsage,
                        base::Unretained(this)));
   }
 
   void GotPermissions(
-      std::unique_ptr<
-          platform_keys::KeyPermissionsService::PermissionsForExtension>
-          extension_permissions) {
-    extension_permissions_ = std::move(extension_permissions);
+      std::unique_ptr<platform_keys::ExtensionKeyPermissionsService>
+          extension_key_permissions_service) {
+    extension_key_permissions_service_ =
+        std::move(extension_key_permissions_service);
     DoStep();
   }
 
@@ -312,16 +317,18 @@ class ExtensionPlatformKeysService::SignTask : public Task {
   }
 
   void GetExtensionPermissions() {
-    service_->key_permissions_service_->GetPermissionsForExtension(
-        extension_id_,
-        base::BindOnce(&SignTask::GotPermissions, base::Unretained(this)));
+    platform_keys::ExtensionKeyPermissionsServiceFactory::
+        GetForBrowserContextAndExtension(
+            base::BindOnce(&SignTask::GotPermissions, base::Unretained(this)),
+            service_->browser_context_, extension_id_,
+            service_->key_permissions_service_);
   }
 
   void GotPermissions(
-      std::unique_ptr<
-          platform_keys::KeyPermissionsService::PermissionsForExtension>
-          extension_permissions) {
-    extension_permissions_ = std::move(extension_permissions);
+      std::unique_ptr<platform_keys::ExtensionKeyPermissionsService>
+          extension_key_permissions_service) {
+    extension_key_permissions_service_ =
+        std::move(extension_key_permissions_service);
     DoStep();
   }
 
@@ -335,7 +342,7 @@ class ExtensionPlatformKeysService::SignTask : public Task {
       return;
     }
 
-    extension_permissions_->CanUseKeyForSigning(
+    extension_key_permissions_service_->CanUseKeyForSigning(
         public_key_spki_der_,
         base::BindOnce(&SignTask::OnCanUseKeyForSigningKnown,
                        base::Unretained(this)));
@@ -354,7 +361,7 @@ class ExtensionPlatformKeysService::SignTask : public Task {
 
   // Updates the signing permissions for |public_key_spki_der_|.
   void UpdateSignPermissions() {
-    extension_permissions_->SetKeyUsedForSigning(
+    extension_key_permissions_service_->SetKeyUsedForSigning(
         public_key_spki_der_,
         base::BindOnce(&SignTask::OnSetKeyUsedForSigningDone,
                        base::Unretained(this)));
@@ -418,8 +425,8 @@ class ExtensionPlatformKeysService::SignTask : public Task {
   const platform_keys::HashAlgorithm hash_algorithm_;
   const std::string extension_id_;
   const SignCallback callback_;
-  std::unique_ptr<platform_keys::KeyPermissionsService::PermissionsForExtension>
-      extension_permissions_;
+  std::unique_ptr<platform_keys::ExtensionKeyPermissionsService>
+      extension_key_permissions_service_;
   ExtensionPlatformKeysService* const service_;
   base::WeakPtrFactory<SignTask> weak_factory_{this};
 
@@ -513,16 +520,18 @@ class ExtensionPlatformKeysService::SelectTask : public Task {
   }
 
   void GetExtensionPermissions() {
-    service_->key_permissions_service_->GetPermissionsForExtension(
-        extension_id_,
-        base::BindOnce(&SelectTask::GotPermissions, base::Unretained(this)));
+    platform_keys::ExtensionKeyPermissionsServiceFactory::
+        GetForBrowserContextAndExtension(
+            base::BindOnce(&SelectTask::GotPermissions, base::Unretained(this)),
+            service_->browser_context_, extension_id_,
+            service_->key_permissions_service_);
   }
 
   void GotPermissions(
-      std::unique_ptr<
-          platform_keys::KeyPermissionsService::PermissionsForExtension>
-          extension_permissions) {
-    extension_permissions_ = std::move(extension_permissions);
+      std::unique_ptr<platform_keys::ExtensionKeyPermissionsService>
+          extension_key_permissions_service) {
+    extension_key_permissions_service_ =
+        std::move(extension_key_permissions_service);
     DoStep();
   }
 
@@ -590,7 +599,7 @@ class ExtensionPlatformKeysService::SelectTask : public Task {
     const std::string public_key_spki_der(
         platform_keys::GetSubjectPublicKeyInfo(certificate));
 
-    extension_permissions_->CanUseKeyForSigning(
+    extension_key_permissions_service_->CanUseKeyForSigning(
         public_key_spki_der,
         base::BindOnce(&SelectTask::OnKeySigningPermissionKnown,
                        base::Unretained(this), public_key_spki_der,
@@ -605,7 +614,10 @@ class ExtensionPlatformKeysService::SelectTask : public Task {
       matches_.push_back(certificate);
       DoStep();
     } else if (interactive_) {
-      service_->key_permissions_service_->CanUserGrantPermissionForKey(
+      platform_keys::KeyPermissionsService* const key_permissions_service =
+          platform_keys::KeyPermissionsServiceFactory::GetForBrowserContext(
+              service_->browser_context_);
+      key_permissions_service->CanUserGrantPermissionForKey(
           public_key_spki_der,
           base::BindOnce(&SelectTask::OnAbilityToGrantPermissionKnown,
                          base::Unretained(this), std::move(certificate)));
@@ -673,7 +685,7 @@ class ExtensionPlatformKeysService::SelectTask : public Task {
     }
     const std::string public_key_spki_der(
         platform_keys::GetSubjectPublicKeyInfo(selected_cert_));
-    extension_permissions_->SetUserGrantedPermission(
+    extension_key_permissions_service_->SetUserGrantedPermission(
         public_key_spki_der, base::BindOnce(&SelectTask::OnPermissionsUpdated,
                                             base::Unretained(this)));
   }
@@ -713,8 +725,8 @@ class ExtensionPlatformKeysService::SelectTask : public Task {
   const std::string extension_id_;
   const SelectCertificatesCallback callback_;
   content::WebContents* const web_contents_;
-  std::unique_ptr<platform_keys::KeyPermissionsService::PermissionsForExtension>
-      extension_permissions_;
+  std::unique_ptr<platform_keys::ExtensionKeyPermissionsService>
+      extension_key_permissions_service_;
   ExtensionPlatformKeysService* const service_;
   base::WeakPtrFactory<SelectTask> weak_factory_{this};
 
@@ -739,7 +751,6 @@ ExtensionPlatformKeysService::ExtensionPlatformKeysService(
           chromeos::platform_keys::KeyPermissionsServiceFactory::
               GetForBrowserContext(browser_context)) {
   DCHECK(platform_keys_service_);
-  DCHECK(key_permissions_service_);
   DCHECK(browser_context);
   DCHECK(state_store);
 }

chrome/browser/chromeos/platform_keys/extension_platform_keys_service.h

@@ -74,7 +74,7 @@ class ExtensionPlatformKeysService : public KeyedService {
 
   // Stores registration information in |state_store|, i.e. for each extension
   // the list of public keys that are valid to be used for signing. See
-  // |KeyPermissionsManager| for details.
+  // |ExtensionKeyPermissionsService| for more details.
   // |browser_context| and |state_store| must not be null and outlive this
   // object.
   explicit ExtensionPlatformKeysService(

chrome/browser/chromeos/platform_keys/key_permissions/extension_key_permissions_service_factory.cc

+// Copyright 2020 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/chromeos/platform_keys/key_permissions/extension_key_permissions_service.h"
+
+#include <memory>
+
+#include "base/logging.h"
+#include "chrome/browser/chromeos/platform_keys/key_permissions/extension_key_permissions_service.h"
+#include "chrome/browser/chromeos/platform_keys/key_permissions/extension_key_permissions_service_factory.h"
+#include "chrome/browser/chromeos/platform_keys/key_permissions/key_permissions_service.h"
+#include "chrome/browser/chromeos/platform_keys/key_permissions/key_permissions_service_factory.h"
+#include "chrome/browser/chromeos/platform_keys/platform_keys_service.h"
+#include "chrome/browser/chromeos/platform_keys/platform_keys_service_factory.h"
+#include "chrome/browser/policy/profile_policy_connector.h"
+#include "chrome/browser/profiles/profile.h"
+#include "extensions/browser/extension_system.h"
+#include "extensions/browser/state_store.h"
+#include "extensions/common/extension_id.h"
+
+namespace chromeos {
+namespace platform_keys {
+
+namespace {
+
+void OnGotExtensionValue(GetExtensionKeyPermissionsServiceCallback callback,
+                         content::BrowserContext* context,
+                         extensions::ExtensionId extension_id,
+                         PlatformKeysService* platform_keys_service,
+                         KeyPermissionsService* key_permissions_service,
+                         std::unique_ptr<base::Value> value) {
+  Profile* profile = Profile::FromBrowserContext(context);
+  if (!profile) {
+    std::move(callback).Run(/*extension_key_permissions_service=*/nullptr);
+    return;
+  }
+
+  std::move(callback).Run(std::make_unique<ExtensionKeyPermissionsService>(
+      extension_id, extensions::ExtensionSystem::Get(profile)->state_store(),
+      std::move(value), profile->GetProfilePolicyConnector()->policy_service(),
+      platform_keys_service, key_permissions_service));
+}
+
+}  // namespace
+
+// static
+void ExtensionKeyPermissionsServiceFactory::GetForBrowserContextAndExtension(
+    GetExtensionKeyPermissionsServiceCallback callback,
+    content::BrowserContext* context,
+    extensions::ExtensionId extension_id,
+    KeyPermissionsService* key_permissions_service) {
+  DCHECK(context);
+  DCHECK(key_permissions_service);
+
+  PlatformKeysService* const platform_keys_service =
+      PlatformKeysServiceFactory::GetForBrowserContext(context);
+  // Must not be nullptr since KeyPermissionsServiceFactory depends on
+  // PlatformKeysServiceFactory.
+  DCHECK(platform_keys_service);
+
+  extensions::StateStore* const state_store =
+      extensions::ExtensionSystem::Get(context)->state_store();
+
+  // Must not be nullptr since KeyPermissionsServiceFactory depends on
+  // ExtensionSystemFactory.
+  DCHECK(state_store);
+
+  state_store->GetExtensionValue(
+      extension_id, kStateStorePlatformKeys,
+      base::BindOnce(OnGotExtensionValue, std::move(callback), context,
+                     extension_id, platform_keys_service,
+                     key_permissions_service));
+}
+
+ExtensionKeyPermissionsServiceFactory::ExtensionKeyPermissionsServiceFactory() =
+    default;
+ExtensionKeyPermissionsServiceFactory::
+    ~ExtensionKeyPermissionsServiceFactory() = default;
+
+}  // namespace platform_keys
+}  // namespace chromeos

chrome/browser/chromeos/platform_keys/key_permissions/extension_key_permissions_service_factory.h

+// Copyright 2020 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.
+
+#ifndef CHROME_BROWSER_CHROMEOS_PLATFORM_KEYS_KEY_PERMISSIONS_EXTENSION_KEY_PERMISSIONS_SERVICE_FACTORY_H_
+#define CHROME_BROWSER_CHROMEOS_PLATFORM_KEYS_KEY_PERMISSIONS_EXTENSION_KEY_PERMISSIONS_SERVICE_FACTORY_H_
+
+#include "base/callback_forward.h"
+#include "memory"
+
+#include "extensions/common/extension_id.h"
+
+namespace content {
+class BrowserContext;
+}
+
+namespace chromeos {
+namespace platform_keys {
+
+class ExtensionKeyPermissionsService;
+class KeyPermissionsService;
+
+using GetExtensionKeyPermissionsServiceCallback =
+    base::OnceCallback<void(std::unique_ptr<ExtensionKeyPermissionsService>
+                                extension_key_permissions_service)>;
+
+// ExtensionKeyPermissionsServiceFactory can be used for retrieving
+// ExtensionKeyPermissionsService instances for a specific (Profile, Extension)
+// pair.
+//
+// Note: The underlying KeyPermissionsService instance used by a provided
+// ExtensionKeyPermissionsService is only valid during the lifetime of the given
+// BrowserContext, i.e., it is the responsibility of the service consumer to
+// make sure that the BrowserContext is valid during the lifetime of the
+// service.
+class ExtensionKeyPermissionsServiceFactory {
+ public:
+  // |context| and |key_permissions_service| must not be nullptr and must
+  // outlive the provided ExtensionKeyPermissionsService instance.
+  // |key_permissions_service| must be a KeyPermissionsService instance
+  // corresponding to |context|.
+  static void GetForBrowserContextAndExtension(
+      GetExtensionKeyPermissionsServiceCallback callback,
+      content::BrowserContext* context,
+      extensions::ExtensionId extension_id,
+      KeyPermissionsService* key_permissions_service);
+
+ private:
+  ExtensionKeyPermissionsServiceFactory();
+  ~ExtensionKeyPermissionsServiceFactory();
+};
+
+}  // namespace platform_keys
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_CHROMEOS_PLATFORM_KEYS_KEY_PERMISSIONS_EXTENSION_KEY_PERMISSIONS_SERVICE_FACTORY_H_

chrome/browser/chromeos/platform_keys/key_permissions/key_permissions_service.cc

@@ -7,12 +7,6 @@
 namespace chromeos {
 namespace platform_keys {
 
-KeyPermissionsService::PermissionsForExtension::PermissionsForExtension() =
-    default;
-
-KeyPermissionsService::PermissionsForExtension::~PermissionsForExtension() =
-    default;
-
 KeyPermissionsService::KeyPermissionsService() = default;
 
 KeyPermissionsService::~KeyPermissionsService() = default;

chrome/browser/chromeos/platform_keys/key_permissions/key_permissions_service.h

@@ -16,16 +16,6 @@
 namespace chromeos {
 namespace platform_keys {
 
-using CanUseKeyForSigningCallback = base::OnceCallback<void(bool allowed)>;
-
-using RegisterKeyForCorporateUsageCallback =
-    base::OnceCallback<void(Status status)>;
-
-using SetUserGrantedPermissionCallback =
-    base::OnceCallback<void(Status status)>;
-
-using SetKeyUsedForSigningCallback = base::OnceCallback<void(Status status)>;
-
 using CanUserGrantPermissionForKeyCallback =
     base::OnceCallback<void(bool allowed)>;
 
@@ -33,99 +23,25 @@ using IsCorporateKeyCallback = base::OnceCallback<void(bool corporate)>;
 
 using SetCorporateKeyCallback = base::OnceCallback<void(Status status)>;
 
-// This service will be responsible for answering queries regarding platform key
-// permissions with respect to a specific profile.
-//
-// Use KeyPermissionsServiceFactory to retrieve instances of this service.
-//
-// This class manages permissions for extensions to use private keys through
-// chrome.platformKeys or chrome.enterprise.platformKeys .
-// The permission model depends on whether the user account is managed or not.
-//
-// ** If the user account is not managed **
-// The user is under full control of the keys that are generated or imported
-// while the device is not managed.  For that, a user can grant a specific
-// extension the permission to sign arbitrary data with a specific key for an
-// unlimited number of times.
-//
-// ** If the user account is managed **
-// The administrator is in charge of granting access to keys that are meant for
-// corporate usage.
+// ** KeyPermissionService Responsibility **
+// A KeyPermissionService instance is responsible for answering queries
+// regarding platform keys permissions with respect to a specific profile.
 //
+// ** Corporate Usage **
 // As not every key is meant for corporate usage but probably for the user's
 // private usage, this class introduces the concept of tagging keys with the
 // intended purpose of the key. Currently, the only usage that can be assigned
 // to a key is "corporate".
-//
 // Every key that is generated by the chrome.enterprise.platformKeys API (which
 // requires the user account to be managed), is marked for corporate usage.
 // Any key that is generated or imported by other means is currently not marked
 // for corporate usage.
-//
-// The KeyPermissions policy allows the administrator to list exactly the
-// extensions that are allowed to use such corporate keys. Non-corporate keys
-// are not affected. This policy is the only means to grant this permission.
-//
-// ** One-off Permission for the Certification Requests **
-// Independent of the above, the extension that generates a key using the
-// chrome.enterprise.platformKeys API is allowed to sign arbitrary data with the
-// private key for a single time in order to create a certification request.
-// The assumption is that certification requests usually require a signature of
-// data including the public key. So the one-off permission implies that once a
-// certificate authority creates the certificate of the generated key, the
-// generating extension isn't able to use the key anymore except if explicitly
-// permitted by the administrator.
+
 class KeyPermissionsService : public KeyedService {
  public:
-  // Allows querying and modifying permissions and registering keys for a
-  // specific extension.
-  class PermissionsForExtension {
-   public:
-    PermissionsForExtension();
-    virtual ~PermissionsForExtension();
-
-    // Determines if the private key matching |public_key_spki_der| can be used
-    // for signing by the extension with id |extension_id_|. |callback| will be
-    // invoked with the result.
-    virtual void CanUseKeyForSigning(const std::string& public_key_spki_der,
-                                     CanUseKeyForSigningCallback callback) = 0;
-
-    // Must be called when the extension with id |extension_id| used the private
-    // key matching |public_key_spki_der| for signing. |callback| will be
-    // invoked with the resulting status. Updates the permissions accordingly.
-    // E.g. if this extension generated the key and no other permission was
-    // granted then the permission to sign with this key is removed.
-    virtual void SetKeyUsedForSigning(
-        const std::string& public_key_spki_der,
-        SetKeyUsedForSigningCallback callback) = 0;
-
-    // Registers the private key matching |public_key_spki_der| as being
-    // generated by the extension with id |extension_id| and marks it for
-    // corporate usage. |callback| will be invoked with the resulting status.
-    virtual void RegisterKeyForCorporateUsage(
-        const std::string& public_key_spki_der,
-        RegisterKeyForCorporateUsageCallback callback) = 0;
-
-    // Sets the user granted permission that the extension with id
-    // |extension_id| can use the private key matching |public_key_spki_der| for
-    // signing. |callback| will be invoked with the resulting status.
-    virtual void SetUserGrantedPermission(
-        const std::string& public_key_spki_der,
-        SetUserGrantedPermissionCallback callback) = 0;
-  };
-
   KeyPermissionsService();
   ~KeyPermissionsService() override;
 
-  using GetPermissionsForExtensionCallback = base::OnceCallback<void(
-      std::unique_ptr<PermissionsForExtension> permissions_for_extension)>;
-  // Passes an object managing the key permissions of the extension with id
-  // |extension_id| to |callback|. This can happen synchronously or
-  // asynchronously.
-  virtual void GetPermissionsForExtension(
-      const std::string& extension_id,
-      GetPermissionsForExtensionCallback callback) = 0;
-
   // Determines if the user can grant any permission for |public_key_spki_der|
   // to extensions. |callback| will be invoked with the result.
   virtual void CanUserGrantPermissionForKey(

chrome/browser/chromeos/platform_keys/key_permissions/key_permissions_service_impl.h

@@ -12,15 +12,13 @@
 #include "base/callback_forward.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/platform_keys/key_permissions/key_permissions_service.h"
 #include "chrome/browser/chromeos/platform_keys/platform_keys.h"
+#include "extensions/browser/state_store.h"
 
 class PrefService;
 
-namespace base {
-class Value;
-}
-
 namespace extensions {
 class StateStore;
 }
@@ -38,105 +36,6 @@ class PlatformKeysService;
 // classes.
 class KeyPermissionsServiceImpl : public KeyPermissionsService {
  public:
-  // Implementation of PermissionsForExtension.
-  class PermissionsForExtensionImpl : public PermissionsForExtension {
-   public:
-    // |key_permissions_service| must not be null and outlive this object.
-    // Methods of this object refer implicitly to the extension with the id
-    // |extension_id|. Don't use this constructor directly. Call
-    // |KeyPermissionsService::GetPermissionsForExtension| instead.
-    PermissionsForExtensionImpl(
-        const std::string& extension_id,
-        std::unique_ptr<base::Value> state_store_value,
-        PrefService* profile_prefs,
-        policy::PolicyService* profile_policies,
-        KeyPermissionsServiceImpl* key_permissions_service_impl);
-
-    PermissionsForExtensionImpl(const PermissionsForExtensionImpl& other) =
-        delete;
-    PermissionsForExtensionImpl& operator=(
-        const PermissionsForExtensionImpl& other) = delete;
-    ~PermissionsForExtensionImpl() override;
-
-    void CanUseKeyForSigning(const std::string& public_key_spki_der,
-                             CanUseKeyForSigningCallback callback) override;
-
-    void SetKeyUsedForSigning(const std::string& public_key_spki_der,
-                              SetKeyUsedForSigningCallback callback) override;
-
-    void RegisterKeyForCorporateUsage(
-        const std::string& public_key_spki_der,
-        RegisterKeyForCorporateUsageCallback callback) override;
-
-    void SetUserGrantedPermission(
-        const std::string& public_key_spki_der,
-        SetUserGrantedPermissionCallback callback) override;
-
-   private:
-    struct KeyEntry;
-
-    // Writes the current |state_store_entries_| to the state store of
-    // |extension_id_|.
-    void WriteToStateStore();
-
-    // Reads a KeyEntry list from |state| and stores them in
-    // |state_store_entries_|.
-    void KeyEntriesFromState(const base::Value& state);
-
-    // Converts |state_store_entries_| to a base::Value for storing in the state
-    // store.
-    std::unique_ptr<base::Value> KeyEntriesToState();
-
-    // Returns an existing entry for |public_key_spki_der_b64| from
-    // |state_store_entries_|. If there is no existing entry, creates, adds and
-    // returns a new entry.
-    // |public_key_spki_der| must be the base64 encoding of the DER of a Subject
-    // Public Key Info.
-    KeyPermissionsServiceImpl::PermissionsForExtensionImpl::KeyEntry*
-    GetStateStoreEntry(const std::string& public_key_spki_der_b64);
-
-    bool PolicyAllowsCorporateKeyUsage() const;
-
-    void CanUseKeyForSigningWithLocations(
-        const std::string& public_key_spki_der,
-        CanUseKeyForSigningCallback callback,
-        const std::vector<TokenId>& key_locations,
-        Status key_locations_retrieval_status);
-    void CanUseKeyForSigningWithFlags(CanUseKeyForSigningCallback callback,
-                                      bool sign_unlimited_allowed,
-                                      bool is_corporate_key);
-
-    void SetKeyUsedForSigningWithLocations(
-        const std::string& public_key_spki_der,
-        SetKeyUsedForSigningCallback callback,
-        const std::vector<TokenId>& key_locations,
-        Status key_locations_retrieval_status);
-    void RegisterKeyForCorporateUsageWithLocations(
-        const std::string& public_key_spki_der,
-        RegisterKeyForCorporateUsageCallback callback,
-        const std::vector<TokenId>& key_locations,
-        Status key_locations_retrieval_status);
-
-    void SetUserGrantedPermissionWithLocations(
-        const std::string& public_key_spki_der,
-        SetUserGrantedPermissionCallback callback,
-        const std::vector<TokenId>& key_locations,
-        Status key_locations_retrieval_status);
-    void SetUserGrantedPermissionWithLocationsAndFlag(
-        const std::string& public_key_spki_der,
-        SetUserGrantedPermissionCallback callback,
-        const std::vector<TokenId>& key_locations,
-        Status key_locations_retrieval_status,
-        bool can_user_grant_permission);
-
-    const std::string extension_id_;
-    std::vector<KeyEntry> state_store_entries_;
-    PrefService* const profile_prefs_;
-    policy::PolicyService* const profile_policies_;
-    KeyPermissionsServiceImpl* const key_permissions_service_;
-    base::WeakPtrFactory<PermissionsForExtensionImpl> weak_factory_{this};
-  };
-
   // |profile_prefs| and |extensions_state_store| must not be null and must
   // outlive this object.
   // If |profile_is_managed| is false, |profile_policies| is ignored. Otherwise,
@@ -155,10 +54,6 @@ class KeyPermissionsServiceImpl : public KeyPermissionsService {
   KeyPermissionsServiceImpl& operator=(const KeyPermissionsServiceImpl& other) =
       delete;
 
-  void GetPermissionsForExtension(
-      const std::string& extension_id,
-      GetPermissionsForExtensionCallback callback) override;
-
   void CanUserGrantPermissionForKey(
       const std::string& public_key_spki_der,
       CanUserGrantPermissionForKeyCallback callback) const override;
@@ -169,23 +64,11 @@ class KeyPermissionsServiceImpl : public KeyPermissionsService {
   void SetCorporateKey(const std::string& public_key_spki_der,
                        SetCorporateKeyCallback callback) const override;
 
-  // Returns the list of apps and extensions ids allowed to use corporate usage
-  // keys by policy in |profile_policies|.
-  static std::vector<std::string> GetCorporateKeyUsageAllowedAppIds(
-      policy::PolicyService* const profile_policies);
+  PlatformKeysService* platform_keys_service() {
+    return platform_keys_service_;
+  }
 
  private:
-  // Creates a PermissionsForExtension object from |extension_id| and |value|
-  // and passes the object to |callback|.
-  void CreatePermissionObjectAndPassToCallback(
-      const std::string& extension_id,
-      GetPermissionsForExtensionCallback callback,
-      std::unique_ptr<base::Value> value);
-
-  // Writes |value| to the state store of the extension with id |extension_id|.
-  void SetPlatformKeysOfExtension(const std::string& extension_id,
-                                  std::unique_ptr<base::Value> value);
-
   // Returns true if |public_key_spki_der_b64| (which is located only on a user
   // token) is marked for corporate usage.
   bool IsUserKeyCorporate(const std::string& public_key_spki_der_b64) const;

chrome/browser/chromeos/platform_keys/key_permissions/mock_key_permissions_service.h

@@ -26,12 +26,6 @@ class MockKeyPermissionsService : public KeyPermissionsService {
   MockKeyPermissionsService();
   ~MockKeyPermissionsService() override;
 
-  MOCK_METHOD(void,
-              GetPermissionsForExtension,
-              (const std::string& extension_id,
-               GetPermissionsForExtensionCallback callback),
-              (override));
-
   MOCK_METHOD(void,
               CanUserGrantPermissionForKey,
               (const std::string& public_key_spki_der,

chrome/browser/extensions/api/platform_keys/platform_keys_apitest_nss.cc

@@ -14,6 +14,8 @@
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/chromeos/platform_keys/extension_platform_keys_service.h"
 #include "chrome/browser/chromeos/platform_keys/extension_platform_keys_service_factory.h"
+#include "chrome/browser/chromeos/platform_keys/key_permissions/extension_key_permissions_service.h"
+#include "chrome/browser/chromeos/platform_keys/key_permissions/extension_key_permissions_service_factory.h"
 #include "chrome/browser/chromeos/platform_keys/key_permissions/key_permissions_service.h"
 #include "chrome/browser/chromeos/platform_keys/key_permissions/key_permissions_service_factory.h"
 #include "chrome/browser/chromeos/platform_keys/platform_keys.h"
@@ -140,10 +142,11 @@ class PlatformKeysTest : public PlatformKeysTestBase {
     ASSERT_TRUE(key_permissions_service);
 
     base::RunLoop run_loop;
-    key_permissions_service->GetPermissionsForExtension(
-        fake_gen_extension->id(),
-        base::Bind(&PlatformKeysTest::GotPermissionsForExtension,
-                   base::Unretained(this), run_loop.QuitClosure()));
+    chromeos::platform_keys::ExtensionKeyPermissionsServiceFactory::
+        GetForBrowserContextAndExtension(
+            base::BindOnce(&PlatformKeysTest::GotPermissionsForExtension,
+                           base::Unretained(this), run_loop.QuitClosure()),
+            profile(), fake_gen_extension->id(), key_permissions_service);
     run_loop.Run();
   }
 
@@ -165,8 +168,8 @@ class PlatformKeysTest : public PlatformKeysTestBase {
   }
 
   void OnKeyRegisteredForCorporateUsage(
-      std::unique_ptr<chromeos::platform_keys::KeyPermissionsService::
-                          PermissionsForExtension> permissions_for_ext,
+      std::unique_ptr<chromeos::platform_keys::ExtensionKeyPermissionsService>
+          extension_key_permissions_service,
       const base::Closure& done_callback,
       chromeos::platform_keys::Status status) {
     ASSERT_EQ(status, chromeos::platform_keys::Status::kSuccess);
@@ -175,15 +178,17 @@ class PlatformKeysTest : public PlatformKeysTestBase {
 
   void GotPermissionsForExtension(
       const base::Closure& done_callback,
-      std::unique_ptr<chromeos::platform_keys::KeyPermissionsService::
-                          PermissionsForExtension> permissions_for_ext) {
-    auto* permissions_for_ext_unowned = permissions_for_ext.get();
+      std::unique_ptr<chromeos::platform_keys::ExtensionKeyPermissionsService>
+          extension_key_permissions_service) {
+    auto* extension_key_permissions_service_unowned =
+        extension_key_permissions_service.get();
     std::string client_cert1_spki =
         chromeos::platform_keys::GetSubjectPublicKeyInfo(client_cert1_);
-    permissions_for_ext_unowned->RegisterKeyForCorporateUsage(
+    extension_key_permissions_service_unowned->RegisterKeyForCorporateUsage(
         client_cert1_spki,
         base::BindOnce(&PlatformKeysTest::OnKeyRegisteredForCorporateUsage,
-                       base::Unretained(this), std::move(permissions_for_ext),
+                       base::Unretained(this),
+                       std::move(extension_key_permissions_service),
                        done_callback));
   }