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() );