chore(flag-removal): [Auth/PM20439] Remove flagging logic and flag (BrowserExtensionLoginApproval) (#16568)

2025-12-11 13:53:34 +00:00 · 2025-09-29 13:29:56 -07:00
parent 0bfc5daa7c
commit 5f7e1f99bf
11 changed files with 64 additions and 127 deletions
--- a/apps/browser/src/auth/popup/settings/account-security.component.html
+++ b/apps/browser/src/auth/popup/settings/account-security.component.html
@@ -102,7 +102,7 @@
      </bit-card>
    </bit-section>

-    <bit-section *ngIf="extensionLoginApprovalFlagEnabled">
+    <bit-section>
      <bit-section-header>
        <h2 bitTypography="h6">{{ "manageDevices" | i18n }}</h2>
      </bit-section-header>
--- a/apps/browser/src/auth/popup/settings/account-security.component.spec.ts
+++ b/apps/browser/src/auth/popup/settings/account-security.component.spec.ts
@@ -1,6 +1,7 @@
 import { Component } from "@angular/core";
 import { ComponentFixture, TestBed } from "@angular/core/testing";
 import { By } from "@angular/platform-browser";
+import { ActivatedRoute } from "@angular/router";
 import { mock } from "jest-mock-extended";
 import { firstValueFrom, of } from "rxjs";

@@ -19,7 +20,6 @@ import {
  VaultTimeoutStringType,
  VaultTimeoutAction,
 } from "@bitwarden/common/key-management/vault-timeout";
-import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
 import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
@@ -67,6 +67,7 @@ describe("AccountSecurityComponent", () => {
      providers: [
        { provide: AccountService, useValue: accountService },
        { provide: AccountSecurityComponent, useValue: mock<AccountSecurityComponent>() },
+        { provide: ActivatedRoute, useValue: mock<ActivatedRoute>() },
        { provide: BiometricsService, useValue: mock<BiometricsService>() },
        { provide: BiometricStateService, useValue: biometricStateService },
        { provide: DialogService, useValue: dialogService },
@@ -88,7 +89,6 @@ describe("AccountSecurityComponent", () => {
        { provide: LogService, useValue: mock<LogService>() },
        { provide: OrganizationService, useValue: mock<OrganizationService>() },
        { provide: CollectionService, useValue: mock<CollectionService>() },
-        { provide: ConfigService, useValue: mock<ConfigService>() },
        { provide: ValidationService, useValue: validationService },
      ],
    })
--- a/apps/browser/src/auth/popup/settings/account-security.component.ts
+++ b/apps/browser/src/auth/popup/settings/account-security.component.ts
@@ -31,7 +31,6 @@ import { getFirstPolicy } from "@bitwarden/common/admin-console/services/policy/
 import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
 import { getUserId } from "@bitwarden/common/auth/services/account.service";
-import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
 import { PinServiceAbstraction } from "@bitwarden/common/key-management/pin/pin.service.abstraction";
 import {
  VaultTimeout,
@@ -41,7 +40,6 @@ import {
  VaultTimeoutSettingsService,
  VaultTimeoutStringType,
 } from "@bitwarden/common/key-management/vault-timeout";
-import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
 import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
@@ -115,7 +113,6 @@ export class AccountSecurityComponent implements OnInit, OnDestroy {
  biometricUnavailabilityReason: string;
  showChangeMasterPass = true;
  pinEnabled$: Observable<boolean> = of(true);
-  extensionLoginApprovalFlagEnabled = false;

  form = this.formBuilder.group({
    vaultTimeout: [null as VaultTimeout | null],
@@ -157,7 +154,6 @@ export class AccountSecurityComponent implements OnInit, OnDestroy {
    private biometricsService: BiometricsService,
    private vaultNudgesService: NudgesService,
    private validationService: ValidationService,
-    private configService: ConfigService,
    private logService: LogService,
  ) {}

@@ -239,10 +235,6 @@ export class AccountSecurityComponent implements OnInit, OnDestroy {
    };
    this.form.patchValue(initialValues, { emitEvent: false });

-    this.extensionLoginApprovalFlagEnabled = await this.configService.getFeatureFlag(
-      FeatureFlag.PM14938_BrowserExtensionLoginApproval,
-    );
-
    timer(0, 1000)
      .pipe(
        switchMap(async () => {
--- a/apps/browser/src/popup/app-routing.module.ts
+++ b/apps/browser/src/popup/app-routing.module.ts
@@ -14,7 +14,6 @@ import {
 } from "@bitwarden/angular/auth/guards";
 import { ChangePasswordComponent } from "@bitwarden/angular/auth/password-management/change-password";
 import { SetInitialPasswordComponent } from "@bitwarden/angular/auth/password-management/set-initial-password/set-initial-password.component";
-import { canAccessFeature } from "@bitwarden/angular/platform/guard/feature-flag.guard";
 import {
  DevicesIcon,
  RegistrationUserAddIcon,
@@ -40,7 +39,6 @@ import {
  TwoFactorAuthComponent,
  TwoFactorAuthGuard,
 } from "@bitwarden/auth/angular";
-import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
 import { AnonLayoutWrapperComponent, AnonLayoutWrapperData } from "@bitwarden/components";
 import { LockComponent, ConfirmKeyConnectorDomainComponent } from "@bitwarden/key-management-ui";

@@ -262,7 +260,7 @@ const routes: Routes = [
  {
    path: "device-management",
    component: ExtensionDeviceManagementComponent,
-    canActivate: [canAccessFeature(FeatureFlag.PM14938_BrowserExtensionLoginApproval), authGuard],
+    canActivate: [authGuard],
    data: { elevation: 1 } satisfies RouteDataProperties,
  },
  {
--- a/apps/browser/src/popup/app.component.ts
+++ b/apps/browser/src/popup/app.component.ts
@@ -42,9 +42,7 @@ import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
 import { TokenService } from "@bitwarden/common/auth/abstractions/token.service";
 import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
 import { PendingAuthRequestsStateService } from "@bitwarden/common/auth/services/auth-request-answering/pending-auth-requests.state";
-import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
 import { AnimationControlService } from "@bitwarden/common/platform/abstractions/animation-control.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 { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
@@ -82,7 +80,6 @@ export class AppComponent implements OnInit, OnDestroy {
  private activeUserId: UserId;
  private routerAnimations = false;
  private processingPendingAuth = false;
-  private extensionLoginApprovalFeatureFlag = false;

  private destroy$ = new Subject<void>();

@@ -118,7 +115,6 @@ export class AppComponent implements OnInit, OnDestroy {
    private authRequestService: AuthRequestServiceAbstraction,
    private pendingAuthRequestsState: PendingAuthRequestsStateService,
    private authRequestAnsweringService: AuthRequestAnsweringServiceAbstraction,
-    private readonly configService: ConfigService,
  ) {
    this.deviceTrustToastService.setupListeners$.pipe(takeUntilDestroyed()).subscribe();

@@ -127,10 +123,6 @@ export class AppComponent implements OnInit, OnDestroy {
  }

  async ngOnInit() {
-    this.extensionLoginApprovalFeatureFlag = await firstValueFrom(
-      this.configService.getFeatureFlag$(FeatureFlag.PM14938_BrowserExtensionLoginApproval),
-    );
-
    initPopupClosedListener();

    this.compactModeService.init();
@@ -140,24 +132,22 @@ export class AppComponent implements OnInit, OnDestroy {
      this.activeUserId = account?.id;
    });

-    if (this.extensionLoginApprovalFeatureFlag) {
-      // Trigger processing auth requests when the active user is in an unlocked state. Runs once when
-      // the popup is open.
-      this.accountService.activeAccount$
-        .pipe(
-          map((a) => a?.id), // Extract active userId
-          distinctUntilChanged(), // Only when userId actually changes
-          filter((userId) => userId != null), // Require a valid userId
-          switchMap((userId) => this.authService.authStatusFor$(userId).pipe(take(1))), // Get current auth status once for new user
-          filter((status) => status === AuthenticationStatus.Unlocked), // Only when the new user is Unlocked
-          tap(() => {
-            // Trigger processing when switching users while popup is open
-            void this.authRequestAnsweringService.processPendingAuthRequests();
-          }),
-          takeUntil(this.destroy$),
-        )
-        .subscribe();
-    }
+    // Trigger processing auth requests when the active user is in an unlocked state. Runs once when
+    // the popup is open.
+    this.accountService.activeAccount$
+      .pipe(
+        map((a) => a?.id), // Extract active userId
+        distinctUntilChanged(), // Only when userId actually changes
+        filter((userId) => userId != null), // Require a valid userId
+        switchMap((userId) => this.authService.authStatusFor$(userId).pipe(take(1))), // Get current auth status once for new user
+        filter((status) => status === AuthenticationStatus.Unlocked), // Only when the new user is Unlocked
+        tap(() => {
+          // Trigger processing when switching users while popup is open
+          void this.authRequestAnsweringService.processPendingAuthRequests();
+        }),
+        takeUntil(this.destroy$),
+      )
+      .subscribe();

    this.authService.activeAccountStatus$
      .pipe(
@@ -169,24 +159,22 @@ export class AppComponent implements OnInit, OnDestroy {
      )
      .subscribe();

-    if (this.extensionLoginApprovalFeatureFlag) {
-      // When the popup is already open and the active account transitions to Unlocked,
-      // process any pending auth requests for the active user. The above subscription does not handle
-      // this case.
-      this.authService.activeAccountStatus$
-        .pipe(
-          startWith(null as unknown as AuthenticationStatus), // Seed previous value to handle initial emission
-          pairwise(), // Compare previous and current statuses
-          filter(
-            ([prev, curr]) =>
-              prev !== AuthenticationStatus.Unlocked && curr === AuthenticationStatus.Unlocked, // Fire on transitions into Unlocked (incl. initial)
-          ),
-          takeUntil(this.destroy$),
-        )
-        .subscribe(() => {
-          void this.authRequestAnsweringService.processPendingAuthRequests();
-        });
-    }
+    // When the popup is already open and the active account transitions to Unlocked,
+    // process any pending auth requests for the active user. The above subscription does not handle
+    // this case.
+    this.authService.activeAccountStatus$
+      .pipe(
+        startWith(null as unknown as AuthenticationStatus), // Seed previous value to handle initial emission
+        pairwise(), // Compare previous and current statuses
+        filter(
+          ([prev, curr]) =>
+            prev !== AuthenticationStatus.Unlocked && curr === AuthenticationStatus.Unlocked, // Fire on transitions into Unlocked (incl. initial)
+        ),
+        takeUntil(this.destroy$),
+      )
+      .subscribe(() => {
+        void this.authRequestAnsweringService.processPendingAuthRequests();
+      });

    this.ngZone.runOutsideAngular(() => {
      window.onmousedown = () => this.recordActivity();
@@ -241,10 +229,7 @@ export class AppComponent implements OnInit, OnDestroy {
            }

            await this.router.navigate(["lock"]);
-          } else if (
-            msg.command === "openLoginApproval" &&
-            this.extensionLoginApprovalFeatureFlag
-          ) {
+          } else if (msg.command === "openLoginApproval") {
            if (this.processingPendingAuth) {
              return;
            }
--- a/apps/desktop/src/vault/app/vault/vault-v2.component.ts
+++ b/apps/desktop/src/vault/app/vault/vault-v2.component.ts
@@ -25,7 +25,6 @@ import { Account, AccountService } from "@bitwarden/common/auth/abstractions/acc
 import { getUserId } from "@bitwarden/common/auth/services/account.service";
 import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service";
 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";
@@ -319,26 +318,13 @@ export class VaultV2Component<C extends CipherViewLike>
    this.searchBarService.setEnabled(true);
    this.searchBarService.setPlaceholderText(this.i18nService.t("searchVault"));

-    if (
-      (await firstValueFrom(
-        this.configService.getFeatureFlag$(FeatureFlag.PM14938_BrowserExtensionLoginApproval),
-      )) === true
-    ) {
-      const authRequests = await firstValueFrom(
-        this.authRequestService.getLatestPendingAuthRequest$(),
-      );
-      if (authRequests != null) {
-        this.messagingService.send("openLoginApproval", {
-          notificationId: authRequests.id,
-        });
-      }
-    } else {
-      const authRequest = await this.apiService.getLastAuthRequest();
-      if (authRequest != null) {
-        this.messagingService.send("openLoginApproval", {
-          notificationId: authRequest.id,
-        });
-      }
+    const authRequests = await firstValueFrom(
+      this.authRequestService.getLatestPendingAuthRequest$(),
+    );
+    if (authRequests != null) {
+      this.messagingService.send("openLoginApproval", {
+        notificationId: authRequests.id,
+      });
    }

    this.activeUserId = await firstValueFrom(
--- a/apps/web/src/app/vault/individual-vault/vault-banners/services/vault-banners.service.spec.ts
+++ b/apps/web/src/app/vault/individual-vault/vault-banners/services/vault-banners.service.spec.ts
@@ -1,6 +1,5 @@
 import { TestBed } from "@angular/core/testing";
-import { mock, MockProxy } from "jest-mock-extended";
-import { BehaviorSubject, firstValueFrom, of, take, timeout } from "rxjs";
+import { BehaviorSubject, firstValueFrom, take, timeout } from "rxjs";

 import {
  AuthRequestServiceAbstraction,
@@ -13,7 +12,6 @@ import { DeviceView } from "@bitwarden/common/auth/abstractions/devices/views/de
 import { AuthRequestResponse } from "@bitwarden/common/auth/models/response/auth-request.response";
 import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service";
 import { DeviceType } from "@bitwarden/common/enums";
-import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
 import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
 import { Utils } from "@bitwarden/common/platform/misc/utils";
 import { StateProvider } from "@bitwarden/common/platform/state";
@@ -45,14 +43,11 @@ describe("VaultBannersService", () => {
  });
  const devices$ = new BehaviorSubject<DeviceView[]>([]);
  const pendingAuthRequests$ = new BehaviorSubject<Array<AuthRequestResponse>>([]);
-  let configService: MockProxy<ConfigService>;

  beforeEach(() => {
    lastSync$.next(new Date("2024-05-14"));
    isSelfHost.mockClear();
    getEmailVerified.mockClear().mockResolvedValue(true);
-    configService = mock<ConfigService>();
-    configService.getFeatureFlag$.mockImplementation(() => of(true));

    TestBed.configureTestingModule({
      providers: [
@@ -99,10 +94,6 @@ describe("VaultBannersService", () => {
          provide: AuthRequestServiceAbstraction,
          useValue: { getPendingAuthRequests$: () => pendingAuthRequests$ },
        },
-        {
-          provide: ConfigService,
-          useValue: configService,
-        },
      ],
    });
  });
--- a/apps/web/src/app/vault/individual-vault/vault-banners/services/vault-banners.service.ts
+++ b/apps/web/src/app/vault/individual-vault/vault-banners/services/vault-banners.service.ts
@@ -6,10 +6,7 @@ import {
  UserDecryptionOptionsServiceAbstraction,
 } from "@bitwarden/auth/common";
 import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
-import { DevicesServiceAbstraction } from "@bitwarden/common/auth/abstractions/devices/devices.service.abstraction";
 import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service";
-import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
-import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
 import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
 import {
  StateProvider,
@@ -70,9 +67,7 @@ export class VaultBannersService {
    private kdfConfigService: KdfConfigService,
    private syncService: SyncService,
    private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction,
-    private devicesService: DevicesServiceAbstraction,
    private authRequestService: AuthRequestServiceAbstraction,
-    private configService: ConfigService,
  ) {}

  /** Returns true when the pending auth request banner should be shown */
@@ -80,24 +75,12 @@ export class VaultBannersService {
    const alreadyDismissed = (await this.getBannerDismissedState(userId)).includes(
      VisibleVaultBanner.PendingAuthRequest,
    );
-    // TODO: PM-20439 remove feature flag
-    const browserLoginApprovalFeatureFlag = await firstValueFrom(
-      this.configService.getFeatureFlag$(FeatureFlag.PM14938_BrowserExtensionLoginApproval),
+
+    const pendingAuthRequests = await firstValueFrom(
+      this.authRequestService.getPendingAuthRequests$(),
    );
-    if (browserLoginApprovalFeatureFlag === true) {
-      const pendingAuthRequests = await firstValueFrom(
-        this.authRequestService.getPendingAuthRequests$(),
-      );

-      return pendingAuthRequests.length > 0 && !alreadyDismissed;
-    } else {
-      const devices = await firstValueFrom(this.devicesService.getDevices$());
-      const hasPendingRequest = devices.some(
-        (device) => device.response?.devicePendingAuthRequest != null,
-      );
-
-      return hasPendingRequest && !alreadyDismissed;
-    }
+    return pendingAuthRequests.length > 0 && !alreadyDismissed;
  }

  shouldShowPremiumBanner$(userId: UserId): Observable<boolean> {
--- a/libs/common/src/enums/feature-flag.enum.ts
+++ b/libs/common/src/enums/feature-flag.enum.ts
@@ -15,7 +15,6 @@ export enum FeatureFlag {
  CollectionVaultRefactor = "pm-25030-resolve-ts-upgrade-errors",

  /* Auth */
-  PM14938_BrowserExtensionLoginApproval = "pm-14938-browser-extension-login-approvals",
  PM22110_DisableAlternateLoginMethods = "pm-22110-disable-alternate-login-methods",

  /* Autofill */
@@ -98,7 +97,6 @@ export const DefaultFeatureFlagValue = {
  [FeatureFlag.PM22136_SdkCipherEncryption]: FALSE,

  /* Auth */
-  [FeatureFlag.PM14938_BrowserExtensionLoginApproval]: FALSE,
  [FeatureFlag.PM22110_DisableAlternateLoginMethods]: FALSE,

  /* Billing */
--- a/libs/common/src/platform/server-notifications/internal/default-server-notifications.multiuser.spec.ts
+++ b/libs/common/src/platform/server-notifications/internal/default-server-notifications.multiuser.spec.ts
@@ -132,7 +132,6 @@ describe("DefaultServerNotificationsService (multi-user)", () => {
      const flagValueByFlag: Partial<Record<FeatureFlag, boolean>> = {
        [FeatureFlag.InactiveUserServerNotification]: true,
        [FeatureFlag.PushNotificationsWhenLocked]: true,
-        [FeatureFlag.PM14938_BrowserExtensionLoginApproval]: true,
      };
      return new BehaviorSubject(flagValueByFlag[flag] ?? false) as any;
    });
--- a/libs/common/src/platform/server-notifications/internal/default-server-notifications.service.ts
+++ b/libs/common/src/platform/server-notifications/internal/default-server-notifications.service.ts
@@ -278,16 +278,21 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer
        await this.syncService.syncDeleteSend(notification.payload as SyncSendNotification);
        break;
      case NotificationType.AuthRequest:
-        if (
-          await firstValueFrom(
-            this.configService.getFeatureFlag$(FeatureFlag.PM14938_BrowserExtensionLoginApproval),
-          )
-        ) {
-          await this.authRequestAnsweringService.receivedPendingAuthRequest(
-            notification.payload.userId,
-            notification.payload.id,
-          );
-        }
+        await this.authRequestAnsweringService.receivedPendingAuthRequest(
+          notification.payload.userId,
+          notification.payload.id,
+        );
+
+        /**
+         * This call is necessary for Desktop, which for the time being uses a noop for the
+         * authRequestAnsweringService.receivedPendingAuthRequest() call just above. Desktop
+         * will eventually use the new AuthRequestAnsweringService, at which point we can remove
+         * this second call.
+         *
+         * The Extension AppComponent has logic (see processingPendingAuth) that only allows one
+         * pending auth request to process at a time, so this second call will not cause any
+         * duplicate processing conflicts on Extension.
+         */
        this.messagingService.send("openLoginApproval", {
          notificationId: notification.payload.id,
        });