Skip to content

Commit 5eff9fa

Browse files
Copilotkuboschek
andauthored
Fix login redirect to honor next parameter (#201)
* Initial plan * Fix login redirect to respect 'next' parameter with security validation Co-authored-by: kuboschek <1071495+kuboschek@users.noreply.github.com> * Add unit tests for login redirect with next parameter Co-authored-by: kuboschek <1071495+kuboschek@users.noreply.github.com> * Fix tests to use testserver host for URL validation Co-authored-by: kuboschek <1071495+kuboschek@users.noreply.github.com> * fix: users are redirected to the page they expected when logging in * fix: use django redirect function --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: kuboschek <1071495+kuboschek@users.noreply.github.com> Co-authored-by: Leo Kuboschek <leo@kuboschek.me>
1 parent 907a09d commit 5eff9fa

2 files changed

Lines changed: 106 additions & 3 deletions

File tree

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
from __future__ import annotations
2+
3+
from django.test import TestCase, RequestFactory
4+
from django.contrib.auth import get_user_model
5+
from django.conf import settings
6+
from unittest import mock
7+
8+
from custom_auth.views import email_token_login
9+
from custom_auth.utils.auth import generate_login_token
10+
11+
User = get_user_model()
12+
13+
14+
class EmailTokenLoginRedirectTest(TestCase):
15+
"""Test that email token login respects the 'next' parameter"""
16+
17+
fixtures = ["registry/tests/fixtures/integration.json"]
18+
19+
def setUp(self):
20+
self.factory = RequestFactory()
21+
self.user = User.objects.get(username="Mounfem")
22+
23+
def test_redirect_with_next_parameter(self):
24+
"""Test that login redirects to the 'next' URL when provided"""
25+
token = generate_login_token(self.user)
26+
27+
# Create a POST request with a valid token and next parameter
28+
request = self.factory.post('/auth/magic/', {
29+
'token': token,
30+
'next': '/payments/'
31+
})
32+
# Set the host for URL validation (use testserver for test compatibility)
33+
request.META['HTTP_HOST'] = 'testserver'
34+
35+
with mock.patch('custom_auth.views.authenticate', return_value=self.user):
36+
with mock.patch('custom_auth.views.login'):
37+
response = email_token_login(request)
38+
39+
# Should redirect to /payments/
40+
self.assertEqual(response.status_code, 302)
41+
self.assertEqual(response.url, '/payments/')
42+
43+
def test_redirect_without_next_parameter(self):
44+
"""Test that login redirects to LOGIN_REDIRECT_URL when 'next' is not provided"""
45+
token = generate_login_token(self.user)
46+
47+
# Create a POST request with a valid token but no next parameter
48+
request = self.factory.post('/auth/magic/', {
49+
'token': token
50+
})
51+
# Set the host for URL validation (use testserver for test compatibility)
52+
request.META['HTTP_HOST'] = 'testserver'
53+
54+
with mock.patch('custom_auth.views.authenticate', return_value=self.user):
55+
with mock.patch('custom_auth.views.login'):
56+
response = email_token_login(request)
57+
58+
# Should redirect to LOGIN_REDIRECT_URL (default: '/')
59+
self.assertEqual(response.status_code, 302)
60+
self.assertEqual(response.url, settings.LOGIN_REDIRECT_URL)
61+
62+
def test_redirect_with_empty_next_parameter(self):
63+
"""Test that login redirects to LOGIN_REDIRECT_URL when 'next' is empty"""
64+
token = generate_login_token(self.user)
65+
66+
# Create a POST request with a valid token and empty next parameter
67+
request = self.factory.post('/auth/magic/', {
68+
'token': token,
69+
'next': ''
70+
})
71+
# Set the host for URL validation (use testserver for test compatibility)
72+
request.META['HTTP_HOST'] = 'testserver'
73+
74+
with mock.patch('custom_auth.views.authenticate', return_value=self.user):
75+
with mock.patch('custom_auth.views.login'):
76+
response = email_token_login(request)
77+
78+
# Should redirect to LOGIN_REDIRECT_URL (default: '/')
79+
self.assertEqual(response.status_code, 302)
80+
self.assertEqual(response.url, settings.LOGIN_REDIRECT_URL)
81+
82+
def test_redirect_blocks_external_urls(self):
83+
"""Test that external URLs are blocked (security check)"""
84+
token = generate_login_token(self.user)
85+
86+
# Create a POST request with a valid token and external next URL
87+
request = self.factory.post('/auth/magic/', {
88+
'token': token,
89+
'next': 'https://evil.com/phishing'
90+
})
91+
# Set the host for URL validation (use testserver for test compatibility)
92+
request.META['HTTP_HOST'] = 'testserver'
93+
94+
with mock.patch('custom_auth.views.authenticate', return_value=self.user):
95+
with mock.patch('custom_auth.views.login'):
96+
response = email_token_login(request)
97+
98+
# Should redirect to LOGIN_REDIRECT_URL, not the external URL
99+
self.assertEqual(response.status_code, 302)
100+
self.assertEqual(response.url, settings.LOGIN_REDIRECT_URL)

custom_auth/views.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,10 @@
44
from django.contrib.auth import authenticate, login, views
55
from django.core.exceptions import SuspiciousOperation
66
from django.http import HttpResponse, HttpResponseForbidden, HttpResponseRedirect
7-
from django.shortcuts import render
7+
from django.shortcuts import redirect, render
88
from django.urls import reverse
99
from django.utils.decorators import method_decorator
10+
from django.utils.http import url_has_allowed_host_and_scheme
1011
from django.views.decorators.cache import never_cache
1112
from django.views.decorators.csrf import csrf_protect
1213
from django.views.decorators.debug import sensitive_post_parameters
@@ -46,8 +47,8 @@ def email_token_login(request: HttpRequest) -> HttpResponse:
4647

4748
if res is not None:
4849
login(request, res)
49-
next_url = request.POST.get("next", None)
50-
return HttpResponseRedirect(next_url)
50+
next_url = request.POST.get("next", settings.LOGIN_REDIRECT_URL)
51+
return redirect(next_url)
5152
else:
5253
return render(request, "auth/token_login.html", context={"error": True})
5354

@@ -77,6 +78,8 @@ class ClientIdLoginView(views.LoginView):
7778
def get_context_data(self, **kwargs):
7879
context = super(views.LoginView, self).get_context_data(**kwargs)
7980
context["googlefail"] = self.request.GET.get("error", "") == "googlefail"
81+
context["next"] = self.request.GET.get("next", settings.LOGIN_REDIRECT_URL)
82+
8083
context.update(self.extra_context)
8184

8285
return context

0 commit comments

Comments
 (0)