diff --git a/.well-known/security.txt b/.well-known/security.txt new file mode 100644 index 00000000..cb2aeeb5 --- /dev/null +++ b/.well-known/security.txt @@ -0,0 +1,5 @@ +Canonical: https://helpwave.de/.well-known/security.txt +Contact: mailto:security@helpwave.de +Encryption: https://keys.openpgp.org/vks/v1/by-fingerprint/720952685A7162BDA45F27DBC62B9749E1C6B631 +Expires: 2028-08-31T23:59:00Z +Preferred-Languages: en, de diff --git a/README.md b/README.md index 213a3844..f11ce493 100644 --- a/README.md +++ b/README.md @@ -5,6 +5,14 @@ **helpwave tasks** is a modern, open-source task and ward-management platform tailored for healthcare - designed to bring clarity, efficiency and structure to hospitals, wards and clinical workflows. +> ⚠️ **Pre-release — not for productive use.** This project is still under active +> development and has **not been released yet**. It is **not ready for +> production or real patient data**, and no stability, security, or data-safety +> guarantees are made at this stage. We expect this to change over the coming +> month. Until then, use it for evaluation and development only. +> +> Found a security issue? Please report it privately — see [`SECURITY.md`](SECURITY.md). + ## Quick Start If you simply want to test the application without modifying code, use the production compose file. This pulls official images and runs them behind a reverse proxy. diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 00000000..19fd7c98 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,36 @@ +# Security Policy + +> **Pre-release software.** `helpwave/tasks` is under active development and is +> **not yet released for productive use**. It has not completed a full security +> review, and no deployment should be treated as production-ready yet. This is +> expected to change over the coming month. + +## Reporting a vulnerability + +Please report security issues privately — do **not** open a public GitHub issue +or pull request for a suspected vulnerability. + +Follow helpwave's central vulnerability disclosure policy: + + +- **Contact:** security@helpwave.de +- **Encryption (PGP):** https://keys.openpgp.org/vks/v1/by-fingerprint/720952685A7162BDA45F27DBC62B9749E1C6B631 +- **Preferred languages:** English, German + +When reporting, please include: + +- affected component and version/commit, +- a description of the issue and its impact, +- reproduction steps or a proof of concept, +- any suggested remediation. + +## How reports are resolved + +1. We acknowledge your report by email. +2. We triage and confirm the issue, and agree a coordinated disclosure timeline + with you. +3. We develop and validate a fix on a private branch, then merge and release it. +4. We credit reporters who wish to be acknowledged once a fix is available. + +A machine-readable copy of these contact details is served from +[`.well-known/security.txt`](.well-known/security.txt). diff --git a/backend/README.md b/backend/README.md index 4c416449..6a3a7d76 100644 --- a/backend/README.md +++ b/backend/README.md @@ -35,8 +35,35 @@ INFLUXDB_URL=http://localhost:8086 INFLUXDB_TOKEN=tasks-token-secret INFLUXDB_ORG=tasks INFLUXDB_BUCKET=audit + +# Optional hardening knobs +ADDITIONAL_ISSUERS= # extra trusted token issuers (comma-separated) +GRAPHQL_MAX_DEPTH=15 # reject documents deeper than this +GRAPHQL_MAX_ALIASES=50 # reject documents with more aliases than this +GRAPHQL_MAX_TOKENS=2000 # reject documents with more tokens than this ``` +## Security model + +Authentication and authorization are enforced server-side, deny-by-default: + +- **Authentication.** Access tokens are verified with the realm JWKS + (signature, expiry, trusted issuer, and audience/`azp`). Tokens are read only + from the `Authorization: Bearer` header (HTTP and WebSocket + `connection_params`); the `access_token` cookie is honoured in development + only, and tokens are never read from the query string. +- **GraphQL is locked down.** Anonymous HTTP requests to `/graphql` are + rejected with `401` in production; the only thing an unauthenticated caller + may do (in development) is introspection. The GraphiQL IDE, GET queries, and + schema introspection are disabled outside development. A schema extension + denies every non-introspection field for an unauthenticated caller, including + fields wrapped in fragments. Subscriptions require a valid token at connect + time. Documents are bounded by depth/alias/token limits. +- **Authorization is location-scoped.** Every resolver restricts reads and + writes to the caller's accessible location subtree (rooted at + `user_root_locations`). Property definitions and saved views are attached to a + scaffold location and are only visible/editable inside that scope. + ## Development Setup 1. **Create virtual environment**: diff --git a/backend/api/context.py b/backend/api/context.py index 21e11554..042deff5 100644 --- a/backend/api/context.py +++ b/backend/api/context.py @@ -5,10 +5,11 @@ import strawberry from auth import get_token_from_connection_params, get_user_payload, verify_token +from config import IS_DEV from database.models.location import LocationNode, location_organizations from database.models.user import User, user_root_locations from database.session import get_db_session -from fastapi import Depends +from fastapi import Depends, HTTPException from graphql import GraphQLError from sqlalchemy import delete, select from sqlalchemy.dialects.postgresql import insert @@ -168,11 +169,22 @@ async def get_user_from_connection_params( try: user_payload = verify_token(token) except Exception as e: - logger.warning("WebSocket auth failed for token: %s", e) + logger.warning("WebSocket authentication rejected: %s", e) return None return await _resolve_user_from_payload(session, user_payload) +def _is_websocket(connection: HTTPConnection) -> bool: + return getattr(connection, "scope", {}).get("type") == "websocket" + + +def _is_graphql_http(connection: HTTPConnection) -> bool: + scope = getattr(connection, "scope", {}) + if scope.get("type") == "websocket": + return False + return str(scope.get("path", "")).rstrip("/").endswith("/graphql") + + async def get_context( connection: HTTPConnection, session=Depends(get_db_session), @@ -185,6 +197,13 @@ async def get_context( organizations = _organizations_from_payload(user_payload) db_user = await _resolve_user_from_payload(session, user_payload) + if db_user is None and not IS_DEV and _is_graphql_http(connection): + raise HTTPException( + status_code=401, + detail="Not authenticated", + headers={"WWW-Authenticate": "Bearer"}, + ) + return Context(db=session, user=db_user, organizations=organizations) @@ -250,13 +269,45 @@ async def _update_user_root_locations( if not root_location_ids: personal_org_title = f"{user.username}'s Organization" - result = await session.execute( - select(LocationNode).where( - LocationNode.title == personal_org_title, + + existing_personal = await session.execute( + select(LocationNode) + .join( + user_root_locations, + LocationNode.id == user_root_locations.c.location_id, + ) + .outerjoin( + location_organizations, + LocationNode.id == location_organizations.c.location_id, + ) + .where( + user_root_locations.c.user_id == user.id, LocationNode.parent_id.is_(None), - ), + location_organizations.c.location_id.is_(None), + ) ) - personal_location = result.scalars().first() + personal_location = existing_personal.scalars().first() + + if not personal_location: + result = await session.execute( + select(LocationNode) + .outerjoin( + location_organizations, + LocationNode.id == location_organizations.c.location_id, + ) + .outerjoin( + user_root_locations, + LocationNode.id == user_root_locations.c.location_id, + ) + .where( + LocationNode.title == personal_org_title, + LocationNode.parent_id.is_(None), + location_organizations.c.location_id.is_(None), + (user_root_locations.c.user_id == user.id) + | (user_root_locations.c.user_id.is_(None)), + ), + ) + personal_location = result.scalars().first() if not personal_location: personal_location = LocationNode( diff --git a/backend/api/errors.py b/backend/api/errors.py index 51fb4712..ad68fa7b 100644 --- a/backend/api/errors.py +++ b/backend/api/errors.py @@ -5,9 +5,18 @@ "if you believe this is an error." ) +UNAUTHENTICATED_MESSAGE = "Not authenticated" + def raise_forbidden(message: str | None = None) -> None: raise GraphQLError( message or FORBIDDEN_MESSAGE, extensions={"code": "FORBIDDEN"}, ) + + +def raise_unauthenticated(message: str | None = None) -> None: + raise GraphQLError( + message or UNAUTHENTICATED_MESSAGE, + extensions={"code": "UNAUTHENTICATED"}, + ) diff --git a/backend/api/extensions.py b/backend/api/extensions.py index 46c1c0c5..1b333e35 100644 --- a/backend/api/extensions.py +++ b/backend/api/extensions.py @@ -1,29 +1,63 @@ -from graphql import FieldNode, GraphQLError +from graphql import ( + FieldNode, + FragmentSpreadNode, + GraphQLError, + InlineFragmentNode, + OperationDefinitionNode, +) from strawberry.extensions import SchemaExtension +def _iter_top_level_fields(document, selection_set, fragments, seen_fragments): + if selection_set is None: + return + for selection in selection_set.selections: + if isinstance(selection, FieldNode): + yield selection + elif isinstance(selection, InlineFragmentNode): + yield from _iter_top_level_fields( + document, selection.selection_set, fragments, seen_fragments + ) + elif isinstance(selection, FragmentSpreadNode): + name = selection.name.value + if name in seen_fragments: + continue + seen_fragments.add(name) + fragment = fragments.get(name) + if fragment is not None: + yield from _iter_top_level_fields( + document, fragment.selection_set, fragments, seen_fragments + ) + + class GlobalAuthExtension(SchemaExtension): def on_execute(self): execution_context = self.execution_context - user = execution_context.context.user + user = getattr(execution_context.context, "user", None) - if user: + if user is not None: yield return document = execution_context.graphql_document - if document: + if document is not None: + fragments = { + definition.name.value: definition + for definition in document.definitions + if not isinstance(definition, OperationDefinitionNode) + and hasattr(definition, "name") + and definition.name is not None + } for definition in document.definitions: - if definition.kind == "operation_definition": - for selection in definition.selection_set.selections: - if not isinstance(selection, FieldNode): - continue - - if selection.name.value.startswith("__"): - continue - - raise GraphQLError( - message="Not authenticated", - extensions={"code": "UNAUTHENTICATED"}, - ) + if not isinstance(definition, OperationDefinitionNode): + continue + for field in _iter_top_level_fields( + document, definition.selection_set, fragments, set() + ): + if field.name.value.startswith("__"): + continue + raise GraphQLError( + message="Not authenticated", + extensions={"code": "UNAUTHENTICATED"}, + ) yield diff --git a/backend/api/inputs.py b/backend/api/inputs.py index 848d4d5c..f70250a9 100644 --- a/backend/api/inputs.py +++ b/backend/api/inputs.py @@ -159,6 +159,7 @@ class CreatePropertyDefinitionInput: description: str | None = None options: list[str] | None = None is_active: bool = True + location_id: strawberry.ID | None = None @strawberry.input @@ -210,6 +211,7 @@ class CreateSavedViewInput: related_sort_definition: str = "{}" related_parameters: str = "{}" visibility: SavedViewVisibility = SavedViewVisibility.LINK_SHARED + location_id: strawberry.ID | None = None @strawberry.input diff --git a/backend/api/resolvers/audit.py b/backend/api/resolvers/audit.py index 879812d8..7fa923c8 100644 --- a/backend/api/resolvers/audit.py +++ b/backend/api/resolvers/audit.py @@ -1,15 +1,56 @@ import logging +import re from datetime import datetime from typing import Any import strawberry from api.audit import AuditLogger from api.context import Info +from api.errors import raise_forbidden, raise_unauthenticated +from api.services.authorization import AuthorizationService from api.types.audit import AuditLogType from config import INFLUXDB_BUCKET, INFLUXDB_ORG, LOGGER +from database import models +from graphql import GraphQLError +from sqlalchemy import select +from sqlalchemy.orm import selectinload logger = logging.getLogger(LOGGER) +_CASE_ID_PATTERN = re.compile(r"^[A-Za-z0-9_-]{1,64}$") +_MAX_AUDIT_LIMIT = 1000 + + +async def _authorize_case_access( + info: Info, + case_id: str, +) -> None: + user = info.context.user + if not user: + raise_unauthenticated() + + auth_service = AuthorizationService(info.context.db) + if await auth_service.can_access_patient_id(user, case_id, info.context): + return + + result = await info.context.db.execute( + select(models.Task) + .where(models.Task.id == case_id) + .options( + selectinload(models.Task.patient).selectinload( + models.Patient.assigned_locations + ), + selectinload(models.Task.patient).selectinload( + models.Patient.teams + ), + ) + ) + task = result.scalars().first() + if task and await auth_service.can_access_task(user, task, info.context): + return + + raise_forbidden() + @strawberry.type class AuditQuery: @@ -21,6 +62,15 @@ async def audit_logs( limit: int | None = None, offset: int | None = None, ) -> list[AuditLogType]: + case_id_str = str(case_id) + if not _CASE_ID_PATTERN.match(case_id_str): + raise GraphQLError( + "Invalid case id.", + extensions={"code": "BAD_REQUEST"}, + ) + + await _authorize_case_access(info, case_id_str) + client = AuditLogger._get_client() if not client: logger.warning( @@ -28,19 +78,21 @@ async def audit_logs( ) return [] + limit_clause = "" + if limit is not None: + safe_limit = max(0, min(int(limit), _MAX_AUDIT_LIMIT)) + safe_offset = max(0, int(offset)) if offset is not None else 0 + limit_clause = f"|> limit(n: {safe_limit}, offset: {safe_offset})" + try: query_api = client.query_api() - limit_clause = f"LIMIT {limit}" if limit else "" - offset_clause = f"OFFSET {offset}" if offset else "" - query = f''' from(bucket: "{INFLUXDB_BUCKET}") |> range(start: 0) |> filter(fn: (r) => r["_measurement"] == "activity") - |> filter(fn: (r) => r["case_id"] == "{case_id}") + |> filter(fn: (r) => r["case_id"] == "{case_id_str}") |> sort(columns: ["_time"], desc: true) - {offset_clause} {limit_clause} ''' diff --git a/backend/api/resolvers/location.py b/backend/api/resolvers/location.py index da246621..b2404f25 100644 --- a/backend/api/resolvers/location.py +++ b/backend/api/resolvers/location.py @@ -137,7 +137,7 @@ async def create_location_node( if not accessible_location_ids: raise_forbidden() - if data.parent_id and data.parent_id not in accessible_location_ids: + if not data.parent_id or data.parent_id not in accessible_location_ids: raise_forbidden() location = models.LocationNode( diff --git a/backend/api/resolvers/patient.py b/backend/api/resolvers/patient.py index 12b7d6c9..d50a57ac 100644 --- a/backend/api/resolvers/patient.py +++ b/backend/api/resolvers/patient.py @@ -15,10 +15,12 @@ from api.query.registry import PATIENT from api.resolvers.base import BaseMutationResolver, BaseSubscriptionResolver from api.services.authorization import AuthorizationService +from api.services.subscription import effective_root_location_ids from api.services.checksum import validate_checksum from api.services.location import LocationService from api.services.notifications import notify_entity_deleted, notify_entity_update from api.services.property import PropertyService +from api.resolvers.property import validate_property_value_inputs from api.types.patient import PatientType, ScopedPatientCountsType from api.errors import raise_forbidden from api.query.dedupe_select import dedupe_orm_select_by_root_id @@ -487,6 +489,12 @@ async def create_patient( data.team_ids ) + if ( + data.assigned_location_id is not None + and data.assigned_location_id not in accessible_location_ids + ): + raise_forbidden() + new_patient = models.Patient( firstname=data.firstname, lastname=data.lastname, @@ -522,6 +530,7 @@ async def create_patient( new_patient.assigned_locations = [location] if location else [] if data.properties is not None: + await validate_property_value_inputs(info, data.properties) property_service = PatientMutation._get_property_service(db) await property_service.process_properties( new_patient, data.properties, "patient" @@ -644,6 +653,7 @@ async def update_patient( patient.assigned_locations = [location] if location else [] if data.properties is not None: + await validate_property_value_inputs(info, data.properties) property_service = PatientMutation._get_property_service(db) await property_service.process_properties( patient, data.properties, "patient" @@ -769,11 +779,11 @@ async def patient_created( subscribe_with_location_filter, ) - root_location_ids_str = ( - [str(lid) for lid in root_location_ids] - if root_location_ids - else None + root_location_ids_str = await effective_root_location_ids( + info, root_location_ids ) + if not root_location_ids_str: + return base = BaseSubscriptionResolver.entity_created(info, "patient") async for patient_id in subscribe_with_location_filter( base, @@ -795,11 +805,11 @@ async def patient_updated( subscribe_with_location_filter, ) - root_location_ids_str = ( - [str(lid) for lid in root_location_ids] - if root_location_ids - else None + root_location_ids_str = await effective_root_location_ids( + info, root_location_ids ) + if not root_location_ids_str: + return base = BaseSubscriptionResolver.entity_updated( info, "patient", patient_id ) @@ -824,11 +834,11 @@ async def patient_state_changed( subscribe_with_location_filter, ) - root_location_ids_str = ( - [str(lid) for lid in root_location_ids] - if root_location_ids - else None + root_location_ids_str = await effective_root_location_ids( + info, root_location_ids ) + if not root_location_ids_str: + return base = create_redis_subscription( "patient_state_changed", str(patient_id) if patient_id else None, @@ -852,11 +862,11 @@ async def patient_deleted( subscribe_with_location_filter, ) - root_location_ids_str = ( - [str(lid) for lid in root_location_ids] - if root_location_ids - else None + root_location_ids_str = await effective_root_location_ids( + info, root_location_ids ) + if not root_location_ids_str: + return base = BaseSubscriptionResolver.entity_deleted(info, "patient") async for patient_id in subscribe_with_location_filter( base, diff --git a/backend/api/resolvers/property.py b/backend/api/resolvers/property.py index 8f63af1c..a0f792c4 100644 --- a/backend/api/resolvers/property.py +++ b/backend/api/resolvers/property.py @@ -1,13 +1,23 @@ import strawberry from api.context import Info +from api.errors import raise_forbidden, raise_unauthenticated from api.inputs import ( CreatePropertyDefinitionInput, UpdatePropertyDefinitionInput, ) from api.resolvers.base import BaseMutationResolver +from api.services.authorization import AuthorizationService from api.types.property import PropertyDefinitionType from database import models -from sqlalchemy import select +from graphql import GraphQLError +from sqlalchemy import or_, select + + +def _require_user(info: Info) -> models.User: + user = info.context.user + if not user: + raise_unauthenticated() + return user @strawberry.type @@ -17,8 +27,18 @@ async def property_definitions( self, info: Info, ) -> list[PropertyDefinitionType]: + user = _require_user(info) + auth_service = AuthorizationService(info.context.db) + accessible = await auth_service.get_user_accessible_location_ids( + user, info.context + ) + conditions = [models.PropertyDefinition.location_id.is_(None)] + if accessible: + conditions.append( + models.PropertyDefinition.location_id.in_(accessible) + ) result = await info.context.db.execute( - select(models.PropertyDefinition), + select(models.PropertyDefinition).where(or_(*conditions)), ) return result.scalars().all() @@ -35,6 +55,24 @@ async def create_property_definition( info: Info, data: CreatePropertyDefinitionInput, ) -> PropertyDefinitionType: + user = _require_user(info) + auth_service = AuthorizationService(info.context.db) + + if data.location_id is not None: + if not await auth_service.can_access_location( + user, str(data.location_id), info.context + ): + raise_forbidden() + location_id = str(data.location_id) + else: + location_id = await auth_service.default_scope_location_id( + user, info.context + ) + if location_id is None: + raise_forbidden( + "You must belong to a location to create property definitions." + ) + entities_str = ",".join([e.value for e in data.allowed_entities]) options_str = ",".join(data.options) if data.options else None @@ -45,6 +83,7 @@ async def create_property_definition( options=options_str, is_active=data.is_active, allowed_entities=entities_str, + location_id=location_id, ) return await BaseMutationResolver.create_and_notify( info, defn, models.PropertyDefinition, "property_definition" @@ -57,11 +96,13 @@ async def update_property_definition( id: strawberry.ID, data: UpdatePropertyDefinitionInput, ) -> PropertyDefinitionType: + user = _require_user(info) db = info.context.db repo = BaseMutationResolver.get_repository(db, models.PropertyDefinition) defn = await repo.get_by_id_or_raise( id, "Property Definition not found" ) + await _require_definition_scope(info, user, defn) if data.name is not None: defn.name = data.name @@ -86,13 +127,68 @@ async def delete_property_definition( info: Info, id: strawberry.ID, ) -> bool: + user = _require_user(info) db = info.context.db repo = BaseMutationResolver.get_repository(db, models.PropertyDefinition) defn = await repo.get_by_id(id) if not defn: return False + await _require_definition_scope(info, user, defn) await BaseMutationResolver.delete_entity( info, defn, models.PropertyDefinition, "property_definition" ) return True + + +async def _require_definition_scope( + info: Info, + user: models.User, + defn: models.PropertyDefinition, +) -> None: + if defn.location_id is None: + raise_forbidden( + "This property definition is global and cannot be modified. " + "Recreate it within a location to manage it." + ) + auth_service = AuthorizationService(info.context.db) + if not await auth_service.can_access_location( + user, defn.location_id, info.context + ): + raise_forbidden() + + +async def user_can_use_definition( + info: Info, + definition_id: str, +) -> bool: + user = info.context.user + if not user: + return False + db = info.context.db + result = await db.execute( + select(models.PropertyDefinition.location_id).where( + models.PropertyDefinition.id == str(definition_id), + ) + ) + row = result.first() + if row is None: + raise GraphQLError( + "Property definition not found.", + extensions={"code": "BAD_REQUEST"}, + ) + location_id = row[0] + if location_id is None: + return True + auth_service = AuthorizationService(db) + return await auth_service.can_access_location(user, location_id, info.context) + + +async def validate_property_value_inputs(info: Info, props) -> None: + if not props: + return + for prop in props: + if not await user_can_use_definition(info, str(prop.definition_id)): + raise_forbidden( + "You cannot use one or more of the selected property definitions." + ) diff --git a/backend/api/resolvers/saved_view.py b/backend/api/resolvers/saved_view.py index e29b96a3..6ff392f6 100644 --- a/backend/api/resolvers/saved_view.py +++ b/backend/api/resolvers/saved_view.py @@ -5,6 +5,8 @@ from sqlalchemy import select from api.context import Info +from api.errors import raise_unauthenticated +from api.services.authorization import AuthorizationService from api.services.base import BaseRepository from api.inputs import ( CreateSavedViewInput, @@ -18,14 +20,49 @@ def _require_user(info: Info) -> models.User: user = info.context.user if not user: - raise GraphQLError("Authentication required") + raise_unauthenticated("Authentication required") return user +async def _can_read_shared( + info: Info, + user: models.User, + row: models.SavedView, +) -> bool: + if row.owner_user_id == user.id: + return True + if row.visibility != SavedViewVisibility.LINK_SHARED.value: + return False + if not row.location_id: + return False + auth_service = AuthorizationService(info.context.db) + return await auth_service.can_access_location( + user, row.location_id, info.context + ) + + +async def _resolve_scope_location( + info: Info, + user: models.User, + requested_location_id: strawberry.ID | None, +) -> str | None: + auth_service = AuthorizationService(info.context.db) + if requested_location_id is not None: + if not await auth_service.can_access_location( + user, str(requested_location_id), info.context + ): + from api.errors import raise_forbidden + + raise_forbidden() + return str(requested_location_id) + return await auth_service.default_scope_location_id(user, info.context) + + @strawberry.type class SavedViewQuery: @strawberry.field async def saved_view(self, info: Info, id: strawberry.ID) -> SavedViewType | None: + user = _require_user(info) db = info.context.db result = await db.execute( select(models.SavedView).where(models.SavedView.id == str(id)) @@ -33,10 +70,9 @@ async def saved_view(self, info: Info, id: strawberry.ID) -> SavedViewType | Non row = result.scalars().first() if not row: return None - uid = info.context.user.id if info.context.user else None - if row.owner_user_id != uid and row.visibility != SavedViewVisibility.LINK_SHARED.value: + if not await _can_read_shared(info, user, row): raise GraphQLError("Not found or access denied") - return SavedViewType.from_model(row, current_user_id=uid) + return SavedViewType.from_model(row, current_user_id=user.id) @strawberry.field async def my_saved_views(self, info: Info) -> list[SavedViewType]: @@ -60,6 +96,7 @@ async def create_saved_view( data: CreateSavedViewInput, ) -> SavedViewType: user = _require_user(info) + location_id = await _resolve_scope_location(info, user, data.location_id) for blob, label in ( (data.filter_definition, "filter_definition"), (data.sort_definition, "sort_definition"), @@ -83,6 +120,7 @@ async def create_saved_view( related_sort_definition=data.related_sort_definition, related_parameters=data.related_parameters, owner_user_id=user.id, + location_id=location_id, visibility=data.visibility.value, ) info.context.db.add(row) @@ -184,7 +222,7 @@ async def duplicate_saved_view( src = result.scalars().first() if not src: raise GraphQLError("View not found") - if src.owner_user_id != user.id and src.visibility != SavedViewVisibility.LINK_SHARED.value: + if not await _can_read_shared(info, user, src): raise GraphQLError("Not found or access denied") clone = models.SavedView( @@ -197,6 +235,7 @@ async def duplicate_saved_view( related_sort_definition=src.related_sort_definition, related_parameters=src.related_parameters, owner_user_id=user.id, + location_id=await _resolve_scope_location(info, user, None), visibility=SavedViewVisibility.PRIVATE.value, ) db.add(clone) diff --git a/backend/api/resolvers/task.py b/backend/api/resolvers/task.py index c11a16aa..94e10cf6 100644 --- a/backend/api/resolvers/task.py +++ b/backend/api/resolvers/task.py @@ -22,10 +22,12 @@ from api.query.registry import TASK from api.resolvers.base import BaseMutationResolver, BaseSubscriptionResolver from api.services.authorization import AuthorizationService +from api.services.subscription import effective_root_location_ids from api.services.checksum import validate_checksum from api.services.datetime import normalize_datetime_to_utc from api.services.notifications import notify_entity_update from api.services.property import PropertyService +from api.resolvers.property import validate_property_value_inputs from api.services.task_graph import ( apply_task_graph_to_patient, graph_dict_from_preset_inputs, @@ -693,6 +695,49 @@ async def _users_by_ids(info: Info, user_ids: list[strawberry.ID] | None) -> lis ) return users + @staticmethod + async def _require_team_in_scope( + info: Info, team_id: strawberry.ID | None + ) -> None: + if team_id is None: + return + auth_service = AuthorizationService(info.context.db) + if not await auth_service.can_access_location( + info.context.user, str(team_id), info.context + ): + raise_forbidden() + + @staticmethod + async def _require_assignees_in_scope( + info: Info, users: list[models.User] + ) -> None: + if not users: + return + auth_service = AuthorizationService(info.context.db) + accessible = await auth_service.get_user_accessible_location_ids( + info.context.user, info.context + ) + caller_id = info.context.user.id if info.context.user else None + for assignee in users: + if assignee.id == caller_id: + continue + if not accessible: + raise_forbidden( + "You cannot assign a user outside your locations." + ) + result = await info.context.db.execute( + select(models.user_root_locations.c.location_id) + .where( + models.user_root_locations.c.user_id == assignee.id, + models.user_root_locations.c.location_id.in_(accessible), + ) + .limit(1) + ) + if result.first() is None: + raise_forbidden( + "You cannot assign a user outside your locations." + ) + @staticmethod def _validate_task_scope( patient_id: strawberry.ID | None, @@ -779,6 +824,8 @@ async def create_task(self, info: Info, data: CreateTaskInput) -> TaskType: raise_forbidden() assignees = await TaskMutation._users_by_ids(info, data.assignee_ids) + await TaskMutation._require_assignees_in_scope(info, assignees) + await TaskMutation._require_team_in_scope(info, data.assignee_team_id) TaskMutation._validate_task_scope( data.patient_id, len(assignees), @@ -797,6 +844,7 @@ async def create_task(self, info: Info, data: CreateTaskInput) -> TaskType: ) if data.properties is not None: + await validate_property_value_inputs(info, data.properties) property_service = TaskMutation._get_property_service( info.context.db, ) @@ -900,10 +948,12 @@ async def update_task( next_assignees = task.assignees if data.assignee_ids is not strawberry.UNSET: next_assignees = await TaskMutation._users_by_ids(info, data.assignee_ids) + await TaskMutation._require_assignees_in_scope(info, next_assignees) task.assignees = next_assignees next_assignee_team_id = task.assignee_team_id if data.assignee_team_id is not strawberry.UNSET: + await TaskMutation._require_team_in_scope(info, data.assignee_team_id) next_assignee_team_id = data.assignee_team_id task.assignee_team_id = data.assignee_team_id if data.assignee_team_id is not None: @@ -920,6 +970,7 @@ async def update_task( ) if data.properties is not None: + await validate_property_value_inputs(info, data.properties) property_service = TaskMutation._get_property_service(db) await property_service.process_properties( task, @@ -1007,6 +1058,7 @@ async def add_task_assignee( "Assignee user was not found.", extensions={"code": "BAD_REQUEST"}, ) + await TaskMutation._require_assignees_in_scope(info, [user]) return await TaskMutation._update_task_field( info, id, @@ -1042,6 +1094,7 @@ async def assign_task_to_team( id: strawberry.ID, team_id: strawberry.ID, ) -> TaskType: + await TaskMutation._require_team_in_scope(info, team_id) return await TaskMutation._update_task_field( info, id, @@ -1236,11 +1289,11 @@ async def task_created( task_belongs_to_root_locations, ) - root_location_ids_str = ( - [str(lid) for lid in root_location_ids] - if root_location_ids - else None + root_location_ids_str = await effective_root_location_ids( + info, root_location_ids ) + if not root_location_ids_str: + return base = BaseSubscriptionResolver.entity_created(info, "task") async for task_id in subscribe_with_location_filter( base, @@ -1262,11 +1315,11 @@ async def task_updated( task_belongs_to_root_locations, ) - root_location_ids_str = ( - [str(lid) for lid in root_location_ids] - if root_location_ids - else None + root_location_ids_str = await effective_root_location_ids( + info, root_location_ids ) + if not root_location_ids_str: + return base = BaseSubscriptionResolver.entity_updated( info, "task", @@ -1291,11 +1344,11 @@ async def task_deleted( task_belongs_to_root_locations, ) - root_location_ids_str = ( - [str(lid) for lid in root_location_ids] - if root_location_ids - else None + root_location_ids_str = await effective_root_location_ids( + info, root_location_ids ) + if not root_location_ids_str: + return base = BaseSubscriptionResolver.entity_deleted(info, "task") async for task_id in subscribe_with_location_filter( base, diff --git a/backend/api/resolvers/task_preset.py b/backend/api/resolvers/task_preset.py index eb606573..472d2aff 100644 --- a/backend/api/resolvers/task_preset.py +++ b/backend/api/resolvers/task_preset.py @@ -51,18 +51,14 @@ def _can_edit_preset( preset: models.TaskPreset, user_id: str, ) -> bool: - if preset.scope == DbTaskPresetScope.PERSONAL.value: - return preset.owner_user_id == user_id - return True + return preset.owner_user_id is not None and preset.owner_user_id == user_id def _can_delete_preset( preset: models.TaskPreset, user_id: str, ) -> bool: - if preset.scope == DbTaskPresetScope.PERSONAL.value: - return preset.owner_user_id == user_id - return True + return preset.owner_user_id is not None and preset.owner_user_id == user_id @strawberry.type @@ -154,10 +150,7 @@ async def create_task_preset( graph_dict = graph_dict_from_preset_inputs(data.graph.nodes, data.graph.edges) validate_task_graph_dict(graph_dict) scope_val = data.scope.value - if scope_val == DbTaskPresetScope.PERSONAL.value: - owner_id = user.id - else: - owner_id = None + owner_id = user.id if data.key: if not await _key_is_available(info.context.db, data.key): raise GraphQLError( diff --git a/backend/api/resolvers/user.py b/backend/api/resolvers/user.py index 4b9ac92a..69f9e053 100644 --- a/backend/api/resolvers/user.py +++ b/backend/api/resolvers/user.py @@ -1,5 +1,6 @@ import strawberry from api.context import Info +from api.errors import raise_unauthenticated from api.inputs import PaginationInput, UpdateProfilePictureInput from api.query.execute import unified_list_query from api.query.inputs import ( @@ -9,10 +10,27 @@ ) from api.query.registry import USER from api.resolvers.base import BaseMutationResolver +from api.services.authorization import AuthorizationService from api.types.user import UserType from database import models from graphql import GraphQLError -from sqlalchemy import select +from sqlalchemy import or_, select + + +async def _visible_user_filter(info: Info): + user = info.context.user + if not user: + raise_unauthenticated() + auth_service = AuthorizationService(info.context.db) + accessible = await auth_service.get_user_accessible_location_ids( + user, info.context + ) + peer_ids = select(models.user_root_locations.c.user_id).where( + models.user_root_locations.c.location_id.in_(accessible) + if accessible + else models.user_root_locations.c.location_id.is_(None) + ) + return or_(models.User.id == user.id, models.User.id.in_(peer_ids)) @strawberry.type @@ -20,7 +38,10 @@ class UserQuery: @strawberry.field async def user(self, info: Info, id: strawberry.ID) -> UserType | None: result = await info.context.db.execute( - select(models.User).where(models.User.id == id), + select(models.User).where( + models.User.id == id, + await _visible_user_filter(info), + ), ) return result.scalars().first() @@ -34,7 +55,7 @@ async def users( pagination: PaginationInput | None = None, search: QuerySearchInput | None = None, ) -> list[UserType]: - query = select(models.User) + query = select(models.User).where(await _visible_user_filter(info)) return query @strawberry.field diff --git a/backend/api/router.py b/backend/api/router.py index c106ec30..8f082f98 100644 --- a/backend/api/router.py +++ b/backend/api/router.py @@ -1,6 +1,7 @@ from config import CLIENT_ID, ISSUER_URI from fastapi import Request from fastapi.responses import HTMLResponse +from strawberry.exceptions import ConnectionRejectionError from strawberry.fastapi import GraphQLRouter from api.context import get_user_from_connection_params @@ -8,6 +9,7 @@ class AuthedGraphQLRouter(GraphQLRouter): async def on_ws_connect(self, context): + user = None if ( hasattr(context, "connection_params") and context.connection_params @@ -16,8 +18,11 @@ async def on_ws_connect(self, context): user = await get_user_from_connection_params( context.connection_params, context.db ) - if user is not None: - context.user = user + + if user is None: + raise ConnectionRejectionError({"code": "UNAUTHENTICATED"}) + + context.user = user return await super().on_ws_connect(context) async def render_graphql_ide(self, request: Request) -> HTMLResponse: diff --git a/backend/api/services/authorization.py b/backend/api/services/authorization.py index 998bdd8e..80cdf81a 100644 --- a/backend/api/services/authorization.py +++ b/backend/api/services/authorization.py @@ -66,6 +66,30 @@ async def _compute_accessible_location_ids( return accessible_ids + async def can_access_location( + self, + user: models.User | None, + location_id: str | None, + context=None, + ) -> bool: + if not user or not location_id: + return False + accessible = await self.get_user_accessible_location_ids(user, context) + return location_id in accessible + + async def default_scope_location_id( + self, user: models.User | None, context=None + ) -> str | None: + if not user: + return None + result = await self.db.execute( + select(models.user_root_locations.c.location_id) + .where(models.user_root_locations.c.user_id == user.id) + .order_by(models.user_root_locations.c.location_id.asc()) + ) + rows = result.fetchall() + return rows[0][0] if rows else None + async def can_access_patient( self, user: models.User | None, patient: models.Patient, context=None ) -> bool: diff --git a/backend/api/services/subscription.py b/backend/api/services/subscription.py index c93938be..b5d0e772 100644 --- a/backend/api/services/subscription.py +++ b/backend/api/services/subscription.py @@ -58,6 +58,35 @@ async def create_redis_subscription( pass +async def effective_root_location_ids( + info, + client_root_location_ids: list[str] | None, +) -> list[str]: + from api.services.authorization import AuthorizationService + + user = getattr(info.context, "user", None) + if not user: + return [] + + auth_service = AuthorizationService(info.context.db) + accessible = await auth_service.get_user_accessible_location_ids( + user, info.context + ) + if not accessible: + return [] + + if client_root_location_ids: + requested = [str(lid) for lid in client_root_location_ids] + return [lid for lid in requested if lid in accessible] + + result = await info.context.db.execute( + select(models.user_root_locations.c.location_id).where( + models.user_root_locations.c.user_id == user.id + ) + ) + return [row[0] for row in result.fetchall()] + + async def patient_belongs_to_root_locations( db: AsyncSession, patient_id: str, diff --git a/backend/api/types/location.py b/backend/api/types/location.py index 3fe2bd3e..ab462d12 100644 --- a/backend/api/types/location.py +++ b/backend/api/types/location.py @@ -3,6 +3,7 @@ import strawberry from api import inputs from api.context import Info +from api.services.authorization import AuthorizationService from database import models from sqlalchemy import select @@ -10,6 +11,13 @@ from api.types.patient import PatientType +async def _accessible_ids(info: Info) -> set[str]: + auth_service = AuthorizationService(info.context.db) + return await auth_service.get_user_accessible_location_ids( + info.context.user, info.context + ) + + @strawberry.type class LocationNodeType: id: strawberry.ID @@ -30,6 +38,9 @@ async def parent( ): if not self.parent_id: return None + accessible = await _accessible_ids(info) + if str(self.parent_id) not in accessible: + return None result = await info.context.db.execute( select(models.LocationNode).where( models.LocationNode.id == self.parent_id, @@ -44,9 +55,13 @@ async def children( ) -> list[ Annotated["LocationNodeType", strawberry.lazy("api.types.location")] ]: + accessible = await _accessible_ids(info) + if not accessible: + return [] result = await info.context.db.execute( select(models.LocationNode).where( models.LocationNode.parent_id == self.id, + models.LocationNode.id.in_(accessible), ), ) return result.scalars().all() @@ -56,12 +71,18 @@ async def patients( self, info: Info, ) -> list[Annotated["PatientType", strawberry.lazy("api.types.patient")]]: - - result = await info.context.db.execute( - select(models.Patient).where( - models.Patient.assigned_location_id == self.id, - ), + accessible = await _accessible_ids(info) + if str(self.id) not in accessible: + return [] + auth_service = AuthorizationService(info.context.db) + query = select(models.Patient).where( + models.Patient.assigned_location_id == self.id, + models.Patient.deleted.is_(False), + ) + query = auth_service.filter_patients_by_access( + info.context.user, query, accessible ) + result = await info.context.db.execute(query) return result.scalars().all() @strawberry.field diff --git a/backend/api/types/property.py b/backend/api/types/property.py index f4354273..f4dfd15e 100644 --- a/backend/api/types/property.py +++ b/backend/api/types/property.py @@ -19,6 +19,7 @@ class PropertyDefinitionType: description: str | None field_type: FieldType is_active: bool + location_id: strawberry.ID | None @strawberry.field def options(self) -> list[str]: @@ -74,6 +75,13 @@ async def team( if not self.user_value or not self.user_value.startswith("team:"): return None team_id = self.user_value[5:] + from api.services.authorization import AuthorizationService + + auth_service = AuthorizationService(info.context.db) + if not await auth_service.can_access_location( + info.context.user, str(team_id), info.context + ): + return None result = await info.context.db.execute( select(models.LocationNode).where( models.LocationNode.id == team_id, diff --git a/backend/api/types/saved_view.py b/backend/api/types/saved_view.py index a2cc7791..35ad5920 100644 --- a/backend/api/types/saved_view.py +++ b/backend/api/types/saved_view.py @@ -18,6 +18,7 @@ class SavedViewType: related_sort_definition: str related_parameters: str owner_user_id: strawberry.ID + location_id: strawberry.ID | None visibility: SavedViewVisibility created_at: str updated_at: str @@ -40,6 +41,9 @@ def from_model( related_sort_definition=row.related_sort_definition, related_parameters=row.related_parameters, owner_user_id=strawberry.ID(row.owner_user_id), + location_id=( + strawberry.ID(row.location_id) if row.location_id else None + ), visibility=SavedViewVisibility(row.visibility), created_at=row.created_at.isoformat() if row.created_at else "", updated_at=row.updated_at.isoformat() if row.updated_at else "", diff --git a/backend/api/types/task.py b/backend/api/types/task.py index a99c20bf..b9d0cc66 100644 --- a/backend/api/types/task.py +++ b/backend/api/types/task.py @@ -58,6 +58,13 @@ async def assignee_team( ) -> Annotated["LocationNodeType", strawberry.lazy("api.types.location")] | None: if not self.assignee_team_id: return None + from api.services.authorization import AuthorizationService + + auth_service = AuthorizationService(info.context.db) + if not await auth_service.can_access_location( + info.context.user, str(self.assignee_team_id), info.context + ): + return None result = await info.context.db.execute( select(models.LocationNode).where(models.LocationNode.id == self.assignee_team_id), ) diff --git a/backend/api/types/user.py b/backend/api/types/user.py index 04ea0271..7fc70788 100644 --- a/backend/api/types/user.py +++ b/backend/api/types/user.py @@ -156,21 +156,14 @@ async def root_locations( ) -> list[ Annotated["LocationNodeType", strawberry.lazy("api.types.location")] ]: - import logging - logger = logging.getLogger(__name__) + from api.services.authorization import AuthorizationService - user_root_check = await info.context.db.execute( - select(models.user_root_locations.c.location_id).where( - models.user_root_locations.c.user_id == self.id - ) - ) - user_root_location_ids = [ - row[0] for row in user_root_check.all() - ] - logger.info( - f"User {self.id} has {len(user_root_location_ids)} " - f"entries in user_root_locations: {user_root_location_ids}" + auth_service = AuthorizationService(info.context.db) + accessible = await auth_service.get_user_accessible_location_ids( + info.context.user, info.context ) + if not accessible: + return [] result = await info.context.db.execute( select(models.LocationNode) @@ -179,29 +172,10 @@ async def root_locations( models.LocationNode.id == models.user_root_locations.c.location_id, ) - .where(models.user_root_locations.c.user_id == self.id) + .where( + models.user_root_locations.c.user_id == self.id, + models.LocationNode.id.in_(accessible), + ) .distinct() ) - locations = result.scalars().all() - logger.info( - f"User {self.id} root_locations query returned " - f"{len(locations)} locations: {[loc.id for loc in locations]}" - ) - - if user_root_location_ids and not locations: - location_check = await info.context.db.execute( - select(models.LocationNode).where( - models.LocationNode.id.in_(user_root_location_ids) - ) - ) - existing_locations = location_check.scalars().all() - logger.warning( - f"User {self.id} has {len(user_root_location_ids)} " - f"root location IDs but query returned empty. " - f"Checking if locations exist: " - f"{[loc.id for loc in existing_locations]} " - f"with parent_ids: " - f"{[loc.parent_id for loc in existing_locations]}" - ) - - return locations + return result.scalars().all() diff --git a/backend/auth.py b/backend/auth.py index 7f821f61..6ccd3e2e 100644 --- a/backend/auth.py +++ b/backend/auth.py @@ -1,24 +1,44 @@ import logging -from typing import Any, Optional +import threading +from typing import Optional -import requests +import jwt from config import ( + ALLOWED_ISSUERS, CLIENT_ID, FRONTEND_CLIENT_ID, + IS_DEV, ISSUER_URI, LOGGER, PUBLIC_ISSUER_URI, ) from fastapi import Request from fastapi.responses import RedirectResponse -from jose import jwk, jwt from starlette.requests import HTTPConnection logger = logging.getLogger(LOGGER) AUTH_COOKIE_NAME = "access_token" -jwks_cache: dict[str, Any] = {} +_ACCEPTED_ALGORITHMS = ["RS256", "RS384", "RS512", "ES256", "ES384", "ES512"] + +_jwk_client: jwt.PyJWKClient | None = None +_jwk_client_lock = threading.Lock() + + +def _get_jwk_client() -> jwt.PyJWKClient: + global _jwk_client + if _jwk_client is None: + with _jwk_client_lock: + if _jwk_client is None: + jwks_uri = f"{ISSUER_URI}/protocol/openid-connect/certs" + _jwk_client = jwt.PyJWKClient( + jwks_uri, + cache_keys=True, + lifespan=3600, + timeout=5, + ) + return _jwk_client def delete_auth_cookie(response): @@ -40,98 +60,52 @@ def get_user_payload(connection: HTTPConnection) -> Optional[dict]: try: return verify_token(token) except Exception as e: - logger.warning(f"Auth failed for token: {e}") + logger.warning("Auth rejected: %s", e) return None -def get_public_key(token: str) -> Any: - try: - header = jwt.get_unverified_header(token) - kid = header.get("kid") - - if not kid: - raise Exception("Token header missing 'kid' field") - - if kid in jwks_cache: - return jwks_cache[kid] - - jwks_uri = f"{ISSUER_URI}/protocol/openid-connect/certs" - - try: - response = requests.get(jwks_uri, timeout=5) - response.raise_for_status() - jwks = response.json() - except Exception as net_err: - logger.error(f"Failed to fetch JWKS from {jwks_uri}: {net_err}") - raise Exception( - "Could not reach authentication server to verify token", - ) - - for key_data in jwks.get("keys", []): - if key_data.get("kid") == kid: - key = jwk.construct(key_data) - jwks_cache[kid] = key - return key +def verify_token(token: str) -> dict: + signing_key = _get_jwk_client().get_signing_key_from_jwt(token) + + payload = jwt.decode( + token, + signing_key.key, + algorithms=_ACCEPTED_ALGORITHMS, + issuer=ALLOWED_ISSUERS, + options={ + "require": ["exp", "iat", "iss"], + "verify_exp": True, + "verify_iat": True, + "verify_iss": True, + "verify_aud": False, + }, + ) - raise Exception(f"Public key (kid={kid}) not found in JWKS") + if not payload.get("sub"): + raise jwt.InvalidTokenError("Token is missing the 'sub' claim") - except Exception as e: - logger.error(f"Key retrieval error: {e}") - raise e + azp = payload.get("azp") + aud = payload.get("aud") + if isinstance(aud, str): + aud = [aud] + elif aud is None: + aud = [] + trusted_clients = {CLIENT_ID, FRONTEND_CLIENT_ID} + if azp in trusted_clients or trusted_clients.intersection(aud): + return payload -def verify_token(token: str) -> dict: - """ - Verifies and decodes the access token JWT. - Reads claims directly from the access token payload (not from /userinfo endpoint). - The token is obtained from either: - - Authorization header (Bearer token) - - Cookie named 'access_token' - """ - try: - public_key = get_public_key(token) - - payload = jwt.decode( - token, - public_key, - algorithms=["RS256"], - options={"verify_aud": False}, - ) - - azp = payload.get("azp") - aud = payload.get("aud") - - if isinstance(aud, str): - aud = [aud] - elif aud is None: - aud = [] - - if (azp and azp == CLIENT_ID) or azp == FRONTEND_CLIENT_ID: - return payload - - if CLIENT_ID in aud or FRONTEND_CLIENT_ID in aud: - return payload - - error_msg = ( - f"Audience/AZP mismatch. " - f"Configured CLIENT_ID='{CLIENT_ID}'. " - f"Token azp='{azp}', aud='{aud}'." - ) - logger.warning(error_msg) - raise Exception(error_msg) - - except jwt.ExpiredSignatureError: - raise Exception("Token has expired") - except jwt.JWTError as e: - raise Exception(f"Invalid token format or signature: {e!s}") - except Exception as e: - raise Exception(f"{e!s}") + raise jwt.InvalidAudienceError( + f"Audience/AZP mismatch: azp={azp!r}, aud={aud!r}" + ) def get_token_from_connection_params(connection_params: dict | None) -> str | None: if not connection_params or not isinstance(connection_params, dict): return None - auth = connection_params.get("authorization") + auth = connection_params.get("authorization") or connection_params.get( + "Authorization" + ) if not auth or not isinstance(auth, str): return None parts = auth.split() @@ -140,33 +114,30 @@ def get_token_from_connection_params(connection_params: dict | None) -> str | No return None +def _bearer_from_header(connection: HTTPConnection) -> str | None: + auth_header = connection.headers.get("authorization") + if not auth_header: + return None + parts = auth_header.split() + if len(parts) == 2 and parts[0].lower() == "bearer": + return parts[1] + return None + + def get_token_source(connection: HTTPConnection) -> str | None: if hasattr(connection, "connection_params") and connection.connection_params: token = get_token_from_connection_params(connection.connection_params) if token: return token - auth_header = connection.headers.get("authorization") - if auth_header: - parts = auth_header.split() - if len(parts) == 2 and parts[0].lower() == "bearer": - return parts[1] - try: - if hasattr(connection, "query_params"): - token_param = connection.query_params.get("token") - if token_param: - return token_param - except (AttributeError, KeyError): - try: - from urllib.parse import urlparse, parse_qs - parsed_url = urlparse(str(connection.url)) - query_params = parse_qs(parsed_url.query) - if "token" in query_params and query_params["token"]: - return query_params["token"][0] - except Exception: - pass - - return connection.cookies.get(AUTH_COOKIE_NAME) + header_token = _bearer_from_header(connection) + if header_token: + return header_token + + if IS_DEV: + return connection.cookies.get(AUTH_COOKIE_NAME) + + return None class UnauthenticatedRedirect(Exception): diff --git a/backend/config.py b/backend/config.py index d1cafd10..9b08c317 100644 --- a/backend/config.py +++ b/backend/config.py @@ -48,6 +48,19 @@ class ScaffoldStrategy(str, Enum): CLIENT_SECRET = os.getenv("CLIENT_SECRET", "tasks-secret") FRONTEND_CLIENT_ID = os.getenv("FRONTEND_CLIENT_ID", "tasks-web") +_additional_issuers = [ + issuer.strip() + for issuer in os.getenv("ADDITIONAL_ISSUERS", "").split(",") + if issuer.strip() +] +ALLOWED_ISSUERS = list( + dict.fromkeys([ISSUER_URI, PUBLIC_ISSUER_URI, *_additional_issuers]) +) + +GRAPHQL_MAX_DEPTH = int(os.getenv("GRAPHQL_MAX_DEPTH", "15")) +GRAPHQL_MAX_ALIASES = int(os.getenv("GRAPHQL_MAX_ALIASES", "50")) +GRAPHQL_MAX_TOKENS = int(os.getenv("GRAPHQL_MAX_TOKENS", "2000")) + if IS_DEV: ALLOWED_ORIGINS = ["*"] else: diff --git a/backend/database/migrations/versions/add_scope_location_to_property_and_views.py b/backend/database/migrations/versions/add_scope_location_to_property_and_views.py new file mode 100644 index 00000000..0b34b5bf --- /dev/null +++ b/backend/database/migrations/versions/add_scope_location_to_property_and_views.py @@ -0,0 +1,59 @@ +"""Attach property definitions and saved views to a scaffold location. + +Adds a nullable ``location_id`` foreign key to ``property_definitions`` and +``saved_views`` so both can be authorized against the location (scaffold) +hierarchy. Existing rows keep ``NULL`` (legacy/global, read-only for +definitions; owner-only for views). + +Revision ID: add_scope_location_prop_view +Revises: add_patient_field_update_ts +Create Date: 2026-09-02 00:00:00.000000 +""" + +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + +revision: str = "add_scope_location_prop_view" +down_revision: Union[str, Sequence[str], None] = "add_patient_field_update_ts" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + with op.batch_alter_table("property_definitions") as batch_op: + batch_op.add_column( + sa.Column("location_id", sa.String(), nullable=True) + ) + batch_op.create_foreign_key( + "fk_property_definitions_location_id", + "location_nodes", + ["location_id"], + ["id"], + ) + + with op.batch_alter_table("saved_views") as batch_op: + batch_op.add_column( + sa.Column("location_id", sa.String(), nullable=True) + ) + batch_op.create_foreign_key( + "fk_saved_views_location_id", + "location_nodes", + ["location_id"], + ["id"], + ) + + +def downgrade() -> None: + with op.batch_alter_table("saved_views") as batch_op: + batch_op.drop_constraint( + "fk_saved_views_location_id", type_="foreignkey" + ) + batch_op.drop_column("location_id") + + with op.batch_alter_table("property_definitions") as batch_op: + batch_op.drop_constraint( + "fk_property_definitions_location_id", type_="foreignkey" + ) + batch_op.drop_column("location_id") diff --git a/backend/database/models/property.py b/backend/database/models/property.py index 680b934b..d8d8a25f 100644 --- a/backend/database/models/property.py +++ b/backend/database/models/property.py @@ -27,6 +27,10 @@ class PropertyDefinition(Base): options: Mapped[str | None] = mapped_column(String, nullable=True) is_active: Mapped[bool] = mapped_column(Boolean, default=True) allowed_entities: Mapped[str] = mapped_column(String, default="PATIENT") + location_id: Mapped[str | None] = mapped_column( + ForeignKey("location_nodes.id"), + nullable=True, + ) class PropertyValue(Base): diff --git a/backend/database/models/saved_view.py b/backend/database/models/saved_view.py index fd7e23d6..345fc3f5 100644 --- a/backend/database/models/saved_view.py +++ b/backend/database/models/saved_view.py @@ -36,6 +36,9 @@ class SavedView(Base): owner_user_id: Mapped[str] = mapped_column( String, ForeignKey("users.id"), nullable=False ) + location_id: Mapped[str | None] = mapped_column( + String, ForeignKey("location_nodes.id"), nullable=True + ) visibility: Mapped[str] = mapped_column(String, nullable=False, default="private") created_at: Mapped[datetime] = mapped_column( DateTime(timezone=True), server_default=func.now() diff --git a/backend/main.py b/backend/main.py index 7ade4a6e..36de2978 100644 --- a/backend/main.py +++ b/backend/main.py @@ -6,7 +6,14 @@ from api.resolvers import Mutation, Query, Subscription from api.router import AuthedGraphQLRouter from auth import UnauthenticatedRedirect, unauthenticated_redirect_handler -from config import ALLOWED_ORIGINS, IS_DEV, LOGGER +from config import ( + ALLOWED_ORIGINS, + GRAPHQL_MAX_ALIASES, + GRAPHQL_MAX_DEPTH, + GRAPHQL_MAX_TOKENS, + IS_DEV, + LOGGER, +) from fastapi import FastAPI, Request from fastapi.middleware.cors import CORSMiddleware from fastapi.responses import JSONResponse @@ -14,6 +21,11 @@ from scaffold import load_scaffold_data from starlette.requests import ClientDisconnect from strawberry import Schema +from strawberry.extensions import ( + MaxAliasesLimiter, + MaxTokensLimiter, + QueryDepthLimiter, +) logger = logging.getLogger(LOGGER) @@ -26,17 +38,30 @@ async def lifespan(app: FastAPI): logger.info("Shutting down application...") +extensions = [ + MaxTokensLimiter(max_token_count=GRAPHQL_MAX_TOKENS), + MaxAliasesLimiter(max_alias_count=GRAPHQL_MAX_ALIASES), + QueryDepthLimiter(max_depth=GRAPHQL_MAX_DEPTH), + GlobalAuthExtension, +] + +if not IS_DEV: + from strawberry.extensions import DisableIntrospection + + extensions.append(DisableIntrospection) + schema = Schema( query=Query, mutation=Mutation, subscription=Subscription, - extensions=[GlobalAuthExtension], + extensions=extensions, ) graphql_app = AuthedGraphQLRouter( schema, context_getter=get_context, - graphql_ide=IS_DEV, + graphql_ide="graphiql" if IS_DEV else None, + allow_queries_via_get=False, ) app = FastAPI( @@ -60,7 +85,22 @@ async def client_disconnect_handler(request: Request, exc: ClientDisconnect): status_code=499, content={"detail": "Client disconnected"} ) -app.include_router(auth.router) + +@app.middleware("http") +async def security_headers_middleware(request: Request, call_next): + response = await call_next(request) + response.headers.setdefault("X-Content-Type-Options", "nosniff") + response.headers.setdefault("X-Frame-Options", "DENY") + response.headers.setdefault("Referrer-Policy", "no-referrer") + response.headers.setdefault( + "Permissions-Policy", "geolocation=(), microphone=(), camera=()" + ) + return response + + +if IS_DEV: + app.include_router(auth.router) + app.include_router(export.router) app.include_router(graphql_app, prefix="/graphql") diff --git a/backend/requirements.txt b/backend/requirements.txt index fac1e680..1abf0285 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -2,7 +2,7 @@ alembic==1.19.1 asyncpg==0.31.0 fastapi==0.141.1 python-dotenv==1.2.3 -python-jose[cryptography]==3.5.0 +PyJWT[crypto]==2.13.0 redis==8.1.0 requests==2.34.2 sqlalchemy==2.0.52 diff --git a/backend/schema.graphql b/backend/schema.graphql index 957c5291..be86ccb3 100644 --- a/backend/schema.graphql +++ b/backend/schema.graphql @@ -42,6 +42,7 @@ input CreatePropertyDefinitionInput { description: String = null options: [String!] = null isActive: Boolean! = true + locationId: ID = null } input CreateSavedViewInput { @@ -54,6 +55,7 @@ input CreateSavedViewInput { relatedSortDefinition: String! = "{}" relatedParameters: String! = "{}" visibility: SavedViewVisibility! = LINK_SHARED + locationId: ID = null } input CreateTaskInput { @@ -117,17 +119,17 @@ enum LocationType { } type Mutation { + clearPatientProperty(propertyDefinitionId: ID!, patientIds: [ID!]!): Int! createPatient(data: CreatePatientInput!): PatientType! updatePatient(id: ID!, data: UpdatePatientInput!): PatientType! - clearPatientProperty(propertyDefinitionId: ID!, patientIds: [ID!]!): Int! deletePatient(id: ID!): Boolean! admitPatient(id: ID!): PatientType! dischargePatient(id: ID!): PatientType! markPatientDead(id: ID!): PatientType! waitPatient(id: ID!): PatientType! + clearTaskProperty(propertyDefinitionId: ID!, taskIds: [ID!]!): Int! createTask(data: CreateTaskInput!): TaskType! updateTask(id: ID!, data: UpdateTaskInput!): TaskType! - clearTaskProperty(propertyDefinitionId: ID!, taskIds: [ID!]!): Int! addTaskAssignee(id: ID!, userId: ID!): TaskType! removeTaskAssignee(id: ID!, userId: ID!): TaskType! assignTaskToTeam(id: ID!, teamId: ID!): TaskType! @@ -164,14 +166,6 @@ enum PatientState { DEAD } -type ScopedPatientCounts { - scopedPatientsTotal: Int! - scopedPatientsWaiting: Int! - scopedPatientsAdmitted: Int! - scopedPatientsDischarged: Int! - scopedPatientsDeceased: Int! -} - type PatientType { id: ID! firstname: String! @@ -190,12 +184,12 @@ type PatientType { clinic: LocationNodeType! position: LocationNodeType teams: [LocationNodeType!]! - tasks(done: Boolean = null): [TaskType!]! - properties: [PropertyValueType!]! updateDate: DateTime stateUpdateDate: DateTime clinicUpdateDate: DateTime positionUpdateDate: DateTime + tasks(done: Boolean = null): [TaskType!]! + properties: [PropertyValueType!]! checksum: String! } @@ -205,6 +199,7 @@ type PropertyDefinitionType { description: String fieldType: FieldType! isActive: Boolean! + locationId: ID options: [String!]! allowedEntities: [PropertyEntity!]! } @@ -245,7 +240,7 @@ type Query { patient(id: ID!): PatientType patients(locationNodeId: ID = null, rootLocationIds: [ID!] = null, states: [PatientState!] = null, filters: [QueryFilterClauseInput!] = null, sorts: [QuerySortClauseInput!] = null, pagination: PaginationInput = null, search: QuerySearchInput = null): [PatientType!]! patientsTotal(locationNodeId: ID = null, rootLocationIds: [ID!] = null, states: [PatientState!] = null, filters: [QueryFilterClauseInput!] = null, sorts: [QuerySortClauseInput!] = null, search: QuerySearchInput = null): Int! - scopedPatientCounts(rootLocationIds: [ID!] = null): ScopedPatientCounts! + scopedPatientCounts(rootLocationIds: [ID!] = null): ScopedPatientCountsType! recentPatients(rootLocationIds: [ID!] = null, filters: [QueryFilterClauseInput!] = null, sorts: [QuerySortClauseInput!] = null, pagination: PaginationInput = null, search: QuerySearchInput = null): [PatientType!]! recentPatientsTotal(rootLocationIds: [ID!] = null, filters: [QueryFilterClauseInput!] = null, sorts: [QuerySortClauseInput!] = null, search: QuerySearchInput = null): Int! task(id: ID!): TaskType @@ -387,6 +382,7 @@ type SavedView { relatedSortDefinition: String! relatedParameters: String! ownerUserId: ID! + locationId: ID visibility: SavedViewVisibility! createdAt: String! updatedAt: String! @@ -403,6 +399,14 @@ enum SavedViewVisibility { LINK_SHARED } +type ScopedPatientCountsType { + scopedPatientsTotal: Int! + scopedPatientsWaiting: Int! + scopedPatientsAdmitted: Int! + scopedPatientsDischarged: Int! + scopedPatientsDeceased: Int! +} + enum Sex { MALE FEMALE @@ -583,4 +587,4 @@ type UserType { organizations: String tasks(rootLocationIds: [ID!] = null): [TaskType!]! rootLocations: [LocationNodeType!]! -} \ No newline at end of file +} diff --git a/backend/tests/integration/test_authorization_scoping.py b/backend/tests/integration/test_authorization_scoping.py new file mode 100644 index 00000000..8566344f --- /dev/null +++ b/backend/tests/integration/test_authorization_scoping.py @@ -0,0 +1,293 @@ +from datetime import date + +import pytest +from sqlalchemy import insert + +from api.context import Context +from api.inputs import ( + CreateLocationNodeInput, + CreatePropertyDefinitionInput, + CreateSavedViewInput, + FieldType, + LocationType, + PatientState, + PropertyEntity, + SavedViewEntityType, + SavedViewVisibility, + Sex, + UpdatePropertyDefinitionInput, +) +from api.resolvers.audit import AuditQuery +from api.resolvers.location import LocationMutation +from api.resolvers.property import ( + PropertyDefinitionMutation, + PropertyDefinitionQuery, + validate_property_value_inputs, +) +from api.resolvers.saved_view import SavedViewQuery +from api.resolvers.task_preset import _can_edit_preset +from api.services.subscription import effective_root_location_ids +from database import models +from database.models.user import user_root_locations +from graphql import GraphQLError + + +class MockInfo: + def __init__(self, db, user=None): + self.context = Context(db=db, user=user) + + +@pytest.fixture(autouse=True) +def _no_redis(monkeypatch): + async def _noop(*args, **kwargs): + return None + + import api.services.notifications as notifications + + monkeypatch.setattr(notifications, "publish_to_redis", _noop, raising=False) + + +async def _add_root(db, user, location): + await db.execute( + insert(user_root_locations).values( + user_id=user.id, location_id=location.id + ) + ) + await db.commit() + + +async def _mk_user(db, uid): + user = models.User(id=uid, username=uid, firstname="F", lastname="L") + db.add(user) + await db.commit() + await db.refresh(user) + return user + + +async def _mk_location(db, lid, title, kind="CLINIC", parent_id=None): + loc = models.LocationNode(id=lid, title=title, kind=kind, parent_id=parent_id) + db.add(loc) + await db.commit() + await db.refresh(loc) + return loc + + +@pytest.fixture +async def two_tenants(db_session): + user1 = await _mk_user(db_session, "user-1") + user2 = await _mk_user(db_session, "user-2") + loc_a = await _mk_location(db_session, "loc-a", "Tenant A") + loc_b = await _mk_location(db_session, "loc-b", "Tenant B") + child_a = await _mk_location( + db_session, "loc-a-child", "Ward A1", kind="WARD", parent_id="loc-a" + ) + await _add_root(db_session, user1, loc_a) + await _add_root(db_session, user2, loc_b) + return { + "user1": user1, + "user2": user2, + "loc_a": loc_a, + "loc_b": loc_b, + "child_a": child_a, + } + + +@pytest.mark.asyncio +async def test_property_definition_is_scoped_to_its_location(two_tenants, db_session): + info1 = MockInfo(db_session, two_tenants["user1"]) + info2 = MockInfo(db_session, two_tenants["user2"]) + + created = await PropertyDefinitionMutation().create_property_definition( + info1, + CreatePropertyDefinitionInput( + name="Blood type", + field_type=FieldType.FIELD_TYPE_TEXT, + allowed_entities=[PropertyEntity.PATIENT], + location_id="loc-a", + ), + ) + assert created.location_id == "loc-a" + + visible_to_owner = await PropertyDefinitionQuery().property_definitions(info1) + assert created.id in [d.id for d in visible_to_owner] + + visible_to_other = await PropertyDefinitionQuery().property_definitions(info2) + assert created.id not in [d.id for d in visible_to_other] + + +@pytest.mark.asyncio +async def test_foreign_user_cannot_modify_definition(two_tenants, db_session): + info1 = MockInfo(db_session, two_tenants["user1"]) + info2 = MockInfo(db_session, two_tenants["user2"]) + created = await PropertyDefinitionMutation().create_property_definition( + info1, + CreatePropertyDefinitionInput( + name="Scoped", + field_type=FieldType.FIELD_TYPE_TEXT, + allowed_entities=[PropertyEntity.PATIENT], + location_id="loc-a", + ), + ) + with pytest.raises(GraphQLError): + await PropertyDefinitionMutation().update_property_definition( + info2, + created.id, + UpdatePropertyDefinitionInput(name="hijacked"), + ) + + +@pytest.mark.asyncio +async def test_create_definition_without_location_defaults_into_scope( + two_tenants, db_session +): + info1 = MockInfo(db_session, two_tenants["user1"]) + created = await PropertyDefinitionMutation().create_property_definition( + info1, + CreatePropertyDefinitionInput( + name="Defaulted", + field_type=FieldType.FIELD_TYPE_TEXT, + allowed_entities=[PropertyEntity.PATIENT], + ), + ) + assert created.location_id == "loc-a" + + +@pytest.mark.asyncio +async def test_global_definition_is_immutable(two_tenants, db_session): + info1 = MockInfo(db_session, two_tenants["user1"]) + legacy = models.PropertyDefinition( + id="legacy-def", + name="Legacy", + field_type="FIELD_TYPE_TEXT", + allowed_entities="PATIENT", + location_id=None, + ) + db_session.add(legacy) + await db_session.commit() + with pytest.raises(GraphQLError): + await PropertyDefinitionMutation().update_property_definition( + info1, "legacy-def", UpdatePropertyDefinitionInput(name="x") + ) + + +@pytest.mark.asyncio +async def test_cannot_use_property_definition_out_of_scope(two_tenants, db_session): + info1 = MockInfo(db_session, two_tenants["user1"]) + info2 = MockInfo(db_session, two_tenants["user2"]) + created = await PropertyDefinitionMutation().create_property_definition( + info1, + CreatePropertyDefinitionInput( + name="Only A", + field_type=FieldType.FIELD_TYPE_TEXT, + allowed_entities=[PropertyEntity.PATIENT], + location_id="loc-a", + ), + ) + from api.inputs import PropertyValueInput + + props = [PropertyValueInput(definition_id=created.id, text_value="v")] + await validate_property_value_inputs(info1, props) + with pytest.raises(GraphQLError): + await validate_property_value_inputs(info2, props) + + +@pytest.mark.asyncio +async def test_link_shared_view_denied_across_scope(two_tenants, db_session): + info1 = MockInfo(db_session, two_tenants["user1"]) + info2 = MockInfo(db_session, two_tenants["user2"]) + from api.resolvers.saved_view import SavedViewMutation + + view = await SavedViewMutation().create_saved_view( + info1, + CreateSavedViewInput( + name="Shared", + base_entity_type=SavedViewEntityType.PATIENT, + filter_definition="{}", + sort_definition="{}", + parameters="{}", + visibility=SavedViewVisibility.LINK_SHARED, + location_id="loc-a", + ), + ) + assert view.location_id == "loc-a" + assert (await SavedViewQuery().saved_view(info1, view.id)) is not None + with pytest.raises(GraphQLError): + await SavedViewQuery().saved_view(info2, view.id) + + +@pytest.mark.asyncio +async def test_audit_rejects_invalid_case_id(two_tenants, db_session): + info1 = MockInfo(db_session, two_tenants["user1"]) + with pytest.raises(GraphQLError): + await AuditQuery().audit_logs(info1, 'x" or true or "') + + +@pytest.mark.asyncio +async def test_audit_denies_foreign_case(two_tenants, db_session): + patient = models.Patient( + id="patient-a", + firstname="A", + lastname="B", + birthdate=date(1990, 1, 1), + sex=Sex.MALE.value, + state=PatientState.ADMITTED.value, + clinic_id="loc-a", + ) + db_session.add(patient) + await db_session.commit() + info2 = MockInfo(db_session, two_tenants["user2"]) + with pytest.raises(GraphQLError): + await AuditQuery().audit_logs(info2, "patient-a") + + +@pytest.mark.asyncio +async def test_user_directory_is_scoped(two_tenants, db_session): + from api.resolvers.user import UserQuery + + info1 = MockInfo(db_session, two_tenants["user1"]) + other = await UserQuery().user(info1, "user-2") + assert other is None + myself = await UserQuery().user(info1, "user-1") + assert myself is not None + + +@pytest.mark.asyncio +async def test_location_children_are_scoped(two_tenants, db_session): + from api.types.location import LocationNodeType + + info1 = MockInfo(db_session, two_tenants["user1"]) + info2 = MockInfo(db_session, two_tenants["user2"]) + node = LocationNodeType( + id="loc-a", title="Tenant A", kind=LocationType.CLINIC, parent_id=None + ) + owner_children = await node.children(info1) + assert "loc-a-child" in [c.id for c in owner_children] + foreign_children = await node.children(info2) + assert foreign_children == [] + + +@pytest.mark.asyncio +async def test_effective_subscription_roots_are_scoped(two_tenants, db_session): + info1 = MockInfo(db_session, two_tenants["user1"]) + assert set(await effective_root_location_ids(info1, None)) == {"loc-a"} + assert await effective_root_location_ids(info1, ["loc-b"]) == [] + assert await effective_root_location_ids(info1, ["loc-a"]) == ["loc-a"] + + +@pytest.mark.asyncio +async def test_create_root_location_is_denied(two_tenants, db_session): + info1 = MockInfo(db_session, two_tenants["user1"]) + with pytest.raises(GraphQLError): + await LocationMutation().create_location_node( + info1, + CreateLocationNodeInput(title="Rogue Root", kind=LocationType.CLINIC), + ) + + +def test_global_preset_not_editable_by_non_owner(): + class _Preset: + scope = "GLOBAL" + owner_user_id = "creator" + + assert _can_edit_preset(_Preset(), "someone-else") is False + assert _can_edit_preset(_Preset(), "creator") is True diff --git a/backend/tests/unit/test_auth_token_verification.py b/backend/tests/unit/test_auth_token_verification.py new file mode 100644 index 00000000..5bfa3bca --- /dev/null +++ b/backend/tests/unit/test_auth_token_verification.py @@ -0,0 +1,97 @@ +import time + +import jwt +import pytest +from cryptography.hazmat.primitives.asymmetric import rsa + +import auth +from config import CLIENT_ID, FRONTEND_CLIENT_ID, ISSUER_URI + +_PRIVATE_KEY = rsa.generate_private_key(public_exponent=65537, key_size=2048) +_PUBLIC_KEY = _PRIVATE_KEY.public_key() + + +class _FakeSigningKey: + key = _PUBLIC_KEY + + +@pytest.fixture(autouse=True) +def _patch_jwks(monkeypatch): + class _FakeClient: + def get_signing_key_from_jwt(self, token): + return _FakeSigningKey() + + monkeypatch.setattr(auth, "_get_jwk_client", lambda: _FakeClient()) + + +def _make_token(**overrides) -> str: + now = int(time.time()) + payload = { + "sub": "user-123", + "iss": ISSUER_URI, + "azp": CLIENT_ID, + "iat": now, + "exp": now + 300, + } + payload.update(overrides) + return jwt.encode(payload, _PRIVATE_KEY, algorithm="RS256") + + +def test_valid_token_is_accepted(): + payload = auth.verify_token(_make_token()) + assert payload["sub"] == "user-123" + + +def test_valid_token_via_frontend_client_is_accepted(): + payload = auth.verify_token(_make_token(azp=FRONTEND_CLIENT_ID)) + assert payload["sub"] == "user-123" + + +def test_audience_via_aud_claim_is_accepted(): + token = _make_token(azp="some-other-client", aud=[CLIENT_ID]) + payload = auth.verify_token(token) + assert payload["sub"] == "user-123" + + +def test_expired_token_is_rejected(): + now = int(time.time()) + token = _make_token(iat=now - 600, exp=now - 300) + with pytest.raises(Exception): + auth.verify_token(token) + + +def test_untrusted_issuer_is_rejected(): + token = _make_token(iss="https://evil.example.com/realms/tasks") + with pytest.raises(Exception): + auth.verify_token(token) + + +def test_wrong_audience_is_rejected(): + token = _make_token(azp="attacker-client", aud=["attacker-client"]) + with pytest.raises(Exception): + auth.verify_token(token) + + +def test_missing_subject_is_rejected(): + token = _make_token(sub=None) + with pytest.raises(Exception): + auth.verify_token(token) + + +def test_tampered_signature_is_rejected(): + token = _make_token() + tampered = token[:-3] + ("aaa" if not token.endswith("aaa") else "bbb") + with pytest.raises(Exception): + auth.verify_token(tampered) + + +def test_token_from_query_string_is_ignored(): + class _Conn: + headers: dict = {} + cookies: dict = {} + query_params = {"token": _make_token()} + + class url: + query = "token=abc" + + assert auth.get_token_source(_Conn()) is None diff --git a/backend/tests/unit/test_graphql_auth_extension.py b/backend/tests/unit/test_graphql_auth_extension.py new file mode 100644 index 00000000..5e648091 --- /dev/null +++ b/backend/tests/unit/test_graphql_auth_extension.py @@ -0,0 +1,55 @@ +import strawberry +import pytest + +from api.extensions import GlobalAuthExtension + + +@strawberry.type +class _Query: + @strawberry.field + def secret(self) -> str: + return "TOP-SECRET" + + +class _Ctx: + def __init__(self, user=None): + self.user = user + + +_schema = strawberry.Schema(query=_Query, extensions=[GlobalAuthExtension]) + + +async def _run(query: str, user=None): + return await _schema.execute(query, context_value=_Ctx(user)) + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "query", + [ + "{ secret }", + "{ ... on Query { secret } }", + "query { ...F } fragment F on Query { secret }", + "query { ...A } fragment A on Query { ...B } fragment B on Query { secret }", + "query { alias: secret }", + ], +) +async def test_anonymous_data_access_is_denied(query): + result = await _run(query) + assert result.data is None + assert result.errors + assert result.errors[0].extensions.get("code") == "UNAUTHENTICATED" + + +@pytest.mark.asyncio +async def test_anonymous_introspection_is_allowed(): + result = await _run("{ __typename }") + assert result.errors is None + assert result.data == {"__typename": "Query"} + + +@pytest.mark.asyncio +async def test_authenticated_access_is_allowed(): + result = await _run("{ secret }", user=object()) + assert result.errors is None + assert result.data == {"secret": "TOP-SECRET"} diff --git a/backend/tests/unit/test_schema_execution_smoke.py b/backend/tests/unit/test_schema_execution_smoke.py new file mode 100644 index 00000000..448f862c --- /dev/null +++ b/backend/tests/unit/test_schema_execution_smoke.py @@ -0,0 +1,29 @@ +import pytest + + +class _Ctx: + def __init__(self, user=None): + self.user = user + + +@pytest.mark.asyncio +async def test_configured_schema_executes_introspection_for_anonymous(): + import main + + result = await main.schema.execute( + "{ __typename }", context_value=_Ctx(user=None) + ) + assert result.errors is None + assert result.data == {"__typename": "Query"} + + +@pytest.mark.asyncio +async def test_configured_schema_denies_data_for_anonymous(): + import main + + result = await main.schema.execute( + "{ users { id } }", context_value=_Ctx(user=None) + ) + assert result.data is None + assert result.errors + assert result.errors[0].extensions.get("code") == "UNAUTHENTICATED" diff --git a/docs/how-it-works.md b/docs/how-it-works.md index 621384bd..c57b0d5a 100644 --- a/docs/how-it-works.md +++ b/docs/how-it-works.md @@ -6,8 +6,18 @@ Core flows and main abstractions. - Frontend redirects to Keycloak for login; receives JWT. - Apollo link sends `Authorization: Bearer ` on every request. -- Backend `auth.py` validates the token and attaches the user to the request context. -- Resolvers use `info.context.user` and `AuthorizationService` to enforce access. +- Backend `auth.py` validates the token against the realm JWKS (signature, + expiry, trusted issuer, audience/`azp`) and attaches the user to the request + context. Tokens are taken only from the bearer header (and WebSocket + `connection_params`); the cookie is a development-only convenience for the IDE. +- Deny-by-default: anonymous HTTP `/graphql` requests are rejected with `401` + in production, a schema extension refuses every non-introspection field for an + unauthenticated caller (fragments included), WebSocket connections are + rejected unless they present a valid token, and the IDE/introspection/GET + queries are off outside development. +- Resolvers use `info.context.user` and `AuthorizationService` to enforce + location-scoped access. Property definitions and saved views carry a + `location_id` and are authorized against the caller's accessible subtree. ## Patient and task CRUD @@ -24,6 +34,7 @@ Core flows and main abstractions. - Base subscriptions (`entity_created`, `entity_updated`, `entity_deleted`) subscribe to Redis channels and yield entity IDs. - **Location filter**: `subscribe_with_location_filter(base_iterator, db, root_location_ids_str, belongs_check)` wraps a base iterator and yields only IDs for which the entity belongs to the given root locations (via `patient_belongs_to_root_locations` or `task_belongs_to_root_locations`). Patient and task subscription resolvers use this so the UI only gets events for the current location scope. +- **Scope enforcement**: `effective_root_location_ids(info, client_root_ids)` intersects any client-supplied roots with the caller's accessible locations (falling back to the caller's own roots when none are supplied, never "everything"). A subscription with no accessible roots yields nothing, so a client can never observe events outside its scope. ## Frontend: table state and property columns diff --git a/nix/packages/proxy.nix b/nix/packages/proxy.nix index b801488f..84244b38 100644 --- a/nix/packages/proxy.nix +++ b/nix/packages/proxy.nix @@ -22,6 +22,7 @@ let include ${nginx}/conf/mime.types; default_type application/octet-stream; access_log /dev/stdout; + server_tokens off; client_max_body_size 20M; client_body_temp_path /tmp/client_body_temp; proxy_temp_path /tmp/proxy_temp; @@ -29,6 +30,13 @@ let uwsgi_temp_path /tmp/uwsgi_temp; scgi_temp_path /tmp/scgi_temp; + map $http_upgrade $connection_upgrade { + default upgrade; + ''' close; + } + + limit_req_zone $binary_remote_addr zone=graphql:10m rate=20r/s; + upstream frontend_upstream { server ''${FRONTEND_HOST}; } @@ -45,7 +53,28 @@ let listen 80; server_name localhost; - location ~ ^/(graphql|callback|export(/.*)?)$ { + add_header X-Content-Type-Options "nosniff" always; + add_header X-Frame-Options "DENY" always; + add_header Referrer-Policy "no-referrer" always; + add_header Permissions-Policy "geolocation=(), microphone=(), camera=()" always; + + location = /graphql { + limit_req zone=graphql burst=40 nodelay; + + set $graphql_ok 0; + if ($request_method = POST) { + set $graphql_ok 1; + } + if ($request_method = OPTIONS) { + set $graphql_ok 1; + } + if ($http_upgrade) { + set $graphql_ok 1; + } + if ($graphql_ok = 0) { + return 405; + } + proxy_pass http://backend_upstream; proxy_set_header Host $host; @@ -55,7 +84,22 @@ let proxy_http_version 1.1; proxy_set_header Upgrade $http_upgrade; - proxy_set_header Connection "upgrade"; + proxy_set_header Connection $connection_upgrade; + } + + location ~ ^/export(/.*)?$ { + limit_except POST OPTIONS { + deny all; + } + + proxy_pass http://backend_upstream; + + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + + proxy_http_version 1.1; } location /keycloak/ { @@ -79,13 +123,17 @@ let proxy_http_version 1.1; proxy_set_header Upgrade $http_upgrade; - proxy_set_header Connection "upgrade"; + proxy_set_header Connection $connection_upgrade; proxy_hide_header Cache-Control; proxy_hide_header Pragma; add_header Cache-Control "no-store, no-cache, must-revalidate, max-age=0" always; add_header Pragma "no-cache" always; add_header Expires "0" always; + add_header X-Content-Type-Options "nosniff" always; + add_header X-Frame-Options "DENY" always; + add_header Referrer-Policy "no-referrer" always; + add_header Permissions-Policy "geolocation=(), microphone=(), camera=()" always; } } } diff --git a/nix/packages/python-env.nix b/nix/packages/python-env.nix index efa619d6..a3ab5f89 100644 --- a/nix/packages/python-env.nix +++ b/nix/packages/python-env.nix @@ -16,7 +16,7 @@ python3.withPackages ( influxdb-client openpyxl python-dotenv - python-jose + pyjwt python-multipart redis requests diff --git a/proxy/nginx.conf b/proxy/nginx.conf index 271d3c4c..fabd9b6b 100644 --- a/proxy/nginx.conf +++ b/proxy/nginx.conf @@ -6,8 +6,17 @@ http { include mime.types; default_type application/octet-stream; + server_tokens off; + client_max_body_size 20M; + map $http_upgrade $connection_upgrade { + default upgrade; + '' close; + } + + limit_req_zone $binary_remote_addr zone=graphql:10m rate=20r/s; + upstream frontend_upstream { server ${FRONTEND_HOST}; } @@ -25,9 +34,30 @@ http { listen 80; server_name localhost; - location ~ ^/(graphql|callback|export(/.*)?)$ { + add_header X-Content-Type-Options "nosniff" always; + add_header X-Frame-Options "DENY" always; + add_header Referrer-Policy "no-referrer" always; + add_header Permissions-Policy "geolocation=(), microphone=(), camera=()" always; + + location = /graphql { + limit_req zone=graphql burst=40 nodelay; + + set $graphql_ok 0; + if ($request_method = POST) { + set $graphql_ok 1; + } + if ($request_method = OPTIONS) { + set $graphql_ok 1; + } + if ($http_upgrade) { + set $graphql_ok 1; + } + if ($graphql_ok = 0) { + return 405; + } + proxy_pass http://backend_upstream; - + proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; @@ -35,7 +65,22 @@ http { proxy_http_version 1.1; proxy_set_header Upgrade $http_upgrade; - proxy_set_header Connection "upgrade"; + proxy_set_header Connection $connection_upgrade; + } + + location ~ ^/export(/.*)?$ { + limit_except POST OPTIONS { + deny all; + } + + proxy_pass http://backend_upstream; + + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + + proxy_http_version 1.1; } location /keycloak/ { @@ -59,13 +104,17 @@ http { proxy_http_version 1.1; proxy_set_header Upgrade $http_upgrade; - proxy_set_header Connection "upgrade"; + proxy_set_header Connection $connection_upgrade; proxy_hide_header Cache-Control; proxy_hide_header Pragma; add_header Cache-Control "no-store, no-cache, must-revalidate, max-age=0" always; add_header Pragma "no-cache" always; add_header Expires "0" always; + add_header X-Content-Type-Options "nosniff" always; + add_header X-Frame-Options "DENY" always; + add_header Referrer-Policy "no-referrer" always; + add_header Permissions-Policy "geolocation=(), microphone=(), camera=()" always; } } } diff --git a/web/schema.graphql b/web/schema.graphql index 7a7ea598..be86ccb3 100644 --- a/web/schema.graphql +++ b/web/schema.graphql @@ -1,3 +1,11 @@ +input ApplyTaskGraphInput { + patientId: ID! + presetId: ID = null + graph: TaskGraphInput = null + sourcePresetId: ID = null + assignToCurrentUser: Boolean! = false +} + type AuditLogType { caseId: String! activity: String! @@ -34,6 +42,7 @@ input CreatePropertyDefinitionInput { description: String = null options: [String!] = null isActive: Boolean! = true + locationId: ID = null } input CreateSavedViewInput { @@ -42,10 +51,11 @@ input CreateSavedViewInput { filterDefinition: String! sortDefinition: String! parameters: String! - relatedFilterDefinition: String = "{}" - relatedSortDefinition: String = "{}" - relatedParameters: String = "{}" - visibility: SavedViewVisibility! = PRIVATE + relatedFilterDefinition: String! = "{}" + relatedSortDefinition: String! = "{}" + relatedParameters: String! = "{}" + visibility: SavedViewVisibility! = LINK_SHARED + locationId: ID = null } input CreateTaskInput { @@ -61,6 +71,13 @@ input CreateTaskInput { estimatedTime: Int = null } +input CreateTaskPresetInput { + name: String! + key: String = null + scope: TaskPresetScope! + graph: TaskGraphInput! +} + """Date (isoformat)""" scalar Date @@ -102,24 +119,28 @@ enum LocationType { } type Mutation { + clearPatientProperty(propertyDefinitionId: ID!, patientIds: [ID!]!): Int! createPatient(data: CreatePatientInput!): PatientType! updatePatient(id: ID!, data: UpdatePatientInput!): PatientType! - clearPatientProperty(propertyDefinitionId: ID!, patientIds: [ID!]!): Int! deletePatient(id: ID!): Boolean! admitPatient(id: ID!): PatientType! dischargePatient(id: ID!): PatientType! markPatientDead(id: ID!): PatientType! waitPatient(id: ID!): PatientType! + clearTaskProperty(propertyDefinitionId: ID!, taskIds: [ID!]!): Int! createTask(data: CreateTaskInput!): TaskType! updateTask(id: ID!, data: UpdateTaskInput!): TaskType! - clearTaskProperty(propertyDefinitionId: ID!, taskIds: [ID!]!): Int! addTaskAssignee(id: ID!, userId: ID!): TaskType! removeTaskAssignee(id: ID!, userId: ID!): TaskType! assignTaskToTeam(id: ID!, teamId: ID!): TaskType! unassignTaskFromTeam(id: ID!): TaskType! completeTask(id: ID!): TaskType! reopenTask(id: ID!): TaskType! + applyTaskGraph(data: ApplyTaskGraphInput!): [TaskType!]! deleteTask(id: ID!): Boolean! + createTaskPreset(data: CreateTaskPresetInput!): TaskPresetType! + updateTaskPreset(id: ID!, data: UpdateTaskPresetInput!): TaskPresetType! + deleteTaskPreset(id: ID!): Boolean! createPropertyDefinition(data: CreatePropertyDefinitionInput!): PropertyDefinitionType! updatePropertyDefinition(id: ID!, data: UpdatePropertyDefinitionInput!): PropertyDefinitionType! deletePropertyDefinition(id: ID!): Boolean! @@ -145,14 +166,6 @@ enum PatientState { DEAD } -type ScopedPatientCounts { - scopedPatientsTotal: Int! - scopedPatientsWaiting: Int! - scopedPatientsAdmitted: Int! - scopedPatientsDischarged: Int! - scopedPatientsDeceased: Int! -} - type PatientType { id: ID! firstname: String! @@ -171,12 +184,12 @@ type PatientType { clinic: LocationNodeType! position: LocationNodeType teams: [LocationNodeType!]! - tasks(done: Boolean = null): [TaskType!]! - properties: [PropertyValueType!]! updateDate: DateTime stateUpdateDate: DateTime clinicUpdateDate: DateTime positionUpdateDate: DateTime + tasks(done: Boolean = null): [TaskType!]! + properties: [PropertyValueType!]! checksum: String! } @@ -186,6 +199,7 @@ type PropertyDefinitionType { description: String fieldType: FieldType! isActive: Boolean! + locationId: ID options: [String!]! allowedEntities: [PropertyEntity!]! } @@ -226,7 +240,7 @@ type Query { patient(id: ID!): PatientType patients(locationNodeId: ID = null, rootLocationIds: [ID!] = null, states: [PatientState!] = null, filters: [QueryFilterClauseInput!] = null, sorts: [QuerySortClauseInput!] = null, pagination: PaginationInput = null, search: QuerySearchInput = null): [PatientType!]! patientsTotal(locationNodeId: ID = null, rootLocationIds: [ID!] = null, states: [PatientState!] = null, filters: [QueryFilterClauseInput!] = null, sorts: [QuerySortClauseInput!] = null, search: QuerySearchInput = null): Int! - scopedPatientCounts(rootLocationIds: [ID!] = null): ScopedPatientCounts! + scopedPatientCounts(rootLocationIds: [ID!] = null): ScopedPatientCountsType! recentPatients(rootLocationIds: [ID!] = null, filters: [QueryFilterClauseInput!] = null, sorts: [QuerySortClauseInput!] = null, pagination: PaginationInput = null, search: QuerySearchInput = null): [PatientType!]! recentPatientsTotal(rootLocationIds: [ID!] = null, filters: [QueryFilterClauseInput!] = null, sorts: [QuerySortClauseInput!] = null, search: QuerySearchInput = null): Int! task(id: ID!): TaskType @@ -234,6 +248,9 @@ type Query { tasksTotal(patientId: ID = null, assigneeId: ID = null, assigneeTeamId: ID = null, rootLocationIds: [ID!] = null, filters: [QueryFilterClauseInput!] = null, sorts: [QuerySortClauseInput!] = null, search: QuerySearchInput = null): Int! recentTasks(rootLocationIds: [ID!] = null, filters: [QueryFilterClauseInput!] = null, sorts: [QuerySortClauseInput!] = null, pagination: PaginationInput = null, search: QuerySearchInput = null): [TaskType!]! recentTasksTotal(rootLocationIds: [ID!] = null, filters: [QueryFilterClauseInput!] = null, sorts: [QuerySortClauseInput!] = null, search: QuerySearchInput = null): Int! + taskPresets: [TaskPresetType!]! + taskPreset(id: ID!): TaskPresetType + taskPresetByKey(key: String!): TaskPresetType locationRoots: [LocationNodeType!]! locationNode(id: ID!): LocationNodeType locationNodes(kind: LocationType = null, search: String = null, parentId: ID = null, recursive: Boolean! = false, orderByName: Boolean! = false, limit: Int = null, offset: Int = null): [LocationNodeType!]! @@ -316,10 +333,10 @@ type QueryableField { sortable: Boolean! sortDirections: [SortDirection!]! searchable: Boolean! - filterable: Boolean! relation: QueryableRelationMeta choice: QueryableChoiceMeta propertyDefinitionId: String + filterable: Boolean! } enum QueryableFieldKind { @@ -365,6 +382,7 @@ type SavedView { relatedSortDefinition: String! relatedParameters: String! ownerUserId: ID! + locationId: ID visibility: SavedViewVisibility! createdAt: String! updatedAt: String! @@ -381,6 +399,14 @@ enum SavedViewVisibility { LINK_SHARED } +type ScopedPatientCountsType { + scopedPatientsTotal: Int! + scopedPatientsWaiting: Int! + scopedPatientsAdmitted: Int! + scopedPatientsDischarged: Int! + scopedPatientsDeceased: Int! +} + enum Sex { MALE FEMALE @@ -405,6 +431,56 @@ type Subscription { locationNodeDeleted: ID! } +input TaskGraphEdgeInput { + fromNodeId: String! + toNodeId: String! +} + +type TaskGraphEdgeType { + fromId: String! + toId: String! +} + +input TaskGraphInput { + nodes: [TaskGraphNodeInput!]! + edges: [TaskGraphEdgeInput!]! +} + +input TaskGraphNodeInput { + nodeId: String! + title: String! + description: String = null + priority: TaskPriority = null + estimatedTime: Int = null +} + +type TaskGraphNodeType { + id: String! + title: String! + description: String + priority: String + estimatedTime: Int +} + +type TaskGraphType { + nodes: [TaskGraphNodeType!]! + edges: [TaskGraphEdgeType!]! +} + +enum TaskPresetScope { + PERSONAL + GLOBAL +} + +type TaskPresetType { + id: ID! + name: String! + key: String! + scope: String! + ownerUserId: ID + graph: TaskGraphType! +} + enum TaskPriority { P1 P2 @@ -422,6 +498,7 @@ type TaskType { updateDate: DateTime assigneeTeamId: ID patientId: ID + sourceTaskPresetId: ID priority: String estimatedTime: Int assignees: [UserType!]! @@ -490,6 +567,12 @@ input UpdateTaskInput { estimatedTime: Int } +input UpdateTaskPresetInput { + name: String = null + key: String = null + graph: TaskGraphInput = null +} + type UserType { id: ID! username: String!