diff --git a/.changeset/org-api-keys.md b/.changeset/org-api-keys.md new file mode 100644 index 0000000000..e2c31044ce --- /dev/null +++ b/.changeset/org-api-keys.md @@ -0,0 +1,7 @@ +--- +'@hyperdx/api': minor +'@hyperdx/app': minor +'@hyperdx/common-utils': minor +--- + +feat: Team members can create team-owned API keys with a description and expiry that keep working after their creator leaves. diff --git a/packages/api/openapi.json b/packages/api/openapi.json index bffa35d627..4c38a71948 100644 --- a/packages/api/openapi.json +++ b/packages/api/openapi.json @@ -8937,6 +8937,9 @@ }, "400": { "description": "User already exists" + }, + "403": { + "description": "Org API keys can't invite people" } } } @@ -8951,7 +8954,7 @@ ], "responses": { "200": { - "description": "Successfully retrieved invitations" + "description": "Successfully retrieved invitations. Invite links are left out for org API keys." } } } @@ -9007,7 +9010,7 @@ "description": "Successfully removed the member" }, "400": { - "description": "Cannot remove yourself from the team" + "description": "Cannot remove yourself or the last team member" }, "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..f5dd34aaa1 --- /dev/null +++ b/packages/api/src/controllers/__tests__/orgApiKey.int.test.ts @@ -0,0 +1,249 @@ +import mongoose from 'mongoose'; + +import { + createOrgApiKey, + listOrgApiKeys, + revokeOrgApiKey, + touchOrgApiKeyLastUsed, + updateOrgApiKeyDescription, +} from '@/controllers/orgApiKey'; +import { createTeam } from '@/controllers/team'; +import { + createVirtualUser, + deleteTeamMember, + findUserByAccessKey, + findUsersByTeam, + hashOrgApiKey, +} from '@/controllers/user'; +import { clearDBCollections, closeDB, connectDB } from '@/fixtures'; +import User from '@/models/user'; + +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', + 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 User.findById(doc._id).lean(); + expect(stored?.accessKey).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 findUserByAccessKey(key))?._id).toEqual(doc._id); + expect( + await findUserByAccessKey(key, doc.expiresAt ?? new Date(0)), + ).toBeNull(); + expect(await findUserByAccessKey(`hdx_org_${'x'.repeat(43)}`)).toBeNull(); + expect( + await findUserByAccessKey('4b9e3c1a-7f2d-4e8b-9a6c-1d2e3f4a5b6c'), + ).toBeNull(); + // The stored hash is not a credential. + expect(await findUserByAccessKey(hashOrgApiKey(key))).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 revokeOrgApiKey(teamB, doc._id.toString())).toBeNull(); + expect(await listOrgApiKeys(teamA._id)).toHaveLength(1); + }); + + it('leaves virtual users that are not org keys alone', async () => { + const team = await createTeam({ name: 'A' }); + const other = await createVirtualUser({ + teamId: team._id, + name: 'actor', + email: 'actor@example.com', + }); + + expect(await listOrgApiKeys(team._id)).toHaveLength(0); + expect(await revokeOrgApiKey(team._id, other._id.toString())).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 User.findById(doc._id))?.lastUsedAt?.getTime(); + }; + + touchOrgApiKeyLastUsed(doc, t0); + expect(await lastUsed()).toBe(t0.getTime()); + + const fresh = (await User.findById(doc._id))!; + touchOrgApiKeyLastUsed(fresh, new Date(t0.getTime() + 60_000)); + expect(await lastUsed()).toBe(t0.getTime()); + + // A request holding a stale copy passes the in-memory check, so only the + // conditional filter stops it overwriting a recent write. + touchOrgApiKeyLastUsed( + { _id: doc._id, lastUsedAt: null }, + 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()); + }); + + describe('service accounts', () => { + it('creates a service account user for each key', async () => { + const team = await createTeam({ name: 'A' }); + const { doc } = await create(team._id); + const account = await User.findById(doc._id); + + expect(account).toMatchObject({ + isVirtual: true, + name: 'terraform', + }); + expect(account?.team).toEqual(team._id); + expect(account?.email).toMatch( + /^sa-[0-9a-f]{24}@service-accounts\.hyperdx\.invalid$/, + ); + }); + + it('keeps service accounts out of members', async () => { + const team = await createTeam({ name: 'A' }); + await create(team._id); + expect(await findUsersByTeam(team._id)).toHaveLength(0); + }); + + it('renames the service account with the key', async () => { + const team = await createTeam({ name: 'A' }); + const { doc } = await create(team._id); + await updateOrgApiKeyDescription(team._id, doc._id.toString(), 'ci'); + expect((await User.findById(doc._id))?.name).toBe('ci'); + }); + + it('keeps the service account when the key is revoked', async () => { + const team = await createTeam({ name: 'A' }); + const { doc, key } = await create(team._id); + await revokeOrgApiKey(team._id, doc._id.toString()); + expect(await findUserByAccessKey(key)).toBeNull(); + expect(await listOrgApiKeys(team._id)).toHaveLength(0); + expect(await User.findById(doc._id)).not.toBeNull(); + }); + }); + + describe('member removal by a service account', () => { + const setupTeam = async (people: number) => { + const team = await createTeam({ name: 'A' }); + const { doc } = await create(team._id); + const users = await Promise.all( + Array.from({ length: people }, (_, i) => + User.create({ email: `p${i}@example.com`, team: team._id }), + ), + ); + return { team, serviceAccount: doc._id, users }; + }; + + afterEach(() => jest.restoreAllMocks()); + + it('refuses the last person without deleting them', async () => { + const { team, serviceAccount, users } = await setupTeam(1); + const deleteSpy = jest.spyOn(User, 'findOneAndDelete'); + + const result = await deleteTeamMember( + team._id, + users[0]._id.toString(), + serviceAccount, + ); + expect(result).toBe('last_member'); + expect(deleteSpy).not.toHaveBeenCalled(); + }); + + it('retries putting a member back when the first insert fails', async () => { + const { team, serviceAccount, users } = await setupTeam(2); + // Simulate a concurrent removal: two people before the delete, none after. + // Widened to SpyInstance: the Mongoose overloads type these mocks as never. + const count: jest.SpyInstance = jest.spyOn(User, 'countDocuments'); + count.mockResolvedValueOnce(2).mockResolvedValueOnce(0); + const insert: jest.SpyInstance = jest.spyOn(User.collection, 'insertOne'); + insert.mockRejectedValueOnce(new Error('transient')); + + const result = await deleteTeamMember( + team._id, + users[0]._id.toString(), + serviceAccount, + ); + expect(result).toBe('last_member'); + expect(await User.findById(users[0]._id)).not.toBeNull(); + }); + + it('treats a duplicate _id on restore as already restored', async () => { + const { team, serviceAccount, users } = await setupTeam(2); + const count: jest.SpyInstance = jest.spyOn(User, 'countDocuments'); + count.mockResolvedValueOnce(2).mockResolvedValueOnce(0); + const insert: jest.SpyInstance = jest.spyOn(User.collection, 'insertOne'); + insert.mockRejectedValueOnce( + Object.assign(new Error('E11000'), { + code: 11000, + keyPattern: { _id: 1 }, + }), + ); + + const result = await deleteTeamMember( + team._id, + users[0]._id.toString(), + serviceAccount, + ); + expect(result).toBe('last_member'); + expect(insert).toHaveBeenCalledTimes(1); + }); + }); +}); diff --git a/packages/api/src/controllers/orgApiKey.ts b/packages/api/src/controllers/orgApiKey.ts new file mode 100644 index 0000000000..059c7ae747 --- /dev/null +++ b/packages/api/src/controllers/orgApiKey.ts @@ -0,0 +1,133 @@ +import type { + CreateOrgApiKeyRequest, + OrgApiKeyApiResponse, + OrgApiKeyExpiry, +} from '@hyperdx/common-utils/dist/types'; +import opentelemetry from '@opentelemetry/api'; +import crypto from 'crypto'; +import mongoose from 'mongoose'; + +import { + ACTIVE_ORG_API_KEY, + createVirtualUser, + hashOrgApiKey, + ORG_API_KEY_PREFIX, +} from '@/controllers/user'; +import type { ObjectId } from '@/models'; +import User, { type IUser, type UserDocument } from '@/models/user'; +import { getCounter } from '@/utils/instrumentation'; +import logger from '@/utils/logger'; + +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': + return new Date(now + 30 * DAY_MS); + case '1y': + return new Date(now + 365 * DAY_MS); + case 'never': + return null; + } +} + +export async function createOrgApiKey( + teamId: string | ObjectId, + createdBy: string | ObjectId, + { description, expiry }: CreateOrgApiKeyRequest, +) { + const key = `${ORG_API_KEY_PREFIX}${crypto.randomBytes(32).toString('base64url')}`; + // `.invalid` is reserved, so the email can never reach a real inbox. + const doc = await createVirtualUser({ + teamId, + name: description, + email: `sa-${crypto.randomBytes(12).toString('hex')}@service-accounts.hyperdx.invalid`, + accessKey: hashOrgApiKey(key), + expiresAt: expiresAtFor(expiry, Date.now()), + createdBy: new mongoose.Types.ObjectId(createdBy), + }); + return { doc, key }; +} + +export function listOrgApiKeys(teamId: string | ObjectId) { + return User.find({ team: teamId, ...ACTIVE_ORG_API_KEY }) + .sort({ createdAt: -1 }) + .populate<{ createdBy: IUser | null }>('createdBy', 'name email'); +} + +export function updateOrgApiKeyDescription( + teamId: string | ObjectId, + id: string, + description: string, +) { + return User.findOneAndUpdate( + { _id: id, team: teamId, ...ACTIVE_ORG_API_KEY }, + { name: description }, + { new: true }, + ).populate<{ createdBy: IUser | null }>('createdBy', 'name email'); +} + +// Marks rather than deletes, so anything the account created keeps its author. +export function revokeOrgApiKey(teamId: string | ObjectId, id: string) { + return User.findOneAndUpdate( + { _id: id, team: teamId, ...ACTIVE_ORG_API_KEY }, + { revokedAt: new Date() }, + ); +} + +// 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: Pick, + now = new Date(), +) { + const staleBefore = new Date(now.getTime() - LAST_USED_WRITE_INTERVAL_MS); + 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 User.updateOne( + { + _id: doc._id, + $or: [{ lastUsedAt: null }, { lastUsedAt: { $lt: staleBefore } }], + }, + { 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', + ); + }); +} + +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.name, + 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..51e8a451e9 100644 --- a/packages/api/src/controllers/user.ts +++ b/packages/api/src/controllers/user.ts @@ -1,14 +1,44 @@ import type { OnboardingTaskId } from '@hyperdx/common-utils/dist/types'; import { isPersistableUserId as isPersistableUserIdHex } from '@hyperdx/common-utils/dist/types'; +import opentelemetry from '@opentelemetry/api'; +import crypto from 'crypto'; import mongoose from 'mongoose'; 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 { isDuplicateKeyError } from '@/utils/errors'; +import { getCounter } from '@/utils/instrumentation'; import logger from '@/utils/logger'; -export function findUserByAccessKey(accessKey: string) { - return User.findOne({ accessKey }); + +export const NOT_VIRTUAL = { isVirtual: { $ne: true } }; + +export const ORG_API_KEY_PREFIX = 'hdx_org_'; + +// Virtual users with no createdBy (such as EE's role and CHC actor accounts) +// are not org keys, so they never authenticate as one or show up in key management. +export const ACTIVE_ORG_API_KEY = { + isVirtual: true, + createdBy: { $ne: null }, + revokedAt: null, +} as const; + +export function hashOrgApiKey(key: string) { + return crypto.createHash('sha256').update(key).digest('hex'); +} + +export function findUserByAccessKey(accessKey: string, now = new Date()) { + // Org keys are stored hashed; only the prefixed plaintext reaches a service + // account, so presenting the stored hash itself never authenticates. + if (accessKey.startsWith(ORG_API_KEY_PREFIX)) { + return User.findOne({ + accessKey: hashOrgApiKey(accessKey), + ...ACTIVE_ORG_API_KEY, + $or: [{ expiresAt: null }, { expiresAt: { $gt: now } }], + }); + } + return User.findOne({ accessKey, ...NOT_VIRTUAL }); } /** @@ -26,13 +56,32 @@ export function findUserById(id: string) { return User.findById(id); } +export function createVirtualUser({ + name, + accessKey, + teamId, + ...fields +}: { + name: string; + accessKey?: string; + teamId: string | ObjectId; +} & Pick) { + return User.create({ + name, + team: teamId, + isVirtual: true, + accessKey, + ...fields, + }); +} + export function findUserByEmail(email: string) { // Case-insensitive email search - lowercase the email since User model stores emails in lowercase return User.findOne({ email: email.toLowerCase() }); } export function findUsersByTeam(team: string | ObjectId) { - return User.find({ team }).sort({ createdAt: 1 }); + return User.find({ team, ...NOT_VIRTUAL }).sort({ createdAt: 1 }); } // Type-guard wrapper over the shared 24-hex check; rejects the synthetic @@ -85,7 +134,11 @@ export function recordOnboardingTaskCompletion( return; } void User.updateOne( - { _id: userId, 'onboardingData.completedTasks': { $ne: taskId } }, + { + _id: userId, + ...NOT_VIRTUAL, + 'onboardingData.completedTasks': { $ne: taskId }, + }, { $addToSet: { 'onboardingData.completedTasks': taskId } }, ).catch(err => { logger.warn( @@ -95,25 +148,103 @@ export function recordOnboardingTaskCompletion( }); } +/** + * Never removes the last person, so someone can always sign in to revoke the + * team's org API keys. Mongo here has no transactions, so delete first and put + * the member back if a concurrent removal emptied the team. + */ export async function deleteTeamMember( teamId: string | ObjectId, userIdToDelete: string, 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), +): Promise<'deleted' | 'not_found' | 'last_member'> { + const countPeople = () => + User.countDocuments({ team: teamId, ...NOT_VIRTUAL }); + const target = { team: teamId, _id: userIdToDelete, ...NOT_VIRTUAL }; + if ((await User.exists(target)) == null) { + return 'not_found'; + } + // Checked first so the usual case never deletes; the restore below only + // covers two removals racing past this check. + if ((await countPeople()) <= 1) { + return 'last_member'; + } + const deletedUser = await User.findOneAndDelete(target) + .select('+hash +salt') + .lean(); + if (deletedUser == null) { + return 'not_found'; + } + let isTeamEmpty = true; + try { + isTeamEmpty = (await countPeople()) === 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 }, + { + $set: { createdBy: new mongoose.Types.ObjectId(userIdRequestingDelete) }, + }, + ); + return 'deleted'; +} + +const memberRestoreFailures = getCounter( + 'hyperdx.team.member_restore_failures', + { + description: + 'Removed team members that could not be put back after the team emptied.', + }, +); + +const RESTORE_ATTEMPTS = 3; +const RESTORE_RETRY_DELAY_MS = 100; + +async function restoreTeamMember(user: IUser) { + for (let attempt = 1; ; attempt++) { + try { + await User.collection.insertOne(user); + return; + } catch (err) { + const isDuplicate = isDuplicateKeyError(err); + const keyPattern = + typeof err === 'object' && err != null && 'keyPattern' in err + ? err.keyPattern + : undefined; + // A duplicate _id means an earlier attempt already landed. + if ( + isDuplicate && + typeof keyPattern === 'object' && + keyPattern != null && + '_id' in keyPattern + ) { + return; + } + // Any other duplicate (the email was taken meanwhile) can't succeed. + if (!isDuplicate && attempt < RESTORE_ATTEMPTS) { + await new Promise(r => setTimeout(r, RESTORE_RETRY_DELAY_MS * attempt)); + continue; + } + memberRestoreFailures.add(1); + opentelemetry.trace + .getActiveSpan() + ?.recordException(err instanceof Error ? err : new Error(String(err))); + logger.error( + { + error: err, + userId: user._id.toString(), + teamId: user.team.toString(), }, - }, - ), - User.findOneAndDelete({ - team: teamId, - _id: userIdToDelete, - }), - ]); - - return deletedUser; + 'Failed to restore a removed team member', + ); + throw err; + } + } } diff --git a/packages/api/src/middleware/auth.ts b/packages/api/src/middleware/auth.ts index 5be32c9798..f33ae6ceaf 100644 --- a/packages/api/src/middleware/auth.ts +++ b/packages/api/src/middleware/auth.ts @@ -4,10 +4,12 @@ import type { NextFunction, Request, Response } from 'express'; import { serializeError } from 'serialize-error'; import * as config from '@/config'; -import { findUserByAccessKey } from '@/controllers/user'; +import { touchOrgApiKeyLastUsed } from '@/controllers/orgApiKey'; +import { findUserByAccessKey, ORG_API_KEY_PREFIX } from '@/controllers/user'; import type { UserDocument } from '@/models/user'; import { getActiveTraceId, + getCounter, getStaticFeatureFlags, setBusinessContext, } from '@/utils/instrumentation'; @@ -95,25 +97,43 @@ export async function validateUserAccessKey( return res.sendStatus(401); } - const user = await findUserByAccessKey(key); - if (!user) { - return res.sendStatus(401); - } + // 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) { + if (key.startsWith(ORG_API_KEY_PREFIX)) { + orgApiKeyAuthFailures.add(1); + } + return res.sendStatus(401); + } - req.user = user; + req.user = user; + if (user.isVirtual) { + touchOrgApiKeyLastUsed(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, + 'hyperdx.user.is_virtual': user.isVirtual || undefined, + ...getStaticFeatureFlags(), + }); + } catch (err) { + return next(err); + } 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.', +}); + /** * 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/user.ts b/packages/api/src/models/user.ts index e5756cc418..04a78d878f 100644 --- a/packages/api/src/models/user.ts +++ b/packages/api/src/models/user.ts @@ -17,6 +17,14 @@ export interface IUser { name: string; // Optional for docs predating this field; the `me` route defaults it. onboardingData?: OnboardingData; + // A service account: no sign-in, not listed as a team member. + isVirtual: boolean; + // Set only on virtual users backing an org API key, whose accessKey holds the + // key's SHA-256. createdBy is what marks a virtual user as one. + expiresAt?: Date | null; + lastUsedAt?: Date | null; + revokedAt?: Date | null; + createdBy?: ObjectId; team: ObjectId; } @@ -36,6 +44,14 @@ const UserSchema = new Schema( return uuidv4(); }, }, + isVirtual: { + type: Boolean, + default: false, + }, + expiresAt: Date, + lastUsedAt: Date, + revokedAt: Date, + createdBy: { type: mongoose.Schema.Types.ObjectId, ref: 'User' }, onboardingData: { type: new Schema( { 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..a5915e1d5d 100644 --- a/packages/api/src/routers/api/__tests__/team.int.test.ts +++ b/packages/api/src/routers/api/__tests__/team.int.test.ts @@ -2,7 +2,7 @@ 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 Team from '@/models/team'; import TeamInvite from '@/models/teamInvite'; @@ -332,6 +332,15 @@ describe('team router', () => { expect(resp2.body.data).toHaveLength(1); }); + it('DELETE /team/member/:userId keeps the last person in the team', async () => { + const { agent, user } = await getLoggedInAgent(server); + + const res = await agent.delete(`/team/member/${user._id}`).expect(400); + + expect(res.body.message).toBe('Cannot remove the last team member'); + expect(await User.findById(user._id)).not.toBeNull(); + }); + it('DELETE /team/member/:userId updates alerts created by the removed user', async () => { const { agent, team, user } = await getLoggedInAgent(server); @@ -427,4 +436,138 @@ describe('team router', () => { expect(resp.body.newApiKey.length).toBeGreaterThan(0); }); + + describe('org API keys', () => { + const body = { + description: 'terraform', + 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', + lastUsedAt: null, + createdBy: { email: user.email }, + }); + }); + + it.each([ + { ...body, description: '' }, + { ...body, description: 'x'.repeat(101) }, + { ...body, expiry: '7d' }, + ])('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' }) + .expect(200); + expect(updated.body).toMatchObject({ + description: 'ci', + }); + }); + + 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 User.create({ + team: new mongoose.Types.ObjectId(), + email: 'sa@service-accounts.hyperdx.invalid', + name: 'other team', + isVirtual: true, + 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 User.findById(other._id))?.revokedAt).toBeUndefined(); + }); + + 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 User.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); + }); + + it('never lets a service account sign in', async () => { + const { agent, user } = await getLoggedInAgent(server); + const { body: created } = await agent + .post('/team/api-keys') + .send(body) + .expect(200); + // Give the service account the signed-in user's password hash, so a + // correct password is presented and only the service-account check stops it. + const human = await User.findById(user._id).select('+hash +salt'); + const account = await User.findByIdAndUpdate( + created.id, + { hash: human?.get('hash'), salt: human?.get('salt') }, + { new: true }, + ); + + const res = await getAgent(server) + .post('/login/password') + .send({ email: account?.email, password: 'TacoCat!2#4X' }); + expect(res.status).toBe(303); + expect(res.headers.location).toContain('/login?err='); + }); + }); }); diff --git a/packages/api/src/routers/api/orgApiKeys.ts b/packages/api/src/routers/api/orgApiKeys.ts new file mode 100644 index 0000000000..6279f34406 --- /dev/null +++ b/packages/api/src/routers/api/orgApiKeys.ts @@ -0,0 +1,113 @@ +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, + formatOrgApiKey, + listOrgApiKeys, + revokeOrgApiKey, + updateOrgApiKeyDescription, +} from '@/controllers/orgApiKey'; +import { getNonNullUserWithTeam } from '@/middleware/auth'; +import { getCounter } from '@/utils/instrumentation'; +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(); + +// Session-only, and service accounts can't hold a session; this keeps them out +// even if that ever changes. +router.use((req, res, next) => { + if (req.user?.team == null || req.user.isVirtual) { + return res.sendStatus(403); + } + next(); +}); + +router.get( + '/', + async (req, res: express.Response, next) => { + try { + const keys = await listOrgApiKeys(getNonNullUserWithTeam(req).teamId); + res.json({ data: keys.map(formatOrgApiKey) }); + } catch (e) { + next(e); + } + }, +); + +router.post( + '/', + processRequest({ body: CreateOrgApiKeyRequestSchema }), + async (req, res: express.Response, next) => { + try { + const { teamId, userId } = getNonNullUserWithTeam(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 }); + } 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( + getNonNullUserWithTeam(req).teamId, + 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 revokeOrgApiKey( + getNonNullUserWithTeam(req).teamId, + 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..a1e5620ff7 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'; @@ -300,7 +301,16 @@ router.delete( throw new Error(`Requesting user has no id`); } - await deleteTeamMember(teamId, userIdToDelete, userIdRequestingDelete); + const result = await deleteTeamMember( + teamId, + userIdToDelete, + userIdRequestingDelete, + ); + if (result === 'last_member') { + return res + .status(400) + .json({ message: 'Cannot remove the last team member' }); + } res.json({ message: 'User deleted' }); } catch (e) { @@ -332,4 +342,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 7ddf6796b2..a5c2b05946 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,34 @@ describe('External API Alerts', () => { }); }; + describe('org API keys', () => { + it('attributes the alert to the key service account', async () => { + const { doc, key } = await createOrgApiKey(team._id, user._id, { + description: 'terraform', + 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).toEqual(doc._id); + }); + }); + // 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..7e61365da3 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,10 @@ 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 User from '@/models/user'; import { IUser } from '@/models/user'; const TEAM_BASE_URL = '/api/v2/team'; @@ -121,4 +124,141 @@ describe('External API v2 Team', () => { ).toBe(1); }); }); + + describe('org API keys', () => { + const createKey = async () => + ( + await createOrgApiKey(team._id, user._id, { + description: 'terraform', + 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('lists members without the key or anyone marked as the caller', async () => { + const key = await createKey(); + const res = await withKey( + key, + agent.get(`${TEAM_BASE_URL}/members`), + ).expect(200); + expect(res.body.data.map((m: { email: string }) => m.email)).toEqual([ + user.email, + ]); + expect(res.body.data[0].isCurrentUser).toBe(false); + }); + + it("can't reach an invite link", async () => { + const key = await createKey(); + await withKey(key, agent.post(`${TEAM_BASE_URL}/invitation`)) + .send({ email: 'new@deploysentinel.com' }) + .expect(403); + + await authRequest('post', `${TEAM_BASE_URL}/invitation`) + .send({ email: 'new@deploysentinel.com' }) + .expect(200); + const list = await withKey( + key, + agent.get(`${TEAM_BASE_URL}/invitations`), + ).expect(200); + expect(list.body.data).toHaveLength(1); + expect(list.body.data[0]).not.toHaveProperty('url'); + }); + + it('can remove members', async () => { + const key = await createKey(); + 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(); + const account = await User.findOne({ isVirtual: true }); + expect(stored?.createdBy).toEqual(account?._id); + }); + + it('stops a key removing the last member', async () => { + const key = await createKey(); + 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'); + // 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 a key removes the last two at once', async () => { + const key = await createKey(); + 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, + isVirtual: { $ne: true }, + }), + ).toBeGreaterThan(0); + }); + + it('returns 404 for a non-member before applying the last-member rule', async () => { + const key = await createKey(); + await withKey( + key, + agent.delete(`${TEAM_BASE_URL}/member/${new Alert()._id}`), + ).expect(404); + }); + + it('hides service accounts from members and member removal', async () => { + await createKey(); + const account = await User.findOne({ isVirtual: true }); + + const list = await authRequest('get', `${TEAM_BASE_URL}/members`).expect( + 200, + ); + expect(list.body.data.map((m: { id: string }) => m.id)).not.toContain( + account?._id.toString(), + ); + + const alert = await Alert.create({ + team: team._id, + threshold: 1, + interval: '5m', + thresholdType: 'above', + source: 'saved_search', + channel: { type: 'webhook', webhookId: 'x' }, + createdBy: account?._id, + }); + + await authRequest( + 'delete', + `${TEAM_BASE_URL}/member/${account?._id}`, + ).expect(404); + expect(await User.findById(account?._id)).not.toBeNull(); + expect((await Alert.findById(alert._id))?.createdBy).toEqual( + account?._id, + ); + }); + }); }); 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..afcb0e8708 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,8 @@ +import { createOrgApiKey } from '@/controllers/orgApiKey'; +import * as userController from '@/controllers/user'; import { getLoggedInAgent, getServer } from '@/fixtures'; +import Dashboard from '@/models/dashboard'; +import User from '@/models/user'; describe('external api v2', () => { const server = getServer(); @@ -24,5 +28,117 @@ 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', + expiry: '30d', + }); + return { agent, team, user, doc, key }; + }; + + it('GET /api/v2 returns the key service account as the user', 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).toMatchObject({ + _id: doc._id.toString(), + team: team._id.toString(), + name: 'terraform', + isVirtual: true, + }); + expect(resp.body.user).not.toHaveProperty('accessKey'); + }); + + it('rejects the stored key hash as a bearer token', async () => { + const { agent, doc } = await setup(); + await agent + .get('/api/v2') + .set('Authorization', `Bearer ${doc.accessKey}`) + .expect(401); + }); + + 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 User.updateOne( + { _id: doc._id }, + { expiresAt: new Date(Date.now() - 1000) }, + ); + await agent + .get('/api/v2') + .set('Authorization', `Bearer ${key}`) + .expect(401); + + await User.updateOne({ _id: doc._id }, { revokedAt: new Date() }); + 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 User.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 User.updateOne({ _id: doc._id }, { revokedAt: new Date() }); + expect((await listTools()).status).toBe(401); + }); + + it('responds with an error when the key lookup fails', async () => { + const { agent, key } = await setup(); + jest + .spyOn(userController, 'findUserByAccessKey') + .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/index.ts b/packages/api/src/routers/external-api/v2/index.ts index 6ef19d6d34..98201576ca 100644 --- a/packages/api/src/routers/external-api/v2/index.ts +++ b/packages/api/src/routers/external-api/v2/index.ts @@ -1,4 +1,5 @@ import express from 'express'; +import omit from 'lodash/omit'; import { EXTERNAL_API_RATE_LIMIT_MAX } from '@/config'; import { validateUserAccessKey } from '@/middleware/auth'; @@ -24,9 +25,11 @@ const defaultRateLimiter = rateLimiter({ }); router.get('/', validateUserAccessKey, (req, res) => { + const user = req.user?.toJSON(); res.json({ version: 'v2', - user: req.user?.toJSON(), + // A service account's accessKey is its org key hash; don't hand it out. + user: user?.isVirtual ? omit(user, 'accessKey') : user, }); }); diff --git a/packages/api/src/routers/external-api/v2/team.ts b/packages/api/src/routers/external-api/v2/team.ts index 2b5dd7584a..291365132b 100644 --- a/packages/api/src/routers/external-api/v2/team.ts +++ b/packages/api/src/routers/external-api/v2/team.ts @@ -98,6 +98,8 @@ router.get('/members', async (req, res, next) => { * description: Successfully created the invitation * '400': * description: User already exists + * '403': + * description: Org API keys can't invite people */ router.post( '/invitation', @@ -113,6 +115,12 @@ router.post( if (!teamId) { return res.sendStatus(403); } + // An invite link lets a key holder join as a person and manage keys. + if (req.user?.isVirtual) { + return res + .status(403) + .json({ message: "Org API keys can't invite people" }); + } const { email, name } = req.body; const normalizedEmail = email.toLowerCase(); @@ -177,7 +185,7 @@ router.post( * tags: [Team] * responses: * '200': - * description: Successfully retrieved invitations + * description: Successfully retrieved invitations. Invite links are left out for org API keys. */ router.get('/invitations', async (req, res, next) => { try { @@ -197,7 +205,7 @@ router.get('/invitations', async (req, res, next) => { createdAt: ti.createdAt, email: ti.email, name: ti.name, - url: getTeamInviteUrl(ti.token), + ...(req.user?.isVirtual ? {} : { url: getTeamInviteUrl(ti.token) }), })), }); } catch (e) { @@ -269,7 +277,7 @@ 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 * '404': * description: Member not found */ @@ -290,16 +298,19 @@ router.delete( }); } - const deletedUser = await deleteTeamMember( + const result = await deleteTeamMember( teamId.toString(), req.params.id, requestingUserId, ); - - if (!deletedUser) { + 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 } }); } catch (e) { next(e); diff --git a/packages/api/src/tasks/usageStats.ts b/packages/api/src/tasks/usageStats.ts index 451f0372f3..4ae691dd9b 100644 --- a/packages/api/src/tasks/usageStats.ts +++ b/packages/api/src/tasks/usageStats.ts @@ -23,6 +23,7 @@ import os from 'os'; import { ClickhouseClient } from '@/clickhouse'; import * as config from '@/config'; +import { NOT_VIRTUAL } from '@/controllers/user'; import Connection from '@/models/connection'; import { Source, SourceDocument } from '@/models/source'; import Team from '@/models/team'; @@ -227,7 +228,7 @@ async function getUsageStats() { try { const nowInMs = Date.now(); const [userCounts, team, chTables] = await Promise.all([ - User.countDocuments(), + User.countDocuments(NOT_VIRTUAL), Team.find( {}, { diff --git a/packages/api/src/utils/passport.ts b/packages/api/src/utils/passport.ts index 99b89243e3..3afb9a0990 100644 --- a/packages/api/src/utils/passport.ts +++ b/packages/api/src/utils/passport.ts @@ -14,7 +14,8 @@ passport.serializeUser(function (user, done) { passport.deserializeUser(function (id: string, done) { findUserById(id) .then(user => { - if (user == null) { + // A session can never belong to a service account. + if (user == null || user.isVirtual) { // The session outlived the user, e.g. the account was deleted. That is // not a server fault, and reporting it as one turned every request // still carrying the cookie into a 500, public routes included, so the @@ -41,6 +42,14 @@ passport.use( username, password, ); + if (user?.isVirtual) { + logger.info({ + message: `Login for "${username}" rejected, service account`, + type: 'user_login', + authType: 'password', + }); + return done(null, false); + } if (error) { logger.info({ message: `Login for "${username}" failed, ${error}"`, diff --git a/packages/app/src/TeamPage.tsx b/packages/app/src/TeamPage.tsx index 260487e96e..4a465afa21 100644 --- a/packages/app/src/TeamPage.tsx +++ b/packages/app/src/TeamPage.tsx @@ -30,13 +30,14 @@ 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'; import TeamQueryConfigSection from './components/TeamSettings/TeamQueryConfigSection'; import api from './api'; import { usePageTitle } from './brand'; -import { IS_IAC_EXPORT_ENABLED } from './config'; +import { IS_IAC_EXPORT_ENABLED, IS_LOCAL_MODE } from './config'; import { APP_CONTENT_SCROLL_CONTAINER_ID, withAppNav } from './layout'; type TeamTab = { @@ -166,6 +167,15 @@ export default function TeamPage() { id: 'team-api-agents-api-keys', content: () => , }, + // Org keys are stored by the API, which local mode doesn't run. + ...(IS_LOCAL_MODE + ? [] + : [ + { + 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 5271dcfdb2..de5bfe5d7a 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..a5795172ed --- /dev/null +++ b/packages/app/src/components/TeamSettings/CreateOrgApiKeyModal.tsx @@ -0,0 +1,135 @@ +import { useState } from 'react'; +import { + type OrgApiKeyExpiry, + OrgApiKeyExpirySchema, +} 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 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 [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(''); + setExpiry('30d'); + setCreatedKey(null); + onClose(); + }; + + const onCreate = () => { + createKey.mutate( + { description: description.trim(), 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 + /> + + + 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..1cfa80b435 --- /dev/null +++ b/packages/app/src/components/TeamSettings/OrgApiKeysSection.tsx @@ -0,0 +1,164 @@ +import { useEffect, useState } from 'react'; +import { Badge, Separator } from '@clickhouse/click-ui'; +import type { OrgApiKeyApiResponse } from '@hyperdx/common-utils/dist/types'; +import { Box, Button, Card, 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 { currentDate } from '@/timeZone'; +import { useConfirm } from '@/useConfirm'; +import { useFormatTime } from '@/useFormatTime'; + +import CreateOrgApiKeyModal from './CreateOrgApiKeyModal'; + +// A minute tick is close enough for flipping the Expired badge. +const EXPIRY_TICK_MS = 60_000; + +// Only read in a state initialiser and the interval, never during render. +const currentTime = () => currentDate().getTime(); + +function useNow() { + const [now, setNow] = useState(currentTime); + useEffect(() => { + const id = window.setInterval(() => setNow(currentTime()), EXPIRY_TICK_MS); + return () => window.clearInterval(id); + }, []); + return now; +} + +function OrgApiKeyRow({ + apiKey, + now, +}: { + apiKey: OrgApiKeyApiResponse; + now: number; +}) { + const updateKey = api.useUpdateOrgApiKey(); + const deleteKey = api.useDeleteOrgApiKey(); + const confirm = useConfirm(); + const formatTime = useFormatTime(); + const formatDate = (iso: string) => formatTime(iso, { format: 'withYear' }); + 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 }).catch(e => { + notifications.show({ + color: 'red', + message: `Couldn't rename API key: ${e.message}`, + }); + throw e; + }) + } + /> + + + {apiKey.createdBy + ? apiKey.createdBy.name || apiKey.createdBy.email + : 'Deleted user'} + + {formatDate(apiKey.createdAt)} + + {apiKey.lastUsedAt ? formatDate(apiKey.lastUsedAt) : 'Never'} + + + {isExpired ? ( + + ) : apiKey.expiresAt ? ( + formatDate(apiKey.expiresAt) + ) : ( + 'Never' + )} + + + + + + ); +} + +export default function OrgApiKeysSection() { + const { data, isLoading, isError } = api.useOrgApiKeys(); + const now = useNow(); + 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 + 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..5e9f52284f --- /dev/null +++ b/packages/app/src/components/TeamSettings/__tests__/OrgApiKeysSection.test.tsx @@ -0,0 +1,178 @@ +import { act, 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', + 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 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', 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(), + ), + ); + }); + + it('marks a key expired once its expiry passes while the page is open', () => { + jest.useFakeTimers({ now: new Date('2026-10-01T00:00:00.000Z') }); + try { + setKeys([key({ expiresAt: '2026-10-01T00:00:30.000Z' })]); + renderWithMantine(); + expect(screen.queryByText('Expired')).not.toBeInTheDocument(); + act(() => { + jest.advanceTimersByTime(60_000); + }); + expect(screen.getByText('Expired')).toBeInTheDocument(); + } finally { + jest.useRealTimers(); + } + }); + + it('limits descriptions to 100 characters and reports failed edits', async () => { + setKeys([key()]); + mockUseUpdate.mockReturnValue({ + mutateAsync: jest.fn().mockRejectedValue(new Error('Save failed')), + }); + renderWithMantine(); + + const input = screen.getByLabelText('API key description'); + expect(input).toHaveAttribute('maxLength', '100'); + await userEvent.clear(input); + await userEvent.type(input, 'ci{Enter}'); + expect( + await screen.findByText("Couldn't rename API key: Save failed"), + ).toBeInTheDocument(); + }); + + it('keeps the create dialog open while the key is being created', async () => { + mockUseCreate.mockReturnValue({ mutate: createMutate, isPending: true }); + renderWithMantine(); + + await userEvent.click( + screen.getByRole('button', { name: 'Create API key' }), + ); + const cancel = await screen.findByRole('button', { name: 'Cancel' }); + expect(cancel).toBeDisabled(); + await userEvent.keyboard('{Escape}'); + expect(screen.getByLabelText('Description')).toBeInTheDocument(); + }); +}); diff --git a/packages/app/src/components/TimePicker/utils.ts b/packages/app/src/components/TimePicker/utils.ts index b6f207b2c0..ef9f4dbd9f 100644 --- a/packages/app/src/components/TimePicker/utils.ts +++ b/packages/app/src/components/TimePicker/utils.ts @@ -6,12 +6,7 @@ import { toWallClock, } from '@hyperdx/common-utils/dist/core/utils'; -import { getLocalTimeZone } from '@/timeZone'; - -function currentDate(): Date { - // eslint-disable-next-line no-restricted-syntax - return new Date(); -} +import { currentDate, getLocalTimeZone } from '@/timeZone'; function resolveTimeZone(timeZone?: string): string { return timeZone ?? getLocalTimeZone(); diff --git a/packages/app/src/timeZone.ts b/packages/app/src/timeZone.ts index af44cd002a..6a559e1411 100644 --- a/packages/app/src/timeZone.ts +++ b/packages/app/src/timeZone.ts @@ -3,6 +3,13 @@ import { UTC_TIME_ZONE } from '@hyperdx/common-utils/dist/core/utils'; /** Time zone setting value that follows the browser's zone. */ export const LOCAL_TIME_ZONE = 'local'; +// For code that needs the time right now, such as a ticking clock or parsing +// "yesterday", rather than the stable NOW from config. +export function currentDate(): Date { + // eslint-disable-next-line no-restricted-syntax + return new Date(); +} + export function isValidTimeZone(timeZone: string): boolean { try { new Intl.DateTimeFormat('en-US', { timeZone }); diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index e3dd7f52a5..16519d95f7 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -3399,6 +3399,56 @@ 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 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, + 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(), + 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