feat: add optional ALTCHA integration for DRF public form submissions - #21
Merged
Conversation
Contributor
Reviewer's GuideAdds an optional ALTCHA contrib app that integrates proof-of-work anti-bot checks into DRF-based public form submissions, exposes ALTCHA as a synthetic field in the form-fields API, and wires challenge generation, verification, and replay protection into the existing Fobi DRF update flow while keeping the base package import-safe when ALTCHA is disabled. Sequence diagram for DRF public form submission with ALTCHA verificationsequenceDiagram
actor User
participant FrontendClient
participant FobiFormFieldsEndpoint as FobiDRFFormFieldsEndpoint
participant AltchaChallengeEndpoint as AltchaChallengeEndpoint
participant FobiFormEntryViewSet as FobiDRFFormEntryViewSet
participant AltchaPatch as AltchaPatchUpdate
participant AltchaChallengeVerifier as AltchaChallengeVerifier
participant DjangoCache
participant FobiHandlers
User->>FrontendClient: Open public form page
FrontendClient->>FobiFormFieldsEndpoint: GET /api/fobi-form-fields/slug/
FobiFormFieldsEndpoint->>FobiFormFieldsEndpoint: Check form_entry.is_public
FobiFormFieldsEndpoint->>FobiFormFieldsEndpoint: Check app installed and is_enabled
FobiFormFieldsEndpoint-->>FrontendClient: Fields JSON including AltchaField
FrontendClient->>AltchaChallengeEndpoint: GET /api/altcha-challenge/
AltchaChallengeEndpoint->>AltchaChallengeVerifier: create_challenge
AltchaChallengeVerifier->>AltchaChallengeVerifier: altcha.create_challenge_v1
AltchaChallengeVerifier-->>AltchaChallengeEndpoint: Challenge dict
AltchaChallengeEndpoint-->>FrontendClient: Challenge JSON
FrontendClient->>FrontendClient: Solve challenge client side
FrontendClient->>FobiFormEntryViewSet: PUT /api/fobi-form-entry/slug/ with form data and altcha payload
FobiFormEntryViewSet->>AltchaPatch: update_with_altcha
AltchaPatch->>AltchaPatch: Check conf.is_enabled
AltchaPatch->>AltchaPatch: instance = get_object, check instance.is_public
AltchaPatch->>AltchaChallengeVerifier: verify_payload(payload_b64)
AltchaChallengeVerifier->>AltchaChallengeVerifier: altcha.verify_solution_v1
AltchaChallengeVerifier->>DjangoCache: Check replay cache
DjangoCache-->>AltchaChallengeVerifier: Replay status
AltchaChallengeVerifier-->>AltchaPatch: ok or error
alt Altcha verification fails
AltchaPatch-->>FrontendClient: HTTP 400 with detail
else Altcha verification succeeds
AltchaPatch->>AltchaPatch: Remove altcha field from request.data
AltchaPatch->>FobiHandlers: original_update
FobiHandlers-->>FrontendClient: Form submission response
end
Class diagram for ALTCHA contrib components and their relationshipsclassDiagram
class UnfoldFobiAltchaConfig {
+str default_auto_field
+str name
+str label
+str verbose_name
}
class AltchaConf {
+get_hmac_secret()
+get_max_number()
+get_algorithm()
+get_field_name()
+get_cache_alias()
+get_challenge_expiry()
+is_enabled()
}
class AltchaChallenge {
+create_challenge()
+verify_payload(payload_b64)
}
class AltchaPatch {
+apply()
+update_with_altcha(self, request, args, kwargs)
-original_update
}
class AltchaViews {
+altcha_challenge(request)
}
class FobiFormEntryViewSet {
+update(self, request, args, kwargs)
}
class DjangoCacheBackend {
+get(key)
+set(key, value, timeout)
}
class AltchaLibrary {
+create_challenge_v1(hmac_key, max_number, algorithm, expires)
+verify_solution_v1(payload_b64, hmac_key, check_expires)
}
UnfoldFobiAltchaConfig --> AltchaConf : configures
AltchaPatch --> AltchaConf : reads_settings
AltchaPatch --> AltchaChallenge : uses_verify_payload
AltchaPatch --> FobiFormEntryViewSet : monkey_patches_update
AltchaChallenge --> AltchaConf : reads_settings
AltchaChallenge --> DjangoCacheBackend : replay_protection
AltchaChallenge --> AltchaLibrary : challenge_and_verification
AltchaViews --> AltchaChallenge : create_challenge
AltchaConf --> UnfoldFobiAltchaConfig : enabled_when_installed
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- Consider short-circuiting the
altcha_challengeview (e.g. 404/400) whenconf.is_enabled()is false to avoid hittingaltcha.create_challenge_v1with a missing HMAC secret and to keep the endpoint contract consistent with the rest of the opt-in integration. - In
verify_payload, using the raw base64 payload (even truncated) as the cache key for replay protection could be fragile or overly long; hashing the payload (e.g. SHA-256) into a fixed-length key would be more robust and cache-backend-friendly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider short-circuiting the `altcha_challenge` view (e.g. 404/400) when `conf.is_enabled()` is false to avoid hitting `altcha.create_challenge_v1` with a missing HMAC secret and to keep the endpoint contract consistent with the rest of the opt-in integration.
- In `verify_payload`, using the raw base64 payload (even truncated) as the cache key for replay protection could be fragile or overly long; hashing the payload (e.g. SHA-256) into a fixed-length key would be more robust and cache-backend-friendly.
## Individual Comments
### Comment 1
<location path="src/unfold_fobi/api/views.py" line_range="192" />
<code_context>
+ "name": get_field_name(),
+ "type": "AltchaField",
+ "widget": "AltchaWidget",
+ "label": "ALTCHA",
+ "required": True,
+ "help_text": "",
</code_context>
<issue_to_address>
**suggestion:** Consider making the ALTCHA label configurable or translatable instead of hardcoding it.
Since this label may be shown in the UI, consider sourcing it from configuration (like `get_field_name()`) or wrapping it in the existing translation/i18n helper so it can be localized consistently with the rest of the app.
Suggested implementation:
```python
from rest_framework.response import Response
from django.utils.translation import gettext_lazy as _
```
```python
form_structure["fields"].append({
"name": get_field_name(),
"type": "AltchaField",
"widget": "AltchaWidget",
"label": _("ALTCHA"),
"required": True,
"help_text": "",
})
```
1. If `django.utils.translation.gettext_lazy` is already imported elsewhere in this file (e.g., `from django.utils.translation import gettext_lazy as _`), remove the duplicate import I added to avoid redundancy.
2. If your project uses a different i18n helper alias (for example `ugettext_lazy` or a differently named `_`), adjust the import and the `_("ALTCHA")` call to match the existing convention.
</issue_to_address>
### Comment 2
<location path="src/unfold_fobi/contrib/altcha/challenge.py" line_range="10-19" />
<code_context>
+from . import conf
+
+
+def create_challenge():
+ """Generate a fresh ALTCHA challenge dict for the frontend."""
+ import altcha
+
+ expires = datetime.now(timezone.utc) + timedelta(seconds=conf.get_challenge_expiry())
+ challenge = altcha.create_challenge_v1(
+ hmac_key=conf.get_hmac_secret(),
+ max_number=conf.get_max_number(),
+ algorithm=conf.get_algorithm(),
+ expires=expires,
+ )
+ return challenge.to_dict()
+
+
</code_context>
<issue_to_address>
**issue (bug_risk):** `create_challenge` does not guard against missing HMAC secret, which could surface as a runtime error.
This assumes `conf.get_hmac_secret()` always returns a valid key. If the contrib app is installed but `UNFOLD_FOBI_ALTCHA_HMAC_SECRET` is missing or ALTCHA isn’t actually enabled, `altcha.create_challenge_v1` will likely raise and surface as a 500 from this endpoint. Consider either short‑circuiting when ALTCHA is disabled or raising an explicit configuration error so misconfiguration is handled predictably.
</issue_to_address>
### Comment 3
<location path="src/unfold_fobi/contrib/altcha/challenge.py" line_range="47-51" />
<code_context>
+ return False, err or "Invalid ALTCHA payload."
+
+ # Replay protection
+ cache = caches[conf.get_cache_alias()]
+ cache_key = f"altcha:replay:{payload_b64[:64]}"
+ if cache.get(cache_key):
+ return False, "ALTCHA payload already used."
+ cache.set(cache_key, 1, conf.get_challenge_expiry() * 2)
+
+ return True, None
</code_context>
<issue_to_address>
**issue (bug_risk):** Accessing the cache alias directly can raise if the alias is misconfigured, breaking verification.
If `UNFOLD_FOBI_ALTCHA_CACHE_ALIAS` is misconfigured, `caches[alias]` will raise and break all ALTCHA verification. Consider wrapping the lookup in a `try/except InvalidCacheBackendError` (or a broader `Exception`) and, on failure, skip replay protection but log the misconfiguration so form submissions still work and the issue is visible.
</issue_to_address>
### Comment 4
<location path="README.md" line_range="250-251" />
<code_context>
+1. `GET /api/fobi-form-fields/<slug>/` includes an `AltchaField` entry for
+ public forms when ALTCHA is enabled.
+2. Frontend fetches a challenge from `GET /api/altcha-challenge/`.
+3. Frontend solves the challenge client-side (e.g. using the `altcha` web
+ component) and includes the base64 payload as `"altcha"` in the PUT body.
+4. Backend verifies the payload before processing the form submission.
</code_context>
<issue_to_address>
**suggestion:** Align the documented payload field name with the configurable `UNFOLD_FOBI_ALTCHA_FIELD_NAME` setting.
Step 3 hardcodes the field name as "altcha", while earlier you introduce `UNFOLD_FOBI_ALTCHA_FIELD_NAME` (default "altcha"). This could confuse users who override the setting. Please reference the setting here and note that "altcha" is the default (e.g., “includes the base64 payload in the field specified by `UNFOLD_FOBI_ALTCHA_FIELD_NAME` (default: "altcha")”).
```suggestion
3. Frontend solves the challenge client-side (e.g. using the `altcha` web
component) and includes the base64 payload in the field specified by
`UNFOLD_FOBI_ALTCHA_FIELD_NAME` (default: `"altcha"`) in the PUT body.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| "name": get_field_name(), | ||
| "type": "AltchaField", | ||
| "widget": "AltchaWidget", | ||
| "label": "ALTCHA", |
Contributor
There was a problem hiding this comment.
suggestion: Consider making the ALTCHA label configurable or translatable instead of hardcoding it.
Since this label may be shown in the UI, consider sourcing it from configuration (like get_field_name()) or wrapping it in the existing translation/i18n helper so it can be localized consistently with the rest of the app.
Suggested implementation:
from rest_framework.response import Response
from django.utils.translation import gettext_lazy as _ form_structure["fields"].append({
"name": get_field_name(),
"type": "AltchaField",
"widget": "AltchaWidget",
"label": _("ALTCHA"),
"required": True,
"help_text": "",
})- If
django.utils.translation.gettext_lazyis already imported elsewhere in this file (e.g.,from django.utils.translation import gettext_lazy as _), remove the duplicate import I added to avoid redundancy. - If your project uses a different i18n helper alias (for example
ugettext_lazyor a differently named_), adjust the import and the_("ALTCHA")call to match the existing convention.
Comment on lines
+10
to
+19
| def create_challenge(): | ||
| """Generate a fresh ALTCHA challenge dict for the frontend.""" | ||
| import altcha | ||
|
|
||
| expires = datetime.now(timezone.utc) + timedelta(seconds=conf.get_challenge_expiry()) | ||
| challenge = altcha.create_challenge_v1( | ||
| hmac_key=conf.get_hmac_secret(), | ||
| max_number=conf.get_max_number(), | ||
| algorithm=conf.get_algorithm(), | ||
| expires=expires, |
Contributor
There was a problem hiding this comment.
issue (bug_risk): create_challenge does not guard against missing HMAC secret, which could surface as a runtime error.
This assumes conf.get_hmac_secret() always returns a valid key. If the contrib app is installed but UNFOLD_FOBI_ALTCHA_HMAC_SECRET is missing or ALTCHA isn’t actually enabled, altcha.create_challenge_v1 will likely raise and surface as a 500 from this endpoint. Consider either short‑circuiting when ALTCHA is disabled or raising an explicit configuration error so misconfiguration is handled predictably.
Comment on lines
+47
to
+51
| cache = caches[conf.get_cache_alias()] | ||
| cache_key = f"altcha:replay:{payload_b64[:64]}" | ||
| if cache.get(cache_key): | ||
| return False, "ALTCHA payload already used." | ||
| cache.set(cache_key, 1, conf.get_challenge_expiry() * 2) |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Accessing the cache alias directly can raise if the alias is misconfigured, breaking verification.
If UNFOLD_FOBI_ALTCHA_CACHE_ALIAS is misconfigured, caches[alias] will raise and break all ALTCHA verification. Consider wrapping the lookup in a try/except InvalidCacheBackendError (or a broader Exception) and, on failure, skip replay protection but log the misconfiguration so form submissions still work and the issue is visible.
Comment on lines
+250
to
+251
| 3. Frontend solves the challenge client-side (e.g. using the `altcha` web | ||
| component) and includes the base64 payload as `"altcha"` in the PUT body. |
Contributor
There was a problem hiding this comment.
suggestion: Align the documented payload field name with the configurable UNFOLD_FOBI_ALTCHA_FIELD_NAME setting.
Step 3 hardcodes the field name as "altcha", while earlier you introduce UNFOLD_FOBI_ALTCHA_FIELD_NAME (default "altcha"). This could confuse users who override the setting. Please reference the setting here and note that "altcha" is the default (e.g., “includes the base64 payload in the field specified by UNFOLD_FOBI_ALTCHA_FIELD_NAME (default: "altcha")”).
Suggested change
| 3. Frontend solves the challenge client-side (e.g. using the `altcha` web | |
| component) and includes the base64 payload as `"altcha"` in the PUT body. | |
| 3. Frontend solves the challenge client-side (e.g. using the `altcha` web | |
| component) and includes the base64 payload in the field specified by | |
| `UNFOLD_FOBI_ALTCHA_FIELD_NAME` (default: `"altcha"`) in the PUT body. |
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
1 participant
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.
Summary by Sourcery
Add optional ALTCHA proof-of-work protection for public DRF form submissions and expose it via a dedicated contrib app and API field.
New Features:
unfold_fobi.contrib.altchaapp providing ALTCHA challenge generation, verification, and settings-backed configuration.Enhancements:
Tests: