`@deny('read'` field-level policies seem to always block reads since v2.2.0

[aloisklink]

Description and expected behavior

Since v2.2.0, @deny('read', ...) field-level policies never seem to return data when the condition is false. Even @deny('read', false) (which I'd expect to do nothing), doesn't seem to do anything.

Screenshots

N/A

Environment (please complete the following information):

• ZenStack version: 2.2.0 and 2.2.1
• Prisma version: 5.13.0
• Database type: Postgresql

Additional context

I've added a test to the this repo that reproduces the problem.

The test works fine on the v2.1.0 tag, but breaks on the latest dev branch (aka commit 92f187f9190517df5baca795f12386c12c6694e9).

See my commit that adds a test case to tests/enhancements/with-policy/field-level-policy.test.ts: https://github.com/zenstackhq/zenstack/commit/4bcb95849a48da116c2af0a222503d89da26c3ab (based on v2.1.0).

This test case works fine on v2.1.0, but breaks on v2.2.0.

`.patch` of test case in the tests/enhancements/with-policy/field-level-policy.test.ts file

```patch From 4bcb95849a48da116c2af0a222503d89da26c3ab Mon Sep 17 00:00:00 2001 From: Alois Klink Date: Tue, 11 Jun 2024 15:25:52 +0900 Subject: [PATCH] test(policy): test `@deny('read'` field policy --- .../with-policy/field-level-policy.test.ts | 41 +++++++++++++------ 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/tests/enhancements/with-policy/field-level-policy.test.ts b/tests/enhancements/with-policy/field-level-policy.test.ts index de778e8..e5d2bf8 100644 --- a/tests/enhancements/with-policy/field-level-policy.test.ts +++ b/tests/enhancements/with-policy/field-level-policy.test.ts @@ -27,6 +27,7 @@ describe('Policy: field-level policy', () => { id Int @id @default(autoincrement()) x Int y Int @allow('read', x > 0) + z Int @deny('read', x <= 0) owner User @relation(fields: [ownerId], references: [id]) ownerId Int @@ -40,71 +41,82 @@ describe('Policy: field-level policy', () => { const db = enhance(); let r; - // y is unreadable + // y and x are unreadable r = await db.model.create({ - data: { id: 1, x: 0, y: 0, ownerId: 1 }, + data: { id: 1, x: 0, y: 0, z: 0, ownerId: 1 }, }); expect(r.x).toEqual(0); expect(r.y).toBeUndefined(); + expect(r.z).toBeUndefined(); r = await db.model.findUnique({ where: { id: 1 } }); expect(r.y).toBeUndefined(); + expect(r.z).toBeUndefined(); r = await db.user.findUnique({ where: { id: 1 }, select: { models: true } }); expect(r.models[0].y).toBeUndefined(); + expect(r.models[0].z).toBeUndefined(); r = await db.user.findUnique({ where: { id: 1 }, select: { models: { select: { y: true } } } }); expect(r.models[0].y).toBeUndefined(); + expect(r.models[0].z).toBeUndefined(); r = await db.user.findUnique({ where: { id: 1 } }).models(); expect(r[0].y).toBeUndefined(); + expect(r[0].z).toBeUndefined(); r = await db.user.findUnique({ where: { id: 1 } }).models({ select: { y: true } }); expect(r[0].y).toBeUndefined(); + expect(r[0].z).toBeUndefined(); r = await db.model.findUnique({ select: { x: true }, where: { id: 1 } }); expect(r.x).toEqual(0); expect(r.y).toBeUndefined(); + expect(r.z).toBeUndefined(); r = await db.model.findUnique({ select: { y: true }, where: { id: 1 } }); expect(r.x).toBeUndefined(); expect(r.y).toBeUndefined(); + expect(r.z).toBeUndefined(); r = await db.model.findUnique({ select: { x: false, y: true }, where: { id: 1 } }); expect(r.x).toBeUndefined(); expect(r.y).toBeUndefined(); + expect(r.z).toBeUndefined(); r = await db.model.findUnique({ select: { x: true, y: true }, where: { id: 1 } }); expect(r.x).toEqual(0); expect(r.y).toBeUndefined(); + expect(r.z).toBeUndefined(); r = await db.model.findUnique({ include: { owner: true }, where: { id: 1 } }); expect(r.x).toEqual(0); expect(r.owner).toBeTruthy(); expect(r.y).toBeUndefined(); + expect(r.z).toBeUndefined(); // y is readable r = await db.model.create({ - data: { id: 2, x: 1, y: 0, ownerId: 1 }, + data: { id: 2, x: 1, y: 0, z: 0, ownerId: 1 }, }); - expect(r).toEqual(expect.objectContaining({ x: 1, y: 0 })); + expect(r).toEqual(expect.objectContaining({ x: 1, y: 0, z: 0 })); r = await db.model.findUnique({ where: { id: 2 } }); - expect(r).toEqual(expect.objectContaining({ x: 1, y: 0 })); + expect(r).toEqual(expect.objectContaining({ x: 1, y: 0, z: 0 })); r = await db.user.findUnique({ where: { id: 1 }, select: { models: { where: { id: 2 } } } }); - expect(r.models[0]).toEqual(expect.objectContaining({ x: 1, y: 0 })); + expect(r.models[0]).toEqual(expect.objectContaining({ x: 1, y: 0, z: 0 })); r = await db.user.findUnique({ where: { id: 1 }, - select: { models: { where: { id: 2 }, select: { y: true } } }, + select: { models: { where: { id: 2 }, select: { y: true, z: true } } }, }); - expect(r.models[0]).toEqual(expect.objectContaining({ y: 0 })); + expect(r.models[0]).toEqual(expect.objectContaining({ y: 0, z: 0 })); r = await db.user.findUnique({ where: { id: 1 } }).models({ where: { id: 2 } }); - expect(r[0]).toEqual(expect.objectContaining({ x: 1, y: 0 })); + expect(r[0]).toEqual(expect.objectContaining({ x: 1, y: 0, z: 0 })); r = await db.user.findUnique({ where: { id: 1 } }).models({ where: { id: 2 }, select: { y: true } }); expect(r[0]).toEqual(expect.objectContaining({ y: 0 })); @@ -112,20 +124,23 @@ describe('Policy: field-level policy', () => { r = await db.model.findUnique({ select: { x: true }, where: { id: 2 } }); expect(r.x).toEqual(1); expect(r.y).toBeUndefined(); + expect(r.z).toBeUndefined(); r = await db.model.findUnique({ select: { y: true }, where: { id: 2 } }); expect(r.x).toBeUndefined(); expect(r.y).toEqual(0); + expect(r.z).toBeUndefined(); - r = await db.model.findUnique({ select: { x: false, y: true }, where: { id: 2 } }); + r = await db.model.findUnique({ select: { x: false, y: true, z: true }, where: { id: 2 } }); expect(r.x).toBeUndefined(); expect(r.y).toEqual(0); + expect(r.z).toEqual(0); - r = await db.model.findUnique({ select: { x: true, y: true }, where: { id: 2 } }); - expect(r).toEqual(expect.objectContaining({ x: 1, y: 0 })); + r = await db.model.findUnique({ select: { x: true, y: true, z: true }, where: { id: 2 } }); + expect(r).toEqual(expect.objectContaining({ x: 1, y: 0, z: 0 })); r = await db.model.findUnique({ include: { owner: true }, where: { id: 2 } }); - expect(r).toEqual(expect.objectContaining({ x: 1, y: 0 })); + expect(r).toEqual(expect.objectContaining({ x: 1, y: 0, z: 0 })); expect(r.owner).toBeTruthy(); }); -- 2.45.2 ```

[ymc9]

Hi @aloisklink , thanks for reporting this!

It is indeed a regression introduced during a refactor. The design is that, for field-level policies, if there are allow rules, at least one of them needs to be satisfied to grant access; however, if there's no allow rule, access is allowed unless explicitly rejected by a deny rule.

I'm making a fix and will publish a patch release soon. I appreciate the updated test case. It made my life easier 😄.

[ymc9]

Fixed in v2.2.2

[aloisklink]

I appreciate the updated test case. It made my life easier 😄.

:heart: I always feel bad reporting a bug unless I have good instructions on how to reproduce it. And writing a new test case is probably the most reproducible bug report!