From 643584543887ed8a25d167f56195053178880b70 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Thu, 21 Nov 2024 15:16:22 +0100 Subject: [PATCH] Make list messaging explicit --- .../desktop_native/core/src/ssh_agent/mod.rs | 8 ++-- .../desktop_native/core/src/ssh_agent/unix.rs | 2 +- .../core/src/ssh_agent/windows.rs | 2 +- apps/desktop/desktop_native/napi/index.d.ts | 2 +- apps/desktop/desktop_native/napi/src/lib.rs | 8 ++-- .../platform/main/main-ssh-agent.service.ts | 3 +- .../platform/services/ssh-agent.service.ts | 47 +++++++++---------- 7 files changed, 36 insertions(+), 36 deletions(-) diff --git a/apps/desktop/desktop_native/core/src/ssh_agent/mod.rs b/apps/desktop/desktop_native/core/src/ssh_agent/mod.rs index 3711851566..9d04ea87cc 100644 --- a/apps/desktop/desktop_native/core/src/ssh_agent/mod.rs +++ b/apps/desktop/desktop_native/core/src/ssh_agent/mod.rs @@ -17,7 +17,7 @@ pub mod importer; pub struct BitwardenDesktopAgent { keystore: ssh_agent::KeyStore, cancellation_token: CancellationToken, - show_ui_request_tx: tokio::sync::mpsc::Sender<(u32, String)>, + show_ui_request_tx: tokio::sync::mpsc::Sender<(u32, (String, bool))>, get_ui_response_rx: Arc>>, request_id: Arc>, /// before first unlock, or after account switching, listing keys should require an unlock to get a list of public keys @@ -35,8 +35,9 @@ impl ssh_agent::Agent for BitwardenDesktopAgent { let request_id = self.get_request_id().await; let mut rx_channel = self.get_ui_response_rx.lock().await.resubscribe(); + let message = (request_id, (ssh_key.cipher_uuid.clone(), false)); self.show_ui_request_tx - .send((request_id, ssh_key.cipher_uuid.clone())) + .send(message) .await .expect("Should send request to ui"); while let Ok((id, response)) = rx_channel.recv().await { @@ -55,8 +56,9 @@ impl ssh_agent::Agent for BitwardenDesktopAgent { let request_id = self.get_request_id().await; let mut rx_channel = self.get_ui_response_rx.lock().await.resubscribe(); + let message = (request_id, ("".to_string(), true)); self.show_ui_request_tx - .send((request_id, "".to_string())) + .send(message) .await .expect("Should send request to ui"); while let Ok((id, response)) = rx_channel.recv().await { diff --git a/apps/desktop/desktop_native/core/src/ssh_agent/unix.rs b/apps/desktop/desktop_native/core/src/ssh_agent/unix.rs index 153fd9ff86..ed2fe9ffab 100644 --- a/apps/desktop/desktop_native/core/src/ssh_agent/unix.rs +++ b/apps/desktop/desktop_native/core/src/ssh_agent/unix.rs @@ -14,7 +14,7 @@ use super::BitwardenDesktopAgent; impl BitwardenDesktopAgent { pub async fn start_server( - auth_request_tx: tokio::sync::mpsc::Sender<(u32, String)>, + auth_request_tx: tokio::sync::mpsc::Sender<(u32, (String, bool))>, auth_response_rx: Arc>>, ) -> Result { let agent = BitwardenDesktopAgent { diff --git a/apps/desktop/desktop_native/core/src/ssh_agent/windows.rs b/apps/desktop/desktop_native/core/src/ssh_agent/windows.rs index ed7026f993..6a99b7cfb0 100644 --- a/apps/desktop/desktop_native/core/src/ssh_agent/windows.rs +++ b/apps/desktop/desktop_native/core/src/ssh_agent/windows.rs @@ -12,7 +12,7 @@ use super::BitwardenDesktopAgent; impl BitwardenDesktopAgent { pub async fn start_server( - auth_request_tx: tokio::sync::mpsc::Sender<(u32, String)>, + auth_request_tx: tokio::sync::mpsc::Sender<(u32, (String, bool))>, auth_response_rx: Arc>>, ) -> Result { let agent_state = BitwardenDesktopAgent { diff --git a/apps/desktop/desktop_native/napi/index.d.ts b/apps/desktop/desktop_native/napi/index.d.ts index ed53194804..956b2d726d 100644 --- a/apps/desktop/desktop_native/napi/index.d.ts +++ b/apps/desktop/desktop_native/napi/index.d.ts @@ -69,7 +69,7 @@ export declare namespace sshagent { status: SshKeyImportStatus sshKey?: SshKey } - export function serve(callback: (err: Error | null, arg: string) => any): Promise + export function serve(callback: (err: Error | null, arg0: string, arg1: boolean) => any): Promise export function stop(agentState: SshAgentState): void export function isRunning(agentState: SshAgentState): boolean export function setKeys(agentState: SshAgentState, newKeys: Array): void diff --git a/apps/desktop/desktop_native/napi/src/lib.rs b/apps/desktop/desktop_native/napi/src/lib.rs index 610140b586..4c7bc8eaa9 100644 --- a/apps/desktop/desktop_native/napi/src/lib.rs +++ b/apps/desktop/desktop_native/napi/src/lib.rs @@ -247,15 +247,15 @@ pub mod sshagent { #[napi] pub async fn serve( - callback: ThreadsafeFunction, + callback: ThreadsafeFunction<(String, bool), CalleeHandled>, ) -> napi::Result { - let (auth_request_tx, mut auth_request_rx) = tokio::sync::mpsc::channel::<(u32, String)>(32); + let (auth_request_tx, mut auth_request_rx) = tokio::sync::mpsc::channel::<(u32, (String, bool))>(32); let (auth_response_tx, auth_response_rx) = tokio::sync::broadcast::channel::<(u32, bool)>(32); let auth_response_tx_arc = Arc::new(Mutex::new(auth_response_tx)); tokio::spawn(async move { let _ = auth_response_rx; - while let Some((request_id, cipher_uuid)) = auth_request_rx.recv().await { + while let Some((request_id, (cipher_uuid, is_list_request))) = auth_request_rx.recv().await { let cloned_request_id = request_id.clone(); let cloned_cipher_uuid = cipher_uuid.clone(); let cloned_response_tx_arc = auth_response_tx_arc.clone(); @@ -266,7 +266,7 @@ pub mod sshagent { let auth_response_tx_arc = cloned_response_tx_arc; let callback = cloned_callback; let promise_result: Result, napi::Error> = - callback.call_async(Ok(cipher_uuid)).await; + callback.call_async(Ok((cipher_uuid, is_list_request))).await; match promise_result { Ok(promise_result) => match promise_result.await { Ok(result) => { diff --git a/apps/desktop/src/platform/main/main-ssh-agent.service.ts b/apps/desktop/src/platform/main/main-ssh-agent.service.ts index 65124f1313..5284029c69 100644 --- a/apps/desktop/src/platform/main/main-ssh-agent.service.ts +++ b/apps/desktop/src/platform/main/main-ssh-agent.service.ts @@ -27,7 +27,7 @@ export class MainSshAgentService { init() { // handle sign request passing to UI sshagent - .serve(async (err: Error, cipherId: string) => { + .serve(async (err: Error, cipherId: string, isListRequest: boolean) => { // clear all old (> SIGN_TIMEOUT) requests this.requestResponses = this.requestResponses.filter( (response) => response.timestamp > new Date(Date.now() - this.SIGN_TIMEOUT), @@ -37,6 +37,7 @@ export class MainSshAgentService { const id_for_this_request = this.request_id; this.messagingService.send("sshagent.signrequest", { cipherId, + isListRequest, requestId: id_for_this_request, }); diff --git a/apps/desktop/src/platform/services/ssh-agent.service.ts b/apps/desktop/src/platform/services/ssh-agent.service.ts index dfcec846d3..b1d30b931b 100644 --- a/apps/desktop/src/platform/services/ssh-agent.service.ts +++ b/apps/desktop/src/platform/services/ssh-agent.service.ts @@ -115,36 +115,33 @@ export class SshAgentService implements OnDestroy { ), ), // This concatMap handles showing the dialog to approve the request. - concatMap(([message, ciphers]) => { + switchMap(([message, ciphers]) => { const cipherId = message.cipherId as string; + const isListRequest = message.isListRequest as boolean; const requestId = message.requestId as number; - // cipherid is empty has the special meaning of "unlock in order to list public keys" - if (cipherId == "") { - return of(true).pipe( - switchMap(async (result) => { - const sshCiphers = ciphers.filter( - (cipher) => cipher.type === CipherType.SshKey && !cipher.isDeleted, - ); - const keys = sshCiphers.map((cipher) => { - return { - name: cipher.name, - privateKey: cipher.sshKey.privateKey, - cipherId: cipher.id, - }; - }); - await ipc.platform.sshAgent.setKeys(keys); - await ipc.platform.sshAgent.signRequestResponse(requestId, Boolean(result)); - }), - ); + if (isListRequest) { + (async () => { + const sshCiphers = ciphers.filter( + (cipher) => cipher.type === CipherType.SshKey && !cipher.isDeleted, + ); + const keys = sshCiphers.map((cipher) => { + return { + name: cipher.name, + privateKey: cipher.sshKey.privateKey, + cipherId: cipher.id, + }; + }); + await ipc.platform.sshAgent.setKeys(keys); + await ipc.platform.sshAgent.signRequestResponse(requestId, true); + })().catch((e) => this.logService.error("Failed to respond to SSH request", e)); + return; } if (ciphers === undefined) { - return of(false).pipe( - switchMap((result) => - ipc.platform.sshAgent.signRequestResponse(requestId, Boolean(result)), - ), - ); + ipc.platform.sshAgent + .signRequestResponse(requestId, false) + .catch((e) => this.logService.error("Failed to respond to SSH request", e)); } const cipher = ciphers.find((cipher) => cipher.id == cipherId); @@ -158,7 +155,7 @@ export class SshAgentService implements OnDestroy { return dialogRef.closed.pipe( switchMap((result) => { - return ipc.platform.sshAgent.signRequestResponse(requestId, Boolean(result)); + return ipc.platform.sshAgent.signRequestResponse(requestId, result); }), ); }),