From c142b2db028c6cb2f1e1ad53d92f3348dfd0bd19 Mon Sep 17 00:00:00 2001 From: Nikolas Garza <90273783+nmgarza5@users.noreply.github.com> Date: Fri, 20 Feb 2026 17:30:36 -0800 Subject: [PATCH] chore: special sauce (#8646) --- backend/onyx/auth/users.py | 5 ++- backend/onyx/server/saml.py | 10 ++++-- .../nginx/app.conf.template.no-letsencrypt | 32 +++++-------------- deployment/data/nginx/app.conf.template.prod | 32 +++++-------------- web/src/app/auth/saml/callback/route.ts | 11 +------ web/src/lib/redirectSS.ts | 18 +++-------- 6 files changed, 32 insertions(+), 76 deletions(-) diff --git a/backend/onyx/auth/users.py b/backend/onyx/auth/users.py index 014e56cc79..4d07626e12 100644 --- a/backend/onyx/auth/users.py +++ b/backend/onyx/auth/users.py @@ -1671,7 +1671,10 @@ def get_oauth_router( if redirect_url is not None: authorize_redirect_url = redirect_url else: - authorize_redirect_url = str(request.url_for(callback_route_name)) + # Use WEB_DOMAIN instead of request.url_for() to prevent host + # header poisoning — request.url_for() trusts the Host header. + callback_path = request.app.url_path_for(callback_route_name) + authorize_redirect_url = f"{WEB_DOMAIN}{callback_path}" next_url = request.query_params.get("next", "/") diff --git a/backend/onyx/server/saml.py b/backend/onyx/server/saml.py index 6fef3df6db..4e49660de6 100644 --- a/backend/onyx/server/saml.py +++ b/backend/onyx/server/saml.py @@ -24,6 +24,7 @@ from onyx.auth.users import get_user_manager from onyx.auth.users import UserManager from onyx.configs.app_configs import REQUIRE_EMAIL_VERIFICATION from onyx.configs.app_configs import SAML_CONF_DIR +from onyx.configs.app_configs import WEB_DOMAIN from onyx.db.auth import get_user_count from onyx.db.auth import get_user_db from onyx.db.engine.async_sql_engine import get_async_session_context_manager @@ -123,9 +124,12 @@ async def prepare_from_fastapi_request(request: Request) -> dict[str, Any]: if request.client is None: raise ValueError("Invalid request for SAML") - # Use X-Forwarded headers if available - http_host = request.headers.get("X-Forwarded-Host") or request.client.host - server_port = request.headers.get("X-Forwarded-Port") or request.url.port + # Derive http_host and server_port from WEB_DOMAIN (a trusted env var) + # instead of X-Forwarded-* headers, which can be spoofed by an attacker + # to poison SAML redirect URLs (host header poisoning). + parsed_domain = urlparse(WEB_DOMAIN) + http_host = parsed_domain.hostname or request.client.host + server_port = parsed_domain.port or (443 if parsed_domain.scheme == "https" else 80) rv: dict[str, Any] = { "http_host": http_host, diff --git a/deployment/data/nginx/app.conf.template.no-letsencrypt b/deployment/data/nginx/app.conf.template.no-letsencrypt index a0f7ea95de..8e6c7fda7b 100644 --- a/deployment/data/nginx/app.conf.template.no-letsencrypt +++ b/deployment/data/nginx/app.conf.template.no-letsencrypt @@ -4,24 +4,6 @@ log_format custom_main '$remote_addr - $remote_user [$time_local] "$request" ' '"$http_user_agent" "$http_x_forwarded_for" ' 'rt=$request_time'; -# Map X-Forwarded-Proto or fallback to $scheme -map $http_x_forwarded_proto $forwarded_proto { - default $http_x_forwarded_proto; - "" $scheme; -} - -# Map X-Forwarded-Host or fallback to $host -map $http_x_forwarded_host $forwarded_host { - default $http_x_forwarded_host; - "" $host; -} - -# Map X-Forwarded-Port or fallback to server port -map $http_x_forwarded_port $forwarded_port { - default $http_x_forwarded_port; - "" $server_port; -} - upstream api_server { # fail_timeout=0 means we always retry an upstream even if it failed # to return a good HTTP response @@ -59,9 +41,10 @@ server { # misc headers proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; - proxy_set_header X-Forwarded-Proto $forwarded_proto; - proxy_set_header X-Forwarded-Host $forwarded_host; - proxy_set_header X-Forwarded-Port $forwarded_port; + # don't trust client-supplied X-Forwarded-* headers — use nginx's own values + proxy_set_header X-Forwarded-Proto $scheme; + proxy_set_header X-Forwarded-Host $host; + proxy_set_header X-Forwarded-Port $server_port; proxy_set_header Host $host; # need to use 1.1 to support chunked transfers @@ -78,9 +61,10 @@ server { # misc headers proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; - proxy_set_header X-Forwarded-Proto $forwarded_proto; - proxy_set_header X-Forwarded-Host $forwarded_host; - proxy_set_header X-Forwarded-Port $forwarded_port; + # don't trust client-supplied X-Forwarded-* headers — use nginx's own values + proxy_set_header X-Forwarded-Proto $scheme; + proxy_set_header X-Forwarded-Host $host; + proxy_set_header X-Forwarded-Port $server_port; proxy_set_header Host $host; proxy_http_version 1.1; diff --git a/deployment/data/nginx/app.conf.template.prod b/deployment/data/nginx/app.conf.template.prod index a57ea8ed04..320c743508 100644 --- a/deployment/data/nginx/app.conf.template.prod +++ b/deployment/data/nginx/app.conf.template.prod @@ -4,24 +4,6 @@ log_format custom_main '$remote_addr - $remote_user [$time_local] "$request" ' '"$http_user_agent" "$http_x_forwarded_for" ' 'rt=$request_time'; -# Map X-Forwarded-Proto or fallback to $scheme -map $http_x_forwarded_proto $forwarded_proto { - default $http_x_forwarded_proto; - "" $scheme; -} - -# Map X-Forwarded-Host or fallback to $host -map $http_x_forwarded_host $forwarded_host { - default $http_x_forwarded_host; - "" $host; -} - -# Map X-Forwarded-Port or fallback to server port -map $http_x_forwarded_port $forwarded_port { - default $http_x_forwarded_port; - "" $server_port; -} - upstream api_server { # fail_timeout=0 means we always retry an upstream even if it failed # to return a good HTTP response @@ -59,9 +41,10 @@ server { # misc headers proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; - proxy_set_header X-Forwarded-Proto $forwarded_proto; - proxy_set_header X-Forwarded-Host $forwarded_host; - proxy_set_header X-Forwarded-Port $forwarded_port; + # don't trust client-supplied X-Forwarded-* headers — use nginx's own values + proxy_set_header X-Forwarded-Proto $scheme; + proxy_set_header X-Forwarded-Host $host; + proxy_set_header X-Forwarded-Port $server_port; proxy_set_header Host $host; # need to use 1.1 to support chunked transfers @@ -83,9 +66,10 @@ server { # misc headers proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; - proxy_set_header X-Forwarded-Proto $forwarded_proto; - proxy_set_header X-Forwarded-Host $forwarded_host; - proxy_set_header X-Forwarded-Port $forwarded_port; + # don't trust client-supplied X-Forwarded-* headers — use nginx's own values + proxy_set_header X-Forwarded-Proto $scheme; + proxy_set_header X-Forwarded-Host $host; + proxy_set_header X-Forwarded-Port $server_port; proxy_set_header Host $host; proxy_http_version 1.1; diff --git a/web/src/app/auth/saml/callback/route.ts b/web/src/app/auth/saml/callback/route.ts index 0b6cb94351..a5003cd45e 100644 --- a/web/src/app/auth/saml/callback/route.ts +++ b/web/src/app/auth/saml/callback/route.ts @@ -17,16 +17,7 @@ async function handleSamlCallback( const fetchOptions: RequestInit = { method, - headers: { - "X-Forwarded-Host": - request.headers.get("X-Forwarded-Host") || - request.headers.get("host") || - "", - "X-Forwarded-Port": - request.headers.get("X-Forwarded-Port") || - new URL(request.url).port || - "", - }, + headers: {}, }; let relayState: string | null = null; diff --git a/web/src/lib/redirectSS.ts b/web/src/lib/redirectSS.ts index 5fa4bd1ccd..4240bb924c 100644 --- a/web/src/lib/redirectSS.ts +++ b/web/src/lib/redirectSS.ts @@ -1,23 +1,13 @@ import { NextRequest } from "next/server"; export const getDomain = (request: NextRequest) => { - // use env variable if set + // Use the WEB_DOMAIN env variable if set (required in production). + // Never trust X-Forwarded-* headers from the request — they can be + // spoofed by an attacker to poison redirect URLs (host header poisoning). if (process.env.WEB_DOMAIN) { return process.env.WEB_DOMAIN; } - // next, try and build domain from headers - const requestedHost = request.headers.get("X-Forwarded-Host"); - const requestedPort = request.headers.get("X-Forwarded-Port"); - const requestedProto = request.headers.get("X-Forwarded-Proto"); - if (requestedHost) { - const url = request.nextUrl.clone(); - url.host = requestedHost; - url.protocol = requestedProto || url.protocol; - url.port = requestedPort || url.port; - return url.origin; - } - - // finally just use whatever is in the request + // Fallback for local development: use Next.js's own origin. return request.nextUrl.origin; };