feat(messaging): let an install end (CHOO-2626)

core/switch_core/bridges/collaboration/install.py

@@ -205,6 +205,31 @@ async def redeem(self, *, code: str, redirect_uri: str) -> InstallGrant:
         grant, including a well-formed response the platform marked as failed.
         """
 
+    @abstractmethod
+    async def revoke(self, *, bot_token: str) -> None:
+        """Tell the platform the credential it granted us is finished with.
+
+        Called when an operator disconnects an install. Deleting our copy is
+        not the same act: the token stays valid at the platform, so a dump
+        taken before the disconnect would still hold a working key into a
+        customer's workspace. This is the half only the platform can do.
+
+        **A token the platform already considers dead is a success, not a
+        failure.** The common reason to be disconnecting at all is that the
+        customer removed the app on their side, and an implementation that
+        raised on "this token is already invalid" would make exactly that
+        install impossible to disconnect, forever.
+
+        Raise :class:`MessagingInstallError` for anything else — a refusal we
+        do not understand leaves a live credential behind and the operator
+        should hear about it rather than see a disconnect that reports
+        success.
+
+        Not the same thing as the platform's own uninstall. Revoking a token
+        does not remove the app from the workspace; it ends this deployment's
+        access with it.
+        """
+
     @abstractmethod
     def verify_webhook(self, *, headers: Mapping[str, str], body: bytes) -> None:
         """Prove an inbound event came from the platform, or raise.
@@ -245,6 +270,24 @@ def workspace_of_event(self, payload: Mapping[str, object]) -> str:
         event we cannot route is not an event we may guess at.
         """
 
+    @abstractmethod
+    def revocation_of_event(self, payload: Mapping[str, object]) -> str | None:
+        """Why this event says the install is over, or `None` if it does not.
+
+        Platforms report the end of an install as an ordinary event on the
+        ordinary endpoint, which makes it easy to treat as one: the app is
+        removed from a workspace, an event arrives saying so, nothing reads
+        it, and the deployment goes on holding a dead token, a running bridge
+        and a claim on a workspace whose owner believes they have left.
+
+        A reason rather than a flag because it is the thing worth logging — an
+        operator asking why their bridge stopped needs the platform's own
+        answer, and there is more than one way an install can end.
+
+        Runs on a payload that has been authenticated and not yet routed, so
+        like :meth:`workspace_of_event` it must not touch the database.
+        """
+
     @abstractmethod
     def connection_config(self, grant: InstallGrant) -> dict[str, object]:
         """Render a grant as the connection config this platform's adapter takes.

core/switch_core/bridges/collaboration/install_routes.py

@@ -42,6 +42,7 @@
 from switch_core.bridges.collaboration.install_service import (
     InstallPlatformMismatch,
     MessagingInstallService,
+    Revocation,
     WebhookBridgeUnavailable,
     WebhookTarget,
     WebhookWorkspaceUnknown,
@@ -175,6 +176,29 @@ async def _deliver(target: WebhookTarget, event: InboundWebhook) -> None:
                 target.tenant_id,
             )
 
+    async def _end_install(platform: str, revocation: Revocation) -> None:
+        """Act on the platform's news after it has been acknowledged.
+
+        Logged and not raised for the same reason as `_deliver`, and with more
+        at stake: the platform redelivers what it gets no answer to, and a
+        traceback out of a background task would leave a dead install claiming
+        a workspace with nothing in the log naming it.
+        """
+        try:
+            await service.revoked(
+                platform=platform,
+                workspace_id=revocation.workspace_id,
+                reason=revocation.reason,
+            )
+        except Exception:
+            logger.exception(
+                "Failed to end the install of %s workspace %s after the platform "
+                "reported it was over (%s)",
+                platform,
+                revocation.workspace_id,
+                revocation.reason,
+            )
+
     async def _inbound(
         platform: str,
         endpoint: WebhookEndpoint,
@@ -217,15 +241,28 @@ async def _inbound(
             return PlainTextResponse(event.handshake)
 
         try:
+            # Before resolving, because this is the one event that arrives as
+            # the bridge it would be resolved to is going away.
+            revocation = service.revocation(platform=platform, event=event)
+            if revocation is not None:
+                background.add_task(_end_install, platform, revocation)
+                return Response(status_code=200)
+
             target = await service.resolve(platform=platform, event=event)
         except WebhookPayloadError as failure:
             logger.error(
                 "A verified %s event named no workspace: %s", platform, failure
             )
             return Response(status_code=400)
         except WebhookWorkspaceUnknown as failure:
+            # A 200 for an event that reached nobody, which is the one place
+            # this file answers something other than what happened. The app
+            # left behind in a workspace whose install ended goes on posting,
+            # the platform cannot act on a 404, and it counts the refusals
+            # against the app as a whole — so the honest answer costs every
+            # other customer's delivery. The log is where it is visible.
             logger.warning("Dropped a %s event: %s", platform, failure)
-            return Response(status_code=404)
+            return Response(status_code=200)
         except WebhookBridgeUnavailable as failure:
             # Deliberately a 503: the platform retrying is the right behaviour
             # while a bridge restarts, and a 200 here would drop a real message

core/switch_core/bridges/collaboration/install_service.py

@@ -26,6 +26,27 @@
 it is an orphan nothing can revoke, whereas an install row with no bridge is a
 recorded credential waiting to be used, which is a state the schema already
 allows for.
+
+**Ending one runs the same steps backwards, and there are two ways in.** An
+operator disconnects here, or the platform tells us the app is gone. They
+differ in exactly one step — whether there is a live token to revoke — and in
+nothing else, because both have to leave the same state behind: no bridge, no
+stored credential, a released workspace and a row saying what happened. A
+deployment that handled only the first would go on holding a dead token and a
+claim on a workspace whose owner believes they have left.
+
+The revoking order is: tell the platform first, then destroy things. A
+revocation we do not understand then leaves the install exactly as it was and
+the operator can try again, where the other order would have removed the
+bridge and left a working key into a customer's workspace in whatever dump was
+taken next.
+
+Then the row, and the bridge last — the reverse of the building order and for
+the same reason read backwards. The install's pointer at the bridge is a real
+foreign key, so the row has to let go before the bridge can be deleted at all;
+and if the deletion then fails, what is left is a bridge with no credential
+that an operator can see and remove, rather than an install still claiming a
+workspace it has already been thrown out of.
 """
 
 from __future__ import annotations
@@ -53,10 +74,15 @@
 from switch_core.bridges.collaboration.lifecycle_service import (
     CollaborationBridgeLifecycleService,
 )
-from switch_core.crypto import encrypt_token
+from switch_core.crypto import decrypt_token, encrypt_token
 from switch_core.db.models import MessagingInstall
 from switch_core.db.session_scope import tenant_session
-from switch_core.db.stores.messaging_install_store import MessagingInstallStore
+from switch_core.db.stores.messaging_install_store import (
+    INSTALL_ACTIVE,
+    INSTALL_DISCONNECTED,
+    INSTALL_REVOKED,
+    MessagingInstallStore,
+)
 from switch_core.db.tenant_lookup import tenant_of_messaging_install
 from switch_core.tenant_context import no_tenant, tenant_scope
 
@@ -68,8 +94,10 @@ class WebhookWorkspaceUnknown(RuntimeError):
 
     Ordinary rather than alarming: an app left in a workspace whose install was
     removed goes on posting for as long as someone leaves it there. It is still
-    an error, because the alternative is answering "fine" to traffic that
-    reaches nobody.
+    an error here, because a resolver that returned nothing would make "nobody
+    holds this" and "here is where it goes" the same shape — but it is one the
+    route absorbs rather than reports, since the platform cannot fix it and
+    telling it repeatedly that its posts fail is held against the app itself.
     """
 
 
@@ -82,6 +110,14 @@ class WebhookBridgeUnavailable(RuntimeError):
     """
 
 
+@dataclass(frozen=True)
+class Revocation:
+    """A platform saying, in an ordinary event, that an install is over."""
+
+    workspace_id: str
+    reason: str
+
+
 @dataclass(frozen=True)
 class WebhookTarget:
     """Where one verified event goes: a tenant, a bridge, and its live adapter."""
@@ -229,6 +265,135 @@ async def complete(
             )
             return attached
 
+    async def list_installs(self, session: AsyncSession) -> list[MessagingInstall]:
+        """The bound tenant's installs, for the operator's own list.
+
+        On the caller's session like `begin`, and scoped by it: there is no
+        tenant argument because there is no tenant to choose.
+        """
+        return await self._store.list_for_tenant(session)
+
+    # ── Ending an install ────────────────────────────────────────────────────
+
+    async def disconnect(self, *, tenant_id: str, install_id: str) -> MessagingInstall:
+        """End an install because somebody here said to.
+
+        Idempotent: disconnecting an install that has already ended returns it
+        unchanged. An operator can reach this for a row the platform's own
+        `app_uninstalled` ended a second earlier, and that is not an error to
+        show them.
+
+        **Removing the bridge detaches every room that used it**, which become
+        internal-only. That is the honest consequence of disconnecting a
+        messaging app and it is not softened here — but it is the reason this
+        is an explicit action with a confirmation in front of it rather than
+        something inferred.
+        """
+        async with tenant_session(self._session_factory, tenant_id) as session:
+            install = await self._store.get(session, install_id=install_id)
+            platform = install.platform
+            workspace_id = install.external_workspace_id
+            bridge_id = install.bridge_id
+            token = install.encrypted_bot_token
+            already_ended = install.status != INSTALL_ACTIVE
+
+        if already_ended:
+            logger.info(
+                "Install %s of %s workspace %s had already ended; nothing to do",
+                install_id,
+                platform,
+                workspace_id,
+            )
+            async with tenant_session(self._session_factory, tenant_id) as session:
+                return await self._store.get(session, install_id=install_id)
+
+        with tenant_scope(tenant_id):
+            if token is not None:
+                await self._installers.get(platform).revoke(
+                    bot_token=decrypt_token(token, self._secret)
+                )
+
+            async with tenant_session(self._session_factory, tenant_id) as session:
+                ended = await self._store.end(
+                    session, install_id=install_id, status=INSTALL_DISCONNECTED
+                )
+                await session.commit()
+
+            if bridge_id is not None:
+                await self._lifecycle.remove(bridge_id)
+
+        logger.info(
+            "Disconnected %s workspace %s for tenant %s",
+            platform,
+            workspace_id,
+            tenant_id,
+        )
+        return ended
+
+    async def revoked(self, *, platform: str, workspace_id: str, reason: str) -> None:
+        """End an install because the platform said it is over.
+
+        Resolves the workspace itself rather than going through `resolve`,
+        which is built for delivering an event and insists on a running bridge.
+        Here a missing bridge is beside the point: the news is that the install
+        is finished, and a deployment that could only record that while the
+        bridge happened to be up would keep the dead ones it most needs to
+        clear.
+
+        A workspace that resolves to nobody is the ordinary case, not a fault.
+        The platform retries these events, so the second delivery arrives after
+        the first has already ended the install.
+
+        No revocation call: the token this would revoke is the one the platform
+        has just told us it killed.
+        """
+        tenant_id = await tenant_of_messaging_install(
+            self._session_factory, platform, workspace_id
+        )
+        if tenant_id is None:
+            logger.info(
+                "Ignoring end-of-install for %s workspace %s (%s): no tenant "
+                "holds it, so it has already ended",
+                platform,
+                workspace_id,
+                reason,
+            )
+            return
+
+        with tenant_scope(tenant_id):
+            async with tenant_session(self._session_factory, tenant_id) as session:
+                install = await self._store.get_for_workspace(
+                    session, platform=platform, external_workspace_id=workspace_id
+                )
+                if install is None:
+                    logger.warning(
+                        "End-of-install for %s workspace %s resolved to tenant "
+                        "%s and could not then be read as that tenant",
+                        platform,
+                        workspace_id,
+                        tenant_id,
+                    )
+                    return
+                install_id = install.id
+                bridge_id = install.bridge_id
+
+            async with tenant_session(self._session_factory, tenant_id) as session:
+                await self._store.end(
+                    session, install_id=install_id, status=INSTALL_REVOKED
+                )
+                await session.commit()
+
+            if bridge_id is not None:
+                await self._lifecycle.remove(bridge_id)
+
+        logger.warning(
+            "Ended the install of %s workspace %s for tenant %s: %s",
+            platform,
+            workspace_id,
+            tenant_id,
+            reason,
+        )
+
     # ── Inbound events ───────────────────────────────────────────────────────
     #
     # The other direction, and the one that runs constantly. Three steps, kept
@@ -263,6 +428,26 @@ def authenticate(
         installer.verify_webhook(headers=headers, body=body)
         return installer.parse_webhook(endpoint=endpoint, body=body)
 
+    def revocation(self, *, platform: str, event: InboundWebhook) -> Revocation | None:
+        """Whether this event is the platform ending the install, and for whom.
+
+        Asked before `resolve` and not after, because the two disagree about
+        what a missing bridge means. `resolve` insists on one, and the event
+        most worth reading here is precisely the one that arrives when the
+        bridge is on its way out — so an uninstall routed through the ordinary
+        path would be dropped exactly when it mattered.
+
+        Pure, like `authenticate`: it reads the payload and nothing else, so
+        the route can answer the platform before any of the work begins.
+        """
+        installer = self._installers.get(platform)
+        reason = installer.revocation_of_event(event.payload)
+        if reason is None:
+            return None
+        return Revocation(
+            workspace_id=installer.workspace_of_event(event.payload), reason=reason
+        )
+
     async def resolve(self, *, platform: str, event: InboundWebhook) -> WebhookTarget:
         """Turn a workspace id into the one bridge entitled to the event.