Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only use CAPTCHA in logged out operations and pages #10607

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

eliasylonen
Copy link
Contributor

Issue #10235

@eliasylonen eliasylonen requested a review from lucasbordeau March 2, 2025 22:07
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR optimizes CAPTCHA loading by restricting it to specific authentication-related paths rather than loading it on every page, improving performance for logged-in users.

  • Added packages/twenty-front/src/modules/captcha/constants/CaptchaProtectedPaths.ts defining specific routes (sign-in/up, verification, password reset, invite) where CAPTCHA is required
  • Added packages/twenty-front/src/modules/captcha/utils/isCaptchaRequiredForPath.ts utility that checks if the current path needs CAPTCHA protection
  • Modified CaptchaProvider.tsx to conditionally render CAPTCHA components only when required for the current path
  • Added captcha-protected-fields.constants.ts on the server side to limit CAPTCHA validation to specific GraphQL operations
  • Updated captcha.guard.ts to skip CAPTCHA validation for operations not in the protected fields list

9 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile


if (!CAPTCHA_PROTECTED_FIELDS.includes(operationName)) {
return true;
}

const { captchaToken: token } = ctx.getArgs();
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider handling the case where token is undefined more explicitly rather than defaulting to an empty string.

@lucasbordeau lucasbordeau requested review from martmull and Weiko and removed request for lucasbordeau and martmull March 3, 2025 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants