diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 39ae00a4ee4..7600d49e44a 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -41,6 +41,9 @@ import { UserVerificationService as UserVerificationServiceAbstraction } from "@ 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"; @@ -401,6 +404,7 @@ export default class MainBackground { individualVaultExportService: IndividualVaultExportServiceAbstraction; organizationVaultExportService: OrganizationVaultExportServiceAbstraction; vaultSettingsService: VaultSettingsServiceAbstraction; + pendingAuthRequestStateService: PendingAuthRequestsStateService; biometricStateService: BiometricStateService; biometricsService: BiometricsService; stateEventRunnerService: StateEventRunnerService; @@ -1129,12 +1133,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, ); diff --git a/apps/browser/src/popup/app.component.ts b/apps/browser/src/popup/app.component.ts index fa1e6c237c9..f1858a86ee0 100644 --- a/apps/browser/src/popup/app.component.ts +++ b/apps/browser/src/popup/app.component.ts @@ -23,13 +23,20 @@ import { map, } 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 { + LogoutReason, + AuthRequestServiceAbstraction, + 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 { AnimationControlService } from "@bitwarden/common/platform/abstractions/animation-control.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; @@ -122,6 +129,9 @@ 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, ) { this.deviceTrustToastService.setupListeners$.pipe(takeUntilDestroyed()).subscribe(); @@ -141,6 +151,8 @@ export class AppComponent implements OnInit, OnDestroy { this.accountService.activeAccount$.pipe(takeUntil(this.destroy$)).subscribe((account) => { this.activeUserId = account?.id; + // Re-evaluate pending auth requests when switching users while popup is open + void this.authRequestAnsweringService.processPendingAuthRequests(); }); this.authService.activeAccountStatus$ @@ -206,6 +218,43 @@ export class AppComponent implements OnInit, OnDestroy { } await this.router.navigate(["lock"]); + } else if (msg.command === "openLoginApproval") { + // 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;} + console.debug( + "[Popup AppComponent] Opening LoginApprovalDialogComponent", + req.id, + ); + 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 + ) { + console.debug( + "[Popup AppComponent] All pending auth requests responded; clearing marker for user", + this.activeUserId, + ); + await this.pendingAuthRequestsState.clearByUserId(this.activeUserId); + } + } + + console.debug( + "[Popup AppComponent] LoginApprovalDialogComponent closed", + req.id, + ); + } + } } 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 @@ -321,6 +370,7 @@ export class AppComponent implements OnInit, OnDestroy { } private async clearComponentStates() { + if (this.activeUserId == null) {return;} if (!(await firstValueFrom(this.tokenService.hasAccessToken$(this.activeUserId)))) { return; } diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index ad2f564f755..241b002c3fc 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 { 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"; @@ -939,6 +940,11 @@ const safeProviders: SafeProvider[] = [ useClass: UnsupportedSystemNotificationsService, deps: [], }), + safeProvider({ + provide: PendingAuthRequestsStateService, + useClass: PendingAuthRequestsStateService, + deps: [StateProvider], + }), safeProvider({ provide: AuthRequestAnsweringServiceAbstraction, useClass: AuthRequestAnsweringService, @@ -948,6 +954,8 @@ const safeProviders: SafeProvider[] = [ AuthServiceAbstraction, I18nServiceAbstraction, MasterPasswordServiceAbstraction, + MessagingServiceAbstraction, + PendingAuthRequestsStateService, PlatformUtilsServiceAbstraction, SystemNotificationsService, ], 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..34e1602c58f 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 @@ -1,5 +1,5 @@ import { CommonModule } from "@angular/common"; -import { Component, OnDestroy, OnInit } from "@angular/core"; +import { Component, HostListener, OnDestroy, OnInit } from "@angular/core"; import { AbstractControl, FormBuilder, @@ -160,6 +160,21 @@ export class SelfHostedEnvConfigDialogComponent implements OnInit, OnDestroy { }); } + @HostListener("document:keydown.control.b", ["$event"]) + onCtrlB(event: KeyboardEvent) { + if (process.env.ENV === "development") { + event.preventDefault(); + this.formGroup.patchValue({ + baseUrl: "", + webVaultUrl: "https://localhost:8080", + apiUrl: "http://localhost:4000", + identityUrl: "http://localhost:33656", + iconsUrl: "http://localhost:50024", + notificationsUrl: "http://localhost:61840", + }); + } + } + submit = async () => { this.showErrorSummary = false; 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..217b2f547d8 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 @@ -5,4 +5,6 @@ export abstract class AuthRequestAnsweringServiceAbstraction { abstract receivedPendingAuthRequest(userId: UserId, notificationId: string): Promise; abstract handleAuthRequestNotificationClicked(event: SystemNotificationEvent): Promise; + + abstract processPendingAuthRequests(): void; } 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 3724fd798c2..adb6f35e395 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 @@ -7,6 +7,7 @@ import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/for import { 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,8 @@ import { UserId } from "@bitwarden/user-core"; import { AuthRequestAnsweringServiceAbstraction } from "../../abstractions/auth-request-answering/auth-request-answering.service.abstraction"; +import { PendingAuthRequestsStateService } from "./pending-auth-requests.state"; + export class AuthRequestAnsweringService implements AuthRequestAnsweringServiceAbstraction { constructor( private readonly accountService: AccountService, @@ -26,10 +29,43 @@ 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 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; + + await this.pendingAuthRequestsState.pruneOlderThan(fifteenMinutesMs); + + const pending = (await firstValueFrom(this.pendingAuthRequestsState.getAll$())) ?? []; + + if (pending.length > 0) { + const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + const pendingForActive = pending.some((e) => e.userId === activeUserId); + + if (pendingForActive) { + const isUnlocked = + (await firstValueFrom(this.authService.authStatusFor$(activeUserId))) === + AuthenticationStatus.Unlocked; + + const forceSetPasswordReason = await firstValueFrom( + this.masterPasswordService.forceSetPasswordReason$(activeUserId), + ); + + if (isUnlocked && forceSetPasswordReason === ForceSetPasswordReason.None) { + console.debug("[AuthRequestAnsweringService] popupOpened - Opening popup."); + this.messagingService.send("openLoginApproval"); + } + } + } + } + async handleAuthRequestNotificationClicked(event: SystemNotificationEvent): Promise { if (event.buttonIdentifier === ButtonLocation.NotificationButton) { await this.systemNotificationsService.clear({ @@ -40,21 +76,37 @@ export class AuthRequestAnsweringService implements AuthRequestAnsweringServiceA } async receivedPendingAuthRequest(userId: UserId, authRequestId: string): Promise { + console.debug( + "[AuthRequestAnsweringService] receivedPendingAuthRequest", + { userId, authRequestId }, + ); const authStatus = await firstValueFrom(this.authService.activeAccountStatus$); const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); const forceSetPasswordReason = await firstValueFrom( this.masterPasswordService.forceSetPasswordReason$(userId), ); + const popupOpen = await this.platformUtilsService.isPopupOpen(); - // Is the popup already open? - if ( - (await this.platformUtilsService.isPopupOpen()) && + console.debug( + "[AuthRequestAnsweringService] current state", + { popupOpen, authStatus, activeUserId, forceSetPasswordReason }, + ); + + // 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 conditionsMet = + popupOpen && authStatus === AuthenticationStatus.Unlocked && activeUserId === userId && - forceSetPasswordReason === ForceSetPasswordReason.None - ) { - // TODO: Handled in 14934 - } else { + forceSetPasswordReason === ForceSetPasswordReason.None; + + if (!conditionsMet) { + console.debug( + "[AuthRequestAnsweringService] receivedPendingAuthRequest - Conditions not met, creating system notification", + ); // Get the user's email to include in the system notification const accounts = await firstValueFrom(this.accountService.accounts$); const emailForUser = accounts[userId].email; @@ -65,6 +117,11 @@ export class AuthRequestAnsweringService implements AuthRequestAnsweringServiceA body: this.i18nService.t("confirmAccessAttempt", emailForUser), buttons: [], }); + return; } + + // Popup is open and conditions are met; open dialog immediately for this request + console.debug("[AuthRequestAnsweringService] receivedPendingAuthRequest - Opening popup."); + this.messagingService.send("openLoginApproval"); } } 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..207600d8a46 --- /dev/null +++ b/libs/common/src/auth/services/auth-request-answering/pending-auth-requests.state.ts @@ -0,0 +1,52 @@ +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 clearByUserId(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/auth/services/auth-request-answering/unsupported-auth-request-answering.service.ts b/libs/common/src/auth/services/auth-request-answering/unsupported-auth-request-answering.service.ts index c4f503bd39c..b987a68ad02 100644 --- a/libs/common/src/auth/services/auth-request-answering/unsupported-auth-request-answering.service.ts +++ b/libs/common/src/auth/services/auth-request-answering/unsupported-auth-request-answering.service.ts @@ -14,4 +14,8 @@ export class UnsupportedAuthRequestAnsweringService async receivedPendingAuthRequest(userId: UserId, notificationId: string): Promise { throw new Error("Received pending auth request not supported."); } + + processPendingAuthRequests(): void { + throw new Error("Popup opened not supported."); + } } 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 69442a91630..3f789713d70 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 @@ -6,6 +6,7 @@ import { filter, firstValueFrom, map, + merge, mergeMap, Observable, share, @@ -62,20 +63,20 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer private readonly authRequestAnsweringService: AuthRequestAnsweringServiceAbstraction, private readonly configService: ConfigService, ) { - this.notifications$ = this.accountService.activeAccount$.pipe( - map((account) => account?.id), - distinctUntilChanged(), - switchMap((activeAccountId) => { - if (activeAccountId == null) { - // We don't emit server-notifications for inactive accounts currently + this.notifications$ = this.accountService.accounts$.pipe( + map((accounts) => Object.keys(accounts) as UserId[]), + switchMap((userIds) => { + if (userIds.length === 0) { return EMPTY; } - return this.userNotifications$(activeAccountId).pipe( - map((notification) => [notification, activeAccountId] as const), + const streams = userIds.map((id) => + this.userNotifications$(id).pipe(map((notification) => [notification, id] as const)), ); + + return merge(...streams); }), - share(), // Multiple subscribers should only create a single connection to the server + share(), // Multiple subscribers should only create a single connection to the server per subscriber ); } @@ -160,6 +161,20 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer return; } + // Allow-list of notification types that are safe to process for non-active users + const multiUserNotificationTypes = new Set([ + NotificationType.AuthRequest, + ]); + + const activeAccountId = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((a) => a?.id)), + ); + + const isActiveUser = activeAccountId === userId; + if (!isActiveUser && !multiUserNotificationTypes.has(notification.type)) { + return; + } + switch (notification.type) { case NotificationType.SyncCipherCreate: case NotificationType.SyncCipherUpdate: @@ -221,7 +236,7 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer case NotificationType.AuthRequest: if ( await firstValueFrom( - this.configService.getFeatureFlag$(FeatureFlag.PM14938_BrowserExtensionLoginApproval), + this.configService.getFeatureFlag$(FeatureFlag.PM14938_BrowserExtensionLoginApproval) ) ) { await this.authRequestAnsweringService.receivedPendingAuthRequest(