From 88bc7625218028cdea3d73663a00f8d24133ba6e Mon Sep 17 00:00:00 2001 From: Daniel Riera Date: Tue, 27 May 2025 14:51:40 -0400 Subject: [PATCH 01/14] PM-16645 (#14649) --- .../src/autofill/services/autofill.service.ts | 248 +----------------- libs/common/src/enums/feature-flag.enum.ts | 2 - 2 files changed, 1 insertion(+), 249 deletions(-) diff --git a/apps/browser/src/autofill/services/autofill.service.ts b/apps/browser/src/autofill/services/autofill.service.ts index 525010bacc1..fdd881c2760 100644 --- a/apps/browser/src/autofill/services/autofill.service.ts +++ b/apps/browser/src/autofill/services/autofill.service.ts @@ -1579,252 +1579,6 @@ export default class AutofillService implements AutofillServiceInterface { return [expectedDateFormat, dateFormatPatterns]; } - /** - * Generates the autofill script for the specified page details and identify cipher item. - * @param {AutofillScript} fillScript - * @param {AutofillPageDetails} pageDetails - * @param {{[p: string]: AutofillField}} filledFields - * @param {GenerateFillScriptOptions} options - * @returns {AutofillScript} - * @private - */ - private async generateIdentityFillScript( - fillScript: AutofillScript, - pageDetails: AutofillPageDetails, - filledFields: { [id: string]: AutofillField }, - options: GenerateFillScriptOptions, - ): Promise { - if (await this.configService.getFeatureFlag(FeatureFlag.GenerateIdentityFillScriptRefactor)) { - return this._generateIdentityFillScript(fillScript, pageDetails, filledFields, options); - } - - if (!options.cipher.identity) { - return null; - } - - const fillFields: { [id: string]: AutofillField } = {}; - - pageDetails.fields.forEach((f) => { - if ( - AutofillService.isExcludedFieldType(f, AutoFillConstants.ExcludedAutofillTypes) || - ["current-password", "new-password"].includes(f.autoCompleteType) - ) { - return; - } - - for (let i = 0; i < IdentityAutoFillConstants.IdentityAttributes.length; i++) { - const attr = IdentityAutoFillConstants.IdentityAttributes[i]; - // eslint-disable-next-line - if (!f.hasOwnProperty(attr) || !f[attr] || !f.viewable) { - continue; - } - - // ref https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#autofill - // ref https://developers.google.com/web/fundamentals/design-and-ux/input/forms/ - if ( - !fillFields.name && - AutofillService.isFieldMatch( - f[attr], - IdentityAutoFillConstants.FullNameFieldNames, - IdentityAutoFillConstants.FullNameFieldNameValues, - ) - ) { - fillFields.name = f; - break; - } else if ( - !fillFields.firstName && - AutofillService.isFieldMatch(f[attr], IdentityAutoFillConstants.FirstnameFieldNames) - ) { - fillFields.firstName = f; - break; - } else if ( - !fillFields.middleName && - AutofillService.isFieldMatch(f[attr], IdentityAutoFillConstants.MiddlenameFieldNames) - ) { - fillFields.middleName = f; - break; - } else if ( - !fillFields.lastName && - AutofillService.isFieldMatch(f[attr], IdentityAutoFillConstants.LastnameFieldNames) - ) { - fillFields.lastName = f; - break; - } else if ( - !fillFields.title && - AutofillService.isFieldMatch(f[attr], IdentityAutoFillConstants.TitleFieldNames) - ) { - fillFields.title = f; - break; - } else if ( - !fillFields.email && - AutofillService.isFieldMatch(f[attr], IdentityAutoFillConstants.EmailFieldNames) - ) { - fillFields.email = f; - break; - } else if ( - !fillFields.address1 && - AutofillService.isFieldMatch(f[attr], IdentityAutoFillConstants.Address1FieldNames) - ) { - fillFields.address1 = f; - break; - } else if ( - !fillFields.address2 && - AutofillService.isFieldMatch(f[attr], IdentityAutoFillConstants.Address2FieldNames) - ) { - fillFields.address2 = f; - break; - } else if ( - !fillFields.address3 && - AutofillService.isFieldMatch(f[attr], IdentityAutoFillConstants.Address3FieldNames) - ) { - fillFields.address3 = f; - break; - } else if ( - !fillFields.address && - AutofillService.isFieldMatch( - f[attr], - IdentityAutoFillConstants.AddressFieldNames, - IdentityAutoFillConstants.AddressFieldNameValues, - ) - ) { - fillFields.address = f; - break; - } else if ( - !fillFields.postalCode && - AutofillService.isFieldMatch(f[attr], IdentityAutoFillConstants.PostalCodeFieldNames) - ) { - fillFields.postalCode = f; - break; - } else if ( - !fillFields.city && - AutofillService.isFieldMatch(f[attr], IdentityAutoFillConstants.CityFieldNames) - ) { - fillFields.city = f; - break; - } else if ( - !fillFields.state && - AutofillService.isFieldMatch(f[attr], IdentityAutoFillConstants.StateFieldNames) - ) { - fillFields.state = f; - break; - } else if ( - !fillFields.country && - AutofillService.isFieldMatch(f[attr], IdentityAutoFillConstants.CountryFieldNames) - ) { - fillFields.country = f; - break; - } else if ( - !fillFields.phone && - AutofillService.isFieldMatch(f[attr], IdentityAutoFillConstants.PhoneFieldNames) - ) { - fillFields.phone = f; - break; - } else if ( - !fillFields.username && - AutofillService.isFieldMatch(f[attr], IdentityAutoFillConstants.UserNameFieldNames) - ) { - fillFields.username = f; - break; - } else if ( - !fillFields.company && - AutofillService.isFieldMatch(f[attr], IdentityAutoFillConstants.CompanyFieldNames) - ) { - fillFields.company = f; - break; - } - } - }); - - const identity = options.cipher.identity; - this.makeScriptAction(fillScript, identity, fillFields, filledFields, "title"); - this.makeScriptAction(fillScript, identity, fillFields, filledFields, "firstName"); - this.makeScriptAction(fillScript, identity, fillFields, filledFields, "middleName"); - this.makeScriptAction(fillScript, identity, fillFields, filledFields, "lastName"); - this.makeScriptAction(fillScript, identity, fillFields, filledFields, "address1"); - this.makeScriptAction(fillScript, identity, fillFields, filledFields, "address2"); - this.makeScriptAction(fillScript, identity, fillFields, filledFields, "address3"); - this.makeScriptAction(fillScript, identity, fillFields, filledFields, "city"); - this.makeScriptAction(fillScript, identity, fillFields, filledFields, "postalCode"); - this.makeScriptAction(fillScript, identity, fillFields, filledFields, "company"); - this.makeScriptAction(fillScript, identity, fillFields, filledFields, "email"); - this.makeScriptAction(fillScript, identity, fillFields, filledFields, "phone"); - this.makeScriptAction(fillScript, identity, fillFields, filledFields, "username"); - - let filledState = false; - if (fillFields.state && identity.state && identity.state.length > 2) { - const stateLower = identity.state.toLowerCase(); - const isoState = - IdentityAutoFillConstants.IsoStates[stateLower] || - IdentityAutoFillConstants.IsoProvinces[stateLower]; - if (isoState) { - filledState = true; - this.makeScriptActionWithValue(fillScript, isoState, fillFields.state, filledFields); - } - } - - if (!filledState) { - this.makeScriptAction(fillScript, identity, fillFields, filledFields, "state"); - } - - let filledCountry = false; - if (fillFields.country && identity.country && identity.country.length > 2) { - const countryLower = identity.country.toLowerCase(); - const isoCountry = IdentityAutoFillConstants.IsoCountries[countryLower]; - if (isoCountry) { - filledCountry = true; - this.makeScriptActionWithValue(fillScript, isoCountry, fillFields.country, filledFields); - } - } - - if (!filledCountry) { - this.makeScriptAction(fillScript, identity, fillFields, filledFields, "country"); - } - - if (fillFields.name && (identity.firstName || identity.lastName)) { - let fullName = ""; - if (AutofillService.hasValue(identity.firstName)) { - fullName = identity.firstName; - } - if (AutofillService.hasValue(identity.middleName)) { - if (fullName !== "") { - fullName += " "; - } - fullName += identity.middleName; - } - if (AutofillService.hasValue(identity.lastName)) { - if (fullName !== "") { - fullName += " "; - } - fullName += identity.lastName; - } - - this.makeScriptActionWithValue(fillScript, fullName, fillFields.name, filledFields); - } - - if (fillFields.address && AutofillService.hasValue(identity.address1)) { - let address = ""; - if (AutofillService.hasValue(identity.address1)) { - address = identity.address1; - } - if (AutofillService.hasValue(identity.address2)) { - if (address !== "") { - address += ", "; - } - address += identity.address2; - } - if (AutofillService.hasValue(identity.address3)) { - if (address !== "") { - address += ", "; - } - address += identity.address3; - } - - this.makeScriptActionWithValue(fillScript, address, fillFields.address, filledFields); - } - - return fillScript; - } - /** * Generates the autofill script for the specified page details and identity cipher item. * @@ -1833,7 +1587,7 @@ export default class AutofillService implements AutofillServiceInterface { * @param filledFields - The fields that have already been filled, passed between method references * @param options - Contains data used to fill cipher items */ - private _generateIdentityFillScript( + private generateIdentityFillScript( fillScript: AutofillScript, pageDetails: AutofillPageDetails, filledFields: { [id: string]: AutofillField }, diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index a5c5c615e70..8cd44f9d627 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -22,7 +22,6 @@ export enum FeatureFlag { BlockBrowserInjectionsByDomain = "block-browser-injections-by-domain", DelayFido2PageScriptInitWithinMv2 = "delay-fido2-page-script-init-within-mv2", EnableNewCardCombinedExpiryAutofill = "enable-new-card-combined-expiry-autofill", - GenerateIdentityFillScriptRefactor = "generate-identity-fill-script-refactor", IdpAutoSubmitLogin = "idp-auto-submit-login", NotificationRefresh = "notification-refresh", UseTreeWalkerApiForPageDetailsCollection = "use-tree-walker-api-for-page-details-collection", @@ -87,7 +86,6 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.BlockBrowserInjectionsByDomain]: FALSE, [FeatureFlag.DelayFido2PageScriptInitWithinMv2]: FALSE, [FeatureFlag.EnableNewCardCombinedExpiryAutofill]: FALSE, - [FeatureFlag.GenerateIdentityFillScriptRefactor]: FALSE, [FeatureFlag.IdpAutoSubmitLogin]: FALSE, [FeatureFlag.NotificationRefresh]: FALSE, [FeatureFlag.UseTreeWalkerApiForPageDetailsCollection]: FALSE, From cb770f5cd3381b38d34d502fb26f07659cf02d49 Mon Sep 17 00:00:00 2001 From: Patrick-Pimentel-Bitwarden Date: Tue, 27 May 2025 16:01:07 -0400 Subject: [PATCH 02/14] refactor(browser-platform-utils): Remove Deprecation and Fix Code (#14709) * refactor(browser-platform-utils): Remove Deprecation and Fix Code - Changed usages of firefox to private and moved the usages to the preferred public method and removed the deprecations. * fix(browser-platform-utils): Remove Deprecation and Fix Code - Tiny changes. * test(browser-platform-utils): Remove Deprecation and Fix Code - Fixed up test --- .../src/platform/listeners/update-badge.ts | 10 +++---- .../browser-platform-utils.service.spec.ts | 3 +- .../browser-platform-utils.service.ts | 30 ++++--------------- 3 files changed, 12 insertions(+), 31 deletions(-) diff --git a/apps/browser/src/platform/listeners/update-badge.ts b/apps/browser/src/platform/listeners/update-badge.ts index cd74b9ebf7d..c168ae44f3c 100644 --- a/apps/browser/src/platform/listeners/update-badge.ts +++ b/apps/browser/src/platform/listeners/update-badge.ts @@ -7,13 +7,13 @@ import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { getOptionalUserId } from "@bitwarden/common/auth/services/account.service"; import { BadgeSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/badge-settings.service"; +import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import MainBackground from "../../background/main.background"; import IconDetails from "../../vault/background/models/icon-details"; import { BrowserApi } from "../browser/browser-api"; -import { BrowserPlatformUtilsService } from "../services/platform-utils/browser-platform-utils.service"; export type BadgeOptions = { tab?: chrome.tabs.Tab; @@ -28,6 +28,7 @@ export class UpdateBadge { private badgeAction: typeof chrome.action | typeof chrome.browserAction; private sidebarAction: OperaSidebarAction | FirefoxSidebarAction; private win: Window & typeof globalThis; + private platformUtilsService: PlatformUtilsService; constructor(win: Window & typeof globalThis, services: MainBackground) { this.badgeAction = BrowserApi.getBrowserAction(); @@ -38,6 +39,7 @@ export class UpdateBadge { this.authService = services.authService; this.cipherService = services.cipherService; this.accountService = services.accountService; + this.platformUtilsService = services.platformUtilsService; } async run(opts?: { tabId?: number; windowId?: number }): Promise { @@ -129,7 +131,7 @@ export class UpdateBadge { 38: "/images/icon38" + iconSuffix + ".png", }, }; - if (windowId && BrowserPlatformUtilsService.isFirefox()) { + if (windowId && this.platformUtilsService.isFirefox()) { options.windowId = windowId; } @@ -204,9 +206,7 @@ export class UpdateBadge { } private get useSyncApiCalls() { - return ( - BrowserPlatformUtilsService.isFirefox() || BrowserPlatformUtilsService.isSafari(this.win) - ); + return this.platformUtilsService.isFirefox() || this.platformUtilsService.isSafari(); } private isOperaSidebar( diff --git a/apps/browser/src/platform/services/platform-utils/browser-platform-utils.service.spec.ts b/apps/browser/src/platform/services/platform-utils/browser-platform-utils.service.spec.ts index 38166d10a08..f75e9cc29a5 100644 --- a/apps/browser/src/platform/services/platform-utils/browser-platform-utils.service.spec.ts +++ b/apps/browser/src/platform/services/platform-utils/browser-platform-utils.service.spec.ts @@ -126,12 +126,11 @@ describe("Browser Utils Service", () => { configurable: true, value: "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0", }); - jest.spyOn(BrowserPlatformUtilsService, "isFirefox"); browserPlatformUtilsService.getDevice(); expect(browserPlatformUtilsService.getDevice()).toBe(DeviceType.FirefoxExtension); - expect(BrowserPlatformUtilsService.isFirefox).toHaveBeenCalledTimes(1); + expect(browserPlatformUtilsService.isFirefox()).toBe(true); }); }); diff --git a/apps/browser/src/platform/services/platform-utils/browser-platform-utils.service.ts b/apps/browser/src/platform/services/platform-utils/browser-platform-utils.service.ts index 22708d8e425..4ae412fbda6 100644 --- a/apps/browser/src/platform/services/platform-utils/browser-platform-utils.service.ts +++ b/apps/browser/src/platform/services/platform-utils/browser-platform-utils.service.ts @@ -60,10 +60,7 @@ export abstract class BrowserPlatformUtilsService implements PlatformUtilsServic return ClientType.Browser; } - /** - * @deprecated Do not call this directly, use getDevice() instead - */ - static isFirefox(): boolean { + private static isFirefox(): boolean { return ( navigator.userAgent.indexOf(" Firefox/") !== -1 || navigator.userAgent.indexOf(" Gecko/") !== -1 @@ -74,9 +71,6 @@ export abstract class BrowserPlatformUtilsService implements PlatformUtilsServic return this.getDevice() === DeviceType.FirefoxExtension; } - /** - * @deprecated Do not call this directly, use getDevice() instead - */ private static isChrome(globalContext: Window | ServiceWorkerGlobalScope): boolean { return globalContext.chrome && navigator.userAgent.indexOf(" Chrome/") !== -1; } @@ -85,9 +79,6 @@ export abstract class BrowserPlatformUtilsService implements PlatformUtilsServic return this.getDevice() === DeviceType.ChromeExtension; } - /** - * @deprecated Do not call this directly, use getDevice() instead - */ private static isEdge(): boolean { return navigator.userAgent.indexOf(" Edg/") !== -1; } @@ -96,9 +87,6 @@ export abstract class BrowserPlatformUtilsService implements PlatformUtilsServic return this.getDevice() === DeviceType.EdgeExtension; } - /** - * @deprecated Do not call this directly, use getDevice() instead - */ private static isOpera(globalContext: Window | ServiceWorkerGlobalScope): boolean { return ( !!globalContext.opr?.addons || @@ -111,9 +99,6 @@ export abstract class BrowserPlatformUtilsService implements PlatformUtilsServic return this.getDevice() === DeviceType.OperaExtension; } - /** - * @deprecated Do not call this directly, use getDevice() instead - */ private static isVivaldi(): boolean { return navigator.userAgent.indexOf(" Vivaldi/") !== -1; } @@ -122,10 +107,7 @@ export abstract class BrowserPlatformUtilsService implements PlatformUtilsServic return this.getDevice() === DeviceType.VivaldiExtension; } - /** - * @deprecated Do not call this directly, use getDevice() instead - */ - static isSafari(globalContext: Window | ServiceWorkerGlobalScope): boolean { + private static isSafari(globalContext: Window | ServiceWorkerGlobalScope): boolean { // Opera masquerades as Safari, so make sure we're not there first return ( !BrowserPlatformUtilsService.isOpera(globalContext) && @@ -137,6 +119,10 @@ export abstract class BrowserPlatformUtilsService implements PlatformUtilsServic return navigator.userAgent.match("Version/([0-9.]*)")?.[1]; } + isSafari(): boolean { + return this.getDevice() === DeviceType.SafariExtension; + } + /** * Safari previous to version 16.1 had a bug which caused artifacts on hover in large extension popups. * https://bugs.webkit.org/show_bug.cgi?id=218704 @@ -151,10 +137,6 @@ export abstract class BrowserPlatformUtilsService implements PlatformUtilsServic return parts?.[0] < 16 || (parts?.[0] === 16 && parts?.[1] === 0); } - isSafari(): boolean { - return this.getDevice() === DeviceType.SafariExtension; - } - isIE(): boolean { return false; } From 50143a4b88dc170de92dd603b507545c334f2227 Mon Sep 17 00:00:00 2001 From: SmithThe4th Date: Tue, 27 May 2025 16:50:39 -0400 Subject: [PATCH 03/14] pushes search text to a subject (#14880) use distinctUntilChanged to prevent duplicate filtering operations Run filtering outside angular zone to prevent change detection issues --- .../vault-search/vault-v2-search.component.ts | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) 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 32f5611f436..b68818454d1 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,8 +1,8 @@ import { CommonModule } from "@angular/common"; -import { Component } from "@angular/core"; +import { Component, NgZone } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { FormsModule } from "@angular/forms"; -import { Subject, Subscription, debounceTime, filter } from "rxjs"; +import { Subject, Subscription, debounceTime, distinctUntilChanged, filter } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; import { SearchModule } from "@bitwarden/components"; @@ -22,13 +22,16 @@ export class VaultV2SearchComponent { private searchText$ = new Subject(); - constructor(private vaultPopupItemsService: VaultPopupItemsService) { + constructor( + private vaultPopupItemsService: VaultPopupItemsService, + private ngZone: NgZone, + ) { this.subscribeToLatestSearchText(); this.subscribeToApplyFilter(); } onSearchTextChanged() { - this.vaultPopupItemsService.applyFilter(this.searchText); + this.searchText$.next(this.searchText); } subscribeToLatestSearchText(): Subscription { @@ -44,9 +47,13 @@ export class VaultV2SearchComponent { subscribeToApplyFilter(): Subscription { return this.searchText$ - .pipe(debounceTime(SearchTextDebounceInterval), takeUntilDestroyed()) + .pipe(debounceTime(SearchTextDebounceInterval), distinctUntilChanged(), takeUntilDestroyed()) .subscribe((data) => { - this.vaultPopupItemsService.applyFilter(data); + this.ngZone.runOutsideAngular(() => { + this.ngZone.run(() => { + this.vaultPopupItemsService.applyFilter(data); + }); + }); }); } } From 4fcc4793bbfff10fd3d2a228be004d28ae5eb838 Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Wed, 28 May 2025 09:41:56 +1000 Subject: [PATCH 04/14] Add additional jsdoc to policyservice (#14934) --- .../policy/policy.service.abstraction.ts | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts b/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts index 68f9843c5bd..bf02872ed7c 100644 --- a/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts +++ b/libs/common/src/admin-console/abstractions/policy/policy.service.abstraction.ts @@ -7,6 +7,9 @@ import { MasterPasswordPolicyOptions } from "../../models/domain/master-password import { Policy } from "../../models/domain/policy"; import { ResetPasswordPolicyOptions } from "../../models/domain/reset-password-policy-options"; +/** + * The primary service for retrieving and evaluating policies from sync data. + */ export abstract class PolicyService { /** * All policies for the provided user from sync data. @@ -24,7 +27,7 @@ export abstract class PolicyService { abstract policiesByType$: (policyType: PolicyType, userId: UserId) => Observable; /** - * @returns true if a policy of the specified type applies to the specified user, otherwise false. + * @returns true if any policy of the specified type applies to the specified user, otherwise false. * A policy "applies" if it is enabled and the user is not exempt (e.g. because they are an Owner). * This does not take into account the policy's configuration - if that is important, use {@link policiesByType$} to get the * {@link Policy} objects and then filter by Policy.data. @@ -35,8 +38,12 @@ export abstract class PolicyService { /** * Combines all Master Password policies that apply to the user. + * If you are evaluating Master Password policies before the first sync has completed, + * you must supply your own `policies` value. + * @param userId The user against whom the policy needs to be enforced. + * @param policies The policies to be evaluated; if null or undefined, it will default to using policies from sync data. * @returns a set of options which represent the minimum Master Password settings that the user must - * comply with in order to comply with **all** Master Password policies. + * comply with in order to comply with **all** applicable Master Password policies. */ abstract masterPasswordPolicyOptions$: ( userId: UserId, @@ -62,7 +69,17 @@ export abstract class PolicyService { ) => [ResetPasswordPolicyOptions, boolean]; } +/** + * An "internal" extension of the `PolicyService` which allows the update of policy data in the local sync data. + * This does not update any policies on the server. + */ export abstract class InternalPolicyService extends PolicyService { + /** + * Upsert a policy in the local sync data. This does not update any policies on the server. + */ abstract upsert: (policy: PolicyData, userId: UserId) => Promise; + /** + * Replace a policy in the local sync data. This does not update any policies on the server. + */ abstract replace: (policies: { [id: string]: PolicyData }, userId: UserId) => Promise; } From d1fb37d696b98c64de0a291af868a56db99788cd Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Wed, 28 May 2025 15:00:30 +0200 Subject: [PATCH 05/14] [PM-17635] [PM-18601] Simplifying mocking and usage of the sdk (#14287) * feat: add our own custom deep mocker * feat: use new mock service in totp tests * feat: implement userClient mocking * chore: move mock files * feat: replace existing manual sdkService mocking * chore: rename to 'client' * chore: improve docs * feat: refactor sdkService to never return undefined BitwardenClient --- .../platform/abstractions/sdk/sdk.service.ts | 9 +- .../services/sdk/default-sdk.service.spec.ts | 8 +- .../services/sdk/default-sdk.service.ts | 5 +- .../src/platform/spec/mock-deep.spec.ts | 58 ++++ libs/common/src/platform/spec/mock-deep.ts | 271 ++++++++++++++++++ .../src/platform/spec/mock-sdk.service.ts | 81 ++++++ .../src/vault/services/totp.service.spec.ts | 31 +- .../src/services/import.service.spec.ts | 10 +- ...symmetric-key-regeneration.service.spec.ts | 25 +- 9 files changed, 444 insertions(+), 54 deletions(-) create mode 100644 libs/common/src/platform/spec/mock-deep.spec.ts create mode 100644 libs/common/src/platform/spec/mock-deep.ts create mode 100644 libs/common/src/platform/spec/mock-sdk.service.ts diff --git a/libs/common/src/platform/abstractions/sdk/sdk.service.ts b/libs/common/src/platform/abstractions/sdk/sdk.service.ts index 07dfb2aa0df..d629e4fe9fa 100644 --- a/libs/common/src/platform/abstractions/sdk/sdk.service.ts +++ b/libs/common/src/platform/abstractions/sdk/sdk.service.ts @@ -53,15 +53,18 @@ export abstract class SdkService { * This client can be used for operations that require a user context, such as retrieving ciphers * and operations involving crypto. It can also be used for operations that don't require a user context. * + * - If the user is not logged when the subscription is created, the observable will complete + * immediately with {@link UserNotLoggedInError}. + * - If the user is logged in, the observable will emit the client and complete whithout an error + * when the user logs out. + * * **WARNING:** Do not use `firstValueFrom(userClient$)`! Any operations on the client must be done within the observable. * The client will be destroyed when the observable is no longer subscribed to. * Please let platform know if you need a client that is not destroyed when the observable is no longer subscribed to. * * @param userId The user id for which to retrieve the client - * - * @throws {UserNotLoggedInError} If the user is not logged in */ - abstract userClient$(userId: UserId): Observable | undefined>; + abstract userClient$(userId: UserId): Observable>; /** * This method is used during/after an authentication procedure to set a new client for a specific user. diff --git a/libs/common/src/platform/services/sdk/default-sdk.service.spec.ts b/libs/common/src/platform/services/sdk/default-sdk.service.spec.ts index 6531be58f05..70a08257471 100644 --- a/libs/common/src/platform/services/sdk/default-sdk.service.spec.ts +++ b/libs/common/src/platform/services/sdk/default-sdk.service.spec.ts @@ -132,15 +132,13 @@ describe("DefaultSdkService", () => { ); keyService.userKey$.calledWith(userId).mockReturnValue(userKey$); - const subject = new BehaviorSubject | undefined>(undefined); - service.userClient$(userId).subscribe(subject); - await new Promise(process.nextTick); + const userClientTracker = new ObservableTracker(service.userClient$(userId), false); + await userClientTracker.pauseUntilReceived(1); userKey$.next(undefined); - await new Promise(process.nextTick); + await userClientTracker.expectCompletion(); expect(mockClient.free).toHaveBeenCalledTimes(1); - expect(subject.value).toBe(undefined); }); }); diff --git a/libs/common/src/platform/services/sdk/default-sdk.service.ts b/libs/common/src/platform/services/sdk/default-sdk.service.ts index 8e84642fb99..6be89a4b376 100644 --- a/libs/common/src/platform/services/sdk/default-sdk.service.ts +++ b/libs/common/src/platform/services/sdk/default-sdk.service.ts @@ -71,7 +71,7 @@ export class DefaultSdkService implements SdkService { private userAgent: string | null = null, ) {} - userClient$(userId: UserId): Observable | undefined> { + userClient$(userId: UserId): Observable> { return this.sdkClientOverrides.pipe( takeWhile((clients) => clients[userId] !== UnsetClient, false), map((clients) => { @@ -88,6 +88,7 @@ export class DefaultSdkService implements SdkService { return this.internalClient$(userId); }), + takeWhile((client) => client !== undefined, false), throwIfEmpty(() => new UserNotLoggedInError(userId)), ); } @@ -112,7 +113,7 @@ export class DefaultSdkService implements SdkService { * @param userId The user id for which to create the client * @returns An observable that emits the client for the user */ - private internalClient$(userId: UserId): Observable | undefined> { + private internalClient$(userId: UserId): Observable> { const cached = this.sdkClientCache.get(userId); if (cached !== undefined) { return cached; diff --git a/libs/common/src/platform/spec/mock-deep.spec.ts b/libs/common/src/platform/spec/mock-deep.spec.ts new file mode 100644 index 00000000000..535e02c11dd --- /dev/null +++ b/libs/common/src/platform/spec/mock-deep.spec.ts @@ -0,0 +1,58 @@ +import { mockDeep } from "./mock-deep"; + +class ToBeMocked { + property = "value"; + + method() { + return "method"; + } + + sub() { + return new SubToBeMocked(); + } +} + +class SubToBeMocked { + subProperty = "subValue"; + + sub() { + return new SubSubToBeMocked(); + } +} + +class SubSubToBeMocked { + subSubProperty = "subSubValue"; +} + +describe("deepMock", () => { + it("can mock properties", () => { + const mock = mockDeep(); + mock.property.replaceProperty("mocked value"); + expect(mock.property).toBe("mocked value"); + }); + + it("can mock methods", () => { + const mock = mockDeep(); + mock.method.mockReturnValue("mocked method"); + expect(mock.method()).toBe("mocked method"); + }); + + it("can mock sub-properties", () => { + const mock = mockDeep(); + mock.sub.mockDeep().subProperty.replaceProperty("mocked sub value"); + expect(mock.sub().subProperty).toBe("mocked sub value"); + }); + + it("can mock sub-sub-properties", () => { + const mock = mockDeep(); + mock.sub.mockDeep().sub.mockDeep().subSubProperty.replaceProperty("mocked sub-sub value"); + expect(mock.sub().sub().subSubProperty).toBe("mocked sub-sub value"); + }); + + it("returns the same mock object when calling mockDeep multiple times", () => { + const mock = mockDeep(); + const subMock1 = mock.sub.mockDeep(); + const subMock2 = mock.sub.mockDeep(); + expect(subMock1).toBe(subMock2); + }); +}); diff --git a/libs/common/src/platform/spec/mock-deep.ts b/libs/common/src/platform/spec/mock-deep.ts new file mode 100644 index 00000000000..89ef9a25451 --- /dev/null +++ b/libs/common/src/platform/spec/mock-deep.ts @@ -0,0 +1,271 @@ +// This is a modification of the code found in https://github.com/marchaos/jest-mock-extended +// to better support deep mocking of objects. + +// MIT License + +// Copyright (c) 2019 Marc McIntyre + +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: + +// The above copyright notice and this permission notice shall be included in all +// copies or substantial portions of the Software. + +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +import { jest } from "@jest/globals"; +import { FunctionLike } from "jest-mock"; +import { calledWithFn, MatchersOrLiterals } from "jest-mock-extended"; +import { PartialDeep } from "type-fest"; + +type ProxiedProperty = string | number | symbol; + +export interface GlobalConfig { + // ignoreProps is required when we don't want to return anything for a mock (for example, when mocking a promise). + ignoreProps?: ProxiedProperty[]; +} + +const DEFAULT_CONFIG: GlobalConfig = { + ignoreProps: ["then"], +}; + +let GLOBAL_CONFIG = DEFAULT_CONFIG; + +export const JestMockExtended = { + DEFAULT_CONFIG, + configure: (config: GlobalConfig) => { + // Shallow merge so they can override anything they want. + GLOBAL_CONFIG = { ...DEFAULT_CONFIG, ...config }; + }, + resetConfig: () => { + GLOBAL_CONFIG = DEFAULT_CONFIG; + }, +}; + +export interface CalledWithMock extends jest.Mock { + calledWith: (...args: [...MatchersOrLiterals>]) => jest.Mock; +} + +export interface MockDeepMock { + mockDeep: () => DeepMockProxy; +} + +export interface ReplaceProperty { + /** + * mockDeep will by default return a jest.fn() for all properties, + * but this allows you to replace the property with a value. + * @param value The value to replace the property with. + */ + replaceProperty(value: T): void; +} + +export type _MockProxy = { + [K in keyof T]: T[K] extends FunctionLike ? T[K] & CalledWithMock : T[K]; +}; + +export type MockProxy = _MockProxy & T; + +export type _DeepMockProxy = { + // This supports deep mocks in the else branch + [K in keyof T]: T[K] extends (...args: infer A) => infer R + ? T[K] & CalledWithMock & MockDeepMock + : T[K] & ReplaceProperty & _DeepMockProxy; +}; + +// we intersect with T here instead of on the mapped type above to +// prevent immediate type resolution on a recursive type, this will +// help to improve performance for deeply nested recursive mocking +// at the same time, this intersection preserves private properties +export type DeepMockProxy = _DeepMockProxy & T; + +export type _DeepMockProxyWithFuncPropSupport = { + // This supports deep mocks in the else branch + [K in keyof T]: T[K] extends FunctionLike + ? CalledWithMock & DeepMockProxy + : DeepMockProxy; +}; + +export type DeepMockProxyWithFuncPropSupport = _DeepMockProxyWithFuncPropSupport & T; + +export interface MockOpts { + deep?: boolean; + fallbackMockImplementation?: (...args: any[]) => any; +} + +export const mockClear = (mock: MockProxy) => { + for (const key of Object.keys(mock)) { + if (mock[key] === null || mock[key] === undefined) { + continue; + } + + if (mock[key]._isMockObject) { + mockClear(mock[key]); + } + + if (mock[key]._isMockFunction) { + mock[key].mockClear(); + } + } + + // This is a catch for if they pass in a jest.fn() + if (!mock._isMockObject) { + return mock.mockClear(); + } +}; + +export const mockReset = (mock: MockProxy) => { + for (const key of Object.keys(mock)) { + if (mock[key] === null || mock[key] === undefined) { + continue; + } + + if (mock[key]._isMockObject) { + mockReset(mock[key]); + } + if (mock[key]._isMockFunction) { + mock[key].mockReset(); + } + } + + // This is a catch for if they pass in a jest.fn() + // Worst case, we will create a jest.fn() (since this is a proxy) + // below in the get and call mockReset on it + if (!mock._isMockObject) { + return mock.mockReset(); + } +}; + +export function mockDeep( + opts: { + funcPropSupport?: true; + fallbackMockImplementation?: MockOpts["fallbackMockImplementation"]; + }, + mockImplementation?: PartialDeep, +): DeepMockProxyWithFuncPropSupport; +export function mockDeep(mockImplementation?: PartialDeep): DeepMockProxy; +export function mockDeep(arg1: any, arg2?: any) { + const [opts, mockImplementation] = + typeof arg1 === "object" && + (typeof arg1.fallbackMockImplementation === "function" || arg1.funcPropSupport === true) + ? [arg1, arg2] + : [{}, arg1]; + return mock(mockImplementation, { + deep: true, + fallbackMockImplementation: opts.fallbackMockImplementation, + }); +} + +const overrideMockImp = (obj: PartialDeep, opts?: MockOpts) => { + const proxy = new Proxy>(obj, handler(opts)); + for (const name of Object.keys(obj)) { + if (typeof obj[name] === "object" && obj[name] !== null) { + proxy[name] = overrideMockImp(obj[name], opts); + } else { + proxy[name] = obj[name]; + } + } + + return proxy; +}; + +const handler = (opts?: MockOpts): ProxyHandler => ({ + ownKeys(target: MockProxy) { + return Reflect.ownKeys(target); + }, + + set: (obj: MockProxy, property: ProxiedProperty, value: any) => { + obj[property] = value; + return true; + }, + + get: (obj: MockProxy, property: ProxiedProperty) => { + const fn = calledWithFn({ fallbackMockImplementation: opts?.fallbackMockImplementation }); + + if (!(property in obj)) { + if (GLOBAL_CONFIG.ignoreProps?.includes(property)) { + return undefined; + } + // Jest's internal equality checking does some wierd stuff to check for iterable equality + if (property === Symbol.iterator) { + return obj[property]; + } + + if (property === "_deepMock") { + return obj[property]; + } + // So this calls check here is totally not ideal - jest internally does a + // check to see if this is a spy - which we want to say no to, but blindly returning + // an proxy for calls results in the spy check returning true. This is another reason + // why deep is opt in. + if (opts?.deep && property !== "calls") { + obj[property] = new Proxy>(fn, handler(opts)); + obj[property].replaceProperty = (value: T[K]) => { + obj[property] = value; + }; + obj[property].mockDeep = () => { + if (obj[property]._deepMock) { + return obj[property]._deepMock; + } + + const mock = mockDeep({ + fallbackMockImplementation: opts?.fallbackMockImplementation, + }); + (obj[property] as CalledWithMock).mockReturnValue(mock); + obj[property]._deepMock = mock; + return mock; + }; + obj[property]._isMockObject = true; + } else { + obj[property] = calledWithFn({ + fallbackMockImplementation: opts?.fallbackMockImplementation, + }); + } + } + + // @ts-expect-error Hack by author of jest-mock-extended + if (obj instanceof Date && typeof obj[property] === "function") { + // @ts-expect-error Hack by author of jest-mock-extended + return obj[property].bind(obj); + } + + return obj[property]; + }, +}); + +const mock = & T = MockProxy & T>( + mockImplementation: PartialDeep = {} as PartialDeep, + opts?: MockOpts, +): MockedReturn => { + // @ts-expect-error private + mockImplementation!._isMockObject = true; + return overrideMockImp(mockImplementation, opts); +}; + +export const mockFn = (): CalledWithMock & T => { + // @ts-expect-error Hack by author of jest-mock-extended + return calledWithFn(); +}; + +export const stub = (): T => { + return new Proxy({} as T, { + get: (obj, property: ProxiedProperty) => { + if (property in obj) { + // @ts-expect-error Hack by author of jest-mock-extended + return obj[property]; + } + return jest.fn(); + }, + }); +}; + +export default mock; diff --git a/libs/common/src/platform/spec/mock-sdk.service.ts b/libs/common/src/platform/spec/mock-sdk.service.ts new file mode 100644 index 00000000000..66a6ab3ec84 --- /dev/null +++ b/libs/common/src/platform/spec/mock-sdk.service.ts @@ -0,0 +1,81 @@ +import { + BehaviorSubject, + distinctUntilChanged, + map, + Observable, + takeWhile, + throwIfEmpty, +} from "rxjs"; + +import { BitwardenClient } from "@bitwarden/sdk-internal"; + +import { UserId } from "../../types/guid"; +import { SdkService, UserNotLoggedInError } from "../abstractions/sdk/sdk.service"; +import { Rc } from "../misc/reference-counting/rc"; + +import { DeepMockProxy, mockDeep } from "./mock-deep"; + +export class MockSdkService implements SdkService { + private userClients$ = new BehaviorSubject<{ + [userId: UserId]: Rc | undefined; + }>({}); + + private _client$ = new BehaviorSubject(mockDeep()); + client$ = this._client$.asObservable(); + + version$ = new BehaviorSubject("0.0.1-test").asObservable(); + + userClient$(userId: UserId): Observable> { + return this.userClients$.pipe( + takeWhile((clients) => clients[userId] !== undefined, false), + map((clients) => clients[userId] as Rc), + distinctUntilChanged(), + throwIfEmpty(() => new UserNotLoggedInError(userId)), + ); + } + + setClient(): void { + throw new Error("Not supported in mock service"); + } + + /** + * Returns the non-user scoped client mock. + * This is what is returned by the `client$` observable. + */ + get client(): DeepMockProxy { + return this._client$.value; + } + + readonly simulate = { + /** + * Simulates a user login, and returns a user-scoped mock for the user. + * This will be return by the `userClient$` observable. + * + * @param userId The userId to simulate login for. + * @returns A user-scoped mock for the user. + */ + userLogin: (userId: UserId) => { + const client = mockDeep(); + this.userClients$.next({ + ...this.userClients$.getValue(), + [userId]: new Rc(client), + }); + return client; + }, + + /** + * Simulates a user logout, and disposes the user-scoped mock for the user. + * This will remove the user-scoped mock from the `userClient$` observable. + * + * @param userId The userId to simulate logout for. + */ + userLogout: (userId: UserId) => { + const clients = this.userClients$.value; + clients[userId]?.markForDisposal(); + this.userClients$.next({ + ...clients, + [userId]: undefined, + }); + }, + }; +} diff --git a/libs/common/src/vault/services/totp.service.spec.ts b/libs/common/src/vault/services/totp.service.spec.ts index c653b4ce1db..4aca262d537 100644 --- a/libs/common/src/vault/services/totp.service.spec.ts +++ b/libs/common/src/vault/services/totp.service.spec.ts @@ -1,38 +1,27 @@ -import { mock } from "jest-mock-extended"; -import { of, take } from "rxjs"; +import { take } from "rxjs"; -import { BitwardenClient, TotpResponse } from "@bitwarden/sdk-internal"; +import { TotpResponse } from "@bitwarden/sdk-internal"; -import { SdkService } from "../../platform/abstractions/sdk/sdk.service"; +import { MockSdkService } from "../../platform/spec/mock-sdk.service"; import { TotpService } from "./totp.service"; describe("TotpService", () => { - let totpService: TotpService; - let generateTotpMock: jest.Mock; - - const sdkService = mock(); + let totpService!: TotpService; + let sdkService!: MockSdkService; beforeEach(() => { - generateTotpMock = jest - .fn() - .mockReturnValueOnce({ + sdkService = new MockSdkService(); + sdkService.client.vault + .mockDeep() + .totp.mockDeep() + .generate_totp.mockReturnValueOnce({ code: "123456", period: 30, }) .mockReturnValueOnce({ code: "654321", period: 30 }) .mockReturnValueOnce({ code: "567892", period: 30 }); - const mockBitwardenClient = { - vault: () => ({ - totp: () => ({ - generate_totp: generateTotpMock, - }), - }), - }; - - sdkService.client$ = of(mockBitwardenClient as unknown as BitwardenClient); - totpService = new TotpService(sdkService); // TOTP is time-based, so we need to mock the current time diff --git a/libs/importer/src/services/import.service.spec.ts b/libs/importer/src/services/import.service.spec.ts index 30309a3d9c2..6c8656f4c1d 100644 --- a/libs/importer/src/services/import.service.spec.ts +++ b/libs/importer/src/services/import.service.spec.ts @@ -1,5 +1,4 @@ import { mock, MockProxy } from "jest-mock-extended"; -import { of } from "rxjs"; // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. // eslint-disable-next-line no-restricted-imports @@ -8,14 +7,13 @@ import { PinServiceAbstraction } from "@bitwarden/auth/common"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; -import { SdkService } from "@bitwarden/common/platform/abstractions/sdk/sdk.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; +import { MockSdkService } from "@bitwarden/common/platform/spec/mock-sdk.service"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { FolderService } from "@bitwarden/common/vault/abstractions/folder/folder.service.abstraction"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { FolderView } from "@bitwarden/common/vault/models/view/folder.view"; import { KeyService } from "@bitwarden/key-management"; -import { BitwardenClient } from "@bitwarden/sdk-internal"; import { BitwardenPasswordProtectedImporter } from "../importers/bitwarden/bitwarden-password-protected-importer"; import { Importer } from "../importers/importer"; @@ -35,7 +33,7 @@ describe("ImportService", () => { let encryptService: MockProxy; let pinService: MockProxy; let accountService: MockProxy; - let sdkService: MockProxy; + let sdkService: MockSdkService; beforeEach(() => { cipherService = mock(); @@ -46,9 +44,7 @@ describe("ImportService", () => { keyService = mock(); encryptService = mock(); pinService = mock(); - const mockClient = mock(); - sdkService = mock(); - sdkService.client$ = of(mockClient, mockClient, mockClient); + sdkService = new MockSdkService(); importService = new ImportService( cipherService, diff --git a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts index 35cef914588..84d1dd7ad72 100644 --- a/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts +++ b/libs/key-management/src/user-asymmetric-key-regeneration/services/default-user-asymmetric-key-regeneration.service.spec.ts @@ -5,17 +5,17 @@ import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; -import { SdkService } from "@bitwarden/common/platform/abstractions/sdk/sdk.service"; import { EncryptedString } from "@bitwarden/common/platform/models/domain/enc-string"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { ContainerService } from "@bitwarden/common/platform/services/container.service"; +import { MockSdkService } from "@bitwarden/common/platform/spec/mock-sdk.service"; import { makeStaticByteArray, mockEnc } from "@bitwarden/common/spec"; import { CsprngArray } from "@bitwarden/common/types/csprng"; import { UserId } from "@bitwarden/common/types/guid"; import { UserKey } from "@bitwarden/common/types/key"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { Cipher } from "@bitwarden/common/vault/models/domain/cipher"; -import { BitwardenClient, VerifyAsymmetricKeysResponse } from "@bitwarden/sdk-internal"; +import { VerifyAsymmetricKeysResponse } from "@bitwarden/sdk-internal"; import { KeyService } from "../../abstractions/key.service"; import { UserAsymmetricKeysRegenerationApiService } from "../abstractions/user-asymmetric-key-regeneration-api.service"; @@ -24,24 +24,17 @@ import { DefaultUserAsymmetricKeysRegenerationService } from "./default-user-asy function setupVerificationResponse( mockVerificationResponse: VerifyAsymmetricKeysResponse, - sdkService: MockProxy, + sdkService: MockSdkService, ) { const mockKeyPairResponse = { userPublicKey: "userPublicKey", userKeyEncryptedPrivateKey: "userKeyEncryptedPrivateKey", }; - sdkService.client$ = of({ - crypto: () => ({ - verify_asymmetric_keys: jest.fn().mockReturnValue(mockVerificationResponse), - make_key_pair: jest.fn().mockReturnValue(mockKeyPairResponse), - }), - free: jest.fn(), - echo: jest.fn(), - version: jest.fn(), - throw: jest.fn(), - catch: jest.fn(), - } as unknown as BitwardenClient); + sdkService.client.crypto + .mockDeep() + .verify_asymmetric_keys.mockReturnValue(mockVerificationResponse); + sdkService.client.crypto.mockDeep().make_key_pair.mockReturnValue(mockKeyPairResponse); } function setupUserKeyValidation( @@ -74,7 +67,7 @@ describe("regenerateIfNeeded", () => { let cipherService: MockProxy; let userAsymmetricKeysRegenerationApiService: MockProxy; let logService: MockProxy; - let sdkService: MockProxy; + let sdkService: MockSdkService; let apiService: MockProxy; let configService: MockProxy; let encryptService: MockProxy; @@ -84,7 +77,7 @@ describe("regenerateIfNeeded", () => { cipherService = mock(); userAsymmetricKeysRegenerationApiService = mock(); logService = mock(); - sdkService = mock(); + sdkService = new MockSdkService(); apiService = mock(); configService = mock(); encryptService = mock(); From 4bf1a3b670961eedbcf6f86dba2ac9e280ace708 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Wed, 28 May 2025 07:16:46 -0700 Subject: [PATCH 06/14] [deps] Platform: Update Rust crate bytes to v1.10.1 (#14922) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- apps/desktop/desktop_native/Cargo.lock | 4 ++-- apps/desktop/desktop_native/Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/desktop/desktop_native/Cargo.lock b/apps/desktop/desktop_native/Cargo.lock index 34819a3981b..28d64e4f504 100644 --- a/apps/desktop/desktop_native/Cargo.lock +++ b/apps/desktop/desktop_native/Cargo.lock @@ -498,9 +498,9 @@ checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b" [[package]] name = "bytes" -version = "1.9.0" +version = "1.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "325918d6fe32f23b19878fe4b34794ae41fc19ddbe53b10571a4874d44ffd39b" +checksum = "d71b6127be86fdcfddb610f7182ac57211d4b18a3e9c82eb2d17662f2227ad6a" [[package]] name = "camino" diff --git a/apps/desktop/desktop_native/Cargo.toml b/apps/desktop/desktop_native/Cargo.toml index c4a2ed98e70..1fce5a7c597 100644 --- a/apps/desktop/desktop_native/Cargo.toml +++ b/apps/desktop/desktop_native/Cargo.toml @@ -18,7 +18,7 @@ base64 = "=0.22.1" bindgen = "=0.71.1" bitwarden-russh = { git = "https://github.com/bitwarden/bitwarden-russh.git", rev = "3d48f140fd506412d186203238993163a8c4e536" } byteorder = "=1.5.0" -bytes = "=1.9.0" +bytes = "=1.10.1" cbc = "=0.1.2" core-foundation = "=0.10.0" dirs = "=6.0.0" From 169fdd5883352acae4fcc976d4122da5e9ea0a29 Mon Sep 17 00:00:00 2001 From: cd-bitwarden <106776772+cd-bitwarden@users.noreply.github.com> Date: Wed, 28 May 2025 10:56:44 -0400 Subject: [PATCH 07/14] [PM-20650] Feature flag addition to clients (#14824) * Feature flag addition to clients * Updating feature flag name --- libs/common/src/enums/feature-flag.enum.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 8cd44f9d627..43b36c5692f 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -59,6 +59,7 @@ export enum FeatureFlag { CipherKeyEncryption = "cipher-key-encryption", PM18520_UpdateDesktopCipherForm = "pm-18520-desktop-cipher-forms", EndUserNotifications = "pm-10609-end-user-notifications", + RemoveCardItemTypePolicy = "pm-16442-remove-card-item-type-policy", /* Platform */ IpcChannelFramework = "ipc-channel-framework", @@ -107,6 +108,7 @@ export const DefaultFeatureFlagValue = { [FeatureFlag.PM18520_UpdateDesktopCipherForm]: FALSE, [FeatureFlag.EndUserNotifications]: FALSE, [FeatureFlag.PM19941MigrateCipherDomainToSdk]: FALSE, + [FeatureFlag.RemoveCardItemTypePolicy]: FALSE, /* Auth */ [FeatureFlag.PM16117_ChangeExistingPasswordRefactor]: FALSE, From 6351fc0e69e46e91da8182bed7be1b15e66d19c4 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Wed, 28 May 2025 13:36:46 -0700 Subject: [PATCH 08/14] fix(tailwind): [Auth/PM-22140] Use Tailwind for Password Settings header (#14978) `PM16117_ChangeExistingPasswordRefactor` flag ON --- .../password-settings/password-settings.component.html | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/apps/web/src/app/auth/settings/security/password-settings/password-settings.component.html b/apps/web/src/app/auth/settings/security/password-settings/password-settings.component.html index 94cf08b5871..fc6620762f9 100644 --- a/apps/web/src/app/auth/settings/security/password-settings/password-settings.component.html +++ b/apps/web/src/app/auth/settings/security/password-settings/password-settings.component.html @@ -1,6 +1,4 @@ -
-

{{ "changeMasterPassword" | i18n }}

-
+

{{ "changeMasterPassword" | i18n }}

{{ "loggedOutWarning" | i18n }} From 798acc7cba58fed70e55190534d60be14653d333 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Thu, 29 May 2025 15:17:04 +0200 Subject: [PATCH 09/14] [PM-21884] Fix DuckDuckGo integration when SDK is enabled for decrypt (#14884) * Fix ddg integration when sdk is enabled for decryption * Fix comments --- .../duckduckgo-message-handler.service.ts | 46 +++++++++++++++++-- 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/apps/desktop/src/services/duckduckgo-message-handler.service.ts b/apps/desktop/src/services/duckduckgo-message-handler.service.ts index 6fb91231be1..7bddaba499c 100644 --- a/apps/desktop/src/services/duckduckgo-message-handler.service.ts +++ b/apps/desktop/src/services/duckduckgo-message-handler.service.ts @@ -188,13 +188,10 @@ export class DuckDuckGoMessageHandlerService { } try { - let decryptedResult = await this.encryptService.decryptString( + const decryptedResult = await this.decryptDuckDuckGoEncString( message.encryptedCommand as EncString, this.duckduckgoSharedSecret, ); - - decryptedResult = this.trimNullCharsFromMessage(decryptedResult); - return JSON.parse(decryptedResult); } catch { this.sendResponse({ @@ -237,7 +234,46 @@ export class DuckDuckGoMessageHandlerService { ipc.platform.nativeMessaging.sendReply(response); } - // Trim all null bytes padded at the end of messages. This happens with C encryption libraries. + /* + * Bitwarden type 2 (AES256-CBC-HMAC256) uses PKCS7 padding. + * DuckDuckGo does not use PKCS7 padding; and instead fills the last CBC block with null bytes. + * ref: https://github.com/duckduckgo/apple-browsers/blob/04d678b447869c3a640714718a466b36407db8b6/macOS/DuckDuckGo/PasswordManager/Bitwarden/Services/BWEncryption.m#L141 + * + * This is incompatible which means the default encryptService cannot be used to decrypt the message, + * a custom EncString decrypt operation is needed. + * + * This function also trims null characters that are a result of the null-padding from the end of the message. + */ + private async decryptDuckDuckGoEncString( + encString: EncString, + key: SymmetricCryptoKey, + ): Promise { + const fastParams = this.cryptoFunctionService.aesDecryptFastParameters( + encString.data, + encString.iv, + encString.mac, + key, + ); + + const computedMac = await this.cryptoFunctionService.hmacFast( + fastParams.macData, + fastParams.macKey, + "sha256", + ); + const macsEqual = await this.cryptoFunctionService.compareFast(fastParams.mac, computedMac); + if (!macsEqual) { + return null; + } + const decryptedPaddedString = await this.cryptoFunctionService.aesDecryptFast({ + mode: "cbc", + parameters: fastParams, + }); + return this.trimNullCharsFromMessage(decryptedPaddedString); + } + + // DuckDuckGo does not use PKCS7 padding, but instead leaves the values as null, + // so null characters need to be trimmed from the end of the message for the last + // CBC-block. private trimNullCharsFromMessage(message: string): string { const charNull = 0; const charRightCurlyBrace = 125; From b48356228c5ca1437f5ede48f34aa0c79e877d79 Mon Sep 17 00:00:00 2001 From: Leslie Tilton <23057410+Banrion@users.noreply.github.com> Date: Thu, 29 May 2025 08:45:40 -0500 Subject: [PATCH 10/14] Update risk insights report to default an invalid uri to the original uri (#14800) --- .../reports/risk-insights/services/ciphers.mock.ts | 3 +++ .../services/risk-insights-report.service.spec.ts | 14 +++++++++----- .../services/risk-insights-report.service.ts | 2 +- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/ciphers.mock.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/ciphers.mock.ts index ca5cdc35b8a..f697d24f208 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/ciphers.mock.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/ciphers.mock.ts @@ -27,6 +27,9 @@ export const mockCiphers: any[] = [ createLoginUriView("accounts.google.com"), createLoginUriView("https://www.google.com"), createLoginUriView("https://www.google.com/login"), + createLoginUriView("www.invalid@uri@.com"), + createLoginUriView("www.invaliduri!.com"), + createLoginUriView("this_is-not|a-valid-uri123@+"), ], }, edit: false, diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-report.service.spec.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-report.service.spec.ts index f9177bf1bf7..3aa624f1e59 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-report.service.spec.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-report.service.spec.ts @@ -50,7 +50,7 @@ describe("RiskInsightsReportService", () => { let testCase = testCaseResults[0]; expect(testCase).toBeTruthy(); expect(testCase.cipherMembers).toHaveLength(2); - expect(testCase.trimmedUris).toHaveLength(2); + expect(testCase.trimmedUris).toHaveLength(5); expect(testCase.weakPasswordDetail).toBeTruthy(); expect(testCase.exposedPasswordDetail).toBeTruthy(); expect(testCase.reusedPasswordCount).toEqual(2); @@ -69,12 +69,16 @@ describe("RiskInsightsReportService", () => { it("should generate the raw data + uri report correctly", async () => { const result = await firstValueFrom(service.generateRawDataUriReport$("orgId")); - expect(result).toHaveLength(8); + expect(result).toHaveLength(11); // Two ciphers that have google.com as their uri. There should be 2 results const googleResults = result.filter((x) => x.trimmedUri === "google.com"); expect(googleResults).toHaveLength(2); + // There is an invalid uri and it should not be trimmed + const invalidUriResults = result.filter((x) => x.trimmedUri === "this_is-not|a-valid-uri123@+"); + expect(invalidUriResults).toHaveLength(1); + // Verify the details for one of the googles matches the password health info // expected const firstGoogle = googleResults.filter( @@ -88,7 +92,7 @@ describe("RiskInsightsReportService", () => { it("should generate applications health report data correctly", async () => { const result = await firstValueFrom(service.generateApplicationsReport$("orgId")); - expect(result).toHaveLength(5); + expect(result).toHaveLength(8); // Two ciphers have google.com associated with them. The first cipher // has 2 members and the second has 4. However, the 2 members in the first @@ -132,7 +136,7 @@ describe("RiskInsightsReportService", () => { expect(reportSummary.totalMemberCount).toEqual(7); expect(reportSummary.totalAtRiskMemberCount).toEqual(6); - expect(reportSummary.totalApplicationCount).toEqual(5); - expect(reportSummary.totalAtRiskApplicationCount).toEqual(4); + expect(reportSummary.totalApplicationCount).toEqual(8); + expect(reportSummary.totalAtRiskApplicationCount).toEqual(7); }); }); diff --git a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-report.service.ts b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-report.service.ts index e4fece801b6..6fdab58115d 100644 --- a/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-report.service.ts +++ b/bitwarden_license/bit-common/src/dirt/reports/risk-insights/services/risk-insights-report.service.ts @@ -433,7 +433,7 @@ export class RiskInsightsReportService { const cipherUris: string[] = []; const uris = cipher.login?.uris ?? []; uris.map((u: { uri: string }) => { - const uri = Utils.getDomain(u.uri); + const uri = Utils.getDomain(u.uri) ?? u.uri; if (!cipherUris.includes(uri)) { cipherUris.push(uri); } From c48e4be14b546d53bf77d837df6d7b0bc737ead6 Mon Sep 17 00:00:00 2001 From: Andreas Coroiu Date: Thu, 29 May 2025 16:05:28 +0200 Subject: [PATCH 11/14] Pin @types/lowdb to v1 (#14957) --- .github/renovate.json5 | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/renovate.json5 b/.github/renovate.json5 index f30bc06e4a2..453e5e29c44 100644 --- a/.github/renovate.json5 +++ b/.github/renovate.json5 @@ -413,6 +413,12 @@ allowedVersions: "1.0.0", description: "Higher versions of lowdb are not compatible with CommonJS", }, + { + // Pin types as well since we are not upgrading past v1 (and also v2+ does not need separate types). + matchPackageNames: ["@types/lowdb"], + allowedVersions: "< 2.0.0", + description: "Higher versions of lowdb do not need separate types", + }, ], ignoreDeps: ["@types/koa-bodyparser", "bootstrap", "node-ipc", "@bitwarden/sdk-internal"], } From 0715597e8e059e22e232df376054a01baf6610c5 Mon Sep 17 00:00:00 2001 From: cyprain-okeke <108260115+cyprain-okeke@users.noreply.github.com> Date: Thu, 29 May 2025 15:06:07 +0100 Subject: [PATCH 12/14] [PM-21603]Invite Member sub text seat count does not account for sponsorships (#14954) * Resolve the membership count * Get the occupied Seat count from metadata --- .../admin-console/organizations/members/members.component.ts | 5 ++++- .../response/organization-billing-metadata.response.ts | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/apps/web/src/app/admin-console/organizations/members/members.component.ts b/apps/web/src/app/admin-console/organizations/members/members.component.ts index e5a94bc4b4f..4f453762b5d 100644 --- a/apps/web/src/app/admin-console/organizations/members/members.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/members.component.ts @@ -110,8 +110,10 @@ export class MembersComponent extends BaseMembersComponent protected rowHeight = 69; protected rowHeightClass = `tw-h-[69px]`; + private organizationUsersCount = 0; + get occupiedSeatCount(): number { - return this.dataSource.activeUserCount; + return this.organizationUsersCount; } constructor( @@ -218,6 +220,7 @@ export class MembersComponent extends BaseMembersComponent ); this.orgIsOnSecretsManagerStandalone = billingMetadata.isOnSecretsManagerStandalone; + this.organizationUsersCount = billingMetadata.organizationOccupiedSeats; await this.load(); diff --git a/libs/common/src/billing/models/response/organization-billing-metadata.response.ts b/libs/common/src/billing/models/response/organization-billing-metadata.response.ts index d30ad76a147..aa34c37bd1d 100644 --- a/libs/common/src/billing/models/response/organization-billing-metadata.response.ts +++ b/libs/common/src/billing/models/response/organization-billing-metadata.response.ts @@ -11,6 +11,7 @@ export class OrganizationBillingMetadataResponse extends BaseResponse { invoiceCreatedDate: Date | null; subPeriodEndDate: Date | null; isSubscriptionCanceled: boolean; + organizationOccupiedSeats: number; constructor(response: any) { super(response); @@ -25,6 +26,7 @@ export class OrganizationBillingMetadataResponse extends BaseResponse { this.invoiceCreatedDate = this.parseDate(this.getResponseProperty("InvoiceCreatedDate")); this.subPeriodEndDate = this.parseDate(this.getResponseProperty("SubPeriodEndDate")); this.isSubscriptionCanceled = this.getResponseProperty("IsSubscriptionCanceled"); + this.organizationOccupiedSeats = this.getResponseProperty("OrganizationOccupiedSeats"); } private parseDate(dateString: any): Date | null { From 058eb9a04be3b7d422694bfb4519a84d477aad25 Mon Sep 17 00:00:00 2001 From: Jared McCannon Date: Thu, 29 May 2025 11:17:30 -0400 Subject: [PATCH 13/14] [PM-19127] - Nested Traverse Optimization (#14881) * Draft optimization of getNestedCollectionTree * Added feature flag to wrap nestedTraverse_vNext. added the old implementation back in for feature flagging. * Correction from CR * Copied tests over for the vNext method. --------- Co-authored-by: Thomas Rittson --- .../collections/utils/collection-utils.ts | 25 ++++++ .../collections/vault.component.ts | 19 ++++- .../vault/individual-vault/vault.component.ts | 15 +++- libs/common/src/enums/feature-flag.enum.ts | 2 + libs/common/src/vault/service-utils.spec.ts | 18 +++++ libs/common/src/vault/service-utils.ts | 81 ++++++++++++++++--- 6 files changed, 141 insertions(+), 19 deletions(-) diff --git a/apps/web/src/app/admin-console/organizations/collections/utils/collection-utils.ts b/apps/web/src/app/admin-console/organizations/collections/utils/collection-utils.ts index 95ae911bbf6..f19c3f64530 100644 --- a/apps/web/src/app/admin-console/organizations/collections/utils/collection-utils.ts +++ b/apps/web/src/app/admin-console/organizations/collections/utils/collection-utils.ts @@ -37,6 +37,31 @@ export function getNestedCollectionTree( return nodes; } +export function getNestedCollectionTree_vNext( + collections: (CollectionView | CollectionAdminView)[], +): TreeNode[] { + if (!collections) { + return []; + } + + // Collections need to be cloned because ServiceUtils.nestedTraverse actively + // modifies the names of collections. + // These changes risk affecting collections store in StateService. + const clonedCollections = collections + .sort((a, b) => a.name.localeCompare(b.name)) + .map(cloneCollection); + + const nodes: TreeNode[] = []; + clonedCollections.forEach((collection) => { + const parts = + collection.name != null + ? collection.name.replace(/^\/+|\/+$/g, "").split(NestingDelimiter) + : []; + ServiceUtils.nestedTraverse_vNext(nodes, 0, parts, collection, null, NestingDelimiter); + }); + return nodes; +} + export function getFlatCollectionTree( nodes: TreeNode[], ): CollectionAdminView[]; diff --git a/apps/web/src/app/admin-console/organizations/collections/vault.component.ts b/apps/web/src/app/admin-console/organizations/collections/vault.component.ts index a3b62838d6a..19373f193d9 100644 --- a/apps/web/src/app/admin-console/organizations/collections/vault.component.ts +++ b/apps/web/src/app/admin-console/organizations/collections/vault.component.ts @@ -125,7 +125,11 @@ import { BulkCollectionsDialogResult, } from "./bulk-collections-dialog"; import { CollectionAccessRestrictedComponent } from "./collection-access-restricted.component"; -import { getNestedCollectionTree, getFlatCollectionTree } from "./utils"; +import { + getNestedCollectionTree, + getFlatCollectionTree, + getNestedCollectionTree_vNext, +} from "./utils"; import { VaultFilterModule } from "./vault-filter/vault-filter.module"; import { VaultHeaderComponent } from "./vault-header/vault-header.component"; @@ -420,9 +424,16 @@ export class VaultComponent implements OnInit, OnDestroy { }), ); - const nestedCollections$ = allCollections$.pipe( - map((collections) => getNestedCollectionTree(collections)), - shareReplay({ refCount: true, bufferSize: 1 }), + const nestedCollections$ = combineLatest([ + this.allCollectionsWithoutUnassigned$, + this.configService.getFeatureFlag$(FeatureFlag.OptimizeNestedTraverseTypescript), + ]).pipe( + map( + ([collections, shouldOptimize]) => + (shouldOptimize + ? getNestedCollectionTree_vNext(collections) + : getNestedCollectionTree(collections)) as TreeNode[], + ), ); const collections$ = combineLatest([ diff --git a/apps/web/src/app/vault/individual-vault/vault.component.ts b/apps/web/src/app/vault/individual-vault/vault.component.ts index 6e751f600dc..0dfaa1ac589 100644 --- a/apps/web/src/app/vault/individual-vault/vault.component.ts +++ b/apps/web/src/app/vault/individual-vault/vault.component.ts @@ -49,7 +49,9 @@ import { OrganizationBillingServiceAbstraction } from "@bitwarden/common/billing import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { BillingApiServiceAbstraction } from "@bitwarden/common/billing/abstractions/billing-api.service.abstraction"; import { EventType } from "@bitwarden/common/enums"; +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { BroadcasterService } from "@bitwarden/common/platform/abstractions/broadcaster.service"; +import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; @@ -82,6 +84,7 @@ import { import { getNestedCollectionTree, getFlatCollectionTree, + getNestedCollectionTree_vNext, } from "../../admin-console/organizations/collections"; import { CollectionDialogAction, @@ -270,6 +273,7 @@ export class VaultComponent implements OnInit, OnDestroy { private trialFlowService: TrialFlowService, private organizationBillingService: OrganizationBillingServiceAbstraction, private billingNotificationService: BillingNotificationService, + private configService: ConfigService, ) {} async ngOnInit() { @@ -326,8 +330,15 @@ export class VaultComponent implements OnInit, OnDestroy { const filter$ = this.routedVaultFilterService.filter$; const allCollections$ = this.collectionService.decryptedCollections$; - const nestedCollections$ = allCollections$.pipe( - map((collections) => getNestedCollectionTree(collections)), + const nestedCollections$ = combineLatest([ + allCollections$, + this.configService.getFeatureFlag$(FeatureFlag.OptimizeNestedTraverseTypescript), + ]).pipe( + map(([collections, shouldOptimize]) => + shouldOptimize + ? getNestedCollectionTree_vNext(collections) + : getNestedCollectionTree(collections), + ), ); this.searchText$ diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 43b36c5692f..696f7028159 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -13,6 +13,7 @@ export enum FeatureFlag { /* Admin Console Team */ LimitItemDeletion = "pm-15493-restrict-item-deletion-to-can-manage-permission", SeparateCustomRolePermissions = "pm-19917-separate-custom-role-permissions", + OptimizeNestedTraverseTypescript = "pm-21695-optimize-nested-traverse-typescript", /* Auth */ PM16117_ChangeExistingPasswordRefactor = "pm-16117-change-existing-password-refactor", @@ -82,6 +83,7 @@ export const DefaultFeatureFlagValue = { /* Admin Console Team */ [FeatureFlag.LimitItemDeletion]: FALSE, [FeatureFlag.SeparateCustomRolePermissions]: FALSE, + [FeatureFlag.OptimizeNestedTraverseTypescript]: FALSE, /* Autofill */ [FeatureFlag.BlockBrowserInjectionsByDomain]: FALSE, diff --git a/libs/common/src/vault/service-utils.spec.ts b/libs/common/src/vault/service-utils.spec.ts index db414da76d7..619d3d72ee6 100644 --- a/libs/common/src/vault/service-utils.spec.ts +++ b/libs/common/src/vault/service-utils.spec.ts @@ -36,6 +36,24 @@ describe("serviceUtils", () => { }); }); + describe("nestedTraverse_vNext", () => { + it("should traverse a tree and add a node at the correct position given a valid path", () => { + const nodeToBeAdded: FakeObject = { id: "1.2.1", name: "1.2.1" }; + const path = ["1", "1.2", "1.2.1"]; + + ServiceUtils.nestedTraverse_vNext(nodeTree, 0, path, nodeToBeAdded, null, "/"); + expect(nodeTree[0].children[1].children[0].node).toEqual(nodeToBeAdded); + }); + + it("should combine the path for missing nodes and use as the added node name given an invalid path", () => { + const nodeToBeAdded: FakeObject = { id: "blank", name: "blank" }; + const path = ["3", "3.1", "3.1.1"]; + + ServiceUtils.nestedTraverse_vNext(nodeTree, 0, path, nodeToBeAdded, null, "/"); + expect(nodeTree[2].children[0].node.name).toEqual("3.1/3.1.1"); + }); + }); + describe("getTreeNodeObject", () => { it("should return a matching node given a single tree branch and a valid id", () => { const id = "1.1.1"; diff --git a/libs/common/src/vault/service-utils.ts b/libs/common/src/vault/service-utils.ts index 5fbc550d6af..96ae406fae4 100644 --- a/libs/common/src/vault/service-utils.ts +++ b/libs/common/src/vault/service-utils.ts @@ -3,15 +3,6 @@ import { ITreeNodeObject, TreeNode } from "./models/domain/tree-node"; export class ServiceUtils { - /** - * Recursively adds a node to nodeTree - * @param {TreeNode[]} nodeTree - An array of TreeNodes that the node will be added to - * @param {number} partIndex - Index of the `parts` array that is being processed - * @param {string[]} parts - Array of strings that represent the path to the `obj` node - * @param {ITreeNodeObject} obj - The node to be added to the tree - * @param {ITreeNodeObject} parent - The parent node of the `obj` node - * @param {string} delimiter - The delimiter used to split the path string, will be used to combine the path for missing nodes - */ static nestedTraverse( nodeTree: TreeNode[], partIndex: number, @@ -70,11 +61,75 @@ export class ServiceUtils { } } + /** + * Recursively adds a node to nodeTree + * @param {TreeNode[]} nodeTree - An array of TreeNodes that the node will be added to + * @param {number} partIndex - Index of the `parts` array that is being processed + * @param {string[]} parts - Array of strings that represent the path to the `obj` node + * @param {ITreeNodeObject} obj - The node to be added to the tree + * @param {ITreeNodeObject} parent - The parent node of the `obj` node + * @param {string} delimiter - The delimiter used to split the path string, will be used to combine the path for missing nodes + */ + static nestedTraverse_vNext( + nodeTree: TreeNode[], + partIndex: number, + parts: string[], + obj: ITreeNodeObject, + parent: TreeNode | undefined, + delimiter: string, + ) { + if (parts.length <= partIndex) { + return; + } + + // 'end' indicates we've traversed as far as we can based on the object name + const end: boolean = partIndex === parts.length - 1; + const partName: string = parts[partIndex]; + + // If we're at the end, just add the node - it doesn't matter what else is here + if (end) { + nodeTree.push(new TreeNode(obj, parent, partName)); + return; + } + + // Get matching nodes at this level by name + // NOTE: this is effectively a loop so we only want to do it once + const matchingNodes = nodeTree.filter((n) => n.node.name === partName); + + // If there are no matching nodes... + if (matchingNodes.length === 0) { + // And we're not at the end of the path (because we didn't trigger the early return above), + // combine the current name with the next name. + // 1, *1.2, 1.2.1 becomes + // 1, *1.2/1.2.1 + const newPartName = partName + delimiter + parts[partIndex + 1]; + ServiceUtils.nestedTraverse_vNext( + nodeTree, + 0, + [newPartName, ...parts.slice(partIndex + 2)], + obj, + parent, + delimiter, + ); + } else { + // There is a node here with the same name, descend into it + ServiceUtils.nestedTraverse_vNext( + matchingNodes[0].children, + partIndex + 1, + parts, + obj, + matchingNodes[0], + delimiter, + ); + return; + } + } + /** * Searches a tree for a node with a matching `id` - * @param {TreeNode} nodeTree - A single TreeNode branch that will be searched + * @param {TreeNode} nodeTree - A single TreeNode branch that will be searched * @param {string} id - The id of the node to be found - * @returns {TreeNode} The node with a matching `id` + * @returns {TreeNode} The node with a matching `id` */ static getTreeNodeObject( nodeTree: TreeNode, @@ -96,9 +151,9 @@ export class ServiceUtils { /** * Searches an array of tree nodes for a node with a matching `id` - * @param {TreeNode} nodeTree - An array of TreeNode branches that will be searched + * @param {TreeNode} nodeTree - An array of TreeNode branches that will be searched * @param {string} id - The id of the node to be found - * @returns {TreeNode} The node with a matching `id` + * @returns {TreeNode} The node with a matching `id` */ static getTreeNodeObjectFromList( nodeTree: TreeNode[], From 8966b4fb50c7dff9a02cfbb22f9cfa8b26ee1553 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Thu, 29 May 2025 20:48:03 +0200 Subject: [PATCH 14/14] Fix flatpak autostart disabling (#14920) --- apps/desktop/src/main/messaging.main.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/apps/desktop/src/main/messaging.main.ts b/apps/desktop/src/main/messaging.main.ts index 556fa293108..bc8d9ae4685 100644 --- a/apps/desktop/src/main/messaging.main.ts +++ b/apps/desktop/src/main/messaging.main.ts @@ -10,7 +10,7 @@ import { autostart } from "@bitwarden/desktop-napi"; import { Main } from "../main"; import { DesktopSettingsService } from "../platform/services/desktop-settings.service"; -import { isFlatpak } from "../utils"; +import { isFlatpak, isLinux, isSnapStore } from "../utils"; import { MenuUpdateRequest } from "./menu/menu.updater"; @@ -26,8 +26,11 @@ export class MessagingMain { async init() { this.scheduleNextSync(); - if (process.platform === "linux") { - await this.desktopSettingsService.setOpenAtLogin(fs.existsSync(this.linuxStartupFile())); + if (isLinux()) { + // Flatpak and snap don't have access to or use the startup file. On flatpak, the autostart portal is used + if (!isFlatpak() && !isSnapStore()) { + await this.desktopSettingsService.setOpenAtLogin(fs.existsSync(this.linuxStartupFile())); + } } else { const loginSettings = app.getLoginItemSettings(); await this.desktopSettingsService.setOpenAtLogin(loginSettings.openAtLogin);