From 336c3e99d23325fbad6d1f41808ba6349d67affb Mon Sep 17 00:00:00 2001 From: Nate Abele Date: Tue, 30 Aug 2011 18:24:42 -0400 Subject: [PATCH 01/25] Fixing whitespace and formatting. Refactoring `Rules` adapter to eliminate static defs, and adding array- and pattern-based IP matching. Fixing docblocks, cleaning up tests. --- README.md | 26 +- extensions/adapter/security/access/Rules.php | 238 +++++---- security/Access.php | 89 ++-- security/AccessDeniedException.php | 3 +- .../adapter/security/access/AuthRbacTest.php | 464 ++++++++++-------- .../adapter/security/access/RulesTest.php | 217 +++++--- .../adapter/security/access/SimpleTest.php | 41 +- tests/cases/security/AccessTest.php | 100 ++-- 8 files changed, 665 insertions(+), 513 deletions(-) diff --git a/README.md b/README.md index 6600563..7ffb098 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ -#Access control library for the Lithium framework. +# Access control library for the Lithium framework. -##Installation +## Installation Checkout the code to either of your library directories: @@ -11,7 +11,7 @@ Include the library in in your `/app/config/bootstrap/libraries.php` Libraries::add('li3_access'); -##Usage +## Usage You must configure the adapter you wish to use first, but once you have it configured it's fairly simple to use. @@ -22,11 +22,11 @@ You must configure the adapter you wish to use first, but once you have it confi If the request validates correctly based on your configuration then `Access::check()` will return an empty `array()` otherwise it will return and array with two keys; `message` and `redirect`. These values are built into the Access class but you can override them but passing them as `$options` to all three of the adapters in this repository. -##Configuration +## Configuration In this repository there are three adapters. All three work in a slightly different way. -###Simple Adapter +### sSimple Adapter The simple adapter is exactly what it says it is. The check method only checks that the data passed to is not empty and as a result the configuration is trivial. @@ -36,7 +36,7 @@ The simple adapter is exactly what it says it is. The check method only checks t And that's it! -###Rules Adapter +### Rules Adapter This adapter effectively allows you to tell it how it should work. It comes with a few preconfigured rules by default but it's very simple to add your own. Its configuration is the same as the `Simple` adapter if you only want to use the built in methods. @@ -63,7 +63,7 @@ Then to use your new rule: One more to go! -###AuthRbac Adapter +### AuthRbac Adapter This is the most complex adapter in this repository at this time. It's used for Role Based Access Control. You define a set of roles (or conditions) to match the request against, if the request matches your conditions the adapter then checks to see if the user is authenticated with the appropriate `\lithium\security\Auth` configurations to be granted access. @@ -101,7 +101,7 @@ First we tell it which adapter to use: Then we set the roles array. This array is required if you want to use this adapter. The roles are evaluated from top to bottom. So if a role at the bottom contradicts one closer to the top, the bottom will take precedence. -####There are five possible options you can specify for a single role. +#### There are five possible options you can specify for a single role. *match* @@ -148,7 +148,7 @@ Setting 'requester' => array('user', 'customer') would only apply the rule to an A boolean that if set to false forces a role that would have been granted access to deny access. This way you can apply a rule to everyone and then proceed to exclude requesters manualy. -###Filters +### Filters The Access::check() method is filterable. You can apply the filters in the configuration like so: @@ -164,9 +164,9 @@ The Access::check() method is filterable. You can apply the filters in the confi ) )); -##Credits +## Credits -###Tom Maiaroto +### Tom Maiaroto The original author of this library. @@ -174,7 +174,7 @@ Github: [tmaiaroto](https://github.com/tmaiaroto/li3_access) Website: [Shift8 Creative](http://www.shift8creative.com) -##Weluse +## Weluse Wrote the original Rbac adapter. @@ -182,7 +182,7 @@ Github: [dgAlien](https://github.com/dgAlien/li3_access) [weluse](https://github Website: [Weluse](http://www.weluse.de) -##rich97 +## rich97 Modified the original Rbac adapter, added some tests and wrote this version of the documentation. diff --git a/extensions/adapter/security/access/Rules.php b/extensions/adapter/security/access/Rules.php index b5900e9..1a10a7a 100644 --- a/extensions/adapter/security/access/Rules.php +++ b/extensions/adapter/security/access/Rules.php @@ -4,114 +4,150 @@ use lithium\util\Set; +/** + * undocumented class + */ class Rules extends \lithium\core\Object { - // Rules are closures that must return true or false - protected static $_rules = array(); - - // Set some default rules to use - public static function __init() { - self::$_rules = array( - 'allowAll' => function() { - return true; - }, - 'denyAll' => function() { - return false; - }, - 'allowAnyUser' => function($user) { - return (!empty($user)) ? true:false; - }, - 'allowIp' => function($user, $request, $options) { - $options += array('ip' => false); - return $_SERVER['REMOTE_ADDR'] == $options['ip']; - } - ); - } - - /** - * The `Rules` adapter will use check to test the provided data - * against a number of given rules. Extra data that may be required - * to make an informed decision about access can be passed in the - * $options array. This extra data will vary from app to app and rules - * will need to be added to handle it. The default rules assume some - * general cases and more can be added or passed directly to this method. - * - * @param mixed $user The user data array that holds all necessary information about - * the user requesting access. Or false (because Auth::check() can return false). - * @param object $request The Lithium Request object. - * @param array $options An array of additional options. - * @return Array An empty array if access is allowed and an array with reasons for denial if denied. - */ - public function check($user, $request, array $options = array()) { - $options += array('rules' => array()); - if(empty($options['rules'])) { - return array('rule' => false, 'message' => $options['message'], 'redirect' => $options['redirect']); + /** + * Rules are named closures that must either return `true` or `false`. + * + * @var array + */ + protected $_rules = array(); + + /** + * Initializes default rules to use. + * + * @return void + */ + protected function _init() { + $this->_rules = array( + 'allowAll' => function() { + return true; + }, + 'denyAll' => function() { + return false; + }, + 'allowAnyUser' => function($user) { + return $user ? true : false; + }, + 'allowIp' => function($user, $request, $options) { + $options += array('ip' => false); + + if (is_string($options['ip']) && strpos($options['ip'], '/') === 0) { + return (boolean) preg_match($options['ip'], $request->env('REMOTE_ADDR')); + } + if (is_array($options['ip'])) { + return in_array($request->env('REMOTE_ADDR'), $options['ip']); + } + return $request->env('REMOTE_ADDR') == $options['ip']; + } + ); } - - // If a single rule was passed, wrap it in an array so it can be iterated as if there were multiple - $rules = (isset($options['rules']['rule'])) ? array($options['rules']):$options['rules']; - - $access_response = array(); - - // Loop through all the rules. They must all pass. - foreach($rules as $rule) { - // make sure the rule is set and is a string to check for a closure to call or a closure itself - if((isset($rule['rule'])) && ((is_string($rule['rule'])) || (is_callable($rule['rule'])))) { - - $rule_result = false; + + /** + * The `Rules` adapter will use check to test the provided data + * against a number of given rules. Extra data that may be required + * to make an informed decision about access can be passed in the + * `$options` array. This extra data will vary from app to app and rules + * will need to be added to handle it. The default rules assume some + * general cases and more can be added or passed directly to this method. + * + * @param mixed $user The user data array that holds all necessary information about + * the user requesting access. Or false (because `Auth::check()` can return `false`). + * @param object $request The Lithium `Request` object. + * @param array $options An array of additional options. + * @return array An empty array if access is allowed and an array with reasons for denial + * if denied. + */ + public function check($user, $request, array $options = array()) { + $defaults = array('rules' => array()); + $options += $defaults; + + if (!$options['rules']) { + return array( + 'rule' => false, + 'message' => $options['message'], + 'redirect' => $options['redirect'] + ); + } + + // If a single rule was passed, wrap it in an array so it can be iterated as if + // there were multiple + $rules = (isset($options['rules']['rule'])) ? array($options['rules']) : $options['rules']; + $result = array(); + + // Loop through all the rules. They must all pass. + foreach ($rules as $rule) { + // make sure the rule is set and is a string to check for a closure to call or a + // closure itself. + $hasRule = ( + isset($rule['rule']) && + (is_string($rule['rule']) || is_callable($rule['rule'])) + ); + + if (!$hasRule) { + continue; + } + + if ($this->_call($rule, $user, $request) === false) { + $result['rule'] = $rule['rule']; + $result['message'] = isset($rule['message']) ? $rule['message'] : $options['message']; + $result['redirect'] = isset($rule['redirect']) ? $rule['redirect'] : $options['redirect']; + } + } + return $result; + } + + protected function _call($rule, $user, $request) { // The added rule closure will be passed the user data - if(in_array($rule['rule'], array_keys(self::$_rules))) { - // The rule closure will be passed the user, request and the rule array itself which could contain extra data required by the specific rule. - $rule_result = call_user_func(self::$_rules[$rule['rule']], $user, $request, $rule); - } elseif(is_callable($rule['rule'])) { - // The rule can be defined as a closure on the fly, no need to call add() - $rule_result = call_user_func($rule['rule'], $user, $request, $rule); + if (in_array($rule['rule'], array_keys($this->_rules))) { + // The rule closure will be passed the user, request and the rule array itself + // which could contain extra data required by the specific rule. + return call_user_func($this->_rules[$rule['rule']], $user, $request, $rule); } - - if($rule_result === false) { - $access_response['rule'] = $rule['rule']; - $access_response['message'] = (isset($rule['message'])) ? $rule['message']:$options['message']; - $access_response['redirect'] = (isset($rule['redirect'])) ? $rule['redirect']:$options['redirect']; + if (is_callable($rule['rule'])) { + // The rule can be defined as a closure on the fly, no need to call add() + return call_user_func($rule['rule'], $user, $request, $rule); } - - } - + return false; } - - return $access_response; - } - - /** - * Adds an Access rule. This works much like the Validator class. - * All rules should be anonymous functions and will be passed - * $user, $request, and $options which will contain the entire - * rule array which contains its own name plus other data that - * could be used to determine access. - * - * @param string $name The rule name. - * @param function $rule The closure for the rule, which has to return true or false. - */ - public static function add($name, $rule = null) { - if (!is_array($name)) { - $name = array($name => $rule); - } - self::$_rules = Set::merge(self::$_rules, $name); - } - - /** - * Simply returns the rules that are currently available. - * Optionally, passing a name will return just that rule - * or false if it doesn't exist. - * - * @param string $name The rule name (optional). - * @return mixed Either an array of rule closures, a single rule closure, or false. - */ - public function getRules($name = false) { - if($name) { - return (isset(self::$_rules[$name])) ? self::$_rules[$name]:false; + + /** + * Adds an Access rule. This works much like the Validator class. + * All rules should be anonymous functions and will be passed + * $user, $request, and $options which will contain the entire + * rule array which contains its own name plus other data that + * could be used to determine access. + * + * @param string $name The rule name. + * @param function $rule The closure for the rule, which has to return true or false. + */ + public function add($name, $rule = null) { + $this->_rules = Set::merge($this->_rules, is_array($name) ? $name : array($name => $rule)); + } + + /** + * Simply returns the rules that are currently available. Optionally, passing a name will return + * just that rule or `false` if it doesn't exist. + * + * @param string $name The rule name (optional). + * @return mixed Either an array of rule closures, a single rule closure, or `false`. + */ + public function get($name = null) { + if ($name) { + return isset($this->_rules[$name]) ? $this->_rules[$name] : false; + } + return $this->_rules; + } + + /** + * @deprecated + */ + public function getRules($name = null) { + return $this->get($name); } - return self::$_rules; - } - } + ?> \ No newline at end of file diff --git a/security/Access.php b/security/Access.php index d4ac4eb..fe53f35 100644 --- a/security/Access.php +++ b/security/Access.php @@ -23,45 +23,45 @@ */ class Access extends \lithium\core\Adaptable { - /** - * Stores configurations for various authentication adapters. - * - * @var object `Collection` of authentication configurations. - */ - protected static $_configurations = array(); + /** + * Stores configurations for various authentication adapters. + * + * @var object `Collection` of authentication configurations. + */ + protected static $_configurations = array(); - /** - * Libraries::locate() compatible path to adapters for this class. - * - * @see lithium\core\Libraries::locate() - * @var string Dot-delimited path. - */ - protected static $_adapters = 'adapter.security.access'; + /** + * Libraries::locate() compatible path to adapters for this class. + * + * @see lithium\core\Libraries::locate() + * @var string Dot-delimited path. + */ + protected static $_adapters = 'adapter.security.access'; - /** - * Dynamic class dependencies. - * - * @var array Associative array of class names & their namespaces. - */ - protected static $_classes = array( - ); + /** + * Dynamic class dependencies. + * + * @var array Associative array of class names & their namespaces. + */ + protected static $_classes = array( + ); - /** - * Called when an adapter configuration is first accessed, this method sets the default - * configuration for session handling. While each configuration can use its own session class - * and options, this method initializes them to the default dependencies written into the class. - * For the session key name, the default value is set to the name of the configuration. - * - * @param string $name The name of the adapter configuration being accessed. - * @param array $config The user-specified configuration. - * @return array Returns an array that merges the user-specified configuration with the - * generated default values. - */ - protected static function _initConfig($name, $config) { - $defaults = array(); - $config = parent::_initConfig($name, $config) + $defaults; - return $config; - } + /** + * Called when an adapter configuration is first accessed, this method sets the default + * configuration for session handling. While each configuration can use its own session class + * and options, this method initializes them to the default dependencies written into the class. + * For the session key name, the default value is set to the name of the configuration. + * + * @param string $name The name of the adapter configuration being accessed. + * @param array $config The user-specified configuration. + * @return array Returns an array that merges the user-specified configuration with the + * generated default values. + */ + protected static function _initConfig($name, $config) { + $defaults = array(); + $config = parent::_initConfig($name, $config) + $defaults; + return $config; + } /** * Performs an access check against the specified configuration, and returns true @@ -77,11 +77,13 @@ protected static function _initConfig($name, $config) { * the user requesting access. Or `false` (because Auth::check() can return `false`). * @param object $request The Lithium Request object. * @param array $options An array of additional options. - * @return Array An empty array if access is allowed and an array with reasons for denial if denied. + * @return Array An empty array if access is allowed and an array with reasons for denial + * if denied. */ public static function check($name, $user, $request, array $options = array()) { $defaults = array( - 'message' => 'You are not permitted to access this area.', 'redirect' => '/' + 'message' => 'You are not permitted to access this area.', + 'redirect' => '/' ); $options += $defaults; @@ -89,13 +91,14 @@ public static function check($name, $user, $request, array $options = array()) { throw new ConfigException("Configuration `{$name}` has not been defined."); } $filter = function($self, $params) use ($name) { - return $self::adapter($name)->check($params['user'], $params['request'], $params['options']); + $user = $params['user']; + $request = $params['request']; + $options = $params['options']; + return $self::adapter($name)->check($user, $request, $options); }; - $filters = (array) $config['filters']; $params = compact('user', 'request', 'options'); - return static::_filter(__FUNCTION__, $params, $filter, $filters); + return static::_filter(__FUNCTION__, $params, $filter, (array) $config['filters']); } - } -?> +?> \ No newline at end of file diff --git a/security/AccessDeniedException.php b/security/AccessDeniedException.php index 9497918..7b05d91 100644 --- a/security/AccessDeniedException.php +++ b/security/AccessDeniedException.php @@ -9,8 +9,7 @@ namespace li3_access\security; /** - * A `NetworkException` may be thrown whenever an unsuccessful attempt is made to connect to a - * remote service over the network. This may be a web service, a database, or another network + * An `AccessDeniedException` is thrown whenever an unhandled attempt is made to access a restricted * resource. */ class AccessDeniedException extends \RuntimeException { diff --git a/tests/cases/extensions/adapter/security/access/AuthRbacTest.php b/tests/cases/extensions/adapter/security/access/AuthRbacTest.php index 0570819..5c0ac47 100644 --- a/tests/cases/extensions/adapter/security/access/AuthRbacTest.php +++ b/tests/cases/extensions/adapter/security/access/AuthRbacTest.php @@ -16,213 +16,259 @@ class AuthRbacTest extends \lithium\test\Unit { - public function setUp() { - Auth::config(array( - 'user' => array( - 'adapter' => '\li3_access\tests\mocks\extensions\adapter\auth\MockAuthAdapter' - ) - )); - - Access::config(array( - 'test_no_roles_configured' => array( - 'adapter' => 'AuthRbac' - ), - 'test_check' => array( - 'adapter' => 'AuthRbac', - 'roles' => array( - 'allow' => array( - 'requesters' => 'user', - 'match' => '*::*' - ) - ) - ), - 'test_closures' => array( - 'adapter' => 'AuthRbac', - 'roles' => array( - array( - 'requesters' => '*', - 'allow' => array(function($request, &$roleOptions) { - $roleOptions['message'] = 'Test allow options set.'; - return $request->params['allow'] ? true : false; - }), - 'match' => array( - function($request) { - return $request->params['match'] ? true : false; - }, - 'controller' => 'TestControllers', - 'action' => 'test_action' - ) - ) - ) - ), - 'test_message_override' => array( - 'adapter' => 'AuthRbac', - 'roles' => array( - array( - 'allow' => false, - 'requesters' => '*', - 'match' => '*::*' - ), - array( - 'message' => 'Rule access denied message.', - 'redirect' => '/', - 'requesters' => 'user', - 'match' => 'TestControllers::test_action' - ), - array( - 'message' => 'Test no overwrite.', - 'redirect' => '/test_no_overwrite', - 'requesters' => 'user', - 'match' => null - ) - ) - ) - )); - } - - public function tearDown() { - Auth::clear('user'); - } - - public function testCheck() { - $request = new Request(array('params' => array('library' => 'test_library', 'controller' => 'test_controllers', 'action' => 'test_action'))); - - $guest = array(); - $user = array('username' => 'test'); - - $request->data = $guest; - $expected = array('message' => 'You are not permitted to access this area.', 'redirect' => '/'); - $result = Access::check('test_check', $guest, $request, array('checkSession' => false)); - $this->assertIdentical($expected, $result); - - $request->data = $user; - $expected = array(); - $result = Access::check('test_check', $user, $request, array('checkSession' => false, 'success' => true)); - $this->assertIdentical($expected, $result); - } - - public function testCheckMessageOverride() { - $request = new Request(array('params' => array('library' => 'test_library', 'controller' => 'test_controllers', 'action' => 'test_action'))); - - $guest = array(); - $user = array('username' => 'test'); - - $request->data = $guest; - $expected = array('message' => 'Rule access denied message.', 'redirect' => '/'); - $result = Access::check('test_message_override', $guest, $request, array('checkSession' => false)); - $this->assertIdentical($expected, $result); - - $request->data = $user; - $expected = array(); - $result = Access::check('test_message_override', $user, $request, array('checkSession' => false, 'success' => 'true')); - $this->assertIdentical($expected, $result); - - $request->params = array('controller' => 'test_controllers', 'action' => 'test_deinied_action'); - - $request->data = $guest; - $expected = array('message' => 'You are not permitted to access this area.', 'redirect' => '/'); - $result = Access::check('test_message_override', $guest, $request, array('checkSession' => false)); - $this->assertIdentical($expected, $result); - - $request->data = $user; - $expected = array('message' => 'You are not permitted to access this area.', 'redirect' => '/'); - $result = Access::check('test_message_override', $user, $request, array('checkSession' => false)); - $this->assertIdentical($expected, $result); - - $request->data = $user; - $expected = array('message' => 'Message override!', 'redirect' => '/new_redirect'); - $result = Access::check('test_message_override', $user, $request, array('checkSession' => false, 'message' => 'Message override!', 'redirect' => '/new_redirect')); - $this->assertIdentical($expected, $result); - } - - public function testParseMatch() { - $request = new Request(array('params' => array('library' => 'test_library', 'controller' => 'test_controllers', 'action' => 'test_action'))); - - $match = array('library' => 'test_library', 'controller' => 'TestControllers', 'action' => 'test_action'); - $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); - - $match = array('controller' => 'TestControllers', 'action' => 'test_action'); - $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); - - $match = array('library' => 'test_library', 'action' => 'test_action'); - $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); - - $match = array('library' => 'test_library', 'controller' => 'TestControllers'); - $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); - - $match = array('library' => 'test_no_match', 'controller' => 'TestControllers', 'action' => 'test_action'); - $this->assertFalse(Access::adapter('test_check')->parseMatch($match, $request)); - - $match = 'TestControllers::test_action'; - $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); - - $match = 'TestControllers::*'; - $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); - - $match = '*::test_action'; - $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); - - $match = '*::*'; - $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); - - $match = array('library' => 'test_library', '*::*'); - $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); - - $match = array('library' => 'test_no_match', '*::*'); - $this->assertFalse(Access::adapter('test_check')->parseMatch($match, $request)); - - $match = null; - $this->assertFalse(Access::adapter('test_check')->parseMatch($match, $request)); - - $match = true; - $test = function() { return true; }; - $this->assertTrue(Access::adapter('test_closures')->parseMatch(array($test), $request)); - - $match = false; - $test = function() { return false; }; - $this->assertFalse(Access::adapter('test_closures')->parseMatch(array($test), $request)); - - $match = false; - $this->assertFalse(Access::adapter('test_closures')->parseMatch(array(), $request)); - } - - public function testClosures() { - $request = new Request(array('params' => array('controller' => 'test_controllers', 'action' => 'test_action'))); - - $user = $request->data = array('username' => 'test'); - $authSuccess = array('checkSession' => false, 'success' => true); - - $request->params['match'] = true; - $request->params['allow'] = true; - $expected = array(); - $result = Access::check('test_closures', $user, $request, $authSuccess); - $this->assertIdentical($expected, $result); - - $request->params['match'] = true; - $request->params['allow'] = false; - $expected = array('message' => 'Test allow options set.', 'redirect' => '/'); - $result = Access::check('test_closures', $user, $request, $authSuccess); - $this->assertIdentical($expected, $result); - - $request->params = array('controller' => 'TestControllers', 'action' => 'bad_action'); - - $request->params['match'] = true; - $request->params['allow'] = true; - $result = Access::check('test_closures', $user, $request, $authSuccess); - $expected = array('message' => 'You are not permitted to access this area.', 'redirect' => '/'); - $this->assertIdentical($expected, $result); - } - - public function testNoRolesConfigured() { - $request = new Request(); - - $config = Access::config('test_no_roles_configured'); - $request->params = array('controller' => 'Tests', 'action' => 'granted'); - - $this->assertTrue(empty($config['roles'])); - $this->expectException('No roles defined for adapter configuration.'); - Access::check('test_no_roles_configured', array('guest' => null), $request); - } - + public function setUp() { + Auth::config(array( + 'user' => array( + 'adapter' => 'li3_access\tests\mocks\extensions\adapter\auth\MockAuthAdapter' + ) + )); + + Access::config(array( + 'test_no_roles_configured' => array('adapter' => 'AuthRbac'), + 'test_check' => array( + 'adapter' => 'AuthRbac', + 'roles' => array( + 'allow' => array( + 'requesters' => 'user', + 'match' => '*::*' + ) + ) + ), + 'test_closures' => array( + 'adapter' => 'AuthRbac', + 'roles' => array( + array( + 'requesters' => '*', + 'allow' => array(function($request, &$roleOptions) { + $roleOptions['message'] = 'Test allow options set.'; + return $request->params['allow'] ? true : false; + }), + 'match' => array( + function($request) { + return $request->params['match'] ? true : false; + }, + 'controller' => 'TestControllers', + 'action' => 'test_action' + ) + ) + ) + ), + 'test_message_override' => array( + 'adapter' => 'AuthRbac', + 'roles' => array( + array( + 'allow' => false, + 'requesters' => '*', + 'match' => '*::*' + ), + array( + 'message' => 'Rule access denied message.', + 'redirect' => '/', + 'requesters' => 'user', + 'match' => 'TestControllers::test_action' + ), + array( + 'message' => 'Test no overwrite.', + 'redirect' => '/test_no_overwrite', + 'requesters' => 'user', + 'match' => null + ) + ) + ) + )); + } + + public function tearDown() { + Auth::clear('user'); + } + + public function testCheck() { + $request = new Request(array('params' => array( + 'library' => 'test_library', + 'controller' => 'test_controllers', + 'action' => 'test_action' + ))); + + $guest = array(); + $user = array('username' => 'test'); + + $request->data = $guest; + $expected = array( + 'message' => 'You are not permitted to access this area.', + 'redirect' => '/' + ); + $result = Access::check('test_check', $guest, $request, array('checkSession' => false)); + $this->assertIdentical($expected, $result); + + $request->data = $user; + $expected = array(); + $result = Access::check('test_check', $user, $request, array( + 'checkSession' => false, + 'success' => true + )); + $this->assertIdentical($expected, $result); + } + + public function testCheckMessageOverride() { + $request = new Request(array('params' => array( + 'library' => 'test_library', + 'controller' => 'test_controllers', + 'action' => 'test_action' + ))); + + $guest = array(); + $user = array('username' => 'test'); + + $request->data = $guest; + $expected = array('message' => 'Rule access denied message.', 'redirect' => '/'); + $result = Access::check('test_message_override', $guest, $request, array( + 'checkSession' => false + )); + $this->assertIdentical($expected, $result); + + $request->data = $user; + $expected = array(); + $result = Access::check('test_message_override', $user, $request, array( + 'checkSession' => false, + 'success' => 'true' + )); + $this->assertIdentical($expected, $result); + + $request->params = array( + 'controller' => 'test_controllers', + 'action' => 'test_deinied_action' + ); + + $request->data = $guest; + $expected = array( + 'message' => 'You are not permitted to access this area.', + 'redirect' => '/' + ); + $result = Access::check('test_message_override', $guest, $request, array( + 'checkSession' => false + )); + $this->assertIdentical($expected, $result); + + $request->data = $user; + $expected = array( + 'message' => 'You are not permitted to access this area.', + 'redirect' => '/' + ); + $result = Access::check('test_message_override', $user, $request, array( + 'checkSession' => false + )); + $this->assertIdentical($expected, $result); + + $request->data = $user; + $expected = array('message' => 'Message override!', 'redirect' => '/new_redirect'); + $result = Access::check('test_message_override', $user, $request, array( + 'checkSession' => false, + 'message' => 'Message override!', + 'redirect' => '/new_redirect' + )); + $this->assertIdentical($expected, $result); + } + + public function testParseMatch() { + $request = new Request(array('params' => array( + 'library' => 'test_library', + 'controller' => 'test_controllers', + 'action' => 'test_action' + ))); + + $match = array( + 'library' => 'test_library', + 'controller' => 'TestControllers', + 'action' => 'test_action' + ); + $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); + + $match = array('controller' => 'TestControllers', 'action' => 'test_action'); + $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); + + $match = array('library' => 'test_library', 'action' => 'test_action'); + $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); + + $match = array('library' => 'test_library', 'controller' => 'TestControllers'); + $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); + + $match = array( + 'library' => 'test_no_match', + 'controller' => 'TestControllers', + 'action' => 'test_action' + ); + $this->assertFalse(Access::adapter('test_check')->parseMatch($match, $request)); + + $match = 'TestControllers::test_action'; + $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); + + $match = 'TestControllers::*'; + $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); + + $match = '*::test_action'; + $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); + + $match = '*::*'; + $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); + + $match = array('library' => 'test_library', '*::*'); + $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); + + $match = array('library' => 'test_no_match', '*::*'); + $this->assertFalse(Access::adapter('test_check')->parseMatch($match, $request)); + + $match = null; + $this->assertFalse(Access::adapter('test_check')->parseMatch($match, $request)); + + $test = function() { return true; }; + $this->assertTrue(Access::adapter('test_closures')->parseMatch(array($test), $request)); + + $test = function() { return false; }; + $this->assertFalse(Access::adapter('test_closures')->parseMatch(array($test), $request)); + $this->assertFalse(Access::adapter('test_closures')->parseMatch(array(), $request)); + } + + public function testClosures() { + $request = new Request(array('params' => array( + 'controller' => 'test_controllers', 'action' => 'test_action' + ))); + + $user = $request->data = array('username' => 'test'); + $authSuccess = array('checkSession' => false, 'success' => true); + + $request->params['match'] = true; + $request->params['allow'] = true; + $result = Access::check('test_closures', $user, $request, $authSuccess); + $this->assertIdentical(array(), $result); + + $request->params['match'] = true; + $request->params['allow'] = false; + $expected = array('message' => 'Test allow options set.', 'redirect' => '/'); + $result = Access::check('test_closures', $user, $request, $authSuccess); + $this->assertIdentical($expected, $result); + + $request->params = array('controller' => 'TestControllers', 'action' => 'bad_action'); + + $request->params['match'] = true; + $request->params['allow'] = true; + $result = Access::check('test_closures', $user, $request, $authSuccess); + $expected = array( + 'message' => 'You are not permitted to access this area.', + 'redirect' => '/' + ); + $this->assertIdentical($expected, $result); + } + + public function testNoRolesConfigured() { + $request = new Request(); + + $config = Access::config('test_no_roles_configured'); + $request->params = array('controller' => 'Tests', 'action' => 'granted'); + + $this->assertTrue(empty($config['roles'])); + $this->expectException('No roles defined for adapter configuration.'); + Access::check('test_no_roles_configured', array('guest' => null), $request); + } } -?> + +?> \ No newline at end of file diff --git a/tests/cases/extensions/adapter/security/access/RulesTest.php b/tests/cases/extensions/adapter/security/access/RulesTest.php index 3b645b0..f0cbcbb 100644 --- a/tests/cases/extensions/adapter/security/access/RulesTest.php +++ b/tests/cases/extensions/adapter/security/access/RulesTest.php @@ -9,85 +9,150 @@ namespace li3_access\tests\cases\extensions\adapter\security\access; -use lithium\net\http\Request; +use lithium\action\Request; use li3_access\security\Access; class RulesTest extends \lithium\test\Unit { - public function setUp() { - Access::config(array( - 'test_rulebased' => array( - 'adapter' => 'Rules' - ) - )); - } - - public function tearDown() {} - - public function testCheck() { - $request = new Request(); - - // Multiple rules, they should all pass - $rules = array( - array('rule' => 'allowAnyUser', 'message' => 'You must be logged in.'), - array('rule' => 'allowAll', 'message' => 'You must be logged in.'), - array('rule' => 'allowIp', 'message' => 'You can not access this from your location. (IP: ' . $_SERVER['REMOTE_ADDR'] . ')', 'ip' => $_SERVER['REMOTE_ADDR']) - ); - $expected = array(); - $result = Access::check('test_rulebased', array('username' => 'Tom'), $request, array('rules' => $rules)); - $this->assertEqual($expected, $result); - - // Single rule in multi-demnsional array - $rules = array( - array('rule' => 'denyAll', 'message' => 'You must be logged in.') - ); - $expected = array('rule' => 'denyAll', 'message' => 'You must be logged in.', 'redirect' => '/'); - $result = Access::check('test_rulebased', array('username' => 'Tom'), $request, array('rules' => $rules)); - $this->assertEqual($expected, $result); - - // Single rule (single array), but it should fail because user is an empty array - $rules = array('rule' => 'allowAnyUser', 'message' => 'You must be logged in.'); - $expected = array('rule' => 'allowAnyUser', 'message' => 'You must be logged in.', 'redirect' => '/'); - $result = Access::check('test_rulebased', array(), $request, array('rules' => $rules)); - $this->assertEqual($expected, $result); - // and if false instead of an empty array (because one might typically run Auth:check() which could return false) - $result = Access::check('test_rulebased', false, $request, array('rules' => $rules)); - $this->assertEqual($expected, $result); - - // No rules - $expected = array('rule' => false, 'message' => 'You are not permitted to access this area.', 'redirect' => '/'); - $result = Access::check('test_rulebased', array('username' => 'Tom'), $request); - $this->assertEqual($expected, $result); - - // Adding a rule "on the fly" by passing a closure, this rule should pass - $rules = array( - array('rule' => function($user, $request, $options) { return $user['username'] == 'Tom'; }, 'message' => 'Access denied.') - ); - $expected = array(); - $result = Access::check('test_rulebased', array('username' => 'Tom'), $request, array('rules' => $rules)); - $this->assertEqual($expected, $result); - } - - public function testAdd() { - $request = new Request(); - - // The add() method to add a rule - Access::adapter('test_rulebased')->add('testDeny', function($user, $request, $options) { - return false; - }); - - $rules = array( - array('rule' => 'testDeny', 'message' => 'Access denied.') - ); - $expected = array('rule' => 'testDeny', 'message' => 'Access denied.', 'redirect' => '/'); - $result = Access::check('test_rulebased', array('username' => 'Tom'), $request, array('rules' => $rules)); - $this->assertEqual($expected, $result); - - // Make sure the rule got added to the $_rules property - $this->assertTrue(is_callable(Access::adapter('test_rulebased')->getRules('testDeny'))); - - $this->assertTrue(is_array(Access::adapter('test_rulebased')->getRules())); - } - + public function setUp() { + Access::config(array( + 'test_rulebased' => array('adapter' => 'Rules') + )); + } + + public function tearDown() {} + + public function testPatternBasedIpMatching() { + $request = new Request(array('env' => array('REMOTE_ADDR' => '10.0.1.2'))); + + // Multiple rules, they should all pass + $rules = array( + array( + 'rule' => 'allowIp', + 'message' => 'You can not access this from your location.', + 'ip' => '/10\.0\.1\.\d+/' + ) + ); + $result = Access::check('test_rulebased', array(), $request, compact('rules')); + $this->assertEqual(array(), $result); + + $request = new Request(array('env' => array('REMOTE_ADDR' => '10.0.1.255'))); + $result = Access::check('test_rulebased', array(), $request, compact('rules')); + $this->assertEqual(array(), $result); + + $request = new Request(array('env' => array('REMOTE_ADDR' => '10.0.2.1'))); + $result = Access::check('test_rulebased', array(), $request, compact('rules')); + $this->assertEqual('You can not access this from your location.', $result['message']); + } + + public function testArrayBasedIpMatching() { + // Multiple rules, they should all pass + $rules = array( + array( + 'rule' => 'allowIp', + 'message' => 'You can not access this from your location.', + 'ip' => array('10.0.1.2', '10.0.1.3', '10.0.1.4') + ) + ); + + foreach (array(2, 3, 4) as $i) { + $request = new Request(array('env' => array('REMOTE_ADDR' => "10.0.1.{$i}"))); + $result = Access::check('test_rulebased', array(), $request, compact('rules')); + $this->assertEqual(array(), $result); + } + + foreach (array(1, 5, 255) as $i) { + $request = new Request(array('env' => array('REMOTE_ADDR' => "10.0.1.{$i}"))); + $result = Access::check('test_rulebased', array(), $request, compact('rules')); + $this->assertEqual('You can not access this from your location.', $result['message']); + } + } + + public function testCheck() { + $request = new Request(array('env' => array('REMOTE_ADDR' => '10.0.1.1'))); + + // Multiple rules, they should all pass + $rules = array( + array('rule' => 'allowAnyUser', 'message' => 'You must be logged in.'), + array('rule' => 'allowAll', 'message' => 'You must be logged in.'), + array( + 'rule' => 'allowIp', + 'message' => 'You can not access this from your location. (IP: 10.0.1.1)', + 'ip' => '10.0.1.1' + ) + ); + $result = Access::check('test_rulebased', array('username' => 'Tom'), $request, array( + 'rules' => $rules + )); + $this->assertEqual(array(), $result); + + // Single rule in multi-demnsional array + $rules = array(array('rule' => 'denyAll', 'message' => 'You must be logged in.')); + $expected = array( + 'rule' => 'denyAll', 'message' => 'You must be logged in.', 'redirect' => '/' + ); + $result = Access::check('test_rulebased', array('username' => 'Tom'), $request, array( + 'rules' => $rules + )); + $this->assertEqual($expected, $result); + + // Single rule (single array), but it should fail because user is an empty array + $rules = array('rule' => 'allowAnyUser', 'message' => 'You must be logged in.'); + $expected = array( + 'rule' => 'allowAnyUser', 'message' => 'You must be logged in.', 'redirect' => '/' + ); + $result = Access::check('test_rulebased', array(), $request, array('rules' => $rules)); + $this->assertEqual($expected, $result); + + // and if false instead of an empty array (because one might typically run Auth:check() + // which could return false) + $result = Access::check('test_rulebased', false, $request, array('rules' => $rules)); + $this->assertEqual($expected, $result); + + // No rules + $expected = array( + 'rule' => false, + 'message' => 'You are not permitted to access this area.', + 'redirect' => '/' + ); + $result = Access::check('test_rulebased', array('username' => 'Tom'), $request); + $this->assertEqual($expected, $result); + + // Adding a rule "on the fly" by passing a closure, this rule should pass + $rules = array( + array( + 'rule' => function($user, $request, $options) { + return $user['username'] == 'Tom'; + }, + 'message' => 'Access denied.' + ) + ); + $expected = array(); + $result = Access::check('test_rulebased', array('username' => 'Tom'), $request, array( + 'rules' => $rules + )); + $this->assertEqual($expected, $result); + } + + public function testAdd() { + $request = new Request(); + + // The add() method to add a rule + Access::adapter('test_rulebased')->add('testDeny', function($user, $request, $options) { + return false; + }); + + $rules = array(array('rule' => 'testDeny', 'message' => 'Access denied.')); + $expected = array('rule' => 'testDeny', 'message' => 'Access denied.', 'redirect' => '/'); + $result = Access::check('test_rulebased', array('username' => 'Tom'), $request, array( + 'rules' => $rules + )); + $this->assertEqual($expected, $result); + + // Make sure the rule got added to the $_rules property + $this->assertTrue(is_callable(Access::adapter('test_rulebased')->get('testDeny'))); + $this->assertTrue(is_array(Access::adapter('test_rulebased')->get())); + } } + ?> diff --git a/tests/cases/extensions/adapter/security/access/SimpleTest.php b/tests/cases/extensions/adapter/security/access/SimpleTest.php index d045f0b..5a117c0 100644 --- a/tests/cases/extensions/adapter/security/access/SimpleTest.php +++ b/tests/cases/extensions/adapter/security/access/SimpleTest.php @@ -9,33 +9,32 @@ namespace li3_access\tests\cases\extensions\adapter\security\access; -use \li3_access\security\Access; -use \lithium\net\http\Request; +use li3_access\security\Access; +use lithium\net\http\Request; class SimpleTest extends \lithium\test\Unit { - public function setUp() { - Access::config(array( - 'test_access' => array( - 'adapter' => 'Simple' - ) - )); - } + public function setUp() { + Access::config(array( + 'test_access' => array('adapter' => 'Simple') + )); + } - public function tearDown() {} + public function tearDown() {} - public function testCheck() { - $request = new Request(); + public function testCheck() { + $request = new Request(); - $expected = array(); - $result = Access::check('test_access', array('username' => 'Tom'), $request); - $this->assertEqual($expected, $result); - - $expected = array('message' => 'Access denied.', 'redirect' => '/login'); - $result = Access::check('test_access', false, $request, array('redirect' => '/login', 'message' => 'Access denied.')); - $this->assertEqual($expected, $result); - } + $result = Access::check('test_access', array('username' => 'Tom'), $request); + $this->assertEqual(array(), $result); + $expected = array('message' => 'Access denied.', 'redirect' => '/login'); + $result = Access::check('test_access', false, $request, array( + 'redirect' => '/login', + 'message' => 'Access denied.' + )); + $this->assertEqual($expected, $result); + } } -?> +?> \ No newline at end of file diff --git a/tests/cases/security/AccessTest.php b/tests/cases/security/AccessTest.php index b433b37..c55cf70 100644 --- a/tests/cases/security/AccessTest.php +++ b/tests/cases/security/AccessTest.php @@ -9,63 +9,67 @@ namespace li3_access\tests\cases\security; -use \li3_access\security\Access; -use \lithium\net\http\Request; +use li3_access\security\Access; +use lithium\net\http\Request; class AccessTest extends \lithium\test\Unit { - public function setUp() { - Access::config(array( - 'test_access' => array( - 'adapter' => 'Simple' - ), - 'test_access_with_filters' => array( - 'adapter' => 'Simple', - 'filters' => array( - function($self, $params, $chain) { - return $chain->next($self, $params, $chain); - }, - function($self, $params, $chain) { - if (!$params['user']) { - return array('message' => 'Access denied.', 'redirect' => $params['options']['redirect']); - } else { - return $chain->next($self, $params, $chain); - } - } - ) - ) - )); - } + public function setUp() { + Access::config(array( + 'test_access' => array('adapter' => 'Simple'), + 'test_access_with_filters' => array( + 'adapter' => 'Simple', + 'filters' => array( + function($self, $params, $chain) { + return $chain->next($self, $params, $chain); + }, + function($self, $params, $chain) { + if (!$params['user']) { + return array( + 'message' => 'Access denied.', + 'redirect' => $params['options']['redirect'] + ); + } + return $chain->next($self, $params, $chain); + } + ) + ) + )); + } - public function tearDown() {} + public function tearDown() {} - public function testCheck() { - $request = new Request(); + public function testCheck() { + $request = new Request(); - $expected = array(); - $result = Access::check('test_access', array('username' => 'Tom'), $request); - $this->assertEqual($expected, $result); + $expected = array(); + $result = Access::check('test_access', array('username' => 'Tom'), $request); + $this->assertEqual($expected, $result); - $expected = array('message' => 'Access denied.', 'redirect' => '/login'); - $result = Access::check('test_access', false, $request, array('redirect' => '/login', 'message' => 'Access denied.')); - $this->assertEqual($expected, $result); - } + $expected = array('message' => 'Access denied.', 'redirect' => '/login'); + $result = Access::check('test_access', false, $request, array( + 'redirect' => '/login', + 'message' => 'Access denied.' + )); + $this->assertEqual($expected, $result); + } - public function testFilters() { - $request = new Request(); + public function testFilters() { + $request = new Request(); - $expected = array('message' => 'Access denied.', 'redirect' => '/login'); - $result = Access::check('test_access_with_filters', false, $request, array('redirect' => '/login')); - $this->assertEqual($expected, $result); - } + $expected = array('message' => 'Access denied.', 'redirect' => '/login'); + $result = Access::check('test_access_with_filters', false, $request, array( + 'redirect' => '/login' + )); + $this->assertEqual($expected, $result); + } - public function testNoConfigurations() { - $request = new Request(); - - Access::reset(); - $this->assertIdentical(array(), Access::config()); - $this->expectException("Configuration `test_no_config` has not been defined."); - Access::check('test_no_config', false, $request); - } + public function testNoConfigurations() { + Access::reset(); + $this->assertIdentical(array(), Access::config()); + $this->expectException("Configuration `test_no_config` has not been defined."); + Access::check('test_no_config', false, new Request()); + } } + ?> From bc51b6d8c915e408e245123fa60355e620d1a941 Mon Sep 17 00:00:00 2001 From: Nate Abele Date: Wed, 31 Aug 2011 08:10:03 -0400 Subject: [PATCH 02/25] Small refactoring to simplify `Rules` adapter. --- extensions/adapter/security/access/Rules.php | 9 ++------- extensions/adapter/security/access/Simple.php | 7 ++++--- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/extensions/adapter/security/access/Rules.php b/extensions/adapter/security/access/Rules.php index 1a10a7a..ed32e9e 100644 --- a/extensions/adapter/security/access/Rules.php +++ b/extensions/adapter/security/access/Rules.php @@ -76,7 +76,6 @@ public function check($user, $request, array $options = array()) { // If a single rule was passed, wrap it in an array so it can be iterated as if // there were multiple $rules = (isset($options['rules']['rule'])) ? array($options['rules']) : $options['rules']; - $result = array(); // Loop through all the rules. They must all pass. foreach ($rules as $rule) { @@ -86,18 +85,14 @@ public function check($user, $request, array $options = array()) { isset($rule['rule']) && (is_string($rule['rule']) || is_callable($rule['rule'])) ); - if (!$hasRule) { continue; } - if ($this->_call($rule, $user, $request) === false) { - $result['rule'] = $rule['rule']; - $result['message'] = isset($rule['message']) ? $rule['message'] : $options['message']; - $result['redirect'] = isset($rule['redirect']) ? $rule['redirect'] : $options['redirect']; + return $rule + array_diff_key($options, $defaults); } } - return $result; + return array(); } protected function _call($rule, $user, $request) { diff --git a/extensions/adapter/security/access/Simple.php b/extensions/adapter/security/access/Simple.php index c1e14fd..62f1d71 100644 --- a/extensions/adapter/security/access/Simple.php +++ b/extensions/adapter/security/access/Simple.php @@ -12,10 +12,11 @@ class Simple extends \lithium\core\Object { * It doesn't care about anything else. * * @param mixed $user The user data array that holds all necessary information about - * the user requesting access. Or false (because Auth::check() can return false). - * @param object $request The Lithium Request object. + * the user requesting access. Or `false` (because `Auth::check()` can return `false`). + * @param object $request The Lithium `Request` object. * @param array $options An array of additional options. - * @return Array An empty array if access is allowed and an array with reasons for denial if denied. + * @return Array An empty array if access is allowed and an array with reasons for denial + * if denied. */ public function check($user, $request, array $options = array()) { return !$user ? $options : array(); From c61d9a55fa2bdb70600227bc47faaef02bc74edd Mon Sep 17 00:00:00 2001 From: Nate Abele Date: Thu, 1 Sep 2011 08:33:35 -0400 Subject: [PATCH 03/25] Continued refactoring `Rules` adapter, cleaning up tests, and implementing `'allowAny'` option, to improve flexibility of rule-based validation. --- extensions/adapter/security/access/Rules.php | 107 +++++++---- .../adapter/security/access/RulesTest.php | 173 ++++++++++-------- 2 files changed, 173 insertions(+), 107 deletions(-) diff --git a/extensions/adapter/security/access/Rules.php b/extensions/adapter/security/access/Rules.php index ed32e9e..debc0a8 100644 --- a/extensions/adapter/security/access/Rules.php +++ b/extensions/adapter/security/access/Rules.php @@ -16,13 +16,48 @@ class Rules extends \lithium\core\Object { */ protected $_rules = array(); + /** + * Lists a subset of rules defined in `$_rules` which should be checked by default on every + * call to `check()` (unless overridden by passed options). + * + * @var array + */ + protected $_default = array(); + + /** + * Configuration that will be automatically assigned to class properties. + * + * @var array + */ + protected $_autoConfig = array('rules', 'default'); + + /** + * Sets default adapter configuration. + * + * @param array $config Adapter configuration, which includes the following default options: + * - `'rules'` _array_: An array of rules to be added to the default rules + * initialized by the adapter. See the `'rules'` option of the `check()` method + * for more information on the acceptable format of these values. + * - `'default'` _array_: The default list of rules to use when performing access + * checks. + * - `'allowAny'` _boolean_: If set to `true`, access checks will return successful + * if _any_ access rule passes. Otherwise, all are required to pass in order for + * the check to succeed. Defaults to `false`. + */ + public function __construct(array $config = array()) { + $defaults = array('rules' => array(), 'default' => array(), 'allowAny' => false); + parent::__construct($config + $defaults); + } + /** * Initializes default rules to use. * * @return void */ protected function _init() { - $this->_rules = array( + parent::_init(); + + $this->_rules += array( 'allowAll' => function() { return true; }, @@ -62,51 +97,61 @@ protected function _init() { * if denied. */ public function check($user, $request, array $options = array()) { - $defaults = array('rules' => array()); + $defaults = array( + 'rules' => $this->_config['default'], + 'allowAny' => $this->_config['allowAny'] + ); $options += $defaults; if (!$options['rules']) { - return array( - 'rule' => false, - 'message' => $options['message'], - 'redirect' => $options['redirect'] - ); + $base = array('rule' => false, 'message' => null, 'redirect' => null); + return array_diff_key($options, $defaults) + $base; } - // If a single rule was passed, wrap it in an array so it can be iterated as if - // there were multiple $rules = (isset($options['rules']['rule'])) ? array($options['rules']) : $options['rules']; + $result = array(); - // Loop through all the rules. They must all pass. foreach ($rules as $rule) { - // make sure the rule is set and is a string to check for a closure to call or a - // closure itself. - $hasRule = ( - isset($rule['rule']) && - (is_string($rule['rule']) || is_callable($rule['rule'])) - ); - if (!$hasRule) { - continue; + if (is_string($rule)) { + $rule = compact('rule'); } - if ($this->_call($rule, $user, $request) === false) { - return $rule + array_diff_key($options, $defaults); + $ruleResult = $this->_call($rule, $user, $request); + + switch (true) { + case ($ruleResult === false && $options['allowAny']): + $result = $rule + array_diff_key($options, $defaults); + break; + case ($ruleResult === false): + return $rule + array_diff_key($options, $defaults); + case ($ruleResult !== false && $options['allowAny']): + return array(); } } - return array(); + return $result; } + /** + * Extracts a callable rule either from a rule definition assigned as a closure, or a string + * reference to a rule defined in a key in the `$_rules` array. + * + * @param array $rule The rule definition array. + * @param mixed $user The value representing the user making the request. Usually an array. + * @param mixed $request The value representing request data or the object being access. + * @return boolean Returns `true` if the call to the rule was successful, otherwise `false` if + * the call failed, or if a callable rule was not found. + */ protected function _call($rule, $user, $request) { - // The added rule closure will be passed the user data - if (in_array($rule['rule'], array_keys($this->_rules))) { - // The rule closure will be passed the user, request and the rule array itself - // which could contain extra data required by the specific rule. - return call_user_func($this->_rules[$rule['rule']], $user, $request, $rule); - } - if (is_callable($rule['rule'])) { - // The rule can be defined as a closure on the fly, no need to call add() - return call_user_func($rule['rule'], $user, $request, $rule); + $callable = null; + + switch (true) { + case (is_callable($rule['rule'])): + $callable = $rule['rule']; + break; + case (in_array($rule['rule'], array_keys($this->_rules))): + $callable = $this->_rules[$rule['rule']]; + break; } - return false; + return is_callable($callable) ? call_user_func($callable, $user, $request, $rule) : false; } /** diff --git a/tests/cases/extensions/adapter/security/access/RulesTest.php b/tests/cases/extensions/adapter/security/access/RulesTest.php index f0cbcbb..4556818 100644 --- a/tests/cases/extensions/adapter/security/access/RulesTest.php +++ b/tests/cases/extensions/adapter/security/access/RulesTest.php @@ -10,68 +10,57 @@ namespace li3_access\tests\cases\extensions\adapter\security\access; use lithium\action\Request; -use li3_access\security\Access; +use li3_access\extensions\adapter\security\access\Rules; class RulesTest extends \lithium\test\Unit { - public function setUp() { - Access::config(array( - 'test_rulebased' => array('adapter' => 'Rules') - )); - } - - public function tearDown() {} - public function testPatternBasedIpMatching() { $request = new Request(array('env' => array('REMOTE_ADDR' => '10.0.1.2'))); + $adapter = new Rules(); - // Multiple rules, they should all pass - $rules = array( - array( - 'rule' => 'allowIp', - 'message' => 'You can not access this from your location.', - 'ip' => '/10\.0\.1\.\d+/' - ) - ); - $result = Access::check('test_rulebased', array(), $request, compact('rules')); + $rules = array(array( + 'rule' => 'allowIp', + 'message' => 'You can not access this from your location.', + 'ip' => '/10\.0\.1\.\d+/' + )); + $result = $adapter->check(array(), $request, compact('rules')); $this->assertEqual(array(), $result); $request = new Request(array('env' => array('REMOTE_ADDR' => '10.0.1.255'))); - $result = Access::check('test_rulebased', array(), $request, compact('rules')); + $result = $adapter->check(array(), $request, compact('rules')); $this->assertEqual(array(), $result); $request = new Request(array('env' => array('REMOTE_ADDR' => '10.0.2.1'))); - $result = Access::check('test_rulebased', array(), $request, compact('rules')); + $result = $adapter->check(array(), $request, compact('rules')); $this->assertEqual('You can not access this from your location.', $result['message']); } public function testArrayBasedIpMatching() { - // Multiple rules, they should all pass - $rules = array( - array( - 'rule' => 'allowIp', - 'message' => 'You can not access this from your location.', - 'ip' => array('10.0.1.2', '10.0.1.3', '10.0.1.4') - ) - ); + $adapter = new Rules(); + $rules = array(array( + 'rule' => 'allowIp', + 'message' => 'You can not access this from your location.', + 'ip' => array('10.0.1.2', '10.0.1.3', '10.0.1.4') + )); foreach (array(2, 3, 4) as $i) { $request = new Request(array('env' => array('REMOTE_ADDR' => "10.0.1.{$i}"))); - $result = Access::check('test_rulebased', array(), $request, compact('rules')); + $result = $adapter->check(array(), $request, compact('rules')); $this->assertEqual(array(), $result); } foreach (array(1, 5, 255) as $i) { $request = new Request(array('env' => array('REMOTE_ADDR' => "10.0.1.{$i}"))); - $result = Access::check('test_rulebased', array(), $request, compact('rules')); + $result = $adapter->check(array(), $request, compact('rules')); $this->assertEqual('You can not access this from your location.', $result['message']); } } public function testCheck() { + $user = array('username' => 'Tom'); $request = new Request(array('env' => array('REMOTE_ADDR' => '10.0.1.1'))); + $adapter = new Rules(); - // Multiple rules, they should all pass $rules = array( array('rule' => 'allowAnyUser', 'message' => 'You must be logged in.'), array('rule' => 'allowAll', 'message' => 'You must be logged in.'), @@ -81,78 +70,110 @@ public function testCheck() { 'ip' => '10.0.1.1' ) ); - $result = Access::check('test_rulebased', array('username' => 'Tom'), $request, array( - 'rules' => $rules - )); + $result = $adapter->check($user, $request, compact('rules')); $this->assertEqual(array(), $result); - // Single rule in multi-demnsional array $rules = array(array('rule' => 'denyAll', 'message' => 'You must be logged in.')); - $expected = array( - 'rule' => 'denyAll', 'message' => 'You must be logged in.', 'redirect' => '/' - ); - $result = Access::check('test_rulebased', array('username' => 'Tom'), $request, array( - 'rules' => $rules - )); + $expected = array('rule' => 'denyAll', 'message' => 'You must be logged in.'); + $result = $adapter->check($user, $request, compact('rules')); $this->assertEqual($expected, $result); - // Single rule (single array), but it should fail because user is an empty array $rules = array('rule' => 'allowAnyUser', 'message' => 'You must be logged in.'); - $expected = array( - 'rule' => 'allowAnyUser', 'message' => 'You must be logged in.', 'redirect' => '/' - ); - $result = Access::check('test_rulebased', array(), $request, array('rules' => $rules)); + $expected = array('rule' => 'allowAnyUser', 'message' => 'You must be logged in.'); + $result = $adapter->check(array(), $request, compact('rules')); $this->assertEqual($expected, $result); - // and if false instead of an empty array (because one might typically run Auth:check() - // which could return false) - $result = Access::check('test_rulebased', false, $request, array('rules' => $rules)); + $result = $adapter->check(false, $request, compact('rules')); $this->assertEqual($expected, $result); + } - // No rules - $expected = array( - 'rule' => false, - 'message' => 'You are not permitted to access this area.', - 'redirect' => '/' - ); - $result = Access::check('test_rulebased', array('username' => 'Tom'), $request); + /** + * Test access checking with no passed or defined rules. + */ + public function testCheckNoRules() { + $user = array('username' => 'Tom'); + $request = new Request(); + $adapter = new Rules(); + + $expected = array('rule' => false, 'message' => null, 'redirect' => null); + $result = $adapter->check($user, $request); $this->assertEqual($expected, $result); + } + + /** + * Tests checking against a list of rules that are passed on the fly. + */ + public function testPassingRules() { + $user = array('username' => 'Tom'); + $request = new Request(); + $adapter = new Rules(); - // Adding a rule "on the fly" by passing a closure, this rule should pass $rules = array( - array( - 'rule' => function($user, $request, $options) { - return $user['username'] == 'Tom'; - }, - 'message' => 'Access denied.' - ) + array('message' => 'Access denied.', 'rule' => function($user, $request, $options) { + return $user['username'] == 'Tom'; + }) ); $expected = array(); - $result = Access::check('test_rulebased', array('username' => 'Tom'), $request, array( - 'rules' => $rules - )); + $result = $adapter->check($user, $request, compact('rules')); $this->assertEqual($expected, $result); } public function testAdd() { $request = new Request(); + $user = array('username' => 'Tom'); + $adapter = new Rules(); - // The add() method to add a rule - Access::adapter('test_rulebased')->add('testDeny', function($user, $request, $options) { + $adapter->add('testDeny', function($user, $request, $options) { return false; }); $rules = array(array('rule' => 'testDeny', 'message' => 'Access denied.')); - $expected = array('rule' => 'testDeny', 'message' => 'Access denied.', 'redirect' => '/'); - $result = Access::check('test_rulebased', array('username' => 'Tom'), $request, array( - 'rules' => $rules - )); + $expected = array('rule' => 'testDeny', 'message' => 'Access denied.'); + $result = $adapter->check($user, $request, compact('rules')); $this->assertEqual($expected, $result); - // Make sure the rule got added to the $_rules property - $this->assertTrue(is_callable(Access::adapter('test_rulebased')->get('testDeny'))); - $this->assertTrue(is_array(Access::adapter('test_rulebased')->get())); + $this->assertTrue(is_callable($adapter->get('testDeny'))); + $this->assertEqual($adapter->get('testDeny'), $adapter->getRules('testDeny')); + + $rules = $adapter->get(); + $this->assertTrue(is_array($rules)); + $this->assertTrue(in_array('testDeny', array_keys($rules))); + } + + /** + * Tests that calls only fail when all rules fail if `'allowAny'` is set. + */ + public function testAllowAnyRule() { + $request = new Request(); + $adapter = new Rules(); + $user = array('username' => 'Tom'); + + $rules = array( + array('rule' => 'allowAll', 'message' => 'Access denied.'), + array('rule' => 'denyAll', 'message' => 'Access denied.') + ); + $result = $adapter->check($user, $request, compact('rules')); + $this->assertEqual(array('rule' => 'denyAll', 'message' => 'Access denied.'), $result); + + $adapter = new Rules(array('allowAny' => true)); + $result = $adapter->check($user, $request, compact('rules')); + $this->assertEqual(array(), $result); + + $result = $adapter->check($user, $request, array('rules' => array('denyAll', 'allowAll'))); + $this->assertEqual(array(), $result); + } + + /** + * Tests that checks against invalid rules return an invalid rule array. + */ + public function testInvalidRule() { + $request = new Request(); + $adapter = new Rules(); + $user = array('username' => 'Tom'); + + $result = $adapter->check($user, $request, array('rules' => array('badness'))); + $this->assertEqual(array('rule' => 'badness'), $result); } } -?> +?> \ No newline at end of file From 6e37c292a77ed8dd628e805d9a26d5b790b5028d Mon Sep 17 00:00:00 2001 From: Nate Abele Date: Fri, 2 Sep 2011 08:57:36 -0400 Subject: [PATCH 04/25] Fixing issue in `Rules` adapter where options passed to `check()` were not passed to individual rules. --- extensions/adapter/security/access/Rules.php | 7 ++++--- .../adapter/security/access/RulesTest.php | 21 +++++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/extensions/adapter/security/access/Rules.php b/extensions/adapter/security/access/Rules.php index debc0a8..a38555c 100644 --- a/extensions/adapter/security/access/Rules.php +++ b/extensions/adapter/security/access/Rules.php @@ -115,7 +115,7 @@ public function check($user, $request, array $options = array()) { if (is_string($rule)) { $rule = compact('rule'); } - $ruleResult = $this->_call($rule, $user, $request); + $ruleResult = $this->_call($rule, $user, $request, $options); switch (true) { case ($ruleResult === false && $options['allowAny']): @@ -137,10 +137,11 @@ public function check($user, $request, array $options = array()) { * @param array $rule The rule definition array. * @param mixed $user The value representing the user making the request. Usually an array. * @param mixed $request The value representing request data or the object being access. + * @param array $options Any options passed to `check()`. * @return boolean Returns `true` if the call to the rule was successful, otherwise `false` if * the call failed, or if a callable rule was not found. */ - protected function _call($rule, $user, $request) { + protected function _call($rule, $user, $request, array $options) { $callable = null; switch (true) { @@ -151,7 +152,7 @@ protected function _call($rule, $user, $request) { $callable = $this->_rules[$rule['rule']]; break; } - return is_callable($callable) ? call_user_func($callable, $user, $request, $rule) : false; + return $callable ? call_user_func($callable, $user, $request, $rule + $options) : false; } /** diff --git a/tests/cases/extensions/adapter/security/access/RulesTest.php b/tests/cases/extensions/adapter/security/access/RulesTest.php index 4556818..d7c6269 100644 --- a/tests/cases/extensions/adapter/security/access/RulesTest.php +++ b/tests/cases/extensions/adapter/security/access/RulesTest.php @@ -174,6 +174,27 @@ public function testInvalidRule() { $result = $adapter->check($user, $request, array('rules' => array('badness'))); $this->assertEqual(array('rule' => 'badness'), $result); } + + /** + * Tests that options passed to `Rules::check()` are passed to each rule doing the checking. + */ + public function testOptionsPassedToRule() { + $request = new Request(); + $user = array('username' => 'Tom'); + $adapter = new Rules(array( + 'rules' => array( + 'foobar' => function($user, $request, $options) { + return $options['foo'] == 'bar'; + } + ), + 'default' => array('foobar') + )); + + $result = $adapter->check($user, $request, array('foo' => 'baz')); + $this->assertEqual(array('rule' => 'foobar', 'foo' => 'baz'), $result); + $result = $adapter->check($user, $request, array('foo' => 'bar')); + $this->assertEqual(array(), $result); + } } ?> \ No newline at end of file From f7595e9d17999f879c633fee615b18eb367b04fb Mon Sep 17 00:00:00 2001 From: Nate Abele Date: Fri, 2 Sep 2011 09:31:56 -0400 Subject: [PATCH 05/25] Implementing `'user'` configuration option in `Rules` adapter to provide support for automatic access to user session information. --- extensions/adapter/security/access/Rules.php | 8 +++++- .../adapter/security/access/RulesTest.php | 27 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/extensions/adapter/security/access/Rules.php b/extensions/adapter/security/access/Rules.php index a38555c..bbe063e 100644 --- a/extensions/adapter/security/access/Rules.php +++ b/extensions/adapter/security/access/Rules.php @@ -45,7 +45,12 @@ class Rules extends \lithium\core\Object { * the check to succeed. Defaults to `false`. */ public function __construct(array $config = array()) { - $defaults = array('rules' => array(), 'default' => array(), 'allowAny' => false); + $defaults = array( + 'rules' => array(), + 'default' => array(), + 'allowAny' => false, + 'user' => function() {} + ); parent::__construct($config + $defaults); } @@ -102,6 +107,7 @@ public function check($user, $request, array $options = array()) { 'allowAny' => $this->_config['allowAny'] ); $options += $defaults; + $user = $user ?: $this->_config['user'](); if (!$options['rules']) { $base = array('rule' => false, 'message' => null, 'redirect' => null); diff --git a/tests/cases/extensions/adapter/security/access/RulesTest.php b/tests/cases/extensions/adapter/security/access/RulesTest.php index d7c6269..82ab2c2 100644 --- a/tests/cases/extensions/adapter/security/access/RulesTest.php +++ b/tests/cases/extensions/adapter/security/access/RulesTest.php @@ -195,6 +195,33 @@ public function testOptionsPassedToRule() { $result = $adapter->check($user, $request, array('foo' => 'bar')); $this->assertEqual(array(), $result); } + + /** + * Tests that user information is automatically retrieved via the closure in the `'user'` + * config. + */ + public function testAutoUser() { + $request = new Request(); + $user = array('username' => 'Tom'); + $adapter = new Rules(array( + 'rules' => array( + 'user' => function($user, $request, $options) { + return isset($user['username']) && $user['username'] == 'Tom'; + } + ), + 'default' => array('user'), + 'user' => function() use ($user) { return $user; } + )); + + $result = $adapter->check($user, $request); + $this->assertEqual(array(), $result); + + $result = $adapter->check(null, $request); + $this->assertEqual(array(), $result); + + $result = $adapter->check(array('username' => 'Bob'), $request); + $this->assertEqual(array('rule' => 'user'), $result); + } } ?> \ No newline at end of file From 1ccb07c5a515f91718c0b5216a74ecdd08d87239 Mon Sep 17 00:00:00 2001 From: Marc Schwering Date: Fri, 4 Nov 2011 15:02:53 +0100 Subject: [PATCH 06/25] fixing lithium_qa issues (cherry-picking from commit e80bfbc39c26335b06aaba639936e19a4560f602) --- .../adapter/security/access/AuthRbac.php | 63 ++++++++++++++----- 1 file changed, 46 insertions(+), 17 deletions(-) diff --git a/extensions/adapter/security/access/AuthRbac.php b/extensions/adapter/security/access/AuthRbac.php index 2c065cb..fcaa929 100644 --- a/extensions/adapter/security/access/AuthRbac.php +++ b/extensions/adapter/security/access/AuthRbac.php @@ -4,8 +4,7 @@ use lithium\security\Auth; use lithium\core\ConfigException; - -use li3_access\security\Access; +use lithium\util\Inflector; class AuthRbac extends \lithium\core\Object { @@ -23,13 +22,14 @@ class AuthRbac extends \lithium\core\Object { /** * The `Rbac` adapter will iterate trough the rbac data Array. * - * @param mixed $user The user data array that holds all necessary information about + * @param mixed $requester The user data array that holds all necessary information about * the user requesting access. Or false (because Auth::check() can return false). * This is an optional parameter, bercause we will fetch the users data trough Auth * seperately. * @param object $request The Lithium Request object. * @param array $options An array of additional options for the _getRolesByAuth method. - * @return Array An empty array if access is allowed and an array with reasons for denial if denied. + * @return Array An empty array if access is allowed or + * an array with reasons for denial if denied. */ public function check($requester, $request, array $options = array()) { if (empty($this->_roles)) { @@ -55,14 +55,22 @@ public function check($requester, $request, array $options = array()) { if (!static::parseMatch($role['match'], $request)) { continue; } + + $accessable = static::_is_accessable($role, $request, $options); + + /* $accessable = true; if (($role['allow'] === false) || (!static::_hasRole($role['requesters'], $request, $options)) || - (is_array($role['allow']) && !static::_parseClosures($role['allow'], $request, $role)) - ) { + ( + is_array($role['allow']) && + !static::_parseClosures($role['allow'], $request, $role) + ) + ){ $accessable = false; } + */ if (!$accessable) { $message = !empty($role['message']) ? $role['message'] : $message; @@ -73,6 +81,23 @@ public function check($requester, $request, array $options = array()) { return !$accessable ? compact('message', 'redirect') : array(); } + /** + * Checks if the Role grants access + * + * @param array $role Array Set of Roles + * @param mixed $request A lithium Request object. + * @param array $options An array of additional options for the _getRolesByAuth method. + * @return boolean $accessable + */ + protected static function _is_accessable($role, $request, $options){ + if (is_array($role['allow'])) { + return static::_parseClosures($role['allow'], $request, $role); + } else if ($role['allow'] === false) { + return false; + } + return static::_hasRole($role['requesters'], $request, $options); + } + /** * parseMatch Matches the current request parameters against a set of given parameters. * Can match against a shorthand string (Controller::action) or a full array. If a parameter @@ -114,30 +139,34 @@ public static function parseMatch($match, $request) { } if ($type === 'controller') { - $value = \lithium\util\Inflector::underscore($value); + $value = Inflector::underscore($value); } - if (!array_key_exists($type, $request->params) || $value !== $request->params[$type]) { + $exists_in_request = array_key_exists($type, $request->params); + if (!$exists_in_request || $value !== Inflector::underscore($request->params[$type])) { return false; } } - return true; } /** - * _parseClosures Itterates over an array and runs any anonymous functions it + * _parseClosures Iterates over an array and runs any anonymous functions it * finds. Returns true if all of the closures it runs evaluate to true. $match * is passed by refference and any closures found are removed from it before the * method is complete. * - * @param array $data - * @param mixed $request - * @static + * @static * @access protected - * @return void + * + * @param array $data dereferenced Array + * @param mixed $request + * @param array $roleOptions dereferenced Array + * @return boolean */ - protected static function _parseClosures(array &$data = array(), $request = null, array &$roleOptions = array()) { + protected static function _parseClosures( + array &$data = array(), $request = null, array &$roleOptions = array() + ) { $return = true; foreach ($data as $key => $item) { if (is_callable($item)) { @@ -154,6 +183,7 @@ protected static function _parseClosures(array &$data = array(), $request = null * @todo reduce Model Overhead (will duplicated in each model) * * @param Request $request Object + * @param array $options * @return array|mixed $roles Roles with attachted User Models */ protected static function _getRolesByAuth($request, array $options = array()){ @@ -190,7 +220,6 @@ protected function _hasRole($requesters, $request, array $options = array()) { } return false; } - } -?> +?> \ No newline at end of file From 39d3ef77da900e874fbbf7f2e7de95ad57199c9f Mon Sep 17 00:00:00 2001 From: Marc Schwering Date: Fri, 4 Nov 2011 15:04:56 +0100 Subject: [PATCH 07/25] fixing a lot of qa issues --- extensions/adapter/security/access/Rules.php | 1 + extensions/adapter/security/access/Simple.php | 4 +++- tests/cases/security/AccessTest.php | 2 +- .../mocks/extensions/adapter/auth/MockAuthAdapter.php | 10 +++++++--- 4 files changed, 12 insertions(+), 5 deletions(-) diff --git a/extensions/adapter/security/access/Rules.php b/extensions/adapter/security/access/Rules.php index bbe063e..f6de1d8 100644 --- a/extensions/adapter/security/access/Rules.php +++ b/extensions/adapter/security/access/Rules.php @@ -191,6 +191,7 @@ public function get($name = null) { /** * @deprecated + * @param string $name The rule name (optional). */ public function getRules($name = null) { return $this->get($name); diff --git a/extensions/adapter/security/access/Simple.php b/extensions/adapter/security/access/Simple.php index 62f1d71..2b315c9 100644 --- a/extensions/adapter/security/access/Simple.php +++ b/extensions/adapter/security/access/Simple.php @@ -2,8 +2,10 @@ namespace li3_access\extensions\adapter\security\access; +/* uncomment it if needed use lithium\core\Libraries; use lithium\util\Set; +*/ class Simple extends \lithium\core\Object { @@ -23,4 +25,4 @@ public function check($user, $request, array $options = array()) { } } -?> +?> \ No newline at end of file diff --git a/tests/cases/security/AccessTest.php b/tests/cases/security/AccessTest.php index c55cf70..4734118 100644 --- a/tests/cases/security/AccessTest.php +++ b/tests/cases/security/AccessTest.php @@ -72,4 +72,4 @@ public function testNoConfigurations() { } } -?> +?> \ No newline at end of file diff --git a/tests/mocks/extensions/adapter/auth/MockAuthAdapter.php b/tests/mocks/extensions/adapter/auth/MockAuthAdapter.php index d946f3e..85f420f 100644 --- a/tests/mocks/extensions/adapter/auth/MockAuthAdapter.php +++ b/tests/mocks/extensions/adapter/auth/MockAuthAdapter.php @@ -5,7 +5,11 @@ class MockAuthAdapter extends \lithium\core\Object { public function check($credentials, array $options = array()) { - return isset($options['success']) && !empty($credentials->data) ? $credentials->data : false; + $granted = false; + if (isset($options['success']) && !empty($credentials->data)) { + $granted = $credentials->data; + } + return $granted; } public function set($data, array $options = array()) { @@ -16,8 +20,8 @@ public function set($data, array $options = array()) { } public function clear(array $options = array()) { - } + } } -?> +?> \ No newline at end of file From 7a2b51f6fc2c4d78a6ca407073da54886a259fd4 Mon Sep 17 00:00:00 2001 From: Marc Schwering Date: Fri, 4 Nov 2011 15:20:30 +0100 Subject: [PATCH 08/25] added a todo hint --- extensions/adapter/security/access/AuthRbac.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/extensions/adapter/security/access/AuthRbac.php b/extensions/adapter/security/access/AuthRbac.php index fcaa929..e15d0c9 100644 --- a/extensions/adapter/security/access/AuthRbac.php +++ b/extensions/adapter/security/access/AuthRbac.php @@ -22,6 +22,8 @@ class AuthRbac extends \lithium\core\Object { /** * The `Rbac` adapter will iterate trough the rbac data Array. * + * @todo: write better tests! + * * @param mixed $requester The user data array that holds all necessary information about * the user requesting access. Or false (because Auth::check() can return false). * This is an optional parameter, bercause we will fetch the users data trough Auth @@ -58,7 +60,7 @@ public function check($requester, $request, array $options = array()) { $accessable = static::_is_accessable($role, $request, $options); - /* + /* old Code: $accessable = true; if (($role['allow'] === false) || From 2466485ced24605ca31a03a1a4f7d1af65c8038d Mon Sep 17 00:00:00 2001 From: Marc Schwering Date: Tue, 8 Nov 2011 12:35:01 +0100 Subject: [PATCH 09/25] renaming Method --- .../adapter/security/access/AuthRbac.php | 20 +++---------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/extensions/adapter/security/access/AuthRbac.php b/extensions/adapter/security/access/AuthRbac.php index e15d0c9..392da45 100644 --- a/extensions/adapter/security/access/AuthRbac.php +++ b/extensions/adapter/security/access/AuthRbac.php @@ -58,23 +58,9 @@ public function check($requester, $request, array $options = array()) { continue; } - $accessable = static::_is_accessable($role, $request, $options); - - /* old Code: - $accessable = true; - - if (($role['allow'] === false) || - (!static::_hasRole($role['requesters'], $request, $options)) || - ( - is_array($role['allow']) && - !static::_parseClosures($role['allow'], $request, $role) - ) - ){ - $accessable = false; - } - */ + $accessable = static::_isAccessible($role, $request, $options); - if (!$accessable) { + if (!$accessable) { $message = !empty($role['message']) ? $role['message'] : $message; $redirect = !empty($role['redirect']) ? $role['redirect'] : $redirect; } @@ -91,7 +77,7 @@ public function check($requester, $request, array $options = array()) { * @param array $options An array of additional options for the _getRolesByAuth method. * @return boolean $accessable */ - protected static function _is_accessable($role, $request, $options){ + protected static function _isAccessible($role, $request, $options){ if (is_array($role['allow'])) { return static::_parseClosures($role['allow'], $request, $role); } else if ($role['allow'] === false) { From aecf42550d6cc3b7359939fa891f5ef5de5b8929 Mon Sep 17 00:00:00 2001 From: Marc Schwering Date: Tue, 8 Nov 2011 12:35:59 +0100 Subject: [PATCH 10/25] renamed var accessable to accessible --- extensions/adapter/security/access/AuthRbac.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/extensions/adapter/security/access/AuthRbac.php b/extensions/adapter/security/access/AuthRbac.php index 392da45..eb28d0e 100644 --- a/extensions/adapter/security/access/AuthRbac.php +++ b/extensions/adapter/security/access/AuthRbac.php @@ -49,7 +49,7 @@ public function check($requester, $request, array $options = array()) { $message = $options['message']; $redirect = $options['redirect']; - $accessable = false; + $accessible = false; foreach ($this->_roles as $role) { $role += $roleDefaults; @@ -58,15 +58,15 @@ public function check($requester, $request, array $options = array()) { continue; } - $accessable = static::_isAccessible($role, $request, $options); + $accessible = static::_isAccessible($role, $request, $options); - if (!$accessable) { + if (!$accessible) { $message = !empty($role['message']) ? $role['message'] : $message; $redirect = !empty($role['redirect']) ? $role['redirect'] : $redirect; } } - return !$accessable ? compact('message', 'redirect') : array(); + return !$accessible ? compact('message', 'redirect') : array(); } /** From e42de4251f0cf315ea45e8a77ef981fe35b2640e Mon Sep 17 00:00:00 2001 From: Marc Schwering Date: Tue, 8 Nov 2011 13:15:21 +0100 Subject: [PATCH 11/25] fixed format errors: spacers => tabs --- .../adapter/security/access/AuthRbac.php | 50 +++++++++---------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/extensions/adapter/security/access/AuthRbac.php b/extensions/adapter/security/access/AuthRbac.php index eb28d0e..06dc58f 100644 --- a/extensions/adapter/security/access/AuthRbac.php +++ b/extensions/adapter/security/access/AuthRbac.php @@ -34,40 +34,40 @@ class AuthRbac extends \lithium\core\Object { * an array with reasons for denial if denied. */ public function check($requester, $request, array $options = array()) { - if (empty($this->_roles)) { - throw new ConfigException('No roles defined for adapter configuration.'); - } + if (empty($this->_roles)) { + throw new ConfigException('No roles defined for adapter configuration.'); + } - $roleDefaults = array( - 'message' => '', - 'redirect' => '', - 'allow' => true, - 'requesters' => '*', - 'match' => '*::*' - ); + $roleDefaults = array( + 'message' => '', + 'redirect' => '', + 'allow' => true, + 'requesters' => '*', + 'match' => '*::*' + ); - $message = $options['message']; - $redirect = $options['redirect']; + $message = $options['message']; + $redirect = $options['redirect']; - $accessible = false; - foreach ($this->_roles as $role) { - $role += $roleDefaults; + $accessible = false; + foreach ($this->_roles as $role) { + $role += $roleDefaults; - // Check to see if this role applies to this request - if (!static::parseMatch($role['match'], $request)) { - continue; - } + // Check to see if this role applies to this request + if (!static::parseMatch($role['match'], $request)) { + continue; + } $accessible = static::_isAccessible($role, $request, $options); if (!$accessible) { - $message = !empty($role['message']) ? $role['message'] : $message; - $redirect = !empty($role['redirect']) ? $role['redirect'] : $redirect; - } - } + $message = !empty($role['message']) ? $role['message'] : $message; + $redirect = !empty($role['redirect']) ? $role['redirect'] : $redirect; + } + } - return !$accessible ? compact('message', 'redirect') : array(); - } + return!$accessible ? compact('message', 'redirect') : array(); + } /** * Checks if the Role grants access From ed554012e09319ab938e1de0bb006b0c02c72cf0 Mon Sep 17 00:00:00 2001 From: Marc Schwering Date: Tue, 8 Nov 2011 13:58:21 +0100 Subject: [PATCH 12/25] removed spacers --- .../adapter/security/access/AuthRbac.php | 219 +++++++++--------- 1 file changed, 110 insertions(+), 109 deletions(-) diff --git a/extensions/adapter/security/access/AuthRbac.php b/extensions/adapter/security/access/AuthRbac.php index 06dc58f..414fd36 100644 --- a/extensions/adapter/security/access/AuthRbac.php +++ b/extensions/adapter/security/access/AuthRbac.php @@ -8,10 +8,10 @@ class AuthRbac extends \lithium\core\Object { - /** - * @var array $_autoConfig - * @see lithium\core\Object::$_autoConfig - */ + /** + * @var array $_autoConfig + * @see lithium\core\Object::$_autoConfig + */ protected $_autoConfig = array('roles'); /** @@ -33,7 +33,7 @@ class AuthRbac extends \lithium\core\Object { * @return Array An empty array if access is allowed or * an array with reasons for denial if denied. */ - public function check($requester, $request, array $options = array()) { + public function check($requester, $request, array $options = array()) { if (empty($this->_roles)) { throw new ConfigException('No roles defined for adapter configuration.'); } @@ -77,7 +77,7 @@ public function check($requester, $request, array $options = array()) { * @param array $options An array of additional options for the _getRolesByAuth method. * @return boolean $accessable */ - protected static function _isAccessible($role, $request, $options){ + protected static function _isAccessible($role, $request, $options) { if (is_array($role['allow'])) { return static::_parseClosures($role['allow'], $request, $role); } else if ($role['allow'] === false) { @@ -86,86 +86,86 @@ protected static function _isAccessible($role, $request, $options){ return static::_hasRole($role['requesters'], $request, $options); } - /** - * parseMatch Matches the current request parameters against a set of given parameters. - * Can match against a shorthand string (Controller::action) or a full array. If a parameter - * is provided then it must have an equivilent in the Request objects parmeters in order - * to validate. * Is also acceptable to match a parameter without a specific value. - * - * @param mixed $match A set of parameters to validate the request against. - * @param mixed $request A lithium Request object. - * @access public - * @return boolean True if a match is found. - */ - public static function parseMatch($match, $request) { - if (empty($match)) { - return false; - } - - if (is_array($match)) { - if (!static::_parseClosures($match, $request)) { - return false; - } - } - - $params = array(); - foreach ((array) $match as $key => $param) { - if (is_string($param)) { - if (preg_match('/^[A-Za-z0-9_\*]+::[A-Za-z0-9_\*]+$/', $param, $regexMatches)) { - list($controller, $action) = explode('::', reset($regexMatches)); - $params += compact('controller', 'action'); - continue; - } - } - - $params[$key] = $param; - } - - foreach ($params as $type => $value) { - if ($value === '*') { - continue; - } - - if ($type === 'controller') { - $value = Inflector::underscore($value); - } + /** + * parseMatch Matches the current request parameters against a set of given parameters. + * Can match against a shorthand string (Controller::action) or a full array. If a parameter + * is provided then it must have an equivilent in the Request objects parmeters in order + * to validate. * Is also acceptable to match a parameter without a specific value. + * + * @param mixed $match A set of parameters to validate the request against. + * @param mixed $request A lithium Request object. + * @access public + * @return boolean True if a match is found. + */ + public static function parseMatch($match, $request) { + if (empty($match)) { + return false; + } + + if (is_array($match)) { + if (!static::_parseClosures($match, $request)) { + return false; + } + } + + $params = array(); + foreach ((array) $match as $key => $param) { + if (is_string($param)) { + if (preg_match('/^[A-Za-z0-9_\*]+::[A-Za-z0-9_\*]+$/', $param, $regexMatches)) { + list($controller, $action) = explode('::', reset($regexMatches)); + $params += compact('controller', 'action'); + continue; + } + } + + $params[$key] = $param; + } + + foreach ($params as $type => $value) { + if ($value === '*') { + continue; + } + + if ($type === 'controller') { + $value = Inflector::underscore($value); + } $exists_in_request = array_key_exists($type, $request->params); - if (!$exists_in_request || $value !== Inflector::underscore($request->params[$type])) { - return false; - } - } - return true; - } - - /** - * _parseClosures Iterates over an array and runs any anonymous functions it - * finds. Returns true if all of the closures it runs evaluate to true. $match - * is passed by refference and any closures found are removed from it before the - * method is complete. - * + if (!$exists_in_request || $value !== Inflector::underscore($request->params[$type])) { + return false; + } + } + return true; + } + + /** + * _parseClosures Iterates over an array and runs any anonymous functions it + * finds. Returns true if all of the closures it runs evaluate to true. $match + * is passed by refference and any closures found are removed from it before the + * method is complete. + * * @static - * @access protected + * @access protected * - * @param array $data dereferenced Array - * @param mixed $request + * @param array $data dereferenced Array + * @param mixed $request * @param array $roleOptions dereferenced Array - * @return boolean - */ - protected static function _parseClosures( - array &$data = array(), $request = null, array &$roleOptions = array() + * @return boolean + */ + protected static function _parseClosures( + array &$data = array(), $request = null, array &$roleOptions = array() ) { - $return = true; - foreach ($data as $key => $item) { - if (is_callable($item)) { - if ($return === true) { - $return = (boolean) $item($request, $roleOptions); - } - unset($data[$key]); - } - } - return $return; - } + $return = true; + foreach ($data as $key => $item) { + if (is_callable($item)) { + if ($return === true) { + $return = (boolean) $item($request, $roleOptions); + } + unset($data[$key]); + } + } + return $return; + } /** * @todo reduce Model Overhead (will duplicated in each model) @@ -174,40 +174,41 @@ protected static function _parseClosures( * @param array $options * @return array|mixed $roles Roles with attachted User Models */ - protected static function _getRolesByAuth($request, array $options = array()){ + protected static function _getRolesByAuth($request, array $options = array()) { $roles = array('*' => '*'); - foreach (array_keys(Auth::config()) as $key){ - if ($check = Auth::check($key, $request, $options)) { - $roles[$key] = $check; - } + foreach (array_keys(Auth::config()) as $key) { + if ($check = Auth::check($key, $request, $options)) { + $roles[$key] = $check; + } } return $roles = array_filter($roles); } - /** - * _hasRole Compares the results from _getRolesByAuth with the array passed to it. - * - * @param mixed $requesters - * @param mixed $request - * @param array $options - * @access protected - * @return void - */ - protected function _hasRole($requesters, $request, array $options = array()) { - $authed = array_keys(static::_getRolesByAuth($request, $options)); - - $requesters = (array) $requesters; - if (in_array('*', $requesters)) { - return true; - } - - foreach ($requesters as $requester) { - if (in_array($requester, $authed)) { - return true; - } - } - return false; - } + /** + * _hasRole Compares the results from _getRolesByAuth with the array passed to it. + * + * @param mixed $requesters + * @param mixed $request + * @param array $options + * @access protected + * @return void + */ + protected function _hasRole($requesters, $request, array $options = array()) { + $authed = array_keys(static::_getRolesByAuth($request, $options)); + + $requesters = (array) $requesters; + if (in_array('*', $requesters)) { + return true; + } + + foreach ($requesters as $requester) { + if (in_array($requester, $authed)) { + return true; + } + } + return false; + } + } ?> \ No newline at end of file From 8ce9f8529c5b4ee615835352be879f0baf2fb297 Mon Sep 17 00:00:00 2001 From: Marc Schwering Date: Tue, 8 Nov 2011 14:03:06 +0100 Subject: [PATCH 13/25] removed new line bfore curly braces --- extensions/adapter/security/access/AuthRbac.php | 1 - 1 file changed, 1 deletion(-) diff --git a/extensions/adapter/security/access/AuthRbac.php b/extensions/adapter/security/access/AuthRbac.php index 414fd36..8ccd0e0 100644 --- a/extensions/adapter/security/access/AuthRbac.php +++ b/extensions/adapter/security/access/AuthRbac.php @@ -208,7 +208,6 @@ protected function _hasRole($requesters, $request, array $options = array()) { } return false; } - } ?> \ No newline at end of file From af7c9883ed32ae3bf4314dcf9b27562a518d6f20 Mon Sep 17 00:00:00 2001 From: Marc Schwering Date: Tue, 8 Nov 2011 14:26:30 +0100 Subject: [PATCH 14/25] fixing non static to static function --- extensions/adapter/security/access/AuthRbac.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/adapter/security/access/AuthRbac.php b/extensions/adapter/security/access/AuthRbac.php index 8ccd0e0..af7f2e6 100644 --- a/extensions/adapter/security/access/AuthRbac.php +++ b/extensions/adapter/security/access/AuthRbac.php @@ -193,7 +193,7 @@ protected static function _getRolesByAuth($request, array $options = array()) { * @access protected * @return void */ - protected function _hasRole($requesters, $request, array $options = array()) { + protected static function _hasRole($requesters, $request, array $options = array()) { $authed = array_keys(static::_getRolesByAuth($request, $options)); $requesters = (array) $requesters; From eba675623d9d1129545130031297bd88dc26ddfe Mon Sep 17 00:00:00 2001 From: Marc Schwering Date: Tue, 8 Nov 2011 15:38:34 +0100 Subject: [PATCH 15/25] fixing dereference bug --- extensions/adapter/security/access/AuthRbac.php | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/extensions/adapter/security/access/AuthRbac.php b/extensions/adapter/security/access/AuthRbac.php index af7f2e6..01decca 100644 --- a/extensions/adapter/security/access/AuthRbac.php +++ b/extensions/adapter/security/access/AuthRbac.php @@ -66,21 +66,22 @@ public function check($requester, $request, array $options = array()) { } } - return!$accessible ? compact('message', 'redirect') : array(); + return !$accessible ? compact('message', 'redirect') : array(); } /** * Checks if the Role grants access * - * @param array $role Array Set of Roles + * @param array $role Array Set of Roles (dereferenced) * @param mixed $request A lithium Request object. * @param array $options An array of additional options for the _getRolesByAuth method. * @return boolean $accessable */ - protected static function _isAccessible($role, $request, $options) { + protected static function _isAccessible(&$role, $request, $options) { if (is_array($role['allow'])) { return static::_parseClosures($role['allow'], $request, $role); - } else if ($role['allow'] === false) { + } + if ($role['allow'] === false) { return false; } return static::_hasRole($role['requesters'], $request, $options); @@ -152,9 +153,7 @@ public static function parseMatch($match, $request) { * @param array $roleOptions dereferenced Array * @return boolean */ - protected static function _parseClosures( - array &$data = array(), $request = null, array &$roleOptions = array() - ) { + protected static function _parseClosures(array &$data = array(), $request = null, array &$roleOptions = array()) { $return = true; foreach ($data as $key => $item) { if (is_callable($item)) { From 546613e430bfc21a84d8fe3eddf0f8684d1e099e Mon Sep 17 00:00:00 2001 From: Marc Schwering Date: Tue, 8 Nov 2011 15:59:45 +0100 Subject: [PATCH 16/25] rearranged isAccessible as intended removed obsolete method headers to succeed li3_qa and li3 test --- extensions/adapter/security/access/AuthRbac.php | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/extensions/adapter/security/access/AuthRbac.php b/extensions/adapter/security/access/AuthRbac.php index 01decca..6bc1c5d 100644 --- a/extensions/adapter/security/access/AuthRbac.php +++ b/extensions/adapter/security/access/AuthRbac.php @@ -71,6 +71,10 @@ public function check($requester, $request, array $options = array()) { /** * Checks if the Role grants access + * If allow === false => no access + * If requesters has no role => no access + * If allows contains closures => return closures return + * Otherwise => grants access * * @param array $role Array Set of Roles (dereferenced) * @param mixed $request A lithium Request object. @@ -78,13 +82,16 @@ public function check($requester, $request, array $options = array()) { * @return boolean $accessable */ protected static function _isAccessible(&$role, $request, $options) { - if (is_array($role['allow'])) { - return static::_parseClosures($role['allow'], $request, $role); - } if ($role['allow'] === false) { return false; } - return static::_hasRole($role['requesters'], $request, $options); + if (!static::_hasRole($role['requesters'], $request, $options)) { + return false; + } + if (is_array($role['allow'])) { + return static::_parseClosures($role['allow'], $request, $role); + } + return true; } /** @@ -153,7 +160,7 @@ public static function parseMatch($match, $request) { * @param array $roleOptions dereferenced Array * @return boolean */ - protected static function _parseClosures(array &$data = array(), $request = null, array &$roleOptions = array()) { + protected static function _parseClosures(array &$data, $request, array &$roleOptions = array()){ $return = true; foreach ($data as $key => $item) { if (is_callable($item)) { From 84ecc1c272b2f3c251cb01b06fbfca216d6fde5e Mon Sep 17 00:00:00 2001 From: joedevon Date: Fri, 9 Dec 2011 14:58:43 -0800 Subject: [PATCH 17/25] I fixed a typo. Unless it really is called sSimple Adapter. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 7ffb098..6db9362 100644 --- a/README.md +++ b/README.md @@ -26,7 +26,7 @@ If the request validates correctly based on your configuration then `Access::che In this repository there are three adapters. All three work in a slightly different way. -### sSimple Adapter +### Simple Adapter The simple adapter is exactly what it says it is. The check method only checks that the data passed to is not empty and as a result the configuration is trivial. From 35db29b38de4335ada314e924b0fb4ca5d7fa81f Mon Sep 17 00:00:00 2001 From: joedevon Date: Fri, 9 Dec 2011 15:22:12 -0800 Subject: [PATCH 18/25] Fixed more typos. There is one section where I don't know what you meant to say. Specifically: "You may use a parameters array where you explicitly set the parameter/value pairs, a shorthand syntax very similar to the one you use when generating urls or even a." a what? --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 6db9362..b2dd08d 100644 --- a/README.md +++ b/README.md @@ -20,7 +20,7 @@ You must configure the adapter you wish to use first, but once you have it confi $this->redirect($access['redirect']); } -If the request validates correctly based on your configuration then `Access::check()` will return an empty `array()` otherwise it will return and array with two keys; `message` and `redirect`. These values are built into the Access class but you can override them but passing them as `$options` to all three of the adapters in this repository. +If the request validates correctly based on your configuration then `Access::check()` will return an empty `array()` otherwise it will return an array with two keys; `message` and `redirect`. These values are built into the Access class but you can override them by passing them as `$options` to all three of the adapters in this repository. ## Configuration @@ -51,7 +51,7 @@ Then to deny all requests from the authenticated user. $this->redirect($access['redirect']); } -There are four built in rules; allowAll, denyAll, allowAnyUser and allowIp, for more information see the adapter itself. However, this adapter is at it's most useful when you add your own rules. +There are four built in rules; allowAll, denyAll, allowAnyUser and allowIp, for more information see the adapter itself. However, this adapter is at its most useful when you add your own rules. Access::adapter('custom_rule')->add(function($user, $request, $options) { // Your logic here. Just make sure it returns an array. @@ -67,7 +67,7 @@ One more to go! This is the most complex adapter in this repository at this time. It's used for Role Based Access Control. You define a set of roles (or conditions) to match the request against, if the request matches your conditions the adapter then checks to see if the user is authenticated with the appropriate `\lithium\security\Auth` configurations to be granted access. -It's difficult to explain (I hope that's clear enough) so lets look at an example configuration to try and achive some clarity: +It's difficult to explain (I hope that's clear enough) so lets look at an example configuration to try and achieve some clarity: Access::config( 'auth_rbac' => array( From ce15b7f290c0cd068ca89e5f461639e40027841e Mon Sep 17 00:00:00 2001 From: Mariano Iglesias Date: Tue, 10 Jan 2012 09:19:02 -0300 Subject: [PATCH 19/25] Fixing issue where namespace were not taken into account in AuthRbac adapter. Fixes #14 --- extensions/adapter/security/access/AuthRbac.php | 10 ++++++---- .../adapter/security/access/AuthRbacTest.php | 11 ++++++++++- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/extensions/adapter/security/access/AuthRbac.php b/extensions/adapter/security/access/AuthRbac.php index 6bc1c5d..b1d8a1b 100644 --- a/extensions/adapter/security/access/AuthRbac.php +++ b/extensions/adapter/security/access/AuthRbac.php @@ -119,9 +119,11 @@ public static function parseMatch($match, $request) { $params = array(); foreach ((array) $match as $key => $param) { if (is_string($param)) { - if (preg_match('/^[A-Za-z0-9_\*]+::[A-Za-z0-9_\*]+$/', $param, $regexMatches)) { - list($controller, $action) = explode('::', reset($regexMatches)); - $params += compact('controller', 'action'); + if (preg_match('/^([A-Za-z0-9_\*\\\]+)::([A-Za-z0-9_\*]+)$/', $param, $regexMatches)) { + $params += array( + 'controller' => $regexMatches[1], + 'action' => $regexMatches[2] + ); continue; } } @@ -216,4 +218,4 @@ protected static function _hasRole($requesters, $request, array $options = array } } -?> \ No newline at end of file +?> diff --git a/tests/cases/extensions/adapter/security/access/AuthRbacTest.php b/tests/cases/extensions/adapter/security/access/AuthRbacTest.php index 5c0ac47..904f3aa 100644 --- a/tests/cases/extensions/adapter/security/access/AuthRbacTest.php +++ b/tests/cases/extensions/adapter/security/access/AuthRbacTest.php @@ -226,6 +226,15 @@ public function testParseMatch() { $test = function() { return false; }; $this->assertFalse(Access::adapter('test_closures')->parseMatch(array($test), $request)); $this->assertFalse(Access::adapter('test_closures')->parseMatch(array(), $request)); + + $request = new Request(array('params' => array( + 'controller' => 'lithium\test\Controller', + 'action' => 'index' + ))); + $match = 'Controller::*'; + $this->assertFalse(Access::adapter('test_check')->parseMatch($match, $request)); + $match = 'lithium\test\Controller::*'; + $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); } public function testClosures() { @@ -271,4 +280,4 @@ public function testNoRolesConfigured() { } } -?> \ No newline at end of file +?> From e5144aefaab225912df182a1f4936ba47fbd34b1 Mon Sep 17 00:00:00 2001 From: Mariano Iglesias Date: Wed, 11 Jan 2012 12:00:58 -0300 Subject: [PATCH 20/25] Fixes #16: allowing allow to be a callable in AuthRbac --- .../adapter/security/access/AuthRbac.php | 3 +++ .../adapter/security/access/AuthRbacTest.php | 23 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/extensions/adapter/security/access/AuthRbac.php b/extensions/adapter/security/access/AuthRbac.php index b1d8a1b..ba11b3a 100644 --- a/extensions/adapter/security/access/AuthRbac.php +++ b/extensions/adapter/security/access/AuthRbac.php @@ -52,6 +52,9 @@ public function check($requester, $request, array $options = array()) { $accessible = false; foreach ($this->_roles as $role) { $role += $roleDefaults; + if (is_callable($role['allow'])) { + $role['allow'] = (array) $role['allow']; + } // Check to see if this role applies to this request if (!static::parseMatch($role['match'], $request)) { diff --git a/tests/cases/extensions/adapter/security/access/AuthRbacTest.php b/tests/cases/extensions/adapter/security/access/AuthRbacTest.php index 904f3aa..1fa7389 100644 --- a/tests/cases/extensions/adapter/security/access/AuthRbacTest.php +++ b/tests/cases/extensions/adapter/security/access/AuthRbacTest.php @@ -53,6 +53,19 @@ function($request) { ) ) ), + 'test_allow_closure' => array( + 'adapter' => 'AuthRbac', + 'roles' => array( + array( + 'requesters' => '*', + 'match' => '*::*', + 'allow' => function($request, &$roleOptions) { + $roleOptions['message'] = 'Test allow options set.'; + return $request->params['allow'] ? true : false; + } + ) + ) + ), 'test_message_override' => array( 'adapter' => 'AuthRbac', 'roles' => array( @@ -266,6 +279,16 @@ public function testClosures() { 'redirect' => '/' ); $this->assertIdentical($expected, $result); + + $request->params['allow'] = true; + $result = Access::check('test_allow_closure', $user, $request, $authSuccess); + $expected = array(); + $this->assertIdentical($expected, $result); + + $request->params['allow'] = false; + $result = Access::check('test_allow_closure', $user, $request, $authSuccess); + $expected = array('message' => 'Test allow options set.', 'redirect' => '/'); + $this->assertIdentical($expected, $result); } public function testNoRolesConfigured() { From 19ebec3d275654085fb8c2e2936231ede289fae7 Mon Sep 17 00:00:00 2001 From: Mariano Iglesias Date: Sat, 25 Feb 2012 19:47:35 -0300 Subject: [PATCH 21/25] Allowing li3_access to take into account Dispatcher rule modifications. Fixes #19 --- .../adapter/security/access/AuthRbac.php | 56 ++++++++------ extensions/adapter/security/access/Rules.php | 9 ++- extensions/adapter/security/access/Simple.php | 7 +- security/Access.php | 22 ++++-- .../adapter/security/access/AuthRbacTest.php | 77 ++++++++++++++----- .../adapter/security/access/RulesTest.php | 44 +++++------ 6 files changed, 132 insertions(+), 83 deletions(-) diff --git a/extensions/adapter/security/access/AuthRbac.php b/extensions/adapter/security/access/AuthRbac.php index ba11b3a..7a374a4 100644 --- a/extensions/adapter/security/access/AuthRbac.php +++ b/extensions/adapter/security/access/AuthRbac.php @@ -28,12 +28,13 @@ class AuthRbac extends \lithium\core\Object { * the user requesting access. Or false (because Auth::check() can return false). * This is an optional parameter, bercause we will fetch the users data trough Auth * seperately. - * @param object $request The Lithium Request object. + * @param mixed $params The Lithium `Request` object, or an array with at least + * 'request', and 'params' * @param array $options An array of additional options for the _getRolesByAuth method. * @return Array An empty array if access is allowed or * an array with reasons for denial if denied. */ - public function check($requester, $request, array $options = array()) { + public function check($requester, $params, array $options = array()) { if (empty($this->_roles)) { throw new ConfigException('No roles defined for adapter configuration.'); } @@ -57,11 +58,11 @@ public function check($requester, $request, array $options = array()) { } // Check to see if this role applies to this request - if (!static::parseMatch($role['match'], $request)) { + if (!static::parseMatch($role['match'], $params)) { continue; } - $accessible = static::_isAccessible($role, $request, $options); + $accessible = static::_isAccessible($role, $params, $options); if (!$accessible) { $message = !empty($role['message']) ? $role['message'] : $message; @@ -80,19 +81,19 @@ public function check($requester, $request, array $options = array()) { * Otherwise => grants access * * @param array $role Array Set of Roles (dereferenced) - * @param mixed $request A lithium Request object. + * @param mixed $quest A lithium Request object. * @param array $options An array of additional options for the _getRolesByAuth method. * @return boolean $accessable */ - protected static function _isAccessible(&$role, $request, $options) { + protected static function _isAccessible(&$role, $params, $options) { if ($role['allow'] === false) { return false; } - if (!static::_hasRole($role['requesters'], $request, $options)) { + if (!static::_hasRole($role['requesters'], $params, $options)) { return false; } if (is_array($role['allow'])) { - return static::_parseClosures($role['allow'], $request, $role); + return static::_parseClosures($role['allow'], $params['request'], $role); } return true; } @@ -104,26 +105,30 @@ protected static function _isAccessible(&$role, $request, $options) { * to validate. * Is also acceptable to match a parameter without a specific value. * * @param mixed $match A set of parameters to validate the request against. - * @param mixed $request A lithium Request object. + * @param mixed $params The Lithium `Request` object, or an array with at least + * 'request', and 'params' * @access public * @return boolean True if a match is found. */ - public static function parseMatch($match, $request) { + public static function parseMatch($match, $params) { if (empty($match)) { return false; } if (is_array($match)) { - if (!static::_parseClosures($match, $request)) { + $_params = $params; + if (!static::_parseClosures($match, $params['request'], $_params)) { return false; } + } elseif (is_callable($match)) { + return (boolean) $match($params['request'], $params); } - $params = array(); + $matchParams = array(); foreach ((array) $match as $key => $param) { if (is_string($param)) { if (preg_match('/^([A-Za-z0-9_\*\\\]+)::([A-Za-z0-9_\*]+)$/', $param, $regexMatches)) { - $params += array( + $matchParams += array( 'controller' => $regexMatches[1], 'action' => $regexMatches[2] ); @@ -131,10 +136,10 @@ public static function parseMatch($match, $request) { } } - $params[$key] = $param; + $matchParams[$key] = $param; } - foreach ($params as $type => $value) { + foreach ($matchParams as $type => $value) { if ($value === '*') { continue; } @@ -143,8 +148,8 @@ public static function parseMatch($match, $request) { $value = Inflector::underscore($value); } - $exists_in_request = array_key_exists($type, $request->params); - if (!$exists_in_request || $value !== Inflector::underscore($request->params[$type])) { + $exists_in_request = array_key_exists($type, $params['params']); + if (!$exists_in_request || $value !== Inflector::underscore($params['params'][$type])) { return false; } } @@ -161,11 +166,11 @@ public static function parseMatch($match, $request) { * @access protected * * @param array $data dereferenced Array - * @param mixed $request + * @param object $request The Lithium `Request` object * @param array $roleOptions dereferenced Array * @return boolean */ - protected static function _parseClosures(array &$data, $request, array &$roleOptions = array()){ + protected static function _parseClosures(array &$data, $request, array &$roleOptions = array()) { $return = true; foreach ($data as $key => $item) { if (is_callable($item)) { @@ -181,14 +186,15 @@ protected static function _parseClosures(array &$data, $request, array &$roleOpt /** * @todo reduce Model Overhead (will duplicated in each model) * - * @param Request $request Object + * @param mixed $params The Lithium `Request` object, or an array with at least + * 'request', and 'params' * @param array $options * @return array|mixed $roles Roles with attachted User Models */ - protected static function _getRolesByAuth($request, array $options = array()) { + protected static function _getRolesByAuth($params, array $options = array()) { $roles = array('*' => '*'); foreach (array_keys(Auth::config()) as $key) { - if ($check = Auth::check($key, $request, $options)) { + if ($check = Auth::check($key, $params['request'], $options)) { $roles[$key] = $check; } } @@ -199,13 +205,13 @@ protected static function _getRolesByAuth($request, array $options = array()) { * _hasRole Compares the results from _getRolesByAuth with the array passed to it. * * @param mixed $requesters - * @param mixed $request + * @param mixed $params * @param array $options * @access protected * @return void */ - protected static function _hasRole($requesters, $request, array $options = array()) { - $authed = array_keys(static::_getRolesByAuth($request, $options)); + protected static function _hasRole($requesters, $params, array $options = array()) { + $authed = array_keys(static::_getRolesByAuth($params, $options)); $requesters = (array) $requesters; if (in_array('*', $requesters)) { diff --git a/extensions/adapter/security/access/Rules.php b/extensions/adapter/security/access/Rules.php index f6de1d8..4a64d96 100644 --- a/extensions/adapter/security/access/Rules.php +++ b/extensions/adapter/security/access/Rules.php @@ -96,12 +96,13 @@ protected function _init() { * * @param mixed $user The user data array that holds all necessary information about * the user requesting access. Or false (because `Auth::check()` can return `false`). - * @param object $request The Lithium `Request` object. + * @param mixed $params The Lithium `Request` object, or an array with at least + * 'request', and 'params' * @param array $options An array of additional options. * @return array An empty array if access is allowed and an array with reasons for denial * if denied. */ - public function check($user, $request, array $options = array()) { + public function check($user, $params, array $options = array()) { $defaults = array( 'rules' => $this->_config['default'], 'allowAny' => $this->_config['allowAny'] @@ -121,7 +122,7 @@ public function check($user, $request, array $options = array()) { if (is_string($rule)) { $rule = compact('rule'); } - $ruleResult = $this->_call($rule, $user, $request, $options); + $ruleResult = $this->_call($rule, $user, $params['request'], $options); switch (true) { case ($ruleResult === false && $options['allowAny']): @@ -198,4 +199,4 @@ public function getRules($name = null) { } } -?> \ No newline at end of file +?> diff --git a/extensions/adapter/security/access/Simple.php b/extensions/adapter/security/access/Simple.php index 2b315c9..4095e54 100644 --- a/extensions/adapter/security/access/Simple.php +++ b/extensions/adapter/security/access/Simple.php @@ -15,14 +15,15 @@ class Simple extends \lithium\core\Object { * * @param mixed $user The user data array that holds all necessary information about * the user requesting access. Or `false` (because `Auth::check()` can return `false`). - * @param object $request The Lithium `Request` object. + * @param mixed $params The Lithium `Request` object, or an array with at least + * 'request', and 'params' * @param array $options An array of additional options. * @return Array An empty array if access is allowed and an array with reasons for denial * if denied. */ - public function check($user, $request, array $options = array()) { + public function check($user, $params, array $options = array()) { return !$user ? $options : array(); } } -?> \ No newline at end of file +?> diff --git a/security/Access.php b/security/Access.php index fe53f35..868145b 100644 --- a/security/Access.php +++ b/security/Access.php @@ -75,12 +75,13 @@ protected static function _initConfig($name, $config) { * @param string $name The name of the `Access` configuration/adapter to check against. * @param mixed $user The user data that holds all necessary information about * the user requesting access. Or `false` (because Auth::check() can return `false`). - * @param object $request The Lithium Request object. + * @param mixed $params The Lithium `Request` object, or an array with at least + * 'request', and 'params' * @param array $options An array of additional options. * @return Array An empty array if access is allowed and an array with reasons for denial * if denied. */ - public static function check($name, $user, $request, array $options = array()) { + public static function check($name, $user, $params, array $options = array()) { $defaults = array( 'message' => 'You are not permitted to access this area.', 'redirect' => '/' @@ -90,15 +91,20 @@ public static function check($name, $user, $request, array $options = array()) { if (($config = static::_config($name)) === null) { throw new ConfigException("Configuration `{$name}` has not been defined."); } + if (!is_array($params)) { + $params = array( + 'request' => $params, + 'params' => isset($params->params) ? $params->params : array() + ); + } $filter = function($self, $params) use ($name) { - $user = $params['user']; - $request = $params['request']; - $options = $params['options']; - return $self::adapter($name)->check($user, $request, $options); + return $self::adapter($name)->check( + $params['user'], $params['params'], $params['options'] + ); }; - $params = compact('user', 'request', 'options'); + $params = compact('user', 'params', 'options'); return static::_filter(__FUNCTION__, $params, $filter, (array) $config['filters']); } } -?> \ No newline at end of file +?> diff --git a/tests/cases/extensions/adapter/security/access/AuthRbacTest.php b/tests/cases/extensions/adapter/security/access/AuthRbacTest.php index 1fa7389..de5d686 100644 --- a/tests/cases/extensions/adapter/security/access/AuthRbacTest.php +++ b/tests/cases/extensions/adapter/security/access/AuthRbacTest.php @@ -66,6 +66,21 @@ function($request) { ) ) ), + 'test_allow_closure_match' => array( + 'adapter' => 'AuthRbac', + 'roles' => array( + array( + 'requesters' => '*', + 'match' => function($request) { + return !empty($request->params['allow_match']); + }, + 'allow' => function($request, &$roleOptions) { + $roleOptions['message'] = 'Test allow options set 2.'; + return $request->params['allow'] ? true : false; + } + ) + ) + ), 'test_message_override' => array( 'adapter' => 'AuthRbac', 'roles' => array( @@ -183,71 +198,73 @@ public function testCheckMessageOverride() { } public function testParseMatch() { - $request = new Request(array('params' => array( + $params = array( 'library' => 'test_library', 'controller' => 'test_controllers', 'action' => 'test_action' - ))); + ); + $request = new Request(array('params' => $params)); $match = array( 'library' => 'test_library', 'controller' => 'TestControllers', 'action' => 'test_action' ); - $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); + $this->assertTrue(Access::adapter('test_check')->parseMatch($match, compact('request', 'params'))); $match = array('controller' => 'TestControllers', 'action' => 'test_action'); - $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); + $this->assertTrue(Access::adapter('test_check')->parseMatch($match, compact('request', 'params'))); $match = array('library' => 'test_library', 'action' => 'test_action'); - $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); + $this->assertTrue(Access::adapter('test_check')->parseMatch($match, compact('request', 'params'))); $match = array('library' => 'test_library', 'controller' => 'TestControllers'); - $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); + $this->assertTrue(Access::adapter('test_check')->parseMatch($match, compact('request', 'params'))); $match = array( 'library' => 'test_no_match', 'controller' => 'TestControllers', 'action' => 'test_action' ); - $this->assertFalse(Access::adapter('test_check')->parseMatch($match, $request)); + $this->assertFalse(Access::adapter('test_check')->parseMatch($match, compact('request', 'params'))); $match = 'TestControllers::test_action'; - $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); + $this->assertTrue(Access::adapter('test_check')->parseMatch($match, compact('request', 'params'))); $match = 'TestControllers::*'; - $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); + $this->assertTrue(Access::adapter('test_check')->parseMatch($match, compact('request', 'params'))); $match = '*::test_action'; - $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); + $this->assertTrue(Access::adapter('test_check')->parseMatch($match, compact('request', 'params'))); $match = '*::*'; - $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); + $this->assertTrue(Access::adapter('test_check')->parseMatch($match, compact('request', 'params'))); $match = array('library' => 'test_library', '*::*'); - $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); + $this->assertTrue(Access::adapter('test_check')->parseMatch($match, compact('request', 'params'))); $match = array('library' => 'test_no_match', '*::*'); - $this->assertFalse(Access::adapter('test_check')->parseMatch($match, $request)); + $this->assertFalse(Access::adapter('test_check')->parseMatch($match, compact('request', 'params'))); $match = null; - $this->assertFalse(Access::adapter('test_check')->parseMatch($match, $request)); + $this->assertFalse(Access::adapter('test_check')->parseMatch($match, compact('request', 'params'))); $test = function() { return true; }; - $this->assertTrue(Access::adapter('test_closures')->parseMatch(array($test), $request)); + $this->assertTrue(Access::adapter('test_closures')->parseMatch(array($test), compact('request', 'params'))); $test = function() { return false; }; - $this->assertFalse(Access::adapter('test_closures')->parseMatch(array($test), $request)); - $this->assertFalse(Access::adapter('test_closures')->parseMatch(array(), $request)); + $this->assertFalse(Access::adapter('test_closures')->parseMatch(array($test), compact('request', 'params'))); + $this->assertFalse(Access::adapter('test_closures')->parseMatch(array(), compact('request', 'params'))); - $request = new Request(array('params' => array( + $params = array( 'controller' => 'lithium\test\Controller', 'action' => 'index' - ))); + ); + $request = new Request(array('params' => $params)); $match = 'Controller::*'; - $this->assertFalse(Access::adapter('test_check')->parseMatch($match, $request)); + $this->assertFalse(Access::adapter('test_check')->parseMatch($match, compact('request', 'params'))); $match = 'lithium\test\Controller::*'; - $this->assertTrue(Access::adapter('test_check')->parseMatch($match, $request)); + $this->assertTrue(Access::adapter('test_check')->parseMatch($match, compact('request', 'params'))); } public function testClosures() { @@ -289,6 +306,24 @@ public function testClosures() { $result = Access::check('test_allow_closure', $user, $request, $authSuccess); $expected = array('message' => 'Test allow options set.', 'redirect' => '/'); $this->assertIdentical($expected, $result); + + $request->params['allow'] = true; + $request->params['allow_match'] = true; + $result = Access::check('test_allow_closure_match', $user, $request, $authSuccess); + $expected = array(); + $this->assertIdentical($expected, $result); + + $request->params['allow'] = false; + $request->params['allow_match'] = true; + $result = Access::check('test_allow_closure_match', $user, $request, $authSuccess); + $expected = array('message' => 'Test allow options set 2.', 'redirect' => '/'); + $this->assertIdentical($expected, $result); + + $request->params['allow'] = true; + $request->params['allow_match'] = false; + $result = Access::check('test_allow_closure_match', $user, $request, $authSuccess); + $expected = array('message' => 'You are not permitted to access this area.', 'redirect' => '/'); + $this->assertIdentical($expected, $result); } public function testNoRolesConfigured() { diff --git a/tests/cases/extensions/adapter/security/access/RulesTest.php b/tests/cases/extensions/adapter/security/access/RulesTest.php index 82ab2c2..b0aaff0 100644 --- a/tests/cases/extensions/adapter/security/access/RulesTest.php +++ b/tests/cases/extensions/adapter/security/access/RulesTest.php @@ -23,15 +23,15 @@ public function testPatternBasedIpMatching() { 'message' => 'You can not access this from your location.', 'ip' => '/10\.0\.1\.\d+/' )); - $result = $adapter->check(array(), $request, compact('rules')); + $result = $adapter->check(array(), compact('request'), compact('rules')); $this->assertEqual(array(), $result); $request = new Request(array('env' => array('REMOTE_ADDR' => '10.0.1.255'))); - $result = $adapter->check(array(), $request, compact('rules')); + $result = $adapter->check(array(), compact('request'), compact('rules')); $this->assertEqual(array(), $result); $request = new Request(array('env' => array('REMOTE_ADDR' => '10.0.2.1'))); - $result = $adapter->check(array(), $request, compact('rules')); + $result = $adapter->check(array(), compact('request'), compact('rules')); $this->assertEqual('You can not access this from your location.', $result['message']); } @@ -45,13 +45,13 @@ public function testArrayBasedIpMatching() { foreach (array(2, 3, 4) as $i) { $request = new Request(array('env' => array('REMOTE_ADDR' => "10.0.1.{$i}"))); - $result = $adapter->check(array(), $request, compact('rules')); + $result = $adapter->check(array(), compact('request'), compact('rules')); $this->assertEqual(array(), $result); } foreach (array(1, 5, 255) as $i) { $request = new Request(array('env' => array('REMOTE_ADDR' => "10.0.1.{$i}"))); - $result = $adapter->check(array(), $request, compact('rules')); + $result = $adapter->check(array(), compact('request'), compact('rules')); $this->assertEqual('You can not access this from your location.', $result['message']); } } @@ -70,20 +70,20 @@ public function testCheck() { 'ip' => '10.0.1.1' ) ); - $result = $adapter->check($user, $request, compact('rules')); + $result = $adapter->check($user, compact('request'), compact('rules')); $this->assertEqual(array(), $result); $rules = array(array('rule' => 'denyAll', 'message' => 'You must be logged in.')); $expected = array('rule' => 'denyAll', 'message' => 'You must be logged in.'); - $result = $adapter->check($user, $request, compact('rules')); + $result = $adapter->check($user, compact('request'), compact('rules')); $this->assertEqual($expected, $result); $rules = array('rule' => 'allowAnyUser', 'message' => 'You must be logged in.'); $expected = array('rule' => 'allowAnyUser', 'message' => 'You must be logged in.'); - $result = $adapter->check(array(), $request, compact('rules')); + $result = $adapter->check(array(), compact('request'), compact('rules')); $this->assertEqual($expected, $result); - $result = $adapter->check(false, $request, compact('rules')); + $result = $adapter->check(false, compact('request'), compact('rules')); $this->assertEqual($expected, $result); } @@ -96,7 +96,7 @@ public function testCheckNoRules() { $adapter = new Rules(); $expected = array('rule' => false, 'message' => null, 'redirect' => null); - $result = $adapter->check($user, $request); + $result = $adapter->check($user, compact('request')); $this->assertEqual($expected, $result); } @@ -114,7 +114,7 @@ public function testPassingRules() { }) ); $expected = array(); - $result = $adapter->check($user, $request, compact('rules')); + $result = $adapter->check($user, compact('request'), compact('rules')); $this->assertEqual($expected, $result); } @@ -129,7 +129,7 @@ public function testAdd() { $rules = array(array('rule' => 'testDeny', 'message' => 'Access denied.')); $expected = array('rule' => 'testDeny', 'message' => 'Access denied.'); - $result = $adapter->check($user, $request, compact('rules')); + $result = $adapter->check($user, compact('request'), compact('rules')); $this->assertEqual($expected, $result); $this->assertTrue(is_callable($adapter->get('testDeny'))); @@ -152,14 +152,14 @@ public function testAllowAnyRule() { array('rule' => 'allowAll', 'message' => 'Access denied.'), array('rule' => 'denyAll', 'message' => 'Access denied.') ); - $result = $adapter->check($user, $request, compact('rules')); + $result = $adapter->check($user, compact('request'), compact('rules')); $this->assertEqual(array('rule' => 'denyAll', 'message' => 'Access denied.'), $result); $adapter = new Rules(array('allowAny' => true)); - $result = $adapter->check($user, $request, compact('rules')); + $result = $adapter->check($user, compact('request'), compact('rules')); $this->assertEqual(array(), $result); - $result = $adapter->check($user, $request, array('rules' => array('denyAll', 'allowAll'))); + $result = $adapter->check($user, compact('request'), array('rules' => array('denyAll', 'allowAll'))); $this->assertEqual(array(), $result); } @@ -171,7 +171,7 @@ public function testInvalidRule() { $adapter = new Rules(); $user = array('username' => 'Tom'); - $result = $adapter->check($user, $request, array('rules' => array('badness'))); + $result = $adapter->check($user, compact('request'), array('rules' => array('badness'))); $this->assertEqual(array('rule' => 'badness'), $result); } @@ -190,9 +190,9 @@ public function testOptionsPassedToRule() { 'default' => array('foobar') )); - $result = $adapter->check($user, $request, array('foo' => 'baz')); + $result = $adapter->check($user, compact('request'), array('foo' => 'baz')); $this->assertEqual(array('rule' => 'foobar', 'foo' => 'baz'), $result); - $result = $adapter->check($user, $request, array('foo' => 'bar')); + $result = $adapter->check($user, compact('request'), array('foo' => 'bar')); $this->assertEqual(array(), $result); } @@ -213,15 +213,15 @@ public function testAutoUser() { 'user' => function() use ($user) { return $user; } )); - $result = $adapter->check($user, $request); + $result = $adapter->check($user, compact('request')); $this->assertEqual(array(), $result); - $result = $adapter->check(null, $request); + $result = $adapter->check(null, compact('request')); $this->assertEqual(array(), $result); - $result = $adapter->check(array('username' => 'Bob'), $request); + $result = $adapter->check(array('username' => 'Bob'), compact('request')); $this->assertEqual(array('rule' => 'user'), $result); } } -?> \ No newline at end of file +?> From cfd76de2cc2f3a335b53ec95cc1e51430d7e8af9 Mon Sep 17 00:00:00 2001 From: Ciaro Vermeire Date: Sat, 3 Mar 2012 03:07:50 +0100 Subject: [PATCH 22/25] Updating and fixing merge conflicts. --- README.md | 166 ++++++++++-------- .../adapter/security/access/AuthRbac.php | 49 +++++- security/Access.php | 6 +- .../adapter/security/access/AuthRbacTest.php | 4 +- .../adapter/security/access/RulesTest.php | 2 +- .../adapter/security/access/SimpleTest.php | 2 +- .../adapter/auth/MockAuthAdapter.php | 7 +- 7 files changed, 152 insertions(+), 84 deletions(-) diff --git a/README.md b/README.md index b2dd08d..b32f11c 100644 --- a/README.md +++ b/README.md @@ -4,21 +4,21 @@ Checkout the code to either of your library directories: - cd libraries - git clone git@github.com + cd libraries + git clone https://github.com/rich97/li3_access.git Include the library in in your `/app/config/bootstrap/libraries.php` - Libraries::add('li3_access'); + Libraries::add('li3_access'); ## Usage You must configure the adapter you wish to use first, but once you have it configured it's fairly simple to use. - $access = Access::check('access_config_name', Auth::check('auth_config_name'), $this->request); - if(!empty($access)) { - $this->redirect($access['redirect']); - } + $access = Access::check('access_config_name', $this->request, Auth::check('auth_config_name')); + if(!empty($access)) { + $this->redirect($access['redirect']); + } If the request validates correctly based on your configuration then `Access::check()` will return an empty `array()` otherwise it will return an array with two keys; `message` and `redirect`. These values are built into the Access class but you can override them by passing them as `$options` to all three of the adapters in this repository. @@ -30,9 +30,9 @@ In this repository there are three adapters. All three work in a slightly differ The simple adapter is exactly what it says it is. The check method only checks that the data passed to is not empty and as a result the configuration is trivial. - Access::config( - 'simple' => array('adapter' => 'Simple') - ); + Access::config( + 'simple' => array('adapter' => 'Simple') + ); And that's it! @@ -40,26 +40,26 @@ And that's it! This adapter effectively allows you to tell it how it should work. It comes with a few preconfigured rules by default but it's very simple to add your own. Its configuration is the same as the `Simple` adapter if you only want to use the built in methods. - Access::config( - 'rules' => array('adapter' => 'Rules') - ); + Access::config( + 'rules' => array('adapter' => 'Rules') + ); Then to deny all requests from the authenticated user. - $access = Access::check('rules', Auth::check('auth_config_name'), $this->request, array('rule' => 'denyAll')); - if(!empty($access)) { - $this->redirect($access['redirect']); - } + $access = Access::check('rules', Auth::check('auth_config_name'), $this->request, array('rule' => 'denyAll')); + if(!empty($access)) { + $this->redirect($access['redirect']); + } There are four built in rules; allowAll, denyAll, allowAnyUser and allowIp, for more information see the adapter itself. However, this adapter is at its most useful when you add your own rules. - Access::adapter('custom_rule')->add(function($user, $request, $options) { - // Your logic here. Just make sure it returns an array. - }); + Access::adapter('custom_rule')->add(function($user, $request, $options) { + // Your logic here. Just make sure it returns an array. + }); Then to use your new rule: - $access = Access::check('rules', Auth::check('auth_config_name'), $this->request, array('rule' => 'custom_rule')); + $access = Access::check('rules', Auth::check('auth_config_name'), $this->request, array('rule' => 'custom_rule')); One more to go! @@ -69,55 +69,79 @@ This is the most complex adapter in this repository at this time. It's used for It's difficult to explain (I hope that's clear enough) so lets look at an example configuration to try and achieve some clarity: - Access::config( - 'auth_rbac' => array( - 'adapter' => 'AuthRbac', - 'roles' => array( - array( - 'requesters' => '*', - 'match' => '*::*' - ), - array( - 'message' => 'No panel for you!', - 'redirect' => array('library' => 'admin', 'Users::login'), - 'requesters' => 'admin', - 'match' => array('library' => 'admin', '*::*') - ), - array( - 'requesters' => '*', - 'match' => array('library' => 'admin', 'Users::login') - ), - array( - 'requesters' => '*', - 'match' => array('library' => 'admin', 'Users::logout') - ) - ) - ) - ) + $accountsEmpty = Accounts::count(); + + Access::config(array( + 'auth_rbac' => array( + 'adapter' => 'AuthRbac', + 'roles' => array( + array( + 'resources' => '*', + 'match' => '*::*' + ), + array( + 'message' => 'No panel for you!', + 'redirect' => array('library' => 'admin', 'Users::login'), + 'resources' => 'admin', + 'match' => array('library' => 'admin', '*::*') + ), + array( + 'resources' => '*', + 'match' => array( + 'library' => 'admin', 'Users::login', + function($request, &$options) { + return !empty($request->data); + } + ), + 'allow' => function($request, &$options) use ($accountsEmpty) { + if ($accountsEmpty) { + $options['message'] = 'No accounts exist yet!'; + } + return $accountsEmpty; + } + ), + array( + 'resources' => '*', + 'match' => array('library' => 'admin', 'Users::logout') + ) + ) + ) + )); First we tell it which adapter to use: - 'adapter' => 'AuthRbac', + 'adapter' => 'AuthRbac', Then we set the roles array. This array is required if you want to use this adapter. The roles are evaluated from top to bottom. So if a role at the bottom contradicts one closer to the top, the bottom will take precedence. #### There are five possible options you can specify for a single role. -*match* +`'message'` -A rule used to match (see: `AuthRbac::parseMatch()`) this role against the request object passed from the `check()` method. You may use a parameters array where you explicitly set the parameter/value pairs, a shorthand syntax very similar to the one you use when generating urls or even a. Without match being set the role will always deny access. +Overwrites the default message to display if the rule matches the request and is disallowed. -Examples: +`'redirect'` -* `'Dashboards::index'` -> `array('controller' => 'Dashboards', 'action' => 'index')` -* `'Dashboards::*'` -> `array('controller' => 'Dashboards', 'action' => '*')` -> `Any action in the Dasboards controller.` -* `array('library' => 'admin', '*::*');` -> `array('library' => 'admin_plugin', 'controller' => '*', 'action' => '*')` -> `Any controller/action combination under the admin library.` +Overwrites the default redirect to use if the rule matches the request and is dissallowed. -**requester** +`'match'` -A string or an array of auth configuration keys that this rule applies to. The string `*` denotes everyone, even those who are not authenticated. A string of `admin` will apply this to everyone who can be authenticated against the user defined `admin` Auth configuration. An array of configuration keys does the same but you can apply it to multiple Auth configurations in one go. +A rule used to match this role against the request object passed from the `check()` method. You may use a parameters array where you explicitly set the parameter/value pairs, a shorthand syntax very similar to the one you use when generating urls or even a closure. Without match being set the role will always deny access. -*Example*: +In the closure example configuration: + + 'match' => array( + 'library' => 'admin', 'Users::login', + function($request, &$roleOptions) { + return !empty($request->data); + } + ) + +Not only must the library, controller and action match but the closure must return true. So this role will only apply to this request if all of the request params match and the request data is set. + +`'resources'` + +A string or an array of auth configuration keys that this rule applies to. The string `*` denotes everyone, even those who are not authenticated. A string of `admin` will validate anyone who can be authenticated against the user defined `admin` Auth configuration. An array of configuration keys does the same but you can apply it to multiple Auth configurations in one go. Assuming we have an Auth configuration like so: @@ -142,27 +166,29 @@ Assuming we have an Auth configuration like so: ) )); -Setting 'requester' => array('user', 'customer') would only apply the rule to anyone that could authenticate as a user or customer. Setting 'requester' => '*' would mean that all of these auth configurations and people that are not authenticated would have this role applied to them. +Setting `'resources' => array('user', 'customer')` would only apply the rule to anyone that could authenticate as a user or customer. Setting `'resource' => '*'` would mean that all of these auth configurations and people that are not authenticated would have this role applied to them. + +`'allow'` -**allow** +A boolean that if set to false forces a role that would have been granted access to deny access. Much like the 'match' option you can also pass a closure to this option. This way you can blacklist every resource and then whitelist resources manually. Also by passing a closure you can deny access based upon the request. -A boolean that if set to false forces a role that would have been granted access to deny access. This way you can apply a rule to everyone and then proceed to exclude requesters manualy. +Finally, if you pass either $request or $options you can modify their values at runtime. ### Filters The Access::check() method is filterable. You can apply the filters in the configuration like so: - Access::config(array( - 'rule_based' => array( - 'adapter' => 'Rules', - 'filters' => array( - function($self, $params, $chain) { - // Filter logic goes here - return $chain->next($self, $params, $chain); - } - ) - ) - )); + Access::config(array( + 'rule_based' => array( + 'adapter' => 'Rules', + 'filters' => array( + function($self, $params, $chain) { + // Filter logic goes here + return $chain->next($self, $params, $chain); + } + ) + ) + )); ## Credits diff --git a/extensions/adapter/security/access/AuthRbac.php b/extensions/adapter/security/access/AuthRbac.php index 7a374a4..530926a 100644 --- a/extensions/adapter/security/access/AuthRbac.php +++ b/extensions/adapter/security/access/AuthRbac.php @@ -3,8 +3,8 @@ namespace li3_access\extensions\adapter\security\access; use lithium\security\Auth; -use lithium\core\ConfigException; use lithium\util\Inflector; +use lithium\core\ConfigException; class AuthRbac extends \lithium\core\Object { @@ -26,7 +26,7 @@ class AuthRbac extends \lithium\core\Object { * * @param mixed $requester The user data array that holds all necessary information about * the user requesting access. Or false (because Auth::check() can return false). - * This is an optional parameter, bercause we will fetch the users data trough Auth + * This is an optional parameter, because we will fetch the users data through Auth * seperately. * @param mixed $params The Lithium `Request` object, or an array with at least * 'request', and 'params' @@ -210,6 +210,51 @@ protected static function _getRolesByAuth($params, array $options = array()) { * @access protected * @return void */ + protected function _accessable($resources, $roles, array $options = array()) { + $resources = (array) $resources; + if (in_array('*', $resources)) { + return true; + } + + foreach ($resources as $resource) { + if (array_key_exists($resource, $roles)) { + return true; + } + } + return false; + } + + /** + * Itterates over an array and runs any anonymous functions it finds. Returns + * true if all of the closures it runs evaluate to true. $match is passed by + * reference and any closures found are removed from it before the method is complete. + * + * @param array $data + * @param mixed $request + * @access protected + * @return void + */ + protected function _run(&$data, $request = null, array &$options = array()) { + if (is_bool($data)) { + return $data; + } + + if (!is_array($data)) { + return false; + } + + $allow = true; + foreach ($data as $key => $item) { + if (is_callable($item)) { + if ($allow === true) { + $allow = (boolean) $item($request, $options); + } + unset($data[$key]); + } + } + return $allow; + } + protected static function _hasRole($requesters, $params, array $options = array()) { $authed = array_keys(static::_getRolesByAuth($params, $options)); diff --git a/security/Access.php b/security/Access.php index 868145b..afd1837 100644 --- a/security/Access.php +++ b/security/Access.php @@ -5,7 +5,7 @@ * @author Tom Maiaroto * @copyright Copyright 2010, Union of RAD (http://union-of-rad.org) * @license http://opensource.org/licenses/bsd-license.php The BSD License -*/ + */ namespace li3_access\security; @@ -29,7 +29,6 @@ class Access extends \lithium\core\Adaptable { * @var object `Collection` of authentication configurations. */ protected static $_configurations = array(); - /** * Libraries::locate() compatible path to adapters for this class. * @@ -73,7 +72,8 @@ protected static function _initConfig($name, $config) { * perhaps, login. * * @param string $name The name of the `Access` configuration/adapter to check against. - * @param mixed $user The user data that holds all necessary information about + * @param object $request A Lithium Request object. + * @param mixed $resource The user data that holds all necessary information about * the user requesting access. Or `false` (because Auth::check() can return `false`). * @param mixed $params The Lithium `Request` object, or an array with at least * 'request', and 'params' diff --git a/tests/cases/extensions/adapter/security/access/AuthRbacTest.php b/tests/cases/extensions/adapter/security/access/AuthRbacTest.php index de5d686..56aa0fc 100644 --- a/tests/cases/extensions/adapter/security/access/AuthRbacTest.php +++ b/tests/cases/extensions/adapter/security/access/AuthRbacTest.php @@ -278,7 +278,7 @@ public function testClosures() { $request->params['match'] = true; $request->params['allow'] = true; $result = Access::check('test_closures', $user, $request, $authSuccess); - $this->assertIdentical(array(), $result); + $this->assertIdentical(array(), $result); $request->params['match'] = true; $request->params['allow'] = false; @@ -338,4 +338,4 @@ public function testNoRolesConfigured() { } } -?> +?> \ No newline at end of file diff --git a/tests/cases/extensions/adapter/security/access/RulesTest.php b/tests/cases/extensions/adapter/security/access/RulesTest.php index b0aaff0..0bcbf3c 100644 --- a/tests/cases/extensions/adapter/security/access/RulesTest.php +++ b/tests/cases/extensions/adapter/security/access/RulesTest.php @@ -224,4 +224,4 @@ public function testAutoUser() { } } -?> +?> \ No newline at end of file diff --git a/tests/cases/extensions/adapter/security/access/SimpleTest.php b/tests/cases/extensions/adapter/security/access/SimpleTest.php index 5a117c0..42ea441 100644 --- a/tests/cases/extensions/adapter/security/access/SimpleTest.php +++ b/tests/cases/extensions/adapter/security/access/SimpleTest.php @@ -5,7 +5,7 @@ * @author Tom Maiaroto * @copyright Copyright 2010, Union of RAD (http://union-of-rad.org) * @license http://opensource.org/licenses/bsd-license.php The BSD License -*/ + */ namespace li3_access\tests\cases\extensions\adapter\security\access; diff --git a/tests/mocks/extensions/adapter/auth/MockAuthAdapter.php b/tests/mocks/extensions/adapter/auth/MockAuthAdapter.php index 85f420f..10e414b 100644 --- a/tests/mocks/extensions/adapter/auth/MockAuthAdapter.php +++ b/tests/mocks/extensions/adapter/auth/MockAuthAdapter.php @@ -9,13 +9,10 @@ public function check($credentials, array $options = array()) { if (isset($options['success']) && !empty($credentials->data)) { $granted = $credentials->data; } - return $granted; + return $granted; } - public function set($data, array $options = array()) { - if (isset($options['fail'])) { - return false; - } + public function set($data) { return $data; } From e040213d7d478e5c376abc7c0ea844b50e9c1c6e Mon Sep 17 00:00:00 2001 From: Ciaro Vermeire Date: Wed, 7 Mar 2012 01:10:09 +0100 Subject: [PATCH 23/25] Minor QA. --- README.md | 2 +- extensions/adapter/security/access/AuthRbac.php | 4 ++-- extensions/adapter/security/access/Simple.php | 5 ----- security/Access.php | 5 ++--- 4 files changed, 5 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index b32f11c..d9cd32a 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ Checkout the code to either of your library directories: cd libraries - git clone https://github.com/rich97/li3_access.git + git clone https://github.com/tmaiaroto/li3_access.git Include the library in in your `/app/config/bootstrap/libraries.php` diff --git a/extensions/adapter/security/access/AuthRbac.php b/extensions/adapter/security/access/AuthRbac.php index 530926a..4336448 100644 --- a/extensions/adapter/security/access/AuthRbac.php +++ b/extensions/adapter/security/access/AuthRbac.php @@ -20,7 +20,7 @@ class AuthRbac extends \lithium\core\Object { protected $_roles = null; /** - * The `Rbac` adapter will iterate trough the rbac data Array. + * The `Rbac` adapter will iterate through the rbac data Array. * * @todo: write better tests! * @@ -189,7 +189,7 @@ protected static function _parseClosures(array &$data, $request, array &$roleOpt * @param mixed $params The Lithium `Request` object, or an array with at least * 'request', and 'params' * @param array $options - * @return array|mixed $roles Roles with attachted User Models + * @return array|mixed $roles Roles with attached User Models */ protected static function _getRolesByAuth($params, array $options = array()) { $roles = array('*' => '*'); diff --git a/extensions/adapter/security/access/Simple.php b/extensions/adapter/security/access/Simple.php index 4095e54..f527002 100644 --- a/extensions/adapter/security/access/Simple.php +++ b/extensions/adapter/security/access/Simple.php @@ -2,11 +2,6 @@ namespace li3_access\extensions\adapter\security\access; -/* uncomment it if needed -use lithium\core\Libraries; -use lithium\util\Set; -*/ - class Simple extends \lithium\core\Object { /** diff --git a/security/Access.php b/security/Access.php index afd1837..8f181b5 100644 --- a/security/Access.php +++ b/security/Access.php @@ -72,9 +72,8 @@ protected static function _initConfig($name, $config) { * perhaps, login. * * @param string $name The name of the `Access` configuration/adapter to check against. - * @param object $request A Lithium Request object. - * @param mixed $resource The user data that holds all necessary information about - * the user requesting access. Or `false` (because Auth::check() can return `false`). + * @param mixed $user The user data array that holds all necessary information about + * the user requesting access. Or `false` (because `Auth::check()` can return `false`). * @param mixed $params The Lithium `Request` object, or an array with at least * 'request', and 'params' * @param array $options An array of additional options. From 20ea0f8cabc989442d3a469fa061570ddce3557f Mon Sep 17 00:00:00 2001 From: Ciaro Vermeire Date: Sat, 18 Aug 2012 16:59:06 +0200 Subject: [PATCH 24/25] Minor tweak to defaults --- security/Access.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/security/Access.php b/security/Access.php index 8f181b5..5a7ff6a 100644 --- a/security/Access.php +++ b/security/Access.php @@ -82,8 +82,8 @@ protected static function _initConfig($name, $config) { */ public static function check($name, $user, $params, array $options = array()) { $defaults = array( - 'message' => 'You are not permitted to access this area.', - 'redirect' => '/' + 'message' => 'You are not authorized to access this page.', + 'redirect' => 'Users::login' ); $options += $defaults; From a7a5fc1c994ffd6b662c87621364fdcb4f364e93 Mon Sep 17 00:00:00 2001 From: Ciaro Vermeire Date: Mon, 8 Jul 2013 07:44:16 +0200 Subject: [PATCH 25/25] Adding basic composer support --- composer.json | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 composer.json diff --git a/composer.json b/composer.json new file mode 100644 index 0000000..f5a5d4a --- /dev/null +++ b/composer.json @@ -0,0 +1,14 @@ +{ + "name": "ciaro/li3_access", + "description": "Lithium Access Plugin.", + "type": "lithium-library", + "authors": [ + { + "name": "Tom Maiaroto" + } + ], + "require": { + "composer/installers": "*" + }, + "minimum-stability": "dev" +} \ No newline at end of file