fix(j-s): Roles Rules (#15416)

* Fixes and improves roles rules logic * Fixes typo --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
island-is · Jul 3, 2024 · dc2ec0a · dc2ec0a
1 parent 38fe437
commit dc2ec0a
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 86 deletions.
diff --git a/apps/judicial-system/backend/src/app/modules/case/case.controller.ts b/apps/judicial-system/backend/src/app/modules/case/case.controller.ts
@@ -84,6 +84,7 @@ import {
   courtOfAppealsRegistrarUpdateRule,
   districtCourtAssistantTransitionRule,
   districtCourtAssistantUpdateRule,
+  districtCourtJudgeSignRulingRule,
   districtCourtJudgeTransitionRule,
   districtCourtJudgeUpdateRule,
   districtCourtRegistrarTransitionRule,
@@ -297,12 +298,6 @@ export class CaseController {
         break
       case CaseTransition.SUBMIT:
         if (isIndictmentCase(theCase.type)) {
-          if (!user.canConfirmIndictment) {
-            throw new ForbiddenException(
-              `User ${user.id} does not have permission to confirm indictments`,
-            )
-          }
-
           update.indictmentDeniedExplanation = null
         }
         break
@@ -387,13 +382,6 @@ export class CaseController {
           update.appealRulingDecision = CaseAppealRulingDecision.DISCONTINUED
         }
         break
-      case CaseTransition.DENY_INDICTMENT:
-        if (!user.canConfirmIndictment) {
-          throw new ForbiddenException(
-            `User ${user.id} does not have permission to reject indictments`,
-          )
-        }
-        break
       case CaseTransition.ASK_FOR_CONFIRMATION:
         update.indictmentReturnedExplanation = null
         break
@@ -777,7 +765,7 @@ export class CaseController {
     new CaseTypeGuard([...restrictionCases, ...investigationCases]),
     CaseWriteGuard,
   )
-  @RolesRules(districtCourtJudgeRule)
+  @RolesRules(districtCourtJudgeSignRulingRule)
   @Post('case/:caseId/ruling/signature')
   @ApiCreatedResponse({
     type: SigningServiceResponse,
@@ -790,12 +778,6 @@ export class CaseController {
   ): Promise<SigningServiceResponse> {
     this.logger.debug(`Requesting a signature for the ruling of case ${caseId}`)
 
-    if (user.id !== theCase.judgeId) {
-      throw new ForbiddenException(
-        'A ruling must be signed by the assigned judge',
-      )
-    }
-
     return this.caseService.requestRulingSignature(theCase).catch((error) => {
       if (error instanceof DokobitError) {
         throw new HttpException(
@@ -820,7 +802,7 @@ export class CaseController {
     new CaseTypeGuard([...restrictionCases, ...investigationCases]),
     CaseWriteGuard,
   )
-  @RolesRules(districtCourtJudgeRule)
+  @RolesRules(districtCourtJudgeSignRulingRule)
   @Get('case/:caseId/ruling/signature')
   @ApiOkResponse({
     type: SignatureConfirmationResponse,
@@ -835,12 +817,6 @@ export class CaseController {
   ): Promise<SignatureConfirmationResponse> {
     this.logger.debug(`Confirming a signature for the ruling of case ${caseId}`)
 
-    if (user.id !== theCase.judgeId) {
-      throw new ForbiddenException(
-        'A ruling must be signed by the assigned judge',
-      )
-    }
-
     return this.caseService.getRulingSignatureConfirmation(
       theCase,
       user,

diff --git a/apps/judicial-system/backend/src/app/modules/case/guards/rolesRules.ts b/apps/judicial-system/backend/src/app/modules/case/guards/rolesRules.ts
@@ -1,6 +1,12 @@
 import { RolesRule, RulesType } from '@island.is/judicial-system/auth'
-import { CaseTransition, UserRole } from '@island.is/judicial-system/types'
+import {
+  CaseTransition,
+  CaseType,
+  User,
+  UserRole,
+} from '@island.is/judicial-system/types'
 
+import { TransitionCaseDto } from '../dto/transitionCase.dto'
 import { UpdateCaseDto } from '../dto/updateCase.dto'
 import { Case } from '../models/case.model'
 
@@ -189,29 +195,39 @@ export const prosecutorTransitionRule: RolesRule = {
   dtoFieldValues: [
     CaseTransition.OPEN,
     CaseTransition.ASK_FOR_CONFIRMATION,
+    CaseTransition.DENY_INDICTMENT,
     CaseTransition.SUBMIT,
     CaseTransition.ASK_FOR_CANCELLATION,
     CaseTransition.DELETE,
     CaseTransition.APPEAL,
     CaseTransition.WITHDRAW_APPEAL,
-    CaseTransition.DENY_INDICTMENT,
   ],
   canActivate: (request) => {
+    const user: User = request.user
+    const dto: TransitionCaseDto = request.body
     const theCase: Case = request.case
 
-    // Deny if the case is missing - shuould never happen
-    if (!theCase) {
+    // Deny if something is missing - shuould never happen
+    if (!user || !dto || !theCase) {
       return false
     }
 
     // Deny transition if prosecutor did not appeal the case
     if (
-      request.body.transition === CaseTransition.WITHDRAW_APPEAL &&
+      dto.transition === CaseTransition.WITHDRAW_APPEAL &&
       !theCase.prosecutorPostponedAppealDate
     ) {
       return false
     }
 
+    if (
+      (dto.transition === CaseTransition.SUBMIT &&
+        theCase.type === CaseType.INDICTMENT) ||
+      dto.transition === CaseTransition.DENY_INDICTMENT
+    ) {
+      return user.canConfirmIndictment
+    }
+
     return true
   },
 }
@@ -221,7 +237,11 @@ export const prosecutorRepresentativeTransitionRule: RolesRule = {
   role: UserRole.PROSECUTOR_REPRESENTATIVE,
   type: RulesType.FIELD_VALUES,
   dtoField: 'transition',
-  dtoFieldValues: [CaseTransition.ASK_FOR_CONFIRMATION, CaseTransition.DELETE],
+  dtoFieldValues: [
+    CaseTransition.ASK_FOR_CONFIRMATION,
+    CaseTransition.ASK_FOR_CANCELLATION,
+    CaseTransition.DELETE,
+  ],
 }
 
 // Allows defenders to transition cases
@@ -231,16 +251,17 @@ export const defenderTransitionRule: RolesRule = {
   dtoField: 'transition',
   dtoFieldValues: [CaseTransition.APPEAL, CaseTransition.WITHDRAW_APPEAL],
   canActivate: (request) => {
+    const dto: TransitionCaseDto = request.body
     const theCase: Case = request.case
 
-    // Deny if the case is missing - should never happen
-    if (!theCase) {
+    // Deny if something is missing - shuould never happen
+    if (!dto || !theCase) {
       return false
     }
 
     // Deny withdrawal if defender did not appeal the case
     if (
-      request.body.transition === CaseTransition.WITHDRAW_APPEAL &&
+      dto.transition === CaseTransition.WITHDRAW_APPEAL &&
       !theCase.accusedPostponedAppealDate
     ) {
       return false
@@ -283,15 +304,15 @@ export const districtCourtRegistrarTransitionRule: RolesRule = {
   ],
 }
 
-// Allows district court assistants to transition cases.
+// Allows district court assistants to transition cases
 export const districtCourtAssistantTransitionRule: RolesRule = {
   role: UserRole.DISTRICT_COURT_ASSISTANT,
   type: RulesType.FIELD_VALUES,
   dtoField: 'transition',
   dtoFieldValues: [CaseTransition.RECEIVE, CaseTransition.COMPLETE],
 }
 
-// Allows court of appeals judges to transition cases.
+// Allows court of appeals judges to transition cases
 export const courtOfAppealsJudgeTransitionRule: RolesRule = {
   role: UserRole.COURT_OF_APPEALS_JUDGE,
   type: RulesType.FIELD_VALUES,
@@ -302,7 +323,7 @@ export const courtOfAppealsJudgeTransitionRule: RolesRule = {
   ],
 }
 
-// Allows court of appeals registrars to transition cases.
+// Allows court of appeals registrars to transition cases
 export const courtOfAppealsRegistrarTransitionRule: RolesRule = {
   role: UserRole.COURT_OF_APPEALS_REGISTRAR,
   type: RulesType.FIELD_VALUES,
@@ -313,7 +334,7 @@ export const courtOfAppealsRegistrarTransitionRule: RolesRule = {
   ],
 }
 
-// Allows court of appeals assistants to transition cases.
+// Allows court of appeals assistants to transition cases
 export const courtOfAppealsAssistantTransitionRule: RolesRule = {
   role: UserRole.COURT_OF_APPEALS_ASSISTANT,
   type: RulesType.FIELD_VALUES,
@@ -323,3 +344,20 @@ export const courtOfAppealsAssistantTransitionRule: RolesRule = {
     CaseTransition.REOPEN_APPEAL,
   ],
 }
+
+// Allows district court judges to sign a ruling
+export const districtCourtJudgeSignRulingRule: RolesRule = {
+  role: UserRole.DISTRICT_COURT_JUDGE,
+  type: RulesType.BASIC,
+  canActivate: (request) => {
+    const user: User = request.user
+    const theCase: Case = request.case
+
+    // Deny if something is missing - shuould never happen
+    if (!user || !theCase) {
+      return false
+    }
+
+    return user.id === theCase.judgeId
+  },
+}
diff --git a/...m/backend/src/app/modules/case/test/caseController/getRulingSignatureConfirmation.spec.ts b/...m/backend/src/app/modules/case/test/caseController/getRulingSignatureConfirmation.spec.ts
@@ -1,8 +1,6 @@
 import { Transaction } from 'sequelize/types'
 import { uuid } from 'uuidv4'
 
-import { ForbiddenException } from '@nestjs/common'
-
 import { MessageService, MessageType } from '@island.is/judicial-system/message'
 import {
   CaseFileState,
@@ -212,25 +210,6 @@ describe('CaseController - Get ruling signature confirmation', () => {
       ])
     })
   })
-
-  describe('user is not the assigned judge', () => {
-    const caseId = uuid()
-    const theCase = { id: caseId, judgeId: uuid() } as Case
-    const documentToken = uuid()
-    let then: Then
-
-    beforeEach(async () => {
-      then = await givenWhenThen(caseId, user, theCase, documentToken)
-    })
-
-    it('should throw ForbiddenException', () => {
-      expect(then.error).toBeInstanceOf(ForbiddenException)
-      expect(then.error.message).toBe(
-        'A ruling must be signed by the assigned judge',
-      )
-    })
-  })
-
   describe('database update fails', () => {
     const caseId = uuid()
     const theCase = {

diff --git a/...src/app/modules/case/test/caseController/getRulingSignatureConfirmationRolesRules.spec.ts b/...src/app/modules/case/test/caseController/getRulingSignatureConfirmationRolesRules.spec.ts
@@ -1,5 +1,5 @@
-import { districtCourtJudgeRule } from '../../../../guards'
 import { CaseController } from '../../case.controller'
+import { districtCourtJudgeSignRulingRule } from '../../guards/rolesRules'
 
 describe('CaseController - Get ruling signature confirmation rules', () => {
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -14,6 +14,6 @@ describe('CaseController - Get ruling signature confirmation rules', () => {
 
   it('should give permission to one roles', () => {
     expect(rules).toHaveLength(1)
-    expect(rules).toContain(districtCourtJudgeRule)
+    expect(rules).toContain(districtCourtJudgeSignRulingRule)
   })
 })
diff --git a/...al-system/backend/src/app/modules/case/test/caseController/requestRulingSignature.spec.ts b/...al-system/backend/src/app/modules/case/test/caseController/requestRulingSignature.spec.ts
@@ -1,7 +1,5 @@
 import { uuid } from 'uuidv4'
 
-import { ForbiddenException } from '@nestjs/common'
-
 import { SigningServiceResponse } from '@island.is/dokobit-signing'
 
 import { User } from '@island.is/judicial-system/types'
@@ -44,7 +42,7 @@ describe('CaseController - Request ruling signature', () => {
     }
   })
 
-  describe('the user is the assigned judge', () => {
+  describe('signature requested', () => {
     const userId = uuid()
     const user = { id: userId } as User
     const caseId = uuid()
@@ -62,22 +60,4 @@ describe('CaseController - Request ruling signature', () => {
       })
     })
   })
-
-  describe('the user is not the assigned judge', () => {
-    const user = { id: uuid() } as User
-    const caseId = uuid()
-    const theCase = { id: caseId, judgeId: uuid() } as Case
-    let then: Then
-
-    beforeEach(async () => {
-      then = await givenWhenThen(caseId, user, theCase)
-    })
-
-    it('should throw ForbiddenException', () => {
-      expect(then.error).toBeInstanceOf(ForbiddenException)
-      expect(then.error.message).toBe(
-        'A ruling must be signed by the assigned judge',
-      )
-    })
-  })
 })
diff --git a/...backend/src/app/modules/case/test/caseController/requestRulingSignatureRolesRules.spec.ts b/...backend/src/app/modules/case/test/caseController/requestRulingSignatureRolesRules.spec.ts
@@ -1,5 +1,5 @@
-import { districtCourtJudgeRule } from '../../../../guards'
 import { CaseController } from '../../case.controller'
+import { districtCourtJudgeSignRulingRule } from '../../guards/rolesRules'
 
 describe('CaseController - Request ruling signature rules', () => {
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -14,6 +14,6 @@ describe('CaseController - Request ruling signature rules', () => {
 
   it('should give permission to one roles', () => {
     expect(rules).toHaveLength(1)
-    expect(rules).toContain(districtCourtJudgeRule)
+    expect(rules).toContain(districtCourtJudgeSignRulingRule)
   })
 })