[RND-556] Fix faulty ownership security on POST requests (#248)

* [RND-556] Faulty ownership security on POST requests

Add test to validate ownership on insert
Update ownership validation, to validate first by documentUuid, if that id is null, it tries to find the document by meadowlarkId.

* Update SecurityValidation.test.ts

Update url

Meadowlark-js/backends/meadowlark-postgresql-backend/src/repository/OwnershipSecurity.ts

@@ -3,11 +3,20 @@
 // The Ed-Fi Alliance licenses this file to you under the Apache License, Version 2.0.
 // See the LICENSE and NOTICES files in the project root for more information.
 
-import { FrontendRequest, writeRequestToLog, DocumentUuid } from '@edfi/meadowlark-core';
+import { meadowlarkIdForDocumentIdentity, FrontendRequest, writeRequestToLog, MeadowlarkId } from '@edfi/meadowlark-core';
 import { Logger } from '@edfi/meadowlark-utilities';
 import type { PoolClient, QueryResult } from 'pg';
 import { SecurityResult } from '../security/SecurityResult';
-import { findOwnershipForDocumentByDocumentUuidSql } from './SqlHelper';
+import { findOwnershipForDocumentByDocumentUuidSql, findOwnershipForDocumentSql } from './SqlHelper';
+
+function extractIdIfUpsert(frontendRequest: FrontendRequest): MeadowlarkId | undefined {
+  if (frontendRequest.action !== 'upsert') return undefined;
+
+  return meadowlarkIdForDocumentIdentity(
+    frontendRequest.middleware.resourceInfo,
+    frontendRequest.middleware.documentInfo.documentIdentity,
+  );
+}
 
 export async function rejectByOwnershipSecurity(
   frontendRequest: FrontendRequest,
@@ -26,33 +35,47 @@ export async function rejectByOwnershipSecurity(
     Logger.debug(`${functionName} GET style request for a descriptor, bypassing ownership check`, frontendRequest.traceId);
     return 'NOT_APPLICABLE';
   }
+  // First we try to use the documentUuid, because it is constant (meadowlarkId could be updated).
+  // An update sends the documentUuid. In this case, we cannot use the meadowlarkId because in some case we could
+  // change the documentUuid.
+  const { documentUuid } = frontendRequest.middleware.pathComponents;
+  let meadowlarkId: MeadowlarkId = '' as MeadowlarkId;
+  let idLogMessage: String = '';
+  // if the request is an insert, the documentUuid is empty. Then, we need the meadowlarkId to validate
+  if (documentUuid == null) {
+    meadowlarkId = extractIdIfUpsert(frontendRequest) ?? ('' as MeadowlarkId);
+    idLogMessage = `MeadowlarkId ${meadowlarkId}`;
+  } else {
+    idLogMessage = `DocumentUuid ${documentUuid}`;
+  }
 
-  const id: DocumentUuid | undefined = frontendRequest.middleware.pathComponents.documentUuid;
-
-  if (id == null) {
+  if (documentUuid == null && meadowlarkId === '') {
     Logger.error(`${functionName} no id to secure against`, frontendRequest.traceId);
     return 'NOT_APPLICABLE';
   }
 
   try {
-    const result: QueryResult = await client.query(findOwnershipForDocumentByDocumentUuidSql(id));
+    const result: QueryResult =
+      documentUuid != null
+        ? await client.query(findOwnershipForDocumentByDocumentUuidSql(documentUuid))
+        : await client.query(findOwnershipForDocumentSql(meadowlarkId));
 
     if (result.rows == null) {
       Logger.error(`${functionName} Unknown Error determining access`, frontendRequest.traceId);
       return 'UNKNOWN_FAILURE';
     }
 
     if (result.rowCount === 0) {
-      Logger.debug(`${functionName} document not found for id ${id}`, frontendRequest.traceId);
+      Logger.debug(`${functionName} document not found for ${idLogMessage}`, frontendRequest.traceId);
       return 'NOT_APPLICABLE';
     }
     const { clientId } = frontendRequest.middleware.security;
 
     if (result.rows[0].created_by === clientId) {
-      Logger.debug(`${functionName} access approved: id ${id}, clientId ${clientId}`, frontendRequest.traceId);
+      Logger.debug(`${functionName} access approved: ${idLogMessage}, clientId ${clientId}`, frontendRequest.traceId);
       return 'ACCESS_APPROVED';
     }
-    Logger.debug(`${functionName} access denied: id ${id}, clientId ${clientId}`, frontendRequest.traceId);
+    Logger.debug(`${functionName} access denied: ${idLogMessage}, clientId ${clientId}`, frontendRequest.traceId);
     return 'ACCESS_DENIED';
   } catch (e) {
     Logger.error(`${functionName} Error determining access`, frontendRequest.traceId, e);

Meadowlark-js/backends/meadowlark-postgresql-backend/src/repository/Upsert.ts

@@ -124,8 +124,7 @@ export async function upsertDocument(
 
     // Perform the document upsert
     Logger.debug(`${moduleName}.upsertDocument: Upserting document meadowlarkId ${meadowlarkId}`, traceId);
-    const insertResult = await client.query(documentUpsertSql);
-    Logger.debug(`${moduleName}.upsertDocument: insert ${insertResult}`, traceId);
+    await client.query(documentUpsertSql);
     // Delete existing values from the aliases table
     await client.query(deleteAliasesForDocumentSql(meadowlarkId));
     // Perform insert of alias ids

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

@@ -228,6 +228,19 @@ describe('given the existence of two vendors and one host', () => {
       });
     });
 
+    describe('when a different vendor tries to insert an existing resource', () => {
+      it('should return error', async () => {
+        await rootURLRequest()
+          .post(resourceLocation.slice(0, resourceLocation.lastIndexOf('/')))
+          .auth(vendor2DataAccessToken, { type: 'bearer' })
+          .send(resourceBodyPutUpdated)
+          .expect(403)
+          .then((response) => {
+            expect(response.body).toBe('');
+          });
+      });
+    });
+
     describe('when a different vendor updates the resource', () => {
       it('should return error', async () => {
         await rootURLRequest()