mirror of
https://github.com/gosticks/plane.git
synced 2025-10-16 12:45:33 +00:00
[WEB-4943] refactor: enhance URL validation and redirection logic in authentication views (#7815)
* refactor: enhance URL validation and redirection logic in authentication views * Updated authentication views (SignInAuthSpaceEndpoint, GitHubCallbackSpaceEndpoint, GitLabCallbackSpaceEndpoint, GoogleCallbackSpaceEndpoint, and MagicSignInSpaceEndpoint) to include url_has_allowed_host_and_scheme checks for safer redirection. * Improved URL construction by ensuring proper formatting and fallback to base host when necessary. * Added get_allowed_hosts function to path_validator.py for better host validation. * refactor: improve comments and clean up code in path_validator.py * Updated comments for clarity in the get_safe_redirect_url function. * Removed unnecessary blank line to enhance
This commit is contained in:
parent
6d3d9e6df7
commit
3d06189723
@ -15,8 +15,7 @@ from plane.authentication.adapter.error import (
|
|||||||
AUTHENTICATION_ERROR_CODES,
|
AUTHENTICATION_ERROR_CODES,
|
||||||
AuthenticationException,
|
AuthenticationException,
|
||||||
)
|
)
|
||||||
from plane.utils.path_validator import get_safe_redirect_url, validate_next_path
|
from plane.utils.path_validator import get_safe_redirect_url, validate_next_path, get_allowed_hosts
|
||||||
|
|
||||||
|
|
||||||
class SignInAuthSpaceEndpoint(View):
|
class SignInAuthSpaceEndpoint(View):
|
||||||
def post(self, request):
|
def post(self, request):
|
||||||
@ -200,7 +199,7 @@ class SignUpAuthSpaceEndpoint(View):
|
|||||||
user_login(request=request, user=user, is_space=True)
|
user_login(request=request, user=user, is_space=True)
|
||||||
# redirect to referer path
|
# redirect to referer path
|
||||||
next_path = validate_next_path(next_path=next_path)
|
next_path = validate_next_path(next_path=next_path)
|
||||||
url = f"{base_host(request=request, is_space=True).rstrip("/")}{next_path}"
|
url = f"{base_host(request=request, is_space=True).rstrip('/')}{next_path}"
|
||||||
if url_has_allowed_host_and_scheme(url, allowed_hosts=get_allowed_hosts()):
|
if url_has_allowed_host_and_scheme(url, allowed_hosts=get_allowed_hosts()):
|
||||||
return HttpResponseRedirect(url)
|
return HttpResponseRedirect(url)
|
||||||
else:
|
else:
|
||||||
|
|||||||
@ -4,6 +4,7 @@ import uuid
|
|||||||
# Django import
|
# Django import
|
||||||
from django.http import HttpResponseRedirect
|
from django.http import HttpResponseRedirect
|
||||||
from django.views import View
|
from django.views import View
|
||||||
|
from django.utils.http import url_has_allowed_host_and_scheme
|
||||||
|
|
||||||
# Module imports
|
# Module imports
|
||||||
from plane.authentication.provider.oauth.github import GitHubOAuthProvider
|
from plane.authentication.provider.oauth.github import GitHubOAuthProvider
|
||||||
@ -14,7 +15,7 @@ from plane.authentication.adapter.error import (
|
|||||||
AUTHENTICATION_ERROR_CODES,
|
AUTHENTICATION_ERROR_CODES,
|
||||||
AuthenticationException,
|
AuthenticationException,
|
||||||
)
|
)
|
||||||
from plane.utils.path_validator import get_safe_redirect_url, validate_next_path
|
from plane.utils.path_validator import get_safe_redirect_url, validate_next_path, get_allowed_hosts
|
||||||
|
|
||||||
|
|
||||||
class GitHubOauthInitiateSpaceEndpoint(View):
|
class GitHubOauthInitiateSpaceEndpoint(View):
|
||||||
@ -94,8 +95,11 @@ class GitHubCallbackSpaceEndpoint(View):
|
|||||||
# Process workspace and project invitations
|
# Process workspace and project invitations
|
||||||
# redirect to referer path
|
# redirect to referer path
|
||||||
next_path = validate_next_path(next_path=next_path)
|
next_path = validate_next_path(next_path=next_path)
|
||||||
url = f"{base_host(request=request, is_space=True).rstrip("/")}{next_path}"
|
url = f"{base_host(request=request, is_space=True).rstrip('/')}{next_path}"
|
||||||
|
if url_has_allowed_host_and_scheme(url, allowed_hosts=get_allowed_hosts()):
|
||||||
return HttpResponseRedirect(url)
|
return HttpResponseRedirect(url)
|
||||||
|
else:
|
||||||
|
return HttpResponseRedirect(base_host(request=request, is_space=True))
|
||||||
except AuthenticationException as e:
|
except AuthenticationException as e:
|
||||||
params = e.get_error_dict()
|
params = e.get_error_dict()
|
||||||
url = get_safe_redirect_url(
|
url = get_safe_redirect_url(
|
||||||
|
|||||||
@ -4,6 +4,7 @@ import uuid
|
|||||||
# Django import
|
# Django import
|
||||||
from django.http import HttpResponseRedirect
|
from django.http import HttpResponseRedirect
|
||||||
from django.views import View
|
from django.views import View
|
||||||
|
from django.utils.http import url_has_allowed_host_and_scheme
|
||||||
|
|
||||||
# Module imports
|
# Module imports
|
||||||
from plane.authentication.provider.oauth.gitlab import GitLabOAuthProvider
|
from plane.authentication.provider.oauth.gitlab import GitLabOAuthProvider
|
||||||
@ -14,7 +15,7 @@ from plane.authentication.adapter.error import (
|
|||||||
AUTHENTICATION_ERROR_CODES,
|
AUTHENTICATION_ERROR_CODES,
|
||||||
AuthenticationException,
|
AuthenticationException,
|
||||||
)
|
)
|
||||||
from plane.utils.path_validator import get_safe_redirect_url, validate_next_path
|
from plane.utils.path_validator import get_safe_redirect_url, get_allowed_hosts, validate_next_path
|
||||||
|
|
||||||
|
|
||||||
class GitLabOauthInitiateSpaceEndpoint(View):
|
class GitLabOauthInitiateSpaceEndpoint(View):
|
||||||
@ -95,8 +96,11 @@ class GitLabCallbackSpaceEndpoint(View):
|
|||||||
# Process workspace and project invitations
|
# Process workspace and project invitations
|
||||||
# redirect to referer path
|
# redirect to referer path
|
||||||
next_path = validate_next_path(next_path=next_path)
|
next_path = validate_next_path(next_path=next_path)
|
||||||
url = f"{base_host(request=request, is_space=True).rstrip("/")}{next_path}"
|
url = f"{base_host(request=request, is_space=True).rstrip('/')}{next_path}"
|
||||||
|
if url_has_allowed_host_and_scheme(url, allowed_hosts=get_allowed_hosts()):
|
||||||
return HttpResponseRedirect(url)
|
return HttpResponseRedirect(url)
|
||||||
|
else:
|
||||||
|
return HttpResponseRedirect(base_host(request=request, is_space=True))
|
||||||
except AuthenticationException as e:
|
except AuthenticationException as e:
|
||||||
params = e.get_error_dict()
|
params = e.get_error_dict()
|
||||||
url = get_safe_redirect_url(
|
url = get_safe_redirect_url(
|
||||||
|
|||||||
@ -4,6 +4,7 @@ import uuid
|
|||||||
# Django import
|
# Django import
|
||||||
from django.http import HttpResponseRedirect
|
from django.http import HttpResponseRedirect
|
||||||
from django.views import View
|
from django.views import View
|
||||||
|
from django.utils.http import url_has_allowed_host_and_scheme
|
||||||
|
|
||||||
# Module imports
|
# Module imports
|
||||||
from plane.authentication.provider.oauth.google import GoogleOAuthProvider
|
from plane.authentication.provider.oauth.google import GoogleOAuthProvider
|
||||||
@ -14,7 +15,7 @@ from plane.authentication.adapter.error import (
|
|||||||
AuthenticationException,
|
AuthenticationException,
|
||||||
AUTHENTICATION_ERROR_CODES,
|
AUTHENTICATION_ERROR_CODES,
|
||||||
)
|
)
|
||||||
from plane.utils.path_validator import get_safe_redirect_url, validate_next_path
|
from plane.utils.path_validator import get_safe_redirect_url, validate_next_path, get_allowed_hosts
|
||||||
|
|
||||||
|
|
||||||
class GoogleOauthInitiateSpaceEndpoint(View):
|
class GoogleOauthInitiateSpaceEndpoint(View):
|
||||||
@ -91,8 +92,11 @@ class GoogleCallbackSpaceEndpoint(View):
|
|||||||
user_login(request=request, user=user, is_space=True)
|
user_login(request=request, user=user, is_space=True)
|
||||||
# redirect to referer path
|
# redirect to referer path
|
||||||
next_path = validate_next_path(next_path=next_path)
|
next_path = validate_next_path(next_path=next_path)
|
||||||
url = f"{base_host(request=request, is_space=True).rstrip("/")}{next_path}"
|
url = f"{base_host(request=request, is_space=True).rstrip('/')}{next_path}"
|
||||||
|
if url_has_allowed_host_and_scheme(url, allowed_hosts=get_allowed_hosts()):
|
||||||
return HttpResponseRedirect(url)
|
return HttpResponseRedirect(url)
|
||||||
|
else:
|
||||||
|
return HttpResponseRedirect(base_host(request=request, is_space=True))
|
||||||
except AuthenticationException as e:
|
except AuthenticationException as e:
|
||||||
params = e.get_error_dict()
|
params = e.get_error_dict()
|
||||||
url = get_safe_redirect_url(
|
url = get_safe_redirect_url(
|
||||||
|
|||||||
@ -96,7 +96,7 @@ class MagicSignInSpaceEndpoint(View):
|
|||||||
user_login(request=request, user=user, is_space=True)
|
user_login(request=request, user=user, is_space=True)
|
||||||
# redirect to referer path
|
# redirect to referer path
|
||||||
next_path = validate_next_path(next_path=next_path)
|
next_path = validate_next_path(next_path=next_path)
|
||||||
url = f"{base_host(request=request, is_space=True).rstrip("/")}{next_path}"
|
url = f"{base_host(request=request, is_space=True).rstrip('/')}{next_path}"
|
||||||
if url_has_allowed_host_and_scheme(url, allowed_hosts=get_allowed_hosts()):
|
if url_has_allowed_host_and_scheme(url, allowed_hosts=get_allowed_hosts()):
|
||||||
return HttpResponseRedirect(url)
|
return HttpResponseRedirect(url)
|
||||||
else:
|
else:
|
||||||
@ -158,7 +158,7 @@ class MagicSignUpSpaceEndpoint(View):
|
|||||||
user_login(request=request, user=user, is_space=True)
|
user_login(request=request, user=user, is_space=True)
|
||||||
# redirect to referer path
|
# redirect to referer path
|
||||||
next_path = validate_next_path(next_path=next_path)
|
next_path = validate_next_path(next_path=next_path)
|
||||||
url = f"{base_host(request=request, is_space=True).rstrip("/")}{next_path}"
|
url = f"{base_host(request=request, is_space=True).rstrip('/')}{next_path}"
|
||||||
if url_has_allowed_host_and_scheme(url, allowed_hosts=get_allowed_hosts()):
|
if url_has_allowed_host_and_scheme(url, allowed_hosts=get_allowed_hosts()):
|
||||||
return HttpResponseRedirect(url)
|
return HttpResponseRedirect(url)
|
||||||
else:
|
else:
|
||||||
|
|||||||
@ -46,7 +46,11 @@ def _contains_suspicious_patterns(path: str) -> bool:
|
|||||||
def get_allowed_hosts() -> list[str]:
|
def get_allowed_hosts() -> list[str]:
|
||||||
"""Get the allowed hosts from the settings."""
|
"""Get the allowed hosts from the settings."""
|
||||||
base_origin = settings.WEB_URL or settings.APP_BASE_URL
|
base_origin = settings.WEB_URL or settings.APP_BASE_URL
|
||||||
allowed_hosts = [base_origin]
|
|
||||||
|
allowed_hosts = []
|
||||||
|
if base_origin:
|
||||||
|
host = urlparse(base_origin).netloc
|
||||||
|
allowed_hosts.append(host)
|
||||||
if settings.ADMIN_BASE_URL:
|
if settings.ADMIN_BASE_URL:
|
||||||
# Get only the host
|
# Get only the host
|
||||||
host = urlparse(settings.ADMIN_BASE_URL).netloc
|
host = urlparse(settings.ADMIN_BASE_URL).netloc
|
||||||
@ -110,16 +114,30 @@ def get_safe_redirect_url(base_url: str, next_path: str = "", params: dict = {})
|
|||||||
|
|
||||||
# Add the next path to the parameters
|
# Add the next path to the parameters
|
||||||
base_url = base_url.rstrip('/')
|
base_url = base_url.rstrip('/')
|
||||||
|
|
||||||
|
# Prepare the query parameters
|
||||||
|
query_parts = []
|
||||||
|
encoded_params = ""
|
||||||
|
|
||||||
|
# Add the next path to the parameters
|
||||||
|
if validated_path:
|
||||||
|
query_parts.append(f"next_path={validated_path}")
|
||||||
|
|
||||||
|
# Add additional parameters
|
||||||
if params:
|
if params:
|
||||||
encoded_params = urlencode(params)
|
encoded_params = urlencode(params)
|
||||||
url = f"{base_url}/?next_path={validated_path}&{encoded_params}"
|
query_parts.append(encoded_params)
|
||||||
|
|
||||||
|
# Construct the url query string
|
||||||
|
if query_parts:
|
||||||
|
query_string = "&".join(query_parts)
|
||||||
|
url = f"{base_url}/?{query_string}"
|
||||||
else:
|
else:
|
||||||
url = f"{base_url}/?next_path={validated_path}"
|
url = base_url
|
||||||
|
|
||||||
# Check if the URL is allowed
|
# Check if the URL is allowed
|
||||||
if url_has_allowed_host_and_scheme(url, allowed_hosts=get_allowed_hosts()):
|
if url_has_allowed_host_and_scheme(url, allowed_hosts=get_allowed_hosts()):
|
||||||
return url
|
return url
|
||||||
|
|
||||||
# Return the base URL if the URL is not allowed
|
# Return the base URL if the URL is not allowed
|
||||||
return base_url
|
return base_url + (f"?{encoded_params}" if encoded_params else "")
|
||||||
|
|
||||||
Loading…
Reference in New Issue
Block a user