Commit 410e7604 authored by Mathias Carlen's avatar Mathias Carlen Committed by Commit Bot

[Autofill Assistant] Browse mode domains whitelist.

This patch introduces a repeated field of domains to be used as a browse
mode whitelist when a user browses the web. In particular, subdomains
should be whitelisted if they should be accessible by a user without AA
shutting down.

Bug: b/154305037
Change-Id: Id3a1ce078b8ece833513beacedf3c529a9cd77d8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2164807
Commit-Queue: Mathias Carlen <mcarlen@chromium.org>
Reviewed-by: default avatarMarian Fechete <marianfe@google.com>
Cr-Commit-Position: refs/heads/master@{#762820}
parent e7bae453
...@@ -120,6 +120,11 @@ class ActionDelegate { ...@@ -120,6 +120,11 @@ class ActionDelegate {
// Have the UI leave the prompt state and go back to its previous state. // Have the UI leave the prompt state and go back to its previous state.
virtual void CleanUpAfterPrompt() = 0; virtual void CleanUpAfterPrompt() = 0;
// Set the list of whitelisted domains to be used when we enter a browse
// state. This list is used to determine whether a user initiated navigation
// to a different domain or subdomain is allowed.
virtual void SetBrowseDomainsWhitelist(std::vector<std::string> domains) = 0;
// Asks the user to provide the requested user data. // Asks the user to provide the requested user data.
virtual void CollectUserData( virtual void CollectUserData(
CollectUserDataOptions* collect_user_data_options) = 0; CollectUserDataOptions* collect_user_data_options) = 0;
......
...@@ -74,6 +74,8 @@ class MockActionDelegate : public ActionDelegate { ...@@ -74,6 +74,8 @@ class MockActionDelegate : public ActionDelegate {
base::OnceCallback<void()> end_on_navigation_callback, base::OnceCallback<void()> end_on_navigation_callback,
bool browse_mode)); bool browse_mode));
MOCK_METHOD0(CleanUpAfterPrompt, void()); MOCK_METHOD0(CleanUpAfterPrompt, void());
MOCK_METHOD1(SetBrowseDomainsWhitelist,
void(std::vector<std::string> domains));
void FillAddressForm( void FillAddressForm(
const autofill::AutofillProfile* profile, const autofill::AutofillProfile* profile,
......
...@@ -39,6 +39,12 @@ void PromptAction::InternalProcessAction(ProcessActionCallback callback) { ...@@ -39,6 +39,12 @@ void PromptAction::InternalProcessAction(ProcessActionCallback callback) {
delegate_->SetStatusMessage(proto_.prompt().message()); delegate_->SetStatusMessage(proto_.prompt().message());
} }
if (proto_.prompt().browse_mode()) {
delegate_->SetBrowseDomainsWhitelist(
{proto_.prompt().browse_domains_whitelist().begin(),
proto_.prompt().browse_domains_whitelist().end()});
}
SetupConditions(); SetupConditions();
UpdateUserActions(); UpdateUserActions();
...@@ -231,6 +237,10 @@ void PromptAction::OnNavigationEnded() { ...@@ -231,6 +237,10 @@ void PromptAction::OnNavigationEnded() {
void PromptAction::EndAction(const ClientStatus& status) { void PromptAction::EndAction(const ClientStatus& status) {
delegate_->CleanUpAfterPrompt(); delegate_->CleanUpAfterPrompt();
// Clear the whitelist when a browse action is done.
if (proto_.prompt().browse_mode()) {
delegate_->SetBrowseDomainsWhitelist({});
}
UpdateProcessedAction(status); UpdateProcessedAction(status);
std::move(callback_).Run(std::move(processed_action_proto_)); std::move(callback_).Run(std::move(processed_action_proto_));
} }
......
...@@ -109,6 +109,18 @@ bool HasSameDomainAs(const GURL& a, const GURL& b) { ...@@ -109,6 +109,18 @@ bool HasSameDomainAs(const GURL& a, const GURL& b) {
return a.host() == b.host(); return a.host() == b.host();
} }
// Check whether |subdomain| is a subdomain of a set of domains in |whitelist|.
bool IsInWhitelist(const std::string& subdomain,
const std::vector<std::string> whitelist) {
const GURL subdomain_gurl = GURL(subdomain);
for (const std::string& parent_domain : whitelist) {
if (HasSameDomainAs(subdomain_gurl, GURL(parent_domain)) ||
IsSubdomainOf(subdomain, parent_domain))
return true;
}
return false;
}
} // namespace } // namespace
Controller::Controller(content::WebContents* web_contents, Controller::Controller(content::WebContents* web_contents,
...@@ -337,6 +349,10 @@ void Controller::SetExpandSheetForPromptAction(bool expand) { ...@@ -337,6 +349,10 @@ void Controller::SetExpandSheetForPromptAction(bool expand) {
expand_sheet_for_prompt_action_ = expand; expand_sheet_for_prompt_action_ = expand;
} }
void Controller::SetBrowseDomainsWhitelist(std::vector<std::string> domains) {
browse_domains_whitelist_ = std::move(domains);
}
bool Controller::PerformUserActionWithContext( bool Controller::PerformUserActionWithContext(
int index, int index,
std::unique_ptr<TriggerContext> context) { std::unique_ptr<TriggerContext> context) {
...@@ -1648,7 +1664,8 @@ void Controller::DidFinishNavigation( ...@@ -1648,7 +1664,8 @@ void Controller::DidFinishNavigation(
auto current_host = web_contents()->GetLastCommittedURL().host(); auto current_host = web_contents()->GetLastCommittedURL().host();
auto script_host = script_url_.host(); auto script_host = script_url_.host();
if (current_host != script_host && if (current_host != script_host &&
!IsSubdomainOf(current_host, script_host)) { !IsSubdomainOf(current_host, script_host) &&
!IsInWhitelist(current_host, browse_domains_whitelist_)) {
OnScriptError(l10n_util::GetStringUTF8(IDS_AUTOFILL_ASSISTANT_GIVE_UP), OnScriptError(l10n_util::GetStringUTF8(IDS_AUTOFILL_ASSISTANT_GIVE_UP),
Metrics::DropOutReason::DOMAIN_CHANGE_DURING_BROWSE_MODE); Metrics::DropOutReason::DOMAIN_CHANGE_DURING_BROWSE_MODE);
} }
......
...@@ -147,6 +147,7 @@ class Controller : public ScriptExecutorDelegate, ...@@ -147,6 +147,7 @@ class Controller : public ScriptExecutorDelegate,
void RemoveListener(NavigationListener* listener) override; void RemoveListener(NavigationListener* listener) override;
void SetExpandSheetForPromptAction(bool expand) override; void SetExpandSheetForPromptAction(bool expand) override;
void SetBrowseDomainsWhitelist(std::vector<std::string> domains) override;
bool EnterState(AutofillAssistantState state) override; bool EnterState(AutofillAssistantState state) override;
void SetCollectUserDataOptions(CollectUserDataOptions* options) override; void SetCollectUserDataOptions(CollectUserDataOptions* options) override;
...@@ -450,6 +451,7 @@ class Controller : public ScriptExecutorDelegate, ...@@ -450,6 +451,7 @@ class Controller : public ScriptExecutorDelegate,
BasicInteractions basic_interactions_{this}; BasicInteractions basic_interactions_{this};
bool expand_sheet_for_prompt_action_ = true; bool expand_sheet_for_prompt_action_ = true;
std::vector<std::string> browse_domains_whitelist_;
// Only set during a ShowGenericUiAction. // Only set during a ShowGenericUiAction.
std::unique_ptr<GenericUserInterfaceProto> generic_user_interface_; std::unique_ptr<GenericUserInterfaceProto> generic_user_interface_;
......
...@@ -1378,6 +1378,86 @@ TEST_F(ControllerTest, BrowseStateStopsOnDifferentDomain) { ...@@ -1378,6 +1378,86 @@ TEST_F(ControllerTest, BrowseStateStopsOnDifferentDomain) {
SimulateNavigateToUrl(GURL("http://other-example.com/")); SimulateNavigateToUrl(GURL("http://other-example.com/"));
} }
TEST_F(ControllerTest, BrowseStateWithDomainWhitelist) {
SupportsScriptResponseProto script_response;
AddRunnableScript(&script_response, "runnable")
->mutable_presentation()
->set_autostart(true);
ActionsResponseProto runnable_script;
auto* prompt = runnable_script.add_actions()->mutable_prompt();
prompt->set_browse_mode(true);
*prompt->add_browse_domains_whitelist() = "example.com";
*prompt->add_browse_domains_whitelist() = "other-example.com";
prompt->add_choices()->mutable_chip()->set_text("continue");
SetupActionsForScript("runnable", runnable_script);
std::string response_str;
script_response.SerializeToString(&response_str);
EXPECT_CALL(*mock_service_,
OnGetScriptsForUrl(GURL("http://a.example.com/"), _, _))
.WillOnce(RunOnceCallback<2>(true, response_str));
Start("http://a.example.com/");
EXPECT_EQ(AutofillAssistantState::BROWSE, controller_->GetState());
SimulateNavigateToUrl(GURL("http://b.example.com/"));
EXPECT_EQ(AutofillAssistantState::BROWSE, controller_->GetState());
SimulateNavigateToUrl(GURL("http://sub.other-example.com/"));
EXPECT_EQ(AutofillAssistantState::BROWSE, controller_->GetState());
// go back.
SetLastCommittedUrl(GURL("http://sub.other-example.com"));
content::NavigationSimulator::GoBack(web_contents());
EXPECT_EQ(AutofillAssistantState::BROWSE, controller_->GetState());
// Same domain navigations as one of the whitelisted domains should not
// shutdown AA.
SimulateNavigateToUrl(GURL("http://other-example.com/"));
EXPECT_EQ(AutofillAssistantState::BROWSE, controller_->GetState());
}
TEST_F(ControllerTest, BrowseStateWithDomainWhitelistCleanup) {
SupportsScriptResponseProto script_response;
AddRunnableScript(&script_response, "runnable")
->mutable_presentation()
->set_autostart(true);
ActionsResponseProto runnable_script;
auto* prompt = runnable_script.add_actions()->mutable_prompt();
prompt->set_browse_mode(true);
*prompt->add_browse_domains_whitelist() = "example.com";
prompt->add_choices()->mutable_chip()->set_text("continue");
// Second browse action without a whitelist.
auto* prompt2 = runnable_script.add_actions()->mutable_prompt();
prompt2->set_browse_mode(true);
prompt2->add_choices()->mutable_chip()->set_text("done");
SetupActionsForScript("runnable", runnable_script);
std::string response_str;
script_response.SerializeToString(&response_str);
EXPECT_CALL(*mock_service_,
OnGetScriptsForUrl(GURL("http://a.example.com/"), _, _))
.WillOnce(RunOnceCallback<2>(true, response_str));
Start("http://a.example.com/");
EXPECT_EQ(AutofillAssistantState::BROWSE, controller_->GetState());
SimulateNavigateToUrl(GURL("http://b.example.com/"));
EXPECT_EQ(AutofillAssistantState::BROWSE, controller_->GetState());
// Click "continue".
EXPECT_EQ(controller_->GetUserActions()[0].chip().text, "continue");
controller_->PerformUserAction(0);
EXPECT_EQ(controller_->GetUserActions()[0].chip().text, "done");
// Make sure the whitelist got reset with the second prompt action.
EXPECT_CALL(
mock_client_,
Shutdown(Metrics::DropOutReason::DOMAIN_CHANGE_DURING_BROWSE_MODE));
SimulateNavigateToUrl(GURL("http://c.example.com/"));
}
TEST_F(ControllerTest, PromptStateStopsOnGoBack) { TEST_F(ControllerTest, PromptStateStopsOnGoBack) {
SupportsScriptResponseProto script_response; SupportsScriptResponseProto script_response;
AddRunnableScript(&script_response, "runnable") AddRunnableScript(&script_response, "runnable")
......
...@@ -177,6 +177,11 @@ void FakeScriptExecutorDelegate::SetExpandSheetForPromptAction(bool expand) { ...@@ -177,6 +177,11 @@ void FakeScriptExecutorDelegate::SetExpandSheetForPromptAction(bool expand) {
expand_sheet_for_prompt_ = expand; expand_sheet_for_prompt_ = expand;
} }
void FakeScriptExecutorDelegate::SetBrowseDomainsWhitelist(
std::vector<std::string> domains) {
browse_domains_ = std::move(domains);
}
bool FakeScriptExecutorDelegate::SetForm( bool FakeScriptExecutorDelegate::SetForm(
std::unique_ptr<FormProto> form, std::unique_ptr<FormProto> form,
base::RepeatingCallback<void(const FormProto::Result*)> changed_callback, base::RepeatingCallback<void(const FormProto::Result*)> changed_callback,
......
...@@ -70,6 +70,8 @@ class FakeScriptExecutorDelegate : public ScriptExecutorDelegate { ...@@ -70,6 +70,8 @@ class FakeScriptExecutorDelegate : public ScriptExecutorDelegate {
void AddListener(NavigationListener* listener) override; void AddListener(NavigationListener* listener) override;
void RemoveListener(NavigationListener* listener) override; void RemoveListener(NavigationListener* listener) override;
void SetExpandSheetForPromptAction(bool expand) override; void SetExpandSheetForPromptAction(bool expand) override;
void SetBrowseDomainsWhitelist(std::vector<std::string> domains) override;
void SetGenericUi( void SetGenericUi(
std::unique_ptr<GenericUserInterfaceProto> generic_ui, std::unique_ptr<GenericUserInterfaceProto> generic_ui,
base::OnceCallback<void(bool, base::OnceCallback<void(bool,
...@@ -144,6 +146,7 @@ class FakeScriptExecutorDelegate : public ScriptExecutorDelegate { ...@@ -144,6 +146,7 @@ class FakeScriptExecutorDelegate : public ScriptExecutorDelegate {
bool expand_or_collapse_updated_ = false; bool expand_or_collapse_updated_ = false;
bool expand_or_collapse_value_ = false; bool expand_or_collapse_value_ = false;
bool expand_sheet_for_prompt_ = true; bool expand_sheet_for_prompt_ = true;
std::vector<std::string> browse_domains_;
UserModel* user_model_ = nullptr; UserModel* user_model_ = nullptr;
bool require_ui_ = false; bool require_ui_ = false;
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "components/autofill_assistant/browser/script_executor.h" #include "components/autofill_assistant/browser/script_executor.h"
#include <cstdio>
#include <ostream> #include <ostream>
#include <string> #include <string>
#include <utility> #include <utility>
...@@ -337,6 +338,11 @@ void ScriptExecutor::CleanUpAfterPrompt() { ...@@ -337,6 +338,11 @@ void ScriptExecutor::CleanUpAfterPrompt() {
delegate_->EnterState(AutofillAssistantState::RUNNING); delegate_->EnterState(AutofillAssistantState::RUNNING);
} }
void ScriptExecutor::SetBrowseDomainsWhitelist(
std::vector<std::string> domains) {
delegate_->SetBrowseDomainsWhitelist(std::move(domains));
}
void ScriptExecutor::OnChosen(UserAction::Callback callback, void ScriptExecutor::OnChosen(UserAction::Callback callback,
std::unique_ptr<TriggerContext> context) { std::unique_ptr<TriggerContext> context) {
if (context->is_direct_action()) { if (context->is_direct_action()) {
......
...@@ -130,6 +130,7 @@ class ScriptExecutor : public ActionDelegate, ...@@ -130,6 +130,7 @@ class ScriptExecutor : public ActionDelegate,
base::OnceCallback<void()> end_on_navigation_callback, base::OnceCallback<void()> end_on_navigation_callback,
bool browse_mode) override; bool browse_mode) override;
void CleanUpAfterPrompt() override; void CleanUpAfterPrompt() override;
void SetBrowseDomainsWhitelist(std::vector<std::string> domains) override;
void FillAddressForm( void FillAddressForm(
const autofill::AutofillProfile* profile, const autofill::AutofillProfile* profile,
const Selector& selector, const Selector& selector,
......
...@@ -139,6 +139,9 @@ class ScriptExecutorDelegate { ...@@ -139,6 +139,9 @@ class ScriptExecutorDelegate {
// Set how the sheet should behave when entering a prompt state. // Set how the sheet should behave when entering a prompt state.
virtual void SetExpandSheetForPromptAction(bool expand) = 0; virtual void SetExpandSheetForPromptAction(bool expand) = 0;
// Set the domains whitelist for browse mode.
virtual void SetBrowseDomainsWhitelist(std::vector<std::string> domains) = 0;
// Sets the generic UI to show to the user. // Sets the generic UI to show to the user.
virtual void SetGenericUi( virtual void SetGenericUi(
std::unique_ptr<GenericUserInterfaceProto> generic_ui, std::unique_ptr<GenericUserInterfaceProto> generic_ui,
......
...@@ -1180,6 +1180,10 @@ message PromptProto { ...@@ -1180,6 +1180,10 @@ message PromptProto {
// action. The result sent back to the server will contain a Choice marked as // action. The result sent back to the server will contain a Choice marked as
// |navigation_ended|. // |navigation_ended|.
optional bool end_on_navigation = 8; optional bool end_on_navigation = 8;
// A list of domains and subdomains to allow users to navigate to when in
// browse mode.
repeated string browse_domains_whitelist = 9;
} }
message ContactDetailsProto { message ContactDetailsProto {
......
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