Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enables the feature catalogue registry to be controlled via uiCapabil… #27945

Merged
merged 7 commits into from
Jan 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/legacy/core_plugins/kibana/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,15 @@ export default function (kibana) {
dashboard: {
showWriteControls: true
},
catalogue: {
discover: true,
dashboard: true,
visualize: true,
console: true,
advanced_settings: true,
saved_objects: true,
index_patterns: true,
},
management: {
/*
* Management settings correspond to management section/link ids, and should not be changed
Expand Down
1 change: 1 addition & 0 deletions src/ui/public/capabilities/ui_capabilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export interface UICapabilities {
management: {
[sectionId: string]: Record<string, boolean>;
};
catalogue: Record<string, boolean>;
[key: string]: Record<string, boolean | Record<string, boolean>>;
}

Expand Down
6 changes: 5 additions & 1 deletion src/ui/public/registry/feature_catalogue.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,17 @@
*/

import { uiRegistry } from './_registry';
import { uiCapabilities } from '../capabilities';

export const FeatureCatalogueRegistryProvider = uiRegistry({
name: 'featureCatalogue',
index: ['id'],
group: ['category'],
order: ['title'],
filter: featureCatalogItem => Object.keys(featureCatalogItem).length > 0
filter: featureCatalogItem => {
const isDisabledViaCapabilities = uiCapabilities.catalogue[featureCatalogItem.id] === false;
return !isDisabledViaCapabilities && Object.keys(featureCatalogItem).length > 0;
}
});

export const FeatureCatalogueCategory = {
Expand Down
102 changes: 102 additions & 0 deletions src/ui/public/registry/feature_catalogue.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/*
* 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.
*/
jest.mock('ui/chrome', () => ({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥇

getInjected: key => {
if (key === 'uiCapabilities') {
return {
navLinks: {},
management: {},
catalogue: {
item1: true,
item2: false,
item3: true,
},
};
}
throw new Error(`Unexpected call to chrome.getInjected with key ${key}`);
},
}));
import { FeatureCatalogueCategory, FeatureCatalogueRegistryProvider } from './feature_catalogue';

describe('FeatureCatalogueRegistryProvider', () => {

beforeAll(() => {
FeatureCatalogueRegistryProvider.register(() => {
return {
id: 'item1',
title: 'foo',
description: 'this is foo',
icon: 'savedObjectsApp',
path: '/app/kibana#/management/kibana/objects',
showOnHomePage: true,
category: FeatureCatalogueCategory.ADMIN,
};
});

FeatureCatalogueRegistryProvider.register(() => {
return {
id: 'item2',
title: 'bar',
description: 'this is bar',
icon: 'savedObjectsApp',
path: '/app/kibana#/management/kibana/objects',
showOnHomePage: true,
category: FeatureCatalogueCategory.ADMIN,
};
});

// intentionally not listed in uiCapabilities.catalogue above
FeatureCatalogueRegistryProvider.register(() => {
return {
id: 'item4',
title: 'secret',
description: 'this is a secret',
icon: 'savedObjectsApp',
path: '/app/kibana#/management/kibana/objects',
showOnHomePage: true,
category: FeatureCatalogueCategory.ADMIN,
};
});
});

it('should not return items hidden by uiCapabilities', () => {
const mockPrivate = entityFn => entityFn();
const mockInjector = () => null;

// eslint-disable-next-line new-cap
const foo = FeatureCatalogueRegistryProvider(mockPrivate, mockInjector).inTitleOrder;
expect(foo).toEqual([{
id: 'item1',
title: 'foo',
description: 'this is foo',
icon: 'savedObjectsApp',
path: '/app/kibana#/management/kibana/objects',
showOnHomePage: true,
category: FeatureCatalogueCategory.ADMIN,
}, {
id: 'item4',
title: 'secret',
description: 'this is a secret',
icon: 'savedObjectsApp',
path: '/app/kibana#/management/kibana/objects',
showOnHomePage: true,
category: FeatureCatalogueCategory.ADMIN,
}]);
});
});
1 change: 1 addition & 0 deletions x-pack/plugins/__mocks__/ui/capabilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { UICapabilities } from 'ui/capabilities';
let internals: UICapabilities = {
navLinks: {},
management: {},
catalogue: {},
spaces: {
manage: true,
},
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/apm/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ export function apm(kibana) {
navLinkId: 'apm',
privileges: {
all: {
catalogue: ['apm'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this do?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This tells the feature controls (the project formerly known as Granular Application Privileges) when to remove the "APM" listing from the Kibana homepage.

If the current user doesn't have access** to APM, then we remove the APM listing from the feature catalogue registry, which is what controls the home page display.

** determining access is rather tricky for an app like APM, which relies on cluster/index privileges. Here, "access" means that the application is enabled in the user's current space, and that the user's roles haven't disabled the application from appearing with Kibana. This is all still WIP, but that's the idea.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. Thanks for enlightening me :)

app: ['apm'],
savedObject: {
all: [],
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/canvas/init.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export default async function(server /*options*/) {
navLinkId: 'canvas',
privileges: {
all: {
catalogue: ['canvas'],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

catalogue instead of catalog, eh? How very Euro of you!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to be consistent with our OG implementation 😄

app: ['canvas'],
savedObject: {
all: ['canvas'],
Expand All @@ -50,6 +51,7 @@ export default async function(server /*options*/) {
ui: [],
},
read: {
catalogue: ['canvas'],
app: ['canvas'],
savedObject: {
all: [],
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/graph/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export function graph(kibana) {
navLinkId: 'graph',
privileges: {
all: {
catalogue: ['graph'],
app: ['graph'],
savedObject: {
all: ['graph-workspace'],
Expand All @@ -65,6 +66,7 @@ export function graph(kibana) {
ui: [],
},
read: {
catalogue: ['graph'],
app: ['graph'],
savedObject: {
all: [],
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/infra/server/kibana.index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export const initServerWithKibana = (kbnServer: KbnServer) => {
navLinkId: 'infra:home',
privileges: {
all: {
catalogue: ['infraops'],
app: ['infra'],
savedObject: {
all: [],
Expand All @@ -54,6 +55,7 @@ export const initServerWithKibana = (kbnServer: KbnServer) => {
navLinkId: 'infra:logs',
privileges: {
all: {
catalogue: ['infralogging'],
app: ['infra'],
savedObject: {
all: [],
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/ml/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ export const ml = (kibana) => {
defaultMessage: 'The machine_learning_user or machine_learning_admin role should be assigned to grant access'
})
},
catalogue: ['ml'],
app: ['ml'],
savedObject: {
all: [],
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/monitoring/init.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ export const init = (monitoringPlugin, server) => {
defaultMessage: 'The monitoring_user role should be assigned to grant access'
})
},
catalogue: ['monitoring'],
app: ['monitoring'],
savedObject: {
all: [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ exports[`<KibanaPrivileges> renders without crashing 1`] = `
}
uiCapabilities={
Object {
"catalogue": Object {},
"management": Object {},
"navLinks": Object {},
"spaces": Object {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const buildProps = (customProps = {}) => {
uiCapabilities: {
navLinks: {},
management: {},
catalogue: {},
spaces: {
manage: true,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ describe('#allNavlinks', () => {
});
});

describe('#allCatalogueEntries', () => {
test('returns ui:catalogue/*', () => {
const uiActions = new UIActions();
expect(uiActions.allCatalogueEntries).toBe('ui:catalogue/*');
});
});

describe('#get', () => {
[null, undefined, '', 1, true, {}].forEach((featureId: any) => {
test(`featureId of ${JSON.stringify(featureId)} throws error`, () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const prefix = 'ui:';
export class UIActions {
public all = `${prefix}*`;
public allNavLinks = `${prefix}navLinks/*`;
public allCatalogueEntries = `${prefix}catalogue/*`;
public allManagementLinks = `${prefix}management/*`;

public get(featureId: keyof UICapabilities, ...uiCapabilityParts: string[]) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ describe('usingPrivileges', () => {
indices: true,
},
},
catalogue: {},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of these test are using an empty catalogue entry, and disable_ui_capabilities is treating it like any other ui capability. We have logic in disable_ui_capabilities to only disable "navLinks" that are registered for a feature, on the surface it seems like we'd want similar logic for "catalogue" entries.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

navLinks is special because we know about every nav link that Kibana has, even if that nav link isn't registered for a feature.

Catalogue entries, much like management entries, are registered client-side, so the Kibana server doesn't have a complete list of them. The only catalogue entries we deal with are the ones explicitly registered via features.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, thanks for the clarification.

fooFeature: {
foo: true,
bar: true,
Expand All @@ -109,6 +110,7 @@ describe('usingPrivileges', () => {
indices: false,
},
},
catalogue: {},
fooFeature: {
foo: false,
bar: false,
Expand Down Expand Up @@ -169,6 +171,7 @@ describe('usingPrivileges', () => {
indices: true,
},
},
catalogue: {},
fooFeature: {
foo: true,
bar: true,
Expand All @@ -190,6 +193,7 @@ describe('usingPrivileges', () => {
indices: false,
},
},
catalogue: {},
fooFeature: {
foo: false,
bar: false,
Expand Down Expand Up @@ -239,6 +243,7 @@ describe('usingPrivileges', () => {
indices: true,
},
},
catalogue: {},
})
).rejects.toThrowErrorMatchingSnapshot();
expect(mockServer.log).not.toHaveBeenCalled();
Expand Down Expand Up @@ -291,6 +296,7 @@ describe('usingPrivileges', () => {
settings: false,
},
},
catalogue: {},
fooFeature: {
foo: true,
bar: true,
Expand All @@ -314,6 +320,7 @@ describe('usingPrivileges', () => {
settings: false,
},
},
catalogue: {},
fooFeature: {
foo: true,
bar: false,
Expand Down Expand Up @@ -367,6 +374,7 @@ describe('usingPrivileges', () => {
indices: false,
},
},
catalogue: {},
fooFeature: {
foo: false,
bar: false,
Expand All @@ -388,6 +396,7 @@ describe('usingPrivileges', () => {
indices: false,
},
},
catalogue: {},
fooFeature: {
foo: false,
bar: false,
Expand Down Expand Up @@ -427,6 +436,7 @@ describe('all', () => {
indices: true,
},
},
catalogue: {},
fooFeature: {
foo: true,
bar: true,
Expand All @@ -447,6 +457,7 @@ describe('all', () => {
indices: false,
},
},
catalogue: {},
fooFeature: {
foo: false,
bar: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,19 @@ export class FeaturesPrivilegesBuilder {
);
}

public getCatalogueReadActions(features: Feature[]): string[] {
return flatten(
features.map(feature => {
const { privileges } = feature;
if (!privileges || !privileges.read || !privileges.read.catalogue) {
return [];
}

return this.buildCatalogueFeaturePrivileges(privileges.read);
})
);
}

public getManagementReadActions(features: Feature[]): string[] {
return flatten(
features.map(feature => {
Expand Down Expand Up @@ -86,10 +99,23 @@ export class FeaturesPrivilegesBuilder {
),
...privilegeDefinition.ui.map(ui => this.actions.ui.get(feature.id, ui)),
...(feature.navLinkId ? [this.actions.ui.get('navLinks', feature.navLinkId)] : []),
...this.buildCatalogueFeaturePrivileges(privilegeDefinition),
...this.buildManagementFeaturePrivileges(privilegeDefinition),
]);
}

private buildCatalogueFeaturePrivileges(
privilegeDefinition: FeaturePrivilegeDefinition
): string[] {
if (!privilegeDefinition.catalogue) {
return [];
}

return privilegeDefinition.catalogue.map(catalogueEntryId =>
this.actions.ui.get('catalogue', catalogueEntryId)
);
}

private buildManagementFeaturePrivileges(
privilegeDefinition: FeaturePrivilegeDefinition
): string[] {
Expand Down
Loading