Adjust safeinput to accept complex URLs in redirect_url value

mig/shared/safeinput.py

@@ -1,3 +1,3 @@
 #!/usr/bin/python
 # -*- coding: utf-8 -*-
 #
@@ -42,7 +42,7 @@
 from builtins import range
 from past.builtins import basestring
 
 import re
 import sys
 from email.utils import parseaddr, formataddr
 from html import escape as escape_html
@@ -127,7 +127,7 @@
 # Explicitly add a few common Northern Atlantic chars as requested in issue 35.
 
 VALID_ACCENTED = \
     'áÁàÀâÂäÄãÃåÅæÆçÇéÉèÈêÊëËíÍìÌîÎïÏñÑóÓòÒôÔöÖõÕøØœŒßúÚùÙûÛüÜ' + 'ıİ' + 'ÐðÝýÞþ'
 
 # NOTE: we carefully avoid shell interpretation of dollar everywhere
 
@@ -145,10 +145,10 @@
     "`|^" + '\\' + '\n\r\t'
 VALID_FQDN_CHARACTERS = ascii_letters + digits + '.-'
 VALID_BACKEND_NAME_CHARACTERS = ascii_letters + digits + '-_'
-VALID_BASEURL_CHARACTERS = VALID_FQDN_CHARACTERS + ':/_'
-VALID_URL_CHARACTERS = VALID_BASEURL_CHARACTERS + '?;&%='
 # According to https://tools.ietf.org/html/rfc3986#section-2 URLs may contain
 # '%'-encoded chars, alphanum chars and query chars "-._~:/?#[]@!$&'()*+,;=`."
+VALID_BASEURL_CHARACTERS = VALID_FQDN_CHARACTERS + ':/_'
+VALID_URL_CHARACTERS = VALID_BASEURL_CHARACTERS + '?;&%='
 VALID_COMPLEXURL_CHARACTERS = VALID_BASEURL_CHARACTERS + \
     "%-._~:/?#[]@!$&'()*+,;=`."
 VALID_JOB_ID_CHARACTERS = VALID_FQDN_CHARACTERS + '_'
@@ -162,7 +162,7 @@
 VALID_CLOUD_NAME_CHARACTERS = VALID_CLOUD_LABEL_CHARACTERS + ' '
 VALID_CLOUD_INSTANCE_ID_CHARACTERS = VALID_CLOUD_NAME_CHARACTERS + ':'
 REJECT_UNSET = 'MUST_BE_SET_AND_NO_DEFAULT_VALUE'
 ALLOW_UNSAFE = \
     'THIS INPUT IS NOT VERIFIED: DO NOT EVER PRINT IT UNESCAPED! '
 
 # Allow these chars in addition to plain letters and digits
@@ -199,7 +199,7 @@
 # Absolute length limits for all passwords - further restricted in backends
 password_min_len = 4
 password_max_len = 256
 dn_max_len = 96
 
 valid_integer_chars = digits + integer_extras
 valid_float_chars = digits + float_extras
@@ -222,8 +222,8 @@
 
 # Type and value guess helpers - filled first time used
 
 __type_map = {}
 __value_map = {}
 
 
 # TODO: consider switching to a re.match with a precompiled expression
@@ -313,7 +313,7 @@
     return result
 
 
 def __wrap_unicode_name(char):
     """Build __NAME__ where NAME is the unicodedata name for the char"""
     return '__%s__' % unicode_name(force_unicode(char))
 
@@ -367,7 +367,7 @@
         max_length)
 
 
 def valid_alphanumeric_and_spaces(contents, min_length=0, max_length=-1,
                                   extra_chars=''):
     """Verify that supplied contents only contain alphanumeric characters and
     spaces"""
@@ -738,7 +738,7 @@
     extra_chars='',
 ):
     """Verify that supplied root VGrid name, only contains characters that we
     consider valid. Root VGrid names are user provided names possibly with common
     special characters.
     """
 
@@ -819,7 +819,7 @@
     valid_job_id(pattern, min_length, max_length, extra_chars)
 
 
 def valid_job_id_patterns(
     pattern_list,
     min_length=1,
     max_length=255,
@@ -1470,7 +1470,7 @@
 
         if ("%s" % default_value).lower() != first.lower():
             result = not default_value
     except:
         pass
     return (result, err)
 
@@ -1487,7 +1487,7 @@
         err += 'Invalid string default value (%s)' % default
     try:
         first = user_arguments_dict[name][0]
     except:
         first = "%s" % default
 
     # Validate input
@@ -1511,7 +1511,7 @@
         err += 'Invalid string default value (%s)' % default
     try:
         first = user_arguments_dict[name][0]
     except:
         first = "%s" % default
 
     # Validate input
@@ -1541,7 +1541,7 @@
         err += 'Invalid string default value (%s)' % default
     try:
         first = user_arguments_dict[name][0]
     except:
         first = "%s" % default
 
     # Validate input
@@ -1571,7 +1571,7 @@
         err += 'Invalid string default value (%s)' % default
     try:
         first = user_arguments_dict[name][0]
     except:
         first = "%s" % default
 
     # Validate input
@@ -1595,7 +1595,7 @@
         err += 'Invalid string default value (%s)' % default
     try:
         first = user_arguments_dict[name][0]
     except:
         first = "%s" % default
 
     # Validate input
@@ -1619,7 +1619,7 @@
         err += 'Invalid string default value (%s)' % default
     try:
         first = user_arguments_dict[name][0]
     except:
         first = "%s" % default
 
     # Validate input
@@ -1643,7 +1643,7 @@
         err += 'Invalid string default value (%s)' % default
     try:
         first = user_arguments_dict[name][0]
     except:
         first = "%s" % default
 
     # Validate input
@@ -2047,9 +2047,12 @@
         for key in ('modauthopenid.referrer',
                     'transfer_src',
                     'transfer_dst',
-                    'redirect_url',
                     ):
             __type_map[key] = valid_url
+        # NOTE: Trac URLs may have user IDs with '@' and can hit twofactor
+        for key in ('redirect_url',
+                    ):
+            __type_map[key] = valid_complex_url
 
         # GDP
 

tests/test_mig_shared_safeinput.py

@@ -1,9 +1,9 @@
 # -*- coding: utf-8 -*-
 #
 # --- BEGIN_HEADER ---
 #
 # test_mig_shared_safeinput - unit tests for shared safeinput validation
-# Copyright (C) 2003-2025  The MiG Project by the Science HPC Center at UCPH
+# Copyright (C) 2003-2026  The MiG Project by the Science HPC Center at UCPH
 #
 # This file is part of MiG.
 #
@@ -36,7 +36,8 @@
 
 from mig.shared.safeinput import main as safeinput_main, InputException, \
     filter_commonname, valid_alphanumeric, valid_commonname, valid_path, \
-    valid_printable, VALID_NAME_CHARACTERS
+    valid_printable, valid_base_url, valid_url, valid_complex_url, \
+    VALID_NAME_CHARACTERS
 
 PY2 = sys.version_info[0] == 2
 
@@ -88,6 +89,11 @@
         'Test Invalid ?',
         'Test HTML Invalid <code/>')
 
+    BASE_URL = 'https://www.migrid.org'
+    REGULAR_URL = 'https://www.migrid.org/wsgi-bin/ls.py?path=README&flags=v'
+    COMPLEX_URL = 'https://www.migrid.org/abc123@some.org/ls.py?path=R+D#HERE'
+    INVALID_URL = 'https://www.migrid.org/¾½§'
+
     def _provide_configuration(self):
         """Provide test configuration"""
         return 'testconfig'
@@ -210,6 +216,66 @@
         # Make sure unicode normalization doesn't raise exception
         self.assertEqual(valid_path(self.DECOMPOSED_UNICODE), None)
 
+    def test_valid_base_url_accepts_sample_base_url(self):
+        """Test that base URL succeeds in valid_base_url"""
+        # Make sure expected URL value doesn't raise exception
+        self.assertEqual(valid_base_url(self.BASE_URL), None)
+
+    def test_valid_base_url_refuses_sample_regular_url(self):
+        """Test that regular URL fails in valid_base_url"""
+        with self.assertRaises(InputException):
+            valid_base_url(self.REGULAR_URL)
+
+    def test_valid_base_url_refuses_sample_complex_url(self):
+        """Test that more complex URL fails in valid_base_url"""
+        with self.assertRaises(InputException):
+            valid_base_url(self.COMPLEX_URL)
+
+    def test_valid_base_url_refuses_sample_invalid_url(self):
+        """Test that invalid URL fails in valid_base_url"""
+        with self.assertRaises(InputException):
+            valid_base_url(self.INVALID_URL)
+
+    def test_valid_url_accepts_sample_base_url(self):
+        """Test that base URL succeeds in valid_url"""
+        # Make sure expected URL value doesn't raise exception
+        self.assertEqual(valid_url(self.BASE_URL), None)
+
+    def test_valid_url_accepts_sample_regular_url(self):
+        """Test that regular URL succeeds in valid_url"""
+        # Make sure expected URL value doesn't raise exception
+        self.assertEqual(valid_url(self.REGULAR_URL), None)
+
+    def test_valid_url_refuses_sample_complex_url(self):
+        """Test that complex URL fails in valid_url"""
+        with self.assertRaises(InputException):
+            valid_url(self.COMPLEX_URL)
+
+    def test_valid_url_refuses_sample_invalid_url(self):
+        """Test that invalid URL fails in valid_url"""
+        with self.assertRaises(InputException):
+            valid_url(self.INVALID_URL)
+
+    def test_valid_complex_url_accepts_sample_base_url(self):
+        """Test that base URL succeeds in valid_complex_url"""
+        # Make sure expected URL value doesn't raise exception
+        self.assertEqual(valid_complex_url(self.BASE_URL), None)
+
+    def test_valid_complex_url_accepts_sample_regular_url(self):
+        """Test that regular URL succeeds in valid_complex_url"""
+        # Make sure expected URL value doesn't raise exception
+        self.assertEqual(valid_complex_url(self.REGULAR_URL), None)
+
+    def test_valid_complex_url_accepts_sample_complex_url(self):
+        """Test that complex URL succeeds in valid_complex_url"""
+        # Make sure expected URL value doesn't raise exception
+        self.assertEqual(valid_complex_url(self.COMPLEX_URL), None)
+
+    def test_valid_complex_url_refuses_sample_invalid_url(self):
+        """Test that invalid URL fails in valid_complex_url"""
+        with self.assertRaises(InputException):
+            valid_complex_url(self.INVALID_URL)
+
 
 class TestMigSharedSafeInput__legacy(MigTestCase):
     """Legacy tests for safeinput module self-checks"""