From b9c4862b8605b653153be29ed902142539f4ffe2 Mon Sep 17 00:00:00 2001 From: Pavel Grafov Date: Wed, 30 Oct 2019 13:29:01 +0000 Subject: [PATCH] Re-activate backup service after cleaning a profile owner Currently backup service is re-activated unconditionally when clearing a device owner but not profile owner. With this CL it should be re-activate in both cases. NB: there are two bits of state related to backup service: 1. activated or deactivated: This is out of user control, but can be changed by the admin via DPM.setBackupServiceEnabled (this name is a bit misleading here). 2. enabled or disabled: this is controlled by the user via Settings and only available when backup service is activated (see 1.) Bug: 143274029 Bug: 147997438 Test: atest CtsAdminTestCases && adb shell bmgr enabled Test: atest com.android.server.devicepolicy.DevicePolicyManagerTest Merged-In: I6f11642abe544c7df265ed7e2ad466d47796e7f9 Change-Id: I6f11642abe544c7df265ed7e2ad466d47796e7f9 (cherry picked from commit 775b26d8844540680015c5df90f8592c1864a9a8) --- .../DevicePolicyManagerService.java | 15 +++----- .../devicepolicy/DevicePolicyManagerTest.java | 35 ++++++++++++++++--- 2 files changed, 35 insertions(+), 15 deletions(-) diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index 37931be4eb10f..c3749c3b0a924 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -8058,15 +8058,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { mSecurityLogMonitor.stop(); setNetworkLoggingActiveInternal(false); deleteTransferOwnershipBundleLocked(userId); - - try { - if (mInjector.getIBackupManager() != null) { - // Reactivate backup service. - mInjector.getIBackupManager().setBackupServiceActive(UserHandle.USER_SYSTEM, true); - } - } catch (RemoteException e) { - throw new IllegalStateException("Failed reactivating backup service.", e); - } + toggleBackupServiceActive(UserHandle.USER_SYSTEM, true); } @Override @@ -8126,7 +8118,6 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { } } - private void toggleBackupServiceActive(int userId, boolean makeActive) { long ident = mInjector.binderClearCallingIdentity(); try { @@ -8135,7 +8126,8 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { .setBackupServiceActive(userId, makeActive); } } catch (RemoteException e) { - throw new IllegalStateException("Failed deactivating backup service.", e); + throw new IllegalStateException(String.format("Failed %s backup service.", + makeActive ? "activating" : "deactivating"), e); } finally { mInjector.binderRestoreCallingIdentity(ident); } @@ -8186,6 +8178,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { mOwners.removeProfileOwner(userId); mOwners.writeProfileOwner(userId); deleteTransferOwnershipBundleLocked(userId); + toggleBackupServiceActive(userId, true); } @Override 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 9ae9824da3e2b..b9ae23cdfe516 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java @@ -909,10 +909,6 @@ public class DevicePolicyManagerTest extends DpmTestBase { verify(getServices().iactivityManager, times(1)).updateDeviceOwner( eq(admin1.getPackageName())); - // TODO We should check if the caller has called clearCallerIdentity(). - verify(getServices().ibackupManager, times(1)).setBackupServiceActive( - eq(UserHandle.USER_SYSTEM), eq(false)); - verify(mContext.spiedContext, times(1)).sendBroadcastAsUser( MockUtils.checkIntentAction(DevicePolicyManager.ACTION_DEVICE_OWNER_CHANGED), MockUtils.checkUserHandle(UserHandle.USER_SYSTEM)); @@ -1141,6 +1137,37 @@ public class DevicePolicyManagerTest extends DpmTestBase { // TODO Check other calls. } + public void testDeviceOwnerBackupActivateDeactivate() throws Exception { + mContext.callerPermissions.add(permission.MANAGE_DEVICE_ADMINS); + mContext.callerPermissions.add(permission.MANAGE_PROFILE_AND_DEVICE_OWNERS); + + // Set admin1 as a DA to the secondary user. + mContext.binder.callingUid = DpmMockContext.CALLER_SYSTEM_USER_UID; + setUpPackageManagerForAdmin(admin1, DpmMockContext.CALLER_SYSTEM_USER_UID); + dpm.setActiveAdmin(admin1, /* replace =*/ false); + assertTrue(dpm.setDeviceOwner(admin1, "owner-name")); + + verify(getServices().ibackupManager, times(1)).setBackupServiceActive( + eq(UserHandle.USER_SYSTEM), eq(false)); + + dpm.clearDeviceOwnerApp(admin1.getPackageName()); + + verify(getServices().ibackupManager, times(1)).setBackupServiceActive( + eq(UserHandle.USER_SYSTEM), eq(true)); + } + + public void testProfileOwnerBackupActivateDeactivate() throws Exception { + setAsProfileOwner(admin1); + + verify(getServices().ibackupManager, times(1)).setBackupServiceActive( + eq(DpmMockContext.CALLER_USER_HANDLE), eq(false)); + + dpm.clearProfileOwner(admin1); + + verify(getServices().ibackupManager, times(1)).setBackupServiceActive( + eq(DpmMockContext.CALLER_USER_HANDLE), eq(true)); + } + public void testClearDeviceOwner_fromDifferentUser() throws Exception { mContext.callerPermissions.add(permission.MANAGE_DEVICE_ADMINS); mContext.callerPermissions.add(permission.MANAGE_USERS);