Commit 2c0292b6 authored by Milica Selakovic's avatar Milica Selakovic Committed by Commit Bot

[AutofillAssistant] Introduce GeneratePasswordForFormFieldAction

In this CL password generation is extracted from SetFormFieldValueAction
to separate action.

Bug: 1064264

Change-Id: If111335c08ec91de250ad8ddc084d15d5ef73cc5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2146545
Commit-Queue: Milica Selakovic <selakovic@google.com>
Reviewed-by: default avatarClemens Arbesser <arbesser@google.com>
Reviewed-by: default avatarMarian Fechete <marianfe@google.com>
Cr-Commit-Position: refs/heads/master@{#759226}
parent 1e509a9a
......@@ -27,6 +27,7 @@ import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.chrome.browser.autofill_assistant.proto.ActionProto;
import org.chromium.chrome.browser.autofill_assistant.proto.ChipProto;
import org.chromium.chrome.browser.autofill_assistant.proto.ElementReferenceProto;
import org.chromium.chrome.browser.autofill_assistant.proto.GeneratePasswordForFormFieldProto;
import org.chromium.chrome.browser.autofill_assistant.proto.PromptProto;
import org.chromium.chrome.browser.autofill_assistant.proto.SetFormFieldValueProto;
import org.chromium.chrome.browser.autofill_assistant.proto.SupportedScriptProto;
......@@ -93,7 +94,8 @@ public class AutofillAssistantPasswordManagerIntegrationTest {
}
/**
* Run a password change flow (fill a form with username, old password, new password).
* Run a password change flow (fill a form with username, old password, new
* password).
*/
@Test
@MediumTest
......@@ -108,19 +110,24 @@ public class AutofillAssistantPasswordManagerIntegrationTest {
.setElement(ElementReferenceProto.newBuilder().addSelectors(
"#username")))
.build());
// TODO(crbug.com/1057608): Implement Android wrapper for PasswordStore to add a step and
// TODO(crbug.com/1057608): Implement Android wrapper for PasswordStore to add a
// step and
// verification for current password filling.
list.add(
(ActionProto) ActionProto.newBuilder()
.setGeneratePasswordForFormField(
GeneratePasswordForFormFieldProto.newBuilder()
.setMemoryKey("memory-key")
.setElement(ElementReferenceProto.newBuilder().addSelectors(
"#new-password")))
.build());
list.add(
(ActionProto) ActionProto.newBuilder()
.setSetFormValue(
SetFormFieldValueProto.newBuilder()
.addValue(SetFormFieldValueProto.KeyPress.newBuilder()
.setGeneratePassword(
SetFormFieldValueProto
.GeneratePassword
.newBuilder()
.setMemoryKey(
"memory-key")))
.setClientMemoryKey("memory-key"))
.setElement(ElementReferenceProto.newBuilder().addSelectors(
"#new-password")))
.build());
......
......@@ -35,6 +35,8 @@ jumbo_static_library("browser") {
"actions/expect_navigation_action.h",
"actions/focus_element_action.cc",
"actions/focus_element_action.h",
"actions/generate_password_for_form_field_action.cc",
"actions/generate_password_for_form_field_action.h",
"actions/highlight_element_action.cc",
"actions/highlight_element_action.h",
"actions/navigate_action.cc",
......@@ -232,6 +234,7 @@ source_set("unit_tests") {
sources = [
"actions/collect_user_data_action_unittest.cc",
"actions/configure_bottom_sheet_action_unittest.cc",
"actions/generate_password_for_form_field_action_unittest.cc",
"actions/popup_message_action_unittest.cc",
"actions/prompt_action_unittest.cc",
"actions/required_fields_fallback_handler_unittest.cc",
......
......@@ -128,6 +128,9 @@ std::ostream& operator<<(std::ostream& out,
case ActionProto::ActionInfoCase::kShowGenericUi:
out << "ShowGenericUi";
break;
case ActionProto::ActionInfoCase::kGeneratePasswordForFormField:
out << "GeneratePasswordForFormField";
break;
case ActionProto::ActionInfoCase::ACTION_INFO_NOT_SET:
out << "ACTION_INFO_NOT_SET";
break;
......
// 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 "components/autofill_assistant/browser/actions/generate_password_for_form_field_action.h"
#include <utility>
#include "base/optional.h"
#include "components/autofill/core/common/form_data.h"
#include "components/autofill/core/common/form_field_data.h"
#include "components/autofill_assistant/browser/actions/action_delegate.h"
#include "components/autofill_assistant/browser/client_status.h"
#include "components/autofill_assistant/browser/value_util.h"
namespace autofill_assistant {
GeneratePasswordForFormFieldAction::GeneratePasswordForFormFieldAction(
ActionDelegate* delegate,
const ActionProto& proto)
: Action(delegate, proto) {
DCHECK(proto_.has_generate_password_for_form_field());
}
GeneratePasswordForFormFieldAction::~GeneratePasswordForFormFieldAction() {}
void GeneratePasswordForFormFieldAction::InternalProcessAction(
ProcessActionCallback callback) {
callback_ = std::move(callback);
selector_ = Selector(proto_.generate_password_for_form_field().element())
.MustBeVisible();
if (selector_.empty()) {
VLOG(1) << __func__ << ": empty selector";
EndAction(ClientStatus(INVALID_SELECTOR));
return;
}
delegate_->RetrieveElementFormAndFieldData(
selector_,
base::BindOnce(&GeneratePasswordForFormFieldAction::
OnGetFormAndFieldDataForGeneration,
weak_ptr_factory_.GetWeakPtr(),
proto_.generate_password_for_form_field().memory_key()));
}
void GeneratePasswordForFormFieldAction::OnGetFormAndFieldDataForGeneration(
const std::string& memory_key,
const ClientStatus& status,
const autofill::FormData& form_data,
const autofill::FormFieldData& field_data) {
if (!status.ok()) {
EndAction(status);
}
uint64_t max_length = field_data.max_length;
std::string password = delegate_->GetWebsiteLoginFetcher()->GeneratePassword(
autofill::CalculateFormSignature(form_data),
autofill::CalculateFieldSignatureForField(field_data), max_length);
delegate_->WriteUserData(base::BindOnce(
&GeneratePasswordForFormFieldAction::StoreGeneratedPasswordToUserData,
weak_ptr_factory_.GetWeakPtr(), memory_key, password));
}
void GeneratePasswordForFormFieldAction::StoreGeneratedPasswordToUserData(
const std::string& memory_key,
const std::string& generated_password,
UserData* user_data,
UserData::FieldChange* field_change) {
DCHECK(user_data);
user_data->additional_values_[memory_key] = SimpleValue(generated_password);
EndAction(ClientStatus(ACTION_APPLIED));
}
void GeneratePasswordForFormFieldAction::EndAction(const ClientStatus& status) {
UpdateProcessedAction(status);
std::move(callback_).Run(std::move(processed_action_proto_));
}
} // namespace autofill_assistant
// 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 COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_ACTIONS_GENERATE_PASSWORD_FOR_FORM_FIELD_ACTION_H_
#define COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_ACTIONS_GENERATE_PASSWORD_FOR_FORM_FIELD_ACTION_H_
#include <memory>
#include <string>
#include <utility>
#include <vector>
#include "base/callback.h"
#include "base/memory/weak_ptr.h"
#include "components/autofill_assistant/browser/actions/action.h"
#include "components/autofill_assistant/browser/user_data.h"
namespace autofill {
struct FormData;
struct FormFieldData;
} // namespace autofill
namespace autofill_assistant {
// Action to generate a password for a form field. Sets up the necessary state
// for subsequent calls to SetFormFieldValueAction.
class GeneratePasswordForFormFieldAction : public Action {
public:
explicit GeneratePasswordForFormFieldAction(ActionDelegate* delegate,
const ActionProto& proto);
~GeneratePasswordForFormFieldAction() override;
GeneratePasswordForFormFieldAction(
const GeneratePasswordForFormFieldAction&) = delete;
GeneratePasswordForFormFieldAction& operator=(
const GeneratePasswordForFormFieldAction&) = delete;
private:
// Overrides Action:
void InternalProcessAction(ProcessActionCallback callback) override;
void EndAction(const ClientStatus& status);
void OnGetFormAndFieldDataForGeneration(
const std::string& memory_key,
const ClientStatus& status,
const autofill::FormData& form_data,
const autofill::FormFieldData& field_data);
void StoreGeneratedPasswordToUserData(const std::string& memory_key,
const std::string& generated_password,
UserData* user_data,
UserData::FieldChange* field_change);
Selector selector_;
ProcessActionCallback callback_;
base::WeakPtrFactory<GeneratePasswordForFormFieldAction> weak_ptr_factory_{
this};
};
} // namespace autofill_assistant
#endif // COMPONENTS_AUTOFILL_ASSISTANT_BROWSER_ACTIONS_GENERATE_PASSWORD_FOR_FORM_FIELD_ACTION_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 "components/autofill_assistant/browser/actions/generate_password_for_form_field_action.h"
#include <string>
#include <utility>
#include "base/test/gmock_callback_support.h"
#include "base/test/mock_callback.h"
#include "components/autofill_assistant/browser/actions/mock_action_delegate.h"
#include "components/autofill_assistant/browser/client_status.h"
#include "components/autofill_assistant/browser/mock_website_login_fetcher.h"
#include "testing/gmock/include/gmock/gmock.h"
namespace {
const char kFakeSelector[] = "#some_selector";
const char kGeneratedPassword[] = "m-W2b-_.7Fu9A.A";
const char kMemoryKeyForGeneratedPassword[] = "memory-key-for-generation";
} // namespace
namespace autofill_assistant {
using ::base::test::RunOnceCallback;
using ::testing::_;
using ::testing::InSequence;
using ::testing::Invoke;
using ::testing::Pointee;
using ::testing::Property;
using ::testing::Return;
class GeneratePasswordForFormFieldActionTest : public testing::Test {
public:
void SetUp() override {
generate_password_proto_ =
proto_.mutable_generate_password_for_form_field();
generate_password_proto_->mutable_element()->add_selectors(kFakeSelector);
generate_password_proto_->mutable_element()->set_visibility_requirement(
MUST_BE_VISIBLE);
ON_CALL(mock_action_delegate_, WriteUserData)
.WillByDefault(
RunOnceCallback<0>(&user_data_, /* field_change = */ nullptr));
ON_CALL(mock_action_delegate_, GetWebsiteLoginFetcher)
.WillByDefault(Return(&mock_website_login_fetcher_));
ON_CALL(mock_website_login_fetcher_, GetGeneratedPassword())
.WillByDefault(Return(kGeneratedPassword));
fake_selector_ = Selector({kFakeSelector}).MustBeVisible();
}
protected:
Selector fake_selector_;
MockActionDelegate mock_action_delegate_;
MockWebsiteLoginFetcher mock_website_login_fetcher_;
base::MockCallback<Action::ProcessActionCallback> callback_;
ActionProto proto_;
GeneratePasswordForFormFieldProto* generate_password_proto_;
UserData user_data_;
};
TEST_F(GeneratePasswordForFormFieldActionTest, GeneratedPassword) {
generate_password_proto_->set_memory_key(kMemoryKeyForGeneratedPassword);
GeneratePasswordForFormFieldAction action(&mock_action_delegate_, proto_);
EXPECT_CALL(
callback_,
Run(Pointee(Property(&ProcessedActionProto::status, ACTION_APPLIED))));
action.ProcessAction(callback_.Get());
EXPECT_EQ(kGeneratedPassword,
user_data_.additional_values_[kMemoryKeyForGeneratedPassword]
.strings()
.values(0));
}
} // namespace autofill_assistant
......@@ -8,8 +8,6 @@
#include "base/bind.h"
#include "base/callback.h"
#include "components/autofill/core/common/form_data.h"
#include "components/autofill/core/common/form_field_data.h"
#include "components/autofill/core/common/signatures_util.h"
#include "components/autofill_assistant/browser/actions/action_delegate.h"
#include "components/autofill_assistant/browser/client_status.h"
......@@ -32,14 +30,8 @@ SetFormFieldValueAction::FieldInput::FieldInput(std::string _value)
: value(_value) {}
SetFormFieldValueAction::FieldInput::FieldInput(
PasswordValueType _password_type,
const std::string& _memory_key)
: password_type(_password_type), memory_key(_memory_key) {
DCHECK_EQ(password_type == PasswordValueType::GENERATED_PASSWORD,
!memory_key.empty())
<< "Wrong |FieldInput| initialisation: iff the password action type is "
"|GENERATED_PASSWORD|, the memory should not be empty";
}
PasswordValueType _password_type)
: password_type(_password_type) {}
SetFormFieldValueAction::FieldInput::FieldInput(FieldInput&& other) = default;
......@@ -143,17 +135,6 @@ void SetFormFieldValueAction::InternalProcessAction(
->strings()
.values(0));
break;
case SetFormFieldValueProto_KeyPress::kGeneratePassword:
if (keypress.generate_password().memory_key().empty()) {
VLOG(1) << "SetFormFieldValueAction_kGeneratePassword: "
"|memory_key| cannot be empty.";
EndAction(ClientStatus(INVALID_ACTION));
return;
}
field_inputs_.emplace_back(
/* password_type = */ PasswordValueType::GENERATED_PASSWORD,
/* memory_key = */ keypress.generate_password().memory_key());
break;
default:
VLOG(1) << "Unrecognized field for SetFormFieldValueProto_KeyPress";
EndAction(ClientStatus(INVALID_ACTION));
......@@ -198,14 +179,6 @@ void SetFormFieldValueAction::OnSetFieldValue(int next,
case PasswordValueType::NOT_SET:
DCHECK(false);
break;
case PasswordValueType::GENERATED_PASSWORD:
delegate_->RetrieveElementFormAndFieldData(
selector_,
base::BindOnce(
&SetFormFieldValueAction::OnGetFormAndFieldDataForGeneration,
weak_ptr_factory_.GetWeakPtr(),
/* field_index = */ next, field_input.memory_key));
break;
case PasswordValueType::STORED_PASSWORD:
delegate_->GetWebsiteLoginFetcher()->GetPasswordForLogin(
*delegate_->GetUserData()->selected_login_,
......@@ -305,52 +278,6 @@ void SetFormFieldValueAction::OnGetStoredPassword(int field_index,
}
}
void SetFormFieldValueAction::OnGetFormAndFieldDataForGeneration(
int field_index,
const std::string memory_key,
const ClientStatus& status,
const autofill::FormData& form_data,
const autofill::FormFieldData& field_data) {
if (!status.ok()) {
EndAction(status);
}
uint64_t max_length = field_data.max_length;
std::string password = delegate_->GetWebsiteLoginFetcher()->GeneratePassword(
autofill::CalculateFormSignature(form_data),
autofill::CalculateFieldSignatureForField(field_data), max_length);
delegate_->WriteUserData(
base::BindOnce(&SetFormFieldValueAction::StoreGeneratedPasswordToUserData,
weak_ptr_factory_.GetWeakPtr(), memory_key, password));
auto fill_strategy = proto_.set_form_value().fill_strategy();
int delay_in_millisecond = proto_.set_form_value().delay_in_millisecond();
if (IsSimulatingKeyPresses(fill_strategy)) {
delegate_->SetFieldValue(
selector_, password, fill_strategy, delay_in_millisecond,
base::BindOnce(&SetFormFieldValueAction::OnSetFieldValue,
weak_ptr_factory_.GetWeakPtr(),
/* next = */ field_index + 1));
} else {
delegate_->SetFieldValue(
selector_, password, fill_strategy, delay_in_millisecond,
base::BindOnce(
&SetFormFieldValueAction::OnSetFieldValueAndCheckFallback,
weak_ptr_factory_.GetWeakPtr(),
/* next = */ field_index, /* requested_value = */ password));
}
}
void SetFormFieldValueAction::StoreGeneratedPasswordToUserData(
const std::string memory_key,
const std::string generated_password,
UserData* user_data,
UserData::FieldChange* field_change) {
DCHECK(user_data);
ValueProto value;
value.mutable_strings()->add_values(generated_password);
user_data->additional_values_[memory_key] = value;
}
void SetFormFieldValueAction::EndAction(const ClientStatus& status) {
// Clear immediately, to prevent sensitive information from staying in memory.
field_inputs_.clear();
......
......@@ -16,11 +16,6 @@
#include "components/autofill_assistant/browser/string_conversions_util.h"
#include "components/autofill_assistant/browser/user_data.h"
namespace autofill {
struct FormData;
struct FormFieldData;
} // namespace autofill
namespace autofill_assistant {
// An action to set the value of a form input element.
......@@ -34,16 +29,16 @@ class SetFormFieldValueAction : public Action {
FRIEND_TEST_ALL_PREFIXES(SetFormFieldValueActionTest,
PasswordIsClearedFromMemory);
// TODO(b/154067717): Remove PasswordValueType enum.
// Helper enum for |FieldInput| to describe the passwords-related actions.
enum class PasswordValueType { NOT_SET, STORED_PASSWORD, GENERATED_PASSWORD };
enum class PasswordValueType { NOT_SET, STORED_PASSWORD };
// A field input as extracted from the proto, but already checked for
// validity.
struct FieldInput {
explicit FieldInput(std::unique_ptr<std::vector<UChar32>> keyboard_input);
explicit FieldInput(std::string value);
explicit FieldInput(PasswordValueType password_type,
const std::string& memory_key = std::string());
explicit FieldInput(PasswordValueType password_type);
FieldInput(FieldInput&& other);
~FieldInput();
......@@ -53,10 +48,6 @@ class SetFormFieldValueAction : public Action {
// If the action is about passwords, the field describes whether the
// password should be retrieved from storage or generated.
PasswordValueType password_type = PasswordValueType::NOT_SET;
// Iff |password_type == GENERATED_PASSWORD|, the field contains a memory
// key to store the generated password for confirmation field filling and
// updating the password store.
std::string memory_key;
// The string to input (for all other cases).
std::string value;
};
......@@ -79,18 +70,6 @@ class SetFormFieldValueAction : public Action {
void OnGetStoredPassword(int field_index, bool success, std::string password);
void OnGetFormAndFieldDataForGeneration(
int field_index,
const std::string memory_key,
const ClientStatus& status,
const autofill::FormData& form_data,
const autofill::FormFieldData& field_data);
void StoreGeneratedPasswordToUserData(const std::string memory_key,
const std::string generated_password,
UserData* user_data,
UserData::FieldChange* field_change);
void EndAction(const ClientStatus& status);
Selector selector_;
......
......@@ -20,8 +20,6 @@ const char kFakeUrl[] = "https://www.example.com";
const char kFakeSelector[] = "#some_selector";
const char kFakeUsername[] = "user@example.com";
const char kFakePassword[] = "example_password";
const char kGeneratedPassword[] = "m-W2b-_.7Fu9A.A";
const char kMemoryKeyForGeneratedPassword[] = "memory-key-for-generation";
} // namespace
namespace autofill_assistant {
......@@ -59,8 +57,6 @@ class SetFormFieldValueActionTest : public testing::Test {
WebsiteLoginFetcher::Login(GURL(kFakeUrl), kFakeUsername)}));
ON_CALL(mock_website_login_fetcher_, OnGetPasswordForLogin(_, _))
.WillByDefault(RunOnceCallback<1>(true, kFakePassword));
ON_CALL(mock_website_login_fetcher_, GetGeneratedPassword())
.WillByDefault(Return(kGeneratedPassword));
user_data_.selected_login_ =
base::make_optional<WebsiteLoginFetcher::Login>(GURL(kFakeUrl),
kFakeUsername);
......@@ -154,27 +150,6 @@ TEST_F(SetFormFieldValueActionTest, PasswordToFill) {
action.ProcessAction(callback_.Get());
}
TEST_F(SetFormFieldValueActionTest, GeneratedPassword) {
auto* value = set_form_field_proto_->add_value();
value->mutable_generate_password()->set_memory_key(
kMemoryKeyForGeneratedPassword);
SetFormFieldValueAction action(&mock_action_delegate_, proto_);
ON_CALL(mock_action_delegate_, OnGetFieldValue(_, _))
.WillByDefault(RunOnceCallback<1>(OkClientStatus(), kGeneratedPassword));
EXPECT_CALL(mock_action_delegate_,
OnSetFieldValue(fake_selector_, kGeneratedPassword, _, _, _))
.WillOnce(RunOnceCallback<4>(OkClientStatus()));
EXPECT_CALL(
callback_,
Run(Pointee(Property(&ProcessedActionProto::status, ACTION_APPLIED))));
action.ProcessAction(callback_.Get());
EXPECT_EQ(kGeneratedPassword,
user_data_.additional_values_[kMemoryKeyForGeneratedPassword]
.strings()
.values(0));
}
TEST_F(SetFormFieldValueActionTest, Keycode) {
auto* value = set_form_field_proto_->add_value();
value->set_keycode(13); // carriage return
......
......@@ -13,6 +13,7 @@
#include "components/autofill_assistant/browser/actions/configure_bottom_sheet_action.h"
#include "components/autofill_assistant/browser/actions/expect_navigation_action.h"
#include "components/autofill_assistant/browser/actions/focus_element_action.h"
#include "components/autofill_assistant/browser/actions/generate_password_for_form_field_action.h"
#include "components/autofill_assistant/browser/actions/highlight_element_action.h"
#include "components/autofill_assistant/browser/actions/navigate_action.h"
#include "components/autofill_assistant/browser/actions/popup_message_action.h"
......@@ -322,6 +323,11 @@ bool ProtocolUtils::ParseActions(ActionDelegate* delegate,
client_action = std::make_unique<ShowGenericUiAction>(delegate, action);
break;
}
case ActionProto::ActionInfoCase::kGeneratePasswordForFormField: {
client_action = std::make_unique<GeneratePasswordForFormFieldAction>(
delegate, action);
break;
}
case ActionProto::ActionInfoCase::ACTION_INFO_NOT_SET: {
VLOG(1) << "Encountered action with ACTION_INFO_NOT_SET";
client_action = std::make_unique<UnsupportedAction>(delegate, action);
......
......@@ -447,6 +447,7 @@ message ActionProto {
PopupMessageProto popup_message = 44;
WaitForDocumentProto wait_for_document = 45;
ShowGenericUiProto show_generic_ui = 49;
GeneratePasswordForFormFieldProto generate_password_for_form_field = 52;
}
// Set to true to make the client remove any contextual information if the
......@@ -454,7 +455,7 @@ message ActionProto {
// action sent to the client after this one. Default is false.
optional bool clean_contextual_ui = 33;
reserved 34, 46, 47, 48;
reserved 34, 46, 47, 48, 50, 51;
}
// Result of |CollectUserDataProto| to be sent to the server.
......@@ -1557,10 +1558,6 @@ message ShowDetailsProto {
// Set the value of an form element.
message SetFormFieldValueProto {
message Result { optional bool fallback_to_simulate_key_presses = 1; }
message GeneratePassword {
// Save the generated password at the specified memory location.
optional string memory_key = 1;
}
message KeyPress {
oneof keypress {
// Text to insert as-is into a form field.
......@@ -1579,9 +1576,9 @@ message SetFormFieldValueProto {
bool use_password = 5;
// Use the value stored at the specified memory location.
string client_memory_key = 6;
// Use a password generated by Chrome password manager.
GeneratePassword generate_password = 7;
}
reserved 7;
}
// A reference to the form element whose value should be set.
......@@ -1599,6 +1596,16 @@ message SetFormFieldValueProto {
reserved 5;
}
// Asks the password manager to generate a suitable password for |element|. The
// generated password can be filled in subsequent SetFormFieldValueProto
// actions.
message GeneratePasswordForFormFieldProto {
// A reference to the form element for which to generate a password.
optional ElementReferenceProto element = 1;
// The client memory key to store the generated password.
optional string memory_key = 2;
}
// Set an element attribute to a specific value.
message SetAttributeProto {
// A reference to the form element whose attribute should be set.
......
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