feat(js,shared): add Prosopo Procaptcha as a CAPTCHA provider#8944
Open
forgetso wants to merge 5 commits into
Open
feat(js,shared): add Prosopo Procaptcha as a CAPTCHA provider#8944forgetso wants to merge 5 commits into
forgetso wants to merge 5 commits into
Conversation
Adds 'prosopo' alongside 'turnstile' in CaptchaProvider and a new getProcaptchaToken that mirrors the invisible + smart flows from turnstile.ts. When displayConfig.captchaProvider is 'prosopo', clerk-js loads the Procaptcha bundle from js.prosopo.io and renders into the existing invisible/smart containers. Turnstile remains the default. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@forgetso is attempting to deploy a commit to the Clerk Production Team on Vercel. A member of the Team first needs to authorize it. |
🦋 Changeset detectedLatest commit: 3629098 The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
📝 WalkthroughWalkthroughAdds Prosopo Procaptcha as an alternative CAPTCHA provider. A shared ChangesProsopo CAPTCHA Provider Integration
Sequence Diagram(s)sequenceDiagram
participant App as CaptchaChallenge
participant GCT as getCaptchaToken
participant Resolver as resolveCaptchaContainer
participant Prosopo as getProcaptchaToken
participant Turnstile as getTurnstileToken
App->>GCT: opts (captchaProvider from displayConfig)
alt captchaProvider === 'prosopo'
GCT->>Prosopo: opts
Prosopo->>Resolver: opts
Resolver-->>Prosopo: containerType, selector, siteKey, attrs
Prosopo-->>GCT: { captchaToken, captchaWidgetType }
else captchaProvider === 'turnstile' (default)
GCT->>Turnstile: opts
Turnstile->>Resolver: opts
Resolver-->>Turnstile: containerType, selector, siteKey, attrs
Turnstile-->>GCT: { captchaToken, captchaWidgetType }
end
GCT-->>App: { captchaToken, captchaWidgetType }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Pull the modal/smart/invisible container decision out of turnstile.ts and prosopo.ts into a single resolveCaptchaContainer helper, plus a matching cleanupCaptchaContainer for the side effects (modal close, invisible div removal). Each provider keeps its own visual styling (Turnstile's maxHeight/min-height dance, Prosopo's reserved height) and its own render() call. Also: - Point the Procaptcha load-failure warning at js.prosopo.io + Prosopo CSP docs rather than the Cloudflare-flavored guidance. - Add router test for getCaptchaToken (turnstile vs prosopo dispatch). - Add CaptchaChallenge test that .invisible() and .managedOrInvisible() both honour displayConfig.captchaProvider. - Expand prosopo.test.ts with smart, modal, modal-not-ready, error- callback, expired-callback, and script-load-failure cases. Validates the warning now points at Prosopo's CSP docs. 15 captcha tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The Procaptcha runtime calls error-callback with an Error built from the underlying challenge failure (procaptcha/modules/Manager.ts:193 and procaptcha-frictionless/ProcaptchaFrictionless.tsx:222,240 both call events.onError(new Error(message)), which defaultCallbacks.ts:129 forwards to the user-supplied error-callback). Earlier draft of this file assumed no argument was passed and rejected with a hardcoded 'procaptcha_error'. Type the callback the way the runtime actually behaves and surface error.message, with the previous identifier kept as a defensive fallback. New test asserts both branches. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/clerk-js/src/utils/captcha/containerResolver.ts`:
- Around line 91-97: The invisible CAPTCHA container resolver creates containers
with a shared class name selector that causes interference when multiple CAPTCHA
instances run concurrently. Instead of using the shared
CAPTCHA_INVISIBLE_CLASSNAME class selector, generate a unique identifier per
instance (such as a data attribute with a generated ID) for the created div
element, then return that unique selector in the containerSelector field. Update
the callers in turnstile.ts and prosopo.ts to pass the resolved unique container
selector into their cleanup functions instead of relying on the shared class
query.
- Around line 63-64: The waitForElement call for modalContainerQuerySelector has
no timeout and can hang indefinitely if the modal container never appears. Wrap
the waitForElement promise with a timeout mechanism that rejects after a
reasonable duration if the element is not found, and throw a captchaError with
an appropriate error message when the timeout is exceeded. This ensures the auth
flow fails fast rather than hanging.
🪄 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: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: f69936f1-602b-4123-a553-15e3b7761394
📒 Files selected for processing (10)
.changeset/prosopo-captcha-provider.mdpackages/clerk-js/src/utils/captcha/CaptchaChallenge.tspackages/clerk-js/src/utils/captcha/__tests__/CaptchaChallenge.test.tspackages/clerk-js/src/utils/captcha/__tests__/getCaptchaToken.test.tspackages/clerk-js/src/utils/captcha/__tests__/prosopo.test.tspackages/clerk-js/src/utils/captcha/containerResolver.tspackages/clerk-js/src/utils/captcha/getCaptchaToken.tspackages/clerk-js/src/utils/captcha/prosopo.tspackages/clerk-js/src/utils/captcha/turnstile.tspackages/shared/src/types/displayConfig.ts
- waitForElement on the modal container is unbounded, so a missing modal
container could hang the auth flow indefinitely. Race against a 5s
timeout and throw { captchaError: 'modal_container_not_found' } on
expiry.
- The invisible flow used a shared .clerk-invisible-captcha class
selector for both render and cleanup. Two concurrent challenges would
render into the same node and the second cleanup could remove the
first's container. Mint a per-instance id ('<class>-<ts>-<n>'),
return that id as the selector, and pass it into cleanup so each
challenge only removes its own node.
- New containerResolver.test.ts covers the timeout path and asserts
concurrent invisible challenges resolve to distinct containers and
clean up independently. Existing turnstile.test.ts updated to match
the new id-based selector format (the element still carries the
class).
Both issues were pre-existing in turnstile.ts and inherited by the
refactor; flagged by CodeRabbit on PR clerk#8944.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/clerk-js/src/utils/captcha/containerResolver.ts`:
- Around line 129-133: The exported function cleanupCaptchaContainer is missing
an explicit return type annotation. Add `: void` to the function signature after
the closing parenthesis of the parameter list to comply with the coding
guideline requiring explicit return types for exported functions.
- Around line 71-79: The MutationObserver created inside the waitForElement
function is not being cleaned up when the Promise.race timeout branch resolves
first, causing the observer to remain connected to document.body indefinitely.
Fix this by either modifying the waitForElement function to accept an abort
signal that it uses to clean up the observer when signaled, then create an
AbortController in the calling code and trigger it after the Promise.race
resolves with a timeout result, or alternatively add explicit cleanup logic in
the calling code after the race resolves to properly stop and disconnect the
observer that was created inside waitForElement. The key is ensuring the
MutationObserver is disconnected regardless of whether the race completes due to
element found or timeout.
🪄 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: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 42e2fd3e-0ea0-4c52-9117-79f7964a9322
📒 Files selected for processing (5)
packages/clerk-js/src/utils/captcha/__tests__/containerResolver.test.tspackages/clerk-js/src/utils/captcha/__tests__/turnstile.test.tspackages/clerk-js/src/utils/captcha/containerResolver.tspackages/clerk-js/src/utils/captcha/prosopo.tspackages/clerk-js/src/utils/captcha/turnstile.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/clerk-js/src/utils/captcha/turnstile.ts
- packages/clerk-js/src/utils/captcha/prosopo.ts
Comment on lines
+71
to
+79
| // waitForElement never rejects, so race it against a timeout to keep the auth flow from hanging. | ||
| const el = await Promise.race<Element | null>([ | ||
| waitForElement(modalContainerQuerySelector), | ||
| new Promise<null>(resolve => setTimeout(() => resolve(null), MODAL_CONTAINER_TIMEOUT_MS)), | ||
| ]); | ||
| if (!el) { | ||
| // eslint-disable-next-line @typescript-eslint/only-throw-error | ||
| throw { captchaError: 'modal_container_not_found' }; | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect waitForElement implementation for observer cleanup / abort support
rg -nP --type=ts -C8 'waitForElement' packages/shared/srcRepository: clerk/javascript
Length of output: 1219
🏁 Script executed:
cat -n packages/shared/src/dom/waitForElement.tsRepository: clerk/javascript
Length of output: 896
🏁 Script executed:
cat -n packages/clerk-js/src/utils/captcha/containerResolver.ts | sed -n '65,85p'Repository: clerk/javascript
Length of output: 1325
Fix waitForElement observer cleanup on timeout.
The waitForElement promise remains unsettled when the timeout branch wins in Promise.race, leaving the MutationObserver connected to document.body indefinitely. Each failed modal mount leaks an observer that continues to fire on DOM mutations for the rest of the page session.
Either add abort signal/timeout support to waitForElement so it cleans up on timeout, or explicitly clean up the observer in the calling code after the race resolves.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/clerk-js/src/utils/captcha/containerResolver.ts` around lines 71 -
79, The MutationObserver created inside the waitForElement function is not being
cleaned up when the Promise.race timeout branch resolves first, causing the
observer to remain connected to document.body indefinitely. Fix this by either
modifying the waitForElement function to accept an abort signal that it uses to
clean up the observer when signaled, then create an AbortController in the
calling code and trigger it after the Promise.race resolves with a timeout
result, or alternatively add explicit cleanup logic in the calling code after
the race resolves to properly stop and disconnect the observer that was created
inside waitForElement. The key is ensuring the MutationObserver is disconnected
regardless of whether the race completes due to element found or timeout.
Comment on lines
+129
to
+133
| export const cleanupCaptchaContainer = ( | ||
| containerType: CaptchaContainerType, | ||
| opts: Pick<CaptchaOptions, 'closeModal'>, | ||
| containerSelector?: string, | ||
| ) => { |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add an explicit : void return type.
cleanupCaptchaContainer is an exported function but omits its return type.
♻️ Proposed change
export const cleanupCaptchaContainer = (
containerType: CaptchaContainerType,
opts: Pick<CaptchaOptions, 'closeModal'>,
containerSelector?: string,
-) => {
+): void => {As per coding guidelines: "Always define explicit return types for functions, especially public APIs", and based on learnings to enforce explicit return type annotations for exported functions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/clerk-js/src/utils/captcha/containerResolver.ts` around lines 129 -
133, The exported function cleanupCaptchaContainer is missing an explicit return
type annotation. Add `: void` to the function signature after the closing
parenthesis of the parameter list to comply with the coding guideline requiring
explicit return types for exported functions.
Sources: Coding guidelines, Learnings
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hey @tmilewski @alexcarpenter. This PR adds Prosopo as an opt-in alternative to Turnstile, behind
displayConfig.captchaProvider.Why
Prosopo is has EU data residency and advanced bot detection - harder to bypass than default Cloudflare Turnstile.
What
CaptchaProviderwidened to'turnstile' | 'prosopo'(default unchanged).getProcaptchaToken;getCaptchaTokendispatches by provider.resolveCaptchaContainerhelper used by both providers.CaptchaChallenge.invisible()now readscaptchaProviderfromdisplayConfiginstead of hardcoding'turnstile'.Tests
16 unit tests, mocked
window.procaptcha(same pattern as the existing Turnstile suite — neither hits a live CDN).Backend dependency
Needs
display_config.captcha_provider: "prosopo"from FAPI to actually fire. Frontend half lands first; happy to coordinate on the backend side.Please let me know your thoughts.
Summary by CodeRabbit
displayConfig.captchaProviderset to'prosopo'or'turnstile'(default). Supports invisible and smart widget types with seamless rendering into existing CAPTCHA containers. Turnstile remains the default; no changes required for existing implementations.