Feat/Feedback ingest proxy, post feedback to a configured location

.bumpversion.toml

@@ -1,5 +1,5 @@
 [tool.bumpversion]
-current_version = "1.19.2+260912"
+current_version = "1.20.0+260913"
 parse = "(?P<major>\\d+)\\.(?P<minor>\\d+)\\.(?P<patch>\\d+)\\+(?P<build>\\d+)"
 serialize = ["{major}.{minor}.{patch}+{build}"]
 search = "{current_version}"

README.rst

@@ -64,6 +64,18 @@ You can find all available variables here https://github.com/pvarki/python-rasen
    * - RM_SQLITE_FILEPATH_DEV
      - /tmp/rm_db.sql
      - location for sqlite database file in "dev", local development
+   * - RM_FEEDBACK_INGEST_URL
+     - None
+     - Where user feedback is POSTed, operator defined. Unset disables the feedback endpoint
+   * - RM_FEEDBACK_INGEST_KEY
+     - None
+     - Key sent with the feedback POST. Unset disables the feedback endpoint
+   * - RM_FEEDBACK_INGEST_KEY_HEADER
+     - Authorization
+     - Header carrying the key, value sent verbatim so "Bearer xyz" works
+   * - RM_FEEDBACK_TIMEOUT
+     - 5.0
+     - Seconds to wait for the feedback ingest
 
 
 You can create `.env` file in the root directory and place all

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "rasenmaeher_api"
-version = "1.19.2+260912"
+version = "1.20.0+260913"
 description = "python-rasenmaeher-api"
 authors = [
     { name = "Aciid", email = "703382+Aciid@users.noreply.github.com" },

src/rasenmaeher_api/__init__.py

@@ -1,3 +1,3 @@
 """python-rasenmaeher-api"""
 
-__version__ = "1.19.2+260912"  # NOTE Use `bump-my-version` to bump versions correctly
+__version__ = "1.20.0+260913"  # NOTE Use `bump-my-version` to bump versions correctly

src/rasenmaeher_api/rmsettings.py

@@ -143,6 +143,14 @@ def log_level_int(self) -> int:
     kc_realm: str = "RASENMAEHER"  # In which realm the real users are
     kc_enabled: bool = True  # Whether to use KC or not (mainly so that unit tests have less dependencies for now)
 
+    # Feedback ingest, the operator decides where user feedback is delivered.
+    # Unset url or key disables the endpoint (it will answer 503).
+    feedback_ingest_url: str | None = None
+    feedback_ingest_key: str | None = None
+    # Header carrying the key, value is sent verbatim so eg. "Bearer xyz" works
+    feedback_ingest_key_header: str = "Authorization"
+    feedback_timeout: float = 5.0
+
     # Enrollment code generation related
     code_size: int = 8
     code_avoid_confusion: bool = True  # Replace 1 and 0 with O and I to avoid confusion

src/rasenmaeher_api/web/api/feedback/__init__.py

@@ -0,0 +1,5 @@
+"""Feedback API."""
+
+from rasenmaeher_api.web.api.feedback.views import router
+
+__all__ = ["router"]

src/rasenmaeher_api/web/api/feedback/schema.py

@@ -0,0 +1,41 @@
+"""Schema for feedback."""
+
+from pydantic import BaseModel, ConfigDict, Field
+
+
+class FeedbackIn(BaseModel):
+    """Feedback submitted by a user from the app's feedback dialog."""
+
+    model_config = ConfigDict(
+        extra="forbid",
+        json_schema_extra={
+            "examples": [
+                {
+                    "role": "admin",
+                    "os": "macos",
+                    "rating": "good",
+                    "comments": "Works great, minor UI nit on mobile.",
+                    "version": "2.6.0+260729",
+                    "web_address": "https://example.pvarki.fi/app/settings",
+                }
+            ]
+        },
+    )
+
+    rating: str = Field(max_length=64, description="User's rating selection")
+    comments: str = Field(max_length=5000, description="Free-form feedback text")
+    role: str | None = Field(default=None, max_length=64, description="User type/role reported by the client")
+    os: str | None = Field(default=None, max_length=64, description="Client OS")
+    version: str | None = Field(default=None, max_length=64, description="Frontend build version")
+    web_address: str | None = Field(default=None, max_length=2048, description="Page the feedback was sent from")
+
+
+class FeedbackOut(BaseModel):
+    """Result of forwarding feedback to the configured ingest."""
+
+    model_config = ConfigDict(
+        extra="forbid",
+        json_schema_extra={"examples": [{"ok": True}]},
+    )
+
+    ok: bool = Field(description="Whether the feedback was accepted and forwarded")

src/rasenmaeher_api/web/api/feedback/views.py

@@ -0,0 +1,113 @@
+"""Feedback API views.
+
+Proxies the app's feedback dialog to the ingest endpoint the operator configured,
+so that the ingest URL and key stay server-side and are never shipped in the
+frontend bundle. RASENMAEHER does not care what the far end is, it only has to
+accept a JSON POST.
+"""
+
+import logging
+import os
+from uuid import uuid4
+
+import aiohttp
+from fastapi import APIRouter, Depends, HTTPException, Request
+
+from ....db.people import Person
+from ....rmsettings import RMSettings
+from ..middleware.user import ValidUser
+from ..utils.auditcontext import build_audit_extra, get_audit_request_context
+from .schema import FeedbackIn, FeedbackOut
+
+router = APIRouter()
+LOGGER = logging.getLogger(__name__)
+
+
+def _deployment_version() -> str:
+    """Release tag of this deployment, same values the healthcheck reports."""
+    version = os.environ.get("RELEASE_TAG", "undefined")
+    if release_status := os.environ.get("RELEASE_STATUS", ""):
+        version += f"-{release_status}"
+    return version
+
+
+@router.post("")
+async def submit_feedback(
+    feedback: FeedbackIn,
+    request: Request,
+    person: Person | None = Depends(ValidUser()),
+) -> FeedbackOut:
+    """Forward a feedback submission to the configured ingest."""
+    conf = RMSettings.singleton()
+    if not conf.feedback_ingest_url or not conf.feedback_ingest_key:
+        LOGGER.error("Feedback ingest is not configured (RM_FEEDBACK_INGEST_URL / RM_FEEDBACK_INGEST_KEY missing)")
+        raise HTTPException(status_code=503, detail="Feedback submission is not configured")
+
+    source_ip = get_audit_request_context(request).get("source.ip", "unknown")
+    callsign = person.callsign if person else None
+    # Which deployment and release this came from is resolved here rather than trusted
+    # from the client, the browser has no business asserting it.
+    deployment = conf.deployment_name
+
+    # Not every ingest renders the structured keys, so the triage metadata is also folded
+    # into the message body. Keep both, the duplication is cheaper than a lost bug report.
+    metadata_lines = [
+        f"{label}: {value}"
+        for label, value in (
+            ("role", feedback.role),
+            ("os", feedback.os),
+            ("version", feedback.version),
+            ("deployment", deployment),
+            ("page", feedback.web_address),
+        )
+        if value
+    ]
+    message = feedback.comments
+    if metadata_lines:
+        message = f"{feedback.comments}\n\n" + "\n".join(metadata_lines)
+
+    payload = {
+        "id": str(uuid4()),
+        "subject": f"{deployment} feedback: {feedback.rating}",
+        "message": message,
+        "name": callsign,
+        "rating": feedback.rating,
+        "role": feedback.role,
+        "os": feedback.os,
+        "version": feedback.version,
+        "deployment": deployment,
+        "deployment_version": _deployment_version(),
+        "web_address": feedback.web_address,
+    }
+
+    try:
+        async with aiohttp.ClientSession(timeout=aiohttp.ClientTimeout(total=conf.feedback_timeout)) as session:
+            headers = {
+                "content-type": "application/json",
+                conf.feedback_ingest_key_header: conf.feedback_ingest_key,
+                "x-forwarded-for": source_ip,
+            }
+            async with session.post(conf.feedback_ingest_url, json=payload, headers=headers) as response:
+                if response.status == 429:
+                    LOGGER.warning("Feedback ingest rate-limited the submission")
+                    raise HTTPException(status_code=429, detail="Too many feedback submissions, try again shortly")
+                response.raise_for_status()
+    except aiohttp.ClientError as exc:
+        LOGGER.error(
+            "Failed to forward feedback to ingest: %s",
+            exc,
+            extra=build_audit_extra(action="feedback.submit", outcome="failure", actor=callsign, request=request),
+        )
+        raise HTTPException(status_code=502, detail="Could not submit feedback, please try again") from exc
+    except TimeoutError as exc:
+        LOGGER.error(
+            "Timed out forwarding feedback to ingest",
+            extra=build_audit_extra(action="feedback.submit", outcome="failure", actor=callsign, request=request),
+        )
+        raise HTTPException(status_code=502, detail="Could not submit feedback, please try again") from exc
+
+    LOGGER.audit(  # type: ignore[attr-defined]
+        "Feedback submitted",
+        extra=build_audit_extra(action="feedback.submit", outcome="success", actor=callsign, request=request),
+    )
+    return FeedbackOut(ok=True)

src/rasenmaeher_api/web/api/router.py

@@ -8,6 +8,7 @@
     descriptions,
     enduserpfx,
     enrollment,
+    feedback,
     firstuser,
     healthcheck,
     instructions,
@@ -32,6 +33,7 @@
 api_router.include_router(people.router, prefix="/people", tags=["people"])
 api_router.include_router(descriptions.router, prefix="/descriptions", tags=["descriptions"])
 api_router.include_router(internal.router, prefix="/internal", tags=["internal"])
+api_router.include_router(feedback.router, prefix="/feedback", tags=["feedback"])
 
 api_router_v2 = APIRouter()
 api_router_v2.include_router(descriptions.router_v2, prefix="/descriptions", tags=["descriptions"])

tests/test_feedback.py

@@ -0,0 +1,142 @@
+"""Test feedback proxy endpoint"""
+
+import logging
+from types import TracebackType
+from typing import Any, ClassVar, Self
+from unittest.mock import patch
+
+import aiohttp
+import pytest
+from async_asgi_testclient import TestClient  # type: ignore[import-untyped]
+
+from rasenmaeher_api.rmsettings import RMSettings
+
+LOGGER = logging.getLogger(__name__)
+
+VALID_FEEDBACK = {
+    "rating": "good",
+    "comments": "Works fine",
+    "role": "admin",
+    "os": "macos",
+    "version": "2.6.0+260729",
+    "web_address": "https://example.pvarki.fi/app/settings",
+}
+
+
+class FakeResponse:
+    """Minimal stand-in for an aiohttp response"""
+
+    def __init__(self, status: int = 200) -> None:
+        self.status = status
+
+    def raise_for_status(self) -> None:
+        """Mimic aiohttp raising on error statuses"""
+        if self.status >= 400:
+            raise aiohttp.ClientError(f"status {self.status}")
+
+    async def __aenter__(self) -> Self:
+        return self
+
+    async def __aexit__(self, *args: object) -> None:
+        return None
+
+
+class FakeSession:
+    """Records the single post the endpoint is expected to make"""
+
+    calls: ClassVar[list[dict[str, Any]]] = []
+
+    def __init__(self, status: int = 200, raises: Exception | None = None) -> None:
+        self.status = status
+        self.raises = raises
+
+    def __call__(self, *args: object, **kwargs: object) -> Self:
+        return self
+
+    async def __aenter__(self) -> Self:
+        return self
+
+    async def __aexit__(
+        self, exc_type: type[BaseException] | None, exc: BaseException | None, tbk: TracebackType | None
+    ) -> None:
+        return None
+
+    def post(self, url: str, **kwargs: Any) -> FakeResponse:
+        """Capture the outgoing request instead of sending it"""
+        FakeSession.calls.append({"url": url, **kwargs})
+        if self.raises:
+            raise self.raises
+        return FakeResponse(self.status)
+
+
+@pytest.fixture()
+def configured_ingest(monkeypatch: pytest.MonkeyPatch) -> None:
+    """Point the settings at a fake ingest"""
+    conf = RMSettings.singleton()
+    monkeypatch.setattr(conf, "feedback_ingest_url", "https://ingest.example.com/feedback")
+    monkeypatch.setattr(conf, "feedback_ingest_key", "secretkey")
+    monkeypatch.setattr(conf, "feedback_ingest_key_header", "x-test-key")
+    FakeSession.calls = []
+
+
+@pytest.mark.asyncio(loop_scope="session")
+async def test_feedback_not_configured(user_mtls_client: TestClient) -> None:
+    """Without an ingest url and key the endpoint refuses instead of silently dropping"""
+    resp = await user_mtls_client.post("/api/v1/feedback", json=VALID_FEEDBACK)
+    assert resp.status_code == 503
+
+
+@pytest.mark.asyncio(loop_scope="session")
+async def test_feedback_forwarded(user_mtls_client: TestClient, configured_ingest: None) -> None:
+    """A valid submission reaches the ingest with server-resolved deployment context"""
+    with patch("aiohttp.ClientSession", FakeSession()):
+        resp = await user_mtls_client.post("/api/v1/feedback", json=VALID_FEEDBACK)
+    assert resp.status_code == 200
+    assert resp.json() == {"ok": True}
+    assert len(FakeSession.calls) == 1
+    call = FakeSession.calls[0]
+    assert call["url"] == "https://ingest.example.com/feedback"
+    assert call["headers"]["x-test-key"] == "secretkey"
+    payload = call["json"]
+    assert payload["rating"] == "good"
+    assert payload["web_address"] == VALID_FEEDBACK["web_address"]
+    # Deployment is resolved server side, the browser does not get to assert it
+    assert payload["deployment"] == RMSettings.singleton().deployment_name
+    assert payload["deployment"] in payload["subject"]
+    # Triage metadata is also folded into the message body
+    assert "os: macos" in payload["message"]
+
+
+@pytest.mark.asyncio(loop_scope="session")
+async def test_feedback_ingest_unreachable(user_mtls_client: TestClient, configured_ingest: None) -> None:
+    """An ingest failure is reported as a bad gateway, not a success"""
+    with patch("aiohttp.ClientSession", FakeSession(raises=aiohttp.ClientError("boom"))):
+        resp = await user_mtls_client.post("/api/v1/feedback", json=VALID_FEEDBACK)
+    assert resp.status_code == 502
+
+
+@pytest.mark.asyncio(loop_scope="session")
+async def test_feedback_ingest_rate_limited(user_mtls_client: TestClient, configured_ingest: None) -> None:
+    """The ingest saying 429 is passed through so the user is told to retry"""
+    with patch("aiohttp.ClientSession", FakeSession(status=429)):
+        resp = await user_mtls_client.post("/api/v1/feedback", json=VALID_FEEDBACK)
+    assert resp.status_code == 429
+
+
+@pytest.mark.asyncio(loop_scope="session")
+@pytest.mark.parametrize(
+    "payload",
+    [
+        pytest.param({"comments": "no rating"}, id="missing_rating"),
+        pytest.param({"rating": "good", "comments": "x" * 5001}, id="comments_too_long"),
+        pytest.param({"rating": "good", "comments": "ok", "sneaky": "field"}, id="extra_field"),
+    ],
+)
+async def test_feedback_rejects_bad_payload(
+    user_mtls_client: TestClient, configured_ingest: None, payload: dict[str, Any]
+) -> None:
+    """Unbounded or unexpected input never reaches the ingest"""
+    with patch("aiohttp.ClientSession", FakeSession()):
+        resp = await user_mtls_client.post("/api/v1/feedback", json=payload)
+    assert resp.status_code == 422
+    assert not FakeSession.calls

tests/test_rasenmaeher_api.py

@@ -15,7 +15,7 @@
 
 def test_version() -> None:
     """Make sure version matches expected"""
-    assert __version__ == "1.19.2+260912"
+    assert __version__ == "1.20.0+260913"
 
 
 @pytest.mark.asyncio(loop_scope="session")