christianwach / civicrm-admin-utilities

CiviCRM Admin Utilities is a WordPress plugin that modifies and enhances CiviCRM's appearance and behaviour in single site and multisite WordPress installs.
https://wordpress.org/plugins/civicrm-admin-utilities/
GNU General Public License v2.0
20 stars 10 forks source link

Incorrect permission check on Manage Groups in Shortcut menu #8

Closed kcristiano closed 5 years ago

kcristiano commented 5 years ago

From CiviCRM Mattermost:

"For the CAU shortcut in WP Admin, I'd suggest removing Manage Groups from that list. Users who do not have administer CiviCRM still see it. Actually as a super admin I do not see it, but lower level users do Not sure how it works."

civi shortcut

christianwach commented 5 years ago

@kcristiano Thanks for opening this issue.

I can't find a capability that determines access to the "Manage Groups" page so I've decided to add a filter to CAU such that people can decide for themselves whether to show the item at all:

// Deny access to everyone.
add_filter( 'civicrm_admin_utilities_manage_groups_menu_item', '__return_false' );

Or alternatively -- if they decide to show it -- who to show it to:

// Deny access to some users by user ID.
function my_manage_groups_is_allowed( $allowed ) {
    $users_to_deny = array( 123, 234, 345, 456 );
    $user = wp_get_current_user();
    if ( $user && in_array( $user->ID, $users_to_deny ) ) {
        $allowed = false;
    }
    return $allowed;
}
add_filter( 'civicrm_admin_utilities_manage_groups_menu_item', 'my_manage_groups_is_allowed' );

Obviously their code will be more cautious about checking the user object and so forth, but this at least means that the appearance of the menu item can now be controlled.

FWIW, I will probably add a setting to enable/disable this item - but more nuanced access control will require the use of this filter.

andyburnsco commented 5 years ago

Thanks, it's working.

For the deny by user ID filter, the inverse approach would be better suited. Meaning allow certain user ID's to see it. No big deal though, thanks for addressing and closing a security loophole :)

christianwach commented 5 years ago

For the deny by user ID filter, the inverse approach would be better suited.

@andyburnsco Yup, for sure. It's merely sample code showing how to use the filter. Glad it's working as expected for you :-)