Commit 34d0a4a1 authored by Yi Gu's avatar Yi Gu Committed by Chromium LUCI CQ

[WebOTP] Update the infobar with proper origins for x-origin support

This patch updates the infobar to show both the top and embedded origins
if WebOTP is used in a cross-origin iframe. The UI remains the same if
the API is used in the top frame.

Note: the exact string to show to users is still under UI review.

Bug: 1136506
Change-Id: I4320545c306abeb3cd44987bf0d2ba2ed94b192e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2643626
Commit-Queue: Yi Gu <yigu@chromium.org>
Reviewed-by: default avatarYaron Friedman <yfriedman@chromium.org>
Reviewed-by: default avatarMajid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#846312}
parent 007c0fbf
......@@ -192,13 +192,13 @@ void TabWebContentsDelegateAndroid::RunFileChooser(
void TabWebContentsDelegateAndroid::CreateSmsPrompt(
content::RenderFrameHost* host,
const url::Origin& origin,
const std::vector<url::Origin>& origin_list,
const std::string& one_time_code,
base::OnceClosure on_confirm,
base::OnceClosure on_cancel) {
auto* web_contents = content::WebContents::FromRenderFrameHost(host);
sms::SmsInfoBar::Create(
web_contents, InfoBarService::FromWebContents(web_contents), origin,
web_contents, InfoBarService::FromWebContents(web_contents), origin_list,
one_time_code, std::move(on_confirm), std::move(on_cancel));
}
......
......@@ -48,7 +48,7 @@ class TabWebContentsDelegateAndroid
scoped_refptr<content::FileSelectListener> listener,
const blink::mojom::FileChooserParams& params) override;
void CreateSmsPrompt(content::RenderFrameHost*,
const url::Origin&,
const std::vector<url::Origin>&,
const std::string& one_time_code,
base::OnceClosure on_confirm,
base::OnceClosure on_cancel) override;
......
......@@ -1392,7 +1392,7 @@ blink::SecurityStyle Browser::GetSecurityStyle(
}
void Browser::CreateSmsPrompt(content::RenderFrameHost*,
const url::Origin&,
const std::vector<url::Origin>&,
const std::string& one_time_code,
base::OnceClosure on_confirm,
base::OnceClosure on_cancel) {
......
......@@ -614,7 +614,7 @@ class Browser : public TabStripModelObserver,
content::WebContents* web_contents,
content::SecurityStyleExplanations* security_style_explanations) override;
void CreateSmsPrompt(content::RenderFrameHost*,
const url::Origin&,
const std::vector<url::Origin>&,
const std::string& one_time_code,
base::OnceClosure on_confirm,
base::OnceClosure on_cancel) override;
......
......@@ -353,6 +353,7 @@ test("components_unittests") {
deps += [
"//base:base_java_unittest_support",
"//components/autofill_assistant/browser:unit_tests",
"//components/browser_ui/sms/android:unit_tests",
"//components/cdm/browser:unit_tests",
"//components/crash/android:java",
"//components/crash/android:unit_tests",
......
......@@ -25,6 +25,21 @@ source_set("android") {
]
}
source_set("unit_tests") {
testonly = true
sources = [ "sms_infobar_delegate_unittest.cc" ]
deps = [
":android",
"//base/test:test_support",
"//components/content_settings/browser",
"//components/content_settings/browser:test_support",
"//components/content_settings/core/browser",
"//components/infobars/content",
"//content/test:test_support",
"//testing/gtest",
]
}
generate_jni("jni_headers") {
sources = [
"java/src/org/chromium/components/browser_ui/sms/WebOTPServiceInfoBar.java",
......
include_rules = [
"+components/infobars/android",
"+components/infobars/content",
"+components/infobars/core",
"+components/resources/android",
"+components/strings",
......
......@@ -20,12 +20,12 @@ namespace sms {
// static
void SmsInfoBar::Create(content::WebContents* web_contents,
infobars::InfoBarManager* manager,
const url::Origin& origin,
const std::vector<url::Origin>& origin_list,
const std::string& one_time_code,
base::OnceClosure on_confirm,
base::OnceClosure on_cancel) {
auto delegate = std::make_unique<SmsInfoBarDelegate>(
origin, one_time_code, std::move(on_confirm), std::move(on_cancel));
origin_list, one_time_code, std::move(on_confirm), std::move(on_cancel));
auto infobar =
std::make_unique<SmsInfoBar>(web_contents, std::move(delegate));
manager->AddInfoBar(std::move(infobar));
......
......@@ -33,7 +33,7 @@ class SmsInfoBar : public infobars::ConfirmInfoBar {
// |infobar_service|.
static void Create(content::WebContents* web_contents,
infobars::InfoBarManager* manager,
const url::Origin& origin,
const std::vector<url::Origin>& origin_list,
const std::string& one_time_code,
base::OnceClosure on_confirm,
base::OnceClosure on_cancel);
......
......@@ -17,12 +17,12 @@
namespace sms {
SmsInfoBarDelegate::SmsInfoBarDelegate(const url::Origin& origin,
SmsInfoBarDelegate::SmsInfoBarDelegate(const OriginList& origin_list,
const std::string& one_time_code,
base::OnceClosure on_confirm,
base::OnceClosure on_cancel)
: ConfirmInfoBarDelegate(),
origin_(origin),
origin_list_(origin_list),
one_time_code_(one_time_code),
on_confirm_(std::move(on_confirm)),
on_cancel_(std::move(on_cancel)) {}
......@@ -39,10 +39,25 @@ int SmsInfoBarDelegate::GetIconId() const {
}
base::string16 SmsInfoBarDelegate::GetMessageText() const {
base::string16 origin = url_formatter::FormatOriginForSecurityDisplay(
origin_, url_formatter::SchemeDisplay::OMIT_HTTP_AND_HTTPS);
return l10n_util::GetStringFUTF16(IDS_SMS_INFOBAR_STATUS_SMS_RECEIVED,
base::UTF8ToUTF16(one_time_code_), origin);
if (origin_list_.size() == 1) {
base::string16 origin = url_formatter::FormatOriginForSecurityDisplay(
origin_list_[0], url_formatter::SchemeDisplay::OMIT_HTTP_AND_HTTPS);
return l10n_util::GetStringFUTF16(IDS_SMS_INFOBAR_STATUS_SMS_RECEIVED,
base::UTF8ToUTF16(one_time_code_),
origin);
}
// Only one cross-origin iframe is allowed.
DCHECK_EQ(origin_list_.size(), 2u);
base::string16 embedded_origin =
url_formatter::FormatOriginForSecurityDisplay(
origin_list_[0], url_formatter::SchemeDisplay::OMIT_HTTP_AND_HTTPS);
base::string16 top_origin = url_formatter::FormatOriginForSecurityDisplay(
origin_list_[1], url_formatter::SchemeDisplay::OMIT_HTTP_AND_HTTPS);
return l10n_util::GetStringFUTF16(
IDS_SMS_INFOBAR_STATUS_SMS_RECEIVED_FROM_EMBEDDED_FRAME,
base::UTF8ToUTF16(one_time_code_), top_origin, embedded_origin);
}
int SmsInfoBarDelegate::GetButtons() const {
......
......@@ -16,7 +16,9 @@ namespace sms {
// is asked for confirmation that it should be shared with the site (WebOTP).
class SmsInfoBarDelegate : public ConfirmInfoBarDelegate {
public:
SmsInfoBarDelegate(const url::Origin& origin,
using OriginList = std::vector<url::Origin>;
SmsInfoBarDelegate(const OriginList& origin_list,
const std::string& one_time_code,
base::OnceClosure on_confirm,
base::OnceClosure on_cancel);
......@@ -34,7 +36,7 @@ class SmsInfoBarDelegate : public ConfirmInfoBarDelegate {
base::string16 GetTitle() const;
private:
const url::Origin origin_;
const OriginList origin_list_;
const std::string one_time_code_;
base::OnceClosure on_confirm_;
base::OnceClosure on_cancel_;
......
// Copyright 2021 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 "components/browser_ui/sms/android/sms_infobar_delegate.h"
#include "base/strings/utf_string_conversions.h"
#include "base/test/bind.h"
#include "components/browser_ui/sms/android/sms_infobar.h"
#include "components/infobars/content/content_infobar_manager.h"
#include "components/infobars/core/infobar.h"
#include "content/public/test/test_renderer_host.h"
namespace sms {
namespace {
class TestInfoBarManager : public infobars::ContentInfoBarManager {
public:
explicit TestInfoBarManager(content::WebContents* web_contents)
: ContentInfoBarManager(web_contents) {}
// infobars::InfoBarManager:
std::unique_ptr<infobars::InfoBar> CreateConfirmInfoBar(
std::unique_ptr<ConfirmInfoBarDelegate> delegate) override {
return std::make_unique<infobars::InfoBar>(std::move(delegate));
}
};
} // namespace
class SmsInfoBarDelegateTest : public content::RenderViewHostTestHarness {
public:
// content::RenderViewHostTestHarness:
void SetUp() override {
content::RenderViewHostTestHarness::SetUp();
infobar_manager_ = std::make_unique<TestInfoBarManager>(web_contents());
}
TestInfoBarManager* infobar_manager() { return infobar_manager_.get(); }
private:
std::unique_ptr<TestInfoBarManager> infobar_manager_;
};
TEST_F(SmsInfoBarDelegateTest, InfoBarForSingleFrame) {
std::string url = "https://example.com";
url::Origin origin = url::Origin::Create(GURL(url));
std::vector<url::Origin> origin_list{origin};
SmsInfoBar::Create(web_contents(), infobar_manager(), origin_list, "1234",
base::OnceClosure(), base::OnceClosure());
EXPECT_EQ(infobar_manager()->infobar_count(), 1u);
std::string expected_message = "1234 is your code for example.com";
EXPECT_EQ(base::UTF16ToUTF8(infobar_manager()
->infobar_at(0)
->delegate()
->AsConfirmInfoBarDelegate()
->GetMessageText()),
expected_message);
}
TEST_F(SmsInfoBarDelegateTest, InfoBarForEmbeddedFrame) {
std::string top_url = "https://top.com";
std::string embedded_url = "https://embedded.com";
url::Origin top_origin = url::Origin::Create(GURL(top_url));
url::Origin embedded_origin = url::Origin::Create(GURL(embedded_url));
std::vector<url::Origin> origin_list{embedded_origin, top_origin};
SmsInfoBar::Create(web_contents(), infobar_manager(), origin_list, "1234",
base::OnceClosure(), base::OnceClosure());
EXPECT_EQ(infobar_manager()->infobar_count(), 1u);
std::string expected_message =
"1234 is your code for top.com (through embedded.com)";
EXPECT_EQ(base::UTF16ToUTF8(infobar_manager()
->infobar_at(0)
->delegate()
->AsConfirmInfoBarDelegate()
->GetMessageText()),
expected_message);
}
} // namespace sms
\ No newline at end of file
......@@ -8,6 +8,10 @@
<ph name="ONE_TIME_CODE">$1<ex>123</ex></ph> is your code for <ph name="ORIGIN">$2<ex>example.com</ex></ph>
</message>
<message name="IDS_SMS_INFOBAR_STATUS_SMS_RECEIVED_FROM_EMBEDDED_FRAME" desc="Message shown when the browser has received an SMS on the user's behalf. EMBEDDED_ORIGIN is from a child frame inside TOP_ORIGIN">
<ph name="ONE_TIME_CODE">$1<ex>123</ex></ph> is your code for <ph name="TOP_ORIGIN">$2<ex>example.com</ex></ph> (through <ph name="EMBEDDED_ORIGIN">$3<ex>iframe.com</ex></ph>)
</message>
<message name="IDS_SMS_INFOBAR_BUTTON_OK" desc="Text for the button shown when the browser has received an SMS on the user's behalf">
Verify
</message>
......
a00bf1a5f456c0268a0892e219ef53e4d47e9f43
\ No newline at end of file
......@@ -137,7 +137,7 @@ class SmsBrowserTest : public ContentBrowserTest {
void ExpectSmsPrompt() {
EXPECT_CALL(delegate_, CreateSmsPrompt(_, _, _, _, _))
.WillOnce(Invoke([&](RenderFrameHost*, const url::Origin&,
.WillOnce(Invoke([&](RenderFrameHost*, const OriginList&,
const std::string&, base::OnceClosure on_confirm,
base::OnceClosure on_cancel) {
confirm_callback_ = std::move(on_confirm);
......
......@@ -18,7 +18,7 @@ class MockSmsWebContentsDelegate : public WebContentsDelegate {
MOCK_METHOD5(CreateSmsPrompt,
void(RenderFrameHost*,
const url::Origin&,
const std::vector<url::Origin>&,
const std::string&,
base::OnceCallback<void()> on_confirm,
base::OnceCallback<void()> on_cancel));
......
......@@ -29,8 +29,8 @@ bool NoopUserConsentHandler::is_async() const {
PromptBasedUserConsentHandler::PromptBasedUserConsentHandler(
RenderFrameHost* frame_host,
const url::Origin& origin)
: frame_host_{frame_host}, origin_{origin} {}
const OriginList& origin_list)
: frame_host_{frame_host}, origin_list_{origin_list} {}
PromptBasedUserConsentHandler::~PromptBasedUserConsentHandler() = default;
void PromptBasedUserConsentHandler::RequestUserConsent(
......@@ -46,7 +46,7 @@ void PromptBasedUserConsentHandler::RequestUserConsent(
on_complete_ = std::move(on_complete);
is_prompt_open_ = true;
web_contents->GetDelegate()->CreateSmsPrompt(
frame_host_, origin_, one_time_code,
frame_host_, origin_list_, one_time_code,
base::BindOnce(&PromptBasedUserConsentHandler::OnConfirm,
weak_ptr_factory_.GetWeakPtr()),
base::BindOnce(&PromptBasedUserConsentHandler::OnCancel,
......
......@@ -45,9 +45,12 @@ class CONTENT_EXPORT NoopUserConsentHandler : public UserConsentHandler {
class CONTENT_EXPORT PromptBasedUserConsentHandler : public UserConsentHandler {
public:
using OriginList = std::vector<url::Origin>;
PromptBasedUserConsentHandler(RenderFrameHost* frame_host,
const url::Origin& origin);
const OriginList& origin_list);
~PromptBasedUserConsentHandler() override;
void RequestUserConsent(const std::string& one_time_code,
CompletionCallback on_complete) override;
bool is_active() const override;
......@@ -58,7 +61,7 @@ class CONTENT_EXPORT PromptBasedUserConsentHandler : public UserConsentHandler {
private:
RenderFrameHost* frame_host_;
const url::Origin origin_;
const OriginList origin_list_;
bool is_prompt_open_{false};
CompletionCallback on_complete_;
......
......@@ -37,11 +37,13 @@ class PromptBasedUserConsentHandlerTest : public RenderViewHostTestHarness {
web_contents_impl->SetDelegate(&delegate_);
}
using OriginList = std::vector<url::Origin>;
void ExpectCreateSmsPrompt(RenderFrameHost* rfh,
const url::Origin& origin,
const OriginList& origin_list,
const std::string& one_time_code) {
EXPECT_CALL(delegate_, CreateSmsPrompt(rfh, origin, one_time_code, _, _))
.WillOnce(Invoke([=](RenderFrameHost*, const Origin& origin,
EXPECT_CALL(delegate_,
CreateSmsPrompt(rfh, origin_list, one_time_code, _, _))
.WillOnce(Invoke([=](RenderFrameHost*, const OriginList& origin_list,
const std::string&, base::OnceClosure on_confirm,
base::OnceClosure on_cancel) {
confirm_callback_ = std::move(on_confirm);
......@@ -84,9 +86,9 @@ TEST_F(PromptBasedUserConsentHandlerTest, PromptsUser) {
web_contents()->GetMainFrame()->GetLastCommittedOrigin();
base::RunLoop loop;
ExpectCreateSmsPrompt(main_rfh(), origin, "12345");
ExpectCreateSmsPrompt(main_rfh(), OriginList{origin}, "12345");
CompletionCallback callback;
PromptBasedUserConsentHandler consent_handler{main_rfh(), origin};
PromptBasedUserConsentHandler consent_handler{main_rfh(), OriginList{origin}};
consent_handler.RequestUserConsent("12345", std::move(callback));
}
......@@ -96,8 +98,8 @@ TEST_F(PromptBasedUserConsentHandlerTest, ConfirmInvokedCallback) {
const url::Origin& origin =
web_contents()->GetMainFrame()->GetLastCommittedOrigin();
ExpectCreateSmsPrompt(main_rfh(), origin, "12345");
PromptBasedUserConsentHandler consent_handler{main_rfh(), origin};
ExpectCreateSmsPrompt(main_rfh(), OriginList{origin}, "12345");
PromptBasedUserConsentHandler consent_handler{main_rfh(), OriginList{origin}};
EXPECT_FALSE(consent_handler.is_active());
bool succeed;
auto callback = base::BindLambdaForTesting([&](UserConsentResult result) {
......@@ -116,8 +118,8 @@ TEST_F(PromptBasedUserConsentHandlerTest, CancelingInvokedCallback) {
const url::Origin& origin =
web_contents()->GetMainFrame()->GetLastCommittedOrigin();
ExpectCreateSmsPrompt(main_rfh(), origin, "12345");
PromptBasedUserConsentHandler consent_handler{main_rfh(), origin};
ExpectCreateSmsPrompt(main_rfh(), OriginList{origin}, "12345");
PromptBasedUserConsentHandler consent_handler{main_rfh(), OriginList{origin}};
EXPECT_FALSE(consent_handler.is_active());
bool cancelled;
auto callback = base::BindLambdaForTesting([&](UserConsentResult result) {
......@@ -142,7 +144,7 @@ TEST_F(PromptBasedUserConsentHandlerTest, CancelsWhenNoDelegate) {
ExpectNoSmsPrompt();
PromptBasedUserConsentHandler consent_handler{main_rfh(), origin};
PromptBasedUserConsentHandler consent_handler{main_rfh(), OriginList{origin}};
bool cancelled;
auto callback = base::BindLambdaForTesting([&](UserConsentResult result) {
cancelled = (result == UserConsentResult::kNoDelegate);
......
......@@ -305,12 +305,8 @@ UserConsentHandler* WebOTPService::CreateConsentHandler(
return consent_handler_for_test_;
if (consent_requirement == UserConsent::kNotObtained) {
// If WebOTP is used in a cross-origin iframe then the first origin in the
// list is the one who calls the WebOTP API. We show it to users in the
// prompt to make sure that they are aware of which frame / origin they are
// granting OTP access.
consent_handler_ = std::make_unique<PromptBasedUserConsentHandler>(
render_frame_host(), origin_list_[0]);
render_frame_host(), origin_list_);
} else {
consent_handler_ = std::make_unique<NoopUserConsentHandler>();
}
......
......@@ -152,7 +152,7 @@ JavaScriptDialogManager* WebContentsDelegate::GetJavaScriptDialogManager(
void WebContentsDelegate::CreateSmsPrompt(
RenderFrameHost* host,
const url::Origin& origin,
const std::vector<url::Origin>& origin_list,
const std::string& one_time_code,
base::OnceCallback<void()> on_confirm,
base::OnceCallback<void()> on_cancel) {}
......
......@@ -430,7 +430,7 @@ class CONTENT_EXPORT WebContentsDelegate {
// Creates an info bar for the user to control the receiving of the SMS.
virtual void CreateSmsPrompt(RenderFrameHost*,
const url::Origin&,
const std::vector<url::Origin>&,
const std::string& one_time_code,
base::OnceCallback<void()> on_confirm,
base::OnceCallback<void()> on_cancel);
......
......@@ -972,7 +972,7 @@ content::ColorChooser* TabImpl::OpenColorChooser(
}
void TabImpl::CreateSmsPrompt(content::RenderFrameHost* render_frame_host,
const url::Origin& origin,
const std::vector<url::Origin>& origin_list,
const std::string& one_time_code,
base::OnceClosure on_confirm,
base::OnceClosure on_cancel) {
......@@ -980,7 +980,7 @@ void TabImpl::CreateSmsPrompt(content::RenderFrameHost* render_frame_host,
auto* web_contents =
content::WebContents::FromRenderFrameHost(render_frame_host);
sms::SmsInfoBar::Create(
web_contents, InfoBarService::FromWebContents(web_contents), origin,
web_contents, InfoBarService::FromWebContents(web_contents), origin_list,
one_time_code, std::move(on_confirm), std::move(on_cancel));
#else
NOTREACHED();
......
......@@ -267,7 +267,7 @@ class TabImpl : public Tab,
scoped_refptr<content::FileSelectListener> listener,
const blink::mojom::FileChooserParams& params) override;
void CreateSmsPrompt(content::RenderFrameHost*,
const url::Origin&,
const std::vector<url::Origin>&,
const std::string& one_time_code,
base::OnceClosure on_confirm,
base::OnceClosure on_cancel) override;
......
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