Skip to content

Commit

Permalink
Subset of permissions check on creation (#5012)
Browse files Browse the repository at this point in the history 
Signed-off-by: Derek Ho <dxho@amazon.com>
  • Loading branch information
@derek-ho
derek-ho authored Feb 21, 2025
1 parent 79f0c46 commit a8b4ac1
Show file tree
Hide file tree
Showing 11 changed files with 263 additions and 91 deletions.
6 changes: 2 additions & 4 deletions src/integrationTest/java/org/opensearch/security/privileges/ActionPrivilegesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
import org.opensearch.core.common.unit.ByteSizeUnit;
import org.opensearch.core.common.unit.ByteSizeValue;
import org.opensearch.security.action.apitokens.ApiToken;
import org.opensearch.security.action.apitokens.ApiTokenRepository;
import org.opensearch.security.action.apitokens.Permissions;
import org.opensearch.security.resolver.IndexResolverReplacer;
import org.opensearch.security.securityconf.FlattenedActionGroups;
Expand All @@ -50,9 +49,8 @@
import org.opensearch.security.user.User;
import org.opensearch.security.util.MockIndexMetadataBuilder;

import org.mockito.Mockito;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.opensearch.security.privileges.ActionPrivilegesTest.IndexPrivileges.IndicesAndAliases.resolved;
import static org.opensearch.security.privileges.PrivilegeEvaluatorResponseMatcher.isAllowed;
import static org.opensearch.security.privileges.PrivilegeEvaluatorResponseMatcher.isForbidden;
import static org.opensearch.security.privileges.PrivilegeEvaluatorResponseMatcher.isPartiallyOk;
Expand Down Expand Up @@ -342,6 +340,7 @@ public void apiToken_succeedsWithActionGroupsExapnded() throws Exception {
"apitoken:" + token,
new Permissions(List.of("CLUSTER_ALL"), List.of())
);

// Explicit succeeds
assertThat(subject.hasExplicitClusterPrivilege(context, "cluster:whatever"), isAllowed());
// Not explicit succeeds
Expand Down Expand Up @@ -1152,7 +1151,6 @@ static RoleBasedPrivilegesEvaluationContext ctx(String... roles) {
static RoleBasedPrivilegesEvaluationContext ctxWithUserName(String userName, String... roles) {
User user = new User(userName);
user.addAttributes(ImmutableMap.of("attrs.dept_no", "a11"));
ApiTokenRepository mockRepository = Mockito.mock(ApiTokenRepository.class);
return new RoleBasedPrivilegesEvaluationContext(
user,
ImmutableSet.copyOf(roles),
Expand Down
32 changes: 25 additions & 7 deletions src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ public final class OpenSearchSecurityPlugin extends OpenSearchSecuritySSLPlugin
private volatile UserService userService;
private volatile RestLayerPrivilegesEvaluator restLayerEvaluator;
private volatile ConfigurationRepository cr;
private volatile ApiTokenRepository ar;
private volatile ApiTokenRepository apiTokenRepository;
private volatile AdminDNs adminDns;
private volatile ClusterService cs;
private volatile AtomicReference<DiscoveryNode> localNode = new AtomicReference<>();
Expand Down Expand Up @@ -649,7 +649,21 @@ public List<RestHandler> getRestHandlers(
)
);
handlers.add(new CreateOnBehalfOfTokenAction(tokenManager));
handlers.add(new ApiTokenAction(ar));
handlers.add(
new ApiTokenAction(
Objects.requireNonNull(threadPool),
cr,
evaluator,
settings,
adminDns,
auditLog,
configPath,
principalExtractor,
apiTokenRepository,
cs,
indexNameExpressionResolver
)
);
handlers.addAll(
SecurityRestApiActions.getHandler(
settings,
Expand Down Expand Up @@ -1111,7 +1125,7 @@ public Collection<Object> createComponents(
final XFFResolver xffResolver = new XFFResolver(threadPool);
backendRegistry = new BackendRegistry(settings, adminDns, xffResolver, auditLog, threadPool);
tokenManager = new SecurityTokenManager(cs, threadPool, userService);
ar = new ApiTokenRepository(localClient, clusterService, tokenManager);
apiTokenRepository = new ApiTokenRepository(localClient, clusterService, tokenManager);

final CompatConfig compatConfig = new CompatConfig(environment, transportPassiveAuthSetting);

Expand All @@ -1128,7 +1142,7 @@ public Collection<Object> createComponents(
cih,
irr,
namedXContentRegistry.get(),
ar
apiTokenRepository
);

dlsFlsBaseContext = new DlsFlsBaseContext(evaluator, threadPool.getThreadContext(), adminDns);
Expand Down Expand Up @@ -1170,7 +1184,7 @@ public Collection<Object> createComponents(
configPath,
compatConfig
);
dcf = new DynamicConfigFactory(cr, settings, configPath, localClient, threadPool, cih, passwordHasher, ar);
dcf = new DynamicConfigFactory(cr, settings, configPath, localClient, threadPool, cih, passwordHasher, apiTokenRepository);
dcf.registerDCFListener(backendRegistry);
dcf.registerDCFListener(compatConfig);
dcf.registerDCFListener(irr);
Expand Down Expand Up @@ -1220,7 +1234,7 @@ public Collection<Object> createComponents(
components.add(dcf);
components.add(userService);
components.add(passwordHasher);
components.add(ar);
components.add(apiTokenRepository);

components.add(sslSettingsManager);
if (isSslCertReloadEnabled(settings) && sslCertificatesHotReloadEnabled(settings)) {
Expand Down Expand Up @@ -2143,7 +2157,11 @@ public Collection<SystemIndexDescriptor> getSystemIndexDescriptors(Settings sett
ConfigConstants.OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX
);
final SystemIndexDescriptor systemIndexDescriptor = new SystemIndexDescriptor(indexPattern, "Security index");
return Collections.singletonList(systemIndexDescriptor);
final SystemIndexDescriptor apiTokenSystemIndexDescriptor = new SystemIndexDescriptor(
ConfigConstants.OPENSEARCH_API_TOKENS_INDEX,
"Security API token index"
);
return List.of(systemIndexDescriptor, apiTokenSystemIndexDescriptor);
}

@Override
Expand Down
109 changes: 84 additions & 25 deletions src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
package org.opensearch.security.action.apitokens;

import java.io.IOException;
import java.nio.file.Path;
import java.time.Instant;
import java.util.Collections;
import java.util.List;
Expand All @@ -24,14 +25,29 @@
import org.apache.logging.log4j.Logger;

import org.opensearch.client.node.NodeClient;
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.concurrent.ThreadContext;
import org.opensearch.core.action.ActionListener;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.core.xcontent.XContentBuilder;
import org.opensearch.rest.BaseRestHandler;
import org.opensearch.rest.BytesRestResponse;
import org.opensearch.rest.RestChannel;
import org.opensearch.rest.RestHandler;
import org.opensearch.rest.RestRequest;
import org.opensearch.security.auditlog.AuditLog;
import org.opensearch.security.configuration.AdminDNs;
import org.opensearch.security.configuration.ConfigurationRepository;
import org.opensearch.security.dlic.rest.api.Endpoint;
import org.opensearch.security.dlic.rest.api.RestApiAdminPrivilegesEvaluator;
import org.opensearch.security.dlic.rest.api.RestApiPrivilegesEvaluator;
import org.opensearch.security.dlic.rest.api.SecurityApiDependencies;
import org.opensearch.security.dlic.rest.support.Utils;
import org.opensearch.security.privileges.PrivilegesEvaluator;
import org.opensearch.security.ssl.transport.PrincipalExtractor;
import org.opensearch.security.support.ConfigConstants;
import org.opensearch.threadpool.ThreadPool;

import static org.opensearch.rest.RestRequest.Method.DELETE;
import static org.opensearch.rest.RestRequest.Method.GET;
Expand All @@ -44,23 +60,57 @@
import static org.opensearch.security.action.apitokens.ApiToken.INDEX_PERMISSIONS_FIELD;
import static org.opensearch.security.action.apitokens.ApiToken.NAME_FIELD;
import static org.opensearch.security.dlic.rest.support.Utils.addRoutesPrefix;
import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ADMIN_ENABLED;
import static org.opensearch.security.util.ParsingUtils.safeMapList;
import static org.opensearch.security.util.ParsingUtils.safeStringList;

public class ApiTokenAction extends BaseRestHandler {
private ApiTokenRepository apiTokenRepository;
private final ApiTokenRepository apiTokenRepository;
public Logger log = LogManager.getLogger(this.getClass());

private static final List<RestHandler.Route> ROUTES = addRoutesPrefix(
ImmutableList.of(
new RestHandler.Route(POST, "/apitokens"),
new RestHandler.Route(DELETE, "/apitokens"),
new RestHandler.Route(GET, "/apitokens")
)
private final ThreadPool threadPool;
private final ConfigurationRepository configurationRepository;
private final PrivilegesEvaluator privilegesEvaluator;
private final SecurityApiDependencies securityApiDependencies;
private final ClusterService clusterService;
private final IndexNameExpressionResolver indexNameExpressionResolver;

private static final List<Route> ROUTES = addRoutesPrefix(
ImmutableList.of(new Route(POST, "/apitokens"), new Route(DELETE, "/apitokens"), new Route(GET, "/apitokens"))
);

public ApiTokenAction(ApiTokenRepository apiTokenRepository) {
public ApiTokenAction(
ThreadPool threadpool,
ConfigurationRepository configurationRepository,
PrivilegesEvaluator privilegesEvaluator,
Settings settings,
AdminDNs adminDns,
AuditLog auditLog,
Path configPath,
PrincipalExtractor principalExtractor,
ApiTokenRepository apiTokenRepository,
ClusterService clusterService,
IndexNameExpressionResolver indexNameExpressionResolver
) {
this.apiTokenRepository = apiTokenRepository;
this.threadPool = threadpool;
this.configurationRepository = configurationRepository;
this.privilegesEvaluator = privilegesEvaluator;
this.securityApiDependencies = new SecurityApiDependencies(
adminDns,
configurationRepository,
privilegesEvaluator,
new RestApiPrivilegesEvaluator(settings, adminDns, privilegesEvaluator, principalExtractor, configPath, threadPool),
new RestApiAdminPrivilegesEvaluator(
threadPool.getThreadContext(),
privilegesEvaluator,
adminDns,
settings.getAsBoolean(SECURITY_RESTAPI_ADMIN_ENABLED, false)
),
auditLog,
settings
);
this.clusterService = clusterService;
this.indexNameExpressionResolver = indexNameExpressionResolver;
}

@Override
Expand All @@ -69,22 +119,28 @@ public String getName() {
}

@Override
public List<RestHandler.Route> routes() {
public List<Route> routes() {
return ROUTES;
}

@Override
protected RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
// TODO: Authorize this API properly
switch (request.method()) {
case POST:
return handlePost(request, client);
case DELETE:
return handleDelete(request, client);
case GET:
return handleGet(request, client);
default:
throw new IllegalArgumentException(request.method() + " not supported");
authorizeSecurityAccess(request);
return doPrepareRequest(request, client);
}

RestChannelConsumer doPrepareRequest(RestRequest request, NodeClient client) {
final var originalUserAndRemoteAddress = Utils.userAndRemoteAddressFrom(client.threadPool().getThreadContext());
try (final ThreadContext.StoredContext ctx = client.threadPool().getThreadContext().stashContext()) {
client.threadPool()
.getThreadContext()
.putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, originalUserAndRemoteAddress.getLeft());
return switch (request.method()) {
case POST -> handlePost(request, client);
case DELETE -> handleDelete(request, client);
case GET -> handleGet(request, client);
default -> throw new IllegalArgumentException(request.method() + " not supported");
};
}
}

Expand Down Expand Up @@ -119,8 +175,6 @@ private RestChannelConsumer handleGet(RestRequest request, NodeClient client) {

private RestChannelConsumer handlePost(RestRequest request, NodeClient client) {
return channel -> {
final XContentBuilder builder = channel.newBuilder();
BytesRestResponse response;
try {
final Map<String, Object> requestBody = request.contentOrSourceParamParser().map();
validateRequestParameters(requestBody);
Expand Down Expand Up @@ -245,8 +299,6 @@ void validateIndexPermissionsList(List<Map<String, Object>> indexPermsList) {

private RestChannelConsumer handleDelete(RestRequest request, NodeClient client) {
return channel -> {
final XContentBuilder builder = channel.newBuilder();
BytesRestResponse response;
try {
final Map<String, Object> requestBody = request.contentOrSourceParamParser().map();

Expand Down Expand Up @@ -295,4 +347,11 @@ private void sendErrorResponse(RestChannel channel, RestStatus status, String er
}
}

protected void authorizeSecurityAccess(RestRequest request) throws IOException {
// Check if user has security API access
if (!(securityApiDependencies.restApiAdminPrivilegesEvaluator().isCurrentUserAdminFor(Endpoint.APITOKENS)
|| securityApiDependencies.restApiPrivilegesEvaluator().checkAccessPermissions(request, Endpoint.APITOKENS) == null)) {
throw new SecurityException("User does not have required security API access");
}
}
}
Loading

0 comments on commit a8b4ac1

Please sign in to comment.