Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Securing taxonomies export #872

Closed
wants to merge 11 commits into from
Closed

Securing taxonomies export #872

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
691ca0c
Validation if array indexes exist and if return object is not wp_error
emilse-webdev Jun 10, 2022
838c79b
Cleaning dev comments
emilse-webdev Jun 10, 2022
7eaa63c
Adding another validation
emilse-webdev Jun 10, 2022
eb38c29
Cleaning dev comments
emilse-webdev Jun 10, 2022
894af74
changing a little detail
emilse-webdev Jun 11, 2022
f0f9202
Update
emilse-webdev Jun 13, 2022
b03c440
Still adding changes
emilse-webdev Jun 13, 2022
f8d6e5c
Update
emilse-webdev Jun 14, 2022
d55ea64
changes
emilse-webdev Jun 14, 2022
7126c31
lint
emilse-webdev Jun 14, 2022
f6caf8e
Issue 869
emilse-webdev Jun 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions custom-post-type-ui.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,23 @@ function cptui_create_custom_post_types() {
return;
}

function sample_admin_notice__warning() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move this function into the inc/utility.php file at the end of the file, and let's also rename it to cptui_undefined_index_notice.

?>
<div class="notice notice-warning">
<p>There's some missing indexes. You might need to re-save.</p>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to get this passed through esc_html_e() with a textdomain of custom-post-type-ui.

Also lets have it say "Custom Post Type UI added new parameters. Please consider reviewing your post types or taxonomies and consider clicking to save them again."

</div>
<?php
}

// Check if old cpts and new cpts are not empty
// Get keys of both new cpts and old cpts
// Compare if they don't have any missing or difference in keys
if ( (is_array($cpts) && !empty($cpts)) &&
(is_array($cpts_override) && !empty($cpts_override)) &&
( array_diff( array_keys($cpts), array_keys($cpts_override) ) )) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpicky detail, but can we check on PHPCS for this section regarding whitespace around function calls and passed parameters?

add_action( 'admin_notices', 'sample_admin_notice__warning' );
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line will need to be updated to match the function rename.

}

// Assume good intent, and we're also not wrecking the option so things are always reversable.
if ( is_array( $cpts_override ) && ! empty( $cpts_override ) ) {
$cpts = $cpts_override;
Expand Down
4 changes: 4 additions & 0 deletions inc/post-types.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1724,6 +1724,10 @@ function cptui_update_post_type( $data = [] ) {
$data['cpt_supports'] = [];
}

if ( empty( $data['cpt_labels'] ) || ! is_array( $data['cpt_labels'] ) ) {
$data['cpt_labels'] = [];
}

foreach ( $data['cpt_labels'] as $key => $label ) {
if ( empty( $label ) ) {
unset( $data['cpt_labels'][ $key ] );
Expand Down
14 changes: 14 additions & 0 deletions inc/taxonomies.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1503,6 +1503,20 @@ function cptui_update_taxonomy( $data = [] ) {
return 'error';
}

$must_have_keys = ['object_types', 'rewrite', 'rewrite_slug', 'name',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we turn this into a function similar to cptui_reserved_taxonomies() where it returns this array? No need to add an apply_filters() for it, just have it return the array.

Could either still assign to $must_have_keys or it could replace the variable and just be used as such:

foreach( my_function_name() as $key ) {
...
}

The reason for this request is to make the array more usable in the future, if need arises.

'rewrite_withfront', 'rewrite_hierarchical', 'public', 'publicly_queryable',
'show_in_quick_edit', 'show_tagcloud', 'show_in_menu', 'show_ui', 'rest_base',
'show_in_nav_menus', 'show_in_rest', 'rest_controller_class', 'label',
'labels', 'singular_label', 'meta_box_cb', 'default_term', 'singular_label',
'show_in_graphql', 'sort', 'query_var', 'show_ui', 'show_admin_column',
'hierarchical', 'graphql_single_name', 'graphql_plural_name'];

foreach( $must_have_keys as $key ) {
if ( ! array_key_exists($key, $data['cpt_custom_tax']) ){
$data['cpt_custom_tax'][$key] = '';
}
}

foreach ( $data['cpt_tax_labels'] as $key => $label ) {
if ( empty( $label ) ) {
unset( $data['cpt_tax_labels'][ $key ] );
Expand Down
60 changes: 49 additions & 11 deletions inc/tools.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,30 @@ function <?php echo esc_html( $callback ); ?>() {
*/
function cptui_get_single_taxonomy_registery( $taxonomy = [] ) {

$might_need_to_resave = false;

$must_have_keys = ['object_types', 'rewrite', 'rewrite_slug', 'name',
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually perfect reason to make the keys function a reusable function. We can reduce code amount here.

'rewrite_withfront', 'rewrite_hierarchical', 'public', 'publicly_queryable',
'show_in_quick_edit', 'show_tagcloud', 'show_in_menu', 'show_ui', 'rest_base',
'show_in_nav_menus', 'show_in_rest', 'rest_controller_class', 'label',
'singular_label', 'meta_box_cb', 'default_term', 'singular_label',
'show_in_graphql', 'sort', 'query_var', 'show_ui', 'show_admin_column',
'hierarchical', 'graphql_single_name', 'graphql_plural_name'];

foreach( $must_have_keys as $key ) {
if ( ! array_key_exists($key, $taxonomy) ){
$taxonomy[$key] = '';

if ( $might_need_to_resave == false){
$might_need_to_resave = true;
}
}
}

if ( ! array_key_exists('labels', $taxonomy) ){
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another nitpick, but spacing around our functions and logic statements. Her and above into the foreach loop above around line 241.

I know Alok is also working on his own PHPCS review so best to not introduce new spots when able.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll run phpcs to check my added code

$taxonomy['labels'] = [];
}

$post_types = "''";
if ( is_array( $taxonomy['object_types'] ) ) {
$post_types = '[ "' . implode( '", "', $taxonomy['object_types'] ) . '" ]';
Expand Down Expand Up @@ -261,6 +285,7 @@ function cptui_get_single_taxonomy_registery( $taxonomy = [] ) {
} else {
$rewrite = disp_boolean( $taxonomy['rewrite'] );
}

$public = isset( $taxonomy['public'] ) ? disp_boolean( $taxonomy['public'] ) : 'true';
$publicly_queryable = isset( $taxonomy['publicly_queryable'] ) ? disp_boolean( $taxonomy['publicly_queryable'] ) : disp_boolean( $taxonomy['public'] );
$show_in_quick_edit = isset( $taxonomy['show_in_quick_edit'] ) ? disp_boolean( $taxonomy['show_in_quick_edit'] ) : disp_boolean( $taxonomy['show_ui'] );
Expand Down Expand Up @@ -311,25 +336,21 @@ function cptui_get_single_taxonomy_registery( $taxonomy = [] ) {
?>

/**
* Taxonomy: <?php echo esc_html( $taxonomy['label'] ); ?>.
*/
* Taxonomy: <?php echo esc_html( $taxonomy['label'] ); ?>.
*/

$labels = [
"name" => __( "<?php echo esc_html( $taxonomy['label'] ); ?>", "<?php echo esc_html( $textdomain ); ?>" ),
"singular_name" => __( "<?php echo esc_html( $taxonomy['singular_label'] ); ?>", "<?php echo esc_html( $textdomain ); ?>" ),
<?php
foreach ( $taxonomy['labels'] as $key => $label ) {
if ( ! empty( $label ) ) {
echo "\t\t" . '"' . esc_html( $key ) . '" => __( "' . esc_html( $label ) . '", "' . esc_html( $textdomain ) . '" ),' . "\n";
foreach ( $taxonomy['labels'] as $key => $label ) {
if ( ! empty( $label ) ) {
echo "\t\t" . '"' . esc_html( $key ) . '" => __( "' . esc_html( $label ) . '", "' . esc_html( $textdomain ) . '" ),' . "\n";
}
}
}
?>
];

<?php
$show_graphql = isset( $taxonomy['show_in_graphql'] ) ? (bool) $taxonomy['show_in_graphql'] : false;
?>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it was accidentally removed and we will want to have it in place still

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, it's this comment above: "For the show_graphql, sort, and queryvar parts, the disp_boolean() function is immediately casting them to string, which is what we're getting from the settings extraction anyway, so may as well not worry about type casting there." but I can leave it as it was,


$args = [
"label" => __( "<?php echo $taxonomy['label']; ?>", "<?php echo $textdomain; ?>" ),
"labels" => $labels,
Expand All @@ -349,7 +370,7 @@ function cptui_get_single_taxonomy_registery( $taxonomy = [] ) {
"rest_namespace" => "<?php echo $rest_namespace; ?>",
"show_in_quick_edit" => <?php echo $show_in_quick_edit; ?>,
"sort" => <?php echo disp_boolean( $taxonomy['sort'] ); ?>,
<?php if ( $show_graphql ) : ?>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same part as restoring the isset for this above.

<?php if ( $taxonomy['show_in_graphql'] ) : ?>
"show_in_graphql" => <?php echo disp_boolean( $taxonomy['show_in_graphql'] ); ?>,
"graphql_single_name" => "<?php echo esc_html( $taxonomy['graphql_single_name'] ); ?>",
"graphql_plural_name" => "<?php echo esc_html( $taxonomy['graphql_plural_name'] ); ?>",
Expand Down Expand Up @@ -412,6 +433,23 @@ function <?php echo esc_html( $callback ); ?>() {
*/
function cptui_get_single_post_type_registery( $post_type = [] ) {

// Check if all keys are present, initialize if not
$cpt_obj_keys = ['name', 'menu_icon', 'register_meta_box_cb',
'label', 'singular_label', 'description', 'rest_base',
'rest_controller_class', 'rest_namespace', 'has_archive_string',
'capability_type', 'rewrite_slug', 'query_var_slug', 'menu_position',
'show_in_menu_string', 'menu_icon', 'custom_supports', 'enter_title_here',
'public', 'publicly_queryable', 'show_ui', 'show_in_nav_menus',
'delete_with_user', 'show_in_rest', 'has_archive', 'exclude_from_search',
'hierarchical', 'can_export', 'rewrite', 'rewrite_withfront', 'query_var',
'show_in_menu'];
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same request as for the taxonomy reusable function.


foreach( $cpt_obj_keys as $key ) {
if ( array_key_exists($key, $post_type) ){
$post_type[$key] = '';
}
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same nitpick around spacing.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll run phpcs


/* This filter is documented in custom-post-type-ui/custom-post-type-ui.php */
$post_type['map_meta_cap'] = apply_filters( 'cptui_map_meta_cap', 'true', $post_type['name'], $post_type );

Expand Down