Send platform, build channel and country code as part of ad confirmation calls where it meets privacy standards #5067

tmancey · 2020-03-26T11:11:19Z

Submitter Checklist:

Test Plan:

Use Charles Proxy to capture traffic and confirm the request to the POST /v1/confirmation/{confirmation_id}/{credential} server end-point contains the following in the body:

countryCode i.e. US if the country has large anonymity, see brave/brave-browser#8100 for list of countries (only added for release builds)

platform i.e. macos, windows, linux, android or ios

channelName i.e. release, beta, nightly or dev

Reviewer Checklist:

New files have MPL-2.0 license header.
Request a security/privacy review as needed.
Adequate test coverage exists to prevent regressions
Verify test plan is specified in PR before merging to source

After-merge Checklist:

The associated issue milestone is set to the smallest version that the
changes has landed on.
All relevant documentation has been updated.

kylehickinson

iOS lgtm

diracdeltas · 2020-03-26T18:19:12Z

vendor/bat-native-confirmations/src/bat/confirmations/internal/static_values.h

@@ -35,6 +37,80 @@ const uint64_t kDebugNextTokenRedemptionAfterSeconds =
 const uint64_t kRetryFailedConfirmationsAfterSeconds =
    5 * base::Time::kSecondsPerMinute;

+const std::map<std::string, bool> kLargeAnonymityCountryCodes = {


what is the size bound on these and when was it generated? just checking

diracdeltas · 2020-03-26T18:21:51Z

it seems that the only new info this adds is the build channel (though isn't that already inferable from the user agent?). we might want to make the country bound larger since when i originally did the analysis, i did not factor in build channel.

tmancey · 2020-03-31T15:24:12Z

it seems that the only new info this adds is the build channel (though isn't that already inferable from the user agent?). we might want to make the country bound larger since when i originally did the analysis, i did not factor in build channel.

I have checked release, beta and nightly builds and unfortunately, the build channel is not available in the user-agent, thanks

bbondy · 2020-04-14T14:37:58Z

common/brave_channel_info.cc

+
+  return channel_name;
+#else  // OFFICIAL_BUILD
+  return "development";


cc @aekeus @bridiver it seems like before we were returning release for both non official and official builds. It might be good to add a selector for development for stats.

The 'developer' channel is available on stats in the database. We would need to add the 'developer' identifier to a few drop down menus. This would not be a large change to make.

I would suggest 'developer' instead of 'development' as that exists on stats already.

channel | description | label -----------+---------------------+------------------ ... developer | Developer channel | Developer ...

Changed development developer, thanks

bbondy

lgtm other than @iefremov 's comments which you have follow up issues for.

tmancey · 2020-04-15T15:04:28Z

macOS and iOS failed CI, restarting

…ion calls where it meets privacy standards

tmancey · 2020-04-17T09:38:51Z

CI failed for macOS, restarting macOS

tmancey · 2020-04-17T12:10:39Z

Restarting macOS as failed CI

tmancey · 2020-04-17T13:45:40Z

Failed due to unrelated rewards browser tests

bridiver · 2020-11-18T16:48:45Z

components/brave_rewards/browser/android_util.cc


 namespace android_util {

 ledger::ClientInfoPtr GetAndroidClientInfo() {
  auto info = ledger::ClientInfo::New();
  info->platform = ledger::Platform::ANDROID_R;
  info->os = ledger::OperatingSystem::UNDEFINED;
+  info->channel = brave::GetChannelName();


when this is fixed to use chrome::GetChannelName the layering violation here needs to be fixed as well. You cannot use brave/common or chrome/common in components

cc @aseren FYI for the refactoring we spoke about

bridiver · 2020-11-18T16:49:21Z

components/brave_rewards/browser/rewards_service_impl.cc

@@ -3273,6 +3274,8 @@ ledger::ClientInfoPtr GetDesktopClientInfo() {
    info->os = ledger::OperatingSystem::UNDEFINED;
  #endif

+  info->channel = brave::GetChannelName();


chrome::GetChannelName should be passed to RewardsServiceImpl in the factory to eliminate this layering violation

@bridiver raised brave/brave-browser#12769 for Rewards and brave/brave-browser#12768 for Ads

tmancey added PR/Pending Review feature/ads labels Mar 26, 2020

tmancey added this to the 1.8.x - Nightly milestone Mar 26, 2020

tmancey requested review from bridiver, ryanml and NejcZdovc March 26, 2020 11:11

tmancey requested review from gdregalo and kylehickinson as code owners March 26, 2020 11:11

tmancey self-assigned this Mar 26, 2020

tmancey force-pushed the issues/8100 branch from 8e0cd59 to 564e0af Compare March 26, 2020 11:12

tmancey requested review from iefremov and emerick March 26, 2020 11:13

tmancey force-pushed the issues/8100 branch 4 times, most recently from 5a5ecbf to 22b4e4c Compare March 26, 2020 14:37

kylehickinson approved these changes Mar 26, 2020

View reviewed changes

tmancey requested review from diracdeltas and pes10k March 26, 2020 15:01

tmancey added the security label Mar 26, 2020

tmancey force-pushed the issues/8100 branch 2 times, most recently from 43327f5 to a2c3ad5 Compare March 26, 2020 16:36

diracdeltas reviewed Mar 26, 2020

View reviewed changes

tmancey force-pushed the issues/8100 branch 2 times, most recently from f68b9a3 to a90f3ea Compare March 31, 2020 12:09

tmancey force-pushed the issues/8100 branch from a90f3ea to 4cc47bd Compare April 1, 2020 22:58

tmancey modified the milestones: 1.8.x - Beta, 1.9.x - Nightly Apr 3, 2020

tmancey requested a review from aekeus April 14, 2020 14:35

bbondy reviewed Apr 14, 2020

View reviewed changes

bbondy approved these changes Apr 14, 2020

View reviewed changes

tmancey force-pushed the issues/8100 branch 6 times, most recently from 757ce20 to a7a49f6 Compare April 15, 2020 09:01

tmancey added CI/skip-android Do not run CI builds for Android CI/skip-linux labels Apr 15, 2020

tmancey force-pushed the issues/8100 branch 2 times, most recently from d9dc885 to 8d24826 Compare April 17, 2020 08:53

Send platform, build channel and country code as part of ad confirmat…

259a5ad

…ion calls where it meets privacy standards

tmancey force-pushed the issues/8100 branch from 8d24826 to 259a5ad Compare April 17, 2020 09:22

tmancey added the CI/skip-ios Do not run CI builds for iOS label Apr 17, 2020

tmancey merged commit 899270b into master Apr 17, 2020

tmancey deleted the issues/8100 branch April 17, 2020 13:45

NejcZdovc mentioned this pull request Apr 20, 2020

Brave Ads build layering violation brave/brave-browser#9332

Closed

tmancey mentioned this pull request May 1, 2020

Refactored supported regions, large anonymity country codes and other country codes to use std::set #5443

Merged

32 tasks

GeetaSarvadnya mentioned this pull request May 5, 2020

Send platform, build channel, and country as part of ad confirmation calls where it meets privacy standards brave/brave-browser#8100

Closed

kjozwiak mentioned this pull request Jun 12, 2020

Refactor supported regions, large anonymity country codes and other country codes to use std::set brave/brave-browser#9205

Closed

bridiver reviewed Nov 18, 2020

View reviewed changes

marshall mentioned this pull request Dec 7, 2020

Refactor BraveRewards to use chrome::GetChannelName instead of brave::GetChannelName to remove layering violation brave/brave-browser#12769

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Send platform, build channel and country code as part of ad confirmation calls where it meets privacy standards #5067

Send platform, build channel and country code as part of ad confirmation calls where it meets privacy standards #5067

tmancey commented Mar 26, 2020 •

edited

Loading

kylehickinson left a comment

diracdeltas Mar 26, 2020

diracdeltas commented Mar 26, 2020

tmancey commented Mar 31, 2020

bbondy Apr 14, 2020

aekeus Apr 14, 2020 •

edited

Loading

aekeus Apr 14, 2020 •

edited

Loading

tmancey Apr 14, 2020

bbondy left a comment

tmancey commented Apr 15, 2020

tmancey commented Apr 17, 2020

tmancey commented Apr 17, 2020

tmancey commented Apr 17, 2020

bridiver Nov 18, 2020

tmancey Apr 14, 2023

bridiver Nov 18, 2020

tmancey Nov 18, 2020 •

edited

Loading

Send platform, build channel and country code as part of ad confirmation calls where it meets privacy standards #5067

Send platform, build channel and country code as part of ad confirmation calls where it meets privacy standards #5067

Conversation

tmancey commented Mar 26, 2020 • edited Loading

Submitter Checklist:

Test Plan:

Reviewer Checklist:

After-merge Checklist:

kylehickinson left a comment

Choose a reason for hiding this comment

diracdeltas Mar 26, 2020

Choose a reason for hiding this comment

diracdeltas commented Mar 26, 2020

tmancey commented Mar 31, 2020

bbondy Apr 14, 2020

Choose a reason for hiding this comment

aekeus Apr 14, 2020 • edited Loading

Choose a reason for hiding this comment

aekeus Apr 14, 2020 • edited Loading

Choose a reason for hiding this comment

tmancey Apr 14, 2020

Choose a reason for hiding this comment

bbondy left a comment

Choose a reason for hiding this comment

tmancey commented Apr 15, 2020

tmancey commented Apr 17, 2020

tmancey commented Apr 17, 2020

tmancey commented Apr 17, 2020

bridiver Nov 18, 2020

Choose a reason for hiding this comment

tmancey Apr 14, 2023

Choose a reason for hiding this comment

bridiver Nov 18, 2020

Choose a reason for hiding this comment

tmancey Nov 18, 2020 • edited Loading

Choose a reason for hiding this comment

tmancey commented Mar 26, 2020 •

edited

Loading

aekeus Apr 14, 2020 •

edited

Loading

aekeus Apr 14, 2020 •

edited

Loading

tmancey Nov 18, 2020 •

edited

Loading