Skip to content

Commit c9c36ca

Browse files
authored
Merge pull request #91 from MicroPyramid/dev
feat: update security and settings documentation for MFA step-up requ…
2 parents 1aa5997 + 8d120d7 commit c9c36ca

47 files changed

Lines changed: 4103 additions & 50 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CHANGELOG.md

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,88 @@ Versions follow [PEP 440](https://peps.python.org/pep-0440/). The version in
1212
`pyproject.toml` is the only place it is written; the git tag and the GitHub
1313
Release are derived from it (see [docs/contributing.md](docs/contributing.md)).
1414

15+
## 4.2.0
16+
17+
### Added
18+
19+
- **Step-up re-authentication.** `mfa_recent_required`/`MfaRecentRequiredMixin`
20+
(`django_mfa.decorators`) and `MFA_STEPUP_MAX_AGE` (default `300` seconds)
21+
require a *recent* challenge, not merely a verified session, before a
22+
factor can be added, removed or regenerated. Set `MFA_STEPUP_MAX_AGE =
23+
None` to switch it off for django-mfa's own three built-in views (they
24+
never pass their own `max_age`, so they fall back to the setting) and
25+
restore 4.1.0 behaviour there. It does **not** override a host view's own
26+
explicit `@mfa_recent_required(max_age=60)` — an explicit per-view
27+
`max_age` always wins over the global setting, by design. See
28+
[docs/enforcement.md](docs/enforcement.md).
29+
- **Four management commands** for day-to-day operation:
30+
`mfa_status` (read-only — one user's enrolled factors and MFA status),
31+
`mfa_reset` (remove every factor from a locked-out user so they can
32+
re-enroll), `mfa_report` (rollout coverage, and who `MFA_REQUIRED` applies
33+
to but who hasn't enrolled — text or CSV), and `mfa_disable` (grant or
34+
`--revoke` an `MfaExemption` from `MFA_REQUIRED` for one user; does not
35+
touch that user's enrolled factors). See
36+
[docs/operations.md](docs/operations.md).
37+
- **Two importers** for migrating factors in from another package:
38+
`mfa_import_django_otp` (also covers **django-two-factor-auth**, which
39+
stores its TOTP and static tokens as django-otp rows) and
40+
`mfa_import_django_mfa2`. Both support `--dry-run`, `--users`, and
41+
`--overwrite`, are idempotent, and never destroy a working factor unless
42+
`--overwrite` is passed. See
43+
[docs/operations.md](docs/operations.md#migrating-from-another-package) for
44+
what each does and does not migrate — several factor shapes (a
45+
clock-drifted django-otp TOTP device, django-mfa2's wider acceptance
46+
window, `RECOVERY` rows, and any factor type absent from `MFA_FACTORS`) are
47+
reported rather than imported, and are worth reading before a cutover.
48+
- **`MfaExemption`** model and manager (`MfaExemption.objects.active_for()`),
49+
and the **`mfa_exemption_changed`** signal (`user`, `reason`, `expires_at`,
50+
`revoked`, `request`) it fires. Written only by `mfa_disable` — there is no
51+
web UI for granting yourself an exemption from a security requirement.
52+
- **System check `django_mfa.E005`**, rejecting an `MFA_STEPUP_MAX_AGE` that
53+
isn't a positive integer or `None`, the same way `E004` already does for
54+
`MFA_REQUIRED`.
55+
56+
### Changed
57+
58+
- **Behaviour change.** Adding, removing or regenerating a factor now
59+
requires a session that completed a challenge within the last
60+
`MFA_STEPUP_MAX_AGE` seconds (default 300), not merely a verified one.
61+
Set `MFA_STEPUP_MAX_AGE = None` to restore 4.1.0 behaviour for
62+
django-mfa's own views (see the Added entry above for the one case this
63+
doesn't cover). This is the one place this release does not upgrade to
64+
byte-identical behaviour by default — see
65+
[docs/upgrading.md](docs/upgrading.md).
66+
- **`MFA_REMEMBER_MY_BROWSER` now interacts with step-up.** A trusted
67+
browser still skips the challenge at *login* exactly as before — the RMB
68+
cookie check marks the session verified immediately — but that session is
69+
only fresh the moment it's created. `MFA_STEPUP_MAX_AGE` is enforced on
70+
every factor change regardless of how the session became verified, so a
71+
trusted browser that adds, removes or regenerates a factor more than
72+
`MFA_STEPUP_MAX_AGE` seconds after logging in is now challenged for that
73+
action — the RMB cookie is consulted only at login, not re-checked by the
74+
step-up gate. This is a visible change for installs that enabled RMB
75+
specifically to avoid challenges. See
76+
[docs/settings.md](docs/settings.md).
77+
- **Signals may now carry `request=None`.** `mfa_reset` and `mfa_disable`
78+
emit `factor_removed`/`mfa_exemption_changed` from outside any request, so
79+
that an operator action is exactly as auditable as the equivalent
80+
user-initiated one. A receiver that reaches for `request.META`
81+
unconditionally must be updated to tolerate `None` first — see
82+
[docs/api.md](docs/api.md)'s Signals section.
83+
- The verification picker now honours `?next=`, so a single-factor user is
84+
returned to the page they requested after logging in rather than to
85+
`LOGIN_REDIRECT_URL`.
86+
87+
### Upgrading
88+
89+
Run `manage.py migrate django_mfa`. Migration `0009_mfa_exemption` adds the
90+
`MfaExemption` table; it is reversible.
91+
92+
Nothing else is required to keep 4.1.0 behaviour, with one exception: factor
93+
changes are gated on `MFA_STEPUP_MAX_AGE` by default (see above). Set it to
94+
`None` if you need the previous, unconditional behaviour immediately after
95+
upgrading.
96+
1597
## 4.1.0
1698

1799
Three additions, all opt-in. **An install that sets none of the new settings

django_mfa/admin.py

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
from django.contrib import admin
22
from django.contrib.auth import get_user_model
33

4-
from .models import Authenticator
4+
from .models import Authenticator, MfaExemption
55

66

77
@admin.register(Authenticator)
@@ -54,3 +54,34 @@ def has_add_permission(self, request):
5454

5555
def has_change_permission(self, request, obj=None):
5656
return False
57+
58+
59+
@admin.register(MfaExemption)
60+
class MfaExemptionAdmin(admin.ModelAdmin):
61+
"""Inspect-and-revoke only, for the same reasons as AuthenticatorAdmin.
62+
63+
Creating an exemption is a deliberate command-line act with a mandatory
64+
reason (`manage.py mfa_disable`), so add and change are off. Deleting is
65+
allowed and is A revoke path: it is fail-safe -- it re-imposes MFA --
66+
and denying it would push operators to editing the database by hand.
67+
It is NOT an audited one, though: unlike `manage.py mfa_disable
68+
--revoke`, deleting here fires no `mfa_exemption_changed` (Django's
69+
admin has nothing django-mfa listens for on delete) -- see
70+
docs/operations.md's "Auditing operator actions" section. An operator
71+
who needs the deletion to reach an audit receiver should use the
72+
command instead of this page.
73+
"""
74+
75+
list_display = ("user", "reason", "created_at", "expires_at")
76+
list_filter = ("created_at", "expires_at")
77+
fields = ("user", "reason", "created_at", "expires_at")
78+
readonly_fields = fields
79+
80+
def get_search_fields(self, request):
81+
return ("reason", f"user__{get_user_model().USERNAME_FIELD}")
82+
83+
def has_add_permission(self, request):
84+
return False
85+
86+
def has_change_permission(self, request, obj=None):
87+
return False

django_mfa/apps.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,14 @@ def ready(self):
1111
from django_mfa.checks import (
1212
check_fido2_rp_id,
1313
check_mfa_required_predicate,
14+
check_stepup_max_age,
1415
check_webauthn_backend_configured,
1516
)
1617

1718
register(check_fido2_rp_id)
1819
register(check_webauthn_backend_configured)
1920
register(check_mfa_required_predicate)
21+
register(check_stepup_max_age)
2022

2123
from django_mfa import (
2224
adapters, # noqa: F401 (registers built-ins)

django_mfa/checks.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,3 +145,32 @@ def check_mfa_required_predicate(app_configs, **kwargs):
145145
id="django_mfa.E004",
146146
)]
147147
return []
148+
149+
150+
def check_stepup_max_age(app_configs, **kwargs):
151+
"""``MFA_STEPUP_MAX_AGE`` must be a positive integer, or None to disable.
152+
153+
Deliberately NOT gated on ``_webauthn_active()`` -- like E004, this
154+
applies to every install.
155+
156+
Zero is refused rather than accepted because it means "always stale":
157+
every gated view would redirect to mfa:verify, which marks the session
158+
verified and redirects back, which is stale again the instant any
159+
measurable time has passed -- a redirect loop rather than a security
160+
setting. django_mfa.ratelimit.parse() refuses a zero count and a zero
161+
window for the same class of reason. bool is excluded explicitly because
162+
it is a subclass of int, so ``True`` would otherwise be accepted as a
163+
one-second window.
164+
"""
165+
value = mfa_settings.MFA_STEPUP_MAX_AGE
166+
if value is None:
167+
return []
168+
if isinstance(value, bool) or not isinstance(value, int) or value < 1:
169+
return [Error(
170+
f"MFA_STEPUP_MAX_AGE must be a positive integer or None, "
171+
f"got {value!r}.",
172+
hint="It is a number of seconds -- 300 is the default. Set it to "
173+
"None to switch step-up re-authentication off entirely.",
174+
id="django_mfa.E005",
175+
)]
176+
return []

django_mfa/conf.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
"MFA_FIDO2_USER_VERIFICATION": "preferred",
2020
"MFA_FIDO2_ATTESTATION_PREFERENCE": "none",
2121
"MFA_EXEMPT_PATHS": [],
22+
"MFA_STEPUP_MAX_AGE": 300,
2223
"MFA_EMAIL_CODE_LENGTH": 6,
2324
"MFA_EMAIL_CODE_VALIDITY": 300,
2425
"MFA_EMAIL_SUBJECT": None,

django_mfa/decorators.py

Lines changed: 129 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,24 @@
2222
from django.urls import reverse
2323

2424
from django_mfa import session
25+
from django_mfa.conf import settings as mfa_settings
2526

2627

27-
def _enforce(request):
28-
"""Return a redirect response, or None to let the request through."""
28+
def _enforce(request, require_primary_factor=True):
29+
"""Return a redirect response, or None to let the request through.
30+
31+
``require_primary_factor`` gates only the third rung below. It exists
32+
for mfa_recent_required/MfaRecentRequiredMixin, which reuse this
33+
function for the authenticated/pending rungs. By default those callers
34+
pass it True too, so a factorless user is redirected here exactly as
35+
mfa_required does. Their own allow_unenrolled=True escape hatch (for the
36+
built-in enrollment views only) passes False instead, deferring to
37+
_enforce_recent()'s own, more permissive handling of a factorless user
38+
(let them through, since there is nothing for them to re-verify) --
39+
which would otherwise never be reached, since this rung would redirect
40+
first. mfa_required/MfaRequiredMixin never pass this, so their
41+
behaviour is unchanged.
42+
"""
2943
from django_mfa.registry import registry
3044

3145
user = request.user
@@ -34,7 +48,7 @@ def _enforce(request):
3448
if session.is_pending(request):
3549
return redirect_to_login(request.get_full_path(),
3650
resolve_url(reverse("mfa:verify")), "next")
37-
if not registry.has_primary_factor(user):
51+
if require_primary_factor and not registry.has_primary_factor(user):
3852
# has_primary_factor(), not enabled_for(): a user holding only
3953
# recovery codes is not protected, and recovery codes must never be
4054
# somebody's sole second factor. has_primary_factor(), not
@@ -69,3 +83,115 @@ def dispatch(self, request, *args, **kwargs):
6983
if response is not None:
7084
return response
7185
return super().dispatch(request, *args, **kwargs)
86+
87+
88+
#: Methods that are safe to replay after a detour through the verify flow.
89+
#: An unsafe request's body cannot survive the redirect, so those are sent
90+
#: to a landing page instead -- see _enforce_recent.
91+
SAFE_METHODS = frozenset({"GET", "HEAD", "OPTIONS", "TRACE"})
92+
93+
94+
def _enforce_recent(request, max_age, next_url):
95+
"""The step-up rung: a recent challenge, not merely a verified session.
96+
97+
Returns a redirect response, or None to let the request through. Runs
98+
only AFTER _enforce() has passed, so request.user is authenticated and
99+
the session is verified by the time this is reached.
100+
"""
101+
from django_mfa.registry import registry
102+
103+
resolved = (max_age if max_age is not None
104+
else mfa_settings.MFA_STEPUP_MAX_AGE)
105+
if resolved is None:
106+
return None
107+
if not registry.has_primary_factor(request.user):
108+
# Only reachable at all when the caller passed allow_unenrolled=True
109+
# (_enforce() already redirected a factorless user away otherwise).
110+
# Nothing to re-verify, and this is the first-enrollment path.
111+
# Gating it would wall a factorless user out of the only pages that
112+
# could give them a factor -- the same lockout
113+
# signals.stamp_pending_verification guards against by refusing to
114+
# stamp such a user pending.
115+
return None
116+
if session.is_fresh(request, resolved):
117+
return None
118+
if request.method in SAFE_METHODS:
119+
target = request.get_full_path()
120+
else:
121+
# A POST body does not survive a redirect, and manage_factors is
122+
# POST-only (405 on GET), so replaying its URL after verification
123+
# would land the user on that 405. Send them to a page they can act
124+
# from instead; they re-click.
125+
target = next_url or reverse("mfa:security_settings")
126+
return redirect_to_login(target, resolve_url(reverse("mfa:verify")), "next")
127+
128+
129+
def mfa_recent_required(max_age=None, next_url=None, allow_unenrolled=False):
130+
"""Require a *recent* second-factor challenge, not just a verified session.
131+
132+
By default (``allow_unenrolled=False``) this is strictly stronger than
133+
``mfa_required``: it applies every rung ``mfa_required`` does --
134+
including redirecting a factorless user to ``mfa:security_settings`` --
135+
and then, for a user who passes that, the freshness rung on top. This is
136+
what a host project's own sensitive views (e.g. ``transfer_funds`` in
137+
docs/enforcement.md) get.
138+
139+
``allow_unenrolled=True`` switches off the factorless-user redirect and
140+
lets such a user through instead, since they have nothing to re-verify.
141+
This is for the built-in enrollment views only (``enroll_factor``,
142+
``recovery_codes``) -- gating the very pages that let a user acquire a
143+
factor would lock them out permanently. Most callers should not pass
144+
this.
145+
146+
Both spellings work -- bare, or called::
147+
148+
@mfa_recent_required
149+
@mfa_recent_required(max_age=60)
150+
@mfa_recent_required(allow_unenrolled=True)
151+
152+
``max_age=None`` means MFA_STEPUP_MAX_AGE, resolved per request so
153+
override_settings() is honoured.
154+
"""
155+
if callable(max_age):
156+
return mfa_recent_required()(max_age)
157+
158+
def decorator(view_func):
159+
@wraps(view_func)
160+
def _wrapped(request, *args, **kwargs):
161+
response = _enforce(
162+
request,
163+
require_primary_factor=not allow_unenrolled) or _enforce_recent(
164+
request, max_age, next_url)
165+
if response is not None:
166+
return response
167+
return view_func(request, *args, **kwargs)
168+
169+
return _wrapped
170+
171+
return decorator
172+
173+
174+
class MfaRecentRequiredMixin:
175+
"""Class-based-view form of ``mfa_recent_required``.
176+
177+
Mix in FIRST, so dispatch() runs before the view's own.
178+
179+
``mfa_allow_unenrolled = False`` by default -- a factorless user is
180+
redirected to ``mfa:security_settings`` exactly as ``MfaRequiredMixin``
181+
would. Set it ``True`` only for the built-in enrollment views, where a
182+
factorless user must be let through instead. See
183+
``mfa_recent_required``'s docstring for the full rationale.
184+
"""
185+
186+
mfa_stepup_max_age = None
187+
mfa_stepup_next_url = None
188+
mfa_allow_unenrolled = False
189+
190+
def dispatch(self, request, *args, **kwargs):
191+
response = _enforce(
192+
request,
193+
require_primary_factor=not self.mfa_allow_unenrolled) or _enforce_recent(
194+
request, self.mfa_stepup_max_age, self.mfa_stepup_next_url)
195+
if response is not None:
196+
return response
197+
return super().dispatch(request, *args, **kwargs)

django_mfa/events.py

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,13 @@
1212
re-export is one-way (signals imports events, never the reverse) and safe.
1313
1414
`sender` is uniformly the Adapter *class* for the factor involved, so a
15-
receiver can narrow with `sender=TOTPAdapter`. `request` is always supplied:
16-
every emission point sits inside a request path.
15+
receiver can narrow with `sender=TOTPAdapter`. `request` is the request the
16+
event happened in, or **None** when it did not happen in one: the management
17+
commands (`mfa_reset`, `mfa_disable`) emit these too, because a factor
18+
removed by an operator is exactly the event an audit receiver most needs to
19+
see. Receivers must therefore tolerate `request=None` rather than reaching
20+
straight for `request.META`. Every emission uses send_robust(), so a receiver
21+
that does not is contained rather than breaking the action.
1722
"""
1823

1924
import django.dispatch
@@ -25,7 +30,9 @@
2530

2631
#: A user removed a factor. kwargs: user, factor_type, name, request.
2732
#: The row is already gone by the time this fires, so its type and name are
28-
#: passed by value rather than as an instance.
33+
#: passed by value rather than as an instance. `request` is None when a
34+
#: management command (mfa_reset, mfa_disable) is the one removing it --
35+
#: there is no request to pass.
2936
factor_removed = django.dispatch.Signal()
3037

3138
#: A second-factor challenge succeeded. kwargs: user, method, request.
@@ -42,3 +49,12 @@
4249
#: Emitted from the adapter rather than the view: only the adapter knows how
4350
#: many codes are left.
4451
recovery_code_used = django.dispatch.Signal()
52+
53+
#: An operator granted or revoked an MFA_REQUIRED exemption.
54+
#: kwargs: user, reason, expires_at, revoked, request.
55+
#: sender is the MfaExemption model class -- the other five signals use the
56+
#: Adapter subclass, and there is no adapter behind this one. Exempting
57+
#: somebody from a security requirement is exactly as audit-worthy as
58+
#: changing their factors, so it gets the same hook. `request` is None when
59+
#: it comes from `manage.py mfa_disable`, which today is the only writer.
60+
mfa_exemption_changed = django.dispatch.Signal()

django_mfa/management/__init__.py

Whitespace-only changes.

django_mfa/management/commands/__init__.py

Whitespace-only changes.

0 commit comments

Comments
 (0)