Reland: Add user to active sessions reporting.

Previously, active sessions and recent users were reported separately,
that is confusing on the server side.
This change adds 'user_email' field to ActiveTimePeriod proto message
and populates it for affiliated users that are eligible for reporting.
Also, user email is reported only if user reporting is turned on.

Initially landed in https://crrev.com/c/904527
Reverted in https://crrev.com/c/923228

TBR=xiyuan@chromium.org

BUG=810093
TEST=Browser tests added.
TEST=ASAN_OPTIONS="detect_leaks=1" ./out/asan/browser_tests --gtest_filter=DeviceStatusCollectorTest*
TEST=ASAN_OPTIONS="detect_leaks=1" ./out/asan/browser_tests --gtest_filter=Blub*

Change-Id: I9db2c07f6aa0ba89b9f5fc6fc890207ad5e9a97b
Reviewed-on: https://chromium-review.googlesource.com/924005
Commit-Queue: Sergey Poromov <poromov@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537641}

chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc

@@ -109,9 +109,6 @@ const char kRegularUsers[] = "LoggedInUsers";
 // A vector pref of the device local accounts defined on this device.
 const char kDeviceLocalAccounts[] = "PublicAccounts";
 
-// Key for list of users that should be reported.
-const char kReportingUsers[] = "reporting_users";
-
 // A string pref that gets set when a device local account is removed but a
 // user is currently logged into that account, requiring the account's data to
 // be removed after logout.
@@ -195,7 +192,7 @@ void ChromeUserManagerImpl::RegisterPrefs(PrefRegistrySimple* registry) {
   registry->RegisterListPref(kDeviceLocalAccounts);
   registry->RegisterStringPref(kDeviceLocalAccountPendingDataRemoval,
                                std::string());
-  registry->RegisterListPref(kReportingUsers);
+  registry->RegisterListPref(prefs::kReportingUsers);
 
   SupervisedUserManager::RegisterPrefs(registry);
   SessionLengthLimiter::RegisterPrefs(registry);
@@ -1332,19 +1329,19 @@ void ChromeUserManagerImpl::SetUserAffiliation(
 
 bool ChromeUserManagerImpl::ShouldReportUser(const std::string& user_id) const {
   const base::ListValue& reporting_users =
-      *(GetLocalState()->GetList(kReportingUsers));
+      *(GetLocalState()->GetList(prefs::kReportingUsers));
   base::Value user_id_value(FullyCanonicalize(user_id));
   return !(reporting_users.Find(user_id_value) == reporting_users.end());
 }
 
 void ChromeUserManagerImpl::AddReportingUser(const AccountId& account_id) {
-  ListPrefUpdate users_update(GetLocalState(), kReportingUsers);
+  ListPrefUpdate users_update(GetLocalState(), prefs::kReportingUsers);
   users_update->AppendIfNotPresent(
       std::make_unique<base::Value>(account_id.GetUserEmail()));
 }
 
 void ChromeUserManagerImpl::RemoveReportingUser(const AccountId& account_id) {
-  ListPrefUpdate users_update(GetLocalState(), kReportingUsers);
+  ListPrefUpdate users_update(GetLocalState(), prefs::kReportingUsers);
   users_update->Remove(
       base::Value(FullyCanonicalize(account_id.GetUserEmail())), NULL);
 }

chrome/browser/chromeos/policy/device_status_collector.cc

@@ -12,6 +12,7 @@
 #include <limits>
 #include <sstream>
 
+#include "base/base64.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/files/file_enumerator.h"
@@ -58,6 +59,7 @@
 #include "components/arc/common/enterprise_reporting.mojom.h"
 #include "components/policy/core/common/cloud/cloud_policy_constants.h"
 #include "components/policy/proto/device_management_backend.pb.h"
+#include "components/prefs/pref_change_registrar.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 #include "components/prefs/scoped_user_pref_update.h"
@@ -97,6 +99,10 @@ const char kDeviceDir[] = "device";
 const char kHwmonDirectoryPattern[] = "hwmon*";
 const char kCPUTempFilePattern[] = "temp*_input";
 
+// Activity periods are keyed with day and user in format:
+// '<day_timestamp>:<BASE64 encoded user email>'
+constexpr char kActivityKeySeparator = ':';
+
 // Determine the day key (milliseconds since epoch for corresponding day in UTC)
 // for a given |timestamp|.
 int64_t TimestampToDayKey(Time timestamp) {
@@ -306,6 +312,56 @@ bool IsKioskApp() {
          user_type == chromeos::LoginState::LOGGED_IN_USER_ARC_KIOSK_APP;
 }
 
+std::string MakeActivityTimesPrefKey(int64_t start,
+                                     const std::string& user_email) {
+  const std::string day_key = base::Int64ToString(start);
+  if (user_email.empty())
+    return day_key;
+
+  std::string encoded_email;
+  base::Base64Encode(user_email, &encoded_email);
+  return day_key + kActivityKeySeparator + encoded_email;
+}
+
+bool ParseActivityTimesPrefKey(const std::string& key,
+                               int64_t* start_timestamp,
+                               std::string* user_email) {
+  auto separator_pos = key.find(kActivityKeySeparator);
+  if (separator_pos == std::string::npos) {
+    user_email->clear();
+    return base::StringToInt64(key, start_timestamp);
+  }
+  return base::StringToInt64(key.substr(0, separator_pos), start_timestamp) &&
+         base::Base64Decode(key.substr(separator_pos + 1), user_email);
+}
+
+void FilterActivityTimesByUsers(const base::DictionaryValue& activity_times,
+                                const std::vector<std::string>& reporting_users,
+                                base::DictionaryValue* const filtered_times) {
+  std::set<std::string> reporting_users_set(reporting_users.begin(),
+                                            reporting_users.end());
+  const std::string empty;
+  for (const auto& it : activity_times.DictItems()) {
+    DCHECK(it.second.is_int());
+    int64_t timestamp;
+    std::string user_email;
+    if (!ParseActivityTimesPrefKey(it.first, &timestamp, &user_email))
+      continue;
+    if (!user_email.empty() && reporting_users_set.count(user_email) == 0) {
+      int value = 0;
+      std::string timestamp_str = MakeActivityTimesPrefKey(timestamp, empty);
+      const base::Value* prev_value = filtered_times->FindKeyOfType(
+          timestamp_str, base::Value::Type::INTEGER);
+      if (prev_value)
+        value = prev_value->GetInt();
+      filtered_times->SetKey(timestamp_str,
+                             base::Value(value + it.second.GetInt()));
+    } else {
+      filtered_times->SetKey(it.first, it.second.Clone());
+    }
+  }
+}
+
 }  // namespace
 
 namespace policy {
@@ -513,6 +569,12 @@ DeviceStatusCollector::DeviceStatusCollector(
   chromeos::version_loader::GetTpmVersion(
       base::BindOnce(&DeviceStatusCollector::OnTpmVersion,
                      weak_factory_.GetWeakPtr()));
+  pref_change_registrar_ = std::make_unique<PrefChangeRegistrar>();
+  pref_change_registrar_->Init(local_state_);
+  pref_change_registrar_->Add(
+      prefs::kReportingUsers,
+      base::BindRepeating(&DeviceStatusCollector::ReportingUsersChanged,
+                          weak_factory_.GetWeakPtr()));
 }
 
 DeviceStatusCollector::~DeviceStatusCollector() {
@@ -623,7 +685,8 @@ void DeviceStatusCollector::TrimStoredActivityPeriods(int64_t min_day_key,
   for (base::DictionaryValue::Iterator it(*activity_times); !it.IsAtEnd();
        it.Advance()) {
     int64_t timestamp;
-    if (base::StringToInt64(it.key(), &timestamp)) {
+    std::string active_user_email;
+    if (ParseActivityTimesPrefKey(it.key(), &timestamp, &active_user_email)) {
       // Remove data that is too old, or too far in the future.
       if (timestamp >= min_day_key && timestamp < max_day_key) {
         if (timestamp == min_day_key) {
@@ -643,7 +706,10 @@ void DeviceStatusCollector::TrimStoredActivityPeriods(int64_t min_day_key,
   local_state_->Set(prefs::kDeviceActivityTimes, *copy);
 }
 
-void DeviceStatusCollector::AddActivePeriod(Time start, Time end) {
+void DeviceStatusCollector::AddActivePeriod(
+    Time start,
+    Time end,
+    const std::string& active_user_email) {
   DCHECK(start < end);
 
   // Maintain the list of active periods in a local_state pref.
@@ -655,10 +721,11 @@ void DeviceStatusCollector::AddActivePeriod(Time start, Time end) {
   while (midnight < end) {
     midnight += TimeDelta::FromDays(1);
     int64_t activity = (std::min(end, midnight) - start).InMilliseconds();
-    std::string day_key = base::Int64ToString(TimestampToDayKey(start));
+    const std::string key =
+        MakeActivityTimesPrefKey(TimestampToDayKey(start), active_user_email);
     int previous_activity = 0;
-    activity_times->GetInteger(day_key, &previous_activity);
-    activity_times->SetInteger(day_key, previous_activity + activity);
+    activity_times->GetInteger(key, &previous_activity);
+    activity_times->SetInteger(key, previous_activity + activity);
     start = midnight;
   }
 }
@@ -678,6 +745,17 @@ void DeviceStatusCollector::IdleStateCallback(ui::IdleState state) {
 
   // For kiosk apps we report total uptime instead of active time.
   if (state == ui::IDLE_STATE_ACTIVE || IsKioskApp()) {
+    // Report only affiliated users.
+    // Primary user is used as unique identifier of a single session,
+    // even for multi-user sessions.
+    std::string primary_user_email;
+    const user_manager::User* const primary_user =
+        user_manager::UserManager::Get()->GetPrimaryUser();
+    if (primary_user && primary_user->HasGaiaAccount() &&
+        chromeos::ChromeUserManager::Get()->ShouldReportUser(
+            primary_user->GetAccountId().GetUserEmail())) {
+      primary_user_email = primary_user->GetAccountId().GetUserEmail();
+    }
     // If it's been too long since the last report, or if the activity is
     // negative (which can happen when the clock changes), assume a single
     // interval of activity.
@@ -685,9 +763,9 @@ void DeviceStatusCollector::IdleStateCallback(ui::IdleState state) {
     if (active_seconds < 0 ||
         active_seconds >= static_cast<int>((2 * kIdlePollIntervalSeconds))) {
       AddActivePeriod(now - TimeDelta::FromSeconds(kIdlePollIntervalSeconds),
-                      now);
+                      now, primary_user_email);
     } else {
-      AddActivePeriod(last_idle_check_, now);
+      AddActivePeriod(last_idle_check_, now, primary_user_email);
     }
 
     PruneStoredActivityPeriods(now);
@@ -787,17 +865,44 @@ void DeviceStatusCollector::ReceiveCPUStatistics(const std::string& stats) {
     resource_usage_.pop_front();
 }
 
+void DeviceStatusCollector::ReportingUsersChanged() {
+  std::vector<std::string> reporting_users;
+  for (auto& value : local_state_->GetList(prefs::kReportingUsers)->GetList()) {
+    if (value.is_string())
+      reporting_users.push_back(value.GetString());
+  }
+
+  const base::DictionaryValue* activity_times =
+      local_state_->GetDictionary(prefs::kDeviceActivityTimes);
+  base::DictionaryValue filtered_activity_times;
+  FilterActivityTimesByUsers(*activity_times, reporting_users,
+                             &filtered_activity_times);
+
+  local_state_->Set(prefs::kDeviceActivityTimes, filtered_activity_times);
+}
+
 bool DeviceStatusCollector::GetActivityTimes(
     em::DeviceStatusReportRequest* status) {
   DictionaryPrefUpdate update(local_state_, prefs::kDeviceActivityTimes);
   base::DictionaryValue* activity_times = update.Get();
 
+  // If user reporting is off, data should be aggregated per day.
+  base::DictionaryValue filtered_activity_times;
+  if (!report_users_) {
+    std::vector<std::string> empty_user_list;
+    FilterActivityTimesByUsers(*activity_times, empty_user_list,
+                               &filtered_activity_times);
+    activity_times = &filtered_activity_times;
+  }
+
   bool anything_reported = false;
   for (base::DictionaryValue::Iterator it(*activity_times); !it.IsAtEnd();
        it.Advance()) {
     int64_t start_timestamp;
+    std::string active_user_email;
     int activity_milliseconds;
-    if (base::StringToInt64(it.key(), &start_timestamp) &&
+    if (ParseActivityTimesPrefKey(it.key(), &start_timestamp,
+                                  &active_user_email) &&
         it.value().GetAsInteger(&activity_milliseconds)) {
       // This is correct even when there are leap seconds, because when a leap
       // second occurs, two consecutive seconds have the same timestamp.
@@ -808,6 +913,9 @@ bool DeviceStatusCollector::GetActivityTimes(
       period->set_start_timestamp(start_timestamp);
       period->set_end_timestamp(end_timestamp);
       active_period->set_active_duration(activity_milliseconds);
+      // Report user email only if users reporting is turned on.
+      if (!active_user_email.empty())
+        active_period->set_user_email(active_user_email);
       if (start_timestamp >= last_reported_day_) {
         last_reported_day_ = start_timestamp;
         duration_for_last_reported_day_ = activity_milliseconds;

chrome/browser/chromeos/policy/device_status_collector.h

@@ -41,6 +41,7 @@ namespace user_manager {
 class User;
 }
 
+class PrefChangeRegistrar;
 class PrefRegistrySimple;
 class PrefService;
 class Profile;
@@ -167,7 +168,9 @@ class DeviceStatusCollector {
                                  int min_day_trim_duration,
                                  int64_t max_day_key);
 
-  void AddActivePeriod(base::Time start, base::Time end);
+  void AddActivePeriod(base::Time start,
+                       base::Time end,
+                       const std::string& active_user_email);
 
   // Clears the cached hardware resource usage.
   void ClearCachedResourceUsage();
@@ -218,6 +221,9 @@ class DeviceStatusCollector {
   // Callback invoked to update our cpu usage information.
   void ReceiveCPUStatistics(const std::string& statistics);
 
+  // Callback invoked when reporting users pref is changed.
+  void ReportingUsersChanged();
+
   PrefService* const local_state_;
 
   // The last time an idle state check was performed.
@@ -299,6 +305,8 @@ class DeviceStatusCollector {
   std::unique_ptr<chromeos::CrosSettings::ObserverSubscription>
       running_kiosk_app_subscription_;
 
+  std::unique_ptr<PrefChangeRegistrar> pref_change_registrar_;
+
   // Task runner in the creation thread where responses are sent to.
   scoped_refptr<base::SequencedTaskRunner> task_runner_;
   base::ThreadChecker thread_checker_;

chrome/browser/chromeos/policy/device_status_collector_browsertest.cc

@@ -544,12 +544,12 @@ class DeviceStatusCollectorTest : public testing::Test {
   em::DeviceStatusReportRequest device_status_;
   em::SessionStatusReportRequest session_status_;
   bool got_session_status_;
+  TestingPrefServiceSimple local_state_;
   std::unique_ptr<TestingDeviceStatusCollector> status_collector_;
   const policy::DeviceLocalAccount fake_kiosk_device_local_account_;
   const policy::ArcKioskAppBasicInfo fake_arc_kiosk_app_basic_info_;
   const policy::DeviceLocalAccount fake_arc_kiosk_device_local_account_;
   base::ScopedPathOverride user_data_dir_override_;
-  TestingPrefServiceSimple local_state_;
   chromeos::FakeUpdateEngineClient* const update_engine_client_;
   std::unique_ptr<base::RunLoop> run_loop_;
 };
@@ -835,6 +835,78 @@ TEST_F(DeviceStatusCollectorTest, ActivityTimesKeptUntilSubmittedSuccessfully) {
   EXPECT_EQ(ActivePeriodMilliseconds(), GetActiveMilliseconds(device_status_));
 }
 
+TEST_F(DeviceStatusCollectorTest, ActivityNoUser) {
+  ui::IdleState test_states[] = {ui::IDLE_STATE_ACTIVE, ui::IDLE_STATE_ACTIVE,
+                                 ui::IDLE_STATE_ACTIVE};
+  settings_helper_.SetBoolean(chromeos::kReportDeviceActivityTimes, true);
+  settings_helper_.SetBoolean(chromeos::kReportDeviceUsers, true);
+
+  status_collector_->Simulate(test_states, 3);
+  GetStatus();
+  EXPECT_EQ(1, device_status_.active_period_size());
+  EXPECT_TRUE(device_status_.active_period(0).user_email().empty());
+}
+
+TEST_F(DeviceStatusCollectorTest, ActivityWithPublicSessionUser) {
+  ui::IdleState test_states[] = {ui::IDLE_STATE_ACTIVE, ui::IDLE_STATE_ACTIVE,
+                                 ui::IDLE_STATE_ACTIVE};
+  settings_helper_.SetBoolean(chromeos::kReportDeviceActivityTimes, true);
+  settings_helper_.SetBoolean(chromeos::kReportDeviceUsers, true);
+  const AccountId public_account_id(
+      AccountId::FromUserEmail("public@localhost"));
+  user_manager_->CreatePublicAccountUser(public_account_id);
+
+  status_collector_->Simulate(test_states, 3);
+  GetStatus();
+  EXPECT_EQ(1, device_status_.active_period_size());
+  EXPECT_TRUE(device_status_.active_period(0).user_email().empty());
+}
+
+TEST_F(DeviceStatusCollectorTest, ActivityWithAffiliatedUser) {
+  ui::IdleState test_states[] = {ui::IDLE_STATE_ACTIVE, ui::IDLE_STATE_ACTIVE,
+                                 ui::IDLE_STATE_ACTIVE};
+  settings_helper_.SetBoolean(chromeos::kReportDeviceActivityTimes, true);
+  settings_helper_.SetBoolean(chromeos::kReportDeviceUsers, true);
+  const AccountId account_id0(AccountId::FromUserEmail("user0@managed.com"));
+  user_manager_->AddUserWithAffiliation(account_id0, true);
+
+  status_collector_->Simulate(test_states, 3);
+  GetStatus();
+  EXPECT_EQ(1, device_status_.active_period_size());
+  EXPECT_EQ(account_id0.GetUserEmail(),
+            device_status_.active_period(0).user_email());
+  device_status_.clear_active_period();  // Clear the result protobuf.
+
+  settings_helper_.SetBoolean(chromeos::kReportDeviceUsers, false);
+
+  status_collector_->Simulate(test_states, 3);
+  GetStatus();
+  EXPECT_EQ(1, device_status_.active_period_size());
+  EXPECT_TRUE(device_status_.active_period(0).user_email().empty());
+}
+
+TEST_F(DeviceStatusCollectorTest, ActivityWithNotAffiliatedUser) {
+  ui::IdleState test_states[] = {ui::IDLE_STATE_ACTIVE, ui::IDLE_STATE_ACTIVE,
+                                 ui::IDLE_STATE_ACTIVE};
+  settings_helper_.SetBoolean(chromeos::kReportDeviceActivityTimes, true);
+  settings_helper_.SetBoolean(chromeos::kReportDeviceUsers, true);
+  const AccountId account_id0(AccountId::FromUserEmail("user0@managed.com"));
+  user_manager_->AddUserWithAffiliation(account_id0, false);
+
+  status_collector_->Simulate(test_states, 3);
+  GetStatus();
+  EXPECT_EQ(1, device_status_.active_period_size());
+  EXPECT_TRUE(device_status_.active_period(0).user_email().empty());
+  device_status_.clear_active_period();  // Clear the result protobuf.
+
+  settings_helper_.SetBoolean(chromeos::kReportDeviceUsers, false);
+
+  status_collector_->Simulate(test_states, 3);
+  GetStatus();
+  EXPECT_EQ(1, device_status_.active_period_size());
+  EXPECT_TRUE(device_status_.active_period(0).user_email().empty());
+}
+
 TEST_F(DeviceStatusCollectorTest, DevSwitchBootMode) {
   // Test that boot mode data is reported by default.
   fake_statistics_provider_.SetMachineStatistic(

chrome/common/pref_names.cc

@@ -1855,6 +1855,9 @@ const char kPowerMetricsIdleSuspendCount[] = "power.metrics.idle_suspend_count";
 const char kPowerMetricsLidClosedSuspendCount[] =
     "power.metrics.lid_closed_suspend_count";
 
+// Key for list of users that should be reported.
+const char kReportingUsers[] = "reporting_users";
+
 #endif  // defined(OS_CHROMEOS)
 
 // Whether there is a Flash version installed that supports clearing LSO data.

chrome/common/pref_names.h

@@ -649,6 +649,7 @@ extern const char kPowerMetricsIdleScreenDimCount[];
 extern const char kPowerMetricsIdleScreenOffCount[];
 extern const char kPowerMetricsIdleSuspendCount[];
 extern const char kPowerMetricsLidClosedSuspendCount[];
+extern const char kReportingUsers[];
 #endif  // defined(OS_CHROMEOS)
 
 extern const char kClearPluginLSODataEnabled[];

components/policy/proto/device_management_backend.proto

@@ -620,6 +620,10 @@ message ActiveTimePeriod {
   // The active duration during the above time period.
   // The unit is milli-second.
   optional int32 active_duration = 2;
+
+  // Email address of the active user. Present only if the user type is managed
+  // and affiliated.
+  optional string user_email = 3;
 }
 
 // Details about a network interface.
@@ -740,7 +744,7 @@ message DeviceStatusReportRequest {
   // e.g. 17.0.963.18.
   optional string browser_version = 5;
 
-  // A list of periods when the device was active, aggregated by day.
+  // A list of periods when the device was active, aggregated by day by user.
   repeated ActiveTimePeriod active_period = 6;
 
   // List of network interfaces.