Make WordPress Core

Opened 12 years ago

Closed 12 years ago

#21024 closed enhancement (fixed)

send_origin_headers for admin-ajax

Reported by: batmoo's profile batmoo Owned by: ryan's profile ryan
Milestone: 3.5 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch needs-testing commit
Focuses: Cc:

Description

admin-ajax should allow cross-domain requests for known domains using by sending the correct Access-Control-Allow-Origin headers using send_origin_headers().

Note that the pre-flighted OPTIONS request that browsers make to check if the origin is allowed, does not send the necessary params (specifically "action"), which means that admin-ajax's if ( empty( $_REQUEST['action'] ) ) check causes the request to fail so that needs to be accounted for.

We should also send the Access-Control-Allow-Credentials: true header to allow authenticated cross-domain requests via the withCredentials: true flag. Maybe this can be an argument for send_origin_headers?

Attachments (3)

21024.diff (1.3 KB) - added by nacin 12 years ago.
21024.2.diff (779 bytes) - added by nacin 12 years ago.
Option A
21024.3.diff (746 bytes) - added by nacin 12 years ago.
Option B

Download all attachments as: .zip

Change History (13)

#1 @nacin
12 years ago

  • Milestone changed from Awaiting Review to 3.5

We discussed this during 3.4 — this is a very good idea.

#2 @nacin
12 years ago

send_origin_headers() already sends Access-Control-Allow-Credentials: true automatically, so that's easy.

In order to get around the check for $_REQUEST['action'] we could detect the OPTIONS request, or if just move the check to after wp-load.php and send_origin_headers().

I wonder if send_origin_headers() should be issuing a die() if the request method is OPTIONS. Otherwise, I'm pretty sure, we risk executing a request twice. Not a big issue when dealing with previews in the customizer, but certainly a problem with many/most/all ajax requests.

@nacin
12 years ago

#3 @nacin
12 years ago

  • Keywords has-patch needs-testing added

21024.diff is my attempt to implement a simplified version of the attempted server-side access control listed here on MDN: https://developer.mozilla.org/En/Server-Side_Access_Control. Unverified, untested.

#4 @johnjamesjacoby
12 years ago

  • Cc johnjamesjacoby added

#5 @ryan
12 years ago

  • Owner set to ryan
  • Resolution set to fixed
  • Status changed from new to closed

In [21988]:

Handle pre-flighted OPTIONS requests in send_origin_headers(). Props nacin. fixes #21024

#6 @ryan
12 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

And now for the admin-ajax bits.

@nacin
12 years ago

Option A

@nacin
12 years ago

Option B

#7 @nacin
12 years ago

  • Status changed from reopened to assigned

Two options. Check twice for $_REQUEST['action'], that way we can bail for non-OPTIONS requests before wp-load.php runs. Or just ignore the potential benefit to not loading WordPress.

#8 @nacin
12 years ago

  • Keywords commit added

Commit candidate. Needs proper testing. Leaving this up to ryan.

#9 @ryan
12 years ago

Perhaps B so that a pre-flighted request and a regular request both send some headers when action is missing.

#10 @ryan
12 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In [22001]:

Call send_origin_headers() from admin-ajax.php. Props nacin. fixes #21024

Note: See TracTickets for help on using tickets.