Skip to content

Grant necessary Kibana application privileges to reporting_user role #118058

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

Conversation

slobodanadamovic
Copy link
Contributor

@slobodanadamovic slobodanadamovic commented Dec 5, 2024

Previously, Kibana was authorizing (and granting application privileges) to create reports, simply based on the reporting_user role name. This PR makes these application privileges explicitly granted to the reporting_user role.

@slobodanadamovic slobodanadamovic added >enhancement :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC Team:Security Meta label for security team labels Dec 5, 2024
@slobodanadamovic slobodanadamovic self-assigned this Dec 5, 2024
@elasticsearchmachine
Copy link
Collaborator

Hi @slobodanadamovic, I've created a changelog YAML for you.

@slobodanadamovic slobodanadamovic marked this pull request as ready for review December 6, 2024 15:27
@slobodanadamovic slobodanadamovic requested a review from a team as a code owner December 6, 2024 15:27
@elasticsearchmachine
Copy link
Collaborator

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

);

final Set<String> allowedApplicationActionPatterns = Set.of(
"login:",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are only test examples of allowed actions and are not ment to represent the accurate list of actions.

@n1v0lg n1v0lg requested review from n1v0lg and jfreden and removed request for a team and n1v0lg December 10, 2024 11:51
"feature_visualize.minimal_read",
"feature_visualize.generate_report"
)
.build() },
null,
null,
MetadataUtils.getDeprecatedReservedMetadata("Please use Kibana feature privileges instead"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're adding this I assume this is no longer considered a deprecated role, so I think this can be removed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the description might be outdated? Since we're doing this we might want to update it.

Copy link
Contributor Author

@slobodanadamovic slobodanadamovic Dec 10, 2024

Choose a reason for hiding this comment

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

Since we're adding this I assume this is no longer considered a deprecated role, so I think this can be removed?

Good question!

@tsullivan Can correct me, but my understanding is that we still want to keep it deprecated, just to make sure it grants necessary application privileges. The preferred way should still be to create a custom role with the least app privileges.

Edit: I just saw this PR, which confirms that we should remove the deprecation warning (but would be nice to confirm):

assign the built-in reporting_user role the necessary Kibana application privileges, and make the role not marked as deprecated.

Copy link
Member

@tsullivan tsullivan Dec 10, 2024

Choose a reason for hiding this comment

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

I confirm that the Kibana team requests that we do remove the deprecated status of this role.

Removing the deprecated status will stop warning messages from being logged, which are not useful or meaningful to users.

Thank you very much!

Copy link
Member

Choose a reason for hiding this comment

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

BTW

The preferred way should still be to create a custom role with the least app privileges.

I think this is accurate, as granting the least app privileges should be recommended. Also worth mentioning, the documentation page linked here is in need of some updates for 8.x. It should offer more clarity about what the xpack.reporting.roles.enabled setting actually does.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed cbd0742 and 4217052, which remove deprecation warning and update docs and description of reporting_user role. Let me know if the updated description makes sense.

@slobodanadamovic
Copy link
Contributor Author

Looks like the description might be outdated? Since we're doing this we might want to update it.

Nice catch! Totally, we should update it.

Copy link
Contributor

@jfreden jfreden left a comment

Choose a reason for hiding this comment

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

👍 LGTM!

Copy link
Member

@tsullivan tsullivan left a comment

Choose a reason for hiding this comment

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

Request changes in the built-in documentation, because the reporting_user role does not grant direct access to the reporting indices

access to the <<roles-indices-priv,indices>> that will be used to generate reports.
Grants the necessary privileges required to use {reporting} features in {kib},
including generating and downloading reports. This role implicitly grants access
to the reporting indices, with each user having access only to their own reports.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
to the reporting indices, with each user having access only to their own reports.
to all Kibana reporting features, with each user having access only to their own reports.

+ "Reporting users should also be assigned additional roles that grant access to Kibana as well as read access "
+ "to the indices that will be used to generate reports."
"Grants the necessary privileges required to use reporting features in Kibana, "
+ "including generating and downloading reports. This role implicitly grants access to the reporting indices, "
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
+ "including generating and downloading reports. This role implicitly grants access to the reporting indices, "
+ "including generating and downloading reports. This role implicitly grants access to all Kibana reporting features, "

@slobodanadamovic slobodanadamovic added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Dec 13, 2024
@elasticsearchmachine elasticsearchmachine merged commit 6c56c32 into elastic:main Dec 13, 2024
21 checks passed
@slobodanadamovic slobodanadamovic deleted the sa-grant-kibana-app-privileges-to-reporting-user branch December 13, 2024 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) >enhancement :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC Team:Security Meta label for security team v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants