_subscriptions_content_node_options() doesn't check 'subscribe to content' access before adding a node-level link to the list of options it returns, only that the node type is not in the static list. shouldn't it do so? the user gets an Access denied error trying to use the link.

CommentFileSizeAuthor
#1 subscriptions-content-node-options-2162519-1.patch1.66 KBbrad.bulger

Comments

brad.bulger’s picture

brad.bulger commented
Status: Active » Needs review
StatusFileSize
new subscriptions-content-node-options-2162519-1.patch1.66 KB

this moves both the node and content type link code inside the respective permission checks.

salvis’s picture

salvis
he/his
commented

How can we reproduce this issue from the GUI side?

Please follow the instructions that were displayed when you created this issue.

brad.bulger’s picture

brad.bulger commented

It's not a GUI issue. If anything, the GUI has the opposite problem, not allowing content-type subscription options to display if the user does not have node subscription permission. But that's apparently a deliberate choice, and a side-issue in any case.

It's about what module_invoke_all('subscriptions', 'node_options', $account, $node); returns. I would think that it should return correct and valid options.

salvis’s picture

salvis
he/his
commented

[...] the user gets an Access denied error trying to use the link.

But in #3 you seem to say that the GUI keeps the bad links from showing up, right?

[...] not allowing content-type subscription options to display if the user does not have node subscription permission.

I would say that makes sense, and you seem to agree.

I would think that it should return correct and valid options.

Without investigating this to the bottom let me tell you about comment.module: it does exactly what you suggest, with the result that a user who is moderator in a certain module (but does not have 'administer comments') is not getting the edit and delete links from comment.module. Creating those links is quite a pain.

The Subscriptions links are much more complex, and it seems wise to return them all, just in case that some other module wants to do something with them.

This is in line with core's use of the '#access' key to suppress form elements that should not be available, rather than removing them. I tend to think of this as a feature rather than a bug...