From 884fba2944c81d9fe51803fef100dcc7a4fb9b49 Mon Sep 17 00:00:00 2001 From: Josh Dover Date: Mon, 30 Dec 2019 14:49:18 -0600 Subject: [PATCH 1/9] Do not remount applications between page navigations (#53851) --- .../integration_tests/router.test.tsx | 84 +++++++++++++++---- .../application/integration_tests/utils.tsx | 56 +++++++------ src/core/public/application/test_types.ts | 14 +++- .../public/application/ui/app_container.tsx | 2 +- src/core/server/rendering/views/styles.tsx | 2 +- 5 files changed, 113 insertions(+), 45 deletions(-) diff --git a/src/core/public/application/integration_tests/router.test.tsx b/src/core/public/application/integration_tests/router.test.tsx index ffc10820a9c37..10544c348afb0 100644 --- a/src/core/public/application/integration_tests/router.test.tsx +++ b/src/core/public/application/integration_tests/router.test.tsx @@ -18,7 +18,7 @@ */ import React from 'react'; -import { createMemoryHistory, History } from 'history'; +import { createMemoryHistory, History, createHashHistory } from 'history'; import { AppRouter, AppNotFound } from '../ui'; import { EitherApp, MockedMounterMap, MockedMounterTuple } from '../test_types'; @@ -27,7 +27,15 @@ import { createRenderer, createAppMounter, createLegacyAppMounter } from './util describe('AppContainer', () => { let mounters: MockedMounterMap; let history: History; - let navigate: ReturnType; + let update: ReturnType; + + const navigate = (path: string) => { + history.push(path); + return update(); + }; + + const mockMountersToMounters = () => + new Map([...mounters].map(([appId, { mounter }]) => [appId, mounter])); beforeEach(() => { mounters = new Map([ @@ -38,14 +46,14 @@ describe('AppContainer', () => { createAppMounter('app3', '
App 3
', '/custom/path'), ] as Array>); history = createMemoryHistory(); - navigate = createRenderer(, history.push); + update = createRenderer(); }); it('calls mount handler and returned unmount function when navigating between apps', async () => { const dom1 = await navigate('/app/app1'); const app1 = mounters.get('app1')!; - expect(app1.mount).toHaveBeenCalled(); + expect(app1.mounter.mount).toHaveBeenCalled(); expect(dom1?.html()).toMatchInlineSnapshot(` "
basename: /app/app1 @@ -53,11 +61,11 @@ describe('AppContainer', () => {
" `); - const app1Unmount = await app1.mount.mock.results[0].value; + const app1Unmount = await app1.mounter.mount.mock.results[0].value; const dom2 = await navigate('/app/app2'); expect(app1Unmount).toHaveBeenCalled(); - expect(mounters.get('app2')!.mount).toHaveBeenCalled(); + expect(mounters.get('app2')!.mounter.mount).toHaveBeenCalled(); expect(dom2?.html()).toMatchInlineSnapshot(` "
basename: /app/app2 @@ -70,29 +78,77 @@ describe('AppContainer', () => { mounters.set(...createAppMounter('spaces', '
Custom Space
', '/spaces/fake-login')); mounters.set(...createAppMounter('login', '
Login Page
', '/fake-login')); history = createMemoryHistory(); - navigate = createRenderer(, history.push); + update = createRenderer(); await navigate('/fake-login'); - expect(mounters.get('spaces')!.mount).not.toHaveBeenCalled(); - expect(mounters.get('login')!.mount).toHaveBeenCalled(); + expect(mounters.get('spaces')!.mounter.mount).not.toHaveBeenCalled(); + expect(mounters.get('login')!.mounter.mount).toHaveBeenCalled(); }); it('should not mount when partial route path has higher specificity', async () => { mounters.set(...createAppMounter('login', '
Login Page
', '/fake-login')); mounters.set(...createAppMounter('spaces', '
Custom Space
', '/spaces/fake-login')); history = createMemoryHistory(); - navigate = createRenderer(, history.push); + update = createRenderer(); await navigate('/spaces/fake-login'); - expect(mounters.get('spaces')!.mount).toHaveBeenCalled(); - expect(mounters.get('login')!.mount).not.toHaveBeenCalled(); + expect(mounters.get('spaces')!.mounter.mount).toHaveBeenCalled(); + expect(mounters.get('login')!.mounter.mount).not.toHaveBeenCalled(); + }); + + it('should not remount when changing pages within app', async () => { + const { mounter, unmount } = mounters.get('app1')!; + await navigate('/app/app1/page1'); + expect(mounter.mount).toHaveBeenCalledTimes(1); + + // Navigating to page within app does not trigger re-render + await navigate('/app/app1/page2'); + expect(mounter.mount).toHaveBeenCalledTimes(1); + expect(unmount).not.toHaveBeenCalled(); + }); + + it('should not remount when going back within app', async () => { + const { mounter, unmount } = mounters.get('app1')!; + await navigate('/app/app1/page1'); + expect(mounter.mount).toHaveBeenCalledTimes(1); + + // Hitting back button within app does not trigger re-render + await navigate('/app/app1/page2'); + history.goBack(); + await update(); + expect(mounter.mount).toHaveBeenCalledTimes(1); + expect(unmount).not.toHaveBeenCalled(); + }); + + it('should not remount when when changing pages within app using hash history', async () => { + history = createHashHistory(); + update = createRenderer(); + + const { mounter, unmount } = mounters.get('app1')!; + await navigate('/app/app1/page1'); + expect(mounter.mount).toHaveBeenCalledTimes(1); + + // Changing hash history does not trigger re-render + await navigate('/app/app1/page2'); + expect(mounter.mount).toHaveBeenCalledTimes(1); + expect(unmount).not.toHaveBeenCalled(); + }); + + it('should unmount when changing between apps', async () => { + const { mounter, unmount } = mounters.get('app1')!; + await navigate('/app/app1/page1'); + expect(mounter.mount).toHaveBeenCalledTimes(1); + + // Navigating to other app triggers unmount + await navigate('/app/app2/page1'); + expect(unmount).toHaveBeenCalledTimes(1); }); it('calls legacy mount handler', async () => { await navigate('/app/legacyApp1'); - expect(mounters.get('legacyApp1')!.mount.mock.calls[0]).toMatchInlineSnapshot(` + expect(mounters.get('legacyApp1')!.mounter.mount.mock.calls[0]).toMatchInlineSnapshot(` Array [ Object { "appBasePath": "/app/legacyApp1", @@ -104,7 +160,7 @@ describe('AppContainer', () => { it('handles legacy apps with subapps', async () => { await navigate('/app/baseApp'); - expect(mounters.get('baseApp:legacyApp2')!.mount.mock.calls[0]).toMatchInlineSnapshot(` + expect(mounters.get('baseApp:legacyApp2')!.mounter.mount.mock.calls[0]).toMatchInlineSnapshot(` Array [ Object { "appBasePath": "/app/baseApp", diff --git a/src/core/public/application/integration_tests/utils.tsx b/src/core/public/application/integration_tests/utils.tsx index b8ade4d1d8787..6367d1fa12697 100644 --- a/src/core/public/application/integration_tests/utils.tsx +++ b/src/core/public/application/integration_tests/utils.tsx @@ -26,19 +26,13 @@ import { App, LegacyApp, AppMountParameters } from '../types'; import { MockedMounter, MockedMounterTuple } from '../test_types'; type Dom = ReturnType | null; -type Renderer = (item: string) => Dom | Promise; +type Renderer = () => Dom | Promise; -export const createRenderer = ( - element: ReactElement | null, - callback?: (item: string) => void | Promise -): Renderer => { +export const createRenderer = (element: ReactElement | null): Renderer => { const dom: Dom = element && mount({element}); - return item => + return () => new Promise(async resolve => { - if (callback) { - await callback(item); - } if (dom) { dom.update(); } @@ -50,19 +44,26 @@ export const createAppMounter = ( appId: string, html: string, appRoute = `/app/${appId}` -): MockedMounterTuple => [ - appId, - { - appRoute, - appBasePath: appRoute, - mount: jest.fn(async ({ appBasePath: basename, element }: AppMountParameters) => { - Object.assign(element, { - innerHTML: `
\nbasename: ${basename}\nhtml: ${html}\n
`, - }); - return jest.fn(() => Object.assign(element, { innerHTML: '' })); - }), - }, -]; +): MockedMounterTuple => { + const unmount = jest.fn(); + return [ + appId, + { + mounter: { + appRoute, + appBasePath: appRoute, + mount: jest.fn(async ({ appBasePath: basename, element }: AppMountParameters) => { + Object.assign(element, { + innerHTML: `
\nbasename: ${basename}\nhtml: ${html}\n
`, + }); + unmount.mockImplementation(() => Object.assign(element, { innerHTML: '' })); + return unmount; + }), + }, + unmount, + }, + ]; +}; export const createLegacyAppMounter = ( appId: string, @@ -70,9 +71,12 @@ export const createLegacyAppMounter = ( ): MockedMounterTuple => [ appId, { - appRoute: `/app/${appId.split(':')[0]}`, - appBasePath: `/app/${appId.split(':')[0]}`, - unmountBeforeMounting: true, - mount: legacyMount, + mounter: { + appRoute: `/app/${appId.split(':')[0]}`, + appBasePath: `/app/${appId.split(':')[0]}`, + unmountBeforeMounting: true, + mount: legacyMount, + }, + unmount: jest.fn(), }, ]; diff --git a/src/core/public/application/test_types.ts b/src/core/public/application/test_types.ts index f5fb639eaa32c..3d992cb950eb4 100644 --- a/src/core/public/application/test_types.ts +++ b/src/core/public/application/test_types.ts @@ -17,7 +17,7 @@ * under the License. */ -import { App, LegacyApp, Mounter } from './types'; +import { App, LegacyApp, Mounter, AppUnmount } from './types'; import { ApplicationService } from './application_service'; /** @internal */ @@ -25,11 +25,19 @@ export type ApplicationServiceContract = PublicMethodsOf; /** @internal */ export type EitherApp = App | LegacyApp; /** @internal */ +export type MockedUnmount = jest.Mocked; +/** @internal */ export type MockedMounter = jest.Mocked>>; /** @internal */ -export type MockedMounterTuple = [string, MockedMounter]; +export type MockedMounterTuple = [ + string, + { mounter: MockedMounter; unmount: MockedUnmount } +]; /** @internal */ -export type MockedMounterMap = Map>; +export type MockedMounterMap = Map< + string, + { mounter: MockedMounter; unmount: MockedUnmount } +>; /** @internal */ export type MockLifecycle< T extends keyof ApplicationService, diff --git a/src/core/public/application/ui/app_container.tsx b/src/core/public/application/ui/app_container.tsx index 96ee91c7c21fb..153582e805fa1 100644 --- a/src/core/public/application/ui/app_container.tsx +++ b/src/core/public/application/ui/app_container.tsx @@ -65,7 +65,7 @@ export const AppContainer: FunctionComponent = ({ mounter, appId }: Props mount(); return unmount; - }); + }, [mounter]); return ( diff --git a/src/core/server/rendering/views/styles.tsx b/src/core/server/rendering/views/styles.tsx index 40261321dcffc..f41627bcfe07f 100644 --- a/src/core/server/rendering/views/styles.tsx +++ b/src/core/server/rendering/views/styles.tsx @@ -78,7 +78,7 @@ export const Styles: FunctionComponent = ({ darkMode }) => { background-repeat: no-repeat; background-size: contain; /* SVG optimized according to http://codepen.io/tigt/post/optimizing-svgs-in-data-uris */ - background-image: url'data:image/svg+xml;base64,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHdpZHRoPSIzMCIgaGVpZ2h0PSIzOSIgdmlld0JveD0iMCAwIDMwIDM5Ij4gIDxnIGZpbGw9Im5vbmUiIGZpbGwtcnVsZT0iZXZlbm9kZCI+ICAgIDxwb2x5Z29uIGZpbGw9IiNGMDRFOTgiIHBvaW50cz0iMCAwIDAgMzQuNTQ3IDI5LjkyMiAuMDIiLz4gICAgPHBhdGggZmlsbD0iIzM0Mzc0MSIgZD0iTTAsMTQuNCBMMCwzNC41NDY4IEwxNC4yODcyLDE4LjA2MTIgQzEwLjA0MTYsMTUuNzM4IDUuMTgwNCwxNC40IDAsMTQuNCIvPiAgICA8cGF0aCBmaWxsPSIjMDBCRkIzIiBkPSJNMTcuMzc0MiwxOS45OTY4IEwyLjcyMSwzNi45MDQ4IEwxLjQzMzQsMzguMzg5MiBMMjkuMjYzOCwzOC4zODkyIEMyNy43NjE0LDMwLjgzODggMjMuNDA0MiwyNC4zMjY0IDE3LjM3NDIsMTkuOTk2OCIvPiAgPC9nPjwvc3ZnPg=='); + background-image: url('data:image/svg+xml;base64,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHdpZHRoPSIzMCIgaGVpZ2h0PSIzOSIgdmlld0JveD0iMCAwIDMwIDM5Ij4gIDxnIGZpbGw9Im5vbmUiIGZpbGwtcnVsZT0iZXZlbm9kZCI+ICAgIDxwb2x5Z29uIGZpbGw9IiNGMDRFOTgiIHBvaW50cz0iMCAwIDAgMzQuNTQ3IDI5LjkyMiAuMDIiLz4gICAgPHBhdGggZmlsbD0iIzM0Mzc0MSIgZD0iTTAsMTQuNCBMMCwzNC41NDY4IEwxNC4yODcyLDE4LjA2MTIgQzEwLjA0MTYsMTUuNzM4IDUuMTgwNCwxNC40IDAsMTQuNCIvPiAgICA8cGF0aCBmaWxsPSIjMDBCRkIzIiBkPSJNMTcuMzc0MiwxOS45OTY4IEwyLjcyMSwzNi45MDQ4IEwxLjQzMzQsMzguMzg5MiBMMjkuMjYzOCwzOC4zODkyIEMyNy43NjE0LDMwLjgzODggMjMuNDA0MiwyNC4zMjY0IDE3LjM3NDIsMTkuOTk2OCIvPiAgPC9nPjwvc3ZnPg=='); } .kibanaWelcomeTitle { From e7ee646124fb1353d1b4ea2ac9a342d3fe5ce4c4 Mon Sep 17 00:00:00 2001 From: Alexey Antonov Date: Tue, 31 Dec 2019 00:48:48 +0300 Subject: [PATCH 2/9] [Vega] Sample [Flights] Airport Connections (Hover Over Airport) visualization not working (#53799) Closes: #53748 Co-authored-by: Elastic Machine --- src/legacy/core_plugins/tile_map/index.ts | 9 ++++++++- src/legacy/core_plugins/vis_type_vega/index.ts | 12 +++++++++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/legacy/core_plugins/tile_map/index.ts b/src/legacy/core_plugins/tile_map/index.ts index 298675e75b0d7..27f019318a82b 100644 --- a/src/legacy/core_plugins/tile_map/index.ts +++ b/src/legacy/core_plugins/tile_map/index.ts @@ -30,7 +30,14 @@ const tileMapPluginInitializer: LegacyPluginInitializer = ({ Plugin }: LegacyPlu uiExports: { styleSheetPaths: resolve(__dirname, 'public/index.scss'), hacks: [resolve(__dirname, 'public/legacy')], - injectDefaultVars: server => ({}), + injectDefaultVars: server => { + const serverConfig = server.config(); + const mapConfig: Record = serverConfig.get('map'); + + return { + emsTileLayerId: mapConfig.emsTileLayerId, + }; + }, }, config(Joi: any) { return Joi.object({ diff --git a/src/legacy/core_plugins/vis_type_vega/index.ts b/src/legacy/core_plugins/vis_type_vega/index.ts index 153cd6afb3ccc..52c253c6ac0b5 100644 --- a/src/legacy/core_plugins/vis_type_vega/index.ts +++ b/src/legacy/core_plugins/vis_type_vega/index.ts @@ -33,9 +33,15 @@ const vegaPluginInitializer: LegacyPluginInitializer = ({ Plugin }: LegacyPlugin uiExports: { styleSheetPaths: resolve(__dirname, 'public/index.scss'), hacks: [resolve(__dirname, 'public/legacy')], - injectDefaultVars: server => ({ - enableExternalUrls: server.config().get('vega.enableExternalUrls'), - }), + injectDefaultVars: server => { + const serverConfig = server.config(); + const mapConfig: Record = serverConfig.get('map'); + + return { + emsTileLayerId: mapConfig.emsTileLayerId, + enableExternalUrls: serverConfig.get('vega.enableExternalUrls'), + }; + }, }, init: (server: Legacy.Server) => ({}), config(Joi: any) { From 9317f16c38c9fcf0aab7176396bf5af8ccf28643 Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Mon, 30 Dec 2019 22:14:58 -0500 Subject: [PATCH 3/9] Skip failing test suite --- test/functional/apps/home/_newsfeed.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/functional/apps/home/_newsfeed.ts b/test/functional/apps/home/_newsfeed.ts index 35d7ac8adefa5..0019b817b72d8 100644 --- a/test/functional/apps/home/_newsfeed.ts +++ b/test/functional/apps/home/_newsfeed.ts @@ -24,7 +24,8 @@ export default function({ getService, getPageObjects }: FtrProviderContext) { const globalNav = getService('globalNav'); const PageObjects = getPageObjects(['common', 'newsfeed']); - describe('Newsfeed', () => { + // Failing: https://github.com/elastic/kibana/issues/53860 + describe.skip('Newsfeed', () => { before(async () => { await PageObjects.newsfeed.resetPage(); }); From 98ac7a64ad76fa664a04a1d156d85a471bdaf5b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20C=C3=B4t=C3=A9?= Date: Tue, 31 Dec 2019 13:36:39 -0500 Subject: [PATCH 4/9] Add tests to ensure AAD isn't broken after performing a change on an alert / action (#53333) --- .../legacy/plugins/alerting/server/plugin.ts | 2 +- .../alerting_api_integration/common/config.ts | 1 + .../common/fixtures/plugins/aad/index.ts | 58 +++++++++++++++++++ .../common/fixtures/plugins/aad/package.json | 7 +++ .../common/lib/check_aad.ts | 20 +++++++ .../common/lib/index.ts | 1 + .../tests/actions/create.ts | 11 +++- .../tests/actions/update.ts | 9 ++- .../tests/alerting/create.ts | 9 ++- .../tests/alerting/disable.ts | 15 ++++- .../tests/alerting/enable.ts | 15 ++++- .../tests/alerting/mute_all.ts | 15 ++++- .../tests/alerting/mute_instance.ts | 15 ++++- .../tests/alerting/unmute_all.ts | 15 ++++- .../tests/alerting/unmute_instance.ts | 15 ++++- .../tests/alerting/update.ts | 9 ++- .../tests/alerting/update_api_key.ts | 15 ++++- .../spaces_only/tests/actions/create.ts | 12 +++- .../spaces_only/tests/actions/update.ts | 10 +++- .../spaces_only/tests/alerting/create.ts | 9 ++- .../spaces_only/tests/alerting/disable.ts | 16 ++++- .../spaces_only/tests/alerting/enable.ts | 16 ++++- .../spaces_only/tests/alerting/mute_all.ts | 16 ++++- .../tests/alerting/mute_instance.ts | 16 ++++- .../spaces_only/tests/alerting/unmute_all.ts | 16 ++++- .../tests/alerting/unmute_instance.ts | 16 ++++- .../spaces_only/tests/alerting/update.ts | 10 +++- .../tests/alerting/update_api_key.ts | 16 ++++- 28 files changed, 360 insertions(+), 25 deletions(-) create mode 100644 x-pack/test/alerting_api_integration/common/fixtures/plugins/aad/index.ts create mode 100644 x-pack/test/alerting_api_integration/common/fixtures/plugins/aad/package.json create mode 100644 x-pack/test/alerting_api_integration/common/lib/check_aad.ts diff --git a/x-pack/legacy/plugins/alerting/server/plugin.ts b/x-pack/legacy/plugins/alerting/server/plugin.ts index 3b17fa066d55a..935431c56ff30 100644 --- a/x-pack/legacy/plugins/alerting/server/plugin.ts +++ b/x-pack/legacy/plugins/alerting/server/plugin.ts @@ -71,7 +71,7 @@ export class Plugin { attributesToEncrypt: new Set(['apiKey']), attributesToExcludeFromAAD: new Set([ 'scheduledTaskId', - 'muted', + 'muteAll', 'mutedInstanceIds', 'updatedBy', ]), diff --git a/x-pack/test/alerting_api_integration/common/config.ts b/x-pack/test/alerting_api_integration/common/config.ts index 61e7ab3fe4639..8356954073ec9 100644 --- a/x-pack/test/alerting_api_integration/common/config.ts +++ b/x-pack/test/alerting_api_integration/common/config.ts @@ -78,6 +78,7 @@ export function createTestConfig(name: string, options: CreateTestConfigOptions) `--plugin-path=${path.join(__dirname, 'fixtures', 'plugins', 'alerts')}`, `--plugin-path=${path.join(__dirname, 'fixtures', 'plugins', 'actions')}`, `--plugin-path=${path.join(__dirname, 'fixtures', 'plugins', 'task_manager')}`, + `--plugin-path=${path.join(__dirname, 'fixtures', 'plugins', 'aad')}`, `--server.xsrf.whitelist=${JSON.stringify(getAllExternalServiceSimulatorPaths())}`, ...(ssl ? [ diff --git a/x-pack/test/alerting_api_integration/common/fixtures/plugins/aad/index.ts b/x-pack/test/alerting_api_integration/common/fixtures/plugins/aad/index.ts new file mode 100644 index 0000000000000..d7bee93f5c94b --- /dev/null +++ b/x-pack/test/alerting_api_integration/common/fixtures/plugins/aad/index.ts @@ -0,0 +1,58 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import Joi from 'joi'; +import Hapi from 'hapi'; +import { Legacy } from 'kibana'; +import KbnServer from '../../../../../../../src/legacy/server/kbn_server'; +import { PluginStartContract } from '../../../../../../plugins/encrypted_saved_objects/server'; + +interface CheckAADRequest extends Hapi.Request { + payload: { + spaceId?: string; + type: string; + id: string; + }; +} + +// eslint-disable-next-line import/no-default-export +export default function(kibana: any) { + return new kibana.Plugin({ + require: ['actions', 'alerting', 'encryptedSavedObjects'], + name: 'aad-fixtures', + init(server: Legacy.Server) { + const newPlatform = ((server as unknown) as KbnServer).newPlatform; + const esoPlugin = newPlatform.start.plugins.encryptedSavedObjects as PluginStartContract; + + server.route({ + method: 'POST', + path: '/api/check_aad', + options: { + validate: { + payload: Joi.object() + .keys({ + spaceId: Joi.string().optional(), + type: Joi.string().required(), + id: Joi.string().required(), + }) + .required(), + }, + }, + async handler(request: CheckAADRequest) { + let namespace: string | undefined; + const spacesPlugin = server.plugins.spaces; + if (spacesPlugin && request.payload.spaceId) { + namespace = spacesPlugin.spaceIdToNamespace(request.payload.spaceId); + } + await esoPlugin.getDecryptedAsInternalUser(request.payload.type, request.payload.id, { + namespace, + }); + return { success: true }; + }, + }); + }, + }); +} diff --git a/x-pack/test/alerting_api_integration/common/fixtures/plugins/aad/package.json b/x-pack/test/alerting_api_integration/common/fixtures/plugins/aad/package.json new file mode 100644 index 0000000000000..f5d174c18a209 --- /dev/null +++ b/x-pack/test/alerting_api_integration/common/fixtures/plugins/aad/package.json @@ -0,0 +1,7 @@ +{ + "name": "aad-fixtures", + "version": "0.0.0", + "kibana": { + "version": "kibana" + } +} diff --git a/x-pack/test/alerting_api_integration/common/lib/check_aad.ts b/x-pack/test/alerting_api_integration/common/lib/check_aad.ts new file mode 100644 index 0000000000000..0a75325aaed59 --- /dev/null +++ b/x-pack/test/alerting_api_integration/common/lib/check_aad.ts @@ -0,0 +1,20 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +interface Opts { + supertest: any; + spaceId?: string; + type: string; + id: string; +} + +export async function checkAAD({ supertest, spaceId, type, id }: Opts) { + await supertest + .post('/api/check_aad') + .set('kbn-xsrf', 'foo') + .send({ spaceId, type, id }) + .expect(200, { success: true }); +} diff --git a/x-pack/test/alerting_api_integration/common/lib/index.ts b/x-pack/test/alerting_api_integration/common/lib/index.ts index 31d257e3b25ac..a2f21264634f8 100644 --- a/x-pack/test/alerting_api_integration/common/lib/index.ts +++ b/x-pack/test/alerting_api_integration/common/lib/index.ts @@ -10,3 +10,4 @@ export { ES_TEST_INDEX_NAME, ESTestIndexTool } from './es_test_index_tool'; export { getTestAlertData } from './get_test_alert_data'; export { AlertUtils } from './alert_utils'; export { TaskManagerUtils } from './task_manager_utils'; +export { checkAAD } from './check_aad'; diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/create.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/create.ts index 57614aa816ff2..32d419bb562b2 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/create.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/create.ts @@ -6,7 +6,7 @@ import expect from '@kbn/expect'; import { UserAtSpaceScenarios } from '../../scenarios'; -import { getUrlPrefix, ObjectRemover } from '../../../common/lib'; +import { checkAAD, getUrlPrefix, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; // eslint-disable-next-line import/no-default-export @@ -52,6 +52,7 @@ export default function createActionTests({ getService }: FtrProviderContext) { case 'superuser at space1': case 'space_1_all at space1': expect(response.statusCode).to.eql(200); + objectRemover.add(space.id, response.body.id, 'action'); expect(response.body).to.eql({ id: response.body.id, name: 'My action', @@ -61,7 +62,13 @@ export default function createActionTests({ getService }: FtrProviderContext) { }, }); expect(typeof response.body.id).to.be('string'); - objectRemover.add(space.id, response.body.id, 'action'); + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: space.id, + type: 'action', + id: response.body.id, + }); break; default: throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/update.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/update.ts index d41a2a10b770a..ed6a1bb3d14a9 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/update.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/update.ts @@ -6,7 +6,7 @@ import expect from '@kbn/expect'; import { UserAtSpaceScenarios } from '../../scenarios'; -import { getUrlPrefix, ObjectRemover } from '../../../common/lib'; +import { checkAAD, getUrlPrefix, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; // eslint-disable-next-line import/no-default-export @@ -75,6 +75,13 @@ export default function updateActionTests({ getService }: FtrProviderContext) { unencrypted: `This value shouldn't get encrypted`, }, }); + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: space.id, + type: 'action', + id: createdAction.id, + }); break; default: throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/create.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/create.ts index a098a1fe02c1a..772f85c4dac8c 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/create.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/create.ts @@ -6,7 +6,7 @@ import expect from '@kbn/expect'; import { UserAtSpaceScenarios } from '../../scenarios'; -import { getTestAlertData, getUrlPrefix, ObjectRemover } from '../../../common/lib'; +import { checkAAD, getTestAlertData, getUrlPrefix, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; // eslint-disable-next-line import/no-default-export @@ -106,6 +106,13 @@ export default function createAlertTests({ getService }: FtrProviderContext) { alertId: response.body.id, spaceId: space.id, }); + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: space.id, + type: 'alert', + id: response.body.id, + }); break; default: throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/disable.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/disable.ts index d2076e0f92b3c..bafca30abf28a 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/disable.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/disable.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { UserAtSpaceScenarios } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; // eslint-disable-next-line import/no-default-export export default function createDisableAlertTests({ getService }: FtrProviderContext) { @@ -65,6 +71,13 @@ export default function createDisableAlertTests({ getService }: FtrProviderConte } catch (e) { expect(e.status).to.eql(404); } + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: space.id, + type: 'alert', + id: createdAlert.id, + }); break; default: throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/enable.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/enable.ts index 528db61dba21c..9df1f955232b1 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/enable.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/enable.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { UserAtSpaceScenarios } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; // eslint-disable-next-line import/no-default-export export default function createEnableAlertTests({ getService }: FtrProviderContext) { @@ -70,6 +76,13 @@ export default function createEnableAlertTests({ getService }: FtrProviderContex alertId: createdAlert.id, spaceId: space.id, }); + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: space.id, + type: 'alert', + id: createdAlert.id, + }); break; default: throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/mute_all.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/mute_all.ts index 9e479064e66c7..ce11fb8052b45 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/mute_all.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/mute_all.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { UserAtSpaceScenarios } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; // eslint-disable-next-line import/no-default-export export default function createMuteAlertTests({ getService }: FtrProviderContext) { @@ -55,6 +61,13 @@ export default function createMuteAlertTests({ getService }: FtrProviderContext) .auth(user.username, user.password) .expect(200); expect(updatedAlert.muteAll).to.eql(true); + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: space.id, + type: 'alert', + id: createdAlert.id, + }); break; default: throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/mute_instance.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/mute_instance.ts index 302b61074e87d..f91b54514ae05 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/mute_instance.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/mute_instance.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { UserAtSpaceScenarios } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; // eslint-disable-next-line import/no-default-export export default function createMuteAlertInstanceTests({ getService }: FtrProviderContext) { @@ -55,6 +61,13 @@ export default function createMuteAlertInstanceTests({ getService }: FtrProvider .auth(user.username, user.password) .expect(200); expect(updatedAlert.mutedInstanceIds).to.eql(['1']); + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: space.id, + type: 'alert', + id: createdAlert.id, + }); break; default: throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/unmute_all.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/unmute_all.ts index 5e9ad66cf40f3..f2598ff7c5493 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/unmute_all.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/unmute_all.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { UserAtSpaceScenarios } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; // eslint-disable-next-line import/no-default-export export default function createUnmuteAlertTests({ getService }: FtrProviderContext) { @@ -60,6 +66,13 @@ export default function createUnmuteAlertTests({ getService }: FtrProviderContex .auth(user.username, user.password) .expect(200); expect(updatedAlert.muteAll).to.eql(false); + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: space.id, + type: 'alert', + id: createdAlert.id, + }); break; default: throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/unmute_instance.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/unmute_instance.ts index b466575841d0a..ca58b58e5e822 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/unmute_instance.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/unmute_instance.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { UserAtSpaceScenarios } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; // eslint-disable-next-line import/no-default-export export default function createMuteAlertInstanceTests({ getService }: FtrProviderContext) { @@ -60,6 +66,13 @@ export default function createMuteAlertInstanceTests({ getService }: FtrProvider .auth(user.username, user.password) .expect(200); expect(updatedAlert.mutedInstanceIds).to.eql([]); + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: space.id, + type: 'alert', + id: createdAlert.id, + }); break; default: throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts index 8cb01b5467388..ec162a75ee0a5 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts @@ -7,7 +7,7 @@ import expect from '@kbn/expect'; import { Response as SupertestResponse } from 'supertest'; import { UserAtSpaceScenarios } from '../../scenarios'; -import { getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; +import { checkAAD, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; // eslint-disable-next-line import/no-default-export @@ -82,6 +82,13 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { mutedInstanceIds: [], scheduledTaskId: createdAlert.scheduledTaskId, }); + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: space.id, + type: 'alert', + id: createdAlert.id, + }); break; default: throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update_api_key.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update_api_key.ts index 4963d749c2935..b54147348d9a3 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update_api_key.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update_api_key.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { UserAtSpaceScenarios } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; // eslint-disable-next-line import/no-default-export export default function createUpdateApiKeyTests({ getService }: FtrProviderContext) { @@ -55,6 +61,13 @@ export default function createUpdateApiKeyTests({ getService }: FtrProviderConte .auth(user.username, user.password) .expect(200); expect(updatedAlert.apiKeyOwner).to.eql(user.username); + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: space.id, + type: 'alert', + id: createdAlert.id, + }); break; default: throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`); diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/create.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/create.ts index 982688cc45c05..74e8e0f832299 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/create.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/create.ts @@ -6,7 +6,7 @@ import expect from '@kbn/expect'; import { Spaces } from '../../scenarios'; -import { getUrlPrefix, ObjectRemover } from '../../../common/lib'; +import { checkAAD, getUrlPrefix, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; // eslint-disable-next-line import/no-default-export @@ -34,6 +34,7 @@ export default function createActionTests({ getService }: FtrProviderContext) { }); expect(response.statusCode).to.eql(200); + objectRemover.add(Spaces.space1.id, response.body.id, 'action'); expect(response.body).to.eql({ id: response.body.id, name: 'My action', @@ -43,7 +44,14 @@ export default function createActionTests({ getService }: FtrProviderContext) { }, }); expect(typeof response.body.id).to.be('string'); - objectRemover.add(Spaces.space1.id, response.body.id, 'action'); + + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: Spaces.space1.id, + type: 'action', + id: response.body.id, + }); }); }); } diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/update.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/update.ts index bb1ee31bddfe0..fb0c5e13c0720 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/actions/update.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/actions/update.ts @@ -5,7 +5,7 @@ */ import { Spaces } from '../../scenarios'; -import { getUrlPrefix, ObjectRemover } from '../../../common/lib'; +import { checkAAD, getUrlPrefix, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; // eslint-disable-next-line import/no-default-export @@ -54,6 +54,14 @@ export default function updateActionTests({ getService }: FtrProviderContext) { unencrypted: `This value shouldn't get encrypted`, }, }); + + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: Spaces.space1.id, + type: 'action', + id: createdAction.id, + }); }); it(`shouldn't update action from another space`, async () => { diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/create.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/create.ts index c61c94bd603fb..d64065b596498 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/create.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/create.ts @@ -6,7 +6,7 @@ import expect from '@kbn/expect'; import { Spaces } from '../../scenarios'; -import { getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; +import { checkAAD, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; // eslint-disable-next-line import/no-default-export @@ -88,6 +88,13 @@ export default function createAlertTests({ getService }: FtrProviderContext) { alertId: response.body.id, spaceId: Spaces.space1.id, }); + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: Spaces.space1.id, + type: 'alert', + id: response.body.id, + }); }); it('should handle create alert request appropriately when an alert is disabled ', async () => { diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/disable.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/disable.ts index 750f94201216a..9d5017530e075 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/disable.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/disable.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { Spaces } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; // eslint-disable-next-line import/no-default-export export default function createDisableAlertTests({ getService }: FtrProviderContext) { @@ -43,6 +49,14 @@ export default function createDisableAlertTests({ getService }: FtrProviderConte } catch (e) { expect(e.status).to.eql(404); } + + // Ensure AAD isn't broken + await checkAAD({ + supertest: supertestWithoutAuth, + spaceId: Spaces.space1.id, + type: 'alert', + id: createdAlert.id, + }); }); it(`shouldn't disable alert from another space`, async () => { diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/enable.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/enable.ts index 00cd40c0e80cd..4459dd744fe4b 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/enable.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/enable.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { Spaces } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; // eslint-disable-next-line import/no-default-export export default function createEnableAlertTests({ getService }: FtrProviderContext) { @@ -49,6 +55,14 @@ export default function createEnableAlertTests({ getService }: FtrProviderContex alertId: createdAlert.id, spaceId: Spaces.space1.id, }); + + // Ensure AAD isn't broken + await checkAAD({ + supertest: supertestWithoutAuth, + spaceId: Spaces.space1.id, + type: 'alert', + id: createdAlert.id, + }); }); it(`shouldn't enable alert from another space`, async () => { diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/mute_all.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/mute_all.ts index dba3046aa4b7f..4781aed640f51 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/mute_all.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/mute_all.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { Spaces } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; // eslint-disable-next-line import/no-default-export export default function createMuteTests({ getService }: FtrProviderContext) { @@ -34,6 +40,14 @@ export default function createMuteTests({ getService }: FtrProviderContext) { .set('kbn-xsrf', 'foo') .expect(200); expect(updatedAlert.muteAll).to.eql(true); + + // Ensure AAD isn't broken + await checkAAD({ + supertest: supertestWithoutAuth, + spaceId: Spaces.space1.id, + type: 'alert', + id: createdAlert.id, + }); }); }); } diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/mute_instance.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/mute_instance.ts index 09ca359716026..0ee4d076d7b3c 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/mute_instance.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/mute_instance.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { Spaces } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; // eslint-disable-next-line import/no-default-export export default function createMuteInstanceTests({ getService }: FtrProviderContext) { @@ -34,6 +40,14 @@ export default function createMuteInstanceTests({ getService }: FtrProviderConte .set('kbn-xsrf', 'foo') .expect(200); expect(updatedAlert.mutedInstanceIds).to.eql(['1']); + + // Ensure AAD isn't broken + await checkAAD({ + supertest: supertestWithoutAuth, + spaceId: Spaces.space1.id, + type: 'alert', + id: createdAlert.id, + }); }); }); } diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/unmute_all.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/unmute_all.ts index 70ee32a9e6fb9..a7bf065d5795d 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/unmute_all.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/unmute_all.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { Spaces } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; // eslint-disable-next-line import/no-default-export export default function createUnmuteTests({ getService }: FtrProviderContext) { @@ -35,6 +41,14 @@ export default function createUnmuteTests({ getService }: FtrProviderContext) { .set('kbn-xsrf', 'foo') .expect(200); expect(updatedAlert.muteAll).to.eql(false); + + // Ensure AAD isn't broken + await checkAAD({ + supertest: supertestWithoutAuth, + spaceId: Spaces.space1.id, + type: 'alert', + id: createdAlert.id, + }); }); }); } diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/unmute_instance.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/unmute_instance.ts index d0f1b2fdb3f9f..868bbf66b1c32 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/unmute_instance.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/unmute_instance.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { Spaces } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; // eslint-disable-next-line import/no-default-export export default function createUnmuteInstanceTests({ getService }: FtrProviderContext) { @@ -35,6 +41,14 @@ export default function createUnmuteInstanceTests({ getService }: FtrProviderCon .set('kbn-xsrf', 'foo') .expect(200); expect(updatedAlert.mutedInstanceIds).to.eql([]); + + // Ensure AAD isn't broken + await checkAAD({ + supertest: supertestWithoutAuth, + spaceId: Spaces.space1.id, + type: 'alert', + id: createdAlert.id, + }); }); }); } diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/update.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/update.ts index fd6d81e296ef0..ecc614ad3807b 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/update.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/update.ts @@ -5,7 +5,7 @@ */ import { Spaces } from '../../scenarios'; -import { getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; +import { checkAAD, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; // eslint-disable-next-line import/no-default-export @@ -53,6 +53,14 @@ export default function createUpdateTests({ getService }: FtrProviderContext) { mutedInstanceIds: [], scheduledTaskId: createdAlert.scheduledTaskId, }); + + // Ensure AAD isn't broken + await checkAAD({ + supertest, + spaceId: Spaces.space1.id, + type: 'alert', + id: createdAlert.id, + }); }); it(`shouldn't update alert from another space`, async () => { diff --git a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/update_api_key.ts b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/update_api_key.ts index 2cd3634043740..4c4e641f90277 100644 --- a/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/update_api_key.ts +++ b/x-pack/test/alerting_api_integration/spaces_only/tests/alerting/update_api_key.ts @@ -6,8 +6,14 @@ import expect from '@kbn/expect'; import { Spaces } from '../../scenarios'; -import { AlertUtils, getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib'; import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { + AlertUtils, + checkAAD, + getUrlPrefix, + getTestAlertData, + ObjectRemover, +} from '../../../common/lib'; /** * Eventhough security is disabled, this test checks the API behavior. @@ -38,6 +44,14 @@ export default function createUpdateApiKeyTests({ getService }: FtrProviderConte .set('kbn-xsrf', 'foo') .expect(200); expect(updatedAlert.apiKeyOwner).to.eql(null); + + // Ensure AAD isn't broken + await checkAAD({ + supertest: supertestWithoutAuth, + spaceId: Spaces.space1.id, + type: 'alert', + id: createdAlert.id, + }); }); it(`shouldn't update alert api key from another space`, async () => { From f8f777b5baf1b33ff73b4495ea50c219d678dd86 Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Tue, 31 Dec 2019 15:14:55 -0500 Subject: [PATCH 5/9] Add kibanamachine support to Github PR comments (#53852) * Add kibanamachine support to Github PR comments * Temporary commit for quick successful pipeline * Only delete the last comment if it was made by kibanamachine * Revert "Temporary commit for quick successful pipeline" This reverts commit d31f579697682b93dfc1381070c98fa45491560c. --- vars/githubPr.groovy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vars/githubPr.groovy b/vars/githubPr.groovy index 09a166192bf7a..ce164ab98ab1e 100644 --- a/vars/githubPr.groovy +++ b/vars/githubPr.groovy @@ -35,7 +35,7 @@ def withDefaultPrComments(closure) { def message = getNextCommentMessage(info) postComment(message) - if (lastComment) { + if (lastComment && lastComment.user.login == 'kibanamachine') { deleteComment(lastComment.id) } } @@ -49,7 +49,7 @@ def isPr() { def getLatestBuildComment() { return getComments() .reverse() - .find { it.user.login == 'elasticmachine' && it.body =~ /