From bc671b859a81d2eefb59a27c3a2cfd9bc4249824 Mon Sep 17 00:00:00 2001 From: Felipe Leme Date: Thu, 28 Oct 2021 17:03:12 -0700 Subject: [PATCH] Fixed DPM.logoutUser() for headless system user mode. On "traditional" devices, this method switches back to the "primary" user, which is the system user. But on devices running with headless system user mode, it should switch to the previous user instead. Test: manual verification (TestDpc/CtsVerifier) on car and phone Test: atest com.android.cts.devicepolicy.DeviceOwnerTest#testCreateAndManageUser_LogoutUser Bug: 204483021 Change-Id: I3ccbc1f31f8bcc4dae8fcb71cf5de7a7ad8882fe --- .../app/admin/DevicePolicyManager.java | 9 +- .../DevicePolicyManagerService.java | 82 +++++++++++++++++-- 2 files changed, 84 insertions(+), 7 deletions(-) diff --git a/core/java/android/app/admin/DevicePolicyManager.java b/core/java/android/app/admin/DevicePolicyManager.java index 5b01a7d2de660..ebe6c4fa0adcb 100644 --- a/core/java/android/app/admin/DevicePolicyManager.java +++ b/core/java/android/app/admin/DevicePolicyManager.java @@ -9565,7 +9565,14 @@ public class DevicePolicyManager { /** * Called by a profile owner of secondary user that is affiliated with the device to stop the - * calling user and switch back to primary. + * calling user and switch back to primary user. + * + *

Notice that on devices running with + * {@link UserManager#isHeadlessSystemUserMode() headless system user mode}, there is no primary + * user, so it switches back to the user that was in the foreground before the first call to + * {@link #switchUser(ComponentName, UserHandle)} (or fails with + * {@link UserManager#USER_OPERATION_ERROR_UNKNOWN} if that method was not called prior to this + * call). * * @param admin Which {@link DeviceAdminReceiver} this request is associated with. * @return one of the following result codes: diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index a3ff6da9782b8..081ccb442b9af 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -697,6 +697,14 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { private DevicePolicyConstants mConstants; + /** + * User to be switched to on {@code logoutUser()}. + * + *

Only used on devices with headless system user mode + */ + @GuardedBy("getLockObject()") + private @UserIdInt int mLogoutUserId = UserHandle.USER_NULL; + private static final boolean ENABLE_LOCK_GUARD = true; /** @@ -9673,6 +9681,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { mStatLogger.dump(pw); pw.println(); pw.println("Encryption Status: " + getEncryptionStatusName(getEncryptionStatus())); + pw.println("Logout user: " + getLogoutUserId()); pw.println(); if (mPendingUserCreatedCallbackTokens.isEmpty()) { @@ -10782,6 +10791,9 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { Preconditions.checkCallAuthorization(isDeviceOwner(caller)); checkCanExecuteOrThrowUnsafe(DevicePolicyManager.OPERATION_SWITCH_USER); + boolean switched = false; + // Save previous logout user id in case of failure + int logoutUserId = getLogoutUserId(); synchronized (getLockObject()) { long id = mInjector.binderClearCallingIdentity(); try { @@ -10789,16 +10801,55 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { if (userHandle != null) { userId = userHandle.getIdentifier(); } - return mInjector.getIActivityManager().switchUser(userId); + Slogf.i(LOG_TAG, "Switching to user %d (logout user is %d)", userId, logoutUserId); + setLogoutUserIdLocked(UserHandle.USER_CURRENT); + switched = mInjector.getIActivityManager().switchUser(userId); + if (!switched) { + Slogf.w(LOG_TAG, "Failed to switch to user %d", userId); + } + return switched; } catch (RemoteException e) { Slogf.e(LOG_TAG, "Couldn't switch user", e); return false; } finally { mInjector.binderRestoreCallingIdentity(id); + if (!switched) { + setLogoutUserIdLocked(logoutUserId); + } } } } + private @UserIdInt int getLogoutUserId() { + if (!mInjector.userManagerIsHeadlessSystemUserMode()) { + // mLogoutUserId is USER_SYSTEM as well, but there's no need to acquire the lock + return UserHandle.USER_SYSTEM; + } + synchronized (getLockObject()) { + return mLogoutUserId; + } + } + + private void setLogoutUserId(@UserIdInt int userId) { + if (!mInjector.userManagerIsHeadlessSystemUserMode()) return; // ignore + + synchronized (getLockObject()) { + setLogoutUserIdLocked(userId); + } + } + + @GuardedBy("getLockObject()") + private void setLogoutUserIdLocked(@UserIdInt int userId) { + if (!mInjector.userManagerIsHeadlessSystemUserMode()) return; // ignore + + if (userId == UserHandle.USER_CURRENT) { + userId = getCurrentForegroundUserId(); + } + + Slogf.d(LOG_TAG, "setLogoutUserId(): %d -> %d", mLogoutUserId, userId); + mLogoutUserId = userId; + } + @Override public int startUserInBackground(ComponentName who, UserHandle userHandle) { Objects.requireNonNull(who, "ComponentName is null"); @@ -10820,10 +10871,11 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { return UserManager.USER_OPERATION_ERROR_MAX_RUNNING_USERS; } + Slogf.i(LOG_TAG, "Starting user %d in background", userId); if (mInjector.getIActivityManager().startUserInBackground(userId)) { - Slogf.i(LOG_TAG, "Started used %d in background", userId); return UserManager.USER_OPERATION_SUCCESS; } else { + Slogf.w(LOG_TAG, "failed to start user %d in background", userId); return UserManager.USER_OPERATION_ERROR_UNKNOWN; } } catch (RemoteException e) { @@ -10871,13 +10923,30 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { return UserManager.USER_OPERATION_ERROR_MANAGED_PROFILE; } + // TODO(b/204585343): remove the headless system user check? + if (mInjector.userManagerIsHeadlessSystemUserMode() && callingUserId != mInjector + .binderWithCleanCallingIdentity(() -> getCurrentForegroundUserId())) { + Slogf.d(LOG_TAG, "logoutUser(): user %d is in background, just stopping, not switching", + callingUserId); + return stopUserUnchecked(callingUserId); + } + + int logoutUserId = getLogoutUserId(); + if (logoutUserId == UserHandle.USER_NULL) { + // Could happen on devices using headless system user mode when called before calling + // switchUser() or startUserInBackground() first + Slogf.w(LOG_TAG, "logoutUser(): could not determine which user to switch to"); + return UserManager.USER_OPERATION_ERROR_UNKNOWN; + } final long id = mInjector.binderClearCallingIdentity(); try { - if (!mInjector.getIActivityManager().switchUser(UserHandle.USER_SYSTEM)) { - Slogf.w(LOG_TAG, "Failed to switch to primary user"); - // This should never happen as target user is UserHandle.USER_SYSTEM + Slogf.i(LOG_TAG, "logoutUser(): switching to user %d", logoutUserId); + if (!mInjector.getIActivityManager().switchUser(logoutUserId)) { + Slogf.w(LOG_TAG, "Failed to switch to user %d", logoutUserId); + // This should never happen as target user is determined by getPreviousUserId() return UserManager.USER_OPERATION_ERROR_UNKNOWN; } + setLogoutUserId(UserHandle.USER_CURRENT); } catch (RemoteException e) { // Same process, should not happen. return UserManager.USER_OPERATION_ERROR_UNKNOWN; @@ -10888,7 +10957,8 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { return stopUserUnchecked(callingUserId); } - private int stopUserUnchecked(int userId) { + private int stopUserUnchecked(@UserIdInt int userId) { + Slogf.i(LOG_TAG, "Stopping user %d", userId); final long id = mInjector.binderClearCallingIdentity(); try { switch (mInjector.getIActivityManager().stopUser(userId, true /*force*/, null)) {