WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#33763 closed defect (bug) (fixed)

Post types with show_ui set to false should not be accessible via the admin area

Reported by: johnbillion Owned by: johnbillion
Milestone: 4.4 Priority: normal
Severity: normal Version: 2.9
Component: Posts, Post Types Keywords: has-patch needs-unit-tests needs-testing
Focuses: administration Cc:

Description

This might be a contentious issue. It's certainly not expected behaviour, but I suspect there are sites which rely on its behaviour.

If a custom post type is registered with show_ui set to false, it's still possible to see the post type listing screen for the post type by manually navigating to the correct URL (eg. example.com/wp-admin/edit.php?post_type=my_hidden_post_type). From there you can access the post editing screen for the posts and update them.

If I register a post type without a UI, I do not expect to be able to access a UI for the post type simply by hacking the URL.

The post type listing UI and post editing UI for such a post type should be disabled. In order to disable this UI currently, you need to set the post type's capabilities to a capability which your users don't have, which has the side effect of preventing posts from being created/edited via other means, such as XML-RPC.

Attachments (3)

33763.diff (495 bytes) - added by swissspidy 3 years ago.
33763.2.diff (1.1 KB) - added by swissspidy 3 years ago.
33763.3.diff (3.1 KB) - added by johnbillion 3 years ago.

Download all attachments as: .zip

Change History (24)

@swissspidy
3 years ago

#2 @swissspidy
3 years ago

  • Keywords has-patch added; needs-patch removed

The attached patch mimics the behaviour of post-new.php and returns an error when show_ui is set to false.

I agree that people do weird things with post types and misusing show_ui may be one of them. Perhaps someone can scan the plugin directory for usages of 'show_ui' => false?

#3 @wonderboymusic
3 years ago

  • Keywords needs-refresh added; 2nd-opinion removed
  • Milestone changed from Awaiting Review to 4.4

I like the approach, but move the check to a new check after that one and make the error message more specific to the particular check

@swissspidy
3 years ago

#4 @swissspidy
3 years ago

  • Keywords needs-refresh removed

33763.2.diff separates the check from the ! $typenow check and also fixes this in wp-admin/post.php like originally suggested.

The error message was changed to You are not allowed to edit posts in this post type., a string that's already used in core and doesn't need a new translation.

#5 @ocean90
3 years ago

#22214 was marked as a duplicate.

#6 @nofearinc
3 years ago

33763.2.diff works for me, the post type is not visible in the admin, neither accessible through edit.php or post.php.

#7 @johnbillion
3 years ago

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

I'm going to go ahead and commit this so we have plenty of time to hear from the authors of any plugins which rely on this unexpected and unintended behaviour.

If your plugin or site does rely on this behaviour, the arguments that are passed to register_post_type() should be altered so that show_ui is true, and arguments such as show_in_menu, show_in_nav_menus, and show_in_admin_bar are false.

#8 @johnbillion
3 years ago

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

In 34177:

Remove the ability to view the post listing screen and post editing screen for post types with show_ui set to false. It is unexpected and unintended behaviour that this is allowed.

If your plugin or site does rely on this behaviour, the arguments that are passed to register_post_type() should be altered so that show_ui is true, and arguments such as show_in_menu, show_in_nav_menus, and show_in_admin_bar are false.

Fixes #33763
Props swissspidy, johnbillion

#9 @DrewAPicture
3 years ago

  • Keywords needs-docs added
  • Resolution fixed deleted
  • Status changed from closed to reopened

@johnbillion: I think we should probably update the docs for the show_ui argument in register_post_type() as well as add a changelog entry for the change in behavior.

#10 @dd32
3 years ago

[34177] has broken the ability to get a link to Revisions.

get_edit_post_link( $revision_id ) => $post_type = 'revision' => the Revision post type is not a show_ui => true post type.

#11 @johnbillion
3 years ago

In 34353:

Add @since docs for the show_ui argument in register_post_type().

See #33763

#12 @johnbillion
3 years ago

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

In 34357:

Revisions are an exception when it comes to the editing UI. The revision post type cannot have its show_ui argument set to true because this allows access to the post type listing, creation, and editing UI, but get_edit_post_link() needs to return a URL for the editing UI for revisions as that's how the revisions UI works.

Fixes #33763

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


3 years ago

#14 @nerrad
3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I have another issue that likely wasn't forseen.

In our case we want show_ui set to false so it restricts access to the native post_editor for our custom post type. However, we were using the get_edit_post_link filter to filter the edit links to point to our custom ui for editing our post type.

The problem is that with the change introduced here, if show_ui is set to false, not only does it restrict access to the native editor (huge plus by the way) but it also prevents the custom filtered edit links from being shown. Currently the only way to rectify that is to set show_ui to true.

I see that this is because of this bit of code:

$allowed = array_merge( array(
		'revision',
	), get_post_types( array(
		'show_ui' => true,
	) ) );

Would it be possible to make that allowed array filterable, then plugins can still show the edit link when they have a custom ui. It *might* even be a place where we could add another argument on the register_post_type array for this explicit scenario?

Last edited 3 years ago by nerrad (previous) (diff)

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


3 years ago

#16 @johnbillion
3 years ago

  • Keywords needs-patch added; has-patch needs-docs removed
  • Status changed from reopened to accepted

I think we can fix this without the need to introduce a new filter. Patch incoming.

@johnbillion
3 years ago

#17 @johnbillion
3 years ago

  • Keywords has-patch needs-unit-tests needs-testing added; needs-patch removed

@nerrad Can you try out 33763.3.diff please? It should fix your issue. The edit post link is always passed through the get_edit_post_link filter, even if it's empty.

#18 @nerrad
3 years ago

yeah I'll get this tested (won't be until tomorrow tho), however on first glance, will this cause a regression on the revision link fix?

#19 @builtbynorthby
3 years ago

I tested the patch and it fixes the issue that @nerrad reported, the edit links appear on the custom post type posts. Thank you.

#20 @nerrad
3 years ago

@johnbillion @builtbynorthby is a workmate and the one who originally discovered the issue, so looks like your patch is all clear for fixing the issue reported.

#21 @johnbillion
3 years ago

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

In 35704:

Move the show_ui logic into the get_edit_post_link() and get_edit_term_link() functions to facilitate post types and terms which specify show_ui as false but provide a custom editing UI via the get_edit_post_link and get_edit_term_link filters.

Fixes #33763
Fixes #33938

Note: See TracTickets for help on using tickets.