From 7ab8dcc008e668698e8ae6ba35e7da0460e1551a Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Mon, 17 Apr 2023 18:02:48 -0700 Subject: [PATCH] [Backport 2.x] Omit adding the `osd-version` header when the Fetch request is to an external origin (#3867) * 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 (cherry picked from commit 0762566ec143fe9a8c1d2b61d4d4c33d64f825d3) Signed-off-by: github-actions[bot] # Conflicts: # CHANGELOG.md * add changelog Signed-off-by: Josh Romero --------- Signed-off-by: Josh Romero Co-authored-by: github-actions[bot] Co-authored-by: Josh Romero --- CHANGELOG.md | 4 ++++ src/core/public/http/fetch.ts | 14 +++++++++++++- .../__snapshots__/ui_settings_api.test.ts.snap | 7 +++++++ .../integration_tests/lifecycle_handlers.test.ts | 5 ++++- src/core/server/http/lifecycle_handlers.test.ts | 9 +++++---- src/core/server/http/lifecycle_handlers.ts | 3 ++- src/plugins/bfetch/public/plugin.ts | 1 + .../public/angular/angular_config.tsx | 4 ++++ 8 files changed, 40 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 64d54a7e4cf0..e12cf79a7399 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### 📈 Features/Enhancements - Add satisfaction survey link to help menu ([#3676](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3676)) +- Add `osd-xsrf` header to all requests that incorrectly used `node-version` to satisfy XSRF protection ([#3643](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3643)) - [Monaco editor] Add json worker support ([#3424](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3424)) - [Dashboard] Indicate that IE is no longer supported ([#3641](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3641)) - [Data] Add geo shape filter field ([#3605](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3605)) @@ -29,6 +30,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### 🐛 Bug Fixes - Clean up and rebuild `@osd/pm` ([#3570](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3570)) +- Omit adding the `osd-version` header when the Fetch request is to an external origin ([#3643](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3643)) - [VisBuilder] Fix multiple warnings thrown on page load ([#3732](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3732)) - [VisBuilder] Fix Firefox legend selection issue ([#3732](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3732)) - [VisBuilder] Fix type errors ([#3732](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3732)) @@ -50,6 +52,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Timeline] Update default expressions from `.es(*)` to `.opensearch(*)`. ([2720](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2720)) ### 🪛 Refactoring + +- Remove automatic addition of `osd-version` header to requests outside of OpenSearch Dashboards - [Multiple DataSource] Refactor dev tool console to use opensearch-js client to send requests ([#3544](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3544)) - [Tech Debt] Clean up docs_link_service organization so that strings are in the right categories. ([#3685](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3685)) - [Console] Replace jQuery usage in console plugin with native methods ([#3733](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3733)) diff --git a/src/core/public/http/fetch.ts b/src/core/public/http/fetch.ts index cefaa353f7fa..694372c46d99 100644 --- 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 index f80d88dd91ff..52d04f52ff8b 100644 --- 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 @@ -9,6 +9,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "osd-version": "opensearchDashboardsVersion", + "osd-xsrf": "osd-fetch", }, "method": "POST", }, @@ -20,6 +21,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "osd-version": "opensearchDashboardsVersion", + "osd-xsrf": "osd-fetch", }, "method": "POST", }, @@ -36,6 +38,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "osd-version": "opensearchDashboardsVersion", + "osd-xsrf": "osd-fetch", }, "method": "POST", }, @@ -47,6 +50,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "osd-version": "opensearchDashboardsVersion", + "osd-xsrf": "osd-fetch", }, "method": "POST", }, @@ -63,6 +67,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "osd-version": "opensearchDashboardsVersion", + "osd-xsrf": "osd-fetch", }, "method": "POST", }, @@ -74,6 +79,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "osd-version": "opensearchDashboardsVersion", + "osd-xsrf": "osd-fetch", }, "method": "POST", }, @@ -113,6 +119,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "osd-version": "opensearchDashboardsVersion", + "osd-xsrf": "osd-fetch", }, "method": "POST", }, diff --git a/src/core/server/http/integration_tests/lifecycle_handlers.test.ts b/src/core/server/http/integration_tests/lifecycle_handlers.test.ts index fc13c1ae3fbb..0742e8c08d6d 100644 --- 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 index 6a49bbfa14fa..cd96f0ea7760 100644 --- 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 index 636bb8af4522..f17b07942e6a 100644 --- 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 index 1a3a0bffc821..e115b35a44e1 100644 --- 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 index ef01d29e4b14..fbe36a289d70 100644 --- 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;