diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index f8f86e6a277..d00c5de8109 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -845,7 +845,6 @@ export default class MainBackground { this.domainSettingsService, this.apiService, this.i18nService, - this.searchService, this.stateService, this.autofillSettingsService, this.encryptService, diff --git a/apps/web/src/app/vault/individual-vault/vault.component.ts b/apps/web/src/app/vault/individual-vault/vault.component.ts index d4441c73719..6c88ea7c840 100644 --- a/apps/web/src/app/vault/individual-vault/vault.component.ts +++ b/apps/web/src/app/vault/individual-vault/vault.component.ts @@ -59,11 +59,13 @@ import { Utils } from "@bitwarden/common/platform/misc/utils"; import { SyncService } from "@bitwarden/common/platform/sync"; import { CipherId, CollectionId, OrganizationId, UserId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction"; import { TotpService } from "@bitwarden/common/vault/abstractions/totp.service"; import { CipherType } from "@bitwarden/common/vault/enums"; import { CipherRepromptType } from "@bitwarden/common/vault/enums/cipher-reprompt-type"; import { TreeNode } from "@bitwarden/common/vault/models/domain/tree-node"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { FilterService } from "@bitwarden/common/vault/search/filter.service"; import { ServiceUtils } from "@bitwarden/common/vault/service-utils"; import { DialogService, Icons, ToastService } from "@bitwarden/components"; import { @@ -114,7 +116,6 @@ import { VaultFilterComponent } from "./vault-filter/components/vault-filter.com import { VaultFilterService } from "./vault-filter/services/abstractions/vault-filter.service"; import { RoutedVaultFilterBridgeService } from "./vault-filter/services/routed-vault-filter-bridge.service"; import { RoutedVaultFilterService } from "./vault-filter/services/routed-vault-filter.service"; -import { createFilterFunction } from "./vault-filter/shared/models/filter-function"; import { All, RoutedVaultFilterModel, @@ -259,6 +260,7 @@ export class VaultComponent implements OnInit, OnDestroy { private totpService: TotpService, private eventCollectionService: EventCollectionService, private searchService: SearchService, + private filterService: FilterService, private searchPipe: SearchPipe, private apiService: ApiService, private billingAccountProfileStateService: BillingAccountProfileStateService, @@ -270,6 +272,7 @@ export class VaultComponent implements OnInit, OnDestroy { private trialFlowService: TrialFlowService, private organizationBillingService: OrganizationBillingServiceAbstraction, private billingNotificationService: BillingNotificationService, + private folderService: FolderService, ) {} async ngOnInit() { @@ -342,30 +345,30 @@ export class VaultComponent implements OnInit, OnDestroy { this.currentSearchText$ = this.route.queryParams.pipe(map((queryParams) => queryParams.search)); - const ciphers$ = combineLatest([ - this.cipherService.cipherViews$(activeUserId).pipe(filter((c) => c !== null)), - filter$, - this.currentSearchText$, - ]).pipe( - filter(([ciphers, filter]) => ciphers != undefined && filter != undefined), - concatMap(async ([ciphers, filter, searchText]) => { - const failedCiphers = await firstValueFrom( - this.cipherService.failedToDecryptCiphers$(activeUserId), - ); - const filterFunction = createFilterFunction(filter); - // Append any failed to decrypt ciphers to the top of the cipher list - const allCiphers = [...failedCiphers, ...ciphers]; + const context$ = this.accountService.activeAccount$.pipe( + switchMap((account) => { + return this.filterService.context$({ + ciphers: this.cipherService.cipherViews$(account.id), + folders: this.folderService.folderViews$(account.id), + collections: this.collectionService.decryptedCollections$, + organizations: this.organizationService.organizations$(account.id), + }); + }), + ); - if (await this.searchService.isSearchable(activeUserId, searchText)) { - return await this.searchService.searchCiphers( - activeUserId, - searchText, - [filterFunction], - allCiphers, - ); + const ciphers$ = combineLatest([this.currentSearchText$, context$]).pipe( + switchMap(([currentText, context]) => { + return this.filterService.filter(of([currentText, context])); + }), + // this.filterService.filter, + switchMap((result) => { + if (result.isError) { + // return all ciphers + return context$.pipe(map((c) => c.ciphers)); + } else { + // return filtered ciphers + return of(result.ciphers); } - - return allCiphers.filter(filterFunction); }), shareReplay({ refCount: true, bufferSize: 1 }), ); @@ -391,14 +394,15 @@ export class VaultComponent implements OnInit, OnDestroy { collectionsToReturn = selectedCollection?.children.map((c) => c.node) ?? []; } - if (await this.searchService.isSearchable(activeUserId, searchText)) { - collectionsToReturn = this.searchPipe.transform( - collectionsToReturn, - searchText, - (collection) => collection.name, - (collection) => collection.id, - ); - } + // TODO: Not sure what this was doing, we probably want to reproduce this + // if (await this.searchService.isSearchable(activeUserId, searchText)) { + // collectionsToReturn = this.searchPipe.transform( + // collectionsToReturn, + // searchText, + // (collection) => collection.name, + // (collection) => collection.id, + // ); + // } return collectionsToReturn; }), diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 04bcb42aa53..488d6690cce 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -269,6 +269,7 @@ import { } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction"; import { TotpService as TotpServiceAbstraction } from "@bitwarden/common/vault/abstractions/totp.service"; import { VaultSettingsService as VaultSettingsServiceAbstraction } from "@bitwarden/common/vault/abstractions/vault-settings/vault-settings.service"; +import { DefaultFilterService, FilterService } from "@bitwarden/common/vault/search/filter.service"; import { CipherAuthorizationService, DefaultCipherAuthorizationService, @@ -491,42 +492,12 @@ const safeProviders: SafeProvider[] = [ }), safeProvider({ provide: CipherServiceAbstraction, - useFactory: ( - keyService: KeyService, - domainSettingsService: DomainSettingsService, - apiService: ApiServiceAbstraction, - i18nService: I18nServiceAbstraction, - searchService: SearchServiceAbstraction, - stateService: StateServiceAbstraction, - autofillSettingsService: AutofillSettingsServiceAbstraction, - encryptService: EncryptService, - bulkEncryptService: BulkEncryptService, - fileUploadService: CipherFileUploadServiceAbstraction, - configService: ConfigService, - stateProvider: StateProvider, - accountService: AccountServiceAbstraction, - ) => - new CipherService( - keyService, - domainSettingsService, - apiService, - i18nService, - searchService, - stateService, - autofillSettingsService, - encryptService, - bulkEncryptService, - fileUploadService, - configService, - stateProvider, - accountService, - ), + useClass: CipherService, deps: [ KeyService, DomainSettingsService, ApiServiceAbstraction, I18nServiceAbstraction, - SearchServiceAbstraction, StateServiceAbstraction, AutofillSettingsServiceAbstraction, EncryptService, @@ -1495,6 +1466,11 @@ const safeProviders: SafeProvider[] = [ useClass: MasterPasswordApiService, deps: [ApiServiceAbstraction, LogService], }), + safeProvider({ + provide: FilterService, + useClass: DefaultFilterService, + deps: [], + }), ]; @NgModule({ diff --git a/libs/angular/src/vault/components/vault-items.component.ts b/libs/angular/src/vault/components/vault-items.component.ts index f7280cb74b3..783a643d44b 100644 --- a/libs/angular/src/vault/components/vault-items.component.ts +++ b/libs/angular/src/vault/components/vault-items.component.ts @@ -1,7 +1,8 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore import { Directive, EventEmitter, Input, OnDestroy, OnInit, Output } from "@angular/core"; -import { BehaviorSubject, Subject, firstValueFrom, from, switchMap, takeUntil } from "rxjs"; +import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; +import { BehaviorSubject, Observable, Subject, firstValueFrom, map } from "rxjs"; import { SearchService } from "@bitwarden/common/abstractions/search.service"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; @@ -10,6 +11,8 @@ import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { UserId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { FilterService } from "@bitwarden/common/vault/search/filter.service"; +import { ProcessInstructions } from "@bitwarden/common/vault/search/query.types"; @Directive() export class VaultItemsComponent implements OnInit, OnDestroy { @@ -31,6 +34,7 @@ export class VaultItemsComponent implements OnInit, OnDestroy { private destroy$ = new Subject(); private searchTimeout: any = null; private isSearchable: boolean = false; + private processInstructions$: Observable; private _searchText$ = new BehaviorSubject(""); get searchText() { return this._searchText$.value; @@ -40,6 +44,7 @@ export class VaultItemsComponent implements OnInit, OnDestroy { } constructor( + protected filterService: FilterService, protected searchService: SearchService, protected cipherService: CipherService, protected accountService: AccountService, @@ -48,14 +53,12 @@ export class VaultItemsComponent implements OnInit, OnDestroy { async ngOnInit() { this.userId = await firstValueFrom(getUserId(this.accountService.activeAccount$)); - this._searchText$ - .pipe( - switchMap((searchText) => from(this.searchService.isSearchable(this.userId, searchText))), - takeUntil(this.destroy$), - ) - .subscribe((isSearchable) => { - this.isSearchable = isSearchable; - }); + const parsed$ = this.filterService.parse(this._searchText$).pipe(takeUntilDestroyed()); + + this.processInstructions$ = parsed$.pipe(map((p) => p.processInstructions)); + parsed$.subscribe(({ isError }) => { + this.isSearchable = !isError; + }); } ngOnDestroy(): void { @@ -135,11 +138,6 @@ export class VaultItemsComponent implements OnInit, OnDestroy { indexedCiphers = [...failedCiphers, ...indexedCiphers]; } - this.ciphers = await this.searchService.searchCiphers( - this.userId, - this.searchText, - [this.filter, this.deletedFilter], - indexedCiphers, - ); + this.ciphers = await firstValueFrom(this.filterService.filter(this.processInstructions$)); } } diff --git a/libs/common/src/vault/search/ast.ts b/libs/common/src/vault/search/ast.ts index 34f69349cfa..eb4a5e97151 100644 --- a/libs/common/src/vault/search/ast.ts +++ b/libs/common/src/vault/search/ast.ts @@ -34,6 +34,7 @@ export type AstNode = | IsFavorite; type AstNodeBase = { + d: object[]; type: AstNodeType; start: number; end: number; @@ -41,7 +42,7 @@ type AstNodeBase = { }; export type Search = AstNodeBase & { type: "search"; - d: Or; + contents: Or; }; export function isSearch(x: AstNode): x is Search { diff --git a/libs/common/src/vault/search/filter.service.ts b/libs/common/src/vault/search/filter.service.ts index 26a7a3fae91..2f3ab2c3532 100644 --- a/libs/common/src/vault/search/filter.service.ts +++ b/libs/common/src/vault/search/filter.service.ts @@ -1,90 +1,105 @@ import * as lunr from "lunr"; -import { - combineLatest, - combineLatestWith, - map, - NEVER, - Observable, - of, - OperatorFunction, - pipe, - switchMap, -} from "rxjs"; +import { combineLatest, map, Observable, of, OperatorFunction, pipe, switchMap } from "rxjs"; -// eslint-disable-next-line no-restricted-imports -- temp poc, so we're ignoring the circular package dependency. We'll need to move this to the vault team's package -import { CollectionService } from "@bitwarden/admin-console/src/common/collections/abstractions"; - -import { OrganizationService } from "../../admin-console/abstractions/organization/organization.service.abstraction"; -import { AccountService } from "../../auth/abstractions/account.service"; import { UriMatchStrategy } from "../../models/domain/domain-service"; -import { CipherService } from "../abstractions/cipher.service"; -import { FolderService } from "../abstractions/folder/folder.service.abstraction"; import { CipherType, FieldType } from "../enums"; import { CipherView } from "../models/view/cipher.view"; import { parseQuery } from "./parse"; -import { ProcessInstructions, SearchContext } from "./query.types"; +import { + FilterResult, + ObservableSearchContextInput, + ParseResult, + SearchContext, +} from "./query.types"; -export class FilterService { +export abstract class FilterService { + abstract readonly parse: OperatorFunction; + abstract readonly filter: OperatorFunction<[string | ParseResult, SearchContext], FilterResult>; + abstract context$(context: ObservableSearchContextInput): Observable; +} + +export class DefaultFilterService implements FilterService { private static registeredPipeline = false; - searchContext$: Observable; - constructor( - accountService: AccountService, - cipherService: CipherService, - organizationService: OrganizationService, - folderService: FolderService, - collectionService: CollectionService, - ) { - const activeAccount$ = accountService.activeAccount$.pipe( - switchMap((account) => { - return account ? of(account) : NEVER; - }), - ); - const viewsAndIndex$ = activeAccount$.pipe( - switchMap((account) => - cipherService - .cipherViews$(account.id) - .pipe(switchMap((views) => of([views, this.buildIndex(views)] as const))), - ), - ); - - this.searchContext$ = activeAccount$.pipe( - switchMap((account) => { - const userId = account.id; - return combineLatest([ - viewsAndIndex$, - organizationService.organizations$(userId), - folderService.folderViews$(userId), - collectionService.decryptedCollections$, - ]); - }), - switchMap(([[ciphers, index], organizations, folders, collections]) => { - return of({ - ciphers, - index, - organizations, - folders, - collections, - }); - }), - ); - + constructor() { // Currently have to ensure this is only done a single time. Lunr allows you to register a function // multiple times but they will add a warning message to the console. The way they do that breaks when ran on a service worker. - if (!FilterService.registeredPipeline) { - FilterService.registeredPipeline = true; + if (!DefaultFilterService.registeredPipeline) { + DefaultFilterService.registeredPipeline = true; //register lunr pipeline function lunr.Pipeline.registerFunction(this.normalizeAccentsPipelineFunction, "normalizeAccents"); } } - parse = map((query: string) => parseQuery(query)); + context$(context: ObservableSearchContextInput): Observable { + return combineLatest([ + context.ciphers, + context.organizations, + context.folders, + context.collections, + ]).pipe( + map(([ciphers, organizations, folders, collections]) => { + return { + ciphers, + organizations, + folders, + collections, + index: this.buildIndex(ciphers), + }; + }), + ); + } - filter(): OperatorFunction { + get parse() { + return (query: Observable | null | undefined) => { + if (query == null) { + return of({ + isError: true as const, + processInstructions: null, + }); + } else { + return query.pipe( + map((query: string) => { + try { + return { + isError: false as const, + processInstructions: parseQuery(query), + }; + } catch { + return { + isError: true as const, + processInstructions: null, + }; + } + }), + ); + } + }; + } + + get filter(): OperatorFunction<[string | ParseResult, SearchContext], FilterResult> { return pipe( - combineLatestWith(this.searchContext$), - map(([instructions, context]) => { - return instructions.filter(context).ciphers; + switchMap(([queryOrParsedQuery, context]) => { + if (queryOrParsedQuery == null || typeof queryOrParsedQuery === "string") { + // it's a string that needs parsing + return combineLatest([this.parse(of(queryOrParsedQuery as string)), of(context)]); + } else { + // It's a parsed query + return combineLatest([of(queryOrParsedQuery), of(context)]); + } + }), + map(([parseResult, context]) => { + if (parseResult.isError) { + return { + ...parseResult, + ciphers: null, + }; + } else { + return { + ...parseResult, + ciphers: parseResult.processInstructions.filter(context).ciphers, + }; + } }), ); } @@ -154,7 +169,7 @@ export class FilterService { const checkFields = fields.every((i: any) => searchableFields.includes(i)); if (checkFields) { - return FilterService.normalizeSearchQuery(token.toString()); + return DefaultFilterService.normalizeSearchQuery(token.toString()); } return token; diff --git a/libs/common/src/vault/search/parse.ts b/libs/common/src/vault/search/parse.ts index 43d3564e7d8..52aeeb5eab5 100644 --- a/libs/common/src/vault/search/parse.ts +++ b/libs/common/src/vault/search/parse.ts @@ -20,23 +20,26 @@ import { import grammar from "./bitwarden-query-grammar"; import { ProcessInstructions } from "./query.types"; +export const PARSE_ERROR = new Error("Invalid search query"); + export function parseQuery(query: string): ProcessInstructions { const parser = new Parser(Grammar.fromCompiled(grammar)); parser.feed(query); if (!parser.results) { // TODO: Better error handling // there should be some invalid token information - throw new Error("Invalid search query"); + throw PARSE_ERROR; } const result = parser.results[0] as AstNode; - return handleNode(result); + const parsed = handleNode(result); + return parsed; } function handleNode(node: AstNode): ProcessInstructions { if (isSearch(node)) { - return handleNode(node.d); + return handleNode(node.contents); } else if (isOr(node)) { const left = handleNode(node.left); const right = handleNode(node.right); diff --git a/libs/common/src/vault/search/query.types.ts b/libs/common/src/vault/search/query.types.ts index a8475c0d43a..2470f6e2cfc 100644 --- a/libs/common/src/vault/search/query.types.ts +++ b/libs/common/src/vault/search/query.types.ts @@ -1,4 +1,5 @@ import lunr from "lunr"; +import { Observable } from "rxjs"; import { CollectionView } from "@bitwarden/admin-console/common"; @@ -8,6 +9,28 @@ import { FolderView } from "../models/view/folder.view"; import { AstNodeType } from "./ast"; +export type ParseResult = + | { + processInstructions: ProcessInstructions; + isError: false; + } + | { + processInstructions: null; + isError: true; + }; + +export type FilterResult = + | { + ciphers: CipherView[]; + processInstructions: ProcessInstructions; + isError: false; + } + | { + ciphers: null; + processInstructions: null; + isError: true; + }; + export type ProcessInstructions = { filter: (context: SearchContext) => SearchContext; sections: { start: number; end: number; type: AstNodeType }[]; @@ -20,3 +43,7 @@ export type SearchContext = { organizations: Organization[]; index: lunr.Index; }; + +export type ObservableSearchContextInput = { + [key in keyof Omit]: Observable; +}; diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts index d774277c4a0..507b223facc 100644 --- a/libs/common/src/vault/services/cipher.service.ts +++ b/libs/common/src/vault/services/cipher.service.ts @@ -17,7 +17,6 @@ import { SemVer } from "semver"; import { KeyService } from "@bitwarden/key-management"; import { ApiService } from "../../abstractions/api.service"; -import { SearchService } from "../../abstractions/search.service"; import { AccountService } from "../../auth/abstractions/account.service"; import { AutofillSettingsServiceAbstraction } from "../../autofill/services/autofill-settings.service"; import { DomainSettingsService } from "../../autofill/services/domain-settings.service"; @@ -102,7 +101,6 @@ export class CipherService implements CipherServiceAbstraction { private domainSettingsService: DomainSettingsService, private apiService: ApiService, private i18nService: I18nService, - private searchService: SearchService, private stateService: StateService, private autofillSettingsService: AutofillSettingsServiceAbstraction, private encryptService: EncryptService, @@ -163,13 +161,6 @@ export class CipherService implements CipherServiceAbstraction { if (value == null || value.length !== 0) { await this.setDecryptedCiphers(value, userId); } - if (this.searchService != null) { - if (value == null) { - await this.searchService.clearIndex(userId); - } else { - await this.searchService.indexCiphers(userId, value); - } - } } async setFailedDecryptedCiphers(cipherViews: CipherView[], userId: UserId) { @@ -394,7 +385,6 @@ export class CipherService implements CipherServiceAbstraction { async getAllDecrypted(userId: UserId): Promise { const decCiphers = await this.getDecryptedCiphers(userId); if (decCiphers != null && decCiphers.length !== 0) { - await this.reindexCiphers(userId); return await this.getDecryptedCiphers(userId); } @@ -477,15 +467,6 @@ export class CipherService implements CipherServiceAbstraction { ); } - private async reindexCiphers(userId: UserId) { - const reindexRequired = - this.searchService != null && - ((await firstValueFrom(this.searchService.indexedEntityId$(userId))) ?? userId) !== userId; - if (reindexRequired) { - await this.searchService.indexCiphers(userId, await this.getDecryptedCiphers(userId), userId); - } - } - async getAllDecryptedForGrouping( groupingId: string, userId: UserId,