From 1f1bb3e909f13c43d6c8c7784a5351daa9bf4acb Mon Sep 17 00:00:00 2001 From: pablohashescobar Date: Tue, 16 Sep 2025 17:26:57 +0530 Subject: [PATCH 1/3] 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. --- apps/api/plane/authentication/views/space/magic.py | 8 ++------ apps/api/plane/utils/path_validator.py | 4 ++-- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/apps/api/plane/authentication/views/space/magic.py b/apps/api/plane/authentication/views/space/magic.py index 0a5f2b42c95..f8ecbdf3827 100644 --- a/apps/api/plane/authentication/views/space/magic.py +++ b/apps/api/plane/authentication/views/space/magic.py @@ -94,9 +94,7 @@ def post(self, request): # 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 - ) + url = f"{base_host(request=request, is_space=True).rstrip('/')}{next_path}" return HttpResponseRedirect(url) except AuthenticationException as e: @@ -154,9 +152,7 @@ def post(self, request): # 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 - ) + 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 ebac7ca0bea..88900b48b4b 100644 --- a/apps/api/plane/utils/path_validator.py +++ b/apps/api/plane/utils/path_validator.py @@ -84,7 +84,7 @@ 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) @@ -94,5 +94,5 @@ def get_safe_redirect_url(base_url: str, next_path: str = "", params: dict = {}) params["next_path"] = validated_path # Return the safe redirect URL - return f"{base_url.rstrip('/')}?{urlencode(params)}" + return f"{base_url.rstrip('/')}?{urlencode(params, quote_via=lambda s, safe, encoding, errors: quote(s, safe="/"))}" \ No newline at end of file From c11a5377a69b0cf4eb612cec8268e88b827357cd Mon Sep 17 00:00:00 2001 From: pablohashescobar Date: Tue, 16 Sep 2025 18:14:12 +0530 Subject: [PATCH 2/3] 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. --- apps/api/plane/authentication/views/space/magic.py | 7 ++++--- apps/api/plane/utils/path_validator.py | 10 +++++----- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/apps/api/plane/authentication/views/space/magic.py b/apps/api/plane/authentication/views/space/magic.py index f8ecbdf3827..370a6d50a65 100644 --- a/apps/api/plane/authentication/views/space/magic.py +++ b/apps/api/plane/authentication/views/space/magic.py @@ -20,7 +20,7 @@ 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,7 +94,8 @@ def post(self, request): # Login the user and record his device info user_login(request=request, user=user, is_space=True) # redirect to referer path - url = f"{base_host(request=request, is_space=True).rstrip('/')}{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: @@ -152,7 +153,7 @@ def post(self, request): # Login the user and record his device info user_login(request=request, user=user, is_space=True) # redirect to referer 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}" 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 88900b48b4b..e5bf7aeb2a2 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. @@ -90,9 +89,10 @@ def get_safe_redirect_url(base_url: str, next_path: str = "", params: dict = {}) 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, quote_via=lambda s, safe, encoding, errors: quote(s, safe="/"))}" + return f"{base_url}/?next_path={validated_path}" \ No newline at end of file From 8cd430b90c2be93bb960fb37eb69bde50808ecef Mon Sep 17 00:00:00 2001 From: pablohashescobar Date: Tue, 16 Sep 2025 18:33:35 +0530 Subject: [PATCH 3/3] 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 | 1 + 5 files changed, 13 insertions(+), 24 deletions(-) diff --git a/apps/api/plane/authentication/views/space/email.py b/apps/api/plane/authentication/views/space/email.py index cd0954db836..d247f6e9899 100644 --- a/apps/api/plane/authentication/views/space/email.py +++ b/apps/api/plane/authentication/views/space/email.py @@ -14,7 +14,7 @@ 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 @@ def post(self, request): # 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 e3b64e8a0d1..dd148b8c10a 100644 --- a/apps/api/plane/authentication/views/space/github.py +++ b/apps/api/plane/authentication/views/space/github.py @@ -14,7 +14,7 @@ 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 @@ def get(self, request): 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 a63466005f1..77a10a91470 100644 --- a/apps/api/plane/authentication/views/space/gitlab.py +++ b/apps/api/plane/authentication/views/space/gitlab.py @@ -14,7 +14,7 @@ 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 @@ def get(self, request): 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 7b9728762fa..d8fef9da453 100644 --- a/apps/api/plane/authentication/views/space/google.py +++ b/apps/api/plane/authentication/views/space/google.py @@ -14,7 +14,7 @@ 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 @@ def get(self, request): # 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 370a6d50a65..f50274a4a23 100644 --- a/apps/api/plane/authentication/views/space/magic.py +++ b/apps/api/plane/authentication/views/space/magic.py @@ -153,6 +153,7 @@ def post(self, request): # Login the user and record his device info user_login(request=request, user=user, is_space=True) # redirect to referer 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)