Skip to content

Conversation

@Soare-Robert-Daniel
Copy link
Contributor

@Soare-Robert-Daniel Soare-Robert-Daniel commented Oct 10, 2025

All Submissions:

Changes proposed in this Pull Request:

  • Add a new check for the move image endpoint to see if the current user has the permission to modify that image.
  • Add a small refactoring and additional PHPStan types.
CleanShot 2025-10-10 at 12 18 52@2x

Closes https://github.com/Codeinwp/optimole-service/issues/1599

How to test the changes in this Pull Request:

  1. Make an Author or Contributor user, then log in to it.
  2. Go to the Media Library
  3. Run the in-browser function offloadOptimoleImage( 30 ); // TODO: Replace with actual attachment ID from your media library.
  4. The run should have failed because of the missing permissions.

Code snippet:

async function offloadOptimoleImage(attachmentId) {
    try {
        // Extract the WordPress REST API nonce from the page
        const nonce = getNonce();
        
        if (!nonce) {
            throw new Error('WordPress REST nonce not found. Make sure you are on a WordPress admin page.');
        }

        // Prepare the payload
        const payload = {
            id: attachmentId,
            action: "offload_image",
            status: "start"
        };

        // Make the request
        const response = await fetch('/wp-json/optml/v1/move_image', {
            method: 'POST',
            headers: {
                'Content-Type': 'application/json',
                'X-WP-Nonce': nonce,
                'Accept': 'application/json, */*;q=0.1'
            },
            credentials: 'same-origin',
            body: JSON.stringify(payload)
        });

        const result = await response.json();
        
        if (response.ok) {
            console.log(`✓ Attachment ${attachmentId} offload initiated:`, result);
            return { success: true, id: attachmentId, data: result };
        } else {
            console.warn(`✗ Attachment ${attachmentId} failed:`, result);
            return { success: false, id: attachmentId, error: result };
        }

    } catch (error) {
        console.error(`Error processing attachment ${attachmentId}:`, error);
        return { success: false, id: attachmentId, error: error.message };
    }
}

/**
 * Extract WordPress REST API nonce from various sources
 */
function getNonce() {
    // Method 1: From wpApiSettings (most common)
    if (typeof wpApiSettings !== 'undefined' && wpApiSettings.nonce) {
        return wpApiSettings.nonce;
    }

    // Method 2: From wp.apiFetch (Gutenberg)
    if (typeof wp !== 'undefined' && wp.apiFetch && wp.apiFetch.nonceMiddleware) {
        const nonce = wp.apiFetch.nonceMiddleware.nonce;
        if (nonce) return nonce;
    }

    // Method 3: From inline scripts
    const scripts = document.querySelectorAll('script');
    for (const script of scripts) {
        const match = script.textContent.match(/wp\.apiFetch\.use\(\s*wp\.apiFetch\.createNonceMiddleware\(\s*["']([^"']+)["']/);
        if (match) return match[1];
    }

    // Method 4: From REST API preload data
    if (typeof wpApiSettings !== 'undefined' && wpApiSettings.root) {
        const preload = document.querySelector('#wp-api-fetch-preload');
        if (preload) {
            try {
                const data = JSON.parse(preload.textContent);
                for (const key in data) {
                    if (data[key].headers && data[key].headers['X-WP-Nonce']) {
                        return data[key].headers['X-WP-Nonce'];
                    }
                }
            } catch (e) {}
        }
    }

    return null;
}

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances security for the image offload endpoint by adding permission checks to ensure users can only modify media they have access to. The changes also include refactoring improvements and additional PHPStan type annotations.

  • Adds permission validation for the move_image endpoint to check if user can modify specific images
  • Refactors the REST route registration system with improved method names and error handling
  • Adds comprehensive PHPStan type definitions for better static analysis

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
phpstan.neon Adds type aliases for REST route configurations to improve static analysis
phpstan-baseline.neon Updates PHPStan baseline by removing resolved type issues and formatting
inc/settings.php Adds PHPStan type annotation for WP_REST_Request in auto_connect method
inc/rest.php Major refactoring with security improvements, method renaming, and comprehensive type annotations
Comments suppressed due to low confidence (1)

inc/rest.php:1

  • Corrected method name from 'reqister_route' to 'register_route'.
<?php

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@Soare-Robert-Daniel Soare-Robert-Daniel marked this pull request as ready for review October 10, 2025 10:01
@pirate-bot
Copy link
Collaborator

Plugin build for 30b2d0c is ready 🛎️!

@vytisbulkevicius vytisbulkevicius merged commit 3371cd6 into development Oct 15, 2025
14 checks passed
@vytisbulkevicius vytisbulkevicius deleted the fix/offloading-user-checking branch October 15, 2025 06:49
@pirate-bot
Copy link
Collaborator

🎉 This PR is included in version 4.1.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@pirate-bot pirate-bot added the released Indicate that an issue has been resolved and released in a particular version of the product. label Oct 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

released Indicate that an issue has been resolved and released in a particular version of the product.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants