Merge branch 'main' into autofill/pm-5189-fix-issues-present-with-inline-menu-rendering-in-iframes

libs/common/src/auth/models/domain/kdf-config.ts

@@ -1,12 +1,7 @@
 import { Jsonify } from "type-fest";
 
-import {
-  ARGON2_ITERATIONS,
-  ARGON2_MEMORY,
-  ARGON2_PARALLELISM,
-  KdfType,
-  PBKDF2_ITERATIONS,
-} from "../../../platform/enums/kdf-type.enum";
+import { KdfType } from "../../../platform/enums/kdf-type.enum";
+import { RangeWithDefault } from "../../../platform/misc/range-with-default";
 
 /**
  * Represents a type safe KDF configuration.
@@ -17,11 +12,12 @@ export type KdfConfig = PBKDF2KdfConfig | Argon2KdfConfig;
  * Password-Based Key Derivation Function 2 (PBKDF2) KDF configuration.
  */
 export class PBKDF2KdfConfig {
+  static ITERATIONS = new RangeWithDefault(600_000, 2_000_000, 600_000);
   kdfType: KdfType.PBKDF2_SHA256 = KdfType.PBKDF2_SHA256;
   iterations: number;
 
   constructor(iterations?: number) {
-    this.iterations = iterations ?? PBKDF2_ITERATIONS.defaultValue;
+    this.iterations = iterations ?? PBKDF2KdfConfig.ITERATIONS.defaultValue;
   }
 
   /**
@@ -29,9 +25,9 @@ export class PBKDF2KdfConfig {
    * A Valid PBKDF2 KDF configuration has KDF iterations between the 600_000 and 2_000_000.
    */
   validateKdfConfig(): void {
-    if (!PBKDF2_ITERATIONS.inRange(this.iterations)) {
+    if (!PBKDF2KdfConfig.ITERATIONS.inRange(this.iterations)) {
       throw new Error(
-        `PBKDF2 iterations must be between ${PBKDF2_ITERATIONS.min} and ${PBKDF2_ITERATIONS.max}`,
+        `PBKDF2 iterations must be between ${PBKDF2KdfConfig.ITERATIONS.min} and ${PBKDF2KdfConfig.ITERATIONS.max}`,
       );
     }
   }
@@ -45,15 +41,18 @@ export class PBKDF2KdfConfig {
  * Argon2 KDF configuration.
  */
 export class Argon2KdfConfig {
+  static MEMORY = new RangeWithDefault(16, 1024, 64);
+  static PARALLELISM = new RangeWithDefault(1, 16, 4);
+  static ITERATIONS = new RangeWithDefault(2, 10, 3);
   kdfType: KdfType.Argon2id = KdfType.Argon2id;
   iterations: number;
   memory: number;
   parallelism: number;
 
   constructor(iterations?: number, memory?: number, parallelism?: number) {
-    this.iterations = iterations ?? ARGON2_ITERATIONS.defaultValue;
-    this.memory = memory ?? ARGON2_MEMORY.defaultValue;
-    this.parallelism = parallelism ?? ARGON2_PARALLELISM.defaultValue;
+    this.iterations = iterations ?? Argon2KdfConfig.ITERATIONS.defaultValue;
+    this.memory = memory ?? Argon2KdfConfig.MEMORY.defaultValue;
+    this.parallelism = parallelism ?? Argon2KdfConfig.PARALLELISM.defaultValue;
   }
 
   /**
@@ -61,21 +60,21 @@ export class Argon2KdfConfig {
    * A Valid Argon2 KDF configuration has iterations between 2 and 10, memory between 16mb and 1024mb, and parallelism between 1 and 16.
    */
   validateKdfConfig(): void {
-    if (!ARGON2_ITERATIONS.inRange(this.iterations)) {
+    if (!Argon2KdfConfig.ITERATIONS.inRange(this.iterations)) {
       throw new Error(
-        `Argon2 iterations must be between ${ARGON2_ITERATIONS.min} and ${ARGON2_ITERATIONS.max}`,
+        `Argon2 iterations must be between ${Argon2KdfConfig.ITERATIONS.min} and ${Argon2KdfConfig.ITERATIONS.max}`,
       );
     }
 
-    if (!ARGON2_MEMORY.inRange(this.memory)) {
+    if (!Argon2KdfConfig.MEMORY.inRange(this.memory)) {
       throw new Error(
-        `Argon2 memory must be between ${ARGON2_MEMORY.min}mb and ${ARGON2_MEMORY.max}mb`,
+        `Argon2 memory must be between ${Argon2KdfConfig.MEMORY.min}mb and ${Argon2KdfConfig.MEMORY.max}mb`,
       );
     }
 
-    if (!ARGON2_PARALLELISM.inRange(this.parallelism)) {
+    if (!Argon2KdfConfig.PARALLELISM.inRange(this.parallelism)) {
       throw new Error(
-        `Argon2 parallelism must be between ${ARGON2_PARALLELISM.min} and ${ARGON2_PARALLELISM.max}.`,
+        `Argon2 parallelism must be between ${Argon2KdfConfig.PARALLELISM.min} and ${Argon2KdfConfig.PARALLELISM.max}.`,
       );
     }
   }
@@ -84,3 +83,5 @@ export class Argon2KdfConfig {
     return new Argon2KdfConfig(json.iterations, json.memory, json.parallelism);
   }
 }
+
+export const DEFAULT_KDF_CONFIG = new PBKDF2KdfConfig(PBKDF2KdfConfig.ITERATIONS.defaultValue);

libs/common/src/auth/models/request/update-two-factor-duo.request.ts

@@ -1,7 +1,7 @@
 import { SecretVerificationRequest } from "./secret-verification.request";
 
 export class UpdateTwoFactorDuoRequest extends SecretVerificationRequest {
-  integrationKey: string;
-  secretKey: string;
+  clientId: string;
+  clientSecret: string;
   host: string;
 }

libs/common/src/auth/models/response/two-factor-duo.response.ts

@@ -3,14 +3,14 @@ import { BaseResponse } from "../../../models/response/base.response";
 export class TwoFactorDuoResponse extends BaseResponse {
   enabled: boolean;
   host: string;
-  secretKey: string;
-  integrationKey: string;
+  clientSecret: string;
+  clientId: string;
 
   constructor(response: any) {
     super(response);
     this.enabled = this.getResponseProperty("Enabled");
     this.host = this.getResponseProperty("Host");
-    this.secretKey = this.getResponseProperty("SecretKey");
-    this.integrationKey = this.getResponseProperty("IntegrationKey");
+    this.clientSecret = this.getResponseProperty("ClientSecret");
+    this.clientId = this.getResponseProperty("ClientId");
   }
 }

libs/common/src/auth/services/kdf-config.service.spec.ts

@@ -1,10 +1,4 @@
 import { FakeAccountService, FakeStateProvider, mockAccountServiceWith } from "../../../spec";
-import {
-  ARGON2_ITERATIONS,
-  ARGON2_MEMORY,
-  ARGON2_PARALLELISM,
-  PBKDF2_ITERATIONS,
-} from "../../platform/enums/kdf-type.enum";
 import { Utils } from "../../platform/misc/utils";
 import { UserId } from "../../types/guid";
 import { Argon2KdfConfig, PBKDF2KdfConfig } from "../models/domain/kdf-config";
@@ -77,28 +71,28 @@ describe("KdfConfigService", () => {
   it("validateKdfConfig(): should throw an error for invalid PBKDF2 iterations", () => {
     const kdfConfig: PBKDF2KdfConfig = new PBKDF2KdfConfig(100);
     expect(() => kdfConfig.validateKdfConfig()).toThrow(
-      `PBKDF2 iterations must be between ${PBKDF2_ITERATIONS.min} and ${PBKDF2_ITERATIONS.max}`,
+      `PBKDF2 iterations must be between ${PBKDF2KdfConfig.ITERATIONS.min} and ${PBKDF2KdfConfig.ITERATIONS.max}`,
     );
   });
 
   it("validateKdfConfig(): should throw an error for invalid Argon2 iterations", () => {
     const kdfConfig: Argon2KdfConfig = new Argon2KdfConfig(11, 64, 4);
     expect(() => kdfConfig.validateKdfConfig()).toThrow(
-      `Argon2 iterations must be between ${ARGON2_ITERATIONS.min} and ${ARGON2_ITERATIONS.max}`,
+      `Argon2 iterations must be between ${Argon2KdfConfig.ITERATIONS.min} and ${Argon2KdfConfig.ITERATIONS.max}`,
     );
   });
 
   it("validateKdfConfig(): should throw an error for invalid Argon2 memory", () => {
     const kdfConfig: Argon2KdfConfig = new Argon2KdfConfig(3, 1025, 4);
     expect(() => kdfConfig.validateKdfConfig()).toThrow(
-      `Argon2 memory must be between ${ARGON2_MEMORY.min}mb and ${ARGON2_MEMORY.max}mb`,
+      `Argon2 memory must be between ${Argon2KdfConfig.MEMORY.min}mb and ${Argon2KdfConfig.MEMORY.max}mb`,
     );
   });
 
   it("validateKdfConfig(): should throw an error for invalid Argon2 parallelism", () => {
     const kdfConfig: Argon2KdfConfig = new Argon2KdfConfig(3, 64, 17);
     expect(() => kdfConfig.validateKdfConfig()).toThrow(
-      `Argon2 parallelism must be between ${ARGON2_PARALLELISM.min} and ${ARGON2_PARALLELISM.max}`,
+      `Argon2 parallelism must be between ${Argon2KdfConfig.PARALLELISM.min} and ${Argon2KdfConfig.PARALLELISM.max}`,
     );
   });
 });

libs/common/src/platform/enums/kdf-type.enum.ts

@@ -1,15 +1,4 @@
-import { PBKDF2KdfConfig } from "../../auth/models/domain/kdf-config";
-import { RangeWithDefault } from "../misc/range-with-default";
-
 export enum KdfType {
   PBKDF2_SHA256 = 0,
   Argon2id = 1,
 }
-
-export const ARGON2_MEMORY = new RangeWithDefault(16, 1024, 64);
-export const ARGON2_PARALLELISM = new RangeWithDefault(1, 16, 4);
-export const ARGON2_ITERATIONS = new RangeWithDefault(2, 10, 3);
-
-export const DEFAULT_KDF_TYPE = KdfType.PBKDF2_SHA256;
-export const PBKDF2_ITERATIONS = new RangeWithDefault(600_000, 2_000_000, 600_000);
-export const DEFAULT_KDF_CONFIG = new PBKDF2KdfConfig(PBKDF2_ITERATIONS.defaultValue);

libs/common/src/platform/services/fido2/fido2-client.service.spec.ts

@@ -19,6 +19,7 @@ import {
 } from "../../abstractions/fido2/fido2-client.service.abstraction";
 import { Utils } from "../../misc/utils";
 
+import * as DomainUtils from "./domain-utils";
 import { Fido2AuthenticatorService } from "./fido2-authenticator.service";
 import { Fido2ClientService } from "./fido2-client.service";
 import { Fido2Utils } from "./fido2-utils";
@@ -36,6 +37,7 @@ describe("FidoAuthenticatorService", () => {
   let domainSettingsService: MockProxy<DomainSettingsService>;
   let client!: Fido2ClientService;
   let tab!: chrome.tabs.Tab;
+  let isValidRpId!: jest.SpyInstance;
 
   beforeEach(async () => {
     authenticator = mock<Fido2AuthenticatorService>();
@@ -44,6 +46,8 @@ describe("FidoAuthenticatorService", () => {
     vaultSettingsService = mock<VaultSettingsService>();
     domainSettingsService = mock<DomainSettingsService>();
 
+    isValidRpId = jest.spyOn(DomainUtils, "isValidRpId");
+
     client = new Fido2ClientService(
       authenticator,
       configService,
@@ -58,6 +62,10 @@ describe("FidoAuthenticatorService", () => {
     tab = { id: 123, windowId: 456 } as chrome.tabs.Tab;
   });
 
+  afterEach(() => {
+    isValidRpId.mockRestore();
+  });
+
   describe("createCredential", () => {
     describe("input parameters validation", () => {
       // Spec: If sameOriginWithAncestors is false, return a "NotAllowedError" DOMException.
@@ -113,6 +121,7 @@ describe("FidoAuthenticatorService", () => {
       });
 
       // Spec: If options.rp.id is not a registrable domain suffix of and is not equal to effectiveDomain, return a DOMException whose name is "SecurityError", and terminate this algorithm.
+      // This is actually checked by `isValidRpId` function, but we'll test it here as well
       it("should throw error if rp.id is not valid for this origin", async () => {
         const params = createParams({
           origin: "https://passwordless.dev",
@@ -126,6 +135,20 @@ describe("FidoAuthenticatorService", () => {
         await rejects.toBeInstanceOf(DOMException);
       });
 
+      // Sanity check to make sure that we use `isValidRpId` to validate the rp.id
+      it("should throw if isValidRpId returns false", async () => {
+        const params = createParams();
+        authenticator.makeCredential.mockResolvedValue(createAuthenticatorMakeResult());
+        // `params` actually has a valid rp.id, but we're mocking the function to return false
+        isValidRpId.mockReturnValue(false);
+
+        const result = async () => await client.createCredential(params, tab);
+
+        const rejects = expect(result).rejects;
+        await rejects.toMatchObject({ name: "SecurityError" });
+        await rejects.toBeInstanceOf(DOMException);
+      });
+
       it("should fallback if origin hostname is found in neverDomains", async () => {
         const params = createParams({
           origin: "https://bitwarden.com",
@@ -151,6 +174,16 @@ describe("FidoAuthenticatorService", () => {
         await rejects.toBeInstanceOf(DOMException);
       });
 
+      it("should not throw error if localhost is http", async () => {
+        const params = createParams({
+          origin: "http://localhost",
+          rp: { id: undefined, name: "localhost" },
+        });
+        authenticator.makeCredential.mockResolvedValue(createAuthenticatorMakeResult());
+
+        await client.createCredential(params, tab);
+      });
+
       // Spec: If credTypesAndPubKeyAlgs is empty, return a DOMException whose name is "NotSupportedError", and terminate this algorithm.
       it("should throw error if no support key algorithms were found", async () => {
         const params = createParams({
@@ -360,6 +393,7 @@ describe("FidoAuthenticatorService", () => {
       });
 
       // Spec: If options.rp.id is not a registrable domain suffix of and is not equal to effectiveDomain, return a DOMException whose name is "SecurityError", and terminate this algorithm.
+      // This is actually checked by `isValidRpId` function, but we'll test it here as well
       it("should throw error if rp.id is not valid for this origin", async () => {
         const params = createParams({
           origin: "https://passwordless.dev",
@@ -373,6 +407,20 @@ describe("FidoAuthenticatorService", () => {
         await rejects.toBeInstanceOf(DOMException);
       });
 
+      // Sanity check to make sure that we use `isValidRpId` to validate the rp.id
+      it("should throw if isValidRpId returns false", async () => {
+        const params = createParams();
+        authenticator.getAssertion.mockResolvedValue(createAuthenticatorAssertResult());
+        // `params` actually has a valid rp.id, but we're mocking the function to return false
+        isValidRpId.mockReturnValue(false);
+
+        const result = async () => await client.assertCredential(params, tab);
+
+        const rejects = expect(result).rejects;
+        await rejects.toMatchObject({ name: "SecurityError" });
+        await rejects.toBeInstanceOf(DOMException);
+      });
+
       it("should fallback if origin hostname is found in neverDomains", async () => {
         const params = createParams({
           origin: "https://bitwarden.com",
@@ -506,6 +554,16 @@ describe("FidoAuthenticatorService", () => {
           expect.anything(),
         );
       });
+
+      it("should not throw error if localhost is http", async () => {
+        const params = createParams({
+          origin: "http://localhost",
+        });
+        params.rpId = undefined;
+        authenticator.getAssertion.mockResolvedValue(createAuthenticatorAssertResult());
+
+        await client.assertCredential(params, tab);
+      });
     });
 
     describe("assert discoverable credential", () => {

libs/common/src/platform/services/fido2/fido2-client.service.ts

@@ -103,7 +103,10 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction {
     }
 
     params.rp.id = params.rp.id ?? parsedOrigin.hostname;
-    if (parsedOrigin.hostname == undefined || !params.origin.startsWith("https://")) {
+    if (
+      parsedOrigin.hostname == undefined ||
+      (!params.origin.startsWith("https://") && parsedOrigin.hostname !== "localhost")
+    ) {
       this.logService?.warning(`[Fido2Client] Invalid https origin: ${params.origin}`);
       throw new DOMException("'origin' is not a valid https origin", "SecurityError");
     }
@@ -238,7 +241,10 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction {
 
     params.rpId = params.rpId ?? parsedOrigin.hostname;
 
-    if (parsedOrigin.hostname == undefined || !params.origin.startsWith("https://")) {
+    if (
+      parsedOrigin.hostname == undefined ||
+      (!params.origin.startsWith("https://") && parsedOrigin.hostname !== "localhost")
+    ) {
       this.logService?.warning(`[Fido2Client] Invalid https origin: ${params.origin}`);
       throw new DOMException("'origin' is not a valid https origin", "SecurityError");
     }

libs/common/src/platform/services/key-generation.service.ts

@@ -1,14 +1,8 @@
-import { KdfConfig } from "../../auth/models/domain/kdf-config";
+import { Argon2KdfConfig, KdfConfig, PBKDF2KdfConfig } from "../../auth/models/domain/kdf-config";
 import { CsprngArray } from "../../types/csprng";
 import { CryptoFunctionService } from "../abstractions/crypto-function.service";
 import { KeyGenerationService as KeyGenerationServiceAbstraction } from "../abstractions/key-generation.service";
-import {
-  ARGON2_ITERATIONS,
-  ARGON2_MEMORY,
-  ARGON2_PARALLELISM,
-  KdfType,
-  PBKDF2_ITERATIONS,
-} from "../enums";
+import { KdfType } from "../enums";
 import { Utils } from "../misc/utils";
 import { SymmetricCryptoKey } from "../models/domain/symmetric-crypto-key";
 
@@ -51,21 +45,21 @@ export class KeyGenerationService implements KeyGenerationServiceAbstraction {
     let key: Uint8Array = null;
     if (kdfConfig.kdfType == null || kdfConfig.kdfType === KdfType.PBKDF2_SHA256) {
       if (kdfConfig.iterations == null) {
-        kdfConfig.iterations = PBKDF2_ITERATIONS.defaultValue;
+        kdfConfig.iterations = PBKDF2KdfConfig.ITERATIONS.defaultValue;
       }
 
       key = await this.cryptoFunctionService.pbkdf2(password, salt, "sha256", kdfConfig.iterations);
     } else if (kdfConfig.kdfType == KdfType.Argon2id) {
       if (kdfConfig.iterations == null) {
-        kdfConfig.iterations = ARGON2_ITERATIONS.defaultValue;
+        kdfConfig.iterations = Argon2KdfConfig.ITERATIONS.defaultValue;
       }
 
       if (kdfConfig.memory == null) {
-        kdfConfig.memory = ARGON2_MEMORY.defaultValue;
+        kdfConfig.memory = Argon2KdfConfig.MEMORY.defaultValue;
       }
 
       if (kdfConfig.parallelism == null) {
-        kdfConfig.parallelism = ARGON2_PARALLELISM.defaultValue;
+        kdfConfig.parallelism = Argon2KdfConfig.PARALLELISM.defaultValue;
       }
 
       const saltHash = await this.cryptoFunctionService.hash(salt, "sha256");