RBAC: decoupled rules from assignments and items, implemented php manager

framework/rbac/Assignment.php

@@ -27,9 +27,9 @@ class Assignment extends Object
      */
     public $manager;
     /**
-     * @var string the business rule associated with this assignment
+     * @var string name of the rule associated with this assignment
      */
-    public $bizRule;
+    public $ruleName;
     /**
      * @var mixed additional data for this assignment
      */

framework/rbac/DbManager.php

@@ -38,19 +38,27 @@ class DbManager extends Manager
      * with a DB connection object.
      */
     public $db = 'db';
+
     /**
-     * @var string the name of the table storing authorization items. Defaults to 'auth_item'.
+     * @var string the name of the table storing authorization items. Defaults to "auth_item".
      */
     public $itemTable = '{{%auth_item}}';
+
     /**
-     * @var string the name of the table storing authorization item hierarchy. Defaults to 'auth_item_child'.
+     * @var string the name of the table storing authorization item hierarchy. Defaults to "auth_item_child".
      */
     public $itemChildTable = '{{%auth_item_child}}';
+
     /**
-     * @var string the name of the table storing authorization item assignments. Defaults to 'auth_assignment'.
+     * @var string the name of the table storing authorization item assignments. Defaults to "auth_assignment".
      */
     public $assignmentTable = '{{%auth_assignment}}';
 
+    /**
+     * @var string the name of the table storing rules. Defaults to "auth_rule".
+     */
+    public $ruleTable = '{{%auth_rule}}';
+
     private $_usingSqlite;
 
     /**
@@ -102,13 +110,13 @@ class DbManager extends Manager
         if (!isset($params['userId'])) {
             $params['userId'] = $userId;
         }
-        if ($this->executeBizRule($item->bizRule, $params, $item->data)) {
+        if ($this->executeRule($item->ruleName, $params, $item->data)) {
             if (in_array($itemName, $this->defaultRoles)) {
                 return true;
             }
             if (isset($assignments[$itemName])) {
                 $assignment = $assignments[$itemName];
-                if ($this->executeBizRule($assignment->bizRule, $params, $assignment->data)) {
+                if ($this->executeRule($assignment->bizRule, $params, $assignment->data)) {
                     return true;
                 }
             }
@@ -233,15 +241,16 @@ class DbManager extends Manager
 
     /**
      * Assigns an authorization item to a user.
-     * @param mixed $userId the user ID (see [[\yii\web\User::id]])
+     *
+*@param mixed $userId the user ID (see [[\yii\web\User::id]])
      * @param string $itemName the item name
-     * @param string $bizRule the business rule to be executed when [[checkAccess()]] is called
+     * @param string $ruleName the business rule to be executed when [[checkAccess()]] is called
      * for this particular authorization item.
      * @param mixed $data additional data associated with this assignment
      * @return Assignment the authorization assignment information.
      * @throws InvalidParamException if the item does not exist or if the item has already been assigned to the user
      */
-    public function assign($userId, $itemName, $bizRule = null, $data = null)
+    public function assign($userId, $itemName, $ruleName = null, $data = null)
     {
         if ($this->usingSqlite() && $this->getItem($itemName) === null) {
             throw new InvalidParamException("The item '$itemName' does not exist.");
@@ -250,7 +259,7 @@ class DbManager extends Manager
             ->insert($this->assignmentTable, [
                 'user_id' => $userId,
                 'item_name' => $itemName,
-                'biz_rule' => $bizRule,
+                'biz_rule' => $ruleName,
                 'data' => $data === null ? null : serialize($data),
             ])
             ->execute();
@@ -259,7 +268,7 @@ class DbManager extends Manager
             'manager' => $this,
             'userId' => $userId,
             'itemName' => $itemName,
-            'bizRule' => $bizRule,
+            'bizRule' => $ruleName,
             'data' => $data,
         ]);
     }
@@ -371,7 +380,7 @@ class DbManager extends Manager
      * Saves the changes to an authorization assignment.
      * @param Assignment $assignment the assignment that has been changed.
      */
-    public function saveAssignment($assignment)
+    public function saveAssignment(Assignment $assignment)
     {
         $this->db->createCommand()
             ->update($this->assignmentTable, [
@@ -437,23 +446,24 @@ class DbManager extends Manager
      * It has three types: operation, task and role.
      * Authorization items form a hierarchy. Higher level items inheirt permissions representing
      * by lower level items.
+     *
      * @param string $name the item name. This must be a unique identifier.
      * @param integer $type the item type (0: operation, 1: task, 2: role).
      * @param string $description description of the item
-     * @param string $bizRule business rule associated with the item. This is a piece of
+     * @param string $rule business rule associated with the item. This is a piece of
      * PHP code that will be executed when [[checkAccess()]] is called for the item.
      * @param mixed $data additional data associated with the item.
      * @return Item the authorization item
      * @throws Exception if an item with the same name already exists
      */
-    public function createItem($name, $type, $description = '', $bizRule = null, $data = null)
+    public function createItem($name, $type, $description = '', $rule = null, $data = null)
     {
         $this->db->createCommand()
             ->insert($this->itemTable, [
                 'name' => $name,
                 'type' => $type,
                 'description' => $description,
-                'biz_rule' => $bizRule,
+                'biz_rule' => $rule,
                 'data' => $data === null ? null : serialize($data),
             ])
             ->execute();
@@ -463,7 +473,7 @@ class DbManager extends Manager
             'name' => $name,
             'type' => $type,
             'description' => $description,
-            'bizRule' => $bizRule,
+            'bizRule' => $rule,
             'data' => $data,
         ]);
     }
@@ -525,7 +535,7 @@ class DbManager extends Manager
      * @param Item $item the item to be saved.
      * @param string $oldName the old item name. If null, it means the item name is not changed.
      */
-    public function saveItem($item, $oldName = null)
+    public function saveItem(Item $item, $oldName = null)
     {
         if ($this->usingSqlite() && $oldName !== null && $item->getName() !== $oldName) {
             $this->db->createCommand()
@@ -544,7 +554,7 @@ class DbManager extends Manager
                 'name' => $item->getName(),
                 'type' => $item->type,
                 'description' => $item->description,
-                'biz_rule' => $item->bizRule,
+                'rule_name' => $item->ruleName,
                 'data' => $item->data === null ? null : serialize($item->data),
             ], [
                 'name' => $oldName === null ? $item->getName() : $oldName,
@@ -604,4 +614,45 @@ class DbManager extends Manager
     {
         return $this->_usingSqlite;
     }
-}
+
+    /**
+     * Removes the specified rule.
+     *
+     * @param string $name the name of the rule to be removed
+     * @return boolean whether the rule exists in the storage and has been removed
+     */
+    public function removeRule($name)
+    {
+        // TODO: Implement removeRule() method.
+    }
+
+    /**
+     * Saves the changes to the rule.
+     *
+     * @param Rule $rule the rule that has been changed.
+     */
+    public function saveRule(Rule $rule)
+    {
+        // TODO: Implement saveRule() method.
+    }
+
+    /**
+     * Returns rule given its name.
+     *
+     * @param string $name name of the rule.
+     * @return Rule
+     */
+    public function getRule($name)
+    {
+        // TODO: Implement getRule() method.
+    }
+
+    /**
+     * Returns all rules.
+     *
+     * @return Rule[]
+     */
+    public function getRules()
+    {
+        // TODO: Implement getRules() method.
+    }}

framework/rbac/Item.php

@@ -40,9 +40,9 @@ class Item extends Object
      */
     public $description;
     /**
-     * @var string the business rule associated with this item
+     * @var string name of the rule associated with this item
      */
-    public $bizRule;
+    public $ruleName;
     /**
      * @var mixed the additional data associated with this item
      */
@@ -66,7 +66,7 @@ class Item extends Object
     public function checkAccess($itemName, $params = [])
     {
         Yii::trace('Checking permission: ' . $this->_name, __METHOD__);
-        if ($this->manager->executeBizRule($this->bizRule, $params, $this->data)) {
+        if ($this->manager->executeRule($this->ruleName, $params, $this->data)) {
             if ($this->_name == $itemName) {
                 return true;
             }
@@ -146,17 +146,18 @@ class Item extends Object
 
     /**
      * Assigns this item to a user.
-     * @param mixed $userId the user ID (see [[\yii\web\User::id]])
-     * @param string $bizRule the business rule to be executed when [[checkAccess()]] is called
+     *
+*@param mixed $userId the user ID (see [[\yii\web\User::id]])
+     * @param Rule $rule the rule to be executed when [[checkAccess()]] is called
      * for this particular authorization item.
      * @param mixed $data additional data associated with this assignment
      * @return Assignment the authorization assignment information.
      * @throws \yii\base\Exception if the item has already been assigned to the user
      * @see Manager::assign
      */
-    public function assign($userId, $bizRule = null, $data = null)
+    public function assign($userId, Rule $rule = null, $data = null)
     {
-        return $this->manager->assign($userId, $this->_name, $bizRule, $data);
+        return $this->manager->assign($userId, $this->_name, $rule, $data);
     }
 
     /**

framework/rbac/Rule.php

+<?php
+/**
+ * @link http://www.yiiframework.com/
+ * @copyright Copyright (c) 2008 Yii Software LLC
+ * @license http://www.yiiframework.com/license/
+ */
+
+namespace yii\rbac;
+
+use yii\base\Object;
+
+/**
+ * Rule
+ * @property string $name
+ */
+abstract class Rule extends Object
+{
+    public $name;
+
+    /**
+     * Constructor.
+     *
+     * @param array $name name of the rule
+     * @param array $config name-value pairs that will be used to initialize the object properties
+     */
+    public function __construct($name = null, $config = [])
+    {
+        if ($name !== null) {
+            $this->name = $name;
+        }
+        parent::__construct($config);
+    }
+
+    /**
+     * Executes the rule.
+     *
+     * @param array $params parameters passed to [[Manager::checkAccess()]].
+     * @param mixed $data additional data associated with the authorization item or assignment.
+     * @return boolean whether the rule execution returns true.
+     */
+    abstract public function execute($params, $data);
+}

framework/rbac/schema-mssql.sql

@@ -12,15 +12,24 @@
 drop table if exists [auth_assignment];
 drop table if exists [auth_item_child];
 drop table if exists [auth_item];
+drop table if exists [auth_rule];
+
+create table [auth_rule]
+(
+    [name]  varchar(64) not null,
+    [data]  text,
+    primary key ([name])
+);
 
 create table [auth_item]
 (
    [name]                 varchar(64) not null,
    [type]                 integer not null,
    [description]          text,
-   [biz_rule]              text,
+   [rule_name]            varchar(64),
    [data]                 text,
    primary key ([name]),
+   foreign key ([rule_name]) references [auth_rule] ([name]) on delete set null on update cascade,
    key [type] ([type])
 );
 
@@ -37,8 +46,9 @@ create table [auth_assignment]
 (
    [item_name]            varchar(64) not null,
    [user_id]              varchar(64) not null,
-   [biz_rule]              text,
+   [rule_name]            varchar(64),
    [data]                 text,
-   primary key ([item_name],[user_id]),
-   foreign key ([item_name]) references [auth_item] ([name]) on delete cascade on update cascade
+   primary key ([item_name], [user_id]),
+   foreign key ([item_name]) references [auth_item] ([name]) on delete cascade on update cascade,
+   foreign key ([rule_name]) references [auth_rule] ([name]) on delete set null on update cascade
 );

framework/rbac/schema-mysql.sql

@@ -12,15 +12,24 @@
 drop table if exists `auth_assignment`;
 drop table if exists `auth_item_child`;
 drop table if exists `auth_item`;
+drop table if exists `auth_rule`;
+
+create table `auth_rule`
+(
+    `name`  varchar(64) not null,
+    `data`  text,
+    primary key (`name`)
+) engine InnoDB;
 
 create table `auth_item`
 (
    `name`                 varchar(64) not null,
    `type`                 integer not null,
    `description`          text,
-   `biz_rule`              text,
+   `rule_name`            varchar(64),
    `data`                 text,
    primary key (`name`),
+   foreign key (`rule_name`) references `auth_rule` (`name`) on delete set null on update cascade,
    key `type` (`type`)
 ) engine InnoDB;
 
@@ -28,7 +37,7 @@ create table `auth_item_child`
 (
    `parent`               varchar(64) not null,
    `child`                varchar(64) not null,
-   primary key (`parent`,`child`),
+   primary key (`parent`, `child`),
    foreign key (`parent`) references `auth_item` (`name`) on delete cascade on update cascade,
    foreign key (`child`) references `auth_item` (`name`) on delete cascade on update cascade
 ) engine InnoDB;
@@ -37,8 +46,9 @@ create table `auth_assignment`
 (
    `item_name`            varchar(64) not null,
    `user_id`              varchar(64) not null,
-   `biz_rule`              text,
+   `rule_name`            varchar(64),
    `data`                 text,
-   primary key (`item_name`,`user_id`),
-   foreign key (`item_name`) references `auth_item` (`name`) on delete cascade on update cascade
-) engine InnoDB;
+   primary key (`item_name`, `user_id`),
+   foreign key (`item_name`) references `auth_item` (`name`) on delete cascade on update cascade,
+   foreign key (`rule_name`) references `auth_rule` (`name`) on delete set null on update cascade
+) engine InnoDB;
\ No newline at end of file

framework/rbac/schema-oci.sql

@@ -12,15 +12,24 @@
 drop table if exists "auth_assignment";
 drop table if exists "auth_item_child";
 drop table if exists "auth_item";
+drop table if exists "auth_rule";
+
+create table "auth_rule"
+(
+    "name"  varchar(64) not null,
+    "data"  text,
+    primary key ("name")
+);
 
 create table "auth_item"
 (
    "name"                 varchar(64) not null,
    "type"                 integer not null,
    "description"          text,
-   "biz_rule"              text,
+   "rule_name"            varchar(64),
    "data"                 text,
    primary key ("name"),
+   foreign key ("rule_name") references "auth_rule" ("name") on delete set null on update cascade,
    key "type" ("type")
 );
 
@@ -37,8 +46,9 @@ create table "auth_assignment"
 (
    "item_name"            varchar(64) not null,
    "user_id"              varchar(64) not null,
-   "biz_rule"              text,
+   "rule_name"            varchar(64),
    "data"                 text,
    primary key ("item_name","user_id"),
-   foreign key ("item_name") references "auth_item" ("name") on delete cascade on update cascade
+   foreign key ("item_name") references "auth_item" ("name") on delete cascade on update cascade,
+   foreign key ("rule_name") references "auth_rule" ("name") on delete set null on update cascade
 );

framework/rbac/schema-pgsql.sql

@@ -12,15 +12,24 @@
 drop table if exists "auth_assignment";
 drop table if exists "auth_item_child";
 drop table if exists "auth_item";
+drop table if exists "auth_rule";
+
+create table "auth_rule"
+(
+    "name"  varchar(64) not null,
+    "data"  text,
+    primary key ("name")
+);
 
 create table "auth_item"
 (
    "name"                 varchar(64) not null,
    "type"                 integer not null,
    "description"          text,
-   "biz_rule"              text,
+   "rule_name"            varchar(64),
    "data"                 text,
-   primary key ("name")
+   primary key ("name"),
+   foreign key ("rule_name") references "auth_rule" ("name") on delete set null on update cascade
 );
 
 create index auth_item_type_idx on "auth_item" ("type");
@@ -38,8 +47,9 @@ create table "auth_assignment"
 (
    "item_name"            varchar(64) not null,
    "user_id"              varchar(64) not null,
-   "biz_rule"              text,
+   "rule_name"            varchar(64),
    "data"                 text,
    primary key ("item_name","user_id"),
-   foreign key ("item_name") references "auth_item" ("name") on delete cascade on update cascade
+   foreign key ("item_name") references "auth_item" ("name") on delete cascade on update cascade,
+   foreign key ("rule_name") references "auth_rule" ("name") on delete set null on update cascade
 );

framework/rbac/schema-sqlite.sql

@@ -9,36 +9,46 @@
  * @since 2.0
  */
 
-drop table if exists 'auth_assignment';
-drop table if exists 'auth_item_child';
-drop table if exists 'auth_item';
+drop table if exists "auth_assignment";
+drop table if exists "auth_item_child";
+drop table if exists "auth_item";
+drop table if exists "auth_rule";
 
-create table 'auth_item'
+create table "auth_rule"
+(
+    "name"  varchar(64) not null,
+    "data"  text,
+    primary key ("name")
+);
+
+create table "auth_item"
 (
    "name"                 varchar(64) not null,
    "type"                 integer not null,
    "description"          text,
-   "biz_rule"              text,
+   "rule_name"            varchar(64),
    "data"                 text,
    primary key ("name"),
+   foreign key ("rule_name") references "auth_rule" ("name") on delete set null on update cascade,
    key "type" ("type")
 );
 
-create table 'auth_item_child'
+create table "auth_item_child"
 (
    "parent"               varchar(64) not null,
    "child"                varchar(64) not null,
    primary key ("parent","child"),
-   foreign key ("parent") references 'auth_item' ("name") on delete cascade on update cascade,
-   foreign key ("child") references 'auth_item' ("name") on delete cascade on update cascade
+   foreign key ("parent") references "auth_item" ("name") on delete cascade on update cascade,
+   foreign key ("child") references "auth_item" ("name") on delete cascade on update cascade
 );
 
-create table 'auth_assignment'
+create table "auth_assignment"
 (
    "item_name"            varchar(64) not null,
    "user_id"              varchar(64) not null,
-   "biz_rule"              text,
+   "rule_name"            varchar(64),
    "data"                 text,
    primary key ("item_name","user_id"),
-   foreign key ("item_name") references 'auth_item' ("name") on delete cascade on update cascade
+   foreign key ("item_name") references "auth_item" ("name") on delete cascade on update cascade,
+   foreign key ("rule_name") references "auth_rule" ("name") on delete set null on update cascade
 );

tests/unit/framework/rbac/AuthorRule.php

+<?php
+namespace yiiunit\framework\rbac;
+
+use yii\rbac\Rule;
+
+/**
+ * Checks if authorID matches userID passed via params
+ */
+class AuthorRule extends Rule
+{
+    public $name = 'isAuthor';
+    public $reallyReally = false;
+
+    /**
+     * @inheritdoc
+     */
+    public function execute($params, $data)
+    {
+        return $params['authorID'] == $params['userID'];
+    }
+}
+ 
\ No newline at end of file

tests/unit/framework/rbac/ManagerTestCase.php

@@ -8,7 +8,7 @@ use yiiunit\TestCase;
 
 abstract class ManagerTestCase extends TestCase
 {
-    /** @var \yii\rbac\PhpManager|\yii\rbac\DbManager */
+    /** @var \yii\rbac\Manager */
     protected $auth;
 
     public function testCreateItem()
@@ -16,24 +16,24 @@ abstract class ManagerTestCase extends TestCase
         $type = Item::TYPE_TASK;
         $name = 'editUser';
         $description = 'edit a user';
-        $bizRule = 'checkUserIdentity()';
+        $ruleName = 'isAuthor';
         $data = [1, 2, 3];
-        $item = $this->auth->createItem($name, $type, $description, $bizRule, $data);
+        $item = $this->auth->createItem($name, $type, $description, $ruleName, $data);
         $this->assertTrue($item instanceof Item);
         $this->assertEquals($item->type, $type);
         $this->assertEquals($item->name, $name);
         $this->assertEquals($item->description, $description);
-        $this->assertEquals($item->bizRule, $bizRule);
+        $this->assertEquals($item->ruleName, $ruleName);
         $this->assertEquals($item->data, $data);
 
         // test shortcut
         $name2 = 'createUser';
-        $item2 = $this->auth->createRole($name2, $description, $bizRule, $data);
+        $item2 = $this->auth->createRole($name2, $description, $ruleName, $data);
         $this->assertEquals($item2->type, Item::TYPE_ROLE);
 
         // test adding an item with the same name
         $this->setExpectedException('\yii\base\Exception');
-        $this->auth->createItem($name, $type, $description, $bizRule, $data);
+        $this->auth->createItem($name, $type, $description, $ruleName, $data);
     }
 
     public function testGetItem()
@@ -102,7 +102,7 @@ abstract class ManagerTestCase extends TestCase
         $this->assertTrue($auth instanceof Assignment);
         $this->assertEquals($auth->userId, 'new user');
         $this->assertEquals($auth->itemName, 'createPost');
-        $this->assertEquals($auth->bizRule, 'rule');
+        $this->assertEquals($auth->ruleName, 'rule');
         $this->assertEquals($auth->data, 'data');
 
         $this->setExpectedException('\yii\base\Exception');
@@ -168,14 +168,64 @@ abstract class ManagerTestCase extends TestCase
         $this->auth->addItemChild('readPost', 'readPost');
     }
 
-    public function testExecuteBizRule()
+    public function testGetRule()
     {
-        $this->assertTrue($this->auth->executeBizRule(null, [], null));
-        $this->assertTrue($this->auth->executeBizRule('return 1 == true;', [], null));
-        $this->assertTrue($this->auth->executeBizRule('return $params[0] == $params[1];', [1, '1'], null));
-        if (!defined('HHVM_VERSION')) { // invalid code crashes on HHVM
-            $this->assertFalse($this->auth->executeBizRule('invalid;', [], null));
+        $rule = $this->auth->getRule('isAuthor');
+        $this->assertInstanceOf('yii\rbac\Rule', $rule);
+        $this->assertEquals('isAuthor', $rule->name);
+
+        $rule = $this->auth->getRule('nonExisting');
+        $this->assertNull($rule);
+    }
+
+    public function testSaveRule()
+    {
+        $ruleName = 'isReallyReallyAuthor';
+        $rule = new AuthorRule($ruleName, ['reallyReally' => true]);
+        $this->auth->saveRule($rule);
+
+        /** @var AuthorRule $rule */
+        $rule = $this->auth->getRule($ruleName);
+        $this->assertEquals($ruleName, $rule->name);
+        $this->assertEquals(true, $rule->reallyReally);
+
+        $rule->reallyReally = false;
+        $this->auth->saveRule($rule);
+
+        /** @var AuthorRule $rule */
+        $rule = $this->auth->getRule($ruleName);
+        $this->assertEquals(false, $rule->reallyReally);
+    }
+
+    public function testGetRules()
+    {
+        $rule = new AuthorRule('isReallyReallyAuthor', ['reallyReally' => true]);
+        $this->auth->saveRule($rule);
+
+        $rules = $this->auth->getRules();
+
+        $ruleNames = [];
+        foreach ($rules as $rule) {
+            $ruleNames[] = $rule->name;
         }
+
+        $this->assertContains('isReallyReallyAuthor', $ruleNames);
+        $this->assertContains('isAuthor', $ruleNames);
+    }
+
+    public function testRemoveRule()
+    {
+        $this->auth->removeRule('isAuthor');
+        $rules = $this->auth->getRules();
+
+        $this->assertEmpty($rules);
+    }
+
+    public function testExecuteRule()
+    {
+        $this->assertTrue($this->auth->executeRule(null, [], null));
+        $this->assertTrue($this->auth->executeRule('isAuthor', ['userID' => 1, 'authorID' => 1], null));
+        $this->assertFalse($this->auth->executeRule('isAuthor', ['userID' => 1, 'authorID' => 2], null));
     }
 
     public function testCheckAccess()
@@ -231,12 +281,14 @@ abstract class ManagerTestCase extends TestCase
 
     protected function prepareData()
     {
+        $this->auth->saveRule(new AuthorRule());
+
         $this->auth->createOperation('createPost', 'create a post');
         $this->auth->createOperation('readPost', 'read a post');
         $this->auth->createOperation('updatePost', 'update a post');
         $this->auth->createOperation('deletePost', 'delete a post');
 
-        $task = $this->auth->createTask('updateOwnPost', 'update a post by author himself', 'return $params["authorID"] == $params["userID"];');
+        $task = $this->auth->createTask('updateOwnPost', 'update a post by author himself', 'isAuthor');
         $task->addChild('updatePost');
 
         $role = $this->auth->createRole('reader');

tests/unit/framework/rbac/PhpManagerTest.php

@@ -7,6 +7,7 @@ use yii\rbac\PhpManager;
 
 /**
  * @group rbac
+ * @property \yii\rbac\PhpManager $auth
  */
 class PhpManagerTest extends ManagerTestCase
 {