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

Add 'monitor_snapshot' cluster privilege #50210

Closed
peterpramb opened this issue Dec 15, 2019 · 9 comments · Fixed by #50489
Closed

Add 'monitor_snapshot' cluster privilege #50210

peterpramb opened this issue Dec 15, 2019 · 9 comments · Fixed by #50489
Labels
good first issue low hanging fruit :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC

Comments

@peterpramb
Copy link

Describe the feature:

Currently retrieving repository and snapshot metadata from Elasticsearch requires at least the create_snapshot privilege, which is too permissive for just monitoring the existing repositories and snaphots.

For read-only access to repositories and snapshots metadata (possibly via unauthenticated users) it would be preferable to allow more fine-grained control by adding an additional monitor_snapshot cluster privilege.

@markharwood markharwood added the :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC label Dec 16, 2019
@elasticmachine
Copy link
Collaborator

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

@albertzaharovits
Copy link
Contributor

Thank you for the proposal @peterpramb ! I think what you're suggesting makes sense.

@albertzaharovits albertzaharovits added the good first issue low hanging fruit label Dec 16, 2019
@peterpramb peterpramb changed the title Add cluster privilege monitor_snapshot Add monitor_snapshot' cluster privilege monitor_snapshot Dec 16, 2019
@peterpramb peterpramb changed the title Add monitor_snapshot' cluster privilege monitor_snapshot Add 'monitor_snapshot' cluster privilege Dec 16, 2019
@amirhmd
Copy link

amirhmd commented Dec 19, 2019

@albertzaharovits is it decided to apply the proposal? anybody working on this?

@albertzaharovits
Copy link
Contributor

Hi @amirhmd !
I assume you want to work on implementing the proposal and you're gauging our interest in accepting your contribution?

As far as I know there's nobody else working on it. I think adding a new cluster privilege, monitor_snapshot, in addition to the sole create_snapshot one, is a nice improvement and I am willing to review your work and get it merged in. I can't think of any reason other maintainers would reject it, IMO a privilege to only read snapshot and repositories info and statuses, without being able to create or delete snapshots or repositories, is consistent with how authorization is currently structured.

As an implementation hint, you can checkout the CREATE_SNAPSHOT_PATTERN in the ClusterPrivilegeResolver class and create a new pattern and the associated privilege for the SnapshotsStatusAction.NAME + "*", GetSnapshotsAction.NAME, SnapshotsStatusAction.NAME and GetRepositoriesAction.NAME actions. I expect you'll have to dig around a little bit to write the tests, the code in that area is a bit dated, but we're always glad to help.

@amirhmd
Copy link

amirhmd commented Dec 24, 2019

hi @albertzaharovits
it looks like @J-Bean already created a pull request.

@albertzaharovits
Copy link
Contributor

Hi @amirhmd . Thank you for your interest!

If you're still eager to contribute, there's another issue raised on the same theme, of creating a new privilege, this time an index type of privilege, see #29998 (comment) . We should wait for @tvernum 's answer on the issue and join the discussion there. If there's consensus on it, we can assign the issue to you to work on it at your own pace.

@amirhmd
Copy link

amirhmd commented Dec 29, 2019

Hi @albertzaharovits sounds good

@albertzaharovits
Copy link
Contributor

@amirhmd looks like you've got green light on the maintenance index privilege.

@amirhmd
Copy link

amirhmd commented Jan 2, 2020

@albertzaharovits received the message, I am on it I will ping you in the other thread if I have a question

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue low hanging fruit :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants