Get account features before taking lock (cherry-pick from master) (86d1eb9b) · Commits · e / os / android_frameworks_base

services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java

+47 −35

Original line number	Diff line number	Diff line
		@@ -5906,8 +5906,10 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
		throw new IllegalArgumentException("Invalid component " + admin
		+ " for device owner");
		}
		final boolean hasIncompatibleAccountsOrNonAdb =
		hasIncompatibleAccountsOrNonAdbNoLock(userId, admin);
		synchronized (this) {
		enforceCanSetDeviceOwnerLocked(admin, userId);
		enforceCanSetDeviceOwnerLocked(admin, userId, hasIncompatibleAccountsOrNonAdb);
		if (getActiveAdminUncheckedLocked(admin, userId) == null
		\|\| getUserData(userId).mRemovingAdmins.contains(admin)) {
		throw new IllegalArgumentException("Not active admin: " + admin);
		@@ -6098,8 +6100,10 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
		throw new IllegalArgumentException("Component " + who
		+ " not installed for userId:" + userHandle);
		}
		final boolean hasIncompatibleAccountsOrNonAdb =
		hasIncompatibleAccountsOrNonAdbNoLock(userHandle, who);
		synchronized (this) {
		enforceCanSetProfileOwnerLocked(who, userHandle);
		enforceCanSetProfileOwnerLocked(who, userHandle, hasIncompatibleAccountsOrNonAdb);

		if (getActiveAdminUncheckedLocked(who, userHandle) == null
		\|\| getUserData(userHandle).mRemovingAdmins.contains(who)) {
		@@ -6420,9 +6424,10 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
		* 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 hasIncompatibleAccountsOrNonAdb is true.
		*/
		private void enforceCanSetProfileOwnerLocked(@Nullable ComponentName owner, int userHandle) {
		private void enforceCanSetProfileOwnerLocked(@Nullable ComponentName owner, int userHandle,
		boolean hasIncompatibleAccountsOrNonAdb) {
		UserInfo info = getUserInfo(userHandle);
		if (info == null) {
		// User doesn't exist.
		@@ -6443,7 +6448,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
		int callingUid = mInjector.binderGetCallingUid();
		if (callingUid == Process.SHELL_UID \|\| callingUid == Process.ROOT_UID) {
		if ((mIsWatch \|\| hasUserSetupCompleted(userHandle))
		&& hasIncompatibleAccountsLocked(userHandle, owner)) {
		&& hasIncompatibleAccountsOrNonAdb) {
		throw new IllegalStateException("Not allowed to set the profile owner because "
		+ "there are already some accounts on the profile");
		}
		@@ -6460,14 +6465,16 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
		* 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 hasIncompatibleAccountsOrNonAdb) {
		int callingUid = mInjector.binderGetCallingUid();
		boolean isAdb = callingUid == Process.SHELL_UID \|\| callingUid == Process.ROOT_UID;
		if (!isAdb) {
		enforceCanManageProfileAndDeviceOwners();
		}

		final int code = checkSetDeviceOwnerPreConditionLocked(owner, userId, isAdb);
		final int code = checkSetDeviceOwnerPreConditionLocked(owner, userId, isAdb,
		hasIncompatibleAccountsOrNonAdb);
		switch (code) {
		case CODE_OK:
		return;
		@@ -8716,7 +8723,8 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
		* except for adb command if no accounts or additional users are present on the device.
		*/
		private synchronized @DeviceOwnerPreConditionCode int checkSetDeviceOwnerPreConditionLocked(
		@Nullable ComponentName owner, int deviceOwnerUserId, boolean isAdb) {
		@Nullable ComponentName owner, int deviceOwnerUserId, boolean isAdb,
		boolean hasIncompatibleAccountsOrNonAdb) {
		if (mOwners.hasDeviceOwner()) {
		return CODE_HAS_DEVICE_OWNER;
		}
		@@ -8736,7 +8744,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
		if (mUserManager.getUserCount() > 1) {
		return CODE_NONSYSTEM_USER_EXISTS;
		}
		if (hasIncompatibleAccountsLocked(UserHandle.USER_SYSTEM, owner)) {
		if (hasIncompatibleAccountsOrNonAdb) {
		return CODE_ACCOUNTS_NOT_EMPTY;
		}
		} else {
		@@ -8764,7 +8772,8 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
		private boolean isDeviceOwnerProvisioningAllowed(int deviceOwnerUserId) {
		synchronized (this) {
		return CODE_OK == checkSetDeviceOwnerPreConditionLocked(
		/* owner unknown / null, deviceOwnerUserId, / isAdb */ false);
		/* owner unknown / null, deviceOwnerUserId, / isAdb */ false,
		/* hasIncompatibleAccountsOrNonAdb=*/ true);
		}
		}

		@@ -9358,8 +9367,25 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
		* - Otherwise, if there's any account that does not have ..._ALLOWED, or does have
		* ..._DISALLOWED, return true.
		* - Otherwise return false.
		*
		* If the caller is not ADB, it also returns true. The returned value shouldn't be used
		* when the caller is not ADB.
		*
		* DO NOT CALL IT WITH THE DPMS LOCK HELD.
		*/
		private boolean hasIncompatibleAccountsLocked(int userId, @Nullable ComponentName owner) {
		private boolean hasIncompatibleAccountsOrNonAdbNoLock(
		int userId, @Nullable ComponentName owner) {
		final boolean isAdb = (mInjector.binderGetCallingUid() == Process.SHELL_UID)
		\|\| (mInjector.binderGetCallingUid() == Process.ROOT_UID);
		if (!isAdb) {
		return true;
		}

		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);
		@@ -9367,22 +9393,30 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
		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) {
		@@ -9390,28 +9424,6 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
		} 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);

services/tests/servicestests/src/com/android/server/devicepolicy/DpmMockContext.java

+15 −8

Original line number	Diff line number	Diff line
		@@ -16,8 +16,15 @@

		package com.android.server.devicepolicy;

		import com.android.internal.widget.LockPatternUtils;
		import static org.mockito.Matchers.anyBoolean;
		import static org.mockito.Matchers.anyInt;
		import static org.mockito.Matchers.eq;
		import static org.mockito.Mockito.mock;
		import static org.mockito.Mockito.spy;
		import static org.mockito.Mockito.when;

		import android.accounts.Account;
		import android.accounts.AccountManager;
		import android.app.IActivityManager;
		import android.app.NotificationManager;
		import android.app.backup.IBackupManager;
		@@ -44,6 +51,8 @@ import android.test.mock.MockContentResolver;
		import android.test.mock.MockContext;
		import android.view.IWindowManager;

		import com.android.internal.widget.LockPatternUtils;

		import org.junit.Assert;
		import org.mockito.invocation.InvocationOnMock;
		import org.mockito.stubbing.Answer;
		@@ -52,13 +61,6 @@ import java.io.File;
		import java.util.ArrayList;
		import java.util.List;

		import static org.mockito.Matchers.anyBoolean;
		import static org.mockito.Matchers.anyInt;
		import static org.mockito.Matchers.eq;
		import static org.mockito.Mockito.mock;
		import static org.mockito.Mockito.spy;
		import static org.mockito.Mockito.when;

		/**
		* Context used throughout DPMS tests.
		*/
		@@ -264,6 +266,7 @@ public class DpmMockContext extends MockContext {
		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;
		@@ -298,6 +301,7 @@ public class DpmMockContext extends MockContext {
		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());
		@@ -360,6 +364,7 @@ public class DpmMockContext extends MockContext {
		}
		}
		);
		when(accountManager.getAccountsAsUser(anyInt())).thenReturn(new Account[0]);


		// Create a data directory.
		@@ -418,6 +423,8 @@ public class DpmMockContext extends MockContext {
		return powerManager;
		case Context.WIFI_SERVICE:
		return wifiManager;
		case Context.ACCOUNT_SERVICE:
		return accountManager;
		}
		throw new UnsupportedOperationException();
		}