Skip to content

Commit 63e46c6

Browse files
committed
bug #7548 Add more security protection to autocomplete action (javiereguiluz)
This PR was squashed before being merged into the 4.x branch. Discussion ---------- Add more security protection to autocomplete action The `index`, `detail`, `edit`, `new`, `delete`, and `batchDelete` actions all call ```php $this->isGranted(Permission::EA_EXECUTE_ACTION, ['action' => Action::…, …]); ``` The `autocomplete` action does **not**. A low-privileged admin user can call the autocomplete endpoint for any CRUD controller FQCN and enumerate related entities (IDs + `__toString()`) they should not be able to see. Commits ------- 7e3a9bb Add more security protection to autocomplete action
2 parents d31b538 + 7e3a9bb commit 63e46c6

3 files changed

Lines changed: 83 additions & 0 deletions

File tree

src/Controller/AbstractCrudController.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,11 @@ public function batchDelete(AdminContext $context, BatchActionDto $batchActionDt
494494

495495
public function autocomplete(AdminContext $context): JsonResponse
496496
{
497+
// not a typo; we intentionally reuse the INDEX action for permission checks in autocomplete
498+
if (!$this->isGranted(Permission::EA_EXECUTE_ACTION, ['action' => Action::INDEX, 'entity' => null, 'entityFqcn' => $context->getEntity()->getFqcn()])) {
499+
throw new ForbiddenActionException($context);
500+
}
501+
497502
$queryBuilder = $this->createIndexQueryBuilder($context->getSearch(), $context->getEntity(), new FieldCollection([]), new FilterCollection());
498503

499504
$autocompleteContext = $context->getRequest()->query->all(AssociationField::PARAM_AUTOCOMPLETE_CONTEXT);
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<?php
2+
3+
namespace EasyCorp\Bundle\EasyAdminBundle\Tests\Functional\Apps\SecuredApp\Controller;
4+
5+
use EasyCorp\Bundle\EasyAdminBundle\Config\Action;
6+
use EasyCorp\Bundle\EasyAdminBundle\Config\Actions;
7+
use EasyCorp\Bundle\EasyAdminBundle\Controller\AbstractCrudController;
8+
use EasyCorp\Bundle\EasyAdminBundle\Field\IdField;
9+
use EasyCorp\Bundle\EasyAdminBundle\Field\TextField;
10+
use EasyCorp\Bundle\EasyAdminBundle\Tests\Functional\Apps\DefaultApp\Entity\Category;
11+
12+
/**
13+
* Used by RolePermissionTest to exercise the INDEX-based permission check
14+
* inside AbstractCrudController::autocomplete().
15+
*
16+
* @extends AbstractCrudController<Category>
17+
*/
18+
class ProtectedCategoryCrudController extends AbstractCrudController
19+
{
20+
public static function getEntityFqcn(): string
21+
{
22+
return Category::class;
23+
}
24+
25+
public function configureFields(string $pageName): iterable
26+
{
27+
return [
28+
IdField::new('id')->hideOnForm(),
29+
TextField::new('name'),
30+
];
31+
}
32+
33+
public function configureActions(Actions $actions): Actions
34+
{
35+
return $actions->setPermission(Action::INDEX, 'ROLE_ADMIN');
36+
}
37+
}

tests/Functional/Security/RolePermissionTest.php

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use EasyCorp\Bundle\EasyAdminBundle\Exception\ForbiddenActionException;
66
use EasyCorp\Bundle\EasyAdminBundle\Test\AbstractCrudTestCase;
77
use EasyCorp\Bundle\EasyAdminBundle\Tests\Functional\Apps\SecuredApp\Controller\CategoryCrudController;
8+
use EasyCorp\Bundle\EasyAdminBundle\Tests\Functional\Apps\SecuredApp\Controller\ProtectedCategoryCrudController;
89
use EasyCorp\Bundle\EasyAdminBundle\Tests\Functional\Apps\SecuredApp\Controller\SecuredDashboardController;
910
use EasyCorp\Bundle\EasyAdminBundle\Tests\Functional\Apps\SecuredApp\Kernel;
1011
use Symfony\Component\HttpFoundation\Response;
@@ -129,4 +130,44 @@ public static function provideRolesForFieldPermission(): \Generator
129130
yield 'admin cannot see protected field' => ['admin', false];
130131
yield 'super_admin can see protected field' => ['super_admin', true];
131132
}
133+
134+
/**
135+
* @dataProvider provideRolesForAutocompleteAction
136+
*/
137+
public function testAutocompleteActionPermission(string $username, int $expectedStatusCode): void
138+
{
139+
if (Response::HTTP_FORBIDDEN === $expectedStatusCode) {
140+
$this->expectException(ForbiddenActionException::class);
141+
$this->client->catchExceptions(false);
142+
}
143+
144+
$autocompleteUrl = $this->getCrudUrl(
145+
'autocomplete',
146+
null,
147+
[
148+
'autocompleteContext[crudControllerFqcn]' => ProtectedCategoryCrudController::class,
149+
'autocompleteContext[propertyName]' => 'name',
150+
'autocompleteContext[originatingPage]' => 'new',
151+
],
152+
SecuredDashboardController::class,
153+
ProtectedCategoryCrudController::class,
154+
);
155+
156+
$this->client->request(
157+
'GET',
158+
$autocompleteUrl,
159+
[],
160+
[],
161+
['PHP_AUTH_USER' => $username, 'PHP_AUTH_PW' => '1234']
162+
);
163+
164+
static::assertResponseStatusCodeSame($expectedStatusCode);
165+
}
166+
167+
public static function provideRolesForAutocompleteAction(): \Generator
168+
{
169+
yield 'user role cannot access autocomplete without INDEX permission' => ['user', Response::HTTP_FORBIDDEN];
170+
yield 'admin role can access autocomplete with INDEX permission' => ['admin', Response::HTTP_OK];
171+
yield 'super_admin role can access autocomplete with INDEX permission' => ['super_admin', Response::HTTP_OK];
172+
}
132173
}

0 commit comments

Comments
 (0)