Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
- User A created a node with reference to node IDs 1, 2, 3
- User B edits node, but doesn't have access to nid 3
- Upon save reference to nid 3 is lost
With patch, we check if there are items user doesn't have access to, and re-add them on validate handler.
If this seems ok, I can adapt the OG tests and move the responsibility of it to ER ;)
Comment | File | Size | Author |
---|---|---|---|
#2 | add-hidden-ids-1.patch | 3.55 KB | amitaibu |
add-hidden-ids.patch | 3.55 KB | amitaibu | |
Comments
Comment #2
amitaibuCorrect patch.
Comment #3
Damien Tournoud CreditAttribution: Damien Tournoud commentedThat's indeed a big oversight, but I don't think the solution here is going to be enough for every widget.
- Will this solution work for all the widgets we provide?
- Do we ever show to the end-users IDs or label of entities it should not see?
- Could we lost ordering in some cases?
Let's start by sketching out some tests about this.
Comment #5
amitaibuLets try to figure out how to solve this:
> - Will this solution work for all the widgets we provide?
Maybe we can try hook into field_presave() and get the inaccessible entities from the $entity->original->{$field_name}, like this we don't need to deal with the widgets directly. However, the problem is that we might hit a field cardinality issue - so upon form validation we need to make sure not to exceed the field allowed values.
> - Do we ever show to the end-users IDs or label of entities it should not see?
We shouldn't, and I think we don't by adding the {entity_type}_access tag to the query.
> - Could we lost ordering in some cases?
I don't think we can maintain the order, as it makes better UX to hide those label from the widget, rather than renaming them to "private" or somethink like this.
Comment #6
amitaibuI've pushed a first stab, untested, far from working, WIP branch
1352690
.Comment #7
Encarte CreditAttribution: Encarte commentedRelated problem in D6: #632914: user without content permission to view/edit node_reference field cannot save node, "this post can't be referenced"
Comment #8
andypostI think field should validate values and this should not depend on widget!
The logic here is slightly depends on node_access used because in some cases you can't access a title of referenced entity so there's no way to disaplay referenced entity in any widget. In this case field should store this disabled IDs in form_state and reuse them when saving data
Comment #9
JonAnthony CreditAttribution: JonAnthony commentedYikes - I just ran into this and it was a bit scary. In essence users with less permissions were undoing the the references created in nodes by admins.
Now I see it I have written my own handlers for my specific use case, but it falls apart in more general scenarios.
The way you are discussing doing it here is more elegant.
Did you ever decide on the best route - i.e. not handle this at widget level but add the values in.
How would you handle the scenario that is was a single value entry, and the lesser permissioned user changes it to another value without being aware that the higher permissioned user had already set another value.
J
In case it helps anyone - as work around (in a separate custom module) I am not allowing users to edit nodes when they contain references with values they do not have permission to
Comment #10
DuaelFrThis issue is really interesting.
I would add a case I had to deal with a few days ago :
I suppose the case I described here is not the most common case but it exists.
Maybe we could add a setting to the instance to let the administrator choose its behavior when the referenced data is not accessible (because of permissions or because of any other reason) :
My 2 cts.