Don't toggle app state for updated system apps.
Jeff Davidson [Tue, 7 Jul 2015 20:29:07 +0000 (13:29 -0700)]
The default carrier app logic to disable/enable apps depending on the
SIM state should only apply to the copy of the app on /system. Once
the app has been updated via /data, we should leave the enabled
setting as the default (though we should still grant carrier
privileges as needed in this case).

Otherwise we may unexpectedly disable a previously enabled app if it
is updated and then the SIM state changes or device reboots at a time
when the SIM isn't loaded.

Bug: 22321187
Change-Id: Ia7281c5a174bd32fc4d63375603ace0b29794894

src/java/com/android/internal/telephony/CarrierAppUtils.java
tests/telephonytests/src/com/android/internal/telephony/CarrierAppUtilsTest.java

index ea8ff94..4be914c 100644 (file)
@@ -88,29 +88,33 @@ public final class CarrierAppUtils {
                 boolean hasPrivileges =
                         telephonyManager.checkCarrierPrivilegesForPackageAnyPhone(packageName) ==
                                 TelephonyManager.CARRIER_PRIVILEGE_STATUS_HAS_ACCESS;
-                if (hasPrivileges) {
-                    if (ai.enabledSetting == PackageManager.COMPONENT_ENABLED_STATE_DEFAULT
+
+                // Only update enabled state for the app on /system. Once it has been updated we
+                // shouldn't touch it.
+                if (!ai.isUpdatedSystemApp()) {
+                    if (hasPrivileges
+                            && (ai.enabledSetting == PackageManager.COMPONENT_ENABLED_STATE_DEFAULT
                             || ai.enabledSetting ==
-                                    PackageManager.COMPONENT_ENABLED_STATE_DISABLED_UNTIL_USED) {
+                                    PackageManager.COMPONENT_ENABLED_STATE_DISABLED_UNTIL_USED)) {
                         Slog.i(TAG, "Update state(" + packageName + "): ENABLED for user "
                                 + userId);
-                        enabledCarrierPackages.add(ai.packageName);
                         packageManager.setApplicationEnabledSetting(packageName,
                                 PackageManager.COMPONENT_ENABLED_STATE_ENABLED,
                                 PackageManager.DONT_KILL_APP, userId, callingPackage);
-                    } else if (ai.enabledSetting ==
-                            PackageManager.COMPONENT_ENABLED_STATE_ENABLED) {
-                        // If we're already enabled, don't bother re-enabling, but treat the app as
-                        // enabled so that we re-grant default permissions in case they were lost.
-                        enabledCarrierPackages.add(ai.packageName);
+                    } else if (!hasPrivileges
+                            && ai.enabledSetting ==
+                                    PackageManager.COMPONENT_ENABLED_STATE_DEFAULT) {
+                        Slog.i(TAG, "Update state(" + packageName
+                                + "): DISABLED_UNTIL_USED for user " + userId);
+                        packageManager.setApplicationEnabledSetting(packageName,
+                                PackageManager.COMPONENT_ENABLED_STATE_DISABLED_UNTIL_USED, 0,
+                                userId, callingPackage);
                     }
-                } else if (!hasPrivileges
-                        && ai.enabledSetting == PackageManager.COMPONENT_ENABLED_STATE_DEFAULT) {
-                    Slog.i(TAG, "Update state(" + packageName + "): DISABLED_UNTIL_USED for user "
-                            + userId);
-                    packageManager.setApplicationEnabledSetting(packageName,
-                            PackageManager.COMPONENT_ENABLED_STATE_DISABLED_UNTIL_USED, 0, userId,
-                            callingPackage);
+                }
+
+                // Always re-grant default permissions to carrier apps w/ privileges.
+                if (hasPrivileges) {
+                    enabledCarrierPackages.add(ai.packageName);
                 }
             }
 
@@ -194,8 +198,7 @@ public final class CarrierAppUtils {
                     // No app found for packageName
                     continue;
                 }
-                boolean isSystemPackage = (ai.flags & ApplicationInfo.FLAG_SYSTEM) != 0;
-                if (!isSystemPackage) {
+                if (!ai.isSystemApp()) {
                     continue;
                 }
                 apps.add(ai);
index ad35019..61e5b4f 100644 (file)
@@ -82,7 +82,10 @@ public class CarrierAppUtilsTest extends InstrumentationTestCase {
         Mockito.verifyNoMoreInteractions(mTelephonyManager);
     }
 
-    /** Configured app has privileges, but was disabled by the user - should do nothing. */
+    /**
+     * Configured app has privileges, but was disabled by the user - should only grant
+     * permissions.
+     */
     public void testDisableCarrierAppsUntilPrivileged_HasPrivileges_DisabledUser()
             throws Exception {
         ApplicationInfo appInfo = new ApplicationInfo();
@@ -98,12 +101,11 @@ public class CarrierAppUtilsTest extends InstrumentationTestCase {
         Mockito.verify(mPackageManager, Mockito.never()).setApplicationEnabledSetting(
                 Mockito.anyString(), Mockito.anyInt(), Mockito.anyInt(), Mockito.anyInt(),
                 Mockito.anyString());
-        Mockito.verify(mPackageManager, Mockito.never())
-                .grantDefaultPermissionsToEnabledCarrierApps(
-                        Mockito.any(String[].class), Mockito.anyInt());
+        Mockito.verify(mPackageManager).grantDefaultPermissionsToEnabledCarrierApps(
+                new String[] {appInfo.packageName}, USER_ID);
     }
 
-    /** Configured app has privileges, but was disabled - should do nothing. */
+    /** Configured app has privileges, but was disabled - should only grant permissions. */
     public void testDisableCarrierAppsUntilPrivileged_HasPrivileges_Disabled() throws Exception {
         ApplicationInfo appInfo = new ApplicationInfo();
         appInfo.packageName = CARRIER_APP;
@@ -118,9 +120,8 @@ public class CarrierAppUtilsTest extends InstrumentationTestCase {
         Mockito.verify(mPackageManager, Mockito.never()).setApplicationEnabledSetting(
                 Mockito.anyString(), Mockito.anyInt(), Mockito.anyInt(), Mockito.anyInt(),
                 Mockito.anyString());
-        Mockito.verify(mPackageManager, Mockito.never())
-                .grantDefaultPermissionsToEnabledCarrierApps(
-                        Mockito.any(String[].class), Mockito.anyInt());
+        Mockito.verify(mPackageManager).grantDefaultPermissionsToEnabledCarrierApps(
+                new String[] {appInfo.packageName}, USER_ID);
     }
 
     /** Configured app has privileges, and is already enabled - should only grant permissions. */
@@ -142,6 +143,25 @@ public class CarrierAppUtilsTest extends InstrumentationTestCase {
                 new String[] {appInfo.packageName}, USER_ID);
     }
 
+    /** Configured /data app has privileges - should only grant permissions. */
+    public void testDisableCarrierAppsUntilPrivileged_HasPrivileges_UpdatedApp() throws Exception {
+        ApplicationInfo appInfo = new ApplicationInfo();
+        appInfo.packageName = CARRIER_APP;
+        appInfo.flags |= ApplicationInfo.FLAG_SYSTEM | ApplicationInfo.FLAG_UPDATED_SYSTEM_APP;
+        appInfo.enabledSetting = PackageManager.COMPONENT_ENABLED_STATE_ENABLED;
+        Mockito.when(mPackageManager.getApplicationInfo(CARRIER_APP,
+                PackageManager.GET_DISABLED_UNTIL_USED_COMPONENTS, USER_ID)).thenReturn(appInfo);
+        Mockito.when(mTelephonyManager.checkCarrierPrivilegesForPackageAnyPhone(CARRIER_APP))
+                .thenReturn(TelephonyManager.CARRIER_PRIVILEGE_STATUS_HAS_ACCESS);
+        CarrierAppUtils.disableCarrierAppsUntilPrivileged(CALLING_PACKAGE, mPackageManager,
+                mTelephonyManager, USER_ID, CARRIER_APPS);
+        Mockito.verify(mPackageManager, Mockito.never()).setApplicationEnabledSetting(
+                Mockito.anyString(), Mockito.anyInt(), Mockito.anyInt(), Mockito.anyInt(),
+                Mockito.anyString());
+        Mockito.verify(mPackageManager).grantDefaultPermissionsToEnabledCarrierApps(
+                new String[] {appInfo.packageName}, USER_ID);
+    }
+
     /** Configured app has privileges, and is in the default state - should enable. */
     public void testDisableCarrierAppsUntilPrivileged_HasPrivileges_Default() throws Exception {
         ApplicationInfo appInfo = new ApplicationInfo();
@@ -241,6 +261,26 @@ public class CarrierAppUtilsTest extends InstrumentationTestCase {
                         Mockito.any(String[].class), Mockito.anyInt());
     }
 
+    /** Configured /data app has no privileges - should do nothing. */
+    public void testDisableCarrierAppsUntilPrivileged_NoPrivileges_UpdatedApp() throws Exception {
+        ApplicationInfo appInfo = new ApplicationInfo();
+        appInfo.packageName = CARRIER_APP;
+        appInfo.flags |= ApplicationInfo.FLAG_SYSTEM | ApplicationInfo.FLAG_UPDATED_SYSTEM_APP;
+        appInfo.enabledSetting = PackageManager.COMPONENT_ENABLED_STATE_ENABLED;
+        Mockito.when(mPackageManager.getApplicationInfo(CARRIER_APP,
+                PackageManager.GET_DISABLED_UNTIL_USED_COMPONENTS, USER_ID)).thenReturn(appInfo);
+        Mockito.when(mTelephonyManager.checkCarrierPrivilegesForPackageAnyPhone(CARRIER_APP))
+                .thenReturn(TelephonyManager.CARRIER_PRIVILEGE_STATUS_NO_ACCESS);
+        CarrierAppUtils.disableCarrierAppsUntilPrivileged(CALLING_PACKAGE, mPackageManager,
+                mTelephonyManager, USER_ID, CARRIER_APPS);
+        Mockito.verify(mPackageManager, Mockito.never()).setApplicationEnabledSetting(
+                Mockito.anyString(), Mockito.anyInt(), Mockito.anyInt(), Mockito.anyInt(),
+                Mockito.anyString());
+        Mockito.verify(mPackageManager, Mockito.never())
+                .grantDefaultPermissionsToEnabledCarrierApps(
+                        Mockito.any(String[].class), Mockito.anyInt());
+    }
+
     /** Configured app has no privileges, and is in the default state - should disable until use. */
     public void testDisableCarrierAppsUntilPrivileged_NoPrivileges_Default() throws Exception {
         ApplicationInfo appInfo = new ApplicationInfo();