Auth/pm 14943/auth request extension dialog approve (#16132)

* feat(notification-processing): [PM-19877] System Notification Implementation - Implemented the full feature set for device approval from extension.

* test(notification-processing): [PM-19877] System Notification Implementation - Updated tests.

---------

Co-authored-by: Justin Baur <19896123+justindbaur@users.noreply.github.com>

libs/angular/src/auth/device-management/device-management.component.ts

@@ -6,12 +6,15 @@ import { firstValueFrom } from "rxjs";
 // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop.
 // eslint-disable-next-line no-restricted-imports
 import { AuthRequestApiServiceAbstraction } from "@bitwarden/auth/common";
+import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
 import { DevicesServiceAbstraction } from "@bitwarden/common/auth/abstractions/devices/devices.service.abstraction";
 import {
   DevicePendingAuthRequest,
   DeviceResponse,
 } from "@bitwarden/common/auth/abstractions/devices/responses/device.response";
 import { DeviceView } from "@bitwarden/common/auth/abstractions/devices/views/device.view";
+import { getUserId } from "@bitwarden/common/auth/services/account.service";
+import { PendingAuthRequestsStateService } from "@bitwarden/common/auth/services/auth-request-answering/pending-auth-requests.state";
 import { DeviceType, DeviceTypeMetadata } from "@bitwarden/common/enums";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
 import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service";
@@ -66,14 +69,16 @@ export class DeviceManagementComponent implements OnInit {
   protected showHeaderInfo = false;
 
   constructor(
-    private authRequestApiService: AuthRequestApiServiceAbstraction,
-    private destroyRef: DestroyRef,
-    private deviceManagementComponentService: DeviceManagementComponentServiceAbstraction,
-    private devicesService: DevicesServiceAbstraction,
-    private dialogService: DialogService,
-    private i18nService: I18nService,
-    private messageListener: MessageListener,
-    private validationService: ValidationService,
+    private readonly accountService: AccountService,
+    private readonly authRequestApiService: AuthRequestApiServiceAbstraction,
+    private readonly destroyRef: DestroyRef,
+    private readonly deviceManagementComponentService: DeviceManagementComponentServiceAbstraction,
+    private readonly devicesService: DevicesServiceAbstraction,
+    private readonly dialogService: DialogService,
+    private readonly i18nService: I18nService,
+    private readonly messageListener: MessageListener,
+    private readonly pendingAuthRequestStateService: PendingAuthRequestsStateService,
+    private readonly validationService: ValidationService,
   ) {
     this.showHeaderInfo = this.deviceManagementComponentService.showHeaderInformation();
   }
@@ -248,6 +253,12 @@ export class DeviceManagementComponent implements OnInit {
       // Auth request was approved or denied, so clear the
       // pending auth request and re-sort the device array
       this.devices = clearAuthRequestAndResortDevices(this.devices, pendingAuthRequest);
+
+      // If a user ignores or doesn't see the auth request dialog, but comes to account settings
+      // to approve a device login attempt, clear out the state for that user.
+      await this.pendingAuthRequestStateService.clear(
+        await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)),
+      );
     }
   }
 }

libs/angular/src/services/jslib-services.module.ts

@@ -106,6 +106,7 @@ import { AccountApiServiceImplementation } from "@bitwarden/common/auth/services
 import { AccountServiceImplementation } from "@bitwarden/common/auth/services/account.service";
 import { AnonymousHubService } from "@bitwarden/common/auth/services/anonymous-hub.service";
 import { NoopAuthRequestAnsweringService } from "@bitwarden/common/auth/services/auth-request-answering/noop-auth-request-answering.service";
+import { PendingAuthRequestsStateService } from "@bitwarden/common/auth/services/auth-request-answering/pending-auth-requests.state";
 import { AuthService } from "@bitwarden/common/auth/services/auth.service";
 import { AvatarService } from "@bitwarden/common/auth/services/avatar.service";
 import { DefaultActiveUserAccessor } from "@bitwarden/common/auth/services/default-active-user.accessor";
@@ -942,6 +943,11 @@ const safeProviders: SafeProvider[] = [
     useClass: UnsupportedSystemNotificationsService,
     deps: [],
   }),
+  safeProvider({
+    provide: PendingAuthRequestsStateService,
+    useClass: PendingAuthRequestsStateService,
+    deps: [StateProvider],
+  }),
   safeProvider({
     provide: AuthRequestAnsweringServiceAbstraction,
     useClass: NoopAuthRequestAnsweringService,

libs/auth/src/angular/self-hosted-env-config-dialog/self-hosted-env-config-dialog.component.ts

@@ -159,7 +159,6 @@ export class SelfHostedEnvConfigDialogComponent implements OnInit, OnDestroy {
         },
       });
   }
-
   submit = async () => {
     this.showErrorSummary = false;
 

libs/common/src/auth/abstractions/auth-request-answering/README.md

@@ -0,0 +1,10 @@
+# Auth Request Answering Service
+
+This feature is to allow for the taking of auth requests that are received via websockets by the background service to
+be acted on when the user loads up a client. Currently only implemented with the browser client.
+
+See diagram for the high level picture of how this is wired up.
+
+## Diagram
+
+![img.png](notification-architecture.png)

libs/common/src/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction.ts

@@ -2,7 +2,29 @@ import { SystemNotificationEvent } from "@bitwarden/common/platform/system-notif
 import { UserId } from "@bitwarden/user-core";
 
 export abstract class AuthRequestAnsweringServiceAbstraction {
-  abstract receivedPendingAuthRequest(userId: UserId, notificationId: string): Promise<void>;
+  /**
+   * Tries to either display the dialog for the user or will preserve its data and show it at a
+   * later time. Even in the event the dialog is shown immediately, this will write to global state
+   * so that even if someone closes a window or a popup and comes back, it could be processed later.
+   * Only way to clear out the global state is to respond to the auth request.
+   *
+   * Currently, this is only implemented for browser extension.
+   *
+   * @param userId The UserId that the auth request is for.
+   * @param authRequestId The id of the auth request that is to be processed.
+   */
+  abstract receivedPendingAuthRequest(userId: UserId, authRequestId: string): Promise<void>;
 
+  /**
+   * When a system notification is clicked, this function is used to process that event.
+   *
+   * @param event The event passed in. Check initNotificationSubscriptions in main.background.ts.
+   */
   abstract handleAuthRequestNotificationClicked(event: SystemNotificationEvent): Promise<void>;
+
+  /**
+   * Process notifications that have been received but didn't meet the conditions to display the
+   * approval dialog.
+   */
+  abstract processPendingAuthRequests(): Promise<void>;
 }

libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.spec.ts

@@ -7,6 +7,7 @@ import { AuthServerNotificationTags } from "@bitwarden/common/auth/enums/auth-se
 import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
 import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
+import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
 import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
 import { ActionsService } from "@bitwarden/common/platform/actions";
 import {
@@ -17,6 +18,7 @@ import {
 import { UserId } from "@bitwarden/user-core";
 
 import { AuthRequestAnsweringService } from "./auth-request-answering.service";
+import { PendingAuthRequestsStateService } from "./pending-auth-requests.state";
 
 describe("AuthRequestAnsweringService", () => {
   let accountService: MockProxy<AccountService>;
@@ -24,6 +26,8 @@ describe("AuthRequestAnsweringService", () => {
   let authService: MockProxy<AuthService>;
   let i18nService: MockProxy<I18nService>;
   let masterPasswordService: any; // MasterPasswordServiceAbstraction has many members; we only use forceSetPasswordReason$
+  let messagingService: MockProxy<MessagingService>;
+  let pendingAuthRequestsState: MockProxy<PendingAuthRequestsStateService>;
   let platformUtilsService: MockProxy<PlatformUtilsService>;
   let systemNotificationsService: MockProxy<SystemNotificationsService>;
 
@@ -37,6 +41,8 @@ describe("AuthRequestAnsweringService", () => {
     authService = mock<AuthService>();
     i18nService = mock<I18nService>();
     masterPasswordService = { forceSetPasswordReason$: jest.fn() };
+    messagingService = mock<MessagingService>();
+    pendingAuthRequestsState = mock<PendingAuthRequestsStateService>();
     platformUtilsService = mock<PlatformUtilsService>();
     systemNotificationsService = mock<SystemNotificationsService>();
 
@@ -66,6 +72,8 @@ describe("AuthRequestAnsweringService", () => {
       authService,
       i18nService,
       masterPasswordService,
+      messagingService,
+      pendingAuthRequestsState,
       platformUtilsService,
       systemNotificationsService,
     );

libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.ts

@@ -5,9 +5,10 @@ import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
 import { AuthServerNotificationTags } from "@bitwarden/common/auth/enums/auth-server-notification-tags";
 import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
 import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason";
-import { getUserId } from "@bitwarden/common/auth/services/account.service";
+import { getOptionalUserId, getUserId } from "@bitwarden/common/auth/services/account.service";
 import { MasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction";
 import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
+import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
 import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
 import { ActionsService } from "@bitwarden/common/platform/actions";
 import {
@@ -19,6 +20,11 @@ import { UserId } from "@bitwarden/user-core";
 
 import { AuthRequestAnsweringServiceAbstraction } from "../../abstractions/auth-request-answering/auth-request-answering.service.abstraction";
 
+import {
+  PendingAuthRequestsStateService,
+  PendingAuthUserMarker,
+} from "./pending-auth-requests.state";
+
 export class AuthRequestAnsweringService implements AuthRequestAnsweringServiceAbstraction {
   constructor(
     private readonly accountService: AccountService,
@@ -26,10 +32,51 @@ export class AuthRequestAnsweringService implements AuthRequestAnsweringServiceA
     private readonly authService: AuthService,
     private readonly i18nService: I18nService,
     private readonly masterPasswordService: MasterPasswordServiceAbstraction,
+    private readonly messagingService: MessagingService,
+    private readonly pendingAuthRequestsState: PendingAuthRequestsStateService,
     private readonly platformUtilsService: PlatformUtilsService,
     private readonly systemNotificationsService: SystemNotificationsService,
   ) {}
 
+  async receivedPendingAuthRequest(userId: UserId, authRequestId: string): Promise<void> {
+    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<void> {
     if (event.buttonIdentifier === ButtonLocation.NotificationButton) {
       await this.systemNotificationsService.clear({
@@ -39,32 +86,26 @@ export class AuthRequestAnsweringService implements AuthRequestAnsweringServiceA
     }
   }
 
-  async receivedPendingAuthRequest(userId: UserId, authRequestId: string): Promise<void> {
-    const authStatus = await firstValueFrom(this.authService.activeAccountStatus$);
-    const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId));
-    const forceSetPasswordReason = await firstValueFrom(
-      this.masterPasswordService.forceSetPasswordReason$(userId),
-    );
+  async processPendingAuthRequests(): Promise<void> {
+    // Prune any stale pending requests (older than 15 minutes)
+    // This comes from GlobalSettings.cs
+    //    public TimeSpan UserRequestExpiration { get; set; } = TimeSpan.FromMinutes(15);
+    const fifteenMinutesMs = 15 * 60 * 1000;
 
-    // Is the popup already open?
-    if (
-      (await this.platformUtilsService.isPopupOpen()) &&
-      authStatus === AuthenticationStatus.Unlocked &&
-      activeUserId === userId &&
-      forceSetPasswordReason === ForceSetPasswordReason.None
-    ) {
-      // TODO: Handled in 14934
-    } else {
-      // Get the user's email to include in the system notification
-      const accounts = await firstValueFrom(this.accountService.accounts$);
-      const emailForUser = accounts[userId].email;
+    await this.pendingAuthRequestsState.pruneOlderThan(fifteenMinutesMs);
 
-      await this.systemNotificationsService.create({
-        id: `${AuthServerNotificationTags.AuthRequest}_${authRequestId}`,
-        title: this.i18nService.t("accountAccessRequested"),
-        body: this.i18nService.t("confirmAccessAttempt", emailForUser),
-        buttons: [],
-      });
+    const pendingAuthRequestsInState: PendingAuthUserMarker[] =
+      (await firstValueFrom(this.pendingAuthRequestsState.getAll$())) ?? [];
+
+    if (pendingAuthRequestsInState.length > 0) {
+      const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId));
+      const pendingAuthRequestsForActiveUser = pendingAuthRequestsInState.some(
+        (e) => e.userId === activeUserId,
+      );
+
+      if (pendingAuthRequestsForActiveUser) {
+        this.messagingService.send("openLoginApproval");
+      }
     }
   }
 }

libs/common/src/auth/services/auth-request-answering/noop-auth-request-answering.service.ts

@@ -5,7 +5,10 @@ import { AuthRequestAnsweringServiceAbstraction } from "../../abstractions/auth-
 
 export class NoopAuthRequestAnsweringService implements AuthRequestAnsweringServiceAbstraction {
   constructor() {}
-  async handleAuthRequestNotificationClicked(event: SystemNotificationEvent): Promise<void> {}
 
   async receivedPendingAuthRequest(userId: UserId, notificationId: string): Promise<void> {}
+
+  async handleAuthRequestNotificationClicked(event: SystemNotificationEvent): Promise<void> {}
+
+  async processPendingAuthRequests(): Promise<void> {}
 }

libs/common/src/auth/services/auth-request-answering/pending-auth-requests.state.ts

@@ -0,0 +1,57 @@
+import { Observable } from "rxjs";
+
+import {
+  AUTH_REQUEST_DISK_LOCAL,
+  GlobalState,
+  KeyDefinition,
+  StateProvider,
+} from "@bitwarden/common/platform/state";
+import { UserId } from "@bitwarden/user-core";
+
+export type PendingAuthUserMarker = {
+  userId: UserId;
+  receivedAtMs: number;
+};
+
+export const PENDING_AUTH_REQUESTS = KeyDefinition.array<PendingAuthUserMarker>(
+  AUTH_REQUEST_DISK_LOCAL,
+  "pendingAuthRequests",
+  {
+    deserializer: (json) => json,
+  },
+);
+
+export class PendingAuthRequestsStateService {
+  private readonly state: GlobalState<PendingAuthUserMarker[]>;
+
+  constructor(private readonly stateProvider: StateProvider) {
+    this.state = this.stateProvider.getGlobal(PENDING_AUTH_REQUESTS);
+  }
+
+  getAll$(): Observable<PendingAuthUserMarker[] | null> {
+    return this.state.state$;
+  }
+
+  async add(userId: UserId): Promise<void> {
+    const now = Date.now();
+    await this.stateProvider.getGlobal(PENDING_AUTH_REQUESTS).update((current) => {
+      const list = (current ?? []).filter((e) => e.userId !== userId);
+      return [...list, { userId, receivedAtMs: now }];
+    });
+  }
+
+  async pruneOlderThan(maxAgeMs: number): Promise<void> {
+    const cutoff = Date.now() - maxAgeMs;
+    await this.stateProvider.getGlobal(PENDING_AUTH_REQUESTS).update((current) => {
+      const list = current ?? [];
+      return list.filter((e) => e.receivedAtMs >= cutoff);
+    });
+  }
+
+  async clear(userId: UserId): Promise<void> {
+    await this.stateProvider.getGlobal(PENDING_AUTH_REQUESTS).update((current) => {
+      const list = current ?? [];
+      return list.filter((e) => e.userId !== userId);
+    });
+  }
+}

libs/common/src/platform/server-notifications/internal/default-server-notifications.multiuser.spec.ts

@@ -131,6 +131,8 @@ describe("DefaultServerNotificationsService (multi-user)", () => {
     configService.getFeatureFlag$.mockImplementation((flag: FeatureFlag) => {
       const flagValueByFlag: Partial<Record<FeatureFlag, boolean>> = {
         [FeatureFlag.InactiveUserServerNotification]: true,
+        [FeatureFlag.PushNotificationsWhenLocked]: true,
+        [FeatureFlag.PM14938_BrowserExtensionLoginApproval]: true,
       };
       return new BehaviorSubject(flagValueByFlag[flag] ?? false) as any;
     });
@@ -253,8 +255,7 @@ describe("DefaultServerNotificationsService (multi-user)", () => {
         .next({ type: "not-supported", reason: "test" } as any);
     }
 
-    // TODO: When PM-14943 goes in, uncomment
-    // authRequestAnsweringService.receivedPendingAuthRequest.mockResolvedValue(undefined as any);
+    authRequestAnsweringService.receivedPendingAuthRequest.mockResolvedValue(undefined as any);
 
     const subscription = defaultServerNotificationsService.startListening();
 
@@ -273,12 +274,10 @@ describe("DefaultServerNotificationsService (multi-user)", () => {
     expect(messagingService.send).toHaveBeenCalledWith("openLoginApproval", {
       notificationId: "auth-id-2",
     });
-
-    // TODO: When PM-14943 goes in, uncomment
-    // expect(authRequestAnsweringService.receivedPendingAuthRequest).toHaveBeenCalledWith(
-    //   mockUserId2,
-    //   "auth-id-2",
-    // );
+    expect(authRequestAnsweringService.receivedPendingAuthRequest).toHaveBeenCalledWith(
+      mockUserId2,
+      "auth-id-2",
+    );
 
     subscription.unsubscribe();
   });