Commit 256fbf85 authored by Ryan Hansberry's avatar Ryan Hansberry Committed by Commit Bot

[CrOS Multidevice] Make EasyUnlockServiceRegular conditionally use DeviceSyncClient.

EasyUnlockServiceRegular now uses either CryptAuthService or DeviceSyncClient
to get remote device info or perform Cryptauth calls, depending on whether the
chromeos::features::kMultiDeviceApi is enabled. Once the migration to DeviceSyncClient
has been completed across all of Smart Lock, code that references CryptAuthService will
be removed.

This is the last of other similar CLs which are migrating Smart Lock to use
DeviceSync Mojo API instead of directly using the CryptAuth API.

This CL also injects DeviceSyncClient into EasyUnlockServiceRegular, making Smart Lock
rely entirely on DeviceSyncClient if chromeos::features::kMultiDeviceApi is enabled.
I have manually verified that it works correctly by testing the following:

1. Setting up Smart Lock.
2. Using Smart Lock in the regular lock-screen manner.
3. Using Smart Lock in the login-screen manner.
4. Ensuring that notifications display at the correct time.
5. Turning off Smart Lock.
6. Ensuring that permits are correctly saved to prefs (the new model can't
   use the old way of grabbing the user's AccountId).

TBR=jhawkins@chromium.org

Bug: 824568, 752273, 848956
Change-Id: I3acc6dec46ff1ddc2875fbfe97261bb195a9494b
Reviewed-on: https://chromium-review.googlesource.com/1102164
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567769}
parent 97f4fb9b
......@@ -64,7 +64,8 @@ constexpr char kInvalidPassword[] = "invalid";
class FakeEasyUnlockService : public EasyUnlockServiceRegular {
public:
explicit FakeEasyUnlockService(Profile* profile)
: EasyUnlockServiceRegular(profile), reauth_count_(0) {}
: EasyUnlockServiceRegular(profile, nullptr /* device_sync_client */),
reauth_count_(0) {}
~FakeEasyUnlockService() override {}
// EasyUnlockServiceRegular:
......
......@@ -8,6 +8,7 @@
#include "base/memory/singleton.h"
#include "build/build_config.h"
#include "chrome/browser/chromeos/cryptauth/chrome_cryptauth_service_factory.h"
#include "chrome/browser/chromeos/device_sync/device_sync_client_factory.h"
#include "chrome/browser/chromeos/login/easy_unlock/easy_unlock_app_manager.h"
#include "chrome/browser/chromeos/login/easy_unlock/easy_unlock_service.h"
#include "chrome/browser/chromeos/login/easy_unlock/easy_unlock_service_regular.h"
......@@ -68,6 +69,7 @@ EasyUnlockServiceFactory::EasyUnlockServiceFactory()
DependsOn(
extensions::ExtensionsBrowserClient::Get()->GetExtensionSystemFactory());
DependsOn(EasyUnlockTpmKeyManagerFactory::GetInstance());
DependsOn(device_sync::DeviceSyncClientFactory::GetInstance());
}
EasyUnlockServiceFactory::~EasyUnlockServiceFactory() {}
......@@ -90,8 +92,10 @@ KeyedService* EasyUnlockServiceFactory::BuildServiceInstanceFor(
}
if (!service) {
service =
new EasyUnlockServiceRegular(Profile::FromBrowserContext(context));
service = new EasyUnlockServiceRegular(
Profile::FromBrowserContext(context),
device_sync::DeviceSyncClientFactory::GetForProfile(
Profile::FromBrowserContext(context)));
manifest_id = IDR_EASY_UNLOCK_MANIFEST;
}
......
......@@ -10,13 +10,16 @@
#include "base/callback.h"
#include "base/macros.h"
#include "base/optional.h"
#include "base/scoped_observer.h"
#include "base/time/time.h"
#include "build/build_config.h"
#include "chrome/browser/chromeos/login/easy_unlock/easy_unlock_service.h"
#include "chrome/browser/chromeos/login/easy_unlock/short_lived_user_context.h"
#include "chromeos/components/proximity_auth/screenlock_bridge.h"
#include "chromeos/services/device_sync/public/cpp/device_sync_client.h"
#include "components/cryptauth/cryptauth_device_manager.h"
#include "components/cryptauth/remote_device_ref.h"
#include "components/prefs/pref_change_registrar.h"
namespace base {
......@@ -47,14 +50,18 @@ class EasyUnlockNotificationController;
class EasyUnlockServiceRegular
: public EasyUnlockService,
public proximity_auth::ScreenlockBridge::Observer,
public cryptauth::CryptAuthDeviceManager::Observer {
public cryptauth::CryptAuthDeviceManager::Observer,
public device_sync::DeviceSyncClient::Observer {
public:
explicit EasyUnlockServiceRegular(Profile* profile);
explicit EasyUnlockServiceRegular(
Profile* profile,
device_sync::DeviceSyncClient* device_sync_client);
// Constructor for tests.
EasyUnlockServiceRegular(Profile* profile,
std::unique_ptr<EasyUnlockNotificationController>
notification_controller);
EasyUnlockServiceRegular(
Profile* profile,
std::unique_ptr<EasyUnlockNotificationController> notification_controller,
device_sync::DeviceSyncClient* device_sync_client);
~EasyUnlockServiceRegular() override;
......@@ -66,6 +73,9 @@ class EasyUnlockServiceRegular
// Called when |remote_device_loader_| completes.
void OnRemoteDevicesLoaded(const cryptauth::RemoteDeviceList& remote_devices);
void UseLoadedRemoteDevices(
const cryptauth::RemoteDeviceRefList& remote_devices);
// EasyUnlockService implementation:
proximity_auth::ProximityAuthPrefManager* GetProximityAuthPrefManager()
override;
......@@ -98,6 +108,15 @@ class EasyUnlockServiceRegular
cryptauth::CryptAuthDeviceManager::DeviceChangeResult
device_change_result) override;
// device_sync::DeviceSyncClient::Observer:
void OnNewDevicesSynced() override;
void ShowNotificationIfNewDevicePresent(
const std::set<std::string>& public_keys_before_sync,
const std::set<std::string>& public_keys_after_sync);
void OnForceSyncCompleted(bool success);
// proximity_auth::ScreenlockBridge::Observer implementation:
void OnScreenDidLock(proximity_auth::ScreenlockBridge::LockHandler::ScreenType
screen_type) override;
......@@ -114,6 +133,12 @@ class EasyUnlockServiceRegular
const cryptauth::ToggleEasyUnlockResponse& response);
void OnToggleEasyUnlockApiFailed(const std::string& error_message);
void OnTurnOffEasyUnlockCompleted(
const base::Optional<std::string>& error_code);
void OnTurnOffEasyUnlockSuccess();
void OnTurnOffEasyUnlockFailure(const std::string& error_message);
// Called with the user's credentials (e.g. username and password) after the
// user reauthenticates to begin setup.
void OpenSetupAppAfterReauth(const UserContext& user_context);
......@@ -138,6 +163,8 @@ class EasyUnlockServiceRegular
// Otherwise, hardlock the device.
void RefreshCryptohomeKeysIfPossible();
cryptauth::RemoteDeviceRefList GetUnlockKeys();
TurnOffFlowStatus turn_off_flow_status_;
std::unique_ptr<cryptauth::CryptAuthClient> cryptauth_client_;
ScopedObserver<cryptauth::CryptAuthDeviceManager, EasyUnlockServiceRegular>
......@@ -174,15 +201,26 @@ class EasyUnlockServiceRegular
// Responsible for showing all the notifications used for EasyUnlock.
std::unique_ptr<EasyUnlockNotificationController> notification_controller_;
device_sync::DeviceSyncClient* device_sync_client_;
// Stores the unlock keys for EasyUnlock before the current device sync, so we
// can compare it to the unlock keys after syncing.
std::vector<cryptauth::ExternalDeviceInfo> unlock_keys_before_sync_;
cryptauth::RemoteDeviceRefList remote_device_unlock_keys_before_sync_;
// True if the pairing changed notification was shown, so that the next time
// the Chromebook is unlocked, we can show the subsequent 'pairing applied'
// notification.
bool shown_pairing_changed_notification_;
// If this service is the first caller on DeviceSyncClient, it won't have
// devices cached yet. |is_waiting_for_initial_sync_| is set to true if
// DeviceSyncClient has no devices, to indicate that we are waiting for the
// initial sync, to be inspected in OnNewDevicesSynced(). OnNewDevicesSynced()
// needs to know that it is receiving the initial sync, not a newly forced
// one, in order to prevent it from running unrelated logic.
bool is_waiting_for_initial_sync_ = false;
// Listens to pref changes.
PrefChangeRegistrar registrar_;
......
......@@ -29,6 +29,7 @@
#include "chromeos/dbus/dbus_thread_manager.h"
#include "chromeos/dbus/fake_power_manager_client.h"
#include "chromeos/dbus/power_manager/suspend.pb.h"
#include "chromeos/services/device_sync/public/cpp/fake_device_sync_client.h"
#include "components/account_id/account_id.h"
#include "components/signin/core/browser/signin_manager_base.h"
#include "components/sync_preferences/testing_pref_service_syncable.h"
......@@ -44,6 +45,7 @@ using testing::AnyNumber;
using testing::Return;
namespace chromeos {
namespace {
// IDs for fake users used in tests.
......@@ -181,10 +183,15 @@ class TestAppManagerFactory {
DISALLOW_COPY_AND_ASSIGN(TestAppManagerFactory);
};
// Global TestAppManager factory. It should be created and desctructed in
// EasyUnlockServiceTest::SetUp and EasyUnlockServiceTest::TearDown
// Global TestAppManager factory. It should be created and destroyed in
// EasyUnlockServiceTest::SetUp and EasyUnlockServiceTest::TearDown,
// respectively.
TestAppManagerFactory* app_manager_factory = nullptr;
// Global FakeDeviceSyncClient. It should be created and destroyed in
// EasyUnlockServiceTest::SetUp and EasyUnlockServiceTest::TearDown,
// respectively.
TestAppManagerFactory* app_manager_factory = NULL;
device_sync::FakeDeviceSyncClient* fake_device_sync_client = nullptr;
// EasyUnlockService factory function injected into testing profiles.
// It creates an EasyUnlockService with test AppManager.
......@@ -203,13 +210,16 @@ std::unique_ptr<KeyedService> CreateEasyUnlockServiceForTest(
std::unique_ptr<EasyUnlockServiceRegular> service(
new EasyUnlockServiceRegular(
Profile::FromBrowserContext(context),
std::make_unique<MockEasyUnlockNotificationController>()));
std::make_unique<MockEasyUnlockNotificationController>(),
fake_device_sync_client));
service->Initialize(std::move(app_manager));
return std::move(service);
}
} // namespace
class EasyUnlockServiceTest : public testing::Test {
public:
protected:
EasyUnlockServiceTest()
: mock_user_manager_(new testing::NiceMock<MockUserManager>()),
scoped_user_manager_(base::WrapUnique(mock_user_manager_)),
......@@ -219,6 +229,7 @@ class EasyUnlockServiceTest : public testing::Test {
void SetUp() override {
app_manager_factory = new TestAppManagerFactory();
fake_device_sync_client = new device_sync::FakeDeviceSyncClient();
mock_adapter_ = new testing::NiceMock<MockBluetoothAdapter>();
device::BluetoothAdapterFactory::SetAdapterForTesting(mock_adapter_);
......@@ -247,8 +258,12 @@ class EasyUnlockServiceTest : public testing::Test {
void TearDown() override {
TestingBrowserProcess::GetGlobal()->SetLocalState(nullptr);
delete app_manager_factory;
app_manager_factory = NULL;
app_manager_factory = nullptr;
delete fake_device_sync_client;
fake_device_sync_client = nullptr;
}
void SetEasyUnlockAllowedPolicy(bool allowed) {
......@@ -286,13 +301,6 @@ class EasyUnlockServiceTest : public testing::Test {
app_manager->SetReady();
}
void SetUpSecondaryProfile() {
SetUpProfile(
&secondary_profile_,
AccountId::FromUserEmailGaiaId(kTestUserSecondary, kSecondaryGaiaId));
}
private:
// Sets up a test profile with a user id.
void SetUpProfile(std::unique_ptr<TestingProfile>* profile,
const AccountId& account_id) {
......@@ -313,11 +321,15 @@ class EasyUnlockServiceTest : public testing::Test {
account_id.GetUserEmail());
}
private:
void SetUpSecondaryProfile() {
SetUpProfile(
&secondary_profile_,
AccountId::FromUserEmailGaiaId(kTestUserSecondary, kSecondaryGaiaId));
}
// Must outlive TestingProfiles.
content::TestBrowserThreadBundle thread_bundle_;
protected:
std::unique_ptr<TestingProfile> profile_;
std::unique_ptr<TestingProfile> secondary_profile_;
MockUserManager* mock_user_manager_;
......@@ -413,5 +425,4 @@ TEST_F(EasyUnlockServiceTest, GetAccountId) {
EasyUnlockService::Get(secondary_profile_.get())->GetAccountId());
}
} // namespace
} // namespace chromeos
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment