From 2e83f18bba4677cea3954e97b341f4e7c4b8b169 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Sun, 23 Aug 2026 16:07:49 +0200 Subject: [PATCH] feat(api): let a guild be set up from the console, by asking the bot to do it Every step of setting a guild up that matters needs a Discord token, and `api` must never hold one. So the console writes an intent -- what should be true -- and the bot's existing ten-second tick makes it true through the same `plan_setup` the slash command uses, then writes back what happened. - `POST`/`GET /api/guilds/{id}/setup`, administrator of that guild only. - `GET /api/invite`, built from the application id alone. - The newest unapplied intent wins; older ones settle as `superseded`. - An attempt settles an intent either way: one try, no retry loop. - `bot.has_arrived` separates "no voice channels" from "not there yet". --- charts/sturnus/values.yaml | 14 +- docs/first-deployment.md | 12 +- docs/operations.md | 45 ++ src/sturnus/application/setup_plan.py | 134 ++-- src/sturnus/console/adapters.py | 78 +++ src/sturnus/console/app.py | 14 + src/sturnus/console/ports.py | 55 ++ src/sturnus/console/routes_setup.py | 391 ++++++++++++ src/sturnus/domain/onboarding.py | 140 ++++- src/sturnus/entrypoints/api.py | 42 +- src/sturnus/entrypoints/bot.py | 7 + src/sturnus/infrastructure/discord/client.py | 59 ++ .../infrastructure/discord/setup_apply.py | 418 +++++++++++++ .../infrastructure/discord/setup_cog.py | 149 +---- src/sturnus/observability/events.py | 17 + tests/application/test_setup_plan.py | 96 ++- tests/console/conftest.py | 62 ++ tests/console/test_setup_directory.py | 189 ++++++ tests/console/test_setup_routes.py | 582 ++++++++++++++++++ tests/domain/test_onboarding.py | 204 ++++++ tests/infrastructure/discord/test_client.py | 144 ++++- .../discord/test_setup_apply.py | 545 ++++++++++++++++ 22 files changed, 3215 insertions(+), 182 deletions(-) create mode 100644 src/sturnus/console/routes_setup.py create mode 100644 src/sturnus/infrastructure/discord/setup_apply.py create mode 100644 tests/console/test_setup_directory.py create mode 100644 tests/console/test_setup_routes.py create mode 100644 tests/domain/test_onboarding.py create mode 100644 tests/infrastructure/discord/test_setup_apply.py diff --git a/charts/sturnus/values.yaml b/charts/sturnus/values.yaml index 75b2dc5..6e5c4e4 100644 --- a/charts/sturnus/values.yaml +++ b/charts/sturnus/values.yaml @@ -389,7 +389,19 @@ api: # on either replica -- which is the property that makes more than one # replica possible at all. replicaCount: 2 - env: {} + env: + # The Discord *application* id, and emphatically not the token: it is + # public by design -- it appears in every invite link ever clicked -- + # so it belongs here rather than in the Secret, and it grants this + # process nothing. It is used for exactly one thing: building the + # `bot`-scope authorize URL the console offers, which is the one + # onboarding step that can be done from the web at all. + # + # Blank is "no invite link configured": the endpoint answers + # `url: null` and the console says so, rather than the deployment + # failing to start. Anything that is not a snowflake does fail at + # startup -- see `ApiSettings._client_id_is_a_snowflake`. + STURNUS_DISCORD_CLIENT_ID: "" # Writes nothing but Python/library scratch to /tmp under # readOnlyRootFilesystem. Decrypted audio never touches disk: it goes from # the chunk buffer to the socket, which is the property the whole diff --git a/docs/first-deployment.md b/docs/first-deployment.md index b22ddcd..dce2654 100644 --- a/docs/first-deployment.md +++ b/docs/first-deployment.md @@ -73,9 +73,19 @@ There is no Sturnus bot application yet. At yet. `Speak` itself is not needed — the bot only listens. 5. Open the generated URL and invite the bot to the server. + The console can generate that URL for you once it is running, from the + application id alone: set `STURNUS_DISCORD_CLIENT_ID` (the **Application + ID** on the same page, not the token — it is public, and it goes in + `api.env`, never in the `Secret`) and `GET /api/invite` serves exactly + this link with exactly these scopes and this bitmask. Leaving it unset + costs nothing but that button; the endpoint answers `url: null` and the + console says so. + **Role position matters.** Discord will not let the bot edit a role positioned above its own. Drag the bot's role above the consent role in -**Server Settings → Roles**, or `/setup` fails with a permissions error. +**Server Settings → Roles**, or `/setup` fails with a permissions error — +and so does a setup requested from the console, which is the same planner +run from the other side (`docs/operations.md` section 6.2.14). ## 3. Collect the eight secret values diff --git a/docs/operations.md b/docs/operations.md index d00fb17..f96a036 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -2335,6 +2335,51 @@ pictures gone sooner than their recordings, shorten `audio_retention_days` — that moves the recording and the picture together, which is the only way this system moves them. +### 6.2.14 Setting a guild up from the console + +Every step of setting a guild up that matters needs a Discord token — +creating the consent role, denying `Speak` to `@everyone` and allowing it +for that role, registering the command tree — and `api` holds none. So the +console does not write `guild_config` for onboarding. It writes an +**intent**, the mirrors run backwards: `api` writes down what should be +true, and the bot's ordinary ten-second tick makes it true through the +same planner `/setup` uses and writes back what happened. + +| Route | What it does | +|---|---| +| `GET /api/invite` | The `bot`-scope authorize URL, built from `STURNUS_DISCORD_CLIENT_ID`. The one step that is genuinely web-doable. `url` is `null` when the variable is unset | +| `POST /api/guilds/{id}/setup` | Asks: `{"channel_ids": ["…"], "consent_role_name": "…"}`. Answers 202 with the guild's setup state | +| `GET /api/guilds/{id}/setup` | The same state. The console polls it until `request.status` stops being `pending` | + +Three things an operator should know about the behaviour. + +**A failure is terminal.** The tick runs six times a minute forever, so an +intent left unapplied after failing would retry a permission error against +Discord's rate limiter just as often. One attempt settles the intent; the +reason is in `request.error`, and an administrator who has fixed the +permission asks again, which is a new row saying who asked and when. There +is no back-off to tune. + +**The newest ask wins.** Two administrators submitting different requests +thirty seconds apart leave two rows; the bot applies the newer and settles +the older as `superseded` without acting on it. An intent states what +should be true, and two statements of what should be true do not compose — +applying both in order would finish on the older one, which is the +correction being overwritten by the mistake it corrected. + +**`pending` for more than a tick means the bot is not there.** A guild the +bot has not joined has no gateway object to iterate, so its intents are +never attempted. `bot.has_arrived` in the payload says so outright — it is +`false` exactly while nothing about the guild has been mirrored, which is +what separates "this server has no voice channels" from "nobody has looked +yet". A channel picker that could not tell those apart would send somebody +hunting for a bug that is not there. + +The permission the invite link asks for that fails latest is **Manage +Roles**, and no bitmask covers the other half of it: the bot's own role +must sit above the consent role in Server Settings → Roles, or Discord +refuses the edit. See `docs/first-deployment.md` section 2. + ### 6.3 Listening to a recording by hand Every automated check this system has can describe a track — its level, diff --git a/src/sturnus/application/setup_plan.py b/src/sturnus/application/setup_plan.py index 6fd6501..8939eeb 100644 --- a/src/sturnus/application/setup_plan.py +++ b/src/sturnus/application/setup_plan.py @@ -1,4 +1,4 @@ -"""The decisions behind `/setup`, separated from Discord (Spec 10.1). +"""The decisions behind setting a guild up, separated from Discord (Spec 10.1). `/setup` exists so the six required configuration keys can be set from typed command parameters -- Discord renders native pickers for a channel @@ -9,19 +9,30 @@ to prose in an operations guide would put the one step nobody may get wrong into the hands of whoever reads the guide least carefully. -A guild names a *list* of channels Sturnus may record in, so `/setup` -**adds** the channel it was given to that list rather than replacing it. +A guild names a *list* of channels Sturnus may record in, so setup +**adds** the channels it was given to that list rather than replacing it. Setting up a second meeting room used to silently un-configure the first, which is the kind of failure nobody notices until the meeting that was supposed to be recorded was not. The permissions are then planned for -*every* allowed channel, not only the one just named: a channel that is +*every* allowed channel, not only the ones just named: a channel that is allowed but whose `@everyone` may still Speak is a hole in the consent protection whether or not this call is the one that added it. +**One planner, two callers.** `/setup` is one of them; +`sturnus.infrastructure.discord.setup_apply`, which applies what the +console asked for through `guild_setup_intent`, is the other. A second +implementation of the consent protection is the last thing this system +should grow -- one that got either overwrite backwards would let somebody +be recorded without having consented -- so the two callers differ only in +where their arguments come from. That is what `added_channel_ids` being a +tuple and the policy pair being optional are for: the slash command names +one room and is always told the policy, the console names a list and is +never told it. + This module holds only the comparison between the desired state and what is already configured -- no Discord object reaches it, only the ids and permission facts already read off them. That is what makes it testable -without a guild, and what makes the command safe to run twice: re-running +without a guild, and what makes setup safe to run twice: re-running `plan_setup` against a correctly configured guild returns an empty plan, not a rewrite. """ @@ -34,12 +45,6 @@ from sturnus.domain import settings -#: Required keys always supplied through `/setup`'s own parameters -- either -#: written this call or already correct, so they are never reported missing. -_ALWAYS_SUPPLIED: frozenset[str] = frozenset( - {settings.VOICE_CHANNEL_IDS, settings.POLICY_URL, settings.POLICY_VERSION} -) - @dataclass(frozen=True) class ChannelPermissions: @@ -110,66 +115,91 @@ class SetupPlan: def plan_setup( current: dict[str, str | None], - channel_id: int, + added_channel_ids: tuple[int, ...], stored_channel_ids: tuple[int, ...], channel_permissions: Sequence[ChannelPermissions], role_id: int | None, stored_role_valid: bool, - policy_url: str, - policy_version: str, + policy_url: str | None, + policy_version: str | None, ) -> SetupPlan: - """Computes what `/setup` still needs to do. + """Computes what setting this guild up still needs to do. `current` maps configuration keys to their stored value; a key absent from `current` means the caller has no information about it, and it is left out of `missing` rather than assumed unset -- `missing` reports only what the caller already knows (an explicit `None`) is still open. - A real caller (`SetupCog`) reads every required key before building - `current`, so in practice every gap does surface; this function itself - stays conservative about keys it was never told about. + Both real callers read every required key before building `current`, + so in practice every gap does surface; this function itself stays + conservative about keys it was never told about. `document_target` cannot be derived from anything Discord exposes -- it names an Outline collection -- so it is never guessed here. If the caller reports it unset, it is reported in `missing`, exactly like a - guild that has never run `/setup` at all. + guild that has never been set up at all. `stored_channel_ids` is what the guild already allows, read through `settings.recording_channel_ids` so the key this replaced still counts. - `channel_id` is **added** to it, never substituted for it: running - `/setup` for a second meeting room must not stop the first one being + `added_channel_ids` is **added** to it, never substituted for it: + setting up a second meeting room must not stop the first one being recorded. Removing one is `/config set voice_channel_ids`, which is why the resulting list is reported back rather than merely written. + A tuple rather than one id because the two callers ask differently: + `/setup` names one room, since Discord renders one channel picker per + parameter, and the console names the whole list a person ticked. Empty + is a legitimate ask from the console -- "leave the channels as they + are and fix the rest" -- and leaves the stored list untouched. + `channel_permissions` describes the Speak overwrites of every allowed - channel the caller could resolve, the newly added one included. Every - allowed channel is planned for, not only the one this call named: a + channel the caller could resolve, the newly added ones included. Every + allowed channel is planned for, not only the ones this call named: a channel Sturnus may record in whose `@everyone` can still Speak is a hole in the consent protection regardless of which call added it. - `role_id` is the consent role explicitly supplied to `/setup`'s - `consent_role` parameter this call, or `None` if that argument was - omitted -- omitting it must never itself be destructive (Spec 10.1), - so it never means "create a new role" the way it once did. What - happens when it is omitted is governed by `stored_role_valid`: `True` - means `current[settings.CONSENT_ROLE_ID]` names a role the caller has + `role_id` is a consent role the caller resolved for this call -- the + slash command's `consent_role` parameter, or the role a console intent + named by name and that turned out to exist -- or `None` if there was + none. Omitting it must never itself be destructive (Spec 10.1), so it + never means "create a new role" the way it once did. What happens when + it is omitted is governed by `stored_role_valid`: `True` means + `current[settings.CONSENT_ROLE_ID]` names a role the caller has confirmed still exists in the guild, so it is kept as-is; `False` means there is nothing usable to keep, and a new role is requested. Discord role existence cannot be checked from here -- there is no guild object in this module -- so the caller resolves that before calling in. + + `policy_url` and `policy_version` are what this call was *told* the + policy is, and `None` means it was told nothing. `/setup` always has + both, because they are required command parameters an administrator + typed. A console intent has neither: the console sets them on the + settings page, where they are ordinary keys, and an onboarding request + that carried them would be a second place to write the two values that + decide whose consent is still valid. `None` therefore writes nothing + and leaves the key to be reported in `missing` if the guild has not + set it -- which is the honest answer for a guild that cannot record + yet. """ writes: dict[str, str] = {} - def _maybe_write(key: str, desired: str) -> None: - if current.get(key) != desired: + def _maybe_write(key: str, desired: str | None) -> None: + # `None` is "this caller was not told", never "clear it": nothing + # in setup removes a value, and a caller with no opinion about a + # key must leave what is stored alone. + if desired is not None and current.get(key) != desired: writes[key] = desired - channel_ids = tuple(sorted({*stored_channel_ids, channel_id})) - # The plural key, always -- `/setup` is one of the two commands that - # moves a guild off the singular one. The old row is left alone rather - # than deleted: it is read only when the plural key is unset, so from - # this write on it is inert, and removing it would be a second write - # nobody asked for. - _maybe_write(settings.VOICE_CHANNEL_IDS, settings.render_channel_ids(channel_ids)) + channel_ids = tuple(sorted({*stored_channel_ids, *added_channel_ids})) + # The plural key, always -- setup is one of the two things that moves a + # guild off the singular one. The old row is left alone rather than + # deleted: it is read only when the plural key is unset, so from this + # write on it is inert, and removing it would be a second write nobody + # asked for. Skipped entirely for a guild that allows nothing and was + # given nothing, because `render_channel_ids(())` is the empty string + # and `ConfigStore.set` refuses it -- rightly, since "allowed to record + # nowhere" is what `/config clear` is for. + if channel_ids: + _maybe_write(settings.VOICE_CHANNEL_IDS, settings.render_channel_ids(channel_ids)) _maybe_write(settings.POLICY_URL, policy_url) _maybe_write(settings.POLICY_VERSION, policy_version) @@ -197,16 +227,28 @@ def _maybe_write(key: str, desired: str) -> None: PermissionChange(channel.channel_id, "consent_role", allow_speak=True) ) + # Required keys this call supplied itself: either written above or + # already correct, so neither is still open. Derived from what the + # caller was actually told rather than tabulated, because the two + # callers are told different things -- a console intent carries no + # policy, and a guild that is allowed to record nowhere and was named + # no channel genuinely is missing `voice_channel_ids`. + supplied = { + # Either already correct (`role_id` given, nothing to write), or a + # new role is about to be created and will get an id once it + # exists -- setup does not fabricate one in advance. + settings.CONSENT_ROLE_ID, + } + if channel_ids: + supplied.add(settings.VOICE_CHANNEL_IDS) + if policy_url is not None: + supplied.add(settings.POLICY_URL) + if policy_version is not None: + supplied.add(settings.POLICY_VERSION) + missing: list[str] = [] for key in sorted(settings.REQUIRED_KEYS): - if key in writes: - continue - if key == settings.CONSENT_ROLE_ID: - # Either already correct (role_id given, nothing to write), or a - # new role is about to be created and will get an id once it - # exists -- setup does not fabricate one in advance. - continue - if key in _ALWAYS_SUPPLIED: + if key in writes or key in supplied: continue if key in current and current[key] is None: missing.append(key) diff --git a/src/sturnus/console/adapters.py b/src/sturnus/console/adapters.py index 008147f..8852505 100644 --- a/src/sturnus/console/adapters.py +++ b/src/sturnus/console/adapters.py @@ -54,6 +54,7 @@ GuildDirectory, GuildQueue, GuildRecording, + GuildSetupState, GuildSignIn, OAuthClient, OwnConsent, @@ -118,6 +119,7 @@ load_session, load_status, ) +from sturnus.infrastructure.db.setup_intents import SetupIntentStore from sturnus.infrastructure.documents.outline_oauth import OutlineOAuth from sturnus.observability.events import Event, log_exception @@ -2344,6 +2346,82 @@ async def for_guild(self, guild_id: int, *, requested_by: int) -> GuildDirectory ) +class ConsoleGuildSetup: + """Asking the bot to set a guild up, and reading back what it did. + + The only write in this module that is not a write about the console + itself, and it is deliberately not a write to `guild_config`. `api` + holds no Discord token (Spec 13.2), so it cannot create the consent + role or set the Speak overwrites; a `guild_config` row saying a guild + allows a channel whose `@everyone` may still Speak would be a guild + that looks configured and is not. So this writes an *intent*, and the + bot's ten-second tick does all of it or none of it. + + **No contradiction rule lives here, and that is the design.** Two + administrators asking thirty seconds apart both get their row: an + administrator asking twice asked twice, and collapsing the two would + lose who asked for which and when. Which one the guild ends up + configured from is settled where the guild is configured -- the bot + applies the newest and settles the rest as superseded + (`sturnus.domain.onboarding.select_intent`). Refusing the second + write here would have needed a lock this process cannot hold across + two replicas, and would have left a guild whose bot has not arrived + unable to correct a typo until it did. + + `seen_at` comes from the guild mirror rather than from the intent, + because it answers a different question: whether the bot has ever + looked at this server. `None` there is the difference between "this + guild has no channels" and "the bot is not there yet", and an empty + channel picker means the first only when this says the bot has been. + """ + + def __init__( + self, + session_factory: async_sessionmaker[AsyncSession], + admins: AdminDirectory, + intents: SetupIntentStore, + ) -> None: + self._session_factory = session_factory + self._admins = admins + self._intents = intents + + async def state(self, guild_id: int, *, requested_by: int) -> GuildSetupState | None: + if not await self._admins.is_admin(guild_id, requested_by): + return None + return await self._state(guild_id) + + async def request( + self, + guild_id: int, + *, + requested_by: int, + channel_ids: str, + consent_role_name: str | None, + now: datetime, + ) -> GuildSetupState | None: + if not await self._admins.is_admin(guild_id, requested_by): + return None + await self._intents.request( + guild_id, + requested_by=requested_by, + channel_ids=channel_ids, + consent_role_name=consent_role_name, + now=now, + ) + # Read back rather than assembled from what was just written. If a + # second administrator wrote a newer intent between the two + # statements, the newer one is the guild's answer and the one this + # response has to show -- inventing a payload from the row this + # call wrote would tell somebody their request is what the bot + # will apply when it is not. + return await self._state(guild_id) + + async def _state(self, guild_id: int) -> GuildSetupState: + async with self._session_factory() as db: + seen_at = await db.scalar(select(Guild.synced_at).where(Guild.guild_id == guild_id)) + return GuildSetupState(seen_at=seen_at, intent=await self._intents.latest_for(guild_id)) + + class ConsoleCollectionNames: """Outline's mirrored collections, for anybody who administers a guild. diff --git a/src/sturnus/console/app.py b/src/sturnus/console/app.py index c671d20..787e782 100644 --- a/src/sturnus/console/app.py +++ b/src/sturnus/console/app.py @@ -50,6 +50,7 @@ GuildNames, GuildOAuthClients, GuildReports, + GuildSetup, LinkDirectory, PersonalConsents, PreferenceDirectory, @@ -81,6 +82,8 @@ from sturnus.console.routes_queue import register as register_queue from sturnus.console.routes_report import GUILD_REPORTS from sturnus.console.routes_report import register as register_report +from sturnus.console.routes_setup import DISCORD_CLIENT_ID, GUILD_SETUP +from sturnus.console.routes_setup import register as register_setup from sturnus.console.session import ( ExpiredSession, InvalidSession, @@ -325,6 +328,14 @@ def build_api( documents: SessionDocumentDirectory, artefacts: DocumentArtefacts, oauth_clients: GuildOAuthClients, + setup: GuildSetup, + #: This deployment's Discord application id, or `None` where the + #: operator has not configured one -- the invite link is then served + #: as `null` rather than as a refusal. Not a credential: an + #: application id is public by design and appears in every invite + #: link ever clicked, which is exactly why the one genuinely + #: web-doable onboarding step can be done from here at all. + discord_client_id: str | None = None, ) -> web.Application: """Builds the application, with every collaborator injected. @@ -365,6 +376,8 @@ def build_api( app[SESSION_DOCUMENTS] = documents app[DOCUMENT_ARTEFACTS] = artefacts app[routes_oauth.GUILD_OAUTH_CLIENTS] = oauth_clients + app[GUILD_SETUP] = setup + app[DISCORD_CLIENT_ID] = discord_client_id app.add_routes( [ web.get("/healthz", healthz), @@ -382,6 +395,7 @@ def build_api( register_report(app) register_me(app) register_directory(app) + register_setup(app) routes_settings.register(app) routes_tags.register(app) routes_recording.register(app) diff --git a/src/sturnus/console/ports.py b/src/sturnus/console/ports.py index e4f1e50..9fc7e34 100644 --- a/src/sturnus/console/ports.py +++ b/src/sturnus/console/ports.py @@ -38,6 +38,7 @@ ) from sturnus.domain.exports import ExportTarget, SessionDocument from sturnus.domain.oauth_clients import GuildOAuthClient +from sturnus.domain.onboarding import SetupIntent from sturnus.infrastructure.documents.outline_oauth import ExternalIdentity @@ -262,6 +263,60 @@ class AdministeredGuild: icon_url: str | None +@dataclass(frozen=True) +class GuildSetupState: + """What the console needs to draw one guild's onboarding page. + + `seen_at` is when the bot last mirrored anything about this guild, and + `None` means it never has. That is the field the interface has to have. + A guild the bot has not joined yet has **empty mirrors** -- no + channels, no roles -- and an empty channel picker rendered for that + reason reads exactly like a server that genuinely has no voice + channels. One sends somebody to wait for the bot to arrive; the other + sends them hunting for a bug that is not there. The payload separates + them so the console never has to guess which it is looking at. + + `intent` is the most recent thing anybody asked for, settled or not, + or `None` if nobody ever has. The most recent and not a list, because + that is the guild's current answer: an older unapplied request is + superseded rather than queued (`sturnus.domain.onboarding`), so what + an administrator is waiting on is always the last one asked. + """ + + seen_at: datetime | None + intent: SetupIntent | None + + +class GuildSetup(Protocol): + """Asking the bot to configure a guild, and reading what came of it. + + `api` holds no Discord token and never will (Spec 13.2), so neither + method here touches Discord. `request` writes down what should be + true; the bot's ten-second tick makes it true and writes back what + happened, which `state` reads. + + `requested_by` is not optional and neither method exists without it, + for the reason `GuildNames` and `ConsentDirectory` have none: the + authorisation rule lives inside the call rather than in a handler that + could forget to apply it. `None` covers "no such guild" and "you do + not administer it" alike -- writing a setup request is an act on + somebody else's server, and a 403 would confirm to somebody just + established as having no business with that guild that it exists. + """ + + async def state(self, guild_id: int, *, requested_by: int) -> GuildSetupState | None: ... + + async def request( + self, + guild_id: int, + *, + requested_by: int, + channel_ids: str, + consent_role_name: str | None, + now: datetime, + ) -> GuildSetupState | None: ... + + class GuildNames(Protocol): """A guild's mirrored names, if the person asking administers it. diff --git a/src/sturnus/console/routes_setup.py b/src/sturnus/console/routes_setup.py new file mode 100644 index 0000000..00372ef --- /dev/null +++ b/src/sturnus/console/routes_setup.py @@ -0,0 +1,391 @@ +"""Setting a guild up from the console, by asking the bot to do it. + +- `POST /api/guilds/{guild_id}/setup` -- ask +- `GET /api/guilds/{guild_id}/setup` -- what came of it +- `GET /api/invite` -- the link that puts the bot in a server + +**Why an ask rather than a write.** Every step of setting a guild up that +matters needs a Discord token: creating the consent role, denying `Speak` +to `@everyone` and allowing it for that role, registering the command +tree. This process holds no Discord token and must not be given one +(Spec 13.2) -- it already holds S3 and the master key, so it can decrypt +every recording ever made, and that is not a process to also hand the +ability to act as the bot. So it writes down what should be true, and the +bot's existing ten-second reconcile tick makes it true and writes back +what happened. The mirrors run backwards. + +That is also why this endpoint does not write `guild_config` directly even +though it could. A guild whose `voice_channel_ids` names a room where +`@everyone` may still speak looks configured and is not; the two halves +are one act, and only the bot can perform either half of it. + +**Poll, do not wait.** `POST` answers immediately with the same payload +`GET` answers, because there is nothing to wait for: the request is a row, +and the bot reaches it within a tick. The console polls `GET` until +`request.status` stops being `pending`. `pending` for more than a few +seconds means the bot is not in this guild yet, which `bot.seen_at` +already says outright. + +**404, never 403.** A guild this person does not administer answers +exactly as a guild that does not exist. Writing a setup request is an act +on somebody else's server, and a 403 would confirm to somebody just +established as having no business with that guild that it exists. + +**Nothing a caller typed is reflected back in a refusal.** Every reason +below is a fixed string, the same rule the rest of `sturnus.console` +follows. The one thing that is echoed is the stored intent -- which the +same person's own guild wrote, and which the console has to render. +""" + +from __future__ import annotations + +import json +import logging + +from aiohttp import web + +from sturnus.console.ports import GuildSetup, GuildSetupState +from sturnus.domain import settings +from sturnus.domain.onboarding import ( + INVITE_PERMISSIONS, + INVITE_SCOPES, + SetupIntent, + invite_url, +) +from sturnus.observability.events import Event, log_event + +log = logging.getLogger(__name__) + +#: Where the collaborators are found. Their own keys rather than +#: parameters to `register`, so `build_api` stays a two-line edit -- +#: several agents are adding sections to that function and each extra +#: line is a merge by hand. +GUILD_SETUP: web.AppKey[GuildSetup] = web.AppKey("guild_setup") + +#: This deployment's Discord application id, or `None` if the operator has +#: not configured one. Not a credential: an application id is public by +#: design and appears in every invite link ever clicked. +DISCORD_CLIENT_ID: web.AppKey[str | None] = web.AppKey("discord_client_id") + +_SETUP_PATH = "/api/guilds/{guild_id}/setup" +_INVITE_PATH = "/api/invite" + +#: Discord's own limit on a role name. Checked here rather than left to +#: the gateway because the alternative is a request that is accepted, sits +#: pending for a tick, and comes back failed for a reason the person could +#: have been told while they were still typing. +_MAX_ROLE_NAME = 100 + +#: The one refusal covering every reason there is to refuse a guild. See +#: the module docstring on why "there is none" and "not yours" are one +#: answer. +_NO_SUCH_GUILD = "no such guild" + +_MALFORMED_BODY = "malformed request body" +_CHANNELS_MUST_BE_STRINGS = "channel_ids must be a list of snowflake strings" +_CHANNELS_REFUSED = "the channel list is not valid" +_ROLE_NAME_MUST_BE_A_STRING = "consent_role_name must be a string" +_ROLE_NAME_REFUSED = "the consent role name is not valid" + +#: What a setup request has asked for is a guild's configuration and who +#: asked for it. Nothing between this and the browser has any business +#: keeping a copy. +_PRIVATE = {"Cache-Control": "private, no-store"} + +#: `applied_at` is null and no outcome has been written: the bot has not +#: reached this request yet. Not one of `sturnus.domain.onboarding`'s +#: outcomes, because it is the absence of one -- the row is not settled. +PENDING = "pending" + + +def register(app: web.Application) -> None: + """Adds the onboarding routes to an application that already has a session. + + `require_session` is applied here rather than as a decorator on each + handler, and to all three without exception -- including the invite + link, whose URL is public. Public is not the same as unauthenticated: + an endpoint of this API that answered without a session would be the + only one, and "visibly public" is a property worth keeping for the + routes that genuinely are. + """ + from sturnus.console.app import require_session + + app.add_routes( + [ + web.get(_SETUP_PATH, require_session(read_setup)), + web.post(_SETUP_PATH, require_session(request_setup)), + web.get(_INVITE_PATH, require_session(bot_invite)), + ] + ) + + +# --------------------------------------------------------------------------- +# Handlers +# --------------------------------------------------------------------------- + + +async def read_setup(request: web.Request) -> web.Response: + """Where this guild's setup has got to, and whether the bot is even there.""" + guild_id = _guild_id(request) + state = await request.app[GUILD_SETUP].state(guild_id, requested_by=_caller(request)) + if state is None: + raise _refusal(web.HTTPNotFound, _NO_SUCH_GUILD) + return _state_response(guild_id, state) + + +async def request_setup(request: web.Request) -> web.Response: + """Asks the bot to configure this guild, and answers with the guild's state. + + The same payload `GET` answers, deliberately. Under the rule that the + newest ask wins, "what did I just ask for" and "what will this guild + be configured from" are the same question -- and if somebody else + asked in between, the honest answer to both is theirs. + """ + from sturnus.console.app import _NOW + + guild_id = _guild_id(request) + discord_user_id = _caller(request) + channel_ids, consent_role_name = await _requested_setup(request) + + state = await request.app[GUILD_SETUP].request( + guild_id, + requested_by=discord_user_id, + channel_ids=channel_ids, + consent_role_name=consent_role_name, + now=request.app[_NOW](), + ) + if state is None: + raise _refusal(web.HTTPNotFound, _NO_SUCH_GUILD) + + # The only record that a person asked, as opposed to that the bot + # acted: the two are separated by a tick, and by however long a guild + # takes to invite the bot. Names who and which guild, never the + # channel list -- ids are already visible to anybody in the server, + # but the role name is free text somebody typed. + log_event( + log, + logging.INFO, + Event.CONSOLE_SETUP_REQUESTED, + "The console asked the bot to set a guild up", + guild_id=guild_id, + discord_user_id=discord_user_id, + ) + # 202, not 201: nothing exists yet that this URL now addresses. The + # bot has been asked, and the body says how to find out whether it + # did anything. + return _state_response(guild_id, state, status=202) + + +async def bot_invite(request: web.Request) -> web.Response: + """The link that puts this deployment's bot into a server. + + The one onboarding step that is genuinely web-doable, and the step + every other one waits on: until the bot is in the guild it mirrors + nothing, so nobody administers it as far as this API is concerned and + every other route here answers 404 for it. + + Answers with `url: null` rather than a refusal when no application id + is configured. A console that could not tell "this deployment has not + been given its client id" from "an API that does not serve this" would + have to guess, and both guesses are wrong somewhere. + """ + client_id = request.app[DISCORD_CLIENT_ID] + return web.json_response( + { + "client_id": client_id, + "url": None if client_id is None else invite_url(client_id), + # Sent even when there is no link, because they are what the + # page tells somebody to tick if they build the link by hand + # in Discord's own URL generator instead. + "permissions": INVITE_PERMISSIONS, + "scopes": list(INVITE_SCOPES), + }, + headers=_PRIVATE, + ) + + +# --------------------------------------------------------------------------- +# The guards +# --------------------------------------------------------------------------- + + +def _guild_id(request: web.Request) -> int: + """The guild in the path, or 404. + + A path segment that is not a number names no guild, which is the same + answer as naming one that does not exist -- and the same answer as + naming one this person does not administer. + """ + try: + return int(request.match_info["guild_id"]) + except ValueError: + raise _refusal(web.HTTPNotFound, _NO_SUCH_GUILD) from None + + +async def _requested_setup(request: web.Request) -> tuple[str, str | None]: + """The body, as the stored spelling of a channel list and a role name. + + Rendered into `guild_config`'s own format here rather than kept as a + list, because that is what an intent stores: applying it is then a + write of the value rather than a second serialisation nobody would + keep in step with the first. + + Every rule about what a channel list may contain is + `settings.parse_channel_ids`', reached by round-tripping what was + rendered. A duplicate, an empty list and a non-integer are all refused + there, with reasons argued there -- and a second copy of those rules + here is how the two drift apart, with the copy nobody exercises going + stale first. + """ + try: + body = await request.json() + except ValueError: + raise _refusal(web.HTTPBadRequest, _MALFORMED_BODY) from None + if not isinstance(body, dict): + raise _refusal(web.HTTPBadRequest, _MALFORMED_BODY) + + channels = body.get("channel_ids") + if not isinstance(channels, list) or not all(isinstance(each, str) for each in channels): + raise _refusal(web.HTTPBadRequest, _CHANNELS_MUST_BE_STRINGS) + # Snowflakes as strings, always: one exceeds JavaScript's safe integer + # range, where a JSON number silently loses its last digits and + # produces an id that looks right and names nothing. A client that + # sent numbers would be a client that has already lost them. + rendered = ",".join(channels) + try: + settings.parse_channel_ids(rendered) + except settings.InvalidChannelList: + raise _refusal(web.HTTPBadRequest, _CHANNELS_REFUSED) from None + + return rendered, _requested_role_name(body) + + +def _requested_role_name(body: dict[str, object]) -> str | None: + """The consent role's name, if the request named one. + + Absent and `null` both mean "do not name one", which leaves whatever + role the guild already has -- omitting it must never be the + destructive path (Spec 10.1). A blank string is refused rather than + treated as absent: a role called nothing is not what anybody meant, + and Discord would refuse it a tick later anyway. + """ + name = body.get("consent_role_name") + if name is None: + return None + if not isinstance(name, str): + raise _refusal(web.HTTPBadRequest, _ROLE_NAME_MUST_BE_A_STRING) + trimmed = name.strip() + if not trimmed or len(trimmed) > _MAX_ROLE_NAME: + raise _refusal(web.HTTPBadRequest, _ROLE_NAME_REFUSED) + return trimmed + + +# --------------------------------------------------------------------------- +# Writing the response +# --------------------------------------------------------------------------- + + +def _state_response(guild_id: int, state: GuildSetupState, *, status: int = 200) -> web.Response: + return web.json_response( + { + # Every Discord id as a string: a snowflake exceeds + # JavaScript's safe integer range. + "guild_id": str(guild_id), + "bot": { + # The field the channel picker depends on. An empty + # channel list means "this server has no voice channels" + # only when this is true; while it is false it means + # nobody has looked yet. + "has_arrived": state.seen_at is not None, + # Present and null rather than absent, so a client never + # has to tell "the bot has never swept this guild" from + # "an API that does not send this". + "seen_at": None if state.seen_at is None else state.seen_at.isoformat(), + }, + "request": None if state.intent is None else _intent_json(state.intent), + }, + status=status, + headers=_PRIVATE, + ) + + +def _intent_json(intent: SetupIntent) -> dict[str, object]: + return { + # A row id rather than a snowflake, and a string anyway: every id + # this API sends is one, and a client that had to remember which + # kind each field was would eventually get one wrong. + "id": str(intent.id), + "status": _status_of(intent), + "requested_by": str(intent.requested_by), + "requested_at": intent.requested_at.isoformat(), + # The list as the console offered it, back in the spelling it sent + # -- so a page can tick the same boxes again without parsing a + # stored format. Empty only for a row written by hand: the API + # refuses a request that names no channel. + "channel_ids": _channel_ids_json(intent.channel_ids), + "consent_role_name": intent.consent_role_name, + # When the bot finished with it, however it finished. Null while + # the status is `pending`, and the two are the same fact said + # twice on purpose: a client keys off the status, and a person + # reading the payload gets a time. + "settled_at": None if intent.applied_at is None else intent.applied_at.isoformat(), + # Null unless the status is `failed`. Free text the bot composed + # for a person to act on -- which channel, which permission, what + # to do about it -- so it is rendered rather than keyed off. + "error": intent.error, + } + + +def _status_of(intent: SetupIntent) -> str: + """`pending`, or whatever the bot wrote when it settled the row. + + Not narrowed to the outcomes this build knows. `outcome` is text + rather than a database enum precisely so a value this code has never + seen is a row a reader can ignore instead of a write that fails inside + a reconcile tick, and an endpoint that refused to render one would + give that property back. + """ + if intent.is_pending or intent.outcome is None: + return PENDING + return intent.outcome + + +def _channel_ids_json(stored: str | None) -> list[str]: + """The stored list, as the strings it was sent as. + + Unparseable is rendered as empty rather than raising. Only a + hand-written row can be unparseable -- this API round-trips every list + through `parse_channel_ids` before storing it -- and a page that 500s + is a worse answer than one that shows a failed request whose `error` + already says the list could not be read. + """ + if stored is None: + return [] + try: + return [str(channel_id) for channel_id in settings.parse_channel_ids(stored)] + except settings.InvalidChannelList: + return [] + + +def _refusal(exception: type[web.HTTPException], reason: str) -> web.HTTPException: + """A refusal with a JSON body, built rather than returned as a response. + + aiohttp deprecated returning an `HTTPException` from a handler, and + raising lets the guards above read as a straight line instead of + threading an optional response back through every caller. + """ + return exception(text=json.dumps({"error": reason}), content_type="application/json") + + +def _caller(request: web.Request) -> int: + """The Discord id of the person making this request. + + Only ever reached from behind `require_session`, which is what + guarantees there is one -- `current_user` raises rather than returning + `None` if that is ever untrue, so a route registered without the + wrapper fails loudly instead of quietly acting on somebody else's + server. + """ + from sturnus.console.app import current_user + + return current_user(request).discord_user_id diff --git a/src/sturnus/domain/onboarding.py b/src/sturnus/domain/onboarding.py index 362ea6a..2b06d35 100644 --- a/src/sturnus/domain/onboarding.py +++ b/src/sturnus/domain/onboarding.py @@ -22,12 +22,31 @@ Both outcomes are terminal. An administrator who has fixed the permission asks again, which is a new intent, which is a row that says who asked and when. + +**The newest ask wins, and it wins outright.** Two administrators +submitting different intents thirty seconds apart, or one impatient +person pressing twice, leave a guild with two unapplied rows. They are +not a queue of two jobs: an intent states what should be *true*, and two +statements of what should be true do not compose -- applying both in +request order would finish on the older list, which is the correction +being overwritten by the mistake it corrected. So `select_intent` applies +the newest and settles every older one as `SUPERSEDED`, unapplied. + +The rejected alternative was refusing a request while one is pending. +That reads tidier and is worse in both directions: an administrator who +mistyped a channel would have to wait out a tick before they could +correct it, and their correction would then be a second full setup +applied on top of the first; and an intent that never settles -- because +the bot has not joined the guild yet, which is the ordinary state during +onboarding -- would lock that guild out of being set up at all. """ from __future__ import annotations +from collections.abc import Sequence from dataclasses import dataclass from datetime import datetime +from urllib.parse import urlencode #: The bot did what was asked. APPLIED = "applied" @@ -36,11 +55,18 @@ #: Terminal, not a retry: see the module docstring. FAILED = "failed" +#: The bot never tried: a newer intent for the same guild replaced this +#: one before the tick reached either. Terminal like the other two, and +#: written by the bot rather than by `api` -- the row stays exactly as it +#: was asked for, so who asked for what and when survives being overtaken. +#: See `select_intent` for why the newer one wins outright. +SUPERSEDED = "superseded" + #: What may be written into `guild_setup_intent.outcome`. Plain strings #: rather than a database enum for the reason `guild_channel.kind` is #: one: a value this code has never seen must be a row a reader can #: ignore rather than a write that fails inside a reconcile tick. -OUTCOMES = frozenset({APPLIED, FAILED}) +OUTCOMES = frozenset({APPLIED, FAILED, SUPERSEDED}) @dataclass(frozen=True, slots=True) @@ -76,3 +102,115 @@ class SetupIntent: @property def is_pending(self) -> bool: return self.applied_at is None + + +@dataclass(frozen=True, slots=True) +class IntentSelection: + """Which of a guild's unapplied intents the bot acts on, and which it buries. + + `apply` is the one the bot configures the guild from, or `None` when + there is nothing to do. `supersede` is every other unapplied intent, + which the bot settles as `SUPERSEDED` without ever acting on it. + + Two fields rather than one, because both halves have to happen in the + same pass. Applying the newest and leaving the older ones pending + would have the next tick apply the newest all over again -- there + would still be an unapplied row, and nothing in the table would say + the guild had already been configured from it. + """ + + apply: SetupIntent | None + supersede: tuple[SetupIntent, ...] + + +def select_intent(pending: Sequence[SetupIntent]) -> IntentSelection: + """The newest unapplied intent wins; every older one is superseded. + + See the module docstring for why the newest rather than the oldest, + and why "refuse while one is pending" was not the rule chosen. + + Ordered by `requested_at` and then by `id`, not by the order the + caller happened to read the rows in. The id breaks a tie because two + requests can share an instant -- a pinned clock in a test, a coarse + one in production -- and it is monotonic, so it settles the tie in the + same direction the timestamp would have. + + Already-settled rows are ignored rather than settled a second time. + The store hands over unapplied rows only, so this is belt and braces; + but rewriting an `applied` outcome to `superseded` would lose the one + record that the bot ever did anything to this guild. + """ + unapplied = sorted( + (intent for intent in pending if intent.is_pending), + key=lambda intent: (intent.requested_at, intent.id), + ) + if not unapplied: + return IntentSelection(apply=None, supersede=()) + return IntentSelection(apply=unapplied[-1], supersede=tuple(unapplied[:-1])) + + +#: Where Discord takes somebody who is adding an application to a server. +_AUTHORIZE_URL = "https://discord.com/oauth2/authorize" + +#: What the bot is invited with. `bot` is what puts it in the server; +#: `applications.commands` is what lets its command tree be registered +#: there, so `/setup`, `/config` and `/consent` exist for the guild +#: without a second authorisation round. +INVITE_SCOPES: tuple[str, ...] = ("bot", "applications.commands") + +#: View Channel, Connect, Send Messages, Manage Roles -- the four +#: `docs/first-deployment.md` section 2 tells an operator to tick, as the +#: number Discord's own URL generator produces for them. +#: +#: `Manage Roles` is the one that fails late if it is missing. It covers +#: both halves of what a setup intent asks for -- creating the consent +#: role, and writing the Speak overwrites that are the primary layer of +#: the consent protection (Spec 3.1) -- and a bot invited without it +#: joins happily, mirrors happily, and fails the first intent it is +#: handed. `Send Messages` fails later still: everything records, +#: transcribes and publishes, and only posting the link back to the +#: channel fails. +#: +#: `Speak` is deliberately absent. The bot only ever listens, and the +#: overwrites it writes are about everybody else. +#: +#: **Discord's role position is not in here and cannot be.** The bot's own +#: role has to sit above the consent role or Discord refuses the edit, and +#: no bitmask expresses that -- it is a drag in Server Settings after the +#: invite, which is why the deployment guide says so, and why an intent +#: can still fail with a permission error on a guild that granted every +#: permission on this list. +INVITE_PERMISSIONS = "269487104" + + +def invite_url(client_id: str) -> str: + """The `bot`-scope authorize link for this deployment's application. + + The one onboarding step that is genuinely web-doable: this URL is + public and buildable from the application's client id alone. `api` + holds no Discord token and never will (Spec 13.2), so everything else + -- the consent role, the Speak overwrites, the command tree -- is an + intent the bot applies. This is the link that gets the bot in the door + so that there is a bot to apply them. + + Refuses a client id that is not a snowflake. It is the one value here + that reaches a URL somebody is asked to click, and building a link out + of whatever happened to be configured would put that into a query + string the console hands an administrator. A misconfigured id is a + deployment error, and `ApiSettings` raises it at startup rather than + on the first onboarding page. + """ + if not client_id.isdigit(): + raise ValueError( + "A Discord application id is a snowflake, so it is digits and nothing else." + ) + query = urlencode( + { + "client_id": client_id, + # Space-separated, which `urlencode` renders as `+` -- the + # spelling Discord's own URL generator produces. + "scope": " ".join(INVITE_SCOPES), + "permissions": INVITE_PERMISSIONS, + } + ) + return f"{_AUTHORIZE_URL}?{query}" diff --git a/src/sturnus/entrypoints/api.py b/src/sturnus/entrypoints/api.py index cbe4d29..46c81e4 100644 --- a/src/sturnus/entrypoints/api.py +++ b/src/sturnus/entrypoints/api.py @@ -33,7 +33,7 @@ from datetime import UTC, datetime, timedelta from aiohttp import web -from pydantic import SecretStr +from pydantic import SecretStr, field_validator from sqlalchemy import inspect from sqlalchemy.ext.asyncio import AsyncEngine, async_sessionmaker, create_async_engine @@ -44,6 +44,7 @@ ConsoleGuildNames, ConsoleGuildOAuthClients, ConsoleGuildReports, + ConsoleGuildSetup, ConsoleLinkDirectory, ConsolePersonalConsents, ConsoleProfileDirectory, @@ -61,6 +62,7 @@ from sturnus.console.audio import AudioDelivery from sturnus.console.queries import ConsoleQueries from sturnus.console.session import SessionCookie +from sturnus.domain.onboarding import invite_url from sturnus.infrastructure.crypto import KeyWrapper from sturnus.infrastructure.db.admin_members import AdminMemberStore from sturnus.infrastructure.db.config_store import ConfigStore @@ -75,12 +77,14 @@ GuildExportTarget, GuildMember, GuildRole, + GuildSetupIntent, OutlineCollection, SessionDocument, UserPreference, ) from sturnus.infrastructure.db.models import GuildOAuthClient as GuildOAuthClientRow from sturnus.infrastructure.db.preferences import PreferenceStore +from sturnus.infrastructure.db.setup_intents import SetupIntentStore from sturnus.infrastructure.documents.outline_oauth import OutlineOAuth from sturnus.infrastructure.objectstore import S3AudioStore, S3DocumentStore from sturnus.infrastructure.observability import init_sentry @@ -133,6 +137,11 @@ # `guild_config` is on this list for. GuildExportTarget.__tablename__, SessionDocument.__tablename__, + # Where onboarding is written down. On the list for the same + # reason `guild_config` is: without it `/readyz` would pass while + # the first person trying to set a guild up got a 500 for their + # trouble. + GuildSetupIntent.__tablename__, "session", "session_participant", "transcription_job", @@ -176,6 +185,35 @@ class ApiSettings(StrictSettings): master_key_id: str health_port: int = 8080 console_origin: str = "https://sturnus.onelitefeather.dev" + #: This deployment's Discord application id, used for one thing: the + #: `bot`-scope invite link the console offers. Public by design -- it + #: appears in every invite URL ever clicked -- so it travels as plain + #: configuration and never through the Secret, and it is emphatically + #: not a token: it grants this process nothing (Spec 13.2). + #: + #: Optional, so a deployment that has not set it yet starts and serves + #: everything else; the invite endpoint then answers `url: null`, + #: which the console renders as "this deployment has no invite link + #: configured" rather than as an error. + discord_client_id: str | None = None + + @field_validator("discord_client_id", mode="after") + @classmethod + def _client_id_is_a_snowflake(cls, value: str | None) -> str | None: + """Blank is absent; anything that is not digits fails at startup. + + The alternative is a console that signs people in, offers an + invite button, and hands whoever clicks it a Discord page that + cannot say what application it is being asked to authorise. + Failing here names the variable while an operator is still looking + at the deployment. + """ + if value is None or not value.strip(): + return None + # Raises `ValueError` for anything that is not a snowflake, with + # the reason argued where the rule lives. + invite_url(value.strip()) + return value.strip() async def _wait_for_schema(engine: AsyncEngine) -> None: @@ -305,6 +343,8 @@ def now() -> datetime: settings.s3_secret_key.get_secret_value(), ), oauth_clients=ConsoleGuildOAuthClients(oauth_clients, admins), + setup=ConsoleGuildSetup(session_factory, admins, SetupIntentStore(session_factory)), + discord_client_id=settings.discord_client_id, ) runner = web.AppRunner(app) diff --git a/src/sturnus/entrypoints/bot.py b/src/sturnus/entrypoints/bot.py index 94769ec..95608b2 100644 --- a/src/sturnus/entrypoints/bot.py +++ b/src/sturnus/entrypoints/bot.py @@ -38,6 +38,7 @@ JobRepository, SessionRepository, ) +from sturnus.infrastructure.db.setup_intents import SetupIntentStore from sturnus.infrastructure.discord.announcer import DiscordAnnouncer from sturnus.infrastructure.discord.client import SturnusClient from sturnus.infrastructure.discord.link_cog import PROVIDER as OUTLINE_PROVIDER @@ -204,6 +205,11 @@ async def _run() -> None: # snowflakes back, because `api` cannot ask what they are called # either. This is where the names come from. directory_mirror = DirectoryStore(session_factory) + # And the same arrangement run backwards. Every step of setting a + # guild up needs a Discord token, so the console writes down what + # should be true and the tick makes it true -- see + # `sturnus.infrastructure.discord.setup_apply`. + setup_intents = SetupIntentStore(session_factory) # `provider` fixed at construction: the bot only ever reads its own # Outline mapping back (`/link status`), never another provider's -- # see `AccountLinkRepository`'s class docstring for why the read and @@ -287,6 +293,7 @@ async def database_ping() -> bool: config_store=config_store, admin_mirror=admin_mirror, directory_mirror=directory_mirror, + setup_intents=setup_intents, consent_repo=consent_repo, session_repo=session_repo, job_repo=traced_job_repo, diff --git a/src/sturnus/infrastructure/discord/client.py b/src/sturnus/infrastructure/discord/client.py index 239ba12..c2ea015 100644 --- a/src/sturnus/infrastructure/discord/client.py +++ b/src/sturnus/infrastructure/discord/client.py @@ -84,6 +84,7 @@ ConsentRepository, SessionRepository, ) +from sturnus.infrastructure.db.setup_intents import SetupIntentStore from sturnus.infrastructure.discord.about_cog import AboutCog from sturnus.infrastructure.discord.admin_sync import sync_administrators from sturnus.infrastructure.discord.announcer import DiscordAnnouncer @@ -93,6 +94,7 @@ from sturnus.infrastructure.discord.directory_sync import sync_directory from sturnus.infrastructure.discord.link_cog import LinkCog from sturnus.infrastructure.discord.queue_cog import QueueCog +from sturnus.infrastructure.discord.setup_apply import apply_setup_intents from sturnus.infrastructure.discord.setup_cog import SetupCog from sturnus.infrastructure.discord.voice import VoiceReceiveAdapter, voice_close_code from sturnus.infrastructure.documents.outline_oauth import OutlineOAuth @@ -349,6 +351,7 @@ def __init__( config_store: ConfigStore, admin_mirror: AdminMemberStore | None = None, directory_mirror: DirectoryStore | None = None, + setup_intents: SetupIntentStore | None = None, consent_repo: ConsentRepository, session_repo: SessionRepository, # Typed against the narrow `JobQueue` port rather than the concrete @@ -408,6 +411,12 @@ def __init__( #: with no interest in the console gets. In production #: `sturnus.entrypoints.bot` always supplies it. self._directory_mirror = directory_mirror + #: The inverse of the two mirrors above: what the console asked + #: this process to do to a guild, because `api` holds no Discord + #: token and cannot do it itself. Optional on the same terms -- + #: a client built without one applies nothing, which is what a + #: test with no interest in onboarding gets. + self._setup_intents = setup_intents self._consent_repo = consent_repo self._session_repo = session_repo self._job_repo = job_repo @@ -1929,6 +1938,11 @@ async def _tick_guild(self, guild_id: int, now: datetime) -> None: # is swept, closed and put back to idle on its own terms. for recording in self._recordings_of(guild_id): await self._sweep_due_session(guild_id, recording, now) + # Before the reconcile, deliberately: an intent writes the very + # configuration keys the reconcile reads, so applying it first + # is what lets a guild set up from the console start recording + # on this tick instead of the next one. + await self._apply_setup_intents(guild_id, now) await self._reconcile(guild_id) await self._mirror_administrators(guild_id, now) await self._mirror_directory(guild_id, now) @@ -1939,6 +1953,51 @@ async def _tick_guild(self, guild_id: int, now: datetime) -> None: if blocked_until is not None and now >= blocked_until: await self._end_capture_cooldown(guild_id) + async def _apply_setup_intents(self, guild_id: int, now: datetime) -> None: + """Does to this guild what the console wrote down that it wanted. + + The mirror arrangement run backwards. `api` must never hold a + Discord token (Spec 13.2), so it cannot create the consent role, + write the Speak overwrites or register the command tree; it writes + an intent and this applies it, through the same `plan_setup` + `/setup` uses. + + On the ordinary tick and not a sweep of its own, for the reason + the mirrors are: it costs one indexed read of one guild's rows, + and does nothing at all unless somebody has asked for something. + + A guild this process cannot currently see is skipped, and its + intents stay pending. That is the honest state during onboarding + -- the bot has not been invited yet, or has not finished joining + -- and it is what the console renders as "waiting for the bot to + arrive" rather than as a failure. + + Failures are logged and swallowed, like the mirrors': a guild + whose setup could not be applied keeps the configuration it had, + and letting this raise would take the session timeout enforcement + in the same tick down with it. The intent itself is settled by + `apply_setup_intents`, so a raise here does not leave a row that + the next tick retries -- see its docstring on the retry bound. + """ + if self._setup_intents is None: + return + guild = self.get_guild(guild_id) + if guild is None: + return + try: + await apply_setup_intents(guild, self._config_store, self._setup_intents, now) + except Exception as exc: + log_exception( + log, + logging.WARNING, + Event.GUILD_TICK_FAILED, + "Could not apply this guild's setup request; the guild keeps the " + "configuration it already had", + exc, + guild_id=guild_id, + shard_id=self._shard(guild_id), + ) + async def _mirror_administrators(self, guild_id: int, now: datetime) -> None: """Writes this guild's administrators where the console's API can read them. diff --git a/src/sturnus/infrastructure/discord/setup_apply.py b/src/sturnus/infrastructure/discord/setup_apply.py new file mode 100644 index 0000000..078dfd7 --- /dev/null +++ b/src/sturnus/infrastructure/discord/setup_apply.py @@ -0,0 +1,418 @@ +"""Setting a guild up against Discord, for `/setup` and for the console alike. + +Two things live here. The first is the Discord half of setting a guild +up -- reading a voice channel's Speak overwrites, and writing them back -- +which `SetupCog` and the console's intent applier both need and which +neither may own. Those two overwrites are the primary layer of the +consent protection (Spec 3.1): deny `Speak` to `@everyone`, allow it for +the consent role, on **every** allowed channel. A second implementation +that got one of them backwards would let somebody be recorded without +having consented, so there is one implementation, called twice, exactly +as there is one `plan_setup`. + +The second is `apply_setup_intents`, the inverse of the mirrors. `api` +must never hold a Discord token (Spec 13.2), so the console cannot create +the consent role, write those overwrites, or register the command tree. +It writes down what should be true instead -- a row in +`guild_setup_intent` -- and this runs on the bot's ordinary ten-second +tick, makes it true through the same `plan_setup` the slash command uses, +and writes back what happened. + +**The retry bound is one attempt, and it is the table's own design.** The +tick runs six times a minute forever. An intent left pending after being +applied would re-create the role and re-write the overwrites for the life +of the guild; one left pending after failing would retry a permission +error against Discord's rate limiter just as often. So an attempt settles +the intent whichever way it went, `error` says what Discord answered, and +an administrator who has fixed the permission asks again -- which is a new +row that says who asked and when. There is no back-off to tune because +there is no second attempt to back off from. + +**A guild the bot has not joined is not a failure.** It has no +`discord.Guild` for the tick to iterate, so its intents are never +attempted at all and stay pending -- which is the honest state, and the +one the console renders as "waiting for the bot to arrive". They are +applied on the tick after it joins. +""" + +from __future__ import annotations + +import logging +from collections.abc import Sequence +from datetime import datetime +from typing import Protocol + +import discord + +from sturnus.application.setup_plan import ChannelPermissions, PermissionChange, plan_setup +from sturnus.domain import settings +from sturnus.domain.onboarding import APPLIED, FAILED, SUPERSEDED, SetupIntent, select_intent +from sturnus.observability.events import Event, log_event + +log = logging.getLogger(__name__) + +#: Reason recorded on Discord's audit log for every permission change and +#: every role creation setup makes. Names the specification section so +#: somebody reading a guild's audit log a year from now can find out why a +#: bot denied `@everyone` the ability to speak. +AUDIT_REASON = "Sturnus setup: consent protection (Spec 3.1)" + + +class GuildConfiguration(Protocol): + """The per-guild configuration store, as narrowly as this module needs it.""" + + async def get_stored(self, guild_id: int, key: str) -> str | None: ... + + async def set(self, guild_id: int, key: str, value: str | None, now: datetime) -> None: ... + + +class SetupIntents(Protocol): + """What the console asked for, and where the answer goes.""" + + async def pending_for(self, guild_id: int) -> Sequence[SetupIntent]: ... + + async def record_outcome( + self, intent_id: int, *, outcome: str, error: str | None, now: datetime + ) -> bool: ... + + +# --------------------------------------------------------------------------- +# The Discord half, shared by both callers +# --------------------------------------------------------------------------- + + +def resolve_voice_channel( + guild: discord.Guild, channel_id: int, named: discord.VoiceChannel | None = None +) -> discord.VoiceChannel | None: + """The voice channel with this id, or `None` if there is no such thing. + + `named` is a channel Discord's own picker resolved for this call, and + it is returned for its own id without a cache lookup -- the object in + hand is at least as good as anything the cache would yield, and it is + the one the administrator actually chose. A console intent carries no + such object and passes nothing. + """ + if named is not None and channel_id == named.id: + return named + channel = guild.get_channel(channel_id) + return channel if isinstance(channel, discord.VoiceChannel) else None + + +def read_channel_permissions( + guild: discord.Guild, + role: discord.Role | None, + channel_ids: Sequence[int], + *, + named: discord.VoiceChannel | None = None, +) -> list[ChannelPermissions]: + """Reads each allowed channel's Speak overwrites, skipping what it cannot see. + + A stored list can name a channel that has since been deleted, or one + this bot has no view of. Such an entry is left out rather than guessed + at: planning an overwrite for a channel that is not there would only + produce an error the administrator can do nothing about, and it must + not stop the channels that *do* exist from being fixed. + """ + permissions: list[ChannelPermissions] = [] + for channel_id in sorted(set(channel_ids)): + channel = resolve_voice_channel(guild, channel_id, named) + if channel is None: + continue + permissions.append( + ChannelPermissions( + channel_id=channel_id, + everyone_may_speak=(channel.overwrites_for(guild.default_role).speak is not False), + role_may_speak=(role is not None and channel.overwrites_for(role).speak is True), + ) + ) + return permissions + + +async def apply_permission_changes( + guild: discord.Guild, + everyone: discord.Role, + consent_role: discord.Role | None, + changes: Sequence[PermissionChange], + *, + named: discord.VoiceChannel | None = None, +) -> tuple[list[str], list[str]]: + """Applies each overwrite independently; one failure never blocks the other. + + Returns the human-readable description of every change that succeeded, + and of every one that did not -- a permission failure is reported, + never swallowed, because a half-applied setup that claims success is + worse than one that admits it stopped. That holds per *channel* as well + as per overwrite: a guild whose second meeting room the bot has no + Manage Permissions on must still have its first one configured, and + must be told which one failed. + + The wording is written to be read by a person in either place it can + surface -- an ephemeral `/setup` reply, or `guild_setup_intent.error` + rendered in the console -- so it names the channel and the next action + rather than quoting an HTTP status. + """ + applied: list[str] = [] + errors: list[str] = [] + for change in changes: + verb = "allow" if change.allow_speak else "deny" + channel = resolve_voice_channel(guild, change.channel_id, named) + if channel is None: + errors.append( + f"Could not {verb} Speak in <#{change.channel_id}>: it is not a voice " + "channel I can see. Remove it with " + f"`/config set {settings.VOICE_CHANNEL_IDS} `." + ) + continue + target: discord.Role | None + label: str + if change.target == "everyone": + target, label = everyone, "@everyone" + else: + target, label = consent_role, "the consent role" + if target is None: + errors.append( + f"Could not {verb} Speak for the consent role in " + f"{channel.mention}: no consent role exists yet. Create one and " + "run /setup again, or set it manually and run " + f"`/config set {settings.CONSENT_ROLE_ID} `." + ) + continue + + overwrite = channel.overwrites_for(target) + # `.update()` rather than `overwrite.speak = ...`: PermissionOverwrite + # defines its permission attributes only under `TYPE_CHECKING`, which + # mypy strict does not accept as a real `__slots__` member for plain + # attribute assignment. + overwrite.update(speak=change.allow_speak) + try: + await channel.set_permissions(target, overwrite=overwrite, reason=AUDIT_REASON) + except discord.Forbidden: + errors.append( + f"Could not {verb} Speak for {label} in {channel.mention}: I am " + "missing the Manage Permissions permission on that channel. Set it " + "manually." + ) + except discord.HTTPException as exc: + errors.append(f"Could not {verb} Speak for {label} in {channel.mention}: {exc}") + else: + applied.append(f"{verb.capitalize()}ed Speak for {label} in {channel.mention}.") + return applied, errors + + +# --------------------------------------------------------------------------- +# The console's half: applying an intent +# --------------------------------------------------------------------------- + + +async def apply_setup_intents( + guild: discord.Guild, + config: GuildConfiguration, + intents: SetupIntents, + now: datetime, +) -> None: + """Applies this guild's newest unapplied intent, and buries the rest. + + Does nothing at all when there is nothing pending, which is every tick + but the handful after somebody presses the button in the console. + + Which intent is applied is `select_intent`'s decision and is argued + where it is made: the newest ask wins outright and every older one + settles as `SUPERSEDED` without being applied. Applying them in + sequence would end on whichever request happened to be older, which is + a correction being overwritten by the mistake it corrected. + """ + pending = await intents.pending_for(guild.id) + if not pending: + return + + selection = select_intent(pending) + for stale in selection.supersede: + await intents.record_outcome(stale.id, outcome=SUPERSEDED, error=None, now=now) + log_event( + log, + logging.INFO, + Event.SETUP_INTENT_SUPERSEDED, + "A newer setup request replaced this one before the bot reached it", + guild_id=guild.id, + discord_user_id=stale.requested_by, + ) + if selection.apply is None: + return + + outcome, error = await _apply_one(guild, config, selection.apply, now) + # Conditional on the intent still being unapplied, so two ticks racing + # on one guild produce one application and one honest `False`. The + # loser says nothing: the winner already logged what happened. + if not await intents.record_outcome(selection.apply.id, outcome=outcome, error=error, now=now): + return + log_event( + log, + logging.WARNING if outcome == FAILED else logging.INFO, + Event.SETUP_INTENT_APPLIED, + "The bot applied a setup request the console wrote", + guild_id=guild.id, + discord_user_id=selection.apply.requested_by, + outcome=outcome, + ) + + +async def _apply_one( + guild: discord.Guild, + config: GuildConfiguration, + intent: SetupIntent, + now: datetime, +) -> tuple[str, str | None]: + """Configures one guild from one intent, and says how it went. + + The database writes happen whether or not the Discord calls succeed, + exactly as `/setup`'s do: the two are independent, and refusing to + store what we *did* determine because a permission edit failed would + leave the guild worse off than before the request. `FAILED` therefore + means "not everything the bot was asked to do happened", not "nothing + happened" -- which is why `error` names each part that did not. + """ + current = { + key: await config.get_stored(guild.id, key) + # The deprecated singular key rides along: a guild untouched since + # the rename still has its channel there, and this must add to that + # list rather than start a new one beside it. + for key in settings.REQUIRED_KEYS | {settings.VOICE_CHANNEL_ID} + } + + problems: list[str] = [] + try: + stored_channel_ids = settings.recording_channel_ids(current) + except settings.InvalidChannelList as exc: + # Only reachable through a direct `UPDATE` -- `ConfigStore.set` + # refuses an unparseable list at the write. Reported rather than + # swallowed, because the list this request goes on to write will + # not contain whatever the unreadable one meant. + problems.append( + f"The stored `{settings.VOICE_CHANNEL_IDS}` could not be read ({exc}), so it " + "was replaced by the channels this request named rather than added to." + ) + stored_channel_ids = () + + try: + requested = ( + () if intent.channel_ids is None else settings.parse_channel_ids(intent.channel_ids) + ) + except settings.InvalidChannelList as exc: + # The API validates the list before it writes one, so this is a row + # somebody wrote by hand. Terminal, and named: there is nothing the + # bot can do with a channel list it cannot read. + return FAILED, f"The requested channel list could not be read: {exc}" + + named_role = _role_named(guild, intent.consent_role_name) + stored_role = _stored_role(guild, current) + # A request that names a role means that role, whatever is stored. If + # it exists it is used as-is; if it does not, the stored one is + # deliberately *not* kept, so the planner asks for a creation and the + # new role gets the name that was asked for. + keeps_stored_role = intent.consent_role_name is None and stored_role is not None + role = named_role if named_role is not None else (stored_role if keeps_stored_role else None) + + channel_permissions = read_channel_permissions(guild, role, (*stored_channel_ids, *requested)) + # A channel this request named that the bot cannot see is refused + # rather than added. The console picks from the mirror, so this means + # the mirror is behind Discord -- and adding the channel anyway would + # put a room on the allowed list whose Speak overwrites nobody wrote, + # which is a hole in the consent protection that looks like a + # configured guild. A channel already *stored* and now unseeable is a + # different case and left alone: it is not this request's doing, and + # failing every future request over it would help nobody. + visible = {each.channel_id for each in channel_permissions} + unseen = tuple(channel_id for channel_id in requested if channel_id not in visible) + problems.extend( + f"This request named <#{channel_id}>, which is not a voice channel I can see, " + "so it was not added. Pick it again once the console shows it." + for channel_id in unseen + ) + + plan = plan_setup( + current=current, + added_channel_ids=tuple(channel_id for channel_id in requested if channel_id in visible), + stored_channel_ids=stored_channel_ids, + channel_permissions=channel_permissions, + role_id=named_role.id if named_role is not None else None, + stored_role_valid=keeps_stored_role, + # Never carried by an intent: the console sets the policy on the + # settings page, and a second place to write `policy_version` is a + # second way to invalidate every consent in the guild. + policy_url=None, + policy_version=None, + ) + + if plan.role_to_create is not None: + # The name the console asked for wins over the planner's default, + # which is what the planner falls back to when nobody named one. + wanted = intent.consent_role_name or plan.role_to_create + try: + role = await guild.create_role(name=wanted, reason=AUDIT_REASON) + except discord.Forbidden: + problems.append( + f"I am missing the Manage Roles permission, so I could not create the " + f"`{wanted}` role. Grant it -- and make sure my own role sits above the " + "consent role in Server Settings -> Roles -- then ask again." + ) + except discord.HTTPException as exc: + problems.append(f"Could not create the `{wanted}` role: {exc}") + + writes = dict(plan.writes) + if plan.role_to_create is not None and role is not None: + # A role was just created; its id is only known now, so the plan + # could not have included it. A reused stored role is already + # correctly stored and needs no write. + writes[settings.CONSENT_ROLE_ID] = str(role.id) + for key, value in writes.items(): + await config.set(guild.id, key, value, now) + + _, permission_errors = await apply_permission_changes( + guild, guild.default_role, role, plan.permission_changes + ) + problems.extend(permission_errors) + + if problems: + return FAILED, "\n".join(problems) + return APPLIED, None + + +def _role_named(guild: discord.Guild, name: str | None) -> discord.Role | None: + """The guild's role with exactly this name, if it already has one. + + Asked before anything is created, so asking twice for the same consent + role is idempotent rather than a guild slowly filling with identically + named roles. Exact match and never fuzzy: "Recorded" and "recorded" + are two roles in Discord, and picking the wrong one would grant + recording consent through a role nobody was told about. + """ + if name is None: + return None + return discord.utils.get(guild.roles, name=name) + + +def _stored_role(guild: discord.Guild, current: dict[str, str | None]) -> discord.Role | None: + """The configured consent role, if the id still names one in this guild. + + A role deleted out from under a stale stored id is not usable, and + counts the same as never having configured one -- the alternative is + writing overwrites for a role that grants nobody anything. + """ + stored = current.get(settings.CONSENT_ROLE_ID) + if stored is None: + return None + try: + return guild.get_role(int(stored)) + except ValueError: + return None + + +__all__ = [ + "AUDIT_REASON", + "GuildConfiguration", + "SetupIntents", + "apply_permission_changes", + "apply_setup_intents", + "read_channel_permissions", + "resolve_voice_channel", +] diff --git a/src/sturnus/infrastructure/discord/setup_cog.py b/src/sturnus/infrastructure/discord/setup_cog.py index d34df79..39c259a 100644 --- a/src/sturnus/infrastructure/discord/setup_cog.py +++ b/src/sturnus/infrastructure/discord/setup_cog.py @@ -32,6 +32,13 @@ a guild. This module only reads Discord state into that function's parameters and turns its result into Discord API calls, reporting exactly what succeeded and what did not. + +**Both halves are shared with the console's intent applier.** `plan_setup` +decides, and `sturnus.infrastructure.discord.setup_apply` reads and writes +the overwrites; this cog owns neither. One planner and one applier, two +callers -- a second implementation of the consent protection that got +either overwrite backwards would let somebody be recorded without having +consented. """ from __future__ import annotations @@ -44,23 +51,19 @@ from sturnus.application.ports import Clock from sturnus.application.reconfigure import Reconfigure, ReconfigureResult -from sturnus.application.setup_plan import ( - ChannelPermissions, - PermissionChange, - RoleAction, - SetupPlan, - plan_setup, -) +from sturnus.application.setup_plan import RoleAction, SetupPlan, plan_setup from sturnus.domain import settings from sturnus.infrastructure.db.config_store import ConfigStore from sturnus.infrastructure.discord.config_cog import render_write_result from sturnus.infrastructure.discord.permissions import require_admin +from sturnus.infrastructure.discord.setup_apply import ( + AUDIT_REASON, + apply_permission_changes, + read_channel_permissions, +) log = logging.getLogger(__name__) -#: Reason recorded on the audit log for every permission change setup makes. -_AUDIT_REASON = "Sturnus /setup: consent protection (Spec 3.1)" - class SetupCog(commands.Cog): """Admin-only `/setup` command; every reply is ephemeral.""" @@ -157,13 +160,16 @@ async def setup( # Every channel the guild will allow after this call, the new one # included -- so the plan can report which of them still need the # consent protection applied, not merely this one. - channel_permissions = self._read_permissions( + channel_permissions = read_channel_permissions( guild, role, (*stored_channel_ids, channel.id), named=channel ) plan = plan_setup( current=current, - channel_id=channel.id, + # One room per call: Discord renders one channel picker per + # command parameter. The console names the whole list at once, + # which is why the planner takes a tuple. + added_channel_ids=(channel.id,), stored_channel_ids=stored_channel_ids, channel_permissions=channel_permissions, role_id=consent_role.id if consent_role is not None else None, @@ -175,7 +181,7 @@ async def setup( role_error: str | None = None if plan.role_to_create is not None: try: - role = await guild.create_role(name=plan.role_to_create, reason=_AUDIT_REASON) + role = await guild.create_role(name=plan.role_to_create, reason=AUDIT_REASON) except discord.Forbidden: role_error = ( "I am missing the Manage Roles permission, so I could not create " @@ -208,7 +214,7 @@ async def setup( log.exception("Reconcile after /setup failed for guild %d", guild.id) result = None - applied, permission_errors = await self._apply_permission_changes( + applied, permission_errors = await apply_permission_changes( guild, guild.default_role, role, plan.permission_changes, named=channel ) @@ -232,121 +238,6 @@ async def setup( ephemeral=True, ) - def _resolve( - self, guild: discord.Guild, channel_id: int, named: discord.VoiceChannel - ) -> discord.VoiceChannel | None: - """The voice channel with this id, or `None` if there is no such thing. - - `named` is the channel Discord's own picker resolved for this call, - and it is returned for its own id without a cache lookup -- the - object in hand is at least as good as anything the cache would - yield, and it is the one the administrator actually chose. - """ - if channel_id == named.id: - return named - channel = guild.get_channel(channel_id) - return channel if isinstance(channel, discord.VoiceChannel) else None - - def _read_permissions( - self, - guild: discord.Guild, - role: discord.Role | None, - channel_ids: tuple[int, ...], - *, - named: discord.VoiceChannel, - ) -> list[ChannelPermissions]: - """Reads each allowed channel's Speak overwrites, skipping what it cannot see. - - A stored list can name a channel that has since been deleted, or one - this bot has no view of. Such an entry is left out rather than - guessed at: planning an overwrite for a channel that is not there - would only produce an error the administrator can do nothing about, - and it must not stop the channels that *do* exist from being fixed. - """ - permissions: list[ChannelPermissions] = [] - for channel_id in sorted(set(channel_ids)): - channel = self._resolve(guild, channel_id, named) - if channel is None: - continue - permissions.append( - ChannelPermissions( - channel_id=channel_id, - everyone_may_speak=( - channel.overwrites_for(guild.default_role).speak is not False - ), - role_may_speak=( - role is not None and channel.overwrites_for(role).speak is True - ), - ) - ) - return permissions - - async def _apply_permission_changes( - self, - guild: discord.Guild, - everyone: discord.Role, - consent_role: discord.Role | None, - changes: list[PermissionChange], - *, - named: discord.VoiceChannel, - ) -> tuple[list[str], list[str]]: - """Applies each overwrite independently; one failure never blocks the other. - - Returns the human-readable description of every change that - succeeded, and of every one that did not -- a permission failure is - reported, never swallowed, because a half-applied setup that claims - success is worse than one that admits it stopped. That now holds - per *channel* as well as per overwrite: a guild whose second - meeting room the bot has no Manage Permissions on must still have - its first one configured, and must be told which one failed. - """ - applied: list[str] = [] - errors: list[str] = [] - for change in changes: - verb = "allow" if change.allow_speak else "deny" - channel = self._resolve(guild, change.channel_id, named) - if channel is None: - errors.append( - f"Could not {verb} Speak in <#{change.channel_id}>: it is not a voice " - "channel I can see. Remove it with " - f"`/config set {settings.VOICE_CHANNEL_IDS} `." - ) - continue - target: discord.Role | None - label: str - if change.target == "everyone": - target, label = everyone, "@everyone" - else: - target, label = consent_role, "the consent role" - if target is None: - errors.append( - f"Could not {verb} Speak for the consent role in " - f"{channel.mention}: no consent role exists yet. Create one and " - "run /setup again, or set it manually and run " - f"`/config set {settings.CONSENT_ROLE_ID} `." - ) - continue - - overwrite = channel.overwrites_for(target) - # `.update()` rather than `overwrite.speak = ...`: PermissionOverwrite - # defines its permission attributes only under `TYPE_CHECKING`, which - # mypy strict does not accept as a real `__slots__` member for plain - # attribute assignment. - overwrite.update(speak=change.allow_speak) - try: - await channel.set_permissions(target, overwrite=overwrite, reason=_AUDIT_REASON) - except discord.Forbidden: - errors.append( - f"Could not {verb} Speak for {label} in {channel.mention}: I am " - "missing the Manage Permissions permission on that channel. Set it " - "manually." - ) - except discord.HTTPException as exc: - errors.append(f"Could not {verb} Speak for {label} in {channel.mention}: {exc}") - else: - applied.append(f"{verb.capitalize()}ed Speak for {label} in {channel.mention}.") - return applied, errors - def _render_summary( writes: dict[str, str], diff --git a/src/sturnus/observability/events.py b/src/sturnus/observability/events.py index 7b2d821..be79da7 100644 --- a/src/sturnus/observability/events.py +++ b/src/sturnus/observability/events.py @@ -92,6 +92,23 @@ class Event(StrEnum): VOICE_LEFT_FAILED = "voice.left_failed" VOICE_REJOIN_BLOCKED = "voice.rejoin_blocked" GUILD_TICK_FAILED = "guild.tick_failed" + #: The bot acted on what the console wrote into `guild_setup_intent`: + #: the consent role, the Speak overwrites, the configuration keys. + #: WARNING when `outcome` is `failed`, because the request came from a + #: person who is watching a page for an answer and the answer is that + #: it did not work. There is no retry to wait for -- an attempt settles + #: the intent either way -- so this line is the whole event. + SETUP_INTENT_APPLIED = "setup.intent_applied" + #: A pending setup request that a newer one replaced before the tick + #: reached either. Logged rather than silent because it is the visible + #: half of the rule: two administrators asking thirty seconds apart + #: leave one application and one of these. + SETUP_INTENT_SUPERSEDED = "setup.intent_superseded" + #: The console wrote a setup request for the bot to apply. The only + #: record that a person asked, as opposed to that the bot acted: the + #: two are separated by up to a tick, and by a guild the bot has not + #: joined for as long as that takes. + CONSOLE_SETUP_REQUESTED = "console.setup_requested" SESSION_CLOSE_FAILED = "session.close_failed" SESSION_OPENED = "session.opened" SESSION_SPEAKER_FIRST_PACKET = "session.speaker_first_packet" diff --git a/tests/application/test_setup_plan.py b/tests/application/test_setup_plan.py index 9841e5d..9adacaa 100644 --- a/tests/application/test_setup_plan.py +++ b/tests/application/test_setup_plan.py @@ -36,7 +36,7 @@ def permissions( def plan(current: dict[str, str | None] | None = None, **kw: object) -> SetupPlan: defaults: dict[str, object] = { "current": current or {}, - "channel_id": CHANNEL, + "added_channel_ids": (CHANNEL,), "stored_channel_ids": (), "channel_permissions": permissions(CHANNEL), "role_id": ROLE, @@ -189,3 +189,97 @@ def test_an_explicitly_supplied_role_overrides_a_stored_one() -> None: def test_an_unchanged_value_is_not_rewritten() -> None: result = plan({settings.VOICE_CHANNEL_IDS: str(CHANNEL)}, stored_channel_ids=(CHANNEL,)) assert settings.VOICE_CHANNEL_IDS not in result.writes + + +# --------------------------------------------------------------------------- +# The second caller: the console, through a setup intent +# --------------------------------------------------------------------------- +# +# One planner, two callers. The console asks for the whole list at once +# and is never told the policy, so the planner has to take both -- a +# second implementation of the consent protection is the last thing this +# system should grow. + + +def test_a_whole_list_of_channels_can_be_added_at_once() -> None: + """What the console asks for: the rooms somebody ticked, not one room.""" + result = plan( + None, + added_channel_ids=(CHANNEL, OTHER_CHANNEL), + channel_permissions=permissions(CHANNEL, OTHER_CHANNEL), + ) + assert result.channel_ids == (CHANNEL, OTHER_CHANNEL) + assert result.writes[settings.VOICE_CHANNEL_IDS] == f"{CHANNEL},{OTHER_CHANNEL}" + + +def test_naming_no_channel_leaves_the_stored_list_exactly_as_it_was() -> None: + """ "Fix the rest and leave the rooms alone" is a legitimate request. + + It is also the only shape in which the consent protection can be + re-applied to a guild whose channels are already correct but whose + overwrites are not. + """ + result = plan( + {settings.VOICE_CHANNEL_IDS: str(OTHER_CHANNEL)}, + added_channel_ids=(), + stored_channel_ids=(OTHER_CHANNEL,), + channel_permissions=permissions(OTHER_CHANNEL), + ) + assert result.channel_ids == (OTHER_CHANNEL,) + assert settings.VOICE_CHANNEL_IDS not in result.writes + assert [change.channel_id for change in result.permission_changes] == [ + OTHER_CHANNEL, + OTHER_CHANNEL, + ] + + +def test_a_guild_allowed_nowhere_and_given_nothing_is_told_the_key_is_missing() -> None: + """Never an empty write. + + `render_channel_ids(())` is the empty string and `ConfigStore.set` + refuses it, rightly -- "allowed to record nowhere" is what + `/config clear` is for. So the key goes unwritten and is reported as + what it is: still open. + """ + result = plan( + {settings.VOICE_CHANNEL_IDS: None}, + added_channel_ids=(), + stored_channel_ids=(), + channel_permissions=[], + ) + assert result.channel_ids == () + assert settings.VOICE_CHANNEL_IDS not in result.writes + assert settings.VOICE_CHANNEL_IDS in result.missing + + +def test_a_caller_that_was_told_no_policy_writes_none_and_overwrites_nothing() -> None: + """The console never carries the policy pair, and must not clear it. + + `policy_version` decides whose consent is still valid. A setup + request that quietly rewrote it would invalidate every consent in the + guild as a side effect of somebody ticking a channel. + """ + stored = { + settings.POLICY_URL: "https://example.org/policy", + settings.POLICY_VERSION: "2026-01-01", + } + result = plan(dict(stored), policy_url=None, policy_version=None) + assert settings.POLICY_URL not in result.writes + assert settings.POLICY_VERSION not in result.writes + + +def test_a_policy_nobody_supplied_and_nobody_stored_is_reported_missing() -> None: + """The guild cannot record yet, and the answer has to say so. + + `/setup` always supplies both from its own typed parameters, so it + never reaches this; a console intent never supplies either, so a guild + that never set them has to hear about it here rather than discover it + the first time nobody is recorded. + """ + result = plan( + {settings.POLICY_URL: None, settings.POLICY_VERSION: None}, + policy_url=None, + policy_version=None, + ) + assert settings.POLICY_URL in result.missing + assert settings.POLICY_VERSION in result.missing diff --git a/tests/console/conftest.py b/tests/console/conftest.py index ce47d70..f6be8ec 100644 --- a/tests/console/conftest.py +++ b/tests/console/conftest.py @@ -49,6 +49,8 @@ GuildQueue, GuildRecording, GuildReports, + GuildSetup, + GuildSetupState, GuildSignIn, LinkDirectory, OAuthClient, @@ -85,6 +87,7 @@ from sturnus.domain import preferences from sturnus.domain.exports import ExportTarget, SessionDocument from sturnus.domain.oauth_clients import GuildOAuthClient, SlugUnavailable, is_valid_slug +from sturnus.domain.onboarding import SetupIntent from sturnus.infrastructure.crypto import CHUNK_SIZE, encrypt_file from sturnus.infrastructure.documents.outline_oauth import ExternalIdentity, LinkExchangeError @@ -1011,6 +1014,61 @@ async def administered(self, *, requested_by: int) -> Sequence[AdministeredGuild ) +class FakeSetup: + """A guild nobody administers, until a test says otherwise. + + Defaults to `None` from both methods, which is what the real adapter + answers for "no such guild or not yours" -- so a test with no interest + in onboarding gets 404s rather than a fake that quietly authorises + everything. + + Writing a request mutates the state it will answer with next, because + the endpoint's whole contract is that `POST` and `GET` return the same + thing: a double that answered a fixed payload could not tell a + round-trip apart from a handler that never wrote anything. + """ + + def __init__(self, state: GuildSetupState | None = None) -> None: + self.state_to_answer = state + #: Every state read, with who asked. + self.asked: list[tuple[int, int]] = [] + #: Every request written: guild, who, the channel list as stored, + #: the role name, and when. + self.written: list[tuple[int, int, str, str | None, datetime]] = [] + + async def state(self, guild_id: int, *, requested_by: int) -> GuildSetupState | None: + self.asked.append((guild_id, requested_by)) + return self.state_to_answer + + async def request( + self, + guild_id: int, + *, + requested_by: int, + channel_ids: str, + consent_role_name: str | None, + now: datetime, + ) -> GuildSetupState | None: + if self.state_to_answer is None: + return None + self.written.append((guild_id, requested_by, channel_ids, consent_role_name, now)) + self.state_to_answer = GuildSetupState( + seen_at=self.state_to_answer.seen_at, + intent=SetupIntent( + id=len(self.written), + guild_id=guild_id, + requested_by=requested_by, + requested_at=now, + channel_ids=channel_ids, + consent_role_name=consent_role_name, + applied_at=None, + outcome=None, + error=None, + ), + ) + return self.state_to_answer + + class FakeCollections: """An Outline mirror nobody may read, until a test says otherwise. @@ -1227,6 +1285,8 @@ def build_test_api( documents: SessionDocumentDirectory | None = None, artefacts: DocumentArtefacts | None = None, oauth_clients: GuildOAuthClients | None = None, + setup: GuildSetup | None = None, + discord_client_id: str | None = None, sessions: SessionCookie | None = None, now: Callable[[], datetime] | None = None, schema_ready: bool = True, @@ -1285,6 +1345,8 @@ def build_test_api( documents=documents or FakeSessionDocuments(), artefacts=artefacts or FakeArtefacts(), oauth_clients=oauth_clients or FakeGuildOAuthClients(admins=administrators), + setup=setup or FakeSetup(), + discord_client_id=discord_client_id, sessions=sessions or SessionCookie(SECRET, timedelta(hours=12)), now=now or now_at(), schema_ready=lambda: schema_ready, diff --git a/tests/console/test_setup_directory.py b/tests/console/test_setup_directory.py new file mode 100644 index 0000000..b2bb62f --- /dev/null +++ b/tests/console/test_setup_directory.py @@ -0,0 +1,189 @@ +"""The console's write into `guild_setup_intent`, against the real database. + +No doubles, on purpose: the two properties under test are properties of +the rows. Authorisation is one `is_admin` inside the one call, and +`seen_at` is a column of the guild mirror rather than of the intent -- +which is the whole point of reading it, because it answers a different +question from anything an intent can. + +The rows are written through the real `SetupIntentStore` and +`DirectoryStore`, the writers the bot actually uses, so a test here reads +what those really produce rather than what this file believes they +produce. +""" + +from __future__ import annotations + +from datetime import UTC, datetime, timedelta + +import pytest +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker, create_async_engine + +from sturnus.application.directory_mirror import MirroredGuild +from sturnus.console.adapters import ConsoleGuildSetup +from sturnus.domain.onboarding import APPLIED +from sturnus.infrastructure.db.admin_members import AdminMemberStore +from sturnus.infrastructure.db.directory import DirectoryStore +from sturnus.infrastructure.db.models import Base +from sturnus.infrastructure.db.setup_intents import SetupIntentStore + +T0 = datetime(2026, 8, 23, 12, 0, 0, tzinfo=UTC) +T1 = T0 + timedelta(minutes=5) + +GUILD, OTHER_GUILD = 4711, 8822 +ANNA, BEN = 100, 200 +STANDUP, RETRO = 10, 11 + + +@pytest.fixture +async def factory(clean_database: str) -> async_sessionmaker[AsyncSession]: + engine = create_async_engine(clean_database) + async with engine.begin() as conn: + await conn.run_sync(Base.metadata.create_all) + return async_sessionmaker(engine, expire_on_commit=False) + + +async def a_setup( + factory: async_sessionmaker[AsyncSession], *, administers: int = ANNA +) -> ConsoleGuildSetup: + admins = AdminMemberStore(factory) + await admins.replace(GUILD, [administers], T0) + return ConsoleGuildSetup(factory, admins, SetupIntentStore(factory)) + + +async def test_somebody_who_administers_nothing_is_told_what_a_missing_guild_would_tell_them( + factory: async_sessionmaker[AsyncSession], +) -> None: + """404 from the route, and `None` from here that makes it one.""" + setup = await a_setup(factory) + assert await setup.state(GUILD, requested_by=BEN) is None + + +async def test_somebody_who_administers_nothing_cannot_ask_for_anything_either( + factory: async_sessionmaker[AsyncSession], +) -> None: + """The authorisation rule lives inside the call, not in the handler. + + A write is the half that matters: reading somebody else's onboarding + state is a leak, writing to it is an act on their server. + """ + setup = await a_setup(factory) + + written = await setup.request( + GUILD, requested_by=BEN, channel_ids=str(STANDUP), consent_role_name=None, now=T0 + ) + + assert written is None + assert await SetupIntentStore(factory).latest_for(GUILD) is None + + +async def test_an_administrator_of_one_guild_is_nobody_in_another( + factory: async_sessionmaker[AsyncSession], +) -> None: + setup = await a_setup(factory) + assert await setup.state(OTHER_GUILD, requested_by=ANNA) is None + + +async def test_a_guild_the_bot_has_never_swept_has_no_moment_to_show( + factory: async_sessionmaker[AsyncSession], +) -> None: + """The state the console renders as "waiting for the bot to arrive". + + Empty mirrors here mean nobody has looked, not that the server has no + channels -- and the two are indistinguishable without this field. + """ + setup = await a_setup(factory) + + state = await setup.state(GUILD, requested_by=ANNA) + + assert state is not None + assert state.seen_at is None + assert state.intent is None + + +async def test_a_guild_the_bot_has_swept_carries_when_it_last_looked( + factory: async_sessionmaker[AsyncSession], +) -> None: + setup = await a_setup(factory) + await DirectoryStore(factory).replace_guild(MirroredGuild(GUILD, "Acme", None), T1) + + state = await setup.state(GUILD, requested_by=ANNA) + + assert state is not None + assert state.seen_at == T1 + + +async def test_a_request_comes_back_as_the_guilds_current_answer( + factory: async_sessionmaker[AsyncSession], +) -> None: + setup = await a_setup(factory) + + state = await setup.request( + GUILD, + requested_by=ANNA, + channel_ids=f"{STANDUP},{RETRO}", + consent_role_name="Recorded", + now=T0, + ) + + assert state is not None + assert state.intent is not None + assert state.intent.requested_by == ANNA + assert state.intent.channel_ids == f"{STANDUP},{RETRO}" + assert state.intent.consent_role_name == "Recorded" + assert state.intent.is_pending + + +async def test_asking_twice_keeps_both_rows_and_answers_with_the_newer( + factory: async_sessionmaker[AsyncSession], +) -> None: + """An administrator who asked twice asked twice. + + Collapsing them would lose who asked for which and when. Which one the + guild is configured from is settled by the bot, which applies the + newest and settles the rest as superseded. + """ + setup = await a_setup(factory) + intents = SetupIntentStore(factory) + + await setup.request( + GUILD, requested_by=ANNA, channel_ids=str(STANDUP), consent_role_name=None, now=T0 + ) + state = await setup.request( + GUILD, requested_by=ANNA, channel_ids=str(RETRO), consent_role_name=None, now=T1 + ) + + assert len(await intents.pending_for(GUILD)) == 2 + assert state is not None + assert state.intent is not None + assert state.intent.channel_ids == str(RETRO) + + +async def test_the_state_shows_what_the_bot_wrote_back( + factory: async_sessionmaker[AsyncSession], +) -> None: + """The whole reason the console polls: the answer arrives on a tick.""" + setup = await a_setup(factory) + intents = SetupIntentStore(factory) + intent_id = await intents.request( + GUILD, requested_by=ANNA, channel_ids=str(STANDUP), consent_role_name=None, now=T0 + ) + await intents.record_outcome(intent_id, outcome=APPLIED, error=None, now=T1) + + state = await setup.state(GUILD, requested_by=ANNA) + + assert state is not None + assert state.intent is not None + assert state.intent.outcome == APPLIED + assert state.intent.applied_at == T1 + + +async def test_one_guilds_request_is_not_anothers( + factory: async_sessionmaker[AsyncSession], +) -> None: + setup = await a_setup(factory) + await setup.request( + GUILD, requested_by=ANNA, channel_ids=str(STANDUP), consent_role_name=None, now=T0 + ) + + assert await SetupIntentStore(factory).latest_for(OTHER_GUILD) is None diff --git a/tests/console/test_setup_routes.py b/tests/console/test_setup_routes.py new file mode 100644 index 0000000..1d1acd0 --- /dev/null +++ b/tests/console/test_setup_routes.py @@ -0,0 +1,582 @@ +"""Setting a guild up from the console, and the link that gets the bot there. + +What is pinned here is the shape of three responses and the rule about who +gets one: the id in the path never authorises anything, the id in the +cookie does, and a guild somebody does not administer is indistinguishable +from a guild that does not exist. + +The two things the console cannot render without are pinned hardest. +`bot.has_arrived` is what separates "this server has no voice channels" +from "the bot is not there yet" -- a channel picker that cannot tell them +apart sends somebody hunting for a bug that is not there. And `POST` and +`GET` answer the same shape, because there is nothing to wait on: the +request is a row, and the console polls until its status stops being +`pending`. + +What is *not* tested here is the contradiction rule. Which of two requests +a guild is configured from is settled where the guild is configured -- the +bot -- and is tested in `tests/domain/test_onboarding.py` and +`tests/infrastructure/discord/test_setup_apply.py`. This endpoint writes +every request down, deliberately: an administrator who asked twice asked +twice. +""" + +from __future__ import annotations + +from datetime import UTC, datetime, timedelta +from urllib.parse import parse_qs, urlparse + +from aiohttp import web +from aiohttp.test_utils import TestClient + +from sturnus.console.app import SESSION_COOKIE +from sturnus.console.ports import GuildSetupState +from sturnus.console.session import SessionCookie, SignedSession +from sturnus.domain.onboarding import APPLIED, FAILED, SUPERSEDED, SetupIntent +from tests.console.conftest import ( + ANNA, + BEN, + GUILD, + SECRET, + T0, + AiohttpClientFactory, + FakeAdmins, + FakeSetup, + build_test_api, +) + +_INVITE = "/api/invite" + +#: A snowflake past 2^53, where a JSON number loses its last digits. +BIG_CHANNEL = 386950399101370374 +OTHER_CHANNEL = 386950399101370375 + +CLIENT_ID = "1289374650912837465" + + +def token(discord_user_id: int = ANNA) -> str: + return SessionCookie(SECRET, timedelta(hours=12)).issue(SignedSession(discord_user_id), now=T0) + + +async def signed_in( + aiohttp_client: AiohttpClientFactory, app: web.Application, as_user: int = ANNA +) -> TestClient[web.Request, web.Application]: + client = await aiohttp_client(app) + client.session.cookie_jar.update_cookies({SESSION_COOKIE: token(as_user)}) + return client + + +def setup_url(guild_id: int | str = GUILD) -> str: + return f"/api/guilds/{guild_id}/setup" + + +def an_intent(**over: object) -> SetupIntent: + base: dict[str, object] = { + "id": 7, + "guild_id": GUILD, + "requested_by": ANNA, + "requested_at": T0, + "channel_ids": str(BIG_CHANNEL), + "consent_role_name": "Recorded", + "applied_at": None, + "outcome": None, + "error": None, + } + base.update(over) + return SetupIntent(**base) # type: ignore[arg-type] + + +def a_state(*, seen_at: datetime | None = T0, intent: SetupIntent | None = None) -> GuildSetupState: + return GuildSetupState(seen_at=seen_at, intent=intent) + + +# --------------------------------------------------------------------------- +# Who may ask +# --------------------------------------------------------------------------- + + +async def test_a_signed_out_visitor_may_not_read_a_guilds_setup( + aiohttp_client: AiohttpClientFactory, +) -> None: + client = await aiohttp_client(build_test_api()) + assert (await client.get(setup_url())).status == 401 + + +async def test_a_signed_out_visitor_may_not_write_one( + aiohttp_client: AiohttpClientFactory, +) -> None: + client = await aiohttp_client(build_test_api()) + response = await client.post(setup_url(), json={"channel_ids": [str(BIG_CHANNEL)]}) + assert response.status == 401 + + +async def test_a_guild_this_person_does_not_administer_answers_as_a_guild_that_is_not_there( + aiohttp_client: AiohttpClientFactory, +) -> None: + """404, never 403. + + Writing a setup request is an act on somebody else's server, and a 403 + would confirm to somebody just established as having no business with + that guild that it exists. + """ + client = await signed_in(aiohttp_client, build_test_api(setup=FakeSetup()), as_user=BEN) + assert (await client.get(setup_url())).status == 404 + + +async def test_a_guild_id_that_is_not_a_number_answers_the_same_way( + aiohttp_client: AiohttpClientFactory, +) -> None: + client = await signed_in(aiohttp_client, build_test_api(setup=FakeSetup(a_state()))) + assert (await client.get(setup_url("nonsense"))).status == 404 + + +async def test_the_signed_in_id_authorises_and_never_one_from_the_url( + aiohttp_client: AiohttpClientFactory, +) -> None: + """The id in the cookie is the only one that decides anything.""" + setup = FakeSetup(a_state()) + client = await signed_in(aiohttp_client, build_test_api(setup=setup), as_user=BEN) + + await client.get(setup_url()) + + assert setup.asked == [(GUILD, BEN)] + + +# --------------------------------------------------------------------------- +# Whether the bot is there at all +# --------------------------------------------------------------------------- + + +async def test_a_guild_the_bot_has_never_swept_says_so_outright( + aiohttp_client: AiohttpClientFactory, +) -> None: + """The field the channel picker depends on. + + An empty picker for a guild the bot has not joined reads exactly like + a server with no voice channels. One means "wait"; the other means + "look for the bug". The API must not make the console guess. + """ + setup = FakeSetup(a_state(seen_at=None)) + client = await signed_in(aiohttp_client, build_test_api(setup=setup)) + + body = await (await client.get(setup_url())).json() + + assert body["bot"] == {"has_arrived": False, "seen_at": None} + + +async def test_a_guild_the_bot_has_swept_carries_when_it_last_looked( + aiohttp_client: AiohttpClientFactory, +) -> None: + setup = FakeSetup(a_state(seen_at=T0)) + client = await signed_in(aiohttp_client, build_test_api(setup=setup)) + + body = await (await client.get(setup_url())).json() + + assert body["bot"] == {"has_arrived": True, "seen_at": T0.isoformat()} + + +# --------------------------------------------------------------------------- +# Reading a request back +# --------------------------------------------------------------------------- + + +async def test_a_guild_nobody_has_ever_asked_about_carries_no_request( + aiohttp_client: AiohttpClientFactory, +) -> None: + """Present and null, never absent: "nobody has asked" is an answer.""" + setup = FakeSetup(a_state(intent=None)) + client = await signed_in(aiohttp_client, build_test_api(setup=setup)) + + body = await (await client.get(setup_url())).json() + + assert body["guild_id"] == str(GUILD) + assert body["request"] is None + + +async def test_a_request_the_bot_has_not_reached_yet_is_pending( + aiohttp_client: AiohttpClientFactory, +) -> None: + setup = FakeSetup(a_state(intent=an_intent())) + client = await signed_in(aiohttp_client, build_test_api(setup=setup)) + + request = (await (await client.get(setup_url())).json())["request"] + + assert request == { + "id": "7", + "status": "pending", + "requested_by": str(ANNA), + "requested_at": T0.isoformat(), + "channel_ids": [str(BIG_CHANNEL)], + "consent_role_name": "Recorded", + "settled_at": None, + "error": None, + } + + +async def test_a_snowflake_never_reaches_the_client_as_a_number( + aiohttp_client: AiohttpClientFactory, +) -> None: + """Past 2^53 a JSON number silently loses its last digits, and produces + an id that looks right and names nothing.""" + setup = FakeSetup(a_state(intent=an_intent(channel_ids=str(BIG_CHANNEL)))) + client = await signed_in(aiohttp_client, build_test_api(setup=setup)) + + raw = await (await client.get(setup_url())).text() + + assert f'"{BIG_CHANNEL}"' in raw + assert str(BIG_CHANNEL) + "," not in raw.replace(f'"{BIG_CHANNEL}"', "") + + +async def test_a_request_the_bot_applied_says_when( + aiohttp_client: AiohttpClientFactory, +) -> None: + settled = T0 + timedelta(seconds=10) + setup = FakeSetup(a_state(intent=an_intent(applied_at=settled, outcome=APPLIED))) + client = await signed_in(aiohttp_client, build_test_api(setup=setup)) + + request = (await (await client.get(setup_url())).json())["request"] + + assert request["status"] == APPLIED + assert request["settled_at"] == settled.isoformat() + assert request["error"] is None + + +async def test_a_request_that_failed_carries_something_a_person_can_act_on( + aiohttp_client: AiohttpClientFactory, +) -> None: + """There is no retry to wait for: an attempt settles the intent either + way, so this text is the whole answer an administrator gets.""" + setup = FakeSetup( + a_state( + intent=an_intent( + applied_at=T0, + outcome=FAILED, + error="I am missing the Manage Roles permission.", + ) + ) + ) + client = await signed_in(aiohttp_client, build_test_api(setup=setup)) + + request = (await (await client.get(setup_url())).json())["request"] + + assert request["status"] == FAILED + assert "Manage Roles" in request["error"] + + +async def test_a_request_a_newer_one_replaced_says_superseded( + aiohttp_client: AiohttpClientFactory, +) -> None: + """So the console can say "somebody else asked after you" rather than + leaving a request that never applied and never failed.""" + setup = FakeSetup(a_state(intent=an_intent(applied_at=T0, outcome=SUPERSEDED))) + client = await signed_in(aiohttp_client, build_test_api(setup=setup)) + + request = (await (await client.get(setup_url())).json())["request"] + + assert request["status"] == SUPERSEDED + + +async def test_an_outcome_this_build_has_never_seen_is_rendered_rather_than_refused( + aiohttp_client: AiohttpClientFactory, +) -> None: + """`outcome` is text and not a database enum precisely so a value this + code does not know is a row a reader can ignore. An endpoint that + refused to render one would give that property back.""" + setup = FakeSetup(a_state(intent=an_intent(applied_at=T0, outcome="from_the_future"))) + client = await signed_in(aiohttp_client, build_test_api(setup=setup)) + + request = (await (await client.get(setup_url())).json())["request"] + + assert request["status"] == "from_the_future" + + +# --------------------------------------------------------------------------- +# Asking +# --------------------------------------------------------------------------- + + +async def test_a_request_is_written_in_the_spelling_the_bot_reads( + aiohttp_client: AiohttpClientFactory, +) -> None: + """Stored exactly as `guild_config` holds it, so applying an intent is + a write of the value rather than a second serialisation.""" + setup = FakeSetup(a_state()) + client = await signed_in(aiohttp_client, build_test_api(setup=setup, admins=FakeAdmins({ANNA}))) + + response = await client.post( + setup_url(), + json={"channel_ids": [str(BIG_CHANNEL), str(OTHER_CHANNEL)], "consent_role_name": "Rec"}, + ) + + assert response.status == 202 + guild_id, asked_by, channel_ids, role_name, _now = setup.written[0] + assert (guild_id, asked_by) == (GUILD, ANNA) + assert channel_ids == f"{BIG_CHANNEL},{OTHER_CHANNEL}" + assert role_name == "Rec" + + +async def test_asking_answers_with_the_same_shape_reading_does( + aiohttp_client: AiohttpClientFactory, +) -> None: + """There is nothing to wait for, so the answer is the guild's state. + + Under the rule that the newest ask wins, "what did I just ask for" and + "what will this guild be configured from" are the same question. + """ + setup = FakeSetup(a_state()) + client = await signed_in(aiohttp_client, build_test_api(setup=setup)) + + body = await (await client.post(setup_url(), json={"channel_ids": [str(BIG_CHANNEL)]})).json() + + assert body["guild_id"] == str(GUILD) + assert body["bot"]["has_arrived"] is True + assert body["request"]["status"] == "pending" + assert body["request"]["channel_ids"] == [str(BIG_CHANNEL)] + + +async def test_naming_no_role_is_allowed_and_means_keep_whatever_is_there( + aiohttp_client: AiohttpClientFactory, +) -> None: + """Omitting it must never be the destructive path (Spec 10.1).""" + setup = FakeSetup(a_state()) + client = await signed_in(aiohttp_client, build_test_api(setup=setup)) + + response = await client.post(setup_url(), json={"channel_ids": [str(BIG_CHANNEL)]}) + + assert response.status == 202 + assert setup.written[0][3] is None + + +async def test_a_guild_this_person_does_not_administer_cannot_be_asked_about( + aiohttp_client: AiohttpClientFactory, +) -> None: + setup = FakeSetup() + client = await signed_in(aiohttp_client, build_test_api(setup=setup)) + + response = await client.post(setup_url(), json={"channel_ids": [str(BIG_CHANNEL)]}) + + assert response.status == 404 + assert setup.written == [] + + +async def test_asking_twice_writes_twice(aiohttp_client: AiohttpClientFactory) -> None: + """An administrator who asked twice asked twice. + + Collapsing the two would lose who asked for which and when. Which one + the guild is configured from is the bot's decision, not this one's. + """ + setup = FakeSetup(a_state()) + client = await signed_in(aiohttp_client, build_test_api(setup=setup)) + + await client.post(setup_url(), json={"channel_ids": [str(BIG_CHANNEL)]}) + await client.post(setup_url(), json={"channel_ids": [str(OTHER_CHANNEL)]}) + + assert [written[2] for written in setup.written] == [ + str(BIG_CHANNEL), + str(OTHER_CHANNEL), + ] + + +# --------------------------------------------------------------------------- +# What a request may say +# --------------------------------------------------------------------------- + + +async def test_a_body_that_is_not_an_object_is_refused( + aiohttp_client: AiohttpClientFactory, +) -> None: + client = await signed_in(aiohttp_client, build_test_api(setup=FakeSetup(a_state()))) + assert (await client.post(setup_url(), json=["10"])).status == 400 + + +async def test_a_channel_list_of_numbers_is_refused_rather_than_coerced( + aiohttp_client: AiohttpClientFactory, +) -> None: + """A client that sent numbers has already lost the last digits of every + snowflake past 2^53. Accepting them would store a room nobody has.""" + setup = FakeSetup(a_state()) + client = await signed_in(aiohttp_client, build_test_api(setup=setup)) + + response = await client.post(setup_url(), json={"channel_ids": [BIG_CHANNEL]}) + + assert response.status == 400 + assert setup.written == [] + + +async def test_naming_no_channel_at_all_is_refused( + aiohttp_client: AiohttpClientFactory, +) -> None: + """ "Allowed to record nowhere" is what `/config clear` is for, and it + is not what somebody pressing a setup button meant.""" + setup = FakeSetup(a_state()) + client = await signed_in(aiohttp_client, build_test_api(setup=setup)) + + assert (await client.post(setup_url(), json={"channel_ids": []})).status == 400 + assert setup.written == [] + + +async def test_a_channel_named_twice_is_refused( + aiohttp_client: AiohttpClientFactory, +) -> None: + """Always a mistake, and silently collapsing it hides the mistake from + whoever made it -- `settings.parse_channel_ids`' rule, not a second + copy of it.""" + setup = FakeSetup(a_state()) + client = await signed_in(aiohttp_client, build_test_api(setup=setup)) + + response = await client.post( + setup_url(), json={"channel_ids": [str(BIG_CHANNEL), str(BIG_CHANNEL)]} + ) + + assert response.status == 400 + assert setup.written == [] + + +async def test_a_channel_id_that_is_not_a_snowflake_is_refused( + aiohttp_client: AiohttpClientFactory, +) -> None: + setup = FakeSetup(a_state()) + client = await signed_in(aiohttp_client, build_test_api(setup=setup)) + + assert (await client.post(setup_url(), json={"channel_ids": ["general"]})).status == 400 + assert setup.written == [] + + +async def test_a_blank_role_name_is_refused_rather_than_treated_as_absent( + aiohttp_client: AiohttpClientFactory, +) -> None: + """A role called nothing is not what anybody meant, and Discord would + refuse it a tick later anyway -- by which point the person has left + the page.""" + setup = FakeSetup(a_state()) + client = await signed_in(aiohttp_client, build_test_api(setup=setup)) + + response = await client.post( + setup_url(), json={"channel_ids": [str(BIG_CHANNEL)], "consent_role_name": " "} + ) + + assert response.status == 400 + assert setup.written == [] + + +async def test_a_role_name_longer_than_discord_allows_is_refused( + aiohttp_client: AiohttpClientFactory, +) -> None: + setup = FakeSetup(a_state()) + client = await signed_in(aiohttp_client, build_test_api(setup=setup)) + + response = await client.post( + setup_url(), json={"channel_ids": [str(BIG_CHANNEL)], "consent_role_name": "x" * 101} + ) + + assert response.status == 400 + assert setup.written == [] + + +async def test_a_role_name_is_trimmed_before_it_is_stored( + aiohttp_client: AiohttpClientFactory, +) -> None: + """A trailing space is a different role name in Discord, and nobody + typed one on purpose.""" + setup = FakeSetup(a_state()) + client = await signed_in(aiohttp_client, build_test_api(setup=setup)) + + await client.post( + setup_url(), json={"channel_ids": [str(BIG_CHANNEL)], "consent_role_name": " Rec "} + ) + + assert setup.written[0][3] == "Rec" + + +async def test_nothing_a_caller_typed_is_reflected_back_in_a_refusal( + aiohttp_client: AiohttpClientFactory, +) -> None: + """Fixed strings, the same rule the rest of `sturnus.console` follows. + + Echoing the value would make this an echo endpoint for anything an + administrator can type into a form. + """ + client = await signed_in(aiohttp_client, build_test_api(setup=FakeSetup(a_state()))) + + response = await client.post( + setup_url(), + json={"channel_ids": ["