Skip to content
This repository was archived by the owner on Jan 30, 2020. It is now read-only.
Closed
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
267 changes: 267 additions & 0 deletions src/Assertion/ExpressionAssertion.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,267 @@
<?php
/**
* Zend Framework (http://framework.zend.com/)
*
* @link http://github.com/zendframework/zf2 for the canonical source repository
* @copyright Copyright (c) 2005-2015 Zend Technologies USA Inc. (http://www.zend.com)
* @license http://framework.zend.com/license/new-bsd New BSD License
*/
namespace Zend\Permissions\Acl\Assertion;

use Zend\Permissions\Acl\Acl;
use Zend\Permissions\Acl\Role\RoleInterface;
use Zend\Permissions\Acl\Resource\ResourceInterface;
use Zend\Permissions\Acl\Assertion\Exception\InvalidAssertionException;
use Zend\Permissions\Acl\Exception\RuntimeException;

/**
* @author Nikola Posa <posa.nikola@gmail.com>
*/
final class ExpressionAssertion implements AssertionInterface
{
const OPERAND_CONTEXT_PROPERTY = '__context';

const OPERATOR_EQ = '=';
const OPERATOR_NEQ = '!=';
const OPERATOR_LT = '<';
const OPERATOR_LTE = '<=';
const OPERATOR_GT = '>';
const OPERATOR_GTE = '>=';
const OPERATOR_IN = 'in';
const OPERATOR_NIN = 'nin';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one would make more sense as !in, making it more akin to !=. I'll make that change on merge.

const OPERATOR_REGEX = 'regex';

/**
* @var mixed
*/
private $left;

/**
* @var string
*/
private $operator;

/**
* @var mixed
*/
private $right;

/**
* @var array
*/
private $assertContext = [];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be removed. assert() should pass the context to methods that need it.

The reason is concurrency. In async systems, if this value gets reset in another process, it can affect the operands in the current process. By passing it around instead, the context of the current process can never be changed.


/**
* @var array
*/
private static $validOperators = [
self::OPERATOR_EQ,
self::OPERATOR_NEQ,
self::OPERATOR_LT,
self::OPERATOR_LTE,
self::OPERATOR_GT,
self::OPERATOR_GTE,
self::OPERATOR_IN,
self::OPERATOR_NIN,
self::OPERATOR_REGEX,
];

private function __construct($left, $operator, $right)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There really needs to be documentation of what constitutes valid operands.

{
$this->left = $left;
$this->operator = $operator;
$this->right = $right;
}

/**
* @param mixed $left
* @param string $operator
* @param mixed $right
* @return self
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should indicate exceptions that can be thrown by validateOperand() and validateOperator().

public static function fromProperties($left, $operator, $right)
{
$operator = strtolower($operator);

self::validateOperand($left);
self::validateOperator($operator);
self::validateOperand($right);

return new self($left, $operator, $right);
}

/**
* @param array $expression
* @throws InvalidAssertionException
* @return self
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment here, and also indicate the more specific exception this method raises.

*/
public static function fromArray(array $expression)
{
$required = ['left', 'operator', 'right'];

if (count(array_intersect_key($expression, array_flip($required))) < count($required)) {
throw new InvalidAssertionException(
"Expression assertion requires 'left', 'operator' and 'right' to be supplied"
);
}

return self::fromProperties(
$expression['left'],
$expression['operator'],
$expression['right']
);
}

private static function validateOperand($operand)
{
if (is_array($operand) && isset($operand[self::OPERAND_CONTEXT_PROPERTY])) {
if (!is_string($operand[self::OPERAND_CONTEXT_PROPERTY])) {
throw new InvalidAssertionException('Expression assertion context operand must be string');
}
}
}

private static function validateOperator($operator)
{
if (!in_array($operator, self::$validOperators)) {
throw new InvalidAssertionException('Provided expression assertion operator is not supported');
}
}

/**
* {@inheritDoc}
*/
public function assert(Acl $acl, RoleInterface $role = null, ResourceInterface $resource = null, $privilege = null)
{
$this->assertContext = [
'acl' => $acl,
'role' => $role,
'resource' => $resource,
'privilege' => $privilege,
];

return $this->evaluate();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As noted above, do not memoize the context, but instead pass it on as an argument.

}

private function evaluate()
{
$left = $this->getLeftValue();
$right = $this->getRightValue();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this method receives the context, it would then pass it to the above two methods.


return static::evaluateExpression($left, $this->operator, $right);
}

private function getLeftValue()
{
return $this->resolveOperandValue($this->left);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this method, and the getRightValue() method, would pass the context on to resolveOperandValue().

}

private function getRightValue()
{
return $this->resolveOperandValue($this->right);
}

private function resolveOperandValue($operand)
{
if (is_array($operand) && isset($operand[self::OPERAND_CONTEXT_PROPERTY])) {
$contextProperty = $operand[self::OPERAND_CONTEXT_PROPERTY];

if (strpos($contextProperty, '.') !== false) { //property path?
list($objectName, $objectField) = explode('.', $contextProperty, 2);

if (!isset($this->assertContext[$objectName])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once we get here, this line (and others in this method) would instead reference the context passed to the method.

throw new RuntimeException(sprintf(
"'%s' is not available in the assertion context",
$objectName
));
}

try {
return $this->getObjectFieldValue($this->assertContext[$objectName], $objectField);
} catch (\RuntimeException $ex) {
throw new RuntimeException(sprintf(
"'%s' property cannot be resolved on the '%s' object",
$objectField,
$objectName
));
}
}

if (!isset($this->assertContext[$contextProperty])) {
throw new RuntimeException(sprintf(
"'%s' is not available in the assertion context",
$contextProperty
));
}

return $this->assertContext[$contextProperty];
}

return $operand;
}

private function getObjectFieldValue($object, $field)
{
$accessors = ['get', 'is'];

$fieldAccessor = $field;

if (false !== strpos($field, '_')) {
$fieldAccessor = str_replace(' ', '', ucwords(str_replace('_', ' ', $field)));
}

foreach ($accessors as $accessor) {
$accessor .= $fieldAccessor;

if (!method_exists($object, $accessor)) {
continue;
}

return $object->$accessor();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The above could be more simply written as:

if (method_exists($object, $accessor)) {
    return $object->accessor();
}

}

if (!property_exists($object, $field)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't do what you think it does. Or, rather, it has an unintentional side-effect you may not be aware of.

Starting in PHP 5.3, property_exists started returning true regardless of visibility. This means that the above will return true even if the property is not public. Which makes line 227 a potential error. The only way to solve this is via reflection.

throw new \RuntimeException('Object property cannot be resolved');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you using a global exception case here, versus the package-specific one elsewhere?

}

return $object->$field;
}

private static function evaluateExpression($left, $operator, $right)
{
switch ($operator) {
case self::OPERATOR_EQ :
return $left == $right;
case self::OPERATOR_NEQ :
return $left != $right;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiousity, why are these using the non-strict forms? Do you think we should have additional operators for strict in/equality?

case self::OPERATOR_LT :
return $left < $right;
case self::OPERATOR_LTE :
return $left <= $right;
case self::OPERATOR_GT :
return $left > $right;
case self::OPERATOR_GTE :
return $left >= $right;
case self::OPERATOR_IN :
return in_array($left, $right);
case self::OPERATOR_NIN :
return !in_array($left, $right);
case self::OPERATOR_REGEX :
return (bool) preg_match($right, $left);
default :
throw new RuntimeException(sprintf(
'Unsupported expression assertion operator: %s',
$operator
));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, there is no possible way to get to the default case, as you've validated the operator previously.

}
}

public function __sleep()
{
return [
'left',
'operator',
'right',
];
}
}
Loading