Skip to content

refactor: apply 2FA to any non-exempt routes #15688

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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