Skip to content

Commit bd8ccde

Browse files
authored
Handle exceptions from json.loads in reauthenticate (#20361)
1 parent 2286929 commit bd8ccde

3 files changed

Lines changed: 60 additions & 17 deletions

File tree

tests/unit/accounts/test_views.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3878,6 +3878,43 @@ def test_reauth_rejects_different_users_password(
38783878
assert pyramid_request.session.record_auth_timestamp.calls == []
38793879
assert pyramid_request.session.record_password_timestamp.calls == []
38803880

3881+
@pytest.mark.parametrize(
3882+
("next_route_matchdict", "next_route_query"),
3883+
[
3884+
("invalid_json", "{}"),
3885+
("{}", "invalid_json"),
3886+
("{'single': 'quotes'}", "{}"),
3887+
("123", "{}"),
3888+
("{}", "123"),
3889+
("[1, 2]", "{}"),
3890+
("{}", "[1, 2]"),
3891+
("true", "{}"),
3892+
("{}", '"string"'),
3893+
],
3894+
)
3895+
def test_reauth_invalid_json_raises_400(
3896+
self, pyramid_request, pyramid_services, next_route_matchdict, next_route_query
3897+
):
3898+
user_service = pretend.stub()
3899+
pyramid_services.register_service(user_service, IUserService, None)
3900+
3901+
pyramid_request.user = pretend.stub(id=pretend.stub(), username=pretend.stub())
3902+
pyramid_request.matched_route = pretend.stub(name=pretend.stub())
3903+
pyramid_request.matchdict = {}
3904+
pyramid_request.GET = pretend.stub(mixed=lambda: {})
3905+
pyramid_request.route_path = pretend.call_recorder(lambda *a, **kw: "/target")
3906+
3907+
form_obj = pretend.stub(
3908+
next_route=pretend.stub(data="/manage/accounts"),
3909+
next_route_matchdict=pretend.stub(data=next_route_matchdict),
3910+
next_route_query=pretend.stub(data=next_route_query),
3911+
validate=lambda: True,
3912+
)
3913+
form_class = pretend.call_recorder(lambda d, **kw: form_obj)
3914+
3915+
with pytest.raises(HTTPBadRequest):
3916+
views.reauthenticate(pyramid_request, _form_class=form_class)
3917+
38813918

38823919
class TestManageAccountPublishingViews:
38833920
def test_initializes(self, metrics):

warehouse/accounts/views.py

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1666,11 +1666,17 @@ def reauthenticate(request, _form_class=ReAuthenticateForm):
16661666
)
16671667

16681668
if form.next_route.data and form.next_route_matchdict.data:
1669-
redirect_to = request.route_path(
1670-
form.next_route.data,
1671-
**json.loads(form.next_route_matchdict.data)
1672-
| {"_query": json.loads(form.next_route_query.data)},
1673-
)
1669+
try:
1670+
matchdict = json.loads(form.next_route_matchdict.data)
1671+
query = json.loads(form.next_route_query.data)
1672+
if not isinstance(matchdict, dict) or not isinstance(query, dict):
1673+
raise HTTPBadRequest
1674+
redirect_to = request.route_path(
1675+
form.next_route.data,
1676+
**matchdict | {"_query": query},
1677+
)
1678+
except json.JSONDecodeError, KeyError, TypeError, ValueError:
1679+
raise HTTPBadRequest
16741680
else:
16751681
redirect_to = request.route_path("manage.projects")
16761682

warehouse/locale/messages.pot

Lines changed: 12 additions & 12 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)