[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 <nikhilschacko@gmail.com>
Co-authored-by: Nikhil <118773738+pablohashescobar@users.noreply.github.com>
This commit is contained in:
Vamsi Krishna 2025-09-17 18:52:35 +05:30 committed by GitHub
parent 3d06189723
commit 877c117c37
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 99 additions and 26 deletions

View File

@ -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(

View File

@ -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)

View File

@ -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)

View File

@ -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)

View File

@ -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(() => {
</div>
);
if (currentUser && isAuthenticated) return <UserLoggedIn />;
if (currentUser && isAuthenticated) {
if (nextPath && isValidNextPath(nextPath)) {
return (
<div className="flex h-screen min-h-[500px] w-full justify-center items-center">
<LogoSpinner />
</div>
);
}
return <UserLoggedIn />;
}
return <AuthView />;
});

View File

@ -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,
/<script/i,
/on\w+=/i, // Event handlers like onclick=, onload=
];
return !maliciousPatterns.some((pattern) => pattern.test(pathname));
} catch (error) {
// If URL constructor fails, it's an invalid path
return false;
}
}