From bdc951194eb08dc7e62b4182ab37b4f3f1ad8047 Mon Sep 17 00:00:00 2001 From: Todd Martin <106564991+trmartin4@users.noreply.github.com> Date: Tue, 13 Feb 2024 11:15:16 -0500 Subject: [PATCH] [PM-5800] Remove passwordless-login feature flag (#7626) * Removed passwordless-login feature flag * Removed conditional on login component. * Added back reference accidentally deleted. * Fixed initialization of the service in tests. * Removed unused private variable. * Updated DI to remove configService * Undid changes to workspace file. * Undid all changes to workspace file * Undid merge changes to collection dialog * Linting --- .../src/app/auth/login/login.component.html | 5 +-- .../settings/change-password.component.html | 4 +- .../settings/change-password.component.ts | 8 ---- .../src/auth/components/login.component.ts | 5 +-- .../src/services/jslib-services.module.ts | 1 - .../webauthn-login.service.abstraction.ts | 7 ---- .../webauthn-login.service.spec.ts | 39 ++++--------------- .../webauthn-login/webauthn-login.service.ts | 8 ---- libs/common/src/enums/feature-flag.enum.ts | 1 - 9 files changed, 10 insertions(+), 68 deletions(-) diff --git a/apps/web/src/app/auth/login/login.component.html b/apps/web/src/app/auth/login/login.component.html index 72315b1a4d..5c68058a3c 100644 --- a/apps/web/src/app/auth/login/login.component.html +++ b/apps/web/src/app/auth/login/login.component.html @@ -51,10 +51,7 @@ -
+

{{ "or" | i18n }}

- + diff --git a/apps/web/src/app/auth/settings/change-password.component.ts b/apps/web/src/app/auth/settings/change-password.component.ts index dd0a68b4f0..c747bbc4f7 100644 --- a/apps/web/src/app/auth/settings/change-password.component.ts +++ b/apps/web/src/app/auth/settings/change-password.component.ts @@ -1,6 +1,5 @@ import { Component } from "@angular/core"; import { Router } from "@angular/router"; -import { Observable } from "rxjs"; import { ChangePasswordComponent as BaseChangePasswordComponent } from "@bitwarden/angular/auth/components/change-password.component"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; @@ -8,7 +7,6 @@ import { AuditService } from "@bitwarden/common/abstractions/audit.service"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { PasswordRequest } from "@bitwarden/common/auth/models/request/password.request"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigServiceAbstraction } from "@bitwarden/common/platform/abstractions/config/config.service.abstraction"; import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -35,8 +33,6 @@ export class ChangePasswordComponent extends BaseChangePasswordComponent { checkForBreaches = true; characterMinimumMessage = ""; - protected showWebauthnLoginSettings$: Observable; - constructor( i18nService: I18nService, cryptoService: CryptoService, @@ -68,10 +64,6 @@ export class ChangePasswordComponent extends BaseChangePasswordComponent { } async ngOnInit() { - this.showWebauthnLoginSettings$ = this.configService.getFeatureFlag$( - FeatureFlag.PasswordlessLogin, - ); - if (!(await this.userVerificationService.hasMasterPassword())) { // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. // eslint-disable-next-line @typescript-eslint/no-floating-promises diff --git a/libs/angular/src/auth/components/login.component.ts b/libs/angular/src/auth/components/login.component.ts index 8917606279..8314bdb2dc 100644 --- a/libs/angular/src/auth/components/login.component.ts +++ b/libs/angular/src/auth/components/login.component.ts @@ -1,7 +1,7 @@ import { Directive, ElementRef, NgZone, OnDestroy, OnInit, ViewChild } from "@angular/core"; import { FormBuilder, Validators } from "@angular/forms"; import { ActivatedRoute, Router } from "@angular/router"; -import { Observable, Subject } from "rxjs"; +import { Subject } from "rxjs"; import { take, takeUntil } from "rxjs/operators"; import { LoginStrategyServiceAbstraction, PasswordLoginCredentials } from "@bitwarden/auth/common"; @@ -54,7 +54,6 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit, protected twoFactorRoute = "2fa"; protected successRoute = "vault"; protected forcePasswordResetRoute = "update-temp-password"; - protected showWebauthnLogin$: Observable; protected destroy$ = new Subject(); @@ -90,8 +89,6 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit, } async ngOnInit() { - this.showWebauthnLogin$ = this.webAuthnLoginService.enabled$; - this.route?.queryParams.pipe(takeUntil(this.destroy$)).subscribe((params) => { if (!params) { return; diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index db3b817e1e..f27756dfa1 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -864,7 +864,6 @@ import { ModalService } from "./modal.service"; deps: [ WebAuthnLoginApiServiceAbstraction, LoginStrategyServiceAbstraction, - ConfigServiceAbstraction, WebAuthnLoginPrfCryptoServiceAbstraction, WINDOW, LogService, diff --git a/libs/common/src/auth/abstractions/webauthn/webauthn-login.service.abstraction.ts b/libs/common/src/auth/abstractions/webauthn/webauthn-login.service.abstraction.ts index aec4c1081b..81ee1e8829 100644 --- a/libs/common/src/auth/abstractions/webauthn/webauthn-login.service.abstraction.ts +++ b/libs/common/src/auth/abstractions/webauthn/webauthn-login.service.abstraction.ts @@ -1,5 +1,3 @@ -import { Observable } from "rxjs"; - import { AuthResult } from "../../models/domain/auth-result"; import { WebAuthnLoginCredentialAssertionOptionsView } from "../../models/view/webauthn-login/webauthn-login-credential-assertion-options.view"; import { WebAuthnLoginCredentialAssertionView } from "../../models/view/webauthn-login/webauthn-login-credential-assertion.view"; @@ -8,11 +6,6 @@ import { WebAuthnLoginCredentialAssertionView } from "../../models/view/webauthn * Service for logging in with WebAuthnLogin credentials. */ export abstract class WebAuthnLoginServiceAbstraction { - /** - * An Observable that emits a boolean indicating whether the WebAuthn login feature is enabled. - */ - readonly enabled$: Observable; - /** * Gets the credential assertion options needed for initiating the WebAuthn * authentication process. It should provide the challenge and other data diff --git a/libs/common/src/auth/services/webauthn-login/webauthn-login.service.spec.ts b/libs/common/src/auth/services/webauthn-login/webauthn-login.service.spec.ts index 6cac8358e3..1c7f045461 100644 --- a/libs/common/src/auth/services/webauthn-login/webauthn-login.service.spec.ts +++ b/libs/common/src/auth/services/webauthn-login/webauthn-login.service.spec.ts @@ -1,9 +1,7 @@ import { mock } from "jest-mock-extended"; -import { firstValueFrom, of } from "rxjs"; import { LoginStrategyServiceAbstraction, WebAuthnLoginCredentials } from "@bitwarden/auth/common"; -import { ConfigServiceAbstraction } from "../../../platform/abstractions/config/config.service.abstraction"; import { LogService } from "../../../platform/abstractions/log.service"; import { Utils } from "../../../platform/misc/utils"; import { SymmetricCryptoKey } from "../../../platform/models/domain/symmetric-crypto-key"; @@ -23,7 +21,6 @@ describe("WebAuthnLoginService", () => { const webAuthnLoginApiService = mock(); const loginStrategyService = mock(); - const configService = mock(); const webAuthnLoginPrfCryptoService = mock(); const navigatorCredentials = mock(); const logService = mock(); @@ -71,12 +68,10 @@ describe("WebAuthnLoginService", () => { }); }); - function createWebAuthnLoginService(config: { featureEnabled: boolean }): WebAuthnLoginService { - configService.getFeatureFlag$.mockReturnValue(of(config.featureEnabled)); + function createWebAuthnLoginService(): WebAuthnLoginService { return new WebAuthnLoginService( webAuthnLoginApiService, loginStrategyService, - configService, webAuthnLoginPrfCryptoService, window, logService, @@ -84,34 +79,14 @@ describe("WebAuthnLoginService", () => { } it("instantiates", () => { - webAuthnLoginService = createWebAuthnLoginService({ featureEnabled: true }); + webAuthnLoginService = createWebAuthnLoginService(); expect(webAuthnLoginService).not.toBeFalsy(); }); - describe("enabled$", () => { - it("should emit true when feature flag for PasswordlessLogin is enabled", async () => { - // Arrange - const webAuthnLoginService = createWebAuthnLoginService({ featureEnabled: true }); - - // Act & Assert - const result = await firstValueFrom(webAuthnLoginService.enabled$); - expect(result).toBe(true); - }); - - it("should emit false when feature flag for PasswordlessLogin is disabled", async () => { - // Arrange - const webAuthnLoginService = createWebAuthnLoginService({ featureEnabled: false }); - - // Act & Assert - const result = await firstValueFrom(webAuthnLoginService.enabled$); - expect(result).toBe(false); - }); - }); - describe("getCredentialAssertionOptions()", () => { it("webAuthnLoginService returns WebAuthnLoginCredentialAssertionOptionsView when getCredentialAssertionOptions is called with the feature enabled", async () => { // Arrange - const webAuthnLoginService = createWebAuthnLoginService({ featureEnabled: true }); + const webAuthnLoginService = createWebAuthnLoginService(); const challenge = "6CG3jqMCVASJVXySMi9KWw"; const token = "BWWebAuthnLoginAssertionOptions_CfDJ_2KBN892w"; @@ -154,7 +129,7 @@ describe("WebAuthnLoginService", () => { describe("assertCredential(...)", () => { it("should assert the credential and return WebAuthnLoginAssertionView on success", async () => { // Arrange - const webAuthnLoginService = createWebAuthnLoginService({ featureEnabled: true }); + const webAuthnLoginService = createWebAuthnLoginService(); const credentialAssertionOptions = buildCredentialAssertionOptions(); // Mock webAuthnUtils functions @@ -222,7 +197,7 @@ describe("WebAuthnLoginService", () => { it("should return undefined on non-PublicKeyCredential browser response", async () => { // Arrange - const webAuthnLoginService = createWebAuthnLoginService({ featureEnabled: true }); + const webAuthnLoginService = createWebAuthnLoginService(); const credentialAssertionOptions = buildCredentialAssertionOptions(); // Mock the navigatorCredentials.get to return null @@ -237,7 +212,7 @@ describe("WebAuthnLoginService", () => { it("should log an error and return undefined when navigatorCredentials.get throws an error", async () => { // Arrange - const webAuthnLoginService = createWebAuthnLoginService({ featureEnabled: true }); + const webAuthnLoginService = createWebAuthnLoginService(); const credentialAssertionOptions = buildCredentialAssertionOptions(); // Mock navigatorCredentials.get to throw an error @@ -269,7 +244,7 @@ describe("WebAuthnLoginService", () => { it("should accept an assertion with a signed challenge and use it to try and login", async () => { // Arrange - const webAuthnLoginService = createWebAuthnLoginService({ featureEnabled: true }); + const webAuthnLoginService = createWebAuthnLoginService(); const assertion = buildWebAuthnLoginCredentialAssertionView(); const mockAuthResult: AuthResult = new AuthResult(); diff --git a/libs/common/src/auth/services/webauthn-login/webauthn-login.service.ts b/libs/common/src/auth/services/webauthn-login/webauthn-login.service.ts index 8c33076b3e..7fca20e615 100644 --- a/libs/common/src/auth/services/webauthn-login/webauthn-login.service.ts +++ b/libs/common/src/auth/services/webauthn-login/webauthn-login.service.ts @@ -1,9 +1,5 @@ -import { Observable } from "rxjs"; - import { LoginStrategyServiceAbstraction, WebAuthnLoginCredentials } from "@bitwarden/auth/common"; -import { FeatureFlag } from "../../../enums/feature-flag.enum"; -import { ConfigServiceAbstraction } from "../../../platform/abstractions/config/config.service.abstraction"; import { LogService } from "../../../platform/abstractions/log.service"; import { PrfKey } from "../../../types/key"; import { WebAuthnLoginApiServiceAbstraction } from "../../abstractions/webauthn/webauthn-login-api.service.abstraction"; @@ -16,19 +12,15 @@ import { WebAuthnLoginCredentialAssertionView } from "../../models/view/webauthn import { WebAuthnLoginAssertionResponseRequest } from "./request/webauthn-login-assertion-response.request"; export class WebAuthnLoginService implements WebAuthnLoginServiceAbstraction { - readonly enabled$: Observable; - private navigatorCredentials: CredentialsContainer; constructor( private webAuthnLoginApiService: WebAuthnLoginApiServiceAbstraction, private loginStrategyService: LoginStrategyServiceAbstraction, - private configService: ConfigServiceAbstraction, private webAuthnLoginPrfCryptoService: WebAuthnLoginPrfCryptoServiceAbstraction, private window: Window, private logService?: LogService, ) { - this.enabled$ = this.configService.getFeatureFlag$(FeatureFlag.PasswordlessLogin, false); this.navigatorCredentials = this.window.navigator.credentials; } diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 813078ca0a..f875e8ad25 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -1,5 +1,4 @@ export enum FeatureFlag { - PasswordlessLogin = "passwordless-login", BrowserFilelessImport = "browser-fileless-import", ItemShare = "item-share", FlexibleCollectionsV1 = "flexible-collections-v-1", // v-1 is intentional