From 877c117c371f92192c6121d0ad1833fa67e9a3f8 Mon Sep 17 00:00:00 2001 From: Vamsi Krishna <46787868+vamsikrishnamathala@users.noreply.github.com> Date: Wed, 17 Sep 2025 18:52:35 +0530 Subject: [PATCH] [WEB-4943]fix: next path url redirection (#7817) * fix: next path url redirection * fix: enhance URL redirection safety in authentication views Updated SignInAuthSpaceEndpoint, GitHubCallbackSpaceEndpoint, GitLabCallbackSpaceEndpoint, and GoogleCallbackSpaceEndpoint to include checks for allowed hosts and schemes before redirecting. This improves the security of URL redirection by ensuring only valid URLs are used. * chore: updated uitl to handle double / --------- Co-authored-by: pablohashescobar Co-authored-by: Nikhil <118773738+pablohashescobar@users.noreply.github.com> --- .../plane/authentication/views/space/email.py | 15 ++-- .../authentication/views/space/github.py | 1 + .../authentication/views/space/gitlab.py | 4 +- .../authentication/views/space/google.py | 1 + apps/space/app/page.tsx | 25 +++++- packages/utils/src/url.ts | 79 +++++++++++++++---- 6 files changed, 99 insertions(+), 26 deletions(-) diff --git a/apps/api/plane/authentication/views/space/email.py b/apps/api/plane/authentication/views/space/email.py index b25e2d015..afba06ddc 100644 --- a/apps/api/plane/authentication/views/space/email.py +++ b/apps/api/plane/authentication/views/space/email.py @@ -17,6 +17,7 @@ from plane.authentication.adapter.error import ( ) from plane.utils.path_validator import get_safe_redirect_url, validate_next_path, get_allowed_hosts + class SignInAuthSpaceEndpoint(View): def post(self, request): next_path = request.POST.get("next_path") @@ -99,13 +100,13 @@ class SignInAuthSpaceEndpoint(View): user = provider.authenticate() # Login the user and record his device info user_login(request=request, user=user, is_space=True) - # redirect to next path - url = get_safe_redirect_url( - base_url=base_host(request=request, is_space=True), - next_path=next_path, - params={} - ) - return HttpResponseRedirect(url) + # 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}" + if url_has_allowed_host_and_scheme(url, allowed_hosts=get_allowed_hosts()): + return HttpResponseRedirect(url) + else: + return HttpResponseRedirect(base_host(request=request, is_space=True)) except AuthenticationException as e: params = e.get_error_dict() url = get_safe_redirect_url( diff --git a/apps/api/plane/authentication/views/space/github.py b/apps/api/plane/authentication/views/space/github.py index acc956cf3..1a7d51d66 100644 --- a/apps/api/plane/authentication/views/space/github.py +++ b/apps/api/plane/authentication/views/space/github.py @@ -95,6 +95,7 @@ class GitHubCallbackSpaceEndpoint(View): # Process workspace and project invitations # 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}" if url_has_allowed_host_and_scheme(url, allowed_hosts=get_allowed_hosts()): return HttpResponseRedirect(url) diff --git a/apps/api/plane/authentication/views/space/gitlab.py b/apps/api/plane/authentication/views/space/gitlab.py index 493cd823e..26ed17f32 100644 --- a/apps/api/plane/authentication/views/space/gitlab.py +++ b/apps/api/plane/authentication/views/space/gitlab.py @@ -15,7 +15,8 @@ from plane.authentication.adapter.error import ( AUTHENTICATION_ERROR_CODES, AuthenticationException, ) -from plane.utils.path_validator import get_safe_redirect_url, get_allowed_hosts, validate_next_path +from plane.utils.path_validator import get_safe_redirect_url, validate_next_path, get_allowed_hosts + class GitLabOauthInitiateSpaceEndpoint(View): @@ -96,6 +97,7 @@ class GitLabCallbackSpaceEndpoint(View): # Process workspace and project invitations # 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}" if url_has_allowed_host_and_scheme(url, allowed_hosts=get_allowed_hosts()): return HttpResponseRedirect(url) diff --git a/apps/api/plane/authentication/views/space/google.py b/apps/api/plane/authentication/views/space/google.py index c7c833771..617216d64 100644 --- a/apps/api/plane/authentication/views/space/google.py +++ b/apps/api/plane/authentication/views/space/google.py @@ -92,6 +92,7 @@ class GoogleCallbackSpaceEndpoint(View): 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}" if url_has_allowed_host_and_scheme(url, allowed_hosts=get_allowed_hosts()): return HttpResponseRedirect(url) diff --git a/apps/space/app/page.tsx b/apps/space/app/page.tsx index a75275e0d..f544bcb10 100644 --- a/apps/space/app/page.tsx +++ b/apps/space/app/page.tsx @@ -1,6 +1,9 @@ "use client"; - +import { useEffect } from "react"; import { observer } from "mobx-react"; +import { useSearchParams, useRouter } from "next/navigation"; +// plane imports +import { isValidNextPath } from "@plane/utils"; // components import { UserLoggedIn } from "@/components/account/user-logged-in"; import { LogoSpinner } from "@/components/common/logo-spinner"; @@ -10,6 +13,15 @@ import { useUser } from "@/hooks/store/use-user"; const HomePage = observer(() => { const { data: currentUser, isAuthenticated, isInitializing } = useUser(); + const searchParams = useSearchParams(); + const router = useRouter(); + const nextPath = searchParams.get("next_path"); + + useEffect(() => { + if (currentUser && isAuthenticated && nextPath && isValidNextPath(nextPath)) { + router.replace(nextPath); + } + }, [currentUser, isAuthenticated, nextPath, router]); if (isInitializing) return ( @@ -18,7 +30,16 @@ const HomePage = observer(() => { ); - if (currentUser && isAuthenticated) return ; + if (currentUser && isAuthenticated) { + if (nextPath && isValidNextPath(nextPath)) { + return ( +
+ +
+ ); + } + return ; + } return ; }); diff --git a/packages/utils/src/url.ts b/packages/utils/src/url.ts index 638839bb0..738d935d5 100644 --- a/packages/utils/src/url.ts +++ b/packages/utils/src/url.ts @@ -114,22 +114,6 @@ export function extractHostname(url: string): string { * - Removes hash fragments (everything after '#') * - Removes port numbers (everything after ':') * 3. Validates the TLD against a list of known TLDs - * - * @example - * // Valid cases (returns the TLD) - * extractTLD('example.com') // returns 'com' - * extractTLD('sub.example.com') // returns 'com' - * extractTLD('example.com/path') // returns 'com' - * extractTLD('example.com:8080') // returns 'com' - * extractTLD('example.com?query=1') // returns 'com' - * extractTLD('example.com#hash') // returns 'com' - * - * // Invalid cases (returns empty string) - * extractTLD('') // returns '' - * extractTLD('.example.com') // returns '' - * extractTLD('example.com.') // returns '' - * extractTLD('example.invalid') // returns '' - * extractTLD('localhost') // returns '' */ export function extractTLD(urlString: string): string { @@ -257,3 +241,66 @@ export function extractURLComponents(url: URL | string): IURLComponents | undefi return undefined; } } + +/** + * Validates that a next_path parameter is safe for redirection. + * Only allows relative paths starting with "/" to prevent open redirect vulnerabilities. + * + * @param url - The next_path URL to validate + * @returns True if the URL is a safe relative path, false otherwise + * + * @example + * isValidNextPath("/dashboard") // true + * isValidNextPath("/workspace/123") // true + * isValidNextPath("https://malicious.com") // false + * isValidNextPath("//malicious.com") // false (protocol-relative) + * isValidNextPath("javascript:alert(1)") // false + * isValidNextPath("") // false + * isValidNextPath("dashboard") // false (must start with /) + * isValidNextPath("\\malicious") // false (backslash) + * isValidNextPath(" /dashboard ") // true (trimmed) + */ +export function isValidNextPath(url: string): boolean { + if (!url || typeof url !== "string") return false; + + // Trim leading/trailing whitespace + const trimmedUrl = url.trim(); + + if (!trimmedUrl) return false; + + // Only allow relative paths starting with / + if (!trimmedUrl.startsWith("/")) return false; + + // Block protocol-relative URLs (//example.com) - open redirect vulnerability + if (trimmedUrl.startsWith("//")) return false; + + // Block backslashes which can be used for path traversal or Windows-style paths + if (trimmedUrl.includes("\\")) return false; + + try { + // Use URL constructor with a dummy base to normalize and validate the path + const normalizedUrl = new URL(trimmedUrl, "http://localhost"); + + // Ensure the path is still relative (no host change from our dummy base) + if (normalizedUrl.hostname !== "localhost" || normalizedUrl.protocol !== "http:") { + return false; + } + + // Use the normalized pathname for additional security checks + const pathname = normalizedUrl.pathname; + + // Additional security checks for malicious patterns in the normalized path + const maliciousPatterns = [ + /javascript:/i, + /data:/i, + /vbscript:/i, + /