From 8cdc7e5887725144da082bc1ff1316eec137a3d5 Mon Sep 17 00:00:00 2001 From: Chandrasekharan M Date: Fri, 10 Jul 2026 11:03:27 +0530 Subject: [PATCH] fix: preserve view function identity in platform-service auth middleware `authentication_middleware` returned an unwrapped closure, so every decorated view function's `__name__` was "wrapper". Flask derives an endpoint name from `__name__`, which meant: - Six routes carried a redundant `endpoint="..."` kwarg purely to dodge the resulting name collision. - `/usage`, the one authenticated route without that kwarg, registered as endpoint `platform.wrapper`. - Adding any new authenticated route without `endpoint=` would fail at import with "View function mapping is overwriting an existing endpoint function: platform.wrapper". Add `functools.wraps` and drop the now-redundant `endpoint=` kwargs. URL rules and methods are unchanged; only the internal Flask endpoint names change (`platform.wrapper` -> `platform.usage`). Nothing in the repo resolves routes via `url_for`, and callers reach platform-service over HTTP by path, so this is not externally observable. x2text-service's equivalent middleware already patched `wrapper.__name__` by hand; this brings platform-service in line. Also repair `tests/test_auth_middleware.py`, which imported `unstract.platform_service.main` (removed) and `get_account_from_bearer_token` (removed) and so broke collection of the entire platform-service suite. It now covers `validate_bearer_token` against a mocked cursor. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_014gUmgQQ7xV73qn6kcB7mXG --- .../platform_service/controller/platform.py | 38 ++++-------- platform-service/tests/conftest.py | 6 ++ .../tests/test_auth_middleware.py | 58 +++++++++++++++++++ .../tests/test_route_endpoints.py | 18 ++++++ 4 files changed, 93 insertions(+), 27 deletions(-) create mode 100644 platform-service/tests/conftest.py create mode 100644 platform-service/tests/test_auth_middleware.py create mode 100644 platform-service/tests/test_route_endpoints.py diff --git a/platform-service/src/unstract/platform_service/controller/platform.py b/platform-service/src/unstract/platform_service/controller/platform.py index 85f759e209..fbc8cdd8bb 100644 --- a/platform-service/src/unstract/platform_service/controller/platform.py +++ b/platform-service/src/unstract/platform_service/controller/platform.py @@ -1,3 +1,4 @@ +import functools import json import uuid from datetime import datetime @@ -33,6 +34,9 @@ def get_token_from_auth_header(request: Request) -> Any: def authentication_middleware(func: Any) -> Any: + # Flask names endpoints after the view's __name__; without wraps every + # decorated route registers as "wrapper" and collides. + @functools.wraps(func) def wrapper(*args: Any, **kwargs: Any) -> Any: token = get_token_from_auth_header(request) # Check if bearer token exists and validate it @@ -111,7 +115,7 @@ def validate_bearer_token(token: str | None) -> bool: return False -@platform_bp.route("/page-usage", methods=["POST"], endpoint="page_usage") +@platform_bp.route("/page-usage", methods=["POST"]) @authentication_middleware def page_usage() -> Any: """Usage endpoint.""" @@ -262,11 +266,7 @@ def usage() -> Any: return make_response(result, 500) -@platform_bp.route( - "/platform_details", - methods=["GET"], - endpoint="platform_details", -) +@platform_bp.route("/platform_details", methods=["GET"]) @authentication_middleware def platform_details() -> Any: """Fetch details associated with the platform. This uses the authenticated @@ -288,7 +288,7 @@ def platform_details() -> Any: return result, 200 -@platform_bp.route("/cache", methods=["POST", "GET", "DELETE"], endpoint="cache") +@platform_bp.route("/cache", methods=["POST", "GET", "DELETE"]) @authentication_middleware def cache() -> Any: """Cache endpoint. @@ -348,11 +348,7 @@ def cache() -> Any: return "OK", 200 -@platform_bp.route( - "/adapter_instance", - methods=["GET"], - endpoint="adapter_instance", -) +@platform_bp.route("/adapter_instance", methods=["GET"]) @authentication_middleware def adapter_instance() -> Any: """Fetch Adapter instance. @@ -399,11 +395,7 @@ def adapter_instance() -> Any: raise APIError(message=msg) -@platform_bp.route( - "/custom_tool_instance", - methods=["GET"], - endpoint="custom_tool_instance", -) +@platform_bp.route("/custom_tool_instance", methods=["GET"]) @authentication_middleware def custom_tool_instance() -> Any: """Fetching exported custom tool instance. @@ -438,11 +430,7 @@ def custom_tool_instance() -> Any: raise APIError(message=msg) from e -@platform_bp.route( - "/agentic_tool_instance", - methods=["GET"], - endpoint="agentic_tool_instance", -) +@platform_bp.route("/agentic_tool_instance", methods=["GET"]) @authentication_middleware def agentic_tool_instance() -> Any: """Fetching exported agentic tool instance. @@ -479,11 +467,7 @@ def agentic_tool_instance() -> Any: raise APIError(message=msg) from e -@platform_bp.route( - "/llm_profile_instance", - methods=["GET"], - endpoint="llm_profile_instance", -) +@platform_bp.route("/llm_profile_instance", methods=["GET"]) @authentication_middleware def llm_profile_instance() -> Any: """Fetching llm profile instance.""" diff --git a/platform-service/tests/conftest.py b/platform-service/tests/conftest.py new file mode 100644 index 0000000000..04b6803b34 --- /dev/null +++ b/platform-service/tests/conftest.py @@ -0,0 +1,6 @@ +import os + +# platform_service.env validates required settings at import time. +os.environ.setdefault("REDIS_HOST", "localhost") +os.environ.setdefault("ENCRYPTION_KEY", "test-key") +os.environ.setdefault("DB_SCHEMA", "unstract") diff --git a/platform-service/tests/test_auth_middleware.py b/platform-service/tests/test_auth_middleware.py new file mode 100644 index 0000000000..28cb0d0e39 --- /dev/null +++ b/platform-service/tests/test_auth_middleware.py @@ -0,0 +1,58 @@ +"""Tests for bearer token validation used by `authentication_middleware`.""" + +from collections.abc import Iterator +from contextlib import contextmanager +from typing import Any + +import pytest +from flask import Flask +from unstract.platform_service.controller import platform + + +@pytest.fixture +def app_ctx() -> Iterator[None]: + """`validate_bearer_token` logs via `flask.current_app`.""" + with Flask(__name__).app_context(): + yield + + +def _mock_safe_cursor(monkeypatch: pytest.MonkeyPatch, result_row: Any) -> None: + class _Cursor: + def fetchone(self) -> Any: + return result_row + + @contextmanager + def _safe_cursor(query: str, params: tuple = ()) -> Iterator[_Cursor]: + yield _Cursor() + + monkeypatch.setattr(platform, "safe_cursor", _safe_cursor) + + +def test_valid_active_token(app_ctx: None, monkeypatch: pytest.MonkeyPatch) -> None: + _mock_safe_cursor(monkeypatch, ("id", "test-token", True)) + assert platform.validate_bearer_token("test-token") is True + + +def test_rejects_missing_token(app_ctx: None) -> None: + assert platform.validate_bearer_token(None) is False + + +def test_rejects_unknown_token(app_ctx: None, monkeypatch: pytest.MonkeyPatch) -> None: + _mock_safe_cursor(monkeypatch, None) + assert platform.validate_bearer_token("test-token") is False + + +def test_rejects_inactive_token(app_ctx: None, monkeypatch: pytest.MonkeyPatch) -> None: + _mock_safe_cursor(monkeypatch, ("id", "test-token", False)) + assert platform.validate_bearer_token("test-token") is False + + +def test_db_error_denies_access(app_ctx: None, monkeypatch: pytest.MonkeyPatch) -> None: + """Auth must fail closed when the token lookup blows up.""" + + # Raising on call suffices: `safe_cursor(...)` is invoked inside the try. + def _boom(query: str, params: tuple = ()) -> Any: + raise RuntimeError("db down") + + monkeypatch.setattr(platform, "safe_cursor", _boom) + assert platform.validate_bearer_token("test-token") is False diff --git a/platform-service/tests/test_route_endpoints.py b/platform-service/tests/test_route_endpoints.py new file mode 100644 index 0000000000..f0b1725258 --- /dev/null +++ b/platform-service/tests/test_route_endpoints.py @@ -0,0 +1,18 @@ +"""Guards that `authentication_middleware` stays transparent to Flask. + +Without `functools.wraps` every decorated route registers as "wrapper", so the +second such route raises at import time. +""" + +from flask import Flask +from unstract.platform_service.controller.platform import platform_bp + + +def test_routes_register_under_their_function_names() -> None: + # NOSONAR — app is never served; bearer-token routes carry no CSRF surface. + app = Flask(__name__) # NOSONAR + app.register_blueprint(platform_bp) + + endpoints = {rule.endpoint for rule in app.url_map.iter_rules()} + assert "platform.wrapper" not in endpoints + assert "platform.usage" in endpoints