From ee2699379f0c2ce751442c318a1961ee8ef5732d Mon Sep 17 00:00:00 2001 From: Mark Date: Fri, 10 Jun 2022 16:55:40 +0000 Subject: [PATCH] Stop using adoptShellPermissionIdentity in setUp Using adoptShellPermissionIdentity in setUp to use the shell permission idnetity in whole test make the test not easy to development. If the test want to use the library which would also start/stop shell permission identity, this let the test lose shell identify unexpectly after using that library. Only adopt shell permission idnetity when necessary instead. Test: atest CtsTetheringTest Change-Id: I6cf24e6cbf227944bc914446337a247dc350562a --- .../tethering/mts/TetheringModuleTest.java | 37 ++++----- .../net/cts/ConnectivityManagerTest.java | 13 ++-- .../net/cts/util/CtsTetheringUtils.java | 70 +++++++++++------ .../tethering/cts/TetheringManagerTest.java | 77 +++++++++---------- 4 files changed, 104 insertions(+), 93 deletions(-) diff --git a/Tethering/tests/mts/src/android/tethering/mts/TetheringModuleTest.java b/Tethering/tests/mts/src/android/tethering/mts/TetheringModuleTest.java index 4525568d9a..85ee466d54 100644 --- a/Tethering/tests/mts/src/android/tethering/mts/TetheringModuleTest.java +++ b/Tethering/tests/mts/src/android/tethering/mts/TetheringModuleTest.java @@ -15,27 +15,23 @@ */ package android.tethering.mts; -import static android.Manifest.permission.ACCESS_WIFI_STATE; import static android.Manifest.permission.MANAGE_TEST_NETWORKS; -import static android.Manifest.permission.NETWORK_SETTINGS; import static android.Manifest.permission.READ_DEVICE_CONFIG; -import static android.Manifest.permission.TETHER_PRIVILEGED; import static android.Manifest.permission.WRITE_SETTINGS; import static android.net.TetheringManager.TETHERING_WIFI; import static android.provider.DeviceConfig.NAMESPACE_CONNECTIVITY; import static com.android.testutils.TestNetworkTrackerKt.initTestNetwork; +import static com.android.testutils.TestPermissionUtil.runAsShell; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; -import android.app.UiAutomation; import android.content.Context; import android.net.IpPrefix; import android.net.LinkAddress; import android.net.TetheringInterface; -import android.net.TetheringManager; import android.net.cts.util.CtsTetheringUtils; import android.net.cts.util.CtsTetheringUtils.TestTetheringEventCallback; import android.provider.DeviceConfig; @@ -46,7 +42,6 @@ import androidx.test.runner.AndroidJUnit4; import com.android.testutils.TestNetworkTracker; -import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -60,26 +55,14 @@ import java.util.List; @RunWith(AndroidJUnit4.class) public class TetheringModuleTest { private Context mContext; - private TetheringManager mTm; private CtsTetheringUtils mCtsTetheringUtils; - private UiAutomation mUiAutomation = - InstrumentationRegistry.getInstrumentation().getUiAutomation(); - @Before public void setUp() throws Exception { - mUiAutomation.adoptShellPermissionIdentity(MANAGE_TEST_NETWORKS, NETWORK_SETTINGS, - WRITE_SETTINGS, READ_DEVICE_CONFIG, TETHER_PRIVILEGED, ACCESS_WIFI_STATE); mContext = InstrumentationRegistry.getContext(); - mTm = mContext.getSystemService(TetheringManager.class); mCtsTetheringUtils = new CtsTetheringUtils(mContext); } - @After - public void tearDown() throws Exception { - mUiAutomation.dropShellPermissionIdentity(); - } - @Test public void testSwitchBasePrefixRangeWhenConflict() throws Exception { addressConflictTest(true); @@ -130,10 +113,8 @@ public class TetheringModuleTest { mCtsTetheringUtils.stopWifiTethering(tetherEventCallback); } finally { - if (tnt != null) { - tnt.teardown(); - } - mTm.stopAllTethering(); + teardown(tnt); + mCtsTetheringUtils.stopAllTethering(); mCtsTetheringUtils.unregisterTetheringEventCallback(tetherEventCallback); } } @@ -169,11 +150,19 @@ public class TetheringModuleTest { } private TestNetworkTracker setUpTestNetwork(final LinkAddress address) throws Exception { - return initTestNetwork(mContext, address, 10_000L /* test timeout ms*/); + return runAsShell(MANAGE_TEST_NETWORKS, WRITE_SETTINGS, + () -> initTestNetwork(mContext, address, 10_000L /* test timeout ms*/)); } + private void teardown(TestNetworkTracker tracker) throws Exception { + if (tracker == null) return; + + runAsShell(MANAGE_TEST_NETWORKS, () -> tracker.teardown()); + } + public static boolean isFeatureEnabled(final String name, final boolean defaultValue) { - return DeviceConfig.getBoolean(NAMESPACE_CONNECTIVITY, name, defaultValue); + return runAsShell(READ_DEVICE_CONFIG, + () -> DeviceConfig.getBoolean(NAMESPACE_CONNECTIVITY, name, defaultValue)); } } diff --git a/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java b/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java index 766d62f70a..a416c6a0de 100644 --- a/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java +++ b/tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java @@ -2500,13 +2500,11 @@ public class ConnectivityManagerTest { final CtsTetheringUtils tetherUtils = new CtsTetheringUtils(mContext); try { tetherEventCallback = tetherUtils.registerTetheringEventCallback(); - // Adopt for NETWORK_SETTINGS permission. - mUiAutomation.adoptShellPermissionIdentity(); // start tethering tetherEventCallback.assumeWifiTetheringSupported(mContext); tetherUtils.startWifiTethering(tetherEventCallback); // Update setting to verify the behavior. - mCm.setAirplaneMode(true); + setAirplaneMode(true); ConnectivitySettingsManager.setPrivateDnsMode(mContext, ConnectivitySettingsManager.PRIVATE_DNS_MODE_OFF); ConnectivitySettingsManager.setNetworkAvoidBadWifi(mContext, @@ -2514,7 +2512,7 @@ public class ConnectivityManagerTest { assertEquals(AIRPLANE_MODE_ON, Settings.Global.getInt( mContext.getContentResolver(), Settings.Global.AIRPLANE_MODE_ON)); // Verify factoryReset - mCm.factoryReset(); + runAsShell(NETWORK_SETTINGS, () -> mCm.factoryReset()); verifySettings(AIRPLANE_MODE_OFF, ConnectivitySettingsManager.PRIVATE_DNS_MODE_OPPORTUNISTIC, ConnectivitySettingsManager.NETWORK_AVOID_BAD_WIFI_PROMPT); @@ -2522,17 +2520,20 @@ public class ConnectivityManagerTest { tetherEventCallback.expectNoTetheringActive(); } finally { // Restore settings. - mCm.setAirplaneMode(false); + setAirplaneMode(false); ConnectivitySettingsManager.setNetworkAvoidBadWifi(mContext, curAvoidBadWifi); ConnectivitySettingsManager.setPrivateDnsMode(mContext, curPrivateDnsMode); if (tetherEventCallback != null) { tetherUtils.unregisterTetheringEventCallback(tetherEventCallback); } tetherUtils.stopAllTethering(); - mUiAutomation.dropShellPermissionIdentity(); } } + private void setAirplaneMode(boolean enable) { + runAsShell(NETWORK_SETTINGS, () -> mCm.setAirplaneMode(enable)); + } + /** * Verify that {@link ConnectivityManager#setProfileNetworkPreference} cannot be called * without required NETWORK_STACK permissions. diff --git a/tests/cts/net/util/java/android/net/cts/util/CtsTetheringUtils.java b/tests/cts/net/util/java/android/net/cts/util/CtsTetheringUtils.java index 8c5372d5ee..d5e9c9e43c 100644 --- a/tests/cts/net/util/java/android/net/cts/util/CtsTetheringUtils.java +++ b/tests/cts/net/util/java/android/net/cts/util/CtsTetheringUtils.java @@ -16,12 +16,18 @@ package android.net.cts.util; +import static android.Manifest.permission.ACCESS_NETWORK_STATE; +import static android.Manifest.permission.ACCESS_WIFI_STATE; +import static android.Manifest.permission.NETWORK_SETTINGS; +import static android.Manifest.permission.TETHER_PRIVILEGED; import static android.net.TetheringManager.TETHERING_WIFI; import static android.net.TetheringManager.TETHER_ERROR_NO_ERROR; import static android.net.TetheringManager.TETHER_HARDWARE_OFFLOAD_FAILED; import static android.net.TetheringManager.TETHER_HARDWARE_OFFLOAD_STARTED; import static android.net.TetheringManager.TETHER_HARDWARE_OFFLOAD_STOPPED; +import static com.android.testutils.TestPermissionUtil.runAsShell; + import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; @@ -396,9 +402,14 @@ public final class CtsTetheringUtils { } } + private static boolean isWifiEnabled(final WifiManager wm) { + return runAsShell(ACCESS_WIFI_STATE, () -> wm.isWifiEnabled()); + + } + private static void waitForWifiEnabled(final Context ctx) throws Exception { WifiManager wm = ctx.getSystemService(WifiManager.class); - if (wm.isWifiEnabled()) return; + if (isWifiEnabled(wm)) return; final ConditionVariable mWaiting = new ConditionVariable(); final BroadcastReceiver receiver = new BroadcastReceiver() { @@ -406,7 +417,7 @@ public final class CtsTetheringUtils { public void onReceive(Context context, Intent intent) { String action = intent.getAction(); if (WifiManager.WIFI_STATE_CHANGED_ACTION.equals(action)) { - if (wm.isWifiEnabled()) mWaiting.open(); + if (isWifiEnabled(wm)) mWaiting.open(); } } }; @@ -414,7 +425,7 @@ public final class CtsTetheringUtils { ctx.registerReceiver(receiver, new IntentFilter(WifiManager.WIFI_STATE_CHANGED_ACTION)); if (!mWaiting.block(DEFAULT_TIMEOUT_MS)) { assertTrue("Wifi did not become enabled after " + DEFAULT_TIMEOUT_MS + "ms", - wm.isWifiEnabled()); + isWifiEnabled(wm)); } } finally { ctx.unregisterReceiver(receiver); @@ -425,14 +436,16 @@ public final class CtsTetheringUtils { final TestTetheringEventCallback tetherEventCallback = new TestTetheringEventCallback(); - mTm.registerTetheringEventCallback(c -> c.run() /* executor */, tetherEventCallback); - tetherEventCallback.expectCallbackStarted(); + runAsShell(ACCESS_NETWORK_STATE, NETWORK_SETTINGS, () -> { + mTm.registerTetheringEventCallback(c -> c.run() /* executor */, tetherEventCallback); + tetherEventCallback.expectCallbackStarted(); + }); return tetherEventCallback; } public void unregisterTetheringEventCallback(final TestTetheringEventCallback callback) { - mTm.unregisterTetheringEventCallback(callback); + runAsShell(ACCESS_NETWORK_STATE, () -> mTm.unregisterTetheringEventCallback(callback)); } private static List getWifiTetherableInterfaceRegexps( @@ -446,11 +459,11 @@ public final class CtsTetheringUtils { if (!pm.hasSystemFeature(PackageManager.FEATURE_WIFI)) return false; final WifiManager wm = ctx.getSystemService(WifiManager.class); // Wifi feature flags only work when wifi is on. - final boolean previousWifiEnabledState = wm.isWifiEnabled(); + final boolean previousWifiEnabledState = isWifiEnabled(wm); try { if (!previousWifiEnabledState) SystemUtil.runShellCommand("svc wifi enable"); waitForWifiEnabled(ctx); - return wm.isPortableHotspotSupported(); + return runAsShell(ACCESS_WIFI_STATE, () -> wm.isPortableHotspotSupported()); } finally { if (!previousWifiEnabledState) SystemUtil.runShellCommand("svc wifi disable"); } @@ -463,17 +476,20 @@ public final class CtsTetheringUtils { final StartTetheringCallback startTetheringCallback = new StartTetheringCallback(); final TetheringRequest request = new TetheringRequest.Builder(TETHERING_WIFI) .setShouldShowEntitlementUi(false).build(); - mTm.startTethering(request, c -> c.run() /* executor */, startTetheringCallback); - startTetheringCallback.verifyTetheringStarted(); - final TetheringInterface iface = - callback.expectTetheredInterfacesChanged(wifiRegexs, TETHERING_WIFI); + return runAsShell(TETHER_PRIVILEGED, () -> { + mTm.startTethering(request, c -> c.run() /* executor */, startTetheringCallback); + startTetheringCallback.verifyTetheringStarted(); - callback.expectOneOfOffloadStatusChanged( - TETHER_HARDWARE_OFFLOAD_STARTED, - TETHER_HARDWARE_OFFLOAD_FAILED); + final TetheringInterface iface = + callback.expectTetheredInterfacesChanged(wifiRegexs, TETHERING_WIFI); - return iface; + callback.expectOneOfOffloadStatusChanged( + TETHER_HARDWARE_OFFLOAD_STARTED, + TETHER_HARDWARE_OFFLOAD_FAILED); + + return iface; + }); } private static class StopSoftApCallback implements SoftApCallback { @@ -501,23 +517,33 @@ public final class CtsTetheringUtils { public void expectSoftApDisabled() { final StopSoftApCallback callback = new StopSoftApCallback(); try { - mWm.registerSoftApCallback(c -> c.run(), callback); + runAsShell(NETWORK_SETTINGS, () -> mWm.registerSoftApCallback(c -> c.run(), callback)); // registerSoftApCallback will immediately call the callback with the current state, so // this callback will fire even if softAp is already disabled. callback.waitForSoftApStopped(); } finally { - mWm.unregisterSoftApCallback(callback); + runAsShell(NETWORK_SETTINGS, () -> mWm.unregisterSoftApCallback(callback)); } } public void stopWifiTethering(final TestTetheringEventCallback callback) { - mTm.stopTethering(TETHERING_WIFI); + runAsShell(TETHER_PRIVILEGED, () -> { + mTm.stopTethering(TETHERING_WIFI); + callback.expectNoTetheringActive(); + callback.expectOneOfOffloadStatusChanged(TETHER_HARDWARE_OFFLOAD_STOPPED); + }); expectSoftApDisabled(); - callback.expectNoTetheringActive(); - callback.expectOneOfOffloadStatusChanged(TETHER_HARDWARE_OFFLOAD_STOPPED); } public void stopAllTethering() { - mTm.stopAllTethering(); + final TestTetheringEventCallback callback = registerTetheringEventCallback(); + try { + runAsShell(TETHER_PRIVILEGED, () -> { + mTm.stopAllTethering(); + callback.expectNoTetheringActive(); + }); + } finally { + unregisterTetheringEventCallback(callback); + } } } diff --git a/tests/cts/tethering/src/android/tethering/cts/TetheringManagerTest.java b/tests/cts/tethering/src/android/tethering/cts/TetheringManagerTest.java index bd1b74ac0e..274596f105 100644 --- a/tests/cts/tethering/src/android/tethering/cts/TetheringManagerTest.java +++ b/tests/cts/tethering/src/android/tethering/cts/TetheringManagerTest.java @@ -15,6 +15,8 @@ */ package android.tethering.test; +import static android.Manifest.permission.MODIFY_PHONE_STATE; +import static android.Manifest.permission.TETHER_PRIVILEGED; import static android.content.pm.PackageManager.FEATURE_TELEPHONY; import static android.net.NetworkCapabilities.NET_CAPABILITY_DUN; import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; @@ -28,6 +30,8 @@ import static android.net.TetheringManager.TETHER_ERROR_NO_CHANGE_TETHERING_PERM import static android.net.TetheringManager.TETHER_ERROR_NO_ERROR; import static android.net.cts.util.CtsTetheringUtils.isAnyIfaceMatch; +import static com.android.testutils.TestPermissionUtil.runAsShell; + import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -37,7 +41,6 @@ import static org.junit.Assert.fail; import static org.junit.Assume.assumeFalse; import static org.junit.Assume.assumeTrue; -import android.app.UiAutomation; import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; @@ -97,21 +100,8 @@ public class TetheringManagerTest { private static final int DEFAULT_TIMEOUT_MS = 60_000; - private void adoptShellPermissionIdentity() { - final UiAutomation uiAutomation = - InstrumentationRegistry.getInstrumentation().getUiAutomation(); - uiAutomation.adoptShellPermissionIdentity(); - } - - private void dropShellPermissionIdentity() { - final UiAutomation uiAutomation = - InstrumentationRegistry.getInstrumentation().getUiAutomation(); - uiAutomation.dropShellPermissionIdentity(); - } - @Before public void setUp() throws Exception { - adoptShellPermissionIdentity(); mContext = InstrumentationRegistry.getContext(); mCm = (ConnectivityManager) mContext.getSystemService(Context.CONNECTIVITY_SERVICE); mTM = (TetheringManager) mContext.getSystemService(Context.TETHERING_SERVICE); @@ -128,9 +118,8 @@ public class TetheringManagerTest { @After public void tearDown() throws Exception { - mTM.stopAllTethering(); + mCtsTetheringUtils.stopAllTethering(); mContext.unregisterReceiver(mTetherChangeReceiver); - dropShellPermissionIdentity(); } private class TetherChangeReceiver extends BroadcastReceiver { @@ -208,22 +197,19 @@ public class TetheringManagerTest { mCtsTetheringUtils.registerTetheringEventCallback(); try { tetherEventCallback.assumeWifiTetheringSupported(mContext); + tetherEventCallback.expectNoTetheringActive(); + + final String[] wifiRegexs = mTM.getTetherableWifiRegexs(); + mCtsTetheringUtils.startWifiTethering(tetherEventCallback); + + mTetherChangeReceiver.expectTethering(true /* active */, wifiRegexs); + + mCtsTetheringUtils.stopWifiTethering(tetherEventCallback); + mTetherChangeReceiver.expectTethering(false /* active */, wifiRegexs); } finally { mCtsTetheringUtils.unregisterTetheringEventCallback(tetherEventCallback); } - final String[] wifiRegexs = mTM.getTetherableWifiRegexs(); - final StartTetheringCallback startTetheringCallback = new StartTetheringCallback(); - final TetheringRequest request = new TetheringRequest.Builder(TETHERING_WIFI) - .setShouldShowEntitlementUi(false).build(); - mTM.startTethering(request, c -> c.run() /* executor */, startTetheringCallback); - startTetheringCallback.verifyTetheringStarted(); - - mTetherChangeReceiver.expectTethering(true /* active */, wifiRegexs); - - mTM.stopTethering(TETHERING_WIFI); - mCtsTetheringUtils.expectSoftApDisabled(); - mTetherChangeReceiver.expectTethering(false /* active */, wifiRegexs); } @Test @@ -267,7 +253,7 @@ public class TetheringManagerTest { mCtsTetheringUtils.stopWifiTethering(tetherEventCallback); try { - final int ret = mTM.tether(wifiTetheringIface); + final int ret = runAsShell(TETHER_PRIVILEGED, () -> mTM.tether(wifiTetheringIface)); // There is no guarantee that the wifi interface will be available after disabling // the hotspot, so don't fail the test if the call to tether() fails. if (ret == TETHER_ERROR_NO_ERROR) { @@ -277,7 +263,7 @@ public class TetheringManagerTest { new TetheringInterface(TETHERING_WIFI, wifiTetheringIface)); } } finally { - mTM.untether(wifiTetheringIface); + runAsShell(TETHER_PRIVILEGED, () -> mTM.untether(wifiTetheringIface)); } } finally { mCtsTetheringUtils.unregisterTetheringEventCallback(tetherEventCallback); @@ -320,7 +306,7 @@ public class TetheringManagerTest { mCtsTetheringUtils.startWifiTethering(tetherEventCallback); - mTM.stopAllTethering(); + mCtsTetheringUtils.stopAllTethering(); tetherEventCallback.expectNoTetheringActive(); } finally { mCtsTetheringUtils.unregisterTetheringEventCallback(tetherEventCallback); @@ -329,7 +315,6 @@ public class TetheringManagerTest { @Test public void testEnableTetheringPermission() throws Exception { - dropShellPermissionIdentity(); final StartTetheringCallback startTetheringCallback = new StartTetheringCallback(); mTM.startTethering(new TetheringRequest.Builder(TETHERING_WIFI).build(), c -> c.run() /* executor */, startTetheringCallback); @@ -352,15 +337,21 @@ public class TetheringManagerTest { private void assertEntitlementResult(final Consumer functor, final int expect) throws Exception { - final EntitlementResultListener listener = new EntitlementResultListener(); - functor.accept(listener); + runAsShell(TETHER_PRIVILEGED, () -> { + final EntitlementResultListener listener = new EntitlementResultListener(); + functor.accept(listener); - assertEquals(expect, listener.get(DEFAULT_TIMEOUT_MS, TimeUnit.MILLISECONDS)); + assertEquals(expect, listener.get(DEFAULT_TIMEOUT_MS, TimeUnit.MILLISECONDS)); + }); + } + + private boolean isTetheringSupported() { + return runAsShell(TETHER_PRIVILEGED, () -> mTM.isTetheringSupported()); } @Test public void testRequestLatestEntitlementResult() throws Exception { - assumeTrue(mTM.isTetheringSupported()); + assumeTrue(isTetheringSupported()); assumeTrue(mPm.hasSystemFeature(FEATURE_TELEPHONY)); // Verify that requestLatestTetheringEntitlementResult() can get entitlement // result(TETHER_ERROR_ENTITLEMENT_UNKNOWN due to invalid downstream type) via listener. @@ -407,7 +398,13 @@ public class TetheringManagerTest { final CarrierConfigManager configManager = (CarrierConfigManager) mContext .getSystemService(Context.CARRIER_CONFIG_SERVICE); final int subId = SubscriptionManager.getDefaultSubscriptionId(); - configManager.overrideConfig(subId, bundle); + runAsShell(MODIFY_PHONE_STATE, () -> configManager.overrideConfig(subId, bundle)); + } + + private boolean isTetheringApnRequired() { + final TelephonyManager tm = mContext.getSystemService(TelephonyManager.class); + return runAsShell(MODIFY_PHONE_STATE, () -> tm.isTetheringApnRequired()); + } @Test @@ -447,10 +444,8 @@ public class TetheringManagerTest { mCtsTetheringUtils.startWifiTethering(tetherEventCallback); - final TelephonyManager telephonyManager = (TelephonyManager) mContext.getSystemService( - Context.TELEPHONY_SERVICE); - final boolean dunRequired = telephonyManager.isTetheringApnRequired(); - final int expectedCap = dunRequired ? NET_CAPABILITY_DUN : NET_CAPABILITY_INTERNET; + final int expectedCap = isTetheringApnRequired() + ? NET_CAPABILITY_DUN : NET_CAPABILITY_INTERNET; final Network network = tetherEventCallback.getCurrentValidUpstream(); final NetworkCapabilities netCap = mCm.getNetworkCapabilities(network); assertTrue(netCap.hasTransport(TRANSPORT_CELLULAR));