Skip to content
Open
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
cf949ae
wip, initial endpoint to fetch all access requests
JRB66955 Nov 5, 2025
7823542
update findAccessRequest function to allow admins to get all accessRe…
JRB66955 Nov 6, 2025
3013b70
flatten if/else block and add more meaningful error msg
JRB66955 Nov 7, 2025
68e2694
Merge branch 'main' into dev/all-access-requests-for-admin-endpoint
JRB66955 Nov 7, 2025
9b3bf36
add initial tests for findAccessRequest func
JRB66955 Nov 7, 2025
fa88707
add tests for new getAccessRequests file
JRB66955 Nov 10, 2025
80f61ff
add back accidentally removed snapshot
JRB66955 Nov 10, 2025
6aa51aa
fix api routing issue, remove temp url to something more suitable whi…
JRB66955 Nov 11, 2025
5f643e0
re-order import to fix class extends value undefined error with authe…
JRB66955 Nov 11, 2025
1945c77
add additional mock getEntities func for auth
JRB66955 Nov 11, 2025
a7e992f
Merge branch 'main' into dev/all-access-requests-for-admin-endpoint
JRB66955 Nov 11, 2025
2affc33
wip commit, updating wrt to PR feedback, making endpoint more akin to…
JRB66955 Nov 12, 2025
f283092
update with new tests to reflect updated endpoint
JRB66955 Nov 17, 2025
26351ea
update one of the tests to exercise the full function, with added req…
JRB66955 Nov 17, 2025
85564c4
Merge branch 'main' into dev/all-access-requests-for-admin-endpoint
JRB66955 Nov 18, 2025
d21d80a
Merge branch 'main' into dev/all-access-requests-for-admin-endpoint
JRB66955 Nov 19, 2025
4da7e94
update with PR feedback, only include relevant ARs in auth connector,…
JRB66955 Nov 19, 2025
6640081
Merge branch 'main' into dev/all-access-requests-for-admin-endpoint
JRB66955 Nov 20, 2025
4703d4c
Slight tweak to new condition, try not to break python tests this time
JRB66955 Nov 20, 2025
84e76a4
rework of accessRequests, a new model/AR aggregation to loop/filter t…
JRB66955 Nov 21, 2025
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
3 changes: 2 additions & 1 deletion backend/src/connectors/authorisation/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,8 @@ export class BasicAuthorisationConnector {
if (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this condition should remain as being for updating and deleting an access and request and a new condition should be added for viewing access requests where we reject if the user cannot view the model or is not named on the access request

!isNamed &&
(await missingRequiredRole(user, model, ['owner'])) &&
([AccessRequestAction.Delete, AccessRequestAction.Update] as AccessRequestActionKeys[]).includes(action)
([AccessRequestAction.Delete, AccessRequestAction.Update] as AccessRequestActionKeys[]).includes(action) &&
(await this.model(user, model, ModelAction.View))
) {
return { success: false, info: 'You cannot change an access request you do not own.', id: request.id }
}
Expand Down
2 changes: 2 additions & 0 deletions backend/src/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { putFileScan } from './routes/v2/filescanning/putFileScan.js'
import { deleteAccessRequest } from './routes/v2/model/accessRequest/deleteAccessRequest.js'
import { getAccessRequest } from './routes/v2/model/accessRequest/getAccessRequest.js'
import { getAccessRequestCurrentUserPermissions } from './routes/v2/model/accessRequest/getAccessRequestCurrentUserPermissions.js'
import { getAccessRequests } from './routes/v2/model/accessRequest/getAccessRequests.js'
import { getModelAccessRequests } from './routes/v2/model/accessRequest/getModelAccessRequests.js'
import { patchAccessRequest } from './routes/v2/model/accessRequest/patchAccessRequest.js'
import { postAccessRequest } from './routes/v2/model/accessRequest/postAccessRequest.js'
Expand Down Expand Up @@ -138,6 +139,7 @@ server.post('/api/v2/model/:modelId/release/:semver/review', ...postReleaseRevie

server.post('/api/v2/model/:modelId/access-requests', ...postAccessRequest)
server.get('/api/v2/model/:modelId/access-requests', getModelAccessRequests)
server.get('/api/v2/access-requests/search', getAccessRequests)
server.get('/api/v2/model/:modelId/access-request/:accessRequestId', ...getAccessRequest)
server.delete('/api/v2/model/:modelId/access-request/:accessRequestId', ...deleteAccessRequest)
server.patch('/api/v2/model/:modelId/access-request/:accessRequestId', ...patchAccessRequest)
Expand Down
57 changes: 57 additions & 0 deletions backend/src/routes/v2/model/accessRequest/getAccessRequests.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { Request, Response } from 'express'
import { z } from 'zod'

import { AuditInfo } from '../../../../connectors/audit/Base.js'
import audit from '../../../../connectors/audit/index.js'
import { AccessRequestInterface } from '../../../../models/AccessRequest.js'
import { findAccessRequest } from '../../../../services/accessRequest.js'
import { accessRequestInterfaceSchema, registerPath } from '../../../../services/specification.js'
import { coerceArray, parse, strictCoerceBoolean } from '../../../../utils/validate.js'

export const GetAccessRequestsSchema = z.object({
query: z.object({
modelId: coerceArray(z.array(z.string()).optional().default([])),
schemaId: z.string().optional().default(''),
mine: strictCoerceBoolean(z.boolean().optional().default(false)),
adminAccess: strictCoerceBoolean(z.boolean().optional().default(false)),
}),
})

registerPath({
method: 'get',
path: '/api/v2/access-requests/search',
tags: ['access-request'],
description: 'Get all access requests for all models.',
schema: GetAccessRequestsSchema,
responses: {
200: {
description: 'An array of access request instances.',
content: {
'application/json': {
schema: z.object({
accessRequests: z.array(accessRequestInterfaceSchema),
}),
},
},
},
},
})

interface GetAccessRequestsResponse {
accessRequests: Array<AccessRequestInterface>
}

export const getAccessRequests = [
async (req: Request, res: Response<GetAccessRequestsResponse>): Promise<void> => {
req.audit = AuditInfo.ViewAccessRequests
const {
query: { modelId, schemaId, mine, adminAccess },
} = parse(req, GetAccessRequestsSchema)

const accessRequests = await findAccessRequest(req.user, modelId, schemaId, mine, adminAccess)

await audit.onViewAccessRequests(req, accessRequests)

res.json({ accessRequests })
},
]
63 changes: 62 additions & 1 deletion backend/src/services/accessRequest.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
// eslint-disable-next-line simple-import-sort/imports
import { Validator } from 'jsonschema'
import { Types } from 'mongoose'

import authentication from '../connectors/authentication/index.js'
import { Roles } from '../connectors/authentication/Base.js'
import { AccessRequestAction } from '../connectors/authorisation/actions.js'
import authorisation from '../connectors/authorisation/index.js'
import { AccessRequestInterface } from '../models/AccessRequest.js'
import AccessRequestModel, { AccessRequestDoc, AccessRequestInterface } from '../models/AccessRequest.js'
import AccessRequest from '../models/AccessRequest.js'
import ResponseModel, { ResponseKind } from '../models/Response.js'
import ReviewModel from '../models/Review.js'
Expand All @@ -23,6 +25,7 @@ import { removeResponsesByParentIds } from './response.js'
import { createAccessRequestReviews, removeAccessRequestReviews } from './review.js'
import { getSchemaById } from './schema.js'
import { sendWebhooks } from './webhook.js'
import ModelModel from '../models/Model.js'

export type CreateAccessRequestParams = Pick<AccessRequestInterface, 'metadata' | 'schemaId'>
export async function createAccessRequest(
Expand Down Expand Up @@ -131,6 +134,64 @@ export async function getAccessRequestById(user: UserInterface, accessRequestId:
return accessRequest
}

export async function findAccessRequest(
user: UserInterface,
modelId: Array<string>,
schemaId: string,
filters: Array<string>,
adminAccess?: boolean,
): Promise<Array<AccessRequestDoc>> {
if (adminAccess) {
if (!(await authentication.hasRole(user, Roles.Admin))) {
throw Forbidden('You do not have the required role.', {
userDn: user.dn,
requiredRole: Roles.Admin,
})
}
}

const query: any = {}

if (modelId.length) {
query.modelId = { $all: modelId }
}

if (schemaId) {
query.schemaId = { $all: schemaId }
}

if (filters.length > 0) {
if (filters.includes('mine')) {
query.metadata.overview = {
$elemMatch: {
entity: { $in: await authentication.getEntities(user) },
},
}
}
}

const cursor = AccessRequestModel.find(query)

const results = await cursor
//Auth already checked, so just need to check if they require admin access
if (adminAccess) {
return results
}

// authorisation
const modelIds = results.map((result) => result.modelId)
let auths: any[] = []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth creating a return type for authorisation.accessRequests() so we don't have to use any here?

Although tracing through to accessRequests it looks a little more complicated than i thought

for (const modelId of modelIds) {
const modelDoc = await ModelModel.findOne({ id: modelId })
if (!modelDoc) {
throw BadReq('Model cannot be found', { modelId })
}
const model = modelDoc.toObject()
auths = auths.concat(await authorisation.accessRequests(user, model, results, AccessRequestAction.View))
}
return results.filter((_, i) => auths[i].success)
}

export type UpdateAccessRequestParams = Pick<AccessRequestInterface, 'metadata'>
export async function updateAccessRequest(
user: UserInterface,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html

exports[`routes > accessRequest > getAccessRequests > 200 > ok 1`] = `
{
"accessRequests": {
"_id": "664e1aa8bda1f88c28e1c0ce",
"deleted": false,
"deletedAt": "",
"deletedBy": "",
"id": "test-access-request-13623",
"modelId": "test-model-4342",
},
}
`;

exports[`routes > accessRequest > getAccessRequests > audit > expected call 1`] = `
{
"_id": "664e1aa8bda1f88c28e1c0ce",
"deleted": false,
"deletedAt": "",
"deletedBy": "",
"id": "test-access-request-13623",
"modelId": "test-model-4342",
}
`;
51 changes: 51 additions & 0 deletions backend/test/routes/model/accessRequest/getAccessRequests.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import qs from 'qs'
import { describe, expect, test, vi } from 'vitest'

import audit from '../../../../src/connectors/audit/__mocks__/index.js'
import AccessRequestModel from '../../../../src/models/AccessRequest.js'
import { GetAccessRequestsSchema } from '../../../../src/routes/v2/model/accessRequest/getAccessRequests.js'
import { createFixture, testGet } from '../../../testUtils/routes.js'
import { testAccessRequest } from '../../../testUtils/testModels.js'

vi.mock('../../../../src/connectors/audit/index.js')

const accessRequestsMock = vi.hoisted(() => {
return {
findAccessRequest: vi.fn(() => undefined as any),
}
})
vi.mock('../../../../src/services/accessRequest.js', () => accessRequestsMock)

const responseMock = vi.hoisted(() => {
return {
findResponses: vi.fn(() => undefined as any),
}
})
vi.mock('../../../../src/services/response.js', () => responseMock)

describe('routes > accessRequest > getAccessRequests', () => {
test('200 > ok', async () => {
const accessRequestDoc = new AccessRequestModel({ ...testAccessRequest })
accessRequestsMock.findAccessRequest.mockResolvedValueOnce(accessRequestDoc)
responseMock.findResponses.mockResolvedValue([testAccessRequest])

const fixture = createFixture(GetAccessRequestsSchema)
const res = await testGet(`/api/v2/access-requests/search?${qs.stringify(fixture.query)}`)

expect(res.statusCode).toBe(200)
expect(res.body).matchSnapshot()
})

test('audit > expected call', async () => {
const accessRequestDoc = new AccessRequestModel({ ...testAccessRequest })
accessRequestsMock.findAccessRequest.mockResolvedValueOnce(accessRequestDoc)
responseMock.findResponses.mockResolvedValue([testAccessRequest])

const fixture = createFixture(GetAccessRequestsSchema)
const res = await testGet(`/api/v2/access-requests/search?${qs.stringify(fixture.query)}`)

expect(res.statusCode).toBe(200)
expect(audit.onViewAccessRequests).toBeCalled()
expect(audit.onViewAccessRequests.mock.calls.at(0)?.at(1)).toMatchSnapshot()
})
})
12 changes: 12 additions & 0 deletions backend/test/services/__snapshots__/accessRequest.spec.ts.snap
Original file line number Diff line number Diff line change
@@ -1,5 +1,17 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html

exports[`services > accessRequest > findAccessRequest > admin access with auth 1`] = `[]`;

exports[`services > accessRequest > findAccessRequest > all filters 1`] = `[]`;

exports[`services > accessRequest > findAccessRequest > no filters 1`] = `
[
{
"_id": "a",
},
]
`;

exports[`services > accessRequest > getAccessRequestsByModel > good 1`] = `
[
{
Expand Down
64 changes: 64 additions & 0 deletions backend/test/services/accessRequest.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import authorisation from '../../src/connectors/authorisation/index.js'
import { UserInterface } from '../../src/models/User.js'
import {
createAccessRequest,
findAccessRequest,
getAccessRequestsByModel,
getCurrentUserPermissionsByAccessRequest,
getModelAccessRequestsForUser,
Expand All @@ -18,11 +19,39 @@ const modelMocks = vi.hoisted(() => ({
}))
vi.mock('../../src/services/model.js', () => modelMocks)

const modelModelMocks = vi.hoisted(() => {
const obj: any = { settings: { mirror: { sourceModelId: '' } } }

obj.aggregate = vi.fn(() => obj)
obj.match = vi.fn(() => obj)
obj.sort = vi.fn(() => obj)
obj.lookup = vi.fn(() => obj)
obj.append = vi.fn(() => obj)
obj.find = vi.fn(() => obj)
obj.findOne = vi.fn(() => obj)
obj.findOneAndUpdate = vi.fn(() => obj)
obj.updateOne = vi.fn(() => obj)
obj.save = vi.fn(() => obj)
obj.findByIdAndUpdate = vi.fn(() => obj)

const model: any = vi.fn(() => obj)
Object.assign(model, obj)

return model
})
vi.mock('../../src/models/Model.js', () => ({ default: modelModelMocks }))

const schemaMocks = vi.hoisted(() => ({
getSchemaById: vi.fn(),
}))
vi.mock('../../src/services/schema.js', () => schemaMocks)

const mockAuthentication = vi.hoisted(() => ({
hasRole: vi.fn(),
getEntities: vi.fn(() => ['user:testUser']),
}))
vi.mock('../../src/connectors/authentication/index.js', async () => ({ default: mockAuthentication }))

const accessRequestModelMocks = vi.hoisted(() => {
const obj: any = {}

Expand Down Expand Up @@ -135,6 +164,41 @@ describe('services > accessRequest', () => {
expect(accessRequests).toMatchSnapshot()
})

test('findAccessRequest > no filters', async () => {
mockAuthentication.hasRole.mockReturnValueOnce(false)
accessRequestModelMocks.find.mockResolvedValue([{ _id: 'a' }])
modelModelMocks.findOne.mockResolvedValueOnce({
_id: 'a',
toObject: vi.fn(() => ({ _id: 'a' })),
})

const accessRequests = await findAccessRequest({} as any, [], '', [], false)
expect(accessRequests).toMatchSnapshot()
})

test('findAccessRequest > all filters', async () => {
mockAuthentication.hasRole.mockReturnValueOnce(false)
accessRequestModelMocks.find.mockResolvedValue([])

const accessRequests = await findAccessRequest({} as any, ['modelId'], 'schemaId', ['filter'], false)
expect(accessRequests).toMatchSnapshot()
})

test('findAccessRequest > admin access without auth', async () => {
mockAuthentication.hasRole.mockReturnValueOnce(false)
await expect(() => findAccessRequest({} as any, [], '', [], true)).rejects.toThrowError(
/^You do not have the required role./,
)
})

test('findAccessRequest > admin access with auth', async () => {
mockAuthentication.hasRole.mockReturnValueOnce(true)
accessRequestModelMocks.find.mockResolvedValue([])

const accessRequests = await findAccessRequest({} as any, ['modelId'], 'schemaId', ['filter'], true)
expect(accessRequests).toMatchSnapshot()
})

test('removeAccessRequest > success', async () => {
reviewModelMocks.find.mockResolvedValue([])
responseModelMocks.find.mockResolvedValue([])
Expand Down
Loading