Hi,
I'd like to ask if there is a chance to get webfm and protected node to work together.

The problem: webfm checks if a user can access a node to which a file is attached via node_access. If access is granted for the node, the user can download files attached to this node.

But protected_node doesn't use node_access (I guess that is because the access permission must be changed on a browser-session base without changing the node_access table (as that would be permanent)) but prevents access via nodeapi('view').

Thus, files attached with webfm to pages that are protected with protected_node can be downloaded, even if the user cannot access the page itself.

If both maintainers agree (I will ask Rob to read this thread, too) I would try to make those two modules work together. I guess all that's needed is a call like drupal_alter('webfm_perm', $MATCH, $nid, $fid) in webfm, so that $MATCH could be changed, based on the nid and fid.

And in protected_node we would need to implement hook_webfm_perm, containing basically the same code as in nodeapi('view'), setting $MATCH to false if the node is protected.

What do you think? I would try to create patches for both webfm and protected_node if there are no reservations about such an approach.
Or maybe one of you has even a better idea how to bring the modules together?

cu,
Frank

CommentFileSizeAuthor
#4 protected_node_webfm_alter_perms.patch3.42 KBFrank Steiner
#4 webfm_file_access_alter.patch538 bytesFrank Steiner
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mtolmacs’s picture

mtolmacs CreditAttribution: mtolmacs commented

protected_node_file_download() cannot take care of this problem?

Also I'm okay with this, but I don't plan to add features to 5.x version. So make this a 6.x patch only.

robmilne’s picture

robmilne CreditAttribution: robmilne commented

Hi Frank - I've been out of the webfm loop due to my workload and I don't know the first thing about "protected_node". My workload won't diminish anytime soon so I will not be able to help out - you have my full support for making changes that improve the module. Ping me again when a patch warrants a commit.

-rob

Frank Steiner’s picture

Frank Steiner CreditAttribution: Frank Steiner commented

> protected_node_file_download() cannot take care of this problem?

No, because webfm uses a completely different mechanism (and sql tables) for handling files. It manages files completely independent from nodes, i.e., you can upload files that are not attached anywhere. Then you can attach uploaded files to as many nodes as you like, but those attachments are more like symbolic links to the real files that are handled in webfm. This doesn't match the normal file upload.

Ok, thanks both of you for the positive feedback, I will try to create a patch.

Frank Steiner’s picture

Frank Steiner CreditAttribution: Frank Steiner commented
FileSize
webfm_file_access_alter.patch538 bytes
protected_node_webfm_alter_perms.patch3.42 KB

I post both patches here if someone wants to test how it works together with webfm.

The protected-node-related patch alone shouldn't change the protected-node behaviour at all.

Frank Steiner’s picture

Frank Steiner CreditAttribution: Frank Steiner commented
Status: Active » Needs review
Frank Steiner’s picture

Frank Steiner CreditAttribution: Frank Steiner commented

Any chance with this feature? :-)

AlexisWilke’s picture

AlexisWilke CreditAttribution: AlexisWilke commented

That seems very specialized to include in this module.

Also, I don't see how your patch would work as several functions are called with $node instead of &$node (i.e. a reference is needed instead of a copy or the $node->body = ''; would have no effect.)

This being said, extracting the access code in a separate function is certainly a good idea.

Thank you.
Alexis Wilke

Frank Steiner’s picture

Frank Steiner CreditAttribution: Frank Steiner commented

Hi,

seeing that you have done that, all we would need to make webfm respect protected nodes was the following code in protected_node to implement the webfm hook:

function protected_node_webfm_file_access_alter(&$access, $node, $fid) {
  $access=protected_node_is_locked($node->nid, 'view')? FALSE : TRUE;
}

Do you think there is a chance to get that in?

cu,
Frank

AlexisWilke’s picture

AlexisWilke CreditAttribution: AlexisWilke commented
Status: Closed (fixed) » Needs review

Okay, it's checked in. Not a problem since it's only reacts if you have webfm.

It will be in 6.x-1.7, in the meantime you have to use 6.x-1.x-dev or implement that in your protected_node.module as I guess you've already done.

Thank you.
Alexis Wilke

Frank Steiner’s picture

Frank Steiner CreditAttribution: Frank Steiner commented
Status: Needs review » Fixed

That's great, thanks a lot :-)

Status: Fixed » Closed (fixed)

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

Status: Needs review » Closed (fixed)

Correcting issue status, see #2125263: Issue status not taken from last comment during D7 migration.