daemon: Allow clients to cancel requests

ARCHITECTURE.md

@@ -102,8 +102,10 @@ method to start the process that hosts the UI Control API.
 
 Credential requests on the Gateway are handled one at a time; if a new request
 comes in, it is immediately rejected, and the client is expected to retry if
-necessary. Currently, clients cannot cancel their own requests; the user has to
-do that via the UI.
+necessary. Clients can cancel their own requests by passing a `request_handle`
+option: the Gateway exports an `org.freedesktop.impl.portal.Request` object at
+that path for the duration of the request, and calling `Close()` on it cancels
+the request and dismisses the UI. Otherwise, the user has to cancel via the UI.
 
 ### `credentialsd/src/credential_service/`
 

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## Improvements
 
 - daemon: Cancel ceremony if UI disappears (Thank you, @michaelbeaumont!)
+- daemon: Allow clients to cancel requests through the `request_handle` option.
 - daemon: Fix UI service failing to launch when started manually instead of via D-Bus activation.
 - daemon: Allow Firefox to contact daemon on more distros with different app IDs (Thank you, @michaelbeaumont!)
 - daemon: Fix all subsequent requests being rejected after the UI failed to launch.

credentialsd/src/dbus/flow_control.rs

@@ -17,6 +17,7 @@ use tokio::sync::mpsc::Receiver;
 use tokio::sync::oneshot;
 use tokio::sync::{Mutex as AsyncMutex, mpsc::Sender};
 use tokio::task::AbortHandle;
+use tokio_util::sync::CancellationToken;
 use zbus::connection::Connection;
 use zbus::zvariant::OwnedObjectPath;
 
@@ -27,7 +28,9 @@ use crate::{
     model::{CredentialRequest, CredentialResponse},
 };
 use crate::{
-    credential_service::{CredentialServiceError, DeviceStateUpdate, ManageDevice, nfc::NfcState},
+    credential_service::{
+        CredentialServiceError, DeviceStateUpdate, ManageDevice, RequestId, nfc::NfcState,
+    },
     model::ClientDetails,
 };
 use crate::{dbus::ui_control::Ceremony, gateway::WebAuthnError};
@@ -38,6 +41,8 @@ pub struct UiRequestContext {
     /// Client window handle
     window_handle: Option<WindowHandle>,
     activation_token: Option<String>,
+    /// Cancelled when the client closes the request before it completes.
+    client_cancellation: CancellationToken,
     response_channel: oneshot::Sender<Result<CredentialResponse, CredentialServiceError>>,
 }
 
@@ -60,6 +65,7 @@ pub async fn start_flow_control_service<M: ManageDevice + Debug + Send + Sync +
                 app,
                 window_handle,
                 activation_token,
+                client_cancellation,
                 response_channel,
             } = ui_request_ctx;
 
@@ -70,6 +76,7 @@ pub async fn start_flow_control_service<M: ManageDevice + Debug + Send + Sync +
                 app,
                 window_handle,
                 activation_token,
+                client_cancellation,
             )
             .await;
 
@@ -90,7 +97,13 @@ async fn handle<M: ManageDevice + Debug + Send + Sync + 'static, UC: UiControlle
     requesting_app: ClientDetails,
     window_handle: Option<WindowHandle>,
     activation_token: Option<String>,
+    client_cancellation: CancellationToken,
 ) -> Result<CredentialResponse, CredentialServiceError> {
+    if client_cancellation.is_cancelled() {
+        return Err(CredentialServiceError::Internal(
+            "Request cancelled by client before it started.".to_string(),
+        ));
+    }
     let (request_tx, request_rx) = oneshot::channel();
     let (request_id, cancellation_token) = svc.lock().await.init_request(&msg, request_tx).await?;
     let operation = msg.operation();
@@ -144,6 +157,26 @@ async fn handle<M: ManageDevice + Debug + Send + Sync + 'static, UC: UiControlle
             return Err(CredentialServiceError::Internal(err));
         }
     };
+    {
+        let svc = svc.clone();
+        let flow = flow.clone();
+        let request_cancellation = cancellation_token.clone();
+        tokio::spawn(async move {
+            if cancel_on_client_request(
+                &svc,
+                request_id,
+                &request_cancellation,
+                &client_cancellation,
+            )
+            .await
+            {
+                // Dismiss the UI.
+                _ = flow
+                    .send_state_update(BackgroundEvent::ErrorCancelled)
+                    .await;
+            }
+        });
+    }
     tokio::spawn(async move {
         let client_pin_tx: Arc<Mutex<Option<Sender<String>>>> = Arc::new(Mutex::new(None));
         let set_pin_tx: Arc<Mutex<Option<Sender<String>>>> = Arc::new(Mutex::new(None));
@@ -297,6 +330,25 @@ async fn handle<M: ManageDevice + Debug + Send + Sync + 'static, UC: UiControlle
         .expect("Credential service not to drop request channel before responding.")
 }
 
+/// Waits until either the request ends or the client cancels it. In the
+/// latter case, the request is cancelled and `true` is returned.
+async fn cancel_on_client_request<M: ManageDevice>(
+    svc: &AsyncMutex<M>,
+    request_id: RequestId,
+    request_cancellation: &CancellationToken,
+    client_cancellation: &CancellationToken,
+) -> bool {
+    tokio::select! {
+        biased;
+        _ = request_cancellation.cancelled() => false,
+        _ = client_cancellation.cancelled() => {
+            tracing::debug!(%request_id, "Client closed the request, cancelling");
+            svc.lock().await.cancel_request(request_id).await;
+            true
+        }
+    }
+}
+
 fn forward_background_event_stream(
     flow: Ceremony,
     mut stream: impl Stream<Item = BackgroundEvent> + Send + Unpin + 'static,
@@ -323,6 +375,7 @@ pub trait CredentialRequestController {
         request: CredentialRequest,
         window_handle: Option<WindowHandle>,
         activation_token: Option<String>,
+        cancellation: CancellationToken,
     ) -> Result<CredentialResponse, WebAuthnError>;
 }
 
@@ -338,6 +391,7 @@ impl CredentialRequestController for CredentialRequestControllerClient {
         request: CredentialRequest,
         window_handle: Option<WindowHandle>,
         activation_token: Option<String>,
+        client_cancellation: CancellationToken,
     ) -> Result<CredentialResponse, WebAuthnError> {
         let (tx, rx) = oneshot::channel();
         self.initiator
@@ -346,6 +400,7 @@ impl CredentialRequestController for CredentialRequestControllerClient {
                 app,
                 window_handle,
                 activation_token,
+                client_cancellation,
                 response_channel: tx,
             })
             .await
@@ -367,10 +422,8 @@ mod tests {
     use std::pin::Pin;
 
     use credentialsd_common::model::{Device, Operation};
-    use tokio_util::sync::CancellationToken;
 
     use super::*;
-    use crate::credential_service::RequestId;
 
     const TEST_REQUEST_ID: RequestId = 42;
 
@@ -466,6 +519,7 @@ mod tests {
             app,
             None,
             None,
+            CancellationToken::new(),
         )
         .await;
 
@@ -475,4 +529,72 @@ mod tests {
             vec![TEST_REQUEST_ID]
         );
     }
+
+    #[tokio::test]
+    async fn test_client_cancellation_cancels_request() {
+        let svc = AsyncMutex::new(MockDeviceManager::default());
+        let request_cancellation = CancellationToken::new();
+        let client_cancellation = CancellationToken::new();
+        client_cancellation.cancel();
+
+        let cancelled = cancel_on_client_request(
+            &svc,
+            TEST_REQUEST_ID,
+            &request_cancellation,
+            &client_cancellation,
+        )
+        .await;
+
+        assert!(cancelled);
+        assert_eq!(
+            *svc.lock().await.cancelled.lock().unwrap(),
+            vec![TEST_REQUEST_ID]
+        );
+    }
+
+    #[tokio::test]
+    async fn test_finished_request_is_not_cancelled_by_client() {
+        let svc = AsyncMutex::new(MockDeviceManager::default());
+        let request_cancellation = CancellationToken::new();
+        let client_cancellation = CancellationToken::new();
+        request_cancellation.cancel();
+        client_cancellation.cancel();
+
+        let cancelled = cancel_on_client_request(
+            &svc,
+            TEST_REQUEST_ID,
+            &request_cancellation,
+            &client_cancellation,
+        )
+        .await;
+
+        assert!(!cancelled);
+        assert!(svc.lock().await.cancelled.lock().unwrap().is_empty());
+    }
+
+    #[tokio::test]
+    async fn test_cancelled_client_request_does_not_start() {
+        let svc = Arc::new(AsyncMutex::new(MockDeviceManager::default()));
+        let app = ClientDetails {
+            app_id: "org.example.App".to_string(),
+            pid: 1,
+        };
+        let client_cancellation = CancellationToken::new();
+        client_cancellation.cancel();
+
+        let result = handle(
+            svc.clone(),
+            FailingUiController,
+            create_test_request().await,
+            app,
+            None,
+            None,
+            client_cancellation,
+        )
+        .await;
+
+        assert!(result.is_err());
+        // The request was never initialized, so there is nothing to cancel.
+        assert!(svc.lock().await.cancelled.lock().unwrap().is_empty());
+    }
 }