From f8339e7850091a4d6b76a6fc0727796cb53c9929 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gu=C3=B0j=C3=B3n=20Gu=C3=B0j=C3=B3nsson?= Date: Tue, 4 Jun 2024 09:53:29 +0000 Subject: [PATCH 1/2] Fixes defender case file access in indictments --- .../src/app/modules/case/case.service.ts | 10 ++-- .../modules/case/limitedAccessCase.service.ts | 11 ++++ .../guards/limitedAccessViewCaseFile.guard.ts | 51 ++++++++++--------- .../limitedAccessViewCaseFileGuard.spec.ts | 32 +----------- .../file/limitedAccessFile.controller.ts | 12 ++--- 5 files changed, 48 insertions(+), 68 deletions(-) diff --git a/apps/judicial-system/backend/src/app/modules/case/case.service.ts b/apps/judicial-system/backend/src/app/modules/case/case.service.ts index 36465195d58d..408ad4b46bd6 100644 --- a/apps/judicial-system/backend/src/app/modules/case/case.service.ts +++ b/apps/judicial-system/backend/src/app/modules/case/case.service.ts @@ -245,6 +245,11 @@ export const include: Includeable[] = [ as: 'appealJudge3', include: [{ model: Institution, as: 'institution' }], }, + { + model: User, + as: 'indictmentReviewer', + include: [{ model: Institution, as: 'institution' }], + }, { model: Case, as: 'parentCase', @@ -289,11 +294,6 @@ export const include: Includeable[] = [ where: { commentType: { [Op.in]: commentTypes } }, }, { model: Notification, as: 'notifications' }, - { - model: User, - as: 'indictmentReviewer', - include: [{ model: Institution, as: 'institution' }], - }, ] export const order: OrderItem[] = [ diff --git a/apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.service.ts b/apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.service.ts index 10ddc4008743..91db06615ba5 100644 --- a/apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.service.ts +++ b/apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.service.ts @@ -26,6 +26,7 @@ import { CaseState, CommentType, DateType, + EventType, NotificationType, UserRole, } from '@island.is/judicial-system/types' @@ -33,6 +34,7 @@ import { import { nowFactory, uuidFactory } from '../../factories' import { AwsS3Service } from '../aws-s3' import { Defendant, DefendantService } from '../defendant' +import { EventLog } from '../event-log' import { CaseFile, defenderCaseFileCategoriesForRestrictionAndInvestigationCases, @@ -110,6 +112,7 @@ export interface LimitedAccessUpdateCase | 'appealRulingDecision' > {} +const eventTypes = Object.values(EventType) const dateTypes = Object.values(DateType) const commentTypes = Object.values(CommentType) @@ -185,6 +188,14 @@ export const include: Includeable[] = [ ], }, }, + { + model: EventLog, + as: 'eventLogs', + required: false, + where: { eventType: { [Op.in]: eventTypes } }, + order: [['created', 'ASC']], + separate: true, + }, { model: DateLog, as: 'dateLogs', diff --git a/apps/judicial-system/backend/src/app/modules/file/guards/limitedAccessViewCaseFile.guard.ts b/apps/judicial-system/backend/src/app/modules/file/guards/limitedAccessViewCaseFile.guard.ts index 0fa6e6fda3ce..3526675d6902 100644 --- a/apps/judicial-system/backend/src/app/modules/file/guards/limitedAccessViewCaseFile.guard.ts +++ b/apps/judicial-system/backend/src/app/modules/file/guards/limitedAccessViewCaseFile.guard.ts @@ -11,9 +11,8 @@ import { isCompletedCase, isDefenceUser, isIndictmentCase, - isInvestigationCase, isPrisonSystemUser, - isRestrictionCase, + isRequestCase, User, } from '@island.is/judicial-system/types' @@ -47,30 +46,32 @@ export class LimitedAccessViewCaseFileGuard implements CanActivate { throw new InternalServerErrorException('Missing case file') } - if (isCompletedCase(theCase.state) && caseFile.category) { - if (isDefenceUser(user)) { - if ( - (isRestrictionCase(theCase.type) || - isInvestigationCase(theCase.type)) && - defenderCaseFileCategoriesForRestrictionAndInvestigationCases.includes( - caseFile.category, - ) - ) { - return true - } + if (isDefenceUser(user) && caseFile.category) { + if ( + isRequestCase(theCase.type) && + isCompletedCase(theCase.state) && + defenderCaseFileCategoriesForRestrictionAndInvestigationCases.includes( + caseFile.category, + ) + ) { + return true + } + + if ( + isIndictmentCase(theCase.type) && + defenderCaseFileCategoriesForIndictmentCases.includes(caseFile.category) + ) { + return true + } + } - if ( - isIndictmentCase(theCase.type) && - defenderCaseFileCategoriesForIndictmentCases.includes( - caseFile.category, - ) - ) { - return true - } - } else if (isPrisonSystemUser(user)) { - if (caseFile.category === CaseFileCategory.APPEAL_RULING) { - return true - } + if (isPrisonSystemUser(user)) { + if ( + isCompletedCase(theCase.state) && + caseFile.category && + caseFile.category === CaseFileCategory.APPEAL_RULING + ) { + return true } } diff --git a/apps/judicial-system/backend/src/app/modules/file/guards/test/limitedAccessViewCaseFileGuard.spec.ts b/apps/judicial-system/backend/src/app/modules/file/guards/test/limitedAccessViewCaseFileGuard.spec.ts index dbe541817035..2deebedc9db4 100644 --- a/apps/judicial-system/backend/src/app/modules/file/guards/test/limitedAccessViewCaseFileGuard.spec.ts +++ b/apps/judicial-system/backend/src/app/modules/file/guards/test/limitedAccessViewCaseFileGuard.spec.ts @@ -150,7 +150,7 @@ describe('Limited Access View Case File Guard', () => { ) describe.each(indictmentCases)('for %s cases', (type) => { - describe.each(completedCaseStates)('in state %s', (state) => { + describe.each(Object.values(CaseState))('in state %s', (state) => { const allowedCaseFileCategories = [ CaseFileCategory.COURT_RECORD, CaseFileCategory.RULING, @@ -208,36 +208,6 @@ describe('Limited Access View Case File Guard', () => { }) }) }) - - describe.each( - Object.keys(CaseState).filter( - (state) => !completedCaseStates.includes(state as CaseState), - ), - )('in state %s', (state) => { - describe.each(Object.keys(CaseFileCategory))( - 'a defender can not view %s', - (category) => { - let then: Then - - beforeEach(() => { - mockRequest.mockImplementationOnce(() => ({ - user: { role: UserRole.DEFENDER }, - case: { type, state }, - caseFile: { category }, - })) - - then = givenWhenThen() - }) - - it('should throw ForbiddenException', () => { - expect(then.error).toBeInstanceOf(ForbiddenException) - expect(then.error.message).toBe( - `Forbidden for ${UserRole.DEFENDER}`, - ) - }) - }, - ) - }) }) }) diff --git a/apps/judicial-system/backend/src/app/modules/file/limitedAccessFile.controller.ts b/apps/judicial-system/backend/src/app/modules/file/limitedAccessFile.controller.ts index f62585f7be9f..3d8b09f65443 100644 --- a/apps/judicial-system/backend/src/app/modules/file/limitedAccessFile.controller.ts +++ b/apps/judicial-system/backend/src/app/modules/file/limitedAccessFile.controller.ts @@ -47,12 +47,7 @@ import { PresignedPost } from './models/presignedPost.model' import { SignedUrl } from './models/signedUrl.model' import { FileService } from './file.service' -@UseGuards( - JwtAuthGuard, - RolesGuard, - LimitedAccessCaseExistsGuard, - CaseCompletedGuard, -) +@UseGuards(JwtAuthGuard, RolesGuard, LimitedAccessCaseExistsGuard) @Controller('api/case/:caseId/limitedAccess') @ApiTags('files') export class LimitedAccessFileController { @@ -64,6 +59,7 @@ export class LimitedAccessFileController { @UseGuards( new CaseTypeGuard([...restrictionCases, ...investigationCases]), CaseWriteGuard, + CaseCompletedGuard, ) @RolesRules(defenderRule) @Post('file/url') @@ -84,6 +80,7 @@ export class LimitedAccessFileController { @UseGuards( new CaseTypeGuard([...restrictionCases, ...investigationCases]), CaseWriteGuard, + CaseCompletedGuard, LimitedAccessWriteCaseFileGuard, ) @RolesRules(defenderRule) @@ -92,7 +89,7 @@ export class LimitedAccessFileController { type: CaseFile, description: 'Creates a new case file', }) - async createCaseFile( + createCaseFile( @Param('caseId') caseId: string, @CurrentHttpUser() user: User, @CurrentCase() theCase: Case, @@ -126,6 +123,7 @@ export class LimitedAccessFileController { @UseGuards( new CaseTypeGuard([...restrictionCases, ...investigationCases]), CaseWriteGuard, + CaseCompletedGuard, CaseFileExistsGuard, LimitedAccessWriteCaseFileGuard, ) From 6df8e202f4f60d8ad5be44f95496a2b35df3a4e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gu=C3=B0j=C3=B3n=20Gu=C3=B0j=C3=B3nsson?= Date: Tue, 4 Jun 2024 11:35:05 +0000 Subject: [PATCH 2/2] Updates unit tests --- .../createCaseFileGuards.spec.ts | 11 ++++++++--- .../createPresignedPostGuards.spec.ts | 9 +++++++-- .../deleteCaseFileGuards.spec.ts | 13 +++++++++---- .../limitedAccessFileControllerGuards.spec.ts | 5 ++--- 4 files changed, 26 insertions(+), 12 deletions(-) diff --git a/apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/createCaseFileGuards.spec.ts b/apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/createCaseFileGuards.spec.ts index a3325a93e8d8..b18200a5cb3e 100644 --- a/apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/createCaseFileGuards.spec.ts +++ b/apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/createCaseFileGuards.spec.ts @@ -3,7 +3,11 @@ import { restrictionCases, } from '@island.is/judicial-system/types' -import { CaseTypeGuard, CaseWriteGuard } from '../../../case' +import { + CaseCompletedGuard, + CaseTypeGuard, + CaseWriteGuard, +} from '../../../case' import { LimitedAccessWriteCaseFileGuard } from '../../guards/limitedAccessWriteCaseFile.guard' import { LimitedAccessFileController } from '../../limitedAccessFile.controller' @@ -19,12 +23,13 @@ describe('LimitedAccessFileController - Create case file guards', () => { }) it('should have the right guard configuration', () => { - expect(guards).toHaveLength(3) + expect(guards).toHaveLength(4) expect(guards[0]).toBeInstanceOf(CaseTypeGuard) expect(guards[0]).toEqual({ allowedCaseTypes: [...restrictionCases, ...investigationCases], }) expect(new guards[1]()).toBeInstanceOf(CaseWriteGuard) - expect(new guards[2]()).toBeInstanceOf(LimitedAccessWriteCaseFileGuard) + expect(new guards[2]()).toBeInstanceOf(CaseCompletedGuard) + expect(new guards[3]()).toBeInstanceOf(LimitedAccessWriteCaseFileGuard) }) }) diff --git a/apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/createPresignedPostGuards.spec.ts b/apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/createPresignedPostGuards.spec.ts index b842f3d9592f..6483025331da 100644 --- a/apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/createPresignedPostGuards.spec.ts +++ b/apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/createPresignedPostGuards.spec.ts @@ -3,7 +3,11 @@ import { restrictionCases, } from '@island.is/judicial-system/types' -import { CaseTypeGuard, CaseWriteGuard } from '../../../case' +import { + CaseCompletedGuard, + CaseTypeGuard, + CaseWriteGuard, +} from '../../../case' import { LimitedAccessFileController } from '../../limitedAccessFile.controller' describe('LimitedAccessFileController - Create presigned post guards', () => { @@ -18,11 +22,12 @@ describe('LimitedAccessFileController - Create presigned post guards', () => { }) it('should have the right guard configuration', () => { - expect(guards).toHaveLength(2) + expect(guards).toHaveLength(3) expect(guards[0]).toBeInstanceOf(CaseTypeGuard) expect(guards[0]).toEqual({ allowedCaseTypes: [...restrictionCases, ...investigationCases], }) expect(new guards[1]()).toBeInstanceOf(CaseWriteGuard) + expect(new guards[2]()).toBeInstanceOf(CaseCompletedGuard) }) }) diff --git a/apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/deleteCaseFileGuards.spec.ts b/apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/deleteCaseFileGuards.spec.ts index 08fc526a564e..fb1e6b7eafb9 100644 --- a/apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/deleteCaseFileGuards.spec.ts +++ b/apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/deleteCaseFileGuards.spec.ts @@ -3,7 +3,11 @@ import { restrictionCases, } from '@island.is/judicial-system/types' -import { CaseTypeGuard, CaseWriteGuard } from '../../../case' +import { + CaseCompletedGuard, + CaseTypeGuard, + CaseWriteGuard, +} from '../../../case' import { CaseFileExistsGuard } from '../../guards/caseFileExists.guard' import { LimitedAccessWriteCaseFileGuard } from '../../guards/limitedAccessWriteCaseFile.guard' import { LimitedAccessFileController } from '../../limitedAccessFile.controller' @@ -20,13 +24,14 @@ describe('LimitedAccessFileController - Delete case file guards', () => { }) it('should have the right guard configuration', () => { - expect(guards).toHaveLength(4) + expect(guards).toHaveLength(5) expect(guards[0]).toBeInstanceOf(CaseTypeGuard) expect(guards[0]).toEqual({ allowedCaseTypes: [...restrictionCases, ...investigationCases], }) expect(new guards[1]()).toBeInstanceOf(CaseWriteGuard) - expect(new guards[2]()).toBeInstanceOf(CaseFileExistsGuard) - expect(new guards[3]()).toBeInstanceOf(LimitedAccessWriteCaseFileGuard) + expect(new guards[2]()).toBeInstanceOf(CaseCompletedGuard) + expect(new guards[3]()).toBeInstanceOf(CaseFileExistsGuard) + expect(new guards[4]()).toBeInstanceOf(LimitedAccessWriteCaseFileGuard) }) }) diff --git a/apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/limitedAccessFileControllerGuards.spec.ts b/apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/limitedAccessFileControllerGuards.spec.ts index 27731303484a..f38b68b9a607 100644 --- a/apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/limitedAccessFileControllerGuards.spec.ts +++ b/apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/limitedAccessFileControllerGuards.spec.ts @@ -1,6 +1,6 @@ import { JwtAuthGuard, RolesGuard } from '@island.is/judicial-system/auth' -import { CaseCompletedGuard, LimitedAccessCaseExistsGuard } from '../../../case' +import { LimitedAccessCaseExistsGuard } from '../../../case' import { LimitedAccessFileController } from '../../limitedAccessFile.controller' describe('LimitedAccessFileController - guards', () => { @@ -12,10 +12,9 @@ describe('LimitedAccessFileController - guards', () => { }) it('should have the right guard configuration', () => { - expect(guards).toHaveLength(4) + expect(guards).toHaveLength(3) expect(new guards[0]()).toBeInstanceOf(JwtAuthGuard) expect(new guards[1]()).toBeInstanceOf(RolesGuard) expect(new guards[2]()).toBeInstanceOf(LimitedAccessCaseExistsGuard) - expect(new guards[3]()).toBeInstanceOf(CaseCompletedGuard) }) })