WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 6 years ago

#27137 closed defect (bug) (wontfix)

Remove required user context from `get_edit_post_link()`

Reported by: danielbachhuber Owned by:
Milestone: Priority: low
Severity: minor Version:
Component: Posts, Post Types Keywords: has-patch
Focuses: template Cc:

Description

In a manner similar to #27113, get_edit_post_link() yields inconsistent behavior when used without a user context.

For example, when using WP-CLI or a cron system to trigger email generation, use of get_edit_post_link() will return an empty string.

The pattern established in core is to check whether the current user can edit a given post before calling get_edit_post_link() because the corresponding HTML will be broken otherwise.

Removing the capability check won't introduce a security hole because WordPress institutes a capability check when loading the link.

Attachments (2)

27137.1.diff (492 bytes) - added by danielbachhuber 7 years ago.
Remove the capability check in get_edit_post_link()
27137.2.diff (593 bytes) - added by danielbachhuber 7 years ago.
Move current_user_can() check from get_ to edit_

Download all attachments as: .zip

Change History (12)

@danielbachhuber
7 years ago

Remove the capability check in get_edit_post_link()

#1 @ericlewis
7 years ago

  • Focuses template added

This makes sense and looks good to me.

Daniel said

Removing the capability check won't introduce a security hole because WordPress institutes a capability check when loading the link.

To add on to that, even if a plugin/theme outputs the link without proper caps checking, it's just a link. Caps check would keep them out of the wp-admin page on click-through.

#2 @danielbachhuber
7 years ago

  • Keywords commit added; dev-feedback removed
  • Milestone changed from Awaiting Review to 4.0

#3 follow-ups: @knutsp
7 years ago

  • Keywords dev-feedback added; commit removed

This would be ok, as long as the behaviour of edit_post_link() is not changed. But, looking at wp-includes/link-template.php, it seems the capabilities check is only done in get_edit_post_link().

Unless the capabilities check is moved over to edit_post_link() this change will pollute sites with unnecessary Edit links, even for anonymous users.

Or am I missing something?

@danielbachhuber
7 years ago

Move current_user_can() check from get_ to edit_

#4 in reply to: ↑ 3 @danielbachhuber
7 years ago

  • Keywords commit added; dev-feedback removed

Replying to knutsp:

This would be ok, as long as the behaviour of edit_post_link() is not changed. But, looking at wp-includes/link-template.php, it seems the capabilities check is only done in get_edit_post_link().

Unless the capabilities check is moved over to edit_post_link() this change will pollute sites with unnecessary Edit links, even for anonymous users.

Or am I missing something?

You are correct. I've updated the patch to move the capability check from get_edit_post_link() to edit_post_link()

#5 @knutsp
7 years ago

  • Keywords needs-testing dev-feedback added; commit removed

Better. Maybe fine.

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


7 years ago

#7 @DrewAPicture
6 years ago

  • Keywords close added

This seems weird to me. Why break away from core precedent and rely on the admin cap check for this one instance only, followed by moving the cap check to edit_post_link()? If we're going to move to rely on the admin cap check here it should be done consistently in core.

Suggest close.

#8 in reply to: ↑ 3 @SergeyBiryukov
6 years ago

Replying to knutsp:

Unless the capabilities check is moved over to edit_post_link() this change will pollute sites with unnecessary Edit links, even for anonymous users.

With 27137.2.diff, the same would happen on sites using get_edit_post_link() directly.

Here's what the Codex says:

Returns edit post url as string value, provided the current user has the 'edit_post' capability. To retrieve a URL without checking user capabilities use admin_url() instead.

#9 @westi
6 years ago

This suggested change seems weird to me too.

I'm not sure why scripts that want to call this can't correctly setup the user before generating the emails.

I think we should close this and leave things as they are.

#10 @SergeyBiryukov
6 years ago

  • Keywords needs-testing dev-feedback close removed
  • Milestone 4.0 deleted
  • Resolution set to wontfix
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.