From a855429993433a007f200465a571bab7364dbff4 Mon Sep 17 00:00:00 2001 From: Pavel Grafov Date: Tue, 9 May 2023 14:08:23 +0100 Subject: [PATCH] Allow rolling back Turn Off Work 2.0 To facilitate it, previous effective value is stored in DevicePolicyData, with "true" default value to avoid storing it. When the system boots, if the current flag value is different from the stored one, suspension is updated. An upgrade step sets the stored state to "false" once, so that devices upgrading from pre-U get their work apps suspended if in quiet mode. Bug: 265683382 Test: Manual, via device_config Test: atest PolicyVersionUpgraderTest Change-Id: I711ace4cc2f8da6ce4f85aa0f0cde740dba2ba02 --- .../server/pm/SuspendPackageHelper.java | 2 - .../server/devicepolicy/DevicePolicyData.java | 20 ++++++++- .../DevicePolicyManagerService.java | 43 +++++++++++-------- .../devicepolicy/PolicyVersionUpgrader.java | 16 +++++++ .../PolicyVersionUpgraderTest.java | 20 ++++++++- 5 files changed, 78 insertions(+), 23 deletions(-) diff --git a/services/core/java/com/android/server/pm/SuspendPackageHelper.java b/services/core/java/com/android/server/pm/SuspendPackageHelper.java index 9127a93a46ee9..ba825774daf64 100644 --- a/services/core/java/com/android/server/pm/SuspendPackageHelper.java +++ b/services/core/java/com/android/server/pm/SuspendPackageHelper.java @@ -697,8 +697,6 @@ public final class SuspendPackageHelper { Computer snapshot, int userId, boolean suspend) { final Set toSuspend = packagesToSuspendInQuietMode(snapshot, userId); if (!suspend) { - // Note: this method is called from DPMS constructor to suspend apps on upgrade, but - // it won't enter here because 'suspend' will equal 'true'. final DevicePolicyManagerInternal dpm = LocalServices.getService(DevicePolicyManagerInternal.class); if (dpm != null) { diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyData.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyData.java index 6d51bd7c77703..d9604395dd98f 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyData.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyData.java @@ -16,6 +16,8 @@ package com.android.server.devicepolicy; +import static com.android.server.devicepolicy.DevicePolicyManagerService.DEFAULT_KEEP_PROFILES_RUNNING_FLAG; + import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.UserIdInt; @@ -70,6 +72,7 @@ class DevicePolicyData { private static final String TAG_PASSWORD_TOKEN_HANDLE = "password-token"; private static final String TAG_PROTECTED_PACKAGES = "protected-packages"; private static final String TAG_BYPASS_ROLE_QUALIFICATIONS = "bypass-role-qualifications"; + private static final String TAG_KEEP_PROFILES_RUNNING = "keep-profiles-running"; private static final String ATTR_VALUE = "value"; private static final String ATTR_ALIAS = "alias"; private static final String ATTR_ID = "id"; @@ -193,6 +196,12 @@ class DevicePolicyData { // starts. String mNewUserDisclaimer = NEW_USER_DISCLAIMER_NOT_NEEDED; + /** + * Effective state of the feature flag. It is updated to the current configuration value + * during boot and doesn't change value after than unless overridden by test code. + */ + boolean mEffectiveKeepProfilesRunning = DEFAULT_KEEP_PROFILES_RUNNING_FLAG; + DevicePolicyData(@UserIdInt int userId) { mUserId = userId; } @@ -392,6 +401,12 @@ class DevicePolicyData { out.endTag(null, TAG_BYPASS_ROLE_QUALIFICATIONS); } + if (policyData.mEffectiveKeepProfilesRunning != DEFAULT_KEEP_PROFILES_RUNNING_FLAG) { + out.startTag(null, TAG_KEEP_PROFILES_RUNNING); + out.attributeBoolean(null, ATTR_VALUE, policyData.mEffectiveKeepProfilesRunning); + out.endTag(null, TAG_KEEP_PROFILES_RUNNING); + } + out.endTag(null, "policies"); out.endDocument(); @@ -574,7 +589,10 @@ class DevicePolicyData { parser.getAttributeBoolean(null, ATTR_VALUE, false); } else if (TAG_BYPASS_ROLE_QUALIFICATIONS.equals(tag)) { policy.mBypassDevicePolicyManagementRoleQualifications = true; - policy.mCurrentRoleHolder = parser.getAttributeValue(null, ATTR_VALUE); + policy.mCurrentRoleHolder = parser.getAttributeValue(null, ATTR_VALUE); + } else if (TAG_KEEP_PROFILES_RUNNING.equals(tag)) { + policy.mEffectiveKeepProfilesRunning = parser.getAttributeBoolean( + null, ATTR_VALUE, DEFAULT_KEEP_PROFILES_RUNNING_FLAG); // Deprecated tags below } else if (TAG_PROTECTED_PACKAGES.equals(tag)) { if (policy.mUserControlDisabledPackages == null) { diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index 02c6d6849ccab..b1306be9c889a 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -678,7 +678,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { // to decide whether an existing policy in the {@link #DEVICE_POLICIES_XML} needs to // be upgraded. See {@link PolicyVersionUpgrader} on instructions how to add an upgrade // step. - static final int DPMS_VERSION = 4; + static final int DPMS_VERSION = 5; static { SECURE_SETTINGS_ALLOWLIST = new ArraySet<>(); @@ -875,7 +875,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { // TODO(b/265683382) remove the flag after rollout. private static final String KEEP_PROFILES_RUNNING_FLAG = "enable_keep_profiles_running"; - private static final boolean DEFAULT_KEEP_PROFILES_RUNNING_FLAG = true; + public static final boolean DEFAULT_KEEP_PROFILES_RUNNING_FLAG = true; private static final String ENABLE_WORK_PROFILE_TELEPHONY_FLAG = "enable_work_profile_telephony"; @@ -889,12 +889,6 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { private static final String APPLICATION_EXEMPTIONS_FLAG = "application_exemptions"; private static final boolean DEFAULT_APPLICATION_EXEMPTIONS_FLAG = true; - /** - * This feature flag is checked once after boot and this value us used until the next reboot to - * avoid needing to handle the flag changing on the fly. - */ - private boolean mKeepProfilesRunning = isKeepProfilesRunningFlagEnabled(); - /** * For apps targeting U+ * Enable multiple admins to coexist on the same device. @@ -2097,12 +2091,6 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { performPolicyVersionUpgrade(); } - // TODO(b/265683382) move it into an upgrade step when removing the flag, so that it is - // executed only once on upgrading devices, not every boot. - if (mKeepProfilesRunning) { - suspendAppsForQuietProfiles(); - } - mUserData = new SparseArray<>(); mOwners = makeOwners(injector, pathProvider); @@ -2203,12 +2191,12 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { return packageNameAndSignature; } - private void suspendAppsForQuietProfiles() { + private void suspendAppsForQuietProfiles(boolean toSuspend) { PackageManagerInternal pmi = mInjector.getPackageManagerInternal(); List users = mUserManager.getUsers(); for (UserInfo user : users) { if (user.isManagedProfile() && user.isQuietModeEnabled()) { - pmi.setPackagesSuspendedForQuietMode(user.id, true); + pmi.setPackagesSuspendedForQuietMode(user.id, toSuspend); } } } @@ -3483,6 +3471,10 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { revertTransferOwnershipIfNecessaryLocked(); } updateUsbDataSignal(); + + // In case flag value has changed, we apply it during boot to avoid doing it concurrently + // with user toggling quiet mode. + setKeepProfileRunningEnabledUnchecked(isKeepProfilesRunningFlagEnabled()); } // TODO(b/230841522) Make it static. @@ -11318,7 +11310,8 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { (size == 1 ? "" : "s")); } pw.println(); - pw.println("Keep profiles running: " + mKeepProfilesRunning); + pw.println("Keep profiles running: " + + getUserData(UserHandle.USER_SYSTEM).mEffectiveKeepProfilesRunning); pw.println(); mPolicyCache.dump(pw); @@ -16249,7 +16242,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { @Override public boolean isKeepProfilesRunningEnabled() { - return mKeepProfilesRunning; + return getUserDataUnchecked(UserHandle.USER_SYSTEM).mEffectiveKeepProfilesRunning; } private @Mode int findInteractAcrossProfilesResetMode(String packageName) { @@ -23736,6 +23729,18 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { return false; } + private void setKeepProfileRunningEnabledUnchecked(boolean keepProfileRunning) { + synchronized (getLockObject()) { + DevicePolicyData policyData = getUserDataUnchecked(UserHandle.USER_SYSTEM); + if (policyData.mEffectiveKeepProfilesRunning == keepProfileRunning) { + return; + } + policyData.mEffectiveKeepProfilesRunning = keepProfileRunning; + saveSettingsLocked(UserHandle.USER_SYSTEM); + } + suspendAppsForQuietProfiles(keepProfileRunning); + } + private boolean isWorkProfileTelephonyEnabled() { return isWorkProfileTelephonyDevicePolicyManagerFlagEnabled() && isWorkProfileTelephonySubscriptionManagerFlagEnabled(); @@ -23760,7 +23765,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { public void setOverrideKeepProfilesRunning(boolean enabled) { Preconditions.checkCallAuthorization( hasCallingOrSelfPermission(MANAGE_PROFILE_AND_DEVICE_OWNERS)); - mKeepProfilesRunning = enabled; + setKeepProfileRunningEnabledUnchecked(enabled); Slog.i(LOG_TAG, "Keep profiles running overridden to: " + enabled); } diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/PolicyVersionUpgrader.java b/services/devicepolicy/java/com/android/server/devicepolicy/PolicyVersionUpgrader.java index 1fe4b571a7701..06f11be20245c 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/PolicyVersionUpgrader.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/PolicyVersionUpgrader.java @@ -110,6 +110,12 @@ public class PolicyVersionUpgrader { currentVersion = 4; } + if (currentVersion == 4) { + Slog.i(LOG_TAG, String.format("Upgrading from version %d", currentVersion)); + initializeEffectiveKeepProfilesRunning(allUsersData); + currentVersion = 5; + } + writePoliciesAndVersion(allUsers, allUsersData, ownersData, currentVersion); } @@ -214,6 +220,16 @@ public class PolicyVersionUpgrader { ownerAdmin.suspendedPackages.size(), ownerPackage, ownerUserId)); } + private void initializeEffectiveKeepProfilesRunning( + SparseArray allUsersData) { + DevicePolicyData systemUserData = allUsersData.get(UserHandle.USER_SYSTEM); + if (systemUserData == null) { + return; + } + systemUserData.mEffectiveKeepProfilesRunning = false; + Slog.i(LOG_TAG, "Keep profile running effective state set to false"); + } + private OwnersData loadOwners(int[] allUsers) { OwnersData ownersData = new OwnersData(mPathProvider); ownersData.load(allUsers); diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/PolicyVersionUpgraderTest.java b/services/tests/servicestests/src/com/android/server/devicepolicy/PolicyVersionUpgraderTest.java index 1d6846c4c807c..eb2ee35161ff7 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/PolicyVersionUpgraderTest.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/PolicyVersionUpgraderTest.java @@ -76,7 +76,7 @@ import javax.xml.parsers.DocumentBuilderFactory; public class PolicyVersionUpgraderTest extends DpmTestBase { // NOTE: Only change this value if the corresponding CL also adds a test to test the upgrade // to the new version. - private static final int LATEST_TESTED_VERSION = 4; + private static final int LATEST_TESTED_VERSION = 5; public static final String PERMISSIONS_TAG = "admin-can-grant-sensors-permissions"; public static final String DEVICE_OWNER_XML = "device_owner_2.xml"; private ComponentName mFakeAdmin; @@ -312,6 +312,24 @@ public class PolicyVersionUpgraderTest extends DpmTestBase { assertThat(storedSuspendedPkgs).isEqualTo(suspendedPkgs); } + @Test + public void testEffectiveKeepProfilesRunningSet() throws Exception { + writeVersionToXml(4); + + final int userId = UserHandle.USER_SYSTEM; + mProvider.mUsers = new int[]{userId}; + preparePoliciesFile(userId, "device_policies.xml"); + + mUpgrader.upgradePolicy(5); + + assertThat(readVersionFromXml()).isAtLeast(5); + + Document policies = readPolicies(userId); + Element keepProfilesRunning = (Element) policies.getDocumentElement() + .getElementsByTagName("keep-profiles-running").item(0); + assertThat(keepProfilesRunning.getAttribute("value")).isEqualTo("false"); + } + @Test public void isLatestVersionTested() { assertThat(DevicePolicyManagerService.DPMS_VERSION).isEqualTo(LATEST_TESTED_VERSION);