diff --git a/README.md b/README.md
index 92b393f..c618ce4 100644
--- a/README.md
+++ b/README.md
@@ -7,11 +7,6 @@ Combines API Platform ORM Filters with AND, OR and NOT according to client reque
- works with built in filters of Api Platform, except for DateFilter
with EXCLUDE_NULL. A DateFilter subclass is provided to correct this.
-SECURIY WARNING: The current version of LogicFilter allows clients
-to bypass criteria set by custom Extensions to limit their access to certain data,
-like the examples do in the docs on [Custom Doctrine ORM Extension](https://api-platform.com/docs/core/extensions/#custom-doctrine-orm-extension)
-see [Issue 10](https://github.com/metaclass-nl/filter-bundle/issues/10).
-
Usage
-----
Once the FilterLogic class and service configuration have been installed in you app,
@@ -89,6 +84,8 @@ class Article {
// ...
}
```
+SECURITY WARNING: do not use this option if the working of any of the extionsions
+you use relies on INNER JOINS selecting only rows with NOT NULL values!
In case you do not like FilterLogic messing with the joins you can make
the built-in filters of Api Platform generate left joins themselves by first adding
@@ -103,6 +100,12 @@ class Article {
// ...
}
```
+SECURITY WARNING: Extensions that use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryBuilderHelper::addJoinOnce
+or ApiPlatform\Core\Bridge\Doctrine\Orm\PropertyHelperTrait::addJoinsForNestedProperty
+may not have been intended to generate left joins instead of inner joins. Of course this would
+technically be their error, not yours, but still it is better to prevent an eventual breach of security
+then having to deal with the consequences.
+
With one fo these workarounds the following will find Articles whose title contains 'pro'
as well as those whose keywords contain one whose word is 'php'.
@@ -142,6 +145,10 @@ with the others by either Doctrine\ORM\Query\Expr\Andx or Doctrine\ORM\Query\Exp
May Fail if a filter or extension uses QueryBuilder::where or ::add.
+May fail if boths extensions and filters add joins or both use the QueryNameGenerator
+because the workaround for [Issue 10](https://github.com/metaclass-nl/filter-bundle/issues/10)
+can not reconize wich ones come from the extensions and not correctly initialize the QueryNameGenerator.
+
You are advised to check the code of all custom and third party Filters and
not to combine those that use QueryBuilder::where or ::add with FilterLogic
or that produce complex logic that is not semantically complete. For an
diff --git a/src/Filter/FilterLogic.php b/src/Filter/FilterLogic.php
index a7f3d66..028ef42 100644
--- a/src/Filter/FilterLogic.php
+++ b/src/Filter/FilterLogic.php
@@ -6,6 +6,7 @@
use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\AbstractContextAwareFilter;
use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\ContextAwareFilterInterface;
use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\OrderFilter;
+use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGenerator;
use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface;
use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
use ApiPlatform\Core\Exception\ResourceClassNotFoundException;
@@ -77,32 +78,79 @@ public function apply(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $q
$this->filters = $this->getFilters($resourceClass, $operationName);
+
+ // Issue #10 workaround, tries to AND with criteria from extensions
+ $existingWhere = (string) $queryBuilder->getDQLPart('where');
+
+ $newQb = new QueryBuilder($queryBuilder->getEntityManager());
+ $newQb->add('select', $queryBuilder->getDQLPart('select'));
+ $newQb->add('from', $queryBuilder->getDQLPart('from'));
+ $newQng = new QueryNameGenerator();
+ // Problem: too hard to add the joins from the extensions and correctly initialize the QueryNameGenerator
+ // Workaround may fail if extensions did any joins and filters also, or if both use the QueryNameGenerator
+
+ $filters = $this->getFilters($resourceClass, $operationName, true);
+ foreach ($filters as $filter) {
+ $filter->apply($newQb, $newQng, $resourceClass, $operationName, $context);
+ }
+ $filterWhere = (string) $newQb->getDQLPart('where');
+
+ $logicExp = new Expr\Andx();
+ $combinator = 'AND';
if (isset($context['filters']['and']) ) {
$expressions = $this->filterProperty('and', $context['filters']['and'], $queryBuilder, $queryNameGenerator, $resourceClass, $operationName, $context);
- foreach($expressions as $exp) {
- $queryBuilder->andWhere($exp);
- };
+ //var_export($expressions);
+ $logicExp->addMultiple($expressions);
}
if (isset($context['filters']['not']) ) {
// NOT expressions are combined by parent logic, here defaulted to AND
$expressions = $this->filterProperty('not', $context['filters']['not'], $queryBuilder, $queryNameGenerator, $resourceClass, $operationName, $context);
foreach($expressions as $exp) {
- $queryBuilder->andWhere(new Expr\Func('NOT', [$exp]));
+ $logicExp->add(new Expr\Func('NOT', [$exp]));
};
}
if (isset($context['filters']['or'])) {
- $expressions = $this->filterProperty('or', $context['filters']['or'], $queryBuilder, $queryNameGenerator, $resourceClass, $operationName, $context);
- foreach($expressions as $exp) {
- $queryBuilder->orWhere($exp);
- };
+ $orExpressions = $this->filterProperty('or', $context['filters']['or'], $queryBuilder, $queryNameGenerator, $resourceClass, $operationName, $context);
+ if (!empty($orExpressions)) {
+ $logicExp = new Expr\Orx([$logicExp]);
+ $logicExp->addMultiple($orExpressions);
+ $combinator = 'OR';
+ }
}
if ($this->innerJoinsLeft) {
$this->replaceInnerJoinsByLeftJoins($queryBuilder);
}
+
+ // if $existingWhere empty no problem
+ // if $filterWhere empty nest OR in an extra AND
+ if (empty($existingWhere) || empty($filterWhere) ) {
+ $queryBuilder->andWhere($logicExp);
+ return;
+ }
+ // elseif only criteria from filters, apply according to operator
+ if ($existingWhere == $filterWhere) {
+ if ($combinator == 'OR') {
+ $queryBuilder->orWhere($logicExp);
+ } else {
+ $queryBuilder->andWhere($logicExp);
+ }
+ return;
+ }
+ // elseif criteria from filters follow AND, replace them
+ if(false!==strpos($existingWhere, " AND $filterWhere")) {
+ $queryBuilder->add('where',
+ str_replace($filterWhere, "($filterWhere $combinator ($logicExp))", $existingWhere)
+ );
+ return;
+ }
+
+ // Could not replace criteria from filters, probably an extension used the QueryNameGenerator
+ //throw new \RuntimeException("Could not replace '$filterWhere' in '$existingWhere'");
+ throw new \RuntimeException("Could not replace criteria from filters");
}
- /**
+ /**
* @return array of Doctrine\ORM\Query\Expr\* and/or string (DQL),
* each of which must be self-contained in the sense that the intended
* logic is not compromised if it is combined with the others and other
@@ -249,10 +297,11 @@ private function getAppliedExpressions($where, $marker)
/**
* @param string $resourceClass
* @param string $operationName
+ * @param bool $ignoreClassExp
* @return ContextAwareFilterInterface[] From resource except $this and OrderFilters
* @throws ResourceClassNotFoundException
*/
- protected function getFilters($resourceClass, $operationName)
+ protected function getFilters($resourceClass, $operationName, $ignoreClassExp=false)
{
$resourceMetadata = $this->resourceMetadataFactory->create($resourceClass);
$resourceFilters = $resourceMetadata->getCollectionOperationAttribute($operationName, 'filters', [], true);
@@ -265,7 +314,7 @@ protected function getFilters($resourceClass, $operationName)
if ($filter instanceof ContextAwareFilterInterface
&& !($filter instanceof OrderFilter)
&& $filter !== $this
- && preg_match($this->classExp, get_class($filter))
+ && ($ignoreClassExp || preg_match($this->classExp, get_class($filter)))
) {
$result[$filterId] = $filter;
}
diff --git a/tests/Filter/FilterLogicWithAnnotationTest.php b/tests/Filter/FilterLogicWithAnnotationTest.php
index a6e63dc..4b19f26 100644
--- a/tests/Filter/FilterLogicWithAnnotationTest.php
+++ b/tests/Filter/FilterLogicWithAnnotationTest.php
@@ -56,10 +56,10 @@ public function setUp(): void
self::assertNotNull($this->filterLogic, "this->filterLogic");
}
- public function testDdFilter()
+ public function testDdFilterAnd()
{
$reqData = null;
- parse_str('&and[or][dd][after]=2021-01-01&and[or][exists][bool]=true', $reqData);
+ parse_str('exists[bool]=true&and[or][dd][after]=2021-01-01', $reqData);
// var_dump($reqData);
$context = ['filters' => $reqData];
foreach ($this->filters as $filter) {
@@ -69,8 +69,8 @@ public function testDdFilter()
$this->assertEquals(
str_replace('
', '', "SELECT o FROM Metaclass\FilterBundle\Entity\TestEntity o WHERE
-(o.dd >= :dd_p1 OR o.dd IS NULL)
-OR o.bool IS NOT NULL"),
+o.bool IS NOT NULL
+AND (o.dd >= :dd_p1 OR o.dd IS NULL)"),
$this->testEntityQb->getDQL(),
'DQL');
$this->assertEquals(
@@ -80,6 +80,143 @@ public function testDdFilter()
}
+ public function testDdFilterNot()
+ {
+ $reqData = null;
+ parse_str('exists[bool]=true¬[dd][after]=2021-01-01', $reqData);
+ // var_dump($reqData);
+ $context = ['filters' => $reqData];
+ foreach ($this->filters as $filter) {
+ $filter->apply($this->testEntityQb, $this->queryNameGen, TestEntity::class, 'get', $context);
+ }
+
+ $this->assertEquals(
+ str_replace('
+', '', "SELECT o FROM Metaclass\FilterBundle\Entity\TestEntity o WHERE
+o.bool IS NOT NULL
+AND (NOT(o.dd >= :dd_p1 OR o.dd IS NULL))"),
+ $this->testEntityQb->getDQL(),
+ 'DQL');
+ $this->assertEquals(
+ '2021-01-01',
+ $this->testEntityQb->getParameter('dd_p1')->getValue()->format('Y-m-d'),
+ 'Parameter dd_p1');
+ }
+
+ public function testDdFilterOr()
+ {
+ $reqData = null;
+ parse_str('exists[bool]=true&or[dd][after]=2021-01-01&or[dd][before]=2010-02-02', $reqData);
+ // var_dump($reqData);
+ $context = ['filters' => $reqData];
+ foreach ($this->filters as $filter) {
+ $filter->apply($this->testEntityQb, $this->queryNameGen, TestEntity::class, 'get', $context);
+ }
+
+ $this->assertEquals(
+ str_replace('
+', '', "SELECT o FROM Metaclass\FilterBundle\Entity\TestEntity o WHERE
+o.bool IS NOT NULL
+OR (
+(o.dd <= :dd_p1 AND o.dd IS NOT NULL)
+OR (o.dd >= :dd_p2 OR o.dd IS NULL)
+)"),
+ $this->testEntityQb->getDQL(),
+ 'DQL');
+ $this->assertEquals(
+ '2010-02-02',
+ $this->testEntityQb->getParameter('dd_p1')->getValue()->format('Y-m-d'),
+ 'Parameter dd_p1');
+ $this->assertEquals(
+ '2021-01-01',
+ $this->testEntityQb->getParameter('dd_p2')->getValue()->format('Y-m-d'),
+ 'Parameter dd_p2');
+ }
+
+ public function testDdFilterAndWithExtsionCriterium()
+ {
+ $this->testEntityQb->andWhere('o.numb >= 0');
+ $reqData = null;
+ parse_str('exists[bool]=true&and[or][dd][after]=2021-01-01', $reqData);
+ // var_dump($reqData);
+ $context = ['filters' => $reqData];
+ foreach ($this->filters as $filter) {
+ $filter->apply($this->testEntityQb, $this->queryNameGen, TestEntity::class, 'get', $context);
+ }
+
+ $this->assertEquals(
+ str_replace('
+', '', "SELECT o FROM Metaclass\FilterBundle\Entity\TestEntity o WHERE
+o.numb >= 0 AND (
+o.bool IS NOT NULL
+AND (o.dd >= :dd_p1 OR o.dd IS NULL)
+)"),
+ $this->testEntityQb->getDQL(),
+ 'DQL');
+ $this->assertEquals(
+ '2021-01-01',
+ $this->testEntityQb->getParameter('dd_p1')->getValue()->format('Y-m-d'),
+ 'Parameter dd_p1');
+
+ }
+
+ public function testDdFilterNotWithExtsionCriterium()
+ {
+ $this->testEntityQb->andWhere('o.numb >= 0');
+ $reqData = null;
+ parse_str('exists[bool]=true¬[dd][after]=2021-01-01', $reqData);
+ // var_dump($reqData);
+ $context = ['filters' => $reqData];
+ foreach ($this->filters as $filter) {
+ $filter->apply($this->testEntityQb, $this->queryNameGen, TestEntity::class, 'get', $context);
+ }
+
+ $this->assertEquals(
+ str_replace('
+', '', "SELECT o FROM Metaclass\FilterBundle\Entity\TestEntity o WHERE
+o.numb >= 0 AND (
+o.bool IS NOT NULL
+AND (NOT(o.dd >= :dd_p1 OR o.dd IS NULL)))"),
+ $this->testEntityQb->getDQL(),
+ 'DQL');
+ $this->assertEquals(
+ '2021-01-01',
+ $this->testEntityQb->getParameter('dd_p1')->getValue()->format('Y-m-d'),
+ 'Parameter dd_p1');
+ }
+
+ public function testDdFilterOrWithExtsionCriterium()
+ {
+ $this->testEntityQb->andWhere('o.numb >= 0');
+ $reqData = null;
+ parse_str('exists[bool]=true&or[dd][after]=2021-01-01&or[dd][before]=2010-02-02', $reqData);
+ // var_dump($reqData);
+ $context = ['filters' => $reqData];
+ foreach ($this->filters as $filter) {
+ $filter->apply($this->testEntityQb, $this->queryNameGen, TestEntity::class, 'get', $context);
+ }
+
+ $this->assertEquals(
+ str_replace('
+', '', "SELECT o FROM Metaclass\FilterBundle\Entity\TestEntity o WHERE
+o.numb >= 0 AND (
+o.bool IS NOT NULL
+OR (
+(o.dd <= :dd_p1 AND o.dd IS NOT NULL)
+OR (o.dd >= :dd_p2 OR o.dd IS NULL)
+))"),
+ $this->testEntityQb->getDQL(),
+ 'DQL');
+ $this->assertEquals(
+ '2010-02-02',
+ $this->testEntityQb->getParameter('dd_p1')->getValue()->format('Y-m-d'),
+ 'Parameter dd_p1');
+ $this->assertEquals(
+ '2021-01-01',
+ $this->testEntityQb->getParameter('dd_p2')->getValue()->format('Y-m-d'),
+ 'Parameter dd_p2');
+ }
+
public function testRexExp()
{
$regExp = '/'. str_replace('\\', '\\\\', DateFilter::class). '/';