WordPress.org

Make WordPress Core

Opened 21 months ago

Closed 11 months ago

Last modified 11 months ago

#35614 closed enhancement (fixed)

Cannot check capabilities on single taxonomy terms

Reported by: johnjamesjacoby Owned by: johnbillion
Milestone: 4.7 Priority: normal
Severity: normal Version:
Component: Taxonomy Keywords:
Focuses: Cc:

Description

Unlike Users and Posts, Taxonomy Terms do not have single capability checks.

Users and Posts have single capabilities that are accompanied by an ID, like:

  • edit_user
  • edit_post

Taxonomy terms however, are only ever checked via their plural version, like:

  • manage_categories

In register_taxonomy(), the default caps are mapped like:

$default_caps = array(
	'manage_terms' => 'manage_categories',
	'edit_terms'   => 'manage_categories',
	'delete_terms' => 'manage_categories',
	'assign_terms' => 'edit_posts',
);

But there is no single equivalent, like:

  • manage_term
  • edit_term
  • delete_term
  • assign_term

This means it's not currently possible (even with filters) to target specific terms in a taxonomy and prevent them from being edited, deleted, or assigned.

Attachments (5)

Screen Shot 2016-01-25 at 9.15.47 PM.png (85.1 KB) - added by johnjamesjacoby 21 months ago.
My use-case is a plugin to "lock" terms from being edited or deleted by users with Editor access or less. The UI and metadata integration is finished and easy, but the functionality doesn't appear to be possible.
35614.patch (10.7 KB) - added by johnbillion 14 months ago.
35614.2.patch (18.0 KB) - added by johnbillion 14 months ago.
35614.3.patch (17.5 KB) - added by johnjamesjacoby 13 months ago.
Same as .2, but removes accidental whitespace change, and removes a $tax variable assignment that's no longer used
35614.diff (720 bytes) - added by nacin 11 months ago.
Playing with an idea.

Download all attachments as: .zip

Change History (28)

@johnjamesjacoby
21 months ago

My use-case is a plugin to "lock" terms from being edited or deleted by users with Editor access or less. The UI and metadata integration is finished and easy, but the functionality doesn't appear to be possible.

#1 @boonebgorges
21 months ago

  • Milestone changed from Awaiting Review to Future Release

+1 to the idea of more fine-grained caps for taxonomy terms, for more flexible development. I don't know that there's a need in core for this at the moment, since Terms don't have the same sorts of properties that dictate caps for Posts (think: authorship, post_status, revisions). But this is partly a chicken/egg problem - we don't have these things because we don't have the infrastructure.

There are a couple of directions we could move here, using post caps as a boilerplate. First, post caps are on a per-post basis - this is what you're referencing here. Second, post caps are, by default, named after the current post type, and we could do the same thing with taxonomies. It might make sense to do both of these at the same time, as it would be easier to port from the post_type system.

Here's the beginning of what a patch would have to have:

  1. As noted above, default mappings for singular caps in register_taxonomy().
  2. Singular cap mapping in map_meta_cap(), accompanied by unit tests.
  3. Convert existing current_user_can() checks to singular checks, as appropriate.

#3 @danielbachhuber
21 months ago

+1

If this is to become the omnibus taxonomy caps ticket, I'd also love to see list_<taxonomy> and create_<taxonomy>. Although I realize there are potential UX changes with these generic caps.

#4 @johnbillion
14 months ago

  • Keywords needs-unit-tests added; 2nd-opinion removed
  • Milestone changed from Future Release to 4.7
  • Owner set to johnbillion
  • Status changed from new to assigned

#5 @johnbillion
14 months ago

#25597 was marked as a duplicate.

@johnbillion
14 months ago

#6 @johnbillion
14 months ago

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

35614.patch is a first pass at this. Needs a bunch of tests and a bunch of testing.

#7 @johnbillion
14 months ago

In 38516:

Taxonomy: Introduce some taxonomy capability tests in preparation for introducing more fine grained capabilities for terms.

See #35614

#8 @johnbillion
14 months ago

In 38521:

Role/Capability: Split meta and primitive capabilities in the helper functions in the roles and capability tests so primitive capability tests can be made more accurate.

See #35614
See #32394

#9 @johnbillion
14 months ago

In 38522:

Role/Capability: Correct the multisite cap tests after [38521].

See #35614
See #32394

#10 @johnjamesjacoby
14 months ago

@johnbillion Thanks for your efforts here. I'll be digging in and testing this week.

#11 @johnbillion
14 months ago

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

35614.2.patch is an updated patch with additional tests.

#12 follow-up: @johnbillion
13 months ago

@johnjamesjacoby It would be great if you could test this by attempting to implement your edit/delete locking functionality with the patch applied. I've been testing using various filters on the map_meta_cap filter (my use-case is preventing edits to a term that's marked as pending approval by an administrator).

#13 in reply to: ↑ 12 @johnjamesjacoby
13 months ago

  • Keywords commit added; needs-testing removed

Replying to johnbillion:

@johnjamesjacoby It would be great if you could test this by attempting to implement your edit/delete locking functionality with the patch applied. I've been testing using various filters on the map_meta_cap filter (my use-case is preventing edits to a term that's marked as pending approval by an administrator).

@johnbillion Patch still applies cleanly, and looks good to my eyes.

I especially like moving the default-term-check to map_meta_cap() -- great work.

Tests are passing, and I walked through vanilla WordPress functionality & everything works the same as before.

Recommend to commit when ready.

@johnjamesjacoby
13 months ago

Same as .2, but removes accidental whitespace change, and removes a $tax variable assignment that's no longer used

#14 @johnbillion
13 months ago

  • Keywords needs-dev-note added

#15 @johnbillion
13 months ago

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

In 38698:

Taxonomy: Introduce more fine grained capabilities for managing taxonomy terms.

This introduces the singular edit_term, delete_term, and assign_term meta capabilities for terms, and switches the base capability name for tags from manage_categories to manage_post_tags and the corresponding edit_post_tags, delete_post_tags, and assign_post_tags.

All of these capabilities ultimately map to manage_categories so by default there is no change in the behaviour of the capabilities for categories, tags, or custom taxonomies. The map_meta_cap filter and the capabilities argument when registering a taxonomy now allow for control over editing, deleting, and assigning individual terms, as well as a separation of capabilities for tags from those of categories.

Fixes #35614
Props johnjamesjacoby for feedback

#16 @johnbillion
13 months ago

  • Keywords has-patch has-unit-tests commit removed

#18 @rachelbaker
11 months ago

In 39402:

REST API: Fix incorrect capability check on term create.

Change the capability check used in WP_REST_Terms_Controller when creating a new term is attempted, from manage_terms to edit_terms. This matches the behavior within the WordPress admin. See #35614.

Props johnbillion, rmccue, rachelbaker, helen, jorbin, SergeyBiryukov.

Fixes #38958.

#19 @rachelbaker
11 months ago

In 39403:

REST API: Fix incorrect capability check on term create.

Change the capability check used in WP_REST_Terms_Controller when creating a new term is attempted, from manage_terms to edit_terms. This matches the behavior within the WordPress admin. See #35614.

Props johnbillion, rmccue, rachelbaker, helen, jorbin, SergeyBiryukov.

Merges [39402] to the 4.7 branch.
Fixes #38958 for 4.7.

#20 follow-up: @kevinB
11 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

As implemented in 4.7 RC, these meta capability checks accept a term_id with no taxonomy specified. This is an issue for legacy installations that have term_id != term_taxonomy_id. Taxonomy determination is now slated to fall to WP_Term::get_instance(), which will try to disambiguate but may return WP_Error.

An obvious option would to require the additional taxonomy argument following term_id. This would cause a fourth item in the $args array passed into the 'map_meta_cap' filter - a break with tradition, but not a problem for API-compliant plugins in my view.

@nacin
11 months ago

Playing with an idea.

#21 in reply to: ↑ 20 @dd32
11 months ago

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

Replying to kevinB:

As implemented in 4.7 RC, these meta capability checks accept a term_id with no taxonomy specified. This is an issue for legacy installations that have term_id != term_taxonomy_id. Taxonomy determination is now slated to fall to WP_Term::get_instance(), which will try to disambiguate but may return WP_Error.

An obvious option would to require the additional taxonomy argument following term_id. This would cause a fourth item in the $args array passed into the 'map_meta_cap' filter - a break with tradition, but not a problem for API-compliant plugins in my view.

I'm punting this to 4.7.1 after chatting with @nacin, I've created #39038 for that.

For most sites, terms are likely to be split, and this should be a reasonably rare case to be hit.
In the event that a term isn't split, this will prevent it from being edited or deleted at present, not an unreasonable situation here for now.

#22 @pento
11 months ago

In 39464:

REST API: Capability check for editing a single term should use the singular form.

As an extra level of sanity checking, the term ID should be cast as an int in map_meta_cap().

Props johnbillion, nacin, dd32, pento.
See #35614.
Fixes #39012.

#23 @pento
11 months ago

In 39465:

REST API: Capability check for editing a single term should use the singular form.

As an extra level of sanity checking, the term ID should be cast as an int in map_meta_cap().

Merge of [39464] to the 4.7 branch.

Props johnbillion, nacin, dd32, pento.
See #35614.
Fixes #39012.

Note: See TracTickets for help on using tickets.