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

Move request interceptors to AuthorizationService #38137

Merged
merged 4 commits into from
Feb 4, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,25 @@ void authorizeIndexAction(RequestInfo requestInfo, AuthorizationInfo authorizati
void loadAuthorizedIndices(RequestInfo requestInfo, AuthorizationInfo authorizationInfo,
Map<String, AliasOrIndex> aliasAndIndexLookup, ActionListener<List<String>> listener);

/**
* Asynchronously checks if that the permissions a user would have for a given list of names do
Copy link
Contributor

Choose a reason for hiding this comment

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

s/if that/that/

* 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,
Copy link
Contributor

Choose a reason for hiding this comment

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

It took me a while to get my head around what this is doing. Good luck coming up with meaningful javadoc for that 😸

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