fix: add validation on data.method when using transport.request (#801)

* fix: add validation on data.method when using tranport.request Signed-off-by: SuZhoue-Joe <[email protected]> * feat: add validation on endpoint Signed-off-by: SuZhoue-Joe <[email protected]> * feat: add unit test Signed-off-by: SuZhoue-Joe <[email protected]> * feat: add more protect Signed-off-by: SuZhoue-Joe <[email protected]> --------- Signed-off-by: SuZhoue-Joe <[email protected]> (cherry picked from commit 13fb97f)
opensearch-project · Jul 5, 2023 · db05f7d · db05f7d
1 parent 619f859
commit db05f7d
Show file tree

Hide file tree

Showing 2 changed files with 130 additions and 1 deletion.
diff --git a/server/services/CommonService.test.ts b/server/services/CommonService.test.ts
@@ -0,0 +1,97 @@
+import {
+  ILegacyCustomClusterClient,
+  OpenSearchDashboardsRequest,
+  OpenSearchDashboardsResponseFactory,
+  RequestHandlerContext,
+} from "opensearch-dashboards/server";
+import CommonService from "./CommonService";
+
+const contextMock = {
+  core: {},
+} as RequestHandlerContext;
+const responseMock = ({
+  custom: jest.fn((args) => args),
+} as unknown) as OpenSearchDashboardsResponseFactory;
+
+const mockedClient = {
+  callAsCurrentUser: jest.fn(),
+  callAsInternalUser: jest.fn(),
+  close: jest.fn(),
+  asScoped: jest.fn(() => ({
+    callAsCurrentUser: jest.fn((...args) => args),
+    callAsInternalUser: jest.fn(),
+  })),
+} as any;
+
+describe("CommonService spec", () => {
+  it("http method should valid when calling transport.request", async () => {
+    const commonService = new CommonService(mockedClient);
+    const result = await commonService.apiCaller(
+      contextMock,
+      {
+        body: {
+          endpoint: "transport.request",
+          data: {
+            method: "invalid method",
+          },
+        },
+      } as OpenSearchDashboardsRequest,
+      responseMock
+    );
+    expect(result).toEqual({
+      statusCode: 200,
+      body: {
+        ok: false,
+        error: `Method must be one of, case insensitive ['HEAD', 'GET', 'POST', 'PUT', 'DELETE']. Received 'invalid method'.`,
+      },
+    });
+  });
+
+  it("should return error when no endpoint is provided", async () => {
+    const commonService = new CommonService(mockedClient);
+    const result = await commonService.apiCaller(
+      contextMock,
+      {
+        body: {
+          endpoint: "",
+        },
+      } as OpenSearchDashboardsRequest,
+      responseMock
+    );
+    expect(result).toEqual({
+      statusCode: 200,
+      body: {
+        ok: false,
+        error: `Expected non-empty string on endpoint`,
+      },
+    });
+  });
+
+  it("should patch path when data.path does not start with /", async () => {
+    const commonService = new CommonService(mockedClient);
+    const result = await commonService.apiCaller(
+      contextMock,
+      {
+        body: {
+          endpoint: "transport.request",
+          data: {
+            path: "",
+          },
+        },
+      } as OpenSearchDashboardsRequest,
+      responseMock
+    );
+    expect(result).toEqual({
+      statusCode: 200,
+      body: {
+        ok: true,
+        response: [
+          "transport.request",
+          {
+            path: "/",
+          },
+        ],
+      },
+    });
+  });
+});
diff --git a/server/services/CommonService.ts b/server/services/CommonService.ts
@@ -14,11 +14,13 @@ import {
 } from "../../../../src/core/server";
 import { IAPICaller } from "../../models/interfaces";
 
+const VALID_METHODS = ["HEAD", "GET", "POST", "PUT", "DELETE"];
+
 export interface ICommonCaller {
   <T>(arg: any): T;
 }
 
-export default class IndexService {
+export default class CommonService {
   osDriver: ILegacyCustomClusterClient;
 
   constructor(osDriver: ILegacyCustomClusterClient) {
@@ -36,12 +38,42 @@ export default class IndexService {
     try {
       const { callAsCurrentUser: callWithRequest } = this.osDriver.asScoped(request);
       const finalData = data;
+
+      /**
+       * The endpoint must not be an empty string, reference from proxy caller
+       */
+      if (!endpoint) {
+        return response.custom({
+          statusCode: 200,
+          body: {
+            ok: false,
+            error: `Expected non-empty string on endpoint`,
+          },
+        });
+      }
+
       /**
        * Update path parameter to follow RFC/generic HTTP convention
        */
       if (endpoint === "transport.request" && typeof finalData?.path === "string" && !/^\//.test(finalData?.path || "")) {
         finalData.path = `/${finalData.path || ""}`;
       }
+
+      /**
+       * Check valid method here
+       */
+      if (endpoint === "transport.request" && data?.method) {
+        if (VALID_METHODS.indexOf(data.method.toUpperCase?.()) === -1) {
+          return response.custom({
+            statusCode: 200,
+            body: {
+              ok: false,
+              error: `Method must be one of, case insensitive ['HEAD', 'GET', 'POST', 'PUT', 'DELETE']. Received '${data.method}'.`,
+            },
+          });
+        }
+      }
+
       const payload = useQuery ? JSON.parse(finalData || "{}") : finalData;
       const commonCallerResponse = await callWithRequest(endpoint, payload || {});
       return response.custom({