Commit abc5b03a authored by Omar Morsi's avatar Omar Morsi Committed by Commit Bot

KeyPermissionsService: Allow usage by non-regular user profiles

A KeyPermissionsService instance needs a KeyPermissionsManager instance
responsible for the private token for the user profile to be able to do
user token operations using this instance. But there is no private token
corresponding to non-regular user profiles (e.g. sign in profile,
lockscreen profile), which means that there will be no corresponding
KeyPermissionsManager instance provided for the non-regular user
profiles from the UserPrivateTokenKpm factory. This CL allows
KeyPermissionsService to be used without a KeyPermissionsManager
instance in case of non-regular user profiles.

Bug: 1148290
Change-Id: I8dfd29732adcee4ebfc3f208b9dbf73713b274b7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2534852
Commit-Queue: Omar Morsi <omorsi@google.com>
Reviewed-by: default avatarPavol Marko <pmarko@chromium.org>
Reviewed-by: default avatarAlexander Hendrich <hendrich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826809}
parent f1d04107
...@@ -199,9 +199,16 @@ KeyPermissionsManagerImpl::GetSystemTokenKeyPermissionsManager() { ...@@ -199,9 +199,16 @@ KeyPermissionsManagerImpl::GetSystemTokenKeyPermissionsManager() {
KeyPermissionsManager* KeyPermissionsManager*
KeyPermissionsManagerImpl::GetUserPrivateTokenKeyPermissionsManager( KeyPermissionsManagerImpl::GetUserPrivateTokenKeyPermissionsManager(
Profile* profile) { Profile* profile) {
return UserPrivateTokenKeyPermissionsManagerServiceFactory::GetInstance() auto* const user_private_token_kpm_service =
->GetForBrowserContext(profile) UserPrivateTokenKeyPermissionsManagerServiceFactory::GetInstance()
->key_permissions_manager(); ->GetForBrowserContext(profile);
if (!user_private_token_kpm_service) {
DCHECK(!ProfileHelper::IsRegularProfile(profile));
return nullptr;
}
return user_private_token_kpm_service->key_permissions_manager();
} }
// static // static
......
...@@ -88,6 +88,7 @@ class KeyPermissionsManagerImpl : public KeyPermissionsManager, ...@@ -88,6 +88,7 @@ class KeyPermissionsManagerImpl : public KeyPermissionsManager,
static KeyPermissionsManager* GetSystemTokenKeyPermissionsManager(); static KeyPermissionsManager* GetSystemTokenKeyPermissionsManager();
// Returns a key permissions manager that manages keys residing on the user // Returns a key permissions manager that manages keys residing on the user
// token corresponding to |profile|. // token corresponding to |profile|.
// Note: A nullptr will be returned for non-regular user profiles.
static KeyPermissionsManager* GetUserPrivateTokenKeyPermissionsManager( static KeyPermissionsManager* GetUserPrivateTokenKeyPermissionsManager(
Profile* profile); Profile* profile);
......
...@@ -26,6 +26,8 @@ using SetCorporateKeyCallback = base::OnceCallback<void(Status status)>; ...@@ -26,6 +26,8 @@ using SetCorporateKeyCallback = base::OnceCallback<void(Status status)>;
// ** KeyPermissionService Responsibility ** // ** KeyPermissionService Responsibility **
// A KeyPermissionService instance is responsible for answering queries // A KeyPermissionService instance is responsible for answering queries
// regarding platform keys permissions with respect to a specific profile. // regarding platform keys permissions with respect to a specific profile.
// Note: KeyPermissionsService instances can be used for sign-in profile to
// answer queries for system-wide keys.
// //
// ** Corporate Usage ** // ** Corporate Usage **
// As not every key is meant for corporate usage but probably for the user's // As not every key is meant for corporate usage but probably for the user's
......
...@@ -51,6 +51,7 @@ KeyedService* KeyPermissionsServiceFactory::BuildServiceInstanceFor( ...@@ -51,6 +51,7 @@ KeyedService* KeyPermissionsServiceFactory::BuildServiceInstanceFor(
} }
return new KeyPermissionsServiceImpl( return new KeyPermissionsServiceImpl(
ProfileHelper::IsRegularProfile(profile),
profile->GetProfilePolicyConnector()->IsManaged(), profile->GetPrefs(), profile->GetProfilePolicyConnector()->IsManaged(), profile->GetPrefs(),
profile->GetProfilePolicyConnector()->policy_service(), profile->GetProfilePolicyConnector()->policy_service(),
extensions::ExtensionSystem::Get(profile)->state_store(), extensions::ExtensionSystem::Get(profile)->state_store(),
......
...@@ -21,6 +21,7 @@ ...@@ -21,6 +21,7 @@
#include "chrome/browser/chromeos/platform_keys/key_permissions/key_permissions_pref_util.h" #include "chrome/browser/chromeos/platform_keys/key_permissions/key_permissions_pref_util.h"
#include "chrome/browser/chromeos/platform_keys/platform_keys.h" #include "chrome/browser/chromeos/platform_keys/platform_keys.h"
#include "chrome/browser/chromeos/platform_keys/platform_keys_service.h" #include "chrome/browser/chromeos/platform_keys/platform_keys_service.h"
#include "chrome/browser/chromeos/profiles/profile_helper.h"
#include "chrome/browser/policy/profile_policy_connector.h" #include "chrome/browser/policy/profile_policy_connector.h"
#include "chrome/common/pref_names.h" #include "chrome/common/pref_names.h"
#include "components/policy/core/common/policy_map.h" #include "components/policy/core/common/policy_map.h"
...@@ -35,13 +36,15 @@ namespace chromeos { ...@@ -35,13 +36,15 @@ namespace chromeos {
namespace platform_keys { namespace platform_keys {
KeyPermissionsServiceImpl::KeyPermissionsServiceImpl( KeyPermissionsServiceImpl::KeyPermissionsServiceImpl(
bool is_regular_user_profile,
bool profile_is_managed, bool profile_is_managed,
PrefService* profile_prefs, PrefService* profile_prefs,
policy::PolicyService* profile_policies, policy::PolicyService* profile_policies,
extensions::StateStore* extensions_state_store, extensions::StateStore* extensions_state_store,
PlatformKeysService* platform_keys_service, PlatformKeysService* platform_keys_service,
KeyPermissionsManager* profile_key_permissions_manager) KeyPermissionsManager* profile_key_permissions_manager)
: profile_is_managed_(profile_is_managed), : is_regular_user_profile_(is_regular_user_profile),
profile_is_managed_(profile_is_managed),
profile_prefs_(profile_prefs), profile_prefs_(profile_prefs),
profile_policies_(profile_policies), profile_policies_(profile_policies),
extensions_state_store_(extensions_state_store), extensions_state_store_(extensions_state_store),
...@@ -50,7 +53,7 @@ KeyPermissionsServiceImpl::KeyPermissionsServiceImpl( ...@@ -50,7 +53,7 @@ KeyPermissionsServiceImpl::KeyPermissionsServiceImpl(
DCHECK(profile_prefs_); DCHECK(profile_prefs_);
DCHECK(extensions_state_store_); DCHECK(extensions_state_store_);
DCHECK(platform_keys_service_); DCHECK(platform_keys_service_);
DCHECK(profile_key_permissions_manager); DCHECK(profile_key_permissions_manager || !is_regular_user_profile);
DCHECK(!profile_is_managed_ || profile_policies_); DCHECK(!profile_is_managed_ || profile_policies_);
} }
...@@ -133,6 +136,8 @@ void KeyPermissionsServiceImpl::IsCorporateKeyWithLocations( ...@@ -133,6 +136,8 @@ void KeyPermissionsServiceImpl::IsCorporateKeyWithLocations(
for (const auto key_location : key_locations) { for (const auto key_location : key_locations) {
switch (key_location) { switch (key_location) {
case TokenId::kUser: case TokenId::kUser:
DCHECK(is_regular_user_profile_);
if (internal::IsUserKeyMarkedCorporateInPref(public_key_spki_der, if (internal::IsUserKeyMarkedCorporateInPref(public_key_spki_der,
profile_prefs_)) { profile_prefs_)) {
std::move(callback).Run(/*corporate=*/true); std::move(callback).Run(/*corporate=*/true);
...@@ -183,6 +188,8 @@ void KeyPermissionsServiceImpl::SetCorporateKeyWithLocations( ...@@ -183,6 +188,8 @@ void KeyPermissionsServiceImpl::SetCorporateKeyWithLocations(
public_key_spki_der); public_key_spki_der);
return; return;
case TokenId::kUser: { case TokenId::kUser: {
DCHECK(is_regular_user_profile_);
internal::MarkUserKeyCorporateInPref(public_key_spki_der, profile_prefs_); internal::MarkUserKeyCorporateInPref(public_key_spki_der, profile_prefs_);
profile_key_permissions_manager_->AllowKeyForUsage( profile_key_permissions_manager_->AllowKeyForUsage(
......
...@@ -44,6 +44,7 @@ class KeyPermissionsServiceImpl : public KeyPermissionsService { ...@@ -44,6 +44,7 @@ class KeyPermissionsServiceImpl : public KeyPermissionsService {
// |profile_is_managed| determines the default usage and permissions for // |profile_is_managed| determines the default usage and permissions for
// keys without explicitly assigned usage. // keys without explicitly assigned usage.
KeyPermissionsServiceImpl( KeyPermissionsServiceImpl(
bool is_regular_user_profile,
bool profile_is_managed, bool profile_is_managed,
PrefService* profile_prefs, PrefService* profile_prefs,
policy::PolicyService* profile_policies, policy::PolicyService* profile_policies,
...@@ -98,6 +99,7 @@ class KeyPermissionsServiceImpl : public KeyPermissionsService { ...@@ -98,6 +99,7 @@ class KeyPermissionsServiceImpl : public KeyPermissionsService {
const std::vector<TokenId>& key_locations, const std::vector<TokenId>& key_locations,
Status key_locations_retrieval_status); Status key_locations_retrieval_status);
const bool is_regular_user_profile_;
const bool profile_is_managed_; const bool profile_is_managed_;
PrefService* const profile_prefs_; PrefService* const profile_prefs_;
policy::PolicyService* const profile_policies_; policy::PolicyService* const profile_policies_;
......
...@@ -124,9 +124,9 @@ class KeyPermissionsServiceImplTest : public ::testing::Test { ...@@ -124,9 +124,9 @@ class KeyPermissionsServiceImplTest : public ::testing::Test {
key_permissions_manager_ = std::make_unique<MockKeyPermissionsManager>(); key_permissions_manager_ = std::make_unique<MockKeyPermissionsManager>();
key_permissions_service_ = std::make_unique<KeyPermissionsServiceImpl>( key_permissions_service_ = std::make_unique<KeyPermissionsServiceImpl>(
/*profile_is_managed=*/true, profile_->GetPrefs(), policy_service_, /*is_regular_profile=*/true, /*profile_is_managed=*/true,
extensions_state_store_, platform_keys_service_.get(), profile_->GetPrefs(), policy_service_, extensions_state_store_,
key_permissions_manager_.get()); platform_keys_service_.get(), key_permissions_manager_.get());
} }
protected: protected:
......
...@@ -77,8 +77,7 @@ UserPrivateTokenKeyPermissionsManagerServiceFactory::BuildServiceInstanceFor( ...@@ -77,8 +77,7 @@ UserPrivateTokenKeyPermissionsManagerServiceFactory::BuildServiceInstanceFor(
content::BrowserContext* context) const { content::BrowserContext* context) const {
Profile* profile = Profile::FromBrowserContext(context); Profile* profile = Profile::FromBrowserContext(context);
if (ProfileHelper::IsSigninProfile(profile) || if (!ProfileHelper::IsRegularProfile(profile)) {
ProfileHelper::IsLockScreenAppProfile(profile)) {
return nullptr; return nullptr;
} }
......
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