1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-18 01:03:35 +00:00

[PM-12047] Remove usage of ActiveUserState from cipher.service (#12814)

* Cipher service web changes

* Updated browser client to pass user id to cipher service observable changes

* Cli changes

* desktop changes

* Fixed test

* Libs changes

* Fixed merge conflicts

* Fixed merge conflicts

* removed duplicate reference fixed conflict

* Fixed test

* Fixed test

* Fixed test

* Fixed desturcturing issue on failed to decrypt ciphers cipher service

* Updated abstraction to use method syntax

* Fixed conflicts

* Fixed test on add edit v2

Passed active userId to delete function

* Used getUserId utility function

* made vault changes

* made suggestion changes

* made suggestion changes

* made suggestion changes

* Replace getUserId function calls with pipe operator syntax for better consistency

* fixed merge conflicts

* revert mistake made of usinf account activity during merge conflict fix

* fixed conflicts

* fixed tests
This commit is contained in:
SmithThe4th
2025-02-12 08:53:31 -05:00
committed by GitHub
parent e45ef6b924
commit a2945203f4
98 changed files with 1174 additions and 725 deletions

View File

@@ -4,6 +4,7 @@ import {
EMPTY,
Subject,
distinctUntilChanged,
filter,
firstValueFrom,
map,
mergeMap,
@@ -12,6 +13,7 @@ import {
} from "rxjs";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { getOptionalUserId } from "@bitwarden/common/auth/services/account.service";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { UriMatchStrategy } from "@bitwarden/common/models/domain/domain-service";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
@@ -27,6 +29,7 @@ import { Utils } from "@bitwarden/common/platform/misc/utils";
import { getCredentialsForAutofill } from "@bitwarden/common/platform/services/fido2/fido2-autofill-utils";
import { Fido2Utils } from "@bitwarden/common/platform/services/fido2/fido2-utils";
import { guidToRawFormat } from "@bitwarden/common/platform/services/fido2/guid-utils";
import { UserId } from "@bitwarden/common/types/guid";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { CipherType } from "@bitwarden/common/vault/enums";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
@@ -60,7 +63,11 @@ export class DesktopAutofillService implements OnDestroy {
return EMPTY;
}
return this.cipherService.cipherViews$;
return this.accountService.activeAccount$.pipe(
map((account) => account?.id),
filter((userId): userId is UserId => userId != null),
switchMap((userId) => this.cipherService.cipherViews$(userId)),
);
}),
// TODO: This will unset all the autofill credentials on the OS
// when the account locks. We should instead explicilty clear the credentials
@@ -164,17 +171,22 @@ export class DesktopAutofillService implements OnDestroy {
// TODO: For some reason the credentialId is passed as an empty array in the request, so we need to
// get it from the cipher. For that we use the recordIdentifier, which is the cipherId.
if (request.recordIdentifier && request.credentialId.length === 0) {
const cipher = await this.cipherService.get(request.recordIdentifier);
const activeUserId = await firstValueFrom(
this.accountService.activeAccount$.pipe(getOptionalUserId),
);
if (!activeUserId) {
this.logService.error("listenPasskeyAssertion error", "Active user not found");
callback(new Error("Active user not found"), null);
return;
}
const cipher = await this.cipherService.get(request.recordIdentifier, activeUserId);
if (!cipher) {
this.logService.error("listenPasskeyAssertion error", "Cipher not found");
callback(new Error("Cipher not found"), null);
return;
}
const activeUserId = await firstValueFrom(
this.accountService.activeAccount$.pipe(map((a) => a?.id)),
);
const decrypted = await cipher.decrypt(
await this.cipherService.getKeyForCipherKeyDecryption(cipher, activeUserId),
);

View File

@@ -29,6 +29,7 @@ import { ConfigService } from "@bitwarden/common/platform/abstractions/config/co
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { CommandDefinition, MessageListener } from "@bitwarden/common/platform/messaging";
import { UserId } from "@bitwarden/common/types/guid";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { CipherType } from "@bitwarden/common/vault/enums";
import { DialogService, ToastService } from "@bitwarden/components";
@@ -92,14 +93,14 @@ export class SshAgentService implements OnDestroy {
}),
filter(({ enabled }) => enabled),
map(({ message }) => message),
withLatestFrom(this.authService.activeAccountStatus$),
withLatestFrom(this.authService.activeAccountStatus$, this.accountService.activeAccount$),
// 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]) => {
switchMap(([message, status, account]) => {
if (status !== AuthenticationStatus.Unlocked) {
ipc.platform.focusWindow();
this.toastService.showToast({
@@ -133,11 +134,11 @@ export class SshAgentService implements OnDestroy {
);
}
return of(message);
return of([message, account.id]);
}),
// This switchMap handles fetching the ciphers from the vault.
switchMap((message) =>
from(this.cipherService.getAllDecrypted()).pipe(
switchMap(([message, userId]: [Record<string, unknown>, UserId]) =>
from(this.cipherService.getAllDecrypted(userId)).pipe(
map((ciphers) => [message, ciphers] as const),
),
),
@@ -245,7 +246,7 @@ export class SshAgentService implements OnDestroy {
return;
}
const ciphers = await this.cipherService.getAllDecrypted();
const ciphers = await this.cipherService.getAllDecrypted(activeAccount.id);
if (ciphers == null) {
await ipc.platform.sshAgent.lock();
return;

View File

@@ -1,12 +1,13 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { firstValueFrom, map } from "rxjs";
import { firstValueFrom } from "rxjs";
import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction";
import { PolicyType } from "@bitwarden/common/admin-console/enums";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
import { getUserId } from "@bitwarden/common/auth/services/account.service";
import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
import { UserId } from "@bitwarden/common/types/guid";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
@@ -65,9 +66,7 @@ export class EncryptedMessageHandlerService {
}
private async checkUserStatus(userId: string): Promise<string> {
const activeUserId = await firstValueFrom(
this.accountService.activeAccount$.pipe(map((a) => a?.id)),
);
const activeUserId = await firstValueFrom(getUserId(this.accountService.activeAccount$));
if (userId !== activeUserId) {
return "not-active-user";
@@ -83,9 +82,7 @@ export class EncryptedMessageHandlerService {
private async statusCommandHandler(): Promise<AccountStatusResponse[]> {
const accounts = await firstValueFrom(this.accountService.accounts$);
const activeUserId = await firstValueFrom(
this.accountService.activeAccount$.pipe(map((a) => a?.id)),
);
const activeUserId = await firstValueFrom(getUserId(this.accountService.activeAccount$));
if (!accounts || !Object.keys(accounts)) {
return [];
@@ -114,16 +111,14 @@ export class EncryptedMessageHandlerService {
}
const ciphersResponse: CipherResponse[] = [];
const activeUserId = await firstValueFrom(
this.accountService.activeAccount$.pipe(map((a) => a?.id)),
);
const activeUserId = await firstValueFrom(getUserId(this.accountService.activeAccount$));
const authStatus = await this.authService.getAuthStatus(activeUserId);
if (authStatus !== AuthenticationStatus.Unlocked) {
return { error: "locked" };
}
const ciphers = await this.cipherService.getAllDecryptedForUrl(payload.uri);
const ciphers = await this.cipherService.getAllDecryptedForUrl(payload.uri, activeUserId);
ciphers.sort((a, b) => this.cipherService.sortCiphersByLastUsedThenName(a, b));
ciphers.forEach((c) => {
@@ -166,9 +161,7 @@ export class EncryptedMessageHandlerService {
cipherView.login.uris[0].uri = credentialCreatePayload.uri;
try {
const activeUserId = await firstValueFrom(
this.accountService.activeAccount$.pipe(map((a) => a?.id)),
);
const activeUserId = await firstValueFrom(getUserId(this.accountService.activeAccount$));
const encrypted = await this.cipherService.encrypt(cipherView, activeUserId);
await this.cipherService.createWithServer(encrypted);
@@ -200,13 +193,16 @@ export class EncryptedMessageHandlerService {
}
try {
const cipher = await this.cipherService.get(credentialUpdatePayload.credentialId);
const activeUserId = await firstValueFrom(getUserId(this.accountService.activeAccount$));
const cipher = await this.cipherService.get(
credentialUpdatePayload.credentialId,
activeUserId,
);
if (cipher === null) {
return { status: "failure" };
}
const activeUserId = await firstValueFrom(
this.accountService.activeAccount$.pipe(map((a) => a?.id)),
);
const cipherView = await cipher.decrypt(
await this.cipherService.getKeyForCipherKeyDecryption(cipher, activeUserId),
);

View File

@@ -5,6 +5,7 @@ import { distinctUntilChanged } from "rxjs";
import { VaultItemsComponent as BaseVaultItemsComponent } from "@bitwarden/angular/vault/components/vault-items.component";
import { SearchService } from "@bitwarden/common/abstractions/search.service";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
@@ -20,8 +21,9 @@ export class VaultItemsComponent extends BaseVaultItemsComponent {
searchService: SearchService,
searchBarService: SearchBarService,
cipherService: CipherService,
accountService: AccountService,
) {
super(searchService, cipherService);
super(searchService, cipherService, accountService);
// eslint-disable-next-line rxjs-angular/prefer-takeuntil
searchBarService.searchText$.pipe(distinctUntilChanged()).subscribe((searchText) => {

View File

@@ -10,7 +10,7 @@ import {
ViewContainerRef,
} from "@angular/core";
import { ActivatedRoute, Router } from "@angular/router";
import { combineLatest, firstValueFrom, Subject, takeUntil, switchMap } from "rxjs";
import { firstValueFrom, Subject, takeUntil, switchMap } from "rxjs";
import { filter, first, map, take } from "rxjs/operators";
import { ModalRef } from "@bitwarden/angular/components/modal/modal.ref";
@@ -19,6 +19,7 @@ import { VaultFilter } from "@bitwarden/angular/vault/vault-filter/models/vault-
import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { getUserId } from "@bitwarden/common/auth/services/account.service";
import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service";
import { EventType } from "@bitwarden/common/enums";
import { BroadcasterService } from "@bitwarden/common/platform/abstractions/broadcaster.service";
@@ -236,15 +237,12 @@ export class VaultComponent implements OnInit, OnDestroy {
});
}
// Store a reference to the current active account during page init
const activeAccount = await firstValueFrom(this.accountService.activeAccount$);
const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId));
// Combine with the activeAccount$ to ensure we only show the dialog for the current account from ngOnInit.
// The account switching process updates the cipherService before Vault is destroyed and would cause duplicate emissions
combineLatest([this.accountService.activeAccount$, this.cipherService.failedToDecryptCiphers$])
this.cipherService
.failedToDecryptCiphers$(activeUserId)
.pipe(
filter(([account]) => account.id === activeAccount.id),
map(([_, ciphers]) => ciphers.filter((c) => !c.isDeleted)),
map((ciphers) => ciphers.filter((c) => !c.isDeleted)),
filter((ciphers) => ciphers.length > 0),
take(1),
takeUntil(this.componentIsDestroyed$),