From c93cdef2fa5403ac46767f1853f4e803824d4d20 Mon Sep 17 00:00:00 2001 From: Charles He Date: Fri, 21 Apr 2017 13:45:34 +0100 Subject: [PATCH] Let unlocked profile verifyCredential under unified lock When unified work challenge is enabled and the primary user is unlocked, LockSettingsService should unlock the managed profile subsequently by calling verifyCredential() in most cases. Previously, verifyCredential() is not called on managed profiles when any one of the two conditions are met: 1. when the profile is not yet running 2. when the profile is already unlocked These were introduced to make sure the managed profile stays locked when it is in QUIET_MODE (i.e. work mode off). However, condition 2 is problematic. Specifically, it also prevents auth tokens, etc., from being refreshed (side effects of verifyCredential()), even when the profile is not in QUIET_MODE. We remove condition 2 in this change to make sure verifyCredential() is still called on the managed profile when it is RUNNING_UNLOCKED. Condition 1 alone should be able to handle the QUIET_MODE case. Unit test is also updated in case regression occurs. Bug: 36851574 Test: runtest frameworks-services -c com.android.server.LockSettingsServiceTests Test: runtest frameworks-services -c com.android.server.SyntheticPasswordTests Test: CTS verifier > BYOD managed provisioning > Authentication-bound keys Test: (all of above are run on both sailfish and angler) Change-Id: Ice89c1eedacaf07e076252a2a571a1eb100ef791 --- .../android/server/LockSettingsService.java | 3 +-- .../server/BaseLockSettingsServiceTests.java | 24 ++++++------------- .../server/LockSettingsServiceTests.java | 10 ++++---- 3 files changed, 13 insertions(+), 24 deletions(-) diff --git a/services/core/java/com/android/server/LockSettingsService.java b/services/core/java/com/android/server/LockSettingsService.java index e26630bc4aee0..9f33512135530 100644 --- a/services/core/java/com/android/server/LockSettingsService.java +++ b/services/core/java/com/android/server/LockSettingsService.java @@ -961,8 +961,7 @@ public class LockSettingsService extends ILockSettings.Stub { if (pi.isManagedProfile() && !mLockPatternUtils.isSeparateProfileChallengeEnabled(pi.id) && mStorage.hasChildProfileLock(pi.id) - && mUserManager.isUserRunning(pi.id) - && !mUserManager.isUserUnlocked(pi.id)) { + && mUserManager.isUserRunning(pi.id)) { unlockChildProfile(pi.id); } } diff --git a/services/tests/servicestests/src/com/android/server/BaseLockSettingsServiceTests.java b/services/tests/servicestests/src/com/android/server/BaseLockSettingsServiceTests.java index 9343449696e67..ca7752819b9cf 100644 --- a/services/tests/servicestests/src/com/android/server/BaseLockSettingsServiceTests.java +++ b/services/tests/servicestests/src/com/android/server/BaseLockSettingsServiceTests.java @@ -20,40 +20,27 @@ import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.app.IActivityManager; import android.app.NotificationManager; import android.app.admin.DevicePolicyManager; import android.content.ComponentName; -import android.content.Context; import android.content.pm.UserInfo; -import android.database.sqlite.SQLiteDatabase; import android.os.FileUtils; -import android.os.Handler; import android.os.IProgressListener; -import android.os.RemoteException; import android.os.UserManager; -import android.os.storage.IStorageManager; import android.security.KeyStore; -import android.service.gatekeeper.GateKeeperResponse; -import android.service.gatekeeper.IGateKeeperService; import android.test.AndroidTestCase; import com.android.internal.widget.LockPatternUtils; -import com.android.internal.widget.VerifyCredentialResponse; -import com.android.server.LockSettingsService.SynchronizedStrongAuthTracker; -import com.android.server.LockSettingsStorage.CredentialHash; -import com.android.server.MockGateKeeperService.AuthToken; -import com.android.server.MockGateKeeperService.VerifyHandle; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; import java.io.File; -import java.util.Arrays; import java.util.ArrayList; +import java.util.Arrays; public class BaseLockSettingsServiceTests extends AndroidTestCase { @@ -110,10 +97,9 @@ public class BaseLockSettingsServiceTests extends AndroidTestCase { when(mUserManager.getUserInfo(eq(PRIMARY_USER_ID))).thenReturn(PRIMARY_USER_INFO); mPrimaryUserProfiles.add(PRIMARY_USER_INFO); installChildProfile(MANAGED_PROFILE_USER_ID); - installQuietModeChildProfile(TURNED_OFF_PROFILE_USER_ID); + installAndTurnOffChildProfile(TURNED_OFF_PROFILE_USER_ID); when(mUserManager.getProfiles(eq(PRIMARY_USER_ID))).thenReturn(mPrimaryUserProfiles); when(mUserManager.getUserInfo(eq(SECONDARY_USER_ID))).thenReturn(SECONDARY_USER_INFO); - when(mUserManager.isUserRunning(eq(MANAGED_PROFILE_USER_ID))).thenReturn(true); when(mActivityManager.unlockUser(anyInt(), any(), any(), any())).thenAnswer( new Answer() { @@ -139,12 +125,16 @@ public class BaseLockSettingsServiceTests extends AndroidTestCase { mPrimaryUserProfiles.add(userInfo); when(mUserManager.getUserInfo(eq(profileId))).thenReturn(userInfo); when(mUserManager.getProfileParent(eq(profileId))).thenReturn(PRIMARY_USER_INFO); + when(mUserManager.isUserRunning(eq(profileId))).thenReturn(true); + when(mUserManager.isUserUnlocked(eq(profileId))).thenReturn(true); return userInfo; } - private UserInfo installQuietModeChildProfile(int profileId) { + private UserInfo installAndTurnOffChildProfile(int profileId) { final UserInfo userInfo = installChildProfile(profileId); userInfo.flags |= UserInfo.FLAG_QUIET_MODE; + when(mUserManager.isUserRunning(eq(profileId))).thenReturn(false); + when(mUserManager.isUserUnlocked(eq(profileId))).thenReturn(false); return userInfo; } diff --git a/services/tests/servicestests/src/com/android/server/LockSettingsServiceTests.java b/services/tests/servicestests/src/com/android/server/LockSettingsServiceTests.java index cfc3962ad15d9..25cc426f766b4 100644 --- a/services/tests/servicestests/src/com/android/server/LockSettingsServiceTests.java +++ b/services/tests/servicestests/src/com/android/server/LockSettingsServiceTests.java @@ -98,14 +98,14 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { mService.setSeparateProfileChallengeEnabled(MANAGED_PROFILE_USER_ID, false, null); final long primarySid = mGateKeeperService.getSecureUserId(PRIMARY_USER_ID); final long profileSid = mGateKeeperService.getSecureUserId(MANAGED_PROFILE_USER_ID); - final long turnedOffprofileSid = + final long turnedOffProfileSid = mGateKeeperService.getSecureUserId(TURNED_OFF_PROFILE_USER_ID); assertTrue(primarySid != 0); assertTrue(profileSid != 0); assertTrue(profileSid != primarySid); - assertTrue(turnedOffprofileSid != 0); - assertTrue(turnedOffprofileSid != primarySid); - assertTrue(turnedOffprofileSid != profileSid); + assertTrue(turnedOffProfileSid != 0); + assertTrue(turnedOffProfileSid != primarySid); + assertTrue(turnedOffProfileSid != profileSid); // clear auth token and wait for verify challenge from primary user to re-generate it. mGateKeeperService.clearAuthToken(MANAGED_PROFILE_USER_ID); @@ -119,7 +119,7 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { assertNotNull(mGateKeeperService.getAuthToken(MANAGED_PROFILE_USER_ID)); assertEquals(profileSid, mGateKeeperService.getSecureUserId(MANAGED_PROFILE_USER_ID)); - // Verify that profile which arent't running (e.g. turn off work) don't get unlocked + // Verify that profile which aren't running (e.g. turn off work) don't get unlocked assertNull(mGateKeeperService.getAuthToken(TURNED_OFF_PROFILE_USER_ID)); /* Currently in LockSettingsService.setLockCredential, unlockUser() is called with the new