Problem/Motivation

Users with access to edit a queue cannot save a queue if there are entities in the queue that they do not have access to.

Steps to reproduce

  1. Add a published node to the queue
  2. edit the node, and unpublished it
  3. try to change the order of the queue and hit save
  4. you'll get an error-message about an invalid entity, you'll have to remove the node from the queue

Proposed resolution

  • Allow reordering entities that already exist in the queue and the user does not have access to
  • Allow removing entities that already exist in the queue and the user does not have access to
  • Allow saving the queue
  • Does not display entity labels, uris or ids for entities the user does not have access to
  • Does not allow the user to add entities to the queue that they do not have access to

Original report by stmh

Hi,

we are facing a problem, where we can't save an entityqueue when there are unpublished nodes in the queue. Also when the current user has no permission on one of the nodes saving the queue will fail with the message "invalid entity".

Here is a simple scenario to reproduce the node.status problem:

1. Add a published node to the queue
2. edit the node, and unpublished it
3. try to change the order of the queue and hit save.
4. you'll get an error-message about an invalid entity, you'll have to remove the node from the queue.

This behavior renders entity queue unusable for automatic content scheduling which is based on the status-flag.

So what is the expected behavior of entity queue? Should entity queue allow unpublished nodes and nodes where the current user has no permissions for? I think yes.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

stmh’s picture

Here's a patch with a working solution.

muschpusch’s picture

Version: 7.x-1.0-alpha3 » 7.x-1.x-dev
Priority: Normal » Major

Moving this to major since entityqueue is not usable for users without the 'administer content' or similar permission. The patch also applies to current dev. Maybe this should be handled upstream in entityreference but it looks like they had a reason for putting the extra permission check.

Another simple way of reproducing this is to create a queue with a published and an unpublished node. Users without access to the unpublished node will not be able to save the queue.

jojonaloha’s picture

Title: Ignore node_access on node and node.status for nodes in a queue. » Cannot save queues with entities user doesn't have access to
Issue summary: View changes

While I understand the problem, I also know that ignoring node_access would be a security issue, so the current patch is not a good solution.

I'm also guessing this is probably a result of queues being Entityreference fields.

Given that, I feel this could be marked as "works as designed", but instead I'm changing the title. I feel the goals of a patch would be to:

  • Allow reordering entities that already exist in the queue and the user does not have access to
  • Allow removing entities that already exist in the queue and the user does not have access to
  • Allow saving the queue
  • Does not display entity labels, uris or ids for entities the user does not have access to
  • Does not allow the user to add entities to the queue that they do not have access to
nerdacus’s picture

In review of jojonaloha's list against the patch proposed in #1:

  • Allow reordering entities that already exist in the queue and the user does not have access to
    • Passes
  • Allow removing entities that already exist in the queue and the user does not have access to
    • Passes
  • Allow saving the queue
    • Passes
  • Does not display entity labels, uris or ids for entities the user does not have access to
  • Does not allow the user to add entities to the queue that they do not have access to
    • Does not pass. Although autocomplete does not reveal the restricted entity, I can still type in gibberish assuming entity IDs and force the addition of something I should not be able to access
amateescu’s picture

For that last point, that's a problem of the Entity reference field and should be fixed there :)

jojonaloha’s picture

Status: Active » Needs review
FileSize
4.54 KB

Updated patch should pass all the criteria I listed in #3

I was hoping that all was needed was the changes to the EntityReference Selection Handler, but it turns out if you specify an entity id directly in the autocomplete, like Marcus mentioned, it doesn't validate it before adding it to the list. You can even add an item that isn't of the correct bundle.

spesic’s picture

Applied #6 against 7.x-1.x

- Allow reordering entities that already exist in the queue and the user does not have access to:
Passes
- Allow removing entities that already exist in the queue and the user does not have access to
Passes
- Allow saving the queue
Passes
- Does not display entity labels, uris or ids for entities the user does not have access to
Passes
- Does not allow the user to add entities to the queue that they do not have access to
Passes

spesic’s picture

Status: Needs review » Reviewed & tested by the community
nerdacus’s picture

Status: Reviewed & tested by the community » Needs work

There is still an issue with adding a new item where validation claims that I do not have the required number of item in the subqueue.

jojonaloha’s picture

Status: Needs work » Needs review
FileSize
2.53 KB
2.05 KB

Ok, looks like adding the validation to the "Add item" button was a bad idea since it also validates the size of the queue. I'll move that to a separate issue.

  • jojonaloha committed b2417e2 on 7.x-1.x
    Issue #2383903 by jojonaloha, stmh: Cannot save queues with entities...
jojonaloha’s picture

Committed and created #2489462: Can add items of wrong bundle to queue for the other issue.

jojonaloha’s picture

Status: Needs review » Fixed

  • jojonaloha committed b2417e2 on 8.x-1.x
    Issue #2383903 by jojonaloha, stmh: Cannot save queues with entities...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

liquidcms’s picture

Status: Closed (fixed) » Needs work

sorry to re-open.. possibly i misunderstand the point of this thread?

i have rev 7.x-1.1, which was released after this was marked as closed.

a user (without administer users access) has access to edit a queue and sees this: http://screencast.com/t/IZ2vCgVOR, is this not the point of this fix?

ideally, for my case i would not only like to see blocked users that are already listed in queue and either remove them of shuffle them (which i think is stated to work here) but i would also like to add new blocked users (which i suspect is not allowed by this fix).

jojonaloha’s picture

Status: Needs work » Closed (fixed)

If I understand correctly, you have an Entityqueue of users. When a user with permission to edit that queue views it, they do not see the usernames, they only see "- Restricted access -".

This is actually by design, as otherwise it was a security issue, which was fixed in #2442875: Displays labels of entities user doesn't have access to. In the case of users, the access callback from the Entity module is entity_metadata_user_access(); , and matches the page callback logic in user_view_access(); . It looks like the view operation is tied to the "access user profiles" permission, so you should be able to grant that to the users who can edit this queue. As for also allowing them to view blocked users, I think you would have to create a custom access callback to allow those users without the "administer users" permission to view blocked usernames. I think that makes sense because you would likely need to do that anyway wherever you are displaying the queue (likely in a View).

You might also want to check out #2329979: Allow blocked users in an entity reference field or maybe Login Disable as an alternative to blocking users?

liquidcms’s picture

but in the autocomplete field to add users to the queue i can still see their names there; so i am at a loss to see what the security is that is being added here.