Skip to content

Commit

Permalink
Cleanup according to PR feedback
Browse files Browse the repository at this point in the history 
Signed-off-by: Derek Ho <[email protected]>
  • Loading branch information
@derek-ho
derek-ho committed Dec 11, 2024
1 parent 1468c9c commit 725c3f8
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 127 deletions.
91 changes: 46 additions & 45 deletions src/main/java/org/opensearch/security/action/apitokens/ApiToken.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> clusterPermissions;
private List<RoleV7.Index> indexPermissions;
private List<IndexPermission> indexPermissions;

public ApiToken(String description, String jti, List<String> clusterPermissions, List<RoleV7.Index> indexPermissions) {
public ApiToken(String name, String jti, List<String> clusterPermissions, List<IndexPermission> indexPermissions) {
this.creationTime = Instant.now();
this.description = description;
this.name = name;
this.jti = jti;
this.clusterPermissions = clusterPermissions;
this.indexPermissions = indexPermissions;
Expand All @@ -42,22 +40,49 @@ public ApiToken(
String description,
String jti,
List<String> clusterPermissions,
List<RoleV7.Index> indexPermissions,
List<IndexPermission> indexPermissions,
Instant creationTime
) {
this.description = description;
this.name = description;
this.jti = jti;
this.clusterPermissions = clusterPermissions;
this.indexPermissions = indexPermissions;
this.creationTime = creationTime;

}

public static class IndexPermission implements ToXContent {
private final List<String> indexPatterns;
private final List<String> allowedActions;

public IndexPermission(List<String> indexPatterns, List<String> allowedActions) {
this.indexPatterns = indexPatterns;
this.allowedActions = allowedActions;
}

public List<String> getAllowedActions() {
return allowedActions;
}

public List<String> 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<String> clusterPermissions = new ArrayList<>();
List<RoleV7.Index> indexPermissions = new ArrayList<>();
List<IndexPermission> indexPermissions = new ArrayList<>();
Instant creationTime = null;

XContentParser.Token token;
Expand Down Expand Up @@ -110,23 +135,16 @@ 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<String> indexPatterns = new ArrayList<>();
List<String> allowedActions = new ArrayList<>();
String dls = "";
List<String> fls = new ArrayList<>();
List<String> maskedFields = new ArrayList<>();

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":
Expand All @@ -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;

}
}
}
Expand All @@ -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<String> getClusterPermissions() {
return clusterPermissions;
}
Expand All @@ -195,7 +196,7 @@ public void setClusterPermissions(List<String> 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);
Expand All @@ -204,11 +205,11 @@ public XContentBuilder toXContent(XContentBuilder xContentBuilder, ToXContent.Pa
return xContentBuilder;
}

public List<RoleV7.Index> getIndexPermissions() {
public List<IndexPermission> getIndexPermissions() {
return indexPermissions;
}

public void setIndexPermissions(List<RoleV7.Index> indexPermissions) {
public void setIndexPermissions(List<IndexPermission> indexPermissions) {
this.indexPermissions = indexPermissions;
}
}
26 changes: 5 additions & 21 deletions src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<RestHandler.Route> ROUTES = addRoutesPrefix(
ImmutableList.of(
Expand Down Expand Up @@ -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();
}
Expand All @@ -120,7 +116,7 @@ private RestChannelConsumer handlePost(RestRequest request, NodeClient client) {
validateRequestParameters(requestBody);

List<String> clusterPermissions = extractClusterPermissions(requestBody);
List<RoleV7.Index> indexPermissions = extractIndexPermissions(requestBody);
List<ApiToken.IndexPermission> indexPermissions = extractIndexPermissions(requestBody);

String token = apiTokenRepository.createApiToken(
(String) requestBody.get(NAME_JSON_PROPERTY),
Expand Down Expand Up @@ -193,7 +189,7 @@ List<String> extractClusterPermissions(Map<String, Object> requestBody) {
/**
* Extracts and builds index permissions from the request body
*/
List<RoleV7.Index> extractIndexPermissions(Map<String, Object> requestBody) {
List<ApiToken.IndexPermission> extractIndexPermissions(Map<String, Object> requestBody) {
if (!requestBody.containsKey(INDEX_PERMISSIONS_FIELD)) {
return Collections.emptyList();
}
Expand All @@ -206,7 +202,7 @@ List<RoleV7.Index> extractIndexPermissions(Map<String, Object> requestBody) {
/**
* Creates a single RoleV7.Index permission from a permission map
*/
RoleV7.Index createIndexPermission(Map<String, Object> indexPerm) {
ApiToken.IndexPermission createIndexPermission(Map<String, Object> indexPerm) {
List<String> indexPatterns;
Object indexPatternObj = indexPerm.get(INDEX_PATTERN_FIELD);
if (indexPatternObj instanceof String) {
Expand All @@ -217,15 +213,7 @@ RoleV7.Index createIndexPermission(Map<String, Object> indexPerm) {

List<String> allowedActions = safeStringList(indexPerm.get(ALLOWED_ACTIONS_FIELD), ALLOWED_ACTIONS_FIELD);

String dls = (String) indexPerm.getOrDefault(DLS_FIELD, "");

List<String> fls = indexPerm.containsKey(FLS_FIELD) ? safeStringList(indexPerm.get(FLS_FIELD), FLS_FIELD) : Collections.emptyList();

List<String> 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);
}

/**
Expand Down Expand Up @@ -265,10 +253,6 @@ void validateIndexPermissionsList(List<Map<String, Object>> 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");
}
}
}

Expand Down
8 changes: 4 additions & 4 deletions src/main/java/org/opensearch/security/action/apitokens/ApiTokenIndexHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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);
Expand Down Expand Up @@ -114,7 +114,7 @@ public Map<String, ApiToken> 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;
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/opensearch/security/action/apitokens/ApiTokenRepository.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> clusterPermissions, List<RoleV7.Index> indexPermissions) {
public String createApiToken(String name, List<String> clusterPermissions, List<ApiToken.IndexPermission> 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));
}

Expand Down
34 changes: 1 addition & 33 deletions src/main/java/org/opensearch/security/securityconf/impl/v7/RoleV7.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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<String> index_patterns = Collections.emptyList();
private String dls;
Expand All @@ -69,31 +62,6 @@ public Index() {
super();
}

public Index(List<String> indexPatterns, List<String> allowedActions, String dls, List<String> fls, List<String> 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<String> getIndex_patterns() {
return index_patterns;
}
Expand Down
Loading

0 comments on commit 725c3f8

Please sign in to comment.