From e9546217615898c466f9bf0bea96cb33d2058721 Mon Sep 17 00:00:00 2001 From: Jared Snider <116684653+JaredSnider-Bitwarden@users.noreply.github.com> Date: Mon, 9 Sep 2024 10:03:30 -0400 Subject: [PATCH] Auth/PM-10684 - Fix CLI asking for MP even if session key provided as command option (#10917) * PM-10684 - ServiceContainer - Add better docs * PM-10684 - UserAutoUnlockKeyService - setUserKeyInMemoryIfAutoUserKeySet - refactor method to return a bool instead of nothing so users can know if a user key was retrieved & set without another call. * PM-10684 - Remove async code ( Program.ts) responsible for setting the auto user key from the session option handler (event emitter which fires when a user passes --session to a command). Returning this to synchronous execution prevents a race condition between the setting of the user key and the command executing the exitIfLocked logic in the base-program which would check if the user key had been set to determine if the command should be allowed to execute or if the user was locked. When running a loop from a script, the command would often trigger the exitIfLocked before the auto user key could be set in state from the option:session session. * PM-10684 - Clean up missed item per PR feedback --- apps/cli/src/base-program.ts | 77 ++++++++++++------- apps/cli/src/program.ts | 13 +--- .../service-container/service-container.ts | 4 + .../services/user-auto-unlock-key.service.ts | 8 +- 4 files changed, 58 insertions(+), 44 deletions(-) diff --git a/apps/cli/src/base-program.ts b/apps/cli/src/base-program.ts index e4340b68e2..0f200d49d9 100644 --- a/apps/cli/src/base-program.ts +++ b/apps/cli/src/base-program.ts @@ -3,6 +3,7 @@ import { firstValueFrom, map } from "rxjs"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { UserId } from "@bitwarden/common/types/guid"; import { UnlockCommand } from "./auth/commands/unlock.command"; import { Response } from "./models/response"; @@ -135,37 +136,55 @@ export abstract class BaseProgram { protected async exitIfLocked() { const userId = await this.exitIfNotAuthed(); - if (await this.serviceContainer.cryptoService.hasUserKey()) { + + // If the process.env does not have a BW_SESSION key, then we will never be able to retrieve + // the auto user key from secure storage. This is because the auto user key is encrypted with + // the session key. + const hasUserKey = + await this.serviceContainer.userAutoUnlockKeyService.setUserKeyInMemoryIfAutoUserKeySet( + userId, + ); + + if (hasUserKey) { + // User is unlocked return; - } else if (process.env.BW_NOINTERACTION !== "true") { - // must unlock - if (await this.serviceContainer.keyConnectorService.getUsesKeyConnector(userId)) { - const response = Response.error( - "Your vault is locked. You must unlock your vault using your session key.\n" + - "If you do not have your session key, you can get a new one by logging out and logging in again.", - ); - this.processResponse(response, true); - } else { - const command = new UnlockCommand( - this.serviceContainer.accountService, - this.serviceContainer.masterPasswordService, - this.serviceContainer.cryptoService, - this.serviceContainer.userVerificationService, - this.serviceContainer.cryptoFunctionService, - this.serviceContainer.logService, - this.serviceContainer.keyConnectorService, - this.serviceContainer.environmentService, - this.serviceContainer.syncService, - this.serviceContainer.organizationApiService, - this.serviceContainer.logout, - ); - const response = await command.run(null, null); - if (!response.success) { - this.processResponse(response, true); - } - } - } else { + } + + // User is locked + await this.handleLockedUser(userId); + } + + private async handleLockedUser(userId: UserId) { + if (process.env.BW_NOINTERACTION === "true") { this.processResponse(Response.error("Vault is locked."), true); + return; + } + + // must unlock with interaction allowed + if (await this.serviceContainer.keyConnectorService.getUsesKeyConnector(userId)) { + const response = Response.error( + "Your vault is locked. You must unlock your vault using your session key.\n" + + "If you do not have your session key, you can get a new one by logging out and logging in again.", + ); + this.processResponse(response, true); + } else { + const command = new UnlockCommand( + this.serviceContainer.accountService, + this.serviceContainer.masterPasswordService, + this.serviceContainer.cryptoService, + this.serviceContainer.userVerificationService, + this.serviceContainer.cryptoFunctionService, + this.serviceContainer.logService, + this.serviceContainer.keyConnectorService, + this.serviceContainer.environmentService, + this.serviceContainer.syncService, + this.serviceContainer.organizationApiService, + this.serviceContainer.logout, + ); + const response = await command.run(null, null); + if (!response.success) { + this.processResponse(response, true); + } } } diff --git a/apps/cli/src/program.ts b/apps/cli/src/program.ts index 6ecdb24931..7582c76095 100644 --- a/apps/cli/src/program.ts +++ b/apps/cli/src/program.ts @@ -1,6 +1,5 @@ import * as chalk from "chalk"; import { program, Command, OptionValues } from "commander"; -import { firstValueFrom } from "rxjs"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; @@ -61,18 +60,8 @@ export class Program extends BaseProgram { process.env.BW_NOINTERACTION = "true"; }); - program.on("option:session", async (key) => { + program.on("option:session", (key) => { process.env.BW_SESSION = key; - - // once we have the session key, we can set the user key in memory - const activeAccount = await firstValueFrom( - this.serviceContainer.accountService.activeAccount$, - ); - if (activeAccount) { - await this.serviceContainer.userAutoUnlockKeyService.setUserKeyInMemoryIfAutoUserKeySet( - activeAccount.id, - ); - } }); program.on("command:*", () => { diff --git a/apps/cli/src/service-container/service-container.ts b/apps/cli/src/service-container/service-container.ts index be1b1cc9e2..ba73c26085 100644 --- a/apps/cli/src/service-container/service-container.ts +++ b/apps/cli/src/service-container/service-container.ts @@ -803,6 +803,10 @@ export class ServiceContainer { await this.i18nService.init(); this.twoFactorService.init(); + // If a user has a BW_SESSION key stored in their env (not process.env.BW_SESSION), + // this should set the user key to unlock the vault on init. + // TODO: ideally, we wouldn't want to do this here but instead only for commands that require the vault to be unlocked + // as this runs on every command and could be a performance hit const activeAccount = await firstValueFrom(this.accountService.activeAccount$); if (activeAccount?.id) { await this.userAutoUnlockKeyService.setUserKeyInMemoryIfAutoUserKeySet(activeAccount.id); diff --git a/libs/common/src/platform/services/user-auto-unlock-key.service.ts b/libs/common/src/platform/services/user-auto-unlock-key.service.ts index 6c8ce3f048..b4a154133c 100644 --- a/libs/common/src/platform/services/user-auto-unlock-key.service.ts +++ b/libs/common/src/platform/services/user-auto-unlock-key.service.ts @@ -16,10 +16,11 @@ export class UserAutoUnlockKeyService { * However, for users that have the auto unlock user key set, we need to set the user key in memory * on application bootstrap and on active account changes so that the user's vault loads unlocked. * @param userId - The user id to check for an auto user key. + * @returns True if the auto user key is set successfully, false otherwise. */ - async setUserKeyInMemoryIfAutoUserKeySet(userId: UserId): Promise { + async setUserKeyInMemoryIfAutoUserKeySet(userId: UserId): Promise { if (userId == null) { - return; + return false; } const autoUserKey = await this.cryptoService.getUserKeyFromStorage( @@ -28,9 +29,10 @@ export class UserAutoUnlockKeyService { ); if (autoUserKey == null) { - return; + return false; } await this.cryptoService.setUserKey(autoUserKey, userId); + return true; } }