diff --git a/chromeos/services/device_sync/BUILD.gn b/chromeos/services/device_sync/BUILD.gn index 85c03177ef56a6..a0e4e6733caad4 100644 --- a/chromeos/services/device_sync/BUILD.gn +++ b/chromeos/services/device_sync/BUILD.gn @@ -67,8 +67,6 @@ static_library("device_sync") { "device_sync_service.h", "device_sync_type_converters.cc", "device_sync_type_converters.h", - "network_aware_enrollment_scheduler.cc", - "network_aware_enrollment_scheduler.h", "network_request_error.cc", "network_request_error.h", "pref_names.cc", @@ -187,7 +185,6 @@ source_set("unit_tests") { "cryptauth_v2_enroller_impl_unittest.cc", "cryptauth_v2_enrollment_manager_impl_unittest.cc", "device_sync_service_unittest.cc", - "network_aware_enrollment_scheduler_unittest.cc", "remote_device_loader_unittest.cc", "remote_device_provider_impl_unittest.cc", "software_feature_manager_impl_unittest.cc", diff --git a/chromeos/services/device_sync/cryptauth_scheduler.cc b/chromeos/services/device_sync/cryptauth_scheduler.cc index 6788576aa34dee..aae09f60a35b82 100644 --- a/chromeos/services/device_sync/cryptauth_scheduler.cc +++ b/chromeos/services/device_sync/cryptauth_scheduler.cc @@ -8,19 +8,39 @@ namespace chromeos { namespace device_sync { -CryptAuthScheduler::CryptAuthScheduler(Delegate* delegate) - : delegate_(delegate) { - DCHECK(delegate); -} +CryptAuthScheduler::CryptAuthScheduler() = default; CryptAuthScheduler::~CryptAuthScheduler() = default; +void CryptAuthScheduler::StartEnrollmentScheduling( + const base::WeakPtr& enrollment_delegate) { + // Ensure this is only called once. + DCHECK(!enrollment_delegate_); + + DCHECK(enrollment_delegate); + enrollment_delegate_ = enrollment_delegate; + + OnEnrollmentSchedulingStarted(); +} + +bool CryptAuthScheduler::HasEnrollmentSchedulingStarted() { + return enrollment_delegate_.get(); +} + void CryptAuthScheduler::NotifyEnrollmentRequested( + const cryptauthv2::ClientMetadata& client_metadata, const base::Optional& client_directive_policy_reference) const { - delegate_->OnEnrollmentRequested(client_directive_policy_reference); + // Do nothing if weak pointer was invalidated. + if (!enrollment_delegate_) + return; + + enrollment_delegate_->OnEnrollmentRequested( + client_metadata, client_directive_policy_reference); } +void CryptAuthScheduler::OnEnrollmentSchedulingStarted() {} + } // namespace device_sync } // namespace chromeos diff --git a/chromeos/services/device_sync/cryptauth_scheduler.h b/chromeos/services/device_sync/cryptauth_scheduler.h index ab18523ec8758f..45b956a4dacd98 100644 --- a/chromeos/services/device_sync/cryptauth_scheduler.h +++ b/chromeos/services/device_sync/cryptauth_scheduler.h @@ -5,7 +5,10 @@ #ifndef CHROMEOS_SERVICES_DEVICE_SYNC_CRYPTAUTH_SCHEDULER_H_ #define CHROMEOS_SERVICES_DEVICE_SYNC_CRYPTAUTH_SCHEDULER_H_ +#include + #include "base/macros.h" +#include "base/memory/weak_ptr.h" #include "base/optional.h" #include "base/time/time.h" #include "chromeos/services/device_sync/cryptauth_enrollment_result.h" @@ -17,34 +20,45 @@ namespace device_sync { // Schedules periodic enrollments, alerting the delegate when an enrollment // attempt is due via Delegate::OnEnrollmentRequested(). The periodic scheduling -// begins on construction. The client can bypass the periodic schedule and -// immediately trigger an enrollment request via RequestEnrollmentNow(). When an +// begins on StartEnrollmentScheduling(). The client can bypass the periodic +// schedule and trigger an enrollment request via RequestEnrollment(). When an // enrollment attempt has completed, successfully or not, the client should // invoke HandleEnrollmentResult() so the scheduler can process the enrollment // attempt outcome. class CryptAuthScheduler { public: - class Delegate { + class EnrollmentDelegate { public: - Delegate() = default; - virtual ~Delegate() = default; + EnrollmentDelegate() = default; + virtual ~EnrollmentDelegate() = default; // Called to alert the delegate that an enrollment attempt has been // requested by the scheduler. - // |client_directive_policy_reference|: Identifies the CryptAuth policy - // associated with the ClientDirective parameters used to schedule this - // enrollment attempt. If no ClientDirective was used by the scheduler, - // base::nullopt is passed. + // |client_metadata|: Contains the retry count, invocation reason, and + // possible session ID of the enrollment request. + // |client_directive_policy_reference|: Identifies the CryptAuth policy + // associated with the ClientDirective parameters used to schedule + // this enrollment attempt. If no ClientDirective was used by the + // scheduler, base::nullopt is passed. virtual void OnEnrollmentRequested( + const cryptauthv2::ClientMetadata& client_metadata, const base::Optional& client_directive_policy_reference) = 0; }; virtual ~CryptAuthScheduler(); - // Cancels the currently scheduled enrollment, and requests an enrollment - // immediately. - virtual void RequestEnrollmentNow() = 0; + // Note: This should only be called once. + void StartEnrollmentScheduling( + const base::WeakPtr& enrollment_delegate); + + bool HasEnrollmentSchedulingStarted(); + + // Requests an enrollment with the desired |invocation_reason| and, if + // relevant, the |session_id| of the GCM message that requested enrollment. + virtual void RequestEnrollment( + const cryptauthv2::ClientMetadata::InvocationReason& invocation_reason, + const base::Optional& session_id) = 0; // Processes the result of the previous enrollment attempt. virtual void HandleEnrollmentResult( @@ -55,10 +69,10 @@ class CryptAuthScheduler { virtual base::Optional GetLastSuccessfulEnrollmentTime() const = 0; - // Returns the scheduler's time period between a successful enrollment and - // its next enrollment request. Note that this period may not be strictly - // adhered to due to RequestEnrollmentNow() calls or the system being offline, - // for instance. + // Returns the scheduler's time period between a successful enrollment and its + // next enrollment request. Note that this period may not be strictly adhered + // to due to RequestEnrollment() calls or the system being offline, for + // instance. virtual base::TimeDelta GetRefreshPeriod() const = 0; // Returns the time until the next scheduled enrollment request. @@ -68,20 +82,24 @@ class CryptAuthScheduler { // has not been processed yet. virtual bool IsWaitingForEnrollmentResult() const = 0; - // The number of consecutive failed enrollment attempts. Once an enrollment - // attempt succeeds, this counter is reset. - virtual size_t GetNumConsecutiveFailures() const = 0; + // The number of times the current enrollment request has failed. Once the + // enrollment request succeeds or a fresh request is made--for example, via a + // forced enrollment--this counter is reset. + virtual size_t GetNumConsecutiveEnrollmentFailures() const = 0; protected: - explicit CryptAuthScheduler(Delegate* delegate); + CryptAuthScheduler(); + + virtual void OnEnrollmentSchedulingStarted(); // Alerts the delegate that an enrollment has been requested. void NotifyEnrollmentRequested( + const cryptauthv2::ClientMetadata& client_metadata, const base::Optional& client_directive_policy_reference) const; private: - Delegate* delegate_; + base::WeakPtr enrollment_delegate_ = nullptr; DISALLOW_COPY_AND_ASSIGN(CryptAuthScheduler); }; diff --git a/chromeos/services/device_sync/cryptauth_scheduler_impl.cc b/chromeos/services/device_sync/cryptauth_scheduler_impl.cc index 4dc39941394210..acc647463f4314 100644 --- a/chromeos/services/device_sync/cryptauth_scheduler_impl.cc +++ b/chromeos/services/device_sync/cryptauth_scheduler_impl.cc @@ -5,14 +5,14 @@ #include "chromeos/services/device_sync/cryptauth_scheduler_impl.h" #include -#include #include #include "base/base64.h" #include "base/memory/ptr_util.h" #include "base/no_destructor.h" -#include "base/optional.h" #include "chromeos/components/multidevice/logging/logging.h" +#include "chromeos/network/network_state.h" +#include "chromeos/network/network_state_handler.h" #include "chromeos/services/device_sync/pref_names.h" #include "components/prefs/pref_registry_simple.h" #include "components/prefs/pref_service.h" @@ -23,6 +23,8 @@ namespace device_sync { namespace { +constexpr base::TimeDelta kZeroTimeDelta = base::TimeDelta::FromSeconds(0); + // The default period between successful enrollments in days. Superseded by the // ClientDirective's checkin_delay_millis sent by CryptAuth in SyncKeysResponse. constexpr base::TimeDelta kDefaultRefreshPeriod = base::TimeDelta::FromDays(30); @@ -43,6 +45,10 @@ constexpr base::TimeDelta kImmediateRetryDelay = // SyncKeysResponse. const int kDefaultMaxImmediateRetries = 3; +const char kNoClientDirective[] = "[No ClientDirective]"; + +const char kNoClientMetadata[] = "[No ClientMetadata]"; + bool IsClientDirectiveValid( const cryptauthv2::ClientDirective& client_directive) { return client_directive.checkin_delay_millis() > 0 && @@ -65,9 +71,10 @@ cryptauthv2::ClientDirective CreateDefaultClientDirective() { } // Decodes and parses the base64-encoded serialized ClientDirective string. +// TODO(https://crbug.com/964563): Replace when utility functions are added. base::Optional ClientDirectiveFromPrefString( const std::string& encoded_serialized_client_directive) { - if (encoded_serialized_client_directive.empty()) + if (encoded_serialized_client_directive == kNoClientDirective) return base::nullopt; std::string decoded_serialized_client_directive; @@ -87,6 +94,7 @@ base::Optional ClientDirectiveFromPrefString( } // Serializes and base64 encodes the input ClientDirective. +// TODO(https://crbug.com/964563): Replace when utility functions are added. std::string ClientDirectiveToPrefString( const cryptauthv2::ClientDirective& client_directive) { std::string encoded_serialized_client_directive; @@ -100,14 +108,59 @@ cryptauthv2::ClientDirective BuildClientDirective(PrefService* pref_service) { DCHECK(pref_service); base::Optional client_directive_from_pref = - ClientDirectiveFromPrefString(pref_service->GetString( - prefs::kCryptAuthEnrollmentSchedulerClientDirective)); + ClientDirectiveFromPrefString( + pref_service->GetString(prefs::kCryptAuthSchedulerClientDirective)); if (client_directive_from_pref) return *client_directive_from_pref; return CreateDefaultClientDirective(); } +cryptauthv2::ClientMetadata BuildClientMetadata( + size_t retry_count, + const cryptauthv2::ClientMetadata::InvocationReason& invocation_reason, + const base::Optional& session_id) { + cryptauthv2::ClientMetadata client_metadata; + client_metadata.set_retry_count(retry_count); + client_metadata.set_invocation_reason(invocation_reason); + if (session_id) + client_metadata.set_session_id(*session_id); + + return client_metadata; +} + +// TODO(https://crbug.com/964563): Replace when utility functions are added. +base::Optional ClientMetadataFromPrefString( + const std::string& encoded_serialized_client_metadata) { + if (encoded_serialized_client_metadata == kNoClientMetadata) + return base::nullopt; + + std::string decoded_serialized_client_metadata; + if (!base::Base64Decode(encoded_serialized_client_metadata, + &decoded_serialized_client_metadata)) { + PA_LOG(ERROR) << "Error decoding ClientMetadata pref string"; + return base::nullopt; + } + + cryptauthv2::ClientMetadata client_metadata; + if (!client_metadata.ParseFromString(decoded_serialized_client_metadata)) { + PA_LOG(ERROR) << "Error parsing ClientMetadata from pref string"; + return base::nullopt; + } + + return client_metadata; +} + +// TODO(https://crbug.com/964563): Replace when utility functions are added. +std::string ClientMetadataToPrefString( + const cryptauthv2::ClientMetadata& client_metadata) { + std::string encoded_serialized_client_metadata; + base::Base64Encode(client_metadata.SerializeAsString(), + &encoded_serialized_client_metadata); + + return encoded_serialized_client_metadata; +} + } // namespace // static @@ -133,105 +186,116 @@ CryptAuthSchedulerImpl::Factory::~Factory() = default; std::unique_ptr CryptAuthSchedulerImpl::Factory::BuildInstance( - Delegate* delegate, PrefService* pref_service, + NetworkStateHandler* network_state_handler, base::Clock* clock, - std::unique_ptr timer, - scoped_refptr task_runner) { + std::unique_ptr enrollment_timer) { return base::WrapUnique(new CryptAuthSchedulerImpl( - delegate, pref_service, clock, std::move(timer), task_runner)); + pref_service, network_state_handler, clock, std::move(enrollment_timer))); } // static void CryptAuthSchedulerImpl::RegisterPrefs(PrefRegistrySimple* registry) { + registry->RegisterStringPref(prefs::kCryptAuthSchedulerClientDirective, + kNoClientDirective); registry->RegisterStringPref( - prefs::kCryptAuthEnrollmentSchedulerClientDirective, std::string()); + prefs::kCryptAuthSchedulerNextEnrollmentRequestClientMetadata, + kNoClientMetadata); registry->RegisterTimePref( - prefs::kCryptAuthEnrollmentSchedulerLastEnrollmentAttemptTime, - base::Time()); + prefs::kCryptAuthSchedulerLastEnrollmentAttemptTime, base::Time()); registry->RegisterTimePref( - prefs::kCryptAuthEnrollmentSchedulerLastSuccessfulEnrollmentTime, - base::Time()); - registry->RegisterUint64Pref( - prefs::kCryptAuthEnrollmentSchedulerNumConsecutiveFailures, 0); + prefs::kCryptAuthSchedulerLastSuccessfulEnrollmentTime, base::Time()); } CryptAuthSchedulerImpl::CryptAuthSchedulerImpl( - Delegate* delegate, PrefService* pref_service, + NetworkStateHandler* network_state_handler, base::Clock* clock, - std::unique_ptr timer, - scoped_refptr task_runner) - : CryptAuthScheduler(delegate), - pref_service_(pref_service), + std::unique_ptr enrollment_timer) + : pref_service_(pref_service), + network_state_handler_(network_state_handler), clock_(clock), - timer_(std::move(timer)), - client_directive_(BuildClientDirective(pref_service)), - weak_ptr_factory_(this) { - DCHECK(pref_service); - DCHECK(clock); + enrollment_timer_(std::move(enrollment_timer)), + client_directive_(BuildClientDirective(pref_service)) { + DCHECK(pref_service_); + DCHECK(network_state_handler_); + DCHECK(clock_); DCHECK(IsClientDirectiveValid(client_directive_)); - // If we are recovering from a failure, set the failure count back to 1 in the + // Queue up the most recently scheduled enrollment request if applicable. + pending_enrollment_request_ = + ClientMetadataFromPrefString(pref_service_->GetString( + prefs::kCryptAuthSchedulerNextEnrollmentRequestClientMetadata)); + + // If we are recovering from a failure, reset the failure count to 1 in the // hopes that the restart solved the issue. This will allow for immediate // retries again if allowed by the ClientDirective. - if (GetNumConsecutiveFailures() > 1) { - pref_service_->SetUint64( - prefs::kCryptAuthEnrollmentSchedulerNumConsecutiveFailures, 1); + if (pending_enrollment_request_ && + pending_enrollment_request_->retry_count() > 0) { + pending_enrollment_request_->set_retry_count(1); } +} + +CryptAuthSchedulerImpl::~CryptAuthSchedulerImpl() { + if (network_state_handler_) + network_state_handler_->RemoveObserver(this, FROM_HERE); +} + +void CryptAuthSchedulerImpl::OnEnrollmentSchedulingStarted() { + network_state_handler_->AddObserver(this, FROM_HERE); - // Schedule the next enrollment as part of a new task. This ensures that the - // delegate can complete its initialization before handling an enrollment - // request. - task_runner->PostTask( - FROM_HERE, base::BindOnce(&CryptAuthSchedulerImpl::ScheduleNextEnrollment, - weak_ptr_factory_.GetWeakPtr())); + ScheduleNextEnrollment(); } -CryptAuthSchedulerImpl::~CryptAuthSchedulerImpl() = default; +void CryptAuthSchedulerImpl::RequestEnrollment( + const cryptauthv2::ClientMetadata::InvocationReason& invocation_reason, + const base::Optional& session_id) { + enrollment_timer_->Stop(); -void CryptAuthSchedulerImpl::RequestEnrollmentNow() { - timer_->Stop(); - NotifyEnrollmentRequested(GetPolicyReference()); + pending_enrollment_request_ = + BuildClientMetadata(0 /* retry_count */, invocation_reason, session_id); + + ScheduleNextEnrollment(); } void CryptAuthSchedulerImpl::HandleEnrollmentResult( const CryptAuthEnrollmentResult& enrollment_result) { - DCHECK(!timer_->IsRunning()); + DCHECK(current_enrollment_request_); + DCHECK(!enrollment_timer_->IsRunning()); base::Time now = clock_->Now(); - if (enrollment_result.IsSuccess()) { - pref_service_->SetUint64( - prefs::kCryptAuthEnrollmentSchedulerNumConsecutiveFailures, 0); + pref_service_->SetTime(prefs::kCryptAuthSchedulerLastEnrollmentAttemptTime, + now); + // If unsuccessful and a more immediate request isn't pending, queue up the + // failure recovery attempt. + if (enrollment_result.IsSuccess()) { pref_service_->SetTime( - prefs::kCryptAuthEnrollmentSchedulerLastSuccessfulEnrollmentTime, now); - } else { - pref_service_->SetUint64( - prefs::kCryptAuthEnrollmentSchedulerNumConsecutiveFailures, - GetNumConsecutiveFailures() + 1); + prefs::kCryptAuthSchedulerLastSuccessfulEnrollmentTime, now); + } else if (!pending_enrollment_request_) { + current_enrollment_request_->set_retry_count( + current_enrollment_request_->retry_count() + 1); + pending_enrollment_request_ = current_enrollment_request_; } - pref_service_->SetTime( - prefs::kCryptAuthEnrollmentSchedulerLastEnrollmentAttemptTime, now); - if (enrollment_result.client_directive() && IsClientDirectiveValid(*enrollment_result.client_directive())) { client_directive_ = *enrollment_result.client_directive(); - pref_service_->SetString( - prefs::kCryptAuthEnrollmentSchedulerClientDirective, - ClientDirectiveToPrefString(client_directive_)); + pref_service_->SetString(prefs::kCryptAuthSchedulerClientDirective, + ClientDirectiveToPrefString(client_directive_)); } + current_enrollment_request_.reset(); + ScheduleNextEnrollment(); } base::Optional CryptAuthSchedulerImpl::GetLastSuccessfulEnrollmentTime() const { base::Time time = pref_service_->GetTime( - prefs::kCryptAuthEnrollmentSchedulerLastSuccessfulEnrollmentTime); + prefs::kCryptAuthSchedulerLastSuccessfulEnrollmentTime); if (time.is_null()) return base::nullopt; @@ -244,61 +308,139 @@ base::TimeDelta CryptAuthSchedulerImpl::GetRefreshPeriod() const { } base::TimeDelta CryptAuthSchedulerImpl::GetTimeToNextEnrollmentRequest() const { + // Enrollment already requested. if (IsWaitingForEnrollmentResult()) - return base::TimeDelta::FromMilliseconds(0); + return kZeroTimeDelta; + + if (!pending_enrollment_request_) + return base::TimeDelta::Max(); + + // Attempt the pending enrollment request immediately unless it is periodic. + if (pending_enrollment_request_->retry_count() == 0) { + if (pending_enrollment_request_->invocation_reason() != + cryptauthv2::ClientMetadata::PERIODIC) { + return kZeroTimeDelta; + } + + base::Optional last_successful_enrollment_time = + GetLastSuccessfulEnrollmentTime(); + if (!last_successful_enrollment_time) + return kZeroTimeDelta; + + base::TimeDelta time_since_last_success = + clock_->Now() - *last_successful_enrollment_time; + return std::max(kZeroTimeDelta, + GetRefreshPeriod() - time_since_last_success); + } - return timer_->GetCurrentDelay(); + base::TimeDelta time_since_last_attempt = + clock_->Now() - pref_service_->GetTime( + prefs::kCryptAuthSchedulerLastEnrollmentAttemptTime); + + // Recover from failure using immediate retry. + if (pending_enrollment_request_->retry_count() <= + client_directive_.retry_attempts()) { + return std::max(kZeroTimeDelta, + kImmediateRetryDelay - time_since_last_attempt); + } + + // Recover from failure after expending allotted immediate retries. + return std::max(kZeroTimeDelta, base::TimeDelta::FromMilliseconds( + client_directive_.retry_period_millis()) - + time_since_last_attempt); } bool CryptAuthSchedulerImpl::IsWaitingForEnrollmentResult() const { - return !timer_->IsRunning(); + return current_enrollment_request_.has_value(); } -size_t CryptAuthSchedulerImpl::GetNumConsecutiveFailures() const { - return pref_service_->GetUint64( - prefs::kCryptAuthEnrollmentSchedulerNumConsecutiveFailures); +size_t CryptAuthSchedulerImpl::GetNumConsecutiveEnrollmentFailures() const { + if (current_enrollment_request_) + return current_enrollment_request_->retry_count(); + + if (pending_enrollment_request_) + return pending_enrollment_request_->retry_count(); + + return 0; } -base::TimeDelta CryptAuthSchedulerImpl::CalculateTimeBetweenEnrollmentRequests() - const { - size_t num_consecutive_failures = GetNumConsecutiveFailures(); - if (num_consecutive_failures == 0) - return GetRefreshPeriod(); +void CryptAuthSchedulerImpl::DefaultNetworkChanged( + const NetworkState* network) { + // The updated default network may not be online. + if (!DoesMachineHaveNetworkConnectivity()) + return; - if (num_consecutive_failures <= (size_t)client_directive_.retry_attempts()) - return kImmediateRetryDelay; + // Now that the device has connectivity, reschedule enrollment. + ScheduleNextEnrollment(); +} - return base::TimeDelta::FromMilliseconds( - client_directive_.retry_period_millis()); +void CryptAuthSchedulerImpl::OnShuttingDown() { + DCHECK(network_state_handler_); + network_state_handler_->RemoveObserver(this, FROM_HERE); + network_state_handler_ = nullptr; +} + +bool CryptAuthSchedulerImpl::DoesMachineHaveNetworkConnectivity() { + if (!network_state_handler_) + return false; + + // TODO(khorimoto): IsConnectedState() can still return true if connected to + // a captive portal; use the "online" boolean once we fetch data via the + // networking Mojo API. See https://crbug.com/862420. + const NetworkState* default_network = + network_state_handler_->DefaultNetwork(); + return default_network && default_network->IsConnectedState(); } void CryptAuthSchedulerImpl::ScheduleNextEnrollment() { - DCHECK(!timer_->IsRunning()); - - base::Time last_attempt_time = pref_service_->GetTime( - prefs::kCryptAuthEnrollmentSchedulerLastEnrollmentAttemptTime); - - base::TimeDelta time_until_next_request = - base::TimeDelta::FromMilliseconds(0); - if (!last_attempt_time.is_null()) { - time_until_next_request = - std::max(base::TimeDelta::FromMilliseconds(0), - CalculateTimeBetweenEnrollmentRequests() - - (clock_->Now() - last_attempt_time)); + // Wait for the current enrollment attempt to finish before determining the + // next enrollment request in case we need to recover from a failure. + if (IsWaitingForEnrollmentResult()) + return; + + // If no enrollment request has been explicitly made, schedule the standard + // periodic enrollment attempt. + if (!pending_enrollment_request_) { + pending_enrollment_request_ = + BuildClientMetadata(0 /* retry_count */, + GetLastSuccessfulEnrollmentTime() + ? cryptauthv2::ClientMetadata::PERIODIC + : cryptauthv2::ClientMetadata::INITIALIZATION, + base::nullopt /* session_id */); } - timer_->Start( - FROM_HERE, time_until_next_request, - base::BindOnce(&CryptAuthSchedulerImpl::NotifyEnrollmentRequested, - base::Unretained(this), GetPolicyReference())); + DCHECK(pending_enrollment_request_); + pref_service_->SetString( + prefs::kCryptAuthSchedulerNextEnrollmentRequestClientMetadata, + ClientMetadataToPrefString(*pending_enrollment_request_)); + + if (!HasEnrollmentSchedulingStarted()) + return; + + enrollment_timer_->Start( + FROM_HERE, GetTimeToNextEnrollmentRequest(), + base::BindOnce(&CryptAuthSchedulerImpl::OnEnrollmentTimerFired, + base::Unretained(this))); } -base::Optional -CryptAuthSchedulerImpl::GetPolicyReference() const { +void CryptAuthSchedulerImpl::OnEnrollmentTimerFired() { + DCHECK(!current_enrollment_request_); + DCHECK(pending_enrollment_request_); + + if (!DoesMachineHaveNetworkConnectivity()) { + PA_LOG(INFO) << "Enrollment triggered while the device is offline. Waiting " + << "for online connectivity before making request."; + return; + } + + current_enrollment_request_ = pending_enrollment_request_; + pending_enrollment_request_.reset(); + + base::Optional policy_reference = base::nullopt; if (client_directive_.has_policy_reference()) - return client_directive_.policy_reference(); + policy_reference = client_directive_.policy_reference(); - return base::nullopt; + NotifyEnrollmentRequested(*current_enrollment_request_, policy_reference); } } // namespace device_sync diff --git a/chromeos/services/device_sync/cryptauth_scheduler_impl.h b/chromeos/services/device_sync/cryptauth_scheduler_impl.h index dc15259fcf8780..f6aa22f988e1ec 100644 --- a/chromeos/services/device_sync/cryptauth_scheduler_impl.h +++ b/chromeos/services/device_sync/cryptauth_scheduler_impl.h @@ -6,14 +6,17 @@ #define CHROMEOS_SERVICES_DEVICE_SYNC_CRYPTAUTH_SCHEDULER_IMPL_H_ #include +#include #include "base/macros.h" #include "base/memory/ref_counted.h" -#include "base/memory/weak_ptr.h" -#include "base/threading/thread_task_runner_handle.h" +#include "base/optional.h" #include "base/time/default_clock.h" #include "base/timer/timer.h" +#include "chromeos/network/network_handler.h" +#include "chromeos/network/network_state_handler_observer.h" #include "chromeos/services/device_sync/cryptauth_scheduler.h" +#include "chromeos/services/device_sync/proto/cryptauth_common.pb.h" #include "chromeos/services/device_sync/proto/cryptauth_directive.pb.h" class PrefRegistrySimple; @@ -21,15 +24,29 @@ class PrefService; namespace chromeos { +class NetworkStateHandler; + namespace device_sync { // CryptAuthScheduler implementation which stores scheduling metadata // persistently so that the enrollment schedule is saved across device reboots. -// If this class is instantiated before at least one enrollment has been -// completed successfully, it requests enrollment immediately. Once enrollment -// has been completed successfully, this class schedules the next enrollment -// attempt at a time provided by the server via a ClientDirective proto. -class CryptAuthSchedulerImpl : public CryptAuthScheduler { +// +// Enrollment scheduling will not start until StartEnrollmentScheduling() is +// called. Enrollment requests made before scheduling has started, while another +// enrollment request is in process, or while offline will be cached and +// rescheduled as soon as possible. +// +// Enrollment requests can come from four sources: +// 1) If an enrollment has never successfully completed, an initialization +// request will be made immediately. +// 2) Periodic enrollment requests are made at time intervals provided by the +// server in the ClientDirective proto. +// 3) Enrollment requests can be made using the scheduler's public function +// RequestEnrollment(). +// 4) Failed enrollment attempts will be retried at time intervals provided by +// the server in the ClientDirective proto. +class CryptAuthSchedulerImpl : public CryptAuthScheduler, + public NetworkStateHandlerObserver { public: class Factory { public: @@ -37,13 +54,12 @@ class CryptAuthSchedulerImpl : public CryptAuthScheduler { static void SetFactoryForTesting(Factory* test_factory); virtual ~Factory(); virtual std::unique_ptr BuildInstance( - Delegate* delegate, PrefService* pref_service, + NetworkStateHandler* network_state_handler = + NetworkHandler::Get()->network_state_handler(), base::Clock* clock = base::DefaultClock::GetInstance(), - std::unique_ptr timer = - std::make_unique(), - scoped_refptr task_runner = - base::ThreadTaskRunnerHandle::Get()); + std::unique_ptr enrollment_timer = + std::make_unique()); private: static Factory* test_factory_; @@ -55,39 +71,47 @@ class CryptAuthSchedulerImpl : public CryptAuthScheduler { ~CryptAuthSchedulerImpl() override; private: - CryptAuthSchedulerImpl(Delegate* delegate, - PrefService* pref_service, + CryptAuthSchedulerImpl(PrefService* pref_service, + NetworkStateHandler* network_state_handler, base::Clock* clock, - std::unique_ptr timer, - scoped_refptr task_runner); + std::unique_ptr enrollment_timer); // CryptAuthScheduler: - void RequestEnrollmentNow() override; + void OnEnrollmentSchedulingStarted() override; + void RequestEnrollment( + const cryptauthv2::ClientMetadata::InvocationReason& invocation_reason, + const base::Optional& session_id) override; void HandleEnrollmentResult( const CryptAuthEnrollmentResult& enrollment_result) override; base::Optional GetLastSuccessfulEnrollmentTime() const override; base::TimeDelta GetRefreshPeriod() const override; base::TimeDelta GetTimeToNextEnrollmentRequest() const override; bool IsWaitingForEnrollmentResult() const override; - size_t GetNumConsecutiveFailures() const override; + size_t GetNumConsecutiveEnrollmentFailures() const override; + + // NetworkStateHandlerObserver: + void DefaultNetworkChanged(const NetworkState* network) override; + void OnShuttingDown() override; - // Calculates the time period between the previous enrollment attempt and the - // next enrollment attempt, taking failures into consideration. - base::TimeDelta CalculateTimeBetweenEnrollmentRequests() const; + bool DoesMachineHaveNetworkConnectivity(); // Starts a new timer that will fire when an enrollment is ready to be // attempted. void ScheduleNextEnrollment(); - // Get the ClientDirective's PolicyReference. If one has not been set, returns - // base::nullopt. - base::Optional GetPolicyReference() const; + void OnEnrollmentTimerFired(); + + base::Optional pending_enrollment_request_; + + // Only non-null while an enrollment attempt is in progress, in other words, + // between NotifyEnrollmentRequested() and HandleEnrollmentResult(). + base::Optional current_enrollment_request_; - PrefService* pref_service_; - base::Clock* clock_; - std::unique_ptr timer_; + PrefService* pref_service_ = nullptr; + NetworkStateHandler* network_state_handler_ = nullptr; + base::Clock* clock_ = nullptr; + std::unique_ptr enrollment_timer_ = nullptr; cryptauthv2::ClientDirective client_directive_; - base::WeakPtrFactory weak_ptr_factory_; DISALLOW_COPY_AND_ASSIGN(CryptAuthSchedulerImpl); }; diff --git a/chromeos/services/device_sync/cryptauth_scheduler_impl_unittest.cc b/chromeos/services/device_sync/cryptauth_scheduler_impl_unittest.cc index 5559ebe5d236fd..aa6e112e8b50cf 100644 --- a/chromeos/services/device_sync/cryptauth_scheduler_impl_unittest.cc +++ b/chromeos/services/device_sync/cryptauth_scheduler_impl_unittest.cc @@ -12,13 +12,16 @@ #include "base/base64.h" #include "base/macros.h" #include "base/memory/ptr_util.h" +#include "base/test/scoped_task_environment.h" #include "base/test/simple_test_clock.h" -#include "base/test/test_simple_task_runner.h" #include "base/timer/mock_timer.h" +#include "chromeos/network/network_state_test_helper.h" #include "chromeos/services/device_sync/fake_cryptauth_scheduler.h" #include "chromeos/services/device_sync/pref_names.h" +#include "chromeos/services/device_sync/proto/cryptauth_v2_test_util.h" #include "components/prefs/testing_pref_service.h" #include "testing/gtest/include/gtest/gtest.h" +#include "third_party/cros_system_api/dbus/shill/dbus-constants.h" namespace chromeos { @@ -26,25 +29,17 @@ namespace device_sync { namespace { -const char kFakePolicyName[] = "fake-policy-name"; -int kFakePolicyVersion = 100; -constexpr base::TimeDelta kFakeRefreshPeriod = base::TimeDelta::FromDays(100); -constexpr base::TimeDelta kFakeRetryPeriod = base::TimeDelta::FromHours(100); -constexpr base::TimeDelta kFakeImmediateRetryDelay = - base::TimeDelta::FromMinutes(5); -const int kFakeMaxImmediateRetries = 2; +enum class NetworkConnectionStatus { kDisconnected, kConnecting, kConnected }; -// The time set on the scheduler's clock during set-up. -const base::Time kFakeTimeNow = base::Time::FromDoubleT(1600600000); -const base::Time kFakeTimeLaterBeforeRetryPeriod = - kFakeTimeNow + kFakeRetryPeriod - base::TimeDelta::FromHours(1); -const base::Time kFakeTimeLaterAfterRetryPeriod = - kFakeTimeNow + kFakeRefreshPeriod + base::TimeDelta::FromHours(1); -const base::Time kFakeTimeLaterAfterRefreshPeriod = - kFakeTimeNow + kFakeRefreshPeriod + base::TimeDelta::FromDays(1); +constexpr base::TimeDelta kZeroTimeDelta = base::TimeDelta::FromSeconds(0); +constexpr base::TimeDelta kImmediateRetryDelay = + base::TimeDelta::FromMinutes(5); +const char kWifiServiceGuid[] = "wifiGuid"; +const char kSessionId[] = "sessionId"; // Serializes and base64 encodes the input ClientDirective. // Copied from cryptauth_enrollment_scheduler_impl.cc. +// TODO(https://crbug.com/964563): Replace when utility functions are added. std::string ClientDirectiveToPrefString( const cryptauthv2::ClientDirective& client_directive) { std::string encoded_serialized_client_directive; @@ -54,21 +49,21 @@ std::string ClientDirectiveToPrefString( return encoded_serialized_client_directive; } +// TODO(https://crbug.com/964563): Replace when utility functions are added. +std::string ClientMetadataToPrefString( + const cryptauthv2::ClientMetadata& client_metadata) { + std::string encoded_serialized_client_metadata; + base::Base64Encode(client_metadata.SerializeAsString(), + &encoded_serialized_client_metadata); + + return encoded_serialized_client_metadata; +} + } // namespace class DeviceSyncCryptAuthSchedulerImplTest : public testing::Test { protected: - DeviceSyncCryptAuthSchedulerImplTest() { - fake_client_directive_.mutable_policy_reference()->set_name( - kFakePolicyName); - fake_client_directive_.mutable_policy_reference()->set_version( - kFakePolicyVersion); - fake_client_directive_.set_checkin_delay_millis( - kFakeRefreshPeriod.InMilliseconds()); - fake_client_directive_.set_retry_period_millis( - kFakeRetryPeriod.InMilliseconds()); - fake_client_directive_.set_retry_attempts(kFakeMaxImmediateRetries); - } + DeviceSyncCryptAuthSchedulerImplTest() = default; ~DeviceSyncCryptAuthSchedulerImplTest() override = default; @@ -76,275 +71,567 @@ class DeviceSyncCryptAuthSchedulerImplTest : public testing::Test { CryptAuthSchedulerImpl::RegisterPrefs(pref_service_.registry()); } - void CreateScheduler() { - if (scheduler_) - return; + void CreateScheduler( + const base::Optional& + persisted_client_directive, + const base::Optional& + persisted_client_metadata, + const base::Optional& persisted_last_enrollment_attempt_time, + const base::Optional& + persisted_last_successful_enrollment_time) { + if (persisted_client_directive) { + pref_service_.SetString( + prefs::kCryptAuthSchedulerClientDirective, + ClientDirectiveToPrefString(*persisted_client_directive)); + } + + if (persisted_client_metadata) { + pref_service_.SetString( + prefs::kCryptAuthSchedulerNextEnrollmentRequestClientMetadata, + ClientMetadataToPrefString(*persisted_client_metadata)); + } - auto mock_timer = std::make_unique(); - mock_timer_ = mock_timer.get(); + if (persisted_last_enrollment_attempt_time) { + pref_service_.SetTime(prefs::kCryptAuthSchedulerLastEnrollmentAttemptTime, + *persisted_last_enrollment_attempt_time); + } + + if (persisted_last_successful_enrollment_time) { + pref_service_.SetTime( + prefs::kCryptAuthSchedulerLastSuccessfulEnrollmentTime, + *persisted_last_successful_enrollment_time); + } - auto test_task_runner = base::MakeRefCounted(); + EXPECT_TRUE(!scheduler_); + + auto mock_enrollment_timer = std::make_unique(); + mock_enrollment_timer_ = mock_enrollment_timer.get(); scheduler_ = CryptAuthSchedulerImpl::Factory::Get()->BuildInstance( - &fake_delegate_, &pref_service_, &test_clock_, std::move(mock_timer), - test_task_runner); + &pref_service_, network_helper_.network_state_handler(), &test_clock_, + std::move(mock_enrollment_timer)); + + VerifyLastEnrollmentAttemptTime(persisted_last_enrollment_attempt_time); + VerifyLastSuccessfulEnrollmentTime( + persisted_last_successful_enrollment_time); + } + + void AddDisconnectedWifiNetwork() { + EXPECT_TRUE(wifi_network_service_path_.empty()); + + std::stringstream ss; + ss << "{" + << " \"GUID\": \"" << kWifiServiceGuid << "\"," + << " \"Type\": \"" << shill::kTypeWifi << "\"," + << " \"State\": \"" << shill::kStateIdle << "\"" + << "}"; + + wifi_network_service_path_ = network_helper_.ConfigureService(ss.str()); + } + + void SetWifiNetworkStatus(NetworkConnectionStatus connection_status) { + std::string shill_connection_status; + switch (connection_status) { + case NetworkConnectionStatus::kDisconnected: + shill_connection_status = shill::kStateIdle; + break; + case NetworkConnectionStatus::kConnecting: + shill_connection_status = shill::kStateAssociation; + break; + case NetworkConnectionStatus::kConnected: + shill_connection_status = shill::kStateReady; + break; + } + + network_helper_.SetServiceProperty(wifi_network_service_path_, + shill::kStateProperty, + base::Value(shill_connection_status)); + base::RunLoop().RunUntilIdle(); + } + + void RemoveWifiNetwork() { + EXPECT_TRUE(!wifi_network_service_path_.empty()); - test_task_runner->RunUntilIdle(); + network_helper_.service_test()->RemoveService(wifi_network_service_path_); + base::RunLoop().RunUntilIdle(); + + wifi_network_service_path_.clear(); } - void VerifyReceivedPolicyReference( - size_t num_expected_received_policy_references, - const base::Optional& - last_expected_received_policy_reference) { + void VerifyLastClientMetadataReceivedByEnrollmentDelegate( + size_t total_received, + const base::Optional& last_received = + base::nullopt) { + EXPECT_EQ(total_received, fake_enrollment_delegate_ + .client_metadata_from_enrollment_requests() + .size()); + + if (fake_enrollment_delegate_.client_metadata_from_enrollment_requests() + .empty()) + return; + + EXPECT_TRUE(last_received); EXPECT_EQ( - num_expected_received_policy_references, - fake_delegate_.policy_references_from_enrollment_requests().size()); + last_received->SerializeAsString(), + fake_enrollment_delegate_.client_metadata_from_enrollment_requests() + .back() + .SerializeAsString()); + } + + void VerifyLastPolicyReferenceReceivedByEnrollmentDelegate( + size_t total_received, + const base::Optional& last_received = + base::nullopt) { + EXPECT_EQ(total_received, fake_enrollment_delegate_ + .policy_references_from_enrollment_requests() + .size()); - if (fake_delegate_.policy_references_from_enrollment_requests().empty()) + if (fake_enrollment_delegate_.policy_references_from_enrollment_requests() + .empty()) return; - EXPECT_EQ(last_expected_received_policy_reference.has_value(), - fake_delegate_.policy_references_from_enrollment_requests() - .back() - .has_value()); + EXPECT_EQ( + last_received.has_value(), + fake_enrollment_delegate_.policy_references_from_enrollment_requests() + .back() + .has_value()); - if (fake_delegate_.policy_references_from_enrollment_requests() + if (fake_enrollment_delegate_.policy_references_from_enrollment_requests() .back() .has_value() && - last_expected_received_policy_reference.has_value()) { - EXPECT_EQ(last_expected_received_policy_reference->SerializeAsString(), - fake_delegate_.policy_references_from_enrollment_requests() - .back() - ->SerializeAsString()); + last_received.has_value()) { + EXPECT_EQ( + last_received->SerializeAsString(), + fake_enrollment_delegate_.policy_references_from_enrollment_requests() + .back() + ->SerializeAsString()); } } - void VerifyLastEnrollmentAttemptTimePref(const base::Time& expected_time) { + void VerifyLastSuccessfulEnrollmentTime( + const base::Optional& expected_time) { + EXPECT_EQ(expected_time, scheduler_->GetLastSuccessfulEnrollmentTime()); + + EXPECT_EQ(pref_service_.GetTime( + prefs::kCryptAuthSchedulerLastSuccessfulEnrollmentTime), + expected_time.value_or(base::Time())); + } + + void VerifyLastEnrollmentAttemptTime( + const base::Optional& expected_time) { + EXPECT_EQ(pref_service_.GetTime( + prefs::kCryptAuthSchedulerLastEnrollmentAttemptTime), + expected_time.value_or(base::Time())); + } + + void VerifyClientDirective( + const cryptauthv2::ClientDirective& expected_client_directive) { EXPECT_EQ( - pref_service_.GetTime( - prefs::kCryptAuthEnrollmentSchedulerLastEnrollmentAttemptTime), - expected_time); + ClientDirectiveToPrefString(expected_client_directive), + pref_service_.GetString(prefs::kCryptAuthSchedulerClientDirective)); + EXPECT_EQ(base::TimeDelta::FromMilliseconds( + expected_client_directive.checkin_delay_millis()), + scheduler()->GetRefreshPeriod()); } - FakeCryptAuthSchedulerDelegate* delegate() { return &fake_delegate_; } + void VerifyScheduledEnrollment( + const cryptauthv2::ClientMetadata& expected_scheduled_enrollment_request, + const base::TimeDelta& expected_delay) { + VerifyNextEnrollmentRequest(expected_scheduled_enrollment_request); + EXPECT_TRUE(mock_enrollment_timer_->IsRunning()); + EXPECT_EQ(expected_delay, mock_enrollment_timer_->GetCurrentDelay()); + EXPECT_EQ(expected_delay, scheduler_->GetTimeToNextEnrollmentRequest()); + EXPECT_FALSE(scheduler()->IsWaitingForEnrollmentResult()); + } + + void VerifyNoEnrollmentsTriggeredButRequestQueued( + const cryptauthv2::ClientMetadata& expected_enrollment_request) { + VerifyNextEnrollmentRequest(expected_enrollment_request); + EXPECT_FALSE(enrollment_timer()->IsRunning()); + EXPECT_FALSE(scheduler()->IsWaitingForEnrollmentResult()); + VerifyLastPolicyReferenceReceivedByEnrollmentDelegate( + 0 /* total_received */); + VerifyLastClientMetadataReceivedByEnrollmentDelegate( + 0 /* total_received */); + } - PrefService* pref_service() { return &pref_service_; } + base::WeakPtr + fake_enrollment_delegate() { + return fake_enrollment_delegate_.GetWeakPtr(); + } base::SimpleTestClock* clock() { return &test_clock_; } - base::MockOneShotTimer* timer() { return mock_timer_; } + base::MockOneShotTimer* enrollment_timer() { return mock_enrollment_timer_; } CryptAuthScheduler* scheduler() { return scheduler_.get(); } - const cryptauthv2::ClientDirective& fake_client_directive() { - return fake_client_directive_; + private: + void VerifyNextEnrollmentRequest( + const cryptauthv2::ClientMetadata& expected_enrollment_request) { + EXPECT_EQ( + ClientMetadataToPrefString(expected_enrollment_request), + pref_service_.GetString( + prefs::kCryptAuthSchedulerNextEnrollmentRequestClientMetadata)); } - private: - FakeCryptAuthSchedulerDelegate fake_delegate_; + base::test::ScopedTaskEnvironment scoped_task_environment_; + FakeCryptAuthSchedulerEnrollmentDelegate fake_enrollment_delegate_; TestingPrefServiceSimple pref_service_; base::SimpleTestClock test_clock_; - base::MockOneShotTimer* mock_timer_; - - cryptauthv2::ClientDirective fake_client_directive_; - + base::MockOneShotTimer* mock_enrollment_timer_; + NetworkStateTestHelper network_helper_{ + false /* use_default_devices_and_services */}; + std::string wifi_network_service_path_; std::unique_ptr scheduler_; DISALLOW_COPY_AND_ASSIGN(DeviceSyncCryptAuthSchedulerImplTest); }; -TEST_F(DeviceSyncCryptAuthSchedulerImplTest, HandleSuccessfulEnrollmentResult) { - clock()->SetNow(kFakeTimeNow); - CreateScheduler(); +TEST_F(DeviceSyncCryptAuthSchedulerImplTest, + SuccessfulInitializationAndPeriodicEnrollments) { + AddDisconnectedWifiNetwork(); + SetWifiNetworkStatus(NetworkConnectionStatus::kConnected); + + const base::Time kStartTime = base::Time::FromDoubleT(1600600000); + const base::Time kInitializationFinishTime = + kStartTime + base::TimeDelta::FromSeconds(5); + + clock()->SetNow(kStartTime); + + CreateScheduler(base::nullopt /* persisted_client_directive */, + base::nullopt /* persisted_client_metadata */, + base::nullopt /* persisted_last_enrollment_attempt_time */, + base::nullopt /* persisted_last_successful_enrollment_time */ + ); - EXPECT_TRUE(timer()->IsRunning()); - EXPECT_FALSE(scheduler()->GetLastSuccessfulEnrollmentTime()); - EXPECT_TRUE(scheduler()->GetRefreshPeriod() > - base::TimeDelta::FromMilliseconds(0)); - EXPECT_EQ(base::TimeDelta::FromMilliseconds(0), + // No enrollment has been scheduled yet. + EXPECT_FALSE(enrollment_timer()->IsRunning()); + EXPECT_EQ(base::TimeDelta::Max(), scheduler()->GetTimeToNextEnrollmentRequest()); - EXPECT_FALSE(scheduler()->IsWaitingForEnrollmentResult()); - EXPECT_EQ(0u, scheduler()->GetNumConsecutiveFailures()); - timer()->Fire(); + EXPECT_FALSE(scheduler()->HasEnrollmentSchedulingStarted()); + scheduler()->StartEnrollmentScheduling(fake_enrollment_delegate()); + EXPECT_TRUE(scheduler()->HasEnrollmentSchedulingStarted()); + // No successful enrollment has ever occurred; attempt immediately. + cryptauthv2::ClientMetadata expected_scheduled_enrollment_request = + cryptauthv2::BuildClientMetadata( + 0 /* retry_count */, cryptauthv2::ClientMetadata::INITIALIZATION, + base::nullopt /* session_id */); + VerifyScheduledEnrollment(expected_scheduled_enrollment_request, + kZeroTimeDelta /* expected_delay */); + + enrollment_timer()->Fire(); EXPECT_TRUE(scheduler()->IsWaitingForEnrollmentResult()); - VerifyReceivedPolicyReference(1, base::nullopt); - CryptAuthEnrollmentResult result( + // There is no policy reference until CryptAuth sends one with a + // ClientDirective. + VerifyLastPolicyReferenceReceivedByEnrollmentDelegate( + 1 /* total_received */, base::nullopt /* last_received*/); + VerifyLastClientMetadataReceivedByEnrollmentDelegate( + 1 /* total_received */, expected_scheduled_enrollment_request); + + clock()->SetNow(kInitializationFinishTime); + scheduler()->HandleEnrollmentResult(CryptAuthEnrollmentResult( CryptAuthEnrollmentResult::ResultCode::kSuccessNewKeysEnrolled, - fake_client_directive()); - scheduler()->HandleEnrollmentResult(result); - - EXPECT_TRUE(timer()->IsRunning()); - ASSERT_TRUE(scheduler()->GetLastSuccessfulEnrollmentTime()); - EXPECT_EQ(kFakeTimeNow, scheduler()->GetLastSuccessfulEnrollmentTime()); - EXPECT_EQ(base::TimeDelta::FromMilliseconds( - fake_client_directive().checkin_delay_millis()), - scheduler()->GetRefreshPeriod()); - EXPECT_EQ(scheduler()->GetRefreshPeriod(), - scheduler()->GetTimeToNextEnrollmentRequest()); - EXPECT_FALSE(scheduler()->IsWaitingForEnrollmentResult()); - EXPECT_EQ(0u, scheduler()->GetNumConsecutiveFailures()); - VerifyLastEnrollmentAttemptTimePref(kFakeTimeNow); + cryptauthv2::GetClientDirectiveForTest())); + VerifyLastEnrollmentAttemptTime(kInitializationFinishTime); + VerifyLastSuccessfulEnrollmentTime(kInitializationFinishTime); + VerifyClientDirective(cryptauthv2::GetClientDirectiveForTest()); + + // A periodic enrollment attempt is now scheduled. + expected_scheduled_enrollment_request = cryptauthv2::BuildClientMetadata( + 0 /* retry_count */, cryptauthv2::ClientMetadata::PERIODIC, + base::nullopt /* session_id */); + VerifyScheduledEnrollment( + expected_scheduled_enrollment_request, + scheduler()->GetRefreshPeriod() /* expected_delay */); + + base::Time periodic_fired_time = + kInitializationFinishTime + scheduler()->GetRefreshPeriod(); + clock()->SetNow(periodic_fired_time); + enrollment_timer()->Fire(); + + VerifyLastPolicyReferenceReceivedByEnrollmentDelegate( + 2 /* total_received */, + cryptauthv2::GetClientDirectiveForTest().policy_reference()); + VerifyLastClientMetadataReceivedByEnrollmentDelegate( + 2 /* total_received */, expected_scheduled_enrollment_request); + + // Assume no new ClientDirective was received from CryptAuth this time; + // scheduler continues to use last-known ClientDirective. + scheduler()->HandleEnrollmentResult(CryptAuthEnrollmentResult( + CryptAuthEnrollmentResult::ResultCode::kSuccessNoNewKeysNeeded, + base::nullopt /* client_directive */)); + VerifyLastEnrollmentAttemptTime(periodic_fired_time); + VerifyLastSuccessfulEnrollmentTime(periodic_fired_time); + VerifyClientDirective(cryptauthv2::GetClientDirectiveForTest()); +} - timer()->Fire(); +TEST_F(DeviceSyncCryptAuthSchedulerImplTest, FailedEnrollments) { + AddDisconnectedWifiNetwork(); + SetWifiNetworkStatus(NetworkConnectionStatus::kConnected); + + CreateScheduler( + cryptauthv2::GetClientDirectiveForTest() /* persisted_client_directive */, + base::nullopt /* persisted_client_metadata */, + base::nullopt /* persisted_last_enrollment_attempt_time */, + base::nullopt /* persisted_last_successful_enrollment_time */ + ); + + scheduler()->StartEnrollmentScheduling(fake_enrollment_delegate()); + + cryptauthv2::ClientMetadata expected_enrollment_request = + cryptauthv2::BuildClientMetadata( + 0 /* retry_count */, cryptauthv2::ClientMetadata::INITIALIZATION, + base::nullopt /* session_id */); + VerifyScheduledEnrollment(expected_enrollment_request, + kZeroTimeDelta /* expected_delay */); + + // After using all immediate failure retry attempts allotted by the client + // directive, schedule retries using client directive's retry_period_millis. + for (int attempt = 1; + attempt <= cryptauthv2::GetClientDirectiveForTest().retry_attempts() + 3; + ++attempt) { + enrollment_timer()->Fire(); + VerifyLastPolicyReferenceReceivedByEnrollmentDelegate( + attempt /* total_received */, + cryptauthv2::GetClientDirectiveForTest().policy_reference()); + VerifyLastClientMetadataReceivedByEnrollmentDelegate( + attempt /* total_received */, expected_enrollment_request); + + scheduler()->HandleEnrollmentResult(CryptAuthEnrollmentResult( + CryptAuthEnrollmentResult::ResultCode::kErrorCryptAuthServerOverloaded, + base::nullopt /* client_directive */)); + + expected_enrollment_request.set_retry_count(attempt); + base::TimeDelta expected_delay = + attempt <= cryptauthv2::GetClientDirectiveForTest().retry_attempts() + ? kImmediateRetryDelay + : base::TimeDelta::FromMilliseconds( + cryptauthv2::GetClientDirectiveForTest() + .retry_period_millis()); + VerifyScheduledEnrollment(expected_enrollment_request, expected_delay); + } +} - VerifyReceivedPolicyReference(2, fake_client_directive().policy_reference()); +TEST_F(DeviceSyncCryptAuthSchedulerImplTest, + EnrollmentRequestNotScheduledUntilSchedulerStarted) { + AddDisconnectedWifiNetwork(); + SetWifiNetworkStatus(NetworkConnectionStatus::kConnected); + + CreateScheduler(base::nullopt /* persisted_client_directive */, + base::nullopt /* persisted_client_metadata */, + base::nullopt /* persisted_last_enrollment_attempt_time */, + base::nullopt /* persisted_last_successful_enrollment_time */ + ); + + scheduler()->RequestEnrollment(cryptauthv2::ClientMetadata::MANUAL, + kSessionId); + cryptauthv2::ClientMetadata expected_enrollment_request = + cryptauthv2::BuildClientMetadata( + 0 /* retry_count */, cryptauthv2::ClientMetadata::MANUAL, kSessionId); + VerifyNoEnrollmentsTriggeredButRequestQueued(expected_enrollment_request); + + scheduler()->StartEnrollmentScheduling(fake_enrollment_delegate()); + VerifyScheduledEnrollment(expected_enrollment_request, + kZeroTimeDelta /* expected_delay */); } TEST_F(DeviceSyncCryptAuthSchedulerImplTest, - NotDueForRefresh_RequestImmediateEnrollment) { - pref_service()->SetString( - prefs::kCryptAuthEnrollmentSchedulerClientDirective, - ClientDirectiveToPrefString(fake_client_directive())); - pref_service()->SetTime( - prefs::kCryptAuthEnrollmentSchedulerLastEnrollmentAttemptTime, - kFakeTimeNow); - pref_service()->SetTime( - prefs::kCryptAuthEnrollmentSchedulerLastSuccessfulEnrollmentTime, - kFakeTimeNow); - pref_service()->SetUint64( - prefs::kCryptAuthEnrollmentSchedulerNumConsecutiveFailures, 0); - - clock()->SetNow(kFakeTimeLaterBeforeRetryPeriod); - base::TimeDelta fake_elapsed_time = - kFakeTimeLaterBeforeRetryPeriod - kFakeTimeNow; - - CreateScheduler(); - - ASSERT_TRUE(scheduler()->GetLastSuccessfulEnrollmentTime()); - EXPECT_EQ(kFakeTimeNow, scheduler()->GetLastSuccessfulEnrollmentTime()); - EXPECT_EQ(base::TimeDelta::FromMilliseconds( - fake_client_directive().checkin_delay_millis()), - scheduler()->GetRefreshPeriod()); - EXPECT_EQ(scheduler()->GetRefreshPeriod() - fake_elapsed_time, - scheduler()->GetTimeToNextEnrollmentRequest()); - EXPECT_FALSE(scheduler()->IsWaitingForEnrollmentResult()); + EnrollmentRequestScheduledAfterCurrentEnrollmentFinishes) { + AddDisconnectedWifiNetwork(); + SetWifiNetworkStatus(NetworkConnectionStatus::kConnected); - scheduler()->RequestEnrollmentNow(); - EXPECT_TRUE(scheduler()->IsWaitingForEnrollmentResult()); - EXPECT_EQ(1u, - delegate()->policy_references_from_enrollment_requests().size()); -} + CreateScheduler(base::nullopt /* persisted_client_directive */, + base::nullopt /* persisted_client_metadata */, + base::nullopt /* persisted_last_enrollment_attempt_time */, + base::nullopt /* persisted_last_successful_enrollment_time */ + ); -TEST_F(DeviceSyncCryptAuthSchedulerImplTest, DueForRefreshBeforeConstructed) { - pref_service()->SetString( - prefs::kCryptAuthEnrollmentSchedulerClientDirective, - ClientDirectiveToPrefString(fake_client_directive())); - pref_service()->SetTime( - prefs::kCryptAuthEnrollmentSchedulerLastEnrollmentAttemptTime, - kFakeTimeNow); - pref_service()->SetTime( - prefs::kCryptAuthEnrollmentSchedulerLastSuccessfulEnrollmentTime, - kFakeTimeNow); - pref_service()->SetUint64( - prefs::kCryptAuthEnrollmentSchedulerNumConsecutiveFailures, 0); + scheduler()->StartEnrollmentScheduling(fake_enrollment_delegate()); - clock()->SetNow(kFakeTimeLaterAfterRefreshPeriod); + // Start a server-initiated enrollment attempt. + scheduler()->RequestEnrollment(cryptauthv2::ClientMetadata::SERVER_INITIATED, + kSessionId); + enrollment_timer()->Fire(); - CreateScheduler(); + // Request an enrollment while an enrollment attempt is in progress. + scheduler()->RequestEnrollment(cryptauthv2::ClientMetadata::MANUAL, + base::nullopt /* session_id */); + EXPECT_FALSE(enrollment_timer()->IsRunning()); - EXPECT_EQ(base::TimeDelta::FromMilliseconds(0), - scheduler()->GetTimeToNextEnrollmentRequest()); + scheduler()->HandleEnrollmentResult(CryptAuthEnrollmentResult( + CryptAuthEnrollmentResult::ResultCode::kErrorCryptAuthServerOverloaded, + cryptauthv2::GetClientDirectiveForTest())); + + // Pending request scheduled after current enrollment attempt finishes, even + // if it fails. + VerifyScheduledEnrollment( + cryptauthv2::BuildClientMetadata(0 /* retry_count */, + cryptauthv2::ClientMetadata::MANUAL, + base::nullopt /* session_id */), + kZeroTimeDelta /* expected_delay */); } -TEST_F(DeviceSyncCryptAuthSchedulerImplTest, HandleFailures) { - clock()->SetNow(kFakeTimeNow); - CreateScheduler(); - - // Expect all failed attempts to be retried immediately until the retry - // attempt limit from the ClientDirective is exceeded. - size_t expected_failure_count = 0; - while (expected_failure_count < - static_cast(fake_client_directive().retry_attempts())) { - timer()->Fire(); - CryptAuthEnrollmentResult result( - CryptAuthEnrollmentResult::ResultCode::kErrorCryptAuthServerOverloaded, - fake_client_directive()); - scheduler()->HandleEnrollmentResult(result); - ++expected_failure_count; +TEST_F(DeviceSyncCryptAuthSchedulerImplTest, + ScheduledEnrollmentRequestOverwritten) { + AddDisconnectedWifiNetwork(); + SetWifiNetworkStatus(NetworkConnectionStatus::kConnected); + + CreateScheduler(base::nullopt /* persisted_client_directive */, + base::nullopt /* persisted_client_metadata */, + base::nullopt /* persisted_last_enrollment_attempt_time */, + base::nullopt /* persisted_last_successful_enrollment_time */ + ); + + scheduler()->StartEnrollmentScheduling(fake_enrollment_delegate()); + scheduler()->RequestEnrollment(cryptauthv2::ClientMetadata::SERVER_INITIATED, + kSessionId); + VerifyScheduledEnrollment( + cryptauthv2::BuildClientMetadata( + 0 /* retry_count */, cryptauthv2::ClientMetadata::SERVER_INITIATED, + kSessionId), + kZeroTimeDelta /* expected_delay */); + + scheduler()->RequestEnrollment(cryptauthv2::ClientMetadata::MANUAL, + base::nullopt /* session_id */); + VerifyScheduledEnrollment( + cryptauthv2::BuildClientMetadata(0 /* retry_count */, + cryptauthv2::ClientMetadata::MANUAL, + base::nullopt /* session_id */), + kZeroTimeDelta /* expected_delay */); +} - EXPECT_EQ(expected_failure_count, scheduler()->GetNumConsecutiveFailures()); - ASSERT_FALSE(scheduler()->GetLastSuccessfulEnrollmentTime()); - EXPECT_EQ(base::TimeDelta::FromMilliseconds( - fake_client_directive().checkin_delay_millis()), - scheduler()->GetRefreshPeriod()); - EXPECT_EQ(kFakeImmediateRetryDelay, - scheduler()->GetTimeToNextEnrollmentRequest()); - VerifyLastEnrollmentAttemptTimePref(kFakeTimeNow); - } +TEST_F(DeviceSyncCryptAuthSchedulerImplTest, + ScheduleFailureRecoveryEnrollmentRequestOnStartUp) { + AddDisconnectedWifiNetwork(); + SetWifiNetworkStatus(NetworkConnectionStatus::kConnected); + + const base::Time kLastEnrollmentTime = base::Time::FromDoubleT(1600600000); + const base::Time kLastEnrollmentAttemptTime = + kLastEnrollmentTime + base::TimeDelta::FromDays(30); + const base::Time kStartTime = + kLastEnrollmentAttemptTime + (kImmediateRetryDelay / 2); + + clock()->SetNow(kStartTime); + + cryptauthv2::ClientMetadata persisted_enrollment_request = + cryptauthv2::BuildClientMetadata(5 /* retry_count */, + cryptauthv2::ClientMetadata::PERIODIC, + base::nullopt /* session_id */); + + CreateScheduler( + cryptauthv2::GetClientDirectiveForTest() /* persisted_client_directive */, + persisted_enrollment_request /* persisted_client_metadata */, + kLastEnrollmentAttemptTime /* persisted_last_enrollment_attempt_time */, + kLastEnrollmentTime /* persisted_last_successful_enrollment_time */ + ); + + scheduler()->StartEnrollmentScheduling(fake_enrollment_delegate()); + + // Retry count is reset to 1 on start-up so quick retry is triggered. + EXPECT_GT(cryptauthv2::GetClientDirectiveForTest().retry_attempts(), 0); + persisted_enrollment_request.set_retry_count(1); + VerifyScheduledEnrollment(persisted_enrollment_request, + kImmediateRetryDelay / 2 /* expected_delay */); +} - // Since all immediate retry attempts were expended above, expect a failed - // attempt to be retried after the ClientDirective's retry period. - timer()->Fire(); - CryptAuthEnrollmentResult result( - CryptAuthEnrollmentResult::ResultCode::kErrorCryptAuthServerOverloaded, - fake_client_directive()); - scheduler()->HandleEnrollmentResult(result); - - EXPECT_EQ(static_cast(fake_client_directive().retry_attempts() + 1), - scheduler()->GetNumConsecutiveFailures()); - EXPECT_TRUE(timer()->IsRunning()); - ASSERT_FALSE(scheduler()->GetLastSuccessfulEnrollmentTime()); - EXPECT_EQ(base::TimeDelta::FromMilliseconds( - fake_client_directive().checkin_delay_millis()), - scheduler()->GetRefreshPeriod()); - EXPECT_EQ(base::TimeDelta::FromMilliseconds( - fake_client_directive().retry_period_millis()), - scheduler()->GetTimeToNextEnrollmentRequest()); - VerifyLastEnrollmentAttemptTimePref(kFakeTimeNow); +TEST_F(DeviceSyncCryptAuthSchedulerImplTest, EnrollmentRequestedWhileOffline) { + AddDisconnectedWifiNetwork(); + SetWifiNetworkStatus(NetworkConnectionStatus::kDisconnected); - clock()->SetNow(kFakeTimeLaterAfterRetryPeriod); + CreateScheduler(base::nullopt /* persisted_client_directive */, + base::nullopt /* persisted_client_metadata */, + base::nullopt /* persisted_last_enrollment_attempt_time */, + base::nullopt /* persisted_last_successful_enrollment_time */ + ); - // Expect a successful attempt to reset the failure count and adjust the - // enrollment schedule to use the refresh period specified in the - // ClientDirective. - timer()->Fire(); - CryptAuthEnrollmentResult success_result( - CryptAuthEnrollmentResult::ResultCode::kSuccessNoNewKeysNeeded, - fake_client_directive()); - scheduler()->HandleEnrollmentResult(success_result); - - EXPECT_EQ(0u, scheduler()->GetNumConsecutiveFailures()); - EXPECT_TRUE(timer()->IsRunning()); - ASSERT_TRUE(scheduler()->GetLastSuccessfulEnrollmentTime()); - EXPECT_EQ(kFakeTimeLaterAfterRetryPeriod, - scheduler()->GetLastSuccessfulEnrollmentTime()); - EXPECT_EQ(base::TimeDelta::FromMilliseconds( - fake_client_directive().checkin_delay_millis()), - scheduler()->GetRefreshPeriod()); - EXPECT_EQ(scheduler()->GetRefreshPeriod(), - scheduler()->GetTimeToNextEnrollmentRequest()); - VerifyLastEnrollmentAttemptTimePref(kFakeTimeLaterAfterRetryPeriod); + scheduler()->StartEnrollmentScheduling(fake_enrollment_delegate()); + + cryptauthv2::ClientMetadata expected_enrollment_request = + cryptauthv2::BuildClientMetadata( + 0 /* retry_count */, cryptauthv2::ClientMetadata::INITIALIZATION, + base::nullopt /* session_id */); + VerifyScheduledEnrollment(expected_enrollment_request, + kZeroTimeDelta /* expected_delay */); + + enrollment_timer()->Fire(); + VerifyNoEnrollmentsTriggeredButRequestQueued(expected_enrollment_request); + + SetWifiNetworkStatus(NetworkConnectionStatus::kConnecting); + VerifyNoEnrollmentsTriggeredButRequestQueued(expected_enrollment_request); + + // Once Wifi network connected, reschedule enrollment. + SetWifiNetworkStatus(NetworkConnectionStatus::kConnected); + VerifyScheduledEnrollment(expected_enrollment_request, + kZeroTimeDelta /* expected_delay */); + + enrollment_timer()->Fire(); + + EXPECT_TRUE(scheduler()->IsWaitingForEnrollmentResult()); + VerifyLastPolicyReferenceReceivedByEnrollmentDelegate( + 1 /* total_received */, base::nullopt /* last_received*/); + VerifyLastClientMetadataReceivedByEnrollmentDelegate( + 1 /* total_received */, expected_enrollment_request); } -TEST_F(DeviceSyncCryptAuthSchedulerImplTest, HandlePersistedFailures) { - // Seed the preferences to simulate the previous scheduler using all of its - // immediate retry attempts and making 10 periodic retry attempts. - pref_service()->SetString( - prefs::kCryptAuthEnrollmentSchedulerClientDirective, - ClientDirectiveToPrefString(fake_client_directive())); - pref_service()->SetTime( - prefs::kCryptAuthEnrollmentSchedulerLastSuccessfulEnrollmentTime, - kFakeTimeNow); - pref_service()->SetTime( - prefs::kCryptAuthEnrollmentSchedulerLastEnrollmentAttemptTime, - kFakeTimeLaterBeforeRetryPeriod); - pref_service()->SetUint64( - prefs::kCryptAuthEnrollmentSchedulerNumConsecutiveFailures, - fake_client_directive().retry_attempts() + 10); - - clock()->SetNow(kFakeTimeLaterBeforeRetryPeriod); - - // On construction, if the persisted failure count is greater than 0, expect - // that the scheduler resets the failure count to 1. - CreateScheduler(); - - EXPECT_EQ(1u, scheduler()->GetNumConsecutiveFailures()); - EXPECT_EQ(kFakeImmediateRetryDelay, - scheduler()->GetTimeToNextEnrollmentRequest()); - VerifyLastEnrollmentAttemptTimePref(kFakeTimeLaterBeforeRetryPeriod); +TEST_F(DeviceSyncCryptAuthSchedulerImplTest, + EnrollmentRequestedWithNoWifiNetwork) { + const base::Time kNow = base::Time::FromDoubleT(1600600000); + clock()->SetNow(kNow); + cryptauthv2::ClientMetadata expected_enrollment_request = + cryptauthv2::BuildClientMetadata(0 /* retry_count */, + cryptauthv2::ClientMetadata::PERIODIC, + base::nullopt /* session_id */); + CreateScheduler( + cryptauthv2::GetClientDirectiveForTest() /* persisted_client_directive */, + expected_enrollment_request /* persisted_client_metadata */, + kNow /* persisted_last_enrollment_attempt_time */, + kNow /* persisted_last_successful_enrollment_time */ + ); + + scheduler()->StartEnrollmentScheduling(fake_enrollment_delegate()); + VerifyScheduledEnrollment(expected_enrollment_request, + scheduler()->GetRefreshPeriod()); + + enrollment_timer()->Fire(); + VerifyNoEnrollmentsTriggeredButRequestQueued(expected_enrollment_request); + + // Once Wifi network connected, reschedule enrollment. + const base::TimeDelta kTimeElapsed = base::TimeDelta::FromHours(10); + clock()->SetNow(kNow + kTimeElapsed); + AddDisconnectedWifiNetwork(); + SetWifiNetworkStatus(NetworkConnectionStatus::kConnected); + VerifyScheduledEnrollment(expected_enrollment_request, + scheduler()->GetRefreshPeriod() - kTimeElapsed); +} + +TEST_F(DeviceSyncCryptAuthSchedulerImplTest, + EnrollmentScheduledAndWifiNetworkRemoved) { + AddDisconnectedWifiNetwork(); + SetWifiNetworkStatus(NetworkConnectionStatus::kConnected); + + CreateScheduler(base::nullopt /* persisted_client_directive */, + base::nullopt /* persisted_client_metadata */, + base::nullopt /* persisted_last_enrollment_attempt_time */, + base::nullopt /* persisted_last_successful_enrollment_time */ + ); + + scheduler()->StartEnrollmentScheduling(fake_enrollment_delegate()); + + cryptauthv2::ClientMetadata expected_enrollment_request = + cryptauthv2::BuildClientMetadata( + 0 /* retry_count */, cryptauthv2::ClientMetadata::INITIALIZATION, + base::nullopt /* session_id */); + VerifyScheduledEnrollment(expected_enrollment_request, + kZeroTimeDelta /* expected_delay */); + + RemoveWifiNetwork(); + + enrollment_timer()->Fire(); + VerifyNoEnrollmentsTriggeredButRequestQueued(expected_enrollment_request); } } // namespace device_sync diff --git a/chromeos/services/device_sync/cryptauth_v2_enrollment_manager_impl.cc b/chromeos/services/device_sync/cryptauth_v2_enrollment_manager_impl.cc index b492c63ffc09ab..7471a0486b50e4 100644 --- a/chromeos/services/device_sync/cryptauth_v2_enrollment_manager_impl.cc +++ b/chromeos/services/device_sync/cryptauth_v2_enrollment_manager_impl.cc @@ -17,7 +17,6 @@ #include "chromeos/services/device_sync/cryptauth_constants.h" #include "chromeos/services/device_sync/cryptauth_key_registry.h" #include "chromeos/services/device_sync/cryptauth_v2_enroller_impl.h" -#include "chromeos/services/device_sync/network_aware_enrollment_scheduler.h" #include "chromeos/services/device_sync/pref_names.h" #include "chromeos/services/device_sync/public/cpp/client_app_metadata_provider.h" #include "components/prefs/pref_registry_simple.h" @@ -148,12 +147,6 @@ void CryptAuthV2EnrollmentManagerImpl::Factory::SetFactoryForTesting( // static void CryptAuthV2EnrollmentManagerImpl::RegisterPrefs( PrefRegistrySimple* registry) { - registry->RegisterIntegerPref( - prefs::kCryptAuthEnrollmentFailureRecoveryInvocationReason, - cryptauthv2::ClientMetadata::INVOCATION_REASON_UNSPECIFIED); - registry->RegisterStringPref( - prefs::kCryptAuthEnrollmentFailureRecoverySessionId, std::string()); - // TODO(nohle): Remove when v1 Enrollment is deprecated. registry->RegisterStringPref(prefs::kCryptAuthEnrollmentUserPublicKey, std::string()); @@ -199,12 +192,13 @@ CryptAuthV2EnrollmentManagerImpl::Factory::BuildInstance( CryptAuthKeyRegistry* key_registry, CryptAuthClientFactory* client_factory, CryptAuthGCMManager* gcm_manager, + CryptAuthScheduler* scheduler, PrefService* pref_service, base::Clock* clock, std::unique_ptr timer) { return base::WrapUnique(new CryptAuthV2EnrollmentManagerImpl( client_app_metadata_provider, key_registry, client_factory, gcm_manager, - pref_service, clock, std::move(timer))); + scheduler, pref_service, clock, std::move(timer))); } CryptAuthV2EnrollmentManagerImpl::CryptAuthV2EnrollmentManagerImpl( @@ -212,6 +206,7 @@ CryptAuthV2EnrollmentManagerImpl::CryptAuthV2EnrollmentManagerImpl( CryptAuthKeyRegistry* key_registry, CryptAuthClientFactory* client_factory, CryptAuthGCMManager* gcm_manager, + CryptAuthScheduler* scheduler, PrefService* pref_service, base::Clock* clock, std::unique_ptr timer) @@ -219,12 +214,16 @@ CryptAuthV2EnrollmentManagerImpl::CryptAuthV2EnrollmentManagerImpl( key_registry_(key_registry), client_factory_(client_factory), gcm_manager_(gcm_manager), + scheduler_(scheduler), pref_service_(pref_service), clock_(clock), timer_(std::move(timer)), - weak_ptr_factory_(this) { + callback_weak_ptr_factory_(this), + scheduler_weak_ptr_factory_(this) { // TODO(nohle): Remove when v1 Enrollment is deprecated. AddV1UserKeyPairToRegistryIfNecessary(); + + gcm_manager_->AddObserver(this); } CryptAuthV2EnrollmentManagerImpl::~CryptAuthV2EnrollmentManagerImpl() { @@ -232,31 +231,15 @@ CryptAuthV2EnrollmentManagerImpl::~CryptAuthV2EnrollmentManagerImpl() { } void CryptAuthV2EnrollmentManagerImpl::Start() { - // Ensure that Start() is only called once. - DCHECK(!scheduler_); - - scheduler_ = NetworkAwareEnrollmentScheduler::Factory::Get()->BuildInstance( - this, pref_service_); - - gcm_manager_->AddObserver(this); + scheduler_->StartEnrollmentScheduling( + scheduler_weak_ptr_factory_.GetWeakPtr()); } void CryptAuthV2EnrollmentManagerImpl::ForceEnrollmentNow( cryptauth::InvocationReason invocation_reason, const base::Optional& session_id) { - if (state_ != State::kIdle) { - PA_LOG(WARNING) << "Forced enrollment requested while an enrollment is in " - << "progress. No action taken."; - return; - } - - current_client_metadata_ = cryptauthv2::ClientMetadata(); - current_client_metadata_->set_invocation_reason( - ConvertInvocationReasonV1ToV2(invocation_reason)); - if (session_id) - current_client_metadata_->set_session_id(*session_id); - - scheduler_->RequestEnrollmentNow(); + scheduler_->RequestEnrollment( + ConvertInvocationReasonV1ToV2(invocation_reason), session_id); } bool CryptAuthV2EnrollmentManagerImpl::IsEnrollmentValid() const { @@ -289,7 +272,7 @@ bool CryptAuthV2EnrollmentManagerImpl::IsEnrollmentInProgress() const { } bool CryptAuthV2EnrollmentManagerImpl::IsRecoveringFromFailure() const { - return scheduler_->GetNumConsecutiveFailures() > 0; + return scheduler_->GetNumConsecutiveEnrollmentFailures() > 0; } std::string CryptAuthV2EnrollmentManagerImpl::GetUserPublicKey() const { @@ -326,44 +309,16 @@ std::string CryptAuthV2EnrollmentManagerImpl::GetUserPrivateKey() const { } void CryptAuthV2EnrollmentManagerImpl::OnEnrollmentRequested( + const cryptauthv2::ClientMetadata& client_metadata, const base::Optional& client_directive_policy_reference) { DCHECK(state_ == State::kIdle); NotifyEnrollmentStarted(); + current_client_metadata_ = client_metadata; client_directive_policy_reference_ = client_directive_policy_reference; - // Build the ClientMetadata if it hasn't already been built by - // ForceEnrollmentNow(). - if (!current_client_metadata_) { - current_client_metadata_ = cryptauthv2::ClientMetadata(); - base::Optional - failure_recovery_invocation_reason = - GetFailureRecoveryInvocationReasonFromPref(); - if (failure_recovery_invocation_reason) { - DCHECK(IsRecoveringFromFailure()); - current_client_metadata_->set_invocation_reason( - *failure_recovery_invocation_reason); - } else if (GetLastEnrollmentTime().is_null()) { - current_client_metadata_->set_invocation_reason( - cryptauthv2::ClientMetadata::INITIALIZATION); - } else if (!IsEnrollmentValid()) { - current_client_metadata_->set_invocation_reason( - cryptauthv2::ClientMetadata::PERIODIC); - } else { - current_client_metadata_->set_invocation_reason( - cryptauthv2::ClientMetadata::INVOCATION_REASON_UNSPECIFIED); - } - - base::Optional failure_recovery_session_id = - GetFailureRecoverySessionIdFromPref(); - if (failure_recovery_session_id) { - DCHECK(IsRecoveringFromFailure()); - current_client_metadata_->set_session_id(*failure_recovery_session_id); - } - } - base::UmaHistogramExactLinear( "CryptAuth.EnrollmentV2.InvocationReason", current_client_metadata_->invocation_reason(), @@ -422,7 +377,7 @@ void CryptAuthV2EnrollmentManagerImpl::AttemptEnrollment() { gcm_manager_->GetRegistrationId(), base::BindOnce( &CryptAuthV2EnrollmentManagerImpl::OnClientAppMetadataFetched, - weak_ptr_factory_.GetWeakPtr())); + callback_weak_ptr_factory_.GetWeakPtr())); return; } @@ -430,12 +385,8 @@ void CryptAuthV2EnrollmentManagerImpl::AttemptEnrollment() { } void CryptAuthV2EnrollmentManagerImpl::Enroll() { - // The invocation reason and session ID were set in ForceEnrollmentNow() for a - // forced enrollment and OnEnrollmentRequested() otherwise. Populate the other - // ClientMetadata fields here. DCHECK(current_client_metadata_); - current_client_metadata_->set_retry_count( - scheduler_->GetNumConsecutiveFailures()); + DCHECK(client_app_metadata_); enroller_ = CryptAuthV2EnrollerImpl::Factory::Get()->BuildInstance( key_registry_, client_factory_); @@ -446,14 +397,14 @@ void CryptAuthV2EnrollmentManagerImpl::Enroll() { *current_client_metadata_, *client_app_metadata_, client_directive_policy_reference_, base::BindOnce(&CryptAuthV2EnrollmentManagerImpl::OnEnrollmentFinished, - base::Unretained(this))); + callback_weak_ptr_factory_.GetWeakPtr())); } void CryptAuthV2EnrollmentManagerImpl::OnEnrollmentFinished( const CryptAuthEnrollmentResult& enrollment_result) { // Once an enrollment attempt finishes, no other callbacks should be // invoked. This is particularly relevant for timeout failures. - weak_ptr_factory_.InvalidateWeakPtrs(); + callback_weak_ptr_factory_.InvalidateWeakPtrs(); enroller_.reset(); if (enrollment_result.IsSuccess()) { @@ -461,24 +412,11 @@ void CryptAuthV2EnrollmentManagerImpl::OnEnrollmentFinished( << current_client_metadata_->invocation_reason() << " succeeded with result code " << enrollment_result.result_code(); - - pref_service_->SetInteger( - prefs::kCryptAuthEnrollmentFailureRecoveryInvocationReason, - cryptauthv2::ClientMetadata::INVOCATION_REASON_UNSPECIFIED); - pref_service_->SetString( - prefs::kCryptAuthEnrollmentFailureRecoverySessionId, std::string()); } else { PA_LOG(WARNING) << "Enrollment attempt with invocation reason " << current_client_metadata_->invocation_reason() << " failed with result code " << enrollment_result.result_code(); - - pref_service_->SetInteger( - prefs::kCryptAuthEnrollmentFailureRecoveryInvocationReason, - current_client_metadata_->invocation_reason()); - pref_service_->SetString( - prefs::kCryptAuthEnrollmentFailureRecoverySessionId, - current_client_metadata_->session_id()); } current_client_metadata_.reset(); @@ -492,7 +430,7 @@ void CryptAuthV2EnrollmentManagerImpl::OnEnrollmentFinished( if (!enrollment_result.IsSuccess()) { PA_LOG(INFO) << "Number of consecutive failures: " - << scheduler_->GetNumConsecutiveFailures(); + << scheduler_->GetNumConsecutiveEnrollmentFailures(); } SetState(State::kIdle); @@ -522,45 +460,11 @@ void CryptAuthV2EnrollmentManagerImpl::SetState(State state) { timer_->Start( FROM_HERE, *timeout_for_state, base::BindOnce(&CryptAuthV2EnrollmentManagerImpl::OnEnrollmentFinished, - base::Unretained(this), + callback_weak_ptr_factory_.GetWeakPtr(), CryptAuthEnrollmentResult( *error_code, base::nullopt /*client_directive */))); } -base::Optional -CryptAuthV2EnrollmentManagerImpl::GetFailureRecoveryInvocationReasonFromPref() - const { - int reason_stored_in_prefs = pref_service_->GetInteger( - prefs::kCryptAuthEnrollmentFailureRecoveryInvocationReason); - - if (!cryptauthv2::ClientMetadata::InvocationReason_IsValid( - reason_stored_in_prefs)) { - PA_LOG(WARNING) << "Unknown invocation reason, " << reason_stored_in_prefs - << ", stored in pref."; - - return base::nullopt; - } - - if (reason_stored_in_prefs == - cryptauthv2::ClientMetadata::INVOCATION_REASON_UNSPECIFIED) { - return base::nullopt; - } - - return static_cast( - reason_stored_in_prefs); -} - -base::Optional -CryptAuthV2EnrollmentManagerImpl::GetFailureRecoverySessionIdFromPref() const { - std::string session_id_stored_in_prefs = pref_service_->GetString( - prefs::kCryptAuthEnrollmentFailureRecoverySessionId); - - if (session_id_stored_in_prefs.empty()) - return base::nullopt; - - return session_id_stored_in_prefs; -} - std::string CryptAuthV2EnrollmentManagerImpl::GetV1UserPublicKey() const { std::string public_key; if (!base::Base64UrlDecode( diff --git a/chromeos/services/device_sync/cryptauth_v2_enrollment_manager_impl.h b/chromeos/services/device_sync/cryptauth_v2_enrollment_manager_impl.h index d04e213d60142a..357d5389ccb1bd 100644 --- a/chromeos/services/device_sync/cryptauth_v2_enrollment_manager_impl.h +++ b/chromeos/services/device_sync/cryptauth_v2_enrollment_manager_impl.h @@ -40,14 +40,14 @@ class CryptAuthV2Enroller; // Implementation of CryptAuthEnrollmentManager for CryptAuth v2 Enrollment. // // This implementation considers three sources of enrollment requests: -// 1) A network-aware enrollment scheduler requests periodic enrollments and -// handles any failed attempts. +// 1) The scheduler requests periodic enrollments and handles any failed +// attempts. // 2) The enrollment manager listens to the GCM manager for re-enrollment // requests. // 3) The ForceEnrollmentNow() method allows for immediate requests. // // All flavors of enrollment attempts are guarded by timeouts. For example, an -// enrollment attempt triggeredd by ForceEnrollmentNow() will always +// enrollment attempt triggered by ForceEnrollmentNow() will always // conclude--successfully or not--in an allotted period of time. // // The v2 Enrollment infrastructure stores keys in a CryptAuthKeyRegistry. In @@ -57,19 +57,10 @@ class CryptAuthV2Enroller; // the v2 key registry to ensure consistency across the v1 to v2 Enrollment // migration. The converse is not true. The v1 prefs will never be modified by // v2 Enrollment. -// -// The enrollment invocation reason sent to CryptAuth in ClientMetadata is -// determined using the following order of priority: -// 1) If ForceEnrollmentNow() was called, use its invocation reason argument. -// 2) If we are recovering from a failed enrollment attempt, reuse the initial -// invocation reason, which is stored as a pref. -// 3) If the user has never enrolled, use INITIALIZATION. -// 4) If the enrollment is no longer valid--due for a refresh, for -// example--use PERIODIC. -// 5) As a last resort, use INVOCATION_REASON_UNSPECIFIED. -class CryptAuthV2EnrollmentManagerImpl : public CryptAuthEnrollmentManager, - public CryptAuthScheduler::Delegate, - public CryptAuthGCMManager::Observer { +class CryptAuthV2EnrollmentManagerImpl + : public CryptAuthEnrollmentManager, + public CryptAuthScheduler::EnrollmentDelegate, + public CryptAuthGCMManager::Observer { public: class Factory { public: @@ -81,6 +72,7 @@ class CryptAuthV2EnrollmentManagerImpl : public CryptAuthEnrollmentManager, CryptAuthKeyRegistry* key_registry, CryptAuthClientFactory* client_factory, CryptAuthGCMManager* gcm_manager, + CryptAuthScheduler* scheduler, PrefService* pref_service, base::Clock* clock = base::DefaultClock::GetInstance(), std::unique_ptr timer = @@ -102,6 +94,7 @@ class CryptAuthV2EnrollmentManagerImpl : public CryptAuthEnrollmentManager, CryptAuthKeyRegistry* key_registry, CryptAuthClientFactory* client_factory, CryptAuthGCMManager* gcm_manager, + CryptAuthScheduler* scheduler, PrefService* pref_service, base::Clock* clock, std::unique_ptr timer); @@ -133,8 +126,9 @@ class CryptAuthV2EnrollmentManagerImpl : public CryptAuthEnrollmentManager, std::string GetUserPublicKey() const override; std::string GetUserPrivateKey() const override; - // CryptAuthEnrollmentScheduler::Delegate: - void OnEnrollmentRequested(const base::Optional& + // CryptAuthScheduler::EnrollmentDelegate: + void OnEnrollmentRequested(const cryptauthv2::ClientMetadata& client_metadata, + const base::Optional& client_directive_policy_reference) override; // CryptAuthGCMManager::Observer: @@ -157,12 +151,6 @@ class CryptAuthV2EnrollmentManagerImpl : public CryptAuthEnrollmentManager, void SetState(State state); - // Returns the invocation reason to be used when recovering from a failed - // enrollment attempt. If no valid reason is stored, returns null. - base::Optional - GetFailureRecoveryInvocationReasonFromPref() const; - base::Optional GetFailureRecoverySessionIdFromPref() const; - std::string GetV1UserPublicKey() const; std::string GetV1UserPrivateKey() const; @@ -174,24 +162,28 @@ class CryptAuthV2EnrollmentManagerImpl : public CryptAuthEnrollmentManager, CryptAuthKeyRegistry* key_registry_; CryptAuthClientFactory* client_factory_; CryptAuthGCMManager* gcm_manager_; + CryptAuthScheduler* scheduler_; PrefService* pref_service_; base::Clock* clock_; std::unique_ptr timer_; State state_ = State::kIdle; - std::unique_ptr scheduler_; - std::unique_ptr enroller_; - - // Only non-null while an enrollment attempt is active. The invocation reason - // and session ID are set in ForceEnrollmentNow() for forced enrollments and - // OnEnrollmentRequested() otherwise. The other ClientMetadata fields are - // populated in Enroll(). base::Optional current_client_metadata_; - - base::Optional client_app_metadata_; base::Optional client_directive_policy_reference_; - base::WeakPtrFactory weak_ptr_factory_; + base::Optional client_app_metadata_; + std::unique_ptr enroller_; + + // For weak pointers used in callbacks. These weak pointers are invalidated + // when the current enrollment attempt finishes in order to cancel outstanding + // callbacks. + base::WeakPtrFactory + callback_weak_ptr_factory_; + + // For sending a weak pointer to the scheduler, whose lifetime exceeds that of + // CryptAuthV2EnrollmentManagerImpl. + base::WeakPtrFactory + scheduler_weak_ptr_factory_; DISALLOW_COPY_AND_ASSIGN(CryptAuthV2EnrollmentManagerImpl); }; diff --git a/chromeos/services/device_sync/cryptauth_v2_enrollment_manager_impl_unittest.cc b/chromeos/services/device_sync/cryptauth_v2_enrollment_manager_impl_unittest.cc index 87d0c133314cb3..3bb8de8db168b7 100644 --- a/chromeos/services/device_sync/cryptauth_v2_enrollment_manager_impl_unittest.cc +++ b/chromeos/services/device_sync/cryptauth_v2_enrollment_manager_impl_unittest.cc @@ -11,13 +11,9 @@ #include "base/base64url.h" #include "base/no_destructor.h" #include "base/optional.h" -#include "base/run_loop.h" #include "base/test/metrics/histogram_tester.h" -#include "base/test/scoped_task_environment.h" #include "base/test/simple_test_clock.h" #include "base/timer/mock_timer.h" -#include "chromeos/dbus/dbus_thread_manager.h" -#include "chromeos/network/network_handler.h" #include "chromeos/services/device_sync/cryptauth_constants.h" #include "chromeos/services/device_sync/cryptauth_enrollment_result.h" #include "chromeos/services/device_sync/cryptauth_key_bundle.h" @@ -30,7 +26,6 @@ #include "chromeos/services/device_sync/fake_cryptauth_scheduler.h" #include "chromeos/services/device_sync/fake_cryptauth_v2_enroller.h" #include "chromeos/services/device_sync/mock_cryptauth_client.h" -#include "chromeos/services/device_sync/network_aware_enrollment_scheduler.h" #include "chromeos/services/device_sync/pref_names.h" #include "chromeos/services/device_sync/proto/cryptauth_api.pb.h" #include "chromeos/services/device_sync/proto/cryptauth_better_together_feature_metadata.pb.h" @@ -71,90 +66,6 @@ const cryptauthv2::PolicyReference& GetClientDirectivePolicyReferenceForTest() { return *policy_reference; } -// A child of FakeCryptAuthScheduler that sets scheduler parameters -// based on the latest enrollment result. -class FakeCryptAuthEnrollmentSchedulerWithResultHandling - : public FakeCryptAuthScheduler { - public: - FakeCryptAuthEnrollmentSchedulerWithResultHandling( - CryptAuthScheduler::Delegate* delegate, - base::Clock* clock) - : FakeCryptAuthScheduler(delegate), clock_(clock) {} - - ~FakeCryptAuthEnrollmentSchedulerWithResultHandling() override = default; - - void HandleEnrollmentResult( - const CryptAuthEnrollmentResult& enrollment_result) override { - FakeCryptAuthScheduler::HandleEnrollmentResult(enrollment_result); - - if (handled_enrollment_results().back().IsSuccess()) { - set_num_consecutive_failures(0); - set_time_to_next_enrollment_request(kFakeRefreshPeriod); - set_last_successful_enrollment_time(clock_->Now()); - } else { - set_num_consecutive_failures(GetNumConsecutiveFailures() + 1); - set_time_to_next_enrollment_request(kFakeRetryPeriod); - } - } - - private: - base::Clock* clock_; -}; - -class FakeNetworkAwareEnrollmentSchedulerFactory - : public NetworkAwareEnrollmentScheduler::Factory { - public: - FakeNetworkAwareEnrollmentSchedulerFactory( - const PrefService* expected_pref_service, - base::Clock* clock) - : expected_pref_service_(expected_pref_service), clock_(clock) {} - - ~FakeNetworkAwareEnrollmentSchedulerFactory() override = default; - - FakeCryptAuthEnrollmentSchedulerWithResultHandling* instance() { - return instance_; - } - - void SetInitialNumConsecutiveFailures(size_t num_consecutive_failures) { - initial_num_consecutive_failures_ = num_consecutive_failures; - } - - private: - // NetworkAwareEnrollmentScheduler::Factory - std::unique_ptr BuildInstance( - CryptAuthScheduler::Delegate* delegate, - PrefService* pref_service, - NetworkStateHandler* network_state_handler) override { - EXPECT_EQ(expected_pref_service_, pref_service); - - auto instance = - std::make_unique( - delegate, clock_); - instance_ = instance.get(); - - // Fix the scheduler's ClientDirective PolicyReference to test that it is - // passed to the enroller properly. - instance->set_client_directive_policy_reference( - GetClientDirectivePolicyReferenceForTest()); - - if (initial_num_consecutive_failures_) { - instance->set_num_consecutive_failures( - *initial_num_consecutive_failures_); - } - - return instance; - } - - const PrefService* expected_pref_service_; - base::Clock* clock_; - - base::Optional initial_num_consecutive_failures_; - - FakeCryptAuthEnrollmentSchedulerWithResultHandling* instance_ = nullptr; - - DISALLOW_COPY_AND_ASSIGN(FakeNetworkAwareEnrollmentSchedulerFactory); -}; - class FakeCryptAuthV2EnrollerFactory : public CryptAuthV2EnrollerImpl::Factory { public: FakeCryptAuthV2EnrollerFactory( @@ -206,10 +117,6 @@ class DeviceSyncCryptAuthV2EnrollmentManagerImplTest // testing::Test: void SetUp() override { - DBusThreadManager::Initialize(); - NetworkHandler::Initialize(); - base::RunLoop().RunUntilIdle(); - test_clock_.SetNow(base::Time::UnixEpoch()); CryptAuthV2EnrollmentManagerImpl::RegisterPrefs( @@ -219,16 +126,15 @@ class DeviceSyncCryptAuthV2EnrollmentManagerImplTest key_registry_ = CryptAuthKeyRegistryImpl::Factory::Get()->BuildInstance( &test_pref_service_); - fake_enrollment_scheduler_factory_ = - std::make_unique( - &test_pref_service_, &test_clock_); - NetworkAwareEnrollmentScheduler::Factory::SetFactoryForTesting( - fake_enrollment_scheduler_factory_.get()); - fake_enroller_factory_ = std::make_unique( key_registry_.get(), &mock_client_factory_); CryptAuthV2EnrollerImpl::Factory::SetFactoryForTesting( fake_enroller_factory_.get()); + + // Fix the scheduler's ClientDirective PolicyReference to test that it is + // passed to the enroller properly. + fake_enrollment_scheduler_.set_client_directive_policy_reference( + GetClientDirectivePolicyReferenceForTest()); } // testing::Test: @@ -236,11 +142,7 @@ class DeviceSyncCryptAuthV2EnrollmentManagerImplTest if (enrollment_manager_) enrollment_manager_->RemoveObserver(this); - NetworkAwareEnrollmentScheduler::Factory::SetFactoryForTesting(nullptr); CryptAuthV2EnrollerImpl::Factory::SetFactoryForTesting(nullptr); - - NetworkHandler::Shutdown(); - DBusThreadManager::Shutdown(); } // CryptAuthEnrollmentManager::Observer: @@ -267,12 +169,6 @@ class DeviceSyncCryptAuthV2EnrollmentManagerImplTest private_key_b64); } - void SetInitialNumConsecutiveFailures(size_t num_consecutive_failures) { - num_consecutive_failures_ = num_consecutive_failures; - fake_enrollment_scheduler_factory_->SetInitialNumConsecutiveFailures( - num_consecutive_failures); - } - void CreateEnrollmentManager() { auto mock_timer = std::make_unique(); mock_timer_ = mock_timer.get(); @@ -282,8 +178,9 @@ class DeviceSyncCryptAuthV2EnrollmentManagerImplTest enrollment_manager_ = CryptAuthV2EnrollmentManagerImpl::Factory::Get()->BuildInstance( &fake_client_app_metadata_provider_, key_registry_.get(), - &mock_client_factory_, &fake_gcm_manager_, &test_pref_service_, - &test_clock_, std::move(mock_timer)); + &mock_client_factory_, &fake_gcm_manager_, + &fake_enrollment_scheduler_, &test_pref_service_, &test_clock_, + std::move(mock_timer)); VerifyUserKeyPairStateHistogram(1u /* total_count */); @@ -338,9 +235,7 @@ class DeviceSyncCryptAuthV2EnrollmentManagerImplTest void FinishEnrollmentAttempt( size_t expected_enroller_instance_index, - const cryptauthv2::ClientMetadata::InvocationReason& - expected_invocation_reason, - const base::Optional& expected_session_id, + const cryptauthv2::ClientMetadata& expected_client_metadata, const CryptAuthEnrollmentResult& enrollment_result) { EXPECT_TRUE(enrollment_manager_->IsEnrollmentInProgress()); @@ -358,30 +253,17 @@ class DeviceSyncCryptAuthV2EnrollmentManagerImplTest fake_enroller_factory_ ->created_instances()[expected_enroller_instance_index]; - VerifyEnrollerData(enroller, expected_invocation_reason, - expected_session_id); + VerifyEnrollerData(enroller, expected_client_metadata); enroller->FinishAttempt(enrollment_result); EXPECT_FALSE(enrollment_manager_->IsEnrollmentInProgress()); - - cryptauthv2::ClientMetadata::InvocationReason expected_persisted_reason = - enrollment_manager_->IsRecoveringFromFailure() - ? expected_invocation_reason - : cryptauthv2::ClientMetadata::INVOCATION_REASON_UNSPECIFIED; - VerifyPersistedFailureRecoveryInvocationReason(expected_persisted_reason); - - std::string expected_persisted_session_id = - enrollment_manager_->IsRecoveringFromFailure() - ? expected_session_id.value_or(std::string()) - : std::string(); - VerifyPersistedFailureRecoverySessionId(expected_persisted_session_id); } void VerifyEnrollmentResults( const std::vector& expected_results) { VerifyResultsSentToEnrollmentManagerObservers(expected_results); - VerifyResultsSentToEnrollmentScheduler(expected_results); + VerifyResultsSentToScheduler(expected_results); VerifyEnrollmentResultHistograms(expected_results); } @@ -412,9 +294,8 @@ class DeviceSyncCryptAuthV2EnrollmentManagerImplTest CryptAuthKeyRegistry* key_registry() { return key_registry_.get(); } - FakeCryptAuthEnrollmentSchedulerWithResultHandling* - fake_enrollment_scheduler() { - return fake_enrollment_scheduler_factory_->instance(); + FakeCryptAuthScheduler* fake_enrollment_scheduler() { + return &fake_enrollment_scheduler_; } base::SimpleTestClock* test_clock() { return &test_clock_; } @@ -428,21 +309,12 @@ class DeviceSyncCryptAuthV2EnrollmentManagerImplTest private: void VerifyEnrollerData( FakeCryptAuthV2Enroller* enroller, - const cryptauthv2::ClientMetadata::InvocationReason& - expected_invocation_reason, - const base::Optional& expected_session_id) { + const cryptauthv2::ClientMetadata& expected_client_metadata) { EXPECT_TRUE(enroller->was_enroll_called()); - - EXPECT_EQ(cryptauthv2::BuildClientMetadata( - fake_enrollment_scheduler() - ->GetNumConsecutiveFailures() /* retry_count */, - expected_invocation_reason, expected_session_id) - .SerializeAsString(), + EXPECT_EQ(expected_client_metadata.SerializeAsString(), enroller->client_metadata()->SerializeAsString()); - EXPECT_EQ(cryptauthv2::GetClientAppMetadataForTest().SerializeAsString(), enroller->client_app_metadata()->SerializeAsString()); - EXPECT_EQ( GetClientDirectivePolicyReferenceForTest().SerializeAsString(), (*enroller->client_directive_policy_reference())->SerializeAsString()); @@ -460,30 +332,12 @@ class DeviceSyncCryptAuthV2EnrollmentManagerImplTest } } - void VerifyResultsSentToEnrollmentScheduler( - const std::vector - expected_enrollment_results) { + void VerifyResultsSentToScheduler(const std::vector + expected_enrollment_results) { EXPECT_EQ(expected_enrollment_results, fake_enrollment_scheduler()->handled_enrollment_results()); } - void VerifyPersistedFailureRecoveryInvocationReason( - const cryptauthv2::ClientMetadata::InvocationReason& - expected_failure_recovery_invocation_reason) { - EXPECT_EQ( - expected_failure_recovery_invocation_reason, - static_cast( - test_pref_service_.GetInteger( - prefs::kCryptAuthEnrollmentFailureRecoveryInvocationReason))); - } - - void VerifyPersistedFailureRecoverySessionId( - const std::string& expected_persisted_session_id) { - EXPECT_EQ(expected_persisted_session_id, - test_pref_service_.GetString( - prefs::kCryptAuthEnrollmentFailureRecoverySessionId)); - } - void VerifyEnrollmentResultHistograms( const std::vector expected_enrollment_results) { @@ -516,22 +370,18 @@ class DeviceSyncCryptAuthV2EnrollmentManagerImplTest false, failure_count); } - base::test::ScopedTaskEnvironment scoped_task_environment_; - size_t num_enrollment_started_notifications_ = 0; - size_t num_consecutive_failures_ = 0; std::vector observer_enrollment_finished_success_list_; TestingPrefServiceSimple test_pref_service_; FakeClientAppMetadataProvider fake_client_app_metadata_provider_; FakeCryptAuthGCMManager fake_gcm_manager_; + FakeCryptAuthScheduler fake_enrollment_scheduler_; base::SimpleTestClock test_clock_; base::MockOneShotTimer* mock_timer_; MockCryptAuthClientFactory mock_client_factory_; base::HistogramTester histogram_tester_; std::unique_ptr key_registry_; - std::unique_ptr - fake_enrollment_scheduler_factory_; std::unique_ptr fake_enroller_factory_; std::unique_ptr enrollment_manager_; @@ -540,17 +390,24 @@ class DeviceSyncCryptAuthV2EnrollmentManagerImplTest TEST_F(DeviceSyncCryptAuthV2EnrollmentManagerImplTest, EnrollmentRequestedFromScheduler_NeverPreviouslyEnrolled) { CreateEnrollmentManager(); - EXPECT_FALSE(fake_enrollment_scheduler()); + EXPECT_FALSE(fake_enrollment_scheduler()->HasEnrollmentSchedulingStarted()); enrollment_manager()->Start(); - EXPECT_TRUE(fake_enrollment_scheduler()); + EXPECT_TRUE(fake_enrollment_scheduler()->HasEnrollmentSchedulingStarted()); // The user has never enrolled with v1 or v2 and has not registered with GCM. + fake_enrollment_scheduler()->set_last_successful_enrollment_time( + base::Time()); EXPECT_TRUE(key_registry()->key_bundles().empty()); EXPECT_TRUE(enrollment_manager()->GetLastEnrollmentTime().is_null()); EXPECT_FALSE(enrollment_manager()->IsEnrollmentValid()); - fake_enrollment_scheduler()->RequestEnrollmentNow(); + // Scheduler triggers an initialization enrollment request. + fake_enrollment_scheduler()->set_num_consecutive_enrollment_failures(0); + fake_enrollment_scheduler()->RequestEnrollment( + cryptauthv2::ClientMetadata::INITIALIZATION /* invocation_reason */, + base::nullopt /* session_id */); + EXPECT_TRUE(enrollment_manager()->IsEnrollmentInProgress()); VerifyEnrollmentManagerObserversNotifiedOfStart( 1 /* expected_num_enrollment_started_notifications */); @@ -562,11 +419,13 @@ TEST_F(DeviceSyncCryptAuthV2EnrollmentManagerImplTest, CryptAuthEnrollmentResult::ResultCode::kSuccessNewKeysEnrolled, cryptauthv2::GetClientDirectiveForTest()); - FinishEnrollmentAttempt(0u /* expected_enroller_instance_index */, - cryptauthv2::ClientMetadata:: - INITIALIZATION /* expected_invocation_reason */, - base::nullopt /* expected_session_id */, - expected_enrollment_result); + FinishEnrollmentAttempt( + 0u /* expected_enroller_instance_index */, + cryptauthv2::BuildClientMetadata( + 0 /* retry_count */, + cryptauthv2::ClientMetadata::INITIALIZATION /* invocation_reason */, + base::nullopt /* session_id */) /* expected_client_metadata */, + expected_enrollment_result); VerifyEnrollmentResults({expected_enrollment_result}); } @@ -574,7 +433,10 @@ TEST_F(DeviceSyncCryptAuthV2EnrollmentManagerImplTest, TEST_F(DeviceSyncCryptAuthV2EnrollmentManagerImplTest, GcmRegistrationFailed) { CreateEnrollmentManager(); enrollment_manager()->Start(); - fake_enrollment_scheduler()->RequestEnrollmentNow(); + + fake_enrollment_scheduler()->RequestEnrollment( + cryptauthv2::ClientMetadata::INITIALIZATION /* invocation_reason */, + base::nullopt /* session_id */); CompleteGcmRegistration(false /* success */); @@ -586,7 +448,10 @@ TEST_F(DeviceSyncCryptAuthV2EnrollmentManagerImplTest, GcmRegistrationFailed) { TEST_F(DeviceSyncCryptAuthV2EnrollmentManagerImplTest, GcmRegistrationTimeout) { CreateEnrollmentManager(); enrollment_manager()->Start(); - fake_enrollment_scheduler()->RequestEnrollmentNow(); + + fake_enrollment_scheduler()->RequestEnrollment( + cryptauthv2::ClientMetadata::INITIALIZATION /* invocation_reason */, + base::nullopt /* session_id */); // Timeout waiting for GcmRegistration. EXPECT_TRUE(mock_timer()->IsRunning()); @@ -602,7 +467,11 @@ TEST_F(DeviceSyncCryptAuthV2EnrollmentManagerImplTest, ClientAppMetadataFetchFailed) { CreateEnrollmentManager(); enrollment_manager()->Start(); - fake_enrollment_scheduler()->RequestEnrollmentNow(); + + fake_enrollment_scheduler()->RequestEnrollment( + cryptauthv2::ClientMetadata::INITIALIZATION /* invocation_reason */, + base::nullopt /* session_id */); + CompleteGcmRegistration(true /* success */); HandleGetClientAppMetadataRequest(false /* success */); @@ -615,7 +484,11 @@ TEST_F(DeviceSyncCryptAuthV2EnrollmentManagerImplTest, ClientAppMetadataTimeout) { CreateEnrollmentManager(); enrollment_manager()->Start(); - fake_enrollment_scheduler()->RequestEnrollmentNow(); + + fake_enrollment_scheduler()->RequestEnrollment( + cryptauthv2::ClientMetadata::INITIALIZATION /* invocation_reason */, + base::nullopt /* session_id */); + CompleteGcmRegistration(true /* success */); // Timeout waiting for ClientAppMetadata. @@ -634,7 +507,7 @@ TEST_F(DeviceSyncCryptAuthV2EnrollmentManagerImplTest, ForcedEnrollment) { enrollment_manager()->ForceEnrollmentNow( cryptauth::InvocationReason::INVOCATION_REASON_FEATURE_TOGGLED, - base::nullopt /* session_id */); + kFakeSessionId); EXPECT_TRUE(enrollment_manager()->IsEnrollmentInProgress()); VerifyEnrollmentManagerObserversNotifiedOfStart( 1 /* expected_num_enrollment_started_notifications */); @@ -642,14 +515,18 @@ TEST_F(DeviceSyncCryptAuthV2EnrollmentManagerImplTest, ForcedEnrollment) { // Simulate a failed enrollment attempt due to CryptAuth server overload. CompleteGcmRegistration(true /* success */); HandleGetClientAppMetadataRequest(true /* success */); + CryptAuthEnrollmentResult expected_enrollment_result( CryptAuthEnrollmentResult::ResultCode::kErrorCryptAuthServerOverloaded, base::nullopt /* client_directive */); - FinishEnrollmentAttempt(0u /* expected_enroller_instance_index */, - cryptauthv2::ClientMetadata:: - FEATURE_TOGGLED /* expected_invocation_reason */, - base::nullopt /* expected_session_id */, - expected_enrollment_result); + + FinishEnrollmentAttempt( + 0u /* expected_enroller_instance_index */, + cryptauthv2::BuildClientMetadata( + 0 /* retry_count */, + cryptauthv2::ClientMetadata::FEATURE_TOGGLED /* invocation_reason */, + kFakeSessionId) /* expected_client_metadata */, + expected_enrollment_result); VerifyEnrollmentResults({expected_enrollment_result}); } @@ -659,34 +536,38 @@ TEST_F(DeviceSyncCryptAuthV2EnrollmentManagerImplTest, CreateEnrollmentManager(); enrollment_manager()->Start(); - // The user has already enrolled and is due for a refresh. + // The user has already enrolled. base::Time expected_last_enrollment_time = test_clock()->Now(); fake_enrollment_scheduler()->set_last_successful_enrollment_time( expected_last_enrollment_time); + EXPECT_EQ(expected_last_enrollment_time, + enrollment_manager()->GetLastEnrollmentTime()); fake_enrollment_scheduler()->set_refresh_period(kFakeRefreshPeriod); + fake_enrollment_scheduler()->set_time_to_next_enrollment_request( + kFakeRefreshPeriod); + EXPECT_TRUE(enrollment_manager()->IsEnrollmentValid()); + EXPECT_EQ(kFakeRefreshPeriod, enrollment_manager()->GetTimeToNextAttempt()); - // Set clock after refresh period. + // Now, user is due for a refresh. test_clock()->SetNow(test_clock()->Now() + kFakeRefreshPeriod + base::TimeDelta::FromSeconds(1)); - + EXPECT_FALSE(enrollment_manager()->IsEnrollmentValid()); base::TimeDelta expected_time_to_next_attempt = base::TimeDelta::FromSeconds(0); fake_enrollment_scheduler()->set_time_to_next_enrollment_request( expected_time_to_next_attempt); - - EXPECT_EQ(expected_last_enrollment_time, - enrollment_manager()->GetLastEnrollmentTime()); - EXPECT_FALSE(enrollment_manager()->IsEnrollmentValid()); EXPECT_EQ(expected_time_to_next_attempt, enrollment_manager()->GetTimeToNextAttempt()); - EXPECT_FALSE(enrollment_manager()->IsRecoveringFromFailure()); - cryptauthv2::ClientMetadata::InvocationReason expected_invocation_reason = - cryptauthv2::ClientMetadata::PERIODIC; + cryptauthv2::ClientMetadata expected_client_metadata = + cryptauthv2::BuildClientMetadata(0 /* retry_count */, + cryptauthv2::ClientMetadata::PERIODIC, + base::nullopt /* session_id */); // First enrollment attempt fails. // Note: User does not yet have a GCM registration ID or ClientAppMetadata. - fake_enrollment_scheduler()->RequestEnrollmentNow(); + fake_enrollment_scheduler()->RequestEnrollment( + cryptauthv2::ClientMetadata::PERIODIC, base::nullopt /* session_id */); EXPECT_TRUE(enrollment_manager()->IsEnrollmentInProgress()); VerifyEnrollmentManagerObserversNotifiedOfStart( 1 /* expected_num_enrollment_started_notifications */); @@ -699,33 +580,44 @@ TEST_F(DeviceSyncCryptAuthV2EnrollmentManagerImplTest, base::nullopt /* client_directive */); FinishEnrollmentAttempt(0u /* expected_enroller_instance_index */, - expected_invocation_reason, - base::nullopt /* expected_session_id */, + expected_client_metadata, first_expected_enrollment_result); + EXPECT_FALSE(enrollment_manager()->IsRecoveringFromFailure()); + fake_enrollment_scheduler()->set_num_consecutive_enrollment_failures(1); + EXPECT_TRUE(enrollment_manager()->IsRecoveringFromFailure()); + + fake_enrollment_scheduler()->set_time_to_next_enrollment_request( + kFakeRetryPeriod); EXPECT_EQ(kFakeRetryPeriod, enrollment_manager()->GetTimeToNextAttempt()); // Second (successful) enrollment attempt bypasses GCM registration and // ClientAppMetadata fetch because they were performed during the failed // attempt. - fake_enrollment_scheduler()->RequestEnrollmentNow(); + test_clock()->SetNow(test_clock()->Now() + kFakeRetryPeriod); + fake_enrollment_scheduler()->RequestEnrollment( + cryptauthv2::ClientMetadata::PERIODIC, base::nullopt /* session_id */); VerifyEnrollmentManagerObserversNotifiedOfStart( 2 /* expected_num_enrollment_started_notifications */); - EXPECT_TRUE(enrollment_manager()->IsRecoveringFromFailure()); CryptAuthEnrollmentResult second_expected_enrollment_result = CryptAuthEnrollmentResult( CryptAuthEnrollmentResult::ResultCode::kSuccessNoNewKeysNeeded, cryptauthv2::GetClientDirectiveForTest()); + expected_client_metadata.set_retry_count(1); FinishEnrollmentAttempt(1u /* expected_enroller_instance_index */, - expected_invocation_reason, - base::nullopt /* expected_session_id */, + expected_client_metadata, second_expected_enrollment_result); VerifyEnrollmentResults( {first_expected_enrollment_result, second_expected_enrollment_result}); + fake_enrollment_scheduler()->set_last_successful_enrollment_time( + test_clock()->Now()); + fake_enrollment_scheduler()->set_time_to_next_enrollment_request( + kFakeRefreshPeriod); + fake_enrollment_scheduler()->set_num_consecutive_enrollment_failures(0); EXPECT_EQ(test_clock()->Now(), enrollment_manager()->GetLastEnrollmentTime()); EXPECT_TRUE(enrollment_manager()->IsEnrollmentValid()); EXPECT_EQ(kFakeRefreshPeriod, enrollment_manager()->GetTimeToNextAttempt()); @@ -747,11 +639,12 @@ TEST_F(DeviceSyncCryptAuthV2EnrollmentManagerImplTest, CryptAuthEnrollmentResult expected_enrollment_result( CryptAuthEnrollmentResult::ResultCode::kSuccessNoNewKeysNeeded, cryptauthv2::GetClientDirectiveForTest()); - FinishEnrollmentAttempt(0u /* expected_enroller_instance_index */, - cryptauthv2::ClientMetadata:: - SERVER_INITIATED /* expected_invocation_reason */, - kFakeSessionId /* expected_session_id */, - expected_enrollment_result); + FinishEnrollmentAttempt( + 0u /* expected_enroller_instance_index */, + cryptauthv2::BuildClientMetadata( + 0 /* retry_count */, cryptauthv2::ClientMetadata::SERVER_INITIATED, + kFakeSessionId) /* expected_client_metadata */, + expected_enrollment_result); VerifyEnrollmentResults({expected_enrollment_result}); } @@ -770,11 +663,12 @@ TEST_F(DeviceSyncCryptAuthV2EnrollmentManagerImplTest, CryptAuthEnrollmentResult expected_enrollment_result( CryptAuthEnrollmentResult::ResultCode::kErrorCryptAuthServerOverloaded, base::nullopt /* client_directive */); - FinishEnrollmentAttempt(0u /* expected_enroller_instance_index */, - cryptauthv2::ClientMetadata:: - SERVER_INITIATED /* expected_invocation_reason */, - kFakeSessionId /* expected_session_id */, - expected_enrollment_result); + FinishEnrollmentAttempt( + 0u /* expected_enroller_instance_index */, + cryptauthv2::BuildClientMetadata( + 0 /* retry_count */, cryptauthv2::ClientMetadata::SERVER_INITIATED, + kFakeSessionId) /* expected_client_metadata */, + expected_enrollment_result); VerifyEnrollmentResults({expected_enrollment_result}); } @@ -857,42 +751,55 @@ TEST_F(DeviceSyncCryptAuthV2EnrollmentManagerImplTest, expected_enrollment_results.emplace_back( CryptAuthEnrollmentResult::ResultCode::kSuccessNewKeysEnrolled, cryptauthv2::GetClientDirectiveForTest()); - fake_enrollment_scheduler()->RequestEnrollmentNow(); + fake_enrollment_scheduler()->RequestEnrollment( + expected_invocation_reasons.back(), kFakeSessionId); VerifyEnrollmentManagerObserversNotifiedOfStart( 1 /* expected_num_enrollment_started_notifications */); CompleteGcmRegistration(true /* success */); HandleGetClientAppMetadataRequest(true /* success */); - FinishEnrollmentAttempt(0u /* expected_enroller_instance_index */, - expected_invocation_reasons.back(), - base::nullopt /* expected_session_id */, - expected_enrollment_results.back()); + FinishEnrollmentAttempt( + 0u /* expected_enroller_instance_index */, + cryptauthv2::BuildClientMetadata( + 0 /* retry_count */, expected_invocation_reasons.back(), + kFakeSessionId) /* expected_client_metadata */, + expected_enrollment_results.back()); // Fail periodic refresh twice due to overloaded CryptAuth server. - test_clock()->SetNow(test_clock()->Now() + kFakeRefreshPeriod + - base::TimeDelta::FromSeconds(1)); expected_invocation_reasons.push_back(cryptauthv2::ClientMetadata::PERIODIC); expected_enrollment_results.emplace_back( CryptAuthEnrollmentResult::ResultCode::kErrorCryptAuthServerOverloaded, cryptauthv2::GetClientDirectiveForTest()); - fake_enrollment_scheduler()->RequestEnrollmentNow(); + fake_enrollment_scheduler()->RequestEnrollment( + expected_invocation_reasons.back(), kFakeSessionId); VerifyEnrollmentManagerObserversNotifiedOfStart( 2 /* expected_num_enrollment_started_notifications */); - FinishEnrollmentAttempt(1u /* expected_enroller_instance_index */, - expected_invocation_reasons.back(), - base::nullopt /* expected_session_id */, - expected_enrollment_results.back()); + FinishEnrollmentAttempt( + 1u /* expected_enroller_instance_index */, + cryptauthv2::BuildClientMetadata( + 0 /* retry_count */, expected_invocation_reasons.back(), + kFakeSessionId) /* expected_client_metadata */, + expected_enrollment_results.back()); + fake_enrollment_scheduler()->set_num_consecutive_enrollment_failures(1); + fake_enrollment_scheduler()->set_time_to_next_enrollment_request( + kFakeRetryPeriod); expected_invocation_reasons.push_back(cryptauthv2::ClientMetadata::PERIODIC); expected_enrollment_results.emplace_back( CryptAuthEnrollmentResult::ResultCode::kErrorCryptAuthServerOverloaded, base::nullopt /* client_directive */); - fake_enrollment_scheduler()->RequestEnrollmentNow(); + fake_enrollment_scheduler()->RequestEnrollment( + expected_invocation_reasons.back(), kFakeSessionId); VerifyEnrollmentManagerObserversNotifiedOfStart( 3 /* expected_num_enrollment_started_notifications */); - FinishEnrollmentAttempt(2u /* expected_enroller_instance_index */, - expected_invocation_reasons.back(), - base::nullopt /* expected_session_id */, - expected_enrollment_results.back()); + FinishEnrollmentAttempt( + 2u /* expected_enroller_instance_index */, + cryptauthv2::BuildClientMetadata( + 1 /* retry_count */, expected_invocation_reasons.back(), + kFakeSessionId) /* expected_client_metadata */, + expected_enrollment_results.back()); + fake_enrollment_scheduler()->set_num_consecutive_enrollment_failures(2); + fake_enrollment_scheduler()->set_time_to_next_enrollment_request( + kFakeRetryPeriod); // While waiting for retry, force a manual enrollment that succeeds. expected_invocation_reasons.push_back(cryptauthv2::ClientMetadata::MANUAL); @@ -903,10 +810,12 @@ TEST_F(DeviceSyncCryptAuthV2EnrollmentManagerImplTest, base::nullopt /* session_id */); VerifyEnrollmentManagerObserversNotifiedOfStart( 4 /* expected_num_enrollment_started_notifications */); - FinishEnrollmentAttempt(3u /* expected_enroller_instance_index */, - expected_invocation_reasons.back(), - base::nullopt /* expected_session_id */, - expected_enrollment_results.back()); + FinishEnrollmentAttempt( + 3u /* expected_enroller_instance_index */, + cryptauthv2::BuildClientMetadata( + 2 /* retry_count */, expected_invocation_reasons.back(), + base::nullopt /* session_id */) /* expected_client_metadata */, + expected_enrollment_results.back()); VerifyInvocationReasonHistogram(expected_invocation_reasons); VerifyEnrollmentResults(expected_enrollment_results); diff --git a/chromeos/services/device_sync/device_sync_impl.cc b/chromeos/services/device_sync/device_sync_impl.cc index 7999061ced9eec..783cd05ce1e1a2 100644 --- a/chromeos/services/device_sync/device_sync_impl.cc +++ b/chromeos/services/device_sync/device_sync_impl.cc @@ -507,6 +507,7 @@ void DeviceSyncImpl::Shutdown() { remote_device_provider_.reset(); cryptauth_device_manager_.reset(); cryptauth_enrollment_manager_.reset(); + cryptauth_scheduler_.reset(); cryptauth_key_registry_.reset(); cryptauth_client_factory_.reset(); cryptauth_gcm_manager_.reset(); @@ -598,11 +599,15 @@ void DeviceSyncImpl::InitializeCryptAuthManagementObjects() { CryptAuthKeyRegistryImpl::Factory::Get()->BuildInstance( pref_service_.get()); + cryptauth_scheduler_ = + CryptAuthSchedulerImpl::Factory::Get()->BuildInstance( + pref_service_.get()); + cryptauth_enrollment_manager_ = CryptAuthV2EnrollmentManagerImpl::Factory::Get()->BuildInstance( client_app_metadata_provider_, cryptauth_key_registry_.get(), cryptauth_client_factory_.get(), cryptauth_gcm_manager_.get(), - pref_service_.get(), clock_); + cryptauth_scheduler_.get(), pref_service_.get(), clock_); } else { cryptauth_enrollment_manager_ = CryptAuthEnrollmentManagerImpl::Factory::NewInstance( diff --git a/chromeos/services/device_sync/device_sync_impl.h b/chromeos/services/device_sync/device_sync_impl.h index a7ed351004d3f3..5f6b545aa2c404 100644 --- a/chromeos/services/device_sync/device_sync_impl.h +++ b/chromeos/services/device_sync/device_sync_impl.h @@ -50,6 +50,7 @@ namespace device_sync { class ClientAppMetadataProvider; class CryptAuthClientFactory; class CryptAuthDeviceManager; +class CryptAuthScheduler; class CryptAuthKeyRegistry; class GcmDeviceInfoProvider; class SoftwareFeatureManager; @@ -233,6 +234,7 @@ class DeviceSyncImpl : public DeviceSyncBase, // Only created and used if v2 Enrollment is enabled; null otherwise. std::unique_ptr cryptauth_key_registry_; + std::unique_ptr cryptauth_scheduler_; std::unique_ptr cryptauth_enrollment_manager_; std::unique_ptr cryptauth_device_manager_; diff --git a/chromeos/services/device_sync/device_sync_service_unittest.cc b/chromeos/services/device_sync/device_sync_service_unittest.cc index 30a200376a9c06..b3bc1c6843e746 100644 --- a/chromeos/services/device_sync/device_sync_service_unittest.cc +++ b/chromeos/services/device_sync/device_sync_service_unittest.cc @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "chromeos/services/device_sync/device_sync_service.h" + #include #include "base/bind.h" @@ -23,12 +25,13 @@ #include "chromeos/services/device_sync/cryptauth_enrollment_manager_impl.h" #include "chromeos/services/device_sync/cryptauth_gcm_manager_impl.h" #include "chromeos/services/device_sync/cryptauth_key_registry_impl.h" +#include "chromeos/services/device_sync/cryptauth_scheduler_impl.h" #include "chromeos/services/device_sync/cryptauth_v2_enrollment_manager_impl.h" #include "chromeos/services/device_sync/device_sync_impl.h" -#include "chromeos/services/device_sync/device_sync_service.h" #include "chromeos/services/device_sync/fake_cryptauth_device_manager.h" #include "chromeos/services/device_sync/fake_cryptauth_enrollment_manager.h" #include "chromeos/services/device_sync/fake_cryptauth_gcm_manager.h" +#include "chromeos/services/device_sync/fake_cryptauth_scheduler.h" #include "chromeos/services/device_sync/fake_device_sync_observer.h" #include "chromeos/services/device_sync/fake_remote_device_provider.h" #include "chromeos/services/device_sync/fake_software_feature_manager.h" @@ -207,6 +210,38 @@ class FakeCryptAuthDeviceManagerFactory FakeCryptAuthDeviceManager* instance_ = nullptr; }; +class FakeCryptAuthSchedulerFactory : public CryptAuthSchedulerImpl::Factory { + public: + FakeCryptAuthSchedulerFactory(TestingPrefServiceSimple* test_pref_service) + : test_pref_service_(test_pref_service) {} + + ~FakeCryptAuthSchedulerFactory() override = default; + + FakeCryptAuthScheduler* instance() { return instance_; } + + // CryptAuthSchedulerImpl::Factory: + std::unique_ptr BuildInstance( + PrefService* pref_service, + NetworkStateHandler* network_state_handler, + base::Clock* clock, + std::unique_ptr enrollment_timer) override { + EXPECT_EQ(test_pref_service_, pref_service); + + // Only one instance is expected to be created per test. + EXPECT_FALSE(instance_); + + auto instance = std::make_unique(); + instance_ = instance.get(); + + return std::move(instance); + } + + private: + TestingPrefServiceSimple* test_pref_service_; + + FakeCryptAuthScheduler* instance_ = nullptr; +}; + class FakeCryptAuthEnrollmentManagerFactory : public CryptAuthEnrollmentManagerImpl::Factory { public: @@ -307,12 +342,14 @@ class FakeCryptAuthV2EnrollmentManagerFactory FakeClientAppMetadataProvider* fake_client_app_metadata_provider, FakeCryptAuthKeyRegistryFactory* fake_cryptauth_key_registry_factory, FakeCryptAuthGCMManagerFactory* fake_cryptauth_gcm_manager_factory, + FakeCryptAuthSchedulerFactory* fake_cryptauth_scheduler_factory, TestingPrefServiceSimple* test_pref_service, base::SimpleTestClock* simple_test_clock) : fake_client_app_metadata_provider_(fake_client_app_metadata_provider), fake_cryptauth_key_registry_factory_( fake_cryptauth_key_registry_factory), fake_cryptauth_gcm_manager_factory_(fake_cryptauth_gcm_manager_factory), + fake_cryptauth_scheduler_factory_(fake_cryptauth_scheduler_factory), test_pref_service_(test_pref_service), simple_test_clock_(simple_test_clock) {} @@ -332,12 +369,14 @@ class FakeCryptAuthV2EnrollmentManagerFactory CryptAuthKeyRegistry* key_registry, CryptAuthClientFactory* client_factory, CryptAuthGCMManager* gcm_manager, + CryptAuthScheduler* scheduler, PrefService* pref_service, base::Clock* clock, std::unique_ptr timer) override { EXPECT_EQ(fake_client_app_metadata_provider_, client_app_metadata_provider); EXPECT_EQ(fake_cryptauth_key_registry_factory_->instance(), key_registry); EXPECT_EQ(fake_cryptauth_gcm_manager_factory_->instance(), gcm_manager); + EXPECT_EQ(fake_cryptauth_scheduler_factory_->instance(), scheduler); EXPECT_EQ(test_pref_service_, pref_service); EXPECT_EQ(simple_test_clock_, clock); @@ -356,6 +395,7 @@ class FakeCryptAuthV2EnrollmentManagerFactory FakeClientAppMetadataProvider* fake_client_app_metadata_provider_; FakeCryptAuthKeyRegistryFactory* fake_cryptauth_key_registry_factory_; FakeCryptAuthGCMManagerFactory* fake_cryptauth_gcm_manager_factory_; + FakeCryptAuthSchedulerFactory* fake_cryptauth_scheduler_factory_; TestingPrefServiceSimple* test_pref_service_; base::SimpleTestClock* simple_test_clock_; @@ -556,6 +596,8 @@ class DeviceSyncServiceTest : public ::testing::TestWithParam { chromeos::features::kCryptAuthV2Enrollment, use_v2_enrollment_); DBusThreadManager::Initialize(); + NetworkHandler::Initialize(); + base::RunLoop().RunUntilIdle(); fake_gcm_driver_ = std::make_unique(); @@ -593,11 +635,17 @@ class DeviceSyncServiceTest : public ::testing::TestWithParam { CryptAuthKeyRegistryImpl::Factory::SetFactoryForTesting( fake_cryptauth_key_registry_factory_.get()); + fake_cryptauth_scheduler_factory_ = + std::make_unique(test_pref_service_); + CryptAuthSchedulerImpl::Factory::SetFactoryForTesting( + fake_cryptauth_scheduler_factory_.get()); + fake_cryptauth_v2_enrollment_manager_factory_ = std::make_unique( fake_client_app_metadata_provider_.get(), fake_cryptauth_key_registry_factory_.get(), - fake_cryptauth_gcm_manager_factory_.get(), test_pref_service_, + fake_cryptauth_gcm_manager_factory_.get(), + fake_cryptauth_scheduler_factory_.get(), test_pref_service_, simple_test_clock_.get()); CryptAuthV2EnrollmentManagerImpl::Factory::SetFactoryForTesting( fake_cryptauth_v2_enrollment_manager_factory_.get()); @@ -667,6 +715,7 @@ class DeviceSyncServiceTest : public ::testing::TestWithParam { SoftwareFeatureManagerImpl::Factory::SetInstanceForTesting(nullptr); DeviceSyncImpl::Factory::SetInstanceForTesting(nullptr); + NetworkHandler::Shutdown(); DBusThreadManager::Shutdown(); } @@ -1101,6 +1150,8 @@ class DeviceSyncServiceTest : public ::testing::TestWithParam { fake_client_app_metadata_provider_; std::unique_ptr fake_cryptauth_key_registry_factory_; + std::unique_ptr + fake_cryptauth_scheduler_factory_; std::unique_ptr fake_cryptauth_v2_enrollment_manager_factory_; std::unique_ptr diff --git a/chromeos/services/device_sync/fake_cryptauth_scheduler.cc b/chromeos/services/device_sync/fake_cryptauth_scheduler.cc index 049131e0a7ef8b..92fc00c0a40913 100644 --- a/chromeos/services/device_sync/fake_cryptauth_scheduler.cc +++ b/chromeos/services/device_sync/fake_cryptauth_scheduler.cc @@ -14,14 +14,24 @@ constexpr base::TimeDelta FakeCryptAuthScheduler::kDefaultRefreshPeriod; constexpr base::TimeDelta FakeCryptAuthScheduler::kDefaultTimeToNextEnrollmentRequest; -FakeCryptAuthScheduler::FakeCryptAuthScheduler(Delegate* delegate) - : CryptAuthScheduler(delegate) {} +FakeCryptAuthScheduler::FakeCryptAuthScheduler() = default; FakeCryptAuthScheduler::~FakeCryptAuthScheduler() = default; -void FakeCryptAuthScheduler::RequestEnrollmentNow() { +void FakeCryptAuthScheduler::RequestEnrollment( + const cryptauthv2::ClientMetadata::InvocationReason& invocation_reason, + const base::Optional& session_id) { + DCHECK(HasEnrollmentSchedulingStarted()); is_waiting_for_enrollment_result_ = true; - NotifyEnrollmentRequested(client_directive_policy_reference_); + + cryptauthv2::ClientMetadata client_metadata; + client_metadata.set_retry_count(num_consecutive_enrollment_failures_); + client_metadata.set_invocation_reason(invocation_reason); + if (session_id) + client_metadata.set_session_id(*session_id); + + NotifyEnrollmentRequested(client_metadata, + client_directive_policy_reference_); } void FakeCryptAuthScheduler::HandleEnrollmentResult( @@ -48,17 +58,27 @@ bool FakeCryptAuthScheduler::IsWaitingForEnrollmentResult() const { return is_waiting_for_enrollment_result_; } -size_t FakeCryptAuthScheduler::GetNumConsecutiveFailures() const { - return num_consecutive_failures_; +size_t FakeCryptAuthScheduler::GetNumConsecutiveEnrollmentFailures() const { + return num_consecutive_enrollment_failures_; } -FakeCryptAuthSchedulerDelegate::FakeCryptAuthSchedulerDelegate() = default; +FakeCryptAuthSchedulerEnrollmentDelegate:: + FakeCryptAuthSchedulerEnrollmentDelegate() + : weak_ptr_factory_(this) {} -FakeCryptAuthSchedulerDelegate::~FakeCryptAuthSchedulerDelegate() = default; +FakeCryptAuthSchedulerEnrollmentDelegate:: + ~FakeCryptAuthSchedulerEnrollmentDelegate() = default; + +base::WeakPtr +FakeCryptAuthSchedulerEnrollmentDelegate::GetWeakPtr() { + return weak_ptr_factory_.GetWeakPtr(); +} -void FakeCryptAuthSchedulerDelegate::OnEnrollmentRequested( +void FakeCryptAuthSchedulerEnrollmentDelegate::OnEnrollmentRequested( + const cryptauthv2::ClientMetadata& client_metadata, const base::Optional& client_directive_policy_reference) { + client_metadata_from_enrollment_requests_.push_back(client_metadata); policy_references_from_enrollment_requests_.push_back( client_directive_policy_reference); } diff --git a/chromeos/services/device_sync/fake_cryptauth_scheduler.h b/chromeos/services/device_sync/fake_cryptauth_scheduler.h index 67c2a46d4ad84e..745c643d102929 100644 --- a/chromeos/services/device_sync/fake_cryptauth_scheduler.h +++ b/chromeos/services/device_sync/fake_cryptauth_scheduler.h @@ -5,9 +5,11 @@ #ifndef CHROMEOS_SERVICES_DEVICE_SYNC_FAKE_CRYPTAUTH_SCHEDULER_H_ #define CHROMEOS_SERVICES_DEVICE_SYNC_FAKE_CRYPTAUTH_SCHEDULER_H_ +#include #include #include "base/macros.h" +#include "base/memory/weak_ptr.h" #include "base/optional.h" #include "base/time/time.h" #include "chromeos/services/device_sync/cryptauth_enrollment_result.h" @@ -26,8 +28,7 @@ class FakeCryptAuthScheduler : public CryptAuthScheduler { static constexpr base::TimeDelta kDefaultTimeToNextEnrollmentRequest = base::TimeDelta::FromHours(12); - explicit FakeCryptAuthScheduler(Delegate* delegate); - + FakeCryptAuthScheduler(); ~FakeCryptAuthScheduler() override; const std::vector& handled_enrollment_results() @@ -55,19 +56,22 @@ class FakeCryptAuthScheduler : public CryptAuthScheduler { time_to_next_enrollment_request_ = time_to_next_enrollment_request; } - void set_num_consecutive_failures(size_t num_consecutive_failures) { - num_consecutive_failures_ = num_consecutive_failures; + void set_num_consecutive_enrollment_failures( + size_t num_consecutive_enrollment_failures) { + num_consecutive_enrollment_failures_ = num_consecutive_enrollment_failures; } // CryptAuthScheduler: - void RequestEnrollmentNow() override; + void RequestEnrollment( + const cryptauthv2::ClientMetadata::InvocationReason& invocation_reason, + const base::Optional& session_id) override; void HandleEnrollmentResult( const CryptAuthEnrollmentResult& enrollment_result) override; base::Optional GetLastSuccessfulEnrollmentTime() const override; base::TimeDelta GetRefreshPeriod() const override; base::TimeDelta GetTimeToNextEnrollmentRequest() const override; bool IsWaitingForEnrollmentResult() const override; - size_t GetNumConsecutiveFailures() const override; + size_t GetNumConsecutiveEnrollmentFailures() const override; private: std::vector handled_enrollment_results_; @@ -77,17 +81,25 @@ class FakeCryptAuthScheduler : public CryptAuthScheduler { base::TimeDelta refresh_period_ = kDefaultRefreshPeriod; base::TimeDelta time_to_next_enrollment_request_ = kDefaultTimeToNextEnrollmentRequest; - size_t num_consecutive_failures_ = 0u; + size_t num_consecutive_enrollment_failures_ = 0u; bool is_waiting_for_enrollment_result_ = false; DISALLOW_COPY_AND_ASSIGN(FakeCryptAuthScheduler); }; -// Fake CryptAuthScheduler::Delegate implementation. -class FakeCryptAuthSchedulerDelegate : public CryptAuthScheduler::Delegate { +// Fake CryptAuthScheduler::EnrollmentDelegate implementation. +class FakeCryptAuthSchedulerEnrollmentDelegate + : public CryptAuthScheduler::EnrollmentDelegate { public: - FakeCryptAuthSchedulerDelegate(); - ~FakeCryptAuthSchedulerDelegate() override; + FakeCryptAuthSchedulerEnrollmentDelegate(); + ~FakeCryptAuthSchedulerEnrollmentDelegate() override; + + base::WeakPtr GetWeakPtr(); + + const std::vector& + client_metadata_from_enrollment_requests() const { + return client_metadata_from_enrollment_requests_; + } const std::vector>& policy_references_from_enrollment_requests() const { @@ -95,14 +107,19 @@ class FakeCryptAuthSchedulerDelegate : public CryptAuthScheduler::Delegate { } private: - // CryptAuthScheduler::Delegate: - void OnEnrollmentRequested(const base::Optional& + // CryptAuthScheduler::EnrollmentDelegate: + void OnEnrollmentRequested(const cryptauthv2::ClientMetadata& client_metadata, + const base::Optional& client_directive_policy_reference) override; + std::vector + client_metadata_from_enrollment_requests_; std::vector> policy_references_from_enrollment_requests_; + base::WeakPtrFactory + weak_ptr_factory_; - DISALLOW_COPY_AND_ASSIGN(FakeCryptAuthSchedulerDelegate); + DISALLOW_COPY_AND_ASSIGN(FakeCryptAuthSchedulerEnrollmentDelegate); }; } // namespace device_sync diff --git a/chromeos/services/device_sync/network_aware_enrollment_scheduler.cc b/chromeos/services/device_sync/network_aware_enrollment_scheduler.cc deleted file mode 100644 index 116544e7c2906b..00000000000000 --- a/chromeos/services/device_sync/network_aware_enrollment_scheduler.cc +++ /dev/null @@ -1,161 +0,0 @@ -// Copyright 2019 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "chromeos/services/device_sync/network_aware_enrollment_scheduler.h" - -#include - -#include "base/memory/ptr_util.h" -#include "base/no_destructor.h" -#include "chromeos/components/multidevice/logging/logging.h" -#include "chromeos/network/network_state.h" -#include "chromeos/network/network_state_handler.h" -#include "chromeos/services/device_sync/cryptauth_scheduler_impl.h" - -namespace chromeos { - -namespace device_sync { - -// static -NetworkAwareEnrollmentScheduler::Factory* - NetworkAwareEnrollmentScheduler::Factory::test_factory_ = nullptr; - -// static -NetworkAwareEnrollmentScheduler::Factory* -NetworkAwareEnrollmentScheduler::Factory::Get() { - if (test_factory_) - return test_factory_; - - static base::NoDestructor factory; - return factory.get(); -} - -// static -void NetworkAwareEnrollmentScheduler::Factory::SetFactoryForTesting( - Factory* test_factory) { - test_factory_ = test_factory; -} - -NetworkAwareEnrollmentScheduler::Factory::~Factory() = default; - -std::unique_ptr -NetworkAwareEnrollmentScheduler::Factory::BuildInstance( - CryptAuthScheduler::Delegate* delegate, - PrefService* pref_service, - NetworkStateHandler* network_state_handler) { - std::unique_ptr network_aware_scheduler = - base::WrapUnique( - new NetworkAwareEnrollmentScheduler(delegate, network_state_handler)); - - std::unique_ptr network_unaware_scheduler = - CryptAuthSchedulerImpl::Factory::Get()->BuildInstance( - network_aware_scheduler.get(), pref_service); - - network_aware_scheduler->network_unaware_scheduler_ = - std::move(network_unaware_scheduler); - - return network_aware_scheduler; -} - -NetworkAwareEnrollmentScheduler::NetworkAwareEnrollmentScheduler( - CryptAuthScheduler::Delegate* delegate, - NetworkStateHandler* network_state_handler) - : CryptAuthScheduler(delegate), - network_state_handler_(network_state_handler) { - DCHECK(network_state_handler_); - network_state_handler_->AddObserver(this, FROM_HERE); -} - -NetworkAwareEnrollmentScheduler::~NetworkAwareEnrollmentScheduler() { - if (network_state_handler_) - network_state_handler_->RemoveObserver(this, FROM_HERE); -} - -void NetworkAwareEnrollmentScheduler::RequestEnrollmentNow() { - network_unaware_scheduler_->RequestEnrollmentNow(); -} - -void NetworkAwareEnrollmentScheduler::HandleEnrollmentResult( - const CryptAuthEnrollmentResult& enrollment_result) { - network_unaware_scheduler_->HandleEnrollmentResult(enrollment_result); -} - -base::Optional -NetworkAwareEnrollmentScheduler::GetLastSuccessfulEnrollmentTime() const { - return network_unaware_scheduler_->GetLastSuccessfulEnrollmentTime(); -} - -base::TimeDelta NetworkAwareEnrollmentScheduler::GetRefreshPeriod() const { - return network_unaware_scheduler_->GetRefreshPeriod(); -} - -base::TimeDelta -NetworkAwareEnrollmentScheduler::GetTimeToNextEnrollmentRequest() const { - return network_unaware_scheduler_->GetTimeToNextEnrollmentRequest(); -} - -bool NetworkAwareEnrollmentScheduler::IsWaitingForEnrollmentResult() const { - return network_unaware_scheduler_->IsWaitingForEnrollmentResult() && - !pending_client_directive_policy_reference_.has_value(); -} - -size_t NetworkAwareEnrollmentScheduler::GetNumConsecutiveFailures() const { - return network_unaware_scheduler_->GetNumConsecutiveFailures(); -} - -void NetworkAwareEnrollmentScheduler::OnEnrollmentRequested( - const base::Optional& - client_directive_policy_reference) { - if (DoesMachineHaveNetworkConnectivity()) { - NotifyEnrollmentRequested(client_directive_policy_reference); - return; - } - - PA_LOG(INFO) << "NetworkAwareEnrollmentScheduler::OnEnrollmentRequested(): " - << "Enrollment scheduled while the device is offline. Waiting " - << "for online connectivity before making request."; - pending_client_directive_policy_reference_ = - client_directive_policy_reference; -} - -void NetworkAwareEnrollmentScheduler::DefaultNetworkChanged( - const NetworkState* network) { - // If enrollment has not been requested, there is nothing to do. - if (!pending_client_directive_policy_reference_) - return; - - // The updated default network may not be online. - if (!DoesMachineHaveNetworkConnectivity()) - return; - - // Now that the device has connectivity, request enrollment. Note that the - // |pending_client_directive_policy_reference_| field is cleared before the - // delegate is notified to ensure internal consistency. - base::Optional policy_reference_for_enrollment = - *pending_client_directive_policy_reference_; - pending_client_directive_policy_reference_.reset(); - NotifyEnrollmentRequested(policy_reference_for_enrollment); -} - -void NetworkAwareEnrollmentScheduler::OnShuttingDown() { - DCHECK(network_state_handler_); - network_state_handler_->RemoveObserver(this, FROM_HERE); - network_state_handler_ = nullptr; -} - -bool NetworkAwareEnrollmentScheduler::DoesMachineHaveNetworkConnectivity() { - if (!network_state_handler_) - return false; - - // TODO(khorimoto): IsConnectedState() can still return true if connected to a - // captive portal; use the "online" boolean once we fetch data via the - // networking Mojo API. See https://crbug.com/862420. - const NetworkState* default_network = - network_state_handler_->DefaultNetwork(); - return default_network && default_network->IsConnectedState(); -} - -} // namespace device_sync - -} // namespace chromeos diff --git a/chromeos/services/device_sync/network_aware_enrollment_scheduler.h b/chromeos/services/device_sync/network_aware_enrollment_scheduler.h deleted file mode 100644 index ac5cac166e11b9..00000000000000 --- a/chromeos/services/device_sync/network_aware_enrollment_scheduler.h +++ /dev/null @@ -1,95 +0,0 @@ -// Copyright 2019 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef CHROMEOS_SERVICES_DEVICE_SYNC_NETWORK_AWARE_ENROLLMENT_SCHEDULER_H_ -#define CHROMEOS_SERVICES_DEVICE_SYNC_NETWORK_AWARE_ENROLLMENT_SCHEDULER_H_ - -#include - -#include "base/macros.h" -#include "chromeos/network/network_handler.h" -#include "chromeos/network/network_state_handler_observer.h" -#include "chromeos/services/device_sync/cryptauth_scheduler.h" - -class PrefService; - -namespace chromeos { - -class NetworkStateHandler; - -namespace device_sync { - -// CryptAuthScheduler implementation which ensures that enrollment is -// only requested while online. This class owns and serves as a delegate to a -// network-unaware scheduler which requests enrollment without checking for -// network connectivity. When this class receives a request from the network- -// unaware scheduler, it: -// *Requests enrollment immediately if there is network connectivity, or -// *Caches the request until network connectivity has been attained, then -// requests enrollment. -class NetworkAwareEnrollmentScheduler : public CryptAuthScheduler, - public CryptAuthScheduler::Delegate, - public NetworkStateHandlerObserver { - public: - class Factory { - public: - static Factory* Get(); - static void SetFactoryForTesting(Factory* test_factory); - virtual ~Factory(); - virtual std::unique_ptr BuildInstance( - CryptAuthScheduler::Delegate* delegate, - PrefService* pref_service, - NetworkStateHandler* network_state_handler = - NetworkHandler::Get()->network_state_handler()); - - private: - static Factory* test_factory_; - }; - - ~NetworkAwareEnrollmentScheduler() override; - - private: - NetworkAwareEnrollmentScheduler(CryptAuthScheduler::Delegate* delegate, - NetworkStateHandler* network_state_handler); - - // CryptAuthScheduler: - void RequestEnrollmentNow() override; - void HandleEnrollmentResult( - const CryptAuthEnrollmentResult& enrollment_result) override; - base::Optional GetLastSuccessfulEnrollmentTime() const override; - base::TimeDelta GetRefreshPeriod() const override; - base::TimeDelta GetTimeToNextEnrollmentRequest() const override; - bool IsWaitingForEnrollmentResult() const override; - size_t GetNumConsecutiveFailures() const override; - - // CryptAuthScheduler::Delegate: - void OnEnrollmentRequested(const base::Optional& - client_directive_policy_reference) override; - - // NetworkStateHandlerObserver: - void DefaultNetworkChanged(const NetworkState* network) override; - void OnShuttingDown() override; - - bool DoesMachineHaveNetworkConnectivity(); - - NetworkStateHandler* network_state_handler_; - std::unique_ptr network_unaware_scheduler_; - - // The pending enrollment request, if it exists. If OnEnrollmentRequested() is - // invoked while offline, this class stores the optional PolicyReference - // parameter for that function to this instance field; then, when online - // connectivity has been restored, this class notifies *its* observer, - // forwarding this instance field. If this class does not have a pending - // enrollment, this field is null. - base::Optional> - pending_client_directive_policy_reference_; - - DISALLOW_COPY_AND_ASSIGN(NetworkAwareEnrollmentScheduler); -}; - -} // namespace device_sync - -} // namespace chromeos - -#endif // CHROMEOS_SERVICES_DEVICE_SYNC_NETWORK_AWARE_ENROLLMENT_SCHEDULER_H_ diff --git a/chromeos/services/device_sync/network_aware_enrollment_scheduler_unittest.cc b/chromeos/services/device_sync/network_aware_enrollment_scheduler_unittest.cc deleted file mode 100644 index 3013f962ef50d5..00000000000000 --- a/chromeos/services/device_sync/network_aware_enrollment_scheduler_unittest.cc +++ /dev/null @@ -1,233 +0,0 @@ -// Copyright 2019 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "chromeos/services/device_sync/network_aware_enrollment_scheduler.h" - -#include -#include -#include -#include -#include - -#include "base/macros.h" -#include "base/run_loop.h" -#include "base/test/scoped_task_environment.h" -#include "chromeos/network/network_state_handler.h" -#include "chromeos/network/network_state_test_helper.h" -#include "chromeos/services/device_sync/cryptauth_scheduler_impl.h" -#include "chromeos/services/device_sync/fake_cryptauth_scheduler.h" -#include "testing/gtest/include/gtest/gtest.h" -#include "third_party/cros_system_api/dbus/shill/dbus-constants.h" - -namespace chromeos { - -namespace device_sync { - -namespace { - -const char kWifiServiceGuid[] = "wifiGuid"; - -enum class NetworkConnectionStatus { kDisconnected, kConnecting, kConnected }; - -class FakeCryptAuthSchedulerImplFactory - : public CryptAuthSchedulerImpl::Factory { - public: - FakeCryptAuthSchedulerImplFactory() = default; - ~FakeCryptAuthSchedulerImplFactory() override = default; - - FakeCryptAuthScheduler* instance() { return instance_; } - - private: - // CryptAuthSchedulerImpl::Factory: - std::unique_ptr BuildInstance( - CryptAuthScheduler::Delegate* delegate, - PrefService* pref_service, - base::Clock* clock, - std::unique_ptr timer, - scoped_refptr task_runner) override { - EXPECT_FALSE(instance_); - auto fake_network_unaware_scheduler = - std::make_unique(delegate); - instance_ = fake_network_unaware_scheduler.get(); - return std::move(fake_network_unaware_scheduler); - } - - FakeCryptAuthScheduler* instance_ = nullptr; - - DISALLOW_COPY_AND_ASSIGN(FakeCryptAuthSchedulerImplFactory); -}; - -} // namespace - -class DeviceSyncNetworkAwareEnrollmentSchedulerTest : public testing::Test { - protected: - DeviceSyncNetworkAwareEnrollmentSchedulerTest() = default; - ~DeviceSyncNetworkAwareEnrollmentSchedulerTest() override = default; - - void SetUp() override { - fake_cryptauth_scheduler_impl_factory_ = - std::make_unique(); - CryptAuthSchedulerImpl::Factory::SetFactoryForTesting( - fake_cryptauth_scheduler_impl_factory_.get()); - - fake_delegate_ = std::make_unique(); - scheduler_ = NetworkAwareEnrollmentScheduler::Factory::Get()->BuildInstance( - fake_delegate_.get(), nullptr /* pref_service */, - helper_.network_state_handler()); - } - - void TearDown() override { - CryptAuthSchedulerImpl::Factory::SetFactoryForTesting(nullptr); - } - - void AddDisconnectedWifiNetwork() { - EXPECT_TRUE(wifi_network_service_path_.empty()); - - std::stringstream ss; - ss << "{" - << " \"GUID\": \"" << kWifiServiceGuid << "\"," - << " \"Type\": \"" << shill::kTypeWifi << "\"," - << " \"State\": \"" << shill::kStateIdle << "\"" - << "}"; - - wifi_network_service_path_ = helper_.ConfigureService(ss.str()); - } - - void SetWifiNetworkStatus(NetworkConnectionStatus connection_status) { - std::string shill_connection_status; - switch (connection_status) { - case NetworkConnectionStatus::kDisconnected: - shill_connection_status = shill::kStateIdle; - break; - case NetworkConnectionStatus::kConnecting: - shill_connection_status = shill::kStateAssociation; - break; - case NetworkConnectionStatus::kConnected: - shill_connection_status = shill::kStateReady; - break; - } - - helper_.SetServiceProperty(wifi_network_service_path_, - shill::kStateProperty, - base::Value(shill_connection_status)); - base::RunLoop().RunUntilIdle(); - } - - void RemoveWifiNetwork() { - EXPECT_TRUE(!wifi_network_service_path_.empty()); - - helper_.service_test()->RemoveService(wifi_network_service_path_); - base::RunLoop().RunUntilIdle(); - - wifi_network_service_path_.clear(); - } - - const std::vector>& - delegate_policy_references() const { - return fake_delegate_->policy_references_from_enrollment_requests(); - } - - CryptAuthScheduler* scheduler() { return scheduler_.get(); } - - FakeCryptAuthScheduler* fake_network_unaware_scheduler() { - return fake_cryptauth_scheduler_impl_factory_->instance(); - } - - private: - base::test::ScopedTaskEnvironment scoped_task_environment_; - NetworkStateTestHelper helper_{false /* use_default_devices_and_services */}; - - std::unique_ptr - fake_cryptauth_scheduler_impl_factory_; - - std::unique_ptr fake_delegate_; - std::unique_ptr scheduler_; - - std::string wifi_network_service_path_; - - DISALLOW_COPY_AND_ASSIGN(DeviceSyncNetworkAwareEnrollmentSchedulerTest); -}; - -TEST_F(DeviceSyncNetworkAwareEnrollmentSchedulerTest, - RequestReceivedWhileOnline) { - AddDisconnectedWifiNetwork(); - SetWifiNetworkStatus(NetworkConnectionStatus::kConnected); - - fake_network_unaware_scheduler()->RequestEnrollmentNow(); - EXPECT_EQ(1u, delegate_policy_references().size()); - - fake_network_unaware_scheduler()->RequestEnrollmentNow(); - EXPECT_EQ(2u, delegate_policy_references().size()); -} - -TEST_F(DeviceSyncNetworkAwareEnrollmentSchedulerTest, - RequestReceivedWhileOffline) { - AddDisconnectedWifiNetwork(); - SetWifiNetworkStatus(NetworkConnectionStatus::kDisconnected); - - // Request enrollment while disconnected using a null PolicyReference; since - // the network is disconnected, nothing should occur. - fake_network_unaware_scheduler()->set_client_directive_policy_reference( - base::nullopt); - fake_network_unaware_scheduler()->RequestEnrollmentNow(); - EXPECT_TRUE(delegate_policy_references().empty()); - EXPECT_TRUE(fake_network_unaware_scheduler()->IsWaitingForEnrollmentResult()); - EXPECT_FALSE(scheduler()->IsWaitingForEnrollmentResult()); - - // Start connecting; no enrollment should have been requested yet. - SetWifiNetworkStatus(NetworkConnectionStatus::kConnecting); - EXPECT_TRUE(delegate_policy_references().empty()); - - // Try requesting while connecting, this time passing a PolicyReference; since - // the network is only connecting, nothing should occur. - static const std::string kPolicyReferenceName = "policyReferenceName"; - cryptauthv2::PolicyReference policy_reference; - policy_reference.set_name(kPolicyReferenceName); - fake_network_unaware_scheduler()->set_client_directive_policy_reference( - policy_reference); - fake_network_unaware_scheduler()->RequestEnrollmentNow(); - EXPECT_TRUE(delegate_policy_references().empty()); - EXPECT_TRUE(fake_network_unaware_scheduler()->IsWaitingForEnrollmentResult()); - EXPECT_FALSE(scheduler()->IsWaitingForEnrollmentResult()); - - // Connect; this should cause the enrollment request to be sent. Because the - // enrollment request with |policy_reference| was sent after the one with a - // null PolicyReference, the received request should have a PolicyReference. - SetWifiNetworkStatus(NetworkConnectionStatus::kConnected); - EXPECT_EQ(1u, delegate_policy_references().size()); - EXPECT_EQ(kPolicyReferenceName, delegate_policy_references()[0]->name()); - EXPECT_TRUE(fake_network_unaware_scheduler()->IsWaitingForEnrollmentResult()); - EXPECT_TRUE(scheduler()->IsWaitingForEnrollmentResult()); - - scheduler()->HandleEnrollmentResult(CryptAuthEnrollmentResult( - CryptAuthEnrollmentResult::ResultCode::kSuccessNoNewKeysNeeded, - base::nullopt /* client_directive */)); - EXPECT_FALSE( - fake_network_unaware_scheduler()->IsWaitingForEnrollmentResult()); - EXPECT_FALSE(scheduler()->IsWaitingForEnrollmentResult()); - - // Now, remove the network entirely. - RemoveWifiNetwork(); - - // Requesting enrollment when there is no default network at all should not - // trigger an enrollment request to be sent. - fake_network_unaware_scheduler()->RequestEnrollmentNow(); - EXPECT_EQ(1u, delegate_policy_references().size()); -} - -TEST_F(DeviceSyncNetworkAwareEnrollmentSchedulerTest, - RequestReceivedWhenNoNetworksExist) { - fake_network_unaware_scheduler()->RequestEnrollmentNow(); - EXPECT_TRUE(delegate_policy_references().empty()); - - // Add a network and connect it; this should cause the pending request to be - // forwarded. - AddDisconnectedWifiNetwork(); - SetWifiNetworkStatus(NetworkConnectionStatus::kConnected); - EXPECT_EQ(1u, delegate_policy_references().size()); -} - -} // namespace device_sync - -} // namespace chromeos diff --git a/chromeos/services/device_sync/pref_names.cc b/chromeos/services/device_sync/pref_names.cc index d149af01856b8c..f2a5a57a39bf49 100644 --- a/chromeos/services/device_sync/pref_names.cc +++ b/chromeos/services/device_sync/pref_names.cc @@ -10,84 +10,77 @@ namespace device_sync { namespace prefs { -// Whether the system is scheduling device_syncs more aggressively to recover -// from the previous device_sync failure. +// (CryptAuth v1) Whether the system is scheduling device_syncs more +// aggressively to recover from the previous device_sync failure. const char kCryptAuthDeviceSyncIsRecoveringFromFailure[] = "cryptauth.device_sync.is_recovering_from_failure"; -// The timestamp of the last successful CryptAuth device_sync in seconds. +// (CryptAuth v1) The timestamp of the last successful CryptAuth device_sync in +// seconds. const char kCryptAuthDeviceSyncLastSyncTimeSeconds[] = "cryptauth.device_sync.last_device_sync_time_seconds"; -// The reason that the next device_sync is performed. This should be one of the -// enum values of cryptauth::InvocationReason in +// (CryptAuth v1) The reason that the next device_sync is performed. This should +// be one of the enum values of cryptauth::InvocationReason in // chromeos/services/device_sync/proto/cryptauth_api.proto. const char kCryptAuthDeviceSyncReason[] = "cryptauth.device_sync.reason"; -// A list of unlock keys (stored as dictionaries) synced from CryptAuth. Unlock -// Keys are phones belonging to the user that can unlock other devices, such as -// desktop PCs. +// (CryptAuth v1) A list of unlock keys (stored as dictionaries) synced from +// CryptAuth. Unlock Keys are phones belonging to the user that can unlock other +// devices, such as desktop PCs. const char kCryptAuthDeviceSyncUnlockKeys[] = "cryptauth.device_sync.unlock_keys"; -// The CryptAuth v2 Enrollment invocation reason to use when retrying a failed -// enrollment attempt. This should be one of the enum values of -// cryptauth2::ClientMetadata::InvocationReason in -// chromeos/services/device_sync/proto/cryptauth_common.proto. -const char kCryptAuthEnrollmentFailureRecoveryInvocationReason[] = - "cryptauth.enrollment.failure_recovery_invocation_reason"; - -// The session ID, sent via a GCM message, to use when retrying a failed -// enrollment attempt. -const char kCryptAuthEnrollmentFailureRecoverySessionId[] = - "cryptauth.enrollment.failure_recovery_session_id"; - -// Whether the system is scheduling enrollments more aggressively to recover -// from the previous enrollment failure. +// (CryptAuth v1) Whether the system is scheduling enrollments more aggressively +// to recover from the previous enrollment failure. const char kCryptAuthEnrollmentIsRecoveringFromFailure[] = "cryptauth.enrollment.is_recovering_from_failure"; -// The timestamp of the last successful CryptAuth enrollment in seconds. +// (CryptAuth v1) The timestamp of the last successful CryptAuth enrollment in +// seconds. const char kCryptAuthEnrollmentLastEnrollmentTimeSeconds[] = "cryptauth.enrollment.last_enrollment_time_seconds"; -// The reason that the next enrollment is performed. This should be one of the -// enum values of cryptauth::InvocationReason in +// (CryptAuth v1) The reason that the next enrollment is performed. This should +// be one of the enum values of cryptauth::InvocationReason in // chromeos/services/device_sync/proto/cryptauth_api.proto. const char kCryptAuthEnrollmentReason[] = "cryptauth.enrollment.reason"; -// The most recent ClientDirective--serialized to a string and base64 -// encoded--sent to the CryptAuthEnrollmentScheduler. -const char kCryptAuthEnrollmentSchedulerClientDirective[] = - "cryptauth.enrollment.scheduler_client_directive"; - -// The time of the last enrollment attempt. -const char kCryptAuthEnrollmentSchedulerLastEnrollmentAttemptTime[] = - "cryptauth.enrollment.scheduler_last_enrollment_attempt_time"; - -// The time of the last successful enrollment. -const char kCryptAuthEnrollmentSchedulerLastSuccessfulEnrollmentTime[] = - "cryptauth.enrollment.scheduler_last_successful_enrollment_time"; - -// The number of failed enrollments since the last successful enrollment. -const char kCryptAuthEnrollmentSchedulerNumConsecutiveFailures[] = - "cryptauth.enrollment.scheduler_num_consecutive_failures"; - -// The public key of the user and device enrolled with CryptAuth. +// (CryptAuth v1 and during migration to v2) The public key of the user and +// device enrolled with CryptAuth. const char kCryptAuthEnrollmentUserPublicKey[] = "cryptauth.enrollment.user_public_key"; -// The private key of the user and device enrolled with CryptAuth. +// (CryptAuth v1 and during migration to v2) The private key of the user and +// device enrolled with CryptAuth. const char kCryptAuthEnrollmentUserPrivateKey[] = "cryptauth.enrollment.user_private_key"; -// The GCM registration id used for receiving push messages from CryptAuth. +// (CryptAuth v1 and v2) The GCM registration id used for receiving push +// messages from CryptAuth. const char kCryptAuthGCMRegistrationId[] = "cryptauth.gcm_registration_id"; -// The dictionary of key bundles enrolled with CryptAuth, used to populate and -// persist the CryptAuthKeyRegistry. +// (CryptAuth v2) The dictionary of key bundles enrolled with CryptAuth, used to +// populate and persist the CryptAuthKeyRegistry. const char kCryptAuthKeyRegistry[] = "cryptauth.key_registry"; +// (CryptAuth v2) The most recent ClientDirective--serialized to a string and +// base64 encoded--sent to the CryptAuthScheduler. +const char kCryptAuthSchedulerClientDirective[] = + "cryptauth.scheduler.client_directive"; + +// (CryptAuth v2) The ClientMetadata of the last scheduled enrollment request. +const char kCryptAuthSchedulerNextEnrollmentRequestClientMetadata[] = + "cryptauth.scheduler.next_enrollment_request_client_metadata"; + +// (CryptAuth v2) The time of the last enrollment attempt. +const char kCryptAuthSchedulerLastEnrollmentAttemptTime[] = + "cryptauth.scheduler.last_enrollment_attempt_time"; + +// (CryptAuth v2) The time of the last successful enrollment. +const char kCryptAuthSchedulerLastSuccessfulEnrollmentTime[] = + "cryptauth.scheduler.last_successful_enrollment_time"; + } // namespace prefs } // namespace device_sync diff --git a/chromeos/services/device_sync/pref_names.h b/chromeos/services/device_sync/pref_names.h index 204384835b4e57..40d12d5fa18f5e 100644 --- a/chromeos/services/device_sync/pref_names.h +++ b/chromeos/services/device_sync/pref_names.h @@ -11,23 +11,28 @@ namespace device_sync { namespace prefs { +// Prefs for CryptAuth v1: extern const char kCryptAuthDeviceSyncLastSyncTimeSeconds[]; extern const char kCryptAuthDeviceSyncIsRecoveringFromFailure[]; extern const char kCryptAuthDeviceSyncReason[]; extern const char kCryptAuthDeviceSyncUnlockKeys[]; -extern const char kCryptAuthEnrollmentFailureRecoveryInvocationReason[]; -extern const char kCryptAuthEnrollmentFailureRecoverySessionId[]; extern const char kCryptAuthEnrollmentIsRecoveringFromFailure[]; extern const char kCryptAuthEnrollmentLastEnrollmentTimeSeconds[]; extern const char kCryptAuthEnrollmentReason[]; -extern const char kCryptAuthEnrollmentSchedulerClientDirective[]; -extern const char kCryptAuthEnrollmentSchedulerLastEnrollmentAttemptTime[]; -extern const char kCryptAuthEnrollmentSchedulerLastSuccessfulEnrollmentTime[]; -extern const char kCryptAuthEnrollmentSchedulerNumConsecutiveFailures[]; + +// Prefs for CryptAuth v1 (and during migration to v2): extern const char kCryptAuthEnrollmentUserPublicKey[]; extern const char kCryptAuthEnrollmentUserPrivateKey[]; + +// Prefs for CryptAuth v1 and v2: extern const char kCryptAuthGCMRegistrationId[]; + +// Prefs for CryptAuth v2: extern const char kCryptAuthKeyRegistry[]; +extern const char kCryptAuthSchedulerClientDirective[]; +extern const char kCryptAuthSchedulerNextEnrollmentRequestClientMetadata[]; +extern const char kCryptAuthSchedulerLastEnrollmentAttemptTime[]; +extern const char kCryptAuthSchedulerLastSuccessfulEnrollmentTime[]; } // namespace prefs