From 6f4771da6cf007b79caac69ab110a483488c4d5e Mon Sep 17 00:00:00 2001 From: Jake Fink Date: Mon, 7 Nov 2022 09:21:16 -0500 Subject: [PATCH] [EC-678] [EC-673] Fix active tab not showing selected while in child route (#3964) * [PS-1114] hide reporting sidebar if only events * [PS-1114] add orgRedirectGuard * [PS-1114] highlight tabs based on route subset * [PS-1114] redirect to correct child route on tab - Use new OrgRedirectGuard * [PS-1114] add settings redirect using guard - refactored guard to accept array of strings * [EC-678] [EC-673] remove remaining methods * [EC-678][EC-673] address PR feedback - change switch to if statements - remove ternary --- .../guards/org-redirect.guard.ts | 32 +++++++++++++++++++ .../organization-layout.component.html | 7 ++-- .../layouts/organization-layout.component.ts | 20 ------------ .../organization-routing.module.ts | 21 +++++++++++- .../organization-reporting-routing.module.ts | 22 ++++++++++++- .../reporting/reporting.component.ts | 3 +- .../organization-settings-routing.module.ts | 30 ++++++++++++++++- .../settings/settings.component.html | 9 ++++-- .../organization.service.abstraction.ts | 8 ++++- .../tabs/tab-nav-bar/tab-link.component.html | 1 + .../tabs/tab-nav-bar/tab-link.component.ts | 9 +++++- 11 files changed, 129 insertions(+), 33 deletions(-) create mode 100644 apps/web/src/app/organizations/guards/org-redirect.guard.ts diff --git a/apps/web/src/app/organizations/guards/org-redirect.guard.ts b/apps/web/src/app/organizations/guards/org-redirect.guard.ts new file mode 100644 index 0000000000..3f3213f6a3 --- /dev/null +++ b/apps/web/src/app/organizations/guards/org-redirect.guard.ts @@ -0,0 +1,32 @@ +import { Injectable } from "@angular/core"; +import { ActivatedRouteSnapshot, CanActivate, Router, RouterStateSnapshot } from "@angular/router"; + +import { + canAccessOrgAdmin, + OrganizationService, +} from "@bitwarden/common/abstractions/organization/organization.service.abstraction"; + +@Injectable({ + providedIn: "root", +}) +export class OrganizationRedirectGuard implements CanActivate { + constructor(private router: Router, private organizationService: OrganizationService) {} + + async canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot) { + const org = this.organizationService.get(route.params.organizationId); + + const customRedirect = route.data?.autoRedirectCallback; + if (customRedirect) { + let redirectPath = customRedirect(org); + if (typeof redirectPath === "string") { + redirectPath = [redirectPath]; + } + return this.router.createUrlTree([state.url, ...redirectPath]); + } + + if (canAccessOrgAdmin(org)) { + return this.router.createUrlTree(["/organizations", org.id]); + } + return this.router.createUrlTree(["/"]); + } +} diff --git a/apps/web/src/app/organizations/layouts/organization-layout.component.html b/apps/web/src/app/organizations/layouts/organization-layout.component.html index 5f5aa497f7..8ed9f0e80f 100644 --- a/apps/web/src/app/organizations/layouts/organization-layout.component.html +++ b/apps/web/src/app/organizations/layouts/organization-layout.component.html @@ -8,13 +8,10 @@ > {{ "vault" | i18n }} - + {{ "manage" | i18n }} - + {{ getReportTabLabel(organization) | i18n }} {{ diff --git a/apps/web/src/app/organizations/layouts/organization-layout.component.ts b/apps/web/src/app/organizations/layouts/organization-layout.component.ts index 8c871f74db..d8f2fdd8a3 100644 --- a/apps/web/src/app/organizations/layouts/organization-layout.component.ts +++ b/apps/web/src/app/organizations/layouts/organization-layout.component.ts @@ -72,24 +72,4 @@ export class OrganizationLayoutComponent implements OnInit, OnDestroy { getReportTabLabel(organization: Organization): string { return organization.useEvents ? "reporting" : "reports"; } - - getReportRoute(organization: Organization): string { - return organization.useEvents ? "reporting/events" : "reporting/reports"; - } - - getManageRoute(organization: Organization): string { - let route: string; - switch (true) { - case organization.canManageUsers: - route = "manage/members"; - break; - case organization.canViewAssignedCollections || organization.canViewAllCollections: - route = "manage/collections"; - break; - case organization.canManageGroups: - route = "manage/groups"; - break; - } - return route; - } } diff --git a/apps/web/src/app/organizations/organization-routing.module.ts b/apps/web/src/app/organizations/organization-routing.module.ts index 29e02ecb00..ad38375fec 100644 --- a/apps/web/src/app/organizations/organization-routing.module.ts +++ b/apps/web/src/app/organizations/organization-routing.module.ts @@ -9,8 +9,10 @@ import { canAccessOrgAdmin, canManageCollections, } from "@bitwarden/common/abstractions/organization/organization.service.abstraction"; +import { Organization } from "@bitwarden/common/models/domain/organization"; import { OrganizationPermissionsGuard } from "./guards/org-permissions.guard"; +import { OrganizationRedirectGuard } from "./guards/org-redirect.guard"; import { OrganizationLayoutComponent } from "./layouts/organization-layout.component"; import { CollectionsComponent } from "./manage/collections.component"; import { GroupsComponent } from "./manage/groups.component"; @@ -47,7 +49,11 @@ const routes: Routes = [ { path: "", pathMatch: "full", - redirectTo: "members", + canActivate: [OrganizationRedirectGuard], + data: { + autoRedirectCallback: getManageRoute, + }, + children: [], // This is required to make the auto redirect work }, { path: "collections", @@ -94,6 +100,19 @@ const routes: Routes = [ }, ]; +function getManageRoute(organization: Organization): string { + if (organization.canManageUsers) { + return "members"; + } + if (organization.canViewAssignedCollections || organization.canViewAllCollections) { + return "collections"; + } + if (organization.canManageGroups) { + return "groups"; + } + return undefined; +} + @NgModule({ imports: [RouterModule.forChild(routes)], exports: [RouterModule], diff --git a/apps/web/src/app/organizations/reporting/organization-reporting-routing.module.ts b/apps/web/src/app/organizations/reporting/organization-reporting-routing.module.ts index 378325a64c..dae0e6d7de 100644 --- a/apps/web/src/app/organizations/reporting/organization-reporting-routing.module.ts +++ b/apps/web/src/app/organizations/reporting/organization-reporting-routing.module.ts @@ -5,6 +5,7 @@ import { canAccessReportingTab } from "@bitwarden/common/abstractions/organizati import { Organization } from "@bitwarden/common/models/domain/organization"; import { OrganizationPermissionsGuard } from "../guards/org-permissions.guard"; +import { OrganizationRedirectGuard } from "../guards/org-redirect.guard"; import { EventsComponent } from "../manage/events.component"; import { ExposedPasswordsReportComponent } from "../tools/exposed-passwords-report.component"; import { InactiveTwoFactorReportComponent } from "../tools/inactive-two-factor-report.component"; @@ -22,7 +23,15 @@ const routes: Routes = [ canActivate: [OrganizationPermissionsGuard], data: { organizationPermissions: canAccessReportingTab }, children: [ - { path: "", pathMatch: "full", redirectTo: "reports" }, + { + path: "", + pathMatch: "full", + canActivate: [OrganizationRedirectGuard], + data: { + autoRedirectCallback: getReportRoute, + }, + children: [], // This is required to make the auto redirect work, + }, { path: "reports", component: ReportsHomeComponent, @@ -80,6 +89,17 @@ const routes: Routes = [ ], }, ]; + +function getReportRoute(organization: Organization): string { + if (organization.canAccessEventLogs) { + return "events"; + } + if (organization.canAccessReports) { + return "reports"; + } + return undefined; +} + @NgModule({ imports: [RouterModule.forChild(routes)], exports: [RouterModule], diff --git a/apps/web/src/app/organizations/reporting/reporting.component.ts b/apps/web/src/app/organizations/reporting/reporting.component.ts index 671e45b796..5e70e6a40e 100644 --- a/apps/web/src/app/organizations/reporting/reporting.component.ts +++ b/apps/web/src/app/organizations/reporting/reporting.component.ts @@ -22,7 +22,8 @@ export class ReportingComponent implements OnInit, OnDestroy { .pipe( concatMap(async (params) => { this.organization = await this.organizationService.get(params.organizationId); - this.showLeftNav = this.organization.canAccessEventLogs; + this.showLeftNav = + this.organization.canAccessEventLogs && this.organization.canAccessReports; }), takeUntil(this.destroy$) ) diff --git a/apps/web/src/app/organizations/settings/organization-settings-routing.module.ts b/apps/web/src/app/organizations/settings/organization-settings-routing.module.ts index 1d4a4df852..b36e82b152 100644 --- a/apps/web/src/app/organizations/settings/organization-settings-routing.module.ts +++ b/apps/web/src/app/organizations/settings/organization-settings-routing.module.ts @@ -5,6 +5,7 @@ import { canAccessSettingsTab } from "@bitwarden/common/abstractions/organizatio import { Organization } from "@bitwarden/common/models/domain/organization"; import { OrganizationPermissionsGuard } from "../guards/org-permissions.guard"; +import { OrganizationRedirectGuard } from "../guards/org-redirect.guard"; import { PoliciesComponent } from "../policies"; import { AccountComponent } from "./account.component"; @@ -18,7 +19,15 @@ const routes: Routes = [ canActivate: [OrganizationPermissionsGuard], data: { organizationPermissions: canAccessSettingsTab }, children: [ - { path: "", pathMatch: "full", redirectTo: "account" }, + { + path: "", + pathMatch: "full", + canActivate: [OrganizationRedirectGuard], + data: { + autoRedirectCallback: getSettingsRoute, + }, + children: [], // This is required to make the auto redirect work, + }, { path: "account", component: AccountComponent, data: { titleId: "organizationInfo" } }, { path: "two-factor", @@ -45,6 +54,25 @@ const routes: Routes = [ }, ]; +function getSettingsRoute(organization: Organization) { + if (organization.isOwner) { + return "account"; + } + if (organization.canManagePolicies) { + return "policies"; + } + if (organization.canAccessImportExport) { + return ["tools", "import"]; + } + if (organization.canManageSso) { + return "sso"; + } + if (organization.canManageScim) { + return "scim"; + } + return undefined; +} + @NgModule({ imports: [RouterModule.forChild(routes)], exports: [RouterModule], diff --git a/apps/web/src/app/organizations/settings/settings.component.html b/apps/web/src/app/organizations/settings/settings.component.html index d81d7553dc..c2bd1ec19a 100644 --- a/apps/web/src/app/organizations/settings/settings.component.html +++ b/apps/web/src/app/organizations/settings/settings.component.html @@ -4,7 +4,12 @@
{{ "settings" | i18n }}
- + {{ "organizationInfo" | i18n }} {{ "twoStepLogin" | i18n }} diff --git a/libs/common/src/abstractions/organization/organization.service.abstraction.ts b/libs/common/src/abstractions/organization/organization.service.abstraction.ts index 9987223b72..ebdd3bdf90 100644 --- a/libs/common/src/abstractions/organization/organization.service.abstraction.ts +++ b/libs/common/src/abstractions/organization/organization.service.abstraction.ts @@ -9,7 +9,13 @@ export function canAccessVaultTab(org: Organization): boolean { } export function canAccessSettingsTab(org: Organization): boolean { - return org.isOwner; + return ( + org.isOwner || + org.canManagePolicies || + org.canManageSso || + org.canManageScim || + org.canAccessImportExport + ); } export function canAccessMembersTab(org: Organization): boolean { diff --git a/libs/components/src/tabs/tab-nav-bar/tab-link.component.html b/libs/components/src/tabs/tab-nav-bar/tab-link.component.html index 3b0ad6b22c..012e32ecaf 100644 --- a/libs/components/src/tabs/tab-nav-bar/tab-link.component.html +++ b/libs/components/src/tabs/tab-nav-bar/tab-link.component.html @@ -2,6 +2,7 @@ bitTabListItem [routerLink]="disabled ? null : route" routerLinkActive + [routerLinkActiveOptions]="routerLinkMatchOptions" #rla="routerLinkActive" [active]="rla.isActive" [disabled]="disabled" diff --git a/libs/components/src/tabs/tab-nav-bar/tab-link.component.ts b/libs/components/src/tabs/tab-nav-bar/tab-link.component.ts index fc067fb69f..4b9da81824 100644 --- a/libs/components/src/tabs/tab-nav-bar/tab-link.component.ts +++ b/libs/components/src/tabs/tab-nav-bar/tab-link.component.ts @@ -1,6 +1,6 @@ import { FocusableOption } from "@angular/cdk/a11y"; import { AfterViewInit, Component, HostListener, Input, OnDestroy, ViewChild } from "@angular/core"; -import { RouterLinkActive } from "@angular/router"; +import { IsActiveMatchOptions, RouterLinkActive } from "@angular/router"; import { Subject, takeUntil } from "rxjs"; import { TabListItemDirective } from "../shared/tab-list-item.directive"; @@ -17,6 +17,13 @@ export class TabLinkComponent implements FocusableOption, AfterViewInit, OnDestr @ViewChild(TabListItemDirective) tabItem: TabListItemDirective; @ViewChild("rla") routerLinkActive: RouterLinkActive; + readonly routerLinkMatchOptions: IsActiveMatchOptions = { + queryParams: "ignored", + matrixParams: "ignored", + paths: "subset", + fragment: "ignored", + }; + @Input() route: string; @Input() disabled = false;