Skip to content

Commit

Permalink
Move request interceptors to AuthorizationService (elastic#38137)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jaymode authored Feb 4, 2019
1 parent 0e1c191 commit e5615d2
Show file tree
Hide file tree
Showing 23 changed files with 720 additions and 496 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,17 @@ public void loadAuthorizedIndices(RequestInfo requestInfo, AuthorizationInfo aut
}
}

@Override
public void validateIndexPermissionsAreSubset(RequestInfo requestInfo, AuthorizationInfo authorizationInfo,
Map<String, List<String>> indexNameToNewNames,
ActionListener<AuthorizationResult> listener) {
if (isSuperuser(requestInfo.getAuthentication().getUser())) {
listener.onResponse(AuthorizationResult.granted());
} else {
listener.onResponse(AuthorizationResult.deny());
}
}

public static class CustomAuthorizationInfo implements AuthorizationInfo {

private final String[] roles;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,8 @@ void authorizeIndexAction(RequestInfo requestInfo, AuthorizationInfo authorizati
ActionListener<IndexAuthorizationResult> listener);

/**
* Asynchronously loads a list of alias and index names for which the user is authorized
* to execute the requested action.
*
* @param requestInfo object contain the request and associated information such as the action
* and associated user(s)
Expand All @@ -139,6 +141,26 @@ void authorizeIndexAction(RequestInfo requestInfo, AuthorizationInfo authorizati
void loadAuthorizedIndices(RequestInfo requestInfo, AuthorizationInfo authorizationInfo,
Map<String, AliasOrIndex> aliasAndIndexLookup, ActionListener<List<String>> listener);

/**
* Asynchronously checks that the permissions a user would have for a given list of names do
* not exceed their permissions for a given name. This is used to ensure that a user cannot
* perform operations that would escalate their privileges over the data. Some examples include
* adding an alias to gain more permissions to a given index and/or resizing an index in order
* to gain more privileges on the data since the index name changes.
*
* @param requestInfo object contain the request and associated information such as the action
* and associated user(s)
* @param authorizationInfo information needed from authorization that was previously retrieved
* from {@link #resolveAuthorizationInfo(RequestInfo, ActionListener)}
* @param indexNameToNewNames A map of an existing index/alias name to a one or more names of
* an index/alias that the user is requesting to create. The method
* should validate that none of the names have more permissions than
* the name in the key would have.
* @param listener the listener to be notified of the authorization result
*/
void validateIndexPermissionsAreSubset(RequestInfo requestInfo, AuthorizationInfo authorizationInfo,
Map<String, List<String>> indexNameToNewNames, ActionListener<AuthorizationResult> listener);

/**
* Interface for objects that contains the information needed to authorize a request
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,12 @@
import org.elasticsearch.xpack.core.ssl.action.TransportGetCertificateInfoAction;
import org.elasticsearch.xpack.core.ssl.rest.RestGetCertificateInfoAction;
import org.elasticsearch.xpack.security.action.filter.SecurityActionFilter;
import org.elasticsearch.xpack.security.action.interceptor.BulkShardRequestInterceptor;
import org.elasticsearch.xpack.security.action.interceptor.IndicesAliasesRequestInterceptor;
import org.elasticsearch.xpack.security.action.interceptor.RequestInterceptor;
import org.elasticsearch.xpack.security.action.interceptor.ResizeRequestInterceptor;
import org.elasticsearch.xpack.security.action.interceptor.SearchRequestInterceptor;
import org.elasticsearch.xpack.security.action.interceptor.UpdateRequestInterceptor;
import org.elasticsearch.xpack.security.authz.interceptor.BulkShardRequestInterceptor;
import org.elasticsearch.xpack.security.authz.interceptor.IndicesAliasesRequestInterceptor;
import org.elasticsearch.xpack.security.authz.interceptor.RequestInterceptor;
import org.elasticsearch.xpack.security.authz.interceptor.ResizeRequestInterceptor;
import org.elasticsearch.xpack.security.authz.interceptor.SearchRequestInterceptor;
import org.elasticsearch.xpack.security.authz.interceptor.UpdateRequestInterceptor;
import org.elasticsearch.xpack.security.action.privilege.TransportDeletePrivilegesAction;
import org.elasticsearch.xpack.security.action.privilege.TransportGetPrivilegesAction;
import org.elasticsearch.xpack.security.action.privilege.TransportPutPrivilegesAction;
Expand Down Expand Up @@ -437,8 +437,24 @@ Collection<Object> createComponents(Client client, ThreadPool threadPool, Cluste
// to keep things simple, just invalidate all cached entries on license change. this happens so rarely that the impact should be
// minimal
getLicenseState().addListener(allRolesStore::invalidateAll);

final Set<RequestInterceptor> requestInterceptors;
if (XPackSettings.DLS_FLS_ENABLED.get(settings)) {
requestInterceptors = Collections.unmodifiableSet(Sets.newHashSet(
new SearchRequestInterceptor(threadPool, getLicenseState()),
new UpdateRequestInterceptor(threadPool, getLicenseState()),
new BulkShardRequestInterceptor(threadPool, getLicenseState()),
new ResizeRequestInterceptor(threadPool, getLicenseState(), auditTrailService),
new IndicesAliasesRequestInterceptor(threadPool.getThreadContext(), getLicenseState(), auditTrailService)));
} else {
requestInterceptors = Collections.unmodifiableSet(Sets.newHashSet(
new ResizeRequestInterceptor(threadPool, getLicenseState(), auditTrailService),
new IndicesAliasesRequestInterceptor(threadPool.getThreadContext(), getLicenseState(), auditTrailService)));
}

final AuthorizationService authzService = new AuthorizationService(settings, allRolesStore, clusterService,
auditTrailService, failureHandler, threadPool, anonymousUser, getAuthorizationEngine());
auditTrailService, failureHandler, threadPool, anonymousUser, getAuthorizationEngine(), requestInterceptors);

components.add(nativeRolesStore); // used by roles actions
components.add(reservedRolesStore); // used by roles actions
components.add(allRolesStore); // for SecurityFeatureSet and clear roles cache
Expand All @@ -450,20 +466,8 @@ Collection<Object> createComponents(Client client, ThreadPool threadPool, Cluste
securityInterceptor.set(new SecurityServerTransportInterceptor(settings, threadPool, authcService.get(),
authzService, getLicenseState(), getSslService(), securityContext.get(), destructiveOperations, clusterService));

final Set<RequestInterceptor> requestInterceptors;
if (XPackSettings.DLS_FLS_ENABLED.get(settings)) {
requestInterceptors = Collections.unmodifiableSet(Sets.newHashSet(
new SearchRequestInterceptor(threadPool, getLicenseState()),
new UpdateRequestInterceptor(threadPool, getLicenseState()),
new BulkShardRequestInterceptor(threadPool, getLicenseState()),
new ResizeRequestInterceptor(threadPool, getLicenseState(), auditTrailService),
new IndicesAliasesRequestInterceptor(threadPool.getThreadContext(), getLicenseState(), auditTrailService)));
} else {
requestInterceptors = Collections.emptySet();
}

securityActionFilter.set(new SecurityActionFilter(authcService.get(), authzService, getLicenseState(),
requestInterceptors, threadPool, securityContext.get(), destructiveOperations));
threadPool, securityContext.get(), destructiveOperations));

return components;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,15 @@
import org.elasticsearch.xpack.core.XPackField;
import org.elasticsearch.xpack.core.security.SecurityContext;
import org.elasticsearch.xpack.core.security.authc.Authentication;
import org.elasticsearch.xpack.core.security.authz.permission.Role;
import org.elasticsearch.xpack.core.security.authz.privilege.HealthAndStatsPrivilege;
import org.elasticsearch.xpack.core.security.support.Automatons;
import org.elasticsearch.xpack.core.security.user.SystemUser;
import org.elasticsearch.xpack.security.action.SecurityActionMapper;
import org.elasticsearch.xpack.security.action.interceptor.RequestInterceptor;
import org.elasticsearch.xpack.security.authc.AuthenticationService;
import org.elasticsearch.xpack.core.security.authz.AuthorizationEngine.AuthorizationInfo;
import org.elasticsearch.xpack.security.authz.AuthorizationService;
import org.elasticsearch.xpack.security.authz.AuthorizationUtils;
import org.elasticsearch.xpack.security.authz.RBACEngine.RBACAuthorizationInfo;

import java.io.IOException;
import java.util.Set;
import java.util.function.Predicate;

public class SecurityActionFilter implements ActionFilter {
Expand All @@ -53,19 +48,17 @@ public class SecurityActionFilter implements ActionFilter {
private final AuthenticationService authcService;
private final AuthorizationService authzService;
private final SecurityActionMapper actionMapper = new SecurityActionMapper();
private final Set<RequestInterceptor> requestInterceptors;
private final XPackLicenseState licenseState;
private final ThreadContext threadContext;
private final SecurityContext securityContext;
private final DestructiveOperations destructiveOperations;

public SecurityActionFilter(AuthenticationService authcService, AuthorizationService authzService,
XPackLicenseState licenseState, Set<RequestInterceptor> requestInterceptors, ThreadPool threadPool,
XPackLicenseState licenseState, ThreadPool threadPool,
SecurityContext securityContext, DestructiveOperations destructiveOperations) {
this.authcService = authcService;
this.authzService = authzService;
this.licenseState = licenseState;
this.requestInterceptors = requestInterceptors;
this.threadContext = threadPool.getThreadContext();
this.securityContext = securityContext;
this.destructiveOperations = destructiveOperations;
Expand Down Expand Up @@ -167,24 +160,8 @@ private <Request extends ActionRequest> void authorizeRequest(Authentication aut
if (authentication == null) {
listener.onFailure(new IllegalArgumentException("authentication must be non null for authorization"));
} else {
authzService.authorize(authentication, securityAction, request, ActionListener.wrap(ignore -> {
/*
* We use a separate concept for code that needs to be run after authentication and authorization that could
* affect the running of the action. This is done to make it more clear of the state of the request.
*/
// FIXME this needs to be done in a way that allows us to operate without a role
Role role = null;
AuthorizationInfo authorizationInfo = threadContext.getTransient("_authz_info");
if (authorizationInfo instanceof RBACAuthorizationInfo) {
role = ((RBACAuthorizationInfo) authorizationInfo).getRole();
}
for (RequestInterceptor interceptor : requestInterceptors) {
if (interceptor.supports(request)) {
interceptor.intercept(request, authentication, role, securityAction);
}
}
listener.onResponse(null);
}, listener::onFailure));
authzService.authorize(authentication, securityAction, request, ActionListener.wrap(ignore -> listener.onResponse(null),
listener::onFailure));
}
}
}

This file was deleted.

Loading

0 comments on commit e5615d2

Please sign in to comment.