Merge pull request #1624 from gchq/feature/BAI-1485-manual-user-access

Feature/bai 1485 manual user access

Author: ARADDCC002

Diff for: backend/config/default.cjs

@@ -201,6 +201,10 @@ module.exports = {
       text: '',
       startTimestamp: '',
     },
+
+    helpPopoverText: {
+      manualEntryAccess: '',
+    },
   },
 
   connectors: {

Diff for: backend/src/migrations/011_find_and_remove_invalid_users.ts

@@ -0,0 +1,44 @@
+import authentication from '../connectors/authentication/index.js'
+import { MigrationMetadata } from '../models/Migration.js'
+import ModelModel from '../models/Model.js'
+
+/**
+ * As we now do backend validation for users being added to model access lists, we
+ * added this script to find and remove all existing users that do not pass the
+ * "getUserInformation" call in the authentication connector. You can find a
+ * list of removed users for all affected models by looking at the "metadata"
+ * property of this migration's database object.
+ **/
+
+export async function up() {
+  const models = await ModelModel.find({})
+  const metadata: MigrationMetadata[] = []
+  for (const model of models) {
+    const invalidUsers: string[] = []
+    await Promise.all(
+      model.collaborators.map(async (collaborator) => {
+        if (collaborator.entity !== '') {
+          try {
+            await authentication.getUserInformation(collaborator.entity)
+          } catch (err) {
+            invalidUsers.push(collaborator.entity)
+          }
+        }
+      }),
+    )
+    if (invalidUsers.length > 0) {
+      const invalidUsersForModel = { modelId: model.id, invalidUsers: invalidUsers }
+      const invalidUsersRemoved = model.collaborators.filter(
+        (collaborator) => !invalidUsers.includes(collaborator.entity),
+      )
+      model.collaborators = invalidUsersRemoved
+      await model.save()
+      metadata.push(invalidUsersForModel)
+    }
+  }
+  return metadata
+}
+
+export async function down() {
+  /* NOOP */
+}

Diff for: backend/src/models/Migration.ts

@@ -1,7 +1,12 @@
 import { Document, model, Schema } from 'mongoose'
 
+export interface MigrationMetadata {
+  [key: string]: any
+}
+
 export interface Migration {
   name: string
+  metadata?: MigrationMetadata
 
   createdAt: Date
   updatedAt: Date
@@ -12,6 +17,7 @@ export type MigrationDoc = Migration & Document<any, any, Migration>
 const MigrationSchema = new Schema<Migration>(
   {
     name: { type: String, required: true },
+    metadata: { type: Schema.Types.Mixed },
   },
   {
     timestamps: true,

Diff for: backend/src/services/migration.ts

@@ -1,4 +1,4 @@
-import MigrationModel from '../models/Migration.js'
+import MigrationModel, { MigrationMetadata } from '../models/Migration.js'
 
 export async function doesMigrationExist(name: string) {
   const migration = await MigrationModel.findOne({
@@ -12,8 +12,13 @@ export async function doesMigrationExist(name: string) {
   return true
 }
 
-export async function markMigrationComplete(name: string) {
-  await MigrationModel.create({
-    name,
-  })
+export async function markMigrationComplete(name: string, metadata: MigrationMetadata | undefined) {
+  metadata
+    ? await MigrationModel.create({
+        name,
+        metadata,
+      })
+    : await MigrationModel.create({
+        name,
+      })
 }

Diff for: backend/src/services/model.ts

@@ -12,9 +12,9 @@ import ModelCardRevisionModel, {
 } from '../models/ModelCardRevision.js'
 import { UserInterface } from '../models/User.js'
 import { GetModelCardVersionOptions, GetModelCardVersionOptionsKeys, GetModelFiltersKeys } from '../types/enums.js'
-import { EntryUserPermissions } from '../types/types.js'
+import { EntityKind, EntryUserPermissions } from '../types/types.js'
 import { isValidatorResultError } from '../types/ValidatorResultError.js'
-import { toEntity } from '../utils/entity.js'
+import { fromEntity, toEntity } from '../utils/entity.js'
 import { BadReq, Forbidden, InternalError, NotFound } from '../utils/error.js'
 import { convertStringToId } from '../utils/id.js'
 import { authResponseToUserPermission } from '../utils/permissions.js'
@@ -33,6 +33,10 @@ export type CreateModelParams = Pick<
 export async function createModel(user: UserInterface, modelParams: CreateModelParams) {
   const modelId = convertStringToId(modelParams.name)
 
+  if (modelParams.collaborators) {
+    await validateCollaborators(modelParams.collaborators)
+  }
+
   let collaborators: CollaboratorEntry[] = []
   if (modelParams.collaborators && modelParams.collaborators.length > 0) {
     const collaboratorListContainsOwner = modelParams.collaborators.some((collaborator) =>
@@ -303,7 +307,7 @@ export async function updateModelCard(
   return revision
 }
 
-export type UpdateModelParams = Pick<ModelInterface, 'name' | 'description' | 'visibility'> & {
+export type UpdateModelParams = Pick<ModelInterface, 'name' | 'description' | 'visibility' | 'collaborators'> & {
   settings: Partial<ModelInterface['settings']>
 }
 export async function updateModel(user: UserInterface, modelId: string, modelDiff: Partial<UpdateModelParams>) {
@@ -317,6 +321,9 @@ export async function updateModel(user: UserInterface, modelId: string, modelDif
   if (modelDiff.settings?.mirror?.destinationModelId && modelDiff.settings?.mirror?.sourceModelId) {
     throw BadReq('You cannot select both mirror settings simultaneously.')
   }
+  if (modelDiff.collaborators) {
+    await validateCollaborators(modelDiff.collaborators, model.collaborators)
+  }
 
   const auth = await authorisation.model(user, model, ModelAction.Update)
   if (!auth.success) {
@@ -335,6 +342,49 @@ export async function updateModel(user: UserInterface, modelId: string, modelDif
   return model
 }
 
+async function validateCollaborators(
+  updatedCollaborators: CollaboratorEntry[],
+  previousCollaborators: CollaboratorEntry[] = [],
+) {
+  const previousCollaboratorEntities: string[] = previousCollaborators.map((collaborator) => collaborator.entity)
+  const duplicates = updatedCollaborators.reduce<string[]>(
+    (duplicates, currentCollaborator, currentCollaboratorIndex) => {
+      if (
+        updatedCollaborators.find(
+          (collaborator, index) =>
+            index !== currentCollaboratorIndex && collaborator.entity === currentCollaborator.entity,
+        ) &&
+        !duplicates.includes(currentCollaborator.entity)
+      ) {
+        duplicates.push(currentCollaborator.entity)
+      }
+      return duplicates
+    },
+    [],
+  )
+  if (duplicates.length > 0) {
+    throw BadReq(`The following duplicate collaborators have been found: ${duplicates.join(', ')}`)
+  }
+  const newCollaborators = updatedCollaborators.reduce<string[]>((acc, currentCollaborator) => {
+    if (!previousCollaboratorEntities.includes(currentCollaborator.entity)) {
+      acc.push(currentCollaborator.entity)
+    }
+    return acc
+  }, [])
+  await Promise.all(
+    newCollaborators.map(async (collaborator) => {
+      if (collaborator === '') {
+        throw BadReq('Collaborator name must be a valid string')
+      }
+      // TODO we currently only check for users, we should consider how we want to handle groups
+      const { kind } = fromEntity(collaborator)
+      if (kind === EntityKind.USER) {
+        await authentication.getUserInformation(collaborator)
+      }
+    }),
+  )
+}
+
 export async function createModelCardFromSchema(
   user: UserInterface,
   modelId: string,

Diff for: backend/src/types/types.ts

@@ -9,6 +9,11 @@ export const RoleKind = {
   SCHEMA: 'schema',
 } as const
 
+export enum EntityKind {
+  USER = 'user',
+  GROUP = 'group',
+}
+
 export type RoleKindKeys = (typeof RoleKind)[keyof typeof RoleKind]
 
 export interface Role {
@@ -91,4 +96,8 @@ export interface UiConfig {
     text: string
     startTimestamp: string
   }
+
+  helpPopoverText: {
+    manualEntryAccess: string
+  }
 }

Diff for: backend/src/utils/database.ts

@@ -57,9 +57,9 @@ export async function runMigrations() {
 
       // run migration
       const migration = await import(join(base, file))
-      await migration.up()
+      const runMigration = await migration.up()
 
-      await markMigrationComplete(file)
+      await markMigrationComplete(file, runMigration)
 
       log.info({ file }, `Finished migration ${file}`)
     }

Diff for: backend/test/services/model.spec.ts

@@ -86,6 +86,7 @@ vi.mock('../../src/models/Model.js', () => ({ default: modelMocks }))
 
 const authenticationMocks = vi.hoisted(() => ({
   getEntities: vi.fn(() => ['user']),
+  getUserInformation: vi.fn(() => ({ name: 'user', email: '[email protected]' })),
 }))
 vi.mock('../../src/connectors/authentication/index.js', async () => ({
   default: authenticationMocks,
@@ -119,6 +120,15 @@ describe('services > model', () => {
     expect(modelMocks.save).not.toBeCalled()
   })
 
+  test('createModel > should throw an internal error if getUserInformation fails due to invalid user', async () => {
+    authenticationMocks.getUserInformation.mockImplementation(() => {
+      throw new Error('Unable to find user user:unknown_user')
+    })
+    expect(() =>
+      createModel({} as any, { collaborators: [{ entity: 'user:unknown_user', roles: [] }] } as any),
+    ).rejects.toThrowError(/^Unable to find user user:unknown_user/)
+  })
+
   test('getModelById > good', async () => {
     modelMocks.findOne.mockResolvedValueOnce('mocked')
 
@@ -326,6 +336,15 @@ describe('services > model', () => {
     ).rejects.toThrowError(/^You cannot select both mirror settings simultaneously./)
   })
 
+  test('updateModel > should throw an internal error if getUserInformation fails due to invalid user', async () => {
+    authenticationMocks.getUserInformation.mockImplementation(() => {
+      throw new Error('Unable to find user user:unknown_user')
+    })
+    expect(() =>
+      updateModel({} as any, '123', { collaborators: [{ entity: 'user:unknown_user', roles: [] }] }),
+    ).rejects.toThrowError(/^Unable to find user user:unknown_user/)
+  })
+
   test('createModelcardFromSchema > should throw an error when attempting to change a model from mirrored to standard', async () => {
     vi.mocked(authorisation.model).mockResolvedValue({
       info: 'Cannot alter a mirrored model.',

Diff for: frontend/src/common/HelpDialog.tsx

@@ -22,7 +22,7 @@ export default function HelpDialog({ title, content }: HelpDialogProps) {
     <>
       <Tooltip title={title}>
         <IconButton size='small' onClick={handleOpen}>
-          <HelpOutlineIcon />
+          <HelpOutlineIcon color='primary' />
         </IconButton>
       </Tooltip>
       <Dialog open={open} onClose={handleClose} maxWidth='md' TransitionComponent={Transition}>

Diff for: frontend/src/common/HelpPopover.tsx

@@ -29,6 +29,7 @@ function HelpPopover({ anchorOrigin, transformOrigin, children }: Props) {
         onMouseEnter={handlePopoverOpen}
         onMouseLeave={handlePopoverClose}
         data-test='helpIcon'
+        color='primary'
       />
       <Popover
         id='help-popover'

Diff for: frontend/src/entry/settings/EntryAccessInput.tsx

@@ -3,8 +3,9 @@ import { useListUsers } from 'actions/user'
 import { debounce } from 'lodash-es'
 import { SyntheticEvent, useCallback, useEffect, useMemo, useState } from 'react'
 import EntityItem from 'src/entry/settings/EntityItem'
+import ManualEntityInput from 'src/entry/settings/ManualEntityInput'
 import MessageAlert from 'src/MessageAlert'
-import { CollaboratorEntry, EntityObject, EntryKindKeys, Role } from 'types/types'
+import { CollaboratorEntry, EntityKind, EntityObject, EntryKindKeys, Role } from 'types/types'
 import { toSentenceCase } from 'utils/stringUtils'
 
 type EntryAccessInputProps = {
@@ -27,6 +28,7 @@ export default function EntryAccessInput({ value, onUpdate, entryKind, entryRole
   const [open, setOpen] = useState(false)
   const [accessList, setAccessList] = useState<CollaboratorEntry[]>(value)
   const [userListQuery, setUserListQuery] = useState('')
+  const [manualEntityInputErrorMessage, setManualEntityInputErrorMessage] = useState('')
 
   const { users, isUsersLoading, isUsersError } = useListUsers(userListQuery)
 
@@ -71,6 +73,15 @@ export default function EntryAccessInput({ value, onUpdate, entryKind, entryRole
     setUserListQuery(value)
   }, [])
 
+  const handleAddEntityManually = (manualEntityName: string) => {
+    setManualEntityInputErrorMessage('')
+    if (accessList.find((collaborator) => collaborator.entity === `${EntityKind.USER}:${manualEntityName}`)) {
+      setManualEntityInputErrorMessage('User has already been added below.')
+    } else {
+      setAccessList([...accessList, { entity: `${EntityKind.USER}:${manualEntityName}`, roles: [] }])
+    }
+  }
+
   const debounceOnInputChange = debounce((event: SyntheticEvent<Element, Event>, value: string) => {
     handleInputChange(event, value)
   }, 500)
@@ -113,6 +124,7 @@ export default function EntryAccessInput({ value, onUpdate, entryKind, entryRole
           />
         )}
       />
+      <ManualEntityInput onAddEntityManually={handleAddEntityManually} errorMessage={manualEntityInputErrorMessage} />
       <Table>
         <TableHead>
           <TableRow>

Diff for: frontend/src/entry/settings/ManualEntityInput.tsx

@@ -0,0 +1,68 @@
+import ExpandMoreIcon from '@mui/icons-material/ExpandMore'
+import { Accordion, AccordionDetails, AccordionSummary, Box, Button, Stack, TextField, Typography } from '@mui/material'
+import { useGetUiConfig } from 'actions/uiConfig'
+import { FormEvent, useState } from 'react'
+import HelpPopover from 'src/common/HelpPopover'
+import Loading from 'src/common/Loading'
+import MessageAlert from 'src/MessageAlert'
+
+interface ManualEntityInputProps {
+  onAddEntityManually: (entityName: string) => void
+  errorMessage: string
+}
+
+export default function ManualEntityInput({ onAddEntityManually, errorMessage }: ManualEntityInputProps) {
+  const [manualEntityName, setManualEntityName] = useState('')
+
+  const { uiConfig, isUiConfigLoading, isUiConfigError } = useGetUiConfig()
+
+  const handleAddEntityManuallyOnClick = (event: FormEvent<HTMLFormElement>) => {
+    event.preventDefault()
+    if (manualEntityName !== undefined && manualEntityName !== '') {
+      setManualEntityName('')
+      onAddEntityManually(manualEntityName)
+    }
+  }
+
+  if (isUiConfigError) {
+    return <MessageAlert message={isUiConfigError.info.message} severity='error' />
+  }
+
+  return (
+    <Accordion sx={{ borderTop: 'none' }}>
+      <AccordionSummary
+        sx={{ pl: 0, borderTop: 'none' }}
+        expandIcon={<ExpandMoreIcon />}
+        aria-controls='manual-user-add-content'
+        id='manual-user-add-header'
+      >
+        <Typography sx={{ mr: 1 }} component='caption'>
+          Trouble finding a user? Click here to add them manually
+        </Typography>
+      </AccordionSummary>
+      <AccordionDetails sx={{ p: 0 }}>
+        {isUiConfigLoading && <Loading />}
+        {!isUiConfigLoading && uiConfig && (
+          <Box component='form' onSubmit={handleAddEntityManuallyOnClick}>
+            <Stack spacing={2} direction={{ xs: 'column', sm: 'row' }} alignItems='center'>
+              <TextField
+                size='small'
+                fullWidth
+                label='User'
+                value={manualEntityName}
+                onChange={(e) => setManualEntityName(e.target.value)}
+              />
+              {uiConfig.helpPopoverText.manualEntryAccess && (
+                <HelpPopover>{uiConfig.helpPopoverText.manualEntryAccess}</HelpPopover>
+              )}
+              <Button variant='contained' type='submit' disabled={manualEntityName === ''}>
+                Add
+              </Button>
+            </Stack>
+          </Box>
+        )}
+        <MessageAlert message={errorMessage} severity='error' />
+      </AccordionDetails>
+    </Accordion>
+  )
+}