Skip to content

Commit

Permalink
[Autofill] Add parameter for relaxed version of shared-autofill.
Browse files Browse the repository at this point in the history 
This CL adds a parameter relax_shared_autofill to the
AutofillSharedAutofill feature that relaxes the policy for filling
across iframes:

Before this CL, or if relax_shared_autofill is false,
(1) certain field types may be filled if
    (a) the target field has the main origin and
    (b) the target field's frame has shared-autofill enabled;
(2) *all* field types may be filled if
    (a) the source field has the main origin and
    (b) the target field's frame has shared-autofill enabled.

Setting relax_shared_autofill to true lifts condition (2.a).
As a consequence, more fields can be filled, and reasoning about
which fields can be filled becomes easier.

Consider the below example and suppose Autofill is triggered on
the cc-number field.

  <form> <!-- origin https://a.com -->
    <input autocomplete=cc-name>
    <iframe allow=shared-autofill src=https://b.com>
      <input autocomplete=cc-number>
    </iframe>
    <input autocomplete=cc-exp>
    <iframe allow=shared-autofill src=https://c.com>
      <input autocomplete=cc-cvc>
    </iframe>
  </form>

With relax_shared_autofill = false, Autofill only fills cc-name.
It does not fill cc-exp (because it's not among the field types
to which (1) applies) and cc-cvc (because of (2.a)).

With relax_shared_autofill = true, Autofill fills all fields.

Bug: 1187842, 1201849
Change-Id: I22710e5be74e6e1bfb48759943cce864ab1bef23
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3500623
Reviewed-by: Dominic Battré <battre@chromium.org>
Commit-Queue: Christoph Schwering <schwering@google.com>
Cr-Commit-Position: refs/heads/main@{#985546}
  • Loading branch information
@schwering
schwering authored and Chromium LUCI CQ committed Mar 25, 2022
1 parent 9e74ce4 commit 8dc2195
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 25 deletions.
22 changes: 14 additions & 8 deletions components/autofill/content/browser/form_forest.cc
View file
Expand Up @@ -15,6 +15,7 @@
#include "base/stl_util.h"
#include "components/autofill/content/browser/form_forest_util_inl.h"
#include "components/autofill/core/common/autofill_constants.h"
#include "components/autofill/core/common/autofill_features.h"
#include "content/public/browser/render_process_host.h"
#include "third_party/abseil-cpp/absl/types/variant.h"
#include "third_party/blink/public/common/permissions_policy/permissions_policy_features.h"
Expand Down Expand Up @@ -618,8 +619,8 @@ FormForest::RendererForms FormForest::GetRendererFormsOfBrowserForm(
return true;
}
};
// Fields in frames whose permissions policy allows shared-autofill may
// be filled if the |triggered_origin| is the main origin.
// Fields whose document enables the policy-controlled feature
// shared-autofill may be safe to fill.
auto HasSharedAutofillPermission = [&mutable_this](
LocalFrameToken frame_token) {
FrameData* frame = mutable_this.GetFrameData(frame_token);
Expand All @@ -632,12 +633,17 @@ FormForest::RendererForms FormForest::GetRendererFormsOfBrowserForm(
auto it = field_type_map.find(field.global_id());
ServerFieldType field_type =
it != field_type_map.end() ? it->second : UNKNOWN_TYPE;
return field.origin == triggered_origin ||
(field.origin == main_origin &&
HasSharedAutofillPermission(renderer_form->host_frame) &&
!IsSensitiveFieldType(field_type)) ||
(triggered_origin == main_origin &&
HasSharedAutofillPermission(renderer_form->host_frame));
if (features::kAutofillSharedAutofillRelaxedParam.Get()) {
return field.origin == triggered_origin ||
HasSharedAutofillPermission(renderer_form->host_frame);
} else {
return field.origin == triggered_origin ||
(field.origin == main_origin &&
HasSharedAutofillPermission(renderer_form->host_frame) &&
!IsSensitiveFieldType(field_type)) ||
(triggered_origin == main_origin &&
HasSharedAutofillPermission(renderer_form->host_frame));
}
};

renderer_form->fields.push_back(browser_field);
Expand Down
29 changes: 20 additions & 9 deletions components/autofill/content/browser/form_forest.h
View file
Expand Up @@ -232,25 +232,36 @@ class FormForest {
// The |field_type_map| should contain the field types of the fields in
// |browser_form|.
//
// A field is *safe to fill* iff at least one of the conditions (1), (2), (3)
// and additionally condition (4) hold:
// There are two modes that determine whether a field is *safe to fill*.
// By default, a field is safe to fill iff at least one of the conditions
// (1–3) and additionally condition (4) hold:
//
// (1) The field's origin is the |triggered_origin|.
// (2) The field's origin is the main origin and the field's type in
// |field_type_map| is not sensitive (see is_sensitive_field_type()).
// (3) The |triggered_origin| is main origin and the field's frame's
// permissions policy allows shared-autofill.
// (2) The field's origin is the main origin, the field's type in
// |field_type_map| is not sensitive (see IsSensitiveFieldType()), and the
// policy-controlled feature shared-autofill is enabled in the field's
// frame.
// (3) The |triggered_origin| is the main origin and the policy-controlled
// feature shared-autofill is enabled in the field's frame.
// (4) No frame on the shortest path from the field on which Autofill was
// triggered to the field in question, except perhaps the shallowest
// frame, is a fenced frame.
//
// If the Finch parameter relax_shared_autofill is true, the restriction to
// the main origin in condition 3 is lifted. Thus, conditions (2) and (3)
// reduce to the following:
//
// (2+3) The policy-controlled feature shared-autofill is enabled in the
// field's document.
//
// The *origin of a field* is the origin of the frame that contains the
// corresponding form-control element.
//
// The *main origin* is `browser_form.main_frame_origin`.
//
// A frame's *permissions policy allows shared-autofill* if that frame is a
// main frame or its embedding <iframe> element lists "shared-autofill" in
// its "allow" attribute (see https://www.w3.org/TR/permissions-policy-1/).
// The "allow" attribute of the <iframe> element controls whether the
// *policy-controlled feature shared-autofill* is enabled in a document
// (see https://www.w3.org/TR/permissions-policy-1/).
RendererForms GetRendererFormsOfBrowserForm(
const FormData& browser_form,
const url::Origin& triggered_origin,
Expand Down
51 changes: 44 additions & 7 deletions components/autofill/content/browser/form_forest_unittest.cc
View file
Expand Up @@ -12,10 +12,12 @@

#include "base/strings/strcat.h"
#include "base/strings/string_piece.h"
#include "base/test/scoped_feature_list.h"
#include "components/autofill/content/browser/form_forest.h"
#include "components/autofill/content/browser/form_forest_test_api.h"
#include "components/autofill/content/browser/form_forest_util_inl.h"
#include "components/autofill/core/browser/autofill_test_utils.h"
#include "components/autofill/core/common/autofill_features.h"
#include "content/public/test/navigation_simulator.h"
#include "content/public/test/test_renderer_host.h"
#include "testing/gmock/include/gmock/gmock.h"
Expand Down Expand Up @@ -312,6 +314,13 @@ class FormForestTest : public content::RenderViewHostTestHarness {
// FormForest::GetBrowserFormOfRendererForm() for details).
enum class Policy { kDefault, kSharedAutofill, kNoSharedAutofill };

explicit FormForestTest(bool relax_shared_autofill = false) {
feature_list_.InitAndEnableFeatureWithParameters(
features::kAutofillSharedAutofill,
{{features::kAutofillSharedAutofillRelaxedParam.name,
relax_shared_autofill ? "true" : "false"}});
}

void SetUp() override {
RenderViewHostTestHarness::SetUp();
CHECK(kOpaqueOrigin.opaque());
Expand Down Expand Up @@ -400,6 +409,7 @@ class FormForestTest : public content::RenderViewHostTestHarness {
return it->second.get();
}

base::test::ScopedFeatureList feature_list_;
std::map<content::RenderFrameHost*,
std::unique_ptr<MockContentAutofillDriver>>
autofill_drivers_;
Expand Down Expand Up @@ -437,6 +447,10 @@ class FormForestTestWithMockedTree : public FormForestTest {
size_t count = base::dynamic_extent;
};

explicit FormForestTestWithMockedTree(bool relax_shared_autofill = false)
: FormForestTest(
/*relax_shared_autofill=*/relax_shared_autofill) {}

void TearDown() override {
mocked_forms_.Reset();
flattened_forms_.Reset();
Expand Down Expand Up @@ -1399,6 +1413,11 @@ INSTANTIATE_TEST_SUITE_P(FormForestTest,
// Tests of FormForest::GetRendererFormsOfBrowserForm().

class FormForestTestUnflatten : public FormForestTestWithMockedTree {
public:
explicit FormForestTestUnflatten(bool relax_shared_autofill = false)
: FormForestTestWithMockedTree(
/*relax_shared_autofill=*/relax_shared_autofill) {}

protected:
// The subject of this test fixture.
std::vector<FormData> GetRendererFormsOfBrowserForm(
Expand Down Expand Up @@ -1570,9 +1589,17 @@ TEST_F(FormForestTestUnflatten, MainOriginPolicyWithoutSharedAutofill) {
}

// Fixture for the shared-autofill policy tests.
// The parameter controls the value of relax_shared_autofill.
class FormForestTestUnflattenSharedAutofillPolicy
: public FormForestTestUnflatten {
: public FormForestTestUnflatten,
public ::testing::WithParamInterface<bool> {
public:
FormForestTestUnflattenSharedAutofillPolicy()
: FormForestTestUnflatten(
/*relax_shared_autofill=*/relax_shared_autofill()) {}

bool relax_shared_autofill() const { return GetParam(); }

void SetUp() override {
FormForestTestUnflatten::SetUp();
MockFormForest(
Expand All @@ -1589,7 +1616,7 @@ class FormForestTestUnflattenSharedAutofillPolicy
};

// Tests filling into frames with shared-autofill policy from the main origin.
TEST_F(FormForestTestUnflattenSharedAutofillPolicy, FromMainOrigin) {
TEST_P(FormForestTestUnflattenSharedAutofillPolicy, FromMainOrigin) {
MockFlattening({{"main"}, {"disallowed"}, {"allowed"}});
std::vector<FormData> expectation = {
WithValues(GetMockedForm("main"), Profile(0)),
Expand All @@ -1600,12 +1627,18 @@ TEST_F(FormForestTestUnflattenSharedAutofillPolicy, FromMainOrigin) {
}

// Tests filling into frames with shared-autofill policy from the main origin.
TEST_F(FormForestTestUnflattenSharedAutofillPolicy, FromOtherOrigin) {
TEST_P(FormForestTestUnflattenSharedAutofillPolicy, FromOtherOrigin) {
MockFlattening({{"main"}, {"disallowed"}, {"allowed"}});
std::vector<FormData> expectation = {
WithoutValues(GetMockedForm("main")),
WithValues(GetMockedForm("disallowed"), Profile(1)),
WithoutValues(GetMockedForm("allowed"))};
std::vector<FormData> expectation;
if (!relax_shared_autofill()) {
expectation = {WithoutValues(GetMockedForm("main")),
WithValues(GetMockedForm("disallowed"), Profile(1)),
WithoutValues(GetMockedForm("allowed"))};
} else {
expectation = {WithValues(GetMockedForm("main"), Profile(0)),
WithValues(GetMockedForm("disallowed"), Profile(1)),
WithValues(GetMockedForm("allowed"), Profile(2))};
}
EXPECT_THAT(GetRendererFormsOfBrowserForm("main", Origin(kOtherUrl), {}),
UnorderedArrayEquals(expectation));
}
Expand Down Expand Up @@ -1680,6 +1713,10 @@ TEST_P(ForEachInSetDifferenceTest, Test) {
EXPECT_EQ(num_equals_calls_, GetParam().expected_comparisons);
}

INSTANTIATE_TEST_SUITE_P(FormForestTest,
FormForestTestUnflattenSharedAutofillPolicy,
testing::Bool());

INSTANTIATE_TEST_SUITE_P(
FormForestTest,
ForEachInSetDifferenceTest,
Expand Down
6 changes: 5 additions & 1 deletion components/autofill/core/common/autofill_features.cc
View file
Expand Up @@ -415,9 +415,13 @@ const base::Feature kAutofillServerCommunication{

// Controls whether Autofill may fill across origins as part of the
// AutofillAcrossIframes experiment.
// TODO(crbug.com/1220038): Clean up when launched.
// TODO(crbug.com/1304721): Clean up when launched.
const base::Feature kAutofillSharedAutofill{"AutofillSharedAutofill",
base::FEATURE_DISABLED_BY_DEFAULT};
// Relaxes the conditions under which a field is safe to fill.
// See FormForest::GetRendererFormsOfBrowserForm() for details.
const base::FeatureParam<bool> kAutofillSharedAutofillRelaxedParam{
&kAutofillSharedAutofill, "relax_shared_autofill", false};

// Controls attaching the autofill type predictions to their respective
// element in the DOM.
Expand Down
2 changes: 2 additions & 0 deletions components/autofill/core/common/autofill_features.h
View file
Expand Up @@ -149,6 +149,8 @@ extern const base::Feature kAutofillServerCommunication;
COMPONENT_EXPORT(AUTOFILL)
extern const base::Feature kAutofillSharedAutofill;
COMPONENT_EXPORT(AUTOFILL)
extern const base::FeatureParam<bool> kAutofillSharedAutofillRelaxedParam;
COMPONENT_EXPORT(AUTOFILL)
extern const base::Feature kAutofillShowTypePredictions;
COMPONENT_EXPORT(AUTOFILL)
extern const base::Feature kAutofillSilentProfileUpdateForInsufficientImport;
Expand Down

0 comments on commit 8dc2195

Please sign in to comment.