WordPress.org
Get WordPress

Make WordPress Core

Opened 9 years ago

Closed 9 years ago

#32207 closed enhancement (fixed)

check_admin_referer action only fires on success

Reported by: markjaquith's profile markjaquith Owned by: markjaquith's profile markjaquith
Milestone: 4.3 Priority: low
Severity: minor Version: 1.5.1
Component: Administration Keywords: has-patch
Focuses: Cc:

Description

The check_admin_referer action in check_admin_referer() fires after the conditional die(). So there is no way to use this hook to log nonce failures in the admin.

I think we could move the action above the if/die() portion without causing any trouble.

Attachments (1)

32207.diff​ (1.0 KB) - added by markjaquith 9 years ago.

Download all attachments as: .zip

Change History (7)

@markjaquith
9 years ago

#1 @markjaquith
9 years ago

  • Keywords has-patch 2nd-opinion added; needs-patch removed
  • Owner set to markjaquith
  • Status changed from new to accepted

Like so.

Further reasoning on the move being okay: it passes $result. So people should have been checking that all along. I doubt anyone was assuming "this hook running means a nonce success".

#2 @MikeHansenMe
9 years ago

  • Keywords 2nd-opinion removed

Looks good to me.

#3 @johnbillion
9 years ago

  • Version changed from trunk to 1.5.1

#4 @obenland
9 years ago

Can we make a decision on this until beta1 next week?

This ticket was mentioned in ​Slack in #core by helen. ​View the logs.
9 years ago

#6 @helen
9 years ago

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

In 33017:

Fire the check_admin_referer action on failure as well as success.

This enables things like logging nonce failures in the admin.

props markjaquith.
fixes #32207.

Note: See TracTickets for help on using tickets.