Commit 41ccf2a4 authored by Josh Nohle's avatar Josh Nohle Committed by Commit Bot

Add a delay between immediate CryptAuth v2 re-enrollment attempts

Add a five-minute delay between immediate-retry attempts in the
CryptAuth v2 Enrollment scheduler. It is probably a good idea to wait a
short amount of time before retrying a failed enrollment attempt;
otherwise, we might burn through all allotted "immediate" retry attempts
specified by CryptAuth because of a transient failure.

Also, CryptAuth throttles SyncKeys requests if more than one is sent
within a five-minute window.

Bug: 899080
Change-Id: Ie8d39b5e9d5af4464ac93200508b680e7039014c
Reviewed-on: https://chromium-review.googlesource.com/c/1495739Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Josh Nohle <nohle@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636559}
parent b4c9360c
......@@ -32,7 +32,13 @@ constexpr base::TimeDelta kDefaultRefreshPeriod = base::TimeDelta::FromDays(30);
// retry_period_millis sent by CryptAuth in SyncKeysResponse.
constexpr base::TimeDelta kDefaultRetryPeriod = base::TimeDelta::FromHours(12);
// The default number of immediate retries after a failed enrollment attempt.
// The time to wait before an "immediate" retry attempt after a failed
// enrollment attempt. Note: SyncKeys requests are throttled by CryptAuth if
// more than one is sent within a five-minute window.
constexpr base::TimeDelta kImmediateRetryDelay =
base::TimeDelta::FromMinutes(5);
// The default number of "immediate" retries after a failed enrollment attempt.
// Superseded by the ClientDirective's retry_attempts sent by CryptAuth in the
// SyncKeysResponse.
const int kDefaultMaxImmediateRetries = 3;
......@@ -264,7 +270,7 @@ PersistentEnrollmentScheduler::CalculateTimeBetweenEnrollmentRequests() const {
return GetRefreshPeriod();
if (num_consecutive_failures <= (size_t)client_directive_.retry_attempts())
return base::TimeDelta::FromMilliseconds(0);
return kImmediateRetryDelay;
return base::TimeDelta::FromMilliseconds(
client_directive_.retry_period_millis());
......
......@@ -30,6 +30,8 @@ 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;
// The time set on the scheduler's clock during set-up.
......@@ -272,7 +274,7 @@ TEST_F(DeviceSyncPersistentEnrollmentSchedulerTest, HandleFailures) {
EXPECT_EQ(base::TimeDelta::FromMilliseconds(
fake_client_directive().checkin_delay_millis()),
scheduler()->GetRefreshPeriod());
EXPECT_EQ(base::TimeDelta::FromMilliseconds(0),
EXPECT_EQ(kFakeImmediateRetryDelay,
scheduler()->GetTimeToNextEnrollmentRequest());
VerifyLastEnrollmentAttemptTimePref(kFakeTimeNow);
}
......@@ -344,7 +346,7 @@ TEST_F(DeviceSyncPersistentEnrollmentSchedulerTest, HandlePersistedFailures) {
CreateScheduler();
EXPECT_EQ(1u, scheduler()->GetNumConsecutiveFailures());
EXPECT_EQ(base::TimeDelta::FromMilliseconds(0),
EXPECT_EQ(kFakeImmediateRetryDelay,
scheduler()->GetTimeToNextEnrollmentRequest());
VerifyLastEnrollmentAttemptTimePref(kFakeTimeLaterBeforeRetryPeriod);
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment