From 0a708f9964c67914ee8c04a15d9e4ca3e069b160 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Fri, 15 Nov 2024 15:58:06 -0500 Subject: [PATCH 01/34] Add api token action to handle creation of api token index Signed-off-by: Derek Ho --- .../security/auth/BackendRegistry.java | 8 ++ .../dlic/rest/api/ApiTokenApiAction.java | 114 ++++++++++++++++++ .../security/dlic/rest/api/Endpoint.java | 1 + .../dlic/rest/api/SecurityRestApiActions.java | 1 + .../securityconf/DynamicConfigModel.java | 5 +- .../securityconf/DynamicConfigModelV7.java | 18 +++ .../securityconf/impl/v7/ConfigV7.java | 48 ++++++++ .../security/support/ConfigConstants.java | 2 + 8 files changed, 196 insertions(+), 1 deletion(-) create mode 100644 src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenApiAction.java diff --git a/src/main/java/org/opensearch/security/auth/BackendRegistry.java b/src/main/java/org/opensearch/security/auth/BackendRegistry.java index 0b00bcf943..9697e9587e 100644 --- a/src/main/java/org/opensearch/security/auth/BackendRegistry.java +++ b/src/main/java/org/opensearch/security/auth/BackendRegistry.java @@ -103,6 +103,7 @@ public class BackendRegistry { private Cache userCache; // rest standard private Cache restImpersonationCache; // used for rest impersonation private Cache> restRoleCache; // + private Cache apiTokensCache; private void createCaches() { userCache = CacheBuilder.newBuilder() @@ -135,6 +136,12 @@ public void onRemoval(RemovalNotification> notification) { }) .build(); + apiTokensCache = CacheBuilder.newBuilder() + .expireAfterWrite(ttlInMin, TimeUnit.MINUTES) + .removalListener((RemovalListener) notification -> log.debug("Clear api token cache for {} due to {}", notification.getKey(), notification.getCause())) + .build(); + + } public BackendRegistry( @@ -170,6 +177,7 @@ public void invalidateCache() { userCache.invalidateAll(); restImpersonationCache.invalidateAll(); restRoleCache.invalidateAll(); + apiTokensCache.invalidateAll(); } @Subscribe diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenApiAction.java new file mode 100644 index 0000000000..c086e69142 --- /dev/null +++ b/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenApiAction.java @@ -0,0 +1,114 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.dlic.rest.api; + +import com.fasterxml.jackson.databind.node.ObjectNode; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import org.opensearch.action.admin.indices.create.CreateIndexRequest; +import org.opensearch.action.index.IndexResponse; +import org.opensearch.client.Client; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.settings.Settings; +import org.opensearch.core.rest.RestStatus; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.security.DefaultObjectMapper; +import org.opensearch.security.dlic.rest.validation.EndpointValidator; +import org.opensearch.security.dlic.rest.validation.RequestContentValidator; +import org.opensearch.security.dlic.rest.validation.RequestContentValidator.DataType; +import org.opensearch.security.dlic.rest.validation.ValidationResult; +import org.opensearch.security.securityconf.impl.CType; +import org.opensearch.security.securityconf.impl.v7.ConfigV7; +import org.opensearch.security.support.ConfigConstants; +import org.opensearch.security.support.SecurityJsonNode; +import org.opensearch.threadpool.ThreadPool; + +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import static org.opensearch.rest.RestRequest.Method.*; +import static org.opensearch.security.dlic.rest.api.RateLimitersApiAction.NAME_JSON_PROPERTY; +import static org.opensearch.security.dlic.rest.api.Responses.*; +import static org.opensearch.security.dlic.rest.support.Utils.addRoutesPrefix; +import static org.opensearch.security.securityconf.impl.v7.ConfigV7.*; + +public class ApiTokenApiAction extends AbstractApiAction { + + public static final String NAME_JSON_PROPERTY = "ip"; + + + private static final List ROUTES = addRoutesPrefix( + ImmutableList.of( + new Route(GET, "/apitokens"), + new Route(PUT, "/apitokens/{name}") +// new Route(DELETE, "/apitokens/{name}"), + ) + ); + + protected ApiTokenApiAction(ClusterService clusterService, ThreadPool threadPool, SecurityApiDependencies securityApiDependencies) { + super(Endpoint.APITOKENS, clusterService, threadPool, securityApiDependencies); + this.requestHandlersBuilder.configureRequestHandlers(this::authFailureConfigApiRequestHandlers); + } + + @Override + public String getName() { + return "API Token actions to retrieve / update configs."; + } + + @Override + public List routes() { + return ROUTES; + } + + @Override + protected CType getConfigType() { + return CType.CONFIG; + } + + private void authFailureConfigApiRequestHandlers(RequestHandler.RequestHandlersBuilder requestHandlersBuilder) { + + requestHandlersBuilder.override( + GET, + (channel, request, client) -> loadConfiguration(getConfigType(), false, false).valid(configuration -> { + if (!apiTokenIndexExists()) { + ok(channel, "empty list"); + } else { + ok(channel, "non-empty list"); + } + }).error((status, toXContent) -> response(channel, status, toXContent))) + .override(PUT, (channel, request, client) -> loadConfiguration(getConfigType(), false, false).valid(configuration -> { + String token = createApiToken(request.param(NAME_JSON_PROPERTY), client); + ok(channel, token + " created successfully"); + }).error((status, toXContent) -> response(channel, status, toXContent))); + + } + + public String createApiToken(String name, Client client) { + createApiTokenIndexIfAbsent(client); + + return "test-token"; + } + + public Boolean apiTokenIndexExists() { + return clusterService.state().metadata().hasConcreteIndex(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX); + } + + public void createApiTokenIndexIfAbsent(Client client) { + if (!apiTokenIndexExists()) { + 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); + logger.info(client.admin().indices().create(createIndexRequest).actionGet().isAcknowledged()); + } + } +} 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..afed070a78 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 @@ -25,6 +25,7 @@ public enum Endpoint { AUTHTOKEN, TENANTS, RATELIMITERS, + APITOKENS, MIGRATE, VALIDATE, WHITELIST, diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java b/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java index c28a1bdc1d..f1dfed03f3 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java @@ -96,6 +96,7 @@ public static Collection getHandler( new AuditApiAction(clusterService, threadPool, securityApiDependencies), new MultiTenancyConfigApiAction(clusterService, threadPool, securityApiDependencies), new RateLimitersApiAction(clusterService, threadPool, securityApiDependencies), + new ApiTokenApiAction(clusterService, threadPool, securityApiDependencies), new ConfigUpgradeApiAction(clusterService, threadPool, securityApiDependencies), new SecuritySSLCertsApiAction( clusterService, diff --git a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModel.java b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModel.java index 064f555a75..0c4737792c 100644 --- a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModel.java +++ b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModel.java @@ -110,6 +110,10 @@ public abstract class DynamicConfigModel { public abstract Settings getDynamicOnBehalfOfSettings(); + public abstract Settings getDynamicApiTokenSettings(); + + + protected final Map authImplMap = new HashMap<>(); public DynamicConfigModel() { @@ -142,5 +146,4 @@ public DynamicConfigModel() { authImplMap.put("ip_authFailureListener", AddressBasedRateLimiter.class.getName()); authImplMap.put("username_authFailureListener", UserNameBasedRateLimiter.class.getName()); } - } diff --git a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java index 4bc9e82882..0d985d4d62 100644 --- a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java +++ b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java @@ -234,6 +234,13 @@ public Settings getDynamicOnBehalfOfSettings() { .build(); } + @Override + public Settings getDynamicApiTokenSettings() { + return Settings.builder() + .put(Settings.builder().loadFromSource(config.dynamic.api_tokens.configAsJson(), XContentType.JSON).build()) + .build(); + } + private void buildAAA() { final SortedSet restAuthDomains0 = new TreeSet<>(); @@ -387,6 +394,17 @@ private void buildAAA() { restAuthDomains0.add(_ad); } + Settings apiTokenSettings = getDynamicApiTokenSettings(); + if (!isKeyNull(apiTokenSettings, "signing_key") && !isKeyNull(apiTokenSettings, "encryption_key")) { + final AuthDomain _ad = new AuthDomain( + new NoOpAuthenticationBackend(Settings.EMPTY, null), + new OnBehalfOfAuthenticator(getDynamicOnBehalfOfSettings(), this.cih.getClusterName()), + false, + -1 + ); + restAuthDomains0.add(_ad); + } + List originalDestroyableComponents = destroyableComponents; restAuthDomains = Collections.unmodifiableSortedSet(restAuthDomains0); diff --git a/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java b/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java index 77fb973a52..e5152e4cdf 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java @@ -86,6 +86,7 @@ public static class Dynamic { public String transport_userrname_attribute; public boolean do_not_fail_on_forbidden_empty; public OnBehalfOfSettings on_behalf_of = new OnBehalfOfSettings(); + public ApiTokenSettings api_tokens = new ApiTokenSettings(); @Override public String toString() { @@ -495,4 +496,51 @@ public String toString() { } } + public static class ApiTokenSettings { + @JsonProperty("enabled") + private Boolean apiTokenEnabled = Boolean.FALSE; + @JsonProperty("signing_key") + private String signingKey; + @JsonProperty("encryption_key") + private String encryptionKey; + + @JsonIgnore + public String configAsJson() { + try { + return DefaultObjectMapper.writeValueAsString(this, false); + } catch (JsonProcessingException e) { + throw new RuntimeException(e); + } + } + + public Boolean getApiTokenEnabled() { + return apiTokenEnabled; + } + + public void setOboEnabled(Boolean apiTokenEnabled) { + this.apiTokenEnabled = apiTokenEnabled; + } + + public String getSigningKey() { + return signingKey; + } + + public void setSigningKey(String signingKey) { + this.signingKey = signingKey; + } + + public String getEncryptionKey() { + return encryptionKey; + } + + public void setEncryptionKey(String encryptionKey) { + this.encryptionKey = encryptionKey; + } + + @Override + public String toString() { + return "ApiTokenSettings [ enabled=" + apiTokenEnabled + ", signing_key=" + signingKey + ", encryption_key=" + encryptionKey + "]"; + } + } + } diff --git a/src/main/java/org/opensearch/security/support/ConfigConstants.java b/src/main/java/org/opensearch/security/support/ConfigConstants.java index f35afc6489..2a3b898bdf 100644 --- a/src/main/java/org/opensearch/security/support/ConfigConstants.java +++ b/src/main/java/org/opensearch/security/support/ConfigConstants.java @@ -370,6 +370,8 @@ public enum RolesMappingResolution { // Variable for initial admin password support public static final String OPENSEARCH_INITIAL_ADMIN_PASSWORD = "OPENSEARCH_INITIAL_ADMIN_PASSWORD"; + public static final String OPENSEARCH_API_TOKENS_INDEX = ".opensearch_security_api_tokens"; + public static Set getSettingAsSet( final Settings settings, final String key, From 3a164245b47c7bd4958b7d9e7d03b57b6c0fa75d Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Fri, 15 Nov 2024 16:12:12 -0500 Subject: [PATCH 02/34] spotless apply and checkstyle Signed-off-by: Derek Ho --- .../security/auth/BackendRegistry.java | 13 +++-- .../dlic/rest/api/ApiTokenApiAction.java | 49 +++++++------------ .../securityconf/DynamicConfigModel.java | 2 - .../securityconf/DynamicConfigModelV7.java | 12 ++--- .../securityconf/impl/v7/ConfigV7.java | 8 ++- 5 files changed, 40 insertions(+), 44 deletions(-) diff --git a/src/main/java/org/opensearch/security/auth/BackendRegistry.java b/src/main/java/org/opensearch/security/auth/BackendRegistry.java index 9697e9587e..52a3e44276 100644 --- a/src/main/java/org/opensearch/security/auth/BackendRegistry.java +++ b/src/main/java/org/opensearch/security/auth/BackendRegistry.java @@ -137,10 +137,15 @@ public void onRemoval(RemovalNotification> notification) { .build(); apiTokensCache = CacheBuilder.newBuilder() - .expireAfterWrite(ttlInMin, TimeUnit.MINUTES) - .removalListener((RemovalListener) notification -> log.debug("Clear api token cache for {} due to {}", notification.getKey(), notification.getCause())) - .build(); - + .expireAfterWrite(ttlInMin, TimeUnit.MINUTES) + .removalListener( + (RemovalListener) notification -> log.debug( + "Clear api token cache for {} due to {}", + notification.getKey(), + notification.getCause() + ) + ) + .build(); } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenApiAction.java index c086e69142..eb209ad347 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenApiAction.java @@ -11,48 +11,33 @@ package org.opensearch.security.dlic.rest.api; -import com.fasterxml.jackson.databind.node.ObjectNode; +import java.util.List; +import java.util.Map; + import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; + import org.opensearch.action.admin.indices.create.CreateIndexRequest; -import org.opensearch.action.index.IndexResponse; import org.opensearch.client.Client; import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.settings.Settings; -import org.opensearch.core.rest.RestStatus; -import org.opensearch.core.xcontent.ToXContent; -import org.opensearch.security.DefaultObjectMapper; -import org.opensearch.security.dlic.rest.validation.EndpointValidator; -import org.opensearch.security.dlic.rest.validation.RequestContentValidator; -import org.opensearch.security.dlic.rest.validation.RequestContentValidator.DataType; -import org.opensearch.security.dlic.rest.validation.ValidationResult; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.securityconf.impl.v7.ConfigV7; import org.opensearch.security.support.ConfigConstants; -import org.opensearch.security.support.SecurityJsonNode; import org.opensearch.threadpool.ThreadPool; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.Set; - -import static org.opensearch.rest.RestRequest.Method.*; -import static org.opensearch.security.dlic.rest.api.RateLimitersApiAction.NAME_JSON_PROPERTY; -import static org.opensearch.security.dlic.rest.api.Responses.*; +import static org.opensearch.rest.RestRequest.Method.GET; +import static org.opensearch.rest.RestRequest.Method.PUT; +import static org.opensearch.security.dlic.rest.api.Responses.ok; +import static org.opensearch.security.dlic.rest.api.Responses.response; import static org.opensearch.security.dlic.rest.support.Utils.addRoutesPrefix; -import static org.opensearch.security.securityconf.impl.v7.ConfigV7.*; public class ApiTokenApiAction extends AbstractApiAction { public static final String NAME_JSON_PROPERTY = "ip"; - private static final List ROUTES = addRoutesPrefix( - ImmutableList.of( - new Route(GET, "/apitokens"), - new Route(PUT, "/apitokens/{name}") -// new Route(DELETE, "/apitokens/{name}"), + ImmutableList.of(new Route(GET, "/apitokens"), new Route(PUT, "/apitokens/{name}") + // new Route(DELETE, "/apitokens/{name}"), ) ); @@ -86,11 +71,11 @@ private void authFailureConfigApiRequestHandlers(RequestHandler.RequestHandlersB } else { ok(channel, "non-empty list"); } - }).error((status, toXContent) -> response(channel, status, toXContent))) - .override(PUT, (channel, request, client) -> loadConfiguration(getConfigType(), false, false).valid(configuration -> { - String token = createApiToken(request.param(NAME_JSON_PROPERTY), client); - ok(channel, token + " created successfully"); - }).error((status, toXContent) -> response(channel, status, toXContent))); + }).error((status, toXContent) -> response(channel, status, toXContent)) + ).override(PUT, (channel, request, client) -> loadConfiguration(getConfigType(), false, false).valid(configuration -> { + String token = createApiToken(request.param(NAME_JSON_PROPERTY), client); + ok(channel, token + " created successfully"); + }).error((status, toXContent) -> response(channel, status, toXContent))); } @@ -107,7 +92,9 @@ public Boolean apiTokenIndexExists() { public void createApiTokenIndexIfAbsent(Client client) { if (!apiTokenIndexExists()) { 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); + final CreateIndexRequest createIndexRequest = new CreateIndexRequest(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX).settings( + indexSettings + ); logger.info(client.admin().indices().create(createIndexRequest).actionGet().isAcknowledged()); } } diff --git a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModel.java b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModel.java index 0c4737792c..a6f1789150 100644 --- a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModel.java +++ b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModel.java @@ -112,8 +112,6 @@ public abstract class DynamicConfigModel { public abstract Settings getDynamicApiTokenSettings(); - - protected final Map authImplMap = new HashMap<>(); public DynamicConfigModel() { diff --git a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java index 0d985d4d62..3d1d48ca94 100644 --- a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java +++ b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java @@ -237,8 +237,8 @@ public Settings getDynamicOnBehalfOfSettings() { @Override public Settings getDynamicApiTokenSettings() { return Settings.builder() - .put(Settings.builder().loadFromSource(config.dynamic.api_tokens.configAsJson(), XContentType.JSON).build()) - .build(); + .put(Settings.builder().loadFromSource(config.dynamic.api_tokens.configAsJson(), XContentType.JSON).build()) + .build(); } private void buildAAA() { @@ -397,10 +397,10 @@ private void buildAAA() { Settings apiTokenSettings = getDynamicApiTokenSettings(); if (!isKeyNull(apiTokenSettings, "signing_key") && !isKeyNull(apiTokenSettings, "encryption_key")) { final AuthDomain _ad = new AuthDomain( - new NoOpAuthenticationBackend(Settings.EMPTY, null), - new OnBehalfOfAuthenticator(getDynamicOnBehalfOfSettings(), this.cih.getClusterName()), - false, - -1 + new NoOpAuthenticationBackend(Settings.EMPTY, null), + new OnBehalfOfAuthenticator(getDynamicOnBehalfOfSettings(), this.cih.getClusterName()), + false, + -1 ); restAuthDomains0.add(_ad); } diff --git a/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java b/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java index e5152e4cdf..c9d431bb06 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java @@ -539,7 +539,13 @@ public void setEncryptionKey(String encryptionKey) { @Override public String toString() { - return "ApiTokenSettings [ enabled=" + apiTokenEnabled + ", signing_key=" + signingKey + ", encryption_key=" + encryptionKey + "]"; + return "ApiTokenSettings [ enabled=" + + apiTokenEnabled + + ", signing_key=" + + signingKey + + ", encryption_key=" + + encryptionKey + + "]"; } } From 064ffa4a45fdfc13ae7a3515fa941567f6a88a9d Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Fri, 15 Nov 2024 16:58:44 -0500 Subject: [PATCH 03/34] Stash context Signed-off-by: Derek Ho --- .../dlic/rest/api/ApiTokenApiAction.java | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenApiAction.java index eb209ad347..df1e96c6e0 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenApiAction.java @@ -20,6 +20,7 @@ import org.opensearch.action.admin.indices.create.CreateIndexRequest; import org.opensearch.client.Client; import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.securityconf.impl.v7.ConfigV7; import org.opensearch.security.support.ConfigConstants; @@ -33,7 +34,7 @@ public class ApiTokenApiAction extends AbstractApiAction { - public static final String NAME_JSON_PROPERTY = "ip"; + public static final String NAME_JSON_PROPERTY = "name"; private static final List ROUTES = addRoutesPrefix( ImmutableList.of(new Route(GET, "/apitokens"), new Route(PUT, "/apitokens/{name}") @@ -43,7 +44,7 @@ public class ApiTokenApiAction extends AbstractApiAction { protected ApiTokenApiAction(ClusterService clusterService, ThreadPool threadPool, SecurityApiDependencies securityApiDependencies) { super(Endpoint.APITOKENS, clusterService, threadPool, securityApiDependencies); - this.requestHandlersBuilder.configureRequestHandlers(this::authFailureConfigApiRequestHandlers); + this.requestHandlersBuilder.configureRequestHandlers(this::apiTokenApiRequestHandlers); } @Override @@ -61,7 +62,7 @@ protected CType getConfigType() { return CType.CONFIG; } - private void authFailureConfigApiRequestHandlers(RequestHandler.RequestHandlersBuilder requestHandlersBuilder) { + private void apiTokenApiRequestHandlers(RequestHandler.RequestHandlersBuilder requestHandlersBuilder) { requestHandlersBuilder.override( GET, @@ -91,11 +92,18 @@ public Boolean apiTokenIndexExists() { public void createApiTokenIndexIfAbsent(Client client) { if (!apiTokenIndexExists()) { - 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 - ); - logger.info(client.admin().indices().create(createIndexRequest).actionGet().isAcknowledged()); + try (final ThreadContext.StoredContext ctx = client.threadPool().getThreadContext().stashContext()) { + 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 + ); + logger.info(client.admin().indices().create(createIndexRequest).actionGet().isAcknowledged()); + } } } } From 65532ca5622147356b5181292a5cff87d3f46933 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Mon, 18 Nov 2024 11:58:31 -0500 Subject: [PATCH 04/34] Add TODO comments Signed-off-by: Derek Ho --- .../security/auth/BackendRegistry.java | 1 + .../dlic/rest/api/ApiTokenApiAction.java | 24 +++++++++++++++---- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/opensearch/security/auth/BackendRegistry.java b/src/main/java/org/opensearch/security/auth/BackendRegistry.java index 52a3e44276..163876b6df 100644 --- a/src/main/java/org/opensearch/security/auth/BackendRegistry.java +++ b/src/main/java/org/opensearch/security/auth/BackendRegistry.java @@ -136,6 +136,7 @@ public void onRemoval(RemovalNotification> notification) { }) .build(); + /* TODO: Listen in to index events on API Tokens index */ apiTokensCache = CacheBuilder.newBuilder() .expireAfterWrite(ttlInMin, TimeUnit.MINUTES) .removalListener( diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenApiAction.java index df1e96c6e0..03bc932027 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenApiAction.java @@ -26,8 +26,10 @@ 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; import static org.opensearch.rest.RestRequest.Method.PUT; +import static org.opensearch.security.dlic.rest.api.Responses.internalServerError; import static org.opensearch.security.dlic.rest.api.Responses.ok; import static org.opensearch.security.dlic.rest.api.Responses.response; import static org.opensearch.security.dlic.rest.support.Utils.addRoutesPrefix; @@ -37,9 +39,7 @@ public class ApiTokenApiAction extends AbstractApiAction { public static final String NAME_JSON_PROPERTY = "name"; private static final List ROUTES = addRoutesPrefix( - ImmutableList.of(new Route(GET, "/apitokens"), new Route(PUT, "/apitokens/{name}") - // new Route(DELETE, "/apitokens/{name}"), - ) + ImmutableList.of(new Route(GET, "/apitokens"), new Route(PUT, "/apitokens/{name}"), new Route(DELETE, "/apitokens/{name}")) ); protected ApiTokenApiAction(ClusterService clusterService, ThreadPool threadPool, SecurityApiDependencies securityApiDependencies) { @@ -58,6 +58,7 @@ public List routes() { } @Override + /* TODO: Alternative to CType for new index */ protected CType getConfigType() { return CType.CONFIG; } @@ -68,21 +69,34 @@ private void apiTokenApiRequestHandlers(RequestHandler.RequestHandlersBuilder re GET, (channel, request, client) -> loadConfiguration(getConfigType(), false, false).valid(configuration -> { if (!apiTokenIndexExists()) { + /** TODO: Return empty list */ ok(channel, "empty list"); } else { + /** TODO: Return list of documents from API Tokens index */ ok(channel, "non-empty list"); } }).error((status, toXContent) -> response(channel, status, toXContent)) ).override(PUT, (channel, request, client) -> loadConfiguration(getConfigType(), false, false).valid(configuration -> { String token = createApiToken(request.param(NAME_JSON_PROPERTY), client); ok(channel, token + " created successfully"); - }).error((status, toXContent) -> response(channel, status, toXContent))); + }).error((status, toXContent) -> response(channel, status, toXContent))) + .override(DELETE, (channel, request, client) -> loadConfiguration(getConfigType(), false, false).valid(configuration -> { + String token = createApiToken(request.param(NAME_JSON_PROPERTY), client); + if (!apiTokenIndexExists()) { + /** TODO: Return error indicating token doesn't exist */ + internalServerError(channel, token + " doesn't exist"); + } else { + /** TODO: Delete document from API Tokens index if it exists, otherwise return error */ + ok(channel, token + " deleted successfully"); + } + + }).error((status, toXContent) -> response(channel, status, toXContent))); } public String createApiToken(String name, Client client) { createApiTokenIndexIfAbsent(client); - + /* TODO: Create document representing the API token */ return "test-token"; } From a56163d7bc0a83441975cec8ab3c4ec498f050f5 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Mon, 18 Nov 2024 12:17:03 -0500 Subject: [PATCH 05/34] Remove dynamic changes for now Signed-off-by: Derek Ho --- .../securityconf/DynamicConfigModel.java | 2 - .../securityconf/DynamicConfigModelV7.java | 18 ------- .../securityconf/impl/v7/ConfigV7.java | 54 ------------------- 3 files changed, 74 deletions(-) diff --git a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModel.java b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModel.java index a6f1789150..422391dcaf 100644 --- a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModel.java +++ b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModel.java @@ -110,8 +110,6 @@ public abstract class DynamicConfigModel { public abstract Settings getDynamicOnBehalfOfSettings(); - public abstract Settings getDynamicApiTokenSettings(); - protected final Map authImplMap = new HashMap<>(); public DynamicConfigModel() { diff --git a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java index 3d1d48ca94..4bc9e82882 100644 --- a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java +++ b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java @@ -234,13 +234,6 @@ public Settings getDynamicOnBehalfOfSettings() { .build(); } - @Override - public Settings getDynamicApiTokenSettings() { - return Settings.builder() - .put(Settings.builder().loadFromSource(config.dynamic.api_tokens.configAsJson(), XContentType.JSON).build()) - .build(); - } - private void buildAAA() { final SortedSet restAuthDomains0 = new TreeSet<>(); @@ -394,17 +387,6 @@ private void buildAAA() { restAuthDomains0.add(_ad); } - Settings apiTokenSettings = getDynamicApiTokenSettings(); - if (!isKeyNull(apiTokenSettings, "signing_key") && !isKeyNull(apiTokenSettings, "encryption_key")) { - final AuthDomain _ad = new AuthDomain( - new NoOpAuthenticationBackend(Settings.EMPTY, null), - new OnBehalfOfAuthenticator(getDynamicOnBehalfOfSettings(), this.cih.getClusterName()), - false, - -1 - ); - restAuthDomains0.add(_ad); - } - List originalDestroyableComponents = destroyableComponents; restAuthDomains = Collections.unmodifiableSortedSet(restAuthDomains0); diff --git a/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java b/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java index c9d431bb06..77fb973a52 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java @@ -86,7 +86,6 @@ public static class Dynamic { public String transport_userrname_attribute; public boolean do_not_fail_on_forbidden_empty; public OnBehalfOfSettings on_behalf_of = new OnBehalfOfSettings(); - public ApiTokenSettings api_tokens = new ApiTokenSettings(); @Override public String toString() { @@ -496,57 +495,4 @@ public String toString() { } } - public static class ApiTokenSettings { - @JsonProperty("enabled") - private Boolean apiTokenEnabled = Boolean.FALSE; - @JsonProperty("signing_key") - private String signingKey; - @JsonProperty("encryption_key") - private String encryptionKey; - - @JsonIgnore - public String configAsJson() { - try { - return DefaultObjectMapper.writeValueAsString(this, false); - } catch (JsonProcessingException e) { - throw new RuntimeException(e); - } - } - - public Boolean getApiTokenEnabled() { - return apiTokenEnabled; - } - - public void setOboEnabled(Boolean apiTokenEnabled) { - this.apiTokenEnabled = apiTokenEnabled; - } - - public String getSigningKey() { - return signingKey; - } - - public void setSigningKey(String signingKey) { - this.signingKey = signingKey; - } - - public String getEncryptionKey() { - return encryptionKey; - } - - public void setEncryptionKey(String encryptionKey) { - this.encryptionKey = encryptionKey; - } - - @Override - public String toString() { - return "ApiTokenSettings [ enabled=" - + apiTokenEnabled - + ", signing_key=" - + signingKey - + ", encryption_key=" - + encryptionKey - + "]"; - } - } - } From 72f2838047dd5a5838408df5854886acc084edf2 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Tue, 19 Nov 2024 11:22:48 -0500 Subject: [PATCH 06/34] Add a indexmanager to handle document creation Signed-off-by: Derek Ho --- .../security/dlic/rest/api/ApiToken.java | 78 +++++++++++++++++++ .../dlic/rest/api/ApiTokenApiAction.java | 2 +- .../dlic/rest/api/ApiTokenIndexManager.java | 59 ++++++++++++++ 3 files changed, 138 insertions(+), 1 deletion(-) create mode 100644 src/main/java/org/opensearch/security/dlic/rest/api/ApiToken.java create mode 100644 src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenIndexManager.java diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/ApiToken.java b/src/main/java/org/opensearch/security/dlic/rest/api/ApiToken.java new file mode 100644 index 0000000000..e54a4f7f85 --- /dev/null +++ b/src/main/java/org/opensearch/security/dlic/rest/api/ApiToken.java @@ -0,0 +1,78 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.dlic.rest.api; + +import java.io.IOException; +import java.time.Instant; +import java.util.List; + +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.XContentBuilder; + +public class ApiToken implements ToXContent { + private String description; + private String jti; + + private Instant creationTime; + private List roles; + + public ApiToken(String description, String jti, List roles) { + this.creationTime = Instant.now(); + this.description = description; + this.jti = jti; + this.roles = roles; + + } + + public String getDescription() { + return description; + } + + public void setDescription(String description) { + this.description = description; + } + + public String getJti() { + return jti; + } + + public void setJti(String jti) { + this.jti = jti; + } + + public Instant getCreationTime() { + return creationTime; + } + + public void setCreationTime(Instant creationTime) { + this.creationTime = creationTime; + } + + public List getRoles() { + return roles; + } + + public void setRoles(List roles) { + this.roles = roles; + } + + @Override + public XContentBuilder toXContent(XContentBuilder xContentBuilder, ToXContent.Params params) throws IOException { + xContentBuilder.startObject(); + xContentBuilder.field("description", description); + xContentBuilder.field("jti", jti); + xContentBuilder.field("roles", roles); + xContentBuilder.field("creation_time", creationTime.toEpochMilli()); + xContentBuilder.endObject(); + return xContentBuilder; + } +} diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenApiAction.java index 03bc932027..075ffcc462 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenApiAction.java @@ -96,7 +96,7 @@ private void apiTokenApiRequestHandlers(RequestHandler.RequestHandlersBuilder re public String createApiToken(String name, Client client) { createApiTokenIndexIfAbsent(client); - /* TODO: Create document representing the API token */ + new ApiTokenIndexManager(client).indexToken(new ApiToken(name, "test-token", List.of())); return "test-token"; } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenIndexManager.java b/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenIndexManager.java new file mode 100644 index 0000000000..06470f5a67 --- /dev/null +++ b/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenIndexManager.java @@ -0,0 +1,59 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.dlic.rest.api; + +import java.io.IOException; + +import org.opensearch.action.get.GetRequest; +import org.opensearch.action.index.IndexRequest; +import org.opensearch.client.Client; +import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.common.xcontent.XContentFactory; +import org.opensearch.common.xcontent.XContentType; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.security.support.ConfigConstants; + +public class ApiTokenIndexManager { + + private Client client; + + public ApiTokenIndexManager(Client client) { + this.client = client; + } + + public void indexToken(ApiToken token) { + try (final ThreadContext.StoredContext ctx = client.threadPool().getThreadContext().stashContext()) { + + XContentBuilder builder = XContentFactory.jsonBuilder(); + String jsonString = token.toXContent(builder, ToXContent.EMPTY_PARAMS).toString(); + + IndexRequest request = new IndexRequest(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX) // Index name + .source(jsonString, XContentType.JSON); // Set JSON source + + client.index(request); + + } catch (IOException e) { + throw new RuntimeException(e); + } + + } + + public Object getTokens() { + try (final ThreadContext.StoredContext ctx = client.threadPool().getThreadContext().stashContext()) { + + return client.get(new GetRequest(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX)); + + } + } + +} From 6bfc31c2fde228f0e9f790f930fbaac16af83561 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Tue, 19 Nov 2024 17:04:24 -0500 Subject: [PATCH 07/34] Clean up Signed-off-by: Derek Ho --- .../security/OpenSearchSecurityPlugin.java | 2 + .../api => action/apitokens}/ApiToken.java | 2 +- .../action/apitokens/ApiTokenAction.java | 129 ++++++++++++++++++ .../apitokens}/ApiTokenIndexManager.java | 2 +- .../action/apitokens/ApiTokenRepository.java | 14 ++ .../dlic/rest/api/ApiTokenApiAction.java | 123 ----------------- .../dlic/rest/api/SecurityRestApiActions.java | 1 - 7 files changed, 147 insertions(+), 126 deletions(-) rename src/main/java/org/opensearch/security/{dlic/rest/api => action/apitokens}/ApiToken.java (97%) create mode 100644 src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java rename src/main/java/org/opensearch/security/{dlic/rest/api => action/apitokens}/ApiTokenIndexManager.java (97%) create mode 100644 src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java delete mode 100644 src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenApiAction.java diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 57ffc4df6f..c6830e7624 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -131,6 +131,7 @@ import org.opensearch.search.internal.ReaderContext; import org.opensearch.search.internal.SearchContext; import org.opensearch.search.query.QuerySearchResult; +import org.opensearch.security.action.apitokens.ApiTokenAction; import org.opensearch.security.action.configupdate.ConfigUpdateAction; import org.opensearch.security.action.configupdate.TransportConfigUpdateAction; import org.opensearch.security.action.onbehalf.CreateOnBehalfOfTokenAction; @@ -635,6 +636,7 @@ public List getRestHandlers( ) ); handlers.add(new CreateOnBehalfOfTokenAction(tokenManager)); + handlers.add(new ApiTokenAction(cs, threadPool)); handlers.addAll( SecurityRestApiActions.getHandler( settings, diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/ApiToken.java b/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java similarity index 97% rename from src/main/java/org/opensearch/security/dlic/rest/api/ApiToken.java rename to src/main/java/org/opensearch/security/action/apitokens/ApiToken.java index e54a4f7f85..34d0dc5d11 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/ApiToken.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java @@ -9,7 +9,7 @@ * GitHub history for details. */ -package org.opensearch.security.dlic.rest.api; +package org.opensearch.security.action.apitokens; import java.io.IOException; import java.time.Instant; diff --git a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java new file mode 100644 index 0000000000..c49cf0c4b6 --- /dev/null +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java @@ -0,0 +1,129 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.action.apitokens; + +import java.io.IOException; +import java.util.List; +import java.util.Map; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; + +import org.opensearch.action.admin.indices.create.CreateIndexRequest; +import org.opensearch.client.Client; +import org.opensearch.client.node.NodeClient; +import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.util.concurrent.ThreadContext; +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.RestHandler; +import org.opensearch.rest.RestRequest; +import org.opensearch.security.support.ConfigConstants; +import org.opensearch.threadpool.ThreadPool; + +import static org.opensearch.rest.RestRequest.Method.POST; +import static org.opensearch.security.dlic.rest.support.Utils.addRoutesPrefix; + +public class ApiTokenAction extends BaseRestHandler { + + private ClusterService clusterService; + private ThreadPool threadpool; + + public static final String NAME_JSON_PROPERTY = "name"; + + private static final List ROUTES = addRoutesPrefix(ImmutableList.of(new RestHandler.Route(POST, "/apitokens"))); + + public ApiTokenAction(ClusterService clusterService, ThreadPool threadPool) { + this.clusterService = clusterService; + this.threadpool = threadPool; + } + + @Override + public String getName() { + return "Actions to get and create API tokens."; + } + + @Override + public List routes() { + return ROUTES; + } + + @Override + protected RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException { + switch (request.method()) { + case POST: + return handlePost(request, client); + default: + throw new IllegalArgumentException(request.method() + " not supported"); + } + } + + 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); + String token = createApiToken((String) requestBody.get(NAME_JSON_PROPERTY), client); + + builder.startObject(); + builder.field("token", token); + builder.endObject(); + + response = new BytesRestResponse(RestStatus.OK, builder); + } catch (final Exception exception) { + builder.startObject().field("error", "An unexpected error occurred. Please check the input and try again.").endObject(); + response = new BytesRestResponse(RestStatus.INTERNAL_SERVER_ERROR, builder); + } + builder.close(); + channel.sendResponse(response); + }; + + } + + private void validateRequestParameters(Map requestBody) { + if (!requestBody.containsKey(NAME_JSON_PROPERTY)) { + throw new IllegalArgumentException("Name parameter is required and cannot be empty."); + } + } + + public String createApiToken(String name, Client client) { + createApiTokenIndexIfAbsent(client); + new ApiTokenIndexManager(client).indexToken(new ApiToken(name, "test-token", List.of())); + return "test-token"; + } + + public Boolean apiTokenIndexExists() { + return clusterService.state().metadata().hasConcreteIndex(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX); + } + + public void createApiTokenIndexIfAbsent(Client client) { + if (!apiTokenIndexExists()) { + try (final ThreadContext.StoredContext ctx = client.threadPool().getThreadContext().stashContext()) { + 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 + ); + logger.info(client.admin().indices().create(createIndexRequest).actionGet().isAcknowledged()); + } + } + } +} diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenIndexManager.java b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexManager.java similarity index 97% rename from src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenIndexManager.java rename to src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexManager.java index 06470f5a67..e8b71577fe 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenIndexManager.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexManager.java @@ -9,7 +9,7 @@ * GitHub history for details. */ -package org.opensearch.security.dlic.rest.api; +package org.opensearch.security.action.apitokens; import java.io.IOException; diff --git a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java new file mode 100644 index 0000000000..b23938e09c --- /dev/null +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java @@ -0,0 +1,14 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.action.apitokens; + +public class ApiTokenRepository {} diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenApiAction.java deleted file mode 100644 index 075ffcc462..0000000000 --- a/src/main/java/org/opensearch/security/dlic/rest/api/ApiTokenApiAction.java +++ /dev/null @@ -1,123 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - * - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.security.dlic.rest.api; - -import java.util.List; -import java.util.Map; - -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; - -import org.opensearch.action.admin.indices.create.CreateIndexRequest; -import org.opensearch.client.Client; -import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.util.concurrent.ThreadContext; -import org.opensearch.security.securityconf.impl.CType; -import org.opensearch.security.securityconf.impl.v7.ConfigV7; -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; -import static org.opensearch.rest.RestRequest.Method.PUT; -import static org.opensearch.security.dlic.rest.api.Responses.internalServerError; -import static org.opensearch.security.dlic.rest.api.Responses.ok; -import static org.opensearch.security.dlic.rest.api.Responses.response; -import static org.opensearch.security.dlic.rest.support.Utils.addRoutesPrefix; - -public class ApiTokenApiAction extends AbstractApiAction { - - public static final String NAME_JSON_PROPERTY = "name"; - - private static final List ROUTES = addRoutesPrefix( - ImmutableList.of(new Route(GET, "/apitokens"), new Route(PUT, "/apitokens/{name}"), new Route(DELETE, "/apitokens/{name}")) - ); - - protected ApiTokenApiAction(ClusterService clusterService, ThreadPool threadPool, SecurityApiDependencies securityApiDependencies) { - super(Endpoint.APITOKENS, clusterService, threadPool, securityApiDependencies); - this.requestHandlersBuilder.configureRequestHandlers(this::apiTokenApiRequestHandlers); - } - - @Override - public String getName() { - return "API Token actions to retrieve / update configs."; - } - - @Override - public List routes() { - return ROUTES; - } - - @Override - /* TODO: Alternative to CType for new index */ - protected CType getConfigType() { - return CType.CONFIG; - } - - private void apiTokenApiRequestHandlers(RequestHandler.RequestHandlersBuilder requestHandlersBuilder) { - - requestHandlersBuilder.override( - GET, - (channel, request, client) -> loadConfiguration(getConfigType(), false, false).valid(configuration -> { - if (!apiTokenIndexExists()) { - /** TODO: Return empty list */ - ok(channel, "empty list"); - } else { - /** TODO: Return list of documents from API Tokens index */ - ok(channel, "non-empty list"); - } - }).error((status, toXContent) -> response(channel, status, toXContent)) - ).override(PUT, (channel, request, client) -> loadConfiguration(getConfigType(), false, false).valid(configuration -> { - String token = createApiToken(request.param(NAME_JSON_PROPERTY), client); - ok(channel, token + " created successfully"); - }).error((status, toXContent) -> response(channel, status, toXContent))) - .override(DELETE, (channel, request, client) -> loadConfiguration(getConfigType(), false, false).valid(configuration -> { - String token = createApiToken(request.param(NAME_JSON_PROPERTY), client); - if (!apiTokenIndexExists()) { - /** TODO: Return error indicating token doesn't exist */ - internalServerError(channel, token + " doesn't exist"); - } else { - /** TODO: Delete document from API Tokens index if it exists, otherwise return error */ - ok(channel, token + " deleted successfully"); - } - - }).error((status, toXContent) -> response(channel, status, toXContent))); - - } - - public String createApiToken(String name, Client client) { - createApiTokenIndexIfAbsent(client); - new ApiTokenIndexManager(client).indexToken(new ApiToken(name, "test-token", List.of())); - return "test-token"; - } - - public Boolean apiTokenIndexExists() { - return clusterService.state().metadata().hasConcreteIndex(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX); - } - - public void createApiTokenIndexIfAbsent(Client client) { - if (!apiTokenIndexExists()) { - try (final ThreadContext.StoredContext ctx = client.threadPool().getThreadContext().stashContext()) { - 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 - ); - logger.info(client.admin().indices().create(createIndexRequest).actionGet().isAcknowledged()); - } - } - } -} diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java b/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java index f1dfed03f3..c28a1bdc1d 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/SecurityRestApiActions.java @@ -96,7 +96,6 @@ public static Collection getHandler( new AuditApiAction(clusterService, threadPool, securityApiDependencies), new MultiTenancyConfigApiAction(clusterService, threadPool, securityApiDependencies), new RateLimitersApiAction(clusterService, threadPool, securityApiDependencies), - new ApiTokenApiAction(clusterService, threadPool, securityApiDependencies), new ConfigUpgradeApiAction(clusterService, threadPool, securityApiDependencies), new SecuritySSLCertsApiAction( clusterService, From 0b1da6d0392b539a09a8ccacfa637c0d60fdb6de Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Wed, 20 Nov 2024 10:42:43 -0500 Subject: [PATCH 08/34] Refactor to use repository model Signed-off-by: Derek Ho --- .../security/OpenSearchSecurityPlugin.java | 2 +- .../action/apitokens/ApiTokenAction.java | 37 +------ .../apitokens/ApiTokenIndexHandler.java | 99 +++++++++++++++++++ .../apitokens/ApiTokenIndexManager.java | 59 ----------- .../action/apitokens/ApiTokenRepository.java | 20 +++- 5 files changed, 123 insertions(+), 94 deletions(-) create mode 100644 src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandler.java delete mode 100644 src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexManager.java diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index c6830e7624..381e181942 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -636,7 +636,7 @@ public List getRestHandlers( ) ); handlers.add(new CreateOnBehalfOfTokenAction(tokenManager)); - handlers.add(new ApiTokenAction(cs, threadPool)); + handlers.add(new ApiTokenAction(cs, threadPool, localClient)); handlers.addAll( SecurityRestApiActions.getHandler( settings, 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 c49cf0c4b6..4ff4fb9a63 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java @@ -16,20 +16,16 @@ import java.util.Map; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import org.opensearch.action.admin.indices.create.CreateIndexRequest; import org.opensearch.client.Client; import org.opensearch.client.node.NodeClient; import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.util.concurrent.ThreadContext; 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.RestHandler; import org.opensearch.rest.RestRequest; -import org.opensearch.security.support.ConfigConstants; import org.opensearch.threadpool.ThreadPool; import static org.opensearch.rest.RestRequest.Method.POST; @@ -39,14 +35,16 @@ public class ApiTokenAction extends BaseRestHandler { private ClusterService clusterService; private ThreadPool threadpool; + private ApiTokenRepository apiTokenRepository; public static final String NAME_JSON_PROPERTY = "name"; private static final List ROUTES = addRoutesPrefix(ImmutableList.of(new RestHandler.Route(POST, "/apitokens"))); - public ApiTokenAction(ClusterService clusterService, ThreadPool threadPool) { + public ApiTokenAction(ClusterService clusterService, ThreadPool threadPool, Client client) { this.clusterService = clusterService; this.threadpool = threadPool; + this.apiTokenRepository = new ApiTokenRepository(client, clusterService); } @Override @@ -77,7 +75,7 @@ private RestChannelConsumer handlePost(RestRequest request, NodeClient client) { final Map requestBody = request.contentOrSourceParamParser().map(); validateRequestParameters(requestBody); - String token = createApiToken((String) requestBody.get(NAME_JSON_PROPERTY), client); + String token = apiTokenRepository.createApiToken((String) requestBody.get(NAME_JSON_PROPERTY)); builder.startObject(); builder.field("token", token); @@ -99,31 +97,4 @@ private void validateRequestParameters(Map requestBody) { throw new IllegalArgumentException("Name parameter is required and cannot be empty."); } } - - public String createApiToken(String name, Client client) { - createApiTokenIndexIfAbsent(client); - new ApiTokenIndexManager(client).indexToken(new ApiToken(name, "test-token", List.of())); - return "test-token"; - } - - public Boolean apiTokenIndexExists() { - return clusterService.state().metadata().hasConcreteIndex(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX); - } - - public void createApiTokenIndexIfAbsent(Client client) { - if (!apiTokenIndexExists()) { - try (final ThreadContext.StoredContext ctx = client.threadPool().getThreadContext().stashContext()) { - 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 - ); - logger.info(client.admin().indices().create(createIndexRequest).actionGet().isAcknowledged()); - } - } - } } diff --git a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandler.java b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandler.java new file mode 100644 index 0000000000..9f36f47ce1 --- /dev/null +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandler.java @@ -0,0 +1,99 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.action.apitokens; + +import java.io.IOException; +import java.util.Map; + +import com.google.common.collect.ImmutableMap; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import org.opensearch.action.admin.indices.create.CreateIndexRequest; +import org.opensearch.action.get.GetRequest; +import org.opensearch.action.index.IndexRequest; +import org.opensearch.action.index.IndexResponse; +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; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.security.support.ConfigConstants; + +public class ApiTokenIndexHandler { + + private Client client; + private ClusterService clusterService; + private static final Logger LOGGER = LogManager.getLogger(ApiTokenIndexHandler.class); + + public ApiTokenIndexHandler(Client client, ClusterService clusterService) { + this.client = client; + this.clusterService = clusterService; + } + + public String indexToken(ApiToken token) { + try (final ThreadContext.StoredContext ctx = client.threadPool().getThreadContext().stashContext()) { + + XContentBuilder builder = XContentFactory.jsonBuilder(); + String jsonString = token.toXContent(builder, ToXContent.EMPTY_PARAMS).toString(); + + IndexRequest request = new IndexRequest(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX).source(jsonString, XContentType.JSON); + + ActionListener irListener = ActionListener.wrap(idxResponse -> { + LOGGER.info("Created {} entry.", ConfigConstants.OPENSEARCH_API_TOKENS_INDEX); + }, (failResponse) -> { + LOGGER.error(failResponse.getMessage()); + LOGGER.info("Failed to create {} entry.", ConfigConstants.OPENSEARCH_API_TOKENS_INDEX); + }); + + client.index(request, irListener); + return token.getDescription(); + + } catch (IOException e) { + throw new RuntimeException(e); + } + + } + + public Object getTokens() { + try (final ThreadContext.StoredContext ctx = client.threadPool().getThreadContext().stashContext()) { + + return client.get(new GetRequest(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX)); + + } + } + + public Boolean apiTokenIndexExists() { + return clusterService.state().metadata().hasConcreteIndex(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX); + } + + public void createApiTokenIndexIfAbsent() { + if (!apiTokenIndexExists()) { + try (final ThreadContext.StoredContext ctx = client.threadPool().getThreadContext().stashContext()) { + 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 + ); + LOGGER.info(client.admin().indices().create(createIndexRequest).actionGet().isAcknowledged()); + } + } + } + +} diff --git a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexManager.java b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexManager.java deleted file mode 100644 index e8b71577fe..0000000000 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexManager.java +++ /dev/null @@ -1,59 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - * - * Modifications Copyright OpenSearch Contributors. See - * GitHub history for details. - */ - -package org.opensearch.security.action.apitokens; - -import java.io.IOException; - -import org.opensearch.action.get.GetRequest; -import org.opensearch.action.index.IndexRequest; -import org.opensearch.client.Client; -import org.opensearch.common.util.concurrent.ThreadContext; -import org.opensearch.common.xcontent.XContentFactory; -import org.opensearch.common.xcontent.XContentType; -import org.opensearch.core.xcontent.ToXContent; -import org.opensearch.core.xcontent.XContentBuilder; -import org.opensearch.security.support.ConfigConstants; - -public class ApiTokenIndexManager { - - private Client client; - - public ApiTokenIndexManager(Client client) { - this.client = client; - } - - public void indexToken(ApiToken token) { - try (final ThreadContext.StoredContext ctx = client.threadPool().getThreadContext().stashContext()) { - - XContentBuilder builder = XContentFactory.jsonBuilder(); - String jsonString = token.toXContent(builder, ToXContent.EMPTY_PARAMS).toString(); - - IndexRequest request = new IndexRequest(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX) // Index name - .source(jsonString, XContentType.JSON); // Set JSON source - - client.index(request); - - } catch (IOException e) { - throw new RuntimeException(e); - } - - } - - public Object getTokens() { - try (final ThreadContext.StoredContext ctx = client.threadPool().getThreadContext().stashContext()) { - - return client.get(new GetRequest(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX)); - - } - } - -} 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 b23938e09c..761ef38988 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java @@ -11,4 +11,22 @@ package org.opensearch.security.action.apitokens; -public class ApiTokenRepository {} +import java.util.List; + +import org.opensearch.client.Client; +import org.opensearch.cluster.service.ClusterService; + +public class ApiTokenRepository { + private ApiTokenIndexHandler apiTokenIndexHandler; + + public ApiTokenRepository(Client client, ClusterService clusterService) { + apiTokenIndexHandler = new ApiTokenIndexHandler(client, clusterService); + } + + public String createApiToken(String name) { + apiTokenIndexHandler.createApiTokenIndexIfAbsent(); + String token = apiTokenIndexHandler.indexToken(new ApiToken(name, "test-token", List.of())); + return token; + } + +} From 77e18d9616b5e783ed9c45cb1b4996fc96cdf4fb Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Wed, 20 Nov 2024 16:01:24 -0500 Subject: [PATCH 09/34] Implement simple functionality for get and delete APIs as well Signed-off-by: Derek Ho --- .../action/apitokens/ApiTokenAction.java | 79 +++++++++++++++++-- .../action/apitokens/ApiTokenException.java | 24 ++++++ .../apitokens/ApiTokenIndexHandler.java | 39 ++++++++- .../action/apitokens/ApiTokenRepository.java | 14 +++- 4 files changed, 144 insertions(+), 12 deletions(-) create mode 100644 src/main/java/org/opensearch/security/action/apitokens/ApiTokenException.java 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 4ff4fb9a63..60706b9a3d 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java @@ -28,22 +28,25 @@ import org.opensearch.rest.RestRequest; import org.opensearch.threadpool.ThreadPool; +import static org.opensearch.rest.RestRequest.Method.DELETE; +import static org.opensearch.rest.RestRequest.Method.GET; import static org.opensearch.rest.RestRequest.Method.POST; import static org.opensearch.security.dlic.rest.support.Utils.addRoutesPrefix; public class ApiTokenAction extends BaseRestHandler { - - private ClusterService clusterService; - private ThreadPool threadpool; - private ApiTokenRepository apiTokenRepository; + private final ApiTokenRepository apiTokenRepository; public static final String NAME_JSON_PROPERTY = "name"; - private static final List ROUTES = addRoutesPrefix(ImmutableList.of(new RestHandler.Route(POST, "/apitokens"))); + private static final List ROUTES = addRoutesPrefix( + ImmutableList.of( + new RestHandler.Route(POST, "/apitokens"), + new RestHandler.Route(DELETE, "/apitokens"), + new RestHandler.Route(GET, "/apitokens") + ) + ); public ApiTokenAction(ClusterService clusterService, ThreadPool threadPool, Client client) { - this.clusterService = clusterService; - this.threadpool = threadPool; this.apiTokenRepository = new ApiTokenRepository(client, clusterService); } @@ -59,15 +62,49 @@ public List 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"); } } + private RestChannelConsumer handleGet(RestRequest request, NodeClient client) { + return channel -> { + final XContentBuilder builder = channel.newBuilder(); + BytesRestResponse response; + try { + List> token = apiTokenRepository.getApiTokens(); + + builder.startArray(); + for (int i = 0; i < token.toArray().length; i++) { + // TODO: refactor this to the helper function + builder.startObject(); + builder.field("name", token.get(i).get("description")); + builder.field("creation_time", token.get(i).get("creation_time")); + builder.endObject(); + } + builder.endArray(); + + response = new BytesRestResponse(RestStatus.OK, builder); + } catch (final Exception exception) { + builder.startObject().field("error", "An unexpected error occurred. Please check the input and try again.").endObject(); + response = new BytesRestResponse(RestStatus.INTERNAL_SERVER_ERROR, builder); + } + builder.close(); + channel.sendResponse(response); + }; + + } + private RestChannelConsumer handlePost(RestRequest request, NodeClient client) { + // TODO: Enforce unique token description return channel -> { final XContentBuilder builder = channel.newBuilder(); BytesRestResponse response; @@ -92,6 +129,34 @@ private RestChannelConsumer handlePost(RestRequest request, NodeClient client) { } + private RestChannelConsumer handleDelete(RestRequest request, NodeClient client) { + return channel -> { + final XContentBuilder builder = channel.newBuilder(); + BytesRestResponse response; + try { + final Map requestBody = request.contentOrSourceParamParser().map(); + + validateRequestParameters(requestBody); + apiTokenRepository.deleteApiToken((String) requestBody.get(NAME_JSON_PROPERTY)); + + builder.startObject(); + builder.field("message", "token " + requestBody.get(NAME_JSON_PROPERTY) + " deleted successfully."); + builder.endObject(); + + response = new BytesRestResponse(RestStatus.OK, builder); + } catch (final ApiTokenException exception) { + builder.startObject().field("error", exception.getMessage()).endObject(); + response = new BytesRestResponse(RestStatus.NOT_FOUND, builder); + } catch (final Exception exception) { + builder.startObject().field("error", "An unexpected error occurred. Please check the input and try again.").endObject(); + response = new BytesRestResponse(RestStatus.INTERNAL_SERVER_ERROR, builder); + } + builder.close(); + channel.sendResponse(response); + }; + + } + private void validateRequestParameters(Map requestBody) { if (!requestBody.containsKey(NAME_JSON_PROPERTY)) { throw new IllegalArgumentException("Name parameter is required and cannot be empty."); diff --git a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenException.java b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenException.java new file mode 100644 index 0000000000..398da40e64 --- /dev/null +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenException.java @@ -0,0 +1,24 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.action.apitokens; + +import org.opensearch.OpenSearchException; + +public class ApiTokenException extends OpenSearchException { + public ApiTokenException(String message) { + super(message); + } + + public ApiTokenException(String message, Throwable cause) { + super(message, cause); + } +} 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 9f36f47ce1..ee15348787 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandler.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandler.java @@ -12,6 +12,8 @@ package org.opensearch.security.action.apitokens; import java.io.IOException; +import java.util.ArrayList; +import java.util.List; import java.util.Map; import com.google.common.collect.ImmutableMap; @@ -19,9 +21,10 @@ import org.apache.logging.log4j.Logger; import org.opensearch.action.admin.indices.create.CreateIndexRequest; -import org.opensearch.action.get.GetRequest; import org.opensearch.action.index.IndexRequest; import org.opensearch.action.index.IndexResponse; +import org.opensearch.action.search.SearchRequest; +import org.opensearch.action.search.SearchResponse; import org.opensearch.client.Client; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.util.concurrent.ThreadContext; @@ -30,6 +33,11 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.index.query.QueryBuilders; +import org.opensearch.index.reindex.BulkByScrollResponse; +import org.opensearch.index.reindex.DeleteByQueryAction; +import org.opensearch.index.reindex.DeleteByQueryRequest; +import org.opensearch.search.SearchHit; import org.opensearch.security.support.ConfigConstants; public class ApiTokenIndexHandler { @@ -67,11 +75,36 @@ public String indexToken(ApiToken token) { } - public Object getTokens() { + public void deleteToken(String name) throws ApiTokenException { try (final ThreadContext.StoredContext ctx = client.threadPool().getThreadContext().stashContext()) { + DeleteByQueryRequest request = new DeleteByQueryRequest(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX).setQuery( + QueryBuilders.matchQuery("description", name) + ).setRefresh(true); // This will refresh the index after deletion - return client.get(new GetRequest(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX)); + BulkByScrollResponse response = client.execute(DeleteByQueryAction.INSTANCE, request).actionGet(); + long deletedDocs = response.getDeleted(); + + if (deletedDocs == 0) { + throw new ApiTokenException("No token found with name " + name); + } + LOGGER.info("Deleted " + deletedDocs + " documents"); + } + } + + public List> getApiTokens() { + try (final ThreadContext.StoredContext ctx = client.threadPool().getThreadContext().stashContext()) { + + SearchRequest searchRequest = new SearchRequest(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX); + + SearchResponse response = client.search(searchRequest).actionGet(); + + List> tokens = new ArrayList<>(); + for (SearchHit hit : response.getHits().getHits()) { + tokens.add(hit.getSourceAsMap()); + } + + return tokens; } } 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 761ef38988..d77e7e8ffa 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java @@ -12,6 +12,7 @@ package org.opensearch.security.action.apitokens; import java.util.List; +import java.util.Map; import org.opensearch.client.Client; import org.opensearch.cluster.service.ClusterService; @@ -25,8 +26,17 @@ public ApiTokenRepository(Client client, ClusterService clusterService) { public String createApiToken(String name) { apiTokenIndexHandler.createApiTokenIndexIfAbsent(); - String token = apiTokenIndexHandler.indexToken(new ApiToken(name, "test-token", List.of())); - return token; + return apiTokenIndexHandler.indexToken(new ApiToken(name, "test-token", List.of())); + } + + public void deleteApiToken(String name) throws ApiTokenException { + apiTokenIndexHandler.createApiTokenIndexIfAbsent(); + apiTokenIndexHandler.deleteToken(name); + } + + public List> getApiTokens() { + apiTokenIndexHandler.createApiTokenIndexIfAbsent(); + return apiTokenIndexHandler.getApiTokens(); } } From 6464a8c5e7da045531345f3ad6a944830067bc64 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Wed, 20 Nov 2024 17:40:17 -0500 Subject: [PATCH 10/34] Clean up and add basic test Signed-off-by: Derek Ho --- .../security/action/apitokens/ApiToken.java | 28 ++++-- .../apitokens/ApiTokenIndexHandler.java | 2 +- .../action/apitokens/ApiTokenRepository.java | 3 +- .../security/dlic/rest/api/Endpoint.java | 1 - .../securityconf/DynamicConfigModel.java | 1 + .../apitokens/ApiTokenIndexHandlerTest.java | 94 +++++++++++++++++++ 6 files changed, 118 insertions(+), 11 deletions(-) create mode 100644 src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java diff --git a/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java b/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java index 34d0dc5d11..7c1e109d1b 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java @@ -17,19 +17,22 @@ import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.security.securityconf.impl.v7.RoleV7; public class ApiToken implements ToXContent { private String description; private String jti; private Instant creationTime; - private List roles; + private List clusterPermissions; + private List indexPermissions; - public ApiToken(String description, String jti, List roles) { + public ApiToken(String description, String jti, List clusterPermissions, List indexPermissions) { this.creationTime = Instant.now(); this.description = description; this.jti = jti; - this.roles = roles; + this.clusterPermissions = clusterPermissions; + this.indexPermissions = indexPermissions; } @@ -57,12 +60,12 @@ public void setCreationTime(Instant creationTime) { this.creationTime = creationTime; } - public List getRoles() { - return roles; + public List getClusterPermissions() { + return clusterPermissions; } - public void setRoles(List roles) { - this.roles = roles; + public void setClusterPermissions(List clusterPermissions) { + this.clusterPermissions = clusterPermissions; } @Override @@ -70,9 +73,18 @@ public XContentBuilder toXContent(XContentBuilder xContentBuilder, ToXContent.Pa xContentBuilder.startObject(); xContentBuilder.field("description", description); xContentBuilder.field("jti", jti); - xContentBuilder.field("roles", roles); + xContentBuilder.field("cluster_permissions", clusterPermissions); + xContentBuilder.field("index_permissions", indexPermissions); xContentBuilder.field("creation_time", creationTime.toEpochMilli()); xContentBuilder.endObject(); return xContentBuilder; } + + public List getIndexPermissions() { + return indexPermissions; + } + + public void setIndexPermissions(List indexPermissions) { + this.indexPermissions = indexPermissions; + } } 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 ee15348787..900f83028f 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandler.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandler.java @@ -124,7 +124,7 @@ public void createApiTokenIndexIfAbsent() { final CreateIndexRequest createIndexRequest = new CreateIndexRequest(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX).settings( indexSettings ); - LOGGER.info(client.admin().indices().create(createIndexRequest).actionGet().isAcknowledged()); + 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 d77e7e8ffa..b485c05a0d 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java @@ -16,6 +16,7 @@ import org.opensearch.client.Client; import org.opensearch.cluster.service.ClusterService; +import org.opensearch.security.securityconf.impl.v7.RoleV7; public class ApiTokenRepository { private ApiTokenIndexHandler apiTokenIndexHandler; @@ -26,7 +27,7 @@ public ApiTokenRepository(Client client, ClusterService clusterService) { public String createApiToken(String name) { apiTokenIndexHandler.createApiTokenIndexIfAbsent(); - return apiTokenIndexHandler.indexToken(new ApiToken(name, "test-token", List.of())); + return apiTokenIndexHandler.indexToken(new ApiToken(name, "test-token", List.of(), List.of(new RoleV7.Index()))); } public void deleteApiToken(String name) throws ApiTokenException { 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 afed070a78..ecc9dcbc59 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 @@ -25,7 +25,6 @@ public enum Endpoint { AUTHTOKEN, TENANTS, RATELIMITERS, - APITOKENS, MIGRATE, VALIDATE, WHITELIST, diff --git a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModel.java b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModel.java index 422391dcaf..064f555a75 100644 --- a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModel.java +++ b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModel.java @@ -142,4 +142,5 @@ public DynamicConfigModel() { authImplMap.put("ip_authFailureListener", AddressBasedRateLimiter.class.getName()); authImplMap.put("username_authFailureListener", UserNameBasedRateLimiter.class.getName()); } + } diff --git a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java new file mode 100644 index 0000000000..b51c1077a6 --- /dev/null +++ b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java @@ -0,0 +1,94 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.action.apitokens; + +import org.junit.Before; +import org.junit.Test; + +import org.opensearch.action.admin.indices.create.CreateIndexRequest; +import org.opensearch.client.AdminClient; +import org.opensearch.client.Client; +import org.opensearch.client.IndicesAdminClient; +import org.opensearch.cluster.ClusterState; +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.support.ConfigConstants; +import org.opensearch.threadpool.ThreadPool; + +import org.mockito.Mock; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class ApiTokenIndexHandlerTest { + + @Mock + private Client client; + + @Mock + private AdminClient adminClient; + + @Mock + private IndicesAdminClient indicesAdminClient; + + @Mock + private ThreadPool threadPool; + + @Mock + private ClusterService clusterService; + + @Mock + private ClusterState clusterState; + + @Mock + private Metadata metadata; + + private ApiTokenIndexHandler indexHandler; + + @Before + public void setup() { + + client = mock(Client.class); + adminClient = mock(AdminClient.class); + indicesAdminClient = mock(IndicesAdminClient.class); + clusterService = mock(ClusterService.class); + clusterState = mock(ClusterState.class); + metadata = mock(Metadata.class); + threadPool = mock(ThreadPool.class); + + when(client.admin()).thenReturn(adminClient); + when(adminClient.indices()).thenReturn(indicesAdminClient); + + when(clusterService.state()).thenReturn(clusterState); + when(clusterState.metadata()).thenReturn(metadata); + + ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + when(client.threadPool()).thenReturn(threadPool); + when(threadPool.getThreadContext()).thenReturn(threadContext); + + indexHandler = new ApiTokenIndexHandler(client, clusterService); + } + + @Test + public void testCreateApiTokenIndex() { + when(metadata.hasConcreteIndex(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX)).thenReturn(false); + + indexHandler.createApiTokenIndexIfAbsent(); + + verify(indicesAdminClient).create(any(CreateIndexRequest.class)); + } + +} From 784809c6173cb66368091d74c87da330af230049 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Tue, 10 Dec 2024 13:12:05 -0500 Subject: [PATCH 11/34] Add comprehensive tests and clean up indexing and getting to xcontent Signed-off-by: Derek Ho --- .../security/action/apitokens/ApiToken.java | 132 +++++++++ .../action/apitokens/ApiTokenAction.java | 10 +- .../apitokens/ApiTokenIndexHandler.java | 29 +- .../action/apitokens/ApiTokenRepository.java | 2 +- .../security/securityconf/impl/v7/RoleV7.java | 37 ++- .../apitokens/ApiTokenIndexHandlerTest.java | 266 ++++++++++++++++-- 6 files changed, 436 insertions(+), 40 deletions(-) diff --git a/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java b/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java index 7c1e109d1b..c14cf9cf36 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java @@ -13,10 +13,12 @@ import java.io.IOException; import java.time.Instant; +import java.util.ArrayList; import java.util.List; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.core.xcontent.XContentParser; import org.opensearch.security.securityconf.impl.v7.RoleV7; public class ApiToken implements ToXContent { @@ -36,6 +38,136 @@ public ApiToken(String description, String jti, List clusterPermissions, } + public ApiToken( + String description, + String jti, + List clusterPermissions, + List indexPermissions, + Instant creationTime + ) { + this.description = description; + this.jti = jti; + this.clusterPermissions = clusterPermissions; + this.indexPermissions = indexPermissions; + this.creationTime = creationTime; + + } + + public static ApiToken fromXContent(XContentParser parser) throws IOException { + String description = null; + String jti = null; + List clusterPermissions = new ArrayList<>(); + List indexPermissions = new ArrayList<>(); + Instant creationTime = null; + + XContentParser.Token token; + String currentFieldName = null; + + while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { + if (token == XContentParser.Token.FIELD_NAME) { + currentFieldName = parser.currentName(); + } else if (token.isValue()) { + switch (currentFieldName) { + case "description": + description = parser.text(); + break; + case "jti": + jti = parser.text(); + break; + case "creation_time": + creationTime = Instant.ofEpochMilli(parser.longValue()); + break; + } + } else if (token == XContentParser.Token.START_ARRAY) { + switch (currentFieldName) { + case "cluster_permissions": + while (parser.nextToken() != XContentParser.Token.END_ARRAY) { + clusterPermissions.add(parser.text()); + } + break; + case "index_permissions": + while (parser.nextToken() != XContentParser.Token.END_ARRAY) { + if (parser.currentToken() == XContentParser.Token.START_OBJECT) { + indexPermissions.add(parseIndexPermission(parser)); + } + } + break; + } + } + } + + // Validate required fields + if (description == null) { + throw new IllegalArgumentException("description is required"); + } + if (jti == null) { + throw new IllegalArgumentException("jti is required"); + } + if (creationTime == null) { + throw new IllegalArgumentException("creation_time is required"); + } + + return new ApiToken(description, jti, clusterPermissions, indexPermissions, creationTime); + } + + private static RoleV7.Index parseIndexPermission(XContentParser parser) throws IOException { + List indexPatterns = new ArrayList<>(); + List allowedActions = new ArrayList<>(); + String dls = null; + List fls = null; + List maskedFields = null; + + String currentFieldName = null; + XContentParser.Token token; + + while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { + if (token == XContentParser.Token.FIELD_NAME) { + currentFieldName = parser.currentName(); + } else if (token.isValue()) { + if ("dls".equals(currentFieldName)) { + dls = parser.text(); + } + } else if (token == XContentParser.Token.START_ARRAY) { + switch (currentFieldName) { + case "index_patterns": + while (parser.nextToken() != XContentParser.Token.END_ARRAY) { + indexPatterns.add(parser.text()); + } + break; + case "allowed_actions": + while (parser.nextToken() != XContentParser.Token.END_ARRAY) { + allowedActions.add(parser.text()); + } + break; + case "fls": + fls = new ArrayList<>(); + while (parser.nextToken() != XContentParser.Token.END_ARRAY) { + fls.add(parser.text()); + } + break; + case "masked_fields": + maskedFields = new ArrayList<>(); + while (parser.nextToken() != XContentParser.Token.END_ARRAY) { + maskedFields.add(parser.text()); + } + break; + } + } + } + + if (indexPatterns.isEmpty()) { + throw new IllegalArgumentException("index_patterns is required for index permission"); + } + + return new RoleV7.Index( + indexPatterns, + allowedActions, + dls, // Now passing String instead of List + fls, + maskedFields + ); + } + public String getDescription() { return description; } 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 60706b9a3d..da35bf7eb5 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java @@ -80,14 +80,13 @@ private RestChannelConsumer handleGet(RestRequest request, NodeClient client) { final XContentBuilder builder = channel.newBuilder(); BytesRestResponse response; try { - List> token = apiTokenRepository.getApiTokens(); + Map tokens = apiTokenRepository.getApiTokens(); builder.startArray(); - for (int i = 0; i < token.toArray().length; i++) { - // TODO: refactor this to the helper function + for (ApiToken token : tokens.values()) { builder.startObject(); - builder.field("name", token.get(i).get("description")); - builder.field("creation_time", token.get(i).get("creation_time")); + builder.field("name", token.getDescription()); + builder.field("creation_time", token.getCreationTime().toEpochMilli()); builder.endObject(); } builder.endArray(); @@ -100,7 +99,6 @@ private RestChannelConsumer handleGet(RestRequest request, NodeClient client) { builder.close(); channel.sendResponse(response); }; - } private RestChannelConsumer handlePost(RestRequest request, NodeClient client) { 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 900f83028f..ab91ccb76b 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandler.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandler.java @@ -12,8 +12,7 @@ package org.opensearch.security.action.apitokens; import java.io.IOException; -import java.util.ArrayList; -import java.util.List; +import java.util.HashMap; import java.util.Map; import com.google.common.collect.ImmutableMap; @@ -31,13 +30,17 @@ import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.common.xcontent.XContentType; import org.opensearch.core.action.ActionListener; +import org.opensearch.core.xcontent.DeprecationHandler; +import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.core.xcontent.XContentParser; import org.opensearch.index.query.QueryBuilders; import org.opensearch.index.reindex.BulkByScrollResponse; import org.opensearch.index.reindex.DeleteByQueryAction; import org.opensearch.index.reindex.DeleteByQueryRequest; import org.opensearch.search.SearchHit; +import org.opensearch.search.builder.SearchSourceBuilder; import org.opensearch.security.support.ConfigConstants; public class ApiTokenIndexHandler { @@ -92,19 +95,31 @@ public void deleteToken(String name) throws ApiTokenException { } } - public List> getApiTokens() { + public Map getApiTokens() { try (final ThreadContext.StoredContext ctx = client.threadPool().getThreadContext().stashContext()) { - SearchRequest searchRequest = new SearchRequest(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX); + searchRequest.source(new SearchSourceBuilder()); SearchResponse response = client.search(searchRequest).actionGet(); - List> tokens = new ArrayList<>(); + Map tokens = new HashMap<>(); for (SearchHit hit : response.getHits().getHits()) { - tokens.add(hit.getSourceAsMap()); + try ( + XContentParser parser = XContentType.JSON.xContent() + .createParser( + NamedXContentRegistry.EMPTY, + DeprecationHandler.THROW_UNSUPPORTED_OPERATION, + hit.getSourceRef().streamInput() + ) + ) { + + ApiToken token = ApiToken.fromXContent(parser); + tokens.put(token.getDescription(), token); // Assuming description is the key + } } - return tokens; + } catch (IOException e) { + throw new RuntimeException(e); } } 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 b485c05a0d..093a35f74e 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java @@ -35,7 +35,7 @@ public void deleteApiToken(String name) throws ApiTokenException { apiTokenIndexHandler.deleteToken(name); } - public List> getApiTokens() { + public Map getApiTokens() { apiTokenIndexHandler.createApiTokenIndexIfAbsent(); return apiTokenIndexHandler.getApiTokens(); } diff --git a/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java b/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java index 2b2da40927..b56185a534 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java @@ -27,11 +27,14 @@ package org.opensearch.security.securityconf.impl.v7; +import java.io.IOException; import java.util.Collections; import java.util.List; import com.fasterxml.jackson.annotation.JsonProperty; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.security.securityconf.Hideable; import org.opensearch.security.securityconf.StaticDefinable; @@ -50,7 +53,7 @@ public RoleV7() { } - public static class Index { + public static class Index implements ToXContent { private List index_patterns = Collections.emptyList(); private String dls; @@ -62,6 +65,38 @@ public Index() { super(); } + public Index(List indexPattern, List allowedActions, String dls, List fls, List maskedFields) { + super(); + this.index_patterns = indexPattern; + this.allowed_actions = allowedActions; + this.dls = dls; + this.fls = fls; + this.masked_fields = maskedFields; + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(); + + builder.field("index_patterns", index_patterns); + builder.field("allowed_actions", allowed_actions); + + if (dls != null) { + builder.field("dls", dls); + } + + if (fls != null && !fls.isEmpty()) { + builder.field("fls", fls); + } + + if (masked_fields != null && !masked_fields.isEmpty()) { + builder.field("masked_fields", masked_fields); + } + + builder.endObject(); + return builder; + } + public List getIndex_patterns() { return index_patterns; } diff --git a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java index b51c1077a6..580e7b81c2 100644 --- a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java +++ b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java @@ -11,26 +11,59 @@ package org.opensearch.security.action.apitokens; +import java.io.IOException; +import java.time.Instant; +import java.util.Arrays; +import java.util.List; +import java.util.Map; + +import org.apache.lucene.search.TotalHits; import org.junit.Before; import org.junit.Test; import org.opensearch.action.admin.indices.create.CreateIndexRequest; -import org.opensearch.client.AdminClient; +import org.opensearch.action.index.IndexRequest; +import org.opensearch.action.index.IndexResponse; +import org.opensearch.action.search.SearchRequest; +import org.opensearch.action.search.SearchResponse; +import org.opensearch.action.support.PlainActionFuture; import org.opensearch.client.Client; import org.opensearch.client.IndicesAdminClient; -import org.opensearch.cluster.ClusterState; 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.common.xcontent.XContentType; +import org.opensearch.core.action.ActionListener; +import org.opensearch.core.common.bytes.BytesReference; +import org.opensearch.core.index.shard.ShardId; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.index.query.MatchQueryBuilder; +import org.opensearch.index.reindex.BulkByScrollResponse; +import org.opensearch.index.reindex.DeleteByQueryAction; +import org.opensearch.index.reindex.DeleteByQueryRequest; +import org.opensearch.search.SearchHit; +import org.opensearch.search.SearchHits; +import org.opensearch.security.securityconf.impl.v7.RoleV7; import org.opensearch.security.support.ConfigConstants; -import org.opensearch.threadpool.ThreadPool; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.RETURNS_DEEP_STUBS; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; public class ApiTokenIndexHandlerTest { @@ -38,21 +71,12 @@ public class ApiTokenIndexHandlerTest { @Mock private Client client; - @Mock - private AdminClient adminClient; - @Mock private IndicesAdminClient indicesAdminClient; - @Mock - private ThreadPool threadPool; - @Mock private ClusterService clusterService; - @Mock - private ClusterState clusterState; - @Mock private Metadata metadata; @@ -61,34 +85,226 @@ public class ApiTokenIndexHandlerTest { @Before public void setup() { - client = mock(Client.class); - adminClient = mock(AdminClient.class); + client = mock(Client.class, RETURNS_DEEP_STUBS); indicesAdminClient = mock(IndicesAdminClient.class); - clusterService = mock(ClusterService.class); - clusterState = mock(ClusterState.class); + clusterService = mock(ClusterService.class, RETURNS_DEEP_STUBS); metadata = mock(Metadata.class); - threadPool = mock(ThreadPool.class); - when(client.admin()).thenReturn(adminClient); - when(adminClient.indices()).thenReturn(indicesAdminClient); + when(client.admin().indices()).thenReturn(indicesAdminClient); - when(clusterService.state()).thenReturn(clusterState); - when(clusterState.metadata()).thenReturn(metadata); + when(clusterService.state().metadata()).thenReturn(metadata); ThreadContext threadContext = new ThreadContext(Settings.EMPTY); - when(client.threadPool()).thenReturn(threadPool); - when(threadPool.getThreadContext()).thenReturn(threadContext); + when(client.threadPool().getThreadContext()).thenReturn(threadContext); indexHandler = new ApiTokenIndexHandler(client, clusterService); } @Test - public void testCreateApiTokenIndex() { + public void testCreateApiTokenIndexWhenIndexNotExist() { when(metadata.hasConcreteIndex(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX)).thenReturn(false); indexHandler.createApiTokenIndexIfAbsent(); - verify(indicesAdminClient).create(any(CreateIndexRequest.class)); + ArgumentCaptor captor = ArgumentCaptor.forClass(CreateIndexRequest.class); + + verify(indicesAdminClient).create(captor.capture()); + assertThat(captor.getValue().index(), equalTo(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX)); + } + + @Test + public void testCreateApiTokenIndexWhenIndexExists() { + when(metadata.hasConcreteIndex(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX)).thenReturn(true); + + indexHandler.createApiTokenIndexIfAbsent(); + + verifyNoInteractions(indicesAdminClient); + } + + @Test + public void testDeleteApiTokeCallsDeleteByQueryWithSuppliedName() { + when(metadata.hasConcreteIndex(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX)).thenReturn(true); + String tokenName = "token"; + try { + indexHandler.deleteToken(tokenName); + } catch (Exception e) { + // Ignore + } + + ArgumentCaptor captor = ArgumentCaptor.forClass(DeleteByQueryRequest.class); + verify(client).execute(eq(DeleteByQueryAction.INSTANCE), captor.capture()); + + // Verify the captured request has the correct query parameters + DeleteByQueryRequest capturedRequest = captor.getValue(); + MatchQueryBuilder query = (MatchQueryBuilder) capturedRequest.getSearchRequest().source().query(); + assertThat(query.fieldName(), equalTo("description")); + assertThat(query.value(), equalTo(tokenName)); + } + + @Test + public void testDeleteTokenThrowsExceptionWhenNoDocumentsDeleted() { + when(metadata.hasConcreteIndex(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX)).thenReturn(true); + + PlainActionFuture future = new PlainActionFuture<>(); + BulkByScrollResponse response = mock(BulkByScrollResponse.class); + when(response.getDeleted()).thenReturn(0L); + future.onResponse(response); + when(client.execute(eq(DeleteByQueryAction.INSTANCE), any(DeleteByQueryRequest.class))).thenReturn(future); + + String tokenName = "nonexistent-token"; + try { + indexHandler.deleteToken(tokenName); + fail("Expected ApiTokenException to be thrown"); + } catch (ApiTokenException e) { + assertThat(e.getMessage(), equalTo("No token found with name " + tokenName)); + } + } + + @Test + public void testDeleteTokenSucceedsWhenDocumentIsDeleted() { + when(metadata.hasConcreteIndex(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX)).thenReturn(true); + + // Mock response with 1 deleted document + PlainActionFuture future = new PlainActionFuture<>(); + BulkByScrollResponse response = mock(BulkByScrollResponse.class); + when(response.getDeleted()).thenReturn(1L); + future.onResponse(response); + when(client.execute(eq(DeleteByQueryAction.INSTANCE), any(DeleteByQueryRequest.class))).thenReturn(future); + + String tokenName = "existing-token"; + try { + indexHandler.deleteToken(tokenName); + } catch (ApiTokenException e) { + fail("Should not have thrown exception"); + } + } + + @Test + public void testIndexTokenStoresToken() { + when(metadata.hasConcreteIndex(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX)).thenReturn(true); + + // Create a real ApiToken + List clusterPermissions = Arrays.asList("cluster:admin/something"); + List indexPermissions = Arrays.asList( + new RoleV7.Index( + Arrays.asList("test-index-*"), + Arrays.asList("read", "write"), + null, // dls + null, // fls + null // masked_fields + ) + ); + ApiToken token = new ApiToken( + "test-token-description", + "test-jti", + clusterPermissions, + indexPermissions, + Instant.now() + ); + + // Mock the index method with ActionListener + @SuppressWarnings("unchecked") + ArgumentCaptor> listenerCaptor = + ArgumentCaptor.forClass((Class>) (Class) ActionListener.class); + + doAnswer(invocation -> { + ActionListener listener = listenerCaptor.getValue(); + listener.onResponse(new IndexResponse( + new ShardId(".opensearch_security_api_tokens", "_na_", 1), + "1", + 0, + 1, + 1, + true + )); + return null; + }).when(client).index(any(IndexRequest.class), listenerCaptor.capture()); + + indexHandler.indexToken(token); + + // Verify the index request + ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(IndexRequest.class); + verify(client).index(requestCaptor.capture(), listenerCaptor.capture()); + + IndexRequest capturedRequest = requestCaptor.getValue(); + assertThat(capturedRequest.index(), equalTo(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX)); + + // Convert the source to a string and verify contents + String source = capturedRequest.source().utf8ToString(); + assertThat(source, containsString("test-token-description")); + assertThat(source, containsString("test-jti")); + assertThat(source, containsString("cluster:admin/something")); + assertThat(source, containsString("test-index-*")); + } + + @Test + public void testGetApiTokens() throws IOException { + when(metadata.hasConcreteIndex(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX)).thenReturn(true); + + // Create sample search hits + SearchHit[] hits = new SearchHit[2]; + + // First token + ApiToken token1 = new ApiToken( + "token1-description", + "jti1", + Arrays.asList("cluster:admin/something"), + Arrays.asList(new RoleV7.Index( + Arrays.asList("index1-*"), + Arrays.asList("read"), + null, null, null + )), + Instant.now() + ); + + // Second token + ApiToken token2 = new ApiToken( + "token2-description", + "jti2", + Arrays.asList("cluster:admin/other"), + Arrays.asList(new RoleV7.Index( + Arrays.asList("index2-*"), + Arrays.asList("write"), + null, null, null + )), + Instant.now() + ); + + // Convert tokens to XContent and create SearchHits + XContentBuilder builder1 = XContentBuilder.builder(XContentType.JSON.xContent()); + token1.toXContent(builder1, ToXContent.EMPTY_PARAMS); + hits[0] = new SearchHit(1, "1", null, null); + hits[0].sourceRef(BytesReference.bytes(builder1)); + + XContentBuilder builder2 = XContentBuilder.builder(XContentType.JSON.xContent()); + token2.toXContent(builder2, ToXContent.EMPTY_PARAMS); + hits[1] = new SearchHit(2, "2", null, null); + hits[1].sourceRef(BytesReference.bytes(builder2)); + + // Create and mock search response + SearchResponse searchResponse = mock(SearchResponse.class); + SearchHits searchHits = new SearchHits(hits, new TotalHits(2, TotalHits.Relation.EQUAL_TO), 1.0f); + when(searchResponse.getHits()).thenReturn(searchHits); + + // Mock client search call + PlainActionFuture future = new PlainActionFuture<>(); + future.onResponse(searchResponse); + when(client.search(any(SearchRequest.class))).thenReturn(future); + + // Get tokens and verify + Map resultTokens = indexHandler.getApiTokens(); + + assertThat(resultTokens.size(), equalTo(2)); + assertThat(resultTokens.containsKey("token1-description"), is(true)); + assertThat(resultTokens.containsKey("token2-description"), is(true)); + + ApiToken resultToken1 = resultTokens.get("token1-description"); + assertThat(resultToken1.getJti(), equalTo("jti1")); + assertThat(resultToken1.getClusterPermissions(), contains("cluster:admin/something")); + + ApiToken resultToken2 = resultTokens.get("token2-description"); + assertThat(resultToken2.getJti(), equalTo("jti2")); + assertThat(resultToken2.getClusterPermissions(), contains("cluster:admin/other")); } } From 11880ec2fbc8585fb3cfbe5689bbc68d6ffecc27 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Tue, 10 Dec 2024 14:23:59 -0500 Subject: [PATCH 12/34] Add ignore unknown properties Signed-off-by: Derek Ho --- .../org/opensearch/security/securityconf/impl/v7/RoleV7.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java b/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java index b56185a534..9fcf4e6d1c 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java @@ -31,6 +31,7 @@ import java.util.Collections; import java.util.List; +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonProperty; import org.opensearch.core.xcontent.ToXContent; @@ -53,6 +54,7 @@ public RoleV7() { } + @JsonIgnoreProperties(ignoreUnknown = true) public static class Index implements ToXContent { private List index_patterns = Collections.emptyList(); From ade56be20bfb7ca563f711b1e96efbbeb621e7a4 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Tue, 10 Dec 2024 15:34:40 -0500 Subject: [PATCH 13/34] @JsonCreator Signed-off-by: Derek Ho --- .../org/opensearch/security/securityconf/impl/v7/RoleV7.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java b/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java index 9fcf4e6d1c..1530aeebe5 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java @@ -31,6 +31,7 @@ import java.util.Collections; import java.util.List; +import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonProperty; @@ -67,6 +68,7 @@ public Index() { super(); } + @JsonCreator public Index(List indexPattern, List allowedActions, String dls, List fls, List maskedFields) { super(); this.index_patterns = indexPattern; From e50496029533117abcbb8046c8113dbb94162fd1 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Tue, 10 Dec 2024 15:39:13 -0500 Subject: [PATCH 14/34] Json property mode Signed-off-by: Derek Ho --- .../security/securityconf/impl/v7/RoleV7.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java b/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java index 1530aeebe5..33de2e8ff5 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java @@ -68,10 +68,15 @@ public Index() { super(); } - @JsonCreator - public Index(List indexPattern, List allowedActions, String dls, List fls, List maskedFields) { - super(); - this.index_patterns = indexPattern; + @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) // Add mode = PROPERTIES + public Index( + @JsonProperty("index_patterns") List indexPatterns, + @JsonProperty("allowed_actions") List allowedActions, + @JsonProperty("dls") String dls, + @JsonProperty("fls") List fls, + @JsonProperty("masked_fields") List maskedFields + ) { + this.index_patterns = indexPatterns; this.allowed_actions = allowedActions; this.dls = dls; this.fls = fls; From 58478d439a26defa7ef4039db8d35c31d6d22b9b Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Tue, 10 Dec 2024 17:13:49 -0500 Subject: [PATCH 15/34] Fix fragment issue Signed-off-by: Derek Ho --- .../security/action/apitokens/ApiToken.java | 16 +++--------- .../security/securityconf/impl/v7/RoleV7.java | 26 +++++-------------- 2 files changed, 11 insertions(+), 31 deletions(-) diff --git a/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java b/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java index c14cf9cf36..54db6762eb 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java @@ -113,9 +113,9 @@ public static ApiToken fromXContent(XContentParser parser) throws IOException { private static RoleV7.Index parseIndexPermission(XContentParser parser) throws IOException { List indexPatterns = new ArrayList<>(); List allowedActions = new ArrayList<>(); - String dls = null; - List fls = null; - List maskedFields = null; + String dls = ""; + List fls = new ArrayList<>(); + List maskedFields = new ArrayList<>(); String currentFieldName = null; XContentParser.Token token; @@ -140,13 +140,11 @@ private static RoleV7.Index parseIndexPermission(XContentParser parser) throws I } break; case "fls": - fls = new ArrayList<>(); while (parser.nextToken() != XContentParser.Token.END_ARRAY) { fls.add(parser.text()); } break; case "masked_fields": - maskedFields = new ArrayList<>(); while (parser.nextToken() != XContentParser.Token.END_ARRAY) { maskedFields.add(parser.text()); } @@ -159,13 +157,7 @@ private static RoleV7.Index parseIndexPermission(XContentParser parser) throws I throw new IllegalArgumentException("index_patterns is required for index permission"); } - return new RoleV7.Index( - indexPatterns, - allowedActions, - dls, // Now passing String instead of List - fls, - maskedFields - ); + return new RoleV7.Index(indexPatterns, allowedActions, dls, fls, maskedFields); } public String getDescription() { diff --git a/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java b/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java index 33de2e8ff5..48f6f5aaed 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java @@ -31,8 +31,8 @@ import java.util.Collections; import java.util.List; -import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; import org.opensearch.core.xcontent.ToXContent; @@ -55,7 +55,8 @@ public RoleV7() { } - @JsonIgnoreProperties(ignoreUnknown = true) + @JsonIgnoreProperties(value = { "fragment" }, ignoreUnknown = true) + @JsonInclude(JsonInclude.Include.NON_NULL) public static class Index implements ToXContent { private List index_patterns = Collections.emptyList(); @@ -68,14 +69,7 @@ public Index() { super(); } - @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) // Add mode = PROPERTIES - public Index( - @JsonProperty("index_patterns") List indexPatterns, - @JsonProperty("allowed_actions") List allowedActions, - @JsonProperty("dls") String dls, - @JsonProperty("fls") List fls, - @JsonProperty("masked_fields") List maskedFields - ) { + public Index(List indexPatterns, List allowedActions, String dls, List fls, List maskedFields) { this.index_patterns = indexPatterns; this.allowed_actions = allowedActions; this.dls = dls; @@ -90,17 +84,11 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.field("index_patterns", index_patterns); builder.field("allowed_actions", allowed_actions); - if (dls != null) { - builder.field("dls", dls); - } + builder.field("dls", dls); - if (fls != null && !fls.isEmpty()) { - builder.field("fls", fls); - } + builder.field("fls", fls); - if (masked_fields != null && !masked_fields.isEmpty()) { - builder.field("masked_fields", masked_fields); - } + builder.field("masked_fields", masked_fields); builder.endObject(); return builder; From 0c43e9deb52debe29758fe799981845c060fa907 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Wed, 11 Dec 2024 10:11:01 -0500 Subject: [PATCH 16/34] Add support for cluster and index creation parsing Signed-off-by: Derek Ho --- .../action/apitokens/ApiTokenAction.java | 142 +++++++++++++++++- .../action/apitokens/ApiTokenRepository.java | 5 +- 2 files changed, 143 insertions(+), 4 deletions(-) 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 da35bf7eb5..f44d573599 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java @@ -12,8 +12,11 @@ package org.opensearch.security.action.apitokens; import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import com.google.common.collect.ImmutableList; @@ -26,6 +29,7 @@ import org.opensearch.rest.BytesRestResponse; import org.opensearch.rest.RestHandler; import org.opensearch.rest.RestRequest; +import org.opensearch.security.securityconf.impl.v7.RoleV7; import org.opensearch.threadpool.ThreadPool; import static org.opensearch.rest.RestRequest.Method.DELETE; @@ -109,8 +113,15 @@ private RestChannelConsumer handlePost(RestRequest request, NodeClient client) { try { final Map requestBody = request.contentOrSourceParamParser().map(); - validateRequestParameters(requestBody); - String token = apiTokenRepository.createApiToken((String) requestBody.get(NAME_JSON_PROPERTY)); + validateRequestParametersForCreate(requestBody); + List clusterPermissions = extractClusterPermissions(requestBody); + List indexPermissions = extractIndexPermissions(requestBody); + + String token = apiTokenRepository.createApiToken( + (String) requestBody.get(NAME_JSON_PROPERTY), + clusterPermissions, + indexPermissions + ); builder.startObject(); builder.field("token", token); @@ -160,4 +171,131 @@ private void validateRequestParameters(Map requestBody) { throw new IllegalArgumentException("Name parameter is required and cannot be empty."); } } + + /** + * Validates the index permissions list structure + */ + private void validateIndexPermissionsList(List> indexPermsList) { + for (Map indexPerm : indexPermsList) { + // Validate index pattern + if (!indexPerm.containsKey("index_pattern")) { + throw new IllegalArgumentException("Each index permission must contain an index_pattern"); + } + Object indexPatternObj = indexPerm.get("index_pattern"); + if (!(indexPatternObj instanceof String) && !(indexPatternObj instanceof List)) { + throw new IllegalArgumentException("index_pattern must be a string or array of strings"); + } + + // Validate allowed actions + if (!indexPerm.containsKey("allowed_actions")) { + throw new IllegalArgumentException("Each index permission must contain allowed_actions"); + } + if (!(indexPerm.get("allowed_actions") instanceof List)) { + throw new IllegalArgumentException("allowed_actions must be an array"); + } + + // Validate DLS if present + if (indexPerm.containsKey("dls") && !(indexPerm.get("dls") instanceof String)) { + throw new IllegalArgumentException("dls must be a string"); + } + + // Validate FLS if present + if (indexPerm.containsKey("fls") && !(indexPerm.get("fls") instanceof List)) { + throw new IllegalArgumentException("fls must be an array"); + } + + // Validate masked fields if present + if (indexPerm.containsKey("masked_fields") && !(indexPerm.get("masked_fields") instanceof List)) { + throw new IllegalArgumentException("masked_fields must be an array"); + } + } + } + + private void validateRequestParametersForCreate(Map requestBody) { + if (!requestBody.containsKey(NAME_JSON_PROPERTY)) { + throw new IllegalArgumentException("Missing required parameter: " + NAME_JSON_PROPERTY); + } + + // Validate cluster permissions if present + if (requestBody.containsKey("cluster_permissions")) { + Object permissions = requestBody.get("cluster_permissions"); + if (!(permissions instanceof List)) { + throw new IllegalArgumentException("cluster_permissions must be an array"); + } + } + + // Validate index permissions if present + if (requestBody.containsKey("index_permissions")) { + Object indexPerms = requestBody.get("index_permissions"); + if (!(indexPerms instanceof List)) { + throw new IllegalArgumentException("index_permissions must be an array"); + } + + @SuppressWarnings("unchecked") + List> indexPermsList = (List>) indexPerms; + validateIndexPermissionsList(indexPermsList); + } + } + + /** + * Extracts cluster permissions from the request body + */ + private List extractClusterPermissions(Map requestBody) { + if (!requestBody.containsKey("cluster_permissions")) { + return Collections.emptyList(); + } + + @SuppressWarnings("unchecked") + List permissions = (List) requestBody.get("cluster_permissions"); + return new ArrayList<>(permissions); + } + + /** + * Extracts and builds index permissions from the request body + */ + private List extractIndexPermissions(Map requestBody) { + if (!requestBody.containsKey("index_permissions")) { + return Collections.emptyList(); + } + + @SuppressWarnings("unchecked") + List> indexPerms = (List>) requestBody.get("index_permissions"); + + return indexPerms.stream().map(this::createIndexPermission).collect(Collectors.toList()); + } + + /** + * Creates a single RoleV7.Index permission from a permission map + */ + private RoleV7.Index createIndexPermission(Map indexPerm) { + // Get index patterns (can be single string or list) + List indexPatterns; + Object indexPatternObj = indexPerm.get("index_pattern"); + if (indexPatternObj instanceof String) { + indexPatterns = Collections.singletonList((String) indexPatternObj); + } else { + @SuppressWarnings("unchecked") + List patterns = (List) indexPatternObj; + indexPatterns = patterns; + } + + // Get allowed actions + @SuppressWarnings("unchecked") + List allowedActions = (List) indexPerm.get("allowed_actions"); + + // Get DLS (Document Level Security) + String dls = (String) indexPerm.getOrDefault("dls", ""); + + // Get FLS (Field Level Security) + @SuppressWarnings("unchecked") + List fls = indexPerm.containsKey("fls") ? (List) indexPerm.get("fls") : Collections.emptyList(); + + // Get masked fields + @SuppressWarnings("unchecked") + List maskedFields = indexPerm.containsKey("masked_fields") + ? (List) indexPerm.get("masked_fields") + : Collections.emptyList(); + + return new RoleV7.Index(indexPatterns, allowedActions, dls, fls, maskedFields); + } } 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 093a35f74e..6b3ab3fdce 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java @@ -25,9 +25,10 @@ public ApiTokenRepository(Client client, ClusterService clusterService) { apiTokenIndexHandler = new ApiTokenIndexHandler(client, clusterService); } - public String createApiToken(String name) { + public String createApiToken(String name, List clusterPermissions, List indexPermissions) { apiTokenIndexHandler.createApiTokenIndexIfAbsent(); - return apiTokenIndexHandler.indexToken(new ApiToken(name, "test-token", List.of(), List.of(new RoleV7.Index()))); + // TODO: Implement logic of creating JTI to match against during authc/z + return apiTokenIndexHandler.indexToken(new ApiToken(name, "test-token", clusterPermissions, indexPermissions)); } public void deleteApiToken(String name) throws ApiTokenException { From 1468c9cdecd0225524dddd6f2ad2ef7b0e15019f Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Wed, 11 Dec 2024 11:58:44 -0500 Subject: [PATCH 17/34] Add tests for validation functions Signed-off-by: Derek Ho --- .../action/apitokens/ApiTokenAction.java | 240 +++++++++--------- .../action/apitokens/ApiTokenActionTest.java | 128 ++++++++++ 2 files changed, 249 insertions(+), 119 deletions(-) create mode 100644 src/test/java/org/opensearch/security/action/apitokens/ApiTokenActionTest.java 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 f44d573599..d2445ee966 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java @@ -12,7 +12,6 @@ package org.opensearch.security.action.apitokens; import java.io.IOException; -import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Map; @@ -40,7 +39,14 @@ public class ApiTokenAction extends BaseRestHandler { private final ApiTokenRepository apiTokenRepository; - public static final String NAME_JSON_PROPERTY = "name"; + private static final String NAME_JSON_PROPERTY = "name"; + private static final String CLUSTER_PERMISSIONS_FIELD = "cluster_permissions"; + private static final String INDEX_PERMISSIONS_FIELD = "index_permissions"; + private static final String INDEX_PATTERN_FIELD = "index_pattern"; + private static final String ALLOWED_ACTIONS_FIELD = "allowed_actions"; + private static final String DLS_FIELD = "dls"; + private static final String FLS_FIELD = "fls"; + private static final String MASKED_FIELDS_FIELD = "masked_fields"; private static final List ROUTES = addRoutesPrefix( ImmutableList.of( @@ -106,14 +112,13 @@ private RestChannelConsumer handleGet(RestRequest request, NodeClient client) { } private RestChannelConsumer handlePost(RestRequest request, NodeClient client) { - // TODO: Enforce unique token description return channel -> { final XContentBuilder builder = channel.newBuilder(); BytesRestResponse response; try { final Map requestBody = request.contentOrSourceParamParser().map(); + validateRequestParameters(requestBody); - validateRequestParametersForCreate(requestBody); List clusterPermissions = extractClusterPermissions(requestBody); List indexPermissions = extractIndexPermissions(requestBody); @@ -129,137 +134,71 @@ private RestChannelConsumer handlePost(RestRequest request, NodeClient client) { response = new BytesRestResponse(RestStatus.OK, builder); } catch (final Exception exception) { - builder.startObject().field("error", "An unexpected error occurred. Please check the input and try again.").endObject(); - response = new BytesRestResponse(RestStatus.INTERNAL_SERVER_ERROR, builder); - } - builder.close(); - channel.sendResponse(response); - }; - - } - - private RestChannelConsumer handleDelete(RestRequest request, NodeClient client) { - return channel -> { - final XContentBuilder builder = channel.newBuilder(); - BytesRestResponse response; - try { - final Map requestBody = request.contentOrSourceParamParser().map(); - - validateRequestParameters(requestBody); - apiTokenRepository.deleteApiToken((String) requestBody.get(NAME_JSON_PROPERTY)); - - builder.startObject(); - builder.field("message", "token " + requestBody.get(NAME_JSON_PROPERTY) + " deleted successfully."); - builder.endObject(); - - response = new BytesRestResponse(RestStatus.OK, builder); - } catch (final ApiTokenException exception) { - builder.startObject().field("error", exception.getMessage()).endObject(); - response = new BytesRestResponse(RestStatus.NOT_FOUND, builder); - } catch (final Exception exception) { - builder.startObject().field("error", "An unexpected error occurred. Please check the input and try again.").endObject(); + builder.startObject() + .field("error", "An unexpected error occurred. Please check the input and try again.") + .field("message", exception.getMessage()) + .endObject(); response = new BytesRestResponse(RestStatus.INTERNAL_SERVER_ERROR, builder); } builder.close(); channel.sendResponse(response); }; - - } - - private void validateRequestParameters(Map requestBody) { - if (!requestBody.containsKey(NAME_JSON_PROPERTY)) { - throw new IllegalArgumentException("Name parameter is required and cannot be empty."); - } } /** - * Validates the index permissions list structure + * Safely casts an Object to List with validation */ - private void validateIndexPermissionsList(List> indexPermsList) { - for (Map indexPerm : indexPermsList) { - // Validate index pattern - if (!indexPerm.containsKey("index_pattern")) { - throw new IllegalArgumentException("Each index permission must contain an index_pattern"); - } - Object indexPatternObj = indexPerm.get("index_pattern"); - if (!(indexPatternObj instanceof String) && !(indexPatternObj instanceof List)) { - throw new IllegalArgumentException("index_pattern must be a string or array of strings"); - } - - // Validate allowed actions - if (!indexPerm.containsKey("allowed_actions")) { - throw new IllegalArgumentException("Each index permission must contain allowed_actions"); - } - if (!(indexPerm.get("allowed_actions") instanceof List)) { - throw new IllegalArgumentException("allowed_actions must be an array"); - } - - // Validate DLS if present - if (indexPerm.containsKey("dls") && !(indexPerm.get("dls") instanceof String)) { - throw new IllegalArgumentException("dls must be a string"); - } - - // Validate FLS if present - if (indexPerm.containsKey("fls") && !(indexPerm.get("fls") instanceof List)) { - throw new IllegalArgumentException("fls must be an array"); - } + List safeStringList(Object obj, String fieldName) { + if (!(obj instanceof List list)) { + throw new IllegalArgumentException(fieldName + " must be an array"); + } - // Validate masked fields if present - if (indexPerm.containsKey("masked_fields") && !(indexPerm.get("masked_fields") instanceof List)) { - throw new IllegalArgumentException("masked_fields must be an array"); + for (Object item : list) { + if (!(item instanceof String)) { + throw new IllegalArgumentException(fieldName + " must contain only strings"); } } - } - private void validateRequestParametersForCreate(Map requestBody) { - if (!requestBody.containsKey(NAME_JSON_PROPERTY)) { - throw new IllegalArgumentException("Missing required parameter: " + NAME_JSON_PROPERTY); - } + return list.stream().map(String.class::cast).collect(Collectors.toList()); + } - // Validate cluster permissions if present - if (requestBody.containsKey("cluster_permissions")) { - Object permissions = requestBody.get("cluster_permissions"); - if (!(permissions instanceof List)) { - throw new IllegalArgumentException("cluster_permissions must be an array"); - } + /** + * Safely casts an Object to List> with validation + */ + @SuppressWarnings("unchecked") + List> safeMapList(Object obj, String fieldName) { + if (!(obj instanceof List list)) { + throw new IllegalArgumentException(fieldName + " must be an array"); } - // Validate index permissions if present - if (requestBody.containsKey("index_permissions")) { - Object indexPerms = requestBody.get("index_permissions"); - if (!(indexPerms instanceof List)) { - throw new IllegalArgumentException("index_permissions must be an array"); + for (Object item : list) { + if (!(item instanceof Map)) { + throw new IllegalArgumentException(fieldName + " must contain object entries"); } - - @SuppressWarnings("unchecked") - List> indexPermsList = (List>) indexPerms; - validateIndexPermissionsList(indexPermsList); } + return list.stream().map(item -> (Map) item).collect(Collectors.toList()); } /** * Extracts cluster permissions from the request body */ - private List extractClusterPermissions(Map requestBody) { - if (!requestBody.containsKey("cluster_permissions")) { + List extractClusterPermissions(Map requestBody) { + if (!requestBody.containsKey(CLUSTER_PERMISSIONS_FIELD)) { return Collections.emptyList(); } - @SuppressWarnings("unchecked") - List permissions = (List) requestBody.get("cluster_permissions"); - return new ArrayList<>(permissions); + return safeStringList(requestBody.get(CLUSTER_PERMISSIONS_FIELD), CLUSTER_PERMISSIONS_FIELD); } /** * Extracts and builds index permissions from the request body */ - private List extractIndexPermissions(Map requestBody) { - if (!requestBody.containsKey("index_permissions")) { + List extractIndexPermissions(Map requestBody) { + if (!requestBody.containsKey(INDEX_PERMISSIONS_FIELD)) { return Collections.emptyList(); } - @SuppressWarnings("unchecked") - List> indexPerms = (List>) requestBody.get("index_permissions"); + List> indexPerms = safeMapList(requestBody.get(INDEX_PERMISSIONS_FIELD), INDEX_PERMISSIONS_FIELD); return indexPerms.stream().map(this::createIndexPermission).collect(Collectors.toList()); } @@ -267,35 +206,98 @@ private List extractIndexPermissions(Map requestBo /** * Creates a single RoleV7.Index permission from a permission map */ - private RoleV7.Index createIndexPermission(Map indexPerm) { - // Get index patterns (can be single string or list) + RoleV7.Index createIndexPermission(Map indexPerm) { List indexPatterns; - Object indexPatternObj = indexPerm.get("index_pattern"); + Object indexPatternObj = indexPerm.get(INDEX_PATTERN_FIELD); if (indexPatternObj instanceof String) { indexPatterns = Collections.singletonList((String) indexPatternObj); } else { - @SuppressWarnings("unchecked") - List patterns = (List) indexPatternObj; - indexPatterns = patterns; + indexPatterns = safeStringList(indexPatternObj, INDEX_PATTERN_FIELD); } - // Get allowed actions - @SuppressWarnings("unchecked") - List allowedActions = (List) indexPerm.get("allowed_actions"); + List allowedActions = safeStringList(indexPerm.get(ALLOWED_ACTIONS_FIELD), ALLOWED_ACTIONS_FIELD); - // Get DLS (Document Level Security) - String dls = (String) indexPerm.getOrDefault("dls", ""); + String dls = (String) indexPerm.getOrDefault(DLS_FIELD, ""); - // Get FLS (Field Level Security) - @SuppressWarnings("unchecked") - List fls = indexPerm.containsKey("fls") ? (List) indexPerm.get("fls") : Collections.emptyList(); + List fls = indexPerm.containsKey(FLS_FIELD) ? safeStringList(indexPerm.get(FLS_FIELD), FLS_FIELD) : Collections.emptyList(); - // Get masked fields - @SuppressWarnings("unchecked") - List maskedFields = indexPerm.containsKey("masked_fields") - ? (List) indexPerm.get("masked_fields") + List maskedFields = indexPerm.containsKey(MASKED_FIELDS_FIELD) + ? safeStringList(indexPerm.get(MASKED_FIELDS_FIELD), MASKED_FIELDS_FIELD) : Collections.emptyList(); return new RoleV7.Index(indexPatterns, allowedActions, dls, fls, maskedFields); } + + /** + * Validates the request parameters + */ + void validateRequestParameters(Map requestBody) { + if (!requestBody.containsKey(NAME_JSON_PROPERTY)) { + throw new IllegalArgumentException("Missing required parameter: " + NAME_JSON_PROPERTY); + } + + if (requestBody.containsKey(CLUSTER_PERMISSIONS_FIELD)) { + Object permissions = requestBody.get(CLUSTER_PERMISSIONS_FIELD); + if (!(permissions instanceof List)) { + throw new IllegalArgumentException(CLUSTER_PERMISSIONS_FIELD + " must be an array"); + } + } + + if (requestBody.containsKey(INDEX_PERMISSIONS_FIELD)) { + List> indexPermsList = safeMapList(requestBody.get(INDEX_PERMISSIONS_FIELD), INDEX_PERMISSIONS_FIELD); + validateIndexPermissionsList(indexPermsList); + } + } + + /** + * Validates the index permissions list structure + */ + void validateIndexPermissionsList(List> indexPermsList) { + for (Map indexPerm : indexPermsList) { + if (!indexPerm.containsKey(INDEX_PATTERN_FIELD)) { + throw new IllegalArgumentException("Each index permission must contain " + INDEX_PATTERN_FIELD); + } + if (!indexPerm.containsKey(ALLOWED_ACTIONS_FIELD)) { + throw new IllegalArgumentException("Each index permission must contain " + ALLOWED_ACTIONS_FIELD); + } + + Object indexPatternObj = indexPerm.get(INDEX_PATTERN_FIELD); + if (!(indexPatternObj instanceof String) && !(indexPatternObj instanceof List)) { + throw new IllegalArgumentException(INDEX_PATTERN_FIELD + " must be a string or array of strings"); + } + + if (indexPerm.containsKey(DLS_FIELD) && !(indexPerm.get(DLS_FIELD) instanceof String)) { + throw new IllegalArgumentException(DLS_FIELD + " must be a string"); + } + } + } + + private RestChannelConsumer handleDelete(RestRequest request, NodeClient client) { + return channel -> { + final XContentBuilder builder = channel.newBuilder(); + BytesRestResponse response; + try { + final Map requestBody = request.contentOrSourceParamParser().map(); + + validateRequestParameters(requestBody); + apiTokenRepository.deleteApiToken((String) requestBody.get(NAME_JSON_PROPERTY)); + + builder.startObject(); + builder.field("message", "token " + requestBody.get(NAME_JSON_PROPERTY) + " deleted successfully."); + builder.endObject(); + + response = new BytesRestResponse(RestStatus.OK, builder); + } catch (final ApiTokenException exception) { + builder.startObject().field("error", exception.getMessage()).endObject(); + response = new BytesRestResponse(RestStatus.NOT_FOUND, builder); + } catch (final Exception exception) { + builder.startObject().field("error", "An unexpected error occurred. Please check the input and try again.").endObject(); + response = new BytesRestResponse(RestStatus.INTERNAL_SERVER_ERROR, builder); + } + builder.close(); + channel.sendResponse(response); + }; + + } + } diff --git a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenActionTest.java b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenActionTest.java new file mode 100644 index 0000000000..287d1544b0 --- /dev/null +++ b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenActionTest.java @@ -0,0 +1,128 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.action.apitokens; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.junit.Test; + +import org.opensearch.security.securityconf.impl.v7.RoleV7; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThrows; + +public class ApiTokenActionTest { + + private final ApiTokenAction apiTokenAction = new ApiTokenAction(null, null, null); // repository not needed for these tests + + @Test + public void testSafeStringList() { + // Valid case + List result = apiTokenAction.safeStringList(Arrays.asList("test1", "test2"), "test_field"); + assertThat(result, is(Arrays.asList("test1", "test2"))); + + // Not a list + assertThrows(IllegalArgumentException.class, () -> apiTokenAction.safeStringList("not a list", "test_field")); + + // List with non-string + assertThrows(IllegalArgumentException.class, () -> apiTokenAction.safeStringList(Arrays.asList("test", 123), "test_field")); + } + + @Test + public void testCreateIndexPermission() { + Map validPermission = new HashMap<>(); + validPermission.put("index_pattern", "test-*"); + validPermission.put("allowed_actions", Arrays.asList("read")); + validPermission.put("dls", ""); + validPermission.put("fls", Arrays.asList("field1", "field2")); + validPermission.put("masked_fields", Arrays.asList("sensitive1")); + + RoleV7.Index result = apiTokenAction.createIndexPermission(validPermission); + + assertThat(result.getIndex_patterns(), is(Arrays.asList("test-*"))); + assertThat(result.getAllowed_actions(), is(Arrays.asList("read"))); + assertThat(result.getDls(), is("")); + assertThat(result.getFls(), is(Arrays.asList("field1", "field2"))); + assertThat(result.getMasked_fields(), is(Arrays.asList("sensitive1"))); + } + + @Test + public void testValidateRequestParameters() { + // Valid case + Map validRequest = new HashMap<>(); + validRequest.put("name", "test-token"); + validRequest.put("cluster_permissions", Arrays.asList("perm1", "perm2")); + apiTokenAction.validateRequestParameters(validRequest); + + // Missing name + Map missingName = new HashMap<>(); + assertThrows(IllegalArgumentException.class, () -> apiTokenAction.validateRequestParameters(missingName)); + + // Invalid cluster_permissions type + Map invalidClusterPerms = new HashMap<>(); + invalidClusterPerms.put("name", "test"); + invalidClusterPerms.put("cluster_permissions", "not a list"); + assertThrows(IllegalArgumentException.class, () -> apiTokenAction.validateRequestParameters(invalidClusterPerms)); + } + + @Test + public void testValidateIndexPermissionsList() { + // Valid case + Map validPerm = new HashMap<>(); + validPerm.put("index_pattern", "test-*"); + validPerm.put("allowed_actions", Arrays.asList("read")); + apiTokenAction.validateIndexPermissionsList(Collections.singletonList(validPerm)); + + // Missing index_pattern + Map missingPattern = new HashMap<>(); + missingPattern.put("allowed_actions", Arrays.asList("read")); + assertThrows( + IllegalArgumentException.class, + () -> apiTokenAction.validateIndexPermissionsList(Collections.singletonList(missingPattern)) + ); + + // Missing allowed_actions + Map missingActions = new HashMap<>(); + missingActions.put("index_pattern", "test-*"); + assertThrows( + IllegalArgumentException.class, + () -> apiTokenAction.validateIndexPermissionsList(Collections.singletonList(missingActions)) + ); + + // Invalid index_pattern type + Map invalidPattern = new HashMap<>(); + invalidPattern.put("index_pattern", 123); + invalidPattern.put("allowed_actions", Arrays.asList("read")); + assertThrows( + IllegalArgumentException.class, + () -> apiTokenAction.validateIndexPermissionsList(Collections.singletonList(invalidPattern)) + ); + } + + @Test + public void testExtractClusterPermissions() { + Map requestBody = new HashMap<>(); + + // Empty case + assertThat(apiTokenAction.extractClusterPermissions(requestBody), is(empty())); + + // Valid case + requestBody.put("cluster_permissions", Arrays.asList("perm1", "perm2")); + assertThat(apiTokenAction.extractClusterPermissions(requestBody), is(Arrays.asList("perm1", "perm2"))); + } +} From 725c3f8d17ec9d326aa9c8be9a101442dbffa074 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Wed, 11 Dec 2024 15:17:20 -0500 Subject: [PATCH 18/34] Cleanup according to PR feedback Signed-off-by: Derek Ho --- .../security/action/apitokens/ApiToken.java | 91 ++++++++++--------- .../action/apitokens/ApiTokenAction.java | 26 +----- .../apitokens/ApiTokenIndexHandler.java | 8 +- .../action/apitokens/ApiTokenRepository.java | 6 +- .../security/securityconf/impl/v7/RoleV7.java | 34 +------ .../action/apitokens/ApiTokenActionTest.java | 11 +-- .../apitokens/ApiTokenIndexHandlerTest.java | 20 ++-- 7 files changed, 69 insertions(+), 127 deletions(-) diff --git a/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java b/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java index 54db6762eb..4adbe6f9bc 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java @@ -19,19 +19,17 @@ import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; -import org.opensearch.security.securityconf.impl.v7.RoleV7; public class ApiToken implements ToXContent { - private String description; - private String jti; - - private Instant creationTime; + private String name; + private final String jti; + private final Instant creationTime; private List clusterPermissions; - private List indexPermissions; + private List indexPermissions; - public ApiToken(String description, String jti, List clusterPermissions, List indexPermissions) { + public ApiToken(String name, String jti, List clusterPermissions, List indexPermissions) { this.creationTime = Instant.now(); - this.description = description; + this.name = name; this.jti = jti; this.clusterPermissions = clusterPermissions; this.indexPermissions = indexPermissions; @@ -42,10 +40,10 @@ public ApiToken( String description, String jti, List clusterPermissions, - List indexPermissions, + List indexPermissions, Instant creationTime ) { - this.description = description; + this.name = description; this.jti = jti; this.clusterPermissions = clusterPermissions; this.indexPermissions = indexPermissions; @@ -53,11 +51,38 @@ public ApiToken( } + public static class IndexPermission implements ToXContent { + private final List indexPatterns; + private final List allowedActions; + + public IndexPermission(List indexPatterns, List allowedActions) { + this.indexPatterns = indexPatterns; + this.allowedActions = allowedActions; + } + + public List getAllowedActions() { + return allowedActions; + } + + public List getIndexPatterns() { + return indexPatterns; + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(); + builder.array("index_patterns", indexPatterns.toArray(new String[0])); + builder.array("allowed_actions", allowedActions.toArray(new String[0])); + builder.endObject(); + return builder; + } + } + public static ApiToken fromXContent(XContentParser parser) throws IOException { String description = null; String jti = null; List clusterPermissions = new ArrayList<>(); - List indexPermissions = new ArrayList<>(); + List indexPermissions = new ArrayList<>(); Instant creationTime = null; XContentParser.Token token; @@ -110,12 +135,9 @@ public static ApiToken fromXContent(XContentParser parser) throws IOException { return new ApiToken(description, jti, clusterPermissions, indexPermissions, creationTime); } - private static RoleV7.Index parseIndexPermission(XContentParser parser) throws IOException { + private static IndexPermission parseIndexPermission(XContentParser parser) throws IOException { List indexPatterns = new ArrayList<>(); List allowedActions = new ArrayList<>(); - String dls = ""; - List fls = new ArrayList<>(); - List maskedFields = new ArrayList<>(); String currentFieldName = null; XContentParser.Token token; @@ -123,10 +145,6 @@ private static RoleV7.Index parseIndexPermission(XContentParser parser) throws I while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { if (token == XContentParser.Token.FIELD_NAME) { currentFieldName = parser.currentName(); - } else if (token.isValue()) { - if ("dls".equals(currentFieldName)) { - dls = parser.text(); - } } else if (token == XContentParser.Token.START_ARRAY) { switch (currentFieldName) { case "index_patterns": @@ -139,16 +157,7 @@ private static RoleV7.Index parseIndexPermission(XContentParser parser) throws I allowedActions.add(parser.text()); } break; - case "fls": - while (parser.nextToken() != XContentParser.Token.END_ARRAY) { - fls.add(parser.text()); - } - break; - case "masked_fields": - while (parser.nextToken() != XContentParser.Token.END_ARRAY) { - maskedFields.add(parser.text()); - } - break; + } } } @@ -157,33 +166,25 @@ private static RoleV7.Index parseIndexPermission(XContentParser parser) throws I throw new IllegalArgumentException("index_patterns is required for index permission"); } - return new RoleV7.Index(indexPatterns, allowedActions, dls, fls, maskedFields); + return new IndexPermission(indexPatterns, allowedActions); } - public String getDescription() { - return description; + public String getName() { + return name; } - public void setDescription(String description) { - this.description = description; + public void setName(String name) { + this.name = name; } public String getJti() { return jti; } - public void setJti(String jti) { - this.jti = jti; - } - public Instant getCreationTime() { return creationTime; } - public void setCreationTime(Instant creationTime) { - this.creationTime = creationTime; - } - public List getClusterPermissions() { return clusterPermissions; } @@ -195,7 +196,7 @@ public void setClusterPermissions(List clusterPermissions) { @Override public XContentBuilder toXContent(XContentBuilder xContentBuilder, ToXContent.Params params) throws IOException { xContentBuilder.startObject(); - xContentBuilder.field("description", description); + xContentBuilder.field("name", name); xContentBuilder.field("jti", jti); xContentBuilder.field("cluster_permissions", clusterPermissions); xContentBuilder.field("index_permissions", indexPermissions); @@ -204,11 +205,11 @@ public XContentBuilder toXContent(XContentBuilder xContentBuilder, ToXContent.Pa return xContentBuilder; } - public List getIndexPermissions() { + public List getIndexPermissions() { return indexPermissions; } - public void setIndexPermissions(List indexPermissions) { + public void setIndexPermissions(List indexPermissions) { this.indexPermissions = indexPermissions; } } 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 d2445ee966..dcec4a64c8 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java @@ -28,7 +28,6 @@ import org.opensearch.rest.BytesRestResponse; import org.opensearch.rest.RestHandler; import org.opensearch.rest.RestRequest; -import org.opensearch.security.securityconf.impl.v7.RoleV7; import org.opensearch.threadpool.ThreadPool; import static org.opensearch.rest.RestRequest.Method.DELETE; @@ -44,9 +43,6 @@ public class ApiTokenAction extends BaseRestHandler { private static final String INDEX_PERMISSIONS_FIELD = "index_permissions"; private static final String INDEX_PATTERN_FIELD = "index_pattern"; private static final String ALLOWED_ACTIONS_FIELD = "allowed_actions"; - private static final String DLS_FIELD = "dls"; - private static final String FLS_FIELD = "fls"; - private static final String MASKED_FIELDS_FIELD = "masked_fields"; private static final List ROUTES = addRoutesPrefix( ImmutableList.of( @@ -95,7 +91,7 @@ private RestChannelConsumer handleGet(RestRequest request, NodeClient client) { builder.startArray(); for (ApiToken token : tokens.values()) { builder.startObject(); - builder.field("name", token.getDescription()); + builder.field("name", token.getName()); builder.field("creation_time", token.getCreationTime().toEpochMilli()); builder.endObject(); } @@ -120,7 +116,7 @@ private RestChannelConsumer handlePost(RestRequest request, NodeClient client) { validateRequestParameters(requestBody); List clusterPermissions = extractClusterPermissions(requestBody); - List indexPermissions = extractIndexPermissions(requestBody); + List indexPermissions = extractIndexPermissions(requestBody); String token = apiTokenRepository.createApiToken( (String) requestBody.get(NAME_JSON_PROPERTY), @@ -193,7 +189,7 @@ List extractClusterPermissions(Map requestBody) { /** * Extracts and builds index permissions from the request body */ - List extractIndexPermissions(Map requestBody) { + List extractIndexPermissions(Map requestBody) { if (!requestBody.containsKey(INDEX_PERMISSIONS_FIELD)) { return Collections.emptyList(); } @@ -206,7 +202,7 @@ List extractIndexPermissions(Map requestBody) { /** * Creates a single RoleV7.Index permission from a permission map */ - RoleV7.Index createIndexPermission(Map indexPerm) { + ApiToken.IndexPermission createIndexPermission(Map indexPerm) { List indexPatterns; Object indexPatternObj = indexPerm.get(INDEX_PATTERN_FIELD); if (indexPatternObj instanceof String) { @@ -217,15 +213,7 @@ RoleV7.Index createIndexPermission(Map indexPerm) { List allowedActions = safeStringList(indexPerm.get(ALLOWED_ACTIONS_FIELD), ALLOWED_ACTIONS_FIELD); - String dls = (String) indexPerm.getOrDefault(DLS_FIELD, ""); - - List fls = indexPerm.containsKey(FLS_FIELD) ? safeStringList(indexPerm.get(FLS_FIELD), FLS_FIELD) : Collections.emptyList(); - - List maskedFields = indexPerm.containsKey(MASKED_FIELDS_FIELD) - ? safeStringList(indexPerm.get(MASKED_FIELDS_FIELD), MASKED_FIELDS_FIELD) - : Collections.emptyList(); - - return new RoleV7.Index(indexPatterns, allowedActions, dls, fls, maskedFields); + return new ApiToken.IndexPermission(indexPatterns, allowedActions); } /** @@ -265,10 +253,6 @@ void validateIndexPermissionsList(List> indexPermsList) { if (!(indexPatternObj instanceof String) && !(indexPatternObj instanceof List)) { throw new IllegalArgumentException(INDEX_PATTERN_FIELD + " must be a string or array of strings"); } - - if (indexPerm.containsKey(DLS_FIELD) && !(indexPerm.get(DLS_FIELD) instanceof String)) { - throw new IllegalArgumentException(DLS_FIELD + " must be a string"); - } } } 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 ab91ccb76b..7d4e45f36e 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandler.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandler.java @@ -45,8 +45,8 @@ public class ApiTokenIndexHandler { - private Client client; - private ClusterService clusterService; + private final Client client; + private final ClusterService clusterService; private static final Logger LOGGER = LogManager.getLogger(ApiTokenIndexHandler.class); public ApiTokenIndexHandler(Client client, ClusterService clusterService) { @@ -70,7 +70,7 @@ public String indexToken(ApiToken token) { }); client.index(request, irListener); - return token.getDescription(); + return token.getName(); } catch (IOException e) { throw new RuntimeException(e); @@ -114,7 +114,7 @@ public Map getApiTokens() { ) { ApiToken token = ApiToken.fromXContent(parser); - tokens.put(token.getDescription(), token); // Assuming description is the key + tokens.put(token.getName(), token); // Assuming description is the key } } return tokens; 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 6b3ab3fdce..dc096b065c 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java @@ -16,18 +16,18 @@ import org.opensearch.client.Client; import org.opensearch.cluster.service.ClusterService; -import org.opensearch.security.securityconf.impl.v7.RoleV7; public class ApiTokenRepository { - private ApiTokenIndexHandler apiTokenIndexHandler; + private final ApiTokenIndexHandler apiTokenIndexHandler; public ApiTokenRepository(Client client, ClusterService clusterService) { apiTokenIndexHandler = new ApiTokenIndexHandler(client, clusterService); } - public String createApiToken(String name, List clusterPermissions, List indexPermissions) { + public String createApiToken(String name, List clusterPermissions, List indexPermissions) { apiTokenIndexHandler.createApiTokenIndexIfAbsent(); // TODO: Implement logic of creating JTI to match against during authc/z + // TODO: Add validation on whether user is creating a token with a subset of their permissions return apiTokenIndexHandler.indexToken(new ApiToken(name, "test-token", clusterPermissions, indexPermissions)); } diff --git a/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java b/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java index 48f6f5aaed..2b2da40927 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java @@ -27,16 +27,11 @@ package org.opensearch.security.securityconf.impl.v7; -import java.io.IOException; import java.util.Collections; import java.util.List; -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; -import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.annotation.JsonProperty; -import org.opensearch.core.xcontent.ToXContent; -import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.security.securityconf.Hideable; import org.opensearch.security.securityconf.StaticDefinable; @@ -55,9 +50,7 @@ public RoleV7() { } - @JsonIgnoreProperties(value = { "fragment" }, ignoreUnknown = true) - @JsonInclude(JsonInclude.Include.NON_NULL) - public static class Index implements ToXContent { + public static class Index { private List index_patterns = Collections.emptyList(); private String dls; @@ -69,31 +62,6 @@ public Index() { super(); } - public Index(List indexPatterns, List allowedActions, String dls, List fls, List maskedFields) { - this.index_patterns = indexPatterns; - this.allowed_actions = allowedActions; - this.dls = dls; - this.fls = fls; - this.masked_fields = maskedFields; - } - - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject(); - - builder.field("index_patterns", index_patterns); - builder.field("allowed_actions", allowed_actions); - - builder.field("dls", dls); - - builder.field("fls", fls); - - builder.field("masked_fields", masked_fields); - - builder.endObject(); - return builder; - } - public List getIndex_patterns() { return index_patterns; } 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 287d1544b0..a5cc6394d6 100644 --- a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenActionTest.java +++ b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenActionTest.java @@ -19,8 +19,6 @@ import org.junit.Test; -import org.opensearch.security.securityconf.impl.v7.RoleV7; - import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.is; @@ -52,13 +50,10 @@ public void testCreateIndexPermission() { validPermission.put("fls", Arrays.asList("field1", "field2")); validPermission.put("masked_fields", Arrays.asList("sensitive1")); - RoleV7.Index result = apiTokenAction.createIndexPermission(validPermission); + ApiToken.IndexPermission result = apiTokenAction.createIndexPermission(validPermission); - assertThat(result.getIndex_patterns(), is(Arrays.asList("test-*"))); - assertThat(result.getAllowed_actions(), is(Arrays.asList("read"))); - assertThat(result.getDls(), is("")); - assertThat(result.getFls(), is(Arrays.asList("field1", "field2"))); - assertThat(result.getMasked_fields(), is(Arrays.asList("sensitive1"))); + assertThat(result.getIndexPatterns(), is(Arrays.asList("test-*"))); + assertThat(result.getAllowedActions(), is(Arrays.asList("read"))); } @Test diff --git a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java index 580e7b81c2..dc51d7834f 100644 --- a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java +++ b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java @@ -45,7 +45,6 @@ import org.opensearch.index.reindex.DeleteByQueryRequest; import org.opensearch.search.SearchHit; import org.opensearch.search.SearchHits; -import org.opensearch.security.securityconf.impl.v7.RoleV7; import org.opensearch.security.support.ConfigConstants; import org.mockito.ArgumentCaptor; @@ -185,13 +184,10 @@ public void testIndexTokenStoresToken() { // Create a real ApiToken List clusterPermissions = Arrays.asList("cluster:admin/something"); - List indexPermissions = Arrays.asList( - new RoleV7.Index( + List indexPermissions = Arrays.asList( + new ApiToken.IndexPermission( Arrays.asList("test-index-*"), - Arrays.asList("read", "write"), - null, // dls - null, // fls - null // masked_fields + Arrays.asList("read", "write") ) ); ApiToken token = new ApiToken( @@ -249,10 +245,9 @@ public void testGetApiTokens() throws IOException { "token1-description", "jti1", Arrays.asList("cluster:admin/something"), - Arrays.asList(new RoleV7.Index( + Arrays.asList(new ApiToken.IndexPermission( Arrays.asList("index1-*"), - Arrays.asList("read"), - null, null, null + Arrays.asList("read") )), Instant.now() ); @@ -262,10 +257,9 @@ public void testGetApiTokens() throws IOException { "token2-description", "jti2", Arrays.asList("cluster:admin/other"), - Arrays.asList(new RoleV7.Index( + Arrays.asList(new ApiToken.IndexPermission( Arrays.asList("index2-*"), - Arrays.asList("write"), - null, null, null + Arrays.asList("write") )), Instant.now() ); From 16c1615d86ca493c4af30b1bbad426b7c4e432d8 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Wed, 11 Dec 2024 15:31:04 -0500 Subject: [PATCH 19/34] Cleanup using constants for better refactorability Signed-off-by: Derek Ho --- .../security/action/apitokens/ApiToken.java | 53 +++++++++++-------- .../action/apitokens/ApiTokenAction.java | 26 ++++----- .../apitokens/ApiTokenIndexHandler.java | 4 +- 3 files changed, 45 insertions(+), 38 deletions(-) diff --git a/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java b/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java index 4adbe6f9bc..f857570648 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java @@ -21,6 +21,14 @@ import org.opensearch.core.xcontent.XContentParser; public class ApiToken implements ToXContent { + public static final String NAME_FIELD = "name"; + public static final String JTI_FIELD = "jti"; + public static final String CREATION_TIME_FIELD = "creation_time"; + public static final String CLUSTER_PERMISSIONS_FIELD = "cluster_permissions"; + public static final String INDEX_PERMISSIONS_FIELD = "index_permissions"; + public static final String INDEX_PATTERN_FIELD = "index_pattern"; + public static final String ALLOWED_ACTIONS_FIELD = "allowed_actions"; + private String name; private final String jti; private final Instant creationTime; @@ -71,15 +79,15 @@ public List getIndexPatterns() { @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); - builder.array("index_patterns", indexPatterns.toArray(new String[0])); - builder.array("allowed_actions", allowedActions.toArray(new String[0])); + builder.array(INDEX_PATTERN_FIELD, indexPatterns.toArray(new String[0])); + builder.array(ALLOWED_ACTIONS_FIELD, allowedActions.toArray(new String[0])); builder.endObject(); return builder; } } public static ApiToken fromXContent(XContentParser parser) throws IOException { - String description = null; + String name = null; String jti = null; List clusterPermissions = new ArrayList<>(); List indexPermissions = new ArrayList<>(); @@ -93,24 +101,24 @@ public static ApiToken fromXContent(XContentParser parser) throws IOException { currentFieldName = parser.currentName(); } else if (token.isValue()) { switch (currentFieldName) { - case "description": - description = parser.text(); + case NAME_FIELD: + name = parser.text(); break; - case "jti": + case JTI_FIELD: jti = parser.text(); break; - case "creation_time": + case CREATION_TIME_FIELD: creationTime = Instant.ofEpochMilli(parser.longValue()); break; } } else if (token == XContentParser.Token.START_ARRAY) { switch (currentFieldName) { - case "cluster_permissions": + case CLUSTER_PERMISSIONS_FIELD: while (parser.nextToken() != XContentParser.Token.END_ARRAY) { clusterPermissions.add(parser.text()); } break; - case "index_permissions": + case INDEX_PERMISSIONS_FIELD: while (parser.nextToken() != XContentParser.Token.END_ARRAY) { if (parser.currentToken() == XContentParser.Token.START_OBJECT) { indexPermissions.add(parseIndexPermission(parser)); @@ -121,18 +129,17 @@ public static ApiToken fromXContent(XContentParser parser) throws IOException { } } - // Validate required fields - if (description == null) { - throw new IllegalArgumentException("description is required"); + if (name == null) { + throw new IllegalArgumentException(NAME_FIELD + " is required"); } if (jti == null) { - throw new IllegalArgumentException("jti is required"); + throw new IllegalArgumentException(JTI_FIELD + " is required"); } if (creationTime == null) { - throw new IllegalArgumentException("creation_time is required"); + throw new IllegalArgumentException(CREATION_TIME_FIELD + " is required"); } - return new ApiToken(description, jti, clusterPermissions, indexPermissions, creationTime); + return new ApiToken(name, jti, clusterPermissions, indexPermissions, creationTime); } private static IndexPermission parseIndexPermission(XContentParser parser) throws IOException { @@ -147,12 +154,12 @@ private static IndexPermission parseIndexPermission(XContentParser parser) throw currentFieldName = parser.currentName(); } else if (token == XContentParser.Token.START_ARRAY) { switch (currentFieldName) { - case "index_patterns": + case INDEX_PATTERN_FIELD: while (parser.nextToken() != XContentParser.Token.END_ARRAY) { indexPatterns.add(parser.text()); } break; - case "allowed_actions": + case ALLOWED_ACTIONS_FIELD: while (parser.nextToken() != XContentParser.Token.END_ARRAY) { allowedActions.add(parser.text()); } @@ -163,7 +170,7 @@ private static IndexPermission parseIndexPermission(XContentParser parser) throw } if (indexPatterns.isEmpty()) { - throw new IllegalArgumentException("index_patterns is required for index permission"); + throw new IllegalArgumentException(INDEX_PATTERN_FIELD + " is required for index permission"); } return new IndexPermission(indexPatterns, allowedActions); @@ -196,11 +203,11 @@ public void setClusterPermissions(List clusterPermissions) { @Override public XContentBuilder toXContent(XContentBuilder xContentBuilder, ToXContent.Params params) throws IOException { xContentBuilder.startObject(); - xContentBuilder.field("name", name); - xContentBuilder.field("jti", jti); - xContentBuilder.field("cluster_permissions", clusterPermissions); - xContentBuilder.field("index_permissions", indexPermissions); - xContentBuilder.field("creation_time", creationTime.toEpochMilli()); + xContentBuilder.field(NAME_FIELD, name); + xContentBuilder.field(JTI_FIELD, jti); + xContentBuilder.field(CLUSTER_PERMISSIONS_FIELD, clusterPermissions); + xContentBuilder.field(INDEX_PERMISSIONS_FIELD, indexPermissions); + xContentBuilder.field(CREATION_TIME_FIELD, creationTime.toEpochMilli()); xContentBuilder.endObject(); return xContentBuilder; } 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 dcec4a64c8..4ff43ec46f 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java @@ -33,17 +33,17 @@ import static org.opensearch.rest.RestRequest.Method.DELETE; import static org.opensearch.rest.RestRequest.Method.GET; import static org.opensearch.rest.RestRequest.Method.POST; +import static org.opensearch.security.action.apitokens.ApiToken.ALLOWED_ACTIONS_FIELD; +import static org.opensearch.security.action.apitokens.ApiToken.CLUSTER_PERMISSIONS_FIELD; +import static org.opensearch.security.action.apitokens.ApiToken.CREATION_TIME_FIELD; +import static org.opensearch.security.action.apitokens.ApiToken.INDEX_PATTERN_FIELD; +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; public class ApiTokenAction extends BaseRestHandler { private final ApiTokenRepository apiTokenRepository; - private static final String NAME_JSON_PROPERTY = "name"; - private static final String CLUSTER_PERMISSIONS_FIELD = "cluster_permissions"; - private static final String INDEX_PERMISSIONS_FIELD = "index_permissions"; - private static final String INDEX_PATTERN_FIELD = "index_pattern"; - private static final String ALLOWED_ACTIONS_FIELD = "allowed_actions"; - private static final List ROUTES = addRoutesPrefix( ImmutableList.of( new RestHandler.Route(POST, "/apitokens"), @@ -91,8 +91,8 @@ private RestChannelConsumer handleGet(RestRequest request, NodeClient client) { builder.startArray(); for (ApiToken token : tokens.values()) { builder.startObject(); - builder.field("name", token.getName()); - builder.field("creation_time", token.getCreationTime().toEpochMilli()); + builder.field(NAME_FIELD, token.getName()); + builder.field(CREATION_TIME_FIELD, token.getCreationTime().toEpochMilli()); builder.endObject(); } builder.endArray(); @@ -119,7 +119,7 @@ private RestChannelConsumer handlePost(RestRequest request, NodeClient client) { List indexPermissions = extractIndexPermissions(requestBody); String token = apiTokenRepository.createApiToken( - (String) requestBody.get(NAME_JSON_PROPERTY), + (String) requestBody.get(NAME_FIELD), clusterPermissions, indexPermissions ); @@ -220,8 +220,8 @@ ApiToken.IndexPermission createIndexPermission(Map indexPerm) { * Validates the request parameters */ void validateRequestParameters(Map requestBody) { - if (!requestBody.containsKey(NAME_JSON_PROPERTY)) { - throw new IllegalArgumentException("Missing required parameter: " + NAME_JSON_PROPERTY); + if (!requestBody.containsKey(NAME_FIELD)) { + throw new IllegalArgumentException("Missing required parameter: " + NAME_FIELD); } if (requestBody.containsKey(CLUSTER_PERMISSIONS_FIELD)) { @@ -264,10 +264,10 @@ private RestChannelConsumer handleDelete(RestRequest request, NodeClient client) final Map requestBody = request.contentOrSourceParamParser().map(); validateRequestParameters(requestBody); - apiTokenRepository.deleteApiToken((String) requestBody.get(NAME_JSON_PROPERTY)); + apiTokenRepository.deleteApiToken((String) requestBody.get(NAME_FIELD)); builder.startObject(); - builder.field("message", "token " + requestBody.get(NAME_JSON_PROPERTY) + " deleted successfully."); + builder.field("message", "token " + requestBody.get(NAME_FIELD) + " deleted successfully."); builder.endObject(); response = new BytesRestResponse(RestStatus.OK, builder); 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 7d4e45f36e..e1caa874f4 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandler.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandler.java @@ -82,7 +82,7 @@ public void deleteToken(String name) throws ApiTokenException { try (final ThreadContext.StoredContext ctx = client.threadPool().getThreadContext().stashContext()) { DeleteByQueryRequest request = new DeleteByQueryRequest(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX).setQuery( QueryBuilders.matchQuery("description", name) - ).setRefresh(true); // This will refresh the index after deletion + ).setRefresh(true); BulkByScrollResponse response = client.execute(DeleteByQueryAction.INSTANCE, request).actionGet(); @@ -114,7 +114,7 @@ public Map getApiTokens() { ) { ApiToken token = ApiToken.fromXContent(parser); - tokens.put(token.getName(), token); // Assuming description is the key + tokens.put(token.getName(), token); } } return tokens; From dad767b9559dd2e9f4d059059a68c642a8aae935 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Wed, 11 Dec 2024 15:59:24 -0500 Subject: [PATCH 20/34] General cleanup Signed-off-by: Derek Ho --- .../action/apitokens/ApiTokenAction.java | 2 +- .../apitokens/ApiTokenIndexHandler.java | 8 ++++--- .../action/apitokens/ApiTokenRepository.java | 4 ++-- .../security/auth/BackendRegistry.java | 14 ------------ .../action/apitokens/ApiTokenActionTest.java | 22 ++++++------------- .../apitokens/ApiTokenIndexHandlerTest.java | 18 +++++++-------- 6 files changed, 24 insertions(+), 44 deletions(-) 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 4ff43ec46f..9fb1ef6d11 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java @@ -200,7 +200,7 @@ List extractIndexPermissions(Map reque } /** - * Creates a single RoleV7.Index permission from a permission map + * Creates a single index permission from a permission map */ ApiToken.IndexPermission createIndexPermission(Map indexPerm) { List indexPatterns; 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 e1caa874f4..904087cd3b 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandler.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandler.java @@ -43,6 +43,8 @@ import org.opensearch.search.builder.SearchSourceBuilder; import org.opensearch.security.support.ConfigConstants; +import static org.opensearch.security.action.apitokens.ApiToken.NAME_FIELD; + public class ApiTokenIndexHandler { private final Client client; @@ -54,7 +56,7 @@ public ApiTokenIndexHandler(Client client, ClusterService clusterService) { this.clusterService = clusterService; } - public String indexToken(ApiToken token) { + public String indexTokenPayload(ApiToken token) { try (final ThreadContext.StoredContext ctx = client.threadPool().getThreadContext().stashContext()) { XContentBuilder builder = XContentFactory.jsonBuilder(); @@ -81,7 +83,7 @@ public String indexToken(ApiToken token) { public void deleteToken(String name) throws ApiTokenException { try (final ThreadContext.StoredContext ctx = client.threadPool().getThreadContext().stashContext()) { DeleteByQueryRequest request = new DeleteByQueryRequest(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX).setQuery( - QueryBuilders.matchQuery("description", name) + QueryBuilders.matchQuery(NAME_FIELD, name) ).setRefresh(true); BulkByScrollResponse response = client.execute(DeleteByQueryAction.INSTANCE, request).actionGet(); @@ -95,7 +97,7 @@ public void deleteToken(String name) throws ApiTokenException { } } - public Map getApiTokens() { + public Map getTokenPayloads() { try (final ThreadContext.StoredContext ctx = client.threadPool().getThreadContext().stashContext()) { SearchRequest searchRequest = new SearchRequest(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX); searchRequest.source(new SearchSourceBuilder()); 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 dc096b065c..8ded6a897f 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java @@ -28,7 +28,7 @@ public String createApiToken(String name, List clusterPermissions, List< apiTokenIndexHandler.createApiTokenIndexIfAbsent(); // TODO: Implement logic of creating JTI to match against during authc/z // TODO: Add validation on whether user is creating a token with a subset of their permissions - return apiTokenIndexHandler.indexToken(new ApiToken(name, "test-token", clusterPermissions, indexPermissions)); + return apiTokenIndexHandler.indexTokenPayload(new ApiToken(name, "test-token", clusterPermissions, indexPermissions)); } public void deleteApiToken(String name) throws ApiTokenException { @@ -38,7 +38,7 @@ public void deleteApiToken(String name) throws ApiTokenException { public Map getApiTokens() { apiTokenIndexHandler.createApiTokenIndexIfAbsent(); - return apiTokenIndexHandler.getApiTokens(); + return apiTokenIndexHandler.getTokenPayloads(); } } diff --git a/src/main/java/org/opensearch/security/auth/BackendRegistry.java b/src/main/java/org/opensearch/security/auth/BackendRegistry.java index 163876b6df..0b00bcf943 100644 --- a/src/main/java/org/opensearch/security/auth/BackendRegistry.java +++ b/src/main/java/org/opensearch/security/auth/BackendRegistry.java @@ -103,7 +103,6 @@ public class BackendRegistry { private Cache userCache; // rest standard private Cache restImpersonationCache; // used for rest impersonation private Cache> restRoleCache; // - private Cache apiTokensCache; private void createCaches() { userCache = CacheBuilder.newBuilder() @@ -136,18 +135,6 @@ public void onRemoval(RemovalNotification> notification) { }) .build(); - /* TODO: Listen in to index events on API Tokens index */ - apiTokensCache = CacheBuilder.newBuilder() - .expireAfterWrite(ttlInMin, TimeUnit.MINUTES) - .removalListener( - (RemovalListener) notification -> log.debug( - "Clear api token cache for {} due to {}", - notification.getKey(), - notification.getCause() - ) - ) - .build(); - } public BackendRegistry( @@ -183,7 +170,6 @@ public void invalidateCache() { userCache.invalidateAll(); restImpersonationCache.invalidateAll(); restRoleCache.invalidateAll(); - apiTokensCache.invalidateAll(); } @Subscribe 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 a5cc6394d6..9e48e9b1a8 100644 --- a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenActionTest.java +++ b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenActionTest.java @@ -26,11 +26,10 @@ public class ApiTokenActionTest { - private final ApiTokenAction apiTokenAction = new ApiTokenAction(null, null, null); // repository not needed for these tests + private final ApiTokenAction apiTokenAction = new ApiTokenAction(null, null, null); @Test public void testSafeStringList() { - // Valid case List result = apiTokenAction.safeStringList(Arrays.asList("test1", "test2"), "test_field"); assertThat(result, is(Arrays.asList("test1", "test2"))); @@ -45,20 +44,16 @@ public void testSafeStringList() { public void testCreateIndexPermission() { Map validPermission = new HashMap<>(); validPermission.put("index_pattern", "test-*"); - validPermission.put("allowed_actions", Arrays.asList("read")); - validPermission.put("dls", ""); - validPermission.put("fls", Arrays.asList("field1", "field2")); - validPermission.put("masked_fields", Arrays.asList("sensitive1")); + validPermission.put("allowed_actions", List.of("read")); ApiToken.IndexPermission result = apiTokenAction.createIndexPermission(validPermission); - assertThat(result.getIndexPatterns(), is(Arrays.asList("test-*"))); - assertThat(result.getAllowedActions(), is(Arrays.asList("read"))); + assertThat(result.getIndexPatterns(), is(List.of("test-*"))); + assertThat(result.getAllowedActions(), is(List.of("read"))); } @Test public void testValidateRequestParameters() { - // Valid case Map validRequest = new HashMap<>(); validRequest.put("name", "test-token"); validRequest.put("cluster_permissions", Arrays.asList("perm1", "perm2")); @@ -77,15 +72,14 @@ public void testValidateRequestParameters() { @Test public void testValidateIndexPermissionsList() { - // Valid case Map validPerm = new HashMap<>(); validPerm.put("index_pattern", "test-*"); - validPerm.put("allowed_actions", Arrays.asList("read")); + validPerm.put("allowed_actions", List.of("read")); apiTokenAction.validateIndexPermissionsList(Collections.singletonList(validPerm)); // Missing index_pattern Map missingPattern = new HashMap<>(); - missingPattern.put("allowed_actions", Arrays.asList("read")); + missingPattern.put("allowed_actions", List.of("read")); assertThrows( IllegalArgumentException.class, () -> apiTokenAction.validateIndexPermissionsList(Collections.singletonList(missingPattern)) @@ -102,7 +96,7 @@ public void testValidateIndexPermissionsList() { // Invalid index_pattern type Map invalidPattern = new HashMap<>(); invalidPattern.put("index_pattern", 123); - invalidPattern.put("allowed_actions", Arrays.asList("read")); + invalidPattern.put("allowed_actions", List.of("read")); assertThrows( IllegalArgumentException.class, () -> apiTokenAction.validateIndexPermissionsList(Collections.singletonList(invalidPattern)) @@ -113,10 +107,8 @@ public void testValidateIndexPermissionsList() { public void testExtractClusterPermissions() { Map requestBody = new HashMap<>(); - // Empty case assertThat(apiTokenAction.extractClusterPermissions(requestBody), is(empty())); - // Valid case requestBody.put("cluster_permissions", Arrays.asList("perm1", "perm2")); assertThat(apiTokenAction.extractClusterPermissions(requestBody), is(Arrays.asList("perm1", "perm2"))); } diff --git a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java index dc51d7834f..6456d56b16 100644 --- a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java +++ b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java @@ -55,6 +55,7 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; +import static org.opensearch.security.action.apitokens.ApiToken.NAME_FIELD; import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.RETURNS_DEEP_STUBS; @@ -133,10 +134,10 @@ public void testDeleteApiTokeCallsDeleteByQueryWithSuppliedName() { ArgumentCaptor captor = ArgumentCaptor.forClass(DeleteByQueryRequest.class); verify(client).execute(eq(DeleteByQueryAction.INSTANCE), captor.capture()); - // Verify the captured request has the correct query parameters + // Captured request has the correct name DeleteByQueryRequest capturedRequest = captor.getValue(); MatchQueryBuilder query = (MatchQueryBuilder) capturedRequest.getSearchRequest().source().query(); - assertThat(query.fieldName(), equalTo("description")); + assertThat(query.fieldName(), equalTo(NAME_FIELD)); assertThat(query.value(), equalTo(tokenName)); } @@ -163,7 +164,7 @@ public void testDeleteTokenThrowsExceptionWhenNoDocumentsDeleted() { public void testDeleteTokenSucceedsWhenDocumentIsDeleted() { when(metadata.hasConcreteIndex(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX)).thenReturn(true); - // Mock response with 1 deleted document + // 1 deleted document PlainActionFuture future = new PlainActionFuture<>(); BulkByScrollResponse response = mock(BulkByScrollResponse.class); when(response.getDeleted()).thenReturn(1L); @@ -179,10 +180,9 @@ public void testDeleteTokenSucceedsWhenDocumentIsDeleted() { } @Test - public void testIndexTokenStoresToken() { + public void testIndexTokenStoresTokenPayload() { when(metadata.hasConcreteIndex(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX)).thenReturn(true); - // Create a real ApiToken List clusterPermissions = Arrays.asList("cluster:admin/something"); List indexPermissions = Arrays.asList( new ApiToken.IndexPermission( @@ -216,7 +216,7 @@ public void testIndexTokenStoresToken() { return null; }).when(client).index(any(IndexRequest.class), listenerCaptor.capture()); - indexHandler.indexToken(token); + indexHandler.indexTokenPayload(token); // Verify the index request ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(IndexRequest.class); @@ -225,7 +225,7 @@ public void testIndexTokenStoresToken() { IndexRequest capturedRequest = requestCaptor.getValue(); assertThat(capturedRequest.index(), equalTo(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX)); - // Convert the source to a string and verify contents + // verify contents String source = capturedRequest.source().utf8ToString(); assertThat(source, containsString("test-token-description")); assertThat(source, containsString("test-jti")); @@ -234,7 +234,7 @@ public void testIndexTokenStoresToken() { } @Test - public void testGetApiTokens() throws IOException { + public void testGetTokenPayloads() throws IOException { when(metadata.hasConcreteIndex(ConfigConstants.OPENSEARCH_API_TOKENS_INDEX)).thenReturn(true); // Create sample search hits @@ -286,7 +286,7 @@ public void testGetApiTokens() throws IOException { when(client.search(any(SearchRequest.class))).thenReturn(future); // Get tokens and verify - Map resultTokens = indexHandler.getApiTokens(); + Map resultTokens = indexHandler.getTokenPayloads(); assertThat(resultTokens.size(), equalTo(2)); assertThat(resultTokens.containsKey("token1-description"), is(true)); From 98d384714bd0684930c4f7987fc29a914a4f1aa3 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Fri, 13 Dec 2024 09:53:07 -0500 Subject: [PATCH 21/34] Add to dynamic config model and expiration Signed-off-by: Derek Ho --- .../security/OpenSearchSecurityPlugin.java | 2 +- .../security/action/apitokens/ApiToken.java | 16 ++++-- .../action/apitokens/ApiTokenAction.java | 18 +++++-- .../action/apitokens/ApiTokenRepository.java | 12 +++-- .../identity/SecurityTokenManager.java | 24 ++++++--- .../securityconf/DynamicConfigModel.java | 2 + .../securityconf/DynamicConfigModelV7.java | 7 +++ .../securityconf/impl/v7/ConfigV7.java | 51 +++++++++++++++++++ .../apitokens/ApiTokenIndexHandlerTest.java | 9 ++-- 9 files changed, 118 insertions(+), 23 deletions(-) diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 381e181942..6c308fd035 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -636,7 +636,7 @@ public List getRestHandlers( ) ); handlers.add(new CreateOnBehalfOfTokenAction(tokenManager)); - handlers.add(new ApiTokenAction(cs, threadPool, localClient)); + handlers.add(new ApiTokenAction(cs, localClient, settings)); handlers.addAll( SecurityRestApiActions.getHandler( settings, diff --git a/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java b/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java index f857570648..72c1e55062 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java @@ -28,20 +28,22 @@ public class ApiToken implements ToXContent { public static final String INDEX_PERMISSIONS_FIELD = "index_permissions"; public static final String INDEX_PATTERN_FIELD = "index_pattern"; public static final String ALLOWED_ACTIONS_FIELD = "allowed_actions"; + public static final String EXPIRATION_FIELD = "expiration"; private String name; private final String jti; private final Instant creationTime; private List clusterPermissions; private List indexPermissions; + private final long expiration; - public ApiToken(String name, String jti, List clusterPermissions, List indexPermissions) { + public ApiToken(String name, String jti, List clusterPermissions, List indexPermissions, Long expiration) { this.creationTime = Instant.now(); this.name = name; this.jti = jti; this.clusterPermissions = clusterPermissions; this.indexPermissions = indexPermissions; - + this.expiration = expiration; } public ApiToken( @@ -49,13 +51,15 @@ public ApiToken( String jti, List clusterPermissions, List indexPermissions, - Instant creationTime + Instant creationTime, + Long expiration ) { this.name = description; this.jti = jti; this.clusterPermissions = clusterPermissions; this.indexPermissions = indexPermissions; this.creationTime = creationTime; + this.expiration = expiration; } @@ -92,6 +96,7 @@ public static ApiToken fromXContent(XContentParser parser) throws IOException { List clusterPermissions = new ArrayList<>(); List indexPermissions = new ArrayList<>(); Instant creationTime = null; + Long expiration = Long.MAX_VALUE; XContentParser.Token token; String currentFieldName = null; @@ -110,6 +115,9 @@ public static ApiToken fromXContent(XContentParser parser) throws IOException { case CREATION_TIME_FIELD: creationTime = Instant.ofEpochMilli(parser.longValue()); break; + case EXPIRATION_FIELD: + expiration = parser.longValue(); + break; } } else if (token == XContentParser.Token.START_ARRAY) { switch (currentFieldName) { @@ -139,7 +147,7 @@ public static ApiToken fromXContent(XContentParser parser) throws IOException { throw new IllegalArgumentException(CREATION_TIME_FIELD + " is required"); } - return new ApiToken(name, jti, clusterPermissions, indexPermissions, creationTime); + return new ApiToken(name, jti, clusterPermissions, indexPermissions, creationTime, expiration); } private static IndexPermission parseIndexPermission(XContentParser parser) throws IOException { 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 9fb1ef6d11..7d79f3d985 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java @@ -22,13 +22,13 @@ import org.opensearch.client.Client; import org.opensearch.client.node.NodeClient; import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.settings.Settings; 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.RestHandler; import org.opensearch.rest.RestRequest; -import org.opensearch.threadpool.ThreadPool; import static org.opensearch.rest.RestRequest.Method.DELETE; import static org.opensearch.rest.RestRequest.Method.GET; @@ -36,6 +36,7 @@ import static org.opensearch.security.action.apitokens.ApiToken.ALLOWED_ACTIONS_FIELD; import static org.opensearch.security.action.apitokens.ApiToken.CLUSTER_PERMISSIONS_FIELD; import static org.opensearch.security.action.apitokens.ApiToken.CREATION_TIME_FIELD; +import static org.opensearch.security.action.apitokens.ApiToken.EXPIRATION_FIELD; import static org.opensearch.security.action.apitokens.ApiToken.INDEX_PATTERN_FIELD; import static org.opensearch.security.action.apitokens.ApiToken.INDEX_PERMISSIONS_FIELD; import static org.opensearch.security.action.apitokens.ApiToken.NAME_FIELD; @@ -52,8 +53,8 @@ public class ApiTokenAction extends BaseRestHandler { ) ); - public ApiTokenAction(ClusterService clusterService, ThreadPool threadPool, Client client) { - this.apiTokenRepository = new ApiTokenRepository(client, clusterService); + public ApiTokenAction(ClusterService clusterService, Client client, Settings settings) { + this.apiTokenRepository = new ApiTokenRepository(client, clusterService, settings); } @Override @@ -121,7 +122,9 @@ private RestChannelConsumer handlePost(RestRequest request, NodeClient client) { String token = apiTokenRepository.createApiToken( (String) requestBody.get(NAME_FIELD), clusterPermissions, - indexPermissions + indexPermissions, + (Long) requestBody.getOrDefault(EXPIRATION_FIELD, Long.MAX_VALUE) + ); builder.startObject(); @@ -224,6 +227,13 @@ void validateRequestParameters(Map requestBody) { throw new IllegalArgumentException("Missing required parameter: " + NAME_FIELD); } + if (requestBody.containsKey(EXPIRATION_FIELD)) { + Object permissions = requestBody.get(EXPIRATION_FIELD); + if (!(permissions instanceof Long)) { + throw new IllegalArgumentException(EXPIRATION_FIELD + " must be an long"); + } + } + if (requestBody.containsKey(CLUSTER_PERMISSIONS_FIELD)) { Object permissions = requestBody.get(CLUSTER_PERMISSIONS_FIELD); if (!(permissions instanceof List)) { 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 8ded6a897f..05aebc562e 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java @@ -16,19 +16,25 @@ import org.opensearch.client.Client; import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.settings.Settings; public class ApiTokenRepository { private final ApiTokenIndexHandler apiTokenIndexHandler; - public ApiTokenRepository(Client client, ClusterService clusterService) { + public ApiTokenRepository(Client client, ClusterService clusterService, Settings settings) { apiTokenIndexHandler = new ApiTokenIndexHandler(client, clusterService); } - public String createApiToken(String name, List clusterPermissions, List indexPermissions) { + public String createApiToken( + String name, + List clusterPermissions, + List indexPermissions, + Long expiration + ) { apiTokenIndexHandler.createApiTokenIndexIfAbsent(); // TODO: Implement logic of creating JTI to match against during authc/z // TODO: Add validation on whether user is creating a token with a subset of their permissions - return apiTokenIndexHandler.indexTokenPayload(new ApiToken(name, "test-token", clusterPermissions, indexPermissions)); + return apiTokenIndexHandler.indexTokenPayload(new ApiToken(name, "test-token", clusterPermissions, indexPermissions, expiration)); } public void deleteApiToken(String name) throws ApiTokenException { diff --git a/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java b/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java index 8a0c3e85f1..69b757feb8 100644 --- a/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java +++ b/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java @@ -50,7 +50,8 @@ public class SecurityTokenManager implements TokenManager { private final ThreadPool threadPool; private final UserService userService; - private JwtVendor jwtVendor = null; + private JwtVendor oboJwtVendor = null; + private JwtVendor apiTokenJwtVendor = null; private ConfigModel configModel = null; public SecurityTokenManager(final ClusterService cs, final ThreadPool threadPool, final UserService userService) { @@ -67,11 +68,14 @@ public void onConfigModelChanged(final ConfigModel configModel) { @Subscribe public void onDynamicConfigModelChanged(final DynamicConfigModel dcm) { final Settings oboSettings = dcm.getDynamicOnBehalfOfSettings(); - final Boolean enabled = oboSettings.getAsBoolean("enabled", false); - if (enabled) { - jwtVendor = createJwtVendor(oboSettings); - } else { - jwtVendor = null; + final Boolean oboEnabled = oboSettings.getAsBoolean("enabled", false); + if (oboEnabled) { + oboJwtVendor = createJwtVendor(oboSettings); + } + final Settings apiTokenSettings = dcm.getDynamicApiTokenSettings(); + final Boolean apiTokenEnabled = apiTokenSettings.getAsBoolean("enabled", false); + if (apiTokenEnabled) { + apiTokenJwtVendor = createJwtVendor(apiTokenSettings); } } @@ -86,7 +90,11 @@ JwtVendor createJwtVendor(final Settings settings) { } public boolean issueOnBehalfOfTokenAllowed() { - return jwtVendor != null && configModel != null; + return oboJwtVendor != null && configModel != null; + } + + public boolean issueApiTokenAllowed() { + return apiTokenJwtVendor != null && configModel != null; } @Override @@ -116,7 +124,7 @@ public ExpiringBearerAuthToken issueOnBehalfOfToken(final Subject subject, final final Set mappedRoles = configModel.mapSecurityRoles(user, callerAddress); try { - return jwtVendor.createJwt( + return oboJwtVendor.createJwt( cs.getClusterName().value(), user.getName(), claims.getAudience(), diff --git a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModel.java b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModel.java index 064f555a75..0d56a41c23 100644 --- a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModel.java +++ b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModel.java @@ -110,6 +110,8 @@ public abstract class DynamicConfigModel { public abstract Settings getDynamicOnBehalfOfSettings(); + public abstract Settings getDynamicApiTokenSettings(); + protected final Map authImplMap = new HashMap<>(); public DynamicConfigModel() { diff --git a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java index 4bc9e82882..43179b0eed 100644 --- a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java +++ b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java @@ -234,6 +234,13 @@ public Settings getDynamicOnBehalfOfSettings() { .build(); } + @Override + public Settings getDynamicApiTokenSettings() { + return Settings.builder() + .put(Settings.builder().loadFromSource(config.dynamic.api_token_settings.configAsJson(), XContentType.JSON).build()) + .build(); + } + private void buildAAA() { final SortedSet restAuthDomains0 = new TreeSet<>(); diff --git a/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java b/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java index 77fb973a52..044b6d4165 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java @@ -86,6 +86,7 @@ public static class Dynamic { public String transport_userrname_attribute; public boolean do_not_fail_on_forbidden_empty; public OnBehalfOfSettings on_behalf_of = new OnBehalfOfSettings(); + public ApiTokenSettings api_token_settings = new ApiTokenSettings(); @Override public String toString() { @@ -101,6 +102,8 @@ public String toString() { + authz + ", on_behalf_of=" + on_behalf_of + + ", api_tokens=" + + api_token_settings + "]"; } } @@ -495,4 +498,52 @@ public String toString() { } } + public static class ApiTokenSettings { + @JsonProperty("enabled") + private Boolean enabled = Boolean.FALSE; + @JsonProperty("signing_key") + private String signingKey; + @JsonProperty("encryption_key") + private String encryptionKey; + + @JsonIgnore + public String configAsJson() { + try { + return DefaultObjectMapper.writeValueAsString(this, false); + } catch (JsonProcessingException e) { + throw new RuntimeException(e); + } + } + + public Boolean getEnabled() { + return enabled; + } + + public void setEnabled(Boolean oboEnabled) { + this.enabled = oboEnabled; + } + + public String getSigningKey() { + return signingKey; + } + + public void setSigningKey(String signingKey) { + this.signingKey = signingKey; + } + + public String getEncryptionKey() { + return encryptionKey; + } + + public void setEncryptionKey(String encryptionKey) { + this.encryptionKey = encryptionKey; + } + + @Override + public String toString() { + return "ApiTokens [ enabled=" + enabled + ", signing_key=" + signingKey + ", encryption_key=" + encryptionKey + "]"; + } + + } + } diff --git a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java index 6456d56b16..0605f50ab9 100644 --- a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java +++ b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java @@ -195,7 +195,8 @@ public void testIndexTokenStoresTokenPayload() { "test-jti", clusterPermissions, indexPermissions, - Instant.now() + Instant.now(), + Long.MAX_VALUE ); // Mock the index method with ActionListener @@ -249,7 +250,8 @@ public void testGetTokenPayloads() throws IOException { Arrays.asList("index1-*"), Arrays.asList("read") )), - Instant.now() + Instant.now(), + Long.MAX_VALUE ); // Second token @@ -261,7 +263,8 @@ public void testGetTokenPayloads() throws IOException { Arrays.asList("index2-*"), Arrays.asList("write") )), - Instant.now() + Instant.now(), + Long.MAX_VALUE ); // Convert tokens to XContent and create SearchHits From fddd37c332924211fe7879730890800b5400f95c Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Mon, 16 Dec 2024 15:50:38 -0500 Subject: [PATCH 22/34] Spotless and missing merge conflict Signed-off-by: Derek Ho --- .../org/opensearch/security/OpenSearchSecurityPlugin.java | 4 ---- .../opensearch/security/action/apitokens/ApiTokenAction.java | 4 ---- .../security/action/apitokens/ApiTokenIndexHandler.java | 1 - 3 files changed, 9 deletions(-) diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index e77ae41679..381e181942 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -636,11 +636,7 @@ public List getRestHandlers( ) ); handlers.add(new CreateOnBehalfOfTokenAction(tokenManager)); -<<<<<<< HEAD - handlers.add(new ApiTokenAction(cs, localClient, settings)); -======= handlers.add(new ApiTokenAction(cs, threadPool, localClient)); ->>>>>>> 3177c349d27b0a3758dfdbba417def1b85902ed1 handlers.addAll( SecurityRestApiActions.getHandler( settings, 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 bf3c33bf3a..2c7f7408da 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java @@ -55,7 +55,6 @@ public class ApiTokenAction extends BaseRestHandler { ) ); - public ApiTokenAction(ClusterService clusterService, ThreadPool threadPool, Client client) { this.apiTokenRepository = new ApiTokenRepository(client, clusterService); } @@ -146,8 +145,6 @@ private RestChannelConsumer handlePost(RestRequest request, NodeClient client) { }; } - - /** * Extracts cluster permissions from the request body */ @@ -177,7 +174,6 @@ ApiToken.IndexPermission createIndexPermission(Map indexPerm) { List allowedActions = safeStringList(indexPerm.get(ALLOWED_ACTIONS_FIELD), ALLOWED_ACTIONS_FIELD); - return new ApiToken.IndexPermission(indexPatterns, allowedActions); } 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 2ce8a278d6..488229a319 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandler.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandler.java @@ -108,7 +108,6 @@ public void deleteToken(String name) throws ApiTokenException { } } - public Map getTokenMetadatas() { final var originalUserAndRemoteAddress = Utils.userAndRemoteAddressFrom(client.threadPool().getThreadContext()); try (final ThreadContext.StoredContext ctx = client.threadPool().getThreadContext().stashContext()) { From ae4e8f88874ebd037a2665a2d0d9fdf81eca09b0 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Tue, 17 Dec 2024 15:03:40 -0500 Subject: [PATCH 23/34] Inject security token manager and now test jti gets indexed Signed-off-by: Derek Ho --- .../security/OpenSearchSecurityPlugin.java | 2 +- .../security/action/apitokens/ApiToken.java | 27 +++++----- .../action/apitokens/ApiTokenAction.java | 9 ++-- .../action/apitokens/ApiTokenRepository.java | 11 +++- .../jwt/ExpiringBearerAuthToken.java | 9 ++++ .../security/authtoken/jwt/JwtVendor.java | 53 +++++++++++++++++++ .../identity/SecurityTokenManager.java | 22 ++++++++ .../securityconf/DynamicConfigModelV7.java | 2 +- .../securityconf/impl/v7/ConfigV7.java | 4 +- .../action/apitokens/ApiTokenActionTest.java | 1 + .../apitokens/ApiTokenIndexHandlerTest.java | 8 +-- .../identity/SecurityTokenManagerTest.java | 2 + 12 files changed, 123 insertions(+), 27 deletions(-) diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 381e181942..c30ef098cb 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -636,7 +636,7 @@ public List getRestHandlers( ) ); handlers.add(new CreateOnBehalfOfTokenAction(tokenManager)); - handlers.add(new ApiTokenAction(cs, threadPool, localClient)); + handlers.add(new ApiTokenAction(cs, localClient, tokenManager)); handlers.addAll( SecurityRestApiActions.getHandler( settings, diff --git a/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java b/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java index 1a00868729..e50806c5cc 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java @@ -16,6 +16,8 @@ import java.util.ArrayList; import java.util.List; +import com.fasterxml.jackson.annotation.JsonIgnore; + import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; @@ -31,25 +33,23 @@ public class ApiToken implements ToXContent { public static final String EXPIRATION_FIELD = "expiration"; private String name; - private final String jti; + private String jti; private final Instant creationTime; private List clusterPermissions; private List indexPermissions; private final long expiration; - public ApiToken(String name, String jti, List clusterPermissions, List indexPermissions, Long expiration) { + public ApiToken(String name, List clusterPermissions, List indexPermissions, Long expiration) { this.creationTime = Instant.now(); this.name = name; - this.jti = jti; this.clusterPermissions = clusterPermissions; this.indexPermissions = indexPermissions; this.expiration = expiration; } - public ApiToken(String name, String jti, List clusterPermissions, List indexPermissions) { + public ApiToken(String name, List clusterPermissions, List indexPermissions) { this.creationTime = Instant.now(); this.name = name; - this.jti = jti; this.clusterPermissions = clusterPermissions; this.indexPermissions = indexPermissions; this.expiration = Long.MAX_VALUE; @@ -57,14 +57,12 @@ public ApiToken(String name, String jti, List clusterPermissions, List clusterPermissions, List indexPermissions, Instant creationTime, Long expiration ) { this.name = name; - this.jti = jti; this.clusterPermissions = clusterPermissions; this.indexPermissions = indexPermissions; this.creationTime = creationTime; @@ -72,6 +70,10 @@ public ApiToken( } + public void setJti(String jti) { + this.jti = jti; + } + public static class IndexPermission implements ToXContent { private final List indexPatterns; private final List allowedActions; @@ -118,7 +120,6 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws */ public static ApiToken fromXContent(XContentParser parser) throws IOException { String name = null; - String jti = null; List clusterPermissions = new ArrayList<>(); List indexPermissions = new ArrayList<>(); Instant creationTime = null; @@ -135,9 +136,6 @@ public static ApiToken fromXContent(XContentParser parser) throws IOException { case NAME_FIELD: name = parser.text(); break; - case JTI_FIELD: - jti = parser.text(); - break; case CREATION_TIME_FIELD: creationTime = Instant.ofEpochMilli(parser.longValue()); break; @@ -163,7 +161,7 @@ public static ApiToken fromXContent(XContentParser parser) throws IOException { } } - return new ApiToken(name, jti, clusterPermissions, indexPermissions, creationTime, expiration); + return new ApiToken(name, clusterPermissions, indexPermissions, creationTime, expiration); } private static IndexPermission parseIndexPermission(XContentParser parser) throws IOException { @@ -202,6 +200,11 @@ public void setName(String name) { this.name = name; } + public Long getExpiration() { + return expiration; + } + + @JsonIgnore public String getJti() { return jti; } 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 2c7f7408da..f2f245b7ab 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java @@ -28,7 +28,7 @@ import org.opensearch.rest.BytesRestResponse; import org.opensearch.rest.RestHandler; import org.opensearch.rest.RestRequest; -import org.opensearch.threadpool.ThreadPool; +import org.opensearch.security.identity.SecurityTokenManager; import static org.opensearch.rest.RestRequest.Method.DELETE; import static org.opensearch.rest.RestRequest.Method.GET; @@ -55,8 +55,8 @@ public class ApiTokenAction extends BaseRestHandler { ) ); - public ApiTokenAction(ClusterService clusterService, ThreadPool threadPool, Client client) { - this.apiTokenRepository = new ApiTokenRepository(client, clusterService); + public ApiTokenAction(ClusterService clusterService, Client client, SecurityTokenManager securityTokenManager) { + this.apiTokenRepository = new ApiTokenRepository(client, clusterService, securityTokenManager); } @Override @@ -96,6 +96,9 @@ private RestChannelConsumer handleGet(RestRequest request, NodeClient client) { builder.startObject(); builder.field(NAME_FIELD, token.getName()); builder.field(CREATION_TIME_FIELD, token.getCreationTime().toEpochMilli()); + builder.field(EXPIRATION_FIELD, token.getExpiration()); + builder.field(CLUSTER_PERMISSIONS_FIELD, token.getClusterPermissions()); + builder.field(INDEX_PERMISSIONS_FIELD, token.getIndexPermissions()); builder.endObject(); } builder.endArray(); 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 454e8d736c..1b9f4446cf 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java @@ -17,12 +17,16 @@ import org.opensearch.client.Client; import org.opensearch.cluster.service.ClusterService; import org.opensearch.index.IndexNotFoundException; +import org.opensearch.security.authtoken.jwt.ExpiringBearerAuthToken; +import org.opensearch.security.identity.SecurityTokenManager; public class ApiTokenRepository { private final ApiTokenIndexHandler apiTokenIndexHandler; + private final SecurityTokenManager securityTokenManager; - public ApiTokenRepository(Client client, ClusterService clusterService) { + public ApiTokenRepository(Client client, ClusterService clusterService, SecurityTokenManager tokenManager) { apiTokenIndexHandler = new ApiTokenIndexHandler(client, clusterService); + securityTokenManager = tokenManager; } public String createApiToken( @@ -34,7 +38,10 @@ public String createApiToken( apiTokenIndexHandler.createApiTokenIndexIfAbsent(); // TODO: Implement logic of creating JTI to match against during authc/z // TODO: Add validation on whether user is creating a token with a subset of their permissions - return apiTokenIndexHandler.indexTokenMetadata(new ApiToken(name, "test-token", clusterPermissions, indexPermissions, expiration)); + ApiToken apiToken = new ApiToken(name, clusterPermissions, indexPermissions, expiration); + ExpiringBearerAuthToken token = securityTokenManager.issueApiToken(apiToken); + apiToken.setJti(token.getCompleteToken()); + return apiTokenIndexHandler.indexTokenMetadata(apiToken); } public void deleteApiToken(String name) throws ApiTokenException, IndexNotFoundException { diff --git a/src/main/java/org/opensearch/security/authtoken/jwt/ExpiringBearerAuthToken.java b/src/main/java/org/opensearch/security/authtoken/jwt/ExpiringBearerAuthToken.java index a0879cd4da..7b321f2001 100644 --- a/src/main/java/org/opensearch/security/authtoken/jwt/ExpiringBearerAuthToken.java +++ b/src/main/java/org/opensearch/security/authtoken/jwt/ExpiringBearerAuthToken.java @@ -10,6 +10,8 @@ */ package org.opensearch.security.authtoken.jwt; +import java.time.Duration; +import java.time.Instant; import java.util.Date; import org.opensearch.identity.tokens.BearerAuthToken; @@ -26,6 +28,13 @@ public ExpiringBearerAuthToken(final String serializedToken, final String subjec this.expiresInSeconds = expiresInSeconds; } + public ExpiringBearerAuthToken(final String serializedToken, final String subject, final Date expiry) { + super(serializedToken); + this.subject = subject; + this.expiry = expiry; + this.expiresInSeconds = Duration.between(Instant.now(), expiry.toInstant()).getSeconds(); + } + public String getSubject() { return subject; } diff --git a/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java b/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java index e21d9257ff..091f6a5fed 100644 --- a/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java +++ b/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java @@ -11,6 +11,8 @@ package org.opensearch.security.authtoken.jwt; +import java.security.AccessController; +import java.security.PrivilegedAction; import java.text.ParseException; import java.util.Base64; import java.util.Date; @@ -24,6 +26,7 @@ import org.opensearch.OpenSearchException; import org.opensearch.common.collect.Tuple; import org.opensearch.common.settings.Settings; +import org.opensearch.security.action.apitokens.ApiToken; import com.nimbusds.jose.JOSEException; import com.nimbusds.jose.JWSAlgorithm; @@ -148,4 +151,54 @@ public ExpiringBearerAuthToken createJwt( return new ExpiringBearerAuthToken(signedJwt.serialize(), subject, expiryTime, expirySeconds); } + + @SuppressWarnings("removal") + public ExpiringBearerAuthToken createJwt( + final String issuer, + final String subject, + final String audience, + final long expiration, + final List clusterPermissions, + final List indexPermissions + ) throws JOSEException, ParseException { + final long currentTimeMs = timeProvider.getAsLong(); + final Date now = new Date(currentTimeMs); + + final JWTClaimsSet.Builder claimsBuilder = new JWTClaimsSet.Builder(); + claimsBuilder.issuer(issuer); + claimsBuilder.issueTime(now); + claimsBuilder.subject(subject); + claimsBuilder.audience(audience); + claimsBuilder.notBeforeTime(now); + + final Date expiryTime = new Date(expiration); + claimsBuilder.expirationTime(expiryTime); + + if (clusterPermissions != null) { + final String listOfClusterPermissions = String.join(",", clusterPermissions); + claimsBuilder.claim("cp", encryptionDecryptionUtil.encrypt(listOfClusterPermissions)); + } + + if (indexPermissions != null) { + final String listOfIndexPermissions = String.join(", ", indexPermissions.toString()); + claimsBuilder.claim("ip", encryptionDecryptionUtil.encrypt(listOfIndexPermissions)); + } + + final JWSHeader header = new JWSHeader.Builder(JWSAlgorithm.parse(signingKey.getAlgorithm().getName())).build(); + + final SignedJWT signedJwt = AccessController.doPrivileged( + (PrivilegedAction) () -> new SignedJWT(header, claimsBuilder.build()) + ); + + // Sign the JWT so it can be serialized + signedJwt.sign(signer); + + if (logger.isDebugEnabled()) { + logger.debug( + "Created JWT: " + signedJwt.serialize() + "\n" + signedJwt.getHeader().toJSONObject() + "\n" + signedJwt.getJWTClaimsSet() + ); + } + + return new ExpiringBearerAuthToken(signedJwt.serialize(), subject, expiryTime); + } } diff --git a/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java b/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java index 69b757feb8..dcdc0267c9 100644 --- a/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java +++ b/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java @@ -27,6 +27,7 @@ import org.opensearch.identity.tokens.AuthToken; import org.opensearch.identity.tokens.OnBehalfOfClaims; import org.opensearch.identity.tokens.TokenManager; +import org.opensearch.security.action.apitokens.ApiToken; import org.opensearch.security.authtoken.jwt.ExpiringBearerAuthToken; import org.opensearch.security.authtoken.jwt.JwtVendor; import org.opensearch.security.securityconf.ConfigModel; @@ -139,6 +140,27 @@ public ExpiringBearerAuthToken issueOnBehalfOfToken(final Subject subject, final } } + public ExpiringBearerAuthToken issueApiToken(final ApiToken apiToken) { + final User user = threadPool.getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER); + if (user == null) { + throw new OpenSearchSecurityException("Unsupported user to generate Api Token"); + } + + try { + return apiTokenJwtVendor.createJwt( + cs.getClusterName().value(), + apiToken.getName(), + apiToken.getName(), + apiToken.getExpiration(), + apiToken.getClusterPermissions(), + apiToken.getIndexPermissions() + ); + } catch (final Exception ex) { + logger.error("Error creating OnBehalfOfToken for " + user.getName(), ex); + throw new OpenSearchSecurityException("Unable to generate OnBehalfOfToken"); + } + } + @Override public AuthToken issueServiceAccountToken(final String serviceId) { try { diff --git a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java index 43179b0eed..9c90e2341f 100644 --- a/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java +++ b/src/main/java/org/opensearch/security/securityconf/DynamicConfigModelV7.java @@ -237,7 +237,7 @@ public Settings getDynamicOnBehalfOfSettings() { @Override public Settings getDynamicApiTokenSettings() { return Settings.builder() - .put(Settings.builder().loadFromSource(config.dynamic.api_token_settings.configAsJson(), XContentType.JSON).build()) + .put(Settings.builder().loadFromSource(config.dynamic.api_tokens.configAsJson(), XContentType.JSON).build()) .build(); } diff --git a/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java b/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java index 044b6d4165..eef5d2c6b5 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java @@ -86,7 +86,7 @@ public static class Dynamic { public String transport_userrname_attribute; public boolean do_not_fail_on_forbidden_empty; public OnBehalfOfSettings on_behalf_of = new OnBehalfOfSettings(); - public ApiTokenSettings api_token_settings = new ApiTokenSettings(); + public ApiTokenSettings api_tokens = new ApiTokenSettings(); @Override public String toString() { @@ -103,7 +103,7 @@ public String toString() { + ", on_behalf_of=" + on_behalf_of + ", api_tokens=" - + api_token_settings + + api_tokens + "]"; } } 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 efcf577930..483fe7c9d7 100644 --- a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenActionTest.java +++ b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenActionTest.java @@ -28,6 +28,7 @@ public class ApiTokenActionTest { private final ApiTokenAction apiTokenAction = new ApiTokenAction(null, null, null); + @Test public void testCreateIndexPermission() { Map validPermission = new HashMap<>(); validPermission.put("index_pattern", "test-*"); diff --git a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java index c1bbc011f8..4e15335dfa 100644 --- a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java +++ b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java @@ -192,12 +192,12 @@ public void testIndexTokenStoresTokenPayload() { ); ApiToken token = new ApiToken( "test-token-description", - "test-jti", clusterPermissions, indexPermissions, Instant.now(), Long.MAX_VALUE ); + token.setJti("test-token-jti"); // Mock the index method with ActionListener @SuppressWarnings("unchecked") @@ -230,8 +230,8 @@ public void testIndexTokenStoresTokenPayload() { // verify contents String source = capturedRequest.source().utf8ToString(); assertThat(source, containsString("test-token-description")); - assertThat(source, containsString("test-jti")); assertThat(source, containsString("cluster:admin/something")); + assertThat(source, containsString("test-token-jti")); assertThat(source, containsString("test-index-*")); } @@ -245,7 +245,6 @@ public void testGetTokenPayloads() throws IOException { // First token ApiToken token1 = new ApiToken( "token1-description", - "jti1", Arrays.asList("cluster:admin/something"), Arrays.asList(new ApiToken.IndexPermission( Arrays.asList("index1-*"), @@ -258,7 +257,6 @@ public void testGetTokenPayloads() throws IOException { // Second token ApiToken token2 = new ApiToken( "token2-description", - "jti2", Arrays.asList("cluster:admin/other"), Arrays.asList(new ApiToken.IndexPermission( Arrays.asList("index2-*"), @@ -297,11 +295,9 @@ public void testGetTokenPayloads() throws IOException { assertThat(resultTokens.containsKey("token2-description"), is(true)); ApiToken resultToken1 = resultTokens.get("token1-description"); - assertThat(resultToken1.getJti(), equalTo("jti1")); assertThat(resultToken1.getClusterPermissions(), contains("cluster:admin/something")); ApiToken resultToken2 = resultTokens.get("token2-description"); - assertThat(resultToken2.getJti(), equalTo("jti2")); assertThat(resultToken2.getClusterPermissions(), contains("cluster:admin/other")); } diff --git a/src/test/java/org/opensearch/security/identity/SecurityTokenManagerTest.java b/src/test/java/org/opensearch/security/identity/SecurityTokenManagerTest.java index d686b145b2..effb4ec399 100644 --- a/src/test/java/org/opensearch/security/identity/SecurityTokenManagerTest.java +++ b/src/test/java/org/opensearch/security/identity/SecurityTokenManagerTest.java @@ -107,6 +107,7 @@ public void onDynamicConfigModelChanged_JwtVendorDisabled() { final Settings settings = Settings.builder().put("enabled", false).build(); final DynamicConfigModel dcm = mock(DynamicConfigModel.class); when(dcm.getDynamicOnBehalfOfSettings()).thenReturn(settings); + when(dcm.getDynamicApiTokenSettings()).thenReturn(settings); tokenManager.onDynamicConfigModelChanged(dcm); assertThat(tokenManager.issueOnBehalfOfTokenAllowed(), equalTo(false)); @@ -119,6 +120,7 @@ private DynamicConfigModel createMockJwtVendorInTokenManager() { final Settings settings = Settings.builder().put("enabled", true).build(); final DynamicConfigModel dcm = mock(DynamicConfigModel.class); when(dcm.getDynamicOnBehalfOfSettings()).thenReturn(settings); + when(dcm.getDynamicApiTokenSettings()).thenReturn(settings); doAnswer((invocation) -> jwtVendor).when(tokenManager).createJwtVendor(settings); tokenManager.onDynamicConfigModelChanged(dcm); return dcm; From 3a2e483473b0613b2976d3044ab93e1ce41bdceb Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Tue, 17 Dec 2024 18:10:50 -0500 Subject: [PATCH 24/34] Add logic to return to user the token and store an encrypted version in the index Signed-off-by: Derek Ho --- .../security/action/apitokens/ApiTokenAction.java | 2 +- .../security/action/apitokens/ApiTokenRepository.java | 9 ++++++--- .../org/opensearch/security/authtoken/jwt/JwtVendor.java | 7 +++++-- .../security/identity/SecurityTokenManager.java | 3 ++- 4 files changed, 14 insertions(+), 7 deletions(-) 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 f2f245b7ab..250e1bf981 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java @@ -132,7 +132,7 @@ private RestChannelConsumer handlePost(RestRequest request, NodeClient client) { ); builder.startObject(); - builder.field("token", token); + builder.field("Api Token: ", token); builder.endObject(); response = new BytesRestResponse(RestStatus.OK, builder); 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 1b9f4446cf..696ddf4231 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java @@ -16,6 +16,7 @@ import org.opensearch.client.Client; import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.collect.Tuple; import org.opensearch.index.IndexNotFoundException; import org.opensearch.security.authtoken.jwt.ExpiringBearerAuthToken; import org.opensearch.security.identity.SecurityTokenManager; @@ -39,9 +40,11 @@ public String createApiToken( // TODO: Implement logic of creating JTI to match against during authc/z // TODO: Add validation on whether user is creating a token with a subset of their permissions ApiToken apiToken = new ApiToken(name, clusterPermissions, indexPermissions, expiration); - ExpiringBearerAuthToken token = securityTokenManager.issueApiToken(apiToken); - apiToken.setJti(token.getCompleteToken()); - return apiTokenIndexHandler.indexTokenMetadata(apiToken); + Tuple token = securityTokenManager.issueApiToken(apiToken); + apiToken.setJti(token.v2()); + apiTokenIndexHandler.indexTokenMetadata(apiToken); + + return token.v1().getCompleteToken(); } public void deleteApiToken(String name) throws ApiTokenException, IndexNotFoundException { diff --git a/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java b/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java index 091f6a5fed..c38fcc853e 100644 --- a/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java +++ b/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java @@ -153,7 +153,7 @@ public ExpiringBearerAuthToken createJwt( } @SuppressWarnings("removal") - public ExpiringBearerAuthToken createJwt( + public Tuple createJwt( final String issuer, final String subject, final String audience, @@ -199,6 +199,9 @@ public ExpiringBearerAuthToken createJwt( ); } - return new ExpiringBearerAuthToken(signedJwt.serialize(), subject, expiryTime); + return Tuple.tuple( + new ExpiringBearerAuthToken(signedJwt.serialize(), subject, expiryTime), + encryptionDecryptionUtil.encrypt(signedJwt.serialize()) + ); } } diff --git a/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java b/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java index dcdc0267c9..5d32d7cfb5 100644 --- a/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java +++ b/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java @@ -20,6 +20,7 @@ import org.opensearch.OpenSearchSecurityException; import org.opensearch.cluster.service.ClusterService; +import org.opensearch.common.collect.Tuple; import org.opensearch.common.settings.Settings; import org.opensearch.core.common.transport.TransportAddress; import org.opensearch.identity.Subject; @@ -140,7 +141,7 @@ public ExpiringBearerAuthToken issueOnBehalfOfToken(final Subject subject, final } } - public ExpiringBearerAuthToken issueApiToken(final ApiToken apiToken) { + public Tuple issueApiToken(final ApiToken apiToken) { final User user = threadPool.getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER); if (user == null) { throw new OpenSearchSecurityException("Unsupported user to generate Api Token"); From 9abd1d03b2b49e95fd89928ed1e7e5ff989157d2 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Wed, 18 Dec 2024 18:32:02 -0500 Subject: [PATCH 25/34] Clean up issuing, encrypting, and decrypting logic, add tests for the feature Signed-off-by: Derek Ho --- .../security/action/apitokens/ApiToken.java | 37 ++++++ .../action/apitokens/ApiTokenRepository.java | 14 +- .../security/authtoken/jwt/JwtVendor.java | 24 +++- .../identity/SecurityTokenManager.java | 15 ++- .../apitokens/ApiTokenRepositoryTest.java | 121 ++++++++++++++++++ .../action/apitokens/ApiTokenTest.java | 83 ++++++++++++ .../security/authtoken/jwt/JwtVendorTest.java | 101 +++++++++++++++ .../identity/SecurityTokenManagerTest.java | 61 +++++++++ 8 files changed, 440 insertions(+), 16 deletions(-) create mode 100644 src/test/java/org/opensearch/security/action/apitokens/ApiTokenRepositoryTest.java create mode 100644 src/test/java/org/opensearch/security/action/apitokens/ApiTokenTest.java diff --git a/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java b/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java index e50806c5cc..ee47cde6db 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java @@ -22,6 +22,12 @@ import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; +import com.nimbusds.jose.shaded.gson.JsonArray; +import com.nimbusds.jose.shaded.gson.JsonElement; +import com.nimbusds.jose.shaded.gson.JsonObject; +import com.nimbusds.jose.shaded.gson.JsonParseException; +import com.nimbusds.jose.shaded.gson.JsonParser; + public class ApiToken implements ToXContent { public static final String NAME_FIELD = "name"; public static final String JTI_FIELD = "jti"; @@ -99,6 +105,37 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.endObject(); return builder; } + + @Override + public String toString() { + JsonObject json = new JsonObject(); + JsonArray patternsArray = new JsonArray(); + JsonArray actionsArray = new JsonArray(); + + for (String pattern : indexPatterns) { + patternsArray.add(pattern); + } + for (String action : allowedActions) { + actionsArray.add(action); + } + + json.add(INDEX_PATTERN_FIELD, patternsArray); + json.add(ALLOWED_ACTIONS_FIELD, actionsArray); + + return json.toString(); + } + + public static IndexPermission fromString(String str) { + try { + JsonObject json = JsonParser.parseString(str).getAsJsonObject(); + return new IndexPermission( + json.get(INDEX_PATTERN_FIELD).getAsJsonArray().asList().stream().map(JsonElement::getAsString).toList(), + json.get(ALLOWED_ACTIONS_FIELD).getAsJsonArray().asList().stream().map(JsonElement::getAsString).toList() + ); + } catch (JsonParseException e) { + throw new IllegalArgumentException("Invalid IndexPermission format", e); + } + } } /** 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 696ddf4231..b554c18e01 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java @@ -16,7 +16,6 @@ import org.opensearch.client.Client; import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.collect.Tuple; import org.opensearch.index.IndexNotFoundException; import org.opensearch.security.authtoken.jwt.ExpiringBearerAuthToken; import org.opensearch.security.identity.SecurityTokenManager; @@ -30,6 +29,11 @@ public ApiTokenRepository(Client client, ClusterService clusterService, Security securityTokenManager = tokenManager; } + public ApiTokenRepository(ApiTokenIndexHandler apiTokenIndexHandler1, SecurityTokenManager tokenManager) { + apiTokenIndexHandler = apiTokenIndexHandler1; + securityTokenManager = tokenManager; + } + public String createApiToken( String name, List clusterPermissions, @@ -37,14 +41,12 @@ public String createApiToken( Long expiration ) { apiTokenIndexHandler.createApiTokenIndexIfAbsent(); - // TODO: Implement logic of creating JTI to match against during authc/z // TODO: Add validation on whether user is creating a token with a subset of their permissions ApiToken apiToken = new ApiToken(name, clusterPermissions, indexPermissions, expiration); - Tuple token = securityTokenManager.issueApiToken(apiToken); - apiToken.setJti(token.v2()); + ExpiringBearerAuthToken token = securityTokenManager.issueApiToken(apiToken); + apiToken.setJti(securityTokenManager.encryptToken(token.getCompleteToken())); apiTokenIndexHandler.indexTokenMetadata(apiToken); - - return token.v1().getCompleteToken(); + return token.getCompleteToken(); } public void deleteApiToken(String name) throws ApiTokenException, IndexNotFoundException { diff --git a/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java b/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java index c38fcc853e..79015118b3 100644 --- a/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java +++ b/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java @@ -14,6 +14,7 @@ import java.security.AccessController; import java.security.PrivilegedAction; import java.text.ParseException; +import java.util.ArrayList; import java.util.Base64; import java.util.Date; import java.util.List; @@ -153,7 +154,7 @@ public ExpiringBearerAuthToken createJwt( } @SuppressWarnings("removal") - public Tuple createJwt( + public ExpiringBearerAuthToken createJwt( final String issuer, final String subject, final String audience, @@ -180,7 +181,11 @@ public Tuple createJwt( } if (indexPermissions != null) { - final String listOfIndexPermissions = String.join(", ", indexPermissions.toString()); + List permissionStrings = new ArrayList<>(); + for (ApiToken.IndexPermission permission : indexPermissions) { + permissionStrings.add(permission.toString()); + } + final String listOfIndexPermissions = String.join(",", permissionStrings); claimsBuilder.claim("ip", encryptionDecryptionUtil.encrypt(listOfIndexPermissions)); } @@ -199,9 +204,16 @@ public Tuple createJwt( ); } - return Tuple.tuple( - new ExpiringBearerAuthToken(signedJwt.serialize(), subject, expiryTime), - encryptionDecryptionUtil.encrypt(signedJwt.serialize()) - ); + return new ExpiringBearerAuthToken(signedJwt.serialize(), subject, expiryTime); + } + + /* Returns the encrypted string based on encryption settings */ + public String encryptString(final String input) { + return encryptionDecryptionUtil.encrypt(input); + } + + /* Returns the decrypted string based on encryption settings */ + public String decryptString(final String input) { + return encryptionDecryptionUtil.decrypt(input); } } diff --git a/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java b/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java index 5d32d7cfb5..9de2b70efe 100644 --- a/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java +++ b/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java @@ -20,7 +20,6 @@ import org.opensearch.OpenSearchSecurityException; import org.opensearch.cluster.service.ClusterService; -import org.opensearch.common.collect.Tuple; import org.opensearch.common.settings.Settings; import org.opensearch.core.common.transport.TransportAddress; import org.opensearch.identity.Subject; @@ -141,7 +140,7 @@ public ExpiringBearerAuthToken issueOnBehalfOfToken(final Subject subject, final } } - public Tuple issueApiToken(final ApiToken apiToken) { + public ExpiringBearerAuthToken issueApiToken(final ApiToken apiToken) { final User user = threadPool.getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER); if (user == null) { throw new OpenSearchSecurityException("Unsupported user to generate Api Token"); @@ -157,11 +156,19 @@ public Tuple issueApiToken(final ApiToken apiTo apiToken.getIndexPermissions() ); } catch (final Exception ex) { - logger.error("Error creating OnBehalfOfToken for " + user.getName(), ex); - throw new OpenSearchSecurityException("Unable to generate OnBehalfOfToken"); + logger.error("Error creating Api Token for " + user.getName(), ex); + throw new OpenSearchSecurityException("Unable to generate Api Token"); } } + public String encryptToken(final String token) { + return apiTokenJwtVendor.encryptString(token); + } + + public String decryptString(final String input) { + return apiTokenJwtVendor.decryptString(input); + } + @Override public AuthToken issueServiceAccountToken(final String serviceId) { try { diff --git a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenRepositoryTest.java b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenRepositoryTest.java new file mode 100644 index 0000000000..f685943a37 --- /dev/null +++ b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenRepositoryTest.java @@ -0,0 +1,121 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.action.apitokens; + +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.junit.Before; +import org.junit.Test; + +import org.opensearch.index.IndexNotFoundException; +import org.opensearch.security.authtoken.jwt.ExpiringBearerAuthToken; +import org.opensearch.security.identity.SecurityTokenManager; + +import org.mockito.Mock; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.argThat; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class ApiTokenRepositoryTest { + @Mock + private SecurityTokenManager securityTokenManager; + + @Mock + private ApiTokenIndexHandler apiTokenIndexHandler; + + private ApiTokenRepository repository; + + @Before + public void setUp() { + apiTokenIndexHandler = mock(ApiTokenIndexHandler.class); + securityTokenManager = mock(SecurityTokenManager.class); + repository = new ApiTokenRepository(apiTokenIndexHandler, securityTokenManager); + } + + @Test + public void testDeleteApiToken() throws ApiTokenException { + String tokenName = "test-token"; + + repository.deleteApiToken(tokenName); + + verify(apiTokenIndexHandler).deleteToken(tokenName); + } + + @Test + public void testGetApiTokens() throws IndexNotFoundException { + Map expectedTokens = new HashMap<>(); + expectedTokens.put("token1", new ApiToken("token1", Arrays.asList("perm1"), Arrays.asList(), Long.MAX_VALUE)); + when(apiTokenIndexHandler.getTokenMetadatas()).thenReturn(expectedTokens); + + Map result = repository.getApiTokens(); + + assertThat(result, equalTo(expectedTokens)); + verify(apiTokenIndexHandler).getTokenMetadatas(); + } + + @Test + public void testCreateApiToken() { + String tokenName = "test-token"; + List clusterPermissions = Arrays.asList("cluster:admin"); + List indexPermissions = Arrays.asList( + new ApiToken.IndexPermission(Arrays.asList("test-*"), Arrays.asList("read")) + ); + Long expiration = 3600L; + + String completeToken = "complete-token"; + String encryptedToken = "encrypted-token"; + ExpiringBearerAuthToken bearerToken = mock(ExpiringBearerAuthToken.class); + when(bearerToken.getCompleteToken()).thenReturn(completeToken); + when(securityTokenManager.issueApiToken(any())).thenReturn(bearerToken); + when(securityTokenManager.encryptToken(completeToken)).thenReturn(encryptedToken); + + String result = repository.createApiToken(tokenName, clusterPermissions, indexPermissions, expiration); + + verify(apiTokenIndexHandler).createApiTokenIndexIfAbsent(); + verify(securityTokenManager).issueApiToken(any(ApiToken.class)); + verify(securityTokenManager).encryptToken(completeToken); + verify(apiTokenIndexHandler).indexTokenMetadata( + argThat( + token -> token.getName().equals(tokenName) + && token.getJti().equals(encryptedToken) + && token.getClusterPermissions().equals(clusterPermissions) + && token.getIndexPermissions().equals(indexPermissions) + ) + ); + assertThat(result, equalTo(completeToken)); + } + + @Test(expected = IndexNotFoundException.class) + public void testGetApiTokensThrowsIndexNotFoundException() throws IndexNotFoundException { + when(apiTokenIndexHandler.getTokenMetadatas()).thenThrow(new IndexNotFoundException("test-index")); + + repository.getApiTokens(); + + } + + @Test(expected = ApiTokenException.class) + public void testDeleteApiTokenThrowsApiTokenException() throws ApiTokenException { + String tokenName = "test-token"; + doThrow(new ApiTokenException("Token not found")).when(apiTokenIndexHandler).deleteToken(tokenName); + + repository.deleteApiToken(tokenName); + } +} diff --git a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenTest.java b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenTest.java new file mode 100644 index 0000000000..822658e7d2 --- /dev/null +++ b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenTest.java @@ -0,0 +1,83 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.action.apitokens; + +import java.util.Arrays; +import java.util.List; + +import org.junit.Before; +import org.junit.Test; + +import org.opensearch.client.Client; +import org.opensearch.client.IndicesAdminClient; +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.mockito.Mock; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.mockito.Mockito.RETURNS_DEEP_STUBS; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class ApiTokenTest { + + @Mock + private Client client; + + @Mock + private IndicesAdminClient indicesAdminClient; + + @Mock + private ClusterService clusterService; + + @Mock + private Metadata metadata; + + private ApiTokenIndexHandler indexHandler; + + @Before + public void setup() { + + client = mock(Client.class, RETURNS_DEEP_STUBS); + indicesAdminClient = mock(IndicesAdminClient.class); + clusterService = mock(ClusterService.class, RETURNS_DEEP_STUBS); + metadata = mock(Metadata.class); + + when(client.admin().indices()).thenReturn(indicesAdminClient); + + when(clusterService.state().metadata()).thenReturn(metadata); + + ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + when(client.threadPool().getThreadContext()).thenReturn(threadContext); + + indexHandler = new ApiTokenIndexHandler(client, clusterService); + } + + @Test + public void testIndexPermissionToStringFromString() { + String indexPermissionString = "{\"index_pattern\":[\"index1\",\"index2\"],\"allowed_actions\":[\"action1\",\"action2\"]}"; + ApiToken.IndexPermission indexPermission = new ApiToken.IndexPermission( + Arrays.asList("index1", "index2"), + Arrays.asList("action1", "action2") + ); + assertThat(indexPermission.toString(), equalTo(indexPermissionString)); + + ApiToken.IndexPermission indexPermissionFromString = ApiToken.IndexPermission.fromString(indexPermissionString); + assertThat(indexPermissionFromString.getIndexPatterns(), equalTo(List.of("index1", "index2"))); + assertThat(indexPermissionFromString.getAllowedActions(), equalTo(List.of("action1", "action2"))); + } + +} diff --git a/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java b/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java index ca8b4ad14d..0e418165c1 100644 --- a/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java +++ b/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java @@ -30,11 +30,13 @@ import org.opensearch.OpenSearchException; import org.opensearch.common.collect.Tuple; import org.opensearch.common.settings.Settings; +import org.opensearch.security.action.apitokens.ApiToken; import org.opensearch.security.support.ConfigConstants; import com.nimbusds.jose.JWSSigner; import com.nimbusds.jose.jwk.JWK; import com.nimbusds.jwt.SignedJWT; +import joptsimple.internal.Strings; import org.mockito.ArgumentCaptor; import static org.hamcrest.MatcherAssert.assertThat; @@ -270,4 +272,103 @@ public void testCreateJwtLogsCorrectly() throws Exception { final String[] parts = logMessage.split("\\."); assertTrue(parts.length >= 3); } + + @Test + public void testCreateJwtForApiTokenLogsCorrectly() throws Exception { + final String issuer = "cluster_0"; + final String subject = "test-token"; + final String audience = "test-token"; + final List clusterPermissions = List.of("cluster:admin/*"); + ApiToken.IndexPermission indexPermission = new ApiToken.IndexPermission(List.of("*"), List.of("read")); + final List indexPermissions = List.of(indexPermission); + final String expectedClusterPermissions = "cluster:admin/*"; + final String expectedIndexPermisions = indexPermission.toString(); + + LongSupplier currentTime = () -> (long) 100; + String claimsEncryptionKey = "1234567890123456"; + Settings settings = Settings.builder() + .put("signing_key", signingKeyB64Encoded) + .put("encryption_key", claimsEncryptionKey) + // CS-SUPPRESS-SINGLE: RegexpSingleline get Extensions Settings + .put(ConfigConstants.EXTENSIONS_BWC_PLUGIN_MODE, true) + .build(); + final JwtVendor jwtVendor = new JwtVendor(settings, Optional.of(currentTime)); + final ExpiringBearerAuthToken authToken = jwtVendor.createJwt( + issuer, + subject, + audience, + Long.MAX_VALUE, + clusterPermissions, + indexPermissions + ); + + SignedJWT signedJWT = SignedJWT.parse(authToken.getCompleteToken()); + + assertThat(signedJWT.getJWTClaimsSet().getClaims().get("iss"), equalTo(issuer)); + assertThat(signedJWT.getJWTClaimsSet().getClaims().get("sub"), equalTo(subject)); + assertThat(signedJWT.getJWTClaimsSet().getClaims().get("aud").toString(), equalTo("[" + audience + "]")); + assertThat(signedJWT.getJWTClaimsSet().getClaims().get("iat"), is(notNullValue())); + // Allow for millisecond to second conversion flexibility + assertThat(((Date) signedJWT.getJWTClaimsSet().getClaims().get("exp")).getTime() / 1000, equalTo(Long.MAX_VALUE / 1000)); + + EncryptionDecryptionUtil encryptionUtil = new EncryptionDecryptionUtil(claimsEncryptionKey); + assertThat( + encryptionUtil.decrypt(signedJWT.getJWTClaimsSet().getClaims().get("cp").toString()), + equalTo(expectedClusterPermissions) + ); + assertThat(encryptionUtil.decrypt(signedJWT.getJWTClaimsSet().getClaims().get("ip").toString()), equalTo(expectedIndexPermisions)); + + // Index permission deserialization works as expected + assertThat( + ApiToken.IndexPermission.fromString(encryptionUtil.decrypt(signedJWT.getJWTClaimsSet().getClaims().get("ip").toString())) + .getIndexPatterns(), + equalTo(indexPermission.getIndexPatterns()) + ); + assertThat( + ApiToken.IndexPermission.fromString(encryptionUtil.decrypt(signedJWT.getJWTClaimsSet().getClaims().get("ip").toString())) + .getAllowedActions(), + equalTo(indexPermission.getAllowedActions()) + ); + } + + @Test + public void testEncryptJwtCorrectly() { + String claimsEncryptionKey = BaseEncoding.base64().encode("1234567890123456".getBytes(StandardCharsets.UTF_8)); + String token = + "eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyLCJkZXJlayI6ImlzIGF3ZXNvbWUifQ.aPp9mSaBRBUzMJ8V_MYWUs8UoGYnJDNVriu3B9MRJpPNZtOhnIfATE0Ghmms2bGRNw9rmyRn1VIDQRmxSOTu3w"; + String expectedEncryptedToken = + "k3JQNRXR57Y4V4W1LNkpEP7FTJZos7fySJDJDGuBQXe7pi9aiEIGJ7JqjezssGRZ1AZGD/QTPQ0jjaV+rEICxBO9oyfTYWIoDdnAg5LijqPAzaULp48hi+/dqXXAAhi1zIlCSjqTDoZMTyjFxq4aRlPLjjQFuVxR3gIDMNnAUnvmFu5xh5AiVeKa1dwGy5X34Ou2i9pnQzmEDJDnf6mh7w2ODkDThJGh8JUlsUlfZEq6NwVN1XNyOr2IhPd3IZYUMgN3vWHyfjs6uwQNyHKHHcxIj4P8bJXLIGxJy3+LV5Y="; + Settings settings = Settings.builder().put("signing_key", signingKeyB64Encoded).put("encryption_key", claimsEncryptionKey).build(); + LongSupplier currentTime = () -> (long) 100; + JwtVendor jwtVendor = new JwtVendor(settings, Optional.of(currentTime)); + assertThat(jwtVendor.encryptString(token), equalTo(expectedEncryptedToken)); + } + + @Test + public void testEncryptDecryptClusterIndexPermissionsCorrectly() { + String claimsEncryptionKey = BaseEncoding.base64().encode("1234567890123456".getBytes(StandardCharsets.UTF_8)); + String clusterPermissions = "cluster:admin/*,cluster:*"; + String encryptedClusterPermissions = "P+KGUkpANJHzHGKVSqJhIyHOKS+JCLOanxCOBWSgZNk="; + // "{\"index_pattern\":[\"*\"],\"allowed_actions\":[\"read\"]},{\"index_pattern\":[\".*\"],\"allowed_actions\":[\"write\"]}" + String indexPermissions = Strings.join( + List.of( + new ApiToken.IndexPermission(List.of("*"), List.of("read")).toString(), + new ApiToken.IndexPermission(List.of(".*"), List.of("write")).toString() + ), + "," + ); + String encryptedIndexPermissions = + "Y9ssHcl6spHC2/zy+L1P0y8e2+T+jGgXcP02DWGeTMk/3KiI4Ik0Df7oXMf9l/Ba0emk9LClnHsJi8iFwRh7ii1Pxb3CTHS/d+p7a3bA6rtJjgOjGlbjdWTdj4+87uBJynsR5CAlUMLeTrjbPe/nWw=="; + Settings settings = Settings.builder().put("signing_key", signingKeyB64Encoded).put("encryption_key", claimsEncryptionKey).build(); + LongSupplier currentTime = () -> (long) 100; + JwtVendor jwtVendor = new JwtVendor(settings, Optional.of(currentTime)); + + // encrypt decrypt cluster permissions + assertThat(jwtVendor.encryptString(clusterPermissions), equalTo(encryptedClusterPermissions)); + assertThat(jwtVendor.decryptString(encryptedClusterPermissions), equalTo(clusterPermissions)); + + // encrypt decrypt index permissions + assertThat(jwtVendor.encryptString(indexPermissions), equalTo(encryptedIndexPermissions)); + assertThat(jwtVendor.decryptString(encryptedIndexPermissions), equalTo(indexPermissions)); + } } diff --git a/src/test/java/org/opensearch/security/identity/SecurityTokenManagerTest.java b/src/test/java/org/opensearch/security/identity/SecurityTokenManagerTest.java index effb4ec399..b61843a64d 100644 --- a/src/test/java/org/opensearch/security/identity/SecurityTokenManagerTest.java +++ b/src/test/java/org/opensearch/security/identity/SecurityTokenManagerTest.java @@ -28,6 +28,7 @@ import org.opensearch.identity.Subject; import org.opensearch.identity.tokens.AuthToken; import org.opensearch.identity.tokens.OnBehalfOfClaims; +import org.opensearch.security.action.apitokens.ApiToken; import org.opensearch.security.authtoken.jwt.ExpiringBearerAuthToken; import org.opensearch.security.authtoken.jwt.JwtVendor; import org.opensearch.security.securityconf.ConfigModel; @@ -81,6 +82,7 @@ public void after() { verifyNoMoreInteractions(userService); } + @Test public void onConfigModelChanged_oboNotSupported() { final ConfigModel configModel = mock(ConfigModel.class); @@ -247,4 +249,63 @@ public void issueOnBehalfOfToken_success() throws Exception { verify(cs).getClusterName(); verify(threadPool).getThreadContext(); } + + @Test + public void issueApiToken_success() throws Exception { + doAnswer(invockation -> new ClusterName("cluster17")).when(cs).getClusterName(); + doAnswer(invocation -> true).when(tokenManager).issueApiTokenAllowed(); + final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + threadContext.putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, new User("Jon", List.of(), null)); + when(threadPool.getThreadContext()).thenReturn(threadContext); + final ConfigModel configModel = mock(ConfigModel.class); + tokenManager.onConfigModelChanged(configModel); + when(configModel.mapSecurityRoles(any(), any())).thenReturn(Set.of()); + + createMockJwtVendorInTokenManager(); + + final ExpiringBearerAuthToken authToken = mock(ExpiringBearerAuthToken.class); + when(jwtVendor.createJwt(anyString(), anyString(), anyString(), anyLong(), any(), any())).thenReturn(authToken); + final AuthToken returnedToken = tokenManager.issueApiToken(new ApiToken("elmo", List.of("*"), List.of())); + + assertThat(returnedToken, equalTo(authToken)); + + verify(cs).getClusterName(); + verify(threadPool).getThreadContext(); + } + + @Test + public void encryptCallsJwtEncrypt() throws Exception { + doAnswer(invockation -> new ClusterName("cluster17")).when(cs).getClusterName(); + doAnswer(invocation -> true).when(tokenManager).issueApiTokenAllowed(); + final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + threadContext.putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, new User("Jon", List.of(), null)); + when(threadPool.getThreadContext()).thenReturn(threadContext); + final ConfigModel configModel = mock(ConfigModel.class); + tokenManager.onConfigModelChanged(configModel); + when(configModel.mapSecurityRoles(any(), any())).thenReturn(Set.of()); + + createMockJwtVendorInTokenManager(); + + final ExpiringBearerAuthToken authToken = mock(ExpiringBearerAuthToken.class); + when(jwtVendor.createJwt(anyString(), anyString(), anyString(), anyLong(), any(), any())).thenReturn(authToken); + final AuthToken returnedToken = tokenManager.issueApiToken(new ApiToken("elmo", List.of("*"), List.of())); + + assertThat(returnedToken, equalTo(authToken)); + + verify(cs).getClusterName(); + verify(threadPool).getThreadContext(); + } + + @Test + public void testEncryptTokenCallsJwtEncrypt() throws Exception { + String tokenToEncrypt = "test-token"; + String encryptedToken = "encrypted-test-token"; + createMockJwtVendorInTokenManager(); + when(jwtVendor.encryptString(tokenToEncrypt)).thenReturn(encryptedToken); + + String result = tokenManager.encryptToken(tokenToEncrypt); + + assertThat(result, equalTo(encryptedToken)); + verify(jwtVendor).encryptString(tokenToEncrypt); + } } From 53169bc470c7e40584fc591fb3099001c791d095 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Wed, 18 Dec 2024 19:08:49 -0500 Subject: [PATCH 26/34] Removes unecessary mock calls Signed-off-by: Derek Ho --- .../security/identity/SecurityTokenManagerTest.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/test/java/org/opensearch/security/identity/SecurityTokenManagerTest.java b/src/test/java/org/opensearch/security/identity/SecurityTokenManagerTest.java index b61843a64d..3af1dddad3 100644 --- a/src/test/java/org/opensearch/security/identity/SecurityTokenManagerTest.java +++ b/src/test/java/org/opensearch/security/identity/SecurityTokenManagerTest.java @@ -253,13 +253,11 @@ public void issueOnBehalfOfToken_success() throws Exception { @Test public void issueApiToken_success() throws Exception { doAnswer(invockation -> new ClusterName("cluster17")).when(cs).getClusterName(); - doAnswer(invocation -> true).when(tokenManager).issueApiTokenAllowed(); final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); threadContext.putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, new User("Jon", List.of(), null)); when(threadPool.getThreadContext()).thenReturn(threadContext); final ConfigModel configModel = mock(ConfigModel.class); tokenManager.onConfigModelChanged(configModel); - when(configModel.mapSecurityRoles(any(), any())).thenReturn(Set.of()); createMockJwtVendorInTokenManager(); @@ -276,13 +274,11 @@ public void issueApiToken_success() throws Exception { @Test public void encryptCallsJwtEncrypt() throws Exception { doAnswer(invockation -> new ClusterName("cluster17")).when(cs).getClusterName(); - doAnswer(invocation -> true).when(tokenManager).issueApiTokenAllowed(); final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); threadContext.putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, new User("Jon", List.of(), null)); when(threadPool.getThreadContext()).thenReturn(threadContext); final ConfigModel configModel = mock(ConfigModel.class); tokenManager.onConfigModelChanged(configModel); - when(configModel.mapSecurityRoles(any(), any())).thenReturn(Set.of()); createMockJwtVendorInTokenManager(); From da8cf45bf24f011c61330a622427b7070178c5cc Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Wed, 18 Dec 2024 19:13:20 -0500 Subject: [PATCH 27/34] PR fixup Signed-off-by: Derek Ho --- .../opensearch/security/action/apitokens/ApiTokenAction.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 250e1bf981..cabbc45f5b 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java @@ -189,8 +189,8 @@ void validateRequestParameters(Map requestBody) { } if (requestBody.containsKey(EXPIRATION_FIELD)) { - Object permissions = requestBody.get(EXPIRATION_FIELD); - if (!(permissions instanceof Long)) { + Object expiration = requestBody.get(EXPIRATION_FIELD); + if (!(expiration instanceof Long)) { throw new IllegalArgumentException(EXPIRATION_FIELD + " must be an long"); } } From 2287742967575d2d920cc53cb65d2e4871cac6fe Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Thu, 19 Dec 2024 16:13:18 -0500 Subject: [PATCH 28/34] PR review Signed-off-by: Derek Ho --- .../security/action/apitokens/ApiToken.java | 43 ++++++------------- .../action/apitokens/ApiTokenAction.java | 2 +- .../action/apitokens/ApiTokenRepository.java | 5 +-- .../security/authtoken/jwt/JwtVendor.java | 4 +- .../security/http/ApiTokenAuthenticator.java | 4 ++ .../identity/SecurityTokenManager.java | 13 +++--- .../securityconf/impl/v7/ConfigV7.java | 2 +- .../apitokens/ApiTokenIndexHandlerTest.java | 4 +- .../apitokens/ApiTokenRepositoryTest.java | 6 +-- .../security/authtoken/jwt/JwtVendorTest.java | 9 +--- 10 files changed, 38 insertions(+), 54 deletions(-) create mode 100644 src/main/java/org/opensearch/security/http/ApiTokenAuthenticator.java diff --git a/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java b/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java index ee47cde6db..94bfb65958 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java @@ -38,46 +38,36 @@ public class ApiToken implements ToXContent { public static final String ALLOWED_ACTIONS_FIELD = "allowed_actions"; public static final String EXPIRATION_FIELD = "expiration"; - private String name; + private final String name; private String jti; private final Instant creationTime; - private List clusterPermissions; - private List indexPermissions; + private final List clusterPermissions; + private final List indexPermissions; private final long expiration; - public ApiToken(String name, List clusterPermissions, List indexPermissions, Long expiration) { + public ApiToken(String name, String jti, List clusterPermissions, List indexPermissions, Long expiration) { this.creationTime = Instant.now(); + this.jti = jti; this.name = name; this.clusterPermissions = clusterPermissions; this.indexPermissions = indexPermissions; this.expiration = expiration; } - public ApiToken(String name, List clusterPermissions, List indexPermissions) { - this.creationTime = Instant.now(); - this.name = name; - this.clusterPermissions = clusterPermissions; - this.indexPermissions = indexPermissions; - this.expiration = Long.MAX_VALUE; - } - public ApiToken( String name, + String jti, List clusterPermissions, List indexPermissions, Instant creationTime, Long expiration ) { this.name = name; + this.jti = jti; this.clusterPermissions = clusterPermissions; this.indexPermissions = indexPermissions; this.creationTime = creationTime; this.expiration = expiration; - - } - - public void setJti(String jti) { - this.jti = jti; } public static class IndexPermission implements ToXContent { @@ -106,6 +96,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return builder; } + // TODO: look into integrating with default object mapper @Override public String toString() { JsonObject json = new JsonObject(); @@ -157,6 +148,7 @@ public static IndexPermission fromString(String str) { */ public static ApiToken fromXContent(XContentParser parser) throws IOException { String name = null; + String jti = null; List clusterPermissions = new ArrayList<>(); List indexPermissions = new ArrayList<>(); Instant creationTime = null; @@ -173,6 +165,9 @@ public static ApiToken fromXContent(XContentParser parser) throws IOException { case NAME_FIELD: name = parser.text(); break; + case JTI_FIELD: + jti = parser.text(); + break; case CREATION_TIME_FIELD: creationTime = Instant.ofEpochMilli(parser.longValue()); break; @@ -198,7 +193,7 @@ public static ApiToken fromXContent(XContentParser parser) throws IOException { } } - return new ApiToken(name, clusterPermissions, indexPermissions, creationTime, expiration); + return new ApiToken(name, jti, clusterPermissions, indexPermissions, creationTime, expiration); } private static IndexPermission parseIndexPermission(XContentParser parser) throws IOException { @@ -233,10 +228,6 @@ public String getName() { return name; } - public void setName(String name) { - this.name = name; - } - public Long getExpiration() { return expiration; } @@ -254,10 +245,6 @@ public List getClusterPermissions() { return clusterPermissions; } - public void setClusterPermissions(List clusterPermissions) { - this.clusterPermissions = clusterPermissions; - } - @Override public XContentBuilder toXContent(XContentBuilder xContentBuilder, ToXContent.Params params) throws IOException { xContentBuilder.startObject(); @@ -273,8 +260,4 @@ public XContentBuilder toXContent(XContentBuilder xContentBuilder, ToXContent.Pa public List getIndexPermissions() { return indexPermissions; } - - public void setIndexPermissions(List indexPermissions) { - this.indexPermissions = indexPermissions; - } } 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 cabbc45f5b..ddec1dc82a 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java @@ -191,7 +191,7 @@ void validateRequestParameters(Map requestBody) { if (requestBody.containsKey(EXPIRATION_FIELD)) { Object expiration = requestBody.get(EXPIRATION_FIELD); if (!(expiration instanceof Long)) { - throw new IllegalArgumentException(EXPIRATION_FIELD + " must be an long"); + throw new IllegalArgumentException(EXPIRATION_FIELD + " must be a long"); } } 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 b554c18e01..8e40a81c07 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java @@ -42,9 +42,8 @@ public String createApiToken( ) { apiTokenIndexHandler.createApiTokenIndexIfAbsent(); // TODO: Add validation on whether user is creating a token with a subset of their permissions - ApiToken apiToken = new ApiToken(name, clusterPermissions, indexPermissions, expiration); - ExpiringBearerAuthToken token = securityTokenManager.issueApiToken(apiToken); - apiToken.setJti(securityTokenManager.encryptToken(token.getCompleteToken())); + ExpiringBearerAuthToken token = securityTokenManager.issueApiToken(name, expiration, clusterPermissions, indexPermissions); + ApiToken apiToken = new ApiToken(name, securityTokenManager.encryptToken(token.getCompleteToken()), clusterPermissions, indexPermissions, expiration); apiTokenIndexHandler.indexTokenMetadata(apiToken); return token.getCompleteToken(); } diff --git a/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java b/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java index 79015118b3..b97384dbe5 100644 --- a/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java +++ b/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java @@ -177,7 +177,7 @@ public ExpiringBearerAuthToken createJwt( if (clusterPermissions != null) { final String listOfClusterPermissions = String.join(",", clusterPermissions); - claimsBuilder.claim("cp", encryptionDecryptionUtil.encrypt(listOfClusterPermissions)); + claimsBuilder.claim("cp", encryptString(listOfClusterPermissions)); } if (indexPermissions != null) { @@ -186,7 +186,7 @@ public ExpiringBearerAuthToken createJwt( permissionStrings.add(permission.toString()); } final String listOfIndexPermissions = String.join(",", permissionStrings); - claimsBuilder.claim("ip", encryptionDecryptionUtil.encrypt(listOfIndexPermissions)); + claimsBuilder.claim("ip", encryptString(listOfIndexPermissions)); } final JWSHeader header = new JWSHeader.Builder(JWSAlgorithm.parse(signingKey.getAlgorithm().getName())).build(); diff --git a/src/main/java/org/opensearch/security/http/ApiTokenAuthenticator.java b/src/main/java/org/opensearch/security/http/ApiTokenAuthenticator.java new file mode 100644 index 0000000000..be69336d8e --- /dev/null +++ b/src/main/java/org/opensearch/security/http/ApiTokenAuthenticator.java @@ -0,0 +1,4 @@ +package org.opensearch.security.http; + +public class ApiTokenAuthenticator { +} diff --git a/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java b/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java index 9de2b70efe..ef0e1de150 100644 --- a/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java +++ b/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java @@ -11,6 +11,7 @@ package org.opensearch.security.identity; +import java.util.List; import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -140,7 +141,7 @@ public ExpiringBearerAuthToken issueOnBehalfOfToken(final Subject subject, final } } - public ExpiringBearerAuthToken issueApiToken(final ApiToken apiToken) { + public ExpiringBearerAuthToken issueApiToken(final String name, final Long expiration, final List clusterPermissions, final List indexPermissions) { final User user = threadPool.getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER); if (user == null) { throw new OpenSearchSecurityException("Unsupported user to generate Api Token"); @@ -149,11 +150,11 @@ public ExpiringBearerAuthToken issueApiToken(final ApiToken apiToken) { try { return apiTokenJwtVendor.createJwt( cs.getClusterName().value(), - apiToken.getName(), - apiToken.getName(), - apiToken.getExpiration(), - apiToken.getClusterPermissions(), - apiToken.getIndexPermissions() + name, + name, + expiration, + clusterPermissions, + indexPermissions ); } catch (final Exception ex) { logger.error("Error creating Api Token for " + user.getName(), ex); diff --git a/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java b/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java index eef5d2c6b5..6555c0838d 100644 --- a/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java +++ b/src/main/java/org/opensearch/security/securityconf/impl/v7/ConfigV7.java @@ -541,7 +541,7 @@ public void setEncryptionKey(String encryptionKey) { @Override public String toString() { - return "ApiTokens [ enabled=" + enabled + ", signing_key=" + signingKey + ", encryption_key=" + encryptionKey + "]"; + return "ApiTokenSettings [ enabled=" + enabled + ", signing_key=" + signingKey + ", encryption_key=" + encryptionKey + "]"; } } diff --git a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java index 4e15335dfa..7e03c14851 100644 --- a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java +++ b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandlerTest.java @@ -192,12 +192,12 @@ public void testIndexTokenStoresTokenPayload() { ); ApiToken token = new ApiToken( "test-token-description", + "test-token-jti", clusterPermissions, indexPermissions, Instant.now(), Long.MAX_VALUE ); - token.setJti("test-token-jti"); // Mock the index method with ActionListener @SuppressWarnings("unchecked") @@ -245,6 +245,7 @@ public void testGetTokenPayloads() throws IOException { // First token ApiToken token1 = new ApiToken( "token1-description", + "token1-jti", Arrays.asList("cluster:admin/something"), Arrays.asList(new ApiToken.IndexPermission( Arrays.asList("index1-*"), @@ -257,6 +258,7 @@ public void testGetTokenPayloads() throws IOException { // Second token ApiToken token2 = new ApiToken( "token2-description", + "token2-jti", Arrays.asList("cluster:admin/other"), Arrays.asList(new ApiToken.IndexPermission( Arrays.asList("index2-*"), diff --git a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenRepositoryTest.java b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenRepositoryTest.java index f685943a37..a17158a077 100644 --- a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenRepositoryTest.java +++ b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenRepositoryTest.java @@ -62,7 +62,7 @@ public void testDeleteApiToken() throws ApiTokenException { @Test public void testGetApiTokens() throws IndexNotFoundException { Map expectedTokens = new HashMap<>(); - expectedTokens.put("token1", new ApiToken("token1", Arrays.asList("perm1"), Arrays.asList(), Long.MAX_VALUE)); + expectedTokens.put("token1", new ApiToken("token1", "token1-jti", Arrays.asList("perm1"), Arrays.asList(), Long.MAX_VALUE)); when(apiTokenIndexHandler.getTokenMetadatas()).thenReturn(expectedTokens); Map result = repository.getApiTokens(); @@ -84,13 +84,13 @@ public void testCreateApiToken() { String encryptedToken = "encrypted-token"; ExpiringBearerAuthToken bearerToken = mock(ExpiringBearerAuthToken.class); when(bearerToken.getCompleteToken()).thenReturn(completeToken); - when(securityTokenManager.issueApiToken(any())).thenReturn(bearerToken); + when(securityTokenManager.issueApiToken(any(), any(), any(), any())).thenReturn(bearerToken); when(securityTokenManager.encryptToken(completeToken)).thenReturn(encryptedToken); String result = repository.createApiToken(tokenName, clusterPermissions, indexPermissions, expiration); verify(apiTokenIndexHandler).createApiTokenIndexIfAbsent(); - verify(securityTokenManager).issueApiToken(any(ApiToken.class)); + verify(securityTokenManager).issueApiToken(any(), any(), any(), any()); verify(securityTokenManager).encryptToken(completeToken); verify(apiTokenIndexHandler).indexTokenMetadata( argThat( diff --git a/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java b/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java index 0e418165c1..874c765cbf 100644 --- a/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java +++ b/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java @@ -274,7 +274,7 @@ public void testCreateJwtLogsCorrectly() throws Exception { } @Test - public void testCreateJwtForApiTokenLogsCorrectly() throws Exception { + public void testCreateJwtForApiTokenSuccess() throws Exception { final String issuer = "cluster_0"; final String subject = "test-token"; final String audience = "test-token"; @@ -286,12 +286,7 @@ public void testCreateJwtForApiTokenLogsCorrectly() throws Exception { LongSupplier currentTime = () -> (long) 100; String claimsEncryptionKey = "1234567890123456"; - Settings settings = Settings.builder() - .put("signing_key", signingKeyB64Encoded) - .put("encryption_key", claimsEncryptionKey) - // CS-SUPPRESS-SINGLE: RegexpSingleline get Extensions Settings - .put(ConfigConstants.EXTENSIONS_BWC_PLUGIN_MODE, true) - .build(); + Settings settings = Settings.builder().put("signing_key", signingKeyB64Encoded).put("encryption_key", claimsEncryptionKey).build(); final JwtVendor jwtVendor = new JwtVendor(settings, Optional.of(currentTime)); final ExpiringBearerAuthToken authToken = jwtVendor.createJwt( issuer, From 98301f80c8e597502a8ce3113005705f665c055b Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Thu, 19 Dec 2024 16:20:39 -0500 Subject: [PATCH 29/34] PR cleanup Signed-off-by: Derek Ho --- .../action/apitokens/ApiTokenAction.java | 4 +++- .../action/apitokens/ApiTokenRepository.java | 8 +++++++- .../security/identity/SecurityTokenManager.java | 16 +++++++--------- .../identity/SecurityTokenManagerTest.java | 5 ++--- 4 files changed, 19 insertions(+), 14 deletions(-) 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 ddec1dc82a..e2e373812f 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java @@ -12,9 +12,11 @@ package org.opensearch.security.action.apitokens; import java.io.IOException; +import java.time.Instant; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; import com.google.common.collect.ImmutableList; @@ -128,7 +130,7 @@ private RestChannelConsumer handlePost(RestRequest request, NodeClient client) { (String) requestBody.get(NAME_FIELD), clusterPermissions, indexPermissions, - (Long) requestBody.getOrDefault(EXPIRATION_FIELD, Long.MAX_VALUE) + (Long) requestBody.getOrDefault(EXPIRATION_FIELD, Instant.now().toEpochMilli() + TimeUnit.DAYS.toMillis(30)) ); builder.startObject(); 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 8e40a81c07..a53b49ff02 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java @@ -43,7 +43,13 @@ public String createApiToken( apiTokenIndexHandler.createApiTokenIndexIfAbsent(); // TODO: Add validation on whether user is creating a token with a subset of their permissions ExpiringBearerAuthToken token = securityTokenManager.issueApiToken(name, expiration, clusterPermissions, indexPermissions); - ApiToken apiToken = new ApiToken(name, securityTokenManager.encryptToken(token.getCompleteToken()), clusterPermissions, indexPermissions, expiration); + ApiToken apiToken = new ApiToken( + name, + securityTokenManager.encryptToken(token.getCompleteToken()), + clusterPermissions, + indexPermissions, + expiration + ); apiTokenIndexHandler.indexTokenMetadata(apiToken); return token.getCompleteToken(); } diff --git a/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java b/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java index ef0e1de150..dc323ea766 100644 --- a/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java +++ b/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java @@ -141,21 +141,19 @@ public ExpiringBearerAuthToken issueOnBehalfOfToken(final Subject subject, final } } - public ExpiringBearerAuthToken issueApiToken(final String name, final Long expiration, final List clusterPermissions, final List indexPermissions) { + public ExpiringBearerAuthToken issueApiToken( + final String name, + final Long expiration, + final List clusterPermissions, + final List indexPermissions + ) { final User user = threadPool.getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER); if (user == null) { throw new OpenSearchSecurityException("Unsupported user to generate Api Token"); } try { - return apiTokenJwtVendor.createJwt( - cs.getClusterName().value(), - name, - name, - expiration, - clusterPermissions, - indexPermissions - ); + return apiTokenJwtVendor.createJwt(cs.getClusterName().value(), name, name, expiration, clusterPermissions, indexPermissions); } catch (final Exception ex) { logger.error("Error creating Api Token for " + user.getName(), ex); throw new OpenSearchSecurityException("Unable to generate Api Token"); diff --git a/src/test/java/org/opensearch/security/identity/SecurityTokenManagerTest.java b/src/test/java/org/opensearch/security/identity/SecurityTokenManagerTest.java index 3af1dddad3..7ecbb6da34 100644 --- a/src/test/java/org/opensearch/security/identity/SecurityTokenManagerTest.java +++ b/src/test/java/org/opensearch/security/identity/SecurityTokenManagerTest.java @@ -28,7 +28,6 @@ import org.opensearch.identity.Subject; import org.opensearch.identity.tokens.AuthToken; import org.opensearch.identity.tokens.OnBehalfOfClaims; -import org.opensearch.security.action.apitokens.ApiToken; import org.opensearch.security.authtoken.jwt.ExpiringBearerAuthToken; import org.opensearch.security.authtoken.jwt.JwtVendor; import org.opensearch.security.securityconf.ConfigModel; @@ -263,7 +262,7 @@ public void issueApiToken_success() throws Exception { final ExpiringBearerAuthToken authToken = mock(ExpiringBearerAuthToken.class); when(jwtVendor.createJwt(anyString(), anyString(), anyString(), anyLong(), any(), any())).thenReturn(authToken); - final AuthToken returnedToken = tokenManager.issueApiToken(new ApiToken("elmo", List.of("*"), List.of())); + final AuthToken returnedToken = tokenManager.issueApiToken("elmo", Long.MAX_VALUE, List.of("*"), List.of()); assertThat(returnedToken, equalTo(authToken)); @@ -284,7 +283,7 @@ public void encryptCallsJwtEncrypt() throws Exception { final ExpiringBearerAuthToken authToken = mock(ExpiringBearerAuthToken.class); when(jwtVendor.createJwt(anyString(), anyString(), anyString(), anyLong(), any(), any())).thenReturn(authToken); - final AuthToken returnedToken = tokenManager.issueApiToken(new ApiToken("elmo", List.of("*"), List.of())); + final AuthToken returnedToken = tokenManager.issueApiToken("elmo", Long.MAX_VALUE, List.of("*"), List.of()); assertThat(returnedToken, equalTo(authToken)); From b84d2cfb34369a11717d102fb04f27977b8d3510 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Thu, 19 Dec 2024 16:34:18 -0500 Subject: [PATCH 30/34] Add test for signing key too short Signed-off-by: Derek Ho --- .../security/authtoken/jwt/JwtVendorTest.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java b/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java index 874c765cbf..758e8fdfd2 100644 --- a/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java +++ b/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java @@ -366,4 +366,15 @@ public void testEncryptDecryptClusterIndexPermissionsCorrectly() { assertThat(jwtVendor.encryptString(indexPermissions), equalTo(encryptedIndexPermissions)); assertThat(jwtVendor.decryptString(encryptedIndexPermissions), equalTo(indexPermissions)); } + + @Test + public void testKeyTooShortThrowsException() { + String claimsEncryptionKey = RandomStringUtils.randomAlphanumeric(16); + String tooShortKey = BaseEncoding.base64().encode("short_key".getBytes()); + Settings settings = Settings.builder().put("signing_key", tooShortKey).put("encryption_key", claimsEncryptionKey).build(); + final Throwable exception = assertThrows(OpenSearchException.class, () -> { new JwtVendor(settings, Optional.empty()); }); + + assertThat(exception.getMessage(), containsString("The secret length must be at least 256 bits")); + } + } From af1de45afdf8e1a3fc26b33bc7cf42ee1fbd801f Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Thu, 19 Dec 2024 16:38:38 -0500 Subject: [PATCH 31/34] Remove file from PR Signed-off-by: Derek Ho --- .../org/opensearch/security/http/ApiTokenAuthenticator.java | 4 ---- 1 file changed, 4 deletions(-) delete mode 100644 src/main/java/org/opensearch/security/http/ApiTokenAuthenticator.java diff --git a/src/main/java/org/opensearch/security/http/ApiTokenAuthenticator.java b/src/main/java/org/opensearch/security/http/ApiTokenAuthenticator.java deleted file mode 100644 index be69336d8e..0000000000 --- a/src/main/java/org/opensearch/security/http/ApiTokenAuthenticator.java +++ /dev/null @@ -1,4 +0,0 @@ -package org.opensearch.security.http; - -public class ApiTokenAuthenticator { -} From 08e08900b914f7df626692108b8099a745343bee Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Fri, 20 Dec 2024 13:41:51 -0500 Subject: [PATCH 32/34] PR review Signed-off-by: Derek Ho --- .../security/action/apitokens/ApiToken.java | 64 ++++++++----------- .../action/apitokens/ApiTokenRepository.java | 13 +++- .../security/authtoken/jwt/JwtVendor.java | 7 +- .../apitokens/ApiTokenRepositoryTest.java | 2 +- .../action/apitokens/ApiTokenTest.java | 19 +++++- .../security/authtoken/jwt/JwtVendorTest.java | 23 ++++--- 6 files changed, 73 insertions(+), 55 deletions(-) diff --git a/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java b/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java index 94bfb65958..d8be267da3 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiToken.java @@ -22,12 +22,6 @@ import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; -import com.nimbusds.jose.shaded.gson.JsonArray; -import com.nimbusds.jose.shaded.gson.JsonElement; -import com.nimbusds.jose.shaded.gson.JsonObject; -import com.nimbusds.jose.shaded.gson.JsonParseException; -import com.nimbusds.jose.shaded.gson.JsonParser; - public class ApiToken implements ToXContent { public static final String NAME_FIELD = "name"; public static final String JTI_FIELD = "jti"; @@ -39,7 +33,7 @@ public class ApiToken implements ToXContent { public static final String EXPIRATION_FIELD = "expiration"; private final String name; - private String jti; + private final String jti; private final Instant creationTime; private final List clusterPermissions; private final List indexPermissions; @@ -96,37 +90,35 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws return builder; } - // TODO: look into integrating with default object mapper - @Override - public String toString() { - JsonObject json = new JsonObject(); - JsonArray patternsArray = new JsonArray(); - JsonArray actionsArray = new JsonArray(); - - for (String pattern : indexPatterns) { - patternsArray.add(pattern); - } - for (String action : allowedActions) { - actionsArray.add(action); + public static IndexPermission fromXContent(XContentParser parser) throws IOException { + List indexPatterns = new ArrayList<>(); + List allowedActions = new ArrayList<>(); + + XContentParser.Token token; + String currentFieldName = null; + + while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { + if (token == XContentParser.Token.FIELD_NAME) { + currentFieldName = parser.currentName(); + } else if (token == XContentParser.Token.START_ARRAY) { + switch (currentFieldName) { + case INDEX_PATTERN_FIELD: + while (parser.nextToken() != XContentParser.Token.END_ARRAY) { + indexPatterns.add(parser.text()); + } + break; + case ALLOWED_ACTIONS_FIELD: + while (parser.nextToken() != XContentParser.Token.END_ARRAY) { + allowedActions.add(parser.text()); + } + break; + } + } } - json.add(INDEX_PATTERN_FIELD, patternsArray); - json.add(ALLOWED_ACTIONS_FIELD, actionsArray); - - return json.toString(); + return new IndexPermission(indexPatterns, allowedActions); } - public static IndexPermission fromString(String str) { - try { - JsonObject json = JsonParser.parseString(str).getAsJsonObject(); - return new IndexPermission( - json.get(INDEX_PATTERN_FIELD).getAsJsonArray().asList().stream().map(JsonElement::getAsString).toList(), - json.get(ALLOWED_ACTIONS_FIELD).getAsJsonArray().asList().stream().map(JsonElement::getAsString).toList() - ); - } catch (JsonParseException e) { - throw new IllegalArgumentException("Invalid IndexPermission format", e); - } - } } /** @@ -152,7 +144,7 @@ public static ApiToken fromXContent(XContentParser parser) throws IOException { List clusterPermissions = new ArrayList<>(); List indexPermissions = new ArrayList<>(); Instant creationTime = null; - long expiration = Long.MAX_VALUE; + long expiration = 0; XContentParser.Token token; String currentFieldName = null; @@ -246,7 +238,7 @@ public List getClusterPermissions() { } @Override - public XContentBuilder toXContent(XContentBuilder xContentBuilder, ToXContent.Params params) throws IOException { + public XContentBuilder toXContent(XContentBuilder xContentBuilder, Params params) throws IOException { xContentBuilder.startObject(); xContentBuilder.field(NAME_FIELD, name); xContentBuilder.field(JTI_FIELD, jti); 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 a53b49ff02..ce81aceb4b 100644 --- a/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java +++ b/src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java @@ -14,6 +14,8 @@ import java.util.List; import java.util.Map; +import com.google.common.annotations.VisibleForTesting; + import org.opensearch.client.Client; import org.opensearch.cluster.service.ClusterService; import org.opensearch.index.IndexNotFoundException; @@ -29,9 +31,14 @@ public ApiTokenRepository(Client client, ClusterService clusterService, Security securityTokenManager = tokenManager; } - public ApiTokenRepository(ApiTokenIndexHandler apiTokenIndexHandler1, SecurityTokenManager tokenManager) { - apiTokenIndexHandler = apiTokenIndexHandler1; - securityTokenManager = tokenManager; + private ApiTokenRepository(ApiTokenIndexHandler apiTokenIndexHandler, SecurityTokenManager securityTokenManager) { + this.apiTokenIndexHandler = apiTokenIndexHandler; + this.securityTokenManager = securityTokenManager; + } + + @VisibleForTesting + static ApiTokenRepository forTest(ApiTokenIndexHandler apiTokenIndexHandler, SecurityTokenManager securityTokenManager) { + return new ApiTokenRepository(apiTokenIndexHandler, securityTokenManager); } public String createApiToken( diff --git a/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java b/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java index b97384dbe5..75ce45912a 100644 --- a/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java +++ b/src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java @@ -11,6 +11,7 @@ package org.opensearch.security.authtoken.jwt; +import java.io.IOException; import java.security.AccessController; import java.security.PrivilegedAction; import java.text.ParseException; @@ -27,6 +28,8 @@ import org.opensearch.OpenSearchException; import org.opensearch.common.collect.Tuple; import org.opensearch.common.settings.Settings; +import org.opensearch.common.xcontent.XContentFactory; +import org.opensearch.core.xcontent.ToXContent; import org.opensearch.security.action.apitokens.ApiToken; import com.nimbusds.jose.JOSEException; @@ -161,7 +164,7 @@ public ExpiringBearerAuthToken createJwt( final long expiration, final List clusterPermissions, final List indexPermissions - ) throws JOSEException, ParseException { + ) throws JOSEException, ParseException, IOException { final long currentTimeMs = timeProvider.getAsLong(); final Date now = new Date(currentTimeMs); @@ -183,7 +186,7 @@ public ExpiringBearerAuthToken createJwt( if (indexPermissions != null) { List permissionStrings = new ArrayList<>(); for (ApiToken.IndexPermission permission : indexPermissions) { - permissionStrings.add(permission.toString()); + permissionStrings.add(permission.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS).toString()); } final String listOfIndexPermissions = String.join(",", permissionStrings); claimsBuilder.claim("ip", encryptString(listOfIndexPermissions)); diff --git a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenRepositoryTest.java b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenRepositoryTest.java index a17158a077..03a2e2c30e 100644 --- a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenRepositoryTest.java +++ b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenRepositoryTest.java @@ -47,7 +47,7 @@ public class ApiTokenRepositoryTest { public void setUp() { apiTokenIndexHandler = mock(ApiTokenIndexHandler.class); securityTokenManager = mock(SecurityTokenManager.class); - repository = new ApiTokenRepository(apiTokenIndexHandler, securityTokenManager); + repository = ApiTokenRepository.forTest(apiTokenIndexHandler, securityTokenManager); } @Test diff --git a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenTest.java b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenTest.java index 822658e7d2..4951507359 100644 --- a/src/test/java/org/opensearch/security/action/apitokens/ApiTokenTest.java +++ b/src/test/java/org/opensearch/security/action/apitokens/ApiTokenTest.java @@ -11,6 +11,7 @@ package org.opensearch.security.action.apitokens; +import java.io.IOException; import java.util.Arrays; import java.util.List; @@ -23,6 +24,12 @@ import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.common.xcontent.XContentFactory; +import org.opensearch.common.xcontent.XContentType; +import org.opensearch.core.xcontent.DeprecationHandler; +import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.XContentParser; import org.mockito.Mock; @@ -67,15 +74,21 @@ public void setup() { } @Test - public void testIndexPermissionToStringFromString() { + public void testIndexPermissionToStringFromString() throws IOException { String indexPermissionString = "{\"index_pattern\":[\"index1\",\"index2\"],\"allowed_actions\":[\"action1\",\"action2\"]}"; ApiToken.IndexPermission indexPermission = new ApiToken.IndexPermission( Arrays.asList("index1", "index2"), Arrays.asList("action1", "action2") ); - assertThat(indexPermission.toString(), equalTo(indexPermissionString)); + assertThat( + indexPermission.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS).toString(), + equalTo(indexPermissionString) + ); + + XContentParser parser = XContentType.JSON.xContent() + .createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, indexPermissionString); - ApiToken.IndexPermission indexPermissionFromString = ApiToken.IndexPermission.fromString(indexPermissionString); + ApiToken.IndexPermission indexPermissionFromString = ApiToken.IndexPermission.fromXContent(parser); assertThat(indexPermissionFromString.getIndexPatterns(), equalTo(List.of("index1", "index2"))); assertThat(indexPermissionFromString.getAllowedActions(), equalTo(List.of("action1", "action2"))); } diff --git a/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java b/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java index 758e8fdfd2..e37b59b98b 100644 --- a/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java +++ b/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java @@ -30,6 +30,10 @@ import org.opensearch.OpenSearchException; import org.opensearch.common.collect.Tuple; import org.opensearch.common.settings.Settings; +import org.opensearch.common.xcontent.XContentType; +import org.opensearch.core.xcontent.DeprecationHandler; +import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.core.xcontent.XContentParser; import org.opensearch.security.action.apitokens.ApiToken; import org.opensearch.security.support.ConfigConstants; @@ -313,17 +317,16 @@ public void testCreateJwtForApiTokenSuccess() throws Exception { ); assertThat(encryptionUtil.decrypt(signedJWT.getJWTClaimsSet().getClaims().get("ip").toString()), equalTo(expectedIndexPermisions)); + XContentParser parser = XContentType.JSON.xContent() + .createParser( + NamedXContentRegistry.EMPTY, + DeprecationHandler.THROW_UNSUPPORTED_OPERATION, + encryptionUtil.decrypt(signedJWT.getJWTClaimsSet().getClaims().get("ip").toString()) + ); + // Index permission deserialization works as expected - assertThat( - ApiToken.IndexPermission.fromString(encryptionUtil.decrypt(signedJWT.getJWTClaimsSet().getClaims().get("ip").toString())) - .getIndexPatterns(), - equalTo(indexPermission.getIndexPatterns()) - ); - assertThat( - ApiToken.IndexPermission.fromString(encryptionUtil.decrypt(signedJWT.getJWTClaimsSet().getClaims().get("ip").toString())) - .getAllowedActions(), - equalTo(indexPermission.getAllowedActions()) - ); + assertThat(ApiToken.IndexPermission.fromXContent(parser).getIndexPatterns(), equalTo(indexPermission.getIndexPatterns())); + assertThat(ApiToken.IndexPermission.fromXContent(parser).getAllowedActions(), equalTo(indexPermission.getAllowedActions())); } @Test From 09018dc5dce3143acb896088887550956874175c Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Fri, 20 Dec 2024 14:47:36 -0500 Subject: [PATCH 33/34] Remove null check and fix test Signed-off-by: Derek Ho --- .../identity/SecurityTokenManager.java | 9 +++---- .../security/authtoken/jwt/JwtVendorTest.java | 25 +++++++++++++------ 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java b/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java index dc323ea766..03ed414a63 100644 --- a/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java +++ b/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java @@ -11,10 +11,10 @@ package org.opensearch.security.identity; +import java.util.ArrayList; import java.util.List; import java.util.Optional; import java.util.Set; -import java.util.stream.Collectors; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -118,9 +118,6 @@ public ExpiringBearerAuthToken issueOnBehalfOfToken(final Subject subject, final } final User user = threadPool.getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER); - if (user == null) { - throw new OpenSearchSecurityException("Unsupported user to generate OnBehalfOfToken"); - } final TransportAddress callerAddress = null; /* OBO tokens must not roles based on location from network address */ final Set mappedRoles = configModel.mapSecurityRoles(user, callerAddress); @@ -131,8 +128,8 @@ public ExpiringBearerAuthToken issueOnBehalfOfToken(final Subject subject, final user.getName(), claims.getAudience(), claims.getExpiration(), - mappedRoles.stream().collect(Collectors.toList()), - user.getRoles().stream().collect(Collectors.toList()), + new ArrayList<>(mappedRoles), + new ArrayList<>(user.getRoles()), false ); } catch (final Exception ex) { diff --git a/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java b/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java index e37b59b98b..48aae6f9b8 100644 --- a/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java +++ b/src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java @@ -11,6 +11,7 @@ package org.opensearch.security.authtoken.jwt; +import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.Date; import java.util.List; @@ -30,9 +31,11 @@ import org.opensearch.OpenSearchException; import org.opensearch.common.collect.Tuple; import org.opensearch.common.settings.Settings; +import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.common.xcontent.XContentType; import org.opensearch.core.xcontent.DeprecationHandler; import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentParser; import org.opensearch.security.action.apitokens.ApiToken; import org.opensearch.security.support.ConfigConstants; @@ -286,7 +289,8 @@ public void testCreateJwtForApiTokenSuccess() throws Exception { ApiToken.IndexPermission indexPermission = new ApiToken.IndexPermission(List.of("*"), List.of("read")); final List indexPermissions = List.of(indexPermission); final String expectedClusterPermissions = "cluster:admin/*"; - final String expectedIndexPermisions = indexPermission.toString(); + final String expectedIndexPermissions = indexPermission.toXContent(XContentFactory.jsonBuilder(), ToXContent.EMPTY_PARAMS) + .toString(); LongSupplier currentTime = () -> (long) 100; String claimsEncryptionKey = "1234567890123456"; @@ -315,7 +319,7 @@ public void testCreateJwtForApiTokenSuccess() throws Exception { encryptionUtil.decrypt(signedJWT.getJWTClaimsSet().getClaims().get("cp").toString()), equalTo(expectedClusterPermissions) ); - assertThat(encryptionUtil.decrypt(signedJWT.getJWTClaimsSet().getClaims().get("ip").toString()), equalTo(expectedIndexPermisions)); + assertThat(encryptionUtil.decrypt(signedJWT.getJWTClaimsSet().getClaims().get("ip").toString()), equalTo(expectedIndexPermissions)); XContentParser parser = XContentType.JSON.xContent() .createParser( @@ -323,10 +327,11 @@ public void testCreateJwtForApiTokenSuccess() throws Exception { DeprecationHandler.THROW_UNSUPPORTED_OPERATION, encryptionUtil.decrypt(signedJWT.getJWTClaimsSet().getClaims().get("ip").toString()) ); + ApiToken.IndexPermission indexPermission1 = ApiToken.IndexPermission.fromXContent(parser); // Index permission deserialization works as expected - assertThat(ApiToken.IndexPermission.fromXContent(parser).getIndexPatterns(), equalTo(indexPermission.getIndexPatterns())); - assertThat(ApiToken.IndexPermission.fromXContent(parser).getAllowedActions(), equalTo(indexPermission.getAllowedActions())); + assertThat(indexPermission1.getIndexPatterns(), equalTo(indexPermission.getIndexPatterns())); + assertThat(indexPermission1.getAllowedActions(), equalTo(indexPermission.getAllowedActions())); } @Test @@ -343,15 +348,21 @@ public void testEncryptJwtCorrectly() { } @Test - public void testEncryptDecryptClusterIndexPermissionsCorrectly() { + public void testEncryptDecryptClusterIndexPermissionsCorrectly() throws IOException { String claimsEncryptionKey = BaseEncoding.base64().encode("1234567890123456".getBytes(StandardCharsets.UTF_8)); String clusterPermissions = "cluster:admin/*,cluster:*"; String encryptedClusterPermissions = "P+KGUkpANJHzHGKVSqJhIyHOKS+JCLOanxCOBWSgZNk="; // "{\"index_pattern\":[\"*\"],\"allowed_actions\":[\"read\"]},{\"index_pattern\":[\".*\"],\"allowed_actions\":[\"write\"]}" String indexPermissions = Strings.join( List.of( - new ApiToken.IndexPermission(List.of("*"), List.of("read")).toString(), - new ApiToken.IndexPermission(List.of(".*"), List.of("write")).toString() + new ApiToken.IndexPermission(List.of("*"), List.of("read")).toXContent( + XContentFactory.jsonBuilder(), + ToXContent.EMPTY_PARAMS + ).toString(), + new ApiToken.IndexPermission(List.of(".*"), List.of("write")).toXContent( + XContentFactory.jsonBuilder(), + ToXContent.EMPTY_PARAMS + ).toString() ), "," ); From 7ab4a2a8055f28f2e6ef3ac3d2c9f7d6df9b2260 Mon Sep 17 00:00:00 2001 From: Derek Ho Date: Fri, 20 Dec 2024 15:09:37 -0500 Subject: [PATCH 34/34] Add it back to OBO and take away from api token Signed-off-by: Derek Ho --- .../opensearch/security/identity/SecurityTokenManager.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java b/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java index 03ed414a63..ca5a17b6f7 100644 --- a/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java +++ b/src/main/java/org/opensearch/security/identity/SecurityTokenManager.java @@ -118,6 +118,9 @@ public ExpiringBearerAuthToken issueOnBehalfOfToken(final Subject subject, final } final User user = threadPool.getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER); + if (user == null) { + throw new OpenSearchSecurityException("Unsupported user to generate OnBehalfOfToken"); + } final TransportAddress callerAddress = null; /* OBO tokens must not roles based on location from network address */ final Set mappedRoles = configModel.mapSecurityRoles(user, callerAddress); @@ -145,9 +148,6 @@ public ExpiringBearerAuthToken issueApiToken( final List indexPermissions ) { final User user = threadPool.getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER); - if (user == null) { - throw new OpenSearchSecurityException("Unsupported user to generate Api Token"); - } try { return apiTokenJwtVendor.createJwt(cs.getClusterName().value(), name, name, expiration, clusterPermissions, indexPermissions);