diff --git a/apps/browser/src/auth/services/auth-request-answering/extension-auth-request-answering.service.spec.ts b/apps/browser/src/auth/services/auth-request-answering/extension-auth-request-answering.service.spec.ts new file mode 100644 index 000000000000..9ca2522939fb --- /dev/null +++ b/apps/browser/src/auth/services/auth-request-answering/extension-auth-request-answering.service.spec.ts @@ -0,0 +1,237 @@ +import { mock, MockProxy } from "jest-mock-extended"; +import { of } from "rxjs"; + +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; +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 { PendingAuthRequestsStateService } from "@bitwarden/common/auth/services/auth-request-answering/pending-auth-requests.state"; +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 { + ButtonLocation, + SystemNotificationEvent, + SystemNotificationsService, +} from "@bitwarden/common/platform/system-notifications/system-notifications.service"; +import { LogService } from "@bitwarden/logging"; +import { UserId } from "@bitwarden/user-core"; + +import { ExtensionAuthRequestAnsweringService } from "./extension-auth-request-answering.service"; + +describe("ExtensionAuthRequestAnsweringService", () => { + let accountService: MockProxy; + let authService: MockProxy; + let masterPasswordService: any; // MasterPasswordServiceAbstraction has many members; we only use forceSetPasswordReason$ + let messagingService: MockProxy; + let pendingAuthRequestsState: MockProxy; + let actionService: MockProxy; + let i18nService: MockProxy; + let platformUtilsService: MockProxy; + let systemNotificationsService: MockProxy; + let logService: MockProxy; + + let sut: AuthRequestAnsweringService; + + const userId = "9f4c3452-6a45-48af-a7d0-74d3e8b65e4c" as UserId; + const authRequestId = "auth-request-id-123"; + + beforeEach(() => { + accountService = mock(); + authService = mock(); + masterPasswordService = { + forceSetPasswordReason$: jest.fn().mockReturnValue(of(ForceSetPasswordReason.None)), + }; + messagingService = mock(); + pendingAuthRequestsState = mock(); + actionService = mock(); + i18nService = mock(); + platformUtilsService = mock(); + systemNotificationsService = mock(); + logService = mock(); + + // Common defaults + authService.activeAccountStatus$ = of(AuthenticationStatus.Locked); + accountService.activeAccount$ = of({ + id: userId, + email: "user@example.com", + emailVerified: true, + name: "User", + }); + accountService.accounts$ = of({ + [userId]: { email: "user@example.com", emailVerified: true, name: "User" }, + }); + platformUtilsService.isPopupOpen.mockResolvedValue(false); + i18nService.t.mockImplementation( + (key: string, p1?: any) => `${key}${p1 != null ? ":" + p1 : ""}`, + ); + systemNotificationsService.create.mockResolvedValue("notif-id"); + + sut = new ExtensionAuthRequestAnsweringService( + accountService, + authService, + masterPasswordService, + messagingService, + pendingAuthRequestsState, + actionService, + i18nService, + platformUtilsService, + systemNotificationsService, + logService, + ); + }); + + describe("receivedPendingAuthRequest()", () => { + it("should throw if authRequestUserId not given", async () => { + // Act + const promise = sut.receivedPendingAuthRequest(undefined, authRequestId); + + // Assert + await expect(promise).rejects.toThrow("authRequestUserId required"); + }); + + it("should throw if authRequestId not given", async () => { + // Act + const promise = sut.receivedPendingAuthRequest(userId, undefined); + + // Assert + await expect(promise).rejects.toThrow("authRequestId required"); + }); + + it("should add a pending marker for the user to state", async () => { + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect(pendingAuthRequestsState.add).toHaveBeenCalledTimes(1); + expect(pendingAuthRequestsState.add).toHaveBeenCalledWith(userId); + }); + + describe("given the active user is the intended recipient of the auth request, unlocked, and not required to set/change their master password", () => { + describe("given the popup is open", () => { + it("should send an 'openLoginApproval' message", async () => { + // Arrange + platformUtilsService.isPopupOpen.mockResolvedValue(true); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect(messagingService.send).toHaveBeenCalledTimes(1); + expect(messagingService.send).toHaveBeenCalledWith("openLoginApproval", { + notificationId: authRequestId, + }); + }); + + it("should not create a system notification", async () => { + // Arrange + platformUtilsService.isPopupOpen.mockResolvedValue(true); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect(systemNotificationsService.create).not.toHaveBeenCalled(); + }); + }); + + describe("given the popup is closed", () => { + it("should not send an 'openLoginApproval' message", async () => { + // Arrange + platformUtilsService.isPopupOpen.mockResolvedValue(false); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect(messagingService.send).not.toHaveBeenCalled(); + }); + + it("should create a system notification", async () => { + // Arrange + platformUtilsService.isPopupOpen.mockResolvedValue(false); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect(i18nService.t).toHaveBeenCalledWith("accountAccessRequested"); + expect(i18nService.t).toHaveBeenCalledWith("confirmAccessAttempt", "user@example.com"); + expect(systemNotificationsService.create).toHaveBeenCalledWith({ + id: `${AuthServerNotificationTags.AuthRequest}_${authRequestId}`, + title: "accountAccessRequested", + body: "confirmAccessAttempt:user@example.com", + buttons: [], + }); + }); + }); + }); + }); + + describe("activeUserMeetsConditionsToShowApprovalDialog()", () => { + describe("given the active user is the intended recipient of the auth request, unlocked, and not required to set/change their master password", () => { + it("should return true if popup is open", async () => { + // Arrange + platformUtilsService.isPopupOpen.mockResolvedValue(true); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + const result = await sut.activeUserMeetsConditionsToShowApprovalDialog(userId); + + // Assert + expect(result).toBe(true); + }); + + it("should return false if popup is closed", async () => { + // Arrange + platformUtilsService.isPopupOpen.mockResolvedValue(false); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + const result = await sut.activeUserMeetsConditionsToShowApprovalDialog(userId); + + // Assert + expect(result).toBe(false); + }); + }); + }); + + describe("handleAuthRequestNotificationClicked()", () => { + it("should clear notification and open popup when notification body is clicked", async () => { + // Arrange + const event: SystemNotificationEvent = { + id: "123", + buttonIdentifier: ButtonLocation.NotificationButton, + }; + + // Act + await sut.handleAuthRequestNotificationClicked(event); + + // Assert + expect(systemNotificationsService.clear).toHaveBeenCalledWith({ id: "123" }); + expect(actionService.openPopup).toHaveBeenCalledTimes(1); + }); + + it("should do nothing when an optional notification button is clicked", async () => { + // Arrange + const event: SystemNotificationEvent = { + id: "123", + buttonIdentifier: ButtonLocation.FirstOptionalButton, + }; + + // Act + await sut.handleAuthRequestNotificationClicked(event); + + // Assert + expect(systemNotificationsService.clear).not.toHaveBeenCalled(); + expect(actionService.openPopup).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/apps/browser/src/auth/services/auth-request-answering/extension-auth-request-answering.service.ts b/apps/browser/src/auth/services/auth-request-answering/extension-auth-request-answering.service.ts new file mode 100644 index 000000000000..988de6859785 --- /dev/null +++ b/apps/browser/src/auth/services/auth-request-answering/extension-auth-request-answering.service.ts @@ -0,0 +1,116 @@ +import { firstValueFrom } from "rxjs"; + +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { AuthServerNotificationTags } from "@bitwarden/common/auth/enums/auth-server-notification-tags"; +import { DefaultAuthRequestAnsweringService } from "@bitwarden/common/auth/services/auth-request-answering/default-auth-request-answering.service"; +import { PendingAuthRequestsStateService } from "@bitwarden/common/auth/services/auth-request-answering/pending-auth-requests.state"; +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 { + ButtonLocation, + SystemNotificationEvent, + SystemNotificationsService, +} from "@bitwarden/common/platform/system-notifications/system-notifications.service"; +import { LogService } from "@bitwarden/logging"; +import { UserId } from "@bitwarden/user-core"; + +export class ExtensionAuthRequestAnsweringService + extends DefaultAuthRequestAnsweringService + implements AuthRequestAnsweringService +{ + constructor( + protected readonly accountService: AccountService, + protected readonly authService: AuthService, + protected readonly masterPasswordService: MasterPasswordServiceAbstraction, + protected readonly messagingService: MessagingService, + protected readonly pendingAuthRequestsState: PendingAuthRequestsStateService, + private readonly actionService: ActionsService, + private readonly i18nService: I18nService, + private readonly platformUtilsService: PlatformUtilsService, + private readonly systemNotificationsService: SystemNotificationsService, + private readonly logService: LogService, + ) { + super( + accountService, + authService, + masterPasswordService, + messagingService, + pendingAuthRequestsState, + ); + } + + async receivedPendingAuthRequest( + authRequestUserId: UserId, + authRequestId: string, + ): Promise { + if (!authRequestUserId) { + throw new Error("authRequestUserId required"); + } + if (!authRequestId) { + throw new Error("authRequestId required"); + } + + // Always persist the pending marker for this user to global state. + await this.pendingAuthRequestsState.add(authRequestUserId); + + const activeUserMeetsConditionsToShowApprovalDialog = + await this.activeUserMeetsConditionsToShowApprovalDialog(authRequestUserId); + + if (activeUserMeetsConditionsToShowApprovalDialog) { + // Send message to open dialog immediately for this request + this.messagingService.send("openLoginApproval", { + // Include the authRequestId so the DeviceManagementComponent can upsert the correct device. + // This will only matter if the user is on the /device-management screen when the auth request is received. + notificationId: authRequestId, + }); + } else { + // Create a system notification + const accounts = await firstValueFrom(this.accountService.accounts$); + const accountInfo = accounts[authRequestUserId]; + + if (!accountInfo) { + this.logService.error("Account not found for authRequestUserId"); + return; + } + + const emailForUser = accountInfo.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: [], + }); + } + } + + async activeUserMeetsConditionsToShowApprovalDialog(authRequestUserId: UserId): Promise { + const meetsBasicConditions = await super.activeUserMeetsConditionsToShowApprovalDialog( + authRequestUserId, + ); + + // To show an approval dialog immediately on Extension, the popup must be open. + const isPopupOpen = await this.platformUtilsService.isPopupOpen(); + const meetsExtensionConditions = meetsBasicConditions && isPopupOpen; + + return meetsExtensionConditions; + } + + /** + * 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. + */ + async handleAuthRequestNotificationClicked(event: SystemNotificationEvent): Promise { + if (event.buttonIdentifier === ButtonLocation.NotificationButton) { + await this.systemNotificationsService.clear({ + id: `${event.id}`, + }); + await this.actionService.openPopup(); + } + } +} diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index f59b66484864..920f0f02caaf 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -40,7 +40,7 @@ import { DefaultPolicyService } from "@bitwarden/common/admin-console/services/p import { PolicyApiService } from "@bitwarden/common/admin-console/services/policy/policy-api.service"; import { ProviderService } from "@bitwarden/common/admin-console/services/provider.service"; import { AccountService as AccountServiceAbstraction } from "@bitwarden/common/auth/abstractions/account.service"; -import { AuthRequestAnsweringServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; +import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; import { AuthService as AuthServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth.service"; import { AvatarService as AvatarServiceAbstraction } from "@bitwarden/common/auth/abstractions/avatar.service"; import { DevicesServiceAbstraction } from "@bitwarden/common/auth/abstractions/devices/devices.service.abstraction"; @@ -52,7 +52,6 @@ import { UserVerificationService as UserVerificationServiceAbstraction } from "@ import { AuthServerNotificationTags } from "@bitwarden/common/auth/enums/auth-server-notification-tags"; 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"; @@ -269,6 +268,7 @@ import { VaultExportServiceAbstraction, } from "@bitwarden/vault-export-core"; +import { ExtensionAuthRequestAnsweringService } from "../auth/services/auth-request-answering/extension-auth-request-answering.service"; import { AuthStatusBadgeUpdaterService } from "../auth/services/auth-status-badge-updater.service"; import { ExtensionLockService } from "../auth/services/extension-lock.service"; import { OverlayNotificationsBackground as OverlayNotificationsBackgroundInterface } from "../autofill/background/abstractions/overlay-notifications.background"; @@ -385,7 +385,7 @@ export default class MainBackground { serverNotificationsService: ServerNotificationsService; systemNotificationService: SystemNotificationsService; actionsService: ActionsService; - authRequestAnsweringService: AuthRequestAnsweringServiceAbstraction; + authRequestAnsweringService: AuthRequestAnsweringService; stateService: StateServiceAbstraction; userNotificationSettingsService: UserNotificationSettingsServiceAbstraction; autofillSettingsService: AutofillSettingsServiceAbstraction; @@ -1170,16 +1170,17 @@ export default class MainBackground { this.pendingAuthRequestStateService = new PendingAuthRequestsStateService(this.stateProvider); - this.authRequestAnsweringService = new AuthRequestAnsweringService( + this.authRequestAnsweringService = new ExtensionAuthRequestAnsweringService( this.accountService, - this.actionsService, this.authService, - this.i18nService, this.masterPasswordService, this.messagingService, this.pendingAuthRequestStateService, + this.actionsService, + this.i18nService, this.platformUtilsService, this.systemNotificationService, + this.logService, ); this.serverNotificationsService = new DefaultServerNotificationsService( diff --git a/apps/browser/src/popup/app.component.ts b/apps/browser/src/popup/app.component.ts index 8f00569b7206..e4cb8a654c46 100644 --- a/apps/browser/src/popup/app.component.ts +++ b/apps/browser/src/popup/app.component.ts @@ -14,16 +14,11 @@ import { NavigationEnd, Router, RouterOutlet } from "@angular/router"; import { catchError, concatMap, - distinctUntilChanged, filter, firstValueFrom, map, of, - pairwise, - startWith, Subject, - switchMap, - take, takeUntil, tap, } from "rxjs"; @@ -38,7 +33,7 @@ import { } from "@bitwarden/auth/common"; import { BrowserApi } from "@bitwarden/browser/platform/browser/browser-api"; 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 { AuthRequestAnsweringService } 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"; @@ -83,7 +78,8 @@ export class AppComponent implements OnInit, OnDestroy { private lastActivity: Date; private activeUserId: UserId; private routerAnimations = false; - private processingPendingAuth = false; + private processingPendingAuthRequests = false; + private shouldRerunAuthRequestProcessing = false; private destroy$ = new Subject(); @@ -118,7 +114,7 @@ export class AppComponent implements OnInit, OnDestroy { private logService: LogService, private authRequestService: AuthRequestServiceAbstraction, private pendingAuthRequestsState: PendingAuthRequestsStateService, - private authRequestAnsweringService: AuthRequestAnsweringServiceAbstraction, + private authRequestAnsweringService: AuthRequestAnsweringService, ) { this.deviceTrustToastService.setupListeners$.pipe(takeUntilDestroyed()).subscribe(); @@ -136,22 +132,7 @@ export class AppComponent implements OnInit, OnDestroy { this.activeUserId = account?.id; }); - // 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.authRequestAnsweringService.setupUnlockListenersForProcessingAuthRequests(this.destroy$); this.authService.activeAccountStatus$ .pipe( @@ -163,23 +144,6 @@ export class AppComponent implements OnInit, OnDestroy { ) .subscribe(); - // 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(); @@ -234,38 +198,31 @@ export class AppComponent implements OnInit, OnDestroy { await this.router.navigate(["lock"]); } else if (msg.command === "openLoginApproval") { - if (this.processingPendingAuth) { + if (this.processingPendingAuthRequests) { + // If an "openLoginApproval" message is received while we are currently processing other + // auth requests, then set a flag so we remember to process that new auth request + this.shouldRerunAuthRequestProcessing = true; 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); - } - } - } + + /** + * This do/while loop allows us to: + * - a) call processPendingAuthRequests() once on "openLoginApproval" + * - b) remember to re-call processPendingAuthRequests() if another "openLoginApproval" was + * received while we were processing the original auth requests + */ + do { + this.shouldRerunAuthRequestProcessing = false; + + try { + await this.processPendingAuthRequests(); + } catch (error) { + this.logService.error(`Error processing pending auth requests: ${error}`); + this.shouldRerunAuthRequestProcessing = false; // Reset flag to prevent infinite loop on persistent errors } - } finally { - this.processingPendingAuth = false; - } + // If an "openLoginApproval" message was received while processPendingAuthRequests() was running, then + // shouldRerunAuthRequestProcessing will have been set to true + } while (this.shouldRerunAuthRequestProcessing); } 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 @@ -403,4 +360,39 @@ export class AppComponent implements OnInit, OnDestroy { this.toastService.showToast(toastOptions); } + + private async processPendingAuthRequests() { + this.processingPendingAuthRequests = 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.processingPendingAuthRequests = false; + } + } } diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index eebf0a08a224..3244967617fb 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -37,6 +37,7 @@ import { SsoUrlService, LogoutService, } from "@bitwarden/auth/common"; +import { ExtensionAuthRequestAnsweringService } from "@bitwarden/browser/auth/services/auth-request-answering/extension-auth-request-answering.service"; import { ExtensionNewDeviceVerificationComponentService } from "@bitwarden/browser/auth/services/new-device-verification/extension-new-device-verification-component.service"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { EventCollectionService as EventCollectionServiceAbstraction } from "@bitwarden/common/abstractions/event/event-collection.service"; @@ -45,13 +46,12 @@ import { AccountService, AccountService as AccountServiceAbstraction, } from "@bitwarden/common/auth/abstractions/account.service"; -import { AuthRequestAnsweringServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; +import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { MasterPasswordApiService } from "@bitwarden/common/auth/abstractions/master-password-api.service.abstraction"; import { SsoLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/sso-login.service.abstraction"; 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, @@ -484,18 +484,19 @@ const safeProviders: SafeProvider[] = [ deps: [], }), safeProvider({ - provide: AuthRequestAnsweringServiceAbstraction, - useClass: AuthRequestAnsweringService, + provide: AuthRequestAnsweringService, + useClass: ExtensionAuthRequestAnsweringService, deps: [ AccountServiceAbstraction, - ActionsService, AuthService, - I18nServiceAbstraction, MasterPasswordServiceAbstraction, MessagingService, PendingAuthRequestsStateService, + ActionsService, + I18nServiceAbstraction, PlatformUtilsService, SystemNotificationsService, + LogService, ], }), safeProvider({ diff --git a/apps/desktop/src/app/app.component.ts b/apps/desktop/src/app/app.component.ts index 6243ba1e538b..18a57b38df7e 100644 --- a/apps/desktop/src/app/app.component.ts +++ b/apps/desktop/src/app/app.component.ts @@ -31,6 +31,7 @@ import { DocumentLangSetter } from "@bitwarden/angular/platform/i18n"; import { ModalService } from "@bitwarden/angular/services/modal.service"; import { FingerprintDialogComponent } from "@bitwarden/auth/angular"; import { + AuthRequestServiceAbstraction, DESKTOP_SSO_CALLBACK, LockService, LogoutReason, @@ -40,11 +41,13 @@ import { EventUploadService } from "@bitwarden/common/abstractions/event/event-u import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthRequestAnsweringService } 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 { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; +import { PendingAuthRequestsStateService } from "@bitwarden/common/auth/services/auth-request-answering/pending-auth-requests.state"; import { ProcessReloadServiceAbstraction } from "@bitwarden/common/key-management/abstractions/process-reload.service"; import { KeyConnectorService } from "@bitwarden/common/key-management/key-connector/abstractions/key-connector.service"; import { MasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; @@ -149,6 +152,8 @@ export class AppComponent implements OnInit, OnDestroy { private isIdle = false; private activeUserId: UserId = null; private activeSimpleDialog: DialogRef = null; + private processingPendingAuthRequests = false; + private shouldRerunAuthRequestProcessing = false; private destroy$ = new Subject(); @@ -197,6 +202,9 @@ export class AppComponent implements OnInit, OnDestroy { private readonly tokenService: TokenService, private desktopAutotypeDefaultSettingPolicy: DesktopAutotypeDefaultSettingPolicy, private readonly lockService: LockService, + private pendingAuthRequestsState: PendingAuthRequestsStateService, + private authRequestService: AuthRequestServiceAbstraction, + private authRequestAnsweringService: AuthRequestAnsweringService, ) { this.deviceTrustToastService.setupListeners$.pipe(takeUntilDestroyed()).subscribe(); @@ -209,6 +217,8 @@ export class AppComponent implements OnInit, OnDestroy { this.activeUserId = account?.id; }); + this.authRequestAnsweringService.setupUnlockListenersForProcessingAuthRequests(this.destroy$); + this.ngZone.runOutsideAngular(() => { setTimeout(async () => { await this.updateAppMenu(); @@ -496,13 +506,31 @@ export class AppComponent implements OnInit, OnDestroy { await this.checkForSystemTimeout(VaultTimeoutStringType.OnIdle); break; case "openLoginApproval": - if (message.notificationId != null) { - this.dialogService.closeAll(); - const dialogRef = LoginApprovalDialogComponent.open(this.dialogService, { - notificationId: message.notificationId, - }); - await firstValueFrom(dialogRef.closed); + if (this.processingPendingAuthRequests) { + // If an "openLoginApproval" message is received while we are currently processing other + // auth requests, then set a flag so we remember to process that new auth request + this.shouldRerunAuthRequestProcessing = true; + return; } + + /** + * This do/while loop allows us to: + * - a) call processPendingAuthRequests() once on "openLoginApproval" + * - b) remember to re-call processPendingAuthRequests() if another "openLoginApproval" was + * received while we were processing the original auth requests + */ + do { + this.shouldRerunAuthRequestProcessing = false; + + try { + await this.processPendingAuthRequests(); + } catch (error) { + this.logService.error(`Error processing pending auth requests: ${error}`); + this.shouldRerunAuthRequestProcessing = false; // Reset flag to prevent infinite loop on persistent errors + } + // If an "openLoginApproval" message was received while processPendingAuthRequests() was running, then + // shouldRerunAuthRequestProcessing will have been set to true + } while (this.shouldRerunAuthRequestProcessing); break; case "redrawMenu": await this.updateAppMenu(); @@ -884,4 +912,39 @@ export class AppComponent implements OnInit, OnDestroy { DeleteAccountComponent.open(this.dialogService); } + + private async processPendingAuthRequests() { + this.processingPendingAuthRequests = 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.processingPendingAuthRequests = false; + } + } } diff --git a/apps/desktop/src/app/services/services.module.ts b/apps/desktop/src/app/services/services.module.ts index 03d6eb5c9084..27736fcb4aa8 100644 --- a/apps/desktop/src/app/services/services.module.ts +++ b/apps/desktop/src/app/services/services.module.ts @@ -5,7 +5,6 @@ import { Router } from "@angular/router"; import { Subject, merge } from "rxjs"; import { OrganizationUserApiService } from "@bitwarden/admin-console/common"; -import { LoginApprovalDialogComponentServiceAbstraction } from "@bitwarden/angular/auth/login-approval"; import { SetInitialPasswordService } from "@bitwarden/angular/auth/password-management/set-initial-password/set-initial-password.service.abstraction"; import { SafeProvider, safeProvider } from "@bitwarden/angular/platform/utils/safe-provider"; import { @@ -45,12 +44,14 @@ import { AccountService, AccountService as AccountServiceAbstraction, } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; import { AuthService, AuthService as AuthServiceAbstraction, } from "@bitwarden/common/auth/abstractions/auth.service"; import { MasterPasswordApiService } from "@bitwarden/common/auth/abstractions/master-password-api.service.abstraction"; import { SsoLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/sso-login.service.abstraction"; +import { PendingAuthRequestsStateService } from "@bitwarden/common/auth/services/auth-request-answering/pending-auth-requests.state"; import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/autofill-settings.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions"; import { ClientType } from "@bitwarden/common/enums"; @@ -59,7 +60,10 @@ import { KeyGenerationService } from "@bitwarden/common/key-management/crypto"; import { CryptoFunctionService as CryptoFunctionServiceAbstraction } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { WebCryptoFunctionService } from "@bitwarden/common/key-management/crypto/services/web-crypto-function.service"; -import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; +import { + InternalMasterPasswordServiceAbstraction, + MasterPasswordServiceAbstraction, +} from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; import { PinServiceAbstraction } from "@bitwarden/common/key-management/pin/pin.service.abstraction"; import { DefaultProcessReloadService } from "@bitwarden/common/key-management/services/default-process-reload.service"; import { @@ -116,8 +120,8 @@ import { import { SerializedMemoryStorageService } from "@bitwarden/storage-core"; import { DefaultSshImportPromptService, SshImportPromptService } from "@bitwarden/vault"; -import { DesktopLoginApprovalDialogComponentService } from "../../auth/login/desktop-login-approval-dialog-component.service"; import { DesktopLoginComponentService } from "../../auth/login/desktop-login-component.service"; +import { DesktopAuthRequestAnsweringService } from "../../auth/services/auth-request-answering/desktop-auth-request-answering.service"; import { DesktopTwoFactorAuthDuoComponentService } from "../../auth/services/desktop-two-factor-auth-duo-component.service"; import { DesktopAutofillSettingsService } from "../../autofill/services/desktop-autofill-settings.service"; import { DesktopAutofillService } from "../../autofill/services/desktop-autofill.service"; @@ -455,11 +459,6 @@ const safeProviders: SafeProvider[] = [ useClass: DefaultSsoComponentService, deps: [], }), - safeProvider({ - provide: LoginApprovalDialogComponentServiceAbstraction, - useClass: DesktopLoginApprovalDialogComponentService, - deps: [I18nServiceAbstraction], - }), safeProvider({ provide: SshImportPromptService, useClass: DefaultSshImportPromptService, @@ -489,6 +488,19 @@ const safeProviders: SafeProvider[] = [ useClass: DesktopSessionTimeoutSettingsComponentService, deps: [I18nServiceAbstraction], }), + safeProvider({ + provide: AuthRequestAnsweringService, + useClass: DesktopAuthRequestAnsweringService, + deps: [ + AccountServiceAbstraction, + AuthService, + MasterPasswordServiceAbstraction, + MessagingServiceAbstraction, + PendingAuthRequestsStateService, + I18nServiceAbstraction, + LogService, + ], + }), ]; @NgModule({ diff --git a/apps/desktop/src/auth/login/desktop-login-approval-dialog-component.service.spec.ts b/apps/desktop/src/auth/login/desktop-login-approval-dialog-component.service.spec.ts deleted file mode 100644 index 2ae584d7e7f7..000000000000 --- a/apps/desktop/src/auth/login/desktop-login-approval-dialog-component.service.spec.ts +++ /dev/null @@ -1,91 +0,0 @@ -import { TestBed } from "@angular/core/testing"; -import { mock, MockProxy } from "jest-mock-extended"; -import { Subject } from "rxjs"; - -import { LoginApprovalDialogComponent } from "@bitwarden/angular/auth/login-approval"; -import { I18nService as I18nServiceAbstraction } from "@bitwarden/common/platform/abstractions/i18n.service"; - -import { DesktopLoginApprovalDialogComponentService } from "./desktop-login-approval-dialog-component.service"; - -describe("DesktopLoginApprovalDialogComponentService", () => { - let service: DesktopLoginApprovalDialogComponentService; - let i18nService: MockProxy; - let originalIpc: any; - - beforeEach(() => { - originalIpc = (global as any).ipc; - (global as any).ipc = { - auth: { - loginRequest: jest.fn(), - }, - platform: { - isWindowVisible: jest.fn(), - }, - }; - - i18nService = mock({ - t: jest.fn(), - userSetLocale$: new Subject(), - locale$: new Subject(), - }); - - TestBed.configureTestingModule({ - providers: [ - DesktopLoginApprovalDialogComponentService, - { provide: I18nServiceAbstraction, useValue: i18nService }, - ], - }); - - service = TestBed.inject(DesktopLoginApprovalDialogComponentService); - }); - - afterEach(() => { - jest.clearAllMocks(); - (global as any).ipc = originalIpc; - }); - - it("is created successfully", () => { - expect(service).toBeTruthy(); - }); - - it("calls ipc.auth.loginRequest with correct parameters when window is not visible", async () => { - const title = "Log in requested"; - const email = "test@bitwarden.com"; - const message = `Confirm access attempt for ${email}`; - const closeText = "Close"; - - const loginApprovalDialogComponent = { email } as LoginApprovalDialogComponent; - i18nService.t.mockImplementation((key: string) => { - switch (key) { - case "accountAccessRequested": - return title; - case "confirmAccessAttempt": - return message; - case "close": - return closeText; - default: - return ""; - } - }); - - jest.spyOn(ipc.platform, "isWindowVisible").mockResolvedValue(false); - jest.spyOn(ipc.auth, "loginRequest").mockResolvedValue(); - - await service.showLoginRequestedAlertIfWindowNotVisible(loginApprovalDialogComponent.email); - - expect(ipc.auth.loginRequest).toHaveBeenCalledWith(title, message, closeText); - }); - - it("does not call ipc.auth.loginRequest when window is visible", async () => { - const loginApprovalDialogComponent = { - email: "test@bitwarden.com", - } as LoginApprovalDialogComponent; - - jest.spyOn(ipc.platform, "isWindowVisible").mockResolvedValue(true); - jest.spyOn(ipc.auth, "loginRequest"); - - await service.showLoginRequestedAlertIfWindowNotVisible(loginApprovalDialogComponent.email); - - expect(ipc.auth.loginRequest).not.toHaveBeenCalled(); - }); -}); diff --git a/apps/desktop/src/auth/login/desktop-login-approval-dialog-component.service.ts b/apps/desktop/src/auth/login/desktop-login-approval-dialog-component.service.ts deleted file mode 100644 index 9c48f71990a9..000000000000 --- a/apps/desktop/src/auth/login/desktop-login-approval-dialog-component.service.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { Injectable } from "@angular/core"; - -import { - DefaultLoginApprovalDialogComponentService, - LoginApprovalDialogComponentServiceAbstraction, -} from "@bitwarden/angular/auth/login-approval"; -import { I18nService as I18nServiceAbstraction } from "@bitwarden/common/platform/abstractions/i18n.service"; - -@Injectable() -export class DesktopLoginApprovalDialogComponentService - extends DefaultLoginApprovalDialogComponentService - implements LoginApprovalDialogComponentServiceAbstraction -{ - constructor(private i18nService: I18nServiceAbstraction) { - super(); - } - - async showLoginRequestedAlertIfWindowNotVisible(email?: string): Promise { - const isVisible = await ipc.platform.isWindowVisible(); - if (!isVisible) { - await ipc.auth.loginRequest( - this.i18nService.t("accountAccessRequested"), - this.i18nService.t("confirmAccessAttempt", email), - this.i18nService.t("close"), - ); - } - } -} diff --git a/apps/desktop/src/auth/services/auth-request-answering/desktop-auth-request-answering.service.spec.ts b/apps/desktop/src/auth/services/auth-request-answering/desktop-auth-request-answering.service.spec.ts new file mode 100644 index 000000000000..aa2f17b8dee1 --- /dev/null +++ b/apps/desktop/src/auth/services/auth-request-answering/desktop-auth-request-answering.service.spec.ts @@ -0,0 +1,271 @@ +import { mock, MockProxy } from "jest-mock-extended"; +import { of } from "rxjs"; + +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; +import { PendingAuthRequestsStateService } from "@bitwarden/common/auth/services/auth-request-answering/pending-auth-requests.state"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { LogService } from "@bitwarden/logging"; +import { UserId } from "@bitwarden/user-core"; + +import { DesktopAuthRequestAnsweringService } from "./desktop-auth-request-answering.service"; + +describe("DesktopAuthRequestAnsweringService", () => { + let accountService: MockProxy; + let authService: MockProxy; + let masterPasswordService: any; // MasterPasswordServiceAbstraction has many members; we only use forceSetPasswordReason$ + let messagingService: MockProxy; + let pendingAuthRequestsState: MockProxy; + let i18nService: MockProxy; + let logService: MockProxy; + + let sut: AuthRequestAnsweringService; + + const userId = "9f4c3452-6a45-48af-a7d0-74d3e8b65e4c" as UserId; + const authRequestId = "auth-request-id-123"; + + beforeEach(() => { + (global as any).ipc = { + platform: { + isWindowVisible: jest.fn(), + }, + auth: { + loginRequest: jest.fn(), + }, + }; + + accountService = mock(); + authService = mock(); + masterPasswordService = { + forceSetPasswordReason$: jest.fn().mockReturnValue(of(ForceSetPasswordReason.None)), + }; + messagingService = mock(); + pendingAuthRequestsState = mock(); + i18nService = mock(); + logService = mock(); + + // Common defaults + authService.activeAccountStatus$ = of(AuthenticationStatus.Locked); + accountService.activeAccount$ = of({ + id: userId, + email: "user@example.com", + emailVerified: true, + name: "User", + }); + accountService.accounts$ = of({ + [userId]: { email: "user@example.com", emailVerified: true, name: "User" }, + }); + (global as any).ipc.platform.isWindowVisible.mockResolvedValue(false); + i18nService.t.mockImplementation( + (key: string, p1?: any) => `${key}${p1 != null ? ":" + p1 : ""}`, + ); + + sut = new DesktopAuthRequestAnsweringService( + accountService, + authService, + masterPasswordService, + messagingService, + pendingAuthRequestsState, + i18nService, + logService, + ); + }); + + describe("receivedPendingAuthRequest()", () => { + it("should throw if authRequestUserId not given", async () => { + // Act + const promise = sut.receivedPendingAuthRequest(undefined, undefined); + + // Assert + await expect(promise).rejects.toThrow("authRequestUserId required"); + }); + + it("should add a pending marker for the user to state", async () => { + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect(pendingAuthRequestsState.add).toHaveBeenCalledTimes(1); + expect(pendingAuthRequestsState.add).toHaveBeenCalledWith(userId); + }); + + describe("given the active user is the intended recipient of the auth request, unlocked, and not required to set/change their master password", () => { + describe("given the Desktop window is visible", () => { + it("should send an 'openLoginApproval' message", async () => { + // Arrange + (global as any).ipc.platform.isWindowVisible.mockResolvedValue(true); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect(messagingService.send).toHaveBeenCalledTimes(1); + expect(messagingService.send).toHaveBeenCalledWith("openLoginApproval"); + }); + + it("should NOT create a system notification", async () => { + // Arrange + (global as any).ipc.platform.isWindowVisible.mockResolvedValue(true); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect((global as any).ipc.auth.loginRequest).not.toHaveBeenCalled(); + }); + }); + + describe("given the Desktop window is NOT visible", () => { + it("should STILL send an 'openLoginApproval' message", async () => { + // Arrange + (global as any).ipc.platform.isWindowVisible.mockResolvedValue(false); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect(messagingService.send).toHaveBeenCalledTimes(1); + expect(messagingService.send).toHaveBeenCalledWith("openLoginApproval"); + }); + + it("should create a system notification", async () => { + // Arrange + (global as any).ipc.platform.isWindowVisible.mockResolvedValue(false); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect(i18nService.t).toHaveBeenCalledWith("accountAccessRequested"); + expect(i18nService.t).toHaveBeenCalledWith("confirmAccessAttempt", "user@example.com"); + expect(i18nService.t).toHaveBeenCalledWith("close"); + + expect((global as any).ipc.auth.loginRequest).toHaveBeenCalledWith( + "accountAccessRequested", + "confirmAccessAttempt:user@example.com", + "close", + ); + }); + }); + }); + + describe("given the active user is Locked", () => { + it("should NOT send an 'openLoginApproval' message", async () => { + // Arrange + (global as any).ipc.platform.isWindowVisible.mockResolvedValue(true); + authService.activeAccountStatus$ = of(AuthenticationStatus.Locked); + + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect(messagingService.send).not.toHaveBeenCalled(); + }); + + it("should create a system notification", async () => { + // Arrange + (global as any).ipc.platform.isWindowVisible.mockResolvedValue(true); + authService.activeAccountStatus$ = of(AuthenticationStatus.Locked); + + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect((global as any).ipc.auth.loginRequest).toHaveBeenCalledWith( + "accountAccessRequested", + "confirmAccessAttempt:user@example.com", + "close", + ); + }); + }); + + describe("given the active user is not the intended recipient of the auth request", () => { + beforeEach(() => { + // Different active user for these tests + const differentUserId = "different-user-id" as UserId; + accountService.activeAccount$ = of({ + id: differentUserId, + email: "different@example.com", + emailVerified: true, + name: "Different User", + }); + }); + + it("should NOT send an 'openLoginApproval' message", async () => { + // Arrange + (global as any).ipc.platform.isWindowVisible.mockResolvedValue(true); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + // Pass in userId, not differentUserId (the active user), to mimic an auth + // request coming in for a user who is not the active user + await sut.receivedPendingAuthRequest(userId, authRequestId); // pass in userId, not differentUserId + + // Assert + expect(messagingService.send).not.toHaveBeenCalled(); + }); + + it("should create a system notification", async () => { + // Arrange + (global as any).ipc.platform.isWindowVisible.mockResolvedValue(true); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + // Pass in userId, not differentUserId (the active user), to mimic an auth + // request coming in for a user who is not the active user + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect((global as any).ipc.auth.loginRequest).toHaveBeenCalledWith( + "accountAccessRequested", + "confirmAccessAttempt:user@example.com", + "close", + ); + }); + }); + + describe("given the active user is required to set/change their master password", () => { + it("should NOT send an 'openLoginApproval' message", async () => { + // Arrange + (global as any).ipc.platform.isWindowVisible.mockResolvedValue(true); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + masterPasswordService.forceSetPasswordReason$ = jest + .fn() + .mockReturnValue(of(ForceSetPasswordReason.WeakMasterPassword)); + + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect(messagingService.send).not.toHaveBeenCalled(); + }); + + it("should create a system notification", async () => { + // Arrange + (global as any).ipc.platform.isWindowVisible.mockResolvedValue(true); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + masterPasswordService.forceSetPasswordReason$ = jest + .fn() + .mockReturnValue(of(ForceSetPasswordReason.WeakMasterPassword)); + + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect((global as any).ipc.auth.loginRequest).toHaveBeenCalledWith( + "accountAccessRequested", + "confirmAccessAttempt:user@example.com", + "close", + ); + }); + }); + }); +}); diff --git a/apps/desktop/src/auth/services/auth-request-answering/desktop-auth-request-answering.service.ts b/apps/desktop/src/auth/services/auth-request-answering/desktop-auth-request-answering.service.ts new file mode 100644 index 000000000000..3b2602660fe9 --- /dev/null +++ b/apps/desktop/src/auth/services/auth-request-answering/desktop-auth-request-answering.service.ts @@ -0,0 +1,85 @@ +import { firstValueFrom } from "rxjs"; + +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { DefaultAuthRequestAnsweringService } from "@bitwarden/common/auth/services/auth-request-answering/default-auth-request-answering.service"; +import { PendingAuthRequestsStateService } from "@bitwarden/common/auth/services/auth-request-answering/pending-auth-requests.state"; +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 { LogService } from "@bitwarden/logging"; +import { UserId } from "@bitwarden/user-core"; + +export class DesktopAuthRequestAnsweringService + extends DefaultAuthRequestAnsweringService + implements AuthRequestAnsweringService +{ + constructor( + protected readonly accountService: AccountService, + protected readonly authService: AuthService, + protected readonly masterPasswordService: MasterPasswordServiceAbstraction, + protected readonly messagingService: MessagingService, + protected readonly pendingAuthRequestsState: PendingAuthRequestsStateService, + private readonly i18nService: I18nService, + private readonly logService: LogService, + ) { + super( + accountService, + authService, + masterPasswordService, + messagingService, + pendingAuthRequestsState, + ); + } + + /** + * @param authRequestUserId The UserId that the auth request is for. + * @param authRequestId The authRequestId param is not used on Desktop because clicks on a + * Desktop notification do not run any auth-request-specific actions. + * All clicks simply open the Desktop window. See electron-main-messaging.service.ts. + */ + async receivedPendingAuthRequest( + authRequestUserId: UserId, + authRequestId: string, + ): Promise { + if (!authRequestUserId) { + throw new Error("authRequestUserId required"); + } + + // Always persist the pending marker for this user to global state. + await this.pendingAuthRequestsState.add(authRequestUserId); + + const activeUserMeetsConditionsToShowApprovalDialog = + await this.activeUserMeetsConditionsToShowApprovalDialog(authRequestUserId); + + if (activeUserMeetsConditionsToShowApprovalDialog) { + // Send message to open dialog immediately for this request + this.messagingService.send("openLoginApproval"); + } + + const isWindowVisible = await ipc.platform.isWindowVisible(); + + // Create a system notification if either of the following are true: + // - User does NOT meet conditions to show dialog + // - User does meet conditions, but the Desktop window is not visible + // - In this second case, we both send the "openLoginApproval" message (above) AND + // also create the system notification to notify the user that the dialog is there. + if (!activeUserMeetsConditionsToShowApprovalDialog || !isWindowVisible) { + const accounts = await firstValueFrom(this.accountService.accounts$); + const accountInfo = accounts[authRequestUserId]; + + if (!accountInfo) { + this.logService.error("Account not found for authRequestUserId"); + return; + } + + const emailForUser = accountInfo.email; + await ipc.auth.loginRequest( + this.i18nService.t("accountAccessRequested"), + this.i18nService.t("confirmAccessAttempt", emailForUser), + this.i18nService.t("close"), + ); + } + } +} 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 bdade04bacdb..fca81c449d90 100644 --- a/apps/desktop/src/vault/app/vault/vault-v2.component.ts +++ b/apps/desktop/src/vault/app/vault/vault-v2.component.ts @@ -16,7 +16,6 @@ import { CollectionService, CollectionView } from "@bitwarden/admin-console/comm import { PremiumBadgeComponent } from "@bitwarden/angular/billing/components/premium-badge"; import { VaultViewPasswordHistoryService } from "@bitwarden/angular/services/view-password-history.service"; import { VaultFilter } from "@bitwarden/angular/vault/vault-filter/models/vault-filter.model"; -import { AuthRequestServiceAbstraction } from "@bitwarden/auth/common"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; @@ -24,11 +23,12 @@ import { PolicyService } from "@bitwarden/common/admin-console/abstractions/poli import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { Account, AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; 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 { MasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; 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"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; @@ -223,11 +223,10 @@ export class VaultV2Component private collectionService: CollectionService, private organizationService: OrganizationService, private folderService: FolderService, - private configService: ConfigService, - private authRequestService: AuthRequestServiceAbstraction, private cipherArchiveService: CipherArchiveService, private policyService: PolicyService, private archiveCipherUtilitiesService: ArchiveCipherUtilitiesService, + private masterPasswordService: MasterPasswordServiceAbstraction, ) {} async ngOnInit() { @@ -337,19 +336,12 @@ export class VaultV2Component this.searchBarService.setEnabled(true); this.searchBarService.setPlaceholderText(this.i18nService.t("searchVault")); - const authRequests = await firstValueFrom( - this.authRequestService.getLatestPendingAuthRequest$()!, - ); - if (authRequests != null) { - this.messagingService.send("openLoginApproval", { - notificationId: authRequests.id, - }); - } - this.activeUserId = await firstValueFrom( this.accountService.activeAccount$.pipe(getUserId), ).catch((): any => null); + await this.sendOpenLoginApprovalMessage(this.activeUserId); + if (this.activeUserId) { this.cipherService .failedToDecryptCiphers$(this.activeUserId) @@ -1015,4 +1007,27 @@ export class VaultV2Component } return repromptResult; } + + /** + * Sends a message that will retrieve any pending auth requests from the server. If there are + * pending auth requests for this user, a LoginApprovalDialogComponent will open. If there + * are no pending auth requests, nothing happens (see AppComponent: "openLoginApproval"). + */ + private async sendOpenLoginApprovalMessage(activeUserId: UserId) { + // This is a defensive check against a race condition where a user may have successfully logged + // in with no forceSetPasswordReason, but while the vault component is loading, a sync sets + // forceSetPasswordReason.TdeUserWithoutPasswordHasPasswordResetPermission (see DefaultSyncService). + // This could potentially happen if an Admin upgrades the user's permissions as the user is logging + // in. In this rare case we do not want to send an "openLoginApproval" message. + // + // This also keeps parity with other usages of the "openLoginApproval" message. That is: don't send + // an "openLoginApproval" message if the user is required to set/change their password. + const forceSetPasswordReason = await firstValueFrom( + this.masterPasswordService.forceSetPasswordReason$(activeUserId), + ); + if (forceSetPasswordReason === ForceSetPasswordReason.None) { + // If there are pending auth requests for this user, a LoginApprovalDialogComponent will open + this.messagingService.send("openLoginApproval"); + } + } } diff --git a/apps/web/src/app/core/core.module.ts b/apps/web/src/app/core/core.module.ts index c0716d99716b..071dcd81b68c 100644 --- a/apps/web/src/app/core/core.module.ts +++ b/apps/web/src/app/core/core.module.ts @@ -59,9 +59,11 @@ import { } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { AccountApiService as AccountApiServiceAbstraction } from "@bitwarden/common/auth/abstractions/account-api.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { MasterPasswordApiService } from "@bitwarden/common/auth/abstractions/master-password-api.service.abstraction"; import { SsoLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/sso-login.service.abstraction"; +import { NoopAuthRequestAnsweringService } from "@bitwarden/common/auth/services/auth-request-answering/noop-auth-request-answering.service"; import { OrganizationInviteService } from "@bitwarden/common/auth/services/organization-invite/organization-invite.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { ClientType } from "@bitwarden/common/enums"; @@ -474,6 +476,11 @@ const safeProviders: SafeProvider[] = [ useClass: WebSessionTimeoutSettingsComponentService, deps: [I18nServiceAbstraction, PlatformUtilsService], }), + safeProvider({ + provide: AuthRequestAnsweringService, + useClass: NoopAuthRequestAnsweringService, + deps: [], + }), ]; @NgModule({ diff --git a/libs/angular/src/auth/login-approval/default-login-approval-dialog-component.service.spec.ts b/libs/angular/src/auth/login-approval/default-login-approval-dialog-component.service.spec.ts deleted file mode 100644 index 018b1ce25473..000000000000 --- a/libs/angular/src/auth/login-approval/default-login-approval-dialog-component.service.spec.ts +++ /dev/null @@ -1,30 +0,0 @@ -import { TestBed } from "@angular/core/testing"; - -import { DefaultLoginApprovalDialogComponentService } from "./default-login-approval-dialog-component.service"; -import { LoginApprovalDialogComponent } from "./login-approval-dialog.component"; - -describe("DefaultLoginApprovalDialogComponentService", () => { - let service: DefaultLoginApprovalDialogComponentService; - - beforeEach(() => { - TestBed.configureTestingModule({ - providers: [DefaultLoginApprovalDialogComponentService], - }); - - service = TestBed.inject(DefaultLoginApprovalDialogComponentService); - }); - - it("is created successfully", () => { - expect(service).toBeTruthy(); - }); - - it("has showLoginRequestedAlertIfWindowNotVisible method that is a no-op", async () => { - const loginApprovalDialogComponent = {} as LoginApprovalDialogComponent; - - const result = await service.showLoginRequestedAlertIfWindowNotVisible( - loginApprovalDialogComponent.email, - ); - - expect(result).toBeUndefined(); - }); -}); diff --git a/libs/angular/src/auth/login-approval/default-login-approval-dialog-component.service.ts b/libs/angular/src/auth/login-approval/default-login-approval-dialog-component.service.ts deleted file mode 100644 index 5fefd3c3abbb..000000000000 --- a/libs/angular/src/auth/login-approval/default-login-approval-dialog-component.service.ts +++ /dev/null @@ -1,16 +0,0 @@ -import { LoginApprovalDialogComponentServiceAbstraction } from "./login-approval-dialog-component.service.abstraction"; - -/** - * Default implementation of the LoginApprovalDialogComponentServiceAbstraction. - */ -export class DefaultLoginApprovalDialogComponentService - implements LoginApprovalDialogComponentServiceAbstraction -{ - /** - * No-op implementation of the showLoginRequestedAlertIfWindowNotVisible method. - * @returns - */ - async showLoginRequestedAlertIfWindowNotVisible(email?: string): Promise { - return; - } -} diff --git a/libs/angular/src/auth/login-approval/index.ts b/libs/angular/src/auth/login-approval/index.ts index 7b34b17d56b9..21cde4df7426 100644 --- a/libs/angular/src/auth/login-approval/index.ts +++ b/libs/angular/src/auth/login-approval/index.ts @@ -1,3 +1 @@ export * from "./login-approval-dialog.component"; -export * from "./login-approval-dialog-component.service.abstraction"; -export * from "./default-login-approval-dialog-component.service"; diff --git a/libs/angular/src/auth/login-approval/login-approval-dialog-component.service.abstraction.ts b/libs/angular/src/auth/login-approval/login-approval-dialog-component.service.abstraction.ts deleted file mode 100644 index f29311402a70..000000000000 --- a/libs/angular/src/auth/login-approval/login-approval-dialog-component.service.abstraction.ts +++ /dev/null @@ -1,9 +0,0 @@ -/** - * Abstraction for the LoginApprovalDialogComponent service. - */ -export abstract class LoginApprovalDialogComponentServiceAbstraction { - /** - * Shows a login requested alert if the window is not visible. - */ - abstract showLoginRequestedAlertIfWindowNotVisible: (email?: string) => Promise; -} diff --git a/libs/angular/src/auth/login-approval/login-approval-dialog.component.spec.ts b/libs/angular/src/auth/login-approval/login-approval-dialog.component.spec.ts index b21264eb7c84..d17fefaca4f2 100644 --- a/libs/angular/src/auth/login-approval/login-approval-dialog.component.spec.ts +++ b/libs/angular/src/auth/login-approval/login-approval-dialog.component.spec.ts @@ -15,7 +15,6 @@ import { UserId } from "@bitwarden/common/types/guid"; import { DialogRef, DIALOG_DATA, ToastService } from "@bitwarden/components"; import { LogService } from "@bitwarden/logging"; -import { LoginApprovalDialogComponentServiceAbstraction } from "./login-approval-dialog-component.service.abstraction"; import { LoginApprovalDialogComponent } from "./login-approval-dialog.component"; describe("LoginApprovalDialogComponent", () => { @@ -67,10 +66,6 @@ describe("LoginApprovalDialogComponent", () => { { provide: LogService, useValue: logService }, { provide: ToastService, useValue: toastService }, { provide: ValidationService, useValue: validationService }, - { - provide: LoginApprovalDialogComponentServiceAbstraction, - useValue: mock(), - }, ], }).compileComponents(); diff --git a/libs/angular/src/auth/login-approval/login-approval-dialog.component.ts b/libs/angular/src/auth/login-approval/login-approval-dialog.component.ts index 35333c43536b..54906047535f 100644 --- a/libs/angular/src/auth/login-approval/login-approval-dialog.component.ts +++ b/libs/angular/src/auth/login-approval/login-approval-dialog.component.ts @@ -24,8 +24,6 @@ import { } from "@bitwarden/components"; import { LogService } from "@bitwarden/logging"; -import { LoginApprovalDialogComponentServiceAbstraction } from "./login-approval-dialog-component.service.abstraction"; - const RequestTimeOut = 60000 * 15; // 15 Minutes const RequestTimeUpdate = 60000 * 5; // 5 Minutes @@ -57,7 +55,6 @@ export class LoginApprovalDialogComponent implements OnInit, OnDestroy { private devicesService: DevicesServiceAbstraction, private dialogRef: DialogRef, private i18nService: I18nService, - private loginApprovalDialogComponentService: LoginApprovalDialogComponentServiceAbstraction, private logService: LogService, private toastService: ToastService, private validationService: ValidationService, @@ -113,10 +110,6 @@ export class LoginApprovalDialogComponent implements OnInit, OnDestroy { this.updateTimeText(); }, RequestTimeUpdate); - await this.loginApprovalDialogComponentService.showLoginRequestedAlertIfWindowNotVisible( - this.email, - ); - this.loading = false; } diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index b1215654cfde..9099c8e884ff 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -93,7 +93,7 @@ import { InternalAccountService, } from "@bitwarden/common/auth/abstractions/account.service"; import { AnonymousHubService as AnonymousHubServiceAbstraction } from "@bitwarden/common/auth/abstractions/anonymous-hub.service"; -import { AuthRequestAnsweringServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; +import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; import { AuthService as AuthServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth.service"; import { AvatarService as AvatarServiceAbstraction } from "@bitwarden/common/auth/abstractions/avatar.service"; import { DevicesServiceAbstraction } from "@bitwarden/common/auth/abstractions/devices/devices.service.abstraction"; @@ -112,7 +112,7 @@ import { SendTokenService, DefaultSendTokenService } from "@bitwarden/common/aut import { AccountApiServiceImplementation } from "@bitwarden/common/auth/services/account-api.service"; 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 { DefaultAuthRequestAnsweringService } from "@bitwarden/common/auth/services/auth-request-answering/default-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"; @@ -383,8 +383,6 @@ import { VaultExportServiceAbstraction, } from "@bitwarden/vault-export-core"; -import { DefaultLoginApprovalDialogComponentService } from "../auth/login-approval/default-login-approval-dialog-component.service"; -import { LoginApprovalDialogComponentServiceAbstraction } from "../auth/login-approval/login-approval-dialog-component.service.abstraction"; import { DefaultSetInitialPasswordService } from "../auth/password-management/set-initial-password/default-set-initial-password.service.implementation"; import { SetInitialPasswordService } from "../auth/password-management/set-initial-password/set-initial-password.service.abstraction"; import { DeviceTrustToastService as DeviceTrustToastServiceAbstraction } from "../auth/services/device-trust-toast.service.abstraction"; @@ -999,9 +997,15 @@ const safeProviders: SafeProvider[] = [ deps: [StateProvider], }), safeProvider({ - provide: AuthRequestAnsweringServiceAbstraction, - useClass: NoopAuthRequestAnsweringService, - deps: [], + provide: AuthRequestAnsweringService, + useClass: DefaultAuthRequestAnsweringService, + deps: [ + AccountServiceAbstraction, + AuthServiceAbstraction, + MasterPasswordServiceAbstraction, + MessagingServiceAbstraction, + PendingAuthRequestsStateService, + ], }), safeProvider({ provide: ServerNotificationsService, @@ -1019,7 +1023,7 @@ const safeProviders: SafeProvider[] = [ SignalRConnectionService, AuthServiceAbstraction, WebPushConnectionService, - AuthRequestAnsweringServiceAbstraction, + AuthRequestAnsweringService, ConfigService, ], }), @@ -1603,11 +1607,6 @@ const safeProviders: SafeProvider[] = [ useClass: DefaultSendPasswordService, deps: [CryptoFunctionServiceAbstraction], }), - safeProvider({ - provide: LoginApprovalDialogComponentServiceAbstraction, - useClass: DefaultLoginApprovalDialogComponentService, - deps: [], - }), safeProvider({ provide: LoginDecryptionOptionsService, useClass: DefaultLoginDecryptionOptionsService, diff --git a/libs/common/src/auth/abstractions/auth-request-answering/README.md b/libs/common/src/auth/abstractions/auth-request-answering/README.md index 9a24f095d702..edc368fd91b9 100644 --- a/libs/common/src/auth/abstractions/auth-request-answering/README.md +++ b/libs/common/src/auth/abstractions/auth-request-answering/README.md @@ -1,7 +1,6 @@ # 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. +This feature is to allow for the taking of auth requests that are received via websockets to be acted on when the user loads up a client. See diagram for the high level picture of how this is wired up. 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 f45cb34496e8..7edae163951d 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 @@ -1,30 +1,50 @@ +import { Observable } from "rxjs"; + import { SystemNotificationEvent } from "@bitwarden/common/platform/system-notifications/system-notifications.service"; import { UserId } from "@bitwarden/user-core"; -export abstract class AuthRequestAnsweringServiceAbstraction { +export abstract class AuthRequestAnsweringService { /** * 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. + * - Implemented on Extension and Desktop. * - * Currently, this is only implemented for browser extension. - * - * @param userId The UserId that the auth request is for. + * @param authRequestUserId 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; + abstract receivedPendingAuthRequest?( + authRequestUserId: UserId, + authRequestId: string, + ): Promise; /** - * When a system notification is clicked, this function is used to process that event. + * Confirms whether or not the user meets the conditions required to show them an + * approval dialog immediately. * - * @param event The event passed in. Check initNotificationSubscriptions in main.background.ts. + * @param authRequestUserId the UserId that the auth request is for. + * @returns boolean stating whether or not the user meets conditions */ - abstract handleAuthRequestNotificationClicked(event: SystemNotificationEvent): Promise; + abstract activeUserMeetsConditionsToShowApprovalDialog( + authRequestUserId: UserId, + ): Promise; /** - * Process notifications that have been received but didn't meet the conditions to display the - * approval dialog. + * Sets up listeners for scenarios where the user unlocks and we want to process + * any pending auth requests in state. + * + * @param destroy$ The destroy$ observable from the caller */ - abstract processPendingAuthRequests(): Promise; + abstract setupUnlockListenersForProcessingAuthRequests(destroy$: Observable): void; + + /** + * When a system notification is clicked, this method is used to process that event. + * - Implemented on Extension only. + * - Desktop does not implement this method because click handling is already setup in + * electron-main-messaging.service.ts. + * + * @param event The event passed in. Check initNotificationSubscriptions in main.background.ts. + */ + abstract handleAuthRequestNotificationClicked(event: SystemNotificationEvent): Promise; } 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 deleted file mode 100644 index 0b12e1cb6615..000000000000 --- a/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.spec.ts +++ /dev/null @@ -1,139 +0,0 @@ -import { mock, MockProxy } from "jest-mock-extended"; -import { of } from "rxjs"; - -import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; -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 { 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 { - ButtonLocation, - SystemNotificationEvent, - SystemNotificationsService, -} from "@bitwarden/common/platform/system-notifications/system-notifications.service"; -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; - let actionService: MockProxy; - 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; - - let sut: AuthRequestAnsweringService; - - const userId = "9f4c3452-6a45-48af-a7d0-74d3e8b65e4c" as UserId; - - beforeEach(() => { - accountService = mock(); - actionService = mock(); - authService = mock(); - i18nService = mock(); - masterPasswordService = { forceSetPasswordReason$: jest.fn() }; - messagingService = mock(); - pendingAuthRequestsState = mock(); - platformUtilsService = mock(); - systemNotificationsService = mock(); - - // Common defaults - authService.activeAccountStatus$ = of(AuthenticationStatus.Locked); - accountService.activeAccount$ = of({ - id: userId, - email: "user@example.com", - emailVerified: true, - name: "User", - }); - accountService.accounts$ = of({ - [userId]: { email: "user@example.com", emailVerified: true, name: "User" }, - }); - (masterPasswordService.forceSetPasswordReason$ as jest.Mock).mockReturnValue( - of(ForceSetPasswordReason.None), - ); - platformUtilsService.isPopupOpen.mockResolvedValue(false); - i18nService.t.mockImplementation( - (key: string, p1?: any) => `${key}${p1 != null ? ":" + p1 : ""}`, - ); - systemNotificationsService.create.mockResolvedValue("notif-id"); - - sut = new AuthRequestAnsweringService( - accountService, - actionService, - authService, - i18nService, - masterPasswordService, - messagingService, - pendingAuthRequestsState, - platformUtilsService, - systemNotificationsService, - ); - }); - - describe("handleAuthRequestNotificationClicked", () => { - it("clears notification and opens popup when notification body is clicked", async () => { - const event: SystemNotificationEvent = { - id: "123", - buttonIdentifier: ButtonLocation.NotificationButton, - }; - - await sut.handleAuthRequestNotificationClicked(event); - - expect(systemNotificationsService.clear).toHaveBeenCalledWith({ id: "123" }); - expect(actionService.openPopup).toHaveBeenCalledTimes(1); - }); - - it("does nothing when an optional button is clicked", async () => { - const event: SystemNotificationEvent = { - id: "123", - buttonIdentifier: ButtonLocation.FirstOptionalButton, - }; - - await sut.handleAuthRequestNotificationClicked(event); - - expect(systemNotificationsService.clear).not.toHaveBeenCalled(); - expect(actionService.openPopup).not.toHaveBeenCalled(); - }); - }); - - describe("receivedPendingAuthRequest", () => { - const authRequestId = "req-abc"; - - it("creates a system notification when popup is not open", async () => { - platformUtilsService.isPopupOpen.mockResolvedValue(false); - authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); - - await sut.receivedPendingAuthRequest(userId, authRequestId); - - expect(i18nService.t).toHaveBeenCalledWith("accountAccessRequested"); - expect(i18nService.t).toHaveBeenCalledWith("confirmAccessAttempt", "user@example.com"); - expect(systemNotificationsService.create).toHaveBeenCalledWith({ - id: `${AuthServerNotificationTags.AuthRequest}_${authRequestId}`, - title: "accountAccessRequested", - body: "confirmAccessAttempt:user@example.com", - buttons: [], - }); - }); - - it("does not create a notification when popup is open, user is active, unlocked, and no force set password", async () => { - platformUtilsService.isPopupOpen.mockResolvedValue(true); - authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); - (masterPasswordService.forceSetPasswordReason$ as jest.Mock).mockReturnValue( - of(ForceSetPasswordReason.None), - ); - - await sut.receivedPendingAuthRequest(userId, authRequestId); - - expect(systemNotificationsService.create).not.toHaveBeenCalled(); - }); - }); -}); 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 deleted file mode 100644 index 834d6ac7bccf..000000000000 --- a/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.ts +++ /dev/null @@ -1,111 +0,0 @@ -import { firstValueFrom } from "rxjs"; - -import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; -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 { 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 { - ButtonLocation, - SystemNotificationEvent, - SystemNotificationsService, -} from "@bitwarden/common/platform/system-notifications/system-notifications.service"; -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, - private readonly actionService: ActionsService, - 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({ - id: `${event.id}`, - }); - await this.actionService.openPopup(); - } - } - - 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 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/default-auth-request-answering.service.spec.ts b/libs/common/src/auth/services/auth-request-answering/default-auth-request-answering.service.spec.ts new file mode 100644 index 000000000000..94ebdb0232f9 --- /dev/null +++ b/libs/common/src/auth/services/auth-request-answering/default-auth-request-answering.service.spec.ts @@ -0,0 +1,472 @@ +import { mock, MockProxy } from "jest-mock-extended"; +import { BehaviorSubject, of, Subject } from "rxjs"; + +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; +import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { + ButtonLocation, + SystemNotificationEvent, +} from "@bitwarden/common/platform/system-notifications/system-notifications.service"; +import { UserId } from "@bitwarden/user-core"; + +import { AuthRequestAnsweringService } from "../../abstractions/auth-request-answering/auth-request-answering.service.abstraction"; + +import { DefaultAuthRequestAnsweringService } from "./default-auth-request-answering.service"; +import { + PendingAuthRequestsStateService, + PendingAuthUserMarker, +} from "./pending-auth-requests.state"; + +describe("DefaultAuthRequestAnsweringService", () => { + let accountService: MockProxy; + let authService: MockProxy; + let masterPasswordService: any; // MasterPasswordServiceAbstraction has many members; we only use forceSetPasswordReason$ + let messagingService: MockProxy; + let pendingAuthRequestsState: MockProxy; + + let sut: AuthRequestAnsweringService; + + const userId = "9f4c3452-6a45-48af-a7d0-74d3e8b65e4c" as UserId; + const otherUserId = "554c3112-9a75-23af-ab80-8dk3e9bl5i8e" as UserId; + + beforeEach(() => { + accountService = mock(); + authService = mock(); + masterPasswordService = { + forceSetPasswordReason$: jest.fn().mockReturnValue(of(ForceSetPasswordReason.None)), + }; + messagingService = mock(); + pendingAuthRequestsState = mock(); + + // Common defaults + authService.activeAccountStatus$ = of(AuthenticationStatus.Locked); + accountService.activeAccount$ = of({ + id: userId, + email: "user@example.com", + emailVerified: true, + name: "User", + }); + accountService.accounts$ = of({ + [userId]: { email: "user@example.com", emailVerified: true, name: "User" }, + [otherUserId]: { email: "other@example.com", emailVerified: true, name: "Other User" }, + }); + + sut = new DefaultAuthRequestAnsweringService( + accountService, + authService, + masterPasswordService, + messagingService, + pendingAuthRequestsState, + ); + }); + + describe("activeUserMeetsConditionsToShowApprovalDialog()", () => { + it("should return false if there is no active user", async () => { + // Arrange + accountService.activeAccount$ = of(null); + + // Act + const result = await sut.activeUserMeetsConditionsToShowApprovalDialog(userId); + + // Assert + expect(result).toBe(false); + }); + + it("should return false if the active user is not the intended recipient of the auth request", async () => { + // Arrange + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + const result = await sut.activeUserMeetsConditionsToShowApprovalDialog(otherUserId); + + // Assert + expect(result).toBe(false); + }); + + it("should return false if the active user is not unlocked", async () => { + // Arrange + authService.activeAccountStatus$ = of(AuthenticationStatus.Locked); + + // Act + const result = await sut.activeUserMeetsConditionsToShowApprovalDialog(userId); + + // Assert + expect(result).toBe(false); + }); + + it("should return false if the active user is required to set/change their master password", async () => { + // Arrange + masterPasswordService.forceSetPasswordReason$.mockReturnValue( + of(ForceSetPasswordReason.WeakMasterPassword), + ); + + // Act + const result = await sut.activeUserMeetsConditionsToShowApprovalDialog(userId); + + // Assert + expect(result).toBe(false); + }); + + it("should return true if the active user is the intended recipient of the auth request, unlocked, and not required to set/change their master password", async () => { + // Arrange + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + const result = await sut.activeUserMeetsConditionsToShowApprovalDialog(userId); + + // Assert + expect(result).toBe(true); + }); + }); + + describe("setupUnlockListenersForProcessingAuthRequests()", () => { + let destroy$: Subject; + let activeAccount$: BehaviorSubject; + let activeAccountStatus$: BehaviorSubject; + let authStatusForSubjects: Map>; + let pendingRequestMarkers: PendingAuthUserMarker[]; + + beforeEach(() => { + destroy$ = new Subject(); + activeAccount$ = new BehaviorSubject({ + id: userId, + email: "user@example.com", + emailVerified: true, + name: "User", + }); + activeAccountStatus$ = new BehaviorSubject(AuthenticationStatus.Locked); + authStatusForSubjects = new Map(); + pendingRequestMarkers = []; + + accountService.activeAccount$ = activeAccount$; + authService.activeAccountStatus$ = activeAccountStatus$; + authService.authStatusFor$.mockImplementation((id: UserId) => { + if (!authStatusForSubjects.has(id)) { + authStatusForSubjects.set(id, new BehaviorSubject(AuthenticationStatus.Locked)); + } + return authStatusForSubjects.get(id)!; + }); + + pendingAuthRequestsState.getAll$.mockReturnValue(of([])); + }); + + afterEach(() => { + destroy$.next(); + destroy$.complete(); + }); + + describe("active account switching", () => { + it("should process pending auth requests when switching to an unlocked user", async () => { + // Arrange + authStatusForSubjects.set(otherUserId, new BehaviorSubject(AuthenticationStatus.Unlocked)); + pendingRequestMarkers = [{ userId: otherUserId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + + // Simulate account switching to an Unlocked account + activeAccount$.next({ + id: otherUserId, + email: "other@example.com", + emailVerified: true, + name: "Other", + }); + + // Assert + await new Promise((resolve) => setTimeout(resolve, 0)); // Allows observable chain to complete before assertion + expect(messagingService.send).toHaveBeenCalledWith("openLoginApproval"); + }); + + it("should NOT process pending auth requests when switching to a locked user", async () => { + // Arrange + authStatusForSubjects.set(otherUserId, new BehaviorSubject(AuthenticationStatus.Locked)); + pendingRequestMarkers = [{ userId: otherUserId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + activeAccount$.next({ + id: otherUserId, + email: "other@example.com", + emailVerified: true, + name: "Other", + }); + + // Assert + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(messagingService.send).not.toHaveBeenCalled(); + }); + + it("should NOT process pending auth requests when switching to a logged out user", async () => { + // Arrange + authStatusForSubjects.set(otherUserId, new BehaviorSubject(AuthenticationStatus.LoggedOut)); + pendingRequestMarkers = [{ userId: otherUserId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + activeAccount$.next({ + id: otherUserId, + email: "other@example.com", + emailVerified: true, + name: "Other", + }); + + // Assert + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(messagingService.send).not.toHaveBeenCalled(); + }); + + it("should NOT process pending auth requests when active account becomes null", async () => { + // Arrange + pendingRequestMarkers = [{ userId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + activeAccount$.next(null); + + // Assert + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(messagingService.send).not.toHaveBeenCalled(); + }); + + it("should handle multiple user switches correctly", async () => { + // Arrange + const secondUserId = "second-user-id" as UserId; + authStatusForSubjects.set(otherUserId, new BehaviorSubject(AuthenticationStatus.Unlocked)); + authStatusForSubjects.set(secondUserId, new BehaviorSubject(AuthenticationStatus.Locked)); + pendingRequestMarkers = [{ userId: otherUserId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + + // Switch to unlocked user (should trigger) + activeAccount$.next({ + id: otherUserId, + email: "other@example.com", + emailVerified: true, + name: "Other", + }); + await new Promise((resolve) => setTimeout(resolve, 0)); + + // Switch to locked user (should NOT trigger) + activeAccount$.next({ + id: secondUserId, + email: "second@example.com", + emailVerified: true, + name: "Second", + }); + await new Promise((resolve) => setTimeout(resolve, 0)); + + // Assert + expect(messagingService.send).toHaveBeenCalledTimes(1); + }); + + it("should NOT process pending auth requests when switching to an Unlocked user who is required to set/change their master password", async () => { + // Arrange + masterPasswordService.forceSetPasswordReason$.mockReturnValue( + of(ForceSetPasswordReason.WeakMasterPassword), + ); + authStatusForSubjects.set(otherUserId, new BehaviorSubject(AuthenticationStatus.Unlocked)); + pendingRequestMarkers = [{ userId: otherUserId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + activeAccount$.next({ + id: otherUserId, + email: "other@example.com", + emailVerified: true, + name: "Other", + }); + + // Assert + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(messagingService.send).not.toHaveBeenCalled(); + }); + }); + + describe("authentication status transitions", () => { + it("should process pending auth requests when active account transitions to Unlocked", async () => { + // Arrange + activeAccountStatus$.next(AuthenticationStatus.Locked); + pendingRequestMarkers = [{ userId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + activeAccountStatus$.next(AuthenticationStatus.Unlocked); + + // Assert + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(messagingService.send).toHaveBeenCalledWith("openLoginApproval"); + }); + + it("should process pending auth requests when transitioning from LoggedOut to Unlocked", async () => { + // Arrange + activeAccountStatus$.next(AuthenticationStatus.LoggedOut); + pendingRequestMarkers = [{ userId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + activeAccountStatus$.next(AuthenticationStatus.Unlocked); + + // Assert + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(messagingService.send).toHaveBeenCalledWith("openLoginApproval"); + }); + + it("should NOT process pending auth requests when transitioning from Unlocked to Locked", async () => { + // Arrange + activeAccountStatus$.next(AuthenticationStatus.Unlocked); + pendingRequestMarkers = [{ userId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + await new Promise((resolve) => setTimeout(resolve, 0)); + + // Clear any calls from the initial trigger (from null -> Unlocked) + messagingService.send.mockClear(); + + activeAccountStatus$.next(AuthenticationStatus.Locked); + + // Assert + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(messagingService.send).not.toHaveBeenCalled(); + }); + + it("should NOT process pending auth requests when transitioning from Locked to LoggedOut", async () => { + // Arrange + activeAccountStatus$.next(AuthenticationStatus.Locked); + pendingRequestMarkers = [{ userId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + activeAccountStatus$.next(AuthenticationStatus.LoggedOut); + + // Assert + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(messagingService.send).not.toHaveBeenCalled(); + }); + + it("should NOT process pending auth requests when staying in Unlocked status", async () => { + // Arrange + activeAccountStatus$.next(AuthenticationStatus.Unlocked); + pendingRequestMarkers = [{ userId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + await new Promise((resolve) => setTimeout(resolve, 0)); + + // Clear any calls from the initial trigger (from null -> Unlocked) + messagingService.send.mockClear(); + + activeAccountStatus$.next(AuthenticationStatus.Unlocked); + + // Assert + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(messagingService.send).not.toHaveBeenCalled(); + }); + + it("should handle multiple status transitions correctly", async () => { + // Arrange + activeAccountStatus$.next(AuthenticationStatus.Locked); + pendingRequestMarkers = [{ userId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + + // Transition to Unlocked (should trigger) + activeAccountStatus$.next(AuthenticationStatus.Unlocked); + await new Promise((resolve) => setTimeout(resolve, 0)); + + // Transition to Locked (should NOT trigger) + activeAccountStatus$.next(AuthenticationStatus.Locked); + await new Promise((resolve) => setTimeout(resolve, 0)); + + // Transition back to Unlocked (should trigger again) + activeAccountStatus$.next(AuthenticationStatus.Unlocked); + await new Promise((resolve) => setTimeout(resolve, 0)); + + // Assert + expect(messagingService.send).toHaveBeenCalledTimes(2); + expect(messagingService.send).toHaveBeenCalledWith("openLoginApproval"); + }); + + it("should NOT process pending auth requests when active account transitions to Unlocked but is required to set/change their master password", async () => { + // Arrange + masterPasswordService.forceSetPasswordReason$.mockReturnValue( + of(ForceSetPasswordReason.WeakMasterPassword), + ); + activeAccountStatus$.next(AuthenticationStatus.Locked); + pendingRequestMarkers = [{ userId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + activeAccountStatus$.next(AuthenticationStatus.Unlocked); + + // Assert + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(messagingService.send).not.toHaveBeenCalled(); + }); + }); + + describe("subscription cleanup", () => { + it("should stop processing when destroy$ emits", async () => { + // Arrange + authStatusForSubjects.set(otherUserId, new BehaviorSubject(AuthenticationStatus.Unlocked)); + pendingRequestMarkers = [{ userId: otherUserId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + + // Emit destroy signal + destroy$.next(); + + // Try to trigger processing after cleanup + activeAccount$.next({ + id: otherUserId, + email: "other@example.com", + emailVerified: true, + name: "Other", + }); + activeAccountStatus$.next(AuthenticationStatus.Unlocked); + + // Assert + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(messagingService.send).not.toHaveBeenCalled(); + }); + }); + }); + + describe("handleAuthRequestNotificationClicked()", () => { + it("should throw an error", async () => { + // Arrange + const event: SystemNotificationEvent = { + id: "123", + buttonIdentifier: ButtonLocation.NotificationButton, + }; + + // Act + const promise = sut.handleAuthRequestNotificationClicked(event); + + // Assert + await expect(promise).rejects.toThrow( + "handleAuthRequestNotificationClicked() not implemented for this client", + ); + }); + }); +}); diff --git a/libs/common/src/auth/services/auth-request-answering/default-auth-request-answering.service.ts b/libs/common/src/auth/services/auth-request-answering/default-auth-request-answering.service.ts new file mode 100644 index 000000000000..ea8067ed351c --- /dev/null +++ b/libs/common/src/auth/services/auth-request-answering/default-auth-request-answering.service.ts @@ -0,0 +1,140 @@ +import { + distinctUntilChanged, + filter, + firstValueFrom, + map, + Observable, + pairwise, + startWith, + switchMap, + take, + takeUntil, + tap, +} from "rxjs"; + +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; +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 { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { SystemNotificationEvent } from "@bitwarden/common/platform/system-notifications/system-notifications.service"; +import { UserId } from "@bitwarden/user-core"; + +import { AuthRequestAnsweringService } from "../../abstractions/auth-request-answering/auth-request-answering.service.abstraction"; + +import { + PendingAuthRequestsStateService, + PendingAuthUserMarker, +} from "./pending-auth-requests.state"; + +export class DefaultAuthRequestAnsweringService implements AuthRequestAnsweringService { + constructor( + protected readonly accountService: AccountService, + protected readonly authService: AuthService, + protected readonly masterPasswordService: MasterPasswordServiceAbstraction, + protected readonly messagingService: MessagingService, + protected readonly pendingAuthRequestsState: PendingAuthRequestsStateService, + ) {} + + async activeUserMeetsConditionsToShowApprovalDialog(authRequestUserId: UserId): Promise { + // If the active user is not the intended recipient of the auth request, return false + const activeUserId: UserId | null = await firstValueFrom( + this.accountService.activeAccount$.pipe(getOptionalUserId), + ); + if (activeUserId !== authRequestUserId) { + return false; + } + + // If the active user is not unlocked, return false + const authStatus = await firstValueFrom(this.authService.activeAccountStatus$); + if (authStatus !== AuthenticationStatus.Unlocked) { + return false; + } + + // If the active user is required to set/change their master password, return false + // Note that by this point we know that the authRequestUserId is the active UserId (see check above) + const forceSetPasswordReason = await firstValueFrom( + this.masterPasswordService.forceSetPasswordReason$(authRequestUserId), + ); + if (forceSetPasswordReason !== ForceSetPasswordReason.None) { + return false; + } + + // User meets conditions: they are the intended recipient, unlocked, and not required to set/change their master password + return true; + } + + setupUnlockListenersForProcessingAuthRequests(destroy$: Observable): void { + // When account switching to a user who is Unlocked, process any pending auth requests. + 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(() => { + void this.processPendingAuthRequests(); + }), + takeUntil(destroy$), + ) + .subscribe(); + + // When the active account transitions TO Unlocked, process any pending auth requests. + 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(destroy$), + ) + .subscribe(() => { + void this.processPendingAuthRequests(); + }); + } + + /** + * Process notifications that have been received but didn't meet the conditions to display the + * approval dialog. + */ + private async processPendingAuthRequests(): Promise { + const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + + // Only continue if the active user is not required to set/change their master password + const forceSetPasswordReason = await firstValueFrom( + this.masterPasswordService.forceSetPasswordReason$(activeUserId), + ); + if (forceSetPasswordReason !== ForceSetPasswordReason.None) { + return; + } + + // 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 pendingAuthRequestsInState: PendingAuthUserMarker[] = + (await firstValueFrom(this.pendingAuthRequestsState.getAll$())) ?? []; + + if (pendingAuthRequestsInState.length > 0) { + const pendingAuthRequestsForActiveUser = pendingAuthRequestsInState.some( + (e) => e.userId === activeUserId, + ); + + if (pendingAuthRequestsForActiveUser) { + this.messagingService.send("openLoginApproval"); + } + } + } + + async handleAuthRequestNotificationClicked(event: SystemNotificationEvent): Promise { + throw new Error("handleAuthRequestNotificationClicked() not implemented for this client"); + } +} 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 730362adfed5..1d858516fe4c 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 @@ -1,14 +1,22 @@ import { SystemNotificationEvent } from "@bitwarden/common/platform/system-notifications/system-notifications.service"; import { UserId } from "@bitwarden/user-core"; -import { AuthRequestAnsweringServiceAbstraction } from "../../abstractions/auth-request-answering/auth-request-answering.service.abstraction"; +import { AuthRequestAnsweringService } from "../../abstractions/auth-request-answering/auth-request-answering.service.abstraction"; -export class NoopAuthRequestAnsweringService implements AuthRequestAnsweringServiceAbstraction { - constructor() {} +export class NoopAuthRequestAnsweringService implements AuthRequestAnsweringService { + async activeUserMeetsConditionsToShowApprovalDialog(authRequestUserId: UserId): Promise { + throw new Error( + "activeUserMeetsConditionsToShowApprovalDialog() not implemented for this client", + ); + } - async receivedPendingAuthRequest(userId: UserId, notificationId: string): Promise {} + setupUnlockListenersForProcessingAuthRequests(): void { + throw new Error( + "setupUnlockListenersForProcessingAuthRequests() not implemented for this client", + ); + } - async handleAuthRequestNotificationClicked(event: SystemNotificationEvent): Promise {} - - async processPendingAuthRequests(): Promise {} + async handleAuthRequestNotificationClicked(event: SystemNotificationEvent): Promise { + throw new Error("handleAuthRequestNotificationClicked() not implemented for this client"); + } } 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 b4d47698e4de..2a29445e230d 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 @@ -3,7 +3,7 @@ import { BehaviorSubject, bufferCount, firstValueFrom, Subject, ObservedValueOf // eslint-disable-next-line no-restricted-imports import { LogoutReason } from "@bitwarden/auth/common"; -import { AuthRequestAnsweringServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; +import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { AccountService } from "../../../auth/abstractions/account.service"; @@ -32,7 +32,7 @@ describe("DefaultServerNotificationsService (multi-user)", () => { let signalRNotificationConnectionService: MockProxy; let authService: MockProxy; let webPushNotificationConnectionService: MockProxy; - let authRequestAnsweringService: MockProxy; + let authRequestAnsweringService: MockProxy; let configService: MockProxy; let activeUserAccount$: BehaviorSubject>; @@ -125,7 +125,7 @@ describe("DefaultServerNotificationsService (multi-user)", () => { return webPushSupportStatusByUser.get(userId)!.asObservable(); }); - authRequestAnsweringService = mock(); + authRequestAnsweringService = mock(); configService = mock(); configService.getFeatureFlag$.mockImplementation((flag: FeatureFlag) => { @@ -270,13 +270,13 @@ describe("DefaultServerNotificationsService (multi-user)", () => { // allow async queue to drain await new Promise((resolve) => setTimeout(resolve, 0)); - expect(messagingService.send).toHaveBeenCalledWith("openLoginApproval", { - notificationId: "auth-id-2", - }); + // When authRequestAnsweringService.receivedPendingAuthRequest exists (Extension/Desktop), + // only that method is called. messagingService.send is only called for Web (NoopAuthRequestAnsweringService). expect(authRequestAnsweringService.receivedPendingAuthRequest).toHaveBeenCalledWith( mockUserId2, "auth-id-2", ); + expect(messagingService.send).not.toHaveBeenCalled(); subscription.unsubscribe(); }); diff --git a/libs/common/src/platform/server-notifications/internal/default-server-notifications.service.spec.ts b/libs/common/src/platform/server-notifications/internal/default-server-notifications.service.spec.ts index b2aa4fbd315d..0d38c6e8dd24 100644 --- a/libs/common/src/platform/server-notifications/internal/default-server-notifications.service.spec.ts +++ b/libs/common/src/platform/server-notifications/internal/default-server-notifications.service.spec.ts @@ -4,7 +4,7 @@ import { BehaviorSubject, bufferCount, firstValueFrom, ObservedValueOf, of, Subj // 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 { LogoutReason } from "@bitwarden/auth/common"; -import { AuthRequestAnsweringServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; +import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; import { awaitAsync } from "../../../../spec"; import { Matrix } from "../../../../spec/matrix"; @@ -40,7 +40,7 @@ describe("NotificationsService", () => { let signalRNotificationConnectionService: MockProxy; let authService: MockProxy; let webPushNotificationConnectionService: MockProxy; - let authRequestAnsweringService: MockProxy; + let authRequestAnsweringService: MockProxy; let configService: MockProxy; let activeAccount: BehaviorSubject>; @@ -69,7 +69,7 @@ describe("NotificationsService", () => { signalRNotificationConnectionService = mock(); authService = mock(); webPushNotificationConnectionService = mock(); - authRequestAnsweringService = mock(); + authRequestAnsweringService = mock(); configService = mock(); // For these tests, use the active-user implementation (feature flag disabled) @@ -391,5 +391,41 @@ describe("NotificationsService", () => { expect(logoutCallback).not.toHaveBeenCalled(); }); }); + + describe("NotificationType.AuthRequest", () => { + it("should call receivedPendingAuthRequest when it exists (Extension/Desktop)", async () => { + authRequestAnsweringService.receivedPendingAuthRequest!.mockResolvedValue(undefined as any); + + const notification = new NotificationResponse({ + type: NotificationType.AuthRequest, + payload: { userId: mockUser1, id: "auth-request-123" }, + contextId: "different-app-id", + }); + + await sut["processNotification"](notification, mockUser1); + + expect(authRequestAnsweringService.receivedPendingAuthRequest).toHaveBeenCalledWith( + mockUser1, + "auth-request-123", + ); + expect(messagingService.send).not.toHaveBeenCalled(); + }); + + it("should call messagingService.send when receivedPendingAuthRequest does not exist (Web)", async () => { + authRequestAnsweringService.receivedPendingAuthRequest = undefined as any; + + const notification = new NotificationResponse({ + type: NotificationType.AuthRequest, + payload: { userId: mockUser1, id: "auth-request-456" }, + contextId: "different-app-id", + }); + + await sut["processNotification"](notification, mockUser1); + + expect(messagingService.send).toHaveBeenCalledWith("openLoginApproval", { + notificationId: "auth-request-456", + }); + }); + }); }); }); 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 efe0a8ae4088..32ce46d84f51 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 @@ -15,7 +15,7 @@ import { // 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 { LogoutReason } from "@bitwarden/auth/common"; -import { AuthRequestAnsweringServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; +import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { trackedMerge } from "@bitwarden/common/platform/misc"; @@ -65,7 +65,7 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer private readonly signalRConnectionService: SignalRConnectionService, private readonly authService: AuthService, private readonly webPushConnectionService: WebPushConnectionService, - private readonly authRequestAnsweringService: AuthRequestAnsweringServiceAbstraction, + private readonly authRequestAnsweringService: AuthRequestAnsweringService, private readonly configService: ConfigService, ) { this.notifications$ = this.configService @@ -293,26 +293,28 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer case NotificationType.SyncSendDelete: await this.syncService.syncDeleteSend(notification.payload as SyncSendNotification); break; - case NotificationType.AuthRequest: - 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, - }); + case NotificationType.AuthRequest: { + // Only Extension and Desktop implement the AuthRequestAnsweringService + if (this.authRequestAnsweringService.receivedPendingAuthRequest) { + try { + await this.authRequestAnsweringService.receivedPendingAuthRequest( + notification.payload.userId, + notification.payload.id, + ); + } catch (error) { + this.logService.error(`Failed to process auth request notification: ${error}`); + } + } else { + // This call is necessary for Web, which uses a NoopAuthRequestAnsweringService + // that does not have a receivedPendingAuthRequest() method + this.messagingService.send("openLoginApproval", { + // Include the authRequestId so the DeviceManagementComponent can upsert the correct device. + // This will only matter if the user is on the /device-management screen when the auth request is received. + notificationId: notification.payload.id, + }); + } break; + } case NotificationType.SyncOrganizationStatusChanged: await this.syncService.fullSync(true); break;