From b79554a13be3bd0ed5f79531a22cd00f01c70621 Mon Sep 17 00:00:00 2001 From: SmithThe4th Date: Thu, 30 Mar 2023 16:27:03 -0400 Subject: [PATCH] [PM-283] Fix Reports UI behavior for premium and free users (#4926) * Prevent rerouting to dispaly modal message, and refactored components where thsi was used * Added upgrade badge to organization reports view * created guard to prevent free organization users from accessing reports * Added isUpgradeRequired getter to organization class * Modifiewd reports home to pass upgrade badge and add new guard to organization reports module * Fixed routing bug when routing to billing subscription page * Refactored to use async pipe and observables * Renamed getter name to be more descriptive * Removed checkAccess from reports * Renamed guard * Removed unused variables * Lint fix * Lint fix * prettier fix * Corrected organiztion service reference * Moved homepage to ngonInit * [PM-1629] Update the upgrade dialog for users without billing rights (#5102) * Show dialog with description when user does not have access to the billing page * switched conditions to nested if to make the logic clearer --- .../organizations/guards/is-paid-org.guard.ts | 44 ++++++++++++ .../organization-reporting-routing.module.ts | 6 ++ .../reporting/reports-home.component.html | 6 +- .../reporting/reports-home.component.ts | 67 ++++++++++--------- .../exposed-passwords-report.component.ts | 3 +- apps/web/src/app/app.component.ts | 2 +- .../reports/pages/cipher-report.component.ts | 12 ---- .../exposed-passwords-report.component.ts | 10 +-- .../inactive-two-factor-report.component.ts | 4 +- .../reused-passwords-report.component.ts | 4 +- .../unsecured-websites-report.component.ts | 4 +- .../pages/weak-passwords-report.component.ts | 4 +- .../report-card/report-card.component.html | 2 +- apps/web/src/locales/en/messages.json | 3 + .../models/domain/organization.ts | 5 ++ 15 files changed, 104 insertions(+), 72 deletions(-) create mode 100644 apps/web/src/app/admin-console/organizations/guards/is-paid-org.guard.ts diff --git a/apps/web/src/app/admin-console/organizations/guards/is-paid-org.guard.ts b/apps/web/src/app/admin-console/organizations/guards/is-paid-org.guard.ts new file mode 100644 index 00000000000..a74e152f889 --- /dev/null +++ b/apps/web/src/app/admin-console/organizations/guards/is-paid-org.guard.ts @@ -0,0 +1,44 @@ +import { Injectable } from "@angular/core"; +import { ActivatedRouteSnapshot, CanActivate, Router, RouterStateSnapshot } from "@angular/router"; + +import { I18nService } from "@bitwarden/common/abstractions/i18n.service"; +import { MessagingService } from "@bitwarden/common/abstractions/messaging.service"; +import { PlatformUtilsService } from "@bitwarden/common/abstractions/platformUtils.service"; +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; + +@Injectable({ + providedIn: "root", +}) +export class IsPaidOrgGuard implements CanActivate { + constructor( + private router: Router, + private organizationService: OrganizationService, + private platformUtilsService: PlatformUtilsService, + private messagingService: MessagingService, + private i18nService: I18nService + ) {} + + async canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot) { + const org = this.organizationService.get(route.params.organizationId); + + if (org == null) { + return this.router.createUrlTree(["/"]); + } + + if (org.isFreeOrg) { + // Users without billing permission can't access billing + if (!org.canManageBilling) { + await this.platformUtilsService.showDialog( + this.i18nService.t("notAvailableForFreeOrganization"), + this.i18nService.t("upgradeOrganization"), + this.i18nService.t("ok") + ); + return false; + } else { + this.messagingService.send("upgradeOrganization", { organizationId: org.id }); + } + } + + return !org.isFreeOrg; + } +} diff --git a/apps/web/src/app/admin-console/organizations/reporting/organization-reporting-routing.module.ts b/apps/web/src/app/admin-console/organizations/reporting/organization-reporting-routing.module.ts index 72a72857cad..1d86332a188 100644 --- a/apps/web/src/app/admin-console/organizations/reporting/organization-reporting-routing.module.ts +++ b/apps/web/src/app/admin-console/organizations/reporting/organization-reporting-routing.module.ts @@ -9,6 +9,7 @@ import { InactiveTwoFactorReportComponent } from "../../../admin-console/organiz import { ReusedPasswordsReportComponent } from "../../../admin-console/organizations/tools/reused-passwords-report.component"; import { UnsecuredWebsitesReportComponent } from "../../../admin-console/organizations/tools/unsecured-websites-report.component"; import { WeakPasswordsReportComponent } from "../../../admin-console/organizations/tools/weak-passwords-report.component"; +import { IsPaidOrgGuard } from "../guards/is-paid-org.guard"; import { OrganizationPermissionsGuard } from "../guards/org-permissions.guard"; import { OrganizationRedirectGuard } from "../guards/org-redirect.guard"; import { EventsComponent } from "../manage/events.component"; @@ -46,6 +47,7 @@ const routes: Routes = [ data: { titleId: "exposedPasswordsReport", }, + canActivate: [IsPaidOrgGuard], }, { path: "inactive-two-factor-report", @@ -53,6 +55,7 @@ const routes: Routes = [ data: { titleId: "inactive2faReport", }, + canActivate: [IsPaidOrgGuard], }, { path: "reused-passwords-report", @@ -60,6 +63,7 @@ const routes: Routes = [ data: { titleId: "reusedPasswordsReport", }, + canActivate: [IsPaidOrgGuard], }, { path: "unsecured-websites-report", @@ -67,6 +71,7 @@ const routes: Routes = [ data: { titleId: "unsecuredWebsitesReport", }, + canActivate: [IsPaidOrgGuard], }, { path: "weak-passwords-report", @@ -74,6 +79,7 @@ const routes: Routes = [ data: { titleId: "weakPasswordsReport", }, + canActivate: [IsPaidOrgGuard], }, ], }, diff --git a/apps/web/src/app/admin-console/organizations/reporting/reports-home.component.html b/apps/web/src/app/admin-console/organizations/reporting/reports-home.component.html index be5ccc5bae6..941788459f4 100644 --- a/apps/web/src/app/admin-console/organizations/reporting/reports-home.component.html +++ b/apps/web/src/app/admin-console/organizations/reporting/reports-home.component.html @@ -1,18 +1,18 @@ - +

{{ "reports" | i18n }}

{{ "orgsReportsDesc" | i18n }}

- +
- + {{ "backToReports" | i18n }} diff --git a/apps/web/src/app/admin-console/organizations/reporting/reports-home.component.ts b/apps/web/src/app/admin-console/organizations/reporting/reports-home.component.ts index dba7c869843..803e76c8bfd 100644 --- a/apps/web/src/app/admin-console/organizations/reporting/reports-home.component.ts +++ b/apps/web/src/app/admin-console/organizations/reporting/reports-home.component.ts @@ -1,8 +1,9 @@ -import { Component, OnDestroy, OnInit } from "@angular/core"; -import { NavigationEnd, Router } from "@angular/router"; -import { filter, Subject, takeUntil } from "rxjs"; +import { Component, OnInit } from "@angular/core"; +import { ActivatedRoute, NavigationEnd, Router } from "@angular/router"; +import { filter, map, Observable, startWith } from "rxjs"; import { StateService } from "@bitwarden/common/abstractions/state.service"; +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { ReportVariant, reports, ReportType, ReportEntry } from "../../../reports"; @@ -10,56 +11,56 @@ import { ReportVariant, reports, ReportType, ReportEntry } from "../../../report selector: "app-org-reports-home", templateUrl: "reports-home.component.html", }) -export class ReportsHomeComponent implements OnInit, OnDestroy { - reports: ReportEntry[]; +export class ReportsHomeComponent implements OnInit { + reports$: Observable; + homepage$: Observable; - homepage = true; - private destrory$: Subject = new Subject(); + constructor( + private route: ActivatedRoute, + private stateService: StateService, + private organizationService: OrganizationService, + private router: Router + ) {} - constructor(private stateService: StateService, router: Router) { - router.events - .pipe( - filter((event) => event instanceof NavigationEnd), - takeUntil(this.destrory$) - ) - .subscribe((event) => { - this.homepage = (event as NavigationEnd).urlAfterRedirects.endsWith("/reports"); - }); + ngOnInit() { + this.homepage$ = this.router.events.pipe( + filter((event) => event instanceof NavigationEnd), + map((event) => (event as NavigationEnd).urlAfterRedirects.endsWith("/reports")), + startWith(true) + ); + + this.reports$ = this.route.params.pipe( + map((params) => this.organizationService.get(params.organizationId)), + map((org) => this.buildReports(org.isFreeOrg)) + ); } - async ngOnInit(): Promise { - const userHasPremium = await this.stateService.getCanAccessPremium(); + private buildReports(upgradeRequired: boolean): ReportEntry[] { + const reportRequiresUpgrade = upgradeRequired + ? ReportVariant.RequiresUpgrade + : ReportVariant.Enabled; - const reportRequiresPremium = userHasPremium - ? ReportVariant.Enabled - : ReportVariant.RequiresPremium; - - this.reports = [ + return [ { ...reports[ReportType.ExposedPasswords], - variant: reportRequiresPremium, + variant: reportRequiresUpgrade, }, { ...reports[ReportType.ReusedPasswords], - variant: reportRequiresPremium, + variant: reportRequiresUpgrade, }, { ...reports[ReportType.WeakPasswords], - variant: reportRequiresPremium, + variant: reportRequiresUpgrade, }, { ...reports[ReportType.UnsecuredWebsites], - variant: reportRequiresPremium, + variant: reportRequiresUpgrade, }, { ...reports[ReportType.Inactive2fa], - variant: reportRequiresPremium, + variant: reportRequiresUpgrade, }, ]; } - - ngOnDestroy(): void { - this.destrory$.next(); - this.destrory$.complete(); - } } diff --git a/apps/web/src/app/admin-console/organizations/tools/exposed-passwords-report.component.ts b/apps/web/src/app/admin-console/organizations/tools/exposed-passwords-report.component.ts index e61b02042fc..ca0f51dda8e 100644 --- a/apps/web/src/app/admin-console/organizations/tools/exposed-passwords-report.component.ts +++ b/apps/web/src/app/admin-console/organizations/tools/exposed-passwords-report.component.ts @@ -33,12 +33,11 @@ export class ExposedPasswordsReportComponent extends BaseExposedPasswordsReportC super(cipherService, auditService, modalService, messagingService, passwordRepromptService); } - ngOnInit() { + async ngOnInit() { // eslint-disable-next-line rxjs-angular/prefer-takeuntil, rxjs/no-async-subscribe this.route.parent.parent.params.subscribe(async (params) => { this.organization = await this.organizationService.get(params.organizationId); this.manageableCiphers = await this.cipherService.getAll(); - await this.checkAccess(); }); } diff --git a/apps/web/src/app/app.component.ts b/apps/web/src/app/app.component.ts index 1e5d72e9887..6826da0467d 100644 --- a/apps/web/src/app/app.component.ts +++ b/apps/web/src/app/app.component.ts @@ -139,8 +139,8 @@ export class AppComponent implements OnDestroy, OnInit { this.router.navigate([ "organizations", message.organizationId, - "settings", "billing", + "subscription", ]); } break; diff --git a/apps/web/src/app/reports/pages/cipher-report.component.ts b/apps/web/src/app/reports/pages/cipher-report.component.ts index 425975ade37..3f73d6d2dca 100644 --- a/apps/web/src/app/reports/pages/cipher-report.component.ts +++ b/apps/web/src/app/reports/pages/cipher-report.component.ts @@ -72,18 +72,6 @@ export class CipherReportComponent { return childComponent; } - protected async checkAccess(): Promise { - if (this.organization != null) { - // TODO: Maybe we want to just make sure they are not on a free plan? Just compare useTotp for now - // since all paid plans include useTotp - if (this.requiresPaid && !this.organization.useTotp) { - this.messagingService.send("upgradeOrganization", { organizationId: this.organization.id }); - return false; - } - } - return true; - } - protected async setCiphers() { this.ciphers = []; } diff --git a/apps/web/src/app/reports/pages/exposed-passwords-report.component.ts b/apps/web/src/app/reports/pages/exposed-passwords-report.component.ts index 6489c076729..e30c9c69b31 100644 --- a/apps/web/src/app/reports/pages/exposed-passwords-report.component.ts +++ b/apps/web/src/app/reports/pages/exposed-passwords-report.component.ts @@ -27,14 +27,8 @@ export class ExposedPasswordsReportComponent extends CipherReportComponent imple super(modalService, messagingService, true, passwordRepromptService); } - ngOnInit() { - this.checkAccess(); - } - - async load() { - if (await this.checkAccess()) { - super.load(); - } + async ngOnInit() { + await super.load(); } async setCiphers() { diff --git a/apps/web/src/app/reports/pages/inactive-two-factor-report.component.ts b/apps/web/src/app/reports/pages/inactive-two-factor-report.component.ts index 3691026d272..ca5cc25c765 100644 --- a/apps/web/src/app/reports/pages/inactive-two-factor-report.component.ts +++ b/apps/web/src/app/reports/pages/inactive-two-factor-report.component.ts @@ -30,9 +30,7 @@ export class InactiveTwoFactorReportComponent extends CipherReportComponent impl } async ngOnInit() { - if (await this.checkAccess()) { - await super.load(); - } + await super.load(); } async setCiphers() { diff --git a/apps/web/src/app/reports/pages/reused-passwords-report.component.ts b/apps/web/src/app/reports/pages/reused-passwords-report.component.ts index 04ee658762f..c01a7ab2901 100644 --- a/apps/web/src/app/reports/pages/reused-passwords-report.component.ts +++ b/apps/web/src/app/reports/pages/reused-passwords-report.component.ts @@ -28,9 +28,7 @@ export class ReusedPasswordsReportComponent extends CipherReportComponent implem } async ngOnInit() { - if (await this.checkAccess()) { - await super.load(); - } + await super.load(); } async setCiphers() { diff --git a/apps/web/src/app/reports/pages/unsecured-websites-report.component.ts b/apps/web/src/app/reports/pages/unsecured-websites-report.component.ts index 456e9bba834..f83fe906734 100644 --- a/apps/web/src/app/reports/pages/unsecured-websites-report.component.ts +++ b/apps/web/src/app/reports/pages/unsecured-websites-report.component.ts @@ -24,9 +24,7 @@ export class UnsecuredWebsitesReportComponent extends CipherReportComponent impl } async ngOnInit() { - if (await this.checkAccess()) { - await super.load(); - } + await super.load(); } async setCiphers() { diff --git a/apps/web/src/app/reports/pages/weak-passwords-report.component.ts b/apps/web/src/app/reports/pages/weak-passwords-report.component.ts index 93a3bfbc574..cd18c9bd9bb 100644 --- a/apps/web/src/app/reports/pages/weak-passwords-report.component.ts +++ b/apps/web/src/app/reports/pages/weak-passwords-report.component.ts @@ -31,9 +31,7 @@ export class WeakPasswordsReportComponent extends CipherReportComponent implemen } async ngOnInit() { - if (await this.checkAccess()) { - await super.load(); - } + await super.load(); } async setCiphers() { diff --git a/apps/web/src/app/reports/shared/report-card/report-card.component.html b/apps/web/src/app/reports/shared/report-card/report-card.component.html index 229bc6a42df..2996b7b4998 100644 --- a/apps/web/src/app/reports/shared/report-card/report-card.component.html +++ b/apps/web/src/app/reports/shared/report-card/report-card.component.html @@ -15,7 +15,7 @@
diff --git a/apps/web/src/locales/en/messages.json b/apps/web/src/locales/en/messages.json index 02ecb4ca219..93d9bd24c4d 100644 --- a/apps/web/src/locales/en/messages.json +++ b/apps/web/src/locales/en/messages.json @@ -6673,5 +6673,8 @@ }, "dismiss": { "message": "Dismiss" + }, + "notAvailableForFreeOrganization": { + "message": "This feature is not available for free organizations. Contact your organization owner to upgrade." } } diff --git a/libs/common/src/admin-console/models/domain/organization.ts b/libs/common/src/admin-console/models/domain/organization.ts index 1ecf2c9f6f6..8f2e703515d 100644 --- a/libs/common/src/admin-console/models/domain/organization.ts +++ b/libs/common/src/admin-console/models/domain/organization.ts @@ -210,6 +210,11 @@ export class Organization { return this.useSecretsManager && this.accessSecretsManager; } + get isFreeOrg() { + // return true if organization needs to be upgraded from a free org + return !this.useTotp; + } + static fromJSON(json: Jsonify) { if (json == null) { return null;