From aa9bae61469c5508140a930a8505c9e384dbf34b Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Tue, 2 Sep 2025 15:58:33 -0400 Subject: [PATCH 1/2] fix(extension-device-approval): [PM-14943] Answering Service Full Implementation - Active user id will no longer throw. --- .../auth-request-answering/auth-request-answering.service.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.ts b/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.ts index d8b2b4eb5f5..63f7d37dff3 100644 --- a/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.ts +++ b/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.ts @@ -5,7 +5,7 @@ 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"; @@ -41,7 +41,7 @@ export class AuthRequestAnsweringService implements AuthRequestAnsweringServiceA { userId, authRequestId }, ); const authStatus = await firstValueFrom(this.authService.activeAccountStatus$); - const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + const activeUserId: UserId | null = await firstValueFrom(this.accountService.activeAccount$.pipe(getOptionalUserId)); const forceSetPasswordReason = await firstValueFrom( this.masterPasswordService.forceSetPasswordReason$(userId), ); From 0c254caa54aee26cd6c3a839f15d49580664a14c Mon Sep 17 00:00:00 2001 From: Patrick Pimentel Date: Tue, 2 Sep 2025 16:02:23 -0400 Subject: [PATCH 2/2] fix(extension-device-approval): [PM-14943] Answering Service Full Implementation - Removed debug statements. --- apps/browser/src/popup/app.component.ts | 38 +++++--------- .../auth-request-answering.service.ts | 18 ++----- .../default-server-notifications.service.ts | 52 +------------------ .../internal/signalr-connection.service.ts | 39 +++----------- libs/common/src/services/api.service.ts | 2 - 5 files changed, 23 insertions(+), 126 deletions(-) diff --git a/apps/browser/src/popup/app.component.ts b/apps/browser/src/popup/app.component.ts index 7aeb9371f49..98f7514b597 100644 --- a/apps/browser/src/popup/app.component.ts +++ b/apps/browser/src/popup/app.component.ts @@ -150,11 +150,9 @@ export class AppComponent implements OnInit, OnDestroy { this.compactModeService.init(); await this.popupSizeService.setHeight(); - this.accountService.activeAccount$ - .pipe(takeUntil(this.destroy$)) - .subscribe((account) => { - this.activeUserId = account?.id; - }); + this.accountService.activeAccount$.pipe(takeUntil(this.destroy$)).subscribe((account) => { + this.activeUserId = account?.id; + }); // Separate subscription: only trigger processing on subsequent user switches while popup is open this.accountService.activeAccount$ @@ -254,19 +252,21 @@ export class AppComponent implements OnInit, OnDestroy { await this.router.navigate(["lock"]); } else if (msg.command === "openLoginApproval") { - if (this.processingPendingAuth) {return;} + if (this.processingPendingAuth) { + return; + } this.processingPendingAuth = true; try { // Always query server for all pending requests and open a dialog for each - const pendingList = await firstValueFrom(this.authRequestService.getPendingAuthRequests$()); + const pendingList = await firstValueFrom( + this.authRequestService.getPendingAuthRequests$(), + ); if (Array.isArray(pendingList) && pendingList.length > 0) { const respondedIds = new Set(); for (const req of pendingList) { - if (req?.id == null) {continue;} - console.debug( - "[Popup AppComponent] Opening LoginApprovalDialogComponent", - req.id, - ); + if (req?.id == null) { + continue; + } const dialogRef = LoginApprovalDialogComponent.open(this.dialogService, { notificationId: req.id, }); @@ -275,22 +275,10 @@ export class AppComponent implements OnInit, OnDestroy { if (result !== undefined && typeof result === "boolean") { respondedIds.add(req.id); - if ( - respondedIds.size === pendingList.length && - this.activeUserId != null - ) { - console.debug( - "[Popup AppComponent] All pending auth requests responded; clearing marker for user", - this.activeUserId, - ); + if (respondedIds.size === pendingList.length && this.activeUserId != null) { await this.pendingAuthRequestsState.clearByUserId(this.activeUserId); } } - - console.debug( - "[Popup AppComponent] LoginApprovalDialogComponent closed", - req.id, - ); } } } finally { diff --git a/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.ts b/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.ts index 63f7d37dff3..b848e93a39b 100644 --- a/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.ts +++ b/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.ts @@ -36,22 +36,15 @@ export class AuthRequestAnsweringService implements AuthRequestAnsweringServiceA ) {} async receivedPendingAuthRequest(userId: UserId, authRequestId: string): Promise { - console.debug( - "[AuthRequestAnsweringService] receivedPendingAuthRequest", - { userId, authRequestId }, - ); const authStatus = await firstValueFrom(this.authService.activeAccountStatus$); - const activeUserId: UserId | null = await firstValueFrom(this.accountService.activeAccount$.pipe(getOptionalUserId)); + 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(); - console.debug( - "[AuthRequestAnsweringService] current state", - { popupOpen, authStatus, activeUserId, forceSetPasswordReason }, - ); - // Always persist the pending marker for this user to global state. await this.pendingAuthRequestsState.add(userId); @@ -64,9 +57,6 @@ export class AuthRequestAnsweringService implements AuthRequestAnsweringServiceA forceSetPasswordReason === ForceSetPasswordReason.None; if (!conditionsMet) { - console.debug( - "[AuthRequestAnsweringService] receivedPendingAuthRequest - Conditions not met, creating system notification", - ); // Get the user's email to include in the system notification const accounts = await firstValueFrom(this.accountService.accounts$); const emailForUser = accounts[userId].email; @@ -81,7 +71,6 @@ export class AuthRequestAnsweringService implements AuthRequestAnsweringServiceA } // Popup is open and conditions are met; open dialog immediately for this request - console.debug("[AuthRequestAnsweringService] receivedPendingAuthRequest - Opening popup."); this.messagingService.send("openLoginApproval"); } @@ -118,7 +107,6 @@ export class AuthRequestAnsweringService implements AuthRequestAnsweringServiceA ); if (isUnlocked && forceSetPasswordReason === ForceSetPasswordReason.None) { - console.debug("[AuthRequestAnsweringService] popupOpened - Opening popup."); this.messagingService.send("openLoginApproval"); } } 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 a775fda069a..0ded797bb1e 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 @@ -11,8 +11,6 @@ import { Observable, share, switchMap, - tap, - finalize, } from "rxjs"; // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. @@ -69,19 +67,12 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer this.notifications$ = this.configService .getFeatureFlag$(FeatureFlag.InactiveUserServerNotification) .pipe( - tap((value) => { - console.debug(`[DefaultServerNotificationsService] feature flag value for inactive user server notifications: ${value}`); - }), distinctUntilChanged(), switchMap((inactiveUserServerNotificationEnabled) => { if (inactiveUserServerNotificationEnabled) { - console.debug("[DefaultServerNotificationsService] inactive notification processing"); return this.accountService.accounts$.pipe( map((accounts) => Object.keys(accounts) as UserId[]), distinctUntilChanged(), - tap((value) => { - console.debug(`[DefaultServerNotificationsService] new accounts: ${value}`); - }), switchMap((userIds) => { if (userIds.length === 0) { return EMPTY; @@ -98,7 +89,6 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer ); } - console.debug("[DefaultServerNotificationsService] NOT inactive notification processing"); return this.accountService.activeAccount$.pipe( map((account) => account?.id), distinctUntilChanged(), @@ -125,18 +115,9 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer private userNotifications$(userId: UserId) { return this.environmentService.getEnvironment$(userId).pipe( map((env) => env.getNotificationsUrl()), - tap((value) => { - console.debug(`[DefaultServerNotificationsService] userNotifications$ before: ${value}`); - }), distinctUntilChanged(), - tap((value) => { - console.debug(`[DefaultServerNotificationsService] userNotifications$ after: ${value}`); - }), switchMap((notificationsUrl) => { if (notificationsUrl === DISABLED_NOTIFICATIONS_URL) { - console.debug( - `[DefaultServerNotificationsService] notifications disabled via URL for user ${userId}`, - ); return EMPTY; } @@ -150,18 +131,12 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer distinctUntilChanged(), switchMap((hasAccessToken) => { if (!hasAccessToken) { - console.debug( - `[DefaultServerNotificationsService] no access token for user ${userId}, skipping notifications`, - ); return EMPTY; } return this.activitySubject; }), switchMap((activityStatus) => { - console.debug( - `[DefaultServerNotificationsService] activity status for user ${userId}: ${activityStatus}`, - ); if (activityStatus === "inactive") { return EMPTY; } @@ -171,24 +146,15 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer supportSwitch({ supported: (service) => { this.logService.info("Using WebPush for server notifications"); - console.debug( - `[DefaultServerNotificationsService] WebPush supported for user ${userId}`, - ); return service.notifications$.pipe( catchError((err: unknown) => { this.logService.warning("Issue with web push, falling back to SignalR", err); - console.debug( - `[DefaultServerNotificationsService] WebPush error for user ${userId}, falling back to SignalR`, - ); return this.connectSignalR$(userId, notificationsUrl); }), ); }, notSupported: () => { this.logService.info("Using SignalR for server notifications"); - console.debug( - `[DefaultServerNotificationsService] WebPush not supported for user ${userId}, using SignalR`, - ); return this.connectSignalR$(userId, notificationsUrl); }, }), @@ -196,17 +162,9 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer } private connectSignalR$(userId: UserId, notificationsUrl: string) { - console.debug( - `[DefaultServerNotificationsService] Connecting SignalR for user ${userId} to ${notificationsUrl}`, - ); return this.signalRConnectionService.connect$(userId, notificationsUrl).pipe( filter((n) => n.type === "ReceiveMessage"), map((n) => (n as ReceiveMessage).message), - finalize(() => - console.debug( - `[DefaultServerNotificationsService] SignalR stream finalized for user ${userId}`, - ), - ), ); } @@ -319,7 +277,7 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer case NotificationType.AuthRequest: if ( await firstValueFrom( - this.configService.getFeatureFlag$(FeatureFlag.PM14938_BrowserExtensionLoginApproval) + this.configService.getFeatureFlag$(FeatureFlag.PM14938_BrowserExtensionLoginApproval), ) ) { await this.authRequestAnsweringService.receivedPendingAuthRequest( @@ -343,14 +301,8 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer } startListening() { - console.debug("[DefaultServerNotificationsService] startListening subscribe"); return this.notifications$ .pipe( - tap(([notification, userId]) => - console.debug( - `[DefaultServerNotificationsService] received notification type ${notification.type} for user ${userId}`, - ), - ), mergeMap(async ([notification, userId]) => this.processNotification(notification, userId)), ) .subscribe({ @@ -360,12 +312,10 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer } reconnectFromActivity(): void { - console.debug("[DefaultServerNotificationsService] reconnectFromActivity called"); this.activitySubject.next("active"); } disconnectFromInactivity(): void { - console.debug("[DefaultServerNotificationsService] disconnectFromInactivity called"); this.activitySubject.next("inactive"); } } diff --git a/libs/common/src/platform/server-notifications/internal/signalr-connection.service.ts b/libs/common/src/platform/server-notifications/internal/signalr-connection.service.ts index ce11c5be927..72188c33fc6 100644 --- a/libs/common/src/platform/server-notifications/internal/signalr-connection.service.ts +++ b/libs/common/src/platform/server-notifications/internal/signalr-connection.service.ts @@ -76,15 +76,9 @@ export class SignalRConnectionService { connect$(userId: UserId, notificationsUrl: string) { return new Observable((subsciber) => { - console.debug( - `[SignalRConnectionService] creating connection for user ${userId} to ${notificationsUrl}`, - ); const connection = this.hubConnectionBuilderFactory() .withUrl(notificationsUrl + "/hub", { accessTokenFactory: async () => { - console.debug( - `[SignalRConnectionService] accessTokenFactory called for user ${userId}`, - ); return this.apiService.getActiveBearerToken(userId); }, skipNegotiation: true, @@ -95,16 +89,10 @@ export class SignalRConnectionService { .build(); connection.on("ReceiveMessage", (data: any) => { - console.debug( - `[SignalRConnectionService] ReceiveMessage for user ${userId}: ${JSON.stringify( - data?.Type ?? data?.type ?? "unknown", - )}`, - ); subsciber.next({ type: "ReceiveMessage", message: new NotificationResponse(data) }); }); connection.on("Heartbeat", () => { - console.debug(`[SignalRConnectionService] Heartbeat for user ${userId}`); subsciber.next({ type: "Heartbeat" }); }); @@ -129,22 +117,13 @@ export class SignalRConnectionService { } const randomTime = this.randomReconnectTime(); - console.debug( - `[SignalRConnectionService] scheduling reconnect for user ${userId} in ${randomTime}ms`, - ); const timeoutHandler = this.timeoutManager.setTimeout(() => { connection .start() .then(() => { - console.debug( - `[SignalRConnectionService] reconnected for user ${userId}`, - ); reconnectSubscription = null; }) .catch(() => { - console.debug( - `[SignalRConnectionService] reconnect failed for user ${userId}, rescheduling`, - ); scheduleReconnect(); }); }, randomTime); @@ -155,26 +134,20 @@ export class SignalRConnectionService { }; connection.onclose((error) => { - console.debug( - `[SignalRConnectionService] onclose for user ${userId}: ${error?.toString?.() ?? "unknown"}`, - ); scheduleReconnect(); }); // Start connection - connection.start().then(() => { - console.debug(`[SignalRConnectionService] connected for user ${userId}`); - }).catch(() => { - console.debug( - `[SignalRConnectionService] initial connect failed for user ${userId}, scheduling reconnect`, - ); - scheduleReconnect(); - }); + connection + .start() + .then(() => {}) + .catch(() => { + scheduleReconnect(); + }); return () => { // Cancel any possible scheduled reconnects reconnectSubscription?.unsubscribe(); - console.debug(`[SignalRConnectionService] stopping connection for user ${userId}`); connection?.stop().catch((error) => { this.logService.error("Error while stopping SignalR connection", error); }); diff --git a/libs/common/src/services/api.service.ts b/libs/common/src/services/api.service.ts index b76d43318de..bbf990122df 100644 --- a/libs/common/src/services/api.service.ts +++ b/libs/common/src/services/api.service.ts @@ -1541,10 +1541,8 @@ export class ApiService implements ApiServiceAbstraction { // Helpers async getActiveBearerToken(userId: UserId): Promise { - console.debug("[ApiService] getActiveBearerToken called"); let accessToken = await this.tokenService.getAccessToken(userId); if (await this.tokenService.tokenNeedsRefresh(userId)) { - console.debug("[ApiService] access token needs refresh; refreshing now"); accessToken = await this.refreshToken(userId); } return accessToken;