diff --git a/src/db/interfaces.ts b/src/db/interfaces.ts index 95db1e9..ef85d8e 100644 --- a/src/db/interfaces.ts +++ b/src/db/interfaces.ts @@ -346,6 +346,10 @@ export interface BotRecord { updated_at: Date; decided_at: Date | null; decided_by_server_user_id: string | null; + /** What an approved bot has asked for since, waiting on an admin (GRYT-2006). */ + more_permissions: Permission[]; + /** Asked for later and turned down, so the next knock doesn't ask again. */ + declined_permissions: Permission[]; } /** `request` records a knock and admits nothing; `disabled` refuses at the door, diff --git a/src/db/sqlite/bots.test.ts b/src/db/sqlite/bots.test.ts index b83c17d..dbde0cb 100644 --- a/src/db/sqlite/bots.test.ts +++ b/src/db/sqlite/bots.test.ts @@ -14,6 +14,7 @@ import { deleteBotRegistration, getBotById, listBots, + recordBotAsksMore, recordBotKnock, updateBotGrant, } from "./bots"; @@ -226,3 +227,46 @@ describe("what an approved bot can actually do", () => { assert.deepEqual([...standing.permissions], []); }); }); + +describe("an approved bot that asks for more (GRYT-2006)", () => { + const BOT_D = "BOT_eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee"; + const PENDING = "BOT_ffffffffffffffffffffffffffffffffffffffff"; + + async function approved() { + await recordBotKnock({ botId: BOT_D, nickname: "Gryt", requestedPermissions: ["read_messages", "send_messages"] }); + await decideBot(BOT_D, "approved", "owner", ["read_messages", "send_messages"]); + } + + it("is held for an admin, and grants nothing on its own", async () => { + await approved(); + assert.equal(await recordBotAsksMore(BOT_D, ["read_messages", "set_activity", "add_reactions"]), true); + const bot = await getBotById(BOT_D); + assert.deepEqual(bot?.more_permissions.sort(), ["add_reactions", "set_activity"]); + assert.deepEqual(bot?.granted_permissions.sort(), ["read_messages", "send_messages"]); + assert.equal(bot?.status, "approved"); + assert.equal(await recordBotAsksMore(BOT_D, ["set_activity"]), false, "asking again is not news"); + }); + + it("gets only what was ticked from the new ask, and keeps what it had", async () => { + await decideBot(BOT_D, "approved", "owner", ["read_messages", "send_messages", "set_activity", "kick_members"]); + const bot = await getBotById(BOT_D); + assert.deepEqual(bot?.granted_permissions.sort(), ["read_messages", "send_messages", "set_activity"]); + assert.deepEqual(bot?.more_permissions, []); + assert.deepEqual(bot?.declined_permissions, ["add_reactions"]); + assert.equal(await recordBotAsksMore(BOT_D, ["add_reactions"]), false, "a no is not asked again"); + }); + + it("stays in, with what it had, when the new ask is declined", async () => { + await recordBotAsksMore(BOT_D, ["view_members"]); + await decideBot(BOT_D, "denied", "owner"); + const bot = await getBotById(BOT_D); + assert.equal(bot?.status, "approved"); + assert.deepEqual(bot?.granted_permissions.sort(), ["read_messages", "send_messages", "set_activity"]); + assert.ok(bot?.declined_permissions.includes("view_members")); + }); + + it("is not asked for a bot still waiting at the door", async () => { + await recordBotKnock({ botId: PENDING, nickname: "Waiting", requestedPermissions: ["read_messages"] }); + assert.equal(await recordBotAsksMore(PENDING, ["kick_members"]), false); + }); +}); diff --git a/src/db/sqlite/bots.ts b/src/db/sqlite/bots.ts index 2fd831a..c74a745 100644 --- a/src/db/sqlite/bots.ts +++ b/src/db/sqlite/bots.ts @@ -55,6 +55,8 @@ function rowToBot(r: Record): BotRecord { updated_at: fromIso(r.updated_at as string), decided_at: fromIsoNullable(r.decided_at as string | null), decided_by_server_user_id: (r.decided_by_server_user_id as string) ?? null, + more_permissions: parsePermissions(r.more_permissions), + declined_permissions: parsePermissions(r.declined_permissions), }; } @@ -115,6 +117,24 @@ export async function recordBotKnock(input: { return { bot, created: true }; } +/** An approved bot declaring permissions it never asked for. Nothing is granted here: it + is held for an admin, and whatever was turned down before is not asked again. */ +export async function recordBotAsksMore(botId: string, declared: Permission[]): Promise { + const existing = await getBotById(botId); + if (!existing || existing.status !== "approved") return false; + const known = new Set([...existing.requested_permissions, ...existing.declined_permissions, ...existing.more_permissions]); + const fresh = normalizePermissions(declared).filter((p) => !known.has(p)); + if (fresh.length === 0) return false; + + const db = getSqliteDb(); + db.prepare(`UPDATE bots SET more_permissions = ?, updated_at = ? WHERE bot_id = ?`).run( + JSON.stringify(normalizePermissions([...existing.more_permissions, ...fresh])), + toIso(new Date()), + botId, + ); + return true; +} + /** The unattended path: an operator decides up front and hands over a token. Approved from the start, since the decision has already been made. */ export async function createBotRegistration(input: { @@ -186,14 +206,37 @@ export async function decideBot( const existing = await getBotById(botId); if (!existing) return null; + const db = getSqliteDb(); + const now = toIso(new Date()); + + /* Answering an approved bot's later ask. Declining it must not turn the bot away: + it keeps what it had, and only the new ones it isn't given are remembered as no. */ + if (existing.status === "approved" && existing.more_permissions.length > 0) { + const more = existing.more_permissions; + const allowed = new Set([...existing.requested_permissions, ...more]); + const wanted = decision === "approved" ? normalizePermissions(grantedPermissions).filter((p) => allowed.has(p)) : []; + const granted = decision === "approved" ? wanted : existing.granted_permissions; + const given = new Set(granted); + db.prepare( + `UPDATE bots SET requested_permissions = ?, granted_permissions = ?, more_permissions = '[]', declined_permissions = ?, decided_at = ?, decided_by_server_user_id = ?, updated_at = ? WHERE bot_id = ?`, + ).run( + JSON.stringify(normalizePermissions([...existing.requested_permissions, ...more.filter((p) => given.has(p))])), + JSON.stringify(normalizePermissions(granted)), + JSON.stringify(normalizePermissions([...existing.declined_permissions, ...more.filter((p) => !given.has(p))])), + now, + decidedByServerUserId, + now, + botId, + ); + return getBotById(botId); + } + const asked = new Set(existing.requested_permissions); const granted = decision === "approved" ? normalizePermissions(grantedPermissions).filter((p) => asked.has(p)) : []; - const db = getSqliteDb(); - const now = toIso(new Date()); db.prepare( `UPDATE bots SET status = ?, granted_permissions = ?, rank = ?, decided_at = ?, decided_by_server_user_id = ?, updated_at = ? WHERE bot_id = ?`, ).run( diff --git a/src/db/sqlite/connection.ts b/src/db/sqlite/connection.ts index 057b2b9..8079247 100644 --- a/src/db/sqlite/connection.ts +++ b/src/db/sqlite/connection.ts @@ -754,6 +754,13 @@ function hasColumn(d: DatabaseSync, table: string, column: string): boolean { function runMigrations(d: DatabaseSync): void { ensureSearchIndex(d); + // A known bot asking for more, and what was turned down (GRYT-2006). + if (!hasColumn(d, "bots", "more_permissions")) { + d.exec("ALTER TABLE bots ADD COLUMN more_permissions TEXT NOT NULL DEFAULT '[]'"); + } + if (!hasColumn(d, "bots", "declined_permissions")) { + d.exec("ALTER TABLE bots ADD COLUMN declined_permissions TEXT NOT NULL DEFAULT '[]'"); + } // Conversations muted on that phone (GRYT-1689). Only dev servers ever had the table without it. if (!hasColumn(d, "push_devices", "muted")) { d.exec("ALTER TABLE push_devices ADD COLUMN muted TEXT NOT NULL DEFAULT '[]'"); diff --git a/src/socket/handlers/admin.ts b/src/socket/handlers/admin.ts index c2c9bad..5530675 100644 --- a/src/socket/handlers/admin.ts +++ b/src/socket/handlers/admin.ts @@ -170,6 +170,8 @@ async function botsView() { description: b.description, requested: b.requested_permissions, granted: b.granted_permissions, + // Asked for since it was approved, and not yet answered (GRYT-2006). + more: b.more_permissions, rank: b.rank, status: b.status, // Whether it is still waiting for a bot to turn up and claim it. diff --git a/src/socket/handlers/join.ts b/src/socket/handlers/join.ts index b1c14c4..ded9b2a 100644 --- a/src/socket/handlers/join.ts +++ b/src/socket/handlers/join.ts @@ -37,6 +37,7 @@ import { clearJoinRequest, getBotById, claimBotRegistration, + recordBotAsksMore, recordBotKnock, insertServerAudit, } from "../../db"; @@ -115,7 +116,19 @@ async function admitBot( const existing = await getBotById(botId); if (existing) { - if (existing.status === "approved") return { ok: true, bot: existing }; + if (existing.status === "approved") { + // Let in on what it already holds. Anything new it declares waits for an admin (GRYT-2006). + if (await recordBotAsksMore(botId, normalizePermissions(declaration?.permissions ?? []))) { + insertServerAudit({ + actorServerUserId: null, + action: "bot_asked_more", + target: botId, + meta: { nickname: existing.nickname, asked: (await getBotById(botId))?.more_permissions ?? [] }, + }).catch((e: unknown) => consola.warn("audit log write failed", e)); + broadcastServerUiUpdate(); + } + return { ok: true, bot: existing }; + } if (existing.status === "denied") { // Told the same thing as a pending bot: confirming somebody looked and // said no invites an argument with the message.