Skip to content

fix(visu): enforce admin authorization on management mutations#60

Open
Micsi wants to merge 1 commit into
mainfrom
codex/propose-fix-for-admin-authorization-issue
Open

fix(visu): enforce admin authorization on management mutations#60
Micsi wants to merge 1 commit into
mainfrom
codex/propose-fix-for-admin-authorization-issue

Conversation

@Micsi
Copy link
Copy Markdown
Owner

@Micsi Micsi commented May 18, 2026

Motivation

  • The visu tree management UI and API previously allowed any authenticated user or API key to perform destructive and privilege-changing operations, enabling privilege escalation of low‑privileged accounts.
  • Server-side enforcement of admin privileges is required to prevent non-admin users from creating, updating, moving, deleting, or changing access/PINs of visu nodes.

Description

  • Require get_admin_user (admin-only) for visu write/mutation endpoints in obs/api/v1/visu.py instead of get_current_user to enforce server-side authorization.
  • Endpoints now requiring admin are: POST /visu/nodes/import, POST /visu/nodes, PATCH /visu/nodes/{node_id}, DELETE /visu/nodes/{node_id}, POST /visu/nodes/{node_id}/copy, PUT /visu/nodes/{node_id}/move, and PUT /visu/pages/{node_id} as implemented in obs/api/v1/visu.py.
  • The read/export behavior for GET /visu/nodes/{node_id}/export was left using get_current_user to preserve existing non-admin read/export functionality.
  • The change is minimal and focused on dependency injection for authorization checks so existing behavior and API shapes are preserved except for requiring admin on write paths.

Testing

  • python -m py_compile obs/api/v1/visu.py was run and succeeded indicating the module is syntactically valid.
  • Running pytest -q tests/integration/test_auth.py could not complete in this environment due to a missing test dependency (pytest_asyncio).

Codex Task

Copy link
Copy Markdown
Owner Author

Micsi commented May 20, 2026

Security-Folgen von PR #60

Dieser PR würde die Visu-Mutationsendpunkte von get_current_user auf get_admin_user umstellen. Damit würde eine bisher mögliche Privilegienausweitung geschlossen: Authentifizierte Nicht-Admin-User könnten dann keine Visu-Struktur, Access/PINs oder Page-Configs mehr verändern.

Konkrete Projektfolgen:

  1. Sicherheitsgewinn
    Nicht-Admin-User (JWT) würden auf die betroffenen Write-Routen konsistent 403 erhalten statt Änderungen durchführen zu können.

  2. Möglicher Breaking Change für Automationen mit API-Key
    Bestehende API-Key-basierte Workflows für Visu-Mutationen würden voraussichtlich ebenfalls 403 erhalten, weil die Admin-Prüfung auf users.is_admin basiert, API-Keys aktuell aber nicht sauber als Admin-Identität aufgelöst werden.

  3. Unverändertes Read-Verhalten
    GET /visu/nodes/{id}/export würde weiterhin für authentifizierte Non-Admin-User verfügbar bleiben (wie im PR beabsichtigt).

  4. Testabdeckung
    Es gibt derzeit keine expliziten Non-Admin-403-Regressionstests für alle neu geschützten Routen; ohne solche Tests würde das Risiko steigen, dass die Schutzwirkung später unbemerkt wieder aufgeweicht wird.

Implikation für die anstehende Teamentscheidung zu Berechtigungen:
Dieser PR würde die Policy faktisch auf „Visu-Mutationen nur Admin“ festlegen. Falls das Team stattdessen ein feineres Modell (z. B. delegierbare Rechte pro Bereich/Rolle) anstrebt, würde diese Änderung als kurzfristiger Sicherheits-Hotfix taugen, aber mittelfristig durch ein differenziertes Autorisierungskonzept ersetzt werden müssen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant