From 9caa781c4ee753b9b9d8f97e86c32b8b98621c1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Zaoral?= Date: Mon, 4 Dec 2023 10:31:51 +0100 Subject: [PATCH 1/2] hub/views: remove Python 2 remnants and dead code --- kobo/hub/views.py | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/kobo/hub/views.py b/kobo/hub/views.py index 3ec341f2..2ba0a9f1 100644 --- a/kobo/hub/views.py +++ b/kobo/hub/views.py @@ -1,13 +1,6 @@ -# -*- coding: utf-8 -*- - +import json import mimetypes import os -import six - -try: - import json -except ImportError: - import simplejson as json import django.contrib.auth.views from django.conf import settings @@ -15,7 +8,6 @@ from django.core.exceptions import ImproperlyConfigured from django.http import HttpResponse, HttpResponseNotFound, StreamingHttpResponse, HttpResponseForbidden from django.shortcuts import render, get_object_or_404 -from django.template import RequestContext from django.urls import reverse from django.views.generic import RedirectView @@ -225,17 +217,16 @@ def task_log_json(request, id, log_name): # check back soon next_poll = LOG_WATCHER_INTERVAL - if six.PY3: - content = str(content, encoding="utf-8", errors="replace") result = { "new_offset": offset + len(content), "task_finished": task.is_finished() and 1 or 0, "next_poll": next_poll, - "content": content, + "content": str(content, encoding="utf-8", errors="replace"), } - return HttpResponse(json.dumps(result), content_type="application/json") + return HttpResponse(json.dumps(result).encode(), + content_type="application/json") class LoginView(django.contrib.auth.views.LoginView): @@ -247,7 +238,6 @@ class LogoutView(django.contrib.auth.views.LogoutView): def krb5login(request, redirect_field_name=REDIRECT_FIELD_NAME): - #middleware = 'django.contrib.auth.middleware.RemoteUserMiddleware' middleware = 'kobo.django.auth.middleware.LimitedRemoteUserMiddleware' if middleware not in settings.MIDDLEWARE: raise ImproperlyConfigured("krb5login view requires '%s' middleware installed" % middleware) From 92c80997adcccd104ad11c77faff9e42facb5f44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Zaoral?= Date: Mon, 4 Dec 2023 16:28:31 +0100 Subject: [PATCH 2/2] 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 --- kobo/admin/templates/hub/settings.py.template | 4 ++++ kobo/hub/views.py | 21 ++++++++++++++----- tests/settings.py | 4 ++++ tests/test_views.py | 2 +- 4 files changed, 25 insertions(+), 6 deletions(-) diff --git a/kobo/admin/templates/hub/settings.py.template b/kobo/admin/templates/hub/settings.py.template index 7ab70757..379723a6 100644 --- a/kobo/admin/templates/hub/settings.py.template +++ b/kobo/admin/templates/hub/settings.py.template @@ -84,6 +84,10 @@ SECRET_KEY = '' # Ensure db transactions ATOMIC_REQUESTS = True +# Default redirects for unsafe login redirections +LOGIN_REDIRECT_URL = 'home/index' +LOGOUT_REDIRECT_URL = 'home/index' + # List of callables that know how to import templates from various sources. TEMPLATE_LOADERS = ( ('django.template.loaders.cached.Loader', ( diff --git a/kobo/hub/views.py b/kobo/hub/views.py index 2ba0a9f1..3442e9a5 100644 --- a/kobo/hub/views.py +++ b/kobo/hub/views.py @@ -11,6 +11,12 @@ from django.urls import reverse from django.views.generic import RedirectView +from kobo.django.django_version import django_version_ge +if django_version_ge('3.2.0'): + from django.utils.http import url_has_allowed_host_and_scheme +else: + from django.utils.http import is_safe_url as url_has_allowed_host_and_scheme + from kobo.hub.models import Arch, Channel, Task from kobo.hub.forms import TaskSearchForm from kobo.django.views.generic import ExtraDetailView, SearchView, UsersAclMixin @@ -241,11 +247,16 @@ def krb5login(request, redirect_field_name=REDIRECT_FIELD_NAME): middleware = 'kobo.django.auth.middleware.LimitedRemoteUserMiddleware' if middleware not in settings.MIDDLEWARE: raise ImproperlyConfigured("krb5login view requires '%s' middleware installed" % middleware) - redirect_to = request.POST.get(redirect_field_name, "") - if not redirect_to: - redirect_to = request.GET.get(redirect_field_name, "") - if not redirect_to: - redirect_to = reverse("home/index") + + redirect_to = request.POST.get(redirect_field_name, request.GET.get(redirect_field_name)) + url_is_safe = url_has_allowed_host_and_scheme( + url=redirect_to, + allowed_hosts=request.get_host(), + require_https=request.is_secure(), + ) + if not url_is_safe: + redirect_to = reverse(settings.LOGIN_REDIRECT_URL) + return RedirectView.as_view(url=redirect_to, permanent=True)(request) def oidclogin(request): diff --git a/tests/settings.py b/tests/settings.py index 16765e40..9828d4f9 100644 --- a/tests/settings.py +++ b/tests/settings.py @@ -20,6 +20,10 @@ UPLOAD_DIR = UPLOAD_DIR_OBJ.name WORKER_DIR = WORKER_DIR_OBJ.name +# Default redirects for unsafe login redirections +LOGIN_REDIRECT_URL = 'home/index' +LOGOUT_REDIRECT_URL = 'home/index' + # The middleware and apps below are the bare minimum required # to let kobo.hub load successfully diff --git a/tests/test_views.py b/tests/test_views.py index d2b4c3d4..9ec14272 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -58,7 +58,7 @@ def test_krb5login_redirect_to(self): def test_logout(self): response = self.client.get('/auth/logout/') - self.assertEqual(response.status_code, 200) + self.assertIn(response.status_code, [200, 302]) self.client.post('/auth/login/', self.credentials) self.client.logout() self.assertFalse(self.client.session.get("_auth_user_id"))