Commit cd8c165e authored by Aga Wronska's avatar Aga Wronska Committed by Commit Bot

Create ConsumerStatusReportingService and move StatusUploader that is used

to report child user status.

ConsumerStatusReportingService is a KeyedService created for child users only.

Moving StatusUploader from UserCloudPolicyManagerChromeOS helps to
enforce that consumer status reports are only uploaded for child user.
Additionally it simplifies DeviceStatusCollector by passing an already
initialized pref service.

Test: Run DeviceStatusCollectorTest, ConsumerDeviceCollectorTest, StatusUploaderTest,
UserCloudPolicyManagerChromeOSTest.
Manually add child account to device (managed/consumer) and observe status upload after ~60s.
Manually sign in as child user to device (managed/consumer) and observe status upload after ~60s.
Manually sign in as regular user and observe that status is not uploaded.
Observe that enterprise reporting on managed device is not disrupted.

Change-Id: Ib0ac071f96416e4c2a9ba30ce0c32a782b351bd1
Reviewed-on: https://chromium-review.googlesource.com/1089462
Commit-Queue: Aga Wronska <agawronska@chromium.org>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Reviewed-by: default avatarMaksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569611}
parent cf0137c1
......@@ -504,6 +504,10 @@ source_set("chromeos") {
"certificate_provider/sign_requests.h",
"certificate_provider/thread_safe_certificate_map.cc",
"certificate_provider/thread_safe_certificate_map.h",
"child_accounts/consumer_status_reporting_service.cc",
"child_accounts/consumer_status_reporting_service.h",
"child_accounts/consumer_status_reporting_service_factory.cc",
"child_accounts/consumer_status_reporting_service_factory.h",
"child_accounts/screen_time_controller.cc",
"child_accounts/screen_time_controller.h",
"child_accounts/screen_time_controller_factory.cc",
......
// Copyright 2018 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/child_accounts/consumer_status_reporting_service.h"
#include "base/logging.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/time/time.h"
#include "chrome/browser/chromeos/policy/device_status_collector.h"
#include "chrome/browser/chromeos/policy/status_uploader.h"
#include "chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.h"
#include "chrome/browser/chromeos/policy/user_policy_manager_factory_chromeos.h"
#include "chrome/browser/profiles/profile.h"
#include "chromeos/system/statistics_provider.h"
#include "components/policy/core/common/cloud/cloud_policy_core.h"
#include "components/prefs/pref_service.h"
#include "content/public/browser/browser_context.h"
namespace chromeos {
namespace {
// Default frequency for uploading status reports.
constexpr base::TimeDelta kStatusUploadFrequency =
base::TimeDelta::FromMinutes(10);
} // namespace
ConsumerStatusReportingService::ConsumerStatusReportingService(
content::BrowserContext* context) {
// If child user is registered with DMServer and has User Policy applied, it
// should upload device status to the server.
policy::UserCloudPolicyManagerChromeOS* user_cloud_policy_manager =
policy::UserPolicyManagerFactoryChromeOS::GetCloudPolicyManagerForProfile(
Profile::FromBrowserContext(context));
if (!user_cloud_policy_manager) {
LOG(WARNING) << "Child user is not managed by User Policy - status reports "
"cannot be uploaded to the server. ";
return;
}
PrefService* pref_service = Profile::FromBrowserContext(context)->GetPrefs();
DCHECK(pref_service->GetInitializationStatus() !=
PrefService::INITIALIZATION_STATUS_WAITING);
status_uploader_ = std::make_unique<policy::StatusUploader>(
user_cloud_policy_manager->core()->client(),
std::make_unique<policy::DeviceStatusCollector>(
pref_service, system::StatisticsProvider::GetInstance(),
policy::DeviceStatusCollector::VolumeInfoFetcher(),
policy::DeviceStatusCollector::CPUStatisticsFetcher(),
policy::DeviceStatusCollector::CPUTempFetcher(),
policy::DeviceStatusCollector::AndroidStatusFetcher(),
false /* is_enterprise_reporting */),
base::ThreadTaskRunnerHandle::Get(), kStatusUploadFrequency);
}
ConsumerStatusReportingService::~ConsumerStatusReportingService() = default;
} // namespace chromeos
// Copyright 2018 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_CHILD_ACCOUNTS_CONSUMER_STATUS_REPORTING_SERVICE_H_
#define CHROME_BROWSER_CHROMEOS_CHILD_ACCOUNTS_CONSUMER_STATUS_REPORTING_SERVICE_H_
#include <memory>
#include "base/macros.h"
#include "components/keyed_service/core/keyed_service.h"
namespace content {
class BrowserContext;
}
namespace policy {
class StatusUploader;
}
namespace chromeos {
// Controls consumer reporting for child user.
// Child user should be registered with DMServer and periodically upload the
// information about the device usage. The reports are only sent during user's
// session and they do not interfere with enterprise reporting that is
// controlled by DeviceCloudPolicyManagerChromeOS.
class ConsumerStatusReportingService : public KeyedService {
public:
explicit ConsumerStatusReportingService(content::BrowserContext* context);
~ConsumerStatusReportingService() override;
private:
// Helper object that controls device status collection/storage and uploads
// gathered reports to the server.
std::unique_ptr<policy::StatusUploader> status_uploader_;
DISALLOW_COPY_AND_ASSIGN(ConsumerStatusReportingService);
};
} // namespace chromeos
#endif // CHROME_BROWSER_CHROMEOS_CHILD_ACCOUNTS_CONSUMER_STATUS_REPORTING_SERVICE_H_
// Copyright 2018 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/child_accounts/consumer_status_reporting_service_factory.h"
#include "base/macros.h"
#include "chrome/browser/chromeos/child_accounts/consumer_status_reporting_service.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h"
namespace chromeos {
// static
ConsumerStatusReportingService*
ConsumerStatusReportingServiceFactory::GetForBrowserContext(
content::BrowserContext* context) {
return static_cast<ConsumerStatusReportingService*>(
GetInstance()->GetServiceForBrowserContext(context, true));
}
// static
ConsumerStatusReportingServiceFactory*
ConsumerStatusReportingServiceFactory::GetInstance() {
static base::NoDestructor<ConsumerStatusReportingServiceFactory> factory;
return factory.get();
}
ConsumerStatusReportingServiceFactory::ConsumerStatusReportingServiceFactory()
: BrowserContextKeyedServiceFactory(
"ConsumerStatusReportingServiceFactory",
BrowserContextDependencyManager::GetInstance()) {}
ConsumerStatusReportingServiceFactory::
~ConsumerStatusReportingServiceFactory() = default;
KeyedService* ConsumerStatusReportingServiceFactory::BuildServiceInstanceFor(
content::BrowserContext* context) const {
return new ConsumerStatusReportingService(context);
}
} // namespace chromeos
// Copyright 2018 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_CHILD_ACCOUNTS_CONSUMER_STATUS_REPORTING_SERVICE_FACTORY_H_
#define CHROME_BROWSER_CHROMEOS_CHILD_ACCOUNTS_CONSUMER_STATUS_REPORTING_SERVICE_FACTORY_H_
#include "base/no_destructor.h"
#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
namespace content {
class BrowserContext;
}
namespace chromeos {
class ConsumerStatusReportingService;
// Singleton that owns all ConsumerStatusReportingService objects and associates
// them with corresponding BrowserContexts. Listens for the BrowserContext's
// destruction notification and cleans up the associated
// ConsumerStatusReportingService.
class ConsumerStatusReportingServiceFactory
: public BrowserContextKeyedServiceFactory {
public:
static ConsumerStatusReportingService* GetForBrowserContext(
content::BrowserContext* context);
static ConsumerStatusReportingServiceFactory* GetInstance();
private:
friend class base::NoDestructor<ConsumerStatusReportingServiceFactory>;
ConsumerStatusReportingServiceFactory();
~ConsumerStatusReportingServiceFactory() override;
// BrowserContextKeyedServiceFactory:
KeyedService* BuildServiceInstanceFor(
content::BrowserContext* context) const override;
DISALLOW_COPY_AND_ASSIGN(ConsumerStatusReportingServiceFactory);
};
} // namespace chromeos
#endif // CHROME_BROWSER_CHROMEOS_CHILD_ACCOUNTS_CONSUMER_STATUS_REPORTING_SERVICE_FACTORY_H_
......@@ -17,6 +17,7 @@
#include "chrome/browser/chromeos/app_mode/kiosk_app_manager.h"
#include "chrome/browser/chromeos/arc/arc_service_launcher.h"
#include "chrome/browser/chromeos/boot_times_recorder.h"
#include "chrome/browser/chromeos/child_accounts/consumer_status_reporting_service_factory.h"
#include "chrome/browser/chromeos/child_accounts/screen_time_controller_factory.h"
#include "chrome/browser/chromeos/lock_screen_apps/state_controller.h"
#include "chrome/browser/chromeos/login/demo_mode/demo_session.h"
......@@ -143,8 +144,10 @@ void StartUserSession(Profile* user_profile, const std::string& login_user_id) {
}
arc::ArcServiceLauncher::Get()->OnPrimaryUserProfilePrepared(user_profile);
if (user->GetType() == user_manager::USER_TYPE_CHILD)
if (user->GetType() == user_manager::USER_TYPE_CHILD) {
ScreenTimeControllerFactory::GetForBrowserContext(user_profile);
ConsumerStatusReportingServiceFactory::GetForBrowserContext(user_profile);
}
// Send the PROFILE_PREPARED notification and call SessionStarted()
// so that the Launcher and other Profile dependent classes are created.
......
......@@ -37,6 +37,7 @@
#include "chrome/browser/chromeos/arc/arc_util.h"
#include "chrome/browser/chromeos/base/locale_util.h"
#include "chrome/browser/chromeos/boot_times_recorder.h"
#include "chrome/browser/chromeos/child_accounts/consumer_status_reporting_service_factory.h"
#include "chrome/browser/chromeos/child_accounts/screen_time_controller_factory.h"
#include "chrome/browser/chromeos/first_run/first_run.h"
#include "chrome/browser/chromeos/first_run/goodies_displayer.h"
......@@ -1417,8 +1418,10 @@ void UserSessionManager::FinalizePrepareProfile(Profile* profile) {
if (tether_service)
tether_service->StartTetherIfPossible();
if (user->GetType() == user_manager::USER_TYPE_CHILD)
if (user->GetType() == user_manager::USER_TYPE_CHILD) {
ScreenTimeControllerFactory::GetForBrowserContext(profile);
ConsumerStatusReportingServiceFactory::GetForBrowserContext(profile);
}
}
UpdateEasyUnlockKeys(user_context_);
......
......@@ -779,18 +779,11 @@ DeviceStatusCollector::DeviceStatusCollector(
weak_factory_.GetWeakPtr()));
}
// User |pref_service| might not be initialized yet. This can be removed once
// creation of DeviceStatusCollector for non-enterprise reporting guarantees
// that pref service is ready.
if (pref_service_->GetInitializationStatus() ==
PrefService::INITIALIZATION_STATUS_WAITING) {
pref_service->AddPrefInitObserver(
base::BindOnce(&DeviceStatusCollector::OnPrefServiceInitialized,
weak_factory_.GetWeakPtr()));
return;
}
OnPrefServiceInitialized(pref_service_->GetInitializationStatus() !=
PrefService::INITIALIZATION_STATUS_ERROR);
DCHECK(pref_service_->GetInitializationStatus() !=
PrefService::INITIALIZATION_STATUS_WAITING);
activity_storage_ = std::make_unique<ActivityStorage>(
pref_service_, (is_enterprise_reporting_ ? prefs::kDeviceActivityTimes
: prefs::kUserActivityTimes));
}
DeviceStatusCollector::~DeviceStatusCollector() {
......@@ -893,9 +886,8 @@ void DeviceStatusCollector::ClearCachedResourceUsage() {
}
void DeviceStatusCollector::IdleStateCallback(ui::IdleState state) {
// Do nothing if |activity_storage_| is not ready or device activity reporting
// is disabled.
if (!activity_storage_ || !report_activity_times_)
// Do nothing if device activity reporting is disabled.
if (!report_activity_times_)
return;
Time now = GetCurrentTime();
......@@ -1020,10 +1012,6 @@ void DeviceStatusCollector::ReceiveCPUStatistics(const std::string& stats) {
}
void DeviceStatusCollector::ReportingUsersChanged() {
// Do nothing if |activity_storage_| is not ready.
if (!activity_storage_)
return;
std::vector<std::string> reporting_users;
for (auto& value :
pref_service_->GetList(prefs::kReportingUsers)->GetList()) {
......@@ -1059,25 +1047,8 @@ bool DeviceStatusCollector::IncludeEmailsInActivityReports() const {
return !is_enterprise_reporting_ || report_users_;
}
void DeviceStatusCollector::OnPrefServiceInitialized(bool succeeded) {
if (!succeeded) {
LOG(ERROR) << "Pref service was not initialized successfully - activity "
"for device status reporting cannot be stored.";
return;
}
DCHECK(!activity_storage_);
activity_storage_ = std::make_unique<ActivityStorage>(
pref_service_, (is_enterprise_reporting_ ? prefs::kDeviceActivityTimes
: prefs::kUserActivityTimes));
}
bool DeviceStatusCollector::GetActivityTimes(
em::DeviceStatusReportRequest* status) {
// Do nothing if |activity_storage_| is not ready.
if (!activity_storage_)
return false;
// If user reporting is off, data should be aggregated per day.
// Signed-in user is reported in non-enterprise reporting.
std::vector<ActivityStorage::ActivityPeriod> activity_times =
......@@ -1590,10 +1561,6 @@ std::string DeviceStatusCollector::GetAppVersion(
}
void DeviceStatusCollector::OnSubmittedSuccessfully() {
// Do nothing if |activity_storage_| is not ready.
if (!activity_storage_)
return;
activity_storage_->TrimActivityPeriods(last_reported_day_,
duration_for_last_reported_day_,
std::numeric_limits<int64_t>::max());
......
......@@ -92,8 +92,9 @@ class DeviceStatusCollector {
// Constructor. Callers can inject their own *Fetcher callbacks, e.g. for unit
// testing. A null callback can be passed for any *Fetcher parameter, to use
// the default implementation. These callbacks are always executed on Blocking
// Pool. If |is_enterprise_device| additional enterprise relevant status data
// will be reported.
// Pool. Caller is responsible for passing already initialized |pref_service|.
// If |is_enterprise_device| additional enterprise relevant status data will
// be reported.
DeviceStatusCollector(PrefService* pref_service,
chromeos::system::StatisticsProvider* provider,
const VolumeInfoFetcher& volume_info_fetcher,
......@@ -221,9 +222,6 @@ class DeviceStatusCollector {
// reports.
bool IncludeEmailsInActivityReports() const;
// Called when |pref_service_| is initialized.
void OnPrefServiceInitialized(bool succeeded);
// Pref service that is mainly used to store activity periods for reporting.
PrefService* const pref_service_;
......
......@@ -25,13 +25,10 @@
#include "chrome/browser/chromeos/login/users/chrome_user_manager_impl.h"
#include "chrome/browser/chromeos/policy/app_install_event_log_uploader.h"
#include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
#include "chrome/browser/chromeos/policy/device_status_collector.h"
#include "chrome/browser/chromeos/policy/policy_oauth2_token_fetcher.h"
#include "chrome/browser/chromeos/policy/remote_commands/user_commands_factory_chromeos.h"
#include "chrome/browser/chromeos/policy/status_uploader.h"
#include "chrome/browser/chromeos/policy/user_policy_manager_factory_chromeos.h"
#include "chrome/browser/chromeos/policy/wildcard_login_checker.h"
#include "chrome/browser/chromeos/profiles/profile_helper.h"
#include "chrome/browser/invalidation/profile_invalidation_provider_factory.h"
#include "chrome/browser/lifetime/application_lifetime.h"
#include "chrome/browser/net/system_network_context_manager.h"
......@@ -39,7 +36,6 @@
#include "chrome/browser/profiles/profile.h"
#include "chrome/common/chrome_content_client.h"
#include "chromeos/chromeos_switches.h"
#include "chromeos/system/statistics_provider.h"
#include "components/crash/core/common/crash_key.h"
#include "components/invalidation/impl/profile_invalidation_provider.h"
#include "components/keyed_service/content/browser_context_keyed_service_shutdown_notifier_factory.h"
......@@ -84,10 +80,6 @@ const char kUMAInitialFetchOAuth2Error[] =
const char kUMAInitialFetchOAuth2NetworkError[] =
"Enterprise.UserPolicyChromeOS.InitialFetch.OAuth2NetworkError";
// Default frequency for uploading non-enterprise status reports.
constexpr base::TimeDelta kDeviceStatusUploadFrequency =
base::TimeDelta::FromMinutes(10);
// This class is used to subscribe for notifications that the current profile is
// being shut down.
class UserCloudPolicyManagerChromeOSNotifierFactory
......@@ -139,7 +131,6 @@ UserCloudPolicyManagerChromeOS::UserCloudPolicyManagerChromeOS(
PolicyEnforcement::kServerCheckRequired ||
!policy_refresh_timeout.is_zero()),
enforcement_type_(enforcement_type),
task_runner_(task_runner),
account_id_(account_id),
fatal_error_callback_(std::move(fatal_error_callback)) {
DCHECK(profile_);
......@@ -327,7 +318,6 @@ void UserCloudPolicyManagerChromeOS::Shutdown() {
if (service())
service()->RemoveObserver(this);
token_fetcher_.reset();
status_uploader_.reset();
external_data_manager_->Disconnect();
CloudPolicyManager::Shutdown();
}
......@@ -415,22 +405,6 @@ void UserCloudPolicyManagerChromeOS::OnRegistrationStateChanged(
CancelWaitForPolicyFetch(true);
}
}
// TODO(agawronska): Move StatusUploader creation to profile keyed
// service, where profile pref service is fully initialized.
// If child is registered with DMServer and has User Policy applied, it
// should upload device status to the server. For enterprise reporting
// status upload is controlled by DeviceCloudPolicyManagerChromeOS.
const user_manager::User* const user =
chromeos::ProfileHelper::Get()->GetUserByProfile(profile_);
if (client()->is_registered() && user &&
user->GetType() == user_manager::USER_TYPE_CHILD) {
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
base::BindOnce(&UserCloudPolicyManagerChromeOS::CreateStatusUploader,
base::Unretained(this)));
}
}
void UserCloudPolicyManagerChromeOS::OnClientError(
......@@ -727,22 +701,4 @@ void UserCloudPolicyManagerChromeOS::ProfileShutdown() {
shutdown_notifier_.reset();
}
void UserCloudPolicyManagerChromeOS::CreateStatusUploader() {
// Do not recreate status uploader if this is called multiple times.
if (status_uploader_)
return;
status_uploader_ = std::make_unique<StatusUploader>(
client(),
std::make_unique<DeviceStatusCollector>(
profile_->GetPrefs(),
chromeos::system::StatisticsProvider::GetInstance(),
DeviceStatusCollector::VolumeInfoFetcher(),
DeviceStatusCollector::CPUStatisticsFetcher(),
DeviceStatusCollector::CPUTempFetcher(),
DeviceStatusCollector::AndroidStatusFetcher(),
false /* is_enterprise_device */),
task_runner_, kDeviceStatusUploadFrequency);
}
} // namespace policy
......@@ -50,7 +50,6 @@ class CloudExternalDataManager;
class DeviceManagementService;
class PolicyOAuth2TokenFetcher;
class RemoteCommandsInvalidator;
class StatusUploader;
// Implements logic for initializing user policy on Chrome OS.
class UserCloudPolicyManagerChromeOS : public CloudPolicyManager,
......@@ -164,10 +163,6 @@ class UserCloudPolicyManagerChromeOS : public CloudPolicyManager,
// Helper function to force a policy fetch timeout.
void ForceTimeoutForTest();
// Return the StatusUploader used to communicate consumer device status to the
// policy server.
StatusUploader* GetStatusUploader() const { return status_uploader_.get(); }
// Sets a SharedURLLoaderFactory that should be used for tests instead of
// retrieving one from the BrowserProcess object in FetchPolicyOAuthToken().
void SetSystemURLLoaderFactoryForTests(
......@@ -225,8 +220,6 @@ class UserCloudPolicyManagerChromeOS : public CloudPolicyManager,
// Observer called on profile shutdown.
void ProfileShutdown();
void CreateStatusUploader();
// Profile associated with the current user.
Profile* const profile_;
......@@ -267,12 +260,6 @@ class UserCloudPolicyManagerChromeOS : public CloudPolicyManager,
// Keeps alive the wildcard checker while its running.
std::unique_ptr<WildcardLoginChecker> wildcard_login_checker_;
// Task runner used for non-enterprise device status upload.
scoped_refptr<base::SequencedTaskRunner> task_runner_;
// Helper object for updating the server with consumer device state.
std::unique_ptr<StatusUploader> status_uploader_;
// The access token passed to OnAccessTokenAvailable. It is stored here so
// that it can be used if OnInitializationCompleted is called later.
std::string access_token_;
......
......@@ -845,36 +845,4 @@ TEST_F(UserCloudPolicyManagerChromeOSTest, TestHasAppInstallEventLogUploader) {
EXPECT_TRUE(manager_->GetAppInstallEventLogUploader());
}
class ConsumerDeviceStatusUploadingTest
: public UserCloudPolicyManagerChromeOSTest {
protected:
ConsumerDeviceStatusUploadingTest() = default;
~ConsumerDeviceStatusUploadingTest() override = default;
private:
DISALLOW_COPY_AND_ASSIGN(ConsumerDeviceStatusUploadingTest);
};
TEST_F(ConsumerDeviceStatusUploadingTest, RegularAccountShouldNotUploadStatus) {
ASSERT_NO_FATAL_FAILURE(MakeManagerWithPreloadedStore(base::TimeDelta()));
manager_->OnRegistrationStateChanged(manager_->core()->client());
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(manager_->core()->client()->is_registered());
EXPECT_FALSE(manager_->GetStatusUploader());
}
TEST_F(ConsumerDeviceStatusUploadingTest, ChildAccountShouldUploadStatus) {
AddAndSwitchToChildAccountWithProfile();
ASSERT_NO_FATAL_FAILURE(MakeManagerWithPreloadedStore(base::TimeDelta()));
manager_->OnRegistrationStateChanged(manager_->core()->client());
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(manager_->core()->client()->is_registered());
EXPECT_TRUE(manager_->GetStatusUploader());
}
} // namespace policy
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