From f2cb190a737f97eaa43262adb68ccac8ad95f936 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Fri, 13 Jun 2025 16:03:39 +0900 Subject: [PATCH] Implement opt-in to custom tabs in captive portals. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will allow portal operators to opt-in to custom tabs in captive portal, either across the board with "true" or with a version of the module above which it wants to opt-in. Ideally there could be a separate key to also list opted-in browsers and their versions, but this would become a lot more complex. To start with, there is no place in CaptivePortalData to store this kind of info, and that would mean adding API which would have to be supported forever. The constant inside the source represents a capport source that contains the custom tabs opt-in. This is not a great place to put this information in, but it has the characteristics of a benign and useful hack : - Time to market is 0 - It's easy to remove once we have a better API - It's not a large problem if it's never cleaned up Also, allowing to choose the browsers and the versions would be uncomfortable since the choice of the browser is the user's choice. On the one hand it does make some sense that the operator could test with a given browser, see the UX is not well adapted, and reject it, but on the other hand giving the operator the choice of what browser the user can choose to use on their portal is disputable, as this should generally be under the control of the user. In any case even if we decided to add a per-browser version check it should be done in some other change. Test: manual Flag: EXEMPT opt-in feature Change-Id: I540f4fcb6fb26f7f18077d9c8d06e5f7df206056 --- .../src/android/net/CaptivePortalData.java | 15 +++- .../java/android/net/CaptivePortalDataTest.kt | 82 ++++++++++++++----- .../ConnectivityServiceIntegrationTest.kt | 61 +++++++++++++- 3 files changed, 132 insertions(+), 26 deletions(-) diff --git a/framework/src/android/net/CaptivePortalData.java b/framework/src/android/net/CaptivePortalData.java index 53aa1b92ed..06cefb588b 100644 --- a/framework/src/android/net/CaptivePortalData.java +++ b/framework/src/android/net/CaptivePortalData.java @@ -50,7 +50,9 @@ public final class CaptivePortalData implements Parcelable { @Retention(RetentionPolicy.SOURCE) @IntDef(prefix = {"CAPTIVE_PORTAL_DATA_SOURCE_"}, value = { CAPTIVE_PORTAL_DATA_SOURCE_OTHER, - CAPTIVE_PORTAL_DATA_SOURCE_PASSPOINT}) + CAPTIVE_PORTAL_DATA_SOURCE_PASSPOINT, + CAPTIVE_PORTAL_DATA_SOURCE_CAPPORT_WITH_CUSTOM_TABS_OPTIN + }) public @interface CaptivePortalDataSource {} /** @@ -63,6 +65,17 @@ public final class CaptivePortalData implements Parcelable { */ public static final int CAPTIVE_PORTAL_DATA_SOURCE_PASSPOINT = 1; + // TODO : Remove this, replace with an explicit boolean for opting in. Better yet, when the + // feature is publicly rolled out, remove without a replacement. Because the network stack and + // connectivity modules are not yet merged, adding anything in this class would require + // public API, which would have to be maintained forever and would force very long time-to- + // market timelines, so this is used instead. + /** + * Source of information : Capport, and capport opts in to custom tabs. + * @hide + */ + public static final int CAPTIVE_PORTAL_DATA_SOURCE_CAPPORT_WITH_CUSTOM_TABS_OPTIN = 2; + private CaptivePortalData(long refreshTimeMillis, Uri userPortalUrl, Uri venueInfoUrl, boolean isSessionExtendable, long byteLimit, long expiryTimeMillis, boolean captive, CharSequence venueFriendlyName, int venueInfoUrlSource, int userPortalUrlSource) { diff --git a/tests/common/java/android/net/CaptivePortalDataTest.kt b/tests/common/java/android/net/CaptivePortalDataTest.kt index 67a523c14f..92d0f5669f 100644 --- a/tests/common/java/android/net/CaptivePortalDataTest.kt +++ b/tests/common/java/android/net/CaptivePortalDataTest.kt @@ -57,20 +57,35 @@ class CaptivePortalDataTest { .apply { if (SdkLevel.isAtLeastS()) { setVenueFriendlyName("venue friendly name") - setUserPortalUrl(Uri.parse("https://tc.example.com/passpoint"), - CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_PASSPOINT) - setVenueInfoUrl(Uri.parse("https://venue.example.com/passpoint"), - CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_PASSPOINT) + setUserPortalUrl( + Uri.parse("https://tc.example.com/passpoint"), + CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_PASSPOINT + ) + setVenueInfoUrl( + Uri.parse("https://venue.example.com/passpoint"), + CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_PASSPOINT + ) } } .build() + private val optInData = CaptivePortalData.Builder().apply { + setCaptive(true) + if (SdkLevel.isAtLeastS()) { + setUserPortalUrl( + Uri.parse("https://tc.example.passpoint"), + CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_CAPPORT_WITH_CUSTOM_TABS_OPTIN + ) + } + }.build() + private fun makeBuilder() = CaptivePortalData.Builder(data) @Test fun testParcelUnparcel() { assertParcelingIsLossless(data) assertParcelingIsLossless(dataFromPasspoint) + if (SdkLevel.isAtLeastS()) assertParcelingIsLossless(optInData) assertParcelingIsLossless(makeBuilder().setUserPortalUrl(null).build()) assertParcelingIsLossless(makeBuilder().setVenueInfoUrl(null).build()) @@ -96,24 +111,35 @@ class CaptivePortalDataTest { assertEquals(dataFromPasspoint, CaptivePortalData.Builder(dataFromPasspoint).build()) assertNotEqualsAfterChange { it.setUserPortalUrl( - Uri.parse("https://tc.example.com/passpoint")) } + Uri.parse("https://tc.example.com/passpoint") + ) } assertNotEqualsAfterChange { it.setUserPortalUrl( Uri.parse("https://tc.example.com/passpoint"), - CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_OTHER) } + CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_OTHER + ) } assertNotEqualsAfterChange { it.setUserPortalUrl( Uri.parse("https://tc.example.com/other"), - CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_PASSPOINT) } + CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_PASSPOINT + ) } assertNotEqualsAfterChange { it.setUserPortalUrl( - Uri.parse("https://tc.example.com/passpoint"), - CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_OTHER) } + Uri.parse("https://tc.example.com/passpoint"), + CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_OTHER + ) } + assertNotEqualsAfterChange { it.setUserPortalUrl( + Uri.parse("https://tc.example.com/passpoint"), + CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_CAPPORT_WITH_CUSTOM_TABS_OPTIN + ) } assertNotEqualsAfterChange { it.setVenueInfoUrl( - Uri.parse("https://venue.example.com/passpoint")) } + Uri.parse("https://venue.example.com/passpoint") + ) } assertNotEqualsAfterChange { it.setVenueInfoUrl( Uri.parse("https://venue.example.com/other"), - CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_PASSPOINT) } + CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_PASSPOINT + ) } assertNotEqualsAfterChange { it.setVenueInfoUrl( Uri.parse("https://venue.example.com/passpoint"), - CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_OTHER) } + CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_OTHER + ) } } } @@ -164,18 +190,32 @@ class CaptivePortalDataTest { @Test @IgnoreUpTo(Build.VERSION_CODES.R) fun testGetVenueInfoUrlSource() { - assertEquals(CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_OTHER, - data.venueInfoUrlSource) - assertEquals(CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_PASSPOINT, - dataFromPasspoint.venueInfoUrlSource) + assertEquals( + CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_OTHER, + data.venueInfoUrlSource + ) + assertEquals( + CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_PASSPOINT, + dataFromPasspoint.venueInfoUrlSource + ) } @Test @IgnoreUpTo(Build.VERSION_CODES.R) fun testGetUserPortalUrlSource() { - assertEquals(CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_OTHER, - data.userPortalUrlSource) - assertEquals(CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_PASSPOINT, - dataFromPasspoint.userPortalUrlSource) + assertEquals( + CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_OTHER, + data.userPortalUrlSource + ) + assertEquals( + CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_PASSPOINT, + dataFromPasspoint.userPortalUrlSource + ) + if (SdkLevel.isAtLeastS()) { + assertEquals( + CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_CAPPORT_WITH_CUSTOM_TABS_OPTIN, + optInData.userPortalUrlSource + ) + } } private fun CaptivePortalData.mutate(mutator: (CaptivePortalData.Builder) -> Unit) = @@ -184,4 +224,4 @@ class CaptivePortalDataTest { private fun assertNotEqualsAfterChange(mutator: (CaptivePortalData.Builder) -> Unit) { assertNotEquals(data, data.mutate(mutator)) } -} \ No newline at end of file +} diff --git a/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt b/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt index 563ced89e5..d9eb23e71a 100644 --- a/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt +++ b/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt @@ -24,7 +24,10 @@ import android.content.Context.BIND_AUTO_CREATE import android.content.Context.BIND_IMPORTANT import android.content.Intent import android.content.ServiceConnection +import android.content.pm.PackageManager import android.content.res.Resources +import android.net.CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_CAPPORT_WITH_CUSTOM_TABS_OPTIN +import android.net.CaptivePortalData.CAPTIVE_PORTAL_DATA_SOURCE_OTHER import android.net.ConnectivityManager import android.net.IDnsResolver import android.net.INetd @@ -236,7 +239,7 @@ class ConnectivityServiceIntegrationTest { } private inner class TestConnectivityService(deps: Dependencies) : ConnectivityService( - context, + context, dnsResolver, log, netd, @@ -253,7 +256,7 @@ class ConnectivityServiceIntegrationTest { override fun getBpfNetMaps( context: Context?, netd: INetd?, - interfaceTracker: InterfaceTracker? + interfaceTracker: InterfaceTracker? ) = mock(BpfNetMaps::class.java) .also { doReturn(PERMISSION_INTERNET).`when`(it).getNetPermForUid(anyInt()) @@ -284,7 +287,7 @@ class ConnectivityServiceIntegrationTest { tm: TelephonyManager, requestRestrictedWifiEnabled: Boolean, listener: BiConsumer, - handler: Handler + handler: Handler, ): CarrierPrivilegeAuthenticator { return CarrierPrivilegeAuthenticator( context, @@ -363,8 +366,44 @@ class ConnectivityServiceIntegrationTest { } } + private fun getModuleVersion(): Long { + val captivePortalActivity = Intent(ConnectivityManager.ACTION_CAPTIVE_PORTAL_SIGN_IN) + .resolveActivity(context.packageManager) + val captivePortalInfo = context.packageManager.getPackageInfo( + captivePortalActivity.packageName, + PackageManager.GET_ACTIVITIES + ) + return captivePortalInfo.longVersionCode + } + + val OPT_IN = true + var OPT_OUT = false + @Test - fun testCapportApi() { + fun testCapportApiWithoutOptInToCustomTabs() = testCapportApi( + optInToCustomTabsString = null, + expectedOptIn = OPT_OUT + ) + + @Test + fun testCapportApiWithFullOptInToCustomTabs() = testCapportApi( + optInToCustomTabsString = "true", + expectedOptIn = OPT_IN + ) + + @Test + fun testCapportApiWithHighVersionOptInToCustomTabs() = testCapportApi( + optInToCustomTabsString = (getModuleVersion() + 10).toString(), + expectedOptIn = OPT_OUT + ) + + @Test + fun testCapportApiWithLowVersionOptInToCustomTabs() = testCapportApi( + optInToCustomTabsString = (getModuleVersion() - 10).toString(), + expectedOptIn = OPT_IN + ) + + fun testCapportApi(optInToCustomTabsString: String?, expectedOptIn: Boolean) { val request = NetworkRequest.Builder() .clearCapabilities() .addCapability(NET_CAPABILITY_INTERNET) @@ -373,11 +412,17 @@ class ConnectivityServiceIntegrationTest { val apiUrl = "https://capport.android.com" cm.registerNetworkCallback(request, testCb) + val optInString = if (optInToCustomTabsString != null) { + "\"x-android-use-custom-tabs\": $optInToCustomTabsString," + } else { + "" + } nsInstrumentation.addHttpResponse(HttpResponse( apiUrl, """ |{ | "captive": true, + | $optInString | "user-portal-url": "https://login.capport.android.com", | "venue-info-url": "https://venueinfo.capport.android.com" |} @@ -412,6 +457,14 @@ class ConnectivityServiceIntegrationTest { Uri.parse("https://venueinfo.capport.android.com"), capportData.venueInfoUrl ) + assertEquals( + if (expectedOptIn == OPT_IN) { + CAPTIVE_PORTAL_DATA_SOURCE_CAPPORT_WITH_CUSTOM_TABS_OPTIN + } else { + CAPTIVE_PORTAL_DATA_SOURCE_OTHER + }, + capportData.userPortalUrlSource + ) testCb.expectCaps(na, TEST_TIMEOUT_MS) { it.hasCapability(NET_CAPABILITY_CAPTIVE_PORTAL) &&