From 962280242cb413c9e776343f8193b096fbd902fd Mon Sep 17 00:00:00 2001 From: Ty Tremblay Date: Thu, 20 Aug 2026 22:22:37 -0400 Subject: [PATCH] Manage user group membership from /hawkmod MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `/hawkmod group add|remove @user @group`, and `/hawkmod deactivate` without which the first command would be lying. Group membership and monitoring are not the same fact, and this is the change that forces the distinction into the open. Removing someone from @students leaves them a student on the roster — `reconcileRoles` is add-only, deliberately — so the command says so in its reply rather than letting the caller assume the roster followed. Ending monitoring is `/hawkmod deactivate`, which demands a person and a reason and is the only operation here that makes hawk-mod see less. CONTEXT.md names the two ideas so they stop sharing a word. The mirror of that asymmetry is new: a deactivated person who reappears in a role group is monitored again immediately, as a `reactivate` decision. Gaining protection never needs approval; losing it always does. Without this a student who rejoined the team next season would sit in @students, known to hawk-mod, and unmonitored — with coverage reporting 100%, because inactive people leave both sides of that ratio. Writes go out on an administrator's own token, not the bot's. Slack accepts a bot token for usergroups.users.update only when the workspace lets *everyone* edit user groups, which §6 forbids; the restriction is the point, so the token is what changes. That token gets its own installation row: Slack issues one token per authorization carrying only that authorization's scopes, so sharing a row would mean an administrator who is also an enrolled mentor losing their DM token the first time they authorized group editing. `saveInstallation` now refuses a `user` write whose scopes cannot read DMs, because nothing else in the system would ever have noticed. Edits go through a pure plan. usergroups.users.update replaces a group's entire member list, so a bad input does not corrupt a group, it empties one — and the eventual spreadsheet-driven sync is exactly the shape of input that fails that way quietly. The plan carries its own refusal so a caller cannot apply a bad one by forgetting a flag, and the single-user command is the degenerate case of that sync rather than a separate path. group_changes records who asked. role_changes cannot: it is written by the sync, which runs from a Slack event long after the human is gone. Requires `usergroups:write` as a *user* scope in the Slack app manifest. Bot scopes are unchanged, so no workspace reinstall. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 59 +++++++- CONTEXT.md | 142 ++++++++++++++++++ docker-compose.yml | 3 + docs/policy-mapping.md | 54 +++---- docs/slack-app-manifest.yaml | 20 ++- migrations/0006_group_membership.sql | 72 +++++++++ src/config.ts | 24 +++ src/db/repo.ts | 169 ++++++++++++++++++++- src/domain/rules/groupMembership.ts | 145 ++++++++++++++++++ src/domain/rules/rosterSync.ts | 47 ++++-- src/jobs/syncRoles.ts | 46 +++++- src/slack/app.ts | 96 +++++++++++- src/slack/commands.ts | 172 ++++++++++++++++++++- src/slack/groupAdmin.ts | 217 +++++++++++++++++++++++++++ src/slack/installStore.ts | 47 +++++- src/slack/userGroups.ts | 54 ++++++- test/groupMembership.test.ts | 182 ++++++++++++++++++++++ test/rosterSync.test.ts | 86 +++++++++++ 18 files changed, 1556 insertions(+), 79 deletions(-) create mode 100644 CONTEXT.md create mode 100644 migrations/0006_group_membership.sql create mode 100644 src/domain/rules/groupMembership.ts create mode 100644 src/slack/groupAdmin.ts create mode 100644 test/groupMembership.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index 644e0b3..fa8b0c6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -45,7 +45,8 @@ code that carries it, and records which controls are deliberately manual. ## Architecture -**Two token planes.** The bot token posts alerts and reads channel membership. +**Two token planes, and a third narrow one.** The bot token posts alerts and +reads channel membership. Each adult's _user_ token is what makes DMs visible at all — Slack exposes no other way below Enterprise Grid. `src/slack/installStore.ts` keeps one `bot` row per workspace and one `user` row per enrolled adult, and `fetchInstallation` @@ -54,6 +55,18 @@ stripped so one adult's token never rides along to a request that did not ask for it. Tokens are encrypted at rest (`src/crypto.ts`); the DB file alone is not enough to read anyone's DMs. +The third is an `admin` row: an administrator's token scoped to +`usergroups:write` and nothing else, granted at `/slack/authorize-groups`. It +exists because Slack accepts a _bot_ token for `usergroups.users.update` only +when the workspace lets everyone edit user groups — which §6 forbids — so group +edits go out as a named person. It is a separate row rather than extra scopes on +that person's `user` row because Slack issues one token per authorization +carrying only that authorization's scopes: sharing a row would mean an +administrator who is also an enrolled mentor losing their DM token the moment +they authorized group editing, with coverage still reading 100%. +`saveInstallation` refuses a `user` write whose scopes lack `im:history` for +exactly that reason. Never merge these two grants. + **Students may not enroll.** `storeInstallation` throws on a student's user token, and the sweep revokes and deletes one that appears later (a person can be moved into the students group after enrolling). Their token would expose @@ -88,12 +101,42 @@ policy that lands in a handler is policy nothing covers. in the sweep, because everything after it reads roles, and again on `subteam_*` events so an edit applies immediately rather than at 3am (the sync is idempotent, which is what makes the duplicate events harmless). The -reconciliation in `domain/rules/rosterSync.ts` is pure and **add-only by -design**: a person dropped from the students group stays a student, since the -alternative is silently ending someone's monitoring. Only an explicit move into -the adults group leaves `student`, and that raises `roster_drift`. Every change -lands in `role_changes` — Slack's audit log API is Grid-only, so that table is -the only trail. Do not make this bidirectional. +reconciliation in `domain/rules/rosterSync.ts` is pure and **may only ever add +monitoring, never subtract it**: a person dropped from the students group stays +a student, since the alternative is silently ending someone's monitoring. Only +an explicit move into the adults group leaves `student`, and that raises +`roster_drift`. Every change lands in `role_changes` — Slack's audit log API is +Grid-only, so that table is the only trail. Do not make this bidirectional. + +The invariant is about _direction_, not about writes, which is why `reactivate` +belongs there: a deactivated person who reappears in a role group is monitored +again immediately, for the same reason `create` needs nobody's approval. Ending +monitoring is `/hawkmod deactivate`, which demands a person and a reason — the +only operation in hawk-mod that makes it see less, and the only one no rule, job +or sync can reach. + +**Group membership is declaration; the roster is monitoring.** `/hawkmod group +add|remove` edits the Slack user group and nothing else. Removing someone from +`@students` leaves them a student on the roster, and the command says so in its +reply rather than letting the caller assume otherwise. `CONTEXT.md` keeps the +two words apart; conflating them is how a graduated student ends up monitored +forever, or a returning one ends up invisible. + +**Group edits go through a plan.** `domain/rules/groupMembership.ts` is pure and +diffs intended membership against actual, because `usergroups.users.update` +_replaces_ a group's whole member list — there is no add-one endpoint. So a bad +input does not corrupt a group, it empties one. The plan carries its own refusal +(over-large removals, emptying a group) so a caller cannot apply a bad one by +forgetting to check a flag elsewhere. `slack/groupAdmin.ts` serializes writes and +re-reads membership inside the lock. The single-user command is the degenerate +case of the spreadsheet-driven sync this was built for — one planner, two +callers. Editable groups are an allowlist (`MANAGED_USERGROUPS`), which is blast +radius rather than authorization: every caller is already a Slack admin who could +edit any group by hand. + +`group_changes` records who asked for an edit. `role_changes` cannot: it is +written by the sync, which runs from a Slack event long after the human is gone, +and most group edits will change no role at all once subteams are managed here. **Findings are the output, and have exactly one door each way.** `src/raise.ts` persists then alerts, and only alerts when the finding is new or has recurred. @@ -221,7 +264,7 @@ Building locally on macOS can trip the "access data from other apps" prompt; detectable without message text; only investigation needs the text. - Non-students with no screening on file do not count toward the two-adult rule, whatever their role. Do not loosen `isScreenedAdult`. -- **Every Slack entry point is gated on `mayAdministerWorkspace`**, and each one +- **Every Slack entry point is gated on `administrator()`**, and each one checks for itself: the slash command (`commands.ts`), the alert buttons and their note modal (`actions.ts`), and the screening and consent submissions (`modals.ts`). There is no middleware doing this centrally — anyone who can diff --git a/CONTEXT.md b/CONTEXT.md new file mode 100644 index 0000000..e23b703 --- /dev/null +++ b/CONTEXT.md @@ -0,0 +1,142 @@ +# hawk-mod + +Youth-protection auditing for a FIRST team's Slack workspace: it records +adult–student direct messages so a human can review them, because Slack cannot +prevent those messages below Enterprise Grid. + +This glossary exists because several words in this domain look like synonyms and +are not. Confusing any pair of them ends someone's monitoring quietly, which is +the failure this project is built to avoid. + +## People + +**Person**: +Someone the roster knows about, identified by their Slack account. +_Avoid_: User, member, account + +**Student**: +A person the roster records as a minor, whose direct messages with adults are +recorded. See **Declared** vs **Monitored** — the word alone is ambiguous. +_Avoid_: Kid, child, minor + +**Adult**: +Any person on the roster who is not a student, a district observer included. +Seniority is never an exemption. +_Avoid_: Mentor, coach, grown-up, leader + +**Screened adult**: +An adult with current Youth Protection Screening, Youth Protection Training, and +CORI on file. Only screened adults count toward the two-adult rule. + +**Administrator**: +Someone Slack records as a Workspace Owner or Admin. It is not a roster role and +is never stored — the roster says who is monitored, never who is in charge. +_Avoid_: Lead coach, admin role, superuser + +## Declaration vs monitoring + +These two are the distinction most easily lost, and the one that matters most. + +**Declared**: +Present in the Slack user group that names a role — `@students` or `@adults`. +Cheap, reversible, and edited by hand in Slack or through hawk-mod. A +declaration is a statement of intent, not a fact about monitoring. +_Avoid_: Enrolled, rostered, assigned + +**Monitored**: +Carried on the roster with a role and marked active. Sticky by design: gained +automatically when someone is declared, and lost only by an explicit, +attributed act. Removing a declaration never removes monitoring. +_Avoid_: Tracked, watched, covered + +**Active**: +The state of a person whose monitoring is in force. The opposite is +**deactivated** — a person the roster remembers but no longer monitors. + +**Deactivation**: +The deliberate act of ending a person's monitoring. The only operation in +hawk-mod that makes it see less, and therefore the only one that always names +who performed it and why. +_Avoid_: Removal, deletion, archiving, offboarding + +**Reactivation**: +Restoring monitoring to a deactivated person. Happens automatically when they +are declared again, because gaining protection never needs permission. + +## Enrollment + +**Enrollment**: +An adult's own authorization letting hawk-mod read their direct messages. It is +what makes DMs visible at all, and it is granted by that adult, never on their +behalf. Students may never enroll. +_Avoid_: Onboarding, signup, opting in, installation + +**Coverage**: +The proportion of adults requiring enrollment who have enrolled. An unenrolled +adult is a gap, and a gap is a finding rather than a silence. + +**Group-editing authorization**: +An administrator's separate authorization letting hawk-mod edit Slack user +groups as them. Distinct from enrollment in purpose, lifetime, and consent, and +held separately so that neither can revoke the other. + +## Conversations + +**Conversation**: +A Slack direct message or group direct message that hawk-mod has classified. +Classification depends only on who is present, never on what was said. + +**Verdict**: +What the rules conclude about a conversation from its participants alone. + +**Two-adult rule**: +The requirement that a student's conversation include at least two screened +adults. Unknown accounts never satisfy it. + +**Remedy**: +Moving a one-to-one conversation into a channel or adding a second adult. A +remedy acknowledges a finding; it never resolves one, because the one-to-one +still happened. +_Avoid_: Fix, resolution, correction + +## Findings + +**Finding**: +A recorded policy problem. Findings name people and conversations, never +message content. +_Avoid_: Alert, violation, issue, incident + +**Condition finding**: +A finding describing something currently true, such as lapsed screening. +Re-detecting one is not news. + +**Occurrence finding**: +A finding describing something that happened, such as a one-to-one message. Only +a newer event can repeat it. + +**Dedupe key**: +The identity of a problem across sweeps. It is what separates an alert channel +someone reads from one nobody does. + +**Guidance**: +An advisory note sent privately to the adults in a conversation that raised a +finding. It never reaches the student, and it never replaces the finding. + +## Reconciliation + +**Sweep**: +The scheduled pass that re-checks conditions and closes what it owns. + +**Backfill**: +The hourly re-walk of enrolled adults' message history, catching what predates +enrollment or was missed while the process was down. + +**Plan**: +The set of additions and removals that would bring a user group to an intended +membership. A plan is inspectable before it is applied, and is refused outright +when it would remove too much of a group at once. +_Avoid_: Diff, changeset, patch + +**Drift**: +A disagreement between what Slack declares and what the roster monitors. Drift +is reported, never silently reconciled in the direction that reduces monitoring. diff --git a/docker-compose.yml b/docker-compose.yml index 45613f3..a6e42bc 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -46,6 +46,9 @@ services: # is told to, and a missing one silently reads as "feature off". STUDENT_USERGROUP: ${STUDENT_USERGROUP:-} ADULT_USERGROUP: ${ADULT_USERGROUP:-} + # Further group handles `/hawkmod group` may edit, comma separated. The + # two above are always editable; this bounds how far a bad plan reaches. + MANAGED_USERGROUPS: ${MANAGED_USERGROUPS:-} # --- Schedules ------------------------------------------------------- TZ: ${TZ:-America/New_York} diff --git a/docs/policy-mapping.md b/docs/policy-mapping.md index 10ec6cb..e9da3a5 100644 --- a/docs/policy-mapping.md +++ b/docs/policy-mapping.md @@ -5,33 +5,33 @@ Each control from and what carries it. "Manual" means hawk-mod cannot do it and does not pretend to. -| § | Control | Carried by | -| --- | ----------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| 2 | Parental consent on file before a student account exists | `consent.ts`; `team_join` event raises `unconsented_account` the moment an account appears; nightly sweep re-checks | -| 2 | Consent re-collected annually | `CONSENT_VALID_YEARS = 1`; consents expire rather than linger | -| 2 | Consents kept on file and producible | `consents` table records `document_ref`; the signed copies themselves live wherever the team files them — **manual** | -| 2 | Parents notified of the PII collected and shared (Slack CSS §IV) | [`consent-form.md`](consent-form.md) — **manual**. The form is the artifact; hawk-mod records only that a version of it was signed (`form_version`) | -| 2 | Consent withdrawable on request | `revokeConsent()` exists in `repo.ts` but **has no caller** — no CLI, no modal. The form promises this; the tool cannot yet do it | -| 3 | Two YPP-screened Lead Coaches | `workspace_config` `screened_admins`: two of the workspace's Owners/Admins must be screened adults on the roster, plus `screening_lapsed` findings | -| 3 | Written communications copied to a second adult | `dmPolicy` — two screened adults required in any student conversation | -| 4.1 | No 1:1 adult–student DMs, ever | `dmPolicy` `one_to_one_adult_student`, raised on each new message and on backfill; a message after a finding is closed raises it again (`recurrence.ts`) | -| 4.2 | Two screened adults in every channel students are in | `twoAdults.ts`; re-evaluated on every join/leave, plus nightly | -| 4.3 | Students and parents told DMs are subject to audit | [`consent-form.md`](consent-form.md) — **manual**, and a precondition of deploying this. Both halves: the guardian signature block and the student acknowledgment below it | -| 4.4 | Quarterly export actually run and spot-checked | continuous instead: DMs are recorded as they happen. The quarterly reminder and runbook keep the human review honest | -| 5 | Business+ / Corporate Export | procurement — **manual**. Note hawk-mod does not depend on Corporate Export; it is the backstop for adults who never enroll | -| 6 | Retention | append-only while retained: deletions are tombstoned, edits keep prior text. No longer "keep everything" — `consent-form.md` promises deletion two years after a student's last day, and **nothing purges yet** | -| 6 | Slack Connect external DMs disabled | workspace setting, not API-readable — **manual** | -| 6 | Invites restricted to Owners/Admins | workspace setting — **manual**; `unknown_account` catches the consequence | -| 6 | Two Workspace Owners minimum, never a student | `workspace_config` findings | -| 6 | User group editing restricted to Owners/Admins | workspace setting — **manual**. Only matters when `STUDENT_USERGROUP`/`ADULT_USERGROUP` are set, since group membership then declares who is monitored | -| 6 | Real names enforced | workspace setting — **manual** | -| 6 | Huddles off | no API to observe huddles — **manual**, and the reason it matters is in the README's gap list | -| 7 | Youth Protection Training annually (the part FIRST requires for clearance) | `screening.ts`, `YPT_VALID_YEARS = 1` | -| 7 | CORI + national fingerprints every 3 years (M.G.L. c. 71 §38R, 603 CMR 51.00) | `screening.ts`, `CORI_VALID_YEARS = 3` | -| 8 | MPS administrator added to the workspace | roster role `district_observer`; counts as an adult only with screening dates recorded. Voluntary — IJNDD does not require it (see the correction below) | -| 8 | Employee-mentor keeps to channels, no student DMs (IJNDD clause k) | **manual**, and deliberately so: the employee manual binds that one person more tightly than §4.1 does, and hawk-mod is not the enforcer of it | -| 8 | Public-records retention for the employee-mentor (M.G.L. c. 66 §10) | substantively covered — messages are retained and producible via `export-conversation`; IJNDD's forward-to-school-e-mail expectation is **manual** | -| 8 | Written approval from the principal before launch | **manual**, and blocking | +| § | Control | Carried by | +| --- | ----------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 2 | Parental consent on file before a student account exists | `consent.ts`; `team_join` event raises `unconsented_account` the moment an account appears; nightly sweep re-checks | +| 2 | Consent re-collected annually | `CONSENT_VALID_YEARS = 1`; consents expire rather than linger | +| 2 | Consents kept on file and producible | `consents` table records `document_ref`; the signed copies themselves live wherever the team files them — **manual** | +| 2 | Parents notified of the PII collected and shared (Slack CSS §IV) | [`consent-form.md`](consent-form.md) — **manual**. The form is the artifact; hawk-mod records only that a version of it was signed (`form_version`) | +| 2 | Consent withdrawable on request | `revokeConsent()` exists in `repo.ts` but **has no caller** — no CLI, no modal. The form promises this; the tool cannot yet do it | +| 3 | Two YPP-screened Lead Coaches | `workspace_config` `screened_admins`: two of the workspace's Owners/Admins must be screened adults on the roster, plus `screening_lapsed` findings | +| 3 | Written communications copied to a second adult | `dmPolicy` — two screened adults required in any student conversation | +| 4.1 | No 1:1 adult–student DMs, ever | `dmPolicy` `one_to_one_adult_student`, raised on each new message and on backfill; a message after a finding is closed raises it again (`recurrence.ts`) | +| 4.2 | Two screened adults in every channel students are in | `twoAdults.ts`; re-evaluated on every join/leave, plus nightly | +| 4.3 | Students and parents told DMs are subject to audit | [`consent-form.md`](consent-form.md) — **manual**, and a precondition of deploying this. Both halves: the guardian signature block and the student acknowledgment below it | +| 4.4 | Quarterly export actually run and spot-checked | continuous instead: DMs are recorded as they happen. The quarterly reminder and runbook keep the human review honest | +| 5 | Business+ / Corporate Export | procurement — **manual**. Note hawk-mod does not depend on Corporate Export; it is the backstop for adults who never enroll | +| 6 | Retention | append-only while retained: deletions are tombstoned, edits keep prior text. No longer "keep everything" — `consent-form.md` promises deletion two years after a student's last day, and **nothing purges yet** | +| 6 | Slack Connect external DMs disabled | workspace setting, not API-readable — **manual** | +| 6 | Invites restricted to Owners/Admins | workspace setting — **manual**; `unknown_account` catches the consequence | +| 6 | Two Workspace Owners minimum, never a student | `workspace_config` findings | +| 6 | User group editing restricted to Owners/Admins | workspace setting — **manual**, and now load-bearing for more than itself: Slack accepts a _bot_ token for `usergroups.users.update` only when editing is open to everyone, so keeping this restricted is what forces `slack/groupAdmin.ts` to write as a named administrator. `/hawkmod group` is gated on `slack/authz.ts` — the same Owner/Admin population, plus a refusal for students | +| 6 | Real names enforced | workspace setting — **manual** | +| 6 | Huddles off | no API to observe huddles — **manual**, and the reason it matters is in the README's gap list | +| 7 | Youth Protection Training annually (the part FIRST requires for clearance) | `screening.ts`, `YPT_VALID_YEARS = 1` | +| 7 | CORI + national fingerprints every 3 years (M.G.L. c. 71 §38R, 603 CMR 51.00) | `screening.ts`, `CORI_VALID_YEARS = 3` | +| 8 | MPS administrator added to the workspace | roster role `district_observer`; counts as an adult only with screening dates recorded. Voluntary — IJNDD does not require it (see the correction below) | +| 8 | Employee-mentor keeps to channels, no student DMs (IJNDD clause k) | **manual**, and deliberately so: the employee manual binds that one person more tightly than §4.1 does, and hawk-mod is not the enforcer of it | +| 8 | Public-records retention for the employee-mentor (M.G.L. c. 66 §10) | substantively covered — messages are retained and producible via `export-conversation`; IJNDD's forward-to-school-e-mail expectation is **manual** | +| 8 | Written approval from the principal before launch | **manual**, and blocking | ## Corrections worth keeping straight diff --git a/docs/slack-app-manifest.yaml b/docs/slack-app-manifest.yaml index dae3d14..a8d00a1 100644 --- a/docs/slack-app-manifest.yaml +++ b/docs/slack-app-manifest.yaml @@ -22,7 +22,7 @@ features: - command: /hawkmod url: https://hawk-mod.example.org/slack/events description: Coverage, findings, and audits - usage_hint: status | findings | whois @user | resolve + usage_hint: status | findings | whois @user | group add @user @group should_escape: true oauth_config: @@ -42,14 +42,28 @@ oauth_config: - users:read - users:read.email - usergroups:read - # Granted per adult, by that adult. This is the only way to see DMs - # without Enterprise Grid. + # Two separate per-person grants live in this one list, and Slack has no + # way to say so: an authorization asks for whichever subset it needs. + # + # im:*/mpim:* — enrolment. Granted per adult, by that adult, at + # /slack/install. The only way to see DMs without + # Enterprise Grid. + # usergroups:write + # — group editing. Granted per administrator at + # /slack/authorize-groups, and asked for on its own so + # nobody hands over their DMs to manage the roster. A bot + # token cannot do this while user group editing is + # restricted to Owners/Admins, which §6 requires it to be. + # + # Both must be listed here or the matching authorization link is rejected + # by Slack before anyone sees a consent screen. user: - im:read - im:history - mpim:read - mpim:history - users:read + - usergroups:write settings: interactivity: diff --git a/migrations/0006_group_membership.sql b/migrations/0006_group_membership.sql new file mode 100644 index 0000000..6c1b199 --- /dev/null +++ b/migrations/0006_group_membership.sql @@ -0,0 +1,72 @@ +-- Group membership management: hawk-mod can now edit the Slack user groups it +-- reads, and needs somewhere to record who asked for each edit. +-- +-- Two changes, for two reasons. + +-- 1. A third kind of installation row. +-- +-- Slack refuses a *bot* token for usergroups.users.update unless the workspace +-- lets everyone edit user groups — which §6 forbids. So group edits ride on an +-- administrator's own user token, granted at a separate authorization that asks +-- for `usergroups:write` and nothing else. +-- +-- That token cannot share a row with an enrolled adult's DM token. Slack issues +-- one token per authorization carrying only the scopes granted at that moment, +-- and `installations` is keyed on (team, kind, slack_user_id) with an upsert — +-- so an administrator who is also an enrolled mentor would have their DM token +-- overwritten by their group-editing token, silently ending their monitoring +-- while /hawkmod status still reported them enrolled. A separate `kind` keeps +-- the two grants in separate rows with independent lifetimes. +-- +-- SQLite cannot alter a CHECK constraint, so the table is rebuilt. This is safe +-- here in a way it was not in 0005: nothing references `installations`, so +-- there are no cascades to fire and no rows anywhere else to lose. + +CREATE TABLE installations_new ( + id INTEGER PRIMARY KEY, + team_id TEXT NOT NULL, + enterprise_id TEXT, + slack_user_id TEXT NOT NULL, + kind TEXT NOT NULL CHECK (kind IN ('bot','user','admin')), + payload_enc TEXT NOT NULL, -- encrypted JSON Installation + scopes TEXT, + installed_at TEXT NOT NULL, + updated_at TEXT NOT NULL, + revoked_at TEXT, + UNIQUE (team_id, kind, slack_user_id) +); + +INSERT INTO installations_new + (id, team_id, enterprise_id, slack_user_id, kind, payload_enc, scopes, + installed_at, updated_at, revoked_at) +SELECT id, team_id, enterprise_id, slack_user_id, kind, payload_enc, scopes, + installed_at, updated_at, revoked_at +FROM installations; + +DROP TABLE installations; +ALTER TABLE installations_new RENAME TO installations; + +-- 2. The trail of who asked. +-- +-- A group edit and its effect on monitoring are different facts, and they need +-- different records. `role_changes` says what happened to someone's monitoring; +-- it is written by the user-group sync, which runs from a Slack event and has +-- no idea a human was involved. This table says who asked for what. +-- +-- It is also the only record of edits that change no role at all — which will +-- be most of them once subteams (@programming, @drive-team) are managed here. +CREATE TABLE group_changes ( + id INTEGER PRIMARY KEY, + usergroup_id TEXT NOT NULL, + handle TEXT NOT NULL, + action TEXT NOT NULL CHECK (action IN ('add','remove')), + subject TEXT NOT NULL, -- Slack id of the person added or removed + person_id INTEGER REFERENCES people(id) ON DELETE SET NULL, + actor TEXT NOT NULL, -- Slack id of the administrator who asked + actor_name TEXT NOT NULL, + reason TEXT, -- required when the edit reduces monitoring + source TEXT NOT NULL, -- 'command' | 'sheet_sync' + changed_at TEXT NOT NULL +); +CREATE INDEX group_changes_group_idx ON group_changes (usergroup_id, changed_at); +CREATE INDEX group_changes_subject_idx ON group_changes (subject, changed_at); diff --git a/src/config.ts b/src/config.ts index fac8fa5..3526f18 100644 --- a/src/config.ts +++ b/src/config.ts @@ -15,6 +15,16 @@ const schema = z.object({ // the roster is maintained purely by CSV import. STUDENT_USERGROUP: z.string().optional(), ADULT_USERGROUP: z.string().optional(), + // Further user group handles hawk-mod may edit, comma separated. The two + // role groups above are always editable; this widens the allowlist to + // subteams (@programming, @drive-team) without widening it to everything. + // + // The allowlist is not authorization — every caller is already a Workspace + // Owner or Admin who could edit any group in Slack's own UI. It is blast + // radius. `usergroups.users.update` replaces a group's entire membership, so + // a bad plan does not corrupt a group, it empties one, and this bounds how + // many groups a single bug can reach. + MANAGED_USERGROUPS: z.string().optional(), TZ: z.string().default("America/New_York"), SWEEP_CRON: z.string().default("0 3 * * *"), BACKFILL_CRON: z.string().default("15 * * * *"), @@ -52,3 +62,17 @@ export function dataDir(): string { export function logMode(): "full" | "metadata" { return process.env.LOG_MODE === "metadata" ? "metadata" : "full"; } + +/** Handles hawk-mod is permitted to edit, lowercased and without the `@`. */ +export function managedGroupHandles(): Set { + const cfg = config(); + const extra = (cfg.MANAGED_USERGROUPS ?? "") + .split(",") + .map((h) => h.trim()) + .filter(Boolean); + return new Set( + [cfg.STUDENT_USERGROUP, cfg.ADULT_USERGROUP, ...extra] + .filter((h): h is string => Boolean(h)) + .map((h) => h.replace(/^@/, "").toLowerCase()) + ); +} diff --git a/src/db/repo.ts b/src/db/repo.ts index 6f0931f..519cc06 100644 --- a/src/db/repo.ts +++ b/src/db/repo.ts @@ -142,6 +142,55 @@ export function setPersonRole(args: { })(); } +/** + * Starts or stops monitoring a person, and records who decided and why. + * + * Deactivation is the only operation in hawk-mod that makes it see less. There + * is no sweep, no sync and no rule that reaches it — a person does, by name, + * with a reason, which is why both are required rather than optional. + * + * The trail goes in `role_changes` with `from_role` and `to_role` equal. That + * reads oddly until you remember what the table is for: it is not a log of role + * strings, it is the answer to "who was monitored, when" — the only such answer + * that exists below Enterprise Grid. A deactivation changes that answer, so it + * belongs in the same place as the changes that alter someone's role. + */ +export function setPersonActive(args: { + personId: number; + active: boolean; + source: string; + actor: string; + reason: string; +}): void { + const now = nowIso(); + const person = personById(args.personId); + if (!person) throw new Error(`No person ${args.personId}`); + db().transaction(() => { + db() + .prepare("UPDATE people SET active = ?, updated_at = ? WHERE id = ?") + .run(args.active ? 1 : 0, now, args.personId); + db() + .prepare( + `INSERT INTO role_changes (person_id, slack_user_id, from_role, to_role, + source, detail, changed_at) + VALUES (?, ?, ?, ?, ?, ?, ?)` + ) + .run( + args.personId, + person.slack_user_id, + person.role, + person.role, + args.source, + JSON.stringify({ + active: args.active, + actor: args.actor, + reason: args.reason, + }), + now + ); + })(); +} + /** * Creates a roster row for a Slack account that is in a user group but was * never imported. Email may be absent, so the row is keyed on the Slack id — @@ -324,13 +373,89 @@ export function revokeConsent(consentId: number, on: string): void { .run(on, consentId); } +/* --------------------------------------------------------- group changes */ + +export type GroupChangeInput = { + usergroupId: string; + handle: string; + action: "add" | "remove"; + subject: string; + personId: number | null; + actor: string; + actorName: string; + reason: string | null; + source: string; +}; + +/** + * Records that somebody edited a user group. + * + * Separate from `role_changes` on purpose. That table is written by the + * user-group sync, which runs from a Slack event and cannot know a human was + * involved — by the time it fires, whoever ran the command is long gone. This + * one answers "who asked for this", and it is the only record of edits that + * change nobody's role at all, which most of them will be once subteams are + * managed here too. + */ +export function insertGroupChange(input: GroupChangeInput): void { + db() + .prepare( + `INSERT INTO group_changes (usergroup_id, handle, action, subject, + person_id, actor, actor_name, reason, + source, changed_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)` + ) + .run( + input.usergroupId, + input.handle, + input.action, + input.subject, + input.personId, + input.actor, + input.actorName, + input.reason, + input.source, + nowIso() + ); +} + +export function listGroupChanges(limit = 100) { + return db() + .prepare( + `SELECT * FROM group_changes ORDER BY changed_at DESC, id DESC LIMIT ?` + ) + .all(limit) as Array<{ + id: number; + usergroup_id: string; + handle: string; + action: "add" | "remove"; + subject: string; + person_id: number | null; + actor: string; + actor_name: string; + reason: string | null; + source: string; + changed_at: string; + }>; +} + /* ----------------------------------------------------------- installations */ +/** + * 'bot' — one per workspace; posts alerts and reads membership. + * 'user' — one per enrolled adult; their DM-reading token. + * 'admin' — one per administrator who authorized group editing. Separate from + * 'user' because Slack issues one token per authorization carrying + * only that authorization's scopes, so sharing a row would mean the + * second grant destroying the first. + */ +export type InstallationKind = "bot" | "user" | "admin"; + export type StoredInstallation = { teamId: string; enterpriseId: string | null; slackUserId: string; - kind: "bot" | "user"; + kind: InstallationKind; payload: Record; scopes: string | null; installedAt: string; @@ -341,7 +466,7 @@ type InstallationRow = { team_id: string; enterprise_id: string | null; slack_user_id: string; - kind: "bot" | "user"; + kind: InstallationKind; payload_enc: string; scopes: string | null; installed_at: string; @@ -351,14 +476,46 @@ type InstallationRow = { /** Bot installations share one row per team; '-' keeps the UNIQUE index honest. */ export const BOT_USER_KEY = "-"; +/** + * Scopes an enrolled adult's token must carry to be worth storing. Without + * these hawk-mod cannot read their DMs, which is the entire reason the row + * exists. + */ +const DM_SCOPES = ["im:history", "mpim:history"]; + +/** + * Thrown rather than swallowed: a caller about to destroy DM monitoring should + * fail loudly at the authorization, not succeed and go quiet for a month. + */ +export class ScopeDowngradeError extends Error {} + export function saveInstallation(args: { teamId: string; enterpriseId?: string | null; slackUserId: string; - kind: "bot" | "user"; + kind: InstallationKind; payload: unknown; scopes?: string | null; }): void { + // Belt and braces on the one failure nobody would notice. A 'user' row is + // what makes an adult's DMs visible; overwriting it with a token that cannot + // read DMs would end their monitoring while /hawkmod status still counted + // them as enrolled — an absent alert, not a wrong one. The routing in + // installStore should mean this never fires; it exists because if the + // routing ever breaks, nothing else in the system would tell us. + if (args.kind === "user") { + const granted = (args.scopes ?? "").split(",").filter(Boolean); + const missing = DM_SCOPES.filter((s) => !granted.includes(s)); + const existing = getInstallation(args.teamId, "user", args.slackUserId); + if (missing.length && existing && !existing.revokedAt) { + throw new ScopeDowngradeError( + `Refusing to replace ${args.slackUserId}'s enrolment token with one ` + + `missing ${missing.join(", ")}; that would silently end their DM ` + + `monitoring.` + ); + } + } + const now = nowIso(); db() .prepare( @@ -399,7 +556,7 @@ function hydrate(row: InstallationRow): StoredInstallation { export function getInstallation( teamId: string, - kind: "bot" | "user", + kind: InstallationKind, slackUserId: string ): StoredInstallation | undefined { const row = db() @@ -432,7 +589,7 @@ export function listUserInstallations( export function markInstallationRevoked( teamId: string, - kind: "bot" | "user", + kind: InstallationKind, slackUserId: string ): void { db() @@ -446,7 +603,7 @@ export function markInstallationRevoked( export function deleteInstallation( teamId: string, - kind: "bot" | "user", + kind: InstallationKind, slackUserId: string ): void { db() diff --git a/src/domain/rules/groupMembership.ts b/src/domain/rules/groupMembership.ts new file mode 100644 index 0000000..9c22628 --- /dev/null +++ b/src/domain/rules/groupMembership.ts @@ -0,0 +1,145 @@ +/** + * Plans an edit to a Slack user group's membership. + * + * Slack sells no add-one/remove-one endpoint: `usergroups.users.update` + * *replaces* the entire member list. So every edit is really "here is the + * complete new membership", and the interesting question is what a caller is + * about to overwrite. This module answers that question purely, from two sets, + * so the answer can be shown to a human before anything is applied. + * + * Adding one person to a group is the degenerate case — `desired` is `current` + * plus one — which is why the single-user command and the eventual + * spreadsheet-driven sync share this and not just a naming convention. + */ + +export type GroupPlan = { + /** Slack ids to be added, sorted. */ + add: string[]; + /** Slack ids to be removed, sorted. */ + remove: string[]; + /** Slack ids already correct, sorted. */ + unchanged: string[]; + /** The full membership to send to Slack if this plan is applied, sorted. */ + result: string[]; + /** + * Why this plan must not be applied, or `null` if it may be. A refusal is a + * property of the plan itself, so a caller cannot apply one by forgetting to + * check a separate flag. + */ + refusal: string | null; +}; + +export type PlanLimits = { + /** + * The largest share of a group a single plan may remove, as a fraction of + * current membership. Removing exactly one person is always allowed however + * small the group, since that is a person deliberately naming a person. + */ + maxRemovedFraction: number; +}; + +export const DEFAULT_LIMITS: PlanLimits = { maxRemovedFraction: 0.25 }; + +/** + * Diffs intended membership against actual, and refuses plans that look like + * an accident rather than an intention. + * + * The refusal exists because of what is on the other end of this: a Google + * Sheet. A shifted header row, a filtered view someone forgot to clear, a + * column of blanks — none of these read as errors, they read as "the roster is + * now these four people", and applying that empties @students. A bulk plan that + * removes most of a group is indistinguishable from a bad spreadsheet, so it is + * refused and shown to a human rather than applied and reported. + */ +export function planGroupMembership( + current: ReadonlySet, + desired: ReadonlySet, + limits: PlanLimits = DEFAULT_LIMITS +): GroupPlan { + const add = [...desired].filter((id) => !current.has(id)).sort(); + const remove = [...current].filter((id) => !desired.has(id)).sort(); + const unchanged = [...current].filter((id) => desired.has(id)).sort(); + const result = [...desired].sort(); + + return { + add, + remove, + unchanged, + result, + refusal: refuse(current, result, remove, limits), + }; +} + +function refuse( + current: ReadonlySet, + result: string[], + remove: string[], + limits: PlanLimits +): string | null { + if (result.length === 0) { + // Slack rejects an empty `users` list outright (`no_users_provided`) — a + // user group must keep at least one member. Saying so here turns an opaque + // API error into a sentence that explains the situation. + return current.size === 0 + ? "That group is empty and Slack will not accept an empty membership." + : "Slack does not allow a user group to be emptied. Remove the group " + + "itself in Slack if it is finished with."; + } + + if (remove.length <= 1 || current.size === 0) return null; + + const fraction = remove.length / current.size; + if (fraction > limits.maxRemovedFraction) { + return ( + `This would remove ${remove.length} of ${current.size} members ` + + `(${Math.round(fraction * 100)}%), which is more than a plan is allowed ` + + `to remove at once. Check the source of this change before applying it.` + ); + } + + return null; +} + +/** The plan for adding one person — what the slash command builds. */ +export function planAdd( + current: ReadonlySet, + slackId: string, + limits?: PlanLimits +): GroupPlan { + return planGroupMembership(current, new Set([...current, slackId]), limits); +} + +/** The plan for removing one person — what the slash command builds. */ +export function planRemove( + current: ReadonlySet, + slackId: string, + limits?: PlanLimits +): GroupPlan { + const desired = new Set(current); + desired.delete(slackId); + return planGroupMembership(current, desired, limits); +} + +/** + * Whether an edit ends someone's monitoring as a student, and so must carry a + * written reason. + * + * Pure, and keyed on the group's *handle* rather than whatever the caller + * typed. Slack sends an escaped mention as ``, so a + * command handler comparing the raw argument to a configured handle compares an + * opaque id to a word and quietly never matches — which would mean the one gate + * standing in front of the most consequential edit never fires. + */ +export function reducesMonitoring(args: { + action: "add" | "remove"; + subjectRole: string; + handle: string; + adultHandle: string; +}): boolean { + return ( + args.action === "add" && + args.subjectRole === "student" && + args.handle.replace(/^@/, "").toLowerCase() === + args.adultHandle.replace(/^@/, "").toLowerCase() + ); +} diff --git a/src/domain/rules/rosterSync.ts b/src/domain/rules/rosterSync.ts index 05644b8..c31e31a 100644 --- a/src/domain/rules/rosterSync.ts +++ b/src/domain/rules/rosterSync.ts @@ -11,6 +11,12 @@ export type RoleDecision = | { kind: "unchanged"; slackId: string } /** In a group but not on the roster — the row gets created from Slack. */ | { kind: "create"; slackId: string; role: Role } + /** + * Deactivated, but declared again by a group. Monitoring resumes: someone + * back in @students is a student again, and requiring a human to confirm + * that would leave a real student unmonitored until somebody got round to it. + */ + | { kind: "reactivate"; personId: number; slackId: string; role: Role } | { kind: "change"; personId: number; @@ -19,6 +25,8 @@ export type RoleDecision = to: Role; /** Moving out of `student` removes someone from monitoring. */ reducesProtection: boolean; + /** Deactivated when the group declared them; monitoring resumes too. */ + reactivates: boolean; } /** In both groups at once; too ambiguous to act on. */ | { kind: "conflict"; slackId: string; personId: number | null }; @@ -32,12 +40,18 @@ function isAdultRole(role: Role): boolean { /** * Reconciles Slack user groups against roster roles. * - * Deliberately one-directional in the safe sense: **membership of a group can - * only ever be added to the roster, never subtracted from it.** Someone dropped - * from the students group is left a student, because the alternative — silently - * ending their monitoring — is the failure this whole system exists to avoid. - * The only way out of `student` is an explicit move into the adults group, - * which is a deliberate admin action and is reported as one. + * Deliberately one-directional in the safe sense: **this reconciliation may only + * ever add monitoring, never subtract it.** Someone dropped from the students + * group is left a student, because the alternative — silently ending their + * monitoring — is the failure this whole system exists to avoid. The only way + * out of `student` is an explicit move into the adults group, which is a + * deliberate admin action and is reported as one. + * + * That invariant is about direction, not about writes. `reactivate` adds + * monitoring back to someone who was deactivated and has since been declared + * again, so it belongs here for the same reason `create` does: gaining + * protection never needs anyone's permission. Losing it always does — which is + * why there is no decision here that deactivates anybody. * * Pure: the caller does the Slack reads and the writes. */ @@ -71,13 +85,21 @@ export function reconcileRoles( // A district observer in the adults group is agreement, not a demotion to // plain `adult`. - if (target === "adult" && isAdultRole(person.role)) { - decisions.push({ kind: "unchanged", slackId }); - continue; - } + const roleAlreadyRight = + person.role === target || + (target === "adult" && isAdultRole(person.role)); - if (person.role === target) { - decisions.push({ kind: "unchanged", slackId }); + if (roleAlreadyRight) { + decisions.push( + person.active === 1 + ? { kind: "unchanged", slackId } + : { + kind: "reactivate", + personId: person.id, + slackId, + role: person.role, + } + ); continue; } @@ -88,6 +110,7 @@ export function reconcileRoles( from: person.role, to: target, reducesProtection: person.role === "student" && target !== "student", + reactivates: person.active !== 1, }); } diff --git a/src/jobs/syncRoles.ts b/src/jobs/syncRoles.ts index 45fdbfd..bf198f5 100644 --- a/src/jobs/syncRoles.ts +++ b/src/jobs/syncRoles.ts @@ -4,6 +4,7 @@ import { createPersonFromSlack, peopleBySlackId, personBySlackId, + setPersonActive, setPersonRole, } from "../db/repo.js"; import { dedupeKey } from "../domain/findings.js"; @@ -18,6 +19,7 @@ export type RoleSyncStats = { adultsInGroup: number; created: number; changed: number; + reactivated: number; conflicts: number; reducedProtection: number; missingGroups: number; @@ -28,15 +30,21 @@ const SOURCE = "usergroup_sync"; /** * Pulls role declarations from Slack user groups into the roster. * - * Membership is only ever *added*. Dropping someone from the students group + * Monitoring is only ever *added*. Dropping someone from the students group * does not un-student them — that would silently end their monitoring, which is * the failure this whole system exists to prevent. Moving out of `student` * requires putting them in the adults group, which is deliberate, and is - * reported as a finding either way. + * reported as a finding either way. Ending monitoring altogether is + * `/hawkmod deactivate`, which names a person and demands a reason. + * + * The same asymmetry runs the other way: a deactivated person who reappears in + * a group is reactivated here without ceremony, because gaining protection back + * never needs approval. * * Group editing is restricted to Workspace Admins in the workspace settings. - * That is not readable through any API, so it lives on the manual §6 checklist - * alongside retention and huddles. + * That is not readable through any API, so it stays on the manual §6 checklist + * alongside retention and huddles — and it is precisely why `slack/groupAdmin.ts` + * edits groups with an administrator's own token rather than the bot's. */ export async function syncRolesFromUserGroups( client: WebClient @@ -48,6 +56,7 @@ export async function syncRolesFromUserGroups( adultsInGroup: 0, created: 0, changed: 0, + reactivated: 0, conflicts: 0, reducedProtection: 0, missingGroups: 0, @@ -122,7 +131,36 @@ export async function syncRolesFromUserGroups( break; } + case "reactivate": { + // Declared again after being deactivated. Monitoring resumes without + // anyone's approval, for the same reason `create` needs none: this + // direction only ever adds protection. + setPersonActive({ + personId: decision.personId, + active: true, + source: SOURCE, + actor: SOURCE, + reason: "declared by a Slack user group", + }); + stats.reactivated += 1; + log.info("monitoring resumed from user group", { + user: decision.slackId, + role: decision.role, + }); + break; + } + case "change": { + if (decision.reactivates) { + setPersonActive({ + personId: decision.personId, + active: true, + source: SOURCE, + actor: SOURCE, + reason: "declared by a Slack user group", + }); + stats.reactivated += 1; + } setPersonRole({ personId: decision.personId, toRole: decision.to, diff --git a/src/slack/app.ts b/src/slack/app.ts index c92369f..ae743a3 100644 --- a/src/slack/app.ts +++ b/src/slack/app.ts @@ -1,4 +1,4 @@ -import { App } from "@slack/bolt"; +import { App, type InstallURLOptions } from "@slack/bolt"; import { APP_NAME, BRAND, ICON_SVG } from "../brand.js"; import { config } from "../config.js"; import { healthHandler } from "../health.js"; @@ -7,7 +7,7 @@ import { registerActions } from "./actions.js"; import { registerCommands } from "./commands.js"; import { registerEvents } from "./events.js"; import { registerViews } from "./modals.js"; -import { installationStore } from "./installStore.js"; +import { GROUP_ADMIN_METADATA, installationStore } from "./installStore.js"; /** Read-only apart from posting alerts. hawk-mod never needs to act as a user. */ export const BOT_SCOPES = [ @@ -41,9 +41,73 @@ export const USER_SCOPES = [ "users:read", ]; +/** + * Scopes for the group-editing grant, and deliberately nothing else. + * + * Slack issues one token per authorization carrying only what that + * authorization asked for, so this list is exactly what an administrator hands + * over: permission to edit user groups. It asks for no DM access, because an + * administrator is not necessarily a monitored mentor and the two consents + * should never be bundled — enrolment is a deliberate, named act, and burying + * it inside "let me edit user groups" would be a trick. + */ +export const GROUP_ADMIN_USER_SCOPES = ["usergroups:write"]; + export function createApp(): App { const cfg = config(); + /** + * Bolt builds an `InstallProvider` inside its receiver but does not put + * either on `App`'s public type, so reaching the URL generator needs this + * shape. `receiver` is private, hence the double assertion; keeping the + * target structural rather than `any` still means a Bolt upgrade that changes + * `generateInstallUrl` breaks here at compile time rather than at an + * administrator's first click. + */ + type WithInstaller = { + receiver?: { + installer?: { + generateInstallUrl(options: InstallURLOptions): Promise; + }; + }; + }; + + // Assigned immediately after construction; the route handler below only runs + // once a request arrives, long after that. + let self: App; + + /** + * Sends an administrator to Slack to grant group-editing permission. + * + * A second authorization route rather than a wider `/slack/install`, because + * the two grants must land in different rows. `metadata` is what tells the + * callback which one came back. + */ + const authorizeGroups = async ( + _req: import("http").IncomingMessage, + res: import("http").ServerResponse + ): Promise => { + try { + const installer = (self as unknown as WithInstaller).receiver?.installer; + if (!installer) throw new Error("no install provider on the receiver"); + const url = await installer.generateInstallUrl({ + // No bot scopes: this authorization adds a permission to one person's + // token and must not re-grant or alter the workspace installation. + scopes: [], + userScopes: GROUP_ADMIN_USER_SCOPES, + metadata: GROUP_ADMIN_METADATA, + }); + res.writeHead(302, { location: url }); + res.end(); + } catch (err) { + log.error("could not build group authorization url", { + error: String(err), + }); + res.writeHead(500, { "content-type": "text/plain; charset=utf-8" }); + res.end("Could not start authorization. Tell a coach."); + } + }; + const app = new App({ signingSecret: cfg.SLACK_SIGNING_SECRET, clientId: cfg.SLACK_CLIENT_ID, @@ -53,6 +117,11 @@ export function createApp(): App { installationStore, customRoutes: [ { path: "/health", method: ["GET"], handler: healthHandler }, + { + path: "/slack/authorize-groups", + method: ["GET"], + handler: authorizeGroups, + }, ], redirectUri: `${cfg.PUBLIC_URL}/slack/oauth_redirect`, installerOptions: { @@ -61,10 +130,27 @@ export function createApp(): App { installPath: "/slack/install", directInstall: true, callbackOptions: { - success: (_installation, _options, _req, res) => { + success: (installation, _options, _req, res) => { + const groupAdmin = installation.metadata === GROUP_ADMIN_METADATA; res.writeHead(200, { "content-type": "text/html; charset=utf-8" }); res.end( - ` + groupAdmin + ? ` + + ${APP_NAME} +
+ ${ICON_SVG} +

You can manage user groups.

+

${APP_NAME}

+

${APP_NAME} can now edit Slack user groups on your behalf when + you run /hawkmod group. Changes will show in Slack as + made by you, which is the point — every roster change has a + name against it.

+

This grants no access to your messages. If you are also an + enrolled mentor, that is a separate permission and this has not + touched it.

+
` + : ` ${APP_NAME}
@@ -89,6 +175,8 @@ export function createApp(): App { }, }); + self = app; + registerEvents(app); registerCommands(app); registerViews(app); diff --git a/src/slack/commands.ts b/src/slack/commands.ts index d29c4eb..d301a21 100644 --- a/src/slack/commands.ts +++ b/src/slack/commands.ts @@ -11,7 +11,9 @@ import { listFindings, listPeople, personByEmail, + personById, personBySlackId, + setPersonActive, } from "../db/repo.js"; import { today } from "../domain/dates.js"; import { severityEmoji } from "../domain/findings.js"; @@ -20,7 +22,8 @@ import { consentStatus } from "../domain/rules/consent.js"; import { screeningStatus } from "../domain/rules/screening.js"; import { log } from "../logger.js"; import { backfillAll } from "../monitor/backfill.js"; -import { administrator, NOT_PERMITTED } from "./authz.js"; +import { administrator, type Actor, NOT_PERMITTED } from "./authz.js"; +import { applyGroupEdit } from "./groupAdmin.js"; import { openConsent, openScreening } from "./modals.js"; import { runSweep } from "../jobs/sweep.js"; import { syncRolesFromUserGroups } from "../jobs/syncRoles.js"; @@ -31,6 +34,9 @@ const HELP = [ "`/hawkmod enroll` — link for a adult to authorize monitoring", "`/hawkmod findings [kind]` — open findings", "`/hawkmod whois @user` — role, consent, screening, enrollment", + "`/hawkmod group add @user @group` — put someone in a user group", + "`/hawkmod group remove @user @group` — take someone out of a user group", + "`/hawkmod deactivate @user ` — stop monitoring someone", "`/hawkmod screening @user` — record YPP / Mentor Ready / CORI dates", "`/hawkmod consent @user` — record a signed parental consent", "`/hawkmod ack ` — acknowledge without closing", @@ -145,6 +151,22 @@ export function registerCommands(app: App): void { return; } + case "group": { + await respond({ + response_type: "ephemeral", + text: await groupText(client, teamId, caller, rest), + }); + return; + } + + case "deactivate": { + await respond({ + response_type: "ephemeral", + text: await deactivateText(client, caller, rest), + }); + return; + } + case "sync": { const stats = await syncRolesFromUserGroups(client); await respond({ @@ -319,3 +341,151 @@ async function whoisText( return lines.join("\n"); } + +/** + * Pulls the group out of a mention. With link escaping on, Slack sends a user + * group as ``; with it off, the raw `@students`. Both + * reach `resolveGroup`, which accepts an id or a handle. + */ +function groupRef(raw: string): string | null { + const trimmed = raw.trim(); + if (!trimmed) return null; + const escaped = trimmed.match(/^ { + const [action, mention, group, ...reasonWords] = rest; + if (action !== "add" && action !== "remove") { + return "Usage: `/hawkmod group add|remove @user @group`."; + } + const ref = groupRef(group ?? ""); + if (!mention || !ref) { + return `Usage: \`/hawkmod group ${action} @user @group\`.`; + } + + const person = await resolvePerson(client, mention); + if (!person) { + return ( + `Couldn't find \`${mention}\` on the roster. hawk-mod only edits groups ` + + `for people it already knows, so the edit is never the first thing it ` + + `learns about someone.` + ); + } + + const reason = reasonWords.join(" ").trim(); + + const outcome = await applyGroupEdit({ + teamId, + actor: caller, + groupRef: ref, + action, + subject: person, + reason: reason || null, + source: "command", + }); + + if (!outcome.ok) { + // The group's handle is only known once it has been resolved, so the + // needs-a-reason refusal comes back from there and is dressed up here, + // where the caller's own words are still to hand. + if ("needsReason" in outcome) { + return ( + `*${person.full_name}* is a student. ${outcome.reason}\n` + + `\`/hawkmod group add ${mention} ${group} \`` + ); + } + return outcome.reason; + } + if (outcome.noop) { + return `*${person.full_name}* was already ${ + action === "add" ? "in" : "out of" + } @${outcome.handle}. Nothing changed.`; + } + + const lines = [ + `${action === "add" ? "Added" : "Removed"} *${person.full_name}* ` + + `${action === "add" ? "to" : "from"} @${outcome.handle}.`, + ]; + + // The honest half. Group membership declares a role; it does not end + // monitoring, and saying otherwise here would be the quiet failure this + // project exists to avoid. + if (action === "remove") { + lines.push( + `_${person.full_name} is still a ${person.role} on the roster and still ` + + `monitored. Leaving a group never ends monitoring — use ` + + `\`/hawkmod deactivate\` for that._` + ); + } + if (outcome.reducedMonitoring) { + lines.push( + `_${person.full_name} is no longer monitored as a student. Recorded ` + + `against your name: ${reason}_` + ); + } + return lines.join("\n"); +} + +/** + * `/hawkmod deactivate @user ` — the only thing here that makes hawk-mod + * see less, which is why it names a person and demands a reason in the same + * breath, exactly as `ack` and `resolve` do. + */ +async function deactivateText( + client: WebClient, + caller: Actor, + rest: string[] +): Promise { + const [mention, ...reasonWords] = rest; + const reason = reasonWords.join(" ").trim(); + if (!mention || !reason) { + return ( + "Usage: `/hawkmod deactivate @user ` — the reason is required.\n" + + "This is the one command that stops hawk-mod watching somebody." + ); + } + + const person = await resolvePerson(client, mention); + if (!person) return `No roster entry for \`${mention}\`.`; + if (person.active !== 1) { + return `*${person.full_name}* is already deactivated.`; + } + + setPersonActive({ + personId: person.id, + active: false, + source: "command", + actor: caller.name, + reason, + }); + + const after = personById(person.id); + const lines = [ + `*${person.full_name}* is no longer monitored. Recorded against your name, ` + + `with the reason you gave.`, + ]; + if (after?.role === "student") { + lines.push( + `_This was a student. Their recorded messages are kept; nothing new will ` + + `be recorded. Adding them back to a role user group resumes monitoring._` + ); + } + return lines.join("\n"); +} diff --git a/src/slack/groupAdmin.ts b/src/slack/groupAdmin.ts new file mode 100644 index 0000000..60d5a99 --- /dev/null +++ b/src/slack/groupAdmin.ts @@ -0,0 +1,217 @@ +import { WebClient } from "@slack/web-api"; +import { config, managedGroupHandles } from "../config.js"; +import { getInstallation, insertGroupChange } from "../db/repo.js"; +import type { Person } from "../domain/people.js"; +import { + planAdd, + planRemove, + reducesMonitoring, + type GroupPlan, +} from "../domain/rules/groupMembership.js"; +import { log } from "../logger.js"; +import type { Actor } from "./authz.js"; +import { resolveGroup, setGroupMembership } from "./userGroups.js"; + +export type GroupEditOutcome = + | { + ok: true; + plan: GroupPlan; + handle: string; + noop: boolean; + /** This edit ended someone's monitoring as a student. */ + reducedMonitoring: boolean; + } + | { ok: false; reason: string; needsAuthorization?: boolean } + /** + * Refused for want of a reason. Raised here rather than in the command + * handler because only this side knows the group's handle: an escaped + * mention arrives as an opaque id. + */ + | { ok: false; needsReason: true; handle: string; reason: string }; + +/** + * Serializes group writes within this process. + * + * Every edit is a read-modify-write against an endpoint that replaces the whole + * member list, so two overlapping edits lose one of them. hawk-mod is a single + * container, so one lock genuinely closes the door on hawk-mod racing itself — + * a command running while an event-driven resync or a sheet sync is in flight. + * + * It cannot close the door on hawk-mod racing a human in Slack's own UI; Slack + * offers no compare-and-swap on this endpoint. Re-reading membership inside the + * lock, immediately before writing, keeps that window to about one round trip. + */ +let queue: Promise = Promise.resolve(); + +function serialize(fn: () => Promise): Promise { + const run = queue.then(fn, fn); + queue = run.catch(() => undefined); + return run; +} + +/** + * The administrator's own Slack client for group edits. + * + * Not the bot's. Slack accepts a bot token for `usergroups.users.update` only + * when the workspace lets everyone edit user groups, which §6 forbids — so the + * write goes out as the administrator who asked for it, which also means Slack + * attributes the change to a real person. + */ +function adminClient(teamId: string, slackUserId: string): WebClient | null { + const row = getInstallation(teamId, "admin", slackUserId); + if (!row || row.revokedAt) return null; + const payload = row.payload as { user?: { token?: string } }; + const token = payload.user?.token; + return token ? new WebClient(token) : null; +} + +/** Where an administrator goes to grant group-editing permission. */ +export function authorizeUrl(): string { + return `${config().PUBLIC_URL}/slack/authorize-groups`; +} + +export type GroupEditRequest = { + teamId: string; + actor: Actor; + /** `@handle`, a raw handle, or a Slack group id from an escaped mention. */ + groupRef: string; + action: "add" | "remove"; + subject: Person | { slackUserId: string }; + /** Required only when the edit reduces someone's monitoring. */ + reason: string | null; + source: "command" | "sheet_sync"; +}; + +function subjectId(s: GroupEditRequest["subject"]): string { + return "id" in s ? (s.slack_user_id ?? "") : s.slackUserId; +} + +function subjectPersonId(s: GroupEditRequest["subject"]): number | null { + return "id" in s ? s.id : null; +} + +/** + * Applies one membership edit, or explains why it did not. + * + * Deliberately does not touch the roster. Slack fires `subteam_members_changed` + * on a successful write, and the existing sync reconciles from that — so there + * is still exactly one path from a group to a role, whether the group was + * edited here or by hand in Slack. Writing the roster here as well would give + * that fact two doors, and they would disagree the first time one of them + * failed. + */ +export async function applyGroupEdit( + req: GroupEditRequest +): Promise { + const target = subjectId(req.subject); + if (!target) { + return { ok: false, reason: "That person has no linked Slack account." }; + } + + const client = adminClient(req.teamId, req.actor.slackUserId); + if (!client) { + return { + ok: false, + needsAuthorization: true, + reason: + `hawk-mod needs your permission to edit user groups on your behalf. ` + + `Slack will not let it do this as itself while group editing is ` + + `restricted to admins, which is the correct setting.\n` + + `Authorize once here: ${authorizeUrl()}`, + }; + } + + return serialize(async () => { + // Read inside the lock so the plan reflects membership as it is now, not + // as it was when the command was typed. + const group = await resolveGroup(client, req.groupRef); + if (!group) { + return { + ok: false as const, + reason: `No user group \`${req.groupRef}\`.`, + }; + } + + if (!managedGroupHandles().has(group.handle.toLowerCase())) { + return { + ok: false as const, + reason: + `hawk-mod is not configured to edit @${group.handle}. Add it to ` + + `MANAGED_USERGROUPS if it should be manageable here, or edit it in ` + + `Slack directly.`, + }; + } + + // Moving a student into the adults group ends their monitoring as a + // student. Allowed — refusing would only push the same act into Slack's own + // UI, where hawk-mod learns of it from an event with no author and no + // reason — but never silently, and never by typo. + const reduces = + "role" in req.subject && + reducesMonitoring({ + action: req.action, + subjectRole: req.subject.role, + handle: group.handle, + adultHandle: config().ADULT_USERGROUP ?? "adults", + }); + + if (reduces && !req.reason) { + return { + ok: false as const, + needsReason: true as const, + handle: group.handle, + reason: + `Adding a student to @${group.handle} ends their monitoring as a ` + + `student, so this one needs a reason.`, + }; + } + + const plan = + req.action === "add" + ? planAdd(group.members, target) + : planRemove(group.members, target); + + if (plan.refusal) { + return { ok: false as const, reason: plan.refusal }; + } + + if (plan.add.length === 0 && plan.remove.length === 0) { + return { + ok: true as const, + plan, + handle: group.handle, + noop: true, + reducedMonitoring: false, + }; + } + + await setGroupMembership(client, group.id, plan.result); + + insertGroupChange({ + usergroupId: group.id, + handle: group.handle, + action: req.action, + subject: target, + personId: subjectPersonId(req.subject), + actor: req.actor.slackUserId, + actorName: req.actor.name, + reason: req.reason, + source: req.source, + }); + + log.info("user group edited", { + handle: group.handle, + action: req.action, + subject: target, + actor: req.actor.slackUserId, + }); + + return { + ok: true as const, + plan, + handle: group.handle, + noop: false, + reducedMonitoring: reduces, + }; + }); +} diff --git a/src/slack/installStore.ts b/src/slack/installStore.ts index 786fe1c..a5ebc5b 100644 --- a/src/slack/installStore.ts +++ b/src/slack/installStore.ts @@ -28,16 +28,30 @@ function teamKey( return id; } +/** Marks an authorization that came from the group-editing flow. */ +export const GROUP_ADMIN_METADATA = "group-admin"; + /** - * Two kinds of row live here: + * Three kinds of row live here: + * + * 'bot' — one per workspace, installed once by a workspace admin. Posts alerts. + * 'user' — one per enrolled adult, holding that adult's user token. This is + * what lets hawk-mod see DMs at all; Slack exposes no other way to + * read them below Enterprise Grid. + * 'admin' — one per administrator who authorized group editing, holding a + * token scoped to `usergroups:write` and nothing else. * - * 'bot' — one per workspace, installed once by a workspace admin. Posts alerts. - * 'user' — one per enrolled adult, holding that adult's user token. This is - * what lets hawk-mod see DMs at all; Slack exposes no other way to - * read them below Enterprise Grid. + * A fetch merges bot and user so a single event can carry both a bot token (to + * reply) and the observing adult's user token (to read the conversation). + * 'admin' rows are deliberately *not* part of that merge: they are fetched by + * name, by the one code path that edits groups, and never ride along on + * anything else. * - * A fetch merges the two so a single event can carry both a bot token (to reply) - * and the observing adult's user token (to read the conversation). + * The reason 'admin' is a separate row rather than extra scopes on 'user' is + * that Slack issues one token per authorization carrying only the scopes + * granted at that moment. An administrator who is also an enrolled mentor would + * otherwise have their DM token overwritten the first time they authorized + * group editing, ending their monitoring while coverage still read 100%. */ export const installationStore: InstallationStore = { async storeInstallation(installation) { @@ -56,6 +70,25 @@ export const installationStore: InstallationStore = { } if (installation.user?.token) { + // The group-editing grant is a different consent for a different purpose + // and gets its own row. Critically it must not fall through to the + // enrolment branch below, which would overwrite a mentor's DM token. + if (installation.metadata === GROUP_ADMIN_METADATA) { + saveInstallation({ + teamId, + enterpriseId, + slackUserId: installation.user.id, + kind: "admin", + payload: installation, + scopes: (installation.user.scopes ?? []).join(","), + }); + log.info("administrator authorized group editing", { + user: installation.user.id, + teamId, + }); + return; + } + // Enrolling grants hawk-mod that account's DM history. For a student // that would expose student-to-student conversations, which are // deliberately never recorded — so the enrolment is refused outright diff --git a/src/slack/userGroups.ts b/src/slack/userGroups.ts index ffd0955..0a94367 100644 --- a/src/slack/userGroups.ts +++ b/src/slack/userGroups.ts @@ -8,24 +8,35 @@ export type ResolvedGroup = { members: Set; }; +/** Slack ids for user groups are `S` followed by uppercase alphanumerics. */ +const GROUP_ID = /^S[A-Z0-9]{4,}$/; + +export function isGroupId(ref: string): boolean { + return GROUP_ID.test(ref.toUpperCase()); +} + /** - * Resolves a user group by its @handle. Handles are what people actually type - * and see, so they are what the config names; ids are opaque. + * Resolves a user group by its @handle or its id. Handles are what people + * actually type and see, so they are what the config names; ids are opaque — + * but an id is what Slack sends when a slash command has link escaping on and + * somebody types `@students`, which arrives as ``. * * Requires the `usergroups:read` bot scope, and User Groups themselves require * a Standard/Business+ plan — they do not exist on the free tier. */ export async function resolveGroup( client: WebClient, - handle: string + ref: string ): Promise { - const wanted = handle.replace(/^@/, "").toLowerCase(); + const raw = ref.replace(/^@/, ""); + const wanted = raw.toLowerCase(); + const byId = isGroupId(raw) ? raw.toUpperCase() : null; const list = await client.usergroups.list({ include_disabled: false }); - const group = (list.usergroups ?? []).find( - (g) => (g.handle ?? "").toLowerCase() === wanted + const group = (list.usergroups ?? []).find((g) => + byId ? g.id === byId : (g.handle ?? "").toLowerCase() === wanted ); if (!group?.id) { - log.warn("user group not found", { handle }); + log.warn("user group not found", { ref }); return null; } @@ -66,3 +77,32 @@ export async function fetchProfiles( } return out; } + +/** + * Replaces a user group's membership. + * + * `usergroups.users.update` takes the *complete* new member list — Slack sells + * no add-one or remove-one endpoint — which is why every caller goes through a + * plan rather than mutating a set in place. + * + * This must be called with an administrator's user token, not the bot token. + * Slack accepts a bot token here only when the workspace lets *everyone* edit + * user groups, and §6 requires that be restricted to Owners and Admins. The + * restriction is the point, so the token is what changes. + */ +export async function setGroupMembership( + client: WebClient, + usergroupId: string, + userIds: readonly string[] +): Promise { + if (userIds.length === 0) { + throw new Error( + "Refusing to send an empty membership; Slack rejects it and a plan " + + "should have caught this." + ); + } + await client.usergroups.users.update({ + usergroup: usergroupId, + users: userIds.join(","), + }); +} diff --git a/test/groupMembership.test.ts b/test/groupMembership.test.ts new file mode 100644 index 0000000..fe6161e --- /dev/null +++ b/test/groupMembership.test.ts @@ -0,0 +1,182 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import { + planAdd, + planGroupMembership, + planRemove, + reducesMonitoring, +} from "../src/domain/rules/groupMembership.js"; + +const set = (...ids: string[]) => new Set(ids); + +describe("group membership plans", () => { + it("adds one person without disturbing the rest", () => { + const plan = planAdd(set("U1", "U2"), "U3"); + assert.deepEqual(plan.add, ["U3"]); + assert.deepEqual(plan.remove, []); + assert.deepEqual(plan.result, ["U1", "U2", "U3"]); + assert.equal(plan.refusal, null); + }); + + it("treats adding someone already present as a no-op", () => { + const plan = planAdd(set("U1", "U2"), "U2"); + assert.deepEqual(plan.add, []); + assert.deepEqual(plan.remove, []); + assert.equal(plan.refusal, null); + }); + + it("removes one person however small the group", () => { + // A person naming a person. Never refused for being proportionally large, + // or removing someone from a group of three would be impossible. + const plan = planRemove(set("U1", "U2", "U3"), "U2"); + assert.deepEqual(plan.remove, ["U2"]); + assert.deepEqual(plan.result, ["U1", "U3"]); + assert.equal(plan.refusal, null); + }); + + it("refuses to empty a group, because Slack rejects it anyway", () => { + const plan = planRemove(set("U1"), "U1"); + assert.match( + plan.refusal ?? "", + /does not allow a user group to be emptied/ + ); + }); + + it("refuses a bulk plan that removes most of a group", () => { + // The shape of a bad spreadsheet: a handful of survivors, everyone else + // silently dropped. + const current = set("U1", "U2", "U3", "U4", "U5", "U6", "U7", "U8"); + const plan = planGroupMembership(current, set("U1", "U2")); + assert.deepEqual(plan.remove, ["U3", "U4", "U5", "U6", "U7", "U8"]); + assert.match(plan.refusal ?? "", /would remove 6 of 8 members \(75%\)/); + }); + + it("allows a bulk plan within the removal limit", () => { + const current = set("U1", "U2", "U3", "U4", "U5", "U6", "U7", "U8"); + const plan = planGroupMembership( + current, + set("U1", "U2", "U3", "U4", "U5", "U6") + ); + assert.deepEqual(plan.remove, ["U7", "U8"]); + assert.equal(plan.refusal, null); + }); + + it("counts additions and removals in the same plan", () => { + const plan = planGroupMembership( + set("U1", "U2", "U3", "U4"), + set("U1", "U2", "U3", "U9") + ); + assert.deepEqual(plan.add, ["U9"]); + assert.deepEqual(plan.remove, ["U4"]); + assert.deepEqual(plan.unchanged, ["U1", "U2", "U3"]); + assert.equal(plan.refusal, null); + }); + + it("carries the refusal on the plan rather than beside it", () => { + // A caller cannot apply a refused plan by forgetting to check a flag + // somewhere else; the refusal travels with the thing being applied. + const plan = planRemove(set("U1"), "U1"); + assert.ok(plan.refusal); + assert.deepEqual(plan.result, []); + }); + + it("respects a caller-supplied removal limit", () => { + const current = set("U1", "U2", "U3", "U4"); + const strict = planGroupMembership(current, set("U1", "U2"), { + maxRemovedFraction: 0.1, + }); + assert.ok(strict.refusal); + const loose = planGroupMembership(current, set("U1", "U2"), { + maxRemovedFraction: 0.9, + }); + assert.equal(loose.refusal, null); + }); +}); + +/** + * The gate in front of the single most consequential edit hawk-mod can make. + * It is keyed on the resolved handle for a reason — see the regression below. + */ +describe("edits that end a student's monitoring", () => { + const adults = "adults"; + + it("flags a student being added to the adults group", () => { + assert.equal( + reducesMonitoring({ + action: "add", + subjectRole: "student", + handle: "adults", + adultHandle: adults, + }), + true + ); + }); + + it("ignores an adult being added to the adults group", () => { + assert.equal( + reducesMonitoring({ + action: "add", + subjectRole: "adult", + handle: "adults", + adultHandle: adults, + }), + false + ); + }); + + it("ignores a student being added to any other group", () => { + assert.equal( + reducesMonitoring({ + action: "add", + subjectRole: "student", + handle: "programming", + adultHandle: adults, + }), + false + ); + }); + + it("ignores removals, which never end monitoring", () => { + assert.equal( + reducesMonitoring({ + action: "remove", + subjectRole: "student", + handle: "adults", + adultHandle: adults, + }), + false + ); + }); + + it("matches regardless of @ prefix or case on either side", () => { + assert.equal( + reducesMonitoring({ + action: "add", + subjectRole: "student", + handle: "Adults", + adultHandle: "@adults", + }), + true + ); + }); + + /** + * Regression. The first version of this compared the *raw slash-command + * argument* to the configured handle. The app sets `should_escape: true`, so + * Slack sends `` and the raw argument is an opaque + * id — which never equals "adults", so the gate never fired and a student + * could be moved into the adults group by typo, with no reason recorded. + * Only the resolved handle is a safe input here. + */ + it("is not fooled by a group id, because it never sees one", () => { + assert.equal( + reducesMonitoring({ + action: "add", + subjectRole: "student", + handle: "S0614TY5A", + adultHandle: adults, + }), + false + ); + }); +}); diff --git a/test/rosterSync.test.ts b/test/rosterSync.test.ts index 7e6ec5f..08be616 100644 --- a/test/rosterSync.test.ts +++ b/test/rosterSync.test.ts @@ -24,6 +24,11 @@ function person(slackId: string, role: Role): Person { }; } +/** Someone the roster remembers but no longer monitors. */ +function deactivated(p: Person): Person { + return { ...p, active: 0 }; +} + function roster(...people: Person[]): Map { return new Map(people.map((p) => [p.slack_user_id!, p])); } @@ -110,4 +115,85 @@ describe("user group reconciliation", () => { { kind: "conflict", slackId: "U9", personId: null }, ]); }); + + /** + * The mirror of the property above. Monitoring is sticky on the way out and + * automatic on the way back in: a deactivated student who is declared again + * is monitored again, without waiting for anyone to notice and approve it. + */ + describe("returning after deactivation", () => { + it("resumes monitoring when a deactivated student is declared again", () => { + const p = deactivated(person("U1", "student")); + const decisions = reconcileRoles(roster(p), groups(["U1"], [])); + assert.deepEqual(decisions, [ + { kind: "reactivate", personId: p.id, slackId: "U1", role: "student" }, + ]); + }); + + it("resumes monitoring for a deactivated adult too", () => { + const p = deactivated(person("U1", "adult")); + const decisions = reconcileRoles(roster(p), groups([], ["U1"])); + assert.deepEqual(decisions, [ + { kind: "reactivate", personId: p.id, slackId: "U1", role: "adult" }, + ]); + }); + + it("keeps a deactivated district observer's role while resuming them", () => { + const p = deactivated(person("U1", "district_observer")); + const decisions = reconcileRoles(roster(p), groups([], ["U1"])); + assert.deepEqual(decisions, [ + { + kind: "reactivate", + personId: p.id, + slackId: "U1", + role: "district_observer", + }, + ]); + }); + + it("reactivates and changes role in one decision", () => { + const p = deactivated(person("U1", "student")); + const decisions = reconcileRoles(roster(p), groups([], ["U1"])); + const d = decisions[0]!; + assert.equal(d.kind, "change"); + if (d.kind !== "change") return; + assert.equal(d.to, "adult"); + assert.equal(d.reactivates, true); + assert.equal(d.reducesProtection, true); + }); + + it("does not reactivate someone who is in no group", () => { + const p = deactivated(person("U1", "student")); + assert.deepEqual(reconcileRoles(roster(p), groups([], [])), []); + }); + + it("says nothing about an active person who is already right", () => { + const decisions = reconcileRoles( + roster(person("U1", "student")), + groups(["U1"], []) + ); + assert.deepEqual(decisions, [{ kind: "unchanged", slackId: "U1" }]); + }); + + /** + * There is no decision that deactivates anybody. If one ever appears here, + * the sync has gained the power to end monitoring on its own. + */ + it("never produces a decision that ends monitoring", () => { + const people = [ + person("U1", "student"), + person("U2", "adult"), + deactivated(person("U3", "student")), + deactivated(person("U4", "adult")), + ]; + const decisions = reconcileRoles( + roster(...people), + groups(["U1", "U3"], ["U2", "U4"]) + ); + for (const d of decisions) { + assert.notEqual(d.kind, "deactivate"); + if (d.kind === "change") assert.notEqual(d.reactivates, undefined); + } + }); + }); });