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

Deprecating kibana_user and kibana_dashboard_only_user roles #46456

Merged
merged 20 commits into from
Jan 14, 2020

Conversation

legrego
Copy link
Member

@legrego legrego commented Sep 6, 2019

This is a proposal for the Elasticsearch portion of elastic/kibana#25722:

  • Introduce new kibana_admin reserved role, with the same privileges as the current kibana_user role.
  • "Deprecate" the kibana_user and kibana_dashboard_only_user roles by introducing a new _deprecated metadata flag on each of these roles.

On its own, the new _deprecated metadata flag does nothing. The intent is for the Kibana UI to read this flag and inform the user that the role is deprecated.

I could see benefit in adding a deprecation response header when a user with a deprecated role authenticates to ES, but I'm not sure where to begin with a change like that, or if it'd even be a welcome change. Open to suggestions there, but we could even do that in a followup PR.

Companion Kibana PR with proposed UI changes: elastic/kibana#45045

@legrego
Copy link
Member Author

legrego commented Sep 6, 2019

run elasticsearch-ci/1

@legrego legrego marked this pull request as ready for review September 9, 2019 11:40
@legrego legrego added the WIP label Sep 9, 2019
@legrego
Copy link
Member Author

legrego commented Sep 9, 2019

@elastic/kibana-security @elastic/es-security - I'm interested in getting your feedback on this. I took a fairly easy/straightforward approach, but I'm open to other ideas.

@kobelb
Copy link
Contributor

kobelb commented Sep 12, 2019

This seems reasonable to me, conceptually. I'll defer to the ES team on the actual code changes.

@legrego legrego requested a review from a team September 13, 2019 16:32
@legrego legrego added :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC and removed WIP labels Sep 13, 2019
@legrego
Copy link
Member Author

legrego commented Sep 24, 2019

@elasticmachine update branch

@@ -117,7 +117,7 @@ If {security-features} are enabled, you must provide a valid user ID and
password so that {filebeat} can connect to {kib}:

.. Create a user on the monitoring cluster that has the
{stack-ov}/built-in-roles.html[`kibana_user` built-in role] or equivalent
{stack-ov}/built-in-roles.html[`kibana_admin` built-in role] or equivalent
Copy link
Member

Choose a reason for hiding this comment

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

Does filebeat need to connect as kibana_admin or would a less privileged role suffice ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@legrego What's our recommendation here? What privileges does filebeat need in order to use the Kibana API to setup dashboards etc?

to any user who authenticates against the `oidc1` OpenID Connect realm:

[source,console]
--------------------------------------------------
PUT /_security/role_mapping/oidc-kibana
{
"roles": [ "kibana_user" ],
"roles": [ "kibana_admin" ],
Copy link
Member

Choose a reason for hiding this comment

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

I'd be wary of suggesting to add kibana_admin to all users of the realm as consumers of the docs tend to follow our examples verbatim

to any user who authenticates against the `saml1` realm:

[source,console]
--------------------------------------------------
PUT /_security/role_mapping/saml-kibana
{
"roles": [ "kibana_user" ],
"roles": [ "kibana_admin" ],
Copy link
Member

Choose a reason for hiding this comment

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

I'd be wary of suggesting to add kibana_admin to all users of the realm as consumers of the docs tend to follow our examples verbatim

@kobelb
Copy link
Contributor

kobelb commented Sep 25, 2019

When chatting with @tvernum, he mentioned the potential for something to be implemented in Elasticsearch to allow roles to be transparently renamed and the old ones deprecated. Tim will be leaving for holiday tomorrow, so it might be worth-while for us to temporarily put this on hold until he gets a chance to weigh-in.

@tvernum tvernum self-assigned this Dec 4, 2019
@tvernum
Copy link
Contributor

tvernum commented Dec 16, 2019

@legrego I've picked this up and will address @jkakavas's comments.

I've updated your PR with 2 additions:

  1. We now write to the deprecation log when a deprecated role is used (not retreived via the API, but loaded by a user logging in).
  2. To support that, I added a _deprecation_reason to the role metadata.

I'm considering adding additional deprecation warnings when assigning these roles to users (in PUT /_security/user/xyz or PUT /_security/role_mapping/xyz)

Can you check the deprecation reasons I added to the 2 Kibana roles - are you happy with them? It's up to you whether you want to include them in the UI.

@tvernum
Copy link
Contributor

tvernum commented Jan 13, 2020

@legrego Are you happy for us to merge this & officially deprecate this role in 7.6? Or wait for 7.7?

@legrego
Copy link
Member Author

legrego commented Jan 13, 2020

Are you happy for us to merge this & officially deprecate this role in 7.6? Or wait for 7.7?

I'd like @kobelb to weigh in here too... but I think we can officially deprecate on the ES side for 7.6 as long as we're comfortable with the corresponding Kibana UI changes not being ready in time. They're still in a draft state pending a design review, so I don't expect to make feature freeze there.

We have doc updates to make on our side as well, but we can hit that for 7.6, especially since they don't have to be done in time for feature freeze.

@kobelb
Copy link
Contributor

kobelb commented Jan 13, 2020

We have doc updates to make on our side as well, but we can hit that for 7.6, especially since they don't have to be done in time for feature freeze.

As long as we can get the doc updates in for 7.6, I'm alright with us merging the Elasticsearch changes.

{kibana-ref}/kibana-privileges.html#kibana-feature-privileges[{kib} feature privileges]
instead).
Grants read-only access to the {kib} Dashboard in every
{kibana-ref}/xpack-spaces.html[Space in {kib}].
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{kibana-ref}/xpack-spaces.html[Space in {kib}].
{kibana-ref}/xpack-spaces.html[space in {kib}].

@@ -87,8 +89,14 @@ see {kibana-ref}/using-kibana-with-security.html[Configuring Security in {kib}].
NOTE: This role should not be assigned to users as the granted permissions may
change between releases.

[[built-in-roles-kibana-admin]] `kibana_admin`::
Grants access to all features in {kib}. For more information on {kib} authorization,
see {kibana-ref}/xpack-security-authorization.html[Kibana Authorization].
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
see {kibana-ref}/xpack-security-authorization.html[Kibana Authorization].
see {kibana-ref}/xpack-security-authorization.html[Kibana authorization].

Grants access to all features in {kib}. For more information on Kibana authorization,
(This role is deprecated, please use the
<<built-in-roles-kibana-admin,`kibana_admin`>> role instead.)
Grants access to all features in {kib}. For more information on {kib} authorization,
see {kibana-ref}/xpack-security-authorization.html[Kibana Authorization].
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
see {kibana-ref}/xpack-security-authorization.html[Kibana Authorization].
see {kibana-ref}/xpack-security-authorization.html[Kibana authorization].

@@ -127,7 +135,8 @@ Grants the minimum privileges required for any user of {monitoring} other than t
required to use {kib}. This role grants access to the monitoring indices and grants
privileges necessary for reading basic cluster information. This role also includes
all {kibana-ref}/kibana-privileges.html[Kibana privileges] for the {stack-monitor-features}.
Monitoring users should also be assigned the `kibana_user` role.
Monitoring users should also be assigned the `kibana_admin` role, or another role
with {kibana-ref}/xpack-security-authorization.html[access to the {kib} instance]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
with {kibana-ref}/xpack-security-authorization.html[access to the {kib} instance]
with {kibana-ref}/xpack-security-authorization.html[access to the {kib} instance].

that will be used to generate reports.
user has access to only their own reports.
Reporting users should also be assigned additional roles that grant
{kibana-ref}/xpack-security-authorization.html[Access to {kib}] as well as read
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{kibana-ref}/xpack-security-authorization.html[Access to {kib}] as well as read
{kibana-ref}/xpack-security-authorization.html[access to {kib}] as well as read

. Assign your {kib} users the `kibana_user` role and your `logstash_reader`
role.
. Assign your {kib} users a role that grants
{kibana-ref}/xpack-security-authorization.html[Access to {kib}]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{kibana-ref}/xpack-security-authorization.html[Access to {kib}]
{kibana-ref}/xpack-security-authorization.html[access to {kib}]

access to Kibana see {kibana-ref}/xpack-security-authorization.html[Kibana Authorization].
This user now has administrative access to all features in {kib}.
For more information about granting access to Kibana see
{kibana-ref}/xpack-security-authorization.html[Kibana Authorization].
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{kibana-ref}/xpack-security-authorization.html[Kibana Authorization].
{kibana-ref}/xpack-security-authorization.html[Kibana authorization].

Copy link
Contributor

@lcawl lcawl left a comment

Choose a reason for hiding this comment

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

Added minor suggestions; otherwise docs LGTM

@tvernum tvernum merged commit fa4869a into elastic:master Jan 14, 2020
tvernum added a commit to tvernum/elasticsearch that referenced this pull request Jan 14, 2020
This change adds a new `kibana_admin` role, and deprecates
the old `kibana_user` and`kibana_dashboard_only_user`roles.

The deprecation is implemented via a new reserved metadata
attribute, which can be consumed from the API and also triggers
deprecation logging when used (by a user authenticating to
Elasticsearch).

Some docs have been updated to avoid references to these
deprecated roles.

Backport of: elastic#46456

Co-authored-by: Tim Vernum <tim@adjective.org>
Co-authored-by: Larry Gregory <legrego@users.noreply.github.com>
tvernum added a commit that referenced this pull request Jan 15, 2020
This change adds a new `kibana_admin` role, and deprecates
the old `kibana_user` and`kibana_dashboard_only_user`roles.

The deprecation is implemented via a new reserved metadata
attribute, which can be consumed from the API and also triggers
deprecation logging when used (by a user authenticating to
Elasticsearch).

Some docs have been updated to avoid references to these
deprecated roles.

Backport of: #46456

Co-authored-by: Larry Gregory <lgregorydev@gmail.com>
SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this pull request Jan 23, 2020
…#46456)

This change adds a new `kibana_admin` role, and deprecates
the old `kibana_user` and`kibana_dashboard_only_user`roles.

The deprecation is implemented via a new reserved metadata
attribute, which can be consumed from the API and also triggers
deprecation logging when used (by a user authenticating to
Elasticsearch).

Some docs have been updated to avoid references to these
deprecated roles.

Co-authored-by: Tim Vernum <tim@adjective.org>
Co-authored-by: Larry Gregory <legrego@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants