diff options
3 files changed, 52 insertions, 15 deletions
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index e81d1721b271..fc67c38560fa 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -2616,9 +2616,11 @@ public class ConnectivityService extends IConnectivityManager.Stub final boolean valid = ((msg.arg1 & NETWORK_VALIDATION_RESULT_VALID) != 0); final boolean wasValidated = nai.lastValidated; final boolean wasDefault = isDefaultNetwork(nai); - if (nai.everCaptivePortalDetected && !nai.captivePortalLoginNotified - && valid) { - nai.captivePortalLoginNotified = true; + // Only show a connected notification if the network is pending validation + // after the captive portal app was open, and it has now validated. + if (nai.captivePortalValidationPending && valid) { + // User is now logged in, network validated. + nai.captivePortalValidationPending = false; showNetworkNotification(nai, NotificationType.LOGGED_IN); } @@ -2689,9 +2691,6 @@ public class ConnectivityService extends IConnectivityManager.Stub final int oldScore = nai.getCurrentScore(); nai.lastCaptivePortalDetected = visible; nai.everCaptivePortalDetected |= visible; - if (visible) { - nai.captivePortalLoginNotified = false; - } if (nai.lastCaptivePortalDetected && Settings.Global.CAPTIVE_PORTAL_MODE_AVOID == getCaptivePortalMode()) { if (DBG) log("Avoiding captive portal network: " + nai.name()); @@ -3500,6 +3499,12 @@ public class ConnectivityService extends IConnectivityManager.Stub new CaptivePortal(new CaptivePortalImpl(network).asBinder())); appIntent.setFlags(Intent.FLAG_ACTIVITY_BROUGHT_TO_FRONT | Intent.FLAG_ACTIVITY_NEW_TASK); + // This runs on a random binder thread, but getNetworkAgentInfoForNetwork is thread-safe, + // and captivePortalValidationPending is volatile. + final NetworkAgentInfo nai = getNetworkAgentInfoForNetwork(network); + if (nai != null) { + nai.captivePortalValidationPending = true; + } Binder.withCleanCallingIdentity(() -> mContext.startActivityAsUser(appIntent, UserHandle.CURRENT)); } diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java index 864a793b8f40..5b043799f848 100644 --- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java +++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java @@ -155,9 +155,9 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> { // Whether a captive portal was found during the last network validation attempt. public boolean lastCaptivePortalDetected; - // Indicates the user was notified of a successful captive portal login since a portal was - // last detected. - public boolean captivePortalLoginNotified; + // Indicates the captive portal app was opened to show a login UI to the user, but the network + // has not validated yet. + public volatile boolean captivePortalValidationPending; // Set to true when partial connectivity was detected. public boolean partialConnectivity; @@ -629,7 +629,7 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> { + "acceptUnvalidated{" + networkMisc.acceptUnvalidated + "} " + "everCaptivePortalDetected{" + everCaptivePortalDetected + "} " + "lastCaptivePortalDetected{" + lastCaptivePortalDetected + "} " - + "captivePortalLoginNotified{" + captivePortalLoginNotified + "} " + + "captivePortalValidationPending{" + captivePortalValidationPending + "} " + "partialConnectivity{" + partialConnectivity + "} " + "acceptPartialConnectivity{" + networkMisc.acceptPartialConnectivity + "} " + "clat{" + clatd + "} " diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 8c024a5a53c7..73ee7f544b5f 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -18,6 +18,7 @@ package com.android.server; import static android.content.pm.PackageManager.GET_PERMISSIONS; import static android.content.pm.PackageManager.MATCH_ANY_USER; +import static android.net.ConnectivityManager.ACTION_CAPTIVE_PORTAL_SIGN_IN; import static android.net.ConnectivityManager.CONNECTIVITY_ACTION; import static android.net.ConnectivityManager.NETID_UNSET; import static android.net.ConnectivityManager.PRIVATE_DNS_MODE_OFF; @@ -80,13 +81,13 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Matchers.anyInt; import static org.mockito.Mockito.any; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; -import static org.mockito.Mockito.eq; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -141,6 +142,7 @@ import android.net.NetworkInfo.DetailedState; import android.net.NetworkMisc; import android.net.NetworkRequest; import android.net.NetworkSpecifier; +import android.net.NetworkStack; import android.net.NetworkStackClient; import android.net.NetworkState; import android.net.NetworkUtils; @@ -154,6 +156,7 @@ import android.net.shared.NetworkMonitorUtils; import android.net.shared.PrivateDnsConfig; import android.net.util.MultinetworkPolicyTracker; import android.os.Binder; +import android.os.Bundle; import android.os.ConditionVariable; import android.os.Handler; import android.os.HandlerThread; @@ -191,6 +194,7 @@ import com.android.server.connectivity.DefaultNetworkMetrics; import com.android.server.connectivity.IpConnectivityMetrics; import com.android.server.connectivity.MockableSystemProperties; import com.android.server.connectivity.Nat464Xlat; +import com.android.server.connectivity.NetworkNotificationManager.NotificationType; import com.android.server.connectivity.ProxyTracker; import com.android.server.connectivity.Tethering; import com.android.server.connectivity.Vpn; @@ -282,6 +286,7 @@ public class ConnectivityServiceTest { @Mock NetworkStackClient mNetworkStack; @Mock PackageManager mPackageManager; @Mock UserManager mUserManager; + @Mock NotificationManager mNotificationManager; private ArgumentCaptor<ResolverParamsParcel> mResolverParamsParcelCaptor = ArgumentCaptor.forClass(ResolverParamsParcel.class); @@ -351,7 +356,7 @@ public class ConnectivityServiceTest { @Override public Object getSystemService(String name) { if (Context.CONNECTIVITY_SERVICE.equals(name)) return mCm; - if (Context.NOTIFICATION_SERVICE.equals(name)) return mock(NotificationManager.class); + if (Context.NOTIFICATION_SERVICE.equals(name)) return mNotificationManager; if (Context.NETWORK_STACK_SERVICE.equals(name)) return mNetworkStack; if (Context.USER_SERVICE.equals(name)) return mUserManager; return super.getSystemService(name); @@ -371,7 +376,17 @@ public class ConnectivityServiceTest { public PackageManager getPackageManager() { return mPackageManager; } - } + + @Override + public void enforceCallingOrSelfPermission(String permission, String message) { + // The mainline permission can only be held if signed with the network stack certificate + // Skip testing for this permission. + if (NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK.equals(permission)) return; + // All other permissions should be held by the test or unnecessary: check as normal to + // make sure the code does not rely on unexpected permissions. + super.enforceCallingOrSelfPermission(permission, message); + } + } public void waitForIdle(int timeoutMsAsInt) { long timeoutMs = timeoutMsAsInt; @@ -2856,6 +2871,9 @@ public class ConnectivityServiceTest { // Expect NET_CAPABILITY_VALIDATED onAvailable callback. validatedCallback.expectAvailableDoubleValidatedCallbacks(mWiFiNetworkAgent); + // Expect no notification to be shown when captive portal disappears by itself + verify(mNotificationManager, never()).notifyAsUser( + anyString(), eq(NotificationType.LOGGED_IN.eventId), any(), any()); // Break network connectivity. // Expect NET_CAPABILITY_VALIDATED onLost callback. @@ -2885,6 +2903,8 @@ public class ConnectivityServiceTest { // Check that calling startCaptivePortalApp does nothing. final int fastTimeoutMs = 100; mCm.startCaptivePortalApp(wifiNetwork); + waitForIdle(); + verify(mWiFiNetworkAgent.mNetworkMonitor, never()).launchCaptivePortalApp(); mServiceContext.expectNoStartActivityIntent(fastTimeoutMs); // Turn into a captive portal. @@ -2895,14 +2915,26 @@ public class ConnectivityServiceTest { // Check that startCaptivePortalApp sends the expected command to NetworkMonitor. mCm.startCaptivePortalApp(wifiNetwork); - verify(mWiFiNetworkAgent.mNetworkMonitor, timeout(TIMEOUT_MS).times(1)) - .launchCaptivePortalApp(); + waitForIdle(); + verify(mWiFiNetworkAgent.mNetworkMonitor).launchCaptivePortalApp(); + + // NetworkMonitor uses startCaptivePortal(Network, Bundle) (startCaptivePortalAppInternal) + final Bundle testBundle = new Bundle(); + final String testKey = "testkey"; + final String testValue = "testvalue"; + testBundle.putString(testKey, testValue); + mCm.startCaptivePortalApp(wifiNetwork, testBundle); + final Intent signInIntent = mServiceContext.expectStartActivityIntent(TIMEOUT_MS); + assertEquals(ACTION_CAPTIVE_PORTAL_SIGN_IN, signInIntent.getAction()); + assertEquals(testValue, signInIntent.getStringExtra(testKey)); // Report that the captive portal is dismissed, and check that callbacks are fired mWiFiNetworkAgent.setNetworkValid(); mWiFiNetworkAgent.mNetworkMonitor.forceReevaluation(Process.myUid()); validatedCallback.expectAvailableCallbacksValidated(mWiFiNetworkAgent); captivePortalCallback.expectCallback(CallbackState.LOST, mWiFiNetworkAgent); + verify(mNotificationManager, times(1)).notifyAsUser(anyString(), + eq(NotificationType.LOGGED_IN.eventId), any(), eq(UserHandle.ALL)); mCm.unregisterNetworkCallback(validatedCallback); mCm.unregisterNetworkCallback(captivePortalCallback); |