fix: enforce API key endpoint restrictions at the auth layer, not middleware (#23637)
The APIKeyRestrictionMiddleware only inspected the Authorization header for sk- tokens, but get_current_user also reads API keys from cookies and x-api-key headers. This allowed complete bypass of endpoint restrictions by sending the key via an alternate transport. Moves the restriction check into get_current_user_by_api_key so it runs regardless of how the API key was delivered. Removes the now-redundant middleware.
This commit is contained in:
@@ -1391,50 +1391,6 @@ app.add_middleware(RedirectMiddleware)
|
|||||||
app.add_middleware(SecurityHeadersMiddleware)
|
app.add_middleware(SecurityHeadersMiddleware)
|
||||||
|
|
||||||
|
|
||||||
class APIKeyRestrictionMiddleware:
|
|
||||||
def __init__(self, app):
|
|
||||||
self.app = app
|
|
||||||
|
|
||||||
async def __call__(self, scope, receive, send):
|
|
||||||
if scope['type'] == 'http':
|
|
||||||
request = Request(scope)
|
|
||||||
auth_header = request.headers.get('Authorization')
|
|
||||||
token = None
|
|
||||||
|
|
||||||
if auth_header:
|
|
||||||
parts = auth_header.split(' ', 1)
|
|
||||||
if len(parts) == 2:
|
|
||||||
token = parts[1]
|
|
||||||
|
|
||||||
# Only apply restrictions if an sk- API key is used
|
|
||||||
if token and token.startswith('sk-'):
|
|
||||||
# Check if restrictions are enabled
|
|
||||||
if app.state.config.ENABLE_API_KEYS_ENDPOINT_RESTRICTIONS:
|
|
||||||
allowed_paths = [
|
|
||||||
path.strip()
|
|
||||||
for path in str(app.state.config.API_KEYS_ALLOWED_ENDPOINTS).split(',')
|
|
||||||
if path.strip()
|
|
||||||
]
|
|
||||||
|
|
||||||
request_path = request.url.path
|
|
||||||
|
|
||||||
# Match exact path or prefix path
|
|
||||||
is_allowed = any(
|
|
||||||
request_path == allowed or request_path.startswith(allowed + '/') for allowed in allowed_paths
|
|
||||||
)
|
|
||||||
|
|
||||||
if not is_allowed:
|
|
||||||
await JSONResponse(
|
|
||||||
status_code=status.HTTP_403_FORBIDDEN,
|
|
||||||
content={'detail': 'API key not allowed to access this endpoint.'},
|
|
||||||
)(scope, receive, send)
|
|
||||||
return
|
|
||||||
|
|
||||||
await self.app(scope, receive, send)
|
|
||||||
|
|
||||||
|
|
||||||
app.add_middleware(APIKeyRestrictionMiddleware)
|
|
||||||
|
|
||||||
|
|
||||||
@app.middleware('http')
|
@app.middleware('http')
|
||||||
async def commit_session_after_request(request: Request, call_next):
|
async def commit_session_after_request(request: Request, call_next):
|
||||||
|
|||||||
@@ -427,6 +427,26 @@ async def get_current_user_by_api_key(request, api_key: str):
|
|||||||
):
|
):
|
||||||
raise HTTPException(status.HTTP_403_FORBIDDEN, detail=ERROR_MESSAGES.API_KEY_NOT_ALLOWED)
|
raise HTTPException(status.HTTP_403_FORBIDDEN, detail=ERROR_MESSAGES.API_KEY_NOT_ALLOWED)
|
||||||
|
|
||||||
|
# Enforce endpoint restrictions — checked here (not in middleware)
|
||||||
|
# so it applies regardless of how the API key was transported
|
||||||
|
# (Authorization header, cookie, x-api-key header, etc.).
|
||||||
|
if request.app.state.config.ENABLE_API_KEYS_ENDPOINT_RESTRICTIONS:
|
||||||
|
allowed_paths = [
|
||||||
|
path.strip()
|
||||||
|
for path in str(request.app.state.config.API_KEYS_ALLOWED_ENDPOINTS).split(',')
|
||||||
|
if path.strip()
|
||||||
|
]
|
||||||
|
request_path = request.url.path
|
||||||
|
is_allowed = any(
|
||||||
|
request_path == allowed or request_path.startswith(allowed + '/')
|
||||||
|
for allowed in allowed_paths
|
||||||
|
)
|
||||||
|
if not is_allowed:
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=status.HTTP_403_FORBIDDEN,
|
||||||
|
detail=ERROR_MESSAGES.ACCESS_PROHIBITED,
|
||||||
|
)
|
||||||
|
|
||||||
# Add user info to current span
|
# Add user info to current span
|
||||||
if ENABLE_OTEL:
|
if ENABLE_OTEL:
|
||||||
from opentelemetry import trace
|
from opentelemetry import trace
|
||||||
|
|||||||
Reference in New Issue
Block a user