Prevent access requests and model cards being created using hidden schemas

backend/src/routes/v2/schema/getSchema.ts

@@ -5,15 +5,13 @@ import { z } from 'zod'
 import { AuditInfo } from '../../../connectors/audit/Base.js'
 import audit from '../../../connectors/audit/index.js'
 import { SchemaInterface } from '../../../models/Schema.js'
-import { findSchemaById } from '../../../services/schema.js'
+import { getSchemaById } from '../../../services/schema.js'
 import { registerPath, schemaInterfaceSchema } from '../../../services/specification.js'
 import { parse } from '../../../utils/validate.js'
 
 export const getSchemaSchema = z.object({
   params: z.object({
-    schemaId: z.string({
-      required_error: 'Must specify schema id as URL parameter',
-    }),
+    schemaId: z.string(),
   }),
 })
 
@@ -47,7 +45,7 @@ export const getSchema = [
     req.audit = AuditInfo.ViewSchema
     const { params } = parse(req, getSchemaSchema)
 
-    const schema = await findSchemaById(params.schemaId)
+    const schema = await getSchemaById(params.schemaId)
     await audit.onViewSchema(req, schema)
 
     return res.json({

backend/src/routes/v2/schema/getSchemas.ts

@@ -5,15 +5,15 @@ import { z } from 'zod'
 import { AuditInfo } from '../../../connectors/audit/Base.js'
 import audit from '../../../connectors/audit/index.js'
 import { SchemaInterface } from '../../../models/Schema.js'
-import { findSchemasByKind } from '../../../services/schema.js'
+import { searchSchemas } from '../../../services/schema.js'
 import { registerPath, schemaInterfaceSchema } from '../../../services/specification.js'
 import { SchemaKind } from '../../../types/enums.js'
 import { parse, strictCoerceBoolean } from '../../../utils/validate.js'
 
 export const getSchemasSchema = z.object({
   query: z.object({
     kind: z.nativeEnum(SchemaKind).optional(),
-    includeHidden: strictCoerceBoolean(z.boolean().optional().default(false)),
+    hidden: strictCoerceBoolean(z.boolean().optional()),
   }),
 })
 
@@ -47,7 +47,7 @@ export const getSchemas = [
     req.audit = AuditInfo.SearchSchemas
     const { query } = parse(req, getSchemasSchema)
 
-    const schemas = await findSchemasByKind(query.kind, query.includeHidden)
+    const schemas = await searchSchemas(query.kind, query.hidden)
     await audit.onSearchSchemas(req, schemas)
 
     return res.json({

backend/src/services/accessRequest.ts

@@ -16,7 +16,7 @@ import { convertStringToId } from '../utils/id.js'
 import log from './log.js'
 import { getModelById } from './model.js'
 import { createAccessRequestReviews } from './review.js'
-import { findSchemaById } from './schema.js'
+import { getSchemaById } from './schema.js'
 import { sendWebhooks } from './webhook.js'
 
 export type CreateAccessRequestParams = Pick<AccessRequestInterface, 'metadata' | 'schemaId'>
@@ -29,7 +29,10 @@ export async function createAccessRequest(
   const model = await getModelById(user, modelId)
 
   // Ensure that the AR meets the schema
-  const schema = await findSchemaById(accessRequestInfo.schemaId)
+  const schema = await getSchemaById(accessRequestInfo.schemaId)
+  if (schema.hidden) {
+    throw BadReq('Cannot create new Access Request using a hidden schema.', { schemaId: accessRequestInfo.schemaId })
+  }
   try {
     new Validator().validate(accessRequestInfo.metadata, schema.jsonSchema, { throwAll: true, required: true })
   } catch (error) {
@@ -128,7 +131,7 @@ export async function updateAccessRequest(
   }
 
   // Ensure that the AR meets the schema
-  const schema = await findSchemaById(accessRequest.schemaId, true)
+  const schema = await getSchemaById(accessRequest.schemaId)
   try {
     new Validator().validate(accessRequest.metadata, schema.jsonSchema, { throwAll: true, required: true })
   } catch (error) {

backend/src/services/model.ts

@@ -16,7 +16,7 @@ import { isValidatorResultError } from '../types/ValidatorResultError.js'
 import { toEntity } from '../utils/entity.js'
 import { BadReq, Forbidden, InternalError, NotFound } from '../utils/error.js'
 import { convertStringToId } from '../utils/id.js'
-import { findSchemaById } from './schema.js'
+import { getSchemaById } from './schema.js'
 
 export function checkModelRestriction(model: ModelInterface) {
   if (model.settings.mirror.sourceModelId) {
@@ -286,7 +286,7 @@ export async function updateModelCard(
     throw BadReq(`This model must first be instantiated before it can be `, { modelId })
   }
 
-  const schema = await findSchemaById(model.card.schemaId, true)
+  const schema = await getSchemaById(model.card.schemaId)
   try {
     new Validator().validate(metadata, schema.jsonSchema, { throwAll: true, required: true })
   } catch (error) {
@@ -347,7 +347,10 @@ export async function createModelCardFromSchema(
   }
 
   // Ensure schema exists
-  await findSchemaById(schemaId)
+  const schema = await getSchemaById(schemaId)
+  if (schema.hidden) {
+    throw BadReq('Cannot create a new Card using a hidden schema.', { schemaId, kind: schema.kind })
+  }
 
   const revision = await _setModelCard(user, modelId, schemaId, 1, {})
   return revision
@@ -397,7 +400,7 @@ export async function saveImportedModelCard(modelCard: ModelCardRevisionInterfac
     })
   }
 
-  const schema = await findSchemaById(modelCard.schemaId, true)
+  const schema = await getSchemaById(modelCard.schemaId)
   try {
     new Validator().validate(modelCard.metadata, schema.jsonSchema, { throwAll: true, required: true })
   } catch (error) {

backend/src/services/modelCardExport.ts

@@ -4,7 +4,7 @@ import showdown from 'showdown'
 import { UserInterface } from '../models/User.js'
 import { GetModelCardVersionOptionsKeys } from '../types/enums.js'
 import { getModelById, getModelCard } from './model.js'
-import { findSchemaById } from './schema.js'
+import { getSchemaById } from './schema.js'
 
 type Common = {
   title: string
@@ -48,7 +48,7 @@ export async function renderToMarkdown(
     throw new Error('Could not find specified model card')
   }
 
-  const schema = await findSchemaById(card.schemaId, true)
+  const schema = await getSchemaById(card.schemaId)
   if (!schema) {
     throw new Error('Trying to export model with no corresponding card')
   }

backend/src/services/schema.ts

@@ -17,18 +17,17 @@ export interface DefaultSchema {
   jsonSchema: JsonSchema
 }
 
-export async function findSchemasByKind(kind?: SchemaKindKeys, includeHidden = false): Promise<SchemaInterface[]> {
-  const baseSchemas = await Schema.find({
+export async function searchSchemas(kind?: SchemaKindKeys, hidden?: boolean): Promise<SchemaInterface[]> {
+  const schemas = await Schema.find({
     ...(kind && { kind }),
-    ...(!includeHidden && { hidden: false }),
+    ...(hidden != undefined && { hidden }),
   }).sort({ createdAt: -1 })
-  return baseSchemas
+  return schemas
 }
 
-export async function findSchemaById(schemaId: string, includeHidden = false) {
+export async function getSchemaById(schemaId: string) {
   const schema = await Schema.findOne({
     id: schemaId,
-    ...(!includeHidden && { hidden: false }),
   })
 
   if (!schema) {
@@ -86,7 +85,7 @@ export async function createSchema(user: UserInterface, schema: Partial<SchemaIn
 export type UpdateSchemaParams = Partial<Pick<SchemaInterface, 'active' | 'hidden'>>
 
 export async function updateSchema(user: UserInterface, schemaId: string, diff: UpdateSchemaParams) {
-  const schema = await findSchemaById(schemaId, true)
+  const schema = await getSchemaById(schemaId)
 
   const auth = await authorisation.schema(user, schema, SchemaAction.Update)
   if (!auth.success) {

backend/test/routes/schema/getSchema.spec.ts

@@ -14,22 +14,22 @@ vi.mock('../../../src/connectors/authorisation/index.js')
 const mockSchemaService = vi.hoisted(() => {
   return {
     addDefaultSchemas: vi.fn(),
-    findSchemaById: vi.fn(),
+    getSchemaById: vi.fn(),
   }
 })
 vi.mock('../../../src/services/schema.js', () => mockSchemaService)
 
 describe('routes > schema > getSchema', () => {
   test('returns the schema with the matching ID', async () => {
-    mockSchemaService.findSchemaById.mockReturnValueOnce(testModelSchema)
+    mockSchemaService.getSchemaById.mockReturnValueOnce(testModelSchema)
     const res = await testGet(`/api/v2/schema/${testModelSchema.id}`)
 
     expect(res.statusCode).toBe(200)
     expect(res.body).matchSnapshot()
   })
 
   test('audit > expected call', async () => {
-    mockSchemaService.findSchemaById.mockReturnValueOnce(testModelSchema)
+    mockSchemaService.getSchemaById.mockReturnValueOnce(testModelSchema)
     const res = await testGet(`/api/v2/schema/${testModelSchema.id}`)
 
     expect(res.statusCode).toBe(200)
@@ -38,7 +38,7 @@ describe('routes > schema > getSchema', () => {
   })
 
   test('returns the schema with the matching ID', async () => {
-    mockSchemaService.findSchemaById.mockRejectedValueOnce(NotFound('Schema not found.'))
+    mockSchemaService.getSchemaById.mockRejectedValueOnce(NotFound('Schema not found.'))
     const res = await testGet(`/api/v2/schema/does-not-exist`)
 
     expect(res.statusCode).toBe(404)

backend/test/routes/schema/getSchemas.spec.ts

@@ -13,7 +13,7 @@ vi.mock('../../../src/connectors/authorisation/index.js')
 const mockSchemaService = vi.hoisted(() => {
   return {
     addDefaultSchemas: vi.fn(),
-    findSchemasByKind: vi.fn(() => [testDeploymentSchema, testModelSchema]),
+    searchSchemas: vi.fn(() => [testDeploymentSchema, testModelSchema]),
   }
 })
 vi.mock('../../../src/services/schema.js', () => mockSchemaService)
@@ -35,15 +35,15 @@ describe('routes > schema > getSchemas', () => {
   })
 
   test('returns only model schemas with the model parameter', async () => {
-    mockSchemaService.findSchemasByKind.mockReturnValueOnce([testModelSchema])
+    mockSchemaService.searchSchemas.mockReturnValueOnce([testModelSchema])
     const res = await testGet(`/api/v2/schemas?kind=model`)
 
     expect(res.statusCode).toBe(200)
     expect(res.body).matchSnapshot()
   })
 
   test('returns only deployment schemas with the accessRequest parameter', async () => {
-    mockSchemaService.findSchemasByKind.mockReturnValueOnce([testDeploymentSchema])
+    mockSchemaService.searchSchemas.mockReturnValueOnce([testDeploymentSchema])
     const res = await testGet(`/api/v2/schemas?kind=accessRequest`)
 
     expect(res.statusCode).toBe(200)
@@ -53,7 +53,7 @@ describe('routes > schema > getSchemas', () => {
   test('rejects unknown query parameter', async () => {
     const res = await testGet(`/api/v2/schemas?kind=notValid`)
 
-    expect(mockSchemaService.findSchemasByKind).not.toBeCalled()
+    expect(mockSchemaService.searchSchemas).not.toBeCalled()
     expect(res.statusCode).toBe(400)
     expect(res.body).matchSnapshot()
   })

backend/test/services/accessRequest.spec.ts

@@ -17,7 +17,7 @@ const modelMocks = vi.hoisted(() => ({
 vi.mock('../../src/services/model.js', () => modelMocks)
 
 const schemaMocks = vi.hoisted(() => ({
-  findSchemaById: vi.fn(),
+  getSchemaById: vi.fn(),
 }))
 vi.mock('../../src/services/schema.js', () => schemaMocks)
 
@@ -62,7 +62,7 @@ const accessRequest = {
 describe('services > accessRequest', () => {
   test('createAccessRequest > simple', async () => {
     modelMocks.getModelById.mockResolvedValue(undefined)
-    schemaMocks.findSchemaById.mockResolvedValue({ jsonSchema: {} })
+    schemaMocks.getSchemaById.mockResolvedValue({ jsonSchema: {} })
 
     await createAccessRequest({} as any, 'example-model', accessRequest)
 
@@ -80,7 +80,7 @@ describe('services > accessRequest', () => {
     })
 
     modelMocks.getModelById.mockResolvedValue(undefined)
-    schemaMocks.findSchemaById.mockResolvedValue({ jsonSchema: {} })
+    schemaMocks.getSchemaById.mockResolvedValue({ jsonSchema: {} })
 
     expect(() => createAccessRequest({} as any, 'example-model', accessRequest)).rejects.toThrowError(
       /^You do not have permission/,

backend/test/services/model.spec.ts

@@ -22,7 +22,7 @@ import {
 vi.mock('../../src/connectors/authorisation/index.js')
 
 const schemaMock = vi.hoisted(() => ({
-  findSchemaById: vi.fn(() => ({ jsonschema: {} })),
+  getSchemaById: vi.fn(() => ({ jsonschema: {} })),
 }))
 vi.mock('../../src/services/schema.js', async () => schemaMock)
 

backend/test/services/modelCardExport.spec.ts

@@ -2,7 +2,7 @@ import { beforeEach, describe, expect, test, vi } from 'vitest'
 
 import { getModelById, getModelCard } from '../../src/services/model.js'
 import { renderToHtml, renderToMarkdown } from '../../src/services/modelCardExport.js'
-import { findSchemaById } from '../../src/services/schema.js'
+import { getSchemaById } from '../../src/services/schema.js'
 
 vi.mock('../../src/services/model.js')
 vi.mock('../../src/services/schema.js')
@@ -18,7 +18,7 @@ describe('services > export', () => {
   beforeEach(() => {
     vi.mocked(getModelById).mockResolvedValue(mockModel as any)
     vi.mocked(getModelCard).mockResolvedValue(mockCard as any)
-    vi.mocked(findSchemaById).mockResolvedValue(mockSchema as any)
+    vi.mocked(getSchemaById).mockResolvedValue(mockSchema as any)
   })
 
   test('renderToMarkdown > should throw error if model has no card', async () => {
@@ -38,7 +38,7 @@ describe('services > export', () => {
   })
 
   test('renderToMarkdown > should throw error if schema is not found', async () => {
-    vi.mocked(findSchemaById).mockResolvedValueOnce(undefined as any)
+    vi.mocked(getSchemaById).mockResolvedValueOnce(undefined as any)
 
     await expect(renderToMarkdown(mockUser, mockModelId, mockVersion)).rejects.toThrow(
       'Trying to export model with no corresponding card',

backend/test/services/schema.spec.ts

@@ -3,7 +3,7 @@ import { describe, expect, test, vi } from 'vitest'
 
 import authorisation from '../../src/connectors/authorisation/index.js'
 import { UserInterface } from '../../src/models/User.js'
-import { createSchema, findSchemaById, findSchemasByKind } from '../../src/services/schema.js'
+import { createSchema, getSchemaById, searchSchemas } from '../../src/services/schema.js'
 import { testModelSchema } from '../testUtils/testModels.js'
 
 vi.mock('../../src/connectors/authorisation/index.js')
@@ -43,7 +43,7 @@ describe('services > schema', () => {
   const testUser = { dn: 'user' } as UserInterface
 
   test('that all schemas can be retrieved', async () => {
-    const result = await findSchemasByKind('model')
+    const result = await searchSchemas('model')
     expect(result).toEqual(['schema-1', 'schema-2'])
   })
 
@@ -93,11 +93,11 @@ describe('services > schema', () => {
 
   test('that a schema can be retrieved by ID', async () => {
     mockSchema.findOne.mockResolvedValueOnce(testModelSchema)
-    const result = await findSchemaById(testModelSchema.id)
+    const result = await getSchemaById(testModelSchema.id)
     expect(result).toEqual(testModelSchema)
   })
 
   test('that a schema cannot be retrieved by ID when schema does not exist', async () => {
-    expect(() => findSchemaById(testModelSchema.id)).rejects.toThrowError(/^The requested schema was not found/)
+    expect(() => getSchemaById(testModelSchema.id)).rejects.toThrowError(/^The requested schema was not found/)
   })
 })

frontend/actions/schema.ts

@@ -14,10 +14,10 @@ export interface PostSchemaParams {
   jsonSchema: any
 }
 
-export function useGetSchemas(kind: SchemaKindKeys, includeHidden = false) {
+export function useGetSchemas(kind: SchemaKindKeys, hidden?: boolean) {
   const queryParams = {
     kind,
-    includeHidden,
+    ...(hidden != undefined && { hidden }),
   }
 
   const { data, isLoading, error, mutate } = useSWR<

frontend/pages/model/[modelId]/access-request/schema.tsx

@@ -28,7 +28,7 @@ import { SchemaInterface, SchemaKind } from 'types/types'
 export default function AccessRequestSchema() {
   const router = useRouter()
   const { modelId }: { modelId?: string } = router.query
-  const { schemas, isSchemasLoading, isSchemasError } = useGetSchemas(SchemaKind.ACCESS_REQUEST)
+  const { schemas, isSchemasLoading, isSchemasError } = useGetSchemas(SchemaKind.ACCESS_REQUEST, false)
 
   const [loading, setLoading] = useState(false)
 

frontend/src/schemas/SchemaList.tsx

@@ -38,7 +38,7 @@ interface ObjectToDelete {
 }
 
 export default function SchemaList({ schemaKind }: SchemaDisplayProps) {
-  const { schemas, isSchemasLoading, isSchemasError, mutateSchemas } = useGetSchemas(schemaKind, true)
+  const { schemas, isSchemasLoading, isSchemasError, mutateSchemas } = useGetSchemas(schemaKind)
   const { reviews, isReviewsLoading, isReviewsError } = useGetReviewRequestsForUser()
 
   const theme = useTheme()

frontend/src/schemas/SchemaSelect.tsx

@@ -35,6 +35,7 @@ export default function SchemaSelect({ entry }: SchemaSelectProps) {
   const [errorMessage, setErrorMessage] = useState('')
   const { schemas, isSchemasLoading, isSchemasError } = useGetSchemas(
     entry.kind === EntryKind.MODEL ? SchemaKind.MODEL : SchemaKind.DATA_CARD,
+    false,
   )
   const { currentUser, isCurrentUserLoading, isCurrentUserError } = useGetCurrentUser()
   const { mutateModel: mutateEntry } = useGetModel(entry.id, entry.kind)