Feat | Migrate from Google reCAPTCHA to Cloudflare Turnstile#121
Open
matiasperrone-exo wants to merge 1 commit intomainfrom
Open
Feat | Migrate from Google reCAPTCHA to Cloudflare Turnstile#121matiasperrone-exo wants to merge 1 commit intomainfrom
matiasperrone-exo wants to merge 1 commit intomainfrom
Conversation
📝 WalkthroughWalkthroughReplaces Google reCAPTCHA with Cloudflare Turnstile across server-side validators, frontend React components, SCSS selectors, Blade templates, environment/config, dependencies; removes legacy jQuery auth scripts; adds Turnstile service config and a Webpack ESM resolution rule. 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 docstrings
🧪 Generate unit tests (beta)
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
+61
to
+62
| TURNSTILE_SITE_KEY= | ||
| TURNSTILE_SECRET_KEY= |
There was a problem hiding this comment.
Fix dotenv key order to satisfy linter.
Line 61-62 is reversed per dotenv-linter (TURNSTILE_SECRET_KEY should come before TURNSTILE_SITE_KEY).
🔧 Proposed fix
-TURNSTILE_SITE_KEY=
TURNSTILE_SECRET_KEY=
+TURNSTILE_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. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| TURNSTILE_SITE_KEY= | |
| TURNSTILE_SECRET_KEY= | |
| TURNSTILE_SECRET_KEY= | |
| TURNSTILE_SITE_KEY= |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 62-62: [UnorderedKey] The TURNSTILE_SECRET_KEY key should go before the TURNSTILE_SITE_KEY key
(UnorderedKey)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.env.example around lines 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).
There was a problem hiding this comment.
@matiasperrone-exo, understood! The key ordering has no functional impact — it's purely a linter preference. I'll note this for future reviews.
✏️ Learnings added
Learnt from: matiasperrone-exo
Repo: OpenStackweb/openstackid PR: 121
File: .env.example:61-62
Timestamp: 2026-03-24T22:07:44.046Z
Learning: In the OpenStackweb/openstackid repository, the team intentionally keeps TURNSTILE_SITE_KEY before TURNSTILE_SECRET_KEY in .env.example (non-alphabetical order) and does not follow dotenv-linter's UnorderedKey rule for these keys.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
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).
matiasperrone-exo
force-pushed
the
feat/migrate-from-google-recaptcha-to-cloudflare-turnstile
branch
from
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
Comment on lines
266
to
273
| {shouldShowCaptcha() && | ||
| <ReCAPTCHA | ||
| className={styles.recaptcha} | ||
| sitekey={captchaPublicKey} | ||
| onChange={onChangeRecaptcha} | ||
| <Turnstile | ||
| className={styles.turnstile} | ||
| siteKey={captchaPublicKey} | ||
| options={{ responseFieldName: "cf-turnstile-response" }} | ||
| onSuccess={onChangeCaptchaProvider} | ||
| /> | ||
| } |
There was a problem hiding this comment.
Inconsistent captcha rendering condition compared to PasswordInputForm.
Line 186 in PasswordInputForm checks both shouldShowCaptcha() && captchaPublicKey, but here only shouldShowCaptcha() is checked. If captchaPublicKey is falsy, the Turnstile component will receive an undefined siteKey, potentially causing a runtime error.
🔧 Suggested fix for consistency
- {shouldShowCaptcha() &&
+ {shouldShowCaptcha() && captchaPublicKey &&
<Turnstile
className={styles.turnstile}
siteKey={captchaPublicKey}
options={{ responseFieldName: "cf-turnstile-response" }}
onSuccess={onChangeCaptchaProvider}
/>
}📝 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
| {shouldShowCaptcha() && | |
| <ReCAPTCHA | |
| className={styles.recaptcha} | |
| sitekey={captchaPublicKey} | |
| onChange={onChangeRecaptcha} | |
| <Turnstile | |
| className={styles.turnstile} | |
| siteKey={captchaPublicKey} | |
| options={{ responseFieldName: "cf-turnstile-response" }} | |
| onSuccess={onChangeCaptchaProvider} | |
| /> | |
| } | |
| {shouldShowCaptcha() && captchaPublicKey && | |
| <Turnstile | |
| className={styles.turnstile} | |
| siteKey={captchaPublicKey} | |
| options={{ responseFieldName: "cf-turnstile-response" }} | |
| onSuccess={onChangeCaptchaProvider} | |
| /> | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@resources/js/login/login.js` around lines 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.
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.
| "file-loader": "^6.2.0", | ||
| "font-awesome": "4.7.0", | ||
| "history": "^4.7.2", | ||
| "html-webpack-plugin": "^3.2.0", |
There was a problem hiding this comment.
font-awesome was removed from package.json, but it is still imported from resources/js/bootstrap.js (import 'font-awesome/css/font-awesome.min.css';). This will break yarn install && yarn build unless the import is removed/replaced or the dependency is restored.
Suggested change
| "html-webpack-plugin": "^3.2.0", | |
| "html-webpack-plugin": "^3.2.0", | |
| "font-awesome": "^4.7.0", |
Comment on lines
266
to
+271
| {shouldShowCaptcha() && | ||
| <ReCAPTCHA | ||
| className={styles.recaptcha} | ||
| sitekey={captchaPublicKey} | ||
| onChange={onChangeRecaptcha} | ||
| <Turnstile | ||
| className={styles.turnstile} | ||
| siteKey={captchaPublicKey} | ||
| options={{ responseFieldName: "cf-turnstile-response" }} | ||
| onSuccess={onChangeCaptchaProvider} |
There was a problem hiding this comment.
In the OTP login flow, the Turnstile widget is rendered when shouldShowCaptcha() is true even if captchaPublicKey is empty/undefined. This is inconsistent with the password flow (which also checks captchaPublicKey) and can lead to a broken login UX (no usable widget / blocked submit) when the key is missing. Gate this render on captchaPublicKey as well, and consider aligning the submit-side check with the same condition.
Comment on lines
+87
to
+91
| // 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/, | ||
| include: /node_modules/, | ||
| resolve: { fullySpecified: false } |
There was a problem hiding this comment.
The fullySpecified workaround rule uses test: /\.m?js/ (no end anchor). This also matches .jsx files in node_modules and could unintentionally affect resolution beyond the intended .js/.mjs case. Consider tightening it to only match .mjs/.js (e.g., with an end-of-string anchor).
| signUpAction: '{{ URL::action("Auth\RegisterController@register") }}', | ||
| signUpError: signUpError, | ||
| captchaPublicKey: '{{ Config::get("recaptcha.public_key") }}', | ||
| captchaPublicKey: '{{ Config::get("services.turnstile.key") }}', |
There was a problem hiding this comment.
The PR description/task list says Blade templates should use Config::get("services.turnstile.site_key"), but the implementation uses services.turnstile.key (which also matches config/services.php). Please align the PR description/acceptance criteria with the actual config key name to avoid confusion for deployers.
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