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 support for authentication based predicate for cluster permission #45431

Conversation

bizybot
Copy link
Contributor

@bizybot bizybot commented Aug 12, 2019

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

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
@bizybot bizybot added >enhancement :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC labels Aug 12, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security

@bizybot bizybot marked this pull request as ready for review August 12, 2019 12:11
Copy link
Contributor

@albertzaharovits albertzaharovits left a comment

Choose a reason for hiding this comment

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

Unless there is a clear motivation why ActionRequestAuthenticationPredicatePermissionCheck uses a BiPredicate instead of two predicates, I think this needs change.
Otherwise, LGTM.

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`.
@bizybot
Copy link
Contributor Author

bizybot commented Aug 20, 2019

Failed with known issue #45605
@elasticmachine run elasticsearch-ci/1

Copy link
Contributor

@albertzaharovits albertzaharovits left a comment

Choose a reason for hiding this comment

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

LGTM

@bizybot
Copy link
Contributor Author

bizybot commented Aug 21, 2019

@elasticmachine run elasticsearch-ci/packaging-sample

@bizybot
Copy link
Contributor Author

bizybot commented Aug 21, 2019

@elasticmachine run elasticsearch-ci/1

Copy link
Contributor

@tvernum tvernum left a comment

Choose a reason for hiding this comment

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

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.

@bizybot bizybot merged commit 5661e98 into elastic:manage-own-api-key-privilege Aug 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants