-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Create snapshot role #35820
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
Create snapshot role #35820
Conversation
|
Pinging @elastic/es-security |
tvernum
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This really needs more tests.
- A unit test for the privilege, that it grants the right actions.
- I think we need to have a proper IntegTest too. We've been bitten so many times by core features not quite working with security, that we need a proper test of "can I snapshot if I have the
create_snapshot" privilege.
Hopefully that's just a matter of running the existing snapshot tests with a minimally-privileged role (just the filesystem based ones should be fine).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think "create_snapshot(s)" is clearer.
"snapshot" leaves open the question about whether you can restore/delete and if someone was auditing that a role did what they expected, they would need to check the docs to be sure what snapshot actually grants.
17fd2f1 to
79b81f8
Compare
|
@tvernum I have addressed your review comments. Thanks! The original PR was indeed subpar. |
tvernum
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+:100:
|
https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+pull-request-1/5989/console
@elasticmachine run elasticsearch-ci/1 |
jaymode
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| RoleDescriptor.IndicesPrivileges.builder() | ||
| .indices(".code-*").privileges("read").build() | ||
| }, null, MetadataUtils.DEFAULT_RESERVED_METADATA)) | ||
| .put("snapshot_user", new RoleDescriptor("snapshot_user", new String[] { "create_snapshot", GetRepositoriesAction.NAME }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add this reserved role to the documentation?
|
https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+pull-request-1/6139/console
@elasticmachine run elasticsearch-ci/1 |
|
ooops the prev failure was legit, the test was counting reserved roles. Pushed a fix! |
This commit introduces the `create_snapshot` cluster privilege and the `snapshot_user` role. This role is to be used by "cronable" tools that call the snapshot API periodically without recurring to the `manage` cluster privilege. The `create_snapshot` cluster privilege is much more limited compared to the `manage` privilege. The `snapshot_user` role grants the privileges to view the metadata of all indices (including restricted ones, i.e. .security). It obviously grants the create snapshot privilege but the repository has to be created using another role. In addition, it grants the privileges to (only) GET repositories and snapshots, but not create and delete them. The role does not allow to create repositories. This distinction is important because snapshotting equates to the `read` index privilege if the user has control of the snapshot destination, but this is not the case in this instance, because the role does not grant control over repository configuration.
A new cluster privilege type has been added to ES in elastic/elasticsearch#35820 .
This is the docs part for the `snapshot_user` role and the `create_snapshot` cluster privilege which were added in [elastic/elasticsearch#35820](elastic/elasticsearch#35820)
This is the docs part for the `snapshot_user` role and the `create_snapshot` cluster privilege which were added in [elastic/elasticsearch#35820](elastic/elasticsearch#35820)
This is the docs part for the `snapshot_user` role and the `create_snapshot` cluster privilege which were added in [elastic/elasticsearch#35820](elastic/elasticsearch#35820)
This is the docs part for the `snapshot_user` role and the `create_snapshot` cluster privilege which were added in [elastic/elasticsearch#35820](elastic/elasticsearch#35820)
A new cluster privilege type has been added to ES in [elastic/elasticsearch#35820](elastic/elasticsearch#35820) .
A new cluster privilege type has been added to ES in [elastic/elasticsearch#35820](elastic/elasticsearch#35820) .
A new cluster privilege type has been added to ES in [elastic/elasticsearch#35820](elastic/elasticsearch#35820) .
A new cluster privilege type has been added to ES in [elastic/elasticsearch#35820](elastic/elasticsearch#35820) .
A new cluster privilege type has been added to ES in [elastic/elasticsearch#35820](elastic/elasticsearch#35820) .
A new cluster privilege type has been added to ES in [elastic/elasticsearch#35820](elastic/elasticsearch#35820) .
A new cluster privilege type has been added to ES in [elastic/elasticsearch#35820](elastic/elasticsearch#35820) .
A new cluster privilege type has been added to ES in [elastic/elasticsearch#35820](elastic/elasticsearch#35820) .
EDITED 2:
This commit introduces the
create_snapshotcluster privilege and thesnapshot_userrole.This role is to be used by "cronable" tools that call the snapshot API periodically without recurring to the
managecluster privilege. Thecreate_snapshotcluster privilege is much more limited compared to themanageprivilege.The
snapshot_userrole grants the privileges to view the metadata of all indices (including restricted ones, i.e. .security). It obviously grants the create snapshot privilege but the repository has to be created using another role. In addition, it grants the privileges to (only) GET repositories andsnapshots, but not create and delete them.
The role does not allow to create repositories. This distinction is important because snapshotting equates to the
readindex privilege if the user has control of the snapshot destination, but this is not the case in this instance, because the role does not grant control over repository configuration.EDITED:This creates thecreate_snapshotrolecluster privilege and thesnapshot_userrole.This role be used by "cron-able" tools that call the snapshot API periodically without recurring to the
managerole. Thecreate_snapshotsnapshot_userrole has much more limited privileges compared to themanagerole.This role has the privilege to view the metadata of all indices (including restricted ones, i.e..security). It can obviously create snapshots but the repository has to be created by another role. In addition it has the privileges to only GET repositories and snapshots.The role does not allow to create repositories. This distinction is important because snapshotting equates to thereadprivilege if the user has control of the snapshot destination, but this is not the case because the role does grant control over repository configuration.Note that this privilege does not allow to "list" indices and will notimprove the experience with "curator". After mulling it over, I think
listing indices is orthogonal to the snapshot operation. I think many
tools (scripts) would benefit of this role alone and granting only "list"
for indices will make up for a sub-optimal experience during searches
(seeing indices and getting denied when using them in get or search).
If the requirement is to support curator out of the box, I propose
we create a
curatoruser in the reserved realm and grant him theprivileges to get its job done (including this role).
NB1 We don't have the code to grant listing the.securityindex evenif we wish so. This is pending discussion in the mailing list.
NB2 This PR is against a feature branch.Relates #34454