Skip to content

wip: scope inline login captcha token selectors to prompt container a…#901

Closed
superdav42 wants to merge 1 commit intomainfrom
feature/inline-login-captcha
Closed

wip: scope inline login captcha token selectors to prompt container a…#901
superdav42 wants to merge 1 commit intomainfrom
feature/inline-login-captcha

Conversation

@superdav42
Copy link
Copy Markdown
Collaborator

@superdav42 superdav42 commented Apr 22, 2026

…nd add reset on error

Summary by CodeRabbit

  • Bug Fixes
    • Improved captcha token collection during inline login to ensure tokens are correctly captured from the login prompt.
    • Enhanced error handling for inline login requests with better error reporting.
    • Added automatic captcha verification reset after login attempts to maintain security.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 22, 2026

📝 Walkthrough

Walkthrough

The checkout.js file was updated to improve inline login captcha handling: token selectors now scope to individual inline login prompt containers instead of the entire page, a captcha reset mechanism was added post-request, and AJAX error handling was improved to pass response JSON to error callbacks.

Changes

Cohort / File(s) Summary
Inline Login Captcha Management
assets/js/checkout.js
Updated captcha token collection to scope selectors to individual inline login prompt containers; added post-request captcha reset invocation (window.wuCaptchaResetInlineLogin()); improved AJAX error handling to pass response JSON (xhr.responseJSON || {}) to error callbacks; restructured error handler definitions and invocation patterns.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

A rabbit hops through login flows so neat,
Captcha tokens scoped—no more bittersweet!
Reset mechanisms guard the way,
Error handlers save the day! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title partially matches the changeset - it mentions scoping captcha token selectors to the prompt container (the primary change), but is truncated and omits the 'reset on error' aspect mentioned in full objectives.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/inline-login-captcha

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

🔨 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!
Playground support for multisite is very limitied, hopefully it will get better in the future.

🚀 Launch in Playground

Login credentials: admin / password

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
assets/js/checkout.js (1)

1436-1443: Duplicate wuCaptchaResetInlineLogin() call in the AJAX error branch.

handleError at lines 1361–1364 already invokes window.wuCaptchaResetInlineLogin(), so the reset block at 1440–1442 fires it a second time on every HTTP error. Drop the redundant block here and let handleError own the reset (keeps the success-but-{success:false} path consistent, which also goes through handleError at line 1432).

♻️ Proposed cleanup
 		error(xhr) {
 			handleError(xhr.responseJSON || {});
-
-			// Reset inline login captcha widgets so the user can re-verify.
-			if (typeof window.wuCaptchaResetInlineLogin === 'function') {
-				window.wuCaptchaResetInlineLogin();
-			}
 		}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@assets/js/checkout.js` around lines 1436 - 1443, The AJAX error branch
currently calls window.wuCaptchaResetInlineLogin() redundantly; handleError
already performs that reset (see handleError and its calls at lines around
1361–1364 and 1432). Remove the inline reset block inside the error(xhr) handler
(the conditional that checks typeof window.wuCaptchaResetInlineLogin ===
'function' and invokes it) so only handleError is responsible for resetting the
captcha; keep the error(xhr) → handleError(xhr.responseJSON || {}) call intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@assets/js/checkout.js`:
- Around line 1243-1273: The inline login success handler in handle_inline_login
currently only handles results.success === true and ignores server 200 responses
with success:false; update the success callback inside the
this.request('wu_inline_login', ...) call to treat a non-true results.success
the same as the error path: set that.login_error from results.data.message
(falling back to wu_checkout.i18n.login_failed), call
window.wuCaptchaResetInlineLogin() if available, and invoke the existing
handleError/results error-handling flow used by setup_inline_login_handlers (or
reuse its logic) so captcha tokens are reset and the user sees the error message
when the server returns {success:false}.
- Around line 1212-1273: The added captcha-collection block (creating
promptContainer, login_data, recaptcha_token/hcaptcha_token/cap_token and
attaching them to login_data) and the two other newly added helper functions
(handleError and the captcha portion inside handleLogin) are incorrectly
under-indented; re-indent those entire blocks to match their surrounding scope
(use the same number of leading tabs as the enclosing methods/blocks where
promptContainer is used and where handleError/handleLogin live), so the
declarations for promptContainer, login_data, recaptcha_token, hcaptcha_token,
cap_token and the wu_inline_login request callback align with the existing 5–7
tab indentation in that file and satisfy the project's tab-based ESLint rules.

---

Nitpick comments:
In `@assets/js/checkout.js`:
- Around line 1436-1443: The AJAX error branch currently calls
window.wuCaptchaResetInlineLogin() redundantly; handleError already performs
that reset (see handleError and its calls at lines around 1361–1364 and 1432).
Remove the inline reset block inside the error(xhr) handler (the conditional
that checks typeof window.wuCaptchaResetInlineLogin === 'function' and invokes
it) so only handleError is responsible for resetting the captcha; keep the
error(xhr) → handleError(xhr.responseJSON || {}) call intact.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5c459599-e6a6-4805-a980-311f121714cd

📥 Commits

Reviewing files that changed from the base of the PR and between 0ca23fb and 33c28a7.

📒 Files selected for processing (1)
  • assets/js/checkout.js

Comment thread assets/js/checkout.js
Comment on lines +1212 to +1273
// Include captcha tokens scoped to the inline login prompt container.
// Using scoped selectors avoids picking up the checkout form's captcha token.
const promptContainer = jQuery('#wu-inline-login-prompt-' + this.login_prompt_field);
const login_data = {
username_or_email,
password: this.inline_login_password,
_wpnonce: jQuery('[name="_wpnonce"]').val()
};

if (hcaptcha_token) {
login_data[ 'h-captcha-response' ] = hcaptcha_token;
}
const recaptcha_token = promptContainer.find('input[name="g-recaptcha-response"]').filter(function() {
return this.value;
}).first().val();
const hcaptcha_token = promptContainer.find('input[name="h-captcha-response"]').filter(function() {
return this.value;
}).first().val();
const cap_token = promptContainer.find('input[name="cap-token"]').filter(function() {
return this.value;
}).first().val();

if (cap_token) {
login_data[ 'cap-token' ] = cap_token;
}
if (recaptcha_token) {
login_data[ 'g-recaptcha-response' ] = recaptcha_token;
}

this.request('wu_inline_login', login_data, function(results) {
if (hcaptcha_token) {
login_data[ 'h-captcha-response' ] = hcaptcha_token;
}

that.logging_in = false;
if (cap_token) {
login_data[ 'cap-token' ] = cap_token;
}

if (results.success) {
this.request('wu_inline_login', login_data, function(results) {

// Login successful - reload page to show logged-in state
window.location.reload();
that.logging_in = false;

}
if (results.success) {

}, function(error) {
// Login successful - reload page to show logged-in state
window.location.reload();

that.logging_in = false;
}

if (error.responseJSON && error.responseJSON.data && error.responseJSON.data.message) {
}, function(error) {

that.login_error = error.responseJSON.data.message;
that.logging_in = false;

} else {
if (error.responseJSON && error.responseJSON.data && error.responseJSON.data.message) {

that.login_error = wu_checkout.i18n.login_failed || 'Login failed. Please try again.';
that.login_error = error.responseJSON.data.message;

}
} else {

});
that.login_error = wu_checkout.i18n.login_failed || 'Login failed. Please try again.';

}

// Reset inline login captcha widgets so the user can re-verify.
if (typeof window.wuCaptchaResetInlineLogin === 'function') {
window.wuCaptchaResetInlineLogin();
}

});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm the added lines are at single-tab indentation while neighbors are deeper,
# and check whether the repo's ESLint config enforces a specific indent depth.
fd -t f eslint -x echo "--- {} ---" \; -x cat {} \;
echo
echo "--- handle_inline_login indentation sample ---"
awk 'NR>=1205 && NR<=1275 { printf "%4d|", NR; for (i=1; i<=length($0); i++) { c=substr($0,i,1); if (c=="\t") printf "."; else { printf "%s", substr($0,i); break } } print "" }' assets/js/checkout.js
echo
echo "--- handleError / handleLogin sample ---"
awk 'NR>=1340 && NR<=1450 { printf "%4d|", NR; for (i=1; i<=length($0); i++) { c=substr($0,i,1); if (c=="\t") printf "."; else { printf "%s", substr($0,i); break } } print "" }' assets/js/checkout.js

Repository: Ultimate-Multisite/ultimate-multisite

Length of output: 5824


Fix indentation: all newly added code blocks are under-indented by 4–6 tabs.

Lines 1212–1273, 1346–1366, and 1390–1444 are indented with a single leading tab, while surrounding unchanged code at the same scope sits at 5–6 tabs (and 6–7 tabs inside the handleLogin function). For example:

  • Line 1210 (......: this.username || '';) sits at 6 tabs, yet the very next added line 1212 (./​/ Include captcha tokens...) drops to 1 tab.
  • Line 1340 (......function hideError() {) is at 6 tabs, but the added handleError function at line 1346 is at 1 tab.
  • Inside handleLogin (starting line 1368 at 7 tabs), the new captcha collection code at line 1390 starts at 1 tab.

This breaks nesting consistency and violates the project's WordPress ESLint tab-based indentation rule. Re-indent all three blocks to match their enclosing scope depth.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@assets/js/checkout.js` around lines 1212 - 1273, The added captcha-collection
block (creating promptContainer, login_data,
recaptcha_token/hcaptcha_token/cap_token and attaching them to login_data) and
the two other newly added helper functions (handleError and the captcha portion
inside handleLogin) are incorrectly under-indented; re-indent those entire
blocks to match their surrounding scope (use the same number of leading tabs as
the enclosing methods/blocks where promptContainer is used and where
handleError/handleLogin live), so the declarations for promptContainer,
login_data, recaptcha_token, hcaptcha_token, cap_token and the wu_inline_login
request callback align with the existing 5–7 tab indentation in that file and
satisfy the project's tab-based ESLint rules.

Comment thread assets/js/checkout.js
Comment on lines +1243 to +1273
this.request('wu_inline_login', login_data, function(results) {

// Login successful - reload page to show logged-in state
window.location.reload();
that.logging_in = false;

}
if (results.success) {

}, function(error) {
// Login successful - reload page to show logged-in state
window.location.reload();

that.logging_in = false;
}

if (error.responseJSON && error.responseJSON.data && error.responseJSON.data.message) {
}, function(error) {

that.login_error = error.responseJSON.data.message;
that.logging_in = false;

} else {
if (error.responseJSON && error.responseJSON.data && error.responseJSON.data.message) {

that.login_error = wu_checkout.i18n.login_failed || 'Login failed. Please try again.';
that.login_error = error.responseJSON.data.message;

}
} else {

});
that.login_error = wu_checkout.i18n.login_failed || 'Login failed. Please try again.';

}

// Reset inline login captcha widgets so the user can re-verify.
if (typeof window.wuCaptchaResetInlineLogin === 'function') {
window.wuCaptchaResetInlineLogin();
}

});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

handle_inline_login silently swallows {success:false} responses and skips the captcha reset.

The success callback only handles results.success === true (reload). When the server responds 200 with { success:false, data:{ message } } — e.g., wrong password, captcha rejected — none of the following happens:

  • this.login_error is not set, so the user sees no feedback.
  • window.wuCaptchaResetInlineLogin() is not called, so a one-time captcha token stays consumed and any retry will fail validation server-side.

Only network/HTTP errors hit the error callback where the reset now lives. The sibling setup_inline_login_handlers flow does route success:false through handleError(results) — these two paths should behave the same.

🐛 Proposed fix
 	this.request('wu_inline_login', login_data, function(results) {

 		that.logging_in = false;

 		if (results.success) {

 			// Login successful - reload page to show logged-in state
 			window.location.reload();
+			return;
+
+		}
+
+		if (results.data && results.data.message) {
+
+			that.login_error = results.data.message;
+
+		} else {
+
+			that.login_error = wu_checkout.i18n.login_failed || 'Login failed. Please try again.';

 		}

+		// Reset inline login captcha widgets so the user can re-verify.
+		if (typeof window.wuCaptchaResetInlineLogin === 'function') {
+			window.wuCaptchaResetInlineLogin();
+		}
+
 	}, function(error) {
📝 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.

Suggested change
this.request('wu_inline_login', login_data, function(results) {
// Login successful - reload page to show logged-in state
window.location.reload();
that.logging_in = false;
}
if (results.success) {
}, function(error) {
// Login successful - reload page to show logged-in state
window.location.reload();
that.logging_in = false;
}
if (error.responseJSON && error.responseJSON.data && error.responseJSON.data.message) {
}, function(error) {
that.login_error = error.responseJSON.data.message;
that.logging_in = false;
} else {
if (error.responseJSON && error.responseJSON.data && error.responseJSON.data.message) {
that.login_error = wu_checkout.i18n.login_failed || 'Login failed. Please try again.';
that.login_error = error.responseJSON.data.message;
}
} else {
});
that.login_error = wu_checkout.i18n.login_failed || 'Login failed. Please try again.';
}
// Reset inline login captcha widgets so the user can re-verify.
if (typeof window.wuCaptchaResetInlineLogin === 'function') {
window.wuCaptchaResetInlineLogin();
}
});
this.request('wu_inline_login', login_data, function(results) {
that.logging_in = false;
if (results.success) {
// Login successful - reload page to show logged-in state
window.location.reload();
return;
}
if (results.data && results.data.message) {
that.login_error = results.data.message;
} else {
that.login_error = wu_checkout.i18n.login_failed || 'Login failed. Please try again.';
}
// Reset inline login captcha widgets so the user can re-verify.
if (typeof window.wuCaptchaResetInlineLogin === 'function') {
window.wuCaptchaResetInlineLogin();
}
}, function(error) {
that.logging_in = false;
if (error.responseJSON && error.responseJSON.data && error.responseJSON.data.message) {
that.login_error = error.responseJSON.data.message;
} else {
that.login_error = wu_checkout.i18n.login_failed || 'Login failed. Please try again.';
}
// Reset inline login captcha widgets so the user can re-verify.
if (typeof window.wuCaptchaResetInlineLogin === 'function') {
window.wuCaptchaResetInlineLogin();
}
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@assets/js/checkout.js` around lines 1243 - 1273, The inline login success
handler in handle_inline_login currently only handles results.success === true
and ignores server 200 responses with success:false; update the success callback
inside the this.request('wu_inline_login', ...) call to treat a non-true
results.success the same as the error path: set that.login_error from
results.data.message (falling back to wu_checkout.i18n.login_failed), call
window.wuCaptchaResetInlineLogin() if available, and invoke the existing
handleError/results error-handling flow used by setup_inline_login_handlers (or
reuse its logic) so captcha tokens are reset and the user sees the error message
when the server returns {success:false}.

@github-actions
Copy link
Copy Markdown

Performance Test Results

Performance test results for b07665d are in 🛎️!

Note: the numbers in parentheses show the difference to the previous (baseline) test run. Differences below 2% or 0.5 in absolute values are not shown.

URL: /

Run DB Queries Memory Before Template Template WP Total LCP TTFB LCP - TTFB
0 41 37.83 MB 779.50 ms (-85.00 ms / -11% ) 160.50 ms 979.00 ms (-72.50 ms / -7% ) 1966.00 ms (-72.00 ms / -4% ) 1851.50 ms (-99.60 ms / -5% ) 80.40 ms (-2.90 ms / -4% )
1 56 49.03 MB 909.00 ms (-40.00 ms / -4% ) 137.00 ms (-3.50 ms / -3% ) 1045.00 ms (-45.50 ms / -4% ) 2036.00 ms (-56.00 ms / -3% ) 1957.10 ms (-57.80 ms / -3% ) 77.05 ms

@superdav42 superdav42 closed this Apr 22, 2026
superdav42 added a commit that referenced this pull request Apr 22, 2026
…ks (#909)

The inline-login flow in checkout.js previously read and reset reCAPTCHA /
hCaptcha / Cap token inputs directly, coupling the core plugin to specific
captcha providers. Replace that with generic wp.hooks extension points so
any addon can participate in the lifecycle:

* wu_inline_login_data (filter) — augment the AJAX request payload
  (e.g. append captcha tokens)
* wu_inline_login_success (action) — react to a successful login
* wu_inline_login_error (action) — react to a failed login
  (e.g. reset a captcha widget)
* wu_inline_login_prompt_ready (action) — initialize widgets once
  the prompt container is live in the DOM

The captcha addon now hooks these instead of relying on hardcoded
selectors and a window.wuCaptchaResetInlineLogin global.

Supersedes #901.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
superdav42 added a commit that referenced this pull request Apr 22, 2026
* refactor(checkout): replace captcha-specific code with generic JS hooks

The inline-login flow in checkout.js previously read and reset reCAPTCHA /
hCaptcha / Cap token inputs directly, coupling the core plugin to specific
captcha providers. Replace that with generic wp.hooks extension points so
any addon can participate in the lifecycle:

* wu_inline_login_data (filter) — augment the AJAX request payload
  (e.g. append captcha tokens)
* wu_inline_login_success (action) — react to a successful login
* wu_inline_login_error (action) — react to a failed login
  (e.g. reset a captcha widget)
* wu_inline_login_prompt_ready (action) — initialize widgets once
  the prompt container is live in the DOM

The captcha addon now hooks these instead of relying on hardcoded
selectors and a window.wuCaptchaResetInlineLogin global.

Supersedes #901.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(checkout): add async wu_before_inline_login_submitted hook

Adds a Promise-collecting pre-submission hook so addons can perform
async work (like solving an invisible captcha) before the inline
login AJAX request is built.

Matches the existing `wu_before_form_submitted` pattern — addons push
promises into the filter array; core awaits `Promise.all(...)` before
proceeding. If any promise rejects, the error message is surfaced to
the user and the request is aborted.

This fixes a timing race where invisible reCAPTCHA / hCaptcha / Cap
widgets had not yet populated their token input by the time the user
clicked Submit on the inline login prompt.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant