[EC-473] Add feature flags to common code (#3324)

apps/browser/src/decorators/dev-flag.decorator.ts

@@ -1,6 +1,6 @@
-import { devFlagEnabled, DevFlagName } from "../flags";
+import { devFlagEnabled, DevFlags } from "../flags";
 
-export function devFlag(flag: DevFlagName) {
+export function devFlag(flag: keyof DevFlags) {
   return function (target: any, propertyKey: string, descriptor: PropertyDescriptor) {
     const originalMethod = descriptor.value;
     descriptor.value = function (...args: any[]) {

apps/browser/src/flags.ts

@@ -1,61 +1,32 @@
+import {
+  flagEnabled as baseFlagEnabled,
+  devFlagEnabled as baseDevFlagEnabled,
+  devFlagValue as baseDevFlagValue,
+  SharedFlags,
+  SharedDevFlags,
+} from "@bitwarden/common/misc/flags";
+
 import { GroupPolicyEnvironment } from "./types/group-policy-environment";
 
-function getFlags<T>(envFlags: string | T): T {
-  if (typeof envFlags === "string") {
-    return JSON.parse(envFlags) as T;
-  } else {
-    return envFlags as T;
-  }
-}
+// required to avoid linting errors when there are no flags
+/* eslint-disable-next-line @typescript-eslint/ban-types */
+export type Flags = {} & SharedFlags;
 
-/* Placeholder for when we have a relevant feature flag
-export type Flags = { test?: boolean };
-export type FlagName = keyof Flags;
-export function flagEnabled(flag: FlagName): boolean {
-  const flags = getFlags<Flags>(process.env.FLAGS);
-  return flags[flag] == null || flags[flag];
-}
-*/
-
-/**
- * These flags are useful for development and testing.
- * Dev Flags are always OFF in production.
- */
+// required to avoid linting errors when there are no flags
+/* eslint-disable-next-line @typescript-eslint/ban-types */
 export type DevFlags = {
   storeSessionDecrypted?: boolean;
   managedEnvironment?: GroupPolicyEnvironment;
-};
+} & SharedDevFlags;
 
-export type DevFlagName = keyof DevFlags;
-
-/**
- * Gets whether the given dev flag is truthy.
- * Gets the value of a dev flag from environment.
- * Will always return false unless in development.
- * @param flag The name of the dev flag to check
- * @returns The value of the flag
- */
-export function devFlagEnabled(flag: DevFlagName): boolean {
-  if (process.env.ENV !== "development") {
-    return false;
-  }
-
-  const devFlags = getFlags<DevFlags>(process.env.DEV_FLAGS);
-  return devFlags[flag] == null || !!devFlags[flag];
+export function flagEnabled(flag: keyof Flags): boolean {
+  return baseFlagEnabled<Flags>(flag);
 }
 
-/**
- * Gets the value of a dev flag from environment.
- * Will always return false unless in development.
- * @param flag The name of the dev flag to check
- * @returns The value of the flag
- * @throws Error if the flag is not enabled
- */
-export function devFlagValue<K extends DevFlagName>(flag: K): DevFlags[K] {
-  if (!devFlagEnabled(flag)) {
-    throw new Error(`This method should not be called, it is protected by a disabled dev flag.`);
-  }
-
-  const devFlags = getFlags<DevFlags>(process.env.DEV_FLAGS);
-  return devFlags[flag];
+export function devFlagEnabled(flag: keyof DevFlags) {
+  return baseDevFlagEnabled<DevFlags>(flag);
+}
+
+export function devFlagValue(flag: keyof DevFlags) {
+  return baseDevFlagValue(flag);
 }

apps/cli/spec/utils.spec.ts

@@ -1,27 +0,0 @@
-import { FlagName } from "../src/flags";
-import { CliUtils } from "../src/utils";
-describe("flagEnabled", () => {
-  it("is true if flag is null", () => {
-    process.env.FLAGS = JSON.stringify({ test: null });
-
-    expect(CliUtils.flagEnabled("test" as FlagName)).toBe(true);
-  });
-
-  it("is true if flag is undefined", () => {
-    process.env.FLAGS = JSON.stringify({});
-
-    expect(CliUtils.flagEnabled("test" as FlagName)).toBe(true);
-  });
-
-  it("is true if flag is true", () => {
-    process.env.FLAGS = JSON.stringify({ test: true });
-
-    expect(CliUtils.flagEnabled("test" as FlagName)).toBe(true);
-  });
-
-  it("is false if flag is false", () => {
-    process.env.FLAGS = JSON.stringify({ test: false });
-
-    expect(CliUtils.flagEnabled("test" as FlagName)).toBe(false);
-  });
-});

apps/cli/src/flags.ts

@@ -1,5 +1,27 @@
-// Remove this linter hint if any flags exist
-// eslint-disable-next-line @typescript-eslint/ban-types
-export type Flags = {};
+import {
+  flagEnabled as baseFlagEnabled,
+  devFlagEnabled as baseDevFlagEnabled,
+  devFlagValue as baseDevFlagValue,
+  SharedFlags,
+  SharedDevFlags,
+} from "@bitwarden/common/misc/flags";
 
-export type FlagName = keyof Flags;
+// required to avoid linting errors when there are no flags
+/* eslint-disable-next-line @typescript-eslint/ban-types */
+export type Flags = {} & SharedFlags;
+
+// required to avoid linting errors when there are no flags
+/* eslint-disable-next-line @typescript-eslint/ban-types */
+export type DevFlags = {} & SharedDevFlags;
+
+export function flagEnabled(flag: keyof Flags): boolean {
+  return baseFlagEnabled<Flags>(flag);
+}
+
+export function devFlagEnabled(flag: keyof DevFlags) {
+  return baseDevFlagEnabled<DevFlags>(flag);
+}
+
+export function devFlagValue(flag: keyof DevFlags) {
+  return baseDevFlagValue(flag);
+}

apps/cli/src/utils.ts

@@ -13,8 +13,6 @@ import { FolderView } from "@bitwarden/common/models/view/folderView";
 import { Response } from "@bitwarden/node/cli/models/response";
 import { MessageResponse } from "@bitwarden/node/cli/models/response/messageResponse";
 
-import { FlagName, Flags } from "./flags";
-
 export class CliUtils {
   static writeLn(s: string, finalLine = false, error = false) {
     const stream = error ? process.stderr : process.stdout;
@@ -253,18 +251,4 @@ export class CliUtils {
   static convertBooleanOption(optionValue: any) {
     return optionValue || optionValue === "" ? true : false;
   }
-
-  static flagEnabled(flag: FlagName) {
-    return this.flags[flag] == null || this.flags[flag];
-  }
-
-  private static get flags(): Flags {
-    const envFlags = process.env.FLAGS;
-
-    if (typeof envFlags === "string") {
-      return JSON.parse(envFlags) as Flags;
-    } else {
-      return envFlags as Flags;
-    }
-  }
 }

apps/desktop/src/flags.ts

@@ -0,0 +1,27 @@
+import {
+  flagEnabled as baseFlagEnabled,
+  devFlagEnabled as baseDevFlagEnabled,
+  devFlagValue as baseDevFlagValue,
+  SharedFlags,
+  SharedDevFlags,
+} from "@bitwarden/common/misc/flags";
+
+// required to avoid linting errors when there are no flags
+/* eslint-disable-next-line @typescript-eslint/ban-types */
+export type Flags = {} & SharedFlags;
+
+// required to avoid linting errors when there are no flags
+/* eslint-disable-next-line @typescript-eslint/ban-types */
+export type DevFlags = {} & SharedDevFlags;
+
+export function flagEnabled(flag: keyof Flags): boolean {
+  return baseFlagEnabled<Flags>(flag);
+}
+
+export function devFlagEnabled(flag: keyof DevFlags) {
+  return baseDevFlagEnabled<DevFlags>(flag);
+}
+
+export function devFlagValue(flag: keyof DevFlags) {
+  return baseDevFlagValue(flag);
+}

apps/web/src/app/oss-routing.module.ts

@@ -5,7 +5,7 @@ import { AuthGuard } from "@bitwarden/angular/guards/auth.guard";
 import { LockGuard } from "@bitwarden/angular/guards/lock.guard";
 import { UnauthGuard } from "@bitwarden/angular/guards/unauth.guard";
 
-import { flagEnabled, FlagName } from "../utils/flags";
+import { flagEnabled, Flags } from "../utils/flags";
 
 import { AcceptEmergencyComponent } from "./accounts/accept-emergency.component";
 import { AcceptOrganizationComponent } from "./accounts/accept-organization.component";
@@ -261,7 +261,7 @@ const routes: Routes = [
 })
 export class OssRoutingModule {}
 
-export function buildFlaggedRoute(flagName: FlagName, route: Route): Route {
+export function buildFlaggedRoute(flagName: keyof Flags, route: Route): Route {
   return flagEnabled(flagName)
     ? route
     : {

apps/web/src/utils/flags.ts

@@ -1,19 +1,29 @@
+import {
+  flagEnabled as baseFlagEnabled,
+  devFlagEnabled as baseDevFlagEnabled,
+  devFlagValue as baseDevFlagValue,
+  SharedFlags,
+  SharedDevFlags,
+} from "@bitwarden/common/misc/flags";
+
+// required to avoid linting errors when there are no flags
+/* eslint-disable-next-line @typescript-eslint/ban-types */
 export type Flags = {
   showTrial?: boolean;
-};
+} & SharedFlags;
 
-export type FlagName = keyof Flags;
+// required to avoid linting errors when there are no flags
+/* eslint-disable-next-line @typescript-eslint/ban-types */
+export type DevFlags = {} & SharedDevFlags;
 
-export function flagEnabled(flag: FlagName): boolean {
-  return flags()[flag] == null || flags()[flag];
+export function flagEnabled(flag: keyof Flags): boolean {
+  return baseFlagEnabled<Flags>(flag);
 }
 
-function flags(): Flags {
-  const envFlags = process.env.FLAGS as string | Flags;
-
-  if (typeof envFlags === "string") {
-    return JSON.parse(envFlags) as Flags;
-  } else {
-    return envFlags as Flags;
-  }
+export function devFlagEnabled(flag: keyof DevFlags) {
+  return baseDevFlagEnabled<DevFlags>(flag);
+}
+
+export function devFlagValue(flag: keyof DevFlags) {
+  return baseDevFlagValue(flag);
 }

libs/common/src/misc/flags.spec.ts

@@ -0,0 +1,99 @@
+import { flagEnabled, devFlagEnabled, devFlagValue } from "./flags";
+
+describe("flagEnabled", () => {
+  beforeEach(() => {
+    process.env.FLAGS = JSON.stringify({});
+  });
+
+  it("returns true by default", () => {
+    expect(flagEnabled<any>("nonExistentFlag")).toBe(true);
+  });
+
+  it("returns true if enabled", () => {
+    process.env.FLAGS = JSON.stringify({
+      newFeature: true,
+    });
+
+    expect(flagEnabled<any>("newFeature")).toBe(true);
+  });
+
+  it("returns false if disabled", () => {
+    process.env.FLAGS = JSON.stringify({
+      newFeature: false,
+    });
+
+    expect(flagEnabled<any>("newFeature")).toBe(false);
+  });
+});
+
+describe("devFlagEnabled", () => {
+  beforeEach(() => {
+    process.env.DEV_FLAGS = JSON.stringify({});
+  });
+
+  describe("in a development environment", () => {
+    beforeEach(() => {
+      process.env.ENV = "development";
+    });
+
+    it("returns true by default", () => {
+      expect(devFlagEnabled<any>("nonExistentFlag")).toBe(true);
+    });
+
+    it("returns true if enabled", () => {
+      process.env.DEV_FLAGS = JSON.stringify({
+        devHack: true,
+      });
+
+      expect(devFlagEnabled<any>("devHack")).toBe(true);
+    });
+
+    it("returns true if truthy", () => {
+      process.env.DEV_FLAGS = JSON.stringify({
+        devHack: { key: 3 },
+      });
+
+      expect(devFlagEnabled<any>("devHack")).toBe(true);
+    });
+
+    it("returns false if disabled", () => {
+      process.env.DEV_FLAGS = JSON.stringify({
+        devHack: false,
+      });
+
+      expect(devFlagEnabled<any>("devHack")).toBe(false);
+    });
+  });
+
+  it("always returns false in prod", () => {
+    process.env.ENV = "production";
+    process.env.DEV_FLAGS = JSON.stringify({
+      devHack: true,
+    });
+
+    expect(devFlagEnabled<any>("devHack")).toBe(false);
+  });
+});
+
+describe("devFlagValue", () => {
+  beforeEach(() => {
+    process.env.DEV_FLAGS = JSON.stringify({});
+    process.env.ENV = "development";
+  });
+
+  it("throws if dev flag is disabled", () => {
+    process.env.DEV_FLAGS = JSON.stringify({
+      devHack: false,
+    });
+
+    expect(() => devFlagValue<any>("devHack")).toThrow("it is protected by a disabled dev flag");
+  });
+
+  it("returns the dev flag value", () => {
+    process.env.DEV_FLAGS = JSON.stringify({
+      devHack: "Hello world",
+    });
+
+    expect(devFlagValue<any>("devHack")).toBe("Hello world");
+  });
+});

libs/common/src/misc/flags.ts

@@ -0,0 +1,61 @@
+// required to avoid linting errors when there are no flags
+/* eslint-disable @typescript-eslint/ban-types */
+export type SharedFlags = {};
+
+// required to avoid linting errors when there are no flags
+/* eslint-disable @typescript-eslint/ban-types */
+export type SharedDevFlags = {};
+
+function getFlags<T>(envFlags: string | T): T {
+  if (typeof envFlags === "string") {
+    return JSON.parse(envFlags) as T;
+  } else {
+    return envFlags as T;
+  }
+}
+
+/**
+ * Gets the value of a feature flag from environment.
+ * All flags default to "on" (true).
+ * Only use for shared code in `libs`, otherwise use the client-specific function.
+ * @param flag The name of the feature flag to check
+ * @returns The value of the flag
+ */
+export function flagEnabled<Flags extends SharedFlags>(flag: keyof Flags): boolean {
+  const flags = getFlags<Flags>(process.env.FLAGS);
+  return flags[flag] == null || !!flags[flag];
+}
+
+/**
+ * Gets the value of a dev flag from environment.
+ * Will always return false unless in development.
+ * Only use for shared code in `libs`, otherwise use the client-specific function.
+ * @param flag The name of the dev flag to check
+ * @returns The value of the flag
+ */
+export function devFlagEnabled<DevFlags extends SharedDevFlags>(flag: keyof DevFlags): boolean {
+  if (process.env.ENV !== "development") {
+    return false;
+  }
+
+  const devFlags = getFlags<DevFlags>(process.env.DEV_FLAGS);
+  return devFlags[flag] == null || !!devFlags[flag];
+}
+
+/**
+ * Gets the value of a dev flag from environment.
+ * Will always return false unless in development.
+ * @param flag The name of the dev flag to check
+ * @returns The value of the flag
+ * @throws Error if the flag is not enabled
+ */
+export function devFlagValue<DevFlags extends SharedDevFlags>(
+  flag: keyof DevFlags
+): DevFlags[keyof DevFlags] {
+  if (!devFlagEnabled(flag)) {
+    throw new Error(`This method should not be called, it is protected by a disabled dev flag.`);
+  }
+
+  const devFlags = getFlags<DevFlags>(process.env.DEV_FLAGS);
+  return devFlags[flag];
+}