Skip to content

Commit 2ecf83f

Browse files
authored
feat: redirect to original page after login (#18545)
Leaving the insecure cookie alive when not logged in leads to confusing client-side include behavior, as the last HTTP request from CSI is the current user indicator, which changes the value of `request.url`. Resolves #12986 Signed-off-by: Mike Fiedler <miketheman@gmail.com>
1 parent 52b9fd0 commit 2ecf83f

File tree

4 files changed

+15
-4
lines changed

4 files changed

+15
-4
lines changed

tests/unit/test_sessions.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -564,7 +564,8 @@ def test_invalidated_deletes_save_non_secure(self, monkeypatch, pyramid_request)
564564
response = pretend.stub(
565565
set_cookie=pretend.call_recorder(
566566
lambda cookie, data, httponly=False, secure=True, samesite=b"none": None
567-
)
567+
),
568+
delete_cookie=pretend.call_recorder(lambda cookie: None),
568569
)
569570
session_factory._process_response(pyramid_request, response)
570571

@@ -594,6 +595,9 @@ def test_invalidated_deletes_save_non_secure(self, monkeypatch, pyramid_request)
594595
samesite=b"lax",
595596
)
596597
]
598+
assert response.delete_cookie.calls == [
599+
pretend.call("user_id__insecure"),
600+
]
597601

598602

599603
class TestSessionView:

warehouse/sessions.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import warehouse.utils.otp as otp
1717
import warehouse.utils.webauthn as webauthn
1818

19+
from warehouse.accounts.views import USER_ID_INSECURE_COOKIE
1920
from warehouse.cache.http import add_vary
2021
from warehouse.utils import crypto
2122
from warehouse.utils.msgpack import object_encode
@@ -305,7 +306,7 @@ def _process_response(self, request, response):
305306
# > Expires: Session
306307
# This will allow effectively allow the cookie to live indefinitely,
307308
# as long as the user has interacted with the session _before_ the
308-
# session key expieres in redis.
309+
# session key expires in redis.
309310
# Once the session key has expired in redis, the session will be marked
310311
# as invalid and will not authenticate the account.
311312
response.set_cookie(
@@ -315,6 +316,12 @@ def _process_response(self, request, response):
315316
secure=request.scheme == "https",
316317
samesite=b"lax",
317318
)
319+
# If there's no user associated with the session, remove the insecure cookie
320+
# to prevent JavaScript access to it, which can confuse the UI.
321+
# We cannot access `request.authenticated_userid` at this point in the
322+
# request lifecycle, so we check the session directly.
323+
if not request.session.get("auth.userid"):
324+
response.delete_cookie(USER_ID_INSECURE_COOKIE)
318325

319326

320327
def session_view(view, info):

warehouse/templates/base.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
class="horizontal-menu__link">{% trans %}Sponsors{% endtrans %}</a>
4141
</li>
4242
<li class="horizontal-menu__item">
43-
<a href="{{ request.route_path('accounts.login') }}"
43+
<a href="{{ request.route_path('accounts.login', _query={'next': request.url }) }}"
4444
class="horizontal-menu__link">{% trans %}Log in{% endtrans %}</a>
4545
</li>
4646
<li class="horizontal-menu__item">

warehouse/templates/pages/sitemap.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ <h2 class="no-top-padding">{% trans %}Using PyPI{% endtrans %}</h2>
2222
<h2>{% trans %}Authentication{% endtrans %}</h2>
2323
<ul>
2424
<li>
25-
<a href="{{ request.route_url('accounts.login') }}">{% trans %}Login{% endtrans %}</a>
25+
<a href="{{ request.route_url('accounts.login', _query={'next': request.url}) }}">{% trans %}Login{% endtrans %}</a>
2626
</li>
2727
<li>
2828
<a href="{{ request.route_url('accounts.register') }}">{% trans %}Register{% endtrans %}</a>

0 commit comments

Comments
 (0)