Fix password strength not enforced on checkout (minStrength hardcoded)#342
Fix password strength not enforced on checkout (minStrength hardcoded)#342superdav42 merged 10 commits intomainfrom
Conversation
checkout.js hardcoded minStrength: 3 when creating the password checker, overriding the PHP-configured minimum strength setting. This meant the "strong" (score 4) and "super_strong" settings were effectively treated as "medium" (score 3), allowing weaker passwords through. Removed the hardcoded override so the checker reads min_strength from wu_password_strength_settings (set by PHP). Also added re-initialization in the Vue updated() hook so the password checker activates when the field appears after initial mount in multi-step checkouts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRemoved explicit minStrength from the frontend password-strength initializer and added late-mount re-initialization; added E2E fixtures and tests for password strength; added libsodium fallback and OpenSSL-first logic for credential encryption; sanitised PWYW inputs with maximum enforcement; adjusted billing rendering/validation and tightened AJAX/Stripe security; plus assorted tests, translations, and minor formatting changes. Changes
Sequence DiagramsequenceDiagram
participant User
participant Browser
participant Vue as Vue Component
participant Strength as PasswordChecker
participant Server as WP-CLI/Server
User->>Browser: open registration page
Browser->>Vue: mount component
alt password field present at mount
Vue->>Strength: initialize checker (options without minStrength)
else password field appears after mount
Browser->>Vue: detect `#field-password` added
Vue->>Strength: initialize checker (re-init on late mount)
end
User->>Vue: type password
Vue->>Strength: compute score
Strength->>Vue: return score & validity
Vue->>Browser: update meter & validation UI
Note right of Server: (E2E) WP‑CLI fixture sets min strength for tests
Browser->>Server: (E2E) assert strength enforcement
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
assets/js/checkout.js (1)
1080-1083:password_strength_checkeris not declared in Vue'sdata, so it's non-reactive.The guard
!this.password_strength_checkerworks fine as a plain JS property check, but since it's not ininitial_data, Vue won't track it reactively. This is acceptable here because you're only using it as a one-shot initialization flag insideupdated(), not in templates or computed properties.Consider declaring
password_strength_checker: nullininitial_data(around Line 166) for clarity and to avoid a potential pitfall if someone later references it in a template or computed.Suggested addition to initial_data
valid_password: true, + password_strength_checker: null, stored_templates: {},tests/e2e/cypress/integration/050-password-strength-enforcement.spec.js (3)
26-26: Consider replacing fixedcy.wait(2000)with a deterministic assertion.Fixed waits are a common source of flakiness in CI (too short → fails; too long → slow). Since the password field is already confirmed visible on Line 25, you could instead wait for Vue to finish mounting, e.g., by asserting the existence of
wu_checkout_formon the window:cy.window().its('wu_checkout_form').should('exist');Same suggestion applies to
cy.wait(500)on Line 34 — you could retry-assert the strength label text instead.
5-15: Hardcoded container path is brittle but acceptable for wp-env.The path
/var/www/html/wp-content/plugins/ultimate-multisite/...is tightly coupled to the wp-env container layout. If the plugin folder is ever renamed or the Docker setup changes, this will silently break. Consider extracting it to a constant or Cypress env variable for easier maintenance, but this is low risk for a dedicated e2e setup.
32-52: Accessing__vue__on the DOM element assumes Vue 2.
win.document.querySelector("#wu_form").__vue__is a Vue 2 internal. This will break if the app migrates to Vue 3 (__vue_app__+getComponentPublicInstance). Fine for now, just worth a note if a Vue upgrade is ever planned.
Address critical, high, and medium security issues found in code audit since v2.4.10: - Add capability check and timing-safe state comparison to Stripe OAuth callback - Require authentication and nonce for payment status polling AJAX - Replace base64 fallback with sodium encryption in credential store - Add pattern-based filtering for sensitive settings in REST API - Add PWYW maximum amount validation and input sanitization on Cart - Restore double-replacement guard in site duplication - Keep billing_country required on free trials for tax calculation - Revert sandbox mode default to true for Stripe gateways - Add GitHub domain validation for beta update package URLs - Add re-entry guard to prevent recursive HTTP filter Fix e2e tests: - Handle custom login page redirect in loginByForm command - Fix billing field visibility with v-show defaulting to visible during AJAX - Reset password strength setting in setup to prevent cross-run failures - Use stronger test passwords that pass all strength levels Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@inc/checkout/signup-fields/class-signup-field-billing-address.php`:
- Line 271: The billing_country input is hidden with v-show but still validated
by get_validation_rules() in class-checkout.php, causing required validation
when users can't edit it; update the billing_country field handling in
class-signup-field-billing-address.php (the field with
wrapper_html_attr['v-show'] and key 'billing_country') so it either uses v-if to
remove it from the DOM when order.should_collect_payment is false, or change the
visibility condition to a separate v-show that keeps billing_country visible
when order.should_collect_payment is false; ensure the change aligns with
get_validation_rules() (the required_with rule for billing_country) so the field
is either present for users to correct or excluded from validation.
In `@inc/helpers/class-credential-store.php`:
- Around line 148-174: In decrypt_sodium(), wrap the call to
sodium_crypto_secretbox_open (the block that gets $key via
self::get_sodium_key() and calls sodium_crypto_secretbox_open($encrypted,
$nonce, $key)) in a try-catch that catches SodiumException and returns an empty
string on failure to preserve the existing graceful-empty-string contract;
ensure the function still returns '' for exceptions and unchanged behavior for
successful decryptions.
- Around line 72-80: The sodium branch currently calls random_bytes(),
get_sodium_key() and sodium_crypto_secretbox() without guarding for exceptions;
wrap the whole sodium path in a try/catch that mirrors the OpenSSL fallback so
any Throwable (or specifically SodiumException and Exception) is caught and
causes the function to return an empty string rather than letting the exception
bubble; ensure you still use self::get_sodium_key(),
SODIUM_CRYPTO_SECRETBOX_NONCEBYTES, sodium_crypto_secretbox(), and return
self::SODIUM_PREFIX . base64_encode($nonce . $encrypted) on success, but on
failure log or ignore the exception and return '' to preserve the
graceful-fallback behavior.
In `@lang/ultimate-multisite.pot`:
- Around line 4622-4623: Fix the two typos in the user-facing string: change
"scarry" to "scary" and "specially" to "especially" in the message literal used
by the Setup_Wizard_Admin_Page (the method that renders the "Starting from
scratch..." message), then update the source PHP (class Setup_Wizard_Admin_Page)
and regenerate the POT so the corrected msgid is exported.
🧹 Nitpick comments (2)
inc/class-wp-ultimo.php (1)
1023-1044: Re-entrancy guard looks correct; consider try/finally for resilience.The static
$is_redirectingflag properly prevents infinite recursion through thehttp_request_args→pre_http_request→wp_remote_getcycle. Currently safe sincewp_remote_getdoesn't throw, but wrapping in try/finally would make it resilient to future changes.♻️ Optional: use try/finally to guarantee flag reset
$is_redirecting = true; - $beta_url = add_query_arg('beta', '1', $request_url); - $result = wp_remote_get($beta_url, $args); - $is_redirecting = false; - - return $result; + try { + $beta_url = add_query_arg('beta', '1', $request_url); + return wp_remote_get($beta_url, $args); + } finally { + $is_redirecting = false; + }Regarding PHPMD's warning about unused
$ron line 1031: this is a false positive —$ris required by WordPress'spre_http_requestfilter signature (3 parameters:$pre,$parsed_args,$url).inc/helpers/class-credential-store.php (1)
187-196:get_sodium_key()is identical toget_encryption_key()— derive distinct keys per algorithm.Using the exact same key material for two different cryptographic primitives (AES-256-CBC and XSalsa20-Poly1305) violates the key-separation principle. Derive algorithm-specific subkeys with distinct context strings, e.g. via HKDF or simply hashing with a different domain label:
Proposed fix
private static function get_sodium_key(): string { - return hash('sha256', wp_salt('auth'), true); + return hash('sha256', 'wu_sodium_secretbox:' . wp_salt('auth'), true); }
⚠️ If you adopt this, any credentials already encrypted with the current sodium key will become undecryptable. Since sodium is only a fallback (used when OpenSSL is absent), this is likely safe if no production data was encrypted with libsodium yet. Verify before merging.
inc/checkout/signup-fields/class-signup-field-billing-address.php
Outdated
Show resolved
Hide resolved
lang/ultimate-multisite.pot
Outdated
| #: inc/admin-pages/class-setup-wizard-admin-page.php:465 | ||
| msgid "Starting from scratch can be scarry, specially when first starting out. In this step, you can create default content to have a starting point for your network. Everything can be customized later." |
There was a problem hiding this comment.
Pre-existing typo: "scarry" → "scary", "specially" → "especially".
The msgid on line 4623 contains "scarry" (should be "scary") and "specially" (should be "especially"). While this string isn't newly introduced in this PR (only the source reference was updated), it's worth fixing in the source PHP file (class-setup-wizard-admin-page.php:465) and regenerating the POT.
🤖 Prompt for AI Agents
In `@lang/ultimate-multisite.pot` around lines 4622 - 4623, Fix the two typos in
the user-facing string: change "scarry" to "scary" and "specially" to
"especially" in the message literal used by the Setup_Wizard_Admin_Page (the
method that renders the "Starting from scratch..." message), then update the
source PHP (class Setup_Wizard_Admin_Page) and regenerate the POT so the
corrected msgid is exported.
…ndling - Fix "scarry" → "scary" and "specially" → "especially" in setup wizard - Use v-if instead of v-show for billing_country so it's removed from the DOM when payment isn't required, preventing required_with validation from firing on a hidden field - Wrap sodium_crypto_secretbox_open in try-catch for SodiumException to preserve the graceful empty-string return contract Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
inc/checkout/signup-fields/class-signup-field-billing-address.php (1)
279-296:⚠️ Potential issue | 🟠 Major
v-ifforbilling_countryis silently overwritten when$zip_onlyis true.When
$zip_onlyis true, line 295 unconditionally setswrapper_html_attr['v-if'], replacing the payment-collection guard set on line 280. This meansbilling_countrywill remain in the DOM (and be submitted/validated) when payment is not required, as long as the gateway isn't Stripe — exactly the bug the newv-ifon line 280 was meant to fix.For non-
billing_countryfields the two directives coexist (v-show+v-if), but forbilling_countryboth arev-if, so the second write wins.Proposed fix — combine both conditions for billing_country
if ($zip_only) { - $field['wrapper_html_attr']['v-if'] = "!(gateway && gateway.startsWith('stripe'))"; + $stripe_condition = "!(gateway && gateway.startsWith('stripe'))"; + + if ('billing_country' === $field_key) { + // Merge the payment-collection guard with the Stripe guard + $field['wrapper_html_attr']['v-if'] = "(order === false || order.should_collect_payment) && {$stripe_condition}"; + } else { + $field['wrapper_html_attr']['v-if'] = $stripe_condition; + } }
🧹 Nitpick comments (1)
inc/helpers/class-credential-store.php (1)
188-202: Consider deriving distinct keys for each encryption backend.
get_sodium_key()andget_encryption_key()return identical values. While not exploitable here (the two schemes are mutually exclusive fallbacks and values are prefix-tagged), cryptographic best practice is to use distinct key material per algorithm. A simple domain-separation tweak would suffice:private static function get_sodium_key(): string { - return hash('sha256', wp_salt('auth'), true); + return hash('sha256', 'wu_sodium_' . wp_salt('auth'), true); }Alternatively, if the duplication is intentional to allow migrating between backends transparently, at minimum add a comment clarifying this.
wp_parse_str() + add_query_arg() decodes percent-encoded values like %2F without re-encoding them, causing parameters inside redirect_to to leak as top-level query params. This broke WooCommerce analytics URLs that use path=%2Fanalytics%2Foverview. In filter_login_url(), preserve the raw query string instead of parsing and rebuilding it. In Primary_Domain redirect, use the full redirect URL directly since replace_url() already handles the complete URL. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
Tests that SSO correctly triggers when visiting a subsite through a mapped domain (127.0.0.1 vs localhost), verifying domain mapping resolution, SSO redirect initiation, and auto-authentication flow. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
Verify each finding against the current code and only fix it if needed.
In `@tests/e2e/cypress/fixtures/setup-sso-test.php`:
- Around line 66-74: The associative arrays passed to $wpdb->update() should be
formatted with each key/value on its own line and you must check the return
value of $wpdb->update() to handle failures; update the call in the block that
runs when $existing_domain is truthy (where $table, $mapped_domain and
$existing_domain are used) so the second and third arguments (the data and where
arrays) are multi-line entries, then capture the return value (e.g. $result =
$wpdb->update(...)) and add error handling when $result === false (log the error
and abort/return early or throw) to avoid silently continuing on update failure.
- Around line 43-48: The multi-line call to wp_json_encode violates PHPCS:
ensure the opening parenthesis follows the function name on the same line and
the closing parenthesis is on its own line; specifically reformat the echo
wp_json_encode(...) call inside the is_wp_error($site_id) block so it reads echo
wp_json_encode( [ 'error' => $site_id->get_error_message(), ] ); with the
opening parenthesis directly after wp_json_encode and the closing parenthesis on
its own line; keep is_wp_error and get_error_message usage unchanged.
- Around line 57-64: The PHPCS warning is caused by interpolating {$table}
directly into the SQL string inside the $wpdb->prepare() call (used when
fetching $existing_domain); since table names cannot be parameterized, silence
the linter by adding an inline PHPCS ignore comment for the rule that flags
interpolated variables in SQL on the line with the $wpdb->prepare(...) call
referencing $table, keeping the rest of the prepared statement and placeholder
bindings intact.
- Around line 91-100: The wp_json_encode call with the array argument is using
inconsistent multi-line formatting; update the echo wp_json_encode([...])
invocation (the echo, wp_json_encode function and the array containing 'error'
=> 'Domain insert failed: ' . $wpdb->last_error and 'site_id' => $site_id) to
follow the project's multi-line call style (either make the entire call
single-line or put the array on its own indented lines with the closing
parenthesis on its own line), ensuring consistent commas and indentation and
then keep exit(1); and $domain_id = $wpdb->insert_id; as-is.
In `@tests/e2e/cypress/integration/060-sso-cross-domain.spec.js`:
- Around line 109-128: The test "Should preserve redirect_to parameter through
SSO flow" visits a specific targetPath (const targetPath =
"/wp-admin/options-general.php") but only asserts that the final URL includes
"/wp-admin/"; update the assertion in that it block to assert the full
targetPath (e.g., cy.url(...).should("include", targetPath)) and/or add a
stronger DOM check (e.g., verify a page-specific selector present on
options-general.php) using mappedDomainUrl and targetPath, or if the environment
cannot guarantee exact path preservation, change the test title and comment to
remove the `redirect_to` claim and document the limitation instead of asserting
exact path preservation.
- Around line 24-38: The before() hook currently runs
cy.wpCliFile(...).then(result) and uses expect() to assert no "error" in
result.stdout, which can lead to unclear failures; update the then callback (the
cy.wpCliFile call and its .then handler) to parse/inspect result.stdout (or
result) and if it contains an error key or any failure text throw a new Error
including the full stdout/stderr (e.g., throw new Error(`SSO setup failed:
${output}`)) so Cypress aborts the suite at setup instead of letting downstream
tests fail; keep the existing expect checks only for additional assertions but
ensure the explicit throw triggers an immediate, clear failure when cy.wpCliFile
indicates an error.
🧹 Nitpick comments (4)
🤖 Fix all nitpicks with AI agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/cypress/fixtures/setup-sso-test.php`: - Around line 43-48: The multi-line call to wp_json_encode violates PHPCS: ensure the opening parenthesis follows the function name on the same line and the closing parenthesis is on its own line; specifically reformat the echo wp_json_encode(...) call inside the is_wp_error($site_id) block so it reads echo wp_json_encode( [ 'error' => $site_id->get_error_message(), ] ); with the opening parenthesis directly after wp_json_encode and the closing parenthesis on its own line; keep is_wp_error and get_error_message usage unchanged. - Around line 57-64: The PHPCS warning is caused by interpolating {$table} directly into the SQL string inside the $wpdb->prepare() call (used when fetching $existing_domain); since table names cannot be parameterized, silence the linter by adding an inline PHPCS ignore comment for the rule that flags interpolated variables in SQL on the line with the $wpdb->prepare(...) call referencing $table, keeping the rest of the prepared statement and placeholder bindings intact. - Around line 91-100: The wp_json_encode call with the array argument is using inconsistent multi-line formatting; update the echo wp_json_encode([...]) invocation (the echo, wp_json_encode function and the array containing 'error' => 'Domain insert failed: ' . $wpdb->last_error and 'site_id' => $site_id) to follow the project's multi-line call style (either make the entire call single-line or put the array on its own indented lines with the closing parenthesis on its own line), ensuring consistent commas and indentation and then keep exit(1); and $domain_id = $wpdb->insert_id; as-is. In `@tests/e2e/cypress/integration/060-sso-cross-domain.spec.js`: - Around line 24-38: The before() hook currently runs cy.wpCliFile(...).then(result) and uses expect() to assert no "error" in result.stdout, which can lead to unclear failures; update the then callback (the cy.wpCliFile call and its .then handler) to parse/inspect result.stdout (or result) and if it contains an error key or any failure text throw a new Error including the full stdout/stderr (e.g., throw new Error(`SSO setup failed: ${output}`)) so Cypress aborts the suite at setup instead of letting downstream tests fail; keep the existing expect checks only for additional assertions but ensure the explicit throw triggers an immediate, clear failure when cy.wpCliFile indicates an error.tests/e2e/cypress/fixtures/setup-sso-test.php (3)
43-48: Fix multi-line function call formatting (PHPCS failures).Static analysis flags Lines 44 and 46: opening parenthesis must be the last content on its line, and closing parenthesis must be on its own line.
Proposed fix
if (is_wp_error($site_id)) { - echo wp_json_encode([ - 'error' => $site_id->get_error_message(), - ]); + echo wp_json_encode( + [ + 'error' => $site_id->get_error_message(), + ] + ); exit(1); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/cypress/fixtures/setup-sso-test.php` around lines 43 - 48, The multi-line call to wp_json_encode violates PHPCS: ensure the opening parenthesis follows the function name on the same line and the closing parenthesis is on its own line; specifically reformat the echo wp_json_encode(...) call inside the is_wp_error($site_id) block so it reads echo wp_json_encode( [ 'error' => $site_id->get_error_message(), ] ); with the opening parenthesis directly after wp_json_encode and the closing parenthesis on its own line; keep is_wp_error and get_error_message usage unchanged.
57-64: Interpolated table name in$wpdb->prepare()— acceptable here but can silence the linter.The PHPCS rule flags
{$table}inside the query string. Table names can't be bound as placeholders, so this is the standard WP pattern. You can silence the warning with an inline ignore if desired:Proposed fix
+// phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- table name cannot be a placeholder. $existing_domain = $wpdb->get_var( $wpdb->prepare( "SELECT id FROM {$table} WHERE domain IN (%s, %s) AND blog_id = %d LIMIT 1",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/cypress/fixtures/setup-sso-test.php` around lines 57 - 64, The PHPCS warning is caused by interpolating {$table} directly into the SQL string inside the $wpdb->prepare() call (used when fetching $existing_domain); since table names cannot be parameterized, silence the linter by adding an inline PHPCS ignore comment for the rule that flags interpolated variables in SQL on the line with the $wpdb->prepare(...) call referencing $table, keeping the rest of the prepared statement and placeholder bindings intact.
91-100: Same multi-line call formatting issue on Lines 93-94.Proposed fix
if (! $inserted) { - echo wp_json_encode([ - 'error' => 'Domain insert failed: ' . $wpdb->last_error, - 'site_id' => $site_id, - ]); + echo wp_json_encode( + [ + 'error' => 'Domain insert failed: ' . $wpdb->last_error, + 'site_id' => $site_id, + ] + ); exit(1); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/cypress/fixtures/setup-sso-test.php` around lines 91 - 100, The wp_json_encode call with the array argument is using inconsistent multi-line formatting; update the echo wp_json_encode([...]) invocation (the echo, wp_json_encode function and the array containing 'error' => 'Domain insert failed: ' . $wpdb->last_error and 'site_id' => $site_id) to follow the project's multi-line call style (either make the entire call single-line or put the array on its own indented lines with the closing parenthesis on its own line), ensuring consistent commas and indentation and then keep exit(1); and $domain_id = $wpdb->insert_id; as-is.tests/e2e/cypress/integration/060-sso-cross-domain.spec.js (1)
24-38: Setup failure doesn't abort the suite — downstream tests will fail with confusing errors.If the fixture returns an error, the
expectassertions will fail insidebefore(), but the messaging may be unclear. Consider throwing explicitly so Cypress marks the suite as failed at the setup stage:expect(output).to.contain("site_id"); expect(output).to.not.contain('"error"'); + + // Parse and expose for debugging if needed. + const data = JSON.parse(output); + if (data.error) { + throw new Error(`SSO setup failed: ${data.error}`); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/cypress/integration/060-sso-cross-domain.spec.js` around lines 24 - 38, The before() hook currently runs cy.wpCliFile(...).then(result) and uses expect() to assert no "error" in result.stdout, which can lead to unclear failures; update the then callback (the cy.wpCliFile call and its .then handler) to parse/inspect result.stdout (or result) and if it contains an error key or any failure text throw a new Error including the full stdout/stderr (e.g., throw new Error(`SSO setup failed: ${output}`)) so Cypress aborts the suite at setup instead of letting downstream tests fail; keep the existing expect checks only for additional assertions but ensure the explicit throw triggers an immediate, clear failure when cy.wpCliFile indicates an error.
| if ($existing_domain) { | ||
| // Update existing record to ensure domain includes port. | ||
| $wpdb->update( | ||
| $table, | ||
| ['domain' => $mapped_domain, 'active' => 1, 'stage' => 'done'], | ||
| ['id' => $existing_domain], | ||
| ['%s', '%d', '%s'], | ||
| ['%d'] | ||
| ); |
There was a problem hiding this comment.
Formatting: multi-item associative array values should each be on their own line.
Static analysis flags Line 70. Also, the $wpdb->update() return value is not checked — a failure here would silently proceed.
Proposed fix
if ($existing_domain) {
// Update existing record to ensure domain includes port.
- $wpdb->update(
+ $updated = $wpdb->update(
$table,
- ['domain' => $mapped_domain, 'active' => 1, 'stage' => 'done'],
+ [
+ 'domain' => $mapped_domain,
+ 'active' => 1,
+ 'stage' => 'done',
+ ],
['id' => $existing_domain],
['%s', '%d', '%s'],
['%d']
);
+
+ if ( false === $updated ) {
+ echo wp_json_encode([
+ 'error' => 'Domain update failed: ' . $wpdb->last_error,
+ 'site_id' => $site_id,
+ ]);
+ exit(1);
+ }
+
$domain_id = (int) $existing_domain;🧰 Tools
🪛 GitHub Check: Code Quality Checks
[failure] 70-70:
When a multi-item array uses associative keys, each value should start on a new line.
[warning] 68-68:
Direct database call without caching detected. Consider using wp_cache_get() / wp_cache_set() or wp_cache_delete().
[warning] 68-68:
Use of a direct database call is discouraged.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/e2e/cypress/fixtures/setup-sso-test.php` around lines 66 - 74, The
associative arrays passed to $wpdb->update() should be formatted with each
key/value on its own line and you must check the return value of $wpdb->update()
to handle failures; update the call in the block that runs when $existing_domain
is truthy (where $table, $mapped_domain and $existing_domain are used) so the
second and third arguments (the data and where arrays) are multi-line entries,
then capture the return value (e.g. $result = $wpdb->update(...)) and add error
handling when $result === false (log the error and abort/return early or throw)
to avoid silently continuing on update failure.
| it( | ||
| "Should preserve redirect_to parameter through SSO flow", | ||
| { retries: 1 }, | ||
| () => { | ||
| // This verifies that URL parameters survive the SSO redirect chain. | ||
| cy.loginByApi(adminUser, adminPass); | ||
|
|
||
| // Visit a specific wp-admin page on the mapped domain. | ||
| const targetPath = "/wp-admin/options-general.php"; | ||
|
|
||
| cy.visit(`${mappedDomainUrl}${targetPath}`, { | ||
| failOnStatusCode: false, | ||
| }); | ||
|
|
||
| // After SSO, the user should land on the requested page (or wp-admin). | ||
| cy.url({ timeout: 60000 }).should("include", "/wp-admin/"); | ||
| cy.get("body", { timeout: 30000 }).should("have.class", "wp-admin"); | ||
| cy.get("#wpadminbar").should("exist"); | ||
| } | ||
| ); |
There was a problem hiding this comment.
The redirect_to preservation test doesn't verify arrival at the target page.
The test visits options-general.php but only asserts /wp-admin/ in the final URL. This would pass even if redirect_to was silently dropped. Consider tightening:
// After SSO, the user should land on the requested page (or wp-admin).
- cy.url({ timeout: 60000 }).should("include", "/wp-admin/");
+ cy.url({ timeout: 60000 }).should("include", "options-general.php");If the wp-env SSO flow can't reliably preserve the exact path (as noted in the environment comments), then consider documenting that limitation explicitly in the test or removing the redirect_to claim from the test name.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/e2e/cypress/integration/060-sso-cross-domain.spec.js` around lines 109
- 128, The test "Should preserve redirect_to parameter through SSO flow" visits
a specific targetPath (const targetPath = "/wp-admin/options-general.php") but
only asserts that the final URL includes "/wp-admin/"; update the assertion in
that it block to assert the full targetPath (e.g., cy.url(...).should("include",
targetPath)) and/or add a stronger DOM check (e.g., verify a page-specific
selector present on options-general.php) using mappedDomainUrl and targetPath,
or if the environment cannot guarantee exact path preservation, change the test
title and comment to remove the `redirect_to` claim and document the limitation
instead of asserting exact path preservation.
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
…ng emails The inline login prompt on the register page had several issues: it would disappear when interacting with other form fields because check_user_exists shared global state across email/username fields, and the DOM-cloning pattern in setup_inline_login_handlers conflicted with browser autofill. Now only email changes can dismiss the email prompt, handlers attach once via a data attribute flag, the close button is removed, and a validation error is shown under the email field when the address already exists. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
- Fix undefined $hook_lines in generate-berlindb-hooks.php causing PHP warnings - Replace misused _n() calls with conditionals to resolve makepot warnings about missing singular placeholders and strings without translatable content - Add missing translators comment for Stripe webhook error string - Standardize translator comments across hosting provider integrations - Update POT file and readme.txt Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
The "every" keyword only appears when duration > 1 (e.g., "every 3 months"). With duration=1, the format is "$19.99 / month" and "month" respectively. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
1 participant
checkout.js hardcoded minStrength: 3 when creating the password checker, overriding the PHP-configured minimum strength setting. This meant the "strong" (score 4) and "super_strong" settings were effectively treated as "medium" (score 3), allowing weaker passwords through.
Removed the hardcoded override so the checker reads min_strength from wu_password_strength_settings (set by PHP). Also added re-initialization in the Vue updated() hook so the password checker activates when the field appears after initial mount in multi-step checkouts.
Summary by CodeRabbit
New Features
Security
Bug Fixes
Tests