Node Links: rewrite output doesn't respect node access levels [#648484]

Firstly a quick thanks for all your work with views - amazing stuff!

Today I noticed a wierd permissions bug when using the 'Rewrite the output of this field' feature.

Basically i have a fairly standard view - node title, a few cck fields, and i want to add a node edit and delete field in the table row for a quick workflow for admins

At first i used the standard text output for these fields - and the permissions seemed to flow through well. I.E - if the user has the correct permissions to edit any/edit own/delete any/delete own of this node type (as defined in the role permissions and access_control module) then the node edit and node delete fields would appear or not appear based on the user's ability to edit or delete the node. so the table gets rendered, and any node the user is able to edit or delete has edit and/or delete links next to it. great!

but - when i rewrite the output of the node edit and node delete field to something like:

(for the purposes of showing an icon, not some text)

then this stops working. that is to say - even if the user does not have the correct permissions to edit/delete the node - they see this field and the link it contains. clicking the link then takes them to an access denied page. not as ideal as if the icons were just shown/not shown depending on their level of access to the node.

I'm using Drupal 6.14, Views 6.x-2.7 and Content Access 6.x-1.2

I tried it two ways - one was just putting the whole link in the 'Rewrite the output of this field' field, and the other was using the 'Output this field as a link' option to create the link, and the 'Rewrite the output of this field' field' option to set the icon tag as the content of the link - both had the same result.

Any thoughts would be appreciated, let me know if this bug report isn't clear enough or if you need more info.

Thanks :)

Comment	File	Size	Author
#15	views-648484_0.patch	11.01 KB	dawehner
#15
#12	views-648484.patch	11.4 KB	dagmar
#12
#11	views-648484.patch	9.49 KB	dagmar
#11
#5	views-648484-rewrite-node-links-permissions.patch	7.39 KB	dagmar
#5

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

_paul_meta CreditAttribution: _paul_meta commented 2 December 2009 at 02:53

oops - under the line where i said..

but - when i rewrite the output of the node edit and node delete field to something like:

i wrote in a html link but (in my browser at least) its being hidden.

this is what i'm using in the rewrite output option (spaces added so it's not read as markup)

< a href='/node/[nid]/edit?destination=myview'>< img src='/files/icon/icon_edit.png'>< / a >

Comment #2

dagmar

Spanish

Argentina

CreditAttribution: dagmar commented 2 December 2009 at 18:05

Status:

Active

» Postponed (maintainer needs more info)

Are you using a filter to show only the nodes that can be edited by the current user? If not, this is the expected behavior, views will show you a link to an access denied page, because Content Access cannot alter this html link.

You can use the module Views Custom Field using a PHP snippet to determine if the user can edit this node or not. However I think that a filter is the best option for this case.

Comment #3

_paul_meta CreditAttribution: _paul_meta commented 3 December 2009 at 01:34

I am not using a filter in the way you describe - as I would like all nodes of the required content type to be visible, but only those nodes that the user has permission to edit or delete should include an edit or delete icon when displayed in the table view.

for example - a list of all blog posts in a site. current user has permission to view all, but only has permission to edit/delete their own blog posts. when using this view - they would see a list of all blog posts, and any that they can edit/delete would have the edit/delete icons in the view. if a user with 'edit any/delete any' permission goes to the same view - they would see all blog posts with an edit/delete icon as they have permission to edit/delete all blog posts.

the thing is - this is how it actually functions when using un-modified output - just the default word edit or delete. i was surprised and impressed that this level of precision with handling permissions works with views.... (seems to be content access and views and drupal permissions all cooperating very nicely!)

but - when i re-write the output of the node edit and delete fields to be an image and not the default text - using the 'rewrite the output of this field' - the view seems to stop checking if the user has permission to edit/delete the node and emits the edit/delete fields regardless. this will confuse the user and lead to lots of permission denied pages.

hence why i think this may be a bug.

if you need me to do some screen shots, let me know :)

Comment #4

_paul_meta CreditAttribution: _paul_meta commented 3 December 2009 at 02:16

by the way - i disabled content access and rebuilt permissions to try and isolate the problem.

the same situation applies. in a view where default text is used for the field output - the node edit and delete fields are shown or hidden based on the user's permission to do these operations on the node.

once i rewrite the output, this intelligent display doesn't take place any more and both edit and delete fields are displayed even if the user can't do these operations.

Comment #5

dagmar

Spanish

Argentina

CreditAttribution: dagmar commented 3 December 2009 at 10:25

Title:	rewrite output causes permissions issue	» Node Links: rewrite output doesn't respect node access levels
Version:	6.x-2.7	» 6.x-3.x-dev
Status:	Postponed (maintainer needs more info)	» Needs review

File	Size
views-648484-rewrite-node-links-permissions.patch	7.39 KB

@_paul_meta: Thanks for your exhaustive explanation. Now I can replicate the bug. Please test this patch.

I'm changing the version of this issue but this patch should apply for both 2.x and 3.x version of views.

Comment #6

dawehner

German

CreditAttribution: dawehner commented 3 December 2009 at 21:45

Status:

Needs review

» Reviewed & tested by the community

Patch works perfect.

The batch does basically only moves things around, so nothing in case of codestyle.

PS: I wondered myself whether there is a constant for published but it isn't

Comment #7

_paul_meta CreditAttribution: _paul_meta commented 4 December 2009 at 05:50

Thanks very much for the quick response!

I will test this patch over the weekend.

Comment #8

_paul_meta CreditAttribution: _paul_meta commented 9 December 2009 at 09:35

Patch tested over the weekend and it seemed to fix up the permissions bug - node edit/delete links are emitted only when user has the ability to do these operations - on a node by node basis.

Thanks so much for a quick response to this - very helpful for a couple of jobs at the moment!

Comment #9

merlinofchaos CreditAttribution: merlinofchaos commented 23 December 2009 at 21:11

I'm not comfortable with this patch.

One thing I noticed: A no-code workaround that works RIght Now is to check "Hide if empty" which should actually remove any edit links that the user doesn't have access to, because that causes the field to be "empty" as far as Views is concerned.

Comment #10

merlinofchaos CreditAttribution: merlinofchaos commented 23 December 2009 at 21:12

In fact, with that workaround I'm not sure we need to patch at all, except to bring the edit handler up to more modern standards.

Comment #11

dagmar

Spanish

Argentina

CreditAttribution: dagmar commented 23 December 2009 at 22:00

Status:

Reviewed & tested by the community

» Needs review

File	Size
views-648484.patch	9.49 KB

I have re factored the patch to adapt it to the modern render_link way.

Comment #12

dagmar

Spanish

Argentina

CreditAttribution: dagmar commented 23 December 2009 at 22:27

File	Size
views-648484.patch	11.4 KB

New patch. This patch has a small problem. If it is applied to a views with filters, users will see the nid instead of Edit or Delete, they always can change this by setting in rewrite the output of this field, but would be nice if this was done automatically.

Also I don't know how to add the drupal_get_destination() to be included into url path.

Comment #13

merlinofchaos CreditAttribution: merlinofchaos commented 23 December 2009 at 22:36

I believe ['alter']['query']. You may have to break apart drupal_get_destination() and do what it does so that you can send data in the format it wants. Not sure about that.

Comment #14

merlinofchaos CreditAttribution: merlinofchaos commented 11 March 2010 at 23:16

Status:

Needs review

» Needs work

I guess this should be needs work based upon dagmar's comments in #12.

Comment #15

dawehner

German

CreditAttribution: dawehner commented 12 March 2010 at 09:35

File	Size
views-648484_0.patch	11.01 KB

drupal_get_destination fails with admin preview, i think thats ok. It fails because $_GET['q'] is nojs/preview etc. But this is also a problem with every other instance of drupal_get_destination.

This updated patch just fixes the destination

Comment #16

mossill CreditAttribution: mossill commented 13 May 2010 at 00:37

Does this work ? really seems like this feature should be very important. I need to pass an additional value to the node edit form but rewrite is displaying the [edit] link to user's who don't have edit permissions.

Comment #17

mossill CreditAttribution: mossill commented 13 May 2010 at 00:45

Hide if Empty works !!!!! doh !
even on older versions of views. wheww......

Comment #18

marcelodornelas CreditAttribution: marcelodornelas commented 19 May 2010 at 10:27

Thanks merlinofchaos , "hyde if empy" saved me a lot of time.

Comment #19

dawehner

German

CreditAttribution: dawehner commented 31 May 2010 at 21:49

Status:

Needs work

» Needs review

I guess this should be NR

Comment #20

dawehner

German

CreditAttribution: dawehner commented 13 August 2010 at 20:42

Issue tags:

+alpha-4 blocker

Add tag

Comment #21

Remon CreditAttribution: Remon commented 3 December 2010 at 14:24

Bumping last alpha-4 blocker.

Comment #22

merlinofchaos CreditAttribution: merlinofchaos commented 18 December 2010 at 00:07

Status:

Needs review

» Needs work

I think the patch in #15 will break existing handlers that have changed the text of the field. It removes the 'text' option in favor of using rewriting. Unfortunately, it doesn't make an attempt to preserve the pre-existing data, which would have to happen in init.