#52600 closed enhancement (fixed)
wp_nonce_ays('log-out') is confusing
Reported by: | david.kryzaniak | Owned by: | peterwilsoncc |
---|---|---|---|
Milestone: | 5.9 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Login and Registration | Keywords: | has-patch dev-feedback |
Focuses: | Cc: |
Description
When calling wp_nonce_ays('log-out')
the page title is "Something went wrong." and the HTTP status is 403. That is a little confusing. I think a 200 response and "You are attempting to log out of %s" for the title makes more sense. Thoughts?
Attachments (2)
Change History (14)
This ticket was mentioned in Slack in #core by peterwilsoncc. View the logs.
4 years ago
#3
@
4 years ago
- Component changed from General to Login and Registration
Hi @davidkryzaniak and welcome to trac!
Improving this messaging makes sense to me.
The next version of WordPress (5.8) is in feature freeze at the moment, so I will put this on the following milestone for visibility.
Thanks.
#5
@
3 years ago
The attempt to logout messaging makes sense to me. +1 improvement
What about the response code? Should an attempt to log out that lands on that page be a 403 or 200?
Some history:
- Prior to 2.9, the response code was 500 (Internal Server Error), i.e. the default in
wp_die()
. - Then in #11289 [12309], the response code was changed to 403 (Forbidden).
Expired link makes sense as a 403. What about when attempting to logout and landing on an are you sure page?
@peterwilsoncc What do you think?
Code Review notes:
- I'd suggest being intentional with the double assignment by changing
$title = $html = sprintf(
to$title = $html;
just below where$html
is assigned. - Alignment of variable assignments
Other than that, the code itself looks good.
#6
@
3 years ago
I agree with @hellofromTonya, on each front:
WP coding standards avoid double assignments for reasons of clarity.
The 403 HTTP response makes sense as the message should only be shown if the nonce has expired. If the nonce is valid, then the confirmation screen is bypassed.
Presuming a nonce of 9a9b9c9d9e
the following will not show the confirmation screen:
http://example.com/wp-login.php?action=logout&_wpnonce=9a9b9c9d9e
The following URLs will show a confirmation screen due to the invalid nonce:
http://example.com/wp-login.php?action=logout&_wpnonce=1a1b1c1d1e
http://example.com/wp-login.php?action=logout
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
3 years ago
#8
follow-up:
↓ 10
@
3 years ago
@peterwilsoncc is this one ready for commit (except for those 2 coding standard needs)?
This ticket was mentioned in PR #1854 on WordPress/wordpress-develop by peterwilsoncc.
3 years ago
#9
#10
in reply to:
↑ 8
@
3 years ago
Replying to hellofromTonya:
@peterwilsoncc is this one ready for commit (except for those 2 coding standard needs)?
Yes, I think it is ready.
The linked PR contains the changes discussed above. Are you able to take a quick look at the code while the tests run and I can commit once they complete.
#11
@
3 years ago
- Owner set to peterwilsoncc
- Resolution set to fixed
- Status changed from new to closed
In 52088:
fixed spacing