From 244539cc386cc27cdf07404a2555871399836c54 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Wed, 8 Jan 2025 14:25:19 +0100 Subject: [PATCH] [PM-16837] Fix agent only loading when featureflag is on during startup (#12742) * Fix ssh generation and import not being available when agent feature-flag is disabled * Fix agent only loading when featureflag is on during startup --- apps/desktop/src/main.ts | 9 +- .../platform/main/main-ssh-agent.service.ts | 8 + apps/desktop/src/platform/preload.ts | 3 + .../platform/services/ssh-agent.service.ts | 301 +++++++++--------- 4 files changed, 169 insertions(+), 152 deletions(-) diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts index 3232eef2b9..01d84a8f76 100644 --- a/apps/desktop/src/main.ts +++ b/apps/desktop/src/main.ts @@ -2,7 +2,7 @@ // @ts-strict-ignore import * as path from "path"; -import { app, ipcMain } from "electron"; +import { app } from "electron"; import { Subject, firstValueFrom } from "rxjs"; import { AccountServiceImplementation } from "@bitwarden/common/auth/services/account.service"; @@ -257,12 +257,7 @@ export class Main { this.clipboardMain = new ClipboardMain(); this.clipboardMain.init(); - ipcMain.handle("sshagent.init", async (event: any, message: any) => { - if (this.sshAgentService == null) { - this.sshAgentService = new MainSshAgentService(this.logService, this.messagingService); - this.sshAgentService.init(); - } - }); + this.sshAgentService = new MainSshAgentService(this.logService, this.messagingService); new EphemeralValueStorageService(); new SSOLocalhostCallbackService(this.environmentService, this.messagingService); 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 f06324c0db..764189635c 100644 --- a/apps/desktop/src/platform/main/main-ssh-agent.service.ts +++ b/apps/desktop/src/platform/main/main-ssh-agent.service.ts @@ -40,6 +40,14 @@ export class MainSshAgentService { return sshagent.importKey(privateKey, password); }, ); + + ipcMain.handle("sshagent.init", async (event: any, message: any) => { + this.init(); + }); + + ipcMain.handle("sshagent.isloaded", async (event: any) => { + return this.agentState != null; + }); } init() { diff --git a/apps/desktop/src/platform/preload.ts b/apps/desktop/src/platform/preload.ts index 9c1986fb61..11a46abd53 100644 --- a/apps/desktop/src/platform/preload.ts +++ b/apps/desktop/src/platform/preload.ts @@ -74,6 +74,9 @@ const sshAgent = { }); return res; }, + isLoaded(): Promise { + return ipcRenderer.invoke("sshagent.isloaded"); + }, }; const powermonitor = { diff --git a/apps/desktop/src/platform/services/ssh-agent.service.ts b/apps/desktop/src/platform/services/ssh-agent.service.ts index 651e67e946..e860ebe1db 100644 --- a/apps/desktop/src/platform/services/ssh-agent.service.ts +++ b/apps/desktop/src/platform/services/ssh-agent.service.ts @@ -61,153 +61,87 @@ export class SshAgentService implements OnDestroy { ) {} async init() { - const isSshAgentFeatureEnabled = await this.configService.getFeatureFlag(FeatureFlag.SSHAgent); - if (isSshAgentFeatureEnabled) { - await ipc.platform.sshAgent.init(); + this.configService + .getFeatureFlag$(FeatureFlag.SSHAgent) + .pipe( + concatMap(async (enabled) => { + if (enabled && !(await ipc.platform.sshAgent.isLoaded())) { + return this.initSshAgent(); + } + }), + takeUntil(this.destroy$), + ) + .subscribe(); + } - this.messageListener - .messages$(new CommandDefinition("sshagent.signrequest")) - .pipe( - withLatestFrom(this.authService.activeAccountStatus$), - // This switchMap handles unlocking the vault if it is locked: - // - If the vault is locked, we will wait for it to be unlocked. - // - If the vault is not unlocked within the timeout, we will abort the flow. - // - If the vault is unlocked, we will continue with the flow. - // switchMap is used here to prevent multiple requests from being processed at the same time, - // and will cancel the previous request if a new one is received. - switchMap(([message, status]) => { - if (status !== AuthenticationStatus.Unlocked) { - ipc.platform.focusWindow(); - this.toastService.showToast({ - variant: "info", - title: null, - message: this.i18nService.t("sshAgentUnlockRequired"), - }); - return this.authService.activeAccountStatus$.pipe( - filter((status) => status === AuthenticationStatus.Unlocked), - timeout({ - first: this.SSH_VAULT_UNLOCK_REQUEST_TIMEOUT, - }), - catchError((error: unknown) => { - if (error instanceof TimeoutError) { - this.toastService.showToast({ - variant: "error", - title: null, - message: this.i18nService.t("sshAgentUnlockTimeout"), - }); - const requestId = message.requestId as number; - // Abort flow by sending a false response. - // Returning an empty observable this will prevent the rest of the flow from executing - return from(ipc.platform.sshAgent.signRequestResponse(requestId, false)).pipe( - map(() => EMPTY), - ); - } - - throw error; - }), - map(() => message), - ); - } - - return of(message); - }), - // This switchMap handles fetching the ciphers from the vault. - switchMap((message) => - from(this.cipherService.getAllDecrypted()).pipe( - map((ciphers) => [message, ciphers] as const), - ), - ), - // This concatMap handles showing the dialog to approve the request. - concatMap(async ([message, ciphers]) => { - const cipherId = message.cipherId as string; - const isListRequest = message.isListRequest as boolean; - const requestId = message.requestId as number; - let application = message.processName as string; - if (application == "") { - application = this.i18nService.t("unknownApplication"); - } - - if (isListRequest) { - 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); - return; - } - - if (ciphers === undefined) { - 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); + private async initSshAgent() { + await ipc.platform.sshAgent.init(); + this.messageListener + .messages$(new CommandDefinition("sshagent.signrequest")) + .pipe( + withLatestFrom(this.authService.activeAccountStatus$), + // This switchMap handles unlocking the vault if it is locked: + // - If the vault is locked, we will wait for it to be unlocked. + // - If the vault is not unlocked within the timeout, we will abort the flow. + // - If the vault is unlocked, we will continue with the flow. + // switchMap is used here to prevent multiple requests from being processed at the same time, + // and will cancel the previous request if a new one is received. + switchMap(([message, status]) => { + if (status !== AuthenticationStatus.Unlocked) { ipc.platform.focusWindow(); - const dialogRef = ApproveSshRequestComponent.open( - this.dialogService, - cipher.name, - application, + this.toastService.showToast({ + variant: "info", + title: null, + message: this.i18nService.t("sshAgentUnlockRequired"), + }); + return this.authService.activeAccountStatus$.pipe( + filter((status) => status === AuthenticationStatus.Unlocked), + timeout({ + first: this.SSH_VAULT_UNLOCK_REQUEST_TIMEOUT, + }), + catchError((error: unknown) => { + if (error instanceof TimeoutError) { + this.toastService.showToast({ + variant: "error", + title: null, + message: this.i18nService.t("sshAgentUnlockTimeout"), + }); + const requestId = message.requestId as number; + // Abort flow by sending a false response. + // Returning an empty observable this will prevent the rest of the flow from executing + return from(ipc.platform.sshAgent.signRequestResponse(requestId, false)).pipe( + map(() => EMPTY), + ); + } + + throw error; + }), + map(() => message), ); + } - const result = await firstValueFrom(dialogRef.closed); - return ipc.platform.sshAgent.signRequestResponse(requestId, result); - }), - takeUntil(this.destroy$), - ) - .subscribe(); - - this.accountService.activeAccount$.pipe(skip(1), takeUntil(this.destroy$)).subscribe({ - next: (account) => { - this.logService.info("Active account changed, clearing SSH keys"); - ipc.platform.sshAgent - .clearKeys() - .catch((e) => this.logService.error("Failed to clear SSH keys", e)); - }, - error: (e: unknown) => { - this.logService.error("Error in active account observable", e); - ipc.platform.sshAgent - .clearKeys() - .catch((e) => this.logService.error("Failed to clear SSH keys", e)); - }, - complete: () => { - this.logService.info("Active account observable completed, clearing SSH keys"); - ipc.platform.sshAgent - .clearKeys() - .catch((e) => this.logService.error("Failed to clear SSH keys", e)); - }, - }); - - combineLatest([ - timer(0, this.SSH_REFRESH_INTERVAL), - this.desktopSettingsService.sshAgentEnabled$, - ]) - .pipe( - concatMap(async ([, enabled]) => { - if (!enabled) { - await ipc.platform.sshAgent.clearKeys(); - return; - } - - const ciphers = await this.cipherService.getAllDecrypted(); - if (ciphers == null) { - await ipc.platform.sshAgent.lock(); - return; - } + return of(message); + }), + // This switchMap handles fetching the ciphers from the vault. + switchMap((message) => + from(this.cipherService.getAllDecrypted()).pipe( + map((ciphers) => [message, ciphers] as const), + ), + ), + // This concatMap handles showing the dialog to approve the request. + concatMap(async ([message, ciphers]) => { + const cipherId = message.cipherId as string; + const isListRequest = message.isListRequest as boolean; + const requestId = message.requestId as number; + let application = message.processName as string; + if (application == "") { + application = this.i18nService.t("unknownApplication"); + } + if (isListRequest) { const sshCiphers = ciphers.filter( - (cipher) => - cipher.type === CipherType.SshKey && - !cipher.isDeleted && - cipher.organizationId === null, + (cipher) => cipher.type === CipherType.SshKey && !cipher.isDeleted, ); const keys = sshCiphers.map((cipher) => { return { @@ -217,11 +151,88 @@ export class SshAgentService implements OnDestroy { }; }); await ipc.platform.sshAgent.setKeys(keys); - }), - takeUntil(this.destroy$), - ) - .subscribe(); - } + await ipc.platform.sshAgent.signRequestResponse(requestId, true); + return; + } + + if (ciphers === undefined) { + 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); + + ipc.platform.focusWindow(); + const dialogRef = ApproveSshRequestComponent.open( + this.dialogService, + cipher.name, + application, + ); + + const result = await firstValueFrom(dialogRef.closed); + return ipc.platform.sshAgent.signRequestResponse(requestId, result); + }), + takeUntil(this.destroy$), + ) + .subscribe(); + + this.accountService.activeAccount$.pipe(skip(1), takeUntil(this.destroy$)).subscribe({ + next: (account) => { + this.logService.info("Active account changed, clearing SSH keys"); + ipc.platform.sshAgent + .clearKeys() + .catch((e) => this.logService.error("Failed to clear SSH keys", e)); + }, + error: (e: unknown) => { + this.logService.error("Error in active account observable", e); + ipc.platform.sshAgent + .clearKeys() + .catch((e) => this.logService.error("Failed to clear SSH keys", e)); + }, + complete: () => { + this.logService.info("Active account observable completed, clearing SSH keys"); + ipc.platform.sshAgent + .clearKeys() + .catch((e) => this.logService.error("Failed to clear SSH keys", e)); + }, + }); + + combineLatest([ + timer(0, this.SSH_REFRESH_INTERVAL), + this.desktopSettingsService.sshAgentEnabled$, + ]) + .pipe( + concatMap(async ([, enabled]) => { + if (!enabled) { + await ipc.platform.sshAgent.clearKeys(); + return; + } + + const ciphers = await this.cipherService.getAllDecrypted(); + if (ciphers == null) { + await ipc.platform.sshAgent.lock(); + return; + } + + const sshCiphers = ciphers.filter( + (cipher) => + cipher.type === CipherType.SshKey && + !cipher.isDeleted && + cipher.organizationId === null, + ); + const keys = sshCiphers.map((cipher) => { + return { + name: cipher.name, + privateKey: cipher.sshKey.privateKey, + cipherId: cipher.id, + }; + }); + await ipc.platform.sshAgent.setKeys(keys); + }), + takeUntil(this.destroy$), + ) + .subscribe(); } ngOnDestroy() {