Feat | Migrate from Google reCAPTCHA to Cloudflare Turnstile#121
Open
matiasperrone-exo wants to merge 2 commits intomainfrom
Open
Feat | Migrate from Google reCAPTCHA to Cloudflare Turnstile#121matiasperrone-exo wants to merge 2 commits intomainfrom
matiasperrone-exo wants to merge 2 commits intomainfrom
Conversation
|
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:
📝 WalkthroughWalkthroughReplaces Google reCAPTCHA with Cloudflare Turnstile across backend validators, frontend React components, SCSS selectors, Blade templates, environment/config and dependencies; removes legacy jQuery auth scripts; adds Turnstile service config and a Webpack rule for ESM resolution. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant Turnstile
participant Backend
participant Config
User->>Browser: Open auth form
Browser->>Config: Read services.turnstile.key
Browser->>Turnstile: Render widget (siteKey)
User->>Turnstile: Complete challenge
Turnstile-->>Browser: Return token (cf-turnstile-response)
Browser->>Backend: Submit form + cf-turnstile-response
Backend->>Config: Read TURNSTILE_SECRET_KEY
Backend->>Turnstile: Verify token with secret
Turnstile-->>Backend: Verification result
Backend-->>Browser: Accept or return validation errors
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ 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 |
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-121/ This page is automatically updated on each push to this PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
webpack.common.js (1)
86-91: Scope thefullySpecified: falseworkaround more narrowly.The rule currently applies to all JS modules globally (
/\.m?js/without anchoring or scoping). While it correctly fixes the Turnstile ESM import issue, limiting the workaround tonode_modulesand anchoring the regex prevents accidentally masking import-resolution issues in your own source code. This follows webpack convention for third-party package workarounds.♻️ Suggested change
{ // Fix for webpack 5 + ESM packages (e.g. `@marsidev/react-turnstile`) that import // react/jsx-runtime without the .js extension, which webpack 5 requires in strict ESM mode. - test: /\.m?js/, + test: /\.m?js$/, + include: /node_modules/, resolve: { fullySpecified: false } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@webpack.common.js` around lines 86 - 91, The current webpack rule with test: /\.m?js/ and resolve: { fullySpecified: false } is too broad and affects your source files; narrow it by anchoring the regex (use /\.m?js$/) and scoping the rule to third-party packages (add an include or test against /node_modules/) so the fullySpecified workaround only applies to node_modules ESM packages (e.g., `@marsidev/react-turnstile`) and does not mask import-resolution issues in your own code.resources/js/signup/signup.js (1)
278-283: Make the Turnstile field name explicit.The backend validators now hardcode
cf-turnstile-response, but this widget is relying on the wrapper default.@marsidev/react-turnstileexposesresponseFieldNamevia its render options, and Cloudflare uses the same default field name, so spelling it out here would make the frontend/backend contract less brittle. (docs.page)♻️ Suggested change
<Turnstile ref={captcha} className={styles.turnstile} siteKey={captchaPublicKey} + options={{ responseFieldName: "cf-turnstile-response" }} onSuccess={onChangeRecaptcha} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/signup/signup.js` around lines 278 - 283, The Turnstile component usage relies on the wrapper default for the form field name while backend validators expect "cf-turnstile-response"; update the Turnstile element (the JSX using Turnstile with ref={captcha}, siteKey={captchaPublicKey}, onSuccess={onChangeRecaptcha}) to set the responseFieldName prop explicitly to "cf-turnstile-response" so the frontend and backend contract is explicit and not brittle.resources/js/reset_password/reset_password.js (1)
175-180: Make the Turnstile field name explicit.The backend validators now hardcode
cf-turnstile-response, but this widget is relying on the wrapper default.@marsidev/react-turnstileexposesresponseFieldNamevia its render options, and Cloudflare uses the same default field name, so spelling it out here would make the frontend/backend contract less brittle. (docs.page)♻️ Suggested change
<Turnstile ref={captcha} className={styles.turnstile} siteKey={captchaPublicKey} + options={{ responseFieldName: "cf-turnstile-response" }} onSuccess={onChangeRecaptcha} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/reset_password/reset_password.js` around lines 175 - 180, The Turnstile component usage should explicitly set the request field name to match backend validators: add the render prop/option responseFieldName with the string "cf-turnstile-response" to the Turnstile component (the element using ref={captcha}, siteKey={captchaPublicKey}, onSuccess={onChangeRecaptcha}, className={styles.turnstile}) so the frontend submits the expected cf-turnstile-response field..env.example (1)
61-62: Document local/test Turnstile keys here.Leaving both vars blank means a copied
.envcannot exercise the new auth flows until real Cloudflare credentials are provisioned. Cloudflare publishes official dummy site/secret pairs that work on localhost and automated test suites, so adding them here as commented examples would make local/CI bring-up much easier. (developers.cloudflare.com)💡 Suggested doc-only addition
TURNSTILE_SITE_KEY= TURNSTILE_SECRET_KEY= + +# Local/CI only: official Cloudflare dummy credentials. +# TURNSTILE_SITE_KEY=1x00000000000000000000AA +# TURNSTILE_SECRET_KEY=1x0000000000000000000000000000000AA🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.example around lines 61 - 62, The .env.example leaves TURNSTILE_SITE_KEY and TURNSTILE_SECRET_KEY blank which prevents local/CI from exercising Turnstile auth; update the file to include commented example dummy Cloudflare test keys and a short note explaining they are only for localhost/CI (not production) so developers can copy the file and run tests immediately—specifically add a commented example line(s) showing the published test site/secret pair and a one-line comment referencing Cloudflare testing docs, near the TURNSTILE_SITE_KEY and TURNSTILE_SECRET_KEY entries.resources/js/forgot_password/forgot_password.js (1)
61-67: Rename variable to reflect Turnstile migration.The variable
recaptchaResponseshould be renamed toturnstileResponse(or similar) for clarity since this is now using Cloudflare Turnstile.✨ Suggested rename
onSubmit: (values) => { - const recaptchaResponse = captcha.current?.getResponse(); - if (!recaptchaResponse) { + const turnstileResponse = captcha.current?.getResponse(); + if (!turnstileResponse) { setCaptchaConfirmation("Remember to check the captcha"); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/forgot_password/forgot_password.js` around lines 61 - 67, Rename the variable recaptchaResponse to turnstileResponse (or similar) where it captures captcha.current?.getResponse() and where it is checked and used; update the conditional that checks the value, the call to setCaptchaConfirmation, and any references before calling doHtmlFormPost() so names reflect Cloudflare Turnstile (update the variable in the function containing captcha.current?.getResponse(), the if (!recaptchaResponse) check, and subsequent usages).resources/js/set_password/set_password.js (1)
78-84: Rename variable to reflect Turnstile migration.Same as in
forgot_password.js, the variablerecaptchaResponseshould be renamed toturnstileResponsefor consistency with the migration.✨ Suggested rename
onSubmit: (values) => { - const recaptchaResponse = captcha.current?.getResponse(); - if (!recaptchaResponse) { + const turnstileResponse = captcha.current?.getResponse(); + if (!turnstileResponse) { setCaptchaConfirmation("Remember to check the captcha"); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/set_password/set_password.js` around lines 78 - 84, The variable recaptchaResponse used in the set_password flow should be renamed to turnstileResponse to match the Turnstile migration and maintain consistency with forgot_password.js; update the declaration that captures captcha.current?.getResponse() to const turnstileResponse, replace all uses of recaptchaResponse (including the truthy check and the setCaptchaConfirmation branch) with turnstileResponse, and ensure any related messaging or comments referencing "recaptcha" are updated accordingly while leaving doHtmlFormPost and setCaptchaConfirmation calls 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 `@package.json`:
- Around line 47-50: package.json currently references an old react-redux that
is incompatible with React 17; update the react-redux dependency to ^7.2.0 or
higher in package.json (replace the existing react-redux entry) and then run
your package manager install and a quick test/build to ensure no runtime
breakage; update any imports if needed (react-redux's
Provider/connect/useSelector/useDispatch remain the same for most apps) and
adjust code where deprecated v5 APIs are used.
In `@resources/js/forgot_password/forgot_password.module.scss`:
- Around line 17-21: Move the .turnstile rule into the
.forgot_password_container block and fix indentation so it matches the other
auth modules; specifically, locate the .turnstile selector currently nested
under .main_container and relocate its declaration inside the
.forgot_password_container selector (ensuring proper two-space indentation like
surrounding rules) so styles are consistently scoped to the forgot_password
form.
---
Nitpick comments:
In @.env.example:
- Around line 61-62: The .env.example leaves TURNSTILE_SITE_KEY and
TURNSTILE_SECRET_KEY blank which prevents local/CI from exercising Turnstile
auth; update the file to include commented example dummy Cloudflare test keys
and a short note explaining they are only for localhost/CI (not production) so
developers can copy the file and run tests immediately—specifically add a
commented example line(s) showing the published test site/secret pair and a
one-line comment referencing Cloudflare testing docs, near the
TURNSTILE_SITE_KEY and TURNSTILE_SECRET_KEY entries.
In `@resources/js/forgot_password/forgot_password.js`:
- Around line 61-67: Rename the variable recaptchaResponse to turnstileResponse
(or similar) where it captures captcha.current?.getResponse() and where it is
checked and used; update the conditional that checks the value, the call to
setCaptchaConfirmation, and any references before calling doHtmlFormPost() so
names reflect Cloudflare Turnstile (update the variable in the function
containing captcha.current?.getResponse(), the if (!recaptchaResponse) check,
and subsequent usages).
In `@resources/js/reset_password/reset_password.js`:
- Around line 175-180: The Turnstile component usage should explicitly set the
request field name to match backend validators: add the render prop/option
responseFieldName with the string "cf-turnstile-response" to the Turnstile
component (the element using ref={captcha}, siteKey={captchaPublicKey},
onSuccess={onChangeRecaptcha}, className={styles.turnstile}) so the frontend
submits the expected cf-turnstile-response field.
In `@resources/js/set_password/set_password.js`:
- Around line 78-84: The variable recaptchaResponse used in the set_password
flow should be renamed to turnstileResponse to match the Turnstile migration and
maintain consistency with forgot_password.js; update the declaration that
captures captcha.current?.getResponse() to const turnstileResponse, replace all
uses of recaptchaResponse (including the truthy check and the
setCaptchaConfirmation branch) with turnstileResponse, and ensure any related
messaging or comments referencing "recaptcha" are updated accordingly while
leaving doHtmlFormPost and setCaptchaConfirmation calls intact.
In `@resources/js/signup/signup.js`:
- Around line 278-283: The Turnstile component usage relies on the wrapper
default for the form field name while backend validators expect
"cf-turnstile-response"; update the Turnstile element (the JSX using Turnstile
with ref={captcha}, siteKey={captchaPublicKey}, onSuccess={onChangeRecaptcha})
to set the responseFieldName prop explicitly to "cf-turnstile-response" so the
frontend and backend contract is explicit and not brittle.
In `@webpack.common.js`:
- Around line 86-91: The current webpack rule with test: /\.m?js/ and resolve: {
fullySpecified: false } is too broad and affects your source files; narrow it by
anchoring the regex (use /\.m?js$/) and scoping the rule to third-party packages
(add an include or test against /node_modules/) so the fullySpecified workaround
only applies to node_modules ESM packages (e.g., `@marsidev/react-turnstile`) and
does not mask import-resolution issues in your own code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 80b882b4-3b33-4119-b079-75529e88d6b2
⛔ Files ignored due to path filters (2)
composer.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (36)
.env.exampleapp/Http/Controllers/Auth/EmailVerificationController.phpapp/Http/Controllers/Auth/ForgotPasswordController.phpapp/Http/Controllers/Auth/PasswordSetController.phpapp/Http/Controllers/Auth/RegisterController.phpapp/Http/Controllers/Auth/ResetPasswordController.phpapp/Http/Controllers/UserController.phpcomposer.jsonconfig/app.phpconfig/recaptcha.phpconfig/services.phppackage.jsonpublic/assets/js/auth/email-verification.jspublic/assets/js/auth/registration.jspublic/assets/js/auth/reset_password.jspublic/assets/js/auth/send_password_link.jspublic/assets/js/auth/set_password.jsresources/js/email_verification/email_verification.jsresources/js/email_verification/email_verification.module.scssresources/js/forgot_password/forgot_password.jsresources/js/forgot_password/forgot_password.module.scssresources/js/login/login.jsresources/js/login/login.module.scssresources/js/reset_password/reset_password.jsresources/js/reset_password/reset_password.module.scssresources/js/set_password/set_password.jsresources/js/set_password/set_password.module.scssresources/js/signup/signup.jsresources/js/signup/signup.module.scssresources/views/auth/email_verification.blade.phpresources/views/auth/login.blade.phpresources/views/auth/passwords/email.blade.phpresources/views/auth/passwords/reset.blade.phpresources/views/auth/passwords/set.blade.phpresources/views/auth/register.blade.phpwebpack.common.js
💤 Files with no reviewable changes (7)
- public/assets/js/auth/email-verification.js
- config/app.php
- public/assets/js/auth/registration.js
- config/recaptcha.php
- public/assets/js/auth/reset_password.js
- public/assets/js/auth/send_password_link.js
- public/assets/js/auth/set_password.js
Contributor
Author
Reviewed Dev Notes
|
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-121/ This page is automatically updated on each push to this PR. |
matiasperrone-exo
force-pushed
the
feat/migrate-from-google-recaptcha-to-cloudflare-turnstile
branch
from
28a88dd to
bbe977e
Compare
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-121/ This page is automatically updated on each push to this PR. |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
resources/js/signup/signup.js (1)
96-100: Consider renaming the legacy recaptcha handler.
onChangeRecaptchais now misleading in a Turnstile flow; renaming to something likeonTurnstileSuccesswould improve readability.Also applies to: 283-283
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/js/signup/signup.js` around lines 96 - 100, The handler name onChangeRecaptcha is misleading for the Turnstile flow; rename the function to onTurnstileSuccess and update all references (including where it's passed to components or event props — e.g., any usage of onChangeRecaptcha around the signup form and the other occurrence referenced at line ~283) so calls, bindings, and tests use the new identifier; keep the implementation unchanged (it should still call setCaptchaConfirmation(null) when captcha.current?.getResponse() is truthy).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.example:
- Around line 61-62: Reorder the TURNSTILE env keys in .env.example so
TURNSTILE_SECRET_KEY appears before TURNSTILE_SITE_KEY to satisfy dotenv-linter;
locate the two lines containing TURNSTILE_SITE_KEY and TURNSTILE_SECRET_KEY and
swap them so the SECRET key is listed first (preserve the variable names and
blank values).
In `@resources/js/forgot_password/forgot_password.js`:
- Around line 14-15: The import uses createTheme which doesn't exist in
`@material-ui/core` v4.11.4; change the import to bring in createMuiTheme instead
of createTheme and update any usage of createTheme(...) to createMuiTheme(...)
(referencing the existing MuiThemeProvider usage to ensure the produced theme is
passed to MuiThemeProvider).
In `@resources/js/set_password/set_password.js`:
- Around line 87-91: Rename the reCAPTCHA-specific identifier to a generic
captcha/turnstile name: change the function onChangeRecaptcha to onChangeCaptcha
(and update any other occurrences, including the other instance at line 289) and
replace any reCAPTCHA-specific checks or variable names (e.g., references to
"recaptcha" in the function or captcha.current?.getResponse()) with the generic
captcha/turnstile equivalents used elsewhere in this module so the flow no
longer contains the "recaptcha" token string.
---
Nitpick comments:
In `@resources/js/signup/signup.js`:
- Around line 96-100: The handler name onChangeRecaptcha is misleading for the
Turnstile flow; rename the function to onTurnstileSuccess and update all
references (including where it's passed to components or event props — e.g., any
usage of onChangeRecaptcha around the signup form and the other occurrence
referenced at line ~283) so calls, bindings, and tests use the new identifier;
keep the implementation unchanged (it should still call
setCaptchaConfirmation(null) when captcha.current?.getResponse() is truthy).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5b6e5675-7070-45e7-95e3-af4248d698c2
⛔ Files ignored due to path filters (2)
composer.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (36)
.env.exampleapp/Http/Controllers/Auth/EmailVerificationController.phpapp/Http/Controllers/Auth/ForgotPasswordController.phpapp/Http/Controllers/Auth/PasswordSetController.phpapp/Http/Controllers/Auth/RegisterController.phpapp/Http/Controllers/Auth/ResetPasswordController.phpapp/Http/Controllers/UserController.phpcomposer.jsonconfig/app.phpconfig/recaptcha.phpconfig/services.phppackage.jsonpublic/assets/js/auth/email-verification.jspublic/assets/js/auth/registration.jspublic/assets/js/auth/reset_password.jspublic/assets/js/auth/send_password_link.jspublic/assets/js/auth/set_password.jsresources/js/email_verification/email_verification.jsresources/js/email_verification/email_verification.module.scssresources/js/forgot_password/forgot_password.jsresources/js/forgot_password/forgot_password.module.scssresources/js/login/login.jsresources/js/login/login.module.scssresources/js/reset_password/reset_password.jsresources/js/reset_password/reset_password.module.scssresources/js/set_password/set_password.jsresources/js/set_password/set_password.module.scssresources/js/signup/signup.jsresources/js/signup/signup.module.scssresources/views/auth/email_verification.blade.phpresources/views/auth/login.blade.phpresources/views/auth/passwords/email.blade.phpresources/views/auth/passwords/reset.blade.phpresources/views/auth/passwords/set.blade.phpresources/views/auth/register.blade.phpwebpack.common.js
💤 Files with no reviewable changes (7)
- public/assets/js/auth/email-verification.js
- config/app.php
- config/recaptcha.php
- public/assets/js/auth/send_password_link.js
- public/assets/js/auth/reset_password.js
- public/assets/js/auth/set_password.js
- public/assets/js/auth/registration.js
✅ Files skipped from review due to trivial changes (13)
- resources/js/reset_password/reset_password.module.scss
- webpack.common.js
- resources/js/email_verification/email_verification.module.scss
- resources/js/forgot_password/forgot_password.module.scss
- app/Http/Controllers/Auth/PasswordSetController.php
- resources/views/auth/login.blade.php
- resources/js/signup/signup.module.scss
- resources/views/auth/register.blade.php
- app/Http/Controllers/Auth/ForgotPasswordController.php
- resources/views/auth/passwords/email.blade.php
- resources/views/auth/passwords/set.blade.php
- config/services.php
- resources/views/auth/email_verification.blade.php
🚧 Files skipped from review as they are similar to previous changes (12)
- app/Http/Controllers/Auth/RegisterController.php
- app/Http/Controllers/Auth/ResetPasswordController.php
- resources/js/login/login.module.scss
- resources/views/auth/passwords/reset.blade.php
- app/Http/Controllers/Auth/EmailVerificationController.php
- resources/js/set_password/set_password.module.scss
- app/Http/Controllers/UserController.php
- resources/js/reset_password/reset_password.js
- resources/js/email_verification/email_verification.js
- package.json
- composer.json
- resources/js/login/login.js
Comment on lines
+14
to
+15
| import { MuiThemeProvider, createTheme } from "@material-ui/core/styles"; | ||
| import { useFormik } from "formik"; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For @material-ui/coreversion 4.11.4, iscreateThemeexported from "@material-ui/core/styles", or shouldcreateMuiTheme be used?
💡 Result:
For @material-ui/core version 4.11.4, use createMuiTheme from "@material-ui/core/styles". createTheme is not exported in this version; it was introduced later as part of v5 preparation, with deprecation warnings for createMuiTheme appearing in v4.11.x and later minor releases like v4.12.x. The official v4 documentation and source code for v4.11.4 confirm createMuiTheme as the function to use, while createTheme examples in docs are forward-looking for v5 migration.
Citations:
- 1: https://stackoverflow.com/questions/68543960/react-material-ui-core-v4-11-0-the-createmuitheme-function-was-renamed-to
- 2: [theme] Deprecate
createMuiThememui/material-ui#26004 - 3: https://v4.mui.com/styles/api/
- 4: https://v4.mui.com/styles/basics/
- 5: Deprecation warnings when on
4.11.4mui/material-ui#27265 - 6: https://v4.mui.com/customization/theming/
- 7: https://github.com/mui/material-ui/blob/v4.11.0/packages/material-ui/src/styles/createMuiTheme.js
- 8: https://github.com/mui/material-ui/blob/v4.11.4/packages/material-ui/src/index.js
- 9: https://github.com/mui/material-ui/blob/v4.6.0/packages/material-ui/src/styles/index.js
Use createMuiTheme for Material-UI v4.11.4 to avoid import failure.
Line 14 imports createTheme, but this function is not exported in @material-ui/core@4.11.4. Use createMuiTheme instead, which is the correct function for v4.x.
Corrected import and usage
-import { MuiThemeProvider, createTheme } from "@material-ui/core/styles";
+import { MuiThemeProvider, createMuiTheme } from "@material-ui/core/styles";
...
-const theme = createTheme({
+const theme = createMuiTheme({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@resources/js/forgot_password/forgot_password.js` around lines 14 - 15, The
import uses createTheme which doesn't exist in `@material-ui/core` v4.11.4; change
the import to bring in createMuiTheme instead of createTheme and update any
usage of createTheme(...) to createMuiTheme(...) (referencing the existing
MuiThemeProvider usage to ensure the produced theme is passed to
MuiThemeProvider).
bbe977e to
83abac7
Compare
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-121/ This page is automatically updated on each push to this PR. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@resources/js/login/login.js`:
- Around line 266-273: The captcha rendering condition in the JSX uses only
shouldShowCaptcha() but may pass an undefined siteKey to the Turnstile
component; update the conditional to match PasswordInputForm by rendering
Turnstile only when both shouldShowCaptcha() and captchaPublicKey are truthy
(e.g., change the guard around the <Turnstile ...> block to use
shouldShowCaptcha() && captchaPublicKey) so Turnstile always receives a valid
siteKey; keep the existing props like options and
onSuccess/onChangeCaptchaProvider unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4c5405dd-6ceb-4595-baa0-668c1526be5f
⛔ Files ignored due to path filters (2)
composer.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (37)
.env.example.nvmrcapp/Http/Controllers/Auth/EmailVerificationController.phpapp/Http/Controllers/Auth/ForgotPasswordController.phpapp/Http/Controllers/Auth/PasswordSetController.phpapp/Http/Controllers/Auth/RegisterController.phpapp/Http/Controllers/Auth/ResetPasswordController.phpapp/Http/Controllers/UserController.phpcomposer.jsonconfig/app.phpconfig/recaptcha.phpconfig/services.phppackage.jsonpublic/assets/js/auth/email-verification.jspublic/assets/js/auth/registration.jspublic/assets/js/auth/reset_password.jspublic/assets/js/auth/send_password_link.jspublic/assets/js/auth/set_password.jsresources/js/email_verification/email_verification.jsresources/js/email_verification/email_verification.module.scssresources/js/forgot_password/forgot_password.jsresources/js/forgot_password/forgot_password.module.scssresources/js/login/login.jsresources/js/login/login.module.scssresources/js/reset_password/reset_password.jsresources/js/reset_password/reset_password.module.scssresources/js/set_password/set_password.jsresources/js/set_password/set_password.module.scssresources/js/signup/signup.jsresources/js/signup/signup.module.scssresources/views/auth/email_verification.blade.phpresources/views/auth/login.blade.phpresources/views/auth/passwords/email.blade.phpresources/views/auth/passwords/reset.blade.phpresources/views/auth/passwords/set.blade.phpresources/views/auth/register.blade.phpwebpack.common.js
💤 Files with no reviewable changes (7)
- public/assets/js/auth/email-verification.js
- config/recaptcha.php
- config/app.php
- public/assets/js/auth/reset_password.js
- public/assets/js/auth/registration.js
- public/assets/js/auth/send_password_link.js
- public/assets/js/auth/set_password.js
✅ Files skipped from review due to trivial changes (11)
- .nvmrc
- resources/js/reset_password/reset_password.module.scss
- resources/views/auth/passwords/reset.blade.php
- resources/views/auth/login.blade.php
- resources/views/auth/register.blade.php
- resources/js/email_verification/email_verification.module.scss
- resources/js/login/login.module.scss
- resources/js/forgot_password/forgot_password.module.scss
- resources/views/auth/passwords/email.blade.php
- resources/views/auth/passwords/set.blade.php
- config/services.php
🚧 Files skipped from review as they are similar to previous changes (14)
- resources/views/auth/email_verification.blade.php
- app/Http/Controllers/Auth/EmailVerificationController.php
- resources/js/set_password/set_password.module.scss
- webpack.common.js
- app/Http/Controllers/Auth/PasswordSetController.php
- app/Http/Controllers/Auth/ForgotPasswordController.php
- app/Http/Controllers/Auth/ResetPasswordController.php
- resources/js/signup/signup.module.scss
- app/Http/Controllers/UserController.php
- resources/js/email_verification/email_verification.js
- resources/js/set_password/set_password.js
- resources/js/reset_password/reset_password.js
- resources/js/signup/signup.js
- package.json
Collaborator
|
@matiasperrone-exo u published secret keys to a public repository |
Contributor
There was a problem hiding this comment.
Pull request overview
Migrates OpenStackID authentication flows from Google reCAPTCHA v2 to Cloudflare Turnstile, updating both Laravel back-end validation and React front-end widgets, plus related configuration and dependency changes.
Changes:
- Replace reCAPTCHA packages/config with Turnstile (Laravel validation rules + services config).
- Update all auth React UIs / Blade config injection to use
@marsidev/react-turnstileandcf-turnstile-response. - Clean up legacy assets and adjust build tooling (webpack ESM workaround, Node version bump).
Reviewed changes
Copilot reviewed 37 out of 39 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| webpack.common.js | Adds webpack 5 ESM resolution workaround for Turnstile dependency. |
| resources/views/auth/register.blade.php | Injects Turnstile site key into signup React config. |
| resources/views/auth/passwords/set.blade.php | Injects Turnstile site key into set-password React config. |
| resources/views/auth/passwords/reset.blade.php | Injects Turnstile site key into reset-password React config. |
| resources/views/auth/passwords/email.blade.php | Injects Turnstile site key into forgot-password React config. |
| resources/views/auth/login.blade.php | Injects Turnstile site key into login React config. |
| resources/views/auth/email_verification.blade.php | Injects Turnstile site key into email-verification React config. |
| resources/js/signup/signup.module.scss | Renames captcha styling hook to .turnstile. |
| resources/js/signup/signup.js | Replaces ReCAPTCHA widget with Turnstile + updated token retrieval. |
| resources/js/set_password/set_password.module.scss | Renames captcha styling hook to .turnstile. |
| resources/js/set_password/set_password.js | Replaces ReCAPTCHA widget with Turnstile + updated token retrieval. |
| resources/js/reset_password/reset_password.module.scss | Renames captcha styling hook to .turnstile. |
| resources/js/reset_password/reset_password.js | Replaces ReCAPTCHA widget with Turnstile + updated token retrieval. |
| resources/js/login/login.module.scss | Renames captcha styling hook to .turnstile. |
| resources/js/login/login.js | Replaces two ReCAPTCHA instances with Turnstile; keeps conditional captcha logic. |
| resources/js/forgot_password/forgot_password.module.scss | Adds missing .turnstile styling hook. |
| resources/js/forgot_password/forgot_password.js | Replaces ReCAPTCHA widget with Turnstile + updated token retrieval. |
| resources/js/email_verification/email_verification.module.scss | Renames captcha styling hook to .turnstile. |
| resources/js/email_verification/email_verification.js | Replaces ReCAPTCHA widget with Turnstile + updated token retrieval. |
| public/assets/js/auth/set_password.js | Deletes dead legacy jQuery auth script. |
| public/assets/js/auth/send_password_link.js | Deletes dead legacy jQuery auth script. |
| public/assets/js/auth/reset_password.js | Deletes dead legacy jQuery auth script. |
| public/assets/js/auth/registration.js | Deletes dead legacy jQuery auth script. |
| public/assets/js/auth/email-verification.js | Deletes dead legacy jQuery auth script. |
| package.json | Removes reCAPTCHA deps, adds Turnstile, bumps React-related packages. |
| config/services.php | Adds services.turnstile.key/secret env wiring. |
| config/recaptcha.php | Removes legacy reCAPTCHA config file. |
| config/app.php | Removes legacy reCAPTCHA service provider and facade alias. |
| composer.lock | Removes legacy PHP recaptcha package; adds Turnstile package. |
| composer.json | Drops legacy recaptcha VCS repo + dependency; adds Turnstile dependency. |
| app/Http/Controllers/UserController.php | Switches login captcha validation to cf-turnstile-response + turnstile rule. |
| app/Http/Controllers/Auth/ResetPasswordController.php | Switches captcha validation to Turnstile rule/field name. |
| app/Http/Controllers/Auth/RegisterController.php | Switches captcha validation to Turnstile rule/field name. |
| app/Http/Controllers/Auth/PasswordSetController.php | Switches captcha validation to Turnstile rule/field name. |
| app/Http/Controllers/Auth/ForgotPasswordController.php | Adds server-side Turnstile validation. |
| app/Http/Controllers/Auth/EmailVerificationController.php | Switches captcha validation to Turnstile rule/field name. |
| .nvmrc | Bumps Node version for local/CI tooling. |
| .env.example | Replaces reCAPTCHA env vars with Turnstile env vars. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
matiasperrone-exo
force-pushed
the
feat/migrate-from-google-recaptcha-to-cloudflare-turnstile
branch
2 times, most recently
from
339eb1c to
7c4c673
Compare
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-121/ This page is automatically updated on each push to this PR. |
1 similar comment
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-121/ This page is automatically updated on each push to this PR. |
Contributor
Author
Fixed |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/Http/Controllers/UserController.php`:
- Around line 422-423: Do not trust the client-provided $login_attempts when
building $rules in UserController; instead compute failed attempts from
server-side state (session, cache, or DB) using a stable key (e.g. user id or
IP/session) and use that server-derived value when comparing to
$max_login_attempts_2_show_captcha to decide whether to add the
'cf-turnstile-response' rule; update the code that currently reads
$login_attempts from request input to read from your server-side counter (or
increment it on failed auth) so $rules and the 'cf-turnstile-response'
requirement are enforced reliably.
In `@resources/js/login/login.js`:
- Around line 187-191: The Turnstile widgets only use onSuccess and leave
this.state.captcha_value populated after expiry/error; add onExpire and onError
handlers on both Turnstile instances to clear the captcha (e.g., call the
existing onChangeCaptchaProvider handler or set this.setState({ captcha_value:
'' })) so the component state reflects an expired/failed widget and the auth
gate (which checks this.state.captcha_value) behaves correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c86f88b6-f9bf-4f5f-af21-07fda56f9e89
⛔ Files ignored due to path filters (2)
composer.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (37)
.env.example.nvmrcapp/Http/Controllers/Auth/EmailVerificationController.phpapp/Http/Controllers/Auth/ForgotPasswordController.phpapp/Http/Controllers/Auth/PasswordSetController.phpapp/Http/Controllers/Auth/RegisterController.phpapp/Http/Controllers/Auth/ResetPasswordController.phpapp/Http/Controllers/UserController.phpcomposer.jsonconfig/app.phpconfig/recaptcha.phpconfig/services.phppackage.jsonpublic/assets/js/auth/email-verification.jspublic/assets/js/auth/registration.jspublic/assets/js/auth/reset_password.jspublic/assets/js/auth/send_password_link.jspublic/assets/js/auth/set_password.jsresources/js/email_verification/email_verification.jsresources/js/email_verification/email_verification.module.scssresources/js/forgot_password/forgot_password.jsresources/js/forgot_password/forgot_password.module.scssresources/js/login/login.jsresources/js/login/login.module.scssresources/js/reset_password/reset_password.jsresources/js/reset_password/reset_password.module.scssresources/js/set_password/set_password.jsresources/js/set_password/set_password.module.scssresources/js/signup/signup.jsresources/js/signup/signup.module.scssresources/views/auth/email_verification.blade.phpresources/views/auth/login.blade.phpresources/views/auth/passwords/email.blade.phpresources/views/auth/passwords/reset.blade.phpresources/views/auth/passwords/set.blade.phpresources/views/auth/register.blade.phpwebpack.common.js
💤 Files with no reviewable changes (7)
- config/app.php
- public/assets/js/auth/send_password_link.js
- config/recaptcha.php
- public/assets/js/auth/email-verification.js
- public/assets/js/auth/registration.js
- public/assets/js/auth/reset_password.js
- public/assets/js/auth/set_password.js
✅ Files skipped from review due to trivial changes (14)
- .nvmrc
- app/Http/Controllers/Auth/PasswordSetController.php
- resources/views/auth/passwords/reset.blade.php
- resources/js/set_password/set_password.module.scss
- webpack.common.js
- resources/js/signup/signup.module.scss
- resources/views/auth/email_verification.blade.php
- resources/js/forgot_password/forgot_password.module.scss
- resources/views/auth/login.blade.php
- resources/js/reset_password/reset_password.module.scss
- app/Http/Controllers/Auth/ResetPasswordController.php
- resources/views/auth/passwords/set.blade.php
- resources/views/auth/register.blade.php
- resources/js/login/login.module.scss
🚧 Files skipped from review as they are similar to previous changes (9)
- resources/js/email_verification/email_verification.module.scss
- app/Http/Controllers/Auth/EmailVerificationController.php
- config/services.php
- resources/views/auth/passwords/email.blade.php
- resources/js/signup/signup.js
- resources/js/reset_password/reset_password.js
- composer.json
- package.json
- app/Http/Controllers/Auth/ForgotPasswordController.php
Comment on lines
+422
to
+423
| if ($login_attempts >= $max_login_attempts_2_show_captcha) { | ||
| $rules['g-recaptcha-response'] = 'required|recaptcha'; | ||
| $rules['cf-turnstile-response'] = 'required|turnstile'; |
There was a problem hiding this comment.
Don't trust login_attempts from the request when deciding whether Turnstile is required.
Because login_attempts is loaded from request input on Line 413, a client can post 0 and skip this rule entirely after repeated failures. Derive the failed-attempt count from server-side state before building $rules.
🔒 Suggested direction
- $login_attempts = intval(Request::input('login_attempts'));
+ $login_attempts = 0;
+ if (!empty($data['username'])) {
+ $candidate = $this->auth_service->getUserByUsername($data['username']);
+ if (!is_null($candidate)) {
+ $login_attempts = $candidate->getLoginFailedAttempt();
+ }
+ }If unknown usernames should also trigger CAPTCHA, use an IP/session-based counter here rather than a client-provided field.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Http/Controllers/UserController.php` around lines 422 - 423, Do not trust
the client-provided $login_attempts when building $rules in UserController;
instead compute failed attempts from server-side state (session, cache, or DB)
using a stable key (e.g. user id or IP/session) and use that server-derived
value when comparing to $max_login_attempts_2_show_captcha to decide whether to
add the 'cf-turnstile-response' rule; update the code that currently reads
$login_attempts from request input to read from your server-side counter (or
increment it on failed auth) so $rules and the 'cf-turnstile-response'
requirement are enforced reliably.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
matiasperrone-exo
force-pushed
the
feat/migrate-from-google-recaptcha-to-cloudflare-turnstile
branch
from
7c4c673 to
ced0eac
Compare
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-121/ This page is automatically updated on each push to this PR. |
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-121/ This page is automatically updated on each push to this PR. |
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
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.
Task:
Ref: https://app.clickup.com/t/86b8xw6ue
Goal
Replace the legacy Google reCAPTCHA v2 implementation (greggilbert/recaptcha + react-google-recaptcha) with Cloudflare Turnstile across all 6 authentication forms (login, registration, forgot
password, reset password, set password, email verification) in OpenStackID. This is a hard replacement.
Tasks
Acceptance Criteria
Dev Notes
Summary by CodeRabbit