Get account features before taking lock
Test: cts-tradefed run cts --skip-device-info --skip-preconditions --skip-system-status-check com.android.compatibility.common.tradefed.targetprep.NetworkConnectivityChecker -a armeabi-v7a -m CtsDevicePolicyManagerTestCases -t com.android.cts.devicepolicy.AccountCheckHostSideTest
* without having Id49f2bd5dfa80ecf35b3a23c789100ade38c2656 *
Test: adb shell am instrument -e class com.android.server.devicepolicy.DevicePolicyManagerTest -w com.android.frameworks.servicestests
Bug: 33481725
Change-Id: I1e4dd9701a76ca366f86fdaf2fc6c282e9dbe5c1
diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
index 10c321b..ea19f41 100644
--- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
+++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
@@ -5958,8 +5958,9 @@
throw new IllegalArgumentException("Invalid component " + admin
+ " for device owner");
}
+ final boolean hasIncompatibleAccounts = hasIncompatibleAccountsNoLock(userId, admin);
synchronized (this) {
- enforceCanSetDeviceOwnerLocked(admin, userId);
+ enforceCanSetDeviceOwnerLocked(admin, userId, hasIncompatibleAccounts);
final ActiveAdmin activeAdmin = getActiveAdminUncheckedLocked(admin, userId);
if (activeAdmin == null
|| getUserData(userId).mRemovingAdmins.contains(admin)) {
@@ -6184,8 +6185,9 @@
throw new IllegalArgumentException("Component " + who
+ " not installed for userId:" + userHandle);
}
+ final boolean hasIncompatibleAccounts = hasIncompatibleAccountsNoLock(userHandle, who);
synchronized (this) {
- enforceCanSetProfileOwnerLocked(who, userHandle);
+ enforceCanSetProfileOwnerLocked(who, userHandle, hasIncompatibleAccounts);
if (getActiveAdminUncheckedLocked(who, userHandle) == null
|| getUserData(userHandle).mRemovingAdmins.contains(who)) {
@@ -6517,9 +6519,10 @@
* The profile owner can only be set before the user setup phase has completed,
* except for:
* - SYSTEM_UID
- * - adb if there are no accounts. (But see {@link #hasIncompatibleAccountsLocked})
+ * - adb unless hasIncompatibleAccounts is true.
*/
- private void enforceCanSetProfileOwnerLocked(@Nullable ComponentName owner, int userHandle) {
+ private void enforceCanSetProfileOwnerLocked(@Nullable ComponentName owner, int userHandle,
+ boolean hasIncompatibleAccounts) {
UserInfo info = getUserInfo(userHandle);
if (info == null) {
// User doesn't exist.
@@ -6539,7 +6542,7 @@
}
if (isAdb()) {
if ((mIsWatch || hasUserSetupCompleted(userHandle))
- && hasIncompatibleAccountsLocked(userHandle, owner)) {
+ && hasIncompatibleAccounts) {
throw new IllegalStateException("Not allowed to set the profile owner because "
+ "there are already some accounts on the profile");
}
@@ -6556,12 +6559,14 @@
* The Device owner can only be set by adb or an app with the MANAGE_PROFILE_AND_DEVICE_OWNERS
* permission.
*/
- private void enforceCanSetDeviceOwnerLocked(@Nullable ComponentName owner, int userId) {
+ private void enforceCanSetDeviceOwnerLocked(@Nullable ComponentName owner, int userId,
+ boolean hasIncompatibleAccounts) {
if (!isAdb()) {
enforceCanManageProfileAndDeviceOwners();
}
- final int code = checkDeviceOwnerProvisioningPreConditionLocked(owner, userId, isAdb());
+ final int code = checkDeviceOwnerProvisioningPreConditionLocked(
+ owner, userId, isAdb(), hasIncompatibleAccounts);
switch (code) {
case CODE_OK:
return;
@@ -8846,7 +8851,7 @@
* except for adb command if no accounts or additional users are present on the device.
*/
private int checkDeviceOwnerProvisioningPreConditionLocked(@Nullable ComponentName owner,
- int deviceOwnerUserId, boolean isAdb) {
+ int deviceOwnerUserId, boolean isAdb, boolean hasIncompatibleAccounts) {
if (mOwners.hasDeviceOwner()) {
return CODE_HAS_DEVICE_OWNER;
}
@@ -8866,7 +8871,7 @@
if (mUserManager.getUserCount() > 1) {
return CODE_NONSYSTEM_USER_EXISTS;
}
- if (hasIncompatibleAccountsLocked(UserHandle.USER_SYSTEM, owner)) {
+ if (hasIncompatibleAccounts) {
return CODE_ACCOUNTS_NOT_EMPTY;
}
} else {
@@ -8893,8 +8898,9 @@
private int checkDeviceOwnerProvisioningPreCondition(int deviceOwnerUserId) {
synchronized (this) {
+ // hasIncompatibleAccounts doesn't matter since the caller is not adb.
return checkDeviceOwnerProvisioningPreConditionLocked(/* owner unknown */ null,
- deviceOwnerUserId, /* isAdb= */ false);
+ deviceOwnerUserId, /* isAdb= */ false, /* hasIncompatibleAccounts=*/ true);
}
}
@@ -9791,8 +9797,15 @@
* - Otherwise, if there's any account that does not have ..._ALLOWED, or does have
* ..._DISALLOWED, return true.
* - Otherwise return false.
+ *
+ * DO NOT CALL IT WITH THE DPMS LOCK HELD.
*/
- private boolean hasIncompatibleAccountsLocked(int userId, @Nullable ComponentName owner) {
+ private boolean hasIncompatibleAccountsNoLock(int userId, @Nullable ComponentName owner) {
+ if (Thread.holdsLock(this)) {
+ Slog.wtf(LOG_TAG, "hasIncompatibleAccountsNoLock() called with the DPMS lock held.");
+ return true;
+ }
+
final long token = mInjector.binderClearCallingIdentity();
try {
final AccountManager am = AccountManager.get(mContext);
@@ -9800,22 +9813,30 @@
if (accounts.length == 0) {
return false;
}
+ synchronized (this) {
+ if (owner == null || !isAdminTestOnlyLocked(owner, userId)) {
+ Log.w(LOG_TAG,
+ "Non test-only owner can't be installed with existing accounts.");
+ return true;
+ }
+ }
+
final String[] feature_allow =
{ DevicePolicyManager.ACCOUNT_FEATURE_DEVICE_OR_PROFILE_OWNER_ALLOWED };
final String[] feature_disallow =
{ DevicePolicyManager.ACCOUNT_FEATURE_DEVICE_OR_PROFILE_OWNER_DISALLOWED };
- // Even if we find incompatible accounts along the way, we still check all accounts
- // for logging.
boolean compatible = true;
for (Account account : accounts) {
if (hasAccountFeatures(am, account, feature_disallow)) {
Log.e(LOG_TAG, account + " has " + feature_disallow[0]);
compatible = false;
+ break;
}
if (!hasAccountFeatures(am, account, feature_allow)) {
Log.e(LOG_TAG, account + " doesn't have " + feature_allow[0]);
compatible = false;
+ break;
}
}
if (compatible) {
@@ -9823,28 +9844,6 @@
} else {
Log.e(LOG_TAG, "Found incompatible accounts");
}
-
- // Then check if the owner is test-only.
- String log;
- if (owner == null) {
- // Owner is unknown. Suppose it's not test-only
- compatible = false;
- log = "Only test-only device/profile owner can be installed with accounts";
- } else if (isAdminTestOnlyLocked(owner, userId)) {
- if (compatible) {
- log = "Installing test-only owner " + owner;
- } else {
- log = "Can't install test-only owner " + owner + " with incompatible accounts";
- }
- } else {
- compatible = false;
- log = "Can't install non test-only owner " + owner + " with accounts";
- }
- if (compatible) {
- Log.w(LOG_TAG, log);
- } else {
- Log.e(LOG_TAG, log);
- }
return !compatible;
} finally {
mInjector.binderRestoreCallingIdentity(token);
diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/DpmMockContext.java b/services/tests/servicestests/src/com/android/server/devicepolicy/DpmMockContext.java
index 1247b2d..65255d9 100644
--- a/services/tests/servicestests/src/com/android/server/devicepolicy/DpmMockContext.java
+++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DpmMockContext.java
@@ -16,6 +16,8 @@
package com.android.server.devicepolicy;
+import android.accounts.Account;
+import android.accounts.AccountManager;
import android.app.IActivityManager;
import android.app.NotificationManager;
import android.app.backup.IBackupManager;
@@ -273,6 +275,7 @@
public final SettingsForMock settings;
public final MockContentResolver contentResolver;
public final TelephonyManager telephonyManager;
+ public final AccountManager accountManager;
/** Note this is a partial mock, not a real mock. */
public final PackageManager packageManager;
@@ -315,6 +318,7 @@
wifiManager = mock(WifiManager.class);
settings = mock(SettingsForMock.class);
telephonyManager = mock(TelephonyManager.class);
+ accountManager = mock(AccountManager.class);
// Package manager is huge, so we use a partial mock instead.
packageManager = spy(context.getPackageManager());
@@ -375,6 +379,7 @@
}
}
);
+ when(accountManager.getAccountsAsUser(anyInt())).thenReturn(new Account[0]);
// Create a data directory.
final File dir = new File(dataDir, "user" + userId);
@@ -464,6 +469,8 @@
return powerManager;
case Context.WIFI_SERVICE:
return wifiManager;
+ case Context.ACCOUNT_SERVICE:
+ return accountManager;
}
throw new UnsupportedOperationException();
}