Ask profile password before unifying to prevent untrusted reset

Test: make -j RunSettingsRoboTests
Test: manual, unify when profile lock is compliant
Test: manual, unify when profile lock is not compliant
Test: manual, unify when profile lock is empty
Fixes: 110262879

Change-Id: I0dfa885f2a0e44e09c217b3e7766b367f1340c9e

src/com/android/settings/security/LockUnificationPreferenceController.java

@@ -43,6 +43,17 @@ import com.android.settingslib.core.AbstractPreferenceController;
 import androidx.preference.Preference;
 import androidx.preference.PreferenceScreen;
 
+/**
+ * Controller for password unification/un-unification flows.
+ *
+ * When password is being unified, there may be two cases:
+ *   1. If work password is not empty and satisfies device-wide policies (if any), it will be made
+ *      into device-wide password. To do that we need both current device and profile passwords
+ *      because both of them will be changed as a result.
+ *   2. Otherwise device-wide password is preserved. In this case we only need current profile
+ *      password, but after unifying the passwords we proceed to ask the user for a new device
+ *      password.
+ */
 public class LockUnificationPreferenceController extends AbstractPreferenceController
         implements PreferenceControllerMixin, Preference.OnPreferenceChangeListener {
 
@@ -51,8 +62,9 @@ public class LockUnificationPreferenceController extends AbstractPreferenceContr
     private static final int MY_USER_ID = UserHandle.myUserId();
 
     private final UserManager mUm;
+    private final DevicePolicyManager mDpm;
     private final LockPatternUtils mLockPatternUtils;
-    private final int mProfileChallengeUserId;
+    private final int mProfileUserId;
     private final SecuritySettings mHost;
 
     private RestrictedSwitchPreference mUnifyProfile;
@@ -60,6 +72,7 @@ public class LockUnificationPreferenceController extends AbstractPreferenceContr
 
     private String mCurrentDevicePassword;
     private String mCurrentProfilePassword;
+    private boolean mKeepDeviceLock;
 
     @Override
     public void displayPreference(PreferenceScreen screen) {
@@ -70,20 +83,18 @@ public class LockUnificationPreferenceController extends AbstractPreferenceContr
     public LockUnificationPreferenceController(Context context, SecuritySettings host) {
         super(context);
         mHost = host;
-        mUm = (UserManager) context.getSystemService(Context.USER_SERVICE);
+        mUm = context.getSystemService(UserManager.class);
+        mDpm = context.getSystemService(DevicePolicyManager.class);
         mLockPatternUtils = FeatureFactory.getFactory(context)
                 .getSecurityFeatureProvider()
                 .getLockPatternUtils(context);
-        mProfileChallengeUserId = Utils.getManagedProfileId(mUm, MY_USER_ID);
+        mProfileUserId = Utils.getManagedProfileId(mUm, MY_USER_ID);
     }
 
     @Override
     public boolean isAvailable() {
-        final boolean allowSeparateProfileChallenge =
+        return mProfileUserId != UserHandle.USER_NULL
-                mProfileChallengeUserId != UserHandle.USER_NULL
+                && mLockPatternUtils.isSeparateProfileChallengeAllowed(mProfileUserId);
-                        && mLockPatternUtils.isSeparateProfileChallengeAllowed(
-                        mProfileChallengeUserId);
-        return allowSeparateProfileChallenge;
     }
 
     @Override
@@ -93,18 +104,18 @@ public class LockUnificationPreferenceController extends AbstractPreferenceContr
 
     @Override
     public boolean onPreferenceChange(Preference preference, Object value) {
-        if (Utils.startQuietModeDialogIfNecessary(mContext, mUm, mProfileChallengeUserId)) {
+        if (Utils.startQuietModeDialogIfNecessary(mContext, mUm, mProfileUserId)) {
             return false;
         }
-        if ((Boolean) value) {
+        final boolean useOneLock = (Boolean) value;
-            final boolean compliantForDevice =
+        if (useOneLock) {
-                    (mLockPatternUtils.getKeyguardStoredPasswordQuality(mProfileChallengeUserId)
+            // Keep current device (personal) lock if the profile lock is empty or is not compliant
-                            >= DevicePolicyManager.PASSWORD_QUALITY_SOMETHING
+            // with the policy on personal side.
-                            && mLockPatternUtils.isSeparateProfileChallengeAllowedToUnify(
+            mKeepDeviceLock =
-                            mProfileChallengeUserId));
+                    mLockPatternUtils.getKeyguardStoredPasswordQuality(mProfileUserId)
-            UnificationConfirmationDialog dialog =
+                            < DevicePolicyManager.PASSWORD_QUALITY_SOMETHING
-                    UnificationConfirmationDialog.newInstance(compliantForDevice);
+                            || !mDpm.isProfileActivePasswordSufficientForParent(mProfileUserId);
-            dialog.show(mHost);
+            UnificationConfirmationDialog.newInstance(!mKeepDeviceLock).show(mHost);
         } else {
             final String title = mContext.getString(R.string.unlock_set_unlock_launch_picker_title);
             final ChooseLockSettingsHelper helper =
@@ -122,12 +133,11 @@ public class LockUnificationPreferenceController extends AbstractPreferenceContr
     public void updateState(Preference preference) {
         if (mUnifyProfile != null) {
             final boolean separate =
-                    mLockPatternUtils.isSeparateProfileChallengeEnabled(mProfileChallengeUserId);
+                    mLockPatternUtils.isSeparateProfileChallengeEnabled(mProfileUserId);
             mUnifyProfile.setChecked(!separate);
             if (separate) {
                 mUnifyProfile.setDisabledByAdmin(RestrictedLockUtils.checkIfRestrictionEnforced(
-                        mContext, UserManager.DISALLOW_UNIFIED_PASSWORD,
+                        mContext, UserManager.DISALLOW_UNIFIED_PASSWORD, mProfileUserId));
-                        mProfileChallengeUserId));
             }
         }
     }
@@ -141,7 +151,7 @@ public class LockUnificationPreferenceController extends AbstractPreferenceContr
                 && resultCode == Activity.RESULT_OK) {
             mCurrentDevicePassword =
                     data.getStringExtra(ChooseLockSettingsHelper.EXTRA_KEY_PASSWORD);
-            launchConfirmProfileLockForUnification();
+            launchConfirmProfileLock();
             return true;
         } else if (requestCode == UNIFY_LOCK_CONFIRM_PROFILE_REQUEST
                 && resultCode == Activity.RESULT_OK) {
@@ -155,7 +165,7 @@ public class LockUnificationPreferenceController extends AbstractPreferenceContr
 
     private void ununifyLocks() {
         final Bundle extras = new Bundle();
-        extras.putInt(Intent.EXTRA_USER_ID, mProfileChallengeUserId);
+        extras.putInt(Intent.EXTRA_USER_ID, mProfileUserId);
         new SubSettingLauncher(mContext)
                 .setDestination(ChooseLockGeneric.ChooseLockGenericFragment.class.getName())
                     .setTitleRes(R.string.lock_settings_picker_title_profile)
@@ -164,54 +174,76 @@ public class LockUnificationPreferenceController extends AbstractPreferenceContr
                 .launch();
     }
 
-    void launchConfirmDeviceLockForUnification() {
+    /** Asks the user to confirm device lock (if there is one) and proceeds to ask profile lock. */
+    private void launchConfirmDeviceAndProfileLock() {
         final String title = mContext.getString(
                 R.string.unlock_set_unlock_launch_picker_title);
         final ChooseLockSettingsHelper helper =
                 new ChooseLockSettingsHelper(mHost.getActivity(), mHost);
         if (!helper.launchConfirmationActivity(
                 UNIFY_LOCK_CONFIRM_DEVICE_REQUEST, title, true, MY_USER_ID)) {
-            launchConfirmProfileLockForUnification();
+            launchConfirmProfileLock();
         }
     }
 
-    private void launchConfirmProfileLockForUnification() {
+    private void launchConfirmProfileLock() {
         final String title = mContext.getString(
                 R.string.unlock_set_unlock_launch_picker_title_profile);
         final ChooseLockSettingsHelper helper =
                 new ChooseLockSettingsHelper(mHost.getActivity(), mHost);
         if (!helper.launchConfirmationActivity(
-                UNIFY_LOCK_CONFIRM_PROFILE_REQUEST, title, true, mProfileChallengeUserId)) {
+                UNIFY_LOCK_CONFIRM_PROFILE_REQUEST, title, true, mProfileUserId)) {
             unifyLocks();
             // TODO: update relevant prefs.
             // createPreferenceHierarchy();
         }
     }
 
+    void startUnification() {
+        // If the device lock stays the same, only confirm profile lock. Otherwise confirm both.
+        if (mKeepDeviceLock) {
+            launchConfirmProfileLock();
+        } else {
+            launchConfirmDeviceAndProfileLock();
+        }
+    }
+
     private void unifyLocks() {
-        int profileQuality =
+        if (mKeepDeviceLock) {
-                mLockPatternUtils.getKeyguardStoredPasswordQuality(mProfileChallengeUserId);
+            unifyKeepingDeviceLock();
+            promptForNewDeviceLock();
+        } else {
+            unifyKeepingWorkLock();
+        }
+        mCurrentDevicePassword = null;
+        mCurrentProfilePassword = null;
+    }
+
+    private void unifyKeepingWorkLock() {
+        final int profileQuality =
+                mLockPatternUtils.getKeyguardStoredPasswordQuality(mProfileUserId);
+        // PASSWORD_QUALITY_SOMETHING means pattern, everything above means PIN/password.
         if (profileQuality == DevicePolicyManager.PASSWORD_QUALITY_SOMETHING) {
             mLockPatternUtils.saveLockPattern(
                     LockPatternUtils.stringToPattern(mCurrentProfilePassword),
                     mCurrentDevicePassword, MY_USER_ID);
         } else {
             mLockPatternUtils.saveLockPassword(
-                    mCurrentProfilePassword, mCurrentDevicePassword,
+                    mCurrentProfilePassword, mCurrentDevicePassword, profileQuality, MY_USER_ID);
-                    profileQuality, MY_USER_ID);
         }
-        mLockPatternUtils.setSeparateProfileChallengeEnabled(mProfileChallengeUserId, false,
+        mLockPatternUtils.setSeparateProfileChallengeEnabled(mProfileUserId, false,
                 mCurrentProfilePassword);
         final boolean profilePatternVisibility =
-                mLockPatternUtils.isVisiblePatternEnabled(mProfileChallengeUserId);
+                mLockPatternUtils.isVisiblePatternEnabled(mProfileUserId);
         mLockPatternUtils.setVisiblePatternEnabled(profilePatternVisibility, MY_USER_ID);
-        mCurrentDevicePassword = null;
-        mCurrentProfilePassword = null;
     }
 
-    void unifyUncompliantLocks() {
+    private void unifyKeepingDeviceLock() {
-        mLockPatternUtils.setSeparateProfileChallengeEnabled(mProfileChallengeUserId, false,
+        mLockPatternUtils.setSeparateProfileChallengeEnabled(mProfileUserId, false,
                 mCurrentProfilePassword);
+    }
+
+    private void promptForNewDeviceLock() {
         new SubSettingLauncher(mContext)
                 .setDestination(ChooseLockGeneric.ChooseLockGenericFragment.class.getName())
                 .setTitleRes(R.string.lock_settings_picker_title)

src/com/android/settings/security/SecuritySettings.java

@@ -96,13 +96,8 @@ public class SecuritySettings extends DashboardFragment {
         super.onActivityResult(requestCode, resultCode, data);
     }
 
-    void launchConfirmDeviceLockForUnification() {
+    void startUnification() {
-        use(LockUnificationPreferenceController.class)
+        use(LockUnificationPreferenceController.class).startUnification();
-                .launchConfirmDeviceLockForUnification();
-    }
-
-    void unifyUncompliantLocks() {
-        use(LockUnificationPreferenceController.class).unifyUncompliantLocks();
     }
 
     void updateUnificationPreference() {

src/com/android/settings/security/UnificationConfirmationDialog.java

@@ -60,14 +60,7 @@ public class UnificationConfirmationDialog extends InstrumentedDialogFragment {
                         compliant ? R.string.lock_settings_profile_unification_dialog_confirm
                                 : R.string
                                       .lock_settings_profile_unification_dialog_uncompliant_confirm,
-                        (dialog, whichButton) -> {
+                        (dialog, whichButton) -> parentFragment.startUnification())
-                            if (compliant) {
-                                parentFragment.launchConfirmDeviceLockForUnification();
-                            } else {
-                                parentFragment.unifyUncompliantLocks();
-                            }
-                        }
-                )
                 .setNegativeButton(R.string.cancel, null)
                 .create();
     }

tests/robotests/src/com/android/settings/security/LockUnificationPreferenceControllerTest.java

@@ -17,11 +17,11 @@
 package com.android.settings.security;
 
 import static com.google.common.truth.Truth.assertThat;
+
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.Mockito.when;
 
 import android.content.Context;
-import android.os.UserHandle;
 import android.os.UserManager;
 
 import com.android.internal.widget.LockPatternUtils;
@@ -35,7 +35,6 @@ import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 import org.robolectric.RuntimeEnvironment;
 import org.robolectric.shadows.ShadowApplication;
-import org.robolectric.util.ReflectionHelpers;
 
 import androidx.preference.Preference;
 import androidx.preference.PreferenceScreen;
@@ -54,7 +53,6 @@ public class LockUnificationPreferenceControllerTest {
     @Mock
     private SecuritySettings mHost;
 
-    private FakeFeatureFactory mFeatureFactory;
     private Context mContext;
     private LockUnificationPreferenceController mController;
     private Preference mPreference;
@@ -66,10 +64,12 @@ public class LockUnificationPreferenceControllerTest {
         ShadowApplication.getInstance().setSystemService(Context.USER_SERVICE, mUm);
         when(mUm.getProfileIdsWithDisabled(anyInt())).thenReturn(new int[] {FAKE_PROFILE_USER_ID});
 
-        mFeatureFactory = FakeFeatureFactory.setupForTest();
+        final FakeFeatureFactory featureFactory = FakeFeatureFactory.setupForTest();
-        when(mFeatureFactory.securityFeatureProvider.getLockPatternUtils(mContext))
+        when(featureFactory.securityFeatureProvider.getLockPatternUtils(mContext))
                 .thenReturn(mLockPatternUtils);
+    }
 
+    private void init() {
         mController = new LockUnificationPreferenceController(mContext, mHost);
         when(mScreen.findPreference(mController.getPreferenceKey())).thenReturn(mPreference);
         mPreference = new Preference(mContext);
@@ -77,7 +77,8 @@ public class LockUnificationPreferenceControllerTest {
 
     @Test
     public void isAvailable_noProfile_false() {
-        ReflectionHelpers.setField(mController, "mProfileChallengeUserId", UserHandle.USER_NULL);
+        when(mUm.getProfileIdsWithDisabled(anyInt())).thenReturn(new int[0]);
+        init();
 
         assertThat(mController.isAvailable()).isFalse();
     }
@@ -85,6 +86,7 @@ public class LockUnificationPreferenceControllerTest {
     @Test
     public void isAvailable_separateChallengeNotAllowed_false() {
         when(mLockPatternUtils.isSeparateProfileChallengeAllowed(anyInt())).thenReturn(false);
+        init();
 
         assertThat(mController.isAvailable()).isFalse();
     }
@@ -92,6 +94,7 @@ public class LockUnificationPreferenceControllerTest {
     @Test
     public void isAvailable_separateChallengeAllowed_true() {
         when(mLockPatternUtils.isSeparateProfileChallengeAllowed(anyInt())).thenReturn(true);
+        init();
 
         assertThat(mController.isAvailable()).isTrue();
     }