From cd4cd645de338dda1dd8708f81d94c201434eb74 Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Fri, 10 Jul 2026 16:21:49 -0400 Subject: [PATCH] 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):