This is a serious problem. Without a serious redesign not sure how this will be possible. There is no call to db_rewrite_sql() when discovering links. But! even a fix there wouldn't solve the problem. The problem *really* shows up when a very privileged user causes a bunch of nodes to be discovered for the node he is viewing. This nodes will be displayed in the block regardless if the next user is an anon user or a privileged user. (imagine a related node by taxonomy but is unpublished for instance)

I believe the solution lies in discovering links during cron run NOT on block loading. Cron run ensures that only nodes that are viewable by anon users will be added to the block.

The other way to do it is on block content generation put the nodes through db_rewrite_sql(). Unfortunately, because the module loads related links on node load, node caching mechanisms get in the way. Thus, a privileged user (this doesn't mean a user role btw, think OG) views a node, and discovered links are added for that user AND node cache is set.

A non privileged user, with the same Drupal role, views the node but isn't member of the same groups, he gets the cache version and hence bad related links.

So! Given the current design of the module, not sure how I would patch it to make it respect node_access. It would require a rewrite of a lot of things in there.

CommentFileSizeAuthor
#5 remove_nonviewable.patch1.53 KBhillaryneaf
#1 remove_nonviewable.patch1.82 KBbkat
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bkat’s picture

bkat CreditAttribution: bkat commented
FileSize
remove_nonviewable.patch1.82 KB

I too have been annoyed that links are listed that the user does not have access to. I just wrote a little mod that is based on the remove_nonviewable_menu_items module.

The mod does the following:

1. ignores img and mailto links since I really don't want them listed
2. modify _relatedlinks_filter() to not return links to nodes that the user can't view

I'm sure it can probably be improved but at least its working and is a starting point.

It would be nice if the relatedlinks table contained the node id of the node linking to and if we had an efficient way to determine if we can access a node by the node id rather than having to load the node

bkat’s picture

bkat CreditAttribution: bkat commented
Status: Active » Needs review
Scott Reynolds’s picture

Scott Reynolds CreditAttribution: Scott Reynolds commented

I haven't tested this patch, as I am no longer using this module for finding related links. We actually have the feature turned off till we figure out what we want to do. I would mention that you should avoid preg_match and use strpos() when possible (like for determining if its a mailto or img link) it is immensely faster. doing node_access view like that all the time is pretty db heavy. would be better if there was a way to db_rewrite_sql()

hillaryneaf’s picture

hillaryneaf CreditAttribution: hillaryneaf commented

I tested the patch manually to version 6.x-1.x-dev and it didn't work for me. The blocks wouldn't show at all.

I'll try to write a patch using strpos() and db_rewrite_sql() as Scott Reynolds mentions.

hillaryneaf’s picture

hillaryneaf CreditAttribution: hillaryneaf commented
Version: 5.x-2.2-beta » 6.x-1.x-dev
FileSize
remove_nonviewable.patch1.53 KB

My first patch... please let me know if I'm doing something wrong. I've applied it to the latest version 6.x-1.x-dev.

I've replaced preg_match with strpos(), wasn't able to figure out db_rewrite_sql() to check access permissions, but at least the module is working for me now and checking access permissions. I also added a bit to allow external links to bypass the node access check.

Hope this helps someone else!

Zen’s picture

Zen CreditAttribution: Zen commented
Version: 6.x-1.x-dev » 7.x-1.x-dev
Status: Needs review » Active

I think that this can only be properly fixed via a fundamental change in how links are stored. I'll look into this for D7.

@nektir: Thanks for the patch. But the approach is by necessity too hacky. Incidentally, you will want to avoid using tabs and maintain other Drupal conventions for your patches.

Cheers,
-K

xarabithan’s picture

xarabithan CreditAttribution: xarabithan commented

This Module ist great, but it doesn't work with permissions!
1. You have to grant permission to a role to add related links (why??? this is not neccessary to SHOW related links in blocks! And this is what the module should do?!). Otherwise this role cannot create or edit a content-type associated with the module BUT the role has permission to do so!
2. If you do this, this role can edit ALL content types exept admin-only-content-types, even this one it is not allowed to! Bypass permissions of user roles is not so cool ...

This is not only a critical bug, it makes the module impossible to use on websites working with granulated user roles. Because a user with a lesser role can edit content that he isn't allowed to edit.

I like this module very much. However in this version it should be unpublished or rewritten. A big warning should be placed at top of project page!