Skip to content

Commit f88eb0c

Browse files
authored
Merge pull request #68 from linux-credentials/user-cancellation
Signals to the cred service that the UI is closing when: - the close button is clicked - the keyboard shortcut (Ctrl+Q) is pressed - the window manager closes the window Cancellation should be idempotent, so cancellations for the same request ID should not cause any problems
2 parents 0ee4074 + 6299f84 commit f88eb0c

File tree

16 files changed

+184
-49
lines changed

16 files changed

+184
-49
lines changed

.vscode/launch.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
"args": [],
2626
"env": {
2727
"GSETTINGS_SCHEMA_DIR": "${workspaceFolder}/build/credentialsd-ui/data",
28-
"RUST_LOG": "creds_ui=debug,zbus::trace,zbus::object_server::debug"
28+
"RUST_LOG": "credentialsd_ui=debug,zbus::trace,zbus::object_server::debug"
2929
},
3030
"sourceLanguages": ["rust"],
3131
"cwd": "${workspaceFolder}",

credentialsd-common/src/client.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@ use std::pin::Pin;
22

33
use futures_lite::Stream;
44

5-
use crate::model::{BackgroundEvent, Device};
5+
use crate::{
6+
model::{BackgroundEvent, Device},
7+
server::RequestId,
8+
};
69

710
/// Used for communication from trusted UI to credential service
811
pub trait FlowController {
@@ -22,4 +25,5 @@ pub trait FlowController {
2225
&self,
2326
credential_id: String,
2427
) -> impl Future<Output = Result<(), ()>> + Send;
28+
fn cancel_request(&self, request_id: RequestId) -> impl Future<Output = Result<(), ()>> + Send;
2529
}

credentialsd-common/src/model.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,7 @@ pub enum ViewUpdate {
188188
HybridConnected,
189189

190190
Completed,
191+
Cancelled,
191192
Failed(String),
192193
}
193194

credentialsd-common/src/server.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,9 @@ impl TryFrom<BackgroundEvent> for crate::model::BackgroundEvent {
3333
}
3434
}
3535

36-
impl TryFrom<crate::model::BackgroundEvent> for BackgroundEvent {
37-
type Error = zvariant::Error;
38-
fn try_from(value: crate::model::BackgroundEvent) -> Result<Self, Self::Error> {
39-
let event = match value {
36+
impl From<crate::model::BackgroundEvent> for BackgroundEvent {
37+
fn from(value: crate::model::BackgroundEvent) -> Self {
38+
match value {
4039
crate::model::BackgroundEvent::HybridQrStateChanged(state) => {
4140
let state: HybridState = state.into();
4241
let value = Value::new(state)
@@ -52,8 +51,7 @@ impl TryFrom<crate::model::BackgroundEvent> for BackgroundEvent {
5251

5352
BackgroundEvent::UsbStateChanged(value)
5453
}
55-
};
56-
Ok(event)
54+
}
5755
}
5856
}
5957

@@ -327,6 +325,9 @@ impl From<HybridState> for Value<'_> {
327325
}
328326
}
329327

328+
/// Identifier for a request to be used for cancellation.
329+
pub type RequestId = u32;
330+
330331
#[derive(Serialize, Deserialize, Type)]
331332
pub enum ServiceError {
332333
/// Some unknown error with the authenticator occurred.
@@ -625,6 +626,7 @@ impl From<UsbState> for Value<'_> {
625626
#[derive(Serialize, Deserialize, Type)]
626627
pub struct ViewRequest {
627628
pub operation: Operation,
629+
pub id: RequestId,
628630
}
629631

630632
fn value_to_owned(value: &Value<'_>) -> OwnedValue {

credentialsd-ui/src/client.rs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use async_std::stream::Stream;
2-
use credentialsd_common::client::FlowController;
2+
use credentialsd_common::{client::FlowController, server::RequestId};
33
use futures_lite::StreamExt;
44
use zbus::{Connection, zvariant};
55

@@ -101,4 +101,17 @@ impl FlowController for DbusCredentialClient {
101101
.await
102102
.map_err(|err| tracing::error!("Failed to select credential: {err}"))
103103
}
104+
105+
async fn cancel_request(&self, request_id: RequestId) -> Result<(), ()> {
106+
if self
107+
.proxy()
108+
.await?
109+
.cancel_request(request_id)
110+
.await
111+
.is_err()
112+
{
113+
tracing::warn!("Failed to cancel request {request_id}");
114+
}
115+
Ok(())
116+
}
104117
}

credentialsd-ui/src/dbus.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use async_std::channel::Sender;
2-
use credentialsd_common::server::{BackgroundEvent, Device, ViewRequest};
2+
use credentialsd_common::server::{BackgroundEvent, Device, RequestId, ViewRequest};
33
use zbus::{fdo, interface, proxy};
44

55
#[proxy(
@@ -20,22 +20,20 @@ pub trait FlowControlService {
2020
async fn select_device(&self, device_id: String) -> fdo::Result<()>;
2121
async fn enter_client_pin(&self, pin: String) -> fdo::Result<()>;
2222
async fn select_credential(&self, credential_id: String) -> fdo::Result<()>;
23+
async fn cancel_request(&self, request_id: RequestId) -> fdo::Result<()>;
2324

2425
#[zbus(signal)]
2526
async fn state_changed(update: BackgroundEvent) -> zbus::Result<()>;
2627
}
2728

2829
pub struct UiControlService {
29-
pub request_tx: Sender<crate::dbus::ViewRequest>,
30+
pub request_tx: Sender<ViewRequest>,
3031
}
3132

3233
/// These methods are called by the credential service to control the UI.
3334
#[interface(name = "xyz.iinuwa.credentialsd.UiControl1")]
3435
impl UiControlService {
35-
async fn launch_ui(
36-
&self,
37-
request: credentialsd_common::server::ViewRequest,
38-
) -> fdo::Result<()> {
36+
async fn launch_ui(&self, request: ViewRequest) -> fdo::Result<()> {
3937
tracing::debug!("Received UI launch request");
4038
self.request_tx
4139
.send(request)

credentialsd-ui/src/gui/mod.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,16 @@ fn run_gui<F: FlowController + Send + Sync + 'static>(
3131
let (tx_update, rx_update) = async_std::channel::unbounded::<ViewUpdate>();
3232
let (tx_event, rx_event) = async_std::channel::unbounded::<ViewEvent>();
3333
let event_loop = async_std::task::spawn(async move {
34-
let mut vm = view_model::ViewModel::new(operation, flow_controller, rx_event, tx_update);
34+
let mut vm =
35+
view_model::ViewModel::new(operation, flow_controller.clone(), rx_event, tx_update);
3536
vm.start_event_loop().await;
36-
println!("event loop ended?");
37+
tracing::debug!("Finishing user request.");
38+
// If cancellation fails, that's fine.
39+
let _ = flow_controller
40+
.lock()
41+
.await
42+
.cancel_request(request.id)
43+
.await;
3744
});
3845

3946
view_model::gtk::start_gtk_app(tx_event, rx_update);

credentialsd-ui/src/gui/view_model/gtk/application.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ use crate::config::{APP_ID, PKGDATADIR, PROFILE, VERSION};
1111
use crate::gui::view_model::{ViewEvent, ViewUpdate};
1212

1313
mod imp {
14+
use crate::gui::view_model::gtk::ModelState;
15+
1416
use super::*;
1517
use glib::{WeakRef, clone};
1618
use std::{
@@ -67,6 +69,20 @@ mod imp {
6769
));
6870
}
6971
});
72+
let window3 = window.clone();
73+
// TODO: merge these state callbacks into a single function
74+
vm2.clone().connect_state_notify(move |vm| {
75+
if let ModelState::Cancelled = vm.state() {
76+
glib::spawn_future_local(clone!(
77+
#[weak]
78+
window3,
79+
async move {
80+
gtk::prelude::WidgetExt::activate_action(&window3, "window.close", None)
81+
.unwrap()
82+
}
83+
));
84+
}
85+
});
7086
self.window
7187
.set(window.downgrade())
7288
.expect("Window already set.");

credentialsd-ui/src/gui/view_model/gtk/mod.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,9 @@ mod imp {
5151
#[property(get, set)]
5252
pub prompt: RefCell<String>,
5353

54+
#[property(get, set, builder(ModelState::Pending))]
55+
pub state: RefCell<ModelState>,
56+
5457
#[property(get, set)]
5558
pub completed: RefCell<bool>,
5659

@@ -189,10 +192,14 @@ impl ViewModel {
189192
view_model.set_failed(true);
190193
view_model.set_prompt(error_msg);
191194
}
195+
ViewUpdate::Cancelled => {
196+
view_model.set_state(ModelState::Cancelled)
197+
}
192198
}
193199
}
194200
Err(e) => {
195201
debug!("ViewModel event listener interrupted: {}", e);
202+
view_model.set_state(ModelState::Cancelled);
196203
break;
197204
}
198205
}
@@ -361,3 +368,13 @@ pub fn start_gtk_app(
361368
let app = ExampleApplication::new(tx_event, rx_update);
362369
app.run();
363370
}
371+
372+
#[derive(Clone, Copy, Debug, Default, glib::Enum)]
373+
#[enum_type(name = "ModelState")]
374+
pub enum ModelState {
375+
#[default]
376+
Pending,
377+
Completed,
378+
Failed,
379+
Cancelled,
380+
}

credentialsd-ui/src/gui/view_model/gtk/window.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ use crate::gui::view_model::Transport;
1717
mod imp {
1818
use gtk::Picture;
1919

20+
use crate::gui::view_model::ViewEvent;
21+
2022
use super::*;
2123

2224
#[derive(Debug, Properties, gtk::CompositeTemplate)]
@@ -106,6 +108,17 @@ mod imp {
106108
impl WindowImpl for ExampleApplicationWindow {
107109
// Save window state on delete event
108110
fn close_request(&self) -> glib::Propagation {
111+
if let Some(vm) = self.view_model.borrow().as_ref() {
112+
if vm
113+
.get_sender()
114+
.send_blocking(ViewEvent::UserCancelled)
115+
.is_err()
116+
{
117+
tracing::warn!(
118+
"Failed to notify the backend service that the user cancelled the request."
119+
);
120+
};
121+
}
109122
if let Err(err) = self.obj().save_window_size() {
110123
tracing::warn!("Failed to save window state, {}", &err);
111124
}

0 commit comments

Comments
 (0)