[wp-trac] [WordPress Trac] #36376: current_user_can/has_cap fails when user has multiple roles

[WordPress Trac]

#36376: current_user_can/has_cap fails when user has multiple roles
-----------------------------------------------+---------------------------
 Reporter:  mikejolley                         |       Owner:
     Type:  defect (bug)                       |      Status:  new
 Priority:  normal                             |   Milestone:  Awaiting
Component:  Role/Capability                    |  Review
 Severity:  normal                             |     Version:
 Keywords:  has-unit-tests dev-feedback close  |  Resolution:
                                               |     Focuses:
-----------------------------------------------+---------------------------
Changes (by dd32):

 * keywords:  has-patch has-unit-tests dev-feedback => has-unit-tests dev-
     feedback close


Comment:

 This is a fun bug involving a very little known functionality of roles I
 believe. (I could be completely off track here though)

 WordPress roles are rather over complicated for how simple they look on
 the surface.
 In WordPress, a user can be assigned to any number of roles (A Role being
 a group of capabilities) AND any capabilities directly.
 Ie. User bob can be in the roles `Role1`, `Role2` and have the capability
 `Cap99`.

 Roles are groups of capabilities, but they're not just which capabilities
 that should be added to a user, they can also explicitly  deny a user that
 capability. ([attachment:fix.36376.diff] removes this functionality I
 believe).
 ie. `Role2` could specifically say `You shalt not have Cap99, even if you
 already do.`.

 The problem I think may be occurring here, is two fold - 1) Someones role
 is denying a cap, and the author didn't realise they were doing so and 2)
 the order of operations here is special.

 The following example demonstrates it kind of:
 {{{
 <?php

 add_role( 'has_test_cap', 'Has the "test" cap', array( 'test' => true ) );
 add_role( 'denied_test_cap', 'Denied the "test" cap', array( 'test' =>
 false ) );

 $user = new WP_User( 1 );

 // Verify no caps = should be false.
 var_dump( $user->has_cap( 'test' ) ); // FALSE (Correct)

 // Add cap via role
 $user->add_role( 'has_test_cap' );
 var_dump( $user->has_cap( 'test' ) ); // TRUE! (Correct)

 // Deny the cap via role
 $user->add_role( 'denied_test_cap' );
 var_dump( $user->has_cap( 'test' ) ); // FALSE! (Correct)

 //  -- User now has both roles, and is denied the capability.

 // Add the cap directly:
 $user->add_cap( 'test', true );
 var_dump( $user->has_cap( 'test' ) ); // TRUE! (Correct)

 //  -- User now has both roles AND the capability, and is allowed to
 perform the action

 // Cleanup
 $user->remove_role( 'has_test_cap' ); $user->remove_role(
 'denied_test_cap' ); $user->remove_cap( 'test' );
 echo '<hr>';

 // Lets add them in the reverse order now.

 // Verify no caps = should be false.
 var_dump( $user->has_cap( 'test' ) ); // FALSE (Correct)

 // Deny the cap via role
 $user->add_role( 'denied_test_cap' );
 var_dump( $user->has_cap( 'test' ) ); // FALSE! (correct)

 // Add cap via role
 $user->add_role( 'has_test_cap' );
 var_dump( $user->has_cap( 'test' ) ); // TRUE! (Correct)

 // -- User now has both roles, and is is allowed to perform the action.

 // Cleanup
 $user->remove_role( 'has_test_cap' ); $user->remove_role(
 'denied_test_cap' ); $user->remove_cap( 'test' );
 echo '<hr>';

 // Verify no caps = should be false.
 var_dump( $user->has_cap( 'test' ) ); // FALSE (Correct)

 // Add cap directly:
 $user->add_cap( 'test', true );
 var_dump( $user->has_cap( 'test' ) ); // TRUE! (Correct)

 // Deny the cap via role
 $user->add_role( 'denied_test_cap' );
 var_dump( $user->has_cap( 'test' ) ); // TRUE! Wait, what? (Correct)

 // -- User has denied_test_cap role, but yet is allowed to perform action
 as capabilities take precedence

 // Cleanup
 $user->remove_role( 'has_test_cap' ); $user->remove_role(
 'denied_test_cap' ); $user->remove_cap( 'test' );
 remove_role( 'has_test_cap' );
 remove_role( 'denied_test_cap' );
 }}}

 So clearly, the order to which the roles are assigned to a user is
 important, and it's likely that roles being applied in different orders
 when a role defines a capability as denied.

 I don't think there's a bug here, I think it's working as intended, even
 if a little funky at present.

 The issue boils down to:
  - If a user has a role which denies a capability, should it take
 precedence over all roles (current and future) which allow it?
 or
  - If a user has a role which denies a capability, it should only take the
 capability away from current roles, and future additional roles can allow
 it

 Based on the above, I'm fairly sure this ticket is `invalid` or `wontfix`.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/36376#comment:10>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform