Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions apps/api/plane/authentication/views/space/email.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
)
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")
Expand Down Expand Up @@ -99,13 +100,13 @@ def post(self, request):
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(
Expand Down
1 change: 1 addition & 0 deletions apps/api/plane/authentication/views/space/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ def get(self, request):
# 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)
Expand Down
4 changes: 3 additions & 1 deletion apps/api/plane/authentication/views/space/gitlab.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
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):
Expand Down Expand Up @@ -96,6 +97,7 @@ def get(self, request):
# 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)
Expand Down
1 change: 1 addition & 0 deletions apps/api/plane/authentication/views/space/google.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ def get(self, request):
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)
Expand Down
25 changes: 23 additions & 2 deletions apps/space/app/page.tsx
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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 (
Expand All @@ -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 />;
});
Expand Down
79 changes: 63 additions & 16 deletions packages/utils/src/url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}
}
Loading