diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiAction.java index e27905f66e..f8531bb72f 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/ActionGroupsApiAction.java @@ -18,6 +18,7 @@ import java.io.IOException; import java.nio.file.Path; import java.util.List; +import java.util.Set; import com.fasterxml.jackson.databind.JsonNode; import com.google.common.collect.ImmutableList; @@ -113,23 +114,20 @@ protected void handlePut(RestChannel channel, RestRequest request, Client client } // Prevent the case where action group and role share a same name. - SecurityDynamicConfiguration existingRoles = load(CType.ROLES, false); - for (String role : existingRoles.getCEntries().keySet()) { - if (role.equals(name)) { - badRequestResponse(channel, name + " is an existing role. A action group cannot be named with an existing role name."); - return; - } + SecurityDynamicConfiguration existingRolesConfig = load(CType.ROLES, false); + Set existingRoles = existingRolesConfig.getCEntries().keySet(); + if (existingRoles.contains(name)) { + badRequestResponse(channel, name + " is an existing role. A action group cannot be named with an existing role name."); + return; } // Prevent the case where action group references to itself in the allowed_actions. - final SecurityDynamicConfiguration existingActionGroups = load(getConfigName(), false); - existingActionGroups.putCObject(name, DefaultObjectMapper.readTree(content, existingActionGroups.getImplementingClass())); - - for (String allowed_action : ((ActionGroupsV7) existingActionGroups.getCEntry(name)).getAllowed_actions()) { - if (allowed_action.equals(name)) { - badRequestResponse(channel, name + " cannot be an allowed_action of itself"); - return; - } + final SecurityDynamicConfiguration existingActionGroupsConfig = load(getConfigName(), false); + existingActionGroupsConfig.putCObject(name, DefaultObjectMapper.readTree(content, existingActionGroupsConfig.getImplementingClass())); + List allowedActions = ((ActionGroupsV7) existingActionGroupsConfig.getCEntry(name)).getAllowed_actions(); + if (allowedActions.contains(name)) { + badRequestResponse(channel, name + " cannot be an allowed_action of itself"); + return; } super.handlePut(channel, request, client, content); diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/PatchableResourceApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/PatchableResourceApiAction.java index 1d4abd4f0f..3d5529ee8e 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/PatchableResourceApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/PatchableResourceApiAction.java @@ -19,6 +19,7 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Path; import java.util.Iterator; +import java.util.List; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; @@ -159,7 +160,7 @@ private void handleSinglePatch(RestChannel channel, RestRequest request, Client , existingConfiguration.getVersion(), existingConfiguration.getSeqNo(), existingConfiguration.getPrimaryTerm()); if (existingConfiguration.getCType().equals(CType.ACTIONGROUPS)) { - if(selfReferencingActionGroup(mdc, name)) { + if(hasActionGroupSelfReference(mdc, name)) { badRequestResponse(channel, name + " cannot be an allowed_action of itself"); return; } @@ -235,7 +236,7 @@ private void handleBulkPatch(RestChannel channel, RestRequest request, Client cl if (existingConfiguration.getCType().equals(CType.ACTIONGROUPS)) { for (String actiongroup : mdc.getCEntries().keySet()) { - if(selfReferencingActionGroup(mdc, actiongroup)) { + if(hasActionGroupSelfReference(mdc, actiongroup)) { badRequestResponse(channel, actiongroup + " cannot be an allowed_action of itself"); return; } @@ -280,12 +281,8 @@ private AbstractConfigurationValidator getValidator(RestRequest request, JsonNod } // Prevent the case where action group references to itself in the allowed_actions. - private Boolean selfReferencingActionGroup(SecurityDynamicConfiguration mdc, String name) { - for (String allowed_action : ((ActionGroupsV7) mdc.getCEntry(name)).getAllowed_actions()) { - if (allowed_action.equals(name)) { - return true; - } - } - return false; + private Boolean hasActionGroupSelfReference(SecurityDynamicConfiguration mdc, String name) { + List allowedActions = ((ActionGroupsV7) mdc.getCEntry(name)).getAllowed_actions(); + return allowedActions.contains(name); } }