Currently running entity_access() on a message entity (eg, as in #1918666]) will call message_access(), which simply returns the value of

<?php
user_access
('create messages', $account);
?>

In some cases, a module or modules may want to introduce a more complex set of access checking criteria. Currently, this can only be accomplished by implementing hook_entity_info_alter() and changing the 'access callback' value. One drawback of that approach is that only a single module or callback can be used within one installation of Drupal.

In Commons, we want the Commons Activity streams module to be enforce access criteria on all messages that it defines, and prevent users from seeing or receiving messages that reference nodes that the viewer/recipient cannot access.

Files: 
CommentFileSizeAuthor
#6 message-hook_message_access-1920560-6.patch4.5 KBmongolito404
PASSED: [[SimpleTest]]: [MySQL] 69 pass(es).
[ View ]
#1 1920560-message-access-alterable.patch2.03 KBezra-g
PASSED: [[SimpleTest]]: [MySQL] 69 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new2.03 KB
PASSED: [[SimpleTest]]: [MySQL] 69 pass(es).
[ View ]

Patch attached defines hook_message_access_alter() and provides an example of how we're implementing this in Commons.

Status:Needs review» Closed (won't fix)

Currently, this can only be accomplished by implementing hook_entity_info_alter() and changing the 'access callback' value. One drawback of that approach is that only a single module or callback can be used within one installation of Drupal.

I believe this is the correct solution. Regarding the draw back, same thing can be said about hook_menu()'s access callback. I'm setting to won't fix, but please re-open if you think otherwise.

Status:Closed (won't fix)» Needs review

I believe this is the correct solution. Regarding the draw back, same thing can be said about hook_menu()'s access callback. I'm setting to won't fix, but please re-open if you think otherwise.

Thanks. For your consideration :

From my perspective, message access is different from a page access callback because it's more likely to have multiple different modules interacting with it depending on the situation.

For example, Commons wants to be able to restrict access for the Commons activity stream messages that contain certain entity reference fields, but have no effect on other message entities, such as the messages a user might add to her site unrelated to Commons.

This differs from a menu callback where it typically makes sense to have a single function responsible for a single path in all all cases.

You could add to the above example with a scenario where one site has Commons controlling access to its own Message entities, and another contrib or custom module listening and controlling access to only its own set of entities.

If anything I think we should follow the pattern of hook_node_access() were you return NODE_ACCESS_IGONRE/ DENY.
However I'd be happy if you could profile it, as it might add a lot of overhead - better to know exactly how much.

Issue summary:View changes
StatusFileSize
new4.5 KB
PASSED: [[SimpleTest]]: [MySQL] 69 pass(es).
[ View ]

message-hook_message_access-1920560-6.patch provide a pluggable access check for message using a hook_message_access() pattern similar to what node_access() does. When no implementation of hook_message_access() is available, it falls back to the original behavior.

@mongolito404, thanks, it looks good. Any chance for a benchmark (e.g. compare view with 100 messages with and without the patch)?

Benchmark would be good. But I dont see a way to make a simpler/opimized version of the wanted behavior.

> But I dont see a way to make a simpler/opimized version of the wanted behavior.

We can always have a variable to skip it, but before adding something like this, lets figure out the performance penalty.

> We can always have a variable to skip it,

Oh, actually I see you already have a permission to bypass it.

Status:Needs review» Needs work
  1. +++ b/message.api.php
    @@ -134,5 +134,30 @@ function hook_default_message_type_category_alter(array &$defaults) {
    +function hook_message_access($message, $op, $account) {
    +
    +}

    We can type-hint Message $message
    Also can we add an example.

  2. +++ b/message.module
    @@ -800,7 +820,52 @@ function message_delete_multiple($mids = array()) {
    +  // If the $op was not one of the supported ones, we return access denied.
    +  if (!in_array($op, array('view', 'update', 'delete'), TRUE)) {
    +    return FALSE;
    +  }

    Lets remove this.

  3. +++ b/message.module
    @@ -800,7 +820,52 @@ function message_delete_multiple($mids = array()) {
    +    $account = $GLOBALS['user'];

    I prefer loading the user, to get the full object.

  4. +++ b/message.module
    @@ -800,7 +820,52 @@ function message_delete_multiple($mids = array()) {
    +  list($cid) = !empty($entity) ? entity_extract_ids($entity_type, $entity) : array($entity_type);

    $cid => $mid

Also, can we have a test? :)