From feceeae4542e8b65523c3c95683427c1888499f1 Mon Sep 17 00:00:00 2001 From: Paulo Date: Sat, 22 Aug 2026 11:08:28 +0200 Subject: [PATCH] Revoking a connection is a state the row keeps, not a deletion A revoked OauthConnection keeps its owner, identity, scopes, and dates; only the refresh token is cleared, and the cached access token is evicted. Every delete site is now a revoke, so oauth.disconnected subscribers can read the row and extensions need no tombstone copies. Reads default to live rows; the audit surfaces show revoked rows as history. A fresh sign-in always creates a new connection; reconsent (?connection=) is the only way a revoked row returns to life. --- backend/druks/mcp/oauth.py | 7 +- backend/druks/mcp/routes.py | 7 +- backend/druks/services/base.py | 4 +- backend/druks/services/models.py | 64 ++++++--- backend/druks/services/oauth.py | 11 +- backend/druks/services/routes.py | 29 ++-- backend/druks/services/schemas.py | 2 + .../b6d4f2a81c93_revoking_is_a_state.py | 34 +++++ backend/tests/test_mcp_oauth.py | 27 ++++ backend/tests/test_oauth_client.py | 47 ++++++- backend/tests/test_services.py | 126 +++++++++++++++++- docs/writing-an-extension.md | 27 ++-- frontend/src/api/types.ts | 2 + frontend/src/components/SettingsModal.tsx | 54 +++++++- frontend/src/styles.css | 1 + 15 files changed, 382 insertions(+), 60 deletions(-) create mode 100644 backend/migrations/versions/b6d4f2a81c93_revoking_is_a_state.py diff --git a/backend/druks/mcp/oauth.py b/backend/druks/mcp/oauth.py index d2a46d71..f5b72780 100644 --- a/backend/druks/mcp/oauth.py +++ b/backend/druks/mcp/oauth.py @@ -26,7 +26,8 @@ def _http() -> httpx.AsyncClient: def get_connection(name: str, account_id: str) -> OauthConnection | None: - # One connection per (server, account) — MCP's policy over the shared table. + # One live connection per (server, account) — MCP's policy over the + # shared table. Revoked rows stay behind as history. rows = OauthConnection.list_for_account(grant_provider(name), account_id) return rows[0] if rows else None @@ -276,10 +277,10 @@ async def evict_access_token(name: str, account_id: str) -> None: await OauthClient(provider=grant_provider(name)).evict_access_token(connection.id) -async def disconnect(name: str, account_id: str) -> None: +async def disconnect(name: str, account_id: str, *, reason: str = "user") -> None: connection = get_connection(name, account_id) if connection: - await OauthClient(provider=grant_provider(name)).disconnect(connection) + await OauthClient(provider=grant_provider(name)).disconnect(connection, reason=reason) registration = McpClientRegistration.get_for_account(name, account_id) if registration: registration.delete() diff --git a/backend/druks/mcp/routes.py b/backend/druks/mcp/routes.py index 111905f1..6830356d 100644 --- a/backend/druks/mcp/routes.py +++ b/backend/druks/mcp/routes.py @@ -163,11 +163,10 @@ async def remove_mcp_server(name: str) -> None: server = McpServer.get_for_name(name) if not server: raise HTTPException(status_code=404, detail=f"MCP server {name!r} not found") - connections = oauth.list_connections(name) + # Revoke before the server row goes — the registration lookup needs it. + for connection in oauth.list_connections(name): + await oauth.disconnect(name, connection.account_id, reason="server_removed") server.delete() - for connection in connections: - await oauth.evict_access_token(name, connection.account_id) - connection.delete() @router.post("/{name}/connect", response_model=ConnectMcpServerResponse) diff --git a/backend/druks/services/base.py b/backend/druks/services/base.py index 5714140c..fb6976be 100644 --- a/backend/druks/services/base.py +++ b/backend/druks/services/base.py @@ -43,7 +43,7 @@ async def get_access_token(self, scopes: tuple[str, ...] = (), cached: bool = Tr ) async def disconnect(self) -> None: - await OauthClient(provider=self.service.name).disconnect(self.row) + await OauthClient(provider=self.service.name).disconnect(self.row, reason="user") class ScopedService: @@ -72,7 +72,7 @@ def list_for_account(self, account_id: str) -> list[Connection]: def get(self, connection_id: str) -> Connection | None: row = OauthConnection.get(connection_id) - if row and row.provider == self.service.name: + if row and row.provider == self.service.name and not row.revoked_at: return Connection(self.service, row) diff --git a/backend/druks/services/models.py b/backend/druks/services/models.py index 1fa58c12..b9d3b5fd 100644 --- a/backend/druks/services/models.py +++ b/backend/druks/services/models.py @@ -51,7 +51,11 @@ class OauthConnection(Base, Uuid7Pk): """One signed-in provider account: the durable outcome of an OAuth consent, owned by the druks account that completed it. An account can hold many per provider — one per mailbox, handle, or workspace. The - engine rotates the refresh token on mint; nothing else writes here.""" + engine rotates the refresh token on mint; nothing else writes here. + + Revoking is a state, never a deletion: the consent happened, and the row + keeps its owner, identity, scopes, and dates forever. Only the refresh + token is cleared. ``reconnect`` returns a revoked row to life.""" __tablename__ = "oauth_connections" @@ -67,6 +71,10 @@ class OauthConnection(Base, Uuid7Pk): # consent; {} when the provider gave none. identity: Mapped[dict[str, Any]] = mapped_column(JSONB, default=dict) connected_at: Mapped[datetime] = mapped_column(default=Base.utc_now) + # NULL = live. Stamped with what revoked it: "user", "client_replaced", + # or "server_removed". + revoked_at: Mapped[datetime | None] = mapped_column(default=None) + revoked_reason: Mapped[str] = mapped_column(default="") @classmethod def get(cls, connection_id: str) -> "OauthConnection | None": @@ -98,17 +106,28 @@ def list_for_account(cls, provider: str, account_id: str) -> "list[OauthConnecti return list( db_session().scalars( select(cls) - .where(cls.provider == provider, cls.account_id == account_id) + .where( + cls.provider == provider, + cls.account_id == account_id, + cls.revoked_at.is_(None), + ) .order_by(cls.connected_at) ) ) @classmethod - def list_for_provider(cls, provider: str) -> "list[OauthConnection]": - return list(db_session().scalars(select(cls).where(cls.provider == provider))) + def list_for_provider( + cls, provider: str, *, include_revoked: bool = False + ) -> "list[OauthConnection]": + query = select(cls).where(cls.provider == provider) + if not include_revoked: + query = query.where(cls.revoked_at.is_(None)) + return list(db_session().scalars(query)) @classmethod def list_owned_by(cls, account_id: str | None) -> "list[OauthConnection]": + # The audit read: everything this account ever authorized, revoked + # rows included. return list( db_session().scalars( select(cls).where(cls.account_id == account_id).order_by(cls.connected_at) @@ -123,12 +142,17 @@ def reconnect( if identity: self.identity = identity self.connected_at = Base.utc_now() + self.revoked_at = None + self.revoked_reason = "" db_session().flush() - def delete(self) -> None: - session = db_session() - session.delete(self) - session.flush() + def revoke(self, reason: str) -> None: + # The consent's facts survive; only the secret is cleared. A second + # revoke keeps the first stamp. + self.revoked_at = self.revoked_at or Base.utc_now() + self.revoked_reason = self.revoked_reason or reason + self.refresh_token = "" + db_session().flush() def _load_refresh_token(self) -> str: # Under the refresh lock: another process may have rotated and @@ -141,11 +165,11 @@ def _load_refresh_token(self) -> str: .where(OauthConnection.id == self.id) .execution_options(populate_existing=True) ) - .one_or_none() + .one() ) - if fresh: - return fresh.refresh_token.decrypt() - raise OauthRefreshError(self.provider, "the connection was removed mid-refresh") + if fresh.revoked_at: + raise OauthRefreshError(self.provider, "the connection was revoked mid-refresh") + return fresh.refresh_token.decrypt() def _save_refresh_token(self, rotated: str) -> None: # The provider invalidated the old token the moment it rotated, so @@ -153,12 +177,18 @@ def _save_refresh_token(self, rotated: str) -> None: # transaction — a step that rolls back later must not brick the # connection. with get_session(db_session().get_bind()) as session: - session.execute( + stored = session.execute( update(OauthConnection) - .where(OauthConnection.id == self.id) + .where(OauthConnection.id == self.id, OauthConnection.revoked_at.is_(None)) .values(refresh_token=rotated) ) session.commit() - # Keep the enclosing transaction's copy true as well. - self.refresh_token = rotated - db_session().flush() + if not stored.rowcount: + # A revoke landed mid-refresh. Nothing secret outlives the + # consent at rest, so the rotated token is not stored. + raise OauthRefreshError(self.provider, "the connection was revoked mid-refresh") + # Expire the stale copy instead of assigning it. The next read loads + # the rotated value, and the enclosing transaction never writes this + # column at commit — a revoke that lands between the two commits + # must keep its cleared token. + db_session().expire(self, ["refresh_token"]) diff --git a/backend/druks/services/oauth.py b/backend/druks/services/oauth.py index 0ac1add3..10575dea 100644 --- a/backend/druks/services/oauth.py +++ b/backend/druks/services/oauth.py @@ -184,6 +184,10 @@ async def get_access_token( scopes. ``cached=False`` skips the cache read for a full-lifetime token, still electing one refresher and filling the cache for later callers.""" + if connection.revoked_at: + raise OauthRefreshError( + self.provider, "the connection is revoked; sign in again to restore it" + ) requested = tuple(sorted(scopes)) if requested and not set(requested) <= set(connection.scopes): missing = ", ".join(sorted(set(requested) - set(connection.scopes))) @@ -273,9 +277,10 @@ async def evict_access_token(self, connection_id: str) -> None: async for key in redis.scan_iter(match=f"{self.provider}:access_token:{connection_id}*"): await redis.delete(key) - async def disconnect(self, connection: OauthConnection) -> None: - """Delete the connection and evict its cached access token.""" - connection.delete() + async def disconnect(self, connection: OauthConnection, *, reason: str) -> None: + """Revoke the connection and evict its cached access token. The row + and its facts survive; the refresh token dies with the consent.""" + connection.revoke(reason) await self.evict_access_token(connection.id) diff --git a/backend/druks/services/routes.py b/backend/druks/services/routes.py index dd7bfac0..26320f89 100644 --- a/backend/druks/services/routes.py +++ b/backend/druks/services/routes.py @@ -29,7 +29,8 @@ async def list_services() -> list[ServiceResponse]: row = None connections = [] if service.token_endpoint: - connections = OauthConnection.list_for_provider(service.name) + # The detail shows revoked connections as history beside the live. + connections = OauthConnection.list_for_provider(service.name, include_revoked=True) entries.append(ServiceResponse.from_row(service, row, connections)) return entries @@ -51,16 +52,16 @@ async def connect_service(name: str, payload: dict[str, str]) -> ServiceResponse except ServiceConnectError as error: raise HTTPException(status_code=422, detail=str(error)) from error if service.token_endpoint: - # A replaced client can never refresh the old client's connections. + # A replaced client can never refresh the old client's connections — + # revoke every live one; the consents stay on record. client = OauthClient(provider=name) for connection in OauthConnection.list_for_provider(name): - connection_id, account_id = connection.id, connection.account_id - await client.disconnect(connection) + await client.disconnect(connection, reason="client_replaced") await publish( "oauth.disconnected", provider=name, - connection_id=connection_id, - account_id=account_id, + connection_id=connection.id, + account_id=connection.account_id, ) return ServiceResponse.from_row(service, row) @@ -132,8 +133,10 @@ async def oauth_callback(state: str = "", code: str = "", error: str = "") -> Re raise HTTPException( status_code=400, detail="The connection was removed while consent was open." ) + # Reconsent names the row, so it also returns a revoked one to life. + # A fresh sign-in never does — it always creates a new connection. row.reconnect(refresh_token=tokens["refresh_token"], scopes=granted, identity=identity) - # A reconsent's narrower cached token must not serve until its TTL runs out. + # A token cached before this consent must not serve the new one. await OauthClient(provider=provider).evict_access_token(row.id) else: row = OauthConnection.create( @@ -170,11 +173,13 @@ async def disconnect_connection(connection_id: str) -> None: row = OauthConnection.get(connection_id) if not row: raise HTTPException(status_code=404, detail=f"No connection {connection_id!r}.") - provider, account_id = row.provider, row.account_id - await OauthClient(provider=provider).disconnect(row) + if row.revoked_at: + # Revoking is idempotent — the second delete finds the state true. + return + await OauthClient(provider=row.provider).disconnect(row, reason="user") await publish( "oauth.disconnected", - provider=provider, - connection_id=connection_id, - account_id=account_id, + provider=row.provider, + connection_id=row.id, + account_id=row.account_id, ) diff --git a/backend/druks/services/schemas.py b/backend/druks/services/schemas.py index 35ab045f..db662f08 100644 --- a/backend/druks/services/schemas.py +++ b/backend/druks/services/schemas.py @@ -28,6 +28,8 @@ class ConnectionResponse(BaseResponse): scopes: list[str] identity: dict[str, Any] connected_at: datetime + revoked_at: datetime | None + revoked_reason: str class ServiceResponse(BaseResponse): diff --git a/backend/migrations/versions/b6d4f2a81c93_revoking_is_a_state.py b/backend/migrations/versions/b6d4f2a81c93_revoking_is_a_state.py new file mode 100644 index 00000000..fce29bb2 --- /dev/null +++ b/backend/migrations/versions/b6d4f2a81c93_revoking_is_a_state.py @@ -0,0 +1,34 @@ +"""revoking is a state + +Revision ID: b6d4f2a81c93 +Revises: c4f8a2d97e15 +Create Date: 2026-08-22 00:00:00.000000 + +""" + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "b6d4f2a81c93" +down_revision: str | Sequence[str] | None = "c4f8a2d97e15" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + op.add_column( + "oauth_connections", + sa.Column("revoked_at", sa.DateTime(timezone=True), nullable=True), + ) + op.add_column( + "oauth_connections", + sa.Column("revoked_reason", sa.String(), server_default="", nullable=False), + ) + + +def downgrade() -> None: + op.drop_column("oauth_connections", "revoked_reason") + op.drop_column("oauth_connections", "revoked_at") diff --git a/backend/tests/test_mcp_oauth.py b/backend/tests/test_mcp_oauth.py index cd8a1abd..6c0b1c29 100644 --- a/backend/tests/test_mcp_oauth.py +++ b/backend/tests/test_mcp_oauth.py @@ -389,6 +389,33 @@ async def test_reconsent_replaces_the_grant_and_evicts_the_stale_token(auth_serv assert not await get_client().get(_token_key(SYSTEM_ACCOUNT_ID)) +async def test_reconnect_after_disconnect_creates_a_new_grant(auth_server, druks_db): + _store_grant(refresh_token="rt-stale") + revoked = oauth.get_connection(_NAME, SYSTEM_ACCOUNT_ID) + await oauth.disconnect(_NAME, SYSTEM_ACCOUNT_ID) + assert not oauth.get_connection(_NAME, SYSTEM_ACCOUNT_ID) + + url = await oauth.begin_connect( + _NAME, + _SERVER_URL, + _ENDPOINT, + account_id=SYSTEM_ACCOUNT_ID, + identity_mode=IdentityMode.SHARED, + ) + state = dict(parse_qsl(urlparse(url).query))["state"] + await oauth.complete_connect(state=state, code="code-1") + + from druks.database import db_session + + db_session().expire_all() + # A re-connect creates a new grant. The revoked row stays as history, + # so at most one live connection holds the (server, account) slot. + grant = oauth.get_connection(_NAME, SYSTEM_ACCOUNT_ID) + assert grant.id != revoked.id + assert grant.refresh_token.decrypt() == "rt-1" + assert revoked.revoked_at + + async def test_two_shared_connects_converge_on_one_grant(auth_server, druks_db): first = Account.get_or_create("first@example.com") second = Account.get_or_create("second@example.com") diff --git a/backend/tests/test_oauth_client.py b/backend/tests/test_oauth_client.py index c4c79a8e..8d8021c7 100644 --- a/backend/tests/test_oauth_client.py +++ b/backend/tests/test_oauth_client.py @@ -170,15 +170,54 @@ async def test_get_refresh_uses_basic_auth(token_endpoint): assert "client_secret" not in token_endpoint.requests[0] -async def test_disconnect_drops_the_connection_and_the_cached_token(token_endpoint): +async def test_disconnect_revokes_the_connection_and_drops_the_cached_token(token_endpoint): connection = _connection() await get_client().set(_token_key(connection), "at-cached") - await _client().disconnect(connection) + await _client().disconnect(connection, reason="user") - assert not OauthConnection.get(connection.id) + revoked = OauthConnection.get(connection.id) + assert revoked.revoked_at + assert revoked.revoked_reason == "user" + # Nothing secret outlives the consent at rest. + assert not revoked.refresh_token + assert revoked.account_id == SYSTEM_ACCOUNT_ID assert not await get_client().get(_token_key(connection)) + # A second revoke keeps the first stamp. + first_stamp = revoked.revoked_at + revoked.revoke("client_replaced") + assert revoked.revoked_at == first_stamp + assert revoked.revoked_reason == "user" + + +async def test_a_revoke_landing_mid_refresh_is_not_overwritten(token_endpoint): + connection = _connection() + exchange = token_endpoint.handler + + def revoke_then_rotate(request: httpx.Request) -> httpx.Response: + connection.revoke("user") + return exchange(request) + + token_endpoint.handler = revoke_then_rotate + + with pytest.raises(OauthRefreshError, match="revoked mid-refresh"): + await _client().get_access_token(connection=connection) + + # The rotated token is not stored and no access token is cached. + assert not OauthConnection.get(connection.id).refresh_token + assert not await get_client().get(_token_key(connection)) + + +async def test_get_refuses_a_revoked_connection(token_endpoint): + connection = _connection() + connection.revoke("user") + + with pytest.raises(OauthRefreshError, match="revoked"): + await _client().get_access_token(connection=connection) + + assert not token_endpoint.requests + async def test_connect_roundtrip_exchanges_with_basic_auth(token_endpoint): url = await _client(basic_auth=True).begin_connect( @@ -326,7 +365,7 @@ async def test_disconnect_evicts_the_scope_variant_keys(token_endpoint): await redis.set(_token_key(connection), "at-full") await redis.set(scoped_key, "at-narrow") - await _client().disconnect(connection) + await _client().disconnect(connection, reason="user") assert not await redis.get(_token_key(connection)) assert not await redis.get(scoped_key) diff --git a/backend/tests/test_services.py b/backend/tests/test_services.py index 132a0ab3..7a558b2e 100644 --- a/backend/tests/test_services.py +++ b/backend/tests/test_services.py @@ -8,6 +8,7 @@ import pytest from druks.core.apis.github import GitHubClient, get_github_client from druks.core.webhooks.github import GitHubEvents +from druks.database import db_session from druks.services.exceptions import ServiceNotConnectedError from druks.services.models import ServiceIdentity from druks.testing import make_settings @@ -522,6 +523,12 @@ class Digest: assert NightWatch.acme.get(row.id).id == row.id assert not NightWatch.acme.get("missing") + # The handle serves live connections only; the revoked row survives. + row.revoke("user") + assert not NightWatch.acme.list_for_account(SYSTEM_ACCOUNT_ID) + assert not NightWatch.acme.get(row.id) + assert OauthConnection.get(row.id).identity == {"email": "night@acme.test"} + async def test_get_identity_without_a_declared_endpoint_is_empty(declared_services): from druks.services import Service @@ -715,6 +722,104 @@ def test_oauth_connect_rejects_an_unknown_reconnect_target(tmp_path, acme, druks assert client.get("/api/oauth/acme/connect?connection=zzz").status_code == 404 +def test_fresh_sign_in_after_revoke_creates_a_new_connection(tmp_path, acme, druks_db, monkeypatch): + from urllib.parse import parse_qsl, urlparse + + from druks.services.models import OauthConnection + from druks.testing import configure_app_for_test + + ServiceIdentity.connect( + "acme", identity={"client_id": "id-1"}, secrets={"client_secret": "sec-1"} + ) + published = [] + + async def record(name, **kwargs): + published.append((name, kwargs)) + + monkeypatch.setattr("druks.services.routes.publish", record) + settings = make_settings(tmp_path, urls={"endpoint": "https://druks.example"}) + with TestClient(configure_app_for_test(settings=settings)) as client: + + def sign_in(): + consent = client.get("/api/oauth/acme/connect", follow_redirects=False) + state = dict(parse_qsl(urlparse(consent.headers["location"]).query))["state"] + assert ( + client.get( + "/api/oauth/callback", params={"state": state, "code": "c-1"} + ).status_code + == 200 + ) + + sign_in() + [first] = OauthConnection.list_for_provider("acme") + assert client.delete(f"/api/oauth/connections/{first.id}").status_code == 204 + + # A fresh sign-in never reuses a row, even for the same provider + # account. The revoked row stays behind as history. + sign_in() + [live] = OauthConnection.list_for_provider("acme") + assert live.id != first.id + assert len(OauthConnection.list_for_provider("acme", include_revoked=True)) == 2 + + events = [name for name, _ in published] + assert events == ["oauth.connected", "oauth.disconnected", "oauth.connected"] + assert published[-1][1] == { + "provider": "acme", + "connection_id": live.id, + "account_id": live.account_id, + "reconsent": False, + } + + +def test_reconsent_returns_a_revoked_connection_to_life(tmp_path, acme, druks_db, monkeypatch): + from urllib.parse import parse_qsl, urlparse + + from druks.services.models import OauthConnection + from druks.testing import configure_app_for_test + + ServiceIdentity.connect( + "acme", identity={"client_id": "id-1"}, secrets={"client_secret": "sec-1"} + ) + published = [] + + async def record(name, **kwargs): + published.append((name, kwargs)) + + monkeypatch.setattr("druks.services.routes.publish", record) + settings = make_settings(tmp_path, urls={"endpoint": "https://druks.example"}) + with TestClient(configure_app_for_test(settings=settings)) as client: + consent = client.get("/api/oauth/acme/connect", follow_redirects=False) + state = dict(parse_qsl(urlparse(consent.headers["location"]).query))["state"] + client.get("/api/oauth/callback", params={"state": state, "code": "c-1"}) + [connection] = OauthConnection.list_for_provider("acme") + assert client.delete(f"/api/oauth/connections/{connection.id}").status_code == 204 + + # Reconsent names the row, so it is the one way back to life. + reconnect = client.get( + f"/api/oauth/acme/connect?connection={connection.id}", follow_redirects=False + ) + state = dict(parse_qsl(urlparse(reconnect.headers["location"]).query))["state"] + assert ( + client.get("/api/oauth/callback", params={"state": state, "code": "c-2"}).status_code + == 200 + ) + + # The routes wrote in their own transactions; drop stale instances. + db_session().expire_all() + [live] = OauthConnection.list_for_provider("acme") + assert live.id == connection.id + assert not live.revoked_at + assert not live.revoked_reason + assert live.refresh_token.decrypt() == "rt-1" + + assert published[-1][1] == { + "provider": "acme", + "connection_id": live.id, + "account_id": live.account_id, + "reconsent": True, + } + + def test_connections_list_and_revoke(tmp_path, acme, druks_db, monkeypatch): from druks.accounts.models import Account from druks.services.models import OauthConnection @@ -735,10 +840,22 @@ async def record(name, **kwargs): assert listed["id"] == row.id assert listed["provider"] == "acme" assert listed["scopes"] == ["profile.read"] + assert listed["revokedAt"] is None assert client.delete(f"/api/oauth/connections/{row.id}").status_code == 204 assert not OauthConnection.list_for_provider("acme") - assert client.delete(f"/api/oauth/connections/{row.id}").status_code == 404 + # The route revoked in its own transaction; drop the stale instance. + db_session().expire_all() + revoked = OauthConnection.get(row.id) + assert revoked.revoked_at + assert revoked.revoked_reason == "user" + assert not revoked.refresh_token + # The audit read keeps serving the revoked row as history. + [listed] = client.get("/api/oauth/connections").json() + assert listed["revokedAt"] + assert listed["revokedReason"] == "user" + # Revoking is idempotent: the second delete finds the state true. + assert client.delete(f"/api/oauth/connections/{row.id}").status_code == 204 assert published == [ ( @@ -748,7 +865,7 @@ async def record(name, **kwargs): ] -def test_replacing_the_client_credentials_deletes_its_connections( +def test_replacing_the_client_credentials_revokes_its_connections( tmp_path, acme, druks_db, monkeypatch ): from druks.accounts.constants import SYSTEM_ACCOUNT_ID @@ -772,7 +889,12 @@ async def record(name, **kwargs): assert response.status_code == 200 # The new client can never refresh the old client's connections. + db_session().expire_all() assert not OauthConnection.list_for_provider("acme") + [revoked] = OauthConnection.list_for_provider("acme", include_revoked=True) + assert revoked.id == row.id + assert revoked.revoked_reason == "client_replaced" + assert not revoked.refresh_token assert published == [ ( "oauth.disconnected", diff --git a/docs/writing-an-extension.md b/docs/writing-an-extension.md index 4b8e8e8a..bf5352f1 100644 --- a/docs/writing-an-extension.md +++ b/docs/writing-an-extension.md @@ -842,13 +842,19 @@ for connection in NightWatch.acme.list_for_account(account_id): `current_account_id.get()` in a route, the handler's argument in a subscriber. `NightWatch.acme.get(connection_id)` returns one connection when your own row stored its id. Each connection carries `id`, `scopes`, `identity` — the -provider's facts for the sign-in — and `connected_at`. +provider's facts for the sign-in — and `connected_at`. The handle serves +live connections only. A revoked connection drops out of `get` and +`list_for_account`, but its platform row survives with its owner and +identity. Your rows never need tombstone copies of either. Your UI starts a sign-in by opening `/api/oauth/acme/connect` — the platform runs the consent with the union of every installed extension's -declared scopes and stores the connection for the signed-in user. To widen -an existing connection's scopes, open +declared scopes and stores the connection for the signed-in user. A fresh +sign-in always creates a new connection, even for a provider account that +was connected before. To widen an existing connection's scopes, open `/api/oauth/acme/connect?connection=`; reconsent replaces its tokens. +Reconsent names the row, so it also returns a revoked connection to life +under its old id — the only way a row comes back. Add `?next=/app/night_watch/accounts` to land the user back on your page after consent instead of the generic "connected" page. `next` must be a bare path starting with `/` — a URL with a scheme or host is @@ -857,10 +863,12 @@ rejected, so the door can never redirect off the box. Register serves every service. React to sign-ins with the signal machinery. The platform publishes -`oauth.connected` when a consent completes — `reconsent` is true when it -replaced an existing connection's tokens — and `oauth.disconnected` when a -connection dies, whether the user revoked it or the service's client -credentials were replaced. Subscribe in `subscribers.py`: +`oauth.connected` when a consent completes. `reconsent` is true when it +replaced an existing connection's tokens, including a revoked row that +returned to life. It publishes `oauth.disconnected` when a connection is +revoked — by the user, or because the service's client credentials were +replaced. Revocation is a state, not a deletion: your subscriber can still +read the connection it is told about. Subscribe in `subscribers.py`: ```python from druks.signals import subscribe @@ -880,8 +888,9 @@ async def drop_sign_in(provider: str, connection_id: str, account_id: str) -> No ``` The user sees and revokes everything in Settings — every connection they -hold, across services. Replacing a service's client credentials deletes its -connections: a new client can never refresh the old client's tokens. +hold, across services, revoked ones shown as history. Replacing a service's +client credentials revokes its connections: a new client can never refresh +the old client's tokens, but the consents stay on record. `get_access_token` serves a Redis-cached access token and lets only one refresher run per connection and scope set. This is necessary: two diff --git a/frontend/src/api/types.ts b/frontend/src/api/types.ts index 759ee877..8eccf60e 100644 --- a/frontend/src/api/types.ts +++ b/frontend/src/api/types.ts @@ -288,6 +288,8 @@ export interface Connection { scopes: string[] identity: Record connectedAt: string + revokedAt: string | null + revokedReason: string } /** One declared service: the appliance's own registered app at an external diff --git a/frontend/src/components/SettingsModal.tsx b/frontend/src/components/SettingsModal.tsx index 2fe7103f..59a909a8 100644 --- a/frontend/src/components/SettingsModal.tsx +++ b/frontend/src/components/SettingsModal.tsx @@ -1108,6 +1108,18 @@ function connectionIdentity(connection: Connection): string | null { return identity.email ?? identity.username ?? identity.login ?? identity.name ?? null } +const revokeReasonCopy: Record = { + user: 'by you', + client_replaced: 'client credentials replaced', + server_removed: 'server removed', +} + +function revokedCopy(connection: Connection): string { + const reason = revokeReasonCopy[connection.revokedReason] + const when = new Date(connection.revokedAt ?? '').toLocaleDateString() + return `revoked ${when}` + (reason ? ` · ${reason}` : '') +} + // The signed-in accounts behind this service, on top of the pasted client // credentials. Connect opens the consent redirect in a new tab; the callback // page broadcasts on druks-service-connect and the pane refetches. @@ -1132,6 +1144,8 @@ function ServiceAccess({ service }: { service: Service }) { } const missingScopes = (connection: Connection) => service.requiredScopes.filter((scope) => !connection.scopes.includes(scope)) + const live = service.connections.filter((connection) => !connection.revokedAt) + const revoked = service.connections.filter((connection) => connection.revokedAt) return (
@@ -1152,7 +1166,7 @@ function ServiceAccess({ service }: { service: Service }) { {service.usedBy.join(', ')}
)} - {service.connections.map((connection) => ( + {live.map((connection) => (
{connectionIdentity(connection) ?? @@ -1179,9 +1193,27 @@ function ServiceAccess({ service }: { service: Service }) {
))} + {revoked.map((connection) => ( +
+ + {connectionIdentity(connection) ?? + new Date(connection.connectedAt).toLocaleDateString()} + + {revokedCopy(connection)} + + + +
+ ))}
@@ -1208,12 +1240,16 @@ export function ConnectionsPane() { } const connections = query.data ?? [] + const live = connections.filter((connection) => !connection.revokedAt) + const revoked = connections.filter((connection) => connection.revokedAt) return (

Connections

-

The accounts you have signed in to. Revoke one here.

+

+ The accounts you have signed in to. Revoke one here; revoked ones stay as history. +

{error && (
@@ -1223,7 +1259,7 @@ export function ConnectionsPane() { {connections.length === 0 &&

No connections yet.

} {connections.length > 0 && (
- {connections.map((connection) => ( + {live.map((connection) => (
{connection.provider} @@ -1235,6 +1271,16 @@ export function ConnectionsPane() {
))} + {revoked.map((connection) => ( +
+ {connection.provider} + + {connectionIdentity(connection) ?? (connection.scopes.join(', ') || 'unlabeled')} ·{' '} + connected {new Date(connection.connectedAt).toLocaleDateString()} ·{' '} + {revokedCopy(connection)} + +
+ ))}
)}
diff --git a/frontend/src/styles.css b/frontend/src/styles.css index 95a420c5..58eb57c0 100644 --- a/frontend/src/styles.css +++ b/frontend/src/styles.css @@ -1236,6 +1236,7 @@ textarea.set-textarea { background-image: none; height: auto; min-height: 96px; .svc-fact { display: flex; gap: 12px; min-width: 0; font-family: var(--font-mono); font-size: 12px; } .svc-fact-key { color: var(--text-dim); flex-shrink: 0; } .svc-fact-val { color: var(--text); white-space: nowrap; overflow: hidden; text-overflow: ellipsis; } +.svc-revoked { opacity: 0.55; } .svc-meta { margin: 0; font-size: 12px; color: var(--text-faint); } .svc-actions { display: flex; align-items: center; gap: 8px; flex-wrap: wrap; } .svc-alt { align-self: flex-start; padding: 0; border: none; background: none; font: inherit; font-size: 12.5px; color: var(--text-mid); cursor: pointer; text-decoration: underline; text-underline-offset: 3px; }