Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Get account features before taking lock (cherry-pick from master)

Browse Source 
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: Ie2fe9aea87d1a7167581f4cd74ae063ef24a4567
Merged-in: I1e4dd9701a76ca366f86fdaf2fc6c282e9dbe5c1
This commit is contained in:
Makoto Onuki
2016-12-15 14:26:55 -08:00
parent 2dd60e81ad
commit fd24353d75
2 changed files with 62 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files

82
services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
View File

@@ -5906,8 +5906,10 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
throw new IllegalArgumentException("Invalid component " + admin throw new IllegalArgumentException("Invalid component " + admin
+ " for device owner"); + " for device owner");
} }
final boolean hasIncompatibleAccountsOrNonAdb =
hasIncompatibleAccountsOrNonAdbNoLock(userId, admin);
synchronized (this) { synchronized (this) {
enforceCanSetDeviceOwnerLocked(admin, userId); enforceCanSetDeviceOwnerLocked(admin, userId, hasIncompatibleAccountsOrNonAdb);
if (getActiveAdminUncheckedLocked(admin, userId) == null if (getActiveAdminUncheckedLocked(admin, userId) == null
|| getUserData(userId).mRemovingAdmins.contains(admin)) { || getUserData(userId).mRemovingAdmins.contains(admin)) {
throw new IllegalArgumentException("Not active admin: " + admin); throw new IllegalArgumentException("Not active admin: " + admin);
@@ -6098,8 +6100,10 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
throw new IllegalArgumentException("Component " + who throw new IllegalArgumentException("Component " + who
+ " not installed for userId:" + userHandle); + " not installed for userId:" + userHandle);
} }
final boolean hasIncompatibleAccountsOrNonAdb =
hasIncompatibleAccountsOrNonAdbNoLock(userHandle, who);
synchronized (this) { synchronized (this) {
enforceCanSetProfileOwnerLocked(who, userHandle); enforceCanSetProfileOwnerLocked(who, userHandle, hasIncompatibleAccountsOrNonAdb);
if (getActiveAdminUncheckedLocked(who, userHandle) == null if (getActiveAdminUncheckedLocked(who, userHandle) == null
|| getUserData(userHandle).mRemovingAdmins.contains(who)) { || 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, * The profile owner can only be set before the user setup phase has completed,
* except for: * except for:
* - SYSTEM_UID * - 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); UserInfo info = getUserInfo(userHandle);
if (info == null) { if (info == null) {
// User doesn't exist. // User doesn't exist.
@@ -6443,7 +6448,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
int callingUid = mInjector.binderGetCallingUid(); int callingUid = mInjector.binderGetCallingUid();
if (callingUid == Process.SHELL_UID || callingUid == Process.ROOT_UID) { if (callingUid == Process.SHELL_UID || callingUid == Process.ROOT_UID) {
if ((mIsWatch || hasUserSetupCompleted(userHandle)) if ((mIsWatch || hasUserSetupCompleted(userHandle))
&& hasIncompatibleAccountsLocked(userHandle, owner)) { && hasIncompatibleAccountsOrNonAdb) {
throw new IllegalStateException("Not allowed to set the profile owner because " throw new IllegalStateException("Not allowed to set the profile owner because "
+ "there are already some accounts on the profile"); + "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 * The Device owner can only be set by adb or an app with the MANAGE_PROFILE_AND_DEVICE_OWNERS
* permission. * permission.
*/ */
private void enforceCanSetDeviceOwnerLocked(@Nullable ComponentName owner, int userId) { private void enforceCanSetDeviceOwnerLocked(@Nullable ComponentName owner, int userId,
boolean hasIncompatibleAccountsOrNonAdb) {
int callingUid = mInjector.binderGetCallingUid(); int callingUid = mInjector.binderGetCallingUid();
boolean isAdb = callingUid == Process.SHELL_UID || callingUid == Process.ROOT_UID; boolean isAdb = callingUid == Process.SHELL_UID || callingUid == Process.ROOT_UID;
if (!isAdb) { if (!isAdb) {
enforceCanManageProfileAndDeviceOwners(); enforceCanManageProfileAndDeviceOwners();
} }
final int code = checkSetDeviceOwnerPreConditionLocked(owner, userId, isAdb); final int code = checkSetDeviceOwnerPreConditionLocked(owner, userId, isAdb,
hasIncompatibleAccountsOrNonAdb);
switch (code) { switch (code) {
case CODE_OK: case CODE_OK:
return; 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. * except for adb command if no accounts or additional users are present on the device.
*/ */
private synchronized @DeviceOwnerPreConditionCode int checkSetDeviceOwnerPreConditionLocked( private synchronized @DeviceOwnerPreConditionCode int checkSetDeviceOwnerPreConditionLocked(
@Nullable ComponentName owner, int deviceOwnerUserId, boolean isAdb) { @Nullable ComponentName owner, int deviceOwnerUserId, boolean isAdb,
boolean hasIncompatibleAccountsOrNonAdb) {
if (mOwners.hasDeviceOwner()) { if (mOwners.hasDeviceOwner()) {
return CODE_HAS_DEVICE_OWNER; return CODE_HAS_DEVICE_OWNER;
} }
@@ -8736,7 +8744,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
if (mUserManager.getUserCount() > 1) { if (mUserManager.getUserCount() > 1) {
return CODE_NONSYSTEM_USER_EXISTS; return CODE_NONSYSTEM_USER_EXISTS;
} }
if (hasIncompatibleAccountsLocked(UserHandle.USER_SYSTEM, owner)) { if (hasIncompatibleAccountsOrNonAdb) {
return CODE_ACCOUNTS_NOT_EMPTY; return CODE_ACCOUNTS_NOT_EMPTY;
} }
} else { } else {
@@ -8764,7 +8772,8 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
private boolean isDeviceOwnerProvisioningAllowed(int deviceOwnerUserId) { private boolean isDeviceOwnerProvisioningAllowed(int deviceOwnerUserId) {
synchronized (this) { synchronized (this) {
return CODE_OK == checkSetDeviceOwnerPreConditionLocked( 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 * - Otherwise, if there's any account that does not have ..._ALLOWED, or does have
* ..._DISALLOWED, return true. * ..._DISALLOWED, return true.
* - Otherwise return false. * - 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(); final long token = mInjector.binderClearCallingIdentity();
try { try {
final AccountManager am = AccountManager.get(mContext); final AccountManager am = AccountManager.get(mContext);
@@ -9367,22 +9393,30 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
if (accounts.length == 0) { if (accounts.length == 0) {
return false; 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 = final String[] feature_allow =
{ DevicePolicyManager.ACCOUNT_FEATURE_DEVICE_OR_PROFILE_OWNER_ALLOWED }; { DevicePolicyManager.ACCOUNT_FEATURE_DEVICE_OR_PROFILE_OWNER_ALLOWED };
final String[] feature_disallow = final String[] feature_disallow =
{ DevicePolicyManager.ACCOUNT_FEATURE_DEVICE_OR_PROFILE_OWNER_DISALLOWED }; { 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; boolean compatible = true;
for (Account account : accounts) { for (Account account : accounts) {
if (hasAccountFeatures(am, account, feature_disallow)) { if (hasAccountFeatures(am, account, feature_disallow)) {
Log.e(LOG_TAG, account + " has " + feature_disallow[0]); Log.e(LOG_TAG, account + " has " + feature_disallow[0]);
compatible = false; compatible = false;
break;
} }
if (!hasAccountFeatures(am, account, feature_allow)) { if (!hasAccountFeatures(am, account, feature_allow)) {
Log.e(LOG_TAG, account + " doesn't have " + feature_allow[0]); Log.e(LOG_TAG, account + " doesn't have " + feature_allow[0]);
compatible = false; compatible = false;
break;
} }
} }
if (compatible) { if (compatible) {
@@ -9390,28 +9424,6 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
} else { } else {
Log.e(LOG_TAG, "Found incompatible accounts"); 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; return !compatible;
} finally { } finally {
mInjector.binderRestoreCallingIdentity(token); mInjector.binderRestoreCallingIdentity(token);

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

@@ -16,8 +16,15 @@
package com.android.server.devicepolicy; 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.IActivityManager;
import android.app.NotificationManager; import android.app.NotificationManager;
import android.app.backup.IBackupManager; import android.app.backup.IBackupManager;
@@ -44,6 +51,8 @@ import android.test.mock.MockContentResolver;
import android.test.mock.MockContext; import android.test.mock.MockContext;
import android.view.IWindowManager; import android.view.IWindowManager;
import com.android.internal.widget.LockPatternUtils;
import org.junit.Assert; import org.junit.Assert;
import org.mockito.invocation.InvocationOnMock; import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer; import org.mockito.stubbing.Answer;
@@ -52,13 +61,6 @@ import java.io.File;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.List; 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. * Context used throughout DPMS tests.
*/ */
@@ -264,6 +266,7 @@ public class DpmMockContext extends MockContext {
public final SettingsForMock settings; public final SettingsForMock settings;
public final MockContentResolver contentResolver; public final MockContentResolver contentResolver;
public final TelephonyManager telephonyManager; public final TelephonyManager telephonyManager;
public final AccountManager accountManager;
/** Note this is a partial mock, not a real mock. */ /** Note this is a partial mock, not a real mock. */
public final PackageManager packageManager; public final PackageManager packageManager;
@@ -298,6 +301,7 @@ public class DpmMockContext extends MockContext {
wifiManager = mock(WifiManager.class); wifiManager = mock(WifiManager.class);
settings = mock(SettingsForMock.class); settings = mock(SettingsForMock.class);
telephonyManager = mock(TelephonyManager.class); telephonyManager = mock(TelephonyManager.class);
accountManager = mock(AccountManager.class);
// Package manager is huge, so we use a partial mock instead. // Package manager is huge, so we use a partial mock instead.
packageManager = spy(context.getPackageManager()); 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. // Create a data directory.
@@ -418,6 +423,8 @@ public class DpmMockContext extends MockContext {
return powerManager; return powerManager;
case Context.WIFI_SERVICE: case Context.WIFI_SERVICE:
return wifiManager; return wifiManager;
case Context.ACCOUNT_SERVICE:
return accountManager;
} }
throw new UnsupportedOperationException(); throw new UnsupportedOperationException();
} }