Show PIN setup during sign in for Family Link users

Shows setup for Family Link users on tablet and clamshell if the device
supports PIN for login. Does not change the behavior for other users.

Bug: 1156890
Test: PinForLoginSetupScreenTest
Change-Id: I2e1f8a29ae6fa2739ee57ade6f1351d46254da48
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2580402Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
Commit-Queue: Aga Wronska <agawronska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837130}

chrome/browser/chromeos/login/screens/pin_setup_screen.cc

@@ -7,10 +7,12 @@
 #include "ash/public/cpp/tablet_mode.h"
 #include "base/check.h"
 #include "base/metrics/histogram_functions.h"
+#include "chrome/browser/chromeos/login/quick_unlock/pin_backend.h"
 #include "chrome/browser/chromeos/login/quick_unlock/quick_unlock_utils.h"
 #include "chrome/browser/chromeos/login/users/chrome_user_manager_util.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/webui/chromeos/login/pin_setup_screen_handler.h"
+#include "chromeos/constants/chromeos_features.h"
 #include "chromeos/constants/chromeos_switches.h"
 #include "components/prefs/pref_service.h"
 
@@ -74,18 +76,12 @@ std::string PinSetupScreen::GetResultString(Result result) {
   }
 }
 
-bool PinSetupScreen::ShouldSkip() {
+// static
+bool PinSetupScreen::ShouldSkipBecauseOfPolicy() {
   PrefService* prefs = ProfileManager::GetActiveUserProfile()->GetPrefs();
   if (chrome_user_manager_util::IsPublicSessionOrEphemeralLogin() ||
-      !chromeos::quick_unlock::IsPinEnabled(prefs) ||
-      chromeos::quick_unlock::IsPinDisabledByPolicy(prefs)) {
-    return true;
-  }
-
-  // Skip the screen if the device is not in tablet mode, unless tablet mode
-  // first user run is forced on the device.
-  if (!ash::TabletMode::Get()->InTabletMode() &&
-      !chromeos::switches::ShouldOobeUseTabletModeFirstRun()) {
+      !quick_unlock::IsPinEnabled(prefs) ||
+      quick_unlock::IsPinDisabledByPolicy(prefs)) {
     return true;
   }
 
@@ -99,6 +95,9 @@ PinSetupScreen::PinSetupScreen(PinSetupScreenView* view,
       exit_callback_(exit_callback) {
   DCHECK(view_);
   view_->Bind(this);
+
+  quick_unlock::PinBackend::GetInstance()->HasLoginSupport(base::BindOnce(
+      &PinSetupScreen::OnHasLoginSupport, weak_ptr_factory_.GetWeakPtr()));
 }
 
 PinSetupScreen::~PinSetupScreen() {
@@ -107,7 +106,23 @@ PinSetupScreen::~PinSetupScreen() {
 }
 
 bool PinSetupScreen::MaybeSkip(WizardContext* context) {
-  if (ShouldSkip()) {
+  if (ShouldSkipBecauseOfPolicy()) {
+    exit_callback_.Run(Result::NOT_APPLICABLE);
+    return true;
+  }
+
+  // Show setup for Family Link users on tablet and clamshell if the device
+  // supports PIN for login.
+  bool show_for_family_link_user =
+      features::IsPinSetupForFamilyLinkEnabled() &&
+      ProfileManager::GetActiveUserProfile()->IsChild() &&
+      has_login_support_.value_or(false);
+
+  // Skip the screen if the device is not in tablet mode, unless tablet mode
+  // first user run is forced on the device.
+  if (!ash::TabletMode::Get()->InTabletMode() &&
+      !switches::ShouldOobeUseTabletModeFirstRun() &&
+      !show_for_family_link_user) {
     exit_callback_.Run(Result::NOT_APPLICABLE);
     return true;
   }
@@ -123,6 +138,10 @@ void PinSetupScreen::HideImpl() {
   view_->Hide();
 }
 
+void PinSetupScreen::OnHasLoginSupport(bool has_login_support) {
+  has_login_support_ = has_login_support;
+}
+
 void PinSetupScreen::OnUserAction(const std::string& action_id) {
   // Only honor finish if discover is currently being shown.
   if (action_id == kFinished) {

chrome/browser/chromeos/login/screens/pin_setup_screen.h

@@ -9,6 +9,8 @@
 
 #include "base/callback.h"
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "base/optional.h"
 #include "chrome/browser/chromeos/login/screens/base_screen.h"
 
 namespace chromeos {
@@ -31,7 +33,11 @@ class PinSetupScreen : public BaseScreen {
   };
 
   static std::string GetResultString(Result result);
-  static bool ShouldSkip();
+
+  // Checks whether PIN setup should be skipped because of the policies.
+  // There is an additional checkpoint that might skip the setup based on user
+  // profile and pin availability information in `MaybeSkip`.
+  static bool ShouldSkipBecauseOfPolicy();
 
   using ScreenExitCallback = base::RepeatingCallback<void(Result result)>;
   PinSetupScreen(PinSetupScreenView* view,
@@ -54,9 +60,18 @@ class PinSetupScreen : public BaseScreen {
   void OnUserAction(const std::string& action_id) override;
 
  private:
+  void OnHasLoginSupport(bool has_login_support);
+
+  // Inticates whether the device supports usage of PIN for login.
+  // This information is retrived in an async way and will not be available
+  // immediately.
+  base::Optional<bool> has_login_support_;
+
   PinSetupScreenView* const view_;
   ScreenExitCallback exit_callback_;
 
+  base::WeakPtrFactory<PinSetupScreen> weak_ptr_factory_{this};
+
   DISALLOW_COPY_AND_ASSIGN(PinSetupScreen);
 };
 

chrome/browser/chromeos/login/screens/pin_setup_screen_browsertest.cc

@@ -7,6 +7,7 @@
 #include "ash/public/cpp/test/shell_test_api.h"
 #include "base/bind.h"
 #include "base/run_loop.h"
+#include "base/test/scoped_feature_list.h"
 #include "chrome/browser/chromeos/login/screen_manager.h"
 #include "chrome/browser/chromeos/login/test/fake_gaia_mixin.h"
 #include "chrome/browser/chromeos/login/test/js_checker.h"
@@ -19,6 +20,9 @@
 #include "chrome/browser/chromeos/login/ui/login_display_host.h"
 #include "chrome/browser/chromeos/login/wizard_controller.h"
 #include "chrome/browser/ui/webui/chromeos/login/pin_setup_screen_handler.h"
+#include "chromeos/constants/chromeos_features.h"
+#include "chromeos/dbus/cryptohome/cryptohome_client.h"
+#include "chromeos/dbus/cryptohome/fake_cryptohome_client.h"
 #include "chromeos/login/auth/stub_authenticator_builder.h"
 #include "components/user_manager/user_type.h"
 #include "content/public/test/browser_test.h"
@@ -45,6 +49,9 @@ class PinSetupScreenTest
       public testing::WithParamInterface<user_manager::UserType> {
  public:
   PinSetupScreenTest() {
+    CryptohomeClient::InitializeFake();
+    FakeCryptohomeClient::Get()->set_supports_low_entropy_credentials(false);
+
     if (GetParam() == user_manager::USER_TYPE_CHILD) {
       fake_gaia_ =
           std::make_unique<FakeGaiaMixin>(&mixin_host_, embedded_test_server());
@@ -233,4 +240,93 @@ IN_PROC_BROWSER_TEST_P(PinSetupScreenTest, FinishedFlow) {
           1)));
 }
 
+// Tests the PIN setup screen in the scenario when the device supports low
+// entropy credentials and PIN can be used for login.
+class PinForLoginSetupScreenTest : public PinSetupScreenTest {
+ protected:
+  PinForLoginSetupScreenTest() {
+    // Enable PIN for login (overrides base class setting).
+    CryptohomeClient::InitializeFake();
+    FakeCryptohomeClient::Get()->set_supports_low_entropy_credentials(true);
+    scoped_feature_list_.InitAndEnableFeature(features::kPinSetupForFamilyLink);
+  }
+
+  ~PinForLoginSetupScreenTest() override = default;
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+INSTANTIATE_TEST_SUITE_P(All,
+                         PinForLoginSetupScreenTest,
+                         ::testing::Values(user_manager::USER_TYPE_REGULAR,
+                                           user_manager::USER_TYPE_CHILD));
+
+// Tests that PIN setup is shown to Family Link users (but not to regular users)
+// on clamshell devices that support low entropy credentials.
+IN_PROC_BROWSER_TEST_P(PinForLoginSetupScreenTest, ClamshellMode) {
+  ShowPinSetupScreen();
+
+  if (GetParam() == user_manager::USER_TYPE_CHILD) {
+    WaitForScreenShown();
+
+    EnterPin();
+    test::OobeJS().TapOnPath(kNextButton);
+    test::OobeJS().CreateVisibilityWaiter(true, {kBackButton})->Wait();
+
+    EnterPin();
+    test::OobeJS().TapOnPath(kNextButton);
+    test::OobeJS().CreateVisibilityWaiter(true, {kDoneButton})->Wait();
+
+    test::OobeJS().TapOnPath(kDoneButton);
+
+    WaitForScreenExit();
+    EXPECT_EQ(screen_result_.value(), PinSetupScreen::Result::NEXT);
+    histogram_tester_.ExpectTotalCount(
+        "OOBE.StepCompletionTimeByExitReason.Discover.Next", 1);
+    histogram_tester_.ExpectTotalCount("OOBE.StepCompletionTime.Discover", 1);
+    EXPECT_THAT(
+        histogram_tester_.GetAllSamples("OOBE.PinSetupScreen.UserActions"),
+        ElementsAre(base::Bucket(
+            static_cast<int>(PinSetupScreen::UserAction::kDoneButtonClicked),
+            1)));
+  } else {
+    WaitForScreenExit();
+    EXPECT_EQ(screen_result_.value(), PinSetupScreen::Result::NOT_APPLICABLE);
+    histogram_tester_.ExpectTotalCount(
+        "OOBE.StepCompletionTimeByExitReason.Discover.Next", 0);
+    histogram_tester_.ExpectTotalCount("OOBE.StepCompletionTime.Discover", 0);
+  }
+}
+
+// Tests that PIN setup is shown to Family Link and regular users in tablet
+// mode.
+IN_PROC_BROWSER_TEST_P(PinForLoginSetupScreenTest, TabletMode) {
+  ash::ShellTestApi().SetTabletModeEnabledForTest(true);
+  ShowPinSetupScreen();
+
+  WaitForScreenShown();
+
+  EnterPin();
+  test::OobeJS().TapOnPath(kNextButton);
+  test::OobeJS().CreateVisibilityWaiter(true, {kBackButton})->Wait();
+
+  EnterPin();
+  test::OobeJS().TapOnPath(kNextButton);
+  test::OobeJS().CreateVisibilityWaiter(true, {kDoneButton})->Wait();
+
+  test::OobeJS().TapOnPath(kDoneButton);
+
+  WaitForScreenExit();
+  EXPECT_EQ(screen_result_.value(), PinSetupScreen::Result::NEXT);
+  histogram_tester_.ExpectTotalCount(
+      "OOBE.StepCompletionTimeByExitReason.Discover.Next", 1);
+  histogram_tester_.ExpectTotalCount("OOBE.StepCompletionTime.Discover", 1);
+  EXPECT_THAT(
+      histogram_tester_.GetAllSamples("OOBE.PinSetupScreen.UserActions"),
+      ElementsAre(base::Bucket(
+          static_cast<int>(PinSetupScreen::UserAction::kDoneButtonClicked),
+          1)));
+}
+
 }  // namespace chromeos

chrome/browser/chromeos/login/ui/login_display_host_common.cc

@@ -366,7 +366,7 @@ void LoginDisplayHostCommon::StartSupervisionTransition() {
 
 void LoginDisplayHostCommon::SetAuthSessionForOnboarding(
     const UserContext& user_context) {
-  if (PinSetupScreen::ShouldSkip())
+  if (PinSetupScreen::ShouldSkipBecauseOfPolicy())
     return;
   chromeos::DiscoverManager::Get()
       ->GetModule<chromeos::DiscoverModulePinSetup>()

chromeos/constants/chromeos_features.cc

@@ -505,6 +505,11 @@ const base::Feature kPhoneHub{"PhoneHub", base::FEATURE_DISABLED_BY_DEFAULT};
 const base::Feature kPhoneHubUseBle{"PhoneHubUseBle",
                                     base::FEATURE_DISABLED_BY_DEFAULT};
 
+// Enables PIN setup in OOBE for Family Link users on all devices supporting low
+// entropy credentials regardless the form factor.
+const base::Feature kPinSetupForFamilyLink{"PinSetupForFamilyLink",
+                                           base::FEATURE_ENABLED_BY_DEFAULT};
+
 // Controls whether the camera permissions should be shown in the Plugin
 // VM app settings.
 const base::Feature kPluginVmShowCameraPermissions{
@@ -817,6 +822,10 @@ bool IsPhoneHubUseBleEnabled() {
   return base::FeatureList::IsEnabled(kPhoneHubUseBle) && IsPhoneHubEnabled();
 }
 
+bool IsPinSetupForFamilyLinkEnabled() {
+  return base::FeatureList::IsEnabled(kPinSetupForFamilyLink);
+}
+
 bool IsPinAutosubmitFeatureEnabled() {
   return base::FeatureList::IsEnabled(kQuickUnlockPinAutosubmit);
 }

chromeos/constants/chromeos_features.h

@@ -225,6 +225,8 @@ extern const base::Feature kPhoneHub;
 COMPONENT_EXPORT(CHROMEOS_CONSTANTS)
 extern const base::Feature kPhoneHubUseBle;
 COMPONENT_EXPORT(CHROMEOS_CONSTANTS)
+extern const base::Feature kPinSetupForFamilyLink;
+COMPONENT_EXPORT(CHROMEOS_CONSTANTS)
 extern const base::Feature kPluginVmShowCameraPermissions;
 COMPONENT_EXPORT(CHROMEOS_CONSTANTS)
 extern const base::Feature kPluginVmShowMicrophonePermissions;
@@ -355,6 +357,8 @@ COMPONENT_EXPORT(CHROMEOS_CONSTANTS) bool IsOobeScreensPriorityEnabled();
 COMPONENT_EXPORT(CHROMEOS_CONSTANTS) bool IsPhoneHubEnabled();
 COMPONENT_EXPORT(CHROMEOS_CONSTANTS) bool IsPhoneHubUseBleEnabled();
 COMPONENT_EXPORT(CHROMEOS_CONSTANTS) bool IsPinAutosubmitFeatureEnabled();
+COMPONENT_EXPORT(CHROMEOS_CONSTANTS) bool IsPinSetupForFamilyLinkEnabled();
+
 COMPONENT_EXPORT(CHROMEOS_CONSTANTS)
 bool IsPinAutosubmitBackfillFeatureEnabled();
 COMPONENT_EXPORT(CHROMEOS_CONSTANTS) bool IsQuickAnswersDogfood();