Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 606da777 authored by Makoto Onuki's avatar Makoto Onuki
Browse files

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
parent ab99c11c
Loading
Loading
Loading
Loading
+34 −35
Original line number Diff line number Diff line
@@ -5958,8 +5958,9 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
            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 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
            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 @@ 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 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 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
        }
        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 @@ 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 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 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
     * 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 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
                    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 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {

    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 @@ 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.
     *
     * 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 @@ 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) {
@@ -9823,28 +9844,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);
+7 −0
Original line number Diff line number Diff line
@@ -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 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;
@@ -315,6 +318,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());
@@ -375,6 +379,7 @@ public class DpmMockContext extends MockContext {
                    }
                }
        );
        when(accountManager.getAccountsAsUser(anyInt())).thenReturn(new Account[0]);

        // Create a data directory.
        final File dir = new File(dataDir, "user" + userId);
@@ -464,6 +469,8 @@ public class DpmMockContext extends MockContext {
                return powerManager;
            case Context.WIFI_SERVICE:
                return wifiManager;
            case Context.ACCOUNT_SERVICE:
                return accountManager;
        }
        throw new UnsupportedOperationException();
    }