From cfa643e1050b914ee9d08e31bac2756b68dbcdc8 Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Thu, 9 Jul 2026 11:16:04 -0400 Subject: [PATCH 01/34] Defensive check on changeset resolve plus tests --- CLAUDE.md | 38 ++++++++--- api/src/osm/repository.py | 17 ++++- api/src/osm/routes.py | 2 +- tests/integration/test_osm.py | 117 ++++++++++++++++++++++++++++++++++ 4 files changed, 161 insertions(+), 13 deletions(-) create mode 100644 tests/integration/test_osm.py diff --git a/CLAUDE.md b/CLAUDE.md index 8e25c07..1e5e9d6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -79,24 +79,42 @@ at all). Validated against the code: * **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 +* **Edit POSM Element** — goes 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. + proxied traffic. Raw changeset commits (proxied `PUT /api/0.6/changeset/...`) + are likewise Contributor-gated; the proxy only *tags* a contributor's new + changeset with `review_requested=yes` when the workspace has `autoFlagReview` + set — it does not enforce validation. -**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. +**Enforced here, Validator-gated (`isWorkspaceLead || isWorkspaceValidator` → +403).** This is a native FastAPI route, not proxied traffic. Leads (and POC via +`isWorkspaceLead`) inherit it: + +| Capability | Endpoint | +|---|---| +| Resolve/Validate Changeset | PUT `/workspaces/{id}/changesets/{changeset_id}/resolve` | + +Resolving clears the `review_requested` tag and stamps `reviewed_by` with the +reviewer's UUID. The gate is enforced both in the route +(`api/src/osm/routes.py`) and, defensively, inside +`OSMRepository.resolveChangeset` (`api/src/osm/repository.py`). + +**The Validator role grants exactly one thing at this layer:** the ability to +resolve changesets via the endpoint above. Aside from that, a Validator and a +Contributor have identical permissions in this backend. `isWorkspaceValidator` +otherwise only appears in the `role` field of `WorkspaceResponse`. **"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. +Changeset *resolution* is Validator/Lead-gated here (see above). But the +Validator/Lead distinction on raw changeset *commits* and TDEI export is not +enforced at this layer; if required, it must be enforced downstream +(`workspaces-openstreetmap-website/`, `workspaces-cgimap/`) — that has not been +audited here. ## Testing diff --git a/api/src/osm/repository.py b/api/src/osm/repository.py index 5ca1c9a..5a29b7e 100644 --- a/api/src/osm/repository.py +++ b/api/src/osm/repository.py @@ -1,7 +1,8 @@ from sqlalchemy import text from sqlmodel.ext.asyncio.session import AsyncSession -from api.core.exceptions import NotFoundException +from api.core.exceptions import ForbiddenException, NotFoundException +from api.core.security import UserInfo class OSMRepository: @@ -50,10 +51,22 @@ async def getChangesetAdiff(self, workspace_id: int, changeset_id: int) -> list: async def resolveChangeset( self, + current_user: UserInfo, workspace_id: int, changeset_id: int, - reviewer_uuid: str, ) -> None: + # Defense in depth: resolving a changeset is a validator/lead + # capability. The route also enforces this, but gate here too so the + # repository cannot be misused from another call site. + if not current_user.isWorkspaceLead( + workspace_id + ) and not current_user.isWorkspaceValidator(workspace_id): + raise ForbiddenException( + "Only workspace leads and validators can resolve changesets" + ) + + reviewer_uuid = str(current_user.user_uuid) + await self.session.execute( text(f"SET search_path TO 'workspace-{int(workspace_id)}', public") ) diff --git a/api/src/osm/routes.py b/api/src/osm/routes.py index 452b2d4..9542e5a 100644 --- a/api/src/osm/routes.py +++ b/api/src/osm/routes.py @@ -72,7 +72,7 @@ async def resolve_changeset( try: await repository_osm.resolveChangeset( - workspace_id, changeset_id, str(current_user.user_uuid) + current_user, workspace_id, changeset_id ) except Exception as e: logger.error( diff --git a/tests/integration/test_osm.py b/tests/integration/test_osm.py new file mode 100644 index 0000000..b4862d8 --- /dev/null +++ b/tests/integration/test_osm.py @@ -0,0 +1,117 @@ +"""Integration tests for the /workspaces OSM routes (api/src/osm/routes.py). + +Each test drives a real HTTP request through the real route + repository, +queueing simulated rows on the fake sessions. + +Focus: PUT /{id}/changesets/{cid}/resolve is gated to workspace leads and +validators (403 otherwise). The gate is enforced both in the route and, +defensively, inside ``OSMRepository.resolveChangeset`` -- so a contributor is +rejected before any DB work, and the repository would reject a bad call site +even if the route check were bypassed. +""" + +import pytest + +from api.src.users.schemas import WorkspaceUserRoleType +from tests.support import factories, fakes + +API = "/api/v1/workspaces" + + +def _resolve_url(workspace_id=1, changeset_id=99): + return f"{API}/{workspace_id}/changesets/{changeset_id}/resolve" + + +def _user_with_role(role, workspace_id=1): + return factories.make_user_info(osm_workspace_roles={workspace_id: [role]}) + + +# === PUT /{id}/changesets/{cid}/resolve ==================================== + + +async def test_resolve_changeset_validator_204( + client, login, task_session, osm_session +): + login(_user_with_role(WorkspaceUserRoleType.VALIDATOR)) + task_session.queue(fakes.rows(factories.make_workspace(id=1))) # getById + osm_session.queue(fakes.affected(1), fakes.affected(1)) # DELETE, INSERT + + response = await client.put(_resolve_url()) + + assert response.status_code == 204 + assert osm_session.commits == 1 # resolveChangeset ran to completion + + +async def test_resolve_changeset_lead_204(client, login, task_session, osm_session): + login(_user_with_role(WorkspaceUserRoleType.LEAD)) + task_session.queue(fakes.rows(factories.make_workspace(id=1))) + osm_session.queue(fakes.affected(1), fakes.affected(1)) + + response = await client.put(_resolve_url()) + + assert response.status_code == 204 + assert osm_session.commits == 1 + + +async def test_resolve_changeset_poc_204(client, login, task_session, osm_session): + # POC on the owning project group satisfies isWorkspaceLead. + login( + factories.make_user_info( + accessible_workspace_ids={factories.DEFAULT_PG_ID: [1]}, + poc_group_ids=(factories.DEFAULT_PG_ID,), + ) + ) + task_session.queue(fakes.rows(factories.make_workspace(id=1))) + osm_session.queue(fakes.affected(1), fakes.affected(1)) + + response = await client.put(_resolve_url()) + + assert response.status_code == 204 + assert osm_session.commits == 1 + + +async def test_resolve_changeset_contributor_403( + client, login, task_session, osm_session +): + # A contributor (PG association, no validator/lead grant) is rejected + # before any DB work -- neither session should be touched. + login( + factories.make_user_info( + accessible_workspace_ids={factories.DEFAULT_PG_ID: [1]} + ) + ) + + response = await client.put(_resolve_url()) + + assert response.status_code == 403 + assert osm_session.commits == 0 + + +async def test_resolve_changeset_no_access_403(client, login): + # A user with no association to the workspace is likewise forbidden. + login() + + response = await client.put(_resolve_url()) + + assert response.status_code == 403 + + +async def test_resolve_changeset_validator_of_other_workspace_403(client, login): + # Validator rights on workspace 2 do not authorize resolving on workspace 1. + login(_user_with_role(WorkspaceUserRoleType.VALIDATOR, workspace_id=2)) + + response = await client.put(_resolve_url(workspace_id=1)) + + assert response.status_code == 403 + + +async def test_resolve_changeset_unexpected_error_500( + error_client, login, task_session, osm_session +): + login(_user_with_role(WorkspaceUserRoleType.VALIDATOR)) + task_session.queue(fakes.rows(factories.make_workspace(id=1))) + osm_session.queue(fakes.raises(RuntimeError("db"))) # DELETE blows up + + response = await error_client.put(_resolve_url()) + + assert response.status_code == 500 From a5191fe9199f3c9a31e72cb50808a392d8b97885 Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Thu, 9 Jul 2026 11:26:53 -0400 Subject: [PATCH 02/34] Update routes.py --- api/src/osm/routes.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/api/src/osm/routes.py b/api/src/osm/routes.py index 9542e5a..cadee1e 100644 --- a/api/src/osm/routes.py +++ b/api/src/osm/routes.py @@ -71,9 +71,7 @@ async def resolve_changeset( await repository_ws.getById(current_user, workspace_id) try: - await repository_osm.resolveChangeset( - current_user, workspace_id, changeset_id - ) + await repository_osm.resolveChangeset(current_user, workspace_id, changeset_id) except Exception as e: logger.error( f"Failed to resolve changeset {changeset_id}" From c87a118ebdbbc655184af6c736fcb8a88bc2439d Mon Sep 17 00:00:00 2001 From: Naresh Kumar D Date: Fri, 10 Jul 2026 15:43:55 +0530 Subject: [PATCH 03/34] Deploy code feature Deploying the required code --- .github/workflows/trigger-deploy-on-merge.yml | 36 +++++++++++++++++++ api/core/config.py | 8 ++--- api/main.py | 4 ++- 3 files changed, 42 insertions(+), 6 deletions(-) create mode 100644 .github/workflows/trigger-deploy-on-merge.yml diff --git a/.github/workflows/trigger-deploy-on-merge.yml b/.github/workflows/trigger-deploy-on-merge.yml new file mode 100644 index 0000000..82265c8 --- /dev/null +++ b/.github/workflows/trigger-deploy-on-merge.yml @@ -0,0 +1,36 @@ +name: Trigger Deployment on Pull Request Merge +on: + pull_request: + types: [closed] + branches: [ develop, staging, production] +permissions: + contents: read + actions: write +jobs: + on-merge: + if: github.event.pull_request.merged == true + runs-on: ubuntu-latest + outputs: + environment: ${{ steps.set-environment.outputs.environment }} + steps: + - name: Set Environment + id: set-environment + run: | + case "${{ github.base_ref }}" in + develop) echo "environment=develop" >> $GITHUB_OUTPUT ;; + staging) echo "environment=staging" >> $GITHUB_OUTPUT ;; + production) echo "environment=production" >> $GITHUB_OUTPUT ;; + testing) echo "environment=testing" >> $GITHUB_OUTPUT ;; + esac + deploy: + needs: on-merge + runs-on: ubuntu-latest + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + steps: + - name: Trigger Deployment Workflow + run: | + gh workflow run push-docker-image.yml \ + --repo ${{ github.repository }} \ + --ref ${{ github.base_ref }} \ + --field environment=${{ needs.on-merge.outputs.environment }} \ No newline at end of file diff --git a/api/core/config.py b/api/core/config.py index c4c6bde..643e586 100644 --- a/api/core/config.py +++ b/api/core/config.py @@ -10,11 +10,9 @@ class Settings(BaseSettings): PROJECT_NAME: str = "Workspaces API" - # JSON array of allowed CORS origins. For example: - # - # ["https://workspaces.example.com", "https://leaderboard.example.com"] - # - CORS_ORIGINS: list[str] = [] + # Comma separated list of origins, or "*" to allow all origins. Defaults to an empty list. + # Example: CORS_ORIGINS="https://workspaces.example.com,https://leaderboard.example.com" + CORS_ORIGINS: str = "" TASK_DATABASE_URL: str = ( "postgresql+asyncpg://user:pass@localhost:5432/tasking_manager" diff --git a/api/main.py b/api/main.py index d537b72..9123a4e 100644 --- a/api/main.py +++ b/api/main.py @@ -97,7 +97,9 @@ async def lifespan(_app: FastAPI): app.add_middleware( CORSMiddleware, - allow_origins=settings.CORS_ORIGINS, + allow_origins=[ + origin.strip() for origin in settings.CORS_ORIGINS.split(",") if origin.strip() + ], allow_credentials=True, allow_methods=["*"], allow_headers=["*"], From 090c4bc747e98a1c378a6682effff4dbb4ea1cfc Mon Sep 17 00:00:00 2001 From: Naresh Kumar D Date: Fri, 10 Jul 2026 15:49:29 +0530 Subject: [PATCH 04/34] Update test_config.py Updated unit tests for CORS --- tests/unit/test_config.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 3a06d3b..4a9ab45 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -14,7 +14,7 @@ def test_defaults_loaded_when_env_unset(): 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.CORS_ORIGINS == "" assert s.DEBUG is False assert s.SENTRY_DSN == "" assert s.WS_OSM_HOST == "http://osm-web" @@ -38,18 +38,18 @@ def test_env_vars_override_members(monkeypatch): def test_cors_origins_parsed_from_json_env(monkeypatch): - monkeypatch.setenv("CORS_ORIGINS", '["https://a.example", "https://b.example"]') + 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"] + 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.CORS_ORIGINS, str) assert isinstance(s.DEBUG, bool) # empty-string default is preserved (not coerced to None): assert s.SENTRY_DSN == "" From 0070a2f28eb1b752bc43b18ad766ddf5a99217a4 Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Fri, 10 Jul 2026 15:11:17 -0400 Subject: [PATCH 05/34] Enable osm-web API endpoints with auth token --- api/core/config.py | 15 +++ api/core/security.py | 139 ++++++++++++++++++++++ tests/unit/test_token_bridge.py | 197 ++++++++++++++++++++++++++++++++ 3 files changed, 351 insertions(+) create mode 100644 tests/unit/test_token_bridge.py diff --git a/api/core/config.py b/api/core/config.py index 643e586..7911264 100644 --- a/api/core/config.py +++ b/api/core/config.py @@ -38,6 +38,21 @@ class Settings(BaseSettings): # proxy destination--"osm-web" is a virtual docker network endpoint WS_OSM_HOST: str = "http://osm-web" + # OSM token bridge: when a TDEI token is validated, mirror it into the OSM + # database's `oauth_access_tokens` table so osm-rails (doorkeeper) and cgimap + # authenticate it via their standard OAuth2 path -- no custom JWT handling + # needed in those services. Disabled while WS_OSM_OAUTH_APPLICATION_ID is 0. + # Set it to the id of the doorkeeper `oauth_applications` row these tokens + # should belong to (create one via the OSM `register_apps` rake task or SQL). + WS_OSM_OAUTH_APPLICATION_ID: int = 0 + + # Scopes granted to the mirrored token. Must cover the OSM API operations the + # frontend performs; see `lib/oauth.rb` in the OSM website for valid values. + WS_OSM_OAUTH_SCOPES: str = ( + "read_prefs write_prefs write_api write_changeset_comments " + "read_gpx write_gpx write_notes" + ) + SENTRY_DSN: str = "" model_config = SettingsConfigDict( diff --git a/api/core/security.py b/api/core/security.py index 0ff32e2..1db25f9 100644 --- a/api/core/security.py +++ b/api/core/security.py @@ -1,3 +1,4 @@ +import time from enum import StrEnum from uuid import UUID @@ -19,6 +20,8 @@ # @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 +# @test: Test that a validated token is mirrored into the OSM oauth_access_tokens table (with the user provisioned and expires_in from the JWT exp) when WS_OSM_OAUTH_APPLICATION_ID is set, and is a no-op otherwise +# @test: Test that re-presenting a token reactivates its OSM row (revoked_at cleared, expiry refreshed) and that a rotated (superseded) token is revoked, both gated on WS_OSM_OAUTH_APPLICATION_ID # Set up logger for this module logger = get_logger(__name__) @@ -316,6 +319,14 @@ async def validate_token( logger.info("Token validation cache hit") return cached logger.info("Token validation cache miss: token rotated") + # The old token is superseded; revoke its OSM row so it stops + # authenticating against osm-rails/cgimap before its own exp. + # TODO: this assumes a single active token per user. Truly concurrent + # sessions (multiple valid jtis for one user) will flap -- each request + # revokes the other session's OSM token and reactivates its own via the + # DO UPDATE upsert. To support multi-session, track multiple active jtis + # per user (cache + revoke set) instead of a single cached token. + await _revoke_osm_token(osm_db_session, cached.credentials) del _user_info_cache[user_uuid] # Cache miss: fetch TDEI roles and DB data: @@ -327,6 +338,123 @@ async def validate_token( return user_info +async def _bridge_token_to_osm( + session: AsyncSession, + *, + user_uuid: UUID, + user_name: str, + email: str | None, + token: str, + exp: int | None, +) -> None: + """Mirror a validated TDEI token into the OSM database so osm-rails + (doorkeeper) and cgimap authenticate it through their standard OAuth2 path, + with no custom JWT handling required in those services. + + Two idempotent writes: provision the OSM ``users`` row (owns the token via + ``resource_owner_id``) and insert a plaintext ``oauth_access_tokens`` row. + Doorkeeper stores tokens plaintext (``SecretStoring::Plain``), so the raw JWT + matches on lookup for both osm-rails and cgimap; ``expires_in`` tracks the + JWT's own ``exp`` so OSM expires it in lockstep with TDEI. + + No-op unless ``WS_OSM_OAUTH_APPLICATION_ID`` is set. Best-effort: a failure + here must not break token validation -- the ``/api/v1`` routes keep working; + only the proxied OSM calls would 401 until the row exists. + """ + if settings.WS_OSM_OAUTH_APPLICATION_ID <= 0: + return + + auth_uid = str(user_uuid) + try: + # Provision the users row (same shape as _provision_users_from_tdei). + await session.execute( + text( + "INSERT INTO users (auth_uid, email, display_name, auth_provider, " + "status, pass_crypt, data_public, email_valid, terms_seen, " + "creation_time, terms_agreed, tou_agreed) " + "VALUES (:auth_uid, :email, :name, 'TDEI', 'active', 'none', " + "true, true, true, (now() at time zone 'utc'), " + "(now() at time zone 'utc'), (now() at time zone 'utc')) " + "ON CONFLICT (auth_uid) DO NOTHING" + ), + {"auth_uid": auth_uid, "email": email, "name": user_name}, + ) + row = ( + await session.execute( + text( + "SELECT id FROM users " + "WHERE auth_provider = 'TDEI' AND auth_uid = :auth_uid" + ), + {"auth_uid": auth_uid}, + ) + ).first() + if row is None: + return + user_id = row[0] + + expires_in = max(0, exp - int(time.time())) if exp else None + await session.execute( + text( + "INSERT INTO oauth_access_tokens (application_id, " + "resource_owner_id, token, scopes, created_at, expires_in) " + "VALUES (:app_id, :user_id, :token, :scopes, " + "(now() at time zone 'utc'), :expires_in) " + # Re-presenting a token reactivates it: refresh the expiry to + # track this validation and clear any prior revocation, so a + # token revoked on rotation heals if it is used again. + "ON CONFLICT (token) DO UPDATE SET " + "resource_owner_id = EXCLUDED.resource_owner_id, " + "scopes = EXCLUDED.scopes, " + "created_at = EXCLUDED.created_at, " + "expires_in = EXCLUDED.expires_in, " + "revoked_at = NULL" + ), + { + "app_id": settings.WS_OSM_OAUTH_APPLICATION_ID, + "user_id": user_id, + "token": token, + "scopes": settings.WS_OSM_OAUTH_SCOPES, + "expires_in": expires_in, + }, + ) + await session.commit() + except Exception as e: + # Never fail auth on a bridge error; the OSM row just won't exist yet. + logger.warning( + "Failed to bridge TDEI token into OSM oauth_access_tokens: %s", e + ) + await session.rollback() + + +async def _revoke_osm_token(session: AsyncSession, token: str) -> None: + """Revoke a superseded token's OSM ``oauth_access_tokens`` row (best-effort). + + Called when a user's token rotates (new ``jti``) so the previous JWT stops + authenticating against osm-rails/cgimap before its own ``exp`` rather than + lingering until it expires. No-op unless the bridge is configured. + + Note: OSM/cgimap are reachable only *through* this proxy, and every request + first passes ``validate_token`` -- so a TDEI-revoked token is already + rejected upstream. This revocation is defense-in-depth for the superseded + token and keeps the OSM token store tidy. + """ + if settings.WS_OSM_OAUTH_APPLICATION_ID <= 0: + return + try: + await session.execute( + text( + "UPDATE oauth_access_tokens " + "SET revoked_at = (now() at time zone 'utc') " + "WHERE token = :token AND revoked_at IS NULL" + ), + {"token": token}, + ) + await session.commit() + except Exception as e: + logger.warning("Failed to revoke superseded OSM token: %s", e) + await session.rollback() + + async def _validate_token_uncached( token: str, user_uuid: UUID, @@ -427,6 +555,17 @@ async def _validate_token_uncached( osmRoles[i["workspace_id"]].append(i["role"]) r.osmWorkspaceRoles = osmRoles + # Mirror this validated token into the OSM DB so proxied OSM/cgimap calls + # authenticate via the standard OAuth2 path. No-op unless configured. + await _bridge_token_to_osm( + osm_db_session, + user_uuid=user_uuid, + user_name=r.user_name, + email=payload.get("email"), + token=token, + exp=payload.get("exp"), + ) + logger.info("Finished validation of token") return r diff --git a/tests/unit/test_token_bridge.py b/tests/unit/test_token_bridge.py new file mode 100644 index 0000000..401ad73 --- /dev/null +++ b/tests/unit/test_token_bridge.py @@ -0,0 +1,197 @@ +"""Tests for the OSM token bridge in ``api/core/security.py``. + +``_bridge_token_to_osm`` mirrors a validated TDEI token into the OSM database's +``oauth_access_tokens`` table so osm-rails (doorkeeper) and cgimap authenticate +it through their standard OAuth2 path. We cover: + +- it is a no-op unless ``WS_OSM_OAUTH_APPLICATION_ID`` is configured, +- when enabled it provisions the ``users`` row and inserts a plaintext + ``oauth_access_tokens`` row owned by that user, with ``expires_in`` derived + from the JWT ``exp``, +- a DB failure never propagates (auth must not break) and rolls back. +""" + +import time +from typing import cast +from uuid import uuid4 + +from sqlmodel.ext.asyncio.session import AsyncSession + +import api.core.security as security +from api.core.config import settings + + +class RecordingSession: + """Async session stand-in that records (sql, params) and returns a user id + row for the ``SELECT id FROM users`` lookup.""" + + def __init__(self, user_id=99): + self.calls: list[tuple[str, dict]] = [] + self.commits = 0 + self.rollbacks = 0 + self._user_id = user_id + + async def execute(self, statement, params=None): + sql = str(statement) + self.calls.append((sql, params or {})) + + class _R: + def __init__(self, rows): + self._rows = rows + + def first(self): + return self._rows[0] if self._rows else None + + if "SELECT id FROM users" in sql: + return _R([(self._user_id,)]) + return _R([]) + + async def commit(self): + self.commits += 1 + + async def rollback(self): + self.rollbacks += 1 + + def sql_for(self, needle): + return [c for c in self.calls if needle in c[0]] + + +async def test_bridge_is_noop_when_disabled(monkeypatch): + """With WS_OSM_OAUTH_APPLICATION_ID == 0 the bridge touches no DB.""" + monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 0) + session = RecordingSession() + + await security._bridge_token_to_osm( + cast(AsyncSession, session), + user_uuid=uuid4(), + user_name="alice", + email="alice@example.com", + token="tok", + exp=None, + ) + + assert session.calls == [] + assert session.commits == 0 + + +async def test_bridge_provisions_user_and_mirrors_token(monkeypatch): + """When enabled: users upsert -> id lookup -> oauth_access_tokens upsert.""" + monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 7) + monkeypatch.setattr(settings, "WS_OSM_OAUTH_SCOPES", "write_api read_prefs") + uid = uuid4() + exp = int(time.time()) + 3600 + session = RecordingSession(user_id=42) + + await security._bridge_token_to_osm( + cast(AsyncSession, session), + user_uuid=uid, + user_name="alice", + email="alice@example.com", + token="jwt-token", + exp=exp, + ) + + # users row provisioned idempotently, keyed by auth_uid = str(sub) + user_inserts = session.sql_for("INSERT INTO users") + assert user_inserts and "ON CONFLICT (auth_uid)" in user_inserts[0][0] + assert user_inserts[0][1]["auth_uid"] == str(uid) + assert user_inserts[0][1]["email"] == "alice@example.com" + + # token mirrored with the configured application/scopes and exp-derived TTL + token_inserts = session.sql_for("oauth_access_tokens") + assert token_inserts and "ON CONFLICT (token)" in token_inserts[0][0] + # Re-presenting a token must reactivate it (clear revocation, refresh expiry). + assert "DO UPDATE" in token_inserts[0][0] + assert "revoked_at = NULL" in token_inserts[0][0] + params = token_inserts[0][1] + assert params["app_id"] == 7 + assert params["user_id"] == 42 + assert params["token"] == "jwt-token" + assert params["scopes"] == "write_api read_prefs" + assert 3590 <= params["expires_in"] <= 3600 + + assert session.commits == 1 + assert session.rollbacks == 0 + + +async def test_bridge_expires_in_none_without_exp(monkeypatch): + """A token without an `exp` claim mirrors with a NULL expires_in.""" + monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 7) + session = RecordingSession() + + await security._bridge_token_to_osm( + cast(AsyncSession, session), + user_uuid=uuid4(), + user_name="alice", + email=None, + token="jwt-token", + exp=None, + ) + + params = session.sql_for("oauth_access_tokens")[0][1] + assert params["expires_in"] is None + + +async def test_bridge_is_best_effort_on_db_error(monkeypatch): + """A DB failure must not propagate out of validation; it rolls back.""" + monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 7) + + class BoomSession(RecordingSession): + async def execute(self, statement, params=None): + raise RuntimeError("db down") + + session = BoomSession() + + # Must not raise. + await security._bridge_token_to_osm( + cast(AsyncSession, session), + user_uuid=uuid4(), + user_name="alice", + email=None, + token="tok", + exp=None, + ) + + assert session.rollbacks == 1 + assert session.commits == 0 + + +async def test_revoke_is_noop_when_disabled(monkeypatch): + """With the bridge off, revoking a rotated token touches no DB.""" + monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 0) + session = RecordingSession() + + await security._revoke_osm_token(cast(AsyncSession, session), "old-token") + + assert session.calls == [] + assert session.commits == 0 + + +async def test_revoke_marks_only_the_superseded_token(monkeypatch): + """Revocation flips revoked_at for exactly the given token, if not already.""" + monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 7) + session = RecordingSession() + + await security._revoke_osm_token(cast(AsyncSession, session), "old-token") + + updates = session.sql_for("UPDATE oauth_access_tokens") + assert updates and "revoked_at" in updates[0][0] + assert "WHERE token = :token AND revoked_at IS NULL" in updates[0][0] + assert updates[0][1]["token"] == "old-token" + assert session.commits == 1 + + +async def test_revoke_is_best_effort_on_db_error(monkeypatch): + """A DB failure during revocation must not propagate; it rolls back.""" + monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 7) + + class BoomSession(RecordingSession): + async def execute(self, statement, params=None): + raise RuntimeError("db down") + + session = BoomSession() + + await security._revoke_osm_token(cast(AsyncSession, session), "tok") + + assert session.rollbacks == 1 + assert session.commits == 0 From 90f8a999c1f52408500ccf114403e8b82a74590f Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Fri, 10 Jul 2026 15:48:01 -0400 Subject: [PATCH 06/34] CORS config file fix --- api/core/config.py | 30 ++++++++++++++++++++++++++++-- api/main.py | 4 +--- tests/unit/test_config.py | 37 +++++++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 5 deletions(-) diff --git a/api/core/config.py b/api/core/config.py index 7911264..dbda4b3 100644 --- a/api/core/config.py +++ b/api/core/config.py @@ -1,3 +1,5 @@ +import json + from pydantic_settings import BaseSettings, SettingsConfigDict @@ -10,8 +12,12 @@ class Settings(BaseSettings): PROJECT_NAME: str = "Workspaces API" - # Comma separated list of origins, or "*" to allow all origins. Defaults to an empty list. - # Example: CORS_ORIGINS="https://workspaces.example.com,https://leaderboard.example.com" + # Allowed CORS origins. Accepts either a comma-separated list OR a JSON + # array (the deployment stack historically supplies a JSON array), plus "*" + # for all origins. Read via `cors_origins_list`, not this raw string. + # Examples: + # CORS_ORIGINS="https://workspaces.example.com,https://leaderboard.example.com" + # CORS_ORIGINS='["https://workspaces.example.com"]' CORS_ORIGINS: str = "" TASK_DATABASE_URL: str = ( @@ -55,6 +61,26 @@ class Settings(BaseSettings): SENTRY_DSN: str = "" + @property + def cors_origins_list(self) -> list[str]: + """Allowed CORS origins as a list. + + Tolerates both formats the deployment has used: a JSON array + (``["https://a","https://b"]``) and a comma-separated string + (``https://a,https://b``). ``"*"`` is passed through as-is. + """ + raw = self.CORS_ORIGINS.strip() + if not raw: + return [] + if raw.startswith("["): + try: + parsed = json.loads(raw) + except json.JSONDecodeError: + parsed = None + if isinstance(parsed, list): + return [str(o).strip() for o in parsed if str(o).strip()] + return [o.strip() for o in raw.split(",") if o.strip()] + model_config = SettingsConfigDict( env_file=".env", env_file_encoding="utf-8", diff --git a/api/main.py b/api/main.py index 9123a4e..c3708b3 100644 --- a/api/main.py +++ b/api/main.py @@ -97,9 +97,7 @@ async def lifespan(_app: FastAPI): app.add_middleware( CORSMiddleware, - allow_origins=[ - origin.strip() for origin in settings.CORS_ORIGINS.split(",") if origin.strip() - ], + allow_origins=settings.cors_origins_list, allow_credentials=True, allow_methods=["*"], allow_headers=["*"], diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 4a9ab45..cc96fa0 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -45,6 +45,43 @@ def test_cors_origins_parsed_from_json_env(monkeypatch): assert s.CORS_ORIGINS == "https://a.example,https://b.example" +def test_cors_origins_list_comma_separated(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_list == ["https://a.example", "https://b.example"] + + +def test_cors_origins_list_json_array(monkeypatch): + # The deployment stack supplies a JSON array; it must parse, not become one + # malformed bracketed origin. + 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_list == ["https://a.example", "https://b.example"] + + +def test_cors_origins_list_single_json_array(monkeypatch): + monkeypatch.setenv("CORS_ORIGINS", '["https://only.example"]') + s = Settings(_env_file=None) # type: ignore[call-arg] # pydantic-settings init kwarg + assert s.cors_origins_list == ["https://only.example"] + + +def test_cors_origins_list_empty_and_wildcard(monkeypatch): + s = Settings(_env_file=None) # type: ignore[call-arg] # pydantic-settings init kwarg + assert s.cors_origins_list == [] + + monkeypatch.setenv("CORS_ORIGINS", "*") + s = Settings(_env_file=None) # type: ignore[call-arg] # pydantic-settings init kwarg + assert s.cors_origins_list == ["*"] + + +def test_cors_origins_list_malformed_json_falls_back_to_comma(monkeypatch): + # A value that starts with "[" but isn't valid JSON should not crash; fall + # back to comma-splitting rather than raising. + monkeypatch.setenv("CORS_ORIGINS", "[not json") + s = Settings(_env_file=None) # type: ignore[call-arg] # pydantic-settings init kwarg + assert s.cors_origins_list == ["[not json"] + + def test_value_types_and_formats(): s = Settings(_env_file=None) # type: ignore[call-arg] # pydantic-settings init kwarg From cd4cd645de338dda1dd8708f81d94c201434eb74 Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Fri, 10 Jul 2026 16:21:49 -0400 Subject: [PATCH 07/34] Tweak to enable OSM token bridge by default --- api/core/config.py | 32 +++++--- api/core/security.py | 137 +++++++++++++++++++++++--------- tests/unit/test_token_bridge.py | 98 ++++++++++++++--------- 3 files changed, 181 insertions(+), 86 deletions(-) diff --git a/api/core/config.py b/api/core/config.py index 7911264..50a860b 100644 --- a/api/core/config.py +++ b/api/core/config.py @@ -38,21 +38,33 @@ class Settings(BaseSettings): # proxy destination--"osm-web" is a virtual docker network endpoint WS_OSM_HOST: str = "http://osm-web" - # OSM token bridge: when a TDEI token is validated, mirror it into the OSM - # database's `oauth_access_tokens` table so osm-rails (doorkeeper) and cgimap - # authenticate it via their standard OAuth2 path -- no custom JWT handling - # needed in those services. Disabled while WS_OSM_OAUTH_APPLICATION_ID is 0. - # Set it to the id of the doorkeeper `oauth_applications` row these tokens - # should belong to (create one via the OSM `register_apps` rake task or SQL). - WS_OSM_OAUTH_APPLICATION_ID: int = 0 - - # Scopes granted to the mirrored token. Must cover the OSM API operations the - # frontend performs; see `lib/oauth.rb` in the OSM website for valid values. + # OSM token bridge: when enabled, a validated TDEI token is mirrored into the + # OSM database's `oauth_access_tokens` table so osm-rails (doorkeeper) and + # cgimap authenticate it via their standard OAuth2 path -- no custom JWT + # handling needed in those services. The backend also auto-creates the + # doorkeeper `oauth_applications` row (keyed by WS_OSM_OAUTH_CLIENT_UID and + # owned by the dedicated system user below) that these tokens belong to, so + # no manual OSM setup is required. + WS_OSM_TOKEN_BRIDGE_ENABLED: bool = True + + # Stable client id (uid) for the auto-created doorkeeper application. Point + # this at an existing application's uid to reuse it instead of creating one. + WS_OSM_OAUTH_CLIENT_UID: str = "workspaces-backend" + + # Scopes granted to the application and mirrored tokens. Must cover the OSM + # API operations the frontend performs; see `lib/oauth.rb` in the OSM website. WS_OSM_OAUTH_SCOPES: str = ( "read_prefs write_prefs write_api write_changeset_comments " "read_gpx write_gpx write_notes" ) + # Dedicated OSM `users` row that owns the auto-created doorkeeper application + # (satisfies oauth_applications.owner_id, a NOT-NULL FK to users). It never + # signs in; these values just need to be stable and unique among users. + WS_OSM_SYSTEM_USER_AUTH_UID: str = "workspaces-backend-system" + WS_OSM_SYSTEM_USER_DISPLAY_NAME: str = "Workspaces Backend (system)" + WS_OSM_SYSTEM_USER_EMAIL: str = "workspaces-backend-system@tdei.us" + SENTRY_DSN: str = "" model_config = SettingsConfigDict( diff --git a/api/core/security.py b/api/core/security.py index 1db25f9..adeaa38 100644 --- a/api/core/security.py +++ b/api/core/security.py @@ -1,3 +1,4 @@ +import secrets import time from enum import StrEnum from uuid import UUID @@ -20,8 +21,8 @@ # @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 -# @test: Test that a validated token is mirrored into the OSM oauth_access_tokens table (with the user provisioned and expires_in from the JWT exp) when WS_OSM_OAUTH_APPLICATION_ID is set, and is a no-op otherwise -# @test: Test that re-presenting a token reactivates its OSM row (revoked_at cleared, expiry refreshed) and that a rotated (superseded) token is revoked, both gated on WS_OSM_OAUTH_APPLICATION_ID +# @test: Test that when WS_OSM_TOKEN_BRIDGE_ENABLED, a validated token is mirrored into oauth_access_tokens with the doorkeeper application + system-owner user + caller user auto-provisioned and expires_in from the JWT exp; and is a no-op when disabled +# @test: Test that re-presenting a token reactivates its OSM row (revoked_at cleared, expiry refreshed) and that a rotated (superseded) token is revoked, both gated on WS_OSM_TOKEN_BRIDGE_ENABLED # Set up logger for this module logger = get_logger(__name__) @@ -338,6 +339,84 @@ async def validate_token( return user_info +async def _ensure_osm_user( + session: AsyncSession, + *, + auth_uid: str, + email: str | None, + display_name: str, +) -> int | None: + """Idempotently provision an OSM ``users`` row (auth_provider ``TDEI``) and + return its id. ``email`` is synthesised when absent -- the column is UNIQUE + and NOT NULL.""" + await session.execute( + text( + "INSERT INTO users (auth_uid, email, display_name, auth_provider, " + "status, pass_crypt, data_public, email_valid, terms_seen, " + "creation_time, terms_agreed, tou_agreed) " + "VALUES (:auth_uid, :email, :name, 'TDEI', 'active', 'none', " + "true, true, true, (now() at time zone 'utc'), " + "(now() at time zone 'utc'), (now() at time zone 'utc')) " + "ON CONFLICT (auth_uid) DO NOTHING" + ), + { + "auth_uid": auth_uid, + "email": email or f"{auth_uid}@tdei.invalid", + "name": display_name, + }, + ) + row = ( + await session.execute( + text( + "SELECT id FROM users " + "WHERE auth_provider = 'TDEI' AND auth_uid = :auth_uid" + ), + {"auth_uid": auth_uid}, + ) + ).first() + return row[0] if row else None + + +async def _ensure_osm_oauth_application(session: AsyncSession) -> int | None: + """Ensure the doorkeeper application the mirrored tokens belong to exists, + creating it (owned by a dedicated system user) if needed. Idempotent by the + configured client uid; returns the application id.""" + owner_id = await _ensure_osm_user( + session, + auth_uid=settings.WS_OSM_SYSTEM_USER_AUTH_UID, + email=settings.WS_OSM_SYSTEM_USER_EMAIL, + display_name=settings.WS_OSM_SYSTEM_USER_DISPLAY_NAME, + ) + if owner_id is None: + return None + await session.execute( + text( + "INSERT INTO oauth_applications (owner_type, owner_id, name, uid, " + "secret, redirect_uri, scopes, confidential, created_at, updated_at) " + "VALUES ('User', :owner_id, :name, :uid, :secret, " + "'urn:ietf:wg:oauth:2.0:oob', :scopes, true, " + "(now() at time zone 'utc'), (now() at time zone 'utc')) " + "ON CONFLICT (uid) DO NOTHING" + ), + { + "owner_id": owner_id, + "name": "Workspaces Backend", + "uid": settings.WS_OSM_OAUTH_CLIENT_UID, + # Never used (tokens are inserted directly rather than issued via the + # OAuth flow), but the column is NOT NULL. + "secret": secrets.token_hex(32), + "scopes": settings.WS_OSM_OAUTH_SCOPES, + }, + ) + row = ( + await session.execute( + text("SELECT id FROM oauth_applications WHERE uid = :uid"), + {"uid": settings.WS_OSM_OAUTH_CLIENT_UID}, + ) + ).first() + return row[0] if row else None + + async def _bridge_token_to_osm( session: AsyncSession, *, @@ -351,46 +430,28 @@ async def _bridge_token_to_osm( (doorkeeper) and cgimap authenticate it through their standard OAuth2 path, with no custom JWT handling required in those services. - Two idempotent writes: provision the OSM ``users`` row (owns the token via - ``resource_owner_id``) and insert a plaintext ``oauth_access_tokens`` row. - Doorkeeper stores tokens plaintext (``SecretStoring::Plain``), so the raw JWT - matches on lookup for both osm-rails and cgimap; ``expires_in`` tracks the - JWT's own ``exp`` so OSM expires it in lockstep with TDEI. + Auto-provisions everything it needs: the doorkeeper ``oauth_applications`` + row (owned by a dedicated system user), the caller's ``users`` row, and a + plaintext ``oauth_access_tokens`` row -- no manual OSM setup. Doorkeeper + stores tokens plaintext (``SecretStoring::Plain``), so the raw JWT matches on + lookup for both osm-rails and cgimap; ``expires_in`` tracks the JWT's ``exp``. - No-op unless ``WS_OSM_OAUTH_APPLICATION_ID`` is set. Best-effort: a failure - here must not break token validation -- the ``/api/v1`` routes keep working; - only the proxied OSM calls would 401 until the row exists. + No-op unless ``WS_OSM_TOKEN_BRIDGE_ENABLED``. Best-effort: a failure here + must not break token validation -- the ``/api/v1`` routes keep working; only + the proxied OSM calls would 401 until the row exists. """ - if settings.WS_OSM_OAUTH_APPLICATION_ID <= 0: + if not settings.WS_OSM_TOKEN_BRIDGE_ENABLED: return - auth_uid = str(user_uuid) try: - # Provision the users row (same shape as _provision_users_from_tdei). - await session.execute( - text( - "INSERT INTO users (auth_uid, email, display_name, auth_provider, " - "status, pass_crypt, data_public, email_valid, terms_seen, " - "creation_time, terms_agreed, tou_agreed) " - "VALUES (:auth_uid, :email, :name, 'TDEI', 'active', 'none', " - "true, true, true, (now() at time zone 'utc'), " - "(now() at time zone 'utc'), (now() at time zone 'utc')) " - "ON CONFLICT (auth_uid) DO NOTHING" - ), - {"auth_uid": auth_uid, "email": email, "name": user_name}, + app_id = await _ensure_osm_oauth_application(session) + if app_id is None: + return + user_id = await _ensure_osm_user( + session, auth_uid=str(user_uuid), email=email, display_name=user_name ) - row = ( - await session.execute( - text( - "SELECT id FROM users " - "WHERE auth_provider = 'TDEI' AND auth_uid = :auth_uid" - ), - {"auth_uid": auth_uid}, - ) - ).first() - if row is None: + if user_id is None: return - user_id = row[0] expires_in = max(0, exp - int(time.time())) if exp else None await session.execute( @@ -410,7 +471,7 @@ async def _bridge_token_to_osm( "revoked_at = NULL" ), { - "app_id": settings.WS_OSM_OAUTH_APPLICATION_ID, + "app_id": app_id, "user_id": user_id, "token": token, "scopes": settings.WS_OSM_OAUTH_SCOPES, @@ -431,14 +492,14 @@ async def _revoke_osm_token(session: AsyncSession, token: str) -> None: Called when a user's token rotates (new ``jti``) so the previous JWT stops authenticating against osm-rails/cgimap before its own ``exp`` rather than - lingering until it expires. No-op unless the bridge is configured. + lingering until it expires. No-op unless the bridge is enabled. Note: OSM/cgimap are reachable only *through* this proxy, and every request first passes ``validate_token`` -- so a TDEI-revoked token is already rejected upstream. This revocation is defense-in-depth for the superseded token and keeps the OSM token store tidy. """ - if settings.WS_OSM_OAUTH_APPLICATION_ID <= 0: + if not settings.WS_OSM_TOKEN_BRIDGE_ENABLED: return try: await session.execute( diff --git a/tests/unit/test_token_bridge.py b/tests/unit/test_token_bridge.py index 401ad73..20e338f 100644 --- a/tests/unit/test_token_bridge.py +++ b/tests/unit/test_token_bridge.py @@ -2,13 +2,15 @@ ``_bridge_token_to_osm`` mirrors a validated TDEI token into the OSM database's ``oauth_access_tokens`` table so osm-rails (doorkeeper) and cgimap authenticate -it through their standard OAuth2 path. We cover: +it through their standard OAuth2 path. It auto-provisions everything it needs: -- it is a no-op unless ``WS_OSM_OAUTH_APPLICATION_ID`` is configured, -- when enabled it provisions the ``users`` row and inserts a plaintext - ``oauth_access_tokens`` row owned by that user, with ``expires_in`` derived - from the JWT ``exp``, -- a DB failure never propagates (auth must not break) and rolls back. +- a dedicated *system* ``users`` row that owns the doorkeeper application, +- the doorkeeper ``oauth_applications`` row (idempotent by the client uid), +- the caller's ``users`` row (owns the token), +- the plaintext ``oauth_access_tokens`` row (expires_in from the JWT exp). + +We cover: the no-op when disabled, the full provisioning chain when enabled, +the wiring of ids between rows, best-effort failure handling, and revocation. """ import time @@ -22,18 +24,21 @@ class RecordingSession: - """Async session stand-in that records (sql, params) and returns a user id - row for the ``SELECT id FROM users`` lookup.""" + """Async session stand-in that records (sql, params) and answers the two id + lookups: ``users`` (system vs caller, by auth_uid) and ``oauth_applications``.""" - def __init__(self, user_id=99): + def __init__(self, system_user_id=1, caller_user_id=42, app_id=7): self.calls: list[tuple[str, dict]] = [] self.commits = 0 self.rollbacks = 0 - self._user_id = user_id + self._system_user_id = system_user_id + self._caller_user_id = caller_user_id + self._app_id = app_id async def execute(self, statement, params=None): + params = params or {} sql = str(statement) - self.calls.append((sql, params or {})) + self.calls.append((sql, params)) class _R: def __init__(self, rows): @@ -43,7 +48,11 @@ def first(self): return self._rows[0] if self._rows else None if "SELECT id FROM users" in sql: - return _R([(self._user_id,)]) + if params.get("auth_uid") == settings.WS_OSM_SYSTEM_USER_AUTH_UID: + return _R([(self._system_user_id,)]) + return _R([(self._caller_user_id,)]) + if "SELECT id FROM oauth_applications" in sql: + return _R([(self._app_id,)]) return _R([]) async def commit(self): @@ -57,8 +66,8 @@ def sql_for(self, needle): async def test_bridge_is_noop_when_disabled(monkeypatch): - """With WS_OSM_OAUTH_APPLICATION_ID == 0 the bridge touches no DB.""" - monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 0) + """With the bridge disabled the bridge touches no DB.""" + monkeypatch.setattr(settings, "WS_OSM_TOKEN_BRIDGE_ENABLED", False) session = RecordingSession() await security._bridge_token_to_osm( @@ -74,13 +83,15 @@ async def test_bridge_is_noop_when_disabled(monkeypatch): assert session.commits == 0 -async def test_bridge_provisions_user_and_mirrors_token(monkeypatch): - """When enabled: users upsert -> id lookup -> oauth_access_tokens upsert.""" - monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 7) +async def test_bridge_provisions_app_users_and_token(monkeypatch): + """Enabled: system user -> application -> caller user -> mirrored token, + with the ids wired between rows.""" + monkeypatch.setattr(settings, "WS_OSM_TOKEN_BRIDGE_ENABLED", True) + monkeypatch.setattr(settings, "WS_OSM_OAUTH_CLIENT_UID", "workspaces-backend") monkeypatch.setattr(settings, "WS_OSM_OAUTH_SCOPES", "write_api read_prefs") uid = uuid4() exp = int(time.time()) + 3600 - session = RecordingSession(user_id=42) + session = RecordingSession(system_user_id=1, caller_user_id=42, app_id=7) await security._bridge_token_to_osm( cast(AsyncSession, session), @@ -91,19 +102,25 @@ async def test_bridge_provisions_user_and_mirrors_token(monkeypatch): exp=exp, ) - # users row provisioned idempotently, keyed by auth_uid = str(sub) + # System user provisioned (owns the app) and caller user provisioned. user_inserts = session.sql_for("INSERT INTO users") - assert user_inserts and "ON CONFLICT (auth_uid)" in user_inserts[0][0] - assert user_inserts[0][1]["auth_uid"] == str(uid) - assert user_inserts[0][1]["email"] == "alice@example.com" - - # token mirrored with the configured application/scopes and exp-derived TTL + inserted_auth_uids = {c[1]["auth_uid"] for c in user_inserts} + assert settings.WS_OSM_SYSTEM_USER_AUTH_UID in inserted_auth_uids + assert str(uid) in inserted_auth_uids + + # Application created idempotently by uid, owned by the system user (id 1). + app_inserts = session.sql_for("INSERT INTO oauth_applications") + assert app_inserts and "ON CONFLICT (uid)" in app_inserts[0][0] + assert app_inserts[0][1]["uid"] == "workspaces-backend" + assert app_inserts[0][1]["owner_id"] == 1 + assert app_inserts[0][1]["scopes"] == "write_api read_prefs" + + # Token mirrored: app id from the lookup, resource_owner = caller (42), + # self-healing upsert, exp-derived TTL. token_inserts = session.sql_for("oauth_access_tokens") - assert token_inserts and "ON CONFLICT (token)" in token_inserts[0][0] - # Re-presenting a token must reactivate it (clear revocation, refresh expiry). - assert "DO UPDATE" in token_inserts[0][0] - assert "revoked_at = NULL" in token_inserts[0][0] - params = token_inserts[0][1] + assert token_inserts + sql, params = token_inserts[0] + assert "DO UPDATE" in sql and "revoked_at = NULL" in sql assert params["app_id"] == 7 assert params["user_id"] == 42 assert params["token"] == "jwt-token" @@ -114,27 +131,33 @@ async def test_bridge_provisions_user_and_mirrors_token(monkeypatch): assert session.rollbacks == 0 -async def test_bridge_expires_in_none_without_exp(monkeypatch): - """A token without an `exp` claim mirrors with a NULL expires_in.""" - monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 7) +async def test_bridge_synthesises_email_when_absent(monkeypatch): + """A caller without an email claim still provisions (email is UNIQUE/NOT NULL).""" + monkeypatch.setattr(settings, "WS_OSM_TOKEN_BRIDGE_ENABLED", True) + uid = uuid4() session = RecordingSession() await security._bridge_token_to_osm( cast(AsyncSession, session), - user_uuid=uuid4(), + user_uuid=uid, user_name="alice", email=None, token="jwt-token", exp=None, ) + caller_insert = next( + c for c in session.sql_for("INSERT INTO users") if c[1]["auth_uid"] == str(uid) + ) + assert caller_insert[1]["email"] == f"{uid}@tdei.invalid" + params = session.sql_for("oauth_access_tokens")[0][1] assert params["expires_in"] is None async def test_bridge_is_best_effort_on_db_error(monkeypatch): """A DB failure must not propagate out of validation; it rolls back.""" - monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 7) + monkeypatch.setattr(settings, "WS_OSM_TOKEN_BRIDGE_ENABLED", True) class BoomSession(RecordingSession): async def execute(self, statement, params=None): @@ -142,7 +165,6 @@ async def execute(self, statement, params=None): session = BoomSession() - # Must not raise. await security._bridge_token_to_osm( cast(AsyncSession, session), user_uuid=uuid4(), @@ -158,7 +180,7 @@ async def execute(self, statement, params=None): async def test_revoke_is_noop_when_disabled(monkeypatch): """With the bridge off, revoking a rotated token touches no DB.""" - monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 0) + monkeypatch.setattr(settings, "WS_OSM_TOKEN_BRIDGE_ENABLED", False) session = RecordingSession() await security._revoke_osm_token(cast(AsyncSession, session), "old-token") @@ -169,7 +191,7 @@ async def test_revoke_is_noop_when_disabled(monkeypatch): async def test_revoke_marks_only_the_superseded_token(monkeypatch): """Revocation flips revoked_at for exactly the given token, if not already.""" - monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 7) + monkeypatch.setattr(settings, "WS_OSM_TOKEN_BRIDGE_ENABLED", True) session = RecordingSession() await security._revoke_osm_token(cast(AsyncSession, session), "old-token") @@ -183,7 +205,7 @@ async def test_revoke_marks_only_the_superseded_token(monkeypatch): async def test_revoke_is_best_effort_on_db_error(monkeypatch): """A DB failure during revocation must not propagate; it rolls back.""" - monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 7) + monkeypatch.setattr(settings, "WS_OSM_TOKEN_BRIDGE_ENABLED", True) class BoomSession(RecordingSession): async def execute(self, statement, params=None): From 935d3442b8aa7402fc6f2a5f715eb509a104c986 Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Fri, 10 Jul 2026 16:40:42 -0400 Subject: [PATCH 08/34] Change stub user to validate OSM rails models --- ...f3a7b9c1d2e4_heal_short_tdei_pass_crypt.py | 47 +++++++++++++++++++ api/core/security.py | 8 +++- api/src/tasking/projects/repository.py | 8 +++- 3 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 alembic_osm/versions/f3a7b9c1d2e4_heal_short_tdei_pass_crypt.py diff --git a/alembic_osm/versions/f3a7b9c1d2e4_heal_short_tdei_pass_crypt.py b/alembic_osm/versions/f3a7b9c1d2e4_heal_short_tdei_pass_crypt.py new file mode 100644 index 0000000..5ff6bca --- /dev/null +++ b/alembic_osm/versions/f3a7b9c1d2e4_heal_short_tdei_pass_crypt.py @@ -0,0 +1,47 @@ +"""heal short TDEI-provisioned users.pass_crypt + +TDEI users are provisioned by the backend via raw SQL with a placeholder +``pass_crypt``. Older rows used a 4-char value (``"none"``), which violates OSM's +``pass_crypt`` length 8..255 ``User`` validation. That makes the user invalid, +and Rails operations that re-validate the author via ``validates :author, +:associated => true`` (posting a changeset comment or a note comment) then fail +to save. Replace any too-short value with a random throwaway -- TDEI manages +auth, so ``pass_crypt`` is never used to authenticate. + +Revision ID: f3a7b9c1d2e4 +Revises: 5303f61d7b3a +Create Date: 2026-07-11 00:00:00.000000 + +""" + +from typing import Sequence, Union + +from alembic import op +from sqlalchemy import inspect, text + +# revision identifiers, used by Alembic. +revision: str = "f3a7b9c1d2e4" +down_revision: Union[str, None] = "5303f61d7b3a" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + bind = op.get_bind() + # `users` is owned by the OSM Rails website, not this tree; guard so the + # migration is a no-op if it hasn't been created yet. + if not inspect(bind).has_table("users"): + return + + # Idempotent: re-running only touches rows that are still too short. + op.execute( + text( + "UPDATE users SET pass_crypt = md5(random()::text) " + "WHERE auth_provider = 'TDEI' AND length(pass_crypt) < 8" + ) + ) + + +def downgrade() -> None: + # The original short values are unrecoverable (and were invalid anyway). + pass diff --git a/api/core/security.py b/api/core/security.py index adeaa38..020abf8 100644 --- a/api/core/security.py +++ b/api/core/security.py @@ -354,7 +354,7 @@ async def _ensure_osm_user( "INSERT INTO users (auth_uid, email, display_name, auth_provider, " "status, pass_crypt, data_public, email_valid, terms_seen, " "creation_time, terms_agreed, tou_agreed) " - "VALUES (:auth_uid, :email, :name, 'TDEI', 'active', 'none', " + "VALUES (:auth_uid, :email, :name, 'TDEI', 'active', :pass_crypt, " "true, true, true, (now() at time zone 'utc'), " "(now() at time zone 'utc'), (now() at time zone 'utc')) " "ON CONFLICT (auth_uid) DO NOTHING" @@ -363,6 +363,12 @@ async def _ensure_osm_user( "auth_uid": auth_uid, "email": email or f"{auth_uid}@tdei.invalid", "name": display_name, + # OSM's User model validates pass_crypt length 8..255. TDEI manages + # auth, so this is a throwaway that just satisfies that rule (a too- + # short value makes the user invalid and breaks Rails operations + # that re-validate it via `validates :author, :associated => true`, + # e.g. posting a changeset comment). + "pass_crypt": secrets.token_hex(16), }, ) row = ( diff --git a/api/src/tasking/projects/repository.py b/api/src/tasking/projects/repository.py index 3aa6820..44bbc42 100644 --- a/api/src/tasking/projects/repository.py +++ b/api/src/tasking/projects/repository.py @@ -1,6 +1,7 @@ from __future__ import annotations import json +import secrets from datetime import datetime from typing import Any from uuid import UUID @@ -313,13 +314,18 @@ async def _provision_users_from_tdei( await self.session.execute( text( "INSERT INTO users (auth_uid, email, display_name, auth_provider, status, pass_crypt, data_public, email_valid, terms_seen, creation_time, terms_agreed, tou_agreed) " - "VALUES (:uid, :email, :name, 'TDEI', 'active', 'none', true, true, true, (now() at time zone 'utc'), (now() at time zone 'utc'), (now() at time zone 'utc')) " + "VALUES (:uid, :email, :name, 'TDEI', 'active', :pass_crypt, true, true, true, (now() at time zone 'utc'), (now() at time zone 'utc'), (now() at time zone 'utc')) " "ON CONFLICT (auth_uid) DO NOTHING" ), params={ "uid": uid, "email": member.email, "name": member.display_name, + # OSM validates pass_crypt length 8..255; a too-short value + # makes the user invalid and breaks Rails ops that re-validate + # the author (changeset/note comments). TDEI manages auth, so + # this is a throwaway. + "pass_crypt": secrets.token_hex(16), }, ) resolved.add(uid) From b56f1cdb5c85961990cfc95f0919959f0c3f11f0 Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Fri, 10 Jul 2026 16:52:01 -0400 Subject: [PATCH 09/34] Docs --- .claude/settings.json | 4 ++- CLAUDE.md | 78 +++++++++++++++++++++++++++++++++++++++++++ README.md | 42 +++++++++++++++++++++++ 3 files changed, 123 insertions(+), 1 deletion(-) diff --git a/.claude/settings.json b/.claude/settings.json index fa7924c..ea52c82 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -10,7 +10,9 @@ "Bash(git config *)", "Bash(command -v gh)", "Bash(gh api *)", - "Bash(python3 -c \"import sys,json; d=json.load\\(sys.stdin\\); print\\('protected:', d.get\\('protected'\\)\\); p=d.get\\('protection',{}\\); print\\('required_pull_request_reviews:', 'yes' if p.get\\('required_pull_request_reviews'\\) else 'no'\\); print\\('enforce_admins:', \\(p.get\\('enforce_admins'\\) or {}\\).get\\('enabled'\\)\\)\")" + "Bash(python3 -c \"import sys,json; d=json.load\\(sys.stdin\\); print\\('protected:', d.get\\('protected'\\)\\); p=d.get\\('protection',{}\\); print\\('required_pull_request_reviews:', 'yes' if p.get\\('required_pull_request_reviews'\\) else 'no'\\); print\\('enforce_admins:', \\(p.get\\('enforce_admins'\\) or {}\\).get\\('enabled'\\)\\)\")", + "Bash(grep -nA6 \"TENANT_BYPASSES: list\" api/main.py)", + "Bash(grep -nA10 \"STRIP_REQUEST_HEADERS = \" api/main.py)" ] } } diff --git a/CLAUDE.md b/CLAUDE.md index 1e5e9d6..d15dea5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -116,6 +116,84 @@ enforced at this layer; if required, it must be enforced downstream (`workspaces-openstreetmap-website/`, `workspaces-cgimap/`) — that has not been audited here. +## The OSM proxy layer: routing, auth, and user provisioning + +This service is a reverse proxy in front of the OSM website (`osm-rails`) and +cgimap. The non-obvious parts, learned the hard way: + +### Deployment routing (workspaces-stack) + +In `workspaces-stack`, the **api container (this backend) serves the public OSM +host** (`osm.workspaces-...`) alongside the API hosts — its traefik router +rule is `Host(new-api) || Host(api) || Host(osm)`, and the `osm-web` / +`osm-log-proxy` routers are commented out. So **every OSM call the frontend +makes routes through this backend**: `validate_token`, the `X-Workspace` gate, +and the CORS middleware all apply. The backend then proxies `/api/0.6/*` to +`WS_OSM_HOST` (config default `http://osm-web` — the internal service, *not* the +public domain). `osm-web`'s nginx splits traffic: changeset read/write subpaths +to cgimap, everything else to osm-rails. + +The frontend (`services/osm.ts`) calls the OSM host directly with +`credentials: 'include'` + a Bearer TDEI token. Because it's *credentialed* +CORS, `CORS_ORIGINS` must list the exact frontend origin (no `*`; +`allow_credentials` is on). The stack supplies `WS_API_CORS_ORIGINS` as a **JSON +array** (`["https://..."]`), while older config comma-split it — a JSON array +would then become one malformed origin. `api/main.py` therefore reads +`settings.cors_origins_list`, which parses **both** a JSON array and a +comma-separated string (and `*`); set `CORS_ORIGINS` in either form. + +### Two databases; `users` is owned by OSM Rails + +`TASK_DATABASE_URL` (`alembic_task` tree) holds workspaces / tasking-manager +tables; `OSM_DATABASE_URL` (`alembic_osm` tree) holds OSM data, `users`, and the +`tasking_*` tables. The **`users` table is owned by the OSM Rails website**, not +either alembic tree — the trees only FK to it, and integration tests stub it +(`tests/integration/conftest.py`). The backend provisions `users` rows itself +via raw SQL with `auth_provider='TDEI'` and `auth_uid = str()` (the OSM +`auth_uid` **is** the token's `sub` claim). + +### How OSM authenticates, and the TDEI token bridge + +* **osm-rails** authenticates API calls *only* via **doorkeeper OAuth2**: it + looks the bearer token up in `oauth_access_tokens` (`token` → + `resource_owner_id` → `users.id`). Doorkeeper stores tokens **plaintext** + (`SecretStoring::Plain`), so the raw JWT matches directly. It has **no** + TDEI/JWT auth path. +* **cgimap** has both: the oauth2 lookup *and* a custom TDEI path + (`get_user_id_for_tdei_token`, which verifies the JWT and matches + `users.auth_uid`). + +To make TDEI tokens work against osm-rails without forking it, the **token +bridge** (`_bridge_token_to_osm` in `api/core/security.py`) mirrors a validated +TDEI JWT into `oauth_access_tokens` — on the cache-miss path of `validate_token` +(so ~once per token, not per request). Gated by `WS_OSM_TOKEN_BRIDGE_ENABLED`. +It auto-provisions everything: a dedicated **system user** (`WS_OSM_SYSTEM_USER_*`) +to own the doorkeeper `oauth_applications` row it creates (keyed by +`WS_OSM_OAUTH_CLIENT_UID`; `oauth_applications.owner_id` is a NOT-NULL FK to +`users`, hence the system user), the caller's `users` row, and the plaintext +`oauth_access_tokens` row (`expires_in` from the JWT `exp`; +`ON CONFLICT (token) DO UPDATE` so re-presenting reactivates it). On token +rotation (new `jti`) the superseded token is revoked. Since the token then lives +in `oauth_access_tokens`, both osm-rails and cgimap authenticate it via plain +OAuth2 — cgimap's custom TDEI path becomes redundant. + +### Provisioning gotcha: `pass_crypt` must be 8..255 chars + +The OSM `User` model has `validates :pass_crypt, :length => 8..255`. When the +backend provisions a `users` row, **`pass_crypt` must be 8–255 chars** (use a +random throwaway — TDEI manages auth, so it's never used to log in). A too-short +value (the old `'none'`) makes the user *invalid*. This is silent for **cgimap** +operations — cgimap runs no Rails validations — but breaks any **osm-rails** +operation that re-validates the author via `validates :author, :associated => +true`: notably `POST /api/0.6/changeset/{id}/comment` and note comments. The +comment fails to save (no `id`), then rendering it throws *"Unable to serialize +… without an id"*. Both provisioning paths (`_ensure_osm_user`, +`_provision_users_from_tdei`) set a valid `pass_crypt`; migration +`alembic_osm/versions/f3a7b9c1d2e4` heals legacy short rows. General principle: +a backend-provisioned `users` row must satisfy OSM's `User` validations (also +`display_name` 3..255 + unique, `email` present + unique) or Rails operations +that touch it will fail even though cgimap operations succeed. + ## Testing Two layers, both fast and dependency-free (no Postgres, PostGIS, Docker, or diff --git a/README.md b/README.md index 8ffa3f5..7b905d3 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,48 @@ This is a combination API backend for workspaces, providing /workspaces* methods the OSM API ("openstreetmap website") plus OSM CGI-map (the C-accelerated methods) that enforces authorization and authentication based on a TDEI/Keycloak JWT token (see main.py for this proxy logic). +## What the proxy must provide for osm-rails / osm-web + +This backend is the **only entry point** to the OSM tier (osm-rails + cgimap, +behind `osm-web`): in the deployment, the public OSM host routes to this +container, which proxies `/api/0.6/*` to `WS_OSM_HOST` (default +`http://osm-web`). For the OSM services to work, the proxy must uphold the +following contract. `CLAUDE.md` has the full rationale. + +1. **Bridge TDEI auth into OSM's OAuth2.** osm-rails authenticates the API *only* + via doorkeeper OAuth2 (`oauth_access_tokens`); it has no TDEI/JWT path. So on + token validation the backend mirrors the TDEI JWT into `oauth_access_tokens` + in the OSM DB (the "token bridge" in `api/core/security.py`), and forwards the + incoming `Authorization: Bearer ` header unchanged. Then osm-rails and + cgimap authenticate the token via plain OAuth2. Controlled by + `WS_OSM_TOKEN_BRIDGE_ENABLED` (on by default) — with it off, osm-rails returns + **401** for TDEI tokens. The backend auto-creates the doorkeeper application + (and a system user to own it), so no manual OSM setup is required. + +2. **Provision valid OSM `users` rows.** The backend creates OSM `users` rows for + TDEI users (`auth_provider='TDEI'`, `auth_uid` = the JWT `sub`). These must + satisfy OSM's `User` validations — in particular `pass_crypt` length 8..255. + A too-short value is invisible to cgimap but makes osm-rails operations that + re-validate the user fail (e.g. posting a changeset comment or a note), + surfacing as *"Unable to serialize … without an id"*. The + `alembic_osm` migration `*_heal_short_tdei_pass_crypt` repairs legacy rows. + +3. **Carry workspace tenancy.** Workspace-scoped OSM requests must include an + `X-Workspace: ` header. The proxy authorizes it against the caller's + workspaces and forwards it (it is *not* stripped) so cgimap/osm-rails scope to + the `workspace-` schema. A few paths are exempt (`TENANT_BYPASSES` in + `api/main.py`): workspace create/delete (`PUT`/`DELETE /api/0.6/workspaces/{id}`) + and user provisioning during sign-in (`PUT /api/0.6/user/{uid}`). + +4. **Set the proxy headers.** The proxy rewrites `Host` to the OSM host and sets + `X-Real-IP` / `X-Forwarded-For` / `-Host` / `-Proto`, while stripping + hop-by-hop headers and any spoofed forwarding headers from the client. It does + *not* strip `Authorization` or `X-Workspace`. + +5. **Connectivity.** `WS_OSM_HOST` must reach `osm-web`, and the backend needs + **both** `OSM_DATABASE_URL` and `TASK_DATABASE_URL` — the token bridge and + user provisioning write to the OSM database. + ## Branch Index * ```develop``` merge your work here; keep this up to date with the "development" environment / dev tag From ecc0f4fff5f58a61511c2688b6e455624cc3ae35 Mon Sep 17 00:00:00 2001 From: Naresh Kumar D Date: Tue, 14 Jul 2026 11:24:12 +0530 Subject: [PATCH 10/34] initial docker setup done - Added notes to local development. - Need to add more details on initial development. --- .gitignore | 1 + README.md | 25 ++++++++++++++ api/core/config.py | 1 + docker-compose.local.yml | 71 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 98 insertions(+) create mode 100644 docker-compose.local.yml diff --git a/.gitignore b/.gitignore index 4516af1..d929d58 100644 --- a/.gitignore +++ b/.gitignore @@ -170,3 +170,4 @@ docs/tasking-mvp/tasking-mvp.postman_environment.json docs/tasking-mvp/_enrich_postman.py docs/tasking-mvp/feature-coverage.md .idea/ +data/ \ No newline at end of file diff --git a/README.md b/README.md index 7b905d3..a8867e1 100644 --- a/README.md +++ b/README.md @@ -82,3 +82,28 @@ uvx pyright --pythonpath .venv/bin/python api tests uv run black api tests && uv run isort api tests ``` +## Development with local environment + +Use the file `docker-compose.local.yml` to build and deploy local code changes. This allows you to run the entire system at once instead of +connecting to existing Databases. + +### Initial setup. +- On first launch, rails-worker will fail because migrations are not done +- Go to the `osm-rails` `/bin/sh` and execute `bundle exec rails db:migrate` +- The above script runs the migration code for osm-rails +- The rails-worker will be able to run +- The backend code will fail first time becase `workspaces-tasks-local` database is not available +- Login to postgresql container and run the following commands + `psql --username postgres` + `create database "workspaces-tasks-local";` + `psql --username postgres --dbname "workspaces-tasks-local";` + `create extension if not exists postgis;` +- Run the backend code now and it should be able to run + +### Commands to start and stop the docker compose + +`docker compose --file docker-compose.local.yml build up -d` + +`docker compose --file docker-compose.local.yml down` + +Backend code will be available at `http://localhost:3000` \ No newline at end of file diff --git a/api/core/config.py b/api/core/config.py index 04693ac..c6554c7 100644 --- a/api/core/config.py +++ b/api/core/config.py @@ -96,6 +96,7 @@ def cors_origins_list(self) -> list[str]: model_config = SettingsConfigDict( env_file=".env", env_file_encoding="utf-8", + extra="ignore", # ignore unknown environment variables ) diff --git a/docker-compose.local.yml b/docker-compose.local.yml new file mode 100644 index 0000000..8bfeae3 --- /dev/null +++ b/docker-compose.local.yml @@ -0,0 +1,71 @@ +services: + database: + image: postgis/postgis:16-3.4 + environment: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: testing + POSTGRES_DB: workspaces-osm-local + ports: + - 5432:5432 + volumes: + - ./data/db:/var/lib/postgresql/data + healthcheck: + test: ["CMD-SHELL", "pg_isready -U postgres -d workspaces-osm-local"] + interval: 10s + timeout: 5s + retries: 5 + osm-cgimap: + image: opensidewalksdev.azurecr.io/workspaces-osm-cgimap:dev + environment: + CGIMAP_INSTANCES: 10 + CGIMAP_HOST: database + CGIMAP_USERNAME: postgres + CGIMAP_PASSWORD: testing + CGIMAP_DBNAME: workspaces-osm-local + CGIMAP_MAX_CHANGESET_ELEMENTS: 100000000 # max features per import or save + CGIMAP_MAX_PAYLOAD: 1000000000 # max size of dataset uploads + CGIMAP_MAP_NODES: 100000000 # max number of nodes per requeset + CGIMAP_MAP_AREA: 1 # max area per request in square degrees + ports: + - 8000 + osm-rails: + image: opensidewalksdev.azurecr.io/workspaces-osm-rails:dev + environment: + RAILS_ENV: production + SECRET_KEY_BASE: osm_secret_key + WS_OSM_DB_HOST: database + WS_OSM_DB_USER: postgres + WS_OSM_DB_PASS: testing + WS_OSM_DB_NAME: workspaces-osm-local + stdin_open: true + tty: true + ports: + - 3000 + command: ["bundle", "exec", "rails", "s", "-p", "3000", "-b", "0.0.0.0"] + + osm-rails-worker: + image: opensidewalksdev.azurecr.io/workspaces-osm-rails:dev + environment: + RAILS_ENV: production + SECRET_KEY_BASE: osm_secret_key + WS_OSM_DB_HOST: database + WS_OSM_DB_USER: postgres + WS_OSM_DB_PASS: testing + WS_OSM_DB_NAME: workspaces-osm-local + stdin_open: true + tty: true + command: ["bundle", "exec", "rake", "jobs:work"] + + backend: + build: . + container_name: workspaces-backend + volumes: + - .:/app + ports: + - 8000:8000 + environment: + TDEI_BACKEND_URL: ${TDEI_BACKEND_URL} + TDEI_OIDC_REALM: ${TDEI_OIDC_REALM} + TDEI_OIDC_URL: ${TDEI_OIDC_URL} + OSM_DATABASE_URL: postgresql+asyncpg://postgres:testing@database:5432/workspaces-osm-local + TASK_DATABASE_URL: postgresql+asyncpg://postgres:testing@database:5432/workspaces-tasks-local \ No newline at end of file From ac2700fb4f54f764dfa4df2664429816fe8ae089 Mon Sep 17 00:00:00 2001 From: Naresh Kumar D Date: Tue, 14 Jul 2026 12:19:16 +0530 Subject: [PATCH 11/34] Added custom_imagery column Migration logic yet to be written --- api/src/tasking/projects/dtos.py | 3 +++ api/src/tasking/projects/repository.py | 4 ++++ api/src/tasking/projects/schemas.py | 7 +++++++ 3 files changed, 14 insertions(+) diff --git a/api/src/tasking/projects/dtos.py b/api/src/tasking/projects/dtos.py index 488e981..709b80f 100644 --- a/api/src/tasking/projects/dtos.py +++ b/api/src/tasking/projects/dtos.py @@ -47,6 +47,7 @@ class ProjectCreateRequest(WireModel): lock_timeout_hours: int = PydField(default=8, ge=1, le=720) aoi: Optional[AoiInput] = None role_assignments: list[ProjectRoleAssignment] = PydField(default_factory=list) + custom_imagery: Optional[Any] = PydField(default=None) @field_validator("name") @classmethod @@ -68,6 +69,7 @@ class ProjectUpdateRequest(WireModel): instructions: Optional[str] = PydField(default=None, max_length=10_000) lock_timeout_hours: Optional[int] = PydField(default=None, ge=1, le=720) review_required: Optional[bool] = None + custom_imagery: Optional[Any] = PydField(default=None) class ProjectResponse(WireModel): @@ -87,6 +89,7 @@ class ProjectResponse(WireModel): created_by_name: Optional[str] = None created_at: datetime updated_at: datetime + custom_imagery: Optional[Any] = None class ProjectListItem(WireModel): diff --git a/api/src/tasking/projects/repository.py b/api/src/tasking/projects/repository.py index 44bbc42..b046436 100644 --- a/api/src/tasking/projects/repository.py +++ b/api/src/tasking/projects/repository.py @@ -243,6 +243,7 @@ def _to_response(project: TaskingProject, task_count: int = 0) -> ProjectRespons created_by_name=project.created_by_name, created_at=project.created_at, updated_at=project.updated_at, + custom_imagery=project.custom_imagery, # type: ignore[arg-type] ) async def _provision_users_from_tdei( @@ -531,6 +532,7 @@ async def create( lock_timeout_hours=body.lock_timeout_hours, created_by=current_user.user_uuid, created_by_name=current_user.user_name, + custom_imagery=body.custom_imagery, # type: ignore[arg-type] ) if body.aoi is not None: geom = _aoi_to_shapely(body.aoi) @@ -632,6 +634,8 @@ async def patch( updates["lock_timeout_hours"] = body.lock_timeout_hours if body.review_required is not None: updates["review_required"] = body.review_required + if body.custom_imagery is not None: + updates["custom_imagery"] = body.custom_imagery # type: ignore[arg-type] if updates: updates["updated_at"] = datetime.now() diff --git a/api/src/tasking/projects/schemas.py b/api/src/tasking/projects/schemas.py index 901d5f6..8150f4b 100644 --- a/api/src/tasking/projects/schemas.py +++ b/api/src/tasking/projects/schemas.py @@ -11,6 +11,7 @@ from sqlalchemy import Column from sqlalchemy import Enum as SAEnum from sqlmodel import Field, SQLModel +from sqlalchemy.dialects.postgresql import JSONB # --------------------------------------------------------------------------- # Enums (mirrors of postgres enums in the migration) @@ -94,6 +95,12 @@ class TaskingProject(SQLModel, table=True): ) deleted_at: Optional[datetime] = None + # Custom Imagery data for the project. Stored as JSONB in the database and converted to / from a Pydantic model in the repository layer. + custom_imagery: Optional[Any] = Field( + default=None, + sa_column=Column(JSONB, nullable=True,default=None) + ) + # --------------------------------------------------------------------------- # Project role enum + table From 70b14ccac6244f0526a0141a1071d94908415cae Mon Sep 17 00:00:00 2001 From: Naresh Kumar D Date: Tue, 14 Jul 2026 15:30:15 +0530 Subject: [PATCH 12/34] imagery and description added --- README.md | 2 +- ...61f527ef_custom_imagery_and_description.py | 37 +++++++++++++++++++ api/src/tasking/projects/dtos.py | 7 +++- api/src/tasking/projects/repository.py | 2 + api/src/tasking/projects/schemas.py | 7 ++-- 5 files changed, 49 insertions(+), 6 deletions(-) create mode 100644 alembic_osm/versions/a92361f527ef_custom_imagery_and_description.py diff --git a/README.md b/README.md index a8867e1..ebba814 100644 --- a/README.md +++ b/README.md @@ -102,7 +102,7 @@ connecting to existing Databases. ### Commands to start and stop the docker compose -`docker compose --file docker-compose.local.yml build up -d` +`docker compose --file docker-compose.local.yml up --build -d` `docker compose --file docker-compose.local.yml down` diff --git a/alembic_osm/versions/a92361f527ef_custom_imagery_and_description.py b/alembic_osm/versions/a92361f527ef_custom_imagery_and_description.py new file mode 100644 index 0000000..6a4fdcc --- /dev/null +++ b/alembic_osm/versions/a92361f527ef_custom_imagery_and_description.py @@ -0,0 +1,37 @@ +"""custom_imagery_and_description + +Revision ID: a92361f527ef +Revises: f3a7b9c1d2e4 +Create Date: 2026-07-14 15:08:42.142553 + +""" + +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op +from sqlalchemy.dialects.postgresql import JSONB + +# revision identifiers, used by Alembic. +revision: str = "a92361f527ef" +down_revision: Union[str, None] = "f3a7b9c1d2e4" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.add_column( + "tasking_projects", + sa.Column("custom_imagery", JSONB(astext_type=sa.Text()), nullable=True), + ) + op.add_column( + "tasking_projects", + sa.Column("description", sa.String(length=10000), nullable=True), + ) + pass + + +def downgrade() -> None: + op.drop_column("tasking_projects", "custom_imagery") + op.drop_column("tasking_projects", "description") + pass diff --git a/api/src/tasking/projects/dtos.py b/api/src/tasking/projects/dtos.py index 709b80f..601c09e 100644 --- a/api/src/tasking/projects/dtos.py +++ b/api/src/tasking/projects/dtos.py @@ -47,7 +47,8 @@ class ProjectCreateRequest(WireModel): lock_timeout_hours: int = PydField(default=8, ge=1, le=720) aoi: Optional[AoiInput] = None role_assignments: list[ProjectRoleAssignment] = PydField(default_factory=list) - custom_imagery: Optional[Any] = PydField(default=None) + custom_imagery: Optional[dict[str, Any]] = PydField(default=None) + description: Optional[str] = PydField(default=None, max_length=10_000) @field_validator("name") @classmethod @@ -69,7 +70,8 @@ class ProjectUpdateRequest(WireModel): instructions: Optional[str] = PydField(default=None, max_length=10_000) lock_timeout_hours: Optional[int] = PydField(default=None, ge=1, le=720) review_required: Optional[bool] = None - custom_imagery: Optional[Any] = PydField(default=None) + custom_imagery: Optional[dict[str, Any]] = PydField(default=None) + description: Optional[str] = PydField(default=None, max_length=10_000) class ProjectResponse(WireModel): @@ -90,6 +92,7 @@ class ProjectResponse(WireModel): created_at: datetime updated_at: datetime custom_imagery: Optional[Any] = None + description: Optional[str] = None class ProjectListItem(WireModel): diff --git a/api/src/tasking/projects/repository.py b/api/src/tasking/projects/repository.py index b046436..71c4eb9 100644 --- a/api/src/tasking/projects/repository.py +++ b/api/src/tasking/projects/repository.py @@ -244,6 +244,7 @@ def _to_response(project: TaskingProject, task_count: int = 0) -> ProjectRespons created_at=project.created_at, updated_at=project.updated_at, custom_imagery=project.custom_imagery, # type: ignore[arg-type] + description=project.description, # type: ignore[arg-type] ) async def _provision_users_from_tdei( @@ -533,6 +534,7 @@ async def create( created_by=current_user.user_uuid, created_by_name=current_user.user_name, custom_imagery=body.custom_imagery, # type: ignore[arg-type] + description=body.description, # type: ignore[arg-type] ) if body.aoi is not None: geom = _aoi_to_shapely(body.aoi) diff --git a/api/src/tasking/projects/schemas.py b/api/src/tasking/projects/schemas.py index 8150f4b..85fc0b2 100644 --- a/api/src/tasking/projects/schemas.py +++ b/api/src/tasking/projects/schemas.py @@ -10,8 +10,8 @@ from pydantic import Field as PydField from sqlalchemy import Column from sqlalchemy import Enum as SAEnum -from sqlmodel import Field, SQLModel from sqlalchemy.dialects.postgresql import JSONB +from sqlmodel import Field, SQLModel # --------------------------------------------------------------------------- # Enums (mirrors of postgres enums in the migration) @@ -97,10 +97,11 @@ class TaskingProject(SQLModel, table=True): # Custom Imagery data for the project. Stored as JSONB in the database and converted to / from a Pydantic model in the repository layer. custom_imagery: Optional[Any] = Field( - default=None, - sa_column=Column(JSONB, nullable=True,default=None) + default=None, sa_column=Column(JSONB, nullable=True, default=None) ) + description: Optional[str] = Field(default=None, nullable=True) + # --------------------------------------------------------------------------- # Project role enum + table From 49c11b6e1ede198768882ef346beb17d11844f4f Mon Sep 17 00:00:00 2001 From: Naresh Kumar D Date: Tue, 14 Jul 2026 16:06:52 +0530 Subject: [PATCH 13/34] Added routes for validate name Added routes for validating the project name and unit tests for the same --- api/src/tasking/projects/dtos.py | 5 ++ api/src/tasking/projects/repository.py | 16 +++++ api/src/tasking/projects/routes.py | 21 +++++++ tests/integration/test_projects_flow.py | 32 ++++++++++ tests/unit/conftest.py | 8 +++ tests/unit/test_project_routes.py | 80 +++++++++++++++++++++++++ 6 files changed, 162 insertions(+) diff --git a/api/src/tasking/projects/dtos.py b/api/src/tasking/projects/dtos.py index 601c09e..01057a8 100644 --- a/api/src/tasking/projects/dtos.py +++ b/api/src/tasking/projects/dtos.py @@ -120,6 +120,10 @@ class ProjectListResponse(WireModel): pagination: Pagination +class ProjectNameValidationResponse(WireModel): + exists: bool + + # --------------------------------------------------------------------------- # AOI response shape (canonical Feature wrapping a MultiPolygon) # --------------------------------------------------------------------------- @@ -190,6 +194,7 @@ class SelfProjectRolesResponse(WireModel): "ProjectCreateRequest", "ProjectListItem", "ProjectListResponse", + "ProjectNameValidationResponse", "ProjectResponse", "ProjectRoleAddRequest", "ProjectRoleAssignment", diff --git a/api/src/tasking/projects/repository.py b/api/src/tasking/projects/repository.py index 71c4eb9..beee8e6 100644 --- a/api/src/tasking/projects/repository.py +++ b/api/src/tasking/projects/repository.py @@ -470,6 +470,22 @@ async def list_projects( pagination=Pagination(page=page, page_size=page_size, total=total), ) + async def project_name_exists(self, workspace_id: int, name: str) -> bool: + result = await self.session.execute( + select(func.count()) + .select_from(TaskingProject) + .where( + (TaskingProject.workspace_id == workspace_id) + & (TaskingProject.name == name) + & ( + TaskingProject.deleted_at.is_( # pyright: ignore[reportAttributeAccessIssue, reportOptionalMemberAccess] + None + ) + ) + ) + ) + return int(result.scalar() or 0) > 0 + async def create( self, workspace_id: int, diff --git a/api/src/tasking/projects/routes.py b/api/src/tasking/projects/routes.py index d4979b0..4d531d0 100644 --- a/api/src/tasking/projects/routes.py +++ b/api/src/tasking/projects/routes.py @@ -12,6 +12,7 @@ AoiFeature, ProjectCreateRequest, ProjectListResponse, + ProjectNameValidationResponse, ProjectResponse, ProjectRoleAddRequest, ProjectRoleItem, @@ -128,6 +129,26 @@ async def create_project( ) +@router.get("/validate-name", response_model=ProjectNameValidationResponse) +async def validate_project_name( + workspace_id: int, + name: str = Query(..., min_length=1, max_length=255), + current_user: UserInfo = Depends(validate_token), + workspace_repo: WorkspaceRepository = Depends(get_workspace_repo), + project_repo: TaskingProjectRepository = Depends(get_project_repo), +): + await assert_workspace_visible(workspace_id, current_user, workspace_repo) + normalized_name = name.strip() + if not normalized_name: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail="name cannot be blank", + ) + return ProjectNameValidationResponse( + exists=await project_repo.project_name_exists(workspace_id, normalized_name) + ) + + @router.get("/{project_id}", response_model=ProjectResponse) async def get_project( workspace_id: int, diff --git a/tests/integration/test_projects_flow.py b/tests/integration/test_projects_flow.py index c1575de..088c614 100644 --- a/tests/integration/test_projects_flow.py +++ b/tests/integration/test_projects_flow.py @@ -206,6 +206,38 @@ async def test_outsider_404s_on_list( assert r.status_code == 404 +class TestProjectNameValidation: + async def test_validate_name_false_then_true( + self, client, as_lead, seeded_workspace_id + ): + """validate-name reports false before create and true after create for same workspace.""" + path = f"{API.format(wid=seeded_workspace_id)}/validate-name" + + r = await client.get(path, params={"name": "name-check"}) + assert r.status_code == 200, r.text + assert r.json() == {"exists": False} + + r = await client.post( + API.format(wid=seeded_workspace_id), + json={"name": "name-check"}, + ) + assert r.status_code == 201, r.text + + r = await client.get(path, params={"name": "name-check"}) + assert r.status_code == 200, r.text + assert r.json() == {"exists": True} + + async def test_validate_name_outsider_404( + self, client, as_outsider, seeded_workspace_id + ): + """Outsider receives 404 from tenancy gate on validate-name.""" + r = await client.get( + f"{API.format(wid=seeded_workspace_id)}/validate-name", + params={"name": "anything"}, + ) + assert r.status_code == 404 + + # --------------------------------------------------------------------------- # Workflow 3b — error mapping (constraint violations → precise HTTP status). # --------------------------------------------------------------------------- diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 20ac3f5..e9fc3ad 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -175,6 +175,14 @@ async def get(self, workspace_id: int, project_id: int): raise NotFoundException(f"Project {project_id} not found") return self._response(p) + async def project_name_exists(self, workspace_id: int, name: str) -> bool: + return any( + p["workspace_id"] == workspace_id + and p["name"] == name + and p["deleted_at"] is None + for p in self._projects.values() + ) + async def patch(self, workspace_id, project_id, body, current_user): p_resp = await self.get(workspace_id, project_id) p = self._projects[project_id] diff --git a/tests/unit/test_project_routes.py b/tests/unit/test_project_routes.py index 1620aba..792103a 100644 --- a/tests/unit/test_project_routes.py +++ b/tests/unit/test_project_routes.py @@ -1,5 +1,7 @@ from __future__ import annotations +import pytest + API = "/api/v1/workspaces/{wid}/tasking/projects" @@ -68,6 +70,17 @@ async def test_create_blank_name_422( ) assert r.status_code == 422 + @pytest.mark.parametrize("bad_value", ["not-an-object", 123, True, ["x"]]) + async def test_create_rejects_non_object_custom_imagery_422( + self, client, as_lead, seeded_workspace_id, fake_repos, bad_value + ): + """custom_imagery must be a JSON object; scalar/array values are rejected (422).""" + r = await client.post( + API.format(wid=seeded_workspace_id), + json={"name": "Pilot", "custom_imagery": bad_value}, + ) + assert r.status_code == 422 + async def test_get_404_when_missing( self, client, as_lead, seeded_workspace_id, fake_repos ): @@ -105,6 +118,24 @@ async def test_patch_name(self, client, as_lead, seeded_workspace_id, fake_repos assert r.status_code == 200 assert r.json()["name"] == "after" + @pytest.mark.parametrize("bad_value", ["not-an-object", 123, False, ["x"]]) + async def test_patch_rejects_non_object_custom_imagery_422( + self, client, as_lead, seeded_workspace_id, fake_repos, bad_value + ): + """PATCH rejects custom_imagery when it is not a JSON object (422).""" + pid = ( + await client.post( + API.format(wid=seeded_workspace_id), + json={"name": "before"}, + ) + ).json()["id"] + + r = await client.patch( + f"{API.format(wid=seeded_workspace_id)}/{pid}", + json={"custom_imagery": bad_value}, + ) + assert r.status_code == 422 + async def test_soft_delete_204_then_404( self, client, as_lead, seeded_workspace_id, fake_repos ): @@ -137,6 +168,55 @@ async def test_duplicate_name_409( assert r.status_code == 409 +class TestProjectNameValidation: + async def test_validate_name_returns_false_when_missing( + self, client, as_contributor, seeded_workspace_id, fake_repos + ): + """Validation returns exists=false when no active project uses the name.""" + r = await client.get( + f"{API.format(wid=seeded_workspace_id)}/validate-name", + params={"name": "new-project"}, + ) + assert r.status_code == 200 + assert r.json() == {"exists": False} + + async def test_validate_name_returns_true_when_exists( + self, client, as_lead, seeded_workspace_id, fake_repos + ): + """Validation returns exists=true when an active project with same name exists.""" + await client.post( + API.format(wid=seeded_workspace_id), + json={"name": "pilot-check"}, + ) + + r = await client.get( + f"{API.format(wid=seeded_workspace_id)}/validate-name", + params={"name": "pilot-check"}, + ) + assert r.status_code == 200 + assert r.json() == {"exists": True} + + async def test_validate_name_blank_rejected_422( + self, client, as_contributor, seeded_workspace_id, fake_repos + ): + """Whitespace-only name is rejected with 422.""" + r = await client.get( + f"{API.format(wid=seeded_workspace_id)}/validate-name", + params={"name": " "}, + ) + assert r.status_code == 422 + + async def test_validate_name_outsider_404( + self, client, as_outsider, seeded_workspace_id, fake_repos + ): + """Tenancy gate hides workspace existence for outsiders.""" + r = await client.get( + f"{API.format(wid=seeded_workspace_id)}/validate-name", + params={"name": "pilot-check"}, + ) + assert r.status_code == 404 + + # --------------------------------------------------------------------------- # Lifecycle gates # --------------------------------------------------------------------------- From ed1b7f45d7e9b9c27212fc4d601e6273c7f9d445 Mon Sep 17 00:00:00 2001 From: Naresh Kumar D Date: Tue, 14 Jul 2026 16:16:09 +0530 Subject: [PATCH 14/34] Update a92361f527ef_custom_imagery_and_description.py removed unnecessary pass statement --- .../versions/a92361f527ef_custom_imagery_and_description.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/alembic_osm/versions/a92361f527ef_custom_imagery_and_description.py b/alembic_osm/versions/a92361f527ef_custom_imagery_and_description.py index 6a4fdcc..6f1bc64 100644 --- a/alembic_osm/versions/a92361f527ef_custom_imagery_and_description.py +++ b/alembic_osm/versions/a92361f527ef_custom_imagery_and_description.py @@ -28,10 +28,8 @@ def upgrade() -> None: "tasking_projects", sa.Column("description", sa.String(length=10000), nullable=True), ) - pass def downgrade() -> None: op.drop_column("tasking_projects", "custom_imagery") op.drop_column("tasking_projects", "description") - pass From 1129280b57018067103b9c986b779a881c949521 Mon Sep 17 00:00:00 2001 From: Naresh Kumar D Date: Tue, 14 Jul 2026 16:26:26 +0530 Subject: [PATCH 15/34] updated required settings --- README.md | 2 +- api/src/tasking/projects/repository.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index ebba814..06ba988 100644 --- a/README.md +++ b/README.md @@ -106,4 +106,4 @@ connecting to existing Databases. `docker compose --file docker-compose.local.yml down` -Backend code will be available at `http://localhost:3000` \ No newline at end of file +Backend code will be available at `http://localhost:8000` \ No newline at end of file diff --git a/api/src/tasking/projects/repository.py b/api/src/tasking/projects/repository.py index beee8e6..95875c1 100644 --- a/api/src/tasking/projects/repository.py +++ b/api/src/tasking/projects/repository.py @@ -654,6 +654,8 @@ async def patch( updates["review_required"] = body.review_required if body.custom_imagery is not None: updates["custom_imagery"] = body.custom_imagery # type: ignore[arg-type] + if body.description is not None: + updates["description"] = body.description if updates: updates["updated_at"] = datetime.now() From 730d4883c017d36385894866a994a89d965f2eed Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Tue, 14 Jul 2026 10:18:05 -0400 Subject: [PATCH 16/34] Expand README with deployment architecture and services Added detailed deployment architecture and services information to README. --- README.md | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 73 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 06ba988..429a472 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,78 @@ following contract. `CLAUDE.md` has the full rationale. * ```develop``` merge your work here; keep this up to date with the "development" environment / dev tag * ```staging``` keep this up to date with the "staging" environment / stage tag * ```production``` keep this up to date with the "production" environment / prod tag + +## Deployment architecture + +The deployed system is defined by [`docker-compose.az.yml`](docker-compose.az.yml). It runs the +**application tier** as four containers; the **data tier** (Postgres/PostGIS) is external, managed +Azure Database for PostgreSQL, not part of this compose file. + +When deployed by `workspaces-stack` this model also holds. + +``` + client (TDEI/Keycloak JWT) + │ + ▼ :8000 + ┌──────────────────────────────────────┐ + │ workspaces-backend │ this repo — FastAPI front door. + │ authn/authz + OSM reverse proxy │ Serves /api/v1/*, proxies the rest. + └───┬──────────────────────────────┬────┘ + WS_OSM_HOST TASK_DATABASE_URL + (→ osm-rails) OSM_DATABASE_URL + │ │ + ▼ │ + ┌──────────────┐ │ + │ osm-rails │ OSM website (Rails); the single OSM entry point. + │ :3000 │ Serves the API/UI and fronts cgimap for the + └──────┬───────┘ performance-critical /api/0.6 calls. + │ (internal) │ + ▼ │ + ┌──────────────┐ │ + │ osm-cgimap │ C-accelerated /api/0.6 (map, changeset bulk) + │ :8000 │ │ + └──────────────┘ │ + │ + osm-rails-worker (rake jobs:work) │ background jobs + │ │ + │ backend, rails, cgimap, worker all connect to ▼ + ┌─────────────────────────────────────────────────────────────────┐ + │ data tier — Azure Postgres (external, PostGIS) │ + │ opensidewalks-${ENV}.postgres.database.azure.com:5432 │ + │ • workspaces-tasks-${ENV} TASK db (alembic_task; backend) │ + │ • workspaces-osm-${ENV} OSM db (alembic_osm; all four) │ + └─────────────────────────────────────────────────────────────────┘ +``` + +### Services + +| Service | Image | Role | +|---|---|---| +| `workspaces-backend` | `workspaces-backend-v2:${ENV}` | This repo. The only host-exposed service (`8000:8000`). Validates the TDEI/Keycloak JWT, enforces workspace authorization, serves `/api/v1/*`, and proxies everything else to the OSM tier. Connects to **both** databases. | +| `osm-rails` | `workspaces-osm-rails-v2:${ENV}` | The OpenStreetMap website (Rails) — the **single OSM entry point** the backend proxies to (`WS_OSM_HOST`). Serves the OSM API/UI and fronts cgimap for the heavy `/api/0.6` calls. Connects to the OSM db. | +| `osm-cgimap` | `workspaces-osm-cgimap-v2:${ENV}` | C++ reimplementation of the performance-critical OSM `0.6` calls (map queries, changeset upload/download), sitting behind `osm-rails`. Tuned here for large imports (`CGIMAP_MAX_*`). Connects to the OSM db. | +| `osm-rails-worker` | `workspaces-osm-rails-v2:${ENV}` | Background job runner (`rake jobs:work`) for the Rails app. Connects to the OSM db. | + +### Two databases + +The backend holds two connections, and the two alembic trees target them independently (see +`CLAUDE.md` and `api/utils/migrations.py`): + +* **TASK db** (`TASK_DATABASE_URL` → `workspaces-tasks-${ENV}`) — the workspaces + tasking-manager + schema, built by the `alembic_task` tree. Only the backend connects here. +* **OSM db** (`OSM_DATABASE_URL` → `workspaces-osm-${ENV}`) — OSM data plus `users` and the + `tasking_*` tables, built by the `alembic_osm` tree. The backend, cgimap, rails, and the worker + all connect here. + +On startup (outside of pytest) the backend runs `alembic -n task upgrade head` and +`alembic -n osm upgrade head`, applying each tree to its database. + +### Environment templating + +Every image tag, database name/user, and server host is parameterized by `${ENV}` +(`dev` / `stage` / `prod`), and secrets are injected from the shell environment +(`${WS_TASKS_DB_PASS}`, `${WS_OSM_DB_PASS}`, `${WS_OSM_SECRET_KEY_BASE}`). Branches map to these +environments — see the Branch Index below. ## To start on your local machine for dev work @@ -106,4 +178,4 @@ connecting to existing Databases. `docker compose --file docker-compose.local.yml down` -Backend code will be available at `http://localhost:8000` \ No newline at end of file +Backend code will be available at `http://localhost:8000` From 90ddfc5c3d122bc68c8ec22ee100c8d52b07bd4b Mon Sep 17 00:00:00 2001 From: Naresh Kumar D Date: Wed, 15 Jul 2026 14:38:06 +0530 Subject: [PATCH 17/34] Update README.md Readme updated with steps to run local development --- README.md | 61 ++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 49 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 06ba988..4a1f9fe 100644 --- a/README.md +++ b/README.md @@ -87,18 +87,55 @@ uv run black api tests && uv run isort api tests Use the file `docker-compose.local.yml` to build and deploy local code changes. This allows you to run the entire system at once instead of connecting to existing Databases. -### Initial setup. -- On first launch, rails-worker will fail because migrations are not done -- Go to the `osm-rails` `/bin/sh` and execute `bundle exec rails db:migrate` -- The above script runs the migration code for osm-rails -- The rails-worker will be able to run -- The backend code will fail first time becase `workspaces-tasks-local` database is not available -- Login to postgresql container and run the following commands - `psql --username postgres` - `create database "workspaces-tasks-local";` - `psql --username postgres --dbname "workspaces-tasks-local";` - `create extension if not exists postgis;` -- Run the backend code now and it should be able to run +### Initial setup for development local environment + +Step 1: Login to azure docker + +The docker compose relies on images in `opensidewalksdev` azure container registry. Make sure your docker system is logged into it before pulling the images and trying to run the containers. + +Docker login command: + +`docker login opensidewalksdev.azurecr.io -u opensidewalksdev ` + +Password needs to be obtained from Azure portal + +Step 2: Run docker compose for the first time + +Use the following command to start the containers first time + +`docker compose --file docker-compose.local.yml up --build` + +Step 3: Run the migration scripts. + +You will observe that only `osm-rails` component seems to work but the backend and other services may be down. this is because the database migrations on the base osm database are not done. To do the base migrations, do the following: + +- Connect to the `osm-rails` container. If you are using docker hub for desktop, just go to the exec section of the container. + If you want to use command line, execute the command `docker exec -it /bin/bash` where `container_name` is the name of osm-rails container +- Execute the migration script in the /bin/bash with `bundle exec rails db:migrate` +- The above command runs the migration script for databases + +Step 4: Add `workspaces-tasks-local` database in postgresql + +Workspaces backend relies on an additional database. This is needed for some older migrations code. + +- Connect to `database` container. +- Run the following set of commands one by one + +```shell +psql --username postgres +create database "workspaces-tasks-local"; +exit; +psql --username postgres --dbname "workspaces-tasks-local"; +create extension if not exists postgis; + +``` + +Step 5: Restart the docker compose again + +- `docker compose --file docker-compose.local.yml down` +- `docker compose --file docker-compose.local.yml up --build` + + ### Commands to start and stop the docker compose From 7b587c48711715556cc8a73f4fb817fb2ee14dea Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Thu, 9 Jul 2026 11:16:04 -0400 Subject: [PATCH 18/34] Defensive check on changeset resolve plus tests --- CLAUDE.md | 38 ++++++++--- api/src/osm/repository.py | 17 ++++- api/src/osm/routes.py | 2 +- tests/integration/test_osm.py | 117 ++++++++++++++++++++++++++++++++++ 4 files changed, 161 insertions(+), 13 deletions(-) create mode 100644 tests/integration/test_osm.py diff --git a/CLAUDE.md b/CLAUDE.md index 8e25c07..1e5e9d6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -79,24 +79,42 @@ at all). Validated against the code: * **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 +* **Edit POSM Element** — goes 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. + proxied traffic. Raw changeset commits (proxied `PUT /api/0.6/changeset/...`) + are likewise Contributor-gated; the proxy only *tags* a contributor's new + changeset with `review_requested=yes` when the workspace has `autoFlagReview` + set — it does not enforce validation. -**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. +**Enforced here, Validator-gated (`isWorkspaceLead || isWorkspaceValidator` → +403).** This is a native FastAPI route, not proxied traffic. Leads (and POC via +`isWorkspaceLead`) inherit it: + +| Capability | Endpoint | +|---|---| +| Resolve/Validate Changeset | PUT `/workspaces/{id}/changesets/{changeset_id}/resolve` | + +Resolving clears the `review_requested` tag and stamps `reviewed_by` with the +reviewer's UUID. The gate is enforced both in the route +(`api/src/osm/routes.py`) and, defensively, inside +`OSMRepository.resolveChangeset` (`api/src/osm/repository.py`). + +**The Validator role grants exactly one thing at this layer:** the ability to +resolve changesets via the endpoint above. Aside from that, a Validator and a +Contributor have identical permissions in this backend. `isWorkspaceValidator` +otherwise only appears in the `role` field of `WorkspaceResponse`. **"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. +Changeset *resolution* is Validator/Lead-gated here (see above). But the +Validator/Lead distinction on raw changeset *commits* and TDEI export is not +enforced at this layer; if required, it must be enforced downstream +(`workspaces-openstreetmap-website/`, `workspaces-cgimap/`) — that has not been +audited here. ## Testing diff --git a/api/src/osm/repository.py b/api/src/osm/repository.py index 5ca1c9a..5a29b7e 100644 --- a/api/src/osm/repository.py +++ b/api/src/osm/repository.py @@ -1,7 +1,8 @@ from sqlalchemy import text from sqlmodel.ext.asyncio.session import AsyncSession -from api.core.exceptions import NotFoundException +from api.core.exceptions import ForbiddenException, NotFoundException +from api.core.security import UserInfo class OSMRepository: @@ -50,10 +51,22 @@ async def getChangesetAdiff(self, workspace_id: int, changeset_id: int) -> list: async def resolveChangeset( self, + current_user: UserInfo, workspace_id: int, changeset_id: int, - reviewer_uuid: str, ) -> None: + # Defense in depth: resolving a changeset is a validator/lead + # capability. The route also enforces this, but gate here too so the + # repository cannot be misused from another call site. + if not current_user.isWorkspaceLead( + workspace_id + ) and not current_user.isWorkspaceValidator(workspace_id): + raise ForbiddenException( + "Only workspace leads and validators can resolve changesets" + ) + + reviewer_uuid = str(current_user.user_uuid) + await self.session.execute( text(f"SET search_path TO 'workspace-{int(workspace_id)}', public") ) diff --git a/api/src/osm/routes.py b/api/src/osm/routes.py index 452b2d4..9542e5a 100644 --- a/api/src/osm/routes.py +++ b/api/src/osm/routes.py @@ -72,7 +72,7 @@ async def resolve_changeset( try: await repository_osm.resolveChangeset( - workspace_id, changeset_id, str(current_user.user_uuid) + current_user, workspace_id, changeset_id ) except Exception as e: logger.error( diff --git a/tests/integration/test_osm.py b/tests/integration/test_osm.py new file mode 100644 index 0000000..b4862d8 --- /dev/null +++ b/tests/integration/test_osm.py @@ -0,0 +1,117 @@ +"""Integration tests for the /workspaces OSM routes (api/src/osm/routes.py). + +Each test drives a real HTTP request through the real route + repository, +queueing simulated rows on the fake sessions. + +Focus: PUT /{id}/changesets/{cid}/resolve is gated to workspace leads and +validators (403 otherwise). The gate is enforced both in the route and, +defensively, inside ``OSMRepository.resolveChangeset`` -- so a contributor is +rejected before any DB work, and the repository would reject a bad call site +even if the route check were bypassed. +""" + +import pytest + +from api.src.users.schemas import WorkspaceUserRoleType +from tests.support import factories, fakes + +API = "/api/v1/workspaces" + + +def _resolve_url(workspace_id=1, changeset_id=99): + return f"{API}/{workspace_id}/changesets/{changeset_id}/resolve" + + +def _user_with_role(role, workspace_id=1): + return factories.make_user_info(osm_workspace_roles={workspace_id: [role]}) + + +# === PUT /{id}/changesets/{cid}/resolve ==================================== + + +async def test_resolve_changeset_validator_204( + client, login, task_session, osm_session +): + login(_user_with_role(WorkspaceUserRoleType.VALIDATOR)) + task_session.queue(fakes.rows(factories.make_workspace(id=1))) # getById + osm_session.queue(fakes.affected(1), fakes.affected(1)) # DELETE, INSERT + + response = await client.put(_resolve_url()) + + assert response.status_code == 204 + assert osm_session.commits == 1 # resolveChangeset ran to completion + + +async def test_resolve_changeset_lead_204(client, login, task_session, osm_session): + login(_user_with_role(WorkspaceUserRoleType.LEAD)) + task_session.queue(fakes.rows(factories.make_workspace(id=1))) + osm_session.queue(fakes.affected(1), fakes.affected(1)) + + response = await client.put(_resolve_url()) + + assert response.status_code == 204 + assert osm_session.commits == 1 + + +async def test_resolve_changeset_poc_204(client, login, task_session, osm_session): + # POC on the owning project group satisfies isWorkspaceLead. + login( + factories.make_user_info( + accessible_workspace_ids={factories.DEFAULT_PG_ID: [1]}, + poc_group_ids=(factories.DEFAULT_PG_ID,), + ) + ) + task_session.queue(fakes.rows(factories.make_workspace(id=1))) + osm_session.queue(fakes.affected(1), fakes.affected(1)) + + response = await client.put(_resolve_url()) + + assert response.status_code == 204 + assert osm_session.commits == 1 + + +async def test_resolve_changeset_contributor_403( + client, login, task_session, osm_session +): + # A contributor (PG association, no validator/lead grant) is rejected + # before any DB work -- neither session should be touched. + login( + factories.make_user_info( + accessible_workspace_ids={factories.DEFAULT_PG_ID: [1]} + ) + ) + + response = await client.put(_resolve_url()) + + assert response.status_code == 403 + assert osm_session.commits == 0 + + +async def test_resolve_changeset_no_access_403(client, login): + # A user with no association to the workspace is likewise forbidden. + login() + + response = await client.put(_resolve_url()) + + assert response.status_code == 403 + + +async def test_resolve_changeset_validator_of_other_workspace_403(client, login): + # Validator rights on workspace 2 do not authorize resolving on workspace 1. + login(_user_with_role(WorkspaceUserRoleType.VALIDATOR, workspace_id=2)) + + response = await client.put(_resolve_url(workspace_id=1)) + + assert response.status_code == 403 + + +async def test_resolve_changeset_unexpected_error_500( + error_client, login, task_session, osm_session +): + login(_user_with_role(WorkspaceUserRoleType.VALIDATOR)) + task_session.queue(fakes.rows(factories.make_workspace(id=1))) + osm_session.queue(fakes.raises(RuntimeError("db"))) # DELETE blows up + + response = await error_client.put(_resolve_url()) + + assert response.status_code == 500 From 2d9c4a61a1613636ca4c4773b3997cdb8a9eed33 Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Thu, 9 Jul 2026 11:26:53 -0400 Subject: [PATCH 19/34] Update routes.py --- api/src/osm/routes.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/api/src/osm/routes.py b/api/src/osm/routes.py index 9542e5a..cadee1e 100644 --- a/api/src/osm/routes.py +++ b/api/src/osm/routes.py @@ -71,9 +71,7 @@ async def resolve_changeset( await repository_ws.getById(current_user, workspace_id) try: - await repository_osm.resolveChangeset( - current_user, workspace_id, changeset_id - ) + await repository_osm.resolveChangeset(current_user, workspace_id, changeset_id) except Exception as e: logger.error( f"Failed to resolve changeset {changeset_id}" From 89978376abb03009190934f2039c1aafb37bcb83 Mon Sep 17 00:00:00 2001 From: Naresh Kumar D Date: Fri, 10 Jul 2026 15:43:55 +0530 Subject: [PATCH 20/34] Deploy code feature Deploying the required code --- .github/workflows/trigger-deploy-on-merge.yml | 36 +++++++++++++++++++ api/core/config.py | 8 ++--- api/main.py | 4 ++- 3 files changed, 42 insertions(+), 6 deletions(-) create mode 100644 .github/workflows/trigger-deploy-on-merge.yml diff --git a/.github/workflows/trigger-deploy-on-merge.yml b/.github/workflows/trigger-deploy-on-merge.yml new file mode 100644 index 0000000..82265c8 --- /dev/null +++ b/.github/workflows/trigger-deploy-on-merge.yml @@ -0,0 +1,36 @@ +name: Trigger Deployment on Pull Request Merge +on: + pull_request: + types: [closed] + branches: [ develop, staging, production] +permissions: + contents: read + actions: write +jobs: + on-merge: + if: github.event.pull_request.merged == true + runs-on: ubuntu-latest + outputs: + environment: ${{ steps.set-environment.outputs.environment }} + steps: + - name: Set Environment + id: set-environment + run: | + case "${{ github.base_ref }}" in + develop) echo "environment=develop" >> $GITHUB_OUTPUT ;; + staging) echo "environment=staging" >> $GITHUB_OUTPUT ;; + production) echo "environment=production" >> $GITHUB_OUTPUT ;; + testing) echo "environment=testing" >> $GITHUB_OUTPUT ;; + esac + deploy: + needs: on-merge + runs-on: ubuntu-latest + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + steps: + - name: Trigger Deployment Workflow + run: | + gh workflow run push-docker-image.yml \ + --repo ${{ github.repository }} \ + --ref ${{ github.base_ref }} \ + --field environment=${{ needs.on-merge.outputs.environment }} \ No newline at end of file diff --git a/api/core/config.py b/api/core/config.py index c4c6bde..643e586 100644 --- a/api/core/config.py +++ b/api/core/config.py @@ -10,11 +10,9 @@ class Settings(BaseSettings): PROJECT_NAME: str = "Workspaces API" - # JSON array of allowed CORS origins. For example: - # - # ["https://workspaces.example.com", "https://leaderboard.example.com"] - # - CORS_ORIGINS: list[str] = [] + # Comma separated list of origins, or "*" to allow all origins. Defaults to an empty list. + # Example: CORS_ORIGINS="https://workspaces.example.com,https://leaderboard.example.com" + CORS_ORIGINS: str = "" TASK_DATABASE_URL: str = ( "postgresql+asyncpg://user:pass@localhost:5432/tasking_manager" diff --git a/api/main.py b/api/main.py index d537b72..9123a4e 100644 --- a/api/main.py +++ b/api/main.py @@ -97,7 +97,9 @@ async def lifespan(_app: FastAPI): app.add_middleware( CORSMiddleware, - allow_origins=settings.CORS_ORIGINS, + allow_origins=[ + origin.strip() for origin in settings.CORS_ORIGINS.split(",") if origin.strip() + ], allow_credentials=True, allow_methods=["*"], allow_headers=["*"], From 3d9ea37b4d97c414c2be02ec56315e7ae6ec6563 Mon Sep 17 00:00:00 2001 From: Naresh Kumar D Date: Fri, 10 Jul 2026 15:49:29 +0530 Subject: [PATCH 21/34] Update test_config.py Updated unit tests for CORS --- tests/unit/test_config.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 3a06d3b..4a9ab45 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -14,7 +14,7 @@ def test_defaults_loaded_when_env_unset(): 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.CORS_ORIGINS == "" assert s.DEBUG is False assert s.SENTRY_DSN == "" assert s.WS_OSM_HOST == "http://osm-web" @@ -38,18 +38,18 @@ def test_env_vars_override_members(monkeypatch): def test_cors_origins_parsed_from_json_env(monkeypatch): - monkeypatch.setenv("CORS_ORIGINS", '["https://a.example", "https://b.example"]') + 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"] + 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.CORS_ORIGINS, str) assert isinstance(s.DEBUG, bool) # empty-string default is preserved (not coerced to None): assert s.SENTRY_DSN == "" From e9456cd504b334f2202322baac67713acbacef3a Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Fri, 10 Jul 2026 15:11:17 -0400 Subject: [PATCH 22/34] Enable osm-web API endpoints with auth token --- api/core/config.py | 15 +++ api/core/security.py | 139 ++++++++++++++++++++++ tests/unit/test_token_bridge.py | 197 ++++++++++++++++++++++++++++++++ 3 files changed, 351 insertions(+) create mode 100644 tests/unit/test_token_bridge.py diff --git a/api/core/config.py b/api/core/config.py index 643e586..7911264 100644 --- a/api/core/config.py +++ b/api/core/config.py @@ -38,6 +38,21 @@ class Settings(BaseSettings): # proxy destination--"osm-web" is a virtual docker network endpoint WS_OSM_HOST: str = "http://osm-web" + # OSM token bridge: when a TDEI token is validated, mirror it into the OSM + # database's `oauth_access_tokens` table so osm-rails (doorkeeper) and cgimap + # authenticate it via their standard OAuth2 path -- no custom JWT handling + # needed in those services. Disabled while WS_OSM_OAUTH_APPLICATION_ID is 0. + # Set it to the id of the doorkeeper `oauth_applications` row these tokens + # should belong to (create one via the OSM `register_apps` rake task or SQL). + WS_OSM_OAUTH_APPLICATION_ID: int = 0 + + # Scopes granted to the mirrored token. Must cover the OSM API operations the + # frontend performs; see `lib/oauth.rb` in the OSM website for valid values. + WS_OSM_OAUTH_SCOPES: str = ( + "read_prefs write_prefs write_api write_changeset_comments " + "read_gpx write_gpx write_notes" + ) + SENTRY_DSN: str = "" model_config = SettingsConfigDict( diff --git a/api/core/security.py b/api/core/security.py index 0ff32e2..1db25f9 100644 --- a/api/core/security.py +++ b/api/core/security.py @@ -1,3 +1,4 @@ +import time from enum import StrEnum from uuid import UUID @@ -19,6 +20,8 @@ # @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 +# @test: Test that a validated token is mirrored into the OSM oauth_access_tokens table (with the user provisioned and expires_in from the JWT exp) when WS_OSM_OAUTH_APPLICATION_ID is set, and is a no-op otherwise +# @test: Test that re-presenting a token reactivates its OSM row (revoked_at cleared, expiry refreshed) and that a rotated (superseded) token is revoked, both gated on WS_OSM_OAUTH_APPLICATION_ID # Set up logger for this module logger = get_logger(__name__) @@ -316,6 +319,14 @@ async def validate_token( logger.info("Token validation cache hit") return cached logger.info("Token validation cache miss: token rotated") + # The old token is superseded; revoke its OSM row so it stops + # authenticating against osm-rails/cgimap before its own exp. + # TODO: this assumes a single active token per user. Truly concurrent + # sessions (multiple valid jtis for one user) will flap -- each request + # revokes the other session's OSM token and reactivates its own via the + # DO UPDATE upsert. To support multi-session, track multiple active jtis + # per user (cache + revoke set) instead of a single cached token. + await _revoke_osm_token(osm_db_session, cached.credentials) del _user_info_cache[user_uuid] # Cache miss: fetch TDEI roles and DB data: @@ -327,6 +338,123 @@ async def validate_token( return user_info +async def _bridge_token_to_osm( + session: AsyncSession, + *, + user_uuid: UUID, + user_name: str, + email: str | None, + token: str, + exp: int | None, +) -> None: + """Mirror a validated TDEI token into the OSM database so osm-rails + (doorkeeper) and cgimap authenticate it through their standard OAuth2 path, + with no custom JWT handling required in those services. + + Two idempotent writes: provision the OSM ``users`` row (owns the token via + ``resource_owner_id``) and insert a plaintext ``oauth_access_tokens`` row. + Doorkeeper stores tokens plaintext (``SecretStoring::Plain``), so the raw JWT + matches on lookup for both osm-rails and cgimap; ``expires_in`` tracks the + JWT's own ``exp`` so OSM expires it in lockstep with TDEI. + + No-op unless ``WS_OSM_OAUTH_APPLICATION_ID`` is set. Best-effort: a failure + here must not break token validation -- the ``/api/v1`` routes keep working; + only the proxied OSM calls would 401 until the row exists. + """ + if settings.WS_OSM_OAUTH_APPLICATION_ID <= 0: + return + + auth_uid = str(user_uuid) + try: + # Provision the users row (same shape as _provision_users_from_tdei). + await session.execute( + text( + "INSERT INTO users (auth_uid, email, display_name, auth_provider, " + "status, pass_crypt, data_public, email_valid, terms_seen, " + "creation_time, terms_agreed, tou_agreed) " + "VALUES (:auth_uid, :email, :name, 'TDEI', 'active', 'none', " + "true, true, true, (now() at time zone 'utc'), " + "(now() at time zone 'utc'), (now() at time zone 'utc')) " + "ON CONFLICT (auth_uid) DO NOTHING" + ), + {"auth_uid": auth_uid, "email": email, "name": user_name}, + ) + row = ( + await session.execute( + text( + "SELECT id FROM users " + "WHERE auth_provider = 'TDEI' AND auth_uid = :auth_uid" + ), + {"auth_uid": auth_uid}, + ) + ).first() + if row is None: + return + user_id = row[0] + + expires_in = max(0, exp - int(time.time())) if exp else None + await session.execute( + text( + "INSERT INTO oauth_access_tokens (application_id, " + "resource_owner_id, token, scopes, created_at, expires_in) " + "VALUES (:app_id, :user_id, :token, :scopes, " + "(now() at time zone 'utc'), :expires_in) " + # Re-presenting a token reactivates it: refresh the expiry to + # track this validation and clear any prior revocation, so a + # token revoked on rotation heals if it is used again. + "ON CONFLICT (token) DO UPDATE SET " + "resource_owner_id = EXCLUDED.resource_owner_id, " + "scopes = EXCLUDED.scopes, " + "created_at = EXCLUDED.created_at, " + "expires_in = EXCLUDED.expires_in, " + "revoked_at = NULL" + ), + { + "app_id": settings.WS_OSM_OAUTH_APPLICATION_ID, + "user_id": user_id, + "token": token, + "scopes": settings.WS_OSM_OAUTH_SCOPES, + "expires_in": expires_in, + }, + ) + await session.commit() + except Exception as e: + # Never fail auth on a bridge error; the OSM row just won't exist yet. + logger.warning( + "Failed to bridge TDEI token into OSM oauth_access_tokens: %s", e + ) + await session.rollback() + + +async def _revoke_osm_token(session: AsyncSession, token: str) -> None: + """Revoke a superseded token's OSM ``oauth_access_tokens`` row (best-effort). + + Called when a user's token rotates (new ``jti``) so the previous JWT stops + authenticating against osm-rails/cgimap before its own ``exp`` rather than + lingering until it expires. No-op unless the bridge is configured. + + Note: OSM/cgimap are reachable only *through* this proxy, and every request + first passes ``validate_token`` -- so a TDEI-revoked token is already + rejected upstream. This revocation is defense-in-depth for the superseded + token and keeps the OSM token store tidy. + """ + if settings.WS_OSM_OAUTH_APPLICATION_ID <= 0: + return + try: + await session.execute( + text( + "UPDATE oauth_access_tokens " + "SET revoked_at = (now() at time zone 'utc') " + "WHERE token = :token AND revoked_at IS NULL" + ), + {"token": token}, + ) + await session.commit() + except Exception as e: + logger.warning("Failed to revoke superseded OSM token: %s", e) + await session.rollback() + + async def _validate_token_uncached( token: str, user_uuid: UUID, @@ -427,6 +555,17 @@ async def _validate_token_uncached( osmRoles[i["workspace_id"]].append(i["role"]) r.osmWorkspaceRoles = osmRoles + # Mirror this validated token into the OSM DB so proxied OSM/cgimap calls + # authenticate via the standard OAuth2 path. No-op unless configured. + await _bridge_token_to_osm( + osm_db_session, + user_uuid=user_uuid, + user_name=r.user_name, + email=payload.get("email"), + token=token, + exp=payload.get("exp"), + ) + logger.info("Finished validation of token") return r diff --git a/tests/unit/test_token_bridge.py b/tests/unit/test_token_bridge.py new file mode 100644 index 0000000..401ad73 --- /dev/null +++ b/tests/unit/test_token_bridge.py @@ -0,0 +1,197 @@ +"""Tests for the OSM token bridge in ``api/core/security.py``. + +``_bridge_token_to_osm`` mirrors a validated TDEI token into the OSM database's +``oauth_access_tokens`` table so osm-rails (doorkeeper) and cgimap authenticate +it through their standard OAuth2 path. We cover: + +- it is a no-op unless ``WS_OSM_OAUTH_APPLICATION_ID`` is configured, +- when enabled it provisions the ``users`` row and inserts a plaintext + ``oauth_access_tokens`` row owned by that user, with ``expires_in`` derived + from the JWT ``exp``, +- a DB failure never propagates (auth must not break) and rolls back. +""" + +import time +from typing import cast +from uuid import uuid4 + +from sqlmodel.ext.asyncio.session import AsyncSession + +import api.core.security as security +from api.core.config import settings + + +class RecordingSession: + """Async session stand-in that records (sql, params) and returns a user id + row for the ``SELECT id FROM users`` lookup.""" + + def __init__(self, user_id=99): + self.calls: list[tuple[str, dict]] = [] + self.commits = 0 + self.rollbacks = 0 + self._user_id = user_id + + async def execute(self, statement, params=None): + sql = str(statement) + self.calls.append((sql, params or {})) + + class _R: + def __init__(self, rows): + self._rows = rows + + def first(self): + return self._rows[0] if self._rows else None + + if "SELECT id FROM users" in sql: + return _R([(self._user_id,)]) + return _R([]) + + async def commit(self): + self.commits += 1 + + async def rollback(self): + self.rollbacks += 1 + + def sql_for(self, needle): + return [c for c in self.calls if needle in c[0]] + + +async def test_bridge_is_noop_when_disabled(monkeypatch): + """With WS_OSM_OAUTH_APPLICATION_ID == 0 the bridge touches no DB.""" + monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 0) + session = RecordingSession() + + await security._bridge_token_to_osm( + cast(AsyncSession, session), + user_uuid=uuid4(), + user_name="alice", + email="alice@example.com", + token="tok", + exp=None, + ) + + assert session.calls == [] + assert session.commits == 0 + + +async def test_bridge_provisions_user_and_mirrors_token(monkeypatch): + """When enabled: users upsert -> id lookup -> oauth_access_tokens upsert.""" + monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 7) + monkeypatch.setattr(settings, "WS_OSM_OAUTH_SCOPES", "write_api read_prefs") + uid = uuid4() + exp = int(time.time()) + 3600 + session = RecordingSession(user_id=42) + + await security._bridge_token_to_osm( + cast(AsyncSession, session), + user_uuid=uid, + user_name="alice", + email="alice@example.com", + token="jwt-token", + exp=exp, + ) + + # users row provisioned idempotently, keyed by auth_uid = str(sub) + user_inserts = session.sql_for("INSERT INTO users") + assert user_inserts and "ON CONFLICT (auth_uid)" in user_inserts[0][0] + assert user_inserts[0][1]["auth_uid"] == str(uid) + assert user_inserts[0][1]["email"] == "alice@example.com" + + # token mirrored with the configured application/scopes and exp-derived TTL + token_inserts = session.sql_for("oauth_access_tokens") + assert token_inserts and "ON CONFLICT (token)" in token_inserts[0][0] + # Re-presenting a token must reactivate it (clear revocation, refresh expiry). + assert "DO UPDATE" in token_inserts[0][0] + assert "revoked_at = NULL" in token_inserts[0][0] + params = token_inserts[0][1] + assert params["app_id"] == 7 + assert params["user_id"] == 42 + assert params["token"] == "jwt-token" + assert params["scopes"] == "write_api read_prefs" + assert 3590 <= params["expires_in"] <= 3600 + + assert session.commits == 1 + assert session.rollbacks == 0 + + +async def test_bridge_expires_in_none_without_exp(monkeypatch): + """A token without an `exp` claim mirrors with a NULL expires_in.""" + monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 7) + session = RecordingSession() + + await security._bridge_token_to_osm( + cast(AsyncSession, session), + user_uuid=uuid4(), + user_name="alice", + email=None, + token="jwt-token", + exp=None, + ) + + params = session.sql_for("oauth_access_tokens")[0][1] + assert params["expires_in"] is None + + +async def test_bridge_is_best_effort_on_db_error(monkeypatch): + """A DB failure must not propagate out of validation; it rolls back.""" + monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 7) + + class BoomSession(RecordingSession): + async def execute(self, statement, params=None): + raise RuntimeError("db down") + + session = BoomSession() + + # Must not raise. + await security._bridge_token_to_osm( + cast(AsyncSession, session), + user_uuid=uuid4(), + user_name="alice", + email=None, + token="tok", + exp=None, + ) + + assert session.rollbacks == 1 + assert session.commits == 0 + + +async def test_revoke_is_noop_when_disabled(monkeypatch): + """With the bridge off, revoking a rotated token touches no DB.""" + monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 0) + session = RecordingSession() + + await security._revoke_osm_token(cast(AsyncSession, session), "old-token") + + assert session.calls == [] + assert session.commits == 0 + + +async def test_revoke_marks_only_the_superseded_token(monkeypatch): + """Revocation flips revoked_at for exactly the given token, if not already.""" + monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 7) + session = RecordingSession() + + await security._revoke_osm_token(cast(AsyncSession, session), "old-token") + + updates = session.sql_for("UPDATE oauth_access_tokens") + assert updates and "revoked_at" in updates[0][0] + assert "WHERE token = :token AND revoked_at IS NULL" in updates[0][0] + assert updates[0][1]["token"] == "old-token" + assert session.commits == 1 + + +async def test_revoke_is_best_effort_on_db_error(monkeypatch): + """A DB failure during revocation must not propagate; it rolls back.""" + monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 7) + + class BoomSession(RecordingSession): + async def execute(self, statement, params=None): + raise RuntimeError("db down") + + session = BoomSession() + + await security._revoke_osm_token(cast(AsyncSession, session), "tok") + + assert session.rollbacks == 1 + assert session.commits == 0 From 474f4230e6ebe5de3876b2ec1d109df31a502d50 Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Fri, 10 Jul 2026 15:48:01 -0400 Subject: [PATCH 23/34] CORS config file fix --- api/core/config.py | 30 ++++++++++++++++++++++++++++-- api/main.py | 4 +--- tests/unit/test_config.py | 37 +++++++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 5 deletions(-) diff --git a/api/core/config.py b/api/core/config.py index 7911264..dbda4b3 100644 --- a/api/core/config.py +++ b/api/core/config.py @@ -1,3 +1,5 @@ +import json + from pydantic_settings import BaseSettings, SettingsConfigDict @@ -10,8 +12,12 @@ class Settings(BaseSettings): PROJECT_NAME: str = "Workspaces API" - # Comma separated list of origins, or "*" to allow all origins. Defaults to an empty list. - # Example: CORS_ORIGINS="https://workspaces.example.com,https://leaderboard.example.com" + # Allowed CORS origins. Accepts either a comma-separated list OR a JSON + # array (the deployment stack historically supplies a JSON array), plus "*" + # for all origins. Read via `cors_origins_list`, not this raw string. + # Examples: + # CORS_ORIGINS="https://workspaces.example.com,https://leaderboard.example.com" + # CORS_ORIGINS='["https://workspaces.example.com"]' CORS_ORIGINS: str = "" TASK_DATABASE_URL: str = ( @@ -55,6 +61,26 @@ class Settings(BaseSettings): SENTRY_DSN: str = "" + @property + def cors_origins_list(self) -> list[str]: + """Allowed CORS origins as a list. + + Tolerates both formats the deployment has used: a JSON array + (``["https://a","https://b"]``) and a comma-separated string + (``https://a,https://b``). ``"*"`` is passed through as-is. + """ + raw = self.CORS_ORIGINS.strip() + if not raw: + return [] + if raw.startswith("["): + try: + parsed = json.loads(raw) + except json.JSONDecodeError: + parsed = None + if isinstance(parsed, list): + return [str(o).strip() for o in parsed if str(o).strip()] + return [o.strip() for o in raw.split(",") if o.strip()] + model_config = SettingsConfigDict( env_file=".env", env_file_encoding="utf-8", diff --git a/api/main.py b/api/main.py index 9123a4e..c3708b3 100644 --- a/api/main.py +++ b/api/main.py @@ -97,9 +97,7 @@ async def lifespan(_app: FastAPI): app.add_middleware( CORSMiddleware, - allow_origins=[ - origin.strip() for origin in settings.CORS_ORIGINS.split(",") if origin.strip() - ], + allow_origins=settings.cors_origins_list, allow_credentials=True, allow_methods=["*"], allow_headers=["*"], diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 4a9ab45..cc96fa0 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -45,6 +45,43 @@ def test_cors_origins_parsed_from_json_env(monkeypatch): assert s.CORS_ORIGINS == "https://a.example,https://b.example" +def test_cors_origins_list_comma_separated(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_list == ["https://a.example", "https://b.example"] + + +def test_cors_origins_list_json_array(monkeypatch): + # The deployment stack supplies a JSON array; it must parse, not become one + # malformed bracketed origin. + 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_list == ["https://a.example", "https://b.example"] + + +def test_cors_origins_list_single_json_array(monkeypatch): + monkeypatch.setenv("CORS_ORIGINS", '["https://only.example"]') + s = Settings(_env_file=None) # type: ignore[call-arg] # pydantic-settings init kwarg + assert s.cors_origins_list == ["https://only.example"] + + +def test_cors_origins_list_empty_and_wildcard(monkeypatch): + s = Settings(_env_file=None) # type: ignore[call-arg] # pydantic-settings init kwarg + assert s.cors_origins_list == [] + + monkeypatch.setenv("CORS_ORIGINS", "*") + s = Settings(_env_file=None) # type: ignore[call-arg] # pydantic-settings init kwarg + assert s.cors_origins_list == ["*"] + + +def test_cors_origins_list_malformed_json_falls_back_to_comma(monkeypatch): + # A value that starts with "[" but isn't valid JSON should not crash; fall + # back to comma-splitting rather than raising. + monkeypatch.setenv("CORS_ORIGINS", "[not json") + s = Settings(_env_file=None) # type: ignore[call-arg] # pydantic-settings init kwarg + assert s.cors_origins_list == ["[not json"] + + def test_value_types_and_formats(): s = Settings(_env_file=None) # type: ignore[call-arg] # pydantic-settings init kwarg From 6a9b034d091936f987a8273652cf66cc32343b87 Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Fri, 10 Jul 2026 16:21:49 -0400 Subject: [PATCH 24/34] Tweak to enable OSM token bridge by default --- api/core/config.py | 32 +++++--- api/core/security.py | 137 +++++++++++++++++++++++--------- tests/unit/test_token_bridge.py | 98 ++++++++++++++--------- 3 files changed, 181 insertions(+), 86 deletions(-) diff --git a/api/core/config.py b/api/core/config.py index dbda4b3..04693ac 100644 --- a/api/core/config.py +++ b/api/core/config.py @@ -44,21 +44,33 @@ class Settings(BaseSettings): # proxy destination--"osm-web" is a virtual docker network endpoint WS_OSM_HOST: str = "http://osm-web" - # OSM token bridge: when a TDEI token is validated, mirror it into the OSM - # database's `oauth_access_tokens` table so osm-rails (doorkeeper) and cgimap - # authenticate it via their standard OAuth2 path -- no custom JWT handling - # needed in those services. Disabled while WS_OSM_OAUTH_APPLICATION_ID is 0. - # Set it to the id of the doorkeeper `oauth_applications` row these tokens - # should belong to (create one via the OSM `register_apps` rake task or SQL). - WS_OSM_OAUTH_APPLICATION_ID: int = 0 - - # Scopes granted to the mirrored token. Must cover the OSM API operations the - # frontend performs; see `lib/oauth.rb` in the OSM website for valid values. + # OSM token bridge: when enabled, a validated TDEI token is mirrored into the + # OSM database's `oauth_access_tokens` table so osm-rails (doorkeeper) and + # cgimap authenticate it via their standard OAuth2 path -- no custom JWT + # handling needed in those services. The backend also auto-creates the + # doorkeeper `oauth_applications` row (keyed by WS_OSM_OAUTH_CLIENT_UID and + # owned by the dedicated system user below) that these tokens belong to, so + # no manual OSM setup is required. + WS_OSM_TOKEN_BRIDGE_ENABLED: bool = True + + # Stable client id (uid) for the auto-created doorkeeper application. Point + # this at an existing application's uid to reuse it instead of creating one. + WS_OSM_OAUTH_CLIENT_UID: str = "workspaces-backend" + + # Scopes granted to the application and mirrored tokens. Must cover the OSM + # API operations the frontend performs; see `lib/oauth.rb` in the OSM website. WS_OSM_OAUTH_SCOPES: str = ( "read_prefs write_prefs write_api write_changeset_comments " "read_gpx write_gpx write_notes" ) + # Dedicated OSM `users` row that owns the auto-created doorkeeper application + # (satisfies oauth_applications.owner_id, a NOT-NULL FK to users). It never + # signs in; these values just need to be stable and unique among users. + WS_OSM_SYSTEM_USER_AUTH_UID: str = "workspaces-backend-system" + WS_OSM_SYSTEM_USER_DISPLAY_NAME: str = "Workspaces Backend (system)" + WS_OSM_SYSTEM_USER_EMAIL: str = "workspaces-backend-system@tdei.us" + SENTRY_DSN: str = "" @property diff --git a/api/core/security.py b/api/core/security.py index 1db25f9..adeaa38 100644 --- a/api/core/security.py +++ b/api/core/security.py @@ -1,3 +1,4 @@ +import secrets import time from enum import StrEnum from uuid import UUID @@ -20,8 +21,8 @@ # @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 -# @test: Test that a validated token is mirrored into the OSM oauth_access_tokens table (with the user provisioned and expires_in from the JWT exp) when WS_OSM_OAUTH_APPLICATION_ID is set, and is a no-op otherwise -# @test: Test that re-presenting a token reactivates its OSM row (revoked_at cleared, expiry refreshed) and that a rotated (superseded) token is revoked, both gated on WS_OSM_OAUTH_APPLICATION_ID +# @test: Test that when WS_OSM_TOKEN_BRIDGE_ENABLED, a validated token is mirrored into oauth_access_tokens with the doorkeeper application + system-owner user + caller user auto-provisioned and expires_in from the JWT exp; and is a no-op when disabled +# @test: Test that re-presenting a token reactivates its OSM row (revoked_at cleared, expiry refreshed) and that a rotated (superseded) token is revoked, both gated on WS_OSM_TOKEN_BRIDGE_ENABLED # Set up logger for this module logger = get_logger(__name__) @@ -338,6 +339,84 @@ async def validate_token( return user_info +async def _ensure_osm_user( + session: AsyncSession, + *, + auth_uid: str, + email: str | None, + display_name: str, +) -> int | None: + """Idempotently provision an OSM ``users`` row (auth_provider ``TDEI``) and + return its id. ``email`` is synthesised when absent -- the column is UNIQUE + and NOT NULL.""" + await session.execute( + text( + "INSERT INTO users (auth_uid, email, display_name, auth_provider, " + "status, pass_crypt, data_public, email_valid, terms_seen, " + "creation_time, terms_agreed, tou_agreed) " + "VALUES (:auth_uid, :email, :name, 'TDEI', 'active', 'none', " + "true, true, true, (now() at time zone 'utc'), " + "(now() at time zone 'utc'), (now() at time zone 'utc')) " + "ON CONFLICT (auth_uid) DO NOTHING" + ), + { + "auth_uid": auth_uid, + "email": email or f"{auth_uid}@tdei.invalid", + "name": display_name, + }, + ) + row = ( + await session.execute( + text( + "SELECT id FROM users " + "WHERE auth_provider = 'TDEI' AND auth_uid = :auth_uid" + ), + {"auth_uid": auth_uid}, + ) + ).first() + return row[0] if row else None + + +async def _ensure_osm_oauth_application(session: AsyncSession) -> int | None: + """Ensure the doorkeeper application the mirrored tokens belong to exists, + creating it (owned by a dedicated system user) if needed. Idempotent by the + configured client uid; returns the application id.""" + owner_id = await _ensure_osm_user( + session, + auth_uid=settings.WS_OSM_SYSTEM_USER_AUTH_UID, + email=settings.WS_OSM_SYSTEM_USER_EMAIL, + display_name=settings.WS_OSM_SYSTEM_USER_DISPLAY_NAME, + ) + if owner_id is None: + return None + await session.execute( + text( + "INSERT INTO oauth_applications (owner_type, owner_id, name, uid, " + "secret, redirect_uri, scopes, confidential, created_at, updated_at) " + "VALUES ('User', :owner_id, :name, :uid, :secret, " + "'urn:ietf:wg:oauth:2.0:oob', :scopes, true, " + "(now() at time zone 'utc'), (now() at time zone 'utc')) " + "ON CONFLICT (uid) DO NOTHING" + ), + { + "owner_id": owner_id, + "name": "Workspaces Backend", + "uid": settings.WS_OSM_OAUTH_CLIENT_UID, + # Never used (tokens are inserted directly rather than issued via the + # OAuth flow), but the column is NOT NULL. + "secret": secrets.token_hex(32), + "scopes": settings.WS_OSM_OAUTH_SCOPES, + }, + ) + row = ( + await session.execute( + text("SELECT id FROM oauth_applications WHERE uid = :uid"), + {"uid": settings.WS_OSM_OAUTH_CLIENT_UID}, + ) + ).first() + return row[0] if row else None + + async def _bridge_token_to_osm( session: AsyncSession, *, @@ -351,46 +430,28 @@ async def _bridge_token_to_osm( (doorkeeper) and cgimap authenticate it through their standard OAuth2 path, with no custom JWT handling required in those services. - Two idempotent writes: provision the OSM ``users`` row (owns the token via - ``resource_owner_id``) and insert a plaintext ``oauth_access_tokens`` row. - Doorkeeper stores tokens plaintext (``SecretStoring::Plain``), so the raw JWT - matches on lookup for both osm-rails and cgimap; ``expires_in`` tracks the - JWT's own ``exp`` so OSM expires it in lockstep with TDEI. + Auto-provisions everything it needs: the doorkeeper ``oauth_applications`` + row (owned by a dedicated system user), the caller's ``users`` row, and a + plaintext ``oauth_access_tokens`` row -- no manual OSM setup. Doorkeeper + stores tokens plaintext (``SecretStoring::Plain``), so the raw JWT matches on + lookup for both osm-rails and cgimap; ``expires_in`` tracks the JWT's ``exp``. - No-op unless ``WS_OSM_OAUTH_APPLICATION_ID`` is set. Best-effort: a failure - here must not break token validation -- the ``/api/v1`` routes keep working; - only the proxied OSM calls would 401 until the row exists. + No-op unless ``WS_OSM_TOKEN_BRIDGE_ENABLED``. Best-effort: a failure here + must not break token validation -- the ``/api/v1`` routes keep working; only + the proxied OSM calls would 401 until the row exists. """ - if settings.WS_OSM_OAUTH_APPLICATION_ID <= 0: + if not settings.WS_OSM_TOKEN_BRIDGE_ENABLED: return - auth_uid = str(user_uuid) try: - # Provision the users row (same shape as _provision_users_from_tdei). - await session.execute( - text( - "INSERT INTO users (auth_uid, email, display_name, auth_provider, " - "status, pass_crypt, data_public, email_valid, terms_seen, " - "creation_time, terms_agreed, tou_agreed) " - "VALUES (:auth_uid, :email, :name, 'TDEI', 'active', 'none', " - "true, true, true, (now() at time zone 'utc'), " - "(now() at time zone 'utc'), (now() at time zone 'utc')) " - "ON CONFLICT (auth_uid) DO NOTHING" - ), - {"auth_uid": auth_uid, "email": email, "name": user_name}, + app_id = await _ensure_osm_oauth_application(session) + if app_id is None: + return + user_id = await _ensure_osm_user( + session, auth_uid=str(user_uuid), email=email, display_name=user_name ) - row = ( - await session.execute( - text( - "SELECT id FROM users " - "WHERE auth_provider = 'TDEI' AND auth_uid = :auth_uid" - ), - {"auth_uid": auth_uid}, - ) - ).first() - if row is None: + if user_id is None: return - user_id = row[0] expires_in = max(0, exp - int(time.time())) if exp else None await session.execute( @@ -410,7 +471,7 @@ async def _bridge_token_to_osm( "revoked_at = NULL" ), { - "app_id": settings.WS_OSM_OAUTH_APPLICATION_ID, + "app_id": app_id, "user_id": user_id, "token": token, "scopes": settings.WS_OSM_OAUTH_SCOPES, @@ -431,14 +492,14 @@ async def _revoke_osm_token(session: AsyncSession, token: str) -> None: Called when a user's token rotates (new ``jti``) so the previous JWT stops authenticating against osm-rails/cgimap before its own ``exp`` rather than - lingering until it expires. No-op unless the bridge is configured. + lingering until it expires. No-op unless the bridge is enabled. Note: OSM/cgimap are reachable only *through* this proxy, and every request first passes ``validate_token`` -- so a TDEI-revoked token is already rejected upstream. This revocation is defense-in-depth for the superseded token and keeps the OSM token store tidy. """ - if settings.WS_OSM_OAUTH_APPLICATION_ID <= 0: + if not settings.WS_OSM_TOKEN_BRIDGE_ENABLED: return try: await session.execute( diff --git a/tests/unit/test_token_bridge.py b/tests/unit/test_token_bridge.py index 401ad73..20e338f 100644 --- a/tests/unit/test_token_bridge.py +++ b/tests/unit/test_token_bridge.py @@ -2,13 +2,15 @@ ``_bridge_token_to_osm`` mirrors a validated TDEI token into the OSM database's ``oauth_access_tokens`` table so osm-rails (doorkeeper) and cgimap authenticate -it through their standard OAuth2 path. We cover: +it through their standard OAuth2 path. It auto-provisions everything it needs: -- it is a no-op unless ``WS_OSM_OAUTH_APPLICATION_ID`` is configured, -- when enabled it provisions the ``users`` row and inserts a plaintext - ``oauth_access_tokens`` row owned by that user, with ``expires_in`` derived - from the JWT ``exp``, -- a DB failure never propagates (auth must not break) and rolls back. +- a dedicated *system* ``users`` row that owns the doorkeeper application, +- the doorkeeper ``oauth_applications`` row (idempotent by the client uid), +- the caller's ``users`` row (owns the token), +- the plaintext ``oauth_access_tokens`` row (expires_in from the JWT exp). + +We cover: the no-op when disabled, the full provisioning chain when enabled, +the wiring of ids between rows, best-effort failure handling, and revocation. """ import time @@ -22,18 +24,21 @@ class RecordingSession: - """Async session stand-in that records (sql, params) and returns a user id - row for the ``SELECT id FROM users`` lookup.""" + """Async session stand-in that records (sql, params) and answers the two id + lookups: ``users`` (system vs caller, by auth_uid) and ``oauth_applications``.""" - def __init__(self, user_id=99): + def __init__(self, system_user_id=1, caller_user_id=42, app_id=7): self.calls: list[tuple[str, dict]] = [] self.commits = 0 self.rollbacks = 0 - self._user_id = user_id + self._system_user_id = system_user_id + self._caller_user_id = caller_user_id + self._app_id = app_id async def execute(self, statement, params=None): + params = params or {} sql = str(statement) - self.calls.append((sql, params or {})) + self.calls.append((sql, params)) class _R: def __init__(self, rows): @@ -43,7 +48,11 @@ def first(self): return self._rows[0] if self._rows else None if "SELECT id FROM users" in sql: - return _R([(self._user_id,)]) + if params.get("auth_uid") == settings.WS_OSM_SYSTEM_USER_AUTH_UID: + return _R([(self._system_user_id,)]) + return _R([(self._caller_user_id,)]) + if "SELECT id FROM oauth_applications" in sql: + return _R([(self._app_id,)]) return _R([]) async def commit(self): @@ -57,8 +66,8 @@ def sql_for(self, needle): async def test_bridge_is_noop_when_disabled(monkeypatch): - """With WS_OSM_OAUTH_APPLICATION_ID == 0 the bridge touches no DB.""" - monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 0) + """With the bridge disabled the bridge touches no DB.""" + monkeypatch.setattr(settings, "WS_OSM_TOKEN_BRIDGE_ENABLED", False) session = RecordingSession() await security._bridge_token_to_osm( @@ -74,13 +83,15 @@ async def test_bridge_is_noop_when_disabled(monkeypatch): assert session.commits == 0 -async def test_bridge_provisions_user_and_mirrors_token(monkeypatch): - """When enabled: users upsert -> id lookup -> oauth_access_tokens upsert.""" - monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 7) +async def test_bridge_provisions_app_users_and_token(monkeypatch): + """Enabled: system user -> application -> caller user -> mirrored token, + with the ids wired between rows.""" + monkeypatch.setattr(settings, "WS_OSM_TOKEN_BRIDGE_ENABLED", True) + monkeypatch.setattr(settings, "WS_OSM_OAUTH_CLIENT_UID", "workspaces-backend") monkeypatch.setattr(settings, "WS_OSM_OAUTH_SCOPES", "write_api read_prefs") uid = uuid4() exp = int(time.time()) + 3600 - session = RecordingSession(user_id=42) + session = RecordingSession(system_user_id=1, caller_user_id=42, app_id=7) await security._bridge_token_to_osm( cast(AsyncSession, session), @@ -91,19 +102,25 @@ async def test_bridge_provisions_user_and_mirrors_token(monkeypatch): exp=exp, ) - # users row provisioned idempotently, keyed by auth_uid = str(sub) + # System user provisioned (owns the app) and caller user provisioned. user_inserts = session.sql_for("INSERT INTO users") - assert user_inserts and "ON CONFLICT (auth_uid)" in user_inserts[0][0] - assert user_inserts[0][1]["auth_uid"] == str(uid) - assert user_inserts[0][1]["email"] == "alice@example.com" - - # token mirrored with the configured application/scopes and exp-derived TTL + inserted_auth_uids = {c[1]["auth_uid"] for c in user_inserts} + assert settings.WS_OSM_SYSTEM_USER_AUTH_UID in inserted_auth_uids + assert str(uid) in inserted_auth_uids + + # Application created idempotently by uid, owned by the system user (id 1). + app_inserts = session.sql_for("INSERT INTO oauth_applications") + assert app_inserts and "ON CONFLICT (uid)" in app_inserts[0][0] + assert app_inserts[0][1]["uid"] == "workspaces-backend" + assert app_inserts[0][1]["owner_id"] == 1 + assert app_inserts[0][1]["scopes"] == "write_api read_prefs" + + # Token mirrored: app id from the lookup, resource_owner = caller (42), + # self-healing upsert, exp-derived TTL. token_inserts = session.sql_for("oauth_access_tokens") - assert token_inserts and "ON CONFLICT (token)" in token_inserts[0][0] - # Re-presenting a token must reactivate it (clear revocation, refresh expiry). - assert "DO UPDATE" in token_inserts[0][0] - assert "revoked_at = NULL" in token_inserts[0][0] - params = token_inserts[0][1] + assert token_inserts + sql, params = token_inserts[0] + assert "DO UPDATE" in sql and "revoked_at = NULL" in sql assert params["app_id"] == 7 assert params["user_id"] == 42 assert params["token"] == "jwt-token" @@ -114,27 +131,33 @@ async def test_bridge_provisions_user_and_mirrors_token(monkeypatch): assert session.rollbacks == 0 -async def test_bridge_expires_in_none_without_exp(monkeypatch): - """A token without an `exp` claim mirrors with a NULL expires_in.""" - monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 7) +async def test_bridge_synthesises_email_when_absent(monkeypatch): + """A caller without an email claim still provisions (email is UNIQUE/NOT NULL).""" + monkeypatch.setattr(settings, "WS_OSM_TOKEN_BRIDGE_ENABLED", True) + uid = uuid4() session = RecordingSession() await security._bridge_token_to_osm( cast(AsyncSession, session), - user_uuid=uuid4(), + user_uuid=uid, user_name="alice", email=None, token="jwt-token", exp=None, ) + caller_insert = next( + c for c in session.sql_for("INSERT INTO users") if c[1]["auth_uid"] == str(uid) + ) + assert caller_insert[1]["email"] == f"{uid}@tdei.invalid" + params = session.sql_for("oauth_access_tokens")[0][1] assert params["expires_in"] is None async def test_bridge_is_best_effort_on_db_error(monkeypatch): """A DB failure must not propagate out of validation; it rolls back.""" - monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 7) + monkeypatch.setattr(settings, "WS_OSM_TOKEN_BRIDGE_ENABLED", True) class BoomSession(RecordingSession): async def execute(self, statement, params=None): @@ -142,7 +165,6 @@ async def execute(self, statement, params=None): session = BoomSession() - # Must not raise. await security._bridge_token_to_osm( cast(AsyncSession, session), user_uuid=uuid4(), @@ -158,7 +180,7 @@ async def execute(self, statement, params=None): async def test_revoke_is_noop_when_disabled(monkeypatch): """With the bridge off, revoking a rotated token touches no DB.""" - monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 0) + monkeypatch.setattr(settings, "WS_OSM_TOKEN_BRIDGE_ENABLED", False) session = RecordingSession() await security._revoke_osm_token(cast(AsyncSession, session), "old-token") @@ -169,7 +191,7 @@ async def test_revoke_is_noop_when_disabled(monkeypatch): async def test_revoke_marks_only_the_superseded_token(monkeypatch): """Revocation flips revoked_at for exactly the given token, if not already.""" - monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 7) + monkeypatch.setattr(settings, "WS_OSM_TOKEN_BRIDGE_ENABLED", True) session = RecordingSession() await security._revoke_osm_token(cast(AsyncSession, session), "old-token") @@ -183,7 +205,7 @@ async def test_revoke_marks_only_the_superseded_token(monkeypatch): async def test_revoke_is_best_effort_on_db_error(monkeypatch): """A DB failure during revocation must not propagate; it rolls back.""" - monkeypatch.setattr(settings, "WS_OSM_OAUTH_APPLICATION_ID", 7) + monkeypatch.setattr(settings, "WS_OSM_TOKEN_BRIDGE_ENABLED", True) class BoomSession(RecordingSession): async def execute(self, statement, params=None): From 9e607eb8e41b32cee840f50315f9f404075271ea Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Fri, 10 Jul 2026 16:40:42 -0400 Subject: [PATCH 25/34] Change stub user to validate OSM rails models --- ...f3a7b9c1d2e4_heal_short_tdei_pass_crypt.py | 47 +++++++++++++++++++ api/core/security.py | 8 +++- api/src/tasking/projects/repository.py | 8 +++- 3 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 alembic_osm/versions/f3a7b9c1d2e4_heal_short_tdei_pass_crypt.py diff --git a/alembic_osm/versions/f3a7b9c1d2e4_heal_short_tdei_pass_crypt.py b/alembic_osm/versions/f3a7b9c1d2e4_heal_short_tdei_pass_crypt.py new file mode 100644 index 0000000..5ff6bca --- /dev/null +++ b/alembic_osm/versions/f3a7b9c1d2e4_heal_short_tdei_pass_crypt.py @@ -0,0 +1,47 @@ +"""heal short TDEI-provisioned users.pass_crypt + +TDEI users are provisioned by the backend via raw SQL with a placeholder +``pass_crypt``. Older rows used a 4-char value (``"none"``), which violates OSM's +``pass_crypt`` length 8..255 ``User`` validation. That makes the user invalid, +and Rails operations that re-validate the author via ``validates :author, +:associated => true`` (posting a changeset comment or a note comment) then fail +to save. Replace any too-short value with a random throwaway -- TDEI manages +auth, so ``pass_crypt`` is never used to authenticate. + +Revision ID: f3a7b9c1d2e4 +Revises: 5303f61d7b3a +Create Date: 2026-07-11 00:00:00.000000 + +""" + +from typing import Sequence, Union + +from alembic import op +from sqlalchemy import inspect, text + +# revision identifiers, used by Alembic. +revision: str = "f3a7b9c1d2e4" +down_revision: Union[str, None] = "5303f61d7b3a" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + bind = op.get_bind() + # `users` is owned by the OSM Rails website, not this tree; guard so the + # migration is a no-op if it hasn't been created yet. + if not inspect(bind).has_table("users"): + return + + # Idempotent: re-running only touches rows that are still too short. + op.execute( + text( + "UPDATE users SET pass_crypt = md5(random()::text) " + "WHERE auth_provider = 'TDEI' AND length(pass_crypt) < 8" + ) + ) + + +def downgrade() -> None: + # The original short values are unrecoverable (and were invalid anyway). + pass diff --git a/api/core/security.py b/api/core/security.py index adeaa38..020abf8 100644 --- a/api/core/security.py +++ b/api/core/security.py @@ -354,7 +354,7 @@ async def _ensure_osm_user( "INSERT INTO users (auth_uid, email, display_name, auth_provider, " "status, pass_crypt, data_public, email_valid, terms_seen, " "creation_time, terms_agreed, tou_agreed) " - "VALUES (:auth_uid, :email, :name, 'TDEI', 'active', 'none', " + "VALUES (:auth_uid, :email, :name, 'TDEI', 'active', :pass_crypt, " "true, true, true, (now() at time zone 'utc'), " "(now() at time zone 'utc'), (now() at time zone 'utc')) " "ON CONFLICT (auth_uid) DO NOTHING" @@ -363,6 +363,12 @@ async def _ensure_osm_user( "auth_uid": auth_uid, "email": email or f"{auth_uid}@tdei.invalid", "name": display_name, + # OSM's User model validates pass_crypt length 8..255. TDEI manages + # auth, so this is a throwaway that just satisfies that rule (a too- + # short value makes the user invalid and breaks Rails operations + # that re-validate it via `validates :author, :associated => true`, + # e.g. posting a changeset comment). + "pass_crypt": secrets.token_hex(16), }, ) row = ( diff --git a/api/src/tasking/projects/repository.py b/api/src/tasking/projects/repository.py index 3aa6820..44bbc42 100644 --- a/api/src/tasking/projects/repository.py +++ b/api/src/tasking/projects/repository.py @@ -1,6 +1,7 @@ from __future__ import annotations import json +import secrets from datetime import datetime from typing import Any from uuid import UUID @@ -313,13 +314,18 @@ async def _provision_users_from_tdei( await self.session.execute( text( "INSERT INTO users (auth_uid, email, display_name, auth_provider, status, pass_crypt, data_public, email_valid, terms_seen, creation_time, terms_agreed, tou_agreed) " - "VALUES (:uid, :email, :name, 'TDEI', 'active', 'none', true, true, true, (now() at time zone 'utc'), (now() at time zone 'utc'), (now() at time zone 'utc')) " + "VALUES (:uid, :email, :name, 'TDEI', 'active', :pass_crypt, true, true, true, (now() at time zone 'utc'), (now() at time zone 'utc'), (now() at time zone 'utc')) " "ON CONFLICT (auth_uid) DO NOTHING" ), params={ "uid": uid, "email": member.email, "name": member.display_name, + # OSM validates pass_crypt length 8..255; a too-short value + # makes the user invalid and breaks Rails ops that re-validate + # the author (changeset/note comments). TDEI manages auth, so + # this is a throwaway. + "pass_crypt": secrets.token_hex(16), }, ) resolved.add(uid) From a7b41092f255bed6a88e6573aebc93488135ff50 Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Fri, 10 Jul 2026 16:52:01 -0400 Subject: [PATCH 26/34] Docs --- .claude/settings.json | 4 ++- CLAUDE.md | 78 +++++++++++++++++++++++++++++++++++++++++++ README.md | 42 +++++++++++++++++++++++ 3 files changed, 123 insertions(+), 1 deletion(-) diff --git a/.claude/settings.json b/.claude/settings.json index fa7924c..ea52c82 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -10,7 +10,9 @@ "Bash(git config *)", "Bash(command -v gh)", "Bash(gh api *)", - "Bash(python3 -c \"import sys,json; d=json.load\\(sys.stdin\\); print\\('protected:', d.get\\('protected'\\)\\); p=d.get\\('protection',{}\\); print\\('required_pull_request_reviews:', 'yes' if p.get\\('required_pull_request_reviews'\\) else 'no'\\); print\\('enforce_admins:', \\(p.get\\('enforce_admins'\\) or {}\\).get\\('enabled'\\)\\)\")" + "Bash(python3 -c \"import sys,json; d=json.load\\(sys.stdin\\); print\\('protected:', d.get\\('protected'\\)\\); p=d.get\\('protection',{}\\); print\\('required_pull_request_reviews:', 'yes' if p.get\\('required_pull_request_reviews'\\) else 'no'\\); print\\('enforce_admins:', \\(p.get\\('enforce_admins'\\) or {}\\).get\\('enabled'\\)\\)\")", + "Bash(grep -nA6 \"TENANT_BYPASSES: list\" api/main.py)", + "Bash(grep -nA10 \"STRIP_REQUEST_HEADERS = \" api/main.py)" ] } } diff --git a/CLAUDE.md b/CLAUDE.md index 1e5e9d6..d15dea5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -116,6 +116,84 @@ enforced at this layer; if required, it must be enforced downstream (`workspaces-openstreetmap-website/`, `workspaces-cgimap/`) — that has not been audited here. +## The OSM proxy layer: routing, auth, and user provisioning + +This service is a reverse proxy in front of the OSM website (`osm-rails`) and +cgimap. The non-obvious parts, learned the hard way: + +### Deployment routing (workspaces-stack) + +In `workspaces-stack`, the **api container (this backend) serves the public OSM +host** (`osm.workspaces-...`) alongside the API hosts — its traefik router +rule is `Host(new-api) || Host(api) || Host(osm)`, and the `osm-web` / +`osm-log-proxy` routers are commented out. So **every OSM call the frontend +makes routes through this backend**: `validate_token`, the `X-Workspace` gate, +and the CORS middleware all apply. The backend then proxies `/api/0.6/*` to +`WS_OSM_HOST` (config default `http://osm-web` — the internal service, *not* the +public domain). `osm-web`'s nginx splits traffic: changeset read/write subpaths +to cgimap, everything else to osm-rails. + +The frontend (`services/osm.ts`) calls the OSM host directly with +`credentials: 'include'` + a Bearer TDEI token. Because it's *credentialed* +CORS, `CORS_ORIGINS` must list the exact frontend origin (no `*`; +`allow_credentials` is on). The stack supplies `WS_API_CORS_ORIGINS` as a **JSON +array** (`["https://..."]`), while older config comma-split it — a JSON array +would then become one malformed origin. `api/main.py` therefore reads +`settings.cors_origins_list`, which parses **both** a JSON array and a +comma-separated string (and `*`); set `CORS_ORIGINS` in either form. + +### Two databases; `users` is owned by OSM Rails + +`TASK_DATABASE_URL` (`alembic_task` tree) holds workspaces / tasking-manager +tables; `OSM_DATABASE_URL` (`alembic_osm` tree) holds OSM data, `users`, and the +`tasking_*` tables. The **`users` table is owned by the OSM Rails website**, not +either alembic tree — the trees only FK to it, and integration tests stub it +(`tests/integration/conftest.py`). The backend provisions `users` rows itself +via raw SQL with `auth_provider='TDEI'` and `auth_uid = str()` (the OSM +`auth_uid` **is** the token's `sub` claim). + +### How OSM authenticates, and the TDEI token bridge + +* **osm-rails** authenticates API calls *only* via **doorkeeper OAuth2**: it + looks the bearer token up in `oauth_access_tokens` (`token` → + `resource_owner_id` → `users.id`). Doorkeeper stores tokens **plaintext** + (`SecretStoring::Plain`), so the raw JWT matches directly. It has **no** + TDEI/JWT auth path. +* **cgimap** has both: the oauth2 lookup *and* a custom TDEI path + (`get_user_id_for_tdei_token`, which verifies the JWT and matches + `users.auth_uid`). + +To make TDEI tokens work against osm-rails without forking it, the **token +bridge** (`_bridge_token_to_osm` in `api/core/security.py`) mirrors a validated +TDEI JWT into `oauth_access_tokens` — on the cache-miss path of `validate_token` +(so ~once per token, not per request). Gated by `WS_OSM_TOKEN_BRIDGE_ENABLED`. +It auto-provisions everything: a dedicated **system user** (`WS_OSM_SYSTEM_USER_*`) +to own the doorkeeper `oauth_applications` row it creates (keyed by +`WS_OSM_OAUTH_CLIENT_UID`; `oauth_applications.owner_id` is a NOT-NULL FK to +`users`, hence the system user), the caller's `users` row, and the plaintext +`oauth_access_tokens` row (`expires_in` from the JWT `exp`; +`ON CONFLICT (token) DO UPDATE` so re-presenting reactivates it). On token +rotation (new `jti`) the superseded token is revoked. Since the token then lives +in `oauth_access_tokens`, both osm-rails and cgimap authenticate it via plain +OAuth2 — cgimap's custom TDEI path becomes redundant. + +### Provisioning gotcha: `pass_crypt` must be 8..255 chars + +The OSM `User` model has `validates :pass_crypt, :length => 8..255`. When the +backend provisions a `users` row, **`pass_crypt` must be 8–255 chars** (use a +random throwaway — TDEI manages auth, so it's never used to log in). A too-short +value (the old `'none'`) makes the user *invalid*. This is silent for **cgimap** +operations — cgimap runs no Rails validations — but breaks any **osm-rails** +operation that re-validates the author via `validates :author, :associated => +true`: notably `POST /api/0.6/changeset/{id}/comment` and note comments. The +comment fails to save (no `id`), then rendering it throws *"Unable to serialize +… without an id"*. Both provisioning paths (`_ensure_osm_user`, +`_provision_users_from_tdei`) set a valid `pass_crypt`; migration +`alembic_osm/versions/f3a7b9c1d2e4` heals legacy short rows. General principle: +a backend-provisioned `users` row must satisfy OSM's `User` validations (also +`display_name` 3..255 + unique, `email` present + unique) or Rails operations +that touch it will fail even though cgimap operations succeed. + ## Testing Two layers, both fast and dependency-free (no Postgres, PostGIS, Docker, or diff --git a/README.md b/README.md index 8ffa3f5..7b905d3 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,48 @@ This is a combination API backend for workspaces, providing /workspaces* methods the OSM API ("openstreetmap website") plus OSM CGI-map (the C-accelerated methods) that enforces authorization and authentication based on a TDEI/Keycloak JWT token (see main.py for this proxy logic). +## What the proxy must provide for osm-rails / osm-web + +This backend is the **only entry point** to the OSM tier (osm-rails + cgimap, +behind `osm-web`): in the deployment, the public OSM host routes to this +container, which proxies `/api/0.6/*` to `WS_OSM_HOST` (default +`http://osm-web`). For the OSM services to work, the proxy must uphold the +following contract. `CLAUDE.md` has the full rationale. + +1. **Bridge TDEI auth into OSM's OAuth2.** osm-rails authenticates the API *only* + via doorkeeper OAuth2 (`oauth_access_tokens`); it has no TDEI/JWT path. So on + token validation the backend mirrors the TDEI JWT into `oauth_access_tokens` + in the OSM DB (the "token bridge" in `api/core/security.py`), and forwards the + incoming `Authorization: Bearer ` header unchanged. Then osm-rails and + cgimap authenticate the token via plain OAuth2. Controlled by + `WS_OSM_TOKEN_BRIDGE_ENABLED` (on by default) — with it off, osm-rails returns + **401** for TDEI tokens. The backend auto-creates the doorkeeper application + (and a system user to own it), so no manual OSM setup is required. + +2. **Provision valid OSM `users` rows.** The backend creates OSM `users` rows for + TDEI users (`auth_provider='TDEI'`, `auth_uid` = the JWT `sub`). These must + satisfy OSM's `User` validations — in particular `pass_crypt` length 8..255. + A too-short value is invisible to cgimap but makes osm-rails operations that + re-validate the user fail (e.g. posting a changeset comment or a note), + surfacing as *"Unable to serialize … without an id"*. The + `alembic_osm` migration `*_heal_short_tdei_pass_crypt` repairs legacy rows. + +3. **Carry workspace tenancy.** Workspace-scoped OSM requests must include an + `X-Workspace: ` header. The proxy authorizes it against the caller's + workspaces and forwards it (it is *not* stripped) so cgimap/osm-rails scope to + the `workspace-` schema. A few paths are exempt (`TENANT_BYPASSES` in + `api/main.py`): workspace create/delete (`PUT`/`DELETE /api/0.6/workspaces/{id}`) + and user provisioning during sign-in (`PUT /api/0.6/user/{uid}`). + +4. **Set the proxy headers.** The proxy rewrites `Host` to the OSM host and sets + `X-Real-IP` / `X-Forwarded-For` / `-Host` / `-Proto`, while stripping + hop-by-hop headers and any spoofed forwarding headers from the client. It does + *not* strip `Authorization` or `X-Workspace`. + +5. **Connectivity.** `WS_OSM_HOST` must reach `osm-web`, and the backend needs + **both** `OSM_DATABASE_URL` and `TASK_DATABASE_URL` — the token bridge and + user provisioning write to the OSM database. + ## Branch Index * ```develop``` merge your work here; keep this up to date with the "development" environment / dev tag From 6df53b032e584f5933ade5f30d95ecb799bf283e Mon Sep 17 00:00:00 2001 From: Naresh Kumar D Date: Tue, 14 Jul 2026 11:24:12 +0530 Subject: [PATCH 27/34] initial docker setup done - Added notes to local development. - Need to add more details on initial development. --- .gitignore | 1 + README.md | 25 ++++++++++++++ api/core/config.py | 1 + docker-compose.local.yml | 71 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 98 insertions(+) create mode 100644 docker-compose.local.yml diff --git a/.gitignore b/.gitignore index 4516af1..d929d58 100644 --- a/.gitignore +++ b/.gitignore @@ -170,3 +170,4 @@ docs/tasking-mvp/tasking-mvp.postman_environment.json docs/tasking-mvp/_enrich_postman.py docs/tasking-mvp/feature-coverage.md .idea/ +data/ \ No newline at end of file diff --git a/README.md b/README.md index 7b905d3..a8867e1 100644 --- a/README.md +++ b/README.md @@ -82,3 +82,28 @@ uvx pyright --pythonpath .venv/bin/python api tests uv run black api tests && uv run isort api tests ``` +## Development with local environment + +Use the file `docker-compose.local.yml` to build and deploy local code changes. This allows you to run the entire system at once instead of +connecting to existing Databases. + +### Initial setup. +- On first launch, rails-worker will fail because migrations are not done +- Go to the `osm-rails` `/bin/sh` and execute `bundle exec rails db:migrate` +- The above script runs the migration code for osm-rails +- The rails-worker will be able to run +- The backend code will fail first time becase `workspaces-tasks-local` database is not available +- Login to postgresql container and run the following commands + `psql --username postgres` + `create database "workspaces-tasks-local";` + `psql --username postgres --dbname "workspaces-tasks-local";` + `create extension if not exists postgis;` +- Run the backend code now and it should be able to run + +### Commands to start and stop the docker compose + +`docker compose --file docker-compose.local.yml build up -d` + +`docker compose --file docker-compose.local.yml down` + +Backend code will be available at `http://localhost:3000` \ No newline at end of file diff --git a/api/core/config.py b/api/core/config.py index 04693ac..c6554c7 100644 --- a/api/core/config.py +++ b/api/core/config.py @@ -96,6 +96,7 @@ def cors_origins_list(self) -> list[str]: model_config = SettingsConfigDict( env_file=".env", env_file_encoding="utf-8", + extra="ignore", # ignore unknown environment variables ) diff --git a/docker-compose.local.yml b/docker-compose.local.yml new file mode 100644 index 0000000..8bfeae3 --- /dev/null +++ b/docker-compose.local.yml @@ -0,0 +1,71 @@ +services: + database: + image: postgis/postgis:16-3.4 + environment: + POSTGRES_USER: postgres + POSTGRES_PASSWORD: testing + POSTGRES_DB: workspaces-osm-local + ports: + - 5432:5432 + volumes: + - ./data/db:/var/lib/postgresql/data + healthcheck: + test: ["CMD-SHELL", "pg_isready -U postgres -d workspaces-osm-local"] + interval: 10s + timeout: 5s + retries: 5 + osm-cgimap: + image: opensidewalksdev.azurecr.io/workspaces-osm-cgimap:dev + environment: + CGIMAP_INSTANCES: 10 + CGIMAP_HOST: database + CGIMAP_USERNAME: postgres + CGIMAP_PASSWORD: testing + CGIMAP_DBNAME: workspaces-osm-local + CGIMAP_MAX_CHANGESET_ELEMENTS: 100000000 # max features per import or save + CGIMAP_MAX_PAYLOAD: 1000000000 # max size of dataset uploads + CGIMAP_MAP_NODES: 100000000 # max number of nodes per requeset + CGIMAP_MAP_AREA: 1 # max area per request in square degrees + ports: + - 8000 + osm-rails: + image: opensidewalksdev.azurecr.io/workspaces-osm-rails:dev + environment: + RAILS_ENV: production + SECRET_KEY_BASE: osm_secret_key + WS_OSM_DB_HOST: database + WS_OSM_DB_USER: postgres + WS_OSM_DB_PASS: testing + WS_OSM_DB_NAME: workspaces-osm-local + stdin_open: true + tty: true + ports: + - 3000 + command: ["bundle", "exec", "rails", "s", "-p", "3000", "-b", "0.0.0.0"] + + osm-rails-worker: + image: opensidewalksdev.azurecr.io/workspaces-osm-rails:dev + environment: + RAILS_ENV: production + SECRET_KEY_BASE: osm_secret_key + WS_OSM_DB_HOST: database + WS_OSM_DB_USER: postgres + WS_OSM_DB_PASS: testing + WS_OSM_DB_NAME: workspaces-osm-local + stdin_open: true + tty: true + command: ["bundle", "exec", "rake", "jobs:work"] + + backend: + build: . + container_name: workspaces-backend + volumes: + - .:/app + ports: + - 8000:8000 + environment: + TDEI_BACKEND_URL: ${TDEI_BACKEND_URL} + TDEI_OIDC_REALM: ${TDEI_OIDC_REALM} + TDEI_OIDC_URL: ${TDEI_OIDC_URL} + OSM_DATABASE_URL: postgresql+asyncpg://postgres:testing@database:5432/workspaces-osm-local + TASK_DATABASE_URL: postgresql+asyncpg://postgres:testing@database:5432/workspaces-tasks-local \ No newline at end of file From ba9cdc24b9dcd635bd31e4f6434925e1808f9e5e Mon Sep 17 00:00:00 2001 From: Naresh Kumar D Date: Tue, 14 Jul 2026 12:19:16 +0530 Subject: [PATCH 28/34] Added custom_imagery column Migration logic yet to be written --- api/src/tasking/projects/dtos.py | 3 +++ api/src/tasking/projects/repository.py | 4 ++++ api/src/tasking/projects/schemas.py | 7 +++++++ 3 files changed, 14 insertions(+) diff --git a/api/src/tasking/projects/dtos.py b/api/src/tasking/projects/dtos.py index 488e981..709b80f 100644 --- a/api/src/tasking/projects/dtos.py +++ b/api/src/tasking/projects/dtos.py @@ -47,6 +47,7 @@ class ProjectCreateRequest(WireModel): lock_timeout_hours: int = PydField(default=8, ge=1, le=720) aoi: Optional[AoiInput] = None role_assignments: list[ProjectRoleAssignment] = PydField(default_factory=list) + custom_imagery: Optional[Any] = PydField(default=None) @field_validator("name") @classmethod @@ -68,6 +69,7 @@ class ProjectUpdateRequest(WireModel): instructions: Optional[str] = PydField(default=None, max_length=10_000) lock_timeout_hours: Optional[int] = PydField(default=None, ge=1, le=720) review_required: Optional[bool] = None + custom_imagery: Optional[Any] = PydField(default=None) class ProjectResponse(WireModel): @@ -87,6 +89,7 @@ class ProjectResponse(WireModel): created_by_name: Optional[str] = None created_at: datetime updated_at: datetime + custom_imagery: Optional[Any] = None class ProjectListItem(WireModel): diff --git a/api/src/tasking/projects/repository.py b/api/src/tasking/projects/repository.py index 44bbc42..b046436 100644 --- a/api/src/tasking/projects/repository.py +++ b/api/src/tasking/projects/repository.py @@ -243,6 +243,7 @@ def _to_response(project: TaskingProject, task_count: int = 0) -> ProjectRespons created_by_name=project.created_by_name, created_at=project.created_at, updated_at=project.updated_at, + custom_imagery=project.custom_imagery, # type: ignore[arg-type] ) async def _provision_users_from_tdei( @@ -531,6 +532,7 @@ async def create( lock_timeout_hours=body.lock_timeout_hours, created_by=current_user.user_uuid, created_by_name=current_user.user_name, + custom_imagery=body.custom_imagery, # type: ignore[arg-type] ) if body.aoi is not None: geom = _aoi_to_shapely(body.aoi) @@ -632,6 +634,8 @@ async def patch( updates["lock_timeout_hours"] = body.lock_timeout_hours if body.review_required is not None: updates["review_required"] = body.review_required + if body.custom_imagery is not None: + updates["custom_imagery"] = body.custom_imagery # type: ignore[arg-type] if updates: updates["updated_at"] = datetime.now() diff --git a/api/src/tasking/projects/schemas.py b/api/src/tasking/projects/schemas.py index 901d5f6..8150f4b 100644 --- a/api/src/tasking/projects/schemas.py +++ b/api/src/tasking/projects/schemas.py @@ -11,6 +11,7 @@ from sqlalchemy import Column from sqlalchemy import Enum as SAEnum from sqlmodel import Field, SQLModel +from sqlalchemy.dialects.postgresql import JSONB # --------------------------------------------------------------------------- # Enums (mirrors of postgres enums in the migration) @@ -94,6 +95,12 @@ class TaskingProject(SQLModel, table=True): ) deleted_at: Optional[datetime] = None + # Custom Imagery data for the project. Stored as JSONB in the database and converted to / from a Pydantic model in the repository layer. + custom_imagery: Optional[Any] = Field( + default=None, + sa_column=Column(JSONB, nullable=True,default=None) + ) + # --------------------------------------------------------------------------- # Project role enum + table From 856ebb66b5a210073cf3a8d6c201de912fe92efb Mon Sep 17 00:00:00 2001 From: Naresh Kumar D Date: Tue, 14 Jul 2026 15:30:15 +0530 Subject: [PATCH 29/34] imagery and description added --- README.md | 2 +- ...61f527ef_custom_imagery_and_description.py | 37 +++++++++++++++++++ api/src/tasking/projects/dtos.py | 7 +++- api/src/tasking/projects/repository.py | 2 + api/src/tasking/projects/schemas.py | 7 ++-- 5 files changed, 49 insertions(+), 6 deletions(-) create mode 100644 alembic_osm/versions/a92361f527ef_custom_imagery_and_description.py diff --git a/README.md b/README.md index a8867e1..ebba814 100644 --- a/README.md +++ b/README.md @@ -102,7 +102,7 @@ connecting to existing Databases. ### Commands to start and stop the docker compose -`docker compose --file docker-compose.local.yml build up -d` +`docker compose --file docker-compose.local.yml up --build -d` `docker compose --file docker-compose.local.yml down` diff --git a/alembic_osm/versions/a92361f527ef_custom_imagery_and_description.py b/alembic_osm/versions/a92361f527ef_custom_imagery_and_description.py new file mode 100644 index 0000000..6a4fdcc --- /dev/null +++ b/alembic_osm/versions/a92361f527ef_custom_imagery_and_description.py @@ -0,0 +1,37 @@ +"""custom_imagery_and_description + +Revision ID: a92361f527ef +Revises: f3a7b9c1d2e4 +Create Date: 2026-07-14 15:08:42.142553 + +""" + +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op +from sqlalchemy.dialects.postgresql import JSONB + +# revision identifiers, used by Alembic. +revision: str = "a92361f527ef" +down_revision: Union[str, None] = "f3a7b9c1d2e4" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.add_column( + "tasking_projects", + sa.Column("custom_imagery", JSONB(astext_type=sa.Text()), nullable=True), + ) + op.add_column( + "tasking_projects", + sa.Column("description", sa.String(length=10000), nullable=True), + ) + pass + + +def downgrade() -> None: + op.drop_column("tasking_projects", "custom_imagery") + op.drop_column("tasking_projects", "description") + pass diff --git a/api/src/tasking/projects/dtos.py b/api/src/tasking/projects/dtos.py index 709b80f..601c09e 100644 --- a/api/src/tasking/projects/dtos.py +++ b/api/src/tasking/projects/dtos.py @@ -47,7 +47,8 @@ class ProjectCreateRequest(WireModel): lock_timeout_hours: int = PydField(default=8, ge=1, le=720) aoi: Optional[AoiInput] = None role_assignments: list[ProjectRoleAssignment] = PydField(default_factory=list) - custom_imagery: Optional[Any] = PydField(default=None) + custom_imagery: Optional[dict[str, Any]] = PydField(default=None) + description: Optional[str] = PydField(default=None, max_length=10_000) @field_validator("name") @classmethod @@ -69,7 +70,8 @@ class ProjectUpdateRequest(WireModel): instructions: Optional[str] = PydField(default=None, max_length=10_000) lock_timeout_hours: Optional[int] = PydField(default=None, ge=1, le=720) review_required: Optional[bool] = None - custom_imagery: Optional[Any] = PydField(default=None) + custom_imagery: Optional[dict[str, Any]] = PydField(default=None) + description: Optional[str] = PydField(default=None, max_length=10_000) class ProjectResponse(WireModel): @@ -90,6 +92,7 @@ class ProjectResponse(WireModel): created_at: datetime updated_at: datetime custom_imagery: Optional[Any] = None + description: Optional[str] = None class ProjectListItem(WireModel): diff --git a/api/src/tasking/projects/repository.py b/api/src/tasking/projects/repository.py index b046436..71c4eb9 100644 --- a/api/src/tasking/projects/repository.py +++ b/api/src/tasking/projects/repository.py @@ -244,6 +244,7 @@ def _to_response(project: TaskingProject, task_count: int = 0) -> ProjectRespons created_at=project.created_at, updated_at=project.updated_at, custom_imagery=project.custom_imagery, # type: ignore[arg-type] + description=project.description, # type: ignore[arg-type] ) async def _provision_users_from_tdei( @@ -533,6 +534,7 @@ async def create( created_by=current_user.user_uuid, created_by_name=current_user.user_name, custom_imagery=body.custom_imagery, # type: ignore[arg-type] + description=body.description, # type: ignore[arg-type] ) if body.aoi is not None: geom = _aoi_to_shapely(body.aoi) diff --git a/api/src/tasking/projects/schemas.py b/api/src/tasking/projects/schemas.py index 8150f4b..85fc0b2 100644 --- a/api/src/tasking/projects/schemas.py +++ b/api/src/tasking/projects/schemas.py @@ -10,8 +10,8 @@ from pydantic import Field as PydField from sqlalchemy import Column from sqlalchemy import Enum as SAEnum -from sqlmodel import Field, SQLModel from sqlalchemy.dialects.postgresql import JSONB +from sqlmodel import Field, SQLModel # --------------------------------------------------------------------------- # Enums (mirrors of postgres enums in the migration) @@ -97,10 +97,11 @@ class TaskingProject(SQLModel, table=True): # Custom Imagery data for the project. Stored as JSONB in the database and converted to / from a Pydantic model in the repository layer. custom_imagery: Optional[Any] = Field( - default=None, - sa_column=Column(JSONB, nullable=True,default=None) + default=None, sa_column=Column(JSONB, nullable=True, default=None) ) + description: Optional[str] = Field(default=None, nullable=True) + # --------------------------------------------------------------------------- # Project role enum + table From dbf41e2854c7838c16ea34f9c6f702185afa10fa Mon Sep 17 00:00:00 2001 From: Naresh Kumar D Date: Tue, 14 Jul 2026 16:06:52 +0530 Subject: [PATCH 30/34] Added routes for validate name Added routes for validating the project name and unit tests for the same --- api/src/tasking/projects/dtos.py | 5 ++ api/src/tasking/projects/repository.py | 16 +++++ api/src/tasking/projects/routes.py | 21 +++++++ tests/integration/test_projects_flow.py | 32 ++++++++++ tests/unit/conftest.py | 8 +++ tests/unit/test_project_routes.py | 80 +++++++++++++++++++++++++ 6 files changed, 162 insertions(+) diff --git a/api/src/tasking/projects/dtos.py b/api/src/tasking/projects/dtos.py index 601c09e..01057a8 100644 --- a/api/src/tasking/projects/dtos.py +++ b/api/src/tasking/projects/dtos.py @@ -120,6 +120,10 @@ class ProjectListResponse(WireModel): pagination: Pagination +class ProjectNameValidationResponse(WireModel): + exists: bool + + # --------------------------------------------------------------------------- # AOI response shape (canonical Feature wrapping a MultiPolygon) # --------------------------------------------------------------------------- @@ -190,6 +194,7 @@ class SelfProjectRolesResponse(WireModel): "ProjectCreateRequest", "ProjectListItem", "ProjectListResponse", + "ProjectNameValidationResponse", "ProjectResponse", "ProjectRoleAddRequest", "ProjectRoleAssignment", diff --git a/api/src/tasking/projects/repository.py b/api/src/tasking/projects/repository.py index 71c4eb9..beee8e6 100644 --- a/api/src/tasking/projects/repository.py +++ b/api/src/tasking/projects/repository.py @@ -470,6 +470,22 @@ async def list_projects( pagination=Pagination(page=page, page_size=page_size, total=total), ) + async def project_name_exists(self, workspace_id: int, name: str) -> bool: + result = await self.session.execute( + select(func.count()) + .select_from(TaskingProject) + .where( + (TaskingProject.workspace_id == workspace_id) + & (TaskingProject.name == name) + & ( + TaskingProject.deleted_at.is_( # pyright: ignore[reportAttributeAccessIssue, reportOptionalMemberAccess] + None + ) + ) + ) + ) + return int(result.scalar() or 0) > 0 + async def create( self, workspace_id: int, diff --git a/api/src/tasking/projects/routes.py b/api/src/tasking/projects/routes.py index d4979b0..4d531d0 100644 --- a/api/src/tasking/projects/routes.py +++ b/api/src/tasking/projects/routes.py @@ -12,6 +12,7 @@ AoiFeature, ProjectCreateRequest, ProjectListResponse, + ProjectNameValidationResponse, ProjectResponse, ProjectRoleAddRequest, ProjectRoleItem, @@ -128,6 +129,26 @@ async def create_project( ) +@router.get("/validate-name", response_model=ProjectNameValidationResponse) +async def validate_project_name( + workspace_id: int, + name: str = Query(..., min_length=1, max_length=255), + current_user: UserInfo = Depends(validate_token), + workspace_repo: WorkspaceRepository = Depends(get_workspace_repo), + project_repo: TaskingProjectRepository = Depends(get_project_repo), +): + await assert_workspace_visible(workspace_id, current_user, workspace_repo) + normalized_name = name.strip() + if not normalized_name: + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail="name cannot be blank", + ) + return ProjectNameValidationResponse( + exists=await project_repo.project_name_exists(workspace_id, normalized_name) + ) + + @router.get("/{project_id}", response_model=ProjectResponse) async def get_project( workspace_id: int, diff --git a/tests/integration/test_projects_flow.py b/tests/integration/test_projects_flow.py index c1575de..088c614 100644 --- a/tests/integration/test_projects_flow.py +++ b/tests/integration/test_projects_flow.py @@ -206,6 +206,38 @@ async def test_outsider_404s_on_list( assert r.status_code == 404 +class TestProjectNameValidation: + async def test_validate_name_false_then_true( + self, client, as_lead, seeded_workspace_id + ): + """validate-name reports false before create and true after create for same workspace.""" + path = f"{API.format(wid=seeded_workspace_id)}/validate-name" + + r = await client.get(path, params={"name": "name-check"}) + assert r.status_code == 200, r.text + assert r.json() == {"exists": False} + + r = await client.post( + API.format(wid=seeded_workspace_id), + json={"name": "name-check"}, + ) + assert r.status_code == 201, r.text + + r = await client.get(path, params={"name": "name-check"}) + assert r.status_code == 200, r.text + assert r.json() == {"exists": True} + + async def test_validate_name_outsider_404( + self, client, as_outsider, seeded_workspace_id + ): + """Outsider receives 404 from tenancy gate on validate-name.""" + r = await client.get( + f"{API.format(wid=seeded_workspace_id)}/validate-name", + params={"name": "anything"}, + ) + assert r.status_code == 404 + + # --------------------------------------------------------------------------- # Workflow 3b — error mapping (constraint violations → precise HTTP status). # --------------------------------------------------------------------------- diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 20ac3f5..e9fc3ad 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -175,6 +175,14 @@ async def get(self, workspace_id: int, project_id: int): raise NotFoundException(f"Project {project_id} not found") return self._response(p) + async def project_name_exists(self, workspace_id: int, name: str) -> bool: + return any( + p["workspace_id"] == workspace_id + and p["name"] == name + and p["deleted_at"] is None + for p in self._projects.values() + ) + async def patch(self, workspace_id, project_id, body, current_user): p_resp = await self.get(workspace_id, project_id) p = self._projects[project_id] diff --git a/tests/unit/test_project_routes.py b/tests/unit/test_project_routes.py index 1620aba..792103a 100644 --- a/tests/unit/test_project_routes.py +++ b/tests/unit/test_project_routes.py @@ -1,5 +1,7 @@ from __future__ import annotations +import pytest + API = "/api/v1/workspaces/{wid}/tasking/projects" @@ -68,6 +70,17 @@ async def test_create_blank_name_422( ) assert r.status_code == 422 + @pytest.mark.parametrize("bad_value", ["not-an-object", 123, True, ["x"]]) + async def test_create_rejects_non_object_custom_imagery_422( + self, client, as_lead, seeded_workspace_id, fake_repos, bad_value + ): + """custom_imagery must be a JSON object; scalar/array values are rejected (422).""" + r = await client.post( + API.format(wid=seeded_workspace_id), + json={"name": "Pilot", "custom_imagery": bad_value}, + ) + assert r.status_code == 422 + async def test_get_404_when_missing( self, client, as_lead, seeded_workspace_id, fake_repos ): @@ -105,6 +118,24 @@ async def test_patch_name(self, client, as_lead, seeded_workspace_id, fake_repos assert r.status_code == 200 assert r.json()["name"] == "after" + @pytest.mark.parametrize("bad_value", ["not-an-object", 123, False, ["x"]]) + async def test_patch_rejects_non_object_custom_imagery_422( + self, client, as_lead, seeded_workspace_id, fake_repos, bad_value + ): + """PATCH rejects custom_imagery when it is not a JSON object (422).""" + pid = ( + await client.post( + API.format(wid=seeded_workspace_id), + json={"name": "before"}, + ) + ).json()["id"] + + r = await client.patch( + f"{API.format(wid=seeded_workspace_id)}/{pid}", + json={"custom_imagery": bad_value}, + ) + assert r.status_code == 422 + async def test_soft_delete_204_then_404( self, client, as_lead, seeded_workspace_id, fake_repos ): @@ -137,6 +168,55 @@ async def test_duplicate_name_409( assert r.status_code == 409 +class TestProjectNameValidation: + async def test_validate_name_returns_false_when_missing( + self, client, as_contributor, seeded_workspace_id, fake_repos + ): + """Validation returns exists=false when no active project uses the name.""" + r = await client.get( + f"{API.format(wid=seeded_workspace_id)}/validate-name", + params={"name": "new-project"}, + ) + assert r.status_code == 200 + assert r.json() == {"exists": False} + + async def test_validate_name_returns_true_when_exists( + self, client, as_lead, seeded_workspace_id, fake_repos + ): + """Validation returns exists=true when an active project with same name exists.""" + await client.post( + API.format(wid=seeded_workspace_id), + json={"name": "pilot-check"}, + ) + + r = await client.get( + f"{API.format(wid=seeded_workspace_id)}/validate-name", + params={"name": "pilot-check"}, + ) + assert r.status_code == 200 + assert r.json() == {"exists": True} + + async def test_validate_name_blank_rejected_422( + self, client, as_contributor, seeded_workspace_id, fake_repos + ): + """Whitespace-only name is rejected with 422.""" + r = await client.get( + f"{API.format(wid=seeded_workspace_id)}/validate-name", + params={"name": " "}, + ) + assert r.status_code == 422 + + async def test_validate_name_outsider_404( + self, client, as_outsider, seeded_workspace_id, fake_repos + ): + """Tenancy gate hides workspace existence for outsiders.""" + r = await client.get( + f"{API.format(wid=seeded_workspace_id)}/validate-name", + params={"name": "pilot-check"}, + ) + assert r.status_code == 404 + + # --------------------------------------------------------------------------- # Lifecycle gates # --------------------------------------------------------------------------- From edb1241afcd2440e777026e3407a3b4bb6723c89 Mon Sep 17 00:00:00 2001 From: Naresh Kumar D Date: Tue, 14 Jul 2026 16:16:09 +0530 Subject: [PATCH 31/34] Update a92361f527ef_custom_imagery_and_description.py removed unnecessary pass statement --- .../versions/a92361f527ef_custom_imagery_and_description.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/alembic_osm/versions/a92361f527ef_custom_imagery_and_description.py b/alembic_osm/versions/a92361f527ef_custom_imagery_and_description.py index 6a4fdcc..6f1bc64 100644 --- a/alembic_osm/versions/a92361f527ef_custom_imagery_and_description.py +++ b/alembic_osm/versions/a92361f527ef_custom_imagery_and_description.py @@ -28,10 +28,8 @@ def upgrade() -> None: "tasking_projects", sa.Column("description", sa.String(length=10000), nullable=True), ) - pass def downgrade() -> None: op.drop_column("tasking_projects", "custom_imagery") op.drop_column("tasking_projects", "description") - pass From 31bd237313810e111f528d33f638ebaefc705759 Mon Sep 17 00:00:00 2001 From: Naresh Kumar D Date: Tue, 14 Jul 2026 16:26:26 +0530 Subject: [PATCH 32/34] updated required settings --- README.md | 2 +- api/src/tasking/projects/repository.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index ebba814..06ba988 100644 --- a/README.md +++ b/README.md @@ -106,4 +106,4 @@ connecting to existing Databases. `docker compose --file docker-compose.local.yml down` -Backend code will be available at `http://localhost:3000` \ No newline at end of file +Backend code will be available at `http://localhost:8000` \ No newline at end of file diff --git a/api/src/tasking/projects/repository.py b/api/src/tasking/projects/repository.py index beee8e6..95875c1 100644 --- a/api/src/tasking/projects/repository.py +++ b/api/src/tasking/projects/repository.py @@ -654,6 +654,8 @@ async def patch( updates["review_required"] = body.review_required if body.custom_imagery is not None: updates["custom_imagery"] = body.custom_imagery # type: ignore[arg-type] + if body.description is not None: + updates["description"] = body.description if updates: updates["updated_at"] = datetime.now() From d7dc66cdd02a2d5df16e9ca7d02f9cc7ac1c192a Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Tue, 14 Jul 2026 10:18:05 -0400 Subject: [PATCH 33/34] Expand README with deployment architecture and services Added detailed deployment architecture and services information to README. --- README.md | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 73 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 06ba988..429a472 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,78 @@ following contract. `CLAUDE.md` has the full rationale. * ```develop``` merge your work here; keep this up to date with the "development" environment / dev tag * ```staging``` keep this up to date with the "staging" environment / stage tag * ```production``` keep this up to date with the "production" environment / prod tag + +## Deployment architecture + +The deployed system is defined by [`docker-compose.az.yml`](docker-compose.az.yml). It runs the +**application tier** as four containers; the **data tier** (Postgres/PostGIS) is external, managed +Azure Database for PostgreSQL, not part of this compose file. + +When deployed by `workspaces-stack` this model also holds. + +``` + client (TDEI/Keycloak JWT) + │ + ▼ :8000 + ┌──────────────────────────────────────┐ + │ workspaces-backend │ this repo — FastAPI front door. + │ authn/authz + OSM reverse proxy │ Serves /api/v1/*, proxies the rest. + └───┬──────────────────────────────┬────┘ + WS_OSM_HOST TASK_DATABASE_URL + (→ osm-rails) OSM_DATABASE_URL + │ │ + ▼ │ + ┌──────────────┐ │ + │ osm-rails │ OSM website (Rails); the single OSM entry point. + │ :3000 │ Serves the API/UI and fronts cgimap for the + └──────┬───────┘ performance-critical /api/0.6 calls. + │ (internal) │ + ▼ │ + ┌──────────────┐ │ + │ osm-cgimap │ C-accelerated /api/0.6 (map, changeset bulk) + │ :8000 │ │ + └──────────────┘ │ + │ + osm-rails-worker (rake jobs:work) │ background jobs + │ │ + │ backend, rails, cgimap, worker all connect to ▼ + ┌─────────────────────────────────────────────────────────────────┐ + │ data tier — Azure Postgres (external, PostGIS) │ + │ opensidewalks-${ENV}.postgres.database.azure.com:5432 │ + │ • workspaces-tasks-${ENV} TASK db (alembic_task; backend) │ + │ • workspaces-osm-${ENV} OSM db (alembic_osm; all four) │ + └─────────────────────────────────────────────────────────────────┘ +``` + +### Services + +| Service | Image | Role | +|---|---|---| +| `workspaces-backend` | `workspaces-backend-v2:${ENV}` | This repo. The only host-exposed service (`8000:8000`). Validates the TDEI/Keycloak JWT, enforces workspace authorization, serves `/api/v1/*`, and proxies everything else to the OSM tier. Connects to **both** databases. | +| `osm-rails` | `workspaces-osm-rails-v2:${ENV}` | The OpenStreetMap website (Rails) — the **single OSM entry point** the backend proxies to (`WS_OSM_HOST`). Serves the OSM API/UI and fronts cgimap for the heavy `/api/0.6` calls. Connects to the OSM db. | +| `osm-cgimap` | `workspaces-osm-cgimap-v2:${ENV}` | C++ reimplementation of the performance-critical OSM `0.6` calls (map queries, changeset upload/download), sitting behind `osm-rails`. Tuned here for large imports (`CGIMAP_MAX_*`). Connects to the OSM db. | +| `osm-rails-worker` | `workspaces-osm-rails-v2:${ENV}` | Background job runner (`rake jobs:work`) for the Rails app. Connects to the OSM db. | + +### Two databases + +The backend holds two connections, and the two alembic trees target them independently (see +`CLAUDE.md` and `api/utils/migrations.py`): + +* **TASK db** (`TASK_DATABASE_URL` → `workspaces-tasks-${ENV}`) — the workspaces + tasking-manager + schema, built by the `alembic_task` tree. Only the backend connects here. +* **OSM db** (`OSM_DATABASE_URL` → `workspaces-osm-${ENV}`) — OSM data plus `users` and the + `tasking_*` tables, built by the `alembic_osm` tree. The backend, cgimap, rails, and the worker + all connect here. + +On startup (outside of pytest) the backend runs `alembic -n task upgrade head` and +`alembic -n osm upgrade head`, applying each tree to its database. + +### Environment templating + +Every image tag, database name/user, and server host is parameterized by `${ENV}` +(`dev` / `stage` / `prod`), and secrets are injected from the shell environment +(`${WS_TASKS_DB_PASS}`, `${WS_OSM_DB_PASS}`, `${WS_OSM_SECRET_KEY_BASE}`). Branches map to these +environments — see the Branch Index below. ## To start on your local machine for dev work @@ -106,4 +178,4 @@ connecting to existing Databases. `docker compose --file docker-compose.local.yml down` -Backend code will be available at `http://localhost:8000` \ No newline at end of file +Backend code will be available at `http://localhost:8000` From 63fab7c65d64191c7df6cb9252317ce420642cfa Mon Sep 17 00:00:00 2001 From: Naresh Kumar D Date: Wed, 15 Jul 2026 14:38:06 +0530 Subject: [PATCH 34/34] Update README.md Readme updated with steps to run local development --- README.md | 61 ++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 49 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 429a472..b76ca45 100644 --- a/README.md +++ b/README.md @@ -159,18 +159,55 @@ uv run black api tests && uv run isort api tests Use the file `docker-compose.local.yml` to build and deploy local code changes. This allows you to run the entire system at once instead of connecting to existing Databases. -### Initial setup. -- On first launch, rails-worker will fail because migrations are not done -- Go to the `osm-rails` `/bin/sh` and execute `bundle exec rails db:migrate` -- The above script runs the migration code for osm-rails -- The rails-worker will be able to run -- The backend code will fail first time becase `workspaces-tasks-local` database is not available -- Login to postgresql container and run the following commands - `psql --username postgres` - `create database "workspaces-tasks-local";` - `psql --username postgres --dbname "workspaces-tasks-local";` - `create extension if not exists postgis;` -- Run the backend code now and it should be able to run +### Initial setup for development local environment + +Step 1: Login to azure docker + +The docker compose relies on images in `opensidewalksdev` azure container registry. Make sure your docker system is logged into it before pulling the images and trying to run the containers. + +Docker login command: + +`docker login opensidewalksdev.azurecr.io -u opensidewalksdev ` + +Password needs to be obtained from Azure portal + +Step 2: Run docker compose for the first time + +Use the following command to start the containers first time + +`docker compose --file docker-compose.local.yml up --build` + +Step 3: Run the migration scripts. + +You will observe that only `osm-rails` component seems to work but the backend and other services may be down. this is because the database migrations on the base osm database are not done. To do the base migrations, do the following: + +- Connect to the `osm-rails` container. If you are using docker hub for desktop, just go to the exec section of the container. + If you want to use command line, execute the command `docker exec -it /bin/bash` where `container_name` is the name of osm-rails container +- Execute the migration script in the /bin/bash with `bundle exec rails db:migrate` +- The above command runs the migration script for databases + +Step 4: Add `workspaces-tasks-local` database in postgresql + +Workspaces backend relies on an additional database. This is needed for some older migrations code. + +- Connect to `database` container. +- Run the following set of commands one by one + +```shell +psql --username postgres +create database "workspaces-tasks-local"; +exit; +psql --username postgres --dbname "workspaces-tasks-local"; +create extension if not exists postgis; + +``` + +Step 5: Restart the docker compose again + +- `docker compose --file docker-compose.local.yml down` +- `docker compose --file docker-compose.local.yml up --build` + + ### Commands to start and stop the docker compose