diff --git a/airflow-core/src/airflow/api_fastapi/core_api/security.py b/airflow-core/src/airflow/api_fastapi/core_api/security.py index b24313319a9fc..39de4dd7e844d 100644 --- a/airflow-core/src/airflow/api_fastapi/core_api/security.py +++ b/airflow-core/src/airflow/api_fastapi/core_api/security.py @@ -19,7 +19,7 @@ from collections.abc import Callable from pathlib import Path from typing import TYPE_CHECKING, Annotated, cast -from urllib.parse import ParseResult, urljoin, urlparse +from urllib.parse import ParseResult, unquote, urljoin, urlparse from fastapi import Depends, HTTPException, Request, status from fastapi.security import HTTPBearer, OAuth2PasswordBearer @@ -623,7 +623,7 @@ def is_safe_url(target_url: str, request: Request | None = None) -> bool: return True for base_url, parsed_base in parsed_bases: - parsed_target = urlparse(urljoin(base_url, target_url)) # Resolves relative URLs + parsed_target = urlparse(urljoin(base_url, unquote(target_url))) # Resolves relative URLs target_path = Path(parsed_target.path).resolve() diff --git a/airflow-core/tests/unit/api_fastapi/core_api/test_security.py b/airflow-core/tests/unit/api_fastapi/core_api/test_security.py index b29803be92f25..bc38644c5c583 100644 --- a/airflow-core/tests/unit/api_fastapi/core_api/test_security.py +++ b/airflow-core/tests/unit/api_fastapi/core_api/test_security.py @@ -154,6 +154,12 @@ def test_requires_access_dag_unauthorized(self, mock_get_auth_manager): ("/some_other_page/", False), # traversal, escaping the `prefix` folder ("/../../../../some_page?with_param=3", False), + # encoded url + ("https%3A%2F%2Frequesting_server_base_url.com%2Fprefix2", True), + ("https%3A%2F%2Fserver_base_url.com%2Fprefix", True), + ("https%3A%2F%2Fsome_netlock.com%2Fprefix%2Fsome_page%3Fwith_param%3D3", False), + ("https%3A%2F%2Frequesting_server_base_url.com%2Fprefix2%2Fsub_path", True), + ("%2F..%2F..%2F..%2F..%2Fsome_page%3Fwith_param%3D3", False), ], ) @conf_vars({("api", "base_url"): "https://server_base_url.com/prefix"}) @@ -162,6 +168,23 @@ def test_is_safe_url(self, url, expected_is_safe): request.base_url = "https://requesting_server_base_url.com/prefix2" assert is_safe_url(url, request=request) == expected_is_safe + @pytest.mark.parametrize( + "url, expected_is_safe", + [ + ("https://server_base_url.com/prefix", False), + ("https://requesting_server_base_url.com/prefix2", True), + ("prefix/some_other", False), + ("https%3A%2F%2Fserver_base_url.com%2Fprefix", False), + ("https%3A%2F%2Frequesting_server_base_url.com%2Fprefix2", True), + ("https%3A%2F%2Frequesting_server_base_url.com%2Fprefix2%2Fsub_path", True), + ("%2F..%2F..%2F..%2F..%2Fsome_page%3Fwith_param%3D3", False), + ], + ) + def test_is_safe_url_with_base_url_unset(self, url, expected_is_safe): + request = Mock() + request.base_url = "https://requesting_server_base_url.com/prefix2" + assert is_safe_url(url, request=request) == expected_is_safe + @pytest.mark.db_test @pytest.mark.parametrize( "team_name",