arc: Implement safe access to ArcAuthService.

chrome/browser/chromeos/arc/arc_auth_service.cc

 // Copyright 2015 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 "chrome/browser/chromeos/arc/arc_auth_service.h"
 
 #include <utility>
 
 #include "ash/shelf/shelf_delegate.h"
 #include "ash/shell.h"
@@ skipping 39 matching lines @@
 namespace arc {
 
 namespace {
 
 // Weak pointer.  This class is owned by ArcServiceManager.
 ArcAuthService* g_arc_auth_service = nullptr;
 
 // Skip creating UI in unit tests
 bool g_disable_ui_for_testing = false;
 
+// Skip checking for cryptohome data ephemeral users.
+bool g_allow_cryptohome_data_ephemeral_user_for_testing = false;

[khmel, 2016/07/18 20:23:42]

In most browser tests user is logged as Ephemeral user, which is disabled by
Arc. As I investigated there is no easy ways to make user non-Ephemeral. There
are Login browser tests that may do this however they have different hierarchy,
incompatible with AppList browser tests.

https://cs.chromium.org/chromium/src/chrome/browser/chromeos/login/login_mana...

There is yet another sample ArcAuthBrowser Tests. However it is support is very
massive and I am not sure that this makes sense to add to AppList browser tests.

https://cs.chromium.org/chromium/src/chrome/browser/chromeos/arc/arc_auth_ser...


Easiest way to solve this is to allow Ephemeral users for Arc tests.

WDYT?

[xiyuan, 2016/07/18 20:59:12]

Sounds like UserManagerBase::IsUserCryptohomeDataEphemeral is not doing a
correct job here. We take a shortcut there by checking whether user is persisted
in the list to test ephemeral. This is not 100% correct. We probably should also
check AreEphemeralUsersEnabled() in [1].

[1]
https://cs.chromium.org/chromium/src/components/user_manager/user_manager_bas...

Can you try if that could work without adding
g_allow_cryptohome_data_ephemeral_user_for_testing ?

[khmel, 2016/07/18 22:15:11]

Yes, this helps

+
 // Use specified ash::ShelfDelegate for unit tests.
 ash::ShelfDelegate* g_shelf_delegate_for_testing = nullptr;
 
 // The Android management check is disabled by default, it's used only for
 // testing.
 bool g_enable_check_android_management_for_testing = false;
 
 const char kStateNotInitialized[] = "NOT_INITIALIZED";
 const char kStateStopped[] = "STOPPED";
 const char kStateFetchingCode[] = "FETCHING_CODE";
@@ skipping 56 matching lines @@
   registry->RegisterBooleanPref(prefs::kArcSignedIn, false);
   registry->RegisterBooleanPref(prefs::kArcBackupRestoreEnabled, true);
 }
 
 // static
 void ArcAuthService::DisableUIForTesting() {
   g_disable_ui_for_testing = true;
 }
 
 // static
+void ArcAuthService::AllowCryptohomeDataEphemeralUserForTesting() {
+  g_allow_cryptohome_data_ephemeral_user_for_testing = true;
+}
+
+// static
 void ArcAuthService::SetShelfDelegateForTesting(
     ash::ShelfDelegate* shelf_delegate) {
   g_shelf_delegate_for_testing = shelf_delegate;
 }
 
 // static
 bool ArcAuthService::IsOptInVerificationDisabled() {
   return base::CommandLine::ForCurrentProcess()->HasSwitch(
       chromeos::switches::kDisableArcOptInVerification);
 }
@@ skipping 25 matching lines @@
     return false;
   }
 
   user_manager::User const* const user =
       chromeos::ProfileHelper::Get()->GetUserByProfile(profile);
   if (!user || !user->HasGaiaAccount()) {
     VLOG(1) << "Users without GAIA accounts are not supported in ARC.";
     return false;
   }
 
-  if (user_manager::UserManager::Get()
+  if (!g_allow_cryptohome_data_ephemeral_user_for_testing &&
+      user_manager::UserManager::Get()
           ->IsCurrentUserCryptohomeDataEphemeral()) {
     VLOG(2) << "Users with ephemeral data are not supported in Arc.";
     return false;
   }
 
   return true;
 }
 
 void ArcAuthService::OnInstanceReady() {
   arc_bridge_service()->auth()->instance()->Init(
@@ skipping 143 matching lines @@
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   return profile_ != nullptr;
 }
 
 void ArcAuthService::OnPrimaryUserProfilePrepared(Profile* profile) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(profile && profile != profile_);
 
   Shutdown();
 
-  profile_ = profile;
-  SetState(State::STOPPED);
-
   if (!IsAllowedForProfile(profile))
     return;
 
   // TODO(khmel): Move this to IsAllowedForProfile.
   if (IsArcDisabledForEnterprise() && IsAccountManaged(profile)) {
     VLOG(2) << "Enterprise users are not supported in ARC.";
     return;
   }
 
+  profile_ = profile;
+  SetState(State::STOPPED);
+
   PrefServiceSyncableFromProfile(profile_)->AddSyncedPrefObserver(
       prefs::kArcEnabled, this);
 
   context_.reset(new ArcAuthContext(this, profile_));
 
   // In case UI is disabled we assume that ARC is opted-in.
   if (IsOptInVerificationDisabled()) {
     auth_code_.clear();
     StartArc();
     return;
@@ skipping 263 matching lines @@
 }
 
 bool ArcAuthService::IsArcManaged() const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(profile_);
   return profile_->GetPrefs()->IsManagedPreference(prefs::kArcEnabled);
 }
 
 bool ArcAuthService::IsArcEnabled() const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  if (!IsAllowed())
+    return false;
+
   DCHECK(profile_);
   return profile_->GetPrefs()->GetBoolean(prefs::kArcEnabled);
 }
 
 void ArcAuthService::EnableArc() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(profile_);
 
   if (IsArcEnabled()) {
     OnOptInPreferenceChanged();
@@ skipping 101 matching lines @@
       return os << kStateFetchingCode;
     case ArcAuthService::State::ACTIVE:
       return os << kStateActive;
     default:
       NOTREACHED();
       return os;
   }
 }
 
 }  // namespace arc