diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 35de9d0253e..37bd85a9be8 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -2,7 +2,17 @@ // @ts-strict-ignore import "core-js/proposals/explicit-resource-management"; -import { filter, firstValueFrom, map, merge, Subject, switchMap, timeout } from "rxjs"; +import { + filter, + firstValueFrom, + from, + map, + merge, + Observable, + Subject, + switchMap, + timeout, +} from "rxjs"; import { CollectionService, DefaultCollectionService } from "@bitwarden/admin-console/common"; import { @@ -42,6 +52,7 @@ import { AuthServerNotificationTags } from "@bitwarden/common/auth/enums/auth-se import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { AccountServiceImplementation } from "@bitwarden/common/auth/services/account.service"; import { AuthRequestAnsweringService } from "@bitwarden/common/auth/services/auth-request-answering/auth-request-answering.service"; +import { PendingAuthRequestsStateService } from "@bitwarden/common/auth/services/auth-request-answering/pending-auth-requests.state"; import { AuthService } from "@bitwarden/common/auth/services/auth.service"; import { AvatarService } from "@bitwarden/common/auth/services/avatar.service"; import { DefaultActiveUserAccessor } from "@bitwarden/common/auth/services/default-active-user.accessor"; @@ -152,6 +163,7 @@ import { SyncService } from "@bitwarden/common/platform/sync"; // eslint-disable-next-line no-restricted-imports -- Needed for service creation import { DefaultSyncService } from "@bitwarden/common/platform/sync/internal"; import { SystemNotificationsService } from "@bitwarden/common/platform/system-notifications/"; +import { SystemNotificationEvent } from "@bitwarden/common/platform/system-notifications/system-notifications.service"; import { UnsupportedSystemNotificationsService } from "@bitwarden/common/platform/system-notifications/unsupported-system-notifications.service"; import { DefaultThemeStateService } from "@bitwarden/common/platform/theming/theme-state.service"; import { ApiService } from "@bitwarden/common/services/api.service"; @@ -407,6 +419,7 @@ export default class MainBackground { individualVaultExportService: IndividualVaultExportServiceAbstraction; organizationVaultExportService: OrganizationVaultExportServiceAbstraction; vaultSettingsService: VaultSettingsServiceAbstraction; + pendingAuthRequestStateService: PendingAuthRequestsStateService; biometricStateService: BiometricStateService; biometricsService: BiometricsService; stateEventRunnerService: StateEventRunnerService; @@ -1135,12 +1148,16 @@ export default class MainBackground { this.systemNotificationService = new UnsupportedSystemNotificationsService(); } + this.pendingAuthRequestStateService = new PendingAuthRequestsStateService(this.stateProvider); + this.authRequestAnsweringService = new AuthRequestAnsweringService( this.accountService, this.actionsService, this.authService, this.i18nService, this.masterPasswordService, + this.messagingService, + this.pendingAuthRequestStateService, this.platformUtilsService, this.systemNotificationService, ); @@ -1421,7 +1438,7 @@ export default class MainBackground { // Only the "true" background should run migrations await this.migrationRunner.run(); - // This is here instead of in in the InitService b/c we don't plan for + // This is here instead of in the InitService b/c we don't plan for // side effects to run in the Browser InitService. const accounts = await firstValueFrom(this.accountService.accounts$); @@ -1800,18 +1817,41 @@ export default class MainBackground { /** * This function is for creating any subscriptions for the background service worker. We do this * here because it's important to run this during the evaluation period of the browser extension - * service worker. + * service worker. If it's not done this way we risk the service worker being closed before it's + * registered these system notification click events. */ initNotificationSubscriptions() { - this.systemNotificationService.notificationClicked$ - .pipe( - filter((n) => n.id.startsWith(AuthServerNotificationTags.AuthRequest + "_")), - map((n) => ({ event: n, authRequestId: n.id.split("_")[1] })), - switchMap(({ event }) => - this.authRequestAnsweringService.handleAuthRequestNotificationClicked(event), + const handlers: Array<{ + startsWith: string; + handler: (event: SystemNotificationEvent) => Promise; + }> = []; + + const register = ( + startsWith: string, + handler: (event: SystemNotificationEvent) => Promise, + ) => { + handlers.push({ startsWith, handler }); + }; + + // ======= Register All System Notification Handlers Here ======= + register(AuthServerNotificationTags.AuthRequest, (event) => + this.authRequestAnsweringService.handleAuthRequestNotificationClicked(event), + ); + // ======= End Register All System Notification Handlers ======= + + const streams: Observable[] = handlers.map(({ startsWith, handler }) => + this.systemNotificationService.notificationClicked$.pipe( + filter((event: SystemNotificationEvent): boolean => event.id.startsWith(startsWith + "_")), + switchMap( + (event: SystemNotificationEvent): Observable => + from(Promise.resolve(handler(event))), ), - ) - .subscribe(); + ), + ); + + if (streams.length > 0) { + merge(...streams).subscribe(); + } } /** diff --git a/apps/browser/src/popup/app.component.ts b/apps/browser/src/popup/app.component.ts index 4f46f889eaa..39f82622b68 100644 --- a/apps/browser/src/popup/app.component.ts +++ b/apps/browser/src/popup/app.component.ts @@ -4,33 +4,47 @@ import { ChangeDetectorRef, Component, DestroyRef, + inject, NgZone, OnDestroy, OnInit, - inject, } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { NavigationEnd, Router, RouterOutlet } from "@angular/router"; import { - Subject, - takeUntil, - firstValueFrom, - concatMap, - filter, - tap, catchError, - of, + concatMap, + distinctUntilChanged, + filter, + firstValueFrom, map, + of, + pairwise, + startWith, + Subject, + switchMap, + take, + takeUntil, + tap, } from "rxjs"; +import { LoginApprovalDialogComponent } from "@bitwarden/angular/auth/login-approval/login-approval-dialog.component"; import { DeviceTrustToastService } from "@bitwarden/angular/auth/services/device-trust-toast.service.abstraction"; import { DocumentLangSetter } from "@bitwarden/angular/platform/i18n"; -import { LogoutReason, UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; +import { + AuthRequestServiceAbstraction, + LogoutReason, + UserDecryptionOptionsServiceAbstraction, +} from "@bitwarden/auth/common"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthRequestAnsweringServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; 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"; @@ -67,6 +81,8 @@ export class AppComponent implements OnInit, OnDestroy { private lastActivity: Date; private activeUserId: UserId; private routerAnimations = false; + private processingPendingAuth = false; + private extensionLoginApprovalFeatureFlag = false; private destroy$ = new Subject(); @@ -99,6 +115,10 @@ export class AppComponent implements OnInit, OnDestroy { private readonly documentLangSetter: DocumentLangSetter, private popupSizeService: PopupSizeService, private logService: LogService, + private authRequestService: AuthRequestServiceAbstraction, + private pendingAuthRequestsState: PendingAuthRequestsStateService, + private authRequestAnsweringService: AuthRequestAnsweringServiceAbstraction, + private readonly configService: ConfigService, ) { this.deviceTrustToastService.setupListeners$.pipe(takeUntilDestroyed()).subscribe(); @@ -107,6 +127,10 @@ export class AppComponent implements OnInit, OnDestroy { } async ngOnInit() { + this.extensionLoginApprovalFeatureFlag = await firstValueFrom( + this.configService.getFeatureFlag$(FeatureFlag.PM14938_BrowserExtensionLoginApproval), + ); + initPopupClosedListener(); this.compactModeService.init(); @@ -116,6 +140,25 @@ 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(); + } + this.authService.activeAccountStatus$ .pipe( filter((status) => status === AuthenticationStatus.Unlocked), @@ -126,6 +169,25 @@ 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(); + }); + } + this.ngZone.runOutsideAngular(() => { window.onmousedown = () => this.recordActivity(); window.ontouchstart = () => this.recordActivity(); @@ -179,6 +241,42 @@ export class AppComponent implements OnInit, OnDestroy { } await this.router.navigate(["lock"]); + } else if ( + msg.command === "openLoginApproval" && + this.extensionLoginApprovalFeatureFlag + ) { + if (this.processingPendingAuth) { + return; + } + this.processingPendingAuth = true; + try { + // Always query server for all pending requests and open a dialog for each + const pendingList = await firstValueFrom( + this.authRequestService.getPendingAuthRequests$(), + ); + if (Array.isArray(pendingList) && pendingList.length > 0) { + const respondedIds = new Set(); + for (const req of pendingList) { + if (req?.id == null) { + continue; + } + const dialogRef = LoginApprovalDialogComponent.open(this.dialogService, { + notificationId: req.id, + }); + + const result = await firstValueFrom(dialogRef.closed); + + if (result !== undefined && typeof result === "boolean") { + respondedIds.add(req.id); + if (respondedIds.size === pendingList.length && this.activeUserId != null) { + await this.pendingAuthRequestsState.clear(this.activeUserId); + } + } + } + } + } finally { + this.processingPendingAuth = false; + } } else if (msg.command === "showDialog") { // 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/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index ab9dafc531a..8da94acdd7a 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -50,6 +50,7 @@ import { SsoLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/ import { TokenService } from "@bitwarden/common/auth/abstractions/token.service"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { AuthRequestAnsweringService } from "@bitwarden/common/auth/services/auth-request-answering/auth-request-answering.service"; +import { PendingAuthRequestsStateService } from "@bitwarden/common/auth/services/auth-request-answering/pending-auth-requests.state"; import { AutofillSettingsService, AutofillSettingsServiceAbstraction, @@ -86,7 +87,10 @@ import { EnvironmentService } from "@bitwarden/common/platform/abstractions/envi import { FileDownloadService } from "@bitwarden/common/platform/abstractions/file-download/file-download.service"; import { I18nService as I18nServiceAbstraction } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; -import { MessagingService as MessagingServiceAbstraction } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { + MessagingService, + MessagingService as MessagingServiceAbstraction, +} from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { SdkClientFactory } from "@bitwarden/common/platform/abstractions/sdk/sdk-client-factory"; import { SdkLoadService } from "@bitwarden/common/platform/abstractions/sdk/sdk-load.service"; @@ -484,6 +488,8 @@ const safeProviders: SafeProvider[] = [ AuthService, I18nServiceAbstraction, MasterPasswordServiceAbstraction, + MessagingService, + PendingAuthRequestsStateService, PlatformUtilsService, SystemNotificationsService, ], diff --git a/libs/angular/src/auth/device-management/device-management.component.ts b/libs/angular/src/auth/device-management/device-management.component.ts index 3ab9b2146c5..2c67812b586 100644 --- a/libs/angular/src/auth/device-management/device-management.component.ts +++ b/libs/angular/src/auth/device-management/device-management.component.ts @@ -6,12 +6,15 @@ import { firstValueFrom } from "rxjs"; // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. // eslint-disable-next-line no-restricted-imports import { AuthRequestApiServiceAbstraction } 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 { DevicePendingAuthRequest, DeviceResponse, } from "@bitwarden/common/auth/abstractions/devices/responses/device.response"; import { DeviceView } from "@bitwarden/common/auth/abstractions/devices/views/device.view"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; +import { PendingAuthRequestsStateService } from "@bitwarden/common/auth/services/auth-request-answering/pending-auth-requests.state"; import { DeviceType, DeviceTypeMetadata } from "@bitwarden/common/enums"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; @@ -66,14 +69,16 @@ export class DeviceManagementComponent implements OnInit { protected showHeaderInfo = false; constructor( - private authRequestApiService: AuthRequestApiServiceAbstraction, - private destroyRef: DestroyRef, - private deviceManagementComponentService: DeviceManagementComponentServiceAbstraction, - private devicesService: DevicesServiceAbstraction, - private dialogService: DialogService, - private i18nService: I18nService, - private messageListener: MessageListener, - private validationService: ValidationService, + private readonly accountService: AccountService, + private readonly authRequestApiService: AuthRequestApiServiceAbstraction, + private readonly destroyRef: DestroyRef, + private readonly deviceManagementComponentService: DeviceManagementComponentServiceAbstraction, + private readonly devicesService: DevicesServiceAbstraction, + private readonly dialogService: DialogService, + private readonly i18nService: I18nService, + private readonly messageListener: MessageListener, + private readonly pendingAuthRequestStateService: PendingAuthRequestsStateService, + private readonly validationService: ValidationService, ) { this.showHeaderInfo = this.deviceManagementComponentService.showHeaderInformation(); } @@ -248,6 +253,12 @@ export class DeviceManagementComponent implements OnInit { // Auth request was approved or denied, so clear the // pending auth request and re-sort the device array this.devices = clearAuthRequestAndResortDevices(this.devices, pendingAuthRequest); + + // If a user ignores or doesn't see the auth request dialog, but comes to account settings + // to approve a device login attempt, clear out the state for that user. + await this.pendingAuthRequestStateService.clear( + await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)), + ); } } } diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 05226179c67..3298e7e5913 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -106,6 +106,7 @@ import { AccountApiServiceImplementation } from "@bitwarden/common/auth/services import { AccountServiceImplementation } from "@bitwarden/common/auth/services/account.service"; import { AnonymousHubService } from "@bitwarden/common/auth/services/anonymous-hub.service"; import { NoopAuthRequestAnsweringService } from "@bitwarden/common/auth/services/auth-request-answering/noop-auth-request-answering.service"; +import { PendingAuthRequestsStateService } from "@bitwarden/common/auth/services/auth-request-answering/pending-auth-requests.state"; import { AuthService } from "@bitwarden/common/auth/services/auth.service"; import { AvatarService } from "@bitwarden/common/auth/services/avatar.service"; import { DefaultActiveUserAccessor } from "@bitwarden/common/auth/services/default-active-user.accessor"; @@ -942,6 +943,11 @@ const safeProviders: SafeProvider[] = [ useClass: UnsupportedSystemNotificationsService, deps: [], }), + safeProvider({ + provide: PendingAuthRequestsStateService, + useClass: PendingAuthRequestsStateService, + deps: [StateProvider], + }), safeProvider({ provide: AuthRequestAnsweringServiceAbstraction, useClass: NoopAuthRequestAnsweringService, diff --git a/libs/auth/src/angular/self-hosted-env-config-dialog/self-hosted-env-config-dialog.component.ts b/libs/auth/src/angular/self-hosted-env-config-dialog/self-hosted-env-config-dialog.component.ts index a7ffca9163c..16c25f2404f 100644 --- a/libs/auth/src/angular/self-hosted-env-config-dialog/self-hosted-env-config-dialog.component.ts +++ b/libs/auth/src/angular/self-hosted-env-config-dialog/self-hosted-env-config-dialog.component.ts @@ -159,7 +159,6 @@ export class SelfHostedEnvConfigDialogComponent implements OnInit, OnDestroy { }, }); } - submit = async () => { this.showErrorSummary = false; diff --git a/libs/common/src/auth/abstractions/auth-request-answering/README.md b/libs/common/src/auth/abstractions/auth-request-answering/README.md new file mode 100644 index 00000000000..9a24f095d70 --- /dev/null +++ b/libs/common/src/auth/abstractions/auth-request-answering/README.md @@ -0,0 +1,10 @@ +# Auth Request Answering Service + +This feature is to allow for the taking of auth requests that are received via websockets by the background service to +be acted on when the user loads up a client. Currently only implemented with the browser client. + +See diagram for the high level picture of how this is wired up. + +## Diagram + +![img.png](notification-architecture.png) diff --git a/libs/common/src/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction.ts b/libs/common/src/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction.ts index 8331ff88d3f..f45cb34496e 100644 --- a/libs/common/src/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction.ts +++ b/libs/common/src/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction.ts @@ -2,7 +2,29 @@ import { SystemNotificationEvent } from "@bitwarden/common/platform/system-notif import { UserId } from "@bitwarden/user-core"; export abstract class AuthRequestAnsweringServiceAbstraction { - abstract receivedPendingAuthRequest(userId: UserId, notificationId: string): Promise; + /** + * Tries to either display the dialog for the user or will preserve its data and show it at a + * later time. Even in the event the dialog is shown immediately, this will write to global state + * so that even if someone closes a window or a popup and comes back, it could be processed later. + * Only way to clear out the global state is to respond to the auth request. + * + * Currently, this is only implemented for browser extension. + * + * @param userId The UserId that the auth request is for. + * @param authRequestId The id of the auth request that is to be processed. + */ + abstract receivedPendingAuthRequest(userId: UserId, authRequestId: string): Promise; + /** + * When a system notification is clicked, this function is used to process that event. + * + * @param event The event passed in. Check initNotificationSubscriptions in main.background.ts. + */ abstract handleAuthRequestNotificationClicked(event: SystemNotificationEvent): Promise; + + /** + * Process notifications that have been received but didn't meet the conditions to display the + * approval dialog. + */ + abstract processPendingAuthRequests(): Promise; } diff --git a/libs/common/src/auth/abstractions/auth-request-answering/notification-architecture.png b/libs/common/src/auth/abstractions/auth-request-answering/notification-architecture.png new file mode 100644 index 00000000000..5d9d9d8bccb Binary files /dev/null and b/libs/common/src/auth/abstractions/auth-request-answering/notification-architecture.png differ diff --git a/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.spec.ts b/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.spec.ts index b0ed63d7cf1..0b12e1cb661 100644 --- a/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.spec.ts +++ b/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.spec.ts @@ -7,6 +7,7 @@ import { AuthServerNotificationTags } from "@bitwarden/common/auth/enums/auth-se import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { ActionsService } from "@bitwarden/common/platform/actions"; import { @@ -17,6 +18,7 @@ import { import { UserId } from "@bitwarden/user-core"; import { AuthRequestAnsweringService } from "./auth-request-answering.service"; +import { PendingAuthRequestsStateService } from "./pending-auth-requests.state"; describe("AuthRequestAnsweringService", () => { let accountService: MockProxy; @@ -24,6 +26,8 @@ describe("AuthRequestAnsweringService", () => { let authService: MockProxy; let i18nService: MockProxy; let masterPasswordService: any; // MasterPasswordServiceAbstraction has many members; we only use forceSetPasswordReason$ + let messagingService: MockProxy; + let pendingAuthRequestsState: MockProxy; let platformUtilsService: MockProxy; let systemNotificationsService: MockProxy; @@ -37,6 +41,8 @@ describe("AuthRequestAnsweringService", () => { authService = mock(); i18nService = mock(); masterPasswordService = { forceSetPasswordReason$: jest.fn() }; + messagingService = mock(); + pendingAuthRequestsState = mock(); platformUtilsService = mock(); systemNotificationsService = mock(); @@ -66,6 +72,8 @@ describe("AuthRequestAnsweringService", () => { authService, i18nService, masterPasswordService, + messagingService, + pendingAuthRequestsState, platformUtilsService, systemNotificationsService, ); diff --git a/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.ts b/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.ts index a562125d1fd..834d6ac7bcc 100644 --- a/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.ts +++ b/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.ts @@ -5,9 +5,10 @@ import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AuthServerNotificationTags } from "@bitwarden/common/auth/enums/auth-server-notification-tags"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; -import { getUserId } from "@bitwarden/common/auth/services/account.service"; +import { getOptionalUserId, getUserId } from "@bitwarden/common/auth/services/account.service"; import { MasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { ActionsService } from "@bitwarden/common/platform/actions"; import { @@ -19,6 +20,11 @@ import { UserId } from "@bitwarden/user-core"; import { AuthRequestAnsweringServiceAbstraction } from "../../abstractions/auth-request-answering/auth-request-answering.service.abstraction"; +import { + PendingAuthRequestsStateService, + PendingAuthUserMarker, +} from "./pending-auth-requests.state"; + export class AuthRequestAnsweringService implements AuthRequestAnsweringServiceAbstraction { constructor( private readonly accountService: AccountService, @@ -26,10 +32,51 @@ export class AuthRequestAnsweringService implements AuthRequestAnsweringServiceA private readonly authService: AuthService, private readonly i18nService: I18nService, private readonly masterPasswordService: MasterPasswordServiceAbstraction, + private readonly messagingService: MessagingService, + private readonly pendingAuthRequestsState: PendingAuthRequestsStateService, private readonly platformUtilsService: PlatformUtilsService, private readonly systemNotificationsService: SystemNotificationsService, ) {} + async receivedPendingAuthRequest(userId: UserId, authRequestId: string): Promise { + const authStatus = await firstValueFrom(this.authService.activeAccountStatus$); + const activeUserId: UserId | null = await firstValueFrom( + this.accountService.activeAccount$.pipe(getOptionalUserId), + ); + const forceSetPasswordReason = await firstValueFrom( + this.masterPasswordService.forceSetPasswordReason$(userId), + ); + const popupOpen = await this.platformUtilsService.isPopupOpen(); + + // Always persist the pending marker for this user to global state. + await this.pendingAuthRequestsState.add(userId); + + // These are the conditions we are looking for to know if the extension is in a state to show + // the approval dialog. + const userIsAvailableToReceiveAuthRequest = + popupOpen && + authStatus === AuthenticationStatus.Unlocked && + activeUserId === userId && + forceSetPasswordReason === ForceSetPasswordReason.None; + + if (!userIsAvailableToReceiveAuthRequest) { + // Get the user's email to include in the system notification + const accounts = await firstValueFrom(this.accountService.accounts$); + const emailForUser = accounts[userId].email; + + await this.systemNotificationsService.create({ + id: `${AuthServerNotificationTags.AuthRequest}_${authRequestId}`, // the underscore is an important delimiter. + title: this.i18nService.t("accountAccessRequested"), + body: this.i18nService.t("confirmAccessAttempt", emailForUser), + buttons: [], + }); + return; + } + + // Popup is open and conditions are met; open dialog immediately for this request + this.messagingService.send("openLoginApproval"); + } + async handleAuthRequestNotificationClicked(event: SystemNotificationEvent): Promise { if (event.buttonIdentifier === ButtonLocation.NotificationButton) { await this.systemNotificationsService.clear({ @@ -39,32 +86,26 @@ export class AuthRequestAnsweringService implements AuthRequestAnsweringServiceA } } - async receivedPendingAuthRequest(userId: UserId, authRequestId: string): Promise { - const authStatus = await firstValueFrom(this.authService.activeAccountStatus$); - const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); - const forceSetPasswordReason = await firstValueFrom( - this.masterPasswordService.forceSetPasswordReason$(userId), - ); + async processPendingAuthRequests(): Promise { + // Prune any stale pending requests (older than 15 minutes) + // This comes from GlobalSettings.cs + // public TimeSpan UserRequestExpiration { get; set; } = TimeSpan.FromMinutes(15); + const fifteenMinutesMs = 15 * 60 * 1000; - // Is the popup already open? - if ( - (await this.platformUtilsService.isPopupOpen()) && - authStatus === AuthenticationStatus.Unlocked && - activeUserId === userId && - forceSetPasswordReason === ForceSetPasswordReason.None - ) { - // TODO: Handled in 14934 - } else { - // Get the user's email to include in the system notification - const accounts = await firstValueFrom(this.accountService.accounts$); - const emailForUser = accounts[userId].email; + await this.pendingAuthRequestsState.pruneOlderThan(fifteenMinutesMs); - await this.systemNotificationsService.create({ - id: `${AuthServerNotificationTags.AuthRequest}_${authRequestId}`, - title: this.i18nService.t("accountAccessRequested"), - body: this.i18nService.t("confirmAccessAttempt", emailForUser), - buttons: [], - }); + const pendingAuthRequestsInState: PendingAuthUserMarker[] = + (await firstValueFrom(this.pendingAuthRequestsState.getAll$())) ?? []; + + if (pendingAuthRequestsInState.length > 0) { + const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + const pendingAuthRequestsForActiveUser = pendingAuthRequestsInState.some( + (e) => e.userId === activeUserId, + ); + + if (pendingAuthRequestsForActiveUser) { + this.messagingService.send("openLoginApproval"); + } } } } diff --git a/libs/common/src/auth/services/auth-request-answering/noop-auth-request-answering.service.ts b/libs/common/src/auth/services/auth-request-answering/noop-auth-request-answering.service.ts index 9e229ef1129..730362adfed 100644 --- a/libs/common/src/auth/services/auth-request-answering/noop-auth-request-answering.service.ts +++ b/libs/common/src/auth/services/auth-request-answering/noop-auth-request-answering.service.ts @@ -5,7 +5,10 @@ import { AuthRequestAnsweringServiceAbstraction } from "../../abstractions/auth- export class NoopAuthRequestAnsweringService implements AuthRequestAnsweringServiceAbstraction { constructor() {} - async handleAuthRequestNotificationClicked(event: SystemNotificationEvent): Promise {} async receivedPendingAuthRequest(userId: UserId, notificationId: string): Promise {} + + async handleAuthRequestNotificationClicked(event: SystemNotificationEvent): Promise {} + + async processPendingAuthRequests(): Promise {} } diff --git a/libs/common/src/auth/services/auth-request-answering/pending-auth-requests.state.ts b/libs/common/src/auth/services/auth-request-answering/pending-auth-requests.state.ts new file mode 100644 index 00000000000..5205df00f84 --- /dev/null +++ b/libs/common/src/auth/services/auth-request-answering/pending-auth-requests.state.ts @@ -0,0 +1,57 @@ +import { Observable } from "rxjs"; + +import { + AUTH_REQUEST_DISK_LOCAL, + GlobalState, + KeyDefinition, + StateProvider, +} from "@bitwarden/common/platform/state"; +import { UserId } from "@bitwarden/user-core"; + +export type PendingAuthUserMarker = { + userId: UserId; + receivedAtMs: number; +}; + +export const PENDING_AUTH_REQUESTS = KeyDefinition.array( + AUTH_REQUEST_DISK_LOCAL, + "pendingAuthRequests", + { + deserializer: (json) => json, + }, +); + +export class PendingAuthRequestsStateService { + private readonly state: GlobalState; + + constructor(private readonly stateProvider: StateProvider) { + this.state = this.stateProvider.getGlobal(PENDING_AUTH_REQUESTS); + } + + getAll$(): Observable { + return this.state.state$; + } + + async add(userId: UserId): Promise { + const now = Date.now(); + await this.stateProvider.getGlobal(PENDING_AUTH_REQUESTS).update((current) => { + const list = (current ?? []).filter((e) => e.userId !== userId); + return [...list, { userId, receivedAtMs: now }]; + }); + } + + async pruneOlderThan(maxAgeMs: number): Promise { + const cutoff = Date.now() - maxAgeMs; + await this.stateProvider.getGlobal(PENDING_AUTH_REQUESTS).update((current) => { + const list = current ?? []; + return list.filter((e) => e.receivedAtMs >= cutoff); + }); + } + + async clear(userId: UserId): Promise { + await this.stateProvider.getGlobal(PENDING_AUTH_REQUESTS).update((current) => { + const list = current ?? []; + return list.filter((e) => e.userId !== userId); + }); + } +} 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 a70623783dc..9bc47437b39 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 @@ -131,6 +131,8 @@ describe("DefaultServerNotificationsService (multi-user)", () => { configService.getFeatureFlag$.mockImplementation((flag: FeatureFlag) => { const flagValueByFlag: Partial> = { [FeatureFlag.InactiveUserServerNotification]: true, + [FeatureFlag.PushNotificationsWhenLocked]: true, + [FeatureFlag.PM14938_BrowserExtensionLoginApproval]: true, }; return new BehaviorSubject(flagValueByFlag[flag] ?? false) as any; }); @@ -253,8 +255,7 @@ describe("DefaultServerNotificationsService (multi-user)", () => { .next({ type: "not-supported", reason: "test" } as any); } - // TODO: When PM-14943 goes in, uncomment - // authRequestAnsweringService.receivedPendingAuthRequest.mockResolvedValue(undefined as any); + authRequestAnsweringService.receivedPendingAuthRequest.mockResolvedValue(undefined as any); const subscription = defaultServerNotificationsService.startListening(); @@ -273,12 +274,10 @@ describe("DefaultServerNotificationsService (multi-user)", () => { expect(messagingService.send).toHaveBeenCalledWith("openLoginApproval", { notificationId: "auth-id-2", }); - - // TODO: When PM-14943 goes in, uncomment - // expect(authRequestAnsweringService.receivedPendingAuthRequest).toHaveBeenCalledWith( - // mockUserId2, - // "auth-id-2", - // ); + expect(authRequestAnsweringService.receivedPendingAuthRequest).toHaveBeenCalledWith( + mockUserId2, + "auth-id-2", + ); subscription.unsubscribe(); });