diff --git a/package.json b/package.json index ef89b71..a087361 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@govoplan/tickets-webui", - "version": "0.1.22", + "version": "0.1.23", "private": true, "description": "Canonical GovOPlaN operational ticket lifecycle module.", "type": "module", diff --git a/pyproject.toml b/pyproject.toml index e60357a..d638a45 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,14 +4,14 @@ build-backend = "setuptools.build_meta" [project] name = "govoplan-tickets" -version = "0.1.22" +version = "0.1.23" description = "Canonical GovOPlaN operational ticket lifecycle module." readme = "README.md" requires-python = ">=3.12" license = { file = "LICENSE" } authors = [{ name = "GovOPlaN" }] dependencies = [ - "govoplan-core>=0.1.30", + "govoplan-core>=0.1.46", "govoplan-access>=0.1.18", ] diff --git a/src/govoplan_tickets/backend/manifest.py b/src/govoplan_tickets/backend/manifest.py index 0358d72..61d10b2 100644 --- a/src/govoplan_tickets/backend/manifest.py +++ b/src/govoplan_tickets/backend/manifest.py @@ -64,7 +64,7 @@ from govoplan_tickets.backend.service import ( MODULE_ID = "tickets" MODULE_NAME = "Tickets" -MODULE_VERSION = "0.1.22" +MODULE_VERSION = "0.1.23" WRITE_SCOPE = LEGACY_WRITE_SCOPE OPTIONAL_DEPENDENCIES = ( "cases", @@ -172,6 +172,36 @@ ROLE_TEMPLATES = ( ) DOCUMENTATION = ( + DocumentationTopic( + id="tickets.authorized-pagination", + title="Authorized ticket pages and totals", + summary="Ticket pages and their exact totals use the same current authorization filter.", + body=( + "Ticket lists apply tenant, deletion, search, queue, status, and current read-access filters in the database " + "before counting or selecting a page. Elevated ticket scopes retain their existing access; other readers see " + "tenant-visible tickets and tickets linked to their current subjects or actor identities. Hidden tickets do " + "not enter totals or consume page slots. Pages contain at most 200 records and use service target, priority, " + "updated time, then stable ticket ID ordering. Each request evaluates current access again; paging is not an " + "immutable snapshot across concurrent edits. Exact JSON subject matching supports SQLite and PostgreSQL and " + "does not coerce numeric or boolean identifiers; unsupported database dialects fail closed." + ), + layer="always", documentation_types=("user", "admin"), + audience=("user", "operator", "tenant_admin"), + translations={"de": { + "title": "Berechtigte Ticketseiten und Gesamtzahlen", + "summary": "Ticketseiten und ihre exakten Gesamtzahlen verwenden denselben aktuellen Berechtigungsfilter.", + "body": ( + "Ticketlisten wenden Mandanten-, Lösch-, Such-, Warteschlangen-, Status- und aktuelle Lesefilter in der " + "Datenbank vor Zählung und Seitenauswahl an. Erweiterte Ticketrechte behalten ihren bisherigen Zugriff; " + "andere Lesende sehen mandantenweit sichtbare Tickets und Tickets mit Bezug zu ihren aktuellen Subjekten " + "oder Akteurskennungen. Verborgene Tickets zählen nicht mit und belegen keine Seitenplätze. Seiten enthalten " + "höchstens 200 Datensätze, sortiert nach Serviceziel, Priorität, Änderungszeit und stabiler Ticketkennung. " + "Jede Anfrage prüft den aktuellen Zugriff erneut; Seitenabrufe bilden bei parallelen Änderungen keinen " + "unveränderlichen Snapshot. Exakte JSON-Subjektvergleiche unterstützen SQLite und PostgreSQL ohne Umwandlung " + "numerischer oder boolescher Kennungen; nicht unterstützte Datenbankdialekte werden sicher abgelehnt." + ), + }}, + ), DocumentationTopic( id="tickets.module-boundary", title="Tickets module boundary", diff --git a/src/govoplan_tickets/backend/service.py b/src/govoplan_tickets/backend/service.py index 276f50b..0ce3bd3 100644 --- a/src/govoplan_tickets/backend/service.py +++ b/src/govoplan_tickets/backend/service.py @@ -6,8 +6,10 @@ import hashlib import json from typing import Any +from sqlalchemy import false, func, or_, true from sqlalchemy.orm import Session +from govoplan_core.core.principal_helpers import principal_actor_ids as _principal_actor_ids from govoplan_core.core.events import ( EventActorRef, EventObjectRef, @@ -22,6 +24,7 @@ from govoplan_core.core.tickets import ( ticket_routing_provider, ) from govoplan_core.security.module_permissions import scopes_grant_compatible +from govoplan_core.db.json_predicates import json_array_contains_object_strings, json_object_matches_strings from govoplan_tickets.backend.db.models import ( Ticket, TicketComment, @@ -163,14 +166,15 @@ def list_tickets( clean_query = query.strip().casefold() if clean_query: statement = statement.filter(Ticket.search_text.contains(clean_query)) - candidates = statement.order_by( + statement = statement.filter(_ticket_read_predicate(principal)) + total = int(statement.with_entities(func.count()).scalar() or 0) + selected = statement.order_by( Ticket.service_target_at.asc().nullslast(), Ticket.priority.desc(), Ticket.updated_at.desc(), - ).all() - accessible = tuple(row for row in candidates if _can_read_row(principal, row)) - selected = accessible[offset : offset + limit] - return tuple(_record(row) for row in selected), len(accessible) + Ticket.id.asc(), + ).offset(offset).limit(limit).all() + return tuple(_record(row) for row in selected), total def triage_ticket( @@ -926,6 +930,26 @@ def _required_ticket(session: Session, principal: object, *, ticket_id: str, loc return row +def _ticket_read_predicate(principal: object): + """Owner policy shared by SQL count/page; never page before authorization.""" + if not _has_scope(principal, READ_SCOPE): + return false() + if _has_any_scope(principal, TRIAGE_SCOPE, ASSIGN_SCOPE, RESOLVE_SCOPE, ADMIN_SCOPE, LEGACY_WRITE_SCOPE): + return true() + conditions = [Ticket.visibility == "tenant"] + actors = _principal_actor_ids(principal) + if actors: + conditions.append(Ticket.created_by.in_(actors)) + for kind, subject_id in _principal_subjects(principal): + fields = {"kind": kind, "id": subject_id} + conditions.extend( + json_object_matches_strings(column, fields) + for column in (Ticket.assignee, Ticket.reporter, Ticket.requester) + ) + conditions.append(json_array_contains_object_strings(Ticket.participants, fields)) + return or_(*conditions) + + def _can_read_row(principal: object, row: Ticket) -> bool: if row.tenant_id != _principal_tenant(principal) or not _has_scope(principal, READ_SCOPE): return False @@ -1020,22 +1044,6 @@ def _principal_subjects(principal: object) -> tuple[tuple[str, str], ...]: return tuple(dict.fromkeys(values)) -def _principal_actor_ids(principal: object) -> tuple[str, ...]: - user = getattr(principal, "user", None) - return tuple( - dict.fromkeys( - str(value) - for value in ( - getattr(principal, "account_id", None), - getattr(principal, "identity_id", None), - getattr(principal, "membership_id", None), - getattr(user, "id", None), - ) - if str(value or "").strip() - ) - ) - - def _principal_actor(principal: object) -> str | None: values = _principal_actor_ids(principal) return values[0] if values else None diff --git a/tests/test_ticket_list_queries.py b/tests/test_ticket_list_queries.py new file mode 100755 index 0000000..3932d1b --- /dev/null +++ b/tests/test_ticket_list_queries.py @@ -0,0 +1,94 @@ +from __future__ import annotations + +from dataclasses import replace +import unittest + +from sqlalchemy import event, select +from sqlalchemy.dialects import postgresql + +import test_ticket_service as fixture +from govoplan_tickets.backend import service +from govoplan_tickets.backend.db.models import Ticket + + +class TicketListQueryTests(unittest.TestCase): + def setUp(self): + self.fixture = fixture.TicketServiceTests() + self.fixture.setUp() + self.engine = self.fixture.Session.kw["bind"] + with self.fixture.Session() as session: + for index in range(50): + service.create_ticket( + session, self.fixture.manager, + record=replace(fixture._record(reporter="reporter-1"), ticket_id=f"ticket-{index:03}", ticket_number=f"TKT-{index:03}"), + idempotency_key=f"fixture-{index}", + ) + session.commit() + + def tearDown(self): + self.engine.dispose() + + def test_one_row_page_loads_one_row_with_exact_total_and_stable_ties(self): + with self.fixture.Session() as session: + session.query(Ticket).update({Ticket.updated_at: fixture.NOW}) + session.commit() + loaded = [] + queries = [] + + def row_loaded(target, context): + loaded.append(target.id) + + def executed(conn, cursor, statement, parameters, context, executemany): + if statement.lstrip().upper().startswith("SELECT"): + queries.append(statement) + + event.listen(Ticket, "load", row_loaded) + event.listen(self.engine, "before_cursor_execute", executed) + try: + with self.fixture.Session() as session: + page, total = service.list_tickets(session, self.fixture.manager, offset=1, limit=1) + self.assertEqual(50, total) + self.assertEqual(["ticket-001"], [item.ticket_id for item in page]) + self.assertEqual(["ticket-001"], loaded) + self.assertEqual(2, len(queries)) + self.assertIn("LIMIT", queries[-1]) + finally: + event.remove(Ticket, "load", row_loaded) + event.remove(self.engine, "before_cursor_execute", executed) + + def test_count_and_page_match_current_object_authorization(self): + viewer = fixture._Principal("viewer-1", {service.READ_SCOPE}) + viewer.group_ids = ("group-1",) + with self.fixture.Session() as session: + rows = session.query(Ticket).order_by(Ticket.id).all() + rows[0].visibility = "tenant" + rows[1].assignee = {"kind": "account", "id": viewer.account_id} + rows[2].reporter = {"kind": "account", "id": viewer.account_id} + rows[3].requester = {"kind": "account", "id": viewer.account_id} + rows[4].participants = [{"kind": "group", "id": "group-1"}] + rows[5].created_by = viewer.account_id + rows[6].participants = ['{"kind":"account","id":"viewer-1"}'] + rows[7].tenant_id = "tenant-2" + rows[7].visibility = "tenant" + session.commit() + expected = {row.id for row in rows if service._can_read_row(viewer, row)} + actual = set() + for offset in range(len(expected)): + page, total = service.list_tickets(session, viewer, offset=offset, limit=1) + self.assertEqual(len(expected), total) + self.assertEqual(1, len(page)) + actual.add(page[0].ticket_id) + self.assertEqual(expected, actual) + self.assertEqual(6, len(actual)) + rows[4].participants = [] + session.commit() + self.assertEqual(5, service.list_tickets(session, viewer, limit=1)[1]) + viewer.scopes = frozenset() + self.assertEqual(((), 0), service.list_tickets(session, viewer, limit=1)) + + def test_read_predicate_compiles_for_postgresql_with_bound_subjects(self): + viewer = fixture._Principal("viewer' OR 1=1 --", {service.READ_SCOPE}) + compiled = select(Ticket.id).where(service._ticket_read_predicate(viewer)).compile(dialect=postgresql.dialect()) + self.assertIn("json_array_elements", str(compiled)) + self.assertNotIn(viewer.account_id, str(compiled)) + self.assertIn(viewer.account_id, compiled.params.values()) diff --git a/webui/package.json b/webui/package.json index 2a4ce6e..6746422 100644 --- a/webui/package.json +++ b/webui/package.json @@ -1,6 +1,6 @@ { "name": "@govoplan/tickets-webui", - "version": "0.1.22", + "version": "0.1.23", "private": true, "type": "module", "main": "src/index.ts",