From 2e184bbc980d071852f7d042c19082ea0b1035c1 Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Fri, 17 Jul 2026 07:11:23 -0500 Subject: [PATCH 1/6] ui: Extract cancel setup to separate method --- credentialsd-ui/src/dbus.rs | 119 +++++++++++++++++++++--------------- 1 file changed, 70 insertions(+), 49 deletions(-) diff --git a/credentialsd-ui/src/dbus.rs b/credentialsd-ui/src/dbus.rs index b7e783f..41aff1c 100644 --- a/credentialsd-ui/src/dbus.rs +++ b/credentialsd-ui/src/dbus.rs @@ -67,55 +67,18 @@ impl CredentialPortalBackend { }; // Set up cancellation background task. - let (cancel_task, client_cancelled_tx, gui_stopped_tx, cancel_gui_rx) = { - let sender = sender.clone(); - let object_path = handle.clone(); - let (client_cancelled_tx, client_cancelled_rx) = channel::bounded(1); - let (gui_stopped_tx, gui_stopped_rx) = channel::bounded(1); - let (cancel_gui_tx, cancel_gui_rx) = channel::bounded(1); - let client_disconnected_rx = - notify_on_disconnected(connection, sender.clone().into()).await?; - let object_server = object_server.clone(); - let cancel_task = async_std::task::spawn(async move { - let disconnect_fut = client_disconnected_rx.recv(); - let cancel_fut = client_cancelled_rx.recv(); - let gui_stopped_fut = gui_stopped_rx.recv(); - - match disconnect_fut.race(cancel_fut).race(gui_stopped_fut).await { - Ok(Ok(())) => { - tracing::debug!(%sender, "Client cancelled or disconnected, dropping request") - } - Ok(Err(err)) => { - tracing::error!(%sender, %err, "Failed to watch for client disconnection") - } - Err(_) => { - tracing::error!(%sender, "Client disconnection task dropped prematurely") - } - } - - if cancel_gui_tx.send(()).await.is_err() { - tracing::error!("Failed to send cancellation request to GUI"); - }; - if let Err(err) = object_server - .remove::(&object_path) - .await - { - tracing::warn!(%object_path, %err, "Failed to remove Ceremony request"); - } - if let Err(err) = object_server - .remove::(&object_path) - .await - { - tracing::warn!(%object_path, %err, "Failed to remove org.freedesktop.impl.portal.Request"); - } - }); - ( - cancel_task, - client_cancelled_tx, - gui_stopped_tx, - cancel_gui_rx, - ) - }; + let CancelHandle { + cancel_task, + client_cancelled_tx, + gui_stopped_tx, + cancel_gui_rx, + } = setup_cancellation( + connection, + object_server.to_owned(), + sender.to_owned().into(), + handle.clone(), + ) + .await?; let app_display_name = DesktopAppInfo::new(&format!("{app_id}.desktop")) .ok_or_else(|| { @@ -161,6 +124,64 @@ impl CredentialPortalBackend { } } +struct CancelHandle { + cancel_task: JoinHandle<()>, + client_cancelled_tx: Sender>, + gui_stopped_tx: Sender>, + cancel_gui_rx: Receiver<()>, +} + +async fn setup_cancellation( + connection: &Connection, + object_server: ObjectServer, + sender: OwnedUniqueName, + object_path: OwnedObjectPath, +) -> fdo::Result { + let (client_cancelled_tx, client_cancelled_rx) = channel::bounded(1); + let (gui_stopped_tx, gui_stopped_rx) = channel::bounded(1); + let (cancel_gui_tx, cancel_gui_rx) = channel::bounded(1); + let client_disconnected_rx = notify_on_disconnected(connection, sender.clone().into()).await?; + let cancel_task = async_std::task::spawn(async move { + let disconnect_fut = client_disconnected_rx.recv(); + let cancel_fut = client_cancelled_rx.recv(); + let gui_stopped_fut = gui_stopped_rx.recv(); + + match disconnect_fut.race(cancel_fut).race(gui_stopped_fut).await { + Ok(Ok(())) => { + tracing::debug!(%sender, "Client cancelled or disconnected, dropping request") + } + Ok(Err(err)) => { + tracing::error!(%sender, %err, "Failed to watch for client disconnection") + } + Err(_) => { + tracing::error!(%sender, "Client disconnection task dropped prematurely") + } + } + + if cancel_gui_tx.send(()).await.is_err() { + tracing::error!("Failed to send cancellation request to GUI"); + }; + if let Err(err) = object_server + .remove::(&object_path) + .await + { + tracing::warn!(%object_path, %err, "Failed to remove Ceremony request"); + } + if let Err(err) = object_server + .remove::(&object_path) + .await + { + tracing::warn!(%object_path, %err, "Failed to remove org.freedesktop.impl.portal.Request"); + } + }); + Ok(CancelHandle { + cancel_task, + client_cancelled_tx, + gui_stopped_tx, + cancel_gui_rx, + }) +} + async fn notify_on_disconnected( conn: &Connection, bus_name: BusName<'static>, From 42dc12f66be04c67cb6def7b9a067af292b5ede9 Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Fri, 17 Jul 2026 06:59:13 -0500 Subject: [PATCH 2/6] ui: Use CreateSession pattern for backend portal --- credentialsd-ui/src/dbus.rs | 234 ++++++++++++++++++++------ credentialsd/src/dbus/flow_control.rs | 8 +- credentialsd/src/dbus/ui_control.rs | 132 +++++++++------ 3 files changed, 275 insertions(+), 99 deletions(-) diff --git a/credentialsd-ui/src/dbus.rs b/credentialsd-ui/src/dbus.rs index 41aff1c..49bfca9 100644 --- a/credentialsd-ui/src/dbus.rs +++ b/credentialsd-ui/src/dbus.rs @@ -1,4 +1,7 @@ -use std::sync::Arc; +use std::sync::{ + Arc, + atomic::{AtomicBool, Ordering}, +}; use async_std::{ channel::{self, Receiver, Sender}, @@ -8,14 +11,15 @@ use async_std::{ use futures_lite::{FutureExt, StreamExt}; use gio_unix::DesktopAppInfo; use gio_unix::prelude::AppInfoExt; +use tracing::Instrument; use zbus::{ Connection, ObjectServer, fdo::{self, DBusProxy}, interface, message::Header, names::{BusName, OwnedUniqueName}, - object_server::SignalEmitter, - zvariant::{Optional, OwnedObjectPath}, + object_server::{InterfaceRef, SignalEmitter}, + zvariant::{ObjectPath, Optional, OwnedObjectPath}, }; use credentialsd_common::model::{ @@ -48,12 +52,13 @@ pub(crate) struct UiContext { impl CredentialPortalBackend { // D-Bus has long argument signatures. #[expect(clippy::too_many_arguments)] - async fn initialize( + async fn create_session( &self, #[zbus(connection)] connection: &Connection, #[zbus(header)] header: Header<'_>, #[zbus(object_server)] object_server: &ObjectServer, - handle: OwnedObjectPath, + #[zbus(signal_emitter)] signal_emitter: SignalEmitter<'_>, + session_handle: OwnedObjectPath, parent_window: Optional, _origin: String, r#type: Operation, @@ -76,7 +81,7 @@ impl CredentialPortalBackend { connection, object_server.to_owned(), sender.to_owned().into(), - handle.clone(), + session_handle.clone(), ) .await?; @@ -99,29 +104,81 @@ impl CredentialPortalBackend { options, }; let ui_events_forwarder_task = Arc::new(AsyncMutex::new(None)); - let mut ceremony = CeremonyObject { + let ceremony = CeremonyObject { ui_context, request_tx: self.request_tx.clone(), return_address: sender.to_owned().into(), ui_events_forwarder_task: ui_events_forwarder_task.clone(), bg_events_tx: None, + session_handle: session_handle.clone(), }; - let request = CeremonyRequest { + + let mut session = CeremonySession::new( ui_events_forwarder_task, - cancel_task: Arc::new(AsyncMutex::new(Some(cancel_task))), + Arc::new(AsyncMutex::new(Some(cancel_task))), client_cancelled_tx, - }; - object_server.at(handle.clone(), request).await?; - - let emitter = SignalEmitter::new(connection, handle.clone())?; - ceremony - .start(gui_stopped_tx, cancel_gui_rx, emitter.to_owned()) + ceremony, + session_handle.clone(), + ); + session + .start(gui_stopped_tx, cancel_gui_rx, signal_emitter.to_owned()) .await?; - object_server.at(handle, ceremony).await?; + let session_created = object_server.at(session_handle.clone(), session).await?; + if !session_created { + tracing::warn!(%session_handle, "Client requested session that already exists"); + return Err(fdo::Error::Failed("Could not create session".to_string())); + } - tracing::debug!("Received UI launch request"); + tracing::debug!(%session_handle, "Received UI launch request"); Ok(()) } + + async fn notify_state_changed( + &self, + #[zbus(object_server)] object_server: &ObjectServer, + session_handle: ObjectPath<'_>, + event: BackgroundEvent, + ) -> fdo::Result<()> { + let span = tracing::info_span!("NotifyStateChanged", %session_handle); + match object_server + .interface::<_, CeremonySession>(&session_handle) + .instrument(span) + .await + { + Ok(session_iface) => { + let session = session_iface.get_mut().await; + if let Err(err) = session.ceremony.on_state_changed(event).await { + tracing::debug!( + %err, "Error occurred while forwarding state update to ceremony session. Closing session." + ); + CeremonySession::shutdown(object_server, session_handle).await?; + return Err(fdo::Error::Failed( + "Error sending state notification. Closing session.".to_string(), + )); + }; + Ok(()) + } + Err(zbus::Error::InterfaceNotFound) => { + tracing::info!(%session_handle, "No session exists at the requested path"); + Err(fdo::Error::Failed(format!( + "No session exists at {session_handle}" + ))) + } + Err(err) => { + tracing::error!(%session_handle, %err, "Unknown error occurred while looking up session"); + Err(fdo::Error::Failed(format!( + "Unknown error occurred while looking up session for {session_handle}" + ))) + } + } + } + + #[zbus(signal)] + async fn user_interacted( + emitter: SignalEmitter<'_>, + session_handle: ObjectPath<'_>, + event: &UserInteractedEvent, + ) -> zbus::Result<()>; } struct CancelHandle { @@ -135,7 +192,7 @@ async fn setup_cancellation( connection: &Connection, object_server: ObjectServer, sender: OwnedUniqueName, - object_path: OwnedObjectPath, + session_handle: OwnedObjectPath, ) -> fdo::Result { let (client_cancelled_tx, client_cancelled_rx) = channel::bounded(1); let (gui_stopped_tx, gui_stopped_rx) = channel::bounded(1); @@ -161,18 +218,16 @@ async fn setup_cancellation( if cancel_gui_tx.send(()).await.is_err() { tracing::error!("Failed to send cancellation request to GUI"); }; - if let Err(err) = object_server - .remove::(&object_path) - .await - { - tracing::warn!(%object_path, %err, "Failed to remove Ceremony request"); - } - if let Err(err) = object_server - .remove::(&object_path) - .await - { - tracing::warn!(%object_path, %err, "Failed to remove org.freedesktop.impl.portal.Request"); - } + + match CeremonySession::shutdown(&object_server, session_handle.as_ref()).await { + Ok(_) => {} + Err(zbus::Error::InterfaceNotFound) => { + tracing::debug!(%session_handle, "Session handle not found"); + } + Err(err) => { + tracing::error!(%session_handle, %err, "Error occurred while shutting down session"); + } + }; }); Ok(CancelHandle { cancel_task, @@ -224,11 +279,11 @@ pub struct CeremonyObject { pub return_address: OwnedUniqueName, ui_events_forwarder_task: Arc>>>, bg_events_tx: Option>, + session_handle: OwnedObjectPath, } impl CeremonyObject { /// Start the UI ceremony with an initial set of available credential interfaces. - /// Call this method after subscribing to the signals. async fn start( &mut self, stopped_tx: Sender>, @@ -250,13 +305,15 @@ impl CeremonyObject { self.bg_events_tx = Some(bg_events_tx); let emitter = emitter - .set_destination(BusName::Unique((&self.return_address).into())) + .set_destination(BusName::Unique(self.return_address.as_ref())) .to_owned(); + let session_handle = self.session_handle.clone(); *ui_events_task = Some(async_std::task::spawn(async move { while let Ok(ui_event) = ui_events_rx.recv().await { - tracing::trace!(?ui_event, "Sending UI event signal to portal"); - if emitter.user_interacted(&ui_event).await.is_err() { - tracing::trace!("Failed to send UI event signal."); + if let Err(err) = + Self::on_user_interacted(&emitter, session_handle.as_ref(), &ui_event).await + { + tracing::trace!(%session_handle, %err, "Failed to send UI event signal."); break; } } @@ -311,11 +368,8 @@ impl CeremonyObject { } Ok(()) } -} -#[interface(name = "org.freedesktop.impl.portal.experimental.Credential.Ceremony")] -impl CeremonyObject { - async fn notify_state_changed(&self, event: BackgroundEvent) -> fdo::Result<()> { + async fn on_state_changed(&self, event: BackgroundEvent) -> fdo::Result<()> { tracing::trace!(?event, "Received background event"); if let Some(tx) = &self.bg_events_tx { if tx.send(event).await.is_ok() { @@ -328,23 +382,33 @@ impl CeremonyObject { Err(fdo::Error::Failed("Failed to handle event".to_string())) } - #[zbus(signal)] - async fn user_interacted( - emitter: SignalEmitter<'_>, - event: &UserInteractedEvent, - ) -> zbus::Result<()>; + async fn on_user_interacted( + emitter: &SignalEmitter<'_>, + session_handle: ObjectPath<'_>, + ui_event: &UserInteractedEvent, + ) -> zbus::Result<()> { + tracing::trace!(?ui_event, "Sending UI event signal to portal"); + emitter.user_interacted(session_handle, &ui_event).await + } } -struct CeremonyRequest { +struct CeremonySession { ui_events_forwarder_task: Arc>>>, cancel_task: Arc>>>, client_cancelled_tx: Sender>, + ceremony: CeremonyObject, + object_path: OwnedObjectPath, + emit_closed_signal: AtomicBool, } -#[interface(name = "org.freedesktop.impl.portal.Request")] -impl CeremonyRequest { - async fn close(&mut self) -> fdo::Result<()> { - tracing::debug!("Client requested cancellation"); +#[interface(name = "org.freedesktop.impl.portal.Session")] +impl CeremonySession { + async fn close( + &mut self, + #[zbus(object_server)] object_server: &ObjectServer, + ) -> fdo::Result<()> { + let session_handle = &self.object_path; + tracing::debug!(%session_handle, "Client requested cancellation"); if let Some(task) = self.ui_events_forwarder_task.lock().await.take() { task.cancel().await; } @@ -352,8 +416,78 @@ impl CeremonyRequest { task.cancel().await; } if self.client_cancelled_tx.send(Ok(())).await.is_err() { - tracing::warn!("Request already cancelled"); + tracing::warn!(%session_handle, "Session already cancelled"); } + // Don't emit the signal when the caller initiates close + self.emit_closed_signal.store(false, Ordering::Relaxed); + tracing::debug!(%session_handle, "Removing session"); + if let Err(err) = Self::shutdown(object_server, session_handle.as_ref()).await { + tracing::warn!(%session_handle, %err, "Failed to tear down session"); + }; Ok(()) } + + #[zbus(signal)] + async fn closed(emitter: &SignalEmitter<'_>) -> zbus::Result<()>; +} + +impl CeremonySession { + fn new( + ui_events_forwarder_task: Arc>>>, + cancel_task: Arc>>>, + client_cancelled_tx: Sender>, + ceremony: CeremonyObject, + object_path: OwnedObjectPath, + ) -> Self { + Self { + ui_events_forwarder_task, + cancel_task, + client_cancelled_tx, + ceremony, + object_path, + emit_closed_signal: AtomicBool::new(true), + } + } + + async fn start( + &mut self, + stopped_tx: Sender>, + cancel_rx: Receiver<()>, + emitter: SignalEmitter<'static>, + ) -> fdo::Result<()> { + self.ceremony.start(stopped_tx, cancel_rx, emitter).await + } + + async fn shutdown( + object_server: &ObjectServer, + session_handle: ObjectPath<'_>, + ) -> zbus::Result<()> { + let iface: InterfaceRef = + match object_server.interface(&session_handle).await { + Ok(iface) => iface, + Err(zbus::Error::InterfaceNotFound) => { + tracing::warn!(%session_handle, "Session not found"); + return Ok(()); + } + Err(err) => { + return Err(err); + } + }; + // Emit the signal once. + let session = iface.get().await; + if session.emit_closed_signal.swap(false, Ordering::Relaxed) + && let Err(err) = iface.closed().await + { + tracing::error!(%session_handle, %err, "Failed to emit Session::Closed signal"); + } + + match object_server.remove::(&session_handle).await { + Ok(_) => Ok(()), + Err(zbus::Error::InterfaceNotFound) => { + tracing::warn!(%session_handle, "Session not found, may have already been cleaned up"); + return Ok(()); + } + Err(err) => Err(err), + } + } } diff --git a/credentialsd/src/dbus/flow_control.rs b/credentialsd/src/dbus/flow_control.rs index 36e4f65..e6cd040 100644 --- a/credentialsd/src/dbus/flow_control.rs +++ b/credentialsd/src/dbus/flow_control.rs @@ -55,7 +55,9 @@ pub async fn start_flow_control_service() ) .try_into() .expect("valid object path"); let flow = match ui_control_client - .initialize( + .create_session( handle, window_handle, origin, diff --git a/credentialsd/src/dbus/ui_control.rs b/credentialsd/src/dbus/ui_control.rs index 5b8ff75..7cee8c0 100644 --- a/credentialsd/src/dbus/ui_control.rs +++ b/credentialsd/src/dbus/ui_control.rs @@ -10,7 +10,7 @@ use tokio::sync::{ use zbus::{ fdo, proxy, zvariant::{ObjectPath, Optional, OwnedObjectPath}, - Connection, + Connection, MatchRule, MessageStream, }; use credentialsd_common::model::{ @@ -21,9 +21,9 @@ use credentialsd_common::model::{ pub trait UiController { // D-Bus has a lot of arguments #[expect(clippy::too_many_arguments)] - fn initialize( + fn create_session( &self, - handle: OwnedObjectPath, + session_handle: OwnedObjectPath, parent_window: Option, origin: String, r#type: Operation, @@ -40,12 +40,12 @@ pub trait UiController { default_service = "xyz.iinuwa.credentialsd.UiControl", default_path = "/org/freedesktop/portal/desktop" )] -trait UiControlService2 { +trait UiControlService { // D-Bus has a lot of arguments #[expect(clippy::too_many_arguments)] - fn initialize( + fn create_session( &self, - handle: ObjectPath<'_>, + session_handle: ObjectPath<'_>, parent_window: Optional, origin: String, r#type: Operation, @@ -54,12 +54,26 @@ trait UiControlService2 { app_pid: u32, options: PortalBackendOptions, ) -> fdo::Result<()>; + + async fn notify_state_changed( + &self, + session_handle: ObjectPath<'_>, + event: BackgroundEvent, + ) -> fdo::Result<()>; + + #[zbus(signal)] + async fn user_interacted( + &self, + session_handle: ObjectPath<'_>, + update: UserInteractedEvent, + ) -> zbus::Result<()>; } #[derive(Clone, Debug)] pub struct Ceremony { - proxy: Arc>, + proxy: Arc>, ui_events_rx: Arc>>, + session_handle: OwnedObjectPath, } impl Ceremony { @@ -68,7 +82,11 @@ impl Ceremony { } pub async fn send_state_update(&self, event: BackgroundEvent) -> Result<(), ()> { - if let Err(err) = self.proxy.notify_state_changed(event).await { + if let Err(err) = self + .proxy + .notify_state_changed(self.session_handle.as_ref(), event) + .await + { match err { fdo::Error::UnknownObject(description) => { tracing::error!(%description, "Flow D-Bus object no longer available at path"); @@ -80,18 +98,13 @@ impl Ceremony { Ok(()) } } + #[proxy( gen_blocking = false, - interface = "org.freedesktop.impl.portal.experimental.Credential.Ceremony", - default_service = "xyz.iinuwa.credentialsd.UiControl" + interface = "org.freedesktop.impl.portal.Session" )] -trait CeremonyObject { - async fn notify_state_changed(&self, event: BackgroundEvent) -> fdo::Result<()>; - - async fn cancel(&self) -> fdo::Result<()>; - - #[zbus(signal)] - async fn user_interacted(&self, update: UserInteractedEvent) -> zbus::Result<()>; +trait CeremonySession { + async fn close(&self) -> fdo::Result<()>; } #[derive(Debug)] @@ -103,16 +116,12 @@ impl UiControlServiceClient { pub fn new(conn: Connection) -> Self { Self { conn } } - - async fn proxy2(&self) -> Result, zbus::Error> { - UiControlService2Proxy::new(&self.conn).await - } } impl UiController for UiControlServiceClient { - async fn initialize( + async fn create_session( &self, - handle: OwnedObjectPath, + session_handle: OwnedObjectPath, parent_window: Option, origin: String, r#type: Operation, @@ -121,17 +130,13 @@ impl UiController for UiControlServiceClient { app_pid: u32, options: PortalBackendOptions, ) -> Result> { - let ceremony = CeremonyObjectProxy::new(&self.conn, handle.clone()).await?; let (from_ui_tx, from_ui_rx) = mpsc::channel(32); - let from_ui_tx2 = from_ui_tx.clone(); - let ui_event_stream = ceremony.receive_user_interacted().await?; - tokio::task::spawn(async move { - _ = forward_ui_events(ui_event_stream, from_ui_tx2).await; - }); - self.proxy2() - .await? - .initialize( - handle.as_ref(), + subscribe_ui_events(self.conn.clone(), session_handle.clone(), from_ui_tx).await?; + + let backend_proxy = UiControlServiceProxy::new(&self.conn).await?; + backend_proxy + .create_session( + session_handle.as_ref(), parent_window.into(), origin, r#type, @@ -141,27 +146,60 @@ impl UiController for UiControlServiceClient { options, ) .await?; - tracing::debug!(path = ?handle, "Path initialized"); + tracing::debug!(path = ?session_handle, "Session initialized"); Ok(Ceremony { - proxy: Arc::new(ceremony), + proxy: Arc::new(backend_proxy), ui_events_rx: Arc::new(AsyncMutex::new(from_ui_rx)), + session_handle, }) } } -async fn forward_ui_events( - mut ui_event_stream: UserInteractedStream, +async fn subscribe_ui_events( + connection: Connection, + session_handle: OwnedObjectPath, tx: mpsc::Sender, -) -> Result<(), Box> { - tracing::debug!("Listening for events from UI"); - while let Some(signal) = ui_event_stream.next().await { - tracing::trace!(?signal, "Received event from UI"); - let event = signal.args()?.update; - if tx.send(event).await.is_err() { - tracing::trace!("credential service event listener stopped listening for UI events. Ending event stream listener"); - break; +) -> zbus::Result<()> { + let match_rule = MatchRule::builder() + .msg_type(zbus::message::Type::Signal) + .interface("org.freedesktop.impl.portal.experimental.Credential")? + .member("UserInteracted")? + .arg_path(0, session_handle.clone())? + .build(); + + let session_handle2 = session_handle.clone(); + let mut ui_event_stream = MessageStream::for_match_rule(match_rule, &connection, Some(16)).await? + .filter_map(move |response| match response { + Ok(msg) => Some(msg), + Err(err) => { + tracing::error!(%session_handle, %err, "Error receiving a message the UserInteracted stream"); + None + } + }) + .filter_map(move |msg| match UserInteracted::from_message(msg) { + Some(ui_event) => Some(ui_event), + None => { + tracing::error!(session_handle = %session_handle2, "Error parsing message as {}", stringify!(UserInteracted)); + None + }, + }); + + // Forward the events to the receiver in the background. + tokio::task::spawn(async move { + // _ = forward_ui_events(Box::pin(ui_event_stream), from_ui_tx2).await; + tracing::debug!("Listening for events from UI"); + while let Some(signal) = ui_event_stream.next().await { + tracing::trace!(?signal, "Received event from UI"); + let event = signal.args()?.update; + if tx.send(event).await.is_err() { + tracing::trace!( + "UI event listener stopped listening events. Ending event stream listener" + ); + break; + } } - } - tracing::trace!("Stopping UI event forwarder"); + tracing::trace!("Stopping UI event forwarder"); + Ok::<_, zbus::Error>(()) + }); Ok(()) } From 9d300644af82f54d4accf816b5d4e663d893a837 Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Sat, 18 Jul 2026 09:22:20 -0500 Subject: [PATCH 3/6] ui: Split UserInteracted signal into separate signals --- Cargo.lock | 1 + credentialsd-common/src/model.rs | 56 ++++++++-- credentialsd-ui/src/dbus.rs | 70 ++++++++++-- credentialsd-ui/src/gui/view_model/mod.rs | 17 ++- credentialsd/Cargo.toml | 1 + credentialsd/src/dbus/ui_control.rs | 125 ++++++++++++++++++---- 6 files changed, 229 insertions(+), 41 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 535be0d..b269242 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -806,6 +806,7 @@ dependencies = [ "serde", "serde_json", "tokio", + "tokio-stream", "tracing", "tracing-subscriber", "zbus", diff --git a/credentialsd-common/src/model.rs b/credentialsd-common/src/model.rs index 260aeec..891070a 100644 --- a/credentialsd-common/src/model.rs +++ b/credentialsd-common/src/model.rs @@ -268,6 +268,21 @@ impl<'de> Deserialize<'de> for BackgroundEvent { } } +/// Emitted when a client enters a PIN for the selected authenticator. +#[derive(Debug, SerializeDict, DeserializeDict, PartialEq, Type)] +#[zvariant(signature = "dict")] +pub struct ClientPinEnteredEvent { + /// Length of the PIN MUST NOT be greater than 63 bytes. + /// File descriptor must be memory-mapped to be read. + pub pin_fd: OwnedFd, +} + +impl From for UserInteractedEvent { + fn from(value: ClientPinEnteredEvent) -> Self { + UserInteractedEvent::ClientPinEntered(value.pin_fd) + } +} + #[derive(Clone, Debug, Default, SerializeDict, DeserializeDict, PartialEq, Type, Value)] #[zvariant(signature = "dict")] pub struct Credential { @@ -276,12 +291,38 @@ pub struct Credential { pub username: Option, } +/// Emitted when an an authenticator presents multiple matching credentials, and +/// the user selects one of them. +#[derive(Clone, Debug, PartialEq, SerializeDict, DeserializeDict, Type)] +pub struct CredentialSelectedEvent { + /// ID of the selected credential, from the original + /// [BackgroundEvent::SelectingCredential] event. + pub id: String, +} + +impl From for UserInteractedEvent { + fn from(value: CredentialSelectedEvent) -> Self { + UserInteractedEvent::CredentialSelected(value.id) + } +} + #[derive(Clone, Debug, PartialEq, Serialize, Deserialize, Type)] pub struct Device { pub id: String, pub transport: Transport, } +/// Emitted when the backend is ready to start credential discovery. +#[derive(Debug, PartialEq, SerializeDict, DeserializeDict, Type)] +#[zvariant(signature = "dict")] +pub struct DiscoveryRequestedEvent {} + +impl From for UserInteractedEvent { + fn from(_: DiscoveryRequestedEvent) -> Self { + UserInteractedEvent::DiscoveryRequested + } +} + #[derive(Debug, Clone)] pub enum Error { /// Some unknown error with the authenticator occurred. @@ -357,14 +398,11 @@ pub struct PortalBackendOptions { #[derive(Clone, Debug, PartialEq, Serialize, Deserialize, Type)] #[zvariant(signature = "s")] pub enum Transport { - #[serde(rename = "BLE")] Ble, HybridLinked, HybridQr, Internal, - #[serde(rename = "NFC")] Nfc, - #[serde(rename = "USB")] Usb, } @@ -382,12 +420,12 @@ impl TryInto for &str { fn try_into(self) -> Result { match self { - "BLE" => Ok(Transport::Ble), + "Ble" => Ok(Transport::Ble), "HybridLinked" => Ok(Transport::HybridLinked), "HybridQr" => Ok(Transport::HybridQr), "Internal" => Ok(Transport::Internal), - "NFC" => Ok(Transport::Nfc), - "USB" => Ok(Transport::Usb), + "Nfc" => Ok(Transport::Nfc), + "Usb" => Ok(Transport::Usb), _ => Err(format!("Unrecognized transport: {}", self.to_owned())), } } @@ -402,12 +440,12 @@ impl From for String { impl Transport { pub fn as_str(&self) -> &'static str { match self { - Transport::Ble => "BLE", + Transport::Ble => "Ble", Transport::HybridLinked => "HybridLinked", Transport::HybridQr => "HybridQr", Transport::Internal => "Internal", - Transport::Nfc => "NFC", - Transport::Usb => "USB", + Transport::Nfc => "Nfc", + Transport::Usb => "Usb", } } } diff --git a/credentialsd-ui/src/dbus.rs b/credentialsd-ui/src/dbus.rs index 49bfca9..81df7ce 100644 --- a/credentialsd-ui/src/dbus.rs +++ b/credentialsd-ui/src/dbus.rs @@ -23,7 +23,8 @@ use zbus::{ }; use credentialsd_common::model::{ - BackgroundEvent, Device, Operation, PortalBackendOptions, UserInteractedEvent, WindowHandle, + BackgroundEvent, ClientPinEnteredEvent, CredentialSelectedEvent, Device, + DiscoveryRequestedEvent, Operation, PortalBackendOptions, UserInteractedEvent, WindowHandle, }; use crate::{RequestingApplication, ViewRequest, client::FlowControlClient}; @@ -121,7 +122,12 @@ impl CredentialPortalBackend { session_handle.clone(), ); session - .start(gui_stopped_tx, cancel_gui_rx, signal_emitter.to_owned()) + .start( + object_server.to_owned(), + gui_stopped_tx, + cancel_gui_rx, + signal_emitter.to_owned(), + ) .await?; let session_created = object_server.at(session_handle.clone(), session).await?; if !session_created { @@ -174,10 +180,24 @@ impl CredentialPortalBackend { } #[zbus(signal)] - async fn user_interacted( + async fn discovery_requested( emitter: SignalEmitter<'_>, session_handle: ObjectPath<'_>, - event: &UserInteractedEvent, + event: &DiscoveryRequestedEvent, + ) -> zbus::Result<()>; + + #[zbus(signal)] + async fn client_pin_entered( + emitter: SignalEmitter<'_>, + session_handle: ObjectPath<'_>, + event: &ClientPinEnteredEvent, + ) -> zbus::Result<()>; + + #[zbus(signal)] + async fn credential_selected( + emitter: SignalEmitter<'_>, + session_handle: ObjectPath<'_>, + event: &CredentialSelectedEvent, ) -> zbus::Result<()>; } @@ -286,6 +306,7 @@ impl CeremonyObject { /// Start the UI ceremony with an initial set of available credential interfaces. async fn start( &mut self, + object_server: ObjectServer, stopped_tx: Sender>, cancel_rx: Receiver<()>, emitter: SignalEmitter<'static>, @@ -310,8 +331,13 @@ impl CeremonyObject { let session_handle = self.session_handle.clone(); *ui_events_task = Some(async_std::task::spawn(async move { while let Ok(ui_event) = ui_events_rx.recv().await { - if let Err(err) = - Self::on_user_interacted(&emitter, session_handle.as_ref(), &ui_event).await + if let Err(err) = Self::on_user_interacted( + &object_server, + &emitter, + session_handle.as_ref(), + ui_event, + ) + .await { tracing::trace!(%session_handle, %err, "Failed to send UI event signal."); break; @@ -383,12 +409,33 @@ impl CeremonyObject { } async fn on_user_interacted( + object_server: &ObjectServer, emitter: &SignalEmitter<'_>, session_handle: ObjectPath<'_>, - ui_event: &UserInteractedEvent, + ui_event: UserInteractedEvent, ) -> zbus::Result<()> { tracing::trace!(?ui_event, "Sending UI event signal to portal"); - emitter.user_interacted(session_handle, &ui_event).await + match ui_event { + UserInteractedEvent::DiscoveryRequested => { + emitter + .discovery_requested(session_handle, &DiscoveryRequestedEvent {}) + .await?; + } + UserInteractedEvent::ClientPinEntered(pin_fd) => { + emitter + .client_pin_entered(session_handle, &ClientPinEnteredEvent { pin_fd }) + .await?; + } + UserInteractedEvent::CredentialSelected(id) => { + emitter + .credential_selected(session_handle, &CredentialSelectedEvent { id }) + .await?; + } + UserInteractedEvent::RequestCancelled => { + CeremonySession::shutdown(object_server, session_handle).await?; + } + } + Ok(()) } } @@ -451,11 +498,14 @@ impl CeremonySession { async fn start( &mut self, + object_server: ObjectServer, stopped_tx: Sender>, cancel_rx: Receiver<()>, emitter: SignalEmitter<'static>, ) -> fdo::Result<()> { - self.ceremony.start(stopped_tx, cancel_rx, emitter).await + self.ceremony + .start(object_server, stopped_tx, cancel_rx, emitter) + .await } async fn shutdown( @@ -485,7 +535,7 @@ impl CeremonySession { Ok(_) => Ok(()), Err(zbus::Error::InterfaceNotFound) => { tracing::warn!(%session_handle, "Session not found, may have already been cleaned up"); - return Ok(()); + Ok(()) } Err(err) => Err(err), } diff --git a/credentialsd-ui/src/gui/view_model/mod.rs b/credentialsd-ui/src/gui/view_model/mod.rs index d66bbf9..b02ffee 100644 --- a/credentialsd-ui/src/gui/view_model/mod.rs +++ b/credentialsd-ui/src/gui/view_model/mod.rs @@ -1,6 +1,6 @@ pub mod gtk; -use std::sync::Arc; +use std::{fmt::Debug, sync::Arc}; use async_std::prelude::*; use async_std::{ @@ -338,7 +338,7 @@ impl ViewModel { } } -#[derive(Debug, Serialize, Deserialize)] +#[derive(Serialize, Deserialize)] pub enum ViewEvent { Initiated, CredentialSelected(String), @@ -346,6 +346,19 @@ pub enum ViewEvent { UserCancelled, } +impl Debug for ViewEvent { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Initiated => write!(f, "Initiated"), + Self::CredentialSelected(arg0) => { + f.debug_tuple("CredentialSelected").field(arg0).finish() + } + Self::PinEntered(_) => f.debug_tuple("PinEntered").field(&"******").finish(), + Self::UserCancelled => write!(f, "UserCancelled"), + } + } +} + #[derive(Debug)] pub enum Event { Background(BackgroundEvent), diff --git a/credentialsd/Cargo.toml b/credentialsd/Cargo.toml index 10885c0..5aaa4d0 100644 --- a/credentialsd/Cargo.toml +++ b/credentialsd/Cargo.toml @@ -21,6 +21,7 @@ ring = "0.17.14" serde.workspace = true serde_json = "1.0.140" tokio = { version = "1.45.0", features = ["rt-multi-thread"] } +tokio-stream = "0.1.18" tracing.workspace = true tracing-subscriber.workspace = true zbus = { workspace = true, default-features = false, features = ["tokio"] } diff --git a/credentialsd/src/dbus/ui_control.rs b/credentialsd/src/dbus/ui_control.rs index 7cee8c0..7523fe6 100644 --- a/credentialsd/src/dbus/ui_control.rs +++ b/credentialsd/src/dbus/ui_control.rs @@ -2,19 +2,22 @@ use std::{error::Error, future::Future, sync::Arc}; -use futures_lite::StreamExt; use tokio::sync::{ mpsc::{self, Receiver}, Mutex as AsyncMutex, }; +use tokio_stream::StreamExt; use zbus::{ - fdo, proxy, + fdo::{self, DBusProxy}, + names::OwnedUniqueName, + proxy, zvariant::{ObjectPath, Optional, OwnedObjectPath}, Connection, MatchRule, MessageStream, }; use credentialsd_common::model::{ - BackgroundEvent, Device, Operation, PortalBackendOptions, UserInteractedEvent, WindowHandle, + BackgroundEvent, ClientPinEnteredEvent, CredentialSelectedEvent, Device, + DiscoveryRequestedEvent, Operation, PortalBackendOptions, UserInteractedEvent, WindowHandle, }; /// Used by the credential service to control the UI. @@ -67,6 +70,27 @@ trait UiControlService { session_handle: ObjectPath<'_>, update: UserInteractedEvent, ) -> zbus::Result<()>; + + #[zbus(signal)] + async fn discovery_requested( + &self, + session_handle: ObjectPath<'_>, + event: DiscoveryRequestedEvent, + ) -> zbus::Result<()>; + + #[zbus(signal)] + async fn client_pin_entered( + &self, + session_handle: ObjectPath<'_>, + event: ClientPinEnteredEvent, + ) -> zbus::Result<()>; + + #[zbus(signal)] + async fn credential_selected( + &self, + session_handle: ObjectPath<'_>, + event: CredentialSelectedEvent, + ) -> zbus::Result<()>; } #[derive(Clone, Debug)] @@ -105,6 +129,9 @@ impl Ceremony { )] trait CeremonySession { async fn close(&self) -> fdo::Result<()>; + + #[zbus(signal)] + async fn closed(&self) -> zbus::Result<()>; } #[derive(Debug)] @@ -131,9 +158,19 @@ impl UiController for UiControlServiceClient { options: PortalBackendOptions, ) -> Result> { let (from_ui_tx, from_ui_rx) = mpsc::channel(32); - subscribe_ui_events(self.conn.clone(), session_handle.clone(), from_ui_tx).await?; - let backend_proxy = UiControlServiceProxy::new(&self.conn).await?; + let dbus_proxy = DBusProxy::new(&self.conn).await?; + let sender = dbus_proxy + .get_name_owner(backend_proxy.as_ref().destination().clone()) + .await?; + subscribe_ui_events( + self.conn.clone(), + sender, + session_handle.clone(), + from_ui_tx, + ) + .await?; + backend_proxy .create_session( session_handle.as_ref(), @@ -157,41 +194,89 @@ impl UiController for UiControlServiceClient { async fn subscribe_ui_events( connection: Connection, + sender: OwnedUniqueName, session_handle: OwnedObjectPath, tx: mpsc::Sender, ) -> zbus::Result<()> { let match_rule = MatchRule::builder() .msg_type(zbus::message::Type::Signal) .interface("org.freedesktop.impl.portal.experimental.Credential")? - .member("UserInteracted")? + .destination( + connection + .unique_name() + .expect("unique name to be set for connection") + .clone(), + )? + .path("/org/freedesktop/portal/desktop")? + .sender(sender.clone())? .arg_path(0, session_handle.clone())? .build(); let session_handle2 = session_handle.clone(); - let mut ui_event_stream = MessageStream::for_match_rule(match_rule, &connection, Some(16)).await? + let ui_event_stream = MessageStream::for_match_rule(match_rule, &connection, Some(16)).await? .filter_map(move |response| match response { - Ok(msg) => Some(msg), + Ok(msg) => { + Some(msg) + }, Err(err) => { - tracing::error!(%session_handle, %err, "Error receiving a message the UserInteracted stream"); + tracing::error!(session_handle = %session_handle2, %err, "Error receiving a message from the UI event stream"); None } }) - .filter_map(move |msg| match UserInteracted::from_message(msg) { - Some(ui_event) => Some(ui_event), - None => { - tracing::error!(session_handle = %session_handle2, "Error parsing message as {}", stringify!(UserInteracted)); - None - }, + .filter_map(|msg| { + let signal_name = msg.header().member().map(|name| name.to_string())?; + + match signal_name.as_str() { + stringify!(DiscoveryRequested) => { + DiscoveryRequested::from_message(msg)? + .args().ok() + .map(|args| args.event) + .map(UserInteractedEvent::from) + } + stringify!(ClientPinEntered) => { + ClientPinEntered::from_message(msg)? + .args().ok() + .map(|args| args.event) + .map(UserInteractedEvent::from) + } + stringify!(CredentialSelected) => { + CredentialSelected::from_message(msg)? + .args().ok() + .map(|args| args.event) + .map(UserInteractedEvent::from) + } + _ => None + } }); + let closed_match_rule = MatchRule::builder() + .msg_type(zbus::message::Type::Signal) + .interface("org.freedesktop.impl.portal.Session")? + .member("Closed")? + .destination( + connection + .unique_name() + .expect("unique name to be set for connection") + .clone(), + )? + .path(session_handle.clone())? + .sender(sender.clone())? + .build(); + + let closed_event_stream = + MessageStream::for_match_rule(closed_match_rule, &connection, Some(1)) + .await? + .filter_map(|s| s.ok()) + .map(|_| UserInteractedEvent::RequestCancelled); + + let mut ui_event_stream = ui_event_stream.merge(closed_event_stream); + // Forward the events to the receiver in the background. tokio::task::spawn(async move { - // _ = forward_ui_events(Box::pin(ui_event_stream), from_ui_tx2).await; tracing::debug!("Listening for events from UI"); - while let Some(signal) = ui_event_stream.next().await { - tracing::trace!(?signal, "Received event from UI"); - let event = signal.args()?.update; - if tx.send(event).await.is_err() { + while let Some(ui_event) = ui_event_stream.next().await { + tracing::trace!(?ui_event, "Received event from UI"); + if tx.send(ui_event).await.is_err() { tracing::trace!( "UI event listener stopped listening events. Ending event stream listener" ); From dfb88f5b66309b4e5d19c33829ca439c8f1703c8 Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Sun, 19 Jul 2026 07:34:23 -0500 Subject: [PATCH 4/6] ui: Extract required signal fields into separate args --- credentialsd-common/src/model.rs | 30 +++++------------------------ credentialsd-ui/src/dbus.rs | 20 ++++++++++--------- credentialsd/src/dbus/ui_control.rs | 23 +++++++++++----------- 3 files changed, 27 insertions(+), 46 deletions(-) diff --git a/credentialsd-common/src/model.rs b/credentialsd-common/src/model.rs index 891070a..54d29e6 100644 --- a/credentialsd-common/src/model.rs +++ b/credentialsd-common/src/model.rs @@ -271,17 +271,7 @@ impl<'de> Deserialize<'de> for BackgroundEvent { /// Emitted when a client enters a PIN for the selected authenticator. #[derive(Debug, SerializeDict, DeserializeDict, PartialEq, Type)] #[zvariant(signature = "dict")] -pub struct ClientPinEnteredEvent { - /// Length of the PIN MUST NOT be greater than 63 bytes. - /// File descriptor must be memory-mapped to be read. - pub pin_fd: OwnedFd, -} - -impl From for UserInteractedEvent { - fn from(value: ClientPinEnteredEvent) -> Self { - UserInteractedEvent::ClientPinEntered(value.pin_fd) - } -} +pub struct ClientPinEnteredOptions {} #[derive(Clone, Debug, Default, SerializeDict, DeserializeDict, PartialEq, Type, Value)] #[zvariant(signature = "dict")] @@ -294,17 +284,7 @@ pub struct Credential { /// Emitted when an an authenticator presents multiple matching credentials, and /// the user selects one of them. #[derive(Clone, Debug, PartialEq, SerializeDict, DeserializeDict, Type)] -pub struct CredentialSelectedEvent { - /// ID of the selected credential, from the original - /// [BackgroundEvent::SelectingCredential] event. - pub id: String, -} - -impl From for UserInteractedEvent { - fn from(value: CredentialSelectedEvent) -> Self { - UserInteractedEvent::CredentialSelected(value.id) - } -} +pub struct CredentialSelectedOptions {} #[derive(Clone, Debug, PartialEq, Serialize, Deserialize, Type)] pub struct Device { @@ -315,10 +295,10 @@ pub struct Device { /// Emitted when the backend is ready to start credential discovery. #[derive(Debug, PartialEq, SerializeDict, DeserializeDict, Type)] #[zvariant(signature = "dict")] -pub struct DiscoveryRequestedEvent {} +pub struct DiscoveryRequestedOptions {} -impl From for UserInteractedEvent { - fn from(_: DiscoveryRequestedEvent) -> Self { +impl From for UserInteractedEvent { + fn from(_: DiscoveryRequestedOptions) -> Self { UserInteractedEvent::DiscoveryRequested } } diff --git a/credentialsd-ui/src/dbus.rs b/credentialsd-ui/src/dbus.rs index 81df7ce..a5abacb 100644 --- a/credentialsd-ui/src/dbus.rs +++ b/credentialsd-ui/src/dbus.rs @@ -19,12 +19,12 @@ use zbus::{ message::Header, names::{BusName, OwnedUniqueName}, object_server::{InterfaceRef, SignalEmitter}, - zvariant::{ObjectPath, Optional, OwnedObjectPath}, + zvariant::{Fd, ObjectPath, Optional, OwnedFd, OwnedObjectPath}, }; use credentialsd_common::model::{ - BackgroundEvent, ClientPinEnteredEvent, CredentialSelectedEvent, Device, - DiscoveryRequestedEvent, Operation, PortalBackendOptions, UserInteractedEvent, WindowHandle, + BackgroundEvent, ClientPinEnteredOptions, CredentialSelectedOptions, Device, + DiscoveryRequestedOptions, Operation, PortalBackendOptions, UserInteractedEvent, WindowHandle, }; use crate::{RequestingApplication, ViewRequest, client::FlowControlClient}; @@ -183,21 +183,23 @@ impl CredentialPortalBackend { async fn discovery_requested( emitter: SignalEmitter<'_>, session_handle: ObjectPath<'_>, - event: &DiscoveryRequestedEvent, + options: DiscoveryRequestedOptions, ) -> zbus::Result<()>; #[zbus(signal)] async fn client_pin_entered( emitter: SignalEmitter<'_>, session_handle: ObjectPath<'_>, - event: &ClientPinEnteredEvent, + pin_fd: OwnedFd, + options: ClientPinEnteredOptions, ) -> zbus::Result<()>; #[zbus(signal)] async fn credential_selected( emitter: SignalEmitter<'_>, session_handle: ObjectPath<'_>, - event: &CredentialSelectedEvent, + id: String, + options: CredentialSelectedOptions, ) -> zbus::Result<()>; } @@ -418,17 +420,17 @@ impl CeremonyObject { match ui_event { UserInteractedEvent::DiscoveryRequested => { emitter - .discovery_requested(session_handle, &DiscoveryRequestedEvent {}) + .discovery_requested(session_handle, DiscoveryRequestedOptions {}) .await?; } UserInteractedEvent::ClientPinEntered(pin_fd) => { emitter - .client_pin_entered(session_handle, &ClientPinEnteredEvent { pin_fd }) + .client_pin_entered(session_handle, pin_fd, ClientPinEnteredOptions {}) .await?; } UserInteractedEvent::CredentialSelected(id) => { emitter - .credential_selected(session_handle, &CredentialSelectedEvent { id }) + .credential_selected(session_handle, id, CredentialSelectedOptions {}) .await?; } UserInteractedEvent::RequestCancelled => { diff --git a/credentialsd/src/dbus/ui_control.rs b/credentialsd/src/dbus/ui_control.rs index 7523fe6..4bf7aef 100644 --- a/credentialsd/src/dbus/ui_control.rs +++ b/credentialsd/src/dbus/ui_control.rs @@ -11,13 +11,13 @@ use zbus::{ fdo::{self, DBusProxy}, names::OwnedUniqueName, proxy, - zvariant::{ObjectPath, Optional, OwnedObjectPath}, + zvariant::{ObjectPath, Optional, OwnedFd, OwnedObjectPath}, Connection, MatchRule, MessageStream, }; use credentialsd_common::model::{ - BackgroundEvent, ClientPinEnteredEvent, CredentialSelectedEvent, Device, - DiscoveryRequestedEvent, Operation, PortalBackendOptions, UserInteractedEvent, WindowHandle, + BackgroundEvent, ClientPinEnteredOptions, CredentialSelectedOptions, Device, + DiscoveryRequestedOptions, Operation, PortalBackendOptions, UserInteractedEvent, WindowHandle, }; /// Used by the credential service to control the UI. @@ -75,21 +75,23 @@ trait UiControlService { async fn discovery_requested( &self, session_handle: ObjectPath<'_>, - event: DiscoveryRequestedEvent, + options: DiscoveryRequestedOptions, ) -> zbus::Result<()>; #[zbus(signal)] async fn client_pin_entered( &self, session_handle: ObjectPath<'_>, - event: ClientPinEnteredEvent, + pin_fd: OwnedFd, + options: ClientPinEnteredOptions, ) -> zbus::Result<()>; #[zbus(signal)] async fn credential_selected( &self, session_handle: ObjectPath<'_>, - event: CredentialSelectedEvent, + id: String, + options: CredentialSelectedOptions, ) -> zbus::Result<()>; } @@ -230,20 +232,17 @@ async fn subscribe_ui_events( stringify!(DiscoveryRequested) => { DiscoveryRequested::from_message(msg)? .args().ok() - .map(|args| args.event) - .map(UserInteractedEvent::from) + .map(|_| UserInteractedEvent::DiscoveryRequested) } stringify!(ClientPinEntered) => { ClientPinEntered::from_message(msg)? .args().ok() - .map(|args| args.event) - .map(UserInteractedEvent::from) + .map(|args| UserInteractedEvent::ClientPinEntered(args.pin_fd)) } stringify!(CredentialSelected) => { CredentialSelected::from_message(msg)? .args().ok() - .map(|args| args.event) - .map(UserInteractedEvent::from) + .map(|args| UserInteractedEvent::CredentialSelected(args.id)) } _ => None } From 5d7c8b39556cb68e39b732afadf88fbcd0bfd991 Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Mon, 20 Jul 2026 09:40:08 -0500 Subject: [PATCH 5/6] ui: Split NotifyStateChanged into multiple methods --- credentialsd-common/src/model.rs | 53 +++++- credentialsd-ui/src/dbus.rs | 247 ++++++++++++++++++++++++--- credentialsd/src/dbus/ui_control.rs | 251 ++++++++++++++++++++++++++-- 3 files changed, 504 insertions(+), 47 deletions(-) diff --git a/credentialsd-common/src/model.rs b/credentialsd-common/src/model.rs index 54d29e6..dd94ef6 100644 --- a/credentialsd-common/src/model.rs +++ b/credentialsd-common/src/model.rs @@ -37,14 +37,14 @@ const BACKGROUND_EVENT_USB_WAITING: u32 = 0x41; const BACKGROUND_EVENT_USB_SELECTING_DEVICE: u32 = 0x42; const BACKGROUND_EVENT_USB_CONNECTED: u32 = 0x43; -const BACKGROUND_EVENT_ERROR_INTERNAL: u32 = 0x80000001; -const BACKGROUND_EVENT_ERROR_TIMED_OUT: u32 = 0x80000002; -const BACKGROUND_EVENT_ERROR_CANCELLED: u32 = 0x80000003; -const BACKGROUND_EVENT_ERROR_AUTHENTICATOR: u32 = 0x80000004; -const BACKGROUND_EVENT_ERROR_NO_CREDENTIALS: u32 = 0x80000005; -const BACKGROUND_EVENT_ERROR_CREDENTIAL_EXCLUDED: u32 = 0x80000006; -const BACKGROUND_EVENT_ERROR_PIN_ATTEMPTS_EXHAUSTED: u32 = 0x80000007; -const BACKGROUND_EVENT_ERROR_PIN_NOT_SET: u32 = 0x80000008; +pub const BACKGROUND_EVENT_ERROR_INTERNAL: u32 = 0x80000001; +pub const BACKGROUND_EVENT_ERROR_TIMED_OUT: u32 = 0x80000002; +pub const BACKGROUND_EVENT_ERROR_CANCELLED: u32 = 0x80000003; +pub const BACKGROUND_EVENT_ERROR_AUTHENTICATOR: u32 = 0x80000004; +pub const BACKGROUND_EVENT_ERROR_NO_CREDENTIALS: u32 = 0x80000005; +pub const BACKGROUND_EVENT_ERROR_CREDENTIAL_EXCLUDED: u32 = 0x80000006; +pub const BACKGROUND_EVENT_ERROR_PIN_ATTEMPTS_EXHAUSTED: u32 = 0x80000007; +pub const BACKGROUND_EVENT_ERROR_PIN_NOT_SET: u32 = 0x80000008; const USER_INTERACTED_EVENT_DISCOVERY_REQUESTED: u32 = 0x01; const USER_INTERACTED_EVENT_CLIENT_PIN_ENTERED: u32 = 0x04; @@ -356,6 +356,43 @@ impl TryFrom<&Value<'_>> for Error { } } +#[derive(Debug, PartialEq, SerializeDict, DeserializeDict, Type)] +#[zvariant(signature = "dict")] +pub struct NotifyNeedsPinOptions {} + +#[derive(Debug, SerializeDict, DeserializeDict, Type)] +#[zvariant(signature = "dict")] +pub struct NotifyNeedsUserVerificationOptions {} + +#[derive(Debug, SerializeDict, DeserializeDict, Type)] +#[zvariant(signature = "dict")] +pub struct NotifyNeedsUserPresenceOptions {} + +#[derive(Debug, SerializeDict, DeserializeDict, Type)] +#[zvariant(signature = "dict")] +pub struct NotifySelectingCredentialOptions {} + +#[derive(Debug, SerializeDict, DeserializeDict, Type)] +#[zvariant(signature = "dict")] +pub struct NotifyHybridStartedOptions {} + +#[derive(Debug, SerializeDict, DeserializeDict, Type)] +#[zvariant(signature = "dict")] +pub struct NotifyHybridConnectingOptions {} + +/// Emitted +#[derive(Debug, SerializeDict, DeserializeDict, Type)] +#[zvariant(signature = "dict")] +pub struct NotifyHybridConnectedOptions {} + +#[derive(Debug, SerializeDict, DeserializeDict, Type)] +#[zvariant(signature = "dict")] +pub struct NotifyNfcConnectedOptions {} + +#[derive(Debug, SerializeDict, DeserializeDict, Type)] +#[zvariant(signature = "dict")] +pub struct NotifyUsbConnectedOptions {} + #[derive(Clone, Debug, Serialize, Deserialize, Type)] pub enum Operation { PublicKeyCreate, diff --git a/credentialsd-ui/src/dbus.rs b/credentialsd-ui/src/dbus.rs index a5abacb..751392f 100644 --- a/credentialsd-ui/src/dbus.rs +++ b/credentialsd-ui/src/dbus.rs @@ -19,12 +19,20 @@ use zbus::{ message::Header, names::{BusName, OwnedUniqueName}, object_server::{InterfaceRef, SignalEmitter}, - zvariant::{Fd, ObjectPath, Optional, OwnedFd, OwnedObjectPath}, + zvariant::{ObjectPath, Optional, OwnedFd, OwnedObjectPath}, }; use credentialsd_common::model::{ - BackgroundEvent, ClientPinEnteredOptions, CredentialSelectedOptions, Device, - DiscoveryRequestedOptions, Operation, PortalBackendOptions, UserInteractedEvent, WindowHandle, + BACKGROUND_EVENT_ERROR_AUTHENTICATOR, BACKGROUND_EVENT_ERROR_CANCELLED, + BACKGROUND_EVENT_ERROR_CREDENTIAL_EXCLUDED, BACKGROUND_EVENT_ERROR_INTERNAL, + BACKGROUND_EVENT_ERROR_NO_CREDENTIALS, BACKGROUND_EVENT_ERROR_PIN_ATTEMPTS_EXHAUSTED, + BACKGROUND_EVENT_ERROR_PIN_NOT_SET, BACKGROUND_EVENT_ERROR_TIMED_OUT, BackgroundEvent, + ClientPinEnteredOptions, Credential, CredentialSelectedOptions, Device, + DiscoveryRequestedOptions, NotifyHybridConnectedOptions, NotifyHybridConnectingOptions, + NotifyHybridStartedOptions, NotifyNeedsPinOptions, NotifyNeedsUserPresenceOptions, + NotifyNeedsUserVerificationOptions, NotifyNfcConnectedOptions, + NotifySelectingCredentialOptions, NotifyUsbConnectedOptions, Operation, PortalBackendOptions, + UserInteractedEvent, WindowHandle, }; use crate::{RequestingApplication, ViewRequest, client::FlowControlClient}; @@ -139,10 +147,218 @@ impl CredentialPortalBackend { Ok(()) } - async fn notify_state_changed( + /// Called when the authenticator needs a client PIN. + async fn notify_needs_pin( + &self, + #[zbus(object_server)] object_server: &ObjectServer, + session_handle: ObjectPath<'_>, + attempts_left: u32, + _options: NotifyNeedsPinOptions, + ) -> fdo::Result<()> { + let attempts_left = if attempts_left == u32::MAX { + None + } else { + Some(attempts_left) + }; + self.notify_state_changed( + object_server, + session_handle, + BackgroundEvent::NeedsPin { attempts_left }, + ) + .await + } + + /// Called when the authenticator needs a user verification gesture. + async fn notify_needs_user_verification( + &self, + #[zbus(object_server)] object_server: &ObjectServer, + session_handle: ObjectPath<'_>, + attempts_left: u32, + _options: NotifyNeedsUserVerificationOptions, + ) -> fdo::Result<()> { + let attempts_left = if attempts_left == u32::MAX { + None + } else { + Some(attempts_left) + }; + self.notify_state_changed( + object_server, + session_handle, + BackgroundEvent::NeedsUserVerification { attempts_left }, + ) + .await + } + + /// Called when the authenticator needs a user presence gesture. + async fn notify_needs_user_presence( + &self, + #[zbus(object_server)] object_server: &ObjectServer, + session_handle: ObjectPath<'_>, + _options: NotifyNeedsUserPresenceOptions, + ) -> fdo::Result<()> { + self.notify_state_changed( + object_server, + session_handle, + BackgroundEvent::NeedsUserPresence, + ) + .await + } + + /// Called when the authenticator detects multiple credentials matching + /// credentials for a request. + async fn notify_selecting_credential( &self, #[zbus(object_server)] object_server: &ObjectServer, session_handle: ObjectPath<'_>, + credentials: Vec, + _options: NotifySelectingCredentialOptions, + ) -> fdo::Result<()> { + self.notify_state_changed( + object_server, + session_handle, + BackgroundEvent::SelectingCredential { creds: credentials }, + ) + .await + } + + /// Called when the platform begins scanning for CTAP2 hybrid advertisements. + async fn notify_hybrid_started( + &self, + #[zbus(object_server)] object_server: &ObjectServer, + session_handle: ObjectPath<'_>, + invocation_data: OwnedFd, + _options: NotifyHybridStartedOptions, + ) -> fdo::Result<()> { + self.notify_state_changed( + object_server, + session_handle, + BackgroundEvent::HybridStarted(invocation_data), + ) + .await + } + + /// Called when the platform has received a CTAP2 hybrid advertisement and is + /// establishing a channel. + async fn notify_hybrid_connecting( + &self, + #[zbus(object_server)] object_server: &ObjectServer, + session_handle: ObjectPath<'_>, + _options: NotifyHybridConnectingOptions, + ) -> fdo::Result<()> { + self.notify_state_changed( + object_server, + session_handle, + BackgroundEvent::HybridConnecting, + ) + .await + } + + /// Called when a CTAP2 hybrid channel has been established. + async fn notify_hybrid_connected( + &self, + #[zbus(object_server)] object_server: &ObjectServer, + session_handle: ObjectPath<'_>, + _options: NotifyHybridConnectedOptions, + ) -> fdo::Result<()> { + self.notify_state_changed( + object_server, + session_handle, + BackgroundEvent::HybridConnected, + ) + .await + } + + /// Called when a NFC authenticator has connected. + async fn notify_nfc_connected( + &self, + #[zbus(object_server)] object_server: &ObjectServer, + session_handle: ObjectPath<'_>, + _options: NotifyNfcConnectedOptions, + ) -> fdo::Result<()> { + self.notify_state_changed(object_server, session_handle, BackgroundEvent::NfcConnected) + .await + } + + /// Called when a USB authenticator has been selected by the user. + async fn notify_usb_connected( + &self, + #[zbus(object_server)] object_server: &ObjectServer, + session_handle: ObjectPath<'_>, + _options: NotifyUsbConnectedOptions, + ) -> fdo::Result<()> { + self.notify_state_changed(object_server, session_handle, BackgroundEvent::UsbConnected) + .await + } + + /// Called when the authentication ceremony completes successfully. + async fn notify_ceremony_completed( + &self, + #[zbus(object_server)] object_server: &ObjectServer, + session_handle: ObjectPath<'_>, + ) -> fdo::Result<()> { + self.notify_state_changed( + object_server, + session_handle, + BackgroundEvent::CeremonyCompleted, + ) + .await + } + + /// Called when an error occurs during the authentication ceremony completes successfully. + async fn notify_error_occurred( + &self, + #[zbus(object_server)] object_server: &ObjectServer, + session_handle: ObjectPath<'_>, + error: u32, + ) -> fdo::Result<()> { + let error_event = match error { + BACKGROUND_EVENT_ERROR_INTERNAL => Ok(BackgroundEvent::ErrorInternal), + BACKGROUND_EVENT_ERROR_TIMED_OUT => Ok(BackgroundEvent::ErrorTimedOut), + BACKGROUND_EVENT_ERROR_CANCELLED => Ok(BackgroundEvent::ErrorCancelled), + BACKGROUND_EVENT_ERROR_AUTHENTICATOR => Ok(BackgroundEvent::ErrorAuthenticator), + BACKGROUND_EVENT_ERROR_NO_CREDENTIALS => Ok(BackgroundEvent::ErrorNoCredentials), + BACKGROUND_EVENT_ERROR_CREDENTIAL_EXCLUDED => { + Ok(BackgroundEvent::ErrorCredentialExcluded) + } + BACKGROUND_EVENT_ERROR_PIN_ATTEMPTS_EXHAUSTED => { + Ok(BackgroundEvent::ErrorPinAttemptsExhausted) + } + BACKGROUND_EVENT_ERROR_PIN_NOT_SET => Ok(BackgroundEvent::ErrorPinNotSet), + _ => Err(fdo::Error::Failed("Unknown error code".to_string())), + }?; + self.notify_state_changed(object_server, session_handle, error_event) + .await + } + + #[zbus(signal)] + async fn discovery_requested( + emitter: SignalEmitter<'_>, + session_handle: ObjectPath<'_>, + options: DiscoveryRequestedOptions, + ) -> zbus::Result<()>; + + #[zbus(signal)] + async fn client_pin_entered( + emitter: SignalEmitter<'_>, + session_handle: ObjectPath<'_>, + pin_fd: OwnedFd, + options: ClientPinEnteredOptions, + ) -> zbus::Result<()>; + + #[zbus(signal)] + async fn credential_selected( + emitter: SignalEmitter<'_>, + session_handle: ObjectPath<'_>, + id: String, + options: CredentialSelectedOptions, + ) -> zbus::Result<()>; +} + +impl CredentialPortalBackend { + async fn notify_state_changed( + &self, + object_server: &ObjectServer, + session_handle: ObjectPath<'_>, event: BackgroundEvent, ) -> fdo::Result<()> { let span = tracing::info_span!("NotifyStateChanged", %session_handle); @@ -178,29 +394,6 @@ impl CredentialPortalBackend { } } } - - #[zbus(signal)] - async fn discovery_requested( - emitter: SignalEmitter<'_>, - session_handle: ObjectPath<'_>, - options: DiscoveryRequestedOptions, - ) -> zbus::Result<()>; - - #[zbus(signal)] - async fn client_pin_entered( - emitter: SignalEmitter<'_>, - session_handle: ObjectPath<'_>, - pin_fd: OwnedFd, - options: ClientPinEnteredOptions, - ) -> zbus::Result<()>; - - #[zbus(signal)] - async fn credential_selected( - emitter: SignalEmitter<'_>, - session_handle: ObjectPath<'_>, - id: String, - options: CredentialSelectedOptions, - ) -> zbus::Result<()>; } struct CancelHandle { diff --git a/credentialsd/src/dbus/ui_control.rs b/credentialsd/src/dbus/ui_control.rs index 4bf7aef..a97ffe6 100644 --- a/credentialsd/src/dbus/ui_control.rs +++ b/credentialsd/src/dbus/ui_control.rs @@ -16,8 +16,16 @@ use zbus::{ }; use credentialsd_common::model::{ - BackgroundEvent, ClientPinEnteredOptions, CredentialSelectedOptions, Device, - DiscoveryRequestedOptions, Operation, PortalBackendOptions, UserInteractedEvent, WindowHandle, + BackgroundEvent, ClientPinEnteredOptions, Credential, CredentialSelectedOptions, Device, + DiscoveryRequestedOptions, NotifyHybridConnectedOptions, NotifyHybridConnectingOptions, + NotifyHybridStartedOptions, NotifyNeedsPinOptions, NotifyNeedsUserPresenceOptions, + NotifyNeedsUserVerificationOptions, NotifyNfcConnectedOptions, + NotifySelectingCredentialOptions, NotifyUsbConnectedOptions, Operation, PortalBackendOptions, + UserInteractedEvent, WindowHandle, BACKGROUND_EVENT_ERROR_AUTHENTICATOR, + BACKGROUND_EVENT_ERROR_CANCELLED, BACKGROUND_EVENT_ERROR_CREDENTIAL_EXCLUDED, + BACKGROUND_EVENT_ERROR_INTERNAL, BACKGROUND_EVENT_ERROR_NO_CREDENTIALS, + BACKGROUND_EVENT_ERROR_PIN_ATTEMPTS_EXHAUSTED, BACKGROUND_EVENT_ERROR_PIN_NOT_SET, + BACKGROUND_EVENT_ERROR_TIMED_OUT, }; /// Used by the credential service to control the UI. @@ -58,12 +66,97 @@ trait UiControlService { options: PortalBackendOptions, ) -> fdo::Result<()>; + #[zbus(no_reply)] async fn notify_state_changed( &self, session_handle: ObjectPath<'_>, event: BackgroundEvent, ) -> fdo::Result<()>; + #[zbus(no_reply)] + async fn notify_needs_pin( + &self, + session_handle: ObjectPath<'_>, + attempts_left: u32, + _options: NotifyNeedsPinOptions, + ) -> fdo::Result<()>; + + /// Emitted when the authenticator needs a user verification gesture. + #[zbus(no_reply)] + async fn notify_needs_user_verification( + &self, + session_handle: ObjectPath<'_>, + attempts_left: u32, + _options: NotifyNeedsUserVerificationOptions, + ) -> fdo::Result<()>; + + /// Emitted when the authenticator needs a user presence gesture. + #[zbus(no_reply)] + async fn notify_needs_user_presence( + &self, + session_handle: ObjectPath<'_>, + _options: NotifyNeedsUserPresenceOptions, + ) -> fdo::Result<()>; + + /// Emitted when the authenticator detects multiple credentials matching + /// credentials for a request. + #[zbus(no_reply)] + async fn notify_selecting_credential( + &self, + session_handle: ObjectPath<'_>, + credentials: Vec, + _options: NotifySelectingCredentialOptions, + ) -> fdo::Result<()>; + + /// Emitted when the platform begins scanning for CTAP2 hybrid advertisements. + #[zbus(no_reply)] + async fn notify_hybrid_started( + &self, + session_handle: ObjectPath<'_>, + invocation_data: OwnedFd, + _options: NotifyHybridStartedOptions, + ) -> fdo::Result<()>; + + /// Emitted when the platform has received a CTAP2 hybrid advertisement and is + /// establishing a channel. + #[zbus(no_reply)] + async fn notify_hybrid_connecting( + &self, + session_handle: ObjectPath<'_>, + _options: NotifyHybridConnectingOptions, + ) -> fdo::Result<()>; + + #[zbus(no_reply)] + async fn notify_hybrid_connected( + &self, + session_handle: ObjectPath<'_>, + _options: NotifyHybridConnectedOptions, + ) -> fdo::Result<()>; + + #[zbus(no_reply)] + async fn notify_nfc_connected( + &self, + session_handle: ObjectPath<'_>, + _options: NotifyNfcConnectedOptions, + ) -> fdo::Result<()>; + + #[zbus(no_reply)] + async fn notify_usb_connected( + &self, + session_handle: ObjectPath<'_>, + _options: NotifyUsbConnectedOptions, + ) -> fdo::Result<()>; + + #[zbus(no_reply)] + async fn notify_ceremony_completed(&self, session_handle: ObjectPath<'_>) -> fdo::Result<()>; + + #[zbus(no_reply)] + async fn notify_error_occurred( + &self, + session_handle: ObjectPath<'_>, + error: u32, + ) -> fdo::Result<()>; + #[zbus(signal)] async fn user_interacted( &self, @@ -108,17 +201,151 @@ impl Ceremony { } pub async fn send_state_update(&self, event: BackgroundEvent) -> Result<(), ()> { - if let Err(err) = self - .proxy - .notify_state_changed(self.session_handle.as_ref(), event) - .await - { - match err { - fdo::Error::UnknownObject(description) => { - tracing::error!(%description, "Flow D-Bus object no longer available at path"); - } - _ => tracing::error!(%err, "Failed to send update to backend"), + let response = match event { + // TODO: Remove these events. They are no longer needed by backends, since the user + // needs to select a device anyway. + BackgroundEvent::NfcIdle + | BackgroundEvent::NfcWaiting + | BackgroundEvent::UsbIdle + | BackgroundEvent::UsbSelectingDevice + | BackgroundEvent::UsbWaiting => { + return Ok(()); + } + BackgroundEvent::NeedsPin { attempts_left } => { + self.proxy + .notify_needs_pin( + self.session_handle.as_ref(), + attempts_left.unwrap_or(u32::MAX), + NotifyNeedsPinOptions {}, + ) + .await + } + BackgroundEvent::NeedsUserVerification { attempts_left } => { + self.proxy + .notify_needs_user_verification( + self.session_handle.as_ref(), + attempts_left.unwrap_or(u32::MAX), + NotifyNeedsUserVerificationOptions {}, + ) + .await + } + BackgroundEvent::NeedsUserPresence => { + self.proxy + .notify_needs_user_presence( + self.session_handle.as_ref(), + NotifyNeedsUserPresenceOptions {}, + ) + .await + } + BackgroundEvent::SelectingCredential { creds } => { + self.proxy + .notify_selecting_credential( + self.session_handle.as_ref(), + creds, + NotifySelectingCredentialOptions {}, + ) + .await } + BackgroundEvent::HybridIdle => todo!(), + BackgroundEvent::HybridStarted(invocation_data_fd) => { + self.proxy + .notify_hybrid_started( + self.session_handle.as_ref(), + invocation_data_fd, + NotifyHybridStartedOptions {}, + ) + .await + } + BackgroundEvent::HybridConnecting => { + self.proxy + .notify_hybrid_connecting( + self.session_handle.as_ref(), + NotifyHybridConnectingOptions {}, + ) + .await + } + BackgroundEvent::HybridConnected => { + self.proxy + .notify_hybrid_connected( + self.session_handle.as_ref(), + NotifyHybridConnectedOptions {}, + ) + .await + } + BackgroundEvent::NfcConnected => { + self.proxy + .notify_nfc_connected( + self.session_handle.as_ref(), + NotifyNfcConnectedOptions {}, + ) + .await + } + BackgroundEvent::UsbConnected => { + self.proxy + .notify_usb_connected( + self.session_handle.as_ref(), + NotifyUsbConnectedOptions {}, + ) + .await + } + BackgroundEvent::ErrorInternal => { + let error = BACKGROUND_EVENT_ERROR_INTERNAL; + self.proxy + .notify_error_occurred(self.session_handle.as_ref(), error) + .await + } + BackgroundEvent::ErrorTimedOut => { + let error = BACKGROUND_EVENT_ERROR_TIMED_OUT; + self.proxy + .notify_error_occurred(self.session_handle.as_ref(), error) + .await + } + BackgroundEvent::ErrorCancelled => { + // TODO: Just call org.freedesktop.impl.portal.Session.Close() + let error = BACKGROUND_EVENT_ERROR_CANCELLED; + self.proxy + .notify_error_occurred(self.session_handle.as_ref(), error) + .await + } + BackgroundEvent::ErrorAuthenticator => { + let error = BACKGROUND_EVENT_ERROR_AUTHENTICATOR; + self.proxy + .notify_error_occurred(self.session_handle.as_ref(), error) + .await + } + BackgroundEvent::ErrorNoCredentials => { + let error = BACKGROUND_EVENT_ERROR_NO_CREDENTIALS; + self.proxy + .notify_error_occurred(self.session_handle.as_ref(), error) + .await + } + BackgroundEvent::ErrorCredentialExcluded => { + let error = BACKGROUND_EVENT_ERROR_CREDENTIAL_EXCLUDED; + self.proxy + .notify_error_occurred(self.session_handle.as_ref(), error) + .await + } + BackgroundEvent::ErrorPinAttemptsExhausted => { + let error = BACKGROUND_EVENT_ERROR_PIN_ATTEMPTS_EXHAUSTED; + self.proxy + .notify_error_occurred(self.session_handle.as_ref(), error) + .await + } + BackgroundEvent::ErrorPinNotSet => { + let error = BACKGROUND_EVENT_ERROR_PIN_NOT_SET; + self.proxy + .notify_error_occurred(self.session_handle.as_ref(), error) + .await + } + BackgroundEvent::CeremonyCompleted => { + self.proxy + .notify_ceremony_completed(self.session_handle.as_ref()) + .await + } + }; + + if let Err(err) = response { + tracing::error!(%err, "Failed to send update to backend"); return Err(()); } Ok(()) From a2099e4ee8a3434d7b57bdcb9a3172a064fb3c26 Mon Sep 17 00:00:00 2001 From: Isaiah Inuwa Date: Mon, 20 Jul 2026 14:13:26 -0500 Subject: [PATCH 6/6] common: Delete unused code background and UI event types --- credentialsd-common/src/model.rs | 457 +--------------------------- credentialsd/src/dbus/ui_control.rs | 14 - 2 files changed, 3 insertions(+), 468 deletions(-) diff --git a/credentialsd-common/src/model.rs b/credentialsd-common/src/model.rs index dd94ef6..4989575 100644 --- a/credentialsd-common/src/model.rs +++ b/credentialsd-common/src/model.rs @@ -1,41 +1,8 @@ use std::fmt::Display; -use serde::{ - Deserialize, Serialize, - de::{DeserializeSeed, Error as _, Visitor}, -}; - -use zvariant::{ - self, Array, DeserializeDict, DynamicDeserialize, Fd, NoneValue, Optional, OwnedFd, OwnedValue, - SerializeDict, Signature, Str, Structure, StructureBuilder, Type, Value, signature::Fields, -}; - -const TAG_VALUE_SIGNATURE: &Signature = &Signature::Structure(Fields::Static { - fields: &[&Signature::U32, &Signature::Variant], -}); - -/// Ceremony completed successfully -const BACKGROUND_EVENT_CEREMONY_COMPLETED: u32 = 0x01; -/// Device needs the client PIN to be entered. The backend should collect the -/// PIN and send it back with `EnterClientPin` event of `UserInteracted` signal. -const BACKGROUND_EVENT_NEEDS_PIN: u32 = 0x10; -const BACKGROUND_EVENT_NEEDS_USER_VERIFICATION: u32 = 0x11; -const BACKGROUND_EVENT_NEEDS_USER_PRESENCE: u32 = 0x12; -const BACKGROUND_EVENT_SELECTING_CREDENTIAL: u32 = 0x13; - -const BACKGROUND_EVENT_HYBRID_IDLE: u32 = 0x20; -const BACKGROUND_EVENT_HYBRID_STARTED: u32 = 0x21; -const BACKGROUND_EVENT_HYBRID_CONNECTING: u32 = 0x22; -const BACKGROUND_EVENT_HYBRID_CONNECTED: u32 = 0x23; - -const BACKGROUND_EVENT_NFC_IDLE: u32 = 0x30; -const BACKGROUND_EVENT_NFC_WAITING: u32 = 0x31; -const BACKGROUND_EVENT_NFC_CONNECTED: u32 = 0x32; - -const BACKGROUND_EVENT_USB_IDLE: u32 = 0x40; -const BACKGROUND_EVENT_USB_WAITING: u32 = 0x41; -const BACKGROUND_EVENT_USB_SELECTING_DEVICE: u32 = 0x42; -const BACKGROUND_EVENT_USB_CONNECTED: u32 = 0x43; +use serde::{Deserialize, Serialize, de::Visitor}; + +use zvariant::{self, DeserializeDict, NoneValue, Optional, OwnedFd, SerializeDict, Type, Value}; pub const BACKGROUND_EVENT_ERROR_INTERNAL: u32 = 0x80000001; pub const BACKGROUND_EVENT_ERROR_TIMED_OUT: u32 = 0x80000002; @@ -46,11 +13,6 @@ pub const BACKGROUND_EVENT_ERROR_CREDENTIAL_EXCLUDED: u32 = 0x80000006; pub const BACKGROUND_EVENT_ERROR_PIN_ATTEMPTS_EXHAUSTED: u32 = 0x80000007; pub const BACKGROUND_EVENT_ERROR_PIN_NOT_SET: u32 = 0x80000008; -const USER_INTERACTED_EVENT_DISCOVERY_REQUESTED: u32 = 0x01; -const USER_INTERACTED_EVENT_CLIENT_PIN_ENTERED: u32 = 0x04; -const USER_INTERACTED_EVENT_CREDENTIAL_SELECTED: u32 = 0x05; -const USER_INTERACTED_EVENT_REQUEST_CANCELLED: u32 = 0x06; - /// Credential service events intended to inform the UI. #[derive(Debug, PartialEq)] pub enum BackgroundEvent { @@ -84,190 +46,6 @@ pub enum BackgroundEvent { ErrorPinNotSet, } -impl BackgroundEvent { - fn tag(&self) -> u32 { - match self { - Self::CeremonyCompleted => BACKGROUND_EVENT_CEREMONY_COMPLETED, - Self::NeedsPin { .. } => BACKGROUND_EVENT_NEEDS_PIN, - Self::NeedsUserVerification { .. } => BACKGROUND_EVENT_NEEDS_USER_VERIFICATION, - Self::NeedsUserPresence => BACKGROUND_EVENT_NEEDS_USER_PRESENCE, - Self::SelectingCredential { .. } => BACKGROUND_EVENT_SELECTING_CREDENTIAL, - - Self::HybridIdle => BACKGROUND_EVENT_HYBRID_IDLE, - Self::HybridStarted(_) => BACKGROUND_EVENT_HYBRID_STARTED, - Self::HybridConnecting => BACKGROUND_EVENT_HYBRID_CONNECTING, - Self::HybridConnected => BACKGROUND_EVENT_HYBRID_CONNECTED, - - Self::NfcIdle => BACKGROUND_EVENT_NFC_IDLE, - Self::NfcWaiting => BACKGROUND_EVENT_NFC_WAITING, - Self::NfcConnected => BACKGROUND_EVENT_NFC_CONNECTED, - - Self::UsbIdle => BACKGROUND_EVENT_USB_IDLE, - Self::UsbWaiting => BACKGROUND_EVENT_USB_WAITING, - Self::UsbSelectingDevice => BACKGROUND_EVENT_USB_SELECTING_DEVICE, - Self::UsbConnected => BACKGROUND_EVENT_USB_CONNECTED, - - Self::ErrorInternal => BACKGROUND_EVENT_ERROR_INTERNAL, - Self::ErrorTimedOut => BACKGROUND_EVENT_ERROR_TIMED_OUT, - Self::ErrorCancelled => BACKGROUND_EVENT_ERROR_CANCELLED, - Self::ErrorAuthenticator => BACKGROUND_EVENT_ERROR_AUTHENTICATOR, - Self::ErrorNoCredentials => BACKGROUND_EVENT_ERROR_NO_CREDENTIALS, - Self::ErrorCredentialExcluded => BACKGROUND_EVENT_ERROR_CREDENTIAL_EXCLUDED, - Self::ErrorPinAttemptsExhausted => BACKGROUND_EVENT_ERROR_PIN_ATTEMPTS_EXHAUSTED, - Self::ErrorPinNotSet => BACKGROUND_EVENT_ERROR_PIN_NOT_SET, - } - } -} - -impl Type for BackgroundEvent { - const SIGNATURE: &'static Signature = TAG_VALUE_SIGNATURE; -} - -impl From<&BackgroundEvent> for Structure<'_> { - fn from(value: &BackgroundEvent) -> Self { - let tag = value.tag(); - let payload = match value { - // States with payloads - BackgroundEvent::NeedsPin { attempts_left } => { - Some(Value::U32(attempts_left.map(u32::from).unwrap_or(u32::MAX))) - } - BackgroundEvent::NeedsUserVerification { attempts_left } => { - Some(Value::U32(attempts_left.map(u32::from).unwrap_or(u32::MAX))) - } - BackgroundEvent::SelectingCredential { creds } => Some(Value::Array(creds.into())), - BackgroundEvent::HybridStarted(qr_data_fd) => Some(Value::Fd(qr_data_fd.into())), - // Empty - BackgroundEvent::CeremonyCompleted => None, - BackgroundEvent::NeedsUserPresence => None, - BackgroundEvent::HybridIdle => None, - BackgroundEvent::HybridConnecting => None, - BackgroundEvent::HybridConnected => None, - BackgroundEvent::NfcIdle => None, - BackgroundEvent::NfcWaiting => None, - BackgroundEvent::NfcConnected => None, - BackgroundEvent::UsbIdle => None, - BackgroundEvent::UsbWaiting => None, - BackgroundEvent::UsbSelectingDevice => None, - BackgroundEvent::UsbConnected => None, - BackgroundEvent::ErrorInternal => None, - BackgroundEvent::ErrorTimedOut => None, - BackgroundEvent::ErrorCancelled => None, - BackgroundEvent::ErrorAuthenticator => None, - BackgroundEvent::ErrorNoCredentials => None, - BackgroundEvent::ErrorCredentialExcluded => None, - BackgroundEvent::ErrorPinAttemptsExhausted => None, - BackgroundEvent::ErrorPinNotSet => None, - }; - tag_value_to_struct(tag, payload) - } -} - -impl TryFrom<&Structure<'_>> for BackgroundEvent { - type Error = zvariant::Error; - - fn try_from(value: &Structure<'_>) -> Result { - let (tag, value) = parse_tag_value_struct(value)?; - - match tag { - BACKGROUND_EVENT_CEREMONY_COMPLETED => Ok(Self::CeremonyCompleted), - BACKGROUND_EVENT_NEEDS_PIN => value.downcast::().map(|attempts_left| { - if attempts_left == u32::MAX { - Self::NeedsPin { - attempts_left: None, - } - } else { - Self::NeedsPin { - attempts_left: Some(attempts_left), - } - } - }), - BACKGROUND_EVENT_NEEDS_USER_VERIFICATION => { - value.downcast::().map(|attempts_left| { - if attempts_left == u32::MAX { - Self::NeedsUserVerification { - attempts_left: None, - } - } else { - Self::NeedsUserVerification { - attempts_left: Some(attempts_left), - } - } - }) - } - BACKGROUND_EVENT_NEEDS_USER_PRESENCE => Ok(Self::NeedsUserPresence), - BACKGROUND_EVENT_SELECTING_CREDENTIAL => { - let creds: Array = value.downcast_ref()?; - let creds: Result, zvariant::Error> = creds - .iter() - .map(|v| v.try_to_owned().unwrap()) - .map(|v| { - let cred: Result = - Value::from(v).downcast::(); - cred - }) - .collect(); - Ok(Self::SelectingCredential { creds: creds? }) - } - - BACKGROUND_EVENT_HYBRID_IDLE => Ok(Self::HybridIdle), - BACKGROUND_EVENT_HYBRID_STARTED => { - let qr_data_fd = value.downcast_ref::()?.try_to_owned()?; - Ok(Self::HybridStarted(qr_data_fd.into())) - } - BACKGROUND_EVENT_HYBRID_CONNECTING => Ok(Self::HybridConnecting), - BACKGROUND_EVENT_HYBRID_CONNECTED => Ok(Self::HybridConnected), - - BACKGROUND_EVENT_NFC_IDLE => Ok(Self::NfcIdle), - BACKGROUND_EVENT_NFC_WAITING => Ok(Self::NfcWaiting), - BACKGROUND_EVENT_NFC_CONNECTED => Ok(Self::NfcConnected), - - BACKGROUND_EVENT_USB_IDLE => Ok(Self::UsbIdle), - BACKGROUND_EVENT_USB_WAITING => Ok(Self::UsbWaiting), - BACKGROUND_EVENT_USB_SELECTING_DEVICE => Ok(Self::UsbSelectingDevice), - BACKGROUND_EVENT_USB_CONNECTED => Ok(Self::UsbConnected), - - BACKGROUND_EVENT_ERROR_AUTHENTICATOR => Ok(Self::ErrorAuthenticator), - BACKGROUND_EVENT_ERROR_NO_CREDENTIALS => Ok(Self::ErrorNoCredentials), - BACKGROUND_EVENT_ERROR_PIN_ATTEMPTS_EXHAUSTED => Ok(Self::ErrorPinAttemptsExhausted), - BACKGROUND_EVENT_ERROR_INTERNAL => Ok(Self::ErrorInternal), - BACKGROUND_EVENT_ERROR_TIMED_OUT => Ok(Self::ErrorTimedOut), - BACKGROUND_EVENT_ERROR_CANCELLED => Ok(Self::ErrorCancelled), - _ => Err(zvariant::Error::Message(format!( - "Unknown BackgroundEvent tag : {tag}" - ))), - } - } -} - -impl Serialize for BackgroundEvent { - fn serialize(&self, serializer: S) -> Result - where - S: serde::Serializer, - { - let structure: Structure = self.into(); - structure.serialize(serializer) - } -} - -impl<'de> Deserialize<'de> for BackgroundEvent { - fn deserialize(deserializer: D) -> Result - where - D: serde::Deserializer<'de>, - { - let d = Structure::deserializer_for_signature(TAG_VALUE_SIGNATURE).map_err(|err| { - D::Error::custom(format!( - "could not create deserializer for tag-value struct: {err}" - )) - })?; - let structure = d.deserialize(deserializer)?; - (&structure).try_into().map_err(|err| { - D::Error::custom(format!( - "could not deserialize structure into BackgroundEvent: {err}" - )) - }) - } -} - /// Emitted when a client enters a PIN for the selected authenticator. #[derive(Debug, SerializeDict, DeserializeDict, PartialEq, Type)] #[zvariant(signature = "dict")] @@ -498,97 +276,6 @@ impl std::fmt::Debug for UserInteractedEvent { } } -impl Type for UserInteractedEvent { - const SIGNATURE: &'static Signature = TAG_VALUE_SIGNATURE; -} - -impl From<&UserInteractedEvent> for Structure<'_> { - fn from(value: &UserInteractedEvent) -> Self { - match value { - UserInteractedEvent::DiscoveryRequested => { - tag_value_to_struct(USER_INTERACTED_EVENT_DISCOVERY_REQUESTED, None) - } - UserInteractedEvent::ClientPinEntered(pin_fd) => tag_value_to_struct( - USER_INTERACTED_EVENT_CLIENT_PIN_ENTERED, - Some(Value::Fd(pin_fd.into())), - ), - UserInteractedEvent::CredentialSelected(credential_id) => tag_value_to_struct( - USER_INTERACTED_EVENT_CREDENTIAL_SELECTED, - Some(Value::Str(credential_id.into())), - ), - UserInteractedEvent::RequestCancelled => { - tag_value_to_struct(USER_INTERACTED_EVENT_REQUEST_CANCELLED, None) - } - } - } -} - -impl TryFrom<&Structure<'_>> for UserInteractedEvent { - type Error = zvariant::Error; - - fn try_from(value: &Structure<'_>) -> Result { - let (tag, value) = parse_tag_value_struct(value)?; - - match tag { - USER_INTERACTED_EVENT_DISCOVERY_REQUESTED => { - Ok(UserInteractedEvent::DiscoveryRequested) - } - USER_INTERACTED_EVENT_CLIENT_PIN_ENTERED => { - let fd = value.downcast_ref::()?; - let owned_fd = fd.try_to_owned()?; - Ok(UserInteractedEvent::ClientPinEntered(owned_fd.into())) - } - USER_INTERACTED_EVENT_CREDENTIAL_SELECTED => { - let s: Str = value.downcast_ref()?; - if s.is_empty() { - return Err(zvariant::Error::invalid_length( - s.len(), - &"a non-empty string", - )); - } - Ok(UserInteractedEvent::CredentialSelected( - s.as_str().to_string(), - )) - } - USER_INTERACTED_EVENT_REQUEST_CANCELLED => Ok(UserInteractedEvent::RequestCancelled), - _ => Err(zvariant::Error::Message(format!( - "Unknown {} tag : {tag}", - stringify!(UserInteractedEvent) - ))), - } - } -} - -impl Serialize for UserInteractedEvent { - fn serialize(&self, serializer: S) -> Result - where - S: serde::Serializer, - { - let structure: Structure = self.into(); - structure.serialize(serializer) - } -} - -impl<'de> Deserialize<'de> for UserInteractedEvent { - fn deserialize(deserializer: D) -> Result - where - D: serde::Deserializer<'de>, - { - let d = Structure::deserializer_for_signature(TAG_VALUE_SIGNATURE).map_err(|err| { - D::Error::custom(format!( - "could not create deserializer for tag-value struct: {err}" - )) - })?; - let structure = d.deserialize(deserializer)?; - (&structure).try_into().map_err(|err| { - D::Error::custom(format!( - "could not deserialize structure into {}: {err}", - stringify!(UserInteractedEvent) - )) - }) - } -} - #[derive(Clone, Debug, PartialEq, Type)] #[zvariant(signature = "s")] pub enum WindowHandle { @@ -671,141 +358,3 @@ impl Display for WindowHandle { } } } - -fn value_to_owned(value: &Value<'_>) -> OwnedValue { - value - .try_to_owned() - .expect("non-file descriptor values to succeed") -} - -fn parse_tag_value_struct<'a>(s: &'a Structure) -> Result<(u32, Value<'a>), zvariant::Error> { - if s.signature() != TAG_VALUE_SIGNATURE { - return Err(zvariant::Error::SignatureMismatch( - s.signature().clone(), - TAG_VALUE_SIGNATURE.to_string(), - )); - } - let tag: u32 = s - .fields() - .first() - .ok_or_else(|| { - zvariant::Error::SignatureMismatch(Signature::U32, "expected a u32 tag".to_string()) - }) - .and_then(|f| f.downcast_ref())?; - let value = s - .fields() - .get(1) - .ok_or_else(|| { - zvariant::Error::SignatureMismatch( - Signature::Variant, - "expected a variant value".to_string(), - ) - })? - .clone(); - Ok((tag, value)) -} - -fn tag_value_to_struct(tag: u32, value: Option>) -> Structure<'static> { - StructureBuilder::new() - .add_field(tag) - .append_field(Value::new(value_to_owned( - &value.unwrap_or_else(|| Value::U8(0)), - ))) - .build() - .expect("create a struct") -} - -#[cfg(test)] -mod test { - use std::os::fd::{FromRawFd, OwnedFd}; - - use zvariant::Type; - - use super::{BackgroundEvent, Credential}; - - #[test] - fn test_round_trip_completed_event() { - let event1 = BackgroundEvent::CeremonyCompleted; - let ctx = zvariant::serialized::Context::new_dbus(zvariant::BE, 0); - let data = zvariant::to_bytes(ctx, &event1).unwrap(); - assert_eq!("(uv)", BackgroundEvent::SIGNATURE.to_string()); - assert_eq!(&[0, 0, 0, 1, 1, b'y', 0, 0], data.bytes()); - let event2 = data.deserialize().unwrap().0; - assert_eq!(event1, event2); - } - - #[test] - fn test_round_trip_background_hybrid_event() { - let mut fds = [0; 2]; - unsafe { - libc::pipe(fds.as_mut_ptr()); - } - // Wrap the raw fds into safe OwnedFd instances - let mock_fd = unsafe { OwnedFd::from_raw_fd(fds[0]) }; - let _mock_fd2 = unsafe { OwnedFd::from_raw_fd(fds[1]) }; - - println!("mock_fd: {mock_fd:?}"); - let event1 = BackgroundEvent::HybridStarted(mock_fd.into()); - let ctx = zvariant::serialized::Context::new_dbus(zvariant::BE, 0); - assert_eq!("(uv)", BackgroundEvent::SIGNATURE.to_string()); - let data = zvariant::to_bytes(ctx, &event1).unwrap(); - println!("data: {data:?}"); - // handle value is 0_u32 because it's the index into the list of fds in the message. Since - // there's only one, it will be 0. - let expected = b"\x00\x00\x00\x21\x01h\0\0\0\0\0\0"; - assert_eq!(expected, data.bytes()); - let event2 = data.deserialize().unwrap().0; - // I believe that the fd is `dup()`'d through the serialization/deserialization process, so - // we can't compare the numbers for equality. - assert!(matches!(event2, BackgroundEvent::HybridStarted(_))); - } - - #[test] - fn test_round_trip_selecting_credential_state() { - let creds = vec![ - Credential { - id: "a1b2c3".to_string(), - name: "user 1".to_string(), - username: Some("u1@example.com".to_string()), - }, - Credential { - id: "321".to_string(), - name: "User 2".to_string(), - username: None, - }, - ]; - let event1 = BackgroundEvent::SelectingCredential { creds }; - let ctx = zvariant::serialized::Context::new_dbus(zvariant::BE, 0); - let data = zvariant::to_bytes(ctx, &event1).unwrap(); - assert_eq!("(uv)", BackgroundEvent::SIGNATURE.to_string()); - - #[rustfmt::skip] - let expected = [ - 0, 0, 0, 0x13, // BACKGROUND_EVENT_SELECTING_CREDENTIAL - 6, b'a', b'a', b'{', b's', b'v', b'}', 0, // Signature aa{sv} + padding(1) - 0, 0, 0, 143, // array(struct) data length - 0, 0, 0, 83, 0, 0, 0, 0, // element 1(struct) length, + padding(4) - 0, 0, 0, 2, 105, 100, 0, // string[2] "id" - 1, 115, 0, 0, 0, // Signature s + padding - 0, 0, 0, 6, 97, 49, 98, 50, 99, 51, 0, 0, // String, len 6, "a1b2c3" + padding(1) - 0, 0, 0, 4, 110, 97, 109, 101, 0, // String, len 4, "name" - 1, 115, 0, // Signature s + padding - 0, 0, 0, 6, 117, 115, 101, 114, 32, 49, 0, 0, // String, len 6, "user 1" + padding(1) - 0, 0, 0, 8, 117, 115, 101, 114, 110, 97, 109, 101, 0, // String, len 8, "username" - 1, 115, 0, // Signature s - 0, 0, 0, 14, 117, 49, 64, 101, 120, 97, 109, 112, 108, 101, 46, 99, 111, 109, 0, 0, // String, len 14, "u1@example.com" + padding(1) - - 0, 0, 0, 47, // element 2, length 69 - 0, 0, 0, 2, 105, 100, 0, // string, len 2, "id" - 1, 115, 0, 0, 0, // Signature s + padding(2) - 0, 0, 0, 3, 51, 50, 49, 0, 0, 0, 0, 0, // string, len 3, "321" + padding(4) - 0, 0, 0, 4, 110, 97, 109, 101, 0, // String, len 4, "name" - 1, 115, 0, // Signature s - 0, 0, 0, 6, 85, 115, 101, 114, 32, 50, 0, // String, len 6, "User 2" + padding(1) - // username omitted - ]; - assert_eq!(expected, data.bytes()); - let event2: BackgroundEvent = data.deserialize().unwrap().0; - assert_eq!(event1, event2); - } -} diff --git a/credentialsd/src/dbus/ui_control.rs b/credentialsd/src/dbus/ui_control.rs index a97ffe6..29bec4e 100644 --- a/credentialsd/src/dbus/ui_control.rs +++ b/credentialsd/src/dbus/ui_control.rs @@ -66,13 +66,6 @@ trait UiControlService { options: PortalBackendOptions, ) -> fdo::Result<()>; - #[zbus(no_reply)] - async fn notify_state_changed( - &self, - session_handle: ObjectPath<'_>, - event: BackgroundEvent, - ) -> fdo::Result<()>; - #[zbus(no_reply)] async fn notify_needs_pin( &self, @@ -157,13 +150,6 @@ trait UiControlService { error: u32, ) -> fdo::Result<()>; - #[zbus(signal)] - async fn user_interacted( - &self, - session_handle: ObjectPath<'_>, - update: UserInteractedEvent, - ) -> zbus::Result<()>; - #[zbus(signal)] async fn discovery_requested( &self,