1
0
mirror of https://github.com/bitwarden/browser.git synced 2024-11-14 10:26:19 +01:00

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 <sessionKey> 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
This commit is contained in:
Jared Snider 2024-09-09 10:03:30 -04:00 committed by GitHub
parent 60e9969017
commit e954621761
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 58 additions and 44 deletions

View File

@ -3,6 +3,7 @@ import { firstValueFrom, map } from "rxjs";
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { UserId } from "@bitwarden/common/types/guid";
import { UnlockCommand } from "./auth/commands/unlock.command"; import { UnlockCommand } from "./auth/commands/unlock.command";
import { Response } from "./models/response"; import { Response } from "./models/response";
@ -135,10 +136,31 @@ export abstract class BaseProgram {
protected async exitIfLocked() { protected async exitIfLocked() {
const userId = await this.exitIfNotAuthed(); 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; return;
} else if (process.env.BW_NOINTERACTION !== "true") { }
// must unlock
// 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)) { if (await this.serviceContainer.keyConnectorService.getUsesKeyConnector(userId)) {
const response = Response.error( const response = Response.error(
"Your vault is locked. You must unlock your vault using your session key.\n" + "Your vault is locked. You must unlock your vault using your session key.\n" +
@ -164,9 +186,6 @@ export abstract class BaseProgram {
this.processResponse(response, true); this.processResponse(response, true);
} }
} }
} else {
this.processResponse(Response.error("Vault is locked."), true);
}
} }
protected async exitIfFeatureFlagDisabled(featureFlag: FeatureFlag) { protected async exitIfFeatureFlagDisabled(featureFlag: FeatureFlag) {

View File

@ -1,6 +1,5 @@
import * as chalk from "chalk"; import * as chalk from "chalk";
import { program, Command, OptionValues } from "commander"; import { program, Command, OptionValues } from "commander";
import { firstValueFrom } from "rxjs";
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
@ -61,18 +60,8 @@ export class Program extends BaseProgram {
process.env.BW_NOINTERACTION = "true"; process.env.BW_NOINTERACTION = "true";
}); });
program.on("option:session", async (key) => { program.on("option:session", (key) => {
process.env.BW_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:*", () => { program.on("command:*", () => {

View File

@ -803,6 +803,10 @@ export class ServiceContainer {
await this.i18nService.init(); await this.i18nService.init();
this.twoFactorService.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$); const activeAccount = await firstValueFrom(this.accountService.activeAccount$);
if (activeAccount?.id) { if (activeAccount?.id) {
await this.userAutoUnlockKeyService.setUserKeyInMemoryIfAutoUserKeySet(activeAccount.id); await this.userAutoUnlockKeyService.setUserKeyInMemoryIfAutoUserKeySet(activeAccount.id);

View File

@ -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 * 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. * 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. * @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<void> { async setUserKeyInMemoryIfAutoUserKeySet(userId: UserId): Promise<boolean> {
if (userId == null) { if (userId == null) {
return; return false;
} }
const autoUserKey = await this.cryptoService.getUserKeyFromStorage( const autoUserKey = await this.cryptoService.getUserKeyFromStorage(
@ -28,9 +29,10 @@ export class UserAutoUnlockKeyService {
); );
if (autoUserKey == null) { if (autoUserKey == null) {
return; return false;
} }
await this.cryptoService.setUserKey(autoUserKey, userId); await this.cryptoService.setUserKey(autoUserKey, userId);
return true;
} }
} }