WSM: Reconnect on network credential change

Currently, we ignore connect request if we're already connected to the
same network. This is not correct for network modifications. Hence,
trigger a reconnect in this case.

Changes in the CL:
1. Add a new |hasCredentialChanged| param in NetworkUpdateResult.
2. Use that in WSM to determine if we need to trigger a reconnect.

While there,
Removed a couple of unused setters in NetworkUpdateResult.
Fixed WifiConfigManager unit test failure due to mockito change.

Bug: 36505419
Bug: 36020928

Test: Unit tests and manual tests via settings UI.
Test: Will send for regression tests.
Change-Id: I1adaac58108c2f65fb36173a468dff000f5372c8

4 files changed, 62 insertions, 41 deletions

diff --git a/service/java/com/android/server/wifi/NetworkUpdateResult.java b/service/java/com/android/server/wifi/NetworkUpdateResult.java
index 1db7c102e..851b13b11 100644
--- a/service/java/com/android/server/wifi/NetworkUpdateResult.java
+++ b/service/java/com/android/server/wifi/NetworkUpdateResult.java
@@ -22,18 +22,21 @@ public class NetworkUpdateResult {
     int netId;
     boolean ipChanged;
     boolean proxyChanged;
+    boolean credentialChanged;
     boolean isNewNetwork = false;
 
     public NetworkUpdateResult(int id) {
         netId = id;
         ipChanged = false;
         proxyChanged = false;
+        credentialChanged = false;
     }
 
-    public NetworkUpdateResult(boolean ip, boolean proxy) {
+    public NetworkUpdateResult(boolean ip, boolean proxy, boolean credential) {
         netId = INVALID_NETWORK_ID;
         ipChanged = ip;
         proxyChanged = proxy;
+        credentialChanged = credential;
     }
 
     public void setNetworkId(int id) {
@@ -44,22 +47,18 @@ public class NetworkUpdateResult {
         return netId;
     }
 
-    public void setIpChanged(boolean ip) {
-        ipChanged = ip;
-    }
-
     public boolean hasIpChanged() {
         return ipChanged;
     }
 
-    public void setProxyChanged(boolean proxy) {
-        proxyChanged = proxy;
-    }
-
     public boolean hasProxyChanged() {
         return proxyChanged;
     }
 
+    public boolean hasCredentialChanged() {
+        return credentialChanged;
+    }
+
     public boolean isNewNetwork() {
         return isNewNetwork;
     }
diff --git a/service/java/com/android/server/wifi/WifiConfigManager.java b/service/java/com/android/server/wifi/WifiConfigManager.java
index ba3c8d305..63168e995 100644
--- a/service/java/com/android/server/wifi/WifiConfigManager.java
+++ b/service/java/com/android/server/wifi/WifiConfigManager.java
@@ -939,6 +939,13 @@ public class WifiConfigManager {
         }
 
         boolean newNetwork = (existingInternalConfig == null);
+        // This is needed to inform IpManager about any IP configuration changes.
+        boolean hasIpChanged =
+                newNetwork || WifiConfigurationUtil.hasIpChanged(
+                        existingInternalConfig, newInternalConfig);
+        boolean hasProxyChanged =
+                newNetwork || WifiConfigurationUtil.hasProxyChanged(
+                        existingInternalConfig, newInternalConfig);
         // Reset the |hasEverConnected| flag if the credential parameters changed in this update.
         boolean hasCredentialChanged =
                 newNetwork || WifiConfigurationUtil.hasCredentialChanged(
@@ -960,14 +967,8 @@ public class WifiConfigManager {
         // Stage the backup of the SettingsProvider package which backs this up.
         mBackupManagerProxy.notifyDataChanged();
 
-        // This is needed to inform IpManager about any IP configuration changes.
-        boolean hasIpChanged =
-                newNetwork || WifiConfigurationUtil.hasIpChanged(
-                        existingInternalConfig, newInternalConfig);
-        boolean hasProxyChanged =
-                newNetwork || WifiConfigurationUtil.hasProxyChanged(
-                        existingInternalConfig, newInternalConfig);
-        NetworkUpdateResult result = new NetworkUpdateResult(hasIpChanged, hasProxyChanged);
+        NetworkUpdateResult result =
+                new NetworkUpdateResult(hasIpChanged, hasProxyChanged, hasCredentialChanged);
         result.setIsNewNetwork(newNetwork);
         result.setNetworkId(newInternalConfig.networkId);
 
diff --git a/service/java/com/android/server/wifi/WifiStateMachine.java b/service/java/com/android/server/wifi/WifiStateMachine.java
index 74a653904..d8971ae8b 100644
--- a/service/java/com/android/server/wifi/WifiStateMachine.java
+++ b/service/java/com/android/server/wifi/WifiStateMachine.java
@@ -1259,8 +1259,15 @@ public class WifiStateMachine extends StateMachine implements WifiNative.WifiRss
     /**
      * Initiates connection to a network specified by the user/app. This method checks if the
      * requesting app holds the WIFI_CONFIG_OVERRIDE permission.
+     *
+     * @param netId Id network to initiate connection.
+     * @param uid UID of the app requesting the connection.
+     * @param forceReconnect Whether to force a connection even if we're connected to the same
+     *                       network currently.
      */
-    private boolean connectToUserSelectNetwork(int netId, int uid) {
+    private boolean connectToUserSelectNetwork(int netId, int uid, boolean forceReconnect) {
+        logd("connectToUserSelectNetwork netId " + netId + ", uid " + uid
+                + ", forceReconnect = " + forceReconnect);
         if (mWifiConfigManager.getConfiguredNetwork(netId) == null) {
             loge("connectToUserSelectNetwork Invalid network Id=" + netId);
             return false;
@@ -1274,9 +1281,9 @@ public class WifiStateMachine extends StateMachine implements WifiNative.WifiRss
             // selection.
             mWifiConnectivityManager.setUserConnectChoice(netId);
         }
-        if (mWifiInfo.getNetworkId() == netId) {
+        if (!forceReconnect && mWifiInfo.getNetworkId() == netId) {
             // We're already connected to the user specified network, don't trigger a
-            // reconnection.
+            // reconnection unless it was forced.
             logi("connectToUserSelectNetwork already connecting/connected=" + netId);
         } else {
             startConnectToNetwork(netId, SUPPLICANT_BSSID_ANY);
@@ -4798,7 +4805,7 @@ public class WifiStateMachine extends StateMachine implements WifiNative.WifiRss
                     if (disableOthers) {
                         // If the app has all the necessary permissions, this will trigger a connect
                         // attempt.
-                        ok = connectToUserSelectNetwork(netId, message.sendingUid);
+                        ok = connectToUserSelectNetwork(netId, message.sendingUid, false);
                     } else {
                         ok = mWifiConfigManager.enableNetwork(netId, false, message.sendingUid);
                     }
@@ -4972,6 +4979,7 @@ public class WifiStateMachine extends StateMachine implements WifiNative.WifiRss
                     netId = message.arg1;
                     config = (WifiConfiguration) message.obj;
                     mWifiConnectionStatistics.numWifiManagerJoinAttempt++;
+                    boolean hasCredentialChanged = false;
                     // New network addition.
                     if (config != null) {
                         result = mWifiConfigManager.addOrUpdateNetwork(config, message.sendingUid);
@@ -4983,8 +4991,10 @@ public class WifiStateMachine extends StateMachine implements WifiNative.WifiRss
                             break;
                         }
                         netId = result.getNetworkId();
+                        hasCredentialChanged = result.hasCredentialChanged();
                     }
-                    if (!connectToUserSelectNetwork(netId, message.sendingUid)) {
+                    if (!connectToUserSelectNetwork(
+                            netId, message.sendingUid, hasCredentialChanged)) {
                         messageHandlingStatus = MESSAGE_HANDLING_STATUS_FAIL;
                         replyToMessage(message, WifiManager.CONNECT_NETWORK_FAILED,
                                 WifiManager.NOT_AUTHORIZED);
@@ -5013,30 +5023,39 @@ public class WifiStateMachine extends StateMachine implements WifiNative.WifiRss
                                 WifiManager.ERROR);
                         break;
                     }
-                    netId = result.getNetworkId();
-                    if (mWifiInfo.getNetworkId() == netId) {
-                        if (result.hasIpChanged()) {
-                            // The currently connection configuration was changed
-                            // We switched from DHCP to static or from static to DHCP, or the
-                            // static IP address has changed.
-                            log("Reconfiguring IP on connection");
-                            // TODO: clear addresses and disable IPv6
-                            // to simplify obtainingIpState.
-                            transitionTo(mObtainingIpState);
-                        }
-                        if (result.hasProxyChanged()) {
-                            log("Reconfiguring proxy on connection");
-                            mIpManager.setHttpProxy(
-                                    getCurrentWifiConfiguration().getHttpProxy());
-                        }
-                    } else if (!mWifiConfigManager.enableNetwork(result.getNetworkId(),
-                            false, message.sendingUid)) {
-                        loge("ENABLE_NETWORK config=" + config + " failed");
+                    if (!mWifiConfigManager.enableNetwork(
+                            result.getNetworkId(), false, message.sendingUid)) {
+                        loge("SAVE_NETWORK enabling config=" + config + " failed");
                         messageHandlingStatus = MESSAGE_HANDLING_STATUS_FAIL;
                         replyToMessage(message, WifiManager.SAVE_NETWORK_FAILED,
                                 WifiManager.ERROR);
                         break;
                     }
+                    netId = result.getNetworkId();
+                    if (mWifiInfo.getNetworkId() == netId) {
+                        if (result.hasCredentialChanged()) {
+                            // The network credentials changed and we're connected to this network,
+                            // start a new connection with the updated credentials.
+                            logi("SAVE_NETWORK credential changed for config=" + config.configKey()
+                                    + ", Reconnecting.");
+                            startConnectToNetwork(netId, SUPPLICANT_BSSID_ANY);
+                        } else {
+                            if (result.hasProxyChanged()) {
+                                log("Reconfiguring proxy on connection");
+                                mIpManager.setHttpProxy(
+                                        getCurrentWifiConfiguration().getHttpProxy());
+                            }
+                            if (result.hasIpChanged()) {
+                                // The current connection configuration was changed
+                                // We switched from DHCP to static or from static to DHCP, or the
+                                // static IP address has changed.
+                                log("Reconfiguring IP on connection");
+                                // TODO(b/36576642): clear addresses and disable IPv6
+                                // to simplify obtainingIpState.
+                                transitionTo(mObtainingIpState);
+                            }
+                        }
+                    }
                     broadcastWifiCredentialChanged(WifiManager.WIFI_CREDENTIAL_SAVED, config);
                     replyToMessage(message, WifiManager.SAVE_NETWORK_SUCCEEDED);
                     break;
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java
index 615f61190..951f8e114 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiConfigManagerTest.java
@@ -1325,6 +1325,7 @@ public class WifiConfigManagerTest {
                 mWifiConfigManager.getConfiguredNetwork(result.getNetworkId());
         assertTrue("Updating network non-credentials config should not clear hasEverConnected.",
                 retrievedNetwork.getNetworkSelectionStatus().getHasEverConnected());
+        assertFalse(result.hasCredentialChanged());
     }
 
     /**
@@ -3790,6 +3791,7 @@ public class WifiConfigManagerTest {
                 mWifiConfigManager.getConfiguredNetwork(result.getNetworkId());
         assertFalse("Updating network credentials config should clear hasEverConnected.",
                 retrievedNetwork.getNetworkSelectionStatus().getHasEverConnected());
+        assertTrue(result.hasCredentialChanged());
     }
 
     /**