From 826626b49015f337020e27c1f43db20627a35d6a Mon Sep 17 00:00:00 2001 From: Jordan Simonovski Date: Thu, 22 Oct 2026 08:35:01 +1100 Subject: [PATCH 1/5] feat: team-owned API keys managed by admins Admins can create API keys owned by the team rather than a person, so Terraform, CI and other automation keep working when someone leaves. Each key has a description, an admin or member scope, and a 30 day, 1 year or no expiry. Keys are shown once, stored hashed, and accepted by the external API v2 and MCP. Member-scoped keys can't manage team members or invitations. Closes HDX-5784 --- .changeset/org-api-keys.md | 7 + packages/api/openapi.json | 14 +- .../__tests__/orgApiKey.int.test.ts | 116 ++++++++++++++ packages/api/src/controllers/alerts.ts | 2 +- packages/api/src/controllers/orgApiKey.ts | 128 +++++++++++++++ packages/api/src/controllers/user.ts | 19 ++- .../api/src/mcp/__tests__/alerts.int.test.ts | 25 +++ packages/api/src/mcp/app.ts | 20 ++- .../api/src/mcp/tools/alerts/saveAlert.ts | 5 +- packages/api/src/mcp/tools/types.ts | 7 +- packages/api/src/mcp/utils/tracing.ts | 8 +- packages/api/src/middleware/auth.ts | 55 ++++++- packages/api/src/models/orgApiKey.ts | 38 +++++ .../routers/api/__tests__/team.int.test.ts | 119 +++++++++++++- packages/api/src/routers/api/orgApiKeys.ts | 111 +++++++++++++ packages/api/src/routers/api/team.ts | 3 + .../external-api/__tests__/alerts.int.test.ts | 30 ++++ .../external-api/__tests__/team.int.test.ts | 94 +++++++++++ .../external-api/__tests__/v2.int.test.ts | 96 ++++++++++++ .../api/src/routers/external-api/v2/alerts.ts | 3 +- .../api/src/routers/external-api/v2/index.ts | 8 + .../api/src/routers/external-api/v2/team.ts | 34 +++- packages/api/src/utils/orgApiKeyPolicy.ts | 25 +++ packages/app/src/TeamPage.tsx | 5 + packages/app/src/api.ts | 53 +++++++ .../TeamSettings/CreateOrgApiKeyModal.tsx | 141 +++++++++++++++++ .../TeamSettings/OrgApiKeysSection.tsx | 146 ++++++++++++++++++ .../__tests__/OrgApiKeysSection.test.tsx | 135 ++++++++++++++++ packages/common-utils/src/types.ts | 55 +++++++ 29 files changed, 1477 insertions(+), 25 deletions(-) create mode 100644 .changeset/org-api-keys.md create mode 100644 packages/api/src/controllers/__tests__/orgApiKey.int.test.ts create mode 100644 packages/api/src/controllers/orgApiKey.ts create mode 100644 packages/api/src/models/orgApiKey.ts create mode 100644 packages/api/src/routers/api/orgApiKeys.ts create mode 100644 packages/api/src/utils/orgApiKeyPolicy.ts create mode 100644 packages/app/src/components/TeamSettings/CreateOrgApiKeyModal.tsx create mode 100644 packages/app/src/components/TeamSettings/OrgApiKeysSection.tsx create mode 100644 packages/app/src/components/TeamSettings/__tests__/OrgApiKeysSection.test.tsx diff --git a/.changeset/org-api-keys.md b/.changeset/org-api-keys.md new file mode 100644 index 0000000000..92f96d2dff --- /dev/null +++ b/.changeset/org-api-keys.md @@ -0,0 +1,7 @@ +--- +'@hyperdx/api': minor +'@hyperdx/app': minor +'@hyperdx/common-utils': minor +--- + +feat: Admins can create team-owned API keys with a description, scope and expiry that keep working after their creator leaves. diff --git a/packages/api/openapi.json b/packages/api/openapi.json index b6263b7423..d75a5d5f4e 100644 --- a/packages/api/openapi.json +++ b/packages/api/openapi.json @@ -8317,6 +8317,9 @@ }, "400": { "description": "User already exists" + }, + "403": { + "description": "The API key does not have admin scope" } } } @@ -8332,6 +8335,9 @@ "responses": { "200": { "description": "Successfully retrieved invitations" + }, + "403": { + "description": "The API key does not have admin scope" } } } @@ -8358,6 +8364,9 @@ "200": { "description": "Successfully deleted the invitation" }, + "403": { + "description": "The API key does not have admin scope" + }, "404": { "description": "Invitation not found" } @@ -8387,7 +8396,10 @@ "description": "Successfully removed the member" }, "400": { - "description": "Cannot remove yourself from the team" + "description": "Cannot remove yourself or the last team member" + }, + "403": { + "description": "The API key does not have admin scope" }, "404": { "description": "Member not found" diff --git a/packages/api/src/controllers/__tests__/orgApiKey.int.test.ts b/packages/api/src/controllers/__tests__/orgApiKey.int.test.ts new file mode 100644 index 0000000000..fd9a0b1437 --- /dev/null +++ b/packages/api/src/controllers/__tests__/orgApiKey.int.test.ts @@ -0,0 +1,116 @@ +import mongoose from 'mongoose'; + +import { + createOrgApiKey, + deleteOrgApiKey, + findActiveOrgApiKey, + hashOrgApiKey, + listOrgApiKeys, + touchOrgApiKeyLastUsed, +} from '@/controllers/orgApiKey'; +import { createTeam } from '@/controllers/team'; +import { clearDBCollections, closeDB, connectDB } from '@/fixtures'; +import OrgApiKey from '@/models/orgApiKey'; + +const DAY_MS = 24 * 60 * 60 * 1000; + +describe('orgApiKey controller', () => { + const creator = new mongoose.Types.ObjectId(); + + beforeAll(async () => { + await connectDB(); + }); + + afterEach(async () => { + await clearDBCollections(); + }); + + afterAll(async () => { + await closeDB(); + }); + + const create = async ( + teamId: mongoose.Types.ObjectId, + expiry: '30d' | '1y' | 'never' = '30d', + ) => + createOrgApiKey(teamId, creator, { + description: 'terraform', + scope: 'member', + expiry, + }); + + it('returns a prefixed key once and stores only its hash', async () => { + const team = await createTeam({ name: 'A' }); + const { doc, key } = await create(team._id); + + expect(key).toMatch(/^hdx_org_[A-Za-z0-9_-]{43}$/); + const stored = await OrgApiKey.findById(doc._id).lean(); + expect(stored?.keyHash).toBe(hashOrgApiKey(key)); + expect(Object.values(stored ?? {})).not.toContain(key); + }); + + it.each([ + ['30d', 30 * DAY_MS], + ['1y', 365 * DAY_MS], + ] as const)('sets expiresAt for %s', async (expiry, offset) => { + const team = await createTeam({ name: 'A' }); + const before = Date.now(); + const { doc } = await create(team._id, expiry); + const expiresAt = doc.expiresAt?.getTime() ?? 0; + expect(Math.abs(expiresAt - (before + offset))).toBeLessThan(1000); + }); + + it('stores null expiresAt for never', async () => { + const team = await createTeam({ name: 'A' }); + const { doc } = await create(team._id, 'never'); + expect(doc.expiresAt).toBeNull(); + }); + + it('finds active keys and rejects expired, unknown, and non-org keys', async () => { + const team = await createTeam({ name: 'A' }); + const { doc, key } = await create(team._id); + + expect((await findActiveOrgApiKey(key))?._id).toEqual(doc._id); + expect( + await findActiveOrgApiKey(key, doc.expiresAt ?? new Date(0)), + ).toBeNull(); + expect(await findActiveOrgApiKey(`hdx_org_${'x'.repeat(43)}`)).toBeNull(); + expect( + await findActiveOrgApiKey('4b9e3c1a-7f2d-4e8b-9a6c-1d2e3f4a5b6c'), + ).toBeNull(); + }); + + it('scopes list and delete to the team', async () => { + const teamA = await createTeam({ name: 'A' }); + // OSS allows a single team, so team B only needs to be a distinct id. + const teamB = new mongoose.Types.ObjectId(); + const { doc } = await create(teamA._id); + + expect(await listOrgApiKeys(teamB)).toHaveLength(0); + expect(await deleteOrgApiKey(teamB, doc._id.toString())).toBeNull(); + expect(await OrgApiKey.findById(doc._id)).not.toBeNull(); + }); + + it('writes lastUsedAt at most once per 5 minutes', async () => { + const team = await createTeam({ name: 'A' }); + const { doc } = await create(team._id); + const t0 = new Date(); + + const lastUsed = async () => { + // Writes are fire-and-forget; give them a moment to land. + await new Promise(r => setTimeout(r, 100)); + return (await OrgApiKey.findById(doc._id))?.lastUsedAt?.getTime(); + }; + + touchOrgApiKeyLastUsed(doc, t0); + expect(await lastUsed()).toBe(t0.getTime()); + + const fresh = (await OrgApiKey.findById(doc._id))!; + touchOrgApiKeyLastUsed(fresh, new Date(t0.getTime() + 60_000)); + expect(await lastUsed()).toBe(t0.getTime()); + + const t6 = new Date(t0.getTime() + 6 * 60_000); + touchOrgApiKeyLastUsed(fresh, t6); + expect(await lastUsed()).toBe(t6.getTime()); + }); +}); diff --git a/packages/api/src/controllers/alerts.ts b/packages/api/src/controllers/alerts.ts index 0e8762e48c..3be85f5292 100644 --- a/packages/api/src/controllers/alerts.ts +++ b/packages/api/src/controllers/alerts.ts @@ -341,7 +341,7 @@ const makeAlertUpdate = ( export const createAlert = async ( teamId: ObjectId, alertInput: z.infer, - userId: ObjectId, + userId: ObjectId | undefined, refs: AlertRefs = {}, ) => { const alert = await new Alert({ diff --git a/packages/api/src/controllers/orgApiKey.ts b/packages/api/src/controllers/orgApiKey.ts new file mode 100644 index 0000000000..2963c64d89 --- /dev/null +++ b/packages/api/src/controllers/orgApiKey.ts @@ -0,0 +1,128 @@ +import type { + CreateOrgApiKeyRequest, + OrgApiKeyApiResponse, + OrgApiKeyExpiry, +} from '@hyperdx/common-utils/dist/types'; +import crypto from 'crypto'; + +import type { ObjectId } from '@/models'; +import OrgApiKey, { type OrgApiKeyDocument } from '@/models/orgApiKey'; +import type { IUser } from '@/models/user'; +import logger from '@/utils/logger'; + +export const ORG_API_KEY_PREFIX = 'hdx_org_'; + +const DAY_MS = 24 * 60 * 60 * 1000; +const LAST_USED_WRITE_INTERVAL_MS = 5 * 60 * 1000; + +function expiresAtFor(expiry: OrgApiKeyExpiry, now: number): Date | null { + switch (expiry) { + case '30d': + return new Date(now + 30 * DAY_MS); + case '1y': + return new Date(now + 365 * DAY_MS); + case 'never': + return null; + } +} + +export function hashOrgApiKey(key: string) { + return crypto.createHash('sha256').update(key).digest('hex'); +} + +export async function createOrgApiKey( + teamId: string | ObjectId, + createdBy: string | ObjectId, + { description, scope, expiry }: CreateOrgApiKeyRequest, +) { + const key = `${ORG_API_KEY_PREFIX}${crypto.randomBytes(32).toString('base64url')}`; + const doc = await OrgApiKey.create({ + team: teamId, + keyHash: hashOrgApiKey(key), + description, + scope, + expiresAt: expiresAtFor(expiry, Date.now()), + createdBy, + }); + return { doc, key }; +} + +export function listOrgApiKeys(teamId: string | ObjectId) { + return OrgApiKey.find({ team: teamId }) + .sort({ createdAt: -1 }) + .populate<{ createdBy: IUser | null }>('createdBy', 'name email'); +} + +export function updateOrgApiKeyDescription( + teamId: string | ObjectId, + id: string, + description: string, +) { + return OrgApiKey.findOneAndUpdate( + { _id: id, team: teamId }, + { description }, + { new: true }, + ).populate<{ createdBy: IUser | null }>('createdBy', 'name email'); +} + +export function deleteOrgApiKey(teamId: string | ObjectId, id: string) { + return OrgApiKey.findOneAndDelete({ _id: id, team: teamId }); +} + +export async function findActiveOrgApiKey(key: string, now = new Date()) { + if (!key.startsWith(ORG_API_KEY_PREFIX)) { + return null; + } + return OrgApiKey.findOne({ + keyHash: hashOrgApiKey(key), + $or: [{ expiresAt: null }, { expiresAt: { $gt: now } }], + }); +} + +// Fire-and-forget so auth never waits on or fails because of this write. The +// conditional filter stops concurrent requests from all writing at once. +export function touchOrgApiKeyLastUsed( + doc: OrgApiKeyDocument, + now = new Date(), +) { + const staleBefore = new Date(now.getTime() - LAST_USED_WRITE_INTERVAL_MS); + if (doc.lastUsedAt != null && doc.lastUsedAt > staleBefore) { + return; + } + void OrgApiKey.updateOne( + { + _id: doc._id, + $or: [{ lastUsedAt: null }, { lastUsedAt: { $lt: staleBefore } }], + }, + { lastUsedAt: now }, + ).catch(err => { + logger.warn( + { error: err, orgApiKeyId: doc._id.toString() }, + 'Failed to record org API key last use', + ); + }); +} + +export function formatOrgApiKey( + doc: Omit & { + createdBy: IUser | ObjectId | null; + }, +): OrgApiKeyApiResponse { + const creator = + doc.createdBy != null && 'email' in doc.createdBy ? doc.createdBy : null; + return { + id: doc._id.toString(), + description: doc.description, + scope: doc.scope, + createdAt: doc.createdAt.toISOString(), + expiresAt: doc.expiresAt?.toISOString() ?? null, + lastUsedAt: doc.lastUsedAt?.toISOString() ?? null, + createdBy: creator + ? { + id: creator._id.toString(), + name: creator.name, + email: creator.email, + } + : null, + }; +} diff --git a/packages/api/src/controllers/user.ts b/packages/api/src/controllers/user.ts index 7c80b782be..03f1860aa7 100644 --- a/packages/api/src/controllers/user.ts +++ b/packages/api/src/controllers/user.ts @@ -31,6 +31,10 @@ export function findUserByEmail(email: string) { return User.findOne({ email: email.toLowerCase() }); } +export function countUsersInTeam(team: string | ObjectId) { + return User.countDocuments({ team }); +} + export function findUsersByTeam(team: string | ObjectId) { return User.find({ team }).sort({ createdAt: 1 }); } @@ -98,16 +102,19 @@ export function recordOnboardingTaskCompletion( export async function deleteTeamMember( teamId: string | ObjectId, userIdToDelete: string, - userIdRequestingDelete: string | ObjectId, + // Undefined when an org API key made the request: alerts lose their owner. + userIdRequestingDelete?: string | ObjectId, ) { const [, deletedUser] = await Promise.all([ Alert.updateMany( { createdBy: new mongoose.Types.ObjectId(userIdToDelete), team: teamId }, - { - $set: { - createdBy: new mongoose.Types.ObjectId(userIdRequestingDelete), - }, - }, + userIdRequestingDelete == null + ? { $unset: { createdBy: '' } } + : { + $set: { + createdBy: new mongoose.Types.ObjectId(userIdRequestingDelete), + }, + }, ), User.findOneAndDelete({ team: teamId, diff --git a/packages/api/src/mcp/__tests__/alerts.int.test.ts b/packages/api/src/mcp/__tests__/alerts.int.test.ts index ac87f72225..127809784a 100644 --- a/packages/api/src/mcp/__tests__/alerts.int.test.ts +++ b/packages/api/src/mcp/__tests__/alerts.int.test.ts @@ -353,6 +353,31 @@ describe('MCP Alert Tools', () => { expect(output.state).toBe('OK'); }); + it('leaves createdBy unset when called with an org API key', async () => { + const savedSearch = await createTestSavedSearch(); + const webhook = await createTestWebhook(); + await client.close(); + client = await createTestClient({ + teamId: team._id.toString(), + userId: undefined, + orgApiKeyId: 'org-key-id', + }); + + const result = await callTool(client, 'clickstack_save_alert', { + source: 'saved_search', + savedSearchId: savedSearch._id.toString(), + threshold: 50, + thresholdType: 'above', + interval: '5m', + channel: { type: 'webhook', webhookId: webhook._id.toString() }, + }); + + expect(result.isError).toBeFalsy(); + const { id } = JSON.parse(getFirstText(result)); + const stored = await Alert.findById(id).lean(); + expect(stored?.createdBy).toBeUndefined(); + }); + it('should create an alert with an explicit displayName and tags', async () => { const savedSearch = await createTestSavedSearch(); const webhook = await createTestWebhook(); diff --git a/packages/api/src/mcp/app.ts b/packages/api/src/mcp/app.ts index 94b05777eb..ccb15bd71f 100644 --- a/packages/api/src/mcp/app.ts +++ b/packages/api/src/mcp/app.ts @@ -51,24 +51,34 @@ app.post('/', mcpRateLimiter, validateUserAccessKey, async (req, res) => { } const userId = req.user?._id?.toString(); - if (!userId) { - logger.warn('MCP request rejected: no userId'); + const orgApiKeyId = req.orgApiKey?.id; + const principal = userId + ? { userId } + : orgApiKeyId + ? { orgApiKeyId } + : undefined; + if (!principal) { + logger.warn('MCP request rejected: no userId or org API key'); res.sendStatus(403); return; } const context: McpContext = { + ...principal, teamId: teamId.toString(), - userId, mcpClient: userAgentClientInfo(req.get('User-Agent')), }; setTraceAttributes({ 'mcp.team.id': context.teamId, - 'mcp.user.id': userId, + ...(userId && { 'mcp.user.id': userId }), + ...(orgApiKeyId && { 'mcp.org_api_key.id': orgApiKeyId }), }); - logger.info({ teamId: context.teamId, userId }, 'MCP request received'); + logger.info( + { teamId: context.teamId, userId, orgApiKeyId }, + 'MCP request received', + ); const server = createServer(context); diff --git a/packages/api/src/mcp/tools/alerts/saveAlert.ts b/packages/api/src/mcp/tools/alerts/saveAlert.ts index 9c5962026d..d04841bf62 100644 --- a/packages/api/src/mcp/tools/alerts/saveAlert.ts +++ b/packages/api/src/mcp/tools/alerts/saveAlert.ts @@ -140,7 +140,10 @@ export function registerSaveAlert({ return e instanceof BaseError ? mcpUserError(msg) : mcpServerError(msg); } - const mongoUserId = new mongoose.Types.ObjectId(userId); + // ObjectId(undefined) mints a random id, so org API keys must skip it. + const mongoUserId = userId + ? new mongoose.Types.ObjectId(userId) + : undefined; // ── Update existing alert ── if (alertId) { diff --git a/packages/api/src/mcp/tools/types.ts b/packages/api/src/mcp/tools/types.ts index 920c6b94a6..d720ffd205 100644 --- a/packages/api/src/mcp/tools/types.ts +++ b/packages/api/src/mcp/tools/types.ts @@ -8,9 +8,12 @@ import type { AnyZodObject } from 'zod'; import type { ToolBehaviorAnnotations } from '@/mcp/tools/annotations'; import type { McpClientInfo } from '@/mcp/utils/mcpClient'; -export type McpContext = { +export type McpContext = ( + | { userId: string; orgApiKeyId?: undefined } + // Org API keys have no user. + | { userId?: undefined; orgApiKeyId: string } +) & { teamId: string; - userId: string; /** * Identity of the calling MCP client application, parsed from User-Agent. */ diff --git a/packages/api/src/mcp/utils/tracing.ts b/packages/api/src/mcp/utils/tracing.ts index 48ae9d9b3f..5c969d61b2 100644 --- a/packages/api/src/mcp/utils/tracing.ts +++ b/packages/api/src/mcp/utils/tracing.ts @@ -88,6 +88,7 @@ export function withToolTracing( tool: toolName, teamId: context.teamId, userId: context.userId, + orgApiKeyId: context.orgApiKeyId, mcpClientName: clientName, mcpClientVersion: clientVersion, }; @@ -98,7 +99,12 @@ export function withToolTracing( const startTime = Date.now(); span.setAttribute('mcp.tool.name', toolName); span.setAttribute('mcp.team.id', context.teamId); - span.setAttribute('mcp.user.id', context.userId); + if (context.userId) { + span.setAttribute('mcp.user.id', context.userId); + } + if (context.orgApiKeyId) { + span.setAttribute('mcp.org_api_key.id', context.orgApiKeyId); + } if (clientName) { span.setAttribute('mcp.client.name', clientName); } diff --git a/packages/api/src/middleware/auth.ts b/packages/api/src/middleware/auth.ts index 5be32c9798..cd163d75fd 100644 --- a/packages/api/src/middleware/auth.ts +++ b/packages/api/src/middleware/auth.ts @@ -1,13 +1,22 @@ import { withQueryAttribution } from '@hyperdx/common-utils/dist/clickhouse/node'; -import { Connection } from '@hyperdx/common-utils/dist/types'; +import type { + Connection, + OrgApiKeyScope, +} from '@hyperdx/common-utils/dist/types'; import type { NextFunction, Request, Response } from 'express'; import { serializeError } from 'serialize-error'; import * as config from '@/config'; +import { + findActiveOrgApiKey, + ORG_API_KEY_PREFIX, + touchOrgApiKeyLastUsed, +} from '@/controllers/orgApiKey'; import { findUserByAccessKey } from '@/controllers/user'; import type { UserDocument } from '@/models/user'; import { getActiveTraceId, + getCounter, getStaticFeatureFlags, setBusinessContext, } from '@/utils/instrumentation'; @@ -22,6 +31,8 @@ declare global { interface User extends UserDocument {} interface Request { _hdx_connection?: Connection; + // Set when the bearer token is a team-owned key rather than a user's. + orgApiKey?: { id: string; description: string; scope: OrgApiKeyScope }; } } } @@ -97,7 +108,7 @@ export async function validateUserAccessKey( const user = await findUserByAccessKey(key); if (!user) { - return res.sendStatus(401); + return authenticateOrgApiKey(key, req, res, next); } req.user = user; @@ -114,6 +125,46 @@ export async function validateUserAccessKey( nextWithQueryAttribution(req, next); } +const orgApiKeyAuthFailures = getCounter('hyperdx.org_api_key.auth_failures', { + description: + 'Bearer requests with an org API key that is unknown, revoked, or expired.', +}); + +async function authenticateOrgApiKey( + key: string, + req: Request, + res: Response, + next: NextFunction, +) { + const orgApiKey = await findActiveOrgApiKey(key); + if (!orgApiKey) { + if (key.startsWith(ORG_API_KEY_PREFIX)) { + orgApiKeyAuthFailures.add(1); + } + return res.sendStatus(401); + } + + // Handlers read the team from req.user. Org keys have no user, so _id and + // email stay unset and nothing is attributed to the admin who created it. + // @ts-expect-error the principal carries only the team, not a full user + req.user = { team: orgApiKey.team }; + req.orgApiKey = { + id: orgApiKey._id.toString(), + description: orgApiKey.description, + scope: orgApiKey.scope, + }; + touchOrgApiKeyLastUsed(orgApiKey); + + setBusinessContext({ + teamId: orgApiKey.team.toString(), + 'hyperdx.org_api_key.id': req.orgApiKey.id, + 'hyperdx.org_api_key.scope': orgApiKey.scope, + ...getStaticFeatureFlags(), + }); + + nextWithQueryAttribution(req, next); +} + /** * Continue the request with its route recorded on any ClickHouse queries it * makes. Here because every authenticated route passes through this point. diff --git a/packages/api/src/models/orgApiKey.ts b/packages/api/src/models/orgApiKey.ts new file mode 100644 index 0000000000..5f70726918 --- /dev/null +++ b/packages/api/src/models/orgApiKey.ts @@ -0,0 +1,38 @@ +import type { OrgApiKeyScope } from '@hyperdx/common-utils/dist/types'; +import mongoose, { Schema } from 'mongoose'; + +import type { ObjectId } from '.'; + +interface IOrgApiKey { + _id: ObjectId; + team: ObjectId; + // SHA-256 of the plaintext key; the key itself is never stored. + keyHash: string; + description: string; + scope: OrgApiKeyScope; + expiresAt: Date | null; + lastUsedAt: Date | null; + createdBy: ObjectId; + createdAt: Date; + updatedAt: Date; +} + +export type OrgApiKeyDocument = mongoose.HydratedDocument; + +const OrgApiKeySchema = new Schema( + { + team: { type: Schema.Types.ObjectId, ref: 'Team', required: true }, + keyHash: { type: String, required: true }, + description: { type: String, required: true }, + scope: { type: String, enum: ['admin', 'member'], required: true }, + expiresAt: { type: Date, default: null }, + lastUsedAt: { type: Date, default: null }, + createdBy: { type: Schema.Types.ObjectId, ref: 'User', required: true }, + }, + { timestamps: true }, +); + +OrgApiKeySchema.index({ keyHash: 1 }, { unique: true }); +OrgApiKeySchema.index({ team: 1 }); + +export default mongoose.model('OrgApiKey', OrgApiKeySchema); diff --git a/packages/api/src/routers/api/__tests__/team.int.test.ts b/packages/api/src/routers/api/__tests__/team.int.test.ts index 93344f53ac..582beccb8e 100644 --- a/packages/api/src/routers/api/__tests__/team.int.test.ts +++ b/packages/api/src/routers/api/__tests__/team.int.test.ts @@ -2,8 +2,9 @@ import _ from 'lodash'; import { ObjectId } from 'mongodb'; import mongoose from 'mongoose'; -import { getLoggedInAgent, getServer } from '@/fixtures'; +import { getAgent, getLoggedInAgent, getServer } from '@/fixtures'; import Alert, { AlertSource, AlertThresholdType } from '@/models/alert'; +import OrgApiKey from '@/models/orgApiKey'; import Team from '@/models/team'; import TeamInvite from '@/models/teamInvite'; import User from '@/models/user'; @@ -427,4 +428,120 @@ describe('team router', () => { expect(resp.body.newApiKey.length).toBeGreaterThan(0); }); + + describe('org API keys', () => { + const body = { + description: 'terraform', + scope: 'member', + expiry: '30d', + } as const; + + it('returns the key on create only', async () => { + const { agent, user } = await getLoggedInAgent(server); + const created = await agent.post('/team/api-keys').send(body).expect(200); + expect(created.body.key).toMatch(/^hdx_org_/); + + const list = await agent.get('/team/api-keys').expect(200); + expect(list.body.data).toHaveLength(1); + expect(list.body.data[0]).not.toHaveProperty('key'); + expect(list.body.data[0]).toMatchObject({ + id: created.body.id, + description: 'terraform', + scope: 'member', + lastUsedAt: null, + createdBy: { email: user.email }, + }); + }); + + it.each([ + { ...body, description: '' }, + { ...body, description: 'x'.repeat(101) }, + { ...body, expiry: '7d' }, + { ...body, scope: 'owner' }, + ])('rejects invalid bodies %#', async invalid => { + const { agent } = await getLoggedInAgent(server); + await agent.post('/team/api-keys').send(invalid).expect(400); + }); + + it('edits only the description', async () => { + const { agent } = await getLoggedInAgent(server); + const { body: created } = await agent + .post('/team/api-keys') + .send(body) + .expect(200); + const updated = await agent + .patch(`/team/api-keys/${created.id}`) + .send({ description: 'ci', scope: 'admin' }) + .expect(200); + expect(updated.body).toMatchObject({ + description: 'ci', + scope: 'member', + }); + }); + + it('revokes immediately', async () => { + const { agent } = await getLoggedInAgent(server); + const { body: created } = await agent + .post('/team/api-keys') + .send(body) + .expect(200); + await agent + .get('/api/v2') + .set('Authorization', `Bearer ${created.key}`) + .expect(200); + await agent.delete(`/team/api-keys/${created.id}`).expect(204); + await agent + .get('/api/v2') + .set('Authorization', `Bearer ${created.key}`) + .expect(401); + }); + + it('hides other teams keys', async () => { + const { agent } = await getLoggedInAgent(server); + // OSS allows a single team, so plant a key under a different team id. + const other = await OrgApiKey.create({ + team: new mongoose.Types.ObjectId(), + keyHash: 'other', + description: 'other team', + scope: 'admin', + createdBy: new mongoose.Types.ObjectId(), + }); + const list = await agent.get('/team/api-keys').expect(200); + expect(list.body.data).toHaveLength(0); + await agent + .patch(`/team/api-keys/${other._id}`) + .send({ description: 'mine now' }) + .expect(404); + await agent.delete(`/team/api-keys/${other._id}`).expect(404); + expect(await OrgApiKey.findById(other._id)).not.toBeNull(); + }); + + it('keeps working after the creator is deleted', async () => { + const { agent, user } = await getLoggedInAgent(server); + const { body: created } = await agent + .post('/team/api-keys') + .send(body) + .expect(200); + await User.deleteOne({ _id: user._id }); + + const key = await OrgApiKey.findById(created.id).populate('createdBy'); + expect(key?.createdBy).toBeNull(); + await getAgent(server) + .get('/api/v2') + .set('Authorization', `Bearer ${created.key}`) + .expect(200); + }); + + it('rejects bearer auth on the management routes', async () => { + const { agent } = await getLoggedInAgent(server); + const { body: created } = await agent + .post('/team/api-keys') + .send(body) + .expect(200); + await getAgent(server) + .get('/team/api-keys') + .set('Authorization', `Bearer ${created.key}`) + .expect(401); + }); + }); }); diff --git a/packages/api/src/routers/api/orgApiKeys.ts b/packages/api/src/routers/api/orgApiKeys.ts new file mode 100644 index 0000000000..ccd8aa850b --- /dev/null +++ b/packages/api/src/routers/api/orgApiKeys.ts @@ -0,0 +1,111 @@ +import type { + CreateOrgApiKeyApiResponse, + OrgApiKeyApiResponse, + OrgApiKeysApiResponse, +} from '@hyperdx/common-utils/dist/types'; +import { + CreateOrgApiKeyRequestSchema, + UpdateOrgApiKeyRequestSchema, +} from '@hyperdx/common-utils/dist/types'; +import express from 'express'; +import { z } from 'zod'; +import { processRequest, validateRequest } from 'zod-express-middleware'; + +import { + createOrgApiKey, + deleteOrgApiKey, + formatOrgApiKey, + listOrgApiKeys, + updateOrgApiKeyDescription, +} from '@/controllers/orgApiKey'; +import { getCounter } from '@/utils/instrumentation'; +import { canManageOrgApiKeys } from '@/utils/orgApiKeyPolicy'; +import { objectIdSchema } from '@/utils/zod'; + +const orgApiKeyOperations = getCounter('hyperdx.org_api_key.operations', { + description: 'Org API key changes made in Team Settings, by operation.', +}); + +// Session-only on purpose: no key can create or revoke another key. +const router = express.Router(); + +router.use((req, res, next) => { + if (req.user == null || !canManageOrgApiKeys(req.user)) { + return res.sendStatus(403); + } + next(); +}); + +router.get( + '/', + async (req, res: express.Response, next) => { + try { + const keys = await listOrgApiKeys(req.user!.team); + res.json({ data: keys.map(formatOrgApiKey) }); + } catch (e) { + next(e); + } + }, +); + +router.post( + '/', + processRequest({ body: CreateOrgApiKeyRequestSchema }), + async (req, res: express.Response, next) => { + try { + const { doc, key } = await createOrgApiKey( + req.user!.team, + req.user!._id, + req.body, + ); + await doc.populate('createdBy', 'name email'); + orgApiKeyOperations.add(1, { operation: 'create' }); + res.json({ ...formatOrgApiKey(doc), key }); + } catch (e) { + next(e); + } + }, +); + +router.patch( + '/:id', + processRequest({ + params: z.object({ id: objectIdSchema }), + body: UpdateOrgApiKeyRequestSchema, + }), + async (req, res: express.Response, next) => { + try { + const doc = await updateOrgApiKeyDescription( + req.user!.team, + req.params.id, + req.body.description, + ); + if (doc == null) { + return res.sendStatus(404); + } + orgApiKeyOperations.add(1, { operation: 'update' }); + res.json(formatOrgApiKey(doc)); + } catch (e) { + next(e); + } + }, +); + +router.delete( + '/:id', + validateRequest({ params: z.object({ id: objectIdSchema }) }), + async (req, res, next) => { + try { + const doc = await deleteOrgApiKey(req.user!.team, req.params.id); + if (doc == null) { + return res.sendStatus(404); + } + orgApiKeyOperations.add(1, { operation: 'revoke' }); + res.sendStatus(204); + } catch (e) { + next(e); + } + }, +); + +export default router; diff --git a/packages/api/src/routers/api/team.ts b/packages/api/src/routers/api/team.ts index 40fc199486..4823414425 100644 --- a/packages/api/src/routers/api/team.ts +++ b/packages/api/src/routers/api/team.ts @@ -31,6 +31,7 @@ import { } from '@/controllers/user'; import { getNonNullUserWithTeam } from '@/middleware/auth'; import TeamInvite from '@/models/teamInvite'; +import orgApiKeysRouter from '@/routers/api/orgApiKeys'; import { sendJson } from '@/utils/serialization'; import { objectIdSchema } from '@/utils/zod'; @@ -332,4 +333,6 @@ router.get( }, ); +router.use('/api-keys', orgApiKeysRouter); + export default router; diff --git a/packages/api/src/routers/external-api/__tests__/alerts.int.test.ts b/packages/api/src/routers/external-api/__tests__/alerts.int.test.ts index 0d79f5ef43..e92f4d3e14 100644 --- a/packages/api/src/routers/external-api/__tests__/alerts.int.test.ts +++ b/packages/api/src/routers/external-api/__tests__/alerts.int.test.ts @@ -3,6 +3,7 @@ import _ from 'lodash'; import { ObjectId } from 'mongodb'; import request from 'supertest'; +import { createOrgApiKey } from '@/controllers/orgApiKey'; import { getLoggedInAgent, getServer, @@ -190,6 +191,35 @@ describe('External API Alerts', () => { }); }; + describe('org API keys', () => { + it('creates an alert with no createdBy', async () => { + const { key } = await createOrgApiKey(team._id, user._id, { + description: 'terraform', + scope: 'member', + expiry: 'never', + }); + const dashboard = await createTestDashboard(); + const webhook = await createTestWebhook(); + + const response = await agent + .post(ALERTS_BASE_URL) + .set('Authorization', `Bearer ${key}`) + .send({ + dashboardId: dashboard._id.toString(), + tileId: dashboard.tiles[0].id, + threshold: 100, + interval: '1h', + source: AlertSource.TILE, + thresholdType: AlertThresholdType.ABOVE, + channel: { type: 'webhook', webhookId: webhook._id.toString() }, + }) + .expect(200); + + const stored = await Alert.findById(response.body.data.id).lean(); + expect(stored?.createdBy).toBeUndefined(); + }); + }); + // The readers behind these routes project the referenced dashboard down to // the handful of fields the derivation needs, rather than pulling every // tile's full chart config. These cases fail if the projection drops one. diff --git a/packages/api/src/routers/external-api/__tests__/team.int.test.ts b/packages/api/src/routers/external-api/__tests__/team.int.test.ts index 1d8c148829..05968fbd44 100644 --- a/packages/api/src/routers/external-api/__tests__/team.int.test.ts +++ b/packages/api/src/routers/external-api/__tests__/team.int.test.ts @@ -1,7 +1,11 @@ import request, { SuperAgentTest } from 'supertest'; +import { createOrgApiKey } from '@/controllers/orgApiKey'; import { getLoggedInAgent, getServer } from '@/fixtures'; +import Alert from '@/models/alert'; import { ITeam } from '@/models/team'; +import TeamInvite from '@/models/teamInvite'; +import User from '@/models/user'; import { IUser } from '@/models/user'; const TEAM_BASE_URL = '/api/v2/team'; @@ -121,4 +125,94 @@ describe('External API v2 Team', () => { ).toBe(1); }); }); + + describe('org API key scopes', () => { + const keyFor = async (scope: 'admin' | 'member') => + ( + await createOrgApiKey(team._id, user._id, { + description: scope, + scope, + expiry: 'never', + }) + ).key; + + const withKey = (key: string, req: request.Test) => + req.set('Authorization', `Bearer ${key}`); + + const createMember = () => + User.create({ email: 'member@deploysentinel.com', team: team._id }); + + it('blocks team administration for member-scoped keys', async () => { + const key = await keyFor('member'); + const invite = await TeamInvite.create({ + teamId: team._id, + email: 'pending@deploysentinel.com', + token: 'token', + }); + const member = await createMember(); + + for (const req of [ + // Invite URLs let anyone join the team, so listing them is admin-only. + agent.get(`${TEAM_BASE_URL}/invitations`), + agent.post(`${TEAM_BASE_URL}/invitation`), + agent.delete(`${TEAM_BASE_URL}/invitation/${invite._id}`), + agent.delete(`${TEAM_BASE_URL}/member/${member._id}`), + ]) { + const res = await withKey(key, req) + .send({ email: 'new@deploysentinel.com' }) + .expect(403); + expect(res.body.message).toBe('This API key does not have admin scope'); + } + }); + + it('lets member-scoped keys list members, none of them current', async () => { + const key = await keyFor('member'); + const res = await withKey( + key, + agent.get(`${TEAM_BASE_URL}/members`), + ).expect(200); + expect(res.body.data.length).toBeGreaterThan(0); + expect( + res.body.data.every( + (m: { isCurrentUser: boolean }) => !m.isCurrentUser, + ), + ).toBe(true); + }); + + it('lets admin-scoped keys invite and remove members', async () => { + const key = await keyFor('admin'); + await withKey(key, agent.post(`${TEAM_BASE_URL}/invitation`)) + .send({ email: 'new@deploysentinel.com' }) + .expect(200); + + const member = await createMember(); + const alert = await Alert.create({ + team: team._id, + threshold: 1, + interval: '5m', + thresholdType: 'above', + source: 'saved_search', + channel: { type: 'webhook', webhookId: 'x' }, + createdBy: member._id, + }); + + await withKey( + key, + agent.delete(`${TEAM_BASE_URL}/member/${member._id}`), + ).expect(200); + expect(await User.findById(member._id)).toBeNull(); + const stored = await Alert.findById(alert._id).lean(); + expect(stored?.createdBy).toBeUndefined(); + }); + + it('stops an admin-scoped key removing the last member', async () => { + const key = await keyFor('admin'); + const res = await withKey( + key, + agent.delete(`${TEAM_BASE_URL}/member/${user._id}`), + ).expect(400); + expect(res.body.message).toBe('Cannot remove the last team member'); + expect(await User.findById(user._id)).not.toBeNull(); + }); + }); }); diff --git a/packages/api/src/routers/external-api/__tests__/v2.int.test.ts b/packages/api/src/routers/external-api/__tests__/v2.int.test.ts index f51c8ebd74..62b599ba83 100644 --- a/packages/api/src/routers/external-api/__tests__/v2.int.test.ts +++ b/packages/api/src/routers/external-api/__tests__/v2.int.test.ts @@ -1,4 +1,7 @@ +import { createOrgApiKey } from '@/controllers/orgApiKey'; import { getLoggedInAgent, getServer } from '@/fixtures'; +import Dashboard from '@/models/dashboard'; +import OrgApiKey from '@/models/orgApiKey'; describe('external api v2', () => { const server = getServer(); @@ -24,5 +27,98 @@ describe('external api v2', () => { .expect(200); expect(resp.body.version).toEqual('v2'); expect(resp.body.user._id).toEqual(user?._id.toString()); + expect(resp.body.user.email).toEqual(user?.email); + }); + + describe('org API keys', () => { + const setup = async () => { + const { agent, team, user } = await getLoggedInAgent(server); + const { doc, key } = await createOrgApiKey(team._id, user._id, { + description: 'terraform', + scope: 'member', + expiry: '30d', + }); + return { agent, team, user, doc, key }; + }; + + it('GET /api/v2 returns only the team and the key', async () => { + const { agent, team, doc, key } = await setup(); + const resp = await agent + .get('/api/v2') + .set('Authorization', `Bearer ${key}`) + .expect(200); + expect(resp.body.user).toEqual({ team: team._id.toString() }); + expect(resp.body.apiKey).toEqual({ + id: doc._id.toString(), + description: 'terraform', + scope: 'member', + }); + }); + + it('lists only its own team data', async () => { + const { agent, team, key } = await setup(); + await Dashboard.create({ name: 'mine', team: team._id, tiles: [] }); + await Dashboard.create({ + name: 'other', + team: new Dashboard()._id, + tiles: [], + }); + const resp = await agent + .get('/api/v2/dashboards') + .set('Authorization', `Bearer ${key}`) + .expect(200); + expect(resp.body.data.map((d: { name: string }) => d.name)).toEqual([ + 'mine', + ]); + }); + + it('rejects expired and revoked keys', async () => { + const { agent, doc, key } = await setup(); + await OrgApiKey.updateOne( + { _id: doc._id }, + { expiresAt: new Date(Date.now() - 1000) }, + ); + await agent + .get('/api/v2') + .set('Authorization', `Bearer ${key}`) + .expect(401); + + await OrgApiKey.deleteOne({ _id: doc._id }); + await agent + .get('/api/v2') + .set('Authorization', `Bearer ${key}`) + .expect(401); + }); + + it('records last use', async () => { + const { agent, doc, key } = await setup(); + await agent + .get('/api/v2') + .set('Authorization', `Bearer ${key}`) + .expect(200); + let lastUsedAt: Date | null | undefined = null; + for (let i = 0; i < 20 && lastUsedAt == null; i++) { + await new Promise(r => setTimeout(r, 50)); + lastUsedAt = (await OrgApiKey.findById(doc._id))?.lastUsedAt; + } + expect(lastUsedAt).toBeInstanceOf(Date); + }); + + it('authenticates MCP requests and rejects revoked keys', async () => { + const { agent, doc, key } = await setup(); + const listTools = () => + agent + .post('/mcp') + .set('Authorization', `Bearer ${key}`) + .set('Accept', 'application/json, text/event-stream') + .send({ jsonrpc: '2.0', id: 1, method: 'tools/list', params: {} }); + + const ok = await listTools(); + expect(ok.status).toBe(200); + expect(ok.text).toContain('clickstack_save_alert'); + + await OrgApiKey.deleteOne({ _id: doc._id }); + expect((await listTools()).status).toBe(401); + }); }); }); diff --git a/packages/api/src/routers/external-api/v2/alerts.ts b/packages/api/src/routers/external-api/v2/alerts.ts index 7a23df9d1a..5603841b47 100644 --- a/packages/api/src/routers/external-api/v2/alerts.ts +++ b/packages/api/src/routers/external-api/v2/alerts.ts @@ -743,8 +743,9 @@ router.post( }), async (req, res, next) => { const teamId = req.user?.team; + // Undefined for org API keys, which leaves createdBy unset. const userId = req.user?._id; - if (teamId == null || userId == null) { + if (teamId == null) { return res.status(403).json({ message: 'Forbidden' }); } try { diff --git a/packages/api/src/routers/external-api/v2/index.ts b/packages/api/src/routers/external-api/v2/index.ts index 6ef19d6d34..439e47053b 100644 --- a/packages/api/src/routers/external-api/v2/index.ts +++ b/packages/api/src/routers/external-api/v2/index.ts @@ -24,6 +24,14 @@ const defaultRateLimiter = rateLimiter({ }); router.get('/', validateUserAccessKey, (req, res) => { + if (req.orgApiKey) { + // Clients such as the CLI source map upload read `user.team`. + return res.json({ + version: 'v2', + user: { team: req.user?.team?.toString() }, + apiKey: req.orgApiKey, + }); + } res.json({ version: 'v2', user: req.user?.toJSON(), diff --git a/packages/api/src/routers/external-api/v2/team.ts b/packages/api/src/routers/external-api/v2/team.ts index 2b5dd7584a..1de6dc9762 100644 --- a/packages/api/src/routers/external-api/v2/team.ts +++ b/packages/api/src/routers/external-api/v2/team.ts @@ -5,11 +5,13 @@ import { validateRequest } from 'zod-express-middleware'; import { getTeam, getTeamInviteUrl } from '@/controllers/team'; import { + countUsersInTeam, deleteTeamMember, findUserByEmail, findUsersByTeam, } from '@/controllers/user'; import TeamInvite from '@/models/teamInvite'; +import { requireAdminRequest } from '@/utils/orgApiKeyPolicy'; import { objectIdSchema } from '@/utils/zod'; const router = express.Router(); @@ -98,9 +100,12 @@ router.get('/members', async (req, res, next) => { * description: Successfully created the invitation * '400': * description: User already exists + * '403': + * description: The API key does not have admin scope */ router.post( '/invitation', + requireAdminRequest, validateRequest({ body: z.object({ email: z.string().email(), @@ -178,8 +183,11 @@ router.post( * responses: * '200': * description: Successfully retrieved invitations + * '403': + * description: The API key does not have admin scope */ -router.get('/invitations', async (req, res, next) => { +// Admin-only because each invite URL lets its holder join the team. +router.get('/invitations', requireAdminRequest, async (req, res, next) => { try { const teamId = req.user?.team; if (!teamId) { @@ -222,11 +230,14 @@ router.get('/invitations', async (req, res, next) => { * responses: * '200': * description: Successfully deleted the invitation + * '403': + * description: The API key does not have admin scope * '404': * description: Invitation not found */ router.delete( '/invitation/:id', + requireAdminRequest, validateRequest({ params: z.object({ id: objectIdSchema }) }), async (req, res, next) => { try { @@ -269,27 +280,42 @@ router.delete( * '200': * description: Successfully removed the member * '400': - * description: Cannot remove yourself from the team + * description: Cannot remove yourself or the last team member + * '403': + * description: The API key does not have admin scope * '404': * description: Member not found */ router.delete( '/member/:id', + requireAdminRequest, validateRequest({ params: z.object({ id: objectIdSchema }) }), async (req, res, next) => { try { const teamId = req.user?.team; + // Undefined for org API keys. const requestingUserId = req.user?._id; - if (!teamId || !requestingUserId) { + if (!teamId) { return res.sendStatus(403); } - if (req.params.id === requestingUserId.toString()) { + if (req.params.id === requestingUserId?.toString()) { return res.status(400).json({ message: 'You cannot remove yourself from the team', }); } + // A user can't remove themselves, so only an org key can empty the team + // and leave nobody able to manage it. + if ( + requestingUserId == null && + (await countUsersInTeam(teamId.toString())) <= 1 + ) { + return res + .status(400) + .json({ message: 'Cannot remove the last team member' }); + } + const deletedUser = await deleteTeamMember( teamId.toString(), req.params.id, diff --git a/packages/api/src/utils/orgApiKeyPolicy.ts b/packages/api/src/utils/orgApiKeyPolicy.ts new file mode 100644 index 0000000000..09d4740b29 --- /dev/null +++ b/packages/api/src/utils/orgApiKeyPolicy.ts @@ -0,0 +1,25 @@ +import type { NextFunction, Request, Response } from 'express'; + +// Permission checks for org API keys. OSS has no roles, so every team member +// counts as an admin; EE replaces both functions with its RBAC checks. + +export function canManageOrgApiKeys(user: Express.User): boolean { + return user.team != null; +} + +function isAdminRequest(req: Request): boolean { + return req.orgApiKey == null || req.orgApiKey.scope === 'admin'; +} + +export function requireAdminRequest( + req: Request, + res: Response, + next: NextFunction, +) { + if (!isAdminRequest(req)) { + return res + .status(403) + .json({ message: 'This API key does not have admin scope' }); + } + next(); +} diff --git a/packages/app/src/TeamPage.tsx b/packages/app/src/TeamPage.tsx index b536fe5290..5236ed4dd1 100644 --- a/packages/app/src/TeamPage.tsx +++ b/packages/app/src/TeamPage.tsx @@ -30,6 +30,7 @@ import ConnectionsSection from './components/TeamSettings/ConnectionsSection'; import IacMigrationSection from './components/TeamSettings/IacMigrationSection'; import IntegrationsSection from './components/TeamSettings/IntegrationsSection'; import McpServerSection from './components/TeamSettings/McpServerSection'; +import OrgApiKeysSection from './components/TeamSettings/OrgApiKeysSection'; import SecurityPoliciesSection from './components/TeamSettings/SecurityPoliciesSection'; import SourcesSection from './components/TeamSettings/SourcesSection'; import TeamMembersSection from './components/TeamSettings/TeamMembersSection'; @@ -166,6 +167,10 @@ export default function TeamPage() { id: 'team-api-agents-api-keys', content: () => , }, + { + id: 'team-api-agents-org-api-keys', + content: () => , + }, { id: 'team-api-agents-mcp-server', content: () => , diff --git a/packages/app/src/api.ts b/packages/app/src/api.ts index 34a49630c2..921ca5509b 100644 --- a/packages/app/src/api.ts +++ b/packages/app/src/api.ts @@ -13,10 +13,14 @@ import type { AlertEvaluationsApiResponse, AlertHistoryRangeApiResponse, AlertsApiResponse, + CreateOrgApiKeyApiResponse, + CreateOrgApiKeyRequest, InstallationApiResponse, MeApiResponse, OnboardingDataApiResponse, OnboardingTaskId, + OrgApiKeyApiResponse, + OrgApiKeysApiResponse, PresetDashboard, PresetDashboardFilter, PrometheusMatrixResult, @@ -88,6 +92,8 @@ const server = ky.create({ timeout: false, }); +const ORG_API_KEYS_QUERY_KEY = ['team', 'api-keys']; + export const hdxServer = ( url: string, options?: Options | undefined, @@ -474,6 +480,53 @@ const api = { }, }); }, + useOrgApiKeys() { + return useQuery({ + queryKey: ORG_API_KEYS_QUERY_KEY, + queryFn: () => hdxServer('team/api-keys').json(), + }); + }, + useCreateOrgApiKey() { + const queryClient = useQueryClient(); + return useMutation< + CreateOrgApiKeyApiResponse, + Error | HTTPError, + CreateOrgApiKeyRequest + >({ + mutationFn: async json => + hdxServer('team/api-keys', { + method: 'POST', + json, + }).json(), + onSuccess: () => + queryClient.invalidateQueries({ queryKey: ORG_API_KEYS_QUERY_KEY }), + }); + }, + useUpdateOrgApiKey() { + const queryClient = useQueryClient(); + return useMutation< + OrgApiKeyApiResponse, + Error | HTTPError, + { id: string; description: string } + >({ + mutationFn: async ({ id, description }) => + hdxServer(`team/api-keys/${id}`, { + method: 'PATCH', + json: { description }, + }).json(), + onSuccess: () => + queryClient.invalidateQueries({ queryKey: ORG_API_KEYS_QUERY_KEY }), + }); + }, + useDeleteOrgApiKey() { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: async ({ id }) => + hdxServer(`team/api-keys/${id}`, { method: 'DELETE' }), + onSuccess: () => + queryClient.invalidateQueries({ queryKey: ORG_API_KEYS_QUERY_KEY }), + }); + }, useDismissOnboarding() { const queryClient = useQueryClient(); return useMutation({ diff --git a/packages/app/src/components/TeamSettings/CreateOrgApiKeyModal.tsx b/packages/app/src/components/TeamSettings/CreateOrgApiKeyModal.tsx new file mode 100644 index 0000000000..cdcb3404a3 --- /dev/null +++ b/packages/app/src/components/TeamSettings/CreateOrgApiKeyModal.tsx @@ -0,0 +1,141 @@ +import { useState } from 'react'; +import { + type OrgApiKeyExpiry, + OrgApiKeyExpirySchema, + type OrgApiKeyScope, + OrgApiKeyScopeSchema, +} from '@hyperdx/common-utils/dist/types'; +import { + Button, + Group, + Modal, + SegmentedControl, + Stack, + Text, + TextInput, +} from '@mantine/core'; +import { notifications } from '@mantine/notifications'; + +import api from '@/api'; +import { RevealSnippet } from '@/components/RevealSnippet/RevealSnippet'; + +const SCOPE_OPTIONS = [ + { value: 'member', label: 'Member' }, + { value: 'admin', label: 'Admin' }, +]; + +const EXPIRY_OPTIONS = [ + { value: '30d', label: '30 days' }, + { value: '1y', label: '1 year' }, + { value: 'never', label: 'Never' }, +]; + +export default function CreateOrgApiKeyModal({ + opened, + onClose, +}: { + opened: boolean; + onClose: () => void; +}) { + const createKey = api.useCreateOrgApiKey(); + const [description, setDescription] = useState(''); + const [scope, setScope] = useState('member'); + const [expiry, setExpiry] = useState('30d'); + const [createdKey, setCreatedKey] = useState(null); + + const close = () => { + setDescription(''); + setScope('member'); + setExpiry('30d'); + setCreatedKey(null); + onClose(); + }; + + const onCreate = () => { + createKey.mutate( + { description: description.trim(), scope, expiry }, + { + onSuccess: data => setCreatedKey(data.key), + onError: e => { + notifications.show({ + color: 'red', + message: e.message, + autoClose: 5000, + }); + }, + }, + ); + }; + + return ( + + {createdKey != null ? ( + + + Copy this key now. You won't be able to see it again. + + + + + + + + + ) : ( + + setDescription(e.currentTarget.value)} + data-autofocus + /> + + + Scope + + setScope(OrgApiKeyScopeSchema.parse(value))} + /> + + Member keys can't manage team members or invitations. + + + + + Expires + + setExpiry(OrgApiKeyExpirySchema.parse(value))} + /> + + + + + + + )} + + ); +} diff --git a/packages/app/src/components/TeamSettings/OrgApiKeysSection.tsx b/packages/app/src/components/TeamSettings/OrgApiKeysSection.tsx new file mode 100644 index 0000000000..b069499ab8 --- /dev/null +++ b/packages/app/src/components/TeamSettings/OrgApiKeysSection.tsx @@ -0,0 +1,146 @@ +import { useState } from 'react'; +import type { OrgApiKeyApiResponse } from '@hyperdx/common-utils/dist/types'; +import { + Badge, + Box, + Button, + Card, + Divider, + Group, + Table, + Text, +} from '@mantine/core'; +import { notifications } from '@mantine/notifications'; + +import api from '@/api'; +import EmptyState from '@/components/EmptyState'; +import { InlineNameInput } from '@/components/InlineNameInput/InlineNameInput'; +import { NOW } from '@/config'; +import { useConfirm } from '@/useConfirm'; + +import CreateOrgApiKeyModal from './CreateOrgApiKeyModal'; + +const formatDate = (iso: string) => new Date(iso).toLocaleDateString(); + +function OrgApiKeyRow({ apiKey }: { apiKey: OrgApiKeyApiResponse }) { + const updateKey = api.useUpdateOrgApiKey(); + const deleteKey = api.useDeleteOrgApiKey(); + const confirm = useConfirm(); + const isExpired = + apiKey.expiresAt != null && Date.parse(apiKey.expiresAt) <= NOW; + + const onRevoke = async () => { + const confirmed = await confirm( + "Revoking this key immediately breaks anything still using it. This can't be undone.", + 'Revoke', + { variant: 'danger' }, + ); + if (!confirmed) { + return; + } + deleteKey.mutate( + { id: apiKey.id }, + { + onError: e => notifications.show({ color: 'red', message: e.message }), + }, + ); + }; + + return ( + + + + updateKey.mutateAsync({ id: apiKey.id, description }) + } + /> + + {apiKey.scope === 'admin' ? 'Admin' : 'Member'} + + {apiKey.createdBy + ? apiKey.createdBy.name || apiKey.createdBy.email + : 'Deleted user'} + + {formatDate(apiKey.createdAt)} + + {apiKey.lastUsedAt ? formatDate(apiKey.lastUsedAt) : 'Never'} + + + {isExpired ? ( + + Expired + + ) : apiKey.expiresAt ? ( + formatDate(apiKey.expiresAt) + ) : ( + 'Never' + )} + + + + + + ); +} + +export default function OrgApiKeysSection() { + const { data, isLoading, isError } = api.useOrgApiKeys(); + const [isCreateOpen, setIsCreateOpen] = useState(false); + const keys = data?.data ?? []; + + return ( + + + + Organization API keys + + Keys owned by the team. They keep working when the person who + created them leaves. + + + + + + {isError ? ( + + Couldn't load organization API keys + + ) : !isLoading && keys.length === 0 ? ( + + ) : ( + + + + + Description + Scope + Created by + Created + Last used + Expires + + + + + {keys.map(apiKey => ( + + ))} + +
+ + )} + setIsCreateOpen(false)} + /> + + ); +} diff --git a/packages/app/src/components/TeamSettings/__tests__/OrgApiKeysSection.test.tsx b/packages/app/src/components/TeamSettings/__tests__/OrgApiKeysSection.test.tsx new file mode 100644 index 0000000000..bc95a95bea --- /dev/null +++ b/packages/app/src/components/TeamSettings/__tests__/OrgApiKeysSection.test.tsx @@ -0,0 +1,135 @@ +import { screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +import api from '@/api'; +import OrgApiKeysSection from '@/components/TeamSettings/OrgApiKeysSection'; +import { useConfirm } from '@/useConfirm'; + +jest.mock('@/api', () => ({ + __esModule: true, + default: { + useOrgApiKeys: jest.fn(), + useCreateOrgApiKey: jest.fn(), + useUpdateOrgApiKey: jest.fn(), + useDeleteOrgApiKey: jest.fn(), + }, + hdxServer: jest.fn(), +})); + +jest.mock('@/useConfirm', () => ({ useConfirm: jest.fn() })); + +const mockUseOrgApiKeys: jest.Mock = jest.mocked(api.useOrgApiKeys); +const mockUseCreate: jest.Mock = jest.mocked(api.useCreateOrgApiKey); +const mockUseUpdate: jest.Mock = jest.mocked(api.useUpdateOrgApiKey); +const mockUseDelete: jest.Mock = jest.mocked(api.useDeleteOrgApiKey); +const mockUseConfirm: jest.Mock = jest.mocked(useConfirm); + +const createMutate = jest.fn(); +const deleteMutate = jest.fn(); +let confirmAccepts = true; + +const key = (overrides = {}) => ({ + id: 'k1', + description: 'terraform', + scope: 'member', + createdAt: '2026-10-01T00:00:00.000Z', + expiresAt: '2099-01-01T00:00:00.000Z', + lastUsedAt: '2026-10-02T00:00:00.000Z', + createdBy: { id: 'u1', name: 'Ada', email: 'ada@example.com' }, + ...overrides, +}); + +const setKeys = (data: ReturnType[]) => + mockUseOrgApiKeys.mockReturnValue({ data: { data }, isLoading: false }); + +beforeEach(() => { + jest.clearAllMocks(); + confirmAccepts = true; + setKeys([]); + mockUseCreate.mockReturnValue({ mutate: createMutate, isPending: false }); + mockUseUpdate.mockReturnValue({ mutateAsync: jest.fn() }); + mockUseDelete.mockReturnValue({ mutate: deleteMutate }); + mockUseConfirm.mockReturnValue(() => Promise.resolve(confirmAccepts)); +}); + +describe('OrgApiKeysSection', () => { + it('shows an empty state with no keys', () => { + renderWithMantine(); + expect( + screen.getByText('No organization API keys yet'), + ).toBeInTheDocument(); + }); + + it('shows an error instead of the empty state when loading fails', () => { + mockUseOrgApiKeys.mockReturnValue({ + data: undefined, + isLoading: false, + isError: true, + }); + renderWithMantine(); + expect( + screen.getByText("Couldn't load organization API keys"), + ).toBeInTheDocument(); + expect( + screen.queryByText('No organization API keys yet'), + ).not.toBeInTheDocument(); + }); + + it('labels deleted creators, unused keys, and expired keys', () => { + setKeys([ + key({ + createdBy: null, + lastUsedAt: null, + expiresAt: '2020-01-01T00:00:00.000Z', + }), + ]); + renderWithMantine(); + expect(screen.getByText('Deleted user')).toBeInTheDocument(); + expect(screen.getByText('Never')).toBeInTheDocument(); + expect(screen.getByText('Expired')).toBeInTheDocument(); + }); + + it('creates a member key expiring in 30 days by default and shows it once', async () => { + createMutate.mockImplementation((_vars, options) => + options?.onSuccess?.({ ...key(), key: 'hdx_org_secret' }), + ); + renderWithMantine(); + + await userEvent.click( + screen.getByRole('button', { name: 'Create API key' }), + ); + await userEvent.type( + await screen.findByLabelText('Description'), + 'terraform', + ); + await userEvent.click(screen.getByRole('button', { name: 'Create' })); + + expect(createMutate).toHaveBeenCalledWith( + { description: 'terraform', scope: 'member', expiry: '30d' }, + expect.anything(), + ); + expect( + await screen.findByText( + "Copy this key now. You won't be able to see it again.", + ), + ).toBeInTheDocument(); + }); + + it('revokes only after confirming', async () => { + setKeys([key()]); + confirmAccepts = false; + renderWithMantine(); + + await userEvent.click(screen.getByRole('button', { name: 'Revoke' })); + await waitFor(() => expect(deleteMutate).not.toHaveBeenCalled()); + + confirmAccepts = true; + await userEvent.click(screen.getByRole('button', { name: 'Revoke' })); + await waitFor(() => + expect(deleteMutate).toHaveBeenCalledWith( + { id: 'k1' }, + expect.anything(), + ), + ); + }); +}); diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index 4c71dbdd34..1bfb8df414 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -3380,6 +3380,61 @@ export type RotateAccessKeyApiResponse = z.infer< typeof RotateAccessKeyApiResponseSchema >; +// Team-owned bearer keys for external API v2 and MCP. They outlive the admin +// who created them; the plaintext `key` is only returned on create. +export const OrgApiKeyScopeSchema = z.enum(['admin', 'member']); +export type OrgApiKeyScope = z.infer; + +export const OrgApiKeyExpirySchema = z.enum(['30d', '1y', 'never']); +export type OrgApiKeyExpiry = z.infer; + +const OrgApiKeyDescriptionSchema = z.string().trim().min(1).max(100); + +export const CreateOrgApiKeyRequestSchema = z.object({ + description: OrgApiKeyDescriptionSchema, + scope: OrgApiKeyScopeSchema, + expiry: OrgApiKeyExpirySchema, +}); +export type CreateOrgApiKeyRequest = z.infer< + typeof CreateOrgApiKeyRequestSchema +>; + +export const UpdateOrgApiKeyRequestSchema = z.object({ + description: OrgApiKeyDescriptionSchema, +}); +export type UpdateOrgApiKeyRequest = z.infer< + typeof UpdateOrgApiKeyRequestSchema +>; + +export const OrgApiKeyApiResponseSchema = z.object({ + id: z.string(), + description: z.string(), + scope: OrgApiKeyScopeSchema, + createdAt: z.string(), + expiresAt: z.string().nullable(), + lastUsedAt: z.string().nullable(), + // Null once the creating user has been removed from the team. + createdBy: z + .object({ + id: z.string(), + name: z.string().optional(), + email: z.string(), + }) + .nullable(), +}); +export type OrgApiKeyApiResponse = z.infer; + +export const OrgApiKeysApiResponseSchema = z.object({ + data: z.array(OrgApiKeyApiResponseSchema), +}); +export type OrgApiKeysApiResponse = z.infer; + +export const CreateOrgApiKeyApiResponseSchema = + OrgApiKeyApiResponseSchema.extend({ key: z.string() }); +export type CreateOrgApiKeyApiResponse = z.infer< + typeof CreateOrgApiKeyApiResponseSchema +>; + // IaC (Terraform) export // // Shared so `GET /iac/import-manifest` and the generators in From 256d5e9b1e93ba93aee79bda51fd41f4bd23105f Mon Sep 17 00:00:00 2001 From: Jordan Simonovski Date: Thu, 22 Oct 2026 09:09:32 +1100 Subject: [PATCH 2/5] fix: address review findings on org API keys - An org key removing members concurrently can no longer empty the team. A removal that leaves it empty puts the member back, credentials intact. - Bearer auth responds with an error on a database failure instead of leaving the request hanging. - Express.User no longer claims _id and email exist on org-key requests; session-only routes get them through getSessionUser. - Failed last-used writes are counted and recorded on the span. - The Expired badge updates while the page is open, failed description edits show an error, and the create dialog can't close mid-request. --- packages/api/src/controllers/orgApiKey.ts | 11 +++ packages/api/src/controllers/user.ts | 80 +++++++++++++++---- packages/api/src/middleware/auth.ts | 48 +++++++---- packages/api/src/routers/api/ai.ts | 4 +- packages/api/src/routers/api/favorites.ts | 8 +- packages/api/src/routers/api/me.ts | 12 +-- packages/api/src/routers/api/orgApiKeys.ts | 17 ++-- .../external-api/__tests__/team.int.test.ts | 25 +++++- .../external-api/__tests__/v2.int.test.ts | 12 +++ .../api/src/routers/external-api/v2/team.ts | 25 +++--- .../TeamSettings/CreateOrgApiKeyModal.tsx | 10 ++- .../TeamSettings/OrgApiKeysSection.tsx | 41 ++++++++-- .../__tests__/OrgApiKeysSection.test.tsx | 46 ++++++++++- 13 files changed, 268 insertions(+), 71 deletions(-) diff --git a/packages/api/src/controllers/orgApiKey.ts b/packages/api/src/controllers/orgApiKey.ts index 2963c64d89..8c7bf74c4c 100644 --- a/packages/api/src/controllers/orgApiKey.ts +++ b/packages/api/src/controllers/orgApiKey.ts @@ -3,11 +3,13 @@ import type { OrgApiKeyApiResponse, OrgApiKeyExpiry, } from '@hyperdx/common-utils/dist/types'; +import opentelemetry from '@opentelemetry/api'; import crypto from 'crypto'; import type { ObjectId } from '@/models'; import OrgApiKey, { type OrgApiKeyDocument } from '@/models/orgApiKey'; import type { IUser } from '@/models/user'; +import { getCounter } from '@/utils/instrumentation'; import logger from '@/utils/logger'; export const ORG_API_KEY_PREFIX = 'hdx_org_'; @@ -15,6 +17,11 @@ export const ORG_API_KEY_PREFIX = 'hdx_org_'; const DAY_MS = 24 * 60 * 60 * 1000; const LAST_USED_WRITE_INTERVAL_MS = 5 * 60 * 1000; +const lastUsedWriteFailures = getCounter( + 'hyperdx.org_api_key.last_used_write_failures', + { description: 'Failed writes of an org API key last-used time.' }, +); + function expiresAtFor(expiry: OrgApiKeyExpiry, now: number): Date | null { switch (expiry) { case '30d': @@ -89,6 +96,8 @@ export function touchOrgApiKeyLastUsed( if (doc.lastUsedAt != null && doc.lastUsedAt > staleBefore) { return; } + // Captured now: the write settles after the request may have moved on. + const span = opentelemetry.trace.getActiveSpan(); void OrgApiKey.updateOne( { _id: doc._id, @@ -96,6 +105,8 @@ export function touchOrgApiKeyLastUsed( }, { lastUsedAt: now }, ).catch(err => { + lastUsedWriteFailures.add(1); + span?.recordException(err instanceof Error ? err : new Error(String(err))); logger.warn( { error: err, orgApiKeyId: doc._id.toString() }, 'Failed to record org API key last use', diff --git a/packages/api/src/controllers/user.ts b/packages/api/src/controllers/user.ts index 03f1860aa7..c1ff8c0252 100644 --- a/packages/api/src/controllers/user.ts +++ b/packages/api/src/controllers/user.ts @@ -5,7 +5,8 @@ import { v4 as uuidv4 } from 'uuid'; import type { ObjectId } from '@/models'; import Alert from '@/models/alert'; -import User from '@/models/user'; +import User, { type IUser } from '@/models/user'; +import { getCounter } from '@/utils/instrumentation'; import logger from '@/utils/logger'; export function findUserByAccessKey(accessKey: string) { return User.findOne({ accessKey }); @@ -31,10 +32,6 @@ export function findUserByEmail(email: string) { return User.findOne({ email: email.toLowerCase() }); } -export function countUsersInTeam(team: string | ObjectId) { - return User.countDocuments({ team }); -} - export function findUsersByTeam(team: string | ObjectId) { return User.find({ team }).sort({ createdAt: 1 }); } @@ -102,19 +99,16 @@ export function recordOnboardingTaskCompletion( export async function deleteTeamMember( teamId: string | ObjectId, userIdToDelete: string, - // Undefined when an org API key made the request: alerts lose their owner. - userIdRequestingDelete?: string | ObjectId, + userIdRequestingDelete: string | ObjectId, ) { const [, deletedUser] = await Promise.all([ Alert.updateMany( { createdBy: new mongoose.Types.ObjectId(userIdToDelete), team: teamId }, - userIdRequestingDelete == null - ? { $unset: { createdBy: '' } } - : { - $set: { - createdBy: new mongoose.Types.ObjectId(userIdRequestingDelete), - }, - }, + { + $set: { + createdBy: new mongoose.Types.ObjectId(userIdRequestingDelete), + }, + }, ), User.findOneAndDelete({ team: teamId, @@ -124,3 +118,61 @@ export async function deleteTeamMember( return deletedUser; } + +/** + * Member removal by an org API key, which has no user to hand alerts to and + * must not empty the team. Mongo here has no transactions, so delete first + * and put the member back if a concurrent removal left the team empty. + */ +export async function deleteTeamMemberKeepingOne( + teamId: string | ObjectId, + userIdToDelete: string, +): Promise<'deleted' | 'not_found' | 'last_member'> { + const deletedUser = await User.findOneAndDelete({ + team: teamId, + _id: userIdToDelete, + }) + .select('+hash +salt') + .lean(); + if (deletedUser == null) { + return 'not_found'; + } + let isTeamEmpty = true; + try { + isTeamEmpty = (await User.countDocuments({ team: teamId })) === 0; + } finally { + // Also runs if the count throws, so a failed check can't lose the member. + if (isTeamEmpty) { + await restoreTeamMember(deletedUser); + } + } + if (isTeamEmpty) { + return 'last_member'; + } + await Alert.updateMany( + { createdBy: deletedUser._id, team: teamId }, + { $unset: { createdBy: '' } }, + ); + return 'deleted'; +} + +const memberRestoreFailures = getCounter( + 'hyperdx.team.member_restore_failures', + { + description: + 'Members deleted by an org API key that could not be put back.', + }, +); + +async function restoreTeamMember(user: IUser) { + try { + await User.collection.insertOne(user); + } catch (err) { + memberRestoreFailures.add(1); + logger.error( + { error: err, userId: user._id.toString(), teamId: user.team.toString() }, + 'Failed to restore team member removed by an org API key', + ); + throw err; + } +} diff --git a/packages/api/src/middleware/auth.ts b/packages/api/src/middleware/auth.ts index cd163d75fd..72eaca39c4 100644 --- a/packages/api/src/middleware/auth.ts +++ b/packages/api/src/middleware/auth.ts @@ -27,8 +27,12 @@ declare global { // no non-namespace / non-empty-interface equivalent for extending these types. // eslint-disable-next-line @typescript-eslint/no-namespace namespace Express { - // eslint-disable-next-line @typescript-eslint/no-empty-object-type - interface User extends UserDocument {} + // _id and email are unset for org API key requests, which carry only the + // team. Session-only routes use getSessionUser to get them non-null. + interface User extends Omit { + _id?: UserDocument['_id']; + email?: UserDocument['email']; + } interface Request { _hdx_connection?: Connection; // Set when the bearer token is a team-owned key rather than a user's. @@ -106,21 +110,27 @@ export async function validateUserAccessKey( return res.sendStatus(401); } - const user = await findUserByAccessKey(key); - if (!user) { - return authenticateOrgApiKey(key, req, res, next); - } + // Express 4 drops rejected promises from middleware, which would leave the + // request hanging on a database error. + try { + const user = await findUserByAccessKey(key); + if (!user) { + return await authenticateOrgApiKey(key, req, res, next); + } - req.user = user; + req.user = user; - // Attribute access-key authenticated requests (external API v2 + MCP HTTP) - // with team/user context so their traces are searchable during incidents. - setBusinessContext({ - teamId: user.team?.toString(), - userId: user._id?.toString(), - email: user.email, - ...getStaticFeatureFlags(), - }); + // Attribute access-key authenticated requests (external API v2 + MCP HTTP) + // with team/user context so their traces are searchable during incidents. + setBusinessContext({ + teamId: user.team?.toString(), + userId: user._id?.toString(), + email: user.email, + ...getStaticFeatureFlags(), + }); + } catch (err) { + return next(err); + } nextWithQueryAttribution(req, next); } @@ -232,3 +242,11 @@ export function getNonNullUserWithTeam(req: Request) { return { teamId: user.team, userId: user._id, email: user.email }; } + +export function getSessionUser(req: Request) { + const { teamId, userId, email } = getNonNullUserWithTeam(req); + if (userId == null || email == null) { + throw new Error('Route requires a signed-in user'); + } + return { teamId, userId, email }; +} diff --git a/packages/api/src/routers/api/ai.ts b/packages/api/src/routers/api/ai.ts index 789bab236a..cd37a5db6e 100644 --- a/packages/api/src/routers/api/ai.ts +++ b/packages/api/src/routers/api/ai.ts @@ -13,7 +13,7 @@ import { getChartConfigFromResolvedConfig, } from '@/controllers/ai'; import { getSource } from '@/controllers/sources'; -import { getNonNullUserWithTeam } from '@/middleware/auth'; +import { getSessionUser } from '@/middleware/auth'; import { llmTelemetry } from '@/utils/aiTelemetry'; import { Api404Error, Api500Error } from '@/utils/errors'; import { withOperationMetrics } from '@/utils/instrumentation'; @@ -34,7 +34,7 @@ router.post( try { const model = getAIModel(); - const { teamId, userId } = getNonNullUserWithTeam(req); + const { teamId, userId } = getSessionUser(req); const { text, sourceId } = req.body; diff --git a/packages/api/src/routers/api/favorites.ts b/packages/api/src/routers/api/favorites.ts index 55ea1e4f96..1dc66e871c 100644 --- a/packages/api/src/routers/api/favorites.ts +++ b/packages/api/src/routers/api/favorites.ts @@ -7,7 +7,7 @@ import { getFavorites, removeFavorite, } from '@/controllers/favorite'; -import { getNonNullUserWithTeam } from '@/middleware/auth'; +import { getSessionUser } from '@/middleware/auth'; import { objectIdSchema } from '@/utils/zod'; const router = express.Router(); @@ -16,7 +16,7 @@ const resourceTypeSchema = z.enum(['dashboard', 'savedSearch']); router.get('/', async (req, res, next) => { try { - const { teamId, userId } = getNonNullUserWithTeam(req); + const { teamId, userId } = getSessionUser(req); const favorites = await getFavorites(userId.toString(), teamId.toString()); @@ -36,7 +36,7 @@ router.put( }), async (req, res, next) => { try { - const { teamId, userId } = getNonNullUserWithTeam(req); + const { teamId, userId } = getSessionUser(req); const favorite = await addFavorite( userId.toString(), @@ -62,7 +62,7 @@ router.delete( }), async (req, res, next) => { try { - const { teamId, userId } = getNonNullUserWithTeam(req); + const { teamId, userId } = getSessionUser(req); await removeFavorite( userId.toString(), diff --git a/packages/api/src/routers/api/me.ts b/packages/api/src/routers/api/me.ts index 4fc71aec0e..0f737db6ba 100644 --- a/packages/api/src/routers/api/me.ts +++ b/packages/api/src/routers/api/me.ts @@ -18,6 +18,7 @@ import { rotateUserAccessKey, setOnboardingDismissed, } from '@/controllers/user'; +import { getSessionUser } from '@/middleware/auth'; import { Api404Error } from '@/utils/errors'; import { sendJson } from '@/utils/serialization'; @@ -29,15 +30,8 @@ router.get('/', async (req, res: express.Response, next) => { throw new Api404Error('Request without user found'); } - const { - _id: id, - accessKey, - createdAt, - email, - name, - onboardingData, - team: teamId, - } = req.user; + const { userId: id, email, teamId } = getSessionUser(req); + const { accessKey, createdAt, name, onboardingData } = req.user; const team = await getTeam(teamId); if (team == null) { diff --git a/packages/api/src/routers/api/orgApiKeys.ts b/packages/api/src/routers/api/orgApiKeys.ts index ccd8aa850b..68a66ca201 100644 --- a/packages/api/src/routers/api/orgApiKeys.ts +++ b/packages/api/src/routers/api/orgApiKeys.ts @@ -18,6 +18,7 @@ import { listOrgApiKeys, updateOrgApiKeyDescription, } from '@/controllers/orgApiKey'; +import { getSessionUser } from '@/middleware/auth'; import { getCounter } from '@/utils/instrumentation'; import { canManageOrgApiKeys } from '@/utils/orgApiKeyPolicy'; import { objectIdSchema } from '@/utils/zod'; @@ -40,7 +41,7 @@ router.get( '/', async (req, res: express.Response, next) => { try { - const keys = await listOrgApiKeys(req.user!.team); + const keys = await listOrgApiKeys(getSessionUser(req).teamId); res.json({ data: keys.map(formatOrgApiKey) }); } catch (e) { next(e); @@ -53,11 +54,8 @@ router.post( processRequest({ body: CreateOrgApiKeyRequestSchema }), async (req, res: express.Response, next) => { try { - const { doc, key } = await createOrgApiKey( - req.user!.team, - req.user!._id, - req.body, - ); + const { teamId, userId } = getSessionUser(req); + const { doc, key } = await createOrgApiKey(teamId, userId, req.body); await doc.populate('createdBy', 'name email'); orgApiKeyOperations.add(1, { operation: 'create' }); res.json({ ...formatOrgApiKey(doc), key }); @@ -76,7 +74,7 @@ router.patch( async (req, res: express.Response, next) => { try { const doc = await updateOrgApiKeyDescription( - req.user!.team, + getSessionUser(req).teamId, req.params.id, req.body.description, ); @@ -96,7 +94,10 @@ router.delete( validateRequest({ params: z.object({ id: objectIdSchema }) }), async (req, res, next) => { try { - const doc = await deleteOrgApiKey(req.user!.team, req.params.id); + const doc = await deleteOrgApiKey( + getSessionUser(req).teamId, + req.params.id, + ); if (doc == null) { return res.sendStatus(404); } diff --git a/packages/api/src/routers/external-api/__tests__/team.int.test.ts b/packages/api/src/routers/external-api/__tests__/team.int.test.ts index 05968fbd44..c260cbd680 100644 --- a/packages/api/src/routers/external-api/__tests__/team.int.test.ts +++ b/packages/api/src/routers/external-api/__tests__/team.int.test.ts @@ -212,7 +212,30 @@ describe('External API v2 Team', () => { agent.delete(`${TEAM_BASE_URL}/member/${user._id}`), ).expect(400); expect(res.body.message).toBe('Cannot remove the last team member'); - expect(await User.findById(user._id)).not.toBeNull(); + // The member is deleted then put back, and must keep their credentials. + const restored = await User.findById(user._id).select('+hash +salt'); + expect(restored?.get('hash')).toBeTruthy(); + expect(restored?.accessKey).toBe(user.accessKey); + }); + + it('keeps a member when an admin key removes the last two at once', async () => { + const key = await keyFor('admin'); + const member = await createMember(); + const results = await Promise.all( + [user._id, member._id].map(id => + withKey(key, agent.delete(`${TEAM_BASE_URL}/member/${id}`)), + ), + ); + expect(results.map(r => r.status).sort()).not.toEqual([200, 200]); + expect(await User.countDocuments({ team: team._id })).toBeGreaterThan(0); + }); + + it('returns 404 for a non-member before applying the last-member rule', async () => { + const key = await keyFor('admin'); + await withKey( + key, + agent.delete(`${TEAM_BASE_URL}/member/${new Alert()._id}`), + ).expect(404); }); }); }); diff --git a/packages/api/src/routers/external-api/__tests__/v2.int.test.ts b/packages/api/src/routers/external-api/__tests__/v2.int.test.ts index 62b599ba83..3523ea144f 100644 --- a/packages/api/src/routers/external-api/__tests__/v2.int.test.ts +++ b/packages/api/src/routers/external-api/__tests__/v2.int.test.ts @@ -1,3 +1,4 @@ +import * as orgApiKeyController from '@/controllers/orgApiKey'; import { createOrgApiKey } from '@/controllers/orgApiKey'; import { getLoggedInAgent, getServer } from '@/fixtures'; import Dashboard from '@/models/dashboard'; @@ -120,5 +121,16 @@ describe('external api v2', () => { await OrgApiKey.deleteOne({ _id: doc._id }); expect((await listTools()).status).toBe(401); }); + + it('responds with an error when the key lookup fails', async () => { + const { agent, key } = await setup(); + jest + .spyOn(orgApiKeyController, 'findActiveOrgApiKey') + .mockRejectedValueOnce(new Error('mongo unavailable')); + await agent + .get('/api/v2') + .set('Authorization', `Bearer ${key}`) + .expect(500); + }); }); }); diff --git a/packages/api/src/routers/external-api/v2/team.ts b/packages/api/src/routers/external-api/v2/team.ts index 1de6dc9762..2178c3c565 100644 --- a/packages/api/src/routers/external-api/v2/team.ts +++ b/packages/api/src/routers/external-api/v2/team.ts @@ -5,8 +5,8 @@ import { validateRequest } from 'zod-express-middleware'; import { getTeam, getTeamInviteUrl } from '@/controllers/team'; import { - countUsersInTeam, deleteTeamMember, + deleteTeamMemberKeepingOne, findUserByEmail, findUsersByTeam, } from '@/controllers/user'; @@ -305,15 +305,20 @@ router.delete( }); } - // A user can't remove themselves, so only an org key can empty the team - // and leave nobody able to manage it. - if ( - requestingUserId == null && - (await countUsersInTeam(teamId.toString())) <= 1 - ) { - return res - .status(400) - .json({ message: 'Cannot remove the last team member' }); + if (requestingUserId == null) { + const result = await deleteTeamMemberKeepingOne( + teamId.toString(), + req.params.id, + ); + if (result === 'not_found') { + return res.sendStatus(404); + } + if (result === 'last_member') { + return res + .status(400) + .json({ message: 'Cannot remove the last team member' }); + } + return res.json({ data: { success: true } }); } const deletedUser = await deleteTeamMember( diff --git a/packages/app/src/components/TeamSettings/CreateOrgApiKeyModal.tsx b/packages/app/src/components/TeamSettings/CreateOrgApiKeyModal.tsx index cdcb3404a3..1b0c9fd932 100644 --- a/packages/app/src/components/TeamSettings/CreateOrgApiKeyModal.tsx +++ b/packages/app/src/components/TeamSettings/CreateOrgApiKeyModal.tsx @@ -43,7 +43,11 @@ export default function CreateOrgApiKeyModal({ const [expiry, setExpiry] = useState('30d'); const [createdKey, setCreatedKey] = useState(null); + // Closing mid-request would let the late response reopen on the old key. const close = () => { + if (createKey.isPending) { + return; + } setDescription(''); setScope('member'); setExpiry('30d'); @@ -122,7 +126,11 @@ export default function CreateOrgApiKeyModal({ /> -