Merge branch 'main' into autofill/pm-21845

.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"],
 }

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<string> {
+    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;

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<CollectionView | CollectionAdminView>[] {
+  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<CollectionView | CollectionAdminView>[] = [];
+  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>[],
 ): CollectionAdminView[];

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<CollectionAdminView>[],
+      ),
     );
 
     const collections$ = combineLatest([

apps/web/src/app/admin-console/organizations/members/members.component.ts

@@ -110,8 +110,10 @@ export class MembersComponent extends BaseMembersComponent<OrganizationUserView>
   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<OrganizationUserView>
           );
 
           this.orgIsOnSecretsManagerStandalone = billingMetadata.isOnSecretsManagerStandalone;
+          this.organizationUsersCount = billingMetadata.organizationOccupiedSeats;
 
           await this.load();
 

apps/web/src/app/auth/settings/security/password-settings/password-settings.component.html

@@ -1,6 +1,4 @@
-<div class="tabbed-header">
-  <h1>{{ "changeMasterPassword" | i18n }}</h1>
-</div>
+<h1 class="tw-mt-6 tw-mb-2 tw-pb-2.5">{{ "changeMasterPassword" | i18n }}</h1>
 
 <div class="tw-max-w-lg tw-mb-12">
   <bit-callout type="warning">{{ "loggedOutWarning" | i18n }}</bit-callout>

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$

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,

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);
   });
 });

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);
       }

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 {

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,

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";

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<ITreeNodeObject>[]} 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<ITreeNodeObject>[],
     partIndex: number,
@@ -70,11 +61,75 @@ export class ServiceUtils {
     }
   }
 
+  /**
+   * Recursively adds a node to nodeTree
+   * @param {TreeNode<ITreeNodeObject>[]} 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<ITreeNodeObject>[],
+    partIndex: number,
+    parts: string[],
+    obj: ITreeNodeObject,
+    parent: TreeNode<ITreeNodeObject> | 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<T>} nodeTree - A single TreeNode branch that will be searched
+   * @param {TreeNode<T extends ITreeNodeObject>} nodeTree - A single TreeNode branch that will be searched
    * @param {string} id - The id of the node to be found
-   * @returns {TreeNode<T>} The node with a matching `id`
+   * @returns {TreeNode<T extends ITreeNodeObject>} The node with a matching `id`
    */
   static getTreeNodeObject<T extends ITreeNodeObject>(
     nodeTree: TreeNode<T>,
@@ -96,9 +151,9 @@ export class ServiceUtils {
 
   /**
    * Searches an array of tree nodes for a node with a matching `id`
-   * @param {TreeNode<T>} nodeTree - An array of TreeNode branches that will be searched
+   * @param {TreeNode<T extends ITreeNodeObject>} nodeTree - An array of TreeNode branches that will be searched
    * @param {string} id - The id of the node to be found
-   * @returns {TreeNode<T>} The node with a matching `id`
+   * @returns {TreeNode<T extends ITreeNodeObject>} The node with a matching `id`
    */
   static getTreeNodeObjectFromList<T extends ITreeNodeObject>(
     nodeTree: TreeNode<T>[],