Skip to content

Commit

Permalink
Omit adding the osd-version header when the Fetch request was expli…
Browse files Browse the repository at this point in the history
…citly asked to not prepend the `basePath`

Fixes #3277

Signed-off-by: Miki <[email protected]>
  • Loading branch information
AMoo-Miki committed Mar 22, 2023
1 parent de06344 commit c55ee28
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 5 deletions.
12 changes: 11 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,16 @@ 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
*
* If url equals `basePath, starts with `basePath` + '/', or is relative, add `osd-version` header.
*/
const basePath = this.params.basePath.get();
if (isRelativeUrl(url) || url === basePath || url.startsWith(`${basePath}/`)) {
fetchOptions.headers['osd-version'] = this.params.opensearchDashboardsVersion;
}

return new Request(url, fetchOptions as RequestInit);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,12 +231,14 @@ 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,
Expand All @@ -245,6 +247,7 @@ describe('core lifecycle handlers', () => {
});
});

// 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
7 changes: 4 additions & 3 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 @@ -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 c55ee28

Please sign in to comment.