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

Allow custom authorization with an authorization engine #38358

Merged
merged 27 commits into from
Feb 5, 2019

Conversation

jaymode
Copy link
Member

@jaymode jaymode commented Feb 4, 2019

For some users, the built in authorization mechanism does not fit their
needs and no feature that we offer would allow them to control the
authorization process to meet their needs. In order to support this,
a concept of an AuthorizationEngine is being introduced, which can be
provided using the security extension mechanism.

An AuthorizationEngine is responsible for making the authorization
decisions about a request. The engine is responsible for knowing how to
authorize and can be backed by whatever mechanism a user wants. The
default mechanism is one backed by roles to provide the authorization
decisions. The AuthorizationEngine will be called by the
AuthorizationService, which handles more of the internal workings that
apply in general to authorization within Elasticsearch.

In order to support external authorization services that would back an
authorization engine, the entire authorization process has become
asynchronous, which also includes all calls to the AuthorizationEngine.

The use of roles also leaked out of the AuthorizationService in our
existing code that is not specifically related to roles so this also
needed to be addressed. RequestInterceptor instances sometimes used a
role to ensure a user was not attempting to escalate their privileges.
Addressing this leakage of roles meant that the RequestInterceptor
execution needed to move within the AuthorizationService and that
AuthorizationEngines needed to support detection of whether a user has
more privileges on a name than another. The second area where roles
leaked to the user is in the handling of a few privilege APIs that
could be used to retrieve the user's privileges or ask if a user has
privileges to perform an action. To remove the leakage of roles from
these actions, the AuthorizationService and AuthorizationEngine gained
methods that enabled an AuthorizationEngine to return the response for
these APIs.

Ultimately this feature is the work included in:
#37785
#37495
#37328
#36245
#38137
#38219

Closes #32435

jaymode and others added 20 commits January 7, 2019 13:43
In order to support the concept of different authorization engines, this
change begins the refactoring of the AuthorizationService to support
this. Previously, the asynchronous work for authorization was performed
by the AsyncAuthorizer class, but this tied the authorization service
to a role based implementation. In this change, the authorize method
become asynchronous and delegates much of the actual permission checking
to an AuthorizationEngine. The pre-existing RBAC permission checking
has been abstracted into the RBACEngine. The majority of calls to
AuthorizationEngine instances are asynchronous as the underlying
implementation may need to make network calls that should not block
the current thread, which are often network threads.

This change is meant to be built upon. The basic concepts are introduced
without proper documentation, plumbing to enable other
AuthorizationEngine types, and some items we may want to refactor.
For example, the AuthorizedIndices class is lazily loaded but this might
actually be something we want to make asynchronous. We pass a lot of the
same arguments to the various methods and it would be prudent to wrap
these in a class; this class would provide a way for us to pass
additional items needed by future enhancements without breaking the
interface and requiring updates to all implementations.

See #32435
This change replaces the AuthorizedIndices class with a simple list.
The change to a simple list does remove the lazy loading of the
authorized indices in favor of simpler code as the loading of this
list is now an asynchronous operation that is delegated to the
authorization engine.
This change introduces a new class called RequestInfo that encapsulates
the common objects that are passed to the authorization engine methods.
By doing so, we give ourselves a way of adding additional data without
breaking the interface. Additionally, this also reduces the need to
ensure we pass these three parameters in the same order everywhere for
consistency.
This commit adds javadocs to the AuthorizationEngine interface aimed at
developers of an authorization engine. Additionally, some classes were
also moved to the core project so that they are ready to be exposed
once we allow authorization engines to be plugged in.
Authorization engines can now be registered by implementing a plugin,
which also has a service implementation of a security extension. Only
one extension may register an authorization engine and this engine will
be used for all users except reserved realm users and internal users.
This change moves the RequestInterceptor iteration from the action
filter to the AuthorizationService. This is done to remove the need
for the use of a role within the request interceptors and replace it
with the AuthorizationEngine. The AuthorizationEngine interface was
also enhanced with a new method that is used to determine if a users
permission on one index is a subset of their permissions on a list
of indices or aliases.

Additionally, this change addresses some leftover cleanups.
This commit moves the evaluation of privileges from a few transport
actions into the authorization engine. The APIs are used by other
applications for making decisions and if a different authorization
engine is used that is not role based, we should still allow these APIs
to work. By moving this evaluation out of the transport action, the
transport actions no longer have a dependency on roles.
@jaymode jaymode added >feature v7.0.0 :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC v6.7.0 labels Feb 4, 2019
@jaymode jaymode requested a review from tvernum February 4, 2019 19:35
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security

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.

LGTM

if (value != null) {
if (stringBuilder.length() > 1) {
stringBuilder.append(",");
}
stringBuilder.append("\"");
jsonStringEncoder.quoteAsString(value, stringBuilder);
jsonStringEncoder.quoteAsString(value.toString(), stringBuilder);

This comment was marked as resolved.

This comment was marked as outdated.

@jaymode jaymode merged commit 7ca5495 into master Feb 5, 2019
@jaymode jaymode deleted the security_authz_engine branch February 5, 2019 20:39
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Feb 11, 2019
* master:
  Add an authentication cache for API keys (elastic#38469)
  Fix exit code in certutil packaging test (elastic#38393)
  Enable logs for intermittent test failure (elastic#38426)
  Disable BWC to backport recovering retention leases (elastic#38477)
  Enable bwc tests now that elastic#38443 is backported. (elastic#38462)
  Fix Master Failover and DataNode Leave Blocking Snapshot (elastic#38460)
  Recover retention leases during peer recovery (elastic#38435)
  Set update mappings mater node timeout to 30 min (elastic#38439)
  Assert job is not null in FullClusterRestartIT (elastic#38218)
  Update ilm-api.asciidoc, point to REMOVE policy (elastic#38235) (elastic#38463)
  SQL: Fix esType for DATETIME/DATE and INTERVALS (elastic#38179)
  Handle deprecation header-AbstractUpgradeTestCase (elastic#38396)
  XPack: core/ccr/Security-cli migration to java-time (elastic#38415)
  Disable bwc tests for elastic#38443 (elastic#38456)
  Bubble-up exceptions from scheduler (elastic#38317)
  Re-enable TasksClientDocumentationIT.testCancelTasks (elastic#38234)
  Allow custom authorization with an authorization engine  (elastic#38358)
  CRUDDocumentationIT fix documentation references
  Remove support for internal versioning for concurrency control (elastic#38254)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants