From 6bb78a6ef29e483ef2ade2ca78f16e7911ca1972 Mon Sep 17 00:00:00 2001 From: Federico Ruggi <1081051+ruggi@users.noreply.github.com> Date: Thu, 28 Mar 2024 17:40:49 +0100 Subject: [PATCH 1/2] destroy access request --- .../projectAccessRequest.server.spec.ts | 96 +++++++++++++++++++ .../app/models/projectAccessRequest.server.ts | 23 +++++ ....$id.access.request.$token.destroy.spec.ts | 70 ++++++++++++++ ...ects.$id.access.request.$token.destroy.tsx | 36 +++++++ 4 files changed, 225 insertions(+) create mode 100644 utopia-remix/app/routes-test/internal.projects.$id.access.request.$token.destroy.spec.ts create mode 100644 utopia-remix/app/routes/internal.projects.$id.access.request.$token.destroy.tsx diff --git a/utopia-remix/app/models/projectAccessRequest.server.spec.ts b/utopia-remix/app/models/projectAccessRequest.server.spec.ts index e1e2dd9887e4..64889bc80fb3 100644 --- a/utopia-remix/app/models/projectAccessRequest.server.spec.ts +++ b/utopia-remix/app/models/projectAccessRequest.server.spec.ts @@ -9,6 +9,7 @@ import { import { AccessRequestStatus, UserProjectRole } from '../types' import { createAccessRequest, + destroyAccessRequest, listProjectAccessRequests, updateAccessRequestStatus, } from './projectAccessRequest.server' @@ -327,4 +328,99 @@ describe('projectAccessRequest', () => { expect(got[3].User?.name).toBe('person4') }) }) + + describe('destroyAccessRequest', () => { + let revokeAllRolesFromUserMock: jest.SpyInstance + + afterEach(async () => { + await truncateTables([ + prisma.projectID, + prisma.projectAccessRequest, + prisma.projectCollaborator, + prisma.project, + prisma.userDetails, + ]) + + revokeAllRolesFromUserMock.mockRestore() + }) + + beforeEach(async () => { + await createTestUser(prisma, { id: 'bob' }) + await createTestUser(prisma, { id: 'alice' }) + await createTestUser(prisma, { id: 'carol' }) + await createTestUser(prisma, { id: 'dorothy' }) + await createTestProject(prisma, { id: 'one', ownerId: 'bob' }) + await createTestProject(prisma, { id: 'two', ownerId: 'alice' }) + await createTestProject(prisma, { id: 'three', ownerId: 'alice' }) + await createTestProjectAccessRequest(prisma, { + projectId: 'two', + token: 'token1', + userId: 'carol', + status: AccessRequestStatus.PENDING, + }) + await createTestProjectAccessRequest(prisma, { + projectId: 'three', + token: 'token2', + userId: 'dorothy', + status: AccessRequestStatus.PENDING, + }) + await createTestProjectAccessRequest(prisma, { + projectId: 'two', + token: 'token3', + userId: 'bob', + status: AccessRequestStatus.PENDING, + }) + + revokeAllRolesFromUserMock = jest.spyOn(permissionsService, 'revokeAllRolesFromUser') + }) + it('errors if the project is not found', async () => { + const fn = async () => + destroyAccessRequest({ projectId: 'UNKNOWN', ownerId: 'bob', token: 'token1' }) + await expect(fn).rejects.toThrow('Record to delete does not exist') + }) + it('errors if the project is not owned by the user', async () => { + const fn = async () => + destroyAccessRequest({ projectId: 'two', ownerId: 'bob', token: 'token1' }) + await expect(fn).rejects.toThrow('Record to delete does not exist') + }) + it('errors if the token is not found', async () => { + const fn = async () => + destroyAccessRequest({ projectId: 'two', ownerId: 'alice', token: 'tokenWRONG' }) + await expect(fn).rejects.toThrow('Record to delete does not exist') + }) + it('errors if the token exists but not on that project', async () => { + const fn = async () => + destroyAccessRequest({ projectId: 'two', ownerId: 'alice', token: 'token2' }) + await expect(fn).rejects.toThrow('Record to delete does not exist') + }) + it('deletes the request and revokes roles on FGA', async () => { + const existing = await prisma.projectAccessRequest.findMany({ where: { project_id: 'two' } }) + expect(existing.length).toBe(2) + await destroyAccessRequest({ projectId: 'two', ownerId: 'alice', token: 'token1' }) + const current = await prisma.projectAccessRequest.findMany({ + where: { project_id: 'two' }, + }) + expect(current.length).toBe(1) + expect(current[0].user_id).toBe('bob') + + expect(revokeAllRolesFromUserMock).toHaveBeenCalledWith('two', 'carol') + }) + it('rolls back if the FGA revoking fails', async () => { + revokeAllRolesFromUserMock.mockImplementation(() => { + throw new Error('boom') + }) + + const existing = await prisma.projectAccessRequest.findMany({ where: { project_id: 'two' } }) + expect(existing.length).toBe(2) + + const fn = async () => + await destroyAccessRequest({ projectId: 'two', ownerId: 'alice', token: 'token1' }) + await expect(fn).rejects.toThrow('boom') + + const current = await prisma.projectAccessRequest.findMany({ + where: { project_id: 'two' }, + }) + expect(current.length).toBe(2) + }) + }) }) diff --git a/utopia-remix/app/models/projectAccessRequest.server.ts b/utopia-remix/app/models/projectAccessRequest.server.ts index ed426632885f..4f0064f148fe 100644 --- a/utopia-remix/app/models/projectAccessRequest.server.ts +++ b/utopia-remix/app/models/projectAccessRequest.server.ts @@ -144,3 +144,26 @@ export async function listProjectAccessRequests(params: { User: users.find((u) => u.user_id === r.user_id) ?? null, })) } + +export async function destroyAccessRequest(params: { + projectId: string + ownerId: string + token: string +}) { + await prisma.$transaction(async (tx) => { + // 1. delete the access request from the DB + const request = await tx.projectAccessRequest.delete({ + where: { + project_id_token: { + project_id: params.projectId, + token: params.token, + }, + Project: { + owner_id: params.ownerId, + }, + }, + }) + // 2. revoke access on FGA + await permissionsService.revokeAllRolesFromUser(params.projectId, request.user_id) + }) +} diff --git a/utopia-remix/app/routes-test/internal.projects.$id.access.request.$token.destroy.spec.ts b/utopia-remix/app/routes-test/internal.projects.$id.access.request.$token.destroy.spec.ts new file mode 100644 index 000000000000..0cb25dd33488 --- /dev/null +++ b/utopia-remix/app/routes-test/internal.projects.$id.access.request.$token.destroy.spec.ts @@ -0,0 +1,70 @@ +import { prisma } from '../db.server' +import { handleDestroyAccessRequest } from '../routes/internal.projects.$id.access.request.$token.destroy' +import { + createTestProject, + createTestProjectAccessRequest, + createTestSession, + createTestUser, + newTestRequest, + truncateTables, +} from '../test-util' +import { AccessRequestStatus } from '../types' + +describe('handleDestroyAccessRequest', () => { + afterEach(async () => { + await truncateTables([ + prisma.projectID, + prisma.projectAccessRequest, + prisma.projectAccess, + prisma.persistentSession, + prisma.project, + prisma.userDetails, + ]) + }) + + beforeEach(async () => { + await createTestUser(prisma, { id: 'bob' }) + await createTestUser(prisma, { id: 'alice' }) + await createTestUser(prisma, { id: 'carol' }) + await createTestSession(prisma, { userId: 'bob', key: 'the-key' }) + await createTestProject(prisma, { id: 'one', ownerId: 'bob' }) + await createTestProjectAccessRequest(prisma, { + projectId: 'one', + userId: 'alice', + status: AccessRequestStatus.PENDING, + token: 'alice-token', + }) + await createTestProjectAccessRequest(prisma, { + projectId: 'one', + userId: 'carol', + status: AccessRequestStatus.PENDING, + token: 'carol-token', + }) + }) + + it('requires a user', async () => { + const fn = async () => handleDestroyAccessRequest(newTestRequest(), {}) + await expect(fn).rejects.toThrow('missing session cookie') + }) + + it('requires a project id', async () => { + const fn = async () => handleDestroyAccessRequest(newTestRequest({ authCookie: 'the-key' }), {}) + await expect(fn).rejects.toThrow('invalid project id') + }) + + it('requires a token', async () => { + const fn = async () => + handleDestroyAccessRequest(newTestRequest({ authCookie: 'the-key' }), { id: 'one' }) + await expect(fn).rejects.toThrow('invalid token') + }) + + it('destroys the access request', async () => { + await handleDestroyAccessRequest(newTestRequest({ authCookie: 'the-key' }), { + id: 'one', + token: 'alice-token', + }) + const reqs = await prisma.projectAccessRequest.findMany({ where: { project_id: 'one' } }) + expect(reqs.length).toBe(1) + expect(reqs[0].user_id).toBe('carol') + }) +}) diff --git a/utopia-remix/app/routes/internal.projects.$id.access.request.$token.destroy.tsx b/utopia-remix/app/routes/internal.projects.$id.access.request.$token.destroy.tsx new file mode 100644 index 000000000000..e06919966d56 --- /dev/null +++ b/utopia-remix/app/routes/internal.projects.$id.access.request.$token.destroy.tsx @@ -0,0 +1,36 @@ +import type { ActionFunctionArgs } from '@remix-run/node' +import type { Params } from '@remix-run/react' +import { validateProjectAccess } from '../handlers/validators' +import { UserProjectPermission } from '../types' +import { ensure, handle, requireUser } from '../util/api.server' +import { Status } from '../util/statusCodes' +import { destroyAccessRequest } from '../models/projectAccessRequest.server' + +export async function action(args: ActionFunctionArgs) { + return handle(args, { + POST: { + handler: handleDestroyAccessRequest, + validator: validateProjectAccess(UserProjectPermission.CAN_MANAGE_PROJECT, { + getProjectId: (params) => params.id, + }), + }, + }) +} + +export async function handleDestroyAccessRequest(req: Request, params: Params) { + const user = await requireUser(req) + + const projectId = params.id + ensure(projectId != null, 'invalid project id', Status.BAD_REQUEST) + + const token = params.token + ensure(token != null && typeof token === 'string', 'invalid token', Status.BAD_REQUEST) + + await destroyAccessRequest({ + projectId: projectId, + ownerId: user.user_id, + token: token, + }) + + return {} +} From b9aa7a6d731d8b65488523593004a80f2cf18dd1 Mon Sep 17 00:00:00 2001 From: Federico Ruggi <1081051+ruggi@users.noreply.github.com> Date: Fri, 29 Mar 2024 11:37:00 +0100 Subject: [PATCH 2/2] action test --- ....$id.access.request.$token.destroy.spec.ts | 78 ++++++++++++++++--- 1 file changed, 66 insertions(+), 12 deletions(-) diff --git a/utopia-remix/app/routes-test/internal.projects.$id.access.request.$token.destroy.spec.ts b/utopia-remix/app/routes-test/internal.projects.$id.access.request.$token.destroy.spec.ts index 0cb25dd33488..8b3844b2eced 100644 --- a/utopia-remix/app/routes-test/internal.projects.$id.access.request.$token.destroy.spec.ts +++ b/utopia-remix/app/routes-test/internal.projects.$id.access.request.$token.destroy.spec.ts @@ -1,5 +1,6 @@ +import type { Params } from '@remix-run/react' import { prisma } from '../db.server' -import { handleDestroyAccessRequest } from '../routes/internal.projects.$id.access.request.$token.destroy' +import { action } from '../routes/internal.projects.$id.access.request.$token.destroy' import { createTestProject, createTestProjectAccessRequest, @@ -9,6 +10,8 @@ import { truncateTables, } from '../test-util' import { AccessRequestStatus } from '../types' +import type { ApiResponse } from '../util/api.server' +import { Status } from '../util/statusCodes' describe('handleDestroyAccessRequest', () => { afterEach(async () => { @@ -28,6 +31,7 @@ describe('handleDestroyAccessRequest', () => { await createTestUser(prisma, { id: 'carol' }) await createTestSession(prisma, { userId: 'bob', key: 'the-key' }) await createTestProject(prisma, { id: 'one', ownerId: 'bob' }) + await createTestProject(prisma, { id: 'two', ownerId: 'alice' }) await createTestProjectAccessRequest(prisma, { projectId: 'one', userId: 'alice', @@ -42,29 +46,79 @@ describe('handleDestroyAccessRequest', () => { }) }) - it('requires a user', async () => { - const fn = async () => handleDestroyAccessRequest(newTestRequest(), {}) - await expect(fn).rejects.toThrow('missing session cookie') + it('requires a project id', async () => { + const got = await getActionResult(newTestRequest({ method: 'POST' }), {}) + expect(got).toEqual({ + message: 'Invalid project id', + status: Status.BAD_REQUEST, + error: 'Error', + }) }) - it('requires a project id', async () => { - const fn = async () => handleDestroyAccessRequest(newTestRequest({ authCookie: 'the-key' }), {}) - await expect(fn).rejects.toThrow('invalid project id') + it('requires an existing project', async () => { + const got = await getActionResult( + newTestRequest({ method: 'POST', authCookie: 'alice-token' }), + { + id: 'WRONG', + }, + ) + expect(got).toEqual({ + message: 'Project not found', + status: Status.NOT_FOUND, + error: 'Error', + }) + }) + + it('requires a valid auth cookie', async () => { + const got = await getActionResult(newTestRequest({ method: 'POST' }), { id: 'one' }) + expect(got).toEqual({ + message: 'Project not found', + status: Status.NOT_FOUND, + error: 'Error', + }) + }) + + it('requires an accessible project', async () => { + const got = await getActionResult(newTestRequest({ method: 'POST', authCookie: 'the-key' }), { + id: 'two', + }) + expect(got).toEqual({ + message: 'Project not found', + status: Status.NOT_FOUND, + error: 'Error', + }) }) - it('requires a token', async () => { - const fn = async () => - handleDestroyAccessRequest(newTestRequest({ authCookie: 'the-key' }), { id: 'one' }) - await expect(fn).rejects.toThrow('invalid token') + it('requires a request token', async () => { + const got = await getActionResult(newTestRequest({ method: 'POST', authCookie: 'the-key' }), { + id: 'one', + }) + expect(got).toEqual({ + message: 'invalid token', + status: Status.BAD_REQUEST, + error: 'Error', + }) }) it('destroys the access request', async () => { - await handleDestroyAccessRequest(newTestRequest({ authCookie: 'the-key' }), { + const got = await getActionResult(newTestRequest({ method: 'POST', authCookie: 'the-key' }), { id: 'one', token: 'alice-token', }) + expect(got).toEqual({}) + const reqs = await prisma.projectAccessRequest.findMany({ where: { project_id: 'one' } }) expect(reqs.length).toBe(1) expect(reqs[0].user_id).toBe('carol') }) }) + +// TODO it would be good to make this a separate reausable helper that supports both actions and loaders +async function getActionResult(req: Request, params: Params) { + const response = await (action({ + request: req, + params: params, + context: {}, + }) as Promise>>) + return await response.json() +}