From ed7a57810e55fcc18e665d6edf7d0493c16e7706 Mon Sep 17 00:00:00 2001 From: cd-bitwarden <106776772+cd-bitwarden@users.noreply.github.com> Date: Tue, 4 Jun 2024 10:58:09 -0400 Subject: [PATCH 01/13] [SM-1192] displaying full project name in tooltip and allowing more of project name in bit badge/project column (#8917) * allowing max width adjustment on bit badge and not truncating the tooltip text on project * suggested changes from WIll * updating values --- .../app/secrets-manager/shared/secrets-list.component.html | 3 ++- libs/components/src/badge/badge.directive.ts | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/shared/secrets-list.component.html b/bitwarden_license/bit-web/src/app/secrets-manager/shared/secrets-list.component.html index f8d5d1081e..4b629ca488 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/shared/secrets-list.component.html +++ b/bitwarden_license/bit-web/src/app/secrets-manager/shared/secrets-list.component.html @@ -93,8 +93,9 @@ variant="secondary" class="tw-ml-1" [title]="project.name" + maxWidthClass="tw-max-w-60" > - {{ project.name | ellipsis: 32 }} + {{ project.name }} Date: Tue, 4 Jun 2024 10:59:43 -0400 Subject: [PATCH 02/13] Removing hanging promises, and adding a guard to projects routing (#8891) * Removing hanging promises, and adding a guard to projects routing * Additional logging * adding tests * Trying to get Jest tests working * coltons suggested changes --- .../overview/overview.component.ts | 13 +- .../dialog/project-dialog.component.ts | 4 +- .../guards/project-access.guard.spec.ts | 120 +++++++++++++++++ .../projects/guards/project-access.guard.ts | 31 +++++ .../project/project-people.component.ts | 5 +- .../project/project-secrets.component.ts | 7 +- .../projects/project/project.component.ts | 14 -- .../projects/projects-routing.module.ts | 2 + .../secrets/dialog/secret-dialog.component.ts | 6 +- .../secrets/secrets.component.ts | 7 +- .../service-account-dialog.component.ts | 4 +- .../service-account-access.guard.spec.ts | 122 ++++++++++++++++++ .../guards/service-account-access.guard.ts | 11 ++ .../service-account.component.ts | 23 +--- .../shared/secrets-list.component.ts | 15 ++- 15 files changed, 321 insertions(+), 63 deletions(-) create mode 100644 bitwarden_license/bit-web/src/app/secrets-manager/projects/guards/project-access.guard.spec.ts create mode 100644 bitwarden_license/bit-web/src/app/secrets-manager/projects/guards/project-access.guard.ts create mode 100644 bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/guards/service-account-access.guard.spec.ts diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/overview/overview.component.ts b/bitwarden_license/bit-web/src/app/secrets-manager/overview/overview.component.ts index 95c1764253..56c02e1ed4 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/overview/overview.component.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/overview/overview.component.ts @@ -17,6 +17,7 @@ import { import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; 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"; import { DialogService } from "@bitwarden/components"; @@ -94,6 +95,7 @@ export class OverviewComponent implements OnInit, OnDestroy { private platformUtilsService: PlatformUtilsService, private i18nService: I18nService, private smOnboardingTasksService: SMOnboardingTasksService, + private logService: LogService, ) {} ngOnInit() { @@ -297,12 +299,13 @@ export class OverviewComponent implements OnInit, OnDestroy { SecretsListComponent.copySecretName(name, this.platformUtilsService, this.i18nService); } - copySecretValue(id: string) { - SecretsListComponent.copySecretValue( + async copySecretValue(id: string) { + await SecretsListComponent.copySecretValue( id, this.platformUtilsService, this.i18nService, this.secretService, + this.logService, ); } @@ -310,11 +313,9 @@ export class OverviewComponent implements OnInit, OnDestroy { SecretsListComponent.copySecretUuid(id, this.platformUtilsService, this.i18nService); } - protected hideOnboarding() { + protected async hideOnboarding() { this.showOnboarding = false; - // 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.saveCompletedTasks(this.organizationId, { + await this.saveCompletedTasks(this.organizationId, { importSecrets: true, createSecret: true, createProject: true, diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/projects/dialog/project-dialog.component.ts b/bitwarden_license/bit-web/src/app/secrets-manager/projects/dialog/project-dialog.component.ts index 0b65bd0a26..d30d5f664e 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/projects/dialog/project-dialog.component.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/projects/dialog/project-dialog.component.ts @@ -82,9 +82,7 @@ export class ProjectDialogComponent implements OnInit { const projectView = this.getProjectView(); if (this.data.operation === OperationType.Add) { const newProject = await this.createProject(projectView); - // 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.router.navigate(["sm", this.data.organizationId, "projects", newProject.id]); + await this.router.navigate(["sm", this.data.organizationId, "projects", newProject.id]); } else { projectView.id = this.data.projectId; await this.updateProject(projectView); diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/projects/guards/project-access.guard.spec.ts b/bitwarden_license/bit-web/src/app/secrets-manager/projects/guards/project-access.guard.spec.ts new file mode 100644 index 0000000000..84bc1483fd --- /dev/null +++ b/bitwarden_license/bit-web/src/app/secrets-manager/projects/guards/project-access.guard.spec.ts @@ -0,0 +1,120 @@ +import { Component } from "@angular/core"; +import { TestBed } from "@angular/core/testing"; +import { Router } from "@angular/router"; +import { RouterTestingModule } from "@angular/router/testing"; +import { MockProxy, mock } from "jest-mock-extended"; + +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; + +import { RouterService } from "../../../../../../../apps/web/src/app/core/router.service"; +import { ProjectView } from "../../models/view/project.view"; +import { ProjectService } from "../project.service"; + +import { projectAccessGuard } from "./project-access.guard"; + +@Component({ + template: "", +}) +export class GuardedRouteTestComponent {} + +@Component({ + template: "", +}) +export class RedirectTestComponent {} + +describe("Project Redirect Guard", () => { + let organizationService: MockProxy; + let routerService: MockProxy; + let projectServiceMock: MockProxy; + let i18nServiceMock: MockProxy; + let platformUtilsService: MockProxy; + let router: Router; + + const smOrg1 = { id: "123", canAccessSecretsManager: true } as Organization; + const projectView = { + id: "123", + organizationId: "123", + name: "project-name", + creationDate: Date.now.toString(), + revisionDate: Date.now.toString(), + read: true, + write: true, + } as ProjectView; + + beforeEach(async () => { + organizationService = mock(); + routerService = mock(); + projectServiceMock = mock(); + i18nServiceMock = mock(); + platformUtilsService = mock(); + + TestBed.configureTestingModule({ + imports: [ + RouterTestingModule.withRoutes([ + { + path: "sm/:organizationId/projects/:projectId", + component: GuardedRouteTestComponent, + canActivate: [projectAccessGuard], + }, + { + path: "sm", + component: RedirectTestComponent, + }, + { + path: "sm/:organizationId/projects", + component: RedirectTestComponent, + }, + ]), + ], + providers: [ + { provide: OrganizationService, useValue: organizationService }, + { provide: RouterService, useValue: routerService }, + { provide: ProjectService, useValue: projectServiceMock }, + { provide: I18nService, useValue: i18nServiceMock }, + { provide: PlatformUtilsService, useValue: platformUtilsService }, + ], + }); + + router = TestBed.inject(Router); + }); + + it("redirects to sm/{orgId}/projects/{projectId} if project exists", async () => { + // Arrange + organizationService.getAll.mockResolvedValue([smOrg1]); + projectServiceMock.getByProjectId.mockReturnValue(Promise.resolve(projectView)); + + // Act + await router.navigateByUrl("sm/123/projects/123"); + + // Assert + expect(router.url).toBe("/sm/123/projects/123"); + }); + + it("redirects to sm/projects if project does not exist", async () => { + // Arrange + organizationService.getAll.mockResolvedValue([smOrg1]); + + // Act + await router.navigateByUrl("sm/123/projects/124"); + + // Assert + expect(router.url).toBe("/sm/123/projects"); + }); + + it("redirects to sm/123/projects if exception occurs while looking for Project", async () => { + // Arrange + jest.spyOn(projectServiceMock, "getByProjectId").mockImplementation(() => { + throw new Error("Test error"); + }); + jest.spyOn(i18nServiceMock, "t").mockReturnValue("Project not found"); + + // Act + await router.navigateByUrl("sm/123/projects/123"); + // Assert + expect(platformUtilsService.showToast).toHaveBeenCalledWith("error", null, "Project not found"); + expect(router.url).toBe("/sm/123/projects"); + }); +}); diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/projects/guards/project-access.guard.ts b/bitwarden_license/bit-web/src/app/secrets-manager/projects/guards/project-access.guard.ts new file mode 100644 index 0000000000..6c08fcc3aa --- /dev/null +++ b/bitwarden_license/bit-web/src/app/secrets-manager/projects/guards/project-access.guard.ts @@ -0,0 +1,31 @@ +import { inject } from "@angular/core"; +import { ActivatedRouteSnapshot, CanActivateFn, createUrlTreeFromSnapshot } from "@angular/router"; + +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; + +import { ProjectService } from "../project.service"; + +/** + * Redirects to projects list if the user doesn't have access to project. + */ +export const projectAccessGuard: CanActivateFn = async (route: ActivatedRouteSnapshot) => { + const projectService = inject(ProjectService); + const platformUtilsService = inject(PlatformUtilsService); + const i18nService = inject(I18nService); + + try { + const project = await projectService.getByProjectId(route.params.projectId); + if (project) { + return true; + } + } catch { + platformUtilsService.showToast( + "error", + null, + i18nService.t("notFound", i18nService.t("project")), + ); + return createUrlTreeFromSnapshot(route, ["/sm", route.params.organizationId, "projects"]); + } + return createUrlTreeFromSnapshot(route, ["/sm", route.params.organizationId, "projects"]); +}; diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project-people.component.ts b/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project-people.component.ts index 835d3825a0..c49008c580 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project-people.component.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project-people.component.ts @@ -4,6 +4,7 @@ import { ActivatedRoute, Router } from "@angular/router"; import { combineLatest, Subject, switchMap, takeUntil, catchError } from "rxjs"; 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"; import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; import { DialogService } from "@bitwarden/components"; @@ -38,8 +39,9 @@ export class ProjectPeopleComponent implements OnInit, OnDestroy { }), ), catchError(async () => { + this.logService.info("Error fetching project people access policies."); await this.router.navigate(["/sm", this.organizationId, "projects"]); - return []; + return undefined; }), ); @@ -70,6 +72,7 @@ export class ProjectPeopleComponent implements OnInit, OnDestroy { private platformUtilsService: PlatformUtilsService, private i18nService: I18nService, private accessPolicySelectorService: AccessPolicySelectorService, + private logService: LogService, ) {} ngOnInit(): void { diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project-secrets.component.ts b/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project-secrets.component.ts index 07d50b28ee..21d6e576a0 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project-secrets.component.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project-secrets.component.ts @@ -4,6 +4,7 @@ import { combineLatest, combineLatestWith, filter, Observable, startWith, switch import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; 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"; import { DialogService } from "@bitwarden/components"; @@ -42,6 +43,7 @@ export class ProjectSecretsComponent { private platformUtilsService: PlatformUtilsService, private i18nService: I18nService, private organizationService: OrganizationService, + private logService: LogService, ) {} ngOnInit() { @@ -109,12 +111,13 @@ export class ProjectSecretsComponent { SecretsListComponent.copySecretName(name, this.platformUtilsService, this.i18nService); } - copySecretValue(id: string) { - SecretsListComponent.copySecretValue( + async copySecretValue(id: string) { + await SecretsListComponent.copySecretValue( id, this.platformUtilsService, this.i18nService, this.secretService, + this.logService, ); } diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project.component.ts b/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project.component.ts index 742c2bea1d..07ca32600a 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project.component.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project.component.ts @@ -1,9 +1,7 @@ import { Component, OnDestroy, OnInit } from "@angular/core"; import { ActivatedRoute, Router } from "@angular/router"; import { - catchError, combineLatest, - EMPTY, filter, Observable, startWith, @@ -58,18 +56,6 @@ export class ProjectComponent implements OnInit, OnDestroy { this.project$ = combineLatest([this.route.params, currentProjectEdited]).pipe( switchMap(([params, _]) => this.projectService.getByProjectId(params.projectId)), - catchError(() => { - // 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.router.navigate(["/sm", this.organizationId, "projects"]).then(() => { - this.platformUtilsService.showToast( - "error", - null, - this.i18nService.t("notFound", this.i18nService.t("project")), - ); - }); - return EMPTY; - }), ); const projectId$ = this.route.params.pipe(map((p) => p.projectId)); diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/projects/projects-routing.module.ts b/bitwarden_license/bit-web/src/app/secrets-manager/projects/projects-routing.module.ts index 6078520989..231486703c 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/projects/projects-routing.module.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/projects/projects-routing.module.ts @@ -1,6 +1,7 @@ import { NgModule } from "@angular/core"; import { RouterModule, Routes } from "@angular/router"; +import { projectAccessGuard } from "./guards/project-access.guard"; import { ProjectPeopleComponent } from "./project/project-people.component"; import { ProjectSecretsComponent } from "./project/project-secrets.component"; import { ProjectServiceAccountsComponent } from "./project/project-service-accounts.component"; @@ -15,6 +16,7 @@ const routes: Routes = [ { path: ":projectId", component: ProjectComponent, + canActivate: [projectAccessGuard], children: [ { path: "", diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/secrets/dialog/secret-dialog.component.ts b/bitwarden_license/bit-web/src/app/secrets-manager/secrets/dialog/secret-dialog.component.ts index b1bd91a04f..0287cdd425 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/secrets/dialog/secret-dialog.component.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/secrets/dialog/secret-dialog.component.ts @@ -199,7 +199,7 @@ export class SecretDialogComponent implements OnInit { return await this.projectService.create(this.data.organizationId, projectView); } - protected openDeleteSecretDialog() { + protected async openDeleteSecretDialog() { const secretListView: SecretListView[] = this.getSecretListView(); const dialogRef = this.dialogService.open( @@ -212,9 +212,7 @@ export class SecretDialogComponent implements OnInit { ); // If the secret is deleted, chain close this dialog after the delete dialog - // 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 - lastValueFrom(dialogRef.closed).then( + await lastValueFrom(dialogRef.closed).then( (closeData) => closeData !== undefined && this.dialogRef.close(), ); } diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/secrets/secrets.component.ts b/bitwarden_license/bit-web/src/app/secrets-manager/secrets/secrets.component.ts index a7413c9b59..2717f96a68 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/secrets/secrets.component.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/secrets/secrets.component.ts @@ -4,6 +4,7 @@ import { combineLatestWith, Observable, startWith, switchMap } from "rxjs"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; 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"; import { DialogService } from "@bitwarden/components"; @@ -39,6 +40,7 @@ export class SecretsComponent implements OnInit { private platformUtilsService: PlatformUtilsService, private i18nService: I18nService, private organizationService: OrganizationService, + private logService: LogService, ) {} ngOnInit() { @@ -97,12 +99,13 @@ export class SecretsComponent implements OnInit { SecretsListComponent.copySecretName(name, this.platformUtilsService, this.i18nService); } - copySecretValue(id: string) { - SecretsListComponent.copySecretValue( + async copySecretValue(id: string) { + await SecretsListComponent.copySecretValue( id, this.platformUtilsService, this.i18nService, this.secretService, + this.logService, ); } diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/dialog/service-account-dialog.component.ts b/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/dialog/service-account-dialog.component.ts index 105ca59e57..de753d8813 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/dialog/service-account-dialog.component.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/dialog/service-account-dialog.component.ts @@ -47,9 +47,7 @@ export class ServiceAccountDialogComponent { async ngOnInit() { if (this.data.operation == OperationType.Edit) { - // 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.loadData(); + await this.loadData(); } } diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/guards/service-account-access.guard.spec.ts b/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/guards/service-account-access.guard.spec.ts new file mode 100644 index 0000000000..956935ac6a --- /dev/null +++ b/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/guards/service-account-access.guard.spec.ts @@ -0,0 +1,122 @@ +import { Component } from "@angular/core"; +import { TestBed } from "@angular/core/testing"; +import { Router } from "@angular/router"; +import { RouterTestingModule } from "@angular/router/testing"; +import { MockProxy, mock } from "jest-mock-extended"; + +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; + +import { RouterService } from "../../../../../../../../clients/apps/web/src/app/core/router.service"; +import { ServiceAccountView } from "../../models/view/service-account.view"; +import { ServiceAccountService } from "../service-account.service"; + +import { serviceAccountAccessGuard } from "./service-account-access.guard"; + +@Component({ + template: "", +}) +export class GuardedRouteTestComponent {} + +@Component({ + template: "", +}) +export class RedirectTestComponent {} + +describe("Service account Redirect Guard", () => { + let organizationService: MockProxy; + let routerService: MockProxy; + let serviceAccountServiceMock: MockProxy; + let i18nServiceMock: MockProxy; + let platformUtilsService: MockProxy; + let router: Router; + + const smOrg1 = { id: "123", canAccessSecretsManager: true } as Organization; + const serviceAccountView = { + id: "123", + organizationId: "123", + name: "service-account-name", + } as ServiceAccountView; + + beforeEach(async () => { + organizationService = mock(); + routerService = mock(); + serviceAccountServiceMock = mock(); + i18nServiceMock = mock(); + platformUtilsService = mock(); + + TestBed.configureTestingModule({ + imports: [ + RouterTestingModule.withRoutes([ + { + path: "sm/:organizationId/machine-accounts/:serviceAccountId", + component: GuardedRouteTestComponent, + canActivate: [serviceAccountAccessGuard], + }, + { + path: "sm", + component: RedirectTestComponent, + }, + { + path: "sm/:organizationId/machine-accounts", + component: RedirectTestComponent, + }, + ]), + ], + providers: [ + { provide: OrganizationService, useValue: organizationService }, + { provide: RouterService, useValue: routerService }, + { provide: ServiceAccountService, useValue: serviceAccountServiceMock }, + { provide: I18nService, useValue: i18nServiceMock }, + { provide: PlatformUtilsService, useValue: platformUtilsService }, + ], + }); + + router = TestBed.inject(Router); + }); + + it("redirects to sm/{orgId}/machine-accounts/{serviceAccountId} if machine account exists", async () => { + // Arrange + organizationService.getAll.mockResolvedValue([smOrg1]); + serviceAccountServiceMock.getByServiceAccountId.mockReturnValue( + Promise.resolve(serviceAccountView), + ); + + // Act + await router.navigateByUrl("sm/123/machine-accounts/123"); + + // Assert + expect(router.url).toBe("/sm/123/machine-accounts/123"); + }); + + it("redirects to sm/machine-accounts if machine account does not exist", async () => { + // Arrange + organizationService.getAll.mockResolvedValue([smOrg1]); + + // Act + await router.navigateByUrl("sm/123/machine-accounts/124"); + + // Assert + expect(router.url).toBe("/sm/123/machine-accounts"); + }); + + it("redirects to sm/123/machine-accounts if exception occurs while looking for service account", async () => { + // Arrange + jest.spyOn(serviceAccountServiceMock, "getByServiceAccountId").mockImplementation(() => { + throw new Error("Test error"); + }); + jest.spyOn(i18nServiceMock, "t").mockReturnValue("Service account not found"); + + // Act + await router.navigateByUrl("sm/123/machine-accounts/123"); + // Assert + expect(platformUtilsService.showToast).toHaveBeenCalledWith( + "error", + null, + "Service account not found", + ); + expect(router.url).toBe("/sm/123/machine-accounts"); + }); +}); diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/guards/service-account-access.guard.ts b/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/guards/service-account-access.guard.ts index c474ec44d5..b72fc5a1fe 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/guards/service-account-access.guard.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/guards/service-account-access.guard.ts @@ -1,6 +1,9 @@ import { inject } from "@angular/core"; import { ActivatedRouteSnapshot, CanActivateFn, createUrlTreeFromSnapshot } from "@angular/router"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; + import { ServiceAccountService } from "../service-account.service"; /** @@ -8,6 +11,8 @@ import { ServiceAccountService } from "../service-account.service"; */ export const serviceAccountAccessGuard: CanActivateFn = async (route: ActivatedRouteSnapshot) => { const serviceAccountService = inject(ServiceAccountService); + const platformUtilsService = inject(PlatformUtilsService); + const i18nService = inject(I18nService); try { const serviceAccount = await serviceAccountService.getByServiceAccountId( @@ -18,6 +23,12 @@ export const serviceAccountAccessGuard: CanActivateFn = async (route: ActivatedR return true; } } catch { + platformUtilsService.showToast( + "error", + null, + i18nService.t("notFound", i18nService.t("machineAccount")), + ); + return createUrlTreeFromSnapshot(route, [ "/sm", route.params.organizationId, diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/service-account.component.ts b/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/service-account.component.ts index bb687c51c6..51b663acce 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/service-account.component.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/service-accounts/service-account.component.ts @@ -1,15 +1,6 @@ import { Component, OnDestroy, OnInit } from "@angular/core"; import { ActivatedRoute, Router } from "@angular/router"; -import { - EMPTY, - Subject, - catchError, - combineLatest, - filter, - startWith, - switchMap, - takeUntil, -} from "rxjs"; +import { Subject, combineLatest, filter, startWith, switchMap, takeUntil } from "rxjs"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; @@ -42,18 +33,6 @@ export class ServiceAccountComponent implements OnInit, OnDestroy { params.organizationId, ), ), - catchError(() => { - // 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.router.navigate(["/sm", this.organizationId, "machine-accounts"]).then(() => { - this.platformUtilsService.showToast( - "error", - null, - this.i18nService.t("notFound", this.i18nService.t("machineAccount")), - ); - }); - return EMPTY; - }), ); constructor( diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/shared/secrets-list.component.ts b/bitwarden_license/bit-web/src/app/secrets-manager/shared/secrets-list.component.ts index d640114e07..9fccacf664 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/shared/secrets-list.component.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/shared/secrets-list.component.ts @@ -3,6 +3,7 @@ import { Component, EventEmitter, Input, OnDestroy, Output } from "@angular/core import { Subject, takeUntil } from "rxjs"; 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"; import { TableDataSource } from "@bitwarden/components"; @@ -134,22 +135,24 @@ export class SecretsListComponent implements OnDestroy { /** * TODO: Refactor to smart component and remove */ - static copySecretValue( + static async copySecretValue( id: string, platformUtilsService: PlatformUtilsService, i18nService: I18nService, secretService: SecretService, + logService: LogService, ) { - const value = secretService.getBySecretId(id).then((secret) => secret.value); - // 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 - SecretsListComponent.copyToClipboardAsync(value, platformUtilsService).then(() => { + try { + const value = await secretService.getBySecretId(id).then((secret) => secret.value); + platformUtilsService.copyToClipboard(value); platformUtilsService.showToast( "success", null, i18nService.t("valueCopied", i18nService.t("value")), ); - }); + } catch { + logService.info("Error fetching secret value."); + } } static copySecretUuid( From 3e93fc946133aec67059abd175804c8b28c541d2 Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Tue, 4 Jun 2024 11:13:04 -0400 Subject: [PATCH 03/13] Prefer UserKeyDefinitions for `getUser` state (#9487) --- libs/common/src/tools/state/buffered-state.spec.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libs/common/src/tools/state/buffered-state.spec.ts b/libs/common/src/tools/state/buffered-state.spec.ts index 09419207ca..8fbb09cb38 100644 --- a/libs/common/src/tools/state/buffered-state.spec.ts +++ b/libs/common/src/tools/state/buffered-state.spec.ts @@ -6,7 +6,7 @@ import { awaitAsync, trackEmissions, } from "../../../spec"; -import { GENERATOR_DISK, KeyDefinition } from "../../platform/state"; +import { GENERATOR_DISK, UserKeyDefinition } from "../../platform/state"; import { UserId } from "../../types/guid"; import { BufferedKeyDefinition } from "./buffered-key-definition"; @@ -16,8 +16,9 @@ const SomeUser = "SomeUser" as UserId; const accountService = mockAccountServiceWith(SomeUser); type SomeType = { foo: boolean; bar: boolean }; -const SOME_KEY = new KeyDefinition(GENERATOR_DISK, "fooBar", { +const SOME_KEY = new UserKeyDefinition(GENERATOR_DISK, "fooBar", { deserializer: (jsonValue) => jsonValue as SomeType, + clearOn: [], }); const BUFFER_KEY = new BufferedKeyDefinition(GENERATOR_DISK, "fooBar_buffer", { deserializer: (jsonValue) => jsonValue as SomeType, From 3acdd9d8fde0d4875b2a61589bcbcfcb53ff03fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=9C=A8=20Audrey=20=E2=9C=A8?= Date: Tue, 4 Jun 2024 11:26:20 -0400 Subject: [PATCH 04/13] [PM-7290] replace legacy abstraction with generation algorithms (#9435) * replace legacy abstraction with generation algorithms * delete mv2-based generator services --- .../generator/abstractions/randomizer.ts | 39 ++ .../legacy-password-generation.service.ts | 10 +- .../legacy-username-generation.service.ts | 12 +- .../passphrase-generator-strategy.spec.ts | 34 +- .../passphrase-generator-strategy.ts | 81 ++-- .../src/tools/generator/password/index.ts | 1 - .../password/password-generation.service.ts | 409 ------------------ .../password-generator-strategy.spec.ts | 38 +- .../password/password-generator-strategy.ts | 140 ++++-- libs/common/src/tools/generator/policies.ts | 48 ++ libs/common/src/tools/generator/random.ts | 62 +++ .../src/tools/generator/rx-operators.ts | 25 +- .../catchall-generator-strategy.spec.ts | 27 +- .../username/catchall-generator-strategy.ts | 54 ++- .../eff-username-generator-strategy.spec.ts | 25 +- .../eff-username-generator-strategy.ts | 47 +- .../forwarder-generator-strategy.spec.ts | 2 +- .../username/forwarder-generator-strategy.ts | 47 +- .../generator/username/forwarders/addy-io.ts | 24 +- .../username/forwarders/duck-duck-go.ts | 24 +- .../generator/username/forwarders/fastmail.ts | 26 +- .../username/forwarders/firefox-relay.ts | 24 +- .../username/forwarders/forward-email.ts | 24 +- .../username/forwarders/simple-login.ts | 24 +- .../src/tools/generator/username/index.ts | 1 - .../subaddress-generator-strategy.spec.ts | 30 +- .../username/subaddress-generator-strategy.ts | 60 +-- .../username/username-generation.service.ts | 198 --------- libs/common/src/tools/generator/util.ts | 41 ++ .../src/tools/generator/word-options.ts | 6 + 30 files changed, 535 insertions(+), 1048 deletions(-) create mode 100644 libs/common/src/tools/generator/abstractions/randomizer.ts delete mode 100644 libs/common/src/tools/generator/password/password-generation.service.ts create mode 100644 libs/common/src/tools/generator/policies.ts create mode 100644 libs/common/src/tools/generator/random.ts delete mode 100644 libs/common/src/tools/generator/username/username-generation.service.ts create mode 100644 libs/common/src/tools/generator/util.ts create mode 100644 libs/common/src/tools/generator/word-options.ts diff --git a/libs/common/src/tools/generator/abstractions/randomizer.ts b/libs/common/src/tools/generator/abstractions/randomizer.ts new file mode 100644 index 0000000000..3322247759 --- /dev/null +++ b/libs/common/src/tools/generator/abstractions/randomizer.ts @@ -0,0 +1,39 @@ +import { WordOptions } from "../word-options"; + +/** Entropy source for credential generation. */ +export interface Randomizer { + /** picks a random entry from a list. + * @param list random entry source. This must have at least one entry. + * @returns a promise that resolves with a random entry from the list. + */ + pick(list: Array): Promise; + + /** picks a random word from a list. + * @param list random entry source. This must have at least one entry. + * @param options customizes the output word + * @returns a promise that resolves with a random word from the list. + */ + pickWord(list: Array, options?: WordOptions): Promise; + + /** Shuffles a list of items + * @param list random entry source. This must have at least two entries. + * @param options.copy shuffles a copy of the input when this is true. + * Defaults to true. + * @returns a promise that resolves with the randomized list. + */ + shuffle(items: Array): Promise>; + + /** Generates a string containing random lowercase ASCII characters and numbers. + * @param length the number of characters to generate + * @returns a promise that resolves with the randomized string. + */ + chars(length: number): Promise; + + /** Selects an integer value from a range by randomly choosing it from + * a uniform distribution. + * @param min the minimum value in the range, inclusive. + * @param max the minimum value in the range, inclusive. + * @returns a promise that resolves with the randomized string. + */ + uniform(min: number, max: number): Promise; +} diff --git a/libs/common/src/tools/generator/legacy-password-generation.service.ts b/libs/common/src/tools/generator/legacy-password-generation.service.ts index db4aa9d2a9..d69d4d2dc0 100644 --- a/libs/common/src/tools/generator/legacy-password-generation.service.ts +++ b/libs/common/src/tools/generator/legacy-password-generation.service.ts @@ -40,11 +40,11 @@ import { import { GeneratedPasswordHistory, PasswordGenerationOptions, - PasswordGenerationService, PasswordGeneratorOptions, PasswordGeneratorPolicy, PasswordGeneratorStrategy, } from "./password"; +import { CryptoServiceRandomizer } from "./random"; type MappedOptions = { generator: GeneratorNavigation; @@ -60,17 +60,15 @@ export function legacyPasswordGenerationServiceFactory( accountService: AccountService, stateProvider: StateProvider, ): PasswordGenerationServiceAbstraction { - // FIXME: Once the password generation service is replaced with this service - // in the clients, factor out the deprecated service in its entirety. - const deprecatedService = new PasswordGenerationService(cryptoService, null, null); + const randomizer = new CryptoServiceRandomizer(cryptoService); const passwords = new DefaultGeneratorService( - new PasswordGeneratorStrategy(deprecatedService, stateProvider), + new PasswordGeneratorStrategy(randomizer, stateProvider), policyService, ); const passphrases = new DefaultGeneratorService( - new PassphraseGeneratorStrategy(deprecatedService, stateProvider), + new PassphraseGeneratorStrategy(randomizer, stateProvider), policyService, ); diff --git a/libs/common/src/tools/generator/legacy-username-generation.service.ts b/libs/common/src/tools/generator/legacy-username-generation.service.ts index 61c19ee314..aaa6bc2c80 100644 --- a/libs/common/src/tools/generator/legacy-username-generation.service.ts +++ b/libs/common/src/tools/generator/legacy-username-generation.service.ts @@ -14,6 +14,7 @@ import { DefaultGeneratorService } from "./default-generator.service"; import { DefaultGeneratorNavigationService } from "./navigation/default-generator-navigation.service"; import { GeneratorNavigation } from "./navigation/generator-navigation"; import { NoPolicy } from "./no-policy"; +import { CryptoServiceRandomizer } from "./random"; import { CatchallGeneratorStrategy, SubaddressGeneratorStrategy, @@ -37,7 +38,6 @@ import { } from "./username/options/forwarder-options"; import { SubaddressGenerationOptions } from "./username/subaddress-generator-options"; import { UsernameGeneratorOptions } from "./username/username-generation-options"; -import { UsernameGenerationService } from "./username/username-generation.service"; type MappedOptions = { generator: GeneratorNavigation; @@ -65,22 +65,20 @@ export function legacyUsernameGenerationServiceFactory( accountService: AccountService, stateProvider: StateProvider, ): UsernameGenerationServiceAbstraction { - // FIXME: Once the username generation service is replaced with this service - // in the clients, factor out the deprecated service in its entirety. - const deprecatedService = new UsernameGenerationService(cryptoService, null, null); + const randomizer = new CryptoServiceRandomizer(cryptoService); const effUsername = new DefaultGeneratorService( - new EffUsernameGeneratorStrategy(deprecatedService, stateProvider), + new EffUsernameGeneratorStrategy(randomizer, stateProvider), policyService, ); const subaddress = new DefaultGeneratorService( - new SubaddressGeneratorStrategy(deprecatedService, stateProvider), + new SubaddressGeneratorStrategy(randomizer, stateProvider), policyService, ); const catchall = new DefaultGeneratorService( - new CatchallGeneratorStrategy(deprecatedService, stateProvider), + new CatchallGeneratorStrategy(randomizer, stateProvider), policyService, ); diff --git a/libs/common/src/tools/generator/passphrase/passphrase-generator-strategy.spec.ts b/libs/common/src/tools/generator/passphrase/passphrase-generator-strategy.spec.ts index 6ad1bd90dd..429f81175a 100644 --- a/libs/common/src/tools/generator/passphrase/passphrase-generator-strategy.spec.ts +++ b/libs/common/src/tools/generator/passphrase/passphrase-generator-strategy.spec.ts @@ -11,7 +11,7 @@ import { PolicyType } from "../../../admin-console/enums"; import { Policy } from "../../../admin-console/models/domain/policy"; import { StateProvider } from "../../../platform/state"; import { UserId } from "../../../types/guid"; -import { PasswordGenerationServiceAbstraction } from "../abstractions/password-generation.service.abstraction"; +import { Randomizer } from "../abstractions/randomizer"; import { PASSPHRASE_SETTINGS } from "../key-definitions"; import { DisabledPassphraseGeneratorPolicy } from "./passphrase-generator-policy"; @@ -65,8 +65,8 @@ describe("Password generation strategy", () => { describe("durableState", () => { it("should use password settings key", () => { const provider = mock(); - const legacy = mock(); - const strategy = new PassphraseGeneratorStrategy(legacy, provider); + const randomizer = mock(); + const strategy = new PassphraseGeneratorStrategy(randomizer, provider); strategy.durableState(SomeUser); @@ -86,36 +86,14 @@ describe("Password generation strategy", () => { describe("policy", () => { it("should use password generator policy", () => { - const legacy = mock(); - const strategy = new PassphraseGeneratorStrategy(legacy, null); + const randomizer = mock(); + const strategy = new PassphraseGeneratorStrategy(randomizer, null); expect(strategy.policy).toBe(PolicyType.PasswordGenerator); }); }); describe("generate()", () => { - it("should call the legacy service with the given options", async () => { - const legacy = mock(); - const strategy = new PassphraseGeneratorStrategy(legacy, null); - const options = { - type: "passphrase", - minNumberWords: 1, - capitalize: true, - includeNumber: true, - }; - - await strategy.generate(options); - - expect(legacy.generatePassphrase).toHaveBeenCalledWith(options); - }); - - it("should set the generation type to passphrase", async () => { - const legacy = mock(); - const strategy = new PassphraseGeneratorStrategy(legacy, null); - - await strategy.generate({ type: "foo" } as any); - - expect(legacy.generatePassphrase).toHaveBeenCalledWith({ type: "passphrase" }); - }); + it.todo("should generate a password using the given options"); }); }); diff --git a/libs/common/src/tools/generator/passphrase/passphrase-generator-strategy.ts b/libs/common/src/tools/generator/passphrase/passphrase-generator-strategy.ts index c7b5ff8b78..3ed6a1219c 100644 --- a/libs/common/src/tools/generator/passphrase/passphrase-generator-strategy.ts +++ b/libs/common/src/tools/generator/passphrase/passphrase-generator-strategy.ts @@ -1,25 +1,20 @@ -import { BehaviorSubject, map, pipe } from "rxjs"; - import { GeneratorStrategy } from ".."; import { PolicyType } from "../../../admin-console/enums"; +import { EFFLongWordList } from "../../../platform/misc/wordlist"; import { StateProvider } from "../../../platform/state"; -import { UserId } from "../../../types/guid"; -import { PasswordGenerationServiceAbstraction } from "../abstractions/password-generation.service.abstraction"; +import { Randomizer } from "../abstractions/randomizer"; import { PASSPHRASE_SETTINGS } from "../key-definitions"; -import { distinctIfShallowMatch, reduceCollection } from "../rx-operators"; +import { Policies } from "../policies"; +import { mapPolicyToEvaluator } from "../rx-operators"; +import { clone$PerUserId, sharedStateByUserId } from "../util"; import { PassphraseGenerationOptions, DefaultPassphraseGenerationOptions, } from "./passphrase-generation-options"; -import { PassphraseGeneratorOptionsEvaluator } from "./passphrase-generator-options-evaluator"; -import { - DisabledPassphraseGeneratorPolicy, - PassphraseGeneratorPolicy, - leastPrivilege, -} from "./passphrase-generator-policy"; +import { PassphraseGeneratorPolicy } from "./passphrase-generator-policy"; -/** {@link GeneratorStrategy} */ +/** Generates passphrases composed of random words */ export class PassphraseGeneratorStrategy implements GeneratorStrategy { @@ -28,36 +23,48 @@ export class PassphraseGeneratorStrategy * @param stateProvider provides durable state */ constructor( - private legacy: PasswordGenerationServiceAbstraction, + private randomizer: Randomizer, private stateProvider: StateProvider, ) {} - /** {@link GeneratorStrategy.durableState} */ - durableState(id: UserId) { - return this.stateProvider.getUser(id, PASSPHRASE_SETTINGS); - } - - /** Gets the default options. */ - defaults$(_: UserId) { - return new BehaviorSubject({ ...DefaultPassphraseGenerationOptions }).asObservable(); - } - - /** {@link GeneratorStrategy.policy} */ - get policy() { - return PolicyType.PasswordGenerator; - } - - /** {@link GeneratorStrategy.toEvaluator} */ + // configuration + durableState = sharedStateByUserId(PASSPHRASE_SETTINGS, this.stateProvider); + defaults$ = clone$PerUserId(DefaultPassphraseGenerationOptions); + readonly policy = PolicyType.PasswordGenerator; toEvaluator() { - return pipe( - reduceCollection(leastPrivilege, DisabledPassphraseGeneratorPolicy), - distinctIfShallowMatch(), - map((policy) => new PassphraseGeneratorOptionsEvaluator(policy)), - ); + return mapPolicyToEvaluator(Policies.Passphrase); } - /** {@link GeneratorStrategy.generate} */ - generate(options: PassphraseGenerationOptions): Promise { - return this.legacy.generatePassphrase({ ...options, type: "passphrase" }); + // algorithm + async generate(options: PassphraseGenerationOptions): Promise { + const o = { ...DefaultPassphraseGenerationOptions, ...options }; + if (o.numWords == null || o.numWords <= 2) { + o.numWords = DefaultPassphraseGenerationOptions.numWords; + } + if (o.capitalize == null) { + o.capitalize = false; + } + if (o.includeNumber == null) { + o.includeNumber = false; + } + + // select which word gets the number, if any + let luckyNumber = -1; + if (o.includeNumber) { + luckyNumber = await this.randomizer.uniform(0, o.numWords); + } + + // generate the passphrase + const wordList = new Array(o.numWords); + for (let i = 0; i < o.numWords; i++) { + const word = await this.randomizer.pickWord(EFFLongWordList, { + titleCase: o.capitalize, + number: i === luckyNumber, + }); + + wordList[i] = word; + } + + return wordList.join(o.wordSeparator); } } diff --git a/libs/common/src/tools/generator/password/index.ts b/libs/common/src/tools/generator/password/index.ts index e17ab8201c..7e16a2c442 100644 --- a/libs/common/src/tools/generator/password/index.ts +++ b/libs/common/src/tools/generator/password/index.ts @@ -7,5 +7,4 @@ export { PasswordGeneratorStrategy } from "./password-generator-strategy"; // legacy interfaces export { PasswordGeneratorOptions } from "./password-generator-options"; export { PasswordGenerationServiceAbstraction } from "../abstractions/password-generation.service.abstraction"; -export { PasswordGenerationService } from "./password-generation.service"; export { GeneratedPasswordHistory } from "./generated-password-history"; diff --git a/libs/common/src/tools/generator/password/password-generation.service.ts b/libs/common/src/tools/generator/password/password-generation.service.ts deleted file mode 100644 index e193b0fd33..0000000000 --- a/libs/common/src/tools/generator/password/password-generation.service.ts +++ /dev/null @@ -1,409 +0,0 @@ -import { from } from "rxjs"; - -import { PolicyService } from "../../../admin-console/abstractions/policy/policy.service.abstraction"; -import { PolicyType } from "../../../admin-console/enums"; -import { PasswordGeneratorPolicyOptions } from "../../../admin-console/models/domain/password-generator-policy-options"; -import { CryptoService } from "../../../platform/abstractions/crypto.service"; -import { StateService } from "../../../platform/abstractions/state.service"; -import { EFFLongWordList } from "../../../platform/misc/wordlist"; -import { EncString } from "../../../platform/models/domain/enc-string"; -import { PasswordGenerationServiceAbstraction } from "../abstractions/password-generation.service.abstraction"; -import { PassphraseGeneratorOptionsEvaluator } from "../passphrase/passphrase-generator-options-evaluator"; - -import { GeneratedPasswordHistory } from "./generated-password-history"; -import { PasswordGeneratorOptions } from "./password-generator-options"; -import { PasswordGeneratorOptionsEvaluator } from "./password-generator-options-evaluator"; - -const DefaultOptions: PasswordGeneratorOptions = { - length: 14, - minLength: 5, - ambiguous: false, - number: true, - minNumber: 1, - uppercase: true, - minUppercase: 0, - lowercase: true, - minLowercase: 0, - special: false, - minSpecial: 0, - type: "password", - numWords: 3, - wordSeparator: "-", - capitalize: false, - includeNumber: false, -}; - -const DefaultPolicy = new PasswordGeneratorPolicyOptions(); - -const MaxPasswordsInHistory = 100; - -export class PasswordGenerationService implements PasswordGenerationServiceAbstraction { - constructor( - private cryptoService: CryptoService, - private policyService: PolicyService, - private stateService: StateService, - ) {} - - async generatePassword(options: PasswordGeneratorOptions): Promise { - if ((options.type ?? DefaultOptions.type) === "passphrase") { - return this.generatePassphrase({ ...DefaultOptions, ...options }); - } - - const evaluator = new PasswordGeneratorOptionsEvaluator(DefaultPolicy); - const o = evaluator.sanitize({ ...DefaultOptions, ...options }); - - const positions: string[] = []; - if (o.lowercase && o.minLowercase > 0) { - for (let i = 0; i < o.minLowercase; i++) { - positions.push("l"); - } - } - if (o.uppercase && o.minUppercase > 0) { - for (let i = 0; i < o.minUppercase; i++) { - positions.push("u"); - } - } - if (o.number && o.minNumber > 0) { - for (let i = 0; i < o.minNumber; i++) { - positions.push("n"); - } - } - if (o.special && o.minSpecial > 0) { - for (let i = 0; i < o.minSpecial; i++) { - positions.push("s"); - } - } - while (positions.length < o.length) { - positions.push("a"); - } - - // shuffle - await this.shuffleArray(positions); - - // build out the char sets - let allCharSet = ""; - - let lowercaseCharSet = "abcdefghijkmnopqrstuvwxyz"; - if (o.ambiguous) { - lowercaseCharSet += "l"; - } - if (o.lowercase) { - allCharSet += lowercaseCharSet; - } - - let uppercaseCharSet = "ABCDEFGHJKLMNPQRSTUVWXYZ"; - if (o.ambiguous) { - uppercaseCharSet += "IO"; - } - if (o.uppercase) { - allCharSet += uppercaseCharSet; - } - - let numberCharSet = "23456789"; - if (o.ambiguous) { - numberCharSet += "01"; - } - if (o.number) { - allCharSet += numberCharSet; - } - - const specialCharSet = "!@#$%^&*"; - if (o.special) { - allCharSet += specialCharSet; - } - - let password = ""; - for (let i = 0; i < o.length; i++) { - let positionChars: string; - switch (positions[i]) { - case "l": - positionChars = lowercaseCharSet; - break; - case "u": - positionChars = uppercaseCharSet; - break; - case "n": - positionChars = numberCharSet; - break; - case "s": - positionChars = specialCharSet; - break; - case "a": - positionChars = allCharSet; - break; - default: - break; - } - - const randomCharIndex = await this.cryptoService.randomNumber(0, positionChars.length - 1); - password += positionChars.charAt(randomCharIndex); - } - - return password; - } - - async generatePassphrase(options: PasswordGeneratorOptions): Promise { - const evaluator = new PassphraseGeneratorOptionsEvaluator(DefaultPolicy); - const o = evaluator.sanitize({ ...DefaultOptions, ...options }); - - if (o.numWords == null || o.numWords <= 2) { - o.numWords = DefaultOptions.numWords; - } - if (o.capitalize == null) { - o.capitalize = false; - } - if (o.includeNumber == null) { - o.includeNumber = false; - } - - const listLength = EFFLongWordList.length - 1; - const wordList = new Array(o.numWords); - for (let i = 0; i < o.numWords; i++) { - const wordIndex = await this.cryptoService.randomNumber(0, listLength); - if (o.capitalize) { - wordList[i] = this.capitalize(EFFLongWordList[wordIndex]); - } else { - wordList[i] = EFFLongWordList[wordIndex]; - } - } - - if (o.includeNumber) { - await this.appendRandomNumberToRandomWord(wordList); - } - return wordList.join(o.wordSeparator); - } - - getOptions$() { - return from(this.getOptions()); - } - - async getOptions(): Promise<[PasswordGeneratorOptions, PasswordGeneratorPolicyOptions]> { - let options = await this.stateService.getPasswordGenerationOptions(); - if (options == null) { - options = Object.assign({}, DefaultOptions); - } else { - options = Object.assign({}, DefaultOptions, options); - } - await this.stateService.setPasswordGenerationOptions(options); - const enforcedOptions = await this.enforcePasswordGeneratorPoliciesOnOptions(options); - options = enforcedOptions[0]; - return [options, enforcedOptions[1]]; - } - - async enforcePasswordGeneratorPoliciesOnOptions( - options: PasswordGeneratorOptions, - ): Promise<[PasswordGeneratorOptions, PasswordGeneratorPolicyOptions]> { - let policy = await this.getPasswordGeneratorPolicyOptions(); - policy = policy ?? new PasswordGeneratorPolicyOptions(); - - // Force default type if password/passphrase selected via policy - if (policy.defaultType === "password" || policy.defaultType === "passphrase") { - options.type = policy.defaultType; - } - - const evaluator = - options.type == "password" - ? new PasswordGeneratorOptionsEvaluator(policy) - : new PassphraseGeneratorOptionsEvaluator(policy); - - // Ensure the options to pass the current rules - const withPolicy = evaluator.applyPolicy(options); - const sanitized = evaluator.sanitize(withPolicy); - - // callers assume this function updates the options parameter - const result = Object.assign(options, sanitized); - return [result, policy]; - } - - async getPasswordGeneratorPolicyOptions(): Promise { - const policies = await this.policyService?.getAll(PolicyType.PasswordGenerator); - let enforcedOptions: PasswordGeneratorPolicyOptions = null; - - if (policies == null || policies.length === 0) { - return enforcedOptions; - } - - policies.forEach((currentPolicy) => { - if (!currentPolicy.enabled || currentPolicy.data == null) { - return; - } - - if (enforcedOptions == null) { - enforcedOptions = new PasswordGeneratorPolicyOptions(); - } - - // Password wins in multi-org collisions - if (currentPolicy.data.defaultType != null && enforcedOptions.defaultType !== "password") { - enforcedOptions.defaultType = currentPolicy.data.defaultType; - } - - if ( - currentPolicy.data.minLength != null && - currentPolicy.data.minLength > enforcedOptions.minLength - ) { - enforcedOptions.minLength = currentPolicy.data.minLength; - } - - if (currentPolicy.data.useUpper) { - enforcedOptions.useUppercase = true; - } - - if (currentPolicy.data.useLower) { - enforcedOptions.useLowercase = true; - } - - if (currentPolicy.data.useNumbers) { - enforcedOptions.useNumbers = true; - } - - if ( - currentPolicy.data.minNumbers != null && - currentPolicy.data.minNumbers > enforcedOptions.numberCount - ) { - enforcedOptions.numberCount = currentPolicy.data.minNumbers; - } - - if (currentPolicy.data.useSpecial) { - enforcedOptions.useSpecial = true; - } - - if ( - currentPolicy.data.minSpecial != null && - currentPolicy.data.minSpecial > enforcedOptions.specialCount - ) { - enforcedOptions.specialCount = currentPolicy.data.minSpecial; - } - - if ( - currentPolicy.data.minNumberWords != null && - currentPolicy.data.minNumberWords > enforcedOptions.minNumberWords - ) { - enforcedOptions.minNumberWords = currentPolicy.data.minNumberWords; - } - - if (currentPolicy.data.capitalize) { - enforcedOptions.capitalize = true; - } - - if (currentPolicy.data.includeNumber) { - enforcedOptions.includeNumber = true; - } - }); - - return enforcedOptions; - } - - async saveOptions(options: PasswordGeneratorOptions) { - await this.stateService.setPasswordGenerationOptions(options); - } - - async getHistory(): Promise { - const hasKey = await this.cryptoService.hasUserKey(); - if (!hasKey) { - return new Array(); - } - - if ((await this.stateService.getDecryptedPasswordGenerationHistory()) == null) { - const encrypted = await this.stateService.getEncryptedPasswordGenerationHistory(); - const decrypted = await this.decryptHistory(encrypted); - await this.stateService.setDecryptedPasswordGenerationHistory(decrypted); - } - - const passwordGenerationHistory = - await this.stateService.getDecryptedPasswordGenerationHistory(); - return passwordGenerationHistory != null - ? passwordGenerationHistory - : new Array(); - } - - async addHistory(password: string): Promise { - // Cannot add new history if no key is available - const hasKey = await this.cryptoService.hasUserKey(); - if (!hasKey) { - return; - } - - const currentHistory = await this.getHistory(); - - // Prevent duplicates - if (this.matchesPrevious(password, currentHistory)) { - return; - } - - currentHistory.unshift(new GeneratedPasswordHistory(password, Date.now())); - - // Remove old items. - if (currentHistory.length > MaxPasswordsInHistory) { - currentHistory.pop(); - } - - const newHistory = await this.encryptHistory(currentHistory); - await this.stateService.setDecryptedPasswordGenerationHistory(currentHistory); - return await this.stateService.setEncryptedPasswordGenerationHistory(newHistory); - } - - async clear(userId?: string): Promise { - await this.stateService.setEncryptedPasswordGenerationHistory(null, { userId: userId }); - await this.stateService.setDecryptedPasswordGenerationHistory(null, { userId: userId }); - return []; - } - - private capitalize(str: string) { - return str.charAt(0).toUpperCase() + str.slice(1); - } - - private async appendRandomNumberToRandomWord(wordList: string[]) { - if (wordList == null || wordList.length <= 0) { - return; - } - const index = await this.cryptoService.randomNumber(0, wordList.length - 1); - const num = await this.cryptoService.randomNumber(0, 9); - wordList[index] = wordList[index] + num; - } - - private async encryptHistory( - history: GeneratedPasswordHistory[], - ): Promise { - if (history == null || history.length === 0) { - return Promise.resolve([]); - } - - const promises = history.map(async (item) => { - const encrypted = await this.cryptoService.encrypt(item.password); - return new GeneratedPasswordHistory(encrypted.encryptedString, item.date); - }); - - return await Promise.all(promises); - } - - private async decryptHistory( - history: GeneratedPasswordHistory[], - ): Promise { - if (history == null || history.length === 0) { - return Promise.resolve([]); - } - - const promises = history.map(async (item) => { - const decrypted = await this.cryptoService.decryptToUtf8(new EncString(item.password)); - return new GeneratedPasswordHistory(decrypted, item.date); - }); - - return await Promise.all(promises); - } - - private matchesPrevious(password: string, history: GeneratedPasswordHistory[]): boolean { - if (history == null || history.length === 0) { - return false; - } - - return history[history.length - 1].password === password; - } - - // ref: https://stackoverflow.com/a/12646864/1090359 - private async shuffleArray(array: string[]) { - for (let i = array.length - 1; i > 0; i--) { - const j = await this.cryptoService.randomNumber(0, i); - [array[i], array[j]] = [array[j], array[i]]; - } - } -} diff --git a/libs/common/src/tools/generator/password/password-generator-strategy.spec.ts b/libs/common/src/tools/generator/password/password-generator-strategy.spec.ts index a7509e8b43..668dd818e2 100644 --- a/libs/common/src/tools/generator/password/password-generator-strategy.spec.ts +++ b/libs/common/src/tools/generator/password/password-generator-strategy.spec.ts @@ -12,13 +12,13 @@ import { PolicyType } from "../../../admin-console/enums"; import { Policy } from "../../../admin-console/models/domain/policy"; import { StateProvider } from "../../../platform/state"; import { UserId } from "../../../types/guid"; +import { Randomizer } from "../abstractions/randomizer"; import { PASSWORD_SETTINGS } from "../key-definitions"; import { DisabledPasswordGeneratorPolicy } from "./password-generator-policy"; import { DefaultPasswordGenerationOptions, - PasswordGenerationServiceAbstraction, PasswordGeneratorOptionsEvaluator, PasswordGeneratorStrategy, } from "."; @@ -74,8 +74,8 @@ describe("Password generation strategy", () => { describe("durableState", () => { it("should use password settings key", () => { const provider = mock(); - const legacy = mock(); - const strategy = new PasswordGeneratorStrategy(legacy, provider); + const randomizer = mock(); + const strategy = new PasswordGeneratorStrategy(randomizer, provider); strategy.durableState(SomeUser); @@ -95,40 +95,14 @@ describe("Password generation strategy", () => { describe("policy", () => { it("should use password generator policy", () => { - const legacy = mock(); - const strategy = new PasswordGeneratorStrategy(legacy, null); + const randomizer = mock(); + const strategy = new PasswordGeneratorStrategy(randomizer, null); expect(strategy.policy).toBe(PolicyType.PasswordGenerator); }); }); describe("generate()", () => { - it("should call the legacy service with the given options", async () => { - const legacy = mock(); - const strategy = new PasswordGeneratorStrategy(legacy, null); - const options = { - type: "password", - minLength: 1, - useUppercase: true, - useLowercase: true, - useNumbers: true, - numberCount: 1, - useSpecial: true, - specialCount: 1, - }; - - await strategy.generate(options); - - expect(legacy.generatePassword).toHaveBeenCalledWith(options); - }); - - it("should set the generation type to password", async () => { - const legacy = mock(); - const strategy = new PasswordGeneratorStrategy(legacy, null); - - await strategy.generate({ type: "foo" } as any); - - expect(legacy.generatePassword).toHaveBeenCalledWith({ type: "password" }); - }); + it.todo("should generate a password using the given options"); }); }); diff --git a/libs/common/src/tools/generator/password/password-generator-strategy.ts b/libs/common/src/tools/generator/password/password-generator-strategy.ts index 23828d7b59..075c331e06 100644 --- a/libs/common/src/tools/generator/password/password-generator-strategy.ts +++ b/libs/common/src/tools/generator/password/password-generator-strategy.ts @@ -1,25 +1,19 @@ -import { BehaviorSubject, map, pipe } from "rxjs"; - import { GeneratorStrategy } from ".."; import { PolicyType } from "../../../admin-console/enums"; import { StateProvider } from "../../../platform/state"; -import { UserId } from "../../../types/guid"; -import { PasswordGenerationServiceAbstraction } from "../abstractions/password-generation.service.abstraction"; +import { Randomizer } from "../abstractions/randomizer"; import { PASSWORD_SETTINGS } from "../key-definitions"; -import { distinctIfShallowMatch, reduceCollection } from "../rx-operators"; +import { Policies } from "../policies"; +import { mapPolicyToEvaluator } from "../rx-operators"; +import { clone$PerUserId, sharedStateByUserId } from "../util"; import { DefaultPasswordGenerationOptions, PasswordGenerationOptions, } from "./password-generation-options"; -import { PasswordGeneratorOptionsEvaluator } from "./password-generator-options-evaluator"; -import { - DisabledPasswordGeneratorPolicy, - PasswordGeneratorPolicy, - leastPrivilege, -} from "./password-generator-policy"; +import { PasswordGeneratorPolicy } from "./password-generator-policy"; -/** {@link GeneratorStrategy} */ +/** Generates passwords composed of random characters */ export class PasswordGeneratorStrategy implements GeneratorStrategy { @@ -27,36 +21,108 @@ export class PasswordGeneratorStrategy * @param legacy generates the password */ constructor( - private legacy: PasswordGenerationServiceAbstraction, + private randomizer: Randomizer, private stateProvider: StateProvider, ) {} - /** {@link GeneratorStrategy.durableState} */ - durableState(id: UserId) { - return this.stateProvider.getUser(id, PASSWORD_SETTINGS); - } - - /** Gets the default options. */ - defaults$(_: UserId) { - return new BehaviorSubject({ ...DefaultPasswordGenerationOptions }).asObservable(); - } - - /** {@link GeneratorStrategy.policy} */ - get policy() { - return PolicyType.PasswordGenerator; - } - - /** {@link GeneratorStrategy.toEvaluator} */ + // configuration + durableState = sharedStateByUserId(PASSWORD_SETTINGS, this.stateProvider); + defaults$ = clone$PerUserId(DefaultPasswordGenerationOptions); + readonly policy = PolicyType.PasswordGenerator; toEvaluator() { - return pipe( - reduceCollection(leastPrivilege, DisabledPasswordGeneratorPolicy), - distinctIfShallowMatch(), - map((policy) => new PasswordGeneratorOptionsEvaluator(policy)), - ); + return mapPolicyToEvaluator(Policies.Password); } - /** {@link GeneratorStrategy.generate} */ - generate(options: PasswordGenerationOptions): Promise { - return this.legacy.generatePassword({ ...options, type: "password" }); + // algorithm + async generate(options: PasswordGenerationOptions): Promise { + const o = { ...DefaultPasswordGenerationOptions, ...options }; + let positions: string[] = []; + if (o.lowercase && o.minLowercase > 0) { + for (let i = 0; i < o.minLowercase; i++) { + positions.push("l"); + } + } + if (o.uppercase && o.minUppercase > 0) { + for (let i = 0; i < o.minUppercase; i++) { + positions.push("u"); + } + } + if (o.number && o.minNumber > 0) { + for (let i = 0; i < o.minNumber; i++) { + positions.push("n"); + } + } + if (o.special && o.minSpecial > 0) { + for (let i = 0; i < o.minSpecial; i++) { + positions.push("s"); + } + } + while (positions.length < o.length) { + positions.push("a"); + } + + // shuffle + positions = await this.randomizer.shuffle(positions); + + // build out the char sets + let allCharSet = ""; + + let lowercaseCharSet = "abcdefghijkmnopqrstuvwxyz"; + if (o.ambiguous) { + lowercaseCharSet += "l"; + } + if (o.lowercase) { + allCharSet += lowercaseCharSet; + } + + let uppercaseCharSet = "ABCDEFGHJKLMNPQRSTUVWXYZ"; + if (o.ambiguous) { + uppercaseCharSet += "IO"; + } + if (o.uppercase) { + allCharSet += uppercaseCharSet; + } + + let numberCharSet = "23456789"; + if (o.ambiguous) { + numberCharSet += "01"; + } + if (o.number) { + allCharSet += numberCharSet; + } + + const specialCharSet = "!@#$%^&*"; + if (o.special) { + allCharSet += specialCharSet; + } + + let password = ""; + for (let i = 0; i < o.length; i++) { + let positionChars: string; + switch (positions[i]) { + case "l": + positionChars = lowercaseCharSet; + break; + case "u": + positionChars = uppercaseCharSet; + break; + case "n": + positionChars = numberCharSet; + break; + case "s": + positionChars = specialCharSet; + break; + case "a": + positionChars = allCharSet; + break; + default: + break; + } + + const randomCharIndex = await this.randomizer.uniform(0, positionChars.length - 1); + password += positionChars.charAt(randomCharIndex); + } + + return password; } } diff --git a/libs/common/src/tools/generator/policies.ts b/libs/common/src/tools/generator/policies.ts new file mode 100644 index 0000000000..27521f0eeb --- /dev/null +++ b/libs/common/src/tools/generator/policies.ts @@ -0,0 +1,48 @@ +import { Policy as AdminPolicy } from "@bitwarden/common/admin-console/models/domain/policy"; + +import { PassphraseGeneratorOptionsEvaluator, PassphraseGeneratorPolicy } from "./passphrase"; +import { + DisabledPassphraseGeneratorPolicy, + leastPrivilege as passphraseLeastPrivilege, +} from "./passphrase/passphrase-generator-policy"; +import { PasswordGeneratorOptionsEvaluator, PasswordGeneratorPolicy } from "./password"; +import { + DisabledPasswordGeneratorPolicy, + leastPrivilege as passwordLeastPrivilege, +} from "./password/password-generator-policy"; + +/** Determines how to construct a password generator policy */ +export type PolicyConfiguration = { + /** The value of the policy when it is not in effect. */ + disabledValue: Policy; + + /** Combines multiple policies set by the administrative console into + * a single policy. + */ + combine: (acc: Policy, policy: AdminPolicy) => Policy; + + /** Converts policy service data into an actionable policy. + */ + createEvaluator: (policy: Policy) => Evaluator; +}; + +const PASSPHRASE = Object.freeze({ + disabledValue: DisabledPassphraseGeneratorPolicy, + combine: passphraseLeastPrivilege, + createEvaluator: (policy) => new PassphraseGeneratorOptionsEvaluator(policy), +} as PolicyConfiguration); + +const PASSWORD = Object.freeze({ + disabledValue: DisabledPasswordGeneratorPolicy, + combine: passwordLeastPrivilege, + createEvaluator: (policy) => new PasswordGeneratorOptionsEvaluator(policy), +} as PolicyConfiguration); + +/** Policy configurations */ +export const Policies = Object.freeze({ + /** Passphrase policy configuration */ + Passphrase: PASSPHRASE, + + /** Passphrase policy configuration */ + Password: PASSWORD, +}); diff --git a/libs/common/src/tools/generator/random.ts b/libs/common/src/tools/generator/random.ts new file mode 100644 index 0000000000..255a5df7a7 --- /dev/null +++ b/libs/common/src/tools/generator/random.ts @@ -0,0 +1,62 @@ +import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service"; + +import { Randomizer } from "./abstractions/randomizer"; +import { WordOptions } from "./word-options"; + +/** A randomizer backed by a CryptoService. */ +export class CryptoServiceRandomizer implements Randomizer { + constructor(private crypto: CryptoService) {} + + async pick(list: Array) { + const index = await this.uniform(0, list.length - 1); + return list[index]; + } + + async pickWord(list: Array, options?: WordOptions) { + let word = await this.pick(list); + + if (options?.titleCase ?? false) { + word = word.charAt(0).toUpperCase() + word.slice(1); + } + + if (options?.number ?? false) { + const num = await this.crypto.randomNumber(1, 9999); + word = word + this.zeroPad(num.toString(), 4); + } + + return word; + } + + // ref: https://stackoverflow.com/a/12646864/1090359 + async shuffle(items: Array, options?: { copy?: boolean }) { + const shuffled = options?.copy ?? true ? [...items] : items; + + for (let i = items.length - 1; i > 0; i--) { + const j = await this.uniform(0, i); + [items[i], items[j]] = [items[j], items[i]]; + } + + return shuffled; + } + + async chars(length: number) { + let str = ""; + const charSet = "abcdefghijklmnopqrstuvwxyz1234567890"; + for (let i = 0; i < length; i++) { + const randomCharIndex = await this.uniform(0, charSet.length - 1); + str += charSet.charAt(randomCharIndex); + } + return str; + } + + async uniform(min: number, max: number) { + return this.crypto.randomNumber(min, max); + } + + // ref: https://stackoverflow.com/a/10073788 + private zeroPad(number: string, width: number) { + return number.length >= width + ? number + : new Array(width - number.length + 1).join("0") + number; + } +} diff --git a/libs/common/src/tools/generator/rx-operators.ts b/libs/common/src/tools/generator/rx-operators.ts index 6524ef7994..47233fa778 100644 --- a/libs/common/src/tools/generator/rx-operators.ts +++ b/libs/common/src/tools/generator/rx-operators.ts @@ -1,4 +1,7 @@ -import { distinctUntilChanged, map, OperatorFunction } from "rxjs"; +import { distinctUntilChanged, map, OperatorFunction, pipe } from "rxjs"; + +import { DefaultPolicyEvaluator } from "./default-policy-evaluator"; +import { PolicyConfiguration } from "./policies"; /** * An observable operator that reduces an emitted collection to a single object, @@ -36,3 +39,23 @@ export function distinctIfShallowMatch(): OperatorFunction { return isDistinct; }); } + +/** Maps an administrative console policy to a policy evaluator using the provided configuration. + * @param configuration the configuration that constructs the evaluator. + */ +export function mapPolicyToEvaluator( + configuration: PolicyConfiguration, +) { + return pipe( + reduceCollection(configuration.combine, configuration.disabledValue), + distinctIfShallowMatch(), + map(configuration.createEvaluator), + ); +} + +/** Constructs a method that maps a policy to the default (no-op) policy. */ +export function newDefaultEvaluator() { + return () => { + return pipe(map((_) => new DefaultPolicyEvaluator())); + }; +} diff --git a/libs/common/src/tools/generator/username/catchall-generator-strategy.spec.ts b/libs/common/src/tools/generator/username/catchall-generator-strategy.spec.ts index 30f11b1e89..45e8716081 100644 --- a/libs/common/src/tools/generator/username/catchall-generator-strategy.spec.ts +++ b/libs/common/src/tools/generator/username/catchall-generator-strategy.spec.ts @@ -7,12 +7,13 @@ import { PolicyType } from "../../../admin-console/enums"; import { Policy } from "../../../admin-console/models/domain/policy"; import { StateProvider } from "../../../platform/state"; import { UserId } from "../../../types/guid"; +import { Randomizer } from "../abstractions/randomizer"; import { DefaultPolicyEvaluator } from "../default-policy-evaluator"; import { CATCHALL_SETTINGS } from "../key-definitions"; -import { CatchallGenerationOptions, DefaultCatchallOptions } from "./catchall-generator-options"; +import { DefaultCatchallOptions } from "./catchall-generator-options"; -import { CatchallGeneratorStrategy, UsernameGenerationServiceAbstraction } from "."; +import { CatchallGeneratorStrategy } from "."; const SomeUser = "some user" as UserId; const SomePolicy = mock({ @@ -40,8 +41,8 @@ describe("Email subaddress list generation strategy", () => { describe("durableState", () => { it("should use password settings key", () => { const provider = mock(); - const legacy = mock(); - const strategy = new CatchallGeneratorStrategy(legacy, provider); + const randomizer = mock(); + const strategy = new CatchallGeneratorStrategy(randomizer, provider); strategy.durableState(SomeUser); @@ -61,26 +62,14 @@ describe("Email subaddress list generation strategy", () => { describe("policy", () => { it("should use password generator policy", () => { - const legacy = mock(); - const strategy = new CatchallGeneratorStrategy(legacy, null); + const randomizer = mock(); + const strategy = new CatchallGeneratorStrategy(randomizer, null); expect(strategy.policy).toBe(PolicyType.PasswordGenerator); }); }); describe("generate()", () => { - it("should call the legacy service with the given options", async () => { - const legacy = mock(); - const strategy = new CatchallGeneratorStrategy(legacy, null); - const options = { - catchallType: "website-name", - catchallDomain: "example.com", - website: "foo.com", - } as CatchallGenerationOptions; - - await strategy.generate(options); - - expect(legacy.generateCatchall).toHaveBeenCalledWith(options); - }); + it.todo("generate catchall email addresses"); }); }); diff --git a/libs/common/src/tools/generator/username/catchall-generator-strategy.ts b/libs/common/src/tools/generator/username/catchall-generator-strategy.ts index ee86fd9fd6..fb015a596f 100644 --- a/libs/common/src/tools/generator/username/catchall-generator-strategy.ts +++ b/libs/common/src/tools/generator/username/catchall-generator-strategy.ts @@ -1,13 +1,11 @@ -import { BehaviorSubject, map, pipe } from "rxjs"; - import { PolicyType } from "../../../admin-console/enums"; import { StateProvider } from "../../../platform/state"; -import { UserId } from "../../../types/guid"; import { GeneratorStrategy } from "../abstractions"; -import { UsernameGenerationServiceAbstraction } from "../abstractions/username-generation.service.abstraction"; -import { DefaultPolicyEvaluator } from "../default-policy-evaluator"; +import { Randomizer } from "../abstractions/randomizer"; import { CATCHALL_SETTINGS } from "../key-definitions"; import { NoPolicy } from "../no-policy"; +import { newDefaultEvaluator } from "../rx-operators"; +import { clone$PerUserId, sharedStateByUserId } from "../util"; import { CatchallGenerationOptions, DefaultCatchallOptions } from "./catchall-generator-options"; @@ -19,34 +17,34 @@ export class CatchallGeneratorStrategy * @param usernameService generates a catchall address for a domain */ constructor( - private usernameService: UsernameGenerationServiceAbstraction, + private random: Randomizer, private stateProvider: StateProvider, + private defaultOptions: CatchallGenerationOptions = DefaultCatchallOptions, ) {} - /** {@link GeneratorStrategy.durableState} */ - durableState(id: UserId) { - return this.stateProvider.getUser(id, CATCHALL_SETTINGS); - } + // configuration + durableState = sharedStateByUserId(CATCHALL_SETTINGS, this.stateProvider); + defaults$ = clone$PerUserId(this.defaultOptions); + toEvaluator = newDefaultEvaluator(); + readonly policy = PolicyType.PasswordGenerator; - /** {@link GeneratorStrategy.defaults$} */ - defaults$(userId: UserId) { - return new BehaviorSubject({ ...DefaultCatchallOptions }).asObservable(); - } + // algorithm + async generate(options: CatchallGenerationOptions) { + const o = Object.assign({}, DefaultCatchallOptions, options); - /** {@link GeneratorStrategy.policy} */ - get policy() { - // Uses password generator since there aren't policies - // specific to usernames. - return PolicyType.PasswordGenerator; - } + if (o.catchallDomain == null || o.catchallDomain === "") { + return null; + } + if (o.catchallType == null) { + o.catchallType = "random"; + } - /** {@link GeneratorStrategy.toEvaluator} */ - toEvaluator() { - return pipe(map((_) => new DefaultPolicyEvaluator())); - } - - /** {@link GeneratorStrategy.generate} */ - generate(options: CatchallGenerationOptions) { - return this.usernameService.generateCatchall(options); + let startString = ""; + if (o.catchallType === "random") { + startString = await this.random.chars(8); + } else if (o.catchallType === "website-name") { + startString = o.website; + } + return startString + "@" + o.catchallDomain; } } diff --git a/libs/common/src/tools/generator/username/eff-username-generator-strategy.spec.ts b/libs/common/src/tools/generator/username/eff-username-generator-strategy.spec.ts index 76e51f609c..128b69e673 100644 --- a/libs/common/src/tools/generator/username/eff-username-generator-strategy.spec.ts +++ b/libs/common/src/tools/generator/username/eff-username-generator-strategy.spec.ts @@ -7,12 +7,13 @@ import { PolicyType } from "../../../admin-console/enums"; import { Policy } from "../../../admin-console/models/domain/policy"; import { StateProvider } from "../../../platform/state"; import { UserId } from "../../../types/guid"; +import { Randomizer } from "../abstractions/randomizer"; import { DefaultPolicyEvaluator } from "../default-policy-evaluator"; import { EFF_USERNAME_SETTINGS } from "../key-definitions"; import { DefaultEffUsernameOptions } from "./eff-username-generator-options"; -import { EffUsernameGeneratorStrategy, UsernameGenerationServiceAbstraction } from "."; +import { EffUsernameGeneratorStrategy } from "."; const SomeUser = "some user" as UserId; const SomePolicy = mock({ @@ -40,8 +41,8 @@ describe("EFF long word list generation strategy", () => { describe("durableState", () => { it("should use password settings key", () => { const provider = mock(); - const legacy = mock(); - const strategy = new EffUsernameGeneratorStrategy(legacy, provider); + const randomizer = mock(); + const strategy = new EffUsernameGeneratorStrategy(randomizer, provider); strategy.durableState(SomeUser); @@ -61,26 +62,14 @@ describe("EFF long word list generation strategy", () => { describe("policy", () => { it("should use password generator policy", () => { - const legacy = mock(); - const strategy = new EffUsernameGeneratorStrategy(legacy, null); + const randomizer = mock(); + const strategy = new EffUsernameGeneratorStrategy(randomizer, null); expect(strategy.policy).toBe(PolicyType.PasswordGenerator); }); }); describe("generate()", () => { - it("should call the legacy service with the given options", async () => { - const legacy = mock(); - const strategy = new EffUsernameGeneratorStrategy(legacy, null); - const options = { - wordCapitalize: false, - wordIncludeNumber: false, - website: null as string, - }; - - await strategy.generate(options); - - expect(legacy.generateWord).toHaveBeenCalledWith(options); - }); + it.todo("generate username tests"); }); }); diff --git a/libs/common/src/tools/generator/username/eff-username-generator-strategy.ts b/libs/common/src/tools/generator/username/eff-username-generator-strategy.ts index 70d1f85420..abd8e6b226 100644 --- a/libs/common/src/tools/generator/username/eff-username-generator-strategy.ts +++ b/libs/common/src/tools/generator/username/eff-username-generator-strategy.ts @@ -1,13 +1,13 @@ -import { BehaviorSubject, map, pipe } from "rxjs"; +import { EFFLongWordList } from "@bitwarden/common/platform/misc/wordlist"; import { PolicyType } from "../../../admin-console/enums"; import { StateProvider } from "../../../platform/state"; -import { UserId } from "../../../types/guid"; import { GeneratorStrategy } from "../abstractions"; -import { UsernameGenerationServiceAbstraction } from "../abstractions/username-generation.service.abstraction"; -import { DefaultPolicyEvaluator } from "../default-policy-evaluator"; +import { Randomizer } from "../abstractions/randomizer"; import { EFF_USERNAME_SETTINGS } from "../key-definitions"; import { NoPolicy } from "../no-policy"; +import { newDefaultEvaluator } from "../rx-operators"; +import { clone$PerUserId, sharedStateByUserId } from "../util"; import { DefaultEffUsernameOptions, @@ -22,34 +22,23 @@ export class EffUsernameGeneratorStrategy * @param usernameService generates a username from EFF word list */ constructor( - private usernameService: UsernameGenerationServiceAbstraction, + private random: Randomizer, private stateProvider: StateProvider, + private defaultOptions: EffUsernameGenerationOptions = DefaultEffUsernameOptions, ) {} - /** {@link GeneratorStrategy.durableState} */ - durableState(id: UserId) { - return this.stateProvider.getUser(id, EFF_USERNAME_SETTINGS); - } + // configuration + durableState = sharedStateByUserId(EFF_USERNAME_SETTINGS, this.stateProvider); + defaults$ = clone$PerUserId(this.defaultOptions); + toEvaluator = newDefaultEvaluator(); + readonly policy = PolicyType.PasswordGenerator; - /** {@link GeneratorStrategy.defaults$} */ - defaults$(userId: UserId) { - return new BehaviorSubject({ ...DefaultEffUsernameOptions }).asObservable(); - } - - /** {@link GeneratorStrategy.policy} */ - get policy() { - // Uses password generator since there aren't policies - // specific to usernames. - return PolicyType.PasswordGenerator; - } - - /** {@link GeneratorStrategy.toEvaluator} */ - toEvaluator() { - return pipe(map((_) => new DefaultPolicyEvaluator())); - } - - /** {@link GeneratorStrategy.generate} */ - generate(options: EffUsernameGenerationOptions) { - return this.usernameService.generateWord(options); + // algorithm + async generate(options: EffUsernameGenerationOptions) { + const word = await this.random.pickWord(EFFLongWordList, { + titleCase: options.wordCapitalize ?? DefaultEffUsernameOptions.wordCapitalize, + number: options.wordIncludeNumber ?? DefaultEffUsernameOptions.wordIncludeNumber, + }); + return word; } } diff --git a/libs/common/src/tools/generator/username/forwarder-generator-strategy.spec.ts b/libs/common/src/tools/generator/username/forwarder-generator-strategy.spec.ts index 7c1b4b9191..e78b432bfb 100644 --- a/libs/common/src/tools/generator/username/forwarder-generator-strategy.spec.ts +++ b/libs/common/src/tools/generator/username/forwarder-generator-strategy.spec.ts @@ -25,7 +25,7 @@ class TestForwarder extends ForwarderGeneratorStrategy { keyService: CryptoService, stateProvider: StateProvider, ) { - super(encryptService, keyService, stateProvider); + super(encryptService, keyService, stateProvider, { website: null, token: "" }); } get key() { diff --git a/libs/common/src/tools/generator/username/forwarder-generator-strategy.ts b/libs/common/src/tools/generator/username/forwarder-generator-strategy.ts index 28ebcba4fd..4655a3fb72 100644 --- a/libs/common/src/tools/generator/username/forwarder-generator-strategy.ts +++ b/libs/common/src/tools/generator/username/forwarder-generator-strategy.ts @@ -1,4 +1,4 @@ -import { Observable, map, pipe } from "rxjs"; +import { map } from "rxjs"; import { PolicyType } from "../../../admin-console/enums"; import { CryptoService } from "../../../platform/abstractions/crypto.service"; @@ -13,8 +13,9 @@ import { SecretKeyDefinition } from "../../state/secret-key-definition"; import { SecretState } from "../../state/secret-state"; import { UserKeyEncryptor } from "../../state/user-key-encryptor"; import { GeneratorStrategy } from "../abstractions"; -import { DefaultPolicyEvaluator } from "../default-policy-evaluator"; import { NoPolicy } from "../no-policy"; +import { newDefaultEvaluator } from "../rx-operators"; +import { clone$PerUserId, sharedByUserId } from "../util"; import { ApiOptions } from "./options/forwarder-options"; @@ -33,29 +34,25 @@ export abstract class ForwarderGeneratorStrategy< private readonly encryptService: EncryptService, private readonly keyService: CryptoService, private stateProvider: StateProvider, + private readonly defaultOptions: Options, ) { super(); - // Uses password generator since there aren't policies - // specific to usernames. - this.policy = PolicyType.PasswordGenerator; } - private durableStates = new Map>(); + /** configures forwarder secret storage */ + protected abstract readonly key: UserKeyDefinition; - /** {@link GeneratorStrategy.durableState} */ - durableState = (userId: UserId) => { - let state = this.durableStates.get(userId); + /** configures forwarder import buffer */ + protected abstract readonly rolloverKey: BufferedKeyDefinition; - if (!state) { - state = this.createState(userId); + // configuration + readonly policy = PolicyType.PasswordGenerator; + defaults$ = clone$PerUserId(this.defaultOptions); + toEvaluator = newDefaultEvaluator(); + durableState = sharedByUserId((userId) => this.getUserSecrets(userId)); - this.durableStates.set(userId, state); - } - - return state; - }; - - private createState(userId: UserId): SingleUserState { + // per-user encrypted state + private getUserSecrets(userId: UserId): SingleUserState { // construct the encryptor const packer = new PaddedDataPacker(OPTIONS_FRAME_SIZE); const encryptor = new UserKeyEncryptor(this.encryptService, this.keyService, packer); @@ -92,18 +89,4 @@ export abstract class ForwarderGeneratorStrategy< return rolloverState; } - - /** Gets the default options. */ - abstract defaults$: (userId: UserId) => Observable; - - /** Determine where forwarder configuration is stored */ - protected abstract readonly key: UserKeyDefinition; - - /** Determine where forwarder rollover configuration is stored */ - protected abstract readonly rolloverKey: BufferedKeyDefinition; - - /** {@link GeneratorStrategy.toEvaluator} */ - toEvaluator = () => { - return pipe(map((_) => new DefaultPolicyEvaluator())); - }; } diff --git a/libs/common/src/tools/generator/username/forwarders/addy-io.ts b/libs/common/src/tools/generator/username/forwarders/addy-io.ts index 1212174951..ecf60da195 100644 --- a/libs/common/src/tools/generator/username/forwarders/addy-io.ts +++ b/libs/common/src/tools/generator/username/forwarders/addy-io.ts @@ -1,11 +1,8 @@ -import { BehaviorSubject } from "rxjs"; - import { ApiService } from "../../../../abstractions/api.service"; import { CryptoService } from "../../../../platform/abstractions/crypto.service"; import { EncryptService } from "../../../../platform/abstractions/encrypt.service"; import { I18nService } from "../../../../platform/abstractions/i18n.service"; import { StateProvider } from "../../../../platform/state"; -import { UserId } from "../../../../types/guid"; import { ADDY_IO_FORWARDER, ADDY_IO_BUFFER } from "../../key-definitions"; import { ForwarderGeneratorStrategy } from "../forwarder-generator-strategy"; import { Forwarders } from "../options/constants"; @@ -36,25 +33,14 @@ export class AddyIoForwarder extends ForwarderGeneratorStrategy< keyService: CryptoService, stateProvider: StateProvider, ) { - super(encryptService, keyService, stateProvider); + super(encryptService, keyService, stateProvider, DefaultAddyIoOptions); } - /** {@link ForwarderGeneratorStrategy.key} */ - get key() { - return ADDY_IO_FORWARDER; - } + // configuration + readonly key = ADDY_IO_FORWARDER; + readonly rolloverKey = ADDY_IO_BUFFER; - /** {@link ForwarderGeneratorStrategy.rolloverKey} */ - get rolloverKey() { - return ADDY_IO_BUFFER; - } - - /** {@link ForwarderGeneratorStrategy.defaults$} */ - defaults$ = (userId: UserId) => { - return new BehaviorSubject({ ...DefaultAddyIoOptions }); - }; - - /** {@link ForwarderGeneratorStrategy.generate} */ + // request generate = async (options: SelfHostedApiOptions & EmailDomainOptions) => { if (!options.token || options.token === "") { const error = this.i18nService.t("forwaderInvalidToken", Forwarders.AddyIo.name); diff --git a/libs/common/src/tools/generator/username/forwarders/duck-duck-go.ts b/libs/common/src/tools/generator/username/forwarders/duck-duck-go.ts index 4a9040d74a..492105dfdf 100644 --- a/libs/common/src/tools/generator/username/forwarders/duck-duck-go.ts +++ b/libs/common/src/tools/generator/username/forwarders/duck-duck-go.ts @@ -1,11 +1,8 @@ -import { BehaviorSubject } from "rxjs"; - import { ApiService } from "../../../../abstractions/api.service"; import { CryptoService } from "../../../../platform/abstractions/crypto.service"; import { EncryptService } from "../../../../platform/abstractions/encrypt.service"; import { I18nService } from "../../../../platform/abstractions/i18n.service"; import { StateProvider } from "../../../../platform/state"; -import { UserId } from "../../../../types/guid"; import { DUCK_DUCK_GO_FORWARDER, DUCK_DUCK_GO_BUFFER } from "../../key-definitions"; import { ForwarderGeneratorStrategy } from "../forwarder-generator-strategy"; import { Forwarders } from "../options/constants"; @@ -32,25 +29,14 @@ export class DuckDuckGoForwarder extends ForwarderGeneratorStrategy keyService: CryptoService, stateProvider: StateProvider, ) { - super(encryptService, keyService, stateProvider); + super(encryptService, keyService, stateProvider, DefaultDuckDuckGoOptions); } - /** {@link ForwarderGeneratorStrategy.key} */ - get key() { - return DUCK_DUCK_GO_FORWARDER; - } + // configuration + readonly key = DUCK_DUCK_GO_FORWARDER; + readonly rolloverKey = DUCK_DUCK_GO_BUFFER; - /** {@link ForwarderGeneratorStrategy.rolloverKey} */ - get rolloverKey() { - return DUCK_DUCK_GO_BUFFER; - } - - /** {@link ForwarderGeneratorStrategy.defaults$} */ - defaults$ = (userId: UserId) => { - return new BehaviorSubject({ ...DefaultDuckDuckGoOptions }); - }; - - /** {@link ForwarderGeneratorStrategy.generate} */ + // request generate = async (options: ApiOptions): Promise => { if (!options.token || options.token === "") { const error = this.i18nService.t("forwaderInvalidToken", Forwarders.DuckDuckGo.name); diff --git a/libs/common/src/tools/generator/username/forwarders/fastmail.ts b/libs/common/src/tools/generator/username/forwarders/fastmail.ts index 0236e658fb..0c4e0e2cfd 100644 --- a/libs/common/src/tools/generator/username/forwarders/fastmail.ts +++ b/libs/common/src/tools/generator/username/forwarders/fastmail.ts @@ -1,11 +1,8 @@ -import { BehaviorSubject } from "rxjs"; - import { ApiService } from "../../../../abstractions/api.service"; import { CryptoService } from "../../../../platform/abstractions/crypto.service"; import { EncryptService } from "../../../../platform/abstractions/encrypt.service"; import { I18nService } from "../../../../platform/abstractions/i18n.service"; import { StateProvider } from "../../../../platform/state"; -import { UserId } from "../../../../types/guid"; import { FASTMAIL_FORWARDER, FASTMAIL_BUFFER } from "../../key-definitions"; import { ForwarderGeneratorStrategy } from "../forwarder-generator-strategy"; import { Forwarders } from "../options/constants"; @@ -34,25 +31,14 @@ export class FastmailForwarder extends ForwarderGeneratorStrategy { - return new BehaviorSubject({ ...DefaultFastmailOptions }); - }; - - /** {@link ForwarderGeneratorStrategy.rolloverKey} */ - get rolloverKey() { - return FASTMAIL_BUFFER; - } - - /** {@link ForwarderGeneratorStrategy.generate} */ + // request generate = async (options: ApiOptions & EmailPrefixOptions) => { if (!options.token || options.token === "") { const error = this.i18nService.t("forwaderInvalidToken", Forwarders.Fastmail.name); @@ -76,7 +62,7 @@ export class FastmailForwarder extends ForwarderGeneratorStrategy { - return new BehaviorSubject({ ...DefaultFirefoxRelayOptions }); - }; - - /** {@link ForwarderGeneratorStrategy.generate} */ + // request generate = async (options: ApiOptions) => { if (!options.token || options.token === "") { const error = this.i18nService.t("forwaderInvalidToken", Forwarders.FirefoxRelay.name); diff --git a/libs/common/src/tools/generator/username/forwarders/forward-email.ts b/libs/common/src/tools/generator/username/forwarders/forward-email.ts index af654d3917..20dfe01291 100644 --- a/libs/common/src/tools/generator/username/forwarders/forward-email.ts +++ b/libs/common/src/tools/generator/username/forwarders/forward-email.ts @@ -1,12 +1,9 @@ -import { BehaviorSubject } from "rxjs"; - import { ApiService } from "../../../../abstractions/api.service"; import { CryptoService } from "../../../../platform/abstractions/crypto.service"; import { EncryptService } from "../../../../platform/abstractions/encrypt.service"; import { I18nService } from "../../../../platform/abstractions/i18n.service"; import { Utils } from "../../../../platform/misc/utils"; import { StateProvider } from "../../../../platform/state"; -import { UserId } from "../../../../types/guid"; import { FORWARD_EMAIL_FORWARDER, FORWARD_EMAIL_BUFFER } from "../../key-definitions"; import { ForwarderGeneratorStrategy } from "../forwarder-generator-strategy"; import { Forwarders } from "../options/constants"; @@ -36,25 +33,14 @@ export class ForwardEmailForwarder extends ForwarderGeneratorStrategy< keyService: CryptoService, stateProvider: StateProvider, ) { - super(encryptService, keyService, stateProvider); + super(encryptService, keyService, stateProvider, DefaultForwardEmailOptions); } - /** {@link ForwarderGeneratorStrategy.key} */ - get key() { - return FORWARD_EMAIL_FORWARDER; - } + // configuration + readonly key = FORWARD_EMAIL_FORWARDER; + readonly rolloverKey = FORWARD_EMAIL_BUFFER; - /** {@link ForwarderGeneratorStrategy.defaults$} */ - defaults$ = (userId: UserId) => { - return new BehaviorSubject({ ...DefaultForwardEmailOptions }); - }; - - /** {@link ForwarderGeneratorStrategy.rolloverKey} */ - get rolloverKey() { - return FORWARD_EMAIL_BUFFER; - } - - /** {@link ForwarderGeneratorStrategy.generate} */ + // request generate = async (options: ApiOptions & EmailDomainOptions) => { if (!options.token || options.token === "") { const error = this.i18nService.t("forwaderInvalidToken", Forwarders.ForwardEmail.name); diff --git a/libs/common/src/tools/generator/username/forwarders/simple-login.ts b/libs/common/src/tools/generator/username/forwarders/simple-login.ts index ee91a41145..593c734641 100644 --- a/libs/common/src/tools/generator/username/forwarders/simple-login.ts +++ b/libs/common/src/tools/generator/username/forwarders/simple-login.ts @@ -1,11 +1,8 @@ -import { BehaviorSubject } from "rxjs"; - import { ApiService } from "../../../../abstractions/api.service"; import { CryptoService } from "../../../../platform/abstractions/crypto.service"; import { EncryptService } from "../../../../platform/abstractions/encrypt.service"; import { I18nService } from "../../../../platform/abstractions/i18n.service"; import { StateProvider } from "../../../../platform/state"; -import { UserId } from "../../../../types/guid"; import { SIMPLE_LOGIN_FORWARDER, SIMPLE_LOGIN_BUFFER } from "../../key-definitions"; import { ForwarderGeneratorStrategy } from "../forwarder-generator-strategy"; import { Forwarders } from "../options/constants"; @@ -33,25 +30,14 @@ export class SimpleLoginForwarder extends ForwarderGeneratorStrategy { - return new BehaviorSubject({ ...DefaultSimpleLoginOptions }); - }; - - /** {@link ForwarderGeneratorStrategy.generate} */ + // request generate = async (options: SelfHostedApiOptions) => { if (!options.token || options.token === "") { const error = this.i18nService.t("forwaderInvalidToken", Forwarders.SimpleLogin.name); diff --git a/libs/common/src/tools/generator/username/index.ts b/libs/common/src/tools/generator/username/index.ts index 7c5ec45f74..a9d8e67608 100644 --- a/libs/common/src/tools/generator/username/index.ts +++ b/libs/common/src/tools/generator/username/index.ts @@ -3,4 +3,3 @@ export { CatchallGeneratorStrategy } from "./catchall-generator-strategy"; export { SubaddressGeneratorStrategy } from "./subaddress-generator-strategy"; export { UsernameGeneratorOptions } from "./username-generation-options"; export { UsernameGenerationServiceAbstraction } from "../abstractions/username-generation.service.abstraction"; -export { UsernameGenerationService } from "./username-generation.service"; diff --git a/libs/common/src/tools/generator/username/subaddress-generator-strategy.spec.ts b/libs/common/src/tools/generator/username/subaddress-generator-strategy.spec.ts index b5ac9c4cf9..ba1d5aa2b8 100644 --- a/libs/common/src/tools/generator/username/subaddress-generator-strategy.spec.ts +++ b/libs/common/src/tools/generator/username/subaddress-generator-strategy.spec.ts @@ -7,15 +7,13 @@ import { PolicyType } from "../../../admin-console/enums"; import { Policy } from "../../../admin-console/models/domain/policy"; import { StateProvider } from "../../../platform/state"; import { UserId } from "../../../types/guid"; +import { Randomizer } from "../abstractions/randomizer"; import { DefaultPolicyEvaluator } from "../default-policy-evaluator"; import { SUBADDRESS_SETTINGS } from "../key-definitions"; -import { - DefaultSubaddressOptions, - SubaddressGenerationOptions, -} from "./subaddress-generator-options"; +import { DefaultSubaddressOptions } from "./subaddress-generator-options"; -import { SubaddressGeneratorStrategy, UsernameGenerationServiceAbstraction } from "."; +import { SubaddressGeneratorStrategy } from "."; const SomeUser = "some user" as UserId; const SomePolicy = mock({ @@ -43,8 +41,8 @@ describe("Email subaddress list generation strategy", () => { describe("durableState", () => { it("should use password settings key", () => { const provider = mock(); - const legacy = mock(); - const strategy = new SubaddressGeneratorStrategy(legacy, provider); + const randomizer = mock(); + const strategy = new SubaddressGeneratorStrategy(randomizer, provider); strategy.durableState(SomeUser); @@ -64,26 +62,14 @@ describe("Email subaddress list generation strategy", () => { describe("policy", () => { it("should use password generator policy", () => { - const legacy = mock(); - const strategy = new SubaddressGeneratorStrategy(legacy, null); + const randomizer = mock(); + const strategy = new SubaddressGeneratorStrategy(randomizer, null); expect(strategy.policy).toBe(PolicyType.PasswordGenerator); }); }); describe("generate()", () => { - it("should call the legacy service with the given options", async () => { - const legacy = mock(); - const strategy = new SubaddressGeneratorStrategy(legacy, null); - const options = { - subaddressType: "website-name", - subaddressEmail: "someone@example.com", - website: "foo.com", - } as SubaddressGenerationOptions; - - await strategy.generate(options); - - expect(legacy.generateSubaddress).toHaveBeenCalledWith(options); - }); + it.todo("generate email subaddress tests"); }); }); diff --git a/libs/common/src/tools/generator/username/subaddress-generator-strategy.ts b/libs/common/src/tools/generator/username/subaddress-generator-strategy.ts index 6106d6d476..e44735c213 100644 --- a/libs/common/src/tools/generator/username/subaddress-generator-strategy.ts +++ b/libs/common/src/tools/generator/username/subaddress-generator-strategy.ts @@ -1,13 +1,11 @@ -import { BehaviorSubject, map, pipe } from "rxjs"; - import { PolicyType } from "../../../admin-console/enums"; import { StateProvider } from "../../../platform/state"; -import { UserId } from "../../../types/guid"; import { GeneratorStrategy } from "../abstractions"; -import { UsernameGenerationServiceAbstraction } from "../abstractions/username-generation.service.abstraction"; -import { DefaultPolicyEvaluator } from "../default-policy-evaluator"; +import { Randomizer } from "../abstractions/randomizer"; import { SUBADDRESS_SETTINGS } from "../key-definitions"; import { NoPolicy } from "../no-policy"; +import { newDefaultEvaluator } from "../rx-operators"; +import { clone$PerUserId, sharedStateByUserId } from "../util"; import { DefaultSubaddressOptions, @@ -26,34 +24,42 @@ export class SubaddressGeneratorStrategy * @param usernameService generates an email subaddress from an email address */ constructor( - private usernameService: UsernameGenerationServiceAbstraction, + private random: Randomizer, private stateProvider: StateProvider, + private defaultOptions: SubaddressGenerationOptions = DefaultSubaddressOptions, ) {} - /** {@link GeneratorStrategy.durableState} */ - durableState(id: UserId) { - return this.stateProvider.getUser(id, SUBADDRESS_SETTINGS); - } + // configuration + durableState = sharedStateByUserId(SUBADDRESS_SETTINGS, this.stateProvider); + defaults$ = clone$PerUserId(this.defaultOptions); + toEvaluator = newDefaultEvaluator(); + readonly policy = PolicyType.PasswordGenerator; - /** {@link GeneratorStrategy.defaults$} */ - defaults$(userId: UserId) { - return new BehaviorSubject({ ...DefaultSubaddressOptions }).asObservable(); - } + // algorithm + async generate(options: SubaddressGenerationOptions) { + const o = Object.assign({}, DefaultSubaddressOptions, options); - /** {@link GeneratorStrategy.policy} */ - get policy() { - // Uses password generator since there aren't policies - // specific to usernames. - return PolicyType.PasswordGenerator; - } + const subaddressEmail = o.subaddressEmail; + if (subaddressEmail == null || subaddressEmail.length < 3) { + return o.subaddressEmail; + } + const atIndex = subaddressEmail.indexOf("@"); + if (atIndex < 1 || atIndex >= subaddressEmail.length - 1) { + return subaddressEmail; + } + if (o.subaddressType == null) { + o.subaddressType = "random"; + } - /** {@link GeneratorStrategy.toEvaluator} */ - toEvaluator() { - return pipe(map((_) => new DefaultPolicyEvaluator())); - } + const emailBeginning = subaddressEmail.substr(0, atIndex); + const emailEnding = subaddressEmail.substr(atIndex + 1, subaddressEmail.length); - /** {@link GeneratorStrategy.generate} */ - generate(options: SubaddressGenerationOptions) { - return this.usernameService.generateSubaddress(options); + let subaddressString = ""; + if (o.subaddressType === "random") { + subaddressString = await this.random.chars(8); + } else if (o.subaddressType === "website-name") { + subaddressString = o.website; + } + return emailBeginning + "+" + subaddressString + "@" + emailEnding; } } diff --git a/libs/common/src/tools/generator/username/username-generation.service.ts b/libs/common/src/tools/generator/username/username-generation.service.ts deleted file mode 100644 index e659aacb51..0000000000 --- a/libs/common/src/tools/generator/username/username-generation.service.ts +++ /dev/null @@ -1,198 +0,0 @@ -import { from } from "rxjs"; - -import { ApiService } from "../../../abstractions/api.service"; -import { CryptoService } from "../../../platform/abstractions/crypto.service"; -import { StateService } from "../../../platform/abstractions/state.service"; -import { EFFLongWordList } from "../../../platform/misc/wordlist"; -import { UsernameGenerationServiceAbstraction } from "../abstractions/username-generation.service.abstraction"; - -import { - AnonAddyForwarder, - DuckDuckGoForwarder, - FastmailForwarder, - FirefoxRelayForwarder, - ForwardEmailForwarder, - Forwarder, - ForwarderOptions, - SimpleLoginForwarder, -} from "./email-forwarders"; -import { UsernameGeneratorOptions } from "./username-generation-options"; - -const DefaultOptions: UsernameGeneratorOptions = { - type: "word", - website: null, - wordCapitalize: true, - wordIncludeNumber: true, - subaddressType: "random", - catchallType: "random", - forwardedService: "", - forwardedAnonAddyDomain: "anonaddy.me", - forwardedAnonAddyBaseUrl: "https://app.addy.io", - forwardedForwardEmailDomain: "hideaddress.net", - forwardedSimpleLoginBaseUrl: "https://app.simplelogin.io", -}; - -export class UsernameGenerationService implements UsernameGenerationServiceAbstraction { - constructor( - private cryptoService: CryptoService, - private stateService: StateService, - private apiService: ApiService, - ) {} - - generateUsername(options: UsernameGeneratorOptions): Promise { - if (options.type === "catchall") { - return this.generateCatchall(options); - } else if (options.type === "subaddress") { - return this.generateSubaddress(options); - } else if (options.type === "forwarded") { - return this.generateForwarded(options); - } else { - return this.generateWord(options); - } - } - - async generateWord(options: UsernameGeneratorOptions): Promise { - const o = Object.assign({}, DefaultOptions, options); - - if (o.wordCapitalize == null) { - o.wordCapitalize = true; - } - if (o.wordIncludeNumber == null) { - o.wordIncludeNumber = true; - } - - const wordIndex = await this.cryptoService.randomNumber(0, EFFLongWordList.length - 1); - let word = EFFLongWordList[wordIndex]; - if (o.wordCapitalize) { - word = word.charAt(0).toUpperCase() + word.slice(1); - } - if (o.wordIncludeNumber) { - const num = await this.cryptoService.randomNumber(1, 9999); - word = word + this.zeroPad(num.toString(), 4); - } - return word; - } - - async generateSubaddress(options: UsernameGeneratorOptions): Promise { - const o = Object.assign({}, DefaultOptions, options); - - const subaddressEmail = o.subaddressEmail; - if (subaddressEmail == null || subaddressEmail.length < 3) { - return o.subaddressEmail; - } - const atIndex = subaddressEmail.indexOf("@"); - if (atIndex < 1 || atIndex >= subaddressEmail.length - 1) { - return subaddressEmail; - } - if (o.subaddressType == null) { - o.subaddressType = "random"; - } - - const emailBeginning = subaddressEmail.substr(0, atIndex); - const emailEnding = subaddressEmail.substr(atIndex + 1, subaddressEmail.length); - - let subaddressString = ""; - if (o.subaddressType === "random") { - subaddressString = await this.randomString(8); - } else if (o.subaddressType === "website-name") { - subaddressString = o.website; - } - return emailBeginning + "+" + subaddressString + "@" + emailEnding; - } - - async generateCatchall(options: UsernameGeneratorOptions): Promise { - const o = Object.assign({}, DefaultOptions, options); - - if (o.catchallDomain == null || o.catchallDomain === "") { - return null; - } - if (o.catchallType == null) { - o.catchallType = "random"; - } - - let startString = ""; - if (o.catchallType === "random") { - startString = await this.randomString(8); - } else if (o.catchallType === "website-name") { - startString = o.website; - } - return startString + "@" + o.catchallDomain; - } - - async generateForwarded(options: UsernameGeneratorOptions): Promise { - const o = Object.assign({}, DefaultOptions, options); - - if (o.forwardedService == null) { - return null; - } - - let forwarder: Forwarder = null; - const forwarderOptions = new ForwarderOptions(); - forwarderOptions.website = o.website; - if (o.forwardedService === "simplelogin") { - forwarder = new SimpleLoginForwarder(); - forwarderOptions.apiKey = o.forwardedSimpleLoginApiKey; - forwarderOptions.simplelogin.baseUrl = o.forwardedSimpleLoginBaseUrl; - } else if (o.forwardedService === "anonaddy") { - forwarder = new AnonAddyForwarder(); - forwarderOptions.apiKey = o.forwardedAnonAddyApiToken; - forwarderOptions.anonaddy.domain = o.forwardedAnonAddyDomain; - forwarderOptions.anonaddy.baseUrl = o.forwardedAnonAddyBaseUrl; - } else if (o.forwardedService === "firefoxrelay") { - forwarder = new FirefoxRelayForwarder(); - forwarderOptions.apiKey = o.forwardedFirefoxApiToken; - } else if (o.forwardedService === "fastmail") { - forwarder = new FastmailForwarder(); - forwarderOptions.apiKey = o.forwardedFastmailApiToken; - } else if (o.forwardedService === "duckduckgo") { - forwarder = new DuckDuckGoForwarder(); - forwarderOptions.apiKey = o.forwardedDuckDuckGoToken; - } else if (o.forwardedService === "forwardemail") { - forwarder = new ForwardEmailForwarder(); - forwarderOptions.apiKey = o.forwardedForwardEmailApiToken; - forwarderOptions.forwardemail.domain = o.forwardedForwardEmailDomain; - } - - if (forwarder == null) { - return null; - } - - return forwarder.generate(this.apiService, forwarderOptions); - } - - getOptions$() { - return from(this.getOptions()); - } - - async getOptions(): Promise { - let options = await this.stateService.getUsernameGenerationOptions(); - if (options == null) { - options = Object.assign({}, DefaultOptions); - } else { - options = Object.assign({}, DefaultOptions, options); - } - await this.stateService.setUsernameGenerationOptions(options); - return options; - } - - async saveOptions(options: UsernameGeneratorOptions) { - await this.stateService.setUsernameGenerationOptions(options); - } - - private async randomString(length: number) { - let str = ""; - const charSet = "abcdefghijklmnopqrstuvwxyz1234567890"; - for (let i = 0; i < length; i++) { - const randomCharIndex = await this.cryptoService.randomNumber(0, charSet.length - 1); - str += charSet.charAt(randomCharIndex); - } - return str; - } - - // ref: https://stackoverflow.com/a/10073788 - private zeroPad(number: string, width: number) { - return number.length >= width - ? number - : new Array(width - number.length + 1).join("0") + number; - } -} diff --git a/libs/common/src/tools/generator/util.ts b/libs/common/src/tools/generator/util.ts new file mode 100644 index 0000000000..ee526fc678 --- /dev/null +++ b/libs/common/src/tools/generator/util.ts @@ -0,0 +1,41 @@ +import { BehaviorSubject } from "rxjs"; + +import { SingleUserState, StateProvider, UserKeyDefinition } from "../../platform/state"; +import { UserId } from "../../types/guid"; + +/** construct a method that outputs a copy of `defaultValue` as an observable. */ +export function clone$PerUserId(defaultValue: Value) { + const _subjects = new Map>(); + + return (key: UserId) => { + let value = _subjects.get(key); + + if (value === undefined) { + value = new BehaviorSubject({ ...defaultValue }); + _subjects.set(key, value); + } + + return value.asObservable(); + }; +} + +/** construct a method that caches user-specific states by userid. */ +export function sharedByUserId(create: (userId: UserId) => SingleUserState) { + const _subjects = new Map>(); + + return (key: UserId) => { + let value = _subjects.get(key); + + if (value === undefined) { + value = create(key); + _subjects.set(key, value); + } + + return value; + }; +} + +/** construct a method that loads a user-specific state from the provider. */ +export function sharedStateByUserId(key: UserKeyDefinition, provider: StateProvider) { + return (id: UserId) => provider.getUser(id, key); +} diff --git a/libs/common/src/tools/generator/word-options.ts b/libs/common/src/tools/generator/word-options.ts new file mode 100644 index 0000000000..1c98d0bac8 --- /dev/null +++ b/libs/common/src/tools/generator/word-options.ts @@ -0,0 +1,6 @@ +export type WordOptions = { + /** set the first letter uppercase */ + titleCase?: boolean; + /** append a number */ + number?: boolean; +}; From 13c2c2ecaaf85fe0224ade2bcfb8fb3ed7b30396 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Tue, 4 Jun 2024 08:38:54 -0700 Subject: [PATCH 05/13] [PM-8236] Allow for additional data properties (#9278) * allow for additional properties * create interface for DataProperties and extend AnonLayouWrapperData * move data properties to RouterService * add docs * add comment * rewrite comment --- apps/web/src/app/core/router.service.ts | 8 +++ apps/web/src/app/oss-routing.module.ts | 67 ++++++++++++++----------- 2 files changed, 46 insertions(+), 29 deletions(-) diff --git a/apps/web/src/app/core/router.service.ts b/apps/web/src/app/core/router.service.ts index c0c1ec2640..2944732aee 100644 --- a/apps/web/src/app/core/router.service.ts +++ b/apps/web/src/app/core/router.service.ts @@ -12,6 +12,14 @@ import { GlobalState, } from "@bitwarden/common/platform/state"; +/** + * Data properties acceptable for use in route objects (see usage in oss-routing.module.ts for example) + */ +export interface DataProperties { + titleId?: string; // sets the title of the current HTML document (shows in browser tab) + doNotSaveUrl?: boolean; // choose to not keep track of the previous URL in memory +} + const DEEP_LINK_REDIRECT_URL = new KeyDefinition(ROUTER_DISK, "deepLinkRedirectUrl", { deserializer: (value: string) => value, }); diff --git a/apps/web/src/app/oss-routing.module.ts b/apps/web/src/app/oss-routing.module.ts index e543a6f083..c7b4631fa3 100644 --- a/apps/web/src/app/oss-routing.module.ts +++ b/apps/web/src/app/oss-routing.module.ts @@ -40,6 +40,7 @@ import { UpdatePasswordComponent } from "./auth/update-password.component"; import { UpdateTempPasswordComponent } from "./auth/update-temp-password.component"; import { VerifyEmailTokenComponent } from "./auth/verify-email-token.component"; import { VerifyRecoverDeleteComponent } from "./auth/verify-recover-delete.component"; +import { DataProperties } from "./core"; import { FrontendLayoutComponent } from "./layouts/frontend-layout.component"; import { UserLayoutComponent } from "./layouts/user-layout.component"; import { DomainRulesComponent } from "./settings/domain-rules.component"; @@ -54,7 +55,7 @@ const routes: Routes = [ { path: "", component: FrontendLayoutComponent, - data: { doNotSaveUrl: true }, + data: { doNotSaveUrl: true } satisfies DataProperties, children: [ { path: "", @@ -66,17 +67,17 @@ const routes: Routes = [ { path: "login-with-device", component: LoginViaAuthRequestComponent, - data: { titleId: "loginWithDevice" }, + data: { titleId: "loginWithDevice" } satisfies DataProperties, }, { path: "login-with-passkey", component: LoginViaWebAuthnComponent, - data: { titleId: "loginWithPasskey" }, + data: { titleId: "loginWithPasskey" } satisfies DataProperties, }, { path: "admin-approval-requested", component: LoginViaAuthRequestComponent, - data: { titleId: "adminApprovalRequested" }, + data: { titleId: "adminApprovalRequested" } satisfies DataProperties, }, { path: "2fa", component: TwoFactorComponent, canActivate: [UnauthGuard] }, { @@ -88,7 +89,7 @@ const routes: Routes = [ path: "register", component: TrialInitiationComponent, canActivate: [UnauthGuard], - data: { titleId: "createAccount" }, + data: { titleId: "createAccount" } satisfies DataProperties, }, { path: "trial", @@ -99,18 +100,18 @@ const routes: Routes = [ path: "sso", component: SsoComponent, canActivate: [UnauthGuard], - data: { titleId: "enterpriseSingleSignOn" }, + data: { titleId: "enterpriseSingleSignOn" } satisfies DataProperties, }, { path: "set-password", component: SetPasswordComponent, - data: { titleId: "setMasterPassword" }, + data: { titleId: "setMasterPassword" } satisfies DataProperties, }, { path: "hint", component: HintComponent, canActivate: [UnauthGuard], - data: { titleId: "passwordHint" }, + data: { titleId: "passwordHint" } satisfies DataProperties, }, { path: "lock", @@ -122,12 +123,12 @@ const routes: Routes = [ path: "accept-organization", canActivate: [deepLinkGuard()], component: AcceptOrganizationComponent, - data: { titleId: "joinOrganization", doNotSaveUrl: false }, + data: { titleId: "joinOrganization", doNotSaveUrl: false } satisfies DataProperties, }, { path: "accept-emergency", canActivate: [deepLinkGuard()], - data: { titleId: "acceptEmergency", doNotSaveUrl: false }, + data: { titleId: "acceptEmergency", doNotSaveUrl: false } satisfies DataProperties, loadComponent: () => import("./auth/emergency-access/accept/accept-emergency.component").then( (mod) => mod.AcceptEmergencyComponent, @@ -137,26 +138,26 @@ const routes: Routes = [ path: "accept-families-for-enterprise", component: AcceptFamilySponsorshipComponent, canActivate: [deepLinkGuard()], - data: { titleId: "acceptFamilySponsorship", doNotSaveUrl: false }, + data: { titleId: "acceptFamilySponsorship", doNotSaveUrl: false } satisfies DataProperties, }, { path: "recover", pathMatch: "full", redirectTo: "recover-2fa" }, { path: "recover-2fa", component: RecoverTwoFactorComponent, canActivate: [UnauthGuard], - data: { titleId: "recoverAccountTwoStep" }, + data: { titleId: "recoverAccountTwoStep" } satisfies DataProperties, }, { path: "recover-delete", component: RecoverDeleteComponent, canActivate: [UnauthGuard], - data: { titleId: "deleteAccount" }, + data: { titleId: "deleteAccount" } satisfies DataProperties, }, { path: "verify-recover-delete", component: VerifyRecoverDeleteComponent, canActivate: [UnauthGuard], - data: { titleId: "deleteAccount" }, + data: { titleId: "deleteAccount" } satisfies DataProperties, }, { path: "verify-recover-delete-org", @@ -168,30 +169,30 @@ const routes: Routes = [ path: "verify-recover-delete-provider", component: VerifyRecoverDeleteProviderComponent, canActivate: [UnauthGuard], - data: { titleId: "deleteAccount" }, + data: { titleId: "deleteAccount" } satisfies DataProperties, }, { path: "send/:sendId/:key", component: AccessComponent, - data: { title: "Bitwarden Send" }, + data: { titleId: "Bitwarden Send" } satisfies DataProperties, }, { path: "update-temp-password", component: UpdateTempPasswordComponent, canActivate: [AuthGuard], - data: { titleId: "updateTempPassword" }, + data: { titleId: "updateTempPassword" } satisfies DataProperties, }, { path: "update-password", component: UpdatePasswordComponent, canActivate: [AuthGuard], - data: { titleId: "updatePassword" }, + data: { titleId: "updatePassword" } satisfies DataProperties, }, { path: "remove-password", component: RemovePasswordComponent, canActivate: [AuthGuard], - data: { titleId: "removeMasterPassword" }, + data: { titleId: "removeMasterPassword" } satisfies DataProperties, }, { path: "migrate-legacy-encryption", @@ -211,21 +212,29 @@ const routes: Routes = [ path: "vault", loadChildren: () => VaultModule, }, - { path: "sends", component: SendComponent, data: { titleId: "send" } }, + { + path: "sends", + component: SendComponent, + data: { titleId: "send" } satisfies DataProperties, + }, { path: "create-organization", component: CreateOrganizationComponent, - data: { titleId: "newOrganization" }, + data: { titleId: "newOrganization" } satisfies DataProperties, }, { path: "settings", children: [ { path: "", pathMatch: "full", redirectTo: "account" }, - { path: "account", component: AccountComponent, data: { titleId: "myAccount" } }, + { + path: "account", + component: AccountComponent, + data: { titleId: "myAccount" } satisfies DataProperties, + }, { path: "preferences", component: PreferencesComponent, - data: { titleId: "preferences" }, + data: { titleId: "preferences" } satisfies DataProperties, }, { path: "security", @@ -234,7 +243,7 @@ const routes: Routes = [ { path: "domain-rules", component: DomainRulesComponent, - data: { titleId: "domainRules" }, + data: { titleId: "domainRules" } satisfies DataProperties, }, { path: "subscription", @@ -249,19 +258,19 @@ const routes: Routes = [ { path: "", component: EmergencyAccessComponent, - data: { titleId: "emergencyAccess" }, + data: { titleId: "emergencyAccess" } satisfies DataProperties, }, { path: ":id", component: EmergencyAccessViewComponent, - data: { titleId: "emergencyAccess" }, + data: { titleId: "emergencyAccess" } satisfies DataProperties, }, ], }, { path: "sponsored-families", component: SponsoredFamiliesComponent, - data: { titleId: "sponsoredFamilies" }, + data: { titleId: "sponsoredFamilies" } satisfies DataProperties, }, ], }, @@ -276,7 +285,7 @@ const routes: Routes = [ import("./tools/import/import-web.component").then((mod) => mod.ImportWebComponent), data: { titleId: "importData", - }, + } satisfies DataProperties, }, { path: "export", @@ -286,7 +295,7 @@ const routes: Routes = [ { path: "generator", component: GeneratorComponent, - data: { titleId: "generator" }, + data: { titleId: "generator" } satisfies DataProperties, }, ], }, From 9322dacbd4d9309c52064d6da2ab9301de175cb5 Mon Sep 17 00:00:00 2001 From: vinith-kovan <156108204+vinith-kovan@users.noreply.github.com> Date: Tue, 4 Jun 2024 23:26:20 +0530 Subject: [PATCH 06/13] [PM 5011][PM-8562] migrate user subscription component (#9499) * migrate user subscription along with update license dialog * migrate user subscription along with update license dialog * migrate user subscription along with update license dialog --- .../individual/user-subscription.component.ts | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/apps/web/src/app/billing/individual/user-subscription.component.ts b/apps/web/src/app/billing/individual/user-subscription.component.ts index 8535f23f82..9b615f3a69 100644 --- a/apps/web/src/app/billing/individual/user-subscription.component.ts +++ b/apps/web/src/app/billing/individual/user-subscription.component.ts @@ -146,19 +146,17 @@ export class UserSubscriptionComponent implements OnInit { } }; - adjustStorage = (add: boolean) => { - return async () => { - const dialogRef = openAdjustStorageDialog(this.dialogService, { - data: { - storageGbPrice: 4, - add: add, - }, - }); - const result = await lastValueFrom(dialogRef.closed); - if (result === AdjustStorageDialogResult.Adjusted) { - await this.load(); - } - }; + adjustStorage = async (add: boolean) => { + const dialogRef = openAdjustStorageDialog(this.dialogService, { + data: { + storageGbPrice: 4, + add: add, + }, + }); + const result = await lastValueFrom(dialogRef.closed); + if (result === AdjustStorageDialogResult.Adjusted) { + await this.load(); + } }; get subscriptionMarkedForCancel() { From fc2953a1266a56c0feeaf0a22ef6537fadae4d5f Mon Sep 17 00:00:00 2001 From: KiruthigaManivannan <162679756+KiruthigaManivannan@users.noreply.github.com> Date: Wed, 5 Jun 2024 01:25:35 +0530 Subject: [PATCH 07/13] AC-2680 Custom permission manage account recovery no longer auto triggers the manage users (#9477) --- .../components/member-dialog/member-dialog.component.html | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.html b/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.html index 149277b817..2a092e2610 100644 --- a/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.html +++ b/apps/web/src/app/admin-console/organizations/members/components/member-dialog/member-dialog.component.html @@ -144,6 +144,7 @@ Date: Tue, 4 Jun 2024 14:09:09 -0700 Subject: [PATCH 08/13] [PM-8455] [PM-7683] Dynamic list items - Copy Action (#9410) * [PM-7683] Add fullAddressForCopy helper to identity.view * [PM-7683] Introduce CopyCipherFieldService to the Vault library - A new CopyCipherFieldService that can be used to copy a cipher's field to the user clipboard - A new appCopyField directive to make it easy to copy a cipher's fields in templates - Tests for the CopyCipherFieldService * [PM-7683] Introduce item-copy-actions.component * [PM-7683] Fix username value in copy cipher directive * [PM-7683] Add title to View item link * [PM-7683] Move disabled logic into own method --- apps/browser/src/_locales/en/messages.json | 65 ++++++- .../item-copy-actions.component.html | 77 ++++++++ .../item-copy-actions.component.ts | 29 +++ .../vault-list-items-container.component.html | 16 +- .../vault-list-items-container.component.ts | 2 + .../src/vault/models/view/identity.view.ts | 11 ++ .../components/copy-cipher-field.directive.ts | 78 ++++++++ libs/vault/src/index.ts | 2 + .../copy-cipher-field.service.spec.ts | 170 ++++++++++++++++++ .../src/services/copy-cipher-field.service.ts | 142 +++++++++++++++ 10 files changed, 580 insertions(+), 12 deletions(-) create mode 100644 apps/browser/src/vault/popup/components/vault-v2/item-copy-action/item-copy-actions.component.html create mode 100644 apps/browser/src/vault/popup/components/vault-v2/item-copy-action/item-copy-actions.component.ts create mode 100644 libs/vault/src/components/copy-cipher-field.directive.ts create mode 100644 libs/vault/src/services/copy-cipher-field.service.spec.ts create mode 100644 libs/vault/src/services/copy-cipher-field.service.ts diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index deb7410a71..3510ed10cb 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -185,7 +185,7 @@ "message": "Continue to browser extension store?" }, "continueToBrowserExtensionStoreDesc": { - "message": "Help others find out if Bitwarden is right for them. Visit your browser's extension store and leave a rating now." + "message": "Help others find out if Bitwarden is right for them. Visit your browser's extension store and leave a rating now." }, "changeMasterPasswordOnWebConfirmation": { "message": "You can change your master password on the Bitwarden web app." @@ -3281,7 +3281,7 @@ "clearFiltersOrTryAnother": { "message": "Clear filters or try another search term" }, - "copyInfo": { + "copyInfoLabel": { "message": "Copy info, $ITEMNAME$", "description": "Aria label for a button that opens a menu with options to copy information from an item.", "placeholders": { @@ -3291,7 +3291,37 @@ } } }, - "moreOptions": { + "copyInfoTitle": { + "message": "Copy info - $ITEMNAME$", + "description": "Title for a button that opens a menu with options to copy information from an item.", + "placeholders": { + "itemname": { + "content": "$1", + "example": "Secret Item" + } + } + }, + "copyNoteLabel": { + "message": "Copy Note, $ITEMNAME$", + "description": "Aria label for a button copies a note to the clipboard.", + "placeholders": { + "itemname": { + "content": "$1", + "example": "Secret Note Item" + } + } + }, + "copyNoteTitle": { + "message": "Copy Note - $ITEMNAME$", + "description": "Title for a button copies a note to the clipboard.", + "placeholders": { + "itemname": { + "content": "$1", + "example": "Secret Note Item" + } + } + }, + "moreOptionsLabel": { "message": "More options, $ITEMNAME$", "description": "Aria label for a button that opens a menu with more options for an item.", "placeholders": { @@ -3301,6 +3331,35 @@ } } }, + "moreOptionsTitle": { + "message": "More options - $ITEMNAME$", + "description": "Title for a button that opens a menu with more options for an item.", + "placeholders": { + "itemname": { + "content": "$1", + "example": "Secret Item" + } + } + }, + "viewItemTitle": { + "message": "View item - $ITEMNAME$", + "description": "Title for a link that opens a view for an item.", + "placeholders": { + "itemname": { + "content": "$1", + "example": "Secret Item" + } + } + }, + "copyEmail": { + "message": "Copy email" + }, + "copyPhone": { + "message": "Copy phone" + }, + "copyAddress": { + "message": "Copy address" + }, "adminConsole": { "message": "Admin Console" }, diff --git a/apps/browser/src/vault/popup/components/vault-v2/item-copy-action/item-copy-actions.component.html b/apps/browser/src/vault/popup/components/vault-v2/item-copy-action/item-copy-actions.component.html new file mode 100644 index 0000000000..08133c6b46 --- /dev/null +++ b/apps/browser/src/vault/popup/components/vault-v2/item-copy-action/item-copy-actions.component.html @@ -0,0 +1,77 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/apps/browser/src/vault/popup/components/vault-v2/item-copy-action/item-copy-actions.component.ts b/apps/browser/src/vault/popup/components/vault-v2/item-copy-action/item-copy-actions.component.ts new file mode 100644 index 0000000000..c89fcca3b3 --- /dev/null +++ b/apps/browser/src/vault/popup/components/vault-v2/item-copy-action/item-copy-actions.component.ts @@ -0,0 +1,29 @@ +import { CommonModule } from "@angular/common"; +import { Component, Input } from "@angular/core"; + +import { JslibModule } from "@bitwarden/angular/jslib.module"; +import { CipherType } from "@bitwarden/common/vault/enums"; +import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { IconButtonModule, ItemModule, MenuModule } from "@bitwarden/components"; +import { CopyCipherFieldDirective } from "@bitwarden/vault"; + +@Component({ + standalone: true, + selector: "app-item-copy-actions", + templateUrl: "item-copy-actions.component.html", + imports: [ + ItemModule, + IconButtonModule, + JslibModule, + MenuModule, + CommonModule, + CopyCipherFieldDirective, + ], +}) +export class ItemCopyActionsComponent { + @Input() cipher: CipherView; + + protected CipherType = CipherType; + + constructor() {} +} diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.html b/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.html index d3bb85c710..47725cf9dc 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.html +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.html @@ -13,7 +13,12 @@ - + {{ cipher.name }} {{ cipher.subTitle }} @@ -22,14 +27,7 @@ - - - + + * ``` + */ +@Directive({ + standalone: true, + selector: "[appCopyField]", +}) +export class CopyCipherFieldDirective implements OnChanges { + @Input({ + alias: "appCopyField", + required: true, + }) + action: Exclude; + + @Input({ required: true }) cipher: CipherView; + + constructor(private copyCipherFieldService: CopyCipherFieldService) {} + + @HostBinding("attr.disabled") + protected disabled: boolean | null = null; + + @HostListener("click") + async copy() { + const value = this.getValueToCopy(); + await this.copyCipherFieldService.copy(value, this.action, this.cipher); + } + + async ngOnChanges() { + await this.updateDisabledState(); + } + + private async updateDisabledState() { + this.disabled = + !this.cipher || + !this.getValueToCopy() || + (this.action === "totp" && !(await this.copyCipherFieldService.totpAllowed(this.cipher))) + ? true + : null; + } + + private getValueToCopy() { + switch (this.action) { + case "username": + return this.cipher.login?.username || this.cipher.identity?.username; + case "password": + return this.cipher.login?.password; + case "totp": + return this.cipher.login?.totp; + case "cardNumber": + return this.cipher.card?.number; + case "securityCode": + return this.cipher.card?.code; + case "email": + return this.cipher.identity?.email; + case "phone": + return this.cipher.identity?.phone; + case "address": + return this.cipher.identity?.fullAddressForCopy; + case "secureNote": + return this.cipher.notes; + default: + return null; + } + } +} diff --git a/libs/vault/src/index.ts b/libs/vault/src/index.ts index bf6b7f91c2..00fa042080 100644 --- a/libs/vault/src/index.ts +++ b/libs/vault/src/index.ts @@ -1 +1,3 @@ export { PasswordRepromptService } from "./services/password-reprompt.service"; +export { CopyCipherFieldService, CopyAction } from "./services/copy-cipher-field.service"; +export { CopyCipherFieldDirective } from "./components/copy-cipher-field.directive"; diff --git a/libs/vault/src/services/copy-cipher-field.service.spec.ts b/libs/vault/src/services/copy-cipher-field.service.spec.ts new file mode 100644 index 0000000000..57c77ebb0b --- /dev/null +++ b/libs/vault/src/services/copy-cipher-field.service.spec.ts @@ -0,0 +1,170 @@ +import { mock, MockProxy } from "jest-mock-extended"; +import { of } from "rxjs"; + +import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service"; +import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; +import { EventType } from "@bitwarden/common/enums"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { TotpService } from "@bitwarden/common/vault/abstractions/totp.service"; +import { CipherRepromptType } from "@bitwarden/common/vault/enums"; +import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { LoginView } from "@bitwarden/common/vault/models/view/login.view"; +import { ToastService } from "@bitwarden/components"; +import { CopyAction, CopyCipherFieldService, PasswordRepromptService } from "@bitwarden/vault"; + +describe("CopyCipherFieldService", () => { + let service: CopyCipherFieldService; + let platformUtilsService: MockProxy; + let toastService: MockProxy; + let eventCollectionService: MockProxy; + let passwordRepromptService: MockProxy; + let totpService: MockProxy; + let i18nService: MockProxy; + let billingAccountProfileStateService: MockProxy; + + beforeEach(() => { + platformUtilsService = mock(); + toastService = mock(); + eventCollectionService = mock(); + passwordRepromptService = mock(); + totpService = mock(); + i18nService = mock(); + billingAccountProfileStateService = mock(); + + service = new CopyCipherFieldService( + platformUtilsService, + toastService, + eventCollectionService, + passwordRepromptService, + totpService, + i18nService, + billingAccountProfileStateService, + ); + }); + + describe("copy", () => { + let cipher: CipherView; + let valueToCopy: string; + let actionType: CopyAction; + let skipReprompt: boolean; + + beforeEach(() => { + cipher = mock(); + valueToCopy = "test"; + actionType = "username"; + skipReprompt = false; + }); + + it("should return early when valueToCopy is null", async () => { + valueToCopy = null; + await service.copy(valueToCopy, actionType, cipher, skipReprompt); + expect(platformUtilsService.copyToClipboard).not.toHaveBeenCalled(); + }); + + it("should return early when cipher.viewPassword is false", async () => { + cipher.viewPassword = false; + await service.copy(valueToCopy, actionType, cipher, skipReprompt); + expect(platformUtilsService.copyToClipboard).not.toHaveBeenCalled(); + }); + + it("should copy value to clipboard", async () => { + await service.copy(valueToCopy, actionType, cipher, skipReprompt); + expect(platformUtilsService.copyToClipboard).toHaveBeenCalledWith(valueToCopy); + }); + + it("should show a success toast on copy", async () => { + i18nService.t.mockReturnValueOnce("Username").mockReturnValueOnce("Username copied"); + await service.copy(valueToCopy, actionType, cipher, skipReprompt); + expect(toastService.showToast).toHaveBeenCalledWith({ + variant: "success", + message: "Username copied", + title: null, + }); + expect(i18nService.t).toHaveBeenCalledWith("username"); + expect(i18nService.t).toHaveBeenCalledWith("valueCopied", "Username"); + }); + + describe("password reprompt", () => { + beforeEach(() => { + actionType = "password"; + cipher.reprompt = CipherRepromptType.Password; + }); + + it("should show password prompt when actionType requires it", async () => { + passwordRepromptService.showPasswordPrompt.mockResolvedValue(true); + await service.copy(valueToCopy, actionType, cipher, skipReprompt); + expect(passwordRepromptService.showPasswordPrompt).toHaveBeenCalled(); + }); + + it("should skip password prompt when cipher.reprompt is 'None'", async () => { + cipher.reprompt = CipherRepromptType.None; + await service.copy(valueToCopy, actionType, cipher, skipReprompt); + expect(passwordRepromptService.showPasswordPrompt).not.toHaveBeenCalled(); + expect(platformUtilsService.copyToClipboard).toHaveBeenCalled(); + }); + + it("should skip password prompt when skipReprompt is true", async () => { + skipReprompt = true; + await service.copy(valueToCopy, actionType, cipher, skipReprompt); + expect(passwordRepromptService.showPasswordPrompt).not.toHaveBeenCalled(); + }); + + it("should return early when password prompt is not confirmed", async () => { + passwordRepromptService.showPasswordPrompt.mockResolvedValue(false); + await service.copy(valueToCopy, actionType, cipher, skipReprompt); + expect(platformUtilsService.copyToClipboard).not.toHaveBeenCalled(); + }); + }); + + describe("totp", () => { + beforeEach(() => { + actionType = "totp"; + cipher.login = new LoginView(); + cipher.login.totp = "secret-totp"; + cipher.reprompt = CipherRepromptType.None; + cipher.organizationUseTotp = false; + }); + + it("should get TOTP code when allowed from premium", async () => { + billingAccountProfileStateService.hasPremiumFromAnySource$ = of(true); + totpService.getCode.mockResolvedValue("123456"); + await service.copy(valueToCopy, actionType, cipher, skipReprompt); + expect(totpService.getCode).toHaveBeenCalledWith(valueToCopy); + expect(platformUtilsService.copyToClipboard).toHaveBeenCalledWith("123456"); + }); + + it("should get TOTP code when allowed from organization", async () => { + cipher.organizationUseTotp = true; + totpService.getCode.mockResolvedValue("123456"); + await service.copy(valueToCopy, actionType, cipher, skipReprompt); + expect(totpService.getCode).toHaveBeenCalledWith(valueToCopy); + expect(platformUtilsService.copyToClipboard).toHaveBeenCalledWith("123456"); + }); + + it("should return early when the user is not allowed to use TOTP", async () => { + billingAccountProfileStateService.hasPremiumFromAnySource$ = of(false); + await service.copy(valueToCopy, actionType, cipher, skipReprompt); + expect(totpService.getCode).not.toHaveBeenCalled(); + expect(platformUtilsService.copyToClipboard).not.toHaveBeenCalled(); + }); + + it("should return early when TOTP is not set", async () => { + cipher.login.totp = null; + await service.copy(valueToCopy, actionType, cipher, skipReprompt); + expect(totpService.getCode).not.toHaveBeenCalled(); + expect(platformUtilsService.copyToClipboard).not.toHaveBeenCalled(); + }); + }); + + it("should collect an event when actionType has one", async () => { + actionType = "password"; + skipReprompt = true; + await service.copy(valueToCopy, actionType, cipher, skipReprompt); + expect(eventCollectionService.collect).toHaveBeenCalledWith( + EventType.Cipher_ClientCopiedPassword, + cipher.id, + ); + }); + }); +}); diff --git a/libs/vault/src/services/copy-cipher-field.service.ts b/libs/vault/src/services/copy-cipher-field.service.ts new file mode 100644 index 0000000000..9cd94f5ce6 --- /dev/null +++ b/libs/vault/src/services/copy-cipher-field.service.ts @@ -0,0 +1,142 @@ +import { Injectable } from "@angular/core"; +import { firstValueFrom } from "rxjs"; + +import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service"; +import { BillingAccountProfileStateService } from "@bitwarden/common/billing/abstractions/account/billing-account-profile-state.service"; +import { EventType } from "@bitwarden/common/enums"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { TotpService } from "@bitwarden/common/vault/abstractions/totp.service"; +import { CipherRepromptType } from "@bitwarden/common/vault/enums"; +import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { ToastService } from "@bitwarden/components"; +import { PasswordRepromptService } from "@bitwarden/vault"; + +/** + * The types of fields that can be copied from a cipher. + */ +export type CopyAction = + | "username" + | "password" + | "totp" + | "cardNumber" + | "securityCode" + | "email" + | "phone" + | "address" + | "secureNote" + | "hiddenField"; + +type CopyActionInfo = { + /** + * The i18n key for the type of field being copied. Will be used to display a toast message. + */ + typeI18nKey: string; + + /** + * Whether the field is protected and requires password re-prompting before being copied. + */ + protected: boolean; + + /** + * Optional event to collect when the field is copied. + */ + event?: EventType; +}; + +const CopyActions: Record = { + username: { typeI18nKey: "username", protected: false }, + password: { + typeI18nKey: "password", + protected: true, + event: EventType.Cipher_ClientCopiedPassword, + }, + totp: { typeI18nKey: "verificationCodeTotp", protected: true }, + cardNumber: { typeI18nKey: "number", protected: true }, + securityCode: { + typeI18nKey: "securityCode", + protected: true, + event: EventType.Cipher_ClientCopiedCardCode, + }, + email: { typeI18nKey: "email", protected: false }, + phone: { typeI18nKey: "phone", protected: false }, + address: { typeI18nKey: "address", protected: false }, + secureNote: { typeI18nKey: "note", protected: false }, + hiddenField: { + typeI18nKey: "value", + protected: true, + event: EventType.Cipher_ClientCopiedHiddenField, + }, +}; + +@Injectable({ + providedIn: "root", +}) +export class CopyCipherFieldService { + constructor( + private platformUtilsService: PlatformUtilsService, + private toastService: ToastService, + private eventCollectionService: EventCollectionService, + private passwordRepromptService: PasswordRepromptService, + private totpService: TotpService, + private i18nService: I18nService, + private billingAccountProfileStateService: BillingAccountProfileStateService, + ) {} + + /** + * Copy a field value from a cipher to the clipboard. + * @param valueToCopy The value to copy. + * @param actionType The type of field being copied. + * @param cipher The cipher containing the field to copy. + * @param skipReprompt Whether to skip password re-prompting. + */ + async copy( + valueToCopy: string, + actionType: CopyAction, + cipher: CipherView, + skipReprompt: boolean = false, + ) { + const action = CopyActions[actionType]; + if ( + !skipReprompt && + cipher.reprompt !== CipherRepromptType.None && + action.protected && + !(await this.passwordRepromptService.showPasswordPrompt()) + ) { + return; + } + + if (valueToCopy == null || !cipher.viewPassword) { + return; + } + + if (actionType === "totp") { + if (!(await this.totpAllowed(cipher))) { + return; + } + valueToCopy = await this.totpService.getCode(valueToCopy); + } + + this.platformUtilsService.copyToClipboard(valueToCopy); + this.toastService.showToast({ + variant: "success", + message: this.i18nService.t("valueCopied", this.i18nService.t(action.typeI18nKey)), + title: null, + }); + + if (action.event !== undefined) { + await this.eventCollectionService.collect(action.event, cipher.id); + } + } + + /** + * Determines if TOTP generation is allowed for a cipher and user. + */ + async totpAllowed(cipher: CipherView): Promise { + return ( + (cipher?.login?.hasTotp ?? false) && + (cipher.organizationUseTotp || + (await firstValueFrom(this.billingAccountProfileStateService.hasPremiumFromAnySource$))) + ); + } +} From 6eb94078f65ff9cbe1b3bf5634c27e78ddba71c0 Mon Sep 17 00:00:00 2001 From: Shane Melton Date: Tue, 4 Jun 2024 14:21:14 -0700 Subject: [PATCH 09/13] [PM-8456] [PM-7683] Dynamic list items - 3 dot menu (#9422) * [PM-7683] Add fullAddressForCopy helper to identity.view * [PM-7683] Introduce CopyCipherFieldService to the Vault library - A new CopyCipherFieldService that can be used to copy a cipher's field to the user clipboard - A new appCopyField directive to make it easy to copy a cipher's fields in templates - Tests for the CopyCipherFieldService * [PM-7683] Introduce item-copy-actions.component * [PM-7683] Fix username value in copy cipher directive * [PM-7683] Add title to View item link * [PM-8456] Introduce initial item-more-options.component * [PM-8456] Add logic to show/hide login menu options * [PM-8456] Implement favorite/unfavorite menu option * [PM-8456] Implement clone menu option * [PM-8456] Hide launch website instead of disabling it * [PM-8456] Ensure cipherList observable updates on cipher changes * [PM-7683] Move disabled logic into own method * [PM-8456] Cleanup spec file to use Angular testbed * [PM-8456] Fix more options tooltip --- apps/browser/src/_locales/en/messages.json | 14 +- .../autofill-vault-list-items.component.html | 2 +- .../item-more-options.component.html | 36 ++++++ .../item-more-options.component.ts | 122 ++++++++++++++++++ .../vault-list-items-container.component.html | 14 +- .../vault-list-items-container.component.ts | 4 +- .../components/vault/vault-v2.component.html | 8 +- .../vault-popup-items.service.spec.ts | 66 ++++------ .../services/vault-popup-items.service.ts | 10 +- .../src/vault/abstractions/cipher.service.ts | 1 + 10 files changed, 215 insertions(+), 62 deletions(-) create mode 100644 apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.html create mode 100644 apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index 3510ed10cb..c18a645c11 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -389,6 +389,9 @@ "favorite": { "message": "Favorite" }, + "unfavorite": { + "message": "Unfavorite" + }, "notes": { "message": "Notes" }, @@ -410,6 +413,9 @@ "launch": { "message": "Launch" }, + "launchWebsite": { + "message": "Launch website" + }, "website": { "message": "Website" }, @@ -822,7 +828,7 @@ }, "exportPasswordDescription": { "message": "This password will be used to export and import this file" - }, + }, "accountRestrictedOptionDescription": { "message": "Use your account encryption key, derived from your account's username and Master Password, to encrypt the export and restrict import to only the current Bitwarden account." }, @@ -1660,6 +1666,9 @@ "autoFillAndSave": { "message": "Auto-fill and save" }, + "fillAndSave": { + "message": "Fill and save" + }, "autoFillSuccessAndSavedUri": { "message": "Item auto-filled and URI saved" }, @@ -3351,6 +3360,9 @@ } } }, + "assignCollections": { + "message": "Assign collections" + }, "copyEmail": { "message": "Copy email" }, diff --git a/apps/browser/src/vault/popup/components/vault-v2/autofill-vault-list-items/autofill-vault-list-items.component.html b/apps/browser/src/vault/popup/components/vault-v2/autofill-vault-list-items/autofill-vault-list-items.component.html index 0b108e8b81..83b07fc14c 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/autofill-vault-list-items/autofill-vault-list-items.component.html +++ b/apps/browser/src/vault/popup/components/vault-v2/autofill-vault-list-items/autofill-vault-list-items.component.html @@ -4,7 +4,7 @@ [title]="'autofillSuggestions' | i18n" [showRefresh]="showRefresh" (onRefresh)="refreshCurrentTab()" - showAutoFill + showAutofillButton > diff --git a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.html b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.html new file mode 100644 index 0000000000..1d7a2a8cd0 --- /dev/null +++ b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.html @@ -0,0 +1,36 @@ + + + + + + + + + + + + + + {{ "clone" | i18n }} + + + + + diff --git a/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts new file mode 100644 index 0000000000..9834dc553e --- /dev/null +++ b/apps/browser/src/vault/popup/components/vault-v2/item-more-options/item-more-options.component.ts @@ -0,0 +1,122 @@ +import { CommonModule } from "@angular/common"; +import { booleanAttribute, Component, Input } from "@angular/core"; +import { Router, RouterModule } from "@angular/router"; + +import { JslibModule } from "@bitwarden/angular/jslib.module"; +import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { CipherRepromptType, CipherType } from "@bitwarden/common/vault/enums"; +import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { DialogService, IconButtonModule, ItemModule, MenuModule } from "@bitwarden/components"; +import { PasswordRepromptService } from "@bitwarden/vault"; + +import { BrowserApi } from "../../../../../platform/browser/browser-api"; +import BrowserPopupUtils from "../../../../../platform/popup/browser-popup-utils"; +import { VaultPopupItemsService } from "../../../services/vault-popup-items.service"; + +@Component({ + standalone: true, + selector: "app-item-more-options", + templateUrl: "./item-more-options.component.html", + imports: [ItemModule, IconButtonModule, MenuModule, CommonModule, JslibModule, RouterModule], +}) +export class ItemMoreOptionsComponent { + @Input({ + required: true, + }) + cipher: CipherView; + + /** + * Flag to hide the login specific menu options. Used for login items that are + * already in the autofill list suggestion. + */ + @Input({ transform: booleanAttribute }) + hideLoginOptions: boolean; + + protected autofillAllowed$ = this.vaultPopupItemsService.autofillAllowed$; + + constructor( + private cipherService: CipherService, + private vaultPopupItemsService: VaultPopupItemsService, + private passwordRepromptService: PasswordRepromptService, + private dialogService: DialogService, + private router: Router, + ) {} + + get canEdit() { + return this.cipher.edit; + } + + get isLogin() { + return this.cipher.type === CipherType.Login; + } + + get favoriteText() { + return this.cipher.favorite ? "unfavorite" : "favorite"; + } + + /** + * Determines if the login cipher can be launched in a new browser tab. + */ + get canLaunch() { + return this.isLogin && this.cipher.login.canLaunch; + } + + /** + * Launches the login cipher in a new browser tab. + */ + async launchCipher() { + if (!this.canLaunch) { + return; + } + + await this.cipherService.updateLastLaunchedDate(this.cipher.id); + + await BrowserApi.createNewTab(this.cipher.login.launchUri); + + if (BrowserPopupUtils.inPopup(window)) { + BrowserApi.closePopup(window); + } + } + + /** + * Toggles the favorite status of the cipher and updates it on the server. + */ + async toggleFavorite() { + this.cipher.favorite = !this.cipher.favorite; + const encryptedCipher = await this.cipherService.encrypt(this.cipher); + await this.cipherService.updateWithServer(encryptedCipher); + } + + /** + * Navigate to the clone cipher page with the current cipher as the source. + * A password reprompt is attempted if the cipher requires it. + * A confirmation dialog is shown if the cipher has FIDO2 credentials. + */ + async clone() { + if ( + this.cipher.reprompt === CipherRepromptType.Password && + !(await this.passwordRepromptService.showPasswordPrompt()) + ) { + return; + } + + if (this.cipher.login?.hasFido2Credentials) { + const confirmed = await this.dialogService.openSimpleDialog({ + title: { key: "passkeyNotCopied" }, + content: { key: "passkeyNotCopiedAlert" }, + type: "info", + }); + + if (!confirmed) { + return; + } + } + + await this.router.navigate(["/clone-cipher"], { + queryParams: { + cloneMode: true, + cipherId: this.cipher.id, + }, + }); + } +} diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.html b/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.html index 47725cf9dc..91c1f52163 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.html +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.html @@ -24,18 +24,14 @@ {{ cipher.subTitle }} - + - - - + diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.ts b/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.ts index e8200cab79..095d0155c6 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.ts @@ -15,6 +15,7 @@ import { import { PopupSectionHeaderComponent } from "../../../../../platform/popup/popup-section-header/popup-section-header.component"; import { ItemCopyActionsComponent } from "../item-copy-action/item-copy-actions.component"; +import { ItemMoreOptionsComponent } from "../item-more-options/item-more-options.component"; @Component({ imports: [ @@ -29,6 +30,7 @@ import { ItemCopyActionsComponent } from "../item-copy-action/item-copy-actions. PopupSectionHeaderComponent, RouterLink, ItemCopyActionsComponent, + ItemMoreOptionsComponent, ], selector: "app-vault-list-items-container", templateUrl: "vault-list-items-container.component.html", @@ -63,5 +65,5 @@ export class VaultListItemsContainerComponent { * Option to show the autofill button for each item. */ @Input({ transform: booleanAttribute }) - showAutoFill: boolean; + showAutofillButton: boolean; } diff --git a/apps/browser/src/vault/popup/components/vault/vault-v2.component.html b/apps/browser/src/vault/popup/components/vault/vault-v2.component.html index 4d75685f53..f99d3cbb30 100644 --- a/apps/browser/src/vault/popup/components/vault/vault-v2.component.html +++ b/apps/browser/src/vault/popup/components/vault/vault-v2.component.html @@ -22,10 +22,12 @@ - - +
+ + - + +
{ + let testBed: TestBed; let service: VaultPopupItemsService; let allCiphers: Record; let autoFillCiphers: CipherView[]; @@ -39,11 +41,12 @@ describe("VaultPopupItemsService", () => { cipherList[2].favorite = true; cipherList[3].favorite = true; - cipherServiceMock.cipherViews$ = new BehaviorSubject(allCiphers).asObservable(); + cipherServiceMock.getAllDecrypted.mockResolvedValue(cipherList); + cipherServiceMock.ciphers$ = new BehaviorSubject(null).asObservable(); searchService.searchCiphers.mockImplementation(async () => cipherList); cipherServiceMock.filterCiphersForUrl.mockImplementation(async () => autoFillCiphers); - vaultSettingsServiceMock.showCardsCurrentTab$ = new BehaviorSubject(false).asObservable(); - vaultSettingsServiceMock.showIdentitiesCurrentTab$ = new BehaviorSubject(false).asObservable(); + vaultSettingsServiceMock.showCardsCurrentTab$ = new BehaviorSubject(false); + vaultSettingsServiceMock.showIdentitiesCurrentTab$ = new BehaviorSubject(false); vaultPopupListFiltersServiceMock.filters$ = new BehaviorSubject({ organization: null, @@ -55,28 +58,26 @@ describe("VaultPopupItemsService", () => { vaultPopupListFiltersServiceMock.filterFunction$ = new BehaviorSubject( (ciphers: CipherView[]) => ciphers, ); - jest.spyOn(BrowserPopupUtils, "inPopout").mockReturnValue(false); jest .spyOn(BrowserApi, "getTabFromCurrentWindow") .mockResolvedValue({ url: "https://example.com" } as chrome.tabs.Tab); - service = new VaultPopupItemsService( - cipherServiceMock, - vaultSettingsServiceMock, - vaultPopupListFiltersServiceMock, - organizationServiceMock, - searchService, - ); + + testBed = TestBed.configureTestingModule({ + providers: [ + { provide: CipherService, useValue: cipherServiceMock }, + { provide: VaultSettingsService, useValue: vaultSettingsServiceMock }, + { provide: SearchService, useValue: searchService }, + { provide: OrganizationService, useValue: organizationServiceMock }, + { provide: VaultPopupListFiltersService, useValue: vaultPopupListFiltersServiceMock }, + ], + }); + + service = testBed.inject(VaultPopupItemsService); }); it("should be created", () => { - service = new VaultPopupItemsService( - cipherServiceMock, - vaultSettingsServiceMock, - vaultPopupListFiltersServiceMock, - organizationServiceMock, - searchService, - ); + service = testBed.inject(VaultPopupItemsService); expect(service).toBeTruthy(); }); @@ -100,18 +101,10 @@ describe("VaultPopupItemsService", () => { it("should filter ciphers for the current tab and types", (done) => { const currentTab = { url: "https://example.com" } as chrome.tabs.Tab; - vaultSettingsServiceMock.showCardsCurrentTab$ = new BehaviorSubject(true).asObservable(); - vaultSettingsServiceMock.showIdentitiesCurrentTab$ = new BehaviorSubject(true).asObservable(); + (vaultSettingsServiceMock.showCardsCurrentTab$ as BehaviorSubject).next(true); + (vaultSettingsServiceMock.showIdentitiesCurrentTab$ as BehaviorSubject).next(true); jest.spyOn(BrowserApi, "getTabFromCurrentWindow").mockResolvedValue(currentTab); - service = new VaultPopupItemsService( - cipherServiceMock, - vaultSettingsServiceMock, - vaultPopupListFiltersServiceMock, - organizationServiceMock, - searchService, - ); - service.autoFillCiphers$.subscribe((ciphers) => { expect(cipherServiceMock.filterCiphersForUrl.mock.calls.length).toBe(1); expect(cipherServiceMock.filterCiphersForUrl).toHaveBeenCalledWith( @@ -136,14 +129,6 @@ describe("VaultPopupItemsService", () => { Object.values(allCiphers), ); - service = new VaultPopupItemsService( - cipherServiceMock, - vaultSettingsServiceMock, - vaultPopupListFiltersServiceMock, - organizationServiceMock, - searchService, - ); - service.autoFillCiphers$.subscribe((ciphers) => { expect(ciphers.length).toBe(10); @@ -248,14 +233,7 @@ describe("VaultPopupItemsService", () => { describe("emptyVault$", () => { it("should return true if there are no ciphers", (done) => { - cipherServiceMock.cipherViews$ = new BehaviorSubject({}).asObservable(); - service = new VaultPopupItemsService( - cipherServiceMock, - vaultSettingsServiceMock, - vaultPopupListFiltersServiceMock, - organizationServiceMock, - searchService, - ); + cipherServiceMock.getAllDecrypted.mockResolvedValue([]); service.emptyVault$.subscribe((empty) => { expect(empty).toBe(true); done(); diff --git a/apps/browser/src/vault/popup/services/vault-popup-items.service.ts b/apps/browser/src/vault/popup/services/vault-popup-items.service.ts index f9c37f6f7d..3fe509dc45 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-items.service.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-items.service.ts @@ -1,4 +1,4 @@ -import { Injectable } from "@angular/core"; +import { inject, Injectable, NgZone } from "@angular/core"; import { BehaviorSubject, combineLatest, @@ -15,12 +15,14 @@ import { import { SearchService } from "@bitwarden/common/abstractions/search.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { Utils } from "@bitwarden/common/platform/misc/utils"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { VaultSettingsService } from "@bitwarden/common/vault/abstractions/vault-settings/vault-settings.service"; import { CipherType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { BrowserApi } from "../../../platform/browser/browser-api"; +import { runInsideAngular } from "../../../platform/browser/run-inside-angular.operator"; import BrowserPopupUtils from "../../../platform/popup/browser-popup-utils"; import { MY_VAULT_ID, VaultPopupListFiltersService } from "./vault-popup-list-filters.service"; @@ -72,9 +74,11 @@ export class VaultPopupItemsService { * Observable that contains the list of all decrypted ciphers. * @private */ - private _cipherList$: Observable = this.cipherService.cipherViews$.pipe( + private _cipherList$: Observable = this.cipherService.ciphers$.pipe( + runInsideAngular(inject(NgZone)), // Workaround to ensure cipher$ state provider emissions are run inside Angular + switchMap(() => Utils.asyncToObservable(() => this.cipherService.getAllDecrypted())), map((ciphers) => Object.values(ciphers)), - shareReplay({ refCount: false, bufferSize: 1 }), + shareReplay({ refCount: true, bufferSize: 1 }), ); private _filteredCipherList$: Observable = combineLatest([ diff --git a/libs/common/src/vault/abstractions/cipher.service.ts b/libs/common/src/vault/abstractions/cipher.service.ts index 0ea628cca1..34bc819355 100644 --- a/libs/common/src/vault/abstractions/cipher.service.ts +++ b/libs/common/src/vault/abstractions/cipher.service.ts @@ -13,6 +13,7 @@ import { AddEditCipherInfo } from "../types/add-edit-cipher-info"; export abstract class CipherService { cipherViews$: Observable>; + ciphers$: Observable>; /** * An observable monitoring the add/edit cipher info saved to memory. */ From d0c1325f0cc79a57c947c1d5925f9173eacb8cc4 Mon Sep 17 00:00:00 2001 From: Nick Krantz <125900171+nick-livefront@users.noreply.github.com> Date: Tue, 4 Jun 2024 16:26:39 -0500 Subject: [PATCH 10/13] update icons for folder & collection filters (#9508) --- .../vault-popup-list-filters.service.spec.ts | 23 +++++++++++++++++++ .../vault-popup-list-filters.service.ts | 17 +++++++++----- 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.spec.ts b/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.spec.ts index eba8f94f12..42626b5291 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.spec.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.spec.ts @@ -169,6 +169,13 @@ describe("VaultPopupListFiltersService", () => { expect(collections.map((c) => c.label)).toEqual(["Test collection 2"]); }); }); + + it("sets collection icon", (done) => { + service.collections$.subscribe((collections) => { + expect(collections.every(({ icon }) => icon === "bwi-collection")).toBeTruthy(); + done(); + }); + }); }); describe("folders$", () => { @@ -210,6 +217,22 @@ describe("VaultPopupListFiltersService", () => { }); }); + it("sets folder icon", (done) => { + service.filterForm.patchValue({ + organization: { id: MY_VAULT_ID } as Organization, + }); + + folderViews$.next([ + { id: "1234", name: "Folder 1" }, + { id: "2345", name: "Folder 2" }, + ]); + + service.folders$.subscribe((folders) => { + expect(folders.every(({ icon }) => icon === "bwi-folder")).toBeTruthy(); + done(); + }); + }); + it("returns folders that have ciphers within the selected organization", (done) => { service.folders$.pipe(skipWhile((folders) => folders.length === 2)).subscribe((folders) => { expect(folders.map((f) => f.label)).toEqual(["Folder 1"]); diff --git a/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.ts b/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.ts index f3522aa8e3..bc42e7cb0a 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-list-filters.service.ts @@ -206,7 +206,7 @@ export class VaultPopupListFiltersService { /** * Folder array structured to be directly passed to `ChipSelectComponent` */ - folders$: Observable[]> = combineLatest([ + folders$: Observable[]> = combineLatest([ this.filters$.pipe( distinctUntilChanged( (previousFilter, currentFilter) => @@ -258,13 +258,15 @@ export class VaultPopupListFiltersService { nestedList: nestedFolders, }); }), - map((folders) => folders.nestedList.map(this.convertToChipSelectOption.bind(this))), + map((folders) => + folders.nestedList.map((f) => this.convertToChipSelectOption(f, "bwi-folder")), + ), ); /** * Collection array structured to be directly passed to `ChipSelectComponent` */ - collections$: Observable[]> = combineLatest([ + collections$: Observable[]> = combineLatest([ this.filters$.pipe( distinctUntilChanged( (previousFilter, currentFilter) => @@ -292,7 +294,9 @@ export class VaultPopupListFiltersService { nestedList: nestedCollections, }); }), - map((collections) => collections.nestedList.map(this.convertToChipSelectOption.bind(this))), + map((collections) => + collections.nestedList.map((c) => this.convertToChipSelectOption(c, "bwi-collection")), + ), ); /** @@ -300,13 +304,14 @@ export class VaultPopupListFiltersService { */ private convertToChipSelectOption( item: TreeNode, + icon: string, ): ChipSelectOption { return { value: item.node, label: item.node.name, - icon: "bwi-folder", // Organization & Folder icons are the same + icon, children: item.children - ? item.children.map(this.convertToChipSelectOption.bind(this)) + ? item.children.map((i) => this.convertToChipSelectOption(i, icon)) : undefined, }; } From f059d136b22749821a0d9263719ed35a1cdbca49 Mon Sep 17 00:00:00 2001 From: Shane Melton Date: Tue, 4 Jun 2024 14:34:48 -0700 Subject: [PATCH 11/13] [PM-8485] [PM-7683] Dynamic list items - Org Details (#9466) * [PM-7683] Add fullAddressForCopy helper to identity.view * [PM-7683] Introduce CopyCipherFieldService to the Vault library - A new CopyCipherFieldService that can be used to copy a cipher's field to the user clipboard - A new appCopyField directive to make it easy to copy a cipher's fields in templates - Tests for the CopyCipherFieldService * [PM-7683] Introduce item-copy-actions.component * [PM-7683] Fix username value in copy cipher directive * [PM-7683] Add title to View item link * [PM-8456] Introduce initial item-more-options.component * [PM-8456] Add logic to show/hide login menu options * [PM-8456] Implement favorite/unfavorite menu option * [PM-8456] Implement clone menu option * [PM-8456] Hide launch website instead of disabling it * [PM-8456] Ensure cipherList observable updates on cipher changes * [PM-7683] Move disabled logic into own method * [PM-8456] Cleanup spec file to use Angular testbed * [PM-8456] Fix more options tooltip * [PM-8485] Introduce new PopupCipherView * [PM-8485] Use new PopupCipherView in items service * [PM-8485] Add org icon for items that belong to an organization * [PM-8485] Fix tests * [PM-8485] Remove share operator from cipherViews$ --- apps/browser/src/_locales/en/messages.json | 9 +++ .../autofill-vault-list-items.component.ts | 4 +- .../vault-list-items-container.component.html | 6 ++ .../vault-list-items-container.component.ts | 19 ++++++- .../vault-popup-items.service.spec.ts | 55 ++++++++++++++++--- .../services/vault-popup-items.service.ts | 38 ++++++++++--- .../vault/popup/views/popup-cipher.view.ts | 41 ++++++++++++++ .../src/vault/services/cipher.service.ts | 10 +--- 8 files changed, 156 insertions(+), 26 deletions(-) create mode 100644 apps/browser/src/vault/popup/views/popup-cipher.view.ts diff --git a/apps/browser/src/_locales/en/messages.json b/apps/browser/src/_locales/en/messages.json index c18a645c11..231637d7af 100644 --- a/apps/browser/src/_locales/en/messages.json +++ b/apps/browser/src/_locales/en/messages.json @@ -1437,6 +1437,15 @@ "collections": { "message": "Collections" }, + "nCollections": { + "message": "$COUNT$ collections", + "placeholders": { + "count": { + "content": "$1", + "example": "2" + } + } + }, "favorites": { "message": "Favorites" }, diff --git a/apps/browser/src/vault/popup/components/vault-v2/autofill-vault-list-items/autofill-vault-list-items.component.ts b/apps/browser/src/vault/popup/components/vault-v2/autofill-vault-list-items/autofill-vault-list-items.component.ts index c00e585e73..9a4670bb4c 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/autofill-vault-list-items/autofill-vault-list-items.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/autofill-vault-list-items/autofill-vault-list-items.component.ts @@ -3,12 +3,12 @@ import { Component } from "@angular/core"; import { combineLatest, map, Observable } from "rxjs"; import { JslibModule } from "@bitwarden/angular/jslib.module"; -import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { IconButtonModule, SectionComponent, TypographyModule } from "@bitwarden/components"; import BrowserPopupUtils from "../../../../../platform/popup/browser-popup-utils"; import { PopupSectionHeaderComponent } from "../../../../../platform/popup/popup-section-header/popup-section-header.component"; import { VaultPopupItemsService } from "../../../services/vault-popup-items.service"; +import { PopupCipherView } from "../../../views/popup-cipher.view"; import { VaultListItemsContainerComponent } from "../vault-list-items-container/vault-list-items-container.component"; @Component({ @@ -30,7 +30,7 @@ export class AutofillVaultListItemsComponent { * The list of ciphers that can be used to autofill the current page. * @protected */ - protected autofillCiphers$: Observable = + protected autofillCiphers$: Observable = this.vaultPopupItemsService.autoFillCiphers$; /** diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.html b/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.html index 91c1f52163..74ee1af1ff 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.html +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.html @@ -21,6 +21,12 @@ > {{ cipher.name }} + {{ cipher.subTitle }} diff --git a/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.ts b/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.ts index 095d0155c6..311619fa0f 100644 --- a/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.ts +++ b/apps/browser/src/vault/popup/components/vault-v2/vault-list-items-container/vault-list-items-container.component.ts @@ -3,7 +3,7 @@ import { booleanAttribute, Component, EventEmitter, Input, Output } from "@angul import { RouterLink } from "@angular/router"; import { JslibModule } from "@bitwarden/angular/jslib.module"; -import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { BadgeModule, ButtonModule, @@ -14,6 +14,7 @@ import { } from "@bitwarden/components"; import { PopupSectionHeaderComponent } from "../../../../../platform/popup/popup-section-header/popup-section-header.component"; +import { PopupCipherView } from "../../../views/popup-cipher.view"; import { ItemCopyActionsComponent } from "../item-copy-action/item-copy-actions.component"; import { ItemMoreOptionsComponent } from "../item-more-options/item-more-options.component"; @@ -41,7 +42,7 @@ export class VaultListItemsContainerComponent { * The list of ciphers to display. */ @Input() - ciphers: CipherView[]; + ciphers: PopupCipherView[] = []; /** * Title for the vault list item section. @@ -66,4 +67,18 @@ export class VaultListItemsContainerComponent { */ @Input({ transform: booleanAttribute }) showAutofillButton: boolean; + + /** + * The tooltip text for the organization icon for ciphers that belong to an organization. + * @param cipher + */ + orgIconTooltip(cipher: PopupCipherView) { + if (cipher.collectionIds.length > 1) { + return this.i18nService.t("nCollections", cipher.collectionIds.length); + } + + return cipher.collections[0]?.name; + } + + constructor(private i18nService: I18nService) {} } diff --git a/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts b/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts index aa8b7c74d1..6a7cbbfc1a 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts @@ -4,11 +4,15 @@ import { BehaviorSubject } from "rxjs"; import { SearchService } from "@bitwarden/common/abstractions/search.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { ProductType } from "@bitwarden/common/enums"; import { CipherId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { CollectionService } from "@bitwarden/common/vault/abstractions/collection.service"; import { VaultSettingsService } from "@bitwarden/common/vault/abstractions/vault-settings/vault-settings.service"; import { CipherType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { CollectionView } from "@bitwarden/common/vault/models/view/collection.view"; import { BrowserApi } from "../../../platform/browser/browser-api"; import BrowserPopupUtils from "../../../platform/popup/browser-popup-utils"; @@ -22,11 +26,15 @@ describe("VaultPopupItemsService", () => { let allCiphers: Record; let autoFillCiphers: CipherView[]; + let mockOrg: Organization; + let mockCollections: CollectionView[]; + const cipherServiceMock = mock(); const vaultSettingsServiceMock = mock(); const organizationServiceMock = mock(); const vaultPopupListFiltersServiceMock = mock(); const searchService = mock(); + const collectionService = mock(); beforeEach(() => { allCiphers = cipherFactory(10); @@ -43,8 +51,10 @@ describe("VaultPopupItemsService", () => { cipherServiceMock.getAllDecrypted.mockResolvedValue(cipherList); cipherServiceMock.ciphers$ = new BehaviorSubject(null).asObservable(); - searchService.searchCiphers.mockImplementation(async () => cipherList); - cipherServiceMock.filterCiphersForUrl.mockImplementation(async () => autoFillCiphers); + searchService.searchCiphers.mockImplementation(async (_, __, ciphers) => ciphers); + cipherServiceMock.filterCiphersForUrl.mockImplementation(async (ciphers) => + ciphers.filter((c) => ["0", "1"].includes(c.id)), + ); vaultSettingsServiceMock.showCardsCurrentTab$ = new BehaviorSubject(false); vaultSettingsServiceMock.showIdentitiesCurrentTab$ = new BehaviorSubject(false); @@ -63,6 +73,20 @@ describe("VaultPopupItemsService", () => { .spyOn(BrowserApi, "getTabFromCurrentWindow") .mockResolvedValue({ url: "https://example.com" } as chrome.tabs.Tab); + mockOrg = { + id: "org1", + name: "Organization 1", + planProductType: ProductType.Enterprise, + } as Organization; + + mockCollections = [ + { id: "col1", name: "Collection 1" } as CollectionView, + { id: "col2", name: "Collection 2" } as CollectionView, + ]; + + organizationServiceMock.organizations$ = new BehaviorSubject([mockOrg]); + collectionService.decryptedCollections$ = new BehaviorSubject(mockCollections); + testBed = TestBed.configureTestingModule({ providers: [ { provide: CipherService, useValue: cipherServiceMock }, @@ -70,17 +94,35 @@ describe("VaultPopupItemsService", () => { { provide: SearchService, useValue: searchService }, { provide: OrganizationService, useValue: organizationServiceMock }, { provide: VaultPopupListFiltersService, useValue: vaultPopupListFiltersServiceMock }, + { provide: CollectionService, useValue: collectionService }, ], }); service = testBed.inject(VaultPopupItemsService); }); + afterEach(() => { + jest.clearAllMocks(); + }); + it("should be created", () => { service = testBed.inject(VaultPopupItemsService); expect(service).toBeTruthy(); }); + it("should merge cipher views with collections and organization", (done) => { + const cipherList = Object.values(allCiphers); + cipherList[0].organizationId = "org1"; + cipherList[0].collectionIds = ["col1", "col2"]; + + service.autoFillCiphers$.subscribe((ciphers) => { + expect(ciphers[0].organization).toEqual(mockOrg); + expect(ciphers[0].collections).toContain(mockCollections[0]); + expect(ciphers[0].collections).toContain(mockCollections[1]); + done(); + }); + }); + describe("autoFillCiphers$", () => { it("should return empty array if there is no current tab", (done) => { jest.spyOn(BrowserApi, "getTabFromCurrentWindow").mockResolvedValue(null); @@ -144,19 +186,18 @@ describe("VaultPopupItemsService", () => { }); it("should filter autoFillCiphers$ down to search term", (done) => { - const cipherList = Object.values(allCiphers); const searchText = "Login"; - searchService.searchCiphers.mockImplementation(async () => { - return cipherList.filter((cipher) => { + searchService.searchCiphers.mockImplementation(async (q, _, ciphers) => { + return ciphers.filter((cipher) => { return cipher.name.includes(searchText); }); }); - // there is only 1 Login returned for filteredCiphers. but two results expected because of other autofill types + // there is only 1 Login returned for filteredCiphers. service.autoFillCiphers$.subscribe((ciphers) => { expect(ciphers[0].name.includes(searchText)).toBe(true); - expect(ciphers.length).toBe(2); + expect(ciphers.length).toBe(1); done(); }); }); diff --git a/apps/browser/src/vault/popup/services/vault-popup-items.service.ts b/apps/browser/src/vault/popup/services/vault-popup-items.service.ts index 3fe509dc45..eacb8e013e 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-items.service.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-items.service.ts @@ -16,7 +16,9 @@ import { import { SearchService } from "@bitwarden/common/abstractions/search.service"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { Utils } from "@bitwarden/common/platform/misc/utils"; +import { CollectionId, OrganizationId } from "@bitwarden/common/types/guid"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; +import { CollectionService } from "@bitwarden/common/vault/abstractions/collection.service"; import { VaultSettingsService } from "@bitwarden/common/vault/abstractions/vault-settings/vault-settings.service"; import { CipherType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; @@ -24,6 +26,7 @@ import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; import { BrowserApi } from "../../../platform/browser/browser-api"; import { runInsideAngular } from "../../../platform/browser/run-inside-angular.operator"; import BrowserPopupUtils from "../../../platform/popup/browser-popup-utils"; +import { PopupCipherView } from "../views/popup-cipher.view"; import { MY_VAULT_ID, VaultPopupListFiltersService } from "./vault-popup-list-filters.service"; @@ -74,14 +77,33 @@ export class VaultPopupItemsService { * Observable that contains the list of all decrypted ciphers. * @private */ - private _cipherList$: Observable = this.cipherService.ciphers$.pipe( + private _cipherList$: Observable = this.cipherService.ciphers$.pipe( runInsideAngular(inject(NgZone)), // Workaround to ensure cipher$ state provider emissions are run inside Angular switchMap(() => Utils.asyncToObservable(() => this.cipherService.getAllDecrypted())), map((ciphers) => Object.values(ciphers)), + switchMap((ciphers) => + combineLatest([ + this.organizationService.organizations$, + this.collectionService.decryptedCollections$, + ]).pipe( + map(([organizations, collections]) => { + const orgMap = Object.fromEntries(organizations.map((org) => [org.id, org])); + const collectionMap = Object.fromEntries(collections.map((col) => [col.id, col])); + return ciphers.map( + (cipher) => + new PopupCipherView( + cipher, + cipher.collectionIds?.map((colId) => collectionMap[colId as CollectionId]), + orgMap[cipher.organizationId as OrganizationId], + ), + ); + }), + ), + ), shareReplay({ refCount: true, bufferSize: 1 }), ); - private _filteredCipherList$: Observable = combineLatest([ + private _filteredCipherList$: Observable = combineLatest([ this._cipherList$, this.searchText$, this.vaultPopupListFiltersService.filterFunction$, @@ -90,8 +112,9 @@ export class VaultPopupItemsService { filterFunction(ciphers), searchText, ]), - switchMap(([ciphers, searchText]) => - this.searchService.searchCiphers(searchText, null, ciphers), + switchMap( + ([ciphers, searchText]) => + this.searchService.searchCiphers(searchText, null, ciphers) as Promise, ), shareReplay({ refCount: true, bufferSize: 1 }), ); @@ -102,7 +125,7 @@ export class VaultPopupItemsService { * * See {@link refreshCurrentTab} to trigger re-evaluation of the current tab. */ - autoFillCiphers$: Observable = combineLatest([ + autoFillCiphers$: Observable = combineLatest([ this._filteredCipherList$, this._otherAutoFillTypes$, this._currentAutofillTab$, @@ -121,7 +144,7 @@ export class VaultPopupItemsService { * List of favorite ciphers that are not currently suggested for autofill. * Ciphers are sorted by last used date, then by name. */ - favoriteCiphers$: Observable = combineLatest([ + favoriteCiphers$: Observable = combineLatest([ this.autoFillCiphers$, this._filteredCipherList$, ]).pipe( @@ -138,7 +161,7 @@ export class VaultPopupItemsService { * List of all remaining ciphers that are not currently suggested for autofill or marked as favorite. * Ciphers are sorted by name. */ - remainingCiphers$: Observable = combineLatest([ + remainingCiphers$: Observable = combineLatest([ this.autoFillCiphers$, this.favoriteCiphers$, this._filteredCipherList$, @@ -208,6 +231,7 @@ export class VaultPopupItemsService { private vaultPopupListFiltersService: VaultPopupListFiltersService, private organizationService: OrganizationService, private searchService: SearchService, + private collectionService: CollectionService, ) {} /** diff --git a/apps/browser/src/vault/popup/views/popup-cipher.view.ts b/apps/browser/src/vault/popup/views/popup-cipher.view.ts new file mode 100644 index 0000000000..4707eb9eb0 --- /dev/null +++ b/apps/browser/src/vault/popup/views/popup-cipher.view.ts @@ -0,0 +1,41 @@ +import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { ProductType } from "@bitwarden/common/enums"; +import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { CollectionView } from "@bitwarden/common/vault/models/view/collection.view"; + +/** + * Extended cipher view for the popup. Includes the associated collections and organization + * if applicable. + */ +export class PopupCipherView extends CipherView { + collections?: CollectionView[]; + organization?: Organization; + + constructor( + cipher: CipherView, + collections: CollectionView[] = null, + organization: Organization = null, + ) { + super(); + Object.assign(this, cipher); + this.collections = collections; + this.organization = organization; + } + + /** + * Get the bwi icon for the cipher according to the organization type. + */ + get orgIcon(): "bwi-family" | "bwi-business" | null { + switch (this.organization?.planProductType) { + case ProductType.Free: + case ProductType.Families: + return "bwi-family"; + case ProductType.Teams: + case ProductType.Enterprise: + case ProductType.TeamsStarter: + return "bwi-business"; + default: + return null; + } + } +} diff --git a/libs/common/src/vault/services/cipher.service.ts b/libs/common/src/vault/services/cipher.service.ts index 9b9841169f..ff99c8b1d8 100644 --- a/libs/common/src/vault/services/cipher.service.ts +++ b/libs/common/src/vault/services/cipher.service.ts @@ -1,4 +1,4 @@ -import { firstValueFrom, map, Observable, share, skipWhile, switchMap } from "rxjs"; +import { firstValueFrom, map, Observable, skipWhile, switchMap } from "rxjs"; import { SemVer } from "semver"; import { ApiService } from "../../abstractions/api.service"; @@ -125,13 +125,7 @@ export class CipherService implements CipherServiceAbstraction { switchMap(() => this.encryptedCiphersState.state$), map((ciphers) => ciphers ?? {}), ); - this.cipherViews$ = this.decryptedCiphersState.state$.pipe( - map((views) => views ?? {}), - - share({ - resetOnRefCountZero: true, - }), - ); + this.cipherViews$ = this.decryptedCiphersState.state$.pipe(map((views) => views ?? {})); this.addEditCipherInfo$ = this.addEditCipherInfoState.state$; } From 90ca4345b3fdbab4dcb10176f90a188453d60df2 Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Tue, 4 Jun 2024 14:45:10 -0700 Subject: [PATCH 12/13] Larger min-w to anonlayout content container (#9502) * add larger min-w to content container * increase min-w --- .../anon-layout/anon-layout.component.html | 6 ++++-- .../angular/anon-layout/anon-layout.stories.ts | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/libs/auth/src/angular/anon-layout/anon-layout.component.html b/libs/auth/src/angular/anon-layout/anon-layout.component.html index bf5edbda82..b6eeb70d5d 100644 --- a/libs/auth/src/angular/anon-layout/anon-layout.component.html +++ b/libs/auth/src/angular/anon-layout/anon-layout.component.html @@ -13,9 +13,11 @@

{{ subtitle }}

-
+
diff --git a/libs/auth/src/angular/anon-layout/anon-layout.stories.ts b/libs/auth/src/angular/anon-layout/anon-layout.stories.ts index 82ca846afb..c9054fb5e6 100644 --- a/libs/auth/src/angular/anon-layout/anon-layout.stories.ts +++ b/libs/auth/src/angular/anon-layout/anon-layout.stories.ts @@ -119,6 +119,24 @@ export const WithLongContent: Story = { }), }; +export const WithThinPrimaryContent: Story = { + render: (args) => ({ + props: args, + template: + // Projected content (the
's) and styling is just a sample and can be replaced with any content/styling. + ` + +
Lorem ipsum
+ +
+
Secondary Projected Content (optional)
+ +
+ + `, + }), +}; + export const WithIcon: Story = { render: (args) => ({ props: args, From 3154d219259e080be25c92217cc84d413d9af82c Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Tue, 4 Jun 2024 18:42:04 -0400 Subject: [PATCH 13/13] [PM-8285] Resolve app id race (#9501) * Do not update appId if it is not null * Prefer linear transformations to side-effect-based changes This leaves us open to repeat emits due to updates, but distinct until changed stops those. Tracker improvements are due to passed in observables with replay causing immediate emits when `expectingEmission`s. This converts to a cold observable that only emits when the tracked observable does _after_ subscribing. * Prefer while * PR review --- libs/common/spec/observable-tracker.ts | 31 ++++--- .../platform/services/app-id.service.spec.ts | 82 +++++++++++-------- .../src/platform/services/app-id.service.ts | 22 +++-- 3 files changed, 80 insertions(+), 55 deletions(-) diff --git a/libs/common/spec/observable-tracker.ts b/libs/common/spec/observable-tracker.ts index 16fad869c3..9bf0475bee 100644 --- a/libs/common/spec/observable-tracker.ts +++ b/libs/common/spec/observable-tracker.ts @@ -1,10 +1,11 @@ -import { Observable, Subscription, firstValueFrom, throwError, timeout } from "rxjs"; +import { Observable, Subject, Subscription, firstValueFrom, throwError, timeout } from "rxjs"; /** Test class to enable async awaiting of observable emissions */ export class ObservableTracker { private subscription: Subscription; + private emissionReceived = new Subject(); emissions: T[] = []; - constructor(private observable: Observable) { + constructor(observable: Observable) { this.emissions = this.trackEmissions(observable); } @@ -21,7 +22,7 @@ export class ObservableTracker { */ async expectEmission(msTimeout = 50): Promise { return await firstValueFrom( - this.observable.pipe( + this.emissionReceived.pipe( timeout({ first: msTimeout, with: () => throwError(() => new Error("Timeout exceeded waiting for another emission.")), @@ -34,40 +35,38 @@ export class ObservableTracker { * @param count The number of emissions to wait for */ async pauseUntilReceived(count: number, msTimeout = 50): Promise { - for (let i = 0; i < count - this.emissions.length; i++) { + while (this.emissions.length < count) { await this.expectEmission(msTimeout); } return this.emissions; } - private trackEmissions(observable: Observable): T[] { + private trackEmissions(observable: Observable): T[] { const emissions: T[] = []; this.subscription = observable.subscribe((value) => { - switch (value) { - case undefined: - case null: - emissions.push(value); - return; - default: - // process by type - break; + if (value == null) { + this.emissionReceived.next(null); + return; } switch (typeof value) { case "string": case "number": case "boolean": - emissions.push(value); + this.emissionReceived.next(value); break; case "symbol": // Cheating types to make symbols work at all - emissions.push(value.toString() as T); + this.emissionReceived.next(value as T); break; default: { - emissions.push(clone(value)); + this.emissionReceived.next(clone(value)); } } }); + this.emissionReceived.subscribe((value) => { + emissions.push(value); + }); return emissions; } } diff --git a/libs/common/src/platform/services/app-id.service.spec.ts b/libs/common/src/platform/services/app-id.service.spec.ts index 10fb153fda..62806204db 100644 --- a/libs/common/src/platform/services/app-id.service.spec.ts +++ b/libs/common/src/platform/services/app-id.service.spec.ts @@ -1,20 +1,23 @@ -import { FakeGlobalStateProvider } from "../../../spec"; +import { FakeGlobalState, FakeGlobalStateProvider, ObservableTracker } from "../../../spec"; import { Utils } from "../misc/utils"; import { ANONYMOUS_APP_ID_KEY, APP_ID_KEY, AppIdService } from "./app-id.service"; describe("AppIdService", () => { - const globalStateProvider = new FakeGlobalStateProvider(); - const appIdState = globalStateProvider.getFake(APP_ID_KEY); - const anonymousAppIdState = globalStateProvider.getFake(ANONYMOUS_APP_ID_KEY); + let globalStateProvider: FakeGlobalStateProvider; + let appIdState: FakeGlobalState; + let anonymousAppIdState: FakeGlobalState; let sut: AppIdService; beforeEach(() => { + globalStateProvider = new FakeGlobalStateProvider(); + appIdState = globalStateProvider.getFake(APP_ID_KEY); + anonymousAppIdState = globalStateProvider.getFake(ANONYMOUS_APP_ID_KEY); sut = new AppIdService(globalStateProvider); }); afterEach(() => { - jest.restoreAllMocks(); + jest.resetAllMocks(); }); describe("getAppId", () => { @@ -26,19 +29,18 @@ describe("AppIdService", () => { expect(appId).toBe("existingAppId"); }); - it.each([null, undefined])( - "uses the util function to create a new id when it AppId does not exist", - async (value) => { - appIdState.stateSubject.next(value); - const spy = jest.spyOn(Utils, "newGuid"); + it("creates a new appId only once", async () => { + appIdState.stateSubject.next(null); - await sut.getAppId(); + const appIds: string[] = []; + const promises = [async () => appIds.push(await sut.getAppId())]; + promises.push(async () => appIds.push(await sut.getAppId())); + await Promise.all(promises); - expect(spy).toHaveBeenCalledTimes(1); - }, - ); + expect(appIds[0]).toBe(appIds[1]); + }); - it.each([null, undefined])("returns a new appId when it does not exist", async (value) => { + it.each([null, undefined])("returns a new appId when %s", async (value) => { appIdState.stateSubject.next(value); const appId = await sut.getAppId(); @@ -46,16 +48,23 @@ describe("AppIdService", () => { expect(appId).toMatch(Utils.guidRegex); }); - it.each([null, undefined])( - "stores the new guid when it an existing one is not found", - async (value) => { - appIdState.stateSubject.next(value); + it.each([null, undefined])("stores the new guid when %s", async (value) => { + appIdState.stateSubject.next(value); - const appId = await sut.getAppId(); + const appId = await sut.getAppId(); - expect(appIdState.nextMock).toHaveBeenCalledWith(appId); - }, - ); + expect(appIdState.nextMock).toHaveBeenCalledWith(appId); + }); + + it("emits only once when creating a new appId", async () => { + appIdState.stateSubject.next(null); + + const tracker = new ObservableTracker(sut.appId$); + const appId = await sut.getAppId(); + + expect(tracker.emissions).toEqual([appId]); + await expect(tracker.pauseUntilReceived(2, 50)).rejects.toThrow("Timeout exceeded"); + }); }); describe("getAnonymousAppId", () => { @@ -67,17 +76,16 @@ describe("AppIdService", () => { expect(appId).toBe("existingAppId"); }); - it.each([null, undefined])( - "uses the util function to create a new id when it AppId does not exist", - async (value) => { - anonymousAppIdState.stateSubject.next(value); - const spy = jest.spyOn(Utils, "newGuid"); + it("creates a new anonymousAppId only once", async () => { + anonymousAppIdState.stateSubject.next(null); - await sut.getAnonymousAppId(); + const appIds: string[] = []; + const promises = [async () => appIds.push(await sut.getAnonymousAppId())]; + promises.push(async () => appIds.push(await sut.getAnonymousAppId())); + await Promise.all(promises); - expect(spy).toHaveBeenCalledTimes(1); - }, - ); + expect(appIds[0]).toBe(appIds[1]); + }); it.each([null, undefined])("returns a new appId when it does not exist", async (value) => { anonymousAppIdState.stateSubject.next(value); @@ -97,5 +105,15 @@ describe("AppIdService", () => { expect(anonymousAppIdState.nextMock).toHaveBeenCalledWith(appId); }, ); + + it("emits only once when creating a new anonymousAppId", async () => { + anonymousAppIdState.stateSubject.next(null); + + const tracker = new ObservableTracker(sut.anonymousAppId$); + const appId = await sut.getAnonymousAppId(); + + expect(tracker.emissions).toEqual([appId]); + await expect(tracker.pauseUntilReceived(2, 50)).rejects.toThrow("Timeout exceeded"); + }); }); }); diff --git a/libs/common/src/platform/services/app-id.service.ts b/libs/common/src/platform/services/app-id.service.ts index 630e629749..56e9516bce 100644 --- a/libs/common/src/platform/services/app-id.service.ts +++ b/libs/common/src/platform/services/app-id.service.ts @@ -1,4 +1,4 @@ -import { Observable, filter, firstValueFrom, tap } from "rxjs"; +import { Observable, concatMap, distinctUntilChanged, firstValueFrom, share } from "rxjs"; import { AppIdService as AppIdServiceAbstraction } from "../abstractions/app-id.service"; import { Utils } from "../misc/utils"; @@ -19,20 +19,28 @@ export class AppIdService implements AppIdServiceAbstraction { const appIdState = globalStateProvider.get(APP_ID_KEY); const anonymousAppIdState = globalStateProvider.get(ANONYMOUS_APP_ID_KEY); this.appId$ = appIdState.state$.pipe( - tap(async (appId) => { + concatMap(async (appId) => { if (!appId) { - await appIdState.update(() => Utils.newGuid()); + return await appIdState.update(() => Utils.newGuid(), { + shouldUpdate: (v) => v == null, + }); } + return appId; }), - filter((appId) => !!appId), + distinctUntilChanged(), + share(), ); this.anonymousAppId$ = anonymousAppIdState.state$.pipe( - tap(async (appId) => { + concatMap(async (appId) => { if (!appId) { - await anonymousAppIdState.update(() => Utils.newGuid()); + return await anonymousAppIdState.update(() => Utils.newGuid(), { + shouldUpdate: (v) => v == null, + }); } + return appId; }), - filter((appId) => !!appId), + distinctUntilChanged(), + share(), ); }