Merge pull request #234 from lzaoral/safe-krb5-redirects

auth: make `krb5login` redirects safer

Author: rohanpm

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', (

kobo/hub/views.py

@@ -1,24 +1,22 @@
-# -*- 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
 from django.contrib.auth import REDIRECT_FIELD_NAME, get_user_model
 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
 
+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
@@ -225,17 +223,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,15 +244,19 @@ 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)
-    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):

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
 

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"))