Omit adding the osd-version header when the Fetch request is to an …

…external origin (#3643) * Making `fetch` requests using core/public/http/fetch, an `osd-version` header is forcefully added, even to external requests. This change examines the destination and only adds the header to relative URLs and those that are to OSD itself. * This change also adds `osd-xsrf` to calls that use `osd-version` incorrectly to satisfy XSRF protection Fixes #3277 Signed-off-by: Miki <[email protected]>
opensearch-project · Apr 17, 2023 · 0762566 · 0762566
1 parent e4fccfc
commit 0762566
Show file tree

Hide file tree

Showing 8 changed files with 38 additions and 7 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -71,6 +71,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - [Vis Builder] Add metric to metric, bucket to bucket aggregation persistence ([#3495](https:/opensearch-project/OpenSearch-Dashboards/pull/3495))
 - Use mirrors to download Node.js binaries to escape sporadic 404 errors ([#3619](https:/opensearch-project/OpenSearch-Dashboards/pull/3619))
 - [Multiple DataSource] Refactor dev tool console to use opensearch-js client to send requests ([#3544](https:/opensearch-project/OpenSearch-Dashboards/pull/3544))
+- Add `osd-xsrf` header to all requests that incorrectly used `node-version` to satisfy XSRF protection ([#3643](https:/opensearch-project/OpenSearch-Dashboards/pull/3643))
 - [Data] Add geo shape filter field ([#3605](https:/opensearch-project/OpenSearch-Dashboards/pull/3605))
 - [VisBuilder] Add UI actions handler ([#3732](https:/opensearch-project/OpenSearch-Dashboards/pull/3732))
 - [Dashboard] Indicate that IE is no longer supported ([#3641](https:/opensearch-project/OpenSearch-Dashboards/pull/3641))
@@ -117,6 +118,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - [Region Maps] Add ui setting to configure custom vector map's size parameter([#3399](https:/opensearch-project/OpenSearch-Dashboards/pull/3399))
 - [Search Telemetry] Fixes search telemetry's observable object that won't be GC-ed([#3390](https:/opensearch-project/OpenSearch-Dashboards/pull/3390))
 - Clean up and rebuild `@osd/pm` ([#3570](https:/opensearch-project/OpenSearch-Dashboards/pull/3570))
+- Omit adding the `osd-version` header when the Fetch request is to an external origin ([#3643](https:/opensearch-project/OpenSearch-Dashboards/pull/3643))
 - [Vega] Add Filter custom label for opensearchDashboardsAddFilter ([#3640](https:/opensearch-project/OpenSearch-Dashboards/pull/3640))
 - [Timeline] Fix y-axis label color in dark mode ([#3698](https:/opensearch-project/OpenSearch-Dashboards/pull/3698))
 - [VisBuilder] Fix multiple warnings thrown on page load ([#3732](https:/opensearch-project/OpenSearch-Dashboards/pull/3732))

diff --git a/src/core/public/http/fetch.ts b/src/core/public/http/fetch.ts
@@ -31,6 +31,7 @@
 import { omitBy } from 'lodash';
 import { format } from 'url';
 import { BehaviorSubject } from 'rxjs';
+import { isRelativeUrl } from '@osd/std';
 
 import {
  IBasePath,
@@ -144,7 +145,6 @@ export class Fetch {
  headers: removedUndefined({
  'Content-Type': 'application/json',
  ...options.headers,
- 'osd-version': this.params.opensearchDashboardsVersion,
  }),
  };
 
@@ -158,6 +158,18 @@ export class Fetch {
  fetchOptions.headers['osd-system-request'] = 'true';
  }
 
+ /* `osd-version` is used on the server-side to make sure that an incoming request originated from a front-end
+ * of the same version; see core/server/http/lifecycle_handlers.ts
+ * `osd-xsrf` is to satisfy XSRF protection but is only meaningful to OpenSearch Dashboards.
+ *
+ * If the `url` equals `basePath`, starts with `basePath` + '/', or is relative, add `osd-version` and `osd-xsrf` headers.
+ */
+ const basePath = this.params.basePath.get();
+ if (isRelativeUrl(url) || url === basePath || url.startsWith(`${basePath}/`)) {
+ fetchOptions.headers['osd-version'] = this.params.opensearchDashboardsVersion;
+ fetchOptions.headers['osd-xsrf'] = 'osd-fetch';
+ }
+
  return new Request(url, fetchOptions as RequestInit);
  }
 

diff --git a/src/core/public/ui_settings/__snapshots__/ui_settings_api.test.ts.snap b/src/core/public/ui_settings/__snapshots__/ui_settings_api.test.ts.snap
diff --git a/src/core/server/http/integration_tests/lifecycle_handlers.test.ts b/src/core/server/http/integration_tests/lifecycle_handlers.test.ts
@@ -231,20 +231,23 @@ describe('core lifecycle handlers', () => {
  .expect(200, 'ok');
  });
 
+ // ToDo: Remove next; `osd-version` incorrectly used for satisfying XSRF protection
  it('accepts requests with the version header', async () => {
  await getSupertest(method.toLowerCase(), testPath)
  .set(versionHeader, actualVersion)
  .expect(200, 'ok');
  });
 
+ // ToDo: Rename next; `osd-version` incorrectly used for satisfying XSRF protection
  it('rejects requests without either an xsrf or version header', async () => {
  await getSupertest(method.toLowerCase(), testPath).expect(400, {
  statusCode: 400,
  error: 'Bad Request',
- message: 'Request must contain a osd-xsrf header.',
+ message: 'Request must contain the osd-xsrf header.',
  });
  });
 
+ // ToDo: Rename next; `osd-version` incorrectly used for satisfying XSRF protection
  it('accepts whitelisted requests without either an xsrf or version header', async () => {
  await getSupertest(method.toLowerCase(), whitelistedTestPath).expect(200, 'ok');
  });

diff --git a/src/core/server/http/lifecycle_handlers.test.ts b/src/core/server/http/lifecycle_handlers.test.ts
@@ -102,6 +102,7 @@ describe('xsrf post-auth handler', () => {
  expect(result).toEqual('next');
  });
 
+ // ToDo: Remove; `osd-version` incorrectly used for satisfying XSRF protection
  it('accepts requests with version header', () => {
  const config = createConfig({ xsrf: { whitelist: [], disableProtection: false } });
  const handler = createXsrfPostAuthHandler(config);
@@ -129,7 +130,7 @@ describe('xsrf post-auth handler', () => {
  expect(responseFactory.badRequest).toHaveBeenCalledTimes(1);
  expect(responseFactory.badRequest.mock.calls[0][0]).toMatchInlineSnapshot(`
  Object {
- "body": "Request must contain a osd-xsrf header.",
+ "body": "Request must contain the osd-xsrf header.",
  }
  `);
  expect(result).toEqual('badRequest');
@@ -199,7 +200,7 @@ describe('versionCheck post-auth handler', () => {
  responseFactory = httpServerMock.createLifecycleResponseFactory();
  });
 
- it('forward the request to the next interceptor if header matches', () => {
+ it('forward the request to the next interceptor if osd-version header matches the actual version', () => {
  const handler = createVersionCheckPostAuthHandler('actual-version');
  const request = forgeRequest({ headers: { 'osd-version': 'actual-version' } });
 
@@ -212,7 +213,7 @@ describe('versionCheck post-auth handler', () => {
  expect(result).toBe('next');
  });
 
- it('returns a badRequest error if header does not match', () => {
+ it('returns a badRequest error if osd-version header exists but does not match the actual version', () => {
  const handler = createVersionCheckPostAuthHandler('actual-version');
  const request = forgeRequest({ headers: { 'osd-version': 'another-version' } });
 
@@ -236,7 +237,7 @@ describe('versionCheck post-auth handler', () => {
  expect(result).toBe('badRequest');
  });
 
- it('forward the request to the next interceptor if header is not present', () => {
+ it('forward the request to the next interceptor if osd-version header is not present', () => {
  const handler = createVersionCheckPostAuthHandler('actual-version');
  const request = forgeRequest({ headers: {} });
 

diff --git a/src/core/server/http/lifecycle_handlers.ts b/src/core/server/http/lifecycle_handlers.ts
@@ -54,8 +54,9 @@ export const createXsrfPostAuthHandler = (config: HttpConfig): OnPostAuthHandler
  const hasVersionHeader = VERSION_HEADER in request.headers;
  const hasXsrfHeader = XSRF_HEADER in request.headers;
 
+ // ToDo: Remove !hasVersionHeader; `osd-version` incorrectly used for satisfying XSRF protection
  if (!isSafeMethod(request.route.method) && !hasVersionHeader && !hasXsrfHeader) {
- return response.badRequest({ body: `Request must contain a ${XSRF_HEADER} header.` });
+ return response.badRequest({ body: `Request must contain the ${XSRF_HEADER} header.` });
  }
 
  return toolkit.next();

diff --git a/src/plugins/bfetch/public/plugin.ts b/src/plugins/bfetch/public/plugin.ts
@@ -95,6 +95,7 @@ export class BfetchPublicPlugin
  url: `${basePath}/${removeLeadingSlash(params.url)}`,
  headers: {
  'Content-Type': 'application/json',
+ 'osd-xsrf': 'osd-bfetch',
  'osd-version': version,
  ...(params.headers || {}),
  },

diff --git a/src/plugins/opensearch_dashboards_legacy/public/angular/angular_config.tsx b/src/plugins/opensearch_dashboards_legacy/public/angular/angular_config.tsx
@@ -159,6 +159,8 @@ export const $setupXsrfRequestInterceptor = (version: string) => {
  // Configure jQuery prefilter
  $.ajaxPrefilter(({ osdXsrfToken = true }: any, originalOptions, jqXHR) => {
  if (osdXsrfToken) {
+ jqXHR.setRequestHeader('osd-xsrf', 'osd-legacy');
+ // ToDo: Remove next; `osd-version` incorrectly used for satisfying XSRF protection
  jqXHR.setRequestHeader('osd-version', version);
  }
  });
@@ -170,6 +172,8 @@ export const $setupXsrfRequestInterceptor = (version: string) => {
  request(opts) {
  const { osdXsrfToken = true } = opts as any;
  if (osdXsrfToken) {
+ set(opts, ['headers', 'osd-xsrf'], 'osd-legacy');
+ // ToDo: Remove next; `osd-version` incorrectly used for satisfying XSRF protection
  set(opts, ['headers', 'osd-version'], version);
  }
  return opts;