I'm looking at an application where I need to do a very custom test to determine if a user can administer another user (it's based off user profile data). So it's very close to what user_protect does, and if I did it from scratch, it would use the same mechanism you use with userprotect.

My customer is already using userprotect for other reasons, so to do what I want, either I need to get in after you call hook_menu_alter and call userprotect_user_edit_access on your behalf, or I need to patch userprotect to do what I need it to do without conflicting with you.

It would be pretty easy for me to work in a hook_userprotect($op, $account) to do this. Would you be open to such a patch for the 6.x branch?

I've done the contributed module thing myself, and know what this looks like from your end. But I hope you'll be willing.

CommentFileSizeAuthor
#4 userprotect-issue-1140350-7.x.patch4.41 KBTorenware
#3 userprotect-issue-1140350-6.x.patch4.14 KBTorenware

Comments

hunmonk’s picture

hunmonk commented
Version: 6.x-1.5 » 7.x-1.x-dev
Category: task » feature

i would consider this for 7.x, but new features are closed for 6.x.

Torenware’s picture

Torenware commented

Fair enough. I'll supply both; the 6.x will then be findable to the folks who want it, and if you like the patch for 7.x, you'll have it handy.

Torenware’s picture

Torenware commented
StatusFileSize
new userprotect-issue-1140350-6.x.patch4.14 KB

Here's the patch for 6.x. The patch for 7.x soon...

Torenware’s picture

Torenware commented
Status: Active » Patch (to be ported)
StatusFileSize
new userprotect-issue-1140350-7.x.patch4.41 KB

And the 7.x patch, made against master.

Torenware’s picture

Torenware commented
Status: Patch (to be ported) » Needs review

Looks like I need to change the status on this. I've been using this patch for a while on D6; works well.

hefox’s picture

hefox commented
Status: Needs review » Needs work 
+++ b/userprotect.module
@@ -388,6 +388,25 @@ function userprotect_menu_alter(&$callbacks) {
+function _userprotect_hook_invoke($op, $account, &$response) {
+  $hook = 'userprotect';

This seems a bit silly, $function = $module . '_userprotect' may be better.

+++ b/userprotect.module
@@ -388,6 +388,25 @@ function userprotect_menu_alter(&$callbacks) {
+    else if (isset($result)) {

elseif (coding standards)

+++ b/userprotect.module
@@ -398,6 +417,8 @@ function userprotect_menu_alter(&$callbacks) {
+    $response = FALSE;
     // Check to see if the user's roles are protecting edits, or the user
     // account itself is protected.
     if (!userprotect_check_bypass('up_edit') && userprotect_get_user_protection($account, 'up_edit')) {
@@ -405,11 +426,15 @@ function userprotect_user_edit_access($account) {

@@ -405,11 +426,15 @@ function userprotect_user_edit_access($account) {
       if (arg(0) == 'user' && is_numeric(arg(1)) && arg(2) == 'edit') {
         drupal_set_message(t('%user is currently being protected from any edits.', array('%user' => $account->name)), 'error');
       }
-      return FALSE;
+      $response = FALSE;
     }
     else {
-      return TRUE;
+      $response = TRUE;

Is something setting $response to not be false or the two later $response = FALSE not needed? (and thus the entire last else {}).

+    $response = FALSE;
     // At this point, we only need the userprotect-specific validation if:
     //   1. The current user and the edited user are not the same.
     //   2. The current user is a user administrator.
@@ -438,15 +465,20 @@ function userprotect_user_cancel_access($account) {

@@ -438,15 +465,20 @@ function userprotect_user_cancel_access($account) {
         if (arg(0) == 'user' && is_numeric(arg(1)) && arg(2) == 'cancel') {
           drupal_set_message(t('%user is currently being protected from cancellation.', array('%user' => $account->name)), 'error');
         }
-        return FALSE;
+        $response = FALSE;
       }
       else {
-        return TRUE;
+        $response = TRUE;
       }

Same

My general opinion is that this a very non-standard way to do an access check (bad developer experience to have non-standard hooks). In most access hooks, the user returns the FALSE instead of overriding a variable.

This is my suggestion:

 foreach (module_implements('userprotect_access') as $module) {
  $function = $module . '_userprotect_access';
  $access = $function($op, $account);
  if (is_bool($access)) {
    return $access;
  }
}
 // fallback to userprotect's logic if no one has set anything.