Commit 77972364 authored by Dominic Battre's avatar Dominic Battre Committed by Commit Bot

Record form and URL signatures in UKM

This CL introduces low entropy hashes of password form signatures and URLs.
This allows us to distinguish different forms on a page as well as different
pages for which we cannot record the full URL in UKM.

Bug: 732846
Change-Id: Ie98ef254286890310f6b6b5a2e3026260b65b676
Reviewed-on: https://chromium-review.googlesource.com/977904Reviewed-by: default avatarSteven Holte <holte@chromium.org>
Reviewed-by: default avatarDominic Battré <battre@chromium.org>
Reviewed-by: default avatarMaxim Kolosovskiy <kolos@chromium.org>
Commit-Queue: Dominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549141}
parent 3d043de8
......@@ -261,6 +261,8 @@ void PasswordFormManager::Init(
client_->IsMainFrameSecure(), client_->GetUkmSourceId());
}
metrics_recorder_->RecordFormSignature(observed_form_signature_);
if (owned_form_fetcher_)
owned_form_fetcher_->Fetch();
form_fetcher_->AddConsumer(this);
......
......@@ -377,6 +377,11 @@ class TestPasswordManagerClient : public StubPasswordManagerClient {
void KillDriver() { driver_.reset(); }
const GURL& GetMainFrameURL() const override {
static GURL url("https://www.example.com");
return url;
}
private:
std::unique_ptr<TestingPrefServiceSimple> prefs_;
std::unique_ptr<MockPasswordManagerDriver> driver_;
......@@ -4296,6 +4301,52 @@ TEST_F(PasswordFormManagerTest, TestUkmForFilling) {
}
}
}
// Verifies that the form signature of forms is recorded in UKMs.
TEST_F(PasswordFormManagerTest, TestUkmContextMetrics) {
ukm::TestAutoSetUkmRecorder test_ukm_recorder;
test_ukm_recorder.UpdateSourceURL(client()->GetUkmSourceId(),
client()->GetMainFrameURL());
// Register two forms on one page.
PasswordForm second_observed_form = *observed_form();
second_observed_form.form_data.action = GURL("https://somewhere-else.com");
for (PasswordForm* form : {observed_form(), &second_observed_form}) {
auto metrics_recorder = base::MakeRefCounted<PasswordFormMetricsRecorder>(
form->origin.SchemeIsCryptographic(), client()->GetUkmSourceId());
FakeFormFetcher fetcher;
PasswordFormManager form_manager(
password_manager(), client(), client()->driver(), *form,
std::make_unique<NiceMock<MockFormSaver>>(), &fetcher);
form_manager.Init(metrics_recorder);
}
// Verify that a form signatures have been recorded in UKM.
int64_t form_signature_1 = PasswordFormMetricsRecorder::HashFormSignature(
CalculateFormSignature(observed_form()->form_data));
int64_t form_signature_2 = PasswordFormMetricsRecorder::HashFormSignature(
CalculateFormSignature(second_observed_form.form_data));
EXPECT_GE(form_signature_1, 0);
EXPECT_GE(form_signature_2, 0);
auto entries = test_ukm_recorder.GetEntriesByName(
ukm::builders::PasswordForm::kEntryName);
ASSERT_EQ(2u, entries.size());
const int64_t* metric1 = test_ukm_recorder.GetEntryMetric(
entries[0], ukm::builders::PasswordForm::kContext_FormSignatureName);
const int64_t* metric2 = test_ukm_recorder.GetEntryMetric(
entries[1], ukm::builders::PasswordForm::kContext_FormSignatureName);
ASSERT_TRUE(metric1);
ASSERT_TRUE(metric2);
EXPECT_THAT(
std::vector<int64_t>({*metric1, *metric2}),
::testing::UnorderedElementsAre(form_signature_1, form_signature_2));
}
TEST_F(PasswordFormManagerTest,
TestSendNotBlacklistedMessage_BlacklistedCredentials) {
// Signing up on a previously visited site. Credentials are found in the
......
......@@ -218,6 +218,19 @@ void PasswordFormMetricsRecorder::RecordDetailedUserAction(
detailed_user_actions_counts_[action]++;
}
// static
int64_t PasswordFormMetricsRecorder::HashFormSignature(
autofill::FormSignature form_signature) {
// Note that this is an intentionally small hash domain for privacy reasons.
return static_cast<uint64_t>(form_signature) % 1021;
}
void PasswordFormMetricsRecorder::RecordFormSignature(
autofill::FormSignature form_signature) {
ukm_entry_builder_.SetContext_FormSignature(
HashFormSignature(form_signature));
}
int PasswordFormMetricsRecorder::GetActionsTaken() const {
return static_cast<int>(user_action_) +
static_cast<int>(UserAction::kMax) *
......
......@@ -14,6 +14,7 @@
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "components/autofill/core/common/password_form.h"
#include "components/autofill/core/common/signatures_util.h"
#include "components/password_manager/core/browser/password_form_user_action.h"
#include "components/password_manager/core/browser/password_manager_metrics_util.h"
#include "services/metrics/public/cpp/ukm_builders.h"
......@@ -223,6 +224,13 @@ class PasswordFormMetricsRecorder
// Records a DetailedUserAction UKM metric.
void RecordDetailedUserAction(DetailedUserAction action);
// Hash algorithm for RecordFormSignature. Public for testing.
static int64_t HashFormSignature(autofill::FormSignature form_signature);
// Records a low entropy hash of the form signature in order to be able to
// distinguish two forms on the same site.
void RecordFormSignature(autofill::FormSignature form_signature);
private:
friend class base::RefCounted<PasswordFormMetricsRecorder>;
......
......@@ -2018,6 +2018,12 @@ be describing additional metrics about the same event.
them. A separate event is generated for each password form discovered on a
site.
</summary>
<metric name="Context.FormSignature">
<summary>
Records a low entropy hash of the form signature in order to be able to
distinguish two forms on the same site.
</summary>
</metric>
<metric name="ManagerFill.Action">
<summary>
Records for each password form (and HTTP auth), whether the password
......
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