Skip to content

Commit

Permalink
bug #6164 Fix ajax editing of fields with invalid permission (danut00…
Browse files Browse the repository at this point in the history
…7ro)

This PR was squashed before being merged into the 4.x branch.

Discussion
----------

Fix ajax editing of fields with invalid permission

When editing a boolean field using ajax from index page it is not checked if it's valid.

The field is now checked in the FieldCollection and an `AccessDeniedException` is thrown if field is not found.

I did this PR with help from `@psihius`

LE : I also added a check to disallow editing if the field is disabled.

Commits
-------

b90a94c Fix ajax editing of fields with invalid permission
  • Loading branch information
javiereguiluz committed Feb 29, 2024
2 parents ae1057d + b90a94c commit 4b6a90d
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 3 deletions.
6 changes: 6 additions & 0 deletions src/Controller/AbstractCrudController.php
Expand Up @@ -59,6 +59,7 @@
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
use Symfony\Component\HttpKernel\Exception\MethodNotAllowedHttpException;
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
use Symfony\Component\Security\Core\Exception\InvalidCsrfTokenException;
use function Symfony\Component\String\u;

Expand Down Expand Up @@ -555,6 +556,11 @@ protected function getContext(): ?AdminContext

protected function ajaxEdit(EntityDto $entityDto, ?string $propertyName, bool $newValue): AfterCrudActionEvent
{
$field = $entityDto->getFields()->getByProperty($propertyName);
if (null === $field || true === $field->getFormTypeOption('disabled')) {
throw new AccessDeniedException(sprintf('The field "%s" is not allowed to be modified.', $propertyName));
}

$this->container->get(EntityUpdater::class)->updateProperty($entityDto, $propertyName, $newValue);

$event = new BeforeEntityUpdatedEvent($entityDto->getInstance());
Expand Down
35 changes: 32 additions & 3 deletions tests/Controller/CategoryCrudControllerTest.php
Expand Up @@ -11,6 +11,7 @@
use EasyCorp\Bundle\EasyAdminBundle\Tests\TestApplication\Controller\SecureDashboardController;
use EasyCorp\Bundle\EasyAdminBundle\Tests\TestApplication\Entity\Category;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
use Symfony\Component\HttpKernel\Exception\MethodNotAllowedHttpException;

class CategoryCrudControllerTest extends AbstractCrudTestCase
Expand Down Expand Up @@ -187,12 +188,18 @@ public function testDetail()
/**
* @dataProvider toggle
*/
public function testToggle(string $method, ?string $invalidCsrfToken, int $expectedStatusCode, bool $toggleIsExpectedToSucceed)
public function testToggle(string $method, ?string $invalidCsrfToken, ?string $fieldName, int $expectedStatusCode, bool $toggleIsExpectedToSucceed)
{
if (Response::HTTP_METHOD_NOT_ALLOWED === $expectedStatusCode) {
$expectedExceptionClass = match ($expectedStatusCode) {
Response::HTTP_METHOD_NOT_ALLOWED => MethodNotAllowedHttpException::class,
Response::HTTP_BAD_REQUEST => BadRequestHttpException::class,
default => null,
};

if (null !== $expectedExceptionClass) {
// needed to not display 'Uncaught PHP exception' messages in PHPUnit output
// see https://stackoverflow.com/questions/50456114/phpunit-dont-report-symfony-exceptions-rendered-to-http-errors/50465691
$this->expectException(MethodNotAllowedHttpException::class);
$this->expectException($expectedExceptionClass);
$this->client->catchExceptions(false);
}

Expand All @@ -214,6 +221,11 @@ public function testToggle(string $method, ?string $invalidCsrfToken, int $expec
$firstFoundToggleUrl = preg_replace('/csrfToken=.+?&/', sprintf('csrfToken=%s&', $invalidCsrfToken), $firstFoundToggleUrl);
}

// Change the field name
if (null !== $fieldName) {
$firstFoundToggleUrl = preg_replace('/fieldName=.+?&/', sprintf('fieldName=%s&', $fieldName), $firstFoundToggleUrl);
}

// Do the AJAX request
$this->client->request($method, $firstFoundToggleUrl, [], [], [
'HTTP_x-requested-with' => 'XMLHttpRequest',
Expand Down Expand Up @@ -286,21 +298,38 @@ public static function toggle(): \Generator
yield [
'GET', // HTTP method
null, // Do not manipulate the CSRF token
null, // Do not manipulate the field name
Response::HTTP_METHOD_NOT_ALLOWED, // Response status code, fails because of wrong method "GET"
false, // Should the toggle successfully change the toggled property?
];
yield [
'PATCH',
'123abc', // Manipulate the CSRF token to this invalid value
null, // Do not manipulate the field name
Response::HTTP_UNAUTHORIZED, // Response status code, fails because of wrong CSRF token
false,
];
yield [
'PATCH',
null, // Do not manipulate the CSRF token
null, // Do not manipulate the field name
Response::HTTP_OK,
true,
];
yield [
'PATCH',
null, // Do not manipulate the CSRF token
'activeWithNoPermission', // Manipulate the field name to field with invalid permission
Response::HTTP_BAD_REQUEST,
false,
];
yield [
'PATCH',
null, // Do not manipulate the CSRF token
'activeDisabled', // Manipulate the field name to disabled field
Response::HTTP_BAD_REQUEST,
false,
];
}

/**
Expand Down
Expand Up @@ -40,6 +40,8 @@ public function configureFields(string $pageName): iterable
TextField::new('name'),
TextField::new('slug'),
BooleanField::new('active'),
BooleanField::new('activeWithNoPermission')->setPermission('ROLE_FOO'),
BooleanField::new('activeDisabled')->setDisabled(),
];
}

Expand Down
15 changes: 15 additions & 0 deletions tests/TestApplication/src/Entity/Category.php
Expand Up @@ -28,6 +28,9 @@ class Category
#[ORM\Column(type: 'boolean')]
private bool $active = false;

#[ORM\Column(type: 'boolean')]
private bool $activeDisabled = false;

public function __construct()
{
$this->blogPosts = new ArrayCollection();
Expand Down Expand Up @@ -100,4 +103,16 @@ public function setActive(bool $active): self

return $this;
}

public function isActiveDisabled(): bool
{
return $this->activeDisabled;
}

public function setActiveDisabled(bool $activeDisabled): self
{
$this->activeDisabled = $activeDisabled;

return $this;
}
}

0 comments on commit 4b6a90d

Please sign in to comment.