From c6be3fa19ccb60cf01c58173ca31568fbc1599cd Mon Sep 17 00:00:00 2001 From: Shane Melton Date: Thu, 8 Jun 2023 09:12:52 -0700 Subject: [PATCH] [AC-1437] Introduce Feature Flag route guard (#5465) * Add feature flag route guard and tests * Add additional test for not showing error toast * Strengthen error toast test with message check * Cleanup leaking test state in platformService mock * Negate if statement to reduce nesting * Update return type to CanActivateFn * Use null check instead of undefined * Introduce interface to support different feature flag types - Switch to observable pattern to access serverConfig$ subject - Add catchError handler to allow navigation in case of unexpected exception - Add additional tests * Add additional test for missing feature flag * Remove subscription to the serverConfig observable Introduce type checking logic to determine the appropriately typed flag getter to use in configService * Update the feature flag to fallback to blocking the route on an unexpected exception * Trigger test action * Fix imports after merge with master --- .../src/guard/feature-flag.guard.spec.ts | 152 ++++++++++++++++++ libs/angular/src/guard/feature-flag.guard.ts | 58 +++++++ 2 files changed, 210 insertions(+) create mode 100644 libs/angular/src/guard/feature-flag.guard.spec.ts create mode 100644 libs/angular/src/guard/feature-flag.guard.ts diff --git a/libs/angular/src/guard/feature-flag.guard.spec.ts b/libs/angular/src/guard/feature-flag.guard.spec.ts new file mode 100644 index 0000000000..bba879278f --- /dev/null +++ b/libs/angular/src/guard/feature-flag.guard.spec.ts @@ -0,0 +1,152 @@ +import { Component } from "@angular/core"; +import { TestBed } from "@angular/core/testing"; +import { CanActivateFn, Router } from "@angular/router"; +import { RouterTestingModule } from "@angular/router/testing"; +import { mock, MockProxy } from "jest-mock-extended"; + +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { ConfigServiceAbstraction } from "@bitwarden/common/platform/abstractions/config/config.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 { I18nMockService } from "@bitwarden/components/src"; + +import { canAccessFeature } from "./feature-flag.guard"; + +@Component({ template: "" }) +export class EmptyComponent {} + +describe("canAccessFeature", () => { + const testFlag: FeatureFlag = "test-flag" as FeatureFlag; + const featureRoute = "enabled-feature"; + const redirectRoute = "redirect"; + + let mockConfigService: MockProxy; + let mockPlatformUtilsService: MockProxy; + + const setup = (featureGuard: CanActivateFn, flagValue: any) => { + mockConfigService = mock(); + mockPlatformUtilsService = mock(); + + // Mock the correct getter based on the type of flagValue; also mock default values if one is not provided + if (typeof flagValue === "boolean") { + mockConfigService.getFeatureFlagBool.mockImplementation((flag, defaultValue = false) => + flag == testFlag ? Promise.resolve(flagValue) : Promise.resolve(defaultValue) + ); + } else if (typeof flagValue === "string") { + mockConfigService.getFeatureFlagString.mockImplementation((flag, defaultValue = "") => + flag == testFlag ? Promise.resolve(flagValue) : Promise.resolve(defaultValue) + ); + } else if (typeof flagValue === "number") { + mockConfigService.getFeatureFlagNumber.mockImplementation((flag, defaultValue = 0) => + flag == testFlag ? Promise.resolve(flagValue) : Promise.resolve(defaultValue) + ); + } + + const testBed = TestBed.configureTestingModule({ + imports: [ + RouterTestingModule.withRoutes([ + { path: "", component: EmptyComponent }, + { + path: featureRoute, + component: EmptyComponent, + canActivate: [featureGuard], + }, + { path: redirectRoute, component: EmptyComponent }, + ]), + ], + providers: [ + { provide: ConfigServiceAbstraction, useValue: mockConfigService }, + { provide: PlatformUtilsService, useValue: mockPlatformUtilsService }, + { provide: LogService, useValue: mock() }, + { + provide: I18nService, + useValue: new I18nMockService({ + accessDenied: "Access Denied!", + }), + }, + ], + }); + return { + router: testBed.inject(Router), + }; + }; + + it("successfully navigates when the feature flag is enabled", async () => { + const { router } = setup(canAccessFeature(testFlag), true); + + await router.navigate([featureRoute]); + + expect(router.url).toBe(`/${featureRoute}`); + }); + + it("successfully navigates when the feature flag value matches the required value", async () => { + const { router } = setup(canAccessFeature(testFlag, "some-value"), "some-value"); + + await router.navigate([featureRoute]); + + expect(router.url).toBe(`/${featureRoute}`); + }); + + it("fails to navigate when the feature flag is disabled", async () => { + const { router } = setup(canAccessFeature(testFlag), false); + + await router.navigate([featureRoute]); + + expect(router.url).toBe("/"); + }); + + it("fails to navigate when the feature flag value does not match the required value", async () => { + const { router } = setup(canAccessFeature(testFlag, "some-value"), "some-wrong-value"); + + await router.navigate([featureRoute]); + + expect(router.url).toBe("/"); + }); + + it("fails to navigate when the feature flag does not exist", async () => { + const { router } = setup(canAccessFeature("missing-flag" as FeatureFlag), true); + + await router.navigate([featureRoute]); + + expect(router.url).toBe("/"); + }); + + it("shows an error toast when the feature flag is disabled", async () => { + const { router } = setup(canAccessFeature(testFlag), false); + + await router.navigate([featureRoute]); + + expect(mockPlatformUtilsService.showToast).toHaveBeenCalledWith( + "error", + null, + "Access Denied!" + ); + }); + + it("does not show an error toast when the feature flag is enabled", async () => { + const { router } = setup(canAccessFeature(testFlag), true); + + await router.navigate([featureRoute]); + + expect(mockPlatformUtilsService.showToast).not.toHaveBeenCalled(); + }); + + it("redirects to the specified redirect url when the feature flag is disabled", async () => { + const { router } = setup(canAccessFeature(testFlag, true, redirectRoute), false); + + await router.navigate([featureRoute]); + + expect(router.url).toBe(`/${redirectRoute}`); + }); + + it("fails to navigate when the config service throws an unexpected exception", async () => { + const { router } = setup(canAccessFeature(testFlag), true); + + mockConfigService.getFeatureFlagBool.mockImplementation(() => Promise.reject("Some error")); + + await router.navigate([featureRoute]); + + expect(router.url).toBe("/"); + }); +}); diff --git a/libs/angular/src/guard/feature-flag.guard.ts b/libs/angular/src/guard/feature-flag.guard.ts new file mode 100644 index 0000000000..f4596f3cf4 --- /dev/null +++ b/libs/angular/src/guard/feature-flag.guard.ts @@ -0,0 +1,58 @@ +import { inject } from "@angular/core"; +import { CanActivateFn, Router } from "@angular/router"; + +import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; +import { ConfigServiceAbstraction } from "@bitwarden/common/platform/abstractions/config/config.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"; + +// Replace this with a type safe lookup of the feature flag values in PM-2282 +type FlagValue = boolean | number | string; + +/** + * Returns a CanActivateFn that checks if the feature flag is enabled. If not, it shows an "Access Denied!" + * toast and optionally redirects to the specified url. + * @param featureFlag - The feature flag to check + * @param requiredFlagValue - Optional value to the feature flag must be equal to, defaults to true + * @param redirectUrlOnDisabled - Optional url to redirect to if the feature flag is disabled + */ +export const canAccessFeature = ( + featureFlag: FeatureFlag, + requiredFlagValue: FlagValue = true, + redirectUrlOnDisabled?: string +): CanActivateFn => { + return async () => { + const configService = inject(ConfigServiceAbstraction); + const platformUtilsService = inject(PlatformUtilsService); + const router = inject(Router); + const i18nService = inject(I18nService); + const logService = inject(LogService); + + let flagValue: FlagValue; + + try { + if (typeof requiredFlagValue === "boolean") { + flagValue = await configService.getFeatureFlagBool(featureFlag); + } else if (typeof requiredFlagValue === "number") { + flagValue = await configService.getFeatureFlagNumber(featureFlag); + } else if (typeof requiredFlagValue === "string") { + flagValue = await configService.getFeatureFlagString(featureFlag); + } + + if (flagValue === requiredFlagValue) { + return true; + } + + platformUtilsService.showToast("error", null, i18nService.t("accessDenied")); + + if (redirectUrlOnDisabled != null) { + return router.createUrlTree([redirectUrlOnDisabled]); + } + return false; + } catch (e) { + logService.error(e); + return false; + } + }; +};