Skip to content

Commit

Permalink
[SAML] Add ability to switch to GAIA page on the lock screen
Browse files Browse the repository at this point in the history
Users who had SAML configured but then changed SSO profile to "None" are
still being redirected to the 3P IdP login page on the lock screen and
can't unlock their device. This cl implements a workaround with "Enter
Google Account Info" button which allows to switch to GAIA page.

Bug: b/259184054, b/259179721, b/250021842
Change-Id: Id05428fee3449306d1978927f26c4fe3c7bf145b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4020200
Commit-Queue: Andrey Davydov <andreydav@google.com>
Reviewed-by: Danila Kuzmin <dkuzmin@google.com>
Reviewed-by: Roman Sorokin <rsorokin@google.com>
Cr-Commit-Position: refs/heads/main@{#1073335}
  • Loading branch information
Andrey Davydov authored and Chromium LUCI CQ committed Nov 18, 2022
1 parent bb3ea0c commit d68ae89
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const test::UIPath kMainCancelButton = {"main-element",
const test::UIPath kErrorCancelButton = {"main-element",
"cancelButtonErrorScreen"};
const test::UIPath kSamlCancelButton = {"main-element", "saml-close-button"};
const test::UIPath kChangeIdPButton = {"main-element", "change-account"};
const test::UIPath kMainScreen = {"main-element", "verifyAccountScreen"};
const test::UIPath kErrorScreen = {"main-element", "errorScreen"};
const test::UIPath kSamlConfirmPasswordScreen = {"main-element",
Expand Down Expand Up @@ -167,6 +168,11 @@ void LockScreenReauthDialogTestHelper::ClickCancelButtonOnSamlScreen() {
DialogJS().TapOnPath(kSamlCancelButton);
}

void LockScreenReauthDialogTestHelper::ClickChangeIdPButtonOnSamlScreen() {
ExpectSamlScreenVisible();
DialogJS().TapOnPath(kChangeIdPButton);
}

void LockScreenReauthDialogTestHelper::WaitForSamlScreen() {
WaitForAuthenticatorToLoad();
DialogJS().CreateVisibilityWaiter(true, kSamlContainer)->Wait();
Expand Down Expand Up @@ -194,6 +200,10 @@ void LockScreenReauthDialogTestHelper::ExpectSamlScreenHidden() {
DialogJS().ExpectHiddenPath(kSamlContainer);
}

void LockScreenReauthDialogTestHelper::ExpectGaiaScreenVisible() {
DialogJS().ExpectAttributeEQ("isDefaultSsoProvider", {"main-element"}, false);
}

void LockScreenReauthDialogTestHelper::ExpectSamlConfirmPasswordVisible() {
DialogJS().CreateVisibilityWaiter(true, kSamlConfirmPasswordScreen)->Wait();
DialogJS().ExpectVisiblePath(kSamlConfirmPasswordScreen);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,9 @@ class LockScreenReauthDialogTestHelper {
// Clicks the 'Cancel' button on the 'Saml Account' screen.
void ClickCancelButtonOnSamlScreen();

// Clicks the 'Enter Google Account Info' button on the SAML screen.
void ClickChangeIdPButtonOnSamlScreen();

// Waits for a screen with the `saml-container` element to be shown.
void WaitForSamlScreen();

Expand All @@ -83,6 +86,8 @@ class LockScreenReauthDialogTestHelper {
void ExpectSamlScreenVisible();
void ExpectSamlScreenHidden();

void ExpectGaiaScreenVisible();

// Next members allow to check visibility of some elements on 'confirm
// password screen' and also help to fill forms. Precondition: 'confirm
// password screen' is visible.
Expand Down
17 changes: 17 additions & 0 deletions chrome/browser/ash/login/saml/saml_lockscreen_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,23 @@ IN_PROC_BROWSER_TEST_F(LockscreenWebUiTest, Login) {
UnlockWithSAML();
}

// Tests that we can switch from SAML page to GAIA page on the lock screen.
IN_PROC_BROWSER_TEST_F(LockscreenWebUiTest, SamlSwitchToGaia) {
fake_saml_idp()->SetLoginHTMLTemplate("saml_login.html");

Login();

// Lock the screen and trigger the lock screen SAML reauth dialog.
ScreenLockerTester().Lock();

absl::optional<LockScreenReauthDialogTestHelper> reauth_dialog_helper =
LockScreenReauthDialogTestHelper::StartSamlAndWaitForIdpPageLoad();

reauth_dialog_helper->ClickChangeIdPButtonOnSamlScreen();

reauth_dialog_helper->ExpectGaiaScreenVisible();
}

// Tests the cancel button in Verify Screen.
IN_PROC_BROWSER_TEST_F(LockscreenWebUiTest, VerifyScreenCancel) {
fake_saml_idp()->SetLoginHTMLTemplate("saml_login.html");
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
<!-- TODO(b/259386106): reduce duplication with login screen code -->
<style>
:host {
--lock-screen-reauth-dialog-buttons-horizontal-padding: 40px;
Expand Down Expand Up @@ -93,8 +94,8 @@
}

#samlContainer[saml-notice-message] {
/* #F1F3F4 */
background: rgb(241, 243, 244);
/* #FFFFFF */
background: white;
}

#samlNoticeMessage {
Expand All @@ -105,6 +106,18 @@
padding-top: 15px;
}

#saml-footer-container {
align-items: center;
background: white;
box-shadow: 0 2px 2px 0 rgba(0, 0, 0, 0.17);
/* #6a6a6a */
color: rgb(106, 106, 106);
display: flex;
height: 58px;
justify-content: flex-end;
min-height: 0;
}

#saml-close-button {
--cr-icon-button-margin-end: 0;
--cr-icon-button-margin-start: 0;
Expand All @@ -116,6 +129,12 @@
width: 100%;
}

#change-account {
margin: 0 4px;
padding-inline-end: 8px;
padding-inline-start: 8px;
}

.title-icon {
/* #1a73e8 */
--iron-icon-fill-color: rgb(26, 115, 232);
Expand Down Expand Up @@ -292,6 +311,14 @@
</div>
<webview id="signin-frame" name="signin-frame" class="flex">
</webview>
<div id="saml-footer-container" hidden="[[!isDefaultSsoProvider]]"
class="layout horizontal">
<div>[[i18nDynamic(locale, 'samlChangeProviderMessage')]]</div>
<oobe-text-button id="change-account"
text-key="samlChangeProviderButton"
on-click="onChangeSigninProviderClicked_">
</oobe-text-button>
</div>
</div>

<div id="samlConfirmPasswordScreen" class="content-wrapper"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import 'chrome://resources/cr_elements/cr_button/cr_button.js';
import 'chrome://resources/cr_elements/cr_input/cr_input.js';
import 'chrome://resources/cr_elements/icons.html.js';
import 'chrome://resources/cr_elements/cr_shared_vars.css.js';
import './components/buttons/oobe_text_button.js';
import './components/oobe_icons.m.js';

import {I18nBehavior} from 'chrome://resources/ash/common/i18n_behavior.js';
Expand Down Expand Up @@ -72,6 +73,14 @@ Polymer({
value: false,
},

/**
* Whether default SAML IdP is shown.
*/
isDefaultSsoProvider: {
type: Boolean,
value: false,
},

/**
* Whether there is a failure to scrape the user's password.
*/
Expand Down Expand Up @@ -191,6 +200,7 @@ Polymer({

this.authenticatorParams_ = params;
this.email_ = data.email;
this.isDefaultSsoProvider = data.doSamlRedirect;
if (!data['doSamlRedirect']) {
this.doGaiaRedirect_();
}
Expand Down Expand Up @@ -381,4 +391,15 @@ Polymer({
'passwordChangedIncorrectOldPassword');
},

/**
* Invoked when "Enter Google Account info" button is pressed on SAML screen.
* @private
*/
onChangeSigninProviderClicked_() {
this.authenticatorParams_.doSamlRedirect = false;
this.authenticatorParams_.enableGaiaActionButtons = true;
this.isDefaultSsoProvider = false;
this.authenticator_.load(AuthMode.DEFAULT, this.authenticatorParams_);
},

});
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/ash/profiles/profile_helper.h"
#include "chrome/browser/ui/webui/ash/in_session_password_change/lock_screen_reauth_handler.h"
#include "chrome/browser/ui/webui/ash/login/oobe_ui.h"
#include "chrome/browser/ui/webui/metrics_handler.h"
#include "chrome/common/webui_url_constants.h"
#include "chrome/grit/browser_resources.h"
Expand Down Expand Up @@ -110,17 +111,19 @@ LockScreenStartReauthUI::LockScreenStartReauthUI(content::WebUI* web_ui)
"passwordChangedOldPasswordHint",
l10n_util::GetStringUTF16(IDS_LOCK_PASSWORD_CHANGED_OLD_PASSWORD_HINT));

source->AddString(
"samlChangeProviderMessage",
l10n_util::GetStringUTF16(IDS_LOGIN_SAML_CHANGE_PROVIDER_MESSAGE));
source->AddString(
"samlChangeProviderButton",
l10n_util::GetStringUTF16(IDS_LOGIN_SAML_CHANGE_PROVIDER_BUTTON));

source->AddResourcePaths(
base::make_span(kPasswordChangeResources, kPasswordChangeResourcesSize));
source->SetDefaultResource(IDR_PASSWORD_CHANGE_LOCK_SCREEN_REAUTH_APP_HTML);

// Add Gaia Authenticator resources
source->AddResourcePaths(
base::make_span(kGaiaAuthHostResources, kGaiaAuthHostResourcesSize));

// Add OOBE resources
source->AddResourcePaths(base::make_span(kOobeUnconditionalResources,
kOobeUnconditionalResourcesSize));
// Add OOBE and Gaia Authenticator resources
OobeUI::AddOobeComponents(source);

content::WebUIDataSource::Add(profile, source);
}
Expand Down

0 comments on commit d68ae89

Please sign in to comment.