[Profile creation] Handle SAML flow in a browser window

This CL adds logic for SAML sign-in flows. Whenever the gaia domain is
left in the sign-in flow, it is treated as a SAML sign-in flow, the
profile creation is finalized and the SAML signin continues in a new
browser window.

Bug: 1126913
Change-Id: I48cac367d45567ba7186eb7d4cfa05dc90a0f97f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2627426
Commit-Queue: Jan Krcal <jkrcal@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843483}

chrome/browser/profiles/profiles_state.cc

@@ -310,13 +310,17 @@ bool ArePublicSessionRestrictionsEnabled() {
 }
 
 #if !BUILDFLAG(IS_CHROMEOS_ASH)
+base::string16 GetDefaultNameForNewEnterpriseProfile() {
+  return l10n_util::GetStringUTF16(
+      IDS_SIGNIN_DICE_WEB_INTERCEPT_ENTERPRISE_PROFILE_NAME);
+}
+
 base::string16 GetDefaultNameForNewSignedInProfile(
     const AccountInfo& account_info) {
   DCHECK(account_info.IsValid());
   if (!account_info.IsManaged())
     return base::UTF8ToUTF16(account_info.given_name);
-  return l10n_util::GetStringUTF16(
-      IDS_SIGNIN_DICE_WEB_INTERCEPT_ENTERPRISE_PROFILE_NAME);
+  return GetDefaultNameForNewEnterpriseProfile();
 }
 
 base::string16 GetDefaultNameForNewSignedInProfileWithIncompleteInfo(

chrome/browser/profiles/profiles_state.h

@@ -120,6 +120,9 @@ bool IsPublicSession();
 bool ArePublicSessionRestrictionsEnabled();
 
 #if !BUILDFLAG(IS_CHROMEOS_ASH)
+// Returns the default name for a new enterprise profile.
+base::string16 GetDefaultNameForNewEnterpriseProfile();
+
 // Returns the default name for a new signed-in profile, based on
 // `account_info`.
 base::string16 GetDefaultNameForNewSignedInProfile(

chrome/browser/signin/dice_tab_helper.cc

@@ -28,11 +28,6 @@ void DiceTabHelper::InitializeSigninFlow(
   DCHECK(signin_url.is_valid());
   DCHECK(signin_url_.is_empty() || signin_url_ == signin_url);
 
-  // The signin page must be loading.
-  DCHECK(web_contents()->GetController().GetPendingEntry());
-  DCHECK_EQ(signin_url,
-            web_contents()->GetController().GetPendingEntry()->GetURL());
-
   signin_url_ = signin_url;
   signin_access_point_ = access_point;
   signin_reason_ = reason;

chrome/browser/ui/views/profiles/profile_picker_interactive_uitest.cc

@@ -27,30 +27,6 @@
 
 namespace {
 
-// Waits until a view is deleted.
-class ViewDeletedWaiter : public views::ViewObserver {
- public:
-  explicit ViewDeletedWaiter(views::View* view) {
-    DCHECK(view);
-    observation_.Observe(view);
-  }
-  ~ViewDeletedWaiter() override = default;
-
-  // Waits until the view is deleted.
-  void Wait() { run_loop_.Run(); }
-
- private:
-  // ViewObserver:
-  void OnViewIsDeleting(views::View* observed_view) override {
-    // Reset the observation before the view is actually deleted.
-    observation_.Reset();
-    run_loop_.Quit();
-  }
-
-  base::RunLoop run_loop_;
-  base::ScopedObservation<views::View, views::ViewObserver> observation_{this};
-};
-
 // Waits until the widget bounds change.
 class WidgetBoundsChangeWaiter : public views::WidgetObserver {
  public:
@@ -98,13 +74,6 @@ class ProfilePickerInteractiveUiTest : public ProfilePickerTestBase {
         widget()->GetNativeWindow(), ui::VKEY_W, control, shift, /*alt=*/false,
         command));
   }
-
-  void WaitForPickerClosed() {
-    if (!ProfilePicker::IsOpen())
-      return;
-    ViewDeletedWaiter(view()).Wait();
-    ASSERT_FALSE(ProfilePicker::IsOpen());
-  }
 };
 
 // Checks that the main picker view can be closed with keyboard shortcut.

chrome/browser/ui/views/profiles/profile_picker_test_base.cc

@@ -82,6 +82,30 @@ class FirstVisuallyNonEmptyPaintObserver : public content::WebContentsObserver {
   GURL url_;
 };
 
+// Waits until a view is deleted.
+class ViewDeletedWaiter : public views::ViewObserver {
+ public:
+  explicit ViewDeletedWaiter(views::View* view) {
+    DCHECK(view);
+    observation_.Observe(view);
+  }
+  ~ViewDeletedWaiter() override = default;
+
+  // Waits until the view is deleted.
+  void Wait() { run_loop_.Run(); }
+
+ private:
+  // ViewObserver:
+  void OnViewIsDeleting(views::View* observed_view) override {
+    // Reset the observation before the view is actually deleted.
+    observation_.Reset();
+    run_loop_.Quit();
+  }
+
+  base::RunLoop run_loop_;
+  base::ScopedObservation<views::View, views::ViewObserver> observation_{this};
+};
+
 }  // namespace
 
 ProfilePickerTestBase::ProfilePickerTestBase() {
@@ -120,6 +144,13 @@ void ProfilePickerTestBase::WaitForFirstPaint(content::WebContents* contents,
   FirstVisuallyNonEmptyPaintObserver(contents, url).Wait();
 }
 
+void ProfilePickerTestBase::WaitForPickerClosed() {
+  if (!ProfilePicker::IsOpen())
+    return;
+  ViewDeletedWaiter(view()).Wait();
+  ASSERT_FALSE(ProfilePicker::IsOpen());
+}
+
 content::WebContents* ProfilePickerTestBase::web_contents() {
   if (!web_view())
     return nullptr;

chrome/browser/ui/views/profiles/profile_picker_test_base.h

@@ -42,6 +42,9 @@ class ProfilePickerTestBase : public InProcessBrowserTest {
   // Waits until the web contents does the first non-empty paint for `url`.
   void WaitForFirstPaint(content::WebContents* contents, const GURL& url);
 
+  // Waits until the picker gets closed.
+  void WaitForPickerClosed();
+
   // Gets the picker's web contents.
   content::WebContents* web_contents();
 

chrome/browser/ui/views/profiles/profile_picker_view.cc

@@ -21,6 +21,7 @@
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/profiles/profile_window.h"
 #include "chrome/browser/profiles/profiles_state.h"
+#include "chrome/browser/signin/dice_tab_helper.h"
 #include "chrome/browser/signin/identity_manager_factory.h"
 #include "chrome/browser/signin/signin_util.h"
 #include "chrome/browser/themes/theme_properties.h"
@@ -58,6 +59,7 @@
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/web_contents.h"
+#include "google_apis/gaia/gaia_auth_util.h"
 #include "google_apis/gaia/gaia_urls.h"
 #include "net/base/url_util.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -147,6 +149,35 @@ GURL GetSigninURL() {
   return signin_url;
 }
 
+bool IsExternalURL(const GURL& url) {
+  // Empty URL is used initially, about:blank is used to stop navigation after
+  // sign-in succeeds.
+  if (url.is_empty() || url == GURL(url::kAboutBlankURL))
+    return false;
+  if (gaia::IsGaiaSignonRealm(url.GetOrigin()))
+    return false;
+  return true;
+}
+
+void ContinueSAMLSignin(std::unique_ptr<content::WebContents> saml_wc,
+                        Browser* browser) {
+  DCHECK(browser);
+
+  // Attach DiceTabHelper to `saml_wc` so that sync consent dialog appears after
+  // a successful sign-in.
+  DiceTabHelper::CreateForWebContents(saml_wc.get());
+  DiceTabHelper* tab_helper = DiceTabHelper::FromWebContents(saml_wc.get());
+  // Use |redirect_url| and not |continue_url|, so that the DiceTabHelper can
+  // redirect to chrome:// URLs such as the NTP.
+  tab_helper->InitializeSigninFlow(
+      GetSigninURL(), signin_metrics::AccessPoint::ACCESS_POINT_USER_MANAGER,
+      signin_metrics::Reason::REASON_SIGNIN_PRIMARY_ACCOUNT,
+      signin_metrics::PromoAction::PROMO_ACTION_NO_SIGNIN_PROMO,
+      GURL(chrome::kChromeUINewTabURL));
+
+  browser->tab_strip_model()->ReplaceWebContentsAt(0, std::move(saml_wc));
+}
+
 class ProfilePickerWidget : public views::Widget {
  public:
   explicit ProfilePickerWidget(ProfilePickerView* profile_picker_view)
@@ -306,7 +337,12 @@ ProfilePickerView::ProfilePickerView()
   // TODO(crbug.com/1063856): Add |RecordDialogCreation|.
 }
 
-ProfilePickerView::~ProfilePickerView() = default;
+ProfilePickerView::~ProfilePickerView() {
+  if (new_profile_contents_)
+    new_profile_contents_->SetDelegate(nullptr);
+  if (system_profile_contents_)
+    system_profile_contents_->SetDelegate(nullptr);
+}
 
 void ProfilePickerView::Display(ProfilePicker::EntryPoint entry_point) {
   // Record creation metrics.
@@ -627,6 +663,13 @@ void ProfilePickerView::AddNewContents(
   Navigate(&params);
 }
 
+void ProfilePickerView::NavigationStateChanged(
+    content::WebContents* source,
+    content::InvalidateTypes changed_flags) {
+  if (IsSigningIn() && IsExternalURL(new_profile_contents_->GetVisibleURL()))
+    FinishSignedInCreationFlowForSAML();
+}
+
 void ProfilePickerView::BuildLayout() {
   SetLayoutManager(std::make_unique<views::FlexLayout>())
       ->SetOrientation(views::LayoutOrientation::kVertical)
@@ -647,6 +690,7 @@ void ProfilePickerView::BuildLayout() {
       views::kFlexBehaviorKey,
       views::FlexSpecification(views::MinimumFlexSizeRule::kPreferred,
                                views::MaximumFlexSizeRule::kPreferred));
+  // It is important for tests that the toolbar starts visible (being empty).
   toolbar_ = AddChildView(std::move(toolbar));
 
   auto web_view = std::make_unique<views::WebView>();
@@ -657,21 +701,24 @@ void ProfilePickerView::BuildLayout() {
 void ProfilePickerView::ShowScreen(content::WebContents* contents,
                                    const GURL& url,
                                    bool show_toolbar) {
-  if (!url.is_empty()) {
-    contents->GetController().LoadURL(url, content::Referrer(),
-                                      ui::PAGE_TRANSITION_AUTO_TOPLEVEL,
-                                      std::string());
-  }
   web_view_->SetWebContents(contents);
   web_view_->RequestFocus();
 
   // Change visibility of the toolbar after swapping wc in `web_view_` to make
   // it easier for tests to detect changing of the screen.
   toolbar_->SetVisible(show_toolbar);
+
+  if (!url.is_empty()) {
+    // Make sure to load the url as the last step so that the UI state is
+    // coherent upon the NavigationStateChanged notification.
+    contents->GetController().LoadURL(url, content::Referrer(),
+                                      ui::PAGE_TRANSITION_AUTO_TOPLEVEL,
+                                      std::string());
+  }
 }
 
 void ProfilePickerView::BackButtonPressed(const ui::Event& event) {
-  if (web_view_->GetWebContents() != new_profile_contents_.get()) {
+  if (!IsSigningIn()) {
     return;
   }
 
@@ -686,6 +733,10 @@ void ProfilePickerView::BackButtonPressed(const ui::Event& event) {
   ShowScreen(system_profile_contents_.get(), GURL(), /*show_toolbar=*/false);
 }
 
+bool ProfilePickerView::IsSigningIn() const {
+  return (state_ == kReady || state_ == kFinalizing) && toolbar_->GetVisible();
+}
+
 void ProfilePickerView::OnRefreshTokenUpdatedForAccount(
     const CoreAccountInfo& account_info) {
   DCHECK(!account_info.IsEmpty());
@@ -782,6 +833,23 @@ void ProfilePickerView::FinishSignedInCreationFlow(
                                  enterprise_sync_consent_needed);
 }
 
+void ProfilePickerView::FinishSignedInCreationFlowForSAML() {
+  DCHECK_NE(state_, kFinalizing);
+  state_ = kFinalizing;
+  DCHECK(name_for_signed_in_profile_.empty());
+
+  name_for_signed_in_profile_ =
+      profiles::GetDefaultNameForNewEnterpriseProfile();
+
+  // Free up `new_profile_contents_` to be moved to a new browser window.
+  new_profile_contents_->SetDelegate(nullptr);
+  ShowScreen(system_profile_contents_.get(), GURL(url::kAboutBlankURL),
+             /*show_toolbar=*/false);
+  FinishSignedInCreationFlowImpl(
+      base::BindOnce(&ContinueSAMLSignin, std::move(new_profile_contents_)),
+      /*enterprise_sync_consent_needed=*/true);
+}
+
 void ProfilePickerView::FinishSignedInCreationFlowImpl(
     BrowserOpenedCallback callback,
     bool enterprise_sync_consent_needed) {

chrome/browser/ui/views/profiles/profile_picker_view.h

@@ -99,6 +99,8 @@ class ProfilePickerView : public views::WidgetDelegateView,
   bool HandleKeyboardEvent(
       content::WebContents* source,
       const content::NativeWebKeyboardEvent& event) override;
+  void NavigationStateChanged(content::WebContents* source,
+                              content::InvalidateTypes changed_flags) override;
 
   // IdentityManager::Observer:
   void OnRefreshTokenUpdatedForAccount(
@@ -116,6 +118,9 @@ class ProfilePickerView : public views::WidgetDelegateView,
 
   void BackButtonPressed(const ui::Event& event);
 
+  // Checks whether the sign-in flow is in progress.
+  bool IsSigningIn() const;
+
   // Helper functions to deal with the lack of extended account info.
   void SetExtendedAccountInfoTimeoutForTesting(base::TimeDelta timeout);
   void OnExtendedAccountInfoTimeout(const CoreAccountInfo& account);
@@ -128,6 +133,10 @@ class ProfilePickerView : public views::WidgetDelegateView,
   void FinishSignedInCreationFlowImpl(BrowserOpenedCallback callback,
                                       bool enterprise_sync_consent_needed);
 
+  // Finishes the flow by finalizing the profile and continuing the SAML sign-in
+  // in a browser window.
+  void FinishSignedInCreationFlowForSAML();
+
   // Internal callback to finish the last steps of the signed-in creation flow.
   void OnBrowserOpened(BrowserOpenedCallback finish_flow_callback,
                        Profile* profile,

chrome/browser/ui/views/profiles/profile_picker_view_browsertest.cc

@@ -15,6 +15,7 @@
 #include "chrome/browser/profiles/profile_attributes_entry.h"
 #include "chrome/browser/profiles/profile_attributes_storage.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/signin/dice_tab_helper.h"
 #include "chrome/browser/signin/identity_manager_factory.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/themes/theme_service.h"
@@ -307,7 +308,7 @@ IN_PROC_BROWSER_TEST_F(ProfilePickerCreationFlowBrowserTest,
                     GURL("chrome://newtab/"));
 
   // Check expectations when the profile creation flow is done.
-  EXPECT_FALSE(ProfilePicker::IsOpen());
+  WaitForPickerClosed();
 
   ProfileAttributesEntry* entry = nullptr;
   ASSERT_TRUE(g_browser_process->profile_manager()
@@ -351,7 +352,7 @@ IN_PROC_BROWSER_TEST_F(ProfilePickerCreationFlowBrowserTest,
   WaitForFirstPaint(new_browser->tab_strip_model()->GetActiveWebContents(),
                     GURL("chrome://newtab/"));
 
-  EXPECT_FALSE(ProfilePicker::IsOpen());
+  WaitForPickerClosed();
 
   // Now the sync consent screen is shown, simulate closing the UI with "Stay
   // signed in"
@@ -400,7 +401,7 @@ IN_PROC_BROWSER_TEST_F(ProfilePickerCreationFlowBrowserTest,
                     GURL("chrome://settings/syncSetup"));
 
   // Check expectations when the profile creation flow is done.
-  EXPECT_FALSE(ProfilePicker::IsOpen());
+  WaitForPickerClosed();
 
   ProfileAttributesEntry* entry = nullptr;
   ASSERT_TRUE(g_browser_process->profile_manager()
@@ -488,7 +489,7 @@ IN_PROC_BROWSER_TEST_F(ProfilePickerCreationFlowBrowserTest,
                     GURL("chrome://newtab/"));
 
   // Check expectations when the profile creation flow is done.
-  EXPECT_FALSE(ProfilePicker::IsOpen());
+  WaitForPickerClosed();
 
   ProfileAttributesEntry* entry = nullptr;
   ASSERT_TRUE(storage.GetProfileAttributesWithPath(
@@ -531,7 +532,7 @@ IN_PROC_BROWSER_TEST_F(ProfilePickerCreationFlowBrowserTest,
                     GURL("chrome://newtab/"));
 
   // Check expectations when the profile creation flow is done.
-  EXPECT_FALSE(ProfilePicker::IsOpen());
+  WaitForPickerClosed();
 
   ProfileAttributesEntry* entry = nullptr;
   ASSERT_TRUE(g_browser_process->profile_manager()
@@ -549,6 +550,38 @@ IN_PROC_BROWSER_TEST_F(ProfilePickerCreationFlowBrowserTest,
             kProfileColor);
 }
 
+IN_PROC_BROWSER_TEST_F(ProfilePickerCreationFlowBrowserTest,
+                       CreateSignedInProfileWithSAML) {
+  const GURL kNonGaiaURL("https://signin.saml-provider.com/");
+
+  ASSERT_EQ(1u, BrowserList::GetInstance()->size());
+  Profile* profile_being_created = StartSigninFlow();
+
+  // Redirect the web contents to a non gaia url (simulating a SAML page).
+  content::WebContents* wc = web_contents();
+  wc->GetController().LoadURL(kNonGaiaURL, content::Referrer(),
+                              ui::PAGE_TRANSITION_AUTO_TOPLEVEL, std::string());
+  Browser* new_browser = BrowserAddedWaiter(2u).Wait();
+  WaitForFirstPaint(wc, kNonGaiaURL);
+  WaitForPickerClosed();
+
+  // Check that the web contents got actually moved to the browser.
+  EXPECT_EQ(wc, new_browser->tab_strip_model()->GetActiveWebContents());
+  EXPECT_NE(nullptr, DiceTabHelper::FromWebContents(wc));
+
+  ProfileAttributesEntry* entry = nullptr;
+  ASSERT_TRUE(g_browser_process->profile_manager()
+                  ->GetProfileAttributesStorage()
+                  .GetProfileAttributesWithPath(
+                      profile_being_created->GetPath(), &entry));
+  EXPECT_FALSE(entry->IsEphemeral());
+  EXPECT_FALSE(entry->IsAuthenticated());
+  EXPECT_EQ(entry->GetLocalProfileName(), base::UTF8ToUTF16(kWork));
+  // The color is not applied if the user enters the SAML flow.
+  EXPECT_FALSE(ThemeServiceFactory::GetForProfile(profile_being_created)
+                   ->UsingAutogeneratedTheme());
+}
+
 class ProfilePickerEnterpriseCreationFlowBrowserTest
     : public ProfilePickerCreationFlowBrowserTest {
  public:
@@ -598,7 +631,7 @@ IN_PROC_BROWSER_TEST_F(ProfilePickerEnterpriseCreationFlowBrowserTest,
   // renders, it is safe to check.
   WaitForFirstPaint(new_browser->tab_strip_model()->GetActiveWebContents(),
                     GURL("chrome://newtab/"));
-  EXPECT_FALSE(ProfilePicker::IsOpen());
+  WaitForPickerClosed();
 
   // Now the sync consent screen is shown, simulate closing the UI with "No,
   // thanks".
@@ -657,7 +690,7 @@ IN_PROC_BROWSER_TEST_F(ProfilePickerEnterpriseCreationFlowBrowserTest,
                     GURL("chrome://settings/syncSetup"));
 
   // Check expectations when the profile creation flow is done.
-  EXPECT_FALSE(ProfilePicker::IsOpen());
+  WaitForPickerClosed();
 
   ProfileAttributesEntry* entry = nullptr;
   ASSERT_TRUE(g_browser_process->profile_manager()
@@ -791,7 +824,7 @@ IN_PROC_BROWSER_TEST_P(ProfilePickerCreationFlowEphemeralProfileBrowserTest,
   WaitForFirstPaint(new_browser->tab_strip_model()->GetActiveWebContents(),
                     GURL("chrome://newtab/"));
 
-  EXPECT_FALSE(ProfilePicker::IsOpen());
+  WaitForPickerClosed();
   EXPECT_EQ(2u, profile_manager()->GetNumberOfProfiles());
   EXPECT_EQ(entry->GetLocalProfileName(), base::UTF8ToUTF16("Joe"));
   // The profile is no longer ephemeral, unless the policy is enabled.