Commit 524b141e authored by Pavol Marko's avatar Pavol Marko Committed by Commit Bot

Make interactions with KeyPermissionsManager testable

Split KeyPermissionsManager,
KeyPermissionsManager::PermissionsForExtension and
KeyPermissionsManagerUserService into a public virtual API and
implementation in *Impl classes.
This will be used in a follow-up CL to test interactions with
KeyPermissionsManager in unit tests.

Note: This CL does not change any behavior.

Bug: 1127506
Change-Id: Iacbbd6e533db0b476b2fb5d6e04e241966c7bafa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2413248Reviewed-by: default avatarMaksim Ivanov <emaxx@chromium.org>
Reviewed-by: default avatarEdman Anjos <edman@chromium.org>
Reviewed-by: default avatarOmar Morsi <omorsi@google.com>
Commit-Queue: Edman Anjos <edman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808017}
parent c86b3be0
......@@ -1927,6 +1927,8 @@ source_set("chromeos") {
"platform_keys/extension_platform_keys_service_factory.h",
"platform_keys/key_permissions/key_permissions_manager.cc",
"platform_keys/key_permissions/key_permissions_manager.h",
"platform_keys/key_permissions/key_permissions_manager_impl.cc",
"platform_keys/key_permissions/key_permissions_manager_impl.h",
"platform_keys/key_permissions/key_permissions_manager_user_service.cc",
"platform_keys/key_permissions/key_permissions_manager_user_service.h",
"platform_keys/key_permissions/key_permissions_policy_handler.cc",
......@@ -2984,6 +2986,8 @@ static_library("test_support") {
"login/test/test_predicate_waiter.h",
"login/version_updater/mock_version_updater_delegate.cc",
"login/version_updater/mock_version_updater_delegate.h",
"platform_keys/key_permissions/mock_key_permissions_manager.cc",
"platform_keys/key_permissions/mock_key_permissions_manager.h",
"platform_keys/mock_platform_keys_service.cc",
"platform_keys/mock_platform_keys_service.h",
"plugin_vm/fake_plugin_vm_features.cc",
......
......@@ -13,7 +13,7 @@
#include "base/callback.h"
#include "base/logging.h"
#include "base/memory/singleton.h"
#include "chrome/browser/chromeos/platform_keys/key_permissions/key_permissions_manager.h"
#include "chrome/browser/chromeos/platform_keys/key_permissions/key_permissions_manager_impl.h"
#include "chrome/browser/chromeos/platform_keys/platform_keys_service.h"
#include "chrome/browser/net/nss_context.h"
#include "chrome/browser/policy/profile_policy_connector.h"
......@@ -82,8 +82,8 @@ bool IsCertificateAllowed(const scoped_refptr<net::X509Certificate>& cert,
std::string spki_der = chromeos::platform_keys::GetSubjectPublicKeyInfo(cert);
std::string public_key_spki_der_b64;
base::Base64Encode(spki_der, &public_key_spki_der_b64);
if (!chromeos::platform_keys::KeyPermissionsManager::IsCorporateKeyForProfile(
public_key_spki_der_b64, prefs)) {
if (!chromeos::platform_keys::KeyPermissionsManagerImpl::
IsCorporateKeyForProfile(public_key_spki_der_b64, prefs)) {
DVLOG(1) << "Certificate is not allowed to be used by ARC.";
return false;
}
......@@ -303,8 +303,9 @@ void ArcCertStoreBridge::OnCertificatesListed(
void ArcCertStoreBridge::UpdateFromKeyPermissionsPolicy() {
DVLOG(1) << "ArcCertStoreBridge::UpdateFromKeyPermissionsPolicy";
std::vector<std::string> app_ids = chromeos::platform_keys::
KeyPermissionsManager::GetCorporateKeyUsageAllowedAppIds(policy_service_);
std::vector<std::string> app_ids =
chromeos::platform_keys::KeyPermissionsManagerImpl::
GetCorporateKeyUsageAllowedAppIds(policy_service_);
std::vector<std::string> permissions;
for (const auto& app_id : app_ids) {
if (LooksLikeAndroidPackageName(app_id))
......
......@@ -10,24 +10,8 @@
#include <vector>
#include "base/callback_forward.h"
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "chrome/browser/chromeos/platform_keys/platform_keys.h"
class PrefService;
namespace base {
class Value;
}
namespace extensions {
class StateStore;
}
namespace policy {
class PolicyService;
}
namespace chromeos {
namespace platform_keys {
......@@ -77,41 +61,32 @@ class KeyPermissionsManager {
// specific extension.
class PermissionsForExtension {
public:
// |key_permissions| 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
// |KeyPermissionsManager::GetPermissionsForExtension| instead.
PermissionsForExtension(const std::string& extension_id,
std::unique_ptr<base::Value> state_store_value,
PrefService* profile_prefs,
policy::PolicyService* profile_policies,
KeyPermissionsManager* key_permissions);
~PermissionsForExtension();
PermissionsForExtension();
virtual ~PermissionsForExtension();
// Returns true if the private key matching |public_key_spki_der| can be
// used for signing by the extension with id |extension_id_|.
// |key_locations| must describe locations available to the user the private
// key is stored on.
bool CanUseKeyForSigning(
virtual bool CanUseKeyForSigning(
const std::string& public_key_spki_der,
const std::vector<platform_keys::TokenId>& key_locations);
const std::vector<platform_keys::TokenId>& key_locations) = 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. |key_locations| must describe locations available to the
// user the private key is stored on.
void RegisterKeyForCorporateUsage(
virtual void RegisterKeyForCorporateUsage(
const std::string& public_key_spki_der,
const std::vector<platform_keys::TokenId>& key_locations);
const std::vector<platform_keys::TokenId>& key_locations) = 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. |key_locations| must describe locations available to the user
// the private key is stored on.
void SetUserGrantedPermission(
virtual void SetUserGrantedPermission(
const std::string& public_key_spki_der,
const std::vector<platform_keys::TokenId>& key_locations);
const std::vector<platform_keys::TokenId>& key_locations) = 0;
// Must be called when the extension with id |extension_id| used the private
// key matching |public_key_spki_der| for signing. |key_locations| must
......@@ -119,56 +94,13 @@ class KeyPermissionsManager {
// 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.
void SetKeyUsedForSigning(
virtual void SetKeyUsedForSigning(
const std::string& public_key_spki_der,
const std::vector<platform_keys::TokenId>& key_locations);
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.
KeyPermissionsManager::PermissionsForExtension::KeyEntry*
GetStateStoreEntry(const std::string& public_key_spki_der_b64);
bool PolicyAllowsCorporateKeyUsage() const;
const std::string extension_id_;
std::vector<KeyEntry> state_store_entries_;
PrefService* const profile_prefs_;
policy::PolicyService* const profile_policies_;
KeyPermissionsManager* const key_permissions_;
DISALLOW_COPY_AND_ASSIGN(PermissionsForExtension);
const std::vector<platform_keys::TokenId>& key_locations) = 0;
};
// |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,
// |profile_policies| must not be null and must outlive this object.
// |profile_is_managed| determines the default usage and permissions for
// keys without explicitly assigned usage.
KeyPermissionsManager(bool profile_is_managed,
PrefService* profile_prefs,
policy::PolicyService* profile_policies,
extensions::StateStore* extensions_state_store);
~KeyPermissionsManager();
KeyPermissionsManager();
virtual ~KeyPermissionsManager();
using PermissionsCallback =
base::Callback<void(std::unique_ptr<PermissionsForExtension>)>;
......@@ -176,49 +108,16 @@ class KeyPermissionsManager {
// Passes an object managing the key permissions of the extension with id
// |extension_id| to |callback|. This can happen synchronously or
// asynchronously.
void GetPermissionsForExtension(const std::string& extension_id,
const PermissionsCallback& callback);
virtual void GetPermissionsForExtension(
const std::string& extension_id,
const PermissionsCallback& callback) = 0;
// Returns true if the user can grant any permission for
// |public_key_spki_derey_id| to extensions. |key_locations| must describe
// locations available to the user the private key is stored on.
bool CanUserGrantPermissionFor(
virtual bool CanUserGrantPermissionFor(
const std::string& public_key_spki_der,
const std::vector<platform_keys::TokenId>& key_locations) const;
// Returns true if |public_key_spki_der_b64| is a corporate usage key.
static bool IsCorporateKeyForProfile(
const std::string& public_key_spki_der_b64,
const PrefService* const profile_prefs);
// 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);
private:
bool IsCorporateKey(
const std::string& public_key_spki_der_b64,
const std::vector<platform_keys::TokenId>& key_locations) const;
// Creates a PermissionsForExtension object from |extension_id| and |value|
// and passes the object to |callback|.
void CreatePermissionObjectAndPassToCallback(
const std::string& extension_id,
const PermissionsCallback& 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);
const bool profile_is_managed_;
PrefService* const profile_prefs_;
policy::PolicyService* const profile_policies_;
extensions::StateStore* const extensions_state_store_;
base::WeakPtrFactory<KeyPermissionsManager> weak_factory_{this};
DISALLOW_COPY_AND_ASSIGN(KeyPermissionsManager);
const std::vector<platform_keys::TokenId>& key_locations) const = 0;
};
} // namespace platform_keys
......
// 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_KEY_PERMISSIONS_MANAGER_IMPL_H_
#define CHROME_BROWSER_CHROMEOS_PLATFORM_KEYS_KEY_PERMISSIONS_KEY_PERMISSIONS_MANAGER_IMPL_H_
#include <memory>
#include <string>
#include <vector>
#include "base/callback_forward.h"
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "chrome/browser/chromeos/platform_keys/key_permissions/key_permissions_manager.h"
#include "chrome/browser/chromeos/platform_keys/platform_keys.h"
class PrefService;
namespace base {
class Value;
}
namespace extensions {
class StateStore;
}
namespace policy {
class PolicyService;
}
namespace chromeos {
namespace platform_keys {
class KeyPermissionsManagerImpl : public KeyPermissionsManager {
public:
// Implementation of PermissionsForExtension.
class PermissionsForExtensionImpl : public PermissionsForExtension {
public:
// |key_permissions| 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
// |KeyPermissionsManager::GetPermissionsForExtension| instead.
PermissionsForExtensionImpl(const std::string& extension_id,
std::unique_ptr<base::Value> state_store_value,
PrefService* profile_prefs,
policy::PolicyService* profile_policies,
KeyPermissionsManagerImpl* key_permissions);
PermissionsForExtensionImpl(const PermissionsForExtensionImpl& other) =
delete;
PermissionsForExtensionImpl& operator=(
const PermissionsForExtensionImpl& other) = delete;
~PermissionsForExtensionImpl() override;
bool CanUseKeyForSigning(
const std::string& public_key_spki_der,
const std::vector<platform_keys::TokenId>& key_locations) override;
void RegisterKeyForCorporateUsage(
const std::string& public_key_spki_der,
const std::vector<platform_keys::TokenId>& key_locations) override;
void SetUserGrantedPermission(
const std::string& public_key_spki_der,
const std::vector<platform_keys::TokenId>& key_locations) override;
void SetKeyUsedForSigning(
const std::string& public_key_spki_der,
const std::vector<platform_keys::TokenId>& key_locations) 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.
KeyPermissionsManagerImpl::PermissionsForExtensionImpl::KeyEntry*
GetStateStoreEntry(const std::string& public_key_spki_der_b64);
bool PolicyAllowsCorporateKeyUsage() const;
const std::string extension_id_;
std::vector<KeyEntry> state_store_entries_;
PrefService* const profile_prefs_;
policy::PolicyService* const profile_policies_;
KeyPermissionsManagerImpl* const key_permissions_;
};
// |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,
// |profile_policies| must not be null and must outlive this object.
// |profile_is_managed| determines the default usage and permissions for
// keys without explicitly assigned usage.
KeyPermissionsManagerImpl(bool profile_is_managed,
PrefService* profile_prefs,
policy::PolicyService* profile_policies,
extensions::StateStore* extensions_state_store);
~KeyPermissionsManagerImpl() override;
KeyPermissionsManagerImpl(const KeyPermissionsManagerImpl& other) = delete;
KeyPermissionsManagerImpl& operator=(const KeyPermissionsManagerImpl& other) =
delete;
void GetPermissionsForExtension(const std::string& extension_id,
const PermissionsCallback& callback) override;
bool CanUserGrantPermissionFor(
const std::string& public_key_spki_der,
const std::vector<platform_keys::TokenId>& key_locations) const override;
// Returns true if |public_key_spki_der_b64| is a corporate usage key.
static bool IsCorporateKeyForProfile(
const std::string& public_key_spki_der_b64,
const PrefService* const profile_prefs);
// 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);
private:
bool IsCorporateKey(
const std::string& public_key_spki_der_b64,
const std::vector<platform_keys::TokenId>& key_locations) const;
// Creates a PermissionsForExtension object from |extension_id| and |value|
// and passes the object to |callback|.
void CreatePermissionObjectAndPassToCallback(
const std::string& extension_id,
const PermissionsCallback& 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);
const bool profile_is_managed_;
PrefService* const profile_prefs_;
policy::PolicyService* const profile_policies_;
extensions::StateStore* const extensions_state_store_;
base::WeakPtrFactory<KeyPermissionsManagerImpl> weak_factory_{this};
};
} // namespace platform_keys
} // namespace chromeos
#endif // CHROME_BROWSER_CHROMEOS_PLATFORM_KEYS_KEY_PERMISSIONS_KEY_PERMISSIONS_MANAGER_IMPL_H_
......@@ -4,7 +4,7 @@
#include "chrome/browser/chromeos/platform_keys/key_permissions/key_permissions_manager_user_service.h"
#include "chrome/browser/chromeos/platform_keys/key_permissions/key_permissions_manager.h"
#include "chrome/browser/chromeos/platform_keys/key_permissions/key_permissions_manager_impl.h"
#include "chrome/browser/chromeos/profiles/profile_helper.h"
#include "chrome/browser/extensions/extension_system_factory.h"
#include "chrome/browser/policy/profile_policy_connector.h"
......@@ -19,16 +19,32 @@ namespace platform_keys {
// ==================== KeyPermissionsManagerUserService =======================
KeyPermissionsManagerUserService::KeyPermissionsManagerUserService(
Profile* profile)
: key_permissions_manager_(
profile->GetProfilePolicyConnector()->IsManaged(),
profile->GetPrefs(),
profile->GetProfilePolicyConnector()->policy_service(),
extensions::ExtensionSystem::Get(profile)->state_store()) {}
KeyPermissionsManagerUserService::KeyPermissionsManagerUserService() = default;
KeyPermissionsManagerUserService::~KeyPermissionsManagerUserService() = default;
// ================== KeyPermissionsManagerUserServiceImpl =====================
class KeyPermissionsManagerUserServiceImpl
: public KeyPermissionsManagerUserService {
public:
explicit KeyPermissionsManagerUserServiceImpl(Profile* profile)
: key_permissions_manager_(
profile->GetProfilePolicyConnector()->IsManaged(),
profile->GetPrefs(),
profile->GetProfilePolicyConnector()->policy_service(),
extensions::ExtensionSystem::Get(profile)->state_store()) {}
~KeyPermissionsManagerUserServiceImpl() override = default;
KeyPermissionsManager* key_permissions_manager() override {
return &key_permissions_manager_;
}
private:
KeyPermissionsManagerImpl key_permissions_manager_;
};
// ================== KeyPermissionsManagerUserServiceFactory ==================
// static
......@@ -61,7 +77,7 @@ KeyedService* KeyPermissionsManagerUserServiceFactory::BuildServiceInstanceFor(
return nullptr;
}
return new KeyPermissionsManagerUserService(profile);
return new KeyPermissionsManagerUserServiceImpl(profile);
}
void KeyPermissionsManagerUserServiceFactory::RegisterProfilePrefs(
......
......@@ -10,8 +10,6 @@
#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
#include "components/keyed_service/core/keyed_service.h"
class Profile;
namespace user_prefs {
class PrefRegistrySyncable;
}
......@@ -24,15 +22,10 @@ namespace platform_keys {
// so as future work can introduce a global device-wide KPM instance.
class KeyPermissionsManagerUserService : public KeyedService {
public:
explicit KeyPermissionsManagerUserService(Profile* profile);
KeyPermissionsManagerUserService();
~KeyPermissionsManagerUserService() override;
KeyPermissionsManager* key_permissions_manager() {
return &key_permissions_manager_;
}
private:
KeyPermissionsManager key_permissions_manager_;
virtual KeyPermissionsManager* key_permissions_manager() = 0;
};
class KeyPermissionsManagerUserServiceFactory
......
// 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/mock_key_permissions_manager.h"
namespace chromeos {
namespace platform_keys {
MockKeyPermissionsManager::MockKeyPermissionsManager() = default;
MockKeyPermissionsManager::~MockKeyPermissionsManager() = default;
} // namespace platform_keys
} // namespace chromeos
// 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_MOCK_KEY_PERMISSIONS_MANAGER_H_
#define CHROME_BROWSER_CHROMEOS_PLATFORM_KEYS_KEY_PERMISSIONS_MOCK_KEY_PERMISSIONS_MANAGER_H_
#include <string>
#include <vector>
#include "base/callback_forward.h"
#include "chrome/browser/chromeos/platform_keys/key_permissions/key_permissions_manager.h"
#include "chrome/browser/chromeos/platform_keys/platform_keys.h"
#include "testing/gmock/include/gmock/gmock.h"
namespace chromeos {
namespace platform_keys {
class MockKeyPermissionsManager : public KeyPermissionsManager {
public:
MockKeyPermissionsManager();
MockKeyPermissionsManager(const MockKeyPermissionsManager&) = delete;
MockKeyPermissionsManager& operator=(const MockKeyPermissionsManager&) =
delete;
~MockKeyPermissionsManager() override;
MOCK_METHOD(void,
GetPermissionsForExtension,
(const std::string& extension_id,
const PermissionsCallback& callback),
(override));
MOCK_METHOD(bool,
CanUserGrantPermissionFor,
(const std::string& public_key_spki_der,
const std::vector<platform_keys::TokenId>& key_locations),
(const override));
MOCK_METHOD(bool,
IsCorporateKey,
(const std::string& public_key_spki_der_b64,
const std::vector<platform_keys::TokenId>& key_locations),
(const override));
MOCK_METHOD(void,
SetCorporateKey,
(const std::string& public_key_spki_der_b64,
platform_keys::TokenId key_location),
(const override));
};
} // namespace platform_keys
} // namespace chromeos
#endif // CHROME_BROWSER_CHROMEOS_PLATFORM_KEYS_KEY_PERMISSIONS_MOCK_KEY_PERMISSIONS_MANAGER_H_
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment