Add remote index privileges to role management #154948
Conversation
thomheymann
changed the title
thomheymann
added
v8.8.0
release_note:enhancement
Team:Security
|
Pinging @elastic/kibana-security (Team:Security) |
x-pack/plugins/security/public/management/roles/edit_role/edit_role_page.tsx
Show resolved
Hide resolved
| id="xpack.security.management.editRole.elasticSearchPrivileges.controlAccessToRemoteClusterDataDescription" | ||
| defaultMessage="Control access to the data in remote clusters. " | ||
| /> | ||
| {this.learnMore(docLinks.links.security.indicesPrivileges)} |
There was a problem hiding this comment.
Would we want a more specific doc link in the near future? I don't think more appropriate page exists at the moment, but a page that links to both the remote cluster config page and the index privileges page could be an option.
There was a problem hiding this comment.
Good catch, the docs for this haven't been written yet unfortunately but totally agree that this should link to a dedicated page for remote index privileges.
| </KibanaContextProvider> | ||
| ); | ||
| await flushPromises(); | ||
| expect(wrapper.find(IndexPrivilegeForm)).toHaveLength(1); |
There was a problem hiding this comment.
Really minor nit: as we're testing that we the render a component for each index privilege, should we use a non-1 test case? I realize this is how it has been (probably) forever, so feel free to ignore.
There was a problem hiding this comment.
Good idea, I'll update the tests
| @@ -34,11 +34,12 @@ const toOption = (value: string) => ({ label: value }); | |||
|
|
|||
| interface Props { | |||
| formIndex: number; | |||
| indexPrivilege: RoleIndexPrivilege; | |||
| indexType: 'indices' | 'remote_indices'; | |||
| indexPrivilege: RoleIndexPrivilege | RoleRemoteIndexPrivilege; | |||
| indexPatterns: string[]; | |||
There was a problem hiding this comment.
Should we ignore the incoming index patterns prop if the indexType is 'remote_indices'? Or will this be superseded by future work where we can populate the actual indices?
There was a problem hiding this comment.
I think it should be down to the consumer of this component to decide whether they want to pass in this props or not (it's optional). For remote indices we currently don't want to auto-complete local indices so I am not passing the prop in. We might want to auto complete remote indices in the future though so I wouldn't restrict it in the component.
| }); | ||
| }); | ||
|
|
||
| test('it requires privileges when an index is defined', () => { |
There was a problem hiding this comment.
Nit: Could add a test for "It requires indices and privileges when clusters is defined"
💚 Build Succeeded
Metrics [docs]Async chunks
Public APIs missing exports
Page load bundle
Unknown metric groupsAPI count
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
Resolves #155289 ~~Todo: Add link to ES remote indices docs once available (#154948 (comment) Update: I don't think there's a better page to link to right now so will leave as is. We can always change it in the future.
Resolves #142399
Summary
Add remote index privileges section to role management screen.
Release note
Added ability to configure remote index privileges directly from within the role management screen.
Screenshots
Platinum license
Basic license