Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions src/routes/v2/authenticated/__tests__/review.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -335,6 +336,35 @@ 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")
mockCollaboratorsService.list.mockResolvedValueOnce([])

// 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", () => {
Expand Down
20 changes: 20 additions & 0 deletions src/routes/v2/authenticated/review.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading