From 56dae6dd8462daead8b66cf8f002c3b0d9303993 Mon Sep 17 00:00:00 2001 From: fajarhide Date: Mon, 17 Aug 2026 17:37:02 +0700 Subject: [PATCH] feat(admin): let the portal see which providers take a key, and give it somewhere to put one The portal drives every provider through POST .../connections/:prefix/authorize. Discord takes a bot token rather than a consent, so that call answers 400 and the portal turns the throw into a 500 on its connections page. It could not have done better. The admin connections list sent provider, grant, maturity, scopes and connected, so nothing in it says whether a provider wants a consent window or a secret. And PUT /v1/connections/:prefix/key is on the tenant router, authenticated by the workspace credential, which the portal does not hold. So there was no route to a working Discord connection at all. Two additions, both mirroring what the tenant router already does: credential rides along on the admin list, and PUT /v1/admin/workspaces/:id/connections/ :prefix/key writes the key through the same setApiKey path, with the workspace check and the audit row the sibling routes carry. The key is never echoed back and the audit row records only that it changed. --- src/interface/http/admin.ts | 35 +++++++++++++++ test/admin-connections.test.ts | 78 ++++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+) diff --git a/src/interface/http/admin.ts b/src/interface/http/admin.ts index 656596a..4be2346 100644 --- a/src/interface/http/admin.ts +++ b/src/interface/http/admin.ts @@ -18,6 +18,7 @@ import { pathParam } from './connections.ts' import { beginConnection, disconnect, + setApiKey, type ConnectionDeps, } from '../../application/connections.ts' @@ -182,6 +183,10 @@ export function adminRoutes(deps: AdminDeps): Router { grant: adapter.grantId, maturity: adapter.maturity, scopes: adapter.scopes, + // The portal has to know whether to open a consent window or ask for + // a secret, and the prefix does not say. Without this it offers + // Connect for every provider and the api_key ones answer 400. + credential: adapter.credential ?? 'oauth', connected: enabled.includes(adapter.prefix), })), request_id: req.requestId, @@ -219,6 +224,36 @@ export function adminRoutes(deps: AdminDeps): Router { }, ) + // The service-token twin of PUT /v1/connections/:prefix/key. The portal holds + // the service token and never the workspace credential, so without this route + // there is no way for it to connect a provider that takes a key at all. + router.put( + '/v1/admin/workspaces/:workspaceId/connections/:prefix/key', + async (req, res, next) => { + try { + const workspaceId = workspaceParam(req) + await readWorkspace(deps.pool, workspaceId) + const prefix = pathParam(req, 'prefix') + if (typeof req.body?.api_key !== 'string') { + throw new GatewayError('invalid_arguments', 'api_key must be a string') + } + await setApiKey(deps.connections, { workspaceId, prefix, key: req.body.api_key }) + // The key itself is never echoed back, not even its tail, so the audit + // row records that it changed and nothing about what it is. + await recordAudit(deps.pool, { + workspaceId, + actor: 'service', + action: 'connection.key_set', + target: prefix, + requestId: req.requestId, + }) + res.json({ connected: prefix, request_id: req.requestId }) + } catch (err) { + next(err) + } + }, + ) + router.delete('/v1/admin/workspaces/:workspaceId/connections/:prefix', async (req, res, next) => { try { const workspaceId = workspaceParam(req) diff --git a/test/admin-connections.test.ts b/test/admin-connections.test.ts index e96d750..76e597e 100644 --- a/test/admin-connections.test.ts +++ b/test/admin-connections.test.ts @@ -1,5 +1,6 @@ import { afterAll, describe, expect, it } from 'vitest' import { closeTestServers, startTestServer, testConfig } from './helpers/server.ts' +import { bootRegistry } from '../src/adapters/providers/boot.ts' import { testPool } from './helpers/db.ts' import { json } from './helpers/http.ts' @@ -167,3 +168,80 @@ describe('admin connections', () => { expect((await json(res)).error.code).toBe('tool_not_found') }) }) + +// The portal drove every provider through authorize, so discord answered 400 +// and the portal turned it into a 500. It needs to read the credential kind off +// the list, and it needs somewhere to put the key once it asks for one. +describe('admin connections for a provider that takes a key', () => { + const KEY = 'MTIzNDU2Nzg5.Gabcde.a-real-looking-bot-token' + + const putKey = (base: string, workspaceId: string, prefix: string, api_key: unknown) => + fetch(`${base}/v1/admin/workspaces/${workspaceId}/connections/${prefix}/key`, { + method: 'PUT', + headers: svc, + body: JSON.stringify({ api_key }), + }) + + it('names the credential kind so consent and key providers can be told apart', async () => { + // bootRegistry leaves out a provider with no client id, so github is only + // in the list when one is configured. + const { base, workspaceId } = await startTestServer({ + enable: [], + overrides: { + ...fakeVendor, + config: { ...testConfig, serviceToken: TOKEN, dashboardUrl: DASHBOARD }, + registry: bootRegistry({ ...process.env, GITHUB_CLIENT_ID: 'cid' }), + }, + }) + const listed = await json( + await fetch(`${base}/v1/admin/workspaces/${workspaceId}/connections`, { headers: svc }), + ) + expect(listed.connections).toContainEqual( + expect.objectContaining({ provider: 'discord', credential: 'api_key', connected: false }), + ) + expect(listed.connections).toContainEqual( + expect.objectContaining({ provider: 'github', credential: 'oauth' }), + ) + }) + + it('stores the key encrypted, turns the provider on and never echoes it back', async () => { + const { base, pool, workspaceId } = await start([]) + const res = await putKey(base, workspaceId, 'discord', KEY) + expect(res.status).toBe(200) + const body = await json(res) + expect(body).toMatchObject({ connected: 'discord' }) + expect(JSON.stringify(body)).not.toContain(KEY.slice(0, 12)) + + const stored = await pool.query( + 'SELECT access_token FROM grants WHERE workspace_id = $1 AND grant_id = $2', + [workspaceId, 'discord'], + ) + expect(stored.rowCount).toBe(1) + expect(JSON.stringify(stored.rows[0].access_token)).not.toContain(KEY) + + const listed = await json( + await fetch(`${base}/v1/admin/workspaces/${workspaceId}/connections`, { headers: svc }), + ) + expect(listed.connections).toContainEqual( + expect.objectContaining({ provider: 'discord', connected: true }), + ) + + const audit = await pool.query( + 'SELECT actor, action, target FROM audit_log WHERE workspace_id = $1 ORDER BY id', + [workspaceId], + ) + expect(audit.rows).toEqual([ + { actor: 'service', action: 'connection.key_set', target: 'discord' }, + ]) + }) + + it('refuses a key for a provider that does not take one, a non string, a blank one and an unknown workspace', async () => { + const { base, workspaceId } = await start([]) + expect((await putKey(base, workspaceId, 'fake', KEY)).status).toBe(400) + expect((await putKey(base, workspaceId, 'discord', 42)).status).toBe(400) + expect((await putKey(base, workspaceId, 'discord', ' ')).status).toBe(400) + expect( + (await putKey(base, '00000000-0000-0000-0000-000000000000', 'discord', KEY)).status, + ).toBe(404) + }) +})