Add new predefined reserved roles #71710

jkakavas · 2021-04-14T20:47:07Z

This change adds two new reserved roles in Elasticsearch, viewer
and editor
Solutions will use these roles in order to provide,out of the box,
a role for full access to data and configuration with no access to
cluster management(editor) and a role for read only access to data
again with no access to cluster management (viewer).

This change adds two new reserved roles in Elasticsearch, viewer and editor Solutions will use these roles in order to provide,out of the box, a role for full access to data and configuration with no access to cluster management(editor) and a role for read only access to data again with no access to cluster management (viewer).

elasticmachine · 2021-04-14T20:47:11Z

Pinging @elastic/es-security (Team:Security)

jkakavas · 2021-04-14T20:47:27Z

cc @bytebilly

...src/test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java

legrego · 2021-04-15T12:03:34Z

...core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStore.java

+    }
+
+    private static RoleDescriptor buildEditorRoleDescriptor() {
+        return new RoleDescriptor("editor",


@bytebilly would we expect the editor role to be able to create/update/delete Kibana spaces? Or is this a task reserved for administrators?

If we need the editor to manage spaces, then this role would instead need to grant the all application privilege, and the reserved_ml_admin privilege (all does not automatically grant ML access until 8.0):

RoleDescriptor.ApplicationResourcePrivileges.builder() .application("kibana-.kibana") .resources("*").privileges("all") .build()

RoleDescriptor.ApplicationResourcePrivileges.builder() .application("kibana-*") .resources("*").privileges("reserved_ml_admin") .build()

@legrego I'm not sure to follow your comment. We are not using the all shortcut but rather assigning all individual permissions (happy to simplify if we can do differently).

Could you also explain why there is kibana-.kibana in the first row, and kibana-* in the second one? Thanks!

Sorry for not being clear -- If we want editors to be able to manage spaces, then we have to use the all shortcut in order to achieve this. We don't have a dedicated "manage spaces" privilege today (elastic/kibana#51759), and so it's currently gated behind this all privilege

Could you also explain why there is kibana-.kibana in the first row, and kibana-* in the second one? Thanks!

That is an implementation detail of Kibana's "reserved privileges", and is something we can potentially simplify in 8.x once Kibana drops support for multi-tenant setups. The -* at the end indicates that this privilege will be granted to all Kibana tenants, not just the default .kibana tenant.

Larry and I had a sync on that, and he is checking if using his proposed approach will give UX problems or not. Based on that, we can define if we want to use one approach or the other.

I didn't get a chance to verify this today. I struggled to get ES to build locally, and my time was quite limited today. I'm on PTO until Wednesday (after feature freeze), so please don't hold this PR up on me. We can tweak the application privileges definition following FF if we have to without any real interruption

jkakavas · 2021-04-19T14:13:12Z

ping @albertzaharovits

albertzaharovits · 2021-04-20T07:21:56Z

...core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStore.java

+            new RoleDescriptor.IndicesPrivileges[] {
+                // Stack
+                RoleDescriptor.IndicesPrivileges.builder()
+                    .indices("/~(([.]|ilm-history-).*)/")


I thought this is supposed to look like /@&~(([.]|ilm-history-).*)/, but tests say it works anyway.

albertzaharovits · 2021-04-20T07:29:00Z

...core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStore.java

+                RoleDescriptor.ApplicationResourcePrivileges.builder()
+                    .application("kibana-.kibana")
+                    .resources("*")
+                    .privileges("read").build() },


There are other indices that might be of interest such as: .monitoring-* and .ml-*, and maybe others (search for ". in ReservedRolesStore).

Only pointing it out, I don't have the full picture, but those two fit the PR description "full access to data and configuration" .

albertzaharovits

The java code is correct 😁

albertzaharovits · 2021-04-20T08:03:11Z

@bytebilly I worry about having all-encompassing hard-coded roles like this.
As soon as the user is required to think beyond "all users read/write all data" it's back to square 0, it has to learn about the low level details, ie the indices backing features used by solutions and the privileges required for them.
I think having viewer and editor per feature, eg apm-viewer, ml-editor would help with composability.

Also, since authorization is shared between Cloud/Kibana/ES, I think it would be easier for maintenance if there was such a feature that other stack components could provision roles without writing ES code, eg something like a REST role provider.

bytebilly · 2021-04-20T08:11:45Z

Thanks @albertzaharovits for the feedback, let me try to add more context on this iteration.

The fact that roles are "generic" is intentional, with the goal is to provide a good getting started experience that will let users to explore and share their deployments with others.
We have "per-solution" fine-grained roles (or better, building blocks) in scope for phase 2 (more here).

We decided to hardcode roles for the sake of time and complexity, but I agree we could consider something more structured. We discussed to use a YML file that is "ingested" at boot time, but also the API based way is an option. Happy to discuss more on a possible proposal.

I hope this clarify, let me know if you still have concerns!

jkakavas · 2021-04-20T08:33:24Z

I'll merge this since the implementation captures the decided decision for phase 1 and what this should look like for the next minor version.

This change adds two new reserved roles in Elasticsearch, viewer and editor Solutions will use these roles in order to provide,out of the box, a role for full access to data and configuration with no access to cluster management(editor) and a role for read only access to data again with no access to cluster management (viewer).

This comment is out of date since elastic#71710.

This comment is out of date since #71710.

This comment is out of date since #71710. Co-authored-by: Greg Back <1045796+gtback@users.noreply.github.com>

jkakavas added :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC v8.0.0 v7.13.0 labels Apr 14, 2021

jkakavas requested a review from albertzaharovits April 14, 2021 20:47

elasticmachine added the Team:Security Meta label for security team label Apr 14, 2021

jkakavas added 2 commits April 14, 2021 23:50

copypaste fix

bf212ec

adjust number of reserved roles in test

0c0b02f

ywangd reviewed Apr 15, 2021

View reviewed changes

...src/test/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStoreTests.java Show resolved Hide resolved

legrego reviewed Apr 15, 2021

View reviewed changes

jkakavas added 2 commits April 15, 2021 15:55

Add couple of checks

d68ea13

simplify role definition

f516d5a

albertzaharovits reviewed Apr 20, 2021

View reviewed changes

albertzaharovits approved these changes Apr 20, 2021

View reviewed changes

jkakavas merged commit 5a7b1c6 into elastic:master Apr 20, 2021

jkakavas mentioned this pull request Apr 20, 2021

[7.x] Add new predefined reserved roles (#71710) #71904

Merged

This was referenced Apr 22, 2021

Fix privileges for predefined roles #72061

Merged

[DOCS] Add documentation for new built-in roles #72146

Merged

gtback added a commit to gtback/elasticsearch that referenced this pull request May 17, 2021

Fix comment about number of roles

3e73f34

This comment is out of date since elastic#71710.

gtback mentioned this pull request May 17, 2021

Fix comment about number of roles #73163

Merged

jrodewig pushed a commit that referenced this pull request May 17, 2021

Fix comment about number of roles (#73163)

6f46ae4

This comment is out of date since #71710.

jrodewig added a commit that referenced this pull request May 17, 2021

Fix comment about number of roles (#73163) (#73166)

bdc6b06

This comment is out of date since #71710. Co-authored-by: Greg Back <1045796+gtback@users.noreply.github.com>

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new predefined reserved roles #71710

Add new predefined reserved roles #71710

jkakavas commented Apr 14, 2021

elasticmachine commented Apr 14, 2021

jkakavas commented Apr 14, 2021

legrego Apr 15, 2021

bytebilly Apr 15, 2021

legrego Apr 15, 2021 •

edited

Loading

legrego Apr 15, 2021

bytebilly Apr 15, 2021

legrego Apr 15, 2021

jkakavas commented Apr 19, 2021

albertzaharovits Apr 20, 2021

albertzaharovits Apr 20, 2021 •

edited

Loading

albertzaharovits left a comment

albertzaharovits commented Apr 20, 2021

bytebilly commented Apr 20, 2021

jkakavas commented Apr 20, 2021

Add new predefined reserved roles #71710

Add new predefined reserved roles #71710

Conversation

jkakavas commented Apr 14, 2021

elasticmachine commented Apr 14, 2021

jkakavas commented Apr 14, 2021

legrego Apr 15, 2021

Choose a reason for hiding this comment

bytebilly Apr 15, 2021

Choose a reason for hiding this comment

legrego Apr 15, 2021 • edited Loading

Choose a reason for hiding this comment

legrego Apr 15, 2021

Choose a reason for hiding this comment

bytebilly Apr 15, 2021

Choose a reason for hiding this comment

legrego Apr 15, 2021

Choose a reason for hiding this comment

jkakavas commented Apr 19, 2021

albertzaharovits Apr 20, 2021

Choose a reason for hiding this comment

albertzaharovits Apr 20, 2021 • edited Loading

Choose a reason for hiding this comment

albertzaharovits left a comment

Choose a reason for hiding this comment

albertzaharovits commented Apr 20, 2021

bytebilly commented Apr 20, 2021

jkakavas commented Apr 20, 2021

legrego Apr 15, 2021 •

edited

Loading

albertzaharovits Apr 20, 2021 •

edited

Loading