From c14ab487ac694b4cc3e2107d3854e11e07db73d2 Mon Sep 17 00:00:00 2001 From: SmithThe4th Date: Wed, 24 May 2023 08:06:41 -0400 Subject: [PATCH] [PM-1500] Add feature flag to enable passkeys (#5406) * Added launch darkly feature flag to passkeys implementation * fixed linter * Updated fido2 client service test to accomodate feature flag * Updated fido2client service to include unit test for feature flag * Renamed enable pass keys to fido2 vault credentials, added unit test when feature flag is not enabled * fixed failing Login domain test case --- .../browser/src/background/main.background.ts | 4 +++ apps/browser/src/browser/browserApi.ts | 1 - libs/common/src/enums/feature-flag.enum.ts | 1 + .../services/fido2-client.service.spec.ts | 27 ++++++++++++++++++- .../fido2/services/fido2-client.service.ts | 26 +++++++++++++++++- .../src/vault/models/domain/login.spec.ts | 26 ++++++++++++++++++ 6 files changed, 82 insertions(+), 3 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 766bf3faeee..896e9fa3e5c 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -56,6 +56,7 @@ import { AvatarUpdateService } from "@bitwarden/common/services/account/avatar-u import { ApiService } from "@bitwarden/common/services/api.service"; import { AppIdService } from "@bitwarden/common/services/appId.service"; import { AuditService } from "@bitwarden/common/services/audit.service"; +import { ConfigApiService } from "@bitwarden/common/services/config/config-api.service"; import { ConfigService } from "@bitwarden/common/services/config/config.service"; import { ConsoleLogService } from "@bitwarden/common/services/consoleLog.service"; import { ContainerService } from "@bitwarden/common/services/container.service"; @@ -509,6 +510,8 @@ export default class MainBackground { this.userVerificationApiService ); + this.configApiService = new ConfigApiService(this.apiService); + this.configService = new ConfigService( this.stateService, this.configApiService, @@ -524,6 +527,7 @@ export default class MainBackground { ); this.fido2ClientService = new Fido2ClientService( this.fido2AuthenticatorService, + this.configService, this.logService ); diff --git a/apps/browser/src/browser/browserApi.ts b/apps/browser/src/browser/browserApi.ts index 05f36ea35c9..cb7c21c1b14 100644 --- a/apps/browser/src/browser/browserApi.ts +++ b/apps/browser/src/browser/browserApi.ts @@ -2,7 +2,6 @@ import { Observable } from "rxjs"; import { DeviceType } from "@bitwarden/common/enums/device-type.enum"; - import BrowserPlatformUtilsService from "../services/browserPlatformUtils.service"; import { TabMessage } from "../types/tab-messages"; diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index e8a05911b9f..f06bb2f1a3d 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -1,5 +1,6 @@ export enum FeatureFlag { DisplayEuEnvironmentFlag = "display-eu-environment", DisplayLowKdfIterationWarningFlag = "display-kdf-iteration-warning", + Fido2VaultCredentials = "fido2-vault-credentials", TrustedDeviceEncryption = "trusted-device-encryption", } diff --git a/libs/common/src/fido2/services/fido2-client.service.spec.ts b/libs/common/src/fido2/services/fido2-client.service.spec.ts index 5eab8e87685..306e8af0407 100644 --- a/libs/common/src/fido2/services/fido2-client.service.spec.ts +++ b/libs/common/src/fido2/services/fido2-client.service.spec.ts @@ -1,5 +1,6 @@ import { mock, MockProxy } from "jest-mock-extended"; +import { ConfigServiceAbstraction } from "../../abstractions/config/config.service.abstraction"; import { Utils } from "../../misc/utils"; import { Fido2AutenticatorError, @@ -10,6 +11,7 @@ import { import { AssertCredentialParams, CreateCredentialParams, + FallbackRequestedError, } from "../abstractions/fido2-client.service.abstraction"; import { Fido2Utils } from "../abstractions/fido2-utils"; @@ -20,11 +22,14 @@ const RpId = "bitwarden.com"; describe("FidoAuthenticatorService", () => { let authenticator!: MockProxy; + let configService!: MockProxy; let client!: Fido2ClientService; beforeEach(async () => { authenticator = mock(); - client = new Fido2ClientService(authenticator); + configService = mock(); + client = new Fido2ClientService(authenticator, configService); + configService.getFeatureFlagBool.mockResolvedValue(true); }); describe("createCredential", () => { @@ -188,6 +193,16 @@ describe("FidoAuthenticatorService", () => { await rejects.toMatchObject({ name: "NotAllowedError" }); await rejects.toBeInstanceOf(DOMException); }); + + it("should throw FallbackRequestedError if feature flag is not enabled", async () => { + const params = createParams(); + configService.getFeatureFlagBool.mockResolvedValue(false); + + const result = async () => await client.createCredential(params); + + const rejects = expect(result).rejects; + await rejects.toThrow(FallbackRequestedError); + }); }); function createParams(params: Partial = {}): CreateCredentialParams { @@ -315,6 +330,16 @@ describe("FidoAuthenticatorService", () => { await rejects.toMatchObject({ name: "NotAllowedError" }); await rejects.toBeInstanceOf(DOMException); }); + + it("should throw FallbackRequestedError if feature flag is not enabled", async () => { + const params = createParams(); + configService.getFeatureFlagBool.mockResolvedValue(false); + + const result = async () => await client.assertCredential(params); + + const rejects = expect(result).rejects; + await rejects.toThrow(FallbackRequestedError); + }); }); describe("assert non-discoverable credential", () => { diff --git a/libs/common/src/fido2/services/fido2-client.service.ts b/libs/common/src/fido2/services/fido2-client.service.ts index 084a8b03cca..e7e76eade58 100644 --- a/libs/common/src/fido2/services/fido2-client.service.ts +++ b/libs/common/src/fido2/services/fido2-client.service.ts @@ -1,6 +1,8 @@ import { parse } from "tldts"; +import { ConfigServiceAbstraction } from "../../abstractions/config/config.service.abstraction"; import { LogService } from "../../abstractions/log.service"; +import { FeatureFlag } from "../../enums/feature-flag.enum"; import { Utils } from "../../misc/utils"; import { Fido2AutenticatorError, @@ -26,12 +28,25 @@ import { Fido2Utils } from "../abstractions/fido2-utils"; import { isValidRpId } from "./domain-utils"; export class Fido2ClientService implements Fido2ClientServiceAbstraction { - constructor(private authenticator: Fido2AuthenticatorService, private logService?: LogService) {} + constructor( + private authenticator: Fido2AuthenticatorService, + private configService: ConfigServiceAbstraction, + private logService?: LogService + ) {} async createCredential( params: CreateCredentialParams, abortController = new AbortController() ): Promise { + const enableFido2VaultCredentials = await this.configService.getFeatureFlagBool( + FeatureFlag.Fido2VaultCredentials + ); + + if (!enableFido2VaultCredentials) { + this.logService?.warning(`[Fido2Client] Fido2VaultCredential is not enabled`); + throw new FallbackRequestedError(); + } + if (!params.sameOriginWithAncestors) { this.logService?.warning( `[Fido2Client] Invalid 'sameOriginWithAncestors' value: ${params.sameOriginWithAncestors}` @@ -176,6 +191,15 @@ export class Fido2ClientService implements Fido2ClientServiceAbstraction { params: AssertCredentialParams, abortController = new AbortController() ): Promise { + const enableFido2VaultCredentials = await this.configService.getFeatureFlagBool( + FeatureFlag.Fido2VaultCredentials + ); + + if (!enableFido2VaultCredentials) { + this.logService?.warning(`[Fido2Client] Fido2VaultCredential is not enabled`); + throw new FallbackRequestedError(); + } + const { domain: effectiveDomain } = parse(params.origin, { allowPrivateDomains: true }); if (effectiveDomain == undefined) { this.logService?.warning(`[Fido2Client] Invalid origin: ${params.origin}`); diff --git a/libs/common/src/vault/models/domain/login.spec.ts b/libs/common/src/vault/models/domain/login.spec.ts index 4cc774bdc3e..cc1474fc470 100644 --- a/libs/common/src/vault/models/domain/login.spec.ts +++ b/libs/common/src/vault/models/domain/login.spec.ts @@ -112,6 +112,19 @@ describe("Login DTO", () => { password: "myPassword", passwordRevisionDate: passwordRevisionDate.toISOString(), totp: "myTotp", + fido2Key: { + nonDiscoverableId: "keyId", + keyType: "keyType", + keyAlgorithm: "keyAlgorithm", + keyCurve: "keyCurve", + keyValue: "keyValue", + rpId: "rpId", + userHandle: "userHandle", + counter: "counter", + rpName: "rpName", + userName: "userName", + origin: "origin", + }, }); expect(actual).toEqual({ @@ -120,6 +133,19 @@ describe("Login DTO", () => { password: "myPassword_fromJSON", passwordRevisionDate: passwordRevisionDate, totp: "myTotp_fromJSON", + fido2Key: { + nonDiscoverableId: "keyId_fromJSON", + keyType: "keyType_fromJSON", + keyAlgorithm: "keyAlgorithm_fromJSON", + keyCurve: "keyCurve_fromJSON", + keyValue: "keyValue_fromJSON", + rpId: "rpId_fromJSON", + userHandle: "userHandle_fromJSON", + counter: "counter_fromJSON", + rpName: "rpName_fromJSON", + userName: "userName_fromJSON", + origin: "origin_fromJSON", + }, }); expect(actual).toBeInstanceOf(Login); });