WordPress - Design Problems

All software has a few problems.

Filters are called with a variable number of parameters, therefore you never know what will be available to your function
Any registered user has the ability to destroy the entire system !@#$%

Filters | "Tools" Security Hole | Deleting Users

Filters

Filters are called with varying numbers of parameters. In the case of the show_password_fields filter, if the function definition has 2 parameters, there will be an error when only one parameter is passed. (Actually, that is a good thing. The problem is that in one case 2 parameters are passed, and in the other only one is passed.)

  function hide_password_fields($value, $profileuser){ // Fails - can not define 2 parameters
  }

Instead, you must write code to accept a variable number of parameters.

// It is necessary to protect the administrator password and role from
// users that have the ability to edit other, lesser users
// it is unbelievable that this is not built-in

// apply_filters('show_password_fields', true, $profileuser); // from wp-admin\user-edit.php
// apply_filters('show_password_fields', true)                // from wp-admin\user-new.php

// add_filter('show_password_fields', array(&$this, 'hide_password_fields', 10, 2); // needs 2 parameters

// This is called with 1 or 2 parameters - piece of crap

  function hide_password_fields($value){ //, $profileuser){ // can not define 2 parameters
    global $current_user;
    $args = array_slice( func_get_args(), 1 );
    $profileuser = $args[0];

     // When a new user is added, $profileuser is null
    if (!$profileuser){return $value;}

    if (($current_user->has_cap('administrator')) || (!$profileuser->has_cap('administrator'))) {
      return $value;
    }else{
      return false;
    }
  }

When the filter is registered, be sure to specify that 2 parameters should be passed if they are available.

add_filter('show_password_fields', array(&$this, 'hide_password_fields', 10, 2); // needs 2 parameters

Security Hole

Every system user that has the ability to log on has the ability to destroy the system. (This is unbelievable.)

The read capability (available to all registered users) provides the ability to install Gears or the Press This applet via the Tools menu option.

This is a major security hole - there is even a warning not to install this on certain systems. This is something that should be reserved for only the administrator ... not every clueless user.

This code removes it for all users

     // caled via - add_action('admin_init', array(&$this, 'admin_init'));
  function admin_init(){

    // Remove the dangerous "Tools" menu
    global $menu;
    unset($menu[75]);
  }

This only removes the bad submenu, but does not completely stop the option from being available.

     // caled via - add_action('admin_init', array(&$this, 'admin_init'));
  function admin_init(){

    // Remove the dangerous "Tools" submenu
    global $submenu;
    unset($submenu['tools.php'][5]);
  }

Unfortunately, some plugins use that menu, something like the following is better

     // caled via - add_action('admin_init', array(&$this, 'admin_init'));
  function admin_init(){

    // Remove the dangerous "Tools" submenu
    global $menu, $current_user;
    if (!$current_user->has_cap('administrator'))
      unset($menu[75]);
  }

Deleting Users

Stupid morons!

When you delete a user, WordPress provides 2 options

Delete everything that the user ever entered
Assign the existing posts to another user

Unfortunately, the default is to delete everything.

At any rate, there is also a comboBox to select the user to reassign the posts to. (Don't get ahead of me :)

A customer, was deleting one user (a test account) and reassigning about a month's worth of data to a new account. This person selected the new account from the list and clicked Confirm Deletion ... but did not notice that the Delete everything in the world option was still selected. Three days later, it was noticed that some data was missing - about a month's worth. This was a very expensive error ... and a terrible design.

Anything this dangerous and so easy to mess up definitely qualifies as a design problem.

So I decided to delete the delete_users capability from the user roles. Get this, when a user does not have permission to delete user profiles, the code still places the Delete link in the list of users. The only prohibited action is that a user can not delete his own account. From wp-admin/includes/template.php

function user_row(..)
  if ( $current_user->ID != $user_object->ID )
    $actions['delete'] = "<a class='submitdelete' href='" 
      . wp_nonce_url("users.php?action=delete&user=$user_object->ID", 'bulk-users') . "'>" 
      . __('Delete') . "</a>";

  $actions = apply_filters('user_row_actions', $actions, $user_object);

Unbelievable. At least it eventually gives a warning if you actually click the link. My suggestion is to add a filter () and to manually remove the action. Something like this (which I never actually used, see below).

add_filter('user_row_actions', array(&$this, 'user_row_actions')); // called once for each user in the list

  function user_row_actions($actions, $user_object){
    if (!$current_user->has_cap('delete_users'))
      unset($actions['delete']);
  }

Trivial, but should not be necessary.

Of course, there are also the Bulk Actions ... these (there are 2) also contain Delete. And, of course, there are no filters to fix this mess. Thus, the only way to clean this up is to modify wp-admin/users.php. Personally, I don't see any advantage for users to be able to perform bulk delete on user profiles. Therefore, my fix is to simply comment out those lines of code. (I left in the bulk "change roles" option and the associated check boxes, though I don't think it is that useful.)

.....

Since I had to modify a core file anyway, I also changed wp-admin/includes/template.php (instead of using the filter, code folded to multiple lines to make it readable)

// patched 09-25-09 by R Clemenzi
//     removed user delete links for users that don't have that permission

//   if  ( $current_user->ID != $user_object->ID ) // old code 09-25-09 
     if (( $current_user->ID != $user_object->ID ) && ( $current_user->has_cap('delete_users') ))
         $actions['delete'] = "<a class='submitdelete' href='" 
              . wp_nonce_url("users.php?action=delete&user=$user_object->ID", 'bulk-users') . "'>" 
              . __('Delete') . "</a>";

Author: Robert Clemenzi