refactor(web): enhance network page (#3247)

## Problem

Beyond the usual enhancements, there was some pending work to make the
network pages more useful for users. The following issues were
identified as high priority:

* Associated devices to connection: it is difficult to identify which
devices belong to each connection. Improving the connection list to
clearly show associated devices would make it easier for users to manage
their networks.

* Contextual actions for connections: Users can only click to view a
connection and then edit it. Would be better providing a contextual menu
with actions such as view details, edit, connect, or delete allows users
to perform common tasks more efficiently.

* Creating new connections: Users can only edit existing connections,
which limits a lot the network configuration. Adding the ability to
create new connections will address this gap and improve overall
usability.

Related to,

-
https://trello.com/c/jvgwGFIl/5581-8-bsc1247441-agama-ui-does-not-allow-to-add-or-remove-network-connections
(protected link)
- https://bugzilla.suse.com/show_bug.cgi?id=1247441 (protected link)
- https://jira.suse.com/browse/PED-15913 (protected link)

## Solution

* Backend enhancements: Added a backend improvement to notify the system
about added or removed connections, as well as changes to devices,
ensuring the internal state stays in sync (see PR #3244 ).

* Connections table: Brought back the connections table, enhanced to
give users more control and visibility over their connections.

* Manage connections: Users can now add and remove connections directly
from the UI.

* IP settings improvements: Added the dnsSearchList field to the
connection IPSettingsForm to support additional DNS configuration
options (related to https://jira.suse.com/browse/PED-15913).

Author: dgdavid

rust/agama-lib/share/profile.schema.json

@@ -365,13 +365,21 @@
                   "type": "string"
                 }
               },
-              "dnsSearchlist": {
+              "dnsSearchList": {
                 "type": "array",
                 "items": {
                   "description": "DNS search domains",
                   "type": "string"
                 }
               },
+              "dnsSearchlist": {
+                "type": "array",
+                "items": {
+                  "description": "DNS search domains",
+                  "type": "string"
+                },
+                "deprecated": true
+              },
               "ignoreAutoDns": {
                 "description": "Whether DNS options provided via DHCP are used or not",
                 "type": "boolean"

rust/agama-network/src/action.rs

@@ -54,6 +54,10 @@ pub enum Action {
     GetConnections(Responder<Vec<Connection>>),
     /// Gets all scanned access points
     GetAccessPoints(Responder<Vec<AccessPoint>>),
+    /// Adds a new access point.
+    AddAccessPoint(Box<AccessPoint>),
+    /// Removes an access point by its hardware address.
+    RemoveAccessPoint(String),
     /// Adds a new device.
     AddDevice(Box<Device>),
     /// Updates a device by its `name`.
@@ -66,7 +70,7 @@ pub enum Action {
     GetDevices(Responder<Vec<Device>>),
     GetGeneralState(Responder<GeneralState>),
     /// Connection state changed
-    ChangeConnectionState(String, ConnectionState),
+    ChangeConnectionState(Uuid, ConnectionState),
     /// Persists existing connections if none exist and the network copy is not disabled.
     ProposeDefault(Responder<Result<(), NetworkStateError>>),
     // Copies persistent connections to the target system
@@ -77,8 +81,8 @@ pub enum Action {
     UpdateGeneralState(GeneralState),
     /// Forces a wireless networks scan refresh
     RefreshScan(Responder<Result<(), NetworkAdapterError>>),
-    /// Remove the connection with the given ID.
-    RemoveConnection(String),
+    /// Remove the connection with the given UUID.
+    RemoveConnection(Uuid),
     /// Apply the current configuration.
     Apply(Responder<Result<(), NetworkAdapterError>>),
 }

rust/agama-network/src/error.rs

@@ -40,6 +40,8 @@ pub enum NetworkStateError {
     CannotUpdateConnection(String),
     #[error("Unknown device '{0}'")]
     UnknownDevice(String),
+    #[error("Unknown access point '{0}'")]
+    UnknownAccessPoint(String),
     #[error("Invalid connection UUID: '{0}'")]
     InvalidUuid(String),
     #[error("Invalid IP address: '{0}'")]

rust/agama-network/src/model.rs

@@ -296,12 +296,10 @@ impl NetworkState {
 
     /// Updates a connection with a new one.
     ///
-    /// It uses the `id` to decide which connection to update.
-    ///
-    /// Additionally, it registers the connection to be removed when the changes are applied.
+    /// It uses the `uuid` to decide which connection to update.
     pub fn update_connection(&mut self, conn: Connection) -> Result<(), NetworkStateError> {
-        let Some(old_conn) = self.get_connection_mut(&conn.id) else {
-            return Err(NetworkStateError::UnknownConnection(conn.id.clone()));
+        let Some(old_conn) = self.get_connection_by_uuid_mut(conn.uuid) else {
+            return Err(NetworkStateError::UnknownConnection(conn.uuid.to_string()));
         };
         *old_conn = conn;
 
@@ -311,9 +309,9 @@ impl NetworkState {
     /// Removes a connection from the state.
     ///
     /// Additionally, it registers the connection to be removed when the changes are applied.
-    pub fn remove_connection(&mut self, id: &str) -> Result<(), NetworkStateError> {
-        let Some(position) = self.connections.iter().position(|d| d.id == id) else {
-            return Err(NetworkStateError::UnknownConnection(id.to_string()));
+    pub fn remove_connection(&mut self, uuid: Uuid) -> Result<(), NetworkStateError> {
+        let Some(position) = self.connections.iter().position(|d| d.uuid == uuid) else {
+            return Err(NetworkStateError::UnknownConnection(uuid.to_string()));
         };
 
         self.connections.remove(position);
@@ -343,6 +341,34 @@ impl NetworkState {
         Ok(())
     }
 
+    pub fn add_access_point(&mut self, ap: AccessPoint) -> Result<(), NetworkStateError> {
+        if let Some(position) = self
+            .access_points
+            .iter()
+            .position(|a| a.hw_address == ap.hw_address)
+        {
+            self.access_points.remove(position);
+        }
+        self.access_points.push(ap);
+
+        Ok(())
+    }
+
+    pub fn remove_access_point(&mut self, hw_address: &str) -> Result<(), NetworkStateError> {
+        let Some(position) = self
+            .access_points
+            .iter()
+            .position(|a| a.hw_address == hw_address)
+        else {
+            return Err(NetworkStateError::UnknownAccessPoint(
+                hw_address.to_string(),
+            ));
+        };
+
+        self.access_points.remove(position);
+        Ok(())
+    }
+
     /// Sets a controller's ports.
     ///
     /// If the connection is not a controller, returns an error.
@@ -417,22 +443,23 @@ mod tests {
     #[test]
     fn test_update_connection() {
         let mut state = NetworkState::default();
+        let uuid = Uuid::new_v4();
         let conn0 = Connection {
             id: "eth0".to_string(),
-            uuid: Uuid::new_v4(),
+            uuid,
             ..Default::default()
         };
         state.add_connection(conn0).unwrap();
 
-        let uuid = Uuid::new_v4();
         let conn1 = Connection {
             id: "eth0".to_string(),
             uuid,
+            firewall_zone: Some("public".to_string()),
             ..Default::default()
         };
         state.update_connection(conn1).unwrap();
-        let found = state.get_connection("eth0").unwrap();
-        assert_eq!(found.uuid, uuid);
+        let found = state.get_connection_by_uuid(uuid).unwrap();
+        assert_eq!(found.firewall_zone, Some("public".to_string()));
     }
 
     #[test]
@@ -446,20 +473,77 @@ mod tests {
     #[test]
     fn test_remove_connection() {
         let mut state = NetworkState::default();
-        let conn0 = Connection::new("eth0".to_string(), DeviceType::Ethernet);
+        let uuid = Uuid::new_v4();
+        let conn0 = Connection {
+            id: "eth0".to_string(),
+            uuid,
+            ..Default::default()
+        };
         state.add_connection(conn0).unwrap();
-        state.remove_connection("eth0".as_ref()).unwrap();
-        let found = state.get_connection("eth0");
+        state.remove_connection(uuid).unwrap();
+        let found = state.get_connection_by_uuid(uuid);
         assert!(found.is_none());
     }
 
     #[test]
     fn test_remove_unknown_connection() {
         let mut state = NetworkState::default();
-        let error = state.remove_connection("unknown".as_ref()).unwrap_err();
+        let uuid = Uuid::new_v4();
+        let error = state.remove_connection(uuid).unwrap_err();
         assert!(matches!(error, NetworkStateError::UnknownConnection(_)));
     }
 
+    #[test]
+    fn test_remove_device() {
+        let mut state = NetworkState::default();
+        let device = Device {
+            name: "eth0".to_string(),
+            ..Default::default()
+        };
+        state.add_device(device).unwrap();
+        state.remove_device("eth0").unwrap();
+        assert!(state.get_device("eth0").is_none());
+    }
+
+    #[test]
+    fn test_add_access_point() {
+        let mut state = NetworkState::default();
+        let ap = AccessPoint {
+            hw_address: "AA:BB:CC:DD:EE:FF".to_string(),
+            ssid: SSID(b"test".to_vec()),
+            ..Default::default()
+        };
+        state.add_access_point(ap.clone()).unwrap();
+        assert_eq!(state.access_points.len(), 1);
+        assert_eq!(state.access_points[0].hw_address, "AA:BB:CC:DD:EE:FF");
+
+        // Adding same AP should replace it (in our implementation we remove and push)
+        let mut ap2 = ap.clone();
+        ap2.strength = 80;
+        state.add_access_point(ap2).unwrap();
+        assert_eq!(state.access_points.len(), 1);
+        assert_eq!(state.access_points[0].strength, 80);
+    }
+
+    #[test]
+    fn test_remove_access_point() {
+        let mut state = NetworkState::default();
+        let ap = AccessPoint {
+            hw_address: "AA:BB:CC:DD:EE:FF".to_string(),
+            ..Default::default()
+        };
+        state.add_access_point(ap).unwrap();
+        state.remove_access_point("AA:BB:CC:DD:EE:FF").unwrap();
+        assert_eq!(state.access_points.len(), 0);
+    }
+
+    #[test]
+    fn test_remove_unknown_access_point() {
+        let mut state = NetworkState::default();
+        let error = state.remove_access_point("unknown").unwrap_err();
+        assert!(matches!(error, NetworkStateError::UnknownAccessPoint(_)));
+    }
+
     #[test]
     fn test_is_loopback() {
         let conn = Connection::new("eth0".to_string(), DeviceType::Ethernet);
@@ -1726,7 +1810,7 @@ pub struct TunConfig {
 #[serde(rename_all = "camelCase")]
 pub enum NetworkChange {
     ConnectionAdded(Connection),
-    ConnectionRemoved(String),
+    ConnectionRemoved(Uuid),
     /// A new device has been added.
     DeviceAdded(Device),
     /// A device has been removed.
@@ -1737,9 +1821,13 @@ pub enum NetworkChange {
     DeviceUpdated(String, Device),
     /// A connection state has changed.
     ConnectionStateChanged {
-        id: String,
+        uuid: Uuid,
         state: ConnectionState,
     },
+    /// A new access point has been added.
+    AccessPointAdded(AccessPoint),
+    /// An access point has been removed.
+    AccessPointRemoved(String),
 }
 
 #[derive(Default, Debug, PartialEq, Clone, Deserialize, Serialize, utoipa::ToSchema)]

rust/agama-network/src/nm/builder.rs

@@ -25,9 +25,12 @@ use crate::{
     nm::{
         dbus::connection_from_dbus,
         model::NmDeviceType,
-        proxies::{ConnectionProxy, DeviceProxy, IP4ConfigProxy, IP6ConfigProxy},
+        proxies::{AccessPointProxy, ConnectionProxy, DeviceProxy, IP4ConfigProxy, IP6ConfigProxy},
+    },
+    types::{
+        AccessPoint, ConnectionFlags, Device, DeviceState, DeviceType, IpConfig, IpRoute,
+        MacAddress, SSID,
     },
-    types::{ConnectionFlags, Device, DeviceState, DeviceType, IpConfig, IpRoute, MacAddress},
 };
 use cidr::IpInet;
 use std::{collections::HashMap, net::IpAddr, str::FromStr};
@@ -44,6 +47,38 @@ pub struct DeviceFromProxyBuilder<'a> {
     proxy: &'a DeviceProxy<'a>,
 }
 
+/// Builder to create an [AccessPoint] from its corresponding NetworkManager D-Bus representation.
+pub struct AccessPointFromProxyBuilder<'a> {
+    device_name: String,
+    proxy: &'a AccessPointProxy<'a>,
+}
+
+impl<'a> AccessPointFromProxyBuilder<'a> {
+    pub fn new(device_name: String, proxy: &'a AccessPointProxy<'a>) -> Self {
+        Self { device_name, proxy }
+    }
+
+    /// Creates an [AccessPoint] starting on the [AccessPointProxy].
+    pub async fn build(&self) -> Result<AccessPoint, NmError> {
+        let ssid = SSID(self.proxy.ssid().await?);
+        let hw_address = self.proxy.hw_address().await?;
+        let strength = self.proxy.strength().await?;
+        let flags = self.proxy.flags().await?;
+        let rsn_flags = self.proxy.rsn_flags().await?;
+        let wpa_flags = self.proxy.wpa_flags().await?;
+
+        Ok(AccessPoint {
+            device: self.device_name.clone(),
+            ssid,
+            hw_address,
+            strength,
+            flags,
+            rsn_flags,
+            wpa_flags,
+        })
+    }
+}
+
 impl<'a> ConnectionFromProxyBuilder<'a> {
     pub fn new(_connection: &zbus::Connection, proxy: &'a ConnectionProxy<'a>) -> Self {
         Self { proxy }
@@ -139,6 +174,7 @@ impl<'a> DeviceFromProxyBuilder<'a> {
     ) -> Result<IpConfig, NmError> {
         let address_data = ip4_proxy.address_data().await?;
         let nameserver_data = ip4_proxy.nameserver_data().await?;
+        let mut dns_searchlist = ip4_proxy.searches().await?;
         let mut addresses: Vec<IpInet> = vec![];
         let mut nameservers: Vec<IpAddr> = vec![];
 
@@ -160,6 +196,12 @@ impl<'a> DeviceFromProxyBuilder<'a> {
                 nameservers.push(address)
             }
         }
+
+        for search in ip6_proxy.searches().await? {
+            if !dns_searchlist.contains(&search) {
+                dns_searchlist.push(search);
+            }
+        }
         // FIXME: Convert from Vec<u8> to [u8; 16] and take into account big vs little endian order,
         // in IP6Config there is no nameserver-data.
         //
@@ -188,6 +230,7 @@ impl<'a> DeviceFromProxyBuilder<'a> {
         let mut ip_config = IpConfig {
             addresses,
             nameservers,
+            dns_searchlist,
             routes4,
             routes6,
             ..Default::default()

rust/agama-network/src/nm/streams/common.rs

@@ -35,6 +35,8 @@ pub enum NmChange {
     ActiveConnectionAdded(OwnedObjectPath),
     ActiveConnectionUpdated(OwnedObjectPath),
     ActiveConnectionRemoved(OwnedObjectPath),
+    AccessPointAdded(OwnedObjectPath, OwnedObjectPath),
+    AccessPointRemoved(OwnedObjectPath, OwnedObjectPath),
 }
 
 pub async fn build_added_and_removed_stream(
@@ -63,3 +65,17 @@ pub async fn build_properties_changed_stream(
     let stream = MessageStream::for_match_rule(rule, connection, Some(1)).await?;
     Ok(stream)
 }
+
+/// Returns a stream of wireless signals to be used by DeviceChangedStream.
+///
+/// It listens for AccessPointAdded and AccessPointRemoved signals.
+pub async fn build_wireless_signals_stream(
+    connection: &zbus::Connection,
+) -> Result<MessageStream, NmError> {
+    let rule = MatchRule::builder()
+        .msg_type(MessageType::Signal)
+        .interface("org.freedesktop.NetworkManager.Device.Wireless")?
+        .build();
+    let stream = MessageStream::for_match_rule(rule, connection, Some(1)).await?;
+    Ok(stream)
+}