Conversation
- Added tests to ensure TOTP secrets are generated using a cryptographically secure random number generator. - Enhanced TOTP verification to enforce single-use codes as per RFC 6238. - Updated the Remember My Browser functionality to bind cookies to all enrolled factors, not just TOTP. - Improved rate limiting to prevent concurrent attempts from overwriting each other's increments. - Secured the Django admin interface by preventing direct editing of Authenticator instances. - Updated documentation to clarify the behavior of MFA_SECRET_ENCRYPTION_KEYS and the handling of TOTP secrets. - Bumped version to 4.0.1 to reflect these changes.
There was a problem hiding this comment.
Pull request overview
This PR bumps django-mfa to 4.0.1 and delivers a set of security hardening changes that affect observable authentication behavior, including replay protection, race-free state updates, admin lockdown, and clearer security documentation.
Changes:
- Introduces an optimistic compare-and-set helper (
django_mfa.atomic.update_data) and uses it to make recovery-code spending, TOTP redemption, and WebAuthn sign-count updates concurrency-safe. - Strengthens TOTP by generating 160-bit secrets via
secretsand enforcing single-use redemption across the validity window. - Hardens operational surfaces (rate-limit counter atomicity, remember-my-browser salt binding, Authenticator admin read-only) and updates docs/tests/CI + release automation for 4.0.1.
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Bumps locked package version to 4.0.1. |
| pyproject.toml | Bumps project version to 4.0.1. |
| docs/upgrading.md | Adds 4.0.1 security-fix upgrade notes and behavioral changes. |
| docs/settings.md | Updates settings docs for remember-my-browser binding and corrects MFA_SECRET_ENCRYPTION_KEYS description. |
| docs/security.md | Expands security guidance (fixed window, atomic cache incr requirement, TOTP properties, admin hardening, signing-not-encryption warning). |
| django_mfa/views/verify.py | Reworks remember-my-browser cookie salt to bind to enrolled factors; avoids writing unusable cookies. |
| django_mfa/totp.py | Adds match() to return the matched counter and refactors verify() to use it. |
| django_mfa/ratelimit.py | Makes failure counting atomic via add+incr approach and documents fixed-window semantics. |
| django_mfa/atomic.py | Adds CAS-style helper to update Authenticator.data safely under concurrency. |
| django_mfa/admin.py | Locks down Authenticator admin: read-only, excludes data, disables add/change, keeps delete. |
| django_mfa/adapters/webauthn.py | Moves sign-count clone detection + update into an atomic CAS update. |
| django_mfa/adapters/totp.py | Uses secrets for stronger secrets; enforces single-use TOTP via stored counter and CAS update. |
| django_mfa/adapters/recovery_codes.py | Makes recovery-code consumption race-free via CAS update. |
| django_mfa/tests/test_views.py | Updates remember-my-browser tests and adds factor-binding behavior tests. |
| django_mfa/tests/test_ratelimit.py | Adds deterministic interleaving tests to prove atomicity under concurrency. |
| django_mfa/tests/test_atomic.py | New tests for CAS helper correctness and retry behavior. |
| django_mfa/tests/test_admin.py | New tests ensuring admin does not expose or allow modifying Authenticator.data. |
| django_mfa/tests/test_adapter_webauthn.py | Adds concurrency regression test for sign-count non-regression. |
| django_mfa/tests/test_adapter_totp.py | Adds tests for secret entropy/length/padding and TOTP replay rejection. |
| django_mfa/tests/test_adapter_recovery.py | Adds concurrency regression tests for recovery-code spending. |
| django_mfa/tests/support/admin_urls.py | Adds admin URLs for HTTP-level admin tests without changing global test routing. |
| .github/workflows/tag-release.yml | Updates Actions dependencies (checkout/setup-uv). |
| .github/workflows/publish.yml | Updates Actions dependencies; explicitly disables uv caching for release safety. |
| .github/workflows/ci.yml | Updates Actions dependencies across CI jobs. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+162
to
+165
| The setting is deprecated and kept only so values written by older versions keep | ||
| reading. Do not enable it expecting encryption at rest. If you need that, use your | ||
| database's own encryption, or a column-encryption library, and treat the TOTP secret | ||
| column as sensitive regardless. |
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.
No description provided.