diff --git a/src/slack/resolve-allowlist-common.test.ts b/src/slack/resolve-allowlist-common.test.ts new file mode 100644 index 00000000000..b47bcf82d93 --- /dev/null +++ b/src/slack/resolve-allowlist-common.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, it, vi } from "vitest"; +import { + collectSlackCursorItems, + resolveSlackAllowlistEntries, +} from "./resolve-allowlist-common.js"; + +describe("collectSlackCursorItems", () => { + it("collects items across cursor pages", async () => { + type MockPage = { + items: string[]; + response_metadata?: { next_cursor?: string }; + }; + const fetchPage = vi + .fn() + .mockResolvedValueOnce({ + items: ["a", "b"], + response_metadata: { next_cursor: "cursor-1" }, + }) + .mockResolvedValueOnce({ + items: ["c"], + response_metadata: { next_cursor: "" }, + }); + + const items = await collectSlackCursorItems({ + fetchPage, + collectPageItems: (response) => response.items, + }); + + expect(items).toEqual(["a", "b", "c"]); + expect(fetchPage).toHaveBeenCalledTimes(2); + }); +}); + +describe("resolveSlackAllowlistEntries", () => { + it("handles id, non-id, and unresolved entries", () => { + const results = resolveSlackAllowlistEntries({ + entries: ["id:1", "name:beta", "missing"], + lookup: [ + { id: "1", name: "alpha" }, + { id: "2", name: "beta" }, + ], + parseInput: (input) => { + if (input.startsWith("id:")) { + return { id: input.slice("id:".length) }; + } + if (input.startsWith("name:")) { + return { name: input.slice("name:".length) }; + } + return {}; + }, + findById: (lookup, id) => lookup.find((entry) => entry.id === id), + buildIdResolved: ({ input, match }) => ({ input, resolved: true, name: match?.name }), + resolveNonId: ({ input, parsed, lookup }) => { + const name = (parsed as { name?: string }).name; + if (!name) { + return undefined; + } + const match = lookup.find((entry) => entry.name === name); + return match ? { input, resolved: true, name: match.name } : undefined; + }, + buildUnresolved: (input) => ({ input, resolved: false }), + }); + + expect(results).toEqual([ + { input: "id:1", resolved: true, name: "alpha" }, + { input: "name:beta", resolved: true, name: "beta" }, + { input: "missing", resolved: false }, + ]); + }); +}); diff --git a/src/slack/resolve-allowlist-common.ts b/src/slack/resolve-allowlist-common.ts new file mode 100644 index 00000000000..033087bb0ae --- /dev/null +++ b/src/slack/resolve-allowlist-common.ts @@ -0,0 +1,68 @@ +type SlackCursorResponse = { + response_metadata?: { next_cursor?: string }; +}; + +function readSlackNextCursor(response: SlackCursorResponse): string | undefined { + const next = response.response_metadata?.next_cursor?.trim(); + return next ? next : undefined; +} + +export async function collectSlackCursorItems< + TItem, + TResponse extends SlackCursorResponse, +>(params: { + fetchPage: (cursor?: string) => Promise; + collectPageItems: (response: TResponse) => TItem[]; +}): Promise { + const items: TItem[] = []; + let cursor: string | undefined; + do { + const response = await params.fetchPage(cursor); + items.push(...params.collectPageItems(response)); + cursor = readSlackNextCursor(response); + } while (cursor); + return items; +} + +export function resolveSlackAllowlistEntries< + TParsed extends { id?: string }, + TLookup, + TResult, +>(params: { + entries: string[]; + lookup: TLookup[]; + parseInput: (input: string) => TParsed; + findById: (lookup: TLookup[], id: string) => TLookup | undefined; + buildIdResolved: (params: { input: string; parsed: TParsed; match?: TLookup }) => TResult; + resolveNonId: (params: { + input: string; + parsed: TParsed; + lookup: TLookup[]; + }) => TResult | undefined; + buildUnresolved: (input: string) => TResult; +}): TResult[] { + const results: TResult[] = []; + + for (const input of params.entries) { + const parsed = params.parseInput(input); + if (parsed.id) { + const match = params.findById(params.lookup, parsed.id); + results.push(params.buildIdResolved({ input, parsed, match })); + continue; + } + + const resolved = params.resolveNonId({ + input, + parsed, + lookup: params.lookup, + }); + if (resolved) { + results.push(resolved); + continue; + } + + results.push(params.buildUnresolved(input)); + } + + return results; +} diff --git a/src/slack/resolve-channels.ts b/src/slack/resolve-channels.ts index 2112a2a3c2d..52ebbaf6835 100644 --- a/src/slack/resolve-channels.ts +++ b/src/slack/resolve-channels.ts @@ -1,5 +1,9 @@ import type { WebClient } from "@slack/web-api"; import { createSlackWebClient } from "./client.js"; +import { + collectSlackCursorItems, + resolveSlackAllowlistEntries, +} from "./resolve-allowlist-common.js"; export type SlackChannelLookup = { id: string; @@ -46,32 +50,31 @@ function parseSlackChannelMention(raw: string): { id?: string; name?: string } { } async function listSlackChannels(client: WebClient): Promise { - const channels: SlackChannelLookup[] = []; - let cursor: string | undefined; - do { - const res = (await client.conversations.list({ - types: "public_channel,private_channel", - exclude_archived: false, - limit: 1000, - cursor, - })) as SlackListResponse; - for (const channel of res.channels ?? []) { - const id = channel.id?.trim(); - const name = channel.name?.trim(); - if (!id || !name) { - continue; - } - channels.push({ - id, - name, - archived: Boolean(channel.is_archived), - isPrivate: Boolean(channel.is_private), - }); - } - const next = res.response_metadata?.next_cursor?.trim(); - cursor = next ? next : undefined; - } while (cursor); - return channels; + return collectSlackCursorItems({ + fetchPage: async (cursor) => + (await client.conversations.list({ + types: "public_channel,private_channel", + exclude_archived: false, + limit: 1000, + cursor, + })) as SlackListResponse, + collectPageItems: (res) => + (res.channels ?? []) + .map((channel) => { + const id = channel.id?.trim(); + const name = channel.name?.trim(); + if (!id || !name) { + return null; + } + return { + id, + name, + archived: Boolean(channel.is_archived), + isPrivate: Boolean(channel.is_private), + } satisfies SlackChannelLookup; + }) + .filter(Boolean) as SlackChannelLookup[], + }); } function resolveByName( @@ -97,36 +100,38 @@ export async function resolveSlackChannelAllowlist(params: { }): Promise { const client = params.client ?? createSlackWebClient(params.token); const channels = await listSlackChannels(client); - const results: SlackChannelResolution[] = []; - - for (const input of params.entries) { - const parsed = parseSlackChannelMention(input); - if (parsed.id) { - const match = channels.find((channel) => channel.id === parsed.id); - results.push({ + return resolveSlackAllowlistEntries< + { id?: string; name?: string }, + SlackChannelLookup, + SlackChannelResolution + >({ + entries: params.entries, + lookup: channels, + parseInput: parseSlackChannelMention, + findById: (lookup, id) => lookup.find((channel) => channel.id === id), + buildIdResolved: ({ input, parsed, match }) => ({ + input, + resolved: true, + id: parsed.id, + name: match?.name ?? parsed.name, + archived: match?.archived, + }), + resolveNonId: ({ input, parsed, lookup }) => { + if (!parsed.name) { + return undefined; + } + const match = resolveByName(parsed.name, lookup); + if (!match) { + return undefined; + } + return { input, resolved: true, - id: parsed.id, - name: match?.name ?? parsed.name, - archived: match?.archived, - }); - continue; - } - if (parsed.name) { - const match = resolveByName(parsed.name, channels); - if (match) { - results.push({ - input, - resolved: true, - id: match.id, - name: match.name, - archived: match.archived, - }); - continue; - } - } - results.push({ input, resolved: false }); - } - - return results; + id: match.id, + name: match.name, + archived: match.archived, + }; + }, + buildUnresolved: (input) => ({ input, resolved: false }), + }); } diff --git a/src/slack/resolve-users.test.ts b/src/slack/resolve-users.test.ts new file mode 100644 index 00000000000..ee05ddabb81 --- /dev/null +++ b/src/slack/resolve-users.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, it, vi } from "vitest"; +import { resolveSlackUserAllowlist } from "./resolve-users.js"; + +describe("resolveSlackUserAllowlist", () => { + it("resolves by email and prefers active human users", async () => { + const client = { + users: { + list: vi.fn().mockResolvedValue({ + members: [ + { + id: "U1", + name: "bot-user", + is_bot: true, + deleted: false, + profile: { email: "person@example.com" }, + }, + { + id: "U2", + name: "person", + is_bot: false, + deleted: false, + profile: { email: "person@example.com", display_name: "Person" }, + }, + ], + }), + }, + }; + + const res = await resolveSlackUserAllowlist({ + token: "xoxb-test", + entries: ["person@example.com"], + client: client as never, + }); + + expect(res[0]).toMatchObject({ + resolved: true, + id: "U2", + name: "Person", + email: "person@example.com", + isBot: false, + }); + }); + + it("keeps unresolved users", async () => { + const client = { + users: { + list: vi.fn().mockResolvedValue({ members: [] }), + }, + }; + + const res = await resolveSlackUserAllowlist({ + token: "xoxb-test", + entries: ["@missing-user"], + client: client as never, + }); + + expect(res[0]).toEqual({ input: "@missing-user", resolved: false }); + }); +}); diff --git a/src/slack/resolve-users.ts b/src/slack/resolve-users.ts index 53d2e4c9a74..340bfa0d6bb 100644 --- a/src/slack/resolve-users.ts +++ b/src/slack/resolve-users.ts @@ -1,5 +1,9 @@ import type { WebClient } from "@slack/web-api"; import { createSlackWebClient } from "./client.js"; +import { + collectSlackCursorItems, + resolveSlackAllowlistEntries, +} from "./resolve-allowlist-common.js"; export type SlackUserLookup = { id: string; @@ -61,35 +65,34 @@ function parseSlackUserInput(raw: string): { id?: string; name?: string; email?: } async function listSlackUsers(client: WebClient): Promise { - const users: SlackUserLookup[] = []; - let cursor: string | undefined; - do { - const res = (await client.users.list({ - limit: 200, - cursor, - })) as SlackListUsersResponse; - for (const member of res.members ?? []) { - const id = member.id?.trim(); - const name = member.name?.trim(); - if (!id || !name) { - continue; - } - const profile = member.profile ?? {}; - users.push({ - id, - name, - displayName: profile.display_name?.trim() || undefined, - realName: profile.real_name?.trim() || member.real_name?.trim() || undefined, - email: profile.email?.trim()?.toLowerCase() || undefined, - deleted: Boolean(member.deleted), - isBot: Boolean(member.is_bot), - isAppUser: Boolean(member.is_app_user), - }); - } - const next = res.response_metadata?.next_cursor?.trim(); - cursor = next ? next : undefined; - } while (cursor); - return users; + return collectSlackCursorItems({ + fetchPage: async (cursor) => + (await client.users.list({ + limit: 200, + cursor, + })) as SlackListUsersResponse, + collectPageItems: (res) => + (res.members ?? []) + .map((member) => { + const id = member.id?.trim(); + const name = member.name?.trim(); + if (!id || !name) { + return null; + } + const profile = member.profile ?? {}; + return { + id, + name, + displayName: profile.display_name?.trim() || undefined, + realName: profile.real_name?.trim() || member.real_name?.trim() || undefined, + email: profile.email?.trim()?.toLowerCase() || undefined, + deleted: Boolean(member.deleted), + isBot: Boolean(member.is_bot), + isAppUser: Boolean(member.is_app_user), + } satisfies SlackUserLookup; + }) + .filter(Boolean) as SlackUserLookup[], + }); } function scoreSlackUser(user: SlackUserLookup, match: { name?: string; email?: string }): number { @@ -143,46 +146,45 @@ export async function resolveSlackUserAllowlist(params: { }): Promise { const client = params.client ?? createSlackWebClient(params.token); const users = await listSlackUsers(client); - const results: SlackUserResolution[] = []; - - for (const input of params.entries) { - const parsed = parseSlackUserInput(input); - if (parsed.id) { - const match = users.find((user) => user.id === parsed.id); - results.push({ - input, - resolved: true, - id: parsed.id, - name: match?.displayName ?? match?.realName ?? match?.name, - email: match?.email, - deleted: match?.deleted, - isBot: match?.isBot, - }); - continue; - } - if (parsed.email) { - const matches = users.filter((user) => user.email === parsed.email); - if (matches.length > 0) { - results.push(resolveSlackUserFromMatches(input, matches, parsed)); - continue; + return resolveSlackAllowlistEntries< + { id?: string; name?: string; email?: string }, + SlackUserLookup, + SlackUserResolution + >({ + entries: params.entries, + lookup: users, + parseInput: parseSlackUserInput, + findById: (lookup, id) => lookup.find((user) => user.id === id), + buildIdResolved: ({ input, parsed, match }) => ({ + input, + resolved: true, + id: parsed.id, + name: match?.displayName ?? match?.realName ?? match?.name, + email: match?.email, + deleted: match?.deleted, + isBot: match?.isBot, + }), + resolveNonId: ({ input, parsed, lookup }) => { + if (parsed.email) { + const matches = lookup.filter((user) => user.email === parsed.email); + if (matches.length > 0) { + return resolveSlackUserFromMatches(input, matches, parsed); + } } - } - if (parsed.name) { - const target = parsed.name.toLowerCase(); - const matches = users.filter((user) => { - const candidates = [user.name, user.displayName, user.realName] - .map((value) => value?.toLowerCase()) - .filter(Boolean) as string[]; - return candidates.includes(target); - }); - if (matches.length > 0) { - results.push(resolveSlackUserFromMatches(input, matches, parsed)); - continue; + if (parsed.name) { + const target = parsed.name.toLowerCase(); + const matches = lookup.filter((user) => { + const candidates = [user.name, user.displayName, user.realName] + .map((value) => value?.toLowerCase()) + .filter(Boolean) as string[]; + return candidates.includes(target); + }); + if (matches.length > 0) { + return resolveSlackUserFromMatches(input, matches, parsed); + } } - } - - results.push({ input, resolved: false }); - } - - return results; + return undefined; + }, + buildUnresolved: (input) => ({ input, resolved: false }), + }); }