Merge "WifiNetworkFactory: Trigger connection immediately if pre-approved" into rvc-dev am: e7b399dcd5

Change-Id: I692bc5c8ac6d551c33e0a4b00f6c6203c1224e7f

2 files changed, 78 insertions, 202 deletions

diff --git a/service/java/com/android/server/wifi/WifiNetworkFactory.java b/service/java/com/android/server/wifi/WifiNetworkFactory.java
index b47afc851..98ba5bc94 100644
--- a/service/java/com/android/server/wifi/WifiNetworkFactory.java
+++ b/service/java/com/android/server/wifi/WifiNetworkFactory.java
@@ -58,7 +58,6 @@ import android.util.Pair;
 
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.server.wifi.proto.nano.WifiMetricsProto;
-import com.android.server.wifi.util.ArrayUtils;
 import com.android.server.wifi.util.ExternalCallbackTracker;
 import com.android.server.wifi.util.ScanResultUtil;
 import com.android.server.wifi.util.WifiPermissionsUtil;
@@ -236,12 +235,9 @@ public class WifiNetworkFactory extends NetworkFactory {
             if (mVerboseLoggingEnabled) {
                 Log.v(TAG, "Received " + scanResults.length + " scan results");
             }
-            if (!handleScanResultsAndTriggerConnectIfUserApprovedMatchFound(scanResults)) {
-                // Didn't find an approved match, send the matching results to UI and schedule the
-                // next scan.
-                sendNetworkRequestMatchCallbacksForActiveRequest(mActiveMatchedScanResults);
-                scheduleNextPeriodicScan();
-            }
+            handleScanResults(scanResults);
+            sendNetworkRequestMatchCallbacksForActiveRequest(mActiveMatchedScanResults);
+            scheduleNextPeriodicScan();
         }
 
         @Override
@@ -601,16 +597,17 @@ public class WifiNetworkFactory extends NetworkFactory {
                     wns.ssidPatternMatcher, wns.bssidPatternMatcher, wns.wifiConfiguration);
             mWifiMetrics.incrementNetworkRequestApiNumRequest();
 
-            // Fetch the latest cached scan results to speed up network matching.
-            ScanResult[] cachedScanResults = getFilteredCachedScanResults();
-            if (mVerboseLoggingEnabled) {
-                Log.v(TAG, "Using cached " + cachedScanResults.length + " scan results");
-            }
-            if (!handleScanResultsAndTriggerConnectIfUserApprovedMatchFound(cachedScanResults)) {
+            if (!triggerConnectIfUserApprovedMatchFound()) {
                 // Start UI to let the user grant/disallow this request from the app.
                 startUi();
                 // Didn't find an approved match, send the matching results to UI and trigger
                 // periodic scans for finding a network in the request.
+                // Fetch the latest cached scan results to speed up network matching.
+                ScanResult[] cachedScanResults = getFilteredCachedScanResults();
+                if (mVerboseLoggingEnabled) {
+                    Log.v(TAG, "Using cached " + cachedScanResults.length + " scan results");
+                }
+                handleScanResults(cachedScanResults);
                 sendNetworkRequestMatchCallbacksForActiveRequest(mActiveMatchedScanResults);
                 startPeriodicScans();
             }
@@ -790,9 +787,15 @@ public class WifiNetworkFactory extends NetworkFactory {
                 new WifiConfiguration(mActiveSpecificNetworkRequestSpecifier.wifiConfiguration);
         networkToConnect.SSID = network.SSID;
         // Set the WifiConfiguration.BSSID field to prevent roaming.
-        networkToConnect.BSSID =
-                findBestBssidFromActiveMatchedScanResultsForNetwork(
-                        ScanResultMatchInfo.fromWifiConfiguration(networkToConnect));
+        if (network.BSSID != null) {
+            // If pre-approved, use the bssid from the request.
+            networkToConnect.BSSID = network.BSSID;
+        } else {
+            // If not pre-approved, find the best bssid matching the request.
+            networkToConnect.BSSID =
+                    findBestBssidFromActiveMatchedScanResultsForNetwork(
+                            ScanResultMatchInfo.fromWifiConfiguration(networkToConnect));
+        }
         networkToConnect.ephemeral = true;
         // Mark it user private to avoid conflicting with any saved networks the user might have.
         // TODO (b/142035508): Use a more generic mechanism to fix this.
@@ -1269,68 +1272,63 @@ public class WifiNetworkFactory extends NetworkFactory {
     }
 
     private boolean isAccessPointApprovedInInternalApprovalList(
-            @NonNull ScanResult scanResult, @NonNull String requestorPackageName) {
+            @NonNull String ssid, @NonNull MacAddress bssid, @SecurityType int networkType,
+            @NonNull String requestorPackageName) {
         Set<AccessPoint> approvedAccessPoints =
                 mUserApprovedAccessPointMap.get(requestorPackageName);
         if (approvedAccessPoints == null) return false;
-        ScanResultMatchInfo fromScanResult = ScanResultMatchInfo.fromScanResult(scanResult);
         AccessPoint accessPoint =
-                new AccessPoint(scanResult.SSID,
-                        MacAddress.fromString(scanResult.BSSID), fromScanResult.networkType);
+                new AccessPoint(ssid, bssid, networkType);
         if (!approvedAccessPoints.contains(accessPoint)) return false;
         // keep the most recently used AP in the end
         approvedAccessPoints.remove(accessPoint);
         approvedAccessPoints.add(accessPoint);
         if (mVerboseLoggingEnabled) {
-            Log.v(TAG, "Found " + scanResult
+            Log.v(TAG, "Found " + bssid
                     + " in internal user approved access point for " + requestorPackageName);
         }
         return true;
     }
 
     private boolean isAccessPointApprovedInCompanionDeviceManager(
-            @NonNull ScanResult scanResult, @NonNull UserHandle requestorUserHandle,
+            @NonNull MacAddress bssid,
+            @NonNull UserHandle requestorUserHandle,
             @NonNull String requestorPackageName) {
         if (mCompanionDeviceManager == null) {
             mCompanionDeviceManager = mContext.getSystemService(CompanionDeviceManager.class);
         }
         boolean approved = mCompanionDeviceManager.isDeviceAssociatedForWifiConnection(
-                requestorPackageName, MacAddress.fromString(scanResult.BSSID), requestorUserHandle);
+                requestorPackageName, bssid, requestorUserHandle);
         if (!approved) return false;
         if (mVerboseLoggingEnabled) {
-            Log.v(TAG, "Found " + scanResult
-                    + " in CDM user approved access point for " + requestorPackageName);
+            Log.v(TAG, "Found " + bssid
+                    + " in CompanionDeviceManager approved access point for "
+                    + requestorPackageName);
         }
         return true;
     }
 
-    private @Nullable ScanResult
-            findUserApprovedAccessPointForActiveRequestFromActiveMatchedScanResults() {
-        if (mActiveSpecificNetworkRequestSpecifier == null
-                || ArrayUtils.size(mActiveMatchedScanResults) != 1) {
-            return null;
-        }
-        // There should only 1 matched scan result since the request contains a specific
-        // SSID + BSSID mentioned.
-        ScanResult scanResult = mActiveMatchedScanResults.get(0);
+    private boolean isAccessPointApprovedForActiveRequest(@NonNull String ssid,
+            @NonNull MacAddress bssid, @SecurityType int networkType) {
         String requestorPackageName = mActiveSpecificNetworkRequest.getRequestorPackageName();
         UserHandle requestorUserHandle =
                 UserHandle.getUserHandleForUid(mActiveSpecificNetworkRequest.getRequestorUid());
-        // Check if access point is approved via CDM first.
+        // Check if access point is approved via CompanionDeviceManager first.
         if (isAccessPointApprovedInCompanionDeviceManager(
-                scanResult, requestorUserHandle, requestorPackageName)) {
-            return scanResult;
+                bssid, requestorUserHandle, requestorPackageName)) {
+            return true;
         }
         // Check if access point is approved in internal approval list next.
-        if (isAccessPointApprovedInInternalApprovalList(scanResult, requestorPackageName)) {
-            return scanResult;
+        if (isAccessPointApprovedInInternalApprovalList(
+                ssid, bssid, networkType, requestorPackageName)) {
+            return true;
         }
         // Shell approved app
         if (TextUtils.equals(mApprovedApp, requestorPackageName)) {
-            return scanResult;
+            return true;
         }
         // no bypass approvals, show UI.
-        return null;
+        return false;
     }
 
 
@@ -1378,17 +1376,44 @@ public class WifiNetworkFactory extends NetworkFactory {
     }
 
     /**
-     * Handle scan results
-     * a) Find all scan results matching the active network request.
-     * b) If the request is for a single bssid, check if the matching ScanResult was pre-approved
+     * 1) If the request is for a single bssid, check if the matching ScanResult was pre-approved
      * by the user.
-     * c) If yes to (b), trigger a connect immediately and returns true. Else, returns false.
+     * 2) If yes to (b), trigger a connect immediately and returns true. Else, returns false.
      *
-     * @param scanResults Array of {@link ScanResult} to be processed.
      * @return true if a pre-approved network was found for connection, false otherwise.
      */
-    private boolean handleScanResultsAndTriggerConnectIfUserApprovedMatchFound(
-            ScanResult[] scanResults) {
+    private boolean triggerConnectIfUserApprovedMatchFound() {
+        if (mActiveSpecificNetworkRequestSpecifier == null) return false;
+        if (!isActiveRequestForSingleAccessPoint()) return false;
+        String ssid = mActiveSpecificNetworkRequestSpecifier.ssidPatternMatcher.getPath();
+        MacAddress bssid = mActiveSpecificNetworkRequestSpecifier.bssidPatternMatcher.first;
+        int networkType =
+                ScanResultMatchInfo.fromWifiConfiguration(
+                        mActiveSpecificNetworkRequestSpecifier.wifiConfiguration).networkType;
+        if (!isAccessPointApprovedForActiveRequest(ssid, bssid, networkType)
+                || mWifiConfigManager.isNetworkTemporarilyDisabledByUser(
+                ScanResultUtil.createQuotedSSID(ssid))) {
+            if (mVerboseLoggingEnabled) {
+                Log.v(TAG, "No approved access point found");
+            }
+            return false;
+        }
+        Log.v(TAG, "Approved access point found in matching scan results. "
+                + "Triggering connect " + ssid + "/" + bssid);
+        WifiConfiguration config = mActiveSpecificNetworkRequestSpecifier.wifiConfiguration;
+        config.SSID = "\"" + ssid + "\"";
+        config.BSSID = bssid.toString();
+        handleConnectToNetworkUserSelectionInternal(config);
+        mWifiMetrics.incrementNetworkRequestApiNumUserApprovalBypass();
+        return true;
+    }
+
+    /**
+     * Handle scan results
+     *
+     * @param scanResults Array of {@link ScanResult} to be processed.
+     */
+    private void handleScanResults(ScanResult[] scanResults) {
         List<ScanResult> matchedScanResults =
                 getNetworksMatchingActiveNetworkRequest(scanResults);
         if ((mActiveMatchedScanResults == null || mActiveMatchedScanResults.isEmpty())
@@ -1399,26 +1424,6 @@ public class WifiNetworkFactory extends NetworkFactory {
                     matchedScanResults.size());
         }
         mActiveMatchedScanResults = matchedScanResults;
-
-        ScanResult approvedScanResult = null;
-        if (isActiveRequestForSingleAccessPoint()) {
-            approvedScanResult =
-                    findUserApprovedAccessPointForActiveRequestFromActiveMatchedScanResults();
-        }
-        if (approvedScanResult != null
-                && !mWifiConfigManager.isNetworkTemporarilyDisabledByUser(
-                ScanResultUtil.createQuotedSSID(approvedScanResult.SSID))) {
-            Log.v(TAG, "Approved access point found in matching scan results. "
-                    + "Triggering connect " + approvedScanResult);
-            handleConnectToNetworkUserSelectionInternal(
-                    ScanResultUtil.createNetworkFromScanResult(approvedScanResult));
-            mWifiMetrics.incrementNetworkRequestApiNumUserApprovalBypass();
-            return true;
-        }
-        if (mVerboseLoggingEnabled) {
-            Log.v(TAG, "No approved access points found in matching scan results");
-        }
-        return false;
     }
 
     /**
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiNetworkFactoryTest.java b/tests/wifitests/src/com/android/server/wifi/WifiNetworkFactoryTest.java
index 17159b5e7..25aaeffd6 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiNetworkFactoryTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiNetworkFactoryTest.java
@@ -86,7 +86,6 @@ import org.xmlpull.v1.XmlSerializer;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.nio.charset.StandardCharsets;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -2095,50 +2094,6 @@ public class WifiNetworkFactoryTest extends WifiBaseTest {
     }
 
     /**
-     * Verify the user approval bypass for a specific request for an access point that was already
-     * approved previously with no cached scan results matching.
-     */
-    @Test
-    public void testNetworkSpecifierMatchSuccessUsingLiteralSsidAndBssidMatchApprovedWithNoCache()
-            throws Exception {
-        // 1. First request (no user approval bypass)
-        sendNetworkRequestAndSetupForConnectionStatus();
-
-        mWifiNetworkFactory.removeCallback(TEST_CALLBACK_IDENTIFIER);
-        reset(mNetworkRequestMatchCallback, mWifiScanner, mAlarmManager, mClientModeImpl);
-
-        // 2. Second request for the same access point (user approval bypass).
-        ScanResult matchingScanResult = mTestScanDatas[0].getResults()[0];
-        PatternMatcher ssidPatternMatch =
-                new PatternMatcher(TEST_SSID_1, PatternMatcher.PATTERN_LITERAL);
-        Pair<MacAddress, MacAddress> bssidPatternMatch =
-                Pair.create(MacAddress.fromString(matchingScanResult.BSSID),
-                        MacAddress.BROADCAST_ADDRESS);
-        attachWifiNetworkSpecifierAndAppInfo(
-                ssidPatternMatch, bssidPatternMatch,
-                WifiConfigurationTestUtil.createPskNetwork(), TEST_UID_1, TEST_PACKAGE_NAME_1);
-        mWifiNetworkFactory.needNetworkFor(mNetworkRequest, 0);
-
-        validateUiStartParams(true);
-
-        mWifiNetworkFactory.addCallback(mAppBinder, mNetworkRequestMatchCallback,
-                TEST_CALLBACK_IDENTIFIER);
-        // Trigger scan results & ensure we triggered a connect.
-        verify(mWifiScanner).startScan(any(), any(), mScanListenerArgumentCaptor.capture(), any());
-        ScanListener scanListener = mScanListenerArgumentCaptor.getValue();
-        assertNotNull(scanListener);
-        scanListener.onResults(mTestScanDatas);
-
-        // Verify we did not trigger the match callback.
-        verify(mNetworkRequestMatchCallback, never()).onMatch(anyList());
-        // Verify that we sent a connection attempt to ClientModeImpl
-        verify(mClientModeImpl).connect(eq(null), anyInt(),
-                any(Binder.class), mConnectListenerArgumentCaptor.capture(), anyInt(), anyInt());
-
-        verify(mWifiMetrics).incrementNetworkRequestApiNumUserApprovalBypass();
-    }
-
-    /**
      * Verify that we don't bypass user approval for a specific request for an access point that was
      * approved previously, but then the user forgot it sometime after.
      */
@@ -2401,13 +2356,8 @@ public class WifiNetworkFactoryTest extends WifiBaseTest {
         mWifiNetworkFactory.needNetworkFor(mNetworkRequest, 0);
         mWifiNetworkFactory.addCallback(mAppBinder, mNetworkRequestMatchCallback,
                 TEST_CALLBACK_IDENTIFIER);
-        // Trigger scan results & ensure we triggered a connect.
-        setupScanData(SCAN_RESULT_TYPE_WPA_PSK,
-                TEST_SSID_1, TEST_SSID_2, TEST_SSID_3, TEST_SSID_4);
-        verify(mWifiScanner).startScan(any(), any(), mScanListenerArgumentCaptor.capture(), any());
-        ScanListener scanListener = mScanListenerArgumentCaptor.getValue();
-        assertNotNull(scanListener);
-        scanListener.onResults(mTestScanDatas);
+        // Ensure we triggered a connect without issuing any scans.
+        verify(mWifiScanner, never()).startScan(any(), any(), any(), any());
 
         // Verify we did not trigger the match callback.
         verify(mNetworkRequestMatchCallback, never()).onMatch(anyList());
@@ -2450,7 +2400,7 @@ public class WifiNetworkFactoryTest extends WifiBaseTest {
      * approved previously and the scan result is present in the cached scan results.
      */
     @Test
-    public void testNetworkSpecifierMatchSuccessUsingLiteralSsidAndBssidMatchApprovedWithCache()
+    public void testNetworkSpecifierMatchSuccessUsingLiteralSsidAndBssidMatchApproved()
             throws Exception {
         // 1. First request (no user approval bypass)
         sendNetworkRequestAndSetupForConnectionStatus();
@@ -2460,11 +2410,6 @@ public class WifiNetworkFactoryTest extends WifiBaseTest {
 
         // 2. Second request for the same access point (user approval bypass).
         ScanResult matchingScanResult = mTestScanDatas[0].getResults()[0];
-        // simulate no cache expiry
-        when(mClock.getElapsedSinceBootMillis()).thenReturn(0L);
-        // Simulate the cached results matching.
-        when(mWifiScanner.getSingleScanResults())
-                .thenReturn(Arrays.asList(mTestScanDatas[0].getResults()));
 
         PatternMatcher ssidPatternMatch =
                 new PatternMatcher(TEST_SSID_1, PatternMatcher.PATTERN_LITERAL);
@@ -2495,7 +2440,7 @@ public class WifiNetworkFactoryTest extends WifiBaseTest {
      */
     @Test
     public void
-            testNetworkSpecifierMatchSuccessUsingLiteralSsidAndBssidMatchApprovedViaCDMWithCache()
+            testNetworkSpecifierMatchSuccessUsingLiteralSsidAndBssidMatchApprovedViaCDM()
             throws Exception {
         // Setup scan data for WPA-PSK networks.
         setupScanData(SCAN_RESULT_TYPE_WPA_PSK,
@@ -2510,12 +2455,6 @@ public class WifiNetworkFactoryTest extends WifiBaseTest {
                 MacAddress.fromString(matchingScanResult.BSSID),
                 UserHandle.getUserHandleForUid(TEST_UID_1))).thenReturn(true);
 
-        // simulate no cache expiry
-        when(mClock.getElapsedSinceBootMillis()).thenReturn(0L);
-        // Simulate the cached results matching.
-        when(mWifiScanner.getSingleScanResults())
-                .thenReturn(Arrays.asList(mTestScanDatas[0].getResults()));
-
         PatternMatcher ssidPatternMatch =
                 new PatternMatcher(TEST_SSID_1, PatternMatcher.PATTERN_LITERAL);
         Pair<MacAddress, MacAddress> bssidPatternMatch =
@@ -2546,7 +2485,7 @@ public class WifiNetworkFactoryTest extends WifiBaseTest {
      */
     @Test
     public void
-            testNetworkSpecifierMatchSuccessUsingLiteralSsidAndBssidMatchApprovedViaShellWithCache()
+            testNetworkSpecifierMatchSuccessUsingLiteralSsidAndBssidMatchApprovedViaShell()
             throws Exception {
         // Setup scan data for WPA-PSK networks.
         setupScanData(SCAN_RESULT_TYPE_WPA_PSK,
@@ -2558,12 +2497,6 @@ public class WifiNetworkFactoryTest extends WifiBaseTest {
         // Setup shell approval for the scan result.
         mWifiNetworkFactory.setUserApprovedApp(TEST_PACKAGE_NAME_1, true);
 
-        // simulate no cache expiry
-        when(mClock.getElapsedSinceBootMillis()).thenReturn(0L);
-        // Simulate the cached results matching.
-        when(mWifiScanner.getSingleScanResults())
-                .thenReturn(Arrays.asList(mTestScanDatas[0].getResults()));
-
         PatternMatcher ssidPatternMatch =
                 new PatternMatcher(TEST_SSID_1, PatternMatcher.PATTERN_LITERAL);
         Pair<MacAddress, MacAddress> bssidPatternMatch =
@@ -2588,68 +2521,6 @@ public class WifiNetworkFactoryTest extends WifiBaseTest {
     }
 
     /**
-     * Verify the user approval bypass for a specific request for an access point that was already
-     * approved previously and the scan result is present in the cached scan results, but the
-     * results are stale.
-     */
-    @Test
-    public void
-            testNetworkSpecifierMatchSuccessUsingLiteralSsidAndBssidMatchApprovedWithStaleCache()
-            throws Exception {
-        // 1. First request (no user approval bypass)
-        sendNetworkRequestAndSetupForConnectionStatus();
-
-        mWifiNetworkFactory.removeCallback(TEST_CALLBACK_IDENTIFIER);
-        reset(mNetworkRequestMatchCallback, mWifiScanner, mAlarmManager, mClientModeImpl);
-
-        long scanResultsTimestampInUs = 39484839202L;
-        mTestScanDatas[0].getResults()[0].timestamp = scanResultsTimestampInUs;
-        mTestScanDatas[0].getResults()[1].timestamp = scanResultsTimestampInUs;
-        mTestScanDatas[0].getResults()[2].timestamp = scanResultsTimestampInUs;
-        mTestScanDatas[0].getResults()[3].timestamp = scanResultsTimestampInUs;
-
-        // 2. Second request for the same access point (user approval bypass).
-        ScanResult matchingScanResult = mTestScanDatas[0].getResults()[0];
-        // simulate cache expiry
-        when(mClock.getElapsedSinceBootMillis())
-                .thenReturn(Long.valueOf(
-                        scanResultsTimestampInUs / 1000
-                                + WifiNetworkFactory.CACHED_SCAN_RESULTS_MAX_AGE_IN_MILLIS + 1));
-        // Simulate the cached results matching.
-        when(mWifiScanner.getSingleScanResults())
-                .thenReturn(Arrays.asList(mTestScanDatas[0].getResults()));
-
-        PatternMatcher ssidPatternMatch =
-                new PatternMatcher(TEST_SSID_1, PatternMatcher.PATTERN_LITERAL);
-        Pair<MacAddress, MacAddress> bssidPatternMatch =
-                Pair.create(MacAddress.fromString(matchingScanResult.BSSID),
-                        MacAddress.BROADCAST_ADDRESS);
-        attachWifiNetworkSpecifierAndAppInfo(
-                ssidPatternMatch, bssidPatternMatch,
-                WifiConfigurationTestUtil.createPskNetwork(), TEST_UID_1, TEST_PACKAGE_NAME_1);
-        mWifiNetworkFactory.needNetworkFor(mNetworkRequest, 0);
-
-        // Ensure we brought up the UI while the scan is ongoing.
-        validateUiStartParams(true);
-
-        mWifiNetworkFactory.addCallback(mAppBinder, mNetworkRequestMatchCallback,
-                TEST_CALLBACK_IDENTIFIER);
-        // Trigger scan results & ensure we triggered a connect.
-        verify(mWifiScanner).startScan(any(), any(), mScanListenerArgumentCaptor.capture(), any());
-        ScanListener scanListener = mScanListenerArgumentCaptor.getValue();
-        assertNotNull(scanListener);
-        scanListener.onResults(mTestScanDatas);
-
-        // Verify we did not trigger the match callback.
-        verify(mNetworkRequestMatchCallback, never()).onMatch(anyList());
-        // Verify that we sent a connection attempt to ClientModeImpl
-        verify(mClientModeImpl).connect(eq(null), anyInt(),
-                any(Binder.class), mConnectListenerArgumentCaptor.capture(), anyInt(), anyInt());
-
-        verify(mWifiMetrics).incrementNetworkRequestApiNumUserApprovalBypass();
-    }
-
-    /**
      * Verify network specifier matching for a specifier containing a specific SSID match using
      * 4 WPA_PSK scan results, each with unique SSID when the UI callback registration is delayed.
      */