From df566a5d9a5a4260a6d3926daac62009179d1395 Mon Sep 17 00:00:00 2001 From: benjamsf Date: Sun, 13 Sep 2026 02:05:19 +0300 Subject: [PATCH 1/5] feat(settings): add operator defined feedback ingest config --- src/rasenmaeher_api/rmsettings.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/rasenmaeher_api/rmsettings.py b/src/rasenmaeher_api/rmsettings.py index 4a90714..790facf 100644 --- a/src/rasenmaeher_api/rmsettings.py +++ b/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 From bb54890abd61735b02b4fa4fb0ab9d821406bcca Mon Sep 17 00:00:00 2001 From: benjamsf Date: Sun, 13 Sep 2026 02:06:30 +0300 Subject: [PATCH 2/5] feat(feedback): proxy feedback to the configured ingest --- .../web/api/feedback/__init__.py | 5 + .../web/api/feedback/schema.py | 41 +++++++ src/rasenmaeher_api/web/api/feedback/views.py | 113 ++++++++++++++++++ src/rasenmaeher_api/web/api/router.py | 2 + 4 files changed, 161 insertions(+) create mode 100644 src/rasenmaeher_api/web/api/feedback/__init__.py create mode 100644 src/rasenmaeher_api/web/api/feedback/schema.py create mode 100644 src/rasenmaeher_api/web/api/feedback/views.py diff --git a/src/rasenmaeher_api/web/api/feedback/__init__.py b/src/rasenmaeher_api/web/api/feedback/__init__.py new file mode 100644 index 0000000..5d53ba6 --- /dev/null +++ b/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"] diff --git a/src/rasenmaeher_api/web/api/feedback/schema.py b/src/rasenmaeher_api/web/api/feedback/schema.py new file mode 100644 index 0000000..0f51359 --- /dev/null +++ b/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") diff --git a/src/rasenmaeher_api/web/api/feedback/views.py b/src/rasenmaeher_api/web/api/feedback/views.py new file mode 100644 index 0000000..dad18ca --- /dev/null +++ b/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) diff --git a/src/rasenmaeher_api/web/api/router.py b/src/rasenmaeher_api/web/api/router.py index 6a02ac0..f1014df 100644 --- a/src/rasenmaeher_api/web/api/router.py +++ b/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"]) From c4255883c63a3daea07dfb3f9cf390e3785f3a69 Mon Sep 17 00:00:00 2001 From: benjamsf Date: Sun, 13 Sep 2026 02:09:50 +0300 Subject: [PATCH 3/5] test(feedback): cover forwarding, failures and payload limits --- tests/test_feedback.py | 142 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 142 insertions(+) create mode 100644 tests/test_feedback.py diff --git a/tests/test_feedback.py b/tests/test_feedback.py new file mode 100644 index 0000000..82fa1c0 --- /dev/null +++ b/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 From 79fefb7d562223104d5c95f47104672ed6c203cb Mon Sep 17 00:00:00 2001 From: benjamsf Date: Sun, 13 Sep 2026 02:10:07 +0300 Subject: [PATCH 4/5] docs: document feedback ingest env vars --- README.rst | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/README.rst b/README.rst index bcbef93..33e1b04 100644 --- a/README.rst +++ b/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 From 081fe7a670ad51229508343c75a06d88a6856556 Mon Sep 17 00:00:00 2001 From: benjamsf Date: Sun, 13 Sep 2026 02:10:27 +0300 Subject: [PATCH 5/5] chore: bump version to 1.20.0+260913 --- .bumpversion.toml | 2 +- pyproject.toml | 2 +- src/rasenmaeher_api/__init__.py | 2 +- tests/test_rasenmaeher_api.py | 2 +- uv.lock | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.bumpversion.toml b/.bumpversion.toml index c65cef5..9bb2c7f 100644 --- a/.bumpversion.toml +++ b/.bumpversion.toml @@ -1,5 +1,5 @@ [tool.bumpversion] -current_version = "1.19.2+260912" +current_version = "1.20.0+260913" parse = "(?P\\d+)\\.(?P\\d+)\\.(?P\\d+)\\+(?P\\d+)" serialize = ["{major}.{minor}.{patch}+{build}"] search = "{current_version}" diff --git a/pyproject.toml b/pyproject.toml index 4e73826..6787366 100644 --- a/pyproject.toml +++ b/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" }, diff --git a/src/rasenmaeher_api/__init__.py b/src/rasenmaeher_api/__init__.py index 86358cd..9f863cd 100644 --- a/src/rasenmaeher_api/__init__.py +++ b/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 diff --git a/tests/test_rasenmaeher_api.py b/tests/test_rasenmaeher_api.py index e674b9a..4b4e881 100644 --- a/tests/test_rasenmaeher_api.py +++ b/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") diff --git a/uv.lock b/uv.lock index 1822903..0009b74 100644 --- a/uv.lock +++ b/uv.lock @@ -2110,7 +2110,7 @@ wheels = [ [[package]] name = "rasenmaeher-api" -version = "1.19.2+260912" +version = "1.20.0+260913" source = { editable = "." } dependencies = [ { name = "aiodns" },