feature: n8n integration for post form #1725
Conversation
WalkthroughAdds an N8N webhook integration and extends settings dependency support to include value-based conditions, updates form-builder dependencies, and adds a password-preview settings field (duplicated declaration present). Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (4)
assets/js/wpuf-form-builder.js (1)
1250-1256: n8n_webhook_url: use URL semantics and basic validation
- Prefer type 'url' and a simple pattern/placeholder to reduce bad inputs (e.g., https://your-n8n-host/webhook/...).
- Confirm the controlling field id 'enable_n8n' matches the actual DOM id used by FormDependencyHandler.
Lib/WeDevs_Settings_API.php (1)
144-145: Good addition of depends_on_value + safe rendering; prefer wp_json_encode
- The new depends_on_value plumbs through correctly and data attributes are allowed in wp_kses. LGTM.
- Minor: use wp_json_encode instead of json_encode for better encoding and consistency in WP.
Also applies to: 177-205, 324-353
includes/Integrations.php (1)
31-38: Simplify error logging.The
print_r()call on Line 37 is redundant since the argument is already a string concatenation. This can be simplified for clarity.Apply this diff:
- \WP_User_Frontend::log( 'integration', print_r( $external_class . ' integration failed', true ) ); + \WP_User_Frontend::log( 'integration', $external_class . ' integration failed' );includes/Integrations/WPUF_N8N_Integration.php (1)
105-139: Consider extracting authentication logic.The authentication switch statement is comprehensive but could benefit from extraction into separate private methods for better maintainability and testability.
Consider refactoring like this:
switch ( $auth_type ) { case 'basic_auth': $this->add_basic_auth_header( $headers, $n8n_settings ); break; case 'header_auth': $this->add_header_auth( $headers, $n8n_settings ); break; case 'jwt_auth': $this->add_jwt_auth_header( $headers, $n8n_settings ); break; }Note: The PHPCS warnings about
base64_encode()on Lines 110, 367, 368, and 371 are false positives. The function is legitimately used for Basic Authentication and JWT token encoding, not code obfuscation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Lib/WeDevs_Settings_API.php(5 hunks)assets/js/wpuf-form-builder.js(1 hunks)includes/Integrations.php(1 hunks)includes/Integrations/WPUF_N8N_Integration.php(1 hunks)wpuf-functions.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
Lib/WeDevs_Settings_API.php (1)
wpuf-functions.php (1)
wpuf_get_pro_preview_html(4696-4702)
wpuf-functions.php (1)
Lib/WeDevs_Settings_API.php (3)
WeDevs_Settings_API(13-892)get_option(579-588)get_field_description(162-170)
includes/Integrations.php (2)
includes/Integrations/WPUF_N8N_Integration.php (1)
__construct(17-25)wpuf.php (2)
__construct(74-88)log(323-326)
includes/Integrations/WPUF_N8N_Integration.php (2)
includes/Integrations.php (2)
Integrations(10-66)__construct(29-48)wpuf-functions.php (1)
wpuf_is_checkbox_or_toggle_on(5643-5645)
🪛 GitHub Check: Run PHPCS inspection
includes/Integrations/WPUF_N8N_Integration.php
[warning] 371-371:
base64_encode() can be used to obfuscate code which is strongly discouraged. Please verify that the function is used for benign reasons.
[warning] 368-368:
base64_encode() can be used to obfuscate code which is strongly discouraged. Please verify that the function is used for benign reasons.
[warning] 367-367:
base64_encode() can be used to obfuscate code which is strongly discouraged. Please verify that the function is used for benign reasons.
[failure] 367-367:
Expected 1 space after the array opener in a single line array. Found: no spaces
[failure] 365-365:
Closing parenthesis of a multi-line function call must be on a line by itself
[failure] 361-361:
Opening parenthesis of a multi-line function call must be the last content on the line
[warning] 361-361:
json_encode() is discouraged. Use wp_json_encode() instead.
[failure] 359-359:
Closing parenthesis of a multi-line function call must be on a line by itself
[failure] 358-358:
There should be a comma after the last array item in a multi-line array.
[failure] 356-356:
Opening parenthesis of a multi-line function call must be the last content on the line
[warning] 356-356:
json_encode() is discouraged. Use wp_json_encode() instead.
[failure] 272-272:
There should be a comma after the last array item in a multi-line array.
[failure] 239-239:
Multi-line function call not indented correctly; expected 24 spaces but found 28
[warning] 110-110:
base64_encode() can be used to obfuscate code which is strongly discouraged. Please verify that the function is used for benign reasons.
[failure] 91-91:
Array keys must be surrounded by spaces unless they contain a string or an integer.
[failure] 88-88:
The shorthand list syntax "[]" to destructure arrays is not available in PHP 7.0 or earlier.
🪛 PHPMD (2.15.0)
includes/Integrations/WPUF_N8N_Integration.php
88-88: Avoid unused local variables such as '$multi_repeated'. (undefined)
(UnusedLocalVariable)
88-88: Avoid unused local variables such as '$files'. (undefined)
(UnusedLocalVariable)
142-142: Avoid unused local variables such as '$response'. (undefined)
(UnusedLocalVariable)
🔇 Additional comments (8)
wpuf-functions.php (1)
4041-4085: No duplicate function declarations detected—verification confirms safe declaration.Ripgrep search found only one definition of
wpuf_settings_password_previewat line 4047 in wpuf-functions.php. No fatal redeclaration issues exist.includes/Integrations.php (2)
26-26: LGTM! N8N integration entry added.The integration registry entry is correctly formatted and consistent with existing integrations.
39-39: LGTM! Correct use of elseif.The change from
iftoelseifcorrectly prevents the N8N integration from being processed twice.includes/Integrations/WPUF_N8N_Integration.php (5)
17-25: LGTM! Constructor properly registers hooks.The constructor correctly registers filters for settings UI and an action hook for post submission with appropriate priority.
39-58: LGTM! Proper validation and sanitization.The method correctly validates that N8N is enabled, the webhook URL exists, and the post is valid before proceeding. The use of
esc_url_raw()for URL sanitization is appropriate.
161-169: LGTM! Settings section added correctly.The method properly adds the N8N settings section to the configuration.
305-317: LGTM! Menu title with icon added correctly.The method properly adds N8N to the form builder menu with a custom SVG icon.
328-344: LGTM! Form settings configured correctly.The method properly adds form-level N8N configuration options. The webhook URL field will be conditionally displayed based on the enable_n8n toggle, as configured in the JavaScript dependencies.
| [ $meta_key_value, $multi_repeated, $files ] = self::prepare_meta_fields( $meta_vars ); | ||
|
|
||
| foreach ( $meta_key_value as $meta_key => $meta_value ) { | ||
| $post_data[$meta_key] = $meta_value; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Address unused variables and potential PHP compatibility issue.
Line 88 has two concerns:
- Array destructuring syntax
[ $meta_key_value, $multi_repeated, $files ]requires PHP 7.1+, which may cause compatibility issues if the plugin supports PHP 7.0. - Variables
$multi_repeatedand$filesare destructured but never used.
Consider one of these solutions:
Solution 1: Only destructure what's needed
- [ $meta_key_value, $multi_repeated, $files ] = self::prepare_meta_fields( $meta_vars );
+ [ $meta_key_value ] = self::prepare_meta_fields( $meta_vars );Solution 2: Use traditional assignment if PHP 7.0 support is required
- [ $meta_key_value, $multi_repeated, $files ] = self::prepare_meta_fields( $meta_vars );
+ $prepared = self::prepare_meta_fields( $meta_vars );
+ $meta_key_value = $prepared[0];Please verify the minimum PHP version requirement for this plugin to determine the appropriate solution.
🏁 Script executed:
#!/bin/bash
# Find PHP version requirement in composer.json or other config files
echo "=== Checking PHP version requirement ==="
if [ -f "composer.json" ]; then
echo "From composer.json:"
jq -r '.require.php // "Not specified"' composer.json
fi
if [ -f "readme.txt" ]; then
echo "From readme.txt (PHP requirement lines):"
grep -i "php" readme.txt | head -5
fi
if [ -f "package.json" ]; then
echo "From package.json:"
jq -r '.engines.php // "Not specified"' package.json 2>/dev/null || echo "N/A"
fiLength of output: 409
🏁 Script executed:
#!/bin/bash
# Examine the specific file and lines in question
echo "=== Examining WPUF_N8N_Integration.php around lines 88-92 ==="
if [ -f "includes/Integrations/WPUF_N8N_Integration.php" ]; then
echo "Lines 80-100:"
sed -n '80,100p' "includes/Integrations/WPUF_N8N_Integration.php"
else
echo "File not found at includes/Integrations/WPUF_N8N_Integration.php"
find . -name "WPUF_N8N_Integration.php" -type f 2>/dev/null
fiLength of output: 837
🏁 Script executed:
#!/bin/bash
# Search for usage of $multi_repeated and $files variables in the file
echo "=== Searching for $multi_repeated usage ==="
rg -n '\$multi_repeated' "includes/Integrations/" -C 3 || echo "No matches for \$multi_repeated"
echo ""
echo "=== Searching for $files usage (excluding function parameter context) ==="
rg -n '\$files' "includes/Integrations/" -C 3 || echo "No matches for \$files"Length of output: 1395
🏁 Script executed:
#!/bin/bash
# Find the prepare_meta_fields method to understand return structure
echo "=== Searching for prepare_meta_fields method definition ==="
rg -n 'prepare_meta_fields' "includes/Integrations/" -A 15 -B 2 | head -50Length of output: 1653
🏁 Script executed:
#!/bin/bash
# Search for prepare_meta_fields method definition more broadly (may be in parent class or trait)
echo "=== Searching for prepare_meta_fields method definition anywhere ==="
rg -n 'function prepare_meta_fields|public.*prepare_meta_fields|private.*prepare_meta_fields' -A 20 || echo "No explicit function definition found"
echo ""
echo "=== Checking for class definition and inheritance in WPUF_N8N_Integration.php ==="
head -50 "includes/Integrations/WPUF_N8N_Integration.php"Length of output: 7965
Fix PHP 7.1+ incompatibility and remove unused variables in line 88.
This file uses FieldableTrait which contains prepare_meta_fields(). The plugin supports PHP 5.6+ (per readme.txt and composer.json), but line 88 uses array destructuring syntax [ ... ] which requires PHP 7.1+. This is a breaking change. Additionally, $multi_repeated and $files are destructured but never used.
Use only what's needed:
- [ $meta_key_value, $multi_repeated, $files ] = self::prepare_meta_fields( $meta_vars );
+ [ $meta_key_value ] = self::prepare_meta_fields( $meta_vars );Or for PHP 5.6 compatibility:
- [ $meta_key_value, $multi_repeated, $files ] = self::prepare_meta_fields( $meta_vars );
+ $prepared = self::prepare_meta_fields( $meta_vars );
+ $meta_key_value = $prepared[0];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [ $meta_key_value, $multi_repeated, $files ] = self::prepare_meta_fields( $meta_vars ); | |
| foreach ( $meta_key_value as $meta_key => $meta_value ) { | |
| $post_data[$meta_key] = $meta_value; | |
| } | |
| $prepared = self::prepare_meta_fields( $meta_vars ); | |
| $meta_key_value = $prepared[0]; | |
| foreach ( $meta_key_value as $meta_key => $meta_value ) { | |
| $post_data[$meta_key] = $meta_value; | |
| } |
🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[failure] 91-91:
Array keys must be surrounded by spaces unless they contain a string or an integer.
[failure] 88-88:
The shorthand list syntax "[]" to destructure arrays is not available in PHP 7.0 or earlier.
🪛 PHPMD (2.15.0)
88-88: Avoid unused local variables such as '$multi_repeated'. (undefined)
(UnusedLocalVariable)
88-88: Avoid unused local variables such as '$files'. (undefined)
(UnusedLocalVariable)
🤖 Prompt for AI Agents
In includes/Integrations/WPUF_N8N_Integration.php around lines 88 to 92, the
code uses PHP 7.1 array destructuring and binds unused variables; replace the
destructuring with PHP 5.6-compatible handling by calling
self::prepare_meta_fields($meta_vars) into a single variable and then extract
only the first element (the $meta_key_value) for use, dropping $multi_repeated
and $files; update the subsequent loop to use that extracted $meta_key_value.
| $response = wp_remote_post( | ||
| $n8n_webhook_url, [ | ||
| 'method' => 'POST', | ||
| 'timeout' => 30, | ||
| 'headers' => $headers, | ||
| 'body' => wp_json_encode( $post_data ), | ||
| ] | ||
| ); | ||
| } |
There was a problem hiding this comment.
Critical: Add error handling and response validation.
The webhook request response is captured but never checked for errors. Failed webhook deliveries will fail silently, making it impossible for users to debug integration issues.
Add error handling and logging:
// Send data to N8N webhook
$response = wp_remote_post(
$n8n_webhook_url, [
'method' => 'POST',
'timeout' => 30,
'headers' => $headers,
'body' => wp_json_encode( $post_data ),
]
);
+
+ // Check for errors
+ if ( is_wp_error( $response ) ) {
+ \WP_User_Frontend::log( 'n8n', 'Webhook delivery failed: ' . $response->get_error_message() );
+ return;
+ }
+
+ $response_code = wp_remote_retrieve_response_code( $response );
+ if ( $response_code < 200 || $response_code >= 300 ) {
+ $response_body = wp_remote_retrieve_body( $response );
+ \WP_User_Frontend::log( 'n8n', sprintf( 'Webhook returned %d: %s', $response_code, $response_body ) );
+ }
}🧰 Tools
🪛 PHPMD (2.15.0)
142-142: Avoid unused local variables such as '$response'. (undefined)
(UnusedLocalVariable)
🤖 Prompt for AI Agents
In includes/Integrations/WPUF_N8N_Integration.php around lines 142 to 150, the
wp_remote_post response is not validated or error-checked; update the code to
check for WP_Error (is_wp_error), verify HTTP status is 2xx, parse/validate the
response body (json_decode and check for expected keys), and log failures.
Specifically: after wp_remote_post, if is_wp_error($response) log the error
message and return/handle accordingly; otherwise get the status with
wp_remote_retrieve_response_code and if it's not between 200 and 299 log the
status, response body and headers; attempt to json_decode the body and validate
expected fields, logging malformed JSON; use a consistent logger (error_log or
WordPress logger) and include contextual info (webhook URL, payload id/user) in
logs to aid debugging.
| public function add_n8n_settings_fields( $fields ) { | ||
| $settings = [ | ||
| 'n8n' => [ | ||
| [ | ||
| 'name' => 'authentication_type', | ||
| 'label' => __( 'Authentication Type', 'wp-user-frontend' ), | ||
| 'desc' => __( | ||
| 'Select the authentication type for the N8N integration.', | ||
| 'wp-user-frontend' | ||
| ), | ||
| 'type' => 'select', | ||
| 'default' => 'none', | ||
| 'options' => [ | ||
| 'none' => __( 'None', 'wp-user-frontend' ), | ||
| 'basic_auth' => __( 'Basic Auth', 'wp-user-frontend' ), | ||
| 'header_auth' => __( 'Header Auth', 'wp-user-frontend' ), | ||
| 'jwt_auth' => __( 'JWT Auth', 'wp-user-frontend' ), | ||
| ], | ||
| ], | ||
| [ | ||
| 'name' => 'basic_auth_username', | ||
| 'label' => __( 'Basic Auth Username', 'wp-user-frontend' ), | ||
| 'desc' => __( | ||
| 'Enter the username for the Basic Auth authentication.', | ||
| 'wp-user-frontend' | ||
| ), | ||
| 'type' => 'text', | ||
| 'depends_on' => 'authentication_type', | ||
| 'depends_on_value' => 'basic_auth', | ||
| ], | ||
| [ | ||
| 'name' => 'basic_auth_password', | ||
| 'label' => __( 'Basic Auth Password', 'wp-user-frontend' ), | ||
| 'desc' => __( | ||
| 'Enter the password for the Basic Auth authentication.', | ||
| 'wp-user-frontend' | ||
| ), | ||
| 'type' => 'text', | ||
| 'callback' => 'wpuf_settings_password_preview', | ||
| 'depends_on' => 'authentication_type', | ||
| 'depends_on_value' => 'basic_auth', | ||
| ], | ||
| [ | ||
| 'name' => 'header_auth_name', | ||
| 'label' => __( 'Name', 'wp-user-frontend' ), | ||
| 'desc' => __( | ||
| 'Enter the name for the Header Auth authentication.', | ||
| 'wp-user-frontend' | ||
| ), | ||
| 'type' => 'text', | ||
| 'depends_on' => 'authentication_type', | ||
| 'depends_on_value' => 'header_auth', | ||
| ], | ||
| [ | ||
| 'name' => 'header_auth_value', | ||
| 'label' => __( 'Header Auth Value', 'wp-user-frontend' ), | ||
| 'desc' => __( | ||
| 'Enter the value for the Header Auth authentication.', | ||
| 'wp-user-frontend' | ||
| ), | ||
| 'type' => 'text', | ||
| 'callback' => 'wpuf_settings_password_preview', | ||
| 'depends_on' => 'authentication_type', | ||
| 'depends_on_value' => 'header_auth', | ||
| ], | ||
| [ | ||
| 'name' => 'jwt_key_type', | ||
| 'label' => __( 'JWT Key Type', 'wp-user-frontend' ), | ||
| 'desc' => __( | ||
| 'Select the type of JWT key for the JWT Auth authentication.', | ||
| 'wp-user-frontend' | ||
| ), | ||
| 'type' => 'select', | ||
| 'default' => 'passphrase', | ||
| 'options' => [ | ||
| 'passphrase' => __( 'Passphrase', 'wp-user-frontend' ), | ||
| 'pem_key' => __( 'PEM Key', 'wp-user-frontend' ), | ||
| ], | ||
| 'depends_on' => 'authentication_type', | ||
| 'depends_on_value' => 'jwt_auth', | ||
| ], | ||
| [ | ||
| 'name' => 'jwt_key_passphrase', | ||
| 'label' => __( 'Passphrase', 'wp-user-frontend' ), | ||
| 'desc' => __( | ||
| 'Enter the passphrase for the JWT Auth authentication.', | ||
| 'wp-user-frontend' | ||
| ), | ||
| 'type' => 'text', | ||
| 'callback' => 'wpuf_settings_password_preview', | ||
| 'depends_on' => [ | ||
| 'authentication_type' => 'jwt_auth', | ||
| 'jwt_key_type' => 'passphrase' | ||
| ], | ||
| ], | ||
| [ | ||
| 'name' => 'jwt_key_pem_key', | ||
| 'label' => __( 'PEM Key', 'wp-user-frontend' ), | ||
| 'desc' => __( | ||
| 'Enter the PEM key for the JWT Auth authentication.', | ||
| 'wp-user-frontend' | ||
| ), | ||
| 'type' => 'text', | ||
| 'callback' => 'wpuf_settings_password_preview', | ||
| 'depends_on' => [ | ||
| 'authentication_type' => 'jwt_auth', | ||
| 'jwt_key_type' => 'pem_key', | ||
| ], | ||
| ], | ||
| ], | ||
| ]; | ||
|
|
||
| return array_merge( $fields, $settings ); | ||
| } |
There was a problem hiding this comment.
Consider encrypting sensitive authentication credentials.
Authentication credentials (passwords, JWT secrets, API keys) are currently stored in plain text in the wp_options table. This poses a security risk if the database is compromised.
Consider implementing encryption for sensitive fields:
- Use WordPress's built-in encryption functions or a library like
sodium(available in PHP 7.2+) - Encrypt values before storing with
update_option() - Decrypt when retrieving with
get_option()
Alternatively, document this security consideration and recommend that users:
- Store N8N webhooks on a secure network
- Use webhook URL query parameters for authentication instead of storing credentials
- Regularly rotate credentials
🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[failure] 272-272:
There should be a comma after the last array item in a multi-line array.
[failure] 239-239:
Multi-line function call not indented correctly; expected 24 spaces but found 28
Fix coding standards violations.
The PHPCS static analysis identified formatting issues that should be corrected for consistency with WordPress coding standards.
Apply these fixes:
'desc' => __(
'Enter the value for the Header Auth authentication.',
- 'wp-user-frontend'
- ),
+ 'wp-user-frontend'
+ ), 'depends_on' => [
'authentication_type' => 'jwt_auth',
- 'jwt_key_type' => 'passphrase'
+ 'jwt_key_type' => 'passphrase',
], 'depends_on' => [
'authentication_type' => 'jwt_auth',
'jwt_key_type' => 'pem_key',
],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public function add_n8n_settings_fields( $fields ) { | |
| $settings = [ | |
| 'n8n' => [ | |
| [ | |
| 'name' => 'authentication_type', | |
| 'label' => __( 'Authentication Type', 'wp-user-frontend' ), | |
| 'desc' => __( | |
| 'Select the authentication type for the N8N integration.', | |
| 'wp-user-frontend' | |
| ), | |
| 'type' => 'select', | |
| 'default' => 'none', | |
| 'options' => [ | |
| 'none' => __( 'None', 'wp-user-frontend' ), | |
| 'basic_auth' => __( 'Basic Auth', 'wp-user-frontend' ), | |
| 'header_auth' => __( 'Header Auth', 'wp-user-frontend' ), | |
| 'jwt_auth' => __( 'JWT Auth', 'wp-user-frontend' ), | |
| ], | |
| ], | |
| [ | |
| 'name' => 'basic_auth_username', | |
| 'label' => __( 'Basic Auth Username', 'wp-user-frontend' ), | |
| 'desc' => __( | |
| 'Enter the username for the Basic Auth authentication.', | |
| 'wp-user-frontend' | |
| ), | |
| 'type' => 'text', | |
| 'depends_on' => 'authentication_type', | |
| 'depends_on_value' => 'basic_auth', | |
| ], | |
| [ | |
| 'name' => 'basic_auth_password', | |
| 'label' => __( 'Basic Auth Password', 'wp-user-frontend' ), | |
| 'desc' => __( | |
| 'Enter the password for the Basic Auth authentication.', | |
| 'wp-user-frontend' | |
| ), | |
| 'type' => 'text', | |
| 'callback' => 'wpuf_settings_password_preview', | |
| 'depends_on' => 'authentication_type', | |
| 'depends_on_value' => 'basic_auth', | |
| ], | |
| [ | |
| 'name' => 'header_auth_name', | |
| 'label' => __( 'Name', 'wp-user-frontend' ), | |
| 'desc' => __( | |
| 'Enter the name for the Header Auth authentication.', | |
| 'wp-user-frontend' | |
| ), | |
| 'type' => 'text', | |
| 'depends_on' => 'authentication_type', | |
| 'depends_on_value' => 'header_auth', | |
| ], | |
| [ | |
| 'name' => 'header_auth_value', | |
| 'label' => __( 'Header Auth Value', 'wp-user-frontend' ), | |
| 'desc' => __( | |
| 'Enter the value for the Header Auth authentication.', | |
| 'wp-user-frontend' | |
| ), | |
| 'type' => 'text', | |
| 'callback' => 'wpuf_settings_password_preview', | |
| 'depends_on' => 'authentication_type', | |
| 'depends_on_value' => 'header_auth', | |
| ], | |
| [ | |
| 'name' => 'jwt_key_type', | |
| 'label' => __( 'JWT Key Type', 'wp-user-frontend' ), | |
| 'desc' => __( | |
| 'Select the type of JWT key for the JWT Auth authentication.', | |
| 'wp-user-frontend' | |
| ), | |
| 'type' => 'select', | |
| 'default' => 'passphrase', | |
| 'options' => [ | |
| 'passphrase' => __( 'Passphrase', 'wp-user-frontend' ), | |
| 'pem_key' => __( 'PEM Key', 'wp-user-frontend' ), | |
| ], | |
| 'depends_on' => 'authentication_type', | |
| 'depends_on_value' => 'jwt_auth', | |
| ], | |
| [ | |
| 'name' => 'jwt_key_passphrase', | |
| 'label' => __( 'Passphrase', 'wp-user-frontend' ), | |
| 'desc' => __( | |
| 'Enter the passphrase for the JWT Auth authentication.', | |
| 'wp-user-frontend' | |
| ), | |
| 'type' => 'text', | |
| 'callback' => 'wpuf_settings_password_preview', | |
| 'depends_on' => [ | |
| 'authentication_type' => 'jwt_auth', | |
| 'jwt_key_type' => 'passphrase' | |
| ], | |
| ], | |
| [ | |
| 'name' => 'jwt_key_pem_key', | |
| 'label' => __( 'PEM Key', 'wp-user-frontend' ), | |
| 'desc' => __( | |
| 'Enter the PEM key for the JWT Auth authentication.', | |
| 'wp-user-frontend' | |
| ), | |
| 'type' => 'text', | |
| 'callback' => 'wpuf_settings_password_preview', | |
| 'depends_on' => [ | |
| 'authentication_type' => 'jwt_auth', | |
| 'jwt_key_type' => 'pem_key', | |
| ], | |
| ], | |
| ], | |
| ]; | |
| return array_merge( $fields, $settings ); | |
| } | |
| public function add_n8n_settings_fields( $fields ) { | |
| $settings = [ | |
| 'n8n' => [ | |
| [ | |
| 'name' => 'authentication_type', | |
| 'label' => __( 'Authentication Type', 'wp-user-frontend' ), | |
| 'desc' => __( | |
| 'Select the authentication type for the N8N integration.', | |
| 'wp-user-frontend' | |
| ), | |
| 'type' => 'select', | |
| 'default' => 'none', | |
| 'options' => [ | |
| 'none' => __( 'None', 'wp-user-frontend' ), | |
| 'basic_auth' => __( 'Basic Auth', 'wp-user-frontend' ), | |
| 'header_auth' => __( 'Header Auth', 'wp-user-frontend' ), | |
| 'jwt_auth' => __( 'JWT Auth', 'wp-user-frontend' ), | |
| ], | |
| ], | |
| [ | |
| 'name' => 'basic_auth_username', | |
| 'label' => __( 'Basic Auth Username', 'wp-user-frontend' ), | |
| 'desc' => __( | |
| 'Enter the username for the Basic Auth authentication.', | |
| 'wp-user-frontend' | |
| ), | |
| 'type' => 'text', | |
| 'depends_on' => 'authentication_type', | |
| 'depends_on_value' => 'basic_auth', | |
| ], | |
| [ | |
| 'name' => 'basic_auth_password', | |
| 'label' => __( 'Basic Auth Password', 'wp-user-frontend' ), | |
| 'desc' => __( | |
| 'Enter the password for the Basic Auth authentication.', | |
| 'wp-user-frontend' | |
| ), | |
| 'type' => 'text', | |
| 'callback' => 'wpuf_settings_password_preview', | |
| 'depends_on' => 'authentication_type', | |
| 'depends_on_value' => 'basic_auth', | |
| ], | |
| [ | |
| 'name' => 'header_auth_name', | |
| 'label' => __( 'Name', 'wp-user-frontend' ), | |
| 'desc' => __( | |
| 'Enter the name for the Header Auth authentication.', | |
| 'wp-user-frontend' | |
| ), | |
| 'type' => 'text', | |
| 'depends_on' => 'authentication_type', | |
| 'depends_on_value' => 'header_auth', | |
| ], | |
| [ | |
| 'name' => 'header_auth_value', | |
| 'label' => __( 'Header Auth Value', 'wp-user-frontend' ), | |
| 'desc' => __( | |
| 'Enter the value for the Header Auth authentication.', | |
| 'wp-user-frontend' | |
| ), | |
| 'type' => 'text', | |
| 'callback' => 'wpuf_settings_password_preview', | |
| 'depends_on' => 'authentication_type', | |
| 'depends_on_value' => 'header_auth', | |
| ], | |
| [ | |
| 'name' => 'jwt_key_type', | |
| 'label' => __( 'JWT Key Type', 'wp-user-frontend' ), | |
| 'desc' => __( | |
| 'Select the type of JWT key for the JWT Auth authentication.', | |
| 'wp-user-frontend' | |
| ), | |
| 'type' => 'select', | |
| 'default' => 'passphrase', | |
| 'options' => [ | |
| 'passphrase' => __( 'Passphrase', 'wp-user-frontend' ), | |
| 'pem_key' => __( 'PEM Key', 'wp-user-frontend' ), | |
| ], | |
| 'depends_on' => 'authentication_type', | |
| 'depends_on_value' => 'jwt_auth', | |
| ], | |
| [ | |
| 'name' => 'jwt_key_passphrase', | |
| 'label' => __( 'Passphrase', 'wp-user-frontend' ), | |
| 'desc' => __( | |
| 'Enter the passphrase for the JWT Auth authentication.', | |
| 'wp-user-frontend' | |
| ), | |
| 'type' => 'text', | |
| 'callback' => 'wpuf_settings_password_preview', | |
| 'depends_on' => [ | |
| 'authentication_type' => 'jwt_auth', | |
| 'jwt_key_type' => 'passphrase', | |
| ], | |
| ], | |
| [ | |
| 'name' => 'jwt_key_pem_key', | |
| 'label' => __( 'PEM Key', 'wp-user-frontend' ), | |
| 'desc' => __( | |
| 'Enter the PEM key for the JWT Auth authentication.', | |
| 'wp-user-frontend' | |
| ), | |
| 'type' => 'text', | |
| 'callback' => 'wpuf_settings_password_preview', | |
| 'depends_on' => [ | |
| 'authentication_type' => 'jwt_auth', | |
| 'jwt_key_type' => 'pem_key', | |
| ], | |
| ], | |
| ], | |
| ]; | |
| return array_merge( $fields, $settings ); | |
| } |
🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[failure] 272-272:
There should be a comma after the last array item in a multi-line array.
[failure] 239-239:
Multi-line function call not indented correctly; expected 24 spaces but found 28
🤖 Prompt for AI Agents
in includes/Integrations/WPUF_N8N_Integration.php around lines 180 to 293, PHPCS
flagged WordPress coding-standards formatting issues; adjust indentation and
spacing to match WP standards: use 1 tab per indent level, align array
keys/values with single space around =>, ensure multiline arrays use trailing
commas on last item, wrap long function argument lists and array entries to
respect 80-120 char limits, place closing array brackets and parentheses on
their own lines and ensure consistent blank lines between blocks; run phpcs
--standard=WordPress and apply its suggested fixes to verify.
| private function create_jwt_token( $secret_key ) { | ||
| // Simple JWT implementation - you might want to use a proper JWT library | ||
| $header = json_encode( [ | ||
| 'typ' => 'JWT', | ||
| 'alg' => 'HS256' | ||
| ] ); | ||
|
|
||
| $payload = json_encode( [ | ||
| 'iat' => time(), | ||
| 'exp' => time() + 3600, // 1 hour expiration | ||
| 'iss' => get_site_url(), | ||
| ] ); | ||
|
|
||
| $base64_header = str_replace( ['+', '/', '='], ['-', '_', ''], base64_encode( $header ) ); | ||
| $base64_payload = str_replace( ['+', '/', '='], ['-', '_', ''], base64_encode( $payload ) ); | ||
|
|
||
| $signature = hash_hmac( 'sha256', $base64_header . '.' . $base64_payload, $secret_key, true ); | ||
| $base64_signature = str_replace( ['+', '/', '='], ['-', '_', ''], base64_encode( $signature ) ); | ||
|
|
||
| return $base64_header . '.' . $base64_payload . '.' . $base64_signature; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use a proper JWT library instead of custom implementation.
The custom JWT implementation works for basic cases but lacks important features and may have security implications. The comment on Line 355 acknowledges this concern.
Consider using a well-tested JWT library like firebase/php-jwt:
composer require firebase/php-jwtThen replace the custom implementation:
use Firebase\JWT\JWT;
private function create_jwt_token( $secret_key ) {
$payload = [
'iat' => time(),
'exp' => time() + 3600,
'iss' => get_site_url(),
];
try {
return JWT::encode( $payload, $secret_key, 'HS256' );
} catch ( \Exception $e ) {
\WP_User_Frontend::log( 'n8n', 'JWT generation failed: ' . $e->getMessage() );
return false;
}
}This provides:
- Proper JWT standard compliance
- Better error handling
- Support for various algorithms
- Security updates from the library maintainers
🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[warning] 371-371:
base64_encode() can be used to obfuscate code which is strongly discouraged. Please verify that the function is used for benign reasons.
[warning] 368-368:
base64_encode() can be used to obfuscate code which is strongly discouraged. Please verify that the function is used for benign reasons.
[warning] 367-367:
base64_encode() can be used to obfuscate code which is strongly discouraged. Please verify that the function is used for benign reasons.
[failure] 367-367:
Expected 1 space after the array opener in a single line array. Found: no spaces
[failure] 365-365:
Closing parenthesis of a multi-line function call must be on a line by itself
[failure] 361-361:
Opening parenthesis of a multi-line function call must be the last content on the line
[warning] 361-361:
json_encode() is discouraged. Use wp_json_encode() instead.
[failure] 359-359:
Closing parenthesis of a multi-line function call must be on a line by itself
[failure] 358-358:
There should be a comma after the last array item in a multi-line array.
[failure] 356-356:
Opening parenthesis of a multi-line function call must be the last content on the line
[warning] 356-356:
json_encode() is discouraged. Use wp_json_encode() instead.
🤖 Prompt for AI Agents
In includes/Integrations/WPUF_N8N_Integration.php around lines 354 to 374,
replace the custom JWT creation with a call to a vetted library (e.g.,
firebase/php-jwt): add the library via composer, import the JWT class at top of
the file, and update create_jwt_token to build the payload and call
JWT::encode($payload, $secret_key, 'HS256'); wrap the encode call in a try/catch
to log exceptions (WP_User_Frontend::log or process logger) and return false on
failure; ensure the function’s return semantics reflect string on success or
false on error.
Fix coding standards violations in JWT method.
WordPress coding standards require using wp_json_encode() instead of json_encode() and proper formatting for multi-line function calls.
Apply these fixes:
- $header = json_encode( [
+ $header = wp_json_encode(
+ [
'typ' => 'JWT',
- 'alg' => 'HS256'
- ] );
+ 'alg' => 'HS256',
+ ]
+ );
- $payload = json_encode( [
+ $payload = wp_json_encode(
+ [
'iat' => time(),
'exp' => time() + 3600, // 1 hour expiration
'iss' => get_site_url(),
- ] );
+ ]
+ );
- $base64_header = str_replace( ['+', '/', '='], ['-', '_', ''], base64_encode( $header ) );
+ $base64_header = str_replace( [ '+', '/', '=' ], [ '-', '_', '' ], base64_encode( $header ) );Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[warning] 371-371:
base64_encode() can be used to obfuscate code which is strongly discouraged. Please verify that the function is used for benign reasons.
[warning] 368-368:
base64_encode() can be used to obfuscate code which is strongly discouraged. Please verify that the function is used for benign reasons.
[warning] 367-367:
base64_encode() can be used to obfuscate code which is strongly discouraged. Please verify that the function is used for benign reasons.
[failure] 367-367:
Expected 1 space after the array opener in a single line array. Found: no spaces
[failure] 365-365:
Closing parenthesis of a multi-line function call must be on a line by itself
[failure] 361-361:
Opening parenthesis of a multi-line function call must be the last content on the line
[warning] 361-361:
json_encode() is discouraged. Use wp_json_encode() instead.
[failure] 359-359:
Closing parenthesis of a multi-line function call must be on a line by itself
[failure] 358-358:
There should be a comma after the last array item in a multi-line array.
[failure] 356-356:
Opening parenthesis of a multi-line function call must be the last content on the line
[warning] 356-356:
json_encode() is discouraged. Use wp_json_encode() instead.
🤖 Prompt for AI Agents
In includes/Integrations/WPUF_N8N_Integration.php around lines 354 to 374, the
create_jwt_token method uses json_encode and has compact multi-line
array/function calls that violate WP coding standards; replace json_encode()
with wp_json_encode() for both $header and $payload and reformat the multi-line
array and function calls to follow WP formatting (each array element and
function argument on its own line with proper indentation and trailing commas
where appropriate) while keeping the same logic and return value.
Lib/WeDevs_Settings_API.php
Outdated
There was a problem hiding this comment.
🧩 Analysis chain
Dependency logic breaks for checkboxes and radios
The script compares .val() to the expected value for all types. For checkboxes, .val() is “on” regardless of checked state; for radios, you must read the checked one. This causes fields not to show/hide correctly.
Apply this focused patch to handle checkbox/radio both during initial evaluation and on change:
- if ($depends_on.val() !== expected_value) {
- all_dependencies_met = false;
- }
+ var actual;
+ if ($depends_on_type === 'checkbox') {
+ const isChecked = $depends_on.is(':checked') || $depends_on.filter(':checked').length > 0;
+ // normalize expected: support booleans and on/off
+ const expectedStr = (expected_value === true) ? 'on' :
+ (expected_value === false) ? 'off' : String(expected_value);
+ actual = isChecked ? 'on' : 'off';
+ if (actual !== expectedStr) all_dependencies_met = false;
+ } else if ($depends_on_type === 'radio') {
+ const radioName = $depends_on.attr('name');
+ actual = $('input[name="'+ radioName +'"]:checked').val() || '';
+ if (String(actual) !== String(expected_value)) all_dependencies_met = false;
+ } else {
+ if (String($depends_on.val()) !== String(expected_value)) all_dependencies_met = false;
+ }
@@
- if ($depends_on.length === 0 || $depends_on.val() !== expected_value) {
- all_met = false;
- break;
- }
+ if ($depends_on.length === 0) { all_met = false; break; }
+ const type = $depends_on.attr('type');
+ let ok;
+ if (type === 'checkbox') {
+ const isChecked = $depends_on.is(':checked') || $depends_on.filter(':checked').length > 0;
+ const expectedStr = (expected_value === true) ? 'on' :
+ (expected_value === false) ? 'off' : String(expected_value);
+ ok = (isChecked ? 'on' : 'off') === expectedStr;
+ } else if (type === 'radio') {
+ const radioName = $depends_on.attr('name');
+ const actual = $('input[name="'+ radioName +'"]:checked').val() || '';
+ ok = String(actual) === String(expected_value);
+ } else {
+ ok = String($depends_on.val()) === String(expected_value);
+ }
+ if (!ok) { all_met = false; break; }After patching, retest with:
- toggle/toggle (on/off)
- checkbox true/false
- select/radio with scalar values
- object/JSON depends_on with multiple fields
🏁 Script executed:
#!/bin/bash
# Examine the file and lines mentioned in the review
fd -H 'WeDevs_Settings_API.php' -x cat {} | head -880 | tail -120Length of output: 6062
🏁 Script executed:
#!/bin/bash
# Check if the file exists and get file info
fd -H 'WeDevs_Settings_API.php' -x ls -lh {}Length of output: 77
🏁 Script executed:
#!/bin/bash
# Look for other dependencies handling in the same file to understand context
fd -H 'WeDevs_Settings_API.php' -x grep -n "depends-on" {} | head -20Length of output: 1454
🌐 Web query:
jQuery checkbox .val() vs .is(':checked') how to get checkbox state
💡 Result:
Short answer: .val() returns the checkbox's value attribute (e.g. "on" by default) — not whether it's checked. Use .is(':checked'), .prop('checked') or the native element.checked to get the checked state (they return a boolean).
Examples:
- $("#cb").is(":checked") // true/false
- $("#cb").prop("checked") // true/false
- document.getElementById("cb").checked // true/false
References: jQuery docs on prop() and checked behavior [1]; common discussion / examples on using .is(':checked') / .prop('checked') instead of .val() [2].
🌐 Web query:
jQuery radio button .val() get selected value checked
💡 Result:
Use the :checked selector on the radio group, then .val():
-
Get selected value:
var val = $('input[name="groupName"]:checked').val(); -
Check boolean state (if needed):
if ($('input[name="groupName"]').is(':checked')) { ... }
or
if ($('#someRadio').prop('checked')) { ... }
Notes:
- Ensure all radios share the same name and each has a value (otherwise the default value is "on"). [1][2]
Sources: [1] StackOverflow examples; [2] jQuery .prop() docs.
🏁 Script executed:
#!/bin/bash
# Verify the event handler loop and closure scope
cat > /tmp/analyze_closure.js << 'EOF'
// Simulating the structure to verify closure handling
var dependency_fields = {
field1: $('input1'),
field2: $('input2')
};
var dependencies = {
field1: 'value1',
field2: 'value2'
};
// Event handler loop
for (var field_name in dependency_fields) {
var $depends_on = dependency_fields[field_name];
var expected_value = dependencies[field_name];
var $depends_on_type = $depends_on.attr('type');
// Handler doesn't capture loop variables directly
// It only calls checkAllDependencies() which re-queries DOM
if ($depends_on_type === 'checkbox') {
// This handler does NOT use $depends_on, expected_value, $depends_on_type
// It only calls checkAllDependencies()
$depends_on.on('change', function() {
checkAllDependencies(); // Uses closure vars: dependencies, $this
});
}
}
// checkAllDependencies re-queries DOM each time
function checkAllDependencies() {
for (var field_name in dependencies) {
// Re-queries, doesn't rely on loop variables from outer loop
var $depends_on = $("input[id*='"+ field_name +"']");
var expected_value = dependencies[field_name];
// ... rest of logic
}
}
EOF
echo "Closure analysis: Event handlers call checkAllDependencies() without capturing loop variables."
echo "The re-query pattern is inefficient but not buggy."Length of output: 145
🏁 Script executed:
#!/bin/bash
# Search for any existing checkbox/radio handling in same file
fd -H 'WeDevs_Settings_API.php' -x grep -n "checkbox\|radio\|:checked\|prop.*checked" {} | head -30Length of output: 1334
Dependency logic breaks for checkboxes and radios
The script compares .val() to the expected value for all input types. For checkboxes, .val() returns the value attribute ("on" by default) regardless of checked state; for radios, you must select the checked one. This prevents fields from showing/hide correctly based on checkbox/radio dependencies.
Apply this focused patch to handle checkbox/radio types during both initial evaluation and on change:
- if ($depends_on.val() !== expected_value) {
- all_dependencies_met = false;
- }
+ var actual;
+ if ($depends_on_type === 'checkbox') {
+ const isChecked = $depends_on.is(':checked') || $depends_on.filter(':checked').length > 0;
+ // normalize expected: support booleans and on/off
+ const expectedStr = (expected_value === true) ? 'on' :
+ (expected_value === false) ? 'off' : String(expected_value);
+ actual = isChecked ? 'on' : 'off';
+ if (actual !== expectedStr) all_dependencies_met = false;
+ } else if ($depends_on_type === 'radio') {
+ const radioName = $depends_on.attr('name');
+ actual = $('input[name="'+ radioName +'"]:checked').val() || '';
+ if (String(actual) !== String(expected_value)) all_dependencies_met = false;
+ } else {
+ if (String($depends_on.val()) !== String(expected_value)) all_dependencies_met = false;
+ }
@@
- if ($depends_on.length === 0 || $depends_on.val() !== expected_value) {
- all_met = false;
- break;
- }
+ if ($depends_on.length === 0) { all_met = false; break; }
+ const type = $depends_on.attr('type');
+ let ok;
+ if (type === 'checkbox') {
+ const isChecked = $depends_on.is(':checked') || $depends_on.filter(':checked').length > 0;
+ const expectedStr = (expected_value === true) ? 'on' :
+ (expected_value === false) ? 'off' : String(expected_value);
+ ok = (isChecked ? 'on' : 'off') === expectedStr;
+ } else if (type === 'radio') {
+ const radioName = $depends_on.attr('name');
+ const actual = $('input[name="'+ radioName +'"]:checked').val() || '';
+ ok = String(actual) === String(expected_value);
+ } else {
+ ok = String($depends_on.val()) === String(expected_value);
+ }
+ if (!ok) { all_met = false; break; }Test after patching with: toggle/checked states, checkbox true/false, radio with scalar values, and object/JSON depends-on with multiple fields.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In Lib/WeDevs_Settings_API.php around lines 762 to 879, the dependency checks
use $depends_on.val() for all input types which fails for checkboxes and radios;
update the logic in both the initial dependency evaluation and the
checkAllDependencies() function to handle input types: if the element is a
checkbox, treat the dependency as met when the checkbox.checked state
(true/false) matches the expected value (coerce expected "true"/"false" or
boolean accordingly); if the element is a radio group, select the checked radio
($("input[name='...']:checked") or filter by id/name) and compare its value to
the expected value; also simplify event binding to use change for checkboxes and
radios and keyup/change for other inputs, and ensure you re-query the correct
checked element inside checkAllDependencies() rather than relying on .val() of
the group element.
| /** | ||
| * Password preview field callback | ||
| * | ||
| * @param array $args Field arguments | ||
| * @since WPUF_PRO_SINCE | ||
| */ | ||
| function wpuf_settings_password_preview( $args ) { | ||
| wpuf_require_once( WPUF_ROOT . '/Lib/WeDevs_Settings_API.php' ); | ||
|
|
||
| $settings = new WeDevs_Settings_API(); | ||
| $value = $settings->get_option( $args['id'], $args['section'], $args['std'] ); | ||
| $disabled = ! empty( $args['is_pro_preview'] ) && $args['is_pro_preview'] ? 'disabled' : ''; | ||
| $size = isset( $args['size'] ) && ! is_null( $args['size'] ) ? $args['size'] : 'regular'; | ||
|
|
||
| // Create masked preview of the password | ||
| $preview_value = ''; | ||
| if ( ! empty( $value ) ) { | ||
| $length = strlen( $value ); | ||
| if ( $length >= 4 ) { | ||
| $preview_value = substr( $value, 0, 2 ) . str_repeat( '*', $length - 4 ) . substr( $value, -2 ); | ||
| } else { | ||
| $preview_value = str_repeat( '*', $length ); | ||
| } | ||
| } | ||
|
|
||
| $depends_on = ! empty( $args['depends_on'] ) ? $args['depends_on'] : ''; | ||
| $depends_on_value = ! empty( $args['depends_on_value'] ) ? $args['depends_on_value'] : ''; | ||
|
|
||
| // Handle array dependencies | ||
| if (is_array($depends_on)) { | ||
| $depends_on_json = esc_attr( json_encode($depends_on) ); | ||
| $depends_on_value = ''; // Not used for array format | ||
| } else { | ||
| $depends_on_json = esc_attr( $depends_on ); | ||
| } | ||
|
|
||
| $html = sprintf( '<input type="text" class="%1$s-text" id="%2$s[%3$s]" name="%2$s[%3$s]" value="%4$s" %5$s data-depends-on=\'%6$s\' data-depends-on-value="%7$s"/>', $size, $args['section'], $args['id'], $preview_value, $disabled, $depends_on_json, esc_attr( $depends_on_value ) ); | ||
| $html .= $settings->get_field_description( $args ); | ||
|
|
||
| if ( ! empty( $args['is_pro_preview'] ) && $args['is_pro_preview'] ) { | ||
| $html .= wpuf_get_pro_preview_html(); | ||
| } | ||
|
|
||
| echo wp_kses( $html, array( 'input' => array( 'type' => array(), 'class' => array(), 'id' => array(), 'name' => array(), 'value' => array(), 'readonly' => array(), 'style' => array(), 'disabled' => array(), 'data-depends-on' => array(), 'data-depends-on-value' => array() ), 'p' => array( 'class' => array() ), 'div' => array( 'class' => array() ), 'a' => array( 'href' => array(), 'target' => array(), 'class' => array() ), 'span' => array( 'class' => array() ), 'svg' => array( 'width' => array(), 'height' => array(), 'viewBox' => array(), 'fill' => array(), 'xmlns' => array() ), 'path' => array( 'd' => array(), 'fill' => array() ) ) ); | ||
| } |
There was a problem hiding this comment.
Critical: masked preview will overwrite the real secret on save
The preview input has the real option name. Submitting the form will save the masked value (e.g., ab****yz) to the database, corrupting the secret.
Fix now:
- Render a disabled, read-only preview without the option name, and post the actual value via a hidden input (or better: handle preservation via sanitize_callback to avoid posting secrets).
Apply this minimal patch to avoid overwriting on save:
- $html = sprintf( '<input type="text" class="%1$s-text" id="%2$s[%3$s]" name="%2$s[%3$s]" value="%4$s" %5$s data-depends-on=\'%6$s\' data-depends-on-value="%7$s"/>', $size, $args['section'], $args['id'], $preview_value, $disabled, $depends_on_json, esc_attr( $depends_on_value ) );
+ // Show preview only; don't bind the option name to avoid saving the masked value.
+ $html = sprintf(
+ '<input type="text" class="%1$s-text" id="preview-%2$s[%3$s]" value="%4$s" readonly disabled aria-readonly="true" data-depends-on=\'%6$s\' data-depends-on-value="%7$s"/>',
+ $size, $args['section'], $args['id'], $preview_value, $disabled, $depends_on_json, esc_attr( $depends_on_value )
+ );
+ // Preserve the saved value on submit. NOTE: This posts the secret back to the server; prefer a sanitize_callback to keep the existing value if unchanged.
+ $html .= sprintf(
+ '<input type="hidden" name="%1$s[%2$s]" value="%3$s" />',
+ $args['section'], $args['id'], esc_attr( $value )
+ );Recommended next step (safer):
- Register this field with a sanitize_callback that detects masked or empty submissions and preserves the existing stored value instead of overwriting it.
I can provide the sanitize_callback and wiring where this field is registered.
🤖 Prompt for AI Agents
In wpuf-functions.php around lines 4041 to 4085 the password preview input uses
the real option name so submitting the form will store the masked preview back
into the option; change the markup to output a non-submittable preview (disabled
and readonly, using no name/id that maps to the option) and add a separate
hidden input that carries the real option value (name matching the option) so
saves persist the original secret; alternatively (recommended) register a
sanitize_callback for this field that detects masked or empty submissions and
returns the existing stored value to prevent overwriting—implement the
disabled/hidden input change now and add the sanitize_callback wiring as the
safer follow-up.
Rubaiyat-E-Mohammad
added
QA Approved
|
Input box for turnstile site key and secret key is not visible in settings page. @sapayth vai 
|
Rubaiyat-E-Mohammad
added
bug
and removed
QA Approved
…tion_for_post_form
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Lib/WeDevs_Settings_API.php (1)
873-873: Dependency logic breaks for checkboxes and radios.The comparison
$depends_on.val() !== expected_valueon lines 873 and 924 fails for checkboxes and radios. For checkboxes,.val()returns the value attribute ("on" by default) regardless of checked state. For radios, you must select the checked radio first.This issue was previously flagged with a detailed fix that has not been implemented.
Apply the patch from the previous review to handle checkbox and radio types correctly by checking the checked state for checkboxes and selecting the checked radio for radio groups before comparing values.
Also applies to: 924-924
🧹 Nitpick comments (2)
Lib/WeDevs_Settings_API.php (2)
847-864: Extract duplicated selector logic into a helper function.The selector fallback logic (trying multiple patterns to find the dependency field) is duplicated in both the initial dependency check and the
checkAllDependencies()function. This makes maintenance harder and increases code size.Consider extracting this logic into a helper function:
+ // Helper function to find dependency field + function findDependencyField(field_name) { + var $field = $("input[id*='"+ field_name +"'], select[id*='"+ field_name +"']"); + if ($field.length === 0) { + $field = $("input[name*='["+ field_name +"]'], select[name*='["+ field_name +"]']"); + } + if ($field.length === 0) { + $field = $("input[id*='["+ field_name +"]'], select[id*='["+ field_name +"]']"); + } + if ($field.length === 0) { + $field = $("input[id*='"+ field_name +"'], select[id*='"+ field_name +"']"); + } + return $field; + }Then use
findDependencyField(field_name)in both locations instead of repeating the selector logic.Also applies to: 907-922
902-936: Function defined inside loop creates unnecessary closures.The
checkAllDependencies()function is defined inside the.each()loop, creating a new function instance for each field with dependencies. While this works due to closures capturing$thisanddependencies, it's inefficient and could impact performance with many dependent fields.Consider refactoring to avoid creating multiple function instances. One approach is to store the dependencies and target element as data attributes or in a map, then use a single shared function.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Lib/WeDevs_Settings_API.php(5 hunks)assets/js/wpuf-form-builder.js(1 hunks)wpuf-functions.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- assets/js/wpuf-form-builder.js
- wpuf-functions.php
🧰 Additional context used
🧬 Code graph analysis (1)
Lib/WeDevs_Settings_API.php (1)
wpuf-functions.php (1)
wpuf_get_pro_preview_html(4698-4704)
| $value = esc_attr( $this->get_option( $args['id'], $args['section'], $args['std'] ) ); | ||
| $disabled = ! empty( $args['is_pro_preview'] ) && $args['is_pro_preview'] ? 'disabled' : ''; | ||
| $size = isset( $args['size'] ) && ! is_null( $args['size'] ) ? $args['size'] : 'regular'; | ||
| $depends_on = ! empty( $args['depends_on'] ) ? $args['depends_on'] : ''; |
There was a problem hiding this comment.
Missing 'placeholder' attribute in wp_kses allowed attributes.
The placeholder attribute is used in the sprintf on line 195 (set on line 181), but it's not included in the wp_kses allowed attributes list on line 204. This will cause the placeholder to be stripped from the output.
Apply this diff to add the missing attribute:
- echo wp_kses( $html, array('input' => ['type' => [],'class' => [],'id' => [],'name' => [],'value' => [],'disabled' => [],'data-depends-on' => [],'data-depends-on-value' => []], 'p' => ['class' => []], 'div' => ['class' => []], 'a' => ['href' => [],'target' => [],'class' => []], 'span' => ['class' => []], 'svg' => ['width' => [],'height' => [],'viewBox' => [],'fill' => [],'xmlns' => [],],));
+ echo wp_kses( $html, array('input' => ['type' => [],'class' => [],'id' => [],'name' => [],'value' => [],'placeholder' => [],'disabled' => [],'data-depends-on' => [],'data-depends-on-value' => []], 'p' => ['class' => []], 'div' => ['class' => []], 'a' => ['href' => [],'target' => [],'class' => []], 'span' => ['class' => []], 'svg' => ['width' => [],'height' => [],'viewBox' => [],'fill' => [],'xmlns' => [],],));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| echo wp_kses( $html, array('input' => ['type' => [],'class' => [],'id' => [],'name' => [],'value' => [],'disabled' => [],'data-depends-on' => [],'data-depends-on-value' => []], 'p' => ['class' => []], 'div' => ['class' => []], 'a' => ['href' => [],'target' => [],'class' => []], 'span' => ['class' => []], 'svg' => ['width' => [],'height' => [],'viewBox' => [],'fill' => [],'xmlns' => [],],)); | |
| echo wp_kses( $html, array('input' => ['type' => [],'class' => [],'id' => [],'name' => [],'value' => [],'placeholder' => [],'disabled' => [],'data-depends-on' => [],'data-depends-on-value' => []], 'p' => ['class' => []], 'div' => ['class' => []], 'a' => ['href' => [],'target' => [],'class' => []], 'span' => ['class' => []], 'svg' => ['width' => [],'height' => [],'viewBox' => [],'fill' => [],'xmlns' => [],],)); |
🤖 Prompt for AI Agents
In Lib/WeDevs_Settings_API.php around line 204, the wp_kses allowed-attributes
list for the input tag is missing the 'placeholder' attribute causing
placeholder text (set earlier) to be stripped; update the allowed attributes
array for 'input' to include 'placeholder' => [] so wp_kses will preserve
placeholder values in the rendered HTML.
2 participants
closes #1027
added n8n integration for post form. user can set up an n8n automation when a new post is created.
Summary by CodeRabbit
New Features
Bug Fixes