Open justintadlock opened 5 years ago
One other extra note:
This is also an issue with the admin menus added by the plugin:
The only way to change the capability for them individually is to unregister the menus and re-add them. By using the CPT caps for $type->cap->edit_posts
(custom fields and field groups) and $type->cap->create_posts
(add new), third-party devs could alter this more easily and not risk breaking other add-ons.
Hi @justintadlock
Thanks for the topic and code examples. I'm on board with this, and agree that using capabilities will allow for finer control without much effort.
I'll add this to my to-do list, which is unfortunately a little bloated at the moment, but I'll try to prioritize it.
I'll also need to freshen up on capabilities too. Can you elaborate a little more about the 'map_meta_cap' => true
setting, and also the ''read', edit_post', 'read_post', delete_post' caps (thy they are different to $cap
)?
Thanks Elliot
Can you elaborate a little more about the 'map_meta_cap' => true setting, and also the ''read', edit_post', 'read_post', delete_post' caps (thy they are different to $cap)?
Sure thing. I'm going to just run through a quick tutorial of the cap system in WordPress, which really isn't well documented.
There are two types of caps in WordPress:
Meta Caps: (ex: edit_post
) These are caps that are used when checking if a user has permission to "do something" to an object (e.g., post, term, user, etc.). Note that the standard practice is to use the "singular" form of the word -- _post
instead of _posts
-- for these. Meta caps aren't actually assigned to roles and can't do anything on their own. They're sort of like placeholders that get mapped to primitive caps.
Primitive Caps: (ex: edit_posts
) These are caps that you assign to roles. They determine whether a user has permission to perform an action.
So the edit_post
, delete_post
, and read_post
meta caps are there merely for mapping purposes. They don't "do" anything on their own. When you register a CPT, these should generally be unique to the CPT.
The reason we want to set 'map_meta_cap' => true
is so that WordPress automatically maps these meta caps to your primitive caps (e.g., edit_posts
, edit_others_posts
, etc.).
An example of how this works:
When checking if a user can edit a post, WordPress checks current_user_can( 'edit_post', $post_id )
. This calls up the core map_meta_cap()
function.
At that point, WP runs through a series of conditionals that look sort of like:
edit_posts
.edit_published_posts
.edit_private_posts
.edit_others_posts
.Depending on the scenario, the user may need 1-4 different primitive caps.
Of course, in your case, all of this could just map back to manage_options
, which is what ACF uses by default now.
Thanks @justintadlock
You explained this very elegantly :)
I'm happy to add this into the next version. Leave this with me for now, and I'll be in touch shortly.
Hi @justintadlock
I'm making good progress with this feature request and wanted to give you an update.
After diving into CPT and Capability documentation, I would like to tackle this in a slightly different approach. Instead of manually applying the capabilities (as suggested in your code snippet), I think we should allow WP to generate the new capabilities based off the CPT name.
We can then hook into a filter (such as "user_has_cap") and dynamically add these "ACF caps" to the current user depending on the ACF "capability setting".
I've already written the code for this, and it appears to work quite well. Can you manually copy the following code (replacing the existing register_post_types()
method) and let me know your thoughts?
/**
* register_post_types
*
* Registers the ACF post types.
*
* @date 22/10/2015
* @since 5.3.2
*
* @param void
* @return void
*/
function register_post_types() {
// Register the Field Group post type.
register_post_type('acf-field-group', array(
'labels' => array(
'name' => __( 'Field Groups', 'acf' ),
'singular_name' => __( 'Field Group', 'acf' ),
'add_new' => __( 'Add New' , 'acf' ),
'add_new_item' => __( 'Add New Field Group' , 'acf' ),
'edit_item' => __( 'Edit Field Group' , 'acf' ),
'new_item' => __( 'New Field Group' , 'acf' ),
'view_item' => __( 'View Field Group', 'acf' ),
'search_items' => __( 'Search Field Groups', 'acf' ),
'not_found' => __( 'No Field Groups found', 'acf' ),
'not_found_in_trash' => __( 'No Field Groups found in Trash', 'acf' ),
),
'public' => false,
'hierarchical' => true,
'show_ui' => true,
'show_in_menu' => false,
'capability_type' => 'acf_field_group',
'map_meta_cap' => true,
'supports' => array('title'),
'rewrite' => false,
'query_var' => false,
));
// Register the Field post type.
register_post_type('acf-field', array(
'labels' => array(
'name' => __( 'Fields', 'acf' ),
'singular_name' => __( 'Field', 'acf' ),
'add_new' => __( 'Add New' , 'acf' ),
'add_new_item' => __( 'Add New Field' , 'acf' ),
'edit_item' => __( 'Edit Field' , 'acf' ),
'new_item' => __( 'New Field' , 'acf' ),
'view_item' => __( 'View Field', 'acf' ),
'search_items' => __( 'Search Fields', 'acf' ),
'not_found' => __( 'No Fields found', 'acf' ),
'not_found_in_trash' => __( 'No Fields found in Trash', 'acf' ),
),
'public' => false,
'hierarchical' => true,
'show_ui' => false,
'show_in_menu' => false,
'capability_type' => 'acf_field',
'map_meta_cap' => true,
'supports' => array('title'),
'rewrite' => false,
'query_var' => false,
));
// Add custom capability types to users with ACF's primary capability.
$acf_cap = acf_get_setting('capability');
if( current_user_can($acf_cap) ) {
add_filter('user_has_cap', array($this, 'user_has_cap'), 10, 4 );
}
}
/**
* user_has_cap
*
* Adds all ACF custom post type capabilities to the current user's caps.
*
* @date 30/8/19
* @since 5.8.1
*
* @param type $var Description. Default.
* @return type Description.
*/
function user_has_cap( $allcaps, $cap, $args, $user ) {
// Loop over capability types.
$capability_types = array( 'acf_field_group', 'acf_field' );
foreach( $capability_types as $capability_type ) {
// Generate array of new caps and append them.
$new_caps = array(
"edit_{$capability_type}",
"read_{$capability_type}",
"delete_{$capability_type}",
"edit_{$capability_type}s",
"edit_others_{$capability_type}s",
"publish_{$capability_type}s",
"read_private_{$capability_type}s",
"delete_{$capability_type}s",
"delete_private_{$capability_type}s",
"delete_published_{$capability_type}s",
"delete_others_{$capability_type}s",
"edit_private_{$capability_type}s",
"edit_published_{$capability_type}s",
);
foreach( $new_caps as $new_cap ) {
$allcaps[ $new_cap ] = true;
}
}
// Return.
return $allcaps;
}
IMO, this would not be an ideal route to take with the filter on user_has_cap
. It'd likely break compatibility with any plugins, such as mine and anyone who has a custom filter on the CPT capabilities. While it wouldn't be hard to overwrite that in my own plugin by removing the filter, it could have some unintended consequences for others unaware of the changes.
What I outlined earlier would preserve back-compatibility and change nothing with how the plugin currently works.
It really comes down to your policy on back-compat with ACF. The filter on user_has_cap
is a breaking change for third-party code that touches caps.
If you decide to stick with the filter on user_has_cap
, I do have a couple of notes.
In the filter, you'd want to check that the user doesn't already have each one of the caps being added. Then, you'd need to make sure that each is not explicitly denied (set to false
) before adding it. Otherwise, you open up a security hole by escalating a user's privileges.
Additionally, there should be a check on the $user
object passed into the filter, not with the current_user_can()
check earlier. The user caps that are being checked at that point may not be the same user as earlier.
Hi @justintadlock
Good points all round. I think you hit every issue with the proposed code - nice :) Backwards compatible and Future Proof are the two items I am most interested in at the moment.
I agree that user_has_cap
might not be the best filter for ACF to use. I also agree with your notes about checking if the cap already exists (as the proposed code would simply override them), and also with your notes about checking the WP_User too.
* These are things I had thought about, but didn't want to complicate the proposal.
For Future Proof development, I like the idea of ACF having its own CPT caps (such as edit_acf_field_group
). It seems more inline with WordPress and we can then write neater conditionals like if( current_user_can('edit_acf_field_group') )
.
In regards to Backwards Compatibility, you mentioned that this kind of chance would break any third-party code that touches caps. Would you mind sharing with me the code you are using to integrate with ACF? This will help me understand what might break by this kind of change.
In theory, If the ACF CPT generates caps like "edit_posts" => "edit_acf_field_groups"
, it should be possible to override this by modifying the CPT object, or by customizing the "edit_acf_field_groups" cap?
I'm confident that we can find a solution that doesn't break third-party code, but also brings ACF inline with WordPress standards. Worst case, we might need to plan and co-ordinate a future release that works with the "before" and "after" ACF CPT settings.
Side note: I'm feeling good about this feature request and see this as a real opportunity for ACF to "do something properly" - which is very inline with my refactoring mission at the moment :)
Hi @justintadlock
Did you get a chance to read my last reply? I'm looking forward to your thoughts on implementing "WordPress style" caps for the ACF CPTs.
I've been looking into the user_can() function and WP_User class to better understand how capabilities are checked, and everything looks very logical.
In my investigation, I came across the map_meta_cap()
function and the map_meta_cap
filter. This would work great for "translating" the new caps (such as "edit_acf_field_group") back to the previous acf_get_setting('capability')
value, but I can see that this wouldn't allow for much third-party support.
I'll keep looking into solutions, and look forward to hearing from you.
I'm creating a plugin that bridges the Members role editor and ACF. One of the issues I've run into is that ACF hardcodes
acf_get_setting( 'capability' )
any time it needs to check a capability.Instead, it should use
get_post_type_object( 'acf-field-group' )->cap->{$capname}
when checking directly if a user can do something.acf_get_setting( 'capability' )
should be set at theregister_post_type()
level. That way, it trickles down.The biggest place this is an issue is in the
save_post
callback for field groups: https://github.com/AdvancedCustomFields/acf/blob/master/includes/admin/admin-field-group.php#L461-L464Instead of checking the ACF cap, it should check
current_user_can( 'edit_post', $post_id )
and let WP map this meta capability back to the CPT's primitive caps.I've managed to code a workaround by filtering the capability setting and faking it during this callback and removing the filter afterward. However, that's not future proof and could fall apart whenever there's an update to the underlying ACF code to do the same cap check elsewhere.
Longer term
Ideally, any action that a user could take when editing or deleting either a field group or a field itself would be tied directly into the CPT caps for these two post types. By default, ACF could set all of its CPT caps to
acf_get_setting( 'capability' )
.Doing this wouldn't change how ACF works internally. However, it'd make it flexible enough for third-party developers to do some really cool stuff with caps.
I'd be happy to look over any work with this if you decide to make the changes.
Quick example of doing this during the CPT registration process:
Extra
The "active" status for a field group should only be available to users who can
$type->cap->publish_posts
. Otherwise, it should fall back to "inactive". Sort of like how draft posts work.