Skip to content

Commit eff49db

Browse files
committed
feat: secure IP detection and configurable reverse proxy support
- Add `UVICORN_PROXY_HEADERS` and `UVICORN_FORWARDED_ALLOW_IPS` settings. - Configure Uvicorn and middleware to respect these settings (defaulting to restricted access). - Refactor `get_client_ip` to rely on trusted `request.client.host` instead of manual header parsing. - Prevent IP spoofing by removing unverified `X-Forwarded-For` trust. - Add comprehensive tests for IP detection and middleware behavior
1 parent ab552f9 commit eff49db

6 files changed

Lines changed: 108 additions & 13 deletions

File tree

app/middlewares/__init__.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
from uvicorn.middleware.proxy_headers import ProxyHeadersMiddleware
44

55
from app.utils.logger import get_logger
6-
from config import cors_settings
6+
from config import cors_settings, server_settings
77

88
from .request_logging import RequestProcessTimeLoggingMiddleware
99

@@ -16,5 +16,6 @@ def setup_middleware(app: FastAPI):
1616
allow_methods=["*"],
1717
allow_headers=["*"],
1818
)
19-
app.add_middleware(ProxyHeadersMiddleware, trusted_hosts="*")
19+
if server_settings.proxy_headers:
20+
app.add_middleware(ProxyHeadersMiddleware, trusted_hosts=server_settings.forwarded_allow_ips)
2021
app.add_middleware(RequestProcessTimeLoggingMiddleware, access_logger=get_logger("uvicorn.access"))

app/routers/admin.py

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,20 +12,19 @@
1212
AdminListQuery,
1313
AdminModify,
1414
AdminSimpleListQuery,
15-
Token,
16-
AdminUsageQuery,
1715
AdminsResponse,
1816
AdminsSimpleResponse,
17+
AdminUsageQuery,
1918
BulkAdminsActionResponse,
2019
BulkAdminSelection,
2120
RemoveAdminsResponse,
21+
Token,
2222
)
2323
from app.models.stats import UserUsageStatsList
2424
from app.operation import OperatorType
2525
from app.operation.admin import AdminOperation
2626
from app.utils import responses
2727
from app.utils.jwt import create_admin_token
28-
from .dependencies import get_admin_list_query, get_admin_simple_list_query, get_admin_usage_query
2928

3029
from .authentication import (
3130
check_sudo_admin,
@@ -34,16 +33,14 @@
3433
validate_admin,
3534
validate_mini_app_admin,
3635
)
36+
from .dependencies import get_admin_list_query, get_admin_simple_list_query, get_admin_usage_query
3737

3838
router = APIRouter(tags=["Admin"], prefix="/api/admin", responses={401: responses._401, 403: responses._403})
3939
admin_operator = AdminOperation(operator_type=OperatorType.API)
4040

4141

4242
def get_client_ip(request: Request) -> str:
43-
"""Extract the client's IP address from the request headers or client."""
44-
forwarded_for = request.headers.get("X-Forwarded-For")
45-
if forwarded_for:
46-
return forwarded_for.split(",")[0].strip()
43+
"""Extract the client's IP address from the request."""
4744
if request.client:
4845
return request.client.host
4946
return "Unknown"

config.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ class ServerSettings(EnvSettings):
5353
ssl_ca_type: str = Field(default="public", validation_alias="UVICORN_SSL_CA_TYPE")
5454
workers: int = Field(default=1, validation_alias="UVICORN_WORKERS")
5555
loop: str = Field(default="auto", validation_alias="UVICORN_LOOP")
56+
proxy_headers: bool = Field(default=False, validation_alias="UVICORN_PROXY_HEADERS")
57+
forwarded_allow_ips: str | list[str] = Field(default="127.0.0.1", validation_alias="UVICORN_FORWARDED_ALLOW_IPS")
5658

5759
@field_validator("ssl_ca_type")
5860
@classmethod

main.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,8 @@
99
from cryptography.hazmat.backends import default_backend
1010

1111
from app import create_app # noqa: F401
12-
from app.utils.logger import get_logger
1312
from app.nats import require_nats_if_multiworker
14-
from app.utils.logger import LOGGING_CONFIG
13+
from app.utils.logger import LOGGING_CONFIG, get_logger
1514
from config import logging_settings, runtime_settings, server_settings
1615

1716
logger = get_logger("uvicorn-main")
@@ -130,7 +129,7 @@ def validate_cert_and_key(cert_file_path, key_file_path, ca_type: str = "public"
130129
131130
If you need external access, please provide the SSL files to allow the server to bind to 0.0.0.0. Alternatively, you can run the server on localhost or a Unix socket and use a reverse proxy, such as Nginx or Caddy, to handle SSL termination and provide external access.
132131
133-
If you wish to continue without SSL, you can use SSH port forwarding to access the application from your machine. note that in this case, subscription functionality will not work.
132+
If you wish to continue without SSL, you can use SSH port forwarding to access the application from your machine. note that in this case, subscription functionality will not work.
134133
135134
Use the following command:
136135
@@ -160,6 +159,8 @@ def validate_cert_and_key(cert_file_path, key_file_path, ca_type: str = "public"
160159
log_config=LOGGING_CONFIG,
161160
log_level=effective_log_level.lower(),
162161
loop=server_settings.loop,
162+
proxy_headers=server_settings.proxy_headers,
163+
forwarded_allow_ips=server_settings.forwarded_allow_ips,
163164
)
164165
except FileNotFoundError: # to prevent error on removing unix sock
165166
pass

tests/api/test_ip_detection.py

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
from fastapi import FastAPI, Request
2+
from uvicorn.middleware.proxy_headers import ProxyHeadersMiddleware
3+
4+
from app.middlewares import setup_middleware
5+
from app.routers.admin import get_client_ip
6+
from config import server_settings
7+
from tests.api import client
8+
9+
10+
def test_get_client_ip_no_proxy():
11+
"""Test that get_client_ip returns the direct client host when no proxy is involved."""
12+
# Use a real Request object with a minimal ASGI scope
13+
scope = {
14+
"type": "http",
15+
"client": ("1.1.1.1", 12345),
16+
"headers": [],
17+
}
18+
request = Request(scope=scope)
19+
assert get_client_ip(request) == "1.1.1.1"
20+
21+
22+
def test_get_client_ip_with_proxy_middleware_behavior(access_token):
23+
"""
24+
Test the behavior of IP detection via the TestClient.
25+
Since we enabled proxy_headers in conftest.py, the middleware should process X-Forwarded-For.
26+
"""
27+
28+
# We use an endpoint that returns the IP, like /api/admin/token (indirectly via notification)
29+
# or we can check a subscription info endpoint which also uses request.client.host
30+
31+
# In tests/conftest.py we set:
32+
# server_settings.proxy_headers = True
33+
# server_settings.forwarded_allow_ips = "*"
34+
35+
# This means X-Forwarded-For should be trusted.
36+
ip = "203.0.113.10"
37+
# We use a known endpoint that returns IP in response for easy verification
38+
# Based on grep, user_subscription_info returns IP
39+
40+
from tests.api.helpers import (
41+
create_core,
42+
create_group,
43+
create_user,
44+
delete_core,
45+
delete_group,
46+
delete_user,
47+
)
48+
49+
core = create_core(access_token)
50+
group = create_group(access_token)
51+
user = create_user(
52+
access_token,
53+
group_ids=[group["id"]],
54+
payload={"username": "iptestuser"},
55+
)
56+
try:
57+
response = client.get(f"{user['subscription_url']}/info", headers={"X-Forwarded-For": ip})
58+
assert response.status_code == 200
59+
# The subscription router uses request.client.host
60+
# If ProxyHeadersMiddleware is working, it will swap request.client.host with the value from X-Forwarded-For
61+
assert response.json()["ip"] == ip
62+
finally:
63+
delete_user(access_token, "iptestuser")
64+
delete_group(access_token, group["id"])
65+
delete_core(access_token, core["id"])
66+
67+
68+
def test_proxy_headers_disabled_logic():
69+
"""
70+
Verify that if proxy_headers was False, the middleware wouldn't be added.
71+
This is a logic check on the middleware setup.
72+
"""
73+
74+
app = FastAPI()
75+
original_val = server_settings.proxy_headers
76+
try:
77+
server_settings.proxy_headers = False
78+
setup_middleware(app)
79+
80+
# Check if ProxyHeadersMiddleware is in the app.user_middleware list
81+
# Note: Middleware are wrapped, but we can check the classes
82+
middleware_classes = [m.cls for m in app.user_middleware]
83+
assert ProxyHeadersMiddleware not in middleware_classes
84+
85+
# Now enable it
86+
app_with_proxy = FastAPI()
87+
server_settings.proxy_headers = True
88+
setup_middleware(app_with_proxy)
89+
middleware_classes_proxy = [m.cls for m in app_with_proxy.user_middleware]
90+
assert ProxyHeadersMiddleware in middleware_classes_proxy
91+
finally:
92+
server_settings.proxy_headers = original_val

tests/conftest.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,12 @@
1010
sys.path.insert(0, project_root)
1111

1212
# Override settings for tests
13-
from config import auth_settings, runtime_settings # noqa: E402
13+
from config import auth_settings, runtime_settings, server_settings # noqa: E402
1414

1515
runtime_settings.testing = True
1616
runtime_settings.debug = True
17+
server_settings.proxy_headers = True
18+
server_settings.forwarded_allow_ips = "*"
1719
auth_settings.sudoers["testadmin"] = "testadmin"
1820

1921

0 commit comments

Comments
 (0)