diff --git a/app.py b/app.py index 4c006ada..e09616ab 100644 --- a/app.py +++ b/app.py @@ -6,7 +6,7 @@ from __future__ import annotations import os -from collections.abc import AsyncIterator +from collections.abc import AsyncGenerator from contextlib import asynccontextmanager import redis.asyncio as aioredis @@ -41,6 +41,7 @@ StaticCacheHeadersMiddleware, configure_cors, ) +from middleware.timeout import RequestTimeoutMiddleware from repositories.indexes import ensure_indexes from routes.api_v1 import router as api_v1_router from routes.auth import router as auth_router @@ -75,12 +76,15 @@ def create_app(settings: AppSettings | None = None) -> FastAPI: ) @asynccontextmanager - async def lifespan(app: FastAPI) -> AsyncIterator[None]: + async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: # ── Startup ────────────────────────────────────────────────────────── mongo_client: AsyncMongoClient = AsyncMongoClient( settings.db.mongodb_uri, maxPoolSize=settings.db.max_pool_size, minPoolSize=settings.db.min_pool_size, + serverSelectionTimeoutMS=settings.db.server_selection_timeout_ms, + connectTimeoutMS=settings.db.connect_timeout_ms, + socketTimeoutMS=settings.db.socket_timeout_ms, ) app.state.mongo_client = mongo_client app.state.db = mongo_client[settings.db.db_name] @@ -235,8 +239,12 @@ async def docs(request: Request): app.add_middleware( MaxContentLengthMiddleware, max_content_length=settings.max_content_length ) - # 5. Request logging — innermost, logs all requests with request_id + # 5. Request logging — logs all requests with request_id app.add_middleware(RequestLoggingMiddleware) + # 6. Request deadline — innermost + app.add_middleware( + RequestTimeoutMiddleware, timeout_seconds=settings.request_timeout_seconds + ) # ── Error handlers + rate limiter ──────────────────────────────────── app.state.limiter = limiter diff --git a/config.py b/config.py index 5e07a889..557c3bf4 100644 --- a/config.py +++ b/config.py @@ -23,6 +23,10 @@ class DatabaseSettings(BaseSettings): db_name: str = "url-shortener" max_pool_size: int = 200 min_pool_size: int = 10 + # socket_timeout sized for /metric aggregation; hot-path cap lives in RequestTimeoutMiddleware. + server_selection_timeout_ms: int = 3000 + connect_timeout_ms: int = 3000 + socket_timeout_ms: int = 120000 class RedisSettings(BaseSettings): @@ -148,6 +152,7 @@ class AppSettings(BaseSettings): max_active_api_keys: int = 20 max_date_range_days: int = 90 http_client_timeout: float = 5.0 + request_timeout_seconds: float = 8.0 # Validator constraints (overridable by self-hosters via env vars) blocked_url_regex_timeout: float = 0.2 @@ -167,7 +172,9 @@ def _must_be_positive_int(cls, v: int, info) -> int: raise ValueError(f"{info.field_name} must be >= 1, got {v}") return v - @field_validator("http_client_timeout", "blocked_url_regex_timeout") + @field_validator( + "http_client_timeout", "blocked_url_regex_timeout", "request_timeout_seconds" + ) @classmethod def _must_be_positive_float(cls, v: float, info) -> float: if v <= 0: diff --git a/config/apps.yaml b/config/apps.yaml index dfaacfbb..426064d4 100644 --- a/config/apps.yaml +++ b/config/apps.yaml @@ -78,10 +78,18 @@ apps: spoo-raycast: name: Raycast Extension icon: raycast.svg - description: Shorten links from Raycast + description: Shorten and manage your spoo.me links from Raycast verified: true - status: coming_soon + status: live type: device_auth + redirect_uris: + - https://raycast.com/redirect?* + links: + raycast: https://www.raycast.com/spoo-me/spoo + permissions: + - Access your spoo.me account + - Create and manage short URLs + - View and export your analytics spoo-vscode: name: VS Code Extension diff --git a/middleware/timeout.py b/middleware/timeout.py new file mode 100644 index 00000000..01c6aa22 --- /dev/null +++ b/middleware/timeout.py @@ -0,0 +1,43 @@ +"""Request deadline middleware — caps request time below the platform timeout.""" + +from __future__ import annotations + +import asyncio + +from starlette.middleware.base import BaseHTTPMiddleware, RequestResponseEndpoint +from starlette.requests import Request +from starlette.responses import JSONResponse, Response + +from infrastructure.logging import get_logger +from shared.ip_utils import get_client_ip + +log = get_logger("spoo.timeout") + +EXEMPT_PATHS: frozenset[str] = frozenset({"/metric"}) + + +class RequestTimeoutMiddleware(BaseHTTPMiddleware): + def __init__(self, app, timeout_seconds: float = 8.0) -> None: + super().__init__(app) + self._timeout = timeout_seconds + + async def dispatch( + self, request: Request, call_next: RequestResponseEndpoint + ) -> Response: + if request.url.path in EXEMPT_PATHS: + return await call_next(request) + + try: + return await asyncio.wait_for(call_next(request), timeout=self._timeout) + except asyncio.TimeoutError: + log.error( + "request_deadline_exceeded", + method=request.method, + path=request.url.path, + timeout_seconds=self._timeout, + client_ip=get_client_ip(request), + ) + return JSONResponse( + status_code=504, + content={"error": "Request timed out", "code": "request_timeout"}, + ) diff --git a/routes/auth/device.py b/routes/auth/device.py index 59c590c7..f1612742 100644 --- a/routes/auth/device.py +++ b/routes/auth/device.py @@ -67,11 +67,15 @@ def _device_error(request: Request, error: str, status_code: int = 400) -> Respo def _build_callback_redirect( - code: str, state: str, redirect_uri: str, app: AppEntry + code: str, + state: str, + redirect_uri: str, + app: AppEntry, + svc: DeviceAuthSvc, ) -> RedirectResponse: """Build the redirect to the callback page or a registered redirect_uri.""" params = urlencode({"code": code, "state": state}) - if redirect_uri and redirect_uri in app.redirect_uris: + if redirect_uri and svc.validate_redirect_uri(redirect_uri, app): separator = "&" if "?" in redirect_uri else "?" return RedirectResponse(f"{redirect_uri}{separator}{params}", status_code=302) return RedirectResponse(f"/auth/device/callback?{params}", status_code=302) @@ -122,7 +126,9 @@ async def device_login( code = await device_auth_service.create_device_auth_code( profile.id, profile.email, app_id=app_id ) - return _build_callback_redirect(code, state, redirect_uri, app) + return _build_callback_redirect( + code, state, redirect_uri, app, device_auth_service + ) # No grant: show consent screen csrf_token = generate_secure_token(_CSRF_TOKEN_BYTES) @@ -193,7 +199,9 @@ async def device_consent_approve( ) # Clear CSRF cookie and redirect - response = _build_callback_redirect(code, state, redirect_uri, app) + response = _build_callback_redirect( + code, state, redirect_uri, app, device_auth_service + ) response.delete_cookie(_CSRF_COOKIE_NAME) return response diff --git a/routes/health_routes.py b/routes/health_routes.py index fe1fcd39..c993e4b2 100644 --- a/routes/health_routes.py +++ b/routes/health_routes.py @@ -1,15 +1,8 @@ -""" -Health check endpoint. - -GET /health — checks MongoDB and Redis connectivity. -Rules: -- MongoDB failure → "unhealthy" (503) — the app cannot function without it. -- Redis failure or absence → "degraded" (200) — Redis is optional. -""" +"""GET /health — liveness probe.""" from __future__ import annotations -from fastapi import APIRouter, Request +from fastapi import APIRouter from fastapi.responses import JSONResponse from middleware.openapi import PUBLIC_SECURITY @@ -23,46 +16,6 @@ operation_id="healthCheck", summary="Health Check", ) -async def health_check(request: Request) -> JSONResponse: - """Check the health of the application and its dependencies. - - Pings MongoDB and Redis to determine overall system status: - - - **healthy** (200): Both MongoDB and Redis are reachable. - - **degraded** (200): MongoDB is reachable but Redis is down or not configured. - - **unhealthy** (503): MongoDB is unreachable -- the app cannot function. - - **Authentication**: Not required (public endpoint) - - **Rate Limits**: None - """ - checks: dict[str, str] = {} - overall = "healthy" - - try: - db = request.app.state.db - await db.client.admin.command("ping") - checks["mongodb"] = "ok" - except Exception: - checks["mongodb"] = "error" - overall = "unhealthy" - - redis = request.app.state.redis - if redis is None: - checks["redis"] = "not_configured" - if overall == "healthy": - overall = "degraded" - else: - try: - await redis.ping() - checks["redis"] = "ok" - except Exception: - checks["redis"] = "error" - if overall == "healthy": - overall = "degraded" - - status_code = 503 if overall == "unhealthy" else 200 - return JSONResponse( - status_code=status_code, - content={"status": overall, "checks": checks}, - ) +async def health_check() -> JSONResponse: + """Liveness probe. Public, unauthenticated, no dependency checks.""" + return JSONResponse(status_code=200, content={"status": "ok"}) diff --git a/routes/legacy/url_shortener.py b/routes/legacy/url_shortener.py index 96badd38..01716183 100644 --- a/routes/legacy/url_shortener.py +++ b/routes/legacy/url_shortener.py @@ -558,7 +558,7 @@ async def metric( async def query() -> dict: start = time.time() - cursor = await db["urls"].aggregate(METRIC_PIPELINE_V1) + cursor = await db["urls"].aggregate(METRIC_PIPELINE_V1, maxTimeMS=90000) results = await cursor.to_list(length=1) v1_result = results[0] if results else {} v1_shortlinks = v1_result.get("total-shortlinks", 0) diff --git a/schemas/dto/responses/common.py b/schemas/dto/responses/common.py index 5799c055..6915b23e 100644 --- a/schemas/dto/responses/common.py +++ b/schemas/dto/responses/common.py @@ -22,18 +22,10 @@ class ErrorResponse(ResponseBase): details: Any | None = None -class HealthChecks(ResponseBase): - """Individual service check statuses inside HealthResponse.""" - - mongodb: str - redis: str - - class HealthResponse(ResponseBase): - """Response body for GET /health.""" + """Response body for GET /health (liveness probe).""" status: str - checks: dict[str, str] class MessageResponse(ResponseBase): diff --git a/services/auth/device.py b/services/auth/device.py index ba678be5..a402e0f8 100644 --- a/services/auth/device.py +++ b/services/auth/device.py @@ -67,8 +67,22 @@ def resolve_app(self, app_id: str) -> AppEntry | None: return entry if entry and entry.is_live_device_app() else None def validate_redirect_uri(self, redirect_uri: str, app: AppEntry) -> bool: - """Return True if redirect_uri is empty or in the app's allowlist.""" - return not redirect_uri or redirect_uri in app.redirect_uris + """Return True if redirect_uri is empty, exact-matches an allowlist + entry, or prefix-matches an entry ending with ``*``. + + The ``*`` suffix is used for OAuth clients (e.g. Raycast) that append + a varying query string to a fixed redirect URL. Only exact or + explicit-wildcard entries are accepted; implicit wildcards are not. + """ + if not redirect_uri: + return True + for allowed in app.redirect_uris: + if allowed.endswith("*"): + if redirect_uri.startswith(allowed[:-1]): + return True + elif redirect_uri == allowed: + return True + return False async def create_device_auth_code( self, user_id: ObjectId, email: str, app_id: str | None = None diff --git a/tests/integration/test_device_auth.py b/tests/integration/test_device_auth.py index f4db93ee..481932c2 100644 --- a/tests/integration/test_device_auth.py +++ b/tests/integration/test_device_auth.py @@ -107,7 +107,13 @@ def device_auth_svc(): # resolve_app and validate_redirect_uri are sync — must not be coroutines svc.resolve_app = MagicMock(side_effect=_resolve_app) svc.validate_redirect_uri = MagicMock( - side_effect=lambda uri, app: not uri or uri in app.redirect_uris + side_effect=lambda uri, app: ( + not uri + or any( + uri.startswith(a[:-1]) if a.endswith("*") else uri == a + for a in app.redirect_uris + ) + ) ) return svc diff --git a/tests/integration/test_health.py b/tests/integration/test_health.py index f47f12e0..0d4c2dc2 100644 --- a/tests/integration/test_health.py +++ b/tests/integration/test_health.py @@ -1,8 +1,6 @@ """Integration tests for GET /health.""" import os -from contextlib import asynccontextmanager -from unittest.mock import AsyncMock, MagicMock from fastapi import FastAPI from fastapi.testclient import TestClient @@ -10,92 +8,27 @@ from middleware.error_handler import register_error_handlers from routes.health_routes import router as health_router -# Ensure a MONGODB_URI is present so AppSettings can be instantiated os.environ.setdefault("MONGODB_URI", "mongodb://localhost:27017/") -def _build_test_app( - mongo_ok: bool = True, - redis_ok: bool = True, - redis_configured: bool = True, -) -> FastAPI: - """ - Build a minimal FastAPI app with mocked DB/Redis injected via lifespan. - No real network connections are made. - """ - mock_db = MagicMock() - if mongo_ok: - mock_db.client.admin.command = AsyncMock(return_value={"ok": 1}) - else: - mock_db.client.admin.command = AsyncMock( - side_effect=Exception("connection refused") - ) - - if not redis_configured: - mock_redis = None - else: - mock_redis = AsyncMock() - if redis_ok: - mock_redis.ping = AsyncMock(return_value=True) - else: - mock_redis.ping = AsyncMock(side_effect=Exception("redis down")) - - @asynccontextmanager - async def lifespan(app: FastAPI): - app.state.db = mock_db - app.state.redis = mock_redis - yield - - app = FastAPI(lifespan=lifespan) +def _build_test_app() -> FastAPI: + app = FastAPI() register_error_handlers(app) app.include_router(health_router) return app class TestHealthEndpoint: - def test_healthy_when_both_ok(self): - app = _build_test_app(mongo_ok=True, redis_ok=True) - with TestClient(app) as client: - resp = client.get("/health") - assert resp.status_code == 200 - body = resp.json() - assert body["status"] == "healthy" - assert body["checks"]["mongodb"] == "ok" - assert body["checks"]["redis"] == "ok" - - def test_unhealthy_when_mongo_fails(self): - app = _build_test_app(mongo_ok=False, redis_ok=True) - with TestClient(app) as client: - resp = client.get("/health") - assert resp.status_code == 503 - body = resp.json() - assert body["status"] == "unhealthy" - assert body["checks"]["mongodb"] == "error" - - def test_degraded_when_redis_fails(self): - app = _build_test_app(mongo_ok=True, redis_ok=False) - with TestClient(app) as client: - resp = client.get("/health") - assert resp.status_code == 200 - body = resp.json() - assert body["status"] == "degraded" - assert body["checks"]["redis"] == "error" - - def test_degraded_when_redis_not_configured(self): - app = _build_test_app(mongo_ok=True, redis_configured=False) + def test_returns_ok(self): + app = _build_test_app() with TestClient(app) as client: resp = client.get("/health") assert resp.status_code == 200 - body = resp.json() - assert body["status"] == "degraded" - assert body["checks"]["redis"] == "not_configured" + assert resp.json() == {"status": "ok"} - def test_response_shape(self): + def test_no_db_dependency(self): + """Health must not touch app.state.db or app.state.redis.""" app = _build_test_app() with TestClient(app) as client: resp = client.get("/health") - body = resp.json() - assert "status" in body - assert "checks" in body - assert "mongodb" in body["checks"] - assert "redis" in body["checks"] + assert resp.status_code == 200 diff --git a/tests/smoke/test_startup.py b/tests/smoke/test_startup.py index 02f556c6..edb5fbb0 100644 --- a/tests/smoke/test_startup.py +++ b/tests/smoke/test_startup.py @@ -17,12 +17,10 @@ def test_app_boots_without_error(smoke_app: FastAPI) -> None: def test_health_endpoint_responds(smoke_client: TestClient) -> None: - """Health endpoint should return a response (mock DB will show unhealthy, but no crash).""" + """Liveness probe — always 200, no dependency checks.""" resp = smoke_client.get("/health") - assert resp.status_code in (200, 503) - data = resp.json() - assert "status" in data - assert "checks" in data + assert resp.status_code == 200 + assert resp.json() == {"status": "ok"} def test_app_title_and_version(smoke_app: FastAPI) -> None: diff --git a/tests/unit/schemas/dto/test_common.py b/tests/unit/schemas/dto/test_common.py index a8abc254..9d8c5254 100644 --- a/tests/unit/schemas/dto/test_common.py +++ b/tests/unit/schemas/dto/test_common.py @@ -31,7 +31,6 @@ def test_without_optional_fields(self): class TestHealthResponse: def test_serialization(self): - r = HealthResponse(status="healthy", checks={"mongodb": "ok", "redis": "ok"}) + r = HealthResponse(status="ok") d = r.model_dump() - assert d["status"] == "healthy" - assert d["checks"]["mongodb"] == "ok" + assert d["status"] == "ok" diff --git a/tests/unit/services/test_redirect_uri.py b/tests/unit/services/test_redirect_uri.py new file mode 100644 index 00000000..af6a621f --- /dev/null +++ b/tests/unit/services/test_redirect_uri.py @@ -0,0 +1,77 @@ +"""Unit tests for wildcard redirect URI matching.""" + +from __future__ import annotations + +from schemas.models.app import AppEntry +from services.auth.device import DeviceAuthService + + +def _app(redirect_uris: list[str]) -> AppEntry: + return AppEntry(name="test", description="test app", redirect_uris=redirect_uris) + + +class TestValidateRedirectUri: + """Tests for DeviceAuthService.validate_redirect_uri.""" + + def setup_method(self): + self.svc = DeviceAuthService.__new__(DeviceAuthService) + + def test_empty_uri_allowed(self): + app = _app(["https://example.com/callback"]) + assert self.svc.validate_redirect_uri("", app) is True + + def test_exact_match(self): + app = _app(["https://example.com/callback"]) + assert ( + self.svc.validate_redirect_uri("https://example.com/callback", app) is True + ) + + def test_exact_no_match(self): + app = _app(["https://example.com/callback"]) + assert self.svc.validate_redirect_uri("https://evil.com/callback", app) is False + + def test_wildcard_query_match(self): + app = _app(["https://raycast.com/redirect?*"]) + assert ( + self.svc.validate_redirect_uri( + "https://raycast.com/redirect?packageName=spoo&state=abc", app + ) + is True + ) + + def test_wildcard_prefix_only(self): + app = _app(["https://raycast.com/redirect?*"]) + assert ( + self.svc.validate_redirect_uri("https://raycast.com/redirect?", app) is True + ) + + def test_wildcard_rejects_different_path(self): + app = _app(["https://raycast.com/redirect?*"]) + assert ( + self.svc.validate_redirect_uri("https://raycast.com/redirected", app) + is False + ) + + def test_wildcard_rejects_different_host(self): + app = _app(["https://raycast.com/redirect?*"]) + assert ( + self.svc.validate_redirect_uri("https://evil.com/redirect?foo=bar", app) + is False + ) + + def test_no_uris_rejects(self): + app = _app([]) + assert self.svc.validate_redirect_uri("https://example.com", app) is False + + def test_multiple_uris_mixed(self): + app = _app(["https://a.com/cb", "https://b.com/redirect?*"]) + assert self.svc.validate_redirect_uri("https://a.com/cb", app) is True + assert self.svc.validate_redirect_uri("https://b.com/redirect?x=1", app) is True + assert self.svc.validate_redirect_uri("https://c.com/cb", app) is False + + def test_bare_star_matches_everything(self): + """A bare '*' entry matches any URI — intentional if configured.""" + app = _app(["*"]) + assert ( + self.svc.validate_redirect_uri("https://anything.com/whatever", app) is True + )