From d90c67e978295394ff8fb0e829929a1ee8429149 Mon Sep 17 00:00:00 2001
From: Sietse Snel <s.t.snel@uu.nl>
Date: Mon, 12 Aug 2024 08:50:12 +0200
Subject: [PATCH] YDA-5890: restrict redirects on login

Permit only redirects to relative URLs. Check both at time the URL
is passed and when the redirect is performed to make this more robust
against future changes in login logic.
---
 unit-tests/test_util.py |  9 +++++++++
 user/user.py            |  6 +++---
 util.py                 | 13 +++++++++++++
 3 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/unit-tests/test_util.py b/unit-tests/test_util.py
index ee645ddc..d9901b7e 100644
--- a/unit-tests/test_util.py
+++ b/unit-tests/test_util.py
@@ -14,6 +14,7 @@
     get_theme_directories,
     get_validated_static_path,
     is_email_in_domains,
+    is_relative_url,
     length_check,
     unicode_secure_filename,
 )
@@ -237,3 +238,11 @@ def test_get_theme_directories_only_files(self) -> None:
             expected_result = ['uu']
             result = get_theme_directories('/test/path')
             assert result == expected_result
+
+    def test_is_relative_url(self) -> None:
+        self.assertEqual(is_relative_url("http://www.uu.nl"), False)
+        self.assertEqual(is_relative_url("https://www.uu.nl"), False)
+        self.assertEqual(is_relative_url("http://username:password@www.uu.nl"), False)
+        self.assertEqual(is_relative_url("https://username:password@www.uu.nl"), False)
+        self.assertEqual(is_relative_url("/foo/bar/bat"), True)
+        self.assertEqual(is_relative_url("foo/bar/bat"), True)
diff --git a/user/user.py b/user/user.py
index f0cd24bc..e57280ac 100644
--- a/user/user.py
+++ b/user/user.py
@@ -32,7 +32,7 @@
 
 import api
 import connman
-from util import is_email_in_domains, log_error
+from util import is_email_in_domains, is_relative_url, log_error
 
 # Blueprint creation
 user_bp = Blueprint('user_bp', __name__,
@@ -60,7 +60,7 @@ def gate() -> Response:
         session['login_username'] = username
 
         redirect_target = request.args.get('redirect_target')
-        if redirect_target is not None:
+        if redirect_target is not None and is_relative_url(redirect_target):
             session['redirect_target'] = redirect_target
 
         # If the username matches the domain set for OIDC
@@ -454,7 +454,7 @@ def authenticated() -> bool:
 
 def original_destination() -> str:
     target = session.get('redirect_target')
-    if target is not None:
+    if target is not None and is_relative_url(target):
         session['redirect_target'] = None
         return target
     else:
diff --git a/util.py b/util.py
index ca2c4f5f..9026a982 100644
--- a/util.py
+++ b/util.py
@@ -5,6 +5,7 @@
 
 import sys
 import traceback
+import urllib
 from os import listdir, name, path
 from re import compile, fullmatch
 from typing import List, Tuple
@@ -183,3 +184,15 @@ def get_theme_directories(theme_path: str) -> List[str]:
         return directories
     except Exception:
         return []
+
+
+def is_relative_url(url: str) -> bool:
+    """
+    Function to check whether whether a URL is relative
+
+    :param url: The URL to check
+
+    :returns: boolean value that indicated whether the URL is relative or not.
+    """
+    parsed_url = urllib.parse.urlparse(url)
+    return parsed_url.scheme == "" and parsed_url.netloc == ""