fix(vapt): prevent users from requesting themselves to be reviewers (#1467)

* fix(review): prevent users from specifying themselves as reivewers

* test(review): add test toe ensure users cannot specify themselves for review request

* chore: remove extra `.list.mock` call

Author: seaerchin

src/routes/v2/authenticated/__tests__/review.spec.ts

@@ -13,6 +13,7 @@ import { mockUserId } from "@fixtures/identity"
 import { MOCK_USER_EMAIL_ONE, MOCK_USER_EMAIL_TWO } from "@fixtures/users"
 import { CollaboratorRoles, ReviewRequestStatus } from "@root/constants"
 import MissingSiteError from "@root/errors/MissingSiteError"
+import { MOCK_USER_SESSION_DATA_ONE } from "@root/fixtures/sessionData"
 import GitHubService from "@root/services/db/GitHubService"
 import CollaboratorsService from "@services/identity/CollaboratorsService"
 import NotificationsService from "@services/identity/NotificationsService"
@@ -335,6 +336,34 @@ describe("Review Requests Router", () => {
       ).not.toHaveBeenCalled()
       expect(mockNotificationsService.create).not.toHaveBeenCalled()
     })
+
+    it("should return 400 if the user requests themselves to review", async () => {
+      // Arrange
+      mockCollaboratorsService.getRole.mockResolvedValueOnce("role")
+      mockIdentityUsersService.findByEmail.mockResolvedValueOnce("user")
+
+      // Act
+      const response = await request(app)
+        .post("/mockSite/review/request")
+        .send({
+          // NOTE: this is injected via `generateRouterForDefaultUserWithSite`
+          // and from there, `attachDefaultUserSessionDataWithSite`
+          reviewers: [MOCK_USER_SESSION_DATA_ONE.email],
+          title: "mockTitle",
+          description: "mockDescription",
+        })
+
+      // Assert
+      expect(response.status).toEqual(400)
+      expect(mockSitesService.getBySiteName).toHaveBeenCalledTimes(1)
+      expect(mockCollaboratorsService.getRole).toHaveBeenCalledTimes(1)
+      expect(mockIdentityUsersService.findByEmail).toHaveBeenCalledTimes(1)
+      expect(mockCollaboratorsService.list).not.toHaveBeenCalled()
+      expect(
+        mockReviewRequestService.createReviewRequest
+      ).not.toHaveBeenCalled()
+      expect(mockNotificationsService.create).not.toHaveBeenCalled()
+    })
   })
 
   describe("listReviews", () => {

src/routes/v2/authenticated/review.ts

@@ -210,6 +210,26 @@ export class ReviewsRouter {
     )
     const { reviewers, title, description } = req.body
 
+    // NOTE: reject requests that asks for the user to be the reviewer
+    // rather than accept and filter later.
+    // We should be stricter because this is prohibited on frontend
+    // so this means that the user is trying to do something they know is forbidden
+    if (reviewers.includes(userWithSiteSessionData.email)) {
+      logger.error({
+        message: `User ${userWithSiteSessionData.email} attempted to request a review from themselves for site ${siteName}`,
+        method: "createReviewRequest",
+        meta: {
+          userId: userWithSiteSessionData.isomerUserId,
+          email: userWithSiteSessionData.email,
+          siteName,
+        },
+      })
+      return res.status(400).send({
+        message:
+          "Please ensure that you are not requesting a review from yourself!",
+      })
+    }
+
     // Step 3: Check if reviewers are admins of repo
     // Check if number of requested reviewers > 0
     if (reviewers.length === 0) {