Skip to content

Commit 6e4f4dd

Browse files
authored
Add support for redirecting to /chatbot after login via ?next param (#1713)
* Add support for redirecting to /chatbot after login via ?next param * Update redirect URL for console view test The test now expects the redirect to include the 'next' parameter in the login URL, reflecting changes in authentication flow. * Secure redirect for authenticated users in LoginView LoginView now validates the 'next' parameter using Django's url_has_allowed_host_and_scheme to prevent open redirect vulnerabilities. Added a test to ensure unsafe 'next' URLs redirect to root instead of external sites.
1 parent 2624b98 commit 6e4f4dd

File tree

4 files changed

+30
-3
lines changed

4 files changed

+30
-3
lines changed

ansible_ai_connect/main/base_views.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ def dispatch(self, request, *args, **kwargs):
5858
return handler(request, *args, **kwargs)
5959

6060
except exceptions.NotAuthenticated:
61-
return HttpResponseRedirect("/login")
61+
return HttpResponseRedirect(f"/login?next={request.path}")
6262

6363
except Exception as exc:
6464
# Map _internal_ errors to a generic PermissionDenied error

ansible_ai_connect/main/tests/test_console_views.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ def test_authentication_error(self, *args):
3838
# self.client.force_authenticate(user=self.user)
3939
r = self.client.get(reverse("console"))
4040
self.assertEqual(r.status_code, HTTPStatus.FOUND)
41-
self.assertEqual(r.url, "/login")
41+
self.assertEqual(r.url, "/login?next=/console/")
4242

4343
@patch.object(IsOrganisationAdministrator, "has_permission", return_value=True)
4444
@patch.object(IsOrganisationLightspeedSubscriber, "has_permission", return_value=True)

ansible_ai_connect/main/tests/test_views.py

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,26 @@ class MockUser:
130130
self.assertTrue(isinstance(response, HttpResponseRedirect))
131131
self.assertEqual(response.url, "/")
132132

133+
def test_no_login_for_auth_user_with_next(self):
134+
class MockUser:
135+
is_authenticated = True
136+
137+
request = RequestFactory().get("/login?next=/chatbot", SERVER_NAME="testserver")
138+
request.user = MockUser()
139+
response = LoginView.as_view()(request)
140+
self.assertTrue(isinstance(response, HttpResponseRedirect))
141+
self.assertEqual(response.url, "/chatbot")
142+
143+
def test_no_login_for_auth_user_with_unsafe_next(self):
144+
class MockUser:
145+
is_authenticated = True
146+
147+
request = RequestFactory().get("/login?next=http://malicious-site.com")
148+
request.user = MockUser()
149+
response = LoginView.as_view()(request)
150+
self.assertTrue(isinstance(response, HttpResponseRedirect))
151+
self.assertEqual(response.url, "/")
152+
133153

134154
@override_settings(ALLOW_METRICS_FOR_ANONYMOUS_USERS=False)
135155
class TestMetricsView(APITransactionTestCase):
@@ -343,7 +363,7 @@ def test_chatbot_link_with_rh_user(self):
343363
def test_chatbot_view_with_anonymous_user(self):
344364
r = self.client.get(reverse("chatbot"))
345365
self.assertEqual(r.status_code, HTTPStatus.FOUND)
346-
self.assertEqual(r.url, "/login")
366+
self.assertEqual(r.url, "/login?next=/chatbot/")
347367

348368
def test_chatbot_view_with_non_rh_user(self):
349369
self.client.force_login(user=self.non_rh_user)

ansible_ai_connect/main/views.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
from django.contrib.auth import views as auth_views
2222
from django.contrib.auth.models import AnonymousUser
2323
from django.http import HttpResponseRedirect
24+
from django.utils.http import url_has_allowed_host_and_scheme
2425
from django_prometheus.exports import ExportToDjangoView
2526
from oauth2_provider.contrib.rest_framework import IsAuthenticatedOrTokenHasScope
2627
from rest_framework.exceptions import PermissionDenied
@@ -50,6 +51,7 @@
5051
class LoginView(auth_views.LoginView):
5152
def get_context_data(self, **kwargs):
5253
context = super().get_context_data(**kwargs)
54+
context["next"] = self.request.GET.get("next")
5355
context["deployment_mode"] = settings.DEPLOYMENT_MODE
5456
context["project_name"] = settings.ANSIBLE_AI_PROJECT_NAME
5557
context["aap_api_provider_name"] = settings.AAP_API_PROVIDER_NAME
@@ -58,6 +60,11 @@ def get_context_data(self, **kwargs):
5860

5961
def dispatch(self, request, *args, **kwargs):
6062
if self.request.user.is_authenticated:
63+
next_url = self.request.GET.get("next", "/")
64+
if url_has_allowed_host_and_scheme(
65+
next_url, allowed_hosts={request.get_host()}, require_https=request.is_secure()
66+
):
67+
return HttpResponseRedirect(next_url)
6168
return HttpResponseRedirect("/")
6269
return super().dispatch(request, *args, **kwargs)
6370

0 commit comments

Comments
 (0)