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"], } 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; 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/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/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 }} 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/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); } 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 { 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[],