Skip to content

Fix #31: Add comprehensive capability checks for critical operations #34

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

Closed
pbking wants to merge 1 commit into from
Closed

Fix #31: Add comprehensive capability checks for critical operations #34

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 9 additions & 6 deletions includes/class-pattern-builder-api.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,25 +63,27 @@ public function register_routes(): void {

/**
* Permission callback for read operations.
* Allows access to all logged-in users who can edit posts.
* Allows access to users who can read pattern blocks.
*
* @return bool True if the user can read patterns, false otherwise.
*/
public function read_permission_callback() {
return current_user_can( 'edit_posts' );
// Allow reading for users who can edit posts or have specific pattern capabilities
return current_user_can( 'edit_posts' ) || current_user_can( 'read_tbell_pattern_block' );
}

/**
* Permission callback for write operations (PUT, POST, DELETE).
* Restricts access to administrators and editors only.
* Restricts access to users with proper pattern block capabilities.
* Also verifies the REST API nonce for additional security.
*
* @param WP_REST_Request $request The REST request object.
* @return bool|WP_Error True if the user can modify patterns, WP_Error otherwise.
*/
public function write_permission_callback( $request ) {
// First check if user has the required capability
if ( ! current_user_can( 'edit_others_posts' ) ) {
// Check if user has the required capability for pattern block operations
// Allow users with either general edit capabilities or specific pattern capabilities
if ( ! current_user_can( 'edit_others_posts' ) && ! current_user_can( 'edit_tbell_pattern_blocks' ) ) {
return new WP_Error(
'rest_forbidden',
__( 'You do not have permission to modify patterns.', 'pattern-builder' ),
Expand Down Expand Up @@ -421,7 +423,8 @@ function handle_hijack_block_update( $response, $handler, $request ) {
if ( $post->post_type === 'tbell_pattern_block' || $convert_user_pattern_to_theme_pattern ) {

// Check write permissions before allowing update
if ( ! current_user_can( 'edit_others_posts' ) ) {
// Allow users with either general edit capabilities or specific pattern capabilities
if ( ! current_user_can( 'edit_others_posts' ) && ! current_user_can( 'edit_tbell_pattern_blocks' ) ) {
return new WP_Error(
'rest_forbidden',
__( 'You do not have permission to edit patterns.', 'pattern-builder' ),
Expand Down
38 changes: 38 additions & 0 deletions includes/class-pattern-builder-controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,16 @@ public function create_tbell_pattern_block_post_for_pattern( $pattern ) {
}

public function update_theme_pattern( Abstract_Pattern $pattern, $options = array() ) {
// Check if user has permission to modify theme patterns
// Allow users with either theme options capability or general edit capability
if ( ! current_user_can( 'edit_theme_options' ) && ! current_user_can( 'edit_others_posts' ) ) {
return new WP_Error(
'insufficient_permissions',
__( 'You do not have permission to modify theme patterns.', 'pattern-builder' ),
array( 'status' => 403 )
);
}

// get the tbell_pattern_block post if it already exists
$post = get_page_by_path( $this->format_pattern_slug_for_post( $pattern->name ), OBJECT, array( 'tbell_pattern_block', 'wp_block' ) );

Expand Down Expand Up @@ -430,6 +440,15 @@ function ( $matches ) use ( $download_and_save_image ) {
* @return Abstract_Pattern|WP_Error
*/
public function update_user_pattern( Abstract_Pattern $pattern ) {
// Check if user has permission to modify user patterns
if ( ! current_user_can( 'edit_posts' ) ) {
return new WP_Error(
'insufficient_permissions',
__( 'You do not have permission to modify user patterns.', 'pattern-builder' ),
array( 'status' => 403 )
);
}

$post = get_page_by_path( $pattern->name, OBJECT, 'wp_block' );
$convert_from_theme_pattern = false;

Expand Down Expand Up @@ -575,6 +594,15 @@ function ( $p ) use ( $pattern ) {
}

public function delete_theme_pattern( Abstract_Pattern $pattern ) {
// Check if user has permission to delete theme patterns
// Allow users with either theme options capability or general edit capability
if ( ! current_user_can( 'edit_theme_options' ) && ! current_user_can( 'edit_others_posts' ) ) {
return new WP_Error(
'insufficient_permissions',
__( 'You do not have permission to delete theme patterns.', 'pattern-builder' ),
array( 'status' => 403 )
);
}

$path = $this->get_pattern_filepath( $pattern );

Expand Down Expand Up @@ -612,6 +640,16 @@ public function delete_theme_pattern( Abstract_Pattern $pattern ) {
}

public function update_theme_pattern_file( Abstract_Pattern $pattern ) {
// Check if user has permission to modify theme patterns
// Allow users with either theme options capability or general edit capability
if ( ! current_user_can( 'edit_theme_options' ) && ! current_user_can( 'edit_others_posts' ) ) {
return new WP_Error(
'insufficient_permissions',
__( 'You do not have permission to modify theme patterns.', 'pattern-builder' ),
array( 'status' => 403 )
);
}

$path = $this->get_pattern_filepath( $pattern );

if ( ! $path ) {
Expand Down
47 changes: 47 additions & 0 deletions tests/php/test-pattern-builder-api.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,29 @@ public function setUp(): void {
$admin_id = self::factory()->user->create(['role' => 'administrator']);
wp_set_current_user($admin_id);

// Ensure administrator has the required pattern builder capabilities for testing
$admin_role = get_role('administrator');
if ($admin_role) {
$capabilities = array(
'edit_tbell_pattern_block',
'read_tbell_pattern_block',
'delete_tbell_pattern_block',
'edit_tbell_pattern_blocks',
'edit_others_tbell_pattern_blocks',
'publish_tbell_pattern_blocks',
'read_private_tbell_pattern_blocks',
'delete_tbell_pattern_blocks',
'delete_private_tbell_pattern_blocks',
'delete_published_tbell_pattern_blocks',
'delete_others_tbell_pattern_blocks',
'edit_private_tbell_pattern_blocks',
'edit_published_tbell_pattern_blocks',
);
foreach ($capabilities as $capability) {
$admin_role->add_cap($capability);
}
}

// Create a temporary directory for the test patterns
$this->test_dir = sys_get_temp_dir() . '/pattern-builder-test';
$this->remove_test_directory($this->test_dir);
Expand All @@ -34,6 +57,30 @@ public function setUp(): void {
public function tearDown(): void {
$this->remove_test_directory($this->test_dir);
remove_filter('stylesheet_directory', [$this, 'get_test_directory']);

// Clean up capabilities added for testing
$admin_role = get_role('administrator');
if ($admin_role) {
$capabilities = array(
'edit_tbell_pattern_block',
'read_tbell_pattern_block',
'delete_tbell_pattern_block',
'edit_tbell_pattern_blocks',
'edit_others_tbell_pattern_blocks',
'publish_tbell_pattern_blocks',
'read_private_tbell_pattern_blocks',
'delete_tbell_pattern_blocks',
'delete_private_tbell_pattern_blocks',
'delete_published_tbell_pattern_blocks',
'delete_others_tbell_pattern_blocks',
'edit_private_tbell_pattern_blocks',
'edit_published_tbell_pattern_blocks',
);
foreach ($capabilities as $capability) {
$admin_role->remove_cap($capability);
}
}

parent::tearDown();
}

Expand Down