EasyAdminBundle icon indicating copy to clipboard operation
EasyAdminBundle copied to clipboard

Batch action - delete - permissions

Open wontroba666 opened this issue 3 years ago • 6 comments

hello, is it possible to set permissions in the case of batchActions DELETE - action-batchDelete? the user does not have permission to the DELETE action but can delete an entity via batchAction- how to protect it?

wontroba666 avatar Feb 25 '22 11:02 wontroba666

Hello,

Look this issue #5004

dwd-akira avatar Feb 25 '22 17:02 dwd-akira

Seems to me like #5004 is about allowing access to the delete batch action as a whole, not applying permission to each item being deleted by the batch action.

I am looking for a way to apply a batch action deleting all selected items but the ones current user might not have the rights to delete.

Is there a way to do such a thing? Or do we have to implement or own custom delete batch action maybe?

pacproduct avatar Jun 10 '22 09:06 pacproduct

Hi,

Maybe, you can do something like this

public function batchDelete(AdminContext $context, BatchActionDto $batchActionDto): Response
{
	// you modify $batchActionDto and delete the id in $batchActionDto->entityIds

	return parent::batchDelete($context, $batchActionDto);
}

dwd-akira avatar Jun 10 '22 14:06 dwd-akira

@dwd-akira Sounds like exactly what I need, but it seems like it's not possible to alter a BatchActionDto object, so instead I created an updated one.

For info, here is the solution I implemented based on your idea:

    /**
     * Overrides the default batchDelete implementation so it applies
     * permissions.
     *
     * @param AdminContext   $context        EZA context.
     * @param BatchActionDto $batchActionDto Object containing info on the list
     *                                       of objects being deleted.
     *
     * @return Response
     */
    public function batchDelete(AdminContext $context, BatchActionDto $batchActionDto): Response
    {
        $repository = $this->entityManager->getRepository($batchActionDto->getEntityFqcn());

        // Filter entities so only the ones current user is authorized to delete
        // are actually deleted.
        $selectedEntityIds = $batchActionDto->getEntityIds();
        $deletableEntityIds = [];
        foreach ($selectedEntityIds as $selectedEntityId) {
            // Load current entity.
            $entityInstance = $repository->find($selectedEntityId);
            // Skip it if it was not found.
            if (!$entityInstance) {
                continue;
            }

            // Flag current entity's ID as deletable only if current user has
            // the right to do so.
            if ($this->isGranted(UserVoter::DELETE, $entityInstance)) {
                $deletableEntityIds[] = $selectedEntityId;
            }
        }

        // Create a new version of the BatchActionDto object with
        // our updated list of IDs that can be deleted.
        $batchActionDto = new BatchActionDto(
            $batchActionDto->getName(),
            $deletableEntityIds,
            $batchActionDto->getEntityFqcn(),
            $batchActionDto->getReferrerUrl(),
            $batchActionDto->getCsrfToken(),
        );

        // Run the batch delete action.
        return parent::batchDelete($context, $batchActionDto);
    }

Many thanks for your suggestion! :smiley:

I'm wondering if there is a reason why EZA does not include a way to test permissions (for each item) in batch operations though? It feels like it could be a configurable option.

pacproduct avatar Jun 15 '22 14:06 pacproduct

Overriding batchDelete the way I did does load entities twice which is not ideal, though.

Besides, I see that EZA already tests some kind of permissions for each entity in \EasyCorp\Bundle\EasyAdminBundle\Controller\AbstractCrudController::batchDelete:

            $entityDto = $context->getEntity()->newWithInstance($entityInstance);
            if (!$this->isGranted(Permission::EA_EXECUTE_ACTION, ['action' => Action::DELETE, 'entity' => $entityDto])) {
                throw new ForbiddenActionException($context);
            }

So I'm wondering if there is a way to interact with this permission test in a cleaner way? :thinking:

Also, I'm not sure what happens when a ForbiddenActionException() is thrown: is the whole batch operation cancelled, or only the forbidden entity skipped?

pacproduct avatar Jun 15 '22 15:06 pacproduct

The problem is $crudController->configureActions is not called for delete or batch_delete actions because $pageName is null https://github.com/EasyCorp/EasyAdminBundle/blob/6bd0489522b3620e1842147592c5cbca22594263/src/Factory/AdminContextFactory.php#L129-L138

gassan avatar Aug 04 '22 18:08 gassan