[iOS] Add sign-in completion to reauthentication flow.

- Clarifies the difference between unified consent add account and reauth.
Add account and reauthentication methods in identity differ in that
one provides the user with a screen to enter their account details
and the other automatically prefills these details.

- Implements sign-in completion for reauth.

Bug: 971989
Change-Id: I2b0d2d2cc830f7e989480d7444002144d26a38ac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2095118
Commit-Queue: Nohemi Fernandez <fernandex@chromium.org>
Reviewed-by: Jérôme Lebel <jlebel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#750557}

ios/chrome/browser/ui/authentication/signin/add_account_signin/BUILD.gn

@@ -9,6 +9,7 @@ source_set("add_account_signin") {
   sources = [
     "add_account_signin_coordinator.h",
     "add_account_signin_coordinator.mm",
+    "add_account_signin_enums.h",
     "add_account_signin_mediator.h",
     "add_account_signin_mediator.mm",
   ]

ios/chrome/browser/ui/authentication/signin/add_account_signin/add_account_signin_coordinator.h

@@ -5,6 +5,7 @@
 #ifndef IOS_CHROME_BROWSER_UI_AUTHENTICATION_SIGNIN_ADD_ACCOUNT_SIGNIN_ADD_ACCOUNT_SIGNIN_COORDINATOR_H_
 #define IOS_CHROME_BROWSER_UI_AUTHENTICATION_SIGNIN_ADD_ACCOUNT_SIGNIN_ADD_ACCOUNT_SIGNIN_COORDINATOR_H_
 
+#import "ios/chrome/browser/ui/authentication/signin/add_account_signin/add_account_signin_enums.h"
 #import "ios/chrome/browser/ui/authentication/signin/signin_coordinator.h"
 
 // Coordinates adding an account with different intents:
@@ -24,13 +25,13 @@
 // |viewController| presents the sign-in.
 // |accessPoint| is the view where the sign-in button was displayed.
 // |promoAction| is promo button used to trigger the sign-in.
-// |signinIntent| is the intent when the user begins a sign-in flow.
+// |signinIntent| is the sign-in flow that will be triggered.
 - (instancetype)
     initWithBaseViewController:(UIViewController*)viewController
                        browser:(Browser*)browser
                    accessPoint:(signin_metrics::AccessPoint)accessPoint
                    promoAction:(signin_metrics::PromoAction)promoAction
-                  signinIntent:(SigninIntent)signinIntent
+                  signinIntent:(AddAccountSigninIntent)signinIntent
     NS_DESIGNATED_INITIALIZER;
 
 @end

ios/chrome/browser/ui/authentication/signin/add_account_signin/add_account_signin_coordinator.mm

@@ -30,25 +30,19 @@ using signin_metrics::PromoAction;
 
 // Coordinator to display modal alerts to the user.
 @property(nonatomic, strong) AlertCoordinator* alertCoordinator;
-
 // Coordinator that handles the sign-in UI flow.
 @property(nonatomic, strong) SigninCoordinator* userSigninCoordinator;
-
 // Mediator that handles sign-in state.
 @property(nonatomic, strong) AddAccountSigninMediator* mediator;
-
 // Manager that handles interactions to add identities.
 @property(nonatomic, strong)
     ChromeIdentityInteractionManager* identityInteractionManager;
-
 // View where the sign-in button was displayed.
 @property(nonatomic, assign) AccessPoint accessPoint;
-
 // Promo button used to trigger the sign-in.
 @property(nonatomic, assign) PromoAction promoAction;
-
-// Intent when the user begins a sign-in flow.
-@property(nonatomic, assign) SigninIntent signinIntent;
+// Add account sign-in intent.
+@property(nonatomic, assign, readonly) AddAccountSigninIntent signinIntent;
 
 @end
 
@@ -60,7 +54,8 @@ using signin_metrics::PromoAction;
                                    browser:(Browser*)browser
                                accessPoint:(AccessPoint)accessPoint
                                promoAction:(PromoAction)promoAction
-                              signinIntent:(SigninIntent)signinIntent {
+                              signinIntent:
+                                  (AddAccountSigninIntent)signinIntent {
   self = [super initWithBaseViewController:viewController browser:browser];
   if (self) {
     _signinIntent = signinIntent;
@@ -74,6 +69,12 @@ using signin_metrics::PromoAction;
 
 - (void)interruptWithAction:(SigninCoordinatorInterruptAction)action
                  completion:(ProceduralBlock)completion {
+  if (self.userSigninCoordinator) {
+    [self.userSigninCoordinator interruptWithAction:action
+                                         completion:completion];
+    return;
+  }
+
   DCHECK(self.identityInteractionManager);
   switch (action) {
     case SigninCoordinatorInterruptActionNoDismiss:
@@ -114,9 +115,9 @@ using signin_metrics::PromoAction;
                                              ->GetPrefs()
                          identityManager:identityManager];
   self.mediator.delegate = self;
-  [self.mediator handleSigninIntent:self.signinIntent
-                        accessPoint:self.accessPoint
-                        promoAction:self.promoAction];
+  [self.mediator handleSigninWithIntent:self.signinIntent
+                            accessPoint:self.accessPoint
+                            promoAction:self.promoAction];
 }
 
 - (void)stop {
@@ -149,13 +150,13 @@ using signin_metrics::PromoAction;
 
 #pragma mark - AddAccountSigninMediatorDelegate
 
-- (void)addAccountSigninMediatorFailedWith:(NSError*)error {
+- (void)addAccountSigninMediatorFailedWithError:(NSError*)error {
   DCHECK(error);
   __weak AddAccountSigninCoordinator* weakSelf = self;
   ProceduralBlock dismissAction = ^{
-    [weakSelf addAccountSigninMediatorFinishedWith:
+    [weakSelf addAccountSigninMediatorFinishedWithSigninResult:
                   SigninCoordinatorResultCanceledByUser
-                                          identity:nil];
+                                                      identity:nil];
   };
 
   self.alertCoordinator =
@@ -163,31 +164,35 @@ using signin_metrics::PromoAction;
   [self.alertCoordinator start];
 }
 
-- (void)addAccountSigninMediatorFinishedWith:
+- (void)addAccountSigninMediatorFinishedWithSigninResult:
             (SigninCoordinatorResult)signinResult
-                                    identity:(ChromeIdentity*)identity {
+                                                identity:
+                                                    (ChromeIdentity*)identity {
+  self.identityInteractionManager = nil;
   switch (self.signinIntent) {
-    case SigninIntentReauth: {
-      // Show the user consent screen to finish the sign-in operation.
-      [self handleUserConsentForIdentity:identity];
-      return;
+    case AddAccountSigninIntentReauthPrimaryAccount: {
+      [self presentUserConsentWithIdentity:identity];
+      break;
     }
-    case SigninIntentAddAccount: {
+    case AddAccountSigninIntentAddSecondaryAccount: {
+      [self addAccountDoneWithSigninResult:signinResult identity:identity];
       break;
     }
   }
+}
 
-  // Cleaning up and calling the |signinCompletion| should be done last.
-  self.identityInteractionManager = nil;
+#pragma mark - Private
+
+// Runs callback completion on finishing the add account flow.
+- (void)addAccountDoneWithSigninResult:(SigninCoordinatorResult)signinResult
+                              identity:(ChromeIdentity*)identity {
   DCHECK(!self.alertCoordinator);
   DCHECK(!self.userSigninCoordinator);
-
   [self runCompletionCallbackWithSigninResult:signinResult identity:identity];
 }
 
-#pragma mark - Private
-
-- (void)handleUserConsentForIdentity:(ChromeIdentity*)identity {
+// Presents the user consent screen with |identity| pre-selected.
+- (void)presentUserConsentWithIdentity:(ChromeIdentity*)identity {
   // The UserSigninViewController is presented on top of the currently displayed
   // view controller.
   self.userSigninCoordinator = [SigninCoordinator
@@ -197,11 +202,14 @@ using signin_metrics::PromoAction;
                                          identity:identity
                                       accessPoint:self.accessPoint
                                       promoAction:self.promoAction];
-  self.userSigninCoordinator.signinCompletion =
-      ^(SigninCoordinatorResult signinResult, ChromeIdentity* identity) {
-        // TODO(crbug.com/971989): Needs implementation.
-        NOTIMPLEMENTED();
-      };
+
+  __weak AddAccountSigninCoordinator* weakSelf = self;
+  self.userSigninCoordinator.signinCompletion = ^(
+      SigninCoordinatorResult signinResult, ChromeIdentity* identity) {
+    [weakSelf.userSigninCoordinator stop];
+    weakSelf.userSigninCoordinator = nil;
+    [weakSelf addAccountDoneWithSigninResult:signinResult identity:identity];
+  };
   [self.userSigninCoordinator start];
 }
 

ios/chrome/browser/ui/authentication/signin/add_account_signin/add_account_signin_enums.h

+// Copyright 2020 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 IOS_CHROME_BROWSER_UI_AUTHENTICATION_SIGNIN_ADD_ACCOUNT_SIGNIN_ADD_ACCOUNT_SIGNIN_ENUMS_H_
+#define IOS_CHROME_BROWSER_UI_AUTHENTICATION_SIGNIN_ADD_ACCOUNT_SIGNIN_ADD_ACCOUNT_SIGNIN_ENUMS_H_
+
+#import <Foundation/Foundation.h>
+
+// Intent for the add account sign-in flow.
+typedef NS_ENUM(NSUInteger, AddAccountSigninIntent) {
+  // Adds a secondary account that will only be used for the web.
+  AddAccountSigninIntentAddSecondaryAccount,
+  // Reauthenticates with the previous primary account. Since it is
+  // a primary account, sign-in consent is required.
+  AddAccountSigninIntentReauthPrimaryAccount,
+};
+
+#endif  // IOS_CHROME_BROWSER_UI_AUTHENTICATION_SIGNIN_ADD_ACCOUNT_SIGNIN_ADD_ACCOUNT_SIGNIN_ENUMS_H_

ios/chrome/browser/ui/authentication/signin/add_account_signin/add_account_signin_mediator.h

@@ -6,6 +6,7 @@
 #define IOS_CHROME_BROWSER_UI_AUTHENTICATION_SIGNIN_ADD_ACCOUNT_SIGNIN_ADD_ACCOUNT_SIGNIN_MEDIATOR_H_
 
 #import "components/signin/public/base/signin_metrics.h"
+#import "ios/chrome/browser/ui/authentication/signin/add_account_signin/add_account_signin_enums.h"
 #import "ios/chrome/browser/ui/authentication/signin/signin_enums.h"
 
 @class ChromeIdentity;
@@ -22,14 +23,15 @@ class IdentityManager;
 
 // Shows alert modal dialog and interrupts sign-in operation.
 // |error| is the error to be displayed.
-- (void)addAccountSigninMediatorFailedWith:(NSError*)error;
+- (void)addAccountSigninMediatorFailedWithError:(NSError*)error;
 
 // Completes the sign-in operation.
 // |signinResult| is the state of sign-in at add account flow completion.
 // |identity| is the identity of the added account.
-- (void)addAccountSigninMediatorFinishedWith:
+- (void)addAccountSigninMediatorFinishedWithSigninResult:
             (SigninCoordinatorResult)signinResult
-                                    identity:(ChromeIdentity*)identity;
+                                                identity:
+                                                    (ChromeIdentity*)identity;
 
 @end
 
@@ -48,12 +50,12 @@ class IdentityManager;
 @property(nonatomic, weak) id<AddAccountSigninMediatorDelegate> delegate;
 
 // Handles the sign-in operation.
-// |signinIntent| specifies the sign-in intent, either adding an account or
-// reauthentication. |accessPoint| is the view where the sign-in button was
-// displayed. |promoAction| is promo button used to trigger the sign-in.
-- (void)handleSigninIntent:(SigninIntent)signinIntent
-               accessPoint:(signin_metrics::AccessPoint)accessPoint
-               promoAction:(signin_metrics::PromoAction)promoAction;
+// |signinIntent| is the add account sign-in flow intent.
+// |accessPoint| is the view where the sign-in button was displayed.
+// |promoAction| is promo button used to trigger the sign-in.
+- (void)handleSigninWithIntent:(AddAccountSigninIntent)addAccountSigninIntent
+                   accessPoint:(signin_metrics::AccessPoint)accessPoint
+                   promoAction:(signin_metrics::PromoAction)promoAction;
 
 @end
 

ios/chrome/browser/ui/authentication/signin/add_account_signin/add_account_signin_mediator.mm

@@ -23,10 +23,8 @@ using signin_metrics::PromoAction;
 // The coordinator's manager that handles interactions to add identities.
 @property(nonatomic, weak)
     ChromeIdentityInteractionManager* identityInteractionManager;
-
 // The Browser state's user-selected preferences.
 @property(nonatomic, assign) PrefService* prefService;
-
 // The Browser state's identity manager.
 @property(nonatomic, assign) signin::IdentityManager* identityManager;
 
@@ -51,15 +49,15 @@ using signin_metrics::PromoAction;
   return self;
 }
 
-- (void)handleSigninIntent:(SigninIntent)signinIntent
-               accessPoint:(AccessPoint)accessPoint
-               promoAction:(PromoAction)promoAction {
+- (void)handleSigninWithIntent:(AddAccountSigninIntent)signinIntent
+                   accessPoint:(AccessPoint)accessPoint
+                   promoAction:(PromoAction)promoAction {
   switch (signinIntent) {
-    case SigninIntentAddAccount: {
+    case AddAccountSigninIntentAddSecondaryAccount: {
       [self addAccount];
       break;
     }
-    case SigninIntentReauth: {
+    case AddAccountSigninIntentReauthPrimaryAccount: {
       signin_metrics::LogSigninAccessPointStarted(accessPoint, promoAction);
       [self reauthenticate];
       break;
@@ -92,10 +90,9 @@ using signin_metrics::PromoAction;
   return SigninCoordinatorResultSuccess;
 }
 
-// Handles the reauthentication operation for a user from the screen to enter
-// credentials with the primary account email pre-entered to the user consent
-// flow. In the case of a sign-in error will display a modal alert and abort the
-// reauth flow.
+// Handles the add account operation for a user. User account will be
+// automatically populated using the primary user account. In the case of a
+// sign-in error will display a modal alert and abort adding an account.
 - (void)reauthenticate {
   CoreAccountInfo accountInfo = self.identityManager->GetPrimaryAccountInfo();
 
@@ -134,12 +131,13 @@ using signin_metrics::PromoAction;
   switch (signinResult) {
     case SigninCoordinatorResultSuccess:
     case SigninCoordinatorResultCanceledByUser: {
-      [self.delegate addAccountSigninMediatorFinishedWith:signinResult
-                                                 identity:identity];
+      [self.delegate
+          addAccountSigninMediatorFinishedWithSigninResult:signinResult
+                                                  identity:identity];
       break;
     }
     case SigninCoordinatorResultInterrupted: {
-      [self.delegate addAccountSigninMediatorFailedWith:error];
+      [self.delegate addAccountSigninMediatorFailedWithError:error];
       break;
     }
   }

ios/chrome/browser/ui/authentication/signin/add_account_signin/add_account_signin_mediator_unittest.mm

@@ -125,14 +125,15 @@ class AddAccountSigninMediatorTest : public PlatformTest {
 TEST_F(AddAccountSigninMediatorTest, AddAccountIntent) {
   // Verify that completion was called with success state.
   [[mediator_delegate_ expect]
-      addAccountSigninMediatorFinishedWith:SigninCoordinatorResultSuccess
-                                  identity:fake_identity_];
+      addAccountSigninMediatorFinishedWithSigninResult:
+          SigninCoordinatorResultSuccess
+                                              identity:fake_identity_];
 
   [mediator_
-      handleSigninIntent:SigninIntentAddAccount
-             accessPoint:signin_metrics::AccessPoint::ACCESS_POINT_SETTINGS
-             promoAction:signin_metrics::PromoAction::
-                             PROMO_ACTION_WITH_DEFAULT];
+      handleSigninWithIntent:AddAccountSigninIntentAddSecondaryAccount
+                 accessPoint:signin_metrics::AccessPoint::ACCESS_POINT_SETTINGS
+                 promoAction:signin_metrics::PromoAction::
+                                 PROMO_ACTION_WITH_DEFAULT];
   [identity_interaction_manager_ addAccountViewControllerDidTapSignIn];
 
   // Account added.
@@ -145,15 +146,17 @@ TEST_F(AddAccountSigninMediatorTest, AddAccountIntent) {
 TEST_F(AddAccountSigninMediatorTest, AddAccountIntentWithUserCancel) {
   // Verify that completion was called with canceled result state.
   [[mediator_delegate_ expect]
-      addAccountSigninMediatorFinishedWith:SigninCoordinatorResultCanceledByUser
-                                  identity:nil];
-  [[mediator_delegate_ reject] addAccountSigninMediatorFailedWith:[OCMArg any]];
+      addAccountSigninMediatorFinishedWithSigninResult:
+          SigninCoordinatorResultCanceledByUser
+                                              identity:nil];
+  [[mediator_delegate_ reject]
+      addAccountSigninMediatorFailedWithError:[OCMArg any]];
 
   [mediator_
-      handleSigninIntent:SigninIntentAddAccount
-             accessPoint:signin_metrics::AccessPoint::ACCESS_POINT_SETTINGS
-             promoAction:signin_metrics::PromoAction::
-                             PROMO_ACTION_WITH_DEFAULT];
+      handleSigninWithIntent:AddAccountSigninIntentAddSecondaryAccount
+                 accessPoint:signin_metrics::AccessPoint::ACCESS_POINT_SETTINGS
+                 promoAction:signin_metrics::PromoAction::
+                                 PROMO_ACTION_WITH_DEFAULT];
   [identity_interaction_manager_ addAccountViewControllerDidTapCancel];
 
   // No account is present.
@@ -168,13 +171,14 @@ TEST_F(AddAccountSigninMediatorTest,
        AddAccountIntentWithErrorHandledByMediator) {
   // Verify that completion was called with canceled result state and an error
   // is shown.
-  [[mediator_delegate_ expect] addAccountSigninMediatorFailedWith:[OCMArg any]];
+  [[mediator_delegate_ expect]
+      addAccountSigninMediatorFailedWithError:[OCMArg any]];
 
   [mediator_
-      handleSigninIntent:SigninIntentAddAccount
-             accessPoint:signin_metrics::AccessPoint::ACCESS_POINT_SETTINGS
-             promoAction:signin_metrics::PromoAction::
-                             PROMO_ACTION_WITH_DEFAULT];
+      handleSigninWithIntent:AddAccountSigninIntentAddSecondaryAccount
+                 accessPoint:signin_metrics::AccessPoint::ACCESS_POINT_SETTINGS
+                 promoAction:signin_metrics::PromoAction::
+                                 PROMO_ACTION_WITH_DEFAULT];
   [identity_interaction_manager_
       addAccountViewControllerDidThrowUnhandledError];
 
@@ -193,15 +197,17 @@ TEST_F(AddAccountSigninMediatorTest,
 TEST_F(AddAccountSigninMediatorTest, ReauthIntentWithUserCancel) {
   // Verify that completion was called with canceled result state.
   [[mediator_delegate_ expect]
-      addAccountSigninMediatorFinishedWith:SigninCoordinatorResultCanceledByUser
-                                  identity:nil];
-  [[mediator_delegate_ reject] addAccountSigninMediatorFailedWith:[OCMArg any]];
+      addAccountSigninMediatorFinishedWithSigninResult:
+          SigninCoordinatorResultCanceledByUser
+                                              identity:nil];
+  [[mediator_delegate_ reject]
+      addAccountSigninMediatorFailedWithError:[OCMArg any]];
 
   [mediator_
-      handleSigninIntent:SigninIntentReauth
-             accessPoint:signin_metrics::AccessPoint::ACCESS_POINT_SETTINGS
-             promoAction:signin_metrics::PromoAction::
-                             PROMO_ACTION_WITH_DEFAULT];
+      handleSigninWithIntent:AddAccountSigninIntentReauthPrimaryAccount
+                 accessPoint:signin_metrics::AccessPoint::ACCESS_POINT_SETTINGS
+                 promoAction:signin_metrics::PromoAction::
+                                 PROMO_ACTION_WITH_DEFAULT];
   [identity_interaction_manager_ addAccountViewControllerDidTapCancel];
 
   // No account is present.
@@ -215,13 +221,14 @@ TEST_F(AddAccountSigninMediatorTest, ReauthIntentWithUserCancel) {
 TEST_F(AddAccountSigninMediatorTest, ReauthIntentWithErrorHandledByMediator) {
   // Verify that completion was called with canceled result state and an error
   // is shown.
-  [[mediator_delegate_ expect] addAccountSigninMediatorFailedWith:[OCMArg any]];
+  [[mediator_delegate_ expect]
+      addAccountSigninMediatorFailedWithError:[OCMArg any]];
 
   [mediator_
-      handleSigninIntent:SigninIntentReauth
-             accessPoint:signin_metrics::AccessPoint::ACCESS_POINT_SETTINGS
-             promoAction:signin_metrics::PromoAction::
-                             PROMO_ACTION_WITH_DEFAULT];
+      handleSigninWithIntent:AddAccountSigninIntentReauthPrimaryAccount
+                 accessPoint:signin_metrics::AccessPoint::ACCESS_POINT_SETTINGS
+                 promoAction:signin_metrics::PromoAction::
+                                 PROMO_ACTION_WITH_DEFAULT];
   [identity_interaction_manager_
       addAccountViewControllerDidThrowUnhandledError];
 

ios/chrome/browser/ui/authentication/signin/signin_coordinator.mm

@@ -85,7 +85,7 @@ using signin_metrics::PromoAction;
                          browser:browser
                      accessPoint:accessPoint
                      promoAction:PromoAction::PROMO_ACTION_NO_SIGNIN_PROMO
-                    signinIntent:SigninIntentAddAccount];
+                    signinIntent:AddAccountSigninIntentAddSecondaryAccount];
 }
 
 + (instancetype)
@@ -99,7 +99,7 @@ using signin_metrics::PromoAction;
                          browser:browser
                      accessPoint:accessPoint
                      promoAction:promoAction
-                    signinIntent:SigninIntentReauth];
+                    signinIntent:AddAccountSigninIntentReauthPrimaryAccount];
 }
 
 - (void)dealloc {

ios/chrome/browser/ui/authentication/signin/signin_enums.h

@@ -7,14 +7,6 @@
 
 #import <Foundation/Foundation.h>
 
-// Intent when the user begins a sign-in flow.
-typedef NS_ENUM(NSUInteger, SigninIntent) {
-  // Sign-in through the add account flow.
-  SigninIntentAddAccount,
-  // Sign-in through the reauthentication flow.
-  SigninIntentReauth,
-};
-
 // Sign-in result returned Sign-in result.
 typedef NS_ENUM(NSUInteger, SigninCoordinatorResult) {
   // Sign-in has been canceled by the user or by another reason.