From 02fcbaa794ada3464a5588913f3b31ce898edcf3 Mon Sep 17 00:00:00 2001 From: Frank Hassanabad Date: Wed, 19 Aug 2020 12:30:11 -0600 Subject: [PATCH] Fixed bug where list index privileges was returned twice instead of list item index (#75256) ## Summary Fixes a bug where the list privileges was returning the `.list` privileges twice instead of returning it once and returning the `.items` privileges second with the call. No UI has to change as the way it was written was dynamic to grab the first key found. This also adds the functional tests to `x-pack/scripts/functional_tests.js` which was not there originally so the end to tend tests should actually run on the CI machine where it was not running on CI before. Adds the functional tests to the code owners file as well. Ensure that you go to the test results page from the Jenkins build: Screen Shot 2020-08-18 at 1 13 18 AM And ensure you see the tests under: ``` X-Pack Lists Integration Tests ``` Then click through it and ensure they are shown as running and passing ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --- .github/CODEOWNERS | 1 + .../routes/read_privileges_route.mock.ts | 181 ++++++++++++++++++ .../server/routes/read_privileges_route.ts | 2 +- x-pack/scripts/functional_tests.js | 1 + .../security_and_spaces/tests/index.ts | 1 + .../tests/read_list_privileges.ts | 85 ++++++++ 6 files changed, 270 insertions(+), 1 deletion(-) create mode 100644 x-pack/plugins/lists/server/routes/read_privileges_route.mock.ts create mode 100644 x-pack/test/lists_api_integration/security_and_spaces/tests/read_list_privileges.ts diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 7e34c931c5feb2..52df586b8bda73 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -267,6 +267,7 @@ x-pack/plugins/telemetry_collection_xpack/schema/xpack_plugins.json @elastic/kib /x-pack/plugins/security_solution/ @elastic/siem @elastic/endpoint-app-team /x-pack/plugins/security_solution/**/*.scss @elastic/security-design /x-pack/test/detection_engine_api_integration @elastic/siem @elastic/endpoint-app-team +/x-pack/test/lists_api_integration @elastic/siem @elastic/endpoint-app-team /x-pack/test/api_integration/apis/security_solution @elastic/siem @elastic/endpoint-app-team /x-pack/plugins/case @elastic/siem @elastic/endpoint-app-team /x-pack/plugins/lists @elastic/siem @elastic/endpoint-app-team diff --git a/x-pack/plugins/lists/server/routes/read_privileges_route.mock.ts b/x-pack/plugins/lists/server/routes/read_privileges_route.mock.ts new file mode 100644 index 00000000000000..cef6233440db69 --- /dev/null +++ b/x-pack/plugins/lists/server/routes/read_privileges_route.mock.ts @@ -0,0 +1,181 @@ +/* + * 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 Cluster { + monitor_ml: boolean; + manage_ccr: boolean; + manage_index_templates: boolean; + monitor_watcher: boolean; + monitor_transform: boolean; + read_ilm: boolean; + manage_security: boolean; + manage_own_api_key: boolean; + manage_saml: boolean; + all: boolean; + manage_ilm: boolean; + manage_ingest_pipelines: boolean; + read_ccr: boolean; + manage_rollup: boolean; + monitor: boolean; + manage_watcher: boolean; + manage: boolean; + manage_transform: boolean; + manage_api_key: boolean; + manage_token: boolean; + manage_ml: boolean; + manage_pipeline: boolean; + monitor_rollup: boolean; + transport_client: boolean; + create_snapshot: boolean; +} + +interface Index { + [indexName: string]: { + all: boolean; + manage_ilm: boolean; + read: boolean; + create_index: boolean; + read_cross_cluster: boolean; + index: boolean; + monitor: boolean; + delete: boolean; + manage: boolean; + delete_index: boolean; + create_doc: boolean; + view_index_metadata: boolean; + create: boolean; + manage_follow_index: boolean; + manage_leader_index: boolean; + write: boolean; + }; +} + +interface IndexPrivilege { + application: {}; + cluster: Cluster; + has_all_requested: boolean; + index: Index; + username: string; +} + +export interface Privilege { + listItems: IndexPrivilege; + lists: IndexPrivilege; + is_authenticated: boolean; +} + +export const getReadPrivilegeMock = ( + listIndex: string = '.lists-default', + listItemsIndex: string = '.items-default', + username = 'elastic', + booleanValues: boolean = true +): Privilege => ({ + is_authenticated: true, + listItems: { + application: {}, + cluster: { + all: booleanValues, + create_snapshot: booleanValues, + manage: booleanValues, + manage_api_key: booleanValues, + manage_ccr: booleanValues, + manage_ilm: booleanValues, + manage_index_templates: booleanValues, + manage_ingest_pipelines: booleanValues, + manage_ml: booleanValues, + manage_own_api_key: false, + manage_pipeline: booleanValues, + manage_rollup: booleanValues, + manage_saml: booleanValues, + manage_security: booleanValues, + manage_token: booleanValues, + manage_transform: booleanValues, + manage_watcher: booleanValues, + monitor: booleanValues, + monitor_ml: booleanValues, + monitor_rollup: booleanValues, + monitor_transform: booleanValues, + monitor_watcher: booleanValues, + read_ccr: booleanValues, + read_ilm: booleanValues, + transport_client: booleanValues, + }, + has_all_requested: false, + index: { + [listItemsIndex]: { + all: booleanValues, + create: booleanValues, + create_doc: booleanValues, + create_index: booleanValues, + delete: booleanValues, + delete_index: booleanValues, + index: booleanValues, + manage: booleanValues, + manage_follow_index: booleanValues, + manage_ilm: booleanValues, + manage_leader_index: booleanValues, + monitor: booleanValues, + read: booleanValues, + read_cross_cluster: booleanValues, + view_index_metadata: booleanValues, + write: booleanValues, + }, + }, + username, + }, + lists: { + application: {}, + cluster: { + all: booleanValues, + create_snapshot: booleanValues, + manage: booleanValues, + manage_api_key: booleanValues, + manage_ccr: booleanValues, + manage_ilm: booleanValues, + manage_index_templates: booleanValues, + manage_ingest_pipelines: booleanValues, + manage_ml: booleanValues, + manage_own_api_key: false, + manage_pipeline: booleanValues, + manage_rollup: booleanValues, + manage_saml: booleanValues, + manage_security: booleanValues, + manage_token: booleanValues, + manage_transform: booleanValues, + manage_watcher: booleanValues, + monitor: booleanValues, + monitor_ml: booleanValues, + monitor_rollup: booleanValues, + monitor_transform: booleanValues, + monitor_watcher: booleanValues, + read_ccr: booleanValues, + read_ilm: booleanValues, + transport_client: booleanValues, + }, + has_all_requested: false, + index: { + [listIndex]: { + all: booleanValues, + create: booleanValues, + create_doc: booleanValues, + create_index: booleanValues, + delete: booleanValues, + delete_index: booleanValues, + index: booleanValues, + manage: booleanValues, + manage_follow_index: booleanValues, + manage_ilm: booleanValues, + manage_leader_index: booleanValues, + monitor: booleanValues, + read: booleanValues, + read_cross_cluster: booleanValues, + view_index_metadata: booleanValues, + write: booleanValues, + }, + }, + username, + }, +}); diff --git a/x-pack/plugins/lists/server/routes/read_privileges_route.ts b/x-pack/plugins/lists/server/routes/read_privileges_route.ts index a4ec878613608a..9d695b348b422d 100644 --- a/x-pack/plugins/lists/server/routes/read_privileges_route.ts +++ b/x-pack/plugins/lists/server/routes/read_privileges_route.ts @@ -36,7 +36,7 @@ export const readPrivilegesRoute = ( ); const clusterPrivilegesListItems = await readPrivileges( clusterClient.callAsCurrentUser, - lists.getListIndex() + lists.getListItemIndex() ); const privileges = merge( { diff --git a/x-pack/scripts/functional_tests.js b/x-pack/scripts/functional_tests.js index 205ff500a36ec4..ef646343871d31 100644 --- a/x-pack/scripts/functional_tests.js +++ b/x-pack/scripts/functional_tests.js @@ -26,6 +26,7 @@ const onlyNotInCoverageTests = [ require.resolve('../test/apm_api_integration/trial/config.ts'), require.resolve('../test/detection_engine_api_integration/security_and_spaces/config.ts'), require.resolve('../test/detection_engine_api_integration/basic/config.ts'), + require.resolve('../test/lists_api_integration/security_and_spaces/config.ts'), require.resolve('../test/plugin_api_integration/config.ts'), require.resolve('../test/kerberos_api_integration/config.ts'), require.resolve('../test/kerberos_api_integration/anonymous_access.config.ts'), diff --git a/x-pack/test/lists_api_integration/security_and_spaces/tests/index.ts b/x-pack/test/lists_api_integration/security_and_spaces/tests/index.ts index 5458b4a9a7db25..9e619811d67ee1 100644 --- a/x-pack/test/lists_api_integration/security_and_spaces/tests/index.ts +++ b/x-pack/test/lists_api_integration/security_and_spaces/tests/index.ts @@ -33,5 +33,6 @@ export default ({ loadTestFile }: FtrProviderContext): void => { loadTestFile(require.resolve('./delete_exception_list_items')); loadTestFile(require.resolve('./find_exception_lists')); loadTestFile(require.resolve('./find_exception_list_items')); + loadTestFile(require.resolve('./read_list_privileges')); }); }; diff --git a/x-pack/test/lists_api_integration/security_and_spaces/tests/read_list_privileges.ts b/x-pack/test/lists_api_integration/security_and_spaces/tests/read_list_privileges.ts new file mode 100644 index 00000000000000..c1696d32052945 --- /dev/null +++ b/x-pack/test/lists_api_integration/security_and_spaces/tests/read_list_privileges.ts @@ -0,0 +1,85 @@ +/* + * 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 expect from '@kbn/expect'; + +import { getReadPrivilegeMock } from '../../../../plugins/lists/server/routes/read_privileges_route.mock'; +import { FtrProviderContext } from '../../common/ftr_provider_context'; +import { LIST_PRIVILEGES_URL } from '../../../../plugins/lists/common/constants'; + +// eslint-disable-next-line import/no-default-export +export default ({ getService }: FtrProviderContext) => { + const supertest = getService('supertest'); + const security = getService('security'); + const spacesService = getService('spaces'); + const supertestWithoutAuth = getService('supertestWithoutAuth'); + + describe('read_list_privileges', () => { + const space1Id = 'space_1'; + + const user1 = { + username: 'user_1', + roleName: 'user_1', + password: 'user_1-password', + }; + + beforeEach(async () => { + await spacesService.create({ + id: space1Id, + name: space1Id, + disabledFeatures: [], + }); + + await security.role.create(user1.roleName, { + kibana: [ + { + feature: { + dashboard: ['all'], + siem: ['all', 'read'], + }, + spaces: [space1Id], + }, + ], + }); + + await security.user.create(user1.username, { + password: user1.password, + roles: [user1.roleName], + }); + }); + + afterEach(async () => { + await spacesService.delete(space1Id); + }); + + it('should return true for all privileges when its the system user of "elastic" in space of "default"', async () => { + const { body } = await supertest.get(LIST_PRIVILEGES_URL).set('kbn-xsrf', 'true').expect(200); + expect(body).to.eql(getReadPrivilegeMock()); + }); + + it('should return true for all privileges when its the system user of "elastic" in space of "space_1"', async () => { + const { body } = await supertest.get(LIST_PRIVILEGES_URL).set('kbn-xsrf', 'true').expect(200); + expect(body).to.eql(getReadPrivilegeMock()); + }); + + it('should return false for all privileges when its the system user of "user_1" in a space of "space_1"', async () => { + const { body } = await supertestWithoutAuth + .get(`/s/${space1Id}${LIST_PRIVILEGES_URL}`) + .auth(user1.username, user1.password) + .send() + .expect(200); + + const privilege = getReadPrivilegeMock( + `.lists-${space1Id}`, + `.items-${space1Id}`, + user1.username, + false + ); + + expect(body).to.eql(privilege); + }); + }); +};