Skip to content

Commit 687c878

Browse files
streamichstreamich
committed
Drilldown allow list (#85779)
* feat: 🎸 add ROW_CLICK_TRIGGER * feat: 🎸 wire row click event to UI Actions trigger in Lens * feat: 🎸 add row click trigger to url drilldown * feat: 🎸 add datatable to row click context * feat: 🎸 pass in row index in row click trigger context * feat: 🎸 add columns to row click trigger context * feat: 🎸 fill values and keys event scope array * feat: 🎸 generate correct row scope variables * fix: 🐛 report triggers from lens embeddable * feat: 🎸 add sample preview for row click trigger * feat: 🎸 remove url drilldown preview box * chore: 🤖 remove mock variable generation functions * feat: 🎸 generate context and global variable lists * feat: 🎸 preview event variable list * feat: 🎸 show empty url error on blur * feat: 🎸 add ability to always show popup for executed actions * refactor: 💡 rename multiple action execution method * fix: 🐛 don't add separator befor group on no main items * feat: 🎸 wire in uiActions service into datatable renderer * feat: 🎸 check each row if it has compatible row click actions * feat: 🎸 allow passing data to expression renderer * feat: 🎸 add isEmbeddable helper * feat: 🎸 pass embeddable to lens table renderer * feat: 🎸 hide lens table row actions which are empty * feat: 🎸 re-render lens embeddable when dynamic actions chagne * feat: 🎸 hide actions column if there are no row actions * feat: 🎸 re-render lens embeddable on view mode chagne * fix: 🐛 fix TypeScript errors * chore: 🤖 fix TypeScript errors * docs: ✏️ update auto-generated docs * feat: 🎸 add hasCompatibleActions to expression layer * feat: 🎸 remove "data" from expression renderer handlers * fix: 🐛 fix TypeScript errors * test: 💍 fix Jest tests * docs: ✏️ update autogenerated docs * fix: 🐛 wrap event payload into data * test: 💍 add "alwaysShowPopup" test * chore: 🤖 add comment requested in review #83167 (comment) * test: 💍 add hasCompatibleActions test * test: 💍 add datatable renderer test * test: 💍 add Lens embeddable input change tests * test: 💍 add embeddable row click test * fix: 🐛 add url validation * test: 💍 add url drilldown tests * docs: ✏️ remove url drilldown preview from docs * docs: ✏️ remove preview from url templating * docs: ✏️ add row click description * chore: 🤖 move 36.5 KB bundle balance to url_drilldown * test: 💍 simplify test case * feat: 🎸 check if external URL is valid before redirecting user * test: 💍 check for external URL validity * feat: 🎸 check if external URL is allowed in exec and getHref * test: 💍 fix test import
1 parent d0c88d5 commit 687c878

3 files changed

Lines changed: 106 additions & 8 deletions

File tree

‎x-pack/plugins/drilldowns/url_drilldown/public/lib/url_drilldown.test.ts‎

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@
44
* you may not use this file except in compliance with the Elastic License.
55
*/
66

7+
import { IExternalUrl } from 'src/core/public';
78
import { UrlDrilldown, ActionContext, Config } from './url_drilldown';
89
import { IEmbeddable } from '../../../../../../src/plugins/embeddable/public/lib/embeddables';
910
import { DatatableColumnType } from '../../../../../../src/plugins/expressions/common';
11+
import { of } from '../../../../../../src/plugins/kibana_utils';
1012
import { createPoint, rowClickData, TestEmbeddable } from './test/data';
1113
import {
1214
VALUE_CLICK_TRIGGER,
@@ -59,13 +61,27 @@ const mockEmbeddable = ({
5961

6062
const mockNavigateToUrl = jest.fn(() => Promise.resolve());
6163

62-
describe('UrlDrilldown', () => {
63-
const urlDrilldown = new UrlDrilldown({
64+
class TextExternalUrl implements IExternalUrl {
65+
constructor(private readonly isCorrect: boolean = true) {}
66+
67+
public validateUrl(url: string): URL | null {
68+
return this.isCorrect ? new URL(url) : null;
69+
}
70+
}
71+
72+
const createDrilldown = (isExternalUrlValid: boolean = true) => {
73+
const drilldown = new UrlDrilldown({
74+
externalUrl: new TextExternalUrl(isExternalUrlValid),
6475
getGlobalScope: () => ({ kibanaUrl: 'http://localhost:5601/' }),
6576
getSyntaxHelpDocsLink: () => 'http://localhost:5601/docs',
6677
getVariablesHelpDocsLink: () => 'http://localhost:5601/docs',
6778
navigateToUrl: mockNavigateToUrl,
6879
});
80+
return drilldown;
81+
};
82+
83+
describe('UrlDrilldown', () => {
84+
const urlDrilldown = createDrilldown();
6985

7086
test('license', () => {
7187
expect(urlDrilldown.minimalLicense).toBe('gold');
@@ -125,6 +141,30 @@ describe('UrlDrilldown', () => {
125141

126142
await expect(urlDrilldown.isCompatible(config, context)).resolves.toBe(false);
127143
});
144+
145+
test('not compatible if external URL is denied', async () => {
146+
const drilldown1 = createDrilldown(true);
147+
const drilldown2 = createDrilldown(false);
148+
const config: Config = {
149+
url: {
150+
template: `https://elasti.co/?{{event.value}}&{{rison context.panel.query}}`,
151+
},
152+
openInNewTab: false,
153+
};
154+
155+
const context: ActionContext = {
156+
data: {
157+
data: mockDataPoints,
158+
},
159+
embeddable: mockEmbeddable,
160+
};
161+
162+
const result1 = await drilldown1.isCompatible(config, context);
163+
const result2 = await drilldown2.isCompatible(config, context);
164+
165+
expect(result1).toBe(true);
166+
expect(result2).toBe(false);
167+
});
128168
});
129169

130170
describe('getHref & execute', () => {
@@ -173,6 +213,42 @@ describe('UrlDrilldown', () => {
173213
await expect(urlDrilldown.execute(config, context)).rejects.toThrowError();
174214
expect(mockNavigateToUrl).not.toBeCalled();
175215
});
216+
217+
test('should throw on denied external URL', async () => {
218+
const drilldown1 = createDrilldown(true);
219+
const drilldown2 = createDrilldown(false);
220+
const config: Config = {
221+
url: {
222+
template: `https://elasti.co/?{{event.value}}&{{rison context.panel.query}}`,
223+
},
224+
openInNewTab: false,
225+
};
226+
227+
const context: ActionContext = {
228+
data: {
229+
data: mockDataPoints,
230+
},
231+
embeddable: mockEmbeddable,
232+
};
233+
234+
const url = await drilldown1.getHref(config, context);
235+
await drilldown1.execute(config, context);
236+
237+
expect(url).toMatchInlineSnapshot(`"https://elasti.co/?test&(language:kuery,query:test)"`);
238+
expect(mockNavigateToUrl).toBeCalledWith(url);
239+
240+
const [, error1] = await of(drilldown2.getHref(config, context));
241+
const [, error2] = await of(drilldown2.execute(config, context));
242+
243+
expect(error1).toBeInstanceOf(Error);
244+
expect(error1.message).toMatchInlineSnapshot(
245+
`"External URL [https://elasti.co/?test&(language:kuery,query:test)] was denied by ExternalUrl service. You can configure external URL policies using \\"externalUrl.policy\\" setting in kibana.yml."`
246+
);
247+
expect(error2).toBeInstanceOf(Error);
248+
expect(error2.message).toMatchInlineSnapshot(
249+
`"External URL [https://elasti.co/?test&(language:kuery,query:test)] was denied by ExternalUrl service. You can configure external URL policies using \\"externalUrl.policy\\" setting in kibana.yml."`
250+
);
251+
});
176252
});
177253

178254
describe('variables', () => {

‎x-pack/plugins/drilldowns/url_drilldown/public/lib/url_drilldown.tsx‎

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import React from 'react';
88
import { getFlattenedObject } from '@kbn/std';
9+
import { IExternalUrl } from 'src/core/public';
910
import { reactToUiComponent } from '../../../../../../src/plugins/kibana_react/public';
1011
import {
1112
ChartActionContext,
@@ -31,6 +32,7 @@ import { getPanelVariables, getEventScope, getEventVariableList } from './url_dr
3132
import { txtUrlDrilldownDisplayName } from './i18n';
3233

3334
interface UrlDrilldownDeps {
35+
externalUrl: IExternalUrl;
3436
getGlobalScope: () => UrlDrilldownGlobalScope;
3537
navigateToUrl: (url: string) => Promise<void>;
3638
getSyntaxHelpDocsLink: () => string;
@@ -55,7 +57,7 @@ const URL_DRILLDOWN = 'URL_DRILLDOWN';
5557
export class UrlDrilldown implements Drilldown<Config, UrlTrigger, ActionFactoryContext> {
5658
public readonly id = URL_DRILLDOWN;
5759

58-
constructor(private deps: UrlDrilldownDeps) {}
60+
constructor(private readonly deps: UrlDrilldownDeps) {}
5961

6062
public readonly order = 8;
6163

@@ -109,18 +111,37 @@ export class UrlDrilldown implements Drilldown<Config, UrlTrigger, ActionFactory
109111
console.warn(
110112
`UrlDrilldown [${config.url.template}] is not valid. Error [${error}]. Skipping execution.`
111113
);
114+
return false;
112115
}
113116

114-
return Promise.resolve(isValid);
117+
const url = this.buildUrl(config, context);
118+
const validUrl = this.deps.externalUrl.validateUrl(url);
119+
if (!validUrl) {
120+
return false;
121+
}
122+
123+
return true;
115124
};
116125

117-
public readonly getHref = async (config: Config, context: ActionContext) => {
118-
const scope = this.getRuntimeVariables(context);
119-
return urlDrilldownCompileUrl(config.url.template, scope);
126+
private buildUrl(config: Config, context: ActionContext): string {
127+
const url = urlDrilldownCompileUrl(config.url.template, this.getRuntimeVariables(context));
128+
return url;
129+
}
130+
131+
public readonly getHref = async (config: Config, context: ActionContext): Promise<string> => {
132+
const url = this.buildUrl(config, context);
133+
const validUrl = this.deps.externalUrl.validateUrl(url);
134+
if (!validUrl) {
135+
throw new Error(
136+
`External URL [${url}] was denied by ExternalUrl service. ` +
137+
`You can configure external URL policies using "externalUrl.policy" setting in kibana.yml.`
138+
);
139+
}
140+
return url;
120141
};
121142

122143
public readonly execute = async (config: Config, context: ActionContext) => {
123-
const url = urlDrilldownCompileUrl(config.url.template, this.getRuntimeVariables(context));
144+
const url = await this.getHref(config, context);
124145
if (config.openInNewTab) {
125146
window.open(url, '_blank', 'noopener');
126147
} else {

‎x-pack/plugins/drilldowns/url_drilldown/public/plugin.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ export class UrlDrilldownPlugin
3838
const startServices = createStartServicesGetter(core.getStartServices);
3939
plugins.uiActionsEnhanced.registerDrilldown(
4040
new UrlDrilldown({
41+
externalUrl: core.http.externalUrl,
4142
getGlobalScope: urlDrilldownGlobalScopeProvider({ core }),
4243
navigateToUrl: (url: string) =>
4344
core.getStartServices().then(([{ application }]) => application.navigateToUrl(url)),

0 commit comments

Comments
 (0)