From 4d17637edf07b3db586f08e3a0a551e451d28a72 Mon Sep 17 00:00:00 2001 From: Nikhil <118773738+pablohashescobar@users.noreply.github.com> Date: Tue, 16 Sep 2025 18:44:26 +0530 Subject: [PATCH] [WEB-4943] refactor: streamline URL construction in authentication views (#7806) * refactor: streamline URL construction in authentication views * Updated MagicSignInSpaceEndpoint and MagicSignUpSpaceEndpoint to directly construct redirect URLs using formatted strings instead of the get_safe_redirect_url function. * Enhanced get_safe_redirect_url to use quote for safer URL encoding of parameters. * refactor: enhance URL validation and redirection in authentication views * Added validate_next_path function to improve the safety of redirect URLs in MagicSignInSpaceEndpoint and MagicSignUpSpaceEndpoint. * Updated URL construction to ensure proper handling of next_path and base_url. * Streamlined the get_safe_redirect_url function for better parameter encoding. * refactor: unify URL redirection logic across authentication views * Introduced validate_next_path function to enhance URL safety in SignInAuthSpaceEndpoint, SignUpAuthSpaceEndpoint, GitHubCallbackSpaceEndpoint, GitLabCallbackSpaceEndpoint, and GoogleCallbackSpaceEndpoint. * Updated URL construction to directly format the redirect URL, improving clarity and consistency across multiple authentication views. --- apps/api/plane/authentication/views/space/email.py | 9 +++------ apps/api/plane/authentication/views/space/github.py | 9 +++------ apps/api/plane/authentication/views/space/gitlab.py | 9 +++------ apps/api/plane/authentication/views/space/google.py | 9 +++------ apps/api/plane/authentication/views/space/magic.py | 12 +++++------- apps/api/plane/utils/path_validator.py | 12 ++++++------ 6 files changed, 23 insertions(+), 37 deletions(-) diff --git a/apps/api/plane/authentication/views/space/email.py b/apps/api/plane/authentication/views/space/email.py index cd0954db8..d247f6e98 100644 --- a/apps/api/plane/authentication/views/space/email.py +++ b/apps/api/plane/authentication/views/space/email.py @@ -14,7 +14,7 @@ from plane.authentication.adapter.error import ( AUTHENTICATION_ERROR_CODES, AuthenticationException, ) -from plane.utils.path_validator import get_safe_redirect_url +from plane.utils.path_validator import get_safe_redirect_url, validate_next_path class SignInAuthSpaceEndpoint(View): @@ -198,11 +198,8 @@ class SignUpAuthSpaceEndpoint(View): # Login the user and record his device info user_login(request=request, user=user, is_space=True) # redirect to referer path - url = get_safe_redirect_url( - base_url=base_host(request=request, is_space=True), - next_path=next_path, - params={} - ) + next_path = validate_next_path(next_path=next_path) + url = f"{base_host(request=request, is_space=True).rstrip("/")}{next_path}" return HttpResponseRedirect(url) except AuthenticationException as e: params = e.get_error_dict() diff --git a/apps/api/plane/authentication/views/space/github.py b/apps/api/plane/authentication/views/space/github.py index e3b64e8a0..dd148b8c1 100644 --- a/apps/api/plane/authentication/views/space/github.py +++ b/apps/api/plane/authentication/views/space/github.py @@ -14,7 +14,7 @@ from plane.authentication.adapter.error import ( AUTHENTICATION_ERROR_CODES, AuthenticationException, ) -from plane.utils.path_validator import get_safe_redirect_url +from plane.utils.path_validator import get_safe_redirect_url, validate_next_path class GitHubOauthInitiateSpaceEndpoint(View): @@ -93,11 +93,8 @@ class GitHubCallbackSpaceEndpoint(View): user_login(request=request, user=user, is_space=True) # Process workspace and project invitations # redirect to referer path - url = get_safe_redirect_url( - base_url=base_host(request=request, is_space=True), - next_path=next_path, - params=params - ) + next_path = validate_next_path(next_path=next_path) + url = f"{base_host(request=request, is_space=True).rstrip("/")}{next_path}" return HttpResponseRedirect(url) except AuthenticationException as e: params = e.get_error_dict() diff --git a/apps/api/plane/authentication/views/space/gitlab.py b/apps/api/plane/authentication/views/space/gitlab.py index a63466005..77a10a914 100644 --- a/apps/api/plane/authentication/views/space/gitlab.py +++ b/apps/api/plane/authentication/views/space/gitlab.py @@ -14,7 +14,7 @@ from plane.authentication.adapter.error import ( AUTHENTICATION_ERROR_CODES, AuthenticationException, ) -from plane.utils.path_validator import get_safe_redirect_url +from plane.utils.path_validator import get_safe_redirect_url, validate_next_path class GitLabOauthInitiateSpaceEndpoint(View): @@ -94,11 +94,8 @@ class GitLabCallbackSpaceEndpoint(View): user_login(request=request, user=user, is_space=True) # Process workspace and project invitations # redirect to referer path - url = get_safe_redirect_url( - base_url=base_host(request=request, is_space=True), - next_path=next_path, - params=params - ) + next_path = validate_next_path(next_path=next_path) + url = f"{base_host(request=request, is_space=True).rstrip("/")}{next_path}" return HttpResponseRedirect(url) except AuthenticationException as e: params = e.get_error_dict() diff --git a/apps/api/plane/authentication/views/space/google.py b/apps/api/plane/authentication/views/space/google.py index 7b9728762..d8fef9da4 100644 --- a/apps/api/plane/authentication/views/space/google.py +++ b/apps/api/plane/authentication/views/space/google.py @@ -14,7 +14,7 @@ from plane.authentication.adapter.error import ( AuthenticationException, AUTHENTICATION_ERROR_CODES, ) -from plane.utils.path_validator import get_safe_redirect_url +from plane.utils.path_validator import get_safe_redirect_url, validate_next_path class GoogleOauthInitiateSpaceEndpoint(View): @@ -90,11 +90,8 @@ class GoogleCallbackSpaceEndpoint(View): # Login the user and record his device info user_login(request=request, user=user, is_space=True) # redirect to referer path - url = get_safe_redirect_url( - base_url=base_host(request=request, is_space=True), - next_path=next_path, - params=params - ) + next_path = validate_next_path(next_path=next_path) + url = f"{base_host(request=request, is_space=True).rstrip("/")}{next_path}" return HttpResponseRedirect(url) except AuthenticationException as e: params = e.get_error_dict() diff --git a/apps/api/plane/authentication/views/space/magic.py b/apps/api/plane/authentication/views/space/magic.py index 0a5f2b42c..f50274a4a 100644 --- a/apps/api/plane/authentication/views/space/magic.py +++ b/apps/api/plane/authentication/views/space/magic.py @@ -20,7 +20,7 @@ from plane.authentication.adapter.error import ( AuthenticationException, AUTHENTICATION_ERROR_CODES, ) -from plane.utils.path_validator import get_safe_redirect_url +from plane.utils.path_validator import get_safe_redirect_url, validate_next_path class MagicGenerateSpaceEndpoint(APIView): @@ -94,9 +94,8 @@ class MagicSignInSpaceEndpoint(View): # Login the user and record his device info user_login(request=request, user=user, is_space=True) # redirect to referer path - url = get_safe_redirect_url( - base_url=base_host(request=request, is_space=True), 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}" return HttpResponseRedirect(url) except AuthenticationException as e: @@ -154,9 +153,8 @@ class MagicSignUpSpaceEndpoint(View): # Login the user and record his device info user_login(request=request, user=user, is_space=True) # redirect to referer path - url = get_safe_redirect_url( - base_url=base_host(request=request, is_space=True), 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}" return HttpResponseRedirect(url) except AuthenticationException as e: diff --git a/apps/api/plane/utils/path_validator.py b/apps/api/plane/utils/path_validator.py index ebac7ca0b..e5bf7aeb2 100644 --- a/apps/api/plane/utils/path_validator.py +++ b/apps/api/plane/utils/path_validator.py @@ -1,7 +1,6 @@ # Python imports from urllib.parse import urlparse - def _contains_suspicious_patterns(path: str) -> bool: """ Check for suspicious patterns that might indicate malicious intent. @@ -84,15 +83,16 @@ def get_safe_redirect_url(base_url: str, next_path: str = "", params: dict = {}) Returns: str: The safe redirect URL """ - from urllib.parse import urlencode + from urllib.parse import urlencode, quote # Validate the next path validated_path = validate_next_path(next_path) # Add the next path to the parameters - if validated_path: - params["next_path"] = validated_path + base_url = base_url.rstrip('/') + if params: + encoded_params = urlencode(params) + return f"{base_url}/?next_path={validated_path}&{encoded_params}" - # Return the safe redirect URL - return f"{base_url.rstrip('/')}?{urlencode(params)}" + return f"{base_url}/?next_path={validated_path}" \ No newline at end of file