From 07f9418952d3dec141d6953099e0456f85f68807 Mon Sep 17 00:00:00 2001 From: Jim Hahn Date: Mon, 29 Mar 2021 16:31:23 -0400 Subject: Validate parameters of REST calls Added code to validate the parameters of the REST calls. As it turned out, validation only needed to be added to one call. Issue-ID: POLICY-2542 Change-Id: Ia9aabf75e06d6d5f996be9e3ed804218319f70c2 Signed-off-by: Jim Hahn --- .../pap/main/rest/PdpGroupDeployProvider.java | 45 +++++++++++++++++++++- .../pap/main/rest/TestPdpGroupDeployProvider.java | 39 +++++++++++++++++++ .../simpleDeploy/PapPoliciesInvalidPolicyName.json | 8 ++++ .../PapPoliciesInvalidPolicyVersion.json | 8 ++++ .../resources/simpleDeploy/PapPoliciesList.json | 16 ++++++++ .../simpleDeploy/PapPoliciesNullItem.json | 3 ++ .../simpleDeploy/PapPoliciesNullPolicyName.json | 8 ++++ 7 files changed, 125 insertions(+), 2 deletions(-) create mode 100644 main/src/test/resources/simpleDeploy/PapPoliciesInvalidPolicyName.json create mode 100644 main/src/test/resources/simpleDeploy/PapPoliciesInvalidPolicyVersion.json create mode 100644 main/src/test/resources/simpleDeploy/PapPoliciesList.json create mode 100644 main/src/test/resources/simpleDeploy/PapPoliciesNullItem.json create mode 100644 main/src/test/resources/simpleDeploy/PapPoliciesNullPolicyName.json (limited to 'main/src') diff --git a/main/src/main/java/org/onap/policy/pap/main/rest/PdpGroupDeployProvider.java b/main/src/main/java/org/onap/policy/pap/main/rest/PdpGroupDeployProvider.java index 56d2ad3f..41f7757b 100644 --- a/main/src/main/java/org/onap/policy/pap/main/rest/PdpGroupDeployProvider.java +++ b/main/src/main/java/org/onap/policy/pap/main/rest/PdpGroupDeployProvider.java @@ -21,6 +21,7 @@ package org.onap.policy.pap.main.rest; +import com.google.gson.annotations.SerializedName; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; @@ -30,11 +31,19 @@ import java.util.Map; import java.util.Set; import java.util.stream.Collectors; import javax.ws.rs.core.Response.Status; +import lombok.Getter; import org.onap.policy.common.parameters.BeanValidationResult; +import org.onap.policy.common.parameters.BeanValidator; import org.onap.policy.common.parameters.ObjectValidationResult; import org.onap.policy.common.parameters.ValidationResult; import org.onap.policy.common.parameters.ValidationStatus; +import org.onap.policy.common.parameters.annotations.NotNull; +import org.onap.policy.common.parameters.annotations.Pattern; +import org.onap.policy.common.parameters.annotations.Valid; +import org.onap.policy.common.utils.coder.CoderException; +import org.onap.policy.common.utils.coder.StandardCoder; import org.onap.policy.common.utils.services.Registry; +import org.onap.policy.models.base.PfKey; import org.onap.policy.models.base.PfModelException; import org.onap.policy.models.base.PfModelRuntimeException; import org.onap.policy.models.pap.concepts.PdpDeployPolicies; @@ -61,6 +70,7 @@ import org.slf4j.LoggerFactory; */ public class PdpGroupDeployProvider extends ProviderBase { private static final Logger logger = LoggerFactory.getLogger(PdpGroupDeployProvider.class); + private static final StandardCoder coder = new StandardCoder(); private static final String POLICY_RESULT_NAME = "policy"; @@ -80,10 +90,8 @@ public class PdpGroupDeployProvider extends ProviderBase { */ public void updateGroupPolicies(DeploymentGroups groups) throws PfModelException { ValidationResult result = groups.validatePapRest(); - if (!result.isValid()) { String msg = result.getResult().trim(); - logger.warn(msg); throw new PfModelException(Status.BAD_REQUEST, msg); } @@ -382,6 +390,17 @@ public class PdpGroupDeployProvider extends ProviderBase { * @throws PfModelException if an error occurred */ public void deployPolicies(PdpDeployPolicies policies) throws PfModelException { + try { + MyPdpDeployPolicies checked = coder.convert(policies, MyPdpDeployPolicies.class); + ValidationResult result = new BeanValidator().validateTop(PdpDeployPolicies.class.getSimpleName(), checked); + if (!result.isValid()) { + String msg = result.getResult().trim(); + throw new PfModelException(Status.BAD_REQUEST, msg); + } + } catch (CoderException e) { + throw new PfModelException(Status.INTERNAL_SERVER_ERROR, "cannot decode request", e); + } + process(policies, this::deploySimplePolicies); } @@ -512,4 +531,26 @@ public class PdpGroupDeployProvider extends ProviderBase { return false; } + + /* + * These are only used to validate the incoming request. + */ + + @Getter + public static class MyPdpDeployPolicies { + @NotNull + private List<@NotNull @Valid PolicyIdent> policies; + } + + @Getter + public static class PolicyIdent { + @SerializedName("policy-id") + @NotNull + @Pattern(regexp = PfKey.NAME_REGEXP) + private String name; + + @SerializedName("policy-version") + @Pattern(regexp = "\\d+([.]\\d+[.]\\d+)?") + private String version; + } } diff --git a/main/src/test/java/org/onap/policy/pap/main/rest/TestPdpGroupDeployProvider.java b/main/src/test/java/org/onap/policy/pap/main/rest/TestPdpGroupDeployProvider.java index a7f48ff3..575dfefc 100644 --- a/main/src/test/java/org/onap/policy/pap/main/rest/TestPdpGroupDeployProvider.java +++ b/main/src/test/java/org/onap/policy/pap/main/rest/TestPdpGroupDeployProvider.java @@ -55,6 +55,7 @@ import org.onap.policy.models.tosca.authorative.concepts.ToscaConceptIdentifier; import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicy; public class TestPdpGroupDeployProvider extends ProviderSuper { + private static final String EXPECTED_EXCEPTION = "expected exception"; private static final String POLICY2_NAME = "policyB"; @@ -474,6 +475,44 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { assertThatCode(() -> prov.deployPolicies(loadEmptyRequest())).doesNotThrowAnyException(); } + /** + * Tests deployPolicies() when the policies are invalid. + */ + @Test + public void testDeployPoliciesInvalidPolicies() throws Exception { + // valid list + PdpDeployPolicies policies0 = loadFile("PapPoliciesList.json", PdpDeployPolicies.class); + assertThatCode(() -> prov.deployPolicies(policies0)).doesNotThrowAnyException(); + + // null list + PdpDeployPolicies policies = new PdpDeployPolicies(); + assertThatThrownBy(() -> prov.deployPolicies(policies)).isInstanceOf(PfModelException.class) + .hasMessageContaining("policies"); + + // list containing null item + PdpDeployPolicies policies2 = loadFile("PapPoliciesNullItem.json", PdpDeployPolicies.class); + assertThatThrownBy(() -> prov.deployPolicies(policies2)).isInstanceOf(PfModelException.class) + .hasMessageContaining("policies").hasMessageContaining("null"); + + // list containing a policy with a null name + PdpDeployPolicies policies3 = loadFile("PapPoliciesNullPolicyName.json", PdpDeployPolicies.class); + assertThatThrownBy(() -> prov.deployPolicies(policies3)).isInstanceOf(PfModelException.class) + .hasMessageContaining("policies").hasMessageContaining("name").hasMessageContaining("null") + .hasMessageNotContaining("\"value\""); + + // list containing a policy with an invalid name + PdpDeployPolicies policies4 = loadFile("PapPoliciesInvalidPolicyName.json", PdpDeployPolicies.class); + assertThatThrownBy(() -> prov.deployPolicies(policies4)).isInstanceOf(PfModelException.class) + .hasMessageContaining("policies").hasMessageContaining("name").hasMessageContaining("$ abc") + .hasMessageNotContaining("version"); + + // list containing a policy with an invalid version + PdpDeployPolicies policies5 = loadFile("PapPoliciesInvalidPolicyVersion.json", PdpDeployPolicies.class); + assertThatThrownBy(() -> prov.deployPolicies(policies5)).isInstanceOf(PfModelException.class) + .hasMessageContaining("policies").hasMessageContaining("version").hasMessageContaining("abc123") + .hasMessageNotContaining("name"); + } + /** * Tests deployPolicies() when the supported policy type uses a wild-card. * diff --git a/main/src/test/resources/simpleDeploy/PapPoliciesInvalidPolicyName.json b/main/src/test/resources/simpleDeploy/PapPoliciesInvalidPolicyName.json new file mode 100644 index 00000000..d155c3b6 --- /dev/null +++ b/main/src/test/resources/simpleDeploy/PapPoliciesInvalidPolicyName.json @@ -0,0 +1,8 @@ +{ + "policies": [ + { + "policy-id" : "$ abc", + "policy-version": "1.2.0" + } + ] +} diff --git a/main/src/test/resources/simpleDeploy/PapPoliciesInvalidPolicyVersion.json b/main/src/test/resources/simpleDeploy/PapPoliciesInvalidPolicyVersion.json new file mode 100644 index 00000000..c4b16671 --- /dev/null +++ b/main/src/test/resources/simpleDeploy/PapPoliciesInvalidPolicyVersion.json @@ -0,0 +1,8 @@ +{ + "policies": [ + { + "policy-id" : "myPolicy", + "policy-version": "abc123" + } + ] +} diff --git a/main/src/test/resources/simpleDeploy/PapPoliciesList.json b/main/src/test/resources/simpleDeploy/PapPoliciesList.json new file mode 100644 index 00000000..a4af6d24 --- /dev/null +++ b/main/src/test/resources/simpleDeploy/PapPoliciesList.json @@ -0,0 +1,16 @@ +{ + "policies": [ + { + "policy-id" : "MyPolicy0", + "policy-version": null + }, + { + "policy-id" : "MyPolicy1", + "policy-version": "1" + }, + { + "policy-id" : "MyPolicy2", + "policy-version": "1.2.2" + } + ] +} diff --git a/main/src/test/resources/simpleDeploy/PapPoliciesNullItem.json b/main/src/test/resources/simpleDeploy/PapPoliciesNullItem.json new file mode 100644 index 00000000..2e035ec5 --- /dev/null +++ b/main/src/test/resources/simpleDeploy/PapPoliciesNullItem.json @@ -0,0 +1,3 @@ +{ + "policies": [null] +} diff --git a/main/src/test/resources/simpleDeploy/PapPoliciesNullPolicyName.json b/main/src/test/resources/simpleDeploy/PapPoliciesNullPolicyName.json new file mode 100644 index 00000000..945e3ca4 --- /dev/null +++ b/main/src/test/resources/simpleDeploy/PapPoliciesNullPolicyName.json @@ -0,0 +1,8 @@ +{ + "policies": [ + { + "policy-id" : null, + "policy-version": "1.2.0" + } + ] +} -- cgit 1.2.3-korg