From 33620664699c77f1d7921c6040b4f9b6dd1ecef2 Mon Sep 17 00:00:00 2001 From: Brandon Kobel Date: Thu, 13 Jun 2019 10:35:26 -0700 Subject: [PATCH] Spaces - Hiding management link (#38472) (#38903) * Changing the Spaces management section to behave like the other FC controlled sections * Adding those glorious tests and fixing a bug * Fixing some test descriptions * Making the mergeCapabilities operation emulate the old behavior * Fixing privileges test with the addition of the new action * Updating jest snapshot * Adding tests, preventing additional clobbering * Changing requireUICapability to use management.kibana.spaces --- .../capabilities/merge_capabilities.test.ts | 84 +++++++++ .../server/capabilities/merge_capabilities.ts | 29 +-- .../privileges/privileges.test.ts | 24 ++- .../authorization/privileges/privileges.ts | 1 + x-pack/plugins/spaces/index.ts | 5 + .../edit_space/manage_space_page.tsx | 4 +- .../spaces/public/views/management/index.tsx | 2 +- .../public/views/management/page_routes.tsx | 3 + .../spaces_grid_pages.test.tsx.snap | 1 + .../spaces_grid/spaces_grid_page.tsx | 2 +- .../feature_controls/spaces_security.ts | 175 ++++++++++++++++++ x-pack/test/functional/apps/spaces/index.ts | 1 + 12 files changed, 312 insertions(+), 19 deletions(-) create mode 100644 src/legacy/server/capabilities/merge_capabilities.test.ts create mode 100644 x-pack/test/functional/apps/spaces/feature_controls/spaces_security.ts diff --git a/src/legacy/server/capabilities/merge_capabilities.test.ts b/src/legacy/server/capabilities/merge_capabilities.test.ts new file mode 100644 index 00000000000000..a73b81bdaf0a32 --- /dev/null +++ b/src/legacy/server/capabilities/merge_capabilities.test.ts @@ -0,0 +1,84 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { mergeCapabilities } from './merge_capabilities'; + +const defaultProps = { + catalogue: {}, + management: {}, + navLinks: {}, +}; + +test(`"{ foo: {} }" doesn't clobber "{ foo: { bar: true } }"`, () => { + const output1 = mergeCapabilities({ foo: { bar: true } }, { foo: {} }); + expect(output1).toEqual({ ...defaultProps, foo: { bar: true } }); + + const output2 = mergeCapabilities({ foo: { bar: true } }, { foo: {} }); + expect(output2).toEqual({ ...defaultProps, foo: { bar: true } }); +}); + +test(`"{ foo: { bar: true } }" doesn't clobber "{ baz: { quz: true } }"`, () => { + const output1 = mergeCapabilities({ foo: { bar: true } }, { baz: { quz: true } }); + expect(output1).toEqual({ ...defaultProps, foo: { bar: true }, baz: { quz: true } }); + + const output2 = mergeCapabilities({ baz: { quz: true } }, { foo: { bar: true } }); + expect(output2).toEqual({ ...defaultProps, foo: { bar: true }, baz: { quz: true } }); +}); + +test(`"{ foo: { bar: { baz: true } } }" doesn't clobber "{ foo: { bar: { quz: true } } }"`, () => { + const output1 = mergeCapabilities( + { foo: { bar: { baz: true } } }, + { foo: { bar: { quz: true } } } + ); + expect(output1).toEqual({ ...defaultProps, foo: { bar: { baz: true, quz: true } } }); + + const output2 = mergeCapabilities( + { foo: { bar: { quz: true } } }, + { foo: { bar: { baz: true } } } + ); + expect(output2).toEqual({ ...defaultProps, foo: { bar: { baz: true, quz: true } } }); +}); + +test(`error is thrown if boolean and object clash`, () => { + expect(() => { + mergeCapabilities({ foo: { bar: { baz: true } } }, { foo: { bar: true } }); + }).toThrowErrorMatchingInlineSnapshot(`"a boolean and an object can't be merged"`); + + expect(() => { + mergeCapabilities({ foo: { bar: true } }, { foo: { bar: { baz: true } } }); + }).toThrowErrorMatchingInlineSnapshot(`"a boolean and an object can't be merged"`); +}); + +test(`supports duplicates as long as the booleans are the same`, () => { + const output1 = mergeCapabilities({ foo: { bar: true } }, { foo: { bar: true } }); + expect(output1).toEqual({ ...defaultProps, foo: { bar: true } }); + + const output2 = mergeCapabilities({ foo: { bar: false } }, { foo: { bar: false } }); + expect(output2).toEqual({ ...defaultProps, foo: { bar: false } }); +}); + +test(`error is thrown if merging "true" and "false"`, () => { + expect(() => { + mergeCapabilities({ foo: { bar: false } }, { foo: { bar: true } }); + }).toThrowErrorMatchingInlineSnapshot(`"\\"true\\" and \\"false\\" can't be merged"`); + + expect(() => { + mergeCapabilities({ foo: { bar: true } }, { foo: { bar: false } }); + }).toThrowErrorMatchingInlineSnapshot(`"\\"true\\" and \\"false\\" can't be merged"`); +}); diff --git a/src/legacy/server/capabilities/merge_capabilities.ts b/src/legacy/server/capabilities/merge_capabilities.ts index e0f2d22ab34adc..5fe31775ba32d8 100644 --- a/src/legacy/server/capabilities/merge_capabilities.ts +++ b/src/legacy/server/capabilities/merge_capabilities.ts @@ -17,23 +17,28 @@ * under the License. */ +import typeDetect from 'type-detect'; +import { merge } from 'lodash'; import { Capabilities } from '../../../core/public'; export const mergeCapabilities = (...sources: Array>): Capabilities => - sources.reduce( - (capabilities: Capabilities, source) => { - Object.entries(source).forEach(([key, value = {}]) => { - capabilities[key] = { - ...value, - ...capabilities[key], - }; - }); - - return capabilities; - }, + merge( { navLinks: {}, management: {}, catalogue: {}, - } as Capabilities + }, + ...sources, + (a: any, b: any) => { + if ( + (typeDetect(a) === 'boolean' && typeDetect(b) === 'Object') || + (typeDetect(b) === 'boolean' && typeDetect(a) === 'Object') + ) { + throw new Error(`a boolean and an object can't be merged`); + } + + if (typeDetect(a) === 'boolean' && typeDetect(b) === 'boolean' && a !== b) { + throw new Error(`"true" and "false" can't be merged`); + } + } ); diff --git a/x-pack/plugins/security/server/lib/authorization/privileges/privileges.test.ts b/x-pack/plugins/security/server/lib/authorization/privileges/privileges.test.ts index 3bd3f53fce5312..1447b45c821218 100644 --- a/x-pack/plugins/security/server/lib/authorization/privileges/privileges.test.ts +++ b/x-pack/plugins/security/server/lib/authorization/privileges/privileges.test.ts @@ -318,7 +318,13 @@ describe('features', () => { actions.login, actions.version, ...(expectGetFeatures ? [actions.api.get('features')] : []), - ...(expectManageSpaces ? [actions.space.manage, actions.ui.get('spaces', 'manage')] : []), + ...(expectManageSpaces + ? [ + actions.space.manage, + actions.ui.get('spaces', 'manage'), + actions.ui.get('management', 'kibana', 'spaces'), + ] + : []), actions.app.get('app-1'), actions.app.get('app-2'), actions.ui.get('catalogue', 'catalogue-1'), @@ -403,7 +409,13 @@ describe('features', () => { actions.login, actions.version, ...(expectGetFeatures ? [actions.api.get('features')] : []), - ...(expectManageSpaces ? [actions.space.manage, actions.ui.get('spaces', 'manage')] : []), + ...(expectManageSpaces + ? [ + actions.space.manage, + actions.ui.get('spaces', 'manage'), + actions.ui.get('management', 'kibana', 'spaces'), + ] + : []), actions.ui.get('catalogue', 'bar-catalogue-1'), actions.ui.get('catalogue', 'bar-catalogue-2'), actions.ui.get('management', 'bar-management', 'bar-management-1'), @@ -614,7 +626,13 @@ describe('features', () => { actions.login, actions.version, ...(expectGetFeatures ? [actions.api.get('features')] : []), - ...(expectManageSpaces ? [actions.space.manage, actions.ui.get('spaces', 'manage')] : []), + ...(expectManageSpaces + ? [ + actions.space.manage, + actions.ui.get('spaces', 'manage'), + actions.ui.get('management', 'kibana', 'spaces'), + ] + : []), actions.allHack, ]); expect(actual).toHaveProperty(`${group}.read`, [actions.login, actions.version]); diff --git a/x-pack/plugins/security/server/lib/authorization/privileges/privileges.ts b/x-pack/plugins/security/server/lib/authorization/privileges/privileges.ts index c858bc61393f55..60c929854aed4c 100644 --- a/x-pack/plugins/security/server/lib/authorization/privileges/privileges.ts +++ b/x-pack/plugins/security/server/lib/authorization/privileges/privileges.ts @@ -65,6 +65,7 @@ export function privilegesFactory(actions: Actions, xpackMainPlugin: XPackMainPl actions.api.get('features'), actions.space.manage, actions.ui.get('spaces', 'manage'), + actions.ui.get('management', 'kibana', 'spaces'), ...allActions, actions.allHack, ], diff --git a/x-pack/plugins/spaces/index.ts b/x-pack/plugins/spaces/index.ts index 13309a800e8b49..443243581434d9 100644 --- a/x-pack/plugins/spaces/index.ts +++ b/x-pack/plugins/spaces/index.ts @@ -49,6 +49,11 @@ export const spaces = (kibana: Record) => spaces: { manage: true, }, + management: { + kibana: { + spaces: true, + }, + }, }; }, diff --git a/x-pack/plugins/spaces/public/views/management/edit_space/manage_space_page.tsx b/x-pack/plugins/spaces/public/views/management/edit_space/manage_space_page.tsx index 9ea3ecdaa5739a..fa0ebfa11201b4 100644 --- a/x-pack/plugins/spaces/public/views/management/edit_space/manage_space_page.tsx +++ b/x-pack/plugins/spaces/public/views/management/edit_space/manage_space_page.tsx @@ -144,7 +144,7 @@ class ManageSpacePageUI extends Component { const { showAlteringActiveSpaceDialog } = this.state; return ( - +
{this.getFormHeading()} @@ -188,7 +188,7 @@ class ManageSpacePageUI extends Component { }} /> )} - +
); }; diff --git a/x-pack/plugins/spaces/public/views/management/index.tsx b/x-pack/plugins/spaces/public/views/management/index.tsx index 2d8d1c7feb2db0..656193b417aa78 100644 --- a/x-pack/plugins/spaces/public/views/management/index.tsx +++ b/x-pack/plugins/spaces/public/views/management/index.tsx @@ -18,7 +18,7 @@ import routes from 'ui/routes'; import { AdvancedSettingsSubtitle } from './components/advanced_settings_subtitle'; import { AdvancedSettingsTitle } from './components/advanced_settings_title'; -const MANAGE_SPACES_KEY = 'manage_spaces'; +const MANAGE_SPACES_KEY = 'spaces'; routes.defaults(/\/management/, { resolve: { diff --git a/x-pack/plugins/spaces/public/views/management/page_routes.tsx b/x-pack/plugins/spaces/public/views/management/page_routes.tsx index 7ef313ce5927d7..34ca4acc53efe0 100644 --- a/x-pack/plugins/spaces/public/views/management/page_routes.tsx +++ b/x-pack/plugins/spaces/public/views/management/page_routes.tsx @@ -21,6 +21,7 @@ const reactRootNodeId = 'manageSpacesReactRoot'; routes.when('/management/spaces/list', { template, k7Breadcrumbs: getListBreadcrumbs, + requireUICapability: 'management.kibana.spaces', controller( $scope: any, $http: any, @@ -53,6 +54,7 @@ routes.when('/management/spaces/list', { routes.when('/management/spaces/create', { template, k7Breadcrumbs: getCreateBreadcrumbs, + requireUICapability: 'management.kibana.spaces', controller( $scope: any, $http: any, @@ -89,6 +91,7 @@ routes.when('/management/spaces/edit', { routes.when('/management/spaces/edit/:spaceId', { template, k7Breadcrumbs: () => getEditBreadcrumbs(), + requireUICapability: 'management.kibana.spaces', controller( $scope: any, $http: any, diff --git a/x-pack/plugins/spaces/public/views/management/spaces_grid/__snapshots__/spaces_grid_pages.test.tsx.snap b/x-pack/plugins/spaces/public/views/management/spaces_grid/__snapshots__/spaces_grid_pages.test.tsx.snap index 0766bc768b120f..cfe14ea07b0109 100644 --- a/x-pack/plugins/spaces/public/views/management/spaces_grid/__snapshots__/spaces_grid_pages.test.tsx.snap +++ b/x-pack/plugins/spaces/public/views/management/spaces_grid/__snapshots__/spaces_grid_pages.test.tsx.snap @@ -3,6 +3,7 @@ exports[`SpacesGridPage renders as expected 1`] = `
{ public render() { return ( -
+
{this.getPageContent()} {this.getConfirmDeleteModal()} diff --git a/x-pack/test/functional/apps/spaces/feature_controls/spaces_security.ts b/x-pack/test/functional/apps/spaces/feature_controls/spaces_security.ts new file mode 100644 index 00000000000000..7ac616ce08962b --- /dev/null +++ b/x-pack/test/functional/apps/spaces/feature_controls/spaces_security.ts @@ -0,0 +1,175 @@ +/* + * 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 { KibanaFunctionalTestDefaultProviders } from '../../../../types/providers'; + +// eslint-disable-next-line import/no-default-export +export default function({ getPageObjects, getService }: KibanaFunctionalTestDefaultProviders) { + const esArchiver = getService('esArchiver'); + const security = getService('security'); + const PageObjects = getPageObjects(['common', 'settings', 'security']); + const appsMenu = getService('appsMenu'); + const testSubjects = getService('testSubjects'); + + describe('security feature controls', () => { + before(async () => { + await esArchiver.load('empty_kibana'); + }); + + after(async () => { + await esArchiver.unload('empty_kibana'); + }); + + describe('global all base privilege', () => { + before(async () => { + await security.role.create('global_all_role', { + kibana: [ + { + base: ['all'], + spaces: ['*'], + }, + ], + }); + + await security.user.create('global_all_user', { + password: 'global_all_user-password', + roles: ['global_all_role'], + full_name: 'test user', + }); + + await PageObjects.security.logout(); + + await PageObjects.security.login('global_all_user', 'global_all_user-password', { + expectSpaceSelector: false, + }); + }); + + after(async () => { + await Promise.all([ + security.role.delete('global_all_role'), + security.user.delete('global_all_user'), + PageObjects.security.logout(), + ]); + }); + + it('shows management navlink', async () => { + const navLinks = (await appsMenu.readLinks()).map( + (link: Record) => link.text + ); + expect(navLinks).to.contain('Management'); + }); + + it(`displays Spaces management section`, async () => { + await PageObjects.settings.navigateTo(); + await testSubjects.existOrFail('spaces'); + }); + + it(`can navigate to spaces grid page`, async () => { + await PageObjects.common.navigateToActualUrl('kibana', 'management/spaces/list', { + ensureCurrentUrl: false, + shouldLoginIfPrompted: false, + }); + + await testSubjects.existOrFail('spaces-grid-page'); + }); + + it(`can navigate to create new space page`, async () => { + await PageObjects.common.navigateToActualUrl('kibana', 'management/spaces/create', { + ensureCurrentUrl: false, + shouldLoginIfPrompted: false, + }); + + await testSubjects.existOrFail('spaces-edit-page'); + }); + + it(`can navigate to edit space page`, async () => { + await PageObjects.common.navigateToActualUrl('kibana', 'management/spaces/edit/default', { + ensureCurrentUrl: false, + shouldLoginIfPrompted: false, + }); + + await testSubjects.existOrFail('spaces-edit-page'); + }); + }); + + describe('default space all base privilege', () => { + before(async () => { + await security.role.create('default_space_all_role', { + kibana: [ + { + base: ['all'], + spaces: ['default'], + }, + ], + }); + + await security.user.create('default_space_all_user', { + password: 'default_space_all_user-password', + roles: ['default_space_all_role'], + full_name: 'test user', + }); + + await PageObjects.security.logout(); + + await PageObjects.security.login( + 'default_space_all_user', + 'default_space_all_user-password', + { + expectSpaceSelector: false, + } + ); + }); + + after(async () => { + await Promise.all([ + security.role.delete('default_space_all_role'), + security.user.delete('default_space_all_user'), + PageObjects.security.logout(), + ]); + }); + + it('shows management navlink', async () => { + const navLinks = (await appsMenu.readLinks()).map( + (link: Record) => link.text + ); + expect(navLinks).to.contain('Management'); + }); + + it(`doesn't display Spaces management section`, async () => { + await PageObjects.settings.navigateTo(); + await testSubjects.existOrFail('objects'); // this ensures we've gotten to the management page + await testSubjects.missingOrFail('spaces'); + }); + + it(`can't navigate to spaces grid page`, async () => { + await PageObjects.common.navigateToActualUrl('kibana', 'management/spaces/list', { + ensureCurrentUrl: false, + shouldLoginIfPrompted: false, + }); + + await testSubjects.existOrFail('homeApp'); + }); + + it(`can't navigate to create new space page`, async () => { + await PageObjects.common.navigateToActualUrl('kibana', 'management/spaces/create', { + ensureCurrentUrl: false, + shouldLoginIfPrompted: false, + }); + + await testSubjects.existOrFail('homeApp'); + }); + + it(`can't navigate to edit space page`, async () => { + await PageObjects.common.navigateToActualUrl('kibana', 'management/spaces/edit/default', { + ensureCurrentUrl: false, + shouldLoginIfPrompted: false, + }); + + await testSubjects.existOrFail('homeApp'); + }); + }); + }); +} diff --git a/x-pack/test/functional/apps/spaces/index.ts b/x-pack/test/functional/apps/spaces/index.ts index b8b4e71b141fea..ccb7d28ae51b6b 100644 --- a/x-pack/test/functional/apps/spaces/index.ts +++ b/x-pack/test/functional/apps/spaces/index.ts @@ -10,6 +10,7 @@ export default function spacesApp({ loadTestFile }: KibanaFunctionalTestDefaultP describe('Spaces app', function spacesAppTestSuite() { this.tags('ciGroup4'); + loadTestFile(require.resolve('./feature_controls/spaces_security')); loadTestFile(require.resolve('./spaces_selection')); }); }