Rules 2.x is a rewrite of Rules 1.x, so a direct port of content access from 6.x to 7.x won't do the job. The Rules integration piece will be rewritten too. This issue will hold the patches, development status and discussions about the integration.

Comments

Hey good_man,

Thanks for creating this issue and congrats on getting an official 7.x branch! Here's a couple of items for our Rules to-do list:

A. We still need to create the "Grant access for a user" and "Revoke access for a user" actions so that Rules can be used to add individual users to an ACL.

B. Get Rules actions working with non-node related events. For instance, on a "User got points" event supplied by Userpoints module, we many want to grant a user access to a node. We'd need to have data selectors available for the node id and for the user (user getting points, current user, etc.). The node id and user could be hard-coded using a direct input field, or it could be derived from something using the data selector autocomplete field (such as the user receiving the points).

Some potential code examples can be found here (for the Userpoints Nodeaccess module that I also help with):
http://drupal.org/files/issues/userpoints_nodeaccess_rules2.patch

Are there other Rules to-do items?

--Ben

Hey good_man,

Long time, no talk! I hope all is well with you. Do you want to get some momentum back in working on this? I've got some time available in the next few days and we had a pretty good workflow going between us last time around... ;-)

--Ben

Here's another Rules to-do item:

C. Add support for conditions (checking what operations a role can perform on a node)

The 6.x version of the Rules integration already had this feature. But we haven't ported it over to 7.x-1.x-dev (and Rules 2.x yet). Basically, it allows you to configure a rules condition that only will trigger the action if a certain node has defined permissions for a particular role. Here's the code from the D6 version (Rules 1.x) that needs to be adapted:

/**
* Implementation of hook_condition_info().
*/
function content_access_rules_condition_info() {
  return array(
    'content_access_condition_check_permissions' => array(
    'label' => t('Check role based settings'),
    'arguments' => array(
      'node' => array('type' => 'node', 'label' => t('Content')),
    ),
    'help' => t('Evaluates to TRUE, if content access allows all selected operations for the given
roles.'),
    'module' => 'Content access',
    ),
  );
}
/**
* Condition implementation: Check if node has permissions.
*
* @note
*   This will only check for the existence of permissions, not the
*   absence of. I.e. a rule that checks just for authenticated write
*   will return TRUE for a node that allows authenticated and anonymous
*   write.
*/
function content_access_condition_check_permissions($node, $settings) {
  if (_content_access_rules_check_setting($node)) {
    // Compare our settings with node's settings.
    foreach (_content_access_get_operations() as $op) {
      $settings += array($op => array());
      $expect_roles = $settings[$op];
      $current_roles = content_access_per_node_setting($op, $node);
      if (count(array_diff($expect_roles, $current_roles)) != 0) {
        return FALSE;
      }
    }
    return TRUE;
  }
  return FALSE;
}
function content_access_condition_check_permissions_form($settings, &$form) {
  content_access_rules_role_based_form($settings, $form);
}
function content_access_condition_check_permissions_submit(&$settings, $form, $form_state) {
  content_access_parse_settings($settings, $form, $form_state);
}

--Ben

In follow up to item A. ("Grant access for a user" and "Revoke access for a user" actions), here's the code from the D6 version (Rules 1.x) that needs to be adapted:

/**
* Implementation of hook_action_info().
*/
function content_access_rules_action_info() {
  if (module_exists('acl')) {
    $items += array(
      'content_access_action_acl_grant_access' => array(
        'label' => t('Grant access for a user'),
        'arguments' => array(
          'node' => array('type' => 'node', 'label' => t('Content')),
          'user' => array('type' => 'user', 'label' => t('User')),
        ),
        'module' => 'Content access',
      ),
      'content_access_action_acl_revoke_access' => array(
        'label' => t('Revoke access for a user'),
        'arguments' => array(
          'node' => array('type' => 'node', 'label' => t('Content')),
          'user' => array('type' => 'user', 'label' => t('User')),
        ),
        'module' => 'Content access',
      ),
    );
  }
  return $items;
}
/**
* Action implementation: Grant access for a user.
*/
function content_access_action_acl_grant_access($node, $user, $settings) {
  if (_content_access_rules_check_setting($node)) {
    module_load_include('inc', 'content_access', 'content_access.admin');
    foreach ($settings['ops'] as $op) {
      $acl_id = content_access_get_acl_id($node, $op);
      acl_add_user($acl_id, $user->uid);
      acl_node_add_acl($node->nid, $acl_id, $op == 'view', $op == 'update', $op == 'delete', content_access_get_settings('priority', $node->type));
    }
    // A following node_save() updates the grants for us.
    return array('node' => $node);
  }
}
function content_access_action_acl_grant_access_form($settings, &$form) {
  $settings += array('ops' => array('view', 'update', 'delete'));
  $form['settings']['ops'] = array(
    '#type' => 'checkboxes',
    '#options' => drupal_map_assoc(array('view', 'update', 'delete')),
    '#title' => t('Operations to grant access for'),
    '#default_value' => $settings['ops'],
  );
}
function content_access_action_acl_grant_access_submit(&$settings, $form, $form_state) {
  $settings['ops'] = array_filter($settings['ops']);
}
function content_access_action_acl_grant_access_help() {
  return t("Note that this action is not going to revoke access for not chosen operations.");
}
function content_access_action_acl_grant_access_label($settings, $argument_labels) {
  return t('Grant access for @user.', $argument_labels);
}
/**
* Action implementation: Revoke access for a user.
*/
function content_access_action_acl_revoke_access($node, $user, $settings) {
  if (_content_access_rules_check_setting($node)) {
    module_load_include('inc', 'content_access', 'content_access.admin');
    foreach ($settings['ops'] as $op) {
      $acl_id = content_access_get_acl_id($node, $op);
      acl_remove_user($acl_id, $user->uid);
    }
    // A following node_save() updates the grants for us.
    return array('node' => $node);
  }
}
function content_access_action_acl_revoke_access_form($settings, &$form) {
  $settings += array('ops' => array('view', 'update', 'delete'));
  $form['settings']['ops'] = array(
    '#type' => 'checkboxes',
    '#options' => drupal_map_assoc(array('view', 'update', 'delete')),
    '#title' => t('Operations to revoke access for'),
    '#default_value' => $settings['ops'],
  );
}
function content_access_action_acl_revoke_access_submit(&$settings, $form, $form_state) {
  $settings['ops'] = array_filter($settings['ops']);
}
function content_access_action_acl_revoke_access_help() {
  return t("Note that this action is only able to revoke access that has been previously granted with the help of the content access module.");
}
function content_access_action_acl_revoke_access_label($settings, $argument_labels) {
  return t('Revoke access for @user.', $argument_labels);
}

--Ben

Subscribing.

Basic actions (grant, revoke, reset) now working in 7.x-1.x branch, please test it, and tell me if it's working.

Hey good_man,

I've been testing your recent Rules updates but have run into a couple of roadblocks:

1. When trying to use the grant access to role action, I'm getting a PDO "duplicate entry" error. To test, I'm trying to grant an "editor" role the "edit any" permission whenever new content is created by me (user #2). Here's how to recreate:

a. Create an "editor" role
b. For the access control on the "article" type, uncheck all (except for administrators)
c. Create rule on event "Saving new content"
d. Create condition using "Data comparison". For the node:author:uid, compare to a value of "2" (the action should be granted if content author is user #2)
e. Create action "Grant access by role" and check "Edit Any" for your "editor" role.

Upon doing this, I get a PDO "duplicate entry" error. I misplaced the actual error message (or else I would have pasted it here) and then blew up that -dev site by accident. But if you can't recreate the actual error message, then let me know and I can rebuild a new -dev site.

2. I'm not seeing either the "Grant access for a user" or "Revoke access for a user" actions. You mentioned that these should be working, but I'm not seeing them as available actions in the Rules interface.

Thanks!

--Ben

Status:Active» Needs review
Issue tags:+Release blocker
StatusFileSize
new2 KB
Test request sent.
[ View ]

Yes I confirm the first case, it's caused only when node save is chosen as a condition, because on node save the grants will be written twice, one by the rule's action from content access, and one from node_save in node module. I added a check to confirm no grants are written and the operation is not node save.

Re. grant/revoke for a user, I said it should be added soon :) I want now to confirm the role based actions are working then we move to the user based actions, then a release :)

A patch provided for future reference, and also I committed the patch to the 7.x-1.x git branch.

Thanks Ben for testing, waiting the green light from you to move to the next step.

@Ben: I've added a newer version, it contains two new events, one on content type access changed, and the other on granting user to a piece of content. Also, the disabled checkboxes now works in Rules :). Also, extra 3 conditions have been added for user grant view/update/delete. Can you please test these hot features?

I'd like to ask you what conditions might be an interesting case to implement? maybe a condition "When access control changes" on a node?

@Ben: I've added a newer version, it contains two new events, one on content type access changed, and the other on granting user to a piece of content. Also, the disabled checkboxes now works in Rules :). Also, extra 3 conditions have been added for user grant view/update/delete. Can you please test these hot features?

I'd like to ask you what conditions might be an interesting case to implement? maybe a condition "When access control changes" on a node?

btw,Rules integration now sets in sub-module, I think it's good for future work and performance.

Cool! Looks like you've been busy! :-) I'll take a look and do some testing later today....

--Ben

Hey Khaled,

I've begun testing and have been working on the error reported in #7. I can confirm that the "duplicate entry" error is gone, but the issue I'm having is that the content type access control defaults aren't being used any more (instead only the rules-generated grants are being used).

To see what I mean, if you set the default access control grants for the "Article" type to grant view access to authenticated users, but then create a rule that grants the "editor" role edit access, then only edit access is granted and the view access grants are never made.

Is there any way around this? Is there any way to avoid the duplicate PDO error without dropping the content type access control defaults altogether? Do you think this is just an edge case and we should move on?

I'll continue with testing the new features....

--Ben

I just tried using the "Grant user view access" action on the "After saving new content" event and got another duplicate PDO error:

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '12-7-acl' for key 'PRIMARY': INSERT INTO {node_access} (nid, realm, gid, grant_view, grant_update, grant_delete) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5), (:db_insert_placeholder_6, :db_insert_placeholder_7, :db_insert_placeholder_8, :db_insert_placeholder_9, :db_insert_placeholder_10, :db_insert_placeholder_11); Array ( [:db_insert_placeholder_0] => 12 [:db_insert_placeholder_1] => acl [:db_insert_placeholder_2] => 7 [:db_insert_placeholder_3] => 1 [:db_insert_placeholder_4] => 0 [:db_insert_placeholder_5] => 0 [:db_insert_placeholder_6] => 12 [:db_insert_placeholder_7] => content_access_rid [:db_insert_placeholder_8] => 3 [:db_insert_placeholder_9] => 1 [:db_insert_placeholder_10] => 0 [:db_insert_placeholder_11] => 0 ) in node_access_write_grants() (line 3332 of /Users/benkaplan/git/commerce_quickstart12b/modules/node/node.module).

Same problem as with the role grant? I think this scenario isn't less of an edge case, however. I think it would be a common use case to want to grant a user access to a node on node creation (via a rule) because you actually can't do this with the regular module (since there are no content type defaults for individual user access).

Also, a few thoughts/suggestions:

a. How about making the direct input selector (for the user being granted access) an autocomplete field on the username? That might be a little easier to use than having to figure out someone's uid (as in the current implementation).

b. Is there a reason you divided up the user grants into separate actions for view, update and delete? In the D6 version of the module, there were just two: "Grant access for a user" and "Revoke access for a user". Just wondering what your idea was here. It might be unnecessarily complex to require three different actions just to grant one user the ability to view, edit, and delete a node.

--Ben

P.S. I'm continuing to test other features... disabled checkboxes working very nicely! :-)

--Ben

I've pushed a new fix for issues in comment #13, also a second push for #14.

Sweet! I'll test it tonight and report back... :-)

Thanks a lot, I'm willing to write some tests ASAP. working with Rules 2.x and access system is very tricky!

Cool. If you need some sample code for comparison's sake, you might take a look at http://drupal.org/node/1200654 (for the Signup Rules Integration module). Jordojuice (who is the maintainer of that module and commenting on the thread) might be a good sounding board for us and he said he's willing to help. He seems quite experienced with Rules.

Talk soon,
Ben

Ah sure I need to talk to fago too, I've made many workaround for the settings form to work with Rules 2.x api.

I'm testing the latest -dev code and can confirm that the PDO error described in #14 is now gone. Nice work!

But I've also noticed that the "Grant user delete access" action is now missing from the Rules UI. Was that intentional? Were you planning on consolidating view, update, and delete permissions into "Grant access for a user" and "Revoke access for a user"?

More testing currently underway...

--Ben

Thanks Ben, I didn't recall a delete grant :\ though it should be added. Your idea is very attractive, instead of 6 actions (3-grant/3-revoke) we can group them into 2 actions grant/revoke and a small settings form with three checkbxoes, view/update/delete.

I think this is the last blocking issue for a release :)

@good_man: Sounds great. One more thing: What did you think about my idea to make the direct input selector (for the user being granted access) an autocomplete field on the username? Currently, you have to know the UID.

--Ben

The user thing is from Rules, I'll make sure of that (autocomplete username), and maybe open and issue on Rules queue.

Ok, cool. From my work on Userpoints integration with Rules, I thought there was an easy way to turn that into an autocomplete.

--Ben

Just noticed a potential bug when granting view access to an individual user via our Rules integration. In addition to the correct ACL entry for the user's view access grant, empty ACL entries are also being created for the update and delete grants. So using the Devel Node Access block, I see this:

18 0 ok acl 18 1 0 0 content_access/view_18: Gary Kaplan
18 0 empty acl 19 0 0 0 content_access/update_18: no users!
18 0 empty acl 20 0 0 0 content_access/delete_18: no users!

We shouldn't be creating these empty acl grants, right? Would this be better solved when we group view/update/delete into two actions (grant and revoke)?

--Ben

I did more testing of the issue reported in comment #13 and I'm getting the same problem (content type defaults are being ignored and Rules are completely overriding). Do you think this is how it should work? I'm not sure. The confusing part is what is the different between grant and revoke if the content type defaults are ignored anyway?

--Ben

Regarding #25 I didn't notice such thing in userpoints, do you specifically where to find it in UI?

For #26 it's ACL thing not Content Access. ACL adds the three op (view, update, delete) even for one op.

RE: #25... Hmmm, maybe I got confused with work on Private Message module Rules integration... will need to investigate this or else ask Berdir.

--Ben

Status:Needs review» Needs work

RE #26: I asked Salvis in the ACL queue about the empty ACL grants and he says its CA not ACL that is causing this. Here's a link to his answer:

http://drupal.org/node/1209160

I did some more investigating on this and it looks like all three empty ops (view, update, and delete) are being automatically added by default to any node in which the per node access control settings are enabled (for that content type). As a result, since I've enabled per node access control for the Article content type, all articles on my site have those three empty ACL grants even when no users have been added to an ACL.

What do you think about this? Is this a bug that should be fixed or no big deal?

--Ben

Status:Needs work» Needs review

Regarding #27 the rules actions has the same effect as node-access-settings, so if you set the following:
- Article content type: authorised users can view.
- A1 is content from Article content type: from it's access tab you set authorised can't view.

Now all content from type Article are viewable by auth. users except A1 (because it's per-node settings overrides content-type settings). This is the same with Rules, you set a rule action settings on a node, these settings will override the settings of node's content type.

I've pushed a newer version with 2 actions for user (instead of the old ones, see #21 and #22): User Grant/Revoke, each one has View/Update/Delete operations.

This should be the latest push before a beta release (unless a critical bug found).

#30 I've replied on that issue, also per-node won't add empty records unless you typed some users into the auto complete boxes (view/update/delete), anyhow if you find this statement is wrong, please open a new issue for that so we can work on it.

I just noticed there's now a Rules event called "Grant user on content". I haven't tested this yet. But I'm not exactly sure what this means... is it an event that fires when an individual user is added to an ACL?

--Ben

Yes there are hidden treasures only available when you enable ACL :) It fires when an individual user is added to an ACL.

Instead of "Grant user on content", here are some possible event names that might be more clear:

"User has been added to ACL"
"User has been granted access"
"User has been granted content access"

What do you think?

--Ben

Also, I'm not 100% clear about what we mean by "Content type access changed". Does this event fire when the access control on an individual node is changed or only when the content type defaults for access control is changed?

If it's intended to be an event at the node level, some suggestions include:

"Access to content has been changed"
"Content access has been changed"
"Content access control has been changed"
"Access control has been changed"
"Content access has been changed for a node"

Is this the meaning you intended or something different? What do you think?

--Ben

Yes the second one fires when a content type defaults for access control is changed.

I'll go with "User has been added to ACL" because it's not yet separate between Grant/Revoke, it'll fire on both, not only grant.

For the second one (content type) any suggestions? and what about adding third even on individual node access change?

Sounds good. Thanks for the clarification. And yes, we should add a third event on individual node access change... that could be very handy.

So here's my language suggestions for each...

Event #1: "User was added to ACL"

Event #2: "Content type access control was changed" or "Access control for a content type was changed"

Event #3: "Per node access control was changed" or "Access control for a node was changed"

What do you think?

--Ben

Yeah super cool, will do these changes, any news re. testing? can we go to the beta?

Update: Prefer:
Content type access control was changed
Per node access control was changed

I've just finished another round of testing and things are looking good... really good! :-) I really like how you combined all of the per user actions into the "Grant" and "Revoke" actions. Tested those new actions and everything worked smoothly.

Here are my last little clean up items that we should possibly do before releasing the beta:

1. Standardize the Rules action names

We're not that consistent with how we name the various actions. Here are my recommendations:

* Reset access to content type defaults
* Grant access by role
* Revoke access by role
* Grant access by user
* Revoke access by user

Question: Does the "reset" action change a node's access control to the content type defaults or to the Drupal core defaults? I'm assuming the former in my label above.

2. The labels and descriptions on some of Rules fields are a little confusing for the new "Grant user" and "Revoke user" actions. I'd recommend changing them as follows:

>> GRANT ACTION

* CONTENT
Current description: "Grant node view to specific user(s)."
Change to: "Grant access to the following content."

* GRANT USER VIEW ACCESS --> Change to: GRANT VIEW ACCESS
Current description: "Grant the following user."
Change to: "Grant view access to the following user."

* GRANT USER UPDATE ACCESS --> Change to: GRANT EDIT ACCESS
Current description: "Grant the following user."
Change to: "Grant edit access to the following user."

* GRANT USER DELETE ACCESS --> Change to: GRANT DELETE ACCESS
Current description: "Grant the following user."
Change to: "Grant delete access to the following user."

>> REVOKE ACTION

(Make the identical label and description changes as shown above, except use the word "Revoke" instead of "Grant" in each case. This way, everything will be consistent between the two.)

3. Update the "Grant access by role" and "Revoke access by role" CONTENT descriptions

Change the description of the CONTENT field to read:

* "Grant access to the following content." (for the Grant by role action)
* "Revoke access to the following content." (for the Revoke by role action)

4. Update event names

As discussed above, we should update the Rules event names to the following:

* "User was added to ACL"
* "Content type access control was changed"
* "Per node access control was changed"

After we do these little clean-up items, let's release the beta version! :-)

--Ben

P.S. One other little thing I noticed in testing: If you are using the "Revoke by user" action and set it to revoke a node that does not have any ACL realms created (because the autocomplete fields of view/update/delete have not been used), then after firing the action (which accomplishes nothing) the three empty ACL realms are created. I'm not sure if this is bad or good, but I wanted to mention it because I observed the behavior. Thoughts?

Status:Needs review» Needs work

Status:Needs work» Needs review

Ok all names are changed to match your suggestions in #40.

Question: Does the "reset" action change a node's access control to the content type defaults or to the Drupal core defaults? I'm assuming the former in my label above.

Content type defaults

P.S. One other little thing I noticed in testing: If you are using the "Revoke by user" action and set it to revoke a node that does not have any ACL realms created (because the autocomplete fields of view/update/delete have not been used), then after firing the action (which accomplishes nothing) the three empty ACL realms are created. I'm not sure if this is bad or good, but I wanted to mention it because I observed the behavior. Thoughts?

Good point, it shouldn't add dump rows like this. I'll try to commit a fix for this soon.

We're almost there! ;-) All of the label changes and cleanup looks great.

I'm just seeing the following notice at the top of the main Rules settings page (admin/config/workflow/rules):

Notice: Undefined index: content_access_content_user in RulesUIController->overviewTableRow() (line 245 of /Users/benkaplan/git/drupaldevsite/profiles/commerce_kickstart/modules/contrib/rules/ui/ui.controller.inc).

Once we get rid of that notice, we're probably ready to release. We can hold off on the "Revoke by user" ACL issue (described in the P.S. above) and I can make a separate issue for that, if you'd like. This way, if the fix is complicated, it won't hold off release.

Do you want me to test again after the above notice is fixed? I've got time all day today if you want.

Talk soon,
Ben

I talked to BenK, the last issue is because some methods names changed, and some old rules was firing with the old names, removing them sorted the issue, so we are now able to release a beta :)

Thanks all and especially Ben, you've done a great job.

I'll keep this issue as needs work to solve:

P.S. One other little thing I noticed in testing: If you are using the "Revoke by user" action and set it to revoke a node that does not have any ACL realms created (because the autocomplete fields of view/update/delete have not been used), then after firing the action (which accomplishes nothing) the three empty ACL realms are created. I'm not sure if this is bad or good, but I wanted to mention it because I observed the behavior. Thoughts?

Status:Needs review» Needs work

I just had a look at the code in the repo. Some remarks:

* We should add 'access callback's to all events, conditions and actions. Just restricting access using the permission should be fine.

* Why a separate a module? Personally I prefer not clutter the modules page + there is not much code beside the conditionally includes .rules.inc anyway. So just the event invocations would have to go into the .module file.

* The events need variables to be useful.

      'description' => t('Reset node permissions to default permissions'),

If the description does not say more than the label, just leave it out.

      'group' => t('Content Access Role'),

Usually just module names are used as group.

  $form['submit']['#submit'] = array('content_access_action_settings_form_submit', 'rules_ui_edit_element_submit');

This won't work as values are extracted earlier. Better use an #after_build callback, verify the form is being submitted (check $form_state['process_input']) and alter the value by using form_set_value().

I got some errors when using it due to the missing form include. So I worked over it and cleaned things up for the role-permission actions. Works for me. Please test.

Don't forget to clear caches after updating.

StatusFileSize
new0 bytes

You know what, I think this should have gone in a new thread - sorry!

Whats the status on this? I just downloaded the green version and it only has grant access to role and doesnt show grant access to a user. I need to hide all nodes from users accept if they are node author or have a relationship with the author.

StatusFileSize
new4.4 KB

As fago summarized in #45, the events provided at the moment in -dev lack variables, so they don't really convey useful information to reactions. In addition, the invocations of content_access's hooks in content_access_page_submit() didn't send along variables related to the action in question. So this patch also affects the module's API, though it's minimally-used outside the context of rules, AFAIK.

(To that end, I've also added an API file for documenting these function prototypes, over on #1145984: API Documentaion)

I've made my best guess as to what information should be passed through to rules for these various events, in particular content_access_user_acl, which was basically just reacting to an ACL being saved, with no sensitivity to the users involved.

This is still "needs work" since I'm not particularly sure what the content_access_content_type event should look like; it doesn't appear to really be used in the module at current.