If a user that does not have permission to print an entity, but have access to the content overview page "admin/content" they can still see the download pdf. When they click on it an error is thrown saying: "You must pass at least entity".

to reproduce:
- enable entity_print
- make sure a user does not have permission "Access Printable version of View"
- go to admin/content page
- select a piece of content
- click on download as PDF.

screenshot

Expected result : " deny access"

Actual result: exception: "You must pass at least entity".
screen shot2

Possible cause
When access is denied $enteties are being passed as an empty array to renderer.
screenhot3

Proposed Solution
Check $entities array is not empty before sending the array to the renderer, patch attached.

Comments

awm created an issue. See original summary.

awm’s picture

StatusFileSize
new745 bytes
awm’s picture

Status: Active » Needs review
awm’s picture

StatusFileSize
new1.34 KB

The last submitted patch, 2: entity_print-2969357.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 4: entity_print-2969357.patch, failed testing. View results

awm’s picture

Issue summary: View changes
awm’s picture

Issue summary: View changes
awm’s picture

StatusFileSize
new70 bytes
new1.12 KB
awm’s picture

StatusFileSize
new2.01 KB
awm’s picture

Status: Needs work » Needs review
benjy’s picture

Normally we'd wrap access control around the button/action displayed in the UI. Is it possible to add the access control to the VBO action?

awm’s picture

This not using VBO. Just core views. With VBO module there is a submodule that does action permissions but that's something else. It perhaps possible to hide the button but I'm not sure where.

Hiding the button is also not enough as you could still access the route somehow and the exception get thrown where it should be access denied.

benjy’s picture

If you access the route directly then the access check should be done at the route level. Sorry I meant core views actions, I thought they supported access control also but i've not checked.

awm’s picture

I doubt they support that that's why they have action_permissions. It's similar to any other action such as edit, delete ..etc. they all are displayed despite lacking the permissions for the operations. If an action is performed by a user with no permission, for example, delete, a denial message is displayed without an error. I think entity_print should do the same.

benjy’s picture

I thought that behaviour was controlled by AccessInterface::access() but i'd have to look closer to say for sure.

+++ b/src/PrintBuilder.php
@@ -52,7 +53,12 @@ class PrintBuilder implements PrintBuilderInterface {
+    try {
+      $renderer = $this->prepareRenderer($entities, $print_engine, $use_default_css);
+    } catch (\InvalidArgumentException $e) {
+      watchdog_exception('entity_print', $e);
+      return FALSE;
+    }

The thing I don't like about the current approach is we are catching all \InvalidArgumentExceptions, which opens up to suppressing other potential failures.

awm’s picture

So entity_print implements access correctly and the user gets the message "No access to execute..." after the exception is thrown when the user refresh the page "admin/content". But for some reason the exception is thrown (either before or after), prevent normal flow. I will change the patch to not catch the exception.

awm’s picture

StatusFileSize
new1.84 KB

Status: Needs review » Needs work

The last submitted patch, 18: entity_print-2969357-8.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

awm’s picture

StatusFileSize
new1.46 KB

Here is what I found is going on between entity_print and views: (code commented)

       // The access is evaluated and entity_print correctly handles it therefore it passes this condition::
        if (!$action->getPlugin()->access($entity, $this->view->getUser())) { 
          // And correctly an access denial message error is displayed:
          $this->drupalSetMessage($this->t('No access to execute %action on the @entity_type_label %entity_label.', [
            '%action' => $action->label(),
            '@entity_type_label' => $entity->getEntityType()->getLabel(),
            '%entity_label' => $entity->label()
          ]), 'error');
          continue; 
        }

        $count++;
        // At this point entities remain empty.
        $entities[$bulk_form_key] = $entity;
      }
       // And eventually passed to deliverPrintable where an exception in thrown.
      $action->execute($entities); 

So unless views is doing something wrong, we need to check if entities are empties; which means access was denied.
updated the patch.

awm’s picture

Status: Needs work » Needs review
awm’s picture

although( entity_print-2969357-9.patch) patch passed my basic testing on my local environment , but on a live env with https, nginx, load balancer, it's causing blank screen. I am assuming because the returned response is malformed. Not sure what's the most appropriate response.

awm’s picture

StatusFileSize
new1.39 KB

Another attempt since it seems the that

return (new StreamedResponse(function () use ($entity, $print_engine, $config) {
      // The Print is sent straight to the browser.
      $this->printBuilder->deliverPrintable([$entity], $print_engine, $config->get('force_download'), $config->get('default_css'));
    }))->send();

Fails when deliverPrintable returns false on an environment with https, nginx and loadbalancer. here is some pics of the response header/preview:

1. https://i.imgur.com/CFNoLOb.png
2. https://i.imgur.com/fdeOzpi.png

benjy’s picture

it's causing blank screen

Is there anything in the nginx or balancer logs?

awm’s picture

Nothing that stood out as far as I looked but will dig more when i have time. But I am almost certain it has to do with StreamedResponse .
Perhaps streamedResponse should not be instantiated if there are no entities?

avpaderno’s picture

I am deleting a file as requested.

jannakha’s picture

Status: Needs review » Closed (outdated)
StatusFileSize
new142.54 KB

this looks like an outdated issue, tested on D11.2 Entity Print Version: 8.x-2.16
 8.x-2.16

avpaderno’s picture

Version: 8.x-2.0 » 8.x-2.x-dev