Adjusting restrictions applied to non-admins.
1. Do not apply DISALLOW_REMOVE_USERS restriction. Enforce this in removeUser method to allow users to remove themselves.
2. Remove DISALLOW_SMS and DISALLOW_OUTGOING_CALLS restrictions that are applied by the framework.
Fixes: 110232368
Bug: 109697822
Test: atest CarUserManagerHelperTest; verified restrictions on mojave.
Change-Id: I036e30088ec9febdab87c19006f32772f2a3b1fc
diff --git a/car-lib/src/android/car/user/CarUserManagerHelper.java b/car-lib/src/android/car/user/CarUserManagerHelper.java
index 7fd7fd2..0ebf0f3 100644
--- a/car-lib/src/android/car/user/CarUserManagerHelper.java
+++ b/car-lib/src/android/car/user/CarUserManagerHelper.java
@@ -59,7 +59,6 @@
* Default set of restrictions for Non-Admin users.
*/
private static final Set<String> DEFAULT_NON_ADMIN_RESTRICTIONS = Sets.newArraySet(
- UserManager.DISALLOW_REMOVE_USER,
UserManager.DISALLOW_FACTORY_RESET
);
@@ -491,6 +490,12 @@
return null;
}
setDefaultNonAdminRestrictions(user, /* enable= */ true);
+
+ // Each non-admin has sms and outgoing call restrictions applied by the UserManager on
+ // creation. We want to enable these permissions by default in the car.
+ setUserRestriction(user, UserManager.DISALLOW_SMS, /* enable= */ false);
+ setUserRestriction(user, UserManager.DISALLOW_OUTGOING_CALLS, /* enable= */ false);
+
assignDefaultIcon(user);
return user;
}
@@ -541,6 +546,12 @@
return false;
}
+ if (!isCurrentProcessAdminUser() && !isCurrentProcessUser(userInfo)) {
+ // If the caller is non-admin, they can only delete themselves.
+ Log.e(TAG, "Non-admins cannot remove other users.");
+ return false;
+ }
+
if (userInfo.id == getCurrentForegroundUserId()) {
startNewGuestSession(guestUserName);
}
diff --git a/tests/carservice_unit_test/src/com/android/car/CarUserManagerHelperTest.java b/tests/carservice_unit_test/src/com/android/car/CarUserManagerHelperTest.java
index d91846a..5c90c97 100644
--- a/tests/carservice_unit_test/src/com/android/car/CarUserManagerHelperTest.java
+++ b/tests/carservice_unit_test/src/com/android/car/CarUserManagerHelperTest.java
@@ -18,6 +18,7 @@
import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -272,15 +273,39 @@
}
@Test
- public void testRemoveUser() {
- // Cannot remove system user.
+ public void testCannotRemoveSystemUser() {
assertThat(mHelper.removeUser(mSystemUser, mGuestUserName)).isFalse();
+ }
- // Removing non-current, non-system user, simply calls removeUser.
- UserInfo userToRemove = createUserInfoForId(mCurrentProcessUser.id + 2);
+ @Test
+ public void testAdminsCanRemoveOtherUsers() {
+ int idToRemove = mCurrentProcessUser.id + 2;
+ UserInfo userToRemove = createUserInfoForId(idToRemove);
+ when(mUserManager.removeUser(idToRemove)).thenReturn(true);
- mHelper.removeUser(userToRemove, mGuestUserName);
- verify(mUserManager).removeUser(mCurrentProcessUser.id + 2);
+ // If Admin is removing non-current, non-system user, simply calls removeUser.
+ when(mUserManager.isAdminUser()).thenReturn(true);
+ assertThat(mHelper.removeUser(userToRemove, mGuestUserName)).isTrue();
+ verify(mUserManager).removeUser(idToRemove);
+ }
+
+ @Test
+ public void testNonAdminsCanOnlyRemoveThemselves() {
+ UserInfo otherUser = createUserInfoForId(mCurrentProcessUser.id + 2);
+
+ // Make current user non-admin.
+ when(mUserManager.isAdminUser()).thenReturn(false);
+
+ // Mock so that removeUser always pretends it's successful.
+ when(mUserManager.removeUser(anyInt())).thenReturn(true);
+
+ // If Non-Admin is trying to remove someone other than themselves, they should fail.
+ assertThat(mHelper.removeUser(otherUser, mGuestUserName)).isFalse();
+ verify(mUserManager, never()).removeUser(otherUser.id);
+
+ // If Non-Admin is trying to remove themselves, that's ok.
+ assertThat(mHelper.removeUser(mCurrentProcessUser, mGuestUserName)).isTrue();
+ verify(mUserManager).removeUser(mCurrentProcessUser.id);
}
@Test
@@ -358,17 +383,18 @@
@Test
public void testDefaultNonAdminRestrictions() {
String testUserName = "Test User";
- int testUserId = 20;
- boolean restrictionEnabled = true;
- UserInfo newNonAdmin = createUserInfoForId(testUserId);
+ int userId = 20;
+ UserInfo newNonAdmin = createUserInfoForId(userId);
when(mUserManager.createUser(testUserName, /* flags= */ 0)).thenReturn(newNonAdmin);
mHelper.createNewNonAdminUser(testUserName);
verify(mUserManager).setUserRestriction(
- UserManager.DISALLOW_REMOVE_USER, restrictionEnabled, UserHandle.of(testUserId));
+ UserManager.DISALLOW_FACTORY_RESET, /* enable= */ true, UserHandle.of(userId));
verify(mUserManager).setUserRestriction(
- UserManager.DISALLOW_FACTORY_RESET, restrictionEnabled, UserHandle.of(testUserId));
+ UserManager.DISALLOW_SMS, /* enable= */ false, UserHandle.of(userId));
+ verify(mUserManager).setUserRestriction(
+ UserManager.DISALLOW_OUTGOING_CALLS, /* enable= */ false, UserHandle.of(userId));
}
@Test
@@ -381,8 +407,6 @@
mHelper.assignAdminPrivileges(testInfo);
verify(mUserManager).setUserRestriction(
- UserManager.DISALLOW_REMOVE_USER, restrictionEnabled, UserHandle.of(testUserId));
- verify(mUserManager).setUserRestriction(
UserManager.DISALLOW_FACTORY_RESET, restrictionEnabled, UserHandle.of(testUserId));
}
