Separate file actions to their own authorisation function

backend/src/connectors/v2/authorisation/Base.ts

@@ -11,8 +11,6 @@ export const ModelAction = {
   Create: 'create',
   View: 'view',
   Update: 'update',
-  DeleteFile: 'delete_file',
-  UploadFile: 'upload_file',
   Write: 'write',
 } as const
 export type ModelActionKeys = (typeof ModelAction)[keyof typeof ModelAction]
@@ -38,6 +36,10 @@ export const SchemaAction = {
 export type SchemaActionKeys = (typeof SchemaAction)[keyof typeof SchemaAction]
 
 export const FileAction = {
+  Delete: 'delete',
+  Upload: 'upload',
+  // 'view' refers to the ability to see metadata about the file.  'download' lets the user view the file contents.
+  View: 'view',
   Download: 'download',
 }
 export type FileActionKeys = (typeof FileAction)[keyof typeof FileAction]

backend/src/services/v2/file.ts

@@ -1,8 +1,9 @@
 import { getObjectStream, putObjectStream } from '../../clients/s3.js'
-import { FileAction, ModelAction } from '../../connectors/v2/authorisation/Base.js'
+import { FileAction } from '../../connectors/v2/authorisation/Base.js'
 import authorisation from '../../connectors/v2/authorisation/index.js'
 import FileModel from '../../models/v2/File.js'
 import { UserDoc } from '../../models/v2/User.js'
+import { asyncFilter } from '../../utils/general.js'
 import config from '../../utils/v2/config.js'
 import { Forbidden, NotFound } from '../../utils/v2/error.js'
 import { longId } from '../../utils/v2/id.js'
@@ -11,27 +12,27 @@ import { getModelById } from './model.js'
 export async function uploadFile(user: UserDoc, modelId: string, name: string, mime: string, stream: ReadableStream) {
   const model = await getModelById(user, modelId)
 
-  if (!(await authorisation.userModelAction(user, model, ModelAction.UploadFile))) {
-    throw Forbidden(`You do not have permission to upload a file to this model.`, { userDn: user.dn })
-  }
-
   const fileId = longId()
 
   const bucket = config.s3.buckets.uploads
   const path = `beta/model/${modelId}/files/${fileId}`
 
-  const { fileSize } = await putObjectStream(bucket, path, stream)
-
   const file = new FileModel({
     modelId,
     name,
     mime,
     bucket,
     path,
-    size: fileSize,
     complete: true,
   })
 
+  if (!(await authorisation.userFileAction(user, model, file, FileAction.Upload))) {
+    throw Forbidden(`You do not have permission to upload a file to this model.`, { userDn: user.dn })
+  }
+
+  const { fileSize } = await putObjectStream(bucket, path, stream)
+  file.size = fileSize
+
   await file.save()
 
   return file
@@ -60,29 +61,28 @@ export async function getFileById(user: UserDoc, fileId: string) {
 
   const model = await getModelById(user, file.modelId)
 
-  if (!(await authorisation.userModelAction(user, model, ModelAction.View))) {
+  if (!(await authorisation.userFileAction(user, model, file, FileAction.View))) {
     throw Forbidden(`You do not have permission to get this file.`, { userDn: user.dn, fileId })
   }
 
   return file
 }
 
 export async function getFilesByModel(user: UserDoc, modelId: string) {
-  await getModelById(user, modelId)
-
+  const model = await getModelById(user, modelId)
   const files = await FileModel.find({ modelId })
-  return files
+
+  return asyncFilter(files, (file) => authorisation.userFileAction(user, model, file, FileAction.View))
 }
 
 export async function removeFile(user: UserDoc, modelId: string, fileId: string) {
   const model = await getModelById(user, modelId)
+  const file = await getFileById(user, fileId)
 
-  if (!(await authorisation.userModelAction(user, model, ModelAction.DeleteFile))) {
+  if (!(await authorisation.userFileAction(user, model, file, FileAction.Delete))) {
     throw Forbidden(`You do not have permission to delete a file from this model.`, { userDn: user.dn })
   }
 
-  const file = await getFileById(user, fileId)
-
   // We don't actually remove the file from storage, we only hide all
   // references to it.  This makes the file not visible to the user.
   await file.delete()

backend/test/services/__snapshots__/file.spec.ts.snap

@@ -1,26 +1,11 @@
 // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
 
 exports[`services > file > getFilesByModel > success 1`] = `
-{
-  "delete": [MockFunction spy],
-  "find": [MockFunction spy] {
-    "calls": [
-      [
-        {
-          "modelId": "testModelId",
-        },
-      ],
-    ],
-    "results": [
-      {
-        "type": "return",
-        "value": [Circular],
-      },
-    ],
+[
+  {
+    "example": "file",
   },
-  "findOne": [MockFunction spy],
-  "save": [MockFunction spy],
-}
+]
 `;
 
 exports[`services > file > removeFile > success 1`] = `
@@ -53,5 +38,6 @@ exports[`services > file > removeFile > success 1`] = `
     ],
   },
   "save": [MockFunction spy],
+  "size": 100,
 }
 `;

backend/test/services/file.spec.ts

@@ -1,6 +1,7 @@
 import { Readable } from 'stream'
 import { describe, expect, test, vi } from 'vitest'
 
+import { FileAction } from '../../src/connectors/v2/authorisation/Base.js'
 import { UserDoc } from '../../src/models/v2/User.js'
 import { downloadFile, getFilesByModel, removeFile, uploadFile } from '../../src/services/v2/file.js'
 
@@ -14,7 +15,7 @@ vi.mock('../../src/clients/s3.js', () => s3Mocks)
 
 const authorisationMocks = vi.hoisted(() => ({
   userModelAction: vi.fn(() => true),
-  userFileAction: vi.fn(() => true),
+  userFileAction: vi.fn((_user, _model, _file, _action) => true),
 }))
 vi.mock('../../src/connectors/v2/authorisation/index.js', async () => ({
   default: authorisationMocks,
@@ -57,7 +58,7 @@ describe('services > file', () => {
   })
 
   test('uploadFile > no permission', async () => {
-    authorisationMocks.userModelAction.mockResolvedValueOnce(false)
+    authorisationMocks.userFileAction.mockResolvedValueOnce(false)
 
     expect(() => uploadFile({} as any, 'modelId', 'name', 'mime', new Readable() as any)).rejects.toThrowError(
       /^You do not have permission to upload a file to this model./,
@@ -75,7 +76,13 @@ describe('services > file', () => {
   })
 
   test('removeFile > no permission', async () => {
-    authorisationMocks.userModelAction.mockResolvedValueOnce(false)
+    authorisationMocks.userModelAction.mockResolvedValueOnce(true)
+    authorisationMocks.userFileAction.mockImplementation((_user, _model, _file, action) => {
+      if (action === FileAction.View) return true
+      if (action === FileAction.Delete) return false
+
+      return false
+    })
 
     const user = { dn: 'testUser' } as UserDoc
     const modelId = 'testModelId'
@@ -89,6 +96,8 @@ describe('services > file', () => {
   })
 
   test('getFilesByModel > success', async () => {
+    fileModelMocks.find.mockResolvedValueOnce([{ example: 'file' }])
+
     const user = { dn: 'testUser' } as UserDoc
     const modelId = 'testModelId'
 
@@ -97,14 +106,16 @@ describe('services > file', () => {
     expect(result).toMatchSnapshot()
   })
 
-  // test('getFilesByModel > no permission', async () => {
-  //   authorisationMocks.userModelAction.mockResolvedValueOnce(false)
+  test('getFilesByModel > no permission', async () => {
+    authorisationMocks.userFileAction.mockResolvedValue(false)
+    fileModelMocks.find.mockResolvedValueOnce([{ example: 'file' }])
 
-  //   const user = { dn: 'testUser' } as UserDoc
-  //   const modelId = 'testModelId'
+    const user = { dn: 'testUser' } as UserDoc
+    const modelId = 'testModelId'
 
-  //   expect(() => getFilesByModel(user, modelId)).rejects.toThrowError(/^You do not have permission to get these files./)
-  // })
+    const files = await getFilesByModel(user, modelId)
+    expect(files).toStrictEqual([])
+  })
 
   test('downloadFile > success', async () => {
     const user = { dn: 'testUser' } as UserDoc
@@ -121,8 +132,12 @@ describe('services > file', () => {
     const fileId = 'testFileId'
     const range = { start: 0, end: 50 }
 
-    authorisationMocks.userFileAction.mockResolvedValueOnce(false)
+    authorisationMocks.userFileAction.mockImplementation((_user, _model, _file, action) => {
+      if (action === FileAction.View) return true
+      if (action === FileAction.Download) return false
 
+      return false
+    })
     await expect(downloadFile(user, fileId, range)).rejects.toThrowError(
       /^You do not have permission to download this model./,
     )