From 8cf490a8c16dde64eb11c4dce8f0af412932a6d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa?= Date: Mon, 24 Feb 2025 17:17:27 +0100 Subject: [PATCH 001/119] Extract common cargo info and edition 2024 fix (#13507) --- apps/desktop/desktop_native/Cargo.toml | 6 ++++++ apps/desktop/desktop_native/core/Cargo.toml | 14 ++++++------- .../desktop_native/macos_provider/Cargo.toml | 14 ++++++------- apps/desktop/desktop_native/napi/Cargo.toml | 20 +++++++++---------- apps/desktop/desktop_native/objc/Cargo.toml | 14 ++++++------- apps/desktop/desktop_native/objc/src/lib.rs | 8 ++++---- apps/desktop/desktop_native/proxy/Cargo.toml | 13 ++++++------ .../windows-plugin-authenticator/Cargo.toml | 8 ++++---- 8 files changed, 51 insertions(+), 46 deletions(-) diff --git a/apps/desktop/desktop_native/Cargo.toml b/apps/desktop/desktop_native/Cargo.toml index 78142d618ed..5478d397651 100644 --- a/apps/desktop/desktop_native/Cargo.toml +++ b/apps/desktop/desktop_native/Cargo.toml @@ -2,6 +2,12 @@ resolver = "2" members = ["napi", "core", "proxy", "macos_provider", "windows-plugin-authenticator"] +[workspace.package] +version = "0.0.0" +license = "GPL-3.0" +edition = "2021" +publish = false + [workspace.dependencies] anyhow = "=1.0.94" log = "=0.4.25" diff --git a/apps/desktop/desktop_native/core/Cargo.toml b/apps/desktop/desktop_native/core/Cargo.toml index 03b5a60b8b1..c821662f112 100644 --- a/apps/desktop/desktop_native/core/Cargo.toml +++ b/apps/desktop/desktop_native/core/Cargo.toml @@ -1,9 +1,9 @@ [package] -edition = "2021" -license = "GPL-3.0" name = "desktop_core" -version = "0.0.0" -publish = false +edition = { workspace = true } +license = { workspace = true } +version = { workspace = true } +publish = { workspace = true } [features] default = [ @@ -19,7 +19,7 @@ manual_test = [] [dependencies] aes = "=0.8.4" -anyhow.workspace = true +anyhow = { workspace = true } arboard = { version = "=3.4.1", default-features = false, features = [ "wayland-data-control", ] } @@ -32,7 +32,7 @@ pin-project = "=1.1.8" dirs = "=6.0.0" futures = "=0.3.31" interprocess = { version = "=2.2.1", features = ["tokio"] } -log.workspace = true +log = { workspace = true } rand = "=0.8.5" russh-cryptovec = "=0.7.3" scopeguard = "=1.2.0" @@ -48,7 +48,7 @@ bitwarden-russh = { git = "https://github.com/bitwarden/bitwarden-russh.git", re tokio = { workspace = true, features = ["io-util", "sync", "macros", "net"] } tokio-stream = { workspace = true, features = ["net"] } tokio-util = { workspace = true, features = ["codec"] } -thiserror.workspace = true +thiserror = { workspace = true } typenum = "=1.17.0" pkcs8 = { version = "=0.10.2", features = ["alloc", "encryption", "pem"] } rsa = "=0.9.6" diff --git a/apps/desktop/desktop_native/macos_provider/Cargo.toml b/apps/desktop/desktop_native/macos_provider/Cargo.toml index a488ce88a1c..e160f7f35d6 100644 --- a/apps/desktop/desktop_native/macos_provider/Cargo.toml +++ b/apps/desktop/desktop_native/macos_provider/Cargo.toml @@ -1,9 +1,9 @@ [package] name = "macos_provider" -license = "GPL-3.0" -version = "0.0.0" -edition = "2021" -publish = false +edition = { workspace = true } +license = { workspace = true } +version = { workspace = true } +publish = { workspace = true } [[bin]] name = "uniffi-bindgen" @@ -16,11 +16,11 @@ bench = false [dependencies] desktop_core = { path = "../core" } futures = "=0.3.31" -log.workspace = true +log = { workspace = true } serde = { workspace = true, features = ["derive"] } -serde_json.workspace = true +serde_json = { workspace = true } tokio = { workspace = true, features = ["sync"] } -tokio-util.workspace = true +tokio-util = { workspace = true } uniffi = { version = "=0.28.3", features = ["cli"] } [target.'cfg(target_os = "macos")'.dependencies] diff --git a/apps/desktop/desktop_native/napi/Cargo.toml b/apps/desktop/desktop_native/napi/Cargo.toml index 73afee72b55..a9513fa49a8 100644 --- a/apps/desktop/desktop_native/napi/Cargo.toml +++ b/apps/desktop/desktop_native/napi/Cargo.toml @@ -1,10 +1,10 @@ [package] -edition = "2021" -exclude = ["index.node"] -license = "GPL-3.0" name = "desktop_napi" -version = "0.0.0" -publish = false +exclude = ["index.node"] +edition = { workspace = true } +license = { workspace = true } +version = { workspace = true } +publish = { workspace = true } [lib] crate-type = ["cdylib"] @@ -16,15 +16,15 @@ manual_test = [] [dependencies] base64 = "=0.22.1" hex = "=0.4.3" -anyhow = "=1.0.94" +anyhow = { workspace = true } desktop_core = { path = "../core" } napi = { version = "=2.16.15", features = ["async"] } napi-derive = "=2.16.13" serde = { workspace = true, features = ["derive"] } -serde_json.workspace = true -tokio.workspace = true -tokio-util.workspace = true -tokio-stream.workspace = true +serde_json = { workspace = true } +tokio = { workspace = true } +tokio-util = { workspace = true } +tokio-stream = { workspace = true } [target.'cfg(windows)'.dependencies] windows-registry = "=0.4.0" diff --git a/apps/desktop/desktop_native/objc/Cargo.toml b/apps/desktop/desktop_native/objc/Cargo.toml index 2d6478c77c9..07961f8e13f 100644 --- a/apps/desktop/desktop_native/objc/Cargo.toml +++ b/apps/desktop/desktop_native/objc/Cargo.toml @@ -1,17 +1,17 @@ [package] -edition = "2021" -license = "GPL-3.0" name = "desktop_objc" -version = "0.0.0" -publish = false +edition = { workspace = true } +license = { workspace = true } +version = { workspace = true } +publish = { workspace = true } [features] default = [] [dependencies] -anyhow.workspace = true -thiserror.workspace = true -tokio.workspace = true +anyhow = { workspace = true } +thiserror = { workspace = true } +tokio = { workspace = true } [target.'cfg(target_os = "macos")'.dependencies] core-foundation = "=0.10.0" diff --git a/apps/desktop/desktop_native/objc/src/lib.rs b/apps/desktop/desktop_native/objc/src/lib.rs index eb969cb5f56..f5a7623cfc3 100644 --- a/apps/desktop/desktop_native/objc/src/lib.rs +++ b/apps/desktop/desktop_native/objc/src/lib.rs @@ -68,13 +68,13 @@ mod objc { use super::*; - extern "C" { - pub fn runCommand(context: *mut c_void, value: *const c_char); - pub fn freeObjCString(value: &ObjCString); + unsafe extern "C" { + pub unsafe fn runCommand(context: *mut c_void, value: *const c_char); + pub unsafe fn freeObjCString(value: &ObjCString); } /// This function is called from the ObjC code to return the output of the command - #[no_mangle] + #[unsafe(no_mangle)] pub extern "C" fn commandReturn(context: &mut CommandContext, value: ObjCString) -> bool { let value: String = match value.try_into() { Ok(value) => value, diff --git a/apps/desktop/desktop_native/proxy/Cargo.toml b/apps/desktop/desktop_native/proxy/Cargo.toml index 27f2856f3a6..10ff1ed8eae 100644 --- a/apps/desktop/desktop_native/proxy/Cargo.toml +++ b/apps/desktop/desktop_native/proxy/Cargo.toml @@ -1,16 +1,15 @@ [package] -edition = "2021" -exclude = ["index.node"] -license = "GPL-3.0" name = "desktop_proxy" -version = "0.0.0" -publish = false +edition = { workspace = true } +license = { workspace = true } +version = { workspace = true } +publish = { workspace = true } [dependencies] -anyhow.workspace = true +anyhow = { workspace = true } desktop_core = { path = "../core" } futures = "=0.3.31" -log.workspace = true +log = { workspace = true } simplelog = "=0.12.2" tokio = { workspace = true, features = ["io-std", "io-util", "macros", "rt"] } tokio-util = { workspace = true, features = ["codec"] } diff --git a/apps/desktop/desktop_native/windows-plugin-authenticator/Cargo.toml b/apps/desktop/desktop_native/windows-plugin-authenticator/Cargo.toml index b8759cfca3f..3443fed4fd6 100644 --- a/apps/desktop/desktop_native/windows-plugin-authenticator/Cargo.toml +++ b/apps/desktop/desktop_native/windows-plugin-authenticator/Cargo.toml @@ -1,9 +1,9 @@ [package] name = "windows-plugin-authenticator" -version = "0.0.0" -edition = "2021" -license = "GPL-3.0" -publish = false +edition = { workspace = true } +license = { workspace = true } +version = { workspace = true } +publish = { workspace = true } [target.'cfg(target_os = "windows")'.build-dependencies] bindgen = "0.71.1" From 030acc6421a53771dc35336cfe3c426a32538613 Mon Sep 17 00:00:00 2001 From: Jordan Aasen <166539328+jaasen-livefront@users.noreply.github.com> Date: Mon, 24 Feb 2025 09:13:02 -0800 Subject: [PATCH 002/119] [PM-16133] - [Vault] Implement persistence on filters in Vault view (#13303) * WIP - cache vault filter/search * wip - vault filter persistence * finalize popup list filters. wip tests * finalize tests * rename test to mock * add remaining specs * fix types. remove use of observable * fix type error * remove unecessary check for undefined userId * remove observable. fix types and tests * fix tests --- .../vault-header-v2.component.spec.ts | 2 +- .../vault-search/vault-v2-search.component.ts | 8 +- .../vault-popup-items.service.spec.ts | 42 +++- .../services/vault-popup-items.service.ts | 43 ++-- .../vault-popup-list-filters.service.spec.ts | 230 +++++++++++++++++- .../vault-popup-list-filters.service.ts | 133 ++++++++-- 6 files changed, 403 insertions(+), 55 deletions(-) diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-header/vault-header-v2.component.spec.ts b/apps/browser/src/vault/popup/components/vault-v2/vault-header/vault-header-v2.component.spec.ts index fb5ac4b3391..cda055176e8 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-header/vault-header-v2.component.spec.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-header/vault-header-v2.component.spec.ts @@ -77,7 +77,7 @@ describe("VaultHeaderV2Component", () => { { provide: LogService, useValue: mock() }, { provide: VaultPopupItemsService, - useValue: mock({ latestSearchText$: new BehaviorSubject("") }), + useValue: mock({ searchText$: new BehaviorSubject("") }), }, { provide: SyncService, diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-search/vault-v2-search.component.ts b/apps/browser/src/vault/popup/components/vault-v2/vault-search/vault-v2-search.component.ts index d9b310da231..32f5611f436 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-search/vault-v2-search.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-search/vault-v2-search.component.ts @@ -1,5 +1,3 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { CommonModule } from "@angular/common"; import { Component } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; @@ -20,7 +18,7 @@ const SearchTextDebounceInterval = 200; templateUrl: "vault-v2-search.component.html", }) export class VaultV2SearchComponent { - searchText: string; + searchText: string = ""; private searchText$ = new Subject(); @@ -30,11 +28,11 @@ export class VaultV2SearchComponent { } onSearchTextChanged() { - this.searchText$.next(this.searchText); + this.vaultPopupItemsService.applyFilter(this.searchText); } subscribeToLatestSearchText(): Subscription { - return this.vaultPopupItemsService.latestSearchText$ + return this.vaultPopupItemsService.searchText$ .pipe( takeUntilDestroyed(), filter((data) => !!data), diff --git a/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts b/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts index 6d7b7b57d23..4d7957930ab 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts @@ -1,4 +1,5 @@ -import { TestBed } from "@angular/core/testing"; +import { WritableSignal, signal } from "@angular/core"; +import { TestBed, discardPeriodicTasks, fakeAsync, tick } from "@angular/core/testing"; import { mock } from "jest-mock-extended"; import { BehaviorSubject, firstValueFrom, timeout } from "rxjs"; @@ -21,6 +22,7 @@ import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { InlineMenuFieldQualificationService } from "../../../autofill/services/inline-menu-field-qualification.service"; import { BrowserApi } from "../../../platform/browser/browser-api"; +import { PopupViewCacheService } from "../../../platform/popup/view-cache/popup-view-cache.service"; import { VaultPopupAutofillService } from "./vault-popup-autofill.service"; import { VaultPopupItemsService } from "./vault-popup-items.service"; @@ -35,6 +37,10 @@ describe("VaultPopupItemsService", () => { let mockOrg: Organization; let mockCollections: CollectionView[]; let activeUserLastSync$: BehaviorSubject; + let viewCacheService: { + signal: jest.Mock; + mockSignal: WritableSignal; + }; let ciphersSubject: BehaviorSubject>; let localDataSubject: BehaviorSubject>; @@ -125,6 +131,12 @@ describe("VaultPopupItemsService", () => { activeUserLastSync$ = new BehaviorSubject(new Date()); syncServiceMock.activeUserLastSync$.mockReturnValue(activeUserLastSync$); + const testSearchSignal = createMockSignal(""); + viewCacheService = { + mockSignal: testSearchSignal, + signal: jest.fn((options) => testSearchSignal), + }; + testBed = TestBed.configureTestingModule({ providers: [ { provide: CipherService, useValue: cipherServiceMock }, @@ -141,6 +153,7 @@ describe("VaultPopupItemsService", () => { provide: InlineMenuFieldQualificationService, useValue: inlineMenuFieldQualificationServiceMock, }, + { provide: PopupViewCacheService, useValue: viewCacheService }, ], }); @@ -455,15 +468,32 @@ describe("VaultPopupItemsService", () => { describe("applyFilter", () => { it("should call search Service with the new search term", (done) => { const searchText = "Hello"; - service.applyFilter(searchText); const searchServiceSpy = jest.spyOn(searchService, "searchCiphers"); + service.applyFilter(searchText); service.favoriteCiphers$.subscribe(() => { - expect(searchServiceSpy).toHaveBeenCalledWith(searchText, null, expect.anything()); + expect(searchServiceSpy).toHaveBeenCalledWith(searchText, undefined, expect.anything()); done(); }); }); }); + + it("should update searchText$ when applyFilter is called", fakeAsync(() => { + let latestValue: string | null; + service.searchText$.subscribe((val) => { + latestValue = val; + }); + tick(); + expect(latestValue!).toEqual(""); + + service.applyFilter("test search"); + tick(); + expect(latestValue!).toEqual("test search"); + + expect(viewCacheService.mockSignal()).toEqual("test search"); + + discardPeriodicTasks(); + })); }); // A function to generate a list of ciphers of different types @@ -518,3 +548,9 @@ function cipherFactory(count: number): Record { } return Object.fromEntries(ciphers.map((c) => [c.id, c])); } + +function createMockSignal(initialValue: T): WritableSignal { + const s = signal(initialValue); + s.set = (value: T) => s.update(() => value); + return s; +} diff --git a/apps/browser/src/vault/popup/services/vault-popup-items.service.ts b/apps/browser/src/vault/popup/services/vault-popup-items.service.ts index c71c2ca32c3..22d4b2eca36 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-items.service.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-items.service.ts @@ -1,8 +1,6 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore -import { Injectable, NgZone } from "@angular/core"; +import { inject, Injectable, NgZone } from "@angular/core"; +import { toObservable } from "@angular/core/rxjs-interop"; import { - BehaviorSubject, combineLatest, concatMap, distinctUntilChanged, @@ -27,13 +25,14 @@ import { OrganizationService } from "@bitwarden/common/admin-console/abstraction import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { SyncService } from "@bitwarden/common/platform/sync"; -import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid"; +import { CollectionId, OrganizationId, UserId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { VaultSettingsService } from "@bitwarden/common/vault/abstractions/vault-settings/vault-settings.service"; import { CipherType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { runInsideAngular } from "../../../platform/browser/run-inside-angular.operator"; +import { PopupViewCacheService } from "../../../platform/popup/view-cache/popup-view-cache.service"; import { waitUntil } from "../../util"; import { PopupCipherView } from "../views/popup-cipher.view"; @@ -47,7 +46,12 @@ import { MY_VAULT_ID, VaultPopupListFiltersService } from "./vault-popup-list-fi providedIn: "root", }) export class VaultPopupItemsService { - private _searchText$ = new BehaviorSubject(""); + private cachedSearchText = inject(PopupViewCacheService).signal({ + key: "vault-search-text", + initialValue: "", + }); + + readonly searchText$ = toObservable(this.cachedSearchText); /** * Subject that emits whenever new ciphers are being processed/filtered. @@ -55,10 +59,13 @@ export class VaultPopupItemsService { */ private _ciphersLoading$ = new Subject(); - latestSearchText$: Observable = this._searchText$.asObservable(); + private activeUserId$ = this.accountService.activeAccount$.pipe( + map((a) => a?.id), + filter((userId): userId is UserId => userId !== null), + ); - private organizations$ = this.accountService.activeAccount$.pipe( - switchMap((account) => this.organizationService.organizations$(account?.id)), + private organizations$ = this.activeUserId$.pipe( + switchMap((userId) => this.organizationService.organizations$(userId)), ); /** * Observable that contains the list of other cipher types that should be shown @@ -88,7 +95,7 @@ export class VaultPopupItemsService { */ private _allDecryptedCiphers$: Observable = this.accountService.activeAccount$.pipe( map((a) => a?.id), - filter((userId) => userId != null), + filter((userId): userId is UserId => userId != null), switchMap((userId) => merge(this.cipherService.ciphers$(userId), this.cipherService.localData$(userId)).pipe( runInsideAngular(this.ngZone), @@ -131,13 +138,13 @@ export class VaultPopupItemsService { * Observable that indicates whether there is search text present that is searchable. * @private */ - private _hasSearchText$ = this._searchText$.pipe( + private _hasSearchText = this.searchText$.pipe( switchMap((searchText) => this.searchService.isSearchable(searchText)), ); private _filteredCipherList$: Observable = combineLatest([ this._activeCipherList$, - this._searchText$, + this.searchText$, this.vaultPopupListFiltersService.filterFunction$, ]).pipe( map(([ciphers, searchText, filterFunction]): [CipherView[], string] => [ @@ -146,7 +153,9 @@ export class VaultPopupItemsService { ]), switchMap( ([ciphers, searchText]) => - this.searchService.searchCiphers(searchText, null, ciphers) as Promise, + this.searchService.searchCiphers(searchText, undefined, ciphers) as Promise< + PopupCipherView[] + >, ), shareReplay({ refCount: true, bufferSize: 1 }), ); @@ -163,7 +172,7 @@ export class VaultPopupItemsService { this.vaultPopupAutofillService.currentAutofillTab$, ]).pipe( switchMap(([ciphers, otherTypes, tab]) => { - if (!tab) { + if (!tab || !tab.url) { return of([]); } return this.cipherService.filterCiphersForUrl(ciphers, tab.url, otherTypes); @@ -215,7 +224,7 @@ export class VaultPopupItemsService { * Observable that indicates whether a filter or search text is currently applied to the ciphers. */ hasFilterApplied$ = combineLatest([ - this._hasSearchText$, + this._hasSearchText, this.vaultPopupListFiltersService.filters$, ]).pipe( map(([hasSearchText, filters]) => { @@ -248,7 +257,7 @@ export class VaultPopupItemsService { return false; } - const org = orgs.find((o) => o.id === filters.organization.id); + const org = orgs.find((o) => o.id === filters?.organization?.id); return org ? !org.enabled : false; }), ); @@ -292,7 +301,7 @@ export class VaultPopupItemsService { ) {} applyFilter(newSearchText: string) { - this._searchText$.next(newSearchText); + this.cachedSearchText.set(newSearchText); } /** diff --git a/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.spec.ts b/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.spec.ts index ec823d5738f..99a27c54bcc 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.spec.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.spec.ts @@ -1,8 +1,9 @@ -import { TestBed } from "@angular/core/testing"; +import { Injector, WritableSignal, runInInjectionContext, signal } from "@angular/core"; +import { TestBed, discardPeriodicTasks, fakeAsync, tick } from "@angular/core/testing"; import { FormBuilder } from "@angular/forms"; import { BehaviorSubject, skipWhile } from "rxjs"; -import { CollectionService, Collection, CollectionView } from "@bitwarden/admin-console/common"; +import { CollectionService, CollectionView } from "@bitwarden/admin-console/common"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; @@ -19,17 +20,27 @@ import { CipherType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { FolderView } from "@bitwarden/common/vault/models/view/folder.view"; -import { MY_VAULT_ID, VaultPopupListFiltersService } from "./vault-popup-list-filters.service"; +import { PopupViewCacheService } from "../../../platform/popup/view-cache/popup-view-cache.service"; + +import { + CachedFilterState, + MY_VAULT_ID, + VaultPopupListFiltersService, +} from "./vault-popup-list-filters.service"; describe("VaultPopupListFiltersService", () => { let service: VaultPopupListFiltersService; - const _memberOrganizations$ = new BehaviorSubject([]); + let _memberOrganizations$ = new BehaviorSubject([]); const memberOrganizations$ = (userId: UserId) => _memberOrganizations$; const organizations$ = new BehaviorSubject([]); - const folderViews$ = new BehaviorSubject([]); + let folderViews$ = new BehaviorSubject([]); const cipherViews$ = new BehaviorSubject({}); - const decryptedCollections$ = new BehaviorSubject([]); + let decryptedCollections$ = new BehaviorSubject([]); const policyAppliesToActiveUser$ = new BehaviorSubject(false); + let viewCacheService: { + signal: jest.Mock; + mockSignal: WritableSignal; + }; const collectionService = { decryptedCollections$, @@ -61,12 +72,19 @@ describe("VaultPopupListFiltersService", () => { const update = jest.fn().mockResolvedValue(undefined); beforeEach(() => { - _memberOrganizations$.next([]); - decryptedCollections$.next([]); + _memberOrganizations$ = new BehaviorSubject([]); // Fresh instance per test + folderViews$ = new BehaviorSubject([]); // Fresh instance per test + decryptedCollections$ = new BehaviorSubject([]); // Fresh instance per test policyAppliesToActiveUser$.next(false); policyService.policyAppliesToActiveUser$.mockClear(); const accountService = mockAccountServiceWith("userId" as UserId); + const mockCachedSignal = createMockSignal({}); + + viewCacheService = { + mockSignal: mockCachedSignal, + signal: jest.fn(() => mockCachedSignal), + }; collectionService.getAllNested = () => Promise.resolve([]); TestBed.configureTestingModule({ @@ -104,6 +122,10 @@ describe("VaultPopupListFiltersService", () => { provide: AccountService, useValue: accountService, }, + { + provide: PopupViewCacheService, + useValue: viewCacheService, + }, ], }); @@ -440,7 +462,7 @@ describe("VaultPopupListFiltersService", () => { }); it("filters by collection", (done) => { - const collection = { id: "1234" } as Collection; + const collection = { id: "1234" } as CollectionView; service.filterFunction$.subscribe((filterFunction) => { expect(filterFunction(ciphers)).toEqual([ciphers[1]]); @@ -505,4 +527,194 @@ describe("VaultPopupListFiltersService", () => { expect(updateCallback()).toBe(false); }); }); + + describe("caching", () => { + it("initializes form from cached state", fakeAsync(() => { + const cachedState: CachedFilterState = { + organizationId: MY_VAULT_ID, + collectionId: "test-collection-id", + folderId: "test-folder-id", + cipherType: CipherType.Login, + }; + + const seededOrganizations: Organization[] = [ + { id: MY_VAULT_ID, name: "Test Org" } as Organization, + ]; + const seededCollections: CollectionView[] = [ + { + id: "test-collection-id", + organizationId: MY_VAULT_ID, + name: "Test collection", + } as CollectionView, + ]; + const seededFolderViews: FolderView[] = [ + { id: "test-folder-id", name: "Test Folder" } as FolderView, + ]; + + const { service } = createSeededVaultPopupListFiltersService( + seededOrganizations, + seededCollections, + seededFolderViews, + cachedState, + ); + + tick(); + + expect(service.filterForm.value).toEqual({ + organization: { id: MY_VAULT_ID }, + collection: { + id: "test-collection-id", + organizationId: MY_VAULT_ID, + name: "Test collection", + }, + folder: { id: "test-folder-id", name: "Test Folder" }, + cipherType: CipherType.Login, + }); + discardPeriodicTasks(); + })); + + it("serializes filters to cache on changes", fakeAsync(() => { + const seededOrganizations: Organization[] = [ + { id: "test-org-id", name: "Org" } as Organization, + ]; + const seededCollections: CollectionView[] = [ + { + id: "test-collection-id", + organizationId: "test-org-id", + name: "Test collection", + } as CollectionView, + ]; + const seededFolderViews: FolderView[] = [ + { id: "test-folder-id", name: "Test Folder" } as FolderView, + ]; + + const { service, cachedSignal } = createSeededVaultPopupListFiltersService( + seededOrganizations, + seededCollections, + seededFolderViews, + {}, + ); + const testOrg = { id: "test-org-id", name: "Org" } as Organization; + const testCollection = { + id: "test-collection-id", + organizationId: "test-org-id", + name: "Test collection", + } as CollectionView; + const testFolder = { id: "test-folder-id", name: "Test Folder" } as FolderView; + + service.filterForm.patchValue({ + organization: testOrg, + collection: testCollection, + folder: testFolder, + cipherType: CipherType.Card, + }); + + tick(300); + + // force another emission by patching with the same value again. workaround for debounce times + service.filterForm.patchValue({ + organization: testOrg, + collection: testCollection, + folder: testFolder, + cipherType: CipherType.Card, + }); + + tick(300); + + expect(cachedSignal()).toEqual({ + organizationId: "test-org-id", + collectionId: "test-collection-id", + folderId: "test-folder-id", + cipherType: CipherType.Card, + }); + discardPeriodicTasks(); + })); + }); }); + +function createMockSignal(initialValue: T): WritableSignal { + const s = signal(initialValue); + s.set = (value: T) => s.update(() => value); + return s; +} + +// Helper function to create a seeded VaultPopupListFiltersService +function createSeededVaultPopupListFiltersService( + organizations: Organization[], + collections: CollectionView[], + folderViews: FolderView[], + cachedState: CachedFilterState = {}, +): { service: VaultPopupListFiltersService; cachedSignal: WritableSignal } { + const seededMemberOrganizations$ = new BehaviorSubject(organizations); + const seededCollections$ = new BehaviorSubject(collections); + const seededFolderViews$ = new BehaviorSubject(folderViews); + + const organizationServiceMock = { + memberOrganizations$: (userId: string) => seededMemberOrganizations$, + organizations$: seededMemberOrganizations$, + } as any; + + const collectionServiceMock = { + decryptedCollections$: seededCollections$, + getAllNested: () => + Promise.resolve( + seededCollections$.value.map((c) => ({ + children: [], + node: c, + parent: null, + })), + ), + } as any; + + const folderServiceMock = { + folderViews$: () => seededFolderViews$, + } as any; + + const cipherServiceMock = { + cipherViews$: () => new BehaviorSubject({}), + } as any; + + const i18nServiceMock = { + t: (key: string) => key, + } as any; + + const policyServiceMock = { + policyAppliesToActiveUser$: jest.fn(() => new BehaviorSubject(false)), + } as any; + + const stateProviderMock = { + getGlobal: () => ({ + state$: new BehaviorSubject(false), + update: jest.fn().mockResolvedValue(undefined), + }), + } as any; + + const accountServiceMock = mockAccountServiceWith("userId" as UserId); + const formBuilderInstance = new FormBuilder(); + + const seededCachedSignal = createMockSignal(cachedState); + const viewCacheServiceMock = { + signal: jest.fn(() => seededCachedSignal), + mockSignal: seededCachedSignal, + } as any; + + // Get an injector from TestBed so that we can run in an injection context. + const injector = TestBed.inject(Injector); + let service: VaultPopupListFiltersService; + runInInjectionContext(injector, () => { + service = new VaultPopupListFiltersService( + folderServiceMock, + cipherServiceMock, + organizationServiceMock, + i18nServiceMock, + collectionServiceMock, + formBuilderInstance, + policyServiceMock, + stateProviderMock, + accountServiceMock, + viewCacheServiceMock, + ); + }); + + return { service: service!, cachedSignal: seededCachedSignal }; +} diff --git a/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.ts b/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.ts index 59ccde3bd44..8d9f6664e45 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.ts @@ -1,20 +1,22 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { Injectable } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { FormBuilder } from "@angular/forms"; import { combineLatest, + debounceTime, distinctUntilChanged, + filter, map, Observable, + of, shareReplay, startWith, switchMap, + take, tap, } from "rxjs"; -import { CollectionService, Collection, CollectionView } from "@bitwarden/admin-console/common"; +import { CollectionService, CollectionView } from "@bitwarden/admin-console/common"; import { DynamicTreeNode } from "@bitwarden/angular/vault/vault-filter/models/dynamic-tree-node.model"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; @@ -29,6 +31,7 @@ import { StateProvider, VAULT_SETTINGS_DISK, } from "@bitwarden/common/platform/state"; +import { 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 { CipherType } from "@bitwarden/common/vault/enums"; @@ -38,14 +41,26 @@ import { FolderView } from "@bitwarden/common/vault/models/view/folder.view"; import { ServiceUtils } from "@bitwarden/common/vault/service-utils"; import { ChipSelectOption } from "@bitwarden/components"; +import { PopupViewCacheService } from "../../../platform/popup/view-cache/popup-view-cache.service"; + const FILTER_VISIBILITY_KEY = new KeyDefinition(VAULT_SETTINGS_DISK, "filterVisibility", { deserializer: (obj) => obj, }); +/** + * Serialized state of the PopupListFilter for interfacing with the PopupViewCacheService + */ +export interface CachedFilterState { + organizationId?: string; + collectionId?: string; + folderId?: string; + cipherType?: CipherType | null; +} + /** All available cipher filters */ export type PopupListFilter = { organization: Organization | null; - collection: Collection | null; + collection: CollectionView | null; folder: FolderView | null; cipherType: CipherType | null; }; @@ -76,8 +91,9 @@ export class VaultPopupListFiltersService { * Observable for `filterForm` value */ filters$ = this.filterForm.valueChanges.pipe( - startWith(INITIAL_FILTERS), - ) as Observable; + startWith(this.filterForm.value), + shareReplay({ bufferSize: 1, refCount: true }), + ); /** Emits the number of applied filters. */ numberOfAppliedFilters$ = this.filters$.pipe( @@ -93,7 +109,65 @@ export class VaultPopupListFiltersService { */ private cipherViews: CipherView[] = []; - private activeUserId$ = this.accountService.activeAccount$.pipe(map((a) => a?.id)); + private activeUserId$ = this.accountService.activeAccount$.pipe( + map((a) => a?.id), + filter((userId): userId is UserId => userId !== null), + ); + + private serializeFilters(): CachedFilterState { + return { + organizationId: this.filterForm.value.organization?.id, + collectionId: this.filterForm.value.collection?.id, + folderId: this.filterForm.value.folder?.id, + cipherType: this.filterForm.value.cipherType, + }; + } + + private deserializeFilters(state: CachedFilterState): void { + combineLatest([this.organizations$, this.collections$, this.folders$]) + .pipe(take(1)) + .subscribe(([orgOptions, collectionOptions, folderOptions]) => { + const patchValue: PopupListFilter = { + organization: null, + collection: null, + folder: null, + cipherType: null, + }; + + if (state.organizationId) { + if (state.organizationId === MY_VAULT_ID) { + patchValue.organization = { id: MY_VAULT_ID } as Organization; + } else { + const orgOption = orgOptions.find((o) => o.value?.id === state.organizationId); + patchValue.organization = orgOption?.value || null; + } + } + + if (state.collectionId) { + const collection = collectionOptions + .flatMap((c) => this.flattenOptions(c)) + .find((c) => c.value?.id === state.collectionId)?.value; + patchValue.collection = collection || null; + } + + if (state.folderId) { + const folder = folderOptions + .flatMap((f) => this.flattenOptions(f)) + .find((f) => f.value?.id === state.folderId)?.value; + patchValue.folder = folder || null; + } + + if (state.cipherType) { + patchValue.cipherType = state.cipherType; + } + + this.filterForm.patchValue(patchValue); + }); + } + + private flattenOptions(option: ChipSelectOption): ChipSelectOption[] { + return [option, ...(option.children?.flatMap((c) => this.flattenOptions(c)) || [])]; + } constructor( private folderService: FolderService, @@ -105,10 +179,30 @@ export class VaultPopupListFiltersService { private policyService: PolicyService, private stateProvider: StateProvider, private accountService: AccountService, + private viewCacheService: PopupViewCacheService, ) { this.filterForm.controls.organization.valueChanges .pipe(takeUntilDestroyed()) .subscribe(this.validateOrganizationChange.bind(this)); + + const cachedFilters = this.viewCacheService.signal({ + key: "vault-filters", + initialValue: {}, + deserializer: (v) => v, + }); + + this.deserializeFilters(cachedFilters()); + + // Save changes to cache + this.filterForm.valueChanges + .pipe( + debounceTime(300), + map(() => this.serializeFilters()), + distinctUntilChanged((a, b) => JSON.stringify(a) === JSON.stringify(b)), + ) + .subscribe((state) => { + cachedFilters.set(state); + }); } /** Stored state for the visibility of the filters. */ @@ -130,14 +224,11 @@ export class VaultPopupListFiltersService { return false; } - if ( - filters.collection !== null && - !cipher.collectionIds.includes(filters.collection.id) - ) { + if (filters.collection && !cipher.collectionIds?.includes(filters.collection.id)) { return false; } - if (filters.folder !== null && cipher.folderId !== filters.folder.id) { + if (filters.folder && cipher.folderId !== filters.folder.id) { return false; } @@ -147,7 +238,7 @@ export class VaultPopupListFiltersService { if (cipher.organizationId !== null) { return false; } - } else if (filters.organization !== null) { + } else if (filters.organization) { if (cipher.organizationId !== filters.organization.id) { return false; } @@ -199,7 +290,9 @@ export class VaultPopupListFiltersService { */ organizations$: Observable[]> = combineLatest([ this.accountService.activeAccount$.pipe( - switchMap((account) => this.organizationService.memberOrganizations$(account?.id)), + switchMap((account) => + account === null ? of([]) : this.organizationService.memberOrganizations$(account.id), + ), ), this.policyService.policyAppliesToActiveUser$(PolicyType.PersonalOwnership), ]).pipe( @@ -285,7 +378,7 @@ export class VaultPopupListFiltersService { 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]; + return [filters as PopupListFilter, [], cipherViews]; } // Sort folders by alphabetic name @@ -304,7 +397,7 @@ export class VaultPopupListFiltersService { // Move the "no folder" option to the end of the list arrangedFolders = [...folders.filter((f) => f.id !== null), updatedNoFolder]; } - return [filters, arrangedFolders, cipherViews]; + return [filters as PopupListFilter, arrangedFolders, cipherViews]; }), map(([filters, folders, cipherViews]) => { const organizationId = filters.organization?.id ?? null; @@ -410,7 +503,7 @@ export class VaultPopupListFiltersService { // Remove "/" from beginning and end of the folder name // then split the folder name by the delimiter const parts = f.name != null ? f.name.replace(/^\/+|\/+$/g, "").split(NESTING_DELIMITER) : []; - ServiceUtils.nestedTraverse(nodes, 0, parts, folderCopy, null, NESTING_DELIMITER); + ServiceUtils.nestedTraverse(nodes, 0, parts, folderCopy, undefined, NESTING_DELIMITER); }); return nodes; @@ -429,7 +522,7 @@ export class VaultPopupListFiltersService { // When the organization filter changes and a collection is already selected, // reset the collection filter if the collection does not belong to the new organization filter if (currentFilters.collection && currentFilters.collection.organizationId !== organization.id) { - this.filterForm.get("collection").setValue(null); + this.filterForm.get("collection")?.setValue(null); } // When the organization filter changes and a folder is already selected, @@ -444,12 +537,12 @@ export class VaultPopupListFiltersService { // Find any ciphers within the organization that belong to the current folder const newOrgContainsFolder = orgCiphers.some( - (oc) => oc.folderId === currentFilters.folder.id, + (oc) => oc.folderId === currentFilters?.folder?.id, ); // If the new organization does not contain the current folder, reset the folder filter if (!newOrgContainsFolder) { - this.filterForm.get("folder").setValue(null); + this.filterForm.get("folder")?.setValue(null); } } } From bc7c22ae01dc945251fd513eab4262fcf80259d6 Mon Sep 17 00:00:00 2001 From: Todd Martin <106564991+trmartin4@users.noreply.github.com> Date: Mon, 24 Feb 2025 12:34:15 -0500 Subject: [PATCH 003/119] [PM-18529] Remove persist-popup-view feature flag (#13519) * Removed feature flag from clients * Removed feature flag test --- .../view-cache/popup-router-cache.service.ts | 25 +++++-------------- .../view-cache/popup-view-cache.service.ts | 16 +++--------- .../popup/view-cache/popup-view-cache.spec.ts | 17 ------------- libs/common/src/enums/feature-flag.enum.ts | 2 -- 4 files changed, 9 insertions(+), 51 deletions(-) diff --git a/apps/browser/src/platform/popup/view-cache/popup-router-cache.service.ts b/apps/browser/src/platform/popup/view-cache/popup-router-cache.service.ts index 2dfbf05e3c3..3215893c634 100644 --- a/apps/browser/src/platform/popup/view-cache/popup-router-cache.service.ts +++ b/apps/browser/src/platform/popup/view-cache/popup-router-cache.service.ts @@ -9,10 +9,8 @@ import { Router, UrlSerializer, } from "@angular/router"; -import { filter, first, firstValueFrom, map, Observable, of, switchMap, tap } from "rxjs"; +import { filter, first, firstValueFrom, map, Observable, switchMap, tap } from "rxjs"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { GlobalStateProvider } from "@bitwarden/common/platform/state"; import { POPUP_ROUTE_HISTORY_KEY } from "../../../platform/services/popup-view-cache-background.service"; @@ -113,29 +111,18 @@ export class PopupRouterCacheService { /** * Redirect to the last visited route. Should be applied to root route. - * - * If `FeatureFlag.PersistPopupView` is disabled, do nothing. **/ export const popupRouterCacheGuard = (() => { - const configService = inject(ConfigService); const popupHistoryService = inject(PopupRouterCacheService); const urlSerializer = inject(UrlSerializer); - return configService.getFeatureFlag$(FeatureFlag.PersistPopupView).pipe( - switchMap((featureEnabled) => { - if (!featureEnabled) { - return of(true); + return popupHistoryService.last$().pipe( + map((url: string) => { + if (!url) { + return true; } - return popupHistoryService.last$().pipe( - map((url: string) => { - if (!url) { - return true; - } - - return urlSerializer.parse(url); - }), - ); + return urlSerializer.parse(url); }), ); }) satisfies CanActivateFn; diff --git a/apps/browser/src/platform/popup/view-cache/popup-view-cache.service.ts b/apps/browser/src/platform/popup/view-cache/popup-view-cache.service.ts index 69a0fa7d9d1..2c29f1e5763 100644 --- a/apps/browser/src/platform/popup/view-cache/popup-view-cache.service.ts +++ b/apps/browser/src/platform/popup/view-cache/popup-view-cache.service.ts @@ -20,8 +20,6 @@ import { SignalCacheOptions, ViewCacheService, } from "@bitwarden/angular/platform/abstractions/view-cache.service"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { MessageSender } from "@bitwarden/common/platform/messaging"; import { GlobalStateProvider } from "@bitwarden/common/platform/state"; @@ -40,13 +38,10 @@ import { providedIn: "root", }) export class PopupViewCacheService implements ViewCacheService { - private configService = inject(ConfigService); private globalStateProvider = inject(GlobalStateProvider); private messageSender = inject(MessageSender); private router = inject(Router); - private featureEnabled: boolean; - private _cache: Record; private get cache(): Record { if (!this._cache) { @@ -59,10 +54,9 @@ export class PopupViewCacheService implements ViewCacheService { * Initialize the service. This should only be called once. */ async init() { - this.featureEnabled = await this.configService.getFeatureFlag(FeatureFlag.PersistPopupView); - const initialState = this.featureEnabled - ? await firstValueFrom(this.globalStateProvider.get(POPUP_VIEW_CACHE_KEY).state$) - : {}; + const initialState = await firstValueFrom( + this.globalStateProvider.get(POPUP_VIEW_CACHE_KEY).state$, + ); this._cache = Object.freeze(initialState ?? {}); this.router.events @@ -122,10 +116,6 @@ export class PopupViewCacheService implements ViewCacheService { } private updateState(key: string, value: string) { - if (!this.featureEnabled) { - return; - } - this.messageSender.send(SAVE_VIEW_CACHE_COMMAND, { key, value, diff --git a/apps/browser/src/platform/popup/view-cache/popup-view-cache.spec.ts b/apps/browser/src/platform/popup/view-cache/popup-view-cache.spec.ts index 72cfd39cd62..b6009c4cc2e 100644 --- a/apps/browser/src/platform/popup/view-cache/popup-view-cache.spec.ts +++ b/apps/browser/src/platform/popup/view-cache/popup-view-cache.spec.ts @@ -206,21 +206,4 @@ describe("popup view cache", () => { expect(messageSenderMock.send).toHaveBeenCalledWith(ClEAR_VIEW_CACHE_COMMAND, {}); expect(service["_cache"]).toEqual({}); }); - - it("should ignore cached values when feature flag is off", async () => { - jest.spyOn(configServiceMock, "getFeatureFlag").mockResolvedValue(false); - - await initServiceWithState({ "foo-123": JSON.stringify("bar") }); - - const injector = TestBed.inject(Injector); - - const signal = service.signal({ - key: "foo-123", - initialValue: "foo", - injector, - }); - - // The cached state is ignored - expect(signal()).toBe("foo"); - }); }); diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index c5cd5dac636..ca6a26bc531 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -30,7 +30,6 @@ export enum FeatureFlag { AC1795_UpdatedSubscriptionStatusSection = "AC-1795_updated-subscription-status-section", ExtensionRefresh = "extension-refresh", - PersistPopupView = "persist-popup-view", PM4154_BulkEncryptionService = "PM-4154-bulk-encryption-service", VaultBulkManagementAction = "vault-bulk-management-action", UnauthenticatedExtensionUIRefresh = "unauth-ui-refresh", @@ -89,7 +88,6 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.AC1795_UpdatedSubscriptionStatusSection]: FALSE, [FeatureFlag.ExtensionRefresh]: FALSE, - [FeatureFlag.PersistPopupView]: FALSE, [FeatureFlag.PM4154_BulkEncryptionService]: FALSE, [FeatureFlag.VaultBulkManagementAction]: FALSE, [FeatureFlag.UnauthenticatedExtensionUIRefresh]: FALSE, From cbbd53803b873c788452c4e9f6ccaf491b98851d Mon Sep 17 00:00:00 2001 From: Alec Rippberger <127791530+alec-livefront@users.noreply.github.com> Date: Mon, 24 Feb 2025 11:44:32 -0600 Subject: [PATCH 004/119] feat(web): [PM-15063] add banner for pending device auth requests Adds a banner in the web vault to notify users when they have pending device authentication requests. The banner links to the device management screen. Also implements real-time updates to the device management table when new auth requests are received. JIRA: PM-15063 --- .../security/device-management.component.html | 3 +- .../device-management.component.spec.ts | 181 ++++++++++++++++++ .../security/device-management.component.ts | 173 ++++++++++++++--- .../services/vault-banners.service.spec.ts | 68 +++++++ .../services/vault-banners.service.ts | 17 ++ .../vault-banners.component.html | 14 ++ .../vault-banners.component.spec.ts | 90 ++++++++- .../vault-banners/vault-banners.component.ts | 36 +++- apps/web/src/locales/en/messages.json | 6 + .../abstractions/devices/views/device.view.ts | 19 +- .../models/response/auth-request.response.ts | 6 +- 11 files changed, 565 insertions(+), 48 deletions(-) create mode 100644 apps/web/src/app/auth/settings/security/device-management.component.spec.ts diff --git a/apps/web/src/app/auth/settings/security/device-management.component.html b/apps/web/src/app/auth/settings/security/device-management.component.html index c38283cfd80..587703c7389 100644 --- a/apps/web/src/app/auth/settings/security/device-management.component.html +++ b/apps/web/src/app/auth/settings/security/device-management.component.html @@ -40,7 +40,8 @@ > {{ col.title }} - + + diff --git a/apps/web/src/app/auth/settings/security/device-management.component.spec.ts b/apps/web/src/app/auth/settings/security/device-management.component.spec.ts new file mode 100644 index 00000000000..84c1dfcb63b --- /dev/null +++ b/apps/web/src/app/auth/settings/security/device-management.component.spec.ts @@ -0,0 +1,181 @@ +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { RouterTestingModule } from "@angular/router/testing"; +import { of, Subject } from "rxjs"; + +import { AuthRequestApiService } from "@bitwarden/auth/common"; +import { DevicesServiceAbstraction } from "@bitwarden/common/auth/abstractions/devices/devices.service.abstraction"; +import { DeviceView } from "@bitwarden/common/auth/abstractions/devices/views/device.view"; +import { DeviceType } from "@bitwarden/common/enums"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; +import { MessageListener } from "@bitwarden/common/platform/messaging"; +import { DialogService, ToastService, TableModule, PopoverModule } from "@bitwarden/components"; + +import { SharedModule } from "../../../shared"; +import { VaultBannersService } from "../../../vault/individual-vault/vault-banners/services/vault-banners.service"; + +import { DeviceManagementComponent } from "./device-management.component"; + +class MockResizeObserver { + observe = jest.fn(); + unobserve = jest.fn(); + disconnect = jest.fn(); +} + +global.ResizeObserver = MockResizeObserver; + +interface Message { + command: string; + notificationId?: string; +} + +describe("DeviceManagementComponent", () => { + let fixture: ComponentFixture; + let messageSubject: Subject; + let mockDevices: DeviceView[]; + let vaultBannersService: VaultBannersService; + + const mockDeviceResponse = { + id: "test-id", + requestDeviceType: "test-type", + requestDeviceTypeValue: DeviceType.Android, + requestDeviceIdentifier: "test-identifier", + requestIpAddress: "127.0.0.1", + creationDate: new Date().toISOString(), + responseDate: null, + key: "test-key", + masterPasswordHash: null, + publicKey: "test-public-key", + requestApproved: false, + origin: "test-origin", + }; + + beforeEach(async () => { + messageSubject = new Subject(); + mockDevices = []; + + await TestBed.configureTestingModule({ + imports: [ + RouterTestingModule, + SharedModule, + TableModule, + PopoverModule, + DeviceManagementComponent, + ], + providers: [ + { + provide: DevicesServiceAbstraction, + useValue: { + getDevices$: jest.fn().mockReturnValue(mockDevices), + getCurrentDevice$: jest.fn().mockReturnValue(of(null)), + getDeviceByIdentifier$: jest.fn().mockReturnValue(of(null)), + updateTrustedDeviceKeys: jest.fn(), + }, + }, + { + provide: AuthRequestApiService, + useValue: { + getAuthRequest: jest.fn().mockResolvedValue(mockDeviceResponse), + }, + }, + { + provide: MessageListener, + useValue: { + allMessages$: messageSubject.asObservable(), + }, + }, + { + provide: DialogService, + useValue: { + openSimpleDialog: jest.fn(), + }, + }, + { + provide: ToastService, + useValue: { + success: jest.fn(), + error: jest.fn(), + }, + }, + { + provide: VaultBannersService, + useValue: { + shouldShowPendingAuthRequestBanner: jest.fn(), + }, + }, + { + provide: I18nService, + useValue: { + t: jest.fn((key: string) => key), + }, + }, + { + provide: ValidationService, + useValue: { + showError: jest.fn(), + }, + }, + ], + }).compileComponents(); + + fixture = TestBed.createComponent(DeviceManagementComponent); + + vaultBannersService = TestBed.inject(VaultBannersService); + }); + + describe("message listener", () => { + beforeEach(() => { + jest.spyOn(vaultBannersService, "shouldShowPendingAuthRequestBanner").mockResolvedValue(true); + }); + + it("ignores other message types", async () => { + const initialDataLength = (fixture.componentInstance as any).dataSource.data.length; + const message: Message = { command: "other", notificationId: "test-id" }; + messageSubject.next(message); + await fixture.whenStable(); + + expect((fixture.componentInstance as any).dataSource.data.length).toBe(initialDataLength); + }); + + it("adds device to table when auth request message received", async () => { + const initialDataLength = (fixture.componentInstance as any).dataSource.data.length; + const message: Message = { + command: "openLoginApproval", + notificationId: "test-id", + }; + + messageSubject.next(message); + fixture.detectChanges(); + await fixture.whenStable(); + + const dataSource = (fixture.componentInstance as any).dataSource; + expect(dataSource.data.length).toBe(initialDataLength + 1); + + const addedDevice = dataSource.data[0]; + expect(addedDevice).toEqual({ + id: "", + type: mockDeviceResponse.requestDeviceTypeValue, + displayName: expect.any(String), + loginStatus: "requestPending", + firstLogin: expect.any(Date), + trusted: false, + devicePendingAuthRequest: { + id: mockDeviceResponse.id, + creationDate: mockDeviceResponse.creationDate, + }, + hasPendingAuthRequest: true, + identifier: mockDeviceResponse.requestDeviceIdentifier, + }); + }); + + it("stops listening when component is destroyed", async () => { + fixture.destroy(); + const message: Message = { + command: "openLoginApproval", + notificationId: "test-id", + }; + messageSubject.next(message); + expect((fixture.componentInstance as any).dataSource.data.length).toBe(0); + }); + }); +}); diff --git a/apps/web/src/app/auth/settings/security/device-management.component.ts b/apps/web/src/app/auth/settings/security/device-management.component.ts index e22122ad9ae..97107cc0c0b 100644 --- a/apps/web/src/app/auth/settings/security/device-management.component.ts +++ b/apps/web/src/app/auth/settings/security/device-management.component.ts @@ -1,9 +1,10 @@ import { CommonModule } from "@angular/common"; -import { Component } from "@angular/core"; +import { Component, DestroyRef } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; -import { combineLatest, firstValueFrom } from "rxjs"; +import { firstValueFrom } from "rxjs"; import { LoginApprovalComponent } from "@bitwarden/auth/angular"; +import { AuthRequestApiService } from "@bitwarden/auth/common"; import { DevicesServiceAbstraction } from "@bitwarden/common/auth/abstractions/devices/devices.service.abstraction"; import { DevicePendingAuthRequest, @@ -13,6 +14,7 @@ import { DeviceView } from "@bitwarden/common/auth/abstractions/devices/views/de import { DeviceType, DeviceTypeMetadata } from "@bitwarden/common/enums"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; +import { MessageListener } from "@bitwarden/common/platform/messaging"; import { DialogService, ToastService, @@ -23,6 +25,9 @@ import { import { SharedModule } from "../../../shared"; +/** + * Interface representing a row in the device management table + */ interface DeviceTableData { id: string; type: DeviceType; @@ -32,6 +37,7 @@ interface DeviceTableData { trusted: boolean; devicePendingAuthRequest: DevicePendingAuthRequest | null; hasPendingAuthRequest: boolean; + identifier: string; } /** @@ -44,7 +50,6 @@ interface DeviceTableData { imports: [CommonModule, SharedModule, TableModule, PopoverModule], }) export class DeviceManagementComponent { - protected readonly tableId = "device-management-table"; protected dataSource = new TableDataSource(); protected currentDevice: DeviceView | undefined; protected loading = true; @@ -56,32 +61,146 @@ export class DeviceManagementComponent { private dialogService: DialogService, private toastService: ToastService, private validationService: ValidationService, + private messageListener: MessageListener, + private authRequestApiService: AuthRequestApiService, + private destroyRef: DestroyRef, ) { - combineLatest([this.devicesService.getCurrentDevice$(), this.devicesService.getDevices$()]) - .pipe(takeUntilDestroyed()) - .subscribe({ - next: ([currentDevice, devices]: [DeviceResponse, Array]) => { - this.currentDevice = new DeviceView(currentDevice); + void this.initializeDevices(); + } - this.dataSource.data = devices.map((device: DeviceView): DeviceTableData => { - return { - id: device.id, - type: device.type, - displayName: this.getHumanReadableDeviceType(device.type), - loginStatus: this.getLoginStatus(device), - firstLogin: new Date(device.creationDate), - trusted: device.response.isTrusted, - devicePendingAuthRequest: device.response.devicePendingAuthRequest, - hasPendingAuthRequest: this.hasPendingAuthRequest(device.response), - }; - }); + /** + * Initialize the devices list and set up the message listener + */ + private async initializeDevices(): Promise { + try { + await this.loadDevices(); - this.loading = false; - }, - error: () => { - this.loading = false; - }, - }); + this.messageListener.allMessages$ + .pipe(takeUntilDestroyed(this.destroyRef)) + .subscribe((message) => { + if (message.command !== "openLoginApproval") { + return; + } + // Handle inserting a new device when an auth request is received + this.upsertDeviceWithPendingAuthRequest( + message as { command: string; notificationId: string }, + ).catch((error) => this.validationService.showError(error)); + }); + } catch (error) { + this.validationService.showError(error); + } + } + + /** + * Handle inserting a new device when an auth request is received + * @param message - The auth request message + */ + private async upsertDeviceWithPendingAuthRequest(message: { + command: string; + notificationId: string; + }): Promise { + const requestId = message.notificationId; + if (!requestId) { + return; + } + + const authRequestResponse = await this.authRequestApiService.getAuthRequest(requestId); + if (!authRequestResponse) { + return; + } + + // Add new device to the table + const upsertDevice: DeviceTableData = { + id: "", + type: authRequestResponse.requestDeviceTypeValue, + displayName: this.getHumanReadableDeviceType(authRequestResponse.requestDeviceTypeValue), + loginStatus: this.i18nService.t("requestPending"), + firstLogin: new Date(authRequestResponse.creationDate), + trusted: false, + devicePendingAuthRequest: { + id: authRequestResponse.id, + creationDate: authRequestResponse.creationDate, + }, + hasPendingAuthRequest: true, + identifier: authRequestResponse.requestDeviceIdentifier, + }; + + // If the device already exists in the DB, update the device id and first login date + if (authRequestResponse.requestDeviceIdentifier) { + const existingDevice = await firstValueFrom( + this.devicesService.getDeviceByIdentifier$(authRequestResponse.requestDeviceIdentifier), + ); + + if (existingDevice?.id && existingDevice.creationDate) { + upsertDevice.id = existingDevice.id; + upsertDevice.firstLogin = new Date(existingDevice.creationDate); + } + } + + const existingDeviceIndex = this.dataSource.data.findIndex( + (device) => device.identifier === upsertDevice.identifier, + ); + + if (existingDeviceIndex >= 0) { + // Update existing device + this.dataSource.data[existingDeviceIndex] = upsertDevice; + this.dataSource.data = [...this.dataSource.data]; + } else { + // Add new device + this.dataSource.data = [upsertDevice, ...this.dataSource.data]; + } + } + + /** + * Load current device and all devices + */ + private async loadDevices(): Promise { + try { + const currentDevice = await firstValueFrom(this.devicesService.getCurrentDevice$()); + const devices = await firstValueFrom(this.devicesService.getDevices$()); + + if (!currentDevice || !devices) { + this.loading = false; + return; + } + + this.currentDevice = new DeviceView(currentDevice); + this.updateDeviceTable(devices); + } catch (error) { + this.validationService.showError(error); + } finally { + this.loading = false; + } + } + + /** + * Updates the device table with the latest device data + * @param devices - Array of device views to display in the table + */ + private updateDeviceTable(devices: Array): void { + this.dataSource.data = devices + .map((device: DeviceView): DeviceTableData | null => { + if (!device.id || !device.type || !device.creationDate) { + this.validationService.showError(new Error("Invalid device data")); + return null; + } + + const hasPendingRequest = device.response + ? this.hasPendingAuthRequest(device.response) + : false; + return { + id: device.id, + type: device.type, + displayName: this.getHumanReadableDeviceType(device.type), + loginStatus: this.getLoginStatus(device), + firstLogin: new Date(device.creationDate), + trusted: device.response?.isTrusted ?? false, + devicePendingAuthRequest: device.response?.devicePendingAuthRequest ?? null, + hasPendingAuthRequest: hasPendingRequest, + identifier: device.identifier ?? "", + }; + }) + .filter((device): device is DeviceTableData => device !== null); } /** @@ -140,7 +259,7 @@ export class DeviceManagementComponent { return this.i18nService.t("currentSession"); } - if (device.response.devicePendingAuthRequest?.creationDate) { + if (device?.response?.devicePendingAuthRequest?.creationDate) { return this.i18nService.t("requestPending"); } diff --git a/apps/web/src/app/vault/individual-vault/vault-banners/services/vault-banners.service.spec.ts b/apps/web/src/app/vault/individual-vault/vault-banners/services/vault-banners.service.spec.ts index 88fae02275f..4ce65b9f771 100644 --- a/apps/web/src/app/vault/individual-vault/vault-banners/services/vault-banners.service.spec.ts +++ b/apps/web/src/app/vault/individual-vault/vault-banners/services/vault-banners.service.spec.ts @@ -6,7 +6,11 @@ import { UserDecryptionOptionsServiceAbstraction, } from "@bitwarden/auth/common"; import { AccountInfo, AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { DevicesServiceAbstraction } from "@bitwarden/common/auth/abstractions/devices/devices.service.abstraction"; +import { DeviceResponse } from "@bitwarden/common/auth/abstractions/devices/responses/device.response"; +import { DeviceView } from "@bitwarden/common/auth/abstractions/devices/views/device.view"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; +import { DeviceType } from "@bitwarden/common/enums"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { StateProvider } from "@bitwarden/common/platform/state"; @@ -36,6 +40,7 @@ describe("VaultBannersService", () => { const accounts$ = new BehaviorSubject>({ [userId]: { email: "test@bitwarden.com", emailVerified: true, name: "name" } as AccountInfo, }); + const devices$ = new BehaviorSubject([]); beforeEach(() => { lastSync$.next(new Date("2024-05-14")); @@ -79,6 +84,10 @@ describe("VaultBannersService", () => { userDecryptionOptionsById$: () => userDecryptionOptions$, }, }, + { + provide: DevicesServiceAbstraction, + useValue: { getDevices$: () => devices$ }, + }, ], }); }); @@ -274,4 +283,63 @@ describe("VaultBannersService", () => { expect(await service.shouldShowVerifyEmailBanner(userId)).toBe(false); }); }); + + describe("PendingAuthRequest", () => { + const now = new Date(); + let deviceResponse: DeviceResponse; + + beforeEach(() => { + deviceResponse = new DeviceResponse({ + Id: "device1", + UserId: userId, + Name: "Test Device", + Identifier: "test-device", + Type: DeviceType.Android, + CreationDate: now.toISOString(), + RevisionDate: now.toISOString(), + IsTrusted: false, + }); + // Reset devices list, single user state, and active user state before each test + devices$.next([]); + fakeStateProvider.singleUser.states.clear(); + fakeStateProvider.activeUser.states.clear(); + }); + + it("shows pending auth request banner when there is a pending request", async () => { + deviceResponse.devicePendingAuthRequest = { + id: "123", + creationDate: now.toISOString(), + }; + devices$.next([new DeviceView(deviceResponse)]); + + service = TestBed.inject(VaultBannersService); + + expect(await service.shouldShowPendingAuthRequestBanner(userId)).toBe(true); + }); + + it("does not show pending auth request banner when there are no pending requests", async () => { + deviceResponse.devicePendingAuthRequest = null; + devices$.next([new DeviceView(deviceResponse)]); + + service = TestBed.inject(VaultBannersService); + + expect(await service.shouldShowPendingAuthRequestBanner(userId)).toBe(false); + }); + + it("dismisses pending auth request banner", async () => { + deviceResponse.devicePendingAuthRequest = { + id: "123", + creationDate: now.toISOString(), + }; + devices$.next([new DeviceView(deviceResponse)]); + + service = TestBed.inject(VaultBannersService); + + expect(await service.shouldShowPendingAuthRequestBanner(userId)).toBe(true); + + await service.dismissBanner(userId, VisibleVaultBanner.PendingAuthRequest); + + expect(await service.shouldShowPendingAuthRequestBanner(userId)).toBe(false); + }); + }); }); diff --git a/apps/web/src/app/vault/individual-vault/vault-banners/services/vault-banners.service.ts b/apps/web/src/app/vault/individual-vault/vault-banners/services/vault-banners.service.ts index 475cfc2df22..1fa5ae1ad8b 100644 --- a/apps/web/src/app/vault/individual-vault/vault-banners/services/vault-banners.service.ts +++ b/apps/web/src/app/vault/individual-vault/vault-banners/services/vault-banners.service.ts @@ -3,6 +3,7 @@ import { Observable, combineLatest, firstValueFrom, map, filter, mergeMap, take import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { DevicesServiceAbstraction } from "@bitwarden/common/auth/abstractions/devices/devices.service.abstraction"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { @@ -21,6 +22,7 @@ export enum VisibleVaultBanner { OutdatedBrowser = "outdated-browser", Premium = "premium", VerifyEmail = "verify-email", + PendingAuthRequest = "pending-auth-request", } type PremiumBannerReprompt = { @@ -60,8 +62,23 @@ export class VaultBannersService { private kdfConfigService: KdfConfigService, private syncService: SyncService, private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction, + private devicesService: DevicesServiceAbstraction, ) {} + /** Returns true when the pending auth request banner should be shown */ + async shouldShowPendingAuthRequestBanner(userId: UserId): Promise { + const devices = await firstValueFrom(this.devicesService.getDevices$()); + const hasPendingRequest = devices.some( + (device) => device.response?.devicePendingAuthRequest != null, + ); + + const alreadyDismissed = (await this.getBannerDismissedState(userId)).includes( + VisibleVaultBanner.PendingAuthRequest, + ); + + return hasPendingRequest && !alreadyDismissed; + } + shouldShowPremiumBanner$(userId: UserId): Observable { const premiumBannerState = this.premiumBannerState(userId); const premiumSources$ = combineLatest([ diff --git a/apps/web/src/app/vault/individual-vault/vault-banners/vault-banners.component.html b/apps/web/src/app/vault/individual-vault/vault-banners/vault-banners.component.html index 29909e26716..e97f1579f57 100644 --- a/apps/web/src/app/vault/individual-vault/vault-banners/vault-banners.component.html +++ b/apps/web/src/app/vault/individual-vault/vault-banners/vault-banners.component.html @@ -50,6 +50,20 @@ + + {{ "youHaveAPendingLoginRequest" | i18n }} + + {{ "reviewLoginRequest" | i18n }} + + + { let component: VaultBannersComponent; let fixture: ComponentFixture; + let messageSubject: Subject<{ command: string }>; const premiumBanner$ = new BehaviorSubject(false); + const pendingAuthRequest$ = new BehaviorSubject(false); const mockUserId = Utils.newGuid() as UserId; const bannerService = mock({ - shouldShowPremiumBanner$: jest.fn((userId$: Observable) => premiumBanner$), + shouldShowPremiumBanner$: jest.fn((userId: UserId) => premiumBanner$), shouldShowUpdateBrowserBanner: jest.fn(), shouldShowVerifyEmailBanner: jest.fn(), shouldShowLowKDFBanner: jest.fn(), + shouldShowPendingAuthRequestBanner: jest.fn((userId: UserId) => + Promise.resolve(pendingAuthRequest$.value), + ), dismissBanner: jest.fn(), }); const accountService: FakeAccountService = mockAccountServiceWith(mockUserId); beforeEach(async () => { + messageSubject = new Subject<{ command: string }>(); bannerService.shouldShowUpdateBrowserBanner.mockResolvedValue(false); bannerService.shouldShowVerifyEmailBanner.mockResolvedValue(false); bannerService.shouldShowLowKDFBanner.mockResolvedValue(false); - + pendingAuthRequest$.next(false); premiumBanner$.next(false); await TestBed.configureTestingModule({ @@ -74,6 +81,12 @@ describe("VaultBannersComponent", () => { provide: AccountService, useValue: accountService, }, + { + provide: MessageListener, + useValue: mock({ + allMessages$: messageSubject.asObservable(), + }), + }, ], }) .overrideProvider(VaultBannersService, { useValue: bannerService }) @@ -153,5 +166,76 @@ describe("VaultBannersComponent", () => { }); }); }); + + describe("PendingAuthRequest", () => { + beforeEach(async () => { + pendingAuthRequest$.next(true); + await component.ngOnInit(); + fixture.detectChanges(); + }); + + it("shows pending auth request banner", async () => { + expect(component.visibleBanners).toEqual([VisibleVaultBanner.PendingAuthRequest]); + }); + + it("dismisses pending auth request banner", async () => { + const dismissButton = fixture.debugElement.nativeElement.querySelector( + 'button[biticonbutton="bwi-close"]', + ); + + pendingAuthRequest$.next(false); + dismissButton.click(); + fixture.detectChanges(); + + expect(bannerService.dismissBanner).toHaveBeenCalledWith( + mockUserId, + VisibleVaultBanner.PendingAuthRequest, + ); + + // Wait for async operations to complete + await fixture.whenStable(); + await component.determineVisibleBanners(); + fixture.detectChanges(); + + expect(component.visibleBanners).toEqual([]); + }); + }); + }); + + describe("message listener", () => { + beforeEach(async () => { + bannerService.shouldShowPendingAuthRequestBanner.mockResolvedValue(true); + messageSubject.next({ command: "openLoginApproval" }); + fixture.detectChanges(); + }); + + it("adds pending auth request banner when openLoginApproval message is received", async () => { + await component.ngOnInit(); + messageSubject.next({ command: "openLoginApproval" }); + fixture.detectChanges(); + + expect(component.visibleBanners).toContain(VisibleVaultBanner.PendingAuthRequest); + }); + + it("does not add duplicate pending auth request banner", async () => { + await component.ngOnInit(); + messageSubject.next({ command: "openLoginApproval" }); + messageSubject.next({ command: "openLoginApproval" }); + fixture.detectChanges(); + + const bannerCount = component.visibleBanners.filter( + (b) => b === VisibleVaultBanner.PendingAuthRequest, + ).length; + expect(bannerCount).toBe(1); + }); + + it("ignores other message types", async () => { + bannerService.shouldShowPendingAuthRequestBanner.mockResolvedValue(false); + await component.ngOnInit(); + messageSubject.next({ command: "someOtherCommand" }); + fixture.detectChanges(); + + expect(component.visibleBanners).not.toContain(VisibleVaultBanner.PendingAuthRequest); + }); }); }); diff --git a/apps/web/src/app/vault/individual-vault/vault-banners/vault-banners.component.ts b/apps/web/src/app/vault/individual-vault/vault-banners/vault-banners.component.ts index 5a0c0a535b4..5f5fc1e218d 100644 --- a/apps/web/src/app/vault/individual-vault/vault-banners/vault-banners.component.ts +++ b/apps/web/src/app/vault/individual-vault/vault-banners/vault-banners.component.ts @@ -1,11 +1,12 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { Component, Input, OnInit } from "@angular/core"; +import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { Router } from "@angular/router"; -import { firstValueFrom, map, Observable, switchMap } from "rxjs"; +import { firstValueFrom, map, Observable, switchMap, filter } from "rxjs"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { MessageListener } from "@bitwarden/common/platform/messaging"; +import { UserId } from "@bitwarden/common/types/guid"; import { BannerModule } from "@bitwarden/components"; import { VerifyEmailComponent } from "../../../auth/settings/verify-email.component"; @@ -34,10 +35,24 @@ export class VaultBannersComponent implements OnInit { private router: Router, private i18nService: I18nService, private accountService: AccountService, + private messageListener: MessageListener, ) { this.premiumBannerVisible$ = this.activeUserId$.pipe( + filter((userId): userId is UserId => userId != null), switchMap((userId) => this.vaultBannerService.shouldShowPremiumBanner$(userId)), ); + + // Listen for auth request messages and show banner immediately + this.messageListener.allMessages$ + .pipe( + filter((message: { command: string }) => message.command === "openLoginApproval"), + takeUntilDestroyed(), + ) + .subscribe(() => { + if (!this.visibleBanners.includes(VisibleVaultBanner.PendingAuthRequest)) { + this.visibleBanners = [...this.visibleBanners, VisibleVaultBanner.PendingAuthRequest]; + } + }); } async ngOnInit(): Promise { @@ -46,8 +61,10 @@ export class VaultBannersComponent implements OnInit { async dismissBanner(banner: VisibleVaultBanner): Promise { const activeUserId = await firstValueFrom(this.activeUserId$); + if (!activeUserId) { + return; + } await this.vaultBannerService.dismissBanner(activeUserId, banner); - await this.determineVisibleBanners(); } @@ -63,19 +80,26 @@ export class VaultBannersComponent implements OnInit { } /** Determine which banners should be present */ - private async determineVisibleBanners(): Promise { + async determineVisibleBanners(): Promise { const activeUserId = await firstValueFrom(this.activeUserId$); + if (!activeUserId) { + return; + } + const showBrowserOutdated = await this.vaultBannerService.shouldShowUpdateBrowserBanner(activeUserId); const showVerifyEmail = await this.vaultBannerService.shouldShowVerifyEmailBanner(activeUserId); const showLowKdf = await this.vaultBannerService.shouldShowLowKDFBanner(activeUserId); + const showPendingAuthRequest = + await this.vaultBannerService.shouldShowPendingAuthRequestBanner(activeUserId); this.visibleBanners = [ showBrowserOutdated ? VisibleVaultBanner.OutdatedBrowser : null, showVerifyEmail ? VisibleVaultBanner.VerifyEmail : null, showLowKdf ? VisibleVaultBanner.KDFSettings : null, - ].filter(Boolean); // remove all falsy values, i.e. null + showPendingAuthRequest ? VisibleVaultBanner.PendingAuthRequest : null, + ].filter((banner): banner is VisibleVaultBanner => banner !== null); // ensures the filtered array contains only VisibleVaultBanner values } freeTrialMessage(organization: FreeTrial) { diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index b6fb8279a72..352dfd1fc72 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -4127,6 +4127,12 @@ "updateBrowserDesc": { "message": "You are using an unsupported web browser. The web vault may not function properly." }, + "youHaveAPendingLoginRequest": { + "message": "You have a pending login request from another device." + }, + "reviewLoginRequest": { + "message": "Review login request" + }, "freeTrialEndPromptCount": { "message": "Your free trial ends in $COUNT$ days.", "placeholders": { diff --git a/libs/common/src/auth/abstractions/devices/views/device.view.ts b/libs/common/src/auth/abstractions/devices/views/device.view.ts index 22e522b9eb0..f3b78f216e0 100644 --- a/libs/common/src/auth/abstractions/devices/views/device.view.ts +++ b/libs/common/src/auth/abstractions/devices/views/device.view.ts @@ -1,20 +1,19 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { DeviceType } from "../../../../enums"; import { View } from "../../../../models/view/view"; import { DeviceResponse } from "../responses/device.response"; export class DeviceView implements View { - id: string; - userId: string; - name: string; - identifier: string; - type: DeviceType; - creationDate: string; - revisionDate: string; - response: DeviceResponse; + id: string | undefined; + userId: string | undefined; + name: string | undefined; + identifier: string | undefined; + type: DeviceType | undefined; + creationDate: string | undefined; + revisionDate: string | undefined; + response: DeviceResponse | undefined; constructor(deviceResponse: DeviceResponse) { Object.assign(this, deviceResponse); + this.response = deviceResponse; } } diff --git a/libs/common/src/auth/models/response/auth-request.response.ts b/libs/common/src/auth/models/response/auth-request.response.ts index d0c5d663065..88e7c542fae 100644 --- a/libs/common/src/auth/models/response/auth-request.response.ts +++ b/libs/common/src/auth/models/response/auth-request.response.ts @@ -6,7 +6,9 @@ const RequestTimeOut = 60000 * 15; //15 Minutes export class AuthRequestResponse extends BaseResponse { id: string; publicKey: string; - requestDeviceType: DeviceType; + requestDeviceType: string; + requestDeviceTypeValue: DeviceType; + requestDeviceIdentifier: string; requestIpAddress: string; key: string; // could be either an encrypted MasterKey or an encrypted UserKey masterPasswordHash: string; // if hash is present, the `key` above is an encrypted MasterKey (else `key` is an encrypted UserKey) @@ -21,6 +23,8 @@ export class AuthRequestResponse extends BaseResponse { this.id = this.getResponseProperty("Id"); this.publicKey = this.getResponseProperty("PublicKey"); this.requestDeviceType = this.getResponseProperty("RequestDeviceType"); + this.requestDeviceTypeValue = this.getResponseProperty("RequestDeviceTypeValue"); + this.requestDeviceIdentifier = this.getResponseProperty("RequestDeviceIdentifier"); this.requestIpAddress = this.getResponseProperty("RequestIpAddress"); this.key = this.getResponseProperty("Key"); this.masterPasswordHash = this.getResponseProperty("MasterPasswordHash"); From c85f6a54f50327426893d5c3c9117469a4df581a Mon Sep 17 00:00:00 2001 From: Vince Grassia <593223+vgrassia@users.noreply.github.com> Date: Mon, 24 Feb 2025 12:45:49 -0500 Subject: [PATCH 005/119] Add artifact names for macOS ARM64 builds (#13503) --- .github/workflows/release-cli.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/release-cli.yml b/.github/workflows/release-cli.yml index 6a10bec1ba2..519fee1989b 100644 --- a/.github/workflows/release-cli.yml +++ b/.github/workflows/release-cli.yml @@ -83,8 +83,12 @@ jobs: apps/cli/bw-windows-sha256-${{ env.PKG_VERSION }}.txt, apps/cli/bw-oss-macos-${{ env.PKG_VERSION }}.zip, apps/cli/bw-oss-macos-sha256-${{ env.PKG_VERSION }}.txt, + apps/cli/bw-oss-macos-arm64-${{ env.PKG_VERSION }}.zip, + apps/cli/bw-oss-macos-arm64-sha256-${{ env.PKG_VERSION }}.txt, apps/cli/bw-macos-${{ env.PKG_VERSION }}.zip, apps/cli/bw-macos-sha256-${{ env.PKG_VERSION }}.txt, + apps/cli/bw-macos-arm64-${{ env.PKG_VERSION }}.zip, + apps/cli/bw-macos-arm64-sha256-${{ env.PKG_VERSION }}.txt, apps/cli/bw-oss-linux-${{ env.PKG_VERSION }}.zip, apps/cli/bw-oss-linux-sha256-${{ env.PKG_VERSION }}.txt, apps/cli/bw-linux-${{ env.PKG_VERSION }}.zip, From a5ea8066a36570fe0d8d429ea73dfa2aceb767bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa?= Date: Mon, 24 Feb 2025 19:24:05 +0100 Subject: [PATCH 006/119] [PM-14214] Initialize PopupViewCacheService earlier (#13540) --- apps/browser/src/popup/app.component.ts | 3 --- apps/browser/src/popup/services/init.service.ts | 5 ++++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/browser/src/popup/app.component.ts b/apps/browser/src/popup/app.component.ts index 66be31969dd..9a3b6429e61 100644 --- a/apps/browser/src/popup/app.component.ts +++ b/apps/browser/src/popup/app.component.ts @@ -24,7 +24,6 @@ import { import { BiometricsService, BiometricStateService } from "@bitwarden/key-management"; import { PopupCompactModeService } from "../platform/popup/layout/popup-compact-mode.service"; -import { PopupViewCacheService } from "../platform/popup/view-cache/popup-view-cache.service"; import { initPopupClosedListener } from "../platform/services/popup-view-cache-background.service"; import { VaultBrowserStateService } from "../vault/services/vault-browser-state.service"; @@ -43,7 +42,6 @@ import { DesktopSyncVerificationDialogComponent } from "./components/desktop-syn `, }) export class AppComponent implements OnInit, OnDestroy { - private viewCacheService = inject(PopupViewCacheService); private compactModeService = inject(PopupCompactModeService); private lastActivity: Date; @@ -74,7 +72,6 @@ export class AppComponent implements OnInit, OnDestroy { async ngOnInit() { initPopupClosedListener(); - await this.viewCacheService.init(); this.compactModeService.init(); diff --git a/apps/browser/src/popup/services/init.service.ts b/apps/browser/src/popup/services/init.service.ts index 069ebf4020d..2ca25d690f1 100644 --- a/apps/browser/src/popup/services/init.service.ts +++ b/apps/browser/src/popup/services/init.service.ts @@ -12,6 +12,8 @@ import { StateService } from "@bitwarden/common/platform/abstractions/state.serv import { BrowserApi } from "../../platform/browser/browser-api"; import BrowserPopupUtils from "../../platform/popup/browser-popup-utils"; import { PopupSizeService } from "../../platform/popup/layout/popup-size.service"; +import { PopupViewCacheService } from "../../platform/popup/view-cache/popup-view-cache.service"; + @Injectable() export class InitService { private sizeService = inject(PopupSizeService); @@ -24,6 +26,7 @@ export class InitService { private logService: LogServiceAbstraction, private themingService: AbstractThemingService, private sdkLoadService: SdkLoadService, + private viewCacheService: PopupViewCacheService, @Inject(DOCUMENT) private document: Document, ) {} @@ -33,7 +36,7 @@ export class InitService { await this.stateService.init({ runMigrations: false }); // Browser background is responsible for migrations await this.i18nService.init(); this.twoFactorService.init(); - + await this.viewCacheService.init(); await this.sizeService.init(); const htmlEl = window.document.documentElement; From 62d3fe21a6f4a2826636e3936d532fa61ef7300d Mon Sep 17 00:00:00 2001 From: Jared Snider <116684653+JaredSnider-Bitwarden@users.noreply.github.com> Date: Mon, 24 Feb 2025 15:48:09 -0500 Subject: [PATCH 007/119] feat(2FA-Setup/Recovery-Dialogs): [Auth/PM-11701] Refactor dialogs to properly use title & subtitle inputs for consistent styling (#13493) --- .../two-factor/two-factor-recovery.component.html | 10 +++++----- .../two-factor-setup-authenticator.component.html | 10 +++++----- .../two-factor/two-factor-setup-duo.component.html | 6 +----- .../two-factor/two-factor-setup-email.component.html | 6 +----- .../two-factor-setup-webauthn.component.html | 10 +++++----- .../two-factor/two-factor-setup-yubikey.component.html | 6 +----- .../two-factor/two-factor-verify.component.html | 6 +----- 7 files changed, 19 insertions(+), 35 deletions(-) diff --git a/apps/web/src/app/auth/settings/two-factor/two-factor-recovery.component.html b/apps/web/src/app/auth/settings/two-factor/two-factor-recovery.component.html index 98676509078..413432e5a02 100644 --- a/apps/web/src/app/auth/settings/two-factor/two-factor-recovery.component.html +++ b/apps/web/src/app/auth/settings/two-factor/two-factor-recovery.component.html @@ -1,8 +1,8 @@ - - - {{ "twoStepLogin" | i18n }} - {{ "recoveryCodeTitle" | i18n }} - +

{{ "twoFactorRecoveryYourCode" | i18n }}:

diff --git a/apps/web/src/app/auth/settings/two-factor/two-factor-setup-authenticator.component.html b/apps/web/src/app/auth/settings/two-factor/two-factor-setup-authenticator.component.html index c9214d59caa..a31d4c33458 100644 --- a/apps/web/src/app/auth/settings/two-factor/two-factor-setup-authenticator.component.html +++ b/apps/web/src/app/auth/settings/two-factor/two-factor-setup-authenticator.component.html @@ -1,9 +1,9 @@
- - - {{ "twoStepLogin" | i18n }} - {{ "authenticatorAppTitle" | i18n }} - + diff --git a/apps/web/src/app/auth/settings/two-factor/two-factor-setup-duo.component.html b/apps/web/src/app/auth/settings/two-factor/two-factor-setup-duo.component.html index f20bd4f5f70..3a62ec91a8e 100644 --- a/apps/web/src/app/auth/settings/two-factor/two-factor-setup-duo.component.html +++ b/apps/web/src/app/auth/settings/two-factor/two-factor-setup-duo.component.html @@ -1,9 +1,5 @@ - - - {{ "twoStepLogin" | i18n }} - Duo - + diff --git a/apps/web/src/app/auth/settings/two-factor/two-factor-setup-email.component.html b/apps/web/src/app/auth/settings/two-factor/two-factor-setup-email.component.html index f79a3bc7b0a..5861d1fba4f 100644 --- a/apps/web/src/app/auth/settings/two-factor/two-factor-setup-email.component.html +++ b/apps/web/src/app/auth/settings/two-factor/two-factor-setup-email.component.html @@ -1,9 +1,5 @@ - - - {{ "twoStepLogin" | i18n }} - {{ "emailTitle" | i18n }} - + diff --git a/apps/web/src/app/auth/settings/two-factor/two-factor-setup-webauthn.component.html b/apps/web/src/app/auth/settings/two-factor/two-factor-setup-webauthn.component.html index c9e2e111481..8e7a789d9d0 100644 --- a/apps/web/src/app/auth/settings/two-factor/two-factor-setup-webauthn.component.html +++ b/apps/web/src/app/auth/settings/two-factor/two-factor-setup-webauthn.component.html @@ -1,9 +1,9 @@ - - - {{ "twoStepLogin" | i18n }} - {{ "webAuthnTitle" | i18n }} - + - - - {{ "twoStepLogin" | i18n }} - YubiKey - + - - - {{ "twoStepLogin" | i18n }} - {{ dialogTitle }} - + Date: Mon, 24 Feb 2025 15:51:56 -0500 Subject: [PATCH 008/119] feat(LoginViaAuthRequestComponent): [Auth/PM-18506] fix missing spaces (#13543) --- .../login-via-auth-request.component.html | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libs/auth/src/angular/login-via-auth-request/login-via-auth-request.component.html b/libs/auth/src/angular/login-via-auth-request/login-via-auth-request.component.html index ba26ba77cb0..22cf8320036 100644 --- a/libs/auth/src/angular/login-via-auth-request/login-via-auth-request.component.html +++ b/libs/auth/src/angular/login-via-auth-request/login-via-auth-request.component.html @@ -32,7 +32,8 @@
- {{ "needAnotherOptionV1" | i18n }} + {{ "needAnotherOptionV1" | i18n }}  {{ "viewAllLogInOptions" | i18n }} @@ -46,7 +47,8 @@ {{ fingerprintPhrase }}
- {{ "troubleLoggingIn" | i18n }} + {{ "troubleLoggingIn" | i18n }}  {{ "viewAllLogInOptions" | i18n }} From 8552578fbb581f4091f000c1ce1a557951ed9533 Mon Sep 17 00:00:00 2001 From: Jordan Aasen <166539328+jaasen-livefront@users.noreply.github.com> Date: Mon, 24 Feb 2025 13:53:02 -0800 Subject: [PATCH 009/119] [PM-12720] - sort organizations in vault item owner select (#13419) * sort owner select options and filters by name * don't sort filters * fix tests * fix tests * set organizations in ngOninit * move assignment up * Revert change to add-edit.component.ts. Move assignment up. * fix tests --- .../item-details-section.component.html | 2 +- .../item-details-section.component.spec.ts | 38 ++++++++++++++++--- .../item-details-section.component.ts | 11 ++++-- 3 files changed, 40 insertions(+), 11 deletions(-) diff --git a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html index c68df5bbfac..40a8954b05a 100644 --- a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html +++ b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html @@ -27,7 +27,7 @@ [label]="userEmail$ | async" > diff --git a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.spec.ts b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.spec.ts index 3995422944c..aa68770774a 100644 --- a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.spec.ts +++ b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.spec.ts @@ -59,6 +59,9 @@ describe("ItemDetailsSectionComponent", () => { initializedWithCachedCipher, }); i18nService = mock(); + i18nService.collator = { + compare: (a: string, b: string) => a.localeCompare(b), + } as Intl.Collator; await TestBed.configureTestingModule({ imports: [ItemDetailsSectionComponent, CommonModule, ReactiveFormsModule], @@ -184,16 +187,18 @@ describe("ItemDetailsSectionComponent", () => { it("should allow ownership change if personal ownership is allowed and there is at least one organization", () => { component.config.allowPersonalOwnership = true; - component.config.organizations = [{ id: "org1" } as Organization]; + component.config.organizations = [{ id: "org1", name: "org1" } as Organization]; + fixture.detectChanges(); expect(component.allowOwnershipChange).toBe(true); }); it("should allow ownership change if personal ownership is not allowed but there is more than one organization", () => { component.config.allowPersonalOwnership = false; component.config.organizations = [ - { id: "org1" } as Organization, - { id: "org2" } as Organization, + { id: "org1", name: "org1" } as Organization, + { id: "org2", name: "org2" } as Organization, ]; + fixture.detectChanges(); expect(component.allowOwnershipChange).toBe(true); }); }); @@ -206,7 +211,8 @@ describe("ItemDetailsSectionComponent", () => { it("should return the first organization id if personal ownership is not allowed", () => { component.config.allowPersonalOwnership = false; - component.config.organizations = [{ id: "org1" } as Organization]; + component.config.organizations = [{ id: "org1", name: "Organization 1" } as Organization]; + fixture.detectChanges(); expect(component.defaultOwner).toBe("org1"); }); }); @@ -250,6 +256,7 @@ describe("ItemDetailsSectionComponent", () => { jest.spyOn(component, "allowOwnershipChange", "get").mockReturnValue(false); component.config.mode = "edit"; component.config.organizations = [{ id: "org1" } as Organization]; + fixture.detectChanges(); expect(component.showOwnership).toBe(true); }); @@ -322,8 +329,8 @@ describe("ItemDetailsSectionComponent", () => { it("should select the first organization if personal ownership is not allowed", async () => { component.config.allowPersonalOwnership = false; component.config.organizations = [ - { id: "org1" } as Organization, - { id: "org2" } as Organization, + { id: "org1", name: "org1" } as Organization, + { id: "org2", name: "org2" } as Organization, ]; component.originalCipherView = { name: "cipher1", @@ -517,4 +524,23 @@ describe("ItemDetailsSectionComponent", () => { expect(component["readOnlyCollectionsNames"]).toEqual(["Collection 1", "Collection 3"]); }); }); + + describe("organizationOptions", () => { + it("should sort the organizations by name", async () => { + component.config.mode = "edit"; + component.config.organizations = [ + { id: "org2", name: "org2" } as Organization, + { id: "org1", name: "org1" } as Organization, + ]; + component.originalCipherView = {} as CipherView; + + await component.ngOnInit(); + fixture.detectChanges(); + + const select = fixture.debugElement.query(By.directive(SelectComponent)); + const { label } = select.componentInstance.items[0]; + + expect(label).toBe("org1"); + }); + }); }); diff --git a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts index 50bafd48b41..dcbc4e8c92f 100644 --- a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts +++ b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts @@ -12,6 +12,7 @@ import { OrganizationUserType } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { Utils } from "@bitwarden/common/platform/misc/utils"; import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { @@ -74,6 +75,8 @@ export class ItemDetailsSectionComponent implements OnInit { /** The email address associated with the active account */ protected userEmail$ = this.accountService.activeAccount$.pipe(map((account) => account.email)); + protected organizations: Organization[] = []; + @Input({ required: true }) config: CipherFormConfig; @@ -90,10 +93,6 @@ export class ItemDetailsSectionComponent implements OnInit { return this.config.mode === "partial-edit"; } - get organizations(): Organization[] { - return this.config.organizations; - } - get allowPersonalOwnership() { return this.config.allowPersonalOwnership; } @@ -186,6 +185,10 @@ export class ItemDetailsSectionComponent implements OnInit { } async ngOnInit() { + this.organizations = this.config.organizations.sort( + Utils.getSortFunction(this.i18nService, "name"), + ); + if (!this.allowPersonalOwnership && this.organizations.length === 0) { throw new Error("No organizations available for ownership."); } From e06a482d6e3d6474927d52e92ddc6e9fb3824dbb Mon Sep 17 00:00:00 2001 From: Jared Snider <116684653+JaredSnider-Bitwarden@users.noreply.github.com> Date: Mon, 24 Feb 2025 17:27:15 -0500 Subject: [PATCH 010/119] fix(NewDeviceVerification): [Auth/PM-18580] Fix loading state on submit (#13545) --- .../new-device-verification.component.html | 1 + 1 file changed, 1 insertion(+) diff --git a/libs/auth/src/angular/new-device-verification/new-device-verification.component.html b/libs/auth/src/angular/new-device-verification/new-device-verification.component.html index 2f807d32993..e731f3afcb6 100644 --- a/libs/auth/src/angular/new-device-verification/new-device-verification.component.html +++ b/libs/auth/src/angular/new-device-verification/new-device-verification.component.html @@ -25,6 +25,7 @@
+ +
+ {{ "newCustomizationOptionsCalloutContent" | i18n }} + + {{ "newCustomizationOptionsCalloutLink" | i18n }} + +
+ + diff --git a/apps/browser/src/vault/popup/components/vault-v2/new-settings-callout/new-settings-callout.component.ts b/apps/browser/src/vault/popup/components/vault-v2/new-settings-callout/new-settings-callout.component.ts new file mode 100644 index 00000000000..71549906474 --- /dev/null +++ b/apps/browser/src/vault/popup/components/vault-v2/new-settings-callout/new-settings-callout.component.ts @@ -0,0 +1,69 @@ +import { CommonModule } from "@angular/common"; +import { Component, OnDestroy, OnInit } from "@angular/core"; +import { Router } from "@angular/router"; +import { firstValueFrom } from "rxjs"; + +import { JslibModule } from "@bitwarden/angular/jslib.module"; +import { VaultProfileService } from "@bitwarden/angular/vault/services/vault-profile.service"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; +import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { UserId } from "@bitwarden/common/types/guid"; +import { ButtonModule, PopoverModule } from "@bitwarden/components"; + +import { VaultPageService } from "../vault-page.service"; + +@Component({ + selector: "new-settings-callout", + templateUrl: "new-settings-callout.component.html", + standalone: true, + imports: [PopoverModule, JslibModule, CommonModule, ButtonModule], + providers: [VaultPageService], +}) +export class NewSettingsCalloutComponent implements OnInit, OnDestroy { + protected showNewCustomizationSettingsCallout = false; + protected activeUserId: UserId | null = null; + + constructor( + private accountService: AccountService, + private vaultProfileService: VaultProfileService, + private vaultPageService: VaultPageService, + private router: Router, + private logService: LogService, + ) {} + + async ngOnInit() { + this.activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + + let profileCreatedDate: Date; + + try { + profileCreatedDate = await this.vaultProfileService.getProfileCreationDate(this.activeUserId); + } catch (e) { + this.logService.error("Error getting profile creation date", e); + // Default to before the cutoff date to ensure the callout is shown + profileCreatedDate = new Date("2024-12-24"); + } + + const hasCalloutBeenDismissed = await firstValueFrom( + this.vaultPageService.isCalloutDismissed(this.activeUserId), + ); + + this.showNewCustomizationSettingsCallout = + !hasCalloutBeenDismissed && profileCreatedDate < new Date("2024-12-25"); + } + + async goToAppearance() { + await this.router.navigate(["/appearance"]); + } + + async dismissCallout() { + if (this.activeUserId) { + await this.vaultPageService.dismissCallout(this.activeUserId); + } + } + + async ngOnDestroy() { + await this.dismissCallout(); + } +} diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-page.service.ts b/apps/browser/src/vault/popup/components/vault-v2/vault-page.service.ts new file mode 100644 index 00000000000..75354298c26 --- /dev/null +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-page.service.ts @@ -0,0 +1,41 @@ +import { inject, Injectable } from "@angular/core"; +import { map, Observable } from "rxjs"; + +import { + BANNERS_DISMISSED_DISK, + StateProvider, + UserKeyDefinition, +} from "@bitwarden/common/platform/state"; +import { UserId } from "@bitwarden/common/types/guid"; + +export const NEW_CUSTOMIZATION_OPTIONS_CALLOUT_DISMISSED_KEY = new UserKeyDefinition( + BANNERS_DISMISSED_DISK, + "newCustomizationOptionsCalloutDismissed", + { + deserializer: (calloutDismissed) => calloutDismissed, + clearOn: [], // Do not clear dismissed callouts + }, +); + +@Injectable() +export class VaultPageService { + private stateProvider = inject(StateProvider); + + async unDismissCallout(userId: UserId): Promise { + await this.stateProvider + .getUser(userId, NEW_CUSTOMIZATION_OPTIONS_CALLOUT_DISMISSED_KEY) + .update(() => false); + } + + isCalloutDismissed(userId: UserId): Observable { + return this.stateProvider + .getUser(userId, NEW_CUSTOMIZATION_OPTIONS_CALLOUT_DISMISSED_KEY) + .state$.pipe(map((dismissed) => !!dismissed)); + } + + async dismissCallout(userId: UserId): Promise { + await this.stateProvider + .getUser(userId, NEW_CUSTOMIZATION_OPTIONS_CALLOUT_DISMISSED_KEY) + .update(() => true); + } +} diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.html b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.html index 8cb538a429a..bb7cd8e52d0 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.html +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.html @@ -85,4 +85,5 @@ >
+ diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts index 73b691bc4ac..1ae1b205af3 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-v2.component.ts @@ -15,6 +15,7 @@ import { } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; +import { VaultProfileService } from "@bitwarden/angular/vault/services/vault-profile.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; @@ -44,7 +45,9 @@ import { NewItemDropdownV2Component, NewItemInitialValues, } from "./new-item-dropdown/new-item-dropdown-v2.component"; +import { NewSettingsCalloutComponent } from "./new-settings-callout/new-settings-callout.component"; import { VaultHeaderV2Component } from "./vault-header/vault-header-v2.component"; +import { VaultPageService } from "./vault-page.service"; import { AutofillVaultListItemsComponent, VaultListItemsContainerComponent } from "."; @@ -77,7 +80,9 @@ enum VaultState { DecryptionFailureDialogComponent, BannerComponent, AtRiskPasswordCalloutComponent, + NewSettingsCalloutComponent, ], + providers: [VaultPageService], }) export class VaultV2Component implements OnInit, AfterViewInit, OnDestroy { @ViewChild(CdkVirtualScrollableElement) virtualScrollElement?: CdkVirtualScrollableElement; @@ -115,6 +120,7 @@ export class VaultV2Component implements OnInit, AfterViewInit, OnDestroy { protected noResultsIcon = Icons.NoResults; protected VaultStateEnum = VaultState; + protected showNewCustomizationSettingsCallout = false; constructor( private vaultPopupItemsService: VaultPopupItemsService, @@ -124,6 +130,8 @@ export class VaultV2Component implements OnInit, AfterViewInit, OnDestroy { private destroyRef: DestroyRef, private cipherService: CipherService, private dialogService: DialogService, + private vaultProfileService: VaultProfileService, + private vaultPageService: VaultPageService, ) { combineLatest([ this.vaultPopupItemsService.emptyVault$, @@ -178,7 +186,7 @@ export class VaultV2Component implements OnInit, AfterViewInit, OnDestroy { }); } - ngOnDestroy(): void { + ngOnDestroy() { this.vaultScrollPositionService.stop(); } From bc415d807c8da0c08eedf658a092bfc08b619146 Mon Sep 17 00:00:00 2001 From: Jordan Aasen <166539328+jaasen-livefront@users.noreply.github.com> Date: Mon, 24 Feb 2025 14:56:15 -0800 Subject: [PATCH 012/119] [PM-13989] - Extension Vault screen - allow copy icon to copy data directly if only 1 piece of data is available (#13520) * wip - copy button overhaul * finalize item copy actions single item copy --- apps/browser/src/_locales/en/messages.json | 14 ++ .../item-copy-actions.component.html | 136 +++++++++++------- .../item-copy-actions.component.ts | 52 ++++++- 3 files changed, 153 insertions(+), 49 deletions(-) diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index b2d042945b0..ea9e62916a2 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -4236,6 +4236,20 @@ } } }, + "copyFieldValue": { + "message": "Copy $FIELD$, $VALUE$", + "description": "Title for a button that copies a field value to the clipboard.", + "placeholders": { + "field": { + "content": "$1", + "example": "Username" + }, + "value": { + "content": "$2", + "example": "Foo" + } + } + }, "noValuesToCopy": { "message": "No values to copy" }, diff --git a/apps/browser/src/vault/popup/components/vault-v2/item-copy-action/item-copy-actions.component.html b/apps/browser/src/vault/popup/components/vault-v2/item-copy-action/item-copy-actions.component.html index fbfebe8efff..bb3a7b12096 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/item-copy-action/item-copy-actions.component.html +++ b/apps/browser/src/vault/popup/components/vault-v2/item-copy-action/item-copy-actions.component.html @@ -36,32 +36,46 @@ - - + - - + bitIconButton="bwi-clone" + size="small" + [appA11yTitle]=" + hasLoginValues ? ('copyInfoTitle' | i18n: cipher.name) : ('noValuesToCopy' | i18n) + " + [disabled]="!hasLoginValues" + [bitMenuTriggerFor]="loginOptions" + > + + + + + + @@ -92,52 +106,78 @@ - - - - + + + + + + + - - - - - - + + + + + + + + + diff --git a/apps/browser/src/vault/popup/components/vault-v2/item-copy-action/item-copy-actions.component.ts b/apps/browser/src/vault/popup/components/vault-v2/item-copy-action/item-copy-actions.component.ts index 53439dc4abd..a51e5f5406a 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/item-copy-action/item-copy-actions.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/item-copy-action/item-copy-actions.component.ts @@ -4,6 +4,7 @@ import { CommonModule } from "@angular/common"; import { Component, Input, inject } from "@angular/core"; import { JslibModule } from "@bitwarden/angular/jslib.module"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { CipherType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { IconButtonModule, ItemModule, MenuModule } from "@bitwarden/components"; @@ -11,6 +12,11 @@ import { CopyCipherFieldDirective } from "@bitwarden/vault"; import { VaultPopupCopyButtonsService } from "../../../services/vault-popup-copy-buttons.service"; +type CipherItem = { + value: string; + key: string; +}; + @Component({ standalone: true, selector: "app-item-copy-actions", @@ -37,6 +43,50 @@ export class ItemCopyActionsComponent { ); } + get singleCopiableLogin() { + const loginItems: CipherItem[] = [ + { value: this.cipher.login.username, key: "username" }, + { value: this.cipher.login.password, key: "password" }, + { value: this.cipher.login.totp, key: "totp" }, + ]; + // If both the password and username are visible but the password is hidden, return the username + if (!this.cipher.viewPassword && this.cipher.login.username && this.cipher.login.password) { + return { value: this.cipher.login.username, key: this.i18nService.t("username") }; + } + return this.findSingleCopiableItem(loginItems); + } + + get singleCopiableCard() { + const cardItems: CipherItem[] = [ + { value: this.cipher.card.code, key: "code" }, + { value: this.cipher.card.number, key: "number" }, + ]; + return this.findSingleCopiableItem(cardItems); + } + + get singleCopiableIdentity() { + const identityItems: CipherItem[] = [ + { value: this.cipher.identity.fullAddressForCopy, key: "address" }, + { value: this.cipher.identity.email, key: "email" }, + { value: this.cipher.identity.username, key: "username" }, + { value: this.cipher.identity.phone, key: "phone" }, + ]; + return this.findSingleCopiableItem(identityItems); + } + + /* + * Given a list of CipherItems, if there is only one item with a value, + * return it with the translated key. Otherwise return null + */ + findSingleCopiableItem(items: { value: string; key: string }[]): CipherItem | null { + const singleItemWithValue = items.find( + (key) => key.value && items.every((f) => f === key || !f.value), + ); + return singleItemWithValue + ? { value: singleItemWithValue.value, key: this.i18nService.t(singleItemWithValue.key) } + : null; + } + get hasCardValues() { return !!this.cipher.card.code || !!this.cipher.card.number; } @@ -62,5 +112,5 @@ export class ItemCopyActionsComponent { ); } - constructor() {} + constructor(private i18nService: I18nService) {} } From f66446fa69ff6f0e54028d35d2130142a6e73360 Mon Sep 17 00:00:00 2001 From: Oscar Hinton Date: Tue, 25 Feb 2025 11:05:33 +0100 Subject: [PATCH 013/119] Renovate: disable major upgrades of angular (#13533) * Renovate: disable major upgrades of angular --- .github/renovate.json5 | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/.github/renovate.json5 b/.github/renovate.json5 index 6d6fbbd2539..a0826039bb8 100644 --- a/.github/renovate.json5 +++ b/.github/renovate.json5 @@ -22,6 +22,18 @@ description: "Determined by Angular", enabled: false, }, + { + matchSourceUrls: [ + "https://github.com/angular-eslint/angular-eslint", + "https://github.com/angular/angular-cli", + "https://github.com/angular/angular", + "https://github.com/angular/components", + "https://github.com/ng-select/ng-select", + ], + matchUpdateTypes: ["major"], + description: "Manually updated using ng update", + enabled: false, + }, { matchPackageNames: ["typescript", "zone.js"], matchUpdateTypes: "patch", @@ -90,12 +102,8 @@ }, { matchPackageNames: [ - "@angular-eslint/eslint-plugin-template", - "@angular-eslint/eslint-plugin", "@angular-eslint/schematics", - "@angular-eslint/template-parser", - "@typescript-eslint/eslint-plugin", - "@typescript-eslint/parser", + "angular-eslint", "eslint-config-prettier", "eslint-import-resolver-typescript", "eslint-plugin-import", @@ -106,6 +114,7 @@ "eslint", "husky", "lint-staged", + "typescript-eslint", ], groupName: "Linting minor-patch", matchUpdateTypes: ["minor", "patch"], From 240f9f934894154f6e98ace93357bd27ded40c88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa?= Date: Tue, 25 Feb 2025 12:30:26 +0100 Subject: [PATCH 014/119] Make native-messaging-test-runner use desktop_proxy (#11923) * Make native-messaging-test-runner use desktop_proxy * Remove node-ipc * Fix build and implement proxy selection * Remove eslint disable --------- Co-authored-by: Matt Bishop --- .../package-lock.json | 81 +-------- .../native-messaging-test-runner/package.json | 4 +- .../src/ipc.service.ts | 169 +++++++++++++----- .../tsconfig.json | 9 +- package-lock.json | 70 -------- package.json | 2 - 6 files changed, 140 insertions(+), 195 deletions(-) diff --git a/apps/desktop/native-messaging-test-runner/package-lock.json b/apps/desktop/native-messaging-test-runner/package-lock.json index f9c5f0709e4..d3f6c53a373 100644 --- a/apps/desktop/native-messaging-test-runner/package-lock.json +++ b/apps/desktop/native-messaging-test-runner/package-lock.json @@ -12,15 +12,13 @@ "@bitwarden/common": "file:../../../libs/common", "@bitwarden/node": "file:../../../libs/node", "module-alias": "2.2.3", - "node-ipc": "9.2.1", "ts-node": "10.9.2", "uuid": "11.0.5", "yargs": "17.7.2" }, "devDependencies": { "@types/node": "22.10.7", - "@types/node-ipc": "9.2.3", - "typescript": "4.7.4" + "typescript": "5.4.2" } }, "../../../libs/common": { @@ -31,10 +29,7 @@ "../../../libs/node": { "name": "@bitwarden/node", "version": "0.0.0", - "license": "GPL-3.0", - "dependencies": { - "@bitwarden/common": "file:../common" - } + "license": "GPL-3.0" }, "node_modules/@bitwarden/common": { "resolved": "../../../libs/common", @@ -114,16 +109,6 @@ "undici-types": "~6.20.0" } }, - "node_modules/@types/node-ipc": { - "version": "9.2.3", - "resolved": "https://registry.npmjs.org/@types/node-ipc/-/node-ipc-9.2.3.tgz", - "integrity": "sha512-/MvSiF71fYf3+zwqkh/zkVkZj1hl1Uobre9EMFy08mqfJNAmpR0vmPgOUdEIDVgifxHj6G1vYMPLSBLLxoDACQ==", - "dev": true, - "license": "MIT", - "dependencies": { - "@types/node": "*" - } - }, "node_modules/acorn": { "version": "8.14.0", "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.14.0.tgz", @@ -225,15 +210,6 @@ "node": ">=0.3.1" } }, - "node_modules/easy-stack": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/easy-stack/-/easy-stack-1.0.1.tgz", - "integrity": "sha512-wK2sCs4feiiJeFXn3zvY0p41mdU5VUgbgs1rNsc/y5ngFUijdWd+iIN8eoyuZHKB8xN6BL4PdWmzqFmxNg6V2w==", - "license": "MIT", - "engines": { - "node": ">=6.0.0" - } - }, "node_modules/emoji-regex": { "version": "8.0.0", "resolved": "https://registry.npmjs.org/emoji-regex/-/emoji-regex-8.0.0.tgz", @@ -249,15 +225,6 @@ "node": ">=6" } }, - "node_modules/event-pubsub": { - "version": "4.3.0", - "resolved": "https://registry.npmjs.org/event-pubsub/-/event-pubsub-4.3.0.tgz", - "integrity": "sha512-z7IyloorXvKbFx9Bpie2+vMJKKx1fH1EN5yiTfp8CiLOTptSYy1g8H4yDpGlEdshL1PBiFtBHepF2cNsqeEeFQ==", - "license": "Unlicense", - "engines": { - "node": ">=4.0.0" - } - }, "node_modules/get-caller-file": { "version": "2.0.5", "resolved": "https://registry.npmjs.org/get-caller-file/-/get-caller-file-2.0.5.tgz", @@ -276,27 +243,6 @@ "node": ">=8" } }, - "node_modules/js-message": { - "version": "1.0.7", - "resolved": "https://registry.npmjs.org/js-message/-/js-message-1.0.7.tgz", - "integrity": "sha512-efJLHhLjIyKRewNS9EGZ4UpI8NguuL6fKkhRxVuMmrGV2xN/0APGdQYwLFky5w9naebSZ0OwAGp0G6/2Cg90rA==", - "license": "MIT", - "engines": { - "node": ">=0.6.0" - } - }, - "node_modules/js-queue": { - "version": "2.0.2", - "resolved": "https://registry.npmjs.org/js-queue/-/js-queue-2.0.2.tgz", - "integrity": "sha512-pbKLsbCfi7kriM3s1J4DDCo7jQkI58zPLHi0heXPzPlj0hjUsm+FesPUbE0DSbIVIK503A36aUBoCN7eMFedkA==", - "license": "MIT", - "dependencies": { - "easy-stack": "^1.0.1" - }, - "engines": { - "node": ">=1.0.0" - } - }, "node_modules/make-error": { "version": "1.3.6", "resolved": "https://registry.npmjs.org/make-error/-/make-error-1.3.6.tgz", @@ -309,20 +255,6 @@ "integrity": "sha512-23g5BFj4zdQL/b6tor7Ji+QY4pEfNH784BMslY9Qb0UnJWRAt+lQGLYmRaM0KDBwIG23ffEBELhZDP2rhi9f/Q==", "license": "MIT" }, - "node_modules/node-ipc": { - "version": "9.2.1", - "resolved": "https://registry.npmjs.org/node-ipc/-/node-ipc-9.2.1.tgz", - "integrity": "sha512-mJzaM6O3xHf9VT8BULvJSbdVbmHUKRNOH7zDDkCrA1/T+CVjq2WVIDfLt0azZRXpgArJtl3rtmEozrbXPZ9GaQ==", - "license": "MIT", - "dependencies": { - "event-pubsub": "4.3.0", - "js-message": "1.0.7", - "js-queue": "2.0.2" - }, - "engines": { - "node": ">=8.0.0" - } - }, "node_modules/require-directory": { "version": "2.1.1", "resolved": "https://registry.npmjs.org/require-directory/-/require-directory-2.1.1.tgz", @@ -402,16 +334,15 @@ } }, "node_modules/typescript": { - "version": "4.7.4", - "resolved": "https://registry.npmjs.org/typescript/-/typescript-4.7.4.tgz", - "integrity": "sha512-C0WQT0gezHuw6AdY1M2jxUO83Rjf0HP7Sk1DtXj6j1EwkQNZrHAg2XPWlq62oqEhYvONq5pkC2Y9oPljWToLmQ==", - "license": "Apache-2.0", + "version": "5.4.2", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-5.4.2.tgz", + "integrity": "sha512-+2/g0Fds1ERlP6JsakQQDXjZdZMM+rqpamFZJEKh4kwTIn3iDkgKtby0CeNd5ATNZ4Ry1ax15TMx0W2V+miizQ==", "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" }, "engines": { - "node": ">=4.2.0" + "node": ">=14.17" } }, "node_modules/undici-types": { diff --git a/apps/desktop/native-messaging-test-runner/package.json b/apps/desktop/native-messaging-test-runner/package.json index 977b93e70d2..0df272c142f 100644 --- a/apps/desktop/native-messaging-test-runner/package.json +++ b/apps/desktop/native-messaging-test-runner/package.json @@ -17,15 +17,13 @@ "@bitwarden/common": "file:../../../libs/common", "@bitwarden/node": "file:../../../libs/node", "module-alias": "2.2.3", - "node-ipc": "9.2.1", "ts-node": "10.9.2", "uuid": "11.0.5", "yargs": "17.7.2" }, "devDependencies": { "@types/node": "22.10.7", - "@types/node-ipc": "9.2.3", - "typescript": "4.7.4" + "typescript": "5.4.2" }, "_moduleAliases": { "@bitwarden/common": "dist/libs/common/src", diff --git a/apps/desktop/native-messaging-test-runner/src/ipc.service.ts b/apps/desktop/native-messaging-test-runner/src/ipc.service.ts index 68c7ac73ab0..b02ff1a4225 100644 --- a/apps/desktop/native-messaging-test-runner/src/ipc.service.ts +++ b/apps/desktop/native-messaging-test-runner/src/ipc.service.ts @@ -1,9 +1,7 @@ /* eslint-disable no-console */ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore -import { homedir } from "os"; - -import * as NodeIPC from "node-ipc"; +import { ChildProcess, spawn } from "child_process"; +import * as fs from "fs"; +import * as path from "path"; // eslint-disable-next-line no-restricted-imports import { MessageCommon } from "../../src/models/native-messaging/message-common"; @@ -13,11 +11,6 @@ import { UnencryptedMessageResponse } from "../../src/models/native-messaging/un import Deferred from "./deferred"; import { race } from "./race"; -NodeIPC.config.id = "native-messaging-test-runner"; -NodeIPC.config.maxRetries = 0; -NodeIPC.config.silent = true; - -const DESKTOP_APP_PATH = `${homedir}/tmp/app.bitwarden`; const DEFAULT_MESSAGE_TIMEOUT = 10 * 1000; // 10 seconds export type MessageHandler = (MessageCommon) => void; @@ -42,6 +35,10 @@ export default class IPCService { // A set of deferred promises that are awaiting socket connection private awaitingConnection = new Set>(); + // The IPC desktop_proxy process + private process?: ChildProcess; + private processOutputBuffer = Buffer.alloc(0); + constructor( private socketName: string, private messageHandler: MessageHandler, @@ -72,47 +69,47 @@ export default class IPCService { private _connect() { this.connectionState = IPCConnectionState.Connecting; - NodeIPC.connectTo(this.socketName, DESKTOP_APP_PATH, () => { - // Process incoming message - this.getSocket().on("message", (message: any) => { - this.processMessage(message); - }); + const proxyPath = selectProxyPath(); + console.log(`[IPCService] connecting to proxy at ${proxyPath}`); - this.getSocket().on("error", (error: Error) => { - // Only makes sense as long as config.maxRetries stays set to 0. Otherwise this will be - // invoked multiple times each time a connection error happens - console.log("[IPCService] errored"); - console.log( - "\x1b[33m Please make sure the desktop app is running locally and 'Allow DuckDuckGo browser integration' setting is enabled \x1b[0m", - ); - this.awaitingConnection.forEach((deferred) => { - console.log(`rejecting: ${deferred}`); - deferred.reject(error); - }); - this.awaitingConnection.clear(); - }); + this.process = spawn(proxyPath, process.argv.slice(1), { + cwd: process.cwd(), + stdio: "pipe", + shell: false, + }); - this.getSocket().on("connect", () => { - console.log("[IPCService] connected"); - this.connectionState = IPCConnectionState.Connected; + this.process.stdout.on("data", (data: Buffer) => { + this.processIpcMessage(data); + }); - this.awaitingConnection.forEach((deferred) => { - deferred.resolve(null); - }); - this.awaitingConnection.clear(); - }); + this.process.stderr.on("data", (data: Buffer) => { + console.error(`proxy log: ${data}`); + }); - this.getSocket().on("disconnect", () => { - console.log("[IPCService] disconnected"); - this.connectionState = IPCConnectionState.Disconnected; + this.process.on("error", (error) => { + // Only makes sense as long as config.maxRetries stays set to 0. Otherwise this will be + // invoked multiple times each time a connection error happens + console.log("[IPCService] errored"); + console.log( + "\x1b[33m Please make sure the desktop app is running locally and 'Allow DuckDuckGo browser integration' setting is enabled \x1b[0m", + ); + this.awaitingConnection.forEach((deferred) => { + console.log(`rejecting: ${deferred}`); + deferred.reject(error); }); + this.awaitingConnection.clear(); + }); + + this.process.on("exit", () => { + console.log("[IPCService] disconnected"); + this.connectionState = IPCConnectionState.Disconnected; }); } disconnect() { console.log("[IPCService] disconnecting..."); if (this.connectionState !== IPCConnectionState.Disconnected) { - NodeIPC.disconnect(this.socketName); + this.process?.kill(); } } @@ -133,7 +130,7 @@ export default class IPCService { this.pendingMessages.set(message.messageId, deferred); - this.getSocket().emit("message", message); + this.sendIpcMessage(message); try { // Since we can not guarantee that a response message will ever be sent, we put a timeout @@ -151,8 +148,56 @@ export default class IPCService { } } - private getSocket() { - return NodeIPC.of[this.socketName]; + // As we're using the desktop_proxy to communicate with the native messaging directly, + // the messages need to follow Native Messaging Host protocol (uint32 size followed by message). + // https://developer.chrome.com/docs/extensions/develop/concepts/native-messaging#native-messaging-host-protocol + private sendIpcMessage(message: MessageCommon) { + const messageStr = JSON.stringify(message); + const buffer = Buffer.alloc(4 + messageStr.length); + buffer.writeUInt32LE(messageStr.length, 0); + buffer.write(messageStr, 4); + + this.process?.stdin.write(buffer); + } + + private processIpcMessage(data: Buffer) { + this.processOutputBuffer = Buffer.concat([this.processOutputBuffer, data]); + + // We might receive more than one IPC message per data event, so we need to process them all + // We continue as long as we have at least 4 + 1 bytes in the buffer, where the first 4 bytes + // represent the message length and the 5th byte is the message + while (this.processOutputBuffer.length > 4) { + // Read the message length and ensure we have the full message + const msgLength = this.processOutputBuffer.readUInt32LE(0); + if (msgLength + 4 < this.processOutputBuffer.length) { + return; + } + + // Parse the message from the buffer + const messageStr = this.processOutputBuffer.subarray(4, msgLength + 4).toString(); + const message = JSON.parse(messageStr); + + // Store the remaining buffer, which is part of the next message + this.processOutputBuffer = this.processOutputBuffer.subarray(msgLength + 4); + + // Process the connect/disconnect messages separately + if (message?.command === "connected") { + console.log("[IPCService] connected"); + this.connectionState = IPCConnectionState.Connected; + + this.awaitingConnection.forEach((deferred) => { + deferred.resolve(null); + }); + this.awaitingConnection.clear(); + continue; + } else if (message?.command === "disconnected") { + console.log("[IPCService] disconnected"); + this.connectionState = IPCConnectionState.Disconnected; + continue; + } + + this.processMessage(message); + } } private processMessage(message: any) { @@ -172,3 +217,41 @@ export default class IPCService { } } } + +function selectProxyPath(): string { + const proxyExtension = process.platform === "win32" ? ".exe" : ""; + + // If the PROXY_PATH environment variable is set, use that + if (process.env.PROXY_PATH) { + if (!fs.existsSync(process.env.PROXY_PATH)) { + throw new Error(`PROXY_PATH is set to ${process.env.PROXY_PATH} but the file does not exist`); + } + return process.env.PROXY_PATH; + } + + // Otherwise try the debug build if present + const debugProxyPath = path.join( + __dirname, + "..", + "..", + "..", + "..", + "..", + "..", + "desktop_native", + "target", + "debug", + `desktop_proxy${proxyExtension}`, + ); + if (fs.existsSync(debugProxyPath)) { + return debugProxyPath; + } + + // On MacOS, try the release build (sandboxed) + const macReleaseProxyPath = `/Applications/Bitwarden.app/Contents/MacOS/desktop_proxy${proxyExtension}`; + if (process.platform === "darwin" && fs.existsSync(macReleaseProxyPath)) { + return macReleaseProxyPath; + } + + throw new Error("Could not find the desktop_proxy executable"); +} diff --git a/apps/desktop/native-messaging-test-runner/tsconfig.json b/apps/desktop/native-messaging-test-runner/tsconfig.json index 72a28de3f7a..59c7040e509 100644 --- a/apps/desktop/native-messaging-test-runner/tsconfig.json +++ b/apps/desktop/native-messaging-test-runner/tsconfig.json @@ -5,12 +5,17 @@ "target": "es6", "module": "CommonJS", "moduleResolution": "node", + "emitDecoratorMetadata": true, + "experimentalDecorators": true, "sourceMap": false, "declaration": false, "paths": { "@src/*": ["src/*"], - "@bitwarden/node/*": ["../../../libs/node/src/*"], - "@bitwarden/common/*": ["../../../libs/common/src/*"] + "@bitwarden/admin-console/*": ["../../../libs/admin-console/src/*"], + "@bitwarden/auth/*": ["../../../libs/auth/src/*"], + "@bitwarden/common/*": ["../../../libs/common/src/*"], + "@bitwarden/key-management": ["../../../libs/key-management/src/"], + "@bitwarden/node/*": ["../../../libs/node/src/*"] }, "plugins": [ { diff --git a/package-lock.json b/package-lock.json index 622ee9bf9d4..01aace3228c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -113,7 +113,6 @@ "@types/node": "22.10.7", "@types/node-fetch": "2.6.4", "@types/node-forge": "1.3.11", - "@types/node-ipc": "9.2.3", "@types/papaparse": "5.3.15", "@types/proper-lockfile": "4.1.4", "@types/retry": "0.12.5", @@ -157,7 +156,6 @@ "json5": "2.2.3", "lint-staged": "15.4.1", "mini-css-extract-plugin": "2.9.2", - "node-ipc": "9.2.1", "postcss": "8.5.1", "postcss-loader": "8.1.1", "prettier": "3.4.2", @@ -10047,16 +10045,6 @@ "@types/node": "*" } }, - "node_modules/@types/node-ipc": { - "version": "9.2.3", - "resolved": "https://registry.npmjs.org/@types/node-ipc/-/node-ipc-9.2.3.tgz", - "integrity": "sha512-/MvSiF71fYf3+zwqkh/zkVkZj1hl1Uobre9EMFy08mqfJNAmpR0vmPgOUdEIDVgifxHj6G1vYMPLSBLLxoDACQ==", - "dev": true, - "license": "MIT", - "dependencies": { - "@types/node": "*" - } - }, "node_modules/@types/node/node_modules/undici-types": { "version": "6.20.0", "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-6.20.0.tgz", @@ -15862,16 +15850,6 @@ "dev": true, "license": "MIT" }, - "node_modules/easy-stack": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/easy-stack/-/easy-stack-1.0.1.tgz", - "integrity": "sha512-wK2sCs4feiiJeFXn3zvY0p41mdU5VUgbgs1rNsc/y5ngFUijdWd+iIN8eoyuZHKB8xN6BL4PdWmzqFmxNg6V2w==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=6.0.0" - } - }, "node_modules/ee-first": { "version": "1.1.1", "resolved": "https://registry.npmjs.org/ee-first/-/ee-first-1.1.1.tgz", @@ -17182,16 +17160,6 @@ "node": ">= 0.6" } }, - "node_modules/event-pubsub": { - "version": "4.3.0", - "resolved": "https://registry.npmjs.org/event-pubsub/-/event-pubsub-4.3.0.tgz", - "integrity": "sha512-z7IyloorXvKbFx9Bpie2+vMJKKx1fH1EN5yiTfp8CiLOTptSYy1g8H4yDpGlEdshL1PBiFtBHepF2cNsqeEeFQ==", - "dev": true, - "license": "Unlicense", - "engines": { - "node": ">=4.0.0" - } - }, "node_modules/event-stream": { "version": "4.0.1", "resolved": "https://registry.npmjs.org/event-stream/-/event-stream-4.0.1.tgz", @@ -21964,29 +21932,6 @@ "integrity": "sha512-m4avr8yL8kmFN8psrbFFFmB/If14iN5o9nw/NgnnM+kybDJpRsAynV2BsfpTYrTRysYUdADVD7CkUUizgkpLfg==", "license": "MIT" }, - "node_modules/js-message": { - "version": "1.0.7", - "resolved": "https://registry.npmjs.org/js-message/-/js-message-1.0.7.tgz", - "integrity": "sha512-efJLHhLjIyKRewNS9EGZ4UpI8NguuL6fKkhRxVuMmrGV2xN/0APGdQYwLFky5w9naebSZ0OwAGp0G6/2Cg90rA==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=0.6.0" - } - }, - "node_modules/js-queue": { - "version": "2.0.2", - "resolved": "https://registry.npmjs.org/js-queue/-/js-queue-2.0.2.tgz", - "integrity": "sha512-pbKLsbCfi7kriM3s1J4DDCo7jQkI58zPLHi0heXPzPlj0hjUsm+FesPUbE0DSbIVIK503A36aUBoCN7eMFedkA==", - "dev": true, - "license": "MIT", - "dependencies": { - "easy-stack": "^1.0.1" - }, - "engines": { - "node": ">=1.0.0" - } - }, "node_modules/js-tokens": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/js-tokens/-/js-tokens-4.0.0.tgz", @@ -25710,21 +25655,6 @@ "license": "MIT", "peer": true }, - "node_modules/node-ipc": { - "version": "9.2.1", - "resolved": "https://registry.npmjs.org/node-ipc/-/node-ipc-9.2.1.tgz", - "integrity": "sha512-mJzaM6O3xHf9VT8BULvJSbdVbmHUKRNOH7zDDkCrA1/T+CVjq2WVIDfLt0azZRXpgArJtl3rtmEozrbXPZ9GaQ==", - "dev": true, - "license": "MIT", - "dependencies": { - "event-pubsub": "4.3.0", - "js-message": "1.0.7", - "js-queue": "2.0.2" - }, - "engines": { - "node": ">=8.0.0" - } - }, "node_modules/node-releases": { "version": "2.0.18", "resolved": "https://registry.npmjs.org/node-releases/-/node-releases-2.0.18.tgz", diff --git a/package.json b/package.json index 44733e4ab07..32b5b2b6af3 100644 --- a/package.json +++ b/package.json @@ -74,7 +74,6 @@ "@types/node": "22.10.7", "@types/node-fetch": "2.6.4", "@types/node-forge": "1.3.11", - "@types/node-ipc": "9.2.3", "@types/papaparse": "5.3.15", "@types/proper-lockfile": "4.1.4", "@types/retry": "0.12.5", @@ -118,7 +117,6 @@ "json5": "2.2.3", "lint-staged": "15.4.1", "mini-css-extract-plugin": "2.9.2", - "node-ipc": "9.2.1", "postcss": "8.5.1", "postcss-loader": "8.1.1", "prettier": "3.4.2", From d11321e28e72f39256d73488a610e01d9528327f Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Tue, 25 Feb 2025 14:47:08 +0100 Subject: [PATCH 015/119] Fix ssh agent on flatpak and mac app store (#13324) --- .../peercred_unix_listener_stream.rs | 17 ++++------------ .../core/src/ssh_agent/peerinfo/models.rs | 4 ++++ .../desktop_native/core/src/ssh_agent/unix.rs | 20 ++++++++++++++----- 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/apps/desktop/desktop_native/core/src/ssh_agent/peercred_unix_listener_stream.rs b/apps/desktop/desktop_native/core/src/ssh_agent/peercred_unix_listener_stream.rs index f0114fc08da..da9d8a54318 100644 --- a/apps/desktop/desktop_native/core/src/ssh_agent/peercred_unix_listener_stream.rs +++ b/apps/desktop/desktop_native/core/src/ssh_agent/peercred_unix_listener_stream.rs @@ -31,26 +31,17 @@ impl Stream for PeercredUnixListenerStream { Ok(peer) => match peer.pid() { Some(pid) => pid, None => { - return Poll::Ready(Some(Err(io::Error::new( - io::ErrorKind::Other, - "Failed to get peer PID", - )))); + return Poll::Ready(Some(Ok((stream, PeerInfo::unknown())))); } }, - Err(err) => { - return Poll::Ready(Some(Err(io::Error::new( - io::ErrorKind::Other, - format!("Failed to get peer credentials: {}", err), - )))); + Err(_) => { + return Poll::Ready(Some(Ok((stream, PeerInfo::unknown())))); } }; let peer_info = peerinfo::gather::get_peer_info(pid as u32); match peer_info { Ok(info) => Poll::Ready(Some(Ok((stream, info)))), - Err(err) => Poll::Ready(Some(Err(io::Error::new( - io::ErrorKind::Other, - format!("Failed to get peer info: {}", err), - )))), + Err(_) => Poll::Ready(Some(Ok((stream, PeerInfo::unknown())))), } } Poll::Ready(Err(err)) => Poll::Ready(Some(Err(err))), diff --git a/apps/desktop/desktop_native/core/src/ssh_agent/peerinfo/models.rs b/apps/desktop/desktop_native/core/src/ssh_agent/peerinfo/models.rs index 823d912883e..9c2ee363e8f 100644 --- a/apps/desktop/desktop_native/core/src/ssh_agent/peerinfo/models.rs +++ b/apps/desktop/desktop_native/core/src/ssh_agent/peerinfo/models.rs @@ -29,4 +29,8 @@ impl PeerInfo { pub fn process_name(&self) -> &str { &self.process_name } + + pub fn unknown() -> Self { + Self::new(0, 0, "Unknown application".to_string()) + } } diff --git a/apps/desktop/desktop_native/core/src/ssh_agent/unix.rs b/apps/desktop/desktop_native/core/src/ssh_agent/unix.rs index ae03421a425..40bc36b1b9e 100644 --- a/apps/desktop/desktop_native/core/src/ssh_agent/unix.rs +++ b/apps/desktop/desktop_native/core/src/ssh_agent/unix.rs @@ -47,11 +47,21 @@ impl BitwardenDesktopAgent { return; } }; - ssh_agent_directory - .join(".bitwarden-ssh-agent.sock") - .to_str() - .expect("Path should be valid") - .to_owned() + + let is_flatpak = std::env::var("container") == Ok("flatpak".to_string()); + if !is_flatpak { + ssh_agent_directory + .join(".bitwarden-ssh-agent.sock") + .to_str() + .expect("Path should be valid") + .to_owned() + } else { + ssh_agent_directory + .join(".var/app/com.bitwarden.desktop/data/.bitwarden-ssh-agent.sock") + .to_str() + .expect("Path should be valid") + .to_owned() + } } }; From 6d1914f43db8af28f97d75ad40a0af6e15c2b530 Mon Sep 17 00:00:00 2001 From: Vicki League Date: Tue, 25 Feb 2025 09:56:01 -0500 Subject: [PATCH 016/119] [CL-485] Add small delay for async action loading state (#12835) --- .../src/async-actions/bit-action.directive.ts | 29 +++-- .../async-actions/form-button.directive.ts | 6 +- .../src/async-actions/standalone.mdx | 29 ++++- .../src/async-actions/standalone.stories.ts | 35 +++++- .../src/button/button.component.html | 4 +- .../components/src/button/button.component.ts | 72 ++++++++--- .../icon-button/icon-button.component.html | 4 +- .../src/icon-button/icon-button.component.ts | 116 +++++++++++++----- .../src/shared/button-like.abstraction.ts | 6 +- .../components/send-form.component.ts | 4 +- .../cipher-attachments.component.spec.ts | 24 ++-- .../cipher-attachments.component.ts | 8 +- .../components/cipher-form.component.ts | 4 +- .../item-details-section.component.spec.ts | 3 + .../add-edit-folder-dialog.component.ts | 2 +- .../assign-collections.component.ts | 4 +- 16 files changed, 253 insertions(+), 97 deletions(-) diff --git a/libs/components/src/async-actions/bit-action.directive.ts b/libs/components/src/async-actions/bit-action.directive.ts index 3e793ae2ecd..ac50082852a 100644 --- a/libs/components/src/async-actions/bit-action.directive.ts +++ b/libs/components/src/async-actions/bit-action.directive.ts @@ -21,30 +21,35 @@ export class BitActionDirective implements OnDestroy { private destroy$ = new Subject(); private _loading$ = new BehaviorSubject(false); - disabled = false; - - @Input("bitAction") handler: FunctionReturningAwaitable; - + /** + * Observable of loading behavior subject + * + * Used in `form-button.directive.ts` + */ readonly loading$ = this._loading$.asObservable(); - constructor( - private buttonComponent: ButtonLikeAbstraction, - @Optional() private validationService?: ValidationService, - @Optional() private logService?: LogService, - ) {} - get loading() { return this._loading$.value; } set loading(value: boolean) { this._loading$.next(value); - this.buttonComponent.loading = value; + this.buttonComponent.loading.set(value); } + disabled = false; + + @Input("bitAction") handler: FunctionReturningAwaitable; + + constructor( + private buttonComponent: ButtonLikeAbstraction, + @Optional() private validationService?: ValidationService, + @Optional() private logService?: LogService, + ) {} + @HostListener("click") protected async onClick() { - if (!this.handler || this.loading || this.disabled || this.buttonComponent.disabled) { + if (!this.handler || this.loading || this.disabled || this.buttonComponent.disabled()) { return; } diff --git a/libs/components/src/async-actions/form-button.directive.ts b/libs/components/src/async-actions/form-button.directive.ts index 7c92865b984..1c2855f32e7 100644 --- a/libs/components/src/async-actions/form-button.directive.ts +++ b/libs/components/src/async-actions/form-button.directive.ts @@ -41,15 +41,15 @@ export class BitFormButtonDirective implements OnDestroy { if (submitDirective && buttonComponent) { submitDirective.loading$.pipe(takeUntil(this.destroy$)).subscribe((loading) => { if (this.type === "submit") { - buttonComponent.loading = loading; + buttonComponent.loading.set(loading); } else { - buttonComponent.disabled = this.disabled || loading; + buttonComponent.disabled.set(this.disabled || loading); } }); submitDirective.disabled$.pipe(takeUntil(this.destroy$)).subscribe((disabled) => { if (this.disabled !== false) { - buttonComponent.disabled = this.disabled || disabled; + buttonComponent.disabled.set(this.disabled || disabled); } }); } diff --git a/libs/components/src/async-actions/standalone.mdx b/libs/components/src/async-actions/standalone.mdx index efde494f2dd..f484ea01c58 100644 --- a/libs/components/src/async-actions/standalone.mdx +++ b/libs/components/src/async-actions/standalone.mdx @@ -1,6 +1,7 @@ -import { Meta } from "@storybook/addon-docs"; +import { Meta, Story } from "@storybook/addon-docs"; +import * as stories from "./standalone.stories.ts"; - + # Standalone Async Actions @@ -8,9 +9,13 @@ These directives should be used when building a standalone button that triggers in the background, eg. Refresh buttons. For non-submit buttons that are associated with forms see [Async Actions In Forms](?path=/story/component-library-async-actions-in-forms-documentation--page). +If the long running background task resolves quickly (e.g. less than 75 ms), the loading spinner +will not display on the button. This prevents an undesirable "flicker" of the loading spinner when +it is not necessary for the user to see it. + ## Usage -Adding async actions to standalone buttons requires the following 2 steps +Adding async actions to standalone buttons requires the following 2 steps: ### 1. Add a handler to your `Component` @@ -60,3 +65,21 @@ from how click handlers are usually defined with the output syntax `(click)="han `; ``` + +## Stories + +### Promise resolves -- loading spinner is displayed + + + +### Promise resolves -- quickly without loading spinner + + + +### Promise rejects + + + +### Observable + + diff --git a/libs/components/src/async-actions/standalone.stories.ts b/libs/components/src/async-actions/standalone.stories.ts index f658dfb0f01..52b85b88561 100644 --- a/libs/components/src/async-actions/standalone.stories.ts +++ b/libs/components/src/async-actions/standalone.stories.ts @@ -11,9 +11,9 @@ import { IconButtonModule } from "../icon-button"; import { BitActionDirective } from "./bit-action.directive"; -const template = ` +const template = /*html*/ ` `; @@ -22,9 +22,30 @@ const template = ` selector: "app-promise-example", }) class PromiseExampleComponent { + statusEmoji = "🟡"; action = async () => { await new Promise((resolve, reject) => { - setTimeout(resolve, 2000); + setTimeout(() => { + resolve(); + this.statusEmoji = "🟢"; + }, 5000); + }); + }; +} + +@Component({ + template, + selector: "app-action-resolves-quickly", +}) +class ActionResolvesQuicklyComponent { + statusEmoji = "🟡"; + + action = async () => { + await new Promise((resolve, reject) => { + setTimeout(() => { + resolve(); + this.statusEmoji = "🟢"; + }, 50); }); }; } @@ -59,6 +80,7 @@ export default { PromiseExampleComponent, ObservableExampleComponent, RejectedPromiseExampleComponent, + ActionResolvesQuicklyComponent, ], imports: [ButtonModule, IconButtonModule, BitActionDirective], providers: [ @@ -100,3 +122,10 @@ export const RejectedPromise: ObservableStory = { template: ``, }), }; + +export const ActionResolvesQuickly: PromiseStory = { + render: (args) => ({ + props: args, + template: ``, + }), +}; diff --git a/libs/components/src/button/button.component.html b/libs/components/src/button/button.component.html index ee4d150dfcc..a07ab9fb99b 100644 --- a/libs/components/src/button/button.component.html +++ b/libs/components/src/button/button.component.html @@ -1,10 +1,10 @@ - + diff --git a/libs/components/src/button/button.component.ts b/libs/components/src/button/button.component.ts index 96311f91529..0b4ce3073c3 100644 --- a/libs/components/src/button/button.component.ts +++ b/libs/components/src/button/button.component.ts @@ -2,7 +2,9 @@ // @ts-strict-ignore import { coerceBooleanProperty } from "@angular/cdk/coercion"; import { NgClass } from "@angular/common"; -import { Input, HostBinding, Component } from "@angular/core"; +import { Input, HostBinding, Component, model, computed } from "@angular/core"; +import { toObservable, toSignal } from "@angular/core/rxjs-interop"; +import { debounce, interval } from "rxjs"; import { ButtonLikeAbstraction, ButtonType } from "../shared/button-like.abstraction"; @@ -49,6 +51,9 @@ const buttonStyles: Record = { providers: [{ provide: ButtonLikeAbstraction, useExisting: ButtonComponent }], standalone: true, imports: [NgClass], + host: { + "[attr.disabled]": "disabledAttr()", + }, }) export class ButtonComponent implements ButtonLikeAbstraction { @HostBinding("class") get classList() { @@ -64,24 +69,41 @@ export class ButtonComponent implements ButtonLikeAbstraction { "tw-no-underline", "hover:tw-no-underline", "focus:tw-outline-none", - "disabled:tw-bg-secondary-300", - "disabled:hover:tw-bg-secondary-300", - "disabled:tw-border-secondary-300", - "disabled:hover:tw-border-secondary-300", - "disabled:!tw-text-muted", - "disabled:hover:!tw-text-muted", - "disabled:tw-cursor-not-allowed", - "disabled:hover:tw-no-underline", ] .concat(this.block ? ["tw-w-full", "tw-block"] : ["tw-inline-block"]) - .concat(buttonStyles[this.buttonType ?? "secondary"]); + .concat(buttonStyles[this.buttonType ?? "secondary"]) + .concat( + this.showDisabledStyles() || this.disabled() + ? [ + "disabled:tw-bg-secondary-300", + "disabled:hover:tw-bg-secondary-300", + "disabled:tw-border-secondary-300", + "disabled:hover:tw-border-secondary-300", + "disabled:!tw-text-muted", + "disabled:hover:!tw-text-muted", + "disabled:tw-cursor-not-allowed", + "disabled:hover:tw-no-underline", + ] + : [], + ); } - @HostBinding("attr.disabled") - get disabledAttr() { - const disabled = this.disabled != null && this.disabled !== false; - return disabled || this.loading ? true : null; - } + protected disabledAttr = computed(() => { + const disabled = this.disabled() != null && this.disabled() !== false; + return disabled || this.loading() ? true : null; + }); + + /** + * Determine whether it is appropriate to display the disabled styles. We only want to show + * the disabled styles if the button is truly disabled, or if the loading styles are also + * visible. + * + * We can't use `disabledAttr` for this, because it returns `true` when `loading` is `true`. + * We only want to show disabled styles during loading if `showLoadingStyles` is `true`. + */ + protected showDisabledStyles = computed(() => { + return this.showLoadingStyle() || (this.disabledAttr() && this.loading() === false); + }); @Input() buttonType: ButtonType; @@ -96,7 +118,23 @@ export class ButtonComponent implements ButtonLikeAbstraction { this._block = coerceBooleanProperty(value); } - @Input() loading = false; + loading = model(false); - @Input() disabled = false; + /** + * Determine whether it is appropriate to display a loading spinner. We only want to show + * a spinner if it's been more than 75 ms since the `loading` state began. This prevents + * a spinner "flash" for actions that are synchronous/nearly synchronous. + * + * We can't use `loading` for this, because we still need to disable the button during + * the full `loading` state. I.e. we only want the spinner to be debounced, not the + * loading state. + * + * This pattern of converting a signal to an observable and back to a signal is not + * recommended. TODO -- find better way to use debounce with signals (CL-596) + */ + protected showLoadingStyle = toSignal( + toObservable(this.loading).pipe(debounce((isLoading) => interval(isLoading ? 75 : 0))), + ); + + disabled = model(false); } diff --git a/libs/components/src/icon-button/icon-button.component.html b/libs/components/src/icon-button/icon-button.component.html index 6eeaaaffaf0..0145f0b0ba5 100644 --- a/libs/components/src/icon-button/icon-button.component.html +++ b/libs/components/src/icon-button/icon-button.component.html @@ -1,10 +1,10 @@ - +