Show last day notification on user sign in

As per product requirements of minimum chrome version policy, it is
required to show notifications (for update required in case of network
limitations and to return the device in case of end-of-life) on login
if it is the last day to reach the deadline. Like other notifications
of this policy, this notification on sign in is also only applicable
to managed users to maintain consistency.

This behaviour helps to curb the problem of notification being missed
if policy is received when un-managed user is signed in and later
a managed user signs in. Only the last day notification is shown on
managed user sign in to avoid too much spamming.


Bug: 1048607
Change-Id: I01a8d52bbc4a003d5e1323140885881887211176
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2248567Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
Commit-Queue: Saurabh Nijhara <snijhara@google.com>
Cr-Commit-Position: refs/heads/master@{#779770}

chrome/browser/chromeos/login/session/user_session_manager.cc

@@ -71,6 +71,7 @@
 #include "chrome/browser/chromeos/login/users/supervised_user_manager.h"
 #include "chrome/browser/chromeos/login/wizard_controller.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
+#include "chrome/browser/chromeos/policy/minimum_version_policy_handler.h"
 #include "chrome/browser/chromeos/policy/tpm_auto_update_mode_policy_handler.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
@@ -386,6 +387,12 @@ bool IsNewProfile(Profile* profile) {
          profile->IsNewProfile();
 }
 
+policy::MinimumVersionPolicyHandler* GetMinimumVersionPolicyHandler() {
+  return g_browser_process->platform_part()
+      ->browser_policy_connector_chromeos()
+      ->GetMinimumVersionPolicyHandler();
+}
+
 }  // namespace
 
 UserSessionManagerDelegate::~UserSessionManagerDelegate() {}
@@ -1840,6 +1847,8 @@ void UserSessionManager::ShowNotificationsIfNeeded(Profile* profile) {
       ->browser_policy_connector_chromeos()
       ->GetTPMAutoUpdateModePolicyHandler()
       ->ShowTPMAutoUpdateNotificationIfNeeded();
+
+  GetMinimumVersionPolicyHandler()->MaybeShowNotificationOnLogin();
 }
 
 void UserSessionManager::MaybeLaunchSettings(Profile* profile) {

chrome/browser/chromeos/policy/minimum_version_policy_handler.cc

@@ -67,6 +67,11 @@ BuildState* GetBuildState() {
   return g_browser_process->GetBuildState();
 }
 
+int GetDaysRounded(base::TimeDelta time) {
+  return std::lround(time.InSecondsF() /
+                     base::TimeDelta::FromDays(1).InSecondsF());
+}
+
 }  // namespace
 
 const char MinimumVersionPolicyHandler::kChromeVersion[] = "chrome_version";
@@ -217,7 +222,6 @@ void MinimumVersionPolicyHandler::OnPolicyChanged() {
         std::move(configs[strongest_config_idx]);
     if (!state_ || state_->Compare(strongest_config.get()) != 0) {
       state_ = std::move(strongest_config);
-      requirements_met_ = false;
       FetchEolInfo();
     }
   } else {
@@ -240,9 +244,9 @@ void MinimumVersionPolicyHandler::HandleUpdateNotRequired() {
 }
 
 void MinimumVersionPolicyHandler::Reset() {
-  requirements_met_ = true;
   deadline_reached = false;
   eol_reached_ = false;
+  update_required_deadline_ = base::Time();
   update_required_deadline_timer_.Stop();
   notification_timer_.Stop();
   GetBuildState()->RemoveObserver(this);
@@ -302,15 +306,15 @@ void MinimumVersionPolicyHandler::HandleUpdateRequired(
   // If update is already required, use the existing timer start time to
   // calculate the new deadline. Else use |update_required_time_|. Do not reduce
   // the warning time if policy is already applied.
-  base::Time deadline;
   if (stored_timer_start_time.is_null()) {
-    deadline = update_required_time_ + warning_time;
+    update_required_deadline_ = update_required_time_ + warning_time;
   } else {
-    deadline =
+    update_required_deadline_ =
         stored_timer_start_time + std::max(stored_warning_time, warning_time);
   }
 
-  const bool deadline_reached = deadline <= update_required_time_;
+  const bool deadline_reached =
+      update_required_deadline_ <= update_required_time_;
   if (deadline_reached) {
     // As per the policy, the deadline for the user cannot reduce.
     // This case can be encountered when :-
@@ -323,7 +327,7 @@ void MinimumVersionPolicyHandler::HandleUpdateRequired(
 
   // Need to start the timer even if the deadline is same as the previous one to
   // handle the case of Chrome reboot.
-  if (deadline == previous_deadline &&
+  if (update_required_deadline_ == previous_deadline &&
       update_required_deadline_timer_.IsRunning()) {
     return;
   }
@@ -340,7 +344,7 @@ void MinimumVersionPolicyHandler::HandleUpdateRequired(
   // The |deadline| can only be equal to or greater than the
   // |previous_deadline|. No need to update the local state if the deadline has
   // not been extended.
-  if (deadline > previous_deadline)
+  if (update_required_deadline_ > previous_deadline)
     UpdateLocalState(warning_time);
 
   // The device has already downloaded the update in-session and waiting for
@@ -351,10 +355,10 @@ void MinimumVersionPolicyHandler::HandleUpdateRequired(
     return;
   }
 
-  StartDeadlineTimer(deadline);
+  StartDeadlineTimer(update_required_deadline_);
   if (!eol_reached_)
     StartObservingUpdate();
-  ShowAndScheduleNotification(deadline);
+  ShowAndScheduleNotification(update_required_deadline_);
 }
 
 void MinimumVersionPolicyHandler::ResetLocalState() {
@@ -390,13 +394,25 @@ void MinimumVersionPolicyHandler::StartObservingUpdate() {
     build_state->AddObserver(this);
 }
 
+void MinimumVersionPolicyHandler::MaybeShowNotificationOnLogin() {
+  const base::Time now = clock_->Now();
+  // This should only be true if |update_required_deadline_timer_| expired while
+  // login was in progress, else we would have shown the update required screen
+  // at startup.
+  if (update_required_deadline_ <= now)
+    return;
+
+  base::TimeDelta time_remaining = update_required_deadline_ - now;
+  int days_remaining = GetDaysRounded(time_remaining);
+  if (days_remaining <= 1)
+    MaybeShowNotification(base::TimeDelta::FromDays(days_remaining));
+}
+
 void MinimumVersionPolicyHandler::MaybeShowNotification(
     base::TimeDelta warning) {
   const NetworkStatus status = GetCurrentNetworkStatus();
   if (status == NetworkStatus::kAllowed || !delegate_->IsUserLoggedIn() ||
       !delegate_->IsUserManaged()) {
-    // TODO(https://crbug.com/1048607): Show notification on managed user log in
-    // if it is the last day of the deadline.
     return;
   }
 
@@ -429,8 +445,7 @@ void MinimumVersionPolicyHandler::ShowAndScheduleNotification(
 
   base::Time expiry;
   base::TimeDelta time_remaining = deadline - now;
-  const int days_remaining = std::lround(
-      time_remaining.InSecondsF() / base::TimeDelta::FromDays(1).InSecondsF());
+  int days_remaining = GetDaysRounded(time_remaining);
 
   // Network limitation notifications are shown when policy is received and on
   // the last day. End of life notifications are shown when policy is received,

chrome/browser/chromeos/policy/minimum_version_policy_handler.h

@@ -135,7 +135,7 @@ class MinimumVersionPolicyHandler
 
   void AddObserver(Observer* observer);
   void RemoveObserver(Observer* observer);
-  bool RequirementsAreSatisfied() const { return requirements_met_; }
+  bool RequirementsAreSatisfied() const { return GetState() == nullptr; }
 
   // Returns |true| if the current version satisfies the given requirement.
   bool CurrentVersionSatisfies(
@@ -147,6 +147,9 @@ class MinimumVersionPolicyHandler
 
   static void RegisterPrefs(PrefRegistrySimple* registry);
 
+  // Show notification on managed user login if it is the last day to deadline.
+  void MaybeShowNotificationOnLogin();
+
   // Callback used in tests and invoked after end-of-life status has been
   // fetched from the update_engine.
   void set_fetch_eol_callback_for_testing(base::OnceClosure callback) {
@@ -222,7 +225,6 @@ class MinimumVersionPolicyHandler
   // version in all the configurations.
   std::unique_ptr<MinimumVersionRequirement> state_;
 
-  bool requirements_met_ = true;
   bool eol_reached_ = false;
 
   // If this flag is true, user should restricted to use the session by logging
@@ -231,6 +233,8 @@ class MinimumVersionPolicyHandler
 
   base::Time update_required_time_;
 
+  base::Time update_required_deadline_;
+
   // Fires when the deadline to update the device has reached or passed.
   util::WallClockTimer update_required_deadline_timer_;
 

chrome/browser/chromeos/policy/minimum_version_policy_handler_browsertest.cc

@@ -19,6 +19,7 @@
 #include "chrome/browser/chromeos/login/test/login_manager_mixin.h"
 #include "chrome/browser/chromeos/login/test/oobe_screen_exit_waiter.h"
 #include "chrome/browser/chromeos/login/test/oobe_screen_waiter.h"
+#include "chrome/browser/chromeos/login/test/user_policy_mixin.h"
 #include "chrome/browser/chromeos/login/ui/login_display_host.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
 #include "chrome/browser/chromeos/policy/device_policy_cros_browser_test.h"
@@ -55,6 +56,7 @@ namespace policy {
 namespace {
 const char kNewVersion[] = "99999.4.2";
 const int kNoWarning = 0;
+const int kLastDayWarningInDays = 1;
 const int kShortWarningInDays = 2;
 const int kLongWarningInDays = 10;
 const int kVeryLongWarningInDays = 100;
@@ -65,6 +67,8 @@ constexpr base::TimeDelta kLongWarning =
 constexpr base::TimeDelta kVeryLongWarning =
     base::TimeDelta::FromDays(kVeryLongWarningInDays);
 const char kPublicSessionId[] = "demo@example.com";
+const char kManagedUserId[] = "user@example.com";
+const char kManagedUserGaiaId[] = "11111";
 const char kUpdateRequiredNotificationId[] = "policy.update_required";
 const char kWifiServicePath[] = "/service/wifi2";
 // This is a randomly chosen long delay in milliseconds to make sure that the
@@ -179,36 +183,48 @@ class MinimumVersionPolicyTest : public MinimumVersionPolicyTestBase {
     MinimumVersionPolicyTestBase::TearDownOnMainThread();
   }
 
-  void Login();
-  void MarkUserManaged();
+  void LoginManagedUser();
+  void LoginUnmanagedUser();
 
  protected:
-  chromeos::LoginManagerMixin login_manager_{&mixin_host_};
+  const chromeos::LoginManagerMixin::TestUserInfo managed_user{
+      AccountId::FromUserEmailGaiaId(kManagedUserId, kManagedUserGaiaId)};
+  chromeos::UserPolicyMixin user_policy_mixin_{&mixin_host_,
+                                               managed_user.account_id};
+  chromeos::LoginManagerMixin login_manager_{&mixin_host_, {managed_user}};
   std::unique_ptr<chromeos::NetworkStateTestHelper> network_state_test_helper_;
   std::unique_ptr<NotificationDisplayServiceTester> display_service_tester_;
   std::unique_ptr<ash::SystemTrayTestApi> tray_test_api_;
 };
 
-void MinimumVersionPolicyTest::Login() {
-  const auto& users = login_manager_.users();
+void MinimumVersionPolicyTest::LoginManagedUser() {
+  user_policy_mixin_.RequestPolicyUpdate();
   EXPECT_EQ(user_manager::UserManager::Get()->GetLoggedInUsers().size(), 0u);
   EXPECT_EQ(session_manager::SessionManager::Get()->session_state(),
             session_manager::SessionState::LOGIN_PRIMARY);
 
-  LoginUser(users[0].account_id);
+  auto context =
+      chromeos::LoginManagerMixin::CreateDefaultUserContext(managed_user);
+  login_manager_.LoginAndWaitForActiveSession(context);
   EXPECT_EQ(user_manager::UserManager::Get()->GetLoggedInUsers().size(), 1u);
   EXPECT_EQ(session_manager::SessionManager::Get()->session_state(),
             session_manager::SessionState::ACTIVE);
 }
 
-void MinimumVersionPolicyTest::MarkUserManaged() {
-  Profile* profile = ProfileManager::GetPrimaryUserProfile();
-  DCHECK(profile);
-  profile->GetProfilePolicyConnector()->OverrideIsManagedForTesting(true);
+void MinimumVersionPolicyTest::LoginUnmanagedUser() {
+  const auto& users = login_manager_.users();
+  EXPECT_EQ(user_manager::UserManager::Get()->GetLoggedInUsers().size(), 0u);
+  EXPECT_EQ(session_manager::SessionManager::Get()->session_state(),
+            session_manager::SessionState::LOGIN_PRIMARY);
+
+  LoginUser(users[1].account_id);
+  EXPECT_EQ(user_manager::UserManager::Get()->GetLoggedInUsers().size(), 1u);
+  EXPECT_EQ(session_manager::SessionManager::Get()->session_state(),
+            session_manager::SessionState::ACTIVE);
 }
 
 IN_PROC_BROWSER_TEST_F(MinimumVersionPolicyTest, CriticalUpdateOnLoginScreen) {
-  EXPECT_EQ(ash::LoginScreenTestApi::GetUsersCount(), 1);
+  EXPECT_EQ(ash::LoginScreenTestApi::GetUsersCount(), 2);
   EXPECT_FALSE(ash::LoginScreenTestApi::IsOobeDialogVisible());
 
   // Create policy value as a list of requirements.
@@ -233,8 +249,7 @@ IN_PROC_BROWSER_TEST_F(MinimumVersionPolicyTest, CriticalUpdateOnLoginScreen) {
 
 IN_PROC_BROWSER_TEST_F(MinimumVersionPolicyTest, PRE_CriticalUpdateInSession) {
   // Login the user into the session and mark as managed.
-  Login();
-  MarkUserManaged();
+  LoginManagedUser();
 
   // Create policy value as a list of requirements.
   base::Value requirement_list(base::Value::Type::LIST);
@@ -258,7 +273,7 @@ IN_PROC_BROWSER_TEST_F(MinimumVersionPolicyTest, CriticalUpdateInSession) {
   // required in session.
   EXPECT_EQ(session_manager::SessionManager::Get()->session_state(),
             session_manager::SessionState::LOGIN_PRIMARY);
-  EXPECT_EQ(ash::LoginScreenTestApi::GetUsersCount(), 1);
+  EXPECT_EQ(ash::LoginScreenTestApi::GetUsersCount(), 2);
   // TODO(https://crbug.com/1048607): Show update required screen after user is
   // logged out of session due to critical update required by policy.
   EXPECT_FALSE(ash::LoginScreenTestApi::IsOobeDialogVisible());
@@ -267,7 +282,7 @@ IN_PROC_BROWSER_TEST_F(MinimumVersionPolicyTest, CriticalUpdateInSession) {
 
 IN_PROC_BROWSER_TEST_F(MinimumVersionPolicyTest, NonCriticalUpdateGoodNetwork) {
   // Login the user into the session.
-  Login();
+  LoginUnmanagedUser();
 
   // Check deadline timer is not running and local state is not set.
   PrefService* prefs = g_browser_process->local_state();
@@ -354,7 +369,7 @@ IN_PROC_BROWSER_TEST_F(MinimumVersionPolicyTest, NonCriticalUpdateGoodNetwork) {
 
 IN_PROC_BROWSER_TEST_F(MinimumVersionPolicyTest, DeviceUpdateStatusChange) {
   // Login the user into the session.
-  Login();
+  LoginUnmanagedUser();
 
   // Create and set policy value with warning time.
   base::Value requirement_short_warning(base::Value::Type::LIST);
@@ -406,7 +421,7 @@ IN_PROC_BROWSER_TEST_F(MinimumVersionPolicyTest, DeviceUpdateStatusChange) {
 IN_PROC_BROWSER_TEST_F(MinimumVersionPolicyTest,
                        CriticalUpdateInSessionUnmanagedUser) {
   // Login the user into the session.
-  Login();
+  LoginUnmanagedUser();
 
   // Create policy value as a list of requirements.
   base::Value requirement_list(base::Value::Type::LIST);
@@ -421,8 +436,7 @@ IN_PROC_BROWSER_TEST_F(MinimumVersionPolicyTest,
 
 IN_PROC_BROWSER_TEST_F(MinimumVersionPolicyTest, NoNetworkNotificationClick) {
   // Login the user into the session.
-  Login();
-  MarkUserManaged();
+  LoginManagedUser();
 
   // Create policy value as a list of requirements.
   base::Value requirement_list(base::Value::Type::LIST);
@@ -451,8 +465,7 @@ IN_PROC_BROWSER_TEST_F(MinimumVersionPolicyTest, NoNetworkNotificationClick) {
 IN_PROC_BROWSER_TEST_F(MinimumVersionPolicyTest,
                        HideNotificationOnGoodNetwork) {
   // Login the user into the session.
-  Login();
-  MarkUserManaged();
+  LoginManagedUser();
 
   // Create policy value as a list of requirements.
   base::Value requirement_list(base::Value::Type::LIST);
@@ -478,6 +491,67 @@ IN_PROC_BROWSER_TEST_F(MinimumVersionPolicyTest,
       display_service_tester_->GetNotification(kUpdateRequiredNotificationId));
 }
 
+IN_PROC_BROWSER_TEST_F(MinimumVersionPolicyTest, LastDayNotificationOnLogin) {
+  EXPECT_FALSE(
+      display_service_tester_->GetNotification(kUpdateRequiredNotificationId));
+
+  // Create and set policy value as a list of requirements.
+  base::Value requirement_list(base::Value::Type::LIST);
+  requirement_list.Append(CreateRequirement(kNewVersion, kLastDayWarningInDays,
+                                            kShortWarningInDays));
+  SetDevicePolicyAndWaitForSettingChange(requirement_list);
+
+  // Login the user into the session and check that notification is shown.
+  LoginManagedUser();
+  EXPECT_TRUE(
+      display_service_tester_->GetNotification(kUpdateRequiredNotificationId));
+  EXPECT_FALSE(tray_test_api_->IsTrayBubbleOpen());
+
+  // Clicking on the no network update required notification button opens the
+  // network settings and hides the notification.
+  display_service_tester_->SimulateClick(NotificationHandler::Type::TRANSIENT,
+                                         kUpdateRequiredNotificationId,
+                                         0 /*action_index*/, base::nullopt);
+  EXPECT_FALSE(
+      display_service_tester_->GetNotification(kUpdateRequiredNotificationId));
+  EXPECT_TRUE(tray_test_api_->IsTrayBubbleOpen());
+}
+
+IN_PROC_BROWSER_TEST_F(MinimumVersionPolicyTest,
+                       LastDayNotificationOnLoginUnmanagedUser) {
+  EXPECT_FALSE(
+      display_service_tester_->GetNotification(kUpdateRequiredNotificationId));
+
+  // Create and set policy value as a list of requirements.
+  base::Value requirement_list(base::Value::Type::LIST);
+  requirement_list.Append(CreateRequirement(kNewVersion, kLastDayWarningInDays,
+                                            kShortWarningInDays));
+  SetDevicePolicyAndWaitForSettingChange(requirement_list);
+
+  // Login the user into the session and check that notification is not shown
+  // for unmanaged user.
+  LoginUnmanagedUser();
+  EXPECT_FALSE(
+      display_service_tester_->GetNotification(kUpdateRequiredNotificationId));
+}
+
+IN_PROC_BROWSER_TEST_F(MinimumVersionPolicyTest, NotificationsOnLogin) {
+  EXPECT_FALSE(
+      display_service_tester_->GetNotification(kUpdateRequiredNotificationId));
+
+  // Create policy value as a list of requirements.
+  base::Value requirement_list(base::Value::Type::LIST);
+  requirement_list.Append(
+      CreateRequirement(kNewVersion, kShortWarningInDays, kShortWarningInDays));
+  SetDevicePolicyAndWaitForSettingChange(requirement_list);
+
+  // Login the user into the session and check that notification is not shown as
+  // it is not the last day to update device.
+  LoginManagedUser();
+  EXPECT_FALSE(
+      display_service_tester_->GetNotification(kUpdateRequiredNotificationId));
+}
+
 class MinimumVersionNoUsersLoginTest : public MinimumVersionPolicyTestBase {
  public:
   MinimumVersionNoUsersLoginTest() = default;