Migrate to Prosopo Procaptcha #1
Closed
Conversation
This file contains 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
* working frontend temp * temp * package json fix * npm version bump * Drip request type changing to procaptcha * Drip request type changing to procaptcha * swapping all occurrences of recaptcha with procaptcha and adding captcha logic * Renaming recaptcha files and updating imports * adding Prosopo outputs to tests * bugtesting * Removing bundle and adding back regular tests * changing frontend to use remote bundle * Add CAPTCHA_PROVIDER config item * add feature flag for captcha provider and update tests for procaptcha * Add react deps * Add test configs for both captcha providers * lint:fix and yarn install * add Paseo testnet config * remove wococo testnet config * Adding forgotten paseo deployment * Address PR comments * Fix dummy body * Add debug * Bump the npm_and_yarn group across 1 directories with 1 update (paritytech#368) * Create e2e test environment for procaptcha * remove rogue file * Update zombienet config in workflow * yarn.lock * yarn.lock * downgrade jest * try fix for buildcheck * Pin polkadot versions * yarn.lock with 1.22.21 * try new yarn.lock * Update faucet lockfile * Procaptcha dev (#2) * dev setup wip * update docker setup for procaptcha * Remove extra unecessary stuff * Undo formatting * Update mock provider URL to use prosopo subdomain * bump prosopo versions * add missing dep * Update to working package versions * Upgrade prosopo deps * Add missing body parameter --------- Co-authored-by: Hugh <[email protected]> Co-authored-by: hugh <[email protected]> Co-authored-by: Pierre Besson <[email protected]> Co-authored-by: Yuri Volkov <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
WalkthroughThis set of changes introduces support for a decentralized captcha system by Prosopo, alongside maintaining support for Google's reCAPTCHA. It involves updating configurations, renaming variables to generalize captcha handling, adding support for Prosopo in both client and server components, and updating tests and documentation to reflect these changes. The transition aims to explore the feasibility of replacing Google's captcha with Prosopo's decentralized solution. Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to path filters (6)
client/package.jsonis excluded by:!**/*.jsonclient/tsconfig.jsonis excluded by:!**/*.jsonclient/yarn.lockis excluded by:!**/*.locke2e/zombienet.native.tomlis excluded by:!**/*.tomlenv.faucet.config.jsonis excluded by:!**/*.jsonpackage.jsonis excluded by:!**/*.json
Files selected for processing (29)
- .env.example (1 hunks)
- .eslintignore (1 hunks)
- .github/workflows/E2E.yml (1 hunks)
- README.md (3 hunks)
- client/.env (1 hunks)
- client/README.md (1 hunks)
- client/playwright.config.procaptcha.ts (1 hunks)
- client/playwright.config.recaptcha.ts (1 hunks)
- client/src/app.d.ts (2 hunks)
- client/src/lib/components/CaptchaV2.svelte (4 hunks)
- client/src/lib/components/Form.svelte (2 hunks)
- client/src/lib/utils/captcha.ts (1 hunks)
- client/src/lib/utils/faucetRequest.ts (1 hunks)
- client/tests/faucet.ts (14 hunks)
- e2e/README.md (3 hunks)
- src/bot/index.ts (1 hunks)
- src/config.ts (1 hunks)
- src/dripper/DripRequestHandler.spec.ts (5 hunks)
- src/dripper/DripRequestHandler.ts (3 hunks)
- src/dripper/Procaptcha.spec.ts (1 hunks)
- src/dripper/Procaptcha.ts (1 hunks)
- src/dripper/polkadot/PolkadotActions.ts (3 hunks)
- src/faucet.e2e.ts (8 hunks)
- src/server/routes/actions.spec.ts (3 hunks)
- src/server/routes/actions.ts (3 hunks)
- src/test/setupE2E.ts (6 hunks)
- src/test/setupE2EProcaptcha.ts (1 hunks)
- src/test/webhookHelpers.ts (1 hunks)
- src/types.ts (2 hunks)
Files skipped from review due to trivial changes (3)
- .eslintignore
- client/src/lib/utils/captcha.ts
- src/config.ts
Additional comments: 51
client/playwright.config.recaptcha.ts (1)
- 8-9: The introduction of
PUBLIC_CAPTCHA_PROVIDERset to "recaptcha" and the renaming ofPUBLIC_CAPTCHA_KEYtoPUBLIC_RECAPTCHA_KEYseem to contradict the PR's objective of migrating from Google's reCAPTCHA to Prosopo Procaptcha. Please verify if these changes are intended or if there's a misunderstanding regarding the captcha provider to be used.client/playwright.config.procaptcha.ts (1)
- 8-11: The configuration changes to support Prosopo Procaptcha, including the introduction of
PUBLIC_CAPTCHA_PROVIDERset to "procaptcha" and the addition ofPUBLIC_PROSOPO_SITE_KEY, align well with the PR's objectives of migrating to a decentralized captcha solution. These changes are correctly implemented for the intended migration.client/.env (1)
- 2-3: The changes in the
.envfile, including the renaming ofPUBLIC_CAPTCHA_KEYtoPUBLIC_RECAPTCHA_KEY, the addition ofPUBLIC_PROSOPO_SITE_KEY, and settingPUBLIC_CAPTCHA_PROVIDERtoprocaptcha, are consistent with the migration to Prosopo Procaptcha. Please confirm if the intention is to support both captcha systems simultaneously or to fully migrate to Prosopo, as bothPUBLIC_RECAPTCHA_KEYandPUBLIC_PROSOPO_SITE_KEYare present.Also applies to: 11-11
client/src/app.d.ts (1)
- 12-13: The changes to the captcha interfaces, including renaming
CaptchatoRecaptchaand adding a newProcaptchainterface, are well-implemented to support both reCAPTCHA and Prosopo Procaptcha. This approach aligns with the flexibility to potentially support multiple captcha providers. Please ensure that the rest of the client-side code correctly utilizes these interfaces based on the configured captcha provider.Also applies to: 19-44
client/src/lib/components/Form.svelte (2)
- 2-2: The import statement has been updated to include
PUBLIC_CAPTCHA_PROVIDER,PUBLIC_PROSOPO_SITE_KEY, andPUBLIC_RECAPTCHA_KEY. This change supports the integration of both Google reCAPTCHA and Prosopo Procaptcha by allowing the component to dynamically select the appropriate captcha key based on the environment configuration.- 62-69: The logic for dynamically selecting the captcha key and provider has been implemented correctly. It uses a ternary operator to choose between
PUBLIC_PROSOPO_SITE_KEYandPUBLIC_RECAPTCHA_KEYbased on the value ofPUBLIC_CAPTCHA_PROVIDER. This approach ensures that the component can support both captcha systems seamlessly. Additionally, theon:tokenevent handler is correctly set up to handle captcha token events.README.md (3)
- 54-54: The JSON payload for the
curlcommand has been updated to usecaptchaResponseinstead ofrecaptcha. This change aligns with the migration to a more flexible captcha handling system that can accommodate different providers, including Prosopo Procaptcha.- 79-79: The JSON payload in the React example has been updated to use
captchaResponseinstead ofrecaptcha. This change is consistent with the broader migration efforts and ensures that the documentation reflects the current implementation.- 92-111: The explanation text has been updated to include details about using a verified rococo address or a recaptcha token with a specific sitekey and secret for testing purposes. This addition provides valuable context for users and developers, helping them understand how to test the faucet functionality with both Prosopo Procaptcha and Google reCAPTCHA.
src/dripper/DripRequestHandler.ts (5)
- 4-4: The import statements have been updated to include
CaptchaProviderandProcaptcha, supporting the integration of the new captcha system. This change is essential for enabling theDripRequestHandlerto work with different captcha providers.- 24-27: The introduction of the
HandleRequestOptstype is a good practice as it unifies the options for handling requests, making the code more maintainable and easier to understand. This change supports the handling of both internal and external requests with different requirements.- 31-31: Replacing the
recaptchaproperty withcaptchaServicein theDripRequestHandlerconstructor is a positive change. It generalizes the captcha handling mechanism, allowing for the integration of different captcha services, such as Prosopo Procaptcha and Google reCAPTCHA.- 34-43: The updated
handleRequestmethod now correctly handles different types of requests based on theexternalflag and uses the appropriate captcha service for validation. This approach enhances the flexibility and scalability of the captcha validation process.- 73-76: The modification of the
getDripRequestHandlerInstancefunction to accept acaptchaProviderparameter and instantiate the appropriate captcha service based on the provider is a significant improvement. It ensures that the system can easily switch between different captcha providers, aligning with the project's goal of exploring decentralized captcha solutions.client/src/lib/components/CaptchaV2.svelte (5)
- 1-1: The script tag has been correctly updated to include
type="module", which is necessary for importing ES modules in Svelte components. This change supports the integration of the new captcha system.- 4-4: The import statement for
CaptchaProviderhas been added, enabling the component to use the constants defined for different captcha providers. This is a crucial step for supporting multiple captcha systems.- 7-7: The introduction of the
captchaProviderprop allows the component to dynamically select the captcha system based on the environment configuration. This flexibility is essential for supporting both Google reCAPTCHA and Prosopo Procaptcha.- 23-64: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [17-61]
The updated logic in the
window.captchaLoadedfunction correctly handles the rendering of captcha based on the selectedcaptchaProvider. This approach ensures that the component can seamlessly switch between different captcha systems. Additionally, the logic to force a new captcha on page reload forprocaptchais a thoughtful addition that enhances the user experience.
- 77-86: The conditional rendering of the captcha script source based on the
captchaProviderselected is implemented correctly. This ensures that the correct script is loaded for the selected captcha system, whether it's Google reCAPTCHA or Prosopo Procaptcha.e2e/README.md (3)
- 38-41: The instructions for downloading the
polkadot-sdkversionv1.5.0and related files have been updated correctly. This ensures that users are working with the latest versions of the required software for end-to-end testing.- 59-59: The Git checkout command for the
polkadotrepository has been updated to versionv1.5.0. This change ensures that users are using the correct version of the software for their testing environment.- 69-69: The Git checkout command for the
cumulusrepository has been updated to versionv1.5.0. This update aligns with the need to use consistent and up-to-date versions of the software for end-to-end testing.src/server/routes/actions.spec.ts (4)
- 79-79: The parameter name in the assertion has been correctly updated to "captchaResponse" to reflect the change in the request payload. This change aligns with the migration to a more flexible captcha handling system.
- 88-90: The request payload in the test case has been updated to use "captchaResponse" instead of "recaptcha". This change is consistent with the broader migration efforts and ensures that the test cases reflect the current implementation.
- 101-103: The request payload and assertion in the error reporting test case have been updated to use "captchaResponse". This ensures that the test cases are aligned with the updated parameter naming convention.
- 111-111: The request payload in the internal error test case has been updated to use "captchaResponse". This change maintains consistency across all test cases and aligns with the migration to a new captcha system.
src/dripper/DripRequestHandler.spec.ts (6)
- 5-6: The introduction of
ProcaptchaandRecaptchaimports supports the integration of the new captcha system. This change is essential for enabling the test cases to work with different captcha providers.- 8-8: The introduction of the
Captchatype, which encompasses bothProcaptchaandRecaptcha, is a good practice. It provides a unified type for handling different captcha services, enhancing the flexibility of the test setup.- 18-18: Renaming the variable to
captchaProviderand defining it as a type ofCaptchais a positive change. It generalizes the captcha handling mechanism, allowing for the integration of different captcha services in the test cases.- 28-28: The instantiation of
DripRequestHandlerwithcaptchaProvidercorrectly reflects the changes made to support multiple captcha systems. This ensures that the test setup aligns with the updated implementation of theDripRequestHandler.- 100-100: Updating the property name in the request object to
captchaResponsealigns with the broader migration to a more flexible captcha handling system. This change ensures that the test cases reflect the current implementation accurately.- 149-150: The test case for validating the captcha response has been updated to use
captchaResponse. This ensures that the test cases are aligned with the updated parameter naming convention and accurately test the captcha validation logic.src/dripper/polkadot/PolkadotActions.ts (2)
- 44-44: The replacement of
waitReadywithcryptoWaitReadyis a significant change. Ensure that all dependencies and usages of thecryptoWaitReadyfunction are correctly handled and that there are no side effects or issues with the initialization of the Polkadot API or keyring.- 99-101: The update to the logging message in the
teleportTokensmethod to include more detailed information about the transaction is a good improvement for clarity and debugging purposes. However, ensure that logging sensitive information is avoided to maintain security.src/test/setupE2EProcaptcha.ts (5)
- 12-36: The
setupProcaptchafunction correctly sets up the Prosopo Procaptcha contract, registers a provider, sets a dataset, and registers an app site key. Ensure that the contract deployment and registration processes are thoroughly tested to prevent any issues during runtime.- 39-59: The
deployProcaptchaContractfunction deploys the Prosopo Captcha contract. Ensure that the contract deployment is successful and that the contract address is correctly returned. It's also important to verify that the contract ABI and wasm bytecode are correctly loaded and used.- 63-79: The
procaptchaProviderRegisterfunction registers a Procaptcha provider. Ensure that the provider registration arguments are correctly formatted and that the registration transaction is successful. Pay attention to the hardcoded value for the provider's URL and the minimum value required for a captcha provider to be active.- 82-101: The
procaptchaProviderSetDatasetfunction sets the dataset for the Procaptcha provider. Ensure that the dataset IDs are correctly hashed and that the dataset setting transaction is successful. It's crucial to verify that the dataset IDs match the expected values.- 104-118: The
procaptchaAppRegisterfunction registers an app with the Prosopo Captcha contract. Ensure that the app registration is successful and that the site key and payee are correctly handled. It's important to verify that the minimum value for an app to be active is correctly set.src/bot/index.ts (1)
- 16-18: The addition of a check for a valid captcha provider before initializing the
dripRequestHandlerinstance is a good practice to ensure that the bot is configured with a supported captcha provider. This prevents runtime errors due to invalid configuration.src/test/setupE2E.ts (2)
- 90-137: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [70-118]
The modifications to the
setupfunction to include the setup for the Prosopo Procaptcha are comprehensive, covering the deployment of the contract, registration of providers and apps, and setting of datasets. Ensure that the setup process is correctly implemented and that all necessary parameters are correctly passed and handled.
- 134-134: The addition of the
procaptchaContainerto the teardown function ensures that the Procaptcha mock provider container is correctly stopped after the end-to-end tests. This is important for cleaning up resources and avoiding potential conflicts in subsequent test runs.src/faucet.e2e.ts (5)
- 7-7: The import of
RandomProviderfrom@prosopo/captcha-contractintroduces a dependency on the Prosopo captcha system. Ensure that the version of@prosopo/captcha-contractused is compatible with the rest of the project's dependencies and that it has been added to the project'spackage.json.- 31-32: The declaration of
procaptchaDetailswithout immediate initialization suggests it will be set up later, likely in a setup function. Ensure thatprocaptchaDetailsis properly initialized before use in tests to avoid runtime errors.- 66-72: The
beforeAllsetup now includes initialization forrococoContractsParachainApiandPROSOPO_SITE_KEY, which is crucial for connecting to the Prosopo captcha service. Ensure that thesetupfunction correctly initializes these components and handles any potential errors during the setup process to prevent flaky tests.- 89-89: Disconnecting from
rococoContractsParachainApiin theafterAllcleanup function is good practice to release resources. Verify that all connections established during tests are properly closed to avoid resource leaks.- 210-214: The use of
procaptchaGetRandomProviderto obtain aRandomProviderinstance for captcha verification introduces a dependency on external services. Ensure that the network and contract details used here are appropriate for the test environment and that any network calls are mocked or handled in a way that does not introduce flakiness to the tests.client/tests/faucet.ts (4)
- 5-5: The update to the
FormSubmittype to usecaptchaResponseinstead ofrecaptchareflects the migration to a more generic captcha handling approach. Ensure that all references to the oldrecaptchafield are updated accordingly in the codebase to prevent type errors.- 9-10: The introduction of the
CaptchaProvidertype is a good practice for maintaining type safety and clarity in the code. It helps ensure that only supported captcha providers are used throughout the tests.- 105-119: The
getCaptchaProvidermethod is a clean way to fetch the captcha provider from the configuration. Ensure that the configuration is correctly set up in the test environment to provide the expected captcha provider value. Additionally, consider handling the case where the configuration might return an unexpected value more gracefully.- 131-138: The tests now correctly pass the captcha provider configuration to
getFormElements. This is crucial for ensuring that the tests interact with the correct captcha system. Verify that the captcha provider is correctly determined in all test environments to prevent test failures due to configuration issues.
|
|
||
| const randomAddress = () => createTestKeyring().addFromSeed(randomAsU8a(32)).address; | ||
| const sha256 = (x: string) => crypto.createHash("sha256").update(x, "utf8").digest("hex"); | ||
|
|
||
| describe("Faucet E2E", () => { | ||
| const PARACHAIN_ID = 1000; // From the zombienet config. | ||
| const PROSOPO_SITE_KEY = "5C4hrfjw9DjXZTzV3MwzrrAr9P1MJhSrvWGWqi1eSuyUpnhM"; // procaptcha demo account (zero address) |
There was a problem hiding this comment.
The PROSOPO_SITE_KEY constant is hard-coded. For production or broader testing environments, consider fetching such sensitive information from environment variables or a secure configuration management system to avoid exposing keys in the source code.
- const PROSOPO_SITE_KEY = "5C4hrfjw9DjXZTzV3MwzrrAr9P1MJhSrvWGWqi1eSuyUpnhM";
+ const PROSOPO_SITE_KEY = process.env.PROSOPO_SITE_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.
Suggested change
| const PROSOPO_SITE_KEY = "5C4hrfjw9DjXZTzV3MwzrrAr9P1MJhSrvWGWqi1eSuyUpnhM"; // procaptcha demo account (zero address) | |
| const PROSOPO_SITE_KEY = process.env.PROSOPO_SITE_KEY; |
Comment on lines
+11
to
+53
| const getFormElements = async (page: Page, captchaProvider: "recaptcha" | "procaptcha", getCaptcha = false) => { | ||
| // eslint-disable-next-line @typescript-eslint/consistent-type-assertions | ||
| let captcha: Locator = {} as Locator; | ||
| if (getCaptcha) { | ||
| // ?: Hack. We need to wait for the frame to load and then invade it. | ||
| await page.reload(); | ||
| const captchaFrame = await new Promise<Frame>((resolve, reject) => { | ||
| let i = 0; | ||
| // function that waits for the frame and timeouts after 3 seconds | ||
| // FIXME consider "until" from "@eng-automation/js"? | ||
| // eslint-disable-next-line no-restricted-syntax | ||
| (function waitForFrame() { | ||
| const captchaFrames = page.frames().filter((f) => f.url().includes("https://www.google.com/recaptcha/api2/")); | ||
| if (captchaFrames.length > 0) { | ||
| return resolve(captchaFrames[0]); | ||
| } else { | ||
| i++; | ||
| if (i > 10) { | ||
| reject(new Error("Timeout")); | ||
| if (captchaProvider === "recaptcha") { | ||
| // ?: Hack. We need to wait for the frame to load and then invade it. | ||
| await page.reload(); | ||
| const captchaFrame = await new Promise<Frame>((resolve, reject) => { | ||
| let i = 0; | ||
| // function that waits for the frame and timeouts after 3 seconds | ||
| // FIXME consider "until" from "@eng-automation/js"? | ||
| // eslint-disable-next-line no-restricted-syntax | ||
| (function waitForFrame() { | ||
| const captchaFrames = page.frames().filter((f) => f.url().includes("https://www.google.com/recaptcha/api2/")); | ||
| if (captchaFrames.length > 0) { | ||
| return resolve(captchaFrames[0]); | ||
| } else { | ||
| i++; | ||
| if (i > 10) { | ||
| reject(new Error("Timeout")); | ||
| } | ||
| } | ||
| } | ||
| setTimeout(waitForFrame, 300); | ||
| })(); | ||
| }); | ||
| captcha = captchaFrame?.locator("#recaptcha-anchor") as Locator; | ||
| setTimeout(waitForFrame, 300); | ||
| })(); | ||
| }); | ||
| captcha = captchaFrame?.locator("#recaptcha-anchor") as Locator; | ||
| } else if (captchaProvider === "procaptcha") { | ||
| const testAccount = "5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY"; // Alice's address | ||
| const testSiteKey = "5C4hrfjw9DjXZTzV3MwzrrAr9P1MJhSrvWGWqi1eSuyUpnhM"; // Mock site key | ||
| const testProvider = "https://mockprovider.prosopo.io"; // Mock provider | ||
|
|
||
| // Tell the page that a captcha provider has previously been used and inject the mock provider | ||
| await page.evaluate((provider) => localStorage.setItem("@prosopo/provider", provider), testProvider); | ||
|
|
||
| // Mock the verify api call and inject Alice's address before clicking the captcha | ||
| await page.route("*/**/v1/prosopo/provider/verify", async (route) => { | ||
| const json = { user: testAccount, dapp: testSiteKey }; | ||
| await route.continue({ postData: json }); | ||
| }); | ||
| captcha = page.locator("#captcha_element input[type='checkbox']"); | ||
| } else { | ||
| throw new Error("Unknown captcha provider"); | ||
| } |
There was a problem hiding this comment.
The modifications to the getFormElements function to handle different captcha providers are comprehensive and include handling for both recaptcha and procaptcha. Ensure that the logic for each captcha provider is thoroughly tested, especially the new procaptcha logic, which includes mocking API calls and setting local storage items. Consider abstracting the captcha-specific logic into separate functions for better readability and maintainability.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes paritytech#337
PR to migrate rococo faucet from reCAPTCHA to Prosopo Procaptcha, which runs on the rococo test network.
More detail to follow...
Copy from paritytech#372
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
Chores