From 9a35608fc3b151a35dcdc3d7d66561b95e130775 Mon Sep 17 00:00:00 2001 From: Opeyemi Date: Tue, 11 Jun 2024 15:31:37 +0100 Subject: [PATCH 1/8] Revert "restrict deployment to USDEV and protect environment (#9571)" (#9583) This reverts commit f9faeeba4c9a928cf00936951484c6673786cf9d. --- .github/workflows/deploy-web.yml | 62 +------------------------------- 1 file changed, 1 insertion(+), 61 deletions(-) diff --git a/.github/workflows/deploy-web.yml b/.github/workflows/deploy-web.yml index 52230a12bc..1ff6767141 100644 --- a/.github/workflows/deploy-web.yml +++ b/.github/workflows/deploy-web.yml @@ -112,48 +112,13 @@ jobs: echo "azure-login-creds=AZURE_KV_US_DEV_SERVICE_PRINCIPAL" >> $GITHUB_OUTPUT echo "retrieve-secrets-keyvault=webvault-eastus-dev" >> $GITHUB_OUTPUT echo "environment-artifact=web-*-cloud-usdev.zip" >> $GITHUB_OUTPUT - echo "environment-name=Web Vault - US DEV Cloud" >> $GITHUB_OUTPUT + echo "environment-name=Web Vault - US Development Cloud" >> $GITHUB_OUTPUT echo "environment-url=http://vault.$ENV_NAME_LOWER.bitwarden.pw" >> $GITHUB_OUTPUT ;; esac # Set the sync utility to use for deployment to the environment (az-sync or azcopy) echo "sync-utility=azcopy" >> $GITHUB_OUTPUT - - name: Environment Protection - env: - TAG: ${{ steps.project_tag.outputs.tag }} - run: | - BRANCH_OR_TAG_LOWER=$(echo ${{ inputs.branch-or-tag }} | awk '{print tolower($0)}') - - PROD_ENV_PATTERN='USPROD|EUPROD' - PROD_ALLOWED_TAGS_PATTERN='web-v[0-9]+\.[0-9]+\.[0-9]+' - - QA_ENV_PATTERN='USQA|EUQA' - QA_ALLOWED_TAGS_PATTERN='.*' - - DEV_ENV_PATTERN='USDEV' - DEV_ALLOWED_TAGS_PATTERN='.*' - - if [[ \ - ${{ inputs.environment }} =~ \.*($PROD_ENV_PATTERN)\.* && \ - ! "$BRANCH_OR_TAG_LOWER" =~ ^($PROD_ALLOWED_TAGS_PATTERN).* \ - ]] || [[ \ - ${{ inputs.environment }} =~ \.*($QA_ENV_PATTERN)\.* && \ - ! "$BRANCH_OR_TAG_LOWER" =~ ^($QA_ALLOWED_TAGS_PATTERN).* \ - ]] || [[ \ - =~ \.*($DEV_ENV_PATTERN)\.* && \ - ! "$BRANCH_OR_TAG_LOWER" =~ ^($DEV_ALLOWED_TAGS_PATTERN).* \ - ]]; then - echo "!Deployment blocked!" - echo "Attempting to deploy a tag that is not allowed in ${{ inputs.environment }} environment" - echo - echo "Environment: ${{ inputs.environment }} - echo "Tag: ${{ inputs.branch-or-tag }} - exit 1 - else - echo "${{ inputs.branch-or-tag }} is allowed to deployed on to ${{ inputs.environment }} environment" - fi - approval: name: Approval for Deployment to ${{ needs.setup.outputs.environment-name }} needs: setup @@ -241,31 +206,6 @@ jobs: echo "commit=${{ steps.download-latest-artifacts.outputs.artifact-build-commit }}" >> $GITHUB_OUTPUT fi - - name: Ensure artifact is from main branch for USDEV environment - if: ${{ 'inputs.environment' == 'USDEV'}} - run: | - # If run-id was used - if [ "${{ inputs.build-web-run-id }}" ]; then - if [ "${{ steps.download-latest-artifacts.outputs.artifact-build-branch }}" != "main" ]; then - echo "Artifact is not from main branch" - exit 1 - fi - - # If artifact download failed - elif [ "${{ steps.download-latest-artifacts.outcome }}" == "failure" ]; then - branch=$(gh api /repos/bitwarden/clients/actions/runs/${{ steps.trigger-build-web.outputs.workflow_id }}/artifacts --jq '.artifacts[0].workflow_run.head_branch') - if [ "$branch" != "main" ]; then - echo "Artifact is not from main branch" - exit 1 - fi - - else - if [ "${{ steps.download-latest-artifacts.outputs.artifact-build-branch }}" != "main" ]; then - echo "Artifact is not from main branch" - exit 1 - fi - fi - notify-start: name: Notify Slack with start message needs: From f6702cd2d74c32dda2d60bc6df0e46b563978867 Mon Sep 17 00:00:00 2001 From: Alex Morask <144709477+amorask-bitwarden@users.noreply.github.com> Date: Tue, 11 Jun 2024 10:36:31 -0400 Subject: [PATCH 2/8] [AC-2595] [AC-2596] Empty clients placeholder and setup provider hint (#9505) * Added empty state to providers clients page * Added bitForm to Setup component and added billing email hint --- apps/web/src/locales/en/messages.json | 7 + .../providers/providers.module.ts | 2 + .../providers/setup/setup.component.html | 55 +++--- .../providers/setup/setup.component.ts | 173 ++++++++++-------- .../app/billing/providers/clients/index.ts | 1 + ...manage-client-organizations.component.html | 149 ++++++++------- .../providers/clients/no-clients.component.ts | 40 ++++ .../src/app/billing/providers/index.ts | 5 +- .../provider-payment-method.component.html | 2 +- .../manage-tax-information.component.html | 4 +- .../manage-tax-information.component.ts | 69 ++++--- 11 files changed, 304 insertions(+), 203 deletions(-) create mode 100644 bitwarden_license/bit-web/src/app/billing/providers/clients/no-clients.component.ts diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index 39423cbd90..9cbd44a963 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -8330,5 +8330,12 @@ }, "viewSecret": { "message": "View secret" + }, + "noClients": { + "message": "There are no clients to list" + }, + "providerBillingEmailHint": { + "message": "This email address will receive all invoices pertaining to this provider", + "description": "A hint that shows up on the Provider setup page to inform the admin the billing email will receive the provider's invoices." } } diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/providers.module.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/providers.module.ts index baa3e5e1bb..00a3872584 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/providers.module.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/providers.module.ts @@ -11,6 +11,7 @@ import { OssModule } from "@bitwarden/web-vault/app/oss.module"; import { CreateClientOrganizationComponent, + NoClientsComponent, ManageClientOrganizationNameComponent, ManageClientOrganizationsComponent, ManageClientOrganizationSubscriptionComponent, @@ -65,6 +66,7 @@ import { SetupComponent } from "./setup/setup.component"; SetupProviderComponent, UserAddEditComponent, CreateClientOrganizationComponent, + NoClientsComponent, ManageClientOrganizationsComponent, ManageClientOrganizationNameComponent, ManageClientOrganizationSubscriptionComponent, diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/setup/setup.component.html b/bitwarden_license/bit-web/src/app/admin-console/providers/setup/setup.component.html index d1cf666874..0fd6725304 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/setup/setup.component.html +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/setup/setup.component.html @@ -1,40 +1,41 @@ -
+ + + {{ "loading" | i18n }} + +

{{ "setupProvider" | i18n }}

{{ "setupProviderDesc" | i18n }}

- -
+

{{ "generalInformation" | i18n }}

-
-
- - +
+
+ + {{ "providerName" | i18n }} + +
-
- - -
-
- +
+ + {{ "billingEmail" | i18n }} + + {{ + "providerBillingEmailHint" | i18n + }} +
- -
- -
+ +
diff --git a/bitwarden_license/bit-web/src/app/admin-console/providers/setup/setup.component.ts b/bitwarden_license/bit-web/src/app/admin-console/providers/setup/setup.component.ts index 258088257d..845f2834b3 100644 --- a/bitwarden_license/bit-web/src/app/admin-console/providers/setup/setup.component.ts +++ b/bitwarden_license/bit-web/src/app/admin-console/providers/setup/setup.component.ts @@ -1,38 +1,41 @@ -import { Component, OnInit, ViewChild } from "@angular/core"; +import { Component, OnDestroy, OnInit, ViewChild } from "@angular/core"; +import { FormBuilder, Validators } from "@angular/forms"; import { ActivatedRoute, Router } from "@angular/router"; -import { firstValueFrom } from "rxjs"; -import { first } from "rxjs/operators"; +import { firstValueFrom, Subject, switchMap } from "rxjs"; +import { first, takeUntil } from "rxjs/operators"; +import { ManageTaxInformationComponent } from "@bitwarden/angular/billing/components"; import { ProviderApiServiceAbstraction } from "@bitwarden/common/admin-console/abstractions/provider/provider-api.service.abstraction"; import { ProviderSetupRequest } from "@bitwarden/common/admin-console/models/request/provider/provider-setup.request"; +import { TaxInformation } from "@bitwarden/common/billing/models/domain"; import { ExpandedTaxInfoUpdateRequest } from "@bitwarden/common/billing/models/request/expanded-tax-info-update.request"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { CryptoService } from "@bitwarden/common/platform/abstractions/crypto.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; -import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; import { ProviderKey } from "@bitwarden/common/types/key"; import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction"; -import { TaxInfoComponent } from "@bitwarden/web-vault/app/billing"; +import { ToastService } from "@bitwarden/components"; @Component({ selector: "provider-setup", templateUrl: "setup.component.html", }) -// eslint-disable-next-line rxjs-angular/prefer-takeuntil -export class SetupComponent implements OnInit { - @ViewChild(TaxInfoComponent) taxInfoComponent: TaxInfoComponent; +export class SetupComponent implements OnInit, OnDestroy { + @ViewChild(ManageTaxInformationComponent) + manageTaxInformationComponent: ManageTaxInformationComponent; loading = true; - authed = false; - email: string; - formPromise: Promise; - providerId: string; token: string; - name: string; - billingEmail: string; + + protected formGroup = this.formBuilder.group({ + name: ["", Validators.required], + billingEmail: ["", [Validators.required, Validators.email]], + }); + + protected readonly TaxInformation = TaxInformation; protected showPaymentMethodWarningBanners$ = this.configService.getFeatureFlag$( FeatureFlag.ShowPaymentMethodWarningBanners, @@ -42,9 +45,10 @@ export class SetupComponent implements OnInit { FeatureFlag.EnableConsolidatedBilling, ); + private destroy$ = new Subject(); + constructor( private router: Router, - private platformUtilsService: PlatformUtilsService, private i18nService: I18nService, private route: ActivatedRoute, private cryptoService: CryptoService, @@ -52,61 +56,81 @@ export class SetupComponent implements OnInit { private validationService: ValidationService, private configService: ConfigService, private providerApiService: ProviderApiServiceAbstraction, + private formBuilder: FormBuilder, + private toastService: ToastService, ) {} ngOnInit() { document.body.classList.remove("layout_frontend"); - // eslint-disable-next-line rxjs-angular/prefer-takeuntil, rxjs/no-async-subscribe - this.route.queryParams.pipe(first()).subscribe(async (qParams) => { - const error = qParams.providerId == null || qParams.email == null || qParams.token == null; - if (error) { - this.platformUtilsService.showToast( - "error", - null, - this.i18nService.t("emergencyInviteAcceptFailed"), - { timeout: 10000 }, - ); - // 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(["/"]); + this.route.queryParams + .pipe( + first(), + switchMap(async (queryParams) => { + const error = + queryParams.providerId == null || + queryParams.email == null || + queryParams.token == null; + + if (error) { + this.toastService.showToast({ + variant: "error", + title: null, + message: this.i18nService.t("emergencyInviteAcceptFailed"), + timeout: 10000, + }); + + return await this.router.navigate(["/"]); + } + + this.providerId = queryParams.providerId; + this.token = queryParams.token; + + try { + const provider = await this.providerApiService.getProvider(this.providerId); + + if (provider.name != null) { + /* + This is currently always going to result in a redirect to the Vault because the `provider-permissions.guard` + checks for the existence of the Provider in state. However, when accessing the Setup page via the email link, + this `navigate` invocation will be hit before the sync can complete, thus resulting in a null Provider. If we want + to resolve it, we'd either need to use the ProviderApiService in the provider-permissions.guard (added expense) + or somehow check that the previous route was /setup. + */ + return await this.router.navigate(["/providers", provider.id], { + replaceUrl: true, + }); + } + this.loading = false; + } catch (error) { + this.validationService.showError(error); + return await this.router.navigate(["/"]); + } + }), + takeUntil(this.destroy$), + ) + .subscribe(); + } + + ngOnDestroy() { + this.destroy$.next(); + this.destroy$.complete(); + } + + submit = async () => { + try { + this.formGroup.markAllAsTouched(); + const taxInformationValid = this.manageTaxInformationComponent.touch(); + if (this.formGroup.invalid || !taxInformationValid) { return; } - this.providerId = qParams.providerId; - this.token = qParams.token; - - // Check if provider exists, redirect if it does - try { - const provider = await this.providerApiService.getProvider(this.providerId); - if (provider.name != null) { - // 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(["/providers", provider.id], { replaceUrl: true }); - } - } catch (e) { - this.validationService.showError(e); - // 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(["/"]); - } - }); - } - - async submit() { - this.formPromise = this.doSubmit(); - await this.formPromise; - this.formPromise = null; - } - - async doSubmit() { - try { const providerKey = await this.cryptoService.makeOrgKey(); const key = providerKey[0].encryptedString; const request = new ProviderSetupRequest(); - request.name = this.name; - request.billingEmail = this.billingEmail; + request.name = this.formGroup.value.name; + request.billingEmail = this.formGroup.value.billingEmail; request.token = this.token; request.key = key; @@ -114,27 +138,32 @@ export class SetupComponent implements OnInit { if (enableConsolidatedBilling) { request.taxInfo = new ExpandedTaxInfoUpdateRequest(); - const taxInfoView = this.taxInfoComponent.taxInfo; - request.taxInfo.country = taxInfoView.country; - request.taxInfo.postalCode = taxInfoView.postalCode; - if (taxInfoView.includeTaxId) { - request.taxInfo.taxId = taxInfoView.taxId; - request.taxInfo.line1 = taxInfoView.line1; - request.taxInfo.line2 = taxInfoView.line2; - request.taxInfo.city = taxInfoView.city; - request.taxInfo.state = taxInfoView.state; + const taxInformation = this.manageTaxInformationComponent.getTaxInformation(); + + request.taxInfo.country = taxInformation.country; + request.taxInfo.postalCode = taxInformation.postalCode; + if (taxInformation.includeTaxId) { + request.taxInfo.taxId = taxInformation.taxId; + request.taxInfo.line1 = taxInformation.line1; + request.taxInfo.line2 = taxInformation.line2; + request.taxInfo.city = taxInformation.city; + request.taxInfo.state = taxInformation.state; } } const provider = await this.providerApiService.postProviderSetup(this.providerId, request); - this.platformUtilsService.showToast("success", null, this.i18nService.t("providerSetup")); + + this.toastService.showToast({ + variant: "success", + title: null, + message: this.i18nService.t("providerSetup"), + }); + await this.syncService.fullSync(true); - // 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(["/providers", provider.id]); + await this.router.navigate(["/providers", provider.id]); } catch (e) { this.validationService.showError(e); } - } + }; } diff --git a/bitwarden_license/bit-web/src/app/billing/providers/clients/index.ts b/bitwarden_license/bit-web/src/app/billing/providers/clients/index.ts index 1968302766..fa1bc137fc 100644 --- a/bitwarden_license/bit-web/src/app/billing/providers/clients/index.ts +++ b/bitwarden_license/bit-web/src/app/billing/providers/clients/index.ts @@ -2,3 +2,4 @@ export * from "./create-client-organization.component"; export * from "./manage-client-organizations.component"; export * from "./manage-client-organization-name.component"; export * from "./manage-client-organization-subscription.component"; +export * from "./no-clients.component"; diff --git a/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-client-organizations.component.html b/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-client-organizations.component.html index d2f8ab7a85..9a84b92837 100644 --- a/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-client-organizations.component.html +++ b/bitwarden_license/bit-web/src/app/billing/providers/clients/manage-client-organizations.component.html @@ -21,80 +21,77 @@ {{ "loading" | i18n }} - -

{{ "noClientsInList" | i18n }}

- - - - - {{ "client" | i18n }} - {{ "assigned" | i18n }} - {{ "used" | i18n }} - {{ "remaining" | i18n }} - {{ "billingPlan" | i18n }} - - - - - - - - - -
- {{ - client.organizationName - }} -
- - - {{ client.seats }} - - - {{ client.userCount }} - - - {{ client.seats - client.userCount }} - - - {{ client.plan }} - - - - - - - - - - - - - + + + + + {{ "client" | i18n }} + {{ "assigned" | i18n }} + {{ "used" | i18n }} + {{ "remaining" | i18n }} + {{ "billingPlan" | i18n }} + + + + + + + + + +
+ {{ + client.organizationName + }} +
+ + + {{ client.seats }} + + + {{ client.userCount }} + + + {{ client.seats - client.userCount }} + + + {{ client.plan }} + + + + + + + + + + + + +
+ +
diff --git a/bitwarden_license/bit-web/src/app/billing/providers/clients/no-clients.component.ts b/bitwarden_license/bit-web/src/app/billing/providers/clients/no-clients.component.ts new file mode 100644 index 0000000000..c785ee8bd0 --- /dev/null +++ b/bitwarden_license/bit-web/src/app/billing/providers/clients/no-clients.component.ts @@ -0,0 +1,40 @@ +import { Component, EventEmitter, Output } from "@angular/core"; + +import { svgIcon } from "@bitwarden/components"; + +const gearIcon = svgIcon` + + + + + + + + + + + + + + + + +`; + +@Component({ + selector: "app-no-clients", + template: `
+ +

{{ "noClients" | i18n }}

+ + + {{ "addNewOrganization" | i18n }} + +
`, +}) +export class NoClientsComponent { + icon = gearIcon; + @Output() addNewOrganizationClicked = new EventEmitter(); + + addNewOrganization = () => this.addNewOrganizationClicked.emit(); +} diff --git a/bitwarden_license/bit-web/src/app/billing/providers/index.ts b/bitwarden_license/bit-web/src/app/billing/providers/index.ts index 9b899f1741..71af56f7b0 100644 --- a/bitwarden_license/bit-web/src/app/billing/providers/index.ts +++ b/bitwarden_license/bit-web/src/app/billing/providers/index.ts @@ -1,7 +1,4 @@ -export * from "./clients/create-client-organization.component"; -export * from "./clients/manage-client-organization-name.component"; -export * from "./clients/manage-client-organization-subscription.component"; -export * from "./clients/manage-client-organizations.component"; +export * from "./clients"; export * from "./guards/has-consolidated-billing.guard"; export * from "./payment-method/provider-select-payment-method-dialog.component"; export * from "./payment-method/provider-payment-method.component"; diff --git a/bitwarden_license/bit-web/src/app/billing/providers/payment-method/provider-payment-method.component.html b/bitwarden_license/bit-web/src/app/billing/providers/payment-method/provider-payment-method.component.html index 1a1e0529fc..e2457294eb 100644 --- a/bitwarden_license/bit-web/src/app/billing/providers/payment-method/provider-payment-method.component.html +++ b/bitwarden_license/bit-web/src/app/billing/providers/payment-method/provider-payment-method.component.html @@ -44,7 +44,7 @@

{{ "taxInformationDesc" | i18n }}

diff --git a/libs/angular/src/billing/components/manage-tax-information/manage-tax-information.component.html b/libs/angular/src/billing/components/manage-tax-information/manage-tax-information.component.html index f9cfa8e0fa..0b041bd4c0 100644 --- a/libs/angular/src/billing/components/manage-tax-information/manage-tax-information.component.html +++ b/libs/angular/src/billing/components/manage-tax-information/manage-tax-information.component.html @@ -1,7 +1,7 @@
- + {{ "country" | i18n }}
- + {{ "zipPostalCode" | i18n }} diff --git a/libs/angular/src/billing/components/manage-tax-information/manage-tax-information.component.ts b/libs/angular/src/billing/components/manage-tax-information/manage-tax-information.component.ts index 58342548ca..f048cf0d36 100644 --- a/libs/angular/src/billing/components/manage-tax-information/manage-tax-information.component.ts +++ b/libs/angular/src/billing/components/manage-tax-information/manage-tax-information.component.ts @@ -1,5 +1,6 @@ -import { Component, EventEmitter, Input, OnInit, Output } from "@angular/core"; +import { Component, EventEmitter, Input, OnDestroy, OnInit, Output } from "@angular/core"; import { FormBuilder, Validators } from "@angular/forms"; +import { Subject, takeUntil } from "rxjs"; import { TaxInformation } from "@bitwarden/common/billing/models/domain"; @@ -13,8 +14,8 @@ type Country = { selector: "app-manage-tax-information", templateUrl: "./manage-tax-information.component.html", }) -export class ManageTaxInformationComponent implements OnInit { - @Input({ required: true }) taxInformation: TaxInformation; +export class ManageTaxInformationComponent implements OnInit, OnDestroy { + @Input() startWith: TaxInformation; @Input() onSubmit?: (taxInformation: TaxInformation) => Promise; @Output() taxInformationUpdated = new EventEmitter(); @@ -29,35 +30,61 @@ export class ManageTaxInformationComponent implements OnInit { state: "", }); + private destroy$ = new Subject(); + + private taxInformation: TaxInformation; + constructor(private formBuilder: FormBuilder) {} - submit = async () => { - await this.onSubmit({ - country: this.formGroup.value.country, - postalCode: this.formGroup.value.postalCode, - taxId: this.formGroup.value.taxId, - line1: this.formGroup.value.line1, - line2: this.formGroup.value.line2, - city: this.formGroup.value.city, - state: this.formGroup.value.state, - }); + getTaxInformation = (): TaxInformation & { includeTaxId: boolean } => ({ + ...this.taxInformation, + includeTaxId: this.formGroup.value.includeTaxId, + }); + submit = async () => { + this.formGroup.markAllAsTouched(); + if (this.formGroup.invalid) { + return; + } + await this.onSubmit(this.taxInformation); this.taxInformationUpdated.emit(); }; + touch = (): boolean => { + this.formGroup.markAllAsTouched(); + return this.formGroup.valid; + }; + async ngOnInit() { - if (this.taxInformation) { + if (this.startWith) { this.formGroup.patchValue({ - ...this.taxInformation, + ...this.startWith, includeTaxId: - this.countrySupportsTax(this.taxInformation.country) && - (!!this.taxInformation.taxId || - !!this.taxInformation.line1 || - !!this.taxInformation.line2 || - !!this.taxInformation.city || - !!this.taxInformation.state), + this.countrySupportsTax(this.startWith.country) && + (!!this.startWith.taxId || + !!this.startWith.line1 || + !!this.startWith.line2 || + !!this.startWith.city || + !!this.startWith.state), }); } + + this.formGroup.valueChanges.pipe(takeUntil(this.destroy$)).subscribe((values) => { + this.taxInformation = { + country: values.country, + postalCode: values.postalCode, + taxId: values.taxId, + line1: values.line1, + line2: values.line2, + city: values.city, + state: values.state, + }; + }); + } + + ngOnDestroy() { + this.destroy$.next(); + this.destroy$.complete(); } protected countrySupportsTax(countryCode: string) { From e6803e05ee0d7bae7d5f580dd40ddc02162dbc65 Mon Sep 17 00:00:00 2001 From: Tom <144813356+ttalty@users.noreply.github.com> Date: Tue, 11 Jun 2024 13:36:31 -0400 Subject: [PATCH 3/8] [PM-8593] CLI - Logout needs to reset active account (#9503) * On logging out the account service active account needs set to null * Auth service logout back to old spot and account switch after cleaning the state --- apps/cli/src/service-container.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/cli/src/service-container.ts b/apps/cli/src/service-container.ts index ff4eb52b84..8749eeb982 100644 --- a/apps/cli/src/service-container.ts +++ b/apps/cli/src/service-container.ts @@ -753,6 +753,7 @@ export class ServiceContainer { await this.stateService.clean(); await this.accountService.clean(userId); + await this.accountService.switchAccount(null); process.env.BW_SESSION = null; } From 832abcd955d7de57dcf549d75ed40f0de6103e07 Mon Sep 17 00:00:00 2001 From: vinith-kovan <156108204+vinith-kovan@users.noreply.github.com> Date: Tue, 11 Jun 2024 23:17:55 +0530 Subject: [PATCH 4/8] [PM-2057] update two factor email dialog (#9547) * migrating two factor email component * two factor email component migration * two factor email component migration * two factor email component migration * two factor email component migration --- .../src/app/auth/settings/two-factor-email.component.html | 4 ++-- .../web/src/app/auth/settings/two-factor-email.component.ts | 6 +++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/apps/web/src/app/auth/settings/two-factor-email.component.html b/apps/web/src/app/auth/settings/two-factor-email.component.html index cf1dba9884..d15bde1d93 100644 --- a/apps/web/src/app/auth/settings/two-factor-email.component.html +++ b/apps/web/src/app/auth/settings/two-factor-email.component.html @@ -26,12 +26,12 @@ appInputVerbatim="false" /> -
+
- {{ "verificationCodeEmailSent" | i18n: sentEmail }} + {{ "emailSent" | i18n }}
diff --git a/apps/web/src/app/auth/settings/two-factor-email.component.ts b/apps/web/src/app/auth/settings/two-factor-email.component.ts index 8a5c029223..b0b7c0a64f 100644 --- a/apps/web/src/app/auth/settings/two-factor-email.component.ts +++ b/apps/web/src/app/auth/settings/two-factor-email.component.ts @@ -31,7 +31,7 @@ export class TwoFactorEmailComponent extends TwoFactorBaseComponent { emailPromise: Promise; override componentName = "app-two-factor-email"; formGroup = this.formBuilder.group({ - token: [null], + token: ["", [Validators.required]], email: ["", [Validators.email, Validators.required]], }); @@ -79,6 +79,10 @@ export class TwoFactorEmailComponent extends TwoFactorBaseComponent { } submit = async () => { + this.formGroup.markAllAsTouched(); + if (this.formGroup.invalid) { + return; + } if (this.enabled) { await this.disableEmail(); this.onChangeStatus.emit(false); From 19d863c9efe24764d335495d43f5e9b0890a8def Mon Sep 17 00:00:00 2001 From: vinith-kovan <156108204+vinith-kovan@users.noreply.github.com> Date: Tue, 11 Jun 2024 23:25:58 +0530 Subject: [PATCH 5/8] [PM-4956] two factor component migration (#9204) * two factor component migration * two factor component migration * two factor component migration * two factor component migration * two factor component migration --- .../src/app/auth/two-factor.component.html | 285 ++++++++---------- apps/web/src/app/auth/two-factor.component.ts | 26 +- apps/web/src/app/oss-routing.module.ts | 28 +- apps/web/src/locales/en/messages.json | 3 + 4 files changed, 172 insertions(+), 170 deletions(-) diff --git a/apps/web/src/app/auth/two-factor.component.html b/apps/web/src/app/auth/two-factor.component.html index df86db99ee..7c63553e43 100644 --- a/apps/web/src/app/auth/two-factor.component.html +++ b/apps/web/src/app/auth/two-factor.component.html @@ -1,173 +1,124 @@ - -
-
+
+ -

{{ title }}

-
-
- + {{ "enterVerificationCodeApp" | i18n }} +

+

+ {{ "enterVerificationCodeEmail" | i18n: twoFactorEmail }} +

+ + {{ "verificationCode" | i18n }} + + + -

- {{ "enterVerificationCodeApp" | i18n }} -

-

- {{ "enterVerificationCodeEmail" | i18n: twoFactorEmail }} -

-
- - - - - {{ "sendVerificationCodeEmailAgain" | i18n }} - - -
- - -

{{ "insertYubiKey" | i18n }}

- - - - - -
- - -
- - -
- -
- - - - -

- {{ "duoRequiredByOrgForAccount" | i18n }} -

-

{{ "launchDuoAndFollowStepsToFinishLoggingIn" | i18n }}

- - - -
- -
- - - -
- - -
- -

{{ "noTwoStepProviders" | i18n }}

-

{{ "noTwoStepProviders2" | i18n }}

- -
-
- -
- -
- - - - {{ "cancel" | i18n }} - -
-
- {{ - "useAnotherTwoStepMethod" | i18n - }} -
-
+ {{ "sendVerificationCodeEmailAgain" | i18n }} + + + + +

{{ "insertYubiKey" | i18n }}

+ + + + + + + {{ "verificationCode" | i18n }} + + + + +
+
+ + + + +

+ {{ "duoRequiredByOrgForAccount" | i18n }} +

+

{{ "launchDuoAndFollowStepsToFinishLoggingIn" | i18n }}

+ + + +
+ +
+ + + + {{ "rememberMe" | i18n }} + + + +

{{ "noTwoStepProviders" | i18n }}

+

{{ "noTwoStepProviders2" | i18n }}

+ +
+
+ +
+ +
+ + + + {{ "cancel" | i18n }} + +
+
+ {{ + "useAnotherTwoStepMethod" | i18n + }}
diff --git a/apps/web/src/app/auth/two-factor.component.ts b/apps/web/src/app/auth/two-factor.component.ts index c91ae4674d..741037cc30 100644 --- a/apps/web/src/app/auth/two-factor.component.ts +++ b/apps/web/src/app/auth/two-factor.component.ts @@ -1,6 +1,7 @@ import { Component, Inject, OnDestroy, ViewChild, ViewContainerRef } from "@angular/core"; +import { FormBuilder, Validators } from "@angular/forms"; import { ActivatedRoute, Router } from "@angular/router"; -import { lastValueFrom } from "rxjs"; +import { Subject, takeUntil, lastValueFrom } from "rxjs"; import { TwoFactorComponent as BaseTwoFactorComponent } from "@bitwarden/angular/auth/components/two-factor.component"; import { WINDOW } from "@bitwarden/angular/services/injection-tokens"; @@ -38,7 +39,17 @@ import { export class TwoFactorComponent extends BaseTwoFactorComponent implements OnDestroy { @ViewChild("twoFactorOptions", { read: ViewContainerRef, static: true }) twoFactorOptionsModal: ViewContainerRef; - + formGroup = this.formBuilder.group({ + token: [ + "", + { + validators: [Validators.required], + updateOn: "submit", + }, + ], + remember: [false], + }); + private destroy$ = new Subject(); constructor( loginStrategyService: LoginStrategyServiceAbstraction, router: Router, @@ -58,6 +69,7 @@ export class TwoFactorComponent extends BaseTwoFactorComponent implements OnDest configService: ConfigService, masterPasswordService: InternalMasterPasswordServiceAbstraction, accountService: AccountService, + private formBuilder: FormBuilder, @Inject(WINDOW) protected win: Window, ) { super( @@ -82,6 +94,16 @@ export class TwoFactorComponent extends BaseTwoFactorComponent implements OnDest ); this.onSuccessfulLoginNavigate = this.goAfterLogIn; } + async ngOnInit() { + await super.ngOnInit(); + this.formGroup.valueChanges.pipe(takeUntil(this.destroy$)).subscribe((value) => { + this.token = value.token; + this.remember = value.remember; + }); + } + submitForm = async () => { + await this.submit(); + }; async anotherMethod() { const dialogRef = TwoFactorOptionsComponent.open(this.dialogService); diff --git a/apps/web/src/app/oss-routing.module.ts b/apps/web/src/app/oss-routing.module.ts index 1115a60bf9..44cfc10873 100644 --- a/apps/web/src/app/oss-routing.module.ts +++ b/apps/web/src/app/oss-routing.module.ts @@ -82,7 +82,6 @@ const routes: Routes = [ component: LoginViaAuthRequestComponent, data: { titleId: "adminApprovalRequested" } satisfies DataProperties, }, - { path: "2fa", component: TwoFactorComponent, canActivate: [UnauthGuard] }, { path: "login-initiated", component: LoginDecryptionOptionsComponent, @@ -189,6 +188,33 @@ const routes: Routes = [ path: "", component: AnonLayoutWrapperComponent, children: [ + { + path: "2fa", + component: TwoFactorComponent, + canActivate: [unauthGuardFn()], + data: { + pageTitle: "verifyIdentity", + }, + }, + { + path: "recover-2fa", + canActivate: [unauthGuardFn()], + children: [ + { + path: "", + component: RecoverTwoFactorComponent, + }, + { + path: "", + component: EnvironmentSelectorComponent, + outlet: "environment-selector", + }, + ], + data: { + pageTitle: "recoverAccountTwoStep", + titleId: "recoverAccountTwoStep", + } satisfies DataProperties & AnonLayoutWrapperData, + }, { path: "accept-emergency", canActivate: [deepLinkGuard()], diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index 9cbd44a963..964cfe7536 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -722,6 +722,9 @@ "logIn": { "message": "Log in" }, + "verifyIdentity": { + "message": "Verify your Identity" + }, "logInInitiated": { "message": "Log in initiated" }, From 9b0250d4fd344efda56ef33e9bc6147cfb7efe7f Mon Sep 17 00:00:00 2001 From: rr-bw <102181210+rr-bw@users.noreply.github.com> Date: Tue, 11 Jun 2024 12:06:02 -0700 Subject: [PATCH 6/8] Check undefined data properties before i18n (#9590) * remove duplicate route * check for undefined before translation --- apps/web/src/app/oss-routing.module.ts | 19 ------------------ .../anon-layout-wrapper.component.ts | 20 +++++++++++++++---- 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/apps/web/src/app/oss-routing.module.ts b/apps/web/src/app/oss-routing.module.ts index 44cfc10873..209a291a71 100644 --- a/apps/web/src/app/oss-routing.module.ts +++ b/apps/web/src/app/oss-routing.module.ts @@ -238,25 +238,6 @@ const routes: Routes = [ }, ], }, - { - path: "recover-2fa", - canActivate: [unauthGuardFn()], - children: [ - { - path: "", - component: RecoverTwoFactorComponent, - }, - { - path: "", - component: EnvironmentSelectorComponent, - outlet: "environment-selector", - }, - ], - data: { - pageTitle: "recoverAccountTwoStep", - titleId: "recoverAccountTwoStep", - } satisfies DataProperties & AnonLayoutWrapperData, - }, { path: "remove-password", component: RemovePasswordComponent, diff --git a/libs/auth/src/angular/anon-layout/anon-layout-wrapper.component.ts b/libs/auth/src/angular/anon-layout/anon-layout-wrapper.component.ts index 5efd2cf9ab..e4472c368f 100644 --- a/libs/auth/src/angular/anon-layout/anon-layout-wrapper.component.ts +++ b/libs/auth/src/angular/anon-layout/anon-layout-wrapper.component.ts @@ -27,9 +27,21 @@ export class AnonLayoutWrapperComponent { private route: ActivatedRoute, private i18nService: I18nService, ) { - this.pageTitle = this.i18nService.t(this.route.snapshot.firstChild.data["pageTitle"]); - this.pageSubtitle = this.i18nService.t(this.route.snapshot.firstChild.data["pageSubtitle"]); - this.pageIcon = this.route.snapshot.firstChild.data["pageIcon"]; - this.showReadonlyHostname = this.route.snapshot.firstChild.data["showReadonlyHostname"]; + const routeData = this.route.snapshot.firstChild?.data; + + if (!routeData) { + return; + } + + if (routeData["pageTitle"] !== undefined) { + this.pageTitle = this.i18nService.t(routeData["pageTitle"]); + } + + if (routeData["pageSubtitle"] !== undefined) { + this.pageSubtitle = this.i18nService.t(routeData["pageSubtitle"]); + } + + this.pageIcon = routeData["pageIcon"]; + this.showReadonlyHostname = routeData["showReadonlyHostname"]; } } From 9e6fabaa39e28871db08c11817100b4246bc5568 Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Wed, 12 Jun 2024 05:58:41 +1000 Subject: [PATCH 7/8] Use --organizationid flag for device-approval commands (#9576) --- .../device-approval.program.ts | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/bitwarden_license/bit-cli/src/admin-console/device-approval/device-approval.program.ts b/bitwarden_license/bit-cli/src/admin-console/device-approval/device-approval.program.ts index 0b0f3bb0f9..408a5b8d81 100644 --- a/bitwarden_license/bit-cli/src/admin-console/device-approval/device-approval.program.ts +++ b/bitwarden_license/bit-cli/src/admin-console/device-approval/device-approval.program.ts @@ -11,6 +11,10 @@ import { DenyAllCommand } from "./deny-all.command"; import { DenyCommand } from "./deny.command"; import { ListCommand } from "./list.command"; +type Options = { + organizationid: string; +}; + export class DeviceApprovalProgram extends BaseProgram { constructor(protected serviceContainer: ServiceContainer) { super(serviceContainer); @@ -33,8 +37,8 @@ export class DeviceApprovalProgram extends BaseProgram { private listCommand(): Command { return new Command("list") .description("List all pending requests for an organization") - .argument("") - .action(async (organizationId: string) => { + .requiredOption("--organizationid ", "The organization id (required)") + .action(async (options: Options) => { await this.exitIfFeatureFlagDisabled(FeatureFlag.BulkDeviceApproval); await this.exitIfLocked(); @@ -42,17 +46,18 @@ export class DeviceApprovalProgram extends BaseProgram { this.serviceContainer.organizationAuthRequestService, this.serviceContainer.organizationService, ); - const response = await cmd.run(organizationId); + + const response = await cmd.run(options.organizationid); this.processResponse(response); }); } private approveCommand(): Command { return new Command("approve") - .argument("", "The id of the organization") .argument("", "The id of the request to approve") + .requiredOption("--organizationid ", "The organization id (required)") .description("Approve a pending request") - .action(async (organizationId: string, id: string) => { + .action(async (id: string, options: Options) => { await this.exitIfFeatureFlagDisabled(FeatureFlag.BulkDeviceApproval); await this.exitIfLocked(); @@ -60,7 +65,7 @@ export class DeviceApprovalProgram extends BaseProgram { this.serviceContainer.organizationService, this.serviceContainer.organizationAuthRequestService, ); - const response = await cmd.run(organizationId, id); + const response = await cmd.run(options.organizationid, id); this.processResponse(response); }); } @@ -68,8 +73,8 @@ export class DeviceApprovalProgram extends BaseProgram { private approveAllCommand(): Command { return new Command("approve-all") .description("Approve all pending requests for an organization") - .argument("") - .action(async (organizationId: string) => { + .requiredOption("--organizationid ", "The organization id (required)") + .action(async (options: Options) => { await this.exitIfFeatureFlagDisabled(FeatureFlag.BulkDeviceApproval); await this.exitIfLocked(); @@ -77,17 +82,17 @@ export class DeviceApprovalProgram extends BaseProgram { this.serviceContainer.organizationAuthRequestService, this.serviceContainer.organizationService, ); - const response = await cmd.run(organizationId); + const response = await cmd.run(options.organizationid); this.processResponse(response); }); } private denyCommand(): Command { return new Command("deny") - .argument("", "The id of the organization") .argument("", "The id of the request to deny") + .requiredOption("--organizationid ", "The organization id (required)") .description("Deny a pending request") - .action(async (organizationId: string, id: string) => { + .action(async (id: string, options: Options) => { await this.exitIfFeatureFlagDisabled(FeatureFlag.BulkDeviceApproval); await this.exitIfLocked(); @@ -95,7 +100,7 @@ export class DeviceApprovalProgram extends BaseProgram { this.serviceContainer.organizationService, this.serviceContainer.organizationAuthRequestService, ); - const response = await cmd.run(organizationId, id); + const response = await cmd.run(options.organizationid, id); this.processResponse(response); }); } @@ -103,8 +108,8 @@ export class DeviceApprovalProgram extends BaseProgram { private denyAllCommand(): Command { return new Command("deny-all") .description("Deny all pending requests for an organization") - .argument("") - .action(async (organizationId: string) => { + .requiredOption("--organizationid ", "The organization id (required)") + .action(async (options: Options) => { await this.exitIfFeatureFlagDisabled(FeatureFlag.BulkDeviceApproval); await this.exitIfLocked(); @@ -112,7 +117,7 @@ export class DeviceApprovalProgram extends BaseProgram { this.serviceContainer.organizationService, this.serviceContainer.organizationAuthRequestService, ); - const response = await cmd.run(organizationId); + const response = await cmd.run(options.organizationid); this.processResponse(response); }); } From fe82dbe2b96e90768eb38d94f830ea523a002556 Mon Sep 17 00:00:00 2001 From: Cesar Gonzalez Date: Tue, 11 Jun 2024 15:00:05 -0500 Subject: [PATCH 8/8] [PM-8510] Implement collect page details observable (#9452) * Working through a POC of a collectPageDetails observable * Implementing collect page details observable * [PM-8510] Implement collectPageDetails observable * [PM-8510] Adding documentation to newly created collectPageDetailsFromTab method * [PM-8510] Removing unnecessary file * [PM-8510] Implementing Jest tests for the collectPageDetailsFromTab$ method * [PM-8510] Implementing Jest tests for the collectPageDetailsFromTab$ method * [PM-8510] Implementing Jest tests for the collectPageDetailsFromTab$ method * [PM-8510] Implementing Jest tests for the collectPageDetailsFromTab$ method * [PM-8510] Removing unnecessary property * [PM-8510] Adding subscription reference to current tab component * [PM-8510] Fixing jest tests --- .../autofill/commands/autofill-tab-command.ts | 71 --------------- .../autofill/enums/autofill-message.enums.ts | 14 +++ .../services/abstractions/autofill.service.ts | 17 ++++ .../services/autofill.service.spec.ts | 89 ++++++++++++++++++- .../src/autofill/services/autofill.service.ts | 44 ++++++++- .../browser/src/background/main.background.ts | 1 + .../src/popup/services/services.module.ts | 1 + .../components/vault/current-tab.component.ts | 38 +++----- .../popup/components/vault/view.component.ts | 24 ++--- 9 files changed, 180 insertions(+), 119 deletions(-) delete mode 100644 apps/browser/src/autofill/commands/autofill-tab-command.ts create mode 100644 apps/browser/src/autofill/enums/autofill-message.enums.ts diff --git a/apps/browser/src/autofill/commands/autofill-tab-command.ts b/apps/browser/src/autofill/commands/autofill-tab-command.ts deleted file mode 100644 index 16ce40ff3d..0000000000 --- a/apps/browser/src/autofill/commands/autofill-tab-command.ts +++ /dev/null @@ -1,71 +0,0 @@ -import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; - -import AutofillPageDetails from "../models/autofill-page-details"; -import { AutofillService } from "../services/abstractions/autofill.service"; - -export class AutofillTabCommand { - constructor(private autofillService: AutofillService) {} - - async doAutofillTabCommand(tab: chrome.tabs.Tab) { - if (!tab.id) { - throw new Error("Tab does not have an id, cannot complete autofill."); - } - - const details = await this.collectPageDetails(tab.id); - await this.autofillService.doAutoFillOnTab( - [ - { - frameId: 0, - tab: tab, - details: details, - }, - ], - tab, - true, - ); - } - - async doAutofillTabWithCipherCommand(tab: chrome.tabs.Tab, cipher: CipherView) { - if (!tab.id) { - throw new Error("Tab does not have an id, cannot complete autofill."); - } - - const details = await this.collectPageDetails(tab.id); - await this.autofillService.doAutoFill({ - tab: tab, - cipher: cipher, - pageDetails: [ - { - frameId: 0, - tab: tab, - details: details, - }, - ], - skipLastUsed: false, - skipUsernameOnlyFill: false, - onlyEmptyFields: false, - onlyVisibleFields: false, - fillNewPassword: true, - allowTotpAutofill: true, - }); - } - - private async collectPageDetails(tabId: number): Promise { - return new Promise((resolve, reject) => { - chrome.tabs.sendMessage( - tabId, - { - command: "collectPageDetailsImmediately", - }, - (response: AutofillPageDetails) => { - if (chrome.runtime.lastError) { - reject(chrome.runtime.lastError); - return; - } - - resolve(response); - }, - ); - }); - } -} diff --git a/apps/browser/src/autofill/enums/autofill-message.enums.ts b/apps/browser/src/autofill/enums/autofill-message.enums.ts new file mode 100644 index 0000000000..4fdae2d914 --- /dev/null +++ b/apps/browser/src/autofill/enums/autofill-message.enums.ts @@ -0,0 +1,14 @@ +export const AutofillMessageCommand = { + collectPageDetails: "collectPageDetails", + collectPageDetailsResponse: "collectPageDetailsResponse", +} as const; + +export type AutofillMessageCommandType = + (typeof AutofillMessageCommand)[keyof typeof AutofillMessageCommand]; + +export const AutofillMessageSender = { + collectPageDetailsFromTabObservable: "collectPageDetailsFromTabObservable", +} as const; + +export type AutofillMessageSenderType = + (typeof AutofillMessageSender)[keyof typeof AutofillMessageSender]; diff --git a/apps/browser/src/autofill/services/abstractions/autofill.service.ts b/apps/browser/src/autofill/services/abstractions/autofill.service.ts index 54a91a5176..9bdb85a3f2 100644 --- a/apps/browser/src/autofill/services/abstractions/autofill.service.ts +++ b/apps/browser/src/autofill/services/abstractions/autofill.service.ts @@ -1,7 +1,11 @@ +import { Observable } from "rxjs"; + import { UriMatchStrategySetting } from "@bitwarden/common/models/domain/domain-service"; +import { CommandDefinition } from "@bitwarden/common/platform/messaging"; import { CipherType } from "@bitwarden/common/vault/enums"; import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view"; +import { AutofillMessageCommand } from "../../enums/autofill-message.enums"; import AutofillField from "../../models/autofill-field"; import AutofillForm from "../../models/autofill-form"; import AutofillPageDetails from "../../models/autofill-page-details"; @@ -44,7 +48,20 @@ export interface GenerateFillScriptOptions { defaultUriMatch: UriMatchStrategySetting; } +export type CollectPageDetailsResponseMessage = { + tab: chrome.tabs.Tab; + details: AutofillPageDetails; + sender?: string; + webExtSender: chrome.runtime.MessageSender; +}; + +export const COLLECT_PAGE_DETAILS_RESPONSE_COMMAND = + new CommandDefinition( + AutofillMessageCommand.collectPageDetailsResponse, + ); + export abstract class AutofillService { + collectPageDetailsFromTab$: (tab: chrome.tabs.Tab) => Observable; loadAutofillScriptsOnInstall: () => Promise; reloadAutofillScripts: () => Promise; injectAutofillScripts: ( diff --git a/apps/browser/src/autofill/services/autofill.service.spec.ts b/apps/browser/src/autofill/services/autofill.service.spec.ts index 23f690544d..2427c754ae 100644 --- a/apps/browser/src/autofill/services/autofill.service.spec.ts +++ b/apps/browser/src/autofill/services/autofill.service.spec.ts @@ -1,5 +1,5 @@ import { mock, mockReset, MockProxy } from "jest-mock-extended"; -import { BehaviorSubject, of } from "rxjs"; +import { BehaviorSubject, of, Subject } from "rxjs"; import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service"; import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status"; @@ -16,12 +16,14 @@ import { EventType } from "@bitwarden/common/enums"; import { UriMatchStrategy } from "@bitwarden/common/models/domain/domain-service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; +import { MessageListener } from "@bitwarden/common/platform/messaging"; import { Utils } from "@bitwarden/common/platform/misc/utils"; import { EventCollectionService } from "@bitwarden/common/services/event/event-collection.service"; import { FakeStateProvider, FakeAccountService, mockAccountServiceWith, + subscribeTo, } from "@bitwarden/common/spec"; import { UserId } from "@bitwarden/common/types/guid"; import { FieldType, LinkedIdType, LoginLinkedId, CipherType } from "@bitwarden/common/vault/enums"; @@ -37,6 +39,7 @@ import { TotpService } from "@bitwarden/common/vault/services/totp.service"; import { BrowserApi } from "../../platform/browser/browser-api"; import { BrowserScriptInjectorService } from "../../platform/services/browser-script-injector.service"; +import { AutofillMessageCommand, AutofillMessageSender } from "../enums/autofill-message.enums"; import { AutofillPort } from "../enums/autofill-port.enums"; import AutofillField from "../models/autofill-field"; import AutofillPageDetails from "../models/autofill-page-details"; @@ -52,6 +55,7 @@ import { flushPromises, triggerTestFailure } from "../spec/testing-utils"; import { AutoFillOptions, + CollectPageDetailsResponseMessage, GenerateFillScriptOptions, PageDetail, } from "./abstractions/autofill.service"; @@ -82,6 +86,7 @@ describe("AutofillService", () => { const platformUtilsService = mock(); let activeAccountStatusMock$: BehaviorSubject; let authService: MockProxy; + let messageListener: MockProxy; beforeEach(() => { scriptInjectorService = new BrowserScriptInjectorService(platformUtilsService, logService); @@ -91,6 +96,7 @@ describe("AutofillService", () => { activeAccountStatusMock$ = new BehaviorSubject(AuthenticationStatus.Unlocked); authService = mock(); authService.activeAccountStatus$ = activeAccountStatusMock$; + messageListener = mock(); autofillService = new AutofillService( cipherService, autofillSettingsService, @@ -103,10 +109,11 @@ describe("AutofillService", () => { scriptInjectorService, accountService, authService, + messageListener, ); - domainSettingsService = new DefaultDomainSettingsService(fakeStateProvider); domainSettingsService.equivalentDomains$ = of(mockEquivalentDomains); + jest.spyOn(BrowserApi, "tabSendMessage"); }); afterEach(() => { @@ -114,6 +121,84 @@ describe("AutofillService", () => { mockReset(cipherService); }); + describe("collectPageDetailsFromTab$", () => { + const tab = mock({ id: 1 }); + const messages = new Subject(); + + function mockCollectPageDetailsResponseMessage( + tab: chrome.tabs.Tab, + webExtSender: chrome.runtime.MessageSender = mock(), + sender: string = AutofillMessageSender.collectPageDetailsFromTabObservable, + ): CollectPageDetailsResponseMessage { + return mock({ + tab, + webExtSender, + sender, + }); + } + + beforeEach(() => { + messageListener.messages$.mockReturnValue(messages.asObservable()); + }); + + it("sends a `collectPageDetails` message to the passed tab", () => { + autofillService.collectPageDetailsFromTab$(tab); + + expect(BrowserApi.tabSendMessage).toHaveBeenCalledWith(tab, { + command: AutofillMessageCommand.collectPageDetails, + sender: AutofillMessageSender.collectPageDetailsFromTabObservable, + tab, + }); + }); + + it("builds an array of page details from received `collectPageDetailsResponse` messages", async () => { + const topLevelSender = mock({ tab, frameId: 0 }); + const subFrameSender = mock({ tab, frameId: 1 }); + + const tracker = subscribeTo(autofillService.collectPageDetailsFromTab$(tab)); + const pausePromise = tracker.pauseUntilReceived(2); + + messages.next(mockCollectPageDetailsResponseMessage(tab, topLevelSender)); + messages.next(mockCollectPageDetailsResponseMessage(tab, subFrameSender)); + + await pausePromise; + + expect(tracker.emissions[1].length).toBe(2); + }); + + it("ignores messages from a different tab", async () => { + const otherTab = mock({ id: 2 }); + + const tracker = subscribeTo(autofillService.collectPageDetailsFromTab$(tab)); + const pausePromise = tracker.pauseUntilReceived(1); + + messages.next(mockCollectPageDetailsResponseMessage(tab)); + messages.next(mockCollectPageDetailsResponseMessage(otherTab)); + + await pausePromise; + + expect(tracker.emissions[1]).toBeUndefined(); + }); + + it("ignores messages from a different sender", async () => { + const tracker = subscribeTo(autofillService.collectPageDetailsFromTab$(tab)); + const pausePromise = tracker.pauseUntilReceived(1); + + messages.next(mockCollectPageDetailsResponseMessage(tab)); + messages.next( + mockCollectPageDetailsResponseMessage( + tab, + mock(), + "some-other-sender", + ), + ); + + await pausePromise; + + expect(tracker.emissions[1]).toBeUndefined(); + }); + }); + describe("loadAutofillScriptsOnInstall", () => { let tab1: chrome.tabs.Tab; let tab2: chrome.tabs.Tab; diff --git a/apps/browser/src/autofill/services/autofill.service.ts b/apps/browser/src/autofill/services/autofill.service.ts index 9ec2052381..ef051e5912 100644 --- a/apps/browser/src/autofill/services/autofill.service.ts +++ b/apps/browser/src/autofill/services/autofill.service.ts @@ -1,4 +1,4 @@ -import { firstValueFrom, startWith } from "rxjs"; +import { filter, firstValueFrom, Observable, scan, startWith } from "rxjs"; import { pairwise } from "rxjs/operators"; import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service"; @@ -17,6 +17,7 @@ import { UriMatchStrategy, } from "@bitwarden/common/models/domain/domain-service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; +import { MessageListener } from "@bitwarden/common/platform/messaging"; import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service"; import { TotpService } from "@bitwarden/common/vault/abstractions/totp.service"; import { FieldType, CipherType } from "@bitwarden/common/vault/enums"; @@ -27,6 +28,7 @@ import { FieldView } from "@bitwarden/common/vault/models/view/field.view"; import { BrowserApi } from "../../platform/browser/browser-api"; import { ScriptInjectorService } from "../../platform/services/abstractions/script-injector.service"; import { openVaultItemPasswordRepromptPopout } from "../../vault/popup/utils/vault-popout-window"; +import { AutofillMessageCommand, AutofillMessageSender } from "../enums/autofill-message.enums"; import { AutofillPort } from "../enums/autofill-port.enums"; import AutofillField from "../models/autofill-field"; import AutofillPageDetails from "../models/autofill-page-details"; @@ -35,6 +37,7 @@ import AutofillScript from "../models/autofill-script"; import { AutoFillOptions, AutofillService as AutofillServiceInterface, + COLLECT_PAGE_DETAILS_RESPONSE_COMMAND, FormData, GenerateFillScriptOptions, PageDetail, @@ -64,8 +67,47 @@ export default class AutofillService implements AutofillServiceInterface { private scriptInjectorService: ScriptInjectorService, private accountService: AccountService, private authService: AuthService, + private messageListener: MessageListener, ) {} + /** + * Collects page details from the specific tab. This method returns an observable that can + * be subscribed to in order to build the results from all collectPageDetailsResponse + * messages from the given tab. + * + * @param tab The tab to collect page details from + */ + collectPageDetailsFromTab$(tab: chrome.tabs.Tab): Observable { + const pageDetailsFromTab$ = this.messageListener + .messages$(COLLECT_PAGE_DETAILS_RESPONSE_COMMAND) + .pipe( + filter( + (message) => + message.tab.id === tab.id && + message.sender === AutofillMessageSender.collectPageDetailsFromTabObservable, + ), + scan( + (acc, message) => [ + ...acc, + { + frameId: message.webExtSender.frameId, + tab: message.tab, + details: message.details, + }, + ], + [] as PageDetail[], + ), + ); + + void BrowserApi.tabSendMessage(tab, { + tab: tab, + command: AutofillMessageCommand.collectPageDetails, + sender: AutofillMessageSender.collectPageDetailsFromTabObservable, + }); + + return pageDetailsFromTab$; + } + /** * Triggers on installation of the extension Handles injecting * content scripts into all tabs that are currently open, and diff --git a/apps/browser/src/background/main.background.ts b/apps/browser/src/background/main.background.ts index 9af1a14132..fe797dc8ba 100644 --- a/apps/browser/src/background/main.background.ts +++ b/apps/browser/src/background/main.background.ts @@ -889,6 +889,7 @@ export default class MainBackground { this.scriptInjectorService, this.accountService, this.authService, + messageListener, ); this.auditService = new AuditService(this.cryptoFunctionService, this.apiService); diff --git a/apps/browser/src/popup/services/services.module.ts b/apps/browser/src/popup/services/services.module.ts index d61fa3b19c..c7b5ca9b41 100644 --- a/apps/browser/src/popup/services/services.module.ts +++ b/apps/browser/src/popup/services/services.module.ts @@ -342,6 +342,7 @@ const safeProviders: SafeProvider[] = [ ScriptInjectorService, AccountServiceAbstraction, AuthService, + MessageListener, ], }), safeProvider({ diff --git a/apps/browser/src/vault/popup/components/vault/current-tab.component.ts b/apps/browser/src/vault/popup/components/vault/current-tab.component.ts index 24ca030284..97856a952c 100644 --- a/apps/browser/src/vault/popup/components/vault/current-tab.component.ts +++ b/apps/browser/src/vault/popup/components/vault/current-tab.component.ts @@ -1,6 +1,6 @@ import { ChangeDetectorRef, Component, NgZone, OnDestroy, OnInit } from "@angular/core"; import { Router } from "@angular/router"; -import { Subject, firstValueFrom, from } from "rxjs"; +import { Subject, firstValueFrom, from, Subscription } from "rxjs"; import { debounceTime, switchMap, takeUntil } from "rxjs/operators"; import { UnassignedItemsBannerService } from "@bitwarden/angular/services/unassigned-items-banner.service"; @@ -51,12 +51,12 @@ export class CurrentTabComponent implements OnInit, OnDestroy { autofillCalloutText: string; protected search$ = new Subject(); private destroy$ = new Subject(); + private collectPageDetailsSubscription: Subscription; private totpCode: string; private totpTimeout: number; private loadedTimeout: number; private searchTimeout: number; - private initPageDetailsTimeout: number; protected unassignedItemsBannerEnabled$ = this.configService.getFeatureFlag$( FeatureFlag.UnassignedItemsBanner, @@ -100,15 +100,6 @@ export class CurrentTabComponent implements OnInit, OnDestroy { }, 500); } break; - case "collectPageDetailsResponse": - if (message.sender === BroadcasterSubscriptionId) { - this.pageDetails.push({ - frameId: message.webExtSender.frameId, - tab: message.tab, - details: message.details, - }); - } - break; default: break; } @@ -266,6 +257,7 @@ export class CurrentTabComponent implements OnInit, OnDestroy { protected async load() { this.isLoading = false; this.tab = await BrowserApi.getTabFromCurrentWindow(); + if (this.tab != null) { this.url = this.tab.url; } else { @@ -274,8 +266,14 @@ export class CurrentTabComponent implements OnInit, OnDestroy { return; } - this.hostname = Utils.getHostname(this.url); this.pageDetails = []; + this.collectPageDetailsSubscription?.unsubscribe(); + this.collectPageDetailsSubscription = this.autofillService + .collectPageDetailsFromTab$(this.tab) + .pipe(takeUntil(this.destroy$)) + .subscribe((pageDetails) => (this.pageDetails = pageDetails)); + + this.hostname = Utils.getHostname(this.url); const otherTypes: CipherType[] = []; const dontShowCards = !(await firstValueFrom(this.vaultSettingsService.showCardsCurrentTab$)); const dontShowIdentities = !(await firstValueFrom( @@ -323,7 +321,6 @@ export class CurrentTabComponent implements OnInit, OnDestroy { } this.isLoading = this.loaded = true; - this.collectTabPageDetails(); } async goToSettings() { @@ -361,19 +358,4 @@ export class CurrentTabComponent implements OnInit, OnDestroy { this.autofillCalloutText = this.i18nService.t("autofillSelectInfoWithoutCommand"); } } - - private collectTabPageDetails() { - void BrowserApi.tabSendMessage(this.tab, { - command: "collectPageDetails", - tab: this.tab, - sender: BroadcasterSubscriptionId, - }); - - window.clearTimeout(this.initPageDetailsTimeout); - this.initPageDetailsTimeout = window.setTimeout(() => { - if (this.pageDetails.length === 0) { - this.collectTabPageDetails(); - } - }, 250); - } } diff --git a/apps/browser/src/vault/popup/components/vault/view.component.ts b/apps/browser/src/vault/popup/components/vault/view.component.ts index 211bd8fc09..ba101aa653 100644 --- a/apps/browser/src/vault/popup/components/vault/view.component.ts +++ b/apps/browser/src/vault/popup/components/vault/view.component.ts @@ -1,7 +1,7 @@ import { DatePipe, Location } from "@angular/common"; import { ChangeDetectorRef, Component, NgZone } from "@angular/core"; import { ActivatedRoute, Router } from "@angular/router"; -import { Subject, firstValueFrom, takeUntil } from "rxjs"; +import { Subject, firstValueFrom, takeUntil, Subscription } from "rxjs"; import { first } from "rxjs/operators"; import { ViewComponent as BaseViewComponent } from "@bitwarden/angular/vault/components/view.component"; @@ -68,6 +68,7 @@ export class ViewComponent extends BaseViewComponent { inPopout = false; cipherType = CipherType; private fido2PopoutSessionData$ = fido2PopoutSessionData$(); + private collectPageDetailsSubscription: Subscription; private destroy$ = new Subject(); @@ -152,15 +153,6 @@ export class ViewComponent extends BaseViewComponent { // eslint-disable-next-line @typescript-eslint/no-floating-promises this.ngZone.run(async () => { switch (message.command) { - case "collectPageDetailsResponse": - if (message.sender === BroadcasterSubscriptionId) { - this.pageDetails.push({ - frameId: message.webExtSender.frameId, - tab: message.tab, - details: message.details, - }); - } - break; case "tabChanged": case "windowChanged": if (this.loadPageDetailsTimeout != null) { @@ -337,6 +329,7 @@ export class ViewComponent extends BaseViewComponent { } private async loadPageDetails() { + this.collectPageDetailsSubscription?.unsubscribe(); this.pageDetails = []; this.tab = this.senderTabId ? await BrowserApi.getTab(this.senderTabId) @@ -346,13 +339,10 @@ export class ViewComponent extends BaseViewComponent { return; } - // 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 - BrowserApi.tabSendMessage(this.tab, { - command: "collectPageDetails", - tab: this.tab, - sender: BroadcasterSubscriptionId, - }); + this.collectPageDetailsSubscription = this.autofillService + .collectPageDetailsFromTab$(this.tab) + .pipe(takeUntil(this.destroy$)) + .subscribe((pageDetails) => (this.pageDetails = pageDetails)); } private async doAutofill() {