Skip to content

Commit

Permalink
Address Web Discovery feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
DJAndries committed Oct 25, 2024
1 parent ba4c64a commit a7d9315
Show file tree
Hide file tree
Showing 23 changed files with 312 additions and 338 deletions.
3 changes: 3 additions & 0 deletions browser/ui/webui/welcome_page/welcome_dom_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "chrome/common/webui_url_constants.h"
#include "chrome/grit/branded_strings.h"
#include "components/prefs/pref_service.h"
#include "extensions/buildflags/buildflags.h"
#include "ui/base/l10n/l10n_util.h"

namespace {
Expand Down Expand Up @@ -170,7 +171,9 @@ void WelcomeDOMHandler::HandleSetMetricsReportingEnabled(
void WelcomeDOMHandler::HandleEnableWebDiscovery(
const base::Value::List& args) {
DCHECK(profile_);
#if BUILDFLAG(ENABLE_EXTENSIONS)
profile_->GetPrefs()->SetBoolean(kWebDiscoveryExtensionEnabled, true);
#endif
}

void WelcomeDOMHandler::SetLocalStateBooleanEnabled(
Expand Down
9 changes: 1 addition & 8 deletions browser/web_discovery/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ if (enable_web_discovery_native) {
"//brave/components/web_discovery/browser",
"//brave/components/web_discovery/common",
"//chrome/browser:browser_process",
"//chrome/browser/profiles:profile",
"//chrome/common:constants",
"//components/keyed_service/content",
"//components/user_prefs",
Expand Down Expand Up @@ -44,14 +45,6 @@ source_set("unit_tests") {
"//content/test:test_support",
"//testing/gtest",
]

if (enable_web_discovery_native) {
sources += [ "web_discovery_service_factory_unittest.cc" ]
deps += [
":web_discovery",
"//brave/components/web_discovery/common",
]
}
}
}

Expand Down
27 changes: 14 additions & 13 deletions browser/web_discovery/web_discovery_service_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,25 @@
#include "brave/components/web_discovery/browser/web_discovery_service.h"
#include "brave/components/web_discovery/common/features.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/profiles/profile_selections.h"
#include "chrome/common/chrome_paths.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h"
#include "components/user_prefs/user_prefs.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/storage_partition.h"

namespace web_discovery {

namespace {

ProfileSelections GetProfileSelections() {
if (!base::FeatureList::IsEnabled(features::kBraveWebDiscoveryNative)) {
return ProfileSelections::BuildNoProfilesSelected();
}
return ProfileSelections::BuildForRegularProfile();
}

} // namespace

WebDiscoveryService* WebDiscoveryServiceFactory::GetForBrowserContext(
content::BrowserContext* context) {
return static_cast<WebDiscoveryService*>(
Expand All @@ -29,9 +40,8 @@ WebDiscoveryServiceFactory* WebDiscoveryServiceFactory::GetInstance() {
}

WebDiscoveryServiceFactory::WebDiscoveryServiceFactory()
: BrowserContextKeyedServiceFactory(
"WebDiscoveryService",
BrowserContextDependencyManager::GetInstance()) {}
: ProfileKeyedServiceFactory("WebDiscoveryService",
GetProfileSelections()) {}

WebDiscoveryServiceFactory::~WebDiscoveryServiceFactory() = default;

Expand All @@ -47,15 +57,6 @@ KeyedService* WebDiscoveryServiceFactory::BuildServiceInstanceFor(
user_data_dir, shared_url_loader_factory);
}

content::BrowserContext* WebDiscoveryServiceFactory::GetBrowserContextToUse(
content::BrowserContext* context) const {
if (!base::FeatureList::IsEnabled(features::kBraveWebDiscoveryNative)) {
return nullptr;
}
// Prevents creation of service instance for incognito/OTR profiles
return context->IsOffTheRecord() ? nullptr : context;
}

bool WebDiscoveryServiceFactory::ServiceIsCreatedWithBrowserContext() const {
return true;
}
Expand Down
6 changes: 2 additions & 4 deletions browser/web_discovery/web_discovery_service_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@
#define BRAVE_BROWSER_WEB_DISCOVERY_WEB_DISCOVERY_SERVICE_FACTORY_H_

#include "base/no_destructor.h"
#include "components/keyed_service/content/browser_context_keyed_service_factory.h"
#include "chrome/browser/profiles/profile_keyed_service_factory.h"

namespace web_discovery {

class WebDiscoveryService;

class WebDiscoveryServiceFactory : public BrowserContextKeyedServiceFactory {
class WebDiscoveryServiceFactory : public ProfileKeyedServiceFactory {
public:
static WebDiscoveryServiceFactory* GetInstance();
static WebDiscoveryService* GetForBrowserContext(
Expand All @@ -31,8 +31,6 @@ class WebDiscoveryServiceFactory : public BrowserContextKeyedServiceFactory {

KeyedService* BuildServiceInstanceFor(
content::BrowserContext* context) const override;
content::BrowserContext* GetBrowserContextToUse(
content::BrowserContext* context) const override;
bool ServiceIsCreatedWithBrowserContext() const override;
};

Expand Down
33 changes: 0 additions & 33 deletions browser/web_discovery/web_discovery_service_factory_unittest.cc

This file was deleted.

5 changes: 4 additions & 1 deletion components/constants/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ source_set("constants") {
]

public_deps = [ ":brave_services_key" ]
deps = [ "//base" ]
deps = [
"//base",
"//extensions/buildflags",
]
}

source_set("brave_service_key_helper") {
Expand Down
1 change: 1 addition & 0 deletions components/constants/DEPS
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
include_rules = [
"+extensions/buildflags",
"+extensions/common",
]
3 changes: 3 additions & 0 deletions components/constants/pref_names.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#define BRAVE_COMPONENTS_CONSTANTS_PREF_NAMES_H_

#include "build/build_config.h"
#include "extensions/buildflags/buildflags.h"

inline constexpr char kBraveAutofillPrivateWindows[] =
"brave.autofill_private_windows";
Expand Down Expand Up @@ -85,8 +86,10 @@ inline constexpr char kBraveShieldsSettingsVersion[] =
inline constexpr char kDefaultBrowserPromptEnabled[] =
"brave.default_browser_prompt_enabled";

#if BUILDFLAG(ENABLE_EXTENSIONS)
inline constexpr char kWebDiscoveryExtensionEnabled[] =
"brave.web_discovery_enabled";
#endif
inline constexpr char kWebDiscoveryCTAState[] = "brave.web_discovery.cta_state";
inline constexpr char kDontAskEnableWebDiscovery[] =
"brave.dont_ask_enable_web_discovery";
Expand Down
5 changes: 4 additions & 1 deletion components/web_discovery/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@ import("//brave/components/web_discovery/buildflags/buildflags.gni")

assert(enable_web_discovery_native)

static_library("browser") {
component("browser") {
output_name = "web_discovery_browser"
sources = [
"background_credential_helper.cc",
"background_credential_helper.h",
"credential_manager.cc",
"credential_manager.h",
"credential_signer.h",
Expand Down
128 changes: 128 additions & 0 deletions components/web_discovery/browser/background_credential_helper.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
/* Copyright (c) 2024 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/components/web_discovery/browser/background_credential_helper.h"

#include <utility>

#include "base/base64.h"
#include "base/containers/span_rust.h"
#include "base/logging.h"
#include "base/threading/thread_restrictions.h"
#include "crypto/sha2.h"

namespace web_discovery {

BackgroundCredentialHelper::BackgroundCredentialHelper()
: anonymous_credential_manager_(
anonymous_credentials::new_credential_manager()) {}

BackgroundCredentialHelper::~BackgroundCredentialHelper() = default;

void BackgroundCredentialHelper::UseFixedSeedForTesting() {
anonymous_credential_manager_ =
anonymous_credentials::new_credential_manager_with_fixed_seed();
}

std::unique_ptr<RSAKeyInfo> BackgroundCredentialHelper::GenerateRSAKey() {
auto key_pair = GenerateRSAKeyPair();
if (!key_pair) {
return nullptr;
}
rsa_private_key_ = std::move(key_pair->key_pair);
return key_pair;
}

void BackgroundCredentialHelper::SetRSAKey(
std::unique_ptr<crypto::RSAPrivateKey> rsa_private_key) {
rsa_private_key_ = std::move(rsa_private_key);
}

std::optional<GenerateJoinRequestResult>
BackgroundCredentialHelper::GenerateJoinRequest(std::string pre_challenge) {
base::AssertLongCPUWorkAllowed();
CHECK(rsa_private_key_);
auto challenge = crypto::SHA256Hash(base::as_byte_span(pre_challenge));

auto join_request = anonymous_credential_manager_->start_join(
base::SpanToRustSlice(challenge));

auto signature = RSASign(rsa_private_key_.get(), join_request.join_request);

if (!signature) {
VLOG(1) << "RSA signature failed";
return std::nullopt;
}

return GenerateJoinRequestResult{.start_join_result = join_request,
.signature = *signature};
}

std::optional<std::string> BackgroundCredentialHelper::FinishJoin(
std::string date,
std::vector<const uint8_t> group_pub_key,
std::vector<const uint8_t> gsk,
std::vector<const uint8_t> join_resp_bytes) {
base::AssertLongCPUWorkAllowed();
auto pub_key_result = anonymous_credentials::load_group_public_key(
base::SpanToRustSlice(group_pub_key));
auto gsk_result =
anonymous_credentials::load_credential_big(base::SpanToRustSlice(gsk));
auto join_resp_result = anonymous_credentials::load_join_response(
base::SpanToRustSlice(join_resp_bytes));
if (!pub_key_result.error_message.empty() ||
!gsk_result.error_message.empty() ||
!join_resp_result.error_message.empty()) {
VLOG(1) << "Failed to finish credential join due to deserialization error "
"with group pub key, gsk, or join response: "
<< pub_key_result.error_message.c_str()
<< gsk_result.error_message.c_str()
<< join_resp_result.error_message.c_str();
return std::nullopt;
}
auto finish_res = anonymous_credential_manager_->finish_join(
*pub_key_result.value, *gsk_result.value,
std::move(join_resp_result.value));
if (!finish_res.error_message.empty()) {
VLOG(1) << "Failed to finish credential join for " << date << ": "
<< finish_res.error_message.c_str();
return std::nullopt;
}
return base::Base64Encode(finish_res.data);
}

std::optional<std::vector<const uint8_t>>
BackgroundCredentialHelper::PerformSign(
std::vector<const uint8_t> msg,
std::vector<const uint8_t> basename,
std::optional<std::vector<uint8_t>> gsk_bytes,
std::optional<std::vector<uint8_t>> credential_bytes) {
base::AssertLongCPUWorkAllowed();
if (gsk_bytes && credential_bytes) {
auto gsk_result = anonymous_credentials::load_credential_big(
base::SpanToRustSlice(*gsk_bytes));
auto credential_result = anonymous_credentials::load_user_credentials(
base::SpanToRustSlice(*credential_bytes));
if (!gsk_result.error_message.empty() ||
!credential_result.error_message.empty()) {
VLOG(1) << "Failed to sign due to deserialization error with gsk, or "
"user credential: "
<< gsk_result.error_message.c_str()
<< credential_result.error_message.c_str();
return std::nullopt;
}
anonymous_credential_manager_->set_gsk_and_credentials(
std::move(gsk_result.value), std::move(credential_result.value));
}
auto sig_res = anonymous_credential_manager_->sign(
base::SpanToRustSlice(msg), base::SpanToRustSlice(basename));
if (!sig_res.error_message.empty()) {
VLOG(1) << "Failed to sign: " << sig_res.error_message.c_str();
return std::nullopt;
}
return std::vector<const uint8_t>(sig_res.data.begin(), sig_res.data.end());
}

} // namespace web_discovery
59 changes: 59 additions & 0 deletions components/web_discovery/browser/background_credential_helper.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/* Copyright (c) 2024 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/. */

#ifndef BRAVE_COMPONENTS_WEB_DISCOVERY_BROWSER_BACKGROUND_CREDENTIAL_HELPER_H_
#define BRAVE_COMPONENTS_WEB_DISCOVERY_BROWSER_BACKGROUND_CREDENTIAL_HELPER_H_

#include <memory>
#include <optional>
#include <string>
#include <vector>

#include "brave/components/web_discovery/browser/anonymous_credentials/rs/cxx/src/lib.rs.h"
#include "brave/components/web_discovery/browser/rsa.h"
#include "crypto/rsa_private_key.h"

namespace web_discovery {

struct GenerateJoinRequestResult {
anonymous_credentials::StartJoinResult start_join_result;
std::string signature;
};

class BackgroundCredentialHelper {
public:
BackgroundCredentialHelper();
~BackgroundCredentialHelper();

BackgroundCredentialHelper(const BackgroundCredentialHelper&) = delete;
BackgroundCredentialHelper& operator=(const BackgroundCredentialHelper&) =
delete;

void UseFixedSeedForTesting();

std::unique_ptr<RSAKeyInfo> GenerateRSAKey();
void SetRSAKey(std::unique_ptr<crypto::RSAPrivateKey> rsa_private_key);
std::optional<GenerateJoinRequestResult> GenerateJoinRequest(
std::string pre_challenge);
std::optional<std::string> FinishJoin(
std::string date,
std::vector<const uint8_t> group_pub_key,
std::vector<const uint8_t> gsk,
std::vector<const uint8_t> join_resp_bytes);
std::optional<std::vector<const uint8_t>> PerformSign(
std::vector<const uint8_t> msg,
std::vector<const uint8_t> basename,
std::optional<std::vector<uint8_t>> gsk_bytes,
std::optional<std::vector<uint8_t>> credential_bytes);

private:
rust::Box<anonymous_credentials::CredentialManager>
anonymous_credential_manager_;
std::unique_ptr<crypto::RSAPrivateKey> rsa_private_key_;
};

} // namespace web_discovery

#endif // BRAVE_COMPONENTS_WEB_DISCOVERY_BROWSER_BACKGROUND_CREDENTIAL_HELPER_H_
Loading

0 comments on commit a7d9315

Please sign in to comment.