bug #257 Fix clients filtering when using doctrine persistence (ajgarlag)

This PR was merged into the 1.x-dev branch.

Discussion
----------

Fix clients filtering when using `doctrine` persistence

It was using an 'IN' clause in fields stored as a space separated
 string of values.

Commits
-------

2ce4f54 Fix clients filtering when using `doctrine` persistence

Author: chalasr

src/Manager/Doctrine/ClientManager.php

@@ -5,6 +5,7 @@
 namespace League\Bundle\OAuth2ServerBundle\Manager\Doctrine;
 
 use Doctrine\ORM\EntityManagerInterface;
+use Doctrine\ORM\QueryBuilder;
 use League\Bundle\OAuth2ServerBundle\Event\PreSaveClientEvent;
 use League\Bundle\OAuth2ServerBundle\Manager\ClientFilter;
 use League\Bundle\OAuth2ServerBundle\Manager\ClientManagerInterface;
@@ -74,38 +75,45 @@ public function remove(ClientInterface $client): void
     public function list(?ClientFilter $clientFilter): array
     {
         $repository = $this->entityManager->getRepository($this->clientFqcn);
-        $criteria = self::filterToCriteria($clientFilter);
+        $qb = $repository->createQueryBuilder('c');
+        self::setQueryBuilderFilters($qb, $clientFilter);
 
         /** @var list<AbstractClient> */
-        return $repository->findBy($criteria);
+        return $qb->getQuery()->getResult();
     }
 
-    /**
-     * @return array{grants?: list<Grant>, redirect_uris?: list<RedirectUri>, scopes?: list<Scope>}
-     */
-    private static function filterToCriteria(?ClientFilter $clientFilter): array
+    private static function setQueryBuilderFilters(QueryBuilder $qb, ?ClientFilter $clientFilter): void
     {
         if (null === $clientFilter || false === $clientFilter->hasFilters()) {
-            return [];
+            return;
         }
 
-        $criteria = [];
+        self::setFieldFilter($qb, 'grants', $clientFilter->getGrants());
 
-        $grants = $clientFilter->getGrants();
-        if ($grants) {
-            $criteria['grants'] = $grants;
-        }
+        self::setFieldFilter($qb, 'redirect_uris', $clientFilter->getRedirectUris());
 
-        $redirectUris = $clientFilter->getRedirectUris();
-        if ($redirectUris) {
-            $criteria['redirect_uris'] = $redirectUris;
-        }
+        self::setFieldFilter($qb, 'scopes', $clientFilter->getScopes());
+    }
 
-        $scopes = $clientFilter->getScopes();
-        if ($scopes) {
-            $criteria['scopes'] = $scopes;
+    /**
+     * @param list<Scope>|list<RedirectUri>|list<Grant> $values
+     */
+    private static function setFieldFilter(QueryBuilder $qb, string $field, array $values): void
+    {
+        foreach ($values as $key => $value) {
+            $qb->andWhere(
+                $qb->expr()->orX(
+                    $qb->expr()->eq('c.' . $field, ':' . $field . $key),
+                    $qb->expr()->like('c.' . $field, ':space_' . $field . $key),
+                    $qb->expr()->like('c.' . $field, ':' . $field . '_space' . $key),
+                    $qb->expr()->like('c.' . $field, ':space_' . $field . '_space' . $key),
+                )
+            )
+                ->setParameter($field . $key, (string) $value)
+                ->setParameter('space_' . $field . $key, '% ' . (string) $value)
+                ->setParameter($field . '_space' . $key, (string) $value . ' %')
+                ->setParameter('space_' . $field . '_space' . $key, '% ' . (string) $value . ' %')
+            ;
         }
-
-        return $criteria;
     }
 }

tests/Acceptance/ListClientsCommandTest.php

@@ -91,20 +91,27 @@ public function testListClientColumns(): void
 
     public function testListFiltersClients(): void
     {
-        $clientA = $this->fakeAClient('client', 'client-a', 'client-a-secret');
+        $clientA = $this->fakeAClient('client', 'client-a', 'client-a-secret')
+            ->setScopes(new Scope('client-a-scope'), new Scope('other-scope-1'));
         $this->getClientManager()->save($clientA);
 
         $clientB = $this
             ->fakeAClient('client', 'client-b', 'client-b-secret')
-            ->setScopes(new Scope('client-b-scope'))
+            ->setScopes(new Scope('client-b-scope'), new Scope('other-scope'))
         ;
         $this->getClientManager()->save($clientB);
 
+        $clientC = $this
+            ->fakeAClient('client', 'client-c', 'client-c-secret')
+            ->setScopes(new Scope('client-c-scope'), new Scope('1-other-scope'))
+        ;
+        $this->getClientManager()->save($clientC);
+
         $command = $this->command();
         $commandTester = new CommandTester($command);
         $commandTester->execute([
             'command' => $command->getName(),
-            '--scope' => $clientB->getScopes(),
+            '--scope' => ['other-scope'],
         ]);
         $output = $commandTester->getDisplay();
 

tests/Acceptance/resource/list-filters-clients.txt

@@ -1,6 +1,6 @@
 
- -------- ------------ ----------------- ---------------- -------------- ------------ 
-  name     identifier   secret            scope            redirect uri   grant type  
- -------- ------------ ----------------- ---------------- -------------- ------------ 
-  client   client-b     client-b-secret   client-b-scope                              
- -------- ------------ ----------------- ---------------- -------------- ------------ 
+ -------- ------------ ----------------- ----------------------------- -------------- ------------ 
+  name     identifier   secret            scope                         redirect uri   grant type  
+ -------- ------------ ----------------- ----------------------------- -------------- ------------ 
+  client   client-b     client-b-secret   client-b-scope, other-scope                              
+ -------- ------------ ----------------- ----------------------------- -------------- ------------