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

Commit c7712cbb authored by Felipe Leme's avatar Felipe Leme
Browse files

Improvements on UserManagerService.

- Uses ActivityManagerInternal to get current user id.
- Removed permission checks from
  isCurrentUserOrRunningProfileOfCurrentUser().
- Added unit tests for these 2 methods.

Test: atest FrameworksMockingServicesTests:com.android.server.pm.UserManagerServiceTest
Test: atest CtsMultiUserTestCases:android.multiuser.cts.UserManagerTest

Bug: 244452695
Fixes: 244765233

Change-Id: Icc13b315f138235152cc2aca988dfe38ca914c67
parent 6b86e2cb
Loading
Loading
Loading
Loading
+72 −33
Original line number Diff line number Diff line
@@ -305,6 +305,7 @@ public class UserManagerService extends IUserManager.Stub {

    private PackageManagerInternal mPmInternal;
    private DevicePolicyManagerInternal mDevicePolicyManagerInternal;
    private ActivityManagerInternal mAmInternal;

    /** Indicates that this is the 1st boot after the system user mode was changed by emulation. */
    private boolean mUpdatingSystemUserMode;
@@ -383,7 +384,7 @@ public class UserManagerService extends IUserManager.Stub {
    }

    @GuardedBy("mUsersLock")
    private final SparseArray<UserData> mUsers = new SparseArray<>();
    private final SparseArray<UserData> mUsers;

    /**
     * Map of user type names to their corresponding {@link UserTypeDetails}.
@@ -626,8 +627,6 @@ public class UserManagerService extends IUserManager.Stub {
    @GuardedBy("mUserStates")
    private final WatchedUserStates mUserStates = new WatchedUserStates();



    /**
     * Set on on devices that support background users (key) running on secondary displays (value).
     */
@@ -707,10 +706,11 @@ public class UserManagerService extends IUserManager.Stub {
        }
    }

    // TODO b/28848102 Add support for test dependencies injection
    // TODO(b/28848102) Add support for test dependencies injection
    @VisibleForTesting
    UserManagerService(Context context) {
        this(context, null, null, new Object(), context.getCacheDir());
        this(context, /* pm= */ null, /* userDataPreparer= */ null,
                /* packagesLock= */ new Object(), context.getCacheDir(), /* users= */ null);
    }

    /**
@@ -720,14 +720,18 @@ public class UserManagerService extends IUserManager.Stub {
     */
    UserManagerService(Context context, PackageManagerService pm, UserDataPreparer userDataPreparer,
            Object packagesLock) {
        this(context, pm, userDataPreparer, packagesLock, Environment.getDataDirectory());
        this(context, pm, userDataPreparer, packagesLock, Environment.getDataDirectory(),
                /* users= */ null);
    }

    private UserManagerService(Context context, PackageManagerService pm,
            UserDataPreparer userDataPreparer, Object packagesLock, File dataDir) {
    @VisibleForTesting
    UserManagerService(Context context, PackageManagerService pm,
            UserDataPreparer userDataPreparer, Object packagesLock, File dataDir,
            SparseArray<UserData> users) {
        mContext = context;
        mPm = pm;
        mPackagesLock = packagesLock;
        mUsers = users != null ? users : new SparseArray<>();
        mHandler = new MainHandler();
        mUserDataPreparer = userDataPreparer;
        mUserTypes = UserTypeFactory.getUserTypes();
@@ -1088,9 +1092,20 @@ public class UserManagerService extends IUserManager.Stub {
    @Override
    public int getProfileParentId(@UserIdInt int userId) {
        checkManageUsersPermission("get the profile parent");
        return mLocalService.getProfileParentId(userId);
        return getProfileParentIdUnchecked(userId);
    }

    private @UserIdInt int getProfileParentIdUnchecked(@UserIdInt int userId) {
        synchronized (mUsersLock) {
            UserInfo profileParent = getProfileParentLU(userId);
            if (profileParent == null) {
                return userId;
            }
            return profileParent.id;
        }
    }


    @GuardedBy("mUsersLock")
    private UserInfo getProfileParentLU(@UserIdInt int userId) {
        UserInfo profile = getUserInfoLU(userId);
@@ -1702,8 +1717,7 @@ public class UserManagerService extends IUserManager.Stub {
                    + ") is running in the foreground");
        }

        int currentUser = Binder.withCleanCallingIdentity(() -> ActivityManager.getCurrentUser());
        return currentUser == userId;
        return userId == getCurrentUserId();
    }

    @Override
@@ -1751,15 +1765,39 @@ public class UserManagerService extends IUserManager.Stub {
        }
    }

    private boolean isCurrentUserOrRunningProfileOfCurrentUser(@UserIdInt int userId) {
        int currentUserId = Binder.withCleanCallingIdentity(() -> ActivityManager.getCurrentUser());
    /**
     * Gets the current user id, calling {@link ActivityManagerInternal} directly (and without
     * performing any permission check).
     *
     * @return id of current foreground user, or {@link UserHandle#USER_NULL} if
     * {@link ActivityManagerInternal} is not available yet.
     */
    @VisibleForTesting
    int getCurrentUserId() {
        ActivityManagerInternal activityManagerInternal = getActivityManagerInternal();
        if (activityManagerInternal == null) {
            Slog.w(LOG_TAG, "getCurrentUserId() called too early, ActivityManagerInternal"
                    + " is not set yet");
            return UserHandle.USER_NULL;
        }
        return activityManagerInternal.getCurrentUserId();
    }

    /**
     * Gets whether the user is the current foreground user or a started profile of that user.
     *
     * <p>Doesn't perform any permission check.
     */
    @VisibleForTesting
    boolean isCurrentUserOrRunningProfileOfCurrentUser(@UserIdInt int userId) {
        int currentUserId = getCurrentUserId();

        if (currentUserId == userId) {
            return true;
        }

        if (isProfile(userId)) {
            int parentId = Binder.withCleanCallingIdentity(() -> getProfileParentId(userId));
        if (isProfileUnchecked(userId)) {
            int parentId = getProfileParentIdUnchecked(userId);
            if (parentId == currentUserId) {
                return isUserRunning(userId);
            }
@@ -5080,7 +5118,7 @@ public class UserManagerService extends IUserManager.Stub {
        final long ident = Binder.clearCallingIdentity();
        try {
            final UserData userData;
            int currentUser = ActivityManager.getCurrentUser();
            int currentUser = getCurrentUserId();
            if (currentUser == userId) {
                Slog.w(LOG_TAG, "Current user cannot be removed.");
                return false;
@@ -5208,7 +5246,7 @@ public class UserManagerService extends IUserManager.Stub {
                }

                // Attempt to immediately remove a non-current user
                final int currentUser = ActivityManager.getCurrentUser();
                final int currentUser = getCurrentUserId();
                if (currentUser != userId) {
                    // Attempt to remove the user. This will fail if the user is the current user
                    if (removeUserUnchecked(userId)) {
@@ -6009,11 +6047,10 @@ public class UserManagerService extends IUserManager.Stub {
            return;
        }

        final ActivityManagerInternal amInternal = LocalServices
                .getService(ActivityManagerInternal.class);
        final int currentUserId = getCurrentUserId();
        pw.print("Current user: ");
        if (amInternal != null) {
            pw.println(amInternal.getCurrentUserId());
        if (currentUserId != UserHandle.USER_NULL) {
            pw.println(currentUserId);
        } else {
            pw.println("N/A");
        }
@@ -6140,13 +6177,13 @@ public class UserManagerService extends IUserManager.Stub {
    private void dumpUser(PrintWriter pw, @UserIdInt int userId, StringBuilder sb, long now,
            long nowRealtime) {
        if (userId == UserHandle.USER_CURRENT) {
            final ActivityManagerInternal amInternal = LocalServices
                    .getService(ActivityManagerInternal.class);
            if (amInternal == null) {
            final int currentUserId = getCurrentUserId();
            pw.print("Current user: ");
            if (currentUserId == UserHandle.USER_NULL) {
                pw.println("Cannot determine current user");
                return;
            }
            userId = amInternal.getCurrentUserId();
            userId = currentUserId;
        }

        synchronized (mUsersLock) {
@@ -6392,7 +6429,7 @@ public class UserManagerService extends IUserManager.Stub {

        @Override
        public void removeAllUsers() {
            if (UserHandle.USER_SYSTEM == ActivityManager.getCurrentUser()) {
            if (UserHandle.USER_SYSTEM == getCurrentUserId()) {
                // Remove the non-system users straight away.
                removeNonSystemUsers();
            } else {
@@ -6574,13 +6611,7 @@ public class UserManagerService extends IUserManager.Stub {

        @Override
        public int getProfileParentId(@UserIdInt int userId) {
            synchronized (mUsersLock) {
                UserInfo profileParent = getProfileParentLU(userId);
                if (profileParent == null) {
                    return userId;
                }
                return profileParent.id;
            }
            return getProfileParentIdUnchecked(userId);
        }

        @Override
@@ -6909,4 +6940,12 @@ public class UserManagerService extends IUserManager.Stub {
        }
        return mDevicePolicyManagerInternal;
    }

    /** Returns the internal activity manager interface. */
    private @Nullable ActivityManagerInternal getActivityManagerInternal() {
        if (mAmInternal == null) {
            mAmInternal = LocalServices.getService(ActivityManagerInternal.class);
        }
        return mAmInternal;
    }
}
+81 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2022 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
package com.android.server;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.mockitoSession;

import android.util.Log;

import com.android.dx.mockito.inline.extended.StaticMockitoSessionBuilder;

import org.junit.After;
import org.junit.Before;
import org.mockito.MockitoSession;
import org.mockito.quality.Strictness;

/**
 * Base class to make it easier to write tests that uses {@code ExtendedMockito}.
 *
 */
public abstract class ExtendedMockitoTestCase {

    private static final String TAG = ExtendedMockitoTestCase.class.getSimpleName();

    private static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG);

    private MockitoSession mSession;

    @Before
    public void startSession() {
        if (DEBUG) {
            Log.d(TAG, "startSession()");
        }
        StaticMockitoSessionBuilder builder = mockitoSession()
                .initMocks(this)
                .strictness(Strictness.LENIENT);
        initializeSession(builder);
        mSession = builder.startMocking();
    }

    /**
     * Initializes the mockito session.
     *
     * <p>Typically used to define which classes should have static methods mocked or spied.
     */
    protected void initializeSession(StaticMockitoSessionBuilder builder) {
        if (DEBUG) {
            Log.d(TAG, "initializeSession()");
        }
    }

    @After
    public final void finishSession() {
        if (mSession == null) {
            Log.w(TAG, "finishSession(): no session");
            return;
        }
        try {
            if (DEBUG) {
                Log.d(TAG, "finishSession()");
            }
        } finally {
            // mSession.finishMocking() must ALWAYS be called (hence the over-protective try/finally
            // statements), otherwise it would cause failures on future tests as mockito
            // cannot start a session when a previous one is not finished
            mSession.finishMocking();
        }
    }
}
+221 −0
Original line number Diff line number Diff line
/*
 * Copyright (C) 2022 The Android Open Source Project
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *      http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
package com.android.server.pm;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.doNothing;
import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn;

import static com.google.common.truth.Truth.assertWithMessage;

import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.when;

import android.annotation.UserIdInt;
import android.app.ActivityManagerInternal;
import android.content.Context;
import android.content.pm.UserInfo;
import android.os.UserHandle;
import android.os.UserManager;
import android.util.Log;
import android.util.SparseArray;

import androidx.test.annotation.UiThreadTest;

import com.android.dx.mockito.inline.extended.StaticMockitoSessionBuilder;
import com.android.server.ExtendedMockitoTestCase;
import com.android.server.LocalServices;
import com.android.server.am.UserState;
import com.android.server.pm.UserManagerService.UserData;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mock;

/**
 * Run as {@code atest FrameworksMockingServicesTests:com.android.server.pm.UserManagerServiceTest}
 */
public final class UserManagerServiceTest extends ExtendedMockitoTestCase {

    private static final String TAG = UserManagerServiceTest.class.getSimpleName();

    private final Object mPackagesLock = new Object();
    private final Context mRealContext = androidx.test.InstrumentationRegistry.getInstrumentation()
            .getTargetContext();
    private Context mSpiedContext;

    private @Mock PackageManagerService mMockPms;
    private @Mock UserDataPreparer mMockUserDataPreparer;
    private @Mock ActivityManagerInternal mActivityManagerInternal;

    private final SparseArray<UserData> mUsers = new SparseArray<>();
    private UserManagerService mUms;
    private UserManagerInternal mUmi;

    @Override
    protected void initializeSession(StaticMockitoSessionBuilder builder) {
        builder
                .spyStatic(UserManager.class)
                .spyStatic(LocalServices.class);
    }

    @Before
    @UiThreadTest // Needed to initialize main handler
    public void setFixtures() {
        mSpiedContext = spy(mRealContext);

        // Called when WatchedUserStates is constructed
        doNothing().when(() -> UserManager.invalidateIsUserUnlockedCache());

        mUms = new UserManagerService(mSpiedContext, mMockPms, mMockUserDataPreparer,
                mPackagesLock, mRealContext.getDataDir(), mUsers);
        mUmi = LocalServices.getService(UserManagerInternal.class);

        assertWithMessage("LocalServices.getService(UserManagerInternal.class)").that(mUmi)
                .isNotNull();
    }

    @After
    public void resetLocalService() {
        // LocalServices follows the "Highlander rule" - There can be only one!
        LocalServices.removeServiceForTest(UserManagerInternal.class);
    }

    @Test
    public void testgetCurrentUserId_amInternalNotReady() {
        mockGetLocalService(ActivityManagerInternal.class, null);

        assertWithMessage("getCurrentUserId()").that(mUms.getCurrentUserId())
                .isEqualTo(UserHandle.USER_NULL);
    }

    @Test
    public void testgetCurrentUserId() {
        mockCurrentUser(42);

        assertWithMessage("getCurrentUserId()").that(mUms.getCurrentUserId())
                .isEqualTo(42);
    }

    @Test
    public void testIsCurrentUserOrRunningProfileOfCurrentUser_currentUser() {
        int userId = 42;
        mockCurrentUser(userId);

        assertWithMessage("isCurrentUserOrRunningProfileOfCurrentUser(%s)", userId)
                .that(mUms.isCurrentUserOrRunningProfileOfCurrentUser(userId)).isTrue();
    }

    @Test
    public void testIsCurrentUserOrRunningProfileOfCurrentUser_notCurrentUser() {
        int userId = 42;
        mockCurrentUser(108);

        assertWithMessage("isCurrentUserOrRunningProfileOfCurrentUser(%s)", userId)
                .that(mUms.isCurrentUserOrRunningProfileOfCurrentUser(userId)).isFalse();
    }

    @Test
    public void testIsCurrentUserOrRunningProfileOfCurrentUser_startedProfileOfCurrentUser() {
        int parentId = 108;
        int profileId = 42;
        addUser(parentId);
        addProfile(profileId, parentId);
        mockCurrentUser(parentId);
        setUserState(profileId, UserState.STATE_RUNNING_UNLOCKED);

        assertWithMessage("isCurrentUserOrRunningProfileOfCurrentUser(%s)", profileId)
                .that(mUms.isCurrentUserOrRunningProfileOfCurrentUser(profileId)).isTrue();
    }

    @Test
    public void testIsCurrentUserOrRunningProfileOfCurrentUser_stoppedProfileOfCurrentUser() {
        int parentId = 108;
        int profileId = 42;
        addUser(parentId);
        addProfile(profileId, parentId);
        mockCurrentUser(parentId);
        // TODO(b/244798930): should set it to STATE_STOPPING or STATE_SHUTDOWN instead
        removeUserState(profileId);

        assertWithMessage("isCurrentUserOrRunningProfileOfCurrentUser(%s)", profileId)
                .that(mUms.isCurrentUserOrRunningProfileOfCurrentUser(profileId)).isFalse();
    }

    @Test
    public void testIsCurrentUserOrRunningProfileOfCurrentUser_profileOfNonCurrentUSer() {
        int parentId = 108;
        int profileId = 42;
        int currentUserId = 666;
        addUser(parentId);
        addProfile(profileId, parentId);
        mockCurrentUser(currentUserId);

        assertWithMessage("isCurrentUserOrRunningProfileOfCurrentUser(%s)", profileId)
                .that(mUms.isCurrentUserOrRunningProfileOfCurrentUser(profileId)).isFalse();
    }

    private void mockCurrentUser(@UserIdInt int userId) {
        mockGetLocalService(ActivityManagerInternal.class, mActivityManagerInternal);

        when(mActivityManagerInternal.getCurrentUserId()).thenReturn(userId);
    }

    private <T> void mockGetLocalService(Class<T> serviceClass, T service) {
        doReturn(service).when(() -> LocalServices.getService(serviceClass));
    }

    private void addProfile(@UserIdInt int profileId, @UserIdInt int parentId) {
        TestUserData profileData = new TestUserData(profileId);
        profileData.info.flags = UserInfo.FLAG_PROFILE;
        profileData.info.profileGroupId = parentId;

        addUserData(profileData);
    }

    private void addUser(@UserIdInt int userId) {
        TestUserData userData = new TestUserData(userId);

        addUserData(userData);
    }

    private void addUserData(TestUserData userData) {
        Log.d(TAG, "Adding " + userData);
        mUsers.put(userData.info.id, userData);
    }

    private void setUserState(@UserIdInt int userId, int userState) {
        mUmi.setUserState(userId, userState);
    }

    private void removeUserState(@UserIdInt int userId) {
        mUmi.removeUserState(userId);
    }

    private static final class TestUserData extends UserData {

        @SuppressWarnings("unused")
        TestUserData(@UserIdInt int userId) {
            info = new UserInfo();
            info.id = userId;
        }

        @Override
        public String toString() {
            return "TestUserData[" + info.toFullString() + "]";
        }
    }
}