Skip to content

Commit

Permalink
Cleanup using constants for better refactorability
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 725c3f8 commit 16c1615
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 38 deletions.
53 changes: 30 additions & 23 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 @@ -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;
Expand Down Expand Up @@ -71,15 +79,15 @@ public List<String> 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<String> clusterPermissions = new ArrayList<>();
List<IndexPermission> indexPermissions = new ArrayList<>();
Expand All @@ -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));
Expand All @@ -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 {
Expand All @@ -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());
}
Expand All @@ -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);
Expand Down Expand Up @@ -196,11 +203,11 @@ public void setClusterPermissions(List<String> 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;
}
Expand Down
26 changes: 13 additions & 13 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 @@ -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<RestHandler.Route> ROUTES = addRoutesPrefix(
ImmutableList.of(
new RestHandler.Route(POST, "/apitokens"),
Expand Down Expand Up @@ -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();
Expand All @@ -119,7 +119,7 @@ private RestChannelConsumer handlePost(RestRequest request, NodeClient client) {
List<ApiToken.IndexPermission> indexPermissions = extractIndexPermissions(requestBody);

String token = apiTokenRepository.createApiToken(
(String) requestBody.get(NAME_JSON_PROPERTY),
(String) requestBody.get(NAME_FIELD),
clusterPermissions,
indexPermissions
);
Expand Down Expand Up @@ -220,8 +220,8 @@ ApiToken.IndexPermission createIndexPermission(Map<String, Object> indexPerm) {
* Validates the request parameters
*/
void validateRequestParameters(Map<String, Object> 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)) {
Expand Down Expand Up @@ -264,10 +264,10 @@ private RestChannelConsumer handleDelete(RestRequest request, NodeClient client)
final Map<String, Object> 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);
Expand Down
4 changes: 2 additions & 2 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 @@ -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();

Expand Down Expand Up @@ -114,7 +114,7 @@ public Map<String, ApiToken> getApiTokens() {
) {

ApiToken token = ApiToken.fromXContent(parser);
tokens.put(token.getName(), token); // Assuming description is the key
tokens.put(token.getName(), token);
}
}
return tokens;
Expand Down

0 comments on commit 16c1615

Please sign in to comment.