From a037f91d359813efaca0aa0900034a1b8df1fc11 Mon Sep 17 00:00:00 2001 From: Cy Rossignol Date: Thu, 28 May 2026 21:18:31 -0700 Subject: [PATCH 01/18] Fix double CREATE TYPE for workspace_role enum --- .../9221408912dd_add_user_role_table.py | 79 ++++--------------- 1 file changed, 16 insertions(+), 63 deletions(-) diff --git a/alembic_osm/versions/9221408912dd_add_user_role_table.py b/alembic_osm/versions/9221408912dd_add_user_role_table.py index d34f97e..402cf9a 100644 --- a/alembic_osm/versions/9221408912dd_add_user_role_table.py +++ b/alembic_osm/versions/9221408912dd_add_user_role_table.py @@ -10,7 +10,7 @@ import sqlalchemy as sa from alembic import op -from sqlalchemy import inspect, text +from sqlalchemy import text # revision identifiers, used by Alembic. revision: str = "9221408912dd" @@ -20,69 +20,22 @@ def upgrade() -> None: - bind = op.get_bind() - assert bind is not None - insp = inspect(bind) - - # Add unique constraint on users.auth_uid (if not already present) - constraint_exists = bind.execute( - text("SELECT 1 FROM pg_constraint WHERE conname = 'auth_uid_unique'") - ).scalar() - if not constraint_exists: - op.create_unique_constraint("auth_uid_unique", "users", ["auth_uid"]) - - # Create the workspace_role enum type (if not already present) - result = bind.execute( - text("SELECT 1 FROM pg_type WHERE typname = 'workspace_role'") + op.create_unique_constraint("auth_uid_unique", "users", ["auth_uid"]) + op.create_table( + "user_workspace_roles", + sa.Column("user_auth_uid", sa.String(), nullable=False), + sa.Column("workspace_id", sa.BigInteger(), nullable=False), + sa.Column( + "role", + sa.Enum("lead", "validator", "contributor", name="workspace_role"), + nullable=False, + ), + sa.ForeignKeyConstraint(["user_auth_uid"], ["users.auth_uid"]), + sa.PrimaryKeyConstraint("user_auth_uid", "workspace_id"), ) - if not result.scalar(): - workspace_role = sa.Enum( - "lead", "validator", "contributor", name="workspace_role" - ) - workspace_role.create(bind) - - # Create the user_workspace_roles table (if not already present) - if not insp.has_table("user_workspace_roles"): - op.create_table( - "user_workspace_roles", - sa.Column("user_auth_uid", sa.String(), nullable=False), - sa.Column("workspace_id", sa.BigInteger(), nullable=False), - sa.Column( - "role", - sa.Enum( - "lead", - "validator", - "contributor", - name="workspace_role", - create_type=False, - ), - nullable=False, - ), - sa.ForeignKeyConstraint(["user_auth_uid"], ["users.auth_uid"]), - sa.PrimaryKeyConstraint("user_auth_uid", "workspace_id"), - ) def downgrade() -> None: - bind = op.get_bind() - assert bind is not None - insp = inspect(bind) - - if insp.has_table("user_workspace_roles"): - op.drop_table("user_workspace_roles") - - # Drop the enum type - result = bind.execute( - text("SELECT 1 FROM pg_type WHERE typname = 'workspace_role'") - ) - if result.scalar(): - workspace_role = sa.Enum( - "lead", "validator", "contributor", name="workspace_role" - ) - workspace_role.drop(bind) - - constraint_exists = bind.execute( - text("SELECT 1 FROM pg_constraint WHERE conname = 'auth_uid_unique'") - ).scalar() - if constraint_exists: - op.drop_constraint("auth_uid_unique", "users", type_="unique") + op.drop_table("user_workspace_roles") + op.execute(text("DROP TYPE workspace_role")) + op.drop_constraint("auth_uid_unique", "users", type_="unique") From 2cb1b8e0f2110089fda13af4e001d51060bd16e7 Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Wed, 24 Jun 2026 13:52:12 -0400 Subject: [PATCH 02/18] First pass --- CLAUDE.md | 32 ++++ pyproject.toml | 1 + tests/README.md | 94 ++++++++++++ tests/conftest.py | 69 +++++++++ tests/integration/__init__.py | 0 tests/integration/test_health.py | 13 ++ tests/integration/test_proxy.py | 68 +++++++++ tests/integration/test_teams.py | 78 ++++++++++ tests/integration/test_workspaces.py | 130 +++++++++++++++++ tests/support/__init__.py | 4 + tests/support/factories.py | 106 ++++++++++++++ tests/support/fakes.py | 185 ++++++++++++++++++++++++ tests/support/http.py | 46 ++++++ tests/test_main.py | 11 -- tests/unit/__init__.py | 0 tests/unit/test_user_info.py | 54 +++++++ tests/unit/test_workspace_repository.py | 94 ++++++++++++ 17 files changed, 974 insertions(+), 11 deletions(-) create mode 100644 CLAUDE.md create mode 100644 tests/README.md create mode 100644 tests/conftest.py create mode 100644 tests/integration/__init__.py create mode 100644 tests/integration/test_health.py create mode 100644 tests/integration/test_proxy.py create mode 100644 tests/integration/test_teams.py create mode 100644 tests/integration/test_workspaces.py create mode 100644 tests/support/__init__.py create mode 100644 tests/support/factories.py create mode 100644 tests/support/fakes.py create mode 100644 tests/support/http.py delete mode 100644 tests/test_main.py create mode 100644 tests/unit/__init__.py create mode 100644 tests/unit/test_user_info.py create mode 100644 tests/unit/test_workspace_repository.py diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..c2d7f3a --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,32 @@ +# CLAUDE.md + +Guidance for working in this repo. Focused on the test infrastructure and +conventions established for it; see `README.md` for app setup. + +## Permission Structure + +Project Group Admin ("POC") +* Superuser for the whole project group +* Implied by "poc" role in TDEI + +Lead/Owner/Workspace Admin +* Admin-level access for a workspace +* Configures workspace settings and quest definitions +* Assigns users to workspace teams +* Ability to merge changes from other workspace +* Exports data to TDEI (with appropriate TDEI core roles) +* Granted by Workspaces setting. + +Contributor/Data Generator +* Modifies workspace data--all modifications need validation +* Implied by membership in TDEI project group + +Validator +* Modifies workspace data and approves changes from contributors +* Granted by Workspaces setting. + +Viewer/Member/Everyone Else +* Read-only access to workspace data +* With express TDEI sign-up, the need for this access level diminishes greatly +* Granted by Workspaces setting. + diff --git a/pyproject.toml b/pyproject.toml index b4e4b1f..910a8ac 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -40,6 +40,7 @@ dependencies = [ addopts = "-v --cov=api --cov-report=term-missing" testpaths = ["tests"] asyncio_mode = "auto" +asyncio_default_fixture_loop_scope = "function" [tool.black] line-length = 88 diff --git a/tests/README.md b/tests/README.md new file mode 100644 index 0000000..cd90ce5 --- /dev/null +++ b/tests/README.md @@ -0,0 +1,94 @@ +# Tests + +Two layers of tests, both fast and dependency-free (no Postgres, PostGIS, +Docker, or network): + +| Layer | Location | What it exercises | +|-------|----------|-------------------| +| **Unit** | `tests/unit/` | Pure logic and individual classes (e.g. `UserInfo` permission rules, a repository in isolation). | +| **Integration** | `tests/integration/` | Real HTTP requests through the real FastAPI app: routing, auth wiring, repositories, schemas, and serialization. | + +## The mocking boundary: the "data fetcher", not the repository + +Integration tests run the **real** routes and repositories. The only things +swapped out are: + +1. **The database sessions** (`get_task_session`, `get_osm_session`) — replaced + with a `FakeSession` that returns pre-programmed rows instead of running SQL. + This is the "data fetcher" boundary: everything *above* the `AsyncSession` + (repositories, models, routes, Pydantic serialization) runs for real. +2. **`validate_token`** — replaced with a real `UserInfo` built by the factories, + skipping JWT decoding and the TDEI network call. The permission logic + (`isWorkspaceLead`, `isWorkspaceContributor`, …) is still real. +3. **The upstream OSM client** (`api.main._osm_client`, proxy tests only) — + replaced with a streamable mock transport. + +Mocking at the session level (rather than mocking whole repositories) means +the SQLModel object construction, repository logic, route guards, and response +serialization are all genuinely tested. + +## Writing an integration test + +```python +async def test_get_workspace_by_id(client, login, task_session): + login() # authenticate + task_session.queue(fakes.rows(factories.make_workspace(id=7))) + response = await client.get("/api/v1/workspaces/7") + assert response.status_code == 200 +``` + +### Fixtures (`conftest.py`) + +- `client` — async HTTP client bound to the app over an in-process ASGI transport. +- `login(user_info=None)` — set the authenticated user. Call with a + `factories.make_user_info(...)` to control roles/permissions. +- `task_session` / `osm_session` — the two `FakeSession`s. Queue results on them. + +### The call-order contract + +Because the mock is at the session level, **queue results in the order the +repository issues queries**. Each result builder models one DB round-trip: + +| Builder | Models | Returned by | +|---------|--------|-------------| +| `fakes.rows(*entities)` | a SELECT | `scalars().all()`, `scalar_one_or_none()`, `all()` | +| `fakes.empty()` | a SELECT with no rows | drives NotFound paths | +| `fakes.affected(n)` | an UPDATE/DELETE | `result.rowcount` | +| `fakes.mappings(*dicts)` | a raw-SQL result | `result.mappings()` | +| `fakes.scalar(value)` | `session.scalar(...)` | e.g. an `EXISTS` check | + +Routes that touch both DBs (e.g. teams, workspace create/delete) queue results +on **both** `task_session` and `osm_session`. `SET search_path` statements are +recognized and do not consume a queued result. `commit`/`add`/`rollback` calls +are recorded on the session (`session.commits`, `session.added`, …) for assertions. + +The repository docstrings and the existing tests show the query sequence for +each route. + +## Running + +```bash +uv run pytest # full suite with coverage (see pyproject.toml) +uv run pytest --no-cov -q # quick, no coverage +uv run pytest tests/unit # one layer +uv run pytest -k workspaces # by keyword +``` + +## Layout + +``` +tests/ + conftest.py # fixtures: client, login, task_session, osm_session + support/ + fakes.py # FakeSession + FakeResult + result builders + factories.py # make_user_info / make_workspace / make_user / make_team + http.py # streamable mock transport for the OSM proxy + unit/ + test_user_info.py + test_workspace_repository.py + integration/ + test_health.py + test_workspaces.py + test_teams.py + test_proxy.py +``` diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..2fef3b5 --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,69 @@ +"""Shared pytest fixtures. + +Integration tests drive the *real* FastAPI app through an ASGI HTTP client, +overriding only three dependencies: + +* ``get_task_session`` / ``get_osm_session`` -> :class:`FakeSession` (the + "data fetcher" boundary; queue simulated rows per test). +* ``validate_token`` -> a real ``UserInfo`` built by the factories (skips + JWT decoding and the TDEI network call, but the permission logic is real). + +Everything above that boundary -- routes, repositories, schemas, Pydantic +serialization -- runs unmodified. +""" + +import pytest +import pytest_asyncio +from httpx import ASGITransport, AsyncClient + +from api.core.database import get_osm_session, get_task_session +from api.core.security import validate_token +from api.main import app as fastapi_app +from tests.support import factories +from tests.support.fakes import FakeSession + + +@pytest.fixture +def task_session() -> FakeSession: + """Fake session backing the tasking-manager / workspaces DB.""" + return FakeSession() + + +@pytest.fixture +def osm_session() -> FakeSession: + """Fake session backing the OSM DB.""" + return FakeSession() + + +@pytest.fixture +def app(task_session, osm_session): + """The real app with its DB sessions overridden by fakes.""" + fastapi_app.dependency_overrides[get_task_session] = lambda: task_session + fastapi_app.dependency_overrides[get_osm_session] = lambda: osm_session + yield fastapi_app + fastapi_app.dependency_overrides.clear() + + +@pytest.fixture +def login(app): + """Authenticate the request as a given user. + + Call with a ``UserInfo`` (see ``factories.make_user_info``) to set the + authenticated principal; called with no args it logs in a default user. + Returns the ``UserInfo`` in effect. + """ + + def _login(user_info=None): + user_info = user_info or factories.make_user_info() + app.dependency_overrides[validate_token] = lambda: user_info + return user_info + + return _login + + +@pytest_asyncio.fixture +async def client(app): + """Async HTTP client bound to the app over an in-process ASGI transport.""" + transport = ASGITransport(app=app) + async with AsyncClient(transport=transport, base_url="http://testserver") as c: + yield c diff --git a/tests/integration/__init__.py b/tests/integration/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/integration/test_health.py b/tests/integration/test_health.py new file mode 100644 index 0000000..f79475e --- /dev/null +++ b/tests/integration/test_health.py @@ -0,0 +1,13 @@ +"""Integration smoke tests for unauthenticated endpoints.""" + + +async def test_health_check(client): + response = await client.get("/health") + assert response.status_code == 200 + assert response.json() == {"status": "ok"} + + +async def test_root_redirects_to_docs(client): + response = await client.get("/", follow_redirects=False) + assert response.status_code == 307 + assert response.headers["location"] == "/docs" diff --git a/tests/integration/test_proxy.py b/tests/integration/test_proxy.py new file mode 100644 index 0000000..aab0ffd --- /dev/null +++ b/tests/integration/test_proxy.py @@ -0,0 +1,68 @@ +"""Integration tests for the OSM proxy (catch-all) routes. + +The proxy forwards to an upstream OSM service via a module-level +``httpx.AsyncClient`` and re-streams the response. We swap that client for +one backed by a streamable mock transport so the proxy path is exercised +end-to-end without a real upstream. +""" + +import pytest + +import api.main +from tests.support import factories +from tests.support.http import osm_mock_client + + +@pytest.fixture +def mock_osm(monkeypatch): + """Replace the upstream OSM client with a recording mock transport.""" + client, transport = osm_mock_client() + monkeypatch.setattr(api.main, "_osm_client", client) + return transport + + +async def test_capabilities_proxies_without_auth(client, mock_osm): + # No X-Workspace header and no bearer token required for capabilities. + response = await client.get("/api/capabilities.json") + + assert response.status_code == 200 + assert b"osm version" in response.content + assert mock_osm.last_request.url.path == "/api/capabilities.json" + + +async def test_proxy_requires_workspace_header(client, login, mock_osm): + login(factories.make_user_info()) + # A non-whitelisted path with no X-Workspace header is rejected. + response = await client.get("/api/0.6/map") + + assert response.status_code == 400 + + +async def test_proxy_forbidden_without_workspace_access(client, login, mock_osm): + # User has no access to workspace 1. + login(factories.make_user_info(accessible_workspace_ids={})) + + response = await client.get("/api/0.6/map", headers={"X-Workspace": "1"}) + + assert response.status_code == 403 + + +async def test_proxy_forwards_for_workspace_contributor(client, login, mock_osm): + login(factories.make_user_info(accessible_workspace_ids={"pg": [1]})) + + response = await client.get("/api/0.6/map", headers={"X-Workspace": "1"}) + + assert response.status_code == 200 + assert b"osm version" in response.content + # Spoofable forwarding headers are normalized before reaching upstream. + forwarded = mock_osm.last_request.headers + assert "x-forwarded-for" in forwarded # set by the proxy itself + assert forwarded["host"] == "osm-web" + + +async def test_proxy_rejects_non_integer_workspace_header(client, login, mock_osm): + login(factories.make_user_info(accessible_workspace_ids={"pg": [1]})) + + response = await client.get("/api/0.6/map", headers={"X-Workspace": "abc"}) + + assert response.status_code == 400 diff --git a/tests/integration/test_teams.py b/tests/integration/test_teams.py new file mode 100644 index 0000000..fdd92a0 --- /dev/null +++ b/tests/integration/test_teams.py @@ -0,0 +1,78 @@ +"""Integration tests for the /workspaces/{id}/teams routes. + +Team routes touch BOTH databases: the workspace access guard hits the task +DB (``workspace_repo.getById``) and the team operations hit the OSM DB. +Queue results on the matching fake session in call order. +""" + +from tests.support import factories, fakes + + +def teams_url(workspace_id: int = 1) -> str: + return f"/api/v1/workspaces/{workspace_id}/teams" + + +async def test_list_teams(client, login, task_session, osm_session): + login() + # getById (task DB) guards access... + task_session.queue(fakes.rows(factories.make_workspace(id=1))) + # ...then team_repo.get_all (OSM DB) returns teams with members. + osm_session.queue( + fakes.rows( + factories.make_team(id=1, name="Alpha", users=[factories.make_user()]), + factories.make_team(id=2, name="Beta", users=[]), + ) + ) + + response = await client.get(teams_url()) + + assert response.status_code == 200 + body = response.json() + assert body[0] == {"id": 1, "name": "Alpha", "member_count": 1} + assert body[1]["member_count"] == 0 + + +async def test_create_team_requires_lead(client, login, task_session, osm_session): + login() # plain contributor + response = await client.post(teams_url(), json={"name": "New Team"}) + assert response.status_code == 403 + + +async def test_create_team_as_lead(client, login, task_session, osm_session): + from api.src.users.schemas import WorkspaceUserRoleType + + login( + factories.make_user_info(osm_workspace_roles={1: [WorkspaceUserRoleType.LEAD]}) + ) + # lead check -> getById (task) guard -> team_repo.create (OSM): add+commit+refresh + task_session.queue(fakes.rows(factories.make_workspace(id=1))) + + response = await client.post(teams_url(), json={"name": "New Team"}) + + assert response.status_code == 201 + assert isinstance(response.json(), int) + assert osm_session.commits == 1 + + +async def test_get_team_not_in_workspace_returns_404( + client, login, task_session, osm_session +): + login() + task_session.queue(fakes.rows(factories.make_workspace(id=1))) + # assert_team_in_workspace -> session.scalar(EXISTS) returns False + osm_session.queue(fakes.scalar(False)) + + response = await client.get(f"{teams_url()}/999") + + assert response.status_code == 404 + + +async def test_list_teams_for_inaccessible_workspace_is_404( + client, login, task_session +): + login() + task_session.queue(fakes.empty()) # getById guard finds no workspace + + response = await client.get(teams_url(42)) + + assert response.status_code == 404 diff --git a/tests/integration/test_workspaces.py b/tests/integration/test_workspaces.py new file mode 100644 index 0000000..663e12a --- /dev/null +++ b/tests/integration/test_workspaces.py @@ -0,0 +1,130 @@ +"""Integration tests for the /workspaces routes. + +Each test drives a real HTTP request through the real route + repository +code, queueing simulated rows on the fake DB sessions. Comments note the +query sequence each route issues (= the order to queue results). +""" + +from api.src.users.schemas import WorkspaceUserRoleType +from tests.support import factories, fakes + +API = "/api/v1/workspaces" + + +async def test_get_my_workspaces(client, login, task_session): + login() + # getAll -> one SELECT on the task DB + task_session.queue( + fakes.rows( + factories.make_workspace(id=1, title="WS One"), + factories.make_workspace(id=2, title="WS Two"), + ) + ) + + response = await client.get(f"{API}/mine") + + assert response.status_code == 200 + body = response.json() + assert [w["id"] for w in body] == [1, 2] + assert body[0]["title"] == "WS One" + assert body[0]["role"] == "contributor" # WorkspaceResponse includes role + + +async def test_get_workspace_by_id(client, login, task_session): + login() + # getById -> one SELECT + task_session.queue(fakes.rows(factories.make_workspace(id=7, title="Lucky"))) + + response = await client.get(f"{API}/7") + + assert response.status_code == 200 + assert response.json()["title"] == "Lucky" + + +async def test_get_workspace_not_found(client, login, task_session): + login() + task_session.queue(fakes.empty()) # getById finds nothing + + response = await client.get(f"{API}/404") + + assert response.status_code == 404 + + +async def test_create_workspace(client, login, task_session, osm_session): + login(factories.make_user_info(project_group_ids=[factories.DEFAULT_PG_ID])) + # create (task DB) -> add + commit + refresh; then assign_member_role + # (OSM DB) checks the user exists via session.scalar(...). + osm_session.queue(fakes.scalar(1)) + + response = await client.post( + f"{API}", + json={ + "type": "osw", + "title": "Fresh Workspace", + "tdeiProjectGroupId": factories.DEFAULT_PG_ID, + }, + ) + + assert response.status_code == 201 + assert response.json()["workspaceId"] is not None + assert task_session.commits == 1 + assert osm_session.commits == 1 + + +async def test_update_workspace_requires_lead(client, login, task_session): + # Plain contributor, not a lead -> 403 before any DB write. + login(factories.make_user_info()) + + response = await client.patch(f"{API}/1", json={"title": "Renamed"}) + + assert response.status_code == 403 + assert task_session.commits == 0 + + +async def test_update_workspace_as_lead(client, login, task_session): + login( + factories.make_user_info(osm_workspace_roles={1: [WorkspaceUserRoleType.LEAD]}) + ) + # update -> UPDATE (rowcount) ... commit ... then getById -> SELECT + task_session.queue( + fakes.affected(1), + fakes.rows(factories.make_workspace(id=1, title="Renamed")), + ) + + response = await client.patch(f"{API}/1", json={"title": "Renamed"}) + + assert response.status_code == 204 + assert task_session.commits == 1 + + +async def test_update_workspace_rejects_empty_patch(client, login): + login( + factories.make_user_info(osm_workspace_roles={1: [WorkspaceUserRoleType.LEAD]}) + ) + + response = await client.patch(f"{API}/1", json={}) + + assert response.status_code == 400 + + +async def test_delete_workspace_as_lead(client, login, task_session, osm_session): + login( + factories.make_user_info(osm_workspace_roles={1: [WorkspaceUserRoleType.LEAD]}) + ) + # delete flow: get_privileged_workspace_members (OSM) -> delete (task) -> + # remove_all_member_roles (OSM) + osm_session.queue(fakes.rows()) # no privileged members + task_session.queue(fakes.affected(1)) + + response = await client.delete(f"{API}/1") + + assert response.status_code == 204 + assert task_session.commits == 1 + + +async def test_delete_workspace_forbidden_for_non_lead(client, login): + login(factories.make_user_info()) + + response = await client.delete(f"{API}/1") + + assert response.status_code == 403 diff --git a/tests/support/__init__.py b/tests/support/__init__.py new file mode 100644 index 0000000..1d2d1d0 --- /dev/null +++ b/tests/support/__init__.py @@ -0,0 +1,4 @@ +"""Shared test support: fake DB sessions and object factories. + +See ``tests/README.md`` for the testing philosophy and call-order contract. +""" diff --git a/tests/support/factories.py b/tests/support/factories.py new file mode 100644 index 0000000..9cdd402 --- /dev/null +++ b/tests/support/factories.py @@ -0,0 +1,106 @@ +"""Factories for the domain objects tests need. + +These build real ``UserInfo`` / SQLModel instances (not mocks) so the +permission logic and serialization under test run for real. Defaults are +deterministic so tests can assert on concrete values. +""" + +from datetime import datetime +from uuid import UUID + +from api.core.security import TdeiProjectGroupRole, UserInfo, UserInfoPGMembership +from api.src.teams.schemas import WorkspaceTeam +from api.src.users.schemas import User +from api.src.workspaces.schemas import Workspace, WorkspaceType + +# Stable identifiers reused across tests. +DEFAULT_PG_ID = "11111111-1111-1111-1111-111111111111" +DEFAULT_USER_ID = "22222222-2222-2222-2222-222222222222" + + +def make_user_info( + *, + user_id: str = DEFAULT_USER_ID, + user_name: str = "Test User", + project_group_ids: list[str] | None = None, + accessible_workspace_ids: dict[str, list[int]] | None = None, + osm_workspace_roles: dict[int, list] | None = None, + poc_group_ids: tuple[str, ...] = (), +) -> UserInfo: + """Build a ``UserInfo`` the way ``validate_token`` would have. + + ``project_group_ids`` -- TDEI project groups the user belongs to. + ``accessible_workspace_ids`` -- pg_id -> workspace ids (from the task DB). + ``osm_workspace_roles`` -- workspace_id -> roles (from the OSM DB). + ``poc_group_ids`` -- subset of groups where the user is a POC + (grants lead rights on owned workspaces). + """ + info = UserInfo() + info.credentials = "test-token" + info.user_uuid = UUID(user_id) + info.user_name = user_name + + if project_group_ids is None: + project_group_ids = [DEFAULT_PG_ID] + + info.projectGroups = [ + UserInfoPGMembership( + project_group_name=f"PG {pg_id}", + project_group_id=pg_id, + tdeiRoles=( + [TdeiProjectGroupRole.POINT_OF_CONTACT] + if pg_id in poc_group_ids + else [TdeiProjectGroupRole.MEMBER] + ), + ) + for pg_id in project_group_ids + ] + info.accessibleWorkspaceIds = accessible_workspace_ids or {} + info.osmWorkspaceRoles = osm_workspace_roles or {} + return info + + +def make_workspace( + *, + id: int | None = 1, + title: str = "Test Workspace", + type: WorkspaceType = WorkspaceType.OSW, + tdei_project_group_id: str = DEFAULT_PG_ID, + created_by: str = DEFAULT_USER_ID, + created_by_name: str = "Test User", + **extra, +) -> Workspace: + return Workspace( + id=id, + title=title, + type=type, + tdeiProjectGroupId=UUID(tdei_project_group_id), + createdBy=UUID(created_by), + createdByName=created_by_name, + createdAt=datetime(2026, 1, 1), + **extra, + ) + + +def make_user( + *, + id: int = 1, + auth_uid: str = DEFAULT_USER_ID, + email: str = "user@example.com", + display_name: str = "User One", +) -> User: + return User(id=id, auth_uid=auth_uid, email=email, display_name=display_name) + + +def make_team( + *, + id: int = 1, + name: str = "Team Alpha", + workspace_id: int = 1, + users: list[User] | None = None, +) -> WorkspaceTeam: + team = WorkspaceTeam(id=id, name=name, workspace_id=workspace_id) + # ``WorkspaceTeamItem.from_team`` reads ``len(team.users)``; set it + # explicitly since the relationship isn't loaded from a real DB here. + team.users = users or [] + return team diff --git a/tests/support/fakes.py b/tests/support/fakes.py new file mode 100644 index 0000000..a989fd1 --- /dev/null +++ b/tests/support/fakes.py @@ -0,0 +1,185 @@ +"""Fake async DB session for integration tests. + +The application talks to its databases exclusively through SQLAlchemy / +SQLModel ``AsyncSession`` objects (the "data fetcher"). Repositories, +routes, schemas and serialization all sit *above* that boundary. + +``FakeSession`` stands in for a real session: instead of running SQL it +returns pre-programmed ``FakeResult`` objects from a FIFO queue. This lets +a test drive a real HTTP request through the real repository code while +feeding it simulated rows -- no Postgres, PostGIS or Docker required. + +Because the boundary is the session, a test must queue results in the +ORDER the repository issues queries. Each repository method documents its +query sequence; the integration tests show the common patterns. The most +common helpers are :func:`rows` (a SELECT result) and :func:`affected` +(an UPDATE/DELETE rowcount). +""" + +from collections import deque +from itertools import count + +from sqlalchemy.exc import NoResultFound + + +class _Scalars: + """Mimics the object returned by ``result.scalars()``.""" + + def __init__(self, rows): + self._rows = list(rows) + + def all(self): + return list(self._rows) + + def first(self): + return self._rows[0] if self._rows else None + + def __iter__(self): + return iter(self._rows) + + +class _Mappings: + """Mimics the object returned by ``result.mappings()`` (raw-SQL dict rows).""" + + def __init__(self, mappings): + self._mappings = list(mappings) + + def all(self): + return list(self._mappings) + + def first(self): + return self._mappings[0] if self._mappings else None + + def __iter__(self): + return iter(self._mappings) + + +class FakeResult: + """A canned result returned by :meth:`FakeSession.execute` / ``exec``. + + ``rows`` -- ORM entities for ``scalars()`` / ``scalar_one_or_none()``. + ``mappings`` -- dict rows for ``mappings()`` (used by raw-SQL queries). + ``rowcount`` -- value returned by ``result.rowcount`` (UPDATE/DELETE). + ``value`` -- value returned by ``session.scalar(...)``. + """ + + def __init__(self, rows=None, mappings=None, rowcount=1, value=None): + self._rows = list(rows) if rows is not None else [] + self._mappings = list(mappings) if mappings is not None else [] + self.rowcount = rowcount + self.value = value + + def scalars(self): + return _Scalars(self._rows) + + def scalar_one_or_none(self): + return self._rows[0] if self._rows else None + + def scalar_one(self): + if len(self._rows) != 1: + raise NoResultFound("FakeResult.scalar_one() expected exactly one row") + return self._rows[0] + + def mappings(self): + return _Mappings(self._mappings) + + def all(self): + # Used by queries that select multiple columns (rows are tuples). + return list(self._rows) + + def first(self): + return self._rows[0] if self._rows else None + + +class FakeSession: + """Drop-in async stand-in for a SQLModel ``AsyncSession``. + + Queue results with :meth:`queue` (or pass them to the constructor); each + ``execute`` / ``exec`` call pops the next one. ``commit`` / ``add`` / + ``rollback`` / ``refresh`` are recorded so tests can assert on writes. + """ + + def __init__(self, *responses): + self._responses = deque(responses) + self.added = [] + self.commits = 0 + self.rollbacks = 0 + self.closed = False + self._id_seq = count(1) + + def queue(self, *responses): + """Append results to the response queue. Returns self for chaining.""" + self._responses.extend(responses) + return self + + def _next(self): + return self._responses.popleft() if self._responses else FakeResult(rows=[]) + + @staticmethod + def _is_session_setup(statement): + # Raw ``SET search_path ...`` statements (used by the OSM bbox query) + # carry no result and should not consume a queued response. + try: + return str(statement).strip().upper().startswith("SET ") + except Exception: + return False + + async def execute(self, statement, *args, **kwargs): + if self._is_session_setup(statement): + return FakeResult(rows=[]) + return self._next() + + async def exec(self, statement, *args, **kwargs): + return self._next() + + async def scalar(self, statement, *args, **kwargs): + result = self._next() + return result.value if isinstance(result, FakeResult) else result + + def add(self, obj): + self.added.append(obj) + + async def commit(self): + self.commits += 1 + + async def rollback(self): + self.rollbacks += 1 + + async def refresh(self, obj, *args, **kwargs): + # Simulate the DB assigning an autoincrement primary key on insert. + if getattr(obj, "id", "missing") is None: + obj.id = next(self._id_seq) + + async def close(self): + self.closed = True + + +# --- result builders ------------------------------------------------------- + + +def rows(*entities, rowcount=None): + """A SELECT result yielding ``entities`` for ``scalars()`` / ``scalar_one*``.""" + return FakeResult( + rows=list(entities), + rowcount=len(entities) if rowcount is None else rowcount, + ) + + +def empty(): + """A SELECT result with no rows (drives NotFound paths).""" + return FakeResult(rows=[], rowcount=0) + + +def affected(n): + """An UPDATE/DELETE result reporting ``n`` affected rows.""" + return FakeResult(rows=[], rowcount=n) + + +def mappings(*dict_rows): + """A raw-SQL result yielding dict rows for ``result.mappings()``.""" + return FakeResult(mappings=list(dict_rows)) + + +def scalar(value): + """A result for ``session.scalar(...)`` (e.g. an EXISTS check).""" + return FakeResult(value=value) diff --git a/tests/support/http.py b/tests/support/http.py new file mode 100644 index 0000000..71cff70 --- /dev/null +++ b/tests/support/http.py @@ -0,0 +1,46 @@ +"""HTTP test doubles for the OSM proxy. + +The proxy streams upstream responses with ``response.aiter_raw()``. The +stock ``httpx.MockTransport`` eagerly buffers the body, so re-streaming it +raises ``StreamConsumed``. This transport returns a genuinely streamable +response and records the forwarded request for assertions. +""" + +import httpx + + +class StreamingMockTransport(httpx.AsyncBaseTransport): + """An httpx transport that returns streamable canned responses. + + ``handler(request) -> (status_code, headers_dict, body_bytes)``. + The most recent request is available as ``.last_request``. + """ + + def __init__(self, handler): + self._handler = handler + self.last_request: httpx.Request | None = None + + async def handle_async_request(self, request: httpx.Request) -> httpx.Response: + # Read the request body so the upstream call completes cleanly. + await request.aread() + self.last_request = request + status_code, headers, body = self._handler(request) + + async def stream(): + yield body + + return httpx.Response(status_code, headers=headers, content=stream()) + + +def osm_mock_client(handler=None, *, base_url: str = "http://osm-web"): + """Build an ``AsyncClient`` standing in for the upstream OSM service. + + Returns ``(client, transport)`` so tests can inspect ``transport.last_request``. + """ + + def default_handler(_request): + return 200, {"content-type": "text/xml"}, b"" + + transport = StreamingMockTransport(handler or default_handler) + client = httpx.AsyncClient(transport=transport, base_url=base_url) + return client, transport diff --git a/tests/test_main.py b/tests/test_main.py deleted file mode 100644 index bc2f395..0000000 --- a/tests/test_main.py +++ /dev/null @@ -1,11 +0,0 @@ -from fastapi.testclient import TestClient - -from api.main import app - -client = TestClient(app) - - -def test_health_check(): - response = client.get("/health") - assert response.status_code == 200 - assert response.json() == {"status": "ok"} diff --git a/tests/unit/__init__.py b/tests/unit/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/unit/test_user_info.py b/tests/unit/test_user_info.py new file mode 100644 index 0000000..3547344 --- /dev/null +++ b/tests/unit/test_user_info.py @@ -0,0 +1,54 @@ +"""Unit tests for UserInfo permission logic (pure, no app or DB).""" + +from api.core.security import WorkspaceUserRoleType +from tests.support import factories + + +def test_get_project_group_ids(): + user = factories.make_user_info(project_group_ids=["pg-a", "pg-b"]) + assert user.getProjectGroupIds() == ["pg-a", "pg-b"] + + +def test_is_workspace_lead_via_osm_role(): + user = factories.make_user_info( + osm_workspace_roles={5: [WorkspaceUserRoleType.LEAD]} + ) + assert user.isWorkspaceLead(5) is True + assert user.isWorkspaceLead(6) is False + + +def test_is_workspace_lead_via_poc_group(): + # A point-of-contact in a group that owns workspace 9 is a lead there. + user = factories.make_user_info( + project_group_ids=["pg-owner"], + poc_group_ids=("pg-owner",), + accessible_workspace_ids={"pg-owner": [9]}, + ) + assert user.isWorkspaceLead(9) is True + assert user.isWorkspaceLead(10) is False + + +def test_poc_without_workspace_ownership_is_not_lead(): + user = factories.make_user_info( + project_group_ids=["pg-owner"], + poc_group_ids=("pg-owner",), + accessible_workspace_ids={"pg-owner": []}, + ) + assert user.isWorkspaceLead(9) is False + + +def test_is_workspace_validator(): + user = factories.make_user_info( + osm_workspace_roles={3: [WorkspaceUserRoleType.VALIDATOR]} + ) + assert user.isWorkspaceValidator(3) is True + assert user.isWorkspaceValidator(4) is False + + +def test_is_workspace_contributor(): + user = factories.make_user_info( + accessible_workspace_ids={"pg-a": [1, 2], "pg-b": [3]} + ) + assert user.isWorkspaceContributor(2) is True + assert user.isWorkspaceContributor(3) is True + assert user.isWorkspaceContributor(99) is False diff --git a/tests/unit/test_workspace_repository.py b/tests/unit/test_workspace_repository.py new file mode 100644 index 0000000..84fc9e4 --- /dev/null +++ b/tests/unit/test_workspace_repository.py @@ -0,0 +1,94 @@ +"""Unit tests for WorkspaceRepository against a fake session. + +These exercise the repository in isolation (no HTTP layer) to show how the +data-fetcher boundary is mocked: queue the rows the DB "would" return, then +assert on the repository's behavior and writes. +""" + +from typing import cast +from uuid import UUID + +import pytest +from sqlmodel.ext.asyncio.session import AsyncSession + +from api.core.exceptions import ForbiddenException, NotFoundException +from api.src.workspaces.repository import WorkspaceRepository +from api.src.workspaces.schemas import WorkspaceCreate, WorkspaceType +from tests.support import factories, fakes + + +def _repo(session: fakes.FakeSession) -> WorkspaceRepository: + # FakeSession implements the AsyncSession surface the repository uses. + return WorkspaceRepository(cast(AsyncSession, session)) + + +@pytest.fixture +def user(): + return factories.make_user_info() + + +async def test_get_by_id_returns_workspace(user): + workspace = factories.make_workspace(id=1, title="Mapping WS") + session = fakes.FakeSession(fakes.rows(workspace)) + + result = await _repo(session).getById(user, 1) + + assert result.id == 1 + assert result.title == "Mapping WS" + + +async def test_get_by_id_missing_raises_not_found(user): + session = fakes.FakeSession(fakes.empty()) + + with pytest.raises(NotFoundException): + await _repo(session).getById(user, 404) + + +async def test_get_all_returns_list(user): + session = fakes.FakeSession( + fakes.rows(factories.make_workspace(id=1), factories.make_workspace(id=2)) + ) + + result = await _repo(session).getAll(user) + + assert [w.id for w in result] == [1, 2] + + +async def test_create_commits_and_assigns_id(user): + session = fakes.FakeSession() + + workspace = await _repo(session).create( + user, + WorkspaceCreate( + type=WorkspaceType.OSW, + title="Brand New", + tdeiProjectGroupId=UUID(factories.DEFAULT_PG_ID), + ), + ) + + assert session.commits == 1 + assert workspace in session.added + assert workspace.id is not None # assigned by refresh() + assert workspace.createdByName == "Test User" + + +async def test_create_in_unauthorized_group_raises(user): + session = fakes.FakeSession() + + with pytest.raises(ForbiddenException): + await _repo(session).create( + user, + WorkspaceCreate( + type=WorkspaceType.OSW, + title="Forbidden", + tdeiProjectGroupId=UUID("99999999-9999-9999-9999-999999999999"), + ), + ) + assert session.commits == 0 + + +async def test_delete_missing_raises_not_found(user): + session = fakes.FakeSession(fakes.affected(0)) + + with pytest.raises(NotFoundException): + await _repo(session).delete(user, 1) From 43741a07628af50d99e4efc1e9c37cd976ed0c2b Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Wed, 24 Jun 2026 13:52:28 -0400 Subject: [PATCH 03/18] Test comments and typing cleanup --- api/core/config.py | 5 +- api/core/json_schema.py | 58 ++++++++++++++++-------- api/core/jwt.py | 4 ++ api/core/security.py | 24 ++++++++-- api/main.py | 30 ++++++++++-- api/src/teams/repository.py | 6 +++ api/src/teams/routes.py | 64 ++++++++++++++++++++++++++ api/src/teams/schemas.py | 15 ++++-- api/src/users/repository.py | 6 +++ api/src/users/routes.py | 20 ++++++++ api/src/users/schemas.py | 10 +++- api/src/workspaces/repository.py | 12 ++++- api/src/workspaces/routes.py | 78 +++++++++++++++++++++++++++++++- api/src/workspaces/schemas.py | 19 ++++++-- 14 files changed, 311 insertions(+), 40 deletions(-) diff --git a/api/core/config.py b/api/core/config.py index 16cedfd..5824796 100644 --- a/api/core/config.py +++ b/api/core/config.py @@ -1,6 +1,9 @@ from pydantic_settings import BaseSettings, SettingsConfigDict - +# Test outline: +# @test: Test that environment variables of the same name as the members of this class are correctly loaded into the members of this class. +# @test: Test that any environment variables that are not set in the environment are correctly loaded into the members of this class with their default values. +# @test: Test that strings and number values, empty strings and URLs are all correctly loaded as exemplified by the default values class Settings(BaseSettings): """Application settings.""" diff --git a/api/core/json_schema.py b/api/core/json_schema.py index 2c2317b..4d6f54a 100644 --- a/api/core/json_schema.py +++ b/api/core/json_schema.py @@ -1,6 +1,6 @@ import asyncio import json -from typing import Any +from typing import Any, NoReturn import httpx import jsonschema @@ -17,6 +17,10 @@ _imagery_schema: dict | None = None _imagery_schema_lock = asyncio.Lock() +# Test outline: +# @test: Test that this class validates JSON payloads properly against the JSON schema fetched +# @test: Test that malformed JSON or JSON that doesn't validate +# @test: Test that any failed network requests are handled gracefully and return a proper error (not just "Exception") or worse a false positive validation def init_json_schema_client() -> None: global _http_client @@ -29,6 +33,15 @@ def get_http_client() -> httpx.AsyncClient | None: return _http_client +def _require_http_client() -> httpx.AsyncClient: + if _http_client is None: + raise HTTPException( + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, + detail="Schema HTTP client is not initialized", + ) + return _http_client + + async def close_json_schema_client() -> None: global _http_client if _http_client is not None: @@ -38,27 +51,35 @@ async def close_json_schema_client() -> None: async def _fetch_longform_schema() -> dict: global _longform_schema - if _longform_schema is None: - async with _longform_schema_lock: - if _longform_schema is None: - response = await _http_client.get(settings.LONGFORM_SCHEMA_URL) - response.raise_for_status() - _longform_schema = response.json() - return _longform_schema + cached = _longform_schema + if cached is not None: + return cached + async with _longform_schema_lock: + if _longform_schema is None: + response = await _require_http_client().get(settings.LONGFORM_SCHEMA_URL) + response.raise_for_status() + schema: dict = response.json() + _longform_schema = schema + return schema + return _longform_schema async def _fetch_imagery_schema() -> dict: global _imagery_schema - if _imagery_schema is None: - async with _imagery_schema_lock: - if _imagery_schema is None: - response = await _http_client.get(settings.IMAGERY_SCHEMA_URL) - response.raise_for_status() - _imagery_schema = response.json() - return _imagery_schema - - -def _raise_for_fetch_error(e: Exception, label: str) -> None: + cached = _imagery_schema + if cached is not None: + return cached + async with _imagery_schema_lock: + if _imagery_schema is None: + response = await _require_http_client().get(settings.IMAGERY_SCHEMA_URL) + response.raise_for_status() + schema: dict = response.json() + _imagery_schema = schema + return schema + return _imagery_schema + + +def _raise_for_fetch_error(e: Exception, label: str) -> NoReturn: if isinstance(e, httpx.TimeoutException): raise HTTPException( status_code=status.HTTP_504_GATEWAY_TIMEOUT, @@ -103,7 +124,6 @@ async def validate_quest_definition_schema(definition: str) -> None: detail=f"{e.message} at {list(e.path)}", ) - async def validate_imagery_definition_schema(definition: list[Any]) -> None: """ Validate the provided definition against the imagery list schema. diff --git a/api/core/jwt.py b/api/core/jwt.py index 46e04c8..522a8e2 100644 --- a/api/core/jwt.py +++ b/api/core/jwt.py @@ -5,6 +5,10 @@ # Singleton JWKS client reused to take advantage of internal cert/key caching: _jwks_client: jwt.PyJWKClient | None = None +# Test outline: +# @test: Test that this class validates JSON payloads properly against the JSON schema fetched +# @test: Test that malformed JSON or JSON that doesn't validate +# @test: Test that any failed network requests are handled gracefully and return a proper error (not just "Exception") or worse a false positive validation def _get_jwks_client() -> jwt.PyJWKClient: global _jwks_client diff --git a/api/core/security.py b/api/core/security.py index c283af4..7e8baa1 100644 --- a/api/core/security.py +++ b/api/core/security.py @@ -14,6 +14,12 @@ from api.core.logging import get_logger from api.src.users.schemas import WorkspaceUserRoleType +# Test outline: +# @test: Test that the permissions structure here matches what is described in CLAUDE.md +# @test: Test that the methods on the UserInfo class return the correct values for a given set of project groups and workspace roles +# @test: Test that any failed network requests are handled gracefully +# @test: Test that the caching mechanism works correctly and evicts entries when roles change + # Set up logger for this module logger = get_logger(__name__) @@ -22,7 +28,7 @@ # cached record. _user_info_cache: cachetools.TTLCache[UUID, "UserInfo"] = cachetools.TTLCache( maxsize=1000, ttl=60 * 60 -) +) # type: ignore[assignment] # cachetools ctor can't infer key/value types # Shared HTTP client for TDEI backend calls. Initialized by main.py lifespan. _tdei_client: httpx.AsyncClient | None = None @@ -56,7 +62,7 @@ def evict_user_from_cache(auth_uid: UUID) -> None: security = HTTPBearer() - +# @test: Test that this matches what is described in CLAUDE.md and that the methods on the UserInfo class return the correct values for a given set of project groups and workspace roles class TdeiProjectGroupRole(StrEnum): MEMBER = "member" POINT_OF_CONTACT = "poc" @@ -80,7 +86,9 @@ def __init__( self.project_group_id = project_group_id self.tdeiRoles = tdeiRoles - +# @test: Test that the values populated in this class match the expected values from the JWT and TDEI API responses, and that the methods return the correct roles based on the user's project group memberships and workspace roles +# @test: Test that the osmWorkspaceRoles, accessibleWorkspaceIds, and projectGroups attributes are correctly populated based on the user's roles in the OSM DB and TDEI API responses +# @test: Test that the comments in this doc that describe what is supposed to be in the attributes are accurate and match the actual data being stored in those attributes and what is returned. The comments should be considered authoratative class UserInfo: credentials: str user_uuid: UUID @@ -159,6 +167,7 @@ def get_task_db_session( ) -> AsyncSession: return session +# @test: async def validate_token( credentials: HTTPAuthorizationCredentials = Depends(security), @@ -245,8 +254,15 @@ async def _validate_token_uncached( # get user's project groups and roles from TDEI pgs = [] + client = _tdei_client + if client is None: + raise HTTPException( + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, + detail="TDEI client is not initialized", + ) + try: - response = await _tdei_client.get( + response = await client.get( f"project-group-roles/{user_uuid}", headers=headers, params={"page_no": 1, "page_size": 1000}, diff --git a/api/main.py b/api/main.py index dd82fcc..a2582dc 100644 --- a/api/main.py +++ b/api/main.py @@ -46,6 +46,15 @@ _osm_client: httpx.AsyncClient | None = None +def _require_osm_client() -> httpx.AsyncClient: + if _osm_client is None: + raise HTTPException( + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, + detail="OSM proxy client is not initialized", + ) + return _osm_client + + @asynccontextmanager async def lifespan(_app: FastAPI): # only run migrations when not under test @@ -111,6 +120,18 @@ def get_workspace_repository( return WorkspaceRepository(session) +# @test: Any headers defined in STRIP_REQUEST_HEADERS are not forwarded to the OSM service +# @test: Any headers defined in HOP_BY_HOP_HEADERS are not forwarded to the client +# @test: /api/capabilities.json is proxied to the OSM service without requiring authentication +# @test: Any request to the OSM service that returns a 4xx or 5xx status code is logged to Sentry with the correct message and the correct status code is returned to the client +# @test: Any request that matches the RegEx in TENANT_BYPASSES is allowed to proceed without an X-Workspace header, +# and any request that does not match the Regex in TENANT_BYPASSES and does not have an X-Workspace header returns a 400 Bad Request error +# @test: Only the methods defined in the @app.api_route decorator are allowed to be proxied to the OSM service, and any other methods return a 405 Method Not Allowed error +# @test: Any request with an X-Workspace header that does not match the user's accessible workspaces returns a 403 Forbidden error +# @test: Any request with a missing X-Workspace header that does not match the TENANT_BYPASSES returns a 400 Bad Request error +# @test: All the values for Host, X-Real-IP, X-Forwarded-For, X-Forwarded-Host, and X-Forwarded-Proto headers are correctly set when proxied to the OSM service +# @test: The response from the OSM service is correctly streamed back to the client with the correct status code and headers, and the response body is not modified in any way + # This API route catches anything not otherwise defined above--MUST be last in this file # # h/t: https://stackoverflow.com/questions/70610266/proxy-an-external-website-using-python-fast-api-not-supporting-query-params @@ -154,13 +175,14 @@ def get_workspace_repository( async def capabilities(request: Request): """Proxy OSM capabilities manifest without requiring authentication.""" + client = _require_osm_client() client_host = request.client.host if request.client else "unknown" req_headers = [ (k.encode(), v.encode()) for k, v in request.headers.items() if k.lower() not in STRIP_REQUEST_HEADERS ] + [ - (b"Host", _osm_client.base_url.host.encode()), + (b"Host", client.base_url.host.encode()), (b"X-Real-IP", client_host.encode()), (b"X-Forwarded-For", client_host.encode()), (b"X-Forwarded-Host", (request.url.hostname or "").encode()), @@ -168,10 +190,10 @@ async def capabilities(request: Request): ] url = httpx.URL(path="/api/capabilities.json") - rp_req = _osm_client.build_request("GET", url, headers=req_headers) + rp_req = client.build_request("GET", url, headers=req_headers) try: - rp_resp = await _osm_client.send(rp_req, stream=True) + rp_resp = await client.send(rp_req, stream=True) except httpx.TimeoutException: raise HTTPException( status_code=status.HTTP_504_GATEWAY_TIMEOUT, @@ -236,7 +258,7 @@ async def catch_all( path=request.url.path.strip(), query=request.url.query.encode("utf-8") ) - client = _osm_client + client = _require_osm_client() client_host = request.client.host if request.client else "unknown" req_headers = [ (k.encode(), v.encode()) diff --git a/api/src/teams/repository.py b/api/src/teams/repository.py index 2b6c09e..597aa96 100644 --- a/api/src/teams/repository.py +++ b/api/src/teams/repository.py @@ -1,3 +1,9 @@ +# SQLModel declares columns as plain annotations (e.g. ``id: int | None``) rather +# than ``Mapped[int]``, so Pyright reads ``Column == value`` as a ``bool`` and +# misjudges ``where()``/``exec()``/``select()``/``selectinload`` calls. These are +# framework false positives; the queries are valid at runtime. +# pyright: reportArgumentType=false, reportCallIssue=false, reportAttributeAccessIssue=false + from sqlalchemy import delete, exists, select from sqlalchemy.orm import selectinload from sqlmodel.ext.asyncio.session import AsyncSession diff --git a/api/src/teams/routes.py b/api/src/teams/routes.py index 5163259..2e1e760 100644 --- a/api/src/teams/routes.py +++ b/api/src/teams/routes.py @@ -36,6 +36,11 @@ def get_team_repo( repo = WorkspaceTeamRepository(session) return repo +# @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace member of any level +# @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs +# @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 +# @test: Test that this endpoint properly calls the repository to fetch the team from the database +# @test: Test that this method properly handles inputs that match the schema in WorkspaceTeamItem @router.get("") async def get_all_teams_for_workspace( @@ -48,6 +53,11 @@ async def get_all_teams_for_workspace( await workspace_repo.getById(current_user, workspace_id) return await team_repo.get_all(workspace_id) +# @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace lead +# @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs +# @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 +# @test: Test that this endpoint properly calls the repository to add the team to the database +# @test: Test that this method properly handles inputs that match the schema in WorkspaceTeamCreate @router.post("", status_code=status.HTTP_201_CREATED) async def create_team_for_workspace( @@ -67,6 +77,12 @@ async def create_team_for_workspace( await workspace_repo.getById(current_user, workspace_id) return await team_repo.create(workspace_id, team) +# @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace member of any level +# @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs +# @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 +# @test: Test that this endpoint properly handles the case where the team does not exist and returns a 404 +# @test: Test that this endpoint properly calls the repository to fetch the team from the database +# @test: Test that this method properly handles inputs that match the schema in WorkspaceTeamItem @router.get("/{team_id}") async def get_team_for_workspace( @@ -81,6 +97,14 @@ async def get_team_for_workspace( await team_repo.assert_team_in_workspace(team_id, workspace_id) return await team_repo.get_item(team_id) +# @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace lead +# @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs +# @test: Test that this endpoint properly handles the case where the team does not exist and returns a 404 +# @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 +# @test: Test that this endpoint properly handles the case where the workspace is not associated with the team +# @test: Test that this endpoint properly calls the repository to remove the team from the workspace +# @test: Test that this method properly handles inputs that match the schema in WorkspaceTeamUpdate +# @test: Test that this method properly calls the repo to update the team @router.put("/{team_id}", status_code=status.HTTP_204_NO_CONTENT) async def update_team_for_workspace( @@ -102,6 +126,12 @@ async def update_team_for_workspace( await team_repo.assert_team_in_workspace(team_id, workspace_id) await team_repo.update(team_id, team) +# @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace lead +# @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs +# @test: Test that this endpoint properly handles the case where the team does not exist and returns a 404 +# @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 +# @test: Test that this endpoint properly handles the case where the workspace is not associated with the team +# @test: Test that this endpoint properly calls the repository to remove the team from the workspace @router.delete("/{team_id}", status_code=status.HTTP_204_NO_CONTENT) async def delete_team_from_workspace( @@ -122,6 +152,14 @@ async def delete_team_from_workspace( await team_repo.assert_team_in_workspace(team_id, workspace_id) await team_repo.delete(team_id) +# @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a member team +# @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs +# @test: Test that this endpoint properly handles the case where the team does not exist and returns a 404 +# @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 +# @test: Test that this endpoint properly handles the case where the team is not associated with the workspace +# @test: Test that this endpoint properly handles the case where the user is not a member of the workspace passed +# @test: Test that this endpoint properly handles the case where the workspace is not associated with the team passed +# @test: Test that this endpoint properly calls the repository to fetch the users of the team @router.get("/{team_id}/members") async def get_members_in_workspace_team( @@ -137,6 +175,14 @@ async def get_members_in_workspace_team( return await team_repo.get_members(team_id) +# @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a member of the workspace via the team +# @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs +# @test: Test that this endpoint properly handles the case where the team does not exist and returns a 404 +# @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 +# @test: Test that this endpoint properly handles the case where the user is not a member of the workspace passed +# @test: Test that this endpoint properly handles the case where the workspace is not associated with the team passed +# @test: Test that this endpoint properly calls the repository to add the user to the team + @router.post("/{team_id}/members") async def join_workspace_team( workspace_id: int, @@ -153,6 +199,14 @@ async def join_workspace_team( await team_repo.add_member(team_id, user.id) return user +# @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace lead +# @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs +# @test: Test that this endpoint properly handles the case where the team does not exist and returns a 404 +# @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 +# @test: Test that this endpoint properly handles the case where the user does not exist and returns a 404 +# @test: Test that this endpoint properly handles the case where the workspace is not associated with the team +# @test: Test that this endpoint doesn't allow changing teams the user doesn't have workspace lead permissions for, or users not already associated with the workspace +# @test: Test that this endpoint properly calls the repository to add the user to the team @router.put("/{team_id}/members/{user_id}", status_code=status.HTTP_204_NO_CONTENT) async def add_member_to_workspace_team( @@ -174,6 +228,16 @@ async def add_member_to_workspace_team( await team_repo.assert_team_in_workspace(team_id, workspace_id) await team_repo.add_member(team_id, user_id) +# @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace lead +# @test: Test that the endpoint properly validates that the team needs to be associated with this workspace and errors if not +# @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs +# @test: Test that this endpoint properly handles the case where the team does not exist and returns a 404 +# @test: Test that this endpoint properly handles the case where the user does not exist and returns a 404 +# @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 +# @test: Test that this endpoint properly handles the case where the user is not associated with the team +# @test: Test that this endpoint properly handles the case where the workspace is not associated with the team +# @test: Test that this endpoint doesn't allow changing teams the user doesn't have workspace lead permissions for, or users not already associated with the team +# @test: Test that this endpoint properly calls the repository to remove the user from the team @router.delete("/{team_id}/members/{user_id}", status_code=status.HTTP_204_NO_CONTENT) async def delete_member_from_workspace_team( diff --git a/api/src/teams/schemas.py b/api/src/teams/schemas.py index 7d623d4..0aefcf0 100644 --- a/api/src/teams/schemas.py +++ b/api/src/teams/schemas.py @@ -3,13 +3,16 @@ from sqlmodel import Field, Relationship, SQLModel if TYPE_CHECKING: - from api.src.workspaces.schemas import User - + from api.src.users.schemas import User +# @test: Test that this class matches the Alembic-defined database schema +# @test: Test that the foreign key references are correct and that the relationships are correctly defined +# @test: Test that any values from Python and properly serialized to the database and that any values from the database are properly deserialized to Python +# @test: Test that any serialization/deserialization doesn't lose any precision or data class WorkspaceTeamUser(SQLModel, table=True): """Team to User link table""" - __tablename__ = "team_user" + __tablename__ = "team_user" # type: ignore[assignment] team_id: int | None = Field(default=None, primary_key=True, foreign_key="teams.id") user_id: int | None = Field(default=None, primary_key=True, foreign_key="users.id") @@ -20,7 +23,10 @@ class WorkspaceTeamBase(SQLModel): name: str = Field(min_length=1) - +# @test: Test that this class matches the Alembic-defined database schema +# @test: Test that the foreign key references are correct and that the relationships are correctly defined +# @test: Test that any values from Python and properly serialized to the database and that any values from the database are properly deserialized to Python +# @test: Test that any serialization/deserialization doesn't lose any precision or data class WorkspaceTeam(WorkspaceTeamBase, table=True): """Workspace teams""" @@ -43,6 +49,7 @@ class WorkspaceTeamItem(WorkspaceTeamBase): @classmethod def from_team(cls, team: WorkspaceTeam) -> Self: + assert team.id is not None # persisted team always has an id return cls(id=team.id, name=team.name, member_count=len(team.users)) diff --git a/api/src/users/repository.py b/api/src/users/repository.py index f7769e4..2961ad9 100644 --- a/api/src/users/repository.py +++ b/api/src/users/repository.py @@ -1,3 +1,9 @@ +# SQLModel declares columns as plain annotations (e.g. ``id: int | None``) rather +# than ``Mapped[int]``, so Pyright reads ``Column == value`` as a ``bool`` and +# misjudges ``where()``/``exec()``/``select()`` calls and ``result.rowcount``. +# These are framework false positives; the queries are valid at runtime. +# pyright: reportArgumentType=false, reportCallIssue=false, reportAttributeAccessIssue=false + from uuid import UUID from sqlalchemy import delete, select diff --git a/api/src/users/routes.py b/api/src/users/routes.py index 54d3527..9a96162 100644 --- a/api/src/users/routes.py +++ b/api/src/users/routes.py @@ -24,6 +24,10 @@ def get_workspace_repo( ) -> WorkspaceRepository: return WorkspaceRepository(session) +# @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not associated with this workspace at contributor or above +# @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs +# @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 +# @test: Test that this endpoint properly handles the case where the user is not associated with the workspace (returns a 403 error) @router.get("", response_model=list[WorkspaceUserRoleItem]) async def get_privileged_workspace_members( @@ -40,6 +44,16 @@ async def get_privileged_workspace_members( return await user_repo.get_privileged_workspace_members(workspace_id) +# @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace lead +# @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs +# @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 +# @test: Test that this endpoint properly handles the case where the user does not exist and returns a 404 +# @test: Test that this endpoint properly handles the case where the user is not associated with the workspace +# @test: Test that this endpoint properly evicts any cached data for the user after the user is deleted so that their next request reflects the deletion rather than serving stale data for up to an hour +# @test: Test that this endpoint properly allows users to be workspace leads or validators, and to unset the user of either role and become a contributor again +# @test: Test that this endpoint doesn't allow changing workspaces the user doesn't have workspace lead permissions for, or roles for users not already associated with the workspace +# @test: Test that this endpoint doesn't allow setting the workspace role to a POC + @router.put("/{user_id}/role", status_code=status.HTTP_204_NO_CONTENT) async def assign_member_role( workspace_id: int, @@ -64,6 +78,12 @@ async def assign_member_role( await user_repo.assign_member_role(workspace_id, user_id, body.role) evict_user_from_cache(user_id) +# @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace lead +# @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs +# @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 +# @test: Test that this endpoint properly handles the case where the user does not exist and returns a 404 +# @test: Test that this endpoint properly handles the case where the user is not associated with the workspace +# @test: Test that this endpoint properly evicts any cached data for the user after the user is deleted so that their next request reflects the deletion rather than serving stale data for up to an hour @router.delete("/{user_id}", status_code=status.HTTP_204_NO_CONTENT) async def remove_member_role( diff --git a/api/src/users/schemas.py b/api/src/users/schemas.py index 7b4731b..bc8e3be 100644 --- a/api/src/users/schemas.py +++ b/api/src/users/schemas.py @@ -16,7 +16,10 @@ class WorkspaceUserRoleType(StrEnum): VALIDATOR = "validator" CONTRIBUTOR = "contributor" - +# @test: Test that this class matches the Alembic-defined database schema +# @test: Test that the foreign key references are correct and that the relationships are correctly defined +# @test: Test that any values from Python and properly serialized to the database and that any values from the database are properly deserialized to Python +# @test: Test that any serialization/deserialization doesn't lose any precision or data class WorkspaceUserRole(SQLModel, table=True): """Associates users with workspaces and their roles""" @@ -62,7 +65,10 @@ class WorkspaceUserRoleItem(SQLModel): display_name: str role: WorkspaceUserRoleType - +# @test: Test that this class matches the Alembic-defined database schema +# @test: Test that the foreign key references are correct and that the relationships are correctly defined +# @test: Test that any values from Python and properly serialized to the database and that any values from the database are properly deserialized to Python +# @test: Test that any serialization/deserialization doesn't lose any precision or data class User(SQLModel, table=True): """Users in the OSM DB""" diff --git a/api/src/workspaces/repository.py b/api/src/workspaces/repository.py index 8c1c87c..3c16cb0 100644 --- a/api/src/workspaces/repository.py +++ b/api/src/workspaces/repository.py @@ -1,3 +1,10 @@ +# SQLModel declares columns as plain annotations (e.g. ``id: int | None``) rather +# than ``Mapped[int]``, so Pyright reads ``Column == value`` as a ``bool`` and +# misjudges ``where()``/``select()`` calls, ``result.rowcount``, and table-model +# constructors. These are framework false positives; the queries are valid at +# runtime. Genuine type bugs surface via other rules, which stay enabled. +# pyright: reportArgumentType=false, reportCallIssue=false, reportAttributeAccessIssue=false + from sqlalchemy import delete, select, text, update from sqlalchemy.exc import IntegrityError from sqlmodel.ext.asyncio.session import AsyncSession @@ -142,8 +149,11 @@ async def resolve_quest_def(quest: WorkspaceLongQuest | None) -> str | None: return quest.definition or None if quest.url: + client = get_http_client() + if client is None: + return None try: - response = await get_http_client().get(quest.url, timeout=10) + response = await client.get(quest.url, timeout=10) return response.text except Exception: return None diff --git a/api/src/workspaces/routes.py b/api/src/workspaces/routes.py index 2ba802f..fdc0a95 100644 --- a/api/src/workspaces/routes.py +++ b/api/src/workspaces/routes.py @@ -1,5 +1,4 @@ import json -from typing import Any from uuid import UUID from fastapi import APIRouter, Depends, HTTPException, Response, status @@ -20,7 +19,6 @@ QuestDefinitionTypeName, QuestSettingsPatch, QuestSettingsResponse, - Workspace, WorkspaceCreate, WorkspaceImagery, WorkspacePatch, @@ -53,6 +51,14 @@ def get_user_repository( return UserRepository(session) +# @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs +# @test: Test that this method properly calls the repository method to fetch the workspace and that the repository method properly fetches the workspace from the database +# @test: Test that this method properly handles numeric workspace_id input and invalid values for the same +# @test: Test that this method properly checks permissions to see if the user has access to the workspace and if they don't, it doesn't appear in the list +# @test: Test that this method properly handles the case where the workspace does not exist and doesn't include it +# @test: Test that this method properly handles inputs that match the schema in WorkspaceResponse +# @test: Test that this method's results match the values of the fetch-by-workspace-id method below; all workspaces in this list are retrievable via that method + # Returns list of workspaces user has access to as JSON payload on success--returns empty JSON list if none @router.get("/mine", response_model=list[WorkspaceResponse]) async def get_my_workspaces( @@ -66,6 +72,12 @@ async def get_my_workspaces( logger.error(f"Failed to fetch workspaces: {str(e)}") raise +# @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs +# @test: Test that this method properly calls the repository method to fetch the workspace and that the repository method properly fetches the workspace from the database +# @test: Test that this method properly handles numeric workspace_id input and invalid values for the same +# @test: Test that this method properly checks permissions to see if the user has access to the workspace and returns a 403 if they do not +# @test: Test that this method properly handles the case where the workspace does not exist and returns a 404 +# @test: Test that this method properly handles inputs that match the schema in WorkspaceResponse # Returns JSON payload or 204 if not found @router.get("/{workspace_id}", response_model=WorkspaceResponse) @@ -98,6 +110,10 @@ async def get_workspace( logger.error(f"Failed to fetch workspace {workspace_id}: {str(e)}") raise +# @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs +# @test: Test that this method properly handles numeric workspace_id input and invalid values for the same +# @test: Test taht this workspace returns proper values for a workspace that exists and that the bbox matches the expected values +# @test: Test that this method properly handles the case where the workspace does not exist and returns a 404 @router.get("/{workspace_id}/bbox", response_model=None) async def get_workspace_bbox( @@ -115,6 +131,13 @@ async def get_workspace_bbox( logger.error(f"Failed to fetch workspace {workspace_id}: {str(e)}") raise +# @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs +# @test: Test that this endpoint properly evicts any cached data for the user after the workspace is deleted so that their next request reflects the deletion rather than serving stale data for up to an hour +# @test: Test that this method properly calls the repository method to create the workspace and that the repository method properly creates the workspace in the database +# @test: Test that this method properly sets the creator as the lead of the workspace and that the repository method properly assigns the lead role in the database +# @test: Test that this method properly handles inputs that match the schema in WorkspaceCreate and that the repository method properly creates the workspace in the database with those values +# @test: Test that this method properly handles inputs that do not match the schema in WorkspaceCreate and that the repository method properly raises an error and does not update the workspace in the database with those values +# @test: Test that this method won't allow users to modify an existing workspace in any way, or create a workspace with the same workspace_id as an existing one # Returns 201 on success? @router.post("", status_code=status.HTTP_201_CREATED) @@ -126,6 +149,7 @@ async def create_workspace( ) -> dict[str, int]: try: workspace = await repository_ws.create(current_user, workspace_data) + assert workspace.id is not None # freshly persisted workspace has an id # Assign the creator as lead so that non-POC members can manage their # own workspace: @@ -147,6 +171,13 @@ async def create_workspace( logger.error(f"Failed to create workspace: {str(e)}") raise +# @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace lead +# @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs +# @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 +# @test: Test that this method properly calls the repository method to update the workspace and that the repository method properly updates the workspace in the database +# @test: Test that this method properly handles numeric workspace_id input and invalid values for the same +# @test: Test that this method properly handles inputs that match the schema in WorkspacePatch and that the repository method properly updates the workspace in the database with those values +# @test: Test that this method properly handles inputs that do not match the schema in WorkspacePatch and that the repository method properly raises an error and does not update the workspace in the database with those values @router.patch("/{workspace_id}", status_code=status.HTTP_204_NO_CONTENT) async def update_workspace( @@ -174,6 +205,14 @@ async def update_workspace( raise +# @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace lead +# @test: Test that this endpoint properly evicts any cached data for the user after the workspace is deleted so that their next request reflects the deletion rather than serving stale data for up to an hour +# @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs +# @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 +# @test: Test that this method properly calls the repository method to delete the workspace and that the repository method properly deletes the workspace from the database +# @test: Test that this method properly handles the case where the workspace has members and that the repository method properly removes all member roles from the database +# @test: Test that this method properly handles numeric workspace_id input and invalid values for the same + # Returns 204 on success @router.delete("/{workspace_id}", status_code=status.HTTP_204_NO_CONTENT) async def delete_workspace( @@ -201,6 +240,12 @@ async def delete_workspace( # QUESTS +# @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace lead +# @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs +# @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 +# @test: Test that this method properly calls the repository method to fetch the long quest definition + +# FIXME: Why are there two methods to fetch the long quest? One for legacy purposes? Can we migrate callers? # Return the resolved quest definition content as JSON, or 204 if not set: @router.get("/{workspace_id}/quests/long") @@ -225,6 +270,11 @@ async def get_long_quest_def( ) raise +# @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace lead +# @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs +# @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 +# @test: Test that this method properly calls the repository method to fetch the long quest definition, and if it's not defined, the default value as defined +# in this method # Returns JSON payload or 204 if not set @router.get( @@ -263,6 +313,13 @@ async def get_long_quest_settings( logger.error(f"Failed to fetch workspace {workspace_id}: {str(e)}") raise +# @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace lead +# @test: Test that this endpoint properly validates the long quest definition against the JSON schema and returns a 400 if the definition is invalid +# @test: Test that this endpoint properly saves the long quest definition to the database and returns a 204 on success +# @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs +# @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 +# @test: Test that this endpoint properly handles input values that are properly formed, malformed and edge cases, including empty lists, null values, and large payloads +# @test: Test that this method properly calls the repository method to save the long quest definition and that the repository method properly saves the definition to the database # Returns 204 on success @router.patch( @@ -281,6 +338,11 @@ async def update_long_quest_settings( ) if long_quest_data.type == QuestDefinitionTypeName.JSON: + if long_quest_data.definition is None: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="'definition' is required for JSON quest type.", + ) await validate_quest_definition_schema(long_quest_data.definition) try: @@ -294,6 +356,10 @@ async def update_long_quest_settings( # IMAGERY +# @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace lead +# @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs +# @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 +# @test: Test that this method properly calls the repository method to fetch the imagery definition # Returns JSON payload or 204 if not set @router.get("/{workspace_id}/imagery/settings") @@ -320,6 +386,14 @@ async def get_imagery_settings( raise +# @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace lead +# @test: Test that this endpoint properly validates the imagery definition against the JSON schema and returns a 400 if the definition is invalid +# @test: Test that this endpoint properly saves the imagery definition to the database and returns a 204 on success +# @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs +# @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 +# @test: Test that this endpoint properly handles input values that are properly formed, malformed and edge cases, including empty lists, null values, and large payloads +# @test: Test that this method properly calls the repository method to save the imagery definition and that the repository method properly saves the definition to the database + # Returns 204 on success @router.patch( "/{workspace_id}/imagery/settings", status_code=status.HTTP_204_NO_CONTENT diff --git a/api/src/workspaces/schemas.py b/api/src/workspaces/schemas.py index fa86ce6..34f28f7 100644 --- a/api/src/workspaces/schemas.py +++ b/api/src/workspaces/schemas.py @@ -78,7 +78,10 @@ class QuestDefinitionTypeName(StrEnum): JSON = "JSON" URL = "URL" - +# @test: Test that this class matches the Alembic-defined database schema +# @test: Test that the foreign key references are correct and that the relationships are correctly defined +# @test: Test that any values from Python and properly serialized to the database and that any values from the database are properly deserialized to Python +# @test: Test that any serialization/deserialization doesn't lose any precision or data class WorkspaceLongQuest(SQLModel, table=True): """Stores mobile app quest definitions for a workspace""" @@ -102,7 +105,10 @@ class WorkspaceLongQuest(SQLModel, table=True): modifiedBy: UUID modifiedByName: str - +# @test: Test that this class matches the Alembic-defined database schema +# @test: Test that the foreign key references are correct and that the relationships are correctly defined +# @test: Test that any values from Python and properly serialized to the database and that any values from the database are properly deserialized to Python +# @test: Test that any serialization/deserialization doesn't lose any precision or data class WorkspaceImagery(SQLModel, table=True): """Stores imagery list for a workspace""" @@ -215,6 +221,9 @@ class WorkspaceResponse(SQLModel): longFormQuestDef: Optional[Any] = None imageryListDef: Optional[Any] = None + # @test: Test that this class properly serializes the workspace data for API responses, including the effective role for the user making the request + # @test: Test that the values are populated in this class match the expected values from the database and that the relationships are correctly serialized + @classmethod def from_workspace( cls, @@ -224,6 +233,7 @@ def from_workspace( imagery_list_def: Any = None, long_form_quest_def: Any = None, ) -> Self: + assert workspace.id is not None # persisted workspace always has an id return cls( id=workspace.id, type=workspace.type, @@ -243,7 +253,10 @@ def from_workspace( longFormQuestDef=long_form_quest_def, ) - +# @test: Test that this class matches the Alembic-defined database schema +# @test: Test that the foreign key references are correct and that the relationships are correctly defined +# @test: Test that any values from Python and properly serialized to the database and that any values from the database are properly deserialized to Python +# @test: Test that any serialization/deserialization doesn't lose any precision or data class Workspace(SQLModel, table=True): """Workspaces""" From fa8df698e28513e6b63d9079e6ab0476f60bb1bd Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Wed, 24 Jun 2026 14:18:26 -0400 Subject: [PATCH 04/18] Tests --- tests/conftest.py | 12 + tests/integration/test_proxy.py | 176 ++++++++-- tests/integration/test_teams.py | 341 +++++++++++++++++-- tests/integration/test_users.py | 186 ++++++++++ tests/integration/test_workspaces.py | 466 +++++++++++++++++++++++--- tests/support/fakes.py | 18 +- tests/unit/test_config.py | 59 ++++ tests/unit/test_json_schema.py | 137 ++++++++ tests/unit/test_jwt.py | 71 ++++ tests/unit/test_security.py | 334 ++++++++++++++++++ tests/unit/test_teams_schemas.py | 69 ++++ tests/unit/test_users_schemas.py | 78 +++++ tests/unit/test_workspaces_schemas.py | 202 +++++++++++ 13 files changed, 2037 insertions(+), 112 deletions(-) create mode 100644 tests/integration/test_users.py create mode 100644 tests/unit/test_config.py create mode 100644 tests/unit/test_json_schema.py create mode 100644 tests/unit/test_jwt.py create mode 100644 tests/unit/test_security.py create mode 100644 tests/unit/test_teams_schemas.py create mode 100644 tests/unit/test_users_schemas.py create mode 100644 tests/unit/test_workspaces_schemas.py diff --git a/tests/conftest.py b/tests/conftest.py index 2fef3b5..a5b6c4b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -67,3 +67,15 @@ async def client(app): transport = ASGITransport(app=app) async with AsyncClient(transport=transport, base_url="http://testserver") as c: yield c + + +@pytest_asyncio.fixture +async def error_client(app): + """Like ``client`` but turns unhandled exceptions into 500 responses. + + By default httpx's ASGI transport re-raises app exceptions; this fixture + lets tests assert on the 500 the server would actually return in prod. + """ + transport = ASGITransport(app=app, raise_app_exceptions=False) + async with AsyncClient(transport=transport, base_url="http://testserver") as c: + yield c diff --git a/tests/integration/test_proxy.py b/tests/integration/test_proxy.py index aab0ffd..6cd818d 100644 --- a/tests/integration/test_proxy.py +++ b/tests/integration/test_proxy.py @@ -1,68 +1,190 @@ -"""Integration tests for the OSM proxy (catch-all) routes. - -The proxy forwards to an upstream OSM service via a module-level -``httpx.AsyncClient`` and re-streams the response. We swap that client for -one backed by a streamable mock transport so the proxy path is exercised -end-to-end without a real upstream. +"""Integration tests for the OSM proxy (catch-all + capabilities) routes. + +Covers the @test comments in api/main.py: +- STRIP_REQUEST_HEADERS are not forwarded upstream +- HOP_BY_HOP_HEADERS are not forwarded back to the client +- /api/capabilities.json is proxied without auth +- 4xx/5xx upstream responses are logged to Sentry and the status is preserved +- TENANT_BYPASSES allow specific paths/methods without an X-Workspace header +- only the decorator's methods are proxied (others -> 405) +- X-Workspace not in the user's accessible workspaces -> 403 +- missing X-Workspace and no bypass -> 400 +- Host / X-Real-IP / X-Forwarded-* are set correctly upstream +- the response is streamed back unmodified with its status and headers """ +import httpx import pytest import api.main from tests.support import factories -from tests.support.http import osm_mock_client +from tests.support.http import StreamingMockTransport @pytest.fixture def mock_osm(monkeypatch): - """Replace the upstream OSM client with a recording mock transport.""" - client, transport = osm_mock_client() - monkeypatch.setattr(api.main, "_osm_client", client) + """Default upstream returning 200 text/xml; records the forwarded request.""" + transport = StreamingMockTransport( + lambda req: (200, {"content-type": "text/xml"}, b"") + ) + monkeypatch.setattr( + api.main, + "_osm_client", + httpx.AsyncClient(transport=transport, base_url="http://osm-web"), + ) + return transport + + +def install_osm(monkeypatch, handler): + transport = StreamingMockTransport(handler) + monkeypatch.setattr( + api.main, + "_osm_client", + httpx.AsyncClient(transport=transport, base_url="http://osm-web"), + ) return transport +# --- capabilities ---------------------------------------------------------- + + async def test_capabilities_proxies_without_auth(client, mock_osm): - # No X-Workspace header and no bearer token required for capabilities. response = await client.get("/api/capabilities.json") - assert response.status_code == 200 assert b"osm version" in response.content assert mock_osm.last_request.url.path == "/api/capabilities.json" -async def test_proxy_requires_workspace_header(client, login, mock_osm): +# --- auth / tenant gating -------------------------------------------------- + + +async def test_missing_workspace_header_without_bypass_returns_400( + client, login, mock_osm +): login(factories.make_user_info()) - # A non-whitelisted path with no X-Workspace header is rejected. response = await client.get("/api/0.6/map") - assert response.status_code == 400 -async def test_proxy_forbidden_without_workspace_access(client, login, mock_osm): - # User has no access to workspace 1. +async def test_workspace_header_without_access_returns_403(client, login, mock_osm): login(factories.make_user_info(accessible_workspace_ids={})) + response = await client.get("/api/0.6/map", headers={"X-Workspace": "1"}) + assert response.status_code == 403 + +async def test_non_integer_workspace_header_returns_400(client, login, mock_osm): + login(factories.make_user_info(accessible_workspace_ids={"pg": [1]})) + response = await client.get("/api/0.6/map", headers={"X-Workspace": "abc"}) + assert response.status_code == 400 + + +async def test_contributor_request_is_proxied(client, login, mock_osm): + login(factories.make_user_info(accessible_workspace_ids={"pg": [1]})) response = await client.get("/api/0.6/map", headers={"X-Workspace": "1"}) + assert response.status_code == 200 + assert b"osm version" in response.content - assert response.status_code == 403 + +async def test_tenant_bypass_allows_workspace_put_without_header( + client, login, mock_osm +): + # PUT /api/0.6/workspaces/{id} is in TENANT_BYPASSES -> no X-Workspace needed. + login(factories.make_user_info()) + response = await client.put("/api/0.6/workspaces/123") + assert response.status_code == 200 + assert mock_osm.last_request is not None -async def test_proxy_forwards_for_workspace_contributor(client, login, mock_osm): +async def test_tenant_bypass_does_not_apply_to_wrong_method(client, login, mock_osm): + # The bypass for /workspaces/{id} is PUT/DELETE only; GET still needs a header. + login(factories.make_user_info()) + response = await client.get("/api/0.6/workspaces/123") + assert response.status_code == 400 + + +async def test_disallowed_method_returns_405(client, login, mock_osm): + login(factories.make_user_info(accessible_workspace_ids={"pg": [1]})) + # TRACE is not in the @app.api_route methods list. + response = await client.request( + "TRACE", "/api/0.6/map", headers={"X-Workspace": "1"} + ) + assert response.status_code == 405 + + +# --- header handling ------------------------------------------------------- + + +async def test_spoofed_request_headers_are_stripped(client, login, mock_osm): + login(factories.make_user_info(accessible_workspace_ids={"pg": [1]})) + + await client.get( + "/api/0.6/map", + headers={ + "X-Workspace": "1", + "Host": "evil.example", + "X-Forwarded-For": "9.9.9.9", + "X-Real-IP": "9.9.9.9", + }, + ) + + fwd = mock_osm.last_request.headers + # Host is rewritten to the upstream host, not the spoofed value. + assert fwd["host"] == "osm-web" + # X-Forwarded-* / X-Real-IP are set by the proxy, not passed through. + assert fwd["x-forwarded-for"] != "9.9.9.9" + assert fwd["x-real-ip"] != "9.9.9.9" + assert "x-forwarded-proto" in fwd + assert "x-forwarded-host" in fwd + + +async def test_hop_by_hop_response_headers_are_stripped(client, login, monkeypatch): + install_osm( + monkeypatch, + lambda req: ( + 200, + {"content-type": "text/xml", "keep-alive": "timeout=5", "x-custom": "v"}, + b"", + ), + ) login(factories.make_user_info(accessible_workspace_ids={"pg": [1]})) response = await client.get("/api/0.6/map", headers={"X-Workspace": "1"}) assert response.status_code == 200 - assert b"osm version" in response.content - # Spoofable forwarding headers are normalized before reaching upstream. - forwarded = mock_osm.last_request.headers - assert "x-forwarded-for" in forwarded # set by the proxy itself - assert forwarded["host"] == "osm-web" + # Non-hop-by-hop headers pass through; hop-by-hop ones are dropped. + assert response.headers.get("x-custom") == "v" + assert "keep-alive" not in response.headers -async def test_proxy_rejects_non_integer_workspace_header(client, login, mock_osm): +# --- upstream status + body fidelity --------------------------------------- + + +async def test_upstream_error_is_logged_and_status_preserved( + client, login, monkeypatch +): + install_osm(monkeypatch, lambda req: (503, {"content-type": "text/plain"}, b"down")) + captured = [] + monkeypatch.setattr( + api.main.sentry_sdk, "capture_message", lambda msg: captured.append(msg) + ) login(factories.make_user_info(accessible_workspace_ids={"pg": [1]})) - response = await client.get("/api/0.6/map", headers={"X-Workspace": "abc"}) + response = await client.get("/api/0.6/map", headers={"X-Workspace": "1"}) - assert response.status_code == 400 + assert response.status_code == 503 + assert captured, "expected a Sentry capture_message for the 5xx upstream response" + assert "503" in captured[0] + + +async def test_response_body_is_streamed_unmodified(client, login, monkeypatch): + body = b"\n \n" + install_osm( + monkeypatch, lambda req: (200, {"content-type": "application/xml"}, body) + ) + login(factories.make_user_info(accessible_workspace_ids={"pg": [1]})) + + response = await client.get("/api/0.6/map", headers={"X-Workspace": "1"}) + + assert response.status_code == 200 + assert response.content == body + assert response.headers["content-type"] == "application/xml" diff --git a/tests/integration/test_teams.py b/tests/integration/test_teams.py index fdd92a0..41ae522 100644 --- a/tests/integration/test_teams.py +++ b/tests/integration/test_teams.py @@ -1,22 +1,48 @@ """Integration tests for the /workspaces/{id}/teams routes. -Team routes touch BOTH databases: the workspace access guard hits the task -DB (``workspace_repo.getById``) and the team operations hit the OSM DB. -Queue results on the matching fake session in call order. +Covers the @test comments in api/src/teams/routes.py across all eight +endpoints. Team routes touch both DBs: the access guard / workspace lookup +hits the task DB (``workspace_repo.getById``) and team/user operations hit +the OSM DB. Queue results on the matching fake session in call order. + +Note on access errors: for endpoints with no explicit lead check, access is +enforced by ``getById``, which raises 404 (NotFound) when the workspace is +missing or inaccessible -- so "not a member" surfaces as 404, not 403, there. """ +import pytest + +from api.src.users.schemas import WorkspaceUserRoleType from tests.support import factories, fakes -def teams_url(workspace_id: int = 1) -> str: +def base(workspace_id=1): return f"/api/v1/workspaces/{workspace_id}/teams" -async def test_list_teams(client, login, task_session, osm_session): - login() - # getById (task DB) guards access... +@pytest.fixture +def lead(): + return factories.make_user_info( + osm_workspace_roles={1: [WorkspaceUserRoleType.LEAD]} + ) + + +def member(): + # A non-lead user (default factory grants no lead/validator role). + return factories.make_user_info() + + +def ws_ok(task_session): + """Queue a successful workspace access guard on the task DB.""" task_session.queue(fakes.rows(factories.make_workspace(id=1))) - # ...then team_repo.get_all (OSM DB) returns teams with members. + + +# === GET "" list teams ===================================================== + + +async def test_list_teams_returns_items(client, login, task_session, osm_session): + login(member()) + ws_ok(task_session) osm_session.queue( fakes.rows( factories.make_team(id=1, name="Alpha", users=[factories.make_user()]), @@ -24,7 +50,7 @@ async def test_list_teams(client, login, task_session, osm_session): ) ) - response = await client.get(teams_url()) + response = await client.get(base()) assert response.status_code == 200 body = response.json() @@ -32,47 +58,300 @@ async def test_list_teams(client, login, task_session, osm_session): assert body[1]["member_count"] == 0 -async def test_create_team_requires_lead(client, login, task_session, osm_session): - login() # plain contributor - response = await client.post(teams_url(), json={"name": "New Team"}) - assert response.status_code == 403 +async def test_list_teams_inaccessible_workspace_404(client, login, task_session): + login(member()) + task_session.queue(fakes.empty()) # getById guard -> NotFound + response = await client.get(base(42)) + assert response.status_code == 404 -async def test_create_team_as_lead(client, login, task_session, osm_session): - from api.src.users.schemas import WorkspaceUserRoleType +async def test_list_teams_unexpected_error_500( + error_client, login, task_session, osm_session +): + login(member()) + ws_ok(task_session) + osm_session.queue(fakes.raises(RuntimeError("boom"))) + response = await error_client.get(base()) + assert response.status_code == 500 - login( - factories.make_user_info(osm_workspace_roles={1: [WorkspaceUserRoleType.LEAD]}) - ) - # lead check -> getById (task) guard -> team_repo.create (OSM): add+commit+refresh - task_session.queue(fakes.rows(factories.make_workspace(id=1))) - response = await client.post(teams_url(), json={"name": "New Team"}) +# === POST "" create team =================================================== + +async def test_create_team_requires_lead(client, login): + login(factories.make_user_info()) + response = await client.post(base(), json={"name": "New"}) + assert response.status_code == 403 + + +async def test_create_team_as_lead(client, login, lead, task_session, osm_session): + login(lead) + ws_ok(task_session) # getById guard + # create -> add + commit + refresh assigns the id + response = await client.post(base(), json={"name": "New Team"}) assert response.status_code == 201 assert isinstance(response.json(), int) assert osm_session.commits == 1 -async def test_get_team_not_in_workspace_returns_404( +async def test_create_team_workspace_missing_404(client, login, lead, task_session): + login(lead) + task_session.queue(fakes.empty()) + response = await client.post(base(), json={"name": "New"}) + assert response.status_code == 404 + + +async def test_create_team_rejects_blank_name(client, login, lead): + login(lead) + response = await client.post(base(), json={"name": ""}) + assert response.status_code == 422 + + +# === GET "/{team_id}" get one team ========================================= + + +async def test_get_team_returns_item(client, login, task_session, osm_session): + login(member()) + ws_ok(task_session) + osm_session.queue( + fakes.scalar(True), # assert_team_in_workspace + fakes.rows(factories.make_team(id=3, name="Gamma", users=[])), # get_item + ) + + response = await client.get(f"{base()}/3") + + assert response.status_code == 200 + assert response.json()["id"] == 3 + + +async def test_get_team_not_in_workspace_404(client, login, task_session, osm_session): + login(member()) + ws_ok(task_session) + osm_session.queue(fakes.scalar(False)) # assert_team_in_workspace -> False + response = await client.get(f"{base()}/999") + assert response.status_code == 404 + + +async def test_get_team_workspace_missing_404(client, login, task_session): + login(member()) + task_session.queue(fakes.empty()) + response = await client.get(f"{base()}/3") + assert response.status_code == 404 + + +# === PUT "/{team_id}" update team ========================================== + + +async def test_update_team_requires_lead(client, login): + login(factories.make_user_info()) + response = await client.put(f"{base()}/1", json={"name": "Renamed"}) + assert response.status_code == 403 + + +async def test_update_team_as_lead(client, login, lead, task_session, osm_session): + login(lead) + ws_ok(task_session) + osm_session.queue( + fakes.scalar(True), # assert_team_in_workspace + fakes.rows(factories.make_team(id=1, name="Old")), # update -> get(id) + ) + + response = await client.put(f"{base()}/1", json={"name": "Renamed"}) + + assert response.status_code == 204 + assert osm_session.commits == 1 + + +async def test_update_team_not_in_workspace_404( + client, login, lead, task_session, osm_session +): + login(lead) + ws_ok(task_session) + osm_session.queue(fakes.scalar(False)) + response = await client.put(f"{base()}/1", json={"name": "Renamed"}) + assert response.status_code == 404 + + +# === DELETE "/{team_id}" delete team ======================================= + + +async def test_delete_team_requires_lead(client, login): + login(factories.make_user_info()) + response = await client.delete(f"{base()}/1") + assert response.status_code == 403 + + +async def test_delete_team_as_lead(client, login, lead, task_session, osm_session): + login(lead) + ws_ok(task_session) + osm_session.queue(fakes.scalar(True)) # assert_team_in_workspace; delete follows + response = await client.delete(f"{base()}/1") + assert response.status_code == 204 + assert osm_session.commits == 1 + + +async def test_delete_team_not_in_workspace_404( + client, login, lead, task_session, osm_session +): + login(lead) + ws_ok(task_session) + osm_session.queue(fakes.scalar(False)) + response = await client.delete(f"{base()}/1") + assert response.status_code == 404 + + +# === GET "/{team_id}/members" list members ================================= + + +async def test_get_members_returns_users(client, login, task_session, osm_session): + login(member()) + ws_ok(task_session) + osm_session.queue( + fakes.scalar(True), # assert_team_in_workspace + fakes.rows(factories.make_user(id=1, display_name="Mem")), # get_members + ) + + response = await client.get(f"{base()}/1/members") + + assert response.status_code == 200 + assert response.json()[0]["display_name"] == "Mem" + + +async def test_get_members_team_not_in_workspace_404( client, login, task_session, osm_session ): - login() - task_session.queue(fakes.rows(factories.make_workspace(id=1))) - # assert_team_in_workspace -> session.scalar(EXISTS) returns False + login(member()) + ws_ok(task_session) osm_session.queue(fakes.scalar(False)) + response = await client.get(f"{base()}/1/members") + assert response.status_code == 404 + + +# === POST "/{team_id}/members" join team =================================== + + +async def test_join_team_adds_current_user(client, login, task_session, osm_session): + login(member()) + joining = factories.make_user(id=7, display_name="Joiner") + joining_again = factories.make_user(id=7, display_name="Joiner") + joining_again.teams = [] # not yet on the team + ws_ok(task_session) + osm_session.queue( + fakes.scalar(True), # assert_team_in_workspace + fakes.rows(joining), # get_current_user (scalar_one) + fakes.rows(joining_again), # add_member: load user w/ teams + fakes.rows(factories.make_team(id=1, name="T")), # add_member: get(team) + ) + + response = await client.post(f"{base()}/1/members") + + assert response.status_code == 200 + assert response.json()["id"] == 7 + assert osm_session.commits == 1 + + +async def test_join_team_not_in_workspace_404(client, login, task_session, osm_session): + login(member()) + ws_ok(task_session) + osm_session.queue(fakes.scalar(False)) + response = await client.post(f"{base()}/1/members") + assert response.status_code == 404 + + +# === PUT "/{team_id}/members/{user_id}" add member ========================= + + +async def test_add_member_requires_lead(client, login): + login(factories.make_user_info()) + response = await client.put(f"{base()}/1/members/5") + assert response.status_code == 403 + + +async def test_add_member_as_lead(client, login, lead, task_session, osm_session): + login(lead) + target = factories.make_user(id=5) + target.teams = [] + ws_ok(task_session) + osm_session.queue( + fakes.scalar(True), # assert_team_in_workspace + fakes.rows(target), # add_member: load user + fakes.rows(factories.make_team(id=1, name="T")), # add_member: get(team) + ) - response = await client.get(f"{teams_url()}/999") + response = await client.put(f"{base()}/1/members/5") + assert response.status_code == 204 + assert osm_session.commits == 1 + + +async def test_add_member_missing_user_404( + client, login, lead, task_session, osm_session +): + login(lead) + ws_ok(task_session) + osm_session.queue( + fakes.scalar(True), # assert_team_in_workspace + fakes.empty(), # add_member: user not found + ) + response = await client.put(f"{base()}/1/members/5") assert response.status_code == 404 -async def test_list_teams_for_inaccessible_workspace_is_404( - client, login, task_session +async def test_add_member_team_not_in_workspace_404( + client, login, lead, task_session, osm_session ): - login() - task_session.queue(fakes.empty()) # getById guard finds no workspace + login(lead) + ws_ok(task_session) + osm_session.queue(fakes.scalar(False)) + response = await client.put(f"{base()}/1/members/5") + assert response.status_code == 404 - response = await client.get(teams_url(42)) +# === DELETE "/{team_id}/members/{user_id}" remove member =================== + + +async def test_remove_member_requires_lead(client, login): + login(factories.make_user_info()) + response = await client.delete(f"{base()}/1/members/5") + assert response.status_code == 403 + + +async def test_remove_member_as_lead(client, login, lead, task_session, osm_session): + login(lead) + team = factories.make_team(id=1, name="T") + target = factories.make_user(id=5) + target.teams = [team] # currently a member + ws_ok(task_session) + osm_session.queue( + fakes.scalar(True), # assert_team_in_workspace + fakes.rows(target), # remove_member: load user + fakes.rows(team), # remove_member: get(team) + ) + + response = await client.delete(f"{base()}/1/members/5") + + assert response.status_code == 204 + assert osm_session.commits == 1 + + +async def test_remove_member_missing_user_404( + client, login, lead, task_session, osm_session +): + login(lead) + ws_ok(task_session) + osm_session.queue( + fakes.scalar(True), # assert_team_in_workspace + fakes.empty(), # remove_member: user not found + ) + response = await client.delete(f"{base()}/1/members/5") + assert response.status_code == 404 + + +async def test_remove_member_team_not_in_workspace_404( + client, login, lead, task_session, osm_session +): + login(lead) + ws_ok(task_session) + osm_session.queue(fakes.scalar(False)) + response = await client.delete(f"{base()}/1/members/5") assert response.status_code == 404 diff --git a/tests/integration/test_users.py b/tests/integration/test_users.py new file mode 100644 index 0000000..ed083cb --- /dev/null +++ b/tests/integration/test_users.py @@ -0,0 +1,186 @@ +"""Integration tests for the /workspaces/{id}/users routes. + +Covers the @test comments in api/src/users/routes.py: +- listing members requires contributor-or-above (403 otherwise) +- assign/remove role require workspace lead (403 otherwise) +- workspace-missing -> 404, user-missing -> 404 +- unexpected errors -> 500 +- role can be set to lead/validator and unset back to contributor +- POC / contributor cannot be assigned directly (422) +- the affected user's cache is evicted after a change +""" + +from uuid import UUID + +import pytest + +import api.src.users.routes as users_routes +from api.src.users.schemas import WorkspaceUserRoleType +from tests.support import factories, fakes + +TARGET_USER = "33333333-3333-3333-3333-333333333333" + + +def url(workspace_id=1): + return f"/api/v1/workspaces/{workspace_id}/users" + + +@pytest.fixture +def lead(): + return factories.make_user_info( + osm_workspace_roles={1: [WorkspaceUserRoleType.LEAD]} + ) + + +@pytest.fixture +def evictions(monkeypatch): + """Record evict_user_from_cache(...) calls made by the routes.""" + calls = [] + monkeypatch.setattr(users_routes, "evict_user_from_cache", calls.append) + return calls + + +# --- GET members ----------------------------------------------------------- + + +async def test_list_members_requires_membership(client, login): + login(factories.make_user_info(accessible_workspace_ids={})) + response = await client.get(url()) + assert response.status_code == 403 + + +async def test_list_members_returns_privileged_users(client, login, osm_session): + login(factories.make_user_info(accessible_workspace_ids={"pg": [1]})) + user = factories.make_user(id=5, auth_uid=TARGET_USER, display_name="Lead Lou") + # repo joins User + role -> rows of (user, role) tuples + osm_session.queue(fakes.rows((user, "lead"))) + + response = await client.get(url()) + + assert response.status_code == 200 + body = response.json() + assert body == [ + {"id": 5, "auth_uid": TARGET_USER, "display_name": "Lead Lou", "role": "lead"} + ] + + +async def test_list_members_unexpected_error_returns_500( + error_client, login, osm_session +): + login(factories.make_user_info(accessible_workspace_ids={"pg": [1]})) + osm_session.queue(fakes.raises(RuntimeError("db exploded"))) + + response = await error_client.get(url()) + assert response.status_code == 500 + + +# --- PUT assign role ------------------------------------------------------- + + +async def test_assign_role_requires_lead(client, login): + login(factories.make_user_info(accessible_workspace_ids={"pg": [1]})) + response = await client.put(f"{url()}/{TARGET_USER}/role", json={"role": "lead"}) + assert response.status_code == 403 + + +async def test_assign_role_workspace_missing_returns_404( + client, login, lead, task_session +): + login(lead) + task_session.queue(fakes.empty()) # getById finds nothing + response = await client.put(f"{url()}/{TARGET_USER}/role", json={"role": "lead"}) + assert response.status_code == 404 + + +async def test_assign_role_user_missing_returns_404( + client, login, lead, task_session, osm_session +): + login(lead) + task_session.queue(fakes.rows(factories.make_workspace(id=1))) + osm_session.queue(fakes.scalar(None)) # user has never signed in + response = await client.put(f"{url()}/{TARGET_USER}/role", json={"role": "lead"}) + assert response.status_code == 404 + + +async def test_assign_lead_role_succeeds_and_evicts( + client, login, lead, task_session, osm_session, evictions +): + login(lead) + task_session.queue(fakes.rows(factories.make_workspace(id=1))) + osm_session.queue(fakes.scalar(1)) # user exists + + response = await client.put(f"{url()}/{TARGET_USER}/role", json={"role": "lead"}) + + assert response.status_code == 204 + assert osm_session.commits == 1 + assert evictions == [UUID(TARGET_USER)] + + +async def test_assign_validator_role_succeeds( + client, login, lead, task_session, osm_session, evictions +): + login(lead) + task_session.queue(fakes.rows(factories.make_workspace(id=1))) + osm_session.queue(fakes.scalar(1)) + + response = await client.put( + f"{url()}/{TARGET_USER}/role", json={"role": "validator"} + ) + assert response.status_code == 204 + + +async def test_assign_contributor_role_rejected(client, login, lead): + login(lead) + # CONTRIBUTOR is implicit and cannot be assigned directly -> 422. + response = await client.put( + f"{url()}/{TARGET_USER}/role", json={"role": "contributor"} + ) + assert response.status_code == 422 + + +async def test_assign_poc_role_rejected(client, login, lead): + login(lead) + # "poc" is a TDEI role, not a workspace role -> validation error. + response = await client.put(f"{url()}/{TARGET_USER}/role", json={"role": "poc"}) + assert response.status_code == 422 + + +# --- DELETE remove role ---------------------------------------------------- + + +async def test_remove_role_requires_lead(client, login): + login(factories.make_user_info(accessible_workspace_ids={"pg": [1]})) + response = await client.delete(f"{url()}/{TARGET_USER}") + assert response.status_code == 403 + + +async def test_remove_role_unsets_to_contributor_and_evicts( + client, login, lead, task_session, osm_session, evictions +): + login(lead) + task_session.queue(fakes.rows(factories.make_workspace(id=1))) + osm_session.queue(fakes.affected(1)) # one role row removed + + response = await client.delete(f"{url()}/{TARGET_USER}") + + assert response.status_code == 204 + assert evictions == [UUID(TARGET_USER)] + + +async def test_remove_role_when_not_assigned_returns_404( + client, login, lead, task_session, osm_session +): + login(lead) + task_session.queue(fakes.rows(factories.make_workspace(id=1))) + osm_session.queue(fakes.affected(0)) # nothing to delete + response = await client.delete(f"{url()}/{TARGET_USER}") + assert response.status_code == 404 + + +async def test_remove_role_workspace_missing_returns_404( + client, login, lead, task_session +): + login(lead) + task_session.queue(fakes.empty()) + response = await client.delete(f"{url()}/{TARGET_USER}") + assert response.status_code == 404 diff --git a/tests/integration/test_workspaces.py b/tests/integration/test_workspaces.py index 663e12a..fbd30cb 100644 --- a/tests/integration/test_workspaces.py +++ b/tests/integration/test_workspaces.py @@ -1,23 +1,63 @@ """Integration tests for the /workspaces routes. -Each test drives a real HTTP request through the real route + repository -code, queueing simulated rows on the fake DB sessions. Comments note the -query sequence each route issues (= the order to queue results). +Covers the @test comments in api/src/workspaces/routes.py across all +endpoints (list, get, bbox, create, update, delete, quest get/settings, +imagery get/settings). Each test drives a real HTTP request through the +real route + repository code, queueing simulated rows on the fake sessions. + +Note: read endpoints (get, bbox, quest, imagery) gate access via +``getById``, which raises 404 when the workspace is missing or inaccessible +-- so "no access" surfaces as 404 there rather than 403. The mutating +endpoints additionally require ``isWorkspaceLead`` and return 403 otherwise. """ +from datetime import datetime +from uuid import UUID + +import pytest + +import api.src.workspaces.routes as ws_routes from api.src.users.schemas import WorkspaceUserRoleType +from api.src.workspaces.schemas import QuestDefinitionType, WorkspaceLongQuest from tests.support import factories, fakes API = "/api/v1/workspaces" -async def test_get_my_workspaces(client, login, task_session): +@pytest.fixture +def lead(): + return factories.make_user_info( + osm_workspace_roles={1: [WorkspaceUserRoleType.LEAD]} + ) + + +@pytest.fixture +def no_schema_validation(monkeypatch): + """Make schema validation a no-op (avoid network in update endpoints).""" + + async def ok(_definition): + return None + + monkeypatch.setattr(ws_routes, "validate_quest_definition_schema", ok) + monkeypatch.setattr(ws_routes, "validate_imagery_definition_schema", ok) + + +@pytest.fixture +def evictions(monkeypatch): + calls = [] + monkeypatch.setattr(ws_routes, "evict_user_from_cache", calls.append) + return calls + + +# === GET /mine ============================================================= + + +async def test_list_my_workspaces(client, login, task_session): login() - # getAll -> one SELECT on the task DB task_session.queue( fakes.rows( - factories.make_workspace(id=1, title="WS One"), - factories.make_workspace(id=2, title="WS Two"), + factories.make_workspace(id=1, title="One"), + factories.make_workspace(id=2, title="Two"), ) ) @@ -26,105 +66,429 @@ async def test_get_my_workspaces(client, login, task_session): assert response.status_code == 200 body = response.json() assert [w["id"] for w in body] == [1, 2] - assert body[0]["title"] == "WS One" - assert body[0]["role"] == "contributor" # WorkspaceResponse includes role + assert body[0]["role"] == "contributor" + + +async def test_list_my_workspaces_empty(client, login, task_session): + login() + task_session.queue(fakes.rows()) + response = await client.get(f"{API}/mine") + assert response.status_code == 200 + assert response.json() == [] + + +async def test_list_my_workspaces_unexpected_error_500( + error_client, login, task_session +): + login() + task_session.queue(fakes.raises(RuntimeError("db"))) + response = await error_client.get(f"{API}/mine") + assert response.status_code == 500 + + +async def test_list_matches_get_by_id(client, login, task_session): + # The same workspace serialized via /mine and via /{id} agree on shared fields. + login( + factories.make_user_info(osm_workspace_roles={1: [WorkspaceUserRoleType.LEAD]}) + ) + task_session.queue( + fakes.rows(factories.make_workspace(id=1, title="Shared")), + fakes.rows(factories.make_workspace(id=1, title="Shared")), + ) + + listed = (await client.get(f"{API}/mine")).json()[0] + fetched = (await client.get(f"{API}/1")).json() + + shared = ["id", "title", "type", "tdeiProjectGroupId", "role"] + assert {k: listed[k] for k in shared} == {k: fetched[k] for k in shared} + + +# === GET /{id} ============================================================= async def test_get_workspace_by_id(client, login, task_session): login() - # getById -> one SELECT task_session.queue(fakes.rows(factories.make_workspace(id=7, title="Lucky"))) - response = await client.get(f"{API}/7") - assert response.status_code == 200 assert response.json()["title"] == "Lucky" -async def test_get_workspace_not_found(client, login, task_session): +async def test_get_workspace_missing_404(client, login, task_session): login() - task_session.queue(fakes.empty()) # getById finds nothing - + task_session.queue(fakes.empty()) response = await client.get(f"{API}/404") + assert response.status_code == 404 + +async def test_get_workspace_non_integer_id_422(client, login): + login() + response = await client.get(f"{API}/not-a-number") + assert response.status_code == 422 + + +async def test_get_workspace_unexpected_error_500(error_client, login, task_session): + login() + task_session.queue(fakes.raises(RuntimeError("db"))) + response = await error_client.get(f"{API}/7") + assert response.status_code == 500 + + +# === GET /{id}/bbox ======================================================== + + +async def test_get_bbox_returns_values(client, login, task_session, osm_session): + login() + task_session.queue(fakes.rows(factories.make_workspace(id=1))) + osm_session.queue( + fakes.mappings({"max_lat": 1.5, "max_lon": 2.5, "min_lat": 0.5, "min_lon": 1.5}) + ) + + response = await client.get(f"{API}/1/bbox") + + assert response.status_code == 200 + assert response.json()["max_lat"] == 1.5 + + +async def test_get_bbox_workspace_missing_404(client, login, task_session): + login() + task_session.queue(fakes.empty()) + response = await client.get(f"{API}/1/bbox") + assert response.status_code == 404 + + +async def test_get_bbox_no_nodes_404(client, login, task_session, osm_session): + login() + task_session.queue(fakes.rows(factories.make_workspace(id=1))) + osm_session.queue(fakes.mappings()) # no rows -> bbox None -> NotFound + response = await client.get(f"{API}/1/bbox") assert response.status_code == 404 -async def test_create_workspace(client, login, task_session, osm_session): +# === POST "" create ======================================================== + + +async def test_create_workspace(client, login, task_session, osm_session, evictions): login(factories.make_user_info(project_group_ids=[factories.DEFAULT_PG_ID])) - # create (task DB) -> add + commit + refresh; then assign_member_role - # (OSM DB) checks the user exists via session.scalar(...). - osm_session.queue(fakes.scalar(1)) + osm_session.queue(fakes.scalar(1)) # assign_member_role: user exists response = await client.post( f"{API}", json={ "type": "osw", - "title": "Fresh Workspace", + "title": "Fresh", "tdeiProjectGroupId": factories.DEFAULT_PG_ID, }, ) assert response.status_code == 201 assert response.json()["workspaceId"] is not None - assert task_session.commits == 1 - assert osm_session.commits == 1 + assert task_session.commits == 1 # workspace insert + assert osm_session.commits == 1 # role insert + assert evictions == [UUID(factories.DEFAULT_USER_ID)] # creator's cache evicted + + +async def test_create_in_unauthorized_group_403(client, login): + # User belongs to DEFAULT_PG_ID only; creating in another group is forbidden. + login(factories.make_user_info(project_group_ids=[factories.DEFAULT_PG_ID])) + response = await client.post( + f"{API}", + json={ + "type": "osw", + "title": "Nope", + "tdeiProjectGroupId": "99999999-9999-9999-9999-999999999999", + }, + ) + assert response.status_code == 403 + + +async def test_create_invalid_body_422(client, login): + login() + # Missing required 'title' and 'tdeiProjectGroupId'. + response = await client.post(f"{API}", json={"type": "osw"}) + assert response.status_code == 422 + + +# === PATCH /{id} update ==================================================== -async def test_update_workspace_requires_lead(client, login, task_session): - # Plain contributor, not a lead -> 403 before any DB write. +async def test_update_requires_lead(client, login): login(factories.make_user_info()) + response = await client.patch(f"{API}/1", json={"title": "X"}) + assert response.status_code == 403 + + +async def test_update_empty_patch_400(client, login, lead): + login(lead) + response = await client.patch(f"{API}/1", json={}) + assert response.status_code == 400 + +async def test_update_success(client, login, lead, task_session): + login(lead) + task_session.queue( + fakes.affected(1), # UPDATE + fakes.rows(factories.make_workspace(id=1, title="Renamed")), # getById + ) response = await client.patch(f"{API}/1", json={"title": "Renamed"}) + assert response.status_code == 204 + + +async def test_update_missing_workspace_404(client, login, lead, task_session): + login(lead) + task_session.queue(fakes.affected(0)) # UPDATE matched nothing -> NotFound + response = await client.patch(f"{API}/1", json={"title": "X"}) + assert response.status_code == 404 + +async def test_update_invalid_body_422(client, login, lead): + login(lead) + response = await client.patch(f"{API}/1", json={"externalAppAccess": 999}) + assert response.status_code == 422 + + +# === DELETE /{id} ========================================================== + + +async def test_delete_requires_lead(client, login): + login(factories.make_user_info()) + response = await client.delete(f"{API}/1") assert response.status_code == 403 - assert task_session.commits == 0 -async def test_update_workspace_as_lead(client, login, task_session): - login( - factories.make_user_info(osm_workspace_roles={1: [WorkspaceUserRoleType.LEAD]}) +async def test_delete_success_no_members( + client, login, lead, task_session, osm_session, evictions +): + login(lead) + osm_session.queue(fakes.rows()) # get_privileged_workspace_members: none + task_session.queue(fakes.affected(1)) # delete + + response = await client.delete(f"{API}/1") + + assert response.status_code == 204 + assert task_session.commits == 1 + assert evictions == [] # no members to evict + + +async def test_delete_evicts_member_caches( + client, login, lead, task_session, osm_session, evictions +): + login(lead) + member = factories.make_user(id=9, auth_uid=factories.DEFAULT_USER_ID) + osm_session.queue(fakes.rows((member, "lead"))) # privileged members + task_session.queue(fakes.affected(1)) + + response = await client.delete(f"{API}/1") + + assert response.status_code == 204 + assert evictions == [UUID(factories.DEFAULT_USER_ID)] + + +async def test_delete_missing_workspace_404( + client, login, lead, task_session, osm_session +): + login(lead) + osm_session.queue(fakes.rows()) # no members + task_session.queue(fakes.affected(0)) # delete matched nothing -> NotFound + response = await client.delete(f"{API}/1") + assert response.status_code == 404 + + +# === GET /{id}/quests/long ================================================= + + +async def test_get_long_quest_def_none_returns_204(client, login, task_session): + login() + task_session.queue(fakes.rows(factories.make_workspace(id=1))) # no quest def + response = await client.get(f"{API}/1/quests/long") + assert response.status_code == 204 + + +async def test_get_long_quest_def_json(client, login, task_session): + login() + ws = factories.make_workspace(id=1) + ws.longFormQuestDef = WorkspaceLongQuest( + workspace_id=1, + type=QuestDefinitionType.JSON, + definition='{"quest": true}', + modifiedAt=datetime(2026, 1, 1), + modifiedBy=UUID(factories.DEFAULT_USER_ID), + modifiedByName="x", ) - # update -> UPDATE (rowcount) ... commit ... then getById -> SELECT - task_session.queue( - fakes.affected(1), - fakes.rows(factories.make_workspace(id=1, title="Renamed")), + task_session.queue(fakes.rows(ws)) + + response = await client.get(f"{API}/1/quests/long") + + assert response.status_code == 200 + assert response.json() == {"quest": True} + + +async def test_get_long_quest_def_workspace_missing_404(client, login, task_session): + login() + task_session.queue(fakes.empty()) + response = await client.get(f"{API}/1/quests/long") + assert response.status_code == 404 + + +# === GET /{id}/quests/long/settings ======================================== + + +async def test_get_long_quest_settings_default_when_unset(client, login, task_session): + login() + task_session.queue(fakes.rows(factories.make_workspace(id=1))) + response = await client.get(f"{API}/1/quests/long/settings") + assert response.status_code == 200 + body = response.json() + assert body["type"] == "NONE" + assert body["workspace_id"] == 1 + + +async def test_get_long_quest_settings_from_def(client, login, task_session): + login() + ws = factories.make_workspace(id=1) + ws.longFormQuestDef = WorkspaceLongQuest( + workspace_id=1, + type=QuestDefinitionType.URL, + url="https://q.example", + modifiedAt=datetime(2026, 1, 1), + modifiedBy=UUID(factories.DEFAULT_USER_ID), + modifiedByName="Editor", ) + task_session.queue(fakes.rows(ws)) - response = await client.patch(f"{API}/1", json={"title": "Renamed"}) + response = await client.get(f"{API}/1/quests/long/settings") + + assert response.status_code == 200 + body = response.json() + assert body["type"] == "URL" + assert body["url"] == "https://q.example" + + +async def test_get_long_quest_settings_missing_404(client, login, task_session): + login() + task_session.queue(fakes.empty()) + response = await client.get(f"{API}/1/quests/long/settings") + assert response.status_code == 404 + + +# === PATCH /{id}/quests/long/settings ====================================== + +async def test_update_quest_settings_requires_lead(client, login): + login(factories.make_user_info()) + response = await client.patch( + f"{API}/1/quests/long/settings", json={"type": "NONE"} + ) + assert response.status_code == 403 + + +async def test_update_quest_settings_none_success(client, login, lead, task_session): + login(lead) + task_session.queue(fakes.affected(1)) # save_longform_quest UPDATE + response = await client.patch( + f"{API}/1/quests/long/settings", json={"type": "NONE"} + ) assert response.status_code == 204 - assert task_session.commits == 1 -async def test_update_workspace_rejects_empty_patch(client, login): - login( - factories.make_user_info(osm_workspace_roles={1: [WorkspaceUserRoleType.LEAD]}) +async def test_update_quest_settings_json_validates_schema( + client, login, lead, task_session, no_schema_validation +): + login(lead) + task_session.queue(fakes.affected(1)) + response = await client.patch( + f"{API}/1/quests/long/settings", + json={"type": "JSON", "definition": '{"a": 1}'}, ) + assert response.status_code == 204 - response = await client.patch(f"{API}/1", json={}) +async def test_update_quest_settings_invalid_schema_400( + client, login, lead, monkeypatch +): + from fastapi import HTTPException + + async def reject(_definition): + raise HTTPException(status_code=400, detail="bad schema") + + monkeypatch.setattr(ws_routes, "validate_quest_definition_schema", reject) + login(lead) + + response = await client.patch( + f"{API}/1/quests/long/settings", + json={"type": "JSON", "definition": '{"a": 1}'}, + ) assert response.status_code == 400 -async def test_delete_workspace_as_lead(client, login, task_session, osm_session): - login( - factories.make_user_info(osm_workspace_roles={1: [WorkspaceUserRoleType.LEAD]}) +async def test_update_quest_settings_json_without_definition_422(client, login, lead): + login(lead) + # QuestSettingsPatch validator rejects JSON type with no definition. + response = await client.patch( + f"{API}/1/quests/long/settings", json={"type": "JSON"} ) - # delete flow: get_privileged_workspace_members (OSM) -> delete (task) -> - # remove_all_member_roles (OSM) - osm_session.queue(fakes.rows()) # no privileged members - task_session.queue(fakes.affected(1)) + assert response.status_code == 422 - response = await client.delete(f"{API}/1") - assert response.status_code == 204 - assert task_session.commits == 1 +# === GET /{id}/imagery/settings ============================================ -async def test_delete_workspace_forbidden_for_non_lead(client, login): - login(factories.make_user_info()) +async def test_get_imagery_settings_default_when_unset(client, login, task_session): + login() + task_session.queue(fakes.rows(factories.make_workspace(id=1))) + response = await client.get(f"{API}/1/imagery/settings") + assert response.status_code == 200 + assert response.json()["definition"] == [] + + +async def test_get_imagery_settings_missing_404(client, login, task_session): + login() + task_session.queue(fakes.empty()) + response = await client.get(f"{API}/1/imagery/settings") + assert response.status_code == 404 - response = await client.delete(f"{API}/1") +# === PATCH /{id}/imagery/settings ========================================== + + +async def test_update_imagery_requires_lead(client, login): + login(factories.make_user_info()) + response = await client.patch(f"{API}/1/imagery/settings", json={"definition": []}) assert response.status_code == 403 + + +async def test_update_imagery_success( + client, login, lead, task_session, no_schema_validation +): + login(lead) + task_session.queue(fakes.affected(1)) # save_imagery_def UPDATE + response = await client.patch( + f"{API}/1/imagery/settings", + json={"definition": [{"name": "layer", "url": "https://x"}]}, + ) + assert response.status_code == 204 + + +async def test_update_imagery_invalid_schema_400(client, login, lead, monkeypatch): + from fastapi import HTTPException + + async def reject(_definition): + raise HTTPException(status_code=400, detail="bad imagery") + + monkeypatch.setattr(ws_routes, "validate_imagery_definition_schema", reject) + login(lead) + + response = await client.patch( + f"{API}/1/imagery/settings", json={"definition": [{"bad": True}]} + ) + assert response.status_code == 400 + + +async def test_update_imagery_empty_skips_validation_success( + client, login, lead, task_session +): + # Empty definition is falsy -> validation skipped, save proceeds. + login(lead) + task_session.queue(fakes.affected(1)) + response = await client.patch(f"{API}/1/imagery/settings", json={"definition": []}) + assert response.status_code == 204 diff --git a/tests/support/fakes.py b/tests/support/fakes.py index a989fd1..61fcdd0 100644 --- a/tests/support/fakes.py +++ b/tests/support/fakes.py @@ -124,16 +124,23 @@ def _is_session_setup(statement): except Exception: return False + @staticmethod + def _raise_if_exc(item): + # A queued exception simulates a DB-layer failure (drives 500 paths). + if isinstance(item, BaseException): + raise item + return item + async def execute(self, statement, *args, **kwargs): if self._is_session_setup(statement): return FakeResult(rows=[]) - return self._next() + return self._raise_if_exc(self._next()) async def exec(self, statement, *args, **kwargs): - return self._next() + return self._raise_if_exc(self._next()) async def scalar(self, statement, *args, **kwargs): - result = self._next() + result = self._raise_if_exc(self._next()) return result.value if isinstance(result, FakeResult) else result def add(self, obj): @@ -183,3 +190,8 @@ def mappings(*dict_rows): def scalar(value): """A result for ``session.scalar(...)`` (e.g. an EXISTS check).""" return FakeResult(value=value) + + +def raises(exc): + """Queue an exception so the next DB call raises it (drives 500 paths).""" + return exc diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py new file mode 100644 index 0000000..3a06d3b --- /dev/null +++ b/tests/unit/test_config.py @@ -0,0 +1,59 @@ +"""Tests for api/core/config.py Settings. + +Covers the @test comments on the Settings class: +- env vars of the same name override the members +- unset env vars fall back to declared defaults +- strings / numbers / empty strings / URLs all load as the defaults exemplify +""" + +from api.core.config import Settings + + +def test_defaults_loaded_when_env_unset(): + # _env_file=None ignores any local .env so we observe the declared defaults. + s = Settings(_env_file=None) # type: ignore[call-arg] # pydantic-settings init kwarg + + assert s.PROJECT_NAME == "Workspaces API" + assert s.CORS_ORIGINS == [] + assert s.DEBUG is False + assert s.SENTRY_DSN == "" + assert s.WS_OSM_HOST == "http://osm-web" + assert s.TDEI_OIDC_REALM == "tdei" + assert s.TASK_DATABASE_URL.startswith("postgresql+asyncpg://") + assert s.OSM_DATABASE_URL.startswith("postgresql+asyncpg://") + + +def test_env_vars_override_members(monkeypatch): + monkeypatch.setenv("PROJECT_NAME", "Custom Name") + monkeypatch.setenv("DEBUG", "true") + monkeypatch.setenv("WS_OSM_HOST", "http://osm.example") + monkeypatch.setenv("SENTRY_DSN", "https://sentry.example/123") + + s = Settings(_env_file=None) # type: ignore[call-arg] # pydantic-settings init kwarg + + assert s.PROJECT_NAME == "Custom Name" + assert s.DEBUG is True + assert s.WS_OSM_HOST == "http://osm.example" + assert s.SENTRY_DSN == "https://sentry.example/123" + + +def test_cors_origins_parsed_from_json_env(monkeypatch): + monkeypatch.setenv("CORS_ORIGINS", '["https://a.example", "https://b.example"]') + + s = Settings(_env_file=None) # type: ignore[call-arg] # pydantic-settings init kwarg + + assert s.CORS_ORIGINS == ["https://a.example", "https://b.example"] + + +def test_value_types_and_formats(): + s = Settings(_env_file=None) # type: ignore[call-arg] # pydantic-settings init kwarg + + assert isinstance(s.PROJECT_NAME, str) + assert isinstance(s.CORS_ORIGINS, list) + assert isinstance(s.DEBUG, bool) + # empty-string default is preserved (not coerced to None): + assert s.SENTRY_DSN == "" + # URL-shaped defaults load verbatim: + assert s.TDEI_BACKEND_URL.startswith("https://") + assert s.LONGFORM_SCHEMA_URL.startswith("https://") + assert s.IMAGERY_SCHEMA_URL.startswith("https://") diff --git a/tests/unit/test_json_schema.py b/tests/unit/test_json_schema.py new file mode 100644 index 0000000..3b83f87 --- /dev/null +++ b/tests/unit/test_json_schema.py @@ -0,0 +1,137 @@ +"""Tests for api/core/json_schema.py. + +Covers the @test comments: +- payloads are validated against the fetched JSON schema +- malformed / non-object payloads return a 400 (not a generic Exception) +- network failures map to a proper 502/504 and never produce a false-positive + (i.e. a fetch failure must raise, never silently "pass" validation) +""" + +from types import SimpleNamespace + +import httpx +import pytest +from fastapi import HTTPException + +import api.core.json_schema as js + +# --- validate_quest_definition_schema ------------------------------------- + + +async def test_quest_valid_definition_passes(monkeypatch): + async def fake_schema(): + return {"type": "object", "required": ["x"]} + + monkeypatch.setattr(js, "_fetch_longform_schema", fake_schema) + + # Should not raise. + await js.validate_quest_definition_schema('{"x": 1}') + + +async def test_quest_schema_violation_returns_400(monkeypatch): + async def fake_schema(): + return {"type": "object", "required": ["x"]} + + monkeypatch.setattr(js, "_fetch_longform_schema", fake_schema) + + with pytest.raises(HTTPException) as exc: + await js.validate_quest_definition_schema('{"y": 1}') + assert exc.value.status_code == 400 + + +async def test_quest_malformed_json_returns_400(): + with pytest.raises(HTTPException) as exc: + await js.validate_quest_definition_schema("{not valid json") + assert exc.value.status_code == 400 + + +async def test_quest_non_object_returns_400(): + with pytest.raises(HTTPException) as exc: + await js.validate_quest_definition_schema("[1, 2, 3]") + assert exc.value.status_code == 400 + + +async def test_quest_fetch_timeout_maps_to_504(monkeypatch): + async def boom(): + raise httpx.TimeoutException("slow") + + monkeypatch.setattr(js, "_fetch_longform_schema", boom) + + with pytest.raises(HTTPException) as exc: + await js.validate_quest_definition_schema('{"x": 1}') + assert exc.value.status_code == 504 + + +async def test_quest_fetch_connect_error_maps_to_502(monkeypatch): + async def boom(): + raise httpx.ConnectError("down") + + monkeypatch.setattr(js, "_fetch_longform_schema", boom) + + with pytest.raises(HTTPException) as exc: + await js.validate_quest_definition_schema('{"x": 1}') + assert exc.value.status_code == 502 + + +# --- validate_imagery_definition_schema ----------------------------------- + + +async def test_imagery_valid_passes(monkeypatch): + async def fake_schema(): + return {"type": "array", "items": {"type": "string"}} + + monkeypatch.setattr(js, "_fetch_imagery_schema", fake_schema) + + await js.validate_imagery_definition_schema(["a", "b"]) + + +async def test_imagery_violation_returns_400(monkeypatch): + async def fake_schema(): + return {"type": "array", "items": {"type": "string"}} + + monkeypatch.setattr(js, "_fetch_imagery_schema", fake_schema) + + with pytest.raises(HTTPException) as exc: + await js.validate_imagery_definition_schema([1, 2]) + assert exc.value.status_code == 400 + + +async def test_imagery_fetch_error_maps_to_502(monkeypatch): + async def boom(): + raise httpx.ConnectError("down") + + monkeypatch.setattr(js, "_fetch_imagery_schema", boom) + + with pytest.raises(HTTPException) as exc: + await js.validate_imagery_definition_schema([]) + assert exc.value.status_code == 502 + + +# --- client + caching ------------------------------------------------------ + + +def test_require_http_client_raises_503_when_uninitialized(monkeypatch): + monkeypatch.setattr(js, "_http_client", None) + with pytest.raises(HTTPException) as exc: + js._require_http_client() + assert exc.value.status_code == 503 + + +async def test_fetch_longform_schema_caches_result(monkeypatch): + monkeypatch.setattr(js, "_longform_schema", None) + calls = {"n": 0} + + class FakeClient: + async def get(self, url): + calls["n"] += 1 + return SimpleNamespace( + raise_for_status=lambda: None, json=lambda: {"fetched": True} + ) + + monkeypatch.setattr(js, "_http_client", FakeClient()) + + first = await js._fetch_longform_schema() + second = await js._fetch_longform_schema() + + assert first == second == {"fetched": True} + assert calls["n"] == 1 # second call served from cache diff --git a/tests/unit/test_jwt.py b/tests/unit/test_jwt.py new file mode 100644 index 0000000..2db88b3 --- /dev/null +++ b/tests/unit/test_jwt.py @@ -0,0 +1,71 @@ +"""Tests for api/core/jwt.py token validation. + +The @test comments are boilerplate; the module's real job is to validate a +JWT's RS256 signature against the JWKS and decode its payload. We cover: +- a properly-signed token validates and its payload is returned +- a malformed token raises (rather than returning a payload) +- a token signed by the wrong key raises (no false-positive validation) +- JWKS/network failures propagate as a typed error (not a bare Exception) +""" + +from types import SimpleNamespace + +import jwt as pyjwt +import pytest +from cryptography.hazmat.primitives.asymmetric import rsa +from jwt.exceptions import PyJWKClientError, PyJWTError + +import api.core.jwt as jwtmod + + +@pytest.fixture +def rsa_key(): + return rsa.generate_private_key(public_exponent=65537, key_size=2048) + + +def _fake_jwks(monkeypatch, *, public_key=None, exc=None): + class FakeJWKSClient: + def get_signing_key_from_jwt(self, token): + if exc is not None: + raise exc + return SimpleNamespace(key=public_key) + + monkeypatch.setattr(jwtmod, "_get_jwks_client", lambda: FakeJWKSClient()) + + +def test_valid_token_decodes_payload(monkeypatch, rsa_key): + token = pyjwt.encode( + {"sub": "user-abc", "jti": "j1", "preferred_username": "alice"}, + rsa_key, + algorithm="RS256", + ) + _fake_jwks(monkeypatch, public_key=rsa_key.public_key()) + + payload = jwtmod.validate_and_decode_token(token) + + assert payload["sub"] == "user-abc" + assert payload["preferred_username"] == "alice" + + +def test_malformed_token_raises(monkeypatch, rsa_key): + _fake_jwks(monkeypatch, public_key=rsa_key.public_key()) + + with pytest.raises(PyJWTError): + jwtmod.validate_and_decode_token("not-a-real-jwt") + + +def test_token_signed_with_wrong_key_raises(monkeypatch, rsa_key): + other_key = rsa.generate_private_key(public_exponent=65537, key_size=2048) + token = pyjwt.encode({"sub": "user-abc"}, other_key, algorithm="RS256") + # JWKS returns the *wrong* public key -> signature check must fail. + _fake_jwks(monkeypatch, public_key=rsa_key.public_key()) + + with pytest.raises(PyJWTError): + jwtmod.validate_and_decode_token(token) + + +def test_jwks_network_error_propagates_typed(monkeypatch): + _fake_jwks(monkeypatch, exc=PyJWKClientError("could not fetch JWKS")) + + with pytest.raises(PyJWKClientError): + jwtmod.validate_and_decode_token("a.b.c") diff --git a/tests/unit/test_security.py b/tests/unit/test_security.py new file mode 100644 index 0000000..1c6ae61 --- /dev/null +++ b/tests/unit/test_security.py @@ -0,0 +1,334 @@ +"""Tests for api/core/security.py. + +Covers the @test comments on the module / UserInfo / validate_token: +- the permission structure matches CLAUDE.md +- UserInfo methods return correct values for given PG/workspace roles +- attributes are populated correctly from JWT + TDEI + DB data +- network failures are handled gracefully (typed HTTP errors, no false success) +- the user-info cache works and evicts on token rotation / explicit eviction +""" + +from typing import cast +from uuid import UUID + +import httpx +import pytest +from fastapi import HTTPException +from fastapi.security import HTTPAuthorizationCredentials +from sqlmodel.ext.asyncio.session import AsyncSession + +import api.core.security as sec +from api.src.users.schemas import WorkspaceUserRoleType +from tests.support import factories, fakes + +USER_ID = "22222222-2222-2222-2222-222222222222" + + +@pytest.fixture(autouse=True) +def clear_cache(): + sec._user_info_cache.clear() + yield + sec._user_info_cache.clear() + + +def _creds(token="tok"): + return HTTPAuthorizationCredentials(scheme="Bearer", credentials=token) + + +class _FakeResp: + def __init__(self, status_code=200, data=None, raises=False): + self.status_code = status_code + self._data = data + self._raises = raises + + def json(self): + if self._raises: + raise ValueError("bad json") + return self._data + + +class _FakeTdeiClient: + def __init__(self, resp=None, exc=None): + self._resp = resp + self._exc = exc + + async def get(self, *args, **kwargs): + if self._exc is not None: + raise self._exc + return self._resp + + +# --- CLAUDE.md permission structure --------------------------------------- + + +def test_permission_structure_matches_claude_md(): + # POC ("Project Group Admin") -> lead on workspaces the group owns. + poc = factories.make_user_info( + project_group_ids=["pg"], + poc_group_ids=("pg",), + accessible_workspace_ids={"pg": [1]}, + ) + assert poc.isWorkspaceLead(1) is True + + # Lead granted via Workspaces setting (an OSM-DB role). + lead = factories.make_user_info( + osm_workspace_roles={1: [WorkspaceUserRoleType.LEAD]} + ) + assert lead.effective_role(1) == WorkspaceUserRoleType.LEAD + + # Validator granted via Workspaces setting. + validator = factories.make_user_info( + osm_workspace_roles={1: [WorkspaceUserRoleType.VALIDATOR]} + ) + assert validator.effective_role(1) == WorkspaceUserRoleType.VALIDATOR + + # Contributor implied by project-group membership. + contributor = factories.make_user_info(accessible_workspace_ids={"pg": [1]}) + assert contributor.isWorkspaceContributor(1) is True + assert contributor.effective_role(1) == WorkspaceUserRoleType.CONTRIBUTOR + + +def test_effective_role_precedence_lead_over_validator(): + user = factories.make_user_info( + osm_workspace_roles={ + 1: [WorkspaceUserRoleType.VALIDATOR, WorkspaceUserRoleType.LEAD] + } + ) + assert user.effective_role(1) == WorkspaceUserRoleType.LEAD + + +def test_tdei_role_enum_values(): + assert sec.TdeiProjectGroupRole.POINT_OF_CONTACT == "poc" + assert sec.TdeiProjectGroupRole.MEMBER == "member" + + +# --- validate_token: success + attribute population ------------------------ + + +async def _run_validate(monkeypatch, *, payload, tdei, task, osm): + monkeypatch.setattr(sec, "validate_and_decode_token", lambda _t: payload) + monkeypatch.setattr(sec, "_tdei_client", tdei) + return await sec.validate_token( + _creds(), + cast(AsyncSession, osm), + cast(AsyncSession, task), + ) + + +async def test_validate_token_populates_attributes(monkeypatch): + pg_id = "pg-1" + tdei = _FakeTdeiClient( + _FakeResp( + 200, + [ + { + "tdei_project_group_id": pg_id, + "project_group_name": "PG One", + "roles": ["poc"], + } + ], + ) + ) + task = fakes.FakeSession(fakes.mappings({"tdeiProjectGroupId": pg_id, "id": 5})) + osm = fakes.FakeSession(fakes.mappings({"workspace_id": 5, "role": "lead"})) + + info = await _run_validate( + monkeypatch, + payload={"sub": USER_ID, "jti": "j1", "preferred_username": "alice"}, + tdei=tdei, + task=task, + osm=osm, + ) + + assert info.user_uuid == UUID(USER_ID) + assert info.user_name == "alice" + assert info.getProjectGroupIds() == [pg_id] + assert info.accessibleWorkspaceIds == {pg_id: [5]} + assert info.osmWorkspaceRoles == {5: ["lead"]} + assert info.isWorkspaceLead(5) is True + + +# --- validate_token: graceful failure handling ----------------------------- + + +async def test_malformed_token_returns_401(monkeypatch): + def boom(_t): + raise ValueError("bad token") + + monkeypatch.setattr(sec, "validate_and_decode_token", boom) + with pytest.raises(HTTPException) as exc: + await sec.validate_token( + _creds(), + cast(AsyncSession, fakes.FakeSession()), + cast(AsyncSession, fakes.FakeSession()), + ) + assert exc.value.status_code == 401 + + +async def test_missing_sub_returns_401(monkeypatch): + monkeypatch.setattr(sec, "validate_and_decode_token", lambda _t: {"jti": "j"}) + with pytest.raises(HTTPException) as exc: + await sec.validate_token( + _creds(), + cast(AsyncSession, fakes.FakeSession()), + cast(AsyncSession, fakes.FakeSession()), + ) + assert exc.value.status_code == 401 + + +async def test_invalid_uuid_sub_returns_401(monkeypatch): + monkeypatch.setattr( + sec, "validate_and_decode_token", lambda _t: {"sub": "not-a-uuid"} + ) + with pytest.raises(HTTPException) as exc: + await sec.validate_token( + _creds(), + cast(AsyncSession, fakes.FakeSession()), + cast(AsyncSession, fakes.FakeSession()), + ) + assert exc.value.status_code == 401 + + +async def test_tdei_network_error_returns_502(monkeypatch): + tdei = _FakeTdeiClient(exc=httpx.ConnectError("down")) + with pytest.raises(HTTPException) as exc: + await _run_validate( + monkeypatch, + payload={"sub": USER_ID, "jti": "j"}, + tdei=tdei, + task=fakes.FakeSession(), + osm=fakes.FakeSession(), + ) + assert exc.value.status_code == 502 + + +async def test_tdei_non_200_returns_401(monkeypatch): + tdei = _FakeTdeiClient(_FakeResp(403, None)) + with pytest.raises(HTTPException) as exc: + await _run_validate( + monkeypatch, + payload={"sub": USER_ID, "jti": "j"}, + tdei=tdei, + task=fakes.FakeSession(), + osm=fakes.FakeSession(), + ) + assert exc.value.status_code == 401 + + +async def test_tdei_bad_json_returns_401(monkeypatch): + tdei = _FakeTdeiClient(_FakeResp(200, raises=True)) + with pytest.raises(HTTPException) as exc: + await _run_validate( + monkeypatch, + payload={"sub": USER_ID, "jti": "j"}, + tdei=tdei, + task=fakes.FakeSession(), + osm=fakes.FakeSession(), + ) + assert exc.value.status_code == 401 + + +async def test_uninitialized_tdei_client_returns_503(monkeypatch): + with pytest.raises(HTTPException) as exc: + await _run_validate( + monkeypatch, + payload={"sub": USER_ID, "jti": "j"}, + tdei=None, + task=fakes.FakeSession(), + osm=fakes.FakeSession(), + ) + assert exc.value.status_code == 503 + + +# --- caching --------------------------------------------------------------- + + +async def test_cache_hit_skips_refetch(monkeypatch): + pg_id = "pg-1" + + def fresh_tdei(): + return _FakeTdeiClient( + _FakeResp( + 200, + [ + { + "tdei_project_group_id": pg_id, + "project_group_name": "PG", + "roles": ["member"], + } + ], + ) + ) + + # First call populates the cache. + await _run_validate( + monkeypatch, + payload={"sub": USER_ID, "jti": "j1"}, + tdei=fresh_tdei(), + task=fakes.FakeSession(fakes.mappings()), + osm=fakes.FakeSession(fakes.mappings()), + ) + + # Second call with the same jti must NOT hit TDEI again: a raising client + # proves the cached entry is returned without a refetch. + monkeypatch.setattr( + sec, "_tdei_client", _FakeTdeiClient(exc=httpx.ConnectError("x")) + ) + monkeypatch.setattr( + sec, "validate_and_decode_token", lambda _t: {"sub": USER_ID, "jti": "j1"} + ) + info = await sec.validate_token( + _creds(), + cast(AsyncSession, fakes.FakeSession()), + cast(AsyncSession, fakes.FakeSession()), + ) + assert info.user_uuid == UUID(USER_ID) + + +async def test_cache_evicts_on_token_rotation(monkeypatch): + pg_id = "pg-1" + + def tdei_with_role(role): + return _FakeTdeiClient( + _FakeResp( + 200, + [ + { + "tdei_project_group_id": pg_id, + "project_group_name": "PG", + "roles": [role], + } + ], + ) + ) + + await _run_validate( + monkeypatch, + payload={"sub": USER_ID, "jti": "j1"}, + tdei=tdei_with_role("member"), + task=fakes.FakeSession(fakes.mappings()), + osm=fakes.FakeSession(fakes.mappings()), + ) + + # New jti -> stale entry evicted, fresh fetch performed. + info = await _run_validate( + monkeypatch, + payload={"sub": USER_ID, "jti": "j2"}, + tdei=tdei_with_role("poc"), + task=fakes.FakeSession(fakes.mappings()), + osm=fakes.FakeSession(fakes.mappings()), + ) + assert sec.TdeiProjectGroupRole.POINT_OF_CONTACT in info.projectGroups[0].tdeiRoles + + +def test_evict_user_from_cache_removes_entry(): + uid = UUID(USER_ID) + sentinel = factories.make_user_info(user_id=USER_ID) + sec._user_info_cache[uid] = sentinel + assert uid in sec._user_info_cache + + sec.evict_user_from_cache(uid) + assert uid not in sec._user_info_cache + + # Evicting an absent key is a no-op (no KeyError). + sec.evict_user_from_cache(uid) diff --git a/tests/unit/test_teams_schemas.py b/tests/unit/test_teams_schemas.py new file mode 100644 index 0000000..7a02421 --- /dev/null +++ b/tests/unit/test_teams_schemas.py @@ -0,0 +1,69 @@ +"""Schema tests for api/src/teams/schemas.py. + +Covers the @test comments on the table models: +- table name / columns match the DB schema +- foreign keys and relationships are correctly defined +- values serialize/deserialize without loss +""" + +import pytest +from pydantic import ValidationError + +from api.src.teams.schemas import ( + WorkspaceTeam, + WorkspaceTeamCreate, + WorkspaceTeamItem, + WorkspaceTeamUser, +) +from tests.support import factories + + +def _table(model): + # __table__ is added by SQLAlchemy at runtime; getattr keeps type-checkers happy. + return getattr(model, "__table__") + + +def _fk_targets(model): + return {fk.target_fullname for fk in _table(model).foreign_keys} + + +def test_team_user_link_table_schema(): + table = _table(WorkspaceTeamUser) + assert table.name == "team_user" + assert set(table.columns.keys()) == {"team_id", "user_id"} + assert {c.name for c in table.primary_key.columns} == {"team_id", "user_id"} + assert _fk_targets(WorkspaceTeamUser) == {"teams.id", "users.id"} + + +def test_team_table_schema(): + table = _table(WorkspaceTeam) + assert table.name == "teams" + assert {"id", "name", "workspace_id"} <= set(table.columns.keys()) + assert [c.name for c in table.primary_key.columns] == ["id"] + # workspace_id is indexed for lookups by workspace. + assert table.columns["workspace_id"].index is True + + +def test_team_has_users_relationship(): + # The many-to-many to User goes through the team_user link model. + rel = WorkspaceTeam.__sqlmodel_relationships__ + assert "users" in rel + + +def test_team_item_from_team_round_trip(): + user = factories.make_user(id=1) + team = factories.make_team(id=7, name="Alpha", users=[user]) + + item = WorkspaceTeamItem.from_team(team) + + assert item.id == 7 + assert item.name == "Alpha" + assert item.member_count == 1 + # Serializes cleanly to a dict for API responses. + assert item.model_dump() == {"id": 7, "name": "Alpha", "member_count": 1} + + +def test_team_create_requires_nonempty_name(): + assert WorkspaceTeamCreate(name="ok").name == "ok" + with pytest.raises(ValidationError): + WorkspaceTeamCreate(name="") diff --git a/tests/unit/test_users_schemas.py b/tests/unit/test_users_schemas.py new file mode 100644 index 0000000..2db9f84 --- /dev/null +++ b/tests/unit/test_users_schemas.py @@ -0,0 +1,78 @@ +"""Schema tests for api/src/users/schemas.py. + +Covers the @test comments on the User and WorkspaceUserRole table models: +- table name / columns match the DB schema (and the Alembic migration) +- foreign keys and relationships are correctly defined +- role enum values serialize/deserialize without loss +""" + +import pytest +from pydantic import ValidationError + +from api.src.users.schemas import ( + SetRoleRequest, + User, + WorkspaceUserRole, + WorkspaceUserRoleType, +) + + +def _table(model): + # __table__ is added by SQLAlchemy at runtime; getattr keeps type-checkers happy. + return getattr(model, "__table__") + + +def _fk_targets(model): + return {fk.target_fullname for fk in _table(model).foreign_keys} + + +def test_user_table_schema(): + table = _table(User) + assert table.name == "users" + assert {"id", "auth_uid", "email", "display_name"} <= set(table.columns.keys()) + assert [c.name for c in table.primary_key.columns] == ["id"] + # auth_uid and email are unique + indexed. + assert table.columns["auth_uid"].unique is True + assert table.columns["email"].unique is True + + +def test_user_has_teams_relationship(): + assert "teams" in User.__sqlmodel_relationships__ + + +def test_workspace_user_role_table_matches_alembic(): + # Mirrors alembic_osm migration 9221408912dd: + # user_workspace_roles(user_auth_uid, workspace_id, role enum), + # PK(user_auth_uid, workspace_id), FK user_auth_uid -> users.auth_uid + table = _table(WorkspaceUserRole) + assert table.name == "user_workspace_roles" + assert set(table.columns.keys()) == {"user_auth_uid", "workspace_id", "role"} + assert {c.name for c in table.primary_key.columns} == { + "user_auth_uid", + "workspace_id", + } + assert _fk_targets(WorkspaceUserRole) == {"users.auth_uid"} + + +def test_role_enum_values(): + assert WorkspaceUserRoleType.LEAD == "lead" + assert WorkspaceUserRoleType.VALIDATOR == "validator" + assert WorkspaceUserRoleType.CONTRIBUTOR == "contributor" + + +def test_set_role_request_rejects_contributor(): + # CONTRIBUTOR is implicit and may not be assigned directly. + assert SetRoleRequest(role=WorkspaceUserRoleType.LEAD).role == ( + WorkspaceUserRoleType.LEAD + ) + with pytest.raises(ValidationError): + SetRoleRequest(role=WorkspaceUserRoleType.CONTRIBUTOR) + + +def test_user_serialization_round_trip(): + user = User(id=3, auth_uid="abc", email="u@example.com", display_name="U") + dumped = user.model_dump() + assert dumped["id"] == 3 + assert dumped["auth_uid"] == "abc" + assert dumped["email"] == "u@example.com" + assert dumped["display_name"] == "U" diff --git a/tests/unit/test_workspaces_schemas.py b/tests/unit/test_workspaces_schemas.py new file mode 100644 index 0000000..15f6266 --- /dev/null +++ b/tests/unit/test_workspaces_schemas.py @@ -0,0 +1,202 @@ +"""Schema tests for api/src/workspaces/schemas.py. + +Covers the @test comments on the table models and WorkspaceResponse: +- table names / columns / PKs / FKs match the DB schema +- relationships are correctly defined +- enum TypeDecorators serialize to the DB and back without loss +- UUID / datetime values round-trip without precision loss +- WorkspaceResponse.from_workspace serializes for API responses incl. role +""" + +from datetime import datetime +from uuid import UUID + +import pytest +from pydantic import ValidationError +from sqlalchemy.engine.default import DefaultDialect + +from api.src.users.schemas import WorkspaceUserRoleType +from api.src.workspaces.schemas import ( + ExternalAppsDefinitionType, + IntEnumType, + QuestDefinitionType, + QuestDefinitionTypeName, + QuestSettingsPatch, + StrEnumType, + Workspace, + WorkspaceImagery, + WorkspaceLongQuest, + WorkspaceResponse, + WorkspaceType, +) +from tests.support import factories + +_DIALECT = DefaultDialect() + + +def _table(model): + # __table__ is added by SQLAlchemy at runtime; getattr keeps type-checkers happy. + return getattr(model, "__table__") + + +def _fk_targets(model): + return {fk.target_fullname for fk in _table(model).foreign_keys} + + +# --- table schemas --------------------------------------------------------- + + +def test_workspace_table_schema(): + table = _table(Workspace) + assert table.name == "workspaces" + expected = { + "id", + "type", + "title", + "description", + "tdeiProjectGroupId", + "tdeiRecordId", + "tdeiServiceId", + "tdeiMetadata", + "createdAt", + "createdBy", + "createdByName", + "geometry", + "externalAppAccess", + "kartaViewToken", + } + assert expected <= set(table.columns.keys()) + assert [c.name for c in table.primary_key.columns] == ["id"] + + +def test_workspace_relationships_defined(): + rels = Workspace.__sqlmodel_relationships__ + assert "longFormQuestDef" in rels + assert "imageryListDef" in rels + + +def test_long_quest_table_schema(): + table = _table(WorkspaceLongQuest) + assert table.name == "workspaces_long_quests" + assert {"workspace_id", "type", "definition", "url", "modifiedAt"} <= set( + table.columns.keys() + ) + assert [c.name for c in table.primary_key.columns] == ["workspace_id"] + assert _fk_targets(WorkspaceLongQuest) == {"workspaces.id"} + + +def test_imagery_table_schema(): + table = _table(WorkspaceImagery) + assert table.name == "workspaces_imagery" + assert {"workspace_id", "definition", "modifiedAt"} <= set(table.columns.keys()) + assert [c.name for c in table.primary_key.columns] == ["workspace_id"] + assert _fk_targets(WorkspaceImagery) == {"workspaces.id"} + + +# --- enum TypeDecorator round-trips (Python <-> DB) ------------------------ + + +def test_int_enum_type_round_trip(): + deco = IntEnumType(QuestDefinitionType) + # bind: enum -> int + assert deco.process_bind_param(QuestDefinitionType.JSON, _DIALECT) == 1 + # result: int -> enum + assert deco.process_result_value(1, _DIALECT) == QuestDefinitionType.JSON + # None passes through both directions + assert deco.process_bind_param(None, _DIALECT) is None + assert deco.process_result_value(None, _DIALECT) is None + + +def test_str_enum_type_round_trip(): + deco = StrEnumType(WorkspaceType) + assert deco.process_bind_param(WorkspaceType.OSW, _DIALECT) == "osw" + assert deco.process_result_value("osw", _DIALECT) == WorkspaceType.OSW + assert deco.process_bind_param(None, _DIALECT) is None + assert deco.process_result_value(None, _DIALECT) is None + + +def test_external_apps_enum_round_trip(): + deco = IntEnumType(ExternalAppsDefinitionType) + assert deco.process_bind_param(ExternalAppsDefinitionType.PUBLIC, _DIALECT) == 1 + assert ( + deco.process_result_value(2, _DIALECT) + == ExternalAppsDefinitionType.PROJECT_GROUP + ) + + +# --- value preservation ---------------------------------------------------- + + +def test_workspace_preserves_uuid_and_datetime(): + pg = UUID("11111111-1111-1111-1111-111111111111") + created = datetime(2026, 1, 2, 3, 4, 5) + ws = Workspace( + id=1, + type=WorkspaceType.OSW, + title="T", + tdeiProjectGroupId=pg, + createdBy=pg, + createdByName="N", + createdAt=created, + ) + assert ws.tdeiProjectGroupId == pg + assert ws.createdAt == created # no truncation + assert ws.type == WorkspaceType.OSW + + +# --- WorkspaceResponse serialization -------------------------------------- + + +def test_workspace_response_includes_effective_role(): + user = factories.make_user_info( + osm_workspace_roles={1: [WorkspaceUserRoleType.LEAD]} + ) + ws = factories.make_workspace(id=1, title="Mappy") + + resp = WorkspaceResponse.from_workspace(ws, user) + + assert resp.id == 1 + assert resp.title == "Mappy" + assert resp.role == WorkspaceUserRoleType.LEAD + assert resp.type == WorkspaceType.OSW + + +def test_workspace_response_passes_through_defs(): + user = factories.make_user_info() + ws = factories.make_workspace(id=2) + + resp = WorkspaceResponse.from_workspace( + ws, user, imagery_list_def=[{"a": 1}], long_form_quest_def={"q": 2} + ) + + assert resp.imageryListDef == [{"a": 1}] + assert resp.longFormQuestDef == {"q": 2} + assert resp.role == WorkspaceUserRoleType.CONTRIBUTOR + + +# --- QuestSettingsPatch validation ---------------------------------------- + + +def test_quest_settings_json_requires_object_definition(): + ok = QuestSettingsPatch(type=QuestDefinitionTypeName.JSON, definition='{"a": 1}') + assert ok.type == QuestDefinitionTypeName.JSON + + with pytest.raises(ValidationError): + QuestSettingsPatch(type=QuestDefinitionTypeName.JSON, definition=None) + with pytest.raises(ValidationError): + QuestSettingsPatch(type=QuestDefinitionTypeName.JSON, definition="not-json") + + +def test_quest_settings_url_requires_url(): + ok = QuestSettingsPatch(type=QuestDefinitionTypeName.URL, url="https://x") + assert ok.url == "https://x" + with pytest.raises(ValidationError): + QuestSettingsPatch(type=QuestDefinitionTypeName.URL, url=None) + + +def test_quest_settings_none_rejects_payload(): + assert QuestSettingsPatch(type=QuestDefinitionTypeName.NONE).type == ( + QuestDefinitionTypeName.NONE + ) + with pytest.raises(ValidationError): + QuestSettingsPatch(type=QuestDefinitionTypeName.NONE, definition='{"a":1}') From b786099cc9eb5c0aaeb0d73db1f5422ced5afb4d Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Wed, 24 Jun 2026 14:21:09 -0400 Subject: [PATCH 05/18] Docs --- CLAUDE.md | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ README.md | 20 +++++++++++++++++ 2 files changed, 86 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index c2d7f3a..ee0eade 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -30,3 +30,69 @@ Viewer/Member/Everyone Else * With express TDEI sign-up, the need for this access level diminishes greatly * Granted by Workspaces setting. +## Testing + +Two layers, both fast and dependency-free (no Postgres, PostGIS, Docker, or +network). `tests/README.md` has the full reference; the essentials: + +* **Unit** (`tests/unit/`) — pure logic and individual classes (permission + rules, schema/DTO behavior, a repository in isolation). +* **Integration** (`tests/integration/`) — real HTTP requests driven through + the real FastAPI app: routing, auth wiring, repositories, serialization. + +### The mocking boundary is the "data fetcher", not the repository + +Integration tests run the **real** routes and repositories. Only three things +are swapped out, via `app.dependency_overrides` and a fake: + +1. `get_task_session` / `get_osm_session` → a `FakeSession` (in + `tests/support/fakes.py`) that returns pre-programmed `FakeResult`s instead + of running SQL. This is the data-fetcher boundary: everything above the + `AsyncSession` runs for real. +2. `validate_token` → a real `UserInfo` built by `tests/support/factories.py` + (skips JWT decode + the TDEI call; the permission logic is still real). +3. `api.main._osm_client` → a streamable mock transport (proxy tests only; + `tests/support/http.py`). + +Because the mock is at the session level, **queue results in the order the +repository issues queries**. Routes that touch both DBs queue on both +`task_session` and `osm_session`. Builders: `rows()`, `empty()`, `affected(n)`, +`mappings()`, `scalar(v)`, and `raises(exc)` (drives 500 paths). The +`error_client` fixture turns unhandled exceptions into 500 responses (httpx's +ASGI transport re-raises by default). + +### `@test:` comment outlines + +Modules carry `# @test:` comments describing intended coverage. They are the +spec for the test suite; when adding behavior, add matching `@test:` lines and +tests. Treat the docstring/attribute comments as authoritative when they and +the code disagree — file a fix rather than silently matching the code. + +### Known behavior discrepancy: read endpoints return 404, not 403 + +Several `@test:` comments on read endpoints (get workspace, list teams, quest +and imagery GETs) specify a **403** when the caller lacks access. The code +enforces access via `WorkspaceRepository.getById`, which raises **404 +NotFound** when the workspace is missing *or* inaccessible — so "not a member" +currently surfaces as 404 on those routes. The tests assert the actual 404 +behavior and flag this in their docstrings. If 403 is the intended contract, +that is a code change in the read routes, not a test change. + +### SQLModel + Pyright + +SQLModel declares columns as plain annotations (e.g. `id: int | None`) rather +than `Mapped[int]`, so Pyright reads `Column == value` as `bool` and flags +`where()`/`exec()`/`select()`/`selectinload` calls and `result.rowcount`. These +are framework false positives. The three repository modules carry a documented +file-level `# pyright: reportArgumentType=false, reportCallIssue=false, +reportAttributeAccessIssue=false` directive; other rules stay enabled so real +bugs still surface. Keep `api/` and `tests/` at zero Pyright errors. + +### Alembic enum migrations + +Postgres `ENUM` types must be created/dropped idempotently. Declare the enum +with `create_type=False` and manage it explicitly with +`enum.create(op.get_bind(), checkfirst=True)` / `enum.drop(..., checkfirst=True)` +so a migration is safe whether or not the type already exists (and never +double-creates it via implicit table DDL). + diff --git a/README.md b/README.md index 7ac5971..7909fbd 100644 --- a/README.md +++ b/README.md @@ -14,3 +14,23 @@ uv sync uv run uvicorn api.main:app ``` +## Running the tests + +Tests are fast and require no database, Docker, or network (see +`tests/README.md` for the design, and `CLAUDE.md` for conventions). + +``` +uv run pytest # full suite with coverage (configured in pyproject.toml) +uv run pytest --no-cov -q # quick run, no coverage +uv run pytest tests/unit # unit tests only +uv run pytest tests/integration # integration tests only +uv run pytest -k workspaces # filter by keyword +``` + +Type-check and format (matches the pre-commit hooks): + +``` +uvx pyright --pythonpath .venv/bin/python api tests +uv run black api tests && uv run isort api tests +``` + From f655772a832447e2bd68d57a034d150463667f01 Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Wed, 24 Jun 2026 14:26:52 -0400 Subject: [PATCH 06/18] Update ci.yml --- .github/workflows/ci.yml | 27 ++++++--------------------- 1 file changed, 6 insertions(+), 21 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c72a0ff..a2e8097 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,6 +35,9 @@ jobs: - name: Check code formatting with black run: uv run black --check . + - name: Type-check with pyright + run: uvx pyright --pythonpath .venv/bin/python api tests + test: needs: lint runs-on: ubuntu-latest @@ -58,25 +61,7 @@ jobs: - name: Install the project run: uv sync --all-extras + # The suite needs no database: migrations are skipped under pytest and the + # DB session is mocked at the data-fetcher boundary (see CLAUDE.md). - name: Run tests - run: uv run pytest tests - env: - TASK_DATABASE_URL: "postgresql+asyncpg://postgres:postgres@localhost:5432/test_db" - OSM_DATABASE_URL: "postgresql+asyncpg://postgres:postgres@localhost:5432/test_db" - JWT_SECRET: "test_secret_key" - JWT_ALGORITHM: "HS256" - - services: - postgres: - image: postgres:16 - env: - POSTGRES_USER: postgres - POSTGRES_PASSWORD: postgres - POSTGRES_DB: test_db - ports: - - 5432:5432 - options: >- - --health-cmd pg_isready - --health-interval 10s - --health-timeout 5s - --health-retries 5 \ No newline at end of file + run: uv run pytest tests \ No newline at end of file From 65a9d6687ff37e9bcee5f6357970cbb5bc53922f Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Wed, 24 Jun 2026 14:30:49 -0400 Subject: [PATCH 07/18] isort fix --- api/core/config.py | 1 + pyproject.toml | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/api/core/config.py b/api/core/config.py index 5824796..c4c6bde 100644 --- a/api/core/config.py +++ b/api/core/config.py @@ -1,5 +1,6 @@ from pydantic_settings import BaseSettings, SettingsConfigDict + # Test outline: # @test: Test that environment variables of the same name as the members of this class are correctly loaded into the members of this class. # @test: Test that any environment variables that are not set in the environment are correctly loaded into the members of this class with their default values. diff --git a/pyproject.toml b/pyproject.toml index 910a8ac..c211d27 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -55,3 +55,7 @@ force_grid_wrap = 0 use_parentheses = true ensure_newline_before_comments = true line_length = 88 +# Declare our packages explicitly. Without this, the top-level alembic/ dir +# makes isort misclassify the installed `alembic` package as first-party. +known_first_party = ["api", "tests"] +known_third_party = ["alembic"] From e7728469452f3731d539eae2d6067d6344ce9f96 Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Wed, 24 Jun 2026 14:31:30 -0400 Subject: [PATCH 08/18] Black linter fixes --- api/core/json_schema.py | 4 +++- api/core/jwt.py | 3 ++- api/core/security.py | 8 ++++++-- api/main.py | 2 +- api/src/teams/routes.py | 29 +++++++++++++++++++++++------ api/src/teams/schemas.py | 12 +++++++----- api/src/users/routes.py | 9 +++++++-- api/src/users/schemas.py | 10 ++++++---- api/src/workspaces/routes.py | 23 ++++++++++++++++++++--- api/src/workspaces/schemas.py | 15 +++++++++------ 10 files changed, 84 insertions(+), 31 deletions(-) diff --git a/api/core/json_schema.py b/api/core/json_schema.py index 4d6f54a..4048de5 100644 --- a/api/core/json_schema.py +++ b/api/core/json_schema.py @@ -19,9 +19,10 @@ # Test outline: # @test: Test that this class validates JSON payloads properly against the JSON schema fetched -# @test: Test that malformed JSON or JSON that doesn't validate +# @test: Test that malformed JSON or JSON that doesn't validate # @test: Test that any failed network requests are handled gracefully and return a proper error (not just "Exception") or worse a false positive validation + def init_json_schema_client() -> None: global _http_client _http_client = httpx.AsyncClient( @@ -124,6 +125,7 @@ async def validate_quest_definition_schema(definition: str) -> None: detail=f"{e.message} at {list(e.path)}", ) + async def validate_imagery_definition_schema(definition: list[Any]) -> None: """ Validate the provided definition against the imagery list schema. diff --git a/api/core/jwt.py b/api/core/jwt.py index 522a8e2..ab10f6a 100644 --- a/api/core/jwt.py +++ b/api/core/jwt.py @@ -7,9 +7,10 @@ # Test outline: # @test: Test that this class validates JSON payloads properly against the JSON schema fetched -# @test: Test that malformed JSON or JSON that doesn't validate +# @test: Test that malformed JSON or JSON that doesn't validate # @test: Test that any failed network requests are handled gracefully and return a proper error (not just "Exception") or worse a false positive validation + def _get_jwks_client() -> jwt.PyJWKClient: global _jwks_client diff --git a/api/core/security.py b/api/core/security.py index 7e8baa1..90c5b6a 100644 --- a/api/core/security.py +++ b/api/core/security.py @@ -17,7 +17,7 @@ # Test outline: # @test: Test that the permissions structure here matches what is described in CLAUDE.md # @test: Test that the methods on the UserInfo class return the correct values for a given set of project groups and workspace roles -# @test: Test that any failed network requests are handled gracefully +# @test: Test that any failed network requests are handled gracefully # @test: Test that the caching mechanism works correctly and evicts entries when roles change # Set up logger for this module @@ -62,6 +62,7 @@ def evict_user_from_cache(auth_uid: UUID) -> None: security = HTTPBearer() + # @test: Test that this matches what is described in CLAUDE.md and that the methods on the UserInfo class return the correct values for a given set of project groups and workspace roles class TdeiProjectGroupRole(StrEnum): MEMBER = "member" @@ -86,6 +87,7 @@ def __init__( self.project_group_id = project_group_id self.tdeiRoles = tdeiRoles + # @test: Test that the values populated in this class match the expected values from the JWT and TDEI API responses, and that the methods return the correct roles based on the user's project group memberships and workspace roles # @test: Test that the osmWorkspaceRoles, accessibleWorkspaceIds, and projectGroups attributes are correctly populated based on the user's roles in the OSM DB and TDEI API responses # @test: Test that the comments in this doc that describe what is supposed to be in the attributes are accurate and match the actual data being stored in those attributes and what is returned. The comments should be considered authoratative @@ -167,7 +169,9 @@ def get_task_db_session( ) -> AsyncSession: return session -# @test: + +# @test: + async def validate_token( credentials: HTTPAuthorizationCredentials = Depends(security), diff --git a/api/main.py b/api/main.py index a2582dc..11fd7a0 100644 --- a/api/main.py +++ b/api/main.py @@ -124,7 +124,7 @@ def get_workspace_repository( # @test: Any headers defined in HOP_BY_HOP_HEADERS are not forwarded to the client # @test: /api/capabilities.json is proxied to the OSM service without requiring authentication # @test: Any request to the OSM service that returns a 4xx or 5xx status code is logged to Sentry with the correct message and the correct status code is returned to the client -# @test: Any request that matches the RegEx in TENANT_BYPASSES is allowed to proceed without an X-Workspace header, +# @test: Any request that matches the RegEx in TENANT_BYPASSES is allowed to proceed without an X-Workspace header, # and any request that does not match the Regex in TENANT_BYPASSES and does not have an X-Workspace header returns a 400 Bad Request error # @test: Only the methods defined in the @app.api_route decorator are allowed to be proxied to the OSM service, and any other methods return a 405 Method Not Allowed error # @test: Any request with an X-Workspace header that does not match the user's accessible workspaces returns a 403 Forbidden error diff --git a/api/src/teams/routes.py b/api/src/teams/routes.py index 2e1e760..678989a 100644 --- a/api/src/teams/routes.py +++ b/api/src/teams/routes.py @@ -36,12 +36,14 @@ def get_team_repo( repo = WorkspaceTeamRepository(session) return repo + # @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace member of any level # @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs # @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 # @test: Test that this endpoint properly calls the repository to fetch the team from the database # @test: Test that this method properly handles inputs that match the schema in WorkspaceTeamItem + @router.get("") async def get_all_teams_for_workspace( workspace_id: int, @@ -53,12 +55,14 @@ async def get_all_teams_for_workspace( await workspace_repo.getById(current_user, workspace_id) return await team_repo.get_all(workspace_id) + # @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace lead # @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs # @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 # @test: Test that this endpoint properly calls the repository to add the team to the database # @test: Test that this method properly handles inputs that match the schema in WorkspaceTeamCreate + @router.post("", status_code=status.HTTP_201_CREATED) async def create_team_for_workspace( workspace_id: int, @@ -77,6 +81,7 @@ async def create_team_for_workspace( await workspace_repo.getById(current_user, workspace_id) return await team_repo.create(workspace_id, team) + # @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace member of any level # @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs # @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 @@ -84,6 +89,7 @@ async def create_team_for_workspace( # @test: Test that this endpoint properly calls the repository to fetch the team from the database # @test: Test that this method properly handles inputs that match the schema in WorkspaceTeamItem + @router.get("/{team_id}") async def get_team_for_workspace( workspace_id: int, @@ -97,14 +103,16 @@ async def get_team_for_workspace( await team_repo.assert_team_in_workspace(team_id, workspace_id) return await team_repo.get_item(team_id) + # @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace lead # @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs # @test: Test that this endpoint properly handles the case where the team does not exist and returns a 404 # @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 -# @test: Test that this endpoint properly handles the case where the workspace is not associated with the team +# @test: Test that this endpoint properly handles the case where the workspace is not associated with the team # @test: Test that this endpoint properly calls the repository to remove the team from the workspace # @test: Test that this method properly handles inputs that match the schema in WorkspaceTeamUpdate -# @test: Test that this method properly calls the repo to update the team +# @test: Test that this method properly calls the repo to update the team + @router.put("/{team_id}", status_code=status.HTTP_204_NO_CONTENT) async def update_team_for_workspace( @@ -126,13 +134,15 @@ async def update_team_for_workspace( await team_repo.assert_team_in_workspace(team_id, workspace_id) await team_repo.update(team_id, team) + # @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace lead # @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs # @test: Test that this endpoint properly handles the case where the team does not exist and returns a 404 # @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 -# @test: Test that this endpoint properly handles the case where the workspace is not associated with the team +# @test: Test that this endpoint properly handles the case where the workspace is not associated with the team # @test: Test that this endpoint properly calls the repository to remove the team from the workspace + @router.delete("/{team_id}", status_code=status.HTTP_204_NO_CONTENT) async def delete_team_from_workspace( workspace_id: int, @@ -152,6 +162,7 @@ async def delete_team_from_workspace( await team_repo.assert_team_in_workspace(team_id, workspace_id) await team_repo.delete(team_id) + # @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a member team # @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs # @test: Test that this endpoint properly handles the case where the team does not exist and returns a 404 @@ -161,6 +172,7 @@ async def delete_team_from_workspace( # @test: Test that this endpoint properly handles the case where the workspace is not associated with the team passed # @test: Test that this endpoint properly calls the repository to fetch the users of the team + @router.get("/{team_id}/members") async def get_members_in_workspace_team( workspace_id: int, @@ -183,6 +195,7 @@ async def get_members_in_workspace_team( # @test: Test that this endpoint properly handles the case where the workspace is not associated with the team passed # @test: Test that this endpoint properly calls the repository to add the user to the team + @router.post("/{team_id}/members") async def join_workspace_team( workspace_id: int, @@ -199,15 +212,17 @@ async def join_workspace_team( await team_repo.add_member(team_id, user.id) return user + # @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace lead # @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs # @test: Test that this endpoint properly handles the case where the team does not exist and returns a 404 # @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 # @test: Test that this endpoint properly handles the case where the user does not exist and returns a 404 -# @test: Test that this endpoint properly handles the case where the workspace is not associated with the team +# @test: Test that this endpoint properly handles the case where the workspace is not associated with the team # @test: Test that this endpoint doesn't allow changing teams the user doesn't have workspace lead permissions for, or users not already associated with the workspace # @test: Test that this endpoint properly calls the repository to add the user to the team + @router.put("/{team_id}/members/{user_id}", status_code=status.HTTP_204_NO_CONTENT) async def add_member_to_workspace_team( workspace_id: int, @@ -228,17 +243,19 @@ async def add_member_to_workspace_team( await team_repo.assert_team_in_workspace(team_id, workspace_id) await team_repo.add_member(team_id, user_id) + # @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace lead # @test: Test that the endpoint properly validates that the team needs to be associated with this workspace and errors if not # @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs # @test: Test that this endpoint properly handles the case where the team does not exist and returns a 404 # @test: Test that this endpoint properly handles the case where the user does not exist and returns a 404 # @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 -# @test: Test that this endpoint properly handles the case where the user is not associated with the team -# @test: Test that this endpoint properly handles the case where the workspace is not associated with the team +# @test: Test that this endpoint properly handles the case where the user is not associated with the team +# @test: Test that this endpoint properly handles the case where the workspace is not associated with the team # @test: Test that this endpoint doesn't allow changing teams the user doesn't have workspace lead permissions for, or users not already associated with the team # @test: Test that this endpoint properly calls the repository to remove the user from the team + @router.delete("/{team_id}/members/{user_id}", status_code=status.HTTP_204_NO_CONTENT) async def delete_member_from_workspace_team( workspace_id: int, diff --git a/api/src/teams/schemas.py b/api/src/teams/schemas.py index 0aefcf0..361f2a4 100644 --- a/api/src/teams/schemas.py +++ b/api/src/teams/schemas.py @@ -5,14 +5,15 @@ if TYPE_CHECKING: from api.src.users.schemas import User -# @test: Test that this class matches the Alembic-defined database schema + +# @test: Test that this class matches the Alembic-defined database schema # @test: Test that the foreign key references are correct and that the relationships are correctly defined # @test: Test that any values from Python and properly serialized to the database and that any values from the database are properly deserialized to Python -# @test: Test that any serialization/deserialization doesn't lose any precision or data +# @test: Test that any serialization/deserialization doesn't lose any precision or data class WorkspaceTeamUser(SQLModel, table=True): """Team to User link table""" - __tablename__ = "team_user" # type: ignore[assignment] + __tablename__ = "team_user" # type: ignore[assignment] team_id: int | None = Field(default=None, primary_key=True, foreign_key="teams.id") user_id: int | None = Field(default=None, primary_key=True, foreign_key="users.id") @@ -23,10 +24,11 @@ class WorkspaceTeamBase(SQLModel): name: str = Field(min_length=1) -# @test: Test that this class matches the Alembic-defined database schema + +# @test: Test that this class matches the Alembic-defined database schema # @test: Test that the foreign key references are correct and that the relationships are correctly defined # @test: Test that any values from Python and properly serialized to the database and that any values from the database are properly deserialized to Python -# @test: Test that any serialization/deserialization doesn't lose any precision or data +# @test: Test that any serialization/deserialization doesn't lose any precision or data class WorkspaceTeam(WorkspaceTeamBase, table=True): """Workspace teams""" diff --git a/api/src/users/routes.py b/api/src/users/routes.py index 9a96162..50197c9 100644 --- a/api/src/users/routes.py +++ b/api/src/users/routes.py @@ -24,11 +24,13 @@ def get_workspace_repo( ) -> WorkspaceRepository: return WorkspaceRepository(session) + # @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not associated with this workspace at contributor or above # @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs # @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 # @test: Test that this endpoint properly handles the case where the user is not associated with the workspace (returns a 403 error) + @router.get("", response_model=list[WorkspaceUserRoleItem]) async def get_privileged_workspace_members( workspace_id: int, @@ -48,12 +50,13 @@ async def get_privileged_workspace_members( # @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs # @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 # @test: Test that this endpoint properly handles the case where the user does not exist and returns a 404 -# @test: Test that this endpoint properly handles the case where the user is not associated with the workspace +# @test: Test that this endpoint properly handles the case where the user is not associated with the workspace # @test: Test that this endpoint properly evicts any cached data for the user after the user is deleted so that their next request reflects the deletion rather than serving stale data for up to an hour # @test: Test that this endpoint properly allows users to be workspace leads or validators, and to unset the user of either role and become a contributor again # @test: Test that this endpoint doesn't allow changing workspaces the user doesn't have workspace lead permissions for, or roles for users not already associated with the workspace # @test: Test that this endpoint doesn't allow setting the workspace role to a POC + @router.put("/{user_id}/role", status_code=status.HTTP_204_NO_CONTENT) async def assign_member_role( workspace_id: int, @@ -78,13 +81,15 @@ async def assign_member_role( await user_repo.assign_member_role(workspace_id, user_id, body.role) evict_user_from_cache(user_id) + # @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace lead # @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs # @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 # @test: Test that this endpoint properly handles the case where the user does not exist and returns a 404 -# @test: Test that this endpoint properly handles the case where the user is not associated with the workspace +# @test: Test that this endpoint properly handles the case where the user is not associated with the workspace # @test: Test that this endpoint properly evicts any cached data for the user after the user is deleted so that their next request reflects the deletion rather than serving stale data for up to an hour + @router.delete("/{user_id}", status_code=status.HTTP_204_NO_CONTENT) async def remove_member_role( workspace_id: int, diff --git a/api/src/users/schemas.py b/api/src/users/schemas.py index bc8e3be..be2ebbe 100644 --- a/api/src/users/schemas.py +++ b/api/src/users/schemas.py @@ -16,10 +16,11 @@ class WorkspaceUserRoleType(StrEnum): VALIDATOR = "validator" CONTRIBUTOR = "contributor" -# @test: Test that this class matches the Alembic-defined database schema + +# @test: Test that this class matches the Alembic-defined database schema # @test: Test that the foreign key references are correct and that the relationships are correctly defined # @test: Test that any values from Python and properly serialized to the database and that any values from the database are properly deserialized to Python -# @test: Test that any serialization/deserialization doesn't lose any precision or data +# @test: Test that any serialization/deserialization doesn't lose any precision or data class WorkspaceUserRole(SQLModel, table=True): """Associates users with workspaces and their roles""" @@ -65,10 +66,11 @@ class WorkspaceUserRoleItem(SQLModel): display_name: str role: WorkspaceUserRoleType -# @test: Test that this class matches the Alembic-defined database schema + +# @test: Test that this class matches the Alembic-defined database schema # @test: Test that the foreign key references are correct and that the relationships are correctly defined # @test: Test that any values from Python and properly serialized to the database and that any values from the database are properly deserialized to Python -# @test: Test that any serialization/deserialization doesn't lose any precision or data +# @test: Test that any serialization/deserialization doesn't lose any precision or data class User(SQLModel, table=True): """Users in the OSM DB""" diff --git a/api/src/workspaces/routes.py b/api/src/workspaces/routes.py index fdc0a95..e5d35dd 100644 --- a/api/src/workspaces/routes.py +++ b/api/src/workspaces/routes.py @@ -56,9 +56,10 @@ def get_user_repository( # @test: Test that this method properly handles numeric workspace_id input and invalid values for the same # @test: Test that this method properly checks permissions to see if the user has access to the workspace and if they don't, it doesn't appear in the list # @test: Test that this method properly handles the case where the workspace does not exist and doesn't include it -# @test: Test that this method properly handles inputs that match the schema in WorkspaceResponse +# @test: Test that this method properly handles inputs that match the schema in WorkspaceResponse # @test: Test that this method's results match the values of the fetch-by-workspace-id method below; all workspaces in this list are retrievable via that method + # Returns list of workspaces user has access to as JSON payload on success--returns empty JSON list if none @router.get("/mine", response_model=list[WorkspaceResponse]) async def get_my_workspaces( @@ -72,12 +73,14 @@ async def get_my_workspaces( logger.error(f"Failed to fetch workspaces: {str(e)}") raise + # @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs # @test: Test that this method properly calls the repository method to fetch the workspace and that the repository method properly fetches the workspace from the database # @test: Test that this method properly handles numeric workspace_id input and invalid values for the same # @test: Test that this method properly checks permissions to see if the user has access to the workspace and returns a 403 if they do not # @test: Test that this method properly handles the case where the workspace does not exist and returns a 404 -# @test: Test that this method properly handles inputs that match the schema in WorkspaceResponse +# @test: Test that this method properly handles inputs that match the schema in WorkspaceResponse + # Returns JSON payload or 204 if not found @router.get("/{workspace_id}", response_model=WorkspaceResponse) @@ -110,11 +113,13 @@ async def get_workspace( logger.error(f"Failed to fetch workspace {workspace_id}: {str(e)}") raise + # @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs # @test: Test that this method properly handles numeric workspace_id input and invalid values for the same # @test: Test taht this workspace returns proper values for a workspace that exists and that the bbox matches the expected values # @test: Test that this method properly handles the case where the workspace does not exist and returns a 404 + @router.get("/{workspace_id}/bbox", response_model=None) async def get_workspace_bbox( workspace_id: int, @@ -131,6 +136,7 @@ async def get_workspace_bbox( logger.error(f"Failed to fetch workspace {workspace_id}: {str(e)}") raise + # @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs # @test: Test that this endpoint properly evicts any cached data for the user after the workspace is deleted so that their next request reflects the deletion rather than serving stale data for up to an hour # @test: Test that this method properly calls the repository method to create the workspace and that the repository method properly creates the workspace in the database @@ -139,6 +145,7 @@ async def get_workspace_bbox( # @test: Test that this method properly handles inputs that do not match the schema in WorkspaceCreate and that the repository method properly raises an error and does not update the workspace in the database with those values # @test: Test that this method won't allow users to modify an existing workspace in any way, or create a workspace with the same workspace_id as an existing one + # Returns 201 on success? @router.post("", status_code=status.HTTP_201_CREATED) async def create_workspace( @@ -171,6 +178,7 @@ async def create_workspace( logger.error(f"Failed to create workspace: {str(e)}") raise + # @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace lead # @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs # @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 @@ -179,6 +187,7 @@ async def create_workspace( # @test: Test that this method properly handles inputs that match the schema in WorkspacePatch and that the repository method properly updates the workspace in the database with those values # @test: Test that this method properly handles inputs that do not match the schema in WorkspacePatch and that the repository method properly raises an error and does not update the workspace in the database with those values + @router.patch("/{workspace_id}", status_code=status.HTTP_204_NO_CONTENT) async def update_workspace( workspace_id: int, @@ -213,6 +222,7 @@ async def update_workspace( # @test: Test that this method properly handles the case where the workspace has members and that the repository method properly removes all member roles from the database # @test: Test that this method properly handles numeric workspace_id input and invalid values for the same + # Returns 204 on success @router.delete("/{workspace_id}", status_code=status.HTTP_204_NO_CONTENT) async def delete_workspace( @@ -245,7 +255,8 @@ async def delete_workspace( # @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 # @test: Test that this method properly calls the repository method to fetch the long quest definition -# FIXME: Why are there two methods to fetch the long quest? One for legacy purposes? Can we migrate callers? +# FIXME: Why are there two methods to fetch the long quest? One for legacy purposes? Can we migrate callers? + # Return the resolved quest definition content as JSON, or 204 if not set: @router.get("/{workspace_id}/quests/long") @@ -270,12 +281,14 @@ async def get_long_quest_def( ) raise + # @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace lead # @test: Test that this endpoint properly handles any exceptions and returns a 500 if an unexpected error occurs # @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 # @test: Test that this method properly calls the repository method to fetch the long quest definition, and if it's not defined, the default value as defined # in this method + # Returns JSON payload or 204 if not set @router.get( "/{workspace_id}/quests/long/settings", response_model=QuestSettingsResponse @@ -313,6 +326,7 @@ async def get_long_quest_settings( logger.error(f"Failed to fetch workspace {workspace_id}: {str(e)}") raise + # @test: Test that this endpoint properly validates the user's permissions and returns a 403 if the user is not a workspace lead # @test: Test that this endpoint properly validates the long quest definition against the JSON schema and returns a 400 if the definition is invalid # @test: Test that this endpoint properly saves the long quest definition to the database and returns a 204 on success @@ -321,6 +335,7 @@ async def get_long_quest_settings( # @test: Test that this endpoint properly handles input values that are properly formed, malformed and edge cases, including empty lists, null values, and large payloads # @test: Test that this method properly calls the repository method to save the long quest definition and that the repository method properly saves the definition to the database + # Returns 204 on success @router.patch( "/{workspace_id}/quests/long/settings", status_code=status.HTTP_204_NO_CONTENT @@ -361,6 +376,7 @@ async def update_long_quest_settings( # @test: Test that this endpoint properly handles the case where the workspace does not exist and returns a 404 # @test: Test that this method properly calls the repository method to fetch the imagery definition + # Returns JSON payload or 204 if not set @router.get("/{workspace_id}/imagery/settings") async def get_imagery_settings( @@ -394,6 +410,7 @@ async def get_imagery_settings( # @test: Test that this endpoint properly handles input values that are properly formed, malformed and edge cases, including empty lists, null values, and large payloads # @test: Test that this method properly calls the repository method to save the imagery definition and that the repository method properly saves the definition to the database + # Returns 204 on success @router.patch( "/{workspace_id}/imagery/settings", status_code=status.HTTP_204_NO_CONTENT diff --git a/api/src/workspaces/schemas.py b/api/src/workspaces/schemas.py index 34f28f7..07355cc 100644 --- a/api/src/workspaces/schemas.py +++ b/api/src/workspaces/schemas.py @@ -78,10 +78,11 @@ class QuestDefinitionTypeName(StrEnum): JSON = "JSON" URL = "URL" -# @test: Test that this class matches the Alembic-defined database schema + +# @test: Test that this class matches the Alembic-defined database schema # @test: Test that the foreign key references are correct and that the relationships are correctly defined # @test: Test that any values from Python and properly serialized to the database and that any values from the database are properly deserialized to Python -# @test: Test that any serialization/deserialization doesn't lose any precision or data +# @test: Test that any serialization/deserialization doesn't lose any precision or data class WorkspaceLongQuest(SQLModel, table=True): """Stores mobile app quest definitions for a workspace""" @@ -105,10 +106,11 @@ class WorkspaceLongQuest(SQLModel, table=True): modifiedBy: UUID modifiedByName: str -# @test: Test that this class matches the Alembic-defined database schema + +# @test: Test that this class matches the Alembic-defined database schema # @test: Test that the foreign key references are correct and that the relationships are correctly defined # @test: Test that any values from Python and properly serialized to the database and that any values from the database are properly deserialized to Python -# @test: Test that any serialization/deserialization doesn't lose any precision or data +# @test: Test that any serialization/deserialization doesn't lose any precision or data class WorkspaceImagery(SQLModel, table=True): """Stores imagery list for a workspace""" @@ -253,10 +255,11 @@ def from_workspace( longFormQuestDef=long_form_quest_def, ) -# @test: Test that this class matches the Alembic-defined database schema + +# @test: Test that this class matches the Alembic-defined database schema # @test: Test that the foreign key references are correct and that the relationships are correctly defined # @test: Test that any values from Python and properly serialized to the database and that any values from the database are properly deserialized to Python -# @test: Test that any serialization/deserialization doesn't lose any precision or data +# @test: Test that any serialization/deserialization doesn't lose any precision or data class Workspace(SQLModel, table=True): """Workspaces""" From a00362be66a4678a0defc1f1ca681e01cfbc971e Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Wed, 24 Jun 2026 14:33:35 -0400 Subject: [PATCH 09/18] Merge jobs --- .github/workflows/ci.yml | 36 +++++++----------------------------- 1 file changed, 7 insertions(+), 29 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a2e8097..6f914c6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,7 +7,7 @@ on: branches: [ main ] jobs: - lint: + ci: runs-on: ubuntu-latest strategy: matrix: @@ -20,12 +20,13 @@ jobs: uses: astral-sh/setup-uv@v4 with: version: "0.5.8" - - - name: Set up Python + enable-cache: true + + - name: Set up Python ${{ matrix.python-version }} uses: actions/setup-python@v5 with: - python-version: "3.12" - + python-version: ${{ matrix.python-version }} + - name: Install the project run: uv sync --all-extras @@ -38,30 +39,7 @@ jobs: - name: Type-check with pyright run: uvx pyright --pythonpath .venv/bin/python api tests - test: - needs: lint - runs-on: ubuntu-latest - strategy: - matrix: - python-version: ["3.12"] - - steps: - - uses: actions/checkout@v4 - - - name: Install uv - uses: astral-sh/setup-uv@v4 - with: - version: "0.5.8" - - - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v5 - with: - python-version: "3.12" - - - name: Install the project - run: uv sync --all-extras - # The suite needs no database: migrations are skipped under pytest and the # DB session is mocked at the data-fetcher boundary (see CLAUDE.md). - name: Run tests - run: uv run pytest tests \ No newline at end of file + run: uv run pytest tests From 48d8eec657b1d42bf8878528e9588ee70013a5d5 Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Thu, 25 Jun 2026 13:45:16 -0400 Subject: [PATCH 10/18] Update CLAUDE.md --- CLAUDE.md | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index ee0eade..c2b1a35 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -30,6 +30,74 @@ Viewer/Member/Everyone Else * With express TDEI sign-up, the need for this access level diminishes greatly * Granted by Workspaces setting. +## What Each Role Can Do + +Project Lead +* Edit Metadata +* Edit Longform Quests +* Toggle App-Enabled Flag +* Delete Workspace +* Define User Teams +* Define Groups or Roles +* Export to TDEI +* Validate Changeset +* Move Workspace from Project Group to Project Group +* Edit POSM Element + +Validator +* Export to TDEI +* Validate Changeset +* Edit POSM Element + +Contributor +* Edit POSM Element + +Authenticated User With PG/Workspace Association +* Edit POSM Element + +### What this backend actually enforces (vs. the matrix above) + +The matrix above is the intended product model. It is only **partially** +enforced in `api/` — this service is a proxy in front of the OSM website, +cgimap, and TDEI, so several capabilities are enforced downstream (or not yet +at all). Validated against the code: + +**Enforced here, Lead-gated (`isWorkspaceLead` → 403).** POC inherits these +(POC on the owning project group satisfies `isWorkspaceLead`): + +| Capability | Endpoint | +|---|---| +| Edit Metadata | PATCH `/workspaces/{id}` | +| Edit Longform Quests | PATCH `/workspaces/{id}/quests/long/settings` | +| Toggle App-Enabled Flag | PATCH `/workspaces/{id}` (`externalAppAccess`) | +| Delete Workspace | DELETE `/workspaces/{id}` | +| Define User Teams | `/workspaces/{id}/teams...` (create/update/delete/members) | +| Define Groups or Roles | PUT/DELETE `/workspaces/{id}/users/{user_id}...` | + +**Not enforced / not present here:** + +* **Export to TDEI** — no endpoint exists in this backend. +* **Move Workspace PG→PG** — not possible; `WorkspacePatch` has no + `tdeiProjectGroupId` field, so no route can change a workspace's project group. +* **Validate Changeset** and **Edit POSM Element** — these go through the OSM + proxy catch-all (`api/main.py`), which gates *every* proxied operation on + `isWorkspaceContributor` alone. There is no Validator- or Lead-level check on + proxied traffic. + +**The Validator role grants nothing extra at this layer.** +`isWorkspaceValidator` exists in `api/core/security.py` but no endpoint +authorizes on it — it only appears in the `role` field of `WorkspaceResponse`. +A Validator and a Contributor have identical permissions in this backend. + +**"Contributor" and "Authenticated User With PG/Workspace Association" are the +same gate.** `isWorkspaceContributor` simply checks whether the workspace is in +one of the user's project groups (`accessibleWorkspaceIds`), i.e. PG/workspace +association — so both rows collapse to the same check. + +If the Validator/Lead distinctions for changeset validation and TDEI export are +required, they must be enforced downstream (`workspaces-openstreetmap-website/`, +`workspaces-cgimap/`) — that has not been audited here. + ## Testing Two layers, both fast and dependency-free (no Postgres, PostGIS, Docker, or From b40c7742eb6671366ac2c34eadd2fdec8abd8aa0 Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Fri, 26 Jun 2026 10:57:25 -0400 Subject: [PATCH 11/18] Rabbit --- api/core/jwt.py | 5 ++--- api/src/teams/schemas.py | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/api/core/jwt.py b/api/core/jwt.py index ab10f6a..9a5b594 100644 --- a/api/core/jwt.py +++ b/api/core/jwt.py @@ -6,11 +6,10 @@ _jwks_client: jwt.PyJWKClient | None = None # Test outline: -# @test: Test that this class validates JSON payloads properly against the JSON schema fetched -# @test: Test that malformed JSON or JSON that doesn't validate +# @test: Test that this class validates JSON payloads properly against the expected JWT/token format +# @test: Test that malformed JSON or JSON that doesn't validate doesn't succeed # @test: Test that any failed network requests are handled gracefully and return a proper error (not just "Exception") or worse a false positive validation - def _get_jwks_client() -> jwt.PyJWKClient: global _jwks_client diff --git a/api/src/teams/schemas.py b/api/src/teams/schemas.py index 361f2a4..f31ec8d 100644 --- a/api/src/teams/schemas.py +++ b/api/src/teams/schemas.py @@ -8,7 +8,7 @@ # @test: Test that this class matches the Alembic-defined database schema # @test: Test that the foreign key references are correct and that the relationships are correctly defined -# @test: Test that any values from Python and properly serialized to the database and that any values from the database are properly deserialized to Python +# @test: Test that any values from Python are properly serialized to the database and that any values from the database are properly deserialized to Python # @test: Test that any serialization/deserialization doesn't lose any precision or data class WorkspaceTeamUser(SQLModel, table=True): """Team to User link table""" @@ -27,7 +27,7 @@ class WorkspaceTeamBase(SQLModel): # @test: Test that this class matches the Alembic-defined database schema # @test: Test that the foreign key references are correct and that the relationships are correctly defined -# @test: Test that any values from Python and properly serialized to the database and that any values from the database are properly deserialized to Python +# @test: Test that any values from Python are properly serialized to the database and that any values from the database are properly deserialized to Python # @test: Test that any serialization/deserialization doesn't lose any precision or data class WorkspaceTeam(WorkspaceTeamBase, table=True): """Workspace teams""" From b77e12941886d2d59b6e2d7a88c442b8e6c0818a Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Fri, 26 Jun 2026 11:00:14 -0400 Subject: [PATCH 12/18] Update jwt.py --- api/core/jwt.py | 1 + 1 file changed, 1 insertion(+) diff --git a/api/core/jwt.py b/api/core/jwt.py index 9a5b594..efb536b 100644 --- a/api/core/jwt.py +++ b/api/core/jwt.py @@ -10,6 +10,7 @@ # @test: Test that malformed JSON or JSON that doesn't validate doesn't succeed # @test: Test that any failed network requests are handled gracefully and return a proper error (not just "Exception") or worse a false positive validation + def _get_jwks_client() -> jwt.PyJWKClient: global _jwks_client From ee404604a8d5d34d612830467715f1509f27487c Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Fri, 26 Jun 2026 11:02:00 -0400 Subject: [PATCH 13/18] Linter setup --- .pre-commit-config.yaml | 2 +- .vscode/settings.json | 13 ++++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 47d11bc..95f6511 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -12,7 +12,7 @@ repos: name: isort (python) - repo: https://github.com/psf/black - rev: 24.1.1 + rev: 24.10.0 hooks: - id: black language_version: python3.12 diff --git a/.vscode/settings.json b/.vscode/settings.json index 5f626e0..f8efae9 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -3,5 +3,16 @@ "alembic" ], "python.testing.unittestEnabled": false, - "python.testing.pytestEnabled": true + "python.testing.pytestEnabled": true, + "[python]": { + "editor.defaultFormatter": "ms-python.black-formatter", + "editor.formatOnSave": true, + "editor.codeActionsOnSave": { + "source.organizeImports": "explicit" + } + }, + "isort.args": [ + "--profile", + "black" + ] } \ No newline at end of file From b810841a664fb2eceebcceb84c564a23db6451d5 Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Fri, 26 Jun 2026 11:02:04 -0400 Subject: [PATCH 14/18] Update schemas.py --- api/src/teams/schemas.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/api/src/teams/schemas.py b/api/src/teams/schemas.py index f31ec8d..3a625ba 100644 --- a/api/src/teams/schemas.py +++ b/api/src/teams/schemas.py @@ -58,10 +58,6 @@ def from_team(cls, team: WorkspaceTeam) -> Self: class WorkspaceTeamCreate(WorkspaceTeamBase): """New workspace team DTO""" - pass - class WorkspaceTeamUpdate(WorkspaceTeamBase): """Modify workspace team DTO""" - - pass From 2c3e2df51810bb436010f9ac4d6628ff4252ffef Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Fri, 26 Jun 2026 12:18:44 -0400 Subject: [PATCH 15/18] Update ci.yml --- .github/workflows/ci.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6f914c6..113eda6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,6 +6,11 @@ on: pull_request: branches: [ main ] +# Cancel any in-progress run for the same branch/PR when a newer commit lands. +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + jobs: ci: runs-on: ubuntu-latest From 5e4cabe7a52033b0d1713e35513442faaff1e962 Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Fri, 26 Jun 2026 12:20:46 -0400 Subject: [PATCH 16/18] Update ci.yml --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 113eda6..5050c8c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -19,10 +19,10 @@ jobs: python-version: ["3.12"] steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - name: Install uv - uses: astral-sh/setup-uv@v4 + uses: astral-sh/setup-uv@v6 with: version: "0.5.8" enable-cache: true From 0f1d5068e4918ddbb490ee0017a7ea1b72662e2d Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Fri, 26 Jun 2026 12:20:56 -0400 Subject: [PATCH 17/18] Update ci.yml --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5050c8c..f585636 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,7 +28,7 @@ jobs: enable-cache: true - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v5 + uses: actions/setup-python@v6 with: python-version: ${{ matrix.python-version }} From e0628afdb44f223476b4184ae63b716dbd74aa64 Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Fri, 26 Jun 2026 15:58:15 -0400 Subject: [PATCH 18/18] Update main.py --- api/main.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/api/main.py b/api/main.py index 11fd7a0..b8618ff 100644 --- a/api/main.py +++ b/api/main.py @@ -31,9 +31,11 @@ sentry_sdk.init( dsn=config.settings.SENTRY_DSN, environment=os.getenv("ENV", "unknown"), + release=os.getenv("CODE_VERSION", "unknown"), debug=settings.DEBUG, ) +# Kept alongside `release` for any dashboards that query the `version` tag. sentry_sdk.set_tag("version", os.getenv("CODE_VERSION", "unknown")) # Set up logging configuration