Skip to content

Commit

Permalink
Show the link data checkbox in all interception dialogs if no policy …
Browse files Browse the repository at this point in the history
…is set

Every time the enterprise interception/welcome dialog is shown, the
default behavior should be to create a new profile unless the user has
the choice to "keep existing data" in which case the existing profile
will be used.

Visual: http://screen/BvoPtrRcFy7pzjh

(cherry picked from commit 1617550)

Bug: 1317969, 1322204
Change-Id: If6efcc99fbc6ddede6379dfa704623b0e715de5e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3625730
Reviewed-by: David Roger <droger@chromium.org>
Commit-Queue: Yann Dago <ydago@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1001668}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3649347
Commit-Queue: David Roger <droger@chromium.org>
Auto-Submit: Yann Dago <ydago@chromium.org>
Cr-Commit-Position: refs/branch-heads/5005@{#804}
Cr-Branched-From: 5b4d945-refs/heads/main@{#992738}
  • Loading branch information
Yann Dago authored and Chromium LUCI CQ committed May 17, 2022
1 parent 14e9af1 commit 5d6e0ec
Show file tree
Hide file tree
Showing 11 changed files with 81 additions and 50 deletions.
1 change: 1 addition & 0 deletions chrome/browser/signin/dice_web_signin_interceptor.cc
Expand Up @@ -541,6 +541,7 @@ void DiceWebSigninInterceptor::OnInterceptionReadyToBeProcessed(
}
} else if (ShouldShowEnterpriseDialog(info)) {
interception_type = SigninInterceptionType::kEnterpriseAcceptManagement;
show_link_data_option = true;
RecordSigninInterceptionHeuristicOutcome(
SigninInterceptionHeuristicOutcome::kInterceptEnterprise);
} else if (!profile_->GetPrefs()->GetBoolean(
Expand Down
14 changes: 8 additions & 6 deletions chrome/browser/ui/signin/dice_web_signin_interceptor_delegate.cc
Expand Up @@ -47,9 +47,10 @@ class ForcedEnterpriseSigninInterceptionHandle
bubble_parameters,
base::OnceCallback<void(SigninInterceptionResult)> callback)
: browser_(browser),
force_new_profile_(bubble_parameters.interception_type ==
DiceWebSigninInterceptor::SigninInterceptionType::
kEnterpriseForced),
profile_creation_required_by_policy_(
bubble_parameters.interception_type ==
DiceWebSigninInterceptor::SigninInterceptionType::
kEnterpriseForced),
show_link_data_option_(bubble_parameters.show_link_data_option),
callback_(std::move(callback)) {
DCHECK(browser_);
Expand All @@ -69,7 +70,8 @@ class ForcedEnterpriseSigninInterceptionHandle
void ShowEnterpriseProfileInterceptionDialog(const AccountInfo& account_info,
SkColor profile_color) {
browser_->signin_view_controller()->ShowModalEnterpriseConfirmationDialog(
account_info, force_new_profile_, show_link_data_option_, profile_color,
account_info, profile_creation_required_by_policy_,
show_link_data_option_, profile_color,
base::BindOnce(&ForcedEnterpriseSigninInterceptionHandle::
OnEnterpriseInterceptionDialogClosed,
base::Unretained(this)));
Expand All @@ -81,7 +83,7 @@ class ForcedEnterpriseSigninInterceptionHandle
std::move(callback_).Run(SigninInterceptionResult::kAccepted);
break;
case signin::SIGNIN_CHOICE_CONTINUE:
DCHECK(!force_new_profile_ || show_link_data_option_);
DCHECK(!profile_creation_required_by_policy_ || show_link_data_option_);
std::move(callback_).Run(
SigninInterceptionResult::kAcceptedWithExistingProfile);
break;
Expand All @@ -96,7 +98,7 @@ class ForcedEnterpriseSigninInterceptionHandle
}

raw_ptr<Browser> browser_;
const bool force_new_profile_;
const bool profile_creation_required_by_policy_;
const bool show_link_data_option_;
base::OnceCallback<void(SigninInterceptionResult)> callback_;
};
Expand Down
5 changes: 4 additions & 1 deletion chrome/browser/ui/signin_view_controller.h
Expand Up @@ -142,9 +142,12 @@ class SigninViewController {
// top of the `browser_`'s window. `domain_name` is the domain of the
// enterprise account being shown. `callback` is called with the user's action
// on the dialog.
// If `profile_creation_required_by_policy` is true, the wording of the dialog
// will tell the user that an admin requires a new profile for the account,
// otherwise the default wording will be used.
void ShowModalEnterpriseConfirmationDialog(
const AccountInfo& account_info,
bool force_new_profile,
bool profile_creation_required_by_policy,
bool show_link_data_option,
SkColor profile_color,
signin::SigninChoiceCallback callback);
Expand Down
Expand Up @@ -165,8 +165,9 @@ void ProfilePickerSignedInFlowController::
/*browser=*/nullptr, type,
IdentityManagerFactory::GetForProfile(profile_)
->FindExtendedAccountInfoByEmailAddress(email_),
/*force_new_profile_=*/false, /*show_link_data_option=*/false,
GetProfileColor(), std::move(proceed_callback));
/*profile_creation_required_by_policy=*/false,
/*show_link_data_option=*/false, GetProfileColor(),
std::move(proceed_callback));
}

bool ProfilePickerSignedInFlowController::IsInitialized() const {
Expand Down
Expand Up @@ -145,7 +145,7 @@ std::unique_ptr<views::WebView>
SigninViewControllerDelegateViews::CreateEnterpriseConfirmationWebView(
Browser* browser,
const AccountInfo& account_info,
bool force_new_profile,
bool profile_creation_required_by_policy,
bool show_link_data_option,
SkColor profile_color,
signin::SigninChoiceCallback callback) {
Expand All @@ -163,8 +163,8 @@ SigninViewControllerDelegateViews::CreateEnterpriseConfirmationWebView(
web_dialog_ui->Initialize(
browser,
EnterpriseProfileWelcomeUI::ScreenType::kEnterpriseAccountCreation,
account_info, force_new_profile, show_link_data_option, profile_color,
std::move(callback));
account_info, profile_creation_required_by_policy, show_link_data_option,
profile_color, std::move(callback));

return web_view;
}
Expand Down Expand Up @@ -427,14 +427,14 @@ SigninViewControllerDelegate*
SigninViewControllerDelegate::CreateEnterpriseConfirmationDelegate(
Browser* browser,
const AccountInfo& account_info,
bool force_new_profile,
bool profile_creation_required_by_policy,
bool show_link_data_option,
SkColor profile_color,
signin::SigninChoiceCallback callback) {
return new SigninViewControllerDelegateViews(
SigninViewControllerDelegateViews::CreateEnterpriseConfirmationWebView(
browser, account_info, force_new_profile, show_link_data_option,
profile_color, std::move(callback)),
browser, account_info, profile_creation_required_by_policy,
show_link_data_option, profile_color, std::move(callback)),
browser, ui::MODAL_TYPE_WINDOW, true, false);
}
#endif
Expand Up @@ -76,7 +76,7 @@ class SigninViewControllerDelegateViews
static std::unique_ptr<views::WebView> CreateEnterpriseConfirmationWebView(
Browser* browser,
const AccountInfo& account_info,
bool force_new_profile,
bool profile_creation_required_by_policy,
bool show_link_data_option,
SkColor profile_color,
signin::SigninChoiceCallback callback);
Expand Down
Expand Up @@ -110,13 +110,13 @@ SkColor GetHighlightColor(absl::optional<SkColor> theme_color) {
EnterpriseProfileWelcomeHandler::EnterpriseProfileWelcomeHandler(
Browser* browser,
EnterpriseProfileWelcomeUI::ScreenType type,
bool force_new_profile,
bool profile_creation_required_by_policy,
const AccountInfo& account_info,
absl::optional<SkColor> profile_color,
signin::SigninChoiceCallback proceed_callback)
: browser_(browser),
type_(type),
force_new_profile_(force_new_profile),
profile_creation_required_by_policy_(profile_creation_required_by_policy),
email_(base::UTF8ToUTF16(account_info.email)),
domain_name_(gaia::ExtractDomainName(account_info.email)),
account_id_(account_info.account_id),
Expand Down Expand Up @@ -222,9 +222,8 @@ void EnterpriseProfileWelcomeHandler::HandleProceed(
if (proceed_callback_) {
bool use_existing_profile = args[0].GetIfBool().value_or(false);
std::move(proceed_callback_)
.Run(use_existing_profile || !force_new_profile_
? signin::SIGNIN_CHOICE_CONTINUE
: signin::SIGNIN_CHOICE_NEW_PROFILE);
.Run(use_existing_profile ? signin::SIGNIN_CHOICE_CONTINUE
: signin::SIGNIN_CHOICE_NEW_PROFILE);
}
}

Expand Down Expand Up @@ -287,7 +286,7 @@ base::Value EnterpriseProfileWelcomeHandler::GetProfileInfoValue() {
dict.SetStringKey(
"proceedLabel",
l10n_util::GetStringUTF8(
force_new_profile_
profile_creation_required_by_policy_
? IDS_ENTERPRISE_PROFILE_WELCOME_CREATE_PROFILE_BUTTON
: IDS_WELCOME_SIGNIN_VIEW_SIGNIN));
break;
Expand Down
Expand Up @@ -40,7 +40,7 @@ class EnterpriseProfileWelcomeHandler
EnterpriseProfileWelcomeHandler(
Browser* browser,
EnterpriseProfileWelcomeUI::ScreenType type,
bool force_new_profile,
bool profile_creation_required_by_policy,
const AccountInfo& account_info,
absl::optional<SkColor> profile_color,
signin::SigninChoiceCallback proceed_callback);
Expand Down Expand Up @@ -110,7 +110,7 @@ class EnterpriseProfileWelcomeHandler

raw_ptr<Browser> browser_ = nullptr;
const EnterpriseProfileWelcomeUI::ScreenType type_;
const bool force_new_profile_;
const bool profile_creation_required_by_policy_;
const std::u16string email_;
const std::string domain_name_;
const CoreAccountId account_id_;
Expand Down
Expand Up @@ -64,21 +64,21 @@ void EnterpriseProfileWelcomeUI::Initialize(
Browser* browser,
EnterpriseProfileWelcomeUI::ScreenType type,
const AccountInfo& account_info,
bool force_new_profile,
bool profile_creation_required_by_policy,
bool show_link_data_option,
absl::optional<SkColor> profile_color,
signin::SigninChoiceCallback proceed_callback) {
auto handler = std::make_unique<EnterpriseProfileWelcomeHandler>(
browser, type, force_new_profile, account_info, profile_color,
std::move(proceed_callback));
browser, type, profile_creation_required_by_policy, account_info,
profile_color, std::move(proceed_callback));
handler_ = handler.get();

if (type ==
EnterpriseProfileWelcomeUI::ScreenType::kEnterpriseAccountCreation) {
base::Value::Dict update_data;
update_data.Set("isModalDialog", true);

int title_id = force_new_profile
int title_id = profile_creation_required_by_policy
? IDS_ENTERPRISE_WELCOME_PROFILE_REQUIRED_TITLE
: IDS_ENTERPRISE_WELCOME_PROFILE_WILL_BE_MANAGED_TITLE;
update_data.Set("enterpriseProfileWelcomeTitle",
Expand Down
15 changes: 13 additions & 2 deletions chrome/browser/ui/webui/signin/enterprise_profile_welcome_ui.h
Expand Up @@ -44,11 +44,22 @@ class EnterpriseProfileWelcomeUI : public content::WebUIController {
EnterpriseProfileWelcomeUI& operator=(const EnterpriseProfileWelcomeUI&) =
delete;

// Initializes the EnterpriseProfileWelcomeUI.

// Initializes the EnterpriseProfileWelcomeUI, which will obtain the user's
// choice about how to set up the profile with the new account.
// `proceed_callback` will be called when the user performs an action to exit
// the screen. Their choice will depend on other flags passed to this method.
// If `profile_creation_required_by_policy` is true, the wording of the dialog
// will tell the user that an admin requires a new profile for the account,
// otherwise the default wording will be used.
// `show_link_data_option` will make the screen display a checkbox, and when
// selected, will indicate that the user wants the current profile to be used
// as dedicated profile for the new account, linking the current data with
// synced data from the new account.
void Initialize(Browser* browser,
ScreenType type,
const AccountInfo& account_info,
bool force_new_profile,
bool profile_creation_required_by_policy,
bool show_link_data_option,
absl::optional<SkColor> profile_color,
signin::SigninChoiceCallback proceed_callback);
Expand Down
54 changes: 34 additions & 20 deletions chrome/browser/ui/webui/signin/turn_sync_on_helper_delegate_impl.cc
Expand Up @@ -10,6 +10,7 @@
#include "base/metrics/user_metrics_action.h"
#include "base/notreached.h"
#include "base/strings/utf_string_conversions.h"
#include "build/chromeos_buildflags.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/net/system_network_context_manager.h"
#include "chrome/browser/new_tab_page/chrome_colors/selected_colors_info.h"
Expand Down Expand Up @@ -105,6 +106,11 @@ void TurnSyncOnHelperDelegateImpl::ShowEnterpriseAccountConfirmation(
const AccountInfo& account_info,
signin::SigninChoiceCallback callback) {
browser_ = EnsureBrowser(browser_, profile_);
#if BUILDFLAG(IS_CHROMEOS_LACROS)
// Profile Separation Enforced is not supported on Lacros.
OnProfileCheckComplete(account_info, std::move(callback),
/*prompt_for_new_profile=*/false);
#else
account_level_signin_restriction_policy_fetcher_ =
std::make_unique<policy::UserCloudSigninRestrictionPolicyFetcher>(
g_browser_process->browser_policy_connector(),
Expand All @@ -117,6 +123,7 @@ void TurnSyncOnHelperDelegateImpl::ShowEnterpriseAccountConfirmation(
base::BindOnce(&TurnSyncOnHelperDelegateImpl::OnProfileCheckComplete,
weak_ptr_factory_.GetWeakPtr(), account_info,
std::move(callback)));
#endif // BUILDFLAG(IS_CHROMEOS_LACROS)
}

void TurnSyncOnHelperDelegateImpl::ShowSyncConfirmation(
Expand Down Expand Up @@ -180,6 +187,7 @@ void TurnSyncOnHelperDelegateImpl::OnBrowserRemoved(Browser* browser) {
browser_ = nullptr;
}

#if !BUILDFLAG(IS_CHROMEOS_LACROS)
void TurnSyncOnHelperDelegateImpl::OnProfileSigninRestrictionsFetched(
const AccountInfo& account_info,
signin::SigninChoiceCallback callback,
Expand All @@ -192,11 +200,14 @@ void TurnSyncOnHelperDelegateImpl::OnProfileSigninRestrictionsFetched(
g_browser_process->profile_manager()
->GetProfileAttributesStorage()
.GetProfileAttributesWithPath(browser_->profile()->GetPath());
auto force_new_profile = signin_util::ProfileSeparationEnforcedByPolicy(
browser_->profile(), signin_restriction);
auto profile_creation_required_by_policy =
signin_util::ProfileSeparationEnforcedByPolicy(browser_->profile(),
signin_restriction);
bool show_link_data_option = signin_util::
ProfileSeparationAllowsKeepingUnmanagedBrowsingDataInManagedProfile(
browser_->profile(), signin_restriction);
browser_->signin_view_controller()->ShowModalEnterpriseConfirmationDialog(
account_info, force_new_profile,
/*show_link_data_option=*/!force_new_profile,
account_info, profile_creation_required_by_policy, show_link_data_option,
GenerateNewProfileColor(entry).color,
base::BindOnce(
[](signin::SigninChoiceCallback callback, Browser* browser,
Expand All @@ -206,6 +217,7 @@ void TurnSyncOnHelperDelegateImpl::OnProfileSigninRestrictionsFetched(
},
std::move(callback), browser_.get()));
}
#endif

void TurnSyncOnHelperDelegateImpl::OnProfileCheckComplete(
const AccountInfo& account_info,
Expand All @@ -215,11 +227,7 @@ void TurnSyncOnHelperDelegateImpl::OnProfileCheckComplete(
std::move(callback).Run(signin::SIGNIN_CHOICE_CANCEL);
return;
}
ProfileAttributesEntry* entry =
g_browser_process->profile_manager()
->GetProfileAttributesStorage()
.GetProfileAttributesWithPath(browser_->profile()->GetPath());

#if !BUILDFLAG(IS_CHROMEOS_LACROS)
if (prompt_for_new_profile) {
account_level_signin_restriction_policy_fetcher_
->GetManagedAccountsSigninRestriction(
Expand All @@ -229,16 +237,22 @@ void TurnSyncOnHelperDelegateImpl::OnProfileCheckComplete(
OnProfileSigninRestrictionsFetched,
weak_ptr_factory_.GetWeakPtr(), account_info,
std::move(callback)));
} else {
browser_->signin_view_controller()->ShowModalEnterpriseConfirmationDialog(
account_info, /*force_new_profile=*/false,
/*show_link_data_option*/ false, GenerateNewProfileColor(entry).color,
base::BindOnce(
[](signin::SigninChoiceCallback callback, Browser* browser,
signin::SigninChoice choice) {
browser->signin_view_controller()->CloseModalSignin();
std::move(callback).Run(choice);
},
std::move(callback), browser_.get()));
return;
}
#endif
DCHECK(!prompt_for_new_profile);
ProfileAttributesEntry* entry =
g_browser_process->profile_manager()
->GetProfileAttributesStorage()
.GetProfileAttributesWithPath(browser_->profile()->GetPath());
browser_->signin_view_controller()->ShowModalEnterpriseConfirmationDialog(
account_info, /*profile_creation_required_by_policy=*/false,
/*show_link_data_option*/ false, GenerateNewProfileColor(entry).color,
base::BindOnce(
[](signin::SigninChoiceCallback callback, Browser* browser,
signin::SigninChoice choice) {
browser->signin_view_controller()->CloseModalSignin();
std::move(callback).Run(choice);
},
std::move(callback), browser_.get()));
}

0 comments on commit 5d6e0ec

Please sign in to comment.