Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add default debug logging for rbac rules when debug is enabled #2

Merged
merged 2 commits into from
Apr 7, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
/vendor
/.idea
composer.lock
.php_cs.*
1 change: 0 additions & 1 deletion src/Auth/ApiKeyAuthenticate.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
*/
class ApiKeyAuthenticate extends BaseAuthenticate
{

const TYPE_QUERYSTRING = 'querystring';
const TYPE_HEADER = 'header';

Expand Down
81 changes: 81 additions & 0 deletions src/Auth/Rbac/PermissionMatchResult.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
<?php
/**
* Copyright 2010 - 2017, Cake Development Corporation (https://www.cakedc.com)
*
* Licensed under The MIT License
* Redistributions of files must retain the above copyright notice.
*
* @copyright Copyright 2010 - 2017, Cake Development Corporation (https://www.cakedc.com)
* @license MIT License (http://www.opensource.org/licenses/mit-license.php)
*/

namespace CakeDC\Auth\Auth\Rbac;

/**
* Provides additional context on the result of a permission match operation,
* for example allows to attach a debug reason on the matched rule
*
* @package Auth\Rbac
*/
class PermissionMatchResult
{
/**
* @var bool
*/
protected $_allowed;

/**
* @var string
*/
protected $_reason;

/**
* PermissionMatchResult constructor.
*
* @param bool $allowed rule was matched, allowed value
* @param string $reason reason to either allow or deny
*/
public function __construct($allowed = false, $reason = '')
{
$this->_allowed = $allowed;
$this->_reason = $reason;
}

/**
* @param bool $allowed allowed value
* @return PermissionMatchResult
*/
public function setAllowed($allowed)
{
$this->_allowed = (bool)$allowed;

return $this;
}

/**
* @return bool
*/
public function isAllowed()
{
return $this->_allowed;
}

/**
* @param string $reason reason
* @return PermissionMatchResult
*/
public function setReason($reason)
{
$this->_reason = (string)$reason;

return $this;
}

/**
* @return string
*/
public function getReason()
{
return $this->_reason;
}
}
52 changes: 30 additions & 22 deletions src/Auth/SimpleRbacAuthorize.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace CakeDC\Auth\Auth;

use CakeDC\Auth\Auth\Rbac\PermissionMatchResult;
use CakeDC\Auth\Auth\Rules\Rule;
use Cake\Auth\BaseAuthorize;
use Cake\Controller\ComponentRegistry;
Expand All @@ -32,11 +33,11 @@ class SimpleRbacAuthorize extends BaseAuthorize
use LogTrait;

protected $_defaultConfig = [
//autoload permissions.php
// autoload permissions.php
'autoload_config' => 'permissions',
//role field in the Users table
// role field in the Users table
'role_field' => 'role',
//default role, used in new users registered and also as role matcher when no role is available
// default role, used in new users registered and also as role matcher when no role is available
'default_role' => 'user',
/*
* This is a quick roles-permissions implementation
Expand Down Expand Up @@ -79,6 +80,8 @@ class SimpleRbacAuthorize extends BaseAuthorize
*
*/
'permissions' => [],
// 'log' will match the value of 'debug' if not set on configuration
'log' => false,
];

/**
Expand Down Expand Up @@ -117,6 +120,9 @@ class SimpleRbacAuthorize extends BaseAuthorize
*/
public function __construct(ComponentRegistry $registry, array $config = [])
{
if (!isset($config['log'])) {
$config['log'] = Configure::read('debug');
}
parent::__construct($registry, $config);
$autoload = $this->getConfig('autoload_config');
if ($autoload) {
Expand Down Expand Up @@ -184,9 +190,13 @@ protected function _checkPermissions(array $user, $role, ServerRequest $request)
{
$permissions = $this->getConfig('permissions');
foreach ($permissions as $permission) {
$allowed = $this->_matchPermission($permission, $user, $role, $request);
if ($allowed !== null) {
return $allowed;
$matchResult = $this->_matchPermission($permission, $user, $role, $request);
if ($matchResult !== null) {
if ($this->getConfig('log')) {
$this->log($matchResult->getReason(), LogLevel::DEBUG);
}

return $matchResult->isAllowed();
}
}

Expand All @@ -201,7 +211,8 @@ protected function _checkPermissions(array $user, $role, ServerRequest $request)
* @param string $role Effective user's role
* @param \Cake\Http\ServerRequest $request Current request
*
* @return null|bool Null if permission is discarded, boolean if a final result is produced
* @return null|PermissionMatchResult Null if permission is discarded, PermissionMatchResult if a final
* result is produced
*/
protected function _matchPermission(array $permission, array $user, $role, ServerRequest $request)
{
Expand All @@ -210,20 +221,14 @@ protected function _matchPermission(array $permission, array $user, $role, Serve
$issetUser = isset($permission['user']) || isset($permission['*user']);

if (!$issetController || !$issetAction) {
$this->log(
"Cannot evaluate permission when 'controller' and/or 'action' keys are absent",
LogLevel::DEBUG
);
$reason = "Cannot evaluate permission when 'controller' and/or 'action' keys are absent";

return false;
return new PermissionMatchResult(false, $reason);
}
if ($issetUser) {
$this->log(
"Permission key 'user' is illegal, cannot evaluate the permission",
LogLevel::DEBUG
);
$reason = "Permission key 'user' is illegal, cannot evaluate the permission";

return false;
return new PermissionMatchResult(false, $reason);
}

$permission += ['allowed' => true];
Expand Down Expand Up @@ -263,7 +268,14 @@ protected function _matchPermission(array $permission, array $user, $role, Serve
$return = !$return;
}
if ($key === 'allowed') {
return $return;
$reason = sprintf(
'For %s --> Rule matched %s with result = %s',
json_encode($reserved),
json_encode($permission),
$return
);

return new PermissionMatchResult($return, $reason);
}
if (!$return) {
break;
Expand All @@ -286,10 +298,6 @@ protected function _matchOrAsterisk($possibleValues, $value, $allowEmpty = false
{
$possibleArray = (array)$possibleValues;

if ($allowEmpty && empty($possibleArray) && $value === null) {
return true;
}

if ($possibleValues === '*' ||
in_array($value, $possibleArray) ||
in_array(Inflector::camelize($value, '-'), $possibleArray)
Expand Down
1 change: 0 additions & 1 deletion tests/App/Controller/AppController.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
*/
class AppController extends Controller
{

public function initialize()
{
parent::initialize();
Expand Down
1 change: 0 additions & 1 deletion tests/TestCase/Auth/ApiKeyAuthenticateTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

class ApiKeyAuthenticateTest extends TestCase
{

public $fixtures = [
'plugin.CakeDC/Auth.users',
];
Expand Down
71 changes: 71 additions & 0 deletions tests/TestCase/Auth/Rbac/PermissionMatchResultTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
<?php
/**
* Copyright 2010 - 2017, Cake Development Corporation (https://www.cakedc.com)
*
* Licensed under The MIT License
* Redistributions of files must retain the above copyright notice.
*
* @copyright Copyright 2010 - 2017, Cake Development Corporation (https://www.cakedc.com)
* @license MIT License (http://www.opensource.org/licenses/mit-license.php)
*/

namespace CakeDC\Auth\Auth\Test\TestCase\Auth\Auth;

use CakeDC\Auth\Auth\Rbac\PermissionMatchResult;
use CakeDC\Auth\Auth\Rules\Rule;
use CakeDC\Auth\Auth\SimpleRbacAuthorize;
use Cake\Controller\ComponentRegistry;
use Cake\Http\Response;
use Cake\Http\ServerRequest;
use Cake\TestSuite\TestCase;
use Cake\Utility\Hash;
use Psr\Log\LogLevel;
use ReflectionClass;

class PermissionMatchResultTest extends TestCase
{
/**
* @var PermissionMatchResult
*/
protected $permissionMatchResult;

/**
* Sets up the fixture, for example, opens a network connection.
* This method is called before a test is executed.
*/
public function setUp()
{
$this->permissionMatchResult = new PermissionMatchResult();
}

/**
* Tears down the fixture, for example, closes a network connection.
* This method is called after a test is executed.
*/
public function tearDown()
{
unset($this->permissionMatchResult);
}

/**
* test
*/
public function testConstruct()
{
$permissionMatchResult = new PermissionMatchResult();
$this->assertFalse($permissionMatchResult->isAllowed());
$this->assertSame('', $permissionMatchResult->getReason());

$permissionMatchResult = new PermissionMatchResult(true, 'bazinga');
$this->assertTrue($permissionMatchResult->isAllowed());
$this->assertSame('bazinga', $permissionMatchResult->getReason());
}

public function testSetGet()
{
$this->assertTrue($this->permissionMatchResult->setAllowed(true)->isAllowed());
$this->assertFalse($this->permissionMatchResult->setAllowed(false)->isAllowed());
$this->assertNotSame('bazinga', $this->permissionMatchResult->setReason('another-reason')->getReason());
$this->assertSame('bazinga', $this->permissionMatchResult->setReason('bazinga')->getReason());
}
}
1 change: 0 additions & 1 deletion tests/TestCase/Auth/RememberMeAuthenticateTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

class RememberMeAuthenticateTest extends TestCase
{

public $fixtures = [
'plugin.CakeDC/Auth.users',
];
Expand Down
1 change: 0 additions & 1 deletion tests/TestCase/Auth/Rules/OwnerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
*/
class OwnerTest extends TestCase
{

public $fixtures = [
'plugin.CakeDC/Auth.posts',
'plugin.CakeDC/Auth.users',
Expand Down
1 change: 1 addition & 0 deletions tests/TestCase/Auth/SimpleRbacAuthorizeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1044,6 +1044,7 @@ public function testBadPermission($permissions, $user, $requestParams, $expected
->setMethods(['_loadPermissions', 'log'])
->disableOriginalConstructor()
->getMock();
$simpleRbacAuthorize->setConfig('log', true);
$simpleRbacAuthorize
->expects($this->once())
->method('log')
Expand Down