From cdd8a697e8687fb14c2d7899c45919ce481eccac Mon Sep 17 00:00:00 2001 From: Nick Krantz <125900171+nick-livefront@users.noreply.github.com> Date: Tue, 25 Nov 2025 08:41:41 -0600 Subject: [PATCH 01/22] do not show copy password button on the web for users that do not have access (#17635) --- .../vault-cipher-row.component.html | 10 +- .../vault-cipher-row.component.spec.ts | 144 ++++++++++++++++++ 2 files changed, 150 insertions(+), 4 deletions(-) create mode 100644 apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.spec.ts diff --git a/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.html b/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.html index c09553dab9c..c8732154ef4 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.html +++ b/apps/web/src/app/vault/components/vault-items/vault-cipher-row.component.html @@ -109,10 +109,12 @@ {{ "copyUsername" | i18n }} - + @if (cipher.viewPassword) { + + } @@ -45,7 +45,7 @@ type="button" bitMenuItem (click)="restore(cipher)" - *ngIf="!cipher.decryptionFailure" + *ngIf="!hasDecryptionFailure(cipher)" > {{ "restore" | i18n }} diff --git a/apps/browser/src/vault/popup/settings/trash-list-items-container/trash-list-items-container.component.ts b/apps/browser/src/vault/popup/settings/trash-list-items-container/trash-list-items-container.component.ts index 70ba6842a0d..bad6011b2d8 100644 --- a/apps/browser/src/vault/popup/settings/trash-list-items-container/trash-list-items-container.component.ts +++ b/apps/browser/src/vault/popup/settings/trash-list-items-container/trash-list-items-container.component.ts @@ -12,7 +12,6 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { CipherId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; -import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { DialogService, IconButtonModule, @@ -85,10 +84,40 @@ export class TrashListItemsContainerComponent { return collections[0]?.name; } - async restore(cipher: CipherView) { + /** + * Check if a cipher has attachments. CipherView has a hasAttachments getter, + * while CipherListView has an attachments count property. + */ + hasAttachments(cipher: PopupCipherViewLike): boolean { + if ("hasAttachments" in cipher) { + return cipher.hasAttachments; + } + return cipher.attachments > 0; + } + + /** + * Get the subtitle for a cipher. CipherView has a subTitle getter, + * while CipherListView has a subtitle property. + */ + getSubtitle(cipher: PopupCipherViewLike): string | undefined { + if ("subTitle" in cipher) { + return cipher.subTitle; + } + return cipher.subtitle; + } + + /** + * Check if a cipher has a decryption failure. CipherView has this property, + * while CipherListView does not. + */ + hasDecryptionFailure(cipher: PopupCipherViewLike): boolean { + return "decryptionFailure" in cipher && cipher.decryptionFailure; + } + + async restore(cipher: PopupCipherViewLike) { try { const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); - await this.cipherService.restoreWithServer(cipher.id, activeUserId); + await this.cipherService.restoreWithServer(cipher.id as string, activeUserId); await this.router.navigate(["/trash"]); this.toastService.showToast({ @@ -101,7 +130,7 @@ export class TrashListItemsContainerComponent { } } - async delete(cipher: CipherView) { + async delete(cipher: PopupCipherViewLike) { const repromptPassed = await this.passwordRepromptService.passwordRepromptCheck(cipher); if (!repromptPassed) { @@ -120,7 +149,7 @@ export class TrashListItemsContainerComponent { try { const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); - await this.cipherService.deleteWithServer(cipher.id, activeUserId); + await this.cipherService.deleteWithServer(cipher.id as string, activeUserId); await this.router.navigate(["/trash"]); this.toastService.showToast({ @@ -133,8 +162,9 @@ export class TrashListItemsContainerComponent { } } - async onViewCipher(cipher: CipherView) { - if (cipher.decryptionFailure) { + async onViewCipher(cipher: PopupCipherViewLike) { + // CipherListView doesn't have decryptionFailure, so we use optional chaining + if ("decryptionFailure" in cipher && cipher.decryptionFailure) { DecryptionFailureDialogComponent.open(this.dialogService, { cipherIds: [cipher.id as CipherId], }); @@ -147,7 +177,7 @@ export class TrashListItemsContainerComponent { } await this.router.navigate(["/view-cipher"], { - queryParams: { cipherId: cipher.id, type: cipher.type }, + queryParams: { cipherId: cipher.id as string, type: cipher.type }, }); } } diff --git a/apps/browser/tsconfig.json b/apps/browser/tsconfig.json index 0fd6cac4230..6fb9dfbe46b 100644 --- a/apps/browser/tsconfig.json +++ b/apps/browser/tsconfig.json @@ -1,5 +1,8 @@ { "extends": "../../tsconfig.base", + "angularCompilerOptions": { + "strictTemplates": true + }, "include": [ "src", "../../libs/common/src/autofill/constants", diff --git a/libs/vault/src/components/can-delete-cipher.directive.ts b/libs/vault/src/components/can-delete-cipher.directive.ts index 8ab59f9d647..f88e38049da 100644 --- a/libs/vault/src/components/can-delete-cipher.directive.ts +++ b/libs/vault/src/components/can-delete-cipher.directive.ts @@ -1,8 +1,8 @@ import { Directive, Input, OnDestroy, TemplateRef, ViewContainerRef } from "@angular/core"; import { Subject, takeUntil } from "rxjs"; -import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { CipherAuthorizationService } from "@bitwarden/common/vault/services/cipher-authorization.service"; +import { CipherViewLike } from "@bitwarden/common/vault/utils/cipher-view-like-utils"; /** * Only shows the element if the user can delete the cipher. @@ -15,7 +15,7 @@ export class CanDeleteCipherDirective implements OnDestroy { // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals // eslint-disable-next-line @angular-eslint/prefer-signals - @Input("appCanDeleteCipher") set cipher(cipher: CipherView) { + @Input("appCanDeleteCipher") set cipher(cipher: CipherViewLike) { this.viewContainer.clear(); this.cipherAuthorizationService diff --git a/libs/vault/src/components/copy-cipher-field.directive.ts b/libs/vault/src/components/copy-cipher-field.directive.ts index b4b1941273f..52a4f59e7a2 100644 --- a/libs/vault/src/components/copy-cipher-field.directive.ts +++ b/libs/vault/src/components/copy-cipher-field.directive.ts @@ -36,7 +36,7 @@ export class CopyCipherFieldDirective implements OnChanges { alias: "appCopyField", required: true, }) - action!: Exclude; + action!: CopyAction; // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals // eslint-disable-next-line @angular-eslint/prefer-signals diff --git a/libs/vault/src/index.ts b/libs/vault/src/index.ts index ccd830cd34e..93a72ba14e0 100644 --- a/libs/vault/src/index.ts +++ b/libs/vault/src/index.ts @@ -3,7 +3,11 @@ export { AtRiskPasswordCalloutData, } from "./services/at-risk-password-callout.service"; export { PasswordRepromptService } from "./services/password-reprompt.service"; -export { CopyCipherFieldService, CopyAction } from "./services/copy-cipher-field.service"; +export { + CopyCipherFieldService, + CopyAction, + CopyFieldAction, +} from "./services/copy-cipher-field.service"; export { CopyCipherFieldDirective } from "./components/copy-cipher-field.directive"; export { OrgIconDirective } from "./components/org-icon.directive"; export { CanDeleteCipherDirective } from "./components/can-delete-cipher.directive"; diff --git a/libs/vault/src/services/copy-cipher-field.service.ts b/libs/vault/src/services/copy-cipher-field.service.ts index 0a7d9e1f68d..539c81b7be3 100644 --- a/libs/vault/src/services/copy-cipher-field.service.ts +++ b/libs/vault/src/services/copy-cipher-field.service.ts @@ -35,6 +35,12 @@ export type CopyAction = | "publicKey" | "keyFingerprint"; +/** + * Copy actions that can be used with the appCopyField directive. + * Excludes "hiddenField" which requires special handling. + */ +export type CopyFieldAction = Exclude; + type CopyActionInfo = { /** * The i18n key for the type of field being copied. Will be used to display a toast message. From c04c1757ea6de993a6c808fdda7d64cdf0651425 Mon Sep 17 00:00:00 2001 From: Ben Brooks <56796209+bensbits91@users.noreply.github.com> Date: Tue, 25 Nov 2025 08:06:03 -0800 Subject: [PATCH 06/22] Revert "Lets shadow DOM check signal page update (#16114)" (commit 6129ca536686cfceb385e9b87f6c569f0ecf9558) (#17503) Signed-off-by: Ben Brooks --- .../abstractions/dom-query.service.ts | 2 +- .../collect-autofill-content.service.spec.ts | 31 +------------------ .../collect-autofill-content.service.ts | 13 ++------ .../services/dom-query.service.spec.ts | 2 ++ .../autofill/services/dom-query.service.ts | 9 ++++-- 5 files changed, 13 insertions(+), 44 deletions(-) diff --git a/apps/browser/src/autofill/services/abstractions/dom-query.service.ts b/apps/browser/src/autofill/services/abstractions/dom-query.service.ts index 32809573223..da7354403e5 100644 --- a/apps/browser/src/autofill/services/abstractions/dom-query.service.ts +++ b/apps/browser/src/autofill/services/abstractions/dom-query.service.ts @@ -6,5 +6,5 @@ export interface DomQueryService { mutationObserver?: MutationObserver, forceDeepQueryAttempt?: boolean, ): T[]; - checkPageContainsShadowDom(): boolean; + checkPageContainsShadowDom(): void; } diff --git a/apps/browser/src/autofill/services/collect-autofill-content.service.spec.ts b/apps/browser/src/autofill/services/collect-autofill-content.service.spec.ts index 9ee329fa150..66a692dbe20 100644 --- a/apps/browser/src/autofill/services/collect-autofill-content.service.spec.ts +++ b/apps/browser/src/autofill/services/collect-autofill-content.service.spec.ts @@ -395,7 +395,7 @@ describe("CollectAutofillContentService", () => { }); }); - it("sets the noFieldsFound property to true if the page has no forms or fields", async function () { + it("sets the noFieldsFond property to true if the page has no forms or fields", async function () { document.body.innerHTML = ""; collectAutofillContentService["noFieldsFound"] = false; jest.spyOn(collectAutofillContentService as any, "buildAutofillFormsData"); @@ -2649,33 +2649,4 @@ describe("CollectAutofillContentService", () => { ); }); }); - - describe("processMutations", () => { - beforeEach(() => { - jest.useFakeTimers(); - }); - - afterEach(() => { - jest.runOnlyPendingTimers(); - jest.useRealTimers(); - }); - - it("will require an update to page details if shadow DOM is present", () => { - jest - .spyOn(domQueryService as any, "checkPageContainsShadowDom") - .mockImplementationOnce(() => true); - - collectAutofillContentService["requirePageDetailsUpdate"] = jest.fn(); - - collectAutofillContentService["mutationsQueue"] = [[], []]; - - collectAutofillContentService["processMutations"](); - - jest.runOnlyPendingTimers(); - - expect(domQueryService.checkPageContainsShadowDom).toHaveBeenCalled(); - expect(collectAutofillContentService["mutationsQueue"]).toHaveLength(0); - expect(collectAutofillContentService["requirePageDetailsUpdate"]).toHaveBeenCalled(); - }); - }); }); diff --git a/apps/browser/src/autofill/services/collect-autofill-content.service.ts b/apps/browser/src/autofill/services/collect-autofill-content.service.ts index 47b1c9ea6df..6f2c00a4dd4 100644 --- a/apps/browser/src/autofill/services/collect-autofill-content.service.ts +++ b/apps/browser/src/autofill/services/collect-autofill-content.service.ts @@ -997,13 +997,6 @@ export class CollectAutofillContentService implements CollectAutofillContentServ * within an idle callback to help with performance and prevent excessive updates. */ private processMutations = () => { - // If the page contains shadow DOM, we require a page details update from the autofill service. - // Will wait for an idle moment on main thread to execute, unless timeout has passed. - requestIdleCallbackPolyfill( - () => this.domQueryService.checkPageContainsShadowDom() && this.requirePageDetailsUpdate(), - { timeout: 500 }, - ); - const queueLength = this.mutationsQueue.length; for (let queueIndex = 0; queueIndex < queueLength; queueIndex++) { @@ -1026,13 +1019,13 @@ export class CollectAutofillContentService implements CollectAutofillContentServ * Triggers several flags that indicate that a collection of page details should * occur again on a subsequent call after a mutation has been observed in the DOM. */ - private requirePageDetailsUpdate = () => { + private flagPageDetailsUpdateIsRequired() { this.domRecentlyMutated = true; if (this.autofillOverlayContentService) { this.autofillOverlayContentService.pageDetailsUpdateRequired = true; } this.noFieldsFound = false; - }; + } /** * Processes all mutation records encountered by the mutation observer. @@ -1060,7 +1053,7 @@ export class CollectAutofillContentService implements CollectAutofillContentServ (this.isAutofillElementNodeMutated(mutation.removedNodes, true) || this.isAutofillElementNodeMutated(mutation.addedNodes)) ) { - this.requirePageDetailsUpdate(); + this.flagPageDetailsUpdateIsRequired(); return; } diff --git a/apps/browser/src/autofill/services/dom-query.service.spec.ts b/apps/browser/src/autofill/services/dom-query.service.spec.ts index 87645c98a45..53862aef735 100644 --- a/apps/browser/src/autofill/services/dom-query.service.spec.ts +++ b/apps/browser/src/autofill/services/dom-query.service.spec.ts @@ -72,6 +72,7 @@ describe("DomQueryService", () => { }); it("queries form field elements that are nested within multiple ShadowDOM elements", () => { + domQueryService["pageContainsShadowDom"] = true; const root = document.createElement("div"); const shadowRoot1 = root.attachShadow({ mode: "open" }); const root2 = document.createElement("div"); @@ -94,6 +95,7 @@ describe("DomQueryService", () => { }); it("will fallback to using the TreeWalker API if a depth larger than 4 ShadowDOM elements is encountered", () => { + domQueryService["pageContainsShadowDom"] = true; const root = document.createElement("div"); const shadowRoot1 = root.attachShadow({ mode: "open" }); const root2 = document.createElement("div"); diff --git a/apps/browser/src/autofill/services/dom-query.service.ts b/apps/browser/src/autofill/services/dom-query.service.ts index 1b0c5681ff0..932bbe47f90 100644 --- a/apps/browser/src/autofill/services/dom-query.service.ts +++ b/apps/browser/src/autofill/services/dom-query.service.ts @@ -78,9 +78,8 @@ export class DomQueryService implements DomQueryServiceInterface { /** * Checks if the page contains any shadow DOM elements. */ - checkPageContainsShadowDom = (): boolean => { + checkPageContainsShadowDom = (): void => { this.pageContainsShadowDom = this.queryShadowRoots(globalThis.document.body, true).length > 0; - return this.pageContainsShadowDom; }; /** @@ -109,7 +108,7 @@ export class DomQueryService implements DomQueryServiceInterface { ): T[] { let elements = this.queryElements(root, queryString); - const shadowRoots = this.pageContainsShadowDom ? this.recursivelyQueryShadowRoots(root) : []; + const shadowRoots = this.recursivelyQueryShadowRoots(root); for (let index = 0; index < shadowRoots.length; index++) { const shadowRoot = shadowRoots[index]; elements = elements.concat(this.queryElements(shadowRoot, queryString)); @@ -152,6 +151,10 @@ export class DomQueryService implements DomQueryServiceInterface { root: Document | ShadowRoot | Element, depth: number = 0, ): ShadowRoot[] { + if (!this.pageContainsShadowDom) { + return []; + } + if (depth >= MAX_DEEP_QUERY_RECURSION_DEPTH) { throw new Error("Max recursion depth reached"); } From cf6569bfea7516cf3e86f2854d5c61c17a547983 Mon Sep 17 00:00:00 2001 From: Dave <3836813+enmande@users.noreply.github.com> Date: Tue, 25 Nov 2025 11:23:22 -0500 Subject: [PATCH 07/22] feat(user-decryption-options) [PM-26413]: Remove ActiveUserState from UserDecryptionOptionsService (#16894) * feat(user-decryption-options) [PM-26413]: Update UserDecryptionOptionsService and tests to use UserId-only APIs. * feat(user-decryption-options) [PM-26413]: Update InternalUserDecryptionOptionsService call sites to use UserId-only API. * feat(user-decryption-options) [PM-26413] Update userDecryptionOptions$ call sites to use the UserId-only API. * feat(user-decryption-options) [PM-26413]: Update additional call sites. * feat(user-decryption-options) [PM-26413]: Update dependencies and an additional call site. * feat(user-verification-service) [PM-26413]: Replace where allowed by unrestricted imports invocation of UserVerificationService.hasMasterPassword (deprecated) with UserDecryptionOptions.hasMasterPasswordById$. Additional work to complete as tech debt tracked in PM-27009. * feat(user-decryption-options) [PM-26413]: Update for non-null strict adherence. * feat(user-decryption-options) [PM-26413]: Update type safety and defensive returns. * chore(user-decryption-options) [PM-26413]: Comment cleanup. * feat(user-decryption-options) [PM-26413]: Update tests. * feat(user-decryption-options) [PM-26413]: Standardize null-checking on active account id for new API consumption. * feat(vault-timeout-settings-service) [PM-26413]: Add test cases to illustrate null active account from AccountService. * fix(fido2-user-verification-service-spec) [PM-26413]: Update test harness to use FakeAccountService. * fix(downstream-components) [PM-26413]: Prefer use of the getUserId operator in all authenticated contexts for user id provided to UserDecryptionOptionsService. --------- Co-authored-by: bnagawiecki <107435978+bnagawiecki@users.noreply.github.com> --- .../browser/src/background/main.background.ts | 7 +- .../src/popup/services/services.module.ts | 8 +- .../fido2-user-verification.service.spec.ts | 31 ++++--- .../fido2-user-verification.service.ts | 16 +++- .../service-container/service-container.ts | 5 +- ...sktop-set-initial-password.service.spec.ts | 4 +- .../web-set-initial-password.service.spec.ts | 4 +- .../settings/account/account.component.ts | 10 +- .../password-settings.component.ts | 7 +- .../security/security-keys.component.ts | 22 ++++- .../settings/security/security.component.ts | 14 ++- .../organization-options.component.ts | 5 +- ...initial-password.service.implementation.ts | 7 +- ...fault-set-initial-password.service.spec.ts | 10 +- .../src/services/jslib-services.module.ts | 3 +- .../login-decryption-options.component.ts | 2 +- libs/auth/src/angular/sso/sso.component.ts | 2 +- .../two-factor-auth.component.spec.ts | 4 +- .../two-factor-auth.component.ts | 2 +- ...-decryption-options.service.abstraction.ts | 35 ++++--- .../login-strategies/login.strategy.spec.ts | 3 +- .../common/login-strategies/login.strategy.ts | 3 +- .../sso-login.strategy.spec.ts | 4 +- .../login-strategies/sso-login.strategy.ts | 2 +- .../user-decryption-options.service.spec.ts | 93 ++++++++++--------- .../user-decryption-options.service.ts | 42 ++++----- .../user-verification.service.abstraction.ts | 3 + .../user-verification.service.spec.ts | 23 +---- .../user-verification.service.ts | 19 ++-- .../device-trust.service.implementation.ts | 17 +++- .../services/device-trust.service.spec.ts | 3 +- .../vault-timeout-settings.service.spec.ts | 25 ++++- .../vault-timeout-settings.service.ts | 17 ++-- 33 files changed, 280 insertions(+), 172 deletions(-) diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index fecc47af981..78b5e323798 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -728,7 +728,9 @@ export default class MainBackground { this.appIdService = new AppIdService(this.storageService, this.logService); - this.userDecryptionOptionsService = new UserDecryptionOptionsService(this.stateProvider); + this.userDecryptionOptionsService = new UserDecryptionOptionsService( + this.singleUserStateProvider, + ); this.organizationService = new DefaultOrganizationService(this.stateProvider); this.policyService = new DefaultPolicyService(this.stateProvider, this.organizationService); @@ -859,8 +861,6 @@ export default class MainBackground { this.stateProvider, ); - this.userDecryptionOptionsService = new UserDecryptionOptionsService(this.stateProvider); - this.devicesApiService = new DevicesApiServiceImplementation(this.apiService); this.deviceTrustService = new DeviceTrustService( this.keyGenerationService, @@ -876,6 +876,7 @@ export default class MainBackground { this.userDecryptionOptionsService, this.logService, this.configService, + this.accountService, ); this.devicesService = new DevicesServiceImplementation( diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index eebf0a08a22..c462319dc2e 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -36,6 +36,7 @@ import { LoginEmailService, SsoUrlService, LogoutService, + UserDecryptionOptionsServiceAbstraction, } from "@bitwarden/auth/common"; import { ExtensionNewDeviceVerificationComponentService } from "@bitwarden/browser/auth/services/new-device-verification/extension-new-device-verification-component.service"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; @@ -607,7 +608,12 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: Fido2UserVerificationService, useClass: Fido2UserVerificationService, - deps: [PasswordRepromptService, UserVerificationService, DialogService], + deps: [ + PasswordRepromptService, + UserDecryptionOptionsServiceAbstraction, + DialogService, + AccountServiceAbstraction, + ], }), safeProvider({ provide: AnimationControlService, diff --git a/apps/browser/src/vault/services/fido2-user-verification.service.spec.ts b/apps/browser/src/vault/services/fido2-user-verification.service.spec.ts index 97a22bb2cf3..e55e3091244 100644 --- a/apps/browser/src/vault/services/fido2-user-verification.service.spec.ts +++ b/apps/browser/src/vault/services/fido2-user-verification.service.spec.ts @@ -1,11 +1,15 @@ import { MockProxy, mock } from "jest-mock-extended"; +import { of } from "rxjs"; import { UserVerificationDialogComponent } from "@bitwarden/auth/angular"; -import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; +import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; import { Utils } from "@bitwarden/common/platform/misc/utils"; +import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec"; +import { UserId } from "@bitwarden/common/types/guid"; import { CipherRepromptType, CipherType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { DialogService } from "@bitwarden/components"; +import { newGuid } from "@bitwarden/guid"; import { PasswordRepromptService } from "@bitwarden/vault"; // FIXME (PM-22628): Popup imports are forbidden in background @@ -31,21 +35,24 @@ describe("Fido2UserVerificationService", () => { let fido2UserVerificationService: Fido2UserVerificationService; let passwordRepromptService: MockProxy; - let userVerificationService: MockProxy; + let userDecryptionOptionsService: MockProxy; let dialogService: MockProxy; + let accountService: FakeAccountService; let cipher: CipherView; beforeEach(() => { passwordRepromptService = mock(); - userVerificationService = mock(); + userDecryptionOptionsService = mock(); dialogService = mock(); + accountService = mockAccountServiceWith(newGuid() as UserId); cipher = createCipherView(); fido2UserVerificationService = new Fido2UserVerificationService( passwordRepromptService, - userVerificationService, + userDecryptionOptionsService, dialogService, + accountService, ); (UserVerificationDialogComponent.open as jest.Mock).mockResolvedValue({ @@ -67,7 +74,7 @@ describe("Fido2UserVerificationService", () => { it("should call master password reprompt dialog if user is redirected from lock screen, has master password and master password reprompt is required", async () => { cipher.reprompt = CipherRepromptType.Password; - userVerificationService.hasMasterPassword.mockResolvedValue(true); + userDecryptionOptionsService.hasMasterPasswordById$.mockReturnValue(of(true)); passwordRepromptService.showPasswordPrompt.mockResolvedValue(true); const result = await fido2UserVerificationService.handleUserVerification( @@ -82,7 +89,7 @@ describe("Fido2UserVerificationService", () => { it("should call user verification dialog if user is redirected from lock screen, does not have a master password and master password reprompt is required", async () => { cipher.reprompt = CipherRepromptType.Password; - userVerificationService.hasMasterPassword.mockResolvedValue(false); + userDecryptionOptionsService.hasMasterPasswordById$.mockReturnValue(of(false)); const result = await fido2UserVerificationService.handleUserVerification( true, @@ -98,7 +105,7 @@ describe("Fido2UserVerificationService", () => { it("should call user verification dialog if user is not redirected from lock screen, does not have a master password and master password reprompt is required", async () => { cipher.reprompt = CipherRepromptType.Password; - userVerificationService.hasMasterPassword.mockResolvedValue(false); + userDecryptionOptionsService.hasMasterPasswordById$.mockReturnValue(of(false)); const result = await fido2UserVerificationService.handleUserVerification( true, @@ -114,7 +121,7 @@ describe("Fido2UserVerificationService", () => { it("should call master password reprompt dialog if user is not redirected from lock screen, has a master password and master password reprompt is required", async () => { cipher.reprompt = CipherRepromptType.Password; - userVerificationService.hasMasterPassword.mockResolvedValue(false); + userDecryptionOptionsService.hasMasterPasswordById$.mockReturnValue(of(false)); passwordRepromptService.showPasswordPrompt.mockResolvedValue(true); const result = await fido2UserVerificationService.handleUserVerification( @@ -176,7 +183,7 @@ describe("Fido2UserVerificationService", () => { it("should call master password reprompt dialog if user is redirected from lock screen, has master password and master password reprompt is required", async () => { cipher.reprompt = CipherRepromptType.Password; - userVerificationService.hasMasterPassword.mockResolvedValue(true); + userDecryptionOptionsService.hasMasterPasswordById$.mockReturnValue(of(true)); passwordRepromptService.showPasswordPrompt.mockResolvedValue(true); const result = await fido2UserVerificationService.handleUserVerification( @@ -191,7 +198,7 @@ describe("Fido2UserVerificationService", () => { it("should call user verification dialog if user is redirected from lock screen, does not have a master password and master password reprompt is required", async () => { cipher.reprompt = CipherRepromptType.Password; - userVerificationService.hasMasterPassword.mockResolvedValue(false); + userDecryptionOptionsService.hasMasterPasswordById$.mockReturnValue(of(false)); const result = await fido2UserVerificationService.handleUserVerification( false, @@ -207,7 +214,7 @@ describe("Fido2UserVerificationService", () => { it("should call user verification dialog if user is not redirected from lock screen, does not have a master password and master password reprompt is required", async () => { cipher.reprompt = CipherRepromptType.Password; - userVerificationService.hasMasterPassword.mockResolvedValue(false); + userDecryptionOptionsService.hasMasterPasswordById$.mockReturnValue(of(false)); const result = await fido2UserVerificationService.handleUserVerification( false, @@ -223,7 +230,7 @@ describe("Fido2UserVerificationService", () => { it("should call master password reprompt dialog if user is not redirected from lock screen, has a master password and master password reprompt is required", async () => { cipher.reprompt = CipherRepromptType.Password; - userVerificationService.hasMasterPassword.mockResolvedValue(false); + userDecryptionOptionsService.hasMasterPasswordById$.mockReturnValue(of(false)); passwordRepromptService.showPasswordPrompt.mockResolvedValue(true); const result = await fido2UserVerificationService.handleUserVerification( diff --git a/apps/browser/src/vault/services/fido2-user-verification.service.ts b/apps/browser/src/vault/services/fido2-user-verification.service.ts index 9bf9be70fc8..db3951d44d9 100644 --- a/apps/browser/src/vault/services/fido2-user-verification.service.ts +++ b/apps/browser/src/vault/services/fido2-user-verification.service.ts @@ -3,7 +3,8 @@ import { firstValueFrom } from "rxjs"; import { UserVerificationDialogComponent } from "@bitwarden/auth/angular"; -import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; +import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { DialogService } from "@bitwarden/components"; import { PasswordRepromptService } from "@bitwarden/vault"; @@ -15,8 +16,9 @@ import { SetPinComponent } from "../../auth/popup/components/set-pin.component"; export class Fido2UserVerificationService { constructor( private passwordRepromptService: PasswordRepromptService, - private userVerificationService: UserVerificationService, + private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction, private dialogService: DialogService, + private accountService: AccountService, ) {} /** @@ -78,7 +80,15 @@ export class Fido2UserVerificationService { } private async handleMasterPasswordReprompt(): Promise { - const hasMasterPassword = await this.userVerificationService.hasMasterPassword(); + const activeAccount = await firstValueFrom(this.accountService.activeAccount$); + + if (!activeAccount?.id) { + return false; + } + + const hasMasterPassword = await firstValueFrom( + this.userDecryptionOptionsService.hasMasterPasswordById$(activeAccount.id), + ); // TDE users have no master password, so we need to use the UserVerification prompt return hasMasterPassword diff --git a/apps/cli/src/service-container/service-container.ts b/apps/cli/src/service-container/service-container.ts index 365205a1329..122dd6ea052 100644 --- a/apps/cli/src/service-container/service-container.ts +++ b/apps/cli/src/service-container/service-container.ts @@ -512,7 +512,9 @@ export class ServiceContainer { ")"; this.biometricStateService = new DefaultBiometricStateService(this.stateProvider); - this.userDecryptionOptionsService = new UserDecryptionOptionsService(this.stateProvider); + this.userDecryptionOptionsService = new UserDecryptionOptionsService( + this.singleUserStateProvider, + ); this.ssoUrlService = new SsoUrlService(); this.organizationService = new DefaultOrganizationService(this.stateProvider); @@ -702,6 +704,7 @@ export class ServiceContainer { this.userDecryptionOptionsService, this.logService, this.configService, + this.accountService, ); this.loginStrategyService = new LoginStrategyService( diff --git a/apps/desktop/src/app/services/set-initial-password/desktop-set-initial-password.service.spec.ts b/apps/desktop/src/app/services/set-initial-password/desktop-set-initial-password.service.spec.ts index 53a1c7dbd4c..717af25a1dc 100644 --- a/apps/desktop/src/app/services/set-initial-password/desktop-set-initial-password.service.spec.ts +++ b/apps/desktop/src/app/services/set-initial-password/desktop-set-initial-password.service.spec.ts @@ -119,7 +119,9 @@ describe("DesktopSetInitialPasswordService", () => { userDecryptionOptions = new UserDecryptionOptions({ hasMasterPassword: true }); userDecryptionOptionsSubject = new BehaviorSubject(userDecryptionOptions); - userDecryptionOptionsService.userDecryptionOptions$ = userDecryptionOptionsSubject; + userDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue( + userDecryptionOptionsSubject, + ); setPasswordRequest = new SetPasswordRequest( credentials.newServerMasterKeyHash, diff --git a/apps/web/src/app/auth/core/services/password-management/set-initial-password/web-set-initial-password.service.spec.ts b/apps/web/src/app/auth/core/services/password-management/set-initial-password/web-set-initial-password.service.spec.ts index 70f7686a2cd..647c9ae83d9 100644 --- a/apps/web/src/app/auth/core/services/password-management/set-initial-password/web-set-initial-password.service.spec.ts +++ b/apps/web/src/app/auth/core/services/password-management/set-initial-password/web-set-initial-password.service.spec.ts @@ -123,7 +123,9 @@ describe("WebSetInitialPasswordService", () => { userDecryptionOptions = new UserDecryptionOptions({ hasMasterPassword: true }); userDecryptionOptionsSubject = new BehaviorSubject(userDecryptionOptions); - userDecryptionOptionsService.userDecryptionOptions$ = userDecryptionOptionsSubject; + userDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue( + userDecryptionOptionsSubject, + ); setPasswordRequest = new SetPasswordRequest( credentials.newServerMasterKeyHash, diff --git a/apps/web/src/app/auth/settings/account/account.component.ts b/apps/web/src/app/auth/settings/account/account.component.ts index 8bae8cd2c1f..3e618b89dbe 100644 --- a/apps/web/src/app/auth/settings/account/account.component.ts +++ b/apps/web/src/app/auth/settings/account/account.component.ts @@ -1,11 +1,10 @@ import { Component, OnInit, OnDestroy } from "@angular/core"; -import { firstValueFrom, from, lastValueFrom, map, Observable, Subject, takeUntil } from "rxjs"; +import { firstValueFrom, lastValueFrom, map, Observable, Subject, takeUntil } from "rxjs"; +import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; -import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { DialogService } from "@bitwarden/components"; import { HeaderModule } from "../../../layouts/header/header.module"; @@ -42,8 +41,7 @@ export class AccountComponent implements OnInit, OnDestroy { constructor( private accountService: AccountService, private dialogService: DialogService, - private userVerificationService: UserVerificationService, - private configService: ConfigService, + private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction, private organizationService: OrganizationService, ) {} @@ -56,7 +54,7 @@ export class AccountComponent implements OnInit, OnDestroy { map((organizations) => organizations.some((o) => o.userIsManagedByOrganization === true)), ); - const hasMasterPassword$ = from(this.userVerificationService.hasMasterPassword()); + const hasMasterPassword$ = this.userDecryptionOptionsService.hasMasterPasswordById$(userId); this.showChangeEmail$ = hasMasterPassword$; diff --git a/apps/web/src/app/auth/settings/security/password-settings/password-settings.component.ts b/apps/web/src/app/auth/settings/security/password-settings/password-settings.component.ts index 0e37c856935..ee283d26415 100644 --- a/apps/web/src/app/auth/settings/security/password-settings/password-settings.component.ts +++ b/apps/web/src/app/auth/settings/security/password-settings/password-settings.component.ts @@ -5,6 +5,8 @@ import { firstValueFrom } from "rxjs"; import { ChangePasswordComponent } from "@bitwarden/angular/auth/password-management/change-password"; import { InputPasswordFlow } from "@bitwarden/auth/angular"; import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { CalloutModule } from "@bitwarden/components"; import { I18nPipe } from "@bitwarden/ui-common"; @@ -24,12 +26,15 @@ export class PasswordSettingsComponent implements OnInit { constructor( private router: Router, private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction, + private accountService: AccountService, ) {} async ngOnInit() { + const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); const userHasMasterPassword = await firstValueFrom( - this.userDecryptionOptionsService.hasMasterPassword$, + this.userDecryptionOptionsService.hasMasterPasswordById$(userId), ); + if (!userHasMasterPassword) { await this.router.navigate(["/settings/security/two-factor"]); return; diff --git a/apps/web/src/app/auth/settings/security/security-keys.component.ts b/apps/web/src/app/auth/settings/security/security-keys.component.ts index 27a555ff343..b62828a2783 100644 --- a/apps/web/src/app/auth/settings/security/security-keys.component.ts +++ b/apps/web/src/app/auth/settings/security/security-keys.component.ts @@ -1,11 +1,10 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { Component, OnInit } from "@angular/core"; import { firstValueFrom, map } from "rxjs"; +import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; -import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { DialogService } from "@bitwarden/components"; import { ChangeKdfModule } from "../../../key-management/change-kdf/change-kdf.module"; @@ -23,20 +22,28 @@ export class SecurityKeysComponent implements OnInit { showChangeKdf = true; constructor( - private userVerificationService: UserVerificationService, + private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction, private accountService: AccountService, private apiService: ApiService, private dialogService: DialogService, ) {} async ngOnInit() { - this.showChangeKdf = await this.userVerificationService.hasMasterPassword(); + const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + this.showChangeKdf = await firstValueFrom( + this.userDecryptionOptionsService.hasMasterPasswordById$(userId), + ); } async viewUserApiKey() { const entityId = await firstValueFrom( this.accountService.activeAccount$.pipe(map((a) => a?.id)), ); + + if (!entityId) { + throw new Error("Active account not found"); + } + await ApiKeyComponent.open(this.dialogService, { data: { keyType: "user", @@ -55,6 +62,11 @@ export class SecurityKeysComponent implements OnInit { const entityId = await firstValueFrom( this.accountService.activeAccount$.pipe(map((a) => a?.id)), ); + + if (!entityId) { + throw new Error("Active account not found"); + } + await ApiKeyComponent.open(this.dialogService, { data: { keyType: "user", diff --git a/apps/web/src/app/auth/settings/security/security.component.ts b/apps/web/src/app/auth/settings/security/security.component.ts index 629de32efc4..85bc29fac63 100644 --- a/apps/web/src/app/auth/settings/security/security.component.ts +++ b/apps/web/src/app/auth/settings/security/security.component.ts @@ -1,7 +1,9 @@ import { Component, OnInit } from "@angular/core"; -import { Observable } from "rxjs"; +import { firstValueFrom, Observable } from "rxjs"; -import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; +import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; @@ -20,7 +22,8 @@ export class SecurityComponent implements OnInit { consolidatedSessionTimeoutComponent$: Observable; constructor( - private userVerificationService: UserVerificationService, + private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction, + private accountService: AccountService, private configService: ConfigService, ) { this.consolidatedSessionTimeoutComponent$ = this.configService.getFeatureFlag$( @@ -29,6 +32,9 @@ export class SecurityComponent implements OnInit { } async ngOnInit() { - this.showChangePassword = await this.userVerificationService.hasMasterPassword(); + const userId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + this.showChangePassword = userId + ? await firstValueFrom(this.userDecryptionOptionsService.hasMasterPasswordById$(userId)) + : false; } } diff --git a/apps/web/src/app/vault/individual-vault/vault-filter/components/organization-options.component.ts b/apps/web/src/app/vault/individual-vault/vault-filter/components/organization-options.component.ts index 3b707f2d78c..37b881406e3 100644 --- a/apps/web/src/app/vault/individual-vault/vault-filter/components/organization-options.component.ts +++ b/apps/web/src/app/vault/individual-vault/vault-filter/components/organization-options.component.ts @@ -95,7 +95,10 @@ export class OrganizationOptionsComponent implements OnInit, OnDestroy { combineLatest([ this.organization$, resetPasswordPolicies$, - this.userDecryptionOptionsService.userDecryptionOptions$, + this.accountService.activeAccount$.pipe( + getUserId, + switchMap((userId) => this.userDecryptionOptionsService.userDecryptionOptionsById$(userId)), + ), managingOrg$, ]) .pipe(takeUntil(this.destroy$)) diff --git a/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.implementation.ts b/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.implementation.ts index dcd4fd93cba..df5220b5255 100644 --- a/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.implementation.ts +++ b/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.implementation.ts @@ -198,10 +198,13 @@ export class DefaultSetInitialPasswordService implements SetInitialPasswordServi userId: UserId, ) { const userDecryptionOpts = await firstValueFrom( - this.userDecryptionOptionsService.userDecryptionOptions$, + this.userDecryptionOptionsService.userDecryptionOptionsById$(userId), ); userDecryptionOpts.hasMasterPassword = true; - await this.userDecryptionOptionsService.setUserDecryptionOptions(userDecryptionOpts); + await this.userDecryptionOptionsService.setUserDecryptionOptionsById( + userId, + userDecryptionOpts, + ); await this.kdfConfigService.setKdfConfig(userId, kdfConfig); await this.masterPasswordService.setMasterKey(masterKey, userId); await this.keyService.setUserKey(masterKeyEncryptedUserKey[0], userId); diff --git a/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.spec.ts b/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.spec.ts index 7a1dfc91e67..8b95090e776 100644 --- a/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.spec.ts +++ b/libs/angular/src/auth/password-management/set-initial-password/default-set-initial-password.service.spec.ts @@ -149,7 +149,9 @@ describe("DefaultSetInitialPasswordService", () => { userDecryptionOptions = new UserDecryptionOptions({ hasMasterPassword: true }); userDecryptionOptionsSubject = new BehaviorSubject(userDecryptionOptions); - userDecryptionOptionsService.userDecryptionOptions$ = userDecryptionOptionsSubject; + userDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue( + userDecryptionOptionsSubject, + ); setPasswordRequest = new SetPasswordRequest( credentials.newServerMasterKeyHash, @@ -362,7 +364,8 @@ describe("DefaultSetInitialPasswordService", () => { // Assert expect(masterPasswordApiService.setPassword).toHaveBeenCalledWith(setPasswordRequest); - expect(userDecryptionOptionsService.setUserDecryptionOptions).toHaveBeenCalledWith( + expect(userDecryptionOptionsService.setUserDecryptionOptionsById).toHaveBeenCalledWith( + userId, userDecryptionOptions, ); expect(kdfConfigService.setKdfConfig).toHaveBeenCalledWith(userId, credentials.kdfConfig); @@ -560,7 +563,8 @@ describe("DefaultSetInitialPasswordService", () => { // Assert expect(masterPasswordApiService.setPassword).toHaveBeenCalledWith(setPasswordRequest); - expect(userDecryptionOptionsService.setUserDecryptionOptions).toHaveBeenCalledWith( + expect(userDecryptionOptionsService.setUserDecryptionOptionsById).toHaveBeenCalledWith( + userId, userDecryptionOptions, ); expect(kdfConfigService.setKdfConfig).toHaveBeenCalledWith(userId, credentials.kdfConfig); diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index ff5caff540c..bcb601a993c 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -684,7 +684,7 @@ const safeProviders: SafeProvider[] = [ safeProvider({ provide: InternalUserDecryptionOptionsServiceAbstraction, useClass: UserDecryptionOptionsService, - deps: [StateProvider], + deps: [SingleUserStateProvider], }), safeProvider({ provide: UserDecryptionOptionsServiceAbstraction, @@ -1292,6 +1292,7 @@ const safeProviders: SafeProvider[] = [ UserDecryptionOptionsServiceAbstraction, LogService, ConfigService, + AccountServiceAbstraction, ], }), safeProvider({ diff --git a/libs/auth/src/angular/login-decryption-options/login-decryption-options.component.ts b/libs/auth/src/angular/login-decryption-options/login-decryption-options.component.ts index 26293285008..fb07069998b 100644 --- a/libs/auth/src/angular/login-decryption-options/login-decryption-options.component.ts +++ b/libs/auth/src/angular/login-decryption-options/login-decryption-options.component.ts @@ -135,7 +135,7 @@ export class LoginDecryptionOptionsComponent implements OnInit { try { const userDecryptionOptions = await firstValueFrom( - this.userDecryptionOptionsService.userDecryptionOptions$, + this.userDecryptionOptionsService.userDecryptionOptionsById$(this.activeAccountId), ); if ( diff --git a/libs/auth/src/angular/sso/sso.component.ts b/libs/auth/src/angular/sso/sso.component.ts index 0b6bb1159f4..bf618ba39f4 100644 --- a/libs/auth/src/angular/sso/sso.component.ts +++ b/libs/auth/src/angular/sso/sso.component.ts @@ -460,7 +460,7 @@ export class SsoComponent implements OnInit { // must come after 2fa check since user decryption options aren't available if 2fa is required const userDecryptionOpts = await firstValueFrom( - this.userDecryptionOptionsService.userDecryptionOptions$, + this.userDecryptionOptionsService.userDecryptionOptionsById$(authResult.userId), ); const tdeEnabled = userDecryptionOpts.trustedDeviceOption diff --git a/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.spec.ts b/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.spec.ts index af9fb03e01e..8c12060168b 100644 --- a/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.spec.ts +++ b/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.spec.ts @@ -176,7 +176,9 @@ describe("TwoFactorAuthComponent", () => { selectedUserDecryptionOptions = new BehaviorSubject( mockUserDecryptionOpts.withMasterPassword, ); - mockUserDecryptionOptionsService.userDecryptionOptions$ = selectedUserDecryptionOptions; + mockUserDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue( + selectedUserDecryptionOptions, + ); TestBed.configureTestingModule({ declarations: [TestTwoFactorComponent], diff --git a/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts b/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts index 8e10539823d..ca19d3652bb 100644 --- a/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts +++ b/libs/auth/src/angular/two-factor-auth/two-factor-auth.component.ts @@ -473,7 +473,7 @@ export class TwoFactorAuthComponent implements OnInit, OnDestroy { } const userDecryptionOpts = await firstValueFrom( - this.userDecryptionOptionsService.userDecryptionOptions$, + this.userDecryptionOptionsService.userDecryptionOptionsById$(authResult.userId), ); const tdeEnabled = await this.isTrustedDeviceEncEnabled(userDecryptionOpts.trustedDeviceOption); diff --git a/libs/auth/src/common/abstractions/user-decryption-options.service.abstraction.ts b/libs/auth/src/common/abstractions/user-decryption-options.service.abstraction.ts index e46fb09cff6..443e43b3151 100644 --- a/libs/auth/src/common/abstractions/user-decryption-options.service.abstraction.ts +++ b/libs/auth/src/common/abstractions/user-decryption-options.service.abstraction.ts @@ -1,34 +1,45 @@ import { Observable } from "rxjs"; +import { UserId } from "@bitwarden/common/types/guid"; + import { UserDecryptionOptions } from "../models"; +/** + * Public service for reading user decryption options. + * For use in components and services that need to evaluate user decryption settings. + */ export abstract class UserDecryptionOptionsServiceAbstraction { /** - * Returns what decryption options are available for the current user. - * @remark This is sent from the server on authentication. + * Returns the user decryption options for the given user id. + * Will only emit when options are set (does not emit null/undefined + * for an unpopulated state), and should not be called in an unauthenticated context. + * @param userId The user id to check. */ - abstract userDecryptionOptions$: Observable; + abstract userDecryptionOptionsById$(userId: UserId): Observable; /** * Uses user decryption options to determine if current user has a master password. * @remark This is sent from the server, and does not indicate if the master password * was used to login and/or if a master key is saved locally. */ - abstract hasMasterPassword$: Observable; - - /** - * Returns the user decryption options for the given user id. - * @param userId The user id to check. - */ - abstract userDecryptionOptionsById$(userId: string): Observable; + abstract hasMasterPasswordById$(userId: UserId): Observable; } +/** + * Internal service for managing user decryption options. + * For use only in authentication flows that need to update decryption options + * (e.g., login strategies). Extends consumer methods from {@link UserDecryptionOptionsServiceAbstraction}. + * @remarks Most consumers should use UserDecryptionOptionsServiceAbstraction instead. + */ export abstract class InternalUserDecryptionOptionsServiceAbstraction extends UserDecryptionOptionsServiceAbstraction { /** - * Sets the current decryption options for the user, contains the current configuration + * Sets the current decryption options for the user. Contains the current configuration * of the users account related to how they can decrypt their vault. * @remark Intended to be used when user decryption options are received from server, does * not update the server. Consider syncing instead of updating locally. * @param userDecryptionOptions Current user decryption options received from server. */ - abstract setUserDecryptionOptions(userDecryptionOptions: UserDecryptionOptions): Promise; + abstract setUserDecryptionOptionsById( + userId: UserId, + userDecryptionOptions: UserDecryptionOptions, + ): Promise; } diff --git a/libs/auth/src/common/login-strategies/login.strategy.spec.ts b/libs/auth/src/common/login-strategies/login.strategy.spec.ts index e9eed27d5a1..38d62cfdd83 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.spec.ts @@ -257,7 +257,8 @@ describe("LoginStrategy", () => { expect(environmentService.seedUserEnvironment).toHaveBeenCalled(); - expect(userDecryptionOptionsService.setUserDecryptionOptions).toHaveBeenCalledWith( + expect(userDecryptionOptionsService.setUserDecryptionOptionsById).toHaveBeenCalledWith( + userId, UserDecryptionOptions.fromResponse(idTokenResponse), ); expect(masterPasswordService.mock.setMasterPasswordUnlockData).toHaveBeenCalledWith( diff --git a/libs/auth/src/common/login-strategies/login.strategy.ts b/libs/auth/src/common/login-strategies/login.strategy.ts index 35f13246593..b8e4ee9e822 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.ts @@ -195,7 +195,8 @@ export abstract class LoginStrategy { // We must set user decryption options before retrieving vault timeout settings // as the user decryption options help determine the available timeout actions. - await this.userDecryptionOptionsService.setUserDecryptionOptions( + await this.userDecryptionOptionsService.setUserDecryptionOptionsById( + userId, UserDecryptionOptions.fromResponse(tokenResponse), ); diff --git a/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts index 03de5f36c2d..acbb680ae6d 100644 --- a/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts @@ -134,7 +134,9 @@ describe("SsoLoginStrategy", () => { ); const userDecryptionOptions = new UserDecryptionOptions(); - userDecryptionOptionsService.userDecryptionOptions$ = of(userDecryptionOptions); + userDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue( + of(userDecryptionOptions), + ); ssoLoginStrategy = new SsoLoginStrategy( {} as SsoLoginStrategyData, diff --git a/libs/auth/src/common/login-strategies/sso-login.strategy.ts b/libs/auth/src/common/login-strategies/sso-login.strategy.ts index ec7914b087e..d806f6d733e 100644 --- a/libs/auth/src/common/login-strategies/sso-login.strategy.ts +++ b/libs/auth/src/common/login-strategies/sso-login.strategy.ts @@ -393,7 +393,7 @@ export class SsoLoginStrategy extends LoginStrategy { // Check for TDE-related conditions const userDecryptionOptions = await firstValueFrom( - this.userDecryptionOptionsService.userDecryptionOptions$, + this.userDecryptionOptionsService.userDecryptionOptionsById$(userId), ); if (!userDecryptionOptions) { diff --git a/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.spec.ts b/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.spec.ts index c2fafc1a2f6..efb00b9aa63 100644 --- a/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.spec.ts +++ b/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.spec.ts @@ -1,12 +1,8 @@ import { firstValueFrom } from "rxjs"; -import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { - FakeAccountService, - FakeStateProvider, - mockAccountServiceWith, -} from "@bitwarden/common/spec"; +import { FakeSingleUserStateProvider } from "@bitwarden/common/spec"; import { UserId } from "@bitwarden/common/types/guid"; +import { newGuid } from "@bitwarden/guid"; import { UserDecryptionOptions } from "../../models/domain/user-decryption-options"; @@ -17,15 +13,10 @@ import { describe("UserDecryptionOptionsService", () => { let sut: UserDecryptionOptionsService; - - const fakeUserId = Utils.newGuid() as UserId; - let fakeAccountService: FakeAccountService; - let fakeStateProvider: FakeStateProvider; + let fakeStateProvider: FakeSingleUserStateProvider; beforeEach(() => { - fakeAccountService = mockAccountServiceWith(fakeUserId); - fakeStateProvider = new FakeStateProvider(fakeAccountService); - + fakeStateProvider = new FakeSingleUserStateProvider(); sut = new UserDecryptionOptionsService(fakeStateProvider); }); @@ -42,55 +33,71 @@ describe("UserDecryptionOptionsService", () => { }, }; - describe("userDecryptionOptions$", () => { - it("should return the active user's decryption options", async () => { - await fakeStateProvider.setUserState(USER_DECRYPTION_OPTIONS, userDecryptionOptions); + describe("userDecryptionOptionsById$", () => { + it("should return user decryption options for a specific user", async () => { + const userId = newGuid() as UserId; - const result = await firstValueFrom(sut.userDecryptionOptions$); + fakeStateProvider.getFake(userId, USER_DECRYPTION_OPTIONS).nextState(userDecryptionOptions); + + const result = await firstValueFrom(sut.userDecryptionOptionsById$(userId)); expect(result).toEqual(userDecryptionOptions); }); }); - describe("hasMasterPassword$", () => { - it("should return the hasMasterPassword property of the active user's decryption options", async () => { - await fakeStateProvider.setUserState(USER_DECRYPTION_OPTIONS, userDecryptionOptions); + describe("hasMasterPasswordById$", () => { + it("should return true when user has a master password", async () => { + const userId = newGuid() as UserId; - const result = await firstValueFrom(sut.hasMasterPassword$); + fakeStateProvider.getFake(userId, USER_DECRYPTION_OPTIONS).nextState(userDecryptionOptions); + + const result = await firstValueFrom(sut.hasMasterPasswordById$(userId)); expect(result).toBe(true); }); - }); - describe("userDecryptionOptionsById$", () => { - it("should return the user decryption options for the given user", async () => { - const givenUser = Utils.newGuid() as UserId; - await fakeAccountService.addAccount(givenUser, { - name: "Test User 1", - email: "test1@email.com", - emailVerified: false, - }); - await fakeStateProvider.setUserState( - USER_DECRYPTION_OPTIONS, - userDecryptionOptions, - givenUser, - ); + it("should return false when user does not have a master password", async () => { + const userId = newGuid() as UserId; + const optionsWithoutMasterPassword = { + ...userDecryptionOptions, + hasMasterPassword: false, + }; - const result = await firstValueFrom(sut.userDecryptionOptionsById$(givenUser)); + fakeStateProvider + .getFake(userId, USER_DECRYPTION_OPTIONS) + .nextState(optionsWithoutMasterPassword); - expect(result).toEqual(userDecryptionOptions); + const result = await firstValueFrom(sut.hasMasterPasswordById$(userId)); + + expect(result).toBe(false); }); }); - describe("setUserDecryptionOptions", () => { - it("should set the active user's decryption options", async () => { - await sut.setUserDecryptionOptions(userDecryptionOptions); + describe("setUserDecryptionOptionsById", () => { + it("should set user decryption options for a specific user", async () => { + const userId = newGuid() as UserId; - const result = await firstValueFrom( - fakeStateProvider.getActive(USER_DECRYPTION_OPTIONS).state$, - ); + await sut.setUserDecryptionOptionsById(userId, userDecryptionOptions); + + const fakeState = fakeStateProvider.getFake(userId, USER_DECRYPTION_OPTIONS); + const result = await firstValueFrom(fakeState.state$); expect(result).toEqual(userDecryptionOptions); }); + + it("should overwrite existing user decryption options", async () => { + const userId = newGuid() as UserId; + const initialOptions = { ...userDecryptionOptions, hasMasterPassword: false }; + const updatedOptions = { ...userDecryptionOptions, hasMasterPassword: true }; + + const fakeState = fakeStateProvider.getFake(userId, USER_DECRYPTION_OPTIONS); + fakeState.nextState(initialOptions); + + await sut.setUserDecryptionOptionsById(userId, updatedOptions); + + const result = await firstValueFrom(fakeState.state$); + + expect(result).toEqual(updatedOptions); + }); }); }); diff --git a/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.ts b/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.ts index 7c44a6f1682..a0075d1987b 100644 --- a/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.ts +++ b/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.ts @@ -1,16 +1,11 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore -import { Observable, map } from "rxjs"; +import { Observable, filter, map } from "rxjs"; import { - ActiveUserState, - StateProvider, + SingleUserStateProvider, USER_DECRYPTION_OPTIONS_DISK, UserKeyDefinition, } from "@bitwarden/common/platform/state"; -// FIXME: remove `src` and fix import -// eslint-disable-next-line no-restricted-imports -import { UserId } from "@bitwarden/common/src/types/guid"; +import { UserId } from "@bitwarden/common/types/guid"; import { InternalUserDecryptionOptionsServiceAbstraction } from "../../abstractions/user-decryption-options.service.abstraction"; import { UserDecryptionOptions } from "../../models"; @@ -27,25 +22,26 @@ export const USER_DECRYPTION_OPTIONS = new UserKeyDefinition; + constructor(private singleUserStateProvider: SingleUserStateProvider) {} - userDecryptionOptions$: Observable; - hasMasterPassword$: Observable; + userDecryptionOptionsById$(userId: UserId): Observable { + return this.singleUserStateProvider + .get(userId, USER_DECRYPTION_OPTIONS) + .state$.pipe(filter((options): options is UserDecryptionOptions => options != null)); + } - constructor(private stateProvider: StateProvider) { - this.userDecryptionOptionsState = this.stateProvider.getActive(USER_DECRYPTION_OPTIONS); - - this.userDecryptionOptions$ = this.userDecryptionOptionsState.state$; - this.hasMasterPassword$ = this.userDecryptionOptions$.pipe( - map((options) => options?.hasMasterPassword ?? false), + hasMasterPasswordById$(userId: UserId): Observable { + return this.userDecryptionOptionsById$(userId).pipe( + map((options) => options.hasMasterPassword ?? false), ); } - userDecryptionOptionsById$(userId: UserId): Observable { - return this.stateProvider.getUser(userId, USER_DECRYPTION_OPTIONS).state$; - } - - async setUserDecryptionOptions(userDecryptionOptions: UserDecryptionOptions): Promise { - await this.userDecryptionOptionsState.update((_) => userDecryptionOptions); + async setUserDecryptionOptionsById( + userId: UserId, + userDecryptionOptions: UserDecryptionOptions, + ): Promise { + await this.singleUserStateProvider + .get(userId, USER_DECRYPTION_OPTIONS) + .update((_) => userDecryptionOptions); } } diff --git a/libs/common/src/auth/abstractions/user-verification/user-verification.service.abstraction.ts b/libs/common/src/auth/abstractions/user-verification/user-verification.service.abstraction.ts index d9749d9467c..b9bc9108e52 100644 --- a/libs/common/src/auth/abstractions/user-verification/user-verification.service.abstraction.ts +++ b/libs/common/src/auth/abstractions/user-verification/user-verification.service.abstraction.ts @@ -48,6 +48,9 @@ export abstract class UserVerificationService { * @param userId The user id to check. If not provided, the current user is used * @returns True if the user has a master password * @deprecated Use UserDecryptionOptionsService.hasMasterPassword$ instead + * @remark To facilitate deprecation, many call sites were removed as part of PM-26413. + * Those remaining are blocked by currently-disallowed imports of auth/common. + * PM-27009 has been filed to track completion of this deprecation. */ abstract hasMasterPassword(userId?: string): Promise; /** diff --git a/libs/common/src/auth/services/user-verification/user-verification.service.spec.ts b/libs/common/src/auth/services/user-verification/user-verification.service.spec.ts index e7fbc002af8..e570c0f4a43 100644 --- a/libs/common/src/auth/services/user-verification/user-verification.service.spec.ts +++ b/libs/common/src/auth/services/user-verification/user-verification.service.spec.ts @@ -3,10 +3,7 @@ import { of } 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 { - UserDecryptionOptions, - UserDecryptionOptionsServiceAbstraction, -} from "@bitwarden/auth/common"; +import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common"; // 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 { @@ -146,11 +143,7 @@ describe("UserVerificationService", () => { describe("server verification type", () => { it("correctly returns master password availability", async () => { - userDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue( - of({ - hasMasterPassword: true, - } as UserDecryptionOptions), - ); + userDecryptionOptionsService.hasMasterPasswordById$.mockReturnValue(of(true)); const result = await sut.getAvailableVerificationOptions("server"); @@ -168,11 +161,7 @@ describe("UserVerificationService", () => { }); it("correctly returns OTP availability", async () => { - userDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue( - of({ - hasMasterPassword: false, - } as UserDecryptionOptions), - ); + userDecryptionOptionsService.hasMasterPasswordById$.mockReturnValue(of(false)); const result = await sut.getAvailableVerificationOptions("server"); @@ -526,11 +515,7 @@ describe("UserVerificationService", () => { // Helpers function setMasterPasswordAvailability(hasMasterPassword: boolean) { - userDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue( - of({ - hasMasterPassword: hasMasterPassword, - } as UserDecryptionOptions), - ); + userDecryptionOptionsService.hasMasterPasswordById$.mockReturnValue(of(hasMasterPassword)); masterPasswordService.masterKeyHash$.mockReturnValue( of(hasMasterPassword ? "masterKeyHash" : null), ); diff --git a/libs/common/src/auth/services/user-verification/user-verification.service.ts b/libs/common/src/auth/services/user-verification/user-verification.service.ts index 7a537b883e3..7d93120148b 100644 --- a/libs/common/src/auth/services/user-verification/user-verification.service.ts +++ b/libs/common/src/auth/services/user-verification/user-verification.service.ts @@ -258,16 +258,19 @@ export class UserVerificationService implements UserVerificationServiceAbstracti } async hasMasterPassword(userId?: string): Promise { - if (userId) { - const decryptionOptions = await firstValueFrom( - this.userDecryptionOptionsService.userDecryptionOptionsById$(userId), - ); + const resolvedUserId = userId ?? (await firstValueFrom(this.accountService.activeAccount$))?.id; - if (decryptionOptions?.hasMasterPassword != undefined) { - return decryptionOptions.hasMasterPassword; - } + if (!resolvedUserId) { + return false; } - return await firstValueFrom(this.userDecryptionOptionsService.hasMasterPassword$); + + // Ideally, this method would accept a UserId over string. To avoid scope creep in PM-26413, we are + // doing the cast here. Future work should be done to make this type-safe, and should be considered + // as part of PM-27009. + + return await firstValueFrom( + this.userDecryptionOptionsService.hasMasterPasswordById$(resolvedUserId as UserId), + ); } async hasMasterPasswordAndMasterKeyHash(userId?: string): Promise { diff --git a/libs/common/src/key-management/device-trust/services/device-trust.service.implementation.ts b/libs/common/src/key-management/device-trust/services/device-trust.service.implementation.ts index aa14c7f0c4f..59bd7bc11f2 100644 --- a/libs/common/src/key-management/device-trust/services/device-trust.service.implementation.ts +++ b/libs/common/src/key-management/device-trust/services/device-trust.service.implementation.ts @@ -1,6 +1,6 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore -import { firstValueFrom, map, Observable, Subject } from "rxjs"; +import { firstValueFrom, map, Observable, Subject, switchMap } 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 @@ -9,6 +9,7 @@ import { UserDecryptionOptionsServiceAbstraction } from "@bitwarden/auth/common" // eslint-disable-next-line no-restricted-imports import { KeyService } from "@bitwarden/key-management"; +import { AccountService } from "../../../auth/abstractions/account.service"; import { DeviceResponse } from "../../../auth/abstractions/devices/responses/device.response"; import { DevicesApiServiceAbstraction } from "../../../auth/abstractions/devices-api.service.abstraction"; import { SecretVerificationRequest } from "../../../auth/models/request/secret-verification.request"; @@ -87,10 +88,18 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction { private userDecryptionOptionsService: UserDecryptionOptionsServiceAbstraction, private logService: LogService, private configService: ConfigService, + private accountService: AccountService, ) { - this.supportsDeviceTrust$ = this.userDecryptionOptionsService.userDecryptionOptions$.pipe( - map((options) => { - return options?.trustedDeviceOption != null; + this.supportsDeviceTrust$ = this.accountService.activeAccount$.pipe( + switchMap((account) => { + if (account == null) { + return [false]; + } + return this.userDecryptionOptionsService.userDecryptionOptionsById$(account.id).pipe( + map((options) => { + return options?.trustedDeviceOption != null; + }), + ); }), ); } diff --git a/libs/common/src/key-management/device-trust/services/device-trust.service.spec.ts b/libs/common/src/key-management/device-trust/services/device-trust.service.spec.ts index e735295f42b..024a21766ee 100644 --- a/libs/common/src/key-management/device-trust/services/device-trust.service.spec.ts +++ b/libs/common/src/key-management/device-trust/services/device-trust.service.spec.ts @@ -914,7 +914,7 @@ describe("deviceTrustService", () => { platformUtilsService.supportsSecureStorage.mockReturnValue(supportsSecureStorage); decryptionOptions.next({} as any); - userDecryptionOptionsService.userDecryptionOptions$ = decryptionOptions; + userDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue(decryptionOptions); return new DeviceTrustService( keyGenerationService, @@ -930,6 +930,7 @@ describe("deviceTrustService", () => { userDecryptionOptionsService, logService, configService, + accountService, ); } }); diff --git a/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.spec.ts b/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.spec.ts index f3fec6d849a..ba58fa80922 100644 --- a/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.spec.ts +++ b/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.spec.ts @@ -53,9 +53,11 @@ describe("VaultTimeoutSettingsService", () => { policyService = mock(); userDecryptionOptionsSubject = new BehaviorSubject(null); - userDecryptionOptionsService.userDecryptionOptions$ = userDecryptionOptionsSubject; - userDecryptionOptionsService.hasMasterPassword$ = userDecryptionOptionsSubject.pipe( - map((options) => options?.hasMasterPassword ?? false), + userDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue( + userDecryptionOptionsSubject, + ); + userDecryptionOptionsService.hasMasterPasswordById$.mockReturnValue( + userDecryptionOptionsSubject.pipe(map((options) => options?.hasMasterPassword ?? false)), ); userDecryptionOptionsService.userDecryptionOptionsById$.mockReturnValue( userDecryptionOptionsSubject, @@ -127,6 +129,23 @@ describe("VaultTimeoutSettingsService", () => { expect(result).not.toContain(VaultTimeoutAction.Lock); }); + + it("should return only LogOut when userId is not provided and there is no active account", async () => { + // Set up accountService to return null for activeAccount + accountService.activeAccount$ = of(null); + pinStateService.isPinSet.mockResolvedValue(false); + biometricStateService.biometricUnlockEnabled$ = of(false); + + // Call availableVaultTimeoutActions$ which internally calls userHasMasterPassword without a userId + const result = await firstValueFrom( + vaultTimeoutSettingsService.availableVaultTimeoutActions$(), + ); + + // Since there's no active account, userHasMasterPassword returns false, + // meaning no master password is available, so Lock should not be available + expect(result).toEqual([VaultTimeoutAction.LogOut]); + expect(result).not.toContain(VaultTimeoutAction.Lock); + }); }); describe("canLock", () => { diff --git a/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.ts b/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.ts index 4ca23bb24bf..00e53596de4 100644 --- a/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.ts +++ b/libs/common/src/key-management/vault-timeout/services/vault-timeout-settings.service.ts @@ -290,14 +290,19 @@ export class VaultTimeoutSettingsService implements VaultTimeoutSettingsServiceA } private async userHasMasterPassword(userId: string): Promise { + let resolvedUserId: UserId; if (userId) { - const decryptionOptions = await firstValueFrom( - this.userDecryptionOptionsService.userDecryptionOptionsById$(userId), - ); - - return !!decryptionOptions?.hasMasterPassword; + resolvedUserId = userId as UserId; } else { - return await firstValueFrom(this.userDecryptionOptionsService.hasMasterPassword$); + const activeAccount = await firstValueFrom(this.accountService.activeAccount$); + if (!activeAccount) { + return false; // No account, can't have master password + } + resolvedUserId = activeAccount.id; } + + return await firstValueFrom( + this.userDecryptionOptionsService.hasMasterPasswordById$(resolvedUserId), + ); } } From 17ae78ea8315e5b266d7ff9af393c61848859825 Mon Sep 17 00:00:00 2001 From: Vincent Salucci <26154748+vincentsalucci@users.noreply.github.com> Date: Tue, 25 Nov 2025 11:23:01 -0600 Subject: [PATCH 08/22] chore: fix feature flag name, refs PM-27766 (#17660) --- libs/common/src/enums/feature-flag.enum.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 17d5f4e9df5..56a25cb213c 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -13,7 +13,7 @@ export enum FeatureFlag { /* Admin Console Team */ CreateDefaultLocation = "pm-19467-create-default-location", AutoConfirm = "pm-19934-auto-confirm-organization-users", - BlockClaimedDomainAccountCreation = "block-claimed-domain-account-creation", + BlockClaimedDomainAccountCreation = "pm-28297-block-uninvited-claimed-domain-registration", /* Auth */ PM23801_PrefetchPasswordPrelogin = "pm-23801-prefetch-password-prelogin", From 441783627b9af11a0ca339dcc7aa789787b3c468 Mon Sep 17 00:00:00 2001 From: Nick Krantz <125900171+nick-livefront@users.noreply.github.com> Date: Tue, 25 Nov 2025 11:28:34 -0600 Subject: [PATCH 09/22] [PM-26359] Archive Upgrade - Browser (#16904) * add archive upgrade flow to more options menu * add reprompt for archiving a cipher * add premium badge for archive in settings * update showArchive to only look at the feature flag * add premium badge for browser settings * add event to prompt for premium * formatting * update test --- apps/browser/src/_locales/en/messages.json | 3 ++ .../item-more-options.component.html | 24 +++++++++-- .../item-more-options.component.spec.ts | 5 ++- .../item-more-options.component.ts | 40 ++++++++++++------- .../settings/vault-settings-v2.component.html | 28 +++++++++---- .../settings/vault-settings-v2.component.ts | 21 +++++++--- 6 files changed, 90 insertions(+), 31 deletions(-) diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index 14915175da1..6009bcba1bd 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -585,6 +585,9 @@ "archiveItemConfirmDesc": { "message": "Archived items are excluded from general search results and autofill suggestions. Are you sure you want to archive this item?" }, + "upgradeToUseArchive": { + "message": "A premium membership is required to use Archive." + }, "edit": { "message": "Edit" }, diff --git a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.html b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.html index b6a7002139c..5c5171ac81d 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.html +++ b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.html @@ -51,10 +51,26 @@ {{ "assignToCollections" | i18n }} - @if (canArchive$ | async) { - + @if (showArchive$ | async) { + @if (canArchive$ | async) { + + } @else { + + } } @if (canDelete$ | async) { + + + + + Foo + Bar + + + `, + }), +}; + +export const Basic: Story = { + render: (args: any) => ({ + props: args, + template: /*html*/ ` + + `, + }), +}; + +export const WithLongTitle: Story = { + render: (arg: any) => ({ + props: arg, + template: /*html*/ ` + + + + `, + }), +}; + +export const WithBreadcrumbs: Story = { + render: (args: any) => ({ + props: args, + template: /*html*/ ` + + + Foo + Bar + + + `, + }), +}; + +export const WithSearch: Story = { + render: (args: any) => ({ + props: args, + template: /*html*/ ` + + + + `, + }), +}; + +export const WithSecondaryContent: Story = { + render: (args) => ({ + props: args, + template: /*html*/ ` + + + + `, + }), +}; + +export const WithTabs: Story = { + render: (args) => ({ + props: args, + template: /*html*/ ` + + + Foo + Bar + + + `, + }), +}; + +export const WithTitleSuffixComponent: Story = { + render: (args) => ({ + props: args, + template: /*html*/ ` + + + + `, + }), +}; diff --git a/libs/components/src/header/index.ts b/libs/components/src/header/index.ts new file mode 100644 index 00000000000..c2d38d375ed --- /dev/null +++ b/libs/components/src/header/index.ts @@ -0,0 +1 @@ +export * from "./header.component"; diff --git a/libs/components/src/index.ts b/libs/components/src/index.ts index 5346747d3b2..410a96f1cb3 100644 --- a/libs/components/src/index.ts +++ b/libs/components/src/index.ts @@ -19,6 +19,7 @@ export * from "./dialog"; export * from "./disclosure"; export * from "./drawer"; export * from "./form-field"; +export * from "./header"; export * from "./icon-button"; export * from "./icon"; export * from "./icon-tile"; From eae894123dfe275371b565591a9c6f1e53715b5d Mon Sep 17 00:00:00 2001 From: Jordan Aasen <166539328+jaasen-livefront@users.noreply.github.com> Date: Tue, 25 Nov 2025 10:33:21 -0800 Subject: [PATCH 11/22] [PM-28376] - update copy for autofill confirmation dialog url list expand button (#17594) * update copy for autofill confirmation dialog url list expand button * fix tests --- apps/browser/src/_locales/en/messages.json | 3 ++ ...utofill-confirmation-dialog.component.html | 2 +- ...fill-confirmation-dialog.component.spec.ts | 53 +++++++++++++++---- 3 files changed, 46 insertions(+), 12 deletions(-) diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index 6009bcba1bd..fe979e129f4 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -597,6 +597,9 @@ "viewAll": { "message": "View all" }, + "showAll": { + "message": "Show all" + }, "viewLess": { "message": "View less" }, diff --git a/apps/browser/src/vault/popup/components/vault-v2/autofill-confirmation-dialog/autofill-confirmation-dialog.component.html b/apps/browser/src/vault/popup/components/vault-v2/autofill-confirmation-dialog/autofill-confirmation-dialog.component.html index 6f61c5fa446..88bff47191a 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/autofill-confirmation-dialog/autofill-confirmation-dialog.component.html +++ b/apps/browser/src/vault/popup/components/vault-v2/autofill-confirmation-dialog/autofill-confirmation-dialog.component.html @@ -24,7 +24,7 @@ class="tw-text-sm tw-font-medium tw-cursor-pointer" (click)="toggleSavedUrlExpandedState()" > - {{ (savedUrlsExpanded() ? "viewLess" : "viewAll") | i18n }} + {{ (savedUrlsExpanded() ? "showLess" : "showAll") | i18n }}
diff --git a/apps/browser/src/vault/popup/components/vault-v2/autofill-confirmation-dialog/autofill-confirmation-dialog.component.spec.ts b/apps/browser/src/vault/popup/components/vault-v2/autofill-confirmation-dialog/autofill-confirmation-dialog.component.spec.ts index e40019d99b6..a28b8730109 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/autofill-confirmation-dialog/autofill-confirmation-dialog.component.spec.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/autofill-confirmation-dialog/autofill-confirmation-dialog.component.spec.ts @@ -91,6 +91,11 @@ describe("AutofillConfirmationDialogComponent", () => { jest.resetAllMocks(); }); + const findShowAll = (inFx?: ComponentFixture) => + (inFx || fixture).nativeElement.querySelector( + "button.tw-text-sm.tw-font-medium.tw-cursor-pointer", + ) as HTMLButtonElement | null; + it("normalizes currentUrl and savedUrls via Utils.getHostname", () => { expect(Utils.getHostname).toHaveBeenCalledTimes(1 + (params.savedUrls?.length ?? 0)); expect(component.currentUrl()).toBe("example.com"); @@ -191,21 +196,47 @@ describe("AutofillConfirmationDialogComponent", () => { expect(text).toContain("two.example.com"); }); - it("shows the 'view all' button when savedUrls > 1 and toggles the button text when clicked", () => { - const findViewAll = () => - fixture.nativeElement.querySelector( - "button.tw-text-sm.tw-font-medium.tw-cursor-pointer", - ) as HTMLButtonElement | null; - - let btn = findViewAll(); + it("shows the 'show all' button when savedUrls > 1", () => { + const btn = findShowAll(); expect(btn).toBeTruthy(); + expect(btn!.textContent).toContain("showAll"); + }); + it('hides the "show all" button when savedUrls is empty', async () => { + const newParams: AutofillConfirmationDialogParams = { + currentUrl: "https://bitwarden.com/help", + savedUrls: [], + }; + + const { fixture: vf } = await createFreshFixture({ params: newParams }); + vf.detectChanges(); + const btn = findShowAll(vf); + expect(btn).toBeNull(); + }); + + it("handles toggling of the 'show all' button correctly", async () => { + const { fixture: vf, component: vc } = await createFreshFixture(); + + let btn = findShowAll(vf); + expect(btn).toBeTruthy(); + expect(vc.savedUrlsExpanded()).toBe(false); + expect(btn!.textContent).toContain("showAll"); + + // click to expand btn!.click(); - fixture.detectChanges(); + vf.detectChanges(); - btn = findViewAll(); - expect(btn!.textContent).toContain("viewLess"); - expect(component.savedUrlsExpanded()).toBe(true); + btn = findShowAll(vf); + expect(btn!.textContent).toContain("showLess"); + expect(vc.savedUrlsExpanded()).toBe(true); + + // click to collapse + btn!.click(); + vf.detectChanges(); + + btn = findShowAll(vf); + expect(btn!.textContent).toContain("showAll"); + expect(vc.savedUrlsExpanded()).toBe(false); }); it("shows autofillWithoutAdding text on autofill button when viewOnly is false", () => { From 3de3bee08ff8d77b0333997b7c60202accd824f4 Mon Sep 17 00:00:00 2001 From: Daniel Riera Date: Tue, 25 Nov 2025 13:42:46 -0500 Subject: [PATCH 12/22] [PM-27821]Add validation of extension origin for uses of window.postMessage (#17476) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * PM-27821 - Replace chrome.runtime.getURL() with BrowserApi.getRuntimeURL() for consistency - Add extension origin validation for all window.postMessage calls - Implement token-based authentication for inline menu communications - Add message source validation (event.source === globalThis.parent) - Add command presence validation (- Update notification bar to validate message origins and commands - Add extensionOrigin property to services using postMessage - Generate session tokens for inline menu containers (32-char random) - Validate tokens in message handlers to prevent unauthorized commands * Add explicit token validation * only set when receiving the trusted initNotificationBar message * await windowmessageorigin before posting to parent * fix tests * the parent must include its origin in the message for notification bar race condition * reduce if statements to one block and comment * extract parentOrigin from the URL and set windoMessageOrigin accordingly * consolidate if statements * add bar.spec file * fix merge conflict --- .../background/notification.background.ts | 2 +- .../autofill/background/overlay.background.ts | 6 +- .../content/content-message-handler.spec.ts | 14 +- .../abstractions/notification-bar.ts | 1 + .../src/autofill/notification/bar.spec.ts | 121 ++++++++++++++++ apps/browser/src/autofill/notification/bar.ts | 34 ++++- .../autofill-inline-menu-button.ts | 1 + .../autofill-inline-menu-container.ts | 2 +- .../abstractions/autofill-inline-menu-list.ts | 1 + ...utofill-inline-menu-iframe.service.spec.ts | 12 +- .../autofill-inline-menu-iframe.service.ts | 10 +- .../autofill-inline-menu-button.spec.ts | 21 +-- .../list/autofill-inline-menu-list.spec.ts | 129 ++++++++++++------ .../autofill-inline-menu-container.ts | 11 +- .../autofill-inline-menu-page-element.ts | 42 ++++-- ...notifications-content.service.spec.ts.snap | 2 +- ...rlay-notifications-content.service.spec.ts | 5 +- .../overlay-notifications-content.service.ts | 16 ++- .../src/autofill/spec/autofill-mocks.ts | 2 + .../src/autofill/spec/testing-utils.ts | 8 +- 20 files changed, 344 insertions(+), 96 deletions(-) create mode 100644 apps/browser/src/autofill/notification/bar.spec.ts diff --git a/apps/browser/src/autofill/background/notification.background.ts b/apps/browser/src/autofill/background/notification.background.ts index de1514f0342..547c5ba1575 100644 --- a/apps/browser/src/autofill/background/notification.background.ts +++ b/apps/browser/src/autofill/background/notification.background.ts @@ -1344,7 +1344,7 @@ export default class NotificationBackground { return; } - const extensionUrl = chrome.runtime.getURL("popup/index.html"); + const extensionUrl = BrowserApi.getRuntimeURL("popup/index.html"); const unlockPopoutTabs = (await BrowserApi.tabsQuery({ url: `${extensionUrl}*` })).filter( (tab) => tab.url?.includes(`singleActionPopout=${AuthPopoutType.unlockExtension}`), ); diff --git a/apps/browser/src/autofill/background/overlay.background.ts b/apps/browser/src/autofill/background/overlay.background.ts index 0eb7d070de3..af8141f1ab8 100644 --- a/apps/browser/src/autofill/background/overlay.background.ts +++ b/apps/browser/src/autofill/background/overlay.background.ts @@ -2949,13 +2949,13 @@ export class OverlayBackground implements OverlayBackgroundInterface { (await this.checkFocusedFieldHasValue(port.sender.tab)) && (await this.shouldShowSaveLoginInlineMenuList(port.sender.tab)); - const iframeUrl = chrome.runtime.getURL( + const iframeUrl = BrowserApi.getRuntimeURL( `overlay/menu-${isInlineMenuListPort ? "list" : "button"}.html`, ); - const styleSheetUrl = chrome.runtime.getURL( + const styleSheetUrl = BrowserApi.getRuntimeURL( `overlay/menu-${isInlineMenuListPort ? "list" : "button"}.css`, ); - const extensionOrigin = new URL(iframeUrl).origin; + const extensionOrigin = iframeUrl ? new URL(iframeUrl).origin : null; this.postMessageToPort(port, { command: `initAutofillInlineMenu${isInlineMenuListPort ? "List" : "Button"}`, diff --git a/apps/browser/src/autofill/content/content-message-handler.spec.ts b/apps/browser/src/autofill/content/content-message-handler.spec.ts index fe023f344d6..874e1cc76ff 100644 --- a/apps/browser/src/autofill/content/content-message-handler.spec.ts +++ b/apps/browser/src/autofill/content/content-message-handler.spec.ts @@ -56,7 +56,11 @@ describe("ContentMessageHandler", () => { }); it("sends an authResult message", () => { - postWindowMessage({ command: "authResult", lastpass: true, code: "code", state: "state" }); + postWindowMessage( + { command: "authResult", lastpass: true, code: "code", state: "state" }, + "https://localhost/", + window, + ); expect(sendMessageSpy).toHaveBeenCalledWith({ command: "authResult", @@ -68,7 +72,11 @@ describe("ContentMessageHandler", () => { }); it("sends a webAuthnResult message", () => { - postWindowMessage({ command: "webAuthnResult", data: "data", remember: true }); + postWindowMessage( + { command: "webAuthnResult", data: "data", remember: true }, + "https://localhost/", + window, + ); expect(sendMessageSpy).toHaveBeenCalledWith({ command: "webAuthnResult", @@ -82,7 +90,7 @@ describe("ContentMessageHandler", () => { const mockCode = "mockCode"; const command = "duoResult"; - postWindowMessage({ command: command, code: mockCode }); + postWindowMessage({ command: command, code: mockCode }, "https://localhost/", window); expect(sendMessageSpy).toHaveBeenCalledWith({ command: command, diff --git a/apps/browser/src/autofill/notification/abstractions/notification-bar.ts b/apps/browser/src/autofill/notification/abstractions/notification-bar.ts index 7881d2f1cac..b23c3c17abb 100644 --- a/apps/browser/src/autofill/notification/abstractions/notification-bar.ts +++ b/apps/browser/src/autofill/notification/abstractions/notification-bar.ts @@ -51,6 +51,7 @@ type NotificationBarWindowMessage = { }; error?: string; initData?: NotificationBarIframeInitData; + parentOrigin?: string; }; type NotificationBarWindowMessageHandlers = { diff --git a/apps/browser/src/autofill/notification/bar.spec.ts b/apps/browser/src/autofill/notification/bar.spec.ts new file mode 100644 index 00000000000..ae60e2efc91 --- /dev/null +++ b/apps/browser/src/autofill/notification/bar.spec.ts @@ -0,0 +1,121 @@ +import { mock } from "jest-mock-extended"; + +import { postWindowMessage } from "../spec/testing-utils"; + +import { NotificationBarWindowMessage } from "./abstractions/notification-bar"; +import "./bar"; + +jest.mock("lit", () => ({ render: jest.fn() })); +jest.mock("@lit-labs/signals", () => ({ + signal: jest.fn((testValue) => ({ get: (): typeof testValue => testValue })), +})); +jest.mock("../content/components/notification/container", () => ({ + NotificationContainer: jest.fn(), +})); + +describe("NotificationBar iframe handleWindowMessage security", () => { + const trustedOrigin = "http://localhost"; + const maliciousOrigin = "https://malicious.com"; + + const createMessage = ( + overrides: Partial = {}, + ): NotificationBarWindowMessage => ({ + command: "initNotificationBar", + ...overrides, + }); + + beforeEach(() => { + Object.defineProperty(globalThis, "location", { + value: { search: `?parentOrigin=${encodeURIComponent(trustedOrigin)}` }, + writable: true, + configurable: true, + }); + Object.defineProperty(globalThis, "parent", { + value: mock(), + writable: true, + configurable: true, + }); + globalThis.dispatchEvent(new Event("load")); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it.each([ + { + description: "not from parent window", + message: () => createMessage(), + origin: trustedOrigin, + source: () => mock(), + }, + { + description: "with mismatched origin", + message: () => createMessage(), + origin: maliciousOrigin, + source: () => globalThis.parent, + }, + { + description: "without command field", + message: () => ({}), + origin: trustedOrigin, + source: () => globalThis.parent, + }, + { + description: "initNotificationBar with mismatched parentOrigin", + message: () => createMessage({ parentOrigin: maliciousOrigin }), + origin: trustedOrigin, + source: () => globalThis.parent, + }, + { + description: "when windowMessageOrigin is not set", + message: () => createMessage(), + origin: "different-origin", + source: () => globalThis.parent, + resetOrigin: true, + }, + { + description: "with null source", + message: () => createMessage(), + origin: trustedOrigin, + source: (): null => null, + }, + { + description: "with unknown command", + message: () => createMessage({ command: "unknownCommand" }), + origin: trustedOrigin, + source: () => globalThis.parent, + }, + ])("should reject messages $description", ({ message, origin, source, resetOrigin }) => { + if (resetOrigin) { + Object.defineProperty(globalThis, "location", { + value: { search: "" }, + writable: true, + configurable: true, + }); + } + const spy = jest.spyOn(globalThis.parent, "postMessage").mockImplementation(); + postWindowMessage(message(), origin, source()); + expect(spy).not.toHaveBeenCalled(); + }); + + it("should accept and handle valid trusted messages", () => { + const spy = jest.spyOn(globalThis.parent, "postMessage").mockImplementation(); + spy.mockClear(); + + const validMessage = createMessage({ + parentOrigin: trustedOrigin, + initData: { + type: "change", + isVaultLocked: false, + removeIndividualVault: false, + importType: null, + launchTimestamp: Date.now(), + }, + }); + postWindowMessage(validMessage, trustedOrigin, globalThis.parent); + expect(validMessage.command).toBe("initNotificationBar"); + expect(validMessage.parentOrigin).toBe(trustedOrigin); + expect(validMessage.initData).toBeDefined(); + }); +}); diff --git a/apps/browser/src/autofill/notification/bar.ts b/apps/browser/src/autofill/notification/bar.ts index 3673a9f7321..333f8d5e534 100644 --- a/apps/browser/src/autofill/notification/bar.ts +++ b/apps/browser/src/autofill/notification/bar.ts @@ -24,6 +24,13 @@ import { let notificationBarIframeInitData: NotificationBarIframeInitData = {}; let windowMessageOrigin: string; +const urlParams = new URLSearchParams(globalThis.location.search); +const trustedParentOrigin = urlParams.get("parentOrigin"); + +if (trustedParentOrigin) { + windowMessageOrigin = trustedParentOrigin; +} + const notificationBarWindowMessageHandlers: NotificationBarWindowMessageHandlers = { initNotificationBar: ({ message }) => initNotificationBar(message), saveCipherAttemptCompleted: ({ message }) => handleSaveCipherConfirmation(message), @@ -395,15 +402,27 @@ function setupWindowMessageListener() { } function handleWindowMessage(event: MessageEvent) { - if (!windowMessageOrigin) { - windowMessageOrigin = event.origin; - } - - if (event.origin !== windowMessageOrigin) { + if (event?.source !== globalThis.parent) { return; } const message = event.data as NotificationBarWindowMessage; + if (!message?.command) { + return; + } + + if (!windowMessageOrigin || event.origin !== windowMessageOrigin) { + return; + } + + if ( + message.command === "initNotificationBar" && + message.parentOrigin && + message.parentOrigin !== event.origin + ) { + return; + } + const handler = notificationBarWindowMessageHandlers[message.command]; if (!handler) { return; @@ -431,5 +450,8 @@ function getResolvedTheme(theme: Theme) { } function postMessageToParent(message: NotificationBarWindowMessage) { - globalThis.parent.postMessage(message, windowMessageOrigin || "*"); + if (!windowMessageOrigin) { + return; + } + globalThis.parent.postMessage(message, windowMessageOrigin); } diff --git a/apps/browser/src/autofill/overlay/inline-menu/abstractions/autofill-inline-menu-button.ts b/apps/browser/src/autofill/overlay/inline-menu/abstractions/autofill-inline-menu-button.ts index 642e7dd24e9..0836ecf5ff1 100644 --- a/apps/browser/src/autofill/overlay/inline-menu/abstractions/autofill-inline-menu-button.ts +++ b/apps/browser/src/autofill/overlay/inline-menu/abstractions/autofill-inline-menu-button.ts @@ -10,6 +10,7 @@ export type InitAutofillInlineMenuButtonMessage = UpdateAuthStatusMessage & { styleSheetUrl: string; translations: Record; portKey: string; + token: string; }; export type AutofillInlineMenuButtonWindowMessageHandlers = { diff --git a/apps/browser/src/autofill/overlay/inline-menu/abstractions/autofill-inline-menu-container.ts b/apps/browser/src/autofill/overlay/inline-menu/abstractions/autofill-inline-menu-container.ts index 64fa8dde124..98fd84373a8 100644 --- a/apps/browser/src/autofill/overlay/inline-menu/abstractions/autofill-inline-menu-container.ts +++ b/apps/browser/src/autofill/overlay/inline-menu/abstractions/autofill-inline-menu-container.ts @@ -5,7 +5,7 @@ import { InlineMenuCipherData } from "../../../background/abstractions/overlay.b export type AutofillInlineMenuContainerMessage = { command: string; portKey: string; - token?: string; + token: string; }; export type InitAutofillInlineMenuElementMessage = AutofillInlineMenuContainerMessage & { diff --git a/apps/browser/src/autofill/overlay/inline-menu/abstractions/autofill-inline-menu-list.ts b/apps/browser/src/autofill/overlay/inline-menu/abstractions/autofill-inline-menu-list.ts index f5e1fe08850..cf778ef7892 100644 --- a/apps/browser/src/autofill/overlay/inline-menu/abstractions/autofill-inline-menu-list.ts +++ b/apps/browser/src/autofill/overlay/inline-menu/abstractions/autofill-inline-menu-list.ts @@ -27,6 +27,7 @@ export type InitAutofillInlineMenuListMessage = AutofillInlineMenuListMessage & showInlineMenuAccountCreation?: boolean; showPasskeysLabels?: boolean; portKey: string; + token: string; generatedPassword?: string; showSaveLoginMenu?: boolean; }; diff --git a/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.spec.ts b/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.spec.ts index 9f2947c2e99..3bb86ee7876 100644 --- a/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.spec.ts +++ b/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.spec.ts @@ -191,7 +191,7 @@ describe("AutofillInlineMenuIframeService", () => { expect( autofillInlineMenuIframeService["iframe"].contentWindow.postMessage, - ).toHaveBeenCalledWith(message, "*"); + ).toHaveBeenCalledWith(message, autofillInlineMenuIframeService["extensionOrigin"]); }); it("handles port messages that are registered with the message handlers and does not pass the message on to the iframe", () => { @@ -217,7 +217,7 @@ describe("AutofillInlineMenuIframeService", () => { expect(autofillInlineMenuIframeService["portKey"]).toBe(portKey); expect( autofillInlineMenuIframeService["iframe"].contentWindow.postMessage, - ).toHaveBeenCalledWith(message, "*"); + ).toHaveBeenCalledWith(message, autofillInlineMenuIframeService["extensionOrigin"]); }); }); @@ -242,7 +242,7 @@ describe("AutofillInlineMenuIframeService", () => { expect(updateElementStylesSpy).not.toHaveBeenCalled(); expect( autofillInlineMenuIframeService["iframe"].contentWindow.postMessage, - ).toHaveBeenCalledWith(message, "*"); + ).toHaveBeenCalledWith(message, autofillInlineMenuIframeService["extensionOrigin"]); }); it("sets a light theme based on the user's system preferences", () => { @@ -262,7 +262,7 @@ describe("AutofillInlineMenuIframeService", () => { command: "initAutofillInlineMenuList", theme: ThemeType.Light, }, - "*", + autofillInlineMenuIframeService["extensionOrigin"], ); }); @@ -283,7 +283,7 @@ describe("AutofillInlineMenuIframeService", () => { command: "initAutofillInlineMenuList", theme: ThemeType.Dark, }, - "*", + autofillInlineMenuIframeService["extensionOrigin"], ); }); @@ -387,7 +387,7 @@ describe("AutofillInlineMenuIframeService", () => { command: "updateAutofillInlineMenuColorScheme", colorScheme: "normal", }, - "*", + autofillInlineMenuIframeService["extensionOrigin"], ); }); diff --git a/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.ts b/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.ts index 9a9821f643c..8b1423b1290 100644 --- a/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.ts +++ b/apps/browser/src/autofill/overlay/inline-menu/iframe-content/autofill-inline-menu-iframe.service.ts @@ -3,6 +3,7 @@ import { EVENTS } from "@bitwarden/common/autofill/constants"; import { ThemeTypes } from "@bitwarden/common/platform/enums"; +import { BrowserApi } from "../../../../platform/browser/browser-api"; import { sendExtensionMessage, setElementStyles } from "../../../utils"; import { BackgroundPortMessageHandlers, @@ -15,6 +16,7 @@ export class AutofillInlineMenuIframeService implements AutofillInlineMenuIframe private readonly sendExtensionMessage = sendExtensionMessage; private port: chrome.runtime.Port | null = null; private portKey: string; + private readonly extensionOrigin: string; private iframeMutationObserver: MutationObserver; private iframe: HTMLIFrameElement; private ariaAlertElement: HTMLDivElement; @@ -69,6 +71,7 @@ export class AutofillInlineMenuIframeService implements AutofillInlineMenuIframe private iframeTitle: string, private ariaAlert?: string, ) { + this.extensionOrigin = BrowserApi.getRuntimeURL("")?.slice(0, -1); this.iframeMutationObserver = new MutationObserver(this.handleMutations); } @@ -81,7 +84,7 @@ export class AutofillInlineMenuIframeService implements AutofillInlineMenuIframe * that is declared. */ initMenuIframe() { - this.defaultIframeAttributes.src = chrome.runtime.getURL("overlay/menu.html"); + this.defaultIframeAttributes.src = BrowserApi.getRuntimeURL("overlay/menu.html"); this.defaultIframeAttributes.title = this.iframeTitle; this.iframe = globalThis.document.createElement("iframe"); @@ -259,7 +262,10 @@ export class AutofillInlineMenuIframeService implements AutofillInlineMenuIframe } private postMessageToIFrame(message: any) { - this.iframe.contentWindow?.postMessage({ portKey: this.portKey, ...message }, "*"); + this.iframe.contentWindow?.postMessage( + { portKey: this.portKey, ...message }, + this.extensionOrigin, + ); } /** diff --git a/apps/browser/src/autofill/overlay/inline-menu/pages/button/autofill-inline-menu-button.spec.ts b/apps/browser/src/autofill/overlay/inline-menu/pages/button/autofill-inline-menu-button.spec.ts index 7fa07850f00..10f6c905342 100644 --- a/apps/browser/src/autofill/overlay/inline-menu/pages/button/autofill-inline-menu-button.spec.ts +++ b/apps/browser/src/autofill/overlay/inline-menu/pages/button/autofill-inline-menu-button.spec.ts @@ -1,5 +1,6 @@ import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { BrowserApi } from "../../../../../platform/browser/browser-api"; import { createInitAutofillInlineMenuButtonMessageMock } from "../../../../spec/autofill-mocks"; import { flushPromises, postWindowMessage } from "../../../../spec/testing-utils"; @@ -10,11 +11,11 @@ describe("AutofillInlineMenuButton", () => { let autofillInlineMenuButton: AutofillInlineMenuButton; const portKey: string = "inlineMenuButtonPortKey"; + const expectedOrigin = BrowserApi.getRuntimeURL("")?.slice(0, -1) || "chrome-extension://id"; beforeEach(() => { document.body.innerHTML = ``; autofillInlineMenuButton = document.querySelector("autofill-inline-menu-button"); - autofillInlineMenuButton["messageOrigin"] = "https://localhost/"; jest.spyOn(globalThis.document, "createElement"); jest.spyOn(globalThis.parent, "postMessage"); }); @@ -56,8 +57,8 @@ describe("AutofillInlineMenuButton", () => { autofillInlineMenuButton["buttonElement"].click(); expect(globalThis.parent.postMessage).toHaveBeenCalledWith( - { command: "autofillInlineMenuButtonClicked", portKey }, - "*", + { command: "autofillInlineMenuButtonClicked", portKey, token: "test-token" }, + expectedOrigin, ); }); }); @@ -70,7 +71,7 @@ describe("AutofillInlineMenuButton", () => { it("does not post a message to close the autofill inline menu if the element is focused during the focus check", async () => { jest.spyOn(globalThis.document, "hasFocus").mockReturnValue(true); - postWindowMessage({ command: "checkAutofillInlineMenuButtonFocused" }); + postWindowMessage({ command: "checkAutofillInlineMenuButtonFocused", token: "test-token" }); await flushPromises(); expect(globalThis.parent.postMessage).not.toHaveBeenCalledWith({ @@ -84,7 +85,7 @@ describe("AutofillInlineMenuButton", () => { .spyOn(autofillInlineMenuButton["buttonElement"], "querySelector") .mockReturnValue(autofillInlineMenuButton["buttonElement"]); - postWindowMessage({ command: "checkAutofillInlineMenuButtonFocused" }); + postWindowMessage({ command: "checkAutofillInlineMenuButtonFocused", token: "test-token" }); await flushPromises(); expect(globalThis.parent.postMessage).not.toHaveBeenCalledWith({ @@ -98,7 +99,7 @@ describe("AutofillInlineMenuButton", () => { jest .spyOn(autofillInlineMenuButton["buttonElement"], "querySelector") .mockReturnValue(autofillInlineMenuButton["buttonElement"]); - postWindowMessage({ command: "checkAutofillInlineMenuButtonFocused" }); + postWindowMessage({ command: "checkAutofillInlineMenuButtonFocused", token: "test-token" }); await flushPromises(); globalThis.document.dispatchEvent(new MouseEvent("mouseout")); @@ -113,12 +114,12 @@ describe("AutofillInlineMenuButton", () => { jest.spyOn(globalThis.document, "hasFocus").mockReturnValue(false); jest.spyOn(autofillInlineMenuButton["buttonElement"], "querySelector").mockReturnValue(null); - postWindowMessage({ command: "checkAutofillInlineMenuButtonFocused" }); + postWindowMessage({ command: "checkAutofillInlineMenuButtonFocused", token: "test-token" }); await flushPromises(); expect(globalThis.parent.postMessage).toHaveBeenCalledWith( - { command: "triggerDelayedAutofillInlineMenuClosure", portKey }, - "*", + { command: "triggerDelayedAutofillInlineMenuClosure", portKey, token: "test-token" }, + expectedOrigin, ); }); @@ -128,6 +129,7 @@ describe("AutofillInlineMenuButton", () => { postWindowMessage({ command: "updateAutofillInlineMenuButtonAuthStatus", authStatus: AuthenticationStatus.Unlocked, + token: "test-token", }); await flushPromises(); @@ -143,6 +145,7 @@ describe("AutofillInlineMenuButton", () => { postWindowMessage({ command: "updateAutofillInlineMenuColorScheme", colorScheme: "dark", + token: "test-token", }); await flushPromises(); diff --git a/apps/browser/src/autofill/overlay/inline-menu/pages/list/autofill-inline-menu-list.spec.ts b/apps/browser/src/autofill/overlay/inline-menu/pages/list/autofill-inline-menu-list.spec.ts index b4e480797da..81bf7240230 100644 --- a/apps/browser/src/autofill/overlay/inline-menu/pages/list/autofill-inline-menu-list.spec.ts +++ b/apps/browser/src/autofill/overlay/inline-menu/pages/list/autofill-inline-menu-list.spec.ts @@ -3,6 +3,7 @@ import { mock } from "jest-mock-extended"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { CipherType } from "@bitwarden/common/vault/enums"; +import { BrowserApi } from "../../../../../platform/browser/browser-api"; import { InlineMenuCipherData } from "../../../../background/abstractions/overlay.background"; import { createAutofillOverlayCipherDataMock, @@ -23,6 +24,7 @@ describe("AutofillInlineMenuList", () => { let autofillInlineMenuList: AutofillInlineMenuList | null; const portKey: string = "inlineMenuListPortKey"; + const expectedOrigin = BrowserApi.getRuntimeURL("")?.slice(0, -1) || "chrome-extension://id"; const events: { eventName: any; callback: any }[] = []; beforeEach(() => { @@ -67,8 +69,8 @@ describe("AutofillInlineMenuList", () => { unlockButton.dispatchEvent(new Event("click")); expect(globalThis.parent.postMessage).toHaveBeenCalledWith( - { command: "unlockVault", portKey }, - "*", + { command: "unlockVault", portKey, token: "test-token" }, + expectedOrigin, ); }); }); @@ -134,8 +136,13 @@ describe("AutofillInlineMenuList", () => { addVaultItemButton.dispatchEvent(new Event("click")); expect(globalThis.parent.postMessage).toHaveBeenCalledWith( - { command: "addNewVaultItem", portKey, addNewCipherType: CipherType.Login }, - "*", + { + command: "addNewVaultItem", + portKey, + addNewCipherType: CipherType.Login, + token: "test-token", + }, + expectedOrigin, ); }); }); @@ -324,8 +331,9 @@ describe("AutofillInlineMenuList", () => { inlineMenuCipherId: "1", usePasskey: false, portKey, + token: "test-token", }, - "*", + expectedOrigin, ); }); @@ -492,8 +500,13 @@ describe("AutofillInlineMenuList", () => { viewCipherButton.dispatchEvent(new Event("click")); expect(globalThis.parent.postMessage).toHaveBeenCalledWith( - { command: "viewSelectedCipher", inlineMenuCipherId: "1", portKey }, - "*", + { + command: "viewSelectedCipher", + inlineMenuCipherId: "1", + portKey, + token: "test-token", + }, + expectedOrigin, ); }); @@ -581,8 +594,13 @@ describe("AutofillInlineMenuList", () => { newVaultItemButtonSpy.dispatchEvent(new Event("click")); expect(globalThis.parent.postMessage).toHaveBeenCalledWith( - { command: "addNewVaultItem", portKey, addNewCipherType: CipherType.Login }, - "*", + { + command: "addNewVaultItem", + portKey, + addNewCipherType: CipherType.Login, + token: "test-token", + }, + expectedOrigin, ); }); @@ -826,8 +844,8 @@ describe("AutofillInlineMenuList", () => { fillGeneratedPasswordButton.dispatchEvent(new Event("click")); expect(globalThis.parent.postMessage).toHaveBeenCalledWith( - { command: "fillGeneratedPassword", portKey }, - "*", + { command: "fillGeneratedPassword", portKey, token: "test-token" }, + expectedOrigin, ); }); @@ -843,7 +861,7 @@ describe("AutofillInlineMenuList", () => { expect(globalThis.parent.postMessage).not.toHaveBeenCalledWith( { command: "fillGeneratedPassword", portKey }, - "*", + expectedOrigin, ); }); @@ -857,8 +875,8 @@ describe("AutofillInlineMenuList", () => { ); expect(globalThis.parent.postMessage).toHaveBeenCalledWith( - { command: "fillGeneratedPassword", portKey }, - "*", + { command: "fillGeneratedPassword", portKey, token: "test-token" }, + expectedOrigin, ); }); @@ -896,8 +914,8 @@ describe("AutofillInlineMenuList", () => { refreshGeneratedPasswordButton.dispatchEvent(new Event("click")); expect(globalThis.parent.postMessage).toHaveBeenCalledWith( - { command: "refreshGeneratedPassword", portKey }, - "*", + { command: "refreshGeneratedPassword", portKey, token: "test-token" }, + expectedOrigin, ); }); @@ -913,7 +931,7 @@ describe("AutofillInlineMenuList", () => { expect(globalThis.parent.postMessage).not.toHaveBeenCalledWith( { command: "refreshGeneratedPassword", portKey }, - "*", + expectedOrigin, ); }); @@ -927,8 +945,8 @@ describe("AutofillInlineMenuList", () => { ); expect(globalThis.parent.postMessage).toHaveBeenCalledWith( - { command: "refreshGeneratedPassword", portKey }, - "*", + { command: "refreshGeneratedPassword", portKey, token: "test-token" }, + expectedOrigin, ); }); @@ -972,7 +990,7 @@ describe("AutofillInlineMenuList", () => { it("does not post a `checkAutofillInlineMenuButtonFocused` message to the parent if the inline menu is currently focused", () => { jest.spyOn(globalThis.document, "hasFocus").mockReturnValue(true); - postWindowMessage({ command: "checkAutofillInlineMenuListFocused" }); + postWindowMessage({ command: "checkAutofillInlineMenuListFocused", token: "test-token" }); expect(globalThis.parent.postMessage).not.toHaveBeenCalled(); }); @@ -983,7 +1001,7 @@ describe("AutofillInlineMenuList", () => { .spyOn(autofillInlineMenuList["inlineMenuListContainer"], "querySelector") .mockReturnValue(autofillInlineMenuList["inlineMenuListContainer"]); - postWindowMessage({ command: "checkAutofillInlineMenuListFocused" }); + postWindowMessage({ command: "checkAutofillInlineMenuListFocused", token: "test-token" }); expect(globalThis.parent.postMessage).not.toHaveBeenCalled(); }); @@ -994,7 +1012,7 @@ describe("AutofillInlineMenuList", () => { jest .spyOn(autofillInlineMenuList["inlineMenuListContainer"], "querySelector") .mockReturnValue(autofillInlineMenuList["inlineMenuListContainer"]); - postWindowMessage({ command: "checkAutofillInlineMenuListFocused" }); + postWindowMessage({ command: "checkAutofillInlineMenuListFocused", token: "test-token" }); await flushPromises(); globalThis.document.dispatchEvent(new MouseEvent("mouseout")); @@ -1010,11 +1028,11 @@ describe("AutofillInlineMenuList", () => { .spyOn(autofillInlineMenuList["inlineMenuListContainer"], "querySelector") .mockReturnValue(null); - postWindowMessage({ command: "checkAutofillInlineMenuListFocused" }); + postWindowMessage({ command: "checkAutofillInlineMenuListFocused", token: "test-token" }); expect(globalThis.parent.postMessage).toHaveBeenCalledWith( - { command: "checkAutofillInlineMenuButtonFocused", portKey }, - "*", + { command: "checkAutofillInlineMenuButtonFocused", portKey, token: "test-token" }, + expectedOrigin, ); }); @@ -1022,7 +1040,7 @@ describe("AutofillInlineMenuList", () => { postWindowMessage(createInitAutofillInlineMenuListMessageMock()); const updateCiphersSpy = jest.spyOn(autofillInlineMenuList as any, "updateListItems"); - postWindowMessage({ command: "updateAutofillInlineMenuListCiphers" }); + postWindowMessage({ command: "updateAutofillInlineMenuListCiphers", token: "test-token" }); expect(updateCiphersSpy).toHaveBeenCalled(); }); @@ -1062,7 +1080,10 @@ describe("AutofillInlineMenuList", () => { postWindowMessage(createInitAutofillInlineMenuListMessageMock()); await flushPromises(); - postWindowMessage({ command: "updateAutofillInlineMenuGeneratedPassword" }); + postWindowMessage({ + command: "updateAutofillInlineMenuGeneratedPassword", + token: "test-token", + }); expect(buildColorizedPasswordElementSpy).not.toHaveBeenCalled(); }); @@ -1074,6 +1095,7 @@ describe("AutofillInlineMenuList", () => { postWindowMessage({ command: "updateAutofillInlineMenuGeneratedPassword", generatedPassword, + token: "test-token", }); expect(buildPasswordGeneratorSpy).toHaveBeenCalled(); @@ -1090,6 +1112,7 @@ describe("AutofillInlineMenuList", () => { postWindowMessage({ command: "updateAutofillInlineMenuGeneratedPassword", generatedPassword, + token: "test-token", }); expect(buildPasswordGeneratorSpy).toHaveBeenCalledTimes(1); @@ -1115,7 +1138,7 @@ describe("AutofillInlineMenuList", () => { ); await flushPromises(); - postWindowMessage({ command: "showSaveLoginInlineMenuList" }); + postWindowMessage({ command: "showSaveLoginInlineMenuList", token: "test-token" }); expect(buildSaveLoginInlineMenuSpy).not.toHaveBeenCalled(); }); @@ -1124,7 +1147,7 @@ describe("AutofillInlineMenuList", () => { postWindowMessage(createInitAutofillInlineMenuListMessageMock()); await flushPromises(); - postWindowMessage({ command: "showSaveLoginInlineMenuList" }); + postWindowMessage({ command: "showSaveLoginInlineMenuList", token: "test-token" }); expect(buildSaveLoginInlineMenuSpy).toHaveBeenCalled(); }); @@ -1143,7 +1166,7 @@ describe("AutofillInlineMenuList", () => { "setAttribute", ); - postWindowMessage({ command: "focusAutofillInlineMenuList" }); + postWindowMessage({ command: "focusAutofillInlineMenuList", token: "test-token" }); expect(inlineMenuContainerSetAttributeSpy).toHaveBeenCalledWith("role", "dialog"); expect(inlineMenuContainerSetAttributeSpy).toHaveBeenCalledWith("aria-modal", "true"); @@ -1161,7 +1184,7 @@ describe("AutofillInlineMenuList", () => { autofillInlineMenuList["inlineMenuListContainer"].querySelector("#unlock-button"); jest.spyOn(unlockButton as HTMLElement, "focus"); - postWindowMessage({ command: "focusAutofillInlineMenuList" }); + postWindowMessage({ command: "focusAutofillInlineMenuList", token: "test-token" }); expect((unlockButton as HTMLElement).focus).toBeCalled(); }); @@ -1173,7 +1196,7 @@ describe("AutofillInlineMenuList", () => { autofillInlineMenuList["inlineMenuListContainer"].querySelector("#new-item-button"); jest.spyOn(newItemButton as HTMLElement, "focus"); - postWindowMessage({ command: "focusAutofillInlineMenuList" }); + postWindowMessage({ command: "focusAutofillInlineMenuList", token: "test-token" }); expect((newItemButton as HTMLElement).focus).toBeCalled(); }); @@ -1184,7 +1207,7 @@ describe("AutofillInlineMenuList", () => { autofillInlineMenuList["inlineMenuListContainer"].querySelector(".fill-cipher-button"); jest.spyOn(firstCipherItem as HTMLElement, "focus"); - postWindowMessage({ command: "focusAutofillInlineMenuList" }); + postWindowMessage({ command: "focusAutofillInlineMenuList", token: "test-token" }); expect((firstCipherItem as HTMLElement).focus).toBeCalled(); }); @@ -1197,8 +1220,8 @@ describe("AutofillInlineMenuList", () => { globalThis.dispatchEvent(new Event("blur")); expect(globalThis.parent.postMessage).toHaveBeenCalledWith( - { command: "autofillInlineMenuBlurred", portKey }, - "*", + { command: "autofillInlineMenuBlurred", portKey, token: "test-token" }, + expectedOrigin, ); }); }); @@ -1220,8 +1243,13 @@ describe("AutofillInlineMenuList", () => { ); expect(globalThis.parent.postMessage).toHaveBeenCalledWith( - { command: "redirectAutofillInlineMenuFocusOut", direction: "previous", portKey }, - "*", + { + command: "redirectAutofillInlineMenuFocusOut", + direction: "previous", + portKey, + token: "test-token", + }, + expectedOrigin, ); }); @@ -1229,8 +1257,13 @@ describe("AutofillInlineMenuList", () => { globalThis.document.dispatchEvent(new KeyboardEvent("keydown", { code: "Tab" })); expect(globalThis.parent.postMessage).toHaveBeenCalledWith( - { command: "redirectAutofillInlineMenuFocusOut", direction: "next", portKey }, - "*", + { + command: "redirectAutofillInlineMenuFocusOut", + direction: "next", + portKey, + token: "test-token", + }, + expectedOrigin, ); }); @@ -1238,8 +1271,13 @@ describe("AutofillInlineMenuList", () => { globalThis.document.dispatchEvent(new KeyboardEvent("keydown", { code: "Escape" })); expect(globalThis.parent.postMessage).toHaveBeenCalledWith( - { command: "redirectAutofillInlineMenuFocusOut", direction: "current", portKey }, - "*", + { + command: "redirectAutofillInlineMenuFocusOut", + direction: "current", + portKey, + token: "test-token", + }, + expectedOrigin, ); }); }); @@ -1274,8 +1312,13 @@ describe("AutofillInlineMenuList", () => { autofillInlineMenuList["handleResizeObserver"](entries as unknown as ResizeObserverEntry[]); expect(globalThis.parent.postMessage).toHaveBeenCalledWith( - { command: "updateAutofillInlineMenuListHeight", styles: { height: "300px" }, portKey }, - "*", + { + command: "updateAutofillInlineMenuListHeight", + styles: { height: "300px" }, + portKey, + token: "test-token", + }, + expectedOrigin, ); }); }); diff --git a/apps/browser/src/autofill/overlay/inline-menu/pages/menu-container/autofill-inline-menu-container.ts b/apps/browser/src/autofill/overlay/inline-menu/pages/menu-container/autofill-inline-menu-container.ts index aea6ef30b99..6c61cfae6b4 100644 --- a/apps/browser/src/autofill/overlay/inline-menu/pages/menu-container/autofill-inline-menu-container.ts +++ b/apps/browser/src/autofill/overlay/inline-menu/pages/menu-container/autofill-inline-menu-container.ts @@ -1,5 +1,6 @@ import { EVENTS } from "@bitwarden/common/autofill/constants"; +import { BrowserApi } from "../../../../../platform/browser/browser-api"; import { generateRandomChars, setElementStyles } from "../../../../utils"; import { InitAutofillInlineMenuElementMessage, @@ -73,7 +74,7 @@ export class AutofillInlineMenuContainer { constructor() { this.token = generateRandomChars(32); - this.extensionOrigin = chrome.runtime.getURL("").slice(0, -1); + this.extensionOrigin = BrowserApi.getRuntimeURL("")?.slice(0, -1); globalThis.addEventListener("message", this.handleWindowMessage); } @@ -203,6 +204,9 @@ export class AutofillInlineMenuContainer { */ private handleWindowMessage = (event: MessageEvent) => { const message = event.data; + if (!message?.command) { + return; + } if (this.isForeignWindowMessage(event)) { return; } @@ -287,7 +291,10 @@ export class AutofillInlineMenuContainer { * every time the inline menu container is recreated. * */ - private isValidSessionToken(message: { token?: string }): boolean { + private isValidSessionToken(message: { token: string }): boolean { + if (!this.token || !message?.token || !message?.token.length) { + return false; + } return message.token === this.token; } diff --git a/apps/browser/src/autofill/overlay/inline-menu/pages/shared/autofill-inline-menu-page-element.ts b/apps/browser/src/autofill/overlay/inline-menu/pages/shared/autofill-inline-menu-page-element.ts index ea77e3e434d..5df6e7cd190 100644 --- a/apps/browser/src/autofill/overlay/inline-menu/pages/shared/autofill-inline-menu-page-element.ts +++ b/apps/browser/src/autofill/overlay/inline-menu/pages/shared/autofill-inline-menu-page-element.ts @@ -38,12 +38,8 @@ export class AutofillInlineMenuPageElement extends HTMLElement { styleSheetUrl: string, translations: Record, portKey: string, - token?: string, ): Promise { this.portKey = portKey; - if (token) { - this.token = token; - } this.translations = translations; globalThis.document.documentElement.setAttribute("lang", this.getTranslation("locale")); @@ -63,11 +59,16 @@ export class AutofillInlineMenuPageElement extends HTMLElement { * @param message - The message to post */ protected postMessageToParent(message: AutofillInlineMenuPageElementWindowMessage) { - const messageWithAuth: Record = { portKey: this.portKey, ...message }; - if (this.token) { - messageWithAuth.token = this.token; + // never send messages containing authentication tokens without a valid token and an established messageOrigin + if (!this.token || !this.messageOrigin) { + return; } - globalThis.parent.postMessage(messageWithAuth, "*"); + const messageWithAuth: Record = { + portKey: this.portKey, + ...message, + token: this.token, + }; + globalThis.parent.postMessage(messageWithAuth, this.messageOrigin); } /** @@ -105,6 +106,10 @@ export class AutofillInlineMenuPageElement extends HTMLElement { return; } + if (event.source !== globalThis.parent) { + return; + } + if (!this.messageOrigin) { this.messageOrigin = event.origin; } @@ -115,12 +120,23 @@ export class AutofillInlineMenuPageElement extends HTMLElement { const message = event?.data; - if ( - message?.token && - (message?.command === "initAutofillInlineMenuButton" || - message?.command === "initAutofillInlineMenuList") - ) { + if (!message?.command) { + return; + } + + const isInitCommand = + message.command === "initAutofillInlineMenuButton" || + message.command === "initAutofillInlineMenuList"; + + if (isInitCommand) { + if (!message?.token) { + return; + } this.token = message.token; + } else { + if (!this.token || !message?.token || message.token !== this.token) { + return; + } } const handler = this.windowMessageHandlers[message?.command]; diff --git a/apps/browser/src/autofill/overlay/notifications/content/__snapshots__/overlay-notifications-content.service.spec.ts.snap b/apps/browser/src/autofill/overlay/notifications/content/__snapshots__/overlay-notifications-content.service.spec.ts.snap index 39ca68d912c..cfcedc9da7a 100644 --- a/apps/browser/src/autofill/overlay/notifications/content/__snapshots__/overlay-notifications-content.service.spec.ts.snap +++ b/apps/browser/src/autofill/overlay/notifications/content/__snapshots__/overlay-notifications-content.service.spec.ts.snap @@ -7,7 +7,7 @@ exports[`OverlayNotificationsContentService opening the notification bar creates >