mirror of
https://github.com/elastic/kibana.git
synced 2025-06-27 10:40:07 -04:00
# Backport This will backport the following commits from `main` to `7.17`: - [[SharedUX/Locators] Add limits around legacy locator (#222273)](https://github.com/elastic/kibana/pull/222273) <!--- Backport version: 10.0.0 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Tim Sullivan","email":"tsullivan@users.noreply.github.com"},"sourceCommit":{"committedDate":"2025-06-04T19:55:47Z","message":"[SharedUX/Locators] Add limits around legacy locator (#222273)\n\n## Summary\n\nWe've decided that we want to enforce sanity checks around the \"legacy\"\nlocator and phase out its usage. This PR ensures the legacy locator can\nnot be used from the Reporting plugin.\n\n### Checklist\n\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n\n---------\n\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"b96e7c37b83dc3f3ec08860d10413127baf13196","branchLabelMapping":{"^v9.1.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport:version","v9.1.0","v8.19.0","v7.17.29","v9.0.3","v8.18.3","v8.17.8"],"title":"[SharedUX/Locators] Add limits around legacy locator","number":222273,"url":"https://github.com/elastic/kibana/pull/222273","mergeCommit":{"message":"[SharedUX/Locators] Add limits around legacy locator (#222273)\n\n## Summary\n\nWe've decided that we want to enforce sanity checks around the \"legacy\"\nlocator and phase out its usage. This PR ensures the legacy locator can\nnot be used from the Reporting plugin.\n\n### Checklist\n\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n\n---------\n\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"b96e7c37b83dc3f3ec08860d10413127baf13196"}},"sourceBranch":"main","suggestedTargetBranches":["8.19","7.17","9.0","8.18","8.17"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/222273","number":222273,"mergeCommit":{"message":"[SharedUX/Locators] Add limits around legacy locator (#222273)\n\n## Summary\n\nWe've decided that we want to enforce sanity checks around the \"legacy\"\nlocator and phase out its usage. This PR ensures the legacy locator can\nnot be used from the Reporting plugin.\n\n### Checklist\n\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios\n\n---------\n\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"b96e7c37b83dc3f3ec08860d10413127baf13196"}},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"7.17","label":"v7.17.29","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"9.0","label":"v9.0.3","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.18","label":"v8.18.3","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.17","label":"v8.17.8","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> --------- Co-authored-by: “jeramysoucy” <jeramy.soucy@elastic.co>
This commit is contained in:
Tim Sullivan
GitHub
parent
cf7ad65fd4
commit
664f3dc887
23 changed files with 458 additions and 12 deletions
|
|
@ -0,0 +1,24 @@
|
|||
<!-- Do not edit this file. It is automatically generated by API Documenter. -->
|
||||
|
||||
[Home](./index.md) > [kibana-plugin-core-public](./kibana-plugin-core-public.md) > [IExternalUrl](./kibana-plugin-core-public.iexternalurl.md) > [isInternalUrl](./kibana-plugin-core-public.iexternalurl.isinternalurl.md)
|
||||
|
||||
## IExternalUrl.isInternalUrl() method
|
||||
|
||||
Determines if the provided URL is an internal url.
|
||||
|
||||
**Signature:**
|
||||
|
||||
```typescript
|
||||
isInternalUrl(relativeOrAbsoluteUrl: string): boolean;
|
||||
```
|
||||
|
||||
## Parameters
|
||||
|
||||
| Parameter | Type | Description |
|
||||
| --- | --- | --- |
|
||||
| relativeOrAbsoluteUrl | string | |
|
||||
|
||||
**Returns:**
|
||||
|
||||
boolean
|
||||
|
||||
|
|
@ -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) | <p>Determines if the provided URL is a valid location to send users. Validation is based on the configured allow list in kibana.yml.</p><p>If the URL is valid, then a URL will be returned. Otherwise, this will return null.</p> |
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,28 @@
|
|||
<!-- Do not edit this file. It is automatically generated by API Documenter. -->
|
||||
|
||||
[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [KibanaRequest](./kibana-plugin-core-server.kibanarequest.md) > [\[inspect.custom\]](./kibana-plugin-core-server.kibanarequest._inspect.custom_.md)
|
||||
|
||||
## KibanaRequest.\[inspect.custom\]() method
|
||||
|
||||
**Signature:**
|
||||
|
||||
```typescript
|
||||
[inspect.custom](): {
|
||||
id: string;
|
||||
uuid: string;
|
||||
url: string;
|
||||
isSystemRequest: boolean;
|
||||
auth: {
|
||||
isAuthenticated: boolean;
|
||||
};
|
||||
route: Readonly<{
|
||||
path: string;
|
||||
method: RecursiveReadonly<Method>;
|
||||
options: RecursiveReadonly<KibanaRequestRouteOptions<Method>>;
|
||||
}>;
|
||||
};
|
||||
```
|
||||
**Returns:**
|
||||
|
||||
{ id: string; uuid: string; url: string; isSystemRequest: boolean; auth: { isAuthenticated: boolean; }; route: Readonly<{ path: string; method: RecursiveReadonly<Method>; options: RecursiveReadonly<KibanaRequestRouteOptions<Method>>; }>; }
|
||||
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
<!-- Do not edit this file. It is automatically generated by API Documenter. -->
|
||||
|
||||
[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [KibanaRequest](./kibana-plugin-core-server.kibanarequest.md) > [toJSON](./kibana-plugin-core-server.kibanarequest.tojson.md)
|
||||
|
||||
## KibanaRequest.toJSON() method
|
||||
|
||||
**Signature:**
|
||||
|
||||
```typescript
|
||||
toJSON(): {
|
||||
id: string;
|
||||
uuid: string;
|
||||
url: string;
|
||||
isSystemRequest: boolean;
|
||||
auth: {
|
||||
isAuthenticated: boolean;
|
||||
};
|
||||
route: Readonly<{
|
||||
path: string;
|
||||
method: RecursiveReadonly<Method>;
|
||||
options: RecursiveReadonly<KibanaRequestRouteOptions<Method>>;
|
||||
}>;
|
||||
};
|
||||
```
|
||||
**Returns:**
|
||||
|
||||
{ id: string; uuid: string; url: string; isSystemRequest: boolean; auth: { isAuthenticated: boolean; }; route: Readonly<{ path: string; method: RecursiveReadonly<Method>; options: RecursiveReadonly<KibanaRequestRouteOptions<Method>>; }>; }
|
||||
|
||||
|
|
@ -0,0 +1,15 @@
|
|||
<!-- Do not edit this file. It is automatically generated by API Documenter. -->
|
||||
|
||||
[Home](./index.md) > [kibana-plugin-core-server](./kibana-plugin-core-server.md) > [KibanaRequest](./kibana-plugin-core-server.kibanarequest.md) > [toString](./kibana-plugin-core-server.kibanarequest.tostring.md)
|
||||
|
||||
## KibanaRequest.toString() method
|
||||
|
||||
**Signature:**
|
||||
|
||||
```typescript
|
||||
toString(): string;
|
||||
```
|
||||
**Returns:**
|
||||
|
||||
string
|
||||
|
||||
|
|
@ -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';
|
||||
|
|
|
|||
|
|
@ -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),
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -37,6 +37,7 @@ const createServiceMock = ({
|
|||
},
|
||||
externalUrl: {
|
||||
validateUrl: jest.fn(),
|
||||
isInternalUrl: jest.fn(),
|
||||
},
|
||||
addLoadingCountSource: jest.fn(),
|
||||
getLoadingCount$: jest.fn().mockReturnValue(new BehaviorSubject(0)),
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -950,6 +950,7 @@ export interface IBasePath {
|
|||
|
||||
// @public
|
||||
export interface IExternalUrl {
|
||||
isInternalUrl(relativeOrAbsoluteUrl: string): boolean;
|
||||
validateUrl(relativeOrAbsoluteUrl: string): URL | null;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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],
|
||||
|
|
|
|||
|
|
@ -396,6 +396,7 @@ exports[`home isNewKibanaInstance should set isNewKibanaInstance to true when th
|
|||
},
|
||||
"delete": [MockFunction],
|
||||
"externalUrl": Object {
|
||||
"isInternalUrl": [MockFunction],
|
||||
"validateUrl": [MockFunction],
|
||||
},
|
||||
"fetch": [MockFunction],
|
||||
|
|
@ -463,6 +464,7 @@ exports[`home isNewKibanaInstance should set isNewKibanaInstance to true when th
|
|||
},
|
||||
"delete": [MockFunction],
|
||||
"externalUrl": Object {
|
||||
"isInternalUrl": [MockFunction],
|
||||
"validateUrl": [MockFunction],
|
||||
},
|
||||
"fetch": [MockFunction],
|
||||
|
|
@ -530,6 +532,7 @@ exports[`home isNewKibanaInstance should set isNewKibanaInstance to true when th
|
|||
},
|
||||
"delete": [MockFunction],
|
||||
"externalUrl": Object {
|
||||
"isInternalUrl": [MockFunction],
|
||||
"validateUrl": [MockFunction],
|
||||
},
|
||||
"fetch": [MockFunction],
|
||||
|
|
|
|||
|
|
@ -182,6 +182,7 @@ exports[`Flyout conflicts should allow conflict resolution 2`] = `
|
|||
},
|
||||
"delete": [MockFunction],
|
||||
"externalUrl": Object {
|
||||
"isInternalUrl": [MockFunction],
|
||||
"validateUrl": [MockFunction],
|
||||
},
|
||||
"fetch": [MockFunction],
|
||||
|
|
|
|||
13
src/plugins/share/jest.integration.config.js
Normal file
13
src/plugins/share/jest.integration.config.js
Normal file
|
|
@ -0,0 +1,13 @@
|
|||
/*
|
||||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
|
||||
* or more contributor license agreements. Licensed under the Elastic License
|
||||
* 2.0 and the Server Side Public License, v 1; you may not use this file except
|
||||
* in compliance with, at your election, the Elastic License 2.0 or the Server
|
||||
* Side Public License, v 1.
|
||||
*/
|
||||
|
||||
module.exports = {
|
||||
preset: '@kbn/test/jest_integration',
|
||||
rootDir: '../../..',
|
||||
roots: ['<rootDir>/src/plugins/share'],
|
||||
};
|
||||
|
|
@ -45,6 +45,9 @@ export const createShortUrlRedirectApp = (
|
|||
}
|
||||
|
||||
const url = core.http.basePath.prepend(redirectUrl);
|
||||
if (!core.http.externalUrl.isInternalUrl(url)) {
|
||||
throw new Error(`Can not redirect to external URL: ${url}`);
|
||||
}
|
||||
|
||||
location.href = url;
|
||||
|
||||
|
|
|
|||
|
|
@ -20,7 +20,9 @@ export const registerUrlServiceRoutes = (
|
|||
router: IRouter,
|
||||
url: ServerUrlService
|
||||
) => {
|
||||
registerCreateRoute(router, url);
|
||||
const { http } = core;
|
||||
|
||||
registerCreateRoute(router, url, http);
|
||||
registerGetRoute(router, url);
|
||||
registerDeleteRoute(router, url);
|
||||
registerResolveRoute(router, url);
|
||||
|
|
|
|||
|
|
@ -0,0 +1,106 @@
|
|||
/*
|
||||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
|
||||
* or more contributor license agreements. Licensed under the Elastic License
|
||||
* 2.0 and the Server Side Public License, v 1; you may not use this file except
|
||||
* in compliance with, at your election, the Elastic License 2.0 or the Server
|
||||
* Side Public License, v 1.
|
||||
*/
|
||||
|
||||
import supertest from 'supertest';
|
||||
import { UnwrapPromise } from '@kbn/utility-types';
|
||||
import { setupServer } from 'src/core/server/test_utils';
|
||||
import { httpServiceMock } from 'src/core/server/mocks';
|
||||
import { MockUrlService } from '../../../../../common/mocks';
|
||||
import { registerCreateRoute } from '../register_create_route';
|
||||
|
||||
const url = new MockUrlService();
|
||||
const http = httpServiceMock.createSetupContract();
|
||||
|
||||
type SetupServerReturn = UnwrapPromise<ReturnType<typeof setupServer>>;
|
||||
|
||||
describe('POST /api/short_url', () => {
|
||||
let server: SetupServerReturn['server'];
|
||||
let httpSetup: SetupServerReturn['httpSetup'];
|
||||
|
||||
beforeAll(async () => {
|
||||
const setup = await setupServer();
|
||||
server = setup.server;
|
||||
httpSetup = setup.httpSetup;
|
||||
|
||||
url.locators.get = jest.fn().mockImplementation((locatorId) => {
|
||||
if (locatorId === 'LEGACY_SHORT_URL_LOCATOR') {
|
||||
return { id: locatorId };
|
||||
}
|
||||
return undefined;
|
||||
});
|
||||
|
||||
http.basePath.get = jest.fn().mockReturnValue('');
|
||||
registerCreateRoute(httpSetup.createRouter(''), url, http);
|
||||
|
||||
await server.start();
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await server.stop();
|
||||
});
|
||||
|
||||
it('returns 200 and the short URL data for a valid internal URL', async () => {
|
||||
url.shortUrls.get = jest.fn().mockReturnValue({
|
||||
create: jest.fn().mockResolvedValue({
|
||||
data: { id: 'abc123', url: '/s/abc123' },
|
||||
}),
|
||||
});
|
||||
|
||||
const payload = {
|
||||
locatorId: 'LEGACY_SHORT_URL_LOCATOR',
|
||||
params: {
|
||||
url: '/internal/app/path',
|
||||
},
|
||||
};
|
||||
|
||||
await supertest(httpSetup.server.listener)
|
||||
.post('/api/short_url')
|
||||
.send(payload)
|
||||
.expect(200)
|
||||
.expect((res) => {
|
||||
expect(res.body).toEqual({ id: 'abc123', url: '/s/abc123' });
|
||||
});
|
||||
});
|
||||
|
||||
it('returns 400 if creating a short URL for an external URL', async () => {
|
||||
const payload = {
|
||||
locatorId: 'LEGACY_SHORT_URL_LOCATOR',
|
||||
params: {
|
||||
url: 'http://example.com/app/management/data/short_urls',
|
||||
},
|
||||
};
|
||||
|
||||
await supertest(httpSetup.server.listener)
|
||||
.post('/api/short_url')
|
||||
.send(payload)
|
||||
.expect(400)
|
||||
.expect((res) => {
|
||||
expect(res.text).toContain('Can not create a short URL for an external URL.');
|
||||
});
|
||||
});
|
||||
|
||||
it('returns 409 if the locator is not found', async () => {
|
||||
const payload = {
|
||||
locatorId: 'NON_EXISTENT_LOCATOR',
|
||||
params: {
|
||||
url: '/internal/path',
|
||||
},
|
||||
};
|
||||
|
||||
// Override the locator mock for this test
|
||||
(url.locators.get as jest.Mock).mockReturnValueOnce(undefined);
|
||||
|
||||
await supertest(httpSetup.server.listener)
|
||||
.post('/api/short_url')
|
||||
.send(payload)
|
||||
.expect(409)
|
||||
.expect((res) => {
|
||||
expect(res.text).toContain('Locator not found.');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -7,10 +7,50 @@
|
|||
*/
|
||||
|
||||
import { schema } from '@kbn/config-schema';
|
||||
import { IRouter } from 'kibana/server';
|
||||
import { HttpServiceSetup, IRouter } from 'kibana/server';
|
||||
import { ServerUrlService } from '../../types';
|
||||
|
||||
export const registerCreateRoute = (router: IRouter, url: ServerUrlService) => {
|
||||
/**
|
||||
* Determine if url is outside of this Kibana install.
|
||||
* Copied from x-pack/plugins/security/common/is_internal_url.ts
|
||||
*/
|
||||
function isInternalURL(url: string, basePath = '') {
|
||||
// We use the WHATWG parser TWICE with completely different dummy base URLs to ensure that the parsed URL always
|
||||
// inherits the origin of the base URL. This means that the specified URL isn't an absolute URL, or a scheme-relative
|
||||
// URL (//), or a scheme-relative URL with an empty host (///). Browsers may process such URLs unexpectedly due to
|
||||
// backward compatibility reasons (e.g., a browser may treat `///abc.com` as just `abc.com`). For more details, refer
|
||||
// to https://url.spec.whatwg.org/#concept-basic-url-parser and https://url.spec.whatwg.org/#url-representation.
|
||||
let normalizedURL: URL;
|
||||
try {
|
||||
for (const baseURL of ['http://example.org:5601', 'https://example.com']) {
|
||||
normalizedURL = new URL(url, baseURL);
|
||||
if (normalizedURL.origin !== baseURL) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Now we need to normalize URL to make sure any relative path segments (`..`) cannot escape expected base path.
|
||||
if (basePath) {
|
||||
return (
|
||||
// Normalized pathname can add a leading slash, but we should also make sure it's included in
|
||||
// the original URL too. We can safely use non-null assertion operator here since we know `normalizedURL` is
|
||||
// always defined, otherwise we would have returned `false` already.
|
||||
url.startsWith('/') &&
|
||||
(normalizedURL!.pathname === basePath || normalizedURL!.pathname.startsWith(`${basePath}/`))
|
||||
);
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
export const registerCreateRoute = (
|
||||
router: IRouter,
|
||||
url: ServerUrlService,
|
||||
http: HttpServiceSetup
|
||||
) => {
|
||||
router.post(
|
||||
{
|
||||
path: '/api/short_url',
|
||||
|
|
@ -42,8 +82,7 @@ export const registerCreateRoute = (router: IRouter, url: ServerUrlService) => {
|
|||
},
|
||||
},
|
||||
router.handleLegacyErrors(async (ctx, req, res) => {
|
||||
const savedObjects = ctx.core.savedObjects.client;
|
||||
const shortUrls = url.shortUrls.get({ savedObjects });
|
||||
const { core } = ctx;
|
||||
const { locatorId, params, slug } = req.body;
|
||||
const locator = url.locators.get(locatorId);
|
||||
|
||||
|
|
@ -57,6 +96,16 @@ export const registerCreateRoute = (router: IRouter, url: ServerUrlService) => {
|
|||
});
|
||||
}
|
||||
|
||||
const urlFromParams = (params as { url: string | undefined }).url;
|
||||
if (urlFromParams && !isInternalURL(urlFromParams, http.basePath.get(req))) {
|
||||
return res.customError({
|
||||
statusCode: 400,
|
||||
body: 'Can not create a short URL for an external URL.',
|
||||
});
|
||||
}
|
||||
const savedObjects = core.savedObjects.client;
|
||||
const shortUrls = url.shortUrls.get({ savedObjects });
|
||||
|
||||
const shortUrl = await shortUrls.create({
|
||||
locator,
|
||||
params,
|
||||
|
|
|
|||
|
|
@ -279,6 +279,7 @@ exports[`TelemetryManagementSectionComponent renders null because allowChangingO
|
|||
},
|
||||
"delete": [MockFunction],
|
||||
"externalUrl": Object {
|
||||
"isInternalUrl": [MockFunction],
|
||||
"validateUrl": [MockFunction],
|
||||
},
|
||||
"fetch": [MockFunction],
|
||||
|
|
|
|||
|
|
@ -66,6 +66,10 @@ class TextExternalUrl implements IExternalUrl {
|
|||
public validateUrl(url: string): URL | null {
|
||||
return this.isCorrect ? new URL(url) : null;
|
||||
}
|
||||
|
||||
public isInternalUrl(_url: string): boolean {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
const createDrilldown = (isExternalUrlValid: boolean = true) => {
|
||||
|
|
|
|||
|
|
@ -115,6 +115,13 @@ export const ILM_POLICY_NAME = 'kibana-reporting';
|
|||
// Management UI route
|
||||
export const REPORTING_MANAGEMENT_HOME = '/app/management/insightsAndAlerting/reporting';
|
||||
|
||||
// Allowed locator types for reporting: the "reportable" analytical apps we expect to redirect to during screenshotting
|
||||
export const REPORTING_REDIRECT_ALLOWED_LOCATOR_TYPES = [
|
||||
'DASHBOARD_APP_LOCATOR',
|
||||
'LENS_APP_LOCATOR',
|
||||
'VISUALIZE_APP_LOCATOR',
|
||||
];
|
||||
|
||||
export const REPORTING_REDIRECT_LOCATOR_STORE_KEY = '__REPORTING_REDIRECT_LOCATOR_STORE_KEY__';
|
||||
|
||||
/**
|
||||
|
|
|
|||
108
x-pack/plugins/reporting/public/redirect/redirect_app.test.tsx
Normal file
108
x-pack/plugins/reporting/public/redirect/redirect_app.test.tsx
Normal file
|
|
@ -0,0 +1,108 @@
|
|||
/*
|
||||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
|
||||
* or more contributor license agreements. Licensed under the Elastic License
|
||||
* 2.0; you may not use this file except in compliance with the Elastic License
|
||||
* 2.0.
|
||||
*/
|
||||
|
||||
import React from 'react';
|
||||
import { render, waitFor, screen } from '@testing-library/react';
|
||||
import { sharePluginMock } from 'src/plugins/share/public/mocks';
|
||||
import { scopedHistoryMock } from 'src/core/public/mocks';
|
||||
import { RedirectApp } from './redirect_app';
|
||||
import { REPORTING_REDIRECT_LOCATOR_STORE_KEY } from '../../common/constants';
|
||||
|
||||
const mockApiClient = {
|
||||
getInfo: jest.fn(),
|
||||
};
|
||||
const mockShare = sharePluginMock.createSetupContract();
|
||||
const historyMock = scopedHistoryMock.create();
|
||||
function setLocationSearch(search: string) {
|
||||
window.history.pushState({}, '', search);
|
||||
}
|
||||
|
||||
describe('RedirectApp', () => {
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
// Clear the window property before each test
|
||||
delete (window as any)[REPORTING_REDIRECT_LOCATOR_STORE_KEY];
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
// Reset the URL to the root after each test
|
||||
window.history.pushState({}, '', '/');
|
||||
// Clean up window property
|
||||
delete (window as any)[REPORTING_REDIRECT_LOCATOR_STORE_KEY];
|
||||
});
|
||||
|
||||
it('navigates using share.navigate when locator params are present in window object', async () => {
|
||||
setLocationSearch('?jobId=happy');
|
||||
const locatorParams = { id: 'LENS_APP_LOCATOR', params: { foo: 'bar' } };
|
||||
// Set the locator params in window object
|
||||
(window as any)[REPORTING_REDIRECT_LOCATOR_STORE_KEY] = locatorParams;
|
||||
|
||||
render(
|
||||
<RedirectApp apiClient={mockApiClient as any} share={mockShare} history={historyMock} />
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockShare.navigate).toHaveBeenCalledWith(locatorParams);
|
||||
});
|
||||
});
|
||||
|
||||
it('displays error when locator params are not available', async () => {
|
||||
setLocationSearch('?jobId=fail');
|
||||
// Do not set the locator params
|
||||
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
|
||||
|
||||
render(
|
||||
<RedirectApp apiClient={mockApiClient as any} share={mockShare} history={historyMock} />
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText('Redirect error')).toBeInTheDocument();
|
||||
expect(screen.getByText('Could not find locator params for report')).toBeInTheDocument();
|
||||
expect(consoleErrorSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Redirect page error:'),
|
||||
'Could not find locator params for report'
|
||||
);
|
||||
});
|
||||
|
||||
consoleErrorSpy.mockRestore();
|
||||
});
|
||||
|
||||
describe('non-app locator', () => {
|
||||
it('throws error when using non-allowed locator type', async () => {
|
||||
setLocationSearch('');
|
||||
// Set a non-allowed locator type
|
||||
(window as any)[REPORTING_REDIRECT_LOCATOR_STORE_KEY] = {
|
||||
id: 'LEGACY_SHORT_URL_LOCATOR',
|
||||
params: {},
|
||||
};
|
||||
|
||||
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
|
||||
|
||||
render(
|
||||
<RedirectApp apiClient={mockApiClient as any} share={mockShare} history={historyMock} />
|
||||
);
|
||||
|
||||
await waitFor(() =>
|
||||
expect(
|
||||
screen.getByText(
|
||||
'Report job execution can only redirect using a locator for an expected analytical app'
|
||||
)
|
||||
).toBeInTheDocument()
|
||||
);
|
||||
|
||||
expect(mockShare.navigate).not.toHaveBeenCalled();
|
||||
expect(consoleErrorSpy).toHaveBeenCalledWith(
|
||||
`Report job execution cannot redirect using LEGACY_SHORT_URL_LOCATOR`
|
||||
);
|
||||
expect(consoleErrorSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Redirect page error:'),
|
||||
'Report job execution can only redirect using a locator for an expected analytical app'
|
||||
);
|
||||
consoleErrorSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -12,7 +12,10 @@ import { EuiTitle, EuiCallOut, EuiCodeBlock } from '@elastic/eui';
|
|||
|
||||
import type { ScopedHistory } from 'src/core/public';
|
||||
|
||||
import { REPORTING_REDIRECT_LOCATOR_STORE_KEY } from '../../common/constants';
|
||||
import {
|
||||
REPORTING_REDIRECT_LOCATOR_STORE_KEY,
|
||||
REPORTING_REDIRECT_ALLOWED_LOCATOR_TYPES,
|
||||
} from '../../common/constants';
|
||||
import { LocatorParams } from '../../common/types';
|
||||
|
||||
import { ReportingAPIClient } from '../lib/reporting_api_client';
|
||||
|
|
@ -52,12 +55,19 @@ export const RedirectApp: FunctionComponent<Props> = ({ share }) => {
|
|||
throw new Error('Could not find locator params for report');
|
||||
}
|
||||
|
||||
if (!REPORTING_REDIRECT_ALLOWED_LOCATOR_TYPES.includes(locatorParams.id)) {
|
||||
// eslint-disable-next-line no-console
|
||||
console.error(`Report job execution cannot redirect using ${locatorParams.id}`);
|
||||
throw new Error(
|
||||
'Report job execution can only redirect using a locator for an expected analytical app'
|
||||
);
|
||||
}
|
||||
|
||||
share.navigate(locatorParams);
|
||||
} catch (e) {
|
||||
setError(e);
|
||||
// eslint-disable-next-line no-console
|
||||
console.error(i18nTexts.consoleMessagePrefix, e.message);
|
||||
throw e;
|
||||
}
|
||||
}, [share]);
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue