WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 3 months ago

#28212 new defect (bug)

determine_current_user filter with priority <10 gets overridden

Reported by: rmccue Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.9
Component: REST API Keywords: needs-testing needs-refresh
Focuses: Cc:
PR Number:

Description

Introduced in #26706.

wp_validate_auth_cookie was shoehorned into this filter, and as such, doesn't return what the filter expects. On any error at all, it returns false, even if the "error" is that the cookie isn't set. If a function hooked into a lower priority (i.e. <10) returns a user ID, this will then be overridden by the built-in auth cookie.

Attachments (2)

28212.patch (881 bytes) - added by skeemer 5 years ago.
28212.2.patch (1.4 KB) - added by skeemer 5 years ago.
Replacement with fix for other overriding condition.

Download all attachments as: .zip

Change History (8)

#1 @rmccue
5 years ago

Oh, and to reproduce:

add_filter( 'determine_current_user', function ( $user ) {
	return 1;
}, 0 );

Expected result: Always logged in as user 1.
Actual result: Still uses cookie authentication.

#2 @DrewAPicture
5 years ago

  • Component changed from General to Users

@skeemer
5 years ago

#3 @skeemer
5 years ago

It appears that #26706 had some extra code. It added two filters. The first callable doesn't take in and output the user id, but cookie info. The second callable uses the function from first addition correctly.

#4 @skeemer
5 years ago

There's an additional condition with a lower priority being ignored if it determines that there should not be a user. With the following filter added, you can still be logged in.

add_filter( 'determine_current_user', function ( $user ) {
  return 0;
}, 0 );

@skeemer
5 years ago

Replacement with fix for other overriding condition.

#5 @dshanske
14 months ago

  • Component changed from Users to REST API
  • Keywords needs-testing needs-refresh added

I'm moving this old ticket into the REST API component on the basis that fixing this would be necessary for alternative authentication. If a client is setting credentials be it basic auth, a bearer token, what have you, that should go before cookie authentication.

This ticket was mentioned in Slack in #core-restapi by timothybjacobs. View the logs.


3 months ago

Note: See TracTickets for help on using tickets.