Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(47)

Unified Diff: chrome/browser/chromeos/arc/arc_auth_service.cc

Issue 1892873002: Add CheckAndroidManagement to ARC sign-in flow. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@check_android_management
Patch Set: Added OnShutdownBridge to observer. Created 4 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/chromeos/arc/arc_auth_service.cc
diff --git a/chrome/browser/chromeos/arc/arc_auth_service.cc b/chrome/browser/chromeos/arc/arc_auth_service.cc
index ed8322b5492e76e9447249f769d40035f91dd0ad..741d5c0d0ada9f305629b48dc8761392a2f02a36 100644
--- a/chrome/browser/chromeos/arc/arc_auth_service.cc
+++ b/chrome/browser/chromeos/arc/arc_auth_service.cc
@@ -10,10 +10,14 @@
#include "base/bind_helpers.h"
bartfab (slow) 2016/05/02 14:55:48 Nit: No longer used.
Polina Bondarenko 2016/05/03 13:41:54 Done.
#include "base/command_line.h"
#include "base/lazy_instance.h"
+#include "base/strings/string16.h"
bartfab (slow) 2016/05/02 14:55:48 Nit: Where is this used?
Polina Bondarenko 2016/05/03 13:41:54 It's used in ShowUI() and it's calls.
#include "base/strings/stringprintf.h"
#include "base/threading/thread_checker.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/browser_process_platform_part.h"
#include "chrome/browser/chromeos/arc/arc_auth_notification.h"
#include "chrome/browser/chromeos/arc/arc_optin_uma.h"
+#include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
#include "chrome/browser/chromeos/profiles/profile_helper.h"
#include "chrome/browser/extensions/extension_util.h"
#include "chrome/browser/policy/profile_policy_connector.h"
@@ -29,6 +33,8 @@
#include "chrome/grit/generated_resources.h"
#include "chromeos/chromeos_switches.h"
#include "components/arc/arc_bridge_service.h"
+#include "components/policy/core/browser/browser_policy_connector.h"
+#include "components/policy/core/common/cloud/device_management_service.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "components/prefs/pref_service.h"
#include "components/signin/core/browser/profile_oauth2_token_service.h"
@@ -60,13 +66,17 @@ const char kArcSupportStorageId[] = "arc_support";
// Skip creating UI in unit tests
bool disable_ui_for_testing = false;
+// Do check Android management requirement in browser tests.
bartfab (slow) 2016/05/02 14:55:48 The comment says "do check" but the default value
Polina Bondarenko 2016/05/03 13:41:54 Done.
+bool enable_check_android_management_for_testing = false;
+
const char kStateStopped[] = "STOPPED";
const char kStateFetchingCode[] = "FETCHING_CODE";
const char kStateActive[] = "ACTIVE";
+
} // namespace
ArcAuthService::ArcAuthService(ArcBridgeService* bridge_service)
- : ArcService(bridge_service), binding_(this) {
+ : ArcService(bridge_service), binding_(this), weak_ptr_factory_(this) {
DCHECK(!arc_auth_service);
DCHECK(thread_checker.Get().CalledOnValidThread());
@@ -111,6 +121,11 @@ bool ArcAuthService::IsOptInVerificationDisabled() {
chromeos::switches::kDisableArcOptInVerification);
}
+// static
+void ArcAuthService::EnableCheckAndroidManagementForTesting() {
+ enable_check_android_management_for_testing = true;
+}
+
void ArcAuthService::OnAuthInstanceReady() {
arc_bridge_service()->auth_instance()->Init(
binding_.CreateInterfacePtrAndBind());
@@ -242,13 +257,23 @@ void ArcAuthService::OnPrimaryUserProfilePrepared(Profile* profile) {
profile_, GURL(site_url));
CHECK(storage_partition_);
+ // Get token service and account ID to fetch auth tokens.
+ token_service_ = ProfileOAuth2TokenServiceFactory::GetForProfile(profile_);
+ SigninManagerBase* signin_manager =
bartfab (slow) 2016/05/02 14:55:48 Nit: const pointer
Polina Bondarenko 2016/05/03 13:41:54 Done.
+ SigninManagerFactory::GetForProfile(profile_);
+ CHECK(token_service_ && signin_manager);
bartfab (slow) 2016/05/02 14:55:48 Nit: #include "base/logging.h"
Polina Bondarenko 2016/05/03 13:41:54 Done.
+ account_id_ = signin_manager->GetAuthenticatedAccountId();
+
// In case UI is disabled we assume that ARC is opted-in.
if (!IsOptInVerificationDisabled()) {
+ if (!disable_ui_for_testing || enable_check_android_management_for_testing)
+ StartAndroidManagementClient();
+
pref_change_registrar_.Init(profile_->GetPrefs());
pref_change_registrar_.Add(
prefs::kArcEnabled,
base::Bind(&ArcAuthService::OnOptInPreferenceChanged,
- base::Unretained(this)));
+ weak_ptr_factory_.GetWeakPtr()));
if (profile_->GetPrefs()->GetBoolean(prefs::kArcEnabled)) {
OnOptInPreferenceChanged();
} else {
@@ -287,6 +312,9 @@ void ArcAuthService::Shutdown() {
pref_service_syncable->RemoveObserver(this);
pref_service_syncable->RemoveSyncedPrefObserver(prefs::kArcEnabled, this);
}
+ // Destroy here, because |android_management_client_| is used for each ARC
bartfab (slow) 2016/05/02 14:55:48 I do not understand what this comment is trying to
Polina Bondarenko 2016/05/03 13:41:54 It doesn't have to, removed. I tried to show that
+ // connect, even if ARC is signed-in.
+ android_management_client_.reset();
pref_change_registrar_.RemoveAll();
profile_ = nullptr;
}
@@ -316,8 +344,7 @@ void ArcAuthService::OnMergeSessionSuccess(const std::string& data) {
DCHECK(thread_checker.Get().CalledOnValidThread());
DCHECK(!initial_opt_in_);
- context_prepared_ = true;
- ShowUI(UIPage::LSO_PROGRESS, base::string16());
+ CheckAndroidManagement();
}
void ArcAuthService::OnMergeSessionFailure(
@@ -367,8 +394,13 @@ void ArcAuthService::OnOptInPreferenceChanged() {
initial_opt_in_ = true;
StartUI();
} else {
- // Ready to start Arc.
- StartArc();
+ // Ready to start Arc, but check android management first.
+ if (!disable_ui_for_testing ||
+ enable_check_android_management_for_testing) {
+ CheckAndroidManagement();
+ } else {
+ StartArc();
+ }
}
UpdateEnabledStateUMA(true);
@@ -381,10 +413,13 @@ void ArcAuthService::OnOptInPreferenceChanged() {
}
void ArcAuthService::ShutdownBridge() {
+ FOR_EACH_OBSERVER(Observer, observer_list_, OnShutdownBridge());
playstore_launcher_.reset();
auth_callback_.reset();
ubertoken_fethcher_.reset();
merger_fetcher_.reset();
+ token_service_ = nullptr;
+ account_id_ = "";
arc_bridge_service()->Shutdown();
SetState(State::STOPPED);
}
@@ -468,7 +503,7 @@ void ArcAuthService::StartLso() {
void ArcAuthService::CancelAuthCode() {
DCHECK(thread_checker.Get().CalledOnValidThread());
- if (state_ != State::FETCHING_CODE)
+ if (state_ != State::FETCHING_CODE && ui_page_ != UIPage::ERROR)
return;
// Update UMA with user cancel only if error is not currently shown.
@@ -491,17 +526,10 @@ void ArcAuthService::DisableArc() {
void ArcAuthService::PrepareContext() {
DCHECK(thread_checker.Get().CalledOnValidThread());
- // Get auth token to continue.
- ProfileOAuth2TokenService* token_service =
- ProfileOAuth2TokenServiceFactory::GetForProfile(profile_);
- SigninManagerBase* signin_manager =
- SigninManagerFactory::GetForProfile(profile_);
- CHECK(token_service && signin_manager);
- const std::string& account_id = signin_manager->GetAuthenticatedAccountId();
ubertoken_fethcher_.reset(
- new UbertokenFetcher(token_service, this, GaiaConstants::kChromeOSSource,
+ new UbertokenFetcher(token_service_, this, GaiaConstants::kChromeOSSource,
storage_partition_->GetURLRequestContext()));
- ubertoken_fethcher_->StartFetchingToken(account_id);
+ ubertoken_fethcher_->StartFetchingToken(account_id_);
}
void ArcAuthService::StartUI() {
@@ -528,6 +556,66 @@ void ArcAuthService::OnPrepareContextFailed() {
UpdateOptInCancelUMA(OptInCancelReason::NETWORK_ERROR);
}
+void ArcAuthService::StartAndroidManagementClient() {
+ policy::BrowserPolicyConnectorChromeOS* const connector =
+ g_browser_process->platform_part()->browser_policy_connector_chromeos();
+ policy::DeviceManagementService* const service =
+ connector->device_management_service();
+ service->ScheduleInitialization(0);
+ android_management_client_.reset(new policy::AndroidManagementClient(
+ service, g_browser_process->system_request_context(), account_id_,
+ token_service_));
+}
+
+void ArcAuthService::CheckAndroidManagement() {
+ // Do not send requests for Chrome OS managed users.
+ if (policy::ProfilePolicyConnectorFactory::GetForBrowserContext(profile_)
+ ->IsManaged()) {
+ OnAndroidManagementChecked(
+ policy::AndroidManagementClient::Result::RESULT_UNMANAGED);
bartfab (slow) 2016/05/02 14:55:48 It is a bit odd semantically that for a *managed*
Polina Bondarenko 2016/05/03 13:41:54 Yes, fixed to a plain code. Hopefully it doesn't l
bartfab (slow) 2016/05/03 14:25:18 You could always extract these four lines to a hel
Polina Bondarenko 2016/05/03 14:55:44 Done.
+ return;
+ }
+
+ // Do not send requests for well-known consumer domains.
+ if (policy::BrowserPolicyConnector::IsNonEnterpriseUser(
+ profile_->GetProfileUserName())) {
+ OnAndroidManagementChecked(
+ policy::AndroidManagementClient::Result::RESULT_UNMANAGED);
+ return;
+ }
+
+ android_management_client_->StartCheckAndroidManagement(
+ base::Bind(&ArcAuthService::OnAndroidManagementChecked,
+ weak_ptr_factory_.GetWeakPtr()));
+}
+
+void ArcAuthService::OnAndroidManagementChecked(
+ policy::AndroidManagementClient::Result result) {
+ switch (result) {
+ case policy::AndroidManagementClient::Result::RESULT_UNMANAGED:
+ context_prepared_ = true;
+ if (!profile_->GetPrefs()->GetBoolean(prefs::kArcSignedIn))
+ ShowUI(UIPage::LSO_PROGRESS, base::string16());
+ else
+ StartArc();
+ break;
+ case policy::AndroidManagementClient::Result::RESULT_MANAGED:
+ ShutdownBridgeAndShowUI(
+ UIPage::ERROR,
+ l10n_util::GetStringUTF16(IDS_ARC_ANDROID_MANAGEMENT_REQUIRED_ERROR));
+ UpdateOptInCancelUMA(OptInCancelReason::ANDROID_MANAGEMENT_REQUIRED);
+ break;
+ case policy::AndroidManagementClient::Result::RESULT_ERROR:
+ ShutdownBridgeAndShowUI(
+ UIPage::ERROR,
+ l10n_util::GetStringUTF16(IDS_ARC_SERVER_COMMUNICATION_ERROR));
+ UpdateOptInCancelUMA(OptInCancelReason::NETWORK_ERROR);
+ break;
+ default:
+ NOTREACHED();
+ }
+}
+
std::ostream& operator<<(std::ostream& os, const ArcAuthService::State& state) {
switch (state) {
case ArcAuthService::State::STOPPED:

Powered by Google App Engine
This is Rietveld 408576698