Commit 220ea40b authored by Martin Sramek's avatar Martin Sramek Committed by Commit Bot

Add an account ID to the UserConsent proto.

This ID will be used to make sure that we don't mistakenly associate
the consent with an incorrect account if the user gives consent for an
account before finishing sign-in, or if the user signs out and signs in
to a different account before the proto can be transmitted.

Currently, the account ID is only an empty string. The field will be
populated once
https://chromium-review.googlesource.com/c/chromium/src/+/986293
lands. However, this CL can unblock the corresponding changes in
UserEventService that will consume this field.

Bug: 781765
Change-Id: I2dae998c599c2480771e54315627ee2be3bafe5d
Reviewed-on: https://chromium-review.googlesource.com/998195
Commit-Queue: Martin Šrámek <msramek@chromium.org>
Reviewed-by: default avatarMikel Astiz <mastiz@chromium.org>
Reviewed-by: default avatarvitaliii <vitaliii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548862}
parent 8e8c8f57
...@@ -103,13 +103,16 @@ void ConsentAuditor::RecordGaiaConsent( ...@@ -103,13 +103,16 @@ void ConsentAuditor::RecordGaiaConsent(
break; break;
} }
// TODO(msramek): Pass in the actual account id.
std::unique_ptr<sync_pb::UserEventSpecifics> specifics = ConstructUserConsent( std::unique_ptr<sync_pb::UserEventSpecifics> specifics = ConstructUserConsent(
feature, description_grd_ids, confirmation_grd_id, status); /* account_id = */ std::string(), feature, description_grd_ids,
confirmation_grd_id, status);
user_event_service_->RecordUserEvent(std::move(specifics)); user_event_service_->RecordUserEvent(std::move(specifics));
} }
std::unique_ptr<sync_pb::UserEventSpecifics> std::unique_ptr<sync_pb::UserEventSpecifics>
ConsentAuditor::ConstructUserConsent( ConsentAuditor::ConstructUserConsent(
const std::string& account_id,
Feature feature, Feature feature,
const std::vector<int>& description_grd_ids, const std::vector<int>& description_grd_ids,
int confirmation_grd_id, int confirmation_grd_id,
...@@ -118,6 +121,7 @@ ConsentAuditor::ConstructUserConsent( ...@@ -118,6 +121,7 @@ ConsentAuditor::ConstructUserConsent(
specifics->set_event_time_usec( specifics->set_event_time_usec(
base::Time::Now().since_origin().InMicroseconds()); base::Time::Now().since_origin().InMicroseconds());
auto* consent = specifics->mutable_user_consent(); auto* consent = specifics->mutable_user_consent();
consent->set_account_id(account_id);
consent->set_feature(FeatureToProtoEnum(feature)); consent->set_feature(FeatureToProtoEnum(feature));
for (int id : description_grd_ids) { for (int id : description_grd_ids) {
consent->add_description_grd_ids(id); consent->add_description_grd_ids(id);
......
...@@ -82,6 +82,7 @@ class ConsentAuditor : public KeyedService { ...@@ -82,6 +82,7 @@ class ConsentAuditor : public KeyedService {
private: private:
std::unique_ptr<sync_pb::UserEventSpecifics> ConstructUserConsent( std::unique_ptr<sync_pb::UserEventSpecifics> ConstructUserConsent(
const std::string& account_id,
Feature feature, Feature feature,
const std::vector<int>& description_grd_ids, const std::vector<int>& description_grd_ids,
int confirmation_grd_id, int confirmation_grd_id,
......
...@@ -191,6 +191,9 @@ TEST_F(ConsentAuditorTest, RecordGaiaConsent) { ...@@ -191,6 +191,9 @@ TEST_F(ConsentAuditorTest, RecordGaiaConsent) {
EXPECT_FALSE(events[0].has_navigation_id()); EXPECT_FALSE(events[0].has_navigation_id());
EXPECT_TRUE(events[0].has_user_consent()); EXPECT_TRUE(events[0].has_user_consent());
auto& consent = events[0].user_consent(); auto& consent = events[0].user_consent();
// TODO(crbug.com/781765): The |account_id| is not passed into
// ConsentAuditor yet.
EXPECT_EQ("", consent.account_id());
EXPECT_EQ(UserEventSpecifics::UserConsent::CHROME_SYNC, consent.feature()); EXPECT_EQ(UserEventSpecifics::UserConsent::CHROME_SYNC, consent.feature());
EXPECT_EQ(3, consent.description_grd_ids_size()); EXPECT_EQ(3, consent.description_grd_ids_size());
EXPECT_EQ(kDescriptionMessageIds[0], consent.description_grd_ids(0)); EXPECT_EQ(kDescriptionMessageIds[0], consent.description_grd_ids(0));
......
...@@ -874,6 +874,7 @@ VISIT_PROTO_FIELDS(const sync_pb::UserEventSpecifics::UserConsent& proto) { ...@@ -874,6 +874,7 @@ VISIT_PROTO_FIELDS(const sync_pb::UserEventSpecifics::UserConsent& proto) {
VISIT(confirmation_grd_id); VISIT(confirmation_grd_id);
VISIT(locale); VISIT(locale);
VISIT_ENUM(status); VISIT_ENUM(status);
VISIT(account_id);
} }
VISIT_PROTO_FIELDS(const sync_pb::TypeHint& proto) { VISIT_PROTO_FIELDS(const sync_pb::TypeHint& proto) {
......
...@@ -114,6 +114,17 @@ message UserEventSpecifics { ...@@ -114,6 +114,17 @@ message UserEventSpecifics {
GIVEN = 2; GIVEN = 2;
} }
optional ConsentStatus status = 5; optional ConsentStatus status = 5;
// The account ID of the user who gave the consent. This field is used
// by UserEventService to distinguish consents from different users,
// as UserConsent does not get deleted when a user signs out. However,
// it should be cleared before being sent over the wire, as the UserEvent
// is sent over an authenticated channel, so this information would be
// redundant.
//
// For semantics and usage of the |account_id| in the signin codebase,
// see SigninManagerBase::GetAuthenticatedAccountId()
// or AccountInfo::account_id.
optional string account_id = 6;
} }
// User reused their GAIA password on another website. // User reused their GAIA password on another website.
......
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