feat(auth): enable recaptcha for signup #318
Conversation
WalkthroughThe pull request introduces several updates to facilitate the integration of Google reCAPTCHA into a web application. Key changes include the addition of reCAPTCHA configuration in the deployment workflow, updates to secret management, and the introduction of a custom signup form that incorporates reCAPTCHA verification. Additional adjustments include environment variable definitions, logger configurations, and dependency management in the Changes
Possibly related PRs
Poem
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Outside diff range and nitpick comments (2)
fiesta/apps/accounts/forms/sign_up.py (1)
8-14: Consider enhancing error handlingThe current implementation uses default error messages. Consider customizing the error handling to provide user-friendly messages and proper logging.
Example enhancement:
from django.utils.translation import gettext_lazy as _ import logging logger = logging.getLogger(__name__) class SignupForm(AllauthSignupForm): recaptcha = ReCaptchaField( widget=ReCaptchaV3( action="signup", attrs={"theme": "clean"}, ), score_threshold=0.5, error_messages={ 'required': _('Please verify that you are not a robot.'), 'invalid-input-response': _('reCAPTCHA verification failed, please try again.'), 'bad-request': _('Invalid reCAPTCHA request, please try again.'), } ) def clean(self): cleaned_data = super().clean() if 'recaptcha' in self._errors: logger.warning( 'reCAPTCHA validation failed', extra={'errors': self._errors['recaptcha']} ) return cleaned_datafiesta/apps/accounts/templates/account/signup_form.html (1)
26-29: LGTM: Clean reCAPTCHA integration with proper error handlingThe reCAPTCHA field is properly integrated into the form with appropriate error handling.
Consider adding ARIA attributes for better accessibility:
- <div class="Forms__field"> + <div class="Forms__field" role="group" aria-label="CAPTCHA verification"> {{ form.recaptcha }} - {% if form.errors.recaptcha %}<div class="Forms__error_text">{{ form.errors.recaptcha }}</div>{% endif %} + {% if form.errors.recaptcha %}<div class="Forms__error_text" role="alert">{{ form.errors.recaptcha }}</div>{% endif %} </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
.github/workflows/deploy.yml(1 hunks)charts/templates/web-secret.yaml(1 hunks)charts/values.yaml(1 hunks)fiesta/.env.base(1 hunks)fiesta/apps/accounts/forms/sign_up.py(1 hunks)fiesta/apps/accounts/templates/account/signup_form.html(1 hunks)fiesta/fiesta/settings/__init__.py(1 hunks)fiesta/fiesta/settings/auth.py(3 hunks)fiesta/fiesta/settings/logging.py(1 hunks)fiesta/fiesta/settings/project.py(1 hunks)pyproject.toml(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- fiesta/.env.base
🔇 Additional comments (11)
charts/values.yaml (1)
42-44: Verify reCAPTCHA secrets in deployment workflow
The configuration looks correct, but ensure these placeholder values are properly replaced in the deployment workflow.
✅ Verification successful
reCAPTCHA secrets are properly configured in deployment workflow
The deployment workflow correctly maps the placeholder values to GitHub secrets RECAPTCHA_SITE_KEY and RECAPTCHA_SECRET_KEY, ensuring secure handling of the reCAPTCHA credentials in production.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if reCAPTCHA secrets are properly configured in deployment workflow
# Check if deployment workflow uses these secrets
rg -l 'RECAPTCHA.*KEY' .github/workflows/
Length of output: 72
Script:
#!/bin/bash
# Check the content of deploy.yml to see how reCAPTCHA secrets are handled
rg -A 5 -B 5 'RECAPTCHA.*KEY' .github/workflows/deploy.yml
Length of output: 659
pyproject.toml (2)
58-58: LGTM: Latest stable version of django-recaptcha
The addition of django-recaptcha = "^4.0.0" is appropriate as it's the latest stable version.
107-109: LGTM: Updated ruff configuration structure
The ruff configuration has been properly restructured under the new lint hierarchy while maintaining the same functionality.
fiesta/fiesta/settings/__init__.py (1)
42-42: LGTM: Appropriate suppression of reCAPTCHA test key warning in development
The suppression of django_recaptcha.recaptcha_test_key_error is correctly scoped to the Development environment only.
Let's verify that production settings are properly configured:
✅ Verification successful
LGTM: reCAPTCHA configuration is properly handled in production
The production environment has proper configuration for reCAPTCHA with both site key and secret key being sourced from Helm values, while the warning suppression is correctly scoped to development only. The implementation follows security best practices by keeping sensitive keys in Kubernetes secrets.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if recaptcha settings are properly configured in production
rg -l "RECAPTCHA" charts/ .env*
Length of output: 72
Script:
#!/bin/bash
# Check the content of web-secret.yaml for RECAPTCHA configuration
cat charts/templates/web-secret.yaml
Length of output: 1239
fiesta/apps/accounts/templates/account/signup_form.html (1)
26-29: Verify complete reCAPTCHA integration
Let's ensure all required components are in place:
✅ Verification successful
reCAPTCHA integration is properly implemented ✅
The verification confirms all required components are in place:
- SignupForm correctly implements ReCaptchaField with ReCaptchaV3 widget
- Required RECAPTCHA configuration keys are present in settings
- Template implementation matches the form field
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for SignupForm implementation with reCAPTCHA
echo "Checking for SignupForm implementation..."
rg -A 5 "class SignupForm" fiesta/apps/accounts/
# Check for reCAPTCHA configuration
echo "Checking for reCAPTCHA configuration..."
rg "RECAPTCHA" fiesta/fiesta/settings/
Length of output: 899
fiesta/fiesta/settings/logging.py (1)
61-64: LGTM! Logger configuration for reCAPTCHA looks good.
The logger configuration follows the established pattern and correctly inherits the log level from LOG_LEVEL.
Let's verify no sensitive reCAPTCHA data is being logged:
✅ Verification successful
LGTM! No sensitive reCAPTCHA data logging detected
Based on the codebase analysis:
- The reCAPTCHA integration is properly configured with secure settings (RECAPTCHA_USE_SSL=True)
- Keys are handled securely through SecretValue() and Kubernetes secrets
- No logging of sensitive data (keys, tokens, or responses) was found in the codebase
- The logger configuration is appropriately set up for standard debug/info level logging
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for potential sensitive data logging in reCAPTCHA related files
rg -i "log\.|logger\." | rg -i "captcha|secret|key"
Length of output: 53
Script:
#!/bin/bash
# Search for reCAPTCHA related files first
fd -e py -e js captcha
Length of output: 24
Script:
#!/bin/bash
# Search for logging patterns in reCAPTCHA related code with context
rg -i "recaptcha" -A 5 -B 5 | rg -i "log\.|logger\.|print|debug"
Length of output: 157
Script:
#!/bin/bash
# Look for reCAPTCHA configuration and usage
rg -i "RECAPTCHA" -A 3 -B 3
Length of output: 5471
fiesta/fiesta/settings/auth.py (2)
102-105: LGTM! reCAPTCHA configuration follows security best practices.
Good security practices observed:
- Using SecretValue for sensitive keys
- SSL enabled by default
Let's verify the secret configuration in deployment:
✅ Verification successful
reCAPTCHA configuration is properly secured in deployment
The verification confirms that reCAPTCHA keys are handled securely:
- Keys are stored as GitHub secrets (
${{ secrets.RECAPTCHA_SITE_KEY }}and${{ secrets.RECAPTCHA_SECRET_KEY }}) - Values are base64 encoded in Helm charts
- Keys are properly injected into Django settings via environment variables
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if reCAPTCHA keys are properly configured in deployment
rg -l "RECAPTCHA.*KEY" charts/ .github/workflows/
Length of output: 113
Script:
#!/bin/bash
# Let's check the content of these files to verify the secure handling of reCAPTCHA keys
rg "RECAPTCHA.*KEY" .github/workflows/deploy.yml -B 2 -A 2
rg "RECAPTCHA.*KEY" charts/templates/web-secret.yaml -B 2 -A 2
Length of output: 643
80-82: LGTM! Custom signup form configuration looks good.
The custom form path is correctly configured in ACCOUNT_FORMS.
Let's verify the custom signup form implementation:
✅ Verification successful
Custom signup form implementation verified and properly configured
The SignupForm is correctly implemented:
- Properly inherits from allauth's SignupForm (aliased as AllauthSignupForm)
- Integrates ReCaptchaField with ReCaptchaV3 widget
- Form path in settings matches the actual implementation at
fiesta/apps/accounts/forms/sign_up.py
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the SignupForm properly extends allauth's form and implements reCAPTCHA
ast-grep --pattern 'class SignupForm(SignupForm):
$$$
captcha = $$$
$$$' apps/accounts/forms/sign_up.py
Length of output: 189
Script:
#!/bin/bash
# First, let's find the correct path for the signup form
fd "sign_up.py"
# Then search for any Python files containing SignupForm class
fd -e py | xargs rg "class SignupForm" -A 5
Length of output: 501
Script:
#!/bin/bash
# Let's check the imports to verify proper inheritance and recaptcha usage
rg -A 3 "^from|^import" fiesta/apps/accounts/forms/sign_up.py
Length of output: 304
fiesta/fiesta/settings/project.py (1)
104-105: LGTM! reCAPTCHA app is properly installed.
The django_recaptcha app is correctly added to INSTALLED_APPS with appropriate separation.
Let's verify the dependency is properly declared:
✅ Verification successful
✓ Dependency verification successful
django-recaptcha v4.0.0 is properly declared in both pyproject.toml and poetry.lock with correct package information and dependencies.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if django-recaptcha is declared in project dependencies
rg -l "django-recaptcha" pyproject.toml poetry.lock
Length of output: 78
Script:
#!/bin/bash
# Check the actual dependency declarations in both files
echo "=== pyproject.toml ==="
rg "django-recaptcha" pyproject.toml -A 2 -B 2
echo -e "\n=== poetry.lock ==="
rg "django-recaptcha" poetry.lock -A 5 -B 2
Length of output: 761
.github/workflows/deploy.yml (2)
185-187: Ensure reCAPTCHA domain configuration.
Since this deployment targets fiesta.plus (as specified in the environment URL), ensure that this domain is properly registered in the Google reCAPTCHA console. Missing domain registration could cause the reCAPTCHA verification to fail in production.
Consider documenting the reCAPTCHA domain configuration steps in your deployment documentation to prevent potential issues during environment setup.
185-187: LGTM! Verify secrets existence.
The reCAPTCHA configuration is properly structured and follows the existing pattern for secret management. The secrets are securely referenced using GitHub's secrets mechanism.
Let's verify that the required secrets are configured:
thejoeejoee
force-pushed
the
feat-auth-recaptcha
branch
from
add8579 to
4947e88
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
fiesta/fiesta/settings/auth.py (1)
102-104: Consider aligning environment variable names with Django's defaultsWhile the SecretValue usage is correct, the environment variable names differ from Django's defaults:
RECAPTCHA_SITE_KEYvs Django'sRECAPTCHA_PUBLIC_KEYRECAPTCHA_SECRET_KEYvs Django'sRECAPTCHA_PRIVATE_KEYThis might cause confusion for developers familiar with Django's conventions.
Consider using Django's standard environment variable names:
- RECAPTCHA_PUBLIC_KEY = SecretValue(environ_name="RECAPTCHA_SITE_KEY") - RECAPTCHA_PRIVATE_KEY = SecretValue(environ_name="RECAPTCHA_SECRET_KEY") + RECAPTCHA_PUBLIC_KEY = SecretValue(environ_name="RECAPTCHA_PUBLIC_KEY") + RECAPTCHA_PRIVATE_KEY = SecretValue(environ_name="RECAPTCHA_PRIVATE_KEY")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
.github/workflows/deploy.yml(1 hunks)charts/templates/web-secret.yaml(1 hunks)charts/values.yaml(1 hunks)fiesta/.env.base(1 hunks)fiesta/apps/accounts/forms/sign_up.py(1 hunks)fiesta/apps/accounts/templates/account/signup_form.html(1 hunks)fiesta/fiesta/settings/__init__.py(1 hunks)fiesta/fiesta/settings/auth.py(3 hunks)fiesta/fiesta/settings/logging.py(1 hunks)fiesta/fiesta/settings/project.py(1 hunks)pyproject.toml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- fiesta/fiesta/settings/project.py
- charts/values.yaml
- fiesta/.env.base
- fiesta/apps/accounts/templates/account/signup_form.html
- fiesta/fiesta/settings/logging.py
- charts/templates/web-secret.yaml
- fiesta/apps/accounts/forms/sign_up.py
- .github/workflows/deploy.yml
- fiesta/fiesta/settings/init.py
- pyproject.toml
🔇 Additional comments (4)
fiesta/fiesta/settings/auth.py (4)
3-3: LGTM: Appropriate use of SecretValue for sensitive configuration
The addition of SecretValue import is well-placed and follows best practices for handling sensitive configuration values.
106-106: LGTM: Secure default for reCAPTCHA SSL
Good security practice to enable SSL by default for reCAPTCHA communications.
102-106: Verify deployment configuration updates
Let's ensure the new reCAPTCHA environment variables are properly configured in deployment files.
✅ Verification successful
reCAPTCHA environment variables are properly configured in deployment files
The verification shows that the reCAPTCHA configuration is properly set up across all deployment files:
- GitHub workflow (
deploy.yml) includes the secretsRECAPTCHA_SITE_KEYandRECAPTCHA_SECRET_KEY - Kubernetes secret template maps these to the expected Django environment variables
DJANGO_RECAPTCHA_SITE_KEYandDJANGO_RECAPTCHA_SECRET_KEY - Values file includes the corresponding configuration structure under
secrets.recaptcha
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify deployment configurations include reCAPTCHA environment variables
# Check deployment workflow
echo "Checking deployment workflow..."
rg -A 5 "RECAPTCHA" .github/workflows/deploy.yml
# Check Kubernetes secrets
echo "Checking Kubernetes secrets configuration..."
rg -A 5 "RECAPTCHA" charts/templates/web-secret.yaml
# Check values file
echo "Checking values configuration..."
rg -A 5 "recaptcha" charts/values.yaml
Length of output: 1083
80-82: Verify the custom SignupForm implementation
The configuration points to a custom signup form. Let's verify its implementation.
✅ Verification successful
Custom SignupForm implementation verified successfully
The form exists at the correct path and is properly implemented with:
- Inheritance from allauth's SignupForm
- Integration of ReCaptchaV3 field with 'signup' action
- Clean implementation using django-recaptcha package
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the custom signup form exists and implements reCAPTCHA
# Check if the form file exists and contains reCAPTCHA implementation
rg -A 10 "class SignupForm" apps/accounts/forms/sign_up.py
# Verify proper inheritance from allauth's SignupForm
ast-grep --pattern 'class SignupForm($base) { $$$ }' apps/accounts/forms/sign_up.py
Length of output: 295
Script:
#!/bin/bash
# Let's first check if the accounts app and forms directory exist
fd -t d "accounts"
# Search for any Python files containing SignupForm class
fd -e py | xargs rg -l "class SignupForm"
# Search for any Python files in the entire codebase containing "signup" or "sign_up"
fd -e py | xargs rg -l -i "sign.?up"
Length of output: 438
Script:
#!/bin/bash
# Now that we found the correct path, let's check the SignupForm implementation
rg -A 10 "class SignupForm" fiesta/apps/accounts/forms/sign_up.py
# Check imports to verify allauth's SignupForm inheritance
rg "from allauth" fiesta/apps/accounts/forms/sign_up.py
# Check for reCAPTCHA related code
rg -i "captcha" fiesta/apps/accounts/forms/sign_up.py
Length of output: 585
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores
django-recaptcha.These changes enhance user experience by improving signup security and ensuring smoother deployment processes.