Allow applications to register feature privileges which are excluded from the base privileges by kobelb · Pull Request #41300 · elastic/kibana

kobelb · 2019-07-16T22:04:21Z

Resolves #38571

#40073 implements the non-test changes as well with ML utilizing the excludeFromBase option if you'd like to play around with the UI itself.

…m-base-privileges

Prior to these changes, if we gave the user global base all and then a custom set of privileges at a space granting access to the reservedAndNormal feature, it'd show "All" for the space specific privileges.

elasticmachine · 2019-07-16T23:18:46Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

…m-base-privileges

elasticmachine · 2019-07-17T00:22:56Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

legrego · 2019-07-25T11:36:04Z

Reviewing today

legrego

When a feature is excluded from base privileges, do you think we can/should add a tooltip explaining why that feature isn't included when granting base all/read? The feature definition (including the excludeFromBasePrivileges flag) is already available in the UI, so it "shouldn't be too hard"

legrego

Small UI Bug:

There is logic in place which only lets users create/update a space privilege if there is something to actually save. It seems this is getting triggered when updating an existing privilege when setting the excluded feature to "None":

Exclude any feature from the base privileges. I chose Saved Objects Management.
On the create role screen, add a space-specific privilege, setting Saved Objects Management to either "all" or "read"
Add the "Global base all" privilege
Edit the space privilege from step 2, and try to set Saved Objects Management to "None"

Result:
The "Update space privilege" button is disabled, preventing edits.

Expected:
I could see this going one of two ways. Either allow the edit to take place, or prevent "None" from being a selectable option in this scenario. Thoughts?

…m-base-privileges

kobelb · 2019-07-26T22:04:00Z

Result:
The "Update space privilege" button is disabled, preventing edits.

Expected:
I could see this going one of two ways. Either allow the edit to take place, or prevent "None" from being a selectable option in this scenario. Thoughts?

We currently have a similar experience without the changes this PR implements.

Add a global or space specific entry granting access to a single feature privilege
Edit the entry and select "None"

Since this is already an issue, I'd prefer we address this in a separate PR.

kobelb · 2019-07-29T19:16:32Z

@legrego I've added the tooltip that you suggested, ready for another review!

elasticmachine · 2019-07-29T20:17:32Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

legrego · 2019-07-30T11:02:14Z

...blic/views/management/edit_role/components/privileges/kibana/feature_table/feature_table.tsx

+            this.props.role.kibana[this.props.spacesIndex].base.length > 0;
+          const tooltipContent =
+            assignedBasePrivilege && actualPrivilegeValue === NO_PRIVILEGE_VALUE
+              ? `Use "Custom" to grant access. ${featureName} isn't part of the base privileges.`


We'll need to i18n'ify this string

Good catch, I'll make it so.

elasticmachine · 2019-07-30T15:16:33Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-07-30T17:17:46Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

legrego

LGTM once CI goes green against latest master

…-from-base-privileges

elasticmachine · 2019-07-30T18:34:37Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

…-from-base-privileges

elasticmachine · 2019-07-30T19:50:27Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

…from the base privileges (#41300) (#42300) * Allowing features with reserved and normal privileges to be listed * Allowing privileges to be excluded from the base privileges * Defaulting to custom when creating new privilege entries * Adjusting logic for the privilege space table and adding tests Prior to these changes, if we gave the user global base all and then a custom set of privileges at a space granting access to the reservedAndNormal feature, it'd show "All" for the space specific privileges. * Better naming for tests * Using a common rawKibanaPrivileges fixture * Rendering tooltip when feature isn't part of the base privileges * i18n'ing a string * Properly i18n'ing the string?

kobelb · 2019-07-30T22:00:05Z

/cc @elastic/ml-ui

…-or-edit-existing-rollup-job * 'master' of github.com:elastic/kibana: (114 commits) [ML] Fixing empty index pattern list (elastic#42299) [Markdown] Shim new platform - cleanup plugin (elastic#41760) [Code] Enable hierarchicalDocumentSymbolSupport for java language server (elastic#42233) Add New Platform mocks for data plugin (elastic#42261) Http server route handler implementation (elastic#41894) [SR] Allow custom index pattern to be used instead of selectable list when choosing indices to restore (elastic#41534) [Code] distributed Code abstraction (elastic#41374) [SIEM] Sets page titles to the current page you are on (elastic#42157) Saved Objects export API stable type order (elastic#42310) cancellation of interpreter execution (elastic#40238) [SIEM] Fixes a crash when Machine Learning influencers is an undefined value (elastic#42198) Changed the job to work with a dedicated index (elastic#42297) FTR: fix testSubjects.missingOrFail (elastic#42290) Increase retry timeout to prevent flaky tests (elastic#42291) Spaces - make space a hidden saved object type (elastic#41688) Allow applications to register feature privileges which are excluded from the base privileges (elastic#41300) Disable flaky log column reorder test (elastic#42285) Fixing add element in element reducer (elastic#42276) Fix infinite loop (elastic#42228) [Maps][File upload] Remove geojson deep clone logic, handle on maps side (elastic#41835) ...

kobelb added 5 commits July 12, 2019 13:15

Allowing privileges to be excluded from the base privileges

4212cde

Allowing features with reserved and normal privileges to be listed

b7c4496

Defaulting to custom when creating new privilege entries

2ecf7dc

Merge remote-tracking branch 'upstream/master' into authz/exclude-fro…

04c3f37

…m-base-privileges

Adjusting logic for the privilege space table and adding tests

68bc4b0

Prior to these changes, if we gave the user global base all and then a custom set of privileges at a space granting access to the reservedAndNormal feature, it'd show "All" for the space specific privileges.

kobelb added release_note:skip Skip the PR/issue when compiling release notes Feature:Security/Authorization Platform Security - Authorization labels Jul 16, 2019

kobelb added 2 commits July 16, 2019 15:15

Better naming for tests

2cbfeb6

Using a common rawKibanaPrivileges fixture

4ddd60c

Merge remote-tracking branch 'upstream/master' into authz/exclude-fro…

7cf90d2

…m-base-privileges

kobelb marked this pull request as ready for review July 18, 2019 14:19

kobelb requested a review from a team as a code owner July 18, 2019 14:19

kobelb requested a review from legrego July 18, 2019 14:21

legrego reviewed Jul 25, 2019

View reviewed changes

Merge remote-tracking branch 'upstream/master' into authz/exclude-fro…

77edd6f

…m-base-privileges

Rendering tooltip when feature isn't part of the base privileges

34df389

legrego reviewed Jul 30, 2019

View reviewed changes

i18n'ing a string

3dff387

Properly i18n'ing the string?

bcc147b

legrego approved these changes Jul 30, 2019

View reviewed changes

Merge branch 'master' of github.com:elastic/kibana into authz/exclude…

a9baff0

…-from-base-privileges

Merge branch 'master' of github.com:elastic/kibana into authz/exclude…

e9f4c23

…-from-base-privileges

kobelb merged commit d4a512f into elastic:master Jul 30, 2019

kobelb deleted the authz/exclude-from-base-privileges branch July 30, 2019 20:43

kobelb mentioned this pull request Jul 30, 2019

[7.x] Allow applications to register feature privileges which are excluded from the base privileges (#41300) #42300

Merged

Conversation

kobelb commented Jul 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Jul 16, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Jul 17, 2019

💚 Build Succeeded

Uh oh!

legrego commented Jul 25, 2019

Uh oh!

legrego left a comment

Choose a reason for hiding this comment

Uh oh!

legrego left a comment

Choose a reason for hiding this comment

Uh oh!

kobelb commented Jul 26, 2019

Uh oh!

kobelb commented Jul 29, 2019

Uh oh!

elasticmachine commented Jul 29, 2019

💚 Build Succeeded

Uh oh!

legrego Jul 30, 2019

Choose a reason for hiding this comment

Uh oh!

kobelb Jul 30, 2019

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Jul 30, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Jul 30, 2019

💚 Build Succeeded

Uh oh!

legrego left a comment

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Jul 30, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Jul 30, 2019

💚 Build Succeeded

Uh oh!

kobelb commented Jul 30, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kobelb commented Jul 16, 2019 •

edited

Loading