WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 6 weeks ago

#14530 reopened enhancement

"Cheatin', uh?" is not helpful feedback for users or developers

Reported by: shidouhikari Owned by:
Milestone: Future Release Priority: normal
Severity: minor Version:
Component: Role/Capability Keywords: has-patch
Focuses: ui Cc:

Description

I've sen this infamous error msg more times than I wanted, in my own site where I'm admin.

That happens with more frequency when adding a new comment, but some times also inside admin pages.

I understand it's generally capability and permission tests that fail, and that happens in pages that ppl without permission to access them shouldn't see links to them, therefore they probably tried direct access to somewhere they shouldn't be going.

But also it happens when session expires or due to some bug. In this cases, the user is effectively not doing anything wrong, it may even be WP fault, and when that's the case the message is rude, even offensive to more emotional ppl.

Even worse, it gives no hint on what went wrong, so that user can try to fix it.

Then I suggest these messages to be changed, to more meaningful and also polite messages. Real cheaters and hackers will already have any info a message may provide, so a better explanation of what went wrong won't help them succeed in their attempt to hack a site, and will help a lot the victims of these errors.

Attachments (10)

chaetin.patch (13.9 KB) - added by mrmist 5 years ago.
Replace the cheatin message with a login message
14530-1.patch (13.6 KB) - added by kraftbj 2 years ago.
Change to You do not have sufficient permissions to access this page.
14530-2.patch (13.2 KB) - added by kraftbj 2 years ago.
Change to You do not have permission to view this page.
14530-3.patch (13.8 KB) - added by kraftbj 2 years ago.
Change to A permissions error occurred while attempting to access this page.
14530.diff (514 bytes) - added by kraftbj 10 months ago.
Update the error message for the Customizer
14530.2.diff (755 bytes) - added by ericlewis 4 months ago.
14530.2.png (98.9 KB) - added by ericlewis 4 months ago.
14530-4.patch (13.5 KB) - added by lukecarbis 2 months ago.
Change "Cheatin', uh?" to "You do not have permission to access this page."
14530.3.diff (18.1 KB) - added by ericlewis 2 months ago.
14530.4.diff (18.0 KB) - added by ericlewis 2 months ago.

Download all attachments as: .zip

Change History (65)

comment:1 @mrmist5 years ago

  • Owner set to mrmist
  • Status changed from new to accepted

Not a problem I've come across, but I agree with the sentiment. I'll see if I can put something together for it.

comment:2 @nacin5 years ago

This warning should never be accessible via the UI. These are nothing more than sanity checks. If they can be accessed in a normal setup via the UI then that is a bug.

@mrmist5 years ago

Replace the cheatin message with a login message

comment:3 @mrmist5 years ago

  • Keywords has-patch added

Having checked this out, it's more difficult than I imagine to actually get one of these warnings. Nevertheless, on the off-chance that it's accessed by a valid user typing in a URL for a random page they don't have access to, attached patch with a more explanatory error.

comment:4 @markmcwilliams5 years ago

Upon looking at the attached patch, would it be better to phrase the text as ... 'There was a problem loading this page, you may not have the necessary permissions, or may need to <a href="' . get_option('siteurl') . '/wp-login.php">' . __('login') . '</a> again?' ... so basically adding the again? after, more, if anything to make it make a little more sense?

comment:5 @mrmist5 years ago

I did do it like that (with "again") originally, but on my screen it created word-wrap to the next line, which I didn't like, hence the way it is now. Trivial to do either way, really.

comment:6 @mrmist5 years ago

  • Owner mrmist deleted
  • Status changed from accepted to assigned

comment:7 @nacin4 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from assigned to closed

Per conversation in IRC from a few months ago, I'm going to close this as wontfix.

As I said, these are sanity checks. They're impossible to reach via the UI. (If they were, that would be a bug.) You need to hit a URL that you specifically don't have access to. You need to be logged in as well, so the link to wp-login is invalid.

comment:8 @kraftbj2 years ago

  • Cc bk@… added
  • Resolution wontfix deleted
  • Status changed from closed to reopened

I'd like to request reconsideration of this ticket.

While rare, when encountered it gives an unprofessional appearance of WordPress. I had a contact from a perspective client who ditched their previous consultant primarily because, after seeing this error message, assumed the guy was doing something shading on his site.

I grant that for it to appear, the consultant was probably doing something wrong, but that isn't the point.

Despite the rarity, is there a reason that it should be kept at "Cheatin'" and not something else?

To recreate message:

  1. Log in to /wp-admin/ as privileged user (administrator, editor), etc. Leave that tab alone.
  2. In separate tab, visit /wp-login.php and login as a Subscriber user.
  3. In original tab, visit the Categories or Tags link (Posts->Tags, etc).

That workflow isn't common, but I could foresee someone in IT showing off a feature of WordPress to someone on the business side of a company and getting that message after mistakenly switching to the wrong tab/not re-logging in first/etc.

Three ideas:

  1. Use the standard permissions error message ("You do not have sufficient permissions to access this page.") and reduce a string for the polyglots.
  2. Use something slightly different to keep it unique to the cheater check. ("You do not have permission to view this page.")
  3. Something more unique ("A permissions error occurred while attempting to access this page.") to help polyglots keep it different.

Patches forthcoming for the three options.

tl;dr -- Sterilize the error a bit to make it more palatable to folks who don't understand our sense of humor. Nothing more needed.

Version 0, edited 2 years ago by kraftbj (next)

@kraftbj2 years ago

Change to You do not have sufficient permissions to access this page.

@kraftbj2 years ago

Change to You do not have permission to view this page.

@kraftbj2 years ago

Change to A permissions error occurred while attempting to access this page.

comment:9 @travisnorthcutt2 years ago

  • Cc travis@… added

+1 on this. I don't see any particular reason not to replace the (snarky, IMO) current message with something more explicit about what's going on.

comment:10 @SergeyBiryukov2 years ago

  • Milestone set to Awaiting Review

comment:11 @johnbillion2 years ago

This message would also benefit from having a link back to either the site home page, or the admin dashboard, so the message isn't such a dead end.

comment:12 @kraftbj2 years ago

John-- I think you're right, but I think we should do that under a different ticket. The other "permission errors" do not include a link back, so my two cents is let's get the text changed, and focus another ticket on the link back.

A quick look at http://core.trac.wordpress.org/browser/tags/3.5.1/wp-includes/functions.php#L2049 tells me there is a 'back_link' ability, so, without knowing the code better, may be another fix that is more proper than changing the text string.

comment:13 @Dorian Speed2 years ago

  • Cc Dorian Speed added

I agree - I have been in the same situation of having a frustrated client encounter that message and assume there's something shady going on with his website. I like "A permissions error occurred while attempting to access this page."

comment:14 @kraftbj2 years ago

FWIW, I'm helping out over on the WordPress.com forums and an user had the Cheatin' message appear. I think it was a similar reason as described in Comment 8.

http://en.forums.wordpress.com/topic/i-want-my-url-to-be-thisonetreewordpresscom-its-attached-to-my-other-email?replies=13#post-1257253

Despite the rarity, I think a more descriptive message would be a better UX.

comment:15 follow-up: @tzkmx21 months ago

Hi, I'm getting this error "message" trying to activate some child themes. The funny part is they are all child of the same parent theme, and just one of the child themes loads in the customize screen, all others die into the "Cheatin' uh?" "message".

Same user, same permissions. The only bit I found wp_die function in customize.php is this:

if ( ! current_user_can( 'edit_theme_options' ) )
        wp_die( __( 'Cheatin&#8217; uh?' ) );

So, why is this? Have I different permissions according to the theme? Is there only one child theme allowed for any parent theme in a given WP installation?

FWIW, we are trying to setup several child themes for the same parent theme in network aimed to teach a few people how to extend WP themes without messing other people work.

comment:16 in reply to: ↑ 15 @tzkmx21 months ago

Replying to tzkmx:

Same user, same permissions. The only bit I found wp_die function in customize.php is this:

if ( ! current_user_can( 'edit_theme_options' ) )
        wp_die( __( 'Cheatin&#8217; uh?' ) );

My bad, we were creating the stylesheets and template names were correct, but the child themes we couldn't activate, were those we haven't activated for the network yet. And we were seeing anyway those themes listed on every site, because a function on Hyper Admins plugin listed it.

I'd like to propose a patch to split the error message on permission, not only on edit_theme_options capability, but checking if the theme is enabled and giving the user the advice to enable for the site first. However I don't know which function could be used, if I discover a way to accomplish this, I'd be more helpful on this thread than just complaining about an uninformative error message.

comment:18 @kraftbj10 months ago

I'm going to mostly flip-flop on this ticket.

I'm okay wontfix'ing it. The error is really difficult to throw except in two situations:

  1. Edit Tags/Categories
  2. Customizer

In instance 1, it should be throwing "You do not have sufficient permissions to access this page" ( https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/menu.php#L317 ), except $pagenow includes the query string (e.g. edit-tags.php?taxonomoy=category) while the $*_nopriv includes only edit-tags.php. (ref: https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/plugin.php#L1637 ).

That is the root issue that should be addresses, IMO.

Instance 2: The Customizer doesn't use the admin menu, so it skips the check that is "misfiring" from instance 1. Changing the error text for the Customizer would be nice.

Attaching a patch to only change for the Customizer.

@kraftbj10 months ago

Update the error message for the Customizer

comment:19 @SergeyBiryukov8 months ago

#29253 was marked as a duplicate.

comment:20 @guidod7 months ago

This happens often when an active user gets its permissions limited while inside the backend. A simple refresh or menu click results in a "Cheating, huh?" message, which is indeed rude and confusing for typical users, when it should actually read something like:

"You don't have permission to access this page. If you think this is a mistake, contact your administrator. [link to dashboard]"

comment:21 @ericlewis6 months ago

  • Focuses ui accessibility added
  • Keywords needs-patch added; has-patch removed
  • Type changed from defect (bug) to enhancement

The ability to debug is hampered with such a cryptic, blanket message for permission errors, as well as users understanding the software that they're using.

We should state clearly what the lacking capability. i.e. "Your user account does not have permission to edit theme options." "Your user account does not have permission to edit terms."

comment:22 @ircbot6 months ago

This ticket was mentioned in IRC in #wordpress-dev by ericandrewlewis. View the logs.

comment:23 @slackbot5 months ago

This ticket was mentioned in Slack in #forums by ceo. View the logs.

comment:24 @slackbot5 months ago

This ticket was mentioned in Slack in #forums by ceo. View the logs.

comment:25 @ericlewis4 months ago

  • Milestone changed from Awaiting Review to 4.2

Let's get after this.

comment:26 @ericlewis4 months ago

  • Summary changed from Cheating huh? to "Cheatin' huh?" is not helpful feedback for users or developers

comment:27 @ericlewis4 months ago

  • Summary changed from "Cheatin' huh?" is not helpful feedback for users or developers to "Cheatin', uh?" is not helpful feedback for users or developers

comment:28 @ericlewis4 months ago

Related: #30798 to ponder other wp_die() messages usefulness.

comment:29 follow-up: @valendesigns4 months ago

Could we possibly add a server log message, so that at the very least we can debug the issue as it arises? I must say that this is more common of a page to see than some of you are saying. Especially when you're developing as a CMS and your CPT's & taxonomies are still in flux, as an example. I propose we do the following.

  1. Switch to a more friendly message for all instances of "Cheatin' huh?".
  2. Add detailed server log, or _doing_it_wrong, messages where applicable.

comment:30 @joedolson4 months ago

  • Focuses accessibility removed

comment:31 @slackbot4 months ago

This ticket was mentioned in Slack in #core by eric. View the logs.

comment:32 in reply to: ↑ 29 @ericlewis4 months ago

Replying to valendesigns:

Could we possibly add a server log message, so that at the very least we can debug the issue as it arises?

I'd say let's wait a little to see if #28319 produces a more robust method for logging application-level errors.

@ericlewis4 months ago

@ericlewis4 months ago

comment:33 @ericlewis4 months ago

In attachment:14530.2.diff, a prototype of a more helpful message.

As this is typically a developer-facing error, this will assist in debugging.

If a user ends up seeing this message, this may still be helpful, helping them realize they're dealing with a permissions error which is in their control. At the very least, it is not cheeky/unprofessional.

I include the exact file and line number in loving memory of my previous debug sessions against this screen.

comment:34 follow-up: @helen4 months ago

Acknowledging that you labeled it a prototype, no full path disclosure, please. :)

comment:35 in reply to: ↑ 34 @ericlewis4 months ago

Replying to helen:

Acknowledging that you labeled it a prototype, no full path disclosure, please. :)

Good point - how about ABSPATH-relative? e.g. wp-admin/edit-comments.php

comment:36 @slackbot4 months ago

This ticket was mentioned in Slack in #core by eric. View the logs.

comment:37 @slackbot3 months ago

This ticket was mentioned in Slack in #core by eric. View the logs.

comment:38 follow-ups: @jorbin3 months ago

"Cheatin', uh?" to me, is a core part of the flavor of WordPress. I wouldn't want to lose it and become a stuffy corporate application. I do think that there is a useful point that we can make this experience better for developers though, so I think we should create a standard filter and pass in the apporiate things so that a message like the one eric is proposing is possible. Essentially, something like this:

$user = wp_get_current_user(); 
$message = apply_filter( 'no_go_message', 'Cheatin&#8217; uh?', $user, $failed_cap, __FILE__ );
wp_die( $message, 403 );

Then, much like we have the debug bar plugin, someone could create a debug access plugin that turned the message into one that works for developing while not changing the flavor of WordPress.

comment:39 follow-up: @helen3 months ago

I wonder if we could add a clearer error message underneath when WP_DEBUG is on.

comment:40 in reply to: ↑ 38 ; follow-up: @guidod3 months ago

The usefulness of a better error message is not just for developers, it's for end users as well. For example, a suscriber or editor that has a permission removed while he is browsing the admin panel would now get "Cheating, huh?" instead of something helpful like "You don't have permission to access this page, perhaps your permissions have changed?".

(And about the stuffy corporate application comment, WordPress IS used in corporate contexts).

Replying to jorbin:

"Cheatin', uh?" to me, is a core part of the flavor of WordPress. I wouldn't want to lose it and become a stuffy corporate application. I do think that there is a useful point that we can make this experience better for developers though, so I think we should create a standard filter and pass in the apporiate things so that a message like the one eric is proposing is possible. Essentially, something like this:

$user = wp_get_current_user(); 
$message = apply_filter( 'no_go_message', 'Cheatin&#8217; uh?', $user, $failed_cap, __FILE__ );
wp_die( $message, 403 );

Then, much like we have the debug bar plugin, someone could create a debug access plugin that turned the message into one that works for developing while not changing the flavor of WordPress.

comment:41 in reply to: ↑ 38 ; follow-up: @ericlewis3 months ago

Replying to jorbin:

"Cheatin', uh?" to me, is a core part of the flavor of WordPress. I wouldn't want to lose it and become a stuffy corporate application.

If we want to sustain the flavor, let's insert a cheeky "something's messed up" image. As guidod notes, this is often a user-facing error, so surfacing a helpful error message can help users solve a capability issue - e.g. "oh, this tells us Jenn doesn't have the capability edit_theme_options. Let's elevate her role to Administrator."

comment:42 in reply to: ↑ 41 @jorbin3 months ago

Replying to ericlewis:

"oh, this tells us Jenn doesn't have the capability edit_theme_options. Let's elevate her role to Administrator."

A filter that you can then choose to log or trigger a message to an admin, or send an event into graphite, etc would enable you to solve this. Changing the message wouldn't.

comment:43 in reply to: ↑ 40 ; follow-up: @jorbin3 months ago

Replying to guidod:

The usefulness of a better error message is not just for developers, it's for end users as well. For example, a suscriber or editor that has a permission removed while he is browsing the admin panel would now get "Cheating, huh?" instead of something helpful like "You don't have permission to access this page, perhaps your permissions have changed?".


As was pointed out in the second comment of this thread. "This warning should never be accessible via the UI. These are nothing more than sanity checks. If they can be accessed in a normal setup via the UI then that is a bug."

(And about the stuffy corporate application comment, WordPress IS used in corporate contexts).

It is, and we haven't needed to lose the personality of WordPress to do so.

comment:44 in reply to: ↑ 43 @ericlewis3 months ago

Replying to jorbin:

As was pointed out in the second comment of this thread. "This warning should never be accessible via the UI. These are nothing more than sanity checks. If they can be accessed in a normal setup via the UI then that is a bug."

[In an imaginary organization's Slack]
Jenn: How do I edit the header image on our website?
Tom: Go here: http://site.com/wp-admin/customize.php
Jenn: Uh that page just says "Cheatin', uh?" Do you know what that means?

comment:45 in reply to: ↑ 39 @rmccue3 months ago

Replying to helen:

I wonder if we could add a clearer error message underneath when WP_DEBUG is on.

This is my exact thought, since we could handle this without losing the personality here.

Something like...

if ( WP_DEBUG ) {
    $message .= '<p>' . sprintf( __( "Psst, <code>%s</code> does not have the <code>%s</code> capability." ), $user, $cap ) . '</p>';
}

Alternatively, we could also do it in a less specific way for all users:

$message .= '<p>' . __( "Psst, you don't have permission to access this page." ) . '</p>';

I'm definitely -1 on showing the actual permission name though for non-debug, as we don't expose these anywhere (roles are our user-facing abstraction for these).

comment:46 follow-up: @jorbin3 months ago

I like the idea of the improved message with WP_DEBUG enabled, but the more I think about it, the more I think this is an event that should be capturable since as has been pointed out multiple times, this message should never be seen by a user and if they are seeing it, it is a bug. Making this capturable would allow a logger (such as the one proposed in #30934 ) to make this bug one a developer could track down and fix.

rmccue is absolutely correct, we don't expose capabilities in the UI anywhere, I don't think we should start.

comment:47 in reply to: ↑ 46 @guidod3 months ago

If they can be accessed in a normal setup via the UI then that is a bug

By that logic, 404s are bugs too, but that doesn't mean they shouldn't be translated to a friendly message to end users. Otherwise, let's replace the template_redirection to 404.php with a "Cheating, uh?" as well.

This should NOT be a debug message. As Eric Lewis points out, this is required information for anyone that follows a URL, either bookmarked or sent by another user of the WordPress instance, that he can't access. This is, in fact, unlikely to happen on development and debug instances; it confuses users on PRODUCTION sites.

Replying to jorbin:

rmccue is absolutely correct, we don't expose capabilities in the UI anywhere, I don't think we should start.

But we expose roles, and users. If the role field isn't called "Uh huh foo-bar" then the role/permissions-related error message shouldn't read "Cheating, uh?".

comment:48 @SergeyBiryukov3 months ago

Replying to jorbin:

"Cheatin', uh?" to me, is a core part of the flavor of WordPress. I wouldn't want to lose it and become a stuffy corporate application.

I don't necessarily disagree, but want to point out that I received some angry emails over the years via Rosetta contact form from users who thought it was either an inappropriate translation or a core bug.

this message should never be seen by a user and if they are seeing it, it is a bug.

Well, apparently it can be seen by a user in certain situations, whether it's a core or plugin/theme bug, so making it less confusing and more helpful would be great.

comment:49 @DrewAPicture3 months ago

  • Severity changed from normal to minor

comment:50 @slackbot3 months ago

This ticket was mentioned in Slack in #core by eric. View the logs.

comment:51 @lukecarbis2 months ago

  • Keywords has-patch added; needs-patch removed

To summarise,

  • While "Cheatin', uh?" is part of the flavour of WordPress, there are scenarios in which it confuses users and seems unprofessional. (comment:18) (comment:48)
  • Debug information (such as a user's capability) should not be displayed when WP_DEBUG is false. (comment:45)
  • Neither should debug information be displayed when WP_DEBUG is true - this should be handled elsewhere. (comment:46) (#30934)

I'm in favour of not introducing a new string for our translators. We already have a string which fits perfectly well, and says the same thing as most suggestions in this thread.

"You do not have permission to access this page."

Last edited 2 months ago by lukecarbis (previous) (diff)

@lukecarbis2 months ago

Change "Cheatin', uh?" to "You do not have permission to access this page."

@ericlewis2 months ago

comment:52 @ericlewis2 months ago

@lukecarbis - thanks for the patch, however, we decided direction on this ticket in core chat last week.

attachment:14530.3.diff is a minimum viable change here. Use generic text (e.g. "item," "items") in places where the item in question is variable.

@ericlewis2 months ago

comment:53 @ericlewis2 months ago

attachment:14530.4.diff removes some stray sprintf()s

comment:54 @slackbot6 weeks ago

This ticket was mentioned in Slack in #core by drew. View the logs.

comment:55 @helen6 weeks ago

  • Milestone changed from 4.2 to Future Release

Seems like we lost track of this a bit in 4.2 - this is a lot of new strings to introduce, and I'm not sure we need to get quite so specific in the user-facing language. As I noted before, we could perhaps show more details with WP_DEBUG on. I am (sadly) punting this - let's maybe hash on some possible language again before spending time re-patching.

Note: See TracTickets for help on using tickets.