fix(j-s): Defender Case File Access #15070
Conversation
WalkthroughThe changes primarily focus on refining access control and data retrieval logic within the judicial system's backend services. Key modifications include adjustments to user role and case type conditions, updates to guard configurations, and enhancements to data inclusion for case and event logging. These improvements aim to streamline authorization processes and ensure accurate data handling. Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant LimitedAccessViewCaseFileGuard
participant CaseService
participant FileController
User->>LimitedAccessViewCaseFileGuard: Request access to case file
LimitedAccessViewCaseFileGuard->>CaseService: Check user role and case type
CaseService-->>LimitedAccessViewCaseFileGuard: Return access status
LimitedAccessViewCaseFileGuard-->>User: Grant/Deny access
User->>FileController: Create/Update case file
FileController->>CaseCompletedGuard: Verify case completion
CaseCompletedGuard-->>FileController: Return verification result
FileController-->>User: Confirm action
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (5)
- apps/judicial-system/backend/src/app/modules/case/case.service.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.service.ts (3 hunks)
- apps/judicial-system/backend/src/app/modules/file/guards/limitedAccessViewCaseFile.guard.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/file/guards/test/limitedAccessViewCaseFileGuard.spec.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/file/limitedAccessFile.controller.ts (5 hunks)
Additional context used
Path-based instructions (5)
apps/judicial-system/backend/src/app/modules/file/guards/limitedAccessViewCaseFile.guard.ts (1)
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/backend/src/app/modules/file/limitedAccessFile.controller.ts (1)
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.service.ts (1)
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/backend/src/app/modules/file/guards/test/limitedAccessViewCaseFileGuard.spec.ts (1)
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/backend/src/app/modules/case/case.service.ts (1)
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
Additional comments not posted (7)
apps/judicial-system/backend/src/app/modules/file/guards/limitedAccessViewCaseFile.guard.ts (1)
49-66: Refined access logic for different user roles and case types looks good.Ensure that unit tests cover these new conditions to verify correct behavior.
#!/bin/bash # Description: Verify unit tests for `LimitedAccessViewCaseFileGuard`. # Test: Search for unit tests related to `LimitedAccessViewCaseFileGuard`. rg --type typescript $'describe\\("Limited Access View Case File Guard"'Also applies to: 68-74
apps/judicial-system/backend/src/app/modules/file/limitedAccessFile.controller.ts (3)
62-62: Addition ofCaseCompletedGuardtocreatePresignedPostmethod enhances security by ensuring operations are only performed on completed cases.#!/bin/bash # Description: Verify integration of `createPresignedPost` with the front-end. # Test: Search for front-end calls to `createPresignedPost`. rg --type typescript $'createPresignedPost'
83-83: Addition ofCaseCompletedGuardandLimitedAccessWriteCaseFileGuardtocreateCaseFilemethod correctly restricts file creation to appropriate cases and users.#!/bin/bash # Description: Verify integration of `createCaseFile` with the front-end. # Test: Search for front-end calls to `createCaseFile`. rg --type typescript $'createCaseFile'
126-126: Addition of multiple guards todeleteCaseFilemethod ensures secure and restricted file deletion in valid, completed cases.#!/bin/bash # Description: Verify integration of `deleteCaseFile` with the front-end. # Test: Search for front-end calls to `deleteCaseFile`. rg --type typescript $'deleteCaseFile'apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.service.ts (1)
29-29: Enhancements inlimitedAccessCase.service.tswith the addition ofEventType,EventLog, and related logic improve event handling capabilities.Ensure that unit tests cover these new functionalities to verify correct behavior.
#!/bin/bash # Description: Verify unit tests for `LimitedAccessCaseService`. # Test: Search for unit tests related to `LimitedAccessCaseService`. rg --type typescript $'describe\\("Limited Access Case Service"'Also applies to: 37-37, 115-198
apps/judicial-system/backend/src/app/modules/file/guards/test/limitedAccessViewCaseFileGuard.spec.ts (1)
153-153: Expansion of test coverage inlimitedAccessViewCaseFileGuard.spec.tsto include all values ofCaseStateis a positive change, ensuring comprehensive testing.#!/bin/bash # Description: Verify completeness of test coverage for `LimitedAccessViewCaseFileGuard`. # Test: Search for test cases covering all `CaseState` values. rg --type typescript $'it\\("should'apps/judicial-system/backend/src/app/modules/case/case.service.ts (1)
248-252: Duplicate entry forindictmentReviewerremoved from theincludearray.#!/bin/bash # Description: Verify that the removal of the duplicate `indictmentReviewer` entry does not affect other functionalities. # Test: Search for usage of `indictmentReviewer` in the application. Expect: No errors or missing data issues. rg --type ts 'indictmentReviewer'
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #15070 +/- ##
==========================================
+ Coverage 37.10% 37.12% +0.02%
==========================================
Files 6415 6406 -9
Lines 130585 130383 -202
Branches 37287 37195 -92
==========================================
- Hits 48452 48405 -47
+ Misses 82133 81978 -155
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 68 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 12 Total Test Services: 0 Failed, 12 Passed Test ServicesThis report shows up to 10 services
|
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (4)
apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/limitedAccessFileControllerGuards.spec.ts (1)
Line range hint
8-8: Replaceanywith a more specific type to enhance type safety.- let guards: any[] + let guards: GuardType[] // Assuming GuardType is the correct type, replace with the actual type used for guards.apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/createPresignedPostGuards.spec.ts (1)
Line range hint
15-15: Replaceanywith a more specific type to enhance type safety.- let guards: any[] + let guards: GuardType[] // Assuming GuardType is the correct type, replace with the actual type used for guards.apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/createCaseFileGuards.spec.ts (1)
Line range hint
16-16: Replaceanywith a more specific type to enhance type safety.- let guards: any[] + let guards: GuardType[] // Assuming GuardType is the correct type, replace with the actual type used for guards.apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/deleteCaseFileGuards.spec.ts (1)
Line range hint
17-17: Replaceanywith a more specific type to enhance type safety.- let guards: any[] + let guards: GuardType[] // Assuming GuardType is the correct type, replace with the actual type used for guards.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/createCaseFileGuards.spec.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/createPresignedPostGuards.spec.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/deleteCaseFileGuards.spec.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/limitedAccessFileControllerGuards.spec.ts (2 hunks)
Additional context used
Path-based instructions (4)
apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/limitedAccessFileControllerGuards.spec.ts (1)
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/createPresignedPostGuards.spec.ts (1)
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/createCaseFileGuards.spec.ts (1)
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/deleteCaseFileGuards.spec.ts (1)
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
Biome
apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/limitedAccessFileControllerGuards.spec.ts
[error] 8-8: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/createPresignedPostGuards.spec.ts
[error] 15-15: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/createCaseFileGuards.spec.ts
[error] 16-16: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/deleteCaseFileGuards.spec.ts
[error] 17-17: Unexpected any. Specify a different type. (lint/suspicious/noExplicitAny)
any disables many type checking rules. Its use should be avoided.
Additional comments not posted (4)
apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/limitedAccessFileControllerGuards.spec.ts (1)
15-15: Ensure the guard configuration is correct.The configuration of guards appears to be correctly updated to reflect the removal of
CaseCompletedGuardfrom the class-level guards, aligning with the PR's objectives to refine access control logic.apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/createPresignedPostGuards.spec.ts (1)
25-31: Verify the updated guard configuration.The guard configuration for creating presigned posts appears correctly updated to include
CaseCompletedGuard, aligning with the PR's objectives.Verification successful
The guard configuration for the
createPresignedPostmethod includesCaseCompletedGuard,CaseWriteGuard, andCaseTypeGuardwith the expected conditions. This aligns with the PR's objectives.
CaseTypeGuardwith allowed case types[...restrictionCases, ...investigationCases]CaseWriteGuardCaseCompletedGuardScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the guard configuration for createPresignedPost matches the expected types and conditions. # Test: Check the guard types and conditions. rg --type typescript $'createPresignedPost' apps/judicial-system/backend/src/app/modules/file/limitedAccessFile.controller.tsLength of output: 159
Script:
#!/bin/bash # Description: Verify that the guard configuration for createPresignedPost matches the expected types and conditions. # Test: Check the guard types and conditions. rg --type ts 'createPresignedPost' apps/judicial-system/backend/src/app/modules/file/limitedAccessFile.controller.tsLength of output: 345
Script:
#!/bin/bash # Description: Verify the guards applied to the createPresignedPost method. # Test: Check for decorators applied to the createPresignedPost method. rg --type ts '@UseGuards' -A 5 apps/judicial-system/backend/src/app/modules/file/limitedAccessFile.controller.tsLength of output: 1129
apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/createCaseFileGuards.spec.ts (1)
26-33: Verify the updated guard configuration.The guard configuration for creating case files appears correctly updated to include
CaseCompletedGuardandLimitedAccessWriteCaseFileGuard, aligning with the PR's objectives.Verification successful
The guard configuration for the
createCaseFilemethod in theLimitedAccessFileControlleris correctly updated to includeCaseCompletedGuardandLimitedAccessWriteCaseFileGuard, aligning with the PR's objectives.
LimitedAccessFileControllerclass uses@UseGuardswithJwtAuthGuard,RolesGuard, andLimitedAccessCaseExistsGuard.- The
createCaseFilemethod specifically uses@UseGuardswithCaseTypeGuard,CaseWriteGuard,CaseCompletedGuard, andLimitedAccessWriteCaseFileGuard.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the guard configuration for createCaseFile matches the expected types and conditions. # Test: Check the guard types and conditions. rg --type typescript $'createCaseFile' apps/judicial-system/backend/src/app/modules/file/limitedAccessFile.controller.tsLength of output: 154
Script:
#!/bin/bash # Description: Verify that the guard configuration for createCaseFile matches the expected types and conditions. # Test: Check the guard types and conditions. rg 'createCaseFile' apps/judicial-system/backend/src/app/modules/file/limitedAccessFile.controller.tsLength of output: 189
Script:
#!/bin/bash # Description: Verify the guard configuration for createCaseFile in the limitedAccessFile.controller.ts file. # Search for the guard configuration in the limitedAccessFile.controller.ts file. rg 'guards' apps/judicial-system/backend/src/app/modules/file/limitedAccessFile.controller.tsLength of output: 472
Script:
#!/bin/bash # Description: Verify the guard configuration for createCaseFile in the limitedAccessFile.controller.ts file. # Search for the createCaseFile method definition and its decorators in the limitedAccessFile.controller.ts file. rg -A 10 'createCaseFile' apps/judicial-system/backend/src/app/modules/file/limitedAccessFile.controller.tsLength of output: 713
Script:
#!/bin/bash # Description: Verify the guard configuration for createCaseFile in the limitedAccessFile.controller.ts file. # Search for the controller class definition and its decorators in the limitedAccessFile.controller.ts file. rg -A 10 '@UseGuards' apps/judicial-system/backend/src/app/modules/file/limitedAccessFile.controller.tsLength of output: 1686
apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/deleteCaseFileGuards.spec.ts (1)
27-35: Verify the updated guard configuration.The guard configuration for deleting case files appears correctly updated to include
CaseCompletedGuard,CaseFileExistsGuard, andLimitedAccessWriteCaseFileGuard, aligning with the PR's objectives.Verification successful
The guard configuration for the
deleteCaseFilemethod inlimitedAccessFile.controller.tsincludes the following guards:
CaseTypeGuardCaseWriteGuardCaseCompletedGuardCaseFileExistsGuardLimitedAccessWriteCaseFileGuardThis matches the expected configuration as outlined in the review comment.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the guard configuration for deleteCaseFile matches the expected types and conditions. # Test: Check the guard types and conditions. rg --type typescript $'deleteCaseFile' apps/judicial-system/backend/src/app/modules/file/limitedAccessFile.controller.tsLength of output: 154
Script:
#!/bin/bash # Description: Verify that the guard configuration for deleteCaseFile matches the expected types and conditions. # Test: Check the guard types and conditions. rg 'deleteCaseFile' apps/judicial-system/backend/src/app/modules/file/limitedAccessFile.controller.tsLength of output: 181
Script:
#!/bin/bash # Description: Verify the guard configuration for deleteCaseFile in the limitedAccessFile.controller.ts file. # Test: Check the guard configuration. rg 'deleteCaseFile' -A 10 apps/judicial-system/backend/src/app/modules/file/limitedAccessFile.controller.tsLength of output: 447
Script:
#!/bin/bash # Description: Verify the guard configuration for deleteCaseFile in the limitedAccessFile.controller.ts file. # Test: Check for the @UseGuards decorator. rg '@UseGuards' -A 5 apps/judicial-system/backend/src/app/modules/file/limitedAccessFile.controller.tsLength of output: 1119
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.service.ts (1)
Line range hint
273-292: Consider replacingforEachwithfor...offor better performance and readability.- theCase.caseFiles?.forEach((caseFile) => { + for (const caseFile of theCase.caseFiles ?? []) { const message = { type: MessageType.DELIVERY_TO_COURT_CASE_FILE, user, caseId: theCase.id, elementId: caseFile.id, } messages.push(message) } + }
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.service.ts (3 hunks)
Additional context used
Path-based instructions (1)
apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.service.ts (1)
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
Biome
apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.service.ts
[error] 226-226: Decorators are not valid here. (parse)
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabledoption totruein your configuration file.
[error] 227-227: Decorators are not valid here. (parse)
Decorators are only valid on class declarations, class expressions, and class methods.
You can enable parameter decorators by setting theunsafeParameterDecoratorsEnabledoption totruein your configuration file.
[error] 273-292: Prefer for...of instead of forEach. (lint/complexity/noForEach)
forEach may lead to performance issues when working with large arrays. When combined with functions like filter or map, this causes multiple iterations over the same type.
Additional comments not posted (3)
apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.service.ts (3)
29-29: Added imports forEventTypeandEventLogenhance the functionality by allowing event logging.Also applies to: 37-37
116-116: Initialization ofeventTypesconstant is a good practice for managing event types centrally.
192-199: The addition ofEventLogto theincludearray with specific conditions is a significant improvement for filtering event logs based on type.
Defender Case File Access
[Verjendur sjá ekki staðfestar ákærur}(https://app.asana.com/0/1199153462262248/1207453585767576/f)
What
Why
Screenshots / Gifs
Screen.Recording.2024-06-04.at.10.01.01.mov
Checklist:
Summary by CodeRabbit
New Features
indictmentReviewerfor better query accuracy.EventLogtracking with specific conditions to improve event monitoring in limited access cases.Bug Fixes
Refactor