Global/Guest restrictions to be saved only on SYSTEM user's xml file

All users on a device share the same global device policies. The existing implementation saves them in each user's individual XML file
This leads to unnecessary duplication and it also doesn't update the global settings in stopped user's XML which could lead to outdated global settings if we change the way we read the XML files in the future.
Similar to that the guest restrictions which is also not attached to a single user is saved in userlist.xml which is intended to store the list of the users.

Since the SYSTEM user is always present on the device, we refactored the device policy global restrictions and guest restrictions writing to save them only in the SYSTEM user's XML file.

Bug: 299475754
Test: atest UserManagerServiceUserInfoTest#testWriteReadDevicePolicyUserRestrictions -c
Test: manually verified the guest and global device policy restrictions are only present on user 0's XML file.
Change-Id: I715f3bc38d4a915930ef5e06fd5a6a1601b9d6dc

2 files changed, 84 insertions, 13 deletions

diff --git a/services/core/java/com/android/server/pm/UserManagerService.java b/services/core/java/com/android/server/pm/UserManagerService.java
index 82d3e9157986..071a03602da7 100644
--- a/services/core/java/com/android/server/pm/UserManagerService.java
+++ b/services/core/java/com/android/server/pm/UserManagerService.java
@@ -75,13 +75,13 @@ import android.content.pm.parsing.FrameworkParsingPackageUtils;
 import android.content.res.Configuration;
 import android.content.res.Resources;
 import android.graphics.Bitmap;
+import android.multiuser.Flags;
 import android.os.Binder;
 import android.os.Build;
 import android.os.Bundle;
 import android.os.Debug;
 import android.os.Environment;
 import android.os.FileUtils;
-import android.os.Flags;
 import android.os.Handler;
 import android.os.IBinder;
 import android.os.IProgressListener;
@@ -1557,7 +1557,7 @@ public class UserManagerService extends IUserManager.Stub {
         logQuietModeEnabled(userId, enableQuietMode, callingPackage);
 
         // Broadcast generic intents for all profiles
-        if (Flags.allowPrivateProfile()) {
+        if (android.os.Flags.allowPrivateProfile()) {
             broadcastProfileAvailabilityChanges(profile, parent.getUserHandle(),
                     enableQuietMode, false);
         }
@@ -3783,6 +3783,8 @@ public class UserManagerService extends IUserManager.Stub {
 
     @GuardedBy({"mPackagesLock"})
     private void readUserListLP() {
+        // Whether guest restrictions are present on userlist.xml
+        boolean guestRestrictionsArePresentOnUserListXml = false;
         try (ResilientAtomicFile file = getUserListFile()) {
             FileInputStream fin = null;
             try {
@@ -3832,6 +3834,7 @@ public class UserManagerService extends IUserManager.Stub {
                                 }
                             }
                         } else if (name.equals(TAG_GUEST_RESTRICTIONS)) {
+                            guestRestrictionsArePresentOnUserListXml = true;
                             while ((type = parser.next()) != XmlPullParser.END_DOCUMENT
                                     && type != XmlPullParser.END_TAG) {
                                 if (type == XmlPullParser.START_TAG) {
@@ -3850,6 +3853,7 @@ public class UserManagerService extends IUserManager.Stub {
 
                 updateUserIds();
                 upgradeIfNecessaryLP();
+                updateUsersWithFeatureFlags(guestRestrictionsArePresentOnUserListXml);
             } catch (Exception e) {
                 // Remove corrupted file and retry.
                 file.failRead(fin, e);
@@ -3875,6 +3879,25 @@ public class UserManagerService extends IUserManager.Stub {
     }
 
     /**
+     * Update any user formats or Xml data that need to be updated based on the current user state
+     * and the feature flag settings.
+     */
+    @GuardedBy({"mPackagesLock"})
+    private void updateUsersWithFeatureFlags(boolean guestRestrictionsArePresentOnUserListXml) {
+        // User Xml re-writes are required when guest restrictions are saved on userlist.xml but
+        // as per the feature flag it should be on the SYSTEM user's xml or guest restrictions
+        // are saved on SYSTEM user's xml but as per the flags it should not be saved there.
+        if (guestRestrictionsArePresentOnUserListXml
+                == Flags.saveGlobalAndGuestRestrictionsOnSystemUserXml()) {
+            for (int userId: getUserIds()) {
+                writeUserLP(getUserDataNoChecks(userId));
+            }
+
+            writeUserListLP();
+        }
+    }
+
+    /**
      * Version of {@link #upgradeIfNecessaryLP()} that takes in the userVersion for testing
      * purposes. For non-tests, use {@link #upgradeIfNecessaryLP()}.
      */
@@ -4389,9 +4412,24 @@ public class UserManagerService extends IUserManager.Stub {
             UserRestrictionsUtils.writeRestrictions(serializer,
                     mBaseUserRestrictions.getRestrictions(userInfo.id), TAG_RESTRICTIONS);
 
-            UserRestrictionsUtils.writeRestrictions(serializer,
-                    mDevicePolicyUserRestrictions.getRestrictions(UserHandle.USER_ALL),
-                    TAG_DEVICE_POLICY_GLOBAL_RESTRICTIONS);
+            if (Flags.saveGlobalAndGuestRestrictionsOnSystemUserXml()) {
+                if (userInfo.id == UserHandle.USER_SYSTEM) {
+                    UserRestrictionsUtils.writeRestrictions(serializer,
+                            mDevicePolicyUserRestrictions.getRestrictions(UserHandle.USER_ALL),
+                            TAG_DEVICE_POLICY_GLOBAL_RESTRICTIONS);
+
+                    serializer.startTag(null, TAG_GUEST_RESTRICTIONS);
+                    synchronized (mGuestRestrictions) {
+                        UserRestrictionsUtils.writeRestrictions(serializer, mGuestRestrictions,
+                                        TAG_RESTRICTIONS);
+                    }
+                    serializer.endTag(null, TAG_GUEST_RESTRICTIONS);
+                }
+            } else {
+                UserRestrictionsUtils.writeRestrictions(serializer,
+                        mDevicePolicyUserRestrictions.getRestrictions(UserHandle.USER_ALL),
+                        TAG_DEVICE_POLICY_GLOBAL_RESTRICTIONS);
+            }
 
             UserRestrictionsUtils.writeRestrictions(serializer,
                     mDevicePolicyUserRestrictions.getRestrictions(userInfo.id),
@@ -4460,12 +4498,15 @@ public class UserManagerService extends IUserManager.Stub {
                 serializer.attributeInt(null, ATTR_USER_VERSION, mUserVersion);
                 serializer.attributeInt(null, ATTR_USER_TYPE_VERSION, mUserTypeVersion);
 
-                serializer.startTag(null, TAG_GUEST_RESTRICTIONS);
-                synchronized (mGuestRestrictions) {
-                    UserRestrictionsUtils
-                            .writeRestrictions(serializer, mGuestRestrictions, TAG_RESTRICTIONS);
+                if (!Flags.saveGlobalAndGuestRestrictionsOnSystemUserXml()) {
+                    serializer.startTag(null, TAG_GUEST_RESTRICTIONS);
+                    synchronized (mGuestRestrictions) {
+                        UserRestrictionsUtils
+                                .writeRestrictions(serializer, mGuestRestrictions,
+                                        TAG_RESTRICTIONS);
+                    }
+                    serializer.endTag(null, TAG_GUEST_RESTRICTIONS);
                 }
-                serializer.endTag(null, TAG_GUEST_RESTRICTIONS);
                 int[] userIdsToWrite;
                 synchronized (mUsersLock) {
                     userIdsToWrite = new int[mUsers.size()];
@@ -4609,6 +4650,19 @@ public class UserManagerService extends IUserManager.Stub {
                     localRestrictions = UserRestrictionsUtils.readRestrictions(parser);
                 } else if (TAG_DEVICE_POLICY_GLOBAL_RESTRICTIONS.equals(tag)) {
                     globalRestrictions = UserRestrictionsUtils.readRestrictions(parser);
+                } else if (name.equals(TAG_GUEST_RESTRICTIONS)) {
+                    while ((type = parser.next()) != XmlPullParser.END_DOCUMENT
+                            && type != XmlPullParser.END_TAG) {
+                        if (type == XmlPullParser.START_TAG) {
+                            if (parser.getName().equals(TAG_RESTRICTIONS)) {
+                                synchronized (mGuestRestrictions) {
+                                    UserRestrictionsUtils
+                                            .readRestrictions(parser, mGuestRestrictions);
+                                }
+                            }
+                            break;
+                        }
+                    }
                 } else if (TAG_ACCOUNT.equals(tag)) {
                     type = parser.next();
                     if (type == XmlPullParser.TEXT) {
diff --git a/services/tests/servicestests/src/com/android/server/pm/UserManagerServiceUserInfoTest.java b/services/tests/servicestests/src/com/android/server/pm/UserManagerServiceUserInfoTest.java
index 9f75cf8d552e..c5ce7f5a0f25 100644
--- a/services/tests/servicestests/src/com/android/server/pm/UserManagerServiceUserInfoTest.java
+++ b/services/tests/servicestests/src/com/android/server/pm/UserManagerServiceUserInfoTest.java
@@ -43,6 +43,7 @@ import android.annotation.UserIdInt;
 import android.app.PropertyInvalidatedCache;
 import android.content.pm.UserInfo;
 import android.content.pm.UserInfo.UserInfoFlag;
+import android.multiuser.Flags;
 import android.os.Looper;
 import android.os.Parcel;
 import android.os.UserHandle;
@@ -124,18 +125,34 @@ public class UserManagerServiceUserInfoTest {
 
         mUserManagerService.putUserInfo(data.info);
 
-        // Set a global and user restriction so they get written out to the user file.
+        //Local restrictions are written to the user specific files and global restrictions
+        // are written to the SYSTEM user file.
         setUserRestrictions(data.info.id, globalRestriction, localRestriction, true);
 
         ByteArrayOutputStream baos = new ByteArrayOutputStream();
         DataOutputStream out = new DataOutputStream(baos);
         mUserManagerService.writeUserLP(data, out);
-        byte[] bytes = baos.toByteArray();
+        byte[] secondaryUserBytes = baos.toByteArray();
+        baos.reset();
+
+        byte[] systemUserBytes = new byte[0];
+        if (Flags.saveGlobalAndGuestRestrictionsOnSystemUserXml()) {
+            UserData systemUserData = new UserData();
+            systemUserData.info = mUserManagerService.getUserInfo(UserHandle.USER_SYSTEM);
+            mUserManagerService.writeUserLP(systemUserData, baos);
+            systemUserBytes = baos.toByteArray();
+        }
 
         // Clear the restrictions to see if they are properly read in from the user file.
         setUserRestrictions(data.info.id, globalRestriction, localRestriction, false);
 
-        mUserManagerService.readUserLP(data.info.id, new ByteArrayInputStream(bytes));
+        //read the secondary and SYSTEM user file to fetch local/global device policy restrictions.
+        mUserManagerService.readUserLP(data.info.id, new ByteArrayInputStream(secondaryUserBytes));
+        if (Flags.saveGlobalAndGuestRestrictionsOnSystemUserXml()) {
+            mUserManagerService.readUserLP(UserHandle.USER_SYSTEM,
+                    new ByteArrayInputStream(systemUserBytes));
+        }
+
         assertTrue(mUserManagerService.hasUserRestrictionOnAnyUser(globalRestriction));
         assertTrue(mUserManagerService.hasUserRestrictionOnAnyUser(localRestriction));
     }