fix: field-level policy should filter out records when the field used for filtering is not allowed to read #1661
Conversation
… for filtering is not allowed to read Fixes #1644
WalkthroughWalkthroughThe changes involve refactoring the Changes
Assessment against linked issues
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 Configuration 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
Outside diff range, codebase verification and nitpick comments (9)
tests/regression/tests/issue-1644.test.ts (1)
2-22: Consider adding more assertions to verify the behavior of thefindManyfunction.The test case is correctly testing the behavior of the
findManyfunction with field-level read policies. However, it can be improved by adding more assertions to verify the behavior of thefindManyfunction, such as checking the content of the returned records.Consider adding assertions like:
await expect(db.user.findMany({ where: { email: { contains: 'example.com' } } })).resolves.toEqual([ { id: 1, email: '[email protected]' } ]);tests/integration/tests/enhancements/with-policy/create-many-and-return.test.ts (2)
Line range hint
4-4: Consider adding more assertions to verify the behavior of thecreateManyAndReturnfunction.The test case is correctly testing the behavior of the
createManyAndReturnfunction with model-level policies. However, it can be improved by adding more assertions to verify the behavior of thecreateManyAndReturnfunction, such as checking the content of the returned records.Consider adding assertions like:
expect(r).toEqual([ { id: 1, title: 'hello1', userId: 1, published: true } ]);Also applies to: 23-23, 37-37, 51-51, 59-59
95-106: Consider adding more assertions to verify the behavior of thecreateManyAndReturnfunction.The test case is correctly testing the behavior of the
createManyAndReturnfunction with field-level policies. However, it can be improved by adding more assertions to verify the behavior of thecreateManyAndReturnfunction, such as checking the content of the returned records.Consider adding assertions like:
await expect(prisma.post.findMany()).resolves.toEqual([ { id: 1, title: 'post1', published: true }, { id: 2, title: 'post2', published: false } ]);packages/runtime/src/enhancements/policy/policy-utils.ts (6)
476-480: Consider adding more comments to explain the logic.The method is correctly injecting the auth guard as a where clause. However, it can be improved by adding more comments to explain the logic, such as explaining the purpose of
buildReadGuardForFields.Consider adding comments like:
// Inject guard for relation fields and regular fields mergedGuard = this.buildReadGuardForFields(db, model, args.where, guard);
489-489: Consider adding more comments to explain the logic.The method is correctly building read guards for fields. However, it can be improved by adding more comments to explain the logic, such as explaining the purpose of
allFieldGuardsandallFieldOverrideGuards.Consider adding comments like:
// Collect guards for all fields const allFieldGuards: object[] = []; const allFieldOverrideGuards: object[] = [];
534-534: Consider adding more comments to explain the logic.The method is correctly injecting read guard for to-many fields. However, it can be improved by adding more comments to explain the logic, such as explaining the purpose of
buildReadGuardForFields.Consider adding comments like:
// Inject guard for "some" condition const mergedGuard = this.buildReadGuardForFields(db, fieldInfo.type, payload.some, guard);Also applies to: 539-539, 549-549
573-573: Consider adding more comments to explain the logic.The method is correctly injecting read guard for to-one fields. However, it can be improved by adding more comments to explain the logic, such as explaining the purpose of
buildReadGuardForFields.Consider adding comments like:
// Inject guard for "is" condition const mergedGuard = this.buildReadGuardForFields(db, fieldInfo.type, payload.is, guard);Also applies to: 579-579, 584-584
604-609: Consider adding more comments to explain the logic.The method is correctly injecting auth guard for read operations. However, it can be improved by adding more comments to explain the logic, such as explaining the purpose of
buildReadGuardForFields.Consider adding comments like:
// Inject guard for relation fields and regular fields const mergedGuard = this.buildReadGuardForFields(db, model, args.where, {});
618-619: Consider adding more comments to explain the logic.The method is correctly injecting read conditions into nested select, include, and _count. However, it can be improved by adding more comments to explain the logic, such as explaining the purpose of
injectAuthGuardAsWhere.Consider adding comments like:
// No user-provided where clause, use the injected one args.where = injected.where;
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- packages/runtime/src/enhancements/policy/policy-utils.ts (6 hunks)
- tests/integration/tests/enhancements/with-policy/create-many-and-return.test.ts (1 hunks)
- tests/regression/tests/issue-1644.test.ts (1 hunks)
Fixes #1644