Commit df39977d authored by Alex Ilin's avatar Alex Ilin Committed by Commit Bot

[signin] chrome://signin-reauth depends on access point

chrome://signin-reauth needs to display different strings depending
on ReauthAccessPoint parameter. Callers pass this parameter to
SigninReauthUI as a URL query parameter.

chrome/browser/signin/reauth_util takes care of generating a
corresponding URL and reading an encoded parameter from in.

Bug: 1045515
Change-Id: Ib83ca63cf179fe04a58ba889d8ad0629d54df688
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2254144
Commit-Queue: Alex Ilin <alexilin@chromium.org>
Reviewed-by: default avatarDavid Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#780728}
parent ff6b045d
......@@ -1719,6 +1719,8 @@ static_library("browser") {
"signin/reauth_result.h",
"signin/reauth_tab_helper.cc",
"signin/reauth_tab_helper.h",
"signin/reauth_util.cc",
"signin/reauth_util.h",
"signin/signin_error_controller_factory.cc",
"signin/signin_error_controller_factory.h",
"signin/signin_features.cc",
......
// 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.
#include <string>
#include "base/strings/string_number_conversions.h"
#include "chrome/browser/signin/reauth_util.h"
#include "chrome/common/webui_url_constants.h"
#include "net/base/url_util.h"
namespace signin {
GURL GetReauthConfirmationURL(signin_metrics::ReauthAccessPoint access_point) {
GURL url = GURL(chrome::kChromeUISigninReauthURL);
url = net::AppendQueryParameter(
url, "access_point",
base::NumberToString(static_cast<int>(access_point)));
return url;
}
signin_metrics::ReauthAccessPoint GetReauthAccessPointForReauthConfirmationURL(
const GURL& url) {
std::string value;
if (!net::GetValueForKeyInQuery(url, "access_point", &value))
return signin_metrics::ReauthAccessPoint::kUnknown;
int access_point = -1;
base::StringToInt(value, &access_point);
if (access_point <=
static_cast<int>(signin_metrics::ReauthAccessPoint::kUnknown) ||
access_point >
static_cast<int>(signin_metrics::ReauthAccessPoint::kMaxValue)) {
return signin_metrics::ReauthAccessPoint::kUnknown;
}
return static_cast<signin_metrics::ReauthAccessPoint>(access_point);
}
} // namespace signin
// 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 CHROME_BROWSER_SIGNIN_REAUTH_UTIL_H_
#define CHROME_BROWSER_SIGNIN_REAUTH_UTIL_H_
#include "components/signin/public/base/signin_metrics.h"
#include "url/gurl.h"
namespace signin {
// Returns a URL to display in the reauth confirmation dialog. The dialog was
// triggered by |access_point|.
GURL GetReauthConfirmationURL(signin_metrics::ReauthAccessPoint access_point);
// Returns ReauthAccessPoint encoded in the query of the reauth confirmation
// URL.
signin_metrics::ReauthAccessPoint GetReauthAccessPointForReauthConfirmationURL(
const GURL& url);
} // namespace signin
#endif // CHROME_BROWSER_SIGNIN_REAUTH_UTIL_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.
#include "chrome/browser/signin/reauth_util.h"
#include "chrome/common/webui_url_constants.h"
#include "components/signin/public/base/signin_metrics.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace signin {
class ReauthUtilURLTest : public ::testing::TestWithParam<int> {};
TEST_P(ReauthUtilURLTest, GetAndParseReauthConfirmationURL) {
auto access_point =
static_cast<signin_metrics::ReauthAccessPoint>(GetParam());
GURL url = GetReauthConfirmationURL(access_point);
ASSERT_TRUE(url.is_valid());
EXPECT_EQ(url.host(), chrome::kChromeUISigninReauthHost);
signin_metrics::ReauthAccessPoint get_access_point =
GetReauthAccessPointForReauthConfirmationURL(url);
EXPECT_EQ(get_access_point, access_point);
}
INSTANTIATE_TEST_CASE_P(
AllAccessPoints,
ReauthUtilURLTest,
::testing::Range(
static_cast<int>(signin_metrics::ReauthAccessPoint::kUnknown),
static_cast<int>(signin_metrics::ReauthAccessPoint::kMaxValue) + 1));
} // namespace signin
......@@ -56,6 +56,7 @@ SigninReauthViewController::SigninReauthViewController(
base::OnceCallback<void(signin::ReauthResult)> reauth_callback)
: browser_(browser),
account_id_(account_id),
access_point_(access_point),
reauth_callback_(std::move(reauth_callback)) {
// Show the confirmation dialog unconditionally for now. We may decide to only
// show it in some cases in the future.
......@@ -195,7 +196,7 @@ void SigninReauthViewController::OnStateChanged() {
void SigninReauthViewController::ShowReauthConfirmationDialog() {
dialog_delegate_ =
SigninViewControllerDelegate::CreateReauthConfirmationDelegate(
browser_, account_id_);
browser_, account_id_, access_point_);
dialog_delegate_observer_.Add(dialog_delegate_);
}
......
......@@ -108,6 +108,7 @@ class SigninReauthViewController
Browser* const browser_;
const CoreAccountId account_id_;
const signin_metrics::ReauthAccessPoint access_point_;
base::OnceCallback<void(signin::ReauthResult)> reauth_callback_;
// Delegate displaying the dialog.
......
......@@ -16,6 +16,10 @@ namespace content {
class WebContents;
}
namespace signin_metrics {
enum class ReauthAccessPoint;
}
// Interface to the platform-specific managers of the Signin and Sync
// confirmation tab-modal dialogs. This and its platform-specific
// implementations are responsible for actually creating and owning the dialogs,
......@@ -52,7 +56,8 @@ class SigninViewControllerDelegate {
// delete itself when the window it's managing is closed.
static SigninViewControllerDelegate* CreateReauthConfirmationDelegate(
Browser* browser,
const CoreAccountId& account_id);
const CoreAccountId& account_id,
signin_metrics::ReauthAccessPoint access_point);
void AddObserver(Observer* observer);
void RemoveObserver(Observer* observer);
......
......@@ -11,6 +11,7 @@
#include "build/build_config.h"
#include "chrome/browser/signin/identity_manager_factory.h"
#include "chrome/browser/signin/reauth_result.h"
#include "chrome/browser/signin/reauth_util.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/signin_view_controller.h"
......@@ -202,17 +203,20 @@ IN_PROC_BROWSER_TEST_F(SignInViewControllerBrowserTest,
// Tests that the confirm button is focused by default in the reauth dialog.
IN_PROC_BROWSER_TEST_F(SignInViewControllerBrowserTest, ReauthDefaultFocus) {
const auto kAccessPoint =
signin_metrics::ReauthAccessPoint::kAutofillDropdown;
content::TestNavigationObserver content_observer(
signin::GetReauthConfirmationURL(kAccessPoint));
content_observer.StartWatchingNewWebContents();
CoreAccountId account_id = signin::SetUnconsentedPrimaryAccount(
GetIdentityManager(), "alice@gmail.com")
.account_id;
signin::ReauthResult reauth_result;
content::TestNavigationObserver content_observer(
GURL("chrome://signin-reauth/"));
content_observer.StartWatchingNewWebContents();
base::RunLoop run_loop;
std::unique_ptr<SigninViewController::ReauthAbortHandle> abort_handle =
browser()->signin_view_controller()->ShowReauthPrompt(
account_id, signin_metrics::ReauthAccessPoint::kAutofillDropdown,
account_id, kAccessPoint,
base::BindLambdaForTesting([&](signin::ReauthResult result) {
reauth_result = result;
run_loop.Quit();
......
......@@ -12,6 +12,7 @@
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_avatar_icon_util.h"
#include "chrome/browser/signin/reauth_result.h"
#include "chrome/browser/signin/reauth_util.h"
#include "chrome/browser/signin/signin_promo.h"
#include "chrome/browser/sync/profile_sync_service_factory.h"
#include "chrome/browser/ui/browser.h"
......@@ -23,6 +24,7 @@
#include "chrome/common/url_constants.h"
#include "chrome/common/webui_url_constants.h"
#include "components/constrained_window/constrained_window_views.h"
#include "components/signin/public/base/signin_metrics.h"
#include "components/web_modal/web_contents_modal_dialog_host.h"
#include "content/public/browser/render_widget_host_view.h"
#include "content/public/browser/web_contents.h"
......@@ -32,6 +34,7 @@
#include "ui/views/bubble/bubble_frame_view.h"
#include "ui/views/controls/webview/webview.h"
#include "ui/views/widget/widget.h"
#include "url/gurl.h"
namespace {
......@@ -57,7 +60,7 @@ std::unique_ptr<views::WebView>
SigninViewControllerDelegateViews::CreateSyncConfirmationWebView(
Browser* browser) {
return CreateDialogWebView(
browser, chrome::kChromeUISyncConfirmationURL,
browser, GURL(chrome::kChromeUISyncConfirmationURL),
GetSyncConfirmationDialogPreferredHeight(browser->profile()),
kSyncConfirmationDialogWidth);
}
......@@ -65,15 +68,17 @@ SigninViewControllerDelegateViews::CreateSyncConfirmationWebView(
// static
std::unique_ptr<views::WebView>
SigninViewControllerDelegateViews::CreateSigninErrorWebView(Browser* browser) {
return CreateDialogWebView(browser, chrome::kChromeUISigninErrorURL,
return CreateDialogWebView(browser, GURL(chrome::kChromeUISigninErrorURL),
kSigninErrorDialogHeight, base::nullopt);
}
// static
std::unique_ptr<views::WebView>
SigninViewControllerDelegateViews::CreateReauthConfirmationWebView(
Browser* browser) {
return CreateDialogWebView(browser, chrome::kChromeUISigninReauthURL,
Browser* browser,
signin_metrics::ReauthAccessPoint access_point) {
return CreateDialogWebView(browser,
signin::GetReauthConfirmationURL(access_point),
kReauthDialogHeight, kReauthDialogWidth);
}
......@@ -208,12 +213,12 @@ SigninViewControllerDelegateViews::~SigninViewControllerDelegateViews() =
std::unique_ptr<views::WebView>
SigninViewControllerDelegateViews::CreateDialogWebView(
Browser* browser,
const std::string& url,
const GURL& url,
int dialog_height,
base::Optional<int> opt_width) {
int dialog_width = opt_width.value_or(kModalDialogWidth);
views::WebView* web_view = new views::WebView(browser->profile());
web_view->LoadInitialURL(GURL(url));
web_view->LoadInitialURL(url);
// To record metrics using javascript, extensions are needed.
extensions::ChromeExtensionWebContentsObserver::CreateForWebContents(
web_view->GetWebContents());
......@@ -294,9 +299,10 @@ SigninViewControllerDelegate::CreateSigninErrorDelegate(Browser* browser) {
SigninViewControllerDelegate*
SigninViewControllerDelegate::CreateReauthConfirmationDelegate(
Browser* browser,
const CoreAccountId& account_id) {
const CoreAccountId& account_id,
signin_metrics::ReauthAccessPoint access_point) {
return new SigninViewControllerDelegateViews(
SigninViewControllerDelegateViews::CreateReauthConfirmationWebView(
browser),
browser, access_point),
browser, ui::MODAL_TYPE_CHILD, false, true);
}
......@@ -13,11 +13,17 @@
#include "ui/views/controls/webview/unhandled_keyboard_event_handler.h"
#include "ui/views/window/dialog_delegate.h"
class GURL;
namespace content {
class WebContents;
class WebContentsDelegate;
}
namespace signin_metrics {
enum class ReauthAccessPoint;
}
namespace views {
class WebView;
}
......@@ -39,7 +45,8 @@ class SigninViewControllerDelegateViews
Browser* browser);
static std::unique_ptr<views::WebView> CreateReauthConfirmationWebView(
Browser* browser);
Browser* browser,
signin_metrics::ReauthAccessPoint);
// views::DialogDelegateView:
views::View* GetContentsView() override;
......@@ -90,7 +97,7 @@ class SigninViewControllerDelegateViews
// Creates a WebView for a dialog with the specified URL.
static std::unique_ptr<views::WebView> CreateDialogWebView(
Browser* browser,
const std::string& url,
const GURL& url,
int dialog_height,
base::Optional<int> dialog_width);
......
......@@ -11,10 +11,12 @@
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_avatar_icon_util.h"
#include "chrome/browser/signin/identity_manager_factory.h"
#include "chrome/browser/signin/reauth_util.h"
#include "chrome/browser/ui/webui/signin/signin_reauth_handler.h"
#include "chrome/common/webui_url_constants.h"
#include "chrome/grit/browser_resources.h"
#include "chrome/grit/generated_resources.h"
#include "components/signin/public/base/signin_metrics.h"
#include "components/signin/public/identity_manager/account_info.h"
#include "components/signin/public/identity_manager/consent_level.h"
#include "components/signin/public/identity_manager/identity_manager.h"
......@@ -46,11 +48,41 @@ std::string GetAccountImageURL(Profile* profile) {
: profiles::GetPlaceholderAvatarIconUrl();
}
int GetReauthTitleStringId(signin_metrics::ReauthAccessPoint access_point) {
switch (access_point) {
case signin_metrics::ReauthAccessPoint::kUnknown:
case signin_metrics::ReauthAccessPoint::kAutofillDropdown:
case signin_metrics::ReauthAccessPoint::kPasswordSettings:
return IDS_ACCOUNT_PASSWORDS_REAUTH_SHOW_TITLE;
case signin_metrics::ReauthAccessPoint::kGeneratePasswordDropdown:
case signin_metrics::ReauthAccessPoint::kGeneratePasswordContextMenu:
case signin_metrics::ReauthAccessPoint::kPasswordSaveBubble:
return IDS_ACCOUNT_PASSWORDS_REAUTH_SAVE_TITLE;
}
}
int GetReauthConfirmButtonLabelStringId(
signin_metrics::ReauthAccessPoint access_point) {
switch (access_point) {
case signin_metrics::ReauthAccessPoint::kUnknown:
case signin_metrics::ReauthAccessPoint::kAutofillDropdown:
case signin_metrics::ReauthAccessPoint::kPasswordSettings:
return IDS_ACCOUNT_PASSWORDS_REAUTH_SHOW_BUTTON_LABEL;
case signin_metrics::ReauthAccessPoint::kGeneratePasswordDropdown:
case signin_metrics::ReauthAccessPoint::kGeneratePasswordContextMenu:
case signin_metrics::ReauthAccessPoint::kPasswordSaveBubble:
return IDS_ACCOUNT_PASSWORDS_REAUTH_SAVE_BUTTON_LABEL;
}
}
} // namespace
SigninReauthUI::SigninReauthUI(content::WebUI* web_ui)
: SigninWebDialogUI(web_ui) {
Profile* profile = Profile::FromWebUI(web_ui);
signin_metrics::ReauthAccessPoint access_point =
signin::GetReauthAccessPointForReauthConfirmationURL(
web_ui->GetWebContents()->GetVisibleURL());
content::WebUIDataSource* source =
content::WebUIDataSource::Create(chrome::kChromeUISigninReauthHost);
......@@ -77,11 +109,11 @@ SigninReauthUI::SigninReauthUI(content::WebUI* web_ui)
"images/signin_reauth_illustration_dark.svg",
IDR_SIGNIN_REAUTH_IMAGES_ACCOUNT_PASSWORDS_REAUTH_ILLUSTRATION_DARK_SVG);
source->AddLocalizedString("signinReauthTitle",
IDS_ACCOUNT_PASSWORDS_REAUTH_SAVE_TITLE);
GetReauthTitleStringId(access_point));
source->AddLocalizedString("signinReauthDesc",
IDS_ACCOUNT_PASSWORDS_REAUTH_DESC);
source->AddLocalizedString("signinReauthConfirmLabel",
IDS_ACCOUNT_PASSWORDS_REAUTH_SAVE_BUTTON_LABEL);
GetReauthConfirmButtonLabelStringId(access_point));
source->AddLocalizedString("signinReauthCloseLabel",
IDS_ACCOUNT_PASSWORDS_REAUTH_CLOSE_BUTTON_LABEL);
......
......@@ -3434,6 +3434,7 @@ test("unit_tests") {
"../browser/signin/chrome_signin_url_loader_throttle_unittest.cc",
"../browser/signin/local_auth_unittest.cc",
"../browser/signin/reauth_tab_helper_unittest.cc",
"../browser/signin/reauth_util_unittest.cc",
"../browser/signin/signin_profile_attributes_updater_unittest.cc",
"../browser/signin/signin_status_metrics_provider_chromeos_unittest.cc",
"../browser/signin/test_signin_client_builder.cc",
......
......@@ -57,7 +57,9 @@ TEST_F('SigninSyncConfirmationTest', 'Dialog', function() {
var SigninReauthTest = class extends SigninBrowserTest {
/** @override */
get browsePreload() {
return 'chrome://signin-reauth/test_loader.html?module=signin/signin_reauth_test.js';
// See signin_metrics::ReauthAccessPoint for definition of the
// "access_point" parameter.
return 'chrome://signin-reauth/test_loader.html?module=signin/signin_reauth_test.js&access_point=2';
}
};
......
......@@ -171,12 +171,15 @@ enum class AccessPoint : int {
// could be initiated. Transactional reauth is used when the user already has
// a valid refresh token but a system still wants to verify user's identity.
enum class ReauthAccessPoint {
// The code expects kUnknown to be the first, so it should not be reordered.
kUnknown = 0,
// Account password storage opt-in:
kAutofillDropdown = 0,
kPasswordSaveBubble = 1,
kPasswordSettings = 2,
kGeneratePasswordDropdown = 3,
kGeneratePasswordContextMenu = 4,
kAutofillDropdown = 1,
kPasswordSaveBubble = 2,
kPasswordSettings = 3,
kGeneratePasswordDropdown = 4,
kGeneratePasswordContextMenu = 5,
kMaxValue = kGeneratePasswordContextMenu
};
......
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