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

Feature Controls - Reserved Role Apps #30525

Merged
Merged
Show file tree
Hide file tree
Changes from 64 commits
Commits
Show all changes
75 commits
Select commit Hold shift + click to select a range
8ae173a
Removing feature privileges from ml/monitoring/apm
kobelb Feb 1, 2019
49b39ce
Adding monitoring/ml/apm as hard-coded global privileges
kobelb Feb 1, 2019
b36b1b2
A poorly named abstraction enters the room
kobelb Feb 2, 2019
34aee5a
No more wildcards, starting to move some stuff around
kobelb Feb 5, 2019
b210249
Splitting out the feature privilege builders
kobelb Feb 6, 2019
0d67f4d
Using actions instead of relying on their implementation
kobelb Feb 6, 2019
5e6e1ce
Merge remote-tracking branch 'upstream/granular-app-privileges' into …
kobelb Feb 6, 2019
6708214
We don't need the saved object types any longer
kobelb Feb 6, 2019
df12744
Explicitly specifying some actions that used to rely on wildcards
kobelb Feb 6, 2019
fb369ce
Fixing api integration test for privileges
kobelb Feb 6, 2019
951ceb3
Test fixture plugin which adds the globaltype now specifies a feature
kobelb Feb 6, 2019
92e7e0b
Unauthorized to find unknown types now
kobelb Feb 6, 2019
3828811
Merge branch 'fc/no-wildcards' into fc/reserved/role-apps-3
kobelb Feb 6, 2019
a78bfe7
Adding reserved privileges tests
kobelb Feb 6, 2019
392819b
Adding reserved privileges in a designated reserved bucket
kobelb Feb 6, 2019
c0477ae
Fixing ui capability tests
kobelb Feb 7, 2019
8649157
Adding spaces api tests for apm/ml/monitoring users
kobelb Feb 7, 2019
132e7f4
Adding more roles to the security only ui capability tests
kobelb Feb 7, 2019
2912e7b
You can put a role with reserved privileges using the API
kobelb Feb 7, 2019
dbe36f6
Adding support to get roles with _reserved privileges
kobelb Feb 7, 2019
2044ccd
Adding APM functional tests
kobelb Feb 8, 2019
1ef0d2c
Adding monitoring functional tests
kobelb Feb 8, 2019
46a023d
Merge branch 'granular-app-privileges' into fc/reserved-role-apps-3
kobelb Feb 8, 2019
7fe8e30
Fixing typo
kobelb Feb 8, 2019
03bb72c
Ensuring apm_user, monitoring_user alone don't authorize you
kobelb Feb 8, 2019
cb19823
Adding ml functional tests
kobelb Feb 8, 2019
d9e38b9
Fixing test
kobelb Feb 8, 2019
e467f1b
Fixing some type errors
kobelb Feb 8, 2019
d6031f3
Merge remote-tracking branch 'upstream/granular-app-privileges' into …
kobelb Feb 8, 2019
908a364
Updating snapshots
kobelb Feb 8, 2019
c1634bb
Fixing privileges tests
kobelb Feb 8, 2019
5396c6f
Trying to force this to run from source
kobelb Feb 8, 2019
13572f3
Merge remote-tracking branch 'upstream/granular-app-privileges' into …
kobelb Feb 11, 2019
b250672
Fixing TS errors
kobelb Feb 11, 2019
f3d8376
Being a less noisy neighbor
kobelb Feb 11, 2019
0693a06
Forcing logout for apm/dashboard feature controls security tests
kobelb Feb 11, 2019
71785e4
Fixing the security only ui capability tests
kobelb Feb 12, 2019
cd3b11a
Removing test that monitoring now tests itself
kobelb Feb 12, 2019
c6a5570
Merge remote-tracking branch 'upstream/granular-app-privileges' into …
kobelb Feb 15, 2019
b3a048e
Fixing some ui capability tests
kobelb Feb 15, 2019
6c28a1e
Cleaning up the error page services
kobelb Feb 15, 2019
db19c22
Fixing misspelling in comment
kobelb Feb 15, 2019
50b5ddd
Using forceLogout for monitoring
kobelb Feb 15, 2019
10bbc4e
Removing code that never should have been there, sorry Larry
kobelb Feb 15, 2019
f724694
Less leniency with the get roles
kobelb Feb 15, 2019
fa44f66
Barely alphabetical for a bit
kobelb Feb 15, 2019
1e4ca00
Apply suggestions from code review
legrego Feb 15, 2019
3a5033b
Removing errant timeout
kobelb Feb 15, 2019
03ee483
No more hard coded esFrom source
kobelb Feb 15, 2019
0671209
More nits
kobelb Feb 15, 2019
b71cd27
Adding back esFrom source
kobelb Feb 16, 2019
c833a41
Merge remote-tracking branch 'upstream/granular-app-privileges' into …
kobelb Mar 15, 2019
11ed79a
APM no longer uses reserved privileges, reserved privileges are
kobelb Mar 15, 2019
13f8831
Fixing typescript errors
kobelb Mar 18, 2019
3f75085
Fixing ui capability test themselves
kobelb Mar 18, 2019
031d1fa
Merge remote-tracking branch 'upstream/granular-app-privileges' into …
kobelb Mar 18, 2019
0b265c3
Displaying reserved privileges for the space aware and simple forms
kobelb Mar 19, 2019
736712f
Removing ability to PUT roles with _reserved privileges.
kobelb Mar 19, 2019
1a495fd
Updating jest snapshots
kobelb Mar 21, 2019
70a9ffc
Changing the interface for a feature to register a reserved privilege to
kobelb Mar 21, 2019
b7ac747
Displaying features with reserved privileges in the feature table
kobelb Mar 22, 2019
92efdb0
Merge remote-tracking branch 'upstream/granular-app-privileges' into …
kobelb Mar 22, 2019
7567c9a
Adjusting the reserved role privileges unit tests
kobelb Mar 22, 2019
8c124cb
Merge remote-tracking branch 'upstream/granular-app-privileges' into …
kobelb Mar 22, 2019
d98c6ca
Merge remote-tracking branch 'upstream/granular-app-privileges' into …
kobelb Mar 29, 2019
61ed5b1
Changing usages of expect.js to @kbn/expect
kobelb Mar 29, 2019
accf3e8
Changing the CalculatedPrivilege's _reserved property to reserved
kobelb Apr 1, 2019
6dbe5c2
Allowing reserved privileges to be assigned at kibana-*
kobelb Apr 1, 2019
5609563
Updating forgotten snapshot
kobelb Apr 1, 2019
1f3e709
Validating reserved privileges
kobelb Apr 2, 2019
a040191
Merge remote-tracking branch 'upstream/granular-app-privileges' into …
kobelb Apr 4, 2019
2c5ab8f
Updating imports
kobelb Apr 4, 2019
773a308
Merge remote-tracking branch 'upstream/granular-app-privileges' into …
kobelb Apr 9, 2019
951b4f5
Removing --esFrom flag, we don't need it anymore
kobelb Apr 9, 2019
5cdc103
Switching from tslint's ignore to eslint's ignore
kobelb Apr 9, 2019
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
5 changes: 5 additions & 0 deletions test/functional/page_objects/common_page.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
17 changes: 13 additions & 4 deletions test/functional/page_objects/error_page.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,22 @@
*/
import expect from 'expect.js';

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,
Expand Down
2 changes: 1 addition & 1 deletion test/scripts/jenkins_xpack_ci_group.sh
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,6 @@ tar -xzf "$linuxBuild" -C "$installDir" --strip=1

echo " -> Running functional and api tests"
cd "$XPACK_DIR"
node scripts/functional_tests --debug --bail --kibana-install-dir "$installDir" --include-tag "ciGroup$CI_GROUP"
node scripts/functional_tests --debug --bail --kibana-install-dir "$installDir" --include-tag "ciGroup$CI_GROUP" --esFrom "source"
Copy link
Member

Choose a reason for hiding this comment

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

nit: stray debug code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can probably remove this, I had CI not run from source during a run or two and forcing this in there made it work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is unfortunately required at the moment to get CI to run from source, I've talked to Tyler and we're trying to track down where this bug is.

echo ""
echo ""
15 changes: 7 additions & 8 deletions x-pack/plugins/ml/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
17 changes: 8 additions & 9 deletions x-pack/plugins/monitoring/init.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
kobelb marked this conversation as resolved.
Show resolved Hide resolved
return [];
}

return Object.keys(featurePrivileges);
}

public getActions(featureId: string, privilege: string): string[] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Up @@ -19,6 +19,7 @@ export interface RoleKibanaPrivilege {
spaces: string[];
base: string[];
feature: FeaturesPrivileges;
_reserved?: string[];
}

export interface Role {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Up @@ -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');
legrego marked this conversation as resolved.
Show resolved Hide resolved
}

const effectiveFeatureActions = this.getFeatureActions(
featureId,
effectiveFeaturePrivilegeExplanation.actualPrivilege
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@ export interface PrivilegeExplanation {
export interface CalculatedPrivilege {
base: PrivilegeExplanation;
feature: {
[featureId: string]: PrivilegeExplanation;
[featureId: string]: PrivilegeExplanation | undefined;
};
_reserved: undefined | string[];
kobelb marked this conversation as resolved.
Show resolved Hide resolved
}

export interface PrivilegeScenario {
Expand All @@ -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
Expand Up @@ -36,6 +36,7 @@ const defaultPrivilegeDefinition = new KibanaPrivileges({
all: ['somethingObsecure:/foo'],
},
},
reserved: {},
});

interface BuildRoleOpts {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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');
legrego marked this conversation as resolved.
Show resolved Hide resolved
}

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');
Copy link
Member

Choose a reason for hiding this comment

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

Same question here w/r/t throwing

}

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');
Copy link
Member

Choose a reason for hiding this comment

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

Same question here w/r/t throwing

}

return allowedFeaturePrivileges.canUnassign;
};

private onChangeAllFeaturePrivileges = (privilege: string) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const buildProps = (customProps = {}) => {
global: {},
space: {},
features: {},
reserved: {},
}),
intl: null as any,
uiCapabilities: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 'x-pack/plugins/xpack_main/types';
Expand All @@ -23,6 +23,7 @@ const buildProps = (customProps: any = {}) => {
},
global: {},
space: {},
reserved: {},
});

const role = {
Expand Down Expand Up @@ -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} />);
Expand Down
Loading