From 3b4aa554ac89be21686f2c5a869d6ca04804fdae Mon Sep 17 00:00:00 2001 From: Pavel Grafov Date: Mon, 11 May 2020 16:58:58 +0100 Subject: [PATCH] Make getPersonalAppsSuspensionReasons more robust. * Make sure that if the time is rolled back after the deadline has been reached, it is not undone. When the deadline is reached it is set to -1 which is far in the past, so timezone change won't affect it. * Return sensible value in case when the deadline has just expired and the suspension itself hasn't been enacted. Previously the deadline expiration wouldn't be reflected until mAppsSuspended gets updated after all apps are suspended. * Update deadline on time changes. This makes it react to time changes via adb. * Additional debug logging to investigate further if the issue persists. Bug: 155878352 Test: atest com.android.server.devicepolicy.DevicePolicyManagerTest Change-Id: I6549f76584121df200ace811285e7a358f262869 --- .../DevicePolicyManagerService.java | 27 +++++++++++-------- .../devicepolicy/DevicePolicyManagerTest.java | 10 +++++++ .../devicepolicy/MockSystemServices.java | 8 ++++++ 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index 966694ad346c0..02f4116d9002c 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -984,6 +984,10 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { // (ACTION_DATE_CHANGED), or when manual clock adjustment is made // (ACTION_TIME_CHANGED) updateSystemUpdateFreezePeriodsRecord(/* saveIfChanged */ true); + final int userId = getManagedUserId(UserHandle.USER_SYSTEM); + if (userId >= 0) { + updatePersonalAppsSuspension(userId, mUserManager.isUserUnlocked(userId)); + } } else if (ACTION_PROFILE_OFF_DEADLINE.equals(action)) { Slog.i(LOG_TAG, "Profile off deadline alarm was triggered"); final int userId = getManagedUserId(UserHandle.USER_SYSTEM); @@ -15912,15 +15916,12 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { false /* parent */); // DO shouldn't be able to use this method. enforceProfileOwnerOfOrganizationOwnedDevice(admin); - final DevicePolicyData userData = - getUserData(getProfileParentId(mInjector.userHandleGetCallingUserId())); - if (!userData.mAppsSuspended) { - return PERSONAL_APPS_NOT_SUSPENDED; - } else { - final long deadline = admin.mProfileOffDeadline; - return makeSuspensionReasons(admin.mSuspendPersonalApps, - deadline != 0 && System.currentTimeMillis() > deadline); - } + final long deadline = admin.mProfileOffDeadline; + final int result = makeSuspensionReasons(admin.mSuspendPersonalApps, + deadline != 0 && mInjector.systemCurrentTimeMillis() > deadline); + Slog.d(LOG_TAG, String.format("getPersonalAppsSuspendedReasons user: %d; result: %d", + mInjector.userHandleGetCallingUserId(), result)); + return result; } } @@ -16033,8 +16034,12 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { int profileUserId, ActiveAdmin profileOwner, boolean unlocked) { final long now = mInjector.systemCurrentTimeMillis(); if (profileOwner.mProfileOffDeadline != 0 && now > profileOwner.mProfileOffDeadline) { - // Profile off deadline is already reached. - Slog.i(LOG_TAG, "Profile off deadline has been reached."); + Slog.i(LOG_TAG, "Profile off deadline has been reached, unlocked: " + unlocked); + if (profileOwner.mProfileOffDeadline != -1) { + // Move the deadline far to the past so that it cannot be rolled back by TZ change. + profileOwner.mProfileOffDeadline = -1; + saveSettingsLocked(profileUserId); + } return PROFILE_OFF_DEADLINE_REACHED; } boolean shouldSaveSettings = false; diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java index ed40fe756ea16..17dd2868f4bc1 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java @@ -6441,6 +6441,16 @@ public class DevicePolicyManagerTest extends DpmTestBase { verify(mContext.spiedContext).startActivityAsUser( MockUtils.checkIntentAction(ACTION_CHECK_POLICY_COMPLIANCE), MockUtils.checkUserHandle(CALLER_USER_HANDLE)); + + // Verify that correct suspension reason is reported to the DPC. + mContext.binder.callingUid = DpmMockContext.CALLER_UID; + assertThat(dpm.getPersonalAppsSuspendedReasons(admin1)) + .isEqualTo(DevicePolicyManager.PERSONAL_APPS_SUSPENDED_PROFILE_TIMEOUT); + + // Verify that rolling time back doesn't change the status. + dpms.mMockInjector.setSystemCurrentTimeMillis(PROFILE_OFF_START); + assertThat(dpm.getPersonalAppsSuspendedReasons(admin1)) + .isEqualTo(DevicePolicyManager.PERSONAL_APPS_SUSPENDED_PROFILE_TIMEOUT); } private void sendBroadcastWithUser(String action, int userHandle) throws Exception { diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/MockSystemServices.java b/services/tests/servicestests/src/com/android/server/devicepolicy/MockSystemServices.java index bbd4472f7d565..b306ff091267c 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/MockSystemServices.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/MockSystemServices.java @@ -226,6 +226,14 @@ public class MockSystemServices { uh.userType = type; uh.profileGroupId = profileGroupId; when(userManager.getUserInfo(eq(userId))).thenReturn(uh); + // Ensure there are no duplicate UserInfo records. + // TODO: fix tests so that this is not needed. + for (int i = 0; i < mUserInfos.size(); i++) { + if (mUserInfos.get(i).id == userId) { + mUserInfos.remove(i); + break; + } + } mUserInfos.add(uh); when(userManager.getUsers()).thenReturn(mUserInfos); when(userManager.getUsers(anyBoolean())).thenReturn(mUserInfos);