Skip to content

Commit

Permalink
[sync] Invoke new APIs in TrustedVaultClientBackend
Browse files Browse the repository at this point in the history
This includes:
1. ClearLocalData, as per pre-existing TODO.
2. GetPublicKeyForIdentity, as part of registration verification.
3. SetDeviceRegistrationPublicKeyVerifierForUMA, guarded behind
   experiment toggle, to actually trigger registration verification.

The internal implementation of the APIs being invoked in this patch
landed recently with https://crrev.com/i/5496191.

The actual logic to verify the registration will be tackled in future
patches as it is quite complex.

Bug: 1416626, 1273080
Change-Id: I0c16aed7de2be3c63d1c213000e3f01b7da499ca
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4275301
Reviewed-by: Jérôme Lebel <jlebel@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1108830}
  • Loading branch information
Mikel Astiz authored and Chromium LUCI CQ committed Feb 23, 2023
1 parent ee7a0cf commit acf2fcf
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ void GetPublicKeyForIdentity(id<SystemIdentity> identity,
void ChromiumTrustedVaultClientBackend::ClearLocalData(
id<SystemIdentity> identity,
base::OnceCallback<void(bool)> callback) {
NOTREACHED();
// Do nothing.
}

void ChromiumTrustedVaultClientBackend::GetPublicKeyForIdentity(
Expand Down
9 changes: 3 additions & 6 deletions ios/chrome/browser/signin/trusted_vault_client_backend.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,8 @@ class TrustedVaultClientBackend : public KeyedService {

// Registers a delegate-like callback that implements device registration
// verification.
// TODO(crbug.com/1416626): Make abstract once all implementations land.
virtual void SetDeviceRegistrationPublicKeyVerifierForUMA(
VerifierCallback verifier);
VerifierCallback verifier) = 0;

// Asynchronously fetches the shared keys for `identity` and invokes
// `callback` with the fetched keys.
Expand Down Expand Up @@ -102,14 +101,12 @@ class TrustedVaultClientBackend : public KeyedService {

// Clears local data belonging to `identity`, such as shared keys. This
// excludes the physical client's key pair, which remains unchanged.
// TODO(crbug.com/1416626): Make abstract once all implementations land.
virtual void ClearLocalData(id<SystemIdentity> identity,
base::OnceCallback<void(bool)> callback);
base::OnceCallback<void(bool)> callback) = 0;

// Returns the member public key used to enroll the local device.
// TODO(crbug.com/1416626): Make abstract once all implementations land.
virtual void GetPublicKeyForIdentity(id<SystemIdentity> identity,
GetPublicKeyCallback callback);
GetPublicKeyCallback callback) = 0;
};

#endif // IOS_CHROME_BROWSER_SIGNIN_TRUSTED_VAULT_CLIENT_BACKEND_H_
20 changes: 0 additions & 20 deletions ios/chrome/browser/signin/trusted_vault_client_backend.mm
Original file line number Diff line number Diff line change
Expand Up @@ -4,30 +4,10 @@

#import "ios/chrome/browser/signin/trusted_vault_client_backend.h"

#import "base/functional/callback.h"
#import "base/notreached.h"

#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
#endif

TrustedVaultClientBackend::TrustedVaultClientBackend() = default;

TrustedVaultClientBackend::~TrustedVaultClientBackend() = default;

void TrustedVaultClientBackend::SetDeviceRegistrationPublicKeyVerifierForUMA(
VerifierCallback verifier) {
NOTREACHED();
}

void TrustedVaultClientBackend::ClearLocalData(
id<SystemIdentity> identity,
base::OnceCallback<void(bool)> callback) {
NOTREACHED();
}

void TrustedVaultClientBackend::GetPublicKeyForIdentity(
id<SystemIdentity> identity,
GetPublicKeyCallback callback) {
NOTREACHED();
}
6 changes: 6 additions & 0 deletions ios/chrome/browser/sync/ios_trusted_vault_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#ifndef IOS_CHROME_BROWSER_SYNC_IOS_TRUSTED_VAULT_CLIENT_H_
#define IOS_CHROME_BROWSER_SYNC_IOS_TRUSTED_VAULT_CLIENT_H_

#include "base/memory/weak_ptr.h"
#include "components/sync/driver/trusted_vault_client.h"

class ChromeAccountManagerService;
Expand Down Expand Up @@ -47,9 +48,14 @@ class IOSTrustedVaultClient : public syncer::TrustedVaultClient {
private:
// Returns the identity for `account_info`.
id<SystemIdentity> IdentityForAccount(const CoreAccountInfo& account_info);
void VerifyDeviceRegistration(const std::string& gaia_id);
void VerifyDeviceRegistrationWithPublicKey(
const std::string& gaia_id,
const std::vector<uint8_t>& public_key);

ChromeAccountManagerService* const account_manager_service_ = nullptr;
TrustedVaultClientBackend* const backend_ = nullptr;
base::WeakPtrFactory<IOSTrustedVaultClient> weak_ptr_factory_{this};
};

#endif // IOS_CHROME_BROWSER_SYNC_IOS_TRUSTED_VAULT_CLIENT_H_
30 changes: 28 additions & 2 deletions ios/chrome/browser/sync/ios_trusted_vault_client.mm
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@

#import "ios/chrome/browser/sync/ios_trusted_vault_client.h"

#import "base/feature_list.h"
#import "base/functional/callback_helpers.h"
#import "components/signin/public/identity_manager/account_info.h"
#import "components/sync/base/features.h"
#import "ios/chrome/browser/signin/chrome_account_manager_service.h"
#import "ios/chrome/browser/signin/trusted_vault_client_backend.h"

Expand All @@ -19,6 +22,13 @@
backend_(trusted_vault_client_backend) {
DCHECK(account_manager_service_);
DCHECK(backend_);

if (base::FeatureList::IsEnabled(
syncer::kSyncTrustedVaultVerifyDeviceRegistration)) {
backend_->SetDeviceRegistrationPublicKeyVerifierForUMA(
base::BindOnce(&IOSTrustedVaultClient::VerifyDeviceRegistration,
weak_ptr_factory_.GetWeakPtr()));
}
}

IOSTrustedVaultClient::~IOSTrustedVaultClient() = default;
Expand Down Expand Up @@ -74,11 +84,27 @@

void IOSTrustedVaultClient::ClearDataForAccount(
const CoreAccountInfo& account_info) {
// TODO(crbug.com/1273080): decide whether this logic needs to be implemented
// on iOS.
backend_->ClearLocalData(IdentityForAccount(account_info), base::DoNothing());
}

id<SystemIdentity> IOSTrustedVaultClient::IdentityForAccount(
const CoreAccountInfo& account_info) {
return account_manager_service_->GetIdentityWithGaiaID(account_info.gaia);
}

void IOSTrustedVaultClient::VerifyDeviceRegistration(
const std::string& gaia_id) {
backend_->GetPublicKeyForIdentity(
account_manager_service_->GetIdentityWithGaiaID(gaia_id),
base::BindOnce(
&IOSTrustedVaultClient::VerifyDeviceRegistrationWithPublicKey,
weak_ptr_factory_.GetWeakPtr(), gaia_id));
}

void IOSTrustedVaultClient::VerifyDeviceRegistrationWithPublicKey(
const std::string& gaia_id,
const std::vector<uint8_t>& public_key) {
// TODO(crbug.com/1416626): Issue a request to the server and log the result
// to metrics.
NOTIMPLEMENTED();
}

0 comments on commit acf2fcf

Please sign in to comment.