Always check capabilites in admin pages

[westi]

WP_List_Table introduces a check_permissions() function which hides away the capabilities check inside the list table class so that it is easy to write a generic AJAX handler.

We should still have current_user_can() checks in the normal admin pages as it makes it easier to review for security holes.

Still doing it in the table classes is good defence in depth.

[nacin]

[16222] never hit the ticket.

wpdavis on IRC mentioned that he was getting 'Cheatin', uh?' on edit-tags.php. I couldn't track it down for a while, but sure enough, $tax isn't actually set at that point, or if it is, then it's because of a variable collision. Fixing now.

[nacin]

Also, what if we had check_permissions() instead just return a cap to check? Then we can just call current_user_can( $wp_list_table->cap() ). That cuts out redundant cycles and also makes it *very* clear what's going on. (It's arguably confusing why we're doing it twice.) We're also no longer really just using strings for capabilities, given the cap objects for taxonomies and post types.

[nacin]

(In [16253]) Set $tax as the taxonomy object, and kill off our colliding global from menu construction. see #15326.

[westi]

I don't like the current_user_can( $wp_list_table->cap()) because it still hides away the required cap in a lot of cases.

[westi]

(In [16776]) Restore some more cap checks for clarity. See #15326.

[automattor]

(In [16779]) Revert brokenness. See #15326.

[westi]

(In [16965]) Move post_type var setup back into edit.php and reinstate the cap check. See #15326

[westi]

(In [16970]) More caps back. See #15326

[nacin]

(In [16990]) Remove check_permissions() calls outside of AJAX context. Also only check for switch_themes in check_permissions() for the themes table. see #15326.

[nacin]

(In [16991]) Move check_permissions() out of ajax_response(). see #15326.

[nacin]

Noticed some issues with how we were still doing things.

We need to change wp_die() in every check_permissions() method to die('-1').

15326.diff​ kills off check_permissions() entirely.

The new method is called ajax_user_can(). You return current_user_can() through it. In admin-ajax, we then die with -1 if appropriate.

Renaming it allows us to ensure that no one is using it incorrectly, as we'd be returning true/false rather than dying.

Conservative alternative: die with -1 in each handler. No need to rename the function.

[westi]

I really like ajax_user_can - I've been wanting to rename the function and this makes the checks even more obvious.

I hate hiding the dies/wp_dies in a function.

[nacin]

(In [16992]) Replace check_permissions() with ajax_user_can(). New method returns true/false to current_user_can(), which we then handle in admin ajax. see #15326.

[nacin]

Ticket is complete. I just want some sanity checks on [16990] and [16992] to make sure I didn't miss anything.

[nacin]

Needs final, close review by ryan.

[ryan]

Looks good.