Skip to content

Commit

Permalink
refactor: apply 2FA to any non-exempt routes (#15688)
Browse files Browse the repository at this point in the history
  • Loading branch information
miketheman committed Apr 1, 2024
1 parent 3f9e0e2 commit 4caaf35
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 33 deletions.
9 changes: 1 addition & 8 deletions tests/unit/accounts/test_security_policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from datetime import datetime

import pretend
import pytest

Expand Down Expand Up @@ -527,8 +525,7 @@ def test_acl(self, monkeypatch, policy_class, principals, expected):
identity=pretend.stub(
__principals__=lambda: principals,
has_primary_verified_email=True,
has_two_factor=False,
date_joined=datetime(2022, 8, 1),
has_two_factor=True,
),
matched_route=pretend.stub(name="random.route"),
)
Expand Down Expand Up @@ -561,7 +558,6 @@ def test_permits_manage_projects_with_2fa(self, monkeypatch, policy_class):
__principals__=lambda: ["user:5"],
has_primary_verified_email=True,
has_two_factor=True,
date_joined=datetime(2022, 8, 1),
),
matched_route=pretend.stub(name="manage.projects"),
)
Expand All @@ -579,7 +575,6 @@ def test_deny_manage_projects_without_2fa(self, monkeypatch, policy_class):
__principals__=lambda: ["user:5"],
has_primary_verified_email=True,
has_two_factor=False,
date_joined=datetime(2023, 8, 9),
),
matched_route=pretend.stub(name="manage.projects"),
)
Expand All @@ -597,7 +592,6 @@ def test_deny_forklift_file_upload_without_2fa(self, monkeypatch, policy_class):
__principals__=lambda: ["user:5"],
has_primary_verified_email=True,
has_two_factor=False,
date_joined=datetime(2023, 8, 9),
),
matched_route=pretend.stub(name="forklift.legacy.file_upload"),
)
Expand Down Expand Up @@ -627,7 +621,6 @@ def test_permits_2fa_routes_without_2fa(
__principals__=lambda: ["user:5"],
has_primary_verified_email=True,
has_two_factor=False,
date_joined=datetime.now(),
),
matched_route=pretend.stub(name=matched_route),
)
Expand Down
41 changes: 18 additions & 23 deletions warehouse/accounts/security_policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,16 @@ def _check_for_mfa(request, context) -> WarehouseDenied | None:
# at this point, and we only a User in these policies.
assert isinstance(request.identity, User)

# If we're in the manage namespace or file uploads, we'll check if the user
# has 2FA enabled, and if they don't we'll deny them.
if request.identity.has_two_factor:
# We're good to go!
return None

# Return a different message for upload endpoint first.
if request.matched_route.name == "forklift.legacy.file_upload":
return WarehouseDenied(
"You must enable two factor authentication to upload",
reason="upload_2fa_required",
)

# Management routes that don't require 2FA, mostly to set up 2FA.
_exempt_routes = [
Expand All @@ -218,26 +226,13 @@ def _check_for_mfa(request, context) -> WarehouseDenied | None:
"accounts.verify-email",
]

if (
request.matched_route.name.startswith("manage")
and request.matched_route.name != "manage.account"
and not any(
request.matched_route.name.startswith(route) for route in _exempt_routes
)
and not request.identity.has_two_factor
):
return WarehouseDenied(
"You must enable two factor authentication to manage other settings",
reason="manage_2fa_required",
)

if (
request.matched_route.name == "forklift.legacy.file_upload"
and not request.identity.has_two_factor
if request.matched_route.name == "manage.account" or any(
request.matched_route.name.startswith(route) for route in _exempt_routes
):
return WarehouseDenied(
"You must enable two factor authentication to upload",
reason="upload_2fa_required",
)
return None

return None
# No exemptions matched, 2FA is required.
return WarehouseDenied(
"You must enable two factor authentication.",
reason="manage_2fa_required",
)
2 changes: 1 addition & 1 deletion warehouse/locale/messages.pot
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,7 @@ msgstr ""
msgid "Provide an Inspector link to specific lines of code."
msgstr ""

#: warehouse/packaging/views.py:213
#: warehouse/packaging/views.py:212
msgid "Your report has been recorded. Thank you for your help."
msgstr ""

Expand Down
1 change: 0 additions & 1 deletion warehouse/packaging/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,6 @@ def includes_submit_malware_observation(project, request):
renderer="packaging/submit-malware-observation.html",
require_csrf=True,
require_methods=False,
require_reauth=True,
route_name="packaging.project.submit_malware_observation",
uses_session=True,
)
Expand Down

0 comments on commit 4caaf35

Please sign in to comment.