Skip to content

Commit 92c8099

Browse files
committed
hub: make redirecting in krb5login safer
Originally, the krb5login page would allow redirects to any URLs, e.g. to Google using http://$HOSTNAME/auth/krb5login/?next=//www.google.com. This commit implements similar sanitization of REDIRECT_FIELD_NAME like Django does in its LoginView. Related: https://github.com/django/django/blob/8fcb9f1f106cf60d953d88aeaa412cc625c60029/django/contrib/auth/views.py#L43C18-L43C18
1 parent 9caa781 commit 92c8099

4 files changed

Lines changed: 25 additions & 6 deletions

File tree

kobo/admin/templates/hub/settings.py.template

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,10 @@ SECRET_KEY = ''
8484
# Ensure db transactions
8585
ATOMIC_REQUESTS = True
8686

87+
# Default redirects for unsafe login redirections
88+
LOGIN_REDIRECT_URL = 'home/index'
89+
LOGOUT_REDIRECT_URL = 'home/index'
90+
8791
# List of callables that know how to import templates from various sources.
8892
TEMPLATE_LOADERS = (
8993
('django.template.loaders.cached.Loader', (

kobo/hub/views.py

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,12 @@
1111
from django.urls import reverse
1212
from django.views.generic import RedirectView
1313

14+
from kobo.django.django_version import django_version_ge
15+
if django_version_ge('3.2.0'):
16+
from django.utils.http import url_has_allowed_host_and_scheme
17+
else:
18+
from django.utils.http import is_safe_url as url_has_allowed_host_and_scheme
19+
1420
from kobo.hub.models import Arch, Channel, Task
1521
from kobo.hub.forms import TaskSearchForm
1622
from kobo.django.views.generic import ExtraDetailView, SearchView, UsersAclMixin
@@ -241,11 +247,16 @@ def krb5login(request, redirect_field_name=REDIRECT_FIELD_NAME):
241247
middleware = 'kobo.django.auth.middleware.LimitedRemoteUserMiddleware'
242248
if middleware not in settings.MIDDLEWARE:
243249
raise ImproperlyConfigured("krb5login view requires '%s' middleware installed" % middleware)
244-
redirect_to = request.POST.get(redirect_field_name, "")
245-
if not redirect_to:
246-
redirect_to = request.GET.get(redirect_field_name, "")
247-
if not redirect_to:
248-
redirect_to = reverse("home/index")
250+
251+
redirect_to = request.POST.get(redirect_field_name, request.GET.get(redirect_field_name))
252+
url_is_safe = url_has_allowed_host_and_scheme(
253+
url=redirect_to,
254+
allowed_hosts=request.get_host(),
255+
require_https=request.is_secure(),
256+
)
257+
if not url_is_safe:
258+
redirect_to = reverse(settings.LOGIN_REDIRECT_URL)
259+
249260
return RedirectView.as_view(url=redirect_to, permanent=True)(request)
250261

251262
def oidclogin(request):

tests/settings.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@
2020
UPLOAD_DIR = UPLOAD_DIR_OBJ.name
2121
WORKER_DIR = WORKER_DIR_OBJ.name
2222

23+
# Default redirects for unsafe login redirections
24+
LOGIN_REDIRECT_URL = 'home/index'
25+
LOGOUT_REDIRECT_URL = 'home/index'
26+
2327
# The middleware and apps below are the bare minimum required
2428
# to let kobo.hub load successfully
2529

tests/test_views.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ def test_krb5login_redirect_to(self):
5858

5959
def test_logout(self):
6060
response = self.client.get('/auth/logout/')
61-
self.assertEqual(response.status_code, 200)
61+
self.assertIn(response.status_code, [200, 302])
6262
self.client.post('/auth/login/', self.credentials)
6363
self.client.logout()
6464
self.assertFalse(self.client.session.get("_auth_user_id"))

0 commit comments

Comments
 (0)