diff --git a/apps/browser/src/auth/popup/settings/account-security.component.html b/apps/browser/src/auth/popup/settings/account-security.component.html index 3de1cc81a69..44900acc065 100644 --- 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 @@ - +

{{ "manageDevices" | i18n }}

 diff --git a/apps/browser/src/auth/popup/settings/account-security.component.spec.ts b/apps/browser/src/auth/popup/settings/account-security.component.spec.ts index 63666440a76..2335c5c2e69 100644 --- 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() }, + { provide: ActivatedRoute, useValue: mock() }, { provide: BiometricsService, useValue: mock() }, { provide: BiometricStateService, useValue: biometricStateService }, { provide: DialogService, useValue: dialogService }, @@ -88,7 +89,6 @@ describe("AccountSecurityComponent", () => { { provide: LogService, useValue: mock() }, { provide: OrganizationService, useValue: mock() }, { provide: CollectionService, useValue: mock() }, - { provide: ConfigService, useValue: mock() }, { provide: ValidationService, useValue: validationService }, ], }) diff --git a/apps/browser/src/auth/popup/settings/account-security.component.ts b/apps/browser/src/auth/popup/settings/account-security.component.ts index 72a389ecf71..0c9b4634569 100644 --- 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 = 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 () => { diff --git a/apps/browser/src/popup/app-routing.module.ts b/apps/browser/src/popup/app-routing.module.ts index 501ef0ba8ff..b69d7b73672 100644 --- 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, }, { diff --git a/apps/browser/src/popup/app.component.ts b/apps/browser/src/popup/app.component.ts index 39f82622b68..998531488d3 100644 --- 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(); @@ -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; } diff --git a/apps/desktop/src/vault/app/vault/vault-v2.component.ts b/apps/desktop/src/vault/app/vault/vault-v2.component.ts index 5a6683ed904..3fdb14aa154 100644 --- 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 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( diff --git 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 index c97b23b1456..89a3757e939 100644 --- 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([]); const pendingAuthRequests$ = new BehaviorSubject>([]); - let configService: MockProxy; beforeEach(() => { lastSync$.next(new Date("2024-05-14")); isSelfHost.mockClear(); getEmailVerified.mockClear().mockResolvedValue(true); - configService = mock(); - configService.getFeatureFlag$.mockImplementation(() => of(true)); TestBed.configureTestingModule({ providers: [ @@ -99,10 +94,6 @@ describe("VaultBannersService", () => { provide: AuthRequestServiceAbstraction, useValue: { getPendingAuthRequests$: () => pendingAuthRequests$ }, }, - { - provide: ConfigService, - useValue: configService, - }, ], }); }); diff --git 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 index dd50c832cc6..c4396940998 100644 --- 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 { diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 18134fee2c3..bd874f934f0 100644 --- 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 */ diff --git 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 index 9bc47437b39..b4d47698e4d 100644 --- 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> = { [FeatureFlag.InactiveUserServerNotification]: true, [FeatureFlag.PushNotificationsWhenLocked]: true, - [FeatureFlag.PM14938_BrowserExtensionLoginApproval]: true, }; return new BehaviorSubject(flagValueByFlag[flag] ?? false) as any; }); diff --git 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 index e934dec185d..47af8f5e00c 100644 --- 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, });