diff --git a/backend/__tests__/service/discordInstallBind.test.js b/backend/__tests__/service/discordInstallBind.test.js new file mode 100644 index 0000000000..7960928c92 --- /dev/null +++ b/backend/__tests__/service/discordInstallBind.test.js @@ -0,0 +1,324 @@ +// Discord install and bind: a Discord bot add started from Commonly's install +// link must come back through `/api/discord/callback`, and the success page's +// two requests must then leave a bound pod. On prod 11-04 the link carried no +// `redirect_uri` and no `response_type=code`, so Discord added the bot and never +// redirected: no callback, no install proof, no Integration, and `GET /binding` +// stayed 404. This walks the whole chain against a real mongod, with Discord's +// HTTP API stubbed at axios: +// +// install-link -> Discord authorize (asserted as a URL) -> callback(code) +// -> /discord/success?install= -> channels(proof) -> POST integrations +// -> GET binding +// +// The redirect the link names must be the one the callback exchanges the code +// with: Discord refuses a token exchange whose redirect_uri differs from the +// authorize request's. +/* eslint-disable global-require */ +const express = require('express'); +const request = require('supertest'); +const mongoose = require('mongoose'); +const { MongoMemoryServer } = require('mongodb-memory-server'); + +process.env.PG_HOST = ''; + +const OWNER = '6a8f6de2a1dccf2e02f31401'; +const MEMBER = '6a8f6de2a1dccf2e02f31402'; +const POD_ID = '6a8f6de2a1dccf2e02f31499'; +const GUILD = '123456789012345678'; +const CHANNEL = '234567890123456789'; +const WEBHOOK = { id: '345678901234567890', token: 'webhook-token' }; + +jest.mock('../../middleware/auth', () => (req, _res, next) => { + const token = String(req.header('Authorization') || '').replace('Bearer ', ''); + req.user = { id: token === 'member-token' ? '6a8f6de2a1dccf2e02f31402' : '6a8f6de2a1dccf2e02f31401' }; + req.userId = req.user.id; + next(); +}); +jest.mock('../../middleware/adminAuth', () => (_req, _res, next) => next()); +jest.mock('../../middleware/integrationRateLimit', () => ({ + writeIntegrationsRateLimit: (_req, _res, next) => next(), + listIntegrationsRateLimit: (_req, _res, next) => next(), + ingestIngressRateLimit: (_req, _res, next) => next(), + ingestIntegrationsRateLimit: (_req, _res, next) => next(), +})); +jest.mock('axios', () => ({ get: jest.fn(), post: jest.fn() })); +jest.mock('../../models/User', () => ({ findById: jest.fn() })); +jest.mock('../../models/Pod', () => ({ findById: jest.fn(), find: jest.fn() })); +jest.mock('../../services/discordService', () => jest.fn().mockImplementation(() => ({ + initialize: jest.fn().mockResolvedValue(true), + connect: jest.fn().mockResolvedValue(true), +}))); +jest.mock('../../services/connectorSecrets', () => ({ + put: jest.fn().mockResolvedValue('secret-ref-1'), + get: jest.fn(), + remove: jest.fn(), +})); + +let mongod; +let app; +let axios; +let Integration; +let DiscordIntegration; +let User; +let Pod; + +const POD = { + _id: POD_ID, name: 'Discord walk', type: 'team', createdBy: OWNER, members: [OWNER, MEMBER], +}; + +beforeAll(async () => { + process.env.JWT_SECRET = 'test-secret'; + process.env.DISCORD_CLIENT_ID = '1111111111'; + process.env.DISCORD_CLIENT_SECRET = 'client-secret'; + process.env.DISCORD_BOT_TOKEN = 'bot-token'; + process.env.BACKEND_URL = 'https://api.example.test'; + process.env.FRONTEND_URL = 'https://app.example.test'; + mongod = await MongoMemoryServer.create(); + await mongoose.connect(mongod.getUri()); + axios = require('axios'); + Integration = require('../../models/Integration'); + DiscordIntegration = require('../../models/DiscordIntegration'); + User = require('../../models/User'); + Pod = require('../../models/Pod'); + await Integration.syncIndexes(); + app = express(); + app.use(express.json()); + app.use('/api/discord', require('../../routes/discord')); + app.use('/api/integrations', require('../../routes/integrations')); +}); + +afterAll(async () => { + await mongoose.disconnect(); + await mongod.stop(); +}); + +beforeEach(async () => { + jest.clearAllMocks(); + await Integration.deleteMany({}); + await DiscordIntegration.deleteMany({}); + User.findById.mockImplementation(async (id) => ({ _id: String(id), role: 'user' })); + Pod.findById.mockImplementation(async (id) => (String(id) === POD_ID ? { ...POD } : null)); + Pod.find.mockImplementation((query) => { + const userId = String(query?.$or?.[0]?.members || ''); + const rows = POD.members.includes(userId) || POD.createdBy === userId ? [{ _id: POD_ID }] : []; + return { select: () => ({ lean: async () => rows }) }; + }); + axios.post.mockImplementation(async (url) => { + if (url === 'https://discord.com/api/oauth2/token') { + return { data: { access_token: 'user-access', guild: { id: GUILD, name: 'Commonly Dev' } } }; + } + if (url === `https://discord.com/api/channels/${CHANNEL}/webhooks`) return { data: WEBHOOK }; + throw new Error(`unexpected POST ${url}`); + }); + axios.get.mockImplementation(async (url) => { + if (url === `https://discord.com/api/guilds/${GUILD}`) return { data: { id: GUILD, name: 'Commonly Dev' } }; + if (url === `https://discord.com/api/guilds/${GUILD}/channels`) { + return { data: [{ id: CHANNEL, name: 'general', type: 0 }, { id: '999999999999999999', name: 'Voice', type: 2 }] }; + } + if (url === `https://discord.com/api/channels/${CHANNEL}`) return { data: { id: CHANNEL, guild_id: GUILD, type: 0 } }; + throw new Error(`unexpected GET ${url}`); + }); +}); + +const CALLBACK = 'https://api.example.test/api/discord/callback'; + +test('the install link sends Discord back to the callback with a code', async () => { + const res = await request(app).get(`/api/discord/install-link/${POD_ID}`).set('Authorization', 'Bearer owner-token'); + expect(res.status).toBe(200); + const url = new URL(res.body.installUrl); + expect(url.origin + url.pathname).toBe('https://discord.com/api/oauth2/authorize'); + expect(url.searchParams.get('client_id')).toBe('1111111111'); + expect(url.searchParams.get('response_type')).toBe('code'); + expect(url.searchParams.get('redirect_uri')).toBe(CALLBACK); + // A signed state, not the bare pod id a crafted link could carry. + expect(url.searchParams.get('state')).not.toBe(`pod_${POD_ID}`); + expect(url.searchParams.get('state')).not.toContain(POD_ID); + expect(url.searchParams.get('scope')).toBe('bot applications.commands'); +}); + +test('the link ignores a caller-supplied client id', async () => { + const res = await request(app) + .get(`/api/discord/install-link/${POD_ID}?clientId=2222222222`) + .set('Authorization', 'Bearer owner-token'); + expect(res.status).toBe(200); + expect(new URL(res.body.installUrl).searchParams.get('client_id')).toBe('1111111111'); +}); + +test('a bot add from the link ends in a bound pod', async () => { + const link = await request(app).get(`/api/discord/install-link/${POD_ID}`).set('Authorization', 'Bearer owner-token'); + const authorize = new URL(link.body.installUrl); + expect((await request(app).get(`/api/discord/binding/${POD_ID}`).set('Authorization', 'Bearer owner-token')).status).toBe(404); + + // Discord redirects the browser to the link's redirect_uri with a code, the + // state it was given and the guild the bot joined. + const back = new URL(authorize.searchParams.get('redirect_uri')); + const cb = await request(app) + .get(`${back.pathname}?code=the-code&state=${authorize.searchParams.get('state')}&guild_id=${GUILD}`); + expect(cb.status).toBe(302); + const success = new URL(cb.headers.location); + expect(success.origin + success.pathname).toBe('https://app.example.test/discord/success'); + expect(success.searchParams.get('pod_id')).toBe(POD_ID); + expect(success.searchParams.get('guild_id')).toBe(GUILD); + const proof = success.searchParams.get('install'); + expect(proof).toBeTruthy(); + // The code exchange names the same redirect the authorize request did. + const exchange = axios.post.mock.calls.find(([url]) => url === 'https://discord.com/api/oauth2/token'); + expect(exchange[1].redirect_uri).toBe(CALLBACK); + + // The success page lists the guild's channels on the proof... + const channels = await request(app) + .get(`/api/discord/channels/${GUILD}?install=${encodeURIComponent(proof)}`) + .set('Authorization', 'Bearer owner-token'); + expect(channels.status).toBe(200); + expect(channels.body.map((c) => c.id)).toContain(CHANNEL); + + // ...and binds the one the person picks. + const bind = await request(app) + .post('/api/integrations') + .set('Authorization', 'Bearer owner-token') + .send({ + podId: POD_ID, + type: 'discord', + config: { + serverId: GUILD, serverName: 'Commonly Dev', channelId: CHANNEL, channelName: 'general', webhookUrl: '', botToken: '', + }, + install: proof, + }); + expect(bind.status).toBe(201); + + const row = await Integration.findOne({ podId: POD_ID, type: 'discord' }).lean(); + expect(row).toBeTruthy(); + expect(String(row.createdBy)).toBe(OWNER); + expect(row.config.serverId).toBe(GUILD); + expect(row.config.channelId).toBe(CHANNEL); + expect(row.config.webhookUrlRef).toBe('secret-ref-1'); + const platform = await DiscordIntegration.findOne({ integrationId: row._id }).lean(); + expect(platform).toMatchObject({ serverId: GUILD, channelId: CHANNEL, webhookId: WEBHOOK.id }); + + const binding = await request(app).get(`/api/discord/binding/${POD_ID}`).set('Authorization', 'Bearer owner-token'); + expect(binding.status).toBe(200); + expect(binding.body.discordIntegration.channelId).toBe(CHANNEL); +}); + +test('a pod member who did not create the pod gets no install link', async () => { + const res = await request(app).get(`/api/discord/install-link/${POD_ID}`).set('Authorization', 'Bearer member-token'); + expect(res.status).toBe(403); +}); + +// The callback has no session, so the state is the only thing tying a code to +// the person who asked for the install. Unsigned, a crafted link carrying an +// attacker's code and a victim's pod would mint a proof the victim's browser then +// spends binding their pod to the attacker's guild (CodeRabbit on this change). +describe('the install state and proof are bound to the person who asked', () => { + const callback = (state) => request(app).get(`/api/discord/callback?code=the-code&state=${encodeURIComponent(state)}&guild_id=${GUILD}`); + const errorOf = (res) => new URL(res.headers.location).pathname; + + test('a bare pod_ state is refused before the code is exchanged', async () => { + const res = await callback(`pod_${POD_ID}`); + expect(res.status).toBe(302); + expect(errorOf(res)).toBe('/discord/error'); + expect(axios.post.mock.calls.find(([url]) => url === 'https://discord.com/api/oauth2/token')).toBeUndefined(); + }); + + test('an expired state is refused', async () => { + const jwt = require('jsonwebtoken'); + const stale = jwt.sign({ pur: 'discord-install-state', pod: POD_ID, uid: OWNER }, process.env.JWT_SECRET, { expiresIn: -10 }); + const res = await callback(stale); + expect(errorOf(res)).toBe('/discord/error'); + }); + + test("a proof minted for one person is refused for another member of the same pod", async () => { + const link = await request(app).get(`/api/discord/install-link/${POD_ID}`).set('Authorization', 'Bearer owner-token'); + const state = new URL(link.body.installUrl).searchParams.get('state'); + const cb = await callback(state); + const proof = new URL(cb.headers.location).searchParams.get('install'); + expect(proof).toBeTruthy(); + + const channels = await request(app) + .get(`/api/discord/channels/${GUILD}?install=${encodeURIComponent(proof)}`) + .set('Authorization', 'Bearer member-token'); + expect(channels.status).toBe(404); + + const bind = await request(app) + .post('/api/integrations') + .set('Authorization', 'Bearer member-token') + .send({ + podId: POD_ID, + type: 'discord', + config: { + serverId: GUILD, serverName: 'Commonly Dev', channelId: CHANNEL, channelName: 'general', webhookUrl: '', botToken: '', + }, + install: proof, + }); + // A member is refused at the pod check (only the creator or an admin binds). + expect(bind.status).toBe(403); + expect(await Integration.findOne({ podId: POD_ID, type: 'discord' })).toBeNull(); + + // An admin passes the pod check, and is still refused another person's proof. + User.findById.mockImplementation(async (id) => ({ _id: String(id), role: String(id) === MEMBER ? 'admin' : 'user' })); + const adminBind = await request(app) + .post('/api/integrations') + .set('Authorization', 'Bearer member-token') + .send({ + podId: POD_ID, + type: 'discord', + config: { + serverId: GUILD, serverName: 'Commonly Dev', channelId: CHANNEL, channelName: 'general', webhookUrl: '', botToken: '', + }, + install: proof, + }); + expect(adminBind.status).toBe(404); + expect(await Integration.findOne({ podId: POD_ID, type: 'discord' })).toBeNull(); + }); +}); + + +// A channel bridge is a pod-level setting, so binding one takes the same +// authority as the install link: the pod's creator or an instance admin. The +// "you bound this guild before" arm proves authority over the GUILD, not over +// the pod, so on its own it let a plain member bridge someone else's pod. +describe('binding a Discord channel takes authority over the pod', () => { + const MEMBERS_OWN_POD = '6a8f6de2a1dccf2e02f314ff'; + const bindAs = (token, install) => request(app) + .post('/api/integrations') + .set('Authorization', `Bearer ${token}`) + .send({ + podId: POD_ID, + type: 'discord', + config: { + serverId: GUILD, serverName: 'Commonly Dev', channelId: CHANNEL, channelName: 'general', webhookUrl: '', botToken: '', + }, + ...(install ? { install } : {}), + }); + + test("a member who bound the guild to their own pod still can't bridge a pod they didn't create", async () => { + Pod.find.mockImplementation((query) => { + const userId = String(query?.$or?.[0]?.members || ''); + const rows = userId === MEMBER ? [{ _id: POD_ID }, { _id: MEMBERS_OWN_POD }] : [{ _id: POD_ID }]; + return { select: () => ({ lean: async () => rows }) }; + }); + await Integration.collection.insertOne({ + podId: new mongoose.Types.ObjectId(MEMBERS_OWN_POD), + type: 'discord', + status: 'connected', + config: { serverId: GUILD, channelId: '777777777777777777' }, + createdBy: new mongoose.Types.ObjectId(MEMBER), + }); + + const res = await bindAs('member-token'); + expect(res.status).toBe(403); + expect(await Integration.findOne({ podId: POD_ID, type: 'discord' })).toBeNull(); + expect(axios.post.mock.calls.find(([url]) => url.endsWith('/webhooks'))).toBeUndefined(); + }); + + test('an instance admin may bind a pod they did not create, on a proof minted for them', async () => { + User.findById.mockImplementation(async (id) => ({ _id: String(id), role: String(id) === MEMBER ? 'admin' : 'user' })); + const link = await request(app).get(`/api/discord/install-link/${POD_ID}`).set('Authorization', 'Bearer member-token'); + expect(link.status).toBe(200); + const state = new URL(link.body.installUrl).searchParams.get('state'); + const cb = await request(app).get(`/api/discord/callback?code=the-code&state=${encodeURIComponent(state)}&guild_id=${GUILD}`); + const proof = new URL(cb.headers.location).searchParams.get('install'); + + expect((await bindAs('member-token', proof)).status).toBe(201); + }); +}); diff --git a/backend/__tests__/unit/routes/discordBindingContainment.test.js b/backend/__tests__/unit/routes/discordBindingContainment.test.js index ae9e4a74ed..43f7e8f617 100644 --- a/backend/__tests__/unit/routes/discordBindingContainment.test.js +++ b/backend/__tests__/unit/routes/discordBindingContainment.test.js @@ -292,7 +292,7 @@ describe('Discord binding containment (TASK-123 a)', () => { }; it('lists channels for a member of the pod the proof names', async () => { - const res = await firstInstall(mintDiscordInstallProof({ podId: 'pod-1', guildId: GUILD })); + const res = await firstInstall(mintDiscordInstallProof({ podId: 'pod-1', guildId: GUILD, userId: MEMBER })); expect(res.status).toBe(200); expect(res.body).toEqual([{ id: CHANNEL, name: 'general', topic: undefined }]); @@ -306,9 +306,9 @@ describe('Discord binding containment (TASK-123 a)', () => { }); it.each([ - ['a proof for another guild', () => mintDiscordInstallProof({ podId: 'pod-1', guildId: OTHER_GUILD })], - ['a proof for a pod the caller is not in', () => mintDiscordInstallProof({ podId: 'pod-9', guildId: GUILD })], - ['an expired proof', () => mintDiscordInstallProof({ podId: 'pod-1', guildId: GUILD }, -60)], + ['a proof for another guild', () => mintDiscordInstallProof({ podId: 'pod-1', guildId: OTHER_GUILD, userId: MEMBER })], + ['a proof for a pod the caller is not in', () => mintDiscordInstallProof({ podId: 'pod-9', guildId: GUILD, userId: MEMBER })], + ['an expired proof', () => mintDiscordInstallProof({ podId: 'pod-1', guildId: GUILD, userId: MEMBER }, -60)], ['a token that is not a proof', () => require('jsonwebtoken').sign({ id: MEMBER }, process.env.JWT_SECRET)], ['a forged proof', () => require('jsonwebtoken').sign({ pur: 'discord-install', pod: 'pod-1', gid: GUILD }, 'not-the-secret')], ])('refuses %s, without an outbound call', async (_label, makeProof) => { @@ -319,16 +319,19 @@ describe('Discord binding containment (TASK-123 a)', () => { }); it('does not authenticate a request on its own', async () => { - const proof = mintDiscordInstallProof({ podId: 'pod-1', guildId: GUILD }); + const proof = mintDiscordInstallProof({ podId: 'pod-1', guildId: GUILD, userId: MEMBER }); expect(require('jsonwebtoken').decode(proof)).not.toHaveProperty('id'); expect(require('jsonwebtoken').decode(proof)).not.toHaveProperty('user'); }); }); describe('GET /api/discord/callback', () => { + // install-link's signed state for MEMBER and pod-1 (a bare pod_ is refused). + const STATE = () => encodeURIComponent(require('../../../utils/discordInstallProof') + .mintDiscordInstallState({ podId: 'pod-1', userId: MEMBER })); it('refuses a malformed guild_id before the code exchange', async () => { const res = await request(app) - .get('/api/discord/callback?code=abc&state=pod_pod-1&guild_id=guild-1'); + .get(`/api/discord/callback?code=abc&state=${STATE()}&guild_id=guild-1`); expect(res.status).toBe(302); expect(res.headers.location).toContain('/discord/error'); @@ -338,7 +341,7 @@ describe('Discord binding containment (TASK-123 a)', () => { it('still completes the exchange for a well-formed guild_id', async () => { const res = await request(app) - .get(`/api/discord/callback?code=abc&state=pod_pod-1&guild_id=${GUILD}`); + .get(`/api/discord/callback?code=abc&state=${STATE()}&guild_id=${GUILD}`); expect(res.status).toBe(302); expect(res.headers.location).toContain('/discord/success'); @@ -350,19 +353,19 @@ describe('Discord binding containment (TASK-123 a)', () => { const { readDiscordInstallProof } = require('../../../utils/discordInstallProof'); const res = await request(app) - .get(`/api/discord/callback?code=abc&state=pod_pod-1&guild_id=${GUILD}`); + .get(`/api/discord/callback?code=abc&state=${STATE()}&guild_id=${GUILD}`); expect(res.status).toBe(302); const location = new URL(res.headers.location); expect(location.pathname).toBe('/discord/success'); - expect(readDiscordInstallProof(location.searchParams.get('install'))).toEqual({ podId: 'pod-1', guildId: GUILD }); + expect(readDiscordInstallProof(location.searchParams.get('install'))).toEqual({ podId: 'pod-1', guildId: GUILD, userId: MEMBER }); }); it('refuses a guild_id that differs from the guild in the token response', async () => { axios.post.mockResolvedValueOnce({ data: { token_type: 'Bearer', guild: { id: OTHER_GUILD } } }); const res = await request(app) - .get(`/api/discord/callback?code=abc&state=pod_pod-1&guild_id=${GUILD}`); + .get(`/api/discord/callback?code=abc&state=${STATE()}&guild_id=${GUILD}`); expect(res.status).toBe(302); expect(res.headers.location).toContain('/discord/error'); @@ -371,7 +374,7 @@ describe('Discord binding containment (TASK-123 a)', () => { it('mints no proof when the token response names no guild', async () => { const res = await request(app) - .get(`/api/discord/callback?code=abc&state=pod_pod-1&guild_id=${GUILD}`); + .get(`/api/discord/callback?code=abc&state=${STATE()}&guild_id=${GUILD}`); expect(res.status).toBe(302); expect(new URL(res.headers.location).searchParams.has('install')).toBe(false); @@ -384,7 +387,7 @@ describe('Discord binding containment (TASK-123 a)', () => { const errorLog = jest.spyOn(console, 'error').mockImplementation(() => {}); try { const res = await request(app) - .get(`/api/discord/callback?code=abc&state=pod_pod-1&guild_id=${GUILD}`); + .get(`/api/discord/callback?code=abc&state=${STATE()}&guild_id=${GUILD}`); expect(axios.post).toHaveBeenCalledWith( 'https://discord.com/api/oauth2/token', expect.anything(), expect.anything(), ); @@ -613,7 +616,7 @@ describe('Discord binding containment (TASK-123 a)', () => { it('accepts a first install on the callback’s proof for this pod and guild, and does not store it', async () => { Integration.findOne.mockResolvedValue(null); - const install = mintDiscordInstallProof({ podId: 'pod-1', guildId: GUILD }); + const install = mintDiscordInstallProof({ podId: 'pod-1', guildId: GUILD, userId: MEMBER }); const res = await post({ ...BODY, install }); @@ -622,9 +625,9 @@ describe('Discord binding containment (TASK-123 a)', () => { }); it.each([ - ['a proof for another pod', () => mintDiscordInstallProof({ podId: 'pod-2', guildId: GUILD })], - ['a proof for another guild', () => mintDiscordInstallProof({ podId: 'pod-1', guildId: OTHER_GUILD })], - ['an expired proof', () => mintDiscordInstallProof({ podId: 'pod-1', guildId: GUILD }, -10)], + ['a proof for another pod', () => mintDiscordInstallProof({ podId: 'pod-2', guildId: GUILD, userId: MEMBER })], + ['a proof for another guild', () => mintDiscordInstallProof({ podId: 'pod-1', guildId: OTHER_GUILD, userId: MEMBER })], + ['an expired proof', () => mintDiscordInstallProof({ podId: 'pod-1', guildId: GUILD, userId: MEMBER }, -10)], ['a session token in the proof field', () => 'user-token'], ])('refuses %s', async (_label, make) => { Integration.findOne.mockResolvedValue(null); diff --git a/backend/__tests__/unit/routes/discordChannelsRateLimit.test.js b/backend/__tests__/unit/routes/discordChannelsRateLimit.test.js index 70767a6565..337bbb1654 100644 --- a/backend/__tests__/unit/routes/discordChannelsRateLimit.test.js +++ b/backend/__tests__/unit/routes/discordChannelsRateLimit.test.js @@ -107,7 +107,10 @@ it('redirects a failed Discord OAuth exchange even without FRONTEND_URL', async axios.post.mockRejectedValueOnce(new Error('exchange failed')); const log = jest.spyOn(console, 'error').mockImplementation(() => {}); try { - const res = await request(app).get('/api/discord/callback?code=oauth-code&state=pod_123'); + process.env.JWT_SECRET = process.env.JWT_SECRET || 'test-secret'; + const state = encodeURIComponent(require('../../../utils/discordInstallProof') + .mintDiscordInstallState({ podId: '123', userId: 'user-1' })); + const res = await request(app).get(`/api/discord/callback?code=oauth-code&state=${state}`); expect(res.status).toBe(302); expect(res.headers.location).toBe('http://localhost:3000/discord/error?error=OAuth%20authorization%20failed'); } finally { diff --git a/backend/__tests__/unit/routes/integrations.discordTokenCopy.test.js b/backend/__tests__/unit/routes/integrations.discordTokenCopy.test.js index 9e9c344205..303c92178f 100644 --- a/backend/__tests__/unit/routes/integrations.discordTokenCopy.test.js +++ b/backend/__tests__/unit/routes/integrations.discordTokenCopy.test.js @@ -167,7 +167,9 @@ describe('discord connector stores no bot token copy (TASK-124)', () => { .set('x-test-user', String(joiner._id)) .send({ podId: String(pod._id), type: 'discord', config: { serverId: GUILD, channelId: CHANNEL } }); - expect(res.status).toBe(404); + // Refused at the pod check now (only the creator or an admin binds a + // channel); before that, at the guild check. Either way nothing is written. + expect(res.status).toBe(403); expect(axios.post).not.toHaveBeenCalled(); expect(await DiscordIntegration.countDocuments({ channelId: CHANNEL })).toBe(0); }); diff --git a/backend/routes/discord.ts b/backend/routes/discord.ts index cd041d3ed6..707b81841e 100644 --- a/backend/routes/discord.ts +++ b/backend/routes/discord.ts @@ -25,7 +25,9 @@ const { runDiscordCommandForIntegrations } = require('../services/discordMultiCo import { listIntegrationsRateLimit, writeIntegrationsRateLimit } from '../middleware/integrationRateLimit'; import { platformIpRateLimit } from '../middleware/platformRateLimit'; import { invalidDiscordIdError, isDiscordSnowflake, isSupplied } from '../utils/discordBinding'; -import { mintDiscordInstallProof, readDiscordInstallProof } from '../utils/discordInstallProof'; +import { + mintDiscordInstallProof, mintDiscordInstallState, readDiscordInstallProof, readDiscordInstallState, +} from '../utils/discordInstallProof'; interface AuthReq { user?: { id: string; role?: string }; @@ -63,6 +65,16 @@ function verifySignature(req: { headers: Record; bod } } +// The bot permissions the install asks for (the value the legacy room link +// uses): view channels, send messages, manage webhooks. +const DISCORD_BOT_PERMISSIONS = '536873984'; + +// One redirect for the authorize request and the code exchange: Discord refuses +// an exchange whose redirect_uri differs from the one the user authorized. +const discordRedirectUri = (): string => ( + `${String(process.env.BACKEND_URL || 'http://localhost:5000').replace(/\/+$/, '')}/api/discord/callback` +); + async function canManagePod(podId: string, userId: string): Promise { const [user, pod] = await Promise.all([User.findById(userId), Pod.findById(podId)]) as [{ role?: string } | null, { createdBy?: { toString: () => string } } | null]; if (!user || !pod) return false; @@ -121,6 +133,7 @@ router.get('/channels/:guildId', listIntegrationsRateLimit, auth, async (req: Au // stands in for one: same guild, and a pod the caller belongs to (F89). const proof = binding ? null : readDiscordInstallProof(req.query?.install); const provenInstall = !!proof && proof.guildId === guildId + && proof.userId === String(req.user?.id) && podIds.some((podId: unknown) => String(podId) === proof.podId); // One answer for a guild that is bound elsewhere and one that does not // exist: the route must not become an oracle for either. @@ -209,13 +222,24 @@ router.post('/interactions', async (req: AuthReq & { body: Buffer }, res: Res) = router.get('/install-link/:podId', auth, async (req: AuthReq, res: Res) => { try { const { podId } = req.params || {}; - const clientId = req.query?.clientId || process.env.DISCORD_CLIENT_ID; + // The instance's own application only. A caller-chosen client id would send + // the bot add to an app whose callback is not ours. + const clientId = process.env.DISCORD_CLIENT_ID; const canManage = await canManagePod(podId || '', req.user?.id || ''); if (!canManage) return res.status(403).json({ error: 'Access denied' }); if (!clientId) return res.status(400).json({ error: 'Discord client ID is required' }); - const baseUrl = 'https://discord.com/api/oauth2/authorize'; - const installUrl = `${baseUrl}?client_id=${clientId}&scope=bot%20applications.commands&permissions=536873984&state=pod_${podId}`; - res.json({ installUrl, podId, status: 'ready' }); + // A code grant with our callback as the redirect. Without `response_type` + // and `redirect_uri`, Discord adds the bot and stops on its own page, so the + // callback never runs: no install proof, no channel picker, no binding (walked + // on prod 2026-11-04). + const installUrl = new URL('https://discord.com/api/oauth2/authorize'); + installUrl.searchParams.set('client_id', String(clientId)); + installUrl.searchParams.set('scope', 'bot applications.commands'); + installUrl.searchParams.set('permissions', DISCORD_BOT_PERMISSIONS); + installUrl.searchParams.set('response_type', 'code'); + installUrl.searchParams.set('redirect_uri', discordRedirectUri()); + installUrl.searchParams.set('state', mintDiscordInstallState({ podId: String(podId), userId: String(req.user?.id) })); + res.json({ installUrl: installUrl.toString(), podId, status: 'ready' }); } catch { console.error('Error generating installation link'); res.status(500).json({ error: 'Failed to generate installation link' }); @@ -327,8 +351,11 @@ router.get('/callback', discordCallbackLimit, async (req: AuthReq, res: Res) => try { const { code, state, guild_id: guildId } = req.query || {}; if (!code) return res.redirect(`${frontend}/discord/error?error=No authorization code received`); - const podId = state?.replace('pod_', ''); - if (!podId) return res.redirect(`${frontend}/discord/error?error=Invalid state parameter`); + // A signed state from install-link, never a bare pod id (see + // utils/discordInstallProof.ts). Refused before the code is exchanged. + const installState = readDiscordInstallState(state); + if (!installState) return res.redirect(`${frontend}/discord/error?error=Invalid state parameter`); + const { podId, userId: installerId } = installState; // Unauthenticated route, so `guild_id` here is request-tainted input on a URL // built with the instance bot token. Refused before the code exchange: a // malformed id must not reach Discord at all, and the same value is forwarded @@ -336,7 +363,7 @@ router.get('/callback', discordCallbackLimit, async (req: AuthReq, res: Res) => if (isSupplied(guildId) && !isDiscordSnowflake(guildId)) { return res.redirect(`${frontend}/discord/error?error=Invalid guild id`); } - const tokenResponse = await axios.post('https://discord.com/api/oauth2/token', { client_id: process.env.DISCORD_CLIENT_ID, client_secret: process.env.DISCORD_CLIENT_SECRET, grant_type: 'authorization_code', code, redirect_uri: `${process.env.BACKEND_URL || 'http://localhost:5000'}/api/discord/callback` }, { headers: { 'Content-Type': 'application/x-www-form-urlencoded' } }); + const tokenResponse = await axios.post('https://discord.com/api/oauth2/token', { client_id: process.env.DISCORD_CLIENT_ID, client_secret: process.env.DISCORD_CLIENT_SECRET, grant_type: 'authorization_code', code, redirect_uri: discordRedirectUri() }, { headers: { 'Content-Type': 'application/x-www-form-urlencoded' } }); // The token response names the guild the bot was added to. `guild_id` in the // query is only Discord's hint and anyone can edit it, so a disagreement is a // refusal, and only the token response's guild earns an install proof (F89). @@ -362,7 +389,7 @@ router.get('/callback', discordCallbackLimit, async (req: AuthReq, res: Res) => successUrl.searchParams.append('pod_id', podId); successUrl.searchParams.append('guild_id', installedGuildId || ''); successUrl.searchParams.append('server_name', serverName); - if (provenGuildId) successUrl.searchParams.append('install', mintDiscordInstallProof({ podId, guildId: provenGuildId })); + if (provenGuildId) successUrl.searchParams.append('install', mintDiscordInstallProof({ podId, guildId: provenGuildId, userId: installerId })); res.redirect(successUrl.toString()); } catch { console.error('Discord OAuth callback error'); diff --git a/backend/routes/integrations.ts b/backend/routes/integrations.ts index 764f9edde8..0293b86269 100644 --- a/backend/routes/integrations.ts +++ b/backend/routes/integrations.ts @@ -87,6 +87,7 @@ const hasDiscordGuildAuthority = async ( })) return true; const proof = readDiscordInstallProof(install); return !!proof && proof.guildId === guildId && proof.podId === podId + && proof.userId === String(userId) && podIds.some((id: unknown) => String(id) === podId); }; @@ -557,6 +558,15 @@ router.post('/', writeIntegrationsRateLimit, auth, async (req: AuthReq, res: Res if (type === 'discord') { // Authority, then the channel: both before the row or the webhook exist. // One answer for both refusals, as on the channel list. + // A channel bridge is a pod-level setting: binding one takes authority + // over the POD, the same as the install link (routes/discord.ts + // canManagePod: the pod's creator or an instance admin). Guild authority + // alone is not enough: the prior-binding arm below proves the guild, and + // on its own let a plain member bridge a pod they did not create. + const binder = await User.findById(String(req.user?.id)) as { role?: string } | null; + if (binder?.role !== 'admin' && targetPod.createdBy?.toString() !== String(req.user?.id)) { + return res.status(403).json({ message: 'Only the pod creator can connect Discord to this pod' }); + } const guildId = String(config.serverId); const install = (req.body as { install?: unknown })?.install; if (!await hasDiscordGuildAuthority(req.user?.id, String(podId), guildId, install) diff --git a/backend/utils/discordInstallProof.ts b/backend/utils/discordInstallProof.ts index 8a8b1351b0..7c5e69f354 100644 --- a/backend/utils/discordInstallProof.ts +++ b/backend/utils/discordInstallProof.ts @@ -24,6 +24,8 @@ interface DiscordInstallProofPayload extends JwtPayload { pur: typeof PURPOSE; pod: string; gid: string; + // The person whose install this is: the state's user. Only they may spend it. + uid: string; } const secret = (): string => { @@ -33,20 +35,63 @@ const secret = (): string => { }; export const mintDiscordInstallProof = ( - { podId, guildId }: { podId: string; guildId: string }, + { podId, guildId, userId }: { podId: string; guildId: string; userId: string }, ttlSeconds: number = DISCORD_INSTALL_PROOF_TTL_SECONDS, ): string => { - const payload: DiscordInstallProofPayload = { pur: PURPOSE, pod: podId, gid: guildId }; + const payload: DiscordInstallProofPayload = { pur: PURPOSE, pod: podId, gid: guildId, uid: userId }; return jwt.sign(payload, secret(), { expiresIn: ttlSeconds }); }; -/** The proof's (pod, guild), or null for anything that is not a live proof. */ -export const readDiscordInstallProof = (token: unknown): { podId: string; guildId: string } | null => { +/** + * The proof's (pod, guild, user), or null for anything that is not a live proof. + * A proof without a user (minted before the state was signed) is not a proof. + */ +export const readDiscordInstallProof = (token: unknown): { podId: string; guildId: string; userId: string } | null => { if (typeof token !== 'string' || !token) return null; try { const decoded = jwt.verify(token, secret()) as DiscordInstallProofPayload; if (decoded.pur !== PURPOSE || typeof decoded.pod !== 'string' || !isDiscordSnowflake(decoded.gid)) return null; - return { podId: decoded.pod, guildId: decoded.gid }; + if (typeof decoded.uid !== 'string' || !decoded.uid) return null; + return { podId: decoded.pod, guildId: decoded.gid, userId: decoded.uid }; + } catch { + return null; + } +}; + +// ---- The install state ---- +// The OAuth `state` the install link sends Discord, and Discord hands back to the +// callback. The callback has no session, so this is the only thing that ties a +// code to the person who asked for the install. It used to be the bare string +// `pod_`: a crafted callback link carrying an attacker's code and a victim's +// pod minted a proof that the victim's browser then spent binding their pod to +// the attacker's guild. Signed, short-lived and naming the user, it can only be +// obtained from install-link by someone allowed to install into that pod, and the +// proof it leads to can be spent only by that same person. +const STATE_PURPOSE = 'discord-install-state'; +export const DISCORD_INSTALL_STATE_TTL_SECONDS = 15 * 60; + +interface DiscordInstallStatePayload extends JwtPayload { + pur: typeof STATE_PURPOSE; + pod: string; + uid: string; +} + +export const mintDiscordInstallState = ( + { podId, userId }: { podId: string; userId: string }, + ttlSeconds: number = DISCORD_INSTALL_STATE_TTL_SECONDS, +): string => { + const payload: DiscordInstallStatePayload = { pur: STATE_PURPOSE, pod: podId, uid: userId }; + return jwt.sign(payload, secret(), { expiresIn: ttlSeconds }); +}; + +/** The state's (pod, user), or null for anything that is not a live state. */ +export const readDiscordInstallState = (token: unknown): { podId: string; userId: string } | null => { + if (typeof token !== 'string' || !token) return null; + try { + const decoded = jwt.verify(token, secret()) as DiscordInstallStatePayload; + if (decoded.pur !== STATE_PURPOSE || typeof decoded.pod !== 'string' || !decoded.pod) return null; + if (typeof decoded.uid !== 'string' || !decoded.uid) return null; + return { podId: decoded.pod, userId: decoded.uid }; } catch { return null; } diff --git a/frontend/src/components/ChatRoom.tsx b/frontend/src/components/ChatRoom.tsx index 2c3fe9e140..2a838be28d 100644 --- a/frontend/src/components/ChatRoom.tsx +++ b/frontend/src/components/ChatRoom.tsx @@ -1145,24 +1145,29 @@ const ChatRoom = () => { } }; - const getDiscordOAuthUrl = (podId, guildId = null) => { - const clientId = process.env.REACT_APP_DISCORD_CLIENT_ID; - const redirectUri = encodeURIComponent(`${getApiBaseUrl()}/api/discord/callback`); - const scopes = encodeURIComponent('bot applications.commands'); - const permissions = '536873984'; - const state = `pod_${podId}`; - const timestamp = Date.now(); - return `https://discord.com/api/oauth2/authorize?client_id=${clientId}&scope=${scopes}&permissions=${permissions}` - + `&redirect_uri=${redirectUri}&response_type=code&state=${state}` - + (guildId ? `&guild_id=${guildId}` : '') - + `&t=${timestamp}`; + // The install link comes from the server: its OAuth state is signed and names + // the person asking, and the callback refuses any other state. Opened in a tab + // synchronously (so the popup blocker allows it), then pointed at the link. + const openDiscordInstall = async (guildId = null) => { + if (!roomId) return; + const tab = window.open('', '_blank'); + try { + const token = localStorage.getItem('token'); + const res = await axios.get(`/api/discord/install-link/${roomId}`, { + headers: { Authorization: `Bearer ${token}` }, + }); + const url = new URL(res.data.installUrl); + if (guildId) url.searchParams.set('guild_id', guildId); + if (tab) tab.location.href = url.toString(); + else window.location.assign(url.toString()); + } catch (err) { + if (tab) tab.close(); + console.warn('Failed to get the Discord install link:', err.response?.status); + } }; const getIntegrationRedirectUrl = (type) => { if (!roomId) return '#'; - if (type === 'discord') { - return getDiscordOAuthUrl(roomId); - } const externalSetupLinks = { slack: 'https://api.slack.com/apps', groupme: 'https://dev.groupme.com/bots/new', @@ -1731,7 +1736,7 @@ const ChatRoom = () => { if (config.serverId && config.channelId) { return `https://discord.com/channels/${config.serverId}/${config.channelId}`; } - return getDiscordOAuthUrl(roomId, config.serverId); + return null; case 'slack': return 'https://api.slack.com/apps'; case 'groupme': @@ -1764,6 +1769,10 @@ const ChatRoom = () => { return; } const url = getIntegrationManageUrl(type, integration); + if (!url && type === 'discord') { + openDiscordInstall(integration?.config?.serverId || null); + return; + } if (url) { window.open(url, '_blank', 'noopener,noreferrer'); } @@ -3626,7 +3635,10 @@ const ChatRoom = () => { target={['groupme', 'telegram', 'x', 'instagram'].includes(item.id) ? undefined : '_blank'} rel={['groupme', 'telegram', 'x', 'instagram'].includes(item.id) ? undefined : 'noopener noreferrer'} onClick={(event) => { - if (item.id === 'groupme') { + if (item.id === 'discord') { + event.preventDefault(); + openDiscordInstall(); + } else if (item.id === 'groupme') { event.preventDefault(); handleGroupmeSetupOpen(); } else if (item.id === 'telegram') { diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index dce54835b6..40efda307d 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -2520,6 +2520,8 @@ "availableLine": "Connect {{label}} to a pod.", "availableSlack": "one click in your workspace", "availableTelegram": "one message", + "availableDiscord": "add the bot to your server", + "linkInstallCreatorOnly": "Only the person who created this pod can add the {{connector}} bot to it.", "cancel": "Cancel", "channelDetails": "Channel details", "closeDetails": "Close", diff --git a/frontend/src/i18n/locales/zh-CN.json b/frontend/src/i18n/locales/zh-CN.json index e1540e3afe..5db387a8c0 100644 --- a/frontend/src/i18n/locales/zh-CN.json +++ b/frontend/src/i18n/locales/zh-CN.json @@ -2508,6 +2508,8 @@ "availableLine": "将 {{label}} 连接到 Pod。", "availableSlack": "在你的工作空间中点一下", "availableTelegram": "发一条消息", + "availableDiscord": "把机器人加进你的服务器", + "linkInstallCreatorOnly": "只有这个 Pod 的创建者才能把 {{connector}} 机器人加进来。", "cancel": "取消", "channelDetails": "频道详情", "closeDetails": "关闭", diff --git a/frontend/src/v2/__tests__/ConnectorsPage.channels.test.tsx b/frontend/src/v2/__tests__/ConnectorsPage.channels.test.tsx index 59e245c501..94ae37365f 100644 --- a/frontend/src/v2/__tests__/ConnectorsPage.channels.test.tsx +++ b/frontend/src/v2/__tests__/ConnectorsPage.channels.test.tsx @@ -1190,18 +1190,18 @@ describe('Connectors page: channels', () => { }); it('offers a rostered provider with Add, and an unmapped one inherits no onboarding sentence', async () => { - // Slack's line is in the map; Discord's is not. The point of the map is that - // Discord renders NO sentence rather than Slack's. + // Slack's line is in the map; GroupMe's is not. The point of the map is that + // GroupMe renders NO sentence rather than Slack's. mockCatalog([ - entry({ installableId: 'discord', label: 'Discord', available: true, offered: true }), + entry({ installableId: 'groupme', label: 'GroupMe', available: true, offered: true }), entry({ installableId: 'slack', label: 'Slack', available: true, offered: true }), ]); renderPage(); - const discord = await openPopover('Discord'); - expect(discord.textContent).not.toContain('one click in your workspace'); - expect(discord.textContent).not.toContain('one message'); - expect(within(discord).getByRole('button', { name: 'Add' })).toBeInTheDocument(); + const groupme = await openPopover('GroupMe'); + expect(groupme.textContent).not.toContain('one click in your workspace'); + expect(groupme.textContent).not.toContain('one message'); + expect(within(groupme).getByRole('button', { name: 'Add' })).toBeInTheDocument(); const slack = await openPopover('Slack'); expect(slack.textContent).toContain('one click in your workspace'); @@ -1210,19 +1210,20 @@ describe('Connectors page: channels', () => { }); it('does not list a provider this instance does not offer (D215)', async () => { - // Discord on commonly.me: usable keys, no builtin Installable row, never - // walked. A stranger must not see it, not even behind "+N off". + // Usable keys, no builtin Installable row, and no install link of its own. + // A stranger must not see it, not even behind "+N off". (Discord was this + // example until it got a link install; see the Discord block below.) mockCatalog([ entry(), - entry({ installableId: 'discord', label: 'Discord', available: true, offered: false }), + entry({ installableId: 'groupme', label: 'GroupMe', available: true, offered: false }), ]); renderPage(); await findTile('Telegram'); // The off button only exists when a dead tile does, so its absence is the - // assertion; a collapsed group would hide a listed Discord from the query. + // assertion; a collapsed group would hide a listed GroupMe from the query. expect(screen.queryByRole('button', { name: /^\+\d+ off$/ })).toBeNull(); - expect(tileNode('Discord')).toBeNull(); + expect(tileNode('GroupMe')).toBeNull(); }); it('keeps an installed but no-longer-offered provider as not connectable, with no Add anywhere', async () => { @@ -1230,17 +1231,17 @@ describe('Connectors page: channels', () => { // the roster row went away: it states the state and offers no action. Before // #1826 the row rendered an Add whose click ended in 404 installable_not_found. mockCatalog([ - entry({ installableId: 'discord', label: 'Discord', available: true, offered: false, installation: { status: 'active', boundPodId: 'p1' } }), + entry({ installableId: 'groupme', label: 'GroupMe', available: true, offered: false, installation: { status: 'active', boundPodId: 'p1' } }), ]); renderPage(); await pageReady(); await expandOff('Channels'); - const tile = await findTile('Discord'); + const tile = await findTile('GroupMe'); expect(tile).toHaveClass('v2-cx-tile--dim'); expect(tile.querySelector('svg')).not.toBeNull(); fireEvent.click(tile); - const drawer = await findDrawer('Discord'); + const drawer = await findDrawer('GroupMe'); expect(within(drawer).getByText('Unavailable here.')).toBeInTheDocument(); // No Add and no connect verb: an offered provider is the only thing that may // produce them, and there is none in this catalog. @@ -1255,15 +1256,96 @@ describe('Connectors page: channels', () => { // went away can leave an integration with no installation. Dropping it from // the list would hide a live connection from its owner. mockCatalog([ - entry({ installableId: 'discord', label: 'Discord', available: true, offered: false, integration: liveIntegration() }), + entry({ installableId: 'groupme', label: 'GroupMe', available: true, offered: false, integration: liveIntegration() }), ]); renderPage(); await pageReady(); await expandOff('Channels'); - expect(await findTile('Discord')).toHaveClass('v2-cx-tile--dim'); + expect(await findTile('GroupMe')).toHaveClass('v2-cx-tile--dim'); }); + describe('Discord: a link install, offered on its keys alone', () => { + // Discord has no builtin Installable row (the catalog says offered: false), + // but its install is the bot-add link plus Discord's callback, so it is + // offered wherever the instance holds its keys (available). Connect sends + // the browser to the link the server builds for the chosen pod; it never + // calls the Installable lifecycle, which would 422 provider_not_offered. + const realLocation = window.location; + let assign; + beforeEach(() => { + assign = jest.fn(); + Object.defineProperty(window, 'location', { configurable: true, value: { ...realLocation, assign } }); + }); + afterEach(() => { + Object.defineProperty(window, 'location', { configurable: true, value: realLocation }); + }); + + const discordEntry = entry({ installableId: 'discord', label: 'Discord', description: '', available: true, offered: false }); + const INSTALL_URL = 'https://discord.com/api/oauth2/authorize?client_id=1&response_type=code'; + + it('lists Discord with Add and its own onboarding line', async () => { + mockCatalog([entry(), discordEntry]); + renderPage(); + + const popover = await openPopover('Discord'); + expect(popover.textContent).toContain('add the bot to your server'); + expect(popover.textContent).not.toContain('Unavailable here.'); + expect(within(popover).getByRole('button', { name: 'Add' })).toBeInTheDocument(); + }); + + it('offers Discord beside Telegram and Slack in Connect a channel, and Connect leaves for its install link', async () => { + mockApi({ + installables: [entry(), entry({ installableId: 'slack', label: 'Slack', offered: true }), discordEntry], + pods: [{ _id: 'p1', name: 'Rewire Live Demo', type: 'chat', members: [] }], + route: (url) => (url === '/api/discord/install-link/p1' + ? Promise.resolve({ data: { installUrl: INSTALL_URL, podId: 'p1', status: 'ready' } }) + : undefined), + }); + renderPage(); + + fireEvent.click(await findTile('Channel')); + const drawer = await findDrawer('Connect a channel'); + const providers = within(drawer).getByRole('group', { name: 'Channel provider' }); + expect(within(providers).getAllByRole('button').map((b) => b.textContent)).toEqual(['Telegram', 'Slack', 'Discord']); + fireEvent.click(within(providers).getByRole('button', { name: 'Discord' })); + fireEvent.click(within(drawer).getByRole('button', { name: 'Connect' })); + + await waitFor(() => expect(assign).toHaveBeenCalledWith(INSTALL_URL)); + expect(axios.get).toHaveBeenCalledWith('/api/discord/install-link/p1', expect.anything()); + expect(axios.post).not.toHaveBeenCalledWith('/api/installables/discord/install', expect.anything(), expect.anything()); + }); + + it('does not list Discord on an instance without its keys', async () => { + mockCatalog([entry(), entry({ installableId: 'discord', label: 'Discord', available: false, offered: false })]); + renderPage(); + + await findTile('Telegram'); + expect(screen.queryByRole('button', { name: /^\+\d+ off$/ })).toBeNull(); + expect(tileNode('Discord')).toBeNull(); + }); + + it('says who may add the bot when the server refuses the link', async () => { + mockApi({ + installables: [discordEntry], + pods: [{ _id: 'p1', name: 'Rewire Live Demo', type: 'chat', members: [] }], + route: (url) => (url === '/api/discord/install-link/p1' + ? Promise.reject(Object.assign(new Error('403'), { response: { status: 403, data: { error: 'Access denied' } } })) + : undefined), + }); + renderPage(); + + const popover = await openPopover('Discord'); + fireEvent.click(within(popover).getByRole('button', { name: 'Add' })); + const drawer = await findDrawer('Connect a channel'); + fireEvent.click(within(drawer).getByRole('button', { name: 'Connect' })); + + expect(await screen.findByText('Only the person who created this pod can add the Discord bot to it.')).toBeInTheDocument(); + expect(assign).not.toHaveBeenCalled(); + }); + }); + + it('renders an unavailable provider with its blocker and an available one with Add', async () => { mockCatalog([ entry(), diff --git a/frontend/src/v2/connectors/ConnectorsPage.tsx b/frontend/src/v2/connectors/ConnectorsPage.tsx index 4a2f9ac408..4febb992bf 100644 --- a/frontend/src/v2/connectors/ConnectorsPage.tsx +++ b/frontend/src/v2/connectors/ConnectorsPage.tsx @@ -7,7 +7,7 @@ import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { useTranslation } from 'react-i18next'; import { useNavigate } from 'react-router-dom'; import { catalogDescription } from '../utils/connectorCatalogDescription'; -import { ADD_PLATFORMS, TYPE_LABELS } from './channelModel'; +import { ADD_PLATFORMS, TYPE_LABELS, entryOffered } from './channelModel'; import type { ListItem } from './channelModel'; import { PER_ENTRY_REVOKE_COPY, entryCovers, entryIsGranted, isDead } from './toolModel'; import type { ToolCatalogEntry, ToolGrant } from './toolModel'; @@ -89,7 +89,7 @@ const ConnectorsPage: React.FC = () => { (connector) => connector.type === type && Boolean(connector.installationId), ); const availableProviders = c.catalog - ? c.catalog.filter((entry) => entry.available && entry.offered !== false && !entry.installation).map((entry) => ({ type: entry.installableId })) + ? c.catalog.filter((entry) => entry.available && entryOffered(entry) && !entry.installation).map((entry) => ({ type: entry.installableId })) : ADD_PLATFORMS.filter((provider) => provider.enabled && !hasInstallation(provider.type)); // ---- tools ---- diff --git a/frontend/src/v2/connectors/channelModel.ts b/frontend/src/v2/connectors/channelModel.ts index 8410922bc8..dbc2b7519d 100644 --- a/frontend/src/v2/connectors/channelModel.ts +++ b/frontend/src/v2/connectors/channelModel.ts @@ -175,8 +175,24 @@ export const mappedTypeLabel = (type: string): string | null => TYPE_LABELS[type export const CONNECT_DETAIL: Record = { telegram: { key: 'connectors.availableTelegram', defaultValue: 'one message' }, slack: { key: 'connectors.availableSlack', defaultValue: 'one click in your workspace' }, + discord: { key: 'connectors.availableDiscord', defaultValue: 'add the bot to your server' }, }; +// Providers whose Connect sends the person to the provider's own install page +// instead of the Installable lifecycle. No builtin row is installed for them, so +// the catalog reports `offered: false`, but the install link and the provider's +// callback are the whole flow and `available` already says this instance holds +// their keys. Discord: the bot add returns to /api/discord/callback, and the +// success page binds the channel. +export const LINK_INSTALL_PROVIDERS = new Set(['discord']); + +// Offered for a channel picker: a rostered provider, or a link-install one +// whose keys this instance holds. Without the keys a link-install provider stays +// unlisted, exactly like an unrostered one. +export const entryOffered = (entry: { installableId: string; offered?: boolean; available?: boolean }): boolean => ( + entry.offered !== false || (entry.available === true && LINK_INSTALL_PROVIDERS.has(entry.installableId)) +); + // Fallback only: when the catalog cannot be read the page still offers the // two providers it can drive, exactly as it did before the catalog existed. export const ADD_PLATFORMS = [ diff --git a/frontend/src/v2/connectors/useChannelConnectors.ts b/frontend/src/v2/connectors/useChannelConnectors.ts index 5cada5161a..a6796b04cf 100644 --- a/frontend/src/v2/connectors/useChannelConnectors.ts +++ b/frontend/src/v2/connectors/useChannelConnectors.ts @@ -27,6 +27,8 @@ import { codeExpiresInMinutes, claimIsStale, projectionMissing, + LINK_INSTALL_PROVIDERS, + entryOffered, } from './channelModel'; import type { ConnectorGate, @@ -378,11 +380,37 @@ export const useChannelConnectors = () => { } }; + // A link-install provider's Connect: the server builds the provider's install + // URL for this pod, and the browser leaves for it. The provider sends the + // person back to its callback, which lands on the success page that binds the + // channel. Only the pod's creator (or an admin) may add a bot to a pod. + const linkInstall = async (type: string, podId: string): Promise => { + try { + const result = await api.get<{ installUrl?: string }>(`/api/${type}/install-link/${encodeURIComponent(podId)}`); + if (!result?.installUrl) throw new Error('no install link'); + window.location.assign(result.installUrl); + return true; + } catch (requestError) { + const response = installErrorResponse(requestError); + setError(response.status === 403 + ? t('connectors.linkInstallCreatorOnly', { + defaultValue: 'Only the person who created this pod can add the {{connector}} bot to it.', + connector: TYPE_LABELS[type] || type, + }) + : t('connectors.createError', { defaultValue: 'Could not create the connector.' })); + return false; + } + }; + const createConnector = async () => { if (!newPodId || !addingType || creating) return; setCreating(true); setError(null); try { + if (LINK_INSTALL_PROVIDERS.has(addingType)) { + await linkInstall(addingType, newPodId); + return; + } const installed = await install(addingType, newPodId); if (installed) setAddingType(null); } finally { @@ -765,7 +793,7 @@ export const useChannelConnectors = () => { // about this instance's keys, not about whether a row exists to install. // Showing the Add anyway is what produced a lowercase `discord` row whose // click ended in 404 installable_not_found. - if (entry.offered === false) { + if (!entryOffered(entry)) { return { action: null, detail: '', @@ -910,7 +938,7 @@ export const useChannelConnectors = () => { // offerability contract), and someone who already installed it keeps their // row, which renders in the not-connectable state below. const listedCatalog = (catalog || []).filter((entry) => ( - entry.offered !== false || Boolean(entry.installation) || Boolean(entry.integration) + entryOffered(entry) || Boolean(entry.installation) || Boolean(entry.integration) )); const allItems: ListItem[] = [ ...listedCatalog.map((entry): ListItem => ({