Test project validator

utopia-remix/app/handlers/validators.spec.ts

@@ -0,0 +1,160 @@
+import { prisma } from '../db.server'
+import {
+  createTestProject,
+  createTestProjectAccess,
+  createTestSession,
+  createTestUser,
+  newTestRequest,
+  truncateTables,
+} from '../test-util'
+import { validateProjectAccess } from './validators'
+import * as permissionsService from '../services/permissionsService.server'
+import { AccessLevel, UserProjectPermission } from '../types'
+import { ApiError } from '../util/errors'
+import type { ValidationResult } from '../util/api.server'
+import { Status } from '../util/statusCodes'
+
+describe('validators', () => {
+  describe('validateProjectAccess', () => {
+    afterEach(async () => {
+      await truncateTables([
+        prisma.projectID,
+        prisma.projectAccess,
+        prisma.project,
+        prisma.userDetails,
+        prisma.persistentSession,
+      ])
+
+      jest.spyOn(permissionsService, 'hasUserProjectPermission').mockRestore()
+    })
+
+    beforeEach(async () => {
+      await createTestUser(prisma, { id: 'bob' })
+      await createTestUser(prisma, { id: 'alice' })
+      await createTestSession(prisma, { key: 'bob-key', userId: 'bob' })
+      await createTestSession(prisma, { key: 'alice-key', userId: 'alice' })
+      await createTestProject(prisma, { id: 'one', ownerId: 'bob' })
+    })
+
+    const perm = UserProjectPermission.CAN_COMMENT_PROJECT // just any of the permissions is fine
+
+    afterAll(async () => {
+      jest.restoreAllMocks()
+    })
+
+    it('errors if the project id is not passed', async () => {
+      const got = await validateProjectAccess(perm, { getProjectId: () => null })(
+        newTestRequest(),
+        {},
+      )
+
+      const error = mustBeApiErrorValidator(got)
+      expect(error.status).toBe(Status.BAD_REQUEST)
+    })
+
+    it('errors if the project does not exist', async () => {
+      const got = await validateProjectAccess(perm, { getProjectId: () => 'unknown' })(
+        newTestRequest(),
+        {},
+      )
+
+      const error = mustBeApiErrorValidator(got)
+      expect(error.status).toBe(Status.NOT_FOUND)
+    })
+
+    it('does nothing if the user is the owner', async () => {
+      await createTestProjectAccess(prisma, {
+        projectId: 'one',
+        accessLevel: AccessLevel.PRIVATE,
+      })
+      const got = await validateProjectAccess(perm, { getProjectId: () => 'one' })(
+        newTestRequest({ authCookie: 'bob-key' }),
+        {},
+      )
+      expect(got.ok).toBe(true)
+    })
+
+    it('does nothing if the user has access', async () => {
+      jest.spyOn(permissionsService, 'hasUserProjectPermission').mockResolvedValue(true)
+
+      const got = await validateProjectAccess(perm, { getProjectId: () => 'one' })(
+        newTestRequest({ authCookie: 'alice-key' }),
+        {},
+      )
+      expect(got.ok).toBe(true)
+    })
+
+    describe('when access can be requested', () => {
+      it('returns a 404 if the user cannot request access', async () => {
+        jest.spyOn(permissionsService, 'hasUserProjectPermission').mockResolvedValue(false)
+
+        const got = await validateProjectAccess(perm, {
+          getProjectId: () => 'one',
+          canRequestAccess: true,
+        })(newTestRequest({ authCookie: 'alice-key' }), {})
+
+        const error = mustBeApiErrorValidator(got)
+        expect(error.status).toBe(Status.NOT_FOUND)
+      })
+
+      it('returns a 403 if the user can request access', async () => {
+        jest
+          .spyOn(permissionsService, 'hasUserProjectPermission')
+          .mockImplementation(async (_, __, p) => {
+            return p === UserProjectPermission.CAN_REQUEST_ACCESS
+          })
+
+        const got = await validateProjectAccess(perm, {
+          getProjectId: () => 'one',
+          canRequestAccess: true,
+        })(newTestRequest({ authCookie: 'alice-key' }), {})
+
+        const error = mustBeApiErrorValidator(got)
+        expect(error.status).toBe(Status.FORBIDDEN)
+      })
+    })
+
+    describe('when access cannot be requested', () => {
+      it('returns a 404 even if the user can request access', async () => {
+        jest
+          .spyOn(permissionsService, 'hasUserProjectPermission')
+          .mockImplementation(async (_, __, p) => {
+            return p === UserProjectPermission.CAN_REQUEST_ACCESS
+          })
+
+        const got = await validateProjectAccess(perm, {
+          getProjectId: () => 'one',
+        })(newTestRequest({ authCookie: 'alice-key' }), {})
+
+        const error = mustBeApiErrorValidator(got)
+        expect(error.status).toBe(Status.NOT_FOUND)
+      })
+
+      it('returns a 404', async () => {
+        await createTestProjectAccess(prisma, {
+          projectId: 'one',
+          accessLevel: AccessLevel.COLLABORATIVE,
+        })
+
+        jest.spyOn(permissionsService, 'hasUserProjectPermission').mockResolvedValue(false)
+
+        const got = await validateProjectAccess(perm, {
+          getProjectId: () => 'one',
+        })(newTestRequest({ authCookie: 'alice-key' }), {})
+
+        const error = mustBeApiErrorValidator(got)
+        expect(error.status).toBe(Status.NOT_FOUND)
+      })
+    })
+  })
+})
+
+function mustBeApiErrorValidator(got: ValidationResult): ApiError {
+  if (got.ok) {
+    throw new Error('expected validation error')
+  }
+  if (!(got.error instanceof ApiError)) {
+    throw new Error('expected api error')
+  }
+  return got.error
+}

utopia-remix/app/handlers/validators.ts

@@ -1,16 +1,15 @@
-import type { AccessValidator } from '../util/api.server'
-import { ensure, getUser } from '../util/api.server'
+import type { AccessValidator, ValidationResult } from '../util/api.server'
+import { getUser, validationError, validationOk } from '../util/api.server'
 import { UserProjectPermission } from '../types'
 import type { Params } from '@remix-run/react'
 import { Status } from '../util/statusCodes'
 import { hasUserProjectPermission } from '../services/permissionsService.server'
 import { getProjectOwnerById } from '../models/project.server'
+import { ApiError } from '../util/errors'
 
 export function validateProjectAccess(
   permission: UserProjectPermission,
   {
-    errorMessage,
-    status,
     getProjectId,
     includeDeleted = false,
     canRequestAccess = false,
@@ -22,33 +21,48 @@ export function validateProjectAccess(
     canRequestAccess?: boolean
   },
 ): AccessValidator {
-  return async function (req: Request, params: Params<string>) {
+  return async function (req: Request, params: Params<string>): Promise<ValidationResult> {
     const projectId = getProjectId(params)
-    ensure(projectId != null, 'project id is null', Status.BAD_REQUEST)
+    if (projectId == null) {
+      return validationError(new ApiError('Invalid project id', Status.BAD_REQUEST))
+    }
 
     const ownerId = await getProjectOwnerById({ id: projectId }, { includeDeleted: includeDeleted })
-    ensure(ownerId != null, `Project ${projectId} not found or has no owner`, Status.NOT_FOUND)
+    if (ownerId == null) {
+      return validationError(new ApiError('Project not found', Status.NOT_FOUND))
+    }
 
     const user = await getUser(req)
     const userId = user?.user_id ?? null
-    const isCreator = userId ? ownerId === userId : false
 
-    const allowed = isCreator || (await hasUserProjectPermission(projectId, userId, permission))
-    let errorMessageToUse = errorMessage ?? 'Unauthorized Access'
-    let statusToUse = status ?? Status.FORBIDDEN
-    if (!allowed && canRequestAccess === true) {
+    // the user owns the project, go on
+    if (userId === ownerId) {
+      return validationOk()
+    }
+
+    // if the user can access the project, go on
+    const hasProjectPermissions = await hasUserProjectPermission(projectId, userId, permission)
+    if (hasProjectPermissions) {
+      return validationOk()
+    }
+
+    // if access can be requested…
+    if (canRequestAccess === true) {
+      // …and the user can request permissions…
       const hasRequestAccessPermission = await hasUserProjectPermission(
         projectId,
         userId,
         UserProjectPermission.CAN_REQUEST_ACCESS,
       )
       if (hasRequestAccessPermission) {
-        errorMessageToUse = 'Request access to this project'
-        statusToUse = Status.FORBIDDEN
+        // …return a 403!
+        return validationError(new ApiError('Forbidden', Status.FORBIDDEN))
       }
     }
-    ensure(allowed, errorMessageToUse, statusToUse)
+
+    // the project is not available, it's conceptually a 401 but just return a 404 so we don't leak
+    return validationError(new ApiError('Project not found', Status.NOT_FOUND))
   }
 }
 
-export const ALLOW: AccessValidator = async (request: Request, params: Params<string>) => true
+export const ALLOW: AccessValidator = async () => validationOk()

utopia-remix/app/routes-test/internal.projects.$id.access.spec.ts

@@ -93,8 +93,8 @@ describe('handleChangeAccess', () => {
     })
     const error = await getActionResult('two', AccessLevel.PRIVATE)
     expect(error).toEqual({
-      message: 'Unauthorized Access',
-      status: Status.FORBIDDEN,
+      message: 'Project not found',
+      status: Status.NOT_FOUND,
       error: 'Error',
     })
   })

utopia-remix/app/routes/v1.project.$id.tsx

@@ -1,7 +1,6 @@
 import type { ActionFunctionArgs, LoaderFunctionArgs } from '@remix-run/node'
 import { proxy } from '../util/proxy.server'
 import { handle, handleOptions } from '../util/api.server'
-import { Status } from '../util/statusCodes'
 import { UserProjectPermission } from '../types'
 import { ALLOW, validateProjectAccess } from '../handlers/validators'
 
@@ -11,8 +10,6 @@ export async function loader(args: LoaderFunctionArgs) {
     GET: {
       handler: proxy,
       validator: validateProjectAccess(UserProjectPermission.CAN_VIEW_PROJECT, {
-        errorMessage: 'Project not found',
-        status: Status.NOT_FOUND,
         canRequestAccess: true,
         getProjectId: (params) => params.id,
       }),

utopia-remix/app/util/api.server.spec.ts

@@ -1,7 +1,7 @@
 import { prisma } from '../db.server'
 import { createTestSession, createTestUser, newTestRequest, truncateTables } from '../test-util'
 import type { ApiResponse } from './api.server'
-import { handle, requireUser } from './api.server'
+import { handle, requireUser, validationOk } from './api.server'
 import { ApiError } from './errors'
 import { Status } from './statusCodes'
 
@@ -66,7 +66,7 @@ describe('handle', () => {
             return { data: 'success' }
           },
           validator: async () => {
-            // passes
+            return validationOk()
           },
         },
       },

utopia-remix/app/util/api.server.ts

@@ -49,7 +49,22 @@ interface HandleRequest {
   params: Params<string>
 }
 
-export type ProjectAccessValidator = (request: Request, params: Params<string>) => Promise<unknown>
+type ValidationResultOk = { ok: true }
+type ValidationResultError = { ok: false; error: Error }
+export type ValidationResult = ValidationResultOk | ValidationResultError
+
+export function validationOk(): ValidationResult {
+  return { ok: true }
+}
+
+export function validationError(error: Error): ValidationResult {
+  return { ok: false, error: error }
+}
+
+export type ProjectAccessValidator = (
+  request: Request,
+  params: Params<string>,
+) => Promise<ValidationResult>
 
 export type AccessValidator = ProjectAccessValidator
 
@@ -80,7 +95,10 @@ async function handleMethod<T>(
 ): Promise<ApiResponse<T> | unknown> {
   try {
     if (validator != null) {
-      await validator(request, params)
+      const result = await validator(request, params)
+      if (!result.ok) {
+        throw result.error
+      }
     }
 
     const resp = await fn(request, params)