From 6c208edf323ced07b15ec4bc3879bddb91d398bc Mon Sep 17 00:00:00 2001
From: Radomir Dopieralski <openstack@sheep.art.pl>
Date: Mon, 7 Sep 2020 21:03:36 +0200
Subject: [PATCH] Fix open redirect

Make sure the "next" URL is in the same origin as Horizon before
redirecting to it.

Conflicts:
	horizon/test/unit/workflows/test_workflows.py

Change-Id: I06b2bfc8e3638591615547780c3fa34b0abe19f6
Closes-bug: #1865026
(cherry picked from commit 252467100f75587e18df9c43ed5802ee8f0017fa)
(cherry picked from commit baa370f84332ad41502daea29a551705696f4421)
---
 horizon/test/unit/workflows/test_workflows.py | 26 ++++++++++++++++++-
 horizon/workflows/views.py                    | 12 +++++++--
 releasenotes/notes/bug-cd9099c1ba78d637.yaml  |  7 +++++
 3 files changed, 42 insertions(+), 3 deletions(-)
 create mode 100644 releasenotes/notes/bug-cd9099c1ba78d637.yaml

diff --git a/horizon/test/unit/workflows/test_workflows.py b/horizon/test/unit/workflows/test_workflows.py
index 04b907b576..f3b152490b 100644
--- a/horizon/test/unit/workflows/test_workflows.py
+++ b/horizon/test/unit/workflows/test_workflows.py
@@ -14,8 +14,8 @@
 
 from django import forms
 from django import http
+from django.test.utils import override_settings
 import mock
-
 import six
 
 from horizon import base
@@ -401,3 +401,27 @@ class WorkflowsTests(test.TestCase):
 
         flow = TestWorkflow(req, entry_point="test_action_two")
         self.assertEqual("test_action_two", flow.get_entry_point())
+
+    @override_settings(ALLOWED_HOSTS=['localhost'])
+    def test_redirect_url_safe(self):
+        url = 'http://localhost/test'
+        view = TestWorkflowView()
+        request = self.factory.get("/", data={
+            'next': url,
+        })
+        request.META['SERVER_NAME'] = "localhost"
+        view.request = request
+        context = view.get_context_data()
+        self.assertEqual(url, context['REDIRECT_URL'])
+
+    @override_settings(ALLOWED_HOSTS=['localhost'])
+    def test_redirect_url_unsafe(self):
+        url = 'http://evilcorp/test'
+        view = TestWorkflowView()
+        request = self.factory.get("/", data={
+            'next': url,
+        })
+        request.META['SERVER_NAME'] = "localhost"
+        view.request = request
+        context = view.get_context_data()
+        self.assertIsNone(context['REDIRECT_URL'])
diff --git a/horizon/workflows/views.py b/horizon/workflows/views.py
index 89ed1c0445..35139f33ca 100644
--- a/horizon/workflows/views.py
+++ b/horizon/workflows/views.py
@@ -18,6 +18,7 @@ import json
 from django import forms
 from django import http
 from django import shortcuts
+from django.utils import http as utils_http
 from django.views import generic
 
 import six
@@ -92,8 +93,15 @@ class WorkflowView(hz_views.ModalBackdropMixin, generic.TemplateView):
         workflow = self.get_workflow()
         workflow.verify_integrity()
         context[self.context_object_name] = workflow
-        next = self.request.GET.get(workflow.redirect_param_name)
-        context['REDIRECT_URL'] = next
+
+        redirect_to = self.request.GET.get(workflow.redirect_param_name)
+        # Make sure the requested redirect is safe
+        if redirect_to and not utils_http.is_safe_url(
+                url=redirect_to,
+                allowed_hosts=[self.request.get_host()]):
+            redirect_to = None
+        context['REDIRECT_URL'] = redirect_to
+
         context['layout'] = self.get_layout()
         # For consistency with Workflow class
         context['modal'] = 'modal' in context['layout']
diff --git a/releasenotes/notes/bug-cd9099c1ba78d637.yaml b/releasenotes/notes/bug-cd9099c1ba78d637.yaml
new file mode 100644
index 0000000000..438e3c30e1
--- /dev/null
+++ b/releasenotes/notes/bug-cd9099c1ba78d637.yaml
@@ -0,0 +1,7 @@
+---
+security:
+  - |
+    An open redirect has been fixed, that could redirect users to arbitrary
+    addresses from certain views by specifying a "next" parameter in the URL.
+    Now the redirect will only work if the target URL is in the same domain,
+    and uses the same protocol.