diff options
21 files changed, 168 insertions, 49 deletions
diff --git a/chrome/browser/chromeos/login/session/user_session_manager.cc b/chrome/browser/chromeos/login/session/user_session_manager.cc index 0a14599..f4721b8 100644 --- a/chrome/browser/chromeos/login/session/user_session_manager.cc +++ b/chrome/browser/chromeos/login/session/user_session_manager.cc @@ -1041,7 +1041,7 @@ void UserSessionManager::UserProfileInitialized(Profile* profile, if (has_auth_cookies_) { const user_manager::User* user = user_manager::UserManager::Get()->FindUser(account_id); - if (user->is_affiliated()) { + if (user->IsAffiliated()) { CrosSettings::Get()->GetBoolean( kAccountsPrefTransferSAMLCookies, &transfer_saml_auth_cookies_on_subsequent_login); diff --git a/chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc b/chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc index cb06709..56af6f7 100644 --- a/chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc +++ b/chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc @@ -1192,7 +1192,7 @@ void ChromeUserManagerImpl::SetUserAffiliation( const bool is_affiliated = chromeos::IsUserAffiliated( user_affiliation_ids, connector->GetDeviceAffiliationIDs(), account_id.GetUserEmail(), connector->GetEnterpriseDomain()); - user->set_affiliation(is_affiliated); + user->SetAffiliation(is_affiliated); if (user->GetType() == user_manager::USER_TYPE_REGULAR) { if (is_affiliated) { diff --git a/chrome/browser/chromeos/login/users/fake_chrome_user_manager.cc b/chrome/browser/chromeos/login/users/fake_chrome_user_manager.cc index 9b739af..8d4f8d7 100644 --- a/chrome/browser/chromeos/login/users/fake_chrome_user_manager.cc +++ b/chrome/browser/chromeos/login/users/fake_chrome_user_manager.cc @@ -42,7 +42,7 @@ const user_manager::User* FakeChromeUserManager::AddUserWithAffiliation( const AccountId& account_id, bool is_affiliated) { user_manager::User* user = user_manager::User::CreateRegularUser(account_id); - user->set_affiliation(is_affiliated); + user->SetAffiliation(is_affiliated); user->set_username_hash(ProfileHelper::GetUserIdHashByUserIdForTesting( account_id.GetUserEmail())); user->SetStubImage(user_manager::UserImage( diff --git a/chrome/browser/chromeos/login/users/mock_user_manager.cc b/chrome/browser/chromeos/login/users/mock_user_manager.cc index 513cb27..c894bd7 100644 --- a/chrome/browser/chromeos/login/users/mock_user_manager.cc +++ b/chrome/browser/chromeos/login/users/mock_user_manager.cc @@ -129,7 +129,7 @@ void MockUserManager::AddUser(const AccountId& account_id) { void MockUserManager::AddUserWithAffiliation(const AccountId& account_id, bool is_affiliated) { user_manager::User* user = user_manager::User::CreateRegularUser(account_id); - user->set_affiliation(is_affiliated); + user->SetAffiliation(is_affiliated); user_list_.push_back(user); ProfileHelper::Get()->SetProfileToUserMappingForTesting(user); } @@ -145,7 +145,7 @@ void MockUserManager::ClearUserList() { bool MockUserManager::ShouldReportUser(const std::string& user_id) const { for (const auto& user : user_list_) { if (user->email() == user_id) - return user->is_affiliated(); + return user->IsAffiliated(); } NOTREACHED(); return false; diff --git a/chrome/browser/chromeos/platform_keys/platform_keys_nss.cc b/chrome/browser/chromeos/platform_keys/platform_keys_nss.cc index d1d9877..f210607 100644 --- a/chrome/browser/chromeos/platform_keys/platform_keys_nss.cc +++ b/chrome/browser/chromeos/platform_keys/platform_keys_nss.cc @@ -782,7 +782,7 @@ void SelectClientCertificates( // Use the device-wide system key slot only if the user is affiliated on the // device. - bool use_system_key_slot = user->is_affiliated(); + const bool use_system_key_slot = user->IsAffiliated(); scoped_ptr<SelectCertificatesState> state(new SelectCertificatesState( user->username_hash(), use_system_key_slot, cert_request_info, callback)); diff --git a/chrome/browser/chromeos/policy/affiliated_invalidation_service_provider_impl.cc b/chrome/browser/chromeos/policy/affiliated_invalidation_service_provider_impl.cc index 3e44c84..d853a88 100644 --- a/chrome/browser/chromeos/policy/affiliated_invalidation_service_provider_impl.cc +++ b/chrome/browser/chromeos/policy/affiliated_invalidation_service_provider_impl.cc @@ -157,7 +157,7 @@ void AffiliatedInvalidationServiceProviderImpl::Observe( } const user_manager::User* user = chromeos::ProfileHelper::Get()->GetUserByProfile(profile); - if (!user || !user->is_affiliated()) { + if (!user || !user->IsAffiliated()) { // If the Profile belongs to a user who is not affiliated on the device, // ignore it. return; diff --git a/chrome/browser/chromeos/policy/device_local_account_browsertest.cc b/chrome/browser/chromeos/policy/device_local_account_browsertest.cc index d628acd..e84881b 100644 --- a/chrome/browser/chromeos/policy/device_local_account_browsertest.cc +++ b/chrome/browser/chromeos/policy/device_local_account_browsertest.cc @@ -883,6 +883,16 @@ IN_PROC_BROWSER_TEST_F(DeviceLocalAccountTest, LoginScreen) { CheckPublicSessionPresent(account_id_1_); CheckPublicSessionPresent(account_id_2_); + + ASSERT_TRUE(user_manager::UserManager::Get()->FindUser(account_id_1_)); + EXPECT_TRUE(user_manager::UserManager::Get() + ->FindUser(account_id_1_) + ->IsAffiliated()); + + ASSERT_TRUE(user_manager::UserManager::Get()->FindUser(account_id_2_)); + EXPECT_TRUE(user_manager::UserManager::Get() + ->FindUser(account_id_2_) + ->IsAffiliated()); } // Flaky: http://crbug.com/512670. diff --git a/chrome/browser/chromeos/policy/user_affiliation_browsertest.cc b/chrome/browser/chromeos/policy/user_affiliation_browsertest.cc index c2a3000..4b63296 100644 --- a/chrome/browser/chromeos/policy/user_affiliation_browsertest.cc +++ b/chrome/browser/chromeos/policy/user_affiliation_browsertest.cc @@ -84,7 +84,7 @@ IN_PROC_BROWSER_TEST_P(UserAffiliationBrowserTest, Affiliated) { EXPECT_EQ(GetParam().affiliated_, user_manager::UserManager::Get() ->FindUser(AccountId::FromUserEmail(kAffiliatedUser)) - ->is_affiliated()); + ->IsAffiliated()); } INSTANTIATE_TEST_CASE_P(AffiliationCheck, diff --git a/chrome/browser/chromeos/proxy_config_service_impl.cc b/chrome/browser/chromeos/proxy_config_service_impl.cc index 6e07ba7..4a626fa 100644 --- a/chrome/browser/chromeos/proxy_config_service_impl.cc +++ b/chrome/browser/chromeos/proxy_config_service_impl.cc @@ -160,7 +160,7 @@ bool ProxyConfigServiceImpl::IgnoreProxy(const PrefService* profile_prefs, if (onc_source == ::onc::ONC_SOURCE_DEVICE_POLICY) { const user_manager::User* logged_in_user = user_manager::UserManager::Get()->GetLoggedInUser(); - if (logged_in_user->is_affiliated()) { + if (logged_in_user->IsAffiliated()) { VLOG(1) << "Respecting proxy for network, as logged-in user belongs to " << "the domain the device is enrolled to."; return false; diff --git a/chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_api.cc b/chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_api.cc index a0024f7..5166107 100644 --- a/chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_api.cc +++ b/chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_api.cc @@ -22,7 +22,7 @@ bool IsPermittedToGetDeviceId(content::BrowserContext* context) { const user_manager::User* user = chromeos::ProfileHelper::Get()->GetUserByProfile( Profile::FromBrowserContext(context)); - return user->is_affiliated(); + return user->IsAffiliated(); } // Returns the directory device id for the permitted extensions or an empty diff --git a/chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc b/chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc index 685e0d7..c9dc8bb 100644 --- a/chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc +++ b/chrome/browser/extensions/api/enterprise_device_attributes/enterprise_device_attributes_apitest.cc @@ -200,7 +200,7 @@ IN_PROC_BROWSER_TEST_P(EnterpriseDeviceAttributesTest, Success) { SetPolicy(); EXPECT_EQ(GetParam().affiliated_, user_manager::UserManager::Get()-> - FindUser(affiliated_account_id_)->is_affiliated()); + FindUser(affiliated_account_id_)->IsAffiliated()); // Device ID is available only for affiliated user. std::string device_id = GetParam().affiliated_ ? kDeviceId : ""; diff --git a/chrome/browser/profiles/profile_io_data.cc b/chrome/browser/profiles/profile_io_data.cc index 20d93b9..65e2862 100644 --- a/chrome/browser/profiles/profile_io_data.cc +++ b/chrome/browser/profiles/profile_io_data.cc @@ -455,7 +455,7 @@ void ProfileIOData::InitializeOnUIThread(Profile* profile) { // Use the device-wide system key slot only if the user is affiliated on // the device. - params->use_system_key_slot = user->is_affiliated(); + params->use_system_key_slot = user->IsAffiliated(); } } diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn index 28460c7..a01d1470 100644 --- a/chrome/test/BUILD.gn +++ b/chrome/test/BUILD.gn @@ -139,7 +139,6 @@ source_set("test_support") { "//components/guest_view/browser:test_support", "//components/infobars/core", "//components/sessions:test_support", - "//components/user_manager:test_support", "//components/web_resource:test_support", "//content/public/child", "//content/public/common", @@ -205,7 +204,10 @@ source_set("test_support") { } } if (is_chromeos) { - public_deps += [ "//components/ownership" ] + public_deps += [ + "//components/ownership", + "//components/user_manager:test_support", + ] } if (use_aura) { @@ -873,7 +875,6 @@ if (!is_android) { "//components/safe_browsing_db:test_database_manager", "//components/strings", "//components/translate/core/common", - "//components/user_manager:test_support", "//crypto:platform", "//crypto:test_support", "//device/bluetooth:mocks", @@ -1046,6 +1047,7 @@ if (!is_android) { "../browser/ui/webui/signin/user_manager_ui_browsertest.cc", ] deps += [ + "//components/user_manager:test_support", "//dbus", "//dbus:test_support", "//ui/login:resources", diff --git a/components/BUILD.gn b/components/BUILD.gn index e8766e7..61e1148 100644 --- a/components/BUILD.gn +++ b/components/BUILD.gn @@ -212,6 +212,7 @@ test("components_unittests") { deps += [ "//components/arc:unit_tests", "//components/ownership:unit_tests", + "//components/user_manager:unit_tests", ] } diff --git a/components/components_tests.gyp b/components/components_tests.gyp index fc03b143..0b8541f 100644 --- a/components/components_tests.gyp +++ b/components/components_tests.gyp @@ -799,6 +799,9 @@ 'url_matcher/url_matcher_factory_unittest.cc', 'url_matcher/url_matcher_unittest.cc', ], + 'user_manager_unittest_sources': [ + 'user_manager/user_unittest.cc', + ], 'variations_unittest_sources': [ 'variations/active_field_trials_unittest.cc', 'variations/caching_permuted_entropy_provider_unittest.cc', @@ -1416,6 +1419,7 @@ 'wifi_sync/wifi_security_class_unittest.cc', '<@(metrics_leak_detector_unittest_sources)', '<@(ownership_unittest_sources)', + '<@(user_manager_unittest_sources)', ], 'sources!': [ 'signin/core/browser/signin_status_metrics_provider_unittest.cc', diff --git a/components/user_manager/BUILD.gn b/components/user_manager/BUILD.gn index 959bcfe..1a2a355 100644 --- a/components/user_manager/BUILD.gn +++ b/components/user_manager/BUILD.gn @@ -46,9 +46,9 @@ component("user_manager") { } } -source_set("test_support") { - testonly = true - if (is_chromeos) { +if (is_chromeos) { + source_set("test_support") { + testonly = true sources = [ "fake_user_manager.cc", "fake_user_manager.h", @@ -62,4 +62,18 @@ source_set("test_support") { "//ui/base", ] } + + source_set("unit_tests") { + testonly = true + sources = [ + "user_unittest.cc", + ] + deps = [ + ":user_manager", + "//components/signin/core/account_id", + "//skia", + "//testing/gtest", + "//ui/gfx", + ] + } } diff --git a/components/user_manager/fake_user_manager.cc b/components/user_manager/fake_user_manager.cc index 7dca0e5..7efb1d3 100644 --- a/components/user_manager/fake_user_manager.cc +++ b/components/user_manager/fake_user_manager.cc @@ -47,7 +47,7 @@ const user_manager::User* FakeUserManager::AddUserWithAffiliation( const AccountId& account_id, bool is_affiliated) { user_manager::User* user = user_manager::User::CreateRegularUser(account_id); - user->set_affiliation(is_affiliated); + user->SetAffiliation(is_affiliated); users_.push_back(user); return user; } diff --git a/components/user_manager/user.cc b/components/user_manager/user.cc index 2a09a22..a77ab8b 100644 --- a/components/user_manager/user.cc +++ b/components/user_manager/user.cc @@ -69,7 +69,22 @@ class GuestUser : public User { DISALLOW_COPY_AND_ASSIGN(GuestUser); }; -class KioskAppUser : public User { +class DeviceLocalAccountUserBase : public User { + public: + // User: + bool IsAffiliated() const override; + + protected: + explicit DeviceLocalAccountUserBase(const AccountId& account_id); + ~DeviceLocalAccountUserBase() override; + // User: + void SetAffiliation(bool) override; + + private: + DISALLOW_COPY_AND_ASSIGN(DeviceLocalAccountUserBase); +}; + +class KioskAppUser : public DeviceLocalAccountUserBase { public: explicit KioskAppUser(const AccountId& kiosk_app_account_id); ~KioskAppUser() override; @@ -94,7 +109,7 @@ class SupervisedUser : public User { DISALLOW_COPY_AND_ASSIGN(SupervisedUser); }; -class PublicAccountUser : public User { +class PublicAccountUser : public DeviceLocalAccountUserBase { public: explicit PublicAccountUser(const AccountId& account_id); ~PublicAccountUser() override; @@ -181,6 +196,14 @@ bool User::is_active() const { return is_active_; } +bool User::IsAffiliated() const { + return is_affiliated_; +} + +void User::SetAffiliation(bool is_affiliated) { + is_affiliated_ = is_affiliated; +} + User* User::CreateRegularUser(const AccountId& account_id) { return new RegularUser(account_id); } @@ -265,8 +288,25 @@ UserType GuestUser::GetType() const { return user_manager::USER_TYPE_GUEST; } +DeviceLocalAccountUserBase::DeviceLocalAccountUserBase( + const AccountId& account_id) : User(account_id) { +} + +DeviceLocalAccountUserBase::~DeviceLocalAccountUserBase() { +} + +bool DeviceLocalAccountUserBase::IsAffiliated() const { + return true; +} + +void DeviceLocalAccountUserBase::SetAffiliation(bool) { + // Device local accounts are always affiliated. No affiliation modification + // must happen. + NOTREACHED(); +} + KioskAppUser::KioskAppUser(const AccountId& kiosk_app_account_id) - : User(kiosk_app_account_id) { + : DeviceLocalAccountUserBase(kiosk_app_account_id) { set_display_email(kiosk_app_account_id.GetUserEmail()); } @@ -293,7 +333,7 @@ std::string SupervisedUser::display_email() const { } PublicAccountUser::PublicAccountUser(const AccountId& account_id) - : User(account_id) {} + : DeviceLocalAccountUserBase(account_id) {} PublicAccountUser::~PublicAccountUser() { } diff --git a/components/user_manager/user.h b/components/user_manager/user.h index 2f5542a..0d6431b6 100644 --- a/components/user_manager/user.h +++ b/components/user_manager/user.h @@ -8,6 +8,7 @@ #include <string> #include <vector> +#include "base/gtest_prod_util.h" #include "base/macros.h" #include "base/strings/string16.h" #include "components/signin/core/account_id/account_id.h" @@ -79,19 +80,6 @@ class USER_MANAGER_EXPORT User : public UserInfo { // Returns true if user type has gaia account. static bool TypeHasGaiaAccount(UserType user_type); - // Returns the user type. - virtual UserType GetType() const = 0; - - // The email the user used to log in. - // TODO(alemate): rename this to GetUserEmail() (see crbug.com/548923) - const std::string& email() const; - - // The displayed user name. - base::string16 display_name() const { return display_name_; } - - // If the user has to use SAML to log in. - bool using_saml() const { return using_saml_; } - // UserInfo std::string GetEmail() const override; base::string16 GetDisplayName() const override; @@ -99,6 +87,9 @@ class USER_MANAGER_EXPORT User : public UserInfo { const gfx::ImageSkia& GetImage() const override; const AccountId& GetAccountId() const override; + // Returns the user type. + virtual UserType GetType() const = 0; + // Allows managing child status of the user. Used for RegularUser. virtual void SetIsChild(bool is_child); @@ -109,6 +100,25 @@ class USER_MANAGER_EXPORT User : public UserInfo { // Returns true if user is supervised. virtual bool IsSupervised() const; + // True if user image can be synced. + virtual bool CanSyncImage() const; + + // The displayed (non-canonical) user email. + virtual std::string display_email() const; + + // True if the user is affiliated to the device. + virtual bool IsAffiliated() const; + + // The email the user used to log in. + // TODO(alemate): rename this to GetUserEmail() (see crbug.com/548923) + const std::string& email() const; + + // The displayed user name. + base::string16 display_name() const { return display_name_; } + + // If the user has to use SAML to log in. + bool using_saml() const { return using_saml_; } + // Returns the account name part of the email. Use the display form of the // email if available and use_display_name == true. Otherwise use canonical. std::string GetAccountName(bool use_display_email) const; @@ -116,9 +126,6 @@ class USER_MANAGER_EXPORT User : public UserInfo { // Whether the user has a default image. bool HasDefaultImage() const; - // True if user image can be synced. - virtual bool CanSyncImage() const; - int image_index() const { return image_index_; } bool has_raw_image() const { return user_image_.has_raw_image(); } // Returns raw representation of static user image. @@ -140,9 +147,6 @@ class USER_MANAGER_EXPORT User : public UserInfo { // True if image is being loaded from file. bool image_is_loading() const { return image_is_loading_; } - // The displayed (non-canonical) user email. - virtual std::string display_email() const; - // OAuth token status for this user. OAuthTokenStatus oauth_token_status() const { return oauth_token_status_; } @@ -166,9 +170,6 @@ class USER_MANAGER_EXPORT User : public UserInfo { // True if the user Profile is created. bool is_profile_created() const { return profile_is_created_; } - // True if the user is affiliated to the device. - bool is_affiliated() const { return is_affiliated_; } - protected: friend class UserManagerBase; friend class chromeos::ChromeUserManagerImpl; @@ -181,6 +182,7 @@ class USER_MANAGER_EXPORT User : public UserInfo { friend class chromeos::FakeChromeUserManager; friend class chromeos::MockUserManager; friend class chromeos::UserAddingScreenTest; + FRIEND_TEST_ALL_PREFIXES(UserTest, DeviceLocalAccountAffiliation); // Do not allow anyone else to create new User instances. static User* CreateRegularUser(const AccountId& account_id); @@ -247,9 +249,7 @@ class USER_MANAGER_EXPORT User : public UserInfo { // True if user has google account (not a guest or managed user). bool has_gaia_account() const; - void set_affiliation(bool is_affiliated) { - is_affiliated_ = is_affiliated; - } + virtual void SetAffiliation(bool is_affiliated); private: AccountId account_id_; diff --git a/components/user_manager/user_unittest.cc b/components/user_manager/user_unittest.cc new file mode 100644 index 0000000..dcc4019 --- /dev/null +++ b/components/user_manager/user_unittest.cc @@ -0,0 +1,45 @@ +// Copyright 2016 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 "components/user_manager/user.h" + +#include "components/signin/core/account_id/account_id.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace user_manager { + +namespace { + +const char kEmail[] = "email@example.com"; +const char kGaiaId[] = "fake_gaia_id"; + +} // namespace + +TEST(UserTest, DeviceLocalAccountAffiliation) { + // This implementation of RAII for User* is to prevent memory leak. + // Smart pointers are not friends of User and can't call protected destructor. + class ScopedUser { + public: + ScopedUser(const User* const user) : user_(user) {} + ~ScopedUser() { delete user_; } + + bool IsAffiliated() const { return user_ && user_->IsAffiliated(); } + + private: + const User* const user_; + + DISALLOW_COPY_AND_ASSIGN(ScopedUser); + }; + + const AccountId account_id = AccountId::FromUserEmailGaiaId(kEmail, kGaiaId); + + ScopedUser kiosk_user(User::CreateKioskAppUser(account_id)); + EXPECT_TRUE(kiosk_user.IsAffiliated()); + + ScopedUser public_session_user(User::CreatePublicAccountUser(account_id)); + EXPECT_TRUE(public_session_user.IsAffiliated()); + +} + +} // namespace user_manager diff --git a/extensions/BUILD.gn b/extensions/BUILD.gn index 3874b03..df4b552 100644 --- a/extensions/BUILD.gn +++ b/extensions/BUILD.gn @@ -325,7 +325,6 @@ source_set("chrome_extensions_browsertests") { "//components/resources", "//components/strings", "//components/translate/core/common", - "//components/user_manager:test_support", "//crypto:platform", "//crypto:test_support", "//device/bluetooth:mocks", @@ -360,6 +359,10 @@ source_set("chrome_extensions_browsertests") { "//ui/web_dialogs:test_support", "//v8", ] + + if (is_chromeos) { + deps += [ "//components/user_manager:test_support" ] + } } # TODO(rockot) bug 505926: This should be deleted for the same reason as |