Always allow internal urls in Vega (#124705)

This commit is contained in:
Tobias Stadler 2022-02-08 15:04:25 +01:00 committed by GitHub
parent 3f702c1fd4
commit ca9c004f1a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 83 additions and 6 deletions

View file

@ -0,0 +1,24 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->
[Home](./index.md) &gt; [kibana-plugin-core-public](./kibana-plugin-core-public.md) &gt; [IExternalUrl](./kibana-plugin-core-public.iexternalurl.md) &gt; [isInternalUrl](./kibana-plugin-core-public.iexternalurl.isinternalurl.md)
## IExternalUrl.isInternalUrl() method
Determines if the provided URL is an internal url.
<b>Signature:</b>
```typescript
isInternalUrl(relativeOrAbsoluteUrl: string): boolean;
```
## Parameters
| Parameter | Type | Description |
| --- | --- | --- |
| relativeOrAbsoluteUrl | string | |
<b>Returns:</b>
boolean

View file

@ -16,5 +16,6 @@ export interface IExternalUrl
| Method | Description |
| --- | --- |
| [isInternalUrl(relativeOrAbsoluteUrl)](./kibana-plugin-core-public.iexternalurl.isinternalurl.md) | Determines if the provided URL is an internal url. |
| [validateUrl(relativeOrAbsoluteUrl)](./kibana-plugin-core-public.iexternalurl.validateurl.md) | Determines if the provided URL is a valid location to send users. Validation is based on the configured allow list in kibana.yml.<!-- -->If the URL is valid, then a URL will be returned. Otherwise, this will return null. |

View file

@ -73,6 +73,23 @@ const internalRequestScenarios = [
];
describe('External Url Service', () => {
describe('#isInternalUrl', () => {
const { setup } = setupService({
location: new URL('https://example.com/app/management?q=1&bar=false#some-hash'),
serverBasePath: '',
policy: [],
});
it('internal request', () => {
expect(setup.isInternalUrl('/')).toBeTruthy();
expect(setup.isInternalUrl('https://example.com/')).toBeTruthy();
});
it('external request', () => {
expect(setup.isInternalUrl('https://elastic.co/')).toBeFalsy();
});
});
describe('#validateUrl', () => {
describe('internal requests with a server base path', () => {
const serverBasePath = '/base-path';

View file

@ -50,20 +50,33 @@ function normalizeProtocol(protocol: string) {
return protocol.endsWith(':') ? protocol.slice(0, -1).toLowerCase() : protocol.toLowerCase();
}
const createIsInternalUrlValidation = (
location: Pick<Location, 'href'>,
serverBasePath: string
) => {
return function isInternallUrl(next: string) {
const base = new URL(location.href);
const url = new URL(next, base);
return (
url.origin === base.origin &&
(!serverBasePath || url.pathname.startsWith(`${serverBasePath}/`))
);
};
};
const createExternalUrlValidation = (
rules: IExternalUrlPolicy[],
location: Pick<Location, 'href'>,
serverBasePath: string
) => {
const isInternalUrl = createIsInternalUrlValidation(location, serverBasePath);
return function validateExternalUrl(next: string) {
const base = new URL(location.href);
const url = new URL(next, base);
const isInternalURL =
url.origin === base.origin &&
(!serverBasePath || url.pathname.startsWith(`${serverBasePath}/`));
if (isInternalURL) {
if (isInternalUrl(next)) {
return url;
}
@ -90,6 +103,7 @@ export class ExternalUrlService implements CoreService<IExternalUrl> {
const { policy } = injectedMetadata.getExternalUrlConfig();
return {
isInternalUrl: createIsInternalUrlValidation(location, serverBasePath),
validateUrl: createExternalUrlValidation(policy, location, serverBasePath),
};
}

View file

@ -36,6 +36,7 @@ const createServiceMock = ({
isAnonymous: jest.fn(),
},
externalUrl: {
isInternalUrl: jest.fn(),
validateUrl: jest.fn(),
},
addLoadingCountSource: jest.fn(),

View file

@ -110,6 +110,13 @@ export interface IBasePath {
* @public
*/
export interface IExternalUrl {
/**
* Determines if the provided URL is an internal url.
*
* @param relativeOrAbsoluteUrl
*/
isInternalUrl(relativeOrAbsoluteUrl: string): boolean;
/**
* Determines if the provided URL is a valid location to send users.
* Validation is based on the configured allow list in kibana.yml.

View file

@ -686,6 +686,7 @@ export interface IBasePath {
// @public
export interface IExternalUrl {
isInternalUrl(relativeOrAbsoluteUrl: string): boolean;
validateUrl(relativeOrAbsoluteUrl: string): URL | null;
}

View file

@ -19,6 +19,7 @@ exports[`DashboardEmptyScreen renders correctly with edit mode 1`] = `
},
"delete": [MockFunction],
"externalUrl": Object {
"isInternalUrl": [MockFunction],
"validateUrl": [MockFunction],
},
"fetch": [MockFunction],
@ -343,6 +344,7 @@ exports[`DashboardEmptyScreen renders correctly with readonly mode 1`] = `
},
"delete": [MockFunction],
"externalUrl": Object {
"isInternalUrl": [MockFunction],
"validateUrl": [MockFunction],
},
"fetch": [MockFunction],
@ -706,6 +708,7 @@ exports[`DashboardEmptyScreen renders correctly with view mode 1`] = `
},
"delete": [MockFunction],
"externalUrl": Object {
"isInternalUrl": [MockFunction],
"validateUrl": [MockFunction],
},
"fetch": [MockFunction],

View file

@ -396,6 +396,7 @@ exports[`home isNewKibanaInstance should set isNewKibanaInstance to true when th
},
"delete": [MockFunction],
"externalUrl": Object {
"isInternalUrl": [MockFunction],
"validateUrl": [MockFunction],
},
"fetch": [MockFunction],
@ -464,6 +465,7 @@ exports[`home isNewKibanaInstance should set isNewKibanaInstance to true when th
},
"delete": [MockFunction],
"externalUrl": Object {
"isInternalUrl": [MockFunction],
"validateUrl": [MockFunction],
},
"fetch": [MockFunction],
@ -533,6 +535,7 @@ exports[`home isNewKibanaInstance should set isNewKibanaInstance to true when th
},
"delete": [MockFunction],
"externalUrl": Object {
"isInternalUrl": [MockFunction],
"validateUrl": [MockFunction],
},
"fetch": [MockFunction],

View file

@ -182,6 +182,7 @@ exports[`Flyout conflicts should allow conflict resolution 2`] = `
},
"delete": [MockFunction],
"externalUrl": Object {
"isInternalUrl": [MockFunction],
"validateUrl": [MockFunction],
},
"fetch": [MockFunction],

View file

@ -280,6 +280,7 @@ exports[`TelemetryManagementSectionComponent renders null because allowChangingO
},
"delete": [MockFunction],
"externalUrl": Object {
"isInternalUrl": [MockFunction],
"validateUrl": [MockFunction],
},
"fetch": [MockFunction],

View file

@ -207,7 +207,7 @@ export class VegaBaseView {
const vegaLoader = loader();
const originalSanitize = vegaLoader.sanitize.bind(vegaLoader);
vegaLoader.sanitize = async (uri, options) => {
if (uri.bypassToken === bypassToken) {
if (uri.bypassToken === bypassToken || this._externalUrl.isInternalUrl(uri)) {
// If uri has a bypass token, the uri was encoded by bypassExternalUrlCheck() above.
// because user can only supply pure JSON data structure.
uri = uri.url;

View file

@ -68,6 +68,10 @@ const mockNavigateToUrl = jest.fn(() => Promise.resolve());
class TextExternalUrl implements IExternalUrl {
constructor(private readonly isCorrect: boolean = true) {}
public isInternalUrl(url: string): boolean {
return false;
}
public validateUrl(url: string): URL | null {
return this.isCorrect ? new URL(url) : null;
}