Skip to content

Commit

Permalink
Feature Controls - Reserved Role Apps (#30525)
Browse files Browse the repository at this point in the history
* Removing feature privileges from ml/monitoring/apm

* Adding monitoring/ml/apm as hard-coded global privileges

* A poorly named abstraction enters the room

* No more wildcards, starting to move some stuff around

* Splitting out the feature privilege builders

* Using actions instead of relying on their implementation

* We don't need the saved object types any longer

* Explicitly specifying some actions that used to rely on wildcards

* Fixing api integration test for privileges

* Test fixture plugin which adds the globaltype now specifies a feature

* Unauthorized to find unknown types now

* Adding reserved privileges tests

* Adding reserved privileges in a designated reserved bucket

* Fixing ui capability tests

* Adding spaces api tests for apm/ml/monitoring users

* Adding more roles to the security only ui capability tests

* You can put a role with reserved privileges using the API

* Adding support to get roles with _reserved privileges

* Adding APM functional tests

* Adding monitoring functional tests

* Fixing typo

* Ensuring apm_user, monitoring_user alone don't authorize you

* Adding ml functional tests

* Fixing test

* Fixing some type errors

* Updating snapshots

* Fixing privileges tests

* Trying to force this to run from source

* Fixing TS errors

* Being a less noisy neighbor

* Forcing logout for apm/dashboard feature controls security tests

* Fixing the security only ui capability tests

* Removing test that monitoring now tests itself

* Fixing some ui capability tests

* Cleaning up the error page services

* Fixing misspelling in comment

* Using forceLogout for monitoring

* Removing code that never should have been there, sorry Larry

* Less leniency with the get roles

* Barely alphabetical for a bit

* Apply suggestions from code review

Co-Authored-By: kobelb <brandon.kobel@gmail.com>

* Removing errant timeout

* No more hard coded esFrom source

* More nits

* Adding back esFrom source

* APM no longer uses reserved privileges, reserved privileges are
pluggable

* Fixing typescript errors

* Fixing ui capability test themselves

* Displaying reserved privileges for the space aware and simple forms

* Removing ability to PUT roles with _reserved privileges.
Removing ability to GET roles that have entries with both reserved and
feature/base privileges.

* Updating jest snapshots

* Changing the interface for a feature to register a reserved privilege to
include a description as well

* Displaying features with reserved privileges in the feature table

* Adjusting the reserved role privileges unit tests

* Changing usages of expect.js to @kbn/expect

* Changing the CalculatedPrivilege's _reserved property to reserved

* Allowing reserved privileges to be assigned at kibana-*

* Updating forgotten snapshot

* Validating reserved privileges

* Updating imports

* Removing --esFrom flag, we don't need it anymore

* Switching from tslint's ignore to eslint's ignore
  • Loading branch information
kobelb authored Apr 9, 2019
1 parent 2308242 commit 80dc0e7
Showing 71 changed files with 2,942 additions and 396 deletions.
5 changes: 5 additions & 0 deletions test/functional/page_objects/common_page.js
Original file line number Diff line number Diff line change
@@ -357,6 +357,11 @@ export function CommonPageProvider({ getService, getPageObjects }) {
}
}
}

async getBodyText() {
const el = await find.byCssSelector('body>pre');
return await el.getVisibleText();
}
}

return new CommonPage();
17 changes: 13 additions & 4 deletions test/functional/page_objects/error_page.js
Original file line number Diff line number Diff line change
@@ -18,13 +18,22 @@
*/
import expect from '@kbn/expect';

export function ErrorPageProvider({ getService }) {
const find = getService('find');
export function ErrorPageProvider({ getPageObjects }) {
const PageObjects = getPageObjects(['common']);

class ErrorPage {
async expectForbidden() {
const messageText = await PageObjects.common.getBodyText();
expect(messageText).to.eql(
JSON.stringify({
statusCode: 403,
error: 'Forbidden',
message: 'Forbidden'
})
);
}
async expectNotFound() {
const el = await find.byCssSelector('body>pre');
const messageText = await el.getVisibleText();
const messageText = await PageObjects.common.getBodyText();
expect(messageText).to.eql(
JSON.stringify({
statusCode: 404,
15 changes: 7 additions & 8 deletions x-pack/plugins/ml/index.js
Original file line number Diff line number Diff line change
@@ -88,20 +88,19 @@ export const ml = (kibana) => {
navLinkId: 'ml',
app: ['ml', 'kibana'],
catalogue: ['ml'],
privileges: {
all: {
catalogue: ['ml'],
grantWithBaseRead: true,
privileges: {},
reserved: {
privilege: {
savedObject: {
all: [],
read: ['config']
},
ui: [],
},
},
privilegesTooltip: i18n.translate('xpack.ml.privileges.tooltip', {
defaultMessage: 'To grant users access, you should also assign either the machine_learning_user or machine_learning_admin role.'
})
description: i18n.translate('xpack.ml.feature.reserved.description', {
defaultMessage: 'To grant users access, you should also assign either the machine_learning_user or machine_learning_admin role.'
})
}
});

// Add server routes and initialize the plugin here
Original file line number Diff line number Diff line change
@@ -406,7 +406,7 @@ export class JobsListView extends Component {
<JobStatsBar
jobsSummaryList={jobsSummaryList}
/>
<div className="job-management">
<div className="job-management" data-test-subj="ml-jobs-list">
<NodeAvailableWarning />
<UpgradeWarning />
<header>
17 changes: 8 additions & 9 deletions x-pack/plugins/monitoring/init.js
Original file line number Diff line number Diff line change
@@ -64,20 +64,19 @@ export const init = (monitoringPlugin, server) => {
navLinkId: 'monitoring',
app: ['monitoring', 'kibana'],
catalogue: ['monitoring'],
privileges: {
all: {
catalogue: ['monitoring'],
grantWithBaseRead: true,
privileges: {},
reserved: {
privilege: {
savedObject: {
all: [],
read: ['config'],
read: ['config']
},
ui: [],
},
},
privilegesTooltip: i18n.translate('xpack.monitoring.privileges.tooltip', {
defaultMessage: 'To grant users access, you should also assign the monitoring_user role.'
})
description: i18n.translate('xpack.monitoring.feature.reserved.description', {
defaultMessage: 'To grant users access, you should also assign the monitoring_user role.'
})
}
});

const bulkUploader = initBulkUploader(kbnServer, server);
2 changes: 2 additions & 0 deletions x-pack/plugins/security/common/constants.ts
Original file line number Diff line number Diff line change
@@ -7,3 +7,5 @@
export const GLOBAL_RESOURCE = '*';
export const IGNORED_TYPES = ['space'];
export const REALMS_ELIGIBLE_FOR_PASSWORD_CHANGE = ['reserved', 'native'];
export const APPLICATION_PREFIX = 'kibana-';
export const RESERVED_PRIVILEGES_APPLICATION_WILDCARD = 'kibana-*';
Original file line number Diff line number Diff line change
@@ -19,7 +19,12 @@ export class KibanaFeaturePrivileges {
}

public getPrivileges(featureId: string): string[] {
return Object.keys(this.featurePrivilegesMap[featureId]);
const featurePrivileges = this.featurePrivilegesMap[featureId];
if (featurePrivileges == null) {
return [];
}

return Object.keys(featurePrivileges);
}

public getActions(featureId: string, privilege: string): string[] {
Original file line number Diff line number Diff line change
@@ -14,4 +14,5 @@ export interface RawKibanaPrivileges {
global: Record<string, string[]>;
features: RawKibanaFeaturePrivileges;
space: Record<string, string[]>;
reserved: Record<string, string[]>;
}
1 change: 1 addition & 0 deletions x-pack/plugins/security/common/model/role.ts
Original file line number Diff line number Diff line change
@@ -19,6 +19,7 @@ export interface RoleKibanaPrivilege {
spaces: string[];
base: string[];
feature: FeaturesPrivileges;
_reserved?: string[];
}

export interface Role {
Original file line number Diff line number Diff line change
@@ -34,4 +34,5 @@ export const defaultPrivilegeDefinition = new KibanaPrivileges({
all: ['ui:/feature3/foo', 'ui:/feature3/foo/*'],
},
},
reserved: {},
});
Original file line number Diff line number Diff line change
@@ -93,6 +93,10 @@ export class KibanaAllowedPrivilegesCalculator {
candidateFeaturePrivileges: string[]
): { privileges: string[]; canUnassign: boolean } {
const effectiveFeaturePrivilegeExplanation = effectivePrivileges.feature[featureId];
if (effectiveFeaturePrivilegeExplanation == null) {
throw new Error('To calculate allowed feature privileges, we need the effective privileges');
}

const effectiveFeatureActions = this.getFeatureActions(
featureId,
effectiveFeaturePrivilegeExplanation.actualPrivilege
Original file line number Diff line number Diff line change
@@ -76,6 +76,7 @@ export class KibanaPrivilegeCalculator {
ignoreAssigned
),
feature: {},
reserved: privilegeSpec._reserved,
};

// If calculations wish to ignoreAssigned, then we still need to know what the real effective base privilege is
Original file line number Diff line number Diff line change
@@ -32,8 +32,9 @@ export interface PrivilegeExplanation {
export interface CalculatedPrivilege {
base: PrivilegeExplanation;
feature: {
[featureId: string]: PrivilegeExplanation;
[featureId: string]: PrivilegeExplanation | undefined;
};
reserved: undefined | string[];
}

export interface PrivilegeScenario {
@@ -50,9 +51,11 @@ export interface AllowedPrivilege {
canUnassign: boolean;
};
feature: {
[featureId: string]: {
privileges: string[];
canUnassign: boolean;
};
[featureId: string]:
| {
privileges: string[];
canUnassign: boolean;
}
| undefined;
};
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -36,6 +36,7 @@ const defaultPrivilegeDefinition = new KibanaPrivileges({
all: ['somethingObsecure:/foo'],
},
},
reserved: {},
});

interface BuildRoleOpts {
Original file line number Diff line number Diff line change
@@ -61,14 +61,32 @@ export class FeatureTable extends Component<Props, {}> {
public render() {
const { role, features, calculatedPrivileges, rankedFeaturePrivileges } = this.props;

const items: TableRow[] = features.map(feature => ({
feature: {
...feature,
hasAnyPrivilegeAssigned:
calculatedPrivileges.feature[feature.id].actualPrivilege !== NO_PRIVILEGE_VALUE,
},
role,
}));
const items: TableRow[] = features
.sort((feature1, feature2) => {
if (feature1.reserved && !feature2.reserved) {
return 1;
}

if (feature2.reserved && !feature1.reserved) {
return -1;
}

return 0;
})
.map(feature => {
const calculatedFeaturePrivileges = calculatedPrivileges.feature[feature.id];
const hasAnyPrivilegeAssigned = Boolean(
calculatedFeaturePrivileges &&
calculatedFeaturePrivileges.actualPrivilege !== NO_PRIVILEGE_VALUE
);
return {
feature: {
...feature,
hasAnyPrivilegeAssigned,
},
role,
};
});

// TODO: This simply grabs the available privileges from the first feature we encounter.
// As of now, features can have 'all' and 'read' as available privileges. Once that assumption breaks,
@@ -147,13 +165,17 @@ export class FeatureTable extends Component<Props, {}> {
</span>
),
render: (roleEntry: Role, record: TableRow) => {
const featureId = record.feature.id;
const { id: featureId, reserved } = record.feature;

if (reserved) {
return <EuiText size={'s'}>{reserved.description}</EuiText>;
}

const featurePrivileges = this.props.kibanaPrivileges
.getFeaturePrivileges()
.getPrivileges(featureId);

if (!featurePrivileges) {
if (featurePrivileges.length === 0) {
return null;
}

@@ -213,18 +235,32 @@ export class FeatureTable extends Component<Props, {}> {
return featurePrivileges;
}

return allowedPrivileges.feature[featureId].privileges;
const allowedFeaturePrivileges = allowedPrivileges.feature[featureId];
if (allowedFeaturePrivileges == null) {
throw new Error('Unable to get enabled feature privileges for a feature without privileges');
}

return allowedFeaturePrivileges.privileges;
};

private getPrivilegeExplanation = (featureId: string): PrivilegeExplanation => {
const { calculatedPrivileges } = this.props;
const calculatedFeaturePrivileges = calculatedPrivileges.feature[featureId];
if (calculatedFeaturePrivileges == null) {
throw new Error('Unable to get privilege explanation for a feature without privileges');
}

return calculatedPrivileges.feature[featureId];
return calculatedFeaturePrivileges;
};

private allowsNoneForPrivilegeAssignment = (featureId: string): boolean => {
const { allowedPrivileges } = this.props;
return allowedPrivileges.feature[featureId].canUnassign;
const allowedFeaturePrivileges = allowedPrivileges.feature[featureId];
if (allowedFeaturePrivileges == null) {
throw new Error('Unable to determine if none is allowed for a feature without privileges');
}

return allowedFeaturePrivileges.canUnassign;
};

private onChangeAllFeaturePrivileges = (privilege: string) => {
Original file line number Diff line number Diff line change
@@ -43,6 +43,7 @@ const buildProps = (customProps = {}) => {
global: {},
space: {},
features: {},
reserved: {},
}),
intl: null as any,
uiCapabilities: {
Original file line number Diff line number Diff line change
@@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/
// @ts-ignore
import { EuiButtonGroup, EuiButtonGroupProps, EuiSuperSelect } from '@elastic/eui';
import { EuiButtonGroup, EuiButtonGroupProps, EuiComboBox, EuiSuperSelect } from '@elastic/eui';
import React from 'react';
import { mountWithIntl, shallowWithIntl } from 'test_utils/enzyme_helpers';
import { Feature } from '../../../../../../../../../xpack_main/types';
@@ -23,6 +23,7 @@ const buildProps = (customProps: any = {}) => {
},
global: {},
space: {},
reserved: {},
});

const role = {
@@ -130,6 +131,29 @@ describe('<SimplePrivilegeForm>', () => {
expect(wrapper.find(UnsupportedSpacePrivilegesWarning)).toHaveLength(0);
});

it('displays the reserved privilege', () => {
const props = buildProps({
role: {
elasticsearch: {},
kibana: [
{
spaces: ['*'],
base: [],
feature: {},
_reserved: ['foo'],
},
],
},
});
const wrapper = shallowWithIntl(<SimplePrivilegeSection {...props} />);
const selector = wrapper.find(EuiComboBox);
expect(selector.props()).toMatchObject({
isDisabled: true,
selectedOptions: [{ label: 'foo' }],
});
expect(wrapper.find(UnsupportedSpacePrivilegesWarning)).toHaveLength(0);
});

it('fires its onChange callback when the privilege changes', () => {
const props = buildProps();
const wrapper = mountWithIntl(<SimplePrivilegeSection {...props} />);
Loading

0 comments on commit 80dc0e7

Please sign in to comment.