#19791 closed defect (bug) (wontfix)
can't correctly grant certain capabilities to roles on multisite
Reported by: | jtclarke | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 3.0 |
Component: | Role/Capability | Keywords: | has-patch |
Focuses: | Cc: |
Description
I noticed after I'd granted a particular role the capability unfilitered_html, they were still unable to save unfiltered html in the default Text widget. In this case, when the user tried to save an iframe, the iframe was stripped out on save.
I double checked that the user had the role i thought they did, and that their role had the unfiltered_html cap. The user, importantly, is not a super admin.
I tracked down this behavior to wp-includes/capabilities.php:
On a multisite install, the map_meta_cap function gives somewhat unpredictable behavior for users who are not superadmins.
In the function below (pulled from wp-includes/capabilties.php) the following capabilities all fall through to
the delete_users check:
- edit_themes
- update_plugins
- delete_plugins
- install_plugins
- update_themes
- delete_themes
- install_themes
- update_core
- unfiltered_html
function map_meta_cap( $cap, $user_id ) { $args = array_slice( func_get_args(), 2 ); $caps = array(); switch ( $cap ) { ... case 'edit_themes': if ( defined('DISALLOW_FILE_EDIT') && DISALLOW_FILE_EDIT ) { $caps[] = 'do_not_allow'; break; } // Fall through if not DISALLOW_FILE_EDIT. case 'update_plugins': case 'delete_plugins': case 'install_plugins': case 'update_themes': case 'delete_themes': case 'install_themes': case 'update_core': // Disallow anything that creates, deletes, or edits core, plugin, or theme files. // Files in uploads are excepted. if ( defined('DISALLOW_FILE_MODS') && DISALLOW_FILE_MODS ) { $caps[] = 'do_not_allow'; break; } // Fall through if not DISALLOW_FILE_MODS. case 'unfiltered_html': // Disallow unfiltered_html for all users, even admins and super admins. if ( defined('DISALLOW_UNFILTERED_HTML') && DISALLOW_UNFILTERED_HTML ) { $caps[] = 'do_not_allow'; break; } // Fall through if not DISALLOW_UNFILTERED_HTML case 'delete_user': case 'delete_users': // If multisite these caps are allowed only for super admins. if ( is_multisite() && !is_super_admin( $user_id ) ) { $caps[] = 'do_not_allow'; } else { if ( 'delete_user' == $cap ) $cap = 'delete_users'; $caps[] = $cap; } break; case 'create_users': if ( !is_multisite() ) $caps[] = $cap; elseif ( is_super_admin() || get_site_option( 'add_new_users' ) ) $caps[] = $cap; else $caps[] = 'do_not_allow'; break; default: // Handle meta capabilities for custom post types. $post_type_meta_caps = _post_type_meta_capabilities(); if ( isset( $post_type_meta_caps[ $cap ] ) ) { $args = array_merge( array( $post_type_meta_caps[ $cap ], $user_id ), $args ); return call_user_func_array( 'map_meta_cap', $args ); } // If no meta caps match, return the original cap. $caps[] = $cap; } return apply_filters('map_meta_cap', $caps, $cap, $user_id, $args); }
Which basically means all of these checks will fall through to 'do_not_allow' -- even if the role has specifically been granted those privileges -- if any of the constants in those blocks are not set. And if they ARE set, you could potentially have an edit_themes check on line 1117 fall through to the DISALLOW_UNFILTERED_HTML constant check on line 1139.
This doesn't seem like the desired behavior.
I think we can solve the problem by moving the delete_user and create_user checks -- so that these other checks are actually allowed to fall through to the default.
Attachments (2)
Change History (11)
#3
@
13 years ago
actually -- ignore the first patch. That doesn't solve the fall through problem for the constants issue.
Attaching a new patch.
#5
@
13 years ago
ug. sorry -- that patch was whack. We use spaces instead of tabs at my company.
this final patch is correct.
#6
@
12 years ago
@jtclarke - Thanks for posting this bug and patch.
However, I disagree with the solution you posted as quoted below:
moving the delete_user and create_user checks -- so that these other checks are actually allowed to fall through to the default.
Why allow capability checks for Managing Themes, Plugins, and update_core to fall through to the default checks? As it stands, these checks do not automatically fall through to the default case and may result in unexpected results.
I've submitted an alternate patch to simply remove the fall through logic from the two case match sets below:
First Case Match Set: Manage Themes / Plugins / Update Core
case 'update_plugins': case 'delete_plugins': case 'install_plugins': case 'update_themes': case 'delete_themes': case 'install_themes': case 'update_core':
Second Case Match Set: Save Unfiltered HTML
case 'unfiltered_html':
The First Case Match should not fall through to Second Case Match (Unfiltered HTML). Likewise, the Second Case Match (Unfiltered HTML) should not fall through to the next case match sets checking for Deleting and Creating Users capabilities.
Thanks,
David Carroll
#7
@
12 years ago
- Milestone changed from 3.4 to Future Release
Punting, as this is a non-trivial and non-new issue.
#8
@
12 years ago
- Milestone Future Release deleted
- Resolution set to wontfix
- Status changed from new to closed
unfiltered_html is granted to editors and administrators. We do not modify our role definitions in multisite. Rather, we forcibly filter it here -- if you are not a super admin, you don't get unfiltered HTML, period. However, you *can* grant unfiltered_html, but it requires filtering on map_meta_cap for unfiltered_html and removing do_not_allow from the array (and adding back unfiltered_html). This is pretty simple.
In my comment above moving this to 3.4, I was referring to a bug related to the same area of code, but unrelated. I will open a separate ticket.
Yeah, I've noticed this offhandedly.