Commit 518f8118 authored by Alex Ilin's avatar Alex Ilin Committed by Chromium LUCI CQ

[profiles] Support the force signin policy in the profile picker

This CL prepares the profile picker to work with the force signin
policy. The list of changes:

- Propagate IsSigninRequired() value from ProfileAttributesEntry to the
  WebUI
- Modify the profile card to display "Sign in [lock]" prompt on profiles
  that require sign-in
- Remove asserts checking that the force signin policy is disabled
- Add native handlers for launching a locked profile and creating a new
  profile when the force signin policy is enabled

Screenshot:
https://drive.google.com/file/d/1Y03Kpe_4CoO5DWLbJ79Fu5j-hjjz0xzz/view?usp=sharing

Bug: 1156096
Change-Id: I8aa315c4459ecf3e9d0faed331a3590aa92a0169
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2584002
Commit-Queue: Alex Ilin <alexilin@chromium.org>
Reviewed-by: default avatarMonica Basta <msalama@chromium.org>
Reviewed-by: default avatarDavid Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837124}
parent f75a7633
......@@ -713,6 +713,9 @@
<message name="IDS_PROFILE_PICKER_BACK_BUTTON_LABEL" desc="Label for a button that navigates user to the previous page">
Back
</message>
<message name="IDS_PROFILE_PICKER_PROFILE_CARD_NEEDS_SIGNIN_PROMPT" desc="Text on the profile card signaling that the user must sign-in in order to open a profile">
Sign in
</message>
<message name="IDS_PROFILE_PICKER_PROFILE_MENU_BUTTON_NAME" desc="Text to be spoken when the focus is set to the menu button of the profile card on the picker main screen or shown on hover.">
Customize your profile, including its name
</message>
......
6745aaa146f26bce81115fc0d62811cb841ce051
\ No newline at end of file
......@@ -236,10 +236,7 @@ js_library("manage_profiles_browser_proxy") {
}
js_library("policy_helper") {
deps = [
"//ui/webui/resources/js:assert.m",
"//ui/webui/resources/js:load_time_data.m",
]
deps = [ "//ui/webui/resources/js:load_time_data.m" ]
}
group("web_components") {
......
......@@ -21,6 +21,10 @@ element.innerHTML = `
<g id="customize-banner" viewBox="0 0 678 180" width="678" height="180" fill="none" xmlns="http://www.w3.org/2000/svg">
<path d="M70.51 115.677c-2.425 3.218-7.276 3.053-9.621-.248-6.711-9.738-6.63-23.107.97-32.928 7.762-9.903 20.538-12.957 31.453-8.5 3.639 1.65 4.852 6.272 2.426 9.408l-25.227 32.268zM531.612 112.52c1.744-1.18 4.236-.252 4.818 1.77 1.744 6.069-.582 12.811-6.064 16.351-5.566 3.624-12.544 2.95-17.279-1.18-1.578-1.433-1.412-3.961.332-5.141l18.193-11.8zM140 128.499c0 2.519-1.98 4.498-4.5 4.498-2.52.09-4.5-1.979-4.5-4.498 0-2.52 1.98-4.499 4.5-4.499 2.43 0 4.5 1.979 4.5 4.499z" fill="var(--theme-shape-color)"/><path d="M160.541 53.57c.993-4.303 5.297-7.035 9.601-6.042l18.294 4.222c4.304.993 7.035 5.297 6.042 9.602-.993 4.304-5.297 7.036-9.602 6.042L166.5 63.173c-4.304-.91-6.953-5.298-5.959-9.602z" stroke="var(--theme-shape-color)" stroke-width="2" stroke-miterlimit="10" stroke-linecap="round" stroke-linejoin="round"/><path d="M526 69c6.075 0 11-4.925 11-11s-4.925-11-11-11-11 4.925-11 11 4.925 11 11 11zM608.042 81.007L630.448 83c.944.08 1.631.876 1.545 1.753l-2.146 20.805c-.086.877-.945 1.515-1.889 1.435L605.552 105c-.944-.079-1.631-.876-1.545-1.753l2.146-20.805c.086-.877.945-1.515 1.889-1.435z" stroke="var(--theme-shape-color)" stroke-width="2"/>
</g>
<g id="lock" viewBox="0 0 48 48">
<path d="M0 0h48v48H0z" fill="none"/><path d="M36 16h-2v-4c0-5.52-4.48-10-10-10S14 6.48 14 12v4h-2c-2.21 0-4 1.79-4 4v20c0 2.21 1.79 4 4 4h24c2.21 0 4-1.79 4-4V20c0-2.21-1.79-4-4-4zM24 34c-2.21 0-4-1.79-4-4s1.79-4 4-4 4 1.79 4 4-1.79 4-4 4zm6.2-18H17.8v-4c0-3.42 2.78-6.2 6.2-6.2 3.42 0 6.2 2.78 6.2 6.2v4z"/>
</g>
</defs>
</svg>`;
document.head.appendChild(element);
......@@ -10,6 +10,7 @@ import {addSingletonGetter, sendWithPromise} from 'chrome://resources/js/cr.m.js
* profilePath: string,
* localProfileName: string,
* isSyncing: boolean,
* needsSignin: boolean,
* gaiaName: string,
* userName: string,
* isManaged: boolean,
......@@ -106,8 +107,8 @@ export class ManageProfilesBrowserProxy {
/**
* Loads Google sign in page (and silently creates a profile with the
* specified color).
* @param {number} profileColor
* specified color, if specified).
* @param {?number} profileColor
*/
loadSignInProfileCreationFlow(profileColor) {}
......
......@@ -18,6 +18,7 @@ const Pages = {
PROFILE_TYPE_CHOICE: 1,
LOCAL_PROFILE_CUSTOMIZATION: 2,
LOAD_SIGNIN: 3,
LOAD_FORCE_SIGNIN: 4,
};
/**
......@@ -35,10 +36,9 @@ export const Routes = {
*/
export const ProfileCreationSteps = {
PROFILE_TYPE_CHOICE: 'profileTypeChoice',
// Not supported yet
LOCAL_PROFILE_CUSTOMIZATION: 'localProfileCustomization',
// Not supported yet
LOAD_SIGNIN: 'loadSignIn',
LOAD_FORCE_SIGNIN: 'loadForceSignIn',
};
/**
......@@ -55,7 +55,7 @@ function computeStep(route) {
return ProfileCreationSteps.LOCAL_PROFILE_CUSTOMIZATION;
}
if (isForceSigninEnabled()) {
return ProfileCreationSteps.LOAD_SIGNIN;
return ProfileCreationSteps.LOAD_FORCE_SIGNIN;
}
return ProfileCreationSteps.PROFILE_TYPE_CHOICE;
default:
......@@ -99,6 +99,8 @@ function recordNavigation() {
break;
case ProfileCreationSteps.LOAD_SIGNIN:
page = Pages.LOAD_SIGNIN;
case ProfileCreationSteps.LOAD_FORCE_SIGNIN:
page = Pages.LOAD_FORCE_SIGNIN;
default:
assertNotReached();
}
......
......@@ -2,7 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import {assert, assertNotReached} from 'chrome://resources/js/assert.m.js';
import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
import './strings.js';
......@@ -23,11 +22,7 @@ export function isBrowserSigninAllowed() {
/** @return {boolean} */
export function isForceSigninEnabled() {
const enabled = loadTimeData.getBoolean('isForceSigninEnabled');
// Force sign in policy is not supported yet. The picker should not be shown
// in case this policy exists.
assert(!enabled);
return enabled;
return loadTimeData.getBoolean('isForceSigninEnabled');
}
/** @return {boolean} */
......
<style include="profile-picker-shared">
<style include="profile-picker-shared cr-hidden-style">
#profileCardContainer {
border-radius: inherit;
/* Allows descendants to be absolute positioned relatively to the
......@@ -59,14 +59,22 @@
right: initial;
}
#iconContainer[hidden] {
display: none;
}
iron-icon {
--iron-icon-fill-color: var(--google-grey-refresh-700);
}
#forceSigninContainer {
display: flex;
flex-direction: row;
justify-content: center;
}
#forceSigninIcon {
height: 16px;
margin: 0 4px;
width: 16px;
}
@media (prefers-color-scheme: dark) {
cr-button {
--card-background-color: var(--google-grey-800);
......@@ -95,7 +103,13 @@
<iron-icon icon="cr:domain"></iron-icon>
</div>
</div>
<div class="profile-card-info">[[profileState.gaiaName]]</div>
<div class="profile-card-info">
<div hidden="[[profileState.needsSignin]]">[[profileState.gaiaName]]</div>
<div id="forceSigninContainer" hidden="[[!profileState.needsSignin]]">
<div>$i18n{needsSigninPrompt}</div>
<iron-icon id="forceSigninIcon" icon="profiles:lock"></iron-icon>
</div>
</div>
</cr-button>
<profile-card-menu profile-state="[[profileState]]"></profile-card-menu>
</div>
......@@ -14,7 +14,7 @@ import {html, Polymer} from 'chrome://resources/polymer/v3_0/polymer/polymer_bun
import {ensureLazyLoaded} from './ensure_lazy_loaded.js';
import {AutogeneratedThemeColorInfo, ManageProfilesBrowserProxy, ManageProfilesBrowserProxyImpl} from './manage_profiles_browser_proxy.js';
import {navigateTo, NavigationBehavior, ProfileCreationSteps, Routes} from './navigation_behavior.js';
import {isProfileCreationAllowed} from './policy_helper.js';
import {isForceSigninEnabled, isProfileCreationAllowed} from './policy_helper.js';
Polymer({
is: 'profile-picker-app',
......@@ -62,7 +62,24 @@ Polymer({
return;
}
if (step == ProfileCreationSteps.LOAD_FORCE_SIGNIN) {
assert(
route == Routes.NEW_PROFILE,
'LOAD_FORCE_SIGNIN step must be a part of NEW_PROFILE route');
assert(
isForceSigninEnabled(),
'Force signin policy must be enabled to start the force signin flow');
this.manageProfilesBrowserProxy_.loadSignInProfileCreationFlow(null);
return;
}
if (step == ProfileCreationSteps.LOAD_SIGNIN) {
assert(
route == Routes.NEW_PROFILE,
'LOAD_SIGNIN step must be a part of NEW_PROFILE route');
assert(
this.currentRoute_ == Routes.NEW_PROFILE,
'NEW_PROFILE route must have been already initialized');
this.manageProfilesBrowserProxy_.loadSignInProfileCreationFlow(
this.newProfileThemeInfo.color);
return;
......
......@@ -22,11 +22,13 @@
#include "chrome/browser/profiles/profile_window.h"
#include "chrome/browser/profiles/profiles_state.h"
#include "chrome/browser/signin/identity_manager_factory.h"
#include "chrome/browser/signin/signin_util.h"
#include "chrome/browser/themes/theme_service.h"
#include "chrome/browser/themes/theme_service_factory.h"
#include "chrome/browser/ui/browser_finder.h"
#include "chrome/browser/ui/browser_navigator.h"
#include "chrome/browser/ui/browser_navigator_params.h"
#include "chrome/browser/ui/user_manager.h"
#include "chrome/browser/ui/views/accelerator_table.h"
#include "chrome/browser/ui/views/profiles/profile_picker_view_sync_delegate.h"
#include "chrome/browser/ui/webui/signin/dice_turn_sync_on_helper.h"
......@@ -333,14 +335,23 @@ void ProfilePickerView::OnProfileForSigninCreated(
return;
}
// Mark this profile ephemeral so that it is deleted upon next startup if the
// browser crashes before finishing the flow.
entry->SetIsEphemeral(true);
// Apply a new color to the profile.
auto* theme_service = ThemeServiceFactory::GetForProfile(profile);
theme_service->BuildAutogeneratedThemeFromColor(profile_color);
if (signin_util::IsForceSigninEnabled()) {
// Show the embedded sign-in flow if the force signin is enabled.
// TODO(https://crbug.com/1156096): set the local profile name at the end of
// the sign-in flow and show the profile customization bubble.
UserManagerProfileDialog::ShowForceSigninDialog(
web_view_->GetWebContents()->GetBrowserContext(), profile->GetPath());
return;
}
// Mark this profile ephemeral so that it is deleted upon next startup if the
// browser crashes before finishing the flow.
entry->SetIsEphemeral(true);
// TODO(crbug.com/1126913): Record also that we show the sign-in promo
// (it has to be plumbed from js to profile_picker_handler.cc):
// signin_metrics::RecordSigninImpressionUserActionForAccessPoint(
......
......@@ -28,7 +28,10 @@
#include "chrome/browser/ui/chrome_pages.h"
#include "chrome/browser/ui/profile_picker.h"
#include "chrome/browser/ui/signin/profile_colors_util.h"
#include "chrome/browser/ui/user_manager.h"
#include "chrome/browser/ui/webui/profile_helper.h"
#include "chrome/browser/ui/webui/signin/login_ui_service.h"
#include "chrome/browser/ui/webui/signin/login_ui_service_factory.h"
#include "chrome/common/pref_names.h"
#include "chrome/common/search/generated_colors_info.h"
#include "chrome/common/themes/autogenerated_theme_util.h"
......@@ -254,9 +257,24 @@ void ProfilePickerHandler::HandleLaunchSelectedProfile(
}
if (entry->IsSigninRequired()) {
// The new profile picker does not yet support force signin policy and
// should not be accessible for devices with this policy.
NOTREACHED();
DCHECK(signin_util::IsForceSigninEnabled());
if (entry->GetActiveTime() != base::Time()) {
// If force-sign-in is enabled, do not allow users to sign in to a
// pre-existing locked profile, as this may force unexpected profile data
// merge. We consider a profile as pre-existing if it has been actived
// previously. A pre-existed profile can still be used if it has been
// signed in with an email address matched RestrictSigninToPattern policy
// already.
LoginUIServiceFactory::GetForProfile(
Profile::FromWebUI(web_ui())->GetOriginalProfile())
->SetProfileBlockingErrorMessage();
UserManagerProfileDialog::ShowDialogAndDisplayErrorMessage(
web_ui()->GetWebContents()->GetBrowserContext());
} else {
// Fresh sign in via user manager without existing email address.
UserManagerProfileDialog::ShowForceSigninDialog(
web_ui()->GetWebContents()->GetBrowserContext(), *profile_path);
}
return;
}
......@@ -497,8 +515,16 @@ void ProfilePickerHandler::OnProfileStatisticsReceived(
void ProfilePickerHandler::HandleLoadSignInProfileCreationFlow(
const base::ListValue* args) {
DCHECK(args->GetList()[0].is_int());
SkColor profile_color = args->GetList()[0].GetInt();
CHECK_EQ(1U, args->GetSize());
SkColor profile_color;
if (args->GetList()[0].is_int()) {
profile_color = args->GetList()[0].GetInt();
} else {
// The profile color must provided in `args` unless the force signin is
// enabled.
DCHECK(signin_util::IsForceSigninEnabled());
profile_color = GenerateNewProfileColor().color;
}
ProfilePicker::SwitchToSignIn(
profile_color, base::BindOnce(&ProfilePickerHandler::OnLoadSigninFinished,
weak_factory_.GetWeakPtr()));
......@@ -557,6 +583,7 @@ base::Value ProfilePickerHandler::GetProfilesList() {
profile_entry->SetBoolPath(
"isSyncing", entry->GetSigninState() ==
SigninState::kSignedInWithConsentedPrimaryAccount);
profile_entry->SetBoolPath("needsSignin", entry->IsSigninRequired());
// GAIA name/user name can be empty, if the profile is not signed in to
// chrome.
profile_entry->SetString("gaiaName", entry->GetGAIANameToDisplay());
......
......@@ -78,6 +78,8 @@ void AddStrings(content::WebUIDataSource* html_source) {
{"addSpaceButton", IDS_PROFILE_PICKER_ADD_SPACE_BUTTON},
{"askOnStartupCheckboxText", IDS_PROFILE_PICKER_ASK_ON_STARTUP},
{"browseAsGuestButton", IDS_PROFILE_PICKER_BROWSE_AS_GUEST_BUTTON},
{"needsSigninPrompt",
IDS_PROFILE_PICKER_PROFILE_CARD_NEEDS_SIGNIN_PROMPT},
{"menu", IDS_MENU},
{"profileMenuName", IDS_PROFILE_PICKER_PROFILE_MENU_BUTTON_NAME},
{"profileMenuRemoveText", IDS_PROFILE_PICKER_PROFILE_MENU_REMOVE_TEXT},
......
......@@ -85,6 +85,7 @@ suite('ProfilePickerMainViewTest', function() {
profilePath: `profilePath${i}`,
localProfileName: `profile${i}`,
isSyncing: sync,
needsSignin: false,
gaiaName: sync ? `User${i}` : '',
userName: sync ? `User${i}@gmail.com` : '',
isManaged: i % 4 === 0,
......@@ -106,8 +107,14 @@ suite('ProfilePickerMainViewTest', function() {
profiles[i].shadowRoot.querySelectorAll('.profile-card-info');
assertEquals(profileCardInfo.length, 2);
assertEquals(
profileCardInfo[0].innerHTML, expectedProfiles[i].localProfileName);
assertEquals(profileCardInfo[1].innerHTML, expectedProfiles[i].gaiaName);
profileCardInfo[0].innerText, expectedProfiles[i].localProfileName);
assertEquals(
profiles[i].$$('#forceSigninContainer').hidden,
!expectedProfiles[i].needsSignin);
if (!expectedProfiles[i].needsSignin) {
assertEquals(
profileCardInfo[1].innerText, expectedProfiles[i].gaiaName);
}
assertEquals(
profiles[i].$$('#iconContainer').hidden,
!expectedProfiles[i].isManaged);
......@@ -251,4 +258,17 @@ suite('ProfilePickerMainViewTest', function() {
// disableAskOnStartup.
assertTrue(mainViewElement.$$('cr-checkbox').hidden);
});
test('ForceSigninIsEnabled', async function() {
loadTimeData.overrideValues({isForceSigninEnabled: true});
resetTest();
await browserProxy.whenCalled('initializeMainView');
const profiles = generateProfilesList(2);
profiles[0].needsSignin = true;
webUIListenerCallback('profiles-list-changed', [...profiles]);
flushTasks();
await verifyProfileCard(
profiles, mainViewElement.shadowRoot.querySelectorAll('profile-card'));
});
});
......@@ -37,6 +37,7 @@ export class TestManageProfilesBrowserProxy extends TestBrowserProxy {
this.profileSample = {
profilePath: 'profile1',
localProfileName: 'Work',
needsSignin: false,
isSyncing: true,
gaiaName: 'Alice',
userName: 'Alice@gmail.com',
......
......@@ -61299,6 +61299,7 @@ https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_2.7.1.pdf
<int value="1" label="profileTypeChoice"/>
<int value="2" label="localProfileCustomization"/>
<int value="3" label="loadSignIn"/>
<int value="4" label="loadForceSignIn"/>
</enum>
<enum name="ProfileResetRequestOriginEnum">
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment