Add functionality to fetch auth codes for Secondary Accounts

A design doc and sequence diagram are linked in the bug id.

Bug: 871690
Test: browser_tests --gtest_filter="*ArcAuthServiceTest*"
Change-Id: If62c7c06d107ac0b6ea73ffa8da9b7ca42365937
Reviewed-on: https://chromium-review.googlesource.com/1172123
Commit-Queue: Kush Sinha <sinhak@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Lutz Justen <ljusten@chromium.org>
Reviewed-by: Mattias Nissler <mnissler@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585460}

chrome/browser/chromeos/arc/auth/arc_auth_service.h

@@ -12,6 +12,7 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
+#include "base/optional.h"
 #include "chrome/browser/chromeos/account_mapper_util.h"
 #include "chrome/browser/chromeos/arc/auth/arc_active_directory_enrollment_token_fetcher.h"
 #include "chromeos/account_manager/account_manager.h"
@@ -19,6 +20,7 @@
 #include "components/arc/connection_observer.h"
 #include "components/keyed_service/core/keyed_service.h"
 
+class AccountTrackerService;
 class Profile;
 
 namespace content {
@@ -31,8 +33,10 @@ class SharedURLLoaderFactory;
 
 namespace arc {
 
-class ArcFetcherBase;
+class ArcAuthCodeFetcher;
+class ArcBackgroundAuthCodeFetcher;
 class ArcBridgeService;
+class ArcFetcherBase;
 
 // Implementation of ARC authorization.
 class ArcAuthService : public KeyedService,
@@ -60,7 +64,9 @@ class ArcAuthService : public KeyedService,
                                bool initial_signin) override;
   void OnSignInCompleteDeprecated() override;
   void OnSignInFailedDeprecated(mojom::ArcSignInStatus reason) override;
-  void RequestAccountInfo(bool initial_signin) override;
+  void RequestAccountInfo(
+      bool initial_signin,
+      const base::Optional<std::string>& account_name) override;
   void ReportMetrics(mojom::MetricsType metrics_type, int32_t value) override;
   void ReportAccountCheckStatus(mojom::AccountCheckStatus status) override;
   void ReportSupervisionChangeStatus(
@@ -78,12 +84,42 @@ class ArcAuthService : public KeyedService,
   void SkipMergeSessionForTesting();
 
  private:
-  // Callbacks when auth info is fetched.
-  void OnEnrollmentTokenFetched(
+  // Callback when Active Directory Enrollment Token is fetched.
+  void OnActiveDirectoryEnrollmentTokenFetched(
+      ArcActiveDirectoryEnrollmentTokenFetcher* fetcher,
       ArcActiveDirectoryEnrollmentTokenFetcher::Status status,
       const std::string& enrollment_token,
       const std::string& user_id);
-  void OnAuthCodeFetched(bool success, const std::string& auth_code);
+
+  // Issues a request for fetching AccountInfo for the Device Account.
+  // |initial_signin| denotes whether this is the initial ARC++ provisioning
+  // flow or a subsequent sign-in.
+  void FetchDeviceAccountInfo(bool initial_signin);
+
+  // Callback for |FetchDeviceAccountInfo|.
+  // |fetcher| is a pointer to the object that issues this callback. Used for
+  // deleting pending requests from |pending_token_requests_|.
+  // |success| and |auth_code| are the callback parameters passed by
+  // |ArcBackgroundAuthCodeFetcher::Fetch|.
+  void OnDeviceAccountAuthCodeFetched(ArcAuthCodeFetcher* fetcher,
+                                      bool success,
+                                      const std::string& auth_code);
+
+  // Issues a request for fetching AccountInfo for a Secondary Account
+  // represented by |account_name|. |account_name| is the account identifier
+  // used by ARC++/Android.
+  void FetchSecondaryAccountInfo(const std::string& account_name);
+
+  // Callback for |FetchSecondaryAccountInfo|, issued by
+  // |ArcBackgroundAuthCodeFetcher::Fetch|. |account_name| is the account
+  // identifier used by ARC++/Android. |fetcher| is used to identify the
+  // |ArcBackgroundAuthCodeFetcher| instance that completed the request.
+  // |success| and |auth_code| are arguments passed by
+  // |ArcBackgroundAuthCodeFetcher::Fetch| callback.
+  void OnSecondaryAccountAuthCodeFetched(const std::string& account_name,
+                                         ArcBackgroundAuthCodeFetcher* fetcher,
+                                         bool success,
+                                         const std::string& auth_code);
 
   // Called to let ARC container know the account info.
   void OnAccountInfoReady(mojom::AccountInfoPtr account_info,
@@ -100,15 +136,30 @@ class ArcAuthService : public KeyedService,
   bool IsDeviceAccount(
       const chromeos::AccountManager::AccountKey& account_key) const;
 
+  // Creates an |ArcBackgroundAuthCodeFetcher| for |account_id|. Can be used for
+  // Device Account and Secondary Accounts. |initial_signin| denotes whether the
+  // fetcher is being created for the initial ARC++ provisioning flow or for a
+  // subsequent sign-in.
+  std::unique_ptr<ArcBackgroundAuthCodeFetcher>
+  CreateArcBackgroundAuthCodeFetcher(const std::string& account_id,
+                                     bool initial_signin);
+
+  // Deletes a completed enrollment token / auth code fetch request from
+  // |pending_token_requests_|.
+  void DeletePendingTokenRequest(ArcFetcherBase* fetcher);
+
   // Non-owning pointers.
   Profile* const profile_;
   chromeos::AccountManager* account_manager_ = nullptr;
+  AccountTrackerService* const account_tracker_service_;
+  ArcBridgeService* const arc_bridge_service_;
 
   chromeos::AccountMapperUtil account_mapper_util_;
-  ArcBridgeService* const arc_bridge_service_;
+
   scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory_;
 
-  std::unique_ptr<ArcFetcherBase> fetcher_;
+  // A list of pending enrollment token / auth code requests.
+  std::vector<std::unique_ptr<ArcFetcherBase>> pending_token_requests_;
 
   bool skip_merge_session_for_testing_ = false;
 

chrome/browser/chromeos/arc/auth/arc_auth_service_browsertest.cc

@@ -11,6 +11,7 @@
 #include "base/files/file_path.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "base/run_loop.h"
 #include "base/test/scoped_feature_list.h"
 #include "chrome/browser/browser_process.h"
@@ -96,9 +97,15 @@ class FakeAuthInstance : public mojom::AuthInstance {
     std::move(done_closure_).Run();
   }
 
-  void RequestAccountInfo(base::Closure done_closure) {
-    done_closure_ = done_closure;
-    host_->RequestAccountInfo(true);
+  void RequestAccountInfo(base::OnceClosure done_closure) {
+    done_closure_ = std::move(done_closure);
+    host_->RequestAccountInfo(true /* initial_signin */, base::nullopt);
+  }
+
+  void RequestSecondaryAccountInfo(base::OnceClosure done_closure,
+                                   const std::string& account_name) {
+    done_closure_ = std::move(done_closure);
+    host_->RequestAccountInfo(false /* initial_signin */, account_name);
   }
 
   void OnSecondaryAccountUpserted(const std::string& account_name) override {
@@ -121,7 +128,7 @@ class FakeAuthInstance : public mojom::AuthInstance {
  private:
   mojom::AuthHostPtr host_;
   mojom::AccountInfoPtr account_info_;
-  base::Closure done_closure_;
+  base::OnceClosure done_closure_;
 };
 
 class ArcAuthServiceTest : public InProcessBrowserTest {
@@ -276,6 +283,11 @@ class ArcAuthServiceTest : public InProcessBrowserTest {
 
     ASSERT_TRUE(account_info.IsValid());
     account_tracker_service->SeedAccountInfo(account_info);
+
+    FakeProfileOAuth2TokenService* token_service =
+        static_cast<FakeProfileOAuth2TokenService*>(
+            ProfileOAuth2TokenServiceFactory::GetForProfile(profile()));
+    token_service->UpdateCredentials(account_info.account_id, kRefreshToken);
   }
 
   Profile* profile() { return profile_.get(); }
@@ -310,7 +322,7 @@ IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest, SuccessfulBackgroundFetch) {
   SetAccountAndProfile(user_manager::USER_TYPE_REGULAR);
   test_url_loader_factory().AddResponse(
       arc::kAuthTokenExchangeEndPoint,
-      "{ \"token\" : \"" + std::string(kFakeAuthCode) + "\" }");
+      R"({ "token" : ")" + std::string(kFakeAuthCode) + R"(" })");
 
   base::RunLoop run_loop;
   auth_instance().RequestAccountInfo(run_loop.QuitClosure());
@@ -324,6 +336,7 @@ IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest, SuccessfulBackgroundFetch) {
             auth_instance().account_info()->account_type);
   EXPECT_FALSE(auth_instance().account_info()->enrollment_token);
   EXPECT_FALSE(auth_instance().account_info()->is_managed);
+  EXPECT_FALSE(auth_instance().account_info()->is_secondary_account);
 }
 
 IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest,
@@ -378,6 +391,30 @@ IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest,
   EXPECT_EQ(0, auth_instance().num_account_upserted_calls_);
 }
 
+IN_PROC_BROWSER_TEST_F(ArcAuthServiceTest, FetchSecondaryAccountInfoSucceeds) {
+  const std::string gaia_id = "123999";
+  const std::string email = "email.111@gmail.com";
+
+  SetAccountAndProfile(user_manager::USER_TYPE_REGULAR);
+  SeedAccountInfo(gaia_id, email);
+  test_url_loader_factory().AddResponse(
+      arc::kAuthTokenExchangeEndPoint,
+      R"({ "token" : ")" + std::string(kFakeAuthCode) + R"(" })");
+
+  base::RunLoop run_loop;
+  auth_instance().RequestSecondaryAccountInfo(run_loop.QuitClosure(), email);
+  run_loop.Run();
+
+  ASSERT_TRUE(auth_instance().account_info());
+  EXPECT_EQ(email, auth_instance().account_info()->account_name.value());
+  EXPECT_EQ(kFakeAuthCode, auth_instance().account_info()->auth_code.value());
+  EXPECT_EQ(mojom::ChromeAccountType::USER_ACCOUNT,
+            auth_instance().account_info()->account_type);
+  EXPECT_FALSE(auth_instance().account_info()->enrollment_token);
+  EXPECT_FALSE(auth_instance().account_info()->is_managed);
+  EXPECT_TRUE(auth_instance().account_info()->is_secondary_account);
+}
+
 class ArcRobotAccountAuthServiceTest : public ArcAuthServiceTest {
  public:
   ArcRobotAccountAuthServiceTest() = default;
@@ -541,7 +578,7 @@ IN_PROC_BROWSER_TEST_F(ArcAuthServiceChildAccountTest, ChildAccountFetch) {
   EXPECT_TRUE(profile()->IsChild());
   test_url_loader_factory().AddResponse(
       arc::kAuthTokenExchangeEndPoint,
-      "{ \"token\" : \"" + std::string(kFakeAuthCode) + "\" }");
+      R"({ "token" : ")" + std::string(kFakeAuthCode) + R"(" })");
 
   base::RunLoop run_loop;
   auth_instance().RequestAccountInfo(run_loop.QuitClosure());

chrome/browser/chromeos/arc/auth/arc_background_auth_code_fetcher.cc

@@ -230,10 +230,12 @@ void ArcBackgroundAuthCodeFetcher::ResetFetchers() {
 void ArcBackgroundAuthCodeFetcher::ReportResult(
     const std::string& auth_code,
     OptInSilentAuthCode uma_status) {
-  if (initial_signin_)
+  if (initial_signin_) {
     UpdateSilentAuthCodeUMA(uma_status);
-  else
+  } else {
+    // TODO(sinhak): Check if we need to migrate this / create a new metric.
     UpdateReauthorizationSilentAuthCodeUMA(uma_status);
+  }
   std::move(callback_).Run(!auth_code.empty(), auth_code);
 }
 

components/arc/common/auth.mojom

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Next MinVersion: 16
+// Next MinVersion: 17
 
 module arc.mojom;
 
@@ -170,6 +170,7 @@ enum MetricsType {
 };
 
 // The necessary information for Android to sign in and provision itself.
+// Next ordinal value: 6.
 struct AccountInfo {
   // Name of account, used to map to existing Android account.
   [MinVersion=9] string? account_name@4;
@@ -189,6 +190,9 @@ struct AccountInfo {
 
   // Whether the account is managed from Chrome OS.
   bool is_managed@2;
+
+  // Whether this account is a Chrome OS Secondary Account.
+  [MinVersion=16] bool is_secondary_account@5;
 };
 
 // Next Method ID: 12.
@@ -209,7 +213,11 @@ interface AuthHost {
   // Asynchronously requests an authorization code, as well as the account
   // information. If |initial_signin| is true then that means request is for
   // initial signin flow. Otherwise it is used for reauthorization flow.
-  [MinVersion=5] RequestAccountInfo@7([MinVersion=11] bool initial_signin);
+  // Optionally, an |account_name| can be provided to fetch |AccountInfo| for
+  // Secondary Accounts in Chrome OS Account Manager. Absence of |account_name|
+  // will default to the Device Account in Chrome OS.
+  [MinVersion=5] RequestAccountInfo@7([MinVersion=11] bool initial_signin,
+                                      [MinVersion=16] string? account_name);
 
   // Reports metrics to Chrome to be recorded in UMA.
   [MinVersion=7] ReportMetrics@8(MetricsType metrics_type, int32 value);