Make WordPress Core

Opened 10 years ago

Last modified 3 years ago

#28212 new defect (bug)

determine_current_user filter with priority <10 gets overridden

Reported by: rmccue's profile rmccue Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.9
Component: Login and Registration Keywords: needs-testing needs-refresh
Focuses: rest-api Cc:

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 9 years ago.
28212.2.patch (1.4 KB) - added by skeemer 9 years ago.
Replacement with fix for other overriding condition.

Download all attachments as: .zip

Change History (9)

#1 @rmccue
10 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
9 years ago

  • Component changed from General to Users

@skeemer
9 years ago

#3 @skeemer
9 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
9 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
9 years ago

Replacement with fix for other overriding condition.

#5 @dshanske
6 years 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.


5 years ago

#7 @TimothyBlynJacobs
3 years ago

  • Component changed from REST API to Login and Registration
  • Focuses rest-api added

Moving this to Login & Registration. We were able to ship App Passwords without this.

Related #40595, #46748.

Note: See TracTickets for help on using tickets.