From 15c04d0612fbf8c70a4b45927f57b1bd0694621b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jean-S=C3=A9bastien=20Herbaux?=
 <jean-sebastien.herbaux@epitech.eu>
Date: Mon, 17 May 2021 08:14:58 +0200
Subject: [PATCH] Fix RBAC permissions without subject ignoring conditions
 (#10291)

* Fix RBAC permissions without subject ignoring conditions

* Add unit test for nil subject in the permission engine
---
 .../__tests__/permissions.engine.test.js      | 31 +++++++++++++++++++
 .../services/permission/engine.js             | 10 ++++--
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/packages/strapi-admin/services/__tests__/permissions.engine.test.js b/packages/strapi-admin/services/__tests__/permissions.engine.test.js
index 269a9df814..30754f7631 100644
--- a/packages/strapi-admin/services/__tests__/permissions.engine.test.js
+++ b/packages/strapi-admin/services/__tests__/permissions.engine.test.js
@@ -349,6 +349,25 @@ describe('Permissions Engine', () => {
     });
   });
 
+  test('It should register the condition even if the subject is Nil', async () => {
+    const permission = {
+      action: 'read',
+      subject: null,
+      properties: {},
+      conditions: ['plugins::test.isCreatedBy'],
+    };
+
+    const user = getUser('alice');
+    const can = jest.fn();
+    const registerFn = engine.createRegisterFunction(can, {}, user);
+
+    await engine.evaluate({ permission, user, registerFn });
+
+    expect(can).toHaveBeenCalledWith('read', 'all', undefined, {
+      $and: [{ $or: [{ created_by: user.firstname }] }],
+    });
+  });
+
   describe('Create Register Function', () => {
     let can;
     let registerFn;
@@ -376,6 +395,18 @@ describe('Permissions Engine', () => {
       expect(can).toHaveBeenCalledTimes(1);
       expect(can).toHaveBeenCalledWith('read', 'article', '*', { created_by: 1 });
     });
+
+    test(`It should use 'all' as a subject if it's Nil`, async () => {
+      await registerFn({
+        action: 'read',
+        subject: null,
+        fields: null,
+        condition: { created_by: 1 },
+      });
+
+      expect(can).toHaveBeenCalledTimes(1);
+      expect(can).toHaveBeenCalledWith('read', 'all', null, { created_by: 1 });
+    });
   });
 
   describe('Check Many', () => {
diff --git a/packages/strapi-admin/services/permission/engine.js b/packages/strapi-admin/services/permission/engine.js
index 326bc0c39d..1d33e2debf 100644
--- a/packages/strapi-admin/services/permission/engine.js
+++ b/packages/strapi-admin/services/permission/engine.js
@@ -8,6 +8,7 @@ const {
   isFunction,
   isBoolean,
   isArray,
+  isNil,
   isEmpty,
   isObject,
   prop,
@@ -160,7 +161,7 @@ module.exports = conditionProvider => {
       await this.applyPermissionProcessors(permission);
 
       // Extract the up-to-date components from the permission
-      const { action, subject = 'all', properties = {}, conditions } = permission;
+      const { action, subject, properties = {}, conditions } = permission;
 
       // Register the permission if there is no condition
       if (isEmpty(conditions)) {
@@ -239,7 +240,12 @@ module.exports = conditionProvider => {
       const registerToCasl = caslPermission => {
         const { action, subject, fields, condition } = caslPermission;
 
-        can(action, subject, fields, isObject(condition) ? condition : undefined);
+        can(
+          action,
+          isNil(subject) ? 'all' : subject,
+          fields,
+          isObject(condition) ? condition : undefined
+        );
       };
 
       const runWillRegisterHook = async caslPermission => {