From e51de1a1066d008e87147d54b1abe5844b11d0fd Mon Sep 17 00:00:00 2001 From: Jigar Thakkar Date: Wed, 9 Nov 2022 19:59:57 +0000 Subject: [PATCH 1/2] Disable contacts sync adapter for clone profile We have added a new user property dictate the behaviour to redirect the contact calls to the parent user profile. This will result in blocking all contact write calls done via ContactsProvider in the clone profile. This change adds support to disable contacts syncs for all such users. Test: Tested manually on prototype, CTS to be added later. Bug: 258489060 Change-Id: I0c5287f49d351f543ff34e3a7ae4e72d66d48327 --- .../android/server/content/SyncManager.java | 39 ++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/content/SyncManager.java b/services/core/java/com/android/server/content/SyncManager.java index eb81e70363d40..ebe1040d0ecc6 100644 --- a/services/core/java/com/android/server/content/SyncManager.java +++ b/services/core/java/com/android/server/content/SyncManager.java @@ -27,6 +27,7 @@ import android.accounts.AccountManager; import android.accounts.AccountManagerInternal; import android.annotation.NonNull; import android.annotation.Nullable; +import android.annotation.SuppressLint; import android.annotation.UserIdInt; import android.app.ActivityManagerInternal; import android.app.AppGlobals; @@ -65,6 +66,7 @@ import android.content.pm.RegisteredServicesCache; import android.content.pm.RegisteredServicesCacheListener; import android.content.pm.ResolveInfo; import android.content.pm.UserInfo; +import android.content.pm.UserProperties; import android.database.ContentObserver; import android.net.ConnectivityManager; import android.net.NetworkInfo; @@ -88,6 +90,7 @@ import android.os.SystemProperties; import android.os.UserHandle; import android.os.UserManager; import android.os.WorkSource; +import android.provider.ContactsContract; import android.provider.Settings; import android.text.TextUtils; import android.text.format.TimeMigrationUtils; @@ -800,9 +803,43 @@ public class SyncManager { return mSyncStorageEngine; } + @SuppressLint("AndroidFrameworkRequiresPermission") + private boolean areContactWritesEnabledForUser(UserInfo userInfo) { + final UserManager um = UserManager.get(mContext); + try { + final UserProperties userProperties = um.getUserProperties(userInfo.getUserHandle()); + return !userProperties.getUseParentsContacts(); + } catch (IllegalArgumentException e) { + Log.w(TAG, "Trying to fetch user properties for non-existing/partial user " + + userInfo.getUserHandle()); + return false; + } + } + + /** + * Check if account sync should be disabled for the given user and provider. + * @param userInfo + * @param providerName + * @return true if sync for the account corresponding to the given user and provider should be + * disabled, false otherwise. Also returns false if either of the inputs are null. + */ + private boolean shouldDisableSyncForUser(UserInfo userInfo, String providerName) { + if (userInfo == null || providerName == null) return false; + return providerName.equals(ContactsContract.AUTHORITY) + && !areContactWritesEnabledForUser(userInfo); + } + private int getIsSyncable(Account account, int userId, String providerName) { int isSyncable = mSyncStorageEngine.getIsSyncable(account, userId, providerName); - UserInfo userInfo = UserManager.get(mContext).getUserInfo(userId); + final UserManager um = UserManager.get(mContext); + UserInfo userInfo = um.getUserInfo(userId); + + // Check if the provider is allowed to sync data from linked accounts for the user + if (shouldDisableSyncForUser(userInfo, providerName)) { + Log.w(TAG, "Account sync is disabled for account: " + account + + " userId: " + userId + " provider: " + providerName); + return AuthorityInfo.NOT_SYNCABLE; + } // If it's not a restricted user, return isSyncable. if (userInfo == null || !userInfo.isRestricted()) return isSyncable; From 731afb12408add5b467a7af7fbeb8b327dbe3e07 Mon Sep 17 00:00:00 2001 From: Jigar Thakkar Date: Tue, 15 Nov 2022 12:12:44 +0000 Subject: [PATCH 2/2] Add tests for shouldDisableSyncForUser This change adds a test for checking the behaviour of SyncManager.shouldDisableSyncForUser method. Test: atest SyncManagerTest#testShouldDisableSync Bug: 258489060 Change-Id: Ib782690cefe2c0ee4792d704863f3e67c68e6b99 --- .../android/server/content/SyncManager.java | 18 ++- .../server/content/SyncManagerTest.java | 106 ++++++++++++++++++ 2 files changed, 121 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/content/SyncManager.java b/services/core/java/com/android/server/content/SyncManager.java index ebe1040d0ecc6..dcc98e17fadf5 100644 --- a/services/core/java/com/android/server/content/SyncManager.java +++ b/services/core/java/com/android/server/content/SyncManager.java @@ -102,6 +102,7 @@ import android.util.SparseBooleanArray; import com.android.internal.R; import com.android.internal.annotations.GuardedBy; +import com.android.internal.annotations.VisibleForTesting; import com.android.internal.app.IBatteryStats; import com.android.internal.messages.nano.SystemMessageProto.SystemMessage; import com.android.internal.notification.SystemNotificationChannels; @@ -501,7 +502,7 @@ public class SyncManager { } mJobScheduler = (JobScheduler) mContext.getSystemService( Context.JOB_SCHEDULER_SERVICE); - mJobSchedulerInternal = LocalServices.getService(JobSchedulerInternal.class); + mJobSchedulerInternal = getJobSchedulerInternal(); // Get all persisted syncs from JobScheduler List pendingJobs = mJobScheduler.getAllPendingJobs(); @@ -539,6 +540,11 @@ public class SyncManager { } } + @VisibleForTesting + protected JobSchedulerInternal getJobSchedulerInternal() { + return LocalServices.getService(JobSchedulerInternal.class); + } + /** * @return whether the device most likely has some periodic syncs. */ @@ -648,7 +654,7 @@ public class SyncManager { mPowerManager = (PowerManager) context.getSystemService(Context.POWER_SERVICE); mUserManager = (UserManager) mContext.getSystemService(Context.USER_SERVICE); mAccountManager = (AccountManager) mContext.getSystemService(Context.ACCOUNT_SERVICE); - mAccountManagerInternal = LocalServices.getService(AccountManagerInternal.class); + mAccountManagerInternal = getAccountManagerInternal(); mPackageManagerInternal = LocalServices.getService(PackageManagerInternal.class); mAmi = LocalServices.getService(ActivityManagerInternal.class); @@ -722,6 +728,11 @@ public class SyncManager { mLogger.log("Sync manager initialized: " + Build.FINGERPRINT); } + @VisibleForTesting + protected AccountManagerInternal getAccountManagerInternal() { + return LocalServices.getService(AccountManagerInternal.class); + } + public void onStartUser(int userId) { // Log on the handler to avoid slowing down device boot. mSyncHandler.post(() -> mLogger.log("onStartUser: user=", userId)); @@ -823,7 +834,8 @@ public class SyncManager { * @return true if sync for the account corresponding to the given user and provider should be * disabled, false otherwise. Also returns false if either of the inputs are null. */ - private boolean shouldDisableSyncForUser(UserInfo userInfo, String providerName) { + @VisibleForTesting + protected boolean shouldDisableSyncForUser(UserInfo userInfo, String providerName) { if (userInfo == null || providerName == null) return false; return providerName.equals(ContactsContract.AUTHORITY) && !areContactWritesEnabledForUser(userInfo); diff --git a/services/tests/servicestests/src/com/android/server/content/SyncManagerTest.java b/services/tests/servicestests/src/com/android/server/content/SyncManagerTest.java index c2a81d9453e4c..d4e3d4418c431 100644 --- a/services/tests/servicestests/src/com/android/server/content/SyncManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/content/SyncManagerTest.java @@ -16,12 +16,40 @@ package com.android.server.content; +import static android.content.pm.UserProperties.INHERIT_DEVICE_POLICY_FROM_PARENT; +import static android.content.pm.UserProperties.SHOW_IN_LAUNCHER_WITH_PARENT; +import static android.content.pm.UserProperties.SHOW_IN_SETTINGS_WITH_PARENT; + +import static com.google.common.truth.Truth.assertThat; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; + +import android.accounts.AccountManagerInternal; import android.content.ContentResolver; +import android.content.Context; +import android.content.pm.UserInfo; +import android.content.pm.UserProperties; import android.os.Bundle; +import android.os.UserManager; +import android.provider.ContactsContract; import android.test.suitebuilder.annotation.SmallTest; +import androidx.test.core.app.ApplicationProvider; + +import com.android.server.job.JobSchedulerInternal; + import junit.framework.TestCase; +import org.jetbrains.annotations.NotNull; +import org.junit.Before; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +import java.util.ArrayList; + /** * Tests for SyncManager. * @@ -33,6 +61,43 @@ public class SyncManagerTest extends TestCase { final String KEY_1 = "key_1"; final String KEY_2 = "key_2"; + private SyncManager mSyncManager; + private Context mContext; + + @Mock + private UserManager mUserManager; + @Mock + private AccountManagerInternal mAccountManagerInternal; + @Mock + private JobSchedulerInternal mJobSchedulerInternal; + + private class SyncManagerWithMockedServices extends SyncManager { + + @Override + protected AccountManagerInternal getAccountManagerInternal() { + return mAccountManagerInternal; + } + + @Override + protected JobSchedulerInternal getJobSchedulerInternal() { + return mJobSchedulerInternal; + } + + private SyncManagerWithMockedServices(Context context, boolean factoryTest) { + super(context, factoryTest); + } + } + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + mContext = spy(ApplicationProvider.getApplicationContext()); + when(mContext.getSystemService(Context.USER_SERVICE)).thenReturn(mUserManager); + doNothing().when(mAccountManagerInternal).addOnAppPermissionChangeListener(any()); + when(mJobSchedulerInternal.getSystemScheduledPendingJobs()).thenReturn(new ArrayList<>()); + mSyncManager = new SyncManagerWithMockedServices(mContext, true); + } + public void testSyncExtrasEquals_WithNull() throws Exception { Bundle b1 = new Bundle(); Bundle b2 = new Bundle(); @@ -140,4 +205,45 @@ public class SyncManagerTest extends TestCase { final StringBuilder sb = new StringBuilder(); assertEquals(expected, SyncManager.formatDurationHMS(sb, time * 1000).toString()); } + + private UserInfo createUserInfo(String name, int id, int groupId, int flags) { + final UserInfo ui = new UserInfo(id, name, flags | UserInfo.FLAG_INITIALIZED); + ui.profileGroupId = groupId; + return ui; + } + + @NotNull + private UserProperties getCloneUserProperties() { + return new UserProperties.Builder() + .setStartWithParent(true) + .setShowInLauncher(SHOW_IN_LAUNCHER_WITH_PARENT) + .setShowInSettings(SHOW_IN_SETTINGS_WITH_PARENT) + .setUseParentsContacts(true) + .setInheritDevicePolicy(INHERIT_DEVICE_POLICY_FROM_PARENT) + .build(); + } + + private void mockUserProperties(UserInfo primaryUserInfo, UserInfo cloneUserInfo) { + UserProperties cloneUserProperties = getCloneUserProperties(); + when(mUserManager.getUserProperties(cloneUserInfo.getUserHandle())) + .thenReturn(cloneUserProperties); + // Set default user properties for primary user + when(mUserManager.getUserProperties(primaryUserInfo.getUserHandle())) + .thenReturn(new UserProperties.Builder().build()); + } + + public void testShouldDisableSync() { + UserInfo primaryUserInfo = createUserInfo("primary", 0 /* id */, 0 /* groupId */, + UserInfo.FLAG_PRIMARY | UserInfo.FLAG_ADMIN); + UserInfo cloneUserInfo = createUserInfo("clone", 10 /* id */, 0 /* groupId */, + UserInfo.FLAG_PROFILE); + + mockUserProperties(primaryUserInfo, cloneUserInfo); + + // Clone user accounts must have contact syncs disabled + assertThat(mSyncManager.shouldDisableSyncForUser(cloneUserInfo, + ContactsContract.AUTHORITY)).isTrue(); + assertThat(mSyncManager.shouldDisableSyncForUser(primaryUserInfo, + ContactsContract.AUTHORITY)).isFalse(); + } }