Skip to content

Commit

Permalink
[DBSC] Session registration in BoundSessionCookieRefreshService
Browse files Browse the repository at this point in the history
This CL adds a new RegisterNewBoundSession() method to
BoundSessionCookieRefreshService which allows registering a new bound
session after fininsing the explicit registration flow.

For now, BoundSessionCookieRefreshServiceImpl supports two modes:
1. Bound session state is tied to the Chrome sign-in state.
2. Bound session can be started explicitly and is independent from the
Chrome sign-in state.

Mode 2 is put behind a feature flag and should replace mode 1 once the
registration flow is implemented.

Session parameters are stored on disk to remember the bound session
state across browser restarts. The parameters are stored in .proto
format in profile prefs.

Proto format has the following advantages:
+ Supports structured and typed messages.
+ No need to write custom serialization code to store a message on disk.
+ New optional fields can be added without breaking backwards
  compatibility.

Bug: b/286222327
Change-Id: I74140d9b650e5a0b3896256a9fbcb9af4b664a9b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4604501
Reviewed-by: Kristian Monsen <kristianm@chromium.org>
Reviewed-by: Monica Basta <msalama@chromium.org>
Commit-Queue: Alex Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1158729}
  • Loading branch information
Alex Ilin authored and Chromium LUCI CQ committed Jun 16, 2023
1 parent fb75aef commit 1c12e91
Show file tree
Hide file tree
Showing 9 changed files with 260 additions and 33 deletions.
8 changes: 8 additions & 0 deletions chrome/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -6868,6 +6868,8 @@ static_library("browser") {
"signin/bound_session_credentials/unexportable_key_service_factory.h",
]

public_deps += [ ":bound_session_credentials_proto" ]

deps += [
"//chrome/common",
"//components/unexportable_keys",
Expand Down Expand Up @@ -7840,6 +7842,12 @@ proto_library("permissions_proto") {
sources = [ "permissions/crowd_deny.proto" ]
}

if (enable_bound_session_credentials) {
proto_library("bound_session_credentials_proto") {
sources = [ "signin/bound_session_credentials/bound_session_registration_params.proto" ]
}
}

grit("resources") {
source = "browser_resources.grd"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "base/functional/callback_forward.h"
#include "base/memory/weak_ptr.h"
#include "chrome/browser/signin/bound_session_credentials/bound_session_registration_params.pb.h"
#include "chrome/common/renderer_configuration.mojom.h"
#include "components/keyed_service/core/keyed_service.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
Expand All @@ -32,6 +33,11 @@ class BoundSessionCookieRefreshService

virtual void Initialize() = 0;

// Registers a new bound session and starts tracking it immediately. The
// session persists across browser startups.
virtual void RegisterNewBoundSession(
const bound_session_credentials::RegistrationParams& params) = 0;

// Returns true if session is bound.
virtual bool IsBoundSession() const = 0;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_keyed_service_factory.h"
#include "chrome/browser/signin/account_consistency_mode_manager_factory.h"
#include "chrome/browser/signin/bound_session_credentials/bound_session_cookie_controller_impl.h"
#include "chrome/browser/signin/bound_session_credentials/bound_session_cookie_refresh_service.h"
#include "chrome/browser/signin/bound_session_credentials/bound_session_cookie_refresh_service_impl.h"
#include "chrome/browser/signin/chrome_signin_client_factory.h"
#include "chrome/browser/signin/identity_manager_factory.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "components/signin/public/base/signin_switches.h"

// static
Expand Down Expand Up @@ -69,3 +71,8 @@ BoundSessionCookieRefreshServiceFactory::BuildServiceInstanceForBrowserContext(
bound_session_cookie_refresh_service->Initialize();
return bound_session_cookie_refresh_service;
}

void BoundSessionCookieRefreshServiceFactory::RegisterProfilePrefs(
user_prefs::PrefRegistrySyncable* registry) {
BoundSessionCookieRefreshServiceImpl::RegisterProfilePrefs(registry);
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@

class BoundSessionCookieRefreshService;

namespace user_prefs {
class PrefRegistrySyncable;
}

class BoundSessionCookieRefreshServiceFactory
: public ProfileKeyedServiceFactory {
public:
Expand All @@ -27,6 +31,8 @@ class BoundSessionCookieRefreshServiceFactory
// ProfileKeyedServiceFactory:
std::unique_ptr<KeyedService> BuildServiceInstanceForBrowserContext(
content::BrowserContext* context) const override;
void RegisterProfilePrefs(
user_prefs::PrefRegistrySyncable* registry) override;
};

#endif // CHROME_BROWSER_SIGNIN_BOUND_SESSION_CREDENTIALS_BOUND_SESSION_COOKIE_REFRESH_SERVICE_FACTORY_H_
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,15 @@

#include "base/check.h"
#include "base/check_op.h"
#include "base/feature_list.h"
#include "base/functional/bind.h"
#include "base/memory/weak_ptr.h"
#include "base/notreached.h"
#include "chrome/browser/signin/bound_session_credentials/bound_session_cookie_controller.h"
#include "chrome/browser/signin/bound_session_credentials/bound_session_cookie_controller_impl.h"
#include "chrome/common/renderer_configuration.mojom.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "components/prefs/pref_service.h"
#include "components/signin/public/base/consent_level.h"
#include "components/signin/public/base/signin_client.h"
#include "components/signin/public/identity_manager/accounts_in_cookie_jar_info.h"
Expand All @@ -25,6 +28,15 @@ using signin::ConsentLevel;
using signin::IdentityManager;
using signin::PrimaryAccountChangeEvent;

namespace {
const char kRegistrationParamsPref[] =
"bound_session_credentials_registration_params";
}

BASE_FEATURE(kBoundSessionExplicitRegistration,
"BoundSessionExplicitRegistration",
base::FEATURE_DISABLED_BY_DEFAULT);

class BoundSessionCookieRefreshServiceImpl::BoundSessionStateTracker
: public IdentityManager::Observer {
public:
Expand Down Expand Up @@ -172,20 +184,48 @@ BoundSessionCookieRefreshServiceImpl::BoundSessionCookieRefreshServiceImpl(
BoundSessionCookieRefreshServiceImpl::~BoundSessionCookieRefreshServiceImpl() =
default;

// static
void BoundSessionCookieRefreshServiceImpl::RegisterProfilePrefs(
user_prefs::PrefRegistrySyncable* registry) {
registry->RegisterStringPref(kRegistrationParamsPref, std::string());
}

void BoundSessionCookieRefreshServiceImpl::Initialize() {
// `base::Unretained(this)` is safe because `this` owns
// `bound_session_tracker_`.
bound_session_tracker_ = std::make_unique<BoundSessionStateTracker>(
identity_manager_,
base::BindRepeating(
&BoundSessionCookieRefreshServiceImpl::OnBoundSessionUpdated,
base::Unretained(this)));
if (!base::FeatureList::IsEnabled(kBoundSessionExplicitRegistration)) {
// `base::Unretained(this)` is safe because `this` owns
// `bound_session_tracker_`.
bound_session_tracker_ = std::make_unique<BoundSessionStateTracker>(
identity_manager_,
base::BindRepeating(
&BoundSessionCookieRefreshServiceImpl::OnBoundSessionUpdated,
base::Unretained(this)));
}
OnBoundSessionUpdated();
}

void BoundSessionCookieRefreshServiceImpl::RegisterNewBoundSession(
const bound_session_credentials::RegistrationParams& params) {
CHECK(base::FeatureList::IsEnabled(kBoundSessionExplicitRegistration));
std::string serialized_params = params.SerializeAsString();
if (serialized_params.empty()) {
DVLOG(1) << "Failed to serialize bound session registration params.";
return;
}
// New session should override the existing one.
if (IsBoundSession()) {
StopManagingBoundSessionCookie();
}
client_->GetPrefs()->SetString(kRegistrationParamsPref, serialized_params);
OnBoundSessionUpdated();
}

bool BoundSessionCookieRefreshServiceImpl::IsBoundSession() const {
DCHECK(bound_session_tracker_);
return bound_session_tracker_->is_bound_session();
if (base::FeatureList::IsEnabled(kBoundSessionExplicitRegistration)) {
return client_->GetPrefs()->HasPrefPath(kRegistrationParamsPref);
} else {
CHECK(bound_session_tracker_);
return bound_session_tracker_->is_bound_session();
}
}

chrome::mojom::BoundSessionParamsPtr
Expand Down Expand Up @@ -219,7 +259,7 @@ void BoundSessionCookieRefreshServiceImpl::OnRequestBlockedOnCookie(
std::move(resume_blocked_request).Run();
return;
}
DCHECK(cookie_controller_);
CHECK(cookie_controller_);
cookie_controller_->OnRequestBlockedOnCookie(
std::move(resume_blocked_request));
}
Expand All @@ -244,9 +284,10 @@ BoundSessionCookieRefreshServiceImpl::CreateBoundSessionCookieController(
}

void BoundSessionCookieRefreshServiceImpl::StartManagingBoundSessionCookie() {
DCHECK(!cookie_controller_);
CHECK(!cookie_controller_);
constexpr char kSIDTSCookieName[] = "__Secure-1PSIDTS";

// TODO(http://b/286222327): pass registration params to controller.
cookie_controller_ = CreateBoundSessionCookieController(
GaiaUrls::GetInstance()->secure_google_url(), kSIDTSCookieName);
cookie_controller_->Initialize();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,26 @@

#include <memory>

#include "base/feature_list.h"
#include "base/functional/callback_forward.h"
#include "base/memory/weak_ptr.h"
#include "chrome/browser/signin/bound_session_credentials/bound_session_cookie_controller.h"
#include "chrome/browser/signin/bound_session_credentials/bound_session_registration_params.pb.h"
#include "components/keyed_service/core/keyed_service.h"
#include "components/signin/public/identity_manager/identity_manager.h"
#include "mojo/public/cpp/bindings/receiver_set.h"

class SigninClient;

namespace user_prefs {
class PrefRegistrySyncable;
}

// If the feature is on, `BoundSessionCookieRefreshServiceImpl` uses only
// explicitly registered sessions instead of relying on the primary account
// state.
BASE_DECLARE_FEATURE(kBoundSessionExplicitRegistration);

class BoundSessionCookieRefreshServiceImpl
: public BoundSessionCookieRefreshService,
public BoundSessionCookieController::Delegate {
Expand All @@ -28,8 +39,13 @@ class BoundSessionCookieRefreshServiceImpl

~BoundSessionCookieRefreshServiceImpl() override;

static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);

// BoundSessionCookieRefreshService:
void Initialize() override;
// Can be called iff the kBoundSessionExplicitRegistration feature is enabled.
void RegisterNewBoundSession(
const bound_session_credentials::RegistrationParams& params) override;
bool IsBoundSession() const override;
chrome::mojom::BoundSessionParamsPtr GetBoundSessionParams() const override;
void AddBoundSessionRequestThrottledListenerReceiver(
Expand Down

0 comments on commit 1c12e91

Please sign in to comment.