New Permissions [#1339068]

While working on the OpenAcaDept distribution, I have realized that it will be very nice to have additional permissions. We have 3 main roles: student, alumnus, faculty/staff. Currently it is configured that each role have access to specific profile fields. It works good when user is editing own profile, however there are some issues when administrator is editing any profile. Administrator can see any fields in this case, which should not happen.

So the idea is to introduce new permissions:

a) Allow other users to edit field_name of entity depending on the author role
b) Allow other users to view field_name of entity depending on the author role

There are similar issue here http://drupal.org/node/1107994, which was closed, cause of probable permissions issue. I don't see there any performance issues.

All permissions should work for any entity which have uid in the data schema.

Comment	File	Size	Author
#3	new-permissions-1339068-2.patch	6.63 KB	Alex Komm
#1	new-permissions-1339068-1.patch	6.21 KB	Alex Komm

Comments

Comment #1

Alex Komm commented 12 November 2011 at 07:39

Status:

Needs work

» Needs review

Status	File	Size
new	new-permissions-1339068-1.patch	6.21 KB

Comment #2

dealancer commented 29 November 2011 at 13:53

Status:

Needs review

» Needs work

In general look very nice, however there are some minor spacing and logical issues.

+++ b/field_permissions.admin.incundefined
@@ -14,7 +14,24 @@
+  $edit_created_by = array();
+  ¶

Spacing issue - spaces on empty line.

+++ b/field_permissions.admin.incundefined
@@ -34,8 +51,10 @@ function field_permissions_list($field_label = '') {
+    ),    ¶
+  ) + $view_created_by + $edit_created_by;
+    ¶

Spacing issue

+++ b/field_permissions.moduleundefined
@@ -145,25 +145,31 @@ function _field_permissions_field_view_access($field_name, $obj_type, $object, $
+  }
+  ¶

Spacing issue

+++ b/field_permissions.moduleundefined
@@ -145,25 +145,31 @@ function _field_permissions_field_view_access($field_name, $obj_type, $object, $
+  // Iterate through the list of user roles
+  foreach(user_roles() as $id => $role) {
+    //if the user has permission to view objects which is owned by user with scpecific role,
+    //return TRUE if object owner has that role or FALSE if he don't
+    if (user_access('view created by ' . $role . ' ' . $field_name)) {
+      return _field_permissions_object_owner_has_role($object, $obj_type, $role);
+    }
+  }
   return FALSE;
 }

We have placed this check after this lines of code:

  if (user_access('view own ' . $field_name, $account)) {
    return _field_permissions_object_is_owned_by_account($object, $account);
  }

This brings some confusion to logic. If view own fielf permissions is unset, then permission 'view created by role' is checked, otherwise it is ignored.

I think there should be OR logic between 'view own field' permission and 'view created by role' permission, and we need to accacmulate return result in variable trough OR operator and then return it.

The same is applicable for edit permissions.

+++ b/field_permissions.moduleundefined
@@ -192,7 +198,15 @@ function _field_permissions_field_edit_access($field_name, $obj_type, $object, $
+  ¶
+  // Iterate through the list of user roles

Spacing issue

+++ b/field_permissions.moduleundefined
@@ -206,3 +220,23 @@ function _field_permissions_object_is_owned_by_account($object, $account) {
+    return FALSE;
+  }  ¶
+  $owner = ($obj_type == 'user') ? $object : user_load($object->uid);
+  ¶
+  return in_array($role, $owner->roles);  ¶
+}

Spacing issue