diff --git a/src/integrationTest/java/org/opensearch/security/privileges/ActionPrivilegesTest.java b/src/integrationTest/java/org/opensearch/security/privileges/ActionPrivilegesTest.java index bfe6e600c9..01f7ad9f3d 100644 --- a/src/integrationTest/java/org/opensearch/security/privileges/ActionPrivilegesTest.java +++ b/src/integrationTest/java/org/opensearch/security/privileges/ActionPrivilegesTest.java @@ -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; @@ -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; @@ -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 @@ -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), diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 1d5091ca04..c68c4fb610 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -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 localNode = new AtomicReference<>(); @@ -649,7 +649,21 @@ public List 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, @@ -1111,7 +1125,7 @@ public Collection 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); @@ -1128,7 +1142,7 @@ public Collection createComponents( cih, irr, namedXContentRegistry.get(), - ar + apiTokenRepository ); dlsFlsBaseContext = new DlsFlsBaseContext(evaluator, threadPool.getThreadContext(), adminDns); @@ -1170,7 +1184,7 @@ public Collection 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); @@ -1220,7 +1234,7 @@ public Collection createComponents( components.add(dcf); components.add(userService); components.add(passwordHasher); - components.add(ar); + components.add(apiTokenRepository); components.add(sslSettingsManager); if (isSslCertReloadEnabled(settings) && sslCertificatesHotReloadEnabled(settings)) { @@ -2143,7 +2157,11 @@ public Collection 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 diff --git a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java index 0b6a7b320a..bcef2d0b25 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java @@ -12,26 +12,57 @@ 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.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.opensearch.OpenSearchException; 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.common.transport.TransportAddress; 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.IndexPattern; +import org.opensearch.security.privileges.PrivilegesEvaluationException; +import org.opensearch.security.privileges.PrivilegesEvaluator; +import org.opensearch.security.securityconf.DynamicConfigFactory; +import org.opensearch.security.securityconf.FlattenedActionGroups; +import org.opensearch.security.securityconf.impl.CType; +import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; +import org.opensearch.security.securityconf.impl.v7.ActionGroupsV7; +import org.opensearch.security.securityconf.impl.v7.RoleV7; +import org.opensearch.security.ssl.transport.PrincipalExtractor; +import org.opensearch.security.support.ConfigConstants; +import org.opensearch.security.support.WildcardMatcher; +import org.opensearch.security.user.User; +import org.opensearch.threadpool.ThreadPool; import static org.opensearch.rest.RestRequest.Method.DELETE; import static org.opensearch.rest.RestRequest.Method.GET; @@ -44,23 +75,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 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 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 @@ -69,22 +134,28 @@ public String getName() { } @Override - public List routes() { + public List 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"); + }; } } @@ -119,8 +190,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 requestBody = request.contentOrSourceParamParser().map(); validateRequestParameters(requestBody); @@ -128,6 +197,8 @@ private RestChannelConsumer handlePost(RestRequest request, NodeClient client) { List clusterPermissions = extractClusterPermissions(requestBody); List indexPermissions = extractIndexPermissions(requestBody); + validateUserPermissions(clusterPermissions, indexPermissions); + String token = apiTokenRepository.createApiToken( (String) requestBody.get(NAME_FIELD), clusterPermissions, @@ -245,8 +316,6 @@ void validateIndexPermissionsList(List> indexPermsList) { private RestChannelConsumer handleDelete(RestRequest request, NodeClient client) { return channel -> { - final XContentBuilder builder = channel.newBuilder(); - BytesRestResponse response; try { final Map requestBody = request.contentOrSourceParamParser().map(); @@ -295,4 +364,104 @@ private void sendErrorResponse(RestChannel channel, RestStatus status, String er } } + /** + * Validates that the user has the required permissions to create an API token (must be a subset of their own permissions) + * */ + @SuppressWarnings("unchecked") + void validateUserPermissions(List clusterPermissions, List indexPermissions) + throws PrivilegesEvaluationException { + final User user = threadPool.getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER); + final TransportAddress caller = threadPool.getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS); + final Set roles = privilegesEvaluator.mapRoles(user, caller); + + // Early return conditions + if (roles.isEmpty()) { + throw new OpenSearchException("User does not have any roles"); + } + + // Verify user has all requested cluster permissions + final SecurityDynamicConfiguration actionGroupsConfiguraiton = (SecurityDynamicConfiguration) load( + CType.ACTIONGROUPS + ); + FlattenedActionGroups flattenedActionGroups = new FlattenedActionGroups(actionGroupsConfiguraiton); + final SecurityDynamicConfiguration rolesConfiguration = load(CType.ROLES); + ImmutableSet resolvedClusterPermissions = flattenedActionGroups.resolve(clusterPermissions); + Set clusterPermissionsWithoutActionGroups = resolvedClusterPermissions.stream() + .filter(permission -> !actionGroupsConfiguraiton.getCEntries().containsKey(permission)) + .collect(Collectors.toSet()); + + // Load all roles the user has access to, remove permissions that + for (String role : roles) { + RoleV7 roleV7 = (RoleV7) rolesConfiguration.getCEntry(role); + ImmutableSet expandedRoleClusterPermissions = flattenedActionGroups.resolve(roleV7.getCluster_permissions()); + for (String clusterPermission : expandedRoleClusterPermissions) { + WildcardMatcher matcher = WildcardMatcher.from(clusterPermission); + clusterPermissionsWithoutActionGroups.removeIf(matcher); + } + } + + if (!clusterPermissionsWithoutActionGroups.isEmpty()) { + throw new OpenSearchException("User does not have all requested cluster permissions"); + } + + // Verify user has all requested index permissions + for (ApiToken.IndexPermission requestedPermission : indexPermissions) { + // First, flatten/resolve any action groups into the underlying actions and remove action group names (which may not exact + // match) + Set resolvedActions = new HashSet<>(flattenedActionGroups.resolve(requestedPermission.getAllowedActions())); + resolvedActions.removeIf(permission -> actionGroupsConfiguraiton.getCEntries().containsKey(permission)); + + // For each index pattern in the requested permission + for (String requestedPattern : requestedPermission.getIndexPatterns()) { + + Set actionsForIndexPattern = new HashSet<>(resolvedActions); + + // Check each role the user has + for (String roleName : roles) { + RoleV7 role = (RoleV7) rolesConfiguration.getCEntry(roleName); + if (role == null || role.getIndex_permissions() == null) continue; + + // Check each index permission block in the role + for (RoleV7.Index indexPermission : role.getIndex_permissions()) { + List rolePatterns = indexPermission.getIndex_patterns(); + List roleIndexPerms = indexPermission.getAllowed_actions(); + + IndexPattern indexPattern = IndexPattern.from(rolePatterns); + + // Check if this role's pattern covers the requested pattern + if (indexPattern.matches( + requestedPattern, + indexNameExpressionResolver, + (String template) -> WildcardMatcher.NONE, + clusterService.state().metadata().getIndicesLookup() + )) { + // Get resolved actions for this role's index permissions + Set roleActions = flattenedActionGroups.resolve(roleIndexPerms); + WildcardMatcher matcher = WildcardMatcher.from(roleActions); + + actionsForIndexPattern.removeIf(matcher); + } + } + } + + // After checking all roles, verify if all requested actions were covered + if (!actionsForIndexPattern.isEmpty()) { + throw new OpenSearchException("User does not have sufficient permissions for index pattern: " + requestedPattern); + } + } + } + } + + private SecurityDynamicConfiguration load(final CType config) { + SecurityDynamicConfiguration loaded = configurationRepository.getConfiguration(config); + return DynamicConfigFactory.addStatics(loaded); + } + + 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"); + } + } } diff --git a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandler.java b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandler.java index 488229a319..9145ee4bb1 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandler.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandler.java @@ -26,7 +26,6 @@ import org.opensearch.action.search.SearchResponse; import org.opensearch.client.Client; import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.common.xcontent.XContentType; import org.opensearch.core.action.ActionListener; @@ -41,7 +40,6 @@ import org.opensearch.index.reindex.DeleteByQueryRequest; import org.opensearch.search.SearchHit; import org.opensearch.search.builder.SearchSourceBuilder; -import org.opensearch.security.dlic.rest.support.Utils; import org.opensearch.security.support.ConfigConstants; import static org.opensearch.security.action.apitokens.ApiToken.NAME_FIELD; @@ -57,14 +55,8 @@ public ApiTokenIndexHandler(Client client, ClusterService clusterService) { this.clusterService = clusterService; } - public String indexTokenMetadata(ApiToken token) { - // TODO: move this out of index handler class, potentially create a layer in between baseresthandler and abstractapiaction which can - // abstract this complexity away - 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()); + public void indexTokenMetadata(ApiToken token) { + try { XContentBuilder builder = XContentFactory.jsonBuilder(); String jsonString = token.toXContent(builder, ToXContent.EMPTY_PARAMS).toString(); @@ -77,10 +69,7 @@ public String indexTokenMetadata(ApiToken token) { LOGGER.error(failResponse.getMessage()); LOGGER.info("Failed to create {} entry.", ConfigConstants.OPENSEARCH_API_TOKENS_INDEX); }); - client.index(request, irListener); - return token.getName(); - } catch (IOException e) { throw new RuntimeException(e); } @@ -88,32 +77,21 @@ public String indexTokenMetadata(ApiToken token) { } public void deleteToken(String name) throws ApiTokenException { - 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()); - DeleteByQueryRequest request = new DeleteByQueryRequest(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX).setQuery( - QueryBuilders.matchQuery(NAME_FIELD, name) - ).setRefresh(true); + DeleteByQueryRequest request = new DeleteByQueryRequest(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX).setQuery( + QueryBuilders.matchQuery(NAME_FIELD, name) + ).setRefresh(true); - BulkByScrollResponse response = client.execute(DeleteByQueryAction.INSTANCE, request).actionGet(); + BulkByScrollResponse response = client.execute(DeleteByQueryAction.INSTANCE, request).actionGet(); - long deletedDocs = response.getDeleted(); + long deletedDocs = response.getDeleted(); - if (deletedDocs == 0) { - throw new ApiTokenException("No token found with name " + name); - } - LOGGER.info("Deleted " + deletedDocs + " documents"); + if (deletedDocs == 0) { + throw new ApiTokenException("No token found with name " + name); } } public Map getTokenMetadatas() { - 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()); + try { SearchRequest searchRequest = new SearchRequest(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX); searchRequest.source(new SearchSourceBuilder()); @@ -145,24 +123,12 @@ public Boolean apiTokenIndexExists() { } public void createApiTokenIndexIfAbsent() { - // TODO: Decide if this should be done at bootstrap if (!apiTokenIndexExists()) { - 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()); - final Map indexSettings = ImmutableMap.of( - "index.number_of_shards", - 1, - "index.auto_expand_replicas", - "0-all" - ); - final CreateIndexRequest createIndexRequest = new CreateIndexRequest(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX).settings( - indexSettings - ); - client.admin().indices().create(createIndexRequest); - } + final Map indexSettings = ImmutableMap.of("index.number_of_shards", 1, "index.auto_expand_replicas", "0-all"); + final CreateIndexRequest createIndexRequest = new CreateIndexRequest(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX).settings( + indexSettings + ); + client.admin().indices().create(createIndexRequest); } } diff --git a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java index b1f99bdbd6..850fad0b66 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java @@ -89,7 +89,6 @@ public String createApiToken( Long expiration ) { apiTokenIndexHandler.createApiTokenIndexIfAbsent(); - // TODO: Add validation on whether user is creating a token with a subset of their permissions ExpiringBearerAuthToken token = securityTokenManager.issueApiToken(name, expiration); ApiToken apiToken = new ApiToken(name, clusterPermissions, indexPermissions, expiration); apiTokenIndexHandler.indexTokenMetadata(apiToken); diff --git a/src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java b/src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java index 9a16cd8bfd..5b90f46f83 100644 --- a/src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java +++ b/src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java @@ -126,7 +126,6 @@ protected AbstractAuditLog( ConfigConstants.SECURITY_CONFIG_INDEX_NAME, ConfigConstants.OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX ); - // TODO: support custom api tokens index? this.securityIndicesMatcher = WildcardMatcher.from( List.of( settings.get(ConfigConstants.SECURITY_CONFIG_INDEX_NAME, ConfigConstants.OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX), diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/Endpoint.java b/src/main/java/org/opensearch/security/dlic/rest/api/Endpoint.java index ecc9dcbc59..d5555b445c 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/Endpoint.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/Endpoint.java @@ -30,5 +30,6 @@ public enum Endpoint { WHITELIST, ALLOWLIST, NODESDN, - SSL; + SSL, + APITOKENS; } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/RestApiAdminPrivilegesEvaluator.java b/src/main/java/org/opensearch/security/dlic/rest/api/RestApiAdminPrivilegesEvaluator.java index faa0217db2..768f9d2f70 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/RestApiAdminPrivilegesEvaluator.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/RestApiAdminPrivilegesEvaluator.java @@ -70,6 +70,7 @@ default String build() { .put(Endpoint.ROLES, action -> buildEndpointPermission(Endpoint.ROLES)) .put(Endpoint.ROLESMAPPING, action -> buildEndpointPermission(Endpoint.ROLESMAPPING)) .put(Endpoint.TENANTS, action -> buildEndpointPermission(Endpoint.TENANTS)) + .put(Endpoint.APITOKENS, action -> buildEndpointPermission(Endpoint.APITOKENS)) .put(Endpoint.SSL, action -> buildEndpointActionPermission(Endpoint.SSL, action)) .build(); diff --git a/src/main/java/org/opensearch/security/http/ApiTokenAuthenticator.java b/src/main/java/org/opensearch/security/http/ApiTokenAuthenticator.java index 0a8e3466d7..50b80ad522 100644 --- a/src/main/java/org/opensearch/security/http/ApiTokenAuthenticator.java +++ b/src/main/java/org/opensearch/security/http/ApiTokenAuthenticator.java @@ -145,7 +145,6 @@ private AuthCredentials extractCredentials0(final SecurityRequest request, final return null; } - // TODO: handle revocation different from deletion? if (!apiTokenRepository.isValidToken(subject)) { log.error("Api token is not allowlisted"); return null; diff --git a/src/main/java/org/opensearch/security/privileges/IndexPattern.java b/src/main/java/org/opensearch/security/privileges/IndexPattern.java index d5d419f72b..e5a4f0f36e 100644 --- a/src/main/java/org/opensearch/security/privileges/IndexPattern.java +++ b/src/main/java/org/opensearch/security/privileges/IndexPattern.java @@ -61,29 +61,33 @@ private IndexPattern(WildcardMatcher staticPattern, ImmutableList patter this.hashCode = staticPattern.hashCode() + patternTemplates.hashCode() + dateMathExpressions.hashCode(); } - public boolean matches(String index, PrivilegesEvaluationContext context, Map indexMetadata) - throws PrivilegesEvaluationException { + @FunctionalInterface + public interface MatcherGenerator { + WildcardMatcher apply(String pattern) throws PrivilegesEvaluationException; + } + + public boolean matches( + String index, + IndexNameExpressionResolver indexNameExpressionResolver, + MatcherGenerator matcherGenerator, + Map indexMetadata + ) throws PrivilegesEvaluationException { if (staticPattern != WildcardMatcher.NONE && staticPattern.test(index)) { return true; } if (!patternTemplates.isEmpty()) { for (String patternTemplate : this.patternTemplates) { - try { - WildcardMatcher matcher = context.getRenderedMatcher(patternTemplate); - if (matcher.test(index)) { - return true; - } - } catch (ExpressionEvaluationException e) { - throw new PrivilegesEvaluationException("Error while evaluating dynamic index pattern: " + patternTemplate, e); + WildcardMatcher matcher = matcherGenerator.apply(patternTemplate); + + if (matcher.test(index)) { + return true; } } } if (!dateMathExpressions.isEmpty()) { - IndexNameExpressionResolver indexNameExpressionResolver = context.getIndexNameExpressionResolver(); - // Note: The use of date math expressions in privileges is a bit odd, as it only provides a very limited // solution for the potential user case. A different approach might be nice. @@ -108,7 +112,12 @@ public boolean matches(String index, PrivilegesEvaluationContext context, Map indexMetadata) + throws PrivilegesEvaluationException { + return matches(index, context.getIndexNameExpressionResolver(), (String patternTemplate) -> { + try { + return context.getRenderedMatcher(patternTemplate); + } catch (ExpressionEvaluationException e) { + throw new PrivilegesEvaluationException("Error while evaluating dynamic index pattern: " + patternTemplate, e); + } + }, indexMetadata); + } + @Override public String toString() { if (patternTemplates.size() == 0 && dateMathExpressions.size() == 0) { diff --git a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenActionTest.java b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenActionTest.java index e7193710e1..838aaf6ed9 100644 --- a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenActionTest.java +++ b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenActionTest.java @@ -16,18 +16,170 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; +import com.google.common.collect.ImmutableMap; +import com.fasterxml.jackson.core.JsonProcessingException; +import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; + +import org.opensearch.OpenSearchException; +import org.opensearch.Version; +import org.opensearch.cluster.ClusterState; +import org.opensearch.cluster.metadata.AliasMetadata; +import org.opensearch.cluster.metadata.IndexMetadata; +import org.opensearch.cluster.metadata.Metadata; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.security.configuration.ConfigurationRepository; +import org.opensearch.security.privileges.PrivilegesEvaluator; +import org.opensearch.security.securityconf.FlattenedActionGroups; +import org.opensearch.security.securityconf.impl.CType; +import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; +import org.opensearch.security.securityconf.impl.v7.ActionGroupsV7; +import org.opensearch.security.securityconf.impl.v7.RoleV7; +import org.opensearch.threadpool.ThreadPool; + +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.is; +import static org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration.fromMap; import static org.junit.Assert.assertThrows; -import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +@RunWith(MockitoJUnitRunner.class) public class ApiTokenActionTest { + @Mock + private ThreadPool threadPool; + + @Mock + private PrivilegesEvaluator privilegesEvaluator; + + @Mock + private ConfigurationRepository configurationRepository; + + @Mock + private ClusterService clusterService; + @Mock + private ClusterState clusterState; + + @Mock + private Metadata metadata; + + private SecurityDynamicConfiguration actionGroupsConfig; + private SecurityDynamicConfiguration rolesConfig; + private FlattenedActionGroups flattenedActionGroups; + private ApiTokenAction apiTokenAction; + + @Before + public void setUp() throws JsonProcessingException { + // Setup basic action groups + + actionGroupsConfig = SecurityDynamicConfiguration.fromMap( + ImmutableMap.of( + "read_group", + Map.of("allowed_actions", List.of("read", "get", "search")), + "write_group", + Map.of("allowed_actions", List.of("write", "create", "index")) + ), + CType.ACTIONGROUPS + ); + + rolesConfig = fromMap( + ImmutableMap.of( + "read_group_logs-123", + ImmutableMap.of( + "index_permissions", + Arrays.asList(ImmutableMap.of("index_patterns", List.of("logs-123"), "allowed_actions", List.of("read_group"))), + "cluster_permissions", + Arrays.asList("*") + ), + "read_group_logs-star", + ImmutableMap.of( + "index_permissions", + Arrays.asList(ImmutableMap.of("index_patterns", List.of("logs-*"), "allowed_actions", List.of("read_group"))), + "cluster_permissions", + Arrays.asList("*") + ), + "write_group_logs-star", + ImmutableMap.of( + "index_permissions", + Arrays.asList(ImmutableMap.of("index_patterns", List.of("logs-*"), "allowed_actions", List.of("write_group"))), + "cluster_permissions", + Arrays.asList("*") + ), + "write_group_logs-123", + ImmutableMap.of( + "index_permissions", + Arrays.asList(ImmutableMap.of("index_patterns", List.of("logs-123"), "allowed_actions", List.of("write_group"))), + "cluster_permissions", + Arrays.asList("*") + ), + "more_permissable_write_group_lo-star", + ImmutableMap.of( + "index_permissions", + Arrays.asList(ImmutableMap.of("index_patterns", List.of("lo*"), "allowed_actions", List.of("write_group"))), + "cluster_permissions", + Arrays.asList("*") + ), + "cluster_monitor", + ImmutableMap.of( + "index_permissions", + Arrays.asList(ImmutableMap.of("index_patterns", List.of("lo*"), "allowed_actions", List.of("write_group"))), + "cluster_permissions", + Arrays.asList("cluster_monitor") + ), + "alias_group", + ImmutableMap.of( + "index_permissions", + Arrays.asList(ImmutableMap.of("index_patterns", List.of("logs"), "allowed_actions", List.of("read"))), + "cluster_permissions", + Arrays.asList("cluster_monitor") + ) + + ), + CType.ROLES + ); + + when(threadPool.getThreadContext()).thenReturn(new ThreadContext(Settings.EMPTY)); + + when(configurationRepository.getConfiguration(CType.ROLES)).thenReturn(rolesConfig); + when(configurationRepository.getConfiguration(CType.ACTIONGROUPS)).thenReturn(actionGroupsConfig); + when(clusterService.state()).thenReturn(clusterState); + + when(clusterState.metadata()).thenReturn( + Metadata.builder() + .put( + IndexMetadata.builder("my-index") + .settings(Settings.builder().put("index.version.created", Version.CURRENT).build()) + .numberOfShards(1) + .numberOfReplicas(0) + .putAlias(AliasMetadata.builder("logs").build()) + ) + .build() + ); + + apiTokenAction = new ApiTokenAction( + + threadPool, + configurationRepository, + privilegesEvaluator, + Settings.EMPTY, + null, + null, + null, + null, + null, + clusterService, + null + ); - private final ApiTokenAction apiTokenAction = new ApiTokenAction(mock(ApiTokenRepository.class)); + } @Test public void testCreateIndexPermission() { @@ -101,4 +253,107 @@ public void testExtractClusterPermissions() { requestBody.put("cluster_permissions", Arrays.asList("perm1", "perm2")); assertThat(apiTokenAction.extractClusterPermissions(requestBody), is(Arrays.asList("perm1", "perm2"))); } + + @Test + public void testExactMatchPermissionsWithActionGroups() throws Exception { + when(privilegesEvaluator.mapRoles(null, null)).thenReturn(Set.of("read_group_logs-123")); + + apiTokenAction.validateUserPermissions(List.of(), List.of(new ApiToken.IndexPermission( + List.of("logs-123"), + List.of("read_group") + ))); + } + + @Test + public void testCreateWildcardPermissionWhenNoAccessThrowsException() { + when(privilegesEvaluator.mapRoles(null, null)).thenReturn(Set.of("read_group_logs-123")); + + assertThrows(OpenSearchException.class, () -> apiTokenAction.validateUserPermissions(List.of(), List.of(new ApiToken.IndexPermission(List.of("logs-*"), List.of("read"))))); + } + + @Test + public void testCreateMorePermissableWildcardPermissionWhenNoAccessThrowsException() { + when(privilegesEvaluator.mapRoles(null, null)).thenReturn(Set.of("write_group_logs-star")); + + assertThrows(OpenSearchException.class, () -> apiTokenAction.validateUserPermissions(List.of(), List.of(new ApiToken.IndexPermission(List.of("lo-*"), List.of("write"))))); + } + + @Test + public void testMultipleRolesCoveringPermissions() throws Exception { + when(privilegesEvaluator.mapRoles(null, null)).thenReturn(Set.of("read_group_logs-123", "write_group_logs-123")); + + ApiToken.IndexPermission requestedPerm = new ApiToken.IndexPermission(List.of("logs-123"), List.of("read", "write")); + + apiTokenAction.validateUserPermissions(List.of(), List.of(requestedPerm)); + } + + @Test + public void testInsufficientPermissions() { + when(privilegesEvaluator.mapRoles(null, null)).thenReturn(Set.of("read_group_logs-star")); + + ApiToken.IndexPermission requestedPerm = new ApiToken.IndexPermission(List.of("logs-2023"), List.of("read", "write")); + + assertThrows(OpenSearchException.class, () -> apiTokenAction.validateUserPermissions(List.of(), List.of(requestedPerm))); + } + + @Test + public void testSeparateIndexPatternThrowsException() { + when(privilegesEvaluator.mapRoles(null, null)).thenReturn(Set.of("read_group_logs-123")); + + ApiToken.IndexPermission requestedPerm = new ApiToken.IndexPermission(List.of("logs-123", "metrics-2023"), List.of("read")); + + assertThrows(OpenSearchException.class, () -> apiTokenAction.validateUserPermissions(List.of(), List.of(requestedPerm))); + } + + @Test + public void testActionGroupResolution() throws Exception{ + when(privilegesEvaluator.mapRoles(null, null)).thenReturn(Set.of("read_group_logs-123", "write_group_logs-123")); + + ApiToken.IndexPermission requestedPerm = new ApiToken.IndexPermission( + List.of("logs-123"), + List.of("read", "write", "get", "create") + ); + + apiTokenAction.validateUserPermissions(List.of(), List.of(requestedPerm)); + } + + @Test + public void testEmptyIndexPermissions() throws Exception { + when(privilegesEvaluator.mapRoles(null, null)).thenReturn(Set.of("read_group_logs-123", "write_group_logs-123")); + + apiTokenAction.validateUserPermissions(List.of("cluster:monitor"), List.of()); + } + + @Test + public void testClusterPermissionsEvaluation() throws Exception { + when(privilegesEvaluator.mapRoles(null, null)).thenReturn(Set.of("cluster_monitor")); + + apiTokenAction.validateUserPermissions(List.of("cluster_monitor"), List.of()); + } + + @Test + public void testClusterPermissionsMorePermissableRegexThrowsException() { + when(privilegesEvaluator.mapRoles(null, null)).thenReturn(Set.of("cluster_monitor")); + + assertThrows(OpenSearchException.class, () -> apiTokenAction.validateUserPermissions(List.of("*"), List.of())); + } + + @Test + public void testClusterPermissionsFromMultipleRoles() throws Exception{ + when(privilegesEvaluator.mapRoles(null, null)).thenReturn(Set.of("cluster_monitor", "read_group_logs-123")); + + apiTokenAction.validateUserPermissions(List.of("cluster_monitor", "cluster_health"), List.of()); + } + + @Test + public void testAliasAllowsAccessOnUnderlyingIndices() throws Exception { + when(privilegesEvaluator.mapRoles(null, null)).thenReturn(Set.of("alias_group")); + + ApiToken.IndexPermission requestedPerm = new ApiToken.IndexPermission( + List.of("my-index"), + List.of("read") + ); + + apiTokenAction.validateUserPermissions(List.of(), List.of(requestedPerm)); + } } diff --git a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenAuthenticatorTest.java b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenAuthenticatorTest.java index c24c632fa0..b6c5e0b0f1 100644 --- a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenAuthenticatorTest.java +++ b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenAuthenticatorTest.java @@ -48,6 +48,7 @@ public class ApiTokenAuthenticatorTest { private ApiTokenAuthenticator authenticator; @Mock private Logger log; + @Mock private ApiTokenRepository apiTokenRepository;