Skip to content

Commit

Permalink
YDA-5890: restrict redirects on login
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
stsnel committed Aug 12, 2024
1 parent d0de40f commit d90c67e
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 3 deletions.
9 changes: 9 additions & 0 deletions unit-tests/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
get_theme_directories,
get_validated_static_path,
is_email_in_domains,
is_relative_url,
length_check,
unicode_secure_filename,
)
Expand Down Expand Up @@ -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:[email protected]"), False)
self.assertEqual(is_relative_url("https://username:[email protected]"), False)
self.assertEqual(is_relative_url("/foo/bar/bat"), True)
self.assertEqual(is_relative_url("foo/bar/bat"), True)
6 changes: 3 additions & 3 deletions user/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
13 changes: 13 additions & 0 deletions util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 == ""

0 comments on commit d90c67e

Please sign in to comment.