Skip to content

Commit

Permalink
[Backport 2.x] Omit adding the osd-version header when the Fetch re…
Browse files Browse the repository at this point in the history
…quest 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 <miki@amazon.com>
(cherry picked from commit 0762566)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md

* add changelog

Signed-off-by: Josh Romero <rmerqg@amazon.com>

---------

Signed-off-by: Josh Romero <rmerqg@amazon.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Josh Romero <rmerqg@amazon.com>
  • Loading branch information
3 people authored Apr 18, 2023
1 parent 6d70817 commit 7ab8dcc
Show file tree
Hide file tree
Showing 8 changed files with 40 additions and 7 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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))
Expand All @@ -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))
Expand Down
14 changes: 13 additions & 1 deletion src/core/public/http/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import { omitBy } from 'lodash';
import { format } from 'url';
import { BehaviorSubject } from 'rxjs';
import { isRelativeUrl } from '@osd/std';

import {
IBasePath,
Expand Down Expand Up @@ -144,7 +145,6 @@ export class Fetch {
headers: removedUndefined({
'Content-Type': 'application/json',
...options.headers,
'osd-version': this.params.opensearchDashboardsVersion,
}),
};

Expand All @@ -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);
}

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
Expand Down
9 changes: 5 additions & 4 deletions src/core/server/http/lifecycle_handlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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' } });

Expand All @@ -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' } });

Expand All @@ -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: {} });

Expand Down
3 changes: 2 additions & 1 deletion src/core/server/http/lifecycle_handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions src/plugins/bfetch/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 || {}),
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
});
Expand All @@ -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;
Expand Down

0 comments on commit 7ab8dcc

Please sign in to comment.