limiting Rules components to specific permissions

While there is an access system in place, there is nothing like the role-option for a component implemented.

I think it would make sense to add that to the component-settings, while alternatively specifying a required permission like Views does makes much sense to me to.

Then, we should probably default to requiring 'admin rules' permission for all other components that have no special access permission configured.

Note that I just improved handling access a bit, see #1248912: fix rules-config access to properly check for edit access. Access to components is now restrict based upon access of all configured elements, i.e. you have access to use a component if you may configure it that way.
You can already incorporate access checking by doing $rules_config->access(). This feature request for custom role-based access remains open though.

Hi, I am trying to achieve this however am not having much luck.

I have created some bulk operations via components/rules that are pulled into the content page via a view. This works fine when logged in as an admin however I need to give users with the role of 'editor' the ability to use these bulk operations also.

Currently the created operations are not showing up in the dropdown list when logged in as an editor (the standard ones are still there).

Any chance you can point me in the right direction to achieve this?

related: #992488: Action list empty for non admin

I might be very far off the bat here, but here is the results of me digging into this problem:

In modules/rules_core.rules.inc we have the function

function rules_element_invoke_component_access_callback($type, $name) {
  // Only allow access to the action/condition if the user has access to the
  // component.
  // Cut of the leading 'component_' from the action name.
  $component = rules_config_load(substr($name, 10));
  return $component && $component->access();
}

The action() function is defined in includes/rules.core.inc which is never called as far as my debugging goes.
Changing rules_element_invoke_component_access_callback to return the following fixes the problem:

function rules_element_invoke_component_access_callback($type, $name) {
  // Only allow access to the action/condition if the user has access to the
  // component.
  // Cut of the leading 'component_' from the action name.
  $component = rules_config_load(substr($name, 10));
  return $component;
}

Is this a step in the right direction?

edit:
I see now that the problem described in the first post is the opposite of my problem. my problem was that vbo component rules were not displayed to nonadmin users, the fix above rectifies this, but does in no way help towards restricting access based on role.

Well, Rules might require some admin permissions to grant access to those actions. I could imagine this is not desirable in some VBO usage scenarios, because you want it to grant using it to certain site-users.

For that to work, VBO could in the meanwhile just skip Rules access and grant everyone access - or we'll implement this feature request and add some optional UI to choose roles that are allowed to use the component.

subscribe

subscribe

Sorry about the double post. I'll keep #13.

Based on my thoughts in #1316568: Create a read-only email action, I'd also like to see the ability to add custom permissions on components. e.g. when you create the component you'd also have the option to have a new execute permission created for your component. Other modules (e.g. Views Bulk Access) could use that role to determine whether or not a user had permission to execute the action. It would then be up to the developer of the rule to determine access permissions (e.g. if I'm writing a component that is treating the nodes as read only I'd only need to check for 'view', but if I'm modifying anything I'd need to check more specifically, such as using field-level permissions)

This request seems to me to be the right place to do this. In fact, I think the custom role would take care of this request (i.e. you create the custom permission for your component and don't grant the permission to roles that you don't want to be allowed to execute it).

I think we should just add an optional setting to components, which allows one to grant access to the component by role. That way, the current default of determining access based upon component configuration access can stay.

Also, that should be rather easy to implement. Add the form element to RulesPluginUI::settingsForm() in case a component is edited + take the settings into account in rules_element_invoke_component_access_callback(). That should do it.

marked #1301940: Permissions required to perform action? as duplicate

I'm a little bit confused by the concept of denying access to rules due to the presence of InputEvaluators. I would expect them only to be relevant for direct form input, not for indirect calls of components with passed parameters. And in the case of the direct form input, I'd expect, that input of people who do not have the right for InputEvalulator, will not be processed with the given InputEvaluator. I hope I could clarify the "problem".

@fago, I somehow see this concept part of this issue, or should this be discussed in a separate one?

derhasi: yes, input evaluators have access restrictions. If someone enters php code, someone else edits the rule but is not allowed to enter php rule, he won't have access to the whole rule.

At processing time, the access does not matter usually as it won't be checked. This is, as the rule usually is evaluated for another user than the creator. For VBO though, we want to configure something and expose it with new access permissions. That's this feature request.

Makes sense?

fago, yes makes sense. I will have a closer look on this over the day, as I reckon I run into a bug dealing with access restriction in nested component calls. But then this would be a new issue.

Here's an initial version to accomplish this, I've created a new table called rules_roles to store the roles that have access to a given component (mimic of rules_tags) and modified a little the access check in rules_element_invoke_component_access_callback

bojanz:

I've changed VBO to just return TRUE for rules component access, until this issue gets solved (and we get a UI for roles). Good enough for now.

I see the change in the code, but I'm still getting "Access violation! You have insufficient access permissions to edit this configuration." when I try to use a component to bulk comment with non-admin users. Is there something I'm missing?

Ahh... My situation is slightly different. I have a configurable component, so VBO is calling rules_ui_form_rules_config_validate().

So how do I grant access to the config form for non-admin users?

+  $component_access = empty($component->roles) ? TRUE : array_intersect(array_keys($component->roles), array_keys($user->roles));
+  return $component && $component->access() && (user_access('bypass rules access', $user) || $component_access);

Let's skip $component->access() totally here, any only determine access to components based on the role-settings.

+  // If the component doesn't have any component selected, access should be
+  // granted.

Let's just grant access to the people having the rules-super-permission by default ("bypass rules access"). Once a role gets selected, apply that.

You're totally right, this only should happen for components.

I've implemented the isset approach as the rest of components do it the same way.

Also, I've included import/export support for roles.

+++ b/includes/rules.core.inc
@@ -1433,7 +1462,7 @@ abstract class RulesAbstractPlugin extends RulesPlugin {
-  protected function exportToArray() {
+  protected function  exportToArray() {

why this additional white space?

And this should come with a test case.

Ok, here it is with tests, I don't know where that extra space came from!, I've also added delete support for roles.

Thank you very much.
I've added this and it solves my performance issue related to the access check!
But I should mention that I didn't have the time yet to play around with it.

Thinking more about this, why do we go for roles and not permissions? Wouldn't it be better if we do something similar to what field_permissions does? Roles can only be exported with hacks like role_export, permissions are far better.

So the battle plan for that would be:
* Create an "access_exposed" flag column in the db
* Expose components in hook_permission() that have the "access_exposed" flag set
* set the access_exposed flag if the user selects roles in the component UI and saves them.
* during the actual access check we can always use user_access(), if the permission does not exist it will return FALSE. And we can always fallback to the bypass access permission.

Advantages:
* no extra db table needed
* uses Drupal's standard permission system for storage
* uses Drupal's standard permission UI for global administration
* better exportability

Sorry that this input comes a bit late, but I talked to fago about it and it seems that this would be a better solution.

Ouch...

Ok, so I agree with the permission based proposal, I kind of chose the role one because of two reasons, one it was the original request (and the one I got also) and two, it's kind of a "edge" use case, as this feature will probably be used just by VBO.

I have a couple of questions before adapting the code to the permissions way.

Create an "access_exposed" flag column in the db

This was something I tried to avoid, I think that it's actually better to have a separated table for this purpose as we are only affecting component rules and not the "normal use case" that are (as I see it) rule reactions.

set the access_exposed flag if the user selects roles in the component UI and saves them.

So we would display just a checkbox in the component settings that would say... "Enable role access to this component", enabling this option will affect hook_permissions and expose that particular component to the permission screen and instead of checking the roles array of the user.

Not sure about extra db table vs. additional db field. An extra table would be only one column holding the component id for which permissions are exposed. I would be fine with that.

We should leave the role checkboxes in place in the component UI for better usability (while the global permission page is a central place for overview, it can get hard to find particular permissions on that page).

After talking to klausi, I agree that the permission approach is better suited. Sry for that late change :/

This was something I tried to avoid, I think that it's actually better to have a separated table for this purpose as we are only affecting component rules and not the "normal use case" that are (as I see it) rule reactions.

Well, we affect the reaction rules anyway. At least we run a query on the roles table for them too, what would be unnecessary. Thus, I'd prefer a column for the flag (0|1) which just is 0 for reaction rules too. That way, we do not increase the number of necessary SQL queries.

We should leave the role checkboxes in place in the component UI for better usability (while the global permission page is a central place for overview, it can get hard to find particular permissions on that page).

Yep, we should keep the UI as now with an additional on/off checkbox and care about saving roles to the permission. Analogously to the new field-permissions module UI, which imo is very nice.

want to add a fundamental point: please please don't limit access with roles!!

know what happens if a sitebuilder adds a role, sets all permissions and... there's hidden permission stuff in rules.
(i know core blocks module started this bullshit but we should fix it there too)

"access by permission not roles"
like
"program against interfaces not classes"

Here we go.

This patch drops all the role stuff and includes a permission based access.
I've based the behavior in the field permissions UI as suggested, but we have a slight difference, when the field is created, the permission appears in a second screen once you've place the name and metainfo of the field, in rules the UI is different and we only can show the permissions once the rule component has been created, so I've chosen to display this setting just in the edit, which is OK as the screen you see after creating a rule component is the rule component itself, so it makes sense.

Oh and I've included tests dedicated to klausi ;)

And btw, if someone installed the patch in #30 or any of the previous ones, they should drop the rules_roles table and reapply the rules_update_7208 to get the new field.

Back to needs review, hopefully we are almost there!

Oops, a update came in the middle so now it's rules_update_7209

i tested the #39 patch.
Using an user with other role than "administrator", With or without permission set for the Rule component
- the Rule component wasn't fired
- i get the same watchdog notice message
Notice: Undefined property: Rule::$access_exposed in rules_element_invoke_component_access_callback() (line 307 of /drupal_path/sites/all/modules/contrib/rules/modules/rules_core.rules.inc).

New patch, rerolled with the latest changes in rules.

I've also added an extra validation for old rules so error in #40 doesn't trigger.

the last one did work for me

+++ b/modules/rules_core.rules.inc
@@ -299,11 +299,17 @@ function rules_element_invoke_component_features_export(&$export, &$pipe, $modul
+  // If the access is not exposed for this component, don't check and allow it.
+  if (!isset($component->access_exposed) || !$component->access_exposed) {
+    return TRUE;
+  }

Hm, we should limit default access else such components would be re-usable by anyone without requiring any permissions. Defaulting to "bypass rules access" as I previously suggested, is no good idea as it would deny people without the permission to re-use their own component ;) So let's just stay with the current access() call by default.

+++ b/rules.module
@@ -989,6 +989,31 @@ function rules_permission() {
+ * Helper function to get all the permissions for the components that have
+ * access exposed.

Minor, but this should be a long (>80) first line without break.

+++ b/rules.module
@@ -989,6 +989,31 @@ function rules_permission() {
+    $perms += array(
+      "access $component->name" => array(

This looks like it could lead to machine name collisions rather easily. Maybe just call it "access component foo". Also, I think we should better call the permission "use component foo".

+++ b/ui/ui.core.inc
@@ -478,11 +478,74 @@ class RulesPluginUI extends FacesExtender implements RulesPluginUIInterface {
+   * @return form element with the matrix of permissions for this component.

Sentence should be in a separate line and start capitalized.

I'll take a look at that

Attached is an updated patch which fixes the stuff mentioned in #43. Apart from that I've gone over it to differentiate "using components" from the regular (edit) access.

Please review, I hope the wording is clear.

Note: We need to fix access for the "schedule a component" action the same way, but I noted it currently hasn't any checks at all. See #1472028: Add permissions to list_add and list_remove

at #44 permission names:
maybe i'm name collision paranoic but i think it should be
permission "use rules component foo"
instead of
permission "use component foo"
;-)

#44: rules_permission.patch queued for re-testing.

ad #46: Sounds good. I've verified we don't have troubles with too long permission names - it's fine so I've done so. Also, I've fixed the tests to account for the permission re-name.

#50: rules_permissions.patch queued for re-testing.

Committed to dev.

Hi I just used the new permission option you guys created. It works thanks !
BUT ! The selected roles are not included in the snippet when trying to EXPORT the component. It only says that access_exposed is set to true but it does not say which roles have access to the component.

Re opening the issue as the config is not properly exported

Confirmed that the selected roles are not exported. I will have to think about how this should work since per-role permissions are stored in the usual {role_permissions} table, and the component just contains the access_exposed key to indicate that there is a specific permission dynamically defined for this component. I don't like the idea of having an imported component granting permissions covertly upon import, and since role ids are site-specific they are really not very 'importable' as part of the component. Perhaps this should be a separate issue.

But also, there are some serious problems with the UI for this feature, which apparently have been there since the commit in #53 almost 7 years ago. I really don't see how this feature was working properly for anyone, and I don't see how anyone reviewing this issue could have missed the UI problem. I linked the related issue here. This issue is definitely still needs work because the feature seems to have never worked.

Oh, and at this point it's a bug, since the feature exists it just doesn't work well.

This issue should not have been reopened.

The pending question about how to handle exports is really an entirely separate issue. The issue summary has nothing to do with the problem raised in #55, it it only hides that problem way down the page where it can't be seen or properly addressed. And the current issue title/summary makes it sound like the feature added in this issue doesn't exist yet, but it does.

I have opened up a new issue and copied comments #55-#59. The new issue is #3119864: Export of Rules component doesn't specify roles that were granted execution permission