Commit 5efd970e authored by Martin Sramek's avatar Martin Sramek Committed by Commit Bot

Pass the Account ID from consent UI surfaces to Consent Auditor

The Account ID of the account for which the user granted consent
is passed from:
- Desktop: SyncConfirmationHandler
- Android: ConsentTracker
- iOS: ChromeSigninViewController
- ChromeOS: ArcSupportHost, ArcTermsOfServiceScreenHandler

to ConsentAuditor. In this CL, we only verify that it's not empty.

A subsequent CL will pass it to UserEventService which will verify that
it's the same account that is currently signed in, to avoid sending
consents for account A over an authenticated channel of account B
(and associating the consent with B on the server).

Bug: 781765
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I5179b8932a3ec8fd865e97c2f8164ac2715ac840
Reviewed-on: https://chromium-review.googlesource.com/986293
Commit-Queue: Martin Šrámek <msramek@chromium.org>
Reviewed-by: default avatarBernhard Bauer <bauerb@chromium.org>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Reviewed-by: default avatarBoris Sazonov <bsazonov@chromium.org>
Reviewed-by: default avatarChristian Dullweber <dullweber@chromium.org>
Reviewed-by: default avatarMihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549870}
parent 553e6443
......@@ -24,14 +24,14 @@ public final class ConsentAuditorBridge {
* @param consentDescription The resource IDs of the text the user read before consenting.
* @param consentConfirmation The resource ID of the text the user clicked when consenting.
*/
public void recordConsent(@ConsentAuditorFeature int feature, List<Integer> consentDescription,
@StringRes int consentConfirmation) {
public void recordConsent(String accountId, @ConsentAuditorFeature int feature,
List<Integer> consentDescription, @StringRes int consentConfirmation) {
int[] consentDescriptionArray = new int[consentDescription.size()];
for (int i = 0; i < consentDescription.size(); ++i) {
consentDescriptionArray[i] = consentDescription.get(i);
}
nativeRecordConsent(Profile.getLastUsedProfile(), feature, consentDescriptionArray,
consentConfirmation);
nativeRecordConsent(Profile.getLastUsedProfile(), accountId, feature,
consentDescriptionArray, consentConfirmation);
}
private ConsentAuditorBridge() {}
......@@ -45,6 +45,6 @@ public final class ConsentAuditorBridge {
return sInstance;
}
private native void nativeRecordConsent(
Profile profile, int feature, int[] consentDescription, int consentConfirmation);
private native void nativeRecordConsent(Profile profile, String accountId, int feature,
int[] consentDescription, int consentConfirmation);
}
......@@ -6,6 +6,7 @@ package org.chromium.chrome.browser.signin;
import android.app.Activity;
import android.content.Context;
import android.os.AsyncTask;
import android.os.Bundle;
import android.os.SystemClock;
import android.support.annotation.IntDef;
......@@ -30,6 +31,7 @@ import org.chromium.chrome.browser.externalauth.UserRecoverableErrorHandler;
import org.chromium.chrome.browser.preferences.PrefServiceBridge;
import org.chromium.chrome.browser.signin.AccountTrackerService.OnSystemAccountsSeededListener;
import org.chromium.chrome.browser.signin.ConfirmImportSyncDataDialog.ImportSyncType;
import org.chromium.components.signin.AccountIdProvider;
import org.chromium.components.signin.AccountManagerDelegateException;
import org.chromium.components.signin.AccountManagerFacade;
import org.chromium.components.signin.AccountManagerResult;
......@@ -797,7 +799,21 @@ public class AccountSigninView extends FrameLayout {
* @param confirmationView The view that the user clicked when consenting.
*/
private void recordConsent(TextView confirmationView) {
mConsentTextTracker.recordConsent(ConsentAuditorFeature.CHROME_SYNC, confirmationView,
findViewById(R.id.signin_confirmation_view), findViewById(R.id.button_bar));
// TODO(crbug.com/831257): Provide the account id synchronously from AccountManagerFacade.
final AccountIdProvider accountIdProvider = AccountIdProvider.getInstance();
new AsyncTask<Void, Void, String>() {
@Override
public String doInBackground(Void... params) {
return accountIdProvider.getAccountId(mSelectedAccountName);
}
@Override
public void onPostExecute(String accountId) {
mConsentTextTracker.recordConsent(accountId, ConsentAuditorFeature.CHROME_SYNC,
confirmationView, findViewById(R.id.signin_confirmation_view),
findViewById(R.id.button_bar));
}
}
.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
}
}
......@@ -154,12 +154,13 @@ public class ConsentTextTracker {
/**
* Records the consent.
* @param accountId The account for which the consent is valid
* @param feature {@link ConsentAuditorFeature} that user has consented to
* @param confirmationView The view that the user clicked when consenting
* @param consentViews View hierarchies that implement the consent screen
*/
public void recordConsent(
@ConsentAuditorFeature int feature, TextView confirmationView, View... consentViews) {
public void recordConsent(String accountId, @ConsentAuditorFeature int feature,
TextView confirmationView, View... consentViews) {
int consentConfirmation = getConsentStringResource(confirmationView);
ArrayList<Integer> consentDescription = new ArrayList<>();
......@@ -177,6 +178,6 @@ public class ConsentTextTracker {
}
ConsentAuditorBridge.getInstance().recordConsent(
feature, consentDescription, consentConfirmation);
accountId, feature, consentDescription, consentConfirmation);
}
}
......@@ -4,6 +4,7 @@
package org.chromium.chrome.browser.signin;
import android.os.AsyncTask;
import android.os.Bundle;
import android.os.SystemClock;
import android.support.annotation.Nullable;
......@@ -23,6 +24,7 @@ import org.chromium.base.metrics.RecordUserAction;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.consent_auditor.ConsentAuditorFeature;
import org.chromium.chrome.browser.externalauth.UserRecoverableErrorHandler;
import org.chromium.components.signin.AccountIdProvider;
import org.chromium.components.signin.AccountManagerDelegateException;
import org.chromium.components.signin.AccountManagerFacade;
import org.chromium.components.signin.AccountManagerResult;
......@@ -242,8 +244,21 @@ public abstract class SigninFragmentBase
* @param confirmationView The view that the user clicked when consenting.
*/
private void recordConsent(TextView confirmationView) {
mConsentTextTracker.recordConsent(
ConsentAuditorFeature.CHROME_SYNC, confirmationView, mView);
// TODO(crbug.com/831257): Provide the account id synchronously from AccountManagerFacade.
final AccountIdProvider accountIdProvider = AccountIdProvider.getInstance();
new AsyncTask<Void, Void, String>() {
@Override
public String doInBackground(Void... params) {
return accountIdProvider.getAccountId(mSelectedAccountName);
}
@Override
public void onPostExecute(String accountId) {
mConsentTextTracker.recordConsent(
accountId, ConsentAuditorFeature.CHROME_SYNC, confirmationView, mView);
}
}
.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
}
private void showAccountPicker() {
......
......@@ -21,6 +21,7 @@ static void JNI_ConsentAuditorBridge_RecordConsent(
JNIEnv* env,
const JavaParamRef<jobject>& obj,
const JavaParamRef<jobject>& j_profile,
const JavaParamRef<jstring>& j_account_id,
jint j_feature,
const JavaParamRef<jintArray>& j_consent_description,
jint j_consent_confirmation) {
......@@ -29,7 +30,8 @@ static void JNI_ConsentAuditorBridge_RecordConsent(
&consent_description);
ConsentAuditorFactory::GetForProfile(
ProfileAndroid::FromProfileAndroid(j_profile))
->RecordGaiaConsent(static_cast<consent_auditor::Feature>(j_feature),
->RecordGaiaConsent(ConvertJavaStringToUTF8(j_account_id),
static_cast<consent_auditor::Feature>(j_feature),
consent_description, j_consent_confirmation,
consent_auditor::ConsentStatus::GIVEN);
}
......@@ -20,6 +20,7 @@
#include "chrome/browser/consent_auditor/consent_auditor_factory.h"
#include "chrome/browser/extensions/extension_util.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/signin/signin_manager_factory.h"
#include "chrome/browser/ui/app_list/arc/arc_app_utils.h"
#include "chrome/browser/ui/ash/multi_user/multi_user_util.h"
#include "chrome/browser/ui/chrome_pages.h"
......@@ -27,6 +28,7 @@
#include "chrome/browser/ui/extensions/application_launch.h"
#include "chrome/grit/generated_resources.h"
#include "components/consent_auditor/consent_auditor.h"
#include "components/signin/core/browser/signin_manager_base.h"
#include "components/user_manager/known_user.h"
#include "extensions/browser/extension_registry.h"
#include "ui/base/l10n/l10n_util.h"
......@@ -645,10 +647,15 @@ void ArcSupportHost::OnMessage(const base::DictionaryValue& message) {
return;
}
SigninManagerBase* signin_manager =
SigninManagerFactory::GetForProfile(profile_);
DCHECK(signin_manager->IsAuthenticated());
std::string account_id = signin_manager->GetAuthenticatedAccountId();
// Record acceptance of ToS if it was shown to the user.
if (tos_shown) {
ConsentAuditorFactory::GetForProfile(profile_)->RecordGaiaConsent(
consent_auditor::Feature::PLAY_STORE,
account_id, consent_auditor::Feature::PLAY_STORE,
ComputePlayToSConsentIds(tos_content),
IDS_ARC_OPT_IN_DIALOG_BUTTON_AGREE,
consent_auditor::ConsentStatus::GIVEN);
......@@ -657,7 +664,7 @@ void ArcSupportHost::OnMessage(const base::DictionaryValue& message) {
// If the user - not policy - chose Backup and Restore, record consent.
if (is_backup_restore_enabled && !is_backup_restore_managed) {
ConsentAuditorFactory::GetForProfile(profile_)->RecordGaiaConsent(
consent_auditor::Feature::BACKUP_AND_RESTORE,
account_id, consent_auditor::Feature::BACKUP_AND_RESTORE,
{IDS_ARC_OPT_IN_DIALOG_BACKUP_RESTORE},
IDS_ARC_OPT_IN_DIALOG_BUTTON_AGREE,
consent_auditor::ConsentStatus::GIVEN);
......@@ -666,7 +673,7 @@ void ArcSupportHost::OnMessage(const base::DictionaryValue& message) {
// If the user - not policy - chose Location Services, record consent.
if (is_location_service_enabled && !is_location_service_managed) {
ConsentAuditorFactory::GetForProfile(profile_)->RecordGaiaConsent(
consent_auditor::Feature::GOOGLE_LOCATION_SERVICE,
account_id, consent_auditor::Feature::GOOGLE_LOCATION_SERVICE,
{IDS_ARC_OPT_IN_LOCATION_SETTING}, IDS_ARC_OPT_IN_DIALOG_BUTTON_AGREE,
consent_auditor::ConsentStatus::GIVEN);
}
......
......@@ -10,9 +10,12 @@
#include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
#include "chrome/browser/consent_auditor/consent_auditor_factory.h"
#include "chrome/browser/consent_auditor/consent_auditor_test_utils.h"
#include "chrome/browser/signin/fake_signin_manager_builder.h"
#include "chrome/browser/signin/signin_manager_factory.h"
#include "chrome/test/base/browser_with_test_window_test.h"
#include "chrome/test/base/testing_profile.h"
#include "components/consent_auditor/fake_consent_auditor.h"
#include "components/signin/core/browser/fake_signin_manager.h"
#include "components/user_manager/scoped_user_manager.h"
#include "content/public/test/test_browser_thread_bundle.h"
#include "testing/gmock/include/gmock/gmock.h"
......@@ -69,6 +72,7 @@ class ArcSupportHostTest : public BrowserWithTestWindowTest {
BrowserWithTestWindowTest::SetUp();
user_manager_enabler_ = std::make_unique<user_manager::ScopedUserManager>(
std::make_unique<chromeos::FakeChromeUserManager>());
signin_manager()->SignIn("testing_account_id");
support_host_ = std::make_unique<ArcSupportHost>(profile());
fake_arc_support_ = std::make_unique<FakeArcSupport>(support_host_.get());
......@@ -119,9 +123,15 @@ class ArcSupportHostTest : public BrowserWithTestWindowTest {
ConsentAuditorFactory::GetForProfile(profile()));
}
FakeSigninManagerBase* signin_manager() {
return static_cast<FakeSigninManagerBase*>(
SigninManagerFactory::GetForProfile(profile()));
}
// BrowserWithTestWindowTest:
TestingProfile::TestingFactories GetTestingFactories() override {
return {{ConsentAuditorFactory::GetInstance(), BuildFakeConsentAuditor}};
return {{SigninManagerFactory::GetInstance(), BuildFakeSigninManagerBase},
{ConsentAuditorFactory::GetInstance(), BuildFakeConsentAuditor}};
}
private:
......
......@@ -16,6 +16,8 @@
#include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
#include "chrome/browser/consent_auditor/consent_auditor_factory.h"
#include "chrome/browser/consent_auditor/consent_auditor_test_utils.h"
#include "chrome/browser/signin/fake_signin_manager_builder.h"
#include "chrome/browser/signin/signin_manager_factory.h"
#include "chrome/grit/generated_resources.h"
#include "chrome/test/base/browser_with_test_window_test.h"
#include "chrome/test/base/testing_browser_process.h"
......@@ -23,6 +25,7 @@
#include "components/arc/arc_prefs.h"
#include "components/consent_auditor/fake_consent_auditor.h"
#include "components/prefs/pref_service.h"
#include "components/signin/core/browser/fake_signin_manager.h"
#include "components/sync_preferences/testing_pref_service_syncable.h"
#include "components/user_manager/scoped_user_manager.h"
#include "content/public/test/test_browser_thread_bundle.h"
......@@ -40,6 +43,7 @@ class ArcTermsOfServiceDefaultNegotiatorTest
BrowserWithTestWindowTest::SetUp();
user_manager_enabler_ = std::make_unique<user_manager::ScopedUserManager>(
std::make_unique<chromeos::FakeChromeUserManager>());
signin_manager()->SignIn("testing_account_id");
support_host_ = std::make_unique<ArcSupportHost>(profile());
fake_arc_support_ = std::make_unique<FakeArcSupport>(support_host_.get());
......@@ -65,9 +69,15 @@ class ArcTermsOfServiceDefaultNegotiatorTest
ConsentAuditorFactory::GetForProfile(profile()));
}
FakeSigninManagerBase* signin_manager() {
return static_cast<FakeSigninManagerBase*>(
SigninManagerFactory::GetForProfile(profile()));
}
// BrowserWithTestWindowTest:
TestingProfile::TestingFactories GetTestingFactories() override {
return {{ConsentAuditorFactory::GetInstance(), BuildFakeConsentAuditor}};
return {{SigninManagerFactory::GetInstance(), BuildFakeSigninManagerBase},
{ConsentAuditorFactory::GetInstance(), BuildFakeConsentAuditor}};
}
private:
......@@ -189,6 +199,8 @@ TEST_F(ArcTermsOfServiceDefaultNegotiatorTest, Accept) {
consent_auditor::ConsentStatus::GIVEN,
consent_auditor::ConsentStatus::GIVEN,
consent_auditor::ConsentStatus::GIVEN};
EXPECT_EQ(consent_auditor()->account_id(),
signin_manager()->GetAuthenticatedAccountId());
EXPECT_EQ(consent_auditor()->recorded_id_vectors(), consent_ids);
EXPECT_EQ(consent_auditor()->recorded_features(), features);
EXPECT_EQ(consent_auditor()->recorded_statuses(), statuses);
......@@ -242,6 +254,8 @@ TEST_F(ArcTermsOfServiceDefaultNegotiatorTest, AcceptWithUnchecked) {
consent_auditor::Feature::PLAY_STORE};
const std::vector<consent_auditor::ConsentStatus> statuses = {
consent_auditor::ConsentStatus::GIVEN};
EXPECT_EQ(consent_auditor()->account_id(),
signin_manager()->GetAuthenticatedAccountId());
EXPECT_EQ(consent_auditor()->recorded_id_vectors(), consent_ids);
EXPECT_EQ(consent_auditor()->recorded_features(), features);
EXPECT_EQ(consent_auditor()->recorded_statuses(), statuses);
......@@ -288,6 +302,8 @@ TEST_F(ArcTermsOfServiceDefaultNegotiatorTest, AcceptWithManagedToS) {
consent_auditor::Feature::GOOGLE_LOCATION_SERVICE};
const std::vector<consent_auditor::ConsentStatus> statuses = {
consent_auditor::ConsentStatus::GIVEN};
EXPECT_EQ(consent_auditor()->account_id(),
signin_manager()->GetAuthenticatedAccountId());
EXPECT_EQ(consent_auditor()->recorded_id_vectors(), consent_ids);
EXPECT_EQ(consent_auditor()->recorded_features(), features);
EXPECT_EQ(consent_auditor()->recorded_statuses(), statuses);
......
......@@ -13,6 +13,7 @@
#include "chrome/browser/consent_auditor/consent_auditor_factory.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/signin/signin_manager_factory.h"
#include "chrome/grit/generated_resources.h"
#include "chromeos/chromeos_switches.h"
#include "chromeos/network/network_handler.h"
......@@ -22,6 +23,7 @@
#include "components/consent_auditor/consent_auditor.h"
#include "components/login/localized_values_builder.h"
#include "components/prefs/pref_service.h"
#include "components/signin/core/browser/signin_manager_base.h"
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_ui.h"
#include "ui/base/l10n/l10n_util.h"
......@@ -258,20 +260,24 @@ void ArcTermsOfServiceScreenHandler::HandleAccept(
pref_handler_->EnableBackupRestore(enable_backup_restore);
pref_handler_->EnableLocationService(enable_location_services);
Profile* profile = ProfileManager::GetActiveUserProfile();
consent_auditor::ConsentAuditor* consent_auditor =
ConsentAuditorFactory::GetForProfile(
ProfileManager::GetPrimaryUserProfile());
ConsentAuditorFactory::GetForProfile(profile);
SigninManagerBase* signin_manager =
SigninManagerFactory::GetForProfile(profile);
DCHECK(signin_manager->IsAuthenticated());
std::string account_id = signin_manager->GetAuthenticatedAccountId();
// Record acceptance of Play ToS.
consent_auditor->RecordGaiaConsent(
consent_auditor::Feature::PLAY_STORE,
account_id, consent_auditor::Feature::PLAY_STORE,
ArcSupportHost::ComputePlayToSConsentIds(tos_content),
IDS_ARC_OOBE_TERMS_BUTTON_ACCEPT, consent_auditor::ConsentStatus::GIVEN);
// If the user - not policy - chose Backup and Restore, record consent.
if (enable_backup_restore && !backup_restore_managed_) {
consent_auditor->RecordGaiaConsent(
consent_auditor::Feature::BACKUP_AND_RESTORE,
account_id, consent_auditor::Feature::BACKUP_AND_RESTORE,
{IDS_ARC_OPT_IN_DIALOG_BACKUP_RESTORE},
IDS_ARC_OOBE_TERMS_BUTTON_ACCEPT,
consent_auditor::ConsentStatus::GIVEN);
......@@ -280,7 +286,7 @@ void ArcTermsOfServiceScreenHandler::HandleAccept(
// If the user - not policy - chose Location Services, record consent.
if (enable_location_services && !location_services_managed_) {
consent_auditor->RecordGaiaConsent(
consent_auditor::Feature::GOOGLE_LOCATION_SERVICE,
account_id, consent_auditor::Feature::GOOGLE_LOCATION_SERVICE,
{IDS_ARC_OPT_IN_LOCATION_SETTING}, IDS_ARC_OOBE_TERMS_BUTTON_ACCEPT,
consent_auditor::ConsentStatus::GIVEN);
}
......
......@@ -117,6 +117,8 @@ void SyncConfirmationHandler::RecordConsent(const base::ListValue* args) {
int consent_confirmation_id = iter->second;
ConsentAuditorFactory::GetForProfile(profile_)->RecordGaiaConsent(
SigninManagerFactory::GetForProfile(profile_)
->GetAuthenticatedAccountId(),
consent_auditor::Feature::CHROME_SYNC, consent_text_ids,
consent_confirmation_id, consent_auditor::ConsentStatus::GIVEN);
}
......
......@@ -389,6 +389,9 @@ TEST_F(SyncConfirmationHandlerTest, TestHandleConfirm) {
// The corresponding string IDs get recorded.
std::vector<std::vector<int>> expected_id_vectors = {{1, 2, 4, 5}};
EXPECT_EQ(expected_id_vectors, consent_auditor()->recorded_id_vectors());
EXPECT_EQ(signin_manager()->GetAuthenticatedAccountId(),
consent_auditor()->account_id());
}
TEST_F(SyncConfirmationHandlerTest, TestHandleConfirmWithAdvancedSyncSettings) {
......@@ -428,4 +431,7 @@ TEST_F(SyncConfirmationHandlerTest, TestHandleConfirmWithAdvancedSyncSettings) {
// The corresponding string IDs get recorded.
std::vector<std::vector<int>> expected_id_vectors = {{2, 3, 5, 2}};
EXPECT_EQ(expected_id_vectors, consent_auditor()->recorded_id_vectors());
EXPECT_EQ(signin_manager()->GetAuthenticatedAccountId(),
consent_auditor()->account_id());
}
......@@ -81,10 +81,13 @@ void ConsentAuditor::RegisterProfilePrefs(PrefRegistrySimple* registry) {
}
void ConsentAuditor::RecordGaiaConsent(
const std::string& account_id,
Feature feature,
const std::vector<int>& description_grd_ids,
int confirmation_grd_id,
ConsentStatus status) {
DCHECK(!account_id.empty()) << "No signed-in account specified.";
if (!base::FeatureList::IsEnabled(switches::kSyncUserConsentEvents))
return;
......@@ -105,8 +108,7 @@ void ConsentAuditor::RecordGaiaConsent(
// TODO(msramek): Pass in the actual account id.
std::unique_ptr<sync_pb::UserEventSpecifics> specifics = ConstructUserConsent(
/* account_id = */ std::string(), feature, description_grd_ids,
confirmation_grd_id, status);
account_id, feature, description_grd_ids, confirmation_grd_id, status);
user_event_service_->RecordUserEvent(std::move(specifics));
}
......
......@@ -50,7 +50,6 @@ enum class ConsentStatus { NOT_GIVEN, GIVEN };
class ConsentAuditor : public KeyedService {
public:
ConsentAuditor(PrefService* pref_service,
syncer::UserEventService* user_event_service,
const std::string& app_version,
......@@ -63,11 +62,13 @@ class ConsentAuditor : public KeyedService {
// Registers the preferences needed by this service.
static void RegisterProfilePrefs(PrefRegistrySimple* registry);
// Records a consent for |feature| for the signed-in GAIA account.
// Records a consent for |feature| for the signed-in GAIA account with
// the ID |account_id| (as defined in AccountInfo).
// Consent text consisted of strings with |consent_grd_ids|, and the UI
// element the user clicked had the ID |confirmation_grd_id|.
// Whether the consent was GIVEN or NOT_GIVEN is passed as |status|.
virtual void RecordGaiaConsent(Feature feature,
virtual void RecordGaiaConsent(const std::string& account_id,
Feature feature,
const std::vector<int>& description_grd_ids,
int confirmation_grd_id,
ConsentStatus status);
......
......@@ -28,6 +28,9 @@ const char kLocalConsentLocaleKey[] = "locale";
const char kCurrentAppVersion[] = "1.2.3.4";
const char kCurrentAppLocale[] = "en-US";
// Fake account ID for testing.
const char kAccountId[] = "testing_account_id";
// A helper function to load the |description|, |confirmation|, |version|,
// and |locale|, in that order, from a record for the |feature| in
// the |consents| dictionary.
......@@ -161,7 +164,7 @@ TEST_F(ConsentAuditorTest, LocalConsentPrefRepresentation) {
}
TEST_F(ConsentAuditorTest, RecordingEnabled) {
consent_auditor()->RecordGaiaConsent(Feature::CHROME_SYNC, {}, 0,
consent_auditor()->RecordGaiaConsent(kAccountId, Feature::CHROME_SYNC, {}, 0,
ConsentStatus::GIVEN);
auto& events = user_event_service()->GetRecordedUserEvents();
EXPECT_EQ(1U, events.size());
......@@ -170,7 +173,7 @@ TEST_F(ConsentAuditorTest, RecordingEnabled) {
TEST_F(ConsentAuditorTest, RecordingDisabled) {
base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndDisableFeature(switches::kSyncUserConsentEvents);
consent_auditor()->RecordGaiaConsent(Feature::CHROME_SYNC, {}, 0,
consent_auditor()->RecordGaiaConsent(kAccountId, Feature::CHROME_SYNC, {}, 0,
ConsentStatus::GIVEN);
auto& events = user_event_service()->GetRecordedUserEvents();
EXPECT_EQ(0U, events.size());
......@@ -181,8 +184,8 @@ TEST_F(ConsentAuditorTest, RecordGaiaConsent) {
int kConfirmationMessageId = 47;
base::Time t1 = base::Time::Now();
consent_auditor()->RecordGaiaConsent(
Feature::CHROME_SYNC, kDescriptionMessageIds, kConfirmationMessageId,
ConsentStatus::GIVEN);
kAccountId, Feature::CHROME_SYNC, kDescriptionMessageIds,
kConfirmationMessageId, ConsentStatus::GIVEN);
base::Time t2 = base::Time::Now();
auto& events = user_event_service()->GetRecordedUserEvents();
EXPECT_EQ(1U, events.size());
......@@ -191,9 +194,7 @@ TEST_F(ConsentAuditorTest, RecordGaiaConsent) {
EXPECT_FALSE(events[0].has_navigation_id());
EXPECT_TRUE(events[0].has_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(kAccountId, consent.account_id());
EXPECT_EQ(UserEventSpecifics::UserConsent::CHROME_SYNC, consent.feature());
EXPECT_EQ(3, consent.description_grd_ids_size());
EXPECT_EQ(kDescriptionMessageIds[0], consent.description_grd_ids(0));
......
......@@ -19,10 +19,12 @@ FakeConsentAuditor::FakeConsentAuditor(
FakeConsentAuditor::~FakeConsentAuditor() {}
void FakeConsentAuditor::RecordGaiaConsent(
const std::string& account_id,
consent_auditor::Feature feature,
const std::vector<int>& description_grd_ids,
int confirmation_grd_id,
consent_auditor::ConsentStatus status) {
account_id_ = account_id;
std::vector<int> ids = description_grd_ids;
ids.push_back(confirmation_grd_id);
recorded_id_vectors_.push_back(std::move(ids));
......
......@@ -24,11 +24,14 @@ class FakeConsentAuditor : public ConsentAuditor {
syncer::UserEventService* user_event_service);
~FakeConsentAuditor() override;
void RecordGaiaConsent(consent_auditor::Feature feature,
void RecordGaiaConsent(const std::string& account_id,
consent_auditor::Feature feature,
const std::vector<int>& description_grd_ids,
int confirmation_grd_id,
consent_auditor::ConsentStatus status) override;
const std::string& account_id() const { return account_id_; }
const std::vector<std::vector<int>>& recorded_id_vectors() {
return recorded_id_vectors_;
}
......@@ -40,6 +43,7 @@ class FakeConsentAuditor : public ConsentAuditor {
}
private:
std::string account_id_;
std::vector<std::vector<int>> recorded_id_vectors_;
std::vector<Feature> recorded_features_;
std::vector<ConsentStatus> recorded_statuses_;
......
......@@ -135,6 +135,7 @@ source_set("unit_tests") {
"//base/test:test_support",
"//components/consent_auditor",
"//components/pref_registry",
"//components/signin/core/browser:browser",
"//components/sync_preferences",
"//components/sync_preferences:test_support",
"//components/version_info",
......
......@@ -17,11 +17,13 @@
#import "base/strings/sys_string_conversions.h"
#include "base/timer/elapsed_timer.h"
#include "components/consent_auditor/consent_auditor.h"
#include "components/signin/core/browser/account_tracker_service.h"
#include "components/signin/core/browser/signin_metrics.h"
#include "components/strings/grit/components_strings.h"
#import "ios/chrome/browser/browser_state/chrome_browser_state.h"
#include "ios/chrome/browser/chrome_url_constants.h"
#include "ios/chrome/browser/consent_auditor/consent_auditor_factory.h"
#include "ios/chrome/browser/signin/account_tracker_service_factory.h"
#import "ios/chrome/browser/signin/authentication_service.h"
#import "ios/chrome/browser/signin/authentication_service_factory.h"
#import "ios/chrome/browser/signin/chrome_identity_service_observer_bridge.h"
......@@ -235,8 +237,13 @@ enum AuthenticationState {
int consent_confirmation_id = showAccountsSettings
? _confirmationVC.openSettingsStringId
: [self acceptSigninButtonStringId];
std::string account_id =
ios::AccountTrackerServiceFactory::GetForBrowserState(_browserState)
->PickAccountIdForAccount(
base::SysNSStringToUTF8([_selectedIdentity gaiaID]),
base::SysNSStringToUTF8([_selectedIdentity userEmail]));
ConsentAuditorFactory::GetForBrowserState(_browserState)
->RecordGaiaConsent(consent_auditor::Feature::CHROME_SYNC,
->RecordGaiaConsent(account_id, consent_auditor::Feature::CHROME_SYNC,
consent_text_ids, consent_confirmation_id,
consent_auditor::ConsentStatus::GIVEN);
_didAcceptSignIn = YES;
......
......@@ -5,13 +5,16 @@
#include "ios/chrome/browser/ui/authentication/chrome_signin_view_controller.h"
#include "base/memory/ptr_util.h"
#import "base/strings/sys_string_conversions.h"
#import "base/test/ios/wait_util.h"
#include "base/timer/mock_timer.h"
#include "components/consent_auditor/consent_auditor.h"
#include "components/signin/core/browser/account_tracker_service.h"
#include "components/version_info/version_info.h"
#include "ios/chrome/browser/application_context.h"
#include "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
#include "ios/chrome/browser/consent_auditor/consent_auditor_factory.h"
#include "ios/chrome/browser/signin/account_tracker_service_factory.h"
#import "ios/chrome/browser/signin/authentication_service_factory.h"
#import "ios/chrome/browser/signin/authentication_service_fake.h"
#include "ios/chrome/browser/sync/ios_user_event_service_factory.h"
......@@ -57,22 +60,26 @@ class FakeConsentAuditor : public consent_auditor::ConsentAuditor {
app_locale) {}
~FakeConsentAuditor() override {}
void RecordGaiaConsent(consent_auditor::Feature feature,
void RecordGaiaConsent(const std::string& account_id,
consent_auditor::Feature feature,
const std::vector<int>& description_grd_ids,
int confirmation_string_id,
consent_auditor::ConsentStatus status) override {
account_id_ = account_id;
feature_ = feature;
recorded_ids_ = description_grd_ids;
confirmation_string_id_ = confirmation_string_id;
status_ = status;
}
const std::string& account_id() const { return account_id_; }
consent_auditor::Feature feature() const { return feature_; }
const std::vector<int>& recorded_ids() const { return recorded_ids_; }
int confirmation_string_id() const { return confirmation_string_id_; }
consent_auditor::ConsentStatus status() const { return status_; }
private:
std::string account_id_;
consent_auditor::Feature feature_;
std::vector<int> recorded_ids_;
int confirmation_string_id_ = -1;
......@@ -105,6 +112,8 @@ class ChromeSigninViewControllerTest : public PlatformTest {
ios::FakeChromeIdentityService* identity_service =
ios::FakeChromeIdentityService::GetInstanceFromChromeProvider();
identity_service->AddIdentity(identity_);
account_tracker_service_ =
ios::AccountTrackerServiceFactory::GetForBrowserState(context_.get());
fake_consent_auditor_ = static_cast<FakeConsentAuditor*>(
ConsentAuditorFactory::GetForBrowserState(context_.get()));
// Setup view controller.
......@@ -228,6 +237,7 @@ class ChromeSigninViewControllerTest : public PlatformTest {
UIWindow* window_;
ChromeSigninViewController* vc_;
FakeConsentAuditor* fake_consent_auditor_;
AccountTrackerService* account_tracker_service_;
};
// Tests that all strings on the screen are either part of the consent string
......@@ -253,6 +263,10 @@ TEST_F(ChromeSigninViewControllerTest, TestConsentWithOKGOTIT) {
fake_consent_auditor_->status());
EXPECT_EQ(consent_auditor::Feature::CHROME_SYNC,
fake_consent_auditor_->feature());
EXPECT_EQ(account_tracker_service_->PickAccountIdForAccount(
base::SysNSStringToUTF8([identity_ gaiaID]),
base::SysNSStringToUTF8([identity_ userEmail])),
fake_consent_auditor_->account_id());
}
// Tests that RecordGaiaConsent() is not called when the user taps on UNDO.
......@@ -278,6 +292,10 @@ TEST_F(ChromeSigninViewControllerTest, TestConsentWithSettings) {
fake_consent_auditor_->status());
EXPECT_EQ(consent_auditor::Feature::CHROME_SYNC,
fake_consent_auditor_->feature());
EXPECT_EQ(account_tracker_service_->PickAccountIdForAccount(
base::SysNSStringToUTF8([identity_ gaiaID]),
base::SysNSStringToUTF8([identity_ userEmail])),
fake_consent_auditor_->account_id());
}
} // namespace
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