diff --git a/backend/druks/chat/channels/whatsapp/routes.py b/backend/druks/chat/channels/whatsapp/routes.py index a034e4b0f..f2646afcd 100644 --- a/backend/druks/chat/channels/whatsapp/routes.py +++ b/backend/druks/chat/channels/whatsapp/routes.py @@ -7,9 +7,7 @@ from druks.accounts.models import Account from druks.api.dependencies import SessionDep from druks.apps.loader import get_app -from druks.chat.bots.service import resume from druks.chat.enums import BotAccess -from druks.chat.models import Conversation from druks.secrets.models import VaultSecret from .schemas import QrResponse, SessionResponse @@ -97,6 +95,3 @@ async def remove_session( # Removing is idempotent: a second delete finds the session removed. if connection.is_live: await Waha.unlink(session, connection, "user") - # The number's chats never take a turn again, so their pauses end. - for conversation in await Conversation.list_for_connection(session, connection.id): - await resume(session, conversation) diff --git a/backend/druks/chat/channels/whatsapp/services.py b/backend/druks/chat/channels/whatsapp/services.py index cc839b6de..4db7ccb65 100644 --- a/backend/druks/chat/channels/whatsapp/services.py +++ b/backend/druks/chat/channels/whatsapp/services.py @@ -6,6 +6,8 @@ from druks.accounts.enums import AccountKind from druks.accounts.models import Account +from druks.chat.bots.service import resume +from druks.chat.models import Conversation from druks.secrets.enums import IdentityStatus, SecretKind from druks.secrets.models import VaultSecret from druks.services import Service @@ -100,6 +102,22 @@ async def unlink(cls, session: AsyncSession, connection: VaultSecret, reason: st await card.delete_session(connection.identity["session"]) await card.delete_key(connection.secrets["key_id"]) await connection.revoke(reason) + # The number's chats never take a turn again, so their pauses end. + for conversation in await Conversation.list_for_connection(session, connection.id): + await resume(session, conversation) + + @classmethod + async def disconnect(cls, session: AsyncSession) -> None: + """Remove every linked number first, because only the card's key deletes a session.""" + for connection in await session.scalars( + select(VaultSecret).where( + VaultSecret.kind == SecretKind.SESSION, + VaultSecret.audience == WAHA_AUDIENCE, + VaultSecret.revoked_at.is_(None), + ) + ): + await cls.unlink(session, connection, "service_disconnected") + await super().disconnect(session) @classmethod async def relink(cls, session: AsyncSession, connection: VaultSecret) -> None: diff --git a/backend/druks/services/base.py b/backend/druks/services/base.py index a273c29e0..8aa4c50a1 100644 --- a/backend/druks/services/base.py +++ b/backend/druks/services/base.py @@ -5,6 +5,7 @@ from typing import Any, ClassVar from pydantic import BaseModel, ValidationError +from sqlalchemy.ext.asyncio import AsyncSession from druks.apps.base import NAME_RE from druks.apps.loader import iter_apps @@ -413,3 +414,13 @@ async def connect(cls, payload: dict[str, str]) -> VaultSecret: ): await client.disconnect(connection, reason="client_replaced") return row + + @classmethod + async def disconnect(cls, session: AsyncSession) -> None: + """Revoke the card and every account's sign-in. The provider keeps its application.""" + audience = Audience.service(cls.slug) + client = OauthClient(provider=cls.slug) + for connection in await VaultSecret.list_connections(session, audience): + await client.disconnect(connection, reason="service_disconnected") + if card := await VaultSecret.lookup(session, cls.secret_kind, audience): + await card.revoke("user") diff --git a/backend/druks/services/routes.py b/backend/druks/services/routes.py index 211157cdb..bd50ee88f 100644 --- a/backend/druks/services/routes.py +++ b/backend/druks/services/routes.py @@ -59,6 +59,14 @@ async def connect_service(slug: str, payload: dict[str, str]) -> ServiceResponse return ServiceResponse.from_row(service, row) +@router.delete("/{slug}", status_code=204, dependencies=[Depends(current_session_account)]) +async def disconnect_service(session: SessionDep, slug: str) -> None: + service = services.get(slug) + if not service: + raise HTTPException(status_code=404, detail=f"No service {slug!r}.") + await service.disconnect(session) + + def _get_oauth_service(slug: str): service = services.get(slug) if not service or not service.token_endpoint: diff --git a/backend/tests/test_auth_boundary.py b/backend/tests/test_auth_boundary.py index 7b02bcca1..7976ccec5 100644 --- a/backend/tests/test_auth_boundary.py +++ b/backend/tests/test_auth_boundary.py @@ -53,6 +53,7 @@ ("DELETE", "/api/providers/{provider_id}/connection"), ("PATCH", "/api/settings/apps"), ("POST", "/api/services/{slug}"), + ("DELETE", "/api/services/{slug}"), ("GET", "/api/oauth/{slug}/connect"), ("GET", "/api/oauth/callback"), ("GET", "/api/core/services/github/manifest/callback"), diff --git a/backend/tests/test_service_disconnect.py b/backend/tests/test_service_disconnect.py new file mode 100644 index 000000000..fc673388c --- /dev/null +++ b/backend/tests/test_service_disconnect.py @@ -0,0 +1,33 @@ +from conftest import bind_ambient_session, connect_service +from druks.accounts.models import Account +from druks.secrets.datastructures import Audience +from druks.secrets.models import VaultSecret + + +async def test_disconnect_revokes_the_card_and_every_sign_in_of_that_service_only( + druks_db, druks_client +): + bind_ambient_session(druks_db) + card = await connect_service( + "github", identity={"app_id": "123"}, secrets={"private_key": "private"} + ) + other = await connect_service( + "github_reviewer", identity={"app_id": "456"}, secrets={"private_key": "reviewer"} + ) + account = await Account.get_or_create(druks_db, "one@example.com") + connection = await VaultSecret.connect( + druks_db, + Audience.service("github"), + account_id=account.id, + refresh_token="refresh", + scopes=[], + ) + + for _ in range(2): + assert (await druks_client.delete("/api/services/github")).status_code == 204 + + for row in (card, other, connection): + await druks_db.refresh(row) + assert card.revoked_at + assert other.is_live + assert connection.revoked_reason == "service_disconnected" diff --git a/backend/tests/test_whatsapp.py b/backend/tests/test_whatsapp.py index d6e624ec9..2b8575c17 100644 --- a/backend/tests/test_whatsapp.py +++ b/backend/tests/test_whatsapp.py @@ -23,7 +23,6 @@ from druks.chat.bots import service as bot_service from druks.chat.bots.constants import OPERATOR_PAIRED_MESSAGE, PAUSE_TOPIC from druks.chat.bridge import Bridge -from druks.chat.channels.whatsapp import routes from druks.chat.channels.whatsapp.channel import WhatsAppChannel from druks.chat.channels.whatsapp.client import WahaClient from druks.chat.channels.whatsapp.constants import WAHA_AUDIENCE @@ -926,7 +925,7 @@ async def test_removing_a_number_holds_its_chats_and_relinking_keeps_its_history await receive(first, message_event(ANA, "Hello", key="M1")) [chat] = await Conversation.list_for_connection(druks_db, first.id) resume = AsyncMock() - monkeypatch.setattr(routes, "resume", resume) + monkeypatch.setattr("druks.chat.channels.whatsapp.services.resume", resume) response = await druks_client.delete(f"/api/chat/services/waha/sessions/{first.id}") identity = {"app": "helpdesk", "admin": first.identity["admin"]} @@ -941,6 +940,18 @@ async def test_removing_a_number_holds_its_chats_and_relinking_keeps_its_history assert first.identity["number"] == "+41000000000" +async def test_disconnecting_waha_removes_its_numbers_first(druks_db, druks_client, waha): + connection = await link(druks_db, await bot_account(druks_db)) + + response = await druks_client.delete("/api/services/waha") + + assert response.status_code == 204 + assert ("DELETE", "/api/sessions/session_one", None) in waha.calls + await druks_db.refresh(connection) + assert connection.revoked_reason == "service_disconnected" + assert not await Waha.is_connected() + + async def test_a_number_that_lost_its_link_takes_a_new_scan_and_keeps_its_chats( druks_db, druks_client, helpdesk, waha, monkeypatch ): diff --git a/docs/writing-an-app.md b/docs/writing-an-app.md index d020529e7..70802ba3c 100644 --- a/docs/writing-an-app.md +++ b/docs/writing-an-app.md @@ -1538,6 +1538,8 @@ It publishes `oauth.disconnected` after a user revokes a connection. A replacement of the service's client credentials also publishes this signal, and so does a token refresh that the provider answers with `invalid_grant`: Druks revokes that connection, because the grant is dead at the provider. +When the operator disconnects the service in Settings, Druks publishes the +signal for each of its connections. Revocation is a state, not a deletion: your subscriber can still read the connection it is told about. Subscribe in `subscribers.py`: diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 8e4c0b8fb..8900ce39f 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -322,6 +322,8 @@ export const api = { services: () => getJSON('/api/services'), connectService: (slug: string, fields: Record) => postJSON(`/api/services/${encodeURIComponent(slug)}`, fields), + disconnectService: (slug: string) => + deleteRequest(`/api/services/${encodeURIComponent(slug)}`), listConnections: () => getJSON('/api/oauth/connections'), disconnectConnection: (connectionId: string) => deleteRequest(`/api/oauth/connections/${encodeURIComponent(connectionId)}`), diff --git a/frontend/src/components/ServicesPane.test.tsx b/frontend/src/components/ServicesPane.test.tsx index db46da4a7..5a687c265 100644 --- a/frontend/src/components/ServicesPane.test.tsx +++ b/frontend/src/components/ServicesPane.test.tsx @@ -85,6 +85,9 @@ function stubFetch(states: Service[][]) { if (url === '/api/services/github' && init?.method === 'POST') { return new Response(JSON.stringify(connected), { status: 200 }) } + if (url === '/api/services/github' && init?.method === 'DELETE') { + return new Response(null, { status: 204 }) + } if (url === '/api/services') { const state = gets.length > 1 ? gets.shift() : gets[0] return new Response(JSON.stringify(state), { status: 200 }) @@ -123,6 +126,22 @@ async function flush() { } describe('ServicesPane', () => { + it('confirms service removal and refreshes its connection state', async () => { + const fetchMock = stubFetch([[connected], [disconnected]]) + const confirm = vi.fn().mockReturnValueOnce(false).mockReturnValueOnce(true) + vi.stubGlobal('confirm', confirm) + renderPane() + fireEvent.click(await screen.findByRole('button', { name: 'Configure GitHub' })) + + fireEvent.click(screen.getByRole('button', { name: 'Disconnect service' })) + expect(fetchMock.mock.calls.some(([, request]) => request?.method === 'DELETE')).toBe(false) + fireEvent.click(screen.getByRole('button', { name: 'Disconnect service' })) + + expect(await screen.findByText('Not connected')).toBeTruthy() + expect(fetchMock).toHaveBeenCalledWith('/api/services/github', expect.objectContaining({ method: 'DELETE' })) + expect(screen.getByRole('button', { name: 'Create GitHub App' })).toBeTruthy() + }) + it('shows compact rows with no credential fields on the overview', async () => { stubFetch([[disconnected, pasteOnly]]) renderPane() diff --git a/frontend/src/components/SettingsPanes.tsx b/frontend/src/components/SettingsPanes.tsx index cfdf713f6..58e20c9d4 100644 --- a/frontend/src/components/SettingsPanes.tsx +++ b/frontend/src/components/SettingsPanes.tsx @@ -902,6 +902,21 @@ function ServiceDetail({ service, onBack }: { service: Service; onBack: () => vo .finally(() => setBusy(false)) } + const disconnect = () => { + if (!window.confirm(`Disconnect ${service.title}? Druks removes its credentials and everything connected through it. Agents lose access.`)) return + setBusy(true) + setError(null) + void api + .disconnectService(service.slug) + .then(() => + queryClient.invalidateQueries({ + predicate: (query) => ['services', 'connections', 'appSettingChoices', 'appSettings', 'mcpServers'].includes(String(query.queryKey[0])), + }), + ) + .catch((e) => setError(e instanceof Error ? e.message : String(e))) + .finally(() => setBusy(false)) + } + const createGithubApp = ( + )} @@ -1070,6 +1088,7 @@ function missingIdentityCopy(connection: Connection): { label: string; hint: str const revokeReasonCopy: Record = { user: 'by you', client_replaced: 'client credentials replaced', + service_disconnected: 'service disconnected', server_removed: 'server removed', }