Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Global Privacy Control #6743

Merged
merged 3 commits into from
Oct 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions app/brave_main_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@
#include "brave/app/brave_command_line_helper.h"
#include "brave/browser/brave_content_browser_client.h"
#include "brave/common/brave_switches.h"
#include "brave/common/brave_features.h"
#include "brave/common/resource_bundle_helper.h"
#include "brave/components/brave_ads/browser/buildflags/buildflags.h"
#include "brave/renderer/brave_content_renderer_client.h"
#include "brave/utility/brave_content_utility_client.h"
#include "build/build_config.h"
#include "chrome/browser/ui/ui_features.h"
#include "chrome/common/channel_info.h"
#include "chrome/common/chrome_features.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/chrome_paths_internal.h"
Expand All @@ -39,6 +41,7 @@
#include "components/security_state/core/features.h"
#include "components/sync/base/sync_base_switches.h"
#include "components/translate/core/browser/translate_prefs.h"
#include "components/version_info/channel.h"
#include "content/public/common/content_features.h"
#include "content/public/common/content_switches.h"
#include "google_apis/gaia/gaia_switches.h"
Expand Down Expand Up @@ -225,6 +228,10 @@ bool BraveMainDelegate::BasicStartupComplete(int* exit_code) {
enabled_features.insert(features::kDnsOverHttps.name);
}

if (chrome::GetChannel() == version_info::Channel::CANARY) {
bbondy marked this conversation as resolved.
Show resolved Hide resolved
enabled_features.insert(features::kGlobalPrivacyControl.name);
}

// Disabled features.
const std::unordered_set<const char*> disabled_features = {
autofill::features::kAutofillEnableAccountWalletStorage.name,
Expand Down
2 changes: 2 additions & 0 deletions browser/net/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ source_set("net") {
"brave_stp_util.h",
"brave_system_request_handler.cc",
"brave_system_request_handler.h",
"global_privacy_control_network_delegate_helper.cc",
"global_privacy_control_network_delegate_helper.h",
"resource_context_data.cc",
"resource_context_data.h",
"url_context.cc",
Expand Down
5 changes: 5 additions & 0 deletions browser/net/brave_request_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "brave/browser/net/brave_httpse_network_delegate_helper.h"
#include "brave/browser/net/brave_site_hacks_network_delegate_helper.h"
#include "brave/browser/net/brave_stp_util.h"
#include "brave/browser/net/global_privacy_control_network_delegate_helper.h"
#include "brave/browser/translate/buildflags/buildflags.h"
#include "brave/common/pref_names.h"
#include "brave/components/brave_referrals/buildflags/buildflags.h"
Expand Down Expand Up @@ -100,6 +101,10 @@ void BraveRequestHandler::SetupCallbacks() {
base::Bind(brave::OnBeforeStartTransaction_SiteHacksWork);
before_start_transaction_callbacks_.push_back(start_transaction_callback);

start_transaction_callback =
base::Bind(brave::OnBeforeStartTransaction_GlobalPrivacyControlWork);
before_start_transaction_callbacks_.push_back(start_transaction_callback);

#if BUILDFLAG(ENABLE_BRAVE_REFERRALS)
start_transaction_callback =
base::Bind(brave::OnBeforeStartTransaction_ReferralsWork);
Expand Down
28 changes: 28 additions & 0 deletions browser/net/global_privacy_control_network_delegate_helper.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/* Copyright (c) 2020 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "brave/browser/net/global_privacy_control_network_delegate_helper.h"

#include <memory>

#include "base/feature_list.h"
#include "brave/common/brave_features.h"
#include "brave/common/network_constants.h"
#include "net/base/net_errors.h"
#include "net/http/http_request_headers.h"

namespace brave {

int OnBeforeStartTransaction_GlobalPrivacyControlWork(
net::HttpRequestHeaders* headers,
const ResponseCallback& next_callback,
std::shared_ptr<BraveRequestInfo> ctx) {
if (base::FeatureList::IsEnabled(features::kGlobalPrivacyControl)) {
headers->SetHeader(kSecGpcHeader, "1");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need it for all requests, or just for navigations/downloads?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Talked to @pes10k about this as it's not currently described anywhere - he will try to get that clarified in the spec but there shouldn't be any harm in doing it this way.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just heard back, its intended to be on every request

}
return net::OK;
}

} // namespace brave
22 changes: 22 additions & 0 deletions browser/net/global_privacy_control_network_delegate_helper.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/* Copyright (c) 2020 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#ifndef BRAVE_BROWSER_NET_GLOBAL_PRIVACY_CONTROL_NETWORK_DELEGATE_HELPER_H_
#define BRAVE_BROWSER_NET_GLOBAL_PRIVACY_CONTROL_NETWORK_DELEGATE_HELPER_H_

#include <memory>

#include "brave/browser/net/url_context.h"

namespace brave {

int OnBeforeStartTransaction_GlobalPrivacyControlWork(
net::HttpRequestHeaders* headers,
const ResponseCallback& next_callback,
std::shared_ptr<BraveRequestInfo> ctx);

} // namespace brave

#endif // BRAVE_BROWSER_NET_GLOBAL_PRIVACY_CONTROL_NETWORK_DELEGATE_HELPER_H_
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
/* Copyright (c) 2020 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at https://mozilla.org/MPL/2.0/. */

#include "brave/common/brave_features.h"
#include "brave/common/network_constants.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "chrome/test/base/ui_test_utils.h"
#include "components/network_session_configurator/common/network_switches.h"
#include "content/public/test/browser_test.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/test_utils.h"
#include "net/dns/mock_host_resolver.h"
#include "net/test/embedded_test_server/http_request.h"

enum class GPCHeaderResult {
kOk,
kNoRequest,
kNoHeader,
kWrongValue,
};

class GlobalPrivacyControlNetworkDelegateBrowserTest
: public InProcessBrowserTest {
public:
GlobalPrivacyControlNetworkDelegateBrowserTest()
: https_server_(net::EmbeddedTestServer::TYPE_HTTPS) {
feature_list_.InitAndEnableFeature(features::kGlobalPrivacyControl);
}

void SetUpOnMainThread() override {
InProcessBrowserTest::SetUpOnMainThread();

header_result_ = GPCHeaderResult::kNoRequest;

host_resolver()->AddRule("*", "127.0.0.1");

https_server_.RegisterRequestMonitor(base::BindRepeating(
&GlobalPrivacyControlNetworkDelegateBrowserTest::HandleRequest,
base::Unretained(this)));

ASSERT_TRUE(https_server_.Start());
}

void HandleRequest(const net::test_server::HttpRequest& request) {
base::AutoLock auto_lock(header_result_lock_);

auto it = request.headers.find(kSecGpcHeader);
if (it == request.headers.end()) {
header_result_ = GPCHeaderResult::kNoHeader;
} else if (it ->second != "1") {
header_result_ = GPCHeaderResult::kWrongValue;
} else {
header_result_ = GPCHeaderResult::kOk;
}
}

void SetUpCommandLine(base::CommandLine* command_line) override {
InProcessBrowserTest::SetUpCommandLine(command_line);
// Allows the embedded test server to serve HTTPS
command_line->AppendSwitch(switches::kIgnoreCertificateErrors);
}

const net::EmbeddedTestServer& https_server() { return https_server_; }

GPCHeaderResult header_result() {
base::AutoLock auto_lock(header_result_lock_);
return header_result_;
}

protected:
base::test::ScopedFeatureList feature_list_;

private:
net::test_server::EmbeddedTestServer https_server_;
mutable base::Lock header_result_lock_;
GPCHeaderResult header_result_;
};

// When kGlobalPrivacyControl is enabled, the Sec-GPC flag should appear on
// request headers.
IN_PROC_BROWSER_TEST_F(GlobalPrivacyControlNetworkDelegateBrowserTest,
IncludesSecGPCHeader) {
const GURL target = https_server().GetURL("example.com", "/index.html");
ui_test_utils::NavigateToURL(browser(), target);
EXPECT_EQ(header_result(), GPCHeaderResult::kOk);
}

// The Global Privacy Control spec also defines the
// `navigator.globalPrivacyControl` JS property, which is read-only. In Brave
// it will always return `true`.
IN_PROC_BROWSER_TEST_F(GlobalPrivacyControlNetworkDelegateBrowserTest,
NavigatorGlobalPrivacyAPI) {
const GURL target = https_server().GetURL("example.com", "/index.html");
ui_test_utils::NavigateToURL(browser(), target);

auto* rfh =
browser()->tab_strip_model()->GetActiveWebContents()->GetMainFrame();

EXPECT_EQ(true, content::EvalJs(rfh, "navigator.globalPrivacyControl"));
EXPECT_EQ(true, content::EvalJs(rfh,
"(function() {"
" navigator.globalPrivacyControl = false;"
" return navigator.globalPrivacyControl;"
"})()"));
}

class DisabledGlobalPrivacyControlNetworkDelegateBrowserTest
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mb just disable feature in the test itself?

: public GlobalPrivacyControlNetworkDelegateBrowserTest {
public:
DisabledGlobalPrivacyControlNetworkDelegateBrowserTest() {
feature_list_.Reset();
feature_list_.InitAndDisableFeature(features::kGlobalPrivacyControl);
}
};

// When kGlobalPrivacyControl is disabled, the Sec-GPC flag should not appear
// on request headers.
IN_PROC_BROWSER_TEST_F(DisabledGlobalPrivacyControlNetworkDelegateBrowserTest,
ExcludesSecGPCHeader) {
const GURL target = https_server().GetURL("example.com", "/index.html");
ui_test_utils::NavigateToURL(browser(), target);
EXPECT_EQ(header_result(), GPCHeaderResult::kNoHeader);
}
2 changes: 2 additions & 0 deletions chromium_src/chrome/browser/flag_descriptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ extern const char kBraveNTPBrandedWallpaperDemoName[];
extern const char kBraveNTPBrandedWallpaperDemoDescription[];
extern const char kBraveAdblockCosmeticFilteringName[];
extern const char kBraveAdblockCosmeticFilteringDescription[];
extern const char kGlobalPrivacyControlName[];
extern const char kGlobalPrivacyControlDescription[];
extern const char kBraveSpeedreaderName[];
extern const char kBraveSpeedreaderDescription[];
extern const char kBraveSyncName[];
Expand Down
6 changes: 6 additions & 0 deletions common/brave_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,10 @@ const base::Feature kBraveRewards{"BraveRewards",
#endif
#endif // defined(OS_ANDROID)

// Toggles Global Privacy Control, which conveys a user's request to websites
// and services to not sell or share their personal information with third
// parties.
const base::Feature kGlobalPrivacyControl{"GlobalPrivacyControl",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add a comment, link to the spec?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spec is currently not public, so there isn't a stable URL for it at the moment unfortunately

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps just a comment or link to a wiki on brave-browser

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a comment from the spec

base::FEATURE_DISABLED_BY_DEFAULT};

} // namespace features
2 changes: 2 additions & 0 deletions common/brave_features.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ COMPONENT_EXPORT(CHROME_FEATURES)
extern const base::Feature kBraveRewards;
#endif // defined(OS_ANDROID)

extern const base::Feature kGlobalPrivacyControl;

} // namespace features

#endif // BRAVE_COMMON_BRAVE_FEATURES_H_
2 changes: 2 additions & 0 deletions common/network_constants.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,5 @@ const char kBraveServicesKeyHeader[] = "BraveServiceKey";

const char kBittorrentMimeType[] = "application/x-bittorrent";
const char kOctetStreamMimeType[] = "application/octet-stream";

const char kSecGpcHeader[] = "Sec-GPC";
2 changes: 2 additions & 0 deletions common/network_constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,6 @@ extern const char kBraveServicesKeyHeader[];

extern const char kBittorrentMimeType[];
extern const char kOctetStreamMimeType[];

extern const char kSecGpcHeader[];
#endif // BRAVE_COMMON_NETWORK_CONSTANTS_H_
1 change: 1 addition & 0 deletions test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,7 @@ test("brave_browser_tests") {
"//brave/browser/net/brave_network_delegate_hsts_fingerprinting_browsertest.cc",
"//brave/browser/net/brave_site_hacks_network_delegate_helper_browsertest.cc",
"//brave/browser/net/brave_system_request_handler_browsertest.cc",
"//brave/browser/net/global_privacy_control_network_delegate_helper_browsertest.cc",
"//brave/browser/policy/brave_policy_browsertest.cc",
"//brave/browser/profiles/brave_bookmark_model_loaded_observer_browsertest.cc",
"//brave/browser/profiles/brave_profile_manager_browsertest.cc",
Expand Down
8 changes: 6 additions & 2 deletions third_party/blink/renderer/includes.gni
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,13 @@ brave_blink_renderer_modules_visibility = [
]

brave_blink_sub_modules = [
"//brave/third_party/blink/renderer/modules/brave"
"//brave/third_party/blink/renderer/modules/brave",
"//brave/third_party/blink/renderer/modules/global_privacy_control"
]

# common includes which can help minimize patches for
# src/third_party/blink/renderer/modules/modules_idl_files.gni
brave_idl_imports = [ "//brave/third_party/blink/renderer/modules/brave/idls.gni" ]
brave_idl_imports = [
"//brave/third_party/blink/renderer/modules/brave/idls.gni",
"//brave/third_party/blink/renderer/modules/global_privacy_control/idls.gni"
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Copyright (c) 2020 The Brave Authors. All rights reserved.
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this file,
# You can obtain one at http://mozilla.org/MPL/2.0/.

import("//third_party/blink/renderer/modules/modules.gni")

blink_modules_sources("global_privacy_control") {
sources = [
"navigator_global_privacy_control.cc",
"navigator_global_privacy_control.h",
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# Copyright (c) 2020 The Brave Authors. All rights reserved.
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this file,
# You can obtain one at http://mozilla.org/MPL/2.0/.

modules_dependency_idl_files = [ "navigator_global_privacy_control.idl" ]
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/* Copyright (c) 2020 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "brave/third_party/blink/renderer/modules/global_privacy_control/navigator_global_privacy_control.h"

#include "third_party/blink/renderer/core/frame/navigator.h"

namespace blink {

NavigatorGlobalPrivacyControl::
NavigatorGlobalPrivacyControl(Navigator& navigator) // NOLINT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just curious - why this is NOLINT

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lint asks for const Navigator& but then it doesn't match the IDL generated header.

: Supplement<Navigator>(navigator) {}

// static
const char NavigatorGlobalPrivacyControl::kSupplementName[] =
"NavigatorGlobalPrivacyControl";

NavigatorGlobalPrivacyControl&
NavigatorGlobalPrivacyControl::From(Navigator& navigator) {
NavigatorGlobalPrivacyControl* supplement =
Supplement<Navigator>::From<NavigatorGlobalPrivacyControl>(navigator);
if (!supplement) {
supplement = MakeGarbageCollected<NavigatorGlobalPrivacyControl>(navigator);
ProvideTo(navigator, supplement);
}
return *supplement;
}

bool NavigatorGlobalPrivacyControl::
globalPrivacyControl(blink::Navigator& navigator) { // NOLINT
return true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we want to check base::FeatureList::IsEnabled here

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bbondy I discussed this with @iefremov and @bridiver in DMs - there isn't a way to gate the presence of the IDL-generated API by a feature flag. We've decided it's fine since this is purely a new API and shouldn't break webcompat anywhere. And if it has to exist, better to just always return true rather than false.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

}

void NavigatorGlobalPrivacyControl::Trace(blink::Visitor* visitor) const {
Supplement<Navigator>::Trace(visitor);
}

} // namespace blink
Loading