Add support for authentication based predicate for cluster permission#45431
Merged
bizybot merged 12 commits intoelastic:manage-own-api-key-privilegefrom Aug 22, 2019
Conversation
Currently, cluster permission checks whether a cluster action is permitted and optionally in the context of a request. There are scenarios where we would want to check whether the cluster action is permitted, optionally in the context of a request and current authentication. For example, management of API keys is only restricted to the API keys owned by the current user. In this case, along with the cluster action and API key request, the check needs to perform whether the currently authenticated user is indeed allowed to operate only on owned API keys. With this commit, we are introducing one more context of the current authentication that can be considered during permission evaluation. Relates: elastic#40031
Collaborator
|
Pinging @elastic/es-security |
Contributor
albertzaharovits
left a comment
There was a problem hiding this comment.
Unless there is a clear motivation why ActionRequestAuthenticationPredicatePermissionCheck uses a BiPredicate instead of two predicates, I think this needs change.
Otherwise, LGTM.
.../src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java
Outdated
Show resolved
Hide resolved
…based-permission-check
The permission checks that are dependent on actions and optionally on request and/or on authentication, now have a way to specify the predicates. By default the implementation will tests all the predicates to be successful for the operation to be allowed. In case customization is required one has option to implement `PermissionCheck`. - Adds a permission check predicate interface that also allows implementers to specify behavior for `implies`.
Contributor
Author
|
Failed with known issue #45605 |
…based-permission-check
Contributor
Author
|
@elasticmachine run elasticsearch-ci/packaging-sample |
Contributor
Author
|
@elasticmachine run elasticsearch-ci/1 |
…based-permission-check
tvernum
approved these changes
Aug 22, 2019
Contributor
tvernum
left a comment
There was a problem hiding this comment.
I left a couple of comments around the exclude patterns.
I'm happy to move forward with this, if you can implement one or the other of those suggestions.
.../src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java
Outdated
Show resolved
Hide resolved
.../src/main/java/org/elasticsearch/xpack/core/security/authz/permission/ClusterPermission.java
Outdated
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently, cluster permission checks whether a cluster action is
permitted and optionally in the context of a request. There are
scenarios where we would want to check whether the cluster action
is permitted, optionally in the context of a request and current
authentication. For example, management of API keys is only
restricted to the API keys owned by the current user. In this case,
along with the cluster action and API key request, the check
needs to perform whether the currently authenticated user is indeed
allowed to operate only on owned API keys.
With this commit, we are introducing one more context of the current
authentication that can be considered during permission evaluation.
Relates: #40031