Skip to content

Commit d575d07

Browse files
committed
feat: require 2fa for new users
Hooks into security policy to require all non-account management actions to have a form of 2FA set up. Adds a time-based restriction for new users, which we can remove when we want to enforce for everyone. Resolves pypi#13762 Signed-off-by: Mike Fiedler <[email protected]>
1 parent adf056e commit d575d07

File tree

3 files changed

+130
-4
lines changed

3 files changed

+130
-4
lines changed

tests/unit/accounts/test_security_policy.py

Lines changed: 96 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
# See the License for the specific language governing permissions and
1111
# limitations under the License.
1212

13+
from datetime import datetime
1314

1415
import pretend
1516
import pytest
@@ -516,8 +517,11 @@ def test_acl(self, monkeypatch, policy_class, principals, expected):
516517

517518
request = pretend.stub(
518519
identity=pretend.stub(
519-
__principals__=lambda: principals, has_primary_verified_email=True
520-
)
520+
__principals__=lambda: principals,
521+
has_primary_verified_email=True,
522+
has_two_factor=False,
523+
),
524+
matched_route=pretend.stub(name="random.route"),
521525
)
522526
context = pretend.stub(__acl__=[(Allow, "user:5", "myperm")])
523527

@@ -545,6 +549,7 @@ def test_2fa_owner_requires(
545549
has_primary_verified_email=True,
546550
has_two_factor=has_mfa,
547551
),
552+
matched_route=pretend.stub(name="random.route"),
548553
registry=pretend.stub(
549554
settings={
550555
"warehouse.two_factor_requirement.enabled": True,
@@ -581,6 +586,7 @@ def test_2fa_pypi_mandates_2fa(
581586
has_primary_verified_email=True,
582587
has_two_factor=has_mfa,
583588
),
589+
matched_route=pretend.stub(name="random.route"),
584590
registry=pretend.stub(
585591
settings={
586592
"warehouse.two_factor_requirement.enabled": False,
@@ -617,6 +623,7 @@ def test_2fa_pypi_mandates_2fa_with_warning(
617623
has_primary_verified_email=True,
618624
has_two_factor=has_mfa,
619625
),
626+
matched_route=pretend.stub(name="random.route"),
620627
registry=pretend.stub(
621628
settings={
622629
"warehouse.two_factor_requirement.enabled": False,
@@ -650,11 +657,97 @@ def test_permits_with_unverified_email(self, monkeypatch, policy_class):
650657

651658
request = pretend.stub(
652659
identity=pretend.stub(
653-
__principals__=lambda: ["user:5"], has_primary_verified_email=False
660+
__principals__=lambda: ["user:5"],
661+
has_primary_verified_email=False,
662+
has_two_factor=False,
663+
),
664+
matched_route=pretend.stub(name="manage.projects"),
665+
)
666+
context = pretend.stub(__acl__=[(Allow, "user:5", "myperm")])
667+
668+
policy = policy_class()
669+
assert not policy.permits(request, context, "myperm")
670+
671+
# TODO: remove this test when we remove the conditional
672+
def test_permits_manage_projects_without_2fa_for_older_users(
673+
self, monkeypatch, policy_class
674+
):
675+
monkeypatch.setattr(security_policy, "User", pretend.stub)
676+
677+
request = pretend.stub(
678+
identity=pretend.stub(
679+
__principals__=lambda: ["user:5"],
680+
has_primary_verified_email=True,
681+
has_two_factor=False,
682+
date_joined=datetime(2019, 1, 1),
683+
),
684+
matched_route=pretend.stub(name="manage.projects"),
685+
)
686+
context = pretend.stub(__acl__=[(Allow, "user:5", "myperm")])
687+
688+
policy = policy_class()
689+
assert policy.permits(request, context, "myperm")
690+
691+
def test_permits_manage_projects_with_2fa(self, monkeypatch, policy_class):
692+
monkeypatch.setattr(security_policy, "User", pretend.stub)
693+
694+
request = pretend.stub(
695+
identity=pretend.stub(
696+
__principals__=lambda: ["user:5"],
697+
has_primary_verified_email=True,
698+
has_two_factor=True,
699+
),
700+
matched_route=pretend.stub(name="manage.projects"),
701+
)
702+
context = pretend.stub(__acl__=[(Allow, "user:5", "myperm")])
703+
704+
policy = policy_class()
705+
assert policy.permits(request, context, "myperm")
706+
707+
def test_deny_manage_projects_without_2fa(self, monkeypatch, policy_class):
708+
monkeypatch.setattr(security_policy, "User", pretend.stub)
709+
710+
request = pretend.stub(
711+
identity=pretend.stub(
712+
__principals__=lambda: ["user:5"],
713+
has_primary_verified_email=True,
714+
has_two_factor=False,
715+
date_joined=datetime.now(),
654716
),
655717
matched_route=pretend.stub(name="manage.projects"),
656718
)
657719
context = pretend.stub(__acl__=[(Allow, "user:5", "myperm")])
658720

659721
policy = policy_class()
660722
assert not policy.permits(request, context, "myperm")
723+
724+
@pytest.mark.parametrize(
725+
"matched_route",
726+
[
727+
"manage.account",
728+
"manage.account.recovery-codes",
729+
"manage.account.totp-provision",
730+
"manage.account.two-factor",
731+
"manage.account.webauthn-provision",
732+
"manage.account.webauthn-provision.validate",
733+
],
734+
)
735+
def test_permits_2fa_routes_without_2fa(
736+
self, monkeypatch, policy_class, matched_route
737+
):
738+
monkeypatch.setattr(security_policy, "User", pretend.stub)
739+
740+
request = pretend.stub(
741+
identity=pretend.stub(
742+
__principals__=lambda: ["user:5"],
743+
has_primary_verified_email=True,
744+
has_two_factor=False,
745+
date_joined=datetime.now(),
746+
),
747+
matched_route=pretend.stub(name=matched_route),
748+
)
749+
750+
context = pretend.stub(__acl__=[(Allow, "user:5", "myperm")])
751+
752+
policy = policy_class()
753+
assert policy.permits(request, context, "myperm")

warehouse/accounts/security_policy.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,4 +309,33 @@ def _check_for_mfa(request, context) -> WarehouseDenied | None:
309309
queue="warning",
310310
)
311311

312+
# Regardless of TwoFactorRequireable, if we're in the manage namespace, we'll
313+
# check if the user has 2FA enabled, and if they don't we'll deny them.
314+
315+
# Management routes that don't require 2FA, mostly to set up 2FA.
316+
_exempt_routes = [
317+
"manage.account.recovery-codes",
318+
"manage.account.totp-provision",
319+
"manage.account.two-factor",
320+
"manage.account.webauthn-provision",
321+
]
322+
323+
if (
324+
request.matched_route.name.startswith("manage")
325+
and request.matched_route.name != "manage.account"
326+
and not any(
327+
request.matched_route.name.startswith(route) for route in _exempt_routes
328+
)
329+
and not request.identity.has_two_factor
330+
) and (
331+
# Start enforcement from 2023-08-01, but we should remove this check
332+
# at the end of 2023.
333+
request.identity.date_joined
334+
and request.identity.date_joined > datetime.datetime(2023, 8, 1)
335+
):
336+
return WarehouseDenied(
337+
"You must enable two factor authentication to manage other settings",
338+
reason="manage_2fa_required",
339+
)
340+
312341
return None

warehouse/views.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,11 @@ def forbidden(exc, request):
152152

153153
# If the forbidden error is because the user doesn't have 2FA enabled, we'll
154154
# redirect them to the 2FA page
155-
if exc.result.reason in {"owners_require_2fa", "pypi_mandates_2fa"}:
155+
if exc.result.reason in {
156+
"owners_require_2fa",
157+
"pypi_mandates_2fa",
158+
"manage_2fa_required",
159+
}:
156160
request.session.flash(
157161
request._(
158162
"Two-factor authentication must be enabled on your account to "

0 commit comments

Comments
 (0)