[RND-596] Require ID in body of PUT requests (#275)

* Update functions to include id property

* Add id to Json to validate document on Update

* Update test parameters

* Update MultipleEdorgTypesResourceAssociations.test.ts

Fix getAccessToken user.

* Add tests to validate id on Put

* Update ResourcesCRUDValidation.test.ts

Revert role change

* Fix  put test to include id

* Update SecurityValidation.test.ts

Fix role used to get token

* Update SecurityValidation.test.ts

Meadowlark-js/packages/meadowlark-core/src/handler/FrontendFacade.ts

@@ -9,7 +9,7 @@ import { MiddlewareModel } from '../middleware/MiddlewareModel';
 import { parsePath } from '../middleware/ParsePathMiddleware';
 import { parseBody } from '../middleware/ParseBodyMiddleware';
 import { resourceValidation } from '../middleware/ValidateResourceMiddleware';
-import { documentValidation } from '../middleware/ValidateDocumentMiddleware';
+import { documentValidation, documentUpdateValidation } from '../middleware/ValidateDocumentMiddleware';
 import { FrontendRequest } from './FrontendRequest';
 import { FrontendResponse } from './FrontendResponse';
 import * as Upsert from './Upsert';
@@ -65,7 +65,7 @@ function putStack(): MiddlewareStack {
       R.andThen(logRequestBody),
       R.andThen(resourceValidation),
       R.andThen(metaEdModelFinding),
-      R.andThen(documentValidation),
+      R.andThen(documentUpdateValidation),
       R.andThen(documentInfoExtraction),
       R.andThen(getDocumentStore().securityMiddleware),
       R.andThen(logTheResponse),

Meadowlark-js/packages/meadowlark-core/src/handler/Update.ts

@@ -15,6 +15,7 @@ import { FrontendRequest } from './FrontendRequest';
 import { FrontendResponse } from './FrontendResponse';
 import { blockingDocumentsToUris } from './UriBuilder';
 import { TraceId } from '../model/BrandedTypes';
+import { documentUuidForDocumentBody } from '../model/DocumentInfo';
 
 const moduleName = 'core.handler.Update';
 
@@ -33,7 +34,21 @@ export async function update(frontendRequest: FrontendRequest): Promise<Frontend
       writeDebugStatusToLog(moduleName, frontendRequest, 'update', 400);
       return { statusCode: 400, headers: headerMetadata };
     }
+    const documentUuidFromBody = documentUuidForDocumentBody(parsedBody);
+    if (documentUuidFromBody !== pathComponents.documentUuid) {
+      const failureMessage = 'The identity of the resource does not match the identity in the updated document.';
+      writeDebugStatusToLog(moduleName, frontendRequest, 'update', 400, failureMessage);
 
+      return {
+        body: {
+          error: {
+            message: failureMessage,
+          },
+        },
+        statusCode: 400,
+        headers: headerMetadata,
+      };
+    }
     const request: UpdateRequest = {
       meadowlarkId: meadowlarkIdForDocumentIdentity(resourceInfo, documentInfo.documentIdentity),
       documentUuid: pathComponents.documentUuid,

Meadowlark-js/packages/meadowlark-core/src/middleware/ValidateDocumentMiddleware.ts

@@ -6,6 +6,7 @@
 import { validateDocument } from '../validation/DocumentValidator';
 import { writeDebugObject, writeRequestToLog } from '../Logger';
 import { MiddlewareModel } from './MiddlewareModel';
+import { FrontendRequest } from '../handler/FrontendRequest';
 
 const moduleName = 'core.middleware.ValidateDocumentMiddleware';
 
@@ -32,3 +33,42 @@ export async function documentValidation({ frontendRequest, frontendResponse }:
 
   return { frontendRequest, frontendResponse: null };
 }
+
+/**
+ * Validates JSON document shape for the update function
+ */
+export async function documentUpdateValidation({
+  frontendRequest,
+  frontendResponse,
+}: MiddlewareModel): Promise<MiddlewareModel> {
+  const id = {
+    type: 'string',
+    description: 'The item id',
+  };
+  // Add id to schema to validate document
+  const frontendRequestUpdate: FrontendRequest = {
+    ...frontendRequest,
+    middleware: {
+      ...frontendRequest.middleware,
+      matchingMetaEdModel: {
+        ...frontendRequest.middleware.matchingMetaEdModel,
+        data: {
+          ...frontendRequest.middleware.matchingMetaEdModel.data,
+          edfiApiSchema: {
+            ...frontendRequest.middleware.matchingMetaEdModel.data.edfiApiSchema,
+            jsonSchema: {
+              ...frontendRequest.middleware.matchingMetaEdModel.data.edfiApiSchema.jsonSchema,
+              properties: {
+                id,
+                ...frontendRequest.middleware.matchingMetaEdModel.data.edfiApiSchema.jsonSchema.properties,
+              },
+              required: ['id', ...frontendRequest.middleware.matchingMetaEdModel.data.edfiApiSchema.jsonSchema.required],
+            },
+          },
+        },
+      },
+    },
+  };
+
+  return documentValidation({ frontendRequest: frontendRequestUpdate, frontendResponse });
+}

Meadowlark-js/packages/meadowlark-core/src/model/DocumentInfo.ts

@@ -51,3 +51,10 @@ export function newDocumentInfo(): DocumentInfo {
 export const NoDocumentInfo = Object.freeze({
   ...newDocumentInfo(),
 });
+/**
+ * Returns the id from the body.
+ */
+export function documentUuidForDocumentBody(parsedBody: object): string {
+  const jsonFromBody = JSON.parse(JSON.stringify(parsedBody));
+  return jsonFromBody.id;
+}

Meadowlark-js/tests/e2e/endpoints/educationContents.test.ts

@@ -86,10 +86,15 @@ describe('Education contents', () => {
 
     it('should edit an education content', async () => {
       const uri = 'uri://ed-fi.org/fake-updated-uri';
+      const id = await rootURLRequest()
+        .get(educationContentLocation)
+        .auth(await getAccessToken('vendor'), { type: 'bearer' })
+        .then((response) => response.body.id);
       await rootURLRequest()
         .put(educationContentLocation)
         .auth(await getAccessToken('vendor'), { type: 'bearer' })
         .send({
+          id,
           contentIdentifier,
           namespace: '43210',
           shortDescription: 'ShortDesc',

Meadowlark-js/tests/e2e/scenarios/ClientCredentialManagementValidation.test.ts

@@ -58,10 +58,15 @@ describe("given it's handling the client permission", () => {
     });
 
     it('returns 403 when updating by vendor', async () => {
+      const id = await rootURLRequest()
+        .get(countryLocation)
+        .auth(vendorToken, { type: 'bearer' })
+        .then((response) => response.body.id);
       await rootURLRequest()
         .put(countryLocation)
         .auth(vendorToken, { type: 'bearer' })
         .send({
+          id,
           codeValue: 'US',
           shortDescription: 'US',
           description: 'USA',

Meadowlark-js/tests/e2e/scenarios/DescriptorReferenceValidation.test.ts

@@ -89,10 +89,15 @@ describe('When creating a resource that has a reference to a descriptor', () =>
         });
 
         it('should allow to edit', async () => {
+          const id = await rootURLRequest()
+            .get(studentLocation)
+            .auth(await getAccessToken('vendor'), { type: 'bearer' })
+            .then((response) => response.body.id);
           await rootURLRequest()
             .put(studentLocation)
             .auth(await getAccessToken('vendor'), { type: 'bearer' })
             .send({
+              id,
               studentUniqueId,
               firstName: 'First',
               lastSurname: 'Last',

Meadowlark-js/tests/e2e/scenarios/MultipleEdorgTypesResourceAssociations.test.ts

@@ -144,10 +144,15 @@ describe('Given the existence of a student, a school, a local education agency a
     });
 
     it('returns error', async () => {
+      const id = await rootURLRequest()
+        .get(studentProgramAssociationLocation)
+        .auth(await getAccessToken('host'), { type: 'bearer' })
+        .then((response) => response.body.id);
       await rootURLRequest()
         .put(studentProgramAssociationLocation)
         .auth(await getAccessToken('host'), { type: 'bearer' })
         .send({
+          id,
           educationOrganizationReference: {
             educationOrganizationId: localEducationAgencyId,
           },

Meadowlark-js/tests/e2e/scenarios/ResourcesCRUDValidation.test.ts

@@ -134,13 +134,61 @@ describe('when performing crud operations', () => {
 
   describe('when updating a resource', () => {
     it('returns 204', async () => {
+      const id = await rootURLRequest()
+        .get(resourceResponse.headers.location)
+        .auth(await getAccessToken('vendor'), { type: 'bearer' })
+        .then((response) => response.body.id);
+
       await rootURLRequest()
         .put(resourceResponse.headers.location)
         .auth(await getAccessToken('host'), { type: 'bearer' })
-        .send(resourceBody)
+        .send({
+          id,
+          ...resourceBody,
+        })
         .expect(204);
     });
 
+    it('should fail when resource ID is different in body on put', async () => {
+      const id = 'differentId';
+      await rootURLRequest()
+        .put(resourceResponse.headers.location)
+        .auth(await getAccessToken('host'), { type: 'bearer' })
+        .send({
+          id,
+          ...resourceBody,
+        })
+        .expect(400)
+        .then((response) => {
+          expect(response.body.error).toMatchInlineSnapshot(`
+            {
+              "message": "The identity of the resource does not match the identity in the updated document.",
+            }
+          `);
+        });
+    });
+
+    it('should fail when resource ID is not included in body on put', async () => {
+      await rootURLRequest()
+        .put(resourceResponse.headers.location)
+        .auth(await getAccessToken('host'), { type: 'bearer' })
+        .send(resourceBody)
+        .expect(400)
+        .then((response) => {
+          expect(response.body.error).toMatchInlineSnapshot(`
+            [
+              {
+                "context": {
+                  "errorType": "required",
+                },
+                "message": "{requestBody} must have required property 'id'",
+                "path": "{requestBody}",
+              },
+            ]
+          `);
+        });
+    });
+
     it('returns updated resource on get', async () => {
       await rootURLRequest()
         .get(resourceResponse.headers.location)
@@ -150,14 +198,13 @@ describe('when performing crud operations', () => {
         });
     });
 
-    // This must be updated once RND-596 is implemented
-    it('should fail when resource ID is included in body', async () => {
+    it('should fail when resource ID is included in body on post', async () => {
       const id = await rootURLRequest()
         .get(resourceResponse.headers.location)
-        .auth(await getAccessToken('vendor'), { type: 'bearer' })
+        .auth(await getAccessToken('host'), { type: 'bearer' })
         .then((response) => response.body.id);
       await baseURLRequest()
-        .put(`${resourceEndpoint}/${id}`)
+        .post(`${resourceEndpoint}`)
         .auth(await getAccessToken('host'), { type: 'bearer' })
         .send({
           id,

Meadowlark-js/tests/e2e/scenarios/SecurityValidation.test.ts

@@ -220,10 +220,17 @@ describe('given the existence of two vendors and one host', () => {
 
     describe('when the same vendor updates the resource', () => {
       it('should return success', async () => {
+        const id = await rootURLRequest()
+          .get(resourceLocation)
+          .auth(vendor1DataAccessToken, { type: 'bearer' })
+          .then((response) => response.body.id);
         await rootURLRequest()
           .put(resourceLocation)
           .auth(vendor1DataAccessToken, { type: 'bearer' })
-          .send(resourceBodyPutUpdated)
+          .send({
+            id,
+            ...resourceBodyPutUpdated,
+          })
           .expect(204);
       });
     });
@@ -243,10 +250,18 @@ describe('given the existence of two vendors and one host', () => {
 
     describe('when a different vendor updates the resource', () => {
       it('should return error', async () => {
+        // Get resource id to update
+        const id = await rootURLRequest()
+          .get(resourceLocation)
+          .auth(vendor1DataAccessToken, { type: 'bearer' })
+          .then((response) => response.body.id);
         await rootURLRequest()
           .put(resourceLocation)
           .auth(vendor2DataAccessToken, { type: 'bearer' })
-          .send(resourceBodyPutUpdated)
+          .send({
+            id,
+            ...resourceBodyPutUpdated,
+          })
           .expect(403)
           .then((response) => {
             expect(response.body).toBe('');
@@ -256,10 +271,17 @@ describe('given the existence of two vendors and one host', () => {
 
     describe('when a host account requests the resource', () => {
       it('should return success', async () => {
+        const id = await rootURLRequest()
+          .get(resourceLocation)
+          .auth(host1DataAccessToken, { type: 'bearer' })
+          .then((response) => response.body.id);
         await rootURLRequest()
           .put(resourceLocation)
           .auth(host1DataAccessToken, { type: 'bearer' })
-          .send(resourceBodyPutUpdated)
+          .send({
+            id,
+            ...resourceBodyPutUpdated,
+          })
           .expect(204);
       });
     });