From 0b1356ff1f3cd2fd06d770af1ad466822173cc3a Mon Sep 17 00:00:00 2001
From: Suprabh Shukla <suprabh@google.com>
Date: Tue, 21 Feb 2017 14:33:50 -0800
Subject: [PATCH] Fix 'Modifying dpm.setSecureSetting call for
 install_non_market_apps'

The previous change was reverted as it broke work profile provisioning.
Clearing binder calling identity before calling into settings provider
should fix the issue.

Test: runtest managed-provisioning
Test: runtest -x services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java
Test: Manually tested that work profile is inflated with expected values
of install_non_market_apps

Bug: 33947615
Bug: 35590590

Change-Id: I3c31a73fef0c25c0e682e18f637272adad39b28d
---
 .../app/admin/DevicePolicyManager.java        | 10 ++-
 core/java/android/provider/Settings.java      | 12 ++++
 .../android/provider/SettingsBackupTest.java  |  1 +
 .../SettingsProvider/res/values/defaults.xml  |  2 +-
 .../providers/settings/SettingsProvider.java  | 10 ++-
 .../InstallNonMarketAppsDeprecationTest.java  | 30 ++++++---
 .../DevicePolicyManagerService.java           | 66 ++++++++++++++++++-
 7 files changed, 117 insertions(+), 14 deletions(-)

diff --git a/core/java/android/app/admin/DevicePolicyManager.java b/core/java/android/app/admin/DevicePolicyManager.java
index 2ace0a211dd67..9cb3dd6e4c0af 100644
--- a/core/java/android/app/admin/DevicePolicyManager.java
+++ b/core/java/android/app/admin/DevicePolicyManager.java
@@ -6343,7 +6343,6 @@ public class DevicePolicyManager {
      * The settings that can be updated by a profile or device owner with this method are:
      * <ul>
      * <li>{@link Settings.Secure#DEFAULT_INPUT_METHOD}</li>
-     * <li>{@link Settings.Secure#INSTALL_NON_MARKET_APPS}</li>
      * <li>{@link Settings.Secure#SKIP_FIRST_USE_HINTS}</li>
      * </ul>
      * <p>
@@ -6352,6 +6351,15 @@ public class DevicePolicyManager {
      * <li>{@link Settings.Secure#LOCATION_MODE}</li>
      * </ul>
      *
+     * <strong>Note: Starting from Android O, apps should no longer call this method with the
+     * setting {@link android.provider.Settings.Secure#INSTALL_NON_MARKET_APPS}, which is
+     * deprecated. Instead, device owners or profile owners should use the restriction
+     * {@link UserManager#DISALLOW_INSTALL_UNKNOWN_SOURCES}.
+     * If any app targeting {@link android.os.Build.VERSION_CODES#O} or higher calls this method
+     * with {@link android.provider.Settings.Secure#INSTALL_NON_MARKET_APPS},
+     * an {@link UnsupportedOperationException} is thrown.
+     * </strong>
+     *
      * @param admin Which {@link DeviceAdminReceiver} this request is associated with.
      * @param setting The name of the setting to update.
      * @param value The value to update the setting to.
diff --git a/core/java/android/provider/Settings.java b/core/java/android/provider/Settings.java
index ebc5b88b57987..8aa2b2c1567d9 100755
--- a/core/java/android/provider/Settings.java
+++ b/core/java/android/provider/Settings.java
@@ -5216,6 +5216,18 @@ public final class Settings {
          */
         public static final String INSTALL_NON_MARKET_APPS = "install_non_market_apps";
 
+        /**
+         * A flag to tell {@link com.android.server.devicepolicy.DevicePolicyManagerService} that
+         * the default for {@link #INSTALL_NON_MARKET_APPS} is reversed for this user on OTA. So it
+         * can set the restriction {@link android.os.UserManager#DISALLOW_INSTALL_UNKNOWN_SOURCES}
+         * on behalf of the profile owner if needed to make the change transparent for profile
+         * owners.
+         *
+         * @hide
+         */
+        public static final String UNKNOWN_SOURCES_DEFAULT_REVERSED =
+                "unknown_sources_default_reversed";
+
         /**
          * Comma-separated list of location providers that activities may access. Do not rely on
          * this value being present in settings.db or on ContentObserver notifications on the
diff --git a/core/tests/coretests/src/android/provider/SettingsBackupTest.java b/core/tests/coretests/src/android/provider/SettingsBackupTest.java
index 76331a865ba31..3584569e95b9b 100644
--- a/core/tests/coretests/src/android/provider/SettingsBackupTest.java
+++ b/core/tests/coretests/src/android/provider/SettingsBackupTest.java
@@ -460,6 +460,7 @@ public class SettingsBackupTest {
                  Settings.Secure.TV_INPUT_CUSTOM_LABELS,
                  Settings.Secure.TV_INPUT_HIDDEN_INPUTS,
                  Settings.Secure.UI_NIGHT_MODE, // candidate?
+                 Settings.Secure.UNKNOWN_SOURCES_DEFAULT_REVERSED,
                  Settings.Secure.UNSAFE_VOLUME_MUSIC_ACTIVE_MS,
                  Settings.Secure.USB_AUDIO_AUTOMATIC_ROUTING_DISABLED,
                  Settings.Secure.USER_SETUP_COMPLETE,
diff --git a/packages/SettingsProvider/res/values/defaults.xml b/packages/SettingsProvider/res/values/defaults.xml
index 136f17e5a77c4..720612726a8f5 100644
--- a/packages/SettingsProvider/res/values/defaults.xml
+++ b/packages/SettingsProvider/res/values/defaults.xml
@@ -38,7 +38,7 @@
 
     <bool name="def_bluetooth_on">true</bool>
     <bool name="def_wifi_display_on">false</bool>
-    <bool name="def_install_non_market_apps">true</bool>
+    <bool name="def_install_non_market_apps">false</bool>
     <bool name="def_package_verifier_enable">true</bool>
     <!-- Comma-separated list of location providers.
          Network location is off by default because it requires
diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java
index 85c153cdb1643..9cf060c5fffcd 100644
--- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java
+++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java
@@ -3174,9 +3174,17 @@ public class SettingsProvider extends ContentProvider {
                     // setting through the UI.
                     final SettingsState secureSetting = getSecureSettingsLocked(userId);
                     if (!mUserManager.hasUserRestriction(
-                            UserManager.DISALLOW_INSTALL_UNKNOWN_SOURCES, UserHandle.of(userId))) {
+                            UserManager.DISALLOW_INSTALL_UNKNOWN_SOURCES, UserHandle.of(userId))
+                            && secureSetting.getSettingLocked(
+                            Settings.Secure.INSTALL_NON_MARKET_APPS).getValue().equals("0")) {
+
                         secureSetting.insertSettingLocked(Settings.Secure.INSTALL_NON_MARKET_APPS,
                                 "1", null, true, SettingsState.SYSTEM_PACKAGE_NAME);
+                        // For managed profiles with profile owners, DevicePolicyManagerService
+                        // may want to set the user restriction in this case
+                        secureSetting.insertSettingLocked(
+                                Settings.Secure.UNKNOWN_SOURCES_DEFAULT_REVERSED, "1", null, true,
+                                SettingsState.SYSTEM_PACKAGE_NAME);
                     }
                     currentVersion = 138;
                 }
diff --git a/packages/SettingsProvider/test/src/com/android/providers/settings/InstallNonMarketAppsDeprecationTest.java b/packages/SettingsProvider/test/src/com/android/providers/settings/InstallNonMarketAppsDeprecationTest.java
index 51e43738b5376..d8ee9b64c7d91 100644
--- a/packages/SettingsProvider/test/src/com/android/providers/settings/InstallNonMarketAppsDeprecationTest.java
+++ b/packages/SettingsProvider/test/src/com/android/providers/settings/InstallNonMarketAppsDeprecationTest.java
@@ -22,6 +22,7 @@ import static junit.framework.Assert.assertTrue;
 
 import android.content.Context;
 import android.content.pm.UserInfo;
+import android.os.Process;
 import android.os.SystemClock;
 import android.os.UserManager;
 import android.provider.Settings;
@@ -37,6 +38,7 @@ import java.io.BufferedReader;
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStreamReader;
+import java.util.ArrayList;
 import java.util.List;
 
 @LargeTest
@@ -47,7 +49,8 @@ public class InstallNonMarketAppsDeprecationTest extends BaseSettingsProviderTes
 
     private UserManager mUm;
     private boolean mHasUserRestriction;
-    private List<UserInfo> mCurrentUsers;
+    private boolean mSystemSetUserRestriction;
+    private List<Integer> mUsersAddedByTest;
 
     private String waitTillValueChanges(String errorMessage, String oldValue) {
         boolean interrupted = false;
@@ -84,7 +87,10 @@ public class InstallNonMarketAppsDeprecationTest extends BaseSettingsProviderTes
     public void setUp() {
         mUm = (UserManager) getContext().getSystemService(Context.USER_SERVICE);
         mHasUserRestriction = mUm.hasUserRestriction(UserManager.DISALLOW_INSTALL_UNKNOWN_SOURCES);
-        mCurrentUsers = mUm.getUsers();
+        mSystemSetUserRestriction = mUm.getUserRestrictionSource(
+                UserManager.DISALLOW_INSTALL_UNKNOWN_SOURCES, Process.myUserHandle())
+                == UserManager.RESTRICTION_SOURCE_SYSTEM;
+        mUsersAddedByTest = new ArrayList<>();
     }
 
     @Test
@@ -108,6 +114,7 @@ public class InstallNonMarketAppsDeprecationTest extends BaseSettingsProviderTes
     @Test
     public void testValueForNewUser() throws Exception {
         UserInfo newUser = mUm.createUser("TEST_USER", 0);
+        mUsersAddedByTest.add(newUser.id);
         String value = getSecureSettingForUserViaShell(newUser.id);
         assertEquals("install_non_market_apps should be 1 for a new user", value, "1");
     }
@@ -117,6 +124,13 @@ public class InstallNonMarketAppsDeprecationTest extends BaseSettingsProviderTes
         String value = getSetting(SETTING_TYPE_SECURE, Settings.Secure.INSTALL_NON_MARKET_APPS);
         assertEquals(value, mHasUserRestriction ? "0" : "1");
 
+        if (mHasUserRestriction && !mSystemSetUserRestriction) {
+            // User restriction set by device policy. This case should be covered in DO/PO related
+            // tests. Pass.
+            Log.w(TAG, "User restriction set by PO/DO. Skipping testValueRespectsUserRestriction");
+            return;
+        }
+
         mUm.setUserRestriction(UserManager.DISALLOW_INSTALL_UNKNOWN_SOURCES, !mHasUserRestriction);
         value = waitTillValueChanges(
                 "Changing user restriction did not change the value of install_non_market_apps",
@@ -132,15 +146,13 @@ public class InstallNonMarketAppsDeprecationTest extends BaseSettingsProviderTes
 
     @After
     public void tearDown() {
-        if (mUm.hasUserRestriction(UserManager.DISALLOW_INSTALL_UNKNOWN_SOURCES)
-                != mHasUserRestriction) {
+        if (!mHasUserRestriction || mSystemSetUserRestriction) {
+            // The test may have modified the user restriction state. Restore it.
             mUm.setUserRestriction(UserManager.DISALLOW_INSTALL_UNKNOWN_SOURCES,
                     mHasUserRestriction);
         }
-        mUm.getUsers().forEach(user -> {
-            if (!mCurrentUsers.contains(user)) {
-                mUm.removeUser(user.id);
-            }
-        });
+        for (int userId : mUsersAddedByTest) {
+            mUm.removeUser(userId);
+        }
     }
 }
diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
index 14b1741346b9b..620d441765d0d 100644
--- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
+++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
@@ -194,6 +194,7 @@ import java.security.cert.CertificateException;
 import java.security.cert.CertificateFactory;
 import java.security.cert.X509Certificate;
 import java.text.DateFormat;
+import java.text.NumberFormat;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -2911,11 +2912,41 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
         }
     }
 
+    private void ensureUnknownSourcesRestrictionForProfileOwners() {
+        synchronized (this) {
+            for (int userId : mOwners.getProfileOwnerKeys()) {
+                if (!mUserManager.isManagedProfile(userId) ||
+                        mInjector.settingsSecureGetIntForUser(
+                        Settings.Secure.UNKNOWN_SOURCES_DEFAULT_REVERSED, 0, userId) == 0) {
+                    continue;
+                }
+                setUserRestrictionOnBehalfOfProfileOwnerLocked(
+                        UserManager.DISALLOW_INSTALL_UNKNOWN_SOURCES, userId);
+                mInjector.settingsSecurePutIntForUser(
+                        Settings.Secure.UNKNOWN_SOURCES_DEFAULT_REVERSED, 0, userId);
+            }
+        }
+    }
+
+    private void setUserRestrictionOnBehalfOfProfileOwnerLocked(String userRestrictionKey,
+            int userId) {
+        if (UserRestrictionsUtils.isValidRestriction(userRestrictionKey) &&
+                UserRestrictionsUtils.canProfileOwnerChange(userRestrictionKey, userId)) {
+            ActiveAdmin profileOwner = getProfileOwnerAdminLocked(userId);
+            if (profileOwner == null) {
+                return;
+            }
+            Bundle restrictions = profileOwner.ensureUserRestrictions();
+            restrictions.putBoolean(userRestrictionKey, true);
+            saveUserRestrictionsLocked(userId);
+        }
+    }
+
     private void onLockSettingsReady() {
         getUserData(UserHandle.USER_SYSTEM);
         loadOwners();
         cleanUpOldUsers();
-
+        ensureUnknownSourcesRestrictionForProfileOwners();
         onStartUser(UserHandle.USER_SYSTEM);
 
         // Register an observer for watching for user setup complete and settings changes.
@@ -6638,6 +6669,17 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
             mOwners.writeProfileOwner(userHandle);
             Slog.i(LOG_TAG, "Profile owner set: " + who + " on user " + userHandle);
 
+            final long id = mInjector.binderClearCallingIdentity();
+            try {
+                if (mUserManager.isManagedProfile(userHandle)) {
+                    setUserRestrictionOnBehalfOfProfileOwnerLocked(
+                            UserManager.DISALLOW_INSTALL_UNKNOWN_SOURCES, userHandle);
+                    mInjector.settingsSecurePutIntForUser(
+                            Settings.Secure.UNKNOWN_SOURCES_DEFAULT_REVERSED, 0, userHandle);
+                }
+            } finally {
+                mInjector.binderRestoreCallingIdentity(id);
+            }
             return true;
         }
     }
@@ -8724,7 +8766,27 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
                 throw new SecurityException(String.format(
                         "Permission denial: Profile owners cannot update %1$s", setting));
             }
-
+            if (setting.equals(Settings.Secure.INSTALL_NON_MARKET_APPS)) {
+                if (getTargetSdk(who.getPackageName(), callingUserId) >= Build.VERSION_CODES.O) {
+                    throw new UnsupportedOperationException(Settings.Secure.INSTALL_NON_MARKET_APPS
+                            + " is deprecated. Please use the user restriction "
+                            + UserManager.DISALLOW_INSTALL_UNKNOWN_SOURCES + " instead.");
+                }
+                if (!mUserManager.isManagedProfile(callingUserId)) {
+                    Slog.e(LOG_TAG, "Ignoring setSecureSetting request for "
+                            + setting + ". User restriction "
+                            + UserManager.DISALLOW_INSTALL_UNKNOWN_SOURCES
+                            + " should be used instead.");
+                } else {
+                    try {
+                        setUserRestriction(who, UserManager.DISALLOW_INSTALL_UNKNOWN_SOURCES,
+                                (Integer.parseInt(value) == 0) ? true : false);
+                    } catch (NumberFormatException exc) {
+                        Slog.e(LOG_TAG, "Invalid value: " + value + " for setting " + setting);
+                    }
+                }
+                return;
+            }
             long id = mInjector.binderClearCallingIdentity();
             try {
                 if (Settings.Secure.DEFAULT_INPUT_METHOD.equals(setting)) {