fix: when field policy only has deny rule, access should be allowed when the rule doesn't satisfy (#818)

Author: ymc9

packages/runtime/src/enhancements/policy/policy-utils.ts

@@ -143,6 +143,11 @@ export class PolicyUtil {
 
         const result: any = {};
         for (const [key, value] of Object.entries<any>(condition)) {
+            if (this.isFalse(result)) {
+                // already false, no need to continue
+                break;
+            }
+
             if (value === null || value === undefined) {
                 result[key] = value;
                 continue;
@@ -184,7 +189,16 @@ export class PolicyUtil {
                 }
 
                 case 'NOT': {
-                    result[key] = this.reduce(value);
+                    const r = this.reduce(value);
+                    if (this.isFalse(r)) {
+                        // NOT false => true, thus eliminated (not adding into result)
+                    } else if (this.isTrue(r)) {
+                        // NOT true => false, eliminate all other keys and set entire condition to false
+                        Object.keys(result).forEach((k) => delete result[k]);
+                        result['OR'] = []; // this will cause the outer loop to exit too
+                    } else {
+                        result[key] = r;
+                    }
                     break;
                 }
 
@@ -256,6 +270,7 @@ export class PolicyUtil {
         }
 
         if (!provider) {
+            // field access is allowed by default
             return this.makeTrue();
         }
         const r = provider({ user: this.user }, db);

packages/schema/src/plugins/access-policy/policy-guard-generator.ts

@@ -606,7 +606,19 @@ export default class PolicyGenerator {
                         throw err;
                     }
                 }
-                writer.write(`return ${FALSE};`);
+
+                if (forField) {
+                    if (allows.length === 0) {
+                        // if there's no allow rule, for field-level rules, by default we allow
+                        writer.write(`return ${TRUE};`);
+                    } else {
+                        // if there's any allow rule, we deny unless any allow rule evaluates to true
+                        writer.write(`return ${FALSE};`);
+                    }
+                } else {
+                    // for model-level rules, the default is always deny
+                    writer.write(`return ${FALSE};`);
+                }
             });
         } else {
             statements.push((writer) => {
@@ -634,14 +646,17 @@ export default class PolicyGenerator {
                 };
 
                 if (allows.length > 0 && denies.length > 0) {
+                    // include both allow and deny rules
                     writer.write('{ AND: [');
                     writeDenies();
                     writer.write(',');
                     writeAllows();
                     writer.write(']}');
                 } else if (denies.length > 0) {
+                    // only deny rules
                     writeDenies();
                 } else if (allows.length > 0) {
+                    // only allow rules
                     writeAllows();
                 } else {
                     // disallow any operation

tests/integration/tests/enhancements/with-policy/field-level-policy.test.ts

@@ -132,8 +132,7 @@ describe('With Policy: field-level policy', () => {
 
             @@allow('all', true)
         }
-        `,
-            { logPrismaQuery: true }
+        `
         );
 
         await prisma.user.create({ data: { id: 1, admin: true } });
@@ -841,4 +840,78 @@ describe('With Policy: field-level policy', () => {
         expect(r.a).toBe(1);
         expect(r.b).toBe(2);
     });
+
+    it('deny only without field access', async () => {
+        const { prisma, withPolicy } = await loadSchema(
+            `
+        model User {
+            id Int @id @default(autoincrement())
+            role String @deny('update', auth().role != 'ADMIN')
+
+            @@allow('all', true)
+        }
+        `
+        );
+
+        const user = await prisma.user.create({
+            data: { role: 'USER' },
+        });
+
+        await expect(
+            withPolicy({ id: 1, role: 'ADMIN' }).user.update({
+                where: { id: user.id },
+                data: { role: 'ADMIN' },
+            })
+        ).toResolveTruthy();
+
+        await expect(
+            withPolicy({ id: 1, role: 'USER' }).user.update({
+                where: { id: user.id },
+                data: { role: 'ADMIN' },
+            })
+        ).toBeRejectedByPolicy();
+    });
+
+    it('deny only with field access', async () => {
+        const { prisma, withPolicy } = await loadSchema(
+            `
+        model User {
+            id Int @id @default(autoincrement())
+            locked Boolean @default(false)
+            role String @deny('update', auth().role != 'ADMIN' || locked)
+
+            @@allow('all', true)
+        }
+        `
+        );
+
+        const user1 = await prisma.user.create({
+            data: { role: 'USER' },
+        });
+
+        await expect(
+            withPolicy({ id: 1, role: 'ADMIN' }).user.update({
+                where: { id: user1.id },
+                data: { role: 'ADMIN' },
+            })
+        ).toResolveTruthy();
+
+        await expect(
+            withPolicy({ id: 1, role: 'USER' }).user.update({
+                where: { id: user1.id },
+                data: { role: 'ADMIN' },
+            })
+        ).toBeRejectedByPolicy();
+
+        const user2 = await prisma.user.create({
+            data: { role: 'USER', locked: true },
+        });
+
+        await expect(
+            withPolicy({ id: 1, role: 'ADMIN' }).user.update({
+                where: { id: user2.id },
+                data: { role: 'ADMIN' },
+            })
+        ).toBeRejectedByPolicy();
+    });
 });