Prohibit specifying primary key route for REST POST requests (#1364)

## Why make this change?
Fixes #1365, to prohibit
specifying primary key route in REST POST requests.

## What is this change?
Added an extra validation to check if primary key route is non-empty for
POST requests.

## Additional changes
Validations for primary key route and query string were of similar
nature for all the operations but had separate functions created for
each of the operations. Removed those specific functions and created a
new function
`RequestValidator.ValidatePrimaryKeyRouteAndQueryStringInURL` which does
those validations for all of these operations.

## How was this tested?

- [x] Integration Tests

## Sample Request(s)

- Example REST POST request:
URL: https://localhost:5001/api/commodities/categoryid/0/pieceid/6
Body: 
```
{
            "categoryid": 0,
            "pieceid": 6,
            "categoryName": "SciFi"
}
```
Response: 
```
{
    "error": {
        "code": "BadRequest",
        "message": "Primary key for POST request can't be specified in request URL. Use request body instead.",
        "status": 400
    }
}
```
In current scenario, we would have allowed such a request to execute.
Also, even if the url parameters here were categoryid/0/pieceid/7, the
record still would have been created corresponding to categoryid=6 and
pieceid=0 as specified in the request body, thus obviously causing
confusion to the user.

Author: ayush3797

src/Service.Tests/SqlTests/RestApiTests/Insert/InsertApiTestBase.cs

@@ -263,9 +263,13 @@ await SetupAndRunRestApiTest(
 
         #region Negative Tests
 
+        /// <summary>
+        /// Test to validate that an exception is encountered when the user specifies primary key route or query string for a POST request via REST.
+        /// </summary>
         [TestMethod]
-        public virtual async Task InsertOneWithInvalidQueryStringTest()
+        public virtual async Task InsertOneWithPrimaryKeyOrQueryStringInURLTest()
         {
+            // Validate that a POST request is not allowed to include a query string in the URL.
             string requestBody = @"
             {
                 ""title"": ""My New Book"",
@@ -274,7 +278,7 @@ public virtual async Task InsertOneWithInvalidQueryStringTest()
 
             await SetupAndRunRestApiTest(
                 primaryKeyRoute: string.Empty,
-                queryString: "?/id/5001",
+                queryString: "?$filter=id eq 5001",
                 entityNameOrPath: _integrationEntityName,
                 sqlQuery: string.Empty,
                 operationType: Config.Operation.Insert,
@@ -283,6 +287,26 @@ await SetupAndRunRestApiTest(
                 expectedErrorMessage: RequestValidator.QUERY_STRING_INVALID_USAGE_ERR_MESSAGE,
                 expectedStatusCode: HttpStatusCode.BadRequest
             );
+
+            //Validate that a POST request is not allowed to include primary key in the URL.
+            requestBody = @"
+            {
+                ""categoryid"": 0,
+                ""pieceid"": 4,
+                ""categoryName"": ""SciFi""
+            }";
+
+            await SetupAndRunRestApiTest(
+                primaryKeyRoute: "categoryid/0/pieceid/4",
+                queryString: string.Empty,
+                entityNameOrPath: _integrationEntityName,
+                sqlQuery: string.Empty,
+                operationType: Config.Operation.Insert,
+                requestBody: requestBody,
+                exceptionExpected: true,
+                expectedErrorMessage: RequestValidator.PRIMARY_KEY_INVALID_USAGE_ERR_MESSAGE,
+                expectedStatusCode: HttpStatusCode.BadRequest
+            );
         }
 
         /// <summary>

src/Service.Tests/Unittests/RequestValidatorUnitTests.cs

@@ -160,15 +160,15 @@ public void RequestBodyJsonParsing(string requestBody, bool expectsException)
             if (expectsException)
             {
                 DataApiBuilderException dabException = Assert.ThrowsException<DataApiBuilderException>(
-                    action: () => RequestValidator.ParseRequestBody(requestBody),
+                    action: () => RequestValidator.ValidateAndParseRequestBody(requestBody),
                     message: RequestValidator.REQUEST_BODY_INVALID_JSON_ERR_MESSAGE);
 
                 Assert.AreEqual(expected: HttpStatusCode.BadRequest, actual: dabException.StatusCode);
                 Assert.AreEqual(expected: DataApiBuilderException.SubStatusCodes.BadRequest, actual: dabException.SubStatusCode);
             }
             else
             {
-                RequestValidator.ParseRequestBody(requestBody);
+                RequestValidator.ValidateAndParseRequestBody(requestBody);
             }
         }
         #endregion

src/Service/Services/RequestValidator.cs

@@ -20,6 +20,7 @@ public class RequestValidator
         public const string REQUEST_BODY_INVALID_JSON_ERR_MESSAGE = "Request body contains invalid JSON.";
         public const string BATCH_MUTATION_UNSUPPORTED_ERR_MESSAGE = "A Mutation operation on more than one entity in a single request is not yet supported.";
         public const string QUERY_STRING_INVALID_USAGE_ERR_MESSAGE = "Query string for this HTTP request type is an invalid URL.";
+        public const string PRIMARY_KEY_INVALID_USAGE_ERR_MESSAGE = "Primary key for POST requests can't be specified in the request URL. Use request body instead.";
         public const string PRIMARY_KEY_NOT_PROVIDED_ERR_MESSAGE = "Primary Key for this HTTP request type is required.";
 
         /// <summary>
@@ -177,13 +178,14 @@ public static void ValidateStoredProcedureRequestContext(
         }
 
         /// <summary>
-        /// Validates the primarykeyroute is populated with respect to an Update or Upsert operation.
+        /// Validates the request body is a valid JSON string and is not a JSON array since we only
+        /// support point operations.
         /// </summary>
         /// <param name="requestBody">Request body content</param>
         /// <exception cref="DataApiBuilderException">Thrown when request body is invalid JSON
         /// or JSON is a batch request (mutation of more than one entity in a single request).</exception>
         /// <returns>JsonElement representing the body of the request.</returns>
-        public static JsonElement ParseRequestBody(string requestBody)
+        public static JsonElement ValidateAndParseRequestBody(string requestBody)
         {
             JsonElement mutationPayloadRoot = new();
 
@@ -217,56 +219,58 @@ public static JsonElement ParseRequestBody(string requestBody)
         }
 
         /// <summary>
-        /// Validates the request body and queryString with respect to an Insert operation.
+        /// Performs validations on primary key route and queryString specified in the request URL, whose specifics depend on the type of operation being executed.
+        /// Eg. a POST request cannot have primary key route/query string in the URL while it is mandatory for a PUT/PATCH/DELETE request to have a primary key route in the URL.
         /// </summary>
+        /// <param name="operationType">Type of operation being executed.</param>
+        /// <param name="primaryKeyRoute">URL route e.g. "Entity/id/1"</param>
         /// <param name="queryString">queryString e.g. "$?filter="</param>
-        /// <param name="requestBody">The string JSON body from the request.</param>
-        /// <exception cref="DataApiBuilderException">Raised when queryString is present or invalid JSON in requestBody is found.</exception>
-        public static JsonElement ValidateInsertRequest(string queryString, string requestBody)
+        /// <exception cref="DataApiBuilderException">Raised when primaryKeyRoute/queryString fail the validations for the operation.</exception>
+        public static void ValidatePrimaryKeyRouteAndQueryStringInURL(Config.Operation operationType, string? primaryKeyRoute = null, string? queryString = null)
         {
-            if (!string.IsNullOrEmpty(queryString))
-            {
-                throw new DataApiBuilderException(
-                    message: QUERY_STRING_INVALID_USAGE_ERR_MESSAGE,
-                    statusCode: HttpStatusCode.BadRequest,
-                    subStatusCode: DataApiBuilderException.SubStatusCodes.BadRequest);
-            }
+            bool isPrimaryKeyRouteEmpty = string.IsNullOrEmpty(primaryKeyRoute);
+            bool isQueryStringEmpty = string.IsNullOrEmpty(queryString);
 
-            return ParseRequestBody(requestBody);
-        }
-
-        /// <summary>
-        /// Validates the primarykeyroute is populated with respect to an Update or Upsert operation.
-        /// </summary>
-        /// <param name="primaryKeyRoute">URL route e.g. "Entity/id/1"</param>
-        /// <param name="requestBody">The string JSON body from the request.</param>
-        /// <exception cref="DataApiBuilderException">Raised when either primary key route is absent or invalid JSON in requestBody is found.</exception>
-        public static JsonElement ValidateUpdateOrUpsertRequest(string? primaryKeyRoute, string requestBody)
-        {
-            if (string.IsNullOrWhiteSpace(primaryKeyRoute))
+            switch (operationType)
             {
-                throw new DataApiBuilderException(
-                    message: PRIMARY_KEY_NOT_PROVIDED_ERR_MESSAGE,
-                    statusCode: HttpStatusCode.BadRequest,
-                    subStatusCode: DataApiBuilderException.SubStatusCodes.BadRequest);
-            }
+                case Config.Operation.Insert:
+                    if (!isPrimaryKeyRouteEmpty)
+                    {
+                        throw new DataApiBuilderException(
+                            message: PRIMARY_KEY_INVALID_USAGE_ERR_MESSAGE,
+                            statusCode: HttpStatusCode.BadRequest,
+                            subStatusCode: DataApiBuilderException.SubStatusCodes.BadRequest);
+                    }
 
-            return ParseRequestBody(requestBody);
-        }
+                    if (!isQueryStringEmpty)
+                    {
+                        throw new DataApiBuilderException(
+                            message: QUERY_STRING_INVALID_USAGE_ERR_MESSAGE,
+                            statusCode: HttpStatusCode.BadRequest,
+                            subStatusCode: DataApiBuilderException.SubStatusCodes.BadRequest);
+                    }
 
-        /// <summary>
-        /// Validates the primarykeyroute is populated with respect to a Delete operation.
-        /// </summary>
-        /// <param name="primaryKeyRoute">URL route e.g. "Entity/id/1"</param>
-        /// <exception cref="DataApiBuilderException">Raised when primary key route is absent</exception>
-        public static void ValidateDeleteRequest(string? primaryKeyRoute)
-        {
-            if (string.IsNullOrWhiteSpace(primaryKeyRoute))
-            {
-                throw new DataApiBuilderException(
-                    message: PRIMARY_KEY_NOT_PROVIDED_ERR_MESSAGE,
-                    statusCode: HttpStatusCode.BadRequest,
-                    subStatusCode: DataApiBuilderException.SubStatusCodes.BadRequest);
+                    break;
+                case Config.Operation.Delete:
+                case Config.Operation.Update:
+                case Config.Operation.UpdateIncremental:
+                case Config.Operation.Upsert:
+                case Config.Operation.UpsertIncremental:
+                    /// Validate that the primarykeyroute is populated for these operations.
+                    if (isPrimaryKeyRouteEmpty)
+                    {
+                        throw new DataApiBuilderException(
+                            message: PRIMARY_KEY_NOT_PROVIDED_ERR_MESSAGE,
+                            statusCode: HttpStatusCode.BadRequest,
+                            subStatusCode: DataApiBuilderException.SubStatusCodes.BadRequest);
+                    }
+
+                    break;
+                default:
+                    throw new DataApiBuilderException(
+                        message: "Unexpected operation encountered. Cannot perform validations on URL components.",
+                        statusCode: HttpStatusCode.InternalServerError,
+                        subStatusCode: DataApiBuilderException.SubStatusCodes.UnexpectedError);
             }
         }
 

src/Service/Services/RestService.cs

@@ -117,7 +117,8 @@ RuntimeConfigProvider runtimeConfigProvider
                             isList: string.IsNullOrEmpty(primaryKeyRoute));
                         break;
                     case Config.Operation.Insert:
-                        JsonElement insertPayloadRoot = RequestValidator.ValidateInsertRequest(queryString, requestBody);
+                        RequestValidator.ValidatePrimaryKeyRouteAndQueryStringInURL(operationType, primaryKeyRoute, queryString);
+                        JsonElement insertPayloadRoot = RequestValidator.ValidateAndParseRequestBody(requestBody);
                         context = new InsertRequestContext(
                             entityName,
                             dbo: dbObject,
@@ -132,7 +133,7 @@ RuntimeConfigProvider runtimeConfigProvider
 
                         break;
                     case Config.Operation.Delete:
-                        RequestValidator.ValidateDeleteRequest(primaryKeyRoute);
+                        RequestValidator.ValidatePrimaryKeyRouteAndQueryStringInURL(operationType, primaryKeyRoute);
                         context = new DeleteRequestContext(entityName,
                                                            dbo: dbObject,
                                                            isList: false);
@@ -141,7 +142,8 @@ RuntimeConfigProvider runtimeConfigProvider
                     case Config.Operation.UpdateIncremental:
                     case Config.Operation.Upsert:
                     case Config.Operation.UpsertIncremental:
-                        JsonElement upsertPayloadRoot = RequestValidator.ValidateUpdateOrUpsertRequest(primaryKeyRoute, requestBody);
+                        RequestValidator.ValidatePrimaryKeyRouteAndQueryStringInURL(operationType, primaryKeyRoute);
+                        JsonElement upsertPayloadRoot = RequestValidator.ValidateAndParseRequestBody(requestBody);
                         context = new UpsertRequestContext(
                             entityName,
                             dbo: dbObject,
@@ -270,10 +272,10 @@ private void PopulateStoredProcedureContext(
                 case Config.Operation.UpdateIncremental:
                 case Config.Operation.Upsert:
                 case Config.Operation.UpsertIncremental:
-                    // Stored procedure call is semantically identical for all methods except Find, so we can
-                    // effectively reuse the ValidateInsertRequest - throws error if query string is nonempty
-                    // and parses the body into json
-                    JsonElement requestPayloadRoot = RequestValidator.ValidateInsertRequest(queryString, requestBody);
+                    // Stored procedure call is semantically identical for all methods except Find.
+                    // So, we can effectively treat it as Insert operation - throws error if query string is non empty.
+                    RequestValidator.ValidatePrimaryKeyRouteAndQueryStringInURL(Config.Operation.Insert, queryString);
+                    JsonElement requestPayloadRoot = RequestValidator.ValidateAndParseRequestBody(requestBody);
                     context = new StoredProcedureRequestContext(
                         entityName,
                         dbo: dbObject,