diff --git a/src/db/sqlite/connection.ts b/src/db/sqlite/connection.ts index 86d84682..8d8138b5 100644 --- a/src/db/sqlite/connection.ts +++ b/src/db/sqlite/connection.ts @@ -753,8 +753,8 @@ function runMigrations(d: DatabaseSync): void { d.exec("ALTER TABLE conversations ADD COLUMN icon_file_id TEXT"); } - // On the membership row, since it is one person's answer. Nothing is deleted, - // and a new message brings it back. + // Unused since GRYT-1379: hiding a conversation is a client-side, per-device + // thing now. Kept so an older binary rolling back still finds the column. if (!hasColumn(d, "conversation_members", "hidden_at")) { d.exec("ALTER TABLE conversation_members ADD COLUMN hidden_at TEXT"); } @@ -829,6 +829,7 @@ function runMigrations(d: DatabaseSync): void { )`); migrateFileOwnership(d); + clearHiddenConversations(d); d.prepare("UPDATE server_config SET avatar_thumb_px = ?").run(AVATAR_THUMB_PX); @@ -848,6 +849,24 @@ function runMigrations(d: DatabaseSync): void { reissueAccessTokensAfterLeak(d); } +export const HIDDEN_CONVERSATIONS_KEY = "hidden_conversations_cleared"; + +/** + * Anything hidden before GRYT-1379 is still filtered out of the old client's + * list, and the new one cannot show what it is not sent. Once, then stamped. + */ +export function clearHiddenConversations(d: DatabaseSync): void { + if (readSchemaMeta(d, HIDDEN_CONVERSATIONS_KEY)) return; + + const result = d + .prepare(`UPDATE conversation_members SET hidden_at = NULL WHERE hidden_at IS NOT NULL`) + .run(); + if (Number(result.changes) > 0) { + console.log(`[conversations] unhid ${result.changes} conversation membership(s)`); + } + writeSchemaMeta(d, HIDDEN_CONVERSATIONS_KEY, "1"); +} + /** * SQLite cannot widen a primary key in place, so this is the standard rebuild. * A rollback leaves the wider table and an old build's inserts fail loudly. diff --git a/src/db/sqlite/conversations.ts b/src/db/sqlite/conversations.ts index df4f46ec..e70264fb 100644 --- a/src/db/sqlite/conversations.ts +++ b/src/db/sqlite/conversations.ts @@ -77,8 +77,8 @@ export interface ConversationSummary extends ConversationRecord { other_server_user_ids: string[]; } -/* Hidden rows and empty ones are filtered here, so one place decides what a list - holds. `created_at` is the fallback sort, for a conversation opened and unused. */ +/* Empty ones are filtered here, so one place decides what a list holds. + `created_at` is the fallback sort, for a conversation opened and unused. */ export async function listConversationsForUser(serverUserId: string): Promise { const db = getSqliteDb(); const rows = db @@ -87,7 +87,7 @@ export async function listConversationsForUser(serverUserId: string): Promise { - const db = getSqliteDb(); - db.prepare( - `UPDATE conversation_members SET hidden_at = ? - WHERE conversation_id = ? AND server_user_id = ? AND hidden_at IS NULL`, - ).run( - toIso(new Date()), - directConversationId(blockerServerUserId, blockedServerUserId), - blockerServerUserId, - ); -} - -export async function setConversationHidden( - conversationId: string, - serverUserId: string, - hidden: boolean, -): Promise { - const db = getSqliteDb(); - const result = db - .prepare( - `UPDATE conversation_members SET hidden_at = ? - WHERE conversation_id = ? AND server_user_id = ? AND (hidden_at IS NULL) = ?`, - ) - .run(hidden ? toIso(new Date()) : null, conversationId, serverUserId, hidden ? 1 : 0); - return result.changes > 0; -} - -/** Hiding is "not in my sidebar", so without this a hidden conversation swallows - everything sent afterwards. Returns whose list changed. */ -export async function clearConversationHidden(conversationId: string): Promise { - const db = getSqliteDb(); - const rows = db - .prepare( - `SELECT server_user_id FROM conversation_members - WHERE conversation_id = ? AND hidden_at IS NOT NULL`, - ) - .all(conversationId) as { server_user_id: string }[]; - if (rows.length === 0) return []; - - db.prepare(`UPDATE conversation_members SET hidden_at = NULL WHERE conversation_id = ?`).run( - conversationId, - ); - return rows.map((r) => r.server_user_id); -} - export async function touchConversation(conversationId: string, at: Date = new Date()): Promise { const db = getSqliteDb(); db.prepare(`UPDATE conversations SET last_message_at = ? WHERE conversation_id = ?`).run(toIso(at), conversationId); diff --git a/src/socket/handlers/blocks.ts b/src/socket/handlers/blocks.ts index f32ec5fe..a145df5b 100644 --- a/src/socket/handlers/blocks.ts +++ b/src/socket/handlers/blocks.ts @@ -6,7 +6,6 @@ import { unblockUser, listBlocks, getUserByServerId, - hideConversationsBetween, } from "../../db"; import { checkRateLimit, RateLimitRule } from "../../utils/rateLimiter"; @@ -63,13 +62,8 @@ export function registerBlockHandlers(ctx: HandlerContext): EventHandlerMap { await blockUser(auth.tokenPayload.grytUserId, target.gryt_user_id); - /* Only the blocker's list. `hidden_at` is per member and already exists - for closing a conversation, so this is what they could do by hand. */ - await hideConversationsBetween( - auth.tokenPayload.serverUserId, - payload.serverUserId, - ); - + /* The conversation used to be hidden here too. Hiding is the client's + own, per device, since GRYT-1379, so it does that on `user:blocked`. */ socket.emit("user:blocked", { serverUserId: payload.serverUserId }); } catch (err) { consola.error("user:block failed", err); diff --git a/src/socket/handlers/chat.ts b/src/socket/handlers/chat.ts index 28271d05..a7dd963d 100644 --- a/src/socket/handlers/chat.ts +++ b/src/socket/handlers/chat.ts @@ -21,7 +21,6 @@ import { getFilesByIds, getServerConfig, getWebhooksByIds, - clearConversationHidden, getConversation, touchConversation, DEFAULT_UPLOAD_MAX_BYTES, @@ -514,14 +513,10 @@ export function registerChatHandlers(ctx: HandlerContext): EventHandlerMap { consola.warn("touchConversation failed", created.conversation_id, err), ); - // A message unhides a conversation, since hiding only means "not in - // my sidebar". After the send, and swallowed: it cannot block one. + // The first message is what puts the conversation in everybody else's + // list. After the send, and swallowed: it cannot block one. try { - const restored = await clearConversationHidden(created.conversation_id); - const announce = wasEmpty - ? [...new Set([...restored, ...access.memberIds])] - : restored; - for (const serverUserId of announce) { + for (const serverUserId of wasEmpty ? new Set(access.memberIds) : []) { const views = await directConversationViews(serverUserId); const view = views.find((v) => v.conversation_id === created.conversation_id); if (!view) continue; @@ -531,7 +526,7 @@ export function registerChatHandlers(ctx: HandlerContext): EventHandlerMap { } } } catch (err) { - consola.warn("un-hiding the conversation failed", created.conversation_id, err); + consola.warn("announcing the conversation failed", created.conversation_id, err); } } diff --git a/src/socket/handlers/directMessages.test.ts b/src/socket/handlers/directMessages.test.ts index bcc6a0ab..054d1324 100644 --- a/src/socket/handlers/directMessages.test.ts +++ b/src/socket/handlers/directMessages.test.ts @@ -5,7 +5,12 @@ import { join } from "node:path"; import { after, before, describe, it } from "node:test"; import { BOT_SUB_PREFIX } from "../../auth/identity"; -import { initSqlite } from "../../db/sqlite/connection"; +import { + clearHiddenConversations, + getSqliteDb, + HIDDEN_CONVERSATIONS_KEY, + initSqlite, +} from "../../db/sqlite/connection"; import { BUILT_IN_ROLES } from "../../constants/permissions"; import { insertFile } from "../../db/sqlite/messages"; import { createRoleDefinition } from "../../db/sqlite/roleDefinitions"; @@ -561,7 +566,7 @@ describe("a role that may message people but not start groups", () => { }); }); -describe("hiding a conversation", () => { +describe("conversations an older build hid", () => { /** What this member's sidebar would show right now. */ async function listFor(who: Participant): Promise { who.clear(); @@ -570,83 +575,49 @@ describe("hiding a conversation", () => { return list.items.map((i) => i.conversation_id); } - async function hide(who: Participant, conversationId: string, hidden: boolean): Promise { - await who.handlers["dm:setHidden"]({ - accessToken: who.accessToken, - conversationId, - hidden, - }); + /** What a build before GRYT-1379 wrote when somebody hid a conversation. */ + function writeHiddenAt(conversationId: string, serverUserId: string): void { + getSqliteDb() + .prepare(`UPDATE conversation_members SET hidden_at = ? WHERE conversation_id = ? AND server_user_id = ?`) + .run(new Date().toISOString(), conversationId, serverUserId); } - it("takes it out of your list and leaves theirs alone", async () => { - // `hidden_at` is on the membership row, so it is one person's answer. A - // column on `conversations` would hide it for both. - const conversationId = await openDm(alice, bob); - - await hide(alice, conversationId, true); - - assert.equal((await listFor(alice)).includes(conversationId), false, "still in Alice's list"); - assert.equal((await listFor(bob)).includes(conversationId), true, "hiding took it off Bob's too"); - }); - - it("keeps every message", async () => { - const conversationId = await openDm(alice, bob); - await alice.handlers["chat:send"]({ conversationId, accessToken: alice.accessToken, text: "still here" }); - - await hide(alice, conversationId, true); - - clearAll(); - await alice.handlers["chat:fetch"]({ conversationId }); - const history = alice.received("chat:history")[0] as { items: { text: string }[] } | undefined; - assert.ok(history, "hiding took the history away, and it must not"); - assert.ok(history.items.some((m) => m.text === "still here")); - }); - - it("comes back when they say something", async () => { - // Otherwise hiding is a way to never hear from somebody again, and the - // only sign would be an unread count on a row that is not there. - const conversationId = await openDm(alice, bob); - await hide(alice, conversationId, true); - assert.equal((await listFor(alice)).includes(conversationId), false); - - clearAll(); - await bob.handlers["chat:send"]({ conversationId, accessToken: bob.accessToken, text: "you there?" }); - - // Read before `listFor`, which clears what the socket has seen. - const toldAlice = alice.received("dm:opened").some( - (v) => (v as { conversation_id?: string }).conversation_id === conversationId, - ); - - assert.equal((await listFor(alice)).includes(conversationId), true, "stayed hidden through a new message"); - assert.ok(toldAlice, "Alice was never told it came back, so her sidebar would not know"); - }); - - it("comes back when you open it again", async () => { - const conversationId = await openDm(alice, bob); - await hide(alice, conversationId, true); - - await alice.handlers["dm:open"]({ accessToken: alice.accessToken, targetServerUserId: bob.serverUserId }); - - assert.equal((await listFor(alice)).includes(conversationId), true); + it("no longer takes dm:setHidden", async () => { + // Hiding is the client's own now, per device. An older client emitting this + // gets no answer, which is the accepted cost of the server letting go. + assert.equal(alice.handlers["dm:setHidden"], undefined); }); - it("can be put back by hand", async () => { + it("lists one that still carries hidden_at", async () => { + // The list stopped filtering on the column, so a row an older build hid is + // sent again and the client decides whether to draw it. const conversationId = await openDm(alice, bob); - await hide(alice, conversationId, true); - await hide(alice, conversationId, false); + writeHiddenAt(conversationId, alice.serverUserId); - assert.equal((await listFor(alice)).includes(conversationId), true); + assert.equal((await listFor(alice)).includes(conversationId), true, "still filtered out of the list"); }); - it("refuses a conversation that is not yours", async () => { + it("clears the column once, and leaves the marker behind", async () => { const conversationId = await openDm(alice, bob); - clearAll(); - - await hide(mallory, conversationId, true); - - assert.ok(mallory.received("dm:error").length > 0, "no refusal"); - assert.equal((await listFor(alice)).includes(conversationId), true, "Mallory hid somebody else's conversation"); - assert.equal((await listFor(bob)).includes(conversationId), true); + writeHiddenAt(conversationId, alice.serverUserId); + + const db = getSqliteDb(); + db.prepare(`DELETE FROM schema_meta WHERE key = ?`).run(HIDDEN_CONVERSATIONS_KEY); + clearHiddenConversations(db); + + const row = db + .prepare(`SELECT hidden_at FROM conversation_members WHERE conversation_id = ? AND server_user_id = ?`) + .get(conversationId, alice.serverUserId) as { hidden_at: string | null } | undefined; + assert.equal(row?.hidden_at, null, "the migration left a hidden_at behind"); + + // Stamped, so it never runs again — and a rollback to an older build finds + // the column still there rather than a missing one. + writeHiddenAt(conversationId, alice.serverUserId); + clearHiddenConversations(db); + const again = db + .prepare(`SELECT hidden_at FROM conversation_members WHERE conversation_id = ? AND server_user_id = ?`) + .get(conversationId, alice.serverUserId) as { hidden_at: string | null } | undefined; + assert.notEqual(again?.hidden_at, null, "the one-shot ran a second time"); }); }); diff --git a/src/socket/handlers/dm.ts b/src/socket/handlers/dm.ts index 1d5b15ee..f5a94051 100644 --- a/src/socket/handlers/dm.ts +++ b/src/socket/handlers/dm.ts @@ -15,7 +15,6 @@ import { MAX_CONVERSATION_MEMBERS, purgeOrphanedConversations, openDirectConversation, - setConversationHidden, setConversationIcon, setConversationName, eitherHasBlocked, @@ -199,10 +198,6 @@ export function registerDirectMessageHandlers(ctx: HandlerContext): EventHandler const conversation = await openDirectConversation(self, target); - // Asking for a hidden conversation is asking for it back. The caller's - // row only; a message is what brings the other party's back. - await setConversationHidden(conversation.conversation_id, self, false); - /* The other end only once there is something to tell them about. An empty conversation is in nobody's list but its opener's. */ const audience = conversation.last_message_at ? [self, target] : [self]; @@ -222,51 +217,6 @@ export function registerDirectMessageHandlers(ctx: HandlerContext): EventHandler } }, - /** `hidden_at` is on the caller's own row and a new message brings it back, - so this is not a way to stop somebody talking to you. */ - 'dm:setHidden': async (payload: { accessToken: string; conversationId: string; hidden: boolean }) => { - try { - const ip = getClientIp(); - const rl = checkRateLimit("dm:setHidden", clientsInfo[clientId]?.serverUserId, ip, RL_OPEN); - if (!rl.allowed) { - socket.emit("dm:error", { error: "rate_limited", retryAfterMs: rl.retryAfterMs, message: `Too fast. Wait ${Math.ceil((rl.retryAfterMs || 0) / 1000)}s.` }); - return; - } - - if (!payload || typeof payload.accessToken !== "string" || typeof payload.conversationId !== "string" || typeof payload.hidden !== "boolean") { - socket.emit("dm:error", { error: "invalid_payload", message: "Invalid payload" }); - return; - } - - const auth = await requireAuth(socket, payload); - if (!auth) return; - - const self = auth.tokenPayload.serverUserId; - - // The membership check is what stops a row being written for somebody - // else's conversation if the table ever gains a different key. - const conversation = await getConversation(payload.conversationId); - if (!conversation || !(await isConversationMember(payload.conversationId, self))) { - socket.emit("dm:error", { error: "not_found", message: "No such conversation" }); - return; - } - - await setConversationHidden(payload.conversationId, self, payload.hidden); - - // Every socket this person has, so hiding on the desktop takes it off - // the phone as well. - for (const cid of socketIdsFor(self)) { - io.sockets.sockets.get(cid)?.emit("dm:hidden", { - conversation_id: payload.conversationId, - hidden: payload.hidden, - }); - } - } catch (err) { - consola.error("dm:setHidden failed", err); - socket.emit("dm:error", { error: "failed", message: "Could not update the conversation" }); - } - }, - /** Adding somebody to a one-to-one does not happen: this makes a new id and the pair conversation stays as it was. */ 'dm:group:create': async (payload: { accessToken: string; memberIds: string[]; name?: string; iconFileId?: string }) => {