From 788bef6b7a35f0a1e24fa086fddcac94d293f06e Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 26 Apr 2024 07:04:21 +0000 Subject: [PATCH 01/41] Autosync the updated translations (#8924) Co-authored-by: bitwarden-devops-bot <106330231+bitwarden-devops-bot@users.noreply.github.com> --- apps/desktop/src/locales/fr/messages.json | 4 ++-- apps/desktop/src/locales/hu/messages.json | 2 +- apps/desktop/src/locales/lv/messages.json | 2 +- apps/desktop/src/locales/nl/messages.json | 2 +- apps/desktop/src/locales/zh_CN/messages.json | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/apps/desktop/src/locales/fr/messages.json b/apps/desktop/src/locales/fr/messages.json index 86550b736f..e82420a1f5 100644 --- a/apps/desktop/src/locales/fr/messages.json +++ b/apps/desktop/src/locales/fr/messages.json @@ -1636,7 +1636,7 @@ "message": "Error enabling browser integration" }, "browserIntegrationErrorDesc": { - "message": "An error has occurred while enabling browser integration." + "message": "Une erreur s'est produite lors de l'action de l'intégration du navigateur." }, "browserIntegrationMasOnlyDesc": { "message": "Malheureusement l'intégration avec le navigateur est uniquement supportée dans la version Mac App Store pour le moment." @@ -2698,7 +2698,7 @@ "description": "Label indicating the most common import formats" }, "success": { - "message": "Success" + "message": "Succès" }, "troubleshooting": { "message": "Résolution de problèmes" diff --git a/apps/desktop/src/locales/hu/messages.json b/apps/desktop/src/locales/hu/messages.json index 5c91fb4b94..838b3fc7c8 100644 --- a/apps/desktop/src/locales/hu/messages.json +++ b/apps/desktop/src/locales/hu/messages.json @@ -2698,7 +2698,7 @@ "description": "Label indicating the most common import formats" }, "success": { - "message": "Success" + "message": "Sikeres" }, "troubleshooting": { "message": "Hibaelhárítás" diff --git a/apps/desktop/src/locales/lv/messages.json b/apps/desktop/src/locales/lv/messages.json index aa057f54ab..e2e068362e 100644 --- a/apps/desktop/src/locales/lv/messages.json +++ b/apps/desktop/src/locales/lv/messages.json @@ -2698,7 +2698,7 @@ "description": "Label indicating the most common import formats" }, "success": { - "message": "Success" + "message": "Izdevās" }, "troubleshooting": { "message": "Sarežģījumu novēršana" diff --git a/apps/desktop/src/locales/nl/messages.json b/apps/desktop/src/locales/nl/messages.json index b5f2a413d6..f56572259b 100644 --- a/apps/desktop/src/locales/nl/messages.json +++ b/apps/desktop/src/locales/nl/messages.json @@ -2698,7 +2698,7 @@ "description": "Label indicating the most common import formats" }, "success": { - "message": "Success" + "message": "Succes" }, "troubleshooting": { "message": "Probleemoplossing" diff --git a/apps/desktop/src/locales/zh_CN/messages.json b/apps/desktop/src/locales/zh_CN/messages.json index 9837be29e3..aad13e06ef 100644 --- a/apps/desktop/src/locales/zh_CN/messages.json +++ b/apps/desktop/src/locales/zh_CN/messages.json @@ -2698,7 +2698,7 @@ "description": "Label indicating the most common import formats" }, "success": { - "message": "Success" + "message": "成功" }, "troubleshooting": { "message": "故障排除" From c7fa376be36a865b5ed9a6d758e9a80ce07a0ca8 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 26 Apr 2024 07:05:43 +0000 Subject: [PATCH 02/41] Autosync the updated translations (#8926) Co-authored-by: bitwarden-devops-bot <106330231+bitwarden-devops-bot@users.noreply.github.com> --- apps/web/src/locales/ca/messages.json | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/apps/web/src/locales/ca/messages.json b/apps/web/src/locales/ca/messages.json index bf3071b506..0f958846cd 100644 --- a/apps/web/src/locales/ca/messages.json +++ b/apps/web/src/locales/ca/messages.json @@ -7748,31 +7748,31 @@ "description": "A machine user which can be used to automate processes and access secrets in the system." }, "machineAccounts": { - "message": "Machine accounts", + "message": "Compte de màquina", "description": "The title for the section that deals with machine accounts." }, "newMachineAccount": { - "message": "New machine account", + "message": "Compte nou de màquina", "description": "Title for creating a new machine account." }, "machineAccountsNoItemsMessage": { - "message": "Create a new machine account to get started automating secret access.", + "message": "Crea un compte de màquina nou per començar a automatitzar l'accés secret.", "description": "Message to encourage the user to start creating machine accounts." }, "machineAccountsNoItemsTitle": { - "message": "Nothing to show yet", + "message": "Encara no hi ha res a mostrar", "description": "Title to indicate that there are no machine accounts to display." }, "deleteMachineAccounts": { - "message": "Delete machine accounts", + "message": "Suprimeix els comptes de màquina", "description": "Title for the action to delete one or multiple machine accounts." }, "deleteMachineAccount": { - "message": "Delete machine account", + "message": "Suprimeix comptes de màquina", "description": "Title for the action to delete a single machine account." }, "viewMachineAccount": { - "message": "View machine account", + "message": "Veure el compte de màquina", "description": "Action to view the details of a machine account." }, "deleteMachineAccountDialogMessage": { From 14b2eb99a2baa74279036b412ad174b31bdc8ef8 Mon Sep 17 00:00:00 2001 From: Oscar Hinton Date: Fri, 26 Apr 2024 12:57:26 +0200 Subject: [PATCH 03/41] [PM-2282] Make feature flags type safe (#8612) Refactors the feature flags in ConfigService to be type safe. It also moves the default value to a centralized location rather than the caller defining it. This ensures consistency across the various places they are used. --- .../fileless-importer.background.ts | 2 +- .../layouts/organization-layout.component.ts | 1 - .../member-dialog/member-dialog.component.ts | 1 - .../settings/account.component.ts | 2 -- .../key-rotation/user-key-rotation.service.ts | 2 +- ...ganization-subscription-cloud.component.ts | 1 - .../src/app/layouts/user-layout.component.ts | 1 - .../collection-dialog.component.ts | 2 +- .../bulk-delete-dialog.component.ts | 1 - .../vault-onboarding.component.ts | 3 +- .../vault/individual-vault/vault.component.ts | 1 - .../vault/org-vault/attachments.component.ts | 2 +- ...-collection-assignment-dialog.component.ts | 5 +-- .../app/vault/org-vault/vault.component.ts | 1 - .../providers/clients/clients.component.ts | 1 - .../providers/providers-layout.component.ts | 2 -- .../providers/settings/account.component.ts | 1 - .../providers/setup/setup.component.ts | 2 -- .../directives/if-feature.directive.spec.ts | 8 ++--- .../src/directives/if-feature.directive.ts | 4 +-- .../platform/guard/feature-flag.guard.spec.ts | 8 ++--- .../vault/components/add-edit.component.ts | 1 - libs/common/src/enums/feature-flag.enum.ts | 36 +++++++++++++++++-- .../abstractions/config/config.service.ts | 14 ++------ .../abstractions/config/server-config.ts | 3 +- .../models/data/server-config.data.ts | 3 +- .../services/config/default-config.service.ts | 17 +++++---- 27 files changed, 67 insertions(+), 58 deletions(-) diff --git a/apps/browser/src/tools/background/fileless-importer.background.ts b/apps/browser/src/tools/background/fileless-importer.background.ts index 07c6408e8d..fed5541f52 100644 --- a/apps/browser/src/tools/background/fileless-importer.background.ts +++ b/apps/browser/src/tools/background/fileless-importer.background.ts @@ -183,7 +183,7 @@ class FilelessImporterBackground implements FilelessImporterBackgroundInterface return; } - const filelessImportFeatureFlagEnabled = await this.configService.getFeatureFlag( + const filelessImportFeatureFlagEnabled = await this.configService.getFeatureFlag( FeatureFlag.BrowserFilelessImport, ); const userAuthStatus = await this.authService.getAuthStatus(); diff --git a/apps/web/src/app/admin-console/organizations/layouts/organization-layout.component.ts b/apps/web/src/app/admin-console/organizations/layouts/organization-layout.component.ts index b1a84c22f3..7de0c98cd5 100644 --- a/apps/web/src/app/admin-console/organizations/layouts/organization-layout.component.ts +++ b/apps/web/src/app/admin-console/organizations/layouts/organization-layout.component.ts @@ -58,7 +58,6 @@ export class OrganizationLayoutComponent implements OnInit, OnDestroy { protected showPaymentMethodWarningBanners$ = this.configService.getFeatureFlag$( FeatureFlag.ShowPaymentMethodWarningBanners, - false, ); constructor( diff --git a/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.ts b/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.ts index f1af950650..3cccd6e28f 100644 --- a/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.ts +++ b/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.ts @@ -218,7 +218,6 @@ export class MemberDialogComponent implements OnDestroy { groups: groups$, flexibleCollectionsV1Enabled: this.configService.getFeatureFlag$( FeatureFlag.FlexibleCollectionsV1, - false, ), }) .pipe(takeUntil(this.destroy$)) diff --git a/apps/web/src/app/admin-console/organizations/settings/account.component.ts b/apps/web/src/app/admin-console/organizations/settings/account.component.ts index 1ce05f7a30..d8091e46ae 100644 --- a/apps/web/src/app/admin-console/organizations/settings/account.component.ts +++ b/apps/web/src/app/admin-console/organizations/settings/account.component.ts @@ -44,12 +44,10 @@ export class AccountComponent { protected flexibleCollectionsMigrationEnabled$ = this.configService.getFeatureFlag$( FeatureFlag.FlexibleCollectionsMigration, - false, ); flexibleCollectionsV1Enabled$ = this.configService.getFeatureFlag$( FeatureFlag.FlexibleCollectionsV1, - false, ); // FormGroup validators taken from server Organization domain object diff --git a/apps/web/src/app/auth/key-rotation/user-key-rotation.service.ts b/apps/web/src/app/auth/key-rotation/user-key-rotation.service.ts index 94c6208115..dc5f933724 100644 --- a/apps/web/src/app/auth/key-rotation/user-key-rotation.service.ts +++ b/apps/web/src/app/auth/key-rotation/user-key-rotation.service.ts @@ -90,7 +90,7 @@ export class UserKeyRotationService { request.emergencyAccessKeys = await this.emergencyAccessService.getRotatedKeys(newUserKey); request.resetPasswordKeys = await this.resetPasswordService.getRotatedKeys(newUserKey); - if (await this.configService.getFeatureFlag(FeatureFlag.KeyRotationImprovements)) { + if (await this.configService.getFeatureFlag(FeatureFlag.KeyRotationImprovements)) { await this.apiService.postUserKeyUpdate(request); } else { await this.rotateUserKeyAndEncryptedDataLegacy(request); diff --git a/apps/web/src/app/billing/organizations/organization-subscription-cloud.component.ts b/apps/web/src/app/billing/organizations/organization-subscription-cloud.component.ts index 9326359bd8..477032deba 100644 --- a/apps/web/src/app/billing/organizations/organization-subscription-cloud.component.ts +++ b/apps/web/src/app/billing/organizations/organization-subscription-cloud.component.ts @@ -84,7 +84,6 @@ export class OrganizationSubscriptionCloudComponent implements OnInit, OnDestroy this.showUpdatedSubscriptionStatusSection$ = this.configService.getFeatureFlag$( FeatureFlag.AC1795_UpdatedSubscriptionStatusSection, - false, ); } diff --git a/apps/web/src/app/layouts/user-layout.component.ts b/apps/web/src/app/layouts/user-layout.component.ts index fea0352867..eb507bd997 100644 --- a/apps/web/src/app/layouts/user-layout.component.ts +++ b/apps/web/src/app/layouts/user-layout.component.ts @@ -40,7 +40,6 @@ export class UserLayoutComponent implements OnInit { protected showPaymentMethodWarningBanners$ = this.configService.getFeatureFlag$( FeatureFlag.ShowPaymentMethodWarningBanners, - false, ); constructor( diff --git a/apps/web/src/app/vault/components/collection-dialog/collection-dialog.component.ts b/apps/web/src/app/vault/components/collection-dialog/collection-dialog.component.ts index 8e0d610c93..4e95bb4bcc 100644 --- a/apps/web/src/app/vault/components/collection-dialog/collection-dialog.component.ts +++ b/apps/web/src/app/vault/components/collection-dialog/collection-dialog.component.ts @@ -76,7 +76,7 @@ export enum CollectionDialogAction { }) export class CollectionDialogComponent implements OnInit, OnDestroy { protected flexibleCollectionsV1Enabled$ = this.configService - .getFeatureFlag$(FeatureFlag.FlexibleCollectionsV1, false) + .getFeatureFlag$(FeatureFlag.FlexibleCollectionsV1) .pipe(first()); private destroy$ = new Subject(); diff --git a/apps/web/src/app/vault/individual-vault/bulk-action-dialogs/bulk-delete-dialog/bulk-delete-dialog.component.ts b/apps/web/src/app/vault/individual-vault/bulk-action-dialogs/bulk-delete-dialog/bulk-delete-dialog.component.ts index 4050823a6d..a678a05ae3 100644 --- a/apps/web/src/app/vault/individual-vault/bulk-action-dialogs/bulk-delete-dialog/bulk-delete-dialog.component.ts +++ b/apps/web/src/app/vault/individual-vault/bulk-action-dialogs/bulk-delete-dialog/bulk-delete-dialog.component.ts @@ -54,7 +54,6 @@ export class BulkDeleteDialogComponent { private flexibleCollectionsV1Enabled$ = this.configService.getFeatureFlag$( FeatureFlag.FlexibleCollectionsV1, - false, ); constructor( diff --git a/apps/web/src/app/vault/individual-vault/vault-onboarding/vault-onboarding.component.ts b/apps/web/src/app/vault/individual-vault/vault-onboarding/vault-onboarding.component.ts index dc3a41cf15..90af89e60e 100644 --- a/apps/web/src/app/vault/individual-vault/vault-onboarding/vault-onboarding.component.ts +++ b/apps/web/src/app/vault/individual-vault/vault-onboarding/vault-onboarding.component.ts @@ -60,9 +60,8 @@ export class VaultOnboardingComponent implements OnInit, OnChanges, OnDestroy { ) {} async ngOnInit() { - this.showOnboardingAccess$ = await this.configService.getFeatureFlag$( + this.showOnboardingAccess$ = await this.configService.getFeatureFlag$( FeatureFlag.VaultOnboarding, - false, ); this.onboardingTasks$ = this.vaultOnboardingService.vaultOnboardingState$; await this.setOnboardingTasks(); diff --git a/apps/web/src/app/vault/individual-vault/vault.component.ts b/apps/web/src/app/vault/individual-vault/vault.component.ts index c97dd93d76..b956a90445 100644 --- a/apps/web/src/app/vault/individual-vault/vault.component.ts +++ b/apps/web/src/app/vault/individual-vault/vault.component.ts @@ -148,7 +148,6 @@ export class VaultComponent implements OnInit, OnDestroy { protected currentSearchText$: Observable; protected flexibleCollectionsV1Enabled$ = this.configService.getFeatureFlag$( FeatureFlag.FlexibleCollectionsV1, - false, ); private searchText$ = new Subject(); diff --git a/apps/web/src/app/vault/org-vault/attachments.component.ts b/apps/web/src/app/vault/org-vault/attachments.component.ts index cf1f0796ec..2aecf277e6 100644 --- a/apps/web/src/app/vault/org-vault/attachments.component.ts +++ b/apps/web/src/app/vault/org-vault/attachments.component.ts @@ -60,7 +60,7 @@ export class AttachmentsComponent extends BaseAttachmentsComponent implements On async ngOnInit() { await super.ngOnInit(); this.flexibleCollectionsV1Enabled = await firstValueFrom( - this.configService.getFeatureFlag$(FeatureFlag.FlexibleCollectionsV1, false), + this.configService.getFeatureFlag$(FeatureFlag.FlexibleCollectionsV1), ); } diff --git a/apps/web/src/app/vault/org-vault/bulk-collection-assignment-dialog/bulk-collection-assignment-dialog.component.ts b/apps/web/src/app/vault/org-vault/bulk-collection-assignment-dialog/bulk-collection-assignment-dialog.component.ts index 091c646178..e9f8401d73 100644 --- a/apps/web/src/app/vault/org-vault/bulk-collection-assignment-dialog/bulk-collection-assignment-dialog.component.ts +++ b/apps/web/src/app/vault/org-vault/bulk-collection-assignment-dialog/bulk-collection-assignment-dialog.component.ts @@ -70,10 +70,7 @@ export class BulkCollectionAssignmentDialogComponent implements OnDestroy, OnIni ) {} async ngOnInit() { - const v1FCEnabled = await this.configService.getFeatureFlag( - FeatureFlag.FlexibleCollectionsV1, - false, - ); + const v1FCEnabled = await this.configService.getFeatureFlag(FeatureFlag.FlexibleCollectionsV1); const org = await this.organizationService.get(this.params.organizationId); if (org.canEditAllCiphers(v1FCEnabled)) { diff --git a/apps/web/src/app/vault/org-vault/vault.component.ts b/apps/web/src/app/vault/org-vault/vault.component.ts index 9de404e969..243dedef93 100644 --- a/apps/web/src/app/vault/org-vault/vault.component.ts +++ b/apps/web/src/app/vault/org-vault/vault.component.ts @@ -193,7 +193,6 @@ export class VaultComponent implements OnInit, OnDestroy { this._flexibleCollectionsV1FlagEnabled = await this.configService.getFeatureFlag( FeatureFlag.FlexibleCollectionsV1, - false, ); const filter$ = this.routedVaultFilterService.filter$; diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/clients/clients.component.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/clients/clients.component.ts index 54e264c666..6875c3816b 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/clients/clients.component.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/clients/clients.component.ts @@ -42,7 +42,6 @@ export class ClientsComponent extends BaseClientsComponent { protected consolidatedBillingEnabled$ = this.configService.getFeatureFlag$( FeatureFlag.EnableConsolidatedBilling, - false, ); constructor( diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/providers-layout.component.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/providers-layout.component.ts index c78bf476c0..8dbb653401 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/providers-layout.component.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/providers-layout.component.ts @@ -37,12 +37,10 @@ export class ProvidersLayoutComponent { protected showPaymentMethodWarningBanners$ = this.configService.getFeatureFlag$( FeatureFlag.ShowPaymentMethodWarningBanners, - false, ); protected enableConsolidatedBilling$ = this.configService.getFeatureFlag$( FeatureFlag.EnableConsolidatedBilling, - false, ); constructor( diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/settings/account.component.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/settings/account.component.ts index 83038d1bfc..70eb8af7ba 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/settings/account.component.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/settings/account.component.ts @@ -30,7 +30,6 @@ export class AccountComponent { protected enableDeleteProvider$ = this.configService.getFeatureFlag$( FeatureFlag.EnableDeleteProvider, - false, ); constructor( diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/setup/setup.component.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/setup/setup.component.ts index cf9af4f68a..258088257d 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/setup/setup.component.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/setup/setup.component.ts @@ -36,12 +36,10 @@ export class SetupComponent implements OnInit { protected showPaymentMethodWarningBanners$ = this.configService.getFeatureFlag$( FeatureFlag.ShowPaymentMethodWarningBanners, - false, ); protected enableConsolidatedBilling$ = this.configService.getFeatureFlag$( FeatureFlag.EnableConsolidatedBilling, - false, ); constructor( diff --git a/libs/angular/src/directives/if-feature.directive.spec.ts b/libs/angular/src/directives/if-feature.directive.spec.ts index 944410be7d..456220b791 100644 --- a/libs/angular/src/directives/if-feature.directive.spec.ts +++ b/libs/angular/src/directives/if-feature.directive.spec.ts @@ -3,7 +3,7 @@ import { ComponentFixture, TestBed } from "@angular/core/testing"; import { By } from "@angular/platform-browser"; import { mock, MockProxy } from "jest-mock-extended"; -import { FeatureFlag, FeatureFlagValue } from "@bitwarden/common/enums/feature-flag.enum"; +import { AllowedFeatureFlagTypes, FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; @@ -41,10 +41,8 @@ describe("IfFeatureDirective", () => { let content: HTMLElement; let mockConfigService: MockProxy; - const mockConfigFlagValue = (flag: FeatureFlag, flagValue: FeatureFlagValue) => { - mockConfigService.getFeatureFlag.mockImplementation((f, defaultValue) => - flag == f ? Promise.resolve(flagValue) : Promise.resolve(defaultValue), - ); + const mockConfigFlagValue = (flag: FeatureFlag, flagValue: AllowedFeatureFlagTypes) => { + mockConfigService.getFeatureFlag.mockImplementation((f) => Promise.resolve(flagValue as any)); }; const queryContent = (testId: string) => diff --git a/libs/angular/src/directives/if-feature.directive.ts b/libs/angular/src/directives/if-feature.directive.ts index 069f306a89..838bd264ad 100644 --- a/libs/angular/src/directives/if-feature.directive.ts +++ b/libs/angular/src/directives/if-feature.directive.ts @@ -1,6 +1,6 @@ import { Directive, Input, OnInit, TemplateRef, ViewContainerRef } from "@angular/core"; -import { FeatureFlag, FeatureFlagValue } from "@bitwarden/common/enums/feature-flag.enum"; +import { AllowedFeatureFlagTypes, FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; @@ -23,7 +23,7 @@ export class IfFeatureDirective implements OnInit { * Optional value to compare against the value of the feature flag in the config service. * @default true */ - @Input() appIfFeatureValue: FeatureFlagValue = true; + @Input() appIfFeatureValue: AllowedFeatureFlagTypes = true; private hasView = false; diff --git a/libs/angular/src/platform/guard/feature-flag.guard.spec.ts b/libs/angular/src/platform/guard/feature-flag.guard.spec.ts index 88637dff97..323e8c2659 100644 --- a/libs/angular/src/platform/guard/feature-flag.guard.spec.ts +++ b/libs/angular/src/platform/guard/feature-flag.guard.spec.ts @@ -34,12 +34,12 @@ describe("canAccessFeature", () => { flag == testFlag ? Promise.resolve(flagValue) : Promise.resolve(defaultValue), ); } else if (typeof flagValue === "string") { - mockConfigService.getFeatureFlag.mockImplementation((flag, defaultValue = "") => - flag == testFlag ? Promise.resolve(flagValue) : Promise.resolve(defaultValue), + mockConfigService.getFeatureFlag.mockImplementation((flag) => + flag == testFlag ? Promise.resolve(flagValue as any) : Promise.resolve(""), ); } else if (typeof flagValue === "number") { - mockConfigService.getFeatureFlag.mockImplementation((flag, defaultValue = 0) => - flag == testFlag ? Promise.resolve(flagValue) : Promise.resolve(defaultValue), + mockConfigService.getFeatureFlag.mockImplementation((flag) => + flag == testFlag ? Promise.resolve(flagValue as any) : Promise.resolve(0), ); } diff --git a/libs/angular/src/vault/components/add-edit.component.ts b/libs/angular/src/vault/components/add-edit.component.ts index d9b73a0e7f..177b4289f4 100644 --- a/libs/angular/src/vault/components/add-edit.component.ts +++ b/libs/angular/src/vault/components/add-edit.component.ts @@ -182,7 +182,6 @@ export class AddEditComponent implements OnInit, OnDestroy { async ngOnInit() { this.flexibleCollectionsV1Enabled = await this.configService.getFeatureFlag( FeatureFlag.FlexibleCollectionsV1, - false, ); this.policyService diff --git a/libs/common/src/enums/feature-flag.enum.ts b/libs/common/src/enums/feature-flag.enum.ts index 636e9bc4ce..d84494362e 100644 --- a/libs/common/src/enums/feature-flag.enum.ts +++ b/libs/common/src/enums/feature-flag.enum.ts @@ -1,3 +1,8 @@ +/** + * Feature flags. + * + * Flags MUST be short lived and SHALL be removed once enabled. + */ export enum FeatureFlag { BrowserFilelessImport = "browser-fileless-import", ItemShare = "item-share", @@ -13,5 +18,32 @@ export enum FeatureFlag { EnableDeleteProvider = "AC-1218-delete-provider", } -// Replace this with a type safe lookup of the feature flag values in PM-2282 -export type FeatureFlagValue = number | string | boolean; +export type AllowedFeatureFlagTypes = boolean | number | string; + +// Helper to ensure the value is treated as a boolean. +const FALSE = false as boolean; + +/** + * Default value for feature flags. + * + * DO NOT enable previously disabled flags, REMOVE them instead. + * We support true as a value as we prefer flags to "enable" not "disable". + */ +export const DefaultFeatureFlagValue = { + [FeatureFlag.BrowserFilelessImport]: FALSE, + [FeatureFlag.ItemShare]: FALSE, + [FeatureFlag.FlexibleCollectionsV1]: FALSE, + [FeatureFlag.VaultOnboarding]: FALSE, + [FeatureFlag.GeneratorToolsModernization]: FALSE, + [FeatureFlag.KeyRotationImprovements]: FALSE, + [FeatureFlag.FlexibleCollectionsMigration]: FALSE, + [FeatureFlag.ShowPaymentMethodWarningBanners]: FALSE, + [FeatureFlag.EnableConsolidatedBilling]: FALSE, + [FeatureFlag.AC1795_UpdatedSubscriptionStatusSection]: FALSE, + [FeatureFlag.UnassignedItemsBanner]: FALSE, + [FeatureFlag.EnableDeleteProvider]: FALSE, +} satisfies Record; + +export type DefaultFeatureFlagValueType = typeof DefaultFeatureFlagValue; + +export type FeatureFlagValueType = DefaultFeatureFlagValueType[Flag]; diff --git a/libs/common/src/platform/abstractions/config/config.service.ts b/libs/common/src/platform/abstractions/config/config.service.ts index 9eca5891ac..6985430acc 100644 --- a/libs/common/src/platform/abstractions/config/config.service.ts +++ b/libs/common/src/platform/abstractions/config/config.service.ts @@ -1,7 +1,7 @@ import { Observable } from "rxjs"; import { SemVer } from "semver"; -import { FeatureFlag } from "../../../enums/feature-flag.enum"; +import { FeatureFlag, FeatureFlagValueType } from "../../../enums/feature-flag.enum"; import { Region } from "../environment.service"; import { ServerConfig } from "./server-config"; @@ -14,23 +14,15 @@ export abstract class ConfigService { /** * Retrieves the value of a feature flag for the currently active user * @param key The feature flag to retrieve - * @param defaultValue The default value to return if the feature flag is not set or the server's config is irretrievable * @returns An observable that emits the value of the feature flag, updates as the server config changes */ - getFeatureFlag$: ( - key: FeatureFlag, - defaultValue?: T, - ) => Observable; + getFeatureFlag$: (key: Flag) => Observable>; /** * Retrieves the value of a feature flag for the currently active user * @param key The feature flag to retrieve - * @param defaultValue The default value to return if the feature flag is not set or the server's config is irretrievable * @returns The value of the feature flag */ - getFeatureFlag: ( - key: FeatureFlag, - defaultValue?: T, - ) => Promise; + getFeatureFlag: (key: Flag) => Promise>; /** * Verifies whether the server version meets the minimum required version * @param minimumRequiredServerVersion The minimum version required diff --git a/libs/common/src/platform/abstractions/config/server-config.ts b/libs/common/src/platform/abstractions/config/server-config.ts index 287e359f18..bb18605964 100644 --- a/libs/common/src/platform/abstractions/config/server-config.ts +++ b/libs/common/src/platform/abstractions/config/server-config.ts @@ -1,5 +1,6 @@ import { Jsonify } from "type-fest"; +import { AllowedFeatureFlagTypes } from "../../../enums/feature-flag.enum"; import { ServerConfigData, ThirdPartyServerConfigData, @@ -14,7 +15,7 @@ export class ServerConfig { server?: ThirdPartyServerConfigData; environment?: EnvironmentServerConfigData; utcDate: Date; - featureStates: { [key: string]: string } = {}; + featureStates: { [key: string]: AllowedFeatureFlagTypes } = {}; constructor(serverConfigData: ServerConfigData) { this.version = serverConfigData.version; diff --git a/libs/common/src/platform/models/data/server-config.data.ts b/libs/common/src/platform/models/data/server-config.data.ts index a4819f7567..57e8fbc628 100644 --- a/libs/common/src/platform/models/data/server-config.data.ts +++ b/libs/common/src/platform/models/data/server-config.data.ts @@ -1,5 +1,6 @@ import { Jsonify } from "type-fest"; +import { AllowedFeatureFlagTypes } from "../../../enums/feature-flag.enum"; import { Region } from "../../abstractions/environment.service"; import { ServerConfigResponse, @@ -13,7 +14,7 @@ export class ServerConfigData { server?: ThirdPartyServerConfigData; environment?: EnvironmentServerConfigData; utcDate: string; - featureStates: { [key: string]: string } = {}; + featureStates: { [key: string]: AllowedFeatureFlagTypes } = {}; constructor(serverConfigResponse: Partial) { this.version = serverConfigResponse?.version; diff --git a/libs/common/src/platform/services/config/default-config.service.ts b/libs/common/src/platform/services/config/default-config.service.ts index e124deccf8..71b76363a3 100644 --- a/libs/common/src/platform/services/config/default-config.service.ts +++ b/libs/common/src/platform/services/config/default-config.service.ts @@ -13,7 +13,11 @@ import { } from "rxjs"; import { SemVer } from "semver"; -import { FeatureFlag, FeatureFlagValue } from "../../../enums/feature-flag.enum"; +import { + DefaultFeatureFlagValue, + FeatureFlag, + FeatureFlagValueType, +} from "../../../enums/feature-flag.enum"; import { UserId } from "../../../types/guid"; import { ConfigApiServiceAbstraction } from "../../abstractions/config/config-api.service.abstraction"; import { ConfigService } from "../../abstractions/config/config.service"; @@ -89,20 +93,21 @@ export class DefaultConfigService implements ConfigService { map((config) => config?.environment?.cloudRegion ?? Region.US), ); } - getFeatureFlag$(key: FeatureFlag, defaultValue?: T) { + + getFeatureFlag$(key: Flag) { return this.serverConfig$.pipe( map((serverConfig) => { if (serverConfig?.featureStates == null || serverConfig.featureStates[key] == null) { - return defaultValue; + return DefaultFeatureFlagValue[key]; } - return serverConfig.featureStates[key] as T; + return serverConfig.featureStates[key] as FeatureFlagValueType; }), ); } - async getFeatureFlag(key: FeatureFlag, defaultValue?: T) { - return await firstValueFrom(this.getFeatureFlag$(key, defaultValue)); + async getFeatureFlag(key: Flag) { + return await firstValueFrom(this.getFeatureFlag$(key)); } checkServerMeetsVersionRequirement$(minimumRequiredServerVersion: SemVer) { From 11ba8d188deb8efd52b1b8ff04c2d2e82f168be3 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 26 Apr 2024 11:06:19 +0000 Subject: [PATCH 04/41] Autosync the updated translations (#8925) Co-authored-by: bitwarden-devops-bot <106330231+bitwarden-devops-bot@users.noreply.github.com> --- apps/browser/src/_locales/pl/messages.json | 2 +- apps/browser/src/_locales/pt_BR/messages.json | 2 +- apps/browser/src/_locales/vi/messages.json | 24 +++++++++---------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/apps/browser/src/_locales/pl/messages.json b/apps/browser/src/_locales/pl/messages.json index d3d9106c15..1a56d32a35 100644 --- a/apps/browser/src/_locales/pl/messages.json +++ b/apps/browser/src/_locales/pl/messages.json @@ -7,7 +7,7 @@ "description": "Extension name, MUST be less than 40 characters (Safari restriction)" }, "extDesc": { - "message": "At home, at work, or on the go, Bitwarden easily secures all your passwords, passkeys, and sensitive information", + "message": "W domu, w pracy, lub w ruchu, Bitwarden z łatwością zabezpiecza Twoje hasła, passkeys i poufne informacje", "description": "Extension description, MUST be less than 112 characters (Safari restriction)" }, "loginOrCreateNewAccount": { diff --git a/apps/browser/src/_locales/pt_BR/messages.json b/apps/browser/src/_locales/pt_BR/messages.json index 417bc977eb..0f40bc63bb 100644 --- a/apps/browser/src/_locales/pt_BR/messages.json +++ b/apps/browser/src/_locales/pt_BR/messages.json @@ -7,7 +7,7 @@ "description": "Extension name, MUST be less than 40 characters (Safari restriction)" }, "extDesc": { - "message": "At home, at work, or on the go, Bitwarden easily secures all your passwords, passkeys, and sensitive information", + "message": "Em qual lugar for, o Bitwarden protege suas senhas, chaves de acesso, e informações confidenciais", "description": "Extension description, MUST be less than 112 characters (Safari restriction)" }, "loginOrCreateNewAccount": { diff --git a/apps/browser/src/_locales/vi/messages.json b/apps/browser/src/_locales/vi/messages.json index 4eba4ffaea..6e530412db 100644 --- a/apps/browser/src/_locales/vi/messages.json +++ b/apps/browser/src/_locales/vi/messages.json @@ -3,11 +3,11 @@ "message": "Bitwarden" }, "extName": { - "message": "Bitwarden Password Manager", + "message": "Bitwarden - Trình Quản lý Mật khẩu", "description": "Extension name, MUST be less than 40 characters (Safari restriction)" }, "extDesc": { - "message": "At home, at work, or on the go, Bitwarden easily secures all your passwords, passkeys, and sensitive information", + "message": "Ở nhà, ở cơ quan, hay trên đường đi, Bitwarden sẽ bảo mật tất cả mật khẩu, passkey, và thông tin cá nhân của bạn", "description": "Extension description, MUST be less than 112 characters (Safari restriction)" }, "loginOrCreateNewAccount": { @@ -650,7 +650,7 @@ "message": "'Thông báo Thêm đăng nhập' sẽ tự động nhắc bạn lưu các đăng nhập mới vào hầm an toàn của bạn bất cứ khi nào bạn đăng nhập trang web lần đầu tiên." }, "addLoginNotificationDescAlt": { - "message": "Ask to add an item if one isn't found in your vault. Applies to all logged in accounts." + "message": "Đưa ra lựa chọn để thêm một mục nếu không tìm thấy mục đó trong hòm của bạn. Áp dụng với mọi tài khoản đăng nhập trên thiết bị." }, "showCardsCurrentTab": { "message": "Hiển thị thẻ trên trang Tab" @@ -685,13 +685,13 @@ "message": "Yêu cầu cập nhật mật khẩu đăng nhập khi phát hiện thay đổi trên trang web." }, "changedPasswordNotificationDescAlt": { - "message": "Ask to update a login's password when a change is detected on a website. Applies to all logged in accounts." + "message": "Đưa ra lựa chọn để cập nhật mật khẩu khi phát hiện có sự thay đổi trên trang web. Áp dụng với mọi tài khoản đăng nhập trên thiết bị." }, "enableUsePasskeys": { - "message": "Ask to save and use passkeys" + "message": "Đưa ra lựa chọn để lưu và sử dụng passkey" }, "usePasskeysDesc": { - "message": "Ask to save new passkeys or log in with passkeys stored in your vault. Applies to all logged in accounts." + "message": "Đưa ra lựa chọn để lưu passkey mới hoặc đăng nhập bằng passkey đã lưu trong hòm. Áp dụng với mọi tài khoản đăng nhập trên thiết bị." }, "notificationChangeDesc": { "message": "Bạn có muốn cập nhật mật khẩu này trên Bitwarden không?" @@ -712,7 +712,7 @@ "message": "Sử dụng một đúp chuột để truy cập vào việc tạo mật khẩu và thông tin đăng nhập phù hợp cho trang web. " }, "contextMenuItemDescAlt": { - "message": "Use a secondary click to access password generation and matching logins for the website. Applies to all logged in accounts." + "message": "Truy cập trình khởi tạo mật khẩu và các mục đăng nhập đã lưu của trang web bằng cách nhấn đúp chuột. Áp dụng với mọi tài khoản đăng nhập trên thiết bị." }, "defaultUriMatchDetection": { "message": "Phương thức kiểm tra URI mặc định", @@ -728,7 +728,7 @@ "message": "Thay đổi màu sắc ứng dụng." }, "themeDescAlt": { - "message": "Change the application's color theme. Applies to all logged in accounts." + "message": "Thay đổi tông màu giao diện của ứng dụng. Áp dụng với mọi tài khoản đăng nhập trên thiết bị." }, "dark": { "message": "Tối", @@ -1061,10 +1061,10 @@ "message": "Tắt cài đặt trình quản lý mật khẩu tích hợp trong trình duyệt của bạn để tránh xung đột." }, "turnOffBrowserBuiltInPasswordManagerSettingsLink": { - "message": "Edit browser settings." + "message": "Thay đổi cài đặt của trình duyệt." }, "autofillOverlayVisibilityOff": { - "message": "Off", + "message": "Tắt", "description": "Overlay setting select option for disabling autofill overlay" }, "autofillOverlayVisibilityOnFieldFocus": { @@ -1168,7 +1168,7 @@ "message": "Hiển thị một ảnh nhận dạng bên cạnh mỗi lần đăng nhập." }, "faviconDescAlt": { - "message": "Show a recognizable image next to each login. Applies to all logged in accounts." + "message": "Hiển thị một biểu tượng dễ nhận dạng bên cạnh mỗi mục đăng nhập. Áp dụng với mọi tài khoản đăng nhập trên thiết bị." }, "enableBadgeCounter": { "message": "Hiển thị biểu tượng bộ đếm" @@ -1500,7 +1500,7 @@ "message": "Mã PIN không hợp lệ." }, "tooManyInvalidPinEntryAttemptsLoggingOut": { - "message": "Too many invalid PIN entry attempts. Logging out." + "message": "Mã PIN bị gõ sai quá nhiều lần. Đang đăng xuất." }, "unlockWithBiometrics": { "message": "Mở khóa bằng sinh trắc học" From 2fa4c6e4f930d6f543ac799efe339e95368b36d7 Mon Sep 17 00:00:00 2001 From: KiruthigaManivannan <162679756+KiruthigaManivannan@users.noreply.github.com> Date: Fri, 26 Apr 2024 18:24:48 +0530 Subject: [PATCH 05/41] PM-4945 Update Two Factor verify dialog (#8580) * PM-4945 Update Two Factor verify dialog * PM-4945 Addressed review comments * PM-4945 Removed legacy User verification component and used new one --- .../settings/two-factor-setup.component.ts | 3 + .../two-factor-authenticator.component.html | 7 -- .../settings/two-factor-duo.component.html | 7 -- .../settings/two-factor-email.component.html | 7 -- .../two-factor-recovery.component.html | 2 - .../settings/two-factor-setup.component.ts | 73 ++++++++++++++---- .../settings/two-factor-verify.component.html | 36 +++++---- .../settings/two-factor-verify.component.ts | 74 ++++++++++++++----- .../two-factor-webauthn.component.html | 7 -- .../two-factor-yubikey.component.html | 7 -- .../src/app/shared/loose-components.module.ts | 6 +- 11 files changed, 142 insertions(+), 87 deletions(-) diff --git a/apps/web/src/app/admin-console/organizations/settings/two-factor-setup.component.ts b/apps/web/src/app/admin-console/organizations/settings/two-factor-setup.component.ts index abf1d249e1..80d77968f2 100644 --- a/apps/web/src/app/admin-console/organizations/settings/two-factor-setup.component.ts +++ b/apps/web/src/app/admin-console/organizations/settings/two-factor-setup.component.ts @@ -10,6 +10,7 @@ import { PolicyService } from "@bitwarden/common/admin-console/abstractions/poli import { TwoFactorProviderType } from "@bitwarden/common/auth/enums/two-factor-provider-type"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; +import { DialogService } from "@bitwarden/components"; import { TwoFactorDuoComponent } from "../../../auth/settings/two-factor-duo.component"; import { TwoFactorSetupComponent as BaseTwoFactorSetupComponent } from "../../../auth/settings/two-factor-setup.component"; @@ -22,6 +23,7 @@ import { TwoFactorSetupComponent as BaseTwoFactorSetupComponent } from "../../.. export class TwoFactorSetupComponent extends BaseTwoFactorSetupComponent { tabbedHeader = false; constructor( + dialogService: DialogService, apiService: ApiService, modalService: ModalService, messagingService: MessagingService, @@ -31,6 +33,7 @@ export class TwoFactorSetupComponent extends BaseTwoFactorSetupComponent { billingAccountProfileStateService: BillingAccountProfileStateService, ) { super( + dialogService, apiService, modalService, messagingService, diff --git a/apps/web/src/app/auth/settings/two-factor-authenticator.component.html b/apps/web/src/app/auth/settings/two-factor-authenticator.component.html index 33bf4fb130..e17714cca7 100644 --- a/apps/web/src/app/auth/settings/two-factor-authenticator.component.html +++ b/apps/web/src/app/auth/settings/two-factor-authenticator.component.html @@ -15,13 +15,6 @@ - - diff --git a/apps/desktop/src/app/layout/account-switcher.component.ts b/apps/desktop/src/app/layout/account-switcher.component.ts index 4e39ab0029..c8a26065c1 100644 --- a/apps/desktop/src/app/layout/account-switcher.component.ts +++ b/apps/desktop/src/app/layout/account-switcher.component.ts @@ -1,19 +1,17 @@ import { animate, state, style, transition, trigger } from "@angular/animations"; import { ConnectedPosition } from "@angular/cdk/overlay"; -import { Component, OnDestroy, OnInit } from "@angular/core"; +import { Component } from "@angular/core"; import { Router } from "@angular/router"; -import { concatMap, firstValueFrom, Subject, takeUntil } from "rxjs"; +import { combineLatest, firstValueFrom, map, Observable, switchMap } from "rxjs"; import { LoginEmailServiceAbstraction } from "@bitwarden/auth/common"; +import { AccountInfo, AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AvatarService } from "@bitwarden/common/auth/abstractions/avatar.service"; -import { TokenService } from "@bitwarden/common/auth/abstractions/token.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; -import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { Account } from "@bitwarden/common/platform/models/domain/account"; import { UserId } from "@bitwarden/common/types/guid"; type ActiveAccount = { @@ -52,12 +50,18 @@ type InactiveAccount = ActiveAccount & { ]), ], }) -export class AccountSwitcherComponent implements OnInit, OnDestroy { - activeAccount?: ActiveAccount; - inactiveAccounts: { [userId: string]: InactiveAccount } = {}; - +export class AccountSwitcherComponent { + activeAccount$: Observable; + inactiveAccounts$: Observable<{ [userId: string]: InactiveAccount }>; authStatus = AuthenticationStatus; + view$: Observable<{ + activeAccount: ActiveAccount | null; + inactiveAccounts: { [userId: string]: InactiveAccount }; + numberOfAccounts: number; + showSwitcher: boolean; + }>; + isOpen = false; overlayPosition: ConnectedPosition[] = [ { @@ -68,21 +72,9 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy { }, ]; - private destroy$ = new Subject(); + showSwitcher$: Observable; - get showSwitcher() { - const userIsInAVault = !Utils.isNullOrWhitespace(this.activeAccount?.email); - const userIsAddingAnAdditionalAccount = Object.keys(this.inactiveAccounts).length > 0; - return userIsInAVault || userIsAddingAnAdditionalAccount; - } - - get numberOfAccounts() { - if (this.inactiveAccounts == null) { - this.isOpen = false; - return 0; - } - return Object.keys(this.inactiveAccounts).length; - } + numberOfAccounts$: Observable; constructor( private stateService: StateService, @@ -90,37 +82,65 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy { private avatarService: AvatarService, private messagingService: MessagingService, private router: Router, - private tokenService: TokenService, private environmentService: EnvironmentService, private loginEmailService: LoginEmailServiceAbstraction, - ) {} + private accountService: AccountService, + ) { + this.activeAccount$ = this.accountService.activeAccount$.pipe( + switchMap(async (active) => { + if (active == null) { + return null; + } - async ngOnInit(): Promise { - this.stateService.accounts$ - .pipe( - concatMap(async (accounts: { [userId: string]: Account }) => { - this.inactiveAccounts = await this.createInactiveAccounts(accounts); + return { + id: active.id, + name: active.name, + email: active.email, + avatarColor: await firstValueFrom(this.avatarService.avatarColor$), + server: (await this.environmentService.getEnvironment())?.getHostname(), + }; + }), + ); + this.inactiveAccounts$ = combineLatest([ + this.activeAccount$, + this.accountService.accounts$, + this.authService.authStatuses$, + ]).pipe( + switchMap(async ([activeAccount, accounts, accountStatuses]) => { + // Filter out logged out accounts and active account + accounts = Object.fromEntries( + Object.entries(accounts).filter( + ([id]: [UserId, AccountInfo]) => + accountStatuses[id] !== AuthenticationStatus.LoggedOut || id === activeAccount?.id, + ), + ); + return this.createInactiveAccounts(accounts); + }), + ); + this.showSwitcher$ = combineLatest([this.activeAccount$, this.inactiveAccounts$]).pipe( + map(([activeAccount, inactiveAccounts]) => { + const hasActiveUser = activeAccount != null; + const userIsAddingAnAdditionalAccount = Object.keys(inactiveAccounts).length > 0; + return hasActiveUser || userIsAddingAnAdditionalAccount; + }), + ); + this.numberOfAccounts$ = this.inactiveAccounts$.pipe( + map((accounts) => Object.keys(accounts).length), + ); - try { - this.activeAccount = { - id: await this.tokenService.getUserId(), - name: (await this.tokenService.getName()) ?? (await this.tokenService.getEmail()), - email: await this.tokenService.getEmail(), - avatarColor: await firstValueFrom(this.avatarService.avatarColor$), - server: (await this.environmentService.getEnvironment())?.getHostname(), - }; - } catch { - this.activeAccount = undefined; - } - }), - takeUntil(this.destroy$), - ) - .subscribe(); - } - - ngOnDestroy(): void { - this.destroy$.next(); - this.destroy$.complete(); + this.view$ = combineLatest([ + this.activeAccount$, + this.inactiveAccounts$, + this.numberOfAccounts$, + this.showSwitcher$, + ]).pipe( + map(([activeAccount, inactiveAccounts, numberOfAccounts, showSwitcher]) => ({ + activeAccount, + inactiveAccounts, + numberOfAccounts, + showSwitcher, + })), + ); } toggle() { @@ -144,11 +164,13 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy { await this.loginEmailService.saveEmailSettings(); await this.router.navigate(["/login"]); - await this.stateService.setActiveUser(null); + const activeAccount = await firstValueFrom(this.accountService.activeAccount$); + await this.stateService.clearDecryptedData(activeAccount?.id as UserId); + await this.accountService.switchAccount(null); } private async createInactiveAccounts(baseAccounts: { - [userId: string]: Account; + [userId: string]: AccountInfo; }): Promise<{ [userId: string]: InactiveAccount }> { const inactiveAccounts: { [userId: string]: InactiveAccount } = {}; @@ -159,8 +181,8 @@ export class AccountSwitcherComponent implements OnInit, OnDestroy { inactiveAccounts[userId] = { id: userId, - name: baseAccounts[userId].profile.name, - email: baseAccounts[userId].profile.email, + name: baseAccounts[userId].name, + email: baseAccounts[userId].email, authenticationStatus: await this.authService.getAuthStatus(userId), avatarColor: await firstValueFrom(this.avatarService.getUserAvatarColor$(userId as UserId)), server: (await this.environmentService.getEnvironment(userId))?.getHostname(), diff --git a/apps/desktop/src/app/layout/search/search.component.ts b/apps/desktop/src/app/layout/search/search.component.ts index 9a7226218a..06c67d8af2 100644 --- a/apps/desktop/src/app/layout/search/search.component.ts +++ b/apps/desktop/src/app/layout/search/search.component.ts @@ -2,7 +2,7 @@ import { Component, OnDestroy, OnInit } from "@angular/core"; import { UntypedFormControl } from "@angular/forms"; import { Subscription } from "rxjs"; -import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { SearchBarService, SearchBarState } from "./search-bar.service"; @@ -18,7 +18,7 @@ export class SearchComponent implements OnInit, OnDestroy { constructor( private searchBarService: SearchBarService, - private stateService: StateService, + private accountService: AccountService, ) { // eslint-disable-next-line rxjs-angular/prefer-takeuntil this.searchBarService.state$.subscribe((state) => { @@ -33,7 +33,7 @@ export class SearchComponent implements OnInit, OnDestroy { ngOnInit() { // eslint-disable-next-line rxjs-angular/prefer-takeuntil - this.activeAccountSubscription = this.stateService.activeAccount$.subscribe((value) => { + this.activeAccountSubscription = this.accountService.activeAccount$.subscribe((_) => { this.searchBarService.setSearchText(""); this.searchText.patchValue(""); }); diff --git a/apps/desktop/src/app/services/services.module.ts b/apps/desktop/src/app/services/services.module.ts index 1e3a7fdfa5..a485b925ba 100644 --- a/apps/desktop/src/app/services/services.module.ts +++ b/apps/desktop/src/app/services/services.module.ts @@ -59,7 +59,6 @@ import { PasswordGenerationServiceAbstraction } from "@bitwarden/common/tools/ge import { CipherService as CipherServiceAbstraction } from "@bitwarden/common/vault/abstractions/cipher.service"; import { DialogService } from "@bitwarden/components"; -import { LoginGuard } from "../../auth/guards/login.guard"; import { DesktopAutofillSettingsService } from "../../autofill/services/desktop-autofill-settings.service"; import { Account } from "../../models/account"; import { DesktopSettingsService } from "../../platform/services/desktop-settings.service"; @@ -102,7 +101,6 @@ const safeProviders: SafeProvider[] = [ safeProvider(InitService), safeProvider(NativeMessagingService), safeProvider(SearchBarService), - safeProvider(LoginGuard), safeProvider(DialogService), safeProvider({ provide: APP_INITIALIZER as SafeInjectionToken<() => void>, @@ -192,6 +190,7 @@ const safeProviders: SafeProvider[] = [ AutofillSettingsServiceAbstraction, VaultTimeoutSettingsService, BiometricStateService, + AccountServiceAbstraction, ], }), safeProvider({ diff --git a/apps/desktop/src/app/tools/generator.component.spec.ts b/apps/desktop/src/app/tools/generator.component.spec.ts index 51b5bf93a2..d908de8ef7 100644 --- a/apps/desktop/src/app/tools/generator.component.spec.ts +++ b/apps/desktop/src/app/tools/generator.component.spec.ts @@ -4,6 +4,7 @@ import { ActivatedRoute } from "@angular/router"; import { mock, MockProxy } from "jest-mock-extended"; import { I18nPipe } from "@bitwarden/angular/platform/pipes/i18n.pipe"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; @@ -59,6 +60,10 @@ describe("GeneratorComponent", () => { provide: CipherService, useValue: mock(), }, + { + provide: AccountService, + useValue: mock(), + }, ], schemas: [NO_ERRORS_SCHEMA], }).compileComponents(); diff --git a/apps/desktop/src/app/tools/send/add-edit.component.ts b/apps/desktop/src/app/tools/send/add-edit.component.ts index 7bdd5efbba..804a390438 100644 --- a/apps/desktop/src/app/tools/send/add-edit.component.ts +++ b/apps/desktop/src/app/tools/send/add-edit.component.ts @@ -4,6 +4,7 @@ import { FormBuilder } from "@angular/forms"; import { AddEditComponent as BaseAddEditComponent } from "@bitwarden/angular/tools/send/add-edit.component"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -34,6 +35,7 @@ export class AddEditComponent extends BaseAddEditComponent { dialogService: DialogService, formBuilder: FormBuilder, billingAccountProfileStateService: BillingAccountProfileStateService, + accountService: AccountService, ) { super( i18nService, @@ -49,6 +51,7 @@ export class AddEditComponent extends BaseAddEditComponent { dialogService, formBuilder, billingAccountProfileStateService, + accountService, ); } diff --git a/apps/desktop/src/auth/guards/login.guard.ts b/apps/desktop/src/auth/guards/login.guard.ts deleted file mode 100644 index f6c67d5af9..0000000000 --- a/apps/desktop/src/auth/guards/login.guard.ts +++ /dev/null @@ -1,29 +0,0 @@ -import { Injectable } from "@angular/core"; -import { CanActivate } from "@angular/router"; -import { firstValueFrom } from "rxjs"; - -import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; -import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; -import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; - -const maxAllowedAccounts = 5; - -@Injectable() -export class LoginGuard implements CanActivate { - protected homepage = "vault"; - constructor( - private stateService: StateService, - private platformUtilsService: PlatformUtilsService, - private i18nService: I18nService, - ) {} - - async canActivate() { - const accounts = await firstValueFrom(this.stateService.accounts$); - if (accounts != null && Object.keys(accounts).length >= maxAllowedAccounts) { - this.platformUtilsService.showToast("error", null, this.i18nService.t("accountLimitReached")); - return false; - } - - return true; - } -} diff --git a/apps/desktop/src/auth/guards/max-accounts.guard.ts b/apps/desktop/src/auth/guards/max-accounts.guard.ts new file mode 100644 index 0000000000..65c4ac99d0 --- /dev/null +++ b/apps/desktop/src/auth/guards/max-accounts.guard.ts @@ -0,0 +1,38 @@ +import { inject } from "@angular/core"; +import { CanActivateFn } from "@angular/router"; +import { Observable, map } from "rxjs"; + +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { ToastService } from "@bitwarden/components"; + +const maxAllowedAccounts = 5; + +function maxAccountsGuard(): Observable { + const authService = inject(AuthService); + const toastService = inject(ToastService); + const i18nService = inject(I18nService); + + return authService.authStatuses$.pipe( + map((statuses) => + Object.values(statuses).filter((status) => status != AuthenticationStatus.LoggedOut), + ), + map((accounts) => { + if (accounts != null && Object.keys(accounts).length >= maxAllowedAccounts) { + toastService.showToast({ + variant: "error", + title: null, + message: i18nService.t("accountLimitReached"), + }); + return false; + } + + return true; + }), + ); +} + +export function maxAccountsGuardFn(): CanActivateFn { + return () => maxAccountsGuard(); +} diff --git a/apps/desktop/src/auth/lock.component.spec.ts b/apps/desktop/src/auth/lock.component.spec.ts index f998e75d7a..2137b707f6 100644 --- a/apps/desktop/src/auth/lock.component.spec.ts +++ b/apps/desktop/src/auth/lock.component.spec.ts @@ -13,6 +13,7 @@ import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeou import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction"; import { KdfConfigService } from "@bitwarden/common/auth/abstractions/kdf-config.service"; import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction"; @@ -50,7 +51,7 @@ describe("LockComponent", () => { let component: LockComponent; let fixture: ComponentFixture; let stateServiceMock: MockProxy; - const biometricStateService = mock(); + let biometricStateService: MockProxy; let messagingServiceMock: MockProxy; let broadcasterServiceMock: MockProxy; let platformUtilsServiceMock: MockProxy; @@ -62,7 +63,6 @@ describe("LockComponent", () => { beforeEach(async () => { stateServiceMock = mock(); - stateServiceMock.activeAccount$ = of(null); messagingServiceMock = mock(); broadcasterServiceMock = mock(); @@ -73,6 +73,7 @@ describe("LockComponent", () => { mockMasterPasswordService = new FakeMasterPasswordService(); + biometricStateService = mock(); biometricStateService.dismissedRequirePasswordOnStartCallout$ = of(false); biometricStateService.promptAutomatically$ = of(false); biometricStateService.promptCancelled$ = of(false); @@ -165,6 +166,10 @@ describe("LockComponent", () => { provide: AccountService, useValue: accountService, }, + { + provide: AuthService, + useValue: mock(), + }, { provide: KdfConfigService, useValue: mock(), diff --git a/apps/desktop/src/auth/lock.component.ts b/apps/desktop/src/auth/lock.component.ts index 8e87b6663f..d95df419e1 100644 --- a/apps/desktop/src/auth/lock.component.ts +++ b/apps/desktop/src/auth/lock.component.ts @@ -10,6 +10,7 @@ import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeou import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/policy/policy-api.service.abstraction"; import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction"; import { KdfConfigService } from "@bitwarden/common/auth/abstractions/kdf-config.service"; import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction"; @@ -64,6 +65,7 @@ export class LockComponent extends BaseLockComponent { pinCryptoService: PinCryptoServiceAbstraction, biometricStateService: BiometricStateService, accountService: AccountService, + authService: AuthService, kdfConfigService: KdfConfigService, ) { super( @@ -89,6 +91,7 @@ export class LockComponent extends BaseLockComponent { pinCryptoService, biometricStateService, accountService, + authService, kdfConfigService, ); } diff --git a/apps/desktop/src/main/menu/menubar.ts b/apps/desktop/src/main/menu/menubar.ts index eb1dacf825..b71774c5af 100644 --- a/apps/desktop/src/main/menu/menubar.ts +++ b/apps/desktop/src/main/menu/menubar.ts @@ -65,9 +65,10 @@ export class Menubar { isLocked = updateRequest.accounts[updateRequest.activeUserId]?.isLocked ?? true; } - const isLockable = !isLocked && updateRequest?.accounts[updateRequest.activeUserId]?.isLockable; + const isLockable = + !isLocked && updateRequest?.accounts?.[updateRequest.activeUserId]?.isLockable; const hasMasterPassword = - updateRequest?.accounts[updateRequest.activeUserId]?.hasMasterPassword ?? false; + updateRequest?.accounts?.[updateRequest.activeUserId]?.hasMasterPassword ?? false; this.items = [ new FileMenu( diff --git a/apps/web/src/app/app.component.ts b/apps/web/src/app/app.component.ts index 1da2d94c15..1939bb11f5 100644 --- a/apps/web/src/app/app.component.ts +++ b/apps/web/src/app/app.component.ts @@ -2,7 +2,7 @@ import { DOCUMENT } from "@angular/common"; import { Component, Inject, NgZone, OnDestroy, OnInit } from "@angular/core"; import { NavigationEnd, Router } from "@angular/router"; import * as jq from "jquery"; -import { Subject, switchMap, takeUntil, timer } from "rxjs"; +import { Subject, firstValueFrom, map, switchMap, takeUntil, timer } from "rxjs"; import { EventUploadService } from "@bitwarden/common/abstractions/event/event-upload.service"; import { NotificationsService } from "@bitwarden/common/abstractions/notifications.service"; @@ -10,6 +10,7 @@ import { SearchService } from "@bitwarden/common/abstractions/search.service"; import { VaultTimeoutService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout.service"; import { InternalOrganizationServiceAbstraction } 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 { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { KeyConnectorService } from "@bitwarden/common/auth/abstractions/key-connector.service"; import { PaymentMethodWarningsServiceAbstraction as PaymentMethodWarningService } from "@bitwarden/common/billing/abstractions/payment-method-warnings-service.abstraction"; @@ -51,7 +52,7 @@ const PaymentMethodWarningsRefresh = 60000; // 1 Minute templateUrl: "app.component.html", }) export class AppComponent implements OnDestroy, OnInit { - private lastActivity: number = null; + private lastActivity: Date = null; private idleTimer: number = null; private isIdle = false; private destroy$ = new Subject(); @@ -86,6 +87,7 @@ export class AppComponent implements OnDestroy, OnInit { private stateEventRunnerService: StateEventRunnerService, private paymentMethodWarningService: PaymentMethodWarningService, private organizationService: InternalOrganizationServiceAbstraction, + private accountService: AccountService, ) {} ngOnInit() { @@ -298,15 +300,16 @@ export class AppComponent implements OnDestroy, OnInit { } private async recordActivity() { - const now = new Date().getTime(); - if (this.lastActivity != null && now - this.lastActivity < 250) { + const activeUserId = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((a) => a?.id)), + ); + const now = new Date(); + if (this.lastActivity != null && now.getTime() - this.lastActivity.getTime() < 250) { return; } this.lastActivity = now; - // 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 - this.stateService.setLastActive(now); + await this.accountService.setAccountActivity(activeUserId, now); // Idle states if (this.isIdle) { this.isIdle = false; diff --git a/apps/web/src/app/layouts/header/web-header.component.html b/apps/web/src/app/layouts/header/web-header.component.html index e24013de6f..e2b3e7910a 100644 --- a/apps/web/src/app/layouts/header/web-header.component.html +++ b/apps/web/src/app/layouts/header/web-header.component.html @@ -58,7 +58,7 @@ [bitMenuTriggerFor]="accountMenu" class="tw-border-0 tw-bg-transparent tw-p-0" > - + @@ -67,7 +67,7 @@ class="tw-flex tw-items-center tw-px-4 tw-py-1 tw-leading-tight tw-text-info" appStopProp > - +
{{ "loggedInAs" | i18n }} diff --git a/apps/web/src/app/layouts/header/web-header.component.ts b/apps/web/src/app/layouts/header/web-header.component.ts index 1f012e52dd..9906bd53ba 100644 --- a/apps/web/src/app/layouts/header/web-header.component.ts +++ b/apps/web/src/app/layouts/header/web-header.component.ts @@ -1,16 +1,17 @@ import { Component, Input } from "@angular/core"; import { ActivatedRoute } from "@angular/router"; -import { combineLatest, map, Observable } from "rxjs"; +import { map, Observable } from "rxjs"; +import { User } from "@bitwarden/angular/pipes/user-name.pipe"; import { UnassignedItemsBannerService } from "@bitwarden/angular/services/unassigned-items-banner.service"; import { VaultTimeoutSettingsService } from "@bitwarden/common/abstractions/vault-timeout/vault-timeout-settings.service"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { VaultTimeoutAction } from "@bitwarden/common/enums/vault-timeout-action.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; -import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; -import { AccountProfile } from "@bitwarden/common/platform/models/domain/account"; +import { UserId } from "@bitwarden/common/types/guid"; @Component({ selector: "app-header", @@ -28,7 +29,7 @@ export class WebHeaderComponent { @Input() icon: string; protected routeData$: Observable<{ titleId: string }>; - protected account$: Observable; + protected account$: Observable; protected canLock$: Observable; protected selfHosted: boolean; protected hostname = location.hostname; @@ -38,12 +39,12 @@ export class WebHeaderComponent { constructor( private route: ActivatedRoute, - private stateService: StateService, private platformUtilsService: PlatformUtilsService, private vaultTimeoutSettingsService: VaultTimeoutSettingsService, private messagingService: MessagingService, protected unassignedItemsBannerService: UnassignedItemsBannerService, private configService: ConfigService, + private accountService: AccountService, ) { this.routeData$ = this.route.data.pipe( map((params) => { @@ -55,14 +56,7 @@ export class WebHeaderComponent { this.selfHosted = this.platformUtilsService.isSelfHost(); - this.account$ = combineLatest([ - this.stateService.activeAccount$, - this.stateService.accounts$, - ]).pipe( - map(([activeAccount, accounts]) => { - return accounts[activeAccount]?.profile; - }), - ); + this.account$ = this.accountService.activeAccount$; this.canLock$ = this.vaultTimeoutSettingsService .availableVaultTimeoutActions$() .pipe(map((actions) => actions.includes(VaultTimeoutAction.Lock))); diff --git a/apps/web/src/app/tools/send/add-edit.component.ts b/apps/web/src/app/tools/send/add-edit.component.ts index ee4be41488..cca416db9c 100644 --- a/apps/web/src/app/tools/send/add-edit.component.ts +++ b/apps/web/src/app/tools/send/add-edit.component.ts @@ -5,6 +5,7 @@ import { FormBuilder } from "@angular/forms"; import { AddEditComponent as BaseAddEditComponent } from "@bitwarden/angular/tools/send/add-edit.component"; import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -40,6 +41,7 @@ export class AddEditComponent extends BaseAddEditComponent { billingAccountProfileStateService: BillingAccountProfileStateService, protected dialogRef: DialogRef, @Inject(DIALOG_DATA) params: { sendId: string }, + accountService: AccountService, ) { super( i18nService, @@ -55,6 +57,7 @@ export class AddEditComponent extends BaseAddEditComponent { dialogService, formBuilder, billingAccountProfileStateService, + accountService, ); this.sendId = params.sendId; diff --git a/apps/web/src/app/vault/components/vault-items/vault-items.stories.ts b/apps/web/src/app/vault/components/vault-items/vault-items.stories.ts index ad80c9f4e5..41aa766e3a 100644 --- a/apps/web/src/app/vault/components/vault-items/vault-items.stories.ts +++ b/apps/web/src/app/vault/components/vault-items/vault-items.stories.ts @@ -55,7 +55,6 @@ export default { { provide: StateService, useValue: { - activeAccount$: new BehaviorSubject("1").asObservable(), accounts$: new BehaviorSubject({ "1": { profile: { name: "Foo" } } }).asObservable(), async getShowFavicon() { return true; diff --git a/libs/angular/src/auth/components/lock.component.ts b/libs/angular/src/auth/components/lock.component.ts index 89af31da81..7eb30d759a 100644 --- a/libs/angular/src/auth/components/lock.component.ts +++ b/libs/angular/src/auth/components/lock.component.ts @@ -1,7 +1,7 @@ import { Directive, NgZone, OnDestroy, OnInit } from "@angular/core"; import { Router } from "@angular/router"; import { firstValueFrom, Subject } from "rxjs"; -import { concatMap, take, takeUntil } from "rxjs/operators"; +import { concatMap, map, take, takeUntil } from "rxjs/operators"; import { PinCryptoServiceAbstraction } from "@bitwarden/auth/common"; import { ApiService } from "@bitwarden/common/abstractions/api.service"; @@ -11,10 +11,12 @@ import { PolicyApiServiceAbstraction } from "@bitwarden/common/admin-console/abs import { InternalPolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { MasterPasswordPolicyOptions } from "@bitwarden/common/admin-console/models/domain/master-password-policy-options"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction"; import { KdfConfigService } from "@bitwarden/common/auth/abstractions/kdf-config.service"; import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction"; import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction"; +import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; import { ForceSetPasswordReason } from "@bitwarden/common/auth/models/domain/force-set-password-reason"; import { SecretVerificationRequest } from "@bitwarden/common/auth/models/request/secret-verification.request"; import { MasterPasswordPolicyResponse } from "@bitwarden/common/auth/models/response/master-password-policy.response"; @@ -30,6 +32,7 @@ import { BiometricStateService } from "@bitwarden/common/platform/biometrics/bio import { HashPurpose, KeySuffixOptions } from "@bitwarden/common/platform/enums"; import { PinLockType } from "@bitwarden/common/services/vault-timeout/vault-timeout-settings.service"; import { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength"; +import { UserId } from "@bitwarden/common/types/guid"; import { UserKey } from "@bitwarden/common/types/key"; import { DialogService } from "@bitwarden/components"; @@ -46,6 +49,7 @@ export class LockComponent implements OnInit, OnDestroy { supportsBiometric: boolean; biometricLock: boolean; + private activeUserId: UserId; protected successRoute = "vault"; protected forcePasswordResetRoute = "update-temp-password"; protected onSuccessfulSubmit: () => Promise; @@ -80,14 +84,16 @@ export class LockComponent implements OnInit, OnDestroy { protected pinCryptoService: PinCryptoServiceAbstraction, protected biometricStateService: BiometricStateService, protected accountService: AccountService, + protected authService: AuthService, protected kdfConfigService: KdfConfigService, ) {} async ngOnInit() { - this.stateService.activeAccount$ + this.accountService.activeAccount$ .pipe( - concatMap(async () => { - await this.load(); + concatMap(async (account) => { + this.activeUserId = account?.id; + await this.load(account?.id); }), takeUntil(this.destroy$), ) @@ -116,7 +122,7 @@ export class LockComponent implements OnInit, OnDestroy { }); if (confirmed) { - this.messagingService.send("logout"); + this.messagingService.send("logout", { userId: this.activeUserId }); } } @@ -321,23 +327,35 @@ export class LockComponent implements OnInit, OnDestroy { } } - private async load() { + private async load(userId: UserId) { // TODO: Investigate PM-3515 // The loading of the lock component works as follows: - // 1. First, is locking a valid timeout action? If not, we will log the user out. - // 2. If locking IS a valid timeout action, we proceed to show the user the lock screen. + // 1. If the user is unlocked, we're here in error so we navigate to the home page + // 2. First, is locking a valid timeout action? If not, we will log the user out. + // 3. If locking IS a valid timeout action, we proceed to show the user the lock screen. // The user will be able to unlock as follows: // - If they have a PIN set, they will be presented with the PIN input // - If they have a master password and no PIN, they will be presented with the master password input // - If they have biometrics enabled, they will be presented with the biometric prompt + const isUnlocked = await firstValueFrom( + this.authService + .authStatusFor$(userId) + .pipe(map((status) => status === AuthenticationStatus.Unlocked)), + ); + if (isUnlocked) { + // navigate to home + await this.router.navigate(["/"]); + return; + } + const availableVaultTimeoutActions = await firstValueFrom( - this.vaultTimeoutSettingsService.availableVaultTimeoutActions$(), + this.vaultTimeoutSettingsService.availableVaultTimeoutActions$(userId), ); const supportsLock = availableVaultTimeoutActions.includes(VaultTimeoutAction.Lock); if (!supportsLock) { - return await this.vaultTimeoutService.logOut(); + return await this.vaultTimeoutService.logOut(userId); } this.pinStatus = await this.vaultTimeoutSettingsService.isPinLockSet(); diff --git a/libs/angular/src/pipes/user-name.pipe.ts b/libs/angular/src/pipes/user-name.pipe.ts index 88b088a7e2..f007f4ad87 100644 --- a/libs/angular/src/pipes/user-name.pipe.ts +++ b/libs/angular/src/pipes/user-name.pipe.ts @@ -1,6 +1,6 @@ import { Pipe, PipeTransform } from "@angular/core"; -interface User { +export interface User { name?: string; email?: string; } diff --git a/libs/angular/src/tools/send/add-edit.component.ts b/libs/angular/src/tools/send/add-edit.component.ts index da859e50bf..b4f7ec171a 100644 --- a/libs/angular/src/tools/send/add-edit.component.ts +++ b/libs/angular/src/tools/send/add-edit.component.ts @@ -5,6 +5,7 @@ import { Subject, firstValueFrom, takeUntil, map, BehaviorSubject, concatMap } f import { PolicyService } from "@bitwarden/common/admin-console/abstractions/policy/policy.service.abstraction"; import { PolicyType } from "@bitwarden/common/admin-console/enums"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; @@ -118,6 +119,7 @@ export class AddEditComponent implements OnInit, OnDestroy { protected dialogService: DialogService, protected formBuilder: FormBuilder, protected billingAccountProfileStateService: BillingAccountProfileStateService, + protected accountService: AccountService, ) { this.typeOptions = [ { name: i18nService.t("sendTypeFile"), value: SendType.File, premium: true }, @@ -215,7 +217,9 @@ export class AddEditComponent implements OnInit, OnDestroy { } async load() { - this.emailVerified = await this.stateService.getEmailVerified(); + this.emailVerified = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((a) => a?.emailVerified ?? false)), + ); this.type = !this.canAccessPremium || !this.emailVerified ? SendType.Text : SendType.File; if (this.send == null) { diff --git a/libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts index a123e30053..0efb9569eb 100644 --- a/libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts @@ -128,6 +128,7 @@ describe("AuthRequestLoginStrategy", () => { masterPasswordService.masterKeySubject.next(masterKey); cryptoService.decryptUserKeyWithMasterKey.mockResolvedValue(userKey); + tokenService.decodeAccessToken.mockResolvedValue({ sub: mockUserId }); await authRequestLoginStrategy.logIn(credentials); diff --git a/libs/auth/src/common/login-strategies/login.strategy.spec.ts b/libs/auth/src/common/login-strategies/login.strategy.spec.ts index 3284f6e947..c3a8f61d78 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.spec.ts @@ -218,7 +218,7 @@ describe("LoginStrategy", () => { expect(messagingService.send).toHaveBeenCalledWith("loggedIn"); }); - it("throws if active account isn't found after being initialized", async () => { + it("throws if new account isn't active after being initialized", async () => { const idTokenResponse = identityTokenResponseFactory(); apiService.postIdentityToken.mockResolvedValue(idTokenResponse); @@ -228,7 +228,8 @@ describe("LoginStrategy", () => { stateService.getVaultTimeoutAction.mockResolvedValue(mockVaultTimeoutAction); stateService.getVaultTimeout.mockResolvedValue(mockVaultTimeout); - accountService.activeAccountSubject.next(null); + accountService.switchAccount = jest.fn(); // block internal switch to new account + accountService.activeAccountSubject.next(null); // simulate no active account await expect(async () => await passwordLoginStrategy.logIn(credentials)).rejects.toThrow(); }); diff --git a/libs/auth/src/common/login-strategies/login.strategy.ts b/libs/auth/src/common/login-strategies/login.strategy.ts index fd268d955e..3a3109349e 100644 --- a/libs/auth/src/common/login-strategies/login.strategy.ts +++ b/libs/auth/src/common/login-strategies/login.strategy.ts @@ -169,6 +169,12 @@ export abstract class LoginStrategy { const vaultTimeoutAction = await this.stateService.getVaultTimeoutAction({ userId }); const vaultTimeout = await this.stateService.getVaultTimeout({ userId }); + await this.accountService.addAccount(userId, { + name: accountInformation.name, + email: accountInformation.email, + emailVerified: accountInformation.email_verified, + }); + // set access token and refresh token before account initialization so authN status can be accurate // User id will be derived from the access token. await this.tokenService.setTokens( @@ -178,6 +184,8 @@ export abstract class LoginStrategy { tokenResponse.refreshToken, // Note: CLI login via API key sends undefined for refresh token. ); + await this.accountService.switchAccount(userId); + await this.stateService.addAccount( new Account({ profile: { diff --git a/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts b/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts index 5c1fe9b1fe..c97639f102 100644 --- a/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts +++ b/libs/auth/src/common/login-strategies/password-login.strategy.spec.ts @@ -164,6 +164,7 @@ describe("PasswordLoginStrategy", () => { masterPasswordService.masterKeySubject.next(masterKey); cryptoService.decryptUserKeyWithMasterKey.mockResolvedValue(userKey); + tokenService.decodeAccessToken.mockResolvedValue({ sub: userId }); await passwordLoginStrategy.logIn(credentials); @@ -199,6 +200,7 @@ describe("PasswordLoginStrategy", () => { it("forces the user to update their master password on successful login when it does not meet master password policy requirements", async () => { passwordStrengthService.getPasswordStrength.mockReturnValue({ score: 0 } as any); policyService.evaluateMasterPassword.mockReturnValue(false); + tokenService.decodeAccessToken.mockResolvedValue({ sub: userId }); const result = await passwordLoginStrategy.logIn(credentials); @@ -213,6 +215,7 @@ describe("PasswordLoginStrategy", () => { it("forces the user to update their master password on successful 2FA login when it does not meet master password policy requirements", async () => { passwordStrengthService.getPasswordStrength.mockReturnValue({ score: 0 } as any); policyService.evaluateMasterPassword.mockReturnValue(false); + tokenService.decodeAccessToken.mockResolvedValue({ sub: userId }); const token2FAResponse = new IdentityTwoFactorResponse({ TwoFactorProviders: ["0"], diff --git a/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.spec.ts b/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.spec.ts index 16479f19ea..ae1813d3d7 100644 --- a/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.spec.ts +++ b/libs/auth/src/common/services/user-decryption-options/user-decryption-options.service.spec.ts @@ -65,6 +65,7 @@ describe("UserDecryptionOptionsService", () => { await fakeAccountService.addAccount(givenUser, { name: "Test User 1", email: "test1@email.com", + emailVerified: false, }); await fakeStateProvider.setUserState( USER_DECRYPTION_OPTIONS, diff --git a/libs/common/spec/fake-account-service.ts b/libs/common/spec/fake-account-service.ts index a8b09b7417..649a158d75 100644 --- a/libs/common/spec/fake-account-service.ts +++ b/libs/common/spec/fake-account-service.ts @@ -1,5 +1,5 @@ import { mock } from "jest-mock-extended"; -import { ReplaySubject } from "rxjs"; +import { ReplaySubject, combineLatest, map } from "rxjs"; import { AccountInfo, AccountService } from "../src/auth/abstractions/account.service"; import { UserId } from "../src/types/guid"; @@ -7,15 +7,20 @@ import { UserId } from "../src/types/guid"; export function mockAccountServiceWith( userId: UserId, info: Partial = {}, + activity: Record = {}, ): FakeAccountService { const fullInfo: AccountInfo = { ...info, ...{ name: "name", email: "email", + emailVerified: true, }, }; - const service = new FakeAccountService({ [userId]: fullInfo }); + + const fullActivity = { [userId]: new Date(), ...activity }; + + const service = new FakeAccountService({ [userId]: fullInfo }, fullActivity); service.activeAccountSubject.next({ id: userId, ...fullInfo }); return service; } @@ -26,17 +31,46 @@ export class FakeAccountService implements AccountService { accountsSubject = new ReplaySubject>(1); // eslint-disable-next-line rxjs/no-exposed-subjects -- test class activeAccountSubject = new ReplaySubject<{ id: UserId } & AccountInfo>(1); + // eslint-disable-next-line rxjs/no-exposed-subjects -- test class + accountActivitySubject = new ReplaySubject>(1); private _activeUserId: UserId; get activeUserId() { return this._activeUserId; } accounts$ = this.accountsSubject.asObservable(); activeAccount$ = this.activeAccountSubject.asObservable(); + accountActivity$ = this.accountActivitySubject.asObservable(); + get sortedUserIds$() { + return this.accountActivity$.pipe( + map((activity) => { + return Object.entries(activity) + .map(([userId, lastActive]: [UserId, Date]) => ({ userId, lastActive })) + .sort((a, b) => a.lastActive.getTime() - b.lastActive.getTime()) + .map((a) => a.userId); + }), + ); + } + get nextUpAccount$() { + return combineLatest([this.accounts$, this.activeAccount$, this.sortedUserIds$]).pipe( + map(([accounts, activeAccount, sortedUserIds]) => { + const nextId = sortedUserIds.find((id) => id !== activeAccount?.id && accounts[id] != null); + return nextId ? { id: nextId, ...accounts[nextId] } : null; + }), + ); + } - constructor(initialData: Record) { + constructor(initialData: Record, accountActivity?: Record) { this.accountsSubject.next(initialData); this.activeAccountSubject.subscribe((data) => (this._activeUserId = data?.id)); this.activeAccountSubject.next(null); + this.accountActivitySubject.next(accountActivity); + } + setAccountActivity(userId: UserId, lastActivity: Date): Promise { + this.accountActivitySubject.next({ + ...this.accountActivitySubject["_buffer"][0], + [userId]: lastActivity, + }); + return this.mock.setAccountActivity(userId, lastActivity); } async addAccount(userId: UserId, accountData: AccountInfo): Promise { @@ -53,10 +87,27 @@ export class FakeAccountService implements AccountService { await this.mock.setAccountEmail(userId, email); } + async setAccountEmailVerified(userId: UserId, emailVerified: boolean): Promise { + await this.mock.setAccountEmailVerified(userId, emailVerified); + } + async switchAccount(userId: UserId): Promise { const next = userId == null ? null : { id: userId, ...this.accountsSubject["_buffer"]?.[0]?.[userId] }; this.activeAccountSubject.next(next); await this.mock.switchAccount(userId); } + + async clean(userId: UserId): Promise { + const current = this.accountsSubject["_buffer"][0] ?? {}; + const updated = { ...current, [userId]: loggedOutInfo }; + this.accountsSubject.next(updated); + await this.mock.clean(userId); + } } + +const loggedOutInfo: AccountInfo = { + name: undefined, + email: "", + emailVerified: false, +}; diff --git a/libs/common/src/auth/abstractions/account.service.ts b/libs/common/src/auth/abstractions/account.service.ts index fa9ad36378..b7fd6d9bb9 100644 --- a/libs/common/src/auth/abstractions/account.service.ts +++ b/libs/common/src/auth/abstractions/account.service.ts @@ -8,18 +8,44 @@ import { UserId } from "../../types/guid"; */ export type AccountInfo = { email: string; + emailVerified: boolean; name: string | undefined; }; export function accountInfoEqual(a: AccountInfo, b: AccountInfo) { - return a?.email === b?.email && a?.name === b?.name; + if (a == null && b == null) { + return true; + } + + if (a == null || b == null) { + return false; + } + + const keys = new Set([...Object.keys(a), ...Object.keys(b)]) as Set; + for (const key of keys) { + if (a[key] !== b[key]) { + return false; + } + } + return true; } export abstract class AccountService { accounts$: Observable>; activeAccount$: Observable<{ id: UserId | undefined } & AccountInfo>; + + /** + * Observable of the last activity time for each account. + */ + accountActivity$: Observable>; + /** Account list in order of descending recency */ + sortedUserIds$: Observable; + /** Next account that is not the current active account */ + nextUpAccount$: Observable<{ id: UserId } & AccountInfo>; /** * Updates the `accounts$` observable with the new account data. + * + * @note Also sets the last active date of the account to `now`. * @param userId * @param accountData */ @@ -36,11 +62,30 @@ export abstract class AccountService { * @param email */ abstract setAccountEmail(userId: UserId, email: string): Promise; + /** + * updates the `accounts$` observable with the new email verification status for the account. + * @param userId + * @param emailVerified + */ + abstract setAccountEmailVerified(userId: UserId, emailVerified: boolean): Promise; /** * Updates the `activeAccount$` observable with the new active account. * @param userId */ abstract switchAccount(userId: UserId): Promise; + /** + * Cleans personal information for the given account from the `accounts$` observable. Does not remove the userId from the observable. + * + * @note Also sets the last active date of the account to `null`. + * @param userId + */ + abstract clean(userId: UserId): Promise; + /** + * Updates the given user's last activity time. + * @param userId + * @param lastActivity + */ + abstract setAccountActivity(userId: UserId, lastActivity: Date): Promise; } export abstract class InternalAccountService extends AccountService { diff --git a/libs/common/src/auth/services/account.service.spec.ts b/libs/common/src/auth/services/account.service.spec.ts index a9cec82c51..0ae14b0cc1 100644 --- a/libs/common/src/auth/services/account.service.spec.ts +++ b/libs/common/src/auth/services/account.service.spec.ts @@ -1,3 +1,8 @@ +/** + * need to update test environment so structuredClone works appropriately + * @jest-environment ../../libs/shared/test.environment.ts + */ + import { MockProxy, mock } from "jest-mock-extended"; import { firstValueFrom } from "rxjs"; @@ -6,15 +11,57 @@ import { FakeGlobalStateProvider } from "../../../spec/fake-state-provider"; import { trackEmissions } from "../../../spec/utils"; import { LogService } from "../../platform/abstractions/log.service"; import { MessagingService } from "../../platform/abstractions/messaging.service"; +import { Utils } from "../../platform/misc/utils"; import { UserId } from "../../types/guid"; -import { AccountInfo } from "../abstractions/account.service"; +import { AccountInfo, accountInfoEqual } from "../abstractions/account.service"; import { ACCOUNT_ACCOUNTS, ACCOUNT_ACTIVE_ACCOUNT_ID, + ACCOUNT_ACTIVITY, AccountServiceImplementation, } from "./account.service"; +describe("accountInfoEqual", () => { + const accountInfo: AccountInfo = { name: "name", email: "email", emailVerified: true }; + + it("compares nulls", () => { + expect(accountInfoEqual(null, null)).toBe(true); + expect(accountInfoEqual(null, accountInfo)).toBe(false); + expect(accountInfoEqual(accountInfo, null)).toBe(false); + }); + + it("compares all keys, not just those defined in AccountInfo", () => { + const different = { ...accountInfo, extra: "extra" }; + + expect(accountInfoEqual(accountInfo, different)).toBe(false); + }); + + it("compares name", () => { + const same = { ...accountInfo }; + const different = { ...accountInfo, name: "name2" }; + + expect(accountInfoEqual(accountInfo, same)).toBe(true); + expect(accountInfoEqual(accountInfo, different)).toBe(false); + }); + + it("compares email", () => { + const same = { ...accountInfo }; + const different = { ...accountInfo, email: "email2" }; + + expect(accountInfoEqual(accountInfo, same)).toBe(true); + expect(accountInfoEqual(accountInfo, different)).toBe(false); + }); + + it("compares emailVerified", () => { + const same = { ...accountInfo }; + const different = { ...accountInfo, emailVerified: false }; + + expect(accountInfoEqual(accountInfo, same)).toBe(true); + expect(accountInfoEqual(accountInfo, different)).toBe(false); + }); +}); + describe("accountService", () => { let messagingService: MockProxy; let logService: MockProxy; @@ -22,8 +69,8 @@ describe("accountService", () => { let sut: AccountServiceImplementation; let accountsState: FakeGlobalState>; let activeAccountIdState: FakeGlobalState; - const userId = "userId" as UserId; - const userInfo = { email: "email", name: "name" }; + const userId = Utils.newGuid() as UserId; + const userInfo = { email: "email", name: "name", emailVerified: true }; beforeEach(() => { messagingService = mock(); @@ -86,6 +133,25 @@ describe("accountService", () => { expect(currentValue).toEqual({ [userId]: userInfo }); }); + + it("sets the last active date of the account to now", async () => { + const state = globalStateProvider.getFake(ACCOUNT_ACTIVITY); + state.stateSubject.next({}); + await sut.addAccount(userId, userInfo); + + expect(state.nextMock).toHaveBeenCalledWith({ [userId]: expect.any(Date) }); + }); + + it.each([null, undefined, 123, "not a guid"])( + "does not set last active if the userId is not a valid guid", + async (userId) => { + const state = globalStateProvider.getFake(ACCOUNT_ACTIVITY); + state.stateSubject.next({}); + await expect(sut.addAccount(userId as UserId, userInfo)).rejects.toThrow( + "userId is required", + ); + }, + ); }); describe("setAccountName", () => { @@ -134,6 +200,58 @@ describe("accountService", () => { }); }); + describe("setAccountEmailVerified", () => { + const initialState = { [userId]: userInfo }; + initialState[userId].emailVerified = false; + beforeEach(() => { + accountsState.stateSubject.next(initialState); + }); + + it("should update the account", async () => { + await sut.setAccountEmailVerified(userId, true); + const currentState = await firstValueFrom(accountsState.state$); + + expect(currentState).toEqual({ + [userId]: { ...userInfo, emailVerified: true }, + }); + }); + + it("should not update if the email is the same", async () => { + await sut.setAccountEmailVerified(userId, false); + const currentState = await firstValueFrom(accountsState.state$); + + expect(currentState).toEqual(initialState); + }); + }); + + describe("clean", () => { + beforeEach(() => { + accountsState.stateSubject.next({ [userId]: userInfo }); + }); + + it("removes account info of the given user", async () => { + await sut.clean(userId); + const currentState = await firstValueFrom(accountsState.state$); + + expect(currentState).toEqual({ + [userId]: { + email: "", + emailVerified: false, + name: undefined, + }, + }); + }); + + it("removes account activity of the given user", async () => { + const state = globalStateProvider.getFake(ACCOUNT_ACTIVITY); + state.stateSubject.next({ [userId]: new Date() }); + + await sut.clean(userId); + + expect(state.nextMock).toHaveBeenCalledWith({}); + }); + }); + describe("switchAccount", () => { beforeEach(() => { accountsState.stateSubject.next({ [userId]: userInfo }); @@ -152,4 +270,83 @@ describe("accountService", () => { expect(sut.switchAccount("unknown" as UserId)).rejects.toThrowError("Account does not exist"); }); }); + + describe("account activity", () => { + let state: FakeGlobalState>; + + beforeEach(() => { + state = globalStateProvider.getFake(ACCOUNT_ACTIVITY); + }); + describe("accountActivity$", () => { + it("returns the account activity state", async () => { + state.stateSubject.next({ + [toId("user1")]: new Date(1), + [toId("user2")]: new Date(2), + }); + + await expect(firstValueFrom(sut.accountActivity$)).resolves.toEqual({ + [toId("user1")]: new Date(1), + [toId("user2")]: new Date(2), + }); + }); + + it("returns an empty object when account activity is null", async () => { + state.stateSubject.next(null); + + await expect(firstValueFrom(sut.accountActivity$)).resolves.toEqual({}); + }); + }); + + describe("sortedUserIds$", () => { + it("returns the sorted user ids by date with most recent first", async () => { + state.stateSubject.next({ + [toId("user1")]: new Date(3), + [toId("user2")]: new Date(2), + [toId("user3")]: new Date(1), + }); + + await expect(firstValueFrom(sut.sortedUserIds$)).resolves.toEqual([ + "user1" as UserId, + "user2" as UserId, + "user3" as UserId, + ]); + }); + + it("returns an empty array when account activity is null", async () => { + state.stateSubject.next(null); + + await expect(firstValueFrom(sut.sortedUserIds$)).resolves.toEqual([]); + }); + }); + + describe("setAccountActivity", () => { + const userId = Utils.newGuid() as UserId; + it("sets the account activity", async () => { + await sut.setAccountActivity(userId, new Date(1)); + + expect(state.nextMock).toHaveBeenCalledWith({ [userId]: new Date(1) }); + }); + + it("does not update if the activity is the same", async () => { + state.stateSubject.next({ [userId]: new Date(1) }); + + await sut.setAccountActivity(userId, new Date(1)); + + expect(state.nextMock).not.toHaveBeenCalled(); + }); + + it.each([null, undefined, 123, "not a guid"])( + "does not set last active if the userId is not a valid guid", + async (userId) => { + await sut.setAccountActivity(userId as UserId, new Date(1)); + + expect(state.nextMock).not.toHaveBeenCalled(); + }, + ); + }); + }); }); + +function toId(userId: string) { + return userId as UserId; +} diff --git a/libs/common/src/auth/services/account.service.ts b/libs/common/src/auth/services/account.service.ts index 77d61fae91..6740387ded 100644 --- a/libs/common/src/auth/services/account.service.ts +++ b/libs/common/src/auth/services/account.service.ts @@ -1,4 +1,4 @@ -import { Subject, combineLatestWith, map, distinctUntilChanged, shareReplay } from "rxjs"; +import { combineLatestWith, map, distinctUntilChanged, shareReplay, combineLatest } from "rxjs"; import { AccountInfo, @@ -7,8 +7,9 @@ import { } from "../../auth/abstractions/account.service"; import { LogService } from "../../platform/abstractions/log.service"; import { MessagingService } from "../../platform/abstractions/messaging.service"; +import { Utils } from "../../platform/misc/utils"; import { - ACCOUNT_MEMORY, + ACCOUNT_DISK, GlobalState, GlobalStateProvider, KeyDefinition, @@ -16,25 +17,36 @@ import { import { UserId } from "../../types/guid"; export const ACCOUNT_ACCOUNTS = KeyDefinition.record( - ACCOUNT_MEMORY, + ACCOUNT_DISK, "accounts", { deserializer: (accountInfo) => accountInfo, }, ); -export const ACCOUNT_ACTIVE_ACCOUNT_ID = new KeyDefinition(ACCOUNT_MEMORY, "activeAccountId", { +export const ACCOUNT_ACTIVE_ACCOUNT_ID = new KeyDefinition(ACCOUNT_DISK, "activeAccountId", { deserializer: (id: UserId) => id, }); +export const ACCOUNT_ACTIVITY = KeyDefinition.record(ACCOUNT_DISK, "activity", { + deserializer: (activity) => new Date(activity), +}); + +const LOGGED_OUT_INFO: AccountInfo = { + email: "", + emailVerified: false, + name: undefined, +}; + export class AccountServiceImplementation implements InternalAccountService { - private lock = new Subject(); - private logout = new Subject(); private accountsState: GlobalState>; private activeAccountIdState: GlobalState; accounts$; activeAccount$; + accountActivity$; + sortedUserIds$; + nextUpAccount$; constructor( private messagingService: MessagingService, @@ -53,14 +65,40 @@ export class AccountServiceImplementation implements InternalAccountService { distinctUntilChanged((a, b) => a?.id === b?.id && accountInfoEqual(a, b)), shareReplay({ bufferSize: 1, refCount: false }), ); + this.accountActivity$ = this.globalStateProvider + .get(ACCOUNT_ACTIVITY) + .state$.pipe(map((activity) => activity ?? {})); + this.sortedUserIds$ = this.accountActivity$.pipe( + map((activity) => { + return Object.entries(activity) + .map(([userId, lastActive]: [UserId, Date]) => ({ userId, lastActive })) + .sort((a, b) => b.lastActive.getTime() - a.lastActive.getTime()) // later dates first + .map((a) => a.userId); + }), + ); + this.nextUpAccount$ = combineLatest([ + this.accounts$, + this.activeAccount$, + this.sortedUserIds$, + ]).pipe( + map(([accounts, activeAccount, sortedUserIds]) => { + const nextId = sortedUserIds.find((id) => id !== activeAccount?.id && accounts[id] != null); + return nextId ? { id: nextId, ...accounts[nextId] } : null; + }), + ); } async addAccount(userId: UserId, accountData: AccountInfo): Promise { + if (!Utils.isGuid(userId)) { + throw new Error("userId is required"); + } + await this.accountsState.update((accounts) => { accounts ||= {}; accounts[userId] = accountData; return accounts; }); + await this.setAccountActivity(userId, new Date()); } async setAccountName(userId: UserId, name: string): Promise { @@ -71,6 +109,15 @@ export class AccountServiceImplementation implements InternalAccountService { await this.setAccountInfo(userId, { email }); } + async setAccountEmailVerified(userId: UserId, emailVerified: boolean): Promise { + await this.setAccountInfo(userId, { emailVerified }); + } + + async clean(userId: UserId) { + await this.setAccountInfo(userId, LOGGED_OUT_INFO); + await this.removeAccountActivity(userId); + } + async switchAccount(userId: UserId): Promise { await this.activeAccountIdState.update( (_, accounts) => { @@ -94,6 +141,37 @@ export class AccountServiceImplementation implements InternalAccountService { ); } + async setAccountActivity(userId: UserId, lastActivity: Date): Promise { + if (!Utils.isGuid(userId)) { + // only store for valid userIds + return; + } + + await this.globalStateProvider.get(ACCOUNT_ACTIVITY).update( + (activity) => { + activity ||= {}; + activity[userId] = lastActivity; + return activity; + }, + { + shouldUpdate: (oldActivity) => oldActivity?.[userId]?.getTime() !== lastActivity?.getTime(), + }, + ); + } + + async removeAccountActivity(userId: UserId): Promise { + await this.globalStateProvider.get(ACCOUNT_ACTIVITY).update( + (activity) => { + if (activity == null) { + return activity; + } + delete activity[userId]; + return activity; + }, + { shouldUpdate: (oldActivity) => oldActivity?.[userId] != null }, + ); + } + // TODO: update to use our own account status settings. Requires inverting direction of state service accounts flow async delete(): Promise { try { diff --git a/libs/common/src/auth/services/auth.service.spec.ts b/libs/common/src/auth/services/auth.service.spec.ts index 3bdf85d3e1..9a93a4207b 100644 --- a/libs/common/src/auth/services/auth.service.spec.ts +++ b/libs/common/src/auth/services/auth.service.spec.ts @@ -56,6 +56,7 @@ describe("AuthService", () => { status: AuthenticationStatus.Unlocked, id: userId, email: "email", + emailVerified: false, name: "name", }; @@ -109,6 +110,7 @@ describe("AuthService", () => { status: AuthenticationStatus.Unlocked, id: Utils.newGuid() as UserId, email: "email2", + emailVerified: false, name: "name2", }; @@ -126,7 +128,11 @@ describe("AuthService", () => { it("requests auth status for all known users", async () => { const userId2 = Utils.newGuid() as UserId; - await accountService.addAccount(userId2, { email: "email2", name: "name2" }); + await accountService.addAccount(userId2, { + email: "email2", + emailVerified: false, + name: "name2", + }); const mockFn = jest.fn().mockReturnValue(of(AuthenticationStatus.Locked)); sut.authStatusFor$ = mockFn; @@ -147,11 +153,14 @@ describe("AuthService", () => { cryptoService.getInMemoryUserKeyFor$.mockReturnValue(of(undefined)); }); - it("emits LoggedOut when userId is null", async () => { - expect(await firstValueFrom(sut.authStatusFor$(null))).toEqual( - AuthenticationStatus.LoggedOut, - ); - }); + it.each([null, undefined, "not a userId"])( + "emits LoggedOut when userId is invalid (%s)", + async () => { + expect(await firstValueFrom(sut.authStatusFor$(null))).toEqual( + AuthenticationStatus.LoggedOut, + ); + }, + ); it("emits LoggedOut when there is no access token", async () => { tokenService.hasAccessToken$.mockReturnValue(of(false)); diff --git a/libs/common/src/auth/services/auth.service.ts b/libs/common/src/auth/services/auth.service.ts index c9e711b4cc..a4529084a2 100644 --- a/libs/common/src/auth/services/auth.service.ts +++ b/libs/common/src/auth/services/auth.service.ts @@ -2,6 +2,7 @@ import { Observable, combineLatest, distinctUntilChanged, + firstValueFrom, map, of, shareReplay, @@ -12,6 +13,7 @@ import { ApiService } from "../../abstractions/api.service"; import { CryptoService } from "../../platform/abstractions/crypto.service"; import { MessagingService } from "../../platform/abstractions/messaging.service"; import { StateService } from "../../platform/abstractions/state.service"; +import { Utils } from "../../platform/misc/utils"; import { UserId } from "../../types/guid"; import { AccountService } from "../abstractions/account.service"; import { AuthService as AuthServiceAbstraction } from "../abstractions/auth.service"; @@ -39,13 +41,16 @@ export class AuthService implements AuthServiceAbstraction { this.authStatuses$ = this.accountService.accounts$.pipe( map((accounts) => Object.keys(accounts) as UserId[]), - switchMap((entries) => - combineLatest( + switchMap((entries) => { + if (entries.length === 0) { + return of([] as { userId: UserId; status: AuthenticationStatus }[]); + } + return combineLatest( entries.map((userId) => this.authStatusFor$(userId).pipe(map((status) => ({ userId, status }))), ), - ), - ), + ); + }), map((statuses) => { return statuses.reduce( (acc, { userId, status }) => { @@ -59,7 +64,7 @@ export class AuthService implements AuthServiceAbstraction { } authStatusFor$(userId: UserId): Observable { - if (userId == null) { + if (!Utils.isGuid(userId)) { return of(AuthenticationStatus.LoggedOut); } @@ -84,17 +89,8 @@ export class AuthService implements AuthServiceAbstraction { } async getAuthStatus(userId?: string): Promise { - // If we don't have an access token or userId, we're logged out - const isAuthenticated = await this.stateService.getIsAuthenticated({ userId: userId }); - if (!isAuthenticated) { - return AuthenticationStatus.LoggedOut; - } - - // Note: since we aggresively set the auto user key to memory if it exists on app init (see InitService) - // we only need to check if the user key is in memory. - const hasUserKey = await this.cryptoService.hasUserKeyInMemory(userId as UserId); - - return hasUserKey ? AuthenticationStatus.Unlocked : AuthenticationStatus.Locked; + userId ??= await firstValueFrom(this.accountService.activeAccount$.pipe(map((a) => a?.id))); + return await firstValueFrom(this.authStatusFor$(userId as UserId)); } logOut(callback: () => void) { diff --git a/libs/common/src/auth/services/password-reset-enrollment.service.implementation.spec.ts b/libs/common/src/auth/services/password-reset-enrollment.service.implementation.spec.ts index fc5060af5f..19b29f0593 100644 --- a/libs/common/src/auth/services/password-reset-enrollment.service.implementation.spec.ts +++ b/libs/common/src/auth/services/password-reset-enrollment.service.implementation.spec.ts @@ -90,6 +90,7 @@ describe("PasswordResetEnrollmentServiceImplementation", () => { const user1AccountInfo: AccountInfo = { name: "Test User 1", email: "test1@email.com", + emailVerified: true, }; activeAccountSubject.next(Object.assign(user1AccountInfo, { id: "userId" as UserId })); diff --git a/libs/common/src/platform/abstractions/state.service.ts b/libs/common/src/platform/abstractions/state.service.ts index 13c33305d1..5ca604b526 100644 --- a/libs/common/src/platform/abstractions/state.service.ts +++ b/libs/common/src/platform/abstractions/state.service.ts @@ -25,11 +25,10 @@ export type InitOptions = { export abstract class StateService { accounts$: Observable<{ [userId: string]: T }>; - activeAccount$: Observable; addAccount: (account: T) => Promise; - setActiveUser: (userId: string) => Promise; - clean: (options?: StorageOptions) => Promise; + clearDecryptedData: (userId: UserId) => Promise; + clean: (options?: StorageOptions) => Promise; init: (initOptions?: InitOptions) => Promise; /** @@ -122,8 +121,6 @@ export abstract class StateService { setDuckDuckGoSharedKey: (value: string, options?: StorageOptions) => Promise; getEmail: (options?: StorageOptions) => Promise; setEmail: (value: string, options?: StorageOptions) => Promise; - getEmailVerified: (options?: StorageOptions) => Promise; - setEmailVerified: (value: boolean, options?: StorageOptions) => Promise; getEnableBrowserIntegration: (options?: StorageOptions) => Promise; setEnableBrowserIntegration: (value: boolean, options?: StorageOptions) => Promise; getEnableBrowserIntegrationFingerprint: (options?: StorageOptions) => Promise; @@ -147,8 +144,6 @@ export abstract class StateService { */ setEncryptedPinProtected: (value: string, options?: StorageOptions) => Promise; getIsAuthenticated: (options?: StorageOptions) => Promise; - getLastActive: (options?: StorageOptions) => Promise; - setLastActive: (value: number, options?: StorageOptions) => Promise; getLastSync: (options?: StorageOptions) => Promise; setLastSync: (value: string, options?: StorageOptions) => Promise; getMinimizeOnCopyToClipboard: (options?: StorageOptions) => Promise; @@ -180,5 +175,4 @@ export abstract class StateService { setVaultTimeout: (value: number, options?: StorageOptions) => Promise; getVaultTimeoutAction: (options?: StorageOptions) => Promise; setVaultTimeoutAction: (value: string, options?: StorageOptions) => Promise; - nextUpActiveUser: () => Promise; } diff --git a/libs/common/src/platform/misc/utils.spec.ts b/libs/common/src/platform/misc/utils.spec.ts index a7a520a77c..964a2a1941 100644 --- a/libs/common/src/platform/misc/utils.spec.ts +++ b/libs/common/src/platform/misc/utils.spec.ts @@ -3,6 +3,33 @@ import * as path from "path"; import { Utils } from "./utils"; describe("Utils Service", () => { + describe("isGuid", () => { + it("is false when null", () => { + expect(Utils.isGuid(null)).toBe(false); + }); + + it("is false when undefined", () => { + expect(Utils.isGuid(undefined)).toBe(false); + }); + + it("is false when empty", () => { + expect(Utils.isGuid("")).toBe(false); + }); + + it("is false when not a string", () => { + expect(Utils.isGuid(123 as any)).toBe(false); + }); + + it("is false when not a guid", () => { + expect(Utils.isGuid("not a guid")).toBe(false); + }); + + it("is true when a guid", () => { + // we use a limited guid scope in which all zeroes is invalid + expect(Utils.isGuid("00000000-0000-1000-8000-000000000000")).toBe(true); + }); + }); + describe("getDomain", () => { it("should fail for invalid urls", () => { expect(Utils.getDomain(null)).toBeNull(); diff --git a/libs/common/src/platform/models/domain/state.ts b/libs/common/src/platform/models/domain/state.ts index 95557e082a..5dde49f99d 100644 --- a/libs/common/src/platform/models/domain/state.ts +++ b/libs/common/src/platform/models/domain/state.ts @@ -9,9 +9,6 @@ export class State< > { accounts: { [userId: string]: TAccount } = {}; globals: TGlobalState; - activeUserId: string; - authenticatedAccounts: string[] = []; - accountActivity: { [userId: string]: number } = {}; constructor(globals: TGlobalState) { this.globals = globals; diff --git a/libs/common/src/platform/services/default-environment.service.spec.ts b/libs/common/src/platform/services/default-environment.service.spec.ts index dd504dc302..7d266e93fc 100644 --- a/libs/common/src/platform/services/default-environment.service.spec.ts +++ b/libs/common/src/platform/services/default-environment.service.spec.ts @@ -31,10 +31,12 @@ describe("EnvironmentService", () => { [testUser]: { name: "name", email: "email", + emailVerified: false, }, [alternateTestUser]: { name: "name", email: "email", + emailVerified: false, }, }); stateProvider = new FakeStateProvider(accountService); @@ -47,6 +49,7 @@ describe("EnvironmentService", () => { id: userId, email: "test@example.com", name: `Test Name ${userId}`, + emailVerified: false, }); await awaitAsync(); }; diff --git a/libs/common/src/platform/services/state.service.ts b/libs/common/src/platform/services/state.service.ts index cab5768d2a..9479d64710 100644 --- a/libs/common/src/platform/services/state.service.ts +++ b/libs/common/src/platform/services/state.service.ts @@ -1,4 +1,4 @@ -import { BehaviorSubject } from "rxjs"; +import { BehaviorSubject, firstValueFrom, map } from "rxjs"; import { Jsonify, JsonValue } from "type-fest"; import { AccountService } from "../../auth/abstractions/account.service"; @@ -33,10 +33,7 @@ const keys = { state: "state", stateVersion: "stateVersion", global: "global", - authenticatedAccounts: "authenticatedAccounts", - activeUserId: "activeUserId", tempAccountSettings: "tempAccountSettings", // used to hold account specific settings (i.e clear clipboard) between initial migration and first account authentication - accountActivity: "accountActivity", }; const partialKeys = { @@ -58,9 +55,6 @@ export class StateService< protected accountsSubject = new BehaviorSubject<{ [userId: string]: TAccount }>({}); accounts$ = this.accountsSubject.asObservable(); - protected activeAccountSubject = new BehaviorSubject(null); - activeAccount$ = this.activeAccountSubject.asObservable(); - private hasBeenInited = false; protected isRecoveredSession = false; @@ -112,36 +106,16 @@ export class StateService< } // Get all likely authenticated accounts - const authenticatedAccounts = ( - (await this.storageService.get(keys.authenticatedAccounts)) ?? [] - ).filter((account) => account != null); + const authenticatedAccounts = await firstValueFrom( + this.accountService.accounts$.pipe(map((accounts) => Object.keys(accounts))), + ); await this.updateState(async (state) => { for (const i in authenticatedAccounts) { state = await this.syncAccountFromDisk(authenticatedAccounts[i]); } - // After all individual accounts have been added - state.authenticatedAccounts = authenticatedAccounts; - - const storedActiveUser = await this.storageService.get(keys.activeUserId); - if (storedActiveUser != null) { - state.activeUserId = storedActiveUser; - } await this.pushAccounts(); - this.activeAccountSubject.next(state.activeUserId); - // TODO: Temporary update to avoid routing all account status changes through account service for now. - // account service tracks logged out accounts, but State service does not, so we need to add the active account - // if it's not in the accounts list. - if (state.activeUserId != null && this.accountsSubject.value[state.activeUserId] == null) { - const activeDiskAccount = await this.getAccountFromDisk({ userId: state.activeUserId }); - await this.accountService.addAccount(state.activeUserId as UserId, { - name: activeDiskAccount.profile.name, - email: activeDiskAccount.profile.email, - }); - } - await this.accountService.switchAccount(state.activeUserId as UserId); - // End TODO return state; }); @@ -161,61 +135,25 @@ export class StateService< return state; }); - // TODO: Temporary update to avoid routing all account status changes through account service for now. - // The determination of state should be handled by the various services that control those values. - await this.accountService.addAccount(userId as UserId, { - name: diskAccount.profile.name, - email: diskAccount.profile.email, - }); - return state; } async addAccount(account: TAccount) { await this.environmentService.seedUserEnvironment(account.profile.userId as UserId); await this.updateState(async (state) => { - state.authenticatedAccounts.push(account.profile.userId); - await this.storageService.save(keys.authenticatedAccounts, state.authenticatedAccounts); state.accounts[account.profile.userId] = account; return state; }); await this.scaffoldNewAccountStorage(account); - await this.setLastActive(new Date().getTime(), { userId: account.profile.userId }); - // TODO: Temporary update to avoid routing all account status changes through account service for now. - await this.accountService.addAccount(account.profile.userId as UserId, { - name: account.profile.name, - email: account.profile.email, - }); - await this.setActiveUser(account.profile.userId); } - async setActiveUser(userId: string): Promise { - await this.clearDecryptedDataForActiveUser(); - await this.updateState(async (state) => { - state.activeUserId = userId; - await this.storageService.save(keys.activeUserId, userId); - this.activeAccountSubject.next(state.activeUserId); - // TODO: temporary update to avoid routing all account status changes through account service for now. - await this.accountService.switchAccount(userId as UserId); - - return state; - }); - - await this.pushAccounts(); - } - - async clean(options?: StorageOptions): Promise { + async clean(options?: StorageOptions): Promise { options = this.reconcileOptions(options, await this.defaultInMemoryOptions()); await this.deAuthenticateAccount(options.userId); - let currentUser = (await this.state())?.activeUserId; - if (options.userId === currentUser) { - currentUser = await this.dynamicallySetActiveUser(); - } await this.removeAccountFromDisk(options?.userId); await this.removeAccountFromMemory(options?.userId); await this.pushAccounts(); - return currentUser as UserId; } /** @@ -515,24 +453,6 @@ export class StateService< ); } - async getEmailVerified(options?: StorageOptions): Promise { - return ( - (await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskOptions()))) - ?.profile.emailVerified ?? false - ); - } - - async setEmailVerified(value: boolean, options?: StorageOptions): Promise { - const account = await this.getAccount( - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - account.profile.emailVerified = value; - await this.saveAccount( - account, - this.reconcileOptions(options, await this.defaultOnDiskOptions()), - ); - } - async getEnableBrowserIntegration(options?: StorageOptions): Promise { return ( (await this.getGlobals(this.reconcileOptions(options, await this.defaultOnDiskOptions()))) @@ -642,35 +562,6 @@ export class StateService< ); } - async getLastActive(options?: StorageOptions): Promise { - options = this.reconcileOptions(options, await this.defaultOnDiskOptions()); - - const accountActivity = await this.storageService.get<{ [userId: string]: number }>( - keys.accountActivity, - options, - ); - - if (accountActivity == null || Object.keys(accountActivity).length < 1) { - return null; - } - - return accountActivity[options.userId]; - } - - async setLastActive(value: number, options?: StorageOptions): Promise { - options = this.reconcileOptions(options, await this.defaultOnDiskOptions()); - if (options.userId == null) { - return; - } - const accountActivity = - (await this.storageService.get<{ [userId: string]: number }>( - keys.accountActivity, - options, - )) ?? {}; - accountActivity[options.userId] = value; - await this.storageService.save(keys.accountActivity, accountActivity, options); - } - async getLastSync(options?: StorageOptions): Promise { return ( await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskMemoryOptions())) @@ -910,24 +801,28 @@ export class StateService< } protected async getAccountFromMemory(options: StorageOptions): Promise { + const userId = + options.userId ?? + (await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + )); + return await this.state().then(async (state) => { if (state.accounts == null) { return null; } - return state.accounts[await this.getUserIdFromMemory(options)]; - }); - } - - protected async getUserIdFromMemory(options: StorageOptions): Promise { - return await this.state().then((state) => { - return options?.userId != null - ? state.accounts[options.userId]?.profile?.userId - : state.activeUserId; + return state.accounts[userId]; }); } protected async getAccountFromDisk(options: StorageOptions): Promise { - if (options?.userId == null && (await this.state())?.activeUserId == null) { + const userId = + options.userId ?? + (await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + )); + + if (userId == null) { return null; } @@ -1086,53 +981,76 @@ export class StateService< } protected async defaultInMemoryOptions(): Promise { + const userId = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + return { storageLocation: StorageLocation.Memory, - userId: (await this.state()).activeUserId, + userId, }; } protected async defaultOnDiskOptions(): Promise { + const userId = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + return { storageLocation: StorageLocation.Disk, htmlStorageLocation: HtmlStorageLocation.Session, - userId: (await this.state())?.activeUserId ?? (await this.getActiveUserIdFromStorage()), + userId, useSecureStorage: false, }; } protected async defaultOnDiskLocalOptions(): Promise { + const userId = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + return { storageLocation: StorageLocation.Disk, htmlStorageLocation: HtmlStorageLocation.Local, - userId: (await this.state())?.activeUserId ?? (await this.getActiveUserIdFromStorage()), + userId, useSecureStorage: false, }; } protected async defaultOnDiskMemoryOptions(): Promise { + const userId = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + return { storageLocation: StorageLocation.Disk, htmlStorageLocation: HtmlStorageLocation.Memory, - userId: (await this.state())?.activeUserId ?? (await this.getUserId()), + userId, useSecureStorage: false, }; } protected async defaultSecureStorageOptions(): Promise { + const userId = await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + return { storageLocation: StorageLocation.Disk, useSecureStorage: true, - userId: (await this.state())?.activeUserId ?? (await this.getActiveUserIdFromStorage()), + userId, }; } protected async getActiveUserIdFromStorage(): Promise { - return await this.storageService.get(keys.activeUserId); + return await firstValueFrom(this.accountService.activeAccount$.pipe(map((a) => a?.id))); } protected async removeAccountFromLocalStorage(userId: string = null): Promise { - userId = userId ?? (await this.state())?.activeUserId; + userId ??= await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + const storedAccount = await this.getAccount( this.reconcileOptions({ userId: userId }, await this.defaultOnDiskLocalOptions()), ); @@ -1143,7 +1061,10 @@ export class StateService< } protected async removeAccountFromSessionStorage(userId: string = null): Promise { - userId = userId ?? (await this.state())?.activeUserId; + userId ??= await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + const storedAccount = await this.getAccount( this.reconcileOptions({ userId: userId }, await this.defaultOnDiskOptions()), ); @@ -1154,7 +1075,10 @@ export class StateService< } protected async removeAccountFromSecureStorage(userId: string = null): Promise { - userId = userId ?? (await this.state())?.activeUserId; + userId ??= await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + await this.setUserKeyAutoUnlock(null, { userId: userId }); await this.setUserKeyBiometric(null, { userId: userId }); await this.setCryptoMasterKeyAuto(null, { userId: userId }); @@ -1163,8 +1087,11 @@ export class StateService< } protected async removeAccountFromMemory(userId: string = null): Promise { + userId ??= await firstValueFrom( + this.accountService.activeAccount$.pipe(map((account) => account?.id)), + ); + await this.updateState(async (state) => { - userId = userId ?? state.activeUserId; delete state.accounts[userId]; return state; }); @@ -1178,15 +1105,16 @@ export class StateService< return Object.assign(this.createAccount(), persistentAccountInformation); } - protected async clearDecryptedDataForActiveUser(): Promise { + async clearDecryptedData(userId: UserId): Promise { await this.updateState(async (state) => { - const userId = state?.activeUserId; if (userId != null && state?.accounts[userId]?.data != null) { state.accounts[userId].data = new AccountData(); } return state; }); + + await this.pushAccounts(); } protected createAccount(init: Partial = null): TAccount { @@ -1201,14 +1129,6 @@ export class StateService< // We must have a manual call to clear tokens as we can't leverage state provider to clean // up our data as we have secure storage in the mix. await this.tokenService.clearTokens(userId as UserId); - await this.setLastActive(null, { userId: userId }); - await this.updateState(async (state) => { - state.authenticatedAccounts = state.authenticatedAccounts.filter((id) => id !== userId); - - await this.storageService.save(keys.authenticatedAccounts, state.authenticatedAccounts); - - return state; - }); } protected async removeAccountFromDisk(userId: string) { @@ -1217,32 +1137,6 @@ export class StateService< await this.removeAccountFromSecureStorage(userId); } - async nextUpActiveUser() { - const accounts = (await this.state())?.accounts; - if (accounts == null || Object.keys(accounts).length < 1) { - return null; - } - - let newActiveUser; - for (const userId in accounts) { - if (userId == null) { - continue; - } - if (await this.getIsAuthenticated({ userId: userId })) { - newActiveUser = userId; - break; - } - newActiveUser = null; - } - return newActiveUser as UserId; - } - - protected async dynamicallySetActiveUser() { - const newActiveUser = await this.nextUpActiveUser(); - await this.setActiveUser(newActiveUser); - return newActiveUser; - } - protected async saveSecureStorageKey( key: string, value: T, diff --git a/libs/common/src/platform/services/system.service.ts b/libs/common/src/platform/services/system.service.ts index d19390c45e..80053673d8 100644 --- a/libs/common/src/platform/services/system.service.ts +++ b/libs/common/src/platform/services/system.service.ts @@ -1,10 +1,12 @@ -import { firstValueFrom, timeout } from "rxjs"; +import { firstValueFrom, map, timeout } from "rxjs"; import { VaultTimeoutSettingsService } from "../../abstractions/vault-timeout/vault-timeout-settings.service"; +import { AccountService } from "../../auth/abstractions/account.service"; import { AuthService } from "../../auth/abstractions/auth.service"; import { AuthenticationStatus } from "../../auth/enums/authentication-status"; import { AutofillSettingsServiceAbstraction } from "../../autofill/services/autofill-settings.service"; import { VaultTimeoutAction } from "../../enums/vault-timeout-action.enum"; +import { UserId } from "../../types/guid"; import { MessagingService } from "../abstractions/messaging.service"; import { PlatformUtilsService } from "../abstractions/platform-utils.service"; import { StateService } from "../abstractions/state.service"; @@ -25,15 +27,18 @@ export class SystemService implements SystemServiceAbstraction { private autofillSettingsService: AutofillSettingsServiceAbstraction, private vaultTimeoutSettingsService: VaultTimeoutSettingsService, private biometricStateService: BiometricStateService, + private accountService: AccountService, ) {} async startProcessReload(authService: AuthService): Promise { - const accounts = await firstValueFrom(this.stateService.accounts$); + const accounts = await firstValueFrom(this.accountService.accounts$); if (accounts != null) { const keys = Object.keys(accounts); if (keys.length > 0) { for (const userId of keys) { - if ((await authService.getAuthStatus(userId)) === AuthenticationStatus.Unlocked) { + let status = await firstValueFrom(authService.authStatusFor$(userId as UserId)); + status = await authService.getAuthStatus(userId); + if (status === AuthenticationStatus.Unlocked) { return; } } @@ -63,15 +68,24 @@ export class SystemService implements SystemServiceAbstraction { clearInterval(this.reloadInterval); this.reloadInterval = null; - const currentUser = await firstValueFrom(this.stateService.activeAccount$.pipe(timeout(500))); + const currentUser = await firstValueFrom( + this.accountService.activeAccount$.pipe( + map((a) => a?.id), + timeout(500), + ), + ); // Replace current active user if they will be logged out on reload if (currentUser != null) { const timeoutAction = await firstValueFrom( this.vaultTimeoutSettingsService.vaultTimeoutAction$().pipe(timeout(500)), ); if (timeoutAction === VaultTimeoutAction.LogOut) { - const nextUser = await this.stateService.nextUpActiveUser(); - await this.stateService.setActiveUser(nextUser); + const nextUser = await firstValueFrom( + this.accountService.nextUpAccount$.pipe(map((account) => account?.id ?? null)), + ); + // Can be removed once we migrate password generation history to state providers + await this.stateService.clearDecryptedData(currentUser); + await this.accountService.switchAccount(nextUser); } } diff --git a/libs/common/src/platform/state/implementations/default-active-user-state.provider.spec.ts b/libs/common/src/platform/state/implementations/default-active-user-state.provider.spec.ts index c1cc15a176..681963f823 100644 --- a/libs/common/src/platform/state/implementations/default-active-user-state.provider.spec.ts +++ b/libs/common/src/platform/state/implementations/default-active-user-state.provider.spec.ts @@ -1,7 +1,6 @@ import { mock } from "jest-mock-extended"; import { mockAccountServiceWith, trackEmissions } from "../../../../spec"; -import { AuthenticationStatus } from "../../../auth/enums/authentication-status"; import { UserId } from "../../../types/guid"; import { SingleUserStateProvider } from "../user-state.provider"; @@ -14,7 +13,7 @@ describe("DefaultActiveUserStateProvider", () => { id: userId, name: "name", email: "email", - status: AuthenticationStatus.Locked, + emailVerified: false, }; const accountService = mockAccountServiceWith(userId, accountInfo); let sut: DefaultActiveUserStateProvider; diff --git a/libs/common/src/platform/state/implementations/default-active-user-state.spec.ts b/libs/common/src/platform/state/implementations/default-active-user-state.spec.ts index 51a972a9dc..c652136a0d 100644 --- a/libs/common/src/platform/state/implementations/default-active-user-state.spec.ts +++ b/libs/common/src/platform/state/implementations/default-active-user-state.spec.ts @@ -82,6 +82,7 @@ describe("DefaultActiveUserState", () => { activeAccountSubject.next({ id: userId, email: `test${id}@example.com`, + emailVerified: false, name: `Test User ${id}`, }); await awaitAsync(); diff --git a/libs/common/src/platform/state/implementations/default-state.provider.spec.ts b/libs/common/src/platform/state/implementations/default-state.provider.spec.ts index 3243b53d67..98d423cf48 100644 --- a/libs/common/src/platform/state/implementations/default-state.provider.spec.ts +++ b/libs/common/src/platform/state/implementations/default-state.provider.spec.ts @@ -69,7 +69,12 @@ describe("DefaultStateProvider", () => { userId?: UserId, ) => Observable, ) => { - const accountInfo = { email: "email", name: "name", status: AuthenticationStatus.LoggedOut }; + const accountInfo = { + email: "email", + emailVerified: false, + name: "name", + status: AuthenticationStatus.LoggedOut, + }; const keyDefinition = new KeyDefinition(new StateDefinition("test", "disk"), "test", { deserializer: (s) => s, }); @@ -114,7 +119,12 @@ describe("DefaultStateProvider", () => { ); describe("getUserState$", () => { - const accountInfo = { email: "email", name: "name", status: AuthenticationStatus.LoggedOut }; + const accountInfo = { + email: "email", + emailVerified: false, + name: "name", + status: AuthenticationStatus.LoggedOut, + }; const keyDefinition = new KeyDefinition(new StateDefinition("test", "disk"), "test", { deserializer: (s) => s, }); diff --git a/libs/common/src/platform/state/state-definitions.ts b/libs/common/src/platform/state/state-definitions.ts index ee5005202f..6b309ecfb9 100644 --- a/libs/common/src/platform/state/state-definitions.ts +++ b/libs/common/src/platform/state/state-definitions.ts @@ -38,6 +38,7 @@ export const BILLING_DISK = new StateDefinition("billing", "disk"); export const KDF_CONFIG_DISK = new StateDefinition("kdfConfig", "disk"); export const KEY_CONNECTOR_DISK = new StateDefinition("keyConnector", "disk"); export const ACCOUNT_MEMORY = new StateDefinition("account", "memory"); +export const ACCOUNT_DISK = new StateDefinition("account", "disk"); export const MASTER_PASSWORD_MEMORY = new StateDefinition("masterPassword", "memory"); export const MASTER_PASSWORD_DISK = new StateDefinition("masterPassword", "disk"); export const TWO_FACTOR_MEMORY = new StateDefinition("twoFactor", "memory"); diff --git a/libs/common/src/services/vault-timeout/vault-timeout.service.spec.ts b/libs/common/src/services/vault-timeout/vault-timeout.service.spec.ts index 5344093a25..12c24dcdef 100644 --- a/libs/common/src/services/vault-timeout/vault-timeout.service.spec.ts +++ b/libs/common/src/services/vault-timeout/vault-timeout.service.spec.ts @@ -1,9 +1,10 @@ import { MockProxy, any, mock } from "jest-mock-extended"; -import { BehaviorSubject } from "rxjs"; +import { BehaviorSubject, of } from "rxjs"; import { FakeAccountService, mockAccountServiceWith } from "../../../spec/fake-account-service"; import { SearchService } from "../../abstractions/search.service"; import { VaultTimeoutSettingsService } from "../../abstractions/vault-timeout/vault-timeout-settings.service"; +import { AccountInfo } from "../../auth/abstractions/account.service"; import { AuthService } from "../../auth/abstractions/auth.service"; import { AuthenticationStatus } from "../../auth/enums/authentication-status"; import { FakeMasterPasswordService } from "../../auth/services/master-password/fake-master-password.service"; @@ -13,7 +14,6 @@ import { MessagingService } from "../../platform/abstractions/messaging.service" import { PlatformUtilsService } from "../../platform/abstractions/platform-utils.service"; import { StateService } from "../../platform/abstractions/state.service"; import { Utils } from "../../platform/misc/utils"; -import { Account } from "../../platform/models/domain/account"; import { StateEventRunnerService } from "../../platform/state"; import { UserId } from "../../types/guid"; import { CipherService } from "../../vault/abstractions/cipher.service"; @@ -39,7 +39,6 @@ describe("VaultTimeoutService", () => { let lockedCallback: jest.Mock, [userId: string]>; let loggedOutCallback: jest.Mock, [expired: boolean, userId?: string]>; - let accountsSubject: BehaviorSubject>; let vaultTimeoutActionSubject: BehaviorSubject; let availableVaultTimeoutActionsSubject: BehaviorSubject; @@ -65,10 +64,6 @@ describe("VaultTimeoutService", () => { lockedCallback = jest.fn(); loggedOutCallback = jest.fn(); - accountsSubject = new BehaviorSubject(null); - - stateService.accounts$ = accountsSubject; - vaultTimeoutActionSubject = new BehaviorSubject(VaultTimeoutAction.Lock); vaultTimeoutSettingsService.vaultTimeoutAction$.mockReturnValue(vaultTimeoutActionSubject); @@ -127,21 +122,39 @@ describe("VaultTimeoutService", () => { return Promise.resolve(accounts[userId]?.vaultTimeout); }); - stateService.getLastActive.mockImplementation((options) => { - return Promise.resolve(accounts[options.userId]?.lastActive); - }); - stateService.getUserId.mockResolvedValue(globalSetups?.userId); - stateService.activeAccount$ = new BehaviorSubject(globalSetups?.userId); - + // Set desired user active and known users on accounts service : note the only thing that matters here is that the ID are set if (globalSetups?.userId) { accountService.activeAccountSubject.next({ id: globalSetups.userId as UserId, email: null, + emailVerified: false, name: null, }); } + accountService.accounts$ = of( + Object.entries(accounts).reduce( + (agg, [id]) => { + agg[id] = { + email: "", + emailVerified: true, + name: "", + }; + return agg; + }, + {} as Record, + ), + ); + accountService.accountActivity$ = of( + Object.entries(accounts).reduce( + (agg, [id, info]) => { + agg[id] = info.lastActive ? new Date(info.lastActive) : null; + return agg; + }, + {} as Record, + ), + ); platformUtilsService.isViewOpen.mockResolvedValue(globalSetups?.isViewOpen ?? false); @@ -158,16 +171,6 @@ describe("VaultTimeoutService", () => { ], ); }); - - const accountsSubjectValue: Record = Object.keys(accounts).reduce( - (agg, key) => { - const newPartial: Record = {}; - newPartial[key] = null; // No values actually matter on this other than the key - return Object.assign(agg, newPartial); - }, - {} as Record, - ); - accountsSubject.next(accountsSubjectValue); }; const expectUserToHaveLocked = (userId: string) => { diff --git a/libs/common/src/services/vault-timeout/vault-timeout.service.ts b/libs/common/src/services/vault-timeout/vault-timeout.service.ts index 8baf6c04c4..8e0978d07d 100644 --- a/libs/common/src/services/vault-timeout/vault-timeout.service.ts +++ b/libs/common/src/services/vault-timeout/vault-timeout.service.ts @@ -1,4 +1,4 @@ -import { firstValueFrom, timeout } from "rxjs"; +import { combineLatest, firstValueFrom, switchMap } from "rxjs"; import { SearchService } from "../../abstractions/search.service"; import { VaultTimeoutSettingsService } from "../../abstractions/vault-timeout/vault-timeout-settings.service"; @@ -64,14 +64,25 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction { // Get whether or not the view is open a single time so it can be compared for each user const isViewOpen = await this.platformUtilsService.isViewOpen(); - const activeUserId = await firstValueFrom(this.stateService.activeAccount$.pipe(timeout(500))); - - const accounts = await firstValueFrom(this.stateService.accounts$); - for (const userId in accounts) { - if (userId != null && (await this.shouldLock(userId, activeUserId, isViewOpen))) { - await this.executeTimeoutAction(userId); - } - } + await firstValueFrom( + combineLatest([ + this.accountService.activeAccount$, + this.accountService.accountActivity$, + ]).pipe( + switchMap(async ([activeAccount, accountActivity]) => { + const activeUserId = activeAccount?.id; + for (const userIdString in accountActivity) { + const userId = userIdString as UserId; + if ( + userId != null && + (await this.shouldLock(userId, accountActivity[userId], activeUserId, isViewOpen)) + ) { + await this.executeTimeoutAction(userId); + } + } + }), + ), + ); } async lock(userId?: string): Promise { @@ -123,6 +134,7 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction { private async shouldLock( userId: string, + lastActive: Date, activeUserId: string, isViewOpen: boolean, ): Promise { @@ -146,13 +158,12 @@ export class VaultTimeoutService implements VaultTimeoutServiceAbstraction { return false; } - const lastActive = await this.stateService.getLastActive({ userId: userId }); if (lastActive == null) { return false; } const vaultTimeoutSeconds = vaultTimeout * 60; - const diffSeconds = (new Date().getTime() - lastActive) / 1000; + const diffSeconds = (new Date().getTime() - lastActive.getTime()) / 1000; return diffSeconds >= vaultTimeoutSeconds; } diff --git a/libs/common/src/state-migrations/migrate.ts b/libs/common/src/state-migrations/migrate.ts index 31bc5460b4..0a1f4b1d11 100644 --- a/libs/common/src/state-migrations/migrate.ts +++ b/libs/common/src/state-migrations/migrate.ts @@ -57,13 +57,14 @@ import { CipherServiceMigrator } from "./migrations/57-move-cipher-service-to-st import { RemoveRefreshTokenMigratedFlagMigrator } from "./migrations/58-remove-refresh-token-migrated-state-provider-flag"; import { KdfConfigMigrator } from "./migrations/59-move-kdf-config-to-state-provider"; import { RemoveLegacyEtmKeyMigrator } from "./migrations/6-remove-legacy-etm-key"; +import { KnownAccountsMigrator } from "./migrations/60-known-accounts"; import { MoveBiometricAutoPromptToAccount } from "./migrations/7-move-biometric-auto-prompt-to-account"; import { MoveStateVersionMigrator } from "./migrations/8-move-state-version"; import { MoveBrowserSettingsToGlobal } from "./migrations/9-move-browser-settings-to-global"; import { MinVersionMigrator } from "./migrations/min-version"; export const MIN_VERSION = 3; -export const CURRENT_VERSION = 59; +export const CURRENT_VERSION = 60; export type MinVersion = typeof MIN_VERSION; export function createMigrationBuilder() { @@ -124,7 +125,8 @@ export function createMigrationBuilder() { .with(AuthRequestMigrator, 55, 56) .with(CipherServiceMigrator, 56, 57) .with(RemoveRefreshTokenMigratedFlagMigrator, 57, 58) - .with(KdfConfigMigrator, 58, CURRENT_VERSION); + .with(KdfConfigMigrator, 58, 59) + .with(KnownAccountsMigrator, 59, CURRENT_VERSION); } export async function currentVersion( diff --git a/libs/common/src/state-migrations/migration-helper.spec.ts b/libs/common/src/state-migrations/migration-helper.spec.ts index 5f366f2597..162fac2fab 100644 --- a/libs/common/src/state-migrations/migration-helper.spec.ts +++ b/libs/common/src/state-migrations/migration-helper.spec.ts @@ -27,6 +27,14 @@ const exampleJSON = { }, global_serviceName_key: "global_serviceName_key", user_userId_serviceName_key: "user_userId_serviceName_key", + global_account_accounts: { + "c493ed01-4e08-4e88-abc7-332f380ca760": { + otherStuff: "otherStuff3", + }, + "23e61a5f-2ece-4f5e-b499-f0bc489482a9": { + otherStuff: "otherStuff4", + }, + }, }; describe("RemoveLegacyEtmKeyMigrator", () => { @@ -81,6 +89,41 @@ describe("RemoveLegacyEtmKeyMigrator", () => { const accounts = await sut.getAccounts(); expect(accounts).toEqual([]); }); + + it("handles global scoped known accounts for version 60 and after", async () => { + sut.currentVersion = 60; + const accounts = await sut.getAccounts(); + expect(accounts).toEqual([ + // Note, still gets values stored in state service objects, just grabs user ids from global + { + userId: "c493ed01-4e08-4e88-abc7-332f380ca760", + account: { otherStuff: "otherStuff1" }, + }, + { + userId: "23e61a5f-2ece-4f5e-b499-f0bc489482a9", + account: { otherStuff: "otherStuff2" }, + }, + ]); + }); + }); + + describe("getKnownUserIds", () => { + it("returns all user ids", async () => { + const userIds = await sut.getKnownUserIds(); + expect(userIds).toEqual([ + "c493ed01-4e08-4e88-abc7-332f380ca760", + "23e61a5f-2ece-4f5e-b499-f0bc489482a9", + ]); + }); + + it("returns all user ids when version is 60 or greater", async () => { + sut.currentVersion = 60; + const userIds = await sut.getKnownUserIds(); + expect(userIds).toEqual([ + "c493ed01-4e08-4e88-abc7-332f380ca760", + "23e61a5f-2ece-4f5e-b499-f0bc489482a9", + ]); + }); }); describe("getFromGlobal", () => { diff --git a/libs/common/src/state-migrations/migration-helper.ts b/libs/common/src/state-migrations/migration-helper.ts index 2505e2b264..5d1de8dd49 100644 --- a/libs/common/src/state-migrations/migration-helper.ts +++ b/libs/common/src/state-migrations/migration-helper.ts @@ -162,7 +162,7 @@ export class MigrationHelper { async getAccounts(): Promise< { userId: string; account: ExpectedAccountType }[] > { - const userIds = (await this.get("authenticatedAccounts")) ?? []; + const userIds = await this.getKnownUserIds(); return Promise.all( userIds.map(async (userId) => ({ userId, @@ -171,6 +171,17 @@ export class MigrationHelper { ); } + /** + * Helper method to read known users ids. + */ + async getKnownUserIds(): Promise { + if (this.currentVersion < 61) { + return knownAccountUserIdsBuilderPre61(this.storageService); + } else { + return knownAccountUserIdsBuilder(this.storageService); + } + } + /** * Builds a user storage key appropriate for the current version. * @@ -233,3 +244,18 @@ function globalKeyBuilder(keyDefinition: KeyDefinitionLike): string { function globalKeyBuilderPre9(): string { throw Error("No key builder should be used for versions prior to 9."); } + +async function knownAccountUserIdsBuilderPre61( + storageService: AbstractStorageService, +): Promise { + return (await storageService.get("authenticatedAccounts")) ?? []; +} + +async function knownAccountUserIdsBuilder( + storageService: AbstractStorageService, +): Promise { + const accounts = await storageService.get>( + globalKeyBuilder({ stateDefinition: { name: "account" }, key: "accounts" }), + ); + return Object.keys(accounts ?? {}); +} diff --git a/libs/common/src/state-migrations/migrations/60-known-accounts.spec.ts b/libs/common/src/state-migrations/migrations/60-known-accounts.spec.ts new file mode 100644 index 0000000000..28dedb3c39 --- /dev/null +++ b/libs/common/src/state-migrations/migrations/60-known-accounts.spec.ts @@ -0,0 +1,145 @@ +import { MockProxy } from "jest-mock-extended"; + +import { MigrationHelper } from "../migration-helper"; +import { mockMigrationHelper } from "../migration-helper.spec"; + +import { + ACCOUNT_ACCOUNTS, + ACCOUNT_ACTIVE_ACCOUNT_ID, + ACCOUNT_ACTIVITY, + KnownAccountsMigrator, +} from "./60-known-accounts"; + +const migrateJson = () => { + return { + authenticatedAccounts: ["user1", "user2"], + activeUserId: "user1", + user1: { + profile: { + email: "user1", + name: "User 1", + emailVerified: true, + }, + }, + user2: { + profile: { + email: "", + emailVerified: false, + }, + }, + accountActivity: { + user1: 1609459200000, // 2021-01-01 + user2: 1609545600000, // 2021-01-02 + }, + }; +}; + +const rollbackJson = () => { + return { + user1: { + profile: { + email: "user1", + name: "User 1", + emailVerified: true, + }, + }, + user2: { + profile: { + email: "", + emailVerified: false, + }, + }, + global_account_accounts: { + user1: { + profile: { + email: "user1", + name: "User 1", + emailVerified: true, + }, + }, + user2: { + profile: { + email: "", + emailVerified: false, + }, + }, + }, + global_account_activeAccountId: "user1", + global_account_activity: { + user1: "2021-01-01T00:00:00.000Z", + user2: "2021-01-02T00:00:00.000Z", + }, + }; +}; + +describe("ReplicateKnownAccounts", () => { + let helper: MockProxy; + let sut: KnownAccountsMigrator; + + describe("migrate", () => { + beforeEach(() => { + helper = mockMigrationHelper(migrateJson(), 59); + sut = new KnownAccountsMigrator(59, 60); + }); + + it("migrates accounts", async () => { + await sut.migrate(helper); + expect(helper.setToGlobal).toHaveBeenCalledWith(ACCOUNT_ACCOUNTS, { + user1: { + email: "user1", + name: "User 1", + emailVerified: true, + }, + user2: { + email: "", + emailVerified: false, + name: undefined, + }, + }); + expect(helper.remove).toHaveBeenCalledWith("authenticatedAccounts"); + }); + + it("migrates active account it", async () => { + await sut.migrate(helper); + expect(helper.setToGlobal).toHaveBeenCalledWith(ACCOUNT_ACTIVE_ACCOUNT_ID, "user1"); + expect(helper.remove).toHaveBeenCalledWith("activeUserId"); + }); + + it("migrates account activity", async () => { + await sut.migrate(helper); + expect(helper.setToGlobal).toHaveBeenCalledWith(ACCOUNT_ACTIVITY, { + user1: '"2021-01-01T00:00:00.000Z"', + user2: '"2021-01-02T00:00:00.000Z"', + }); + expect(helper.remove).toHaveBeenCalledWith("accountActivity"); + }); + }); + + describe("rollback", () => { + beforeEach(() => { + helper = mockMigrationHelper(rollbackJson(), 60); + sut = new KnownAccountsMigrator(59, 60); + }); + + it("rolls back authenticated accounts", async () => { + await sut.rollback(helper); + expect(helper.set).toHaveBeenCalledWith("authenticatedAccounts", ["user1", "user2"]); + expect(helper.removeFromGlobal).toHaveBeenCalledWith(ACCOUNT_ACCOUNTS); + }); + + it("rolls back active account id", async () => { + await sut.rollback(helper); + expect(helper.set).toHaveBeenCalledWith("activeUserId", "user1"); + expect(helper.removeFromGlobal).toHaveBeenCalledWith(ACCOUNT_ACTIVE_ACCOUNT_ID); + }); + + it("rolls back account activity", async () => { + await sut.rollback(helper); + expect(helper.set).toHaveBeenCalledWith("accountActivity", { + user1: 1609459200000, + user2: 1609545600000, + }); + expect(helper.removeFromGlobal).toHaveBeenCalledWith(ACCOUNT_ACTIVITY); + }); + }); +}); diff --git a/libs/common/src/state-migrations/migrations/60-known-accounts.ts b/libs/common/src/state-migrations/migrations/60-known-accounts.ts new file mode 100644 index 0000000000..75117da5b4 --- /dev/null +++ b/libs/common/src/state-migrations/migrations/60-known-accounts.ts @@ -0,0 +1,111 @@ +import { KeyDefinitionLike, MigrationHelper } from "../migration-helper"; +import { Migrator } from "../migrator"; + +export const ACCOUNT_ACCOUNTS: KeyDefinitionLike = { + stateDefinition: { + name: "account", + }, + key: "accounts", +}; + +export const ACCOUNT_ACTIVE_ACCOUNT_ID: KeyDefinitionLike = { + stateDefinition: { + name: "account", + }, + key: "activeAccountId", +}; + +export const ACCOUNT_ACTIVITY: KeyDefinitionLike = { + stateDefinition: { + name: "account", + }, + key: "activity", +}; + +type ExpectedAccountType = { + profile?: { + email?: string; + name?: string; + emailVerified?: boolean; + }; +}; + +export class KnownAccountsMigrator extends Migrator<59, 60> { + async migrate(helper: MigrationHelper): Promise { + await this.migrateAuthenticatedAccounts(helper); + await this.migrateActiveAccountId(helper); + await this.migrateAccountActivity(helper); + } + async rollback(helper: MigrationHelper): Promise { + // authenticated account are removed, but the accounts record also contains logged out accounts. Best we can do is to add them all back + const accounts = (await helper.getFromGlobal>(ACCOUNT_ACCOUNTS)) ?? {}; + await helper.set("authenticatedAccounts", Object.keys(accounts)); + await helper.removeFromGlobal(ACCOUNT_ACCOUNTS); + + // Active Account Id + const activeAccountId = await helper.getFromGlobal(ACCOUNT_ACTIVE_ACCOUNT_ID); + if (activeAccountId) { + await helper.set("activeUserId", activeAccountId); + } + await helper.removeFromGlobal(ACCOUNT_ACTIVE_ACCOUNT_ID); + + // Account Activity + const accountActivity = await helper.getFromGlobal>(ACCOUNT_ACTIVITY); + if (accountActivity) { + const toStore = Object.entries(accountActivity).reduce( + (agg, [userId, dateString]) => { + agg[userId] = new Date(dateString).getTime(); + return agg; + }, + {} as Record, + ); + await helper.set("accountActivity", toStore); + } + await helper.removeFromGlobal(ACCOUNT_ACTIVITY); + } + + private async migrateAuthenticatedAccounts(helper: MigrationHelper) { + const authenticatedAccounts = (await helper.get("authenticatedAccounts")) ?? []; + const accounts = await Promise.all( + authenticatedAccounts.map(async (userId) => { + const account = await helper.get(userId); + return { userId, account }; + }), + ); + const accountsToStore = accounts.reduce( + (agg, { userId, account }) => { + if (account?.profile) { + agg[userId] = { + email: account.profile.email ?? "", + emailVerified: account.profile.emailVerified ?? false, + name: account.profile.name, + }; + } + return agg; + }, + {} as Record, + ); + + await helper.setToGlobal(ACCOUNT_ACCOUNTS, accountsToStore); + await helper.remove("authenticatedAccounts"); + } + + private async migrateAccountActivity(helper: MigrationHelper) { + const stored = await helper.get>("accountActivity"); + const accountActivity = Object.entries(stored ?? {}).reduce( + (agg, [userId, dateMs]) => { + agg[userId] = JSON.stringify(new Date(dateMs)); + return agg; + }, + {} as Record, + ); + await helper.setToGlobal(ACCOUNT_ACTIVITY, accountActivity); + await helper.remove("accountActivity"); + } + + private async migrateActiveAccountId(helper: MigrationHelper) { + const activeAccountId = await helper.get("activeUserId"); + await helper.setToGlobal(ACCOUNT_ACTIVE_ACCOUNT_ID, activeAccountId); + await helper.remove("activeUserId"); + } +} diff --git a/libs/common/src/tools/send/services/send.service.spec.ts b/libs/common/src/tools/send/services/send.service.spec.ts index 41183c42af..2f0f50c616 100644 --- a/libs/common/src/tools/send/services/send.service.spec.ts +++ b/libs/common/src/tools/send/services/send.service.spec.ts @@ -62,6 +62,7 @@ describe("SendService", () => { accountService.activeAccountSubject.next({ id: mockUserId, email: "email", + emailVerified: false, name: "name", }); diff --git a/libs/common/src/vault/services/sync/sync.service.ts b/libs/common/src/vault/services/sync/sync.service.ts index 73869ff488..995ab7319b 100644 --- a/libs/common/src/vault/services/sync/sync.service.ts +++ b/libs/common/src/vault/services/sync/sync.service.ts @@ -326,7 +326,10 @@ export class SyncService implements SyncServiceAbstraction { await this.cryptoService.setOrgKeys(response.organizations, response.providerOrganizations); await this.avatarService.setSyncAvatarColor(response.id as UserId, response.avatarColor); await this.tokenService.setSecurityStamp(response.securityStamp, response.id as UserId); - await this.stateService.setEmailVerified(response.emailVerified); + await this.accountService.setAccountEmailVerified( + response.id as UserId, + response.emailVerified, + ); await this.billingAccountProfileStateService.setHasPremium( response.premiumPersonally, From e7416384dcb1b1b242a4672366bf5a6e81d1ee09 Mon Sep 17 00:00:00 2001 From: Will Martin Date: Tue, 30 Apr 2024 10:27:47 -0400 Subject: [PATCH 35/41] [CL-220] item components (#8870) --- .../popup/layout/popup-layout.stories.ts | 41 ++- .../src/a11y/a11y-cell.directive.ts | 33 ++ .../src/a11y/a11y-grid.directive.ts | 145 ++++++++ .../components/src/a11y/a11y-row.directive.ts | 31 ++ libs/components/src/badge/badge.directive.ts | 9 +- .../src/icon-button/icon-button.component.ts | 16 +- libs/components/src/index.ts | 1 + .../src/input/autofocus.directive.ts | 9 +- libs/components/src/item/index.ts | 1 + .../src/item/item-action.component.ts | 12 + .../src/item/item-content.component.html | 16 + .../src/item/item-content.component.ts | 15 + .../src/item/item-group.component.ts | 13 + libs/components/src/item/item.component.html | 21 ++ libs/components/src/item/item.component.ts | 29 ++ libs/components/src/item/item.mdx | 141 ++++++++ libs/components/src/item/item.module.ts | 12 + libs/components/src/item/item.stories.ts | 326 ++++++++++++++++++ .../components/src/search/search.component.ts | 6 +- .../src/shared/focusable-element.ts | 8 + libs/components/src/styles.scss | 2 +- 21 files changed, 858 insertions(+), 29 deletions(-) create mode 100644 libs/components/src/a11y/a11y-cell.directive.ts create mode 100644 libs/components/src/a11y/a11y-grid.directive.ts create mode 100644 libs/components/src/a11y/a11y-row.directive.ts create mode 100644 libs/components/src/item/index.ts create mode 100644 libs/components/src/item/item-action.component.ts create mode 100644 libs/components/src/item/item-content.component.html create mode 100644 libs/components/src/item/item-content.component.ts create mode 100644 libs/components/src/item/item-group.component.ts create mode 100644 libs/components/src/item/item.component.html create mode 100644 libs/components/src/item/item.component.ts create mode 100644 libs/components/src/item/item.mdx create mode 100644 libs/components/src/item/item.module.ts create mode 100644 libs/components/src/item/item.stories.ts create mode 100644 libs/components/src/shared/focusable-element.ts diff --git a/apps/browser/src/platform/popup/layout/popup-layout.stories.ts b/apps/browser/src/platform/popup/layout/popup-layout.stories.ts index 1b10e50c0c..77530d06e5 100644 --- a/apps/browser/src/platform/popup/layout/popup-layout.stories.ts +++ b/apps/browser/src/platform/popup/layout/popup-layout.stories.ts @@ -6,9 +6,11 @@ import { Meta, StoryObj, applicationConfig, moduleMetadata } from "@storybook/an import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { AvatarModule, + BadgeModule, ButtonModule, I18nMockService, IconButtonModule, + ItemModule, } from "@bitwarden/components"; import { PopupFooterComponent } from "./popup-footer.component"; @@ -30,23 +32,34 @@ class ExtensionContainerComponent {} @Component({ selector: "vault-placeholder", template: ` -
vault item
-
vault item
-
vault item
-
vault item
-
vault item
-
vault item
-
vault item
-
vault item
-
vault item
-
vault item
-
vault item
-
vault item
-
vault item last item
+ + + + + + + + + + + + + + + + + `, standalone: true, + imports: [CommonModule, ItemModule, BadgeModule, IconButtonModule], }) -class VaultComponent {} +class VaultComponent { + protected data = Array.from(Array(20).keys()); +} @Component({ selector: "generator-placeholder", diff --git a/libs/components/src/a11y/a11y-cell.directive.ts b/libs/components/src/a11y/a11y-cell.directive.ts new file mode 100644 index 0000000000..fdd75c076f --- /dev/null +++ b/libs/components/src/a11y/a11y-cell.directive.ts @@ -0,0 +1,33 @@ +import { ContentChild, Directive, ElementRef, HostBinding } from "@angular/core"; + +import { FocusableElement } from "../shared/focusable-element"; + +@Directive({ + selector: "bitA11yCell", + standalone: true, + providers: [{ provide: FocusableElement, useExisting: A11yCellDirective }], +}) +export class A11yCellDirective implements FocusableElement { + @HostBinding("attr.role") + role: "gridcell" | null; + + @ContentChild(FocusableElement) + private focusableChild: FocusableElement; + + getFocusTarget() { + let focusTarget: HTMLElement; + if (this.focusableChild) { + focusTarget = this.focusableChild.getFocusTarget(); + } else { + focusTarget = this.elementRef.nativeElement.querySelector("button, a"); + } + + if (!focusTarget) { + return this.elementRef.nativeElement; + } + + return focusTarget; + } + + constructor(private elementRef: ElementRef) {} +} diff --git a/libs/components/src/a11y/a11y-grid.directive.ts b/libs/components/src/a11y/a11y-grid.directive.ts new file mode 100644 index 0000000000..c632376f4f --- /dev/null +++ b/libs/components/src/a11y/a11y-grid.directive.ts @@ -0,0 +1,145 @@ +import { + AfterViewInit, + ContentChildren, + Directive, + HostBinding, + HostListener, + Input, + QueryList, +} from "@angular/core"; + +import type { A11yCellDirective } from "./a11y-cell.directive"; +import { A11yRowDirective } from "./a11y-row.directive"; + +@Directive({ + selector: "bitA11yGrid", + standalone: true, +}) +export class A11yGridDirective implements AfterViewInit { + @HostBinding("attr.role") + role = "grid"; + + @ContentChildren(A11yRowDirective) + rows: QueryList; + + /** The number of pages to navigate on `PageUp` and `PageDown` */ + @Input() pageSize = 5; + + private grid: A11yCellDirective[][]; + + /** The row that currently has focus */ + private activeRow = 0; + + /** The cell that currently has focus */ + private activeCol = 0; + + @HostListener("keydown", ["$event"]) + onKeyDown(event: KeyboardEvent) { + switch (event.code) { + case "ArrowUp": + this.updateCellFocusByDelta(-1, 0); + break; + case "ArrowRight": + this.updateCellFocusByDelta(0, 1); + break; + case "ArrowDown": + this.updateCellFocusByDelta(1, 0); + break; + case "ArrowLeft": + this.updateCellFocusByDelta(0, -1); + break; + case "Home": + this.updateCellFocusByDelta(-this.activeRow, -this.activeCol); + break; + case "End": + this.updateCellFocusByDelta(this.grid.length, this.grid[this.grid.length - 1].length); + break; + case "PageUp": + this.updateCellFocusByDelta(-this.pageSize, 0); + break; + case "PageDown": + this.updateCellFocusByDelta(this.pageSize, 0); + break; + default: + return; + } + + /** Prevent default scrolling behavior */ + event.preventDefault(); + } + + ngAfterViewInit(): void { + this.initializeGrid(); + } + + private initializeGrid(): void { + try { + this.grid = this.rows.map((listItem) => { + listItem.role = "row"; + return [...listItem.cells]; + }); + this.grid.flat().forEach((cell) => { + cell.role = "gridcell"; + cell.getFocusTarget().tabIndex = -1; + }); + + this.getActiveCellContent().tabIndex = 0; + } catch (error) { + // eslint-disable-next-line no-console + console.error("Unable to initialize grid"); + } + } + + /** Get the focusable content of the active cell */ + private getActiveCellContent(): HTMLElement { + return this.grid[this.activeRow][this.activeCol].getFocusTarget(); + } + + /** Move focus via a delta against the currently active gridcell */ + private updateCellFocusByDelta(rowDelta: number, colDelta: number) { + const prevActive = this.getActiveCellContent(); + + this.activeCol += colDelta; + this.activeRow += rowDelta; + + // Row upper bound + if (this.activeRow >= this.grid.length) { + this.activeRow = this.grid.length - 1; + } + + // Row lower bound + if (this.activeRow < 0) { + this.activeRow = 0; + } + + // Column upper bound + if (this.activeCol >= this.grid[this.activeRow].length) { + if (this.activeRow < this.grid.length - 1) { + // Wrap to next row on right arrow + this.activeCol = 0; + this.activeRow += 1; + } else { + this.activeCol = this.grid[this.activeRow].length - 1; + } + } + + // Column lower bound + if (this.activeCol < 0) { + if (this.activeRow > 0) { + // Wrap to prev row on left arrow + this.activeRow -= 1; + this.activeCol = this.grid[this.activeRow].length - 1; + } else { + this.activeCol = 0; + } + } + + const nextActive = this.getActiveCellContent(); + nextActive.tabIndex = 0; + nextActive.focus(); + + if (nextActive !== prevActive) { + prevActive.tabIndex = -1; + } + } +} diff --git a/libs/components/src/a11y/a11y-row.directive.ts b/libs/components/src/a11y/a11y-row.directive.ts new file mode 100644 index 0000000000..e062eb2b5a --- /dev/null +++ b/libs/components/src/a11y/a11y-row.directive.ts @@ -0,0 +1,31 @@ +import { + AfterViewInit, + ContentChildren, + Directive, + HostBinding, + QueryList, + ViewChildren, +} from "@angular/core"; + +import { A11yCellDirective } from "./a11y-cell.directive"; + +@Directive({ + selector: "bitA11yRow", + standalone: true, +}) +export class A11yRowDirective implements AfterViewInit { + @HostBinding("attr.role") + role: "row" | null; + + cells: A11yCellDirective[]; + + @ViewChildren(A11yCellDirective) + private viewCells: QueryList; + + @ContentChildren(A11yCellDirective) + private contentCells: QueryList; + + ngAfterViewInit(): void { + this.cells = [...this.viewCells, ...this.contentCells]; + } +} diff --git a/libs/components/src/badge/badge.directive.ts b/libs/components/src/badge/badge.directive.ts index b81b9f80e2..acce4a18aa 100644 --- a/libs/components/src/badge/badge.directive.ts +++ b/libs/components/src/badge/badge.directive.ts @@ -1,5 +1,7 @@ import { Directive, ElementRef, HostBinding, Input } from "@angular/core"; +import { FocusableElement } from "../shared/focusable-element"; + export type BadgeVariant = "primary" | "secondary" | "success" | "danger" | "warning" | "info"; const styles: Record = { @@ -22,8 +24,9 @@ const hoverStyles: Record = { @Directive({ selector: "span[bitBadge], a[bitBadge], button[bitBadge]", + providers: [{ provide: FocusableElement, useExisting: BadgeDirective }], }) -export class BadgeDirective { +export class BadgeDirective implements FocusableElement { @HostBinding("class") get classList() { return [ "tw-inline-block", @@ -62,6 +65,10 @@ export class BadgeDirective { */ @Input() truncate = true; + getFocusTarget() { + return this.el.nativeElement; + } + private hasHoverEffects = false; constructor(private el: ElementRef) { diff --git a/libs/components/src/icon-button/icon-button.component.ts b/libs/components/src/icon-button/icon-button.component.ts index 53e8032795..54f6dfda96 100644 --- a/libs/components/src/icon-button/icon-button.component.ts +++ b/libs/components/src/icon-button/icon-button.component.ts @@ -1,6 +1,7 @@ -import { Component, HostBinding, Input } from "@angular/core"; +import { Component, ElementRef, HostBinding, Input } from "@angular/core"; import { ButtonLikeAbstraction, ButtonType } from "../shared/button-like.abstraction"; +import { FocusableElement } from "../shared/focusable-element"; export type IconButtonType = ButtonType | "contrast" | "main" | "muted" | "light"; @@ -123,9 +124,12 @@ const sizes: Record = { @Component({ selector: "button[bitIconButton]:not(button[bitButton])", templateUrl: "icon-button.component.html", - providers: [{ provide: ButtonLikeAbstraction, useExisting: BitIconButtonComponent }], + providers: [ + { provide: ButtonLikeAbstraction, useExisting: BitIconButtonComponent }, + { provide: FocusableElement, useExisting: BitIconButtonComponent }, + ], }) -export class BitIconButtonComponent implements ButtonLikeAbstraction { +export class BitIconButtonComponent implements ButtonLikeAbstraction, FocusableElement { @Input("bitIconButton") icon: string; @Input() buttonType: IconButtonType; @@ -162,4 +166,10 @@ export class BitIconButtonComponent implements ButtonLikeAbstraction { setButtonType(value: "primary" | "secondary" | "danger" | "unstyled") { this.buttonType = value; } + + getFocusTarget() { + return this.elementRef.nativeElement; + } + + constructor(private elementRef: ElementRef) {} } diff --git a/libs/components/src/index.ts b/libs/components/src/index.ts index 36185911a6..1e4a3a86ff 100644 --- a/libs/components/src/index.ts +++ b/libs/components/src/index.ts @@ -16,6 +16,7 @@ export * from "./form-field"; export * from "./icon-button"; export * from "./icon"; export * from "./input"; +export * from "./item"; export * from "./layout"; export * from "./link"; export * from "./menu"; diff --git a/libs/components/src/input/autofocus.directive.ts b/libs/components/src/input/autofocus.directive.ts index f8161ee6e0..625e7fbc92 100644 --- a/libs/components/src/input/autofocus.directive.ts +++ b/libs/components/src/input/autofocus.directive.ts @@ -3,12 +3,7 @@ import { take } from "rxjs/operators"; import { Utils } from "@bitwarden/common/platform/misc/utils"; -/** - * Interface for implementing focusable components. Used by the AutofocusDirective. - */ -export abstract class FocusableElement { - focus: () => void; -} +import { FocusableElement } from "../shared/focusable-element"; /** * Directive to focus an element. @@ -46,7 +41,7 @@ export class AutofocusDirective { private focus() { if (this.focusableElement) { - this.focusableElement.focus(); + this.focusableElement.getFocusTarget().focus(); } else { this.el.nativeElement.focus(); } diff --git a/libs/components/src/item/index.ts b/libs/components/src/item/index.ts new file mode 100644 index 0000000000..56896cdc3c --- /dev/null +++ b/libs/components/src/item/index.ts @@ -0,0 +1 @@ +export * from "./item.module"; diff --git a/libs/components/src/item/item-action.component.ts b/libs/components/src/item/item-action.component.ts new file mode 100644 index 0000000000..8cabf5c5c2 --- /dev/null +++ b/libs/components/src/item/item-action.component.ts @@ -0,0 +1,12 @@ +import { Component } from "@angular/core"; + +import { A11yCellDirective } from "../a11y/a11y-cell.directive"; + +@Component({ + selector: "bit-item-action", + standalone: true, + imports: [], + template: ``, + providers: [{ provide: A11yCellDirective, useExisting: ItemActionComponent }], +}) +export class ItemActionComponent extends A11yCellDirective {} diff --git a/libs/components/src/item/item-content.component.html b/libs/components/src/item/item-content.component.html new file mode 100644 index 0000000000..d034a4a001 --- /dev/null +++ b/libs/components/src/item/item-content.component.html @@ -0,0 +1,16 @@ +
+ + +
+
+ +
+
+ +
+
+
+ +
+ +
diff --git a/libs/components/src/item/item-content.component.ts b/libs/components/src/item/item-content.component.ts new file mode 100644 index 0000000000..58a1198512 --- /dev/null +++ b/libs/components/src/item/item-content.component.ts @@ -0,0 +1,15 @@ +import { CommonModule } from "@angular/common"; +import { ChangeDetectionStrategy, Component } from "@angular/core"; + +@Component({ + selector: "bit-item-content, [bit-item-content]", + standalone: true, + imports: [CommonModule], + templateUrl: `item-content.component.html`, + host: { + class: + "fvw-target tw-outline-none tw-text-main hover:tw-text-main hover:tw-no-underline tw-text-base tw-py-2 tw-px-4 tw-bg-transparent tw-w-full tw-border-none tw-flex tw-gap-4 tw-items-center tw-justify-between", + }, + changeDetection: ChangeDetectionStrategy.OnPush, +}) +export class ItemContentComponent {} diff --git a/libs/components/src/item/item-group.component.ts b/libs/components/src/item/item-group.component.ts new file mode 100644 index 0000000000..2a9a8275cc --- /dev/null +++ b/libs/components/src/item/item-group.component.ts @@ -0,0 +1,13 @@ +import { ChangeDetectionStrategy, Component } from "@angular/core"; + +@Component({ + selector: "bit-item-group", + standalone: true, + imports: [], + template: ``, + changeDetection: ChangeDetectionStrategy.OnPush, + host: { + class: "tw-block", + }, +}) +export class ItemGroupComponent {} diff --git a/libs/components/src/item/item.component.html b/libs/components/src/item/item.component.html new file mode 100644 index 0000000000..0c91c6848e --- /dev/null +++ b/libs/components/src/item/item.component.html @@ -0,0 +1,21 @@ + +
+ + + + +
+ +
+
diff --git a/libs/components/src/item/item.component.ts b/libs/components/src/item/item.component.ts new file mode 100644 index 0000000000..4b7b57fa9f --- /dev/null +++ b/libs/components/src/item/item.component.ts @@ -0,0 +1,29 @@ +import { CommonModule } from "@angular/common"; +import { ChangeDetectionStrategy, Component, HostListener, signal } from "@angular/core"; + +import { A11yRowDirective } from "../a11y/a11y-row.directive"; + +import { ItemActionComponent } from "./item-action.component"; + +@Component({ + selector: "bit-item", + standalone: true, + imports: [CommonModule, ItemActionComponent], + changeDetection: ChangeDetectionStrategy.OnPush, + templateUrl: "item.component.html", + providers: [{ provide: A11yRowDirective, useExisting: ItemComponent }], +}) +export class ItemComponent extends A11yRowDirective { + /** + * We have `:focus-within` and `:focus-visible` but no `:focus-visible-within` + */ + protected focusVisibleWithin = signal(false); + @HostListener("focusin", ["$event.target"]) + onFocusIn(target: HTMLElement) { + this.focusVisibleWithin.set(target.matches(".fvw-target:focus-visible")); + } + @HostListener("focusout") + onFocusOut() { + this.focusVisibleWithin.set(false); + } +} diff --git a/libs/components/src/item/item.mdx b/libs/components/src/item/item.mdx new file mode 100644 index 0000000000..8506de72bb --- /dev/null +++ b/libs/components/src/item/item.mdx @@ -0,0 +1,141 @@ +import { Meta, Story, Primary, Controls, Canvas } from "@storybook/addon-docs"; + +import * as stories from "./item.stories"; + + + +```ts +import { ItemModule } from "@bitwarden/components"; +``` + +# Item + +`` is a horizontal card that contains one or more interactive actions. + +It is a generic container that can be used for either standalone content, an alternative to tables, +or to list nav links. + + + + + +## Primary Content + +The primary content of an item is supplied by `bit-item-content`. + +### Content Types + +The content can be a button, anchor, or static container. + +```html + + Hi, I am a link. + + + + + + + + I'm just static :( + +``` + + + + + +### Content Slots + +`bit-item-content` contains the following slots to help position the content: + +| Slot | Description | +| ------------------ | --------------------------------------------------- | +| default | primary text or arbitrary content; fan favorite | +| `slot="secondary"` | supporting text; under the default slot | +| `slot="start"` | commonly an icon or avatar; before the default slot | +| `slot="end"` | commonly an icon; after the default slot | + +- Note: There is also an `end` slot within `bit-item` itself. Place + [interactive secondary actions](#secondary-actions) there, and place non-interactive content (such + as icons) in `bit-item-content` + +```html + + + +``` + + + + + +## Secondary Actions + +Secondary interactive actions can be placed in the item through the `"end"` slot, outside of +`bit-item-content`. + +Each action must be wrapped by ``. + +Actions are commonly icon buttons or badge buttons. + +```html + + + + + + + + + + + + + + + +``` + +## Item Groups + +Groups of items can be associated by wrapping them in the ``. + + + + + + + + + +### A11y + +Keyboard nav is currently disabled due to a bug when used within a virtual scroll viewport. + +Item groups utilize arrow-based keyboard navigation +([further reading here](https://www.w3.org/WAI/ARIA/apg/patterns/grid/examples/layout-grids/#kbd_label)). + +Use `aria-label` or `aria-labelledby` to give groups an accessible name. + +```html + + ... + ... + ... + +``` + +### Virtual Scrolling + + + + diff --git a/libs/components/src/item/item.module.ts b/libs/components/src/item/item.module.ts new file mode 100644 index 0000000000..226fed11d8 --- /dev/null +++ b/libs/components/src/item/item.module.ts @@ -0,0 +1,12 @@ +import { NgModule } from "@angular/core"; + +import { ItemActionComponent } from "./item-action.component"; +import { ItemContentComponent } from "./item-content.component"; +import { ItemGroupComponent } from "./item-group.component"; +import { ItemComponent } from "./item.component"; + +@NgModule({ + imports: [ItemComponent, ItemContentComponent, ItemActionComponent, ItemGroupComponent], + exports: [ItemComponent, ItemContentComponent, ItemActionComponent, ItemGroupComponent], +}) +export class ItemModule {} diff --git a/libs/components/src/item/item.stories.ts b/libs/components/src/item/item.stories.ts new file mode 100644 index 0000000000..b9d8d6cc2e --- /dev/null +++ b/libs/components/src/item/item.stories.ts @@ -0,0 +1,326 @@ +import { ScrollingModule } from "@angular/cdk/scrolling"; +import { CommonModule } from "@angular/common"; +import { Meta, StoryObj, componentWrapperDecorator, moduleMetadata } from "@storybook/angular"; + +import { A11yGridDirective } from "../a11y/a11y-grid.directive"; +import { AvatarModule } from "../avatar"; +import { BadgeModule } from "../badge"; +import { IconButtonModule } from "../icon-button"; +import { TypographyModule } from "../typography"; + +import { ItemActionComponent } from "./item-action.component"; +import { ItemContentComponent } from "./item-content.component"; +import { ItemGroupComponent } from "./item-group.component"; +import { ItemComponent } from "./item.component"; + +export default { + title: "Component Library/Item", + component: ItemComponent, + decorators: [ + moduleMetadata({ + imports: [ + CommonModule, + ItemGroupComponent, + AvatarModule, + IconButtonModule, + BadgeModule, + TypographyModule, + ItemActionComponent, + ItemContentComponent, + A11yGridDirective, + ScrollingModule, + ], + }), + componentWrapperDecorator((story) => `
${story}
`), + ], +} as Meta; + +type Story = StoryObj; + +export const Default: Story = { + render: (args) => ({ + props: args, + template: /*html*/ ` + + + + + + + + + + + + + + + + `, + }), +}; + +export const ContentSlots: Story = { + render: (args) => ({ + props: args, + template: /*html*/ ` + + + + `, + }), +}; + +export const ContentTypes: Story = { + render: (args) => ({ + props: args, + template: /*html*/ ` + + + Hi, I am a link. + + + + + + + + I'm just static :( + + + `, + }), +}; + +export const TextOverflow: Story = { + render: (args) => ({ + props: args, + template: /*html*/ ` +
TODO: Fix truncation
+ + + Helloooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo + + + `, + }), +}; + +export const MultipleActionList: Story = { + render: (args) => ({ + props: args, + template: /*html*/ ` + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + `, + }), +}; + +export const SingleActionList: Story = { + render: (args) => ({ + props: args, + template: /*html*/ ` + + + + Foobar + + + + + + Foobar + + + + + + Foobar + + + + + + Foobar + + + + + + Foobar + + + + + + Foobar + + + + + `, + }), +}; + +export const VirtualScrolling: Story = { + render: (_args) => ({ + props: { + data: Array.from(Array(100000).keys()), + }, + template: /*html*/ ` + + + + + + + + + + + + + + + + + + + + `, + }), +}; diff --git a/libs/components/src/search/search.component.ts b/libs/components/src/search/search.component.ts index a0f3eb363f..27170d5d7b 100644 --- a/libs/components/src/search/search.component.ts +++ b/libs/components/src/search/search.component.ts @@ -1,7 +1,7 @@ import { Component, ElementRef, Input, ViewChild } from "@angular/core"; import { ControlValueAccessor, NG_VALUE_ACCESSOR } from "@angular/forms"; -import { FocusableElement } from "../input/autofocus.directive"; +import { FocusableElement } from "../shared/focusable-element"; let nextId = 0; @@ -32,8 +32,8 @@ export class SearchComponent implements ControlValueAccessor, FocusableElement { @Input() disabled: boolean; @Input() placeholder: string; - focus() { - this.input.nativeElement.focus(); + getFocusTarget() { + return this.input.nativeElement; } onChange(searchText: string) { diff --git a/libs/components/src/shared/focusable-element.ts b/libs/components/src/shared/focusable-element.ts new file mode 100644 index 0000000000..1ea422aa6f --- /dev/null +++ b/libs/components/src/shared/focusable-element.ts @@ -0,0 +1,8 @@ +/** + * Interface for implementing focusable components. + * + * Used by the `AutofocusDirective` and `A11yGridDirective`. + */ +export abstract class FocusableElement { + getFocusTarget: () => HTMLElement; +} diff --git a/libs/components/src/styles.scss b/libs/components/src/styles.scss index ae97838e09..7ddcb1b64b 100644 --- a/libs/components/src/styles.scss +++ b/libs/components/src/styles.scss @@ -49,6 +49,6 @@ $card-icons-base: "../../src/billing/images/cards/"; @import "multi-select/scss/bw.theme.scss"; // Workaround for https://bitwarden.atlassian.net/browse/CL-110 -#storybook-docs pre.prismjs { +.sbdocs-preview pre.prismjs { color: white; } From 418d4642da81e09de6a4b445042a26592a017c98 Mon Sep 17 00:00:00 2001 From: Alex Morask <144709477+amorask-bitwarden@users.noreply.github.com> Date: Tue, 30 Apr 2024 10:55:00 -0400 Subject: [PATCH 36/41] Hide grace period note when in self-serve trial (#8768) --- ...organization-subscription-selfhost.component.html | 5 ++++- .../self-hosted-organization-subscription.view.ts | 12 ++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/apps/web/src/app/billing/organizations/organization-subscription-selfhost.component.html b/apps/web/src/app/billing/organizations/organization-subscription-selfhost.component.html index 6d6691f336..b4c1224db9 100644 --- a/apps/web/src/app/billing/organizations/organization-subscription-selfhost.component.html +++ b/apps/web/src/app/billing/organizations/organization-subscription-selfhost.component.html @@ -42,7 +42,10 @@ : subscription.expirationWithGracePeriod ) | date: "mediumDate" }} -
+
{{ "selfHostGracePeriodHelp" | i18n: (subscription.expirationWithGracePeriod | date: "mediumDate") diff --git a/libs/common/src/billing/models/view/self-hosted-organization-subscription.view.ts b/libs/common/src/billing/models/view/self-hosted-organization-subscription.view.ts index c1f5640207..7b49688294 100644 --- a/libs/common/src/billing/models/view/self-hosted-organization-subscription.view.ts +++ b/libs/common/src/billing/models/view/self-hosted-organization-subscription.view.ts @@ -58,4 +58,16 @@ export class SelfHostedOrganizationSubscriptionView implements View { get isExpiredAndOutsideGracePeriod() { return this.hasExpiration && this.expirationWithGracePeriod < new Date(); } + + /** + * In the case of a trial, where there is no grace period, the expirationWithGracePeriod and expirationWithoutGracePeriod will + * be exactly the same. This can be used to hide the grace period note. + */ + get isInTrial() { + return ( + this.expirationWithGracePeriod && + this.expirationWithoutGracePeriod && + this.expirationWithGracePeriod.getTime() === this.expirationWithoutGracePeriod.getTime() + ); + } } From 04decd1c09a69bde07c389c481264badba3355e9 Mon Sep 17 00:00:00 2001 From: cyprain-okeke <108260115+cyprain-okeke@users.noreply.github.com> Date: Tue, 30 Apr 2024 16:35:39 +0100 Subject: [PATCH 37/41] [AC-2265] As a Provider Admin, I shouldn't be able to use my client organizations' billing pages (#8981) * initial commit * add the feature flag * Resolve pr comments --- .../icons/manage-billing.icon.ts | 25 +++++++++++++++++++ ...nization-subscription-cloud.component.html | 12 ++++++++- ...ganization-subscription-cloud.component.ts | 16 +++++++++++- apps/web/src/locales/en/messages.json | 3 +++ 4 files changed, 54 insertions(+), 2 deletions(-) create mode 100644 apps/web/src/app/billing/organizations/icons/manage-billing.icon.ts diff --git a/apps/web/src/app/billing/organizations/icons/manage-billing.icon.ts b/apps/web/src/app/billing/organizations/icons/manage-billing.icon.ts new file mode 100644 index 0000000000..6f583bf2e8 --- /dev/null +++ b/apps/web/src/app/billing/organizations/icons/manage-billing.icon.ts @@ -0,0 +1,25 @@ +import { svgIcon } from "@bitwarden/components"; + +export const ManageBilling = svgIcon` + + + + + + + + + + + + + + + + + + + + + + `; diff --git a/apps/web/src/app/billing/organizations/organization-subscription-cloud.component.html b/apps/web/src/app/billing/organizations/organization-subscription-cloud.component.html index 16641c0d52..38903bab19 100644 --- a/apps/web/src/app/billing/organizations/organization-subscription-cloud.component.html +++ b/apps/web/src/app/billing/organizations/organization-subscription-cloud.component.html @@ -1,6 +1,6 @@ - + {{ "loading" | i18n }} @@ -256,3 +256,13 @@ + +
+ + {{ + "manageBillingFromProviderPortalMessage" | i18n + }} +
+ diff --git a/apps/web/src/app/billing/organizations/organization-subscription-cloud.component.ts b/apps/web/src/app/billing/organizations/organization-subscription-cloud.component.ts index 477032deba..7acb108808 100644 --- a/apps/web/src/app/billing/organizations/organization-subscription-cloud.component.ts +++ b/apps/web/src/app/billing/organizations/organization-subscription-cloud.component.ts @@ -5,7 +5,7 @@ import { concatMap, firstValueFrom, lastValueFrom, Observable, Subject, takeUnti import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { OrganizationApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/organization/organization-api.service.abstraction"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; -import { OrganizationApiKeyType } from "@bitwarden/common/admin-console/enums"; +import { OrganizationApiKeyType, ProviderType } from "@bitwarden/common/admin-console/enums"; import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { PlanType } from "@bitwarden/common/billing/enums"; import { OrganizationSubscriptionResponse } from "@bitwarden/common/billing/models/response/organization-subscription.response"; @@ -28,6 +28,7 @@ import { } from "../shared/offboarding-survey.component"; import { BillingSyncApiKeyComponent } from "./billing-sync-api-key.component"; +import { ManageBilling } from "./icons/manage-billing.icon"; import { SecretsManagerSubscriptionOptions } from "./sm-adjust-subscription.component"; @Component({ @@ -47,11 +48,17 @@ export class OrganizationSubscriptionCloudComponent implements OnInit, OnDestroy loading: boolean; locale: string; showUpdatedSubscriptionStatusSection$: Observable; + manageBillingFromProviderPortal = ManageBilling; + IsProviderManaged = false; protected readonly teamsStarter = ProductType.TeamsStarter; private destroy$ = new Subject(); + protected enableConsolidatedBilling$ = this.configService.getFeatureFlag$( + FeatureFlag.EnableConsolidatedBilling, + ); + constructor( private apiService: ApiService, private platformUtilsService: PlatformUtilsService, @@ -99,6 +106,13 @@ export class OrganizationSubscriptionCloudComponent implements OnInit, OnDestroy this.loading = true; this.locale = await firstValueFrom(this.i18nService.locale$); this.userOrg = await this.organizationService.get(this.organizationId); + const enableConsolidatedBilling = await firstValueFrom(this.enableConsolidatedBilling$); + this.IsProviderManaged = + this.userOrg.hasProvider && + this.userOrg.providerType == ProviderType.Msp && + enableConsolidatedBilling + ? true + : false; if (this.userOrg.canViewSubscription) { this.sub = await this.organizationApiService.getSubscription(this.organizationId); this.lineItems = this.sub?.subscription?.items; diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index 63069a83de..3a590622b8 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -8049,5 +8049,8 @@ }, "collectionItemSelect": { "message": "Select collection item" + }, + "manageBillingFromProviderPortalMessage": { + "message": "Manage billing from the Provider Portal" } } From be50a174def9763b88086c60552cc31320c34f78 Mon Sep 17 00:00:00 2001 From: Colton Hurst Date: Tue, 30 Apr 2024 12:31:09 -0400 Subject: [PATCH 38/41] SM-1196: Update export file name (#8865) --- .../event-logs/service-accounts-events.component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/event-logs/service-accounts-events.component.ts b/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/event-logs/service-accounts-events.component.ts index 554e7fa37d..0547c4fcba 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/event-logs/service-accounts-events.component.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/event-logs/service-accounts-events.component.ts @@ -17,7 +17,7 @@ import { ServiceAccountEventLogApiService } from "./service-account-event-log-ap templateUrl: "./service-accounts-events.component.html", }) export class ServiceAccountEventsComponent extends BaseEventsComponent implements OnDestroy { - exportFileName = "service-account-events"; + exportFileName = "machine-account-events"; private destroy$ = new Subject(); private serviceAccountId: string; From 3acbffa0726d8b4bda5c410d2ad98cf226a8ab7a Mon Sep 17 00:00:00 2001 From: Justin Baur <19896123+justindbaur@users.noreply.github.com> Date: Tue, 30 Apr 2024 12:35:36 -0400 Subject: [PATCH 39/41] [PM-6144] Basic auth autofill in Manifest v3 (#8975) * Add Support for autofilling Basic Auth to MV3 * Remove `any` --- .../background/web-request.background.ts | 34 ++++++------------- .../browser/src/background/main.background.ts | 7 ++-- apps/browser/src/manifest.v3.json | 4 ++- 3 files changed, 16 insertions(+), 29 deletions(-) diff --git a/apps/browser/src/autofill/background/web-request.background.ts b/apps/browser/src/autofill/background/web-request.background.ts index 8cdfa0f027..2eb976529f 100644 --- a/apps/browser/src/autofill/background/web-request.background.ts +++ b/apps/browser/src/autofill/background/web-request.background.ts @@ -4,40 +4,29 @@ import { UriMatchStrategy } from "@bitwarden/common/models/domain/domain-service import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; -import { BrowserApi } from "../../platform/browser/browser-api"; - export default class WebRequestBackground { - private pendingAuthRequests: any[] = []; - private webRequest: any; + private pendingAuthRequests: Set = new Set([]); private isFirefox: boolean; constructor( platformUtilsService: PlatformUtilsService, private cipherService: CipherService, private authService: AuthService, + private readonly webRequest: typeof chrome.webRequest, ) { - if (BrowserApi.isManifestVersion(2)) { - this.webRequest = chrome.webRequest; - } this.isFirefox = platformUtilsService.isFirefox(); } - async init() { - if (!this.webRequest || !this.webRequest.onAuthRequired) { - return; - } - + startListening() { this.webRequest.onAuthRequired.addListener( - async (details: any, callback: any) => { - if (!details.url || this.pendingAuthRequests.indexOf(details.requestId) !== -1) { + async (details, callback) => { + if (!details.url || this.pendingAuthRequests.has(details.requestId)) { if (callback) { - callback(); + callback(null); } return; } - - this.pendingAuthRequests.push(details.requestId); - + this.pendingAuthRequests.add(details.requestId); if (this.isFirefox) { // eslint-disable-next-line return new Promise(async (resolve, reject) => { @@ -51,7 +40,7 @@ export default class WebRequestBackground { [this.isFirefox ? "blocking" : "asyncBlocking"], ); - this.webRequest.onCompleted.addListener((details: any) => this.completeAuthRequest(details), { + this.webRequest.onCompleted.addListener((details) => this.completeAuthRequest(details), { urls: ["http://*/*"], }); this.webRequest.onErrorOccurred.addListener( @@ -91,10 +80,7 @@ export default class WebRequestBackground { } } - private completeAuthRequest(details: any) { - const i = this.pendingAuthRequests.indexOf(details.requestId); - if (i > -1) { - this.pendingAuthRequests.splice(i, 1); - } + private completeAuthRequest(details: chrome.webRequest.WebResponseCacheDetails) { + this.pendingAuthRequests.delete(details.requestId); } } diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 01e325ad51..adcb4a21ba 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -1056,11 +1056,12 @@ export default class MainBackground { this.cipherService, ); - if (BrowserApi.isManifestVersion(2)) { + if (chrome.webRequest != null && chrome.webRequest.onAuthRequired != null) { this.webRequestBackground = new WebRequestBackground( this.platformUtilsService, this.cipherService, this.authService, + chrome.webRequest, ); } } @@ -1106,9 +1107,7 @@ export default class MainBackground { await this.tabsBackground.init(); this.contextMenusBackground?.init(); await this.idleBackground.init(); - if (BrowserApi.isManifestVersion(2)) { - await this.webRequestBackground.init(); - } + this.webRequestBackground?.startListening(); if (this.platformUtilsService.isFirefox() && !this.isPrivateMode) { // Set Private Mode windows to the default icon - they do not share state with the background page diff --git a/apps/browser/src/manifest.v3.json b/apps/browser/src/manifest.v3.json index c0c88706b8..b7aaba2e0e 100644 --- a/apps/browser/src/manifest.v3.json +++ b/apps/browser/src/manifest.v3.json @@ -60,7 +60,9 @@ "clipboardWrite", "idle", "scripting", - "offscreen" + "offscreen", + "webRequest", + "webRequestAuthProvider" ], "optional_permissions": ["nativeMessaging", "privacy"], "host_permissions": [""], From 200b0f75341dd38186f1ea05dab399ab12c4d771 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Tue, 30 Apr 2024 12:46:01 -0400 Subject: [PATCH 40/41] Correct and test changeover point for userId source in storage migration (#8990) --- .../src/state-migrations/migration-helper.spec.ts | 5 +++++ .../src/state-migrations/migration-helper.ts | 6 +++--- .../migrations/60-known-accounts.spec.ts | 14 +++++--------- .../migrations/60-known-accounts.ts | 4 ++-- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/libs/common/src/state-migrations/migration-helper.spec.ts b/libs/common/src/state-migrations/migration-helper.spec.ts index 162fac2fab..21c5c72a18 100644 --- a/libs/common/src/state-migrations/migration-helper.spec.ts +++ b/libs/common/src/state-migrations/migration-helper.spec.ts @@ -235,6 +235,11 @@ export function mockMigrationHelper( helper.setToUser(userId, keyDefinition, value), ); mockHelper.getAccounts.mockImplementation(() => helper.getAccounts()); + mockHelper.getKnownUserIds.mockImplementation(() => helper.getKnownUserIds()); + mockHelper.removeFromGlobal.mockImplementation((keyDefinition) => + helper.removeFromGlobal(keyDefinition), + ); + mockHelper.remove.mockImplementation((key) => helper.remove(key)); mockHelper.type = helper.type; diff --git a/libs/common/src/state-migrations/migration-helper.ts b/libs/common/src/state-migrations/migration-helper.ts index 5d1de8dd49..b377df8ef9 100644 --- a/libs/common/src/state-migrations/migration-helper.ts +++ b/libs/common/src/state-migrations/migration-helper.ts @@ -175,8 +175,8 @@ export class MigrationHelper { * Helper method to read known users ids. */ async getKnownUserIds(): Promise { - if (this.currentVersion < 61) { - return knownAccountUserIdsBuilderPre61(this.storageService); + if (this.currentVersion < 60) { + return knownAccountUserIdsBuilderPre60(this.storageService); } else { return knownAccountUserIdsBuilder(this.storageService); } @@ -245,7 +245,7 @@ function globalKeyBuilderPre9(): string { throw Error("No key builder should be used for versions prior to 9."); } -async function knownAccountUserIdsBuilderPre61( +async function knownAccountUserIdsBuilderPre60( storageService: AbstractStorageService, ): Promise { return (await storageService.get("authenticatedAccounts")) ?? []; diff --git a/libs/common/src/state-migrations/migrations/60-known-accounts.spec.ts b/libs/common/src/state-migrations/migrations/60-known-accounts.spec.ts index 28dedb3c39..01be4adb6a 100644 --- a/libs/common/src/state-migrations/migrations/60-known-accounts.spec.ts +++ b/libs/common/src/state-migrations/migrations/60-known-accounts.spec.ts @@ -51,17 +51,13 @@ const rollbackJson = () => { }, global_account_accounts: { user1: { - profile: { - email: "user1", - name: "User 1", - emailVerified: true, - }, + email: "user1", + name: "User 1", + emailVerified: true, }, user2: { - profile: { - email: "", - emailVerified: false, - }, + email: "", + emailVerified: false, }, }, global_account_activeAccountId: "user1", diff --git a/libs/common/src/state-migrations/migrations/60-known-accounts.ts b/libs/common/src/state-migrations/migrations/60-known-accounts.ts index 75117da5b4..3b02a5acc4 100644 --- a/libs/common/src/state-migrations/migrations/60-known-accounts.ts +++ b/libs/common/src/state-migrations/migrations/60-known-accounts.ts @@ -38,8 +38,8 @@ export class KnownAccountsMigrator extends Migrator<59, 60> { } async rollback(helper: MigrationHelper): Promise { // authenticated account are removed, but the accounts record also contains logged out accounts. Best we can do is to add them all back - const accounts = (await helper.getFromGlobal>(ACCOUNT_ACCOUNTS)) ?? {}; - await helper.set("authenticatedAccounts", Object.keys(accounts)); + const userIds = (await helper.getKnownUserIds()) ?? []; + await helper.set("authenticatedAccounts", userIds); await helper.removeFromGlobal(ACCOUNT_ACCOUNTS); // Active Account Id From b4631b0dd164ee34de9f5dff43a1bf559880ebd0 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Tue, 30 Apr 2024 12:58:16 -0400 Subject: [PATCH 41/41] Ps/improve-log-service (#8989) * Match console method signatures in logService abstraction * Add a few usages of improved signature * Remove reality check test * Improve electron logging --- .../src/background/runtime.background.ts | 3 +- .../offscreen-document.spec.ts | 4 +- .../offscreen-document/offscreen-document.ts | 2 +- .../services/console-log.service.spec.ts | 36 ++++++------ .../platform/services/console-log.service.ts | 6 +- apps/desktop/src/platform/preload.ts | 3 +- .../services/electron-log.main.service.ts | 14 ++--- .../services/electron-log.renderer.service.ts | 14 +++-- .../services/logging-error-handler.ts | 2 +- libs/common/spec/intercept-console.ts | 23 +++----- .../src/platform/abstractions/log.service.ts | 10 ++-- .../services/console-log.service.spec.ts | 57 ++++++++++--------- .../platform/services/console-log.service.ts | 26 ++++----- 13 files changed, 104 insertions(+), 96 deletions(-) diff --git a/apps/browser/src/background/runtime.background.ts b/apps/browser/src/background/runtime.background.ts index 98b1df9c80..d8f3cf840f 100644 --- a/apps/browser/src/background/runtime.background.ts +++ b/apps/browser/src/background/runtime.background.ts @@ -76,7 +76,8 @@ export default class RuntimeBackground { void this.processMessageWithSender(msg, sender).catch((err) => this.logService.error( - `Error while processing message in RuntimeBackground '${msg?.command}'. Error: ${err?.message ?? "Unknown Error"}`, + `Error while processing message in RuntimeBackground '${msg?.command}'.`, + err, ), ); return false; diff --git a/apps/browser/src/platform/offscreen-document/offscreen-document.spec.ts b/apps/browser/src/platform/offscreen-document/offscreen-document.spec.ts index 1cbcc7a94c..933cd08c2e 100644 --- a/apps/browser/src/platform/offscreen-document/offscreen-document.spec.ts +++ b/apps/browser/src/platform/offscreen-document/offscreen-document.spec.ts @@ -28,6 +28,7 @@ describe("OffscreenDocument", () => { }); it("shows a console message if the handler throws an error", async () => { + const error = new Error("test error"); browserClipboardServiceCopySpy.mockRejectedValueOnce(new Error("test error")); sendExtensionRuntimeMessage({ command: "offscreenCopyToClipboard", text: "test" }); @@ -35,7 +36,8 @@ describe("OffscreenDocument", () => { expect(browserClipboardServiceCopySpy).toHaveBeenCalled(); expect(consoleErrorSpy).toHaveBeenCalledWith( - "Error resolving extension message response: Error: test error", + "Error resolving extension message response", + error, ); }); diff --git a/apps/browser/src/platform/offscreen-document/offscreen-document.ts b/apps/browser/src/platform/offscreen-document/offscreen-document.ts index 627036b80b..4994a6e9ba 100644 --- a/apps/browser/src/platform/offscreen-document/offscreen-document.ts +++ b/apps/browser/src/platform/offscreen-document/offscreen-document.ts @@ -71,7 +71,7 @@ class OffscreenDocument implements OffscreenDocumentInterface { Promise.resolve(messageResponse) .then((response) => sendResponse(response)) .catch((error) => - this.consoleLogService.error(`Error resolving extension message response: ${error}`), + this.consoleLogService.error("Error resolving extension message response", error), ); return true; }; diff --git a/apps/cli/src/platform/services/console-log.service.spec.ts b/apps/cli/src/platform/services/console-log.service.spec.ts index 10a0ad8cca..03598b16e6 100644 --- a/apps/cli/src/platform/services/console-log.service.spec.ts +++ b/apps/cli/src/platform/services/console-log.service.spec.ts @@ -2,13 +2,18 @@ import { interceptConsole, restoreConsole } from "@bitwarden/common/spec"; import { ConsoleLogService } from "./console-log.service"; -let caughtMessage: any = {}; - describe("CLI Console log service", () => { + const error = new Error("this is an error"); + const obj = { a: 1, b: 2 }; let logService: ConsoleLogService; + let consoleSpy: { + log: jest.Mock; + warn: jest.Mock; + error: jest.Mock; + }; + beforeEach(() => { - caughtMessage = {}; - interceptConsole(caughtMessage); + consoleSpy = interceptConsole(); logService = new ConsoleLogService(true); }); @@ -19,24 +24,21 @@ describe("CLI Console log service", () => { it("should redirect all console to error if BW_RESPONSE env is true", () => { process.env.BW_RESPONSE = "true"; - logService.debug("this is a debug message"); - expect(caughtMessage).toMatchObject({ - error: { 0: "this is a debug message" }, - }); + logService.debug("this is a debug message", error, obj); + expect(consoleSpy.error).toHaveBeenCalledWith("this is a debug message", error, obj); }); it("should not redirect console to error if BW_RESPONSE != true", () => { process.env.BW_RESPONSE = "false"; - logService.debug("debug"); - logService.info("info"); - logService.warning("warning"); - logService.error("error"); + logService.debug("debug", error, obj); + logService.info("info", error, obj); + logService.warning("warning", error, obj); + logService.error("error", error, obj); - expect(caughtMessage).toMatchObject({ - log: { 0: "info" }, - warn: { 0: "warning" }, - error: { 0: "error" }, - }); + expect(consoleSpy.log).toHaveBeenCalledWith("debug", error, obj); + expect(consoleSpy.log).toHaveBeenCalledWith("info", error, obj); + expect(consoleSpy.warn).toHaveBeenCalledWith("warning", error, obj); + expect(consoleSpy.error).toHaveBeenCalledWith("error", error, obj); }); }); diff --git a/apps/cli/src/platform/services/console-log.service.ts b/apps/cli/src/platform/services/console-log.service.ts index a35dae71fc..5bdc0b4015 100644 --- a/apps/cli/src/platform/services/console-log.service.ts +++ b/apps/cli/src/platform/services/console-log.service.ts @@ -6,17 +6,17 @@ export class ConsoleLogService extends BaseConsoleLogService { super(isDev, filter); } - write(level: LogLevelType, message: string) { + write(level: LogLevelType, message?: any, ...optionalParams: any[]) { if (this.filter != null && this.filter(level)) { return; } if (process.env.BW_RESPONSE === "true") { // eslint-disable-next-line - console.error(message); + console.error(message, ...optionalParams); return; } - super.write(level, message); + super.write(level, message, ...optionalParams); } } diff --git a/apps/desktop/src/platform/preload.ts b/apps/desktop/src/platform/preload.ts index 771d25ef0a..d81d647652 100644 --- a/apps/desktop/src/platform/preload.ts +++ b/apps/desktop/src/platform/preload.ts @@ -103,7 +103,8 @@ export default { isMacAppStore: isMacAppStore(), isWindowsStore: isWindowsStore(), reloadProcess: () => ipcRenderer.send("reload-process"), - log: (level: LogLevelType, message: string) => ipcRenderer.invoke("ipc.log", { level, message }), + log: (level: LogLevelType, message?: any, ...optionalParams: any[]) => + ipcRenderer.invoke("ipc.log", { level, message, optionalParams }), openContextMenu: ( menu: { diff --git a/apps/desktop/src/platform/services/electron-log.main.service.ts b/apps/desktop/src/platform/services/electron-log.main.service.ts index 832365785c..0725de3dc9 100644 --- a/apps/desktop/src/platform/services/electron-log.main.service.ts +++ b/apps/desktop/src/platform/services/electron-log.main.service.ts @@ -25,28 +25,28 @@ export class ElectronLogMainService extends BaseLogService { } log.initialize(); - ipcMain.handle("ipc.log", (_event, { level, message }) => { - this.write(level, message); + ipcMain.handle("ipc.log", (_event, { level, message, optionalParams }) => { + this.write(level, message, ...optionalParams); }); } - write(level: LogLevelType, message: string) { + write(level: LogLevelType, message?: any, ...optionalParams: any[]) { if (this.filter != null && this.filter(level)) { return; } switch (level) { case LogLevelType.Debug: - log.debug(message); + log.debug(message, ...optionalParams); break; case LogLevelType.Info: - log.info(message); + log.info(message, ...optionalParams); break; case LogLevelType.Warning: - log.warn(message); + log.warn(message, ...optionalParams); break; case LogLevelType.Error: - log.error(message); + log.error(message, ...optionalParams); break; default: break; diff --git a/apps/desktop/src/platform/services/electron-log.renderer.service.ts b/apps/desktop/src/platform/services/electron-log.renderer.service.ts index e0e0757e6a..cea939f160 100644 --- a/apps/desktop/src/platform/services/electron-log.renderer.service.ts +++ b/apps/desktop/src/platform/services/electron-log.renderer.service.ts @@ -6,27 +6,29 @@ export class ElectronLogRendererService extends BaseLogService { super(ipc.platform.isDev, filter); } - write(level: LogLevelType, message: string) { + write(level: LogLevelType, message?: any, ...optionalParams: any[]) { if (this.filter != null && this.filter(level)) { return; } /* eslint-disable no-console */ - ipc.platform.log(level, message).catch((e) => console.log("Error logging", e)); + ipc.platform + .log(level, message, ...optionalParams) + .catch((e) => console.log("Error logging", e)); /* eslint-disable no-console */ switch (level) { case LogLevelType.Debug: - console.debug(message); + console.debug(message, ...optionalParams); break; case LogLevelType.Info: - console.info(message); + console.info(message, ...optionalParams); break; case LogLevelType.Warning: - console.warn(message); + console.warn(message, ...optionalParams); break; case LogLevelType.Error: - console.error(message); + console.error(message, ...optionalParams); break; default: break; diff --git a/libs/angular/src/platform/services/logging-error-handler.ts b/libs/angular/src/platform/services/logging-error-handler.ts index 522412dd28..5644272d35 100644 --- a/libs/angular/src/platform/services/logging-error-handler.ts +++ b/libs/angular/src/platform/services/logging-error-handler.ts @@ -14,7 +14,7 @@ export class LoggingErrorHandler extends ErrorHandler { override handleError(error: any): void { try { const logService = this.injector.get(LogService, null); - logService.error(error); + logService.error("Unhandled error in angular", error); } catch { super.handleError(error); } diff --git a/libs/common/spec/intercept-console.ts b/libs/common/spec/intercept-console.ts index 01c4063e7a..565d475cae 100644 --- a/libs/common/spec/intercept-console.ts +++ b/libs/common/spec/intercept-console.ts @@ -2,22 +2,17 @@ const originalConsole = console; declare let console: any; -export function interceptConsole(interceptions: any): object { +export function interceptConsole(): { + log: jest.Mock; + warn: jest.Mock; + error: jest.Mock; +} { console = { - log: function () { - // eslint-disable-next-line - interceptions.log = arguments; - }, - warn: function () { - // eslint-disable-next-line - interceptions.warn = arguments; - }, - error: function () { - // eslint-disable-next-line - interceptions.error = arguments; - }, + log: jest.fn(), + warn: jest.fn(), + error: jest.fn(), }; - return interceptions; + return console; } export function restoreConsole() { diff --git a/libs/common/src/platform/abstractions/log.service.ts b/libs/common/src/platform/abstractions/log.service.ts index dffa3ca8d3..d77a4f6990 100644 --- a/libs/common/src/platform/abstractions/log.service.ts +++ b/libs/common/src/platform/abstractions/log.service.ts @@ -1,9 +1,9 @@ import { LogLevelType } from "../enums/log-level-type.enum"; export abstract class LogService { - abstract debug(message: string): void; - abstract info(message: string): void; - abstract warning(message: string): void; - abstract error(message: string): void; - abstract write(level: LogLevelType, message: string): void; + abstract debug(message?: any, ...optionalParams: any[]): void; + abstract info(message?: any, ...optionalParams: any[]): void; + abstract warning(message?: any, ...optionalParams: any[]): void; + abstract error(message?: any, ...optionalParams: any[]): void; + abstract write(level: LogLevelType, message?: any, ...optionalParams: any[]): void; } diff --git a/libs/common/src/platform/services/console-log.service.spec.ts b/libs/common/src/platform/services/console-log.service.spec.ts index 129969bbc4..508ca4eb32 100644 --- a/libs/common/src/platform/services/console-log.service.spec.ts +++ b/libs/common/src/platform/services/console-log.service.spec.ts @@ -2,13 +2,18 @@ import { interceptConsole, restoreConsole } from "../../../spec"; import { ConsoleLogService } from "./console-log.service"; -let caughtMessage: any; - describe("ConsoleLogService", () => { + const error = new Error("this is an error"); + const obj = { a: 1, b: 2 }; + let consoleSpy: { + log: jest.Mock; + warn: jest.Mock; + error: jest.Mock; + }; let logService: ConsoleLogService; + beforeEach(() => { - caughtMessage = {}; - interceptConsole(caughtMessage); + consoleSpy = interceptConsole(); logService = new ConsoleLogService(true); }); @@ -18,41 +23,41 @@ describe("ConsoleLogService", () => { it("filters messages below the set threshold", () => { logService = new ConsoleLogService(true, () => true); - logService.debug("debug"); - logService.info("info"); - logService.warning("warning"); - logService.error("error"); + logService.debug("debug", error, obj); + logService.info("info", error, obj); + logService.warning("warning", error, obj); + logService.error("error", error, obj); - expect(caughtMessage).toEqual({}); + expect(consoleSpy.log).not.toHaveBeenCalled(); + expect(consoleSpy.warn).not.toHaveBeenCalled(); + expect(consoleSpy.error).not.toHaveBeenCalled(); }); + it("only writes debug messages in dev mode", () => { logService = new ConsoleLogService(false); logService.debug("debug message"); - expect(caughtMessage.log).toBeUndefined(); + expect(consoleSpy.log).not.toHaveBeenCalled(); }); it("writes debug/info messages to console.log", () => { - logService.debug("this is a debug message"); - expect(caughtMessage).toMatchObject({ - log: { "0": "this is a debug message" }, - }); + logService.debug("this is a debug message", error, obj); + logService.info("this is an info message", error, obj); - logService.info("this is an info message"); - expect(caughtMessage).toMatchObject({ - log: { "0": "this is an info message" }, - }); + expect(consoleSpy.log).toHaveBeenCalledTimes(2); + expect(consoleSpy.log).toHaveBeenCalledWith("this is a debug message", error, obj); + expect(consoleSpy.log).toHaveBeenCalledWith("this is an info message", error, obj); }); + it("writes warning messages to console.warn", () => { - logService.warning("this is a warning message"); - expect(caughtMessage).toMatchObject({ - warn: { 0: "this is a warning message" }, - }); + logService.warning("this is a warning message", error, obj); + + expect(consoleSpy.warn).toHaveBeenCalledWith("this is a warning message", error, obj); }); + it("writes error messages to console.error", () => { - logService.error("this is an error message"); - expect(caughtMessage).toMatchObject({ - error: { 0: "this is an error message" }, - }); + logService.error("this is an error message", error, obj); + + expect(consoleSpy.error).toHaveBeenCalledWith("this is an error message", error, obj); }); }); diff --git a/libs/common/src/platform/services/console-log.service.ts b/libs/common/src/platform/services/console-log.service.ts index 3eb3ad1881..a1480a0c26 100644 --- a/libs/common/src/platform/services/console-log.service.ts +++ b/libs/common/src/platform/services/console-log.service.ts @@ -9,26 +9,26 @@ export class ConsoleLogService implements LogServiceAbstraction { protected filter: (level: LogLevelType) => boolean = null, ) {} - debug(message: string) { + debug(message?: any, ...optionalParams: any[]) { if (!this.isDev) { return; } - this.write(LogLevelType.Debug, message); + this.write(LogLevelType.Debug, message, ...optionalParams); } - info(message: string) { - this.write(LogLevelType.Info, message); + info(message?: any, ...optionalParams: any[]) { + this.write(LogLevelType.Info, message, ...optionalParams); } - warning(message: string) { - this.write(LogLevelType.Warning, message); + warning(message?: any, ...optionalParams: any[]) { + this.write(LogLevelType.Warning, message, ...optionalParams); } - error(message: string) { - this.write(LogLevelType.Error, message); + error(message?: any, ...optionalParams: any[]) { + this.write(LogLevelType.Error, message, ...optionalParams); } - write(level: LogLevelType, message: string) { + write(level: LogLevelType, message?: any, ...optionalParams: any[]) { if (this.filter != null && this.filter(level)) { return; } @@ -36,19 +36,19 @@ export class ConsoleLogService implements LogServiceAbstraction { switch (level) { case LogLevelType.Debug: // eslint-disable-next-line - console.log(message); + console.log(message, ...optionalParams); break; case LogLevelType.Info: // eslint-disable-next-line - console.log(message); + console.log(message, ...optionalParams); break; case LogLevelType.Warning: // eslint-disable-next-line - console.warn(message); + console.warn(message, ...optionalParams); break; case LogLevelType.Error: // eslint-disable-next-line - console.error(message); + console.error(message, ...optionalParams); break; default: break;