Do not announce robot account token before account ID is available

When announcing the availability of the robot account token to its
observers, DeviceOAuth2TokenService needs to know both the token and
the robot account ID. During enrollment, the token becomes available
before the account ID. Thus, DeviceOAuth2TokenService needs to wait
for the account ID to be available before announcing the token.

BUG=453828
TEST=New unit test

Review URL: https://codereview.chromium.org/892633003

Cr-Commit-Position: refs/heads/master@{#314312}

chrome/browser/chromeos/policy/affiliated_invalidation_service_provider_unittest.cc

@@ -143,12 +143,12 @@ AffiliatedInvalidationServiceProviderTest()
 void AffiliatedInvalidationServiceProviderTest::SetUp() {
   chromeos::SystemSaltGetter::Initialize();
   chromeos::DBusThreadManager::Initialize();
-  chromeos::DeviceOAuth2TokenServiceFactory::Initialize();
   ASSERT_TRUE(profile_manager_.SetUp());
 
   test_device_settings_service_.reset(new
       chromeos::ScopedTestDeviceSettingsService);
   test_cros_settings_.reset(new chromeos::ScopedTestCrosSettings);
+  chromeos::DeviceOAuth2TokenServiceFactory::Initialize();
 
   invalidation::ProfileInvalidationProviderFactory::GetInstance()->
       RegisterTestingFactory(BuildProfileInvalidationProvider);

chrome/browser/chromeos/policy/device_cloud_policy_invalidator_unittest.cc

@@ -76,7 +76,6 @@ DeviceCloudPolicyInvalidatorTest::~DeviceCloudPolicyInvalidatorTest() {
 void DeviceCloudPolicyInvalidatorTest::SetUp() {
   chromeos::SystemSaltGetter::Initialize();
   chromeos::DBusThreadManager::Initialize();
-  chromeos::DeviceOAuth2TokenServiceFactory::Initialize();
   TestingBrowserProcess::GetGlobal()->SetSystemRequestContext(
       system_request_context_.get());
   ASSERT_TRUE(profile_manager_.SetUp());
@@ -84,6 +83,8 @@ void DeviceCloudPolicyInvalidatorTest::SetUp() {
   test_device_settings_service_.reset(new
       chromeos::ScopedTestDeviceSettingsService);
   test_cros_settings_.reset(new chromeos::ScopedTestCrosSettings);
+  chromeos::DeviceOAuth2TokenServiceFactory::Initialize();
+
   scoped_refptr<ownership::MockOwnerKeyUtil> owner_key_util(
       new ownership::MockOwnerKeyUtil);
   owner_key_util->SetPublicKeyFromPrivateKey(

chrome/browser/chromeos/settings/device_oauth2_token_service.cc

@@ -8,16 +8,17 @@
 #include <vector>
 
 #include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/memory/weak_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
-#include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/chromeos/settings/token_encryptor.h"
 #include "chrome/common/pref_names.h"
 #include "chromeos/cryptohome/system_salt_getter.h"
+#include "chromeos/settings/cros_settings_names.h"
 #include "google_apis/gaia/gaia_constants.h"
 #include "google_apis/gaia/gaia_urls.h"
 #include "google_apis/gaia/google_service_auth_error.h"
@@ -42,6 +43,11 @@ struct DeviceOAuth2TokenService::PendingRequest {
   const ScopeSet scopes;
 };
 
+void DeviceOAuth2TokenService::OnServiceAccountIdentityChanged() {
+  if (!GetRobotAccountId().empty() && !refresh_token_.empty())
+    FireRefreshTokenAvailable(GetRobotAccountId());
+}
+
 DeviceOAuth2TokenService::DeviceOAuth2TokenService(
     net::URLRequestContextGetter* getter,
     PrefService* local_state)
@@ -49,6 +55,12 @@ DeviceOAuth2TokenService::DeviceOAuth2TokenService(
       local_state_(local_state),
       state_(STATE_LOADING),
       max_refresh_token_validation_retries_(3),
+      service_account_identity_subscription_(
+          CrosSettings::Get()->AddSettingsObserver(
+              kServiceAccountIdentity,
+              base::Bind(
+                  &DeviceOAuth2TokenService::OnServiceAccountIdentityChanged,
+                  base::Unretained(this))).Pass()),
       weak_ptr_factory_(this) {
   // Pull in the system salt.
   SystemSaltGetter::Get()->GetSystemSalt(
@@ -75,7 +87,12 @@ void DeviceOAuth2TokenService::SetAndSaveRefreshToken(
   bool waiting_for_salt = state_ == STATE_LOADING;
   refresh_token_ = refresh_token;
   state_ = STATE_VALIDATION_PENDING;
-  FireRefreshTokenAvailable(GetRobotAccountId());
+
+  // If the robot account ID is not available yet, do not announce the token. It
+  // will be done from OnServiceAccountIdentityChanged() once the robot account
+  // ID becomes available as well.
+  if (!GetRobotAccountId().empty())
+    FireRefreshTokenAvailable(GetRobotAccountId());
 
   token_save_callbacks_.push_back(result_callback);
   if (!waiting_for_salt) {
@@ -261,10 +278,10 @@ void DeviceOAuth2TokenService::CheckRobotAccountId(
     const std::string& gaia_robot_id) {
   // Make sure the value returned by GetRobotAccountId has been validated
   // against current device settings.
-  switch (CrosSettings::Get()->PrepareTrustedValues(
-      base::Bind(&DeviceOAuth2TokenService::CheckRobotAccountId,
-                 weak_ptr_factory_.GetWeakPtr(),
-                 gaia_robot_id))) {
+  switch (CrosSettings::Get()->PrepareTrustedValues(base::Bind(
+              &DeviceOAuth2TokenService::CheckRobotAccountId,
+              weak_ptr_factory_.GetWeakPtr(),
+              gaia_robot_id))) {
     case CrosSettingsProvider::TRUSTED:
       // All good, compare account ids below.
       break;

chrome/browser/chromeos/settings/device_oauth2_token_service.h

@@ -15,6 +15,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/stl_util.h"
 #include "base/time/time.h"
+#include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "google_apis/gaia/gaia_oauth_client.h"
 #include "google_apis/gaia/oauth2_token_service.h"
 #include "net/url_request/url_request_context_getter.h"
@@ -104,6 +105,9 @@ class DeviceOAuth2TokenService : public OAuth2TokenService,
     STATE_TOKEN_VALID,
   };
 
+  // Invoked by CrosSettings when the robot account ID becomes available.
+  void OnServiceAccountIdentityChanged();
+
   // Use DeviceOAuth2TokenServiceFactory to get an instance of this class.
   // Ownership of |token_encryptor| will be taken.
   explicit DeviceOAuth2TokenService(net::URLRequestContextGetter* getter,
@@ -161,6 +165,9 @@ class DeviceOAuth2TokenService : public OAuth2TokenService,
 
   scoped_ptr<gaia::GaiaOAuthClient> gaia_oauth_client_;
 
+  scoped_ptr<CrosSettings::ObserverSubscription>
+      service_account_identity_subscription_;
+
   base::WeakPtrFactory<DeviceOAuth2TokenService> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(DeviceOAuth2TokenService);

chrome/browser/chromeos/settings/device_oauth2_token_service_factory.h

@@ -26,13 +26,13 @@ class DeviceOAuth2TokenServiceFactory {
 
   // Called by ChromeBrowserMainPartsChromeOS in order to bootstrap the
   // DeviceOAuth2TokenService instance after the required global data is
-  // available (local state and request context getter).
+  // available (local state, request context getter and CrosSettings).
   static void Initialize();
 
   // Called by ChromeBrowserMainPartsChromeOS in order to shutdown the
   // DeviceOAuth2TokenService instance and cancel all in-flight requests before
-  // the required global data is destroyed (local state and request context
-  // getter).
+  // the required global data is destroyed (local state, request context getter
+  // and CrosSettings).
   static void Shutdown();
 
  private:

chrome/browser/chromeos/settings/device_oauth2_token_service_unittest.cc

@@ -28,10 +28,29 @@
 #include "net/url_request/test_url_fetcher_factory.h"
 #include "net/url_request/url_fetcher_delegate.h"
 #include "net/url_request/url_request_test_util.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace chromeos {
 
+namespace {
+
+class MockOAuth2TokenServiceObserver : public OAuth2TokenService::Observer {
+ public:
+  MockOAuth2TokenServiceObserver();
+  ~MockOAuth2TokenServiceObserver() override;
+
+  MOCK_METHOD1(OnRefreshTokenAvailable, void(const std::string&));
+};
+
+MockOAuth2TokenServiceObserver::MockOAuth2TokenServiceObserver() {
+}
+
+MockOAuth2TokenServiceObserver::~MockOAuth2TokenServiceObserver() {
+}
+
+}  // namespace
+
 static const int kOAuthTokenServiceUrlFetcherId = 0;
 static const int kValidatorUrlFetcherId = gaia::GaiaOAuthClient::kUrlFetcherId;
 
@@ -96,6 +115,7 @@ class DeviceOAuth2TokenServiceTest : public testing::Test {
   }
 
   void TearDown() override {
+    oauth2_service_.reset();
     CrosSettings::Shutdown();
     TestingBrowserProcess::GetGlobal()->SetBrowserPolicyConnector(NULL);
     content::BrowserThread::GetBlockingPool()->FlushForTesting();
@@ -430,4 +450,25 @@ TEST_F(DeviceOAuth2TokenServiceTest, RefreshTokenValidation_Retry) {
   AssertConsumerTokensAndErrors(1, 1);
 }
 
+TEST_F(DeviceOAuth2TokenServiceTest, DoNotAnnounceTokenWithoutAccountID) {
+  CreateService();
+
+  testing::StrictMock<MockOAuth2TokenServiceObserver> observer;
+  oauth2_service_->AddObserver(&observer);
+
+  // Make a token available during enrollment. Verify that the token is not
+  // announced yet.
+  oauth2_service_->SetAndSaveRefreshToken(
+      "test-token", DeviceOAuth2TokenService::StatusCallback());
+  testing::Mock::VerifyAndClearExpectations(&observer);
+
+  // Also make the robot account ID available. Verify that the token is
+  // announced now.
+  EXPECT_CALL(observer, OnRefreshTokenAvailable("robot@example.com"));
+  SetRobotAccountId("robot@example.com");
+  testing::Mock::VerifyAndClearExpectations(&observer);
+
+  oauth2_service_->RemoveObserver(&observer);
+}
+
 }  // namespace chromeos