Skip to content

Commit

Permalink
Simplified to accept a single user id instead of an observable
Browse files Browse the repository at this point in the history
  • Loading branch information
gbubemismith committed Nov 19, 2024
1 parent faba3d0 commit 47364bf
Show file tree
Hide file tree
Showing 33 changed files with 226 additions and 234 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { mock, MockProxy } from "jest-mock-extended";
import { BehaviorSubject, firstValueFrom } from "rxjs";

import { PolicyService } from "@bitwarden/common/admin-console/services/policy/policy.service";
import { AccountInfo } from "@bitwarden/common/auth/abstractions/account.service";
import { AccountInfo, AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
import { AuthService } from "@bitwarden/common/auth/services/auth.service";
import { ExtensionCommand } from "@bitwarden/common/autofill/constants";
Expand All @@ -11,10 +11,8 @@ import { UserNotificationSettingsService } from "@bitwarden/common/autofill/serv
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { Utils } from "@bitwarden/common/platform/misc/utils";
import { SelfHostedEnvironment } from "@bitwarden/common/platform/services/default-environment.service";
import { ThemeStateService } from "@bitwarden/common/platform/theming/theme-state.service";
import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec";
import { UserId } from "@bitwarden/common/types/guid";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { FolderView } from "@bitwarden/common/vault/models/view/folder.view";
Expand Down Expand Up @@ -60,15 +58,20 @@ describe("NotificationBackground", () => {
const logService = mock<LogService>();
const themeStateService = mock<ThemeStateService>();
const configService = mock<ConfigService>();
let accountService: FakeAccountService;
const accountService = mock<AccountService>();

const userId = Utils.newGuid() as UserId;
const activeAccountSubject = new BehaviorSubject<{ id: UserId } & AccountInfo>({
id: "testId" as UserId,
email: "[email protected]",
emailVerified: true,
name: "Test User",
});

beforeEach(() => {
accountService = mockAccountServiceWith(userId);
activeAccountStatusMock$ = new BehaviorSubject(AuthenticationStatus.Locked);
authService = mock<AuthService>();
authService.activeAccountStatus$ = activeAccountStatusMock$;
accountService.activeAccount$ = activeAccountSubject;
notificationBackground = new NotificationBackground(
autofillService,
cipherService,
Expand Down Expand Up @@ -688,13 +691,6 @@ describe("NotificationBackground", () => {
});

describe("saveOrUpdateCredentials", () => {
const activeAccountSubject = new BehaviorSubject<{ id: UserId } & AccountInfo>({
id: "testId" as UserId,
email: "[email protected]",
emailVerified: true,
name: "Test User",
});

let getDecryptedCipherByIdSpy: jest.SpyInstance;
let getAllDecryptedForUrlSpy: jest.SpyInstance;
let updatePasswordSpy: jest.SpyInstance;
Expand Down
21 changes: 8 additions & 13 deletions apps/browser/src/autofill/background/notification.background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -569,9 +569,7 @@ export default class NotificationBackground {
return;
}

const activeUserId = await firstValueFrom(
this.accountService.activeAccount$.pipe(map((a) => a?.id)),
);
const activeUserId = await firstValueFrom(this.activeUserId$);

const cipher = await this.cipherService.encrypt(newCipher, activeUserId);
try {
Expand Down Expand Up @@ -611,10 +609,8 @@ export default class NotificationBackground {
return;
}

const cipher = await this.cipherService.encrypt(
cipherView,
await firstValueFrom(this.activeUserId$),
);
const activeUserId = await firstValueFrom(this.activeUserId$);
const cipher = await this.cipherService.encrypt(cipherView, activeUserId);
try {
// We've only updated the password, no need to broadcast editedCipher message
await this.cipherService.updateWithServer(cipher);
Expand Down Expand Up @@ -646,17 +642,15 @@ export default class NotificationBackground {
if (Utils.isNullOrWhitespace(folderId) || folderId === "null") {
return false;
}

const folders = await firstValueFrom(this.folderService.folderViews$(this.activeUserId$));
const activeUserId = await firstValueFrom(this.activeUserId$);
const folders = await firstValueFrom(this.folderService.folderViews$(activeUserId));

Check warning on line 646 in apps/browser/src/autofill/background/notification.background.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/autofill/background/notification.background.ts#L645-L646

Added lines #L645 - L646 were not covered by tests
return folders.some((x) => x.id === folderId);
}

private async getDecryptedCipherById(cipherId: string) {
const cipher = await this.cipherService.get(cipherId);
if (cipher != null && cipher.type === CipherType.Login) {
const activeUserId = await firstValueFrom(
this.accountService.activeAccount$.pipe(map((a) => a?.id)),
);
const activeUserId = await firstValueFrom(this.activeUserId$);

Check warning on line 653 in apps/browser/src/autofill/background/notification.background.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/autofill/background/notification.background.ts#L653

Added line #L653 was not covered by tests

return await cipher.decrypt(
await this.cipherService.getKeyForCipherKeyDecryption(cipher, activeUserId),
Expand Down Expand Up @@ -696,7 +690,8 @@ export default class NotificationBackground {
* Returns the first value found from the folder service's folderViews$ observable.
*/
private async getFolderData() {
return await firstValueFrom(this.folderService.folderViews$(this.activeUserId$));
const activeUserId = await firstValueFrom(this.activeUserId$);
return await firstValueFrom(this.folderService.folderViews$(activeUserId));
}

private async getWebVaultUrl(): Promise<string> {
Expand Down
6 changes: 1 addition & 5 deletions apps/browser/src/background/main.background.ts
Original file line number Diff line number Diff line change
Expand Up @@ -833,11 +833,7 @@ export default class MainBackground {
this.cipherService,
this.stateProvider,
);
this.folderApiService = new FolderApiService(
this.folderService,
this.apiService,
this.accountService,
);
this.folderApiService = new FolderApiService(this.folderService, this.apiService);

this.userVerificationService = new UserVerificationService(
this.keyService,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ describe("AddEditFolderDialogComponent", () => {
it("deletes the folder", async () => {
await component.deleteFolder();

expect(deleteFolder).toHaveBeenCalledWith(folderView.id);
expect(deleteFolder).toHaveBeenCalledWith(folderView.id, "");
expect(showToast).toHaveBeenCalledWith({
variant: "success",
title: null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
} from "@angular/core";
import { takeUntilDestroyed } from "@angular/core/rxjs-interop";
import { FormBuilder, ReactiveFormsModule, Validators } from "@angular/forms";
import { firstValueFrom } from "rxjs";
import { firstValueFrom, map } from "rxjs";

import { JslibModule } from "@bitwarden/angular/jslib.module";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
Expand Down Expand Up @@ -65,6 +65,7 @@ export class AddEditFolderDialogComponent implements AfterViewInit, OnInit {
name: ["", Validators.required],
});

private activeUserId$ = this.accountService.activeAccount$.pipe(map((a) => a?.id));
private destroyRef = inject(DestroyRef);

constructor(
Expand Down Expand Up @@ -112,10 +113,10 @@ export class AddEditFolderDialogComponent implements AfterViewInit, OnInit {
this.folder.name = this.folderForm.controls.name.value;

try {
const activeUserId = await firstValueFrom(this.accountService.activeAccount$);
const userKey = await this.keyService.getUserKeyWithLegacySupport(activeUserId.id);
const activeUserId = await firstValueFrom(this.activeUserId$);
const userKey = await this.keyService.getUserKeyWithLegacySupport(activeUserId);
const folder = await this.folderService.encrypt(this.folder, userKey);
await this.folderApiService.save(folder);
await this.folderApiService.save(folder, activeUserId);

this.toastService.showToast({
variant: "success",
Expand All @@ -142,7 +143,8 @@ export class AddEditFolderDialogComponent implements AfterViewInit, OnInit {
}

try {
await this.folderApiService.delete(this.folder.id);
const activeUserId = await firstValueFrom(this.activeUserId$);
await this.folderApiService.delete(this.folder.id, activeUserId);
this.toastService.showToast({
variant: "success",
title: null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,60 +243,64 @@ export class VaultPopupListFiltersService {
/**
* Folder array structured to be directly passed to `ChipSelectComponent`
*/
folders$: Observable<ChipSelectOption<FolderView>[]> = combineLatest([
this.filters$.pipe(
distinctUntilChanged(
(previousFilter, currentFilter) =>
// Only update the collections when the organizationId filter changes
previousFilter.organization?.id === currentFilter.organization?.id,
),
),
this.folderService.folderViews$(this.activeUserId$),
this.cipherViews$,
]).pipe(
map(([filters, folders, cipherViews]): [PopupListFilter, FolderView[], CipherView[]] => {
if (folders.length === 1 && folders[0].id === null) {
// Do not display folder selections when only the "no folder" option is available.
return [filters, [], cipherViews];
}
folders$: Observable<ChipSelectOption<FolderView>[]> = this.activeUserId$.pipe(
switchMap((userId) =>
combineLatest([
this.filters$.pipe(
distinctUntilChanged(
(previousFilter, currentFilter) =>
// Only update the collections when the organizationId filter changes
previousFilter.organization?.id === currentFilter.organization?.id,
),
),
this.folderService.folderViews$(userId),
this.cipherViews$,
]).pipe(
map(([filters, folders, cipherViews]): [PopupListFilter, FolderView[], CipherView[]] => {
if (folders.length === 1 && folders[0].id === null) {
// Do not display folder selections when only the "no folder" option is available.
return [filters, [], cipherViews];
}

// Sort folders by alphabetic name
folders.sort(Utils.getSortFunction(this.i18nService, "name"));
let arrangedFolders = folders;
// Sort folders by alphabetic name
folders.sort(Utils.getSortFunction(this.i18nService, "name"));
let arrangedFolders = folders;

const noFolder = folders.find((f) => f.id === null);
const noFolder = folders.find((f) => f.id === null);

if (noFolder) {
// Update `name` of the "no folder" option to "Items with no folder"
noFolder.name = this.i18nService.t("itemsWithNoFolder");
if (noFolder) {
// Update `name` of the "no folder" option to "Items with no folder"
noFolder.name = this.i18nService.t("itemsWithNoFolder");

// Move the "no folder" option to the end of the list
arrangedFolders = [...folders.filter((f) => f.id !== null), noFolder];
}
return [filters, arrangedFolders, cipherViews];
}),
map(([filters, folders, cipherViews]) => {
const organizationId = filters.organization?.id ?? null;
// Move the "no folder" option to the end of the list
arrangedFolders = [...folders.filter((f) => f.id !== null), noFolder];
}
return [filters, arrangedFolders, cipherViews];
}),
map(([filters, folders, cipherViews]) => {
const organizationId = filters.organization?.id ?? null;

// When no org or "My vault" is selected, return all folders
if (organizationId === null || organizationId === MY_VAULT_ID) {
return folders;
}
// When no org or "My vault" is selected, return all folders
if (organizationId === null || organizationId === MY_VAULT_ID) {
return folders;
}

const orgCiphers = cipherViews.filter((c) => c.organizationId === organizationId);
const orgCiphers = cipherViews.filter((c) => c.organizationId === organizationId);

// Return only the folders that have ciphers within the filtered organization
return folders.filter((f) => orgCiphers.some((oc) => oc.folderId === f.id));
}),
map((folders) => {
const nestedFolders = this.getAllFoldersNested(folders);
return new DynamicTreeNode<FolderView>({
fullList: folders,
nestedList: nestedFolders,
});
}),
map((folders) =>
folders.nestedList.map((f) => this.convertToChipSelectOption(f, "bwi-folder")),
// Return only the folders that have ciphers within the filtered organization
return folders.filter((f) => orgCiphers.some((oc) => oc.folderId === f.id));
}),
map((folders) => {
const nestedFolders = this.getAllFoldersNested(folders);
return new DynamicTreeNode<FolderView>({
fullList: folders,
nestedList: nestedFolders,
});
}),
map((folders) =>
folders.nestedList.map((f) => this.convertToChipSelectOption(f, "bwi-folder")),
),
),
),
);

Expand Down
6 changes: 3 additions & 3 deletions apps/browser/src/vault/popup/settings/folders-v2.component.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { CommonModule } from "@angular/common";
import { Component } from "@angular/core";
import { map, Observable } from "rxjs";
import { map, Observable, switchMap } from "rxjs";

import { JslibModule } from "@bitwarden/angular/jslib.module";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
Expand Down Expand Up @@ -53,13 +53,13 @@ export class FoldersV2Component {
private dialogService: DialogService,
private accountService: AccountService,
) {
this.folders$ = this.folderService.folderViews$(this.activeUserId$).pipe(
this.folders$ = this.activeUserId$.pipe(
switchMap((userId) => this.folderService.folderViews$(userId)),
map((folders) => {
// Remove the last folder, which is the "no folder" option folder
if (folders.length > 0) {
return folders.slice(0, folders.length - 1);
}

return folders;
}),
);
Expand Down
9 changes: 5 additions & 4 deletions apps/browser/src/vault/popup/settings/folders.component.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Component } from "@angular/core";
import { Router } from "@angular/router";
import { map, Observable } from "rxjs";
import { map, Observable, switchMap } from "rxjs";

Check warning on line 3 in apps/browser/src/vault/popup/settings/folders.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/vault/popup/settings/folders.component.ts#L3

Added line #L3 was not covered by tests

import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";

Check warning on line 5 in apps/browser/src/vault/popup/settings/folders.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/vault/popup/settings/folders.component.ts#L5

Added line #L5 was not covered by tests
import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction";
Expand All @@ -20,12 +20,13 @@ export class FoldersComponent {
private router: Router,
private accountService: AccountService,

Check warning on line 21 in apps/browser/src/vault/popup/settings/folders.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/vault/popup/settings/folders.component.ts#L21

Added line #L21 was not covered by tests
) {
this.folders$ = this.folderService.folderViews$(this.activeUserId$).pipe(
this.folders$ = this.activeUserId$.pipe(
switchMap((userId) => this.folderService.folderViews$(userId)),

Check warning on line 24 in apps/browser/src/vault/popup/settings/folders.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/vault/popup/settings/folders.component.ts#L23-L24

Added lines #L23 - L24 were not covered by tests
map((folders) => {
// Remove the last folder, which is the "no folder" option folder
if (folders.length > 0) {
folders = folders.slice(0, folders.length - 1);
return folders.slice(0, folders.length - 1);

Check warning on line 28 in apps/browser/src/vault/popup/settings/folders.component.ts

View check run for this annotation

Codecov / codecov/patch

apps/browser/src/vault/popup/settings/folders.component.ts#L28

Added line #L28 was not covered by tests
}

return folders;
}),
);
Expand Down
10 changes: 5 additions & 5 deletions apps/cli/src/commands/edit.command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,20 +136,20 @@ export class EditCommand {
}

private async editFolder(id: string, req: FolderExport) {
const folder = await this.folderService.getFromState(id, this.activeUserId$);
const activeUserId = await firstValueFrom(this.activeUserId$);
const folder = await this.folderService.getFromState(id, activeUserId);

Check warning on line 140 in apps/cli/src/commands/edit.command.ts

View check run for this annotation

Codecov / codecov/patch

apps/cli/src/commands/edit.command.ts#L139-L140

Added lines #L139 - L140 were not covered by tests
if (folder == null) {
return Response.notFound();
}

let folderView = await folder.decrypt();
folderView = FolderExport.toView(req, folderView);

const activeUserId = await firstValueFrom(this.accountService.activeAccount$);
const userKey = await this.keyService.getUserKeyWithLegacySupport(activeUserId.id);
const userKey = await this.keyService.getUserKeyWithLegacySupport(activeUserId);

Check warning on line 148 in apps/cli/src/commands/edit.command.ts

View check run for this annotation

Codecov / codecov/patch

apps/cli/src/commands/edit.command.ts#L148

Added line #L148 was not covered by tests
const encFolder = await this.folderService.encrypt(folderView, userKey);
try {
await this.folderApiService.save(encFolder);
const updatedFolder = await this.folderService.get(folder.id, this.activeUserId$);
await this.folderApiService.save(encFolder, activeUserId);
const updatedFolder = await this.folderService.get(folder.id, activeUserId);

Check warning on line 152 in apps/cli/src/commands/edit.command.ts

View check run for this annotation

Codecov / codecov/patch

apps/cli/src/commands/edit.command.ts#L151-L152

Added lines #L151 - L152 were not covered by tests
const decFolder = await updatedFolder.decrypt();
const res = new FolderResponse(decFolder);
return Response.success(res);
Expand Down
Loading

0 comments on commit 47364bf

Please sign in to comment.