[signin] Allow dasher accounts to abort signin

Dasher accounts generally are not allowed to signout, because they may
have synced data that belongs to their administrator, and this data must
not leave their account.
The intended way to remove a dasher account from Chrome is to delete the
associated profile.

Due to this restriction, aborting signin was not possible for a dasher
account, even though the UI was shown. This results in a bad user
experience where the user clicks on the "cancel" button, but signin is
actually not cancelled.

The fix implemented here is to allow the user to signout without deleting
the profile. This is acceptable because the data has not been synced yet.

Bug: 864761, 804360
Change-Id: Iffbcde87ee242ada2815cd13d0bf8c660726d900
Reviewed-on: https://chromium-review.googlesource.com/c/1278734Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Commit-Queue: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599610}

chrome/browser/signin/chrome_signin_client.cc

@@ -74,13 +74,38 @@ SigninClient::SignoutDecision IsSignoutAllowed(
   // prohibits ALL sign-outs with the exception of ACCOUNT_REMOVED_FROM_DEVICE
   // because this preserves the original behavior. A follow-up CL will make the
   // slightly riskier change described above.
-  if (signin_util::IsUserSignoutAllowedForProfile(profile) ||
-      signout_source_metric ==
-          signin_metrics::ProfileSignout::ACCOUNT_REMOVED_FROM_DEVICE) {
+  if (signin_util::IsUserSignoutAllowedForProfile(profile))
     return SigninClient::SignoutDecision::ALLOW_SIGNOUT;
-  }
 
-  return SigninClient::SignoutDecision::DISALLOW_SIGNOUT;
+  switch (signout_source_metric) {
+    case signin_metrics::ProfileSignout::ACCOUNT_REMOVED_FROM_DEVICE:
+      return SigninClient::SignoutDecision::ALLOW_SIGNOUT;
+
+    case signin_metrics::ProfileSignout::ABORT_SIGNIN:
+#if BUILDFLAG(ENABLE_DICE_SUPPORT)
+      // Allowed, because data has not been synced yet.
+      return SigninClient::SignoutDecision::ALLOW_SIGNOUT;
+#else
+      // ABORT_SIGNIN is only used on Dice platforms.
+      NOTREACHED();
+      return SigninClient::SignoutDecision::DISALLOW_SIGNOUT;
+#endif
+
+    case signin_metrics::ProfileSignout::SIGNOUT_PREF_CHANGED:
+    case signin_metrics::ProfileSignout::GOOGLE_SERVICE_NAME_PATTERN_CHANGED:
+    case signin_metrics::ProfileSignout::SIGNIN_PREF_CHANGED_DURING_SIGNIN:
+    case signin_metrics::ProfileSignout::USER_CLICKED_SIGNOUT_SETTINGS:
+    case signin_metrics::ProfileSignout::SERVER_FORCED_DISABLE:
+    case signin_metrics::ProfileSignout::TRANSFER_CREDENTIALS:
+    case signin_metrics::ProfileSignout::
+        AUTHENTICATION_FAILED_WITH_FORCE_SIGNIN:
+    case signin_metrics::ProfileSignout::USER_TUNED_OFF_SYNC_FROM_DICE_UI:
+      return SigninClient::SignoutDecision::DISALLOW_SIGNOUT;
+
+    case signin_metrics::ProfileSignout::NUM_PROFILE_SIGNOUT_METRICS:
+      NOTREACHED();
+      return SigninClient::SignoutDecision::DISALLOW_SIGNOUT;
+  }
 }
 }  // namespace
 

chrome/browser/signin/chrome_signin_client_unittest.cc

@@ -190,7 +190,7 @@ class ChromeSigninClientSignoutTest : public BrowserWithTestWindowTest {
 
 TEST_F(ChromeSigninClientSignoutTest, SignOut) {
   signin_metrics::ProfileSignout source_metric =
-      signin_metrics::ProfileSignout::ABORT_SIGNIN;
+      signin_metrics::ProfileSignout::USER_CLICKED_SIGNOUT_SETTINGS;
   signin_metrics::SignoutDelete delete_metric =
       signin_metrics::SignoutDelete::IGNORE_METRIC;
 
@@ -210,7 +210,7 @@ TEST_F(ChromeSigninClientSignoutTest, SignOut) {
 
 TEST_F(ChromeSigninClientSignoutTest, SignOutWithoutManager) {
   signin_metrics::ProfileSignout source_metric =
-      signin_metrics::ProfileSignout::ABORT_SIGNIN;
+      signin_metrics::ProfileSignout::USER_CLICKED_SIGNOUT_SETTINGS;
   signin_metrics::SignoutDelete delete_metric =
       signin_metrics::SignoutDelete::IGNORE_METRIC;
 
@@ -251,7 +251,7 @@ TEST_F(ChromeSigninClientSignoutTest, SignOutWithoutForceSignin) {
                                                  fake_controller_.get());
 
   signin_metrics::ProfileSignout source_metric =
-      signin_metrics::ProfileSignout::ABORT_SIGNIN;
+      signin_metrics::ProfileSignout::USER_CLICKED_SIGNOUT_SETTINGS;
   signin_metrics::SignoutDelete delete_metric =
       signin_metrics::SignoutDelete::IGNORE_METRIC;
 
@@ -278,7 +278,7 @@ TEST_F(ChromeSigninClientSignoutTest, SignOutGuestSession) {
                                                  fake_controller_.get());
 
   signin_metrics::ProfileSignout source_metric =
-      signin_metrics::ProfileSignout::ABORT_SIGNIN;
+      signin_metrics::ProfileSignout::USER_CLICKED_SIGNOUT_SETTINGS;
   signin_metrics::SignoutDelete delete_metric =
       signin_metrics::SignoutDelete::IGNORE_METRIC;
 
@@ -299,14 +299,15 @@ class ChromeSigninClientSignoutSourceTest
     : public ::testing::WithParamInterface<signin_metrics::ProfileSignout>,
       public ChromeSigninClientSignoutTest {};
 
-bool IsUserDrivenSignout(signin_metrics::ProfileSignout signout_source) {
+// Returns true if signout can be disallowed by policy for the given source.
+bool IsSignoutDisallowedByPolicy(
+    signin_metrics::ProfileSignout signout_source) {
   switch (signout_source) {
     // NOTE: SIGNOUT_TEST == SIGNOUT_PREF_CHANGED.
     case signin_metrics::ProfileSignout::SIGNOUT_PREF_CHANGED:
     case signin_metrics::ProfileSignout::GOOGLE_SERVICE_NAME_PATTERN_CHANGED:
     case signin_metrics::ProfileSignout::SIGNIN_PREF_CHANGED_DURING_SIGNIN:
     case signin_metrics::ProfileSignout::USER_CLICKED_SIGNOUT_SETTINGS:
-    case signin_metrics::ProfileSignout::ABORT_SIGNIN:
     case signin_metrics::ProfileSignout::SERVER_FORCED_DISABLE:
     case signin_metrics::ProfileSignout::TRANSFER_CREDENTIALS:
     case signin_metrics::ProfileSignout::
@@ -319,6 +320,9 @@ bool IsUserDrivenSignout(signin_metrics::ProfileSignout signout_source) {
       // quo. Additional internal sources of sign-out will be moved here in a
       // follow up CL.
       return false;
+    case signin_metrics::ProfileSignout::ABORT_SIGNIN:
+      // Allow signout because data has not been synced yet.
+      return false;
     case signin_metrics::ProfileSignout::NUM_PROFILE_SIGNOUT_METRICS:
       NOTREACHED();
       return false;
@@ -371,7 +375,7 @@ TEST_P(ChromeSigninClientSignoutSourceTest, UserSignoutDisallowed) {
   // Verify SigninManager gets callback indicating sign-out is disallowed iff
   // the source of the sign-out is a user-action.
   SigninClient::SignoutDecision signout_decision =
-      IsUserDrivenSignout(signout_source)
+      IsSignoutDisallowedByPolicy(signout_source)
           ? SigninClient::SignoutDecision::DISALLOW_SIGNOUT
           : SigninClient::SignoutDecision::ALLOW_SIGNOUT;
   signin_metrics::SignoutDelete delete_metric =