Remove NetworkMonitor dependency on ICaptivePortal
ICaptivePortal is used in the framework and cannot be used as a
dependency in NetworkMonitor, as the framework class takes precedence
when linking.
Also fix NetworkMonitorTest that was not verifying the right
startCaptivePortalApp call.
Test: atest FrameworksNetTests NetworkStackTests
Bug: 124033493
Change-Id: I8e7bb79e50650ae182a2e4277fb49abf5fb6d910
Merged-In: Ib6a89e54312628662b130fbeec18d11e139f09fa
diff --git a/Android.bp b/Android.bp
index 4ba757a..9df0372 100644
--- a/Android.bp
+++ b/Android.bp
@@ -186,6 +186,7 @@
"core/java/android/hardware/radio/ITunerCallback.aidl",
"core/java/android/hardware/soundtrigger/IRecognitionStatusCallback.aidl",
"core/java/android/hardware/usb/IUsbManager.aidl",
+ "core/java/android/net/ICaptivePortal.aidl",
"core/java/android/net/IConnectivityManager.aidl",
"core/java/android/net/IIpConnectivityMetrics.aidl",
"core/java/android/net/IEthernetManager.aidl",
@@ -815,7 +816,6 @@
srcs: [
"core/java/android/net/ApfCapabilitiesParcelable.aidl",
"core/java/android/net/DhcpResultsParcelable.aidl",
- "core/java/android/net/ICaptivePortal.aidl",
"core/java/android/net/INetworkMonitor.aidl",
"core/java/android/net/INetworkMonitorCallbacks.aidl",
"core/java/android/net/IIpMemoryStore.aidl",
diff --git a/api/system-current.txt b/api/system-current.txt
index 33cc416..8efc3de 100644
--- a/api/system-current.txt
+++ b/api/system-current.txt
@@ -3101,7 +3101,7 @@
method @RequiresPermission(android.Manifest.permission.TETHER_PRIVILEGED) public void getLatestTetheringEntitlementValue(int, boolean, @NonNull android.net.ConnectivityManager.TetheringEntitlementValueListener, @Nullable android.os.Handler);
method @RequiresPermission(anyOf={android.Manifest.permission.TETHER_PRIVILEGED, android.Manifest.permission.WRITE_SETTINGS}) public boolean isTetheringSupported();
method @RequiresPermission(anyOf={"android.permission.NETWORK_SETTINGS", android.Manifest.permission.NETWORK_SETUP_WIZARD, "android.permission.NETWORK_STACK"}) public void setAirplaneMode(boolean);
- method @RequiresPermission(android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK) public void startCaptivePortalApp(android.os.Bundle);
+ method @RequiresPermission(android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK) public void startCaptivePortalApp(android.net.Network, android.os.Bundle);
method @RequiresPermission(android.Manifest.permission.TETHER_PRIVILEGED) public void startTethering(int, boolean, android.net.ConnectivityManager.OnStartTetheringCallback);
method @RequiresPermission(android.Manifest.permission.TETHER_PRIVILEGED) public void startTethering(int, boolean, android.net.ConnectivityManager.OnStartTetheringCallback, android.os.Handler);
method @RequiresPermission(android.Manifest.permission.TETHER_PRIVILEGED) public void stopTethering(int);
diff --git a/api/test-current.txt b/api/test-current.txt
index 0112724..aceddea 100644
--- a/api/test-current.txt
+++ b/api/test-current.txt
@@ -608,7 +608,7 @@
}
public class ConnectivityManager {
- method @RequiresPermission(android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK) public void startCaptivePortalApp(android.os.Bundle);
+ method @RequiresPermission(android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK) public void startCaptivePortalApp(android.net.Network, android.os.Bundle);
field public static final String EXTRA_CAPTIVE_PORTAL_PROBE_SPEC = "android.net.extra.CAPTIVE_PORTAL_PROBE_SPEC";
field public static final String EXTRA_CAPTIVE_PORTAL_USER_AGENT = "android.net.extra.CAPTIVE_PORTAL_USER_AGENT";
}
diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java
index 68ac46c..99db040 100644
--- a/core/java/android/net/ConnectivityManager.java
+++ b/core/java/android/net/ConnectivityManager.java
@@ -3911,15 +3911,16 @@
*
* <p>This endpoint is exclusively for use by the NetworkStack and is protected by the
* corresponding permission.
+ * @param network Network on which the captive portal was detected.
* @param appExtras Extras to include in the app start intent.
* @hide
*/
@SystemApi
@TestApi
@RequiresPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK)
- public void startCaptivePortalApp(Bundle appExtras) {
+ public void startCaptivePortalApp(Network network, Bundle appExtras) {
try {
- mService.startCaptivePortalAppInternal(appExtras);
+ mService.startCaptivePortalAppInternal(network, appExtras);
} catch (RemoteException e) {
throw e.rethrowFromSystemServer();
}
diff --git a/core/java/android/net/IConnectivityManager.aidl b/core/java/android/net/IConnectivityManager.aidl
index 92a5839..83bb3a0 100644
--- a/core/java/android/net/IConnectivityManager.aidl
+++ b/core/java/android/net/IConnectivityManager.aidl
@@ -168,7 +168,7 @@
void setAcceptUnvalidated(in Network network, boolean accept, boolean always);
void setAvoidUnvalidated(in Network network);
void startCaptivePortalApp(in Network network);
- void startCaptivePortalAppInternal(in Bundle appExtras);
+ void startCaptivePortalAppInternal(in Network network, in Bundle appExtras);
boolean getAvoidBadWifi();
int getMultipathPreference(in Network Network);
diff --git a/core/java/android/net/INetworkMonitor.aidl b/core/java/android/net/INetworkMonitor.aidl
index 41f969a..c94cdde 100644
--- a/core/java/android/net/INetworkMonitor.aidl
+++ b/core/java/android/net/INetworkMonitor.aidl
@@ -34,6 +34,7 @@
void start();
void launchCaptivePortalApp();
+ void notifyCaptivePortalAppFinished(int response);
void forceReevaluation(int uid);
void notifyPrivateDnsChanged(in PrivateDnsConfigParcel config);
void notifyDnsResponse(int returnCode);
diff --git a/core/java/android/net/INetworkMonitorCallbacks.aidl b/core/java/android/net/INetworkMonitorCallbacks.aidl
index 5146585..2c61511 100644
--- a/core/java/android/net/INetworkMonitorCallbacks.aidl
+++ b/core/java/android/net/INetworkMonitorCallbacks.aidl
@@ -26,5 +26,4 @@
void notifyPrivateDnsConfigResolved(in PrivateDnsConfigParcel config);
void showProvisioningNotification(String action, String packageName);
void hideProvisioningNotification();
- void logCaptivePortalLoginEvent(int eventId, String packageName);
}
\ No newline at end of file
diff --git a/packages/NetworkStack/src/com/android/server/NetworkStackService.java b/packages/NetworkStack/src/com/android/server/NetworkStackService.java
index c6a207f..90db207 100644
--- a/packages/NetworkStack/src/com/android/server/NetworkStackService.java
+++ b/packages/NetworkStack/src/com/android/server/NetworkStackService.java
@@ -247,6 +247,12 @@
}
@Override
+ public void notifyCaptivePortalAppFinished(int response) {
+ checkNetworkStackCallingPermission();
+ mNm.notifyCaptivePortalAppFinished(response);
+ }
+
+ @Override
public void forceReevaluation(int uid) {
checkNetworkStackCallingPermission();
mNm.forceReevaluation(uid);
diff --git a/packages/NetworkStack/src/com/android/server/connectivity/NetworkMonitor.java b/packages/NetworkStack/src/com/android/server/connectivity/NetworkMonitor.java
index 2e72d82..0c159ea 100644
--- a/packages/NetworkStack/src/com/android/server/connectivity/NetworkMonitor.java
+++ b/packages/NetworkStack/src/com/android/server/connectivity/NetworkMonitor.java
@@ -39,9 +39,7 @@
import android.content.Context;
import android.content.Intent;
import android.content.IntentFilter;
-import android.net.CaptivePortal;
import android.net.ConnectivityManager;
-import android.net.ICaptivePortal;
import android.net.INetworkMonitor;
import android.net.INetworkMonitorCallbacks;
import android.net.LinkProperties;
@@ -466,6 +464,13 @@
sendMessage(CMD_LAUNCH_CAPTIVE_PORTAL_APP);
}
+ /**
+ * Notify that the captive portal app was closed with the provided response code.
+ */
+ public void notifyCaptivePortalAppFinished(int response) {
+ sendMessage(CMD_CAPTIVE_PORTAL_APP_FINISHED, response);
+ }
+
@Override
protected void log(String s) {
if (DBG) Log.d(TAG + "/" + mNetwork.toString(), s);
@@ -677,29 +682,8 @@
case CMD_LAUNCH_CAPTIVE_PORTAL_APP:
final Bundle appExtras = new Bundle();
// OneAddressPerFamilyNetwork is not parcelable across processes.
- appExtras.putParcelable(
- ConnectivityManager.EXTRA_NETWORK, new Network(mNetwork));
- appExtras.putParcelable(ConnectivityManager.EXTRA_CAPTIVE_PORTAL,
- new CaptivePortal(new ICaptivePortal.Stub() {
- @Override
- public void appResponse(int response) {
- if (response == APP_RETURN_WANTED_AS_IS) {
- mContext.enforceCallingPermission(
- PERMISSION_NETWORK_SETTINGS,
- "CaptivePortal");
- }
- sendMessage(CMD_CAPTIVE_PORTAL_APP_FINISHED, response);
- }
-
- @Override
- public void logEvent(int eventId, String packageName)
- throws RemoteException {
- mContext.enforceCallingPermission(
- PERMISSION_NETWORK_SETTINGS,
- "CaptivePortal");
- mCallback.logCaptivePortalLoginEvent(eventId, packageName);
- }
- }));
+ final Network network = new Network(mNetwork);
+ appExtras.putParcelable(ConnectivityManager.EXTRA_NETWORK, network);
final CaptivePortalProbeResult probeRes = mLastPortalProbeResult;
appExtras.putString(EXTRA_CAPTIVE_PORTAL_URL, probeRes.detectUrl);
if (probeRes.probeSpec != null) {
@@ -708,7 +692,7 @@
}
appExtras.putString(ConnectivityManager.EXTRA_CAPTIVE_PORTAL_USER_AGENT,
mCaptivePortalUserAgent);
- mCm.startCaptivePortalApp(appExtras);
+ mCm.startCaptivePortalApp(network, appExtras);
return HANDLED;
default:
return NOT_HANDLED;
diff --git a/packages/NetworkStack/tests/src/com/android/server/connectivity/NetworkMonitorTest.java b/packages/NetworkStack/tests/src/com/android/server/connectivity/NetworkMonitorTest.java
index b98b0f7..9a16bb7 100644
--- a/packages/NetworkStack/tests/src/com/android/server/connectivity/NetworkMonitorTest.java
+++ b/packages/NetworkStack/tests/src/com/android/server/connectivity/NetworkMonitorTest.java
@@ -16,8 +16,7 @@
package com.android.server.connectivity;
-import static android.net.ConnectivityManager.ACTION_CAPTIVE_PORTAL_SIGN_IN;
-import static android.net.ConnectivityManager.EXTRA_CAPTIVE_PORTAL;
+import static android.net.CaptivePortal.APP_RETURN_DISMISSED;
import static android.net.INetworkMonitor.NETWORK_TEST_RESULT_INVALID;
import static android.net.INetworkMonitor.NETWORK_TEST_RESULT_VALID;
import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET;
@@ -41,8 +40,6 @@
import static org.mockito.Mockito.when;
import android.content.Context;
-import android.content.Intent;
-import android.net.CaptivePortal;
import android.net.ConnectivityManager;
import android.net.INetworkMonitorCallbacks;
import android.net.InetAddresses;
@@ -54,10 +51,10 @@
import android.net.metrics.IpConnectivityLog;
import android.net.util.SharedLog;
import android.net.wifi.WifiManager;
+import android.os.Bundle;
import android.os.ConditionVariable;
import android.os.Handler;
import android.os.SystemClock;
-import android.os.UserHandle;
import android.provider.Settings;
import android.support.test.filters.SmallTest;
import android.support.test.runner.AndroidJUnit4;
@@ -487,19 +484,23 @@
// Check that startCaptivePortalApp sends the expected intent.
nm.launchCaptivePortalApp();
- final ArgumentCaptor<Intent> intentCaptor = ArgumentCaptor.forClass(Intent.class);
- verify(mContext, timeout(HANDLER_TIMEOUT_MS).times(1))
- .startActivityAsUser(intentCaptor.capture(), eq(UserHandle.CURRENT));
- final Intent intent = intentCaptor.getValue();
- assertEquals(ACTION_CAPTIVE_PORTAL_SIGN_IN, intent.getAction());
- final Network network = intent.getParcelableExtra(ConnectivityManager.EXTRA_NETWORK);
- assertEquals(TEST_NETID, network.netId);
+ final ArgumentCaptor<Bundle> bundleCaptor = ArgumentCaptor.forClass(Bundle.class);
+ final ArgumentCaptor<Network> networkCaptor = ArgumentCaptor.forClass(Network.class);
+ verify(mCm, timeout(HANDLER_TIMEOUT_MS).times(1))
+ .startCaptivePortalApp(networkCaptor.capture(), bundleCaptor.capture());
+ final Bundle bundle = bundleCaptor.getValue();
+ final Network bundleNetwork = bundle.getParcelable(ConnectivityManager.EXTRA_NETWORK);
+ assertEquals(TEST_NETID, bundleNetwork.netId);
+ // network is passed both in bundle and as parameter, as the bundle is opaque to the
+ // framework and only intended for the captive portal app, but the framework needs
+ // the network to identify the right NetworkMonitor.
+ assertEquals(TEST_NETID, networkCaptor.getValue().netId);
// Have the app report that the captive portal is dismissed, and check that we revalidate.
setStatus(mHttpsConnection, 204);
setStatus(mHttpConnection, 204);
- final CaptivePortal captivePortal = intent.getParcelableExtra(EXTRA_CAPTIVE_PORTAL);
- captivePortal.reportCaptivePortalDismissed();
+
+ nm.notifyCaptivePortalAppFinished(APP_RETURN_DISMISSED);
verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1))
.notifyNetworkTested(NETWORK_TEST_RESULT_VALID, null);
}
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index af2bbfb..2cd25e4 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -57,8 +57,10 @@
import android.content.IntentFilter;
import android.content.res.Configuration;
import android.database.ContentObserver;
+import android.net.CaptivePortal;
import android.net.ConnectionInfo;
import android.net.ConnectivityManager;
+import android.net.ICaptivePortal;
import android.net.IConnectivityManager;
import android.net.IIpConnectivityMetrics;
import android.net.INetd;
@@ -2689,11 +2691,6 @@
EVENT_PROVISIONING_NOTIFICATION, PROVISIONING_NOTIFICATION_HIDE,
mNai.network.netId));
}
-
- @Override
- public void logCaptivePortalLoginEvent(int eventId, String packageName) {
- new MetricsLogger().action(eventId, packageName);
- }
}
private boolean networkRequiresValidation(NetworkAgentInfo nai) {
@@ -3237,22 +3234,63 @@
/**
* NetworkStack endpoint to start the captive portal app. The NetworkStack needs to use this
* endpoint as it does not have INTERACT_ACROSS_USERS_FULL itself.
+ * @param network Network on which the captive portal was detected.
* @param appExtras Bundle to use as intent extras for the captive portal application.
* Must be treated as opaque to avoid preventing the captive portal app to
* update its arguments.
*/
@Override
- public void startCaptivePortalAppInternal(Bundle appExtras) {
+ public void startCaptivePortalAppInternal(Network network, Bundle appExtras) {
mContext.checkCallingOrSelfPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK);
final Intent appIntent = new Intent(ConnectivityManager.ACTION_CAPTIVE_PORTAL_SIGN_IN);
appIntent.putExtras(appExtras);
+ appIntent.putExtra(ConnectivityManager.EXTRA_CAPTIVE_PORTAL,
+ new CaptivePortal(new CaptivePortalImpl(network).asBinder()));
appIntent.setFlags(Intent.FLAG_ACTIVITY_BROUGHT_TO_FRONT | Intent.FLAG_ACTIVITY_NEW_TASK);
Binder.withCleanCallingIdentity(() ->
mContext.startActivityAsUser(appIntent, UserHandle.CURRENT));
}
+ private class CaptivePortalImpl extends ICaptivePortal.Stub {
+ private final Network mNetwork;
+
+ private CaptivePortalImpl(Network network) {
+ mNetwork = network;
+ }
+
+ @Override
+ public void appResponse(final int response) throws RemoteException {
+ if (response == CaptivePortal.APP_RETURN_WANTED_AS_IS) {
+ enforceSettingsPermission();
+ }
+
+ // getNetworkAgentInfoForNetwork is thread-safe
+ final NetworkAgentInfo nai = getNetworkAgentInfoForNetwork(mNetwork);
+ if (nai == null) return;
+
+ // nai.networkMonitor() is thread-safe
+ final INetworkMonitor nm = nai.networkMonitor();
+ if (nm == null) return;
+
+ final long token = Binder.clearCallingIdentity();
+ try {
+ nm.notifyCaptivePortalAppFinished(response);
+ } finally {
+ // Not using Binder.withCleanCallingIdentity() to keep the checked RemoteException
+ Binder.restoreCallingIdentity(token);
+ }
+ }
+
+ @Override
+ public void logEvent(int eventId, String packageName) {
+ enforceSettingsPermission();
+
+ new MetricsLogger().action(eventId, packageName);
+ }
+ }
+
public boolean avoidBadWifi() {
return mMultinetworkPolicyTracker.getAvoidBadWifi();
}