Linting

app.py

@@ -5,17 +5,12 @@
 
 from os import path
 from typing import Dict, Optional
-from re import fullmatch
-from werkzeug.security import safe_join
-from werkzeug.utils import secure_filename
 
 from flask import Flask, g, redirect, request, Response, send_from_directory, url_for
 from flask_session import Session
 from flask_wtf.csrf import CSRFProtect
 from jinja2 import ChoiceLoader, FileSystemLoader
 
-from util import get_validated_static_path
-
 from api import api_bp
 from datarequest.datarequest import datarequest_bp
 from deposit.deposit import deposit_bp
@@ -29,6 +24,7 @@
 from user.user import user_bp
 from vault.vault import vault_bp
 
+from util import get_validated_static_path
 
 app = Flask(__name__, static_folder='assets')
 app.json.sort_keys = False
@@ -127,11 +123,16 @@ def static_loader() -> Optional[Response]:
 
     :returns: Static file
     """
-    result = get_validated_static_path(request.full_path, request.path, app.config.get('YODA_THEME_PATH'), app.config.get('YODA_THEME'))
+    result = get_validated_static_path(
+        request.full_path,
+        request.path,
+        app.config.get('YODA_THEME_PATH'),
+        app.config.get('YODA_THEME')
+    )
     if result is not None:
         static_dir, asset_name = result
         return send_from_directory(static_dir, asset_name)
- 
+
 
 @app.before_request
 def protect_pages() -> Optional[Response]:

unit-tests/test_util.py

@@ -2,21 +2,20 @@
 """Unit tests for portal utility functions.
 """
 
-__copyright__ = 'Copyright (c) 2023, Utrecht University'
-__license__   = 'GPLv3, see LICENSE'
+__copyright__ = "Copyright (c) 2023, Utrecht University"
+__license__   = "GPLv3, see LICENSE"
 
 import sys
 from unittest import TestCase
 from unittest.mock import patch
 
-sys.path.append('..')
+sys.path.append("..")
 
-from util import is_email_in_domains
 from util import get_validated_static_path
+from util import is_email_in_domains
 
 
 class UtilTest(TestCase):
-
     def test_is_email_in_domains(self):
         self.assertEqual(is_email_in_domains("peter", ["uu.nl"]), False)
         self.assertEqual(is_email_in_domains("[email protected]", ["uu.nl"]), True)
@@ -29,65 +28,132 @@ def test_is_email_in_domains(self):
         self.assertEqual(is_email_in_domains("[email protected]", ["*.uu.nl"]), True)
         self.assertEqual(is_email_in_domains("[email protected]", ["*.cs.uu.nl"]), True)
         self.assertEqual(is_email_in_domains("[email protected]", ["*.cs.uu.nl"]), False)
-
-
-    # If path contains theme and uu, path does not exist
-    # if path contains anything else, path does exist
-    def exists_return_value(self, pathname):
-        return not ('theme' in pathname and 'uu' in pathname)
 
+    def exists_return_value(self, pathname):
+        """ Mock path.exists function. True if path does not contain "theme" and "uu" """
+        return not ("theme" in pathname and "uu" in pathname)
 
-    @patch('os.path.exists')
+    @patch("os.path.exists")
     def test_static_loader_valid_path(self, mock_exists):
         mock_exists.side_effect = self.exists_return_value
         # uu theme
-        static_dir, asset_name = get_validated_static_path('/assets/img/logo.svg?wekr', '/assets/img/logo.svg', "/var/www/yoda/themes", "uu")
-        self.assertEqual(static_dir, '/var/www/yoda/static/img')
-        self.assertEqual(asset_name, 'logo.svg')
+        static_dir, asset_name = get_validated_static_path(
+            "/assets/img/logo.svg?wekr",
+            "/assets/img/logo.svg",
+            "/var/www/yoda/themes",
+            "uu",
+        )
+        self.assertEqual(static_dir, "/var/www/yoda/static/img")
+        self.assertEqual(asset_name, "logo.svg")
         # other theme
-        static_dir, asset_name = get_validated_static_path('/assets/img/logo.svg?wekr', '/assets/img/logo.svg', "/var/www/yoda/themes", "wur")
-        self.assertEqual(static_dir, '/var/www/yoda/themes/wur/static/img')
-        self.assertEqual(asset_name, 'logo.svg')
-
+        static_dir, asset_name = get_validated_static_path(
+            "/assets/img/logo.svg?wekr",
+            "/assets/img/logo.svg",
+            "/var/www/yoda/themes",
+            "wur",
+        )
+        self.assertEqual(static_dir, "/var/www/yoda/themes/wur/static/img")
+        self.assertEqual(asset_name, "logo.svg")
 
-    @patch('os.path.exists')
+    @patch("os.path.exists")
     def test_static_loader_invalid_path(self, mock_exists):
         mock_exists.side_effect = self.exists_return_value
         # Too short
-        self.assertIsNone(get_validated_static_path("/?sawerw", "/", "/var/www/yoda/themes", "uu"))
+        self.assertIsNone(
+            get_validated_static_path(
+                "/?sawerw", "/", "/var/www/yoda/themes", "uu"
+            )
+        )
         # Path traversal attack
-        self.assertIsNone(get_validated_static_path("/assets/../../../../etc/passwd?werwrwr", "/assets/../../../../etc/passwd", "/var/www/yoda/themes", "uu"))
+        self.assertIsNone(
+            get_validated_static_path(
+                "/assets/../../../../etc/passwd?werwrwr",
+                "/assets/../../../../etc/passwd",
+                "/var/www/yoda/themes",
+                "uu",
+            )
+        )
         # non-printable characters
         full_path = "/assets/" + chr(13) + "img/logo.svg?werwer"
         path = "/assets/" + chr(13) + "img/logo.svg"
-        self.assertIsNone(get_validated_static_path(full_path, path, "/var/www/yoda/themes", "uu"))
-        self.assertIsNone(get_validated_static_path(full_path, path, "/var/www/yoda/themes", "wur"))
+        self.assertIsNone(
+            get_validated_static_path(
+                full_path, path, "/var/www/yoda/themes", "uu"
+            )
+        )
+        self.assertIsNone(
+            get_validated_static_path(
+                full_path, path, "/var/www/yoda/themes", "wur"
+            )
+        )
         # non-printable characters in asset name
         full_path = "/assets/img/l" + chr(13) + "ogo.svg?werwer"
         path = "/assets/img/l" + chr(13) + "ogo.svg"
-        self.assertIsNone(get_validated_static_path(full_path, path, "/var/www/yoda/themes", "uu"))
-        self.assertIsNone(get_validated_static_path(full_path, path, "/var/www/yoda/themes", "wur"))
+        self.assertIsNone(
+            get_validated_static_path(
+                full_path, path, "/var/www/yoda/themes", "uu"
+            )
+        )
+        self.assertIsNone(
+            get_validated_static_path(
+                full_path, path, "/var/www/yoda/themes", "wur"
+            )
+        )
         # .. in file name
-        self.assertIsNone(get_validated_static_path("/assets/img/lo..go.svg?sklaerw", "/assets/img/lo..go.svg?sklaerw", "/var/www/yoda/themes", "uu"))
+        self.assertIsNone(
+            get_validated_static_path(
+                "/assets/img/lo..go.svg?sklaerw",
+                "/assets/img/lo..go.svg?sklaerw",
+                "/var/www/yoda/themes",
+                "uu",
+            )
+        )
 
-
-    @patch('os.path.exists')
+    @patch("os.path.exists")
     def test_static_loader_module_valid_path(self, mock_exists):
         mock_exists.side_effect = self.exists_return_value
         # uu theme
-        static_dir, asset_name = get_validated_static_path('/group_manager/assets/js/group_manager.js?wekr', '/group_manager/assets/js/group_manager.js', "/var/www/yoda/themes", "uu")
-        self.assertEqual(static_dir, '/var/www/yoda/group_manager/static/group_manager/js')
-        self.assertEqual(asset_name, 'group_manager.js')
+        static_dir, asset_name = get_validated_static_path(
+            "/group_manager/assets/js/group_manager.js?wekr",
+            "/group_manager/assets/js/group_manager.js",
+            "/var/www/yoda/themes",
+            "uu",
+        )
+        self.assertEqual(
+            static_dir, "/var/www/yoda/group_manager/static/group_manager/js"
+        )
+        self.assertEqual(asset_name, "group_manager.js")
         # other theme
-        static_dir, asset_name = get_validated_static_path('/group_manager/assets/lib/select2-bootstrap-5-theme/select2-bootstrap-5-theme.css?wekr', '/group_manager/assets/lib/select2-bootstrap-5-theme/select2-bootstrap-5-theme.css', "/var/www/yoda/themes", "wur")
-        self.assertEqual(static_dir, '/var/www/yoda/themes/wur/group_manager/static/group_manager/lib/select2-bootstrap-5-theme')
-        self.assertEqual(asset_name, 'select2-bootstrap-5-theme.css')
-
+        static_dir, asset_name = get_validated_static_path(
+            "/group_manager/assets/lib/select2-bootstrap-5-theme/select2-bootstrap-5-theme.css?wekr",
+            "/group_manager/assets/lib/select2-bootstrap-5-theme/select2-bootstrap-5-theme.css",
+            "/var/www/yoda/themes",
+            "wur",
+        )
+        self.assertEqual(
+            static_dir,
+            "/var/www/yoda/themes/wur/group_manager/static/group_manager/lib/select2-bootstrap-5-theme",
+        )
+        self.assertEqual(asset_name, "select2-bootstrap-5-theme.css")
 
-    @patch('os.path.exists')
+    @patch("os.path.exists")
     def test_static_loader_module_invalid_path(self, mock_exists):
         mock_exists.side_effect = self.exists_return_value
         # Invalid module name
-        self.assertIsNone(get_validated_static_path("/../assets/../research/static/research/css/research.css?sklwrawe", "/../assets/../research/static/research/css/research.css", "/var/www/yoda/themes", "uu"))
+        self.assertIsNone(
+            get_validated_static_path(
+                "/../assets/../research/static/research/css/research.css?sklwrawe",
+                "/../assets/../research/static/research/css/research.css",
+                "/var/www/yoda/themes",
+                "uu",
+            )
+        )
         # Path traversal attack
-        self.assertIsNone(get_validated_static_path("/group_manager/assets/../../../../../../etc/passwd?werwrwr", "/group_manager/assets/../../../../../../etc/passwd", "/var/www/yoda/themes", "uu"))
+        self.assertIsNone(
+            get_validated_static_path(
+                "/group_manager/assets/../../../../../../etc/passwd?werwrwr",
+                "/group_manager/assets/../../../../../../etc/passwd",
+                "/var/www/yoda/themes",
+                "uu",
+            )
+        )

util.py

@@ -1,24 +1,24 @@
 #!/usr/bin/env python3
 
-__copyright__ = 'Copyright (c) 2021-2022, Utrecht University'
-__license__   = 'GPLv3, see LICENSE'
+__copyright__ = "Copyright (c) 2021-2022, Utrecht University"
+__license__ = "GPLv3, see LICENSE"
 
 import sys
 import traceback
-from typing import List, Optional, Tuple
-
 from os import path
 from re import fullmatch
+from typing import List, Optional, Tuple
+
 from werkzeug.security import safe_join
 from werkzeug.utils import secure_filename
 
 
 def log_error(message: str, print_exception: bool = False) -> None:
     """Writes an error message, and optionally an exception trace to the
-       web server error log.
+    web server error log.
 
-       :param message:         Error message to print
-       :param print_exception: Boolean, whether to print an exception trace
+    :param message:         Error message to print
+    :param print_exception: Boolean, whether to print an exception trace
     """
     print(message, file=sys.stderr)
     if print_exception:
@@ -28,13 +28,13 @@ def log_error(message: str, print_exception: bool = False) -> None:
 def is_email_in_domains(email: str, domain_list: List[str]) -> bool:
     """Determines if an email address is in a list of domains
 
-       :param email: email address of a user
-       :param domain_list: list of domains, which can also include wildcard domains that
-                           match a domain and any of its subdomains
-                           (e.g. "*.uu.nl" matches both [email protected], [email protected])
+    :param email: email address of a user
+    :param domain_list: list of domains, which can also include wildcard domains that
+                        match a domain and any of its subdomains
+                        (e.g. "*.uu.nl" matches both [email protected], [email protected])
 
-       :returns: boolean value that indicates whether this email address is in one of the
-                 domains
+    :returns: boolean value that indicates whether this email address is in one of the
+              domains
     """
     for domain in domain_list:
         if domain.startswith("*."):
@@ -47,49 +47,58 @@ def is_email_in_domains(email: str, domain_list: List[str]) -> bool:
     return False
 
 
-def get_validated_static_path(full_path, request_path, yoda_theme_path, yoda_theme) -> Optional[Tuple[str, str]]:
+def get_validated_static_path(
+    full_path, request_path, yoda_theme_path, yoda_theme
+) -> Optional[Tuple[str, str]]:
     """
     Static files handling - recognisable through '/assets/'
     Confirms that input path is valid and return corresponding static path
-    
+
+    :param full_path: Full path of request
+    :param request_path: Short path of request
+    :param yoda_theme_path: Path to the yoda themes
+    :param yoda_theme: Name of the chosen theme
+
     :returns: Tuple of static directory and filename for correct path, None for incorrect path
     """
     # Only allow printable ascii
-    if fullmatch('[ -~]*', full_path) is not None and '/assets/' in full_path:
+    if fullmatch("[ -~]*", full_path) is not None and "/assets/" in full_path:
         user_static_area = path.join(yoda_theme_path, yoda_theme)
-        parts = full_path.split('/')
-        
+        parts = full_path.split("/")
+
         # Trim empty string and file name from path
         parts = parts[1:-1]
         _, asset_name = path.split(request_path)
         # Make sure asset_name is safe
         if asset_name != secure_filename(asset_name):
             return
 
-        if parts[0] == 'assets':
+        if parts[0] == "assets":
             # Main assets
-            static_dir = safe_join(user_static_area + '/static', *parts[1:])
+            static_dir = safe_join(user_static_area + "/static", *parts[1:])
             if not static_dir:
                 return
             user_static_filename = path.join(static_dir, asset_name)
             if not path.exists(user_static_filename):
-                static_dir = safe_join('/var/www/yoda/static', *parts[1:])
+                static_dir = safe_join("/var/www/yoda/static", *parts[1:])
         else:
             # Module specific assets
             module = parts[0]
             # Make sure module name is safe
             if module != secure_filename(module):
                 return
 
-            module_static_area = path.join(module, 'static', module)
-            user_static_filename = safe_join(path.join(user_static_area, module_static_area), *parts[2:], asset_name)
+            module_static_area = path.join(module, "static", module)
+            user_static_filename = safe_join(
+                path.join(user_static_area, module_static_area), *parts[2:], asset_name
+            )
             if not user_static_filename:
                 return
 
             if path.exists(user_static_filename):
                 static_dir = path.join(user_static_area, module_static_area, *parts[2:])
             else:
-                static_dir = path.join('/var/www/yoda/', module_static_area, *parts[2:])
+                static_dir = path.join("/var/www/yoda/", module_static_area, *parts[2:])
 
         full_path = path.join(static_dir, asset_name)
         # Check that path is correct