From ddc817689a1de49c794fbd92bb2447252929f809 Mon Sep 17 00:00:00 2001 From: Jordan Aasen <166539328+jaasen-livefront@users.noreply.github.com> Date: Mon, 6 Jan 2025 13:52:42 -0800 Subject: [PATCH] [PM-13365] - don't display totp capture when in popout (#12645) * don't display totp capture when in popout * add canCaptureTotp method * dry up logic * add unit tests * fix failing tests * add missing mock to cipher-form story --- .../services/browser-totp-capture.service.spec.ts | 15 +++++++++++++++ .../services/browser-totp-capture.service.ts | 5 +++++ .../abstractions/totp-capture.service.ts | 5 +++++ libs/vault/src/cipher-form/cipher-form.stories.ts | 1 + .../login-details-section.component.spec.ts | 4 +++- .../login-details-section.component.ts | 8 ++++++-- 6 files changed, 35 insertions(+), 3 deletions(-) diff --git a/apps/browser/src/vault/popup/services/browser-totp-capture.service.spec.ts b/apps/browser/src/vault/popup/services/browser-totp-capture.service.spec.ts index 2c9afacffd7..2b309e8f817 100644 --- a/apps/browser/src/vault/popup/services/browser-totp-capture.service.spec.ts +++ b/apps/browser/src/vault/popup/services/browser-totp-capture.service.spec.ts @@ -2,6 +2,7 @@ import { TestBed } from "@angular/core/testing"; import qrcodeParser from "qrcode-parser"; import { BrowserApi } from "../../../platform/browser/browser-api"; +import BrowserPopupUtils from "../../../platform/popup/browser-popup-utils"; import { BrowserTotpCaptureService } from "./browser-totp-capture.service"; @@ -13,12 +14,14 @@ describe("BrowserTotpCaptureService", () => { let testBed: TestBed; let service: BrowserTotpCaptureService; let mockCaptureVisibleTab: jest.SpyInstance; + let mockBrowserPopupUtilsInPopout: jest.SpyInstance; const validTotpUrl = "otpauth://totp/label?secret=123"; beforeEach(() => { mockCaptureVisibleTab = jest.spyOn(BrowserApi, "captureVisibleTab"); mockCaptureVisibleTab.mockResolvedValue("screenshot"); + mockBrowserPopupUtilsInPopout = jest.spyOn(BrowserPopupUtils, "inPopout"); testBed = TestBed.configureTestingModule({ providers: [BrowserTotpCaptureService], @@ -66,4 +69,16 @@ describe("BrowserTotpCaptureService", () => { expect(result).toBeNull(); }); + + describe("canCaptureTotp", () => { + it("should return true when not in a popout window", () => { + mockBrowserPopupUtilsInPopout.mockReturnValue(false); + expect(service.canCaptureTotp({} as Window)).toBe(true); + }); + + it("should return false when in a popout window", () => { + mockBrowserPopupUtilsInPopout.mockReturnValue(true); + expect(service.canCaptureTotp({} as Window)).toBe(false); + }); + }); }); diff --git a/apps/browser/src/vault/popup/services/browser-totp-capture.service.ts b/apps/browser/src/vault/popup/services/browser-totp-capture.service.ts index 3f8ba61ed36..ac73b271c84 100644 --- a/apps/browser/src/vault/popup/services/browser-totp-capture.service.ts +++ b/apps/browser/src/vault/popup/services/browser-totp-capture.service.ts @@ -4,6 +4,7 @@ import qrcodeParser from "qrcode-parser"; import { TotpCaptureService } from "@bitwarden/vault"; import { BrowserApi } from "../../../platform/browser/browser-api"; +import BrowserPopupUtils from "../../../platform/popup/browser-popup-utils"; /** * Implementation of TotpCaptureService for the browser which captures the @@ -20,4 +21,8 @@ export class BrowserTotpCaptureService implements TotpCaptureService { } return null; } + + canCaptureTotp(window: Window) { + return !BrowserPopupUtils.inPopout(window); + } } diff --git a/libs/vault/src/cipher-form/abstractions/totp-capture.service.ts b/libs/vault/src/cipher-form/abstractions/totp-capture.service.ts index d6d95565869..72bbb0da12c 100644 --- a/libs/vault/src/cipher-form/abstractions/totp-capture.service.ts +++ b/libs/vault/src/cipher-form/abstractions/totp-capture.service.ts @@ -6,4 +6,9 @@ export abstract class TotpCaptureService { * Captures a TOTP secret and returns it as a string. Returns null if no TOTP secret was found. */ abstract captureTotpSecret(): Promise; + /** + * Returns whether the TOTP secret can be captured from the current tab. + * Only available in the browser extension and when not in a popout window. + */ + abstract canCaptureTotp(window: Window): boolean; } diff --git a/libs/vault/src/cipher-form/cipher-form.stories.ts b/libs/vault/src/cipher-form/cipher-form.stories.ts index 1d44e4542bc..72c4acb23cd 100644 --- a/libs/vault/src/cipher-form/cipher-form.stories.ts +++ b/libs/vault/src/cipher-form/cipher-form.stories.ts @@ -152,6 +152,7 @@ export default { provide: TotpCaptureService, useValue: { captureTotpSecret: () => Promise.resolve("some-value"), + canCaptureTotp: () => true, }, }, { diff --git a/libs/vault/src/cipher-form/components/login-details-section/login-details-section.component.spec.ts b/libs/vault/src/cipher-form/components/login-details-section/login-details-section.component.spec.ts index 232a4b2d27b..182427f7f42 100644 --- a/libs/vault/src/cipher-form/components/login-details-section/login-details-section.component.spec.ts +++ b/libs/vault/src/cipher-form/components/login-details-section/login-details-section.component.spec.ts @@ -434,6 +434,7 @@ describe("LoginDetailsSectionComponent", () => { }); it("should call captureTotp when the capture totp button is clicked", fakeAsync(() => { + jest.spyOn(component, "canCaptureTotp", "get").mockReturnValue(true); component.captureTotp = jest.fn(); fixture.detectChanges(); @@ -445,7 +446,8 @@ describe("LoginDetailsSectionComponent", () => { })); describe("canCaptureTotp", () => { - it("should return true when totpCaptureService is present and totp is editable", () => { + it("should return true when totpCaptureService is present and totpCaptureService.canCaptureTotp is true and totp is editable", () => { + jest.spyOn(component, "canCaptureTotp", "get").mockReturnValue(true); component.loginDetailsForm.controls.totp.enable(); expect(component.canCaptureTotp).toBe(true); }); diff --git a/libs/vault/src/cipher-form/components/login-details-section/login-details-section.component.ts b/libs/vault/src/cipher-form/components/login-details-section/login-details-section.component.ts index 0a6772e5ef1..2296932aca3 100644 --- a/libs/vault/src/cipher-form/components/login-details-section/login-details-section.component.ts +++ b/libs/vault/src/cipher-form/components/login-details-section/login-details-section.component.ts @@ -65,10 +65,14 @@ export class LoginDetailsSectionComponent implements OnInit { newPasswordGenerated: boolean; /** - * Whether the TOTP field can be captured from the current tab. Only available in the browser extension. + * Whether the TOTP field can be captured from the current tab. Only available in the browser extension and + * when not in a popout window. */ get canCaptureTotp() { - return this.totpCaptureService != null && this.loginDetailsForm.controls.totp.enabled; + return ( + !!this.totpCaptureService?.canCaptureTotp(window) && + this.loginDetailsForm.controls.totp.enabled + ); } private datePipe = inject(DatePipe);