userId check on patch/delete routes (#545)

Makes it so that users can only delete and modify their own listings and reviews # Description Closes #351 #352 ## How to Test Try and delete another users listing ## Checklist - [x] The code includes tests if relevant - [x] I have *actually* self-reviewed my changes and done QA
UVicMartletplace · Jul 30, 2024 · 4465363 · 4465363
1 parent 05bc5a8
commit 4465363
Show file tree

Hide file tree

Showing 9 changed files with 29 additions and 16 deletions.
diff --git a/apps/backend/listing/src/deleteListing.ts b/apps/backend/listing/src/deleteListing.ts
@@ -18,8 +18,8 @@ const deleteListing = async (
   try {
     const result = await db.result(
       `DELETE FROM listings
-       WHERE listing_id = $1`,
-      [id],
+       WHERE listing_id = $1 AND seller_id = $2;`,
+      [id, req.user.userId],
     );
 
     if (result.rowCount === 0) {

diff --git a/apps/backend/listing/src/updateListing.ts b/apps/backend/listing/src/updateListing.ts
@@ -63,8 +63,8 @@ const updateListing = async (
            location = $4,
            image_urls = $5,
            status = $6,
-           charity_id = $8
-       WHERE listing_id = $7
+           charity_id = $7
+       WHERE listing_id = $8 AND seller_id = $9
        RETURNING listing_id, seller_id, buyer_id, title, price, location, status, description, image_urls, created_at, modified_at, charity_id;`,
       [
         title,
@@ -73,8 +73,9 @@ const updateListing = async (
         formattedLocation,
         images.map((image: { url: string }) => image.url),
         status,
-        id,
         charity_id,
+        id,
+        req.user.userId,
       ],
     );
 

diff --git a/apps/backend/listing/tests/deleteListing.test.ts b/apps/backend/listing/tests/deleteListing.test.ts
@@ -2,12 +2,14 @@ import { describe, it, expect, vi } from 'vitest';
 import { deleteListing } from '../src/deleteListing';
 import { Request, Response } from 'express';
 import { IDatabase } from 'pg-promise';
+import { AuthenticatedRequest } from '../../lib/src/auth';
 
 describe('Delete Listing Endpoint', () => {
   it('should delete a listing successfully', async () => {
     const req = {
       params: { id: '3' },
-    } as unknown as Request;
+      user: { userId: 1 }
+    } as unknown as AuthenticatedRequest;
 
     const res = {
       status: vi.fn().mockReturnThis(),
@@ -27,7 +29,8 @@ describe('Delete Listing Endpoint', () => {
   it('should fail to delete a non-existent listing', async () => {
     const req = {
       params: { id: '9999' },
-    } as unknown as Request;
+      user: { userId: 1 },
+    } as unknown as AuthenticatedRequest;
 
     const res = {
       status: vi.fn().mockReturnThis(),

diff --git a/apps/backend/listing/tests/updateListing.test.ts b/apps/backend/listing/tests/updateListing.test.ts
@@ -2,11 +2,13 @@ import { describe, it, expect, vi } from 'vitest';
 import { updateListing } from '../src/updateListing';
 import { Request, Response } from 'express';
 import { IDatabase } from 'pg-promise';
+import { AuthenticatedRequest } from '../../lib/src/auth';
 
 describe('Update Listing Endpoint', () => {
   it('should update a listing successfully', async () => {
     const req = {
       params: { id: '1' },
+      user: { userId: 1 },
       body: {
         listing: {
           title: 'Updated Listing One',
@@ -24,7 +26,7 @@ describe('Update Listing Endpoint', () => {
         },
         status: 'AVAILABLE',
       },
-    } as unknown as Request;
+    } as unknown as AuthenticatedRequest;
 
     const res = {
       status: vi.fn().mockReturnThis(),
@@ -75,6 +77,7 @@ describe('Update Listing Endpoint', () => {
   it('should fail to update a non-existent listing', async () => {
     const req = {
       params: { id: '9999' },
+      user: { userId: 1 },
       body: {
         listing: {
           title: 'Updated Listing Non-existent',
@@ -92,7 +95,7 @@ describe('Update Listing Endpoint', () => {
         },
         status: 'AVAILABLE',
       },
-    } as unknown as Request;
+    } as unknown as AuthenticatedRequest;
 
     const res = {
       status: vi.fn().mockReturnThis(),
@@ -112,6 +115,7 @@ describe('Update Listing Endpoint', () => {
   it('should update a listing with markedForCharity and fetch charity id', async () => {
     const req = {
       params: { id: '1' },
+      user: { userId: 1 },
       body: {
         listing: {
           title: 'Updated Listing Two',
@@ -129,7 +133,7 @@ describe('Update Listing Endpoint', () => {
         },
         status: 'AVAILABLE',
       },
-    } as unknown as Request;
+    } as unknown as AuthenticatedRequest;
 
     const res = {
       status: vi.fn().mockReturnThis(),

diff --git a/apps/backend/review/src/deleteReview.ts b/apps/backend/review/src/deleteReview.ts
@@ -18,8 +18,8 @@ const deleteReview = async (
   try {
     const result = await db.result(
       `DELETE FROM reviews
-       WHERE review_id = $1`,
-      [id],
+       WHERE review_id = $1 AND user_id = $2;`,
+      [id, req.user.userId],
     );
 
     if (result.rowCount === 0) {

diff --git a/apps/backend/review/src/updateReview.ts b/apps/backend/review/src/updateReview.ts
@@ -22,9 +22,9 @@ const updateReview = async (
        SET review = $1,
            rating_value = $2,
             listing_id = $3
-       WHERE review_id = $4
+       WHERE review_id = $4 AND user_id = $5
        RETURNING review_id`,
-      [comment, stars, listingID, id],
+      [comment, stars, listingID, id, req.user.userId],
     );
 
     if (!updatedReview) {

diff --git a/apps/backend/review/tests/deleteReview.test.ts b/apps/backend/review/tests/deleteReview.test.ts
@@ -8,6 +8,7 @@ describe('Delete Review Endpoint', () => {
   it('should delete a review successfully', async () => {
     const req = {
       params: { id: '1' },
+      user: { userId: 1 }
     } as unknown as AuthenticatedRequest;
 
     const res = {
@@ -25,9 +26,10 @@ describe('Delete Review Endpoint', () => {
     expect(res.json).toHaveBeenCalledWith({ message: 'Review deleted successfully' });
   });
 
-  it('should return an error if review not found', async () => {
+  it('should return 200 if review not found', async () => {
     const req = {
       params: { id: '9999' },
+      user: { userId: 1 }
     } as unknown as AuthenticatedRequest;
 
     const res = {

diff --git a/apps/backend/review/tests/updateReview.test.ts b/apps/backend/review/tests/updateReview.test.ts
@@ -13,6 +13,7 @@ describe('Update Review Endpoint', () => {
         comment: 'Updated review',
         listingID: 1,
       },
+      user: { userId: 1 }
     } as unknown as AuthenticatedRequest;
 
     const res = {
@@ -46,7 +47,9 @@ describe('Update Review Endpoint', () => {
         comment: 'Updated review',
         listingID: 1,
       },
+      user: { userId: 1 }
     } as unknown as AuthenticatedRequest;
+
 
     const res = {
       status: vi.fn().mockReturnThis(),

diff --git a/apps/data/database/initdb.sql b/apps/data/database/initdb.sql
@@ -96,7 +96,7 @@ CREATE TABLE user_searches (
 CREATE TABLE user_clicks (
     click_id SERIAL PRIMARY KEY,
     user_id INTEGER NOT NULL REFERENCES users(user_id) ON DELETE CASCADE,
-    listing_id INTEGER NOT NULL REFERENCES listings(listing_id),
+    listing_id INTEGER NOT NULL REFERENCES listings(listing_id) ON DELETE CASCADE,
     listing_title VARCHAR NOT NULL,
     created_at TIMESTAMP NOT NULL DEFAULT NOW()
 );