diff --git a/backend/__tests__/unit/routes/discordBindingContainment.test.js b/backend/__tests__/unit/routes/discordBindingContainment.test.js index a5dbd6780..3cd3ca835 100644 --- a/backend/__tests__/unit/routes/discordBindingContainment.test.js +++ b/backend/__tests__/unit/routes/discordBindingContainment.test.js @@ -168,6 +168,7 @@ describe('Discord binding containment (TASK-123 a)', () => { process.env.DISCORD_BOT_TOKEN = 'bot-secret'; process.env.DISCORD_CLIENT_ID = 'client-id'; process.env.DISCORD_CLIENT_SECRET = 'client-secret'; + process.env.JWT_SECRET = 'test-jwt-secret'; // The create path encrypts the webhook URL (TASK-124 part 2), so a connector // secret ring is part of the deployment this route runs against. process.env.CONNECTOR_SECRET_KEYS = `k1:${Buffer.alloc(32, 1).toString('base64')}`; @@ -277,6 +278,53 @@ describe('Discord binding containment (TASK-123 a)', () => { }); }); + // F89: on a first install no binding exists yet, so the channel picker reads + // through the proof the OAuth callback minted. It is scoped to one pod and one + // guild, the guild taken from Discord's token response, and the caller must + // still belong to that pod. + describe('GET /api/discord/channels/:guildId on a first install', () => { + const { mintDiscordInstallProof } = require('../../../utils/discordInstallProof'); + const firstInstall = (proof) => { + Integration.findOne.mockResolvedValue(null); + return request(app) + .get(`/api/discord/channels/${GUILD}${proof === undefined ? '' : `?install=${encodeURIComponent(proof)}`}`) + .set('Authorization', 'Bearer user-token'); + }; + + it('lists channels for a member of the pod the proof names', async () => { + const res = await firstInstall(mintDiscordInstallProof({ podId: 'pod-1', guildId: GUILD })); + + expect(res.status).toBe(200); + expect(res.body).toEqual([{ id: CHANNEL, name: 'general', topic: undefined }]); + }); + + it('still answers 404 with no proof', async () => { + const res = await firstInstall(undefined); + + expect(res.status).toBe(404); + expect(outboundCalls()).toBe(0); + }); + + 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 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) => { + const res = await firstInstall(makeProof()); + + expect(res.status).toBe(404); + expect(outboundCalls()).toBe(0); + }); + + it('does not authenticate a request on its own', async () => { + const proof = mintDiscordInstallProof({ podId: 'pod-1', guildId: GUILD }); + expect(require('jsonwebtoken').decode(proof)).not.toHaveProperty('id'); + expect(require('jsonwebtoken').decode(proof)).not.toHaveProperty('user'); + }); + }); + describe('GET /api/discord/callback', () => { it('refuses a malformed guild_id before the code exchange', async () => { const res = await request(app) @@ -297,6 +345,38 @@ describe('Discord binding containment (TASK-123 a)', () => { expect(axios.post.mock.calls[0][0]).toBe('https://discord.com/api/oauth2/token'); }); + it('hands the success page a proof for the guild Discord names in the token response', async () => { + axios.post.mockResolvedValueOnce({ data: { token_type: 'Bearer', guild: { id: GUILD, name: 'Guild' } } }); + const { readDiscordInstallProof } = require('../../../utils/discordInstallProof'); + + const res = await request(app) + .get(`/api/discord/callback?code=abc&state=pod_pod-1&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 }); + }); + + 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}`); + + expect(res.status).toBe(302); + expect(res.headers.location).toContain('/discord/error'); + expect(axios.get).not.toHaveBeenCalled(); + }); + + 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}`); + + expect(res.status).toBe(302); + expect(new URL(res.headers.location).searchParams.has('install')).toBe(false); + }); + it('redirects a post-exchange guild failure without logging provider data', async () => { const sentinel = 'dummy-bot-token-sentinel'; axios.get.mockRejectedValueOnce({ response: { data: { detail: sentinel } } }); diff --git a/backend/routes/discord.ts b/backend/routes/discord.ts index 45424cf7d..cd041d3ed 100644 --- a/backend/routes/discord.ts +++ b/backend/routes/discord.ts @@ -25,6 +25,7 @@ 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'; interface AuthReq { user?: { id: string; role?: string }; @@ -116,9 +117,14 @@ router.get('/channels/:guildId', listIntegrationsRateLimit, auth, async (req: Au const binding = podIds.length ? await Integration.findOne({ type: 'discord', 'config.serverId': guildId, podId: { $in: podIds } }) : null; + // A first install has no binding yet. The proof the OAuth callback minted + // 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 + && 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. - if (!binding) return res.status(404).json({ message: 'Guild not found' }); + if (!binding && !provenInstall) return res.status(404).json({ message: 'Guild not found' }); const response = await axios.get(`https://discord.com/api/guilds/${guildId}/channels`, { headers: { Authorization: `Bot ${process.env.DISCORD_BOT_TOKEN}` } }); const textChannels = (response.data as Array<{ type: number; id: string; name: string; topic?: string }>) .filter((channel) => channel.type === 0) @@ -330,11 +336,20 @@ router.get('/callback', discordCallbackLimit, async (req: AuthReq, res: Res) => if (isSupplied(guildId) && !isDiscordSnowflake(guildId)) { return res.redirect(`${frontend}/discord/error?error=Invalid guild id`); } - 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: `${process.env.BACKEND_URL || 'http://localhost:5000'}/api/discord/callback` }, { 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). + const tokenGuildId = (tokenResponse?.data as { guild?: { id?: unknown } } | undefined)?.guild?.id; + const provenGuildId = isDiscordSnowflake(tokenGuildId) ? String(tokenGuildId) : null; + if (provenGuildId && isSupplied(guildId) && guildId !== provenGuildId) { + return res.redirect(`${frontend}/discord/error?error=OAuth%20authorization%20failed`); + } + const installedGuildId = provenGuildId || guildId; let serverName = 'Unknown Server'; - if (guildId) { + if (isDiscordSnowflake(installedGuildId)) { try { - const guildResponse = await axios.get(`https://discord.com/api/guilds/${guildId}`, { headers: { Authorization: `Bot ${process.env.DISCORD_BOT_TOKEN}` } }); + const guildResponse = await axios.get(`https://discord.com/api/guilds/${installedGuildId}`, { headers: { Authorization: `Bot ${process.env.DISCORD_BOT_TOKEN}` } }); serverName = (guildResponse.data as { name?: string }).name || serverName; } catch { // Discord may echo request credentials in its error body. A failed @@ -345,8 +360,9 @@ router.get('/callback', discordCallbackLimit, async (req: AuthReq, res: Res) => } const successUrl = new URL(`${frontend}/discord/success`); successUrl.searchParams.append('pod_id', podId); - successUrl.searchParams.append('guild_id', guildId || ''); + successUrl.searchParams.append('guild_id', installedGuildId || ''); successUrl.searchParams.append('server_name', serverName); + if (provenGuildId) successUrl.searchParams.append('install', mintDiscordInstallProof({ podId, guildId: provenGuildId })); res.redirect(successUrl.toString()); } catch { console.error('Discord OAuth callback error'); diff --git a/backend/utils/discordInstallProof.ts b/backend/utils/discordInstallProof.ts new file mode 100644 index 000000000..8a8b1351b --- /dev/null +++ b/backend/utils/discordInstallProof.ts @@ -0,0 +1,53 @@ +/** + * Discord first-install proof (F89). + * + * The success page's channel picker reads `GET /api/discord/channels/:guildId`, + * which answers only for a guild already bound to one of the caller's pods. On a + * first install nothing is bound yet, so the picker never loaded. The OAuth + * callback is where Commonly learns which guild the bot was just added to: the + * code exchange's token response names it. The callback has no session (Discord + * redirects a browser there), so it hands the page a short-lived signed proof of + * (pod, guild), and the channels route accepts that proof in place of a binding + * when the caller belongs to the pod it names. + * + * The payload carries neither `id` nor `user`, so `middleware/auth` never reads + * it as a session. + */ +import jwt, { type JwtPayload } from 'jsonwebtoken'; + +import { isDiscordSnowflake } from './discordBinding'; + +const PURPOSE = 'discord-install'; +export const DISCORD_INSTALL_PROOF_TTL_SECONDS = 30 * 60; + +interface DiscordInstallProofPayload extends JwtPayload { + pur: typeof PURPOSE; + pod: string; + gid: string; +} + +const secret = (): string => { + const value = process.env.JWT_SECRET; + if (!value) throw new Error('JWT_SECRET is not set'); + return value; +}; + +export const mintDiscordInstallProof = ( + { podId, guildId }: { podId: string; guildId: string }, + ttlSeconds: number = DISCORD_INSTALL_PROOF_TTL_SECONDS, +): string => { + const payload: DiscordInstallProofPayload = { pur: PURPOSE, pod: podId, gid: guildId }; + 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 => { + 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 }; + } catch { + return null; + } +}; diff --git a/frontend/src/sentry.test.ts b/frontend/src/sentry.test.ts index f2f337e3e..d552eb408 100644 --- a/frontend/src/sentry.test.ts +++ b/frontend/src/sentry.test.ts @@ -118,6 +118,25 @@ describe('Sentry frontend instrumentation', () => { expect(ordinaryFragmentEvent.request.url).toBe('https://commonly.me/v2/register#section'); }); + // F89: the Discord success page carries a 30-minute install proof in `install` + // and sends it on the channel request, so it must not reach Sentry either. + it('scrubs the Discord install proof from breadcrumbs and request URLs', () => { + process.env.REACT_APP_SENTRY_DSN = 'https://public@example.ingest.sentry.io/456'; + loadSentryModule(); + const options = mockSentryInit.mock.calls[0][0]; + + const breadcrumb = options.beforeBreadcrumb({ + category: 'xhr', + data: { url: '/api/discord/channels/123456789012345678?install=PROOF-SECRET' }, + }); + expect(breadcrumb.data.url).toBe('/api/discord/channels/123456789012345678'); + + const event = options.beforeSend({ + request: { url: 'https://commonly.me/discord/success?pod_id=p1&guild_id=123456789012345678&install=PROOF-SECRET' }, + }); + expect(event.request.url).toBe('https://commonly.me/discord/success?pod_id=p1&guild_id=123456789012345678'); + }); + it('loads the SDK only through the DSN-gated dynamic import', () => { const indexSource = fs.readFileSync(path.join(__dirname, 'index.tsx'), 'utf8'); const viteSource = fs.readFileSync(path.join(__dirname, '../vite.config.ts'), 'utf8'); diff --git a/frontend/src/sentry.ts b/frontend/src/sentry.ts index 9d66be5e4..9969f764c 100644 --- a/frontend/src/sentry.ts +++ b/frontend/src/sentry.ts @@ -8,6 +8,8 @@ const scrubInviteFromUrl = (raw: string): string => { const isProtocolRelative = raw.startsWith('//'); const url = new URL(raw, 'https://commonly.invalid'); url.searchParams.delete('invite'); + // The Discord first-install proof (F89): a signed 30-minute credential. + url.searchParams.delete('install'); if (url.hash) { const hashParams = new URLSearchParams(url.hash.slice(1)); if (hashParams.has('invite')) { diff --git a/frontend/src/v2/__tests__/V2DiscordCallback.test.tsx b/frontend/src/v2/__tests__/V2DiscordCallback.test.tsx index 8740df52b..eaa98bccd 100644 --- a/frontend/src/v2/__tests__/V2DiscordCallback.test.tsx +++ b/frontend/src/v2/__tests__/V2DiscordCallback.test.tsx @@ -149,6 +149,44 @@ describe('V2DiscordCallback (d215 s-handoff)', () => { expect(route('error')?.trim()).toBe('type="error"'); }); + // F89: on a first install the channel list only answers on the callback's + // install proof, so the page must forward it, and send nothing extra without one. + test('forwards the install proof to the channel list', async () => { + // Production-shaped: the callback mints a three-part JWT, and the whole of it + // must arrive, not a value cut at a dot or a URL-decoding boundary. Joined at + // runtime so the secret scanner does not read a fixture as a token. + const proof = ['eyJhbGciOiJIUzI1NiJ9', 'eyJwdXIiOiJkaXNjb3JkLWluc3RhbGwifQ', 'c2lnbmF0dXJlLV8'].join('.'); + localStorage.setItem('token', 't'); + axios.get.mockResolvedValue({ data: [] }); + renderAt(`/discord/success?pod_id=pod-1&guild_id=123456789012345678&server_name=Guild&install=${encodeURIComponent(proof)}`); + await waitFor(() => expect(axios.get).toHaveBeenCalled()); + expect(axios.get).toHaveBeenCalledWith('/api/discord/channels/123456789012345678', { + headers: { Authorization: 'Bearer t' }, + params: { install: proof }, + }); + }); + + test('sends no params to the channel list without a proof', async () => { + localStorage.setItem('token', 't'); + axios.get.mockResolvedValue({ data: [] }); + renderAt('/discord/success?pod_id=pod-1&guild_id=123456789012345678&server_name=Guild'); + await waitFor(() => expect(axios.get).toHaveBeenCalled()); + expect(axios.get.mock.calls[0][1]).toEqual({ headers: { Authorization: 'Bearer t' } }); + }); + + test('a failed channel fetch logs its status, not the install proof', async () => { + const proof = ['eyJhbGciOiJIUzI1NiJ9', 'eyJwdXIiOiJkaXNjb3JkLWluc3RhbGwifQ', 'c2lnbmF0dXJlLV8'].join('.'); + axios.get.mockRejectedValueOnce(Object.assign(new Error('Request failed'), { + config: { params: { install: proof } }, + response: { status: 404 }, + })); + const errors = jest.spyOn(console, 'error').mockImplementation(() => {}); + renderAt(`/discord/success?pod_id=p1&guild_id=123456789012345678&server_name=S&install=${encodeURIComponent(proof)}`); + expect(await screen.findByRole('alert')).toHaveTextContent('Failed to fetch server channels'); + expect(errors).toHaveBeenCalledWith('Error fetching channels:', 404); + expect(JSON.stringify(errors.mock.calls, (_k, v) => (v instanceof Error ? { ...v, message: v.message } : v))).not.toContain(proof); + }); + test('reads in Chinese under zh-CN', async () => { await act(async () => { await i18n.changeLanguage('zh-CN'); }); renderAt('/discord/error'); diff --git a/frontend/src/v2/components/V2DiscordCallback.tsx b/frontend/src/v2/components/V2DiscordCallback.tsx index e1cec7794..4836034c2 100644 --- a/frontend/src/v2/components/V2DiscordCallback.tsx +++ b/frontend/src/v2/components/V2DiscordCallback.tsx @@ -47,6 +47,9 @@ const V2DiscordCallback: React.FC = ({ type = 'callback' const guildId = urlParams.get('guild_id'); const serverName = urlParams.get('server_name'); const errorMessage = urlParams.get('error'); + // The callback's first-install proof: the channel list answers on it before a + // binding exists (F89). + const installProof = urlParams.get('install'); useEffect(() => { if (type === 'success' && guildId) { @@ -64,10 +67,13 @@ const V2DiscordCallback: React.FC = ({ type = 'callback' const token = localStorage.getItem('token'); const response = await axios.get(`/api/discord/channels/${id}`, { headers: { Authorization: `Bearer ${token}` }, + ...(installProof ? { params: { install: installProof } } : {}), }); setChannels(response.data); } catch (err) { - console.error('Error fetching channels:', err); + // The status only: an axios error carries its request config, and with it + // the install proof (F89). + console.error('Error fetching channels:', (err as { response?: { status?: number } })?.response?.status ?? 'no response'); setError(t('auth.discordHandoff.channelsFailed')); } finally { setLoading(false);