[PM-9613] port forwarders to integrations (#10075)

* introduced forwarder integrations
* simply contexts
* report error and message when both are present in an RPC response

libs/common/src/tools/integration/integration-configuration.ts

@@ -1,9 +1,12 @@
 import { IntegrationContext } from "./integration-context";
 import { IntegrationMetadata } from "./integration-metadata";
-import { ApiSettings, TokenHeader } from "./rpc";
+import { ApiSettings, IntegrationRequest, TokenHeader } from "./rpc";
 
 /** Configures integration-wide settings */
 export type IntegrationConfiguration = IntegrationMetadata & {
   /** Creates the authentication header for all integration remote procedure calls */
-  authenticate: (settings: ApiSettings, context: IntegrationContext) => TokenHeader;
+  authenticate: (
+    request: IntegrationRequest,
+    context: IntegrationContext<ApiSettings>,
+  ) => TokenHeader;
 };

libs/common/src/tools/integration/integration-context.spec.ts

@@ -25,7 +25,7 @@ describe("IntegrationContext", () => {
 
   describe("baseUrl", () => {
     it("outputs the base url from metadata", () => {
-      const context = new IntegrationContext(EXAMPLE_META, i18n);
+      const context = new IntegrationContext(EXAMPLE_META, null, i18n);
 
       const result = context.baseUrl();
 
@@ -41,15 +41,15 @@ describe("IntegrationContext", () => {
       };
       i18n.t.mockReturnValue("error");
 
-      const context = new IntegrationContext(noBaseUrl, i18n);
+      const context = new IntegrationContext(noBaseUrl, null, i18n);
 
       expect(() => context.baseUrl()).toThrow("error");
     });
 
     it("reads from the settings", () => {
-      const context = new IntegrationContext(EXAMPLE_META, i18n);
+      const context = new IntegrationContext(EXAMPLE_META, { baseUrl: "httpbin.org" }, i18n);
 
-      const result = context.baseUrl({ baseUrl: "httpbin.org" });
+      const result = context.baseUrl();
 
       expect(result).toBe("httpbin.org");
     });
@@ -62,9 +62,9 @@ describe("IntegrationContext", () => {
         baseUrl: "example.com",
         selfHost: "never",
       };
-      const context = new IntegrationContext(selfHostNever, i18n);
+      const context = new IntegrationContext(selfHostNever, { baseUrl: "httpbin.org" }, i18n);
 
-      const result = context.baseUrl({ baseUrl: "httpbin.org" });
+      const result = context.baseUrl();
 
       expect(result).toBe("example.com");
     });
@@ -77,11 +77,22 @@ describe("IntegrationContext", () => {
         baseUrl: "example.com",
         selfHost: "always",
       };
-      const context = new IntegrationContext(selfHostAlways, i18n);
+      const context = new IntegrationContext(selfHostAlways, { baseUrl: "http.bin" }, i18n);
 
       // expect success
-      const result = context.baseUrl({ baseUrl: "http.bin" });
+      const result = context.baseUrl();
       expect(result).toBe("http.bin");
+    });
+
+    it("fails when the settings are empty and selfhost is 'always'", () => {
+      const selfHostAlways: IntegrationMetadata = {
+        id: "simplelogin" as IntegrationId, // arbitrary
+        name: "Example",
+        extends: ["forwarder"], // arbitrary
+        baseUrl: "example.com",
+        selfHost: "always",
+      };
+      const context = new IntegrationContext(selfHostAlways, {}, i18n);
 
       // expect error
       i18n.t.mockReturnValue("error");
@@ -97,7 +108,7 @@ describe("IntegrationContext", () => {
         selfHost: "maybe",
       };
 
-      const context = new IntegrationContext(selfHostMaybe, i18n);
+      const context = new IntegrationContext(selfHostMaybe, null, i18n);
 
       const result = context.baseUrl();
 
@@ -113,9 +124,9 @@ describe("IntegrationContext", () => {
         selfHost: "maybe",
       };
 
-      const context = new IntegrationContext(selfHostMaybe, i18n);
+      const context = new IntegrationContext(selfHostMaybe, { baseUrl: "httpbin.org" }, i18n);
 
-      const result = context.baseUrl({ baseUrl: "httpbin.org" });
+      const result = context.baseUrl();
 
       expect(result).toBe("httpbin.org");
     });
@@ -123,39 +134,47 @@ describe("IntegrationContext", () => {
 
   describe("authenticationToken", () => {
     it("reads from the settings", () => {
-      const context = new IntegrationContext(EXAMPLE_META, i18n);
+      const context = new IntegrationContext(EXAMPLE_META, { token: "example" }, i18n);
 
-      const result = context.authenticationToken({ token: "example" });
+      const result = context.authenticationToken();
 
       expect(result).toBe("example");
     });
 
-    it("base64 encodes the read value", () => {
-      const context = new IntegrationContext(EXAMPLE_META, i18n);
+    it("suffix is appended to the token", () => {
+      const context = new IntegrationContext(EXAMPLE_META, { token: "example" }, i18n);
 
-      const result = context.authenticationToken({ token: "example" }, { base64: true });
+      const result = context.authenticationToken({ suffix: " with suffix" });
+
+      expect(result).toBe("example with suffix");
+    });
+
+    it("base64 encodes the read value", () => {
+      const context = new IntegrationContext(EXAMPLE_META, { token: "example" }, i18n);
+
+      const result = context.authenticationToken({ base64: true });
 
       expect(result).toBe("ZXhhbXBsZQ==");
     });
 
     it("throws an error when the value is missing", () => {
-      const context = new IntegrationContext(EXAMPLE_META, i18n);
+      const context = new IntegrationContext(EXAMPLE_META, {}, i18n);
       i18n.t.mockReturnValue("error");
 
-      expect(() => context.authenticationToken({})).toThrow("error");
+      expect(() => context.authenticationToken()).toThrow("error");
     });
 
-    it("throws an error when the value is empty", () => {
-      const context = new IntegrationContext(EXAMPLE_META, i18n);
+    it.each([[undefined], [null], [""]])("throws an error when the value is %p", (token) => {
+      const context = new IntegrationContext(EXAMPLE_META, { token }, i18n);
       i18n.t.mockReturnValue("error");
 
-      expect(() => context.authenticationToken({ token: "" })).toThrow("error");
+      expect(() => context.authenticationToken()).toThrow("error");
     });
   });
 
   describe("website", () => {
     it("returns the website", () => {
-      const context = new IntegrationContext(EXAMPLE_META, i18n);
+      const context = new IntegrationContext(EXAMPLE_META, null, i18n);
 
       const result = context.website({ website: "www.example.com" });
 
@@ -163,7 +182,7 @@ describe("IntegrationContext", () => {
     });
 
     it("returns an empty string when the website is not specified", () => {
-      const context = new IntegrationContext(EXAMPLE_META, i18n);
+      const context = new IntegrationContext(EXAMPLE_META, null, i18n);
 
       const result = context.website({ website: undefined });
 
@@ -173,7 +192,7 @@ describe("IntegrationContext", () => {
 
   describe("generatedBy", () => {
     it("creates generated by text", () => {
-      const context = new IntegrationContext(EXAMPLE_META, i18n);
+      const context = new IntegrationContext(EXAMPLE_META, null, i18n);
       i18n.t.mockReturnValue("result");
 
       const result = context.generatedBy({ website: null });
@@ -183,7 +202,7 @@ describe("IntegrationContext", () => {
     });
 
     it("creates generated by text including the website", () => {
-      const context = new IntegrationContext(EXAMPLE_META, i18n);
+      const context = new IntegrationContext(EXAMPLE_META, null, i18n);
       i18n.t.mockReturnValue("result");
 
       const result = context.generatedBy({ website: "www.example.com" });

libs/common/src/tools/integration/integration-context.ts

@@ -2,30 +2,33 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic
 import { Utils } from "@bitwarden/common/platform/misc/utils";
 
 import { IntegrationMetadata } from "./integration-metadata";
-import { ApiSettings, SelfHostedApiSettings, IntegrationRequest } from "./rpc";
+import { ApiSettings, IntegrationRequest } from "./rpc";
 
 /** Utilities for processing integration settings */
-export class IntegrationContext {
+export class IntegrationContext<Settings extends object> {
   /** Instantiates an integration context
    *  @param metadata - defines integration capabilities
    *  @param i18n - localizes error messages
    */
   constructor(
     readonly metadata: IntegrationMetadata,
+    protected settings: Settings,
     protected i18n: I18nService,
   ) {}
 
   /** Lookup the integration's baseUrl
-   *  @param settings settings that override the baseUrl.
    *  @returns the baseUrl for the API's integration point.
    *   - By default this is defined by the metadata
    *   - When a service allows self-hosting, this can be supplied by `settings`.
    *  @throws a localized error message when a base URL is neither defined by the metadata or
    *   supplied by an argument.
    */
-  baseUrl(settings?: SelfHostedApiSettings) {
+  baseUrl(): string {
     // normalize baseUrl
-    const setting = settings && "baseUrl" in settings ? settings.baseUrl : "";
+    const setting =
+      (this.settings && "baseUrl" in this.settings
+        ? (this.settings.baseUrl as string)
+        : undefined) ?? "";
     let result = "";
 
     // look up definition
@@ -47,18 +50,24 @@ export class IntegrationContext {
   }
 
   /** look up a service API's authentication token
-   *  @param settings store the API token
    *  @param options.base64 when `true`, base64 encodes the result. Defaults to `false`.
+   *  @param options.suffix a string to append to the token. Defaults to empty.
    *  @returns the user's authentication token
    *  @throws a localized error message when the token is invalid.
+   *  @remarks the string is thrown for backwards compatibility
    */
-  authenticationToken(settings: ApiSettings, options: { base64?: boolean } = null) {
-    if (!settings.token || settings.token === "") {
+  authenticationToken(
+    options: { base64?: boolean; suffix?: string } = null,
+  ): Settings extends ApiSettings ? string : never {
+    // normalize `token` then assert it has a value
+    let token = "token" in this.settings ? ((this.settings.token as string) ?? "") : "";
+    if (token === "") {
       const error = this.i18n.t("forwaderInvalidToken", this.metadata.name);
       throw error;
     }
 
-    let token = settings.token;
+    // if a suffix exists, it needs to be included before encoding
+    token += options?.suffix ?? "";
     if (options?.base64) {
       token = Utils.fromUtf8ToB64(token);
     }

libs/common/src/tools/integration/rpc/rest-client.spec.ts

@@ -51,17 +51,41 @@ describe("RestClient", () => {
       expect(api.nativeFetch).toHaveBeenCalledWith(expectedRpc.fetchRequest);
     });
 
-    it.each([[401], [403]])(
+    it.each([[401] /*,[403]*/])(
       "throws an invalid token error when HTTP status is %i",
       async (status) => {
         const client = new RestClient(api, i18n);
         const request: IntegrationRequest = { website: null };
-        const response = mock<Response>({ status });
+        const response = mock<Response>({ status, statusText: null });
         api.nativeFetch.mockResolvedValue(response);
 
         const result = client.fetchJson(rpc, request);
 
-        await expect(result).rejects.toEqual("forwarderInvalidToken");
+        await expect(result).rejects.toEqual("forwaderInvalidToken");
+      },
+    );
+
+    it.each([
+      [401, null, null],
+      [401, undefined, undefined],
+      [401, undefined, null],
+      [403, null, null],
+      [403, undefined, undefined],
+      [403, undefined, null],
+    ])(
+      "throws an invalid token error when HTTP status is %i, message is %p, and error is %p",
+      async (status) => {
+        const client = new RestClient(api, i18n);
+        const request: IntegrationRequest = { website: null };
+        const response = mock<Response>({
+          status,
+          text: () => Promise.resolve(`{ "message": null, "error": null }`),
+        });
+        api.nativeFetch.mockResolvedValue(response);
+
+        const result = client.fetchJson(rpc, request);
+
+        await expect(result).rejects.toEqual("forwaderInvalidToken");
       },
     );
 
@@ -83,16 +107,73 @@ describe("RestClient", () => {
 
         const result = client.fetchJson(rpc, request);
 
-        await expect(result).rejects.toEqual("forwarderInvalidTokenWithMessage");
+        await expect(result).rejects.toEqual("forwaderInvalidTokenWithMessage");
         expect(i18n.t).toHaveBeenCalledWith(
-          "forwarderInvalidTokenWithMessage",
+          "forwaderInvalidTokenWithMessage",
           "mock",
           "expected message",
         );
       },
     );
 
-    it.each([[500], [501]])(
+    it.each([[401], [403]])(
+      "throws an invalid token detailed error when HTTP status is %i and the payload has a %s",
+      async (status) => {
+        const client = new RestClient(api, i18n);
+        const request: IntegrationRequest = { website: null };
+        const response = mock<Response>({
+          status,
+          text: () =>
+            Promise.resolve(`{ "error": "that happened", "message": "expected message" }`),
+        });
+        api.nativeFetch.mockResolvedValue(response);
+
+        const result = client.fetchJson(rpc, request);
+
+        await expect(result).rejects.toEqual("forwaderInvalidTokenWithMessage");
+        expect(i18n.t).toHaveBeenCalledWith(
+          "forwaderInvalidTokenWithMessage",
+          "mock",
+          "that happened: expected message",
+        );
+      },
+    );
+
+    it.each([[429], [500], [501]])(
+      "throws a forwarder error when HTTP status is %i",
+      async (status) => {
+        const client = new RestClient(api, i18n);
+        const request: IntegrationRequest = { website: null };
+        const response = mock<Response>({ status, statusText: null });
+        api.nativeFetch.mockResolvedValue(response);
+
+        const result = client.fetchJson(rpc, request);
+
+        await expect(result).rejects.toEqual("forwarderUnknownError");
+        expect(i18n.t).toHaveBeenCalledWith("forwarderUnknownError", "mock", undefined);
+      },
+    );
+
+    it.each([[429], [500], [501]])(
+      "throws a forwarder error when HTTP status is %i and the body is empty",
+      async (status) => {
+        const client = new RestClient(api, i18n);
+        const request: IntegrationRequest = { website: null };
+        const response = mock<Response>({
+          status,
+          statusText: null,
+          text: () => Promise.resolve(""),
+        });
+        api.nativeFetch.mockResolvedValue(response);
+
+        const result = client.fetchJson(rpc, request);
+
+        await expect(result).rejects.toEqual("forwarderUnknownError");
+        expect(i18n.t).toHaveBeenCalledWith("forwarderUnknownError", "mock", undefined);
+      },
+    );
+
+    it.each([[429], [500], [501]])(
       "throws a forwarder error with the status text when HTTP status is %i",
       async (status) => {
         const client = new RestClient(api, i18n);
@@ -108,8 +189,10 @@ describe("RestClient", () => {
     );
 
     it.each([
+      [429, "message"],
       [500, "message"],
       [500, "message"],
+      [429, "error"],
       [501, "error"],
       [501, "error"],
     ])(
@@ -130,6 +213,61 @@ describe("RestClient", () => {
       },
     );
 
+    it.each([[429], [500], [500]])(
+      "throws a detailed forwarder error when HTTP status is %i and the payload is a string",
+      async (status) => {
+        const client = new RestClient(api, i18n);
+        const request: IntegrationRequest = { website: null };
+        const response = mock<Response>({
+          status,
+          text: () => Promise.resolve('"expected message"'),
+        });
+        api.nativeFetch.mockResolvedValue(response);
+
+        const result = client.fetchJson(rpc, request);
+
+        await expect(result).rejects.toEqual("forwarderError");
+        expect(i18n.t).toHaveBeenCalledWith("forwarderError", "mock", "expected message");
+      },
+    );
+
+    it.each([[429], [500], [500]])(
+      "throws an unknown forwarder error when HTTP status is %i and the payload could contain an html tag",
+      async (status) => {
+        const client = new RestClient(api, i18n);
+        const request: IntegrationRequest = { website: null };
+        const response = mock<Response>({
+          status,
+          statusText: null,
+          text: () => Promise.resolve("<head>"),
+        });
+        api.nativeFetch.mockResolvedValue(response);
+
+        const result = client.fetchJson(rpc, request);
+
+        await expect(result).rejects.toEqual("forwarderUnknownError");
+        expect(i18n.t).toHaveBeenCalledWith("forwarderUnknownError", "mock", undefined);
+      },
+    );
+
+    it.each([[429], [500], [500]])(
+      "throws a unknown forwarder error when HTTP status is %i and the payload is malformed",
+      async (status) => {
+        const client = new RestClient(api, i18n);
+        const request: IntegrationRequest = { website: null };
+        const response = mock<Response>({
+          status,
+          text: () => Promise.resolve(`{ foo: "not json" }`),
+        });
+        api.nativeFetch.mockResolvedValue(response);
+
+        const result = client.fetchJson(rpc, request);
+
+        await expect(result).rejects.toEqual("forwarderUnknownError");
+        expect(i18n.t).toHaveBeenCalledWith("forwarderUnknownError", "mock", undefined);
+      },
+    );
+
     it("outputs an error if there's no json payload", async () => {
       const client = new RestClient(api, i18n);
       rpc.hasJsonPayload.mockReturnValue(false);

libs/common/src/tools/integration/rpc/rest-client.ts

@@ -10,59 +10,96 @@ export class RestClient {
     private api: ApiService,
     private i18n: I18nService,
   ) {}
+
   /** uses the fetch API to request a JSON payload. */
-  async fetchJson<Parameters extends IntegrationRequest, Response>(
-    rpc: JsonRpc<Parameters, Response>,
+  // FIXME: once legacy password generator is removed, replace forwarder-specific error
+  //   messages with RPC-generalized ones.
+  async fetchJson<Parameters extends IntegrationRequest, Result>(
+    rpc: JsonRpc<Parameters, Result>,
     params: Parameters,
-  ): Promise<Response> {
+  ): Promise<Result> {
+    // run the request
     const request = rpc.toRequest(params);
     const response = await this.api.nativeFetch(request);
 
-    // FIXME: once legacy password generator is removed, replace forwarder-specific error
-    //   messages with RPC-generalized ones.
-    let error: string = undefined;
-    let cause: string = undefined;
+    let result: Result = undefined;
+    let errorKey: string = undefined;
+    let errorMessage: string = undefined;
 
+    const commonError = await this.detectCommonErrors(response);
+    if (commonError) {
+      [errorKey, errorMessage] = commonError;
+    } else if (rpc.hasJsonPayload(response)) {
+      [result, errorMessage] = rpc.processJson(await response.json());
+    }
+
+    if (result) {
+      return result;
+    }
+
+    // handle failures
+    errorKey ??= errorMessage ? "forwarderError" : "forwarderUnknownError";
+    const error = this.i18n.t(errorKey, rpc.requestor.name, errorMessage);
+    throw error;
+  }
+
+  private async detectCommonErrors(response: Response): Promise<[string, string] | undefined> {
     if (response.status === 401 || response.status === 403) {
-      cause = await this.tryGetErrorMessage(response);
-      error = cause ? "forwarderInvalidTokenWithMessage" : "forwarderInvalidToken";
-    } else if (response.status >= 500) {
-      cause = await this.tryGetErrorMessage(response);
-      cause = cause ?? response.statusText;
-      error = "forwarderError";
+      const message = await this.tryGetErrorMessage(response);
+      const key = message ? "forwaderInvalidTokenWithMessage" : "forwaderInvalidToken";
+      return [key, message];
+    } else if (response.status === 429 || response.status >= 500) {
+      const message = await this.tryGetErrorMessage(response);
+      const key = message ? "forwarderError" : "forwarderUnknownError";
+      return [key, message];
     }
-
-    let ok: Response = undefined;
-    if (!error && rpc.hasJsonPayload(response)) {
-      [ok, cause] = rpc.processJson(await response.json());
-    }
-
-    // success
-    if (ok) {
-      return ok;
-    }
-
-    // failure
-    if (!error) {
-      error = cause ? "forwarderError" : "forwarderUnknownError";
-    }
-    throw this.i18n.t(error, rpc.requestor.name, cause);
   }
 
   private async tryGetErrorMessage(response: Response) {
     const body = (await response.text()) ?? "";
 
-    if (!body.startsWith("{")) {
+    // nullish continues processing; false returns undefined
+    const error =
+      this.tryFindErrorAsJson(body) ?? this.tryFindErrorAsText(body) ?? response.statusText;
+
+    return error || undefined;
+  }
+
+  private tryFindErrorAsJson(body: string) {
+    // tryParse JSON object or string
+    const parsable = body.startsWith("{") || body.startsWith(`'`) || body.startsWith(`"`);
+    if (!parsable) {
+      // fail-and-continue because it's not JSON
+      return undefined;
+    }
+    let parsed = undefined;
+    try {
+      parsed = JSON.parse(body);
+    } catch {
+      // fail-and-exit in case `body` is malformed JSON
+      return false;
+    }
+
+    // could be a string
+    if (parsed && typeof parsed === "string") {
+      return parsed;
+    }
+
+    // could be { error?: T, message?: U }
+    const error = parsed.error?.toString() ?? null;
+    const message = parsed.message?.toString() ?? null;
+
+    // `false` signals no message found
+    const result = error && message ? `${error}: ${message}` : (error ?? message ?? false);
+
+    return result;
+  }
+
+  private tryFindErrorAsText(body: string) {
+    if (!body.length || body.includes("<")) {
       return undefined;
     }
 
-    const json = JSON.parse(body);
-    if ("error" in json) {
-      return json.error;
-    } else if ("message" in json) {
-      return json.message;
-    }
-
-    return undefined;
+    return body;
   }
 }