Decouple VpnManagerService from ConnectivityServiceTest
The test scope of ConnectivityService should verify how and
what ConnectivityService works in the unit test, so remove
the VpnManagerService related mocking from ConnectivityServiceTest.
This commit replaces the way to handle package added/remove and
user unlock intent in VpnManagerService. The flows interacting
with those intents in VpnManagerService are verified in
VpnManagerServiceTest, so unit test still maintains the same
coverage.
Bug: 230548427
Test: atest ConnectivityServiceTest
Change-Id: Ia2279bd1023296e3c949fa05bd74e921bf17da79
diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
index 4b832dd..2397663 100644
--- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
@@ -35,7 +35,6 @@
import static android.content.Intent.ACTION_PACKAGE_REPLACED;
import static android.content.Intent.ACTION_USER_ADDED;
import static android.content.Intent.ACTION_USER_REMOVED;
-import static android.content.Intent.ACTION_USER_UNLOCKED;
import static android.content.pm.PackageInfo.REQUESTED_PERMISSION_GRANTED;
import static android.content.pm.PackageManager.FEATURE_ETHERNET;
import static android.content.pm.PackageManager.FEATURE_WIFI;
@@ -375,6 +374,7 @@
import com.android.server.connectivity.UidRangeUtils;
import com.android.server.connectivity.Vpn;
import com.android.server.connectivity.VpnProfileStore;
+import com.android.server.net.LockdownVpnTracker;
import com.android.server.net.NetworkPinner;
import com.android.testutils.DevSdkIgnoreRule;
import com.android.testutils.DevSdkIgnoreRunner;
@@ -520,7 +520,6 @@
private MockContext mServiceContext;
private HandlerThread mCsHandlerThread;
- private HandlerThread mVMSHandlerThread;
private ConnectivityServiceDependencies mDeps;
private ConnectivityService mService;
private WrappedConnectivityManager mCm;
@@ -536,7 +535,6 @@
private TestNetIdManager mNetIdManager;
private QosCallbackMockHelper mQosCallbackMockHelper;
private QosCallbackTracker mQosCallbackTracker;
- private VpnManagerService mVpnManagerService;
private TestNetworkCallback mDefaultNetworkCallback;
private TestNetworkCallback mSystemDefaultNetworkCallback;
private TestNetworkCallback mProfileDefaultNetworkCallback;
@@ -1584,32 +1582,6 @@
return ranges.stream().map(r -> new UidRangeParcel(r, r)).toArray(UidRangeParcel[]::new);
}
- private VpnManagerService makeVpnManagerService() {
- final VpnManagerService.Dependencies deps = new VpnManagerService.Dependencies() {
- public int getCallingUid() {
- return mDeps.getCallingUid();
- }
-
- public HandlerThread makeHandlerThread() {
- return mVMSHandlerThread;
- }
-
- @Override
- public VpnProfileStore getVpnProfileStore() {
- return mVpnProfileStore;
- }
-
- public INetd getNetd() {
- return mMockNetd;
- }
-
- public INetworkManagementService getINetworkManagementService() {
- return mNetworkManagementService;
- }
- };
- return new VpnManagerService(mServiceContext, deps);
- }
-
private void assertVpnTransportInfo(NetworkCapabilities nc, int type) {
assertNotNull(nc);
final TransportInfo ti = nc.getTransportInfo();
@@ -1621,17 +1593,12 @@
private void processBroadcast(Intent intent) {
mServiceContext.sendBroadcast(intent);
- HandlerUtils.waitForIdle(mVMSHandlerThread, TIMEOUT_MS);
waitForIdle();
}
private void mockVpn(int uid) {
- synchronized (mVpnManagerService.mVpns) {
- int userId = UserHandle.getUserId(uid);
- mMockVpn = new MockVpn(userId);
- // Every running user always has a Vpn in the mVpns array, even if no VPN is running.
- mVpnManagerService.mVpns.put(userId, mMockVpn);
- }
+ int userId = UserHandle.getUserId(uid);
+ mMockVpn = new MockVpn(userId);
}
private void mockUidNetworkingBlocked() {
@@ -1812,7 +1779,6 @@
initAlarmManager(mAlarmManager, mAlarmManagerThread.getThreadHandler());
mCsHandlerThread = new HandlerThread("TestConnectivityService");
- mVMSHandlerThread = new HandlerThread("TestVpnManagerService");
mProxyTracker = new ProxyTracker(mServiceContext, mock(Handler.class),
16 /* EVENT_PROXY_HAS_CHANGED */);
@@ -1841,8 +1807,7 @@
mCm = new WrappedConnectivityManager(InstrumentationRegistry.getContext(), mService);
mService.systemReadyInternal();
verify(mMockDnsResolver).registerUnsolicitedEventListener(any());
- mVpnManagerService = makeVpnManagerService();
- mVpnManagerService.systemReady();
+
mockVpn(Process.myUid());
mCm.bindProcessToNetwork(null);
mQosCallbackTracker = mock(QosCallbackTracker.class);
@@ -8465,12 +8430,8 @@
doReturn(UserHandle.getUid(RESTRICTED_USER, VPN_UID)).when(mPackageManager)
.getPackageUidAsUser(ALWAYS_ON_PACKAGE, RESTRICTED_USER);
- final Intent addedIntent = new Intent(ACTION_USER_ADDED);
- addedIntent.putExtra(Intent.EXTRA_USER, UserHandle.of(RESTRICTED_USER));
- addedIntent.putExtra(Intent.EXTRA_USER_HANDLE, RESTRICTED_USER);
-
- // Send a USER_ADDED broadcast for it.
- processBroadcast(addedIntent);
+ // New user added
+ mMockVpn.onUserAdded(RESTRICTED_USER);
// Expect that the VPN UID ranges contain both |uid| and the UID range for the newly-added
// restricted user.
@@ -8494,11 +8455,8 @@
&& caps.hasTransport(TRANSPORT_VPN)
&& !caps.hasTransport(TRANSPORT_WIFI));
- // Send a USER_REMOVED broadcast and expect to lose the UID range for the restricted user.
- final Intent removedIntent = new Intent(ACTION_USER_REMOVED);
- removedIntent.putExtra(Intent.EXTRA_USER, UserHandle.of(RESTRICTED_USER));
- removedIntent.putExtra(Intent.EXTRA_USER_HANDLE, RESTRICTED_USER);
- processBroadcast(removedIntent);
+ // User removed and expect to lose the UID range for the restricted user.
+ mMockVpn.onUserRemoved(RESTRICTED_USER);
// Expect that the VPN gains the UID range for the restricted user, and that the capability
// change made just before that (i.e., loss of TRANSPORT_WIFI) is preserved.
@@ -8551,6 +8509,7 @@
doReturn(asList(PRIMARY_USER_INFO, RESTRICTED_USER_INFO)).when(mUserManager)
.getAliveUsers();
// TODO: check that VPN app within restricted profile still has access, etc.
+ mMockVpn.onUserAdded(RESTRICTED_USER);
final Intent addedIntent = new Intent(ACTION_USER_ADDED);
addedIntent.putExtra(Intent.EXTRA_USER, UserHandle.of(RESTRICTED_USER));
addedIntent.putExtra(Intent.EXTRA_USER_HANDLE, RESTRICTED_USER);
@@ -8562,6 +8521,7 @@
doReturn(asList(PRIMARY_USER_INFO)).when(mUserManager).getAliveUsers();
// Send a USER_REMOVED broadcast and expect to lose the UID range for the restricted user.
+ mMockVpn.onUserRemoved(RESTRICTED_USER);
final Intent removedIntent = new Intent(ACTION_USER_REMOVED);
removedIntent.putExtra(Intent.EXTRA_USER, UserHandle.of(RESTRICTED_USER));
removedIntent.putExtra(Intent.EXTRA_USER_HANDLE, RESTRICTED_USER);
@@ -9257,7 +9217,7 @@
doAsUid(Process.SYSTEM_UID, () -> mCm.unregisterNetworkCallback(perUidCb));
}
- private void setupLegacyLockdownVpn() {
+ private VpnProfile setupLegacyLockdownVpn() {
final String profileName = "testVpnProfile";
final byte[] profileTag = profileName.getBytes(StandardCharsets.UTF_8);
doReturn(profileTag).when(mVpnProfileStore).get(Credentials.LOCKDOWN_VPN);
@@ -9269,6 +9229,8 @@
profile.type = VpnProfile.TYPE_IPSEC_XAUTH_PSK;
final byte[] encodedProfile = profile.encode();
doReturn(encodedProfile).when(mVpnProfileStore).get(Credentials.VPN + profileName);
+
+ return profile;
}
private void establishLegacyLockdownVpn(Network underlying) throws Exception {
@@ -9301,21 +9263,28 @@
new Handler(ConnectivityThread.getInstanceLooper()));
// Pretend lockdown VPN was configured.
- setupLegacyLockdownVpn();
+ final VpnProfile profile = setupLegacyLockdownVpn();
// LockdownVpnTracker disables the Vpn teardown code and enables lockdown.
// Check the VPN's state before it does so.
assertTrue(mMockVpn.getEnableTeardown());
assertFalse(mMockVpn.getLockdown());
- // Send a USER_UNLOCKED broadcast so CS starts LockdownVpnTracker.
- final int userId = UserHandle.getUserId(Process.myUid());
- final Intent addedIntent = new Intent(ACTION_USER_UNLOCKED);
- addedIntent.putExtra(Intent.EXTRA_USER, UserHandle.of(userId));
- addedIntent.putExtra(Intent.EXTRA_USER_HANDLE, userId);
- processBroadcast(addedIntent);
+ // VMSHandlerThread was used inside VpnManagerService and taken into LockDownVpnTracker.
+ // VpnManagerService was decoupled from this test but this handlerThread is still required
+ // in LockDownVpnTracker. Keep it until LockDownVpnTracker related verification is moved to
+ // its own test.
+ final HandlerThread VMSHandlerThread = new HandlerThread("TestVpnManagerService");
+ VMSHandlerThread.start();
+ // LockdownVpnTracker is created from VpnManagerService but VpnManagerService is decoupled
+ // from ConnectivityServiceTest. Create it directly to simulate LockdownVpnTracker is
+ // created.
+ // TODO: move LockdownVpnTracker related tests to its own test.
// Lockdown VPN disables teardown and enables lockdown.
+ final LockdownVpnTracker lockdownVpnTracker = new LockdownVpnTracker(mServiceContext,
+ VMSHandlerThread.getThreadHandler(), mMockVpn, profile);
+ lockdownVpnTracker.init();
assertFalse(mMockVpn.getEnableTeardown());
assertTrue(mMockVpn.getLockdown());
@@ -9485,6 +9454,8 @@
mMockVpn.expectStopVpnRunnerPrivileged();
callback.expectCallback(CallbackEntry.LOST, mMockVpn);
b2.expectBroadcast();
+
+ VMSHandlerThread.quitSafely();
}
@Test @IgnoreUpTo(Build.VERSION_CODES.S_V2)