From 1499fa7ab94b1e08753f56e4fdaa7100d60a3a16 Mon Sep 17 00:00:00 2001 From: Andrey Pleskach Date: Thu, 14 Sep 2023 18:06:37 +0200 Subject: [PATCH] Change HTTP routs for Audit and Config PUT methods --- .../dlic/rest/api/AbstractApiAction.java | 9 +++++-- .../dlic/rest/api/AuditApiAction.java | 16 ++++-------- .../rest/api/SecurityConfigApiAction.java | 25 +++++++------------ .../api/AuditApiActionValidationTest.java | 12 --------- ....java => SecurityConfigApiActionTest.java} | 4 +-- ...SecurityConfigApiActionValidationTest.java | 15 ----------- ...> LegacySecurityConfigApiActionTests.java} | 4 +-- 7 files changed, 25 insertions(+), 60 deletions(-) rename src/test/java/org/opensearch/security/dlic/rest/api/{SecurityConfigApiTest.java => SecurityConfigApiActionTest.java} (98%) rename src/test/java/org/opensearch/security/dlic/rest/api/legacy/{LegacySecurityConfigApiTests.java => LegacySecurityConfigApiActionTests.java} (77%) diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/AbstractApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/AbstractApiAction.java index 6d3710871b..293f934434 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/AbstractApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/AbstractApiAction.java @@ -309,8 +309,13 @@ protected final Set patchEntityNames(final JsonNode patchRequestContent) } protected final ValidationResult processPutRequest(final RestRequest request) throws IOException { - return endpointValidator.withRequiredEntityName(nameParam(request)) - .map(entityName -> loadConfigurationWithRequestContent(entityName, request)) + return processPutRequest(nameParam(request), request); + } + + protected final ValidationResult processPutRequest(final String entityName, final RestRequest request) + throws IOException { + return endpointValidator.withRequiredEntityName(entityName) + .map(ignore -> loadConfigurationWithRequestContent(entityName, request)) .map(endpointValidator::onConfigChange) .map(this::addEntityToConfig); } diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/AuditApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/AuditApiAction.java index 488a72c958..20e424e959 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/AuditApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/AuditApiAction.java @@ -38,7 +38,6 @@ import java.util.Set; import static org.opensearch.security.dlic.rest.api.RequestHandler.methodNotImplementedHandler; -import static org.opensearch.security.dlic.rest.api.Responses.badRequestMessage; import static org.opensearch.security.dlic.rest.api.Responses.conflictMessage; import static org.opensearch.security.dlic.rest.api.Responses.methodNotImplementedMessage; import static org.opensearch.security.dlic.rest.support.Utils.addRoutesPrefix; @@ -124,7 +123,7 @@ public class AuditApiAction extends AbstractApiAction { private static final List routes = addRoutesPrefix( ImmutableList.of( new Route(RestRequest.Method.GET, "/audit/"), - new Route(RestRequest.Method.PUT, "/audit/{name}"), + new Route(RestRequest.Method.PUT, "/audit/config"), new Route(RestRequest.Method.PATCH, "/audit/") ) ); @@ -237,6 +236,9 @@ protected CType getConfigType() { return CType.AUDIT; } + @Override + protected void consumeParameters(RestRequest request) {} + private void auditApiRequestHandlers(RequestHandler.RequestHandlersBuilder requestHandlersBuilder) { requestHandlersBuilder.onGetRequest( request -> withEnabledAuditApi(request).map(this::processGetRequest).map(securityConfiguration -> { @@ -248,7 +250,7 @@ private void auditApiRequestHandlers(RequestHandler.RequestHandlersBuilder reque .onChangeRequest(RestRequest.Method.PATCH, request -> withEnabledAuditApi(request).map(this::processPatchRequest)) .onChangeRequest( RestRequest.Method.PUT, - request -> withEnabledAuditApi(request).map(this::withConfigEntityNameOnly).map(ignore -> processPutRequest(request)) + request -> withEnabledAuditApi(request).map(ignore -> processPutRequest("config", request)) ) .override(RestRequest.Method.POST, methodNotImplementedHandler) .override(RestRequest.Method.DELETE, methodNotImplementedHandler); @@ -261,14 +263,6 @@ ValidationResult withEnabledAuditApi(final RestRequest request) { return ValidationResult.success(request); } - ValidationResult withConfigEntityNameOnly(final RestRequest request) { - final var name = nameParam(request); - if (!"config".equals(name)) { - return ValidationResult.error(RestStatus.BAD_REQUEST, badRequestMessage("name must be config")); - } - return ValidationResult.success(name); - } - @Override protected EndpointValidator createEndpointValidator() { return new EndpointValidator() { diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiAction.java b/src/main/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiAction.java index 62865cf2e1..6c6ea0f827 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiAction.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiAction.java @@ -11,12 +11,7 @@ package org.opensearch.security.dlic.rest.api; -import java.util.Collections; -import java.util.List; -import java.util.Map; - import com.google.common.collect.ImmutableList; - import com.google.common.collect.ImmutableMap; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.inject.Inject; @@ -32,8 +27,11 @@ import org.opensearch.security.support.ConfigConstants; import org.opensearch.threadpool.ThreadPool; +import java.util.Collections; +import java.util.List; +import java.util.Map; + import static org.opensearch.security.dlic.rest.api.RequestHandler.methodNotImplementedHandler; -import static org.opensearch.security.dlic.rest.api.Responses.badRequestMessage; import static org.opensearch.security.dlic.rest.api.Responses.methodNotImplementedMessage; import static org.opensearch.security.dlic.rest.support.Utils.addRoutesPrefix; @@ -43,7 +41,7 @@ public class SecurityConfigApiAction extends AbstractApiAction { private static final List allRoutes = new ImmutableList.Builder().addAll(getRoutes) .addAll( - addRoutesPrefix(ImmutableList.of(new Route(Method.PUT, "/securityconfig/{name}"), new Route(Method.PATCH, "/securityconfig/"))) + addRoutesPrefix(ImmutableList.of(new Route(Method.PUT, "/securityconfig/config"), new Route(Method.PATCH, "/securityconfig/"))) ) .build(); @@ -72,10 +70,13 @@ protected CType getConfigType() { return CType.CONFIG; } + @Override + protected void consumeParameters(RestRequest request) {} + private void securityConfigApiActionRequestHandlers(RequestHandler.RequestHandlersBuilder requestHandlersBuilder) { requestHandlersBuilder.onChangeRequest( Method.PUT, - request -> withAllowedEndpoint(request).map(this::withConfigEntityNameOnly).map(ignore -> processPutRequest(request)) + request -> withAllowedEndpoint(request).map(ignore -> processPutRequest("config", request)) ) .onChangeRequest(Method.PATCH, request -> withAllowedEndpoint(request).map(this::processPatchRequest)) .override(Method.DELETE, methodNotImplementedHandler) @@ -89,14 +90,6 @@ ValidationResult withAllowedEndpoint(final RestRequest request) { return ValidationResult.success(request); } - ValidationResult withConfigEntityNameOnly(final RestRequest request) { - final var name = nameParam(request); - if (!"config".equals(name)) { - return ValidationResult.error(RestStatus.BAD_REQUEST, badRequestMessage("name must be config")); - } - return ValidationResult.success(name); - } - @Override protected EndpointValidator createEndpointValidator() { return new EndpointValidator() { diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/AuditApiActionValidationTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/AuditApiActionValidationTest.java index b42072dcbf..7ffbda2fce 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/AuditApiActionValidationTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/AuditApiActionValidationTest.java @@ -20,7 +20,6 @@ import org.opensearch.security.util.FakeRestRequest; import java.util.List; -import java.util.Map; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -51,17 +50,6 @@ public void enabledAuditApi() { } } - @Test - public void configEntityNameOnly() { - final var auditApiAction = new AuditApiAction(clusterService, threadPool, securityApiDependencies); - var result = auditApiAction.withConfigEntityNameOnly(FakeRestRequest.builder().withParams(Map.of("name", "aaaaa")).build()); - assertFalse(result.isValid()); - assertEquals(RestStatus.BAD_REQUEST, result.status()); - - result = auditApiAction.withConfigEntityNameOnly(FakeRestRequest.builder().withParams(Map.of("name", "config")).build()); - assertTrue(result.isValid()); - } - @Test public void onChangeVerifyReadonlyFields() throws Exception { final var auditApiActionEndpointValidator = new AuditApiAction( diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiActionTest.java similarity index 98% rename from src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiTest.java rename to src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiActionTest.java index 8fc6ae1dd8..e0bee59cec 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiActionTest.java @@ -24,14 +24,14 @@ import static org.opensearch.security.OpenSearchSecurityPlugin.PLUGINS_PREFIX; -public class SecurityConfigApiTest extends AbstractRestApiUnitTest { +public class SecurityConfigApiActionTest extends AbstractRestApiUnitTest { private final String ENDPOINT; protected String getEndpointPrefix() { return PLUGINS_PREFIX; } - public SecurityConfigApiTest() { + public SecurityConfigApiActionTest() { ENDPOINT = getEndpointPrefix() + "/api"; } diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiActionValidationTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiActionValidationTest.java index e5993b3698..e3cbfe4120 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiActionValidationTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/SecurityConfigApiActionValidationTest.java @@ -17,27 +17,12 @@ import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.util.FakeRestRequest; -import java.util.Map; - import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; public class SecurityConfigApiActionValidationTest extends AbstractApiActionValidationTest { - @Test - public void configEntityNameOnly() { - final var securityConfigApiAction = new SecurityConfigApiAction(clusterService, threadPool, securityApiDependencies); - var result = securityConfigApiAction.withConfigEntityNameOnly( - FakeRestRequest.builder().withParams(Map.of("name", "aaaaa")).build() - ); - assertFalse(result.isValid()); - assertEquals(RestStatus.BAD_REQUEST, result.status()); - - result = securityConfigApiAction.withConfigEntityNameOnly(FakeRestRequest.builder().withParams(Map.of("name", "config")).build()); - assertTrue(result.isValid()); - } - @Test public void withAllowedEndpoint() { var securityConfigApiAction = new SecurityConfigApiAction( diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/legacy/LegacySecurityConfigApiTests.java b/src/test/java/org/opensearch/security/dlic/rest/api/legacy/LegacySecurityConfigApiActionTests.java similarity index 77% rename from src/test/java/org/opensearch/security/dlic/rest/api/legacy/LegacySecurityConfigApiTests.java rename to src/test/java/org/opensearch/security/dlic/rest/api/legacy/LegacySecurityConfigApiActionTests.java index 6175809b4a..7d94de03bd 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/legacy/LegacySecurityConfigApiTests.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/legacy/LegacySecurityConfigApiActionTests.java @@ -11,11 +11,11 @@ package org.opensearch.security.dlic.rest.api.legacy; -import org.opensearch.security.dlic.rest.api.SecurityConfigApiTest; +import org.opensearch.security.dlic.rest.api.SecurityConfigApiActionTest; import static org.opensearch.security.OpenSearchSecurityPlugin.LEGACY_OPENDISTRO_PREFIX; -public class LegacySecurityConfigApiTests extends SecurityConfigApiTest { +public class LegacySecurityConfigApiActionTests extends SecurityConfigApiActionTest { @Override protected String getEndpointPrefix() { return LEGACY_OPENDISTRO_PREFIX;