From 0f56b3f3dd9d6fec454b855bcc434415a01ab800 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 5 Jan 2026 12:30:09 -0800 Subject: [PATCH 01/17] [deps] Autofill: Update @lit-labs/signals to v0.2.0 (#18201) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- package-lock.json | 10 +++++----- package.json | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index e2556015113..78b9dce23db 100644 --- a/package-lock.json +++ b/package-lock.json @@ -81,7 +81,7 @@ "@electron/notarize": "3.0.1", "@electron/rebuild": "4.0.1", "@eslint/compat": "2.0.0", - "@lit-labs/signals": "0.1.3", + "@lit-labs/signals": "0.2.0", "@ngtools/webpack": "20.3.12", "@nx/devkit": "21.6.10", "@nx/eslint": "21.6.10", @@ -8785,14 +8785,14 @@ "license": "BSD-3-Clause" }, "node_modules/@lit-labs/signals": { - "version": "0.1.3", - "resolved": "https://registry.npmjs.org/@lit-labs/signals/-/signals-0.1.3.tgz", - "integrity": "sha512-P0yWgH5blwVyEwBg+WFspLzeu1i0ypJP1QB0l1Omr9qZLIPsUu0p4Fy2jshOg7oQyha5n163K3GJGeUhQQ682Q==", + "version": "0.2.0", + "resolved": "https://registry.npmjs.org/@lit-labs/signals/-/signals-0.2.0.tgz", + "integrity": "sha512-68plyIbciumbwKaiilhLNyhz4Vg6/+nJwDufG2xxWA9r/fUw58jxLHCAlKs+q1CE5Lmh3cZ3ShyYKnOCebEpVA==", "dev": true, "license": "BSD-3-Clause", "dependencies": { "lit": "^2.0.0 || ^3.0.0", - "signal-polyfill": "^0.2.0" + "signal-polyfill": "^0.2.2" } }, "node_modules/@lit-labs/ssr-dom-shim": { diff --git a/package.json b/package.json index be6658964a0..1cfddb16c42 100644 --- a/package.json +++ b/package.json @@ -48,7 +48,7 @@ "@electron/notarize": "3.0.1", "@electron/rebuild": "4.0.1", "@eslint/compat": "2.0.0", - "@lit-labs/signals": "0.1.3", + "@lit-labs/signals": "0.2.0", "@ngtools/webpack": "20.3.12", "@nx/devkit": "21.6.10", "@nx/eslint": "21.6.10", From 86764d807ab286597f13799072fa0283ffa5ce66 Mon Sep 17 00:00:00 2001 From: Vincent Salucci <26154748+vincentsalucci@users.noreply.github.com> Date: Mon, 5 Jan 2026 16:25:57 -0600 Subject: [PATCH 02/17] [PM-22434] Remove CreateDefaultLocation feature flag references and definition (#18057) * chore: remove ff from vault-popup-list-filters.service, refs PM-22434 * chore: remove ff from confirm.command, refs PM-22434 * chore: remove ff from bulk-confirm-dialog.component, refs PM-22434 * chore: remove ff from member-actions.service and clean up leftover imports, refs PM-22434 * chore: remove ff from policy-edit-dialog.component, refs PM-22434 * chore: remove ff from organization-data-ownership.component, refs PM-22434 * chore: remove ff from vnext-organization-data-ownership.component, refs PM-22434 * chore: remove ff from vault-filter.service, refs PM-22434 * chore: remove ff from vault-filter.service (libs), refs PM-22434 * chore: remove ff from export.component, refs PM-22434 * chore: update observeMyItemsExclusionCriteria method documentation comments, refs PM-22434 * chore: remove ff from item-details-section.component, refs PM-22434 * chore: remove ff definition, refs PM-22434 * fix: remove configService from superclasses, refs PM-22434 * chore: update injection for VaultPopupListFilters service instantiation, refs PM-22434 * chore: update ConfirmCommand instantiation, refs PM-22434 * chore: update import order in member-actions.service, refs PM-22434 * fix: constructor argument update to amend merge conflict, refs PM-22434 * chore: remove unnecessary feature flag related tests for confirm user, refs PM-22434 * fix: remove unused services from member-actions.service.spec, refs PM-22434 --- .../vault-popup-list-filters.service.spec.ts | 1 - .../vault-popup-list-filters.service.ts | 9 +-- .../admin-console/commands/confirm.command.ts | 9 +-- apps/cli/src/oss-serve-configurator.ts | 1 - apps/cli/src/vault.program.ts | 1 - .../vault-filter/vault-filter.service.ts | 3 - .../bulk/bulk-confirm-dialog.component.ts | 20 +------ .../member-actions.service.spec.ts | 55 +------------------ .../member-actions/member-actions.service.ts | 46 ++-------------- ...to-confirm-edit-policy-dialog.component.ts | 3 - .../organization-data-ownership.component.ts | 8 +-- ...t-organization-data-ownership.component.ts | 9 +-- .../policies/policy-edit-dialog.component.ts | 16 +----- .../services/vault-filter.service.ts | 15 +---- .../services/vault-filter.service.ts | 10 +--- libs/common/src/enums/feature-flag.enum.ts | 2 - .../src/components/export.component.ts | 36 +++--------- .../item-details-section.component.ts | 11 ---- 18 files changed, 26 insertions(+), 229 deletions(-) diff --git a/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.spec.ts b/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.spec.ts index 692e21d0084..866c5dd2e89 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.spec.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.spec.ts @@ -822,7 +822,6 @@ function createSeededVaultPopupListFiltersService( accountServiceMock, viewCacheServiceMock, restrictedItemTypesServiceMock, - configService, ); }); diff --git a/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.ts b/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.ts index 08db7d5d4ab..439353cab50 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.ts @@ -29,8 +29,6 @@ import { Organization } from "@bitwarden/common/admin-console/models/domain/orga import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { ProductTierType } from "@bitwarden/common/billing/enums"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { asUuid } from "@bitwarden/common/platform/abstractions/sdk/sdk.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; @@ -191,7 +189,6 @@ export class VaultPopupListFiltersService { private accountService: AccountService, private viewCacheService: ViewCacheService, private restrictedItemTypesService: RestrictedItemTypesService, - private configService: ConfigService, ) { this.filterForm.controls.organization.valueChanges .pipe(takeUntilDestroyed()) @@ -455,19 +452,15 @@ export class VaultPopupListFiltersService { ), this.collectionService.decryptedCollections$(userId), this.organizationService.memberOrganizations$(userId), - this.configService.getFeatureFlag$(FeatureFlag.CreateDefaultLocation), ]), ), - map(([filters, allCollections, orgs, defaultVaultEnabled]) => { + map(([filters, allCollections, orgs]) => { const orgFilterId = filters.organization?.id ?? null; // When the organization filter is selected, filter out collections that do not belong to the selected organization const filtered = orgFilterId ? allCollections.filter((c) => c.organizationId === orgFilterId) : allCollections; - if (!defaultVaultEnabled) { - return filtered; - } return sortDefaultCollections(filtered, orgs, this.i18nService.collator); }), map((fullList) => { diff --git a/apps/cli/src/admin-console/commands/confirm.command.ts b/apps/cli/src/admin-console/commands/confirm.command.ts index adae5091172..4458054e244 100644 --- a/apps/cli/src/admin-console/commands/confirm.command.ts +++ b/apps/cli/src/admin-console/commands/confirm.command.ts @@ -9,9 +9,7 @@ import { import { ApiService } from "@bitwarden/common/abstractions/api.service"; 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 { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { OrganizationId } from "@bitwarden/common/types/guid"; @@ -28,7 +26,6 @@ export class ConfirmCommand { private encryptService: EncryptService, private organizationUserApiService: OrganizationUserApiService, private accountService: AccountService, - private configService: ConfigService, private i18nService: I18nService, ) {} @@ -80,11 +77,7 @@ export class ConfirmCommand { const key = await this.encryptService.encapsulateKeyUnsigned(orgKey, publicKey); const req = new OrganizationUserConfirmRequest(); req.key = key.encryptedString; - if ( - await firstValueFrom(this.configService.getFeatureFlag$(FeatureFlag.CreateDefaultLocation)) - ) { - req.defaultUserCollectionName = await this.getEncryptedDefaultUserCollectionName(orgKey); - } + req.defaultUserCollectionName = await this.getEncryptedDefaultUserCollectionName(orgKey); await this.organizationUserApiService.postOrganizationUserConfirm( options.organizationId, id, diff --git a/apps/cli/src/oss-serve-configurator.ts b/apps/cli/src/oss-serve-configurator.ts index e8f5e6acd9a..b2cca2a644b 100644 --- a/apps/cli/src/oss-serve-configurator.ts +++ b/apps/cli/src/oss-serve-configurator.ts @@ -147,7 +147,6 @@ export class OssServeConfigurator { this.serviceContainer.encryptService, this.serviceContainer.organizationUserApiService, this.serviceContainer.accountService, - this.serviceContainer.configService, this.serviceContainer.i18nService, ); this.restoreCommand = new RestoreCommand( diff --git a/apps/cli/src/vault.program.ts b/apps/cli/src/vault.program.ts index 21f87feab00..3e08038fe64 100644 --- a/apps/cli/src/vault.program.ts +++ b/apps/cli/src/vault.program.ts @@ -494,7 +494,6 @@ export class VaultProgram extends BaseProgram { this.serviceContainer.encryptService, this.serviceContainer.organizationUserApiService, this.serviceContainer.accountService, - this.serviceContainer.configService, this.serviceContainer.i18nService, ); const response = await command.run(object, id, cmd); diff --git a/apps/web/src/app/admin-console/organizations/collections/vault-filter/vault-filter.service.ts b/apps/web/src/app/admin-console/organizations/collections/vault-filter/vault-filter.service.ts index dc05248d7ba..f4b6f41fab6 100644 --- a/apps/web/src/app/admin-console/organizations/collections/vault-filter/vault-filter.service.ts +++ b/apps/web/src/app/admin-console/organizations/collections/vault-filter/vault-filter.service.ts @@ -5,7 +5,6 @@ import { CollectionAdminView, CollectionService } from "@bitwarden/admin-console import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { StateProvider } from "@bitwarden/common/platform/state"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; @@ -35,7 +34,6 @@ export class VaultFilterService extends BaseVaultFilterService implements OnDest stateProvider: StateProvider, collectionService: CollectionService, accountService: AccountService, - configService: ConfigService, ) { super( organizationService, @@ -46,7 +44,6 @@ export class VaultFilterService extends BaseVaultFilterService implements OnDest stateProvider, collectionService, accountService, - configService, ); } diff --git a/apps/web/src/app/admin-console/organizations/members/components/bulk/bulk-confirm-dialog.component.ts b/apps/web/src/app/admin-console/organizations/members/components/bulk/bulk-confirm-dialog.component.ts index 3a624e11d95..c95f94b1e11 100644 --- a/apps/web/src/app/admin-console/organizations/members/components/bulk/bulk-confirm-dialog.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/components/bulk/bulk-confirm-dialog.component.ts @@ -6,7 +6,6 @@ import { firstValueFrom, map, Observable, switchMap } from "rxjs"; import { OrganizationUserApiService, - OrganizationUserBulkConfirmRequest, OrganizationUserBulkPublicKeyResponse, OrganizationUserBulkResponse, OrganizationUserService, @@ -15,10 +14,8 @@ import { OrganizationUserStatusType } from "@bitwarden/common/admin-console/enum import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { ProviderUserBulkPublicKeyResponse } from "@bitwarden/common/admin-console/models/response/provider/provider-user-bulk-public-key.response"; import { ProviderUserBulkResponse } from "@bitwarden/common/admin-console/models/response/provider/provider-user-bulk.response"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { ListResponse } from "@bitwarden/common/models/response/list.response"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { SymmetricCryptoKey } from "@bitwarden/common/platform/models/domain/symmetric-crypto-key"; import { StateProvider } from "@bitwarden/common/platform/state"; @@ -54,7 +51,6 @@ export class BulkConfirmDialogComponent extends BaseBulkConfirmComponent { protected i18nService: I18nService, private stateProvider: StateProvider, private organizationUserService: OrganizationUserService, - private configService: ConfigService, ) { super(keyService, encryptService, i18nService); @@ -84,19 +80,9 @@ export class BulkConfirmDialogComponent extends BaseBulkConfirmComponent { protected postConfirmRequest = async ( userIdsWithKeys: { id: string; key: string }[], ): Promise> => { - if ( - await firstValueFrom(this.configService.getFeatureFlag$(FeatureFlag.CreateDefaultLocation)) - ) { - return await firstValueFrom( - this.organizationUserService.bulkConfirmUsers(this.organization, userIdsWithKeys), - ); - } else { - const request = new OrganizationUserBulkConfirmRequest(userIdsWithKeys); - return await this.organizationUserApiService.postOrganizationUserBulkConfirm( - this.organization.id, - request, - ); - } + return await firstValueFrom( + this.organizationUserService.bulkConfirmUsers(this.organization, userIdsWithKeys), + ); }; static open(dialogService: DialogService, config: DialogConfig) { diff --git a/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.spec.ts b/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.spec.ts index 80a330b0db1..1dd75a79180 100644 --- a/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.spec.ts +++ b/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.spec.ts @@ -12,15 +12,10 @@ import { } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { OrganizationMetadataServiceAbstraction } from "@bitwarden/common/billing/abstractions/organization-metadata.service.abstraction"; -import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; -import { EncString } from "@bitwarden/common/key-management/crypto/models/enc-string"; import { ListResponse } from "@bitwarden/common/models/response/list.response"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; -import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec"; import { OrganizationId, UserId } from "@bitwarden/common/types/guid"; -import { OrgKey } from "@bitwarden/common/types/key"; import { newGuid } from "@bitwarden/guid"; -import { KeyService } from "@bitwarden/key-management"; import { OrganizationUserView } from "../../../core/views/organization-user.view"; @@ -30,13 +25,9 @@ describe("MemberActionsService", () => { let service: MemberActionsService; let organizationUserApiService: MockProxy; let organizationUserService: MockProxy; - let keyService: MockProxy; - let encryptService: MockProxy; let configService: MockProxy; - let accountService: FakeAccountService; let organizationMetadataService: MockProxy; - const userId = newGuid() as UserId; const organizationId = newGuid() as OrganizationId; const userIdToManage = newGuid(); @@ -46,10 +37,7 @@ describe("MemberActionsService", () => { beforeEach(() => { organizationUserApiService = mock(); organizationUserService = mock(); - keyService = mock(); - encryptService = mock(); configService = mock(); - accountService = mockAccountServiceWith(userId); organizationMetadataService = mock(); mockOrganization = { @@ -71,10 +59,7 @@ describe("MemberActionsService", () => { service = new MemberActionsService( organizationUserApiService, organizationUserService, - keyService, - encryptService, configService, - accountService, organizationMetadataService, ); }); @@ -242,8 +227,7 @@ describe("MemberActionsService", () => { describe("confirmUser", () => { const publicKey = new Uint8Array([1, 2, 3, 4, 5]); - it("should confirm user using new flow when feature flag is enabled", async () => { - configService.getFeatureFlag$.mockReturnValue(of(true)); + it("should confirm user", async () => { organizationUserService.confirmUser.mockReturnValue(of(undefined)); const result = await service.confirmUser(mockOrgUser, publicKey, mockOrganization); @@ -257,44 +241,7 @@ describe("MemberActionsService", () => { expect(organizationUserApiService.postOrganizationUserConfirm).not.toHaveBeenCalled(); }); - it("should confirm user using exising flow when feature flag is disabled", async () => { - configService.getFeatureFlag$.mockReturnValue(of(false)); - - const mockOrgKey = mock(); - const mockOrgKeys = { [organizationId]: mockOrgKey }; - keyService.orgKeys$.mockReturnValue(of(mockOrgKeys)); - - const mockEncryptedKey = new EncString("encrypted-key-data"); - encryptService.encapsulateKeyUnsigned.mockResolvedValue(mockEncryptedKey); - - organizationUserApiService.postOrganizationUserConfirm.mockResolvedValue(undefined); - - const result = await service.confirmUser(mockOrgUser, publicKey, mockOrganization); - - expect(result).toEqual({ success: true }); - expect(keyService.orgKeys$).toHaveBeenCalledWith(userId); - expect(encryptService.encapsulateKeyUnsigned).toHaveBeenCalledWith(mockOrgKey, publicKey); - expect(organizationUserApiService.postOrganizationUserConfirm).toHaveBeenCalledWith( - organizationId, - userIdToManage, - expect.objectContaining({ - key: "encrypted-key-data", - }), - ); - }); - - it("should handle missing organization keys", async () => { - configService.getFeatureFlag$.mockReturnValue(of(false)); - keyService.orgKeys$.mockReturnValue(of({})); - - const result = await service.confirmUser(mockOrgUser, publicKey, mockOrganization); - - expect(result.success).toBe(false); - expect(result.error).toContain("Organization keys not found"); - }); - it("should handle confirm errors", async () => { - configService.getFeatureFlag$.mockReturnValue(of(true)); const errorMessage = "Confirm failed"; organizationUserService.confirmUser.mockImplementation(() => { throw new Error(errorMessage); diff --git a/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts b/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts index f3774e3cb25..a44bfa4b19c 100644 --- a/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts +++ b/apps/web/src/app/admin-console/organizations/members/services/member-actions/member-actions.service.ts @@ -1,10 +1,9 @@ import { Injectable } from "@angular/core"; -import { firstValueFrom, switchMap, map } from "rxjs"; +import { firstValueFrom } from "rxjs"; import { OrganizationUserApiService, OrganizationUserBulkResponse, - OrganizationUserConfirmRequest, OrganizationUserService, } from "@bitwarden/admin-console/common"; import { @@ -12,14 +11,10 @@ import { OrganizationUserStatusType, } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; -import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; -import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { OrganizationMetadataServiceAbstraction } from "@bitwarden/common/billing/abstractions/organization-metadata.service.abstraction"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; -import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { ListResponse } from "@bitwarden/common/models/response/list.response"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; -import { KeyService } from "@bitwarden/key-management"; import { UserId } from "@bitwarden/user-core"; import { OrganizationUserView } from "../../../core/views/organization-user.view"; @@ -38,15 +33,10 @@ export interface BulkActionResult { @Injectable() export class MemberActionsService { - private userId$ = this.accountService.activeAccount$.pipe(getUserId); - constructor( private organizationUserApiService: OrganizationUserApiService, private organizationUserService: OrganizationUserService, - private keyService: KeyService, - private encryptService: EncryptService, private configService: ConfigService, - private accountService: AccountService, private organizationMetadataService: OrganizationMetadataServiceAbstraction, ) {} @@ -128,37 +118,9 @@ export class MemberActionsService { organization: Organization, ): Promise { try { - if ( - await firstValueFrom(this.configService.getFeatureFlag$(FeatureFlag.CreateDefaultLocation)) - ) { - await firstValueFrom( - this.organizationUserService.confirmUser(organization, user.id, publicKey), - ); - } else { - const request = await firstValueFrom( - this.userId$.pipe( - switchMap((userId) => this.keyService.orgKeys$(userId)), - map((orgKeys) => { - if (orgKeys == null || orgKeys[organization.id] == null) { - throw new Error("Organization keys not found for provided User."); - } - return orgKeys[organization.id]; - }), - switchMap((orgKey) => this.encryptService.encapsulateKeyUnsigned(orgKey, publicKey)), - map((encKey) => { - const req = new OrganizationUserConfirmRequest(); - req.key = encKey.encryptedString; - return req; - }), - ), - ); - - await this.organizationUserApiService.postOrganizationUserConfirm( - organization.id, - user.id, - request, - ); - } + await firstValueFrom( + this.organizationUserService.confirmUser(organization, user.id, publicKey), + ); return { success: true }; } catch (error) { return { success: false, error: (error as Error).message ?? String(error) }; diff --git a/apps/web/src/app/admin-console/organizations/policies/auto-confirm-edit-policy-dialog.component.ts b/apps/web/src/app/admin-console/organizations/policies/auto-confirm-edit-policy-dialog.component.ts index 99d484f04f2..63a8a4341d6 100644 --- a/apps/web/src/app/admin-console/organizations/policies/auto-confirm-edit-policy-dialog.component.ts +++ b/apps/web/src/app/admin-console/organizations/policies/auto-confirm-edit-policy-dialog.component.ts @@ -30,7 +30,6 @@ import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { PolicyRequest } from "@bitwarden/common/admin-console/models/request/policy.request"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { getById } from "@bitwarden/common/platform/misc"; import { @@ -115,7 +114,6 @@ export class AutoConfirmPolicyDialogComponent formBuilder: FormBuilder, dialogRef: DialogRef, toastService: ToastService, - configService: ConfigService, keyService: KeyService, private organizationService: OrganizationService, private policyService: PolicyService, @@ -131,7 +129,6 @@ export class AutoConfirmPolicyDialogComponent formBuilder, dialogRef, toastService, - configService, keyService, ); diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/organization-data-ownership.component.ts b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/organization-data-ownership.component.ts index ec857478a21..ceecf8f2ecc 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/organization-data-ownership.component.ts +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/organization-data-ownership.component.ts @@ -1,9 +1,8 @@ import { ChangeDetectionStrategy, Component } from "@angular/core"; -import { map, Observable } from "rxjs"; +import { of, Observable } from "rxjs"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { SharedModule } from "../../../../shared"; @@ -16,9 +15,8 @@ export class OrganizationDataOwnershipPolicy extends BasePolicyEditDefinition { component = OrganizationDataOwnershipPolicyComponent; display$(organization: Organization, configService: ConfigService): Observable { - return configService - .getFeatureFlag$(FeatureFlag.CreateDefaultLocation) - .pipe(map((enabled) => !enabled)); + // TODO Remove this entire component upon verifying that it can be deleted due to its sole reliance of the CreateDefaultLocation feature flag + return of(false); } } diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/vnext-organization-data-ownership.component.ts b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/vnext-organization-data-ownership.component.ts index ed26dd37801..59670457d88 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/vnext-organization-data-ownership.component.ts +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit-definitions/vnext-organization-data-ownership.component.ts @@ -1,12 +1,9 @@ import { ChangeDetectionStrategy, Component, OnInit, TemplateRef, ViewChild } from "@angular/core"; -import { lastValueFrom, Observable } from "rxjs"; +import { lastValueFrom } from "rxjs"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; -import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { PolicyRequest } from "@bitwarden/common/admin-console/models/request/policy.request"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { OrgKey } from "@bitwarden/common/types/key"; import { CenterPositionStrategy, DialogService } from "@bitwarden/components"; @@ -28,10 +25,6 @@ export class vNextOrganizationDataOwnershipPolicy extends BasePolicyEditDefiniti type = PolicyType.OrganizationDataOwnership; component = vNextOrganizationDataOwnershipPolicyComponent; showDescription = false; - - override display$(organization: Organization, configService: ConfigService): Observable { - return configService.getFeatureFlag$(FeatureFlag.CreateDefaultLocation); - } } @Component({ diff --git a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialog.component.ts b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialog.component.ts index 98b6d1c6bee..c633ff5f421 100644 --- a/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialog.component.ts +++ b/apps/web/src/app/admin-console/organizations/policies/policy-edit-dialog.component.ts @@ -14,8 +14,6 @@ import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { PolicyResponse } from "@bitwarden/common/admin-console/models/response/policy.response"; 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"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { OrganizationId } from "@bitwarden/common/types/guid"; import { @@ -75,7 +73,6 @@ export class PolicyEditDialogComponent implements AfterViewInit { private formBuilder: FormBuilder, protected dialogRef: DialogRef, protected toastService: ToastService, - private configService: ConfigService, private keyService: KeyService, ) {} @@ -132,10 +129,7 @@ export class PolicyEditDialogComponent implements AfterViewInit { } try { - if ( - this.policyComponent instanceof vNextOrganizationDataOwnershipPolicyComponent && - (await this.isVNextEnabled()) - ) { + if (this.policyComponent instanceof vNextOrganizationDataOwnershipPolicyComponent) { await this.handleVNextSubmission(this.policyComponent); } else { await this.handleStandardSubmission(); @@ -154,14 +148,6 @@ export class PolicyEditDialogComponent implements AfterViewInit { } }; - private async isVNextEnabled(): Promise { - const isVNextFeatureEnabled = await firstValueFrom( - this.configService.getFeatureFlag$(FeatureFlag.CreateDefaultLocation), - ); - - return isVNextFeatureEnabled; - } - private async handleStandardSubmission(): Promise { if (!this.policyComponent) { throw new Error("PolicyComponent not initialized."); diff --git a/apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.ts b/apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.ts index 1f27773c467..aad42506777 100644 --- a/apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.ts +++ b/apps/web/src/app/vault/individual-vault/vault-filter/services/vault-filter.service.ts @@ -24,8 +24,6 @@ import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; 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"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { SingleUserState, StateProvider } from "@bitwarden/common/platform/state"; import { OrganizationId, UserId } from "@bitwarden/common/types/guid"; @@ -111,11 +109,8 @@ export class VaultFilterService implements VaultFilterServiceAbstraction { collectionTree$: Observable> = combineLatest([ this.filteredCollections$, this.memberOrganizations$, - this.configService.getFeatureFlag$(FeatureFlag.CreateDefaultLocation), ]).pipe( - map(([collections, organizations, defaultCollectionsFlagEnabled]) => - this.buildCollectionTree(collections, organizations, defaultCollectionsFlagEnabled), - ), + map(([collections, organizations]) => this.buildCollectionTree(collections, organizations)), ); cipherTypeTree$: Observable> = this.buildCipherTypeTree(); @@ -133,7 +128,6 @@ export class VaultFilterService implements VaultFilterServiceAbstraction { protected stateProvider: StateProvider, protected collectionService: CollectionService, protected accountService: AccountService, - protected configService: ConfigService, ) {} async getCollectionNodeFromTree(id: string) { @@ -241,18 +235,13 @@ export class VaultFilterService implements VaultFilterServiceAbstraction { protected buildCollectionTree( collections?: CollectionView[], orgs?: Organization[], - defaultCollectionsFlagEnabled?: boolean, ): TreeNode { const headNode = this.getCollectionFilterHead(); if (!collections) { return headNode; } const all: TreeNode[] = []; - - if (defaultCollectionsFlagEnabled) { - collections = sortDefaultCollections(collections, orgs, this.i18nService.collator); - } - + collections = sortDefaultCollections(collections, orgs, this.i18nService.collator); const groupedByOrg = this.collectionService.groupByOrganization(collections); for (const group of groupedByOrg.values()) { diff --git a/libs/angular/src/vault/vault-filter/services/vault-filter.service.ts b/libs/angular/src/vault/vault-filter/services/vault-filter.service.ts index 9bc10e5ffc5..6777da7a9e5 100644 --- a/libs/angular/src/vault/vault-filter/services/vault-filter.service.ts +++ b/libs/angular/src/vault/vault-filter/services/vault-filter.service.ts @@ -14,8 +14,6 @@ import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; 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"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { SingleUserState, StateProvider } from "@bitwarden/common/platform/state"; import { UserId } from "@bitwarden/common/types/guid"; @@ -45,7 +43,6 @@ export class VaultFilterService implements DeprecatedVaultFilterServiceAbstracti protected policyService: PolicyService, protected stateProvider: StateProvider, protected accountService: AccountService, - protected configService: ConfigService, protected i18nService: I18nService, ) {} @@ -116,18 +113,13 @@ export class VaultFilterService implements DeprecatedVaultFilterServiceAbstracti ), ); const orgs = await this.buildOrganizations(); - const defaulCollectionsFlagEnabled = await this.configService.getFeatureFlag( - FeatureFlag.CreateDefaultLocation, - ); let collections = organizationId == null ? storedCollections : storedCollections.filter((c) => c.organizationId === organizationId); - if (defaulCollectionsFlagEnabled) { - collections = sortDefaultCollections(collections, orgs, this.i18nService.collator); - } + collections = sortDefaultCollections(collections, orgs, this.i18nService.collator); const nestedCollections = await this.collectionService.getAllNested(collections); return new DynamicTreeNode({ diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index e5c29636585..850c3be4d5e 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -11,7 +11,6 @@ import { ServerConfig } from "../platform/abstractions/config/server-config"; // eslint-disable-next-line @bitwarden/platform/no-enums export enum FeatureFlag { /* Admin Console Team */ - CreateDefaultLocation = "pm-19467-create-default-location", AutoConfirm = "pm-19934-auto-confirm-organization-users", BlockClaimedDomainAccountCreation = "pm-28297-block-uninvited-claimed-domain-registration", IncreaseBulkReinviteLimitForCloud = "pm-28251-increase-bulk-reinvite-limit-for-cloud", @@ -97,7 +96,6 @@ const FALSE = false as boolean; */ export const DefaultFeatureFlagValue = { /* Admin Console Team */ - [FeatureFlag.CreateDefaultLocation]: FALSE, [FeatureFlag.AutoConfirm]: FALSE, [FeatureFlag.BlockClaimedDomainAccountCreation]: FALSE, [FeatureFlag.IncreaseBulkReinviteLimitForCloud]: FALSE, diff --git a/libs/tools/export/vault-export/vault-export-ui/src/components/export.component.ts b/libs/tools/export/vault-export/vault-export-ui/src/components/export.component.ts index 232fb40aeb2..058b7db1299 100644 --- a/libs/tools/export/vault-export/vault-export-ui/src/components/export.component.ts +++ b/libs/tools/export/vault-export/vault-export-ui/src/components/export.component.ts @@ -18,7 +18,6 @@ import { BehaviorSubject, combineLatest, firstValueFrom, - from, map, merge, Observable, @@ -43,8 +42,6 @@ import { Organization } from "@bitwarden/common/admin-console/models/domain/orga import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { ClientType, EventType } from "@bitwarden/common/enums"; -import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { FileDownloadService } from "@bitwarden/common/platform/abstractions/file-download/file-download.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; @@ -100,7 +97,6 @@ import { ExportScopeCalloutComponent } from "./export-scope-callout.component"; }) export class ExportComponent implements OnInit, OnDestroy, AfterViewInit { private _organizationId$ = new BehaviorSubject(undefined); - private createDefaultLocationFlagEnabled$: Observable; private _showExcludeMyItems = false; /** @@ -259,13 +255,11 @@ export class ExportComponent implements OnInit, OnDestroy, AfterViewInit { protected organizationService: OrganizationService, private accountService: AccountService, private collectionService: CollectionService, - private configService: ConfigService, private platformUtilsService: PlatformUtilsService, @Optional() private router?: Router, ) {} async ngOnInit() { - this.observeFeatureFlags(); this.observeFormState(); this.observePolicyStatus(); this.observeFormSelections(); @@ -286,12 +280,6 @@ export class ExportComponent implements OnInit, OnDestroy, AfterViewInit { this.setupPolicyBasedFormState(); } - private observeFeatureFlags(): void { - this.createDefaultLocationFlagEnabled$ = from( - this.configService.getFeatureFlag(FeatureFlag.CreateDefaultLocation), - ).pipe(shareReplay({ bufferSize: 1, refCount: true })); - } - private observeFormState(): void { this.exportForm.statusChanges.pipe(takeUntil(this.destroy$)).subscribe((c) => { this.formDisabled.emit(c === "DISABLED"); @@ -380,32 +368,24 @@ export class ExportComponent implements OnInit, OnDestroy, AfterViewInit { /** * Determine value of showExcludeMyItems. Returns true when: - * CreateDefaultLocation feature flag is on - * AND organizationDataOwnershipPolicy is enabled for the selected organization + * organizationDataOwnershipPolicy is enabled for the selected organization * AND a valid OrganizationId is present (not exporting from individual vault) */ private observeMyItemsExclusionCriteria(): void { combineLatest({ - createDefaultLocationFlagEnabled: this.createDefaultLocationFlagEnabled$, organizationDataOwnershipPolicyEnabledForOrg: this.organizationDataOwnershipPolicyEnabledForOrg$, organizationId: this._organizationId$, }) .pipe(takeUntil(this.destroy$)) - .subscribe( - ({ - createDefaultLocationFlagEnabled, - organizationDataOwnershipPolicyEnabledForOrg, - organizationId, - }) => { - if (!createDefaultLocationFlagEnabled || !organizationId) { - this._showExcludeMyItems = false; - return; - } + .subscribe(({ organizationDataOwnershipPolicyEnabledForOrg, organizationId }) => { + if (!organizationId) { + this._showExcludeMyItems = false; + return; + } - this._showExcludeMyItems = organizationDataOwnershipPolicyEnabledForOrg; - }, - ); + this._showExcludeMyItems = organizationDataOwnershipPolicyEnabledForOrg; + }); } // Setup validator adjustments based on format and encryption type changes diff --git a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts index 58959a957a8..ad61c0f216f 100644 --- a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts +++ b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.ts @@ -15,8 +15,6 @@ import { OrganizationUserType, PolicyType } from "@bitwarden/common/admin-consol import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; 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"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { CollectionId, OrganizationId, UserId } from "@bitwarden/common/types/guid"; @@ -141,7 +139,6 @@ export class ItemDetailsSectionComponent implements OnInit { private i18nService: I18nService, private destroyRef: DestroyRef, private accountService: AccountService, - private configService: ConfigService, private policyService: PolicyService, ) { this.cipherFormContainer.registerChildForm("itemDetails", this.itemDetailsForm); @@ -285,14 +282,6 @@ export class ItemDetailsSectionComponent implements OnInit { return; } - const isFeatureEnabled = await this.configService.getFeatureFlag( - FeatureFlag.CreateDefaultLocation, - ); - - if (!isFeatureEnabled) { - return; - } - const selectedOrgHasPolicyEnabled = ( await firstValueFrom( this.policyService.policiesByType$(PolicyType.OrganizationDataOwnership, this.userId), From 1cb5d5ce7aff87d440e7fdef272964c492c5b227 Mon Sep 17 00:00:00 2001 From: Jordan Aasen <166539328+jaasen-livefront@users.noreply.github.com> Date: Mon, 5 Jan 2026 15:18:00 -0800 Subject: [PATCH 03/17] [PM-30249] - allow org ciphers to be archived (#18214) * allow org ciphers to be archived * fix title in item footer unarchive --- apps/desktop/src/vault/app/vault/item-footer.component.html | 2 +- apps/desktop/src/vault/app/vault/item-footer.component.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/desktop/src/vault/app/vault/item-footer.component.html b/apps/desktop/src/vault/app/vault/item-footer.component.html index 859b2f1bdc5..9d5b1e5e560 100644 --- a/apps/desktop/src/vault/app/vault/item-footer.component.html +++ b/apps/desktop/src/vault/app/vault/item-footer.component.html @@ -59,7 +59,7 @@ type="button" *ngIf="showUnarchiveButton" (click)="unarchive()" - appA11yTitle="{{ 'unarchive' | i18n }}" + appA11yTitle="{{ 'unArchive' | i18n }}" > diff --git a/apps/desktop/src/vault/app/vault/item-footer.component.ts b/apps/desktop/src/vault/app/vault/item-footer.component.ts index 0ac12c928f2..8873b74932c 100644 --- a/apps/desktop/src/vault/app/vault/item-footer.component.ts +++ b/apps/desktop/src/vault/app/vault/item-footer.component.ts @@ -218,7 +218,7 @@ export class ItemFooterComponent implements OnInit, OnChanges { } private async checkArchiveState() { - const cipherCanBeArchived = !this.cipher.isDeleted && this.cipher.organizationId == null; + const cipherCanBeArchived = !this.cipher.isDeleted; const [userCanArchive, hasArchiveFlagEnabled] = await firstValueFrom( this.accountService.activeAccount$.pipe( getUserId, From e25dd785a617bcc86fa8ec667be4625ff372129d Mon Sep 17 00:00:00 2001 From: Jordan Aasen <166539328+jaasen-livefront@users.noreply.github.com> Date: Mon, 5 Jan 2026 15:18:56 -0800 Subject: [PATCH 04/17] open help links in new tab (#18109) --- .../components/setup-extension/setup-extension.component.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/web/src/app/vault/components/setup-extension/setup-extension.component.html b/apps/web/src/app/vault/components/setup-extension/setup-extension.component.html index 1976321b4ee..8cfd394b854 100644 --- a/apps/web/src/app/vault/components/setup-extension/setup-extension.component.html +++ b/apps/web/src/app/vault/components/setup-extension/setup-extension.component.html @@ -64,11 +64,11 @@

{{ "gettingStartedWithBitwardenPart1" | i18n }} - + {{ "gettingStartedWithBitwardenPart2" | i18n }} {{ "and" | i18n }} - + {{ "gettingStartedWithBitwardenPart3" | i18n }}

From c022878d816b62fcf16e7c4f8a49e39e361f1e69 Mon Sep 17 00:00:00 2001 From: Bernd Schoolmann Date: Tue, 6 Jan 2026 07:44:24 +0100 Subject: [PATCH 05/17] Fix tsconfig.base.json ts-go compat (#18176) * Fix tsconfig.base.json * Undo baseurl change --- tsconfig.base.json | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/tsconfig.base.json b/tsconfig.base.json index 2f6499eb374..d91e8cb9890 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -20,48 +20,48 @@ "paths": { "@bitwarden/admin-console/common": ["./libs/admin-console/src/common"], "@bitwarden/angular/*": ["./libs/angular/src/*"], - "@bitwarden/assets": ["libs/assets/src/index.ts"], - "@bitwarden/assets/svg": ["libs/assets/src/svg/index.ts"], + "@bitwarden/assets": ["./libs/assets/src/index.ts"], + "@bitwarden/assets/svg": ["./libs/assets/src/svg/index.ts"], "@bitwarden/auth/angular": ["./libs/auth/src/angular"], "@bitwarden/auth/common": ["./libs/auth/src/common"], "@bitwarden/billing": ["./libs/billing/src"], "@bitwarden/bit-common/*": ["./bitwarden_license/bit-common/src/*"], "@bitwarden/browser/*": ["./apps/browser/src/*"], "@bitwarden/cli/*": ["./apps/cli/src/*"], - "@bitwarden/client-type": ["libs/client-type/src/index.ts"], + "@bitwarden/client-type": ["./libs/client-type/src/index.ts"], "@bitwarden/common/spec": ["./libs/common/spec"], "@bitwarden/common/*": ["./libs/common/src/*"], "@bitwarden/components": ["./libs/components/src"], - "@bitwarden/core-test-utils": ["libs/core-test-utils/src/index.ts"], + "@bitwarden/core-test-utils": ["./libs/core-test-utils/src/index.ts"], "@bitwarden/dirt-card": ["./libs/dirt/card/src"], "@bitwarden/generator-components": ["./libs/tools/generator/components/src"], "@bitwarden/generator-core": ["./libs/tools/generator/core/src"], "@bitwarden/generator-history": ["./libs/tools/generator/extensions/history/src"], "@bitwarden/generator-legacy": ["./libs/tools/generator/extensions/legacy/src"], "@bitwarden/generator-navigation": ["./libs/tools/generator/extensions/navigation/src"], - "@bitwarden/guid": ["libs/guid/src/index.ts"], + "@bitwarden/guid": ["./libs/guid/src/index.ts"], "@bitwarden/importer-core": ["./libs/importer/src"], "@bitwarden/importer-ui": ["./libs/importer/src/components"], "@bitwarden/key-management": ["./libs/key-management/src"], "@bitwarden/key-management-ui": ["./libs/key-management-ui/src"], - "@bitwarden/logging": ["libs/logging/src"], - "@bitwarden/messaging": ["libs/messaging/src/index.ts"], + "@bitwarden/logging": ["./libs/logging/src"], + "@bitwarden/messaging": ["./libs/messaging/src/index.ts"], "@bitwarden/node/*": ["./libs/node/src/*"], - "@bitwarden/nx-plugin": ["libs/nx-plugin/src/index.ts"], + "@bitwarden/nx-plugin": ["./libs/nx-plugin/src/index.ts"], "@bitwarden/platform": ["./libs/platform/src"], "@bitwarden/platform/*": ["./libs/platform/src/*"], - "@bitwarden/pricing": ["libs/pricing/src/index.ts"], + "@bitwarden/pricing": ["./libs/pricing/src/index.ts"], "@bitwarden/send-ui": ["./libs/tools/send/send-ui/src"], - "@bitwarden/serialization": ["libs/serialization/src/index.ts"], - "@bitwarden/state": ["libs/state/src/index.ts"], - "@bitwarden/state-internal": ["libs/state-internal/src/index.ts"], - "@bitwarden/state-test-utils": ["libs/state-test-utils/src/index.ts"], - "@bitwarden/storage-core": ["libs/storage-core/src/index.ts"], - "@bitwarden/storage-test-utils": ["libs/storage-test-utils/src/index.ts"], - "@bitwarden/subscription": ["libs/subscription/src/index.ts"], + "@bitwarden/serialization": ["./libs/serialization/src/index.ts"], + "@bitwarden/state": ["./libs/state/src/index.ts"], + "@bitwarden/state-internal": ["./libs/state-internal/src/index.ts"], + "@bitwarden/state-test-utils": ["./libs/state-test-utils/src/index.ts"], + "@bitwarden/storage-core": ["./libs/storage-core/src/index.ts"], + "@bitwarden/storage-test-utils": ["./libs/storage-test-utils/src/index.ts"], + "@bitwarden/subscription": ["./libs/subscription/src/index.ts"], "@bitwarden/ui-common": ["./libs/ui/common/src"], "@bitwarden/ui-common/setup-jest": ["./libs/ui/common/src/setup-jest"], - "@bitwarden/user-core": ["libs/user-core/src/index.ts"], + "@bitwarden/user-core": ["./libs/user-core/src/index.ts"], "@bitwarden/vault": ["./libs/vault/src"], "@bitwarden/vault-export-core": ["./libs/tools/export/vault-export/vault-export-core/src"], "@bitwarden/vault-export-ui": ["./libs/tools/export/vault-export/vault-export-ui/src"], From eeb4ac46a4f546ae749dcf5c6578187bc5504154 Mon Sep 17 00:00:00 2001 From: neuronull <9162534+neuronull@users.noreply.github.com> Date: Tue, 6 Jan 2026 08:10:42 -0700 Subject: [PATCH 06/17] Add CI validations install and run Desktop client (Linux) (#18095) * Add CI validations install and run Desktop client (Linux) * add support for arm for snap and flatpak * fix flatpak pkg doesn't match installed name * name change * remove unused dep * add other validations to check-failures * add wayland validation * wayland deps * simulate X server * remove unused id --- .github/workflows/build-desktop.yml | 241 ++++++++++++++++++++++++++++ 1 file changed, 241 insertions(+) diff --git a/.github/workflows/build-desktop.yml b/.github/workflows/build-desktop.yml index 8e43127770c..74a397b406f 100644 --- a/.github/workflows/build-desktop.yml +++ b/.github/workflows/build-desktop.yml @@ -1884,6 +1884,242 @@ jobs: upload_sources: true upload_translations: false + validate-linux-x64-deb: + name: Validate Linux x64 .deb + runs-on: ubuntu-22.04 + needs: + - setup + - linux + env: + _PACKAGE_VERSION: ${{ needs.setup.outputs.package_version }} + steps: + - name: Check out repo + uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 + with: + fetch-depth: 1 + ref: ${{ github.event.pull_request.head.sha }} + persist-credentials: false + + - name: Download deb artifact + uses: bitwarden/gh-actions/download-artifacts@main + with: + path: apps/desktop/artifacts/linux/deb + artifacts: Bitwarden-${{ env._PACKAGE_VERSION }}-amd64.deb + + - name: Install deps + run: | + sudo apt-get update + sudo apt-get install -y libasound2 xvfb + + - name: Install .deb + working-directory: apps/desktop/artifacts/linux/deb + run: sudo apt-get install -y ./Bitwarden-${_PACKAGE_VERSION}-amd64.deb + + - name: Run .deb + run: | + xvfb-run -a bitwarden & + sleep 30 + if pgrep bitwarden > /dev/null; then + pkill -9 bitwarden + echo "Bitwarden is running." + else + echo "Bitwarden is not running." + exit 1 + fi + + validate-linux-x64-appimage: + name: Validate Linux x64 appimage + runs-on: ubuntu-22.04 + needs: + - setup + - linux + env: + _PACKAGE_VERSION: ${{ needs.setup.outputs.package_version }} + steps: + - name: Check out repo + uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 + with: + fetch-depth: 1 + ref: ${{ github.event.workflow_run.head_sha }} + persist-credentials: false + + - name: Download appimage artifact + uses: bitwarden/gh-actions/download-artifacts@main + with: + path: apps/desktop/artifacts/linux/appimage + artifacts: Bitwarden-${{ env._PACKAGE_VERSION }}-x86_64.AppImage + + - name: Install deps + run: | + sudo apt-get update + sudo apt-get install -y libasound2 libfuse2 xvfb + + - name: Run AppImage + working-directory: apps/desktop/artifacts/linux/appimage + run: | + chmod a+x ./Bitwarden-${_PACKAGE_VERSION}-x86_64.AppImage + xvfb-run -a ./Bitwarden-${_PACKAGE_VERSION}-x86_64.AppImage --no-sandbox & + sleep 30 + if pgrep bitwarden > /dev/null; then + pkill -9 bitwarden + echo "Bitwarden is running." + else + echo "Bitwarden is not running." + exit 1 + fi + + validate-linux-wayland: + name: Validate Linux Wayland + runs-on: ubuntu-22.04 + needs: + - setup + - linux + env: + _PACKAGE_VERSION: ${{ needs.setup.outputs.package_version }} + steps: + - name: Check out repo + uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 + with: + fetch-depth: 1 + ref: ${{ github.event.workflow_run.head_sha }} + persist-credentials: false + + - name: Download appimage artifact + uses: bitwarden/gh-actions/download-artifacts@main + with: + path: apps/desktop/artifacts/linux/appimage + artifacts: Bitwarden-${{ env._PACKAGE_VERSION }}-x86_64.AppImage + + - name: Install deps + run: | + sudo apt-get update + sudo apt-get install -y libasound2 libfuse2 xvfb + sudo apt-get install -y weston libwayland-client0 libwayland-server0 libwayland-dev + + - name: Run headless Wayland compositor + run: | + # Start Weston in a virtual terminal in headless mode + weston --headless --socket=wayland-0 & + # Let the compositor start + sleep 5 + + - name: Run AppImage + working-directory: apps/desktop/artifacts/linux/appimage + env: + WAYLAND_DISPLAY: wayland-0 + run: | + chmod a+x ./Bitwarden-${_PACKAGE_VERSION}-x86_64.AppImage + xvfb-run -a ./Bitwarden-${_PACKAGE_VERSION}-x86_64.AppImage --no-sandbox & + sleep 30 + if pgrep bitwarden > /dev/null; then + pkill -9 bitwarden + echo "Bitwarden is running." + else + echo "Bitwarden is not running." + exit 1 + fi + + validate-linux-flatpak: + name: Validate Linux ${{ matrix.os }} Flatpak + runs-on: ${{ matrix.os || 'ubuntu-22.04' }} + strategy: + matrix: + os: + - ubuntu-22.04 + - ubuntu-22.04-arm + needs: + - setup + - linux + - linux-arm64 + steps: + - name: Check out repo + uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 + with: + fetch-depth: 1 + ref: ${{ github.event.workflow_run.head_sha }} + persist-credentials: false + + - name: Download flatpak artifact + uses: bitwarden/gh-actions/download-artifacts@main + with: + path: apps/desktop/artifacts/linux/flatpak/ + artifacts: com.bitwarden.${{ matrix.os == 'ubuntu-22.04' && 'desktop' || 'desktop-arm64' }}.flatpak + + - name: Install deps + run: | + sudo apt-get update + sudo apt-get install -y libasound2 flatpak xvfb dbus-x11 + flatpak remote-add --if-not-exists --user flathub https://flathub.org/repo/flathub.flatpakrepo + flatpak install -y --user flathub + + - name: Install flatpak + working-directory: apps/desktop/artifacts/linux/flatpak + run: flatpak install -y --user --bundle com.bitwarden.desktop.flatpak + + - name: Run Flatpak + run: | + export $(dbus-launch) + xvfb-run -a flatpak run com.bitwarden.desktop & + sleep 30 + if pgrep bitwarden > /dev/null; then + pkill -9 bitwarden + echo "Bitwarden is running." + else + echo "Bitwarden is not running." + exit 1 + fi + + validate-linux-snap: + name: Validate Linux ${{ matrix.os }} Snap + runs-on: ${{ matrix.os || 'ubuntu-22.04' }} + strategy: + matrix: + os: + - ubuntu-22.04 + - ubuntu-22.04-arm + needs: + - setup + - linux + - linux-arm64 + env: + _PACKAGE_VERSION: ${{ needs.setup.outputs.package_version }} + _CPU_ARCH: ${{ matrix.os == 'ubuntu-22.04' && 'amd64' || 'arm64' }} + steps: + - name: Check out repo + uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 + with: + fetch-depth: 1 + ref: ${{ github.event.workflow_run.head_sha }} + persist-credentials: false + + - name: Download snap artifact + uses: bitwarden/gh-actions/download-artifacts@main + with: + path: apps/desktop/artifacts/linux/snap + artifacts: bitwarden_${{ env._PACKAGE_VERSION }}_${{ env._CPU_ARCH }}.snap + + - name: Install deps + run: | + sudo apt-get update + sudo apt-get install -y libasound2 snapd xvfb + + - name: Install snap + working-directory: apps/desktop/artifacts/linux/snap + run: | + sudo snap install --dangerous ./bitwarden_${_PACKAGE_VERSION}_${_CPU_ARCH}.snap + + - name: Run Snap + run: | + xvfb-run -a snap run bitwarden & + sleep 30 + if pgrep bitwarden > /dev/null; then + pkill -9 bitwarden + echo "Bitwarden is running." + else + echo "Bitwarden is not running." + exit 1 + fi + check-failures: name: Check for failures if: always() @@ -1898,6 +2134,11 @@ jobs: - macos-package-github - macos-package-mas - crowdin-push + - validate-linux-x64-deb + - validate-linux-x64-appimage + - validate-linux-flatpak + - validate-linux-snap + - validate-linux-wayland permissions: contents: read id-token: write From 78f7a311279f98c6076fb3c82ae217063c428db1 Mon Sep 17 00:00:00 2001 From: Dubzer Date: Tue, 6 Jan 2026 18:20:31 +0300 Subject: [PATCH 07/17] [PM-28648] Handle delayed availability of biometric unlock (#17603) * Handle delayed availability of biometric unlock * Fix linting --------- Co-authored-by: Bernd Schoolmann --- .../src/lock/components/lock.component.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/libs/key-management-ui/src/lock/components/lock.component.ts b/libs/key-management-ui/src/lock/components/lock.component.ts index ec7ef822335..4b43f0ec784 100644 --- a/libs/key-management-ui/src/lock/components/lock.component.ts +++ b/libs/key-management-ui/src/lock/components/lock.component.ts @@ -217,12 +217,20 @@ export class LockComponent implements OnInit, OnDestroy { .pipe( mergeMap(async () => { if (this.activeAccount?.id != null) { + const prevBiometricsEnabled = this.unlockOptions?.biometrics.enabled; + this.unlockOptions = await firstValueFrom( this.lockComponentService.getAvailableUnlockOptions$(this.activeAccount.id), ); + if (this.activeUnlockOption == null) { this.loading = false; await this.setDefaultActiveUnlockOption(this.unlockOptions); + } else if (!prevBiometricsEnabled && this.unlockOptions?.biometrics.enabled) { + await this.setDefaultActiveUnlockOption(this.unlockOptions); + if (this.activeUnlockOption === UnlockOption.Biometrics) { + await this.handleBiometricsUnlockEnabled(); + } } } }), From e344d342be9c2c290c2ef4c2b7f10ace757506c1 Mon Sep 17 00:00:00 2001 From: Jason Ng Date: Tue, 6 Jan 2026 10:29:15 -0500 Subject: [PATCH 08/17] [PM-30135] deleted archived items restored to archive (#18212) --- .../vault-filter/shared/models/filter-function.ts | 6 +++++- libs/common/src/vault/services/cipher.service.spec.ts | 2 +- libs/common/src/vault/services/cipher.service.ts | 1 - 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/apps/web/src/app/vault/individual-vault/vault-filter/shared/models/filter-function.ts b/apps/web/src/app/vault/individual-vault/vault-filter/shared/models/filter-function.ts index c15dd51a969..f010c529110 100644 --- a/apps/web/src/app/vault/individual-vault/vault-filter/shared/models/filter-function.ts +++ b/apps/web/src/app/vault/individual-vault/vault-filter/shared/models/filter-function.ts @@ -47,7 +47,11 @@ export function createFilterFunction( if (filter.type === "archive" && !CipherViewLikeUtils.isArchived(cipher)) { return false; } - if (filter.type !== "archive" && CipherViewLikeUtils.isArchived(cipher)) { + if ( + filter.type !== "archive" && + filter.type !== "trash" && + CipherViewLikeUtils.isArchived(cipher) + ) { return false; } } diff --git a/libs/common/src/vault/services/cipher.service.spec.ts b/libs/common/src/vault/services/cipher.service.spec.ts index 50823807fcf..153bb01403c 100644 --- a/libs/common/src/vault/services/cipher.service.spec.ts +++ b/libs/common/src/vault/services/cipher.service.spec.ts @@ -868,7 +868,7 @@ describe("Cipher Service", () => { const result = await firstValueFrom( stateProvider.singleUser.getFake(mockUserId, ENCRYPTED_CIPHERS).state$, ); - expect(result[cipherId].archivedDate).toBeNull(); + expect(result[cipherId].archivedDate).toEqual("2024-01-01T12:00:00.000Z"); expect(result[cipherId].deletedDate).toBeDefined(); }); }); diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts index 3c44b854de7..d25aa62ea3a 100644 --- a/libs/common/src/vault/services/cipher.service.ts +++ b/libs/common/src/vault/services/cipher.service.ts @@ -1461,7 +1461,6 @@ export class CipherService implements CipherServiceAbstraction { return; } ciphers[cipherId].deletedDate = new Date().toISOString(); - ciphers[cipherId].archivedDate = null; }; if (typeof id === "string") { From 98d0960c2aeb02a507f7b64ab08b71f53ee92ab8 Mon Sep 17 00:00:00 2001 From: gitclonebrian <235774926+gitclonebrian@users.noreply.github.com> Date: Tue, 6 Jan 2026 10:33:01 -0500 Subject: [PATCH 09/17] added commands to pack:lin and pack:lin:arm64 scripts to include icons and .desktop file in tar.gz. (#18170) --- apps/desktop/package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/desktop/package.json b/apps/desktop/package.json index 97ab8585a69..17322c42a84 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -40,8 +40,8 @@ "clean:dist": "rimraf ./dist", "pack:dir": "npm run clean:dist && electron-builder --dir -p never", "pack:lin:flatpak": "flatpak-builder --repo=../../.flatpak-repo ../../.flatpak ./resources/com.bitwarden.desktop.devel.yaml --install-deps-from=flathub --force-clean && flatpak build-bundle ../../.flatpak-repo/ ./dist/com.bitwarden.desktop.flatpak com.bitwarden.desktop", - "pack:lin": "npm run clean:dist && electron-builder --linux --x64 -p never && export SNAP_FILE=$(realpath ./dist/bitwarden_*.snap) && unsquashfs -d ./dist/tmp-snap/ $SNAP_FILE && mkdir -p ./dist/tmp-snap/meta/polkit/ && cp ./resources/com.bitwarden.desktop.policy ./dist/tmp-snap/meta/polkit/polkit.com.bitwarden.desktop.policy && rm $SNAP_FILE && snap pack --compression=lzo ./dist/tmp-snap/ && mv ./*.snap ./dist/ && rm -rf ./dist/tmp-snap/ && tar -czvf ./dist/bitwarden_desktop_x64.tar.gz -C ./dist/linux-unpacked/ .", - "pack:lin:arm64": "npm run clean:dist && electron-builder --linux --arm64 -p never && export SNAP_FILE=$(realpath ./dist/bitwarden_*.snap) && unsquashfs -d ./dist/tmp-snap/ $SNAP_FILE && mkdir -p ./dist/tmp-snap/meta/polkit/ && cp ./resources/com.bitwarden.desktop.policy ./dist/tmp-snap/meta/polkit/polkit.com.bitwarden.desktop.policy && rm $SNAP_FILE && snap pack --compression=lzo ./dist/tmp-snap/ && mv ./*.snap ./dist/ && rm -rf ./dist/tmp-snap/ && tar -czvf ./dist/bitwarden_desktop_arm64.tar.gz -C ./dist/linux-arm64-unpacked/ .", + "pack:lin": "npm run clean:dist && electron-builder --linux --x64 -p never && export SNAP_FILE=$(realpath ./dist/bitwarden_*.snap) && unsquashfs -d ./dist/tmp-snap/ $SNAP_FILE && mkdir -p ./dist/tmp-snap/meta/polkit/ && cp ./resources/com.bitwarden.desktop.policy ./dist/tmp-snap/meta/polkit/polkit.com.bitwarden.desktop.policy && rm $SNAP_FILE && snap pack --compression=lzo ./dist/tmp-snap/ && mv ./*.snap ./dist/ && rm -rf ./dist/tmp-snap/ && cp ./resources/com.bitwarden.desktop.desktop ./dist/linux-unpacked/resources && cp -r ./resources/icons ./dist/linux-unpacked/resources && tar -czvf ./dist/bitwarden_desktop_x64.tar.gz -C ./dist/linux-unpacked/ .", + "pack:lin:arm64": "npm run clean:dist && electron-builder --linux --arm64 -p never && export SNAP_FILE=$(realpath ./dist/bitwarden_*.snap) && unsquashfs -d ./dist/tmp-snap/ $SNAP_FILE && mkdir -p ./dist/tmp-snap/meta/polkit/ && cp ./resources/com.bitwarden.desktop.policy ./dist/tmp-snap/meta/polkit/polkit.com.bitwarden.desktop.policy && rm $SNAP_FILE && snap pack --compression=lzo ./dist/tmp-snap/ && mv ./*.snap ./dist/ && rm -rf ./dist/tmp-snap/ && cp ./resources/com.bitwarden.desktop.desktop ./dist/linux-arm64-unpacked/resources && cp -r ./resources/icons ./dist/linux-arm64-unpacked/resources && tar -czvf ./dist/bitwarden_desktop_arm64.tar.gz -C ./dist/linux-arm64-unpacked/ .", "pack:mac": "npm run clean:dist && electron-builder --mac --universal -p never", "pack:mac:with-extension": "npm run clean:dist && npm run build:macos-extension:mac && electron-builder --mac --universal -p never", "pack:mac:arm64": "npm run clean:dist && electron-builder --mac --arm64 -p never", From 7d496febb77de4a314e1d6820757710c9033a9a2 Mon Sep 17 00:00:00 2001 From: Oscar Hinton Date: Tue, 6 Jan 2026 16:41:36 +0100 Subject: [PATCH 10/17] [PM-30473] Add links to tools in the sidebar (#18217) * Add links to tools in the sidebar * Fix test --- .../app/layout/desktop-layout.component.html | 10 ++++++--- .../layout/desktop-layout.component.spec.ts | 6 +++++- .../app/layout/desktop-layout.component.ts | 21 +++++++++++++++++-- .../web/src/app/layouts/navbar.component.html | 0 4 files changed, 31 insertions(+), 6 deletions(-) delete mode 100644 apps/web/src/app/layouts/navbar.component.html diff --git a/apps/desktop/src/app/layout/desktop-layout.component.html b/apps/desktop/src/app/layout/desktop-layout.component.html index 1717b29acd1..7e101ae1b6e 100644 --- a/apps/desktop/src/app/layout/desktop-layout.component.html +++ b/apps/desktop/src/app/layout/desktop-layout.component.html @@ -1,9 +1,13 @@ - + - - + + + + + + diff --git a/apps/desktop/src/app/layout/desktop-layout.component.spec.ts b/apps/desktop/src/app/layout/desktop-layout.component.spec.ts index 253444232e5..2fb49e723ef 100644 --- a/apps/desktop/src/app/layout/desktop-layout.component.spec.ts +++ b/apps/desktop/src/app/layout/desktop-layout.component.spec.ts @@ -5,7 +5,7 @@ import { mock } from "jest-mock-extended"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { FakeGlobalStateProvider } from "@bitwarden/common/spec"; -import { NavigationModule } from "@bitwarden/components"; +import { DialogService, NavigationModule } from "@bitwarden/components"; import { GlobalStateProvider } from "@bitwarden/state"; import { SendFiltersNavComponent } from "../tools/send-v2/send-filters-nav.component"; @@ -52,6 +52,10 @@ describe("DesktopLayoutComponent", () => { provide: GlobalStateProvider, useValue: fakeGlobalStateProvider, }, + { + provide: DialogService, + useValue: mock(), + }, ], }) .overrideComponent(DesktopLayoutComponent, { diff --git a/apps/desktop/src/app/layout/desktop-layout.component.ts b/apps/desktop/src/app/layout/desktop-layout.component.ts index 0ee7065fba8..85339bc06c9 100644 --- a/apps/desktop/src/app/layout/desktop-layout.component.ts +++ b/apps/desktop/src/app/layout/desktop-layout.component.ts @@ -1,10 +1,13 @@ -import { Component } from "@angular/core"; +import { Component, inject } from "@angular/core"; import { RouterModule } from "@angular/router"; import { PasswordManagerLogo } from "@bitwarden/assets/svg"; -import { LayoutComponent, NavigationModule } from "@bitwarden/components"; +import { DialogService, LayoutComponent, NavigationModule } from "@bitwarden/components"; import { I18nPipe } from "@bitwarden/ui-common"; +import { ExportDesktopComponent } from "../tools/export/export-desktop.component"; +import { CredentialGeneratorComponent } from "../tools/generator/credential-generator.component"; +import { ImportDesktopComponent } from "../tools/import/import-desktop.component"; import { SendFiltersNavComponent } from "../tools/send-v2/send-filters-nav.component"; import { DesktopSideNavComponent } from "./desktop-side-nav.component"; @@ -24,5 +27,19 @@ import { DesktopSideNavComponent } from "./desktop-side-nav.component"; templateUrl: "./desktop-layout.component.html", }) export class DesktopLayoutComponent { + private dialogService = inject(DialogService); + protected readonly logo = PasswordManagerLogo; + + protected openGenerator() { + this.dialogService.open(CredentialGeneratorComponent); + } + + protected openImport() { + this.dialogService.open(ImportDesktopComponent); + } + + protected openExport() { + this.dialogService.open(ExportDesktopComponent); + } } diff --git a/apps/web/src/app/layouts/navbar.component.html b/apps/web/src/app/layouts/navbar.component.html deleted file mode 100644 index e69de29bb2d..00000000000 From 9c8a92c8ac99aa485cb3fa3fb70798a60e11d643 Mon Sep 17 00:00:00 2001 From: Jordan Aasen <166539328+jaasen-livefront@users.noreply.github.com> Date: Tue, 6 Jan 2026 10:22:06 -0800 Subject: [PATCH 11/17] [PM-29214] - update at-risk launch link (#18093) * update at-risk launch link * use bit-hint * remove getter --- apps/browser/src/_locales/en/messages.json | 10 ++++++++-- .../login-credentials-view.component.html | 15 +++++++++------ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index 95d3f662994..d70025016ca 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -5664,6 +5664,12 @@ "changeAtRiskPasswordAndAddWebsite": { "message": "This login is at-risk and missing a website. Add a website and change the password for stronger security." }, + "vulnerablePassword": { + "message": "Vulnerable password." + }, + "changeNow": { + "message": "Change now" + }, "missingWebsite": { "message": "Missing website" }, @@ -6018,7 +6024,7 @@ "placeholders": { "organization": { "content": "$1", - "example": "My Org Name" + "example": "My Org Name" } } }, @@ -6027,7 +6033,7 @@ "placeholders": { "organization": { "content": "$1", - "example": "My Org Name" + "example": "My Org Name" } } }, diff --git a/libs/vault/src/cipher-view/login-credentials/login-credentials-view.component.html b/libs/vault/src/cipher-view/login-credentials/login-credentials-view.component.html index 2566752813c..8d7673cc710 100644 --- a/libs/vault/src/cipher-view/login-credentials/login-credentials-view.component.html +++ b/libs/vault/src/cipher-view/login-credentials/login-credentials-view.component.html @@ -90,12 +90,15 @@ data-testid="copy-password" (click)="logCopyEvent()" > - - - {{ "changeAtRiskPassword" | i18n }} - - - + @if (showChangePasswordLink) { + + + {{ "vulnerablePassword" | i18n }} + + {{ "changeNow" | i18n }} + + + }
Date: Tue, 6 Jan 2026 11:34:46 -0800 Subject: [PATCH 12/17] [PM-29800] - fix icon alignment in attachment view (#18112) * fix icon alignment in attachment view * move class to bit-item-action --- .../cipher-view/attachments/attachments-v2-view.component.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/vault/src/cipher-view/attachments/attachments-v2-view.component.html b/libs/vault/src/cipher-view/attachments/attachments-v2-view.component.html index 67ded3f8358..2ba9a16357a 100644 --- a/libs/vault/src/cipher-view/attachments/attachments-v2-view.component.html +++ b/libs/vault/src/cipher-view/attachments/attachments-v2-view.component.html @@ -9,7 +9,7 @@ {{ attachment.sizeName }} - + Date: Tue, 6 Jan 2026 15:24:03 -0500 Subject: [PATCH 13/17] feat(tokens): [BEEEP] Refresh access token on 401 API response * Update to handle 401 to refresh token. * Updated to revert changes to extract token comparison. * Fixed tests * Adjusted tests. * Removed debug logging * Test updates * Added race condition test. * Added clarified logout reason * Fixed typo Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> * Fixed tests * Fixed extra space * Removed extra logout reasons to be introduced later. * Added warning on 401 and retry --------- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> --- .../src/common/types/logout-reason.type.ts | 14 +- libs/common/src/abstractions/api.service.ts | 7 + libs/common/src/services/api.service.spec.ts | 794 ++++++++++++++++++ libs/common/src/services/api.service.ts | 220 +++-- 4 files changed, 961 insertions(+), 74 deletions(-) diff --git a/libs/auth/src/common/types/logout-reason.type.ts b/libs/auth/src/common/types/logout-reason.type.ts index 71fff51064a..dab19ca9418 100644 --- a/libs/auth/src/common/types/logout-reason.type.ts +++ b/libs/auth/src/common/types/logout-reason.type.ts @@ -1,10 +1,10 @@ export type LogoutReason = - | "invalidGrantError" - | "vaultTimeout" - | "invalidSecurityStamp" - | "logoutNotification" - | "keyConnectorError" - | "sessionExpired" | "accessTokenUnableToBeDecrypted" + | "accountDeleted" + | "invalidAccessToken" + | "invalidSecurityStamp" + | "keyConnectorError" + | "logoutNotification" | "refreshTokenSecureStorageRetrievalFailure" - | "accountDeleted"; + | "sessionExpired" + | "vaultTimeout"; diff --git a/libs/common/src/abstractions/api.service.ts b/libs/common/src/abstractions/api.service.ts index 72a17f0fa87..7e4ff031ef2 100644 --- a/libs/common/src/abstractions/api.service.ts +++ b/libs/common/src/abstractions/api.service.ts @@ -446,6 +446,13 @@ export abstract class ApiService { abstract postBitPayInvoice(request: BitPayInvoiceRequest): Promise; abstract postSetupPayment(): Promise; + /** + * Retrieves the bearer access token for the user. + * If the access token is expired or within 5 minutes of expiration, attempts to refresh the token + * and persists the refresh token to state before returning it. + * @param userId The user for whom we're retrieving the access token + * @returns The access token, or an Error if no access token exists. + */ abstract getActiveBearerToken(userId: UserId): Promise; abstract fetch(request: Request): Promise; abstract nativeFetch(request: Request): Promise; diff --git a/libs/common/src/services/api.service.spec.ts b/libs/common/src/services/api.service.spec.ts index 9ab84ecb16b..faed9ff77a7 100644 --- a/libs/common/src/services/api.service.spec.ts +++ b/libs/common/src/services/api.service.spec.ts @@ -449,4 +449,798 @@ describe("ApiService", () => { ).rejects.toThrow(InsecureUrlNotAllowedError); expect(nativeFetch).not.toHaveBeenCalled(); }); + + describe("When a 401 Unauthorized status is received", () => { + it("retries request with refreshed token when initial request with access token returns 401", async () => { + // This test verifies the 401 retry flow: + // 1. Initial request with valid token returns 401 (token expired server-side) + // 2. After 401, buildRequest is called again, which checks tokenNeedsRefresh + // 3. tokenNeedsRefresh returns true, triggering refreshToken via getActiveBearerToken + // 4. refreshToken makes an HTTP call to /connect/token to get new tokens + // 5. setTokens is called to store the new tokens, returning the refreshed access token + // 6. Request is retried with the refreshed token and succeeds + environmentService.getEnvironment$.calledWith(testActiveUser).mockReturnValue( + of({ + getApiUrl: () => "https://example.com", + getIdentityUrl: () => "https://identity.example.com", + } satisfies Partial as Environment), + ); + + httpOperations.createRequest.mockImplementation((url, request) => { + return { + url: url, + cache: request.cache, + credentials: request.credentials, + method: request.method, + mode: request.mode, + signal: request.signal, + headers: new Headers(request.headers), + } satisfies Partial as unknown as Request; + }); + tokenService.getAccessToken.calledWith(testActiveUser).mockResolvedValue("access_token"); + // First call (initial request): token doesn't need refresh yet + // Subsequent calls (after 401): token needs refresh, triggering the refresh flow + tokenService.tokenNeedsRefresh + .calledWith(testActiveUser) + .mockResolvedValueOnce(false) + .mockResolvedValue(true); + + tokenService.getRefreshToken.calledWith(testActiveUser).mockResolvedValue("refresh_token"); + + tokenService.decodeAccessToken + .calledWith(testActiveUser) + .mockResolvedValue({ client_id: "web" }); + + tokenService.decodeAccessToken + .calledWith("new_access_token") + .mockResolvedValue({ sub: testActiveUser }); + + vaultTimeoutSettingsService.getVaultTimeoutActionByUserId$ + .calledWith(testActiveUser) + .mockReturnValue(of(VaultTimeoutAction.Lock)); + + vaultTimeoutSettingsService.getVaultTimeoutByUserId$ + .calledWith(testActiveUser) + .mockReturnValue(of(VaultTimeoutStringType.Never)); + + tokenService.setTokens + .calledWith( + "new_access_token", + VaultTimeoutAction.Lock, + VaultTimeoutStringType.Never, + "new_refresh_token", + ) + .mockResolvedValue({ accessToken: "new_access_token" }); + + const nativeFetch = jest.fn, [request: Request]>(); + let callCount = 0; + + nativeFetch.mockImplementation((request) => { + callCount++; + + // First call: initial request with expired token returns 401 + if (callCount === 1) { + return Promise.resolve({ + ok: false, + status: 401, + json: () => Promise.resolve({ message: "Unauthorized" }), + headers: new Headers({ + "content-type": "application/json", + }), + } satisfies Partial as Response); + } + + // Second call: token refresh request + if (callCount === 2 && request.url.includes("identity")) { + return Promise.resolve({ + ok: true, + status: 200, + json: () => + Promise.resolve({ + access_token: "new_access_token", + token_type: "Bearer", + refresh_token: "new_refresh_token", + }), + } satisfies Partial as Response); + } + + // Third call: retry with refreshed token succeeds + if (callCount === 3) { + expect(request.headers.get("Authorization")).toBe("Bearer new_access_token"); + return Promise.resolve({ + ok: true, + status: 200, + json: () => Promise.resolve({ data: "success" }), + headers: new Headers({ + "content-type": "application/json", + }), + } satisfies Partial as Response); + } + + throw new Error(`Unexpected call #${callCount}: ${request.method} ${request.url}`); + }); + + sut.nativeFetch = nativeFetch; + + const response = await sut.send("GET", "/something", null, true, true, null, null); + + expect(nativeFetch).toHaveBeenCalledTimes(3); + expect(response).toEqual({ data: "success" }); + }); + + it("does not retry when request has no access token and returns 401", async () => { + environmentService.environment$ = of({ + getApiUrl: () => "https://example.com", + } satisfies Partial as Environment); + + httpOperations.createRequest.mockImplementation((url, request) => { + return { + url: url, + cache: request.cache, + credentials: request.credentials, + method: request.method, + mode: request.mode, + signal: request.signal, + headers: new Headers(request.headers), + } satisfies Partial as unknown as Request; + }); + + const nativeFetch = jest.fn, [request: Request]>(); + + nativeFetch.mockImplementation((request) => { + return Promise.resolve({ + ok: false, + status: 401, + json: () => Promise.resolve({ message: "Unauthorized" }), + headers: new Headers({ + "content-type": "application/json", + }), + } satisfies Partial as Response); + }); + + sut.nativeFetch = nativeFetch; + + await expect( + async () => await sut.send("GET", "/something", null, false, true, null, null), + ).rejects.toMatchObject({ message: "Unauthorized" }); + + // Should only be called once (no retry) + expect(nativeFetch).toHaveBeenCalledTimes(1); + }); + + it("does not retry when request returns non-401 error", async () => { + environmentService.getEnvironment$.calledWith(testActiveUser).mockReturnValue( + of({ + getApiUrl: () => "https://example.com", + getIdentityUrl: () => "https://identity.example.com", + } satisfies Partial as Environment), + ); + + httpOperations.createRequest.mockImplementation((url, request) => { + return { + url: url, + cache: request.cache, + credentials: request.credentials, + method: request.method, + mode: request.mode, + signal: request.signal, + headers: new Headers(request.headers), + } satisfies Partial as unknown as Request; + }); + + tokenService.getAccessToken.calledWith(testActiveUser).mockResolvedValue("valid_token"); + tokenService.tokenNeedsRefresh.calledWith(testActiveUser).mockResolvedValue(false); + + const nativeFetch = jest.fn, [request: Request]>(); + + nativeFetch.mockImplementation((request) => { + return Promise.resolve({ + ok: false, + status: 400, + json: () => Promise.resolve({ message: "Bad Request" }), + headers: new Headers({ + "content-type": "application/json", + }), + } satisfies Partial as Response); + }); + + sut.nativeFetch = nativeFetch; + + await expect( + async () => await sut.send("GET", "/something", null, true, true, null, null), + ).rejects.toMatchObject({ message: "Bad Request" }); + + // Should only be called once (no retry for non-401 errors) + expect(nativeFetch).toHaveBeenCalledTimes(1); + }); + + it("does not attempt to log out unauthenticated user", async () => { + environmentService.environment$ = of({ + getApiUrl: () => "https://example.com", + } satisfies Partial as Environment); + + httpOperations.createRequest.mockImplementation((url, request) => { + return { + url: url, + cache: request.cache, + credentials: request.credentials, + method: request.method, + mode: request.mode, + signal: request.signal, + headers: new Headers(request.headers), + } satisfies Partial as unknown as Request; + }); + + const nativeFetch = jest.fn, [request: Request]>(); + + nativeFetch.mockImplementation((request) => { + return Promise.resolve({ + ok: false, + status: 401, + json: () => Promise.resolve({ message: "Unauthorized" }), + headers: new Headers({ + "content-type": "application/json", + }), + } satisfies Partial as Response); + }); + + sut.nativeFetch = nativeFetch; + + await expect( + async () => await sut.send("GET", "/something", null, false, true, null, null), + ).rejects.toMatchObject({ message: "Unauthorized" }); + + expect(logoutCallback).not.toHaveBeenCalled(); + }); + + it("does not retry when hasResponse is false", async () => { + environmentService.environment$ = of({ + getApiUrl: () => "https://example.com", + } satisfies Partial as Environment); + + environmentService.getEnvironment$.calledWith(testActiveUser).mockReturnValue( + of({ + getApiUrl: () => "https://example.com", + getIdentityUrl: () => "https://identity.example.com", + } satisfies Partial as Environment), + ); + + httpOperations.createRequest.mockImplementation((url, request) => { + return { + url: url, + cache: request.cache, + credentials: request.credentials, + method: request.method, + mode: request.mode, + signal: request.signal, + headers: new Headers(request.headers), + } satisfies Partial as unknown as Request; + }); + + tokenService.getAccessToken.calledWith(testActiveUser).mockResolvedValue("expired_token"); + tokenService.tokenNeedsRefresh.calledWith(testActiveUser).mockResolvedValue(false); + + const nativeFetch = jest.fn, [request: Request]>(); + + nativeFetch.mockImplementation((request) => { + return Promise.resolve({ + ok: false, + status: 401, + json: () => Promise.resolve({ message: "Unauthorized" }), + headers: new Headers({ + "content-type": "application/json", + }), + } satisfies Partial as Response); + }); + + sut.nativeFetch = nativeFetch; + + // When hasResponse is false, the method should throw even though no retry happens + await expect( + async () => await sut.send("POST", "/something", null, true, false, null, null), + ).rejects.toMatchObject({ message: "Unauthorized" }); + + // Should only be called once (no retry when hasResponse is false) + expect(nativeFetch).toHaveBeenCalledTimes(1); + }); + + it("uses original user token for retry even if active user changes between requests", async () => { + // Setup: Initial request is for testActiveUser, but during the retry, the active user switches + // to testInactiveUser. The retry should still use testActiveUser's refreshed token. + + let activeUserId = testActiveUser; + + // Mock accountService to return different active users based on when it's called + accountService.activeAccount$ = of({ + id: activeUserId, + email: "user1@example.com", + emailVerified: true, + name: "Test Name", + } satisfies ObservedValueOf); + + environmentService.getEnvironment$.calledWith(testActiveUser).mockReturnValue( + of({ + getApiUrl: () => "https://example.com", + getIdentityUrl: () => "https://identity.example.com", + } satisfies Partial as Environment), + ); + + environmentService.getEnvironment$.calledWith(testInactiveUser).mockReturnValue( + of({ + getApiUrl: () => "https://inactive.example.com", + getIdentityUrl: () => "https://identity.inactive.example.com", + } satisfies Partial as Environment), + ); + + httpOperations.createRequest.mockImplementation((url, request) => { + return { + url: url, + cache: request.cache, + credentials: request.credentials, + method: request.method, + mode: request.mode, + signal: request.signal, + headers: new Headers(request.headers), + } satisfies Partial as unknown as Request; + }); + + tokenService.getAccessToken + .calledWith(testActiveUser) + .mockResolvedValue("active_access_token"); + tokenService.tokenNeedsRefresh + .calledWith(testActiveUser) + .mockResolvedValueOnce(false) + .mockResolvedValue(true); + + tokenService.getRefreshToken + .calledWith(testActiveUser) + .mockResolvedValue("active_refresh_token"); + + tokenService.decodeAccessToken + .calledWith(testActiveUser) + .mockResolvedValue({ client_id: "web" }); + + tokenService.decodeAccessToken + .calledWith("active_new_access_token") + .mockResolvedValue({ sub: testActiveUser }); + + vaultTimeoutSettingsService.getVaultTimeoutActionByUserId$ + .calledWith(testActiveUser) + .mockReturnValue(of(VaultTimeoutAction.Lock)); + + vaultTimeoutSettingsService.getVaultTimeoutByUserId$ + .calledWith(testActiveUser) + .mockReturnValue(of(VaultTimeoutStringType.Never)); + + tokenService.setTokens + .calledWith( + "active_new_access_token", + VaultTimeoutAction.Lock, + VaultTimeoutStringType.Never, + "active_new_refresh_token", + ) + .mockResolvedValue({ accessToken: "active_new_access_token" }); + + // Mock tokens for inactive user (should NOT be used) + tokenService.getAccessToken + .calledWith(testInactiveUser) + .mockResolvedValue("inactive_access_token"); + + const nativeFetch = jest.fn, [request: Request]>(); + let callCount = 0; + + nativeFetch.mockImplementation((request) => { + callCount++; + + // First call: initial request with active user's token returns 401 + if (callCount === 1) { + expect(request.url).toBe("https://example.com/something"); + expect(request.headers.get("Authorization")).toBe("Bearer active_access_token"); + + // After the 401, simulate active user changing + activeUserId = testInactiveUser; + accountService.activeAccount$ = of({ + id: testInactiveUser, + email: "user2@example.com", + emailVerified: true, + name: "Inactive User", + } satisfies ObservedValueOf); + + return Promise.resolve({ + ok: false, + status: 401, + json: () => Promise.resolve({ message: "Unauthorized" }), + headers: new Headers({ + "content-type": "application/json", + }), + } satisfies Partial as Response); + } + + // Second call: token refresh request for ORIGINAL user (testActiveUser) + if (callCount === 2 && request.url.includes("identity")) { + expect(request.url).toContain("identity.example.com"); + return Promise.resolve({ + ok: true, + status: 200, + json: () => + Promise.resolve({ + access_token: "active_new_access_token", + token_type: "Bearer", + refresh_token: "active_new_refresh_token", + }), + } satisfies Partial as Response); + } + + // Third call: retry with ORIGINAL user's refreshed token, NOT the new active user's token + if (callCount === 3) { + expect(request.url).toBe("https://example.com/something"); + expect(request.headers.get("Authorization")).toBe("Bearer active_new_access_token"); + // Verify we're NOT using the inactive user's endpoint + expect(request.url).not.toContain("inactive"); + return Promise.resolve({ + ok: true, + status: 200, + json: () => Promise.resolve({ data: "success with original user" }), + headers: new Headers({ + "content-type": "application/json", + }), + } satisfies Partial as Response); + } + + throw new Error(`Unexpected call #${callCount}: ${request.method} ${request.url}`); + }); + + sut.nativeFetch = nativeFetch; + + // Explicitly pass testActiveUser to ensure the request is for that specific user + const response = await sut.send("GET", "/something", null, testActiveUser, true, null, null); + + expect(nativeFetch).toHaveBeenCalledTimes(3); + expect(response).toEqual({ data: "success with original user" }); + + // Verify that inactive user's token was never requested + expect(tokenService.getAccessToken.calledWith(testInactiveUser)).not.toHaveBeenCalled(); + }); + + it("throws error when retry also returns 401", async () => { + environmentService.getEnvironment$.calledWith(testActiveUser).mockReturnValue( + of({ + getApiUrl: () => "https://example.com", + getIdentityUrl: () => "https://identity.example.com", + } satisfies Partial as Environment), + ); + + httpOperations.createRequest.mockImplementation((url, request) => { + return { + url: url, + cache: request.cache, + credentials: request.credentials, + method: request.method, + mode: request.mode, + signal: request.signal, + headers: new Headers(request.headers), + } satisfies Partial as unknown as Request; + }); + + tokenService.getAccessToken.calledWith(testActiveUser).mockResolvedValue("access_token"); + // First call (initial request): token doesn't need refresh yet + // Subsequent calls (after 401): token needs refresh, triggering the refresh flow + tokenService.tokenNeedsRefresh + .calledWith(testActiveUser) + .mockResolvedValueOnce(false) + .mockResolvedValue(true); + + tokenService.getRefreshToken.calledWith(testActiveUser).mockResolvedValue("refresh_token"); + + tokenService.decodeAccessToken + .calledWith(testActiveUser) + .mockResolvedValue({ client_id: "web" }); + + tokenService.decodeAccessToken + .calledWith("new_access_token") + .mockResolvedValue({ sub: testActiveUser }); + + vaultTimeoutSettingsService.getVaultTimeoutActionByUserId$ + .calledWith(testActiveUser) + .mockReturnValue(of(VaultTimeoutAction.Lock)); + + vaultTimeoutSettingsService.getVaultTimeoutByUserId$ + .calledWith(testActiveUser) + .mockReturnValue(of(VaultTimeoutStringType.Never)); + + tokenService.setTokens + .calledWith( + "new_access_token", + VaultTimeoutAction.Lock, + VaultTimeoutStringType.Never, + "new_refresh_token", + ) + .mockResolvedValue({ accessToken: "new_access_token" }); + + const nativeFetch = jest.fn, [request: Request]>(); + let callCount = 0; + + nativeFetch.mockImplementation((request) => { + callCount++; + + // First call: initial request with expired token returns 401 + if (callCount === 1) { + return Promise.resolve({ + ok: false, + status: 401, + json: () => Promise.resolve({ message: "Unauthorized" }), + headers: new Headers({ + "content-type": "application/json", + }), + } satisfies Partial as Response); + } + + // Second call: token refresh request + if (callCount === 2 && request.url.includes("identity")) { + return Promise.resolve({ + ok: true, + status: 200, + json: () => + Promise.resolve({ + access_token: "new_access_token", + token_type: "Bearer", + refresh_token: "new_refresh_token", + }), + } satisfies Partial as Response); + } + + // Third call: retry with refreshed token still returns 401 (user no longer has permission) + if (callCount === 3) { + return Promise.resolve({ + ok: false, + status: 401, + json: () => Promise.resolve({ message: "Still Unauthorized" }), + headers: new Headers({ + "content-type": "application/json", + }), + } satisfies Partial as Response); + } + + throw new Error("Unexpected call"); + }); + + sut.nativeFetch = nativeFetch; + + await expect( + async () => await sut.send("GET", "/something", null, true, true, null, null), + ).rejects.toMatchObject({ message: "Still Unauthorized" }); + + expect(nativeFetch).toHaveBeenCalledTimes(3); + expect(logoutCallback).toHaveBeenCalledWith("invalidAccessToken"); + }); + + it("handles concurrent requests that both receive 401 and share token refresh", async () => { + // This test verifies the race condition scenario: + // 1. Request A starts with valid token + // 2. Request B starts with valid token + // 3. Request A gets 401, triggers refresh + // 4. Request B gets 401 while A is refreshing + // 5. Request B should wait for A's refresh to complete (via refreshTokenPromise cache) + // 6. Both requests retry with the new token + + environmentService.getEnvironment$.calledWith(testActiveUser).mockReturnValue( + of({ + getApiUrl: () => "https://example.com", + getIdentityUrl: () => "https://identity.example.com", + } satisfies Partial as Environment), + ); + + httpOperations.createRequest.mockImplementation((url, request) => { + return { + url: url, + cache: request.cache, + credentials: request.credentials, + method: request.method, + mode: request.mode, + signal: request.signal, + headers: new Headers(request.headers), + } satisfies Partial as unknown as Request; + }); + + tokenService.getAccessToken.calledWith(testActiveUser).mockResolvedValue("expired_token"); + + // First two calls: token doesn't need refresh yet + // Subsequent calls: token needs refresh + tokenService.tokenNeedsRefresh + .calledWith(testActiveUser) + .mockResolvedValueOnce(false) // Request A initial + .mockResolvedValueOnce(false) // Request B initial + .mockResolvedValue(true); // Both retries after 401 + + tokenService.getRefreshToken.calledWith(testActiveUser).mockResolvedValue("refresh_token"); + + tokenService.decodeAccessToken + .calledWith(testActiveUser) + .mockResolvedValue({ client_id: "web" }); + + tokenService.decodeAccessToken + .calledWith("new_access_token") + .mockResolvedValue({ sub: testActiveUser }); + + vaultTimeoutSettingsService.getVaultTimeoutActionByUserId$ + .calledWith(testActiveUser) + .mockReturnValue(of(VaultTimeoutAction.Lock)); + + vaultTimeoutSettingsService.getVaultTimeoutByUserId$ + .calledWith(testActiveUser) + .mockReturnValue(of(VaultTimeoutStringType.Never)); + + tokenService.setTokens + .calledWith( + "new_access_token", + VaultTimeoutAction.Lock, + VaultTimeoutStringType.Never, + "new_refresh_token", + ) + .mockResolvedValue({ accessToken: "new_access_token" }); + + const nativeFetch = jest.fn, [request: Request]>(); + let apiRequestCount = 0; + let refreshRequestCount = 0; + + nativeFetch.mockImplementation((request) => { + if (request.url.includes("identity")) { + refreshRequestCount++; + // Simulate slow token refresh to expose race condition + return new Promise((resolve) => + setTimeout( + () => + resolve({ + ok: true, + status: 200, + json: () => + Promise.resolve({ + access_token: "new_access_token", + token_type: "Bearer", + refresh_token: "new_refresh_token", + }), + } satisfies Partial as Response), + 100, + ), + ); + } + + apiRequestCount++; + const currentCall = apiRequestCount; + + // First two calls (Request A and B initial attempts): both return 401 + if (currentCall === 1 || currentCall === 2) { + return Promise.resolve({ + ok: false, + status: 401, + json: () => Promise.resolve({ message: "Unauthorized" }), + headers: new Headers({ + "content-type": "application/json", + }), + } satisfies Partial as Response); + } + + // Third and fourth calls (retries after refresh): both succeed + if (currentCall === 3 || currentCall === 4) { + expect(request.headers.get("Authorization")).toBe("Bearer new_access_token"); + return Promise.resolve({ + ok: true, + status: 200, + json: () => Promise.resolve({ data: `success-${currentCall}` }), + headers: new Headers({ + "content-type": "application/json", + }), + } satisfies Partial as Response); + } + + throw new Error(`Unexpected API call #${currentCall}: ${request.method} ${request.url}`); + }); + + sut.nativeFetch = nativeFetch; + + // Make two concurrent requests + const [responseA, responseB] = await Promise.all([ + sut.send("GET", "/endpoint-a", null, testActiveUser, true, null, null), + sut.send("GET", "/endpoint-b", null, testActiveUser, true, null, null), + ]); + + // Both requests should succeed + expect(responseA).toMatchObject({ data: expect.stringContaining("success") }); + expect(responseB).toMatchObject({ data: expect.stringContaining("success") }); + + // Verify only ONE token refresh was made (they shared the refresh) + expect(refreshRequestCount).toBe(1); + + // Verify the total number of API requests: 2 initial + 2 retries = 4 + expect(apiRequestCount).toBe(4); + + // Verify setTokens was only called once + expect(tokenService.setTokens).toHaveBeenCalledTimes(1); + }); + }); + + describe("When 403 Forbidden response is received from API request", () => { + it("logs out the authenticated user", async () => { + environmentService.getEnvironment$.calledWith(testActiveUser).mockReturnValue( + of({ + getApiUrl: () => "https://example.com", + } satisfies Partial as Environment), + ); + + httpOperations.createRequest.mockImplementation((url, request) => { + return { + url: url, + cache: request.cache, + credentials: request.credentials, + method: request.method, + mode: request.mode, + signal: request.signal, + headers: new Headers(request.headers), + } satisfies Partial as unknown as Request; + }); + + tokenService.getAccessToken.calledWith(testActiveUser).mockResolvedValue("valid_token"); + tokenService.tokenNeedsRefresh.calledWith(testActiveUser).mockResolvedValue(false); + + const nativeFetch = jest.fn, [request: Request]>(); + + nativeFetch.mockImplementation((request) => { + return Promise.resolve({ + ok: false, + status: 403, + json: () => Promise.resolve({ message: "Forbidden" }), + headers: new Headers({ + "content-type": "application/json", + }), + } satisfies Partial as Response); + }); + + sut.nativeFetch = nativeFetch; + + await expect( + async () => await sut.send("GET", "/something", null, true, true, null, null), + ).rejects.toMatchObject({ message: "Forbidden" }); + + expect(logoutCallback).toHaveBeenCalledWith("invalidAccessToken"); + }); + + it("does not attempt to log out unauthenticated user", async () => { + environmentService.environment$ = of({ + getApiUrl: () => "https://example.com", + } satisfies Partial as Environment); + + httpOperations.createRequest.mockImplementation((url, request) => { + return { + url: url, + cache: request.cache, + credentials: request.credentials, + method: request.method, + mode: request.mode, + signal: request.signal, + headers: new Headers(request.headers), + } satisfies Partial as unknown as Request; + }); + + const nativeFetch = jest.fn, [request: Request]>(); + + nativeFetch.mockImplementation((request) => { + return Promise.resolve({ + ok: false, + status: 403, + json: () => Promise.resolve({ message: "Forbidden" }), + headers: new Headers({ + "content-type": "application/json", + }), + } satisfies Partial as Response); + }); + + sut.nativeFetch = nativeFetch; + + await expect( + async () => await sut.send("GET", "/something", null, false, true, null, null), + ).rejects.toMatchObject({ message: "Forbidden" }); + + expect(logoutCallback).not.toHaveBeenCalled(); + }); + }); }); diff --git a/libs/common/src/services/api.service.ts b/libs/common/src/services/api.service.ts index c60f6c5e907..10f349fbec7 100644 --- a/libs/common/src/services/api.service.ts +++ b/libs/common/src/services/api.service.ts @@ -74,7 +74,7 @@ import { BillingHistoryResponse } from "../billing/models/response/billing-histo import { PaymentResponse } from "../billing/models/response/payment.response"; import { PlanResponse } from "../billing/models/response/plan.response"; import { SubscriptionResponse } from "../billing/models/response/subscription.response"; -import { ClientType, DeviceType } from "../enums"; +import { ClientType, DeviceType, HttpStatusCode } from "../enums"; import { KeyConnectorUserKeyRequest } from "../key-management/key-connector/models/key-connector-user-key.request"; import { SetKeyConnectorKeyRequest } from "../key-management/key-connector/models/set-key-connector-key.request"; import { VaultTimeoutSettingsService } from "../key-management/vault-timeout"; @@ -1252,8 +1252,8 @@ export class ApiService implements ApiServiceAbstraction { }), ); - if (response.status !== 200) { - const error = await this.handleError(response, false, true); + if (response.status !== HttpStatusCode.Ok) { + const error = await this.handleApiRequestError(response, true); return Promise.reject(error); } @@ -1283,8 +1283,8 @@ export class ApiService implements ApiServiceAbstraction { }), ); - if (response.status !== 200) { - const error = await this.handleError(response, false, true); + if (response.status !== HttpStatusCode.Ok) { + const error = await this.handleApiRequestError(response, true); return Promise.reject(error); } } @@ -1301,14 +1301,12 @@ export class ApiService implements ApiServiceAbstraction { }), ); - if (response.status !== 200) { - const error = await this.handleError(response, false, true); + if (response.status !== HttpStatusCode.Ok) { + const error = await this.handleApiRequestError(response, true); return Promise.reject(error); } } - // Helpers - async getActiveBearerToken(userId: UserId): Promise { let accessToken = await this.tokenService.getAccessToken(userId); if (await this.tokenService.tokenNeedsRefresh(userId)) { @@ -1370,7 +1368,7 @@ export class ApiService implements ApiServiceAbstraction { const body = await response.json(); return new SsoPreValidateResponse(body); } else { - const error = await this.handleError(response, false, true); + const error = await this.handleApiRequestError(response, false); return Promise.reject(error); } } @@ -1525,7 +1523,7 @@ export class ApiService implements ApiServiceAbstraction { ); return refreshedTokens.accessToken; } else { - const error = await this.handleError(response, true, true); + const error = await this.handleTokenRefreshRequestError(response); return Promise.reject(error); } } @@ -1580,6 +1578,89 @@ export class ApiService implements ApiServiceAbstraction { apiUrl?: string | null, alterHeaders?: (headers: Headers) => void, ): Promise { + // We assume that if there is a UserId making the request, it is also an authenticated + // request and we will attempt to add an access token to the request. + const userIdMakingRequest = await this.getUserIdMakingRequest(authedOrUserId); + + const environment = await firstValueFrom( + userIdMakingRequest == null + ? this.environmentService.environment$ + : this.environmentService.getEnvironment$(userIdMakingRequest), + ); + apiUrl = Utils.isNullOrWhitespace(apiUrl) ? environment.getApiUrl() : apiUrl; + + const requestUrl = await this.buildSafeApiRequestUrl(apiUrl, path); + + let request = await this.buildRequest( + method, + userIdMakingRequest, + environment, + hasResponse, + body, + alterHeaders, + ); + + let response = await this.fetch(this.httpOperations.createRequest(requestUrl, request)); + + // First, check to see if we were making an authenticated request and received an Unauthorized (401) + // response. This could mean that we attempted to make a request with an expired access token. + // If so, attempt to refresh the token and try again. + if ( + hasResponse && + userIdMakingRequest != null && + response.status === HttpStatusCode.Unauthorized + ) { + this.logService.warning( + "Unauthorized response received for request to " + path + ". Attempting request again.", + ); + request = await this.buildRequest( + method, + userIdMakingRequest, + environment, + hasResponse, + body, + alterHeaders, + ); + response = await this.fetch(this.httpOperations.createRequest(requestUrl, request)); + } + + // At this point we are processing either the initial response or the response for the retry with the refreshed + // access token. + const responseType = response.headers.get("content-type"); + const responseIsJson = responseType != null && responseType.indexOf("application/json") !== -1; + const responseIsCsv = responseType != null && responseType.indexOf("text/csv") !== -1; + if (hasResponse && response.status === HttpStatusCode.Ok && responseIsJson) { + const responseJson = await response.json(); + return responseJson; + } else if (hasResponse && response.status === HttpStatusCode.Ok && responseIsCsv) { + return await response.text(); + } else if ( + response.status !== HttpStatusCode.Ok && + response.status !== HttpStatusCode.NoContent + ) { + const error = await this.handleApiRequestError(response, userIdMakingRequest != null); + return Promise.reject(error); + } + } + + private buildSafeApiRequestUrl(apiUrl: string, path: string): string { + const pathParts = path.split("?"); + + // Check for path traversal patterns from any URL. + const fullUrlPath = apiUrl + pathParts[0] + (pathParts.length > 1 ? `?${pathParts[1]}` : ""); + + const isInvalidUrl = Utils.invalidUrlPatterns(fullUrlPath); + if (isInvalidUrl) { + throw new Error("The request URL contains dangerous patterns."); + } + + const requestUrl = + apiUrl + Utils.normalizePath(pathParts[0]) + (pathParts.length > 1 ? `?${pathParts[1]}` : ""); + + return requestUrl; + } + + private async getUserIdMakingRequest(authedOrUserId: UserId | boolean): Promise { if (authedOrUserId == null) { throw new Error("A user id was given but it was null, cannot complete API request."); } @@ -1591,29 +1672,19 @@ export class ApiService implements ApiServiceAbstraction { } else if (typeof authedOrUserId === "string") { userId = authedOrUserId; } + return userId; + } - const env = await firstValueFrom( - userId == null - ? this.environmentService.environment$ - : this.environmentService.getEnvironment$(userId), - ); - apiUrl = Utils.isNullOrWhitespace(apiUrl) ? env.getApiUrl() : apiUrl; - - const pathParts = path.split("?"); - // Check for path traversal patterns from any URL. - const fullUrlPath = apiUrl + pathParts[0] + (pathParts.length > 1 ? `?${pathParts[1]}` : ""); - - const isInvalidUrl = Utils.invalidUrlPatterns(fullUrlPath); - if (isInvalidUrl) { - throw new Error("The request URL contains dangerous patterns."); - } - - // Prevent directory traversal from malicious paths - const requestUrl = - apiUrl + Utils.normalizePath(pathParts[0]) + (pathParts.length > 1 ? `?${pathParts[1]}` : ""); - + private async buildRequest( + method: "GET" | "POST" | "PUT" | "DELETE" | "PATCH", + userForAccessToken: UserId | null, + environment: Environment, + hasResponse: boolean, + body: string, + alterHeaders?: (headers: Headers) => void, + ): Promise { const [requestHeaders, requestBody] = await this.buildHeadersAndBody( - userId, + userForAccessToken, hasResponse, body, alterHeaders, @@ -1621,29 +1692,17 @@ export class ApiService implements ApiServiceAbstraction { const requestInit: RequestInit = { cache: "no-store", - credentials: await this.getCredentials(env), + credentials: await this.getCredentials(environment), method: method, }; requestInit.headers = requestHeaders; requestInit.body = requestBody; - const response = await this.fetch(this.httpOperations.createRequest(requestUrl, requestInit)); - const responseType = response.headers.get("content-type"); - const responseIsJson = responseType != null && responseType.indexOf("application/json") !== -1; - const responseIsCsv = responseType != null && responseType.indexOf("text/csv") !== -1; - if (hasResponse && response.status === 200 && responseIsJson) { - const responseJson = await response.json(); - return responseJson; - } else if (hasResponse && response.status === 200 && responseIsCsv) { - return await response.text(); - } else if (response.status !== 200 && response.status !== 204) { - const error = await this.handleError(response, false, userId != null); - return Promise.reject(error); - } + return requestInit; } private async buildHeadersAndBody( - userToAuthenticate: UserId | null, + userForAccessToken: UserId | null, hasResponse: boolean, body: any, alterHeaders: (headers: Headers) => void, @@ -1665,8 +1724,8 @@ export class ApiService implements ApiServiceAbstraction { if (alterHeaders != null) { alterHeaders(headers); } - if (userToAuthenticate != null) { - const authHeader = await this.getActiveBearerToken(userToAuthenticate); + if (userForAccessToken != null) { + const authHeader = await this.getActiveBearerToken(userForAccessToken); headers.set("Authorization", "Bearer " + authHeader); } else { // For unauthenticated requests, we need to tell the server what the device is for flag targeting, @@ -1692,32 +1751,59 @@ export class ApiService implements ApiServiceAbstraction { return [headers, requestBody]; } - private async handleError( + /** + * Handle an error response from a request to the Bitwarden API. + * If the request is made with an access token (aka the user is authenticated), + * and we receive a 401 or 403 response, we will log the user out, as this indicates + * that the access token used on the request is either expired or does not have the appropriate permissions. + * It is unlikely that it is expired, as we attempt to refresh the token on initial failure. + * @param response The response from the API request + * @param userIsAuthenticated A boolean indicating whether this is an authenticated request. + * @returns An ErrorResponse with a message based on the response status. + */ + private async handleApiRequestError( response: Response, - tokenError: boolean, - authed: boolean, + userIsAuthenticated: boolean, ): Promise { + if ( + userIsAuthenticated && + (response.status === HttpStatusCode.Unauthorized || + response.status === HttpStatusCode.Forbidden) + ) { + await this.logoutCallback("invalidAccessToken"); + } + + const responseJson = await this.getJsonResponse(response); + return new ErrorResponse(responseJson, response.status); + } + + /** + * Handle an error response when trying to refresh an access token. + * If the error indicates that the user's session has expired, it will log the user out. + * @param response The response from the token refresh request. + * @returns An ErrorResponse with a message based on the response status. + */ + private async handleTokenRefreshRequestError(response: Response): Promise { + const responseJson = await this.getJsonResponse(response); + + // IdentityServer will return an invalid_grant response if the refresh token has expired. + // This means that the user's session has expired, and they need to log out. + // We issue the logoutCallback() to log the user out through messaging. + if (response.status === HttpStatusCode.BadRequest && responseJson?.error === "invalid_grant") { + await this.logoutCallback("sessionExpired"); + } + + return new ErrorResponse(responseJson, response.status, true); + } + + private async getJsonResponse(response: Response): Promise { let responseJson: any = null; if (this.isJsonResponse(response)) { responseJson = await response.json(); } else if (this.isTextPlainResponse(response)) { responseJson = { Message: await response.text() }; } - - if (authed) { - if ( - response.status === 401 || - response.status === 403 || - (tokenError && - response.status === 400 && - responseJson != null && - responseJson.error === "invalid_grant") - ) { - await this.logoutCallback("invalidGrantError"); - } - } - - return new ErrorResponse(responseJson, response.status, tokenError); + return responseJson; } private qsStringify(params: any): string { From f8ca91c8a72baf775712aea5226b94edbeef3f01 Mon Sep 17 00:00:00 2001 From: Jordan Aasen <166539328+jaasen-livefront@users.noreply.github.com> Date: Tue, 6 Jan 2026 12:50:46 -0800 Subject: [PATCH 14/17] [PM-25693] - hide import sshKey button for view-only users (#17985) * hide import sshKey button for view-only users * use @if * add optional chain * use computed property. update tests * move comment down --- .../sshkey-section.component.html | 19 +- .../sshkey-section.component.spec.ts | 261 ++++++++++++++++++ .../sshkey-section.component.ts | 28 +- 3 files changed, 283 insertions(+), 25 deletions(-) create mode 100644 libs/vault/src/cipher-form/components/sshkey-section/sshkey-section.component.spec.ts diff --git a/libs/vault/src/cipher-form/components/sshkey-section/sshkey-section.component.html b/libs/vault/src/cipher-form/components/sshkey-section/sshkey-section.component.html index ec9d715ff19..419791125fb 100644 --- a/libs/vault/src/cipher-form/components/sshkey-section/sshkey-section.component.html +++ b/libs/vault/src/cipher-form/components/sshkey-section/sshkey-section.component.html @@ -15,15 +15,16 @@ data-testid="toggle-privateKey-visibility" bitPasswordInputToggle > - + @if (showImport()) { + + } diff --git a/libs/vault/src/cipher-form/components/sshkey-section/sshkey-section.component.spec.ts b/libs/vault/src/cipher-form/components/sshkey-section/sshkey-section.component.spec.ts new file mode 100644 index 00000000000..3f4a7500388 --- /dev/null +++ b/libs/vault/src/cipher-form/components/sshkey-section/sshkey-section.component.spec.ts @@ -0,0 +1,261 @@ +import { NO_ERRORS_SCHEMA } from "@angular/core"; +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { By } from "@angular/platform-browser"; +import { mock } from "jest-mock-extended"; +import { BehaviorSubject, Subject } from "rxjs"; + +import { ClientType } from "@bitwarden/common/enums"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { SdkService } from "@bitwarden/common/platform/abstractions/sdk/sdk.service"; +import { SshKeyView } from "@bitwarden/common/vault/models/view/ssh-key.view"; +import { generate_ssh_key } from "@bitwarden/sdk-internal"; + +import { SshImportPromptService } from "../../../services/ssh-import-prompt.service"; +import { CipherFormContainer } from "../../cipher-form-container"; + +import { SshKeySectionComponent } from "./sshkey-section.component"; + +jest.mock("@bitwarden/sdk-internal", () => { + return { + generate_ssh_key: jest.fn(), + }; +}); + +describe("SshKeySectionComponent", () => { + let fixture: ComponentFixture; + let component: SshKeySectionComponent; + const mockI18nService = mock(); + + let formStatusChange$: Subject; + + let cipherFormContainer: { + registerChildForm: jest.Mock; + patchCipher: jest.Mock; + getInitialCipherView: jest.Mock; + formStatusChange$: Subject; + }; + + let sdkClient$: BehaviorSubject; + let sdkService: { client$: BehaviorSubject }; + + let sshImportPromptService: { importSshKeyFromClipboard: jest.Mock }; + + let platformUtilsService: { getClientType: jest.Mock }; + + beforeEach(async () => { + formStatusChange$ = new Subject(); + + cipherFormContainer = { + registerChildForm: jest.fn(), + patchCipher: jest.fn(), + getInitialCipherView: jest.fn(), + formStatusChange$, + }; + + sdkClient$ = new BehaviorSubject({}); + sdkService = { client$: sdkClient$ }; + + sshImportPromptService = { + importSshKeyFromClipboard: jest.fn(), + }; + + platformUtilsService = { + getClientType: jest.fn(), + }; + + await TestBed.configureTestingModule({ + imports: [SshKeySectionComponent], + providers: [ + { provide: I18nService, useValue: mockI18nService }, + { provide: CipherFormContainer, useValue: cipherFormContainer }, + { provide: SdkService, useValue: sdkService }, + { provide: SshImportPromptService, useValue: sshImportPromptService }, + { provide: PlatformUtilsService, useValue: platformUtilsService }, + ], + schemas: [NO_ERRORS_SCHEMA], + }).compileComponents(); + + fixture = TestBed.createComponent(SshKeySectionComponent); + component = fixture.componentInstance; + + // minimal required inputs + fixture.componentRef.setInput("originalCipherView", { edit: true, sshKey: null }); + fixture.componentRef.setInput("disabled", false); + + (generate_ssh_key as unknown as jest.Mock).mockReset(); + }); + + it("registers the sshKeyDetails form with the container in the constructor", () => { + expect(cipherFormContainer.registerChildForm).toHaveBeenCalledTimes(1); + expect(cipherFormContainer.registerChildForm).toHaveBeenCalledWith( + "sshKeyDetails", + component.sshKeyForm, + ); + }); + + it("patches cipher sshKey whenever the form changes", () => { + component.sshKeyForm.setValue({ + privateKey: "priv", + publicKey: "pub", + keyFingerprint: "fp", + }); + + expect(cipherFormContainer.patchCipher).toHaveBeenCalledTimes(1); + const patchFn = cipherFormContainer.patchCipher.mock.calls[0][0] as (c: any) => any; + + const cipher: any = {}; + const patched = patchFn(cipher); + + expect(patched.sshKey).toBeInstanceOf(SshKeyView); + expect(patched.sshKey.privateKey).toBe("priv"); + expect(patched.sshKey.publicKey).toBe("pub"); + expect(patched.sshKey.keyFingerprint).toBe("fp"); + }); + + it("ngOnInit uses initial cipher sshKey (prefill) when present and does not generate", async () => { + cipherFormContainer.getInitialCipherView.mockReturnValue({ + sshKey: { privateKey: "p1", publicKey: "p2", keyFingerprint: "p3" }, + }); + + platformUtilsService.getClientType.mockReturnValue(ClientType.Desktop); + + await component.ngOnInit(); + + expect(generate_ssh_key).not.toHaveBeenCalled(); + expect(component.sshKeyForm.get("privateKey")?.value).toBe("p1"); + expect(component.sshKeyForm.get("publicKey")?.value).toBe("p2"); + expect(component.sshKeyForm.get("keyFingerprint")?.value).toBe("p3"); + }); + + it("ngOnInit falls back to originalCipherView sshKey when prefill is missing", async () => { + cipherFormContainer.getInitialCipherView.mockReturnValue(null); + fixture.componentRef.setInput("originalCipherView", { + edit: true, + sshKey: { privateKey: "o1", publicKey: "o2", keyFingerprint: "o3" }, + }); + + platformUtilsService.getClientType.mockReturnValue(ClientType.Desktop); + + await component.ngOnInit(); + + expect(generate_ssh_key).not.toHaveBeenCalled(); + expect(component.sshKeyForm.get("privateKey")?.value).toBe("o1"); + expect(component.sshKeyForm.get("publicKey")?.value).toBe("o2"); + expect(component.sshKeyForm.get("keyFingerprint")?.value).toBe("o3"); + }); + + it("ngOnInit generates an ssh key when no sshKey exists and populates the form", async () => { + cipherFormContainer.getInitialCipherView.mockReturnValue(null); + fixture.componentRef.setInput("originalCipherView", { edit: true, sshKey: null }); + + (generate_ssh_key as unknown as jest.Mock).mockReturnValue({ + privateKey: "genPriv", + publicKey: "genPub", + fingerprint: "genFp", + }); + + platformUtilsService.getClientType.mockReturnValue(ClientType.Desktop); + + await component.ngOnInit(); + + expect(generate_ssh_key).toHaveBeenCalledTimes(1); + expect(generate_ssh_key).toHaveBeenCalledWith("Ed25519"); + expect(component.sshKeyForm.get("privateKey")?.value).toBe("genPriv"); + expect(component.sshKeyForm.get("publicKey")?.value).toBe("genPub"); + expect(component.sshKeyForm.get("keyFingerprint")?.value).toBe("genFp"); + }); + + it("ngOnInit disables the form", async () => { + cipherFormContainer.getInitialCipherView.mockReturnValue({ + sshKey: { privateKey: "p1", publicKey: "p2", keyFingerprint: "p3" }, + }); + platformUtilsService.getClientType.mockReturnValue(ClientType.Desktop); + + await component.ngOnInit(); + + expect(component.sshKeyForm.disabled).toBe(true); + }); + + it("sets showImport true when not Web and originalCipherView.edit is true", async () => { + cipherFormContainer.getInitialCipherView.mockReturnValue({ + sshKey: { privateKey: "p1", publicKey: "p2", keyFingerprint: "p3" }, + }); + + platformUtilsService.getClientType.mockReturnValue(ClientType.Desktop); + fixture.componentRef.setInput("originalCipherView", { edit: true, sshKey: null } as any); + + await component.ngOnInit(); + + expect(component.showImport()).toBe(true); + }); + + it("keeps showImport false when client type is Web", async () => { + cipherFormContainer.getInitialCipherView.mockReturnValue({ + sshKey: { privateKey: "p1", publicKey: "p2", keyFingerprint: "p3" }, + }); + + platformUtilsService.getClientType.mockReturnValue(ClientType.Web); + fixture.componentRef.setInput("originalCipherView", { edit: true, sshKey: null } as any); + + await component.ngOnInit(); + + expect(component.showImport()).toBe(false); + }); + + it("disables the ssh key form when formStatusChange emits enabled", async () => { + cipherFormContainer.getInitialCipherView.mockReturnValue({ + sshKey: { privateKey: "p1", publicKey: "p2", keyFingerprint: "p3" }, + }); + + platformUtilsService.getClientType.mockReturnValue(ClientType.Desktop); + + await component.ngOnInit(); + + component.sshKeyForm.enable(); + expect(component.sshKeyForm.disabled).toBe(false); + + formStatusChange$.next("enabled"); + expect(component.sshKeyForm.disabled).toBe(true); + }); + + it("renders the import button only when showImport is true", async () => { + cipherFormContainer.getInitialCipherView.mockReturnValue({ + sshKey: { privateKey: "p1", publicKey: "p2", keyFingerprint: "p3" }, + }); + + platformUtilsService.getClientType.mockReturnValue(ClientType.Desktop); + fixture.componentRef.setInput("originalCipherView", { edit: true, sshKey: null } as any); + + await component.ngOnInit(); + fixture.detectChanges(); + + const importBtn = fixture.debugElement.query(By.css('[data-testid="import-privateKey"]')); + expect(importBtn).not.toBeNull(); + }); + + it("importSshKeyFromClipboard sets form values when a key is returned", async () => { + sshImportPromptService.importSshKeyFromClipboard.mockResolvedValue({ + privateKey: "cPriv", + publicKey: "cPub", + keyFingerprint: "cFp", + }); + + await component.importSshKeyFromClipboard(); + + expect(component.sshKeyForm.get("privateKey")?.value).toBe("cPriv"); + expect(component.sshKeyForm.get("publicKey")?.value).toBe("cPub"); + expect(component.sshKeyForm.get("keyFingerprint")?.value).toBe("cFp"); + }); + + it("importSshKeyFromClipboard does nothing when null is returned", async () => { + component.sshKeyForm.setValue({ privateKey: "a", publicKey: "b", keyFingerprint: "c" }); + sshImportPromptService.importSshKeyFromClipboard.mockResolvedValue(null); + + await component.importSshKeyFromClipboard(); + + expect(component.sshKeyForm.get("privateKey")?.value).toBe("a"); + expect(component.sshKeyForm.get("publicKey")?.value).toBe("b"); + expect(component.sshKeyForm.get("keyFingerprint")?.value).toBe("c"); + }); +}); diff --git a/libs/vault/src/cipher-form/components/sshkey-section/sshkey-section.component.ts b/libs/vault/src/cipher-form/components/sshkey-section/sshkey-section.component.ts index 990de9574ab..32d572cf2f3 100644 --- a/libs/vault/src/cipher-form/components/sshkey-section/sshkey-section.component.ts +++ b/libs/vault/src/cipher-form/components/sshkey-section/sshkey-section.component.ts @@ -1,7 +1,7 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore import { CommonModule } from "@angular/common"; -import { Component, DestroyRef, inject, Input, OnInit } from "@angular/core"; +import { Component, computed, DestroyRef, inject, input, OnInit } from "@angular/core"; import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { FormBuilder, ReactiveFormsModule } from "@angular/forms"; import { firstValueFrom } from "rxjs"; @@ -43,15 +43,9 @@ import { CipherFormContainer } from "../../cipher-form-container"; ], }) export class SshKeySectionComponent implements OnInit { - /** The original cipher */ - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input() originalCipherView: CipherView; + readonly originalCipherView = input(null); - /** True when all fields should be disabled */ - // FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals - // eslint-disable-next-line @angular-eslint/prefer-signals - @Input() disabled: boolean; + readonly disabled = input(false); /** * All form fields associated with the ssh key @@ -65,7 +59,14 @@ export class SshKeySectionComponent implements OnInit { keyFingerprint: [""], }); - showImport = false; + readonly showImport = computed(() => { + return ( + // Web does not support clipboard access + this.platformUtilsService.getClientType() !== ClientType.Web && + this.originalCipherView()?.edit + ); + }); + private destroyRef = inject(DestroyRef); constructor( @@ -90,7 +91,7 @@ export class SshKeySectionComponent implements OnInit { async ngOnInit() { const prefillCipher = this.cipherFormContainer.getInitialCipherView(); - const sshKeyView = prefillCipher?.sshKey ?? this.originalCipherView?.sshKey; + const sshKeyView = prefillCipher?.sshKey ?? this.originalCipherView()?.sshKey; if (sshKeyView) { this.setInitialValues(sshKeyView); @@ -100,11 +101,6 @@ export class SshKeySectionComponent implements OnInit { this.sshKeyForm.disable(); - // Web does not support clipboard access - if (this.platformUtilsService.getClientType() !== ClientType.Web) { - this.showImport = true; - } - // Disable the form if the cipher form container is enabled // to prevent user interaction this.cipherFormContainer.formStatusChange$ From 2d488c29d5ca644fb0ced380d923a3c7a5493912 Mon Sep 17 00:00:00 2001 From: neuronull <9162534+neuronull@users.noreply.github.com> Date: Tue, 6 Jan 2026 14:03:09 -0700 Subject: [PATCH 15/17] Add CI validations install and run Desktop client (MacOS) (#18096) --- .github/workflows/build-desktop.yml | 45 +++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/.github/workflows/build-desktop.yml b/.github/workflows/build-desktop.yml index 74a397b406f..914566dc496 100644 --- a/.github/workflows/build-desktop.yml +++ b/.github/workflows/build-desktop.yml @@ -2120,6 +2120,50 @@ jobs: exit 1 fi + validate-macos-dmg: + name: Validate MacOS dmg + runs-on: macos-15 + needs: + - setup + - macos-package-github + env: + _PACKAGE_VERSION: ${{ needs.setup.outputs.package_version }} + steps: + - name: Check out repo + uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 + with: + fetch-depth: 1 + ref: ${{ github.event.workflow_run.head_sha }} + persist-credentials: false + + - name: Download dmg artifact + uses: bitwarden/gh-actions/download-artifacts@main + with: + path: apps/desktop/artifacts/macos/dmg + artifacts: Bitwarden-${{ env._PACKAGE_VERSION }}-universal.dmg + + - name: Install dmg + working-directory: apps/desktop/artifacts/macos/dmg + run: | + # mount + hdiutil attach Bitwarden-${_PACKAGE_VERSION}-universal.dmg + # install + cp -r /Volumes/Bitwarden\ ${_PACKAGE_VERSION}-universal/Bitwarden.app /Applications/ + # unmount + hdiutil detach /Volumes/Bitwarden\ ${_PACKAGE_VERSION}-universal + + - name: Run dmg + run: | + open /Applications/Bitwarden.app/Contents/MacOS/Bitwarden & + sleep 30 + if pgrep Bitwarden > /dev/null; then + pkill -9 Bitwarden + echo "Bitwarden is running." + else + echo "Bitwarden is not running." + exit 1 + fi + check-failures: name: Check for failures if: always() @@ -2139,6 +2183,7 @@ jobs: - validate-linux-flatpak - validate-linux-snap - validate-linux-wayland + - validate-macos-dmg permissions: contents: read id-token: write From a4b5192bd87ad2348cab06fdda27c639b5084224 Mon Sep 17 00:00:00 2001 From: Jason Ng Date: Tue, 6 Jan 2026 16:34:52 -0500 Subject: [PATCH 16/17] [PM-26516] Archive Vault Updates Non Premium (#18068) * add callout to vault-items for non premium users, add upgrade premium flow * add archive badge to item details only for desktop * update desktop edit item save for unarchive * updated success toast for edited archive item non premium --- apps/browser/src/_locales/en/messages.json | 3 + apps/desktop/src/locales/en/messages.json | 22 +++- .../app/vault/item-footer.component.html | 4 +- .../vault/app/vault/item-footer.component.ts | 3 + .../app/vault/vault-items-v2.component.html | 15 +++ .../app/vault/vault-items-v2.component.ts | 17 ++- .../vault/app/vault/vault-v2.component.html | 11 +- .../src/vault/app/vault/vault-v2.component.ts | 101 +++++++++++----- .../vault-item-dialog.component.html | 2 +- apps/web/src/locales/en/messages.json | 6 + .../default-cipher-archive.service.spec.ts | 1 + .../default-cipher-archive.service.ts | 11 +- .../components/cipher-form.component.ts | 4 +- .../item-details-section.component.html | 3 + .../item-details-section.component.spec.ts | 114 ++++++++++++------ .../item-details-section.component.ts | 30 +++-- .../item-details-v2.component.html | 11 +- .../item-details-v2.component.spec.ts | 32 +++++ .../item-details/item-details-v2.component.ts | 15 ++- 19 files changed, 316 insertions(+), 89 deletions(-) diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index d70025016ca..ca9dde99a95 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -585,6 +585,9 @@ "upgradeToUseArchive": { "message": "A premium membership is required to use Archive." }, + "itemRestored": { + "message": "Item has been restored" + }, "edit": { "message": "Edit" }, diff --git a/apps/desktop/src/locales/en/messages.json b/apps/desktop/src/locales/en/messages.json index 3ace5308d27..c47817f3ee4 100644 --- a/apps/desktop/src/locales/en/messages.json +++ b/apps/desktop/src/locales/en/messages.json @@ -4306,6 +4306,9 @@ "unArchive": { "message": "Unarchive" }, + "archived": { + "message": "Archived" + }, "itemsInArchive": { "message": "Items in archive" }, @@ -4327,6 +4330,21 @@ "archiveItemConfirmDesc": { "message": "Archived items are excluded from general search results and autofill suggestions. Are you sure you want to archive this item?" }, + "unArchiveAndSave": { + "message": "Unarchive and save" + }, + "restartPremium": { + "message": "Restart Premium" + }, + "premiumSubscriptionEnded": { + "message": "Your Premium subscription ended" + }, + "premiumSubscriptionEndedDesc": { + "message": "To regain access to your archive, restart your Premium subscription. If you edit details for an archived item before restarting, it’ll be moved back into your vault." + }, + "itemRestored": { + "message": "Item has been restored" + }, "zipPostalCodeLabel": { "message": "ZIP / Postal code" }, @@ -4475,7 +4493,7 @@ "placeholders": { "organization": { "content": "$1", - "example": "My Org Name" + "example": "My Org Name" } } }, @@ -4484,7 +4502,7 @@ "placeholders": { "organization": { "content": "$1", - "example": "My Org Name" + "example": "My Org Name" } } }, diff --git a/apps/desktop/src/vault/app/vault/item-footer.component.html b/apps/desktop/src/vault/app/vault/item-footer.component.html index 9d5b1e5e560..a03f3e96b06 100644 --- a/apps/desktop/src/vault/app/vault/item-footer.component.html +++ b/apps/desktop/src/vault/app/vault/item-footer.component.html @@ -7,9 +7,9 @@ [hidden]="action === 'view'" bitButton class="primary" - appA11yTitle="{{ 'save' | i18n }}" + appA11yTitle="{{ submitButtonText() }}" > - {{ "save" | i18n }} + {{ submitButtonText() }}
+ @if (showPremiumCallout()) { +
+ + +
+ {{ "premiumSubscriptionEndedDesc" | i18n }} +
+ + {{ "restartPremium" | i18n }} + + + +
+ } +
extends BaseVaultItemsComponent { + readonly showPremiumCallout = input(false); + readonly organizationId = input(undefined); + protected CipherViewLikeUtils = CipherViewLikeUtils; + constructor( searchService: SearchService, private readonly searchBarService: SearchBarService, @@ -37,6 +43,7 @@ export class VaultItemsV2Component extends BaseVaultIt accountService: AccountService, restrictedItemTypesService: RestrictedItemTypesService, configService: ConfigService, + private premiumUpgradePromptService: PremiumUpgradePromptService, ) { super(searchService, cipherService, accountService, restrictedItemTypesService, configService); @@ -47,6 +54,10 @@ export class VaultItemsV2Component extends BaseVaultIt }); } + async navigateToGetPremium() { + await this.premiumUpgradePromptService.promptForPremium(this.organizationId()); + } + trackByFn(index: number, c: C): string { return uuidAsString(c.id!); } diff --git a/apps/desktop/src/vault/app/vault/vault-v2.component.html b/apps/desktop/src/vault/app/vault/vault-v2.component.html index 2696dd0d452..d10b3fd85c6 100644 --- a/apps/desktop/src/vault/app/vault/vault-v2.component.html +++ b/apps/desktop/src/vault/app/vault/vault-v2.component.html @@ -6,13 +6,15 @@ (onCipherClicked)="viewCipher($event)" (onCipherRightClicked)="viewCipherMenu($event)" (onAddCipher)="addCipher($event)" + [showPremiumCallout]="showPremiumCallout$ | async" + [organizationId]="organizationId" >
- + type: CipherType | null = null; folderId: string | null = null; collectionId: string | null = null; - organizationId: string | null = null; + organizationId: OrganizationId | null = null; myVaultOnly = false; addType: CipherType | undefined = undefined; addOrganizationId: string | null = null; @@ -172,11 +183,25 @@ export class VaultV2Component deleted = false; userHasPremiumAccess = false; activeFilter: VaultFilter = new VaultFilter(); + private activeFilterSubject = new BehaviorSubject(new VaultFilter()); + private activeFilter$ = this.activeFilterSubject.asObservable(); + private userId$ = this.accountService.activeAccount$.pipe(getUserId); + showPremiumCallout$ = this.userId$.pipe( + switchMap((userId) => + combineLatest([ + this.activeFilter$, + this.cipherArchiveService.showSubscriptionEndedMessaging$(userId), + ]).pipe( + map(([activeFilter, showMessaging]) => activeFilter.status === "archive" && showMessaging), + ), + ), + ); activeUserId: UserId | null = null; cipherRepromptId: string | null = null; - cipher: CipherView | null = new CipherView(); + readonly cipher = signal(null); collections: CollectionView[] | null = null; config: CipherFormConfig | null = null; + readonly userHasPremium = signal(false); /** Tracks the disabled status of the edit cipher form */ protected formDisabled: boolean = false; @@ -187,12 +212,13 @@ export class VaultV2Component switchMap((id) => this.organizationService.organizations$(id)), ); - protected canAccessAttachments$ = this.accountService.activeAccount$.pipe( - filter((account): account is Account => !!account), - switchMap((account) => - this.billingAccountProfileStateService.hasPremiumFromAnySource$(account.id), - ), - ); + protected readonly submitButtonText = computed(() => { + return this.cipher()?.isArchived && + !this.userHasPremium() && + this.cipherArchiveService.hasArchiveFlagEnabled$ + ? this.i18nService.t("unArchiveAndSave") + : this.i18nService.t("save"); + }); private componentIsDestroyed$ = new Subject(); private allOrganizations: Organization[] = []; @@ -241,6 +267,7 @@ export class VaultV2Component ) .subscribe((canAccessPremium: boolean) => { this.userHasPremiumAccess = canAccessPremium; + this.userHasPremium.set(canAccessPremium); }); this.broadcasterService.subscribe(BroadcasterSubscriptionId, (message: any) => { @@ -288,30 +315,40 @@ export class VaultV2Component this.showingModal = false; break; case "copyUsername": { - if (this.cipher?.login?.username) { - this.copyValue(this.cipher, this.cipher?.login?.username, "username", "Username"); + if (this.cipher()?.login?.username) { + this.copyValue( + this.cipher(), + this.cipher()?.login?.username, + "username", + "Username", + ); } break; } case "copyPassword": { - if (this.cipher?.login?.password && this.cipher.viewPassword) { - this.copyValue(this.cipher, this.cipher.login.password, "password", "Password"); + if (this.cipher()?.login?.password && this.cipher().viewPassword) { + this.copyValue( + this.cipher(), + this.cipher().login.password, + "password", + "Password", + ); await this.eventCollectionService - .collect(EventType.Cipher_ClientCopiedPassword, this.cipher.id) + .collect(EventType.Cipher_ClientCopiedPassword, this.cipher().id) .catch(() => {}); } break; } case "copyTotp": { if ( - this.cipher?.login?.hasTotp && - (this.cipher.organizationUseTotp || this.userHasPremiumAccess) + this.cipher()?.login?.hasTotp && + (this.cipher()?.organizationUseTotp || this.userHasPremiumAccess) ) { const value = await firstValueFrom( - this.totpService.getCode$(this.cipher.login.totp), + this.totpService.getCode$(this.cipher()?.login.totp), ).catch((): any => null); if (value) { - this.copyValue(this.cipher, value.code, "verificationCodeTotp", "TOTP"); + this.copyValue(this.cipher(), value.code, "verificationCodeTotp", "TOTP"); } } break; @@ -416,6 +453,7 @@ export class VaultV2Component selectedOrganizationId: params.selectedOrganizationId, myVaultOnly: params.myVaultOnly ?? false, }); + this.activeFilterSubject.next(this.activeFilter); if (this.vaultItemsComponent) { await this.vaultItemsComponent.reload(this.activeFilter.buildFilter()).catch(() => {}); } @@ -440,7 +478,7 @@ export class VaultV2Component return; } this.cipherId = cipher.id; - this.cipher = cipher; + this.cipher.set(cipher); this.collections = this.vaultFilterComponent?.collections?.fullList.filter((c) => cipher.collectionIds.includes(c.id), @@ -679,7 +717,7 @@ export class VaultV2Component return; } this.cipherId = cipher.id; - this.cipher = cipher; + this.cipher.set(cipher); await this.buildFormConfig("edit"); if (!cipher.edit && this.config) { this.config.mode = "partial-edit"; @@ -693,7 +731,7 @@ export class VaultV2Component return; } this.cipherId = cipher.id; - this.cipher = cipher; + this.cipher.set(cipher); await this.buildFormConfig("clone"); this.action = "clone"; await this.go().catch(() => {}); @@ -742,7 +780,7 @@ export class VaultV2Component return; } this.addType = type || this.activeFilter.cipherType; - this.cipher = new CipherView(); + this.cipher.set(new CipherView()); this.cipherId = null; await this.buildFormConfig("add"); this.action = "add"; @@ -774,14 +812,14 @@ export class VaultV2Component ); this.cipherId = cipher.id; - this.cipher = cipher; + this.cipher.set(cipher); await this.go().catch(() => {}); await this.vaultItemsComponent?.refresh().catch(() => {}); } async deleteCipher() { this.cipherId = null; - this.cipher = null; + this.cipher.set(null); this.action = null; await this.go().catch(() => {}); await this.vaultItemsComponent?.refresh().catch(() => {}); @@ -796,7 +834,7 @@ export class VaultV2Component async cancelCipher(cipher: CipherView) { this.cipherId = cipher.id; - this.cipher = cipher; + this.cipher.set(cipher); this.action = this.cipherId ? "view" : null; await this.go().catch(() => {}); } @@ -806,6 +844,7 @@ export class VaultV2Component this.i18nService.t(this.calculateSearchBarLocalizationString(vaultFilter)), ); this.activeFilter = vaultFilter; + this.activeFilterSubject.next(vaultFilter); await this.vaultItemsComponent ?.reload( this.activeFilter.buildFilter(), @@ -887,14 +926,16 @@ export class VaultV2Component /** Refresh the current cipher object */ protected async refreshCurrentCipher() { - if (!this.cipher) { + if (!this.cipher()) { return; } - this.cipher = await firstValueFrom( - this.cipherService.cipherViews$(this.activeUserId!).pipe( - filter((c) => !!c), - map((ciphers) => ciphers.find((c) => c.id === this.cipherId) ?? null), + this.cipher.set( + await firstValueFrom( + this.cipherService.cipherViews$(this.activeUserId!).pipe( + filter((c) => !!c), + map((ciphers) => ciphers.find((c) => c.id === this.cipherId) ?? null), + ), ), ); } diff --git a/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.html b/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.html index 3aa2f4b3bc1..16256ab875a 100644 --- a/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.html +++ b/apps/web/src/app/vault/components/vault-item-dialog/vault-item-dialog.component.html @@ -3,7 +3,7 @@ {{ title }} @if (cipherIsArchived) { - {{ "archiveNoun" | i18n }} + {{ "archived" | i18n }} }
diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index 98f847e1d36..57ea900fa69 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -3146,6 +3146,9 @@ "premiumSubscriptionEndedDesc": { "message": "To regain access to your archive, restart your Premium subscription. If you edit details for an archived item before restarting, it'll be moved back into your vault." }, + "itemRestored": { + "message": "Item has been restored" + }, "restartPremium": { "message": "Restart Premium" }, @@ -11613,6 +11616,9 @@ "unArchive": { "message": "Unarchive" }, + "archived": { + "message": "Archived" + }, "unArchiveAndSave": { "message": "Unarchive and save" }, diff --git a/libs/common/src/vault/services/default-cipher-archive.service.spec.ts b/libs/common/src/vault/services/default-cipher-archive.service.spec.ts index 2f5e69d65ed..2078d1f29ea 100644 --- a/libs/common/src/vault/services/default-cipher-archive.service.spec.ts +++ b/libs/common/src/vault/services/default-cipher-archive.service.spec.ts @@ -165,6 +165,7 @@ describe("DefaultCipherArchiveService", () => { mockCipherService.cipherListViews$.mockReturnValue(of(mockCiphers)); mockBillingAccountProfileStateService.hasPremiumFromAnySource$.mockReturnValue(of(false)); + featureFlag.next(true); const result = await firstValueFrom(service.showSubscriptionEndedMessaging$(userId)); diff --git a/libs/common/src/vault/services/default-cipher-archive.service.ts b/libs/common/src/vault/services/default-cipher-archive.service.ts index c1daade0dad..12d67ab07f9 100644 --- a/libs/common/src/vault/services/default-cipher-archive.service.ts +++ b/libs/common/src/vault/services/default-cipher-archive.service.ts @@ -71,8 +71,15 @@ export class DefaultCipherArchiveService implements CipherArchiveService { /** Returns true when the user has previously archived ciphers but lost their premium membership. */ showSubscriptionEndedMessaging$(userId: UserId): Observable { - return combineLatest([this.archivedCiphers$(userId), this.userHasPremium$(userId)]).pipe( - map(([archivedCiphers, hasPremium]) => archivedCiphers.length > 0 && !hasPremium), + return combineLatest([ + this.archivedCiphers$(userId), + this.userHasPremium$(userId), + this.hasArchiveFlagEnabled$, + ]).pipe( + map( + ([archivedCiphers, hasPremium, flagEnabled]) => + flagEnabled && archivedCiphers.length > 0 && !hasPremium, + ), shareReplay({ refCount: true, bufferSize: 1 }), ); } diff --git a/libs/vault/src/cipher-form/components/cipher-form.component.ts b/libs/vault/src/cipher-form/components/cipher-form.component.ts index c9e867f8d3a..c0df34fadb2 100644 --- a/libs/vault/src/cipher-form/components/cipher-form.component.ts +++ b/libs/vault/src/cipher-form/components/cipher-form.component.ts @@ -358,6 +358,7 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci } submit = async () => { + let successToast: string = "editedItem"; if (this.cipherForm.invalid) { this.cipherForm.markAllAsTouched(); @@ -392,6 +393,7 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci // If the item is archived but user has lost archive permissions, unarchive the item. if (!userCanArchive && this.updatedCipherView.archivedDate) { this.updatedCipherView.archivedDate = null; + successToast = "itemRestored"; } const savedCipher = await this.addEditFormService.saveCipher( @@ -407,7 +409,7 @@ export class CipherFormComponent implements AfterViewInit, OnInit, OnChanges, Ci title: null, message: this.i18nService.t( this.config.mode === "edit" || this.config.mode === "partial-edit" - ? "editedItem" + ? successToast : "addedItem", ), }); diff --git a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html index 9bf6dc32758..6e6bd70ec3f 100644 --- a/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html +++ b/libs/vault/src/cipher-form/components/item-details/item-details-section.component.html @@ -1,6 +1,9 @@

{{ "itemDetails" | i18n }}

+ @if (showArchiveBadge()) { + {{ "archived" | i18n }} + }
diff --git a/libs/vault/src/cipher-view/item-details/item-details-v2.component.spec.ts b/libs/vault/src/cipher-view/item-details/item-details-v2.component.spec.ts index ead2979fac7..ae78c49bdb4 100644 --- a/libs/vault/src/cipher-view/item-details/item-details-v2.component.spec.ts +++ b/libs/vault/src/cipher-view/item-details/item-details-v2.component.spec.ts @@ -1,6 +1,7 @@ import { ComponentRef } from "@angular/core"; import { ComponentFixture, TestBed } from "@angular/core/testing"; import { By } from "@angular/platform-browser"; +import { mock, MockProxy } from "jest-mock-extended"; import { of } from "rxjs"; // This import has been flagged as unallowed for this class. It may be involved in a circular dependency loop. @@ -21,6 +22,7 @@ describe("ItemDetailsV2Component", () => { let component: ItemDetailsV2Component; let fixture: ComponentFixture; let componentRef: ComponentRef; + let mockPlatformUtilsService: MockProxy; const cipher = { id: "cipher1", @@ -51,6 +53,8 @@ describe("ItemDetailsV2Component", () => { } as FolderView; beforeEach(async () => { + mockPlatformUtilsService = mock(); + await TestBed.configureTestingModule({ imports: [ItemDetailsV2Component], providers: [ @@ -61,6 +65,7 @@ describe("ItemDetailsV2Component", () => { useValue: { environment$: of({ getIconsUrl: () => "https://icons.example.com" }) }, }, { provide: DomainSettingsService, useValue: { showFavicons$: of(true) } }, + { provide: PlatformUtilsService, useValue: mockPlatformUtilsService }, ], }).compileComponents(); }); @@ -98,4 +103,31 @@ describe("ItemDetailsV2Component", () => { const owner = fixture.debugElement.query(By.css('[data-testid="owner"]')); expect(owner).toBeNull(); }); + + it("should show archive badge when cipher is archived and client is Desktop", () => { + jest.spyOn(mockPlatformUtilsService, "getClientType").mockReturnValue(ClientType.Desktop); + + const archivedCipher = { ...cipher, isArchived: true }; + componentRef.setInput("cipher", archivedCipher); + + expect((component as any).showArchiveBadge()).toBe(true); + }); + + it("should not show archive badge when cipher is not archived", () => { + jest.spyOn(mockPlatformUtilsService, "getClientType").mockReturnValue(ClientType.Desktop); + + const unarchivedCipher = { ...cipher, isArchived: false }; + componentRef.setInput("cipher", unarchivedCipher); + + expect((component as any).showArchiveBadge()).toBe(false); + }); + + it("should not show archive badge when client is not Desktop", () => { + jest.spyOn(mockPlatformUtilsService, "getClientType").mockReturnValue(ClientType.Web); + + const archivedCipher = { ...cipher, isArchived: true }; + componentRef.setInput("cipher", archivedCipher); + + expect((component as any).showArchiveBadge()).toBe(false); + }); }); diff --git a/libs/vault/src/cipher-view/item-details/item-details-v2.component.ts b/libs/vault/src/cipher-view/item-details/item-details-v2.component.ts index 2c310daad76..8132780ccf4 100644 --- a/libs/vault/src/cipher-view/item-details/item-details-v2.component.ts +++ b/libs/vault/src/cipher-view/item-details/item-details-v2.component.ts @@ -9,11 +9,14 @@ import { fromEvent, map, startWith } from "rxjs"; // eslint-disable-next-line no-restricted-imports import { CollectionTypes, CollectionView } from "@bitwarden/admin-console/common"; import { JslibModule } from "@bitwarden/angular/jslib.module"; +import { ClientType } from "@bitwarden/client-type"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { FolderView } from "@bitwarden/common/vault/models/view/folder.view"; import { + BadgeModule, ButtonLinkDirective, CardComponent, FormFieldModule, @@ -35,6 +38,7 @@ import { OrgIconDirective } from "../../components/org-icon.directive"; OrgIconDirective, FormFieldModule, ButtonLinkDirective, + BadgeModule, ], }) export class ItemDetailsV2Component { @@ -85,7 +89,16 @@ export class ItemDetailsV2Component { } }); - constructor(private i18nService: I18nService) {} + protected readonly showArchiveBadge = computed(() => { + return ( + this.cipher().isArchived && this.platformUtilsService.getClientType() === ClientType.Desktop + ); + }); + + constructor( + private i18nService: I18nService, + private platformUtilsService: PlatformUtilsService, + ) {} toggleShowMore() { this.showAllDetails.update((value) => !value); From d6b23670aadc698048f5da9f408ab451e44152f9 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Tue, 6 Jan 2026 13:48:07 -0800 Subject: [PATCH 17/17] feat(auth-request-answering): [Auth / PM-26209] Use AuthRequestAnsweringService on Desktop (#16906) Update Desktop to use the AuthRequestAnsweringService, bringing it into feature parity with the Extension. --- ...ion-auth-request-answering.service.spec.ts | 242 ++++++++++ ...xtension-auth-request-answering.service.ts | 116 +++++ .../browser/src/background/main.background.ts | 13 +- apps/browser/src/popup/app.component.ts | 128 +++-- .../src/popup/services/services.module.ts | 13 +- apps/desktop/src/app/app.component.ts | 75 ++- .../src/app/services/services.module.ts | 28 +- ...-approval-dialog-component.service.spec.ts | 91 ---- ...login-approval-dialog-component.service.ts | 28 -- ...top-auth-request-answering.service.spec.ts | 277 +++++++++++ .../desktop-auth-request-answering.service.ts | 85 ++++ .../src/vault/app/vault/vault-v2.component.ts | 41 +- apps/web/src/app/core/core.module.ts | 7 + ...-approval-dialog-component.service.spec.ts | 30 -- ...login-approval-dialog-component.service.ts | 14 - libs/angular/src/auth/login-approval/index.ts | 2 - ...al-dialog-component.service.abstraction.ts | 9 - .../login-approval-dialog.component.spec.ts | 5 - .../login-approval-dialog.component.ts | 7 - .../src/services/jslib-services.module.ts | 25 +- .../auth-request-answering/README.md | 3 +- ...h-request-answering.service.abstraction.ts | 44 +- .../auth-request-answering.service.spec.ts | 142 ------ .../auth-request-answering.service.ts | 111 ----- ...ult-auth-request-answering.service.spec.ts | 444 ++++++++++++++++++ .../default-auth-request-answering.service.ts | 140 ++++++ .../noop-auth-request-answering.service.ts | 22 +- ...ult-server-notifications.multiuser.spec.ts | 12 +- ...fault-server-notifications.service.spec.ts | 42 +- .../default-server-notifications.service.ts | 44 +- 30 files changed, 1630 insertions(+), 610 deletions(-) create mode 100644 apps/browser/src/auth/services/auth-request-answering/extension-auth-request-answering.service.spec.ts create mode 100644 apps/browser/src/auth/services/auth-request-answering/extension-auth-request-answering.service.ts delete mode 100644 apps/desktop/src/auth/login/desktop-login-approval-dialog-component.service.spec.ts delete mode 100644 apps/desktop/src/auth/login/desktop-login-approval-dialog-component.service.ts create mode 100644 apps/desktop/src/auth/services/auth-request-answering/desktop-auth-request-answering.service.spec.ts create mode 100644 apps/desktop/src/auth/services/auth-request-answering/desktop-auth-request-answering.service.ts delete mode 100644 libs/angular/src/auth/login-approval/default-login-approval-dialog-component.service.spec.ts delete mode 100644 libs/angular/src/auth/login-approval/default-login-approval-dialog-component.service.ts delete mode 100644 libs/angular/src/auth/login-approval/login-approval-dialog-component.service.abstraction.ts delete mode 100644 libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.spec.ts delete mode 100644 libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.ts create mode 100644 libs/common/src/auth/services/auth-request-answering/default-auth-request-answering.service.spec.ts create mode 100644 libs/common/src/auth/services/auth-request-answering/default-auth-request-answering.service.ts diff --git a/apps/browser/src/auth/services/auth-request-answering/extension-auth-request-answering.service.spec.ts b/apps/browser/src/auth/services/auth-request-answering/extension-auth-request-answering.service.spec.ts new file mode 100644 index 00000000000..4817576a8e0 --- /dev/null +++ b/apps/browser/src/auth/services/auth-request-answering/extension-auth-request-answering.service.spec.ts @@ -0,0 +1,242 @@ +import { mock, MockProxy } from "jest-mock-extended"; +import { of } from "rxjs"; + +import { Account, AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { AuthServerNotificationTags } from "@bitwarden/common/auth/enums/auth-server-notification-tags"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; +import { PendingAuthRequestsStateService } from "@bitwarden/common/auth/services/auth-request-answering/pending-auth-requests.state"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { ActionsService } from "@bitwarden/common/platform/actions"; +import { + ButtonLocation, + SystemNotificationEvent, + SystemNotificationsService, +} from "@bitwarden/common/platform/system-notifications/system-notifications.service"; +import { mockAccountInfoWith } from "@bitwarden/common/spec"; +import { LogService } from "@bitwarden/logging"; +import { UserId } from "@bitwarden/user-core"; + +import { ExtensionAuthRequestAnsweringService } from "./extension-auth-request-answering.service"; + +describe("ExtensionAuthRequestAnsweringService", () => { + let accountService: MockProxy; + let authService: MockProxy; + let masterPasswordService: any; // MasterPasswordServiceAbstraction has many members; we only use forceSetPasswordReason$ + let messagingService: MockProxy; + let pendingAuthRequestsState: MockProxy; + let actionService: MockProxy; + let i18nService: MockProxy; + let platformUtilsService: MockProxy; + let systemNotificationsService: MockProxy; + let logService: MockProxy; + + let sut: AuthRequestAnsweringService; + + const userId = "9f4c3452-6a45-48af-a7d0-74d3e8b65e4c" as UserId; + const userAccountInfo = mockAccountInfoWith({ + name: "User", + email: "user@example.com", + }); + const userAccount: Account = { + id: userId, + ...userAccountInfo, + }; + + const authRequestId = "auth-request-id-123"; + + beforeEach(() => { + accountService = mock(); + authService = mock(); + masterPasswordService = { + forceSetPasswordReason$: jest.fn().mockReturnValue(of(ForceSetPasswordReason.None)), + }; + messagingService = mock(); + pendingAuthRequestsState = mock(); + actionService = mock(); + i18nService = mock(); + platformUtilsService = mock(); + systemNotificationsService = mock(); + logService = mock(); + + // Common defaults + authService.activeAccountStatus$ = of(AuthenticationStatus.Locked); + accountService.activeAccount$ = of(userAccount); + accountService.accounts$ = of({ + [userId]: userAccountInfo, + }); + platformUtilsService.isPopupOpen.mockResolvedValue(false); + i18nService.t.mockImplementation( + (key: string, p1?: any) => `${key}${p1 != null ? ":" + p1 : ""}`, + ); + systemNotificationsService.create.mockResolvedValue("notif-id"); + + sut = new ExtensionAuthRequestAnsweringService( + accountService, + authService, + masterPasswordService, + messagingService, + pendingAuthRequestsState, + actionService, + i18nService, + platformUtilsService, + systemNotificationsService, + logService, + ); + }); + + describe("receivedPendingAuthRequest()", () => { + it("should throw if authRequestUserId not given", async () => { + // Act + const promise = sut.receivedPendingAuthRequest(undefined, authRequestId); + + // Assert + await expect(promise).rejects.toThrow("authRequestUserId required"); + }); + + it("should throw if authRequestId not given", async () => { + // Act + const promise = sut.receivedPendingAuthRequest(userId, undefined); + + // Assert + await expect(promise).rejects.toThrow("authRequestId required"); + }); + + it("should add a pending marker for the user to state", async () => { + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect(pendingAuthRequestsState.add).toHaveBeenCalledTimes(1); + expect(pendingAuthRequestsState.add).toHaveBeenCalledWith(userId); + }); + + describe("given the active user is the intended recipient of the auth request, unlocked, and not required to set/change their master password", () => { + describe("given the popup is open", () => { + it("should send an 'openLoginApproval' message", async () => { + // Arrange + platformUtilsService.isPopupOpen.mockResolvedValue(true); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect(messagingService.send).toHaveBeenCalledTimes(1); + expect(messagingService.send).toHaveBeenCalledWith("openLoginApproval", { + notificationId: authRequestId, + }); + }); + + it("should not create a system notification", async () => { + // Arrange + platformUtilsService.isPopupOpen.mockResolvedValue(true); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect(systemNotificationsService.create).not.toHaveBeenCalled(); + }); + }); + + describe("given the popup is closed", () => { + it("should not send an 'openLoginApproval' message", async () => { + // Arrange + platformUtilsService.isPopupOpen.mockResolvedValue(false); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect(messagingService.send).not.toHaveBeenCalled(); + }); + + it("should create a system notification", async () => { + // Arrange + platformUtilsService.isPopupOpen.mockResolvedValue(false); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect(i18nService.t).toHaveBeenCalledWith("accountAccessRequested"); + expect(i18nService.t).toHaveBeenCalledWith("confirmAccessAttempt", "user@example.com"); + expect(systemNotificationsService.create).toHaveBeenCalledWith({ + id: `${AuthServerNotificationTags.AuthRequest}_${authRequestId}`, + title: "accountAccessRequested", + body: "confirmAccessAttempt:user@example.com", + buttons: [], + }); + }); + }); + }); + }); + + describe("activeUserMeetsConditionsToShowApprovalDialog()", () => { + describe("given the active user is the intended recipient of the auth request, unlocked, and not required to set/change their master password", () => { + it("should return true if popup is open", async () => { + // Arrange + platformUtilsService.isPopupOpen.mockResolvedValue(true); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + const result = await sut.activeUserMeetsConditionsToShowApprovalDialog(userId); + + // Assert + expect(result).toBe(true); + }); + + it("should return false if popup is closed", async () => { + // Arrange + platformUtilsService.isPopupOpen.mockResolvedValue(false); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + const result = await sut.activeUserMeetsConditionsToShowApprovalDialog(userId); + + // Assert + expect(result).toBe(false); + }); + }); + }); + + describe("handleAuthRequestNotificationClicked()", () => { + it("should clear notification and open popup when notification body is clicked", async () => { + // Arrange + const event: SystemNotificationEvent = { + id: "123", + buttonIdentifier: ButtonLocation.NotificationButton, + }; + + // Act + await sut.handleAuthRequestNotificationClicked(event); + + // Assert + expect(systemNotificationsService.clear).toHaveBeenCalledWith({ id: "123" }); + expect(actionService.openPopup).toHaveBeenCalledTimes(1); + }); + + it("should do nothing when an optional notification button is clicked", async () => { + // Arrange + const event: SystemNotificationEvent = { + id: "123", + buttonIdentifier: ButtonLocation.FirstOptionalButton, + }; + + // Act + await sut.handleAuthRequestNotificationClicked(event); + + // Assert + expect(systemNotificationsService.clear).not.toHaveBeenCalled(); + expect(actionService.openPopup).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/apps/browser/src/auth/services/auth-request-answering/extension-auth-request-answering.service.ts b/apps/browser/src/auth/services/auth-request-answering/extension-auth-request-answering.service.ts new file mode 100644 index 00000000000..988de685978 --- /dev/null +++ b/apps/browser/src/auth/services/auth-request-answering/extension-auth-request-answering.service.ts @@ -0,0 +1,116 @@ +import { firstValueFrom } from "rxjs"; + +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { AuthServerNotificationTags } from "@bitwarden/common/auth/enums/auth-server-notification-tags"; +import { DefaultAuthRequestAnsweringService } from "@bitwarden/common/auth/services/auth-request-answering/default-auth-request-answering.service"; +import { PendingAuthRequestsStateService } from "@bitwarden/common/auth/services/auth-request-answering/pending-auth-requests.state"; +import { MasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { ActionsService } from "@bitwarden/common/platform/actions"; +import { + ButtonLocation, + SystemNotificationEvent, + SystemNotificationsService, +} from "@bitwarden/common/platform/system-notifications/system-notifications.service"; +import { LogService } from "@bitwarden/logging"; +import { UserId } from "@bitwarden/user-core"; + +export class ExtensionAuthRequestAnsweringService + extends DefaultAuthRequestAnsweringService + implements AuthRequestAnsweringService +{ + constructor( + protected readonly accountService: AccountService, + protected readonly authService: AuthService, + protected readonly masterPasswordService: MasterPasswordServiceAbstraction, + protected readonly messagingService: MessagingService, + protected readonly pendingAuthRequestsState: PendingAuthRequestsStateService, + private readonly actionService: ActionsService, + private readonly i18nService: I18nService, + private readonly platformUtilsService: PlatformUtilsService, + private readonly systemNotificationsService: SystemNotificationsService, + private readonly logService: LogService, + ) { + super( + accountService, + authService, + masterPasswordService, + messagingService, + pendingAuthRequestsState, + ); + } + + async receivedPendingAuthRequest( + authRequestUserId: UserId, + authRequestId: string, + ): Promise { + if (!authRequestUserId) { + throw new Error("authRequestUserId required"); + } + if (!authRequestId) { + throw new Error("authRequestId required"); + } + + // Always persist the pending marker for this user to global state. + await this.pendingAuthRequestsState.add(authRequestUserId); + + const activeUserMeetsConditionsToShowApprovalDialog = + await this.activeUserMeetsConditionsToShowApprovalDialog(authRequestUserId); + + if (activeUserMeetsConditionsToShowApprovalDialog) { + // Send message to open dialog immediately for this request + this.messagingService.send("openLoginApproval", { + // Include the authRequestId so the DeviceManagementComponent can upsert the correct device. + // This will only matter if the user is on the /device-management screen when the auth request is received. + notificationId: authRequestId, + }); + } else { + // Create a system notification + const accounts = await firstValueFrom(this.accountService.accounts$); + const accountInfo = accounts[authRequestUserId]; + + if (!accountInfo) { + this.logService.error("Account not found for authRequestUserId"); + return; + } + + const emailForUser = accountInfo.email; + await this.systemNotificationsService.create({ + id: `${AuthServerNotificationTags.AuthRequest}_${authRequestId}`, // the underscore is an important delimiter. + title: this.i18nService.t("accountAccessRequested"), + body: this.i18nService.t("confirmAccessAttempt", emailForUser), + buttons: [], + }); + } + } + + async activeUserMeetsConditionsToShowApprovalDialog(authRequestUserId: UserId): Promise { + const meetsBasicConditions = await super.activeUserMeetsConditionsToShowApprovalDialog( + authRequestUserId, + ); + + // To show an approval dialog immediately on Extension, the popup must be open. + const isPopupOpen = await this.platformUtilsService.isPopupOpen(); + const meetsExtensionConditions = meetsBasicConditions && isPopupOpen; + + return meetsExtensionConditions; + } + + /** + * When a system notification is clicked, this function is used to process that event. + * + * @param event The event passed in. Check initNotificationSubscriptions in main.background.ts. + */ + async handleAuthRequestNotificationClicked(event: SystemNotificationEvent): Promise { + if (event.buttonIdentifier === ButtonLocation.NotificationButton) { + await this.systemNotificationsService.clear({ + id: `${event.id}`, + }); + await this.actionService.openPopup(); + } + } +} diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 3cd8b59aabc..b9b41943b04 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -40,7 +40,7 @@ import { DefaultPolicyService } from "@bitwarden/common/admin-console/services/p import { PolicyApiService } from "@bitwarden/common/admin-console/services/policy/policy-api.service"; import { ProviderService } from "@bitwarden/common/admin-console/services/provider.service"; import { AccountService as AccountServiceAbstraction } from "@bitwarden/common/auth/abstractions/account.service"; -import { AuthRequestAnsweringServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; +import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; import { AuthService as AuthServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth.service"; import { AvatarService as AvatarServiceAbstraction } from "@bitwarden/common/auth/abstractions/avatar.service"; import { DevicesServiceAbstraction } from "@bitwarden/common/auth/abstractions/devices/devices.service.abstraction"; @@ -52,7 +52,6 @@ import { UserVerificationService as UserVerificationServiceAbstraction } from "@ import { AuthServerNotificationTags } from "@bitwarden/common/auth/enums/auth-server-notification-tags"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { AccountServiceImplementation } from "@bitwarden/common/auth/services/account.service"; -import { AuthRequestAnsweringService } from "@bitwarden/common/auth/services/auth-request-answering/auth-request-answering.service"; import { PendingAuthRequestsStateService } from "@bitwarden/common/auth/services/auth-request-answering/pending-auth-requests.state"; import { AuthService } from "@bitwarden/common/auth/services/auth.service"; import { AvatarService } from "@bitwarden/common/auth/services/avatar.service"; @@ -275,6 +274,7 @@ import { VaultExportServiceAbstraction, } from "@bitwarden/vault-export-core"; +import { ExtensionAuthRequestAnsweringService } from "../auth/services/auth-request-answering/extension-auth-request-answering.service"; import { AuthStatusBadgeUpdaterService } from "../auth/services/auth-status-badge-updater.service"; import { ExtensionLockService } from "../auth/services/extension-lock.service"; import { OverlayNotificationsBackground as OverlayNotificationsBackgroundInterface } from "../autofill/background/abstractions/overlay-notifications.background"; @@ -392,7 +392,7 @@ export default class MainBackground { serverNotificationsService: ServerNotificationsService; systemNotificationService: SystemNotificationsService; actionsService: ActionsService; - authRequestAnsweringService: AuthRequestAnsweringServiceAbstraction; + authRequestAnsweringService: AuthRequestAnsweringService; stateService: StateServiceAbstraction; userNotificationSettingsService: UserNotificationSettingsServiceAbstraction; autofillSettingsService: AutofillSettingsServiceAbstraction; @@ -1208,16 +1208,17 @@ export default class MainBackground { this.pendingAuthRequestStateService = new PendingAuthRequestsStateService(this.stateProvider); - this.authRequestAnsweringService = new AuthRequestAnsweringService( + this.authRequestAnsweringService = new ExtensionAuthRequestAnsweringService( this.accountService, - this.actionsService, this.authService, - this.i18nService, this.masterPasswordService, this.messagingService, this.pendingAuthRequestStateService, + this.actionsService, + this.i18nService, this.platformUtilsService, this.systemNotificationService, + this.logService, ); this.serverNotificationsService = new DefaultServerNotificationsService( diff --git a/apps/browser/src/popup/app.component.ts b/apps/browser/src/popup/app.component.ts index 8f00569b720..e4cb8a654c4 100644 --- a/apps/browser/src/popup/app.component.ts +++ b/apps/browser/src/popup/app.component.ts @@ -14,16 +14,11 @@ import { NavigationEnd, Router, RouterOutlet } from "@angular/router"; import { catchError, concatMap, - distinctUntilChanged, filter, firstValueFrom, map, of, - pairwise, - startWith, Subject, - switchMap, - take, takeUntil, tap, } from "rxjs"; @@ -38,7 +33,7 @@ import { } from "@bitwarden/auth/common"; import { BrowserApi } from "@bitwarden/browser/platform/browser/browser-api"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; -import { AuthRequestAnsweringServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; +import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { TokenService } from "@bitwarden/common/auth/abstractions/token.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; @@ -83,7 +78,8 @@ export class AppComponent implements OnInit, OnDestroy { private lastActivity: Date; private activeUserId: UserId; private routerAnimations = false; - private processingPendingAuth = false; + private processingPendingAuthRequests = false; + private shouldRerunAuthRequestProcessing = false; private destroy$ = new Subject(); @@ -118,7 +114,7 @@ export class AppComponent implements OnInit, OnDestroy { private logService: LogService, private authRequestService: AuthRequestServiceAbstraction, private pendingAuthRequestsState: PendingAuthRequestsStateService, - private authRequestAnsweringService: AuthRequestAnsweringServiceAbstraction, + private authRequestAnsweringService: AuthRequestAnsweringService, ) { this.deviceTrustToastService.setupListeners$.pipe(takeUntilDestroyed()).subscribe(); @@ -136,22 +132,7 @@ export class AppComponent implements OnInit, OnDestroy { this.activeUserId = account?.id; }); - // Trigger processing auth requests when the active user is in an unlocked state. Runs once when - // the popup is open. - this.accountService.activeAccount$ - .pipe( - map((a) => a?.id), // Extract active userId - distinctUntilChanged(), // Only when userId actually changes - filter((userId) => userId != null), // Require a valid userId - switchMap((userId) => this.authService.authStatusFor$(userId).pipe(take(1))), // Get current auth status once for new user - filter((status) => status === AuthenticationStatus.Unlocked), // Only when the new user is Unlocked - tap(() => { - // Trigger processing when switching users while popup is open - void this.authRequestAnsweringService.processPendingAuthRequests(); - }), - takeUntil(this.destroy$), - ) - .subscribe(); + this.authRequestAnsweringService.setupUnlockListenersForProcessingAuthRequests(this.destroy$); this.authService.activeAccountStatus$ .pipe( @@ -163,23 +144,6 @@ export class AppComponent implements OnInit, OnDestroy { ) .subscribe(); - // When the popup is already open and the active account transitions to Unlocked, - // process any pending auth requests for the active user. The above subscription does not handle - // this case. - this.authService.activeAccountStatus$ - .pipe( - startWith(null as unknown as AuthenticationStatus), // Seed previous value to handle initial emission - pairwise(), // Compare previous and current statuses - filter( - ([prev, curr]) => - prev !== AuthenticationStatus.Unlocked && curr === AuthenticationStatus.Unlocked, // Fire on transitions into Unlocked (incl. initial) - ), - takeUntil(this.destroy$), - ) - .subscribe(() => { - void this.authRequestAnsweringService.processPendingAuthRequests(); - }); - this.ngZone.runOutsideAngular(() => { window.onmousedown = () => this.recordActivity(); window.ontouchstart = () => this.recordActivity(); @@ -234,38 +198,31 @@ export class AppComponent implements OnInit, OnDestroy { await this.router.navigate(["lock"]); } else if (msg.command === "openLoginApproval") { - if (this.processingPendingAuth) { + if (this.processingPendingAuthRequests) { + // If an "openLoginApproval" message is received while we are currently processing other + // auth requests, then set a flag so we remember to process that new auth request + this.shouldRerunAuthRequestProcessing = true; return; } - this.processingPendingAuth = true; - try { - // Always query server for all pending requests and open a dialog for each - const pendingList = await firstValueFrom( - this.authRequestService.getPendingAuthRequests$(), - ); - if (Array.isArray(pendingList) && pendingList.length > 0) { - const respondedIds = new Set(); - for (const req of pendingList) { - if (req?.id == null) { - continue; - } - const dialogRef = LoginApprovalDialogComponent.open(this.dialogService, { - notificationId: req.id, - }); - const result = await firstValueFrom(dialogRef.closed); + /** + * This do/while loop allows us to: + * - a) call processPendingAuthRequests() once on "openLoginApproval" + * - b) remember to re-call processPendingAuthRequests() if another "openLoginApproval" was + * received while we were processing the original auth requests + */ + do { + this.shouldRerunAuthRequestProcessing = false; - if (result !== undefined && typeof result === "boolean") { - respondedIds.add(req.id); - if (respondedIds.size === pendingList.length && this.activeUserId != null) { - await this.pendingAuthRequestsState.clear(this.activeUserId); - } - } - } + try { + await this.processPendingAuthRequests(); + } catch (error) { + this.logService.error(`Error processing pending auth requests: ${error}`); + this.shouldRerunAuthRequestProcessing = false; // Reset flag to prevent infinite loop on persistent errors } - } finally { - this.processingPendingAuth = false; - } + // If an "openLoginApproval" message was received while processPendingAuthRequests() was running, then + // shouldRerunAuthRequestProcessing will have been set to true + } while (this.shouldRerunAuthRequestProcessing); } else if (msg.command === "showDialog") { // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. // eslint-disable-next-line @typescript-eslint/no-floating-promises @@ -403,4 +360,39 @@ export class AppComponent implements OnInit, OnDestroy { this.toastService.showToast(toastOptions); } + + private async processPendingAuthRequests() { + this.processingPendingAuthRequests = true; + + try { + // Always query server for all pending requests and open a dialog for each + const pendingList = await firstValueFrom(this.authRequestService.getPendingAuthRequests$()); + + if (Array.isArray(pendingList) && pendingList.length > 0) { + const respondedIds = new Set(); + + for (const req of pendingList) { + if (req?.id == null) { + continue; + } + + const dialogRef = LoginApprovalDialogComponent.open(this.dialogService, { + notificationId: req.id, + }); + + const result = await firstValueFrom(dialogRef.closed); + + if (result !== undefined && typeof result === "boolean") { + respondedIds.add(req.id); + + if (respondedIds.size === pendingList.length && this.activeUserId != null) { + await this.pendingAuthRequestsState.clear(this.activeUserId); + } + } + } + } + } finally { + this.processingPendingAuthRequests = false; + } + } } diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index 7a2ded5bb83..cb6ee51f98c 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -40,6 +40,7 @@ import { LogoutService, UserDecryptionOptionsServiceAbstraction, } from "@bitwarden/auth/common"; +import { ExtensionAuthRequestAnsweringService } from "@bitwarden/browser/auth/services/auth-request-answering/extension-auth-request-answering.service"; import { ExtensionNewDeviceVerificationComponentService } from "@bitwarden/browser/auth/services/new-device-verification/extension-new-device-verification-component.service"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { EventCollectionService as EventCollectionServiceAbstraction } from "@bitwarden/common/abstractions/event/event-collection.service"; @@ -49,13 +50,12 @@ import { AccountService, AccountService as AccountServiceAbstraction, } from "@bitwarden/common/auth/abstractions/account.service"; -import { AuthRequestAnsweringServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; +import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { MasterPasswordApiService } from "@bitwarden/common/auth/abstractions/master-password-api.service.abstraction"; import { SsoLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/sso-login.service.abstraction"; import { TokenService } from "@bitwarden/common/auth/abstractions/token.service"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; -import { AuthRequestAnsweringService } from "@bitwarden/common/auth/services/auth-request-answering/auth-request-answering.service"; import { PendingAuthRequestsStateService } from "@bitwarden/common/auth/services/auth-request-answering/pending-auth-requests.state"; import { AutofillSettingsService, @@ -494,18 +494,19 @@ const safeProviders: SafeProvider[] = [ deps: [], }), safeProvider({ - provide: AuthRequestAnsweringServiceAbstraction, - useClass: AuthRequestAnsweringService, + provide: AuthRequestAnsweringService, + useClass: ExtensionAuthRequestAnsweringService, deps: [ AccountServiceAbstraction, - ActionsService, AuthService, - I18nServiceAbstraction, MasterPasswordServiceAbstraction, MessagingService, PendingAuthRequestsStateService, + ActionsService, + I18nServiceAbstraction, PlatformUtilsService, SystemNotificationsService, + LogService, ], }), safeProvider({ diff --git a/apps/desktop/src/app/app.component.ts b/apps/desktop/src/app/app.component.ts index d75702ee8b8..d1919c77bb5 100644 --- a/apps/desktop/src/app/app.component.ts +++ b/apps/desktop/src/app/app.component.ts @@ -31,6 +31,7 @@ import { DocumentLangSetter } from "@bitwarden/angular/platform/i18n"; import { ModalService } from "@bitwarden/angular/services/modal.service"; import { FingerprintDialogComponent } from "@bitwarden/auth/angular"; import { + AuthRequestServiceAbstraction, DESKTOP_SSO_CALLBACK, LockService, LogoutReason, @@ -40,11 +41,13 @@ import { EventUploadService } from "@bitwarden/common/abstractions/event/event-u import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { TokenService } from "@bitwarden/common/auth/abstractions/token.service"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; +import { PendingAuthRequestsStateService } from "@bitwarden/common/auth/services/auth-request-answering/pending-auth-requests.state"; import { ProcessReloadServiceAbstraction } from "@bitwarden/common/key-management/abstractions/process-reload.service"; import { KeyConnectorService } from "@bitwarden/common/key-management/key-connector/abstractions/key-connector.service"; import { MasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; @@ -151,6 +154,8 @@ export class AppComponent implements OnInit, OnDestroy { private isIdle = false; private activeUserId: UserId = null; private activeSimpleDialog: DialogRef = null; + private processingPendingAuthRequests = false; + private shouldRerunAuthRequestProcessing = false; private destroy$ = new Subject(); @@ -200,6 +205,9 @@ export class AppComponent implements OnInit, OnDestroy { private desktopAutotypeDefaultSettingPolicy: DesktopAutotypeDefaultSettingPolicy, private readonly lockService: LockService, private premiumUpgradePromptService: PremiumUpgradePromptService, + private pendingAuthRequestsState: PendingAuthRequestsStateService, + private authRequestService: AuthRequestServiceAbstraction, + private authRequestAnsweringService: AuthRequestAnsweringService, ) { this.deviceTrustToastService.setupListeners$.pipe(takeUntilDestroyed()).subscribe(); @@ -212,6 +220,8 @@ export class AppComponent implements OnInit, OnDestroy { this.activeUserId = account?.id; }); + this.authRequestAnsweringService.setupUnlockListenersForProcessingAuthRequests(this.destroy$); + this.ngZone.runOutsideAngular(() => { setTimeout(async () => { await this.updateAppMenu(); @@ -499,13 +509,31 @@ export class AppComponent implements OnInit, OnDestroy { await this.checkForSystemTimeout(VaultTimeoutStringType.OnIdle); break; case "openLoginApproval": - if (message.notificationId != null) { - this.dialogService.closeAll(); - const dialogRef = LoginApprovalDialogComponent.open(this.dialogService, { - notificationId: message.notificationId, - }); - await firstValueFrom(dialogRef.closed); + if (this.processingPendingAuthRequests) { + // If an "openLoginApproval" message is received while we are currently processing other + // auth requests, then set a flag so we remember to process that new auth request + this.shouldRerunAuthRequestProcessing = true; + return; } + + /** + * This do/while loop allows us to: + * - a) call processPendingAuthRequests() once on "openLoginApproval" + * - b) remember to re-call processPendingAuthRequests() if another "openLoginApproval" was + * received while we were processing the original auth requests + */ + do { + this.shouldRerunAuthRequestProcessing = false; + + try { + await this.processPendingAuthRequests(); + } catch (error) { + this.logService.error(`Error processing pending auth requests: ${error}`); + this.shouldRerunAuthRequestProcessing = false; // Reset flag to prevent infinite loop on persistent errors + } + // If an "openLoginApproval" message was received while processPendingAuthRequests() was running, then + // shouldRerunAuthRequestProcessing will have been set to true + } while (this.shouldRerunAuthRequestProcessing); break; case "redrawMenu": await this.updateAppMenu(); @@ -887,4 +915,39 @@ export class AppComponent implements OnInit, OnDestroy { DeleteAccountComponent.open(this.dialogService); } + + private async processPendingAuthRequests() { + this.processingPendingAuthRequests = true; + + try { + // Always query server for all pending requests and open a dialog for each + const pendingList = await firstValueFrom(this.authRequestService.getPendingAuthRequests$()); + + if (Array.isArray(pendingList) && pendingList.length > 0) { + const respondedIds = new Set(); + + for (const req of pendingList) { + if (req?.id == null) { + continue; + } + + const dialogRef = LoginApprovalDialogComponent.open(this.dialogService, { + notificationId: req.id, + }); + + const result = await firstValueFrom(dialogRef.closed); + + if (result !== undefined && typeof result === "boolean") { + respondedIds.add(req.id); + + if (respondedIds.size === pendingList.length && this.activeUserId != null) { + await this.pendingAuthRequestsState.clear(this.activeUserId); + } + } + } + } + } finally { + this.processingPendingAuthRequests = false; + } + } } diff --git a/apps/desktop/src/app/services/services.module.ts b/apps/desktop/src/app/services/services.module.ts index 382b680efbc..752c09e2e92 100644 --- a/apps/desktop/src/app/services/services.module.ts +++ b/apps/desktop/src/app/services/services.module.ts @@ -5,7 +5,6 @@ import { Router } from "@angular/router"; import { Subject, merge } from "rxjs"; import { OrganizationUserApiService } from "@bitwarden/admin-console/common"; -import { LoginApprovalDialogComponentServiceAbstraction } from "@bitwarden/angular/auth/login-approval"; import { SetInitialPasswordService } from "@bitwarden/angular/auth/password-management/set-initial-password/set-initial-password.service.abstraction"; import { SafeProvider, safeProvider } from "@bitwarden/angular/platform/utils/safe-provider"; import { @@ -45,6 +44,7 @@ import { AccountService, AccountService as AccountServiceAbstraction, } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; import { AuthService, AuthService as AuthServiceAbstraction, @@ -52,6 +52,7 @@ import { import { MasterPasswordApiService } from "@bitwarden/common/auth/abstractions/master-password-api.service.abstraction"; import { SsoLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/sso-login.service.abstraction"; import { TokenService } from "@bitwarden/common/auth/abstractions/token.service"; +import { PendingAuthRequestsStateService } from "@bitwarden/common/auth/services/auth-request-answering/pending-auth-requests.state"; import { AutofillSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/autofill-settings.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions"; import { ClientType } from "@bitwarden/common/enums"; @@ -61,7 +62,10 @@ import { KeyGenerationService } from "@bitwarden/common/key-management/crypto"; import { CryptoFunctionService as CryptoFunctionServiceAbstraction } from "@bitwarden/common/key-management/crypto/abstractions/crypto-function.service"; import { EncryptService } from "@bitwarden/common/key-management/crypto/abstractions/encrypt.service"; import { WebCryptoFunctionService } from "@bitwarden/common/key-management/crypto/services/web-crypto-function.service"; -import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; +import { + InternalMasterPasswordServiceAbstraction, + MasterPasswordServiceAbstraction, +} from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; import { PinServiceAbstraction } from "@bitwarden/common/key-management/pin/pin.service.abstraction"; import { DefaultProcessReloadService } from "@bitwarden/common/key-management/services/default-process-reload.service"; import { SessionTimeoutTypeService } from "@bitwarden/common/key-management/session-timeout"; @@ -120,8 +124,8 @@ import { import { SerializedMemoryStorageService } from "@bitwarden/storage-core"; import { DefaultSshImportPromptService, SshImportPromptService } from "@bitwarden/vault"; -import { DesktopLoginApprovalDialogComponentService } from "../../auth/login/desktop-login-approval-dialog-component.service"; import { DesktopLoginComponentService } from "../../auth/login/desktop-login-component.service"; +import { DesktopAuthRequestAnsweringService } from "../../auth/services/auth-request-answering/desktop-auth-request-answering.service"; import { DesktopTwoFactorAuthDuoComponentService } from "../../auth/services/desktop-two-factor-auth-duo-component.service"; import { DesktopAutofillSettingsService } from "../../autofill/services/desktop-autofill-settings.service"; import { DesktopAutofillService } from "../../autofill/services/desktop-autofill.service"; @@ -469,11 +473,6 @@ const safeProviders: SafeProvider[] = [ useClass: DefaultSsoComponentService, deps: [], }), - safeProvider({ - provide: LoginApprovalDialogComponentServiceAbstraction, - useClass: DesktopLoginApprovalDialogComponentService, - deps: [I18nServiceAbstraction], - }), safeProvider({ provide: SshImportPromptService, useClass: DefaultSshImportPromptService, @@ -509,6 +508,19 @@ const safeProviders: SafeProvider[] = [ useClass: SessionTimeoutSettingsComponentService, deps: [I18nServiceAbstraction, SessionTimeoutTypeService, PolicyServiceAbstraction], }), + safeProvider({ + provide: AuthRequestAnsweringService, + useClass: DesktopAuthRequestAnsweringService, + deps: [ + AccountServiceAbstraction, + AuthService, + MasterPasswordServiceAbstraction, + MessagingServiceAbstraction, + PendingAuthRequestsStateService, + I18nServiceAbstraction, + LogService, + ], + }), ]; @NgModule({ diff --git a/apps/desktop/src/auth/login/desktop-login-approval-dialog-component.service.spec.ts b/apps/desktop/src/auth/login/desktop-login-approval-dialog-component.service.spec.ts deleted file mode 100644 index 2ae584d7e7f..00000000000 --- a/apps/desktop/src/auth/login/desktop-login-approval-dialog-component.service.spec.ts +++ /dev/null @@ -1,91 +0,0 @@ -import { TestBed } from "@angular/core/testing"; -import { mock, MockProxy } from "jest-mock-extended"; -import { Subject } from "rxjs"; - -import { LoginApprovalDialogComponent } from "@bitwarden/angular/auth/login-approval"; -import { I18nService as I18nServiceAbstraction } from "@bitwarden/common/platform/abstractions/i18n.service"; - -import { DesktopLoginApprovalDialogComponentService } from "./desktop-login-approval-dialog-component.service"; - -describe("DesktopLoginApprovalDialogComponentService", () => { - let service: DesktopLoginApprovalDialogComponentService; - let i18nService: MockProxy; - let originalIpc: any; - - beforeEach(() => { - originalIpc = (global as any).ipc; - (global as any).ipc = { - auth: { - loginRequest: jest.fn(), - }, - platform: { - isWindowVisible: jest.fn(), - }, - }; - - i18nService = mock({ - t: jest.fn(), - userSetLocale$: new Subject(), - locale$: new Subject(), - }); - - TestBed.configureTestingModule({ - providers: [ - DesktopLoginApprovalDialogComponentService, - { provide: I18nServiceAbstraction, useValue: i18nService }, - ], - }); - - service = TestBed.inject(DesktopLoginApprovalDialogComponentService); - }); - - afterEach(() => { - jest.clearAllMocks(); - (global as any).ipc = originalIpc; - }); - - it("is created successfully", () => { - expect(service).toBeTruthy(); - }); - - it("calls ipc.auth.loginRequest with correct parameters when window is not visible", async () => { - const title = "Log in requested"; - const email = "test@bitwarden.com"; - const message = `Confirm access attempt for ${email}`; - const closeText = "Close"; - - const loginApprovalDialogComponent = { email } as LoginApprovalDialogComponent; - i18nService.t.mockImplementation((key: string) => { - switch (key) { - case "accountAccessRequested": - return title; - case "confirmAccessAttempt": - return message; - case "close": - return closeText; - default: - return ""; - } - }); - - jest.spyOn(ipc.platform, "isWindowVisible").mockResolvedValue(false); - jest.spyOn(ipc.auth, "loginRequest").mockResolvedValue(); - - await service.showLoginRequestedAlertIfWindowNotVisible(loginApprovalDialogComponent.email); - - expect(ipc.auth.loginRequest).toHaveBeenCalledWith(title, message, closeText); - }); - - it("does not call ipc.auth.loginRequest when window is visible", async () => { - const loginApprovalDialogComponent = { - email: "test@bitwarden.com", - } as LoginApprovalDialogComponent; - - jest.spyOn(ipc.platform, "isWindowVisible").mockResolvedValue(true); - jest.spyOn(ipc.auth, "loginRequest"); - - await service.showLoginRequestedAlertIfWindowNotVisible(loginApprovalDialogComponent.email); - - expect(ipc.auth.loginRequest).not.toHaveBeenCalled(); - }); -}); diff --git a/apps/desktop/src/auth/login/desktop-login-approval-dialog-component.service.ts b/apps/desktop/src/auth/login/desktop-login-approval-dialog-component.service.ts deleted file mode 100644 index 9c48f71990a..00000000000 --- a/apps/desktop/src/auth/login/desktop-login-approval-dialog-component.service.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { Injectable } from "@angular/core"; - -import { - DefaultLoginApprovalDialogComponentService, - LoginApprovalDialogComponentServiceAbstraction, -} from "@bitwarden/angular/auth/login-approval"; -import { I18nService as I18nServiceAbstraction } from "@bitwarden/common/platform/abstractions/i18n.service"; - -@Injectable() -export class DesktopLoginApprovalDialogComponentService - extends DefaultLoginApprovalDialogComponentService - implements LoginApprovalDialogComponentServiceAbstraction -{ - constructor(private i18nService: I18nServiceAbstraction) { - super(); - } - - async showLoginRequestedAlertIfWindowNotVisible(email?: string): Promise { - const isVisible = await ipc.platform.isWindowVisible(); - if (!isVisible) { - await ipc.auth.loginRequest( - this.i18nService.t("accountAccessRequested"), - this.i18nService.t("confirmAccessAttempt", email), - this.i18nService.t("close"), - ); - } - } -} diff --git a/apps/desktop/src/auth/services/auth-request-answering/desktop-auth-request-answering.service.spec.ts b/apps/desktop/src/auth/services/auth-request-answering/desktop-auth-request-answering.service.spec.ts new file mode 100644 index 00000000000..2caaf713473 --- /dev/null +++ b/apps/desktop/src/auth/services/auth-request-answering/desktop-auth-request-answering.service.spec.ts @@ -0,0 +1,277 @@ +import { mock, MockProxy } from "jest-mock-extended"; +import { of } from "rxjs"; + +import { Account, AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; +import { PendingAuthRequestsStateService } from "@bitwarden/common/auth/services/auth-request-answering/pending-auth-requests.state"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { mockAccountInfoWith } from "@bitwarden/common/spec"; +import { LogService } from "@bitwarden/logging"; +import { UserId } from "@bitwarden/user-core"; + +import { DesktopAuthRequestAnsweringService } from "./desktop-auth-request-answering.service"; + +describe("DesktopAuthRequestAnsweringService", () => { + let accountService: MockProxy; + let authService: MockProxy; + let masterPasswordService: any; // MasterPasswordServiceAbstraction has many members; we only use forceSetPasswordReason$ + let messagingService: MockProxy; + let pendingAuthRequestsState: MockProxy; + let i18nService: MockProxy; + let logService: MockProxy; + + let sut: AuthRequestAnsweringService; + + const userId = "9f4c3452-6a45-48af-a7d0-74d3e8b65e4c" as UserId; + const userAccountInfo = mockAccountInfoWith({ + name: "User", + email: "user@example.com", + }); + const userAccount: Account = { + id: userId, + ...userAccountInfo, + }; + + const authRequestId = "auth-request-id-123"; + + beforeEach(() => { + (global as any).ipc = { + platform: { + isWindowVisible: jest.fn(), + }, + auth: { + loginRequest: jest.fn(), + }, + }; + + accountService = mock(); + authService = mock(); + masterPasswordService = { + forceSetPasswordReason$: jest.fn().mockReturnValue(of(ForceSetPasswordReason.None)), + }; + messagingService = mock(); + pendingAuthRequestsState = mock(); + i18nService = mock(); + logService = mock(); + + // Common defaults + authService.activeAccountStatus$ = of(AuthenticationStatus.Locked); + accountService.activeAccount$ = of(userAccount); + accountService.accounts$ = of({ + [userId]: userAccountInfo, + }); + (global as any).ipc.platform.isWindowVisible.mockResolvedValue(false); + i18nService.t.mockImplementation( + (key: string, p1?: any) => `${key}${p1 != null ? ":" + p1 : ""}`, + ); + + sut = new DesktopAuthRequestAnsweringService( + accountService, + authService, + masterPasswordService, + messagingService, + pendingAuthRequestsState, + i18nService, + logService, + ); + }); + + describe("receivedPendingAuthRequest()", () => { + it("should throw if authRequestUserId not given", async () => { + // Act + const promise = sut.receivedPendingAuthRequest(undefined, undefined); + + // Assert + await expect(promise).rejects.toThrow("authRequestUserId required"); + }); + + it("should add a pending marker for the user to state", async () => { + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect(pendingAuthRequestsState.add).toHaveBeenCalledTimes(1); + expect(pendingAuthRequestsState.add).toHaveBeenCalledWith(userId); + }); + + describe("given the active user is the intended recipient of the auth request, unlocked, and not required to set/change their master password", () => { + describe("given the Desktop window is visible", () => { + it("should send an 'openLoginApproval' message", async () => { + // Arrange + (global as any).ipc.platform.isWindowVisible.mockResolvedValue(true); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect(messagingService.send).toHaveBeenCalledTimes(1); + expect(messagingService.send).toHaveBeenCalledWith("openLoginApproval"); + }); + + it("should NOT create a system notification", async () => { + // Arrange + (global as any).ipc.platform.isWindowVisible.mockResolvedValue(true); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect((global as any).ipc.auth.loginRequest).not.toHaveBeenCalled(); + }); + }); + + describe("given the Desktop window is NOT visible", () => { + it("should STILL send an 'openLoginApproval' message", async () => { + // Arrange + (global as any).ipc.platform.isWindowVisible.mockResolvedValue(false); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect(messagingService.send).toHaveBeenCalledTimes(1); + expect(messagingService.send).toHaveBeenCalledWith("openLoginApproval"); + }); + + it("should create a system notification", async () => { + // Arrange + (global as any).ipc.platform.isWindowVisible.mockResolvedValue(false); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect(i18nService.t).toHaveBeenCalledWith("accountAccessRequested"); + expect(i18nService.t).toHaveBeenCalledWith("confirmAccessAttempt", "user@example.com"); + expect(i18nService.t).toHaveBeenCalledWith("close"); + + expect((global as any).ipc.auth.loginRequest).toHaveBeenCalledWith( + "accountAccessRequested", + "confirmAccessAttempt:user@example.com", + "close", + ); + }); + }); + }); + + describe("given the active user is Locked", () => { + it("should NOT send an 'openLoginApproval' message", async () => { + // Arrange + (global as any).ipc.platform.isWindowVisible.mockResolvedValue(true); + authService.activeAccountStatus$ = of(AuthenticationStatus.Locked); + + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect(messagingService.send).not.toHaveBeenCalled(); + }); + + it("should create a system notification", async () => { + // Arrange + (global as any).ipc.platform.isWindowVisible.mockResolvedValue(true); + authService.activeAccountStatus$ = of(AuthenticationStatus.Locked); + + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect((global as any).ipc.auth.loginRequest).toHaveBeenCalledWith( + "accountAccessRequested", + "confirmAccessAttempt:user@example.com", + "close", + ); + }); + }); + + describe("given the active user is not the intended recipient of the auth request", () => { + beforeEach(() => { + // Different active user for these tests + const differentUserId = "different-user-id" as UserId; + accountService.activeAccount$ = of({ + id: differentUserId, + ...mockAccountInfoWith({ + name: "Different User", + email: "different@example.com", + }), + }); + }); + + it("should NOT send an 'openLoginApproval' message", async () => { + // Arrange + (global as any).ipc.platform.isWindowVisible.mockResolvedValue(true); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + // Pass in userId, not differentUserId (the active user), to mimic an auth + // request coming in for a user who is not the active user + await sut.receivedPendingAuthRequest(userId, authRequestId); // pass in userId, not differentUserId + + // Assert + expect(messagingService.send).not.toHaveBeenCalled(); + }); + + it("should create a system notification", async () => { + // Arrange + (global as any).ipc.platform.isWindowVisible.mockResolvedValue(true); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + // Pass in userId, not differentUserId (the active user), to mimic an auth + // request coming in for a user who is not the active user + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect((global as any).ipc.auth.loginRequest).toHaveBeenCalledWith( + "accountAccessRequested", + "confirmAccessAttempt:user@example.com", + "close", + ); + }); + }); + + describe("given the active user is required to set/change their master password", () => { + it("should NOT send an 'openLoginApproval' message", async () => { + // Arrange + (global as any).ipc.platform.isWindowVisible.mockResolvedValue(true); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + masterPasswordService.forceSetPasswordReason$ = jest + .fn() + .mockReturnValue(of(ForceSetPasswordReason.WeakMasterPassword)); + + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect(messagingService.send).not.toHaveBeenCalled(); + }); + + it("should create a system notification", async () => { + // Arrange + (global as any).ipc.platform.isWindowVisible.mockResolvedValue(true); + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + masterPasswordService.forceSetPasswordReason$ = jest + .fn() + .mockReturnValue(of(ForceSetPasswordReason.WeakMasterPassword)); + + // Act + await sut.receivedPendingAuthRequest(userId, authRequestId); + + // Assert + expect((global as any).ipc.auth.loginRequest).toHaveBeenCalledWith( + "accountAccessRequested", + "confirmAccessAttempt:user@example.com", + "close", + ); + }); + }); + }); +}); diff --git a/apps/desktop/src/auth/services/auth-request-answering/desktop-auth-request-answering.service.ts b/apps/desktop/src/auth/services/auth-request-answering/desktop-auth-request-answering.service.ts new file mode 100644 index 00000000000..3b2602660fe --- /dev/null +++ b/apps/desktop/src/auth/services/auth-request-answering/desktop-auth-request-answering.service.ts @@ -0,0 +1,85 @@ +import { firstValueFrom } from "rxjs"; + +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { DefaultAuthRequestAnsweringService } from "@bitwarden/common/auth/services/auth-request-answering/default-auth-request-answering.service"; +import { PendingAuthRequestsStateService } from "@bitwarden/common/auth/services/auth-request-answering/pending-auth-requests.state"; +import { MasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { LogService } from "@bitwarden/logging"; +import { UserId } from "@bitwarden/user-core"; + +export class DesktopAuthRequestAnsweringService + extends DefaultAuthRequestAnsweringService + implements AuthRequestAnsweringService +{ + constructor( + protected readonly accountService: AccountService, + protected readonly authService: AuthService, + protected readonly masterPasswordService: MasterPasswordServiceAbstraction, + protected readonly messagingService: MessagingService, + protected readonly pendingAuthRequestsState: PendingAuthRequestsStateService, + private readonly i18nService: I18nService, + private readonly logService: LogService, + ) { + super( + accountService, + authService, + masterPasswordService, + messagingService, + pendingAuthRequestsState, + ); + } + + /** + * @param authRequestUserId The UserId that the auth request is for. + * @param authRequestId The authRequestId param is not used on Desktop because clicks on a + * Desktop notification do not run any auth-request-specific actions. + * All clicks simply open the Desktop window. See electron-main-messaging.service.ts. + */ + async receivedPendingAuthRequest( + authRequestUserId: UserId, + authRequestId: string, + ): Promise { + if (!authRequestUserId) { + throw new Error("authRequestUserId required"); + } + + // Always persist the pending marker for this user to global state. + await this.pendingAuthRequestsState.add(authRequestUserId); + + const activeUserMeetsConditionsToShowApprovalDialog = + await this.activeUserMeetsConditionsToShowApprovalDialog(authRequestUserId); + + if (activeUserMeetsConditionsToShowApprovalDialog) { + // Send message to open dialog immediately for this request + this.messagingService.send("openLoginApproval"); + } + + const isWindowVisible = await ipc.platform.isWindowVisible(); + + // Create a system notification if either of the following are true: + // - User does NOT meet conditions to show dialog + // - User does meet conditions, but the Desktop window is not visible + // - In this second case, we both send the "openLoginApproval" message (above) AND + // also create the system notification to notify the user that the dialog is there. + if (!activeUserMeetsConditionsToShowApprovalDialog || !isWindowVisible) { + const accounts = await firstValueFrom(this.accountService.accounts$); + const accountInfo = accounts[authRequestUserId]; + + if (!accountInfo) { + this.logService.error("Account not found for authRequestUserId"); + return; + } + + const emailForUser = accountInfo.email; + await ipc.auth.loginRequest( + this.i18nService.t("accountAccessRequested"), + this.i18nService.t("confirmAccessAttempt", emailForUser), + this.i18nService.t("close"), + ); + } + } +} diff --git a/apps/desktop/src/vault/app/vault/vault-v2.component.ts b/apps/desktop/src/vault/app/vault/vault-v2.component.ts index d7cf475036d..ade4af928fc 100644 --- a/apps/desktop/src/vault/app/vault/vault-v2.component.ts +++ b/apps/desktop/src/vault/app/vault/vault-v2.component.ts @@ -27,7 +27,6 @@ import { CollectionService, CollectionView } from "@bitwarden/admin-console/comm import { PremiumBadgeComponent } from "@bitwarden/angular/billing/components/premium-badge"; import { VaultViewPasswordHistoryService } from "@bitwarden/angular/services/view-password-history.service"; import { VaultFilter } from "@bitwarden/angular/vault/vault-filter/models/vault-filter.model"; -import { AuthRequestServiceAbstraction } from "@bitwarden/auth/common"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; @@ -35,11 +34,12 @@ import { PolicyService } from "@bitwarden/common/admin-console/abstractions/poli import { PolicyType } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { Account, AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; import { getUserId } from "@bitwarden/common/auth/services/account.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { EventType } from "@bitwarden/common/enums"; +import { MasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; import { BroadcasterService } from "@bitwarden/common/platform/abstractions/broadcaster.service"; -import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; @@ -249,11 +249,10 @@ export class VaultV2Component private collectionService: CollectionService, private organizationService: OrganizationService, private folderService: FolderService, - private configService: ConfigService, - private authRequestService: AuthRequestServiceAbstraction, private cipherArchiveService: CipherArchiveService, private policyService: PolicyService, private archiveCipherUtilitiesService: ArchiveCipherUtilitiesService, + private masterPasswordService: MasterPasswordServiceAbstraction, ) {} async ngOnInit() { @@ -374,19 +373,12 @@ export class VaultV2Component this.searchBarService.setEnabled(true); this.searchBarService.setPlaceholderText(this.i18nService.t("searchVault")); - const authRequests = await firstValueFrom( - this.authRequestService.getLatestPendingAuthRequest$()!, - ); - if (authRequests != null) { - this.messagingService.send("openLoginApproval", { - notificationId: authRequests.id, - }); - } - this.activeUserId = await firstValueFrom( this.accountService.activeAccount$.pipe(getUserId), ).catch((): any => null); + await this.sendOpenLoginApprovalMessage(this.activeUserId); + if (this.activeUserId) { this.cipherService .failedToDecryptCiphers$(this.activeUserId) @@ -1061,4 +1053,27 @@ export class VaultV2Component } return repromptResult; } + + /** + * Sends a message that will retrieve any pending auth requests from the server. If there are + * pending auth requests for this user, a LoginApprovalDialogComponent will open. If there + * are no pending auth requests, nothing happens (see AppComponent: "openLoginApproval"). + */ + private async sendOpenLoginApprovalMessage(activeUserId: UserId) { + // This is a defensive check against a race condition where a user may have successfully logged + // in with no forceSetPasswordReason, but while the vault component is loading, a sync sets + // forceSetPasswordReason.TdeUserWithoutPasswordHasPasswordResetPermission (see DefaultSyncService). + // This could potentially happen if an Admin upgrades the user's permissions as the user is logging + // in. In this rare case we do not want to send an "openLoginApproval" message. + // + // This also keeps parity with other usages of the "openLoginApproval" message. That is: don't send + // an "openLoginApproval" message if the user is required to set/change their password. + const forceSetPasswordReason = await firstValueFrom( + this.masterPasswordService.forceSetPasswordReason$(activeUserId), + ); + if (forceSetPasswordReason === ForceSetPasswordReason.None) { + // If there are pending auth requests for this user, a LoginApprovalDialogComponent will open + this.messagingService.send("openLoginApproval"); + } + } } diff --git a/apps/web/src/app/core/core.module.ts b/apps/web/src/app/core/core.module.ts index bd557dc5947..e436e194e9e 100644 --- a/apps/web/src/app/core/core.module.ts +++ b/apps/web/src/app/core/core.module.ts @@ -59,9 +59,11 @@ import { } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { AccountApiService as AccountApiServiceAbstraction } from "@bitwarden/common/auth/abstractions/account-api.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { MasterPasswordApiService } from "@bitwarden/common/auth/abstractions/master-password-api.service.abstraction"; import { SsoLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/sso-login.service.abstraction"; +import { NoopAuthRequestAnsweringService } from "@bitwarden/common/auth/services/auth-request-answering/noop-auth-request-answering.service"; import { OrganizationInviteService } from "@bitwarden/common/auth/services/organization-invite/organization-invite.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { ClientType } from "@bitwarden/common/enums"; @@ -483,6 +485,11 @@ const safeProviders: SafeProvider[] = [ useClass: SessionTimeoutSettingsComponentService, deps: [I18nServiceAbstraction, SessionTimeoutTypeService, PolicyService], }), + safeProvider({ + provide: AuthRequestAnsweringService, + useClass: NoopAuthRequestAnsweringService, + deps: [], + }), ]; @NgModule({ diff --git a/libs/angular/src/auth/login-approval/default-login-approval-dialog-component.service.spec.ts b/libs/angular/src/auth/login-approval/default-login-approval-dialog-component.service.spec.ts deleted file mode 100644 index 018b1ce2547..00000000000 --- a/libs/angular/src/auth/login-approval/default-login-approval-dialog-component.service.spec.ts +++ /dev/null @@ -1,30 +0,0 @@ -import { TestBed } from "@angular/core/testing"; - -import { DefaultLoginApprovalDialogComponentService } from "./default-login-approval-dialog-component.service"; -import { LoginApprovalDialogComponent } from "./login-approval-dialog.component"; - -describe("DefaultLoginApprovalDialogComponentService", () => { - let service: DefaultLoginApprovalDialogComponentService; - - beforeEach(() => { - TestBed.configureTestingModule({ - providers: [DefaultLoginApprovalDialogComponentService], - }); - - service = TestBed.inject(DefaultLoginApprovalDialogComponentService); - }); - - it("is created successfully", () => { - expect(service).toBeTruthy(); - }); - - it("has showLoginRequestedAlertIfWindowNotVisible method that is a no-op", async () => { - const loginApprovalDialogComponent = {} as LoginApprovalDialogComponent; - - const result = await service.showLoginRequestedAlertIfWindowNotVisible( - loginApprovalDialogComponent.email, - ); - - expect(result).toBeUndefined(); - }); -}); diff --git a/libs/angular/src/auth/login-approval/default-login-approval-dialog-component.service.ts b/libs/angular/src/auth/login-approval/default-login-approval-dialog-component.service.ts deleted file mode 100644 index 4a9a37fd0de..00000000000 --- a/libs/angular/src/auth/login-approval/default-login-approval-dialog-component.service.ts +++ /dev/null @@ -1,14 +0,0 @@ -import { LoginApprovalDialogComponentServiceAbstraction } from "./login-approval-dialog-component.service.abstraction"; - -/** - * Default implementation of the LoginApprovalDialogComponentServiceAbstraction. - */ -export class DefaultLoginApprovalDialogComponentService implements LoginApprovalDialogComponentServiceAbstraction { - /** - * No-op implementation of the showLoginRequestedAlertIfWindowNotVisible method. - * @returns - */ - async showLoginRequestedAlertIfWindowNotVisible(email?: string): Promise { - return; - } -} diff --git a/libs/angular/src/auth/login-approval/index.ts b/libs/angular/src/auth/login-approval/index.ts index 7b34b17d56b..21cde4df742 100644 --- a/libs/angular/src/auth/login-approval/index.ts +++ b/libs/angular/src/auth/login-approval/index.ts @@ -1,3 +1 @@ export * from "./login-approval-dialog.component"; -export * from "./login-approval-dialog-component.service.abstraction"; -export * from "./default-login-approval-dialog-component.service"; diff --git a/libs/angular/src/auth/login-approval/login-approval-dialog-component.service.abstraction.ts b/libs/angular/src/auth/login-approval/login-approval-dialog-component.service.abstraction.ts deleted file mode 100644 index f29311402a7..00000000000 --- a/libs/angular/src/auth/login-approval/login-approval-dialog-component.service.abstraction.ts +++ /dev/null @@ -1,9 +0,0 @@ -/** - * Abstraction for the LoginApprovalDialogComponent service. - */ -export abstract class LoginApprovalDialogComponentServiceAbstraction { - /** - * Shows a login requested alert if the window is not visible. - */ - abstract showLoginRequestedAlertIfWindowNotVisible: (email?: string) => Promise; -} diff --git a/libs/angular/src/auth/login-approval/login-approval-dialog.component.spec.ts b/libs/angular/src/auth/login-approval/login-approval-dialog.component.spec.ts index 4dc7522c1b8..0640c32ea4c 100644 --- a/libs/angular/src/auth/login-approval/login-approval-dialog.component.spec.ts +++ b/libs/angular/src/auth/login-approval/login-approval-dialog.component.spec.ts @@ -16,7 +16,6 @@ import { UserId } from "@bitwarden/common/types/guid"; import { DialogRef, DIALOG_DATA, ToastService } from "@bitwarden/components"; import { LogService } from "@bitwarden/logging"; -import { LoginApprovalDialogComponentServiceAbstraction } from "./login-approval-dialog-component.service.abstraction"; import { LoginApprovalDialogComponent } from "./login-approval-dialog.component"; describe("LoginApprovalDialogComponent", () => { @@ -69,10 +68,6 @@ describe("LoginApprovalDialogComponent", () => { { provide: LogService, useValue: logService }, { provide: ToastService, useValue: toastService }, { provide: ValidationService, useValue: validationService }, - { - provide: LoginApprovalDialogComponentServiceAbstraction, - useValue: mock(), - }, ], }).compileComponents(); diff --git a/libs/angular/src/auth/login-approval/login-approval-dialog.component.ts b/libs/angular/src/auth/login-approval/login-approval-dialog.component.ts index 35333c43536..54906047535 100644 --- a/libs/angular/src/auth/login-approval/login-approval-dialog.component.ts +++ b/libs/angular/src/auth/login-approval/login-approval-dialog.component.ts @@ -24,8 +24,6 @@ import { } from "@bitwarden/components"; import { LogService } from "@bitwarden/logging"; -import { LoginApprovalDialogComponentServiceAbstraction } from "./login-approval-dialog-component.service.abstraction"; - const RequestTimeOut = 60000 * 15; // 15 Minutes const RequestTimeUpdate = 60000 * 5; // 5 Minutes @@ -57,7 +55,6 @@ export class LoginApprovalDialogComponent implements OnInit, OnDestroy { private devicesService: DevicesServiceAbstraction, private dialogRef: DialogRef, private i18nService: I18nService, - private loginApprovalDialogComponentService: LoginApprovalDialogComponentServiceAbstraction, private logService: LogService, private toastService: ToastService, private validationService: ValidationService, @@ -113,10 +110,6 @@ export class LoginApprovalDialogComponent implements OnInit, OnDestroy { this.updateTimeText(); }, RequestTimeUpdate); - await this.loginApprovalDialogComponentService.showLoginRequestedAlertIfWindowNotVisible( - this.email, - ); - this.loading = false; } diff --git a/libs/angular/src/services/jslib-services.module.ts b/libs/angular/src/services/jslib-services.module.ts index 7899ff5281a..5eaac4033eb 100644 --- a/libs/angular/src/services/jslib-services.module.ts +++ b/libs/angular/src/services/jslib-services.module.ts @@ -94,7 +94,7 @@ import { InternalAccountService, } from "@bitwarden/common/auth/abstractions/account.service"; import { AnonymousHubService as AnonymousHubServiceAbstraction } from "@bitwarden/common/auth/abstractions/anonymous-hub.service"; -import { AuthRequestAnsweringServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; +import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; import { AuthService as AuthServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth.service"; import { AvatarService as AvatarServiceAbstraction } from "@bitwarden/common/auth/abstractions/avatar.service"; import { DevicesServiceAbstraction } from "@bitwarden/common/auth/abstractions/devices/devices.service.abstraction"; @@ -112,7 +112,7 @@ import { SendTokenService, DefaultSendTokenService } from "@bitwarden/common/aut import { AccountApiServiceImplementation } from "@bitwarden/common/auth/services/account-api.service"; import { AccountServiceImplementation } from "@bitwarden/common/auth/services/account.service"; import { AnonymousHubService } from "@bitwarden/common/auth/services/anonymous-hub.service"; -import { NoopAuthRequestAnsweringService } from "@bitwarden/common/auth/services/auth-request-answering/noop-auth-request-answering.service"; +import { DefaultAuthRequestAnsweringService } from "@bitwarden/common/auth/services/auth-request-answering/default-auth-request-answering.service"; import { PendingAuthRequestsStateService } from "@bitwarden/common/auth/services/auth-request-answering/pending-auth-requests.state"; import { AuthService } from "@bitwarden/common/auth/services/auth.service"; import { AvatarService } from "@bitwarden/common/auth/services/avatar.service"; @@ -397,8 +397,6 @@ import { VaultExportServiceAbstraction, } from "@bitwarden/vault-export-core"; -import { DefaultLoginApprovalDialogComponentService } from "../auth/login-approval/default-login-approval-dialog-component.service"; -import { LoginApprovalDialogComponentServiceAbstraction } from "../auth/login-approval/login-approval-dialog-component.service.abstraction"; import { DefaultSetInitialPasswordService } from "../auth/password-management/set-initial-password/default-set-initial-password.service.implementation"; import { SetInitialPasswordService } from "../auth/password-management/set-initial-password/set-initial-password.service.abstraction"; import { DeviceTrustToastService as DeviceTrustToastServiceAbstraction } from "../auth/services/device-trust-toast.service.abstraction"; @@ -1040,9 +1038,15 @@ const safeProviders: SafeProvider[] = [ deps: [StateProvider], }), safeProvider({ - provide: AuthRequestAnsweringServiceAbstraction, - useClass: NoopAuthRequestAnsweringService, - deps: [], + provide: AuthRequestAnsweringService, + useClass: DefaultAuthRequestAnsweringService, + deps: [ + AccountServiceAbstraction, + AuthServiceAbstraction, + MasterPasswordServiceAbstraction, + MessagingServiceAbstraction, + PendingAuthRequestsStateService, + ], }), safeProvider({ provide: ServerNotificationsService, @@ -1060,7 +1064,7 @@ const safeProviders: SafeProvider[] = [ SignalRConnectionService, AuthServiceAbstraction, WebPushConnectionService, - AuthRequestAnsweringServiceAbstraction, + AuthRequestAnsweringService, ConfigService, InternalPolicyService, ], @@ -1666,11 +1670,6 @@ const safeProviders: SafeProvider[] = [ useClass: DefaultSendPasswordService, deps: [CryptoFunctionServiceAbstraction], }), - safeProvider({ - provide: LoginApprovalDialogComponentServiceAbstraction, - useClass: DefaultLoginApprovalDialogComponentService, - deps: [], - }), safeProvider({ provide: LoginDecryptionOptionsService, useClass: DefaultLoginDecryptionOptionsService, diff --git a/libs/common/src/auth/abstractions/auth-request-answering/README.md b/libs/common/src/auth/abstractions/auth-request-answering/README.md index 9a24f095d70..edc368fd91b 100644 --- a/libs/common/src/auth/abstractions/auth-request-answering/README.md +++ b/libs/common/src/auth/abstractions/auth-request-answering/README.md @@ -1,7 +1,6 @@ # Auth Request Answering Service -This feature is to allow for the taking of auth requests that are received via websockets by the background service to -be acted on when the user loads up a client. Currently only implemented with the browser client. +This feature is to allow for the taking of auth requests that are received via websockets to be acted on when the user loads up a client. See diagram for the high level picture of how this is wired up. diff --git a/libs/common/src/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction.ts b/libs/common/src/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction.ts index f45cb34496e..7edae163951 100644 --- a/libs/common/src/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction.ts +++ b/libs/common/src/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction.ts @@ -1,30 +1,50 @@ +import { Observable } from "rxjs"; + import { SystemNotificationEvent } from "@bitwarden/common/platform/system-notifications/system-notifications.service"; import { UserId } from "@bitwarden/user-core"; -export abstract class AuthRequestAnsweringServiceAbstraction { +export abstract class AuthRequestAnsweringService { /** * Tries to either display the dialog for the user or will preserve its data and show it at a * later time. Even in the event the dialog is shown immediately, this will write to global state * so that even if someone closes a window or a popup and comes back, it could be processed later. * Only way to clear out the global state is to respond to the auth request. + * - Implemented on Extension and Desktop. * - * Currently, this is only implemented for browser extension. - * - * @param userId The UserId that the auth request is for. + * @param authRequestUserId The UserId that the auth request is for. * @param authRequestId The id of the auth request that is to be processed. */ - abstract receivedPendingAuthRequest(userId: UserId, authRequestId: string): Promise; + abstract receivedPendingAuthRequest?( + authRequestUserId: UserId, + authRequestId: string, + ): Promise; /** - * When a system notification is clicked, this function is used to process that event. + * Confirms whether or not the user meets the conditions required to show them an + * approval dialog immediately. + * + * @param authRequestUserId the UserId that the auth request is for. + * @returns boolean stating whether or not the user meets conditions + */ + abstract activeUserMeetsConditionsToShowApprovalDialog( + authRequestUserId: UserId, + ): Promise; + + /** + * Sets up listeners for scenarios where the user unlocks and we want to process + * any pending auth requests in state. + * + * @param destroy$ The destroy$ observable from the caller + */ + abstract setupUnlockListenersForProcessingAuthRequests(destroy$: Observable): void; + + /** + * When a system notification is clicked, this method is used to process that event. + * - Implemented on Extension only. + * - Desktop does not implement this method because click handling is already setup in + * electron-main-messaging.service.ts. * * @param event The event passed in. Check initNotificationSubscriptions in main.background.ts. */ abstract handleAuthRequestNotificationClicked(event: SystemNotificationEvent): Promise; - - /** - * Process notifications that have been received but didn't meet the conditions to display the - * approval dialog. - */ - abstract processPendingAuthRequests(): Promise; } diff --git a/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.spec.ts b/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.spec.ts deleted file mode 100644 index a44dde04f5f..00000000000 --- a/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.spec.ts +++ /dev/null @@ -1,142 +0,0 @@ -import { mock, MockProxy } from "jest-mock-extended"; -import { of } from "rxjs"; - -import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; -import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; -import { AuthServerNotificationTags } from "@bitwarden/common/auth/enums/auth-server-notification-tags"; -import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; -import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; -import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; -import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; -import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; -import { ActionsService } from "@bitwarden/common/platform/actions"; -import { - ButtonLocation, - SystemNotificationEvent, - SystemNotificationsService, -} from "@bitwarden/common/platform/system-notifications/system-notifications.service"; -import { mockAccountInfoWith } from "@bitwarden/common/spec"; -import { UserId } from "@bitwarden/user-core"; - -import { AuthRequestAnsweringService } from "./auth-request-answering.service"; -import { PendingAuthRequestsStateService } from "./pending-auth-requests.state"; - -describe("AuthRequestAnsweringService", () => { - let accountService: MockProxy; - let actionService: MockProxy; - let authService: MockProxy; - let i18nService: MockProxy; - let masterPasswordService: any; // MasterPasswordServiceAbstraction has many members; we only use forceSetPasswordReason$ - let messagingService: MockProxy; - let pendingAuthRequestsState: MockProxy; - let platformUtilsService: MockProxy; - let systemNotificationsService: MockProxy; - - let sut: AuthRequestAnsweringService; - - const userId = "9f4c3452-6a45-48af-a7d0-74d3e8b65e4c" as UserId; - - beforeEach(() => { - accountService = mock(); - actionService = mock(); - authService = mock(); - i18nService = mock(); - masterPasswordService = { forceSetPasswordReason$: jest.fn() }; - messagingService = mock(); - pendingAuthRequestsState = mock(); - platformUtilsService = mock(); - systemNotificationsService = mock(); - - // Common defaults - authService.activeAccountStatus$ = of(AuthenticationStatus.Locked); - const accountInfo = mockAccountInfoWith({ - email: "user@example.com", - name: "User", - }); - accountService.activeAccount$ = of({ - id: userId, - ...accountInfo, - }); - accountService.accounts$ = of({ - [userId]: accountInfo, - }); - (masterPasswordService.forceSetPasswordReason$ as jest.Mock).mockReturnValue( - of(ForceSetPasswordReason.None), - ); - platformUtilsService.isPopupOpen.mockResolvedValue(false); - i18nService.t.mockImplementation( - (key: string, p1?: any) => `${key}${p1 != null ? ":" + p1 : ""}`, - ); - systemNotificationsService.create.mockResolvedValue("notif-id"); - - sut = new AuthRequestAnsweringService( - accountService, - actionService, - authService, - i18nService, - masterPasswordService, - messagingService, - pendingAuthRequestsState, - platformUtilsService, - systemNotificationsService, - ); - }); - - describe("handleAuthRequestNotificationClicked", () => { - it("clears notification and opens popup when notification body is clicked", async () => { - const event: SystemNotificationEvent = { - id: "123", - buttonIdentifier: ButtonLocation.NotificationButton, - }; - - await sut.handleAuthRequestNotificationClicked(event); - - expect(systemNotificationsService.clear).toHaveBeenCalledWith({ id: "123" }); - expect(actionService.openPopup).toHaveBeenCalledTimes(1); - }); - - it("does nothing when an optional button is clicked", async () => { - const event: SystemNotificationEvent = { - id: "123", - buttonIdentifier: ButtonLocation.FirstOptionalButton, - }; - - await sut.handleAuthRequestNotificationClicked(event); - - expect(systemNotificationsService.clear).not.toHaveBeenCalled(); - expect(actionService.openPopup).not.toHaveBeenCalled(); - }); - }); - - describe("receivedPendingAuthRequest", () => { - const authRequestId = "req-abc"; - - it("creates a system notification when popup is not open", async () => { - platformUtilsService.isPopupOpen.mockResolvedValue(false); - authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); - - await sut.receivedPendingAuthRequest(userId, authRequestId); - - expect(i18nService.t).toHaveBeenCalledWith("accountAccessRequested"); - expect(i18nService.t).toHaveBeenCalledWith("confirmAccessAttempt", "user@example.com"); - expect(systemNotificationsService.create).toHaveBeenCalledWith({ - id: `${AuthServerNotificationTags.AuthRequest}_${authRequestId}`, - title: "accountAccessRequested", - body: "confirmAccessAttempt:user@example.com", - buttons: [], - }); - }); - - it("does not create a notification when popup is open, user is active, unlocked, and no force set password", async () => { - platformUtilsService.isPopupOpen.mockResolvedValue(true); - authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); - (masterPasswordService.forceSetPasswordReason$ as jest.Mock).mockReturnValue( - of(ForceSetPasswordReason.None), - ); - - await sut.receivedPendingAuthRequest(userId, authRequestId); - - expect(systemNotificationsService.create).not.toHaveBeenCalled(); - }); - }); -}); diff --git a/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.ts b/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.ts deleted file mode 100644 index 834d6ac7bcc..00000000000 --- a/libs/common/src/auth/services/auth-request-answering/auth-request-answering.service.ts +++ /dev/null @@ -1,111 +0,0 @@ -import { firstValueFrom } from "rxjs"; - -import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; -import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; -import { AuthServerNotificationTags } from "@bitwarden/common/auth/enums/auth-server-notification-tags"; -import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; -import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; -import { getOptionalUserId, getUserId } from "@bitwarden/common/auth/services/account.service"; -import { MasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; -import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; -import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; -import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; -import { ActionsService } from "@bitwarden/common/platform/actions"; -import { - ButtonLocation, - SystemNotificationEvent, - SystemNotificationsService, -} from "@bitwarden/common/platform/system-notifications/system-notifications.service"; -import { UserId } from "@bitwarden/user-core"; - -import { AuthRequestAnsweringServiceAbstraction } from "../../abstractions/auth-request-answering/auth-request-answering.service.abstraction"; - -import { - PendingAuthRequestsStateService, - PendingAuthUserMarker, -} from "./pending-auth-requests.state"; - -export class AuthRequestAnsweringService implements AuthRequestAnsweringServiceAbstraction { - constructor( - private readonly accountService: AccountService, - private readonly actionService: ActionsService, - private readonly authService: AuthService, - private readonly i18nService: I18nService, - private readonly masterPasswordService: MasterPasswordServiceAbstraction, - private readonly messagingService: MessagingService, - private readonly pendingAuthRequestsState: PendingAuthRequestsStateService, - private readonly platformUtilsService: PlatformUtilsService, - private readonly systemNotificationsService: SystemNotificationsService, - ) {} - - async receivedPendingAuthRequest(userId: UserId, authRequestId: string): Promise { - const authStatus = await firstValueFrom(this.authService.activeAccountStatus$); - const activeUserId: UserId | null = await firstValueFrom( - this.accountService.activeAccount$.pipe(getOptionalUserId), - ); - const forceSetPasswordReason = await firstValueFrom( - this.masterPasswordService.forceSetPasswordReason$(userId), - ); - const popupOpen = await this.platformUtilsService.isPopupOpen(); - - // Always persist the pending marker for this user to global state. - await this.pendingAuthRequestsState.add(userId); - - // These are the conditions we are looking for to know if the extension is in a state to show - // the approval dialog. - const userIsAvailableToReceiveAuthRequest = - popupOpen && - authStatus === AuthenticationStatus.Unlocked && - activeUserId === userId && - forceSetPasswordReason === ForceSetPasswordReason.None; - - if (!userIsAvailableToReceiveAuthRequest) { - // Get the user's email to include in the system notification - const accounts = await firstValueFrom(this.accountService.accounts$); - const emailForUser = accounts[userId].email; - - await this.systemNotificationsService.create({ - id: `${AuthServerNotificationTags.AuthRequest}_${authRequestId}`, // the underscore is an important delimiter. - title: this.i18nService.t("accountAccessRequested"), - body: this.i18nService.t("confirmAccessAttempt", emailForUser), - buttons: [], - }); - return; - } - - // Popup is open and conditions are met; open dialog immediately for this request - this.messagingService.send("openLoginApproval"); - } - - async handleAuthRequestNotificationClicked(event: SystemNotificationEvent): Promise { - if (event.buttonIdentifier === ButtonLocation.NotificationButton) { - await this.systemNotificationsService.clear({ - id: `${event.id}`, - }); - await this.actionService.openPopup(); - } - } - - async processPendingAuthRequests(): Promise { - // Prune any stale pending requests (older than 15 minutes) - // This comes from GlobalSettings.cs - // public TimeSpan UserRequestExpiration { get; set; } = TimeSpan.FromMinutes(15); - const fifteenMinutesMs = 15 * 60 * 1000; - - await this.pendingAuthRequestsState.pruneOlderThan(fifteenMinutesMs); - - const pendingAuthRequestsInState: PendingAuthUserMarker[] = - (await firstValueFrom(this.pendingAuthRequestsState.getAll$())) ?? []; - - if (pendingAuthRequestsInState.length > 0) { - const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); - const pendingAuthRequestsForActiveUser = pendingAuthRequestsInState.some( - (e) => e.userId === activeUserId, - ); - - if (pendingAuthRequestsForActiveUser) { - this.messagingService.send("openLoginApproval"); - } - } - } -} diff --git a/libs/common/src/auth/services/auth-request-answering/default-auth-request-answering.service.spec.ts b/libs/common/src/auth/services/auth-request-answering/default-auth-request-answering.service.spec.ts new file mode 100644 index 00000000000..624133d1e31 --- /dev/null +++ b/libs/common/src/auth/services/auth-request-answering/default-auth-request-answering.service.spec.ts @@ -0,0 +1,444 @@ +import { mock, MockProxy } from "jest-mock-extended"; +import { BehaviorSubject, of, Subject } from "rxjs"; + +import { Account, AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; +import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { + ButtonLocation, + SystemNotificationEvent, +} from "@bitwarden/common/platform/system-notifications/system-notifications.service"; +import { mockAccountInfoWith } from "@bitwarden/common/spec"; +import { UserId } from "@bitwarden/user-core"; + +import { AuthRequestAnsweringService } from "../../abstractions/auth-request-answering/auth-request-answering.service.abstraction"; + +import { DefaultAuthRequestAnsweringService } from "./default-auth-request-answering.service"; +import { + PendingAuthRequestsStateService, + PendingAuthUserMarker, +} from "./pending-auth-requests.state"; + +describe("DefaultAuthRequestAnsweringService", () => { + let accountService: MockProxy; + let authService: MockProxy; + let masterPasswordService: any; // MasterPasswordServiceAbstraction has many members; we only use forceSetPasswordReason$ + let messagingService: MockProxy; + let pendingAuthRequestsState: MockProxy; + + let sut: AuthRequestAnsweringService; + + const userId = "9f4c3452-6a45-48af-a7d0-74d3e8b65e4c" as UserId; + const userAccountInfo = mockAccountInfoWith({ + name: "User", + email: "user@example.com", + }); + const userAccount: Account = { + id: userId, + ...userAccountInfo, + }; + + const otherUserId = "554c3112-9a75-23af-ab80-8dk3e9bl5i8e" as UserId; + const otherUserAccountInfo = mockAccountInfoWith({ + name: "Other", + email: "other@example.com", + }); + const otherUserAccount: Account = { + id: otherUserId, + ...otherUserAccountInfo, + }; + + beforeEach(() => { + accountService = mock(); + authService = mock(); + masterPasswordService = { + forceSetPasswordReason$: jest.fn().mockReturnValue(of(ForceSetPasswordReason.None)), + }; + messagingService = mock(); + pendingAuthRequestsState = mock(); + + // Common defaults + authService.activeAccountStatus$ = of(AuthenticationStatus.Locked); + accountService.activeAccount$ = of(userAccount); + accountService.accounts$ = of({ + [userId]: userAccountInfo, + [otherUserId]: otherUserAccountInfo, + }); + + sut = new DefaultAuthRequestAnsweringService( + accountService, + authService, + masterPasswordService, + messagingService, + pendingAuthRequestsState, + ); + }); + + describe("activeUserMeetsConditionsToShowApprovalDialog()", () => { + it("should return false if there is no active user", async () => { + // Arrange + accountService.activeAccount$ = of(null); + + // Act + const result = await sut.activeUserMeetsConditionsToShowApprovalDialog(userId); + + // Assert + expect(result).toBe(false); + }); + + it("should return false if the active user is not the intended recipient of the auth request", async () => { + // Arrange + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + const result = await sut.activeUserMeetsConditionsToShowApprovalDialog(otherUserId); + + // Assert + expect(result).toBe(false); + }); + + it("should return false if the active user is not unlocked", async () => { + // Arrange + authService.activeAccountStatus$ = of(AuthenticationStatus.Locked); + + // Act + const result = await sut.activeUserMeetsConditionsToShowApprovalDialog(userId); + + // Assert + expect(result).toBe(false); + }); + + it("should return false if the active user is required to set/change their master password", async () => { + // Arrange + masterPasswordService.forceSetPasswordReason$.mockReturnValue( + of(ForceSetPasswordReason.WeakMasterPassword), + ); + + // Act + const result = await sut.activeUserMeetsConditionsToShowApprovalDialog(userId); + + // Assert + expect(result).toBe(false); + }); + + it("should return true if the active user is the intended recipient of the auth request, unlocked, and not required to set/change their master password", async () => { + // Arrange + authService.activeAccountStatus$ = of(AuthenticationStatus.Unlocked); + + // Act + const result = await sut.activeUserMeetsConditionsToShowApprovalDialog(userId); + + // Assert + expect(result).toBe(true); + }); + }); + + describe("setupUnlockListenersForProcessingAuthRequests()", () => { + let destroy$: Subject; + let activeAccount$: BehaviorSubject; + let activeAccountStatus$: BehaviorSubject; + let authStatusForSubjects: Map>; + let pendingRequestMarkers: PendingAuthUserMarker[]; + + beforeEach(() => { + destroy$ = new Subject(); + activeAccount$ = new BehaviorSubject(userAccount); + activeAccountStatus$ = new BehaviorSubject(AuthenticationStatus.Locked); + authStatusForSubjects = new Map(); + pendingRequestMarkers = []; + + accountService.activeAccount$ = activeAccount$; + authService.activeAccountStatus$ = activeAccountStatus$; + authService.authStatusFor$.mockImplementation((id: UserId) => { + if (!authStatusForSubjects.has(id)) { + authStatusForSubjects.set(id, new BehaviorSubject(AuthenticationStatus.Locked)); + } + return authStatusForSubjects.get(id)!; + }); + + pendingAuthRequestsState.getAll$.mockReturnValue(of([])); + }); + + afterEach(() => { + destroy$.next(); + destroy$.complete(); + }); + + describe("active account switching", () => { + it("should process pending auth requests when switching to an unlocked user", async () => { + // Arrange + authStatusForSubjects.set(otherUserId, new BehaviorSubject(AuthenticationStatus.Unlocked)); + pendingRequestMarkers = [{ userId: otherUserId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + + // Simulate account switching to an Unlocked account + activeAccount$.next(otherUserAccount); + + // Assert + await new Promise((resolve) => setTimeout(resolve, 0)); // Allows observable chain to complete before assertion + expect(messagingService.send).toHaveBeenCalledWith("openLoginApproval"); + }); + + it("should NOT process pending auth requests when switching to a locked user", async () => { + // Arrange + authStatusForSubjects.set(otherUserId, new BehaviorSubject(AuthenticationStatus.Locked)); + pendingRequestMarkers = [{ userId: otherUserId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + activeAccount$.next(otherUserAccount); + + // Assert + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(messagingService.send).not.toHaveBeenCalled(); + }); + + it("should NOT process pending auth requests when switching to a logged out user", async () => { + // Arrange + authStatusForSubjects.set(otherUserId, new BehaviorSubject(AuthenticationStatus.LoggedOut)); + pendingRequestMarkers = [{ userId: otherUserId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + activeAccount$.next(otherUserAccount); + + // Assert + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(messagingService.send).not.toHaveBeenCalled(); + }); + + it("should NOT process pending auth requests when active account becomes null", async () => { + // Arrange + pendingRequestMarkers = [{ userId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + activeAccount$.next(null); + + // Assert + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(messagingService.send).not.toHaveBeenCalled(); + }); + + it("should handle multiple user switches correctly", async () => { + // Arrange + authStatusForSubjects.set(userId, new BehaviorSubject(AuthenticationStatus.Locked)); + authStatusForSubjects.set(otherUserId, new BehaviorSubject(AuthenticationStatus.Unlocked)); + pendingRequestMarkers = [{ userId: otherUserId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + + // Switch to unlocked user (should trigger) + activeAccount$.next(otherUserAccount); + await new Promise((resolve) => setTimeout(resolve, 0)); + + // Switch to locked user (should NOT trigger) + activeAccount$.next(userAccount); + await new Promise((resolve) => setTimeout(resolve, 0)); + + // Assert + expect(messagingService.send).toHaveBeenCalledTimes(1); + }); + + it("should NOT process pending auth requests when switching to an Unlocked user who is required to set/change their master password", async () => { + // Arrange + masterPasswordService.forceSetPasswordReason$.mockReturnValue( + of(ForceSetPasswordReason.WeakMasterPassword), + ); + authStatusForSubjects.set(otherUserId, new BehaviorSubject(AuthenticationStatus.Unlocked)); + pendingRequestMarkers = [{ userId: otherUserId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + activeAccount$.next(otherUserAccount); + + // Assert + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(messagingService.send).not.toHaveBeenCalled(); + }); + }); + + describe("authentication status transitions", () => { + it("should process pending auth requests when active account transitions to Unlocked", async () => { + // Arrange + activeAccountStatus$.next(AuthenticationStatus.Locked); + pendingRequestMarkers = [{ userId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + activeAccountStatus$.next(AuthenticationStatus.Unlocked); + + // Assert + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(messagingService.send).toHaveBeenCalledWith("openLoginApproval"); + }); + + it("should process pending auth requests when transitioning from LoggedOut to Unlocked", async () => { + // Arrange + activeAccountStatus$.next(AuthenticationStatus.LoggedOut); + pendingRequestMarkers = [{ userId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + activeAccountStatus$.next(AuthenticationStatus.Unlocked); + + // Assert + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(messagingService.send).toHaveBeenCalledWith("openLoginApproval"); + }); + + it("should NOT process pending auth requests when transitioning from Unlocked to Locked", async () => { + // Arrange + activeAccountStatus$.next(AuthenticationStatus.Unlocked); + pendingRequestMarkers = [{ userId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + await new Promise((resolve) => setTimeout(resolve, 0)); + + // Clear any calls from the initial trigger (from null -> Unlocked) + messagingService.send.mockClear(); + + activeAccountStatus$.next(AuthenticationStatus.Locked); + + // Assert + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(messagingService.send).not.toHaveBeenCalled(); + }); + + it("should NOT process pending auth requests when transitioning from Locked to LoggedOut", async () => { + // Arrange + activeAccountStatus$.next(AuthenticationStatus.Locked); + pendingRequestMarkers = [{ userId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + activeAccountStatus$.next(AuthenticationStatus.LoggedOut); + + // Assert + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(messagingService.send).not.toHaveBeenCalled(); + }); + + it("should NOT process pending auth requests when staying in Unlocked status", async () => { + // Arrange + activeAccountStatus$.next(AuthenticationStatus.Unlocked); + pendingRequestMarkers = [{ userId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + await new Promise((resolve) => setTimeout(resolve, 0)); + + // Clear any calls from the initial trigger (from null -> Unlocked) + messagingService.send.mockClear(); + + activeAccountStatus$.next(AuthenticationStatus.Unlocked); + + // Assert + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(messagingService.send).not.toHaveBeenCalled(); + }); + + it("should handle multiple status transitions correctly", async () => { + // Arrange + activeAccountStatus$.next(AuthenticationStatus.Locked); + pendingRequestMarkers = [{ userId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + + // Transition to Unlocked (should trigger) + activeAccountStatus$.next(AuthenticationStatus.Unlocked); + await new Promise((resolve) => setTimeout(resolve, 0)); + + // Transition to Locked (should NOT trigger) + activeAccountStatus$.next(AuthenticationStatus.Locked); + await new Promise((resolve) => setTimeout(resolve, 0)); + + // Transition back to Unlocked (should trigger again) + activeAccountStatus$.next(AuthenticationStatus.Unlocked); + await new Promise((resolve) => setTimeout(resolve, 0)); + + // Assert + expect(messagingService.send).toHaveBeenCalledTimes(2); + expect(messagingService.send).toHaveBeenCalledWith("openLoginApproval"); + }); + + it("should NOT process pending auth requests when active account transitions to Unlocked but is required to set/change their master password", async () => { + // Arrange + masterPasswordService.forceSetPasswordReason$.mockReturnValue( + of(ForceSetPasswordReason.WeakMasterPassword), + ); + activeAccountStatus$.next(AuthenticationStatus.Locked); + pendingRequestMarkers = [{ userId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + activeAccountStatus$.next(AuthenticationStatus.Unlocked); + + // Assert + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(messagingService.send).not.toHaveBeenCalled(); + }); + }); + + describe("subscription cleanup", () => { + it("should stop processing when destroy$ emits", async () => { + // Arrange + authStatusForSubjects.set(otherUserId, new BehaviorSubject(AuthenticationStatus.Unlocked)); + pendingRequestMarkers = [{ userId: otherUserId, receivedAtMs: Date.now() }]; + pendingAuthRequestsState.getAll$.mockReturnValue(of(pendingRequestMarkers)); + + // Act + sut.setupUnlockListenersForProcessingAuthRequests(destroy$); + + // Emit destroy signal + destroy$.next(); + + // Try to trigger processing after cleanup + activeAccount$.next(otherUserAccount); + activeAccountStatus$.next(AuthenticationStatus.Unlocked); + + // Assert + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(messagingService.send).not.toHaveBeenCalled(); + }); + }); + }); + + describe("handleAuthRequestNotificationClicked()", () => { + it("should throw an error", async () => { + // Arrange + const event: SystemNotificationEvent = { + id: "123", + buttonIdentifier: ButtonLocation.NotificationButton, + }; + + // Act + const promise = sut.handleAuthRequestNotificationClicked(event); + + // Assert + await expect(promise).rejects.toThrow( + "handleAuthRequestNotificationClicked() not implemented for this client", + ); + }); + }); +}); diff --git a/libs/common/src/auth/services/auth-request-answering/default-auth-request-answering.service.ts b/libs/common/src/auth/services/auth-request-answering/default-auth-request-answering.service.ts new file mode 100644 index 00000000000..ea8067ed351 --- /dev/null +++ b/libs/common/src/auth/services/auth-request-answering/default-auth-request-answering.service.ts @@ -0,0 +1,140 @@ +import { + distinctUntilChanged, + filter, + firstValueFrom, + map, + Observable, + pairwise, + startWith, + switchMap, + take, + takeUntil, + tap, +} from "rxjs"; + +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; +import { getOptionalUserId, getUserId } from "@bitwarden/common/auth/services/account.service"; +import { MasterPasswordServiceAbstraction } from "@bitwarden/common/key-management/master-password/abstractions/master-password.service.abstraction"; +import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { SystemNotificationEvent } from "@bitwarden/common/platform/system-notifications/system-notifications.service"; +import { UserId } from "@bitwarden/user-core"; + +import { AuthRequestAnsweringService } from "../../abstractions/auth-request-answering/auth-request-answering.service.abstraction"; + +import { + PendingAuthRequestsStateService, + PendingAuthUserMarker, +} from "./pending-auth-requests.state"; + +export class DefaultAuthRequestAnsweringService implements AuthRequestAnsweringService { + constructor( + protected readonly accountService: AccountService, + protected readonly authService: AuthService, + protected readonly masterPasswordService: MasterPasswordServiceAbstraction, + protected readonly messagingService: MessagingService, + protected readonly pendingAuthRequestsState: PendingAuthRequestsStateService, + ) {} + + async activeUserMeetsConditionsToShowApprovalDialog(authRequestUserId: UserId): Promise { + // If the active user is not the intended recipient of the auth request, return false + const activeUserId: UserId | null = await firstValueFrom( + this.accountService.activeAccount$.pipe(getOptionalUserId), + ); + if (activeUserId !== authRequestUserId) { + return false; + } + + // If the active user is not unlocked, return false + const authStatus = await firstValueFrom(this.authService.activeAccountStatus$); + if (authStatus !== AuthenticationStatus.Unlocked) { + return false; + } + + // If the active user is required to set/change their master password, return false + // Note that by this point we know that the authRequestUserId is the active UserId (see check above) + const forceSetPasswordReason = await firstValueFrom( + this.masterPasswordService.forceSetPasswordReason$(authRequestUserId), + ); + if (forceSetPasswordReason !== ForceSetPasswordReason.None) { + return false; + } + + // User meets conditions: they are the intended recipient, unlocked, and not required to set/change their master password + return true; + } + + setupUnlockListenersForProcessingAuthRequests(destroy$: Observable): void { + // When account switching to a user who is Unlocked, process any pending auth requests. + this.accountService.activeAccount$ + .pipe( + map((a) => a?.id), // Extract active userId + distinctUntilChanged(), // Only when userId actually changes + filter((userId) => userId != null), // Require a valid userId + switchMap((userId) => this.authService.authStatusFor$(userId).pipe(take(1))), // Get current auth status once for new user + filter((status) => status === AuthenticationStatus.Unlocked), // Only when the new user is Unlocked + tap(() => { + void this.processPendingAuthRequests(); + }), + takeUntil(destroy$), + ) + .subscribe(); + + // When the active account transitions TO Unlocked, process any pending auth requests. + this.authService.activeAccountStatus$ + .pipe( + startWith(null as unknown as AuthenticationStatus), // Seed previous value to handle initial emission + pairwise(), // Compare previous and current statuses + filter( + ([prev, curr]) => + prev !== AuthenticationStatus.Unlocked && curr === AuthenticationStatus.Unlocked, // Fire on transitions into Unlocked (incl. initial) + ), + takeUntil(destroy$), + ) + .subscribe(() => { + void this.processPendingAuthRequests(); + }); + } + + /** + * Process notifications that have been received but didn't meet the conditions to display the + * approval dialog. + */ + private async processPendingAuthRequests(): Promise { + const activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); + + // Only continue if the active user is not required to set/change their master password + const forceSetPasswordReason = await firstValueFrom( + this.masterPasswordService.forceSetPasswordReason$(activeUserId), + ); + if (forceSetPasswordReason !== ForceSetPasswordReason.None) { + return; + } + + // Prune any stale pending requests (older than 15 minutes) + // This comes from GlobalSettings.cs + // public TimeSpan UserRequestExpiration { get; set; } = TimeSpan.FromMinutes(15); + const fifteenMinutesMs = 15 * 60 * 1000; + + await this.pendingAuthRequestsState.pruneOlderThan(fifteenMinutesMs); + + const pendingAuthRequestsInState: PendingAuthUserMarker[] = + (await firstValueFrom(this.pendingAuthRequestsState.getAll$())) ?? []; + + if (pendingAuthRequestsInState.length > 0) { + const pendingAuthRequestsForActiveUser = pendingAuthRequestsInState.some( + (e) => e.userId === activeUserId, + ); + + if (pendingAuthRequestsForActiveUser) { + this.messagingService.send("openLoginApproval"); + } + } + } + + async handleAuthRequestNotificationClicked(event: SystemNotificationEvent): Promise { + throw new Error("handleAuthRequestNotificationClicked() not implemented for this client"); + } +} diff --git a/libs/common/src/auth/services/auth-request-answering/noop-auth-request-answering.service.ts b/libs/common/src/auth/services/auth-request-answering/noop-auth-request-answering.service.ts index 730362adfed..1d858516fe4 100644 --- a/libs/common/src/auth/services/auth-request-answering/noop-auth-request-answering.service.ts +++ b/libs/common/src/auth/services/auth-request-answering/noop-auth-request-answering.service.ts @@ -1,14 +1,22 @@ import { SystemNotificationEvent } from "@bitwarden/common/platform/system-notifications/system-notifications.service"; import { UserId } from "@bitwarden/user-core"; -import { AuthRequestAnsweringServiceAbstraction } from "../../abstractions/auth-request-answering/auth-request-answering.service.abstraction"; +import { AuthRequestAnsweringService } from "../../abstractions/auth-request-answering/auth-request-answering.service.abstraction"; -export class NoopAuthRequestAnsweringService implements AuthRequestAnsweringServiceAbstraction { - constructor() {} +export class NoopAuthRequestAnsweringService implements AuthRequestAnsweringService { + async activeUserMeetsConditionsToShowApprovalDialog(authRequestUserId: UserId): Promise { + throw new Error( + "activeUserMeetsConditionsToShowApprovalDialog() not implemented for this client", + ); + } - async receivedPendingAuthRequest(userId: UserId, notificationId: string): Promise {} + setupUnlockListenersForProcessingAuthRequests(): void { + throw new Error( + "setupUnlockListenersForProcessingAuthRequests() not implemented for this client", + ); + } - async handleAuthRequestNotificationClicked(event: SystemNotificationEvent): Promise {} - - async processPendingAuthRequests(): Promise {} + async handleAuthRequestNotificationClicked(event: SystemNotificationEvent): Promise { + throw new Error("handleAuthRequestNotificationClicked() not implemented for this client"); + } } diff --git a/libs/common/src/platform/server-notifications/internal/default-server-notifications.multiuser.spec.ts b/libs/common/src/platform/server-notifications/internal/default-server-notifications.multiuser.spec.ts index 7aacc783e65..2795e4c3003 100644 --- a/libs/common/src/platform/server-notifications/internal/default-server-notifications.multiuser.spec.ts +++ b/libs/common/src/platform/server-notifications/internal/default-server-notifications.multiuser.spec.ts @@ -4,7 +4,7 @@ import { BehaviorSubject, bufferCount, firstValueFrom, Subject, ObservedValueOf // eslint-disable-next-line no-restricted-imports import { LogoutReason } from "@bitwarden/auth/common"; import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; -import { AuthRequestAnsweringServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; +import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; import { mockAccountInfoWith } from "../../../../spec"; import { AccountService } from "../../../auth/abstractions/account.service"; @@ -33,7 +33,7 @@ describe("DefaultServerNotificationsService (multi-user)", () => { let signalRNotificationConnectionService: MockProxy; let authService: MockProxy; let webPushNotificationConnectionService: MockProxy; - let authRequestAnsweringService: MockProxy; + let authRequestAnsweringService: MockProxy; let configService: MockProxy; let policyService: MockProxy; @@ -127,7 +127,7 @@ describe("DefaultServerNotificationsService (multi-user)", () => { return webPushSupportStatusByUser.get(userId)!.asObservable(); }); - authRequestAnsweringService = mock(); + authRequestAnsweringService = mock(); policyService = mock(); @@ -270,13 +270,13 @@ describe("DefaultServerNotificationsService (multi-user)", () => { // allow async queue to drain await new Promise((resolve) => setTimeout(resolve, 0)); - expect(messagingService.send).toHaveBeenCalledWith("openLoginApproval", { - notificationId: "auth-id-2", - }); + // When authRequestAnsweringService.receivedPendingAuthRequest exists (Extension/Desktop), + // only that method is called. messagingService.send is only called for Web (NoopAuthRequestAnsweringService). expect(authRequestAnsweringService.receivedPendingAuthRequest).toHaveBeenCalledWith( mockUserId2, "auth-id-2", ); + expect(messagingService.send).not.toHaveBeenCalled(); subscription.unsubscribe(); }); diff --git a/libs/common/src/platform/server-notifications/internal/default-server-notifications.service.spec.ts b/libs/common/src/platform/server-notifications/internal/default-server-notifications.service.spec.ts index 9c84981b7f9..f058e8794ac 100644 --- a/libs/common/src/platform/server-notifications/internal/default-server-notifications.service.spec.ts +++ b/libs/common/src/platform/server-notifications/internal/default-server-notifications.service.spec.ts @@ -6,7 +6,7 @@ import { BehaviorSubject, bufferCount, firstValueFrom, ObservedValueOf, of, Subj import { LogoutReason } from "@bitwarden/auth/common"; import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; -import { AuthRequestAnsweringServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; +import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; import { awaitAsync, mockAccountInfoWith } from "../../../../spec"; import { Matrix } from "../../../../spec/matrix"; @@ -42,7 +42,7 @@ describe("NotificationsService", () => { let signalRNotificationConnectionService: MockProxy; let authService: MockProxy; let webPushNotificationConnectionService: MockProxy; - let authRequestAnsweringService: MockProxy; + let authRequestAnsweringService: MockProxy; let configService: MockProxy; let policyService: MockProxy; @@ -72,7 +72,7 @@ describe("NotificationsService", () => { signalRNotificationConnectionService = mock(); authService = mock(); webPushNotificationConnectionService = mock(); - authRequestAnsweringService = mock(); + authRequestAnsweringService = mock(); configService = mock(); policyService = mock(); @@ -471,5 +471,41 @@ describe("NotificationsService", () => { ); }); }); + + describe("NotificationType.AuthRequest", () => { + it("should call receivedPendingAuthRequest when it exists (Extension/Desktop)", async () => { + authRequestAnsweringService.receivedPendingAuthRequest!.mockResolvedValue(undefined as any); + + const notification = new NotificationResponse({ + type: NotificationType.AuthRequest, + payload: { userId: mockUser1, id: "auth-request-123" }, + contextId: "different-app-id", + }); + + await sut["processNotification"](notification, mockUser1); + + expect(authRequestAnsweringService.receivedPendingAuthRequest).toHaveBeenCalledWith( + mockUser1, + "auth-request-123", + ); + expect(messagingService.send).not.toHaveBeenCalled(); + }); + + it("should call messagingService.send when receivedPendingAuthRequest does not exist (Web)", async () => { + authRequestAnsweringService.receivedPendingAuthRequest = undefined as any; + + const notification = new NotificationResponse({ + type: NotificationType.AuthRequest, + payload: { userId: mockUser1, id: "auth-request-456" }, + contextId: "different-app-id", + }); + + await sut["processNotification"](notification, mockUser1); + + expect(messagingService.send).toHaveBeenCalledWith("openLoginApproval", { + notificationId: "auth-request-456", + }); + }); + }); }); }); diff --git a/libs/common/src/platform/server-notifications/internal/default-server-notifications.service.ts b/libs/common/src/platform/server-notifications/internal/default-server-notifications.service.ts index 5b026add1a2..83ea12bf154 100644 --- a/libs/common/src/platform/server-notifications/internal/default-server-notifications.service.ts +++ b/libs/common/src/platform/server-notifications/internal/default-server-notifications.service.ts @@ -17,7 +17,7 @@ import { import { LogoutReason } from "@bitwarden/auth/common"; import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { PolicyData } from "@bitwarden/common/admin-console/models/data/policy.data"; -import { AuthRequestAnsweringServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; +import { AuthRequestAnsweringService } from "@bitwarden/common/auth/abstractions/auth-request-answering/auth-request-answering.service.abstraction"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { trackedMerge } from "@bitwarden/common/platform/misc"; @@ -67,7 +67,7 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer private readonly signalRConnectionService: SignalRConnectionService, private readonly authService: AuthService, private readonly webPushConnectionService: WebPushConnectionService, - private readonly authRequestAnsweringService: AuthRequestAnsweringServiceAbstraction, + private readonly authRequestAnsweringService: AuthRequestAnsweringService, private readonly configService: ConfigService, private readonly policyService: InternalPolicyService, ) { @@ -250,26 +250,28 @@ export class DefaultServerNotificationsService implements ServerNotificationsSer case NotificationType.SyncSendDelete: await this.syncService.syncDeleteSend(notification.payload as SyncSendNotification); break; - case NotificationType.AuthRequest: - await this.authRequestAnsweringService.receivedPendingAuthRequest( - notification.payload.userId, - notification.payload.id, - ); - - /** - * This call is necessary for Desktop, which for the time being uses a noop for the - * authRequestAnsweringService.receivedPendingAuthRequest() call just above. Desktop - * will eventually use the new AuthRequestAnsweringService, at which point we can remove - * this second call. - * - * The Extension AppComponent has logic (see processingPendingAuth) that only allows one - * pending auth request to process at a time, so this second call will not cause any - * duplicate processing conflicts on Extension. - */ - this.messagingService.send("openLoginApproval", { - notificationId: notification.payload.id, - }); + case NotificationType.AuthRequest: { + // Only Extension and Desktop implement the AuthRequestAnsweringService + if (this.authRequestAnsweringService.receivedPendingAuthRequest) { + try { + await this.authRequestAnsweringService.receivedPendingAuthRequest( + notification.payload.userId, + notification.payload.id, + ); + } catch (error) { + this.logService.error(`Failed to process auth request notification: ${error}`); + } + } else { + // This call is necessary for Web, which uses a NoopAuthRequestAnsweringService + // that does not have a receivedPendingAuthRequest() method + this.messagingService.send("openLoginApproval", { + // Include the authRequestId so the DeviceManagementComponent can upsert the correct device. + // This will only matter if the user is on the /device-management screen when the auth request is received. + notificationId: notification.payload.id, + }); + } break; + } case NotificationType.SyncOrganizationStatusChanged: await this.syncService.fullSync(true); break;