diff --git a/.changeset/rotatable-personal-access-key.md b/.changeset/rotatable-personal-access-key.md new file mode 100644 index 0000000000..b02d20c71a --- /dev/null +++ b/.changeset/rotatable-personal-access-key.md @@ -0,0 +1,7 @@ +--- +'@hyperdx/api': patch +'@hyperdx/app': patch +'@hyperdx/common-utils': patch +--- + +Add a Rotate action for the personal API access key in Team Settings → API & Agents. Previously the personal access key — the bearer token for the external API v2 and the MCP server — was generated once at account creation and could never be changed, so a leaked key could only be remediated by deleting the user. Rotating immediately revokes the previous key, so MCP / AI agent configs, external API v2 clients, Terraform / IaC providers, and CI scripts using the old key must be updated with the new one. Browser sessions are unaffected. diff --git a/AGENTS.md b/AGENTS.md index 24c1c24400..74c995b493 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -61,8 +61,12 @@ directory: - `agent_docs/architecture.md` - Detailed architecture patterns and data models - `agent_docs/tech_stack.md` - Technology stack details and component patterns - `agent_docs/development.md` - Development workflows, testing, and common tasks -- `agent_docs/code_style.md` - Code patterns and best practices (read only when - actively coding) +- `agent_docs/code_style.md` - Code patterns and best practices. **Read this + before writing or planning any `packages/app` UI change**, not just while + typing code. It carries required patterns that are invisible from the + surrounding file (sentence-case UI text, mandated Button/ActionIcon variants, + `useConfirm` for confirmation dialogs, `EmptyState`), so copying the + conventions of the component you are editing is not sufficient. - `agent_docs/observability.md` - Instrumentation standards (tracing, metrics, context) and the shared helpers (read when adding or changing a feature) @@ -87,7 +91,9 @@ before stopping. similar files before implementing 4. **Component size**: Keep files under 300 lines; break down large components 5. **UI Components**: Use custom Button/ActionIcon variants (`primary`, - `secondary`, `danger`) - see `agent_docs/code_style.md` for required patterns + `secondary`, `danger`), `useConfirm` for "are you sure?" dialogs rather than + a hand-rolled `Modal`, and sentence case for all user-facing text - see + `agent_docs/code_style.md` for required patterns 6. **Testing**: Tests live in `__tests__/` directories; use Jest for unit/integration tests 7. **Observability**: This is an observability product - instrument new code as diff --git a/MCP.md b/MCP.md index ffedf22873..132a7a9d79 100644 --- a/MCP.md +++ b/MCP.md @@ -9,8 +9,8 @@ data sources directly. - A running HyperDX instance (see [CONTRIBUTING.md](/CONTRIBUTING.md) for local development setup, or [DEPLOY.md](/DEPLOY.md) for self-hosted deployment) -- A **Personal API Access Key** — find yours in the HyperDX UI under **Team - Settings > API Keys > Personal API Access Key** +- A **Personal API access key** — find yours in the HyperDX UI under **Team + Settings > API keys > Personal API access key** > **Note:** HyperDX v1 ([hyperdx.io](https://hyperdx.io)) does not yet support > the MCP server. The documentation below applies to self-hosted HyperDX v2 diff --git a/agent_docs/code_style.md b/agent_docs/code_style.md index d40dbb2c0b..9cbcca85d0 100644 --- a/agent_docs/code_style.md +++ b/agent_docs/code_style.md @@ -131,6 +131,56 @@ The variant → token mapping is centralized in `packages/app/src/theme/themes/s **Note**: Existing `` call sites are untouched; the semantic variants are opt-in. Prefer the variant for any **new** callout, and migrate nearby `color="..."` alerts when you touch them. +### Confirmation dialogs: use `useConfirm` (REQUIRED) + +**Use `useConfirm` (`@/useConfirm`) for any "are you sure?" step. Do not +hand-roll a `` with Cancel/Confirm buttons.** The provider is already +mounted app-wide in `pages/_app.tsx`, so there is no setup at the call site. + +```tsx +const confirm = useConfirm(); + +const handleDelete = async () => { + if ( + await confirm( + <> + Deleting {name} is not reversible. + , + 'Delete', + { variant: 'danger' }, + ) + ) { + await deleteThing.mutateAsync({ id }); + } +}; +``` + +- The message is a `ReactNode`, so it can carry emphasis and multiple sentences. +- Pass `{ variant: 'danger' }` for destructive actions; the confirm label + defaults to `Confirm`. +- It resolves **exactly once**, so a double click on Confirm during the modal's + exit transition cannot fire the action twice. A hand-rolled modal has to guard + that itself. +- Test ids are shared and already exist: `confirm-modal`, + `confirm-confirm-button`, `confirm-cancel-button`. **Do not invent per-flow + confirm/cancel test ids** — E2E page objects key off the shared ones. + +**Known limits.** It passes no `title` to the Modal and renders the body at +`size="sm" opacity={0.7}`, and CSS opacity applies to the whole subtree so a +nested `` cannot opt back out. If a flow genuinely needs a heading or +full-contrast body, **extend `useConfirm`** (an optional prop, applied to all +call sites) rather than forking a one-off modal. + +**In component tests**, mock it — `ConfirmProvider` pulls in `next/router`, +which is not available in jsdom: + +```tsx +jest.mock('@/useConfirm', () => ({ useConfirm: jest.fn() })); +``` + +Assert on the arguments (and render the message `ReactNode` if you need to check +the copy). Exercise the real dialog in E2E instead. + ### EmptyState Component (REQUIRED) **Use `EmptyState` (`@/components/EmptyState`) for all empty/no-data states.** Do not create ad-hoc inline empty states. diff --git a/packages/api/src/controllers/user.ts b/packages/api/src/controllers/user.ts index 55258c2db6..cde81ea9d8 100644 --- a/packages/api/src/controllers/user.ts +++ b/packages/api/src/controllers/user.ts @@ -1,4 +1,5 @@ import mongoose from 'mongoose'; +import { v4 as uuidv4 } from 'uuid'; import type { ObjectId } from '@/models'; import Alert from '@/models/alert'; @@ -7,6 +8,17 @@ export function findUserByAccessKey(accessKey: string) { return User.findOne({ accessKey }); } +/** + * Rotates a user's personal access key, immediately revoking the previous one. + * + * There is exactly one key per user and no grace period: findUserByAccessKey + * above is hit uncached on every bearer request (see validateUserAccessKey), so + * requests presenting the old key start 401ing the instant this returns. + */ +export function rotateUserAccessKey(userId: string | ObjectId) { + return User.findByIdAndUpdate(userId, { accessKey: uuidv4() }, { new: true }); +} + export function findUserById(id: string) { return User.findById(id); } diff --git a/packages/api/src/routers/api/__tests__/me.int.test.ts b/packages/api/src/routers/api/__tests__/me.int.test.ts new file mode 100644 index 0000000000..b2ad3fa8a1 --- /dev/null +++ b/packages/api/src/routers/api/__tests__/me.int.test.ts @@ -0,0 +1,105 @@ +import { getAgent, getLoggedInAgent, getServer } from '@/fixtures'; +import User from '@/models/user'; + +describe('me router', () => { + const server = getServer(); + + beforeAll(async () => { + await server.start(); + }); + + afterEach(async () => { + await server.clearDBs(); + }); + + afterAll(async () => { + await server.stop(); + }); + + describe('GET /me', () => { + it('returns the calling user', async () => { + const { agent, team, user } = await getLoggedInAgent(server); + + const resp = await agent.get('/me').expect(200); + + expect(resp.body.id).toEqual(user._id.toString()); + expect(resp.body.email).toEqual('fake@deploysentinel.com'); + expect(resp.body.accessKey).toEqual(user.accessKey); + expect(resp.body.team.id).toEqual(team._id.toString()); + }); + + it('rejects an unauthenticated request', async () => { + await getAgent(server).get('/me').expect(401); + }); + }); + + describe('PATCH /me/accessKey', () => { + it('rejects an unauthenticated request', async () => { + // The new verb is covered by the mount-time isUserAuthenticated in + // api-app.ts, not by anything in the handler itself. + await getAgent(server).patch('/me/accessKey').expect(401); + }); + + it('returns a new key and persists it', async () => { + const { agent, user } = await getLoggedInAgent(server); + + const resp = await agent.patch('/me/accessKey').expect(200); + + expect(resp.body.newAccessKey).toEqual(expect.any(String)); + expect(resp.body.newAccessKey).not.toEqual(user.accessKey); + expect((await User.findById(user._id))?.accessKey).toEqual( + resp.body.newAccessKey, + ); + }); + + it('revokes the old key and accepts the new one', async () => { + const { agent, user } = await getLoggedInAgent(server); + const oldAccessKey = user.accessKey; + + // GET /api/v2 is the bearer-authed surface with no rate limiter attached, + // so three sequential calls here are safe. + await agent + .get('/api/v2') + .set('Authorization', `Bearer ${oldAccessKey}`) + .expect(200); + + const { body } = await agent.patch('/me/accessKey').expect(200); + + await agent + .get('/api/v2') + .set('Authorization', `Bearer ${oldAccessKey}`) + .expect(401); + await agent + .get('/api/v2') + .set('Authorization', `Bearer ${body.newAccessKey}`) + .expect(200); + }); + + it('does not sign the user out of their browser session', async () => { + // Session auth never reads accessKey (see isUserAuthenticated), so + // rotating must leave the cookie session intact. + const { agent } = await getLoggedInAgent(server); + + const { body } = await agent.patch('/me/accessKey').expect(200); + + const resp = await agent.get('/me').expect(200); + expect(resp.body.accessKey).toEqual(body.newAccessKey); + }); + + it("does not touch another user's key", async () => { + const { agent, user } = await getLoggedInAgent(server); + // Created directly rather than via /register/password, which is gated to + // the first user. We only read the schema-defaulted accessKey off it. + const other = await User.create({ + email: 'other@deploysentinel.com', + team: user.team, + }); + + await agent.patch('/me/accessKey').expect(200); + + expect((await User.findById(other._id))?.accessKey).toEqual( + other.accessKey, + ); + }); + }); +}); diff --git a/packages/api/src/routers/api/me.ts b/packages/api/src/routers/api/me.ts index b574928528..3586a6f6ac 100644 --- a/packages/api/src/routers/api/me.ts +++ b/packages/api/src/routers/api/me.ts @@ -1,8 +1,12 @@ -import type { MeApiResponse } from '@hyperdx/common-utils/dist/types'; +import type { + MeApiResponse, + RotateAccessKeyApiResponse, +} from '@hyperdx/common-utils/dist/types'; import express from 'express'; import { AI_API_KEY, ANTHROPIC_API_KEY, USAGE_STATS_ENABLED } from '@/config'; import { getTeam } from '@/controllers/team'; +import { rotateUserAccessKey } from '@/controllers/user'; import { Api404Error } from '@/utils/errors'; import { sendJson } from '@/utils/serialization'; @@ -43,4 +47,31 @@ router.get('/', async (req, res: express.Response, next) => { } }); +type RotateAccessKeyExpRes = express.Response; + +// Rotating your own personal access key. The user id comes from the session +// (isUserAuthenticated, applied at mount time in api-app.ts) and never from the +// request, so this route can only ever rotate the caller's own key. +// +// Deliberately NOT mirrored onto the bearer-authed external API v2: `GET /api/v2` +// echoes the caller's accessKey back, so a leaked key that could also rotate +// would let an attacker lock the legitimate owner out of their own tooling. +router.patch('/accessKey', async (req, res: RotateAccessKeyExpRes, next) => { + try { + const userId = req.user?._id; + if (userId == null) { + throw new Api404Error('Request without user found'); + } + + const user = await rotateUserAccessKey(userId); + if (user?.accessKey == null) { + throw new Error(`Failed to rotate access key for user ${userId}`); + } + + return sendJson(res, { newAccessKey: user.accessKey }); + } catch (e) { + next(e); + } +}); + export default router; diff --git a/packages/app/src/api.ts b/packages/app/src/api.ts index a8dea6a600..0260eaf281 100644 --- a/packages/app/src/api.ts +++ b/packages/app/src/api.ts @@ -11,6 +11,7 @@ import type { MeApiResponse, PresetDashboard, PresetDashboardFilter, + RotateAccessKeyApiResponse, RotateApiKeyApiResponse, TeamApiResponse, TeamClickHouseSettingsUpdate, @@ -23,7 +24,12 @@ import type { WebhookTestApiResponse, WebhookUpdateApiResponse, } from '@hyperdx/common-utils/dist/types'; -import { useInfiniteQuery, useMutation, useQuery } from '@tanstack/react-query'; +import { + useInfiniteQuery, + useMutation, + useQuery, + useQueryClient, +} from '@tanstack/react-query'; import { IS_LOCAL_MODE } from './config'; import { getLocalDashboardTags } from './dashboard'; @@ -269,6 +275,24 @@ const api = { }).json(), }); }, + useRotatePersonalAccessKey() { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: async () => + hdxServer(`me/accessKey`, { + method: 'PATCH', + }).json(), + // Seed the cache from the response rather than refetching `me`. The old + // key is already revoked by the time this runs, so a refetch that fails + // would leave every `useMe` consumer rendering a dead credential with no + // way to reach the new one short of a reload. + onSuccess: data => { + queryClient.setQueryData(['me'], prev => + prev == null ? prev : { ...prev, accessKey: data.newAccessKey }, + ); + }, + }); + }, useDeleteTeamMember() { return useMutation< { message: string }, diff --git a/packages/app/src/components/TeamSettings/ApiKeysSection.tsx b/packages/app/src/components/TeamSettings/ApiKeysSection.tsx index 248ec5db90..edbc77f7aa 100644 --- a/packages/app/src/components/TeamSettings/ApiKeysSection.tsx +++ b/packages/app/src/components/TeamSettings/ApiKeysSection.tsx @@ -1,10 +1,11 @@ import { useState } from 'react'; import { CopyToClipboard } from 'react-copy-to-clipboard'; -import { Box, Button, Card, Divider, Group, Modal, Text } from '@mantine/core'; +import { Box, Button, Card, Divider, Group, Text } from '@mantine/core'; import { notifications } from '@mantine/notifications'; import { IconCheck, IconClipboard } from '@tabler/icons-react'; import api from '@/api'; +import { useConfirm } from '@/useConfirm'; function APIKeyCopyButton({ value, @@ -28,7 +29,7 @@ function APIKeyCopyButton({ } > -
+
{value}
@@ -40,13 +41,25 @@ export default function ApiKeysSection() { const { data: team, refetch: refetchTeam } = api.useTeam(); const { data: me, isLoading: isLoadingMe } = api.useMe(); const rotateTeamApiKey = api.useRotateTeamApiKey(); + const rotatePersonalAccessKey = api.useRotatePersonalAccessKey(); + const confirm = useConfirm(); const hasAdminAccess = true; - const [ - rotateApiKeyConfirmationModalShow, - setRotateApiKeyConfirmationModalShow, - ] = useState(false); - const rotateTeamApiKeyAction = () => { + // `confirm` resolves exactly once, so a double click on its Confirm button + // during the modal's exit transition cannot fire a second rotation. + const onRotateTeamApiKey = async () => { + const confirmed = await confirm( + <> + Rotating the API key will invalidate your existing API key and generate + a new one for you. This action is not reversible. + , + 'Rotate key', + { variant: 'danger' }, + ); + if (!confirmed) { + return; + } + rotateTeamApiKey.mutate(undefined, { onSuccess: () => { notifications.show({ @@ -65,76 +78,82 @@ export default function ApiKeysSection() { }); }; - const onConfirmUpdateTeamApiKey = () => { - rotateTeamApiKeyAction(); - setRotateApiKeyConfirmationModalShow(false); + const onRotateAccessKey = async () => { + const confirmed = await confirm( + <> + Rotating your personal access key immediately revokes the current one + and generates a new one. This action is not reversible. Anything + still using the old key will start failing with 401 until you update it, + including MCP / AI agent configs (Claude Code, Cursor, VS Code, Codex), + external API v2 clients, Terraform / IaC providers, and CI scripts. Your + browser session is not affected; you will stay signed in. + , + 'Rotate key', + { variant: 'danger' }, + ); + if (!confirmed) { + return; + } + + rotatePersonalAccessKey.mutate(undefined, { + onSuccess: () => { + notifications.show({ + color: 'green', + message: + 'Revoked your old personal access key and generated a new one.', + }); + }, + onError: e => { + notifications.show({ + color: 'red', + message: e.message, + autoClose: 5000, + }); + }, + }); }; return ( - API Keys + API keys - Ingestion API Key + Ingestion API key {team?.apiKey && ( - + )} {hasAdminAccess && ( )} - setRotateApiKeyConfirmationModalShow(false)} - opened={rotateApiKeyConfirmationModalShow} - size="lg" - title={ - - Rotate API Key - - } - > - - - Rotating the API key will invalidate your existing API key and - generate a new one for you. This action is not reversible. - - - + + {!isLoadingMe && me != null && ( + + + Personal API access key + + - - - - {!isLoadingMe && me != null && ( - - - Personal API Access Key - )} diff --git a/packages/app/src/components/TeamSettings/__tests__/ApiKeysSection.test.tsx b/packages/app/src/components/TeamSettings/__tests__/ApiKeysSection.test.tsx new file mode 100644 index 0000000000..ff9c6226bd --- /dev/null +++ b/packages/app/src/components/TeamSettings/__tests__/ApiKeysSection.test.tsx @@ -0,0 +1,202 @@ +import type { ReactNode } from 'react'; +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +import api from '@/api'; +import ApiKeysSection from '@/components/TeamSettings/ApiKeysSection'; +import { useConfirm } from '@/useConfirm'; + +jest.mock('@/api', () => ({ + __esModule: true, + default: { + useMe: jest.fn(), + useTeam: jest.fn(), + useRotateTeamApiKey: jest.fn(), + useRotatePersonalAccessKey: jest.fn(), + }, + hdxServer: jest.fn(), +})); + +// The real ConfirmProvider lives in pages/_app.tsx and pulls in next/router, +// so the shared dialog is mocked here and exercised in the E2E specs instead. +jest.mock('@/useConfirm', () => ({ useConfirm: jest.fn() })); + +// Annotated as the loose `jest.Mock` rather than `jest.mocked(...)`'s exact +// MockedFunction: these hooks return TanStack `UseQueryResult` / +// `UseMutationResult` objects with ~25 members, and the tests only need the +// few the component reads. The loose type keeps `mockReturnValue` at `any` +// so the partial fixtures below need no type assertions. +const mockUseMe: jest.Mock = jest.mocked(api.useMe); +const mockUseTeam: jest.Mock = jest.mocked(api.useTeam); +const mockUseRotateTeamApiKey: jest.Mock = jest.mocked(api.useRotateTeamApiKey); +const mockUseRotatePersonalAccessKey: jest.Mock = jest.mocked( + api.useRotatePersonalAccessKey, +); +const mockUseConfirm: jest.Mock = jest.mocked(useConfirm); + +/** Options object the component hands to `mutate`, captured so the tests can + * drive `onSuccess` / `onError` without a real mutation. */ +type MutateOptions = { + onSuccess?: () => void; + onError?: (e: Error) => void; +}; + +let capturedPersonalOptions: MutateOptions | undefined; +const rotatePersonalMutate = jest.fn( + (_vars: undefined, options?: MutateOptions) => { + capturedPersonalOptions = options; + }, +); +const rotateTeamMutate = jest.fn(); + +/** Resolution of the next `confirm(...)` call, i.e. did the user accept. */ +let confirmAccepts = true; +// Params are declared so `mock.calls[n][i]` stays typed; the body ignores them. +const confirmSpy = jest.fn( + ( + _message: ReactNode, + _confirmLabel?: string, + _options?: { variant?: 'primary' | 'danger' }, + ) => Promise.resolve(confirmAccepts), +); + +function setMe(accessKey: string | null, isLoading = false) { + mockUseMe.mockReturnValue({ + data: + accessKey === null + ? null + : { + id: 'u1', + email: 'a@b.com', + accessKey, + name: 'User', + createdAt: '', + }, + isLoading, + }); +} + +/** Renders the ReactNode the component passed to `confirm` so the dialog copy + * stays asserted even though the dialog itself is mocked out. */ +function renderConfirmMessage(callIndex = 0) { + const message = confirmSpy.mock.calls[callIndex][0]; + return render(<>{message}); +} + +beforeEach(() => { + jest.clearAllMocks(); + capturedPersonalOptions = undefined; + confirmAccepts = true; + + setMe('personal_key_abc'); + mockUseConfirm.mockReturnValue(confirmSpy); + mockUseTeam.mockReturnValue({ + data: { apiKey: 'ingestion_key_xyz' }, + refetch: jest.fn(), + }); + mockUseRotateTeamApiKey.mockReturnValue({ mutate: rotateTeamMutate }); + mockUseRotatePersonalAccessKey.mockReturnValue({ + mutate: rotatePersonalMutate, + }); +}); + +describe('ApiKeysSection', () => { + // The two keys previously shared dataTestId="api-key" on an attribute + // (`data-test-id`) that no test runner queries by default. Both getByTestId + // calls below throw on duplicates, so this locks in the split. + it('renders the ingestion and personal keys under distinct test ids', () => { + renderWithMantine(); + + expect(screen.getByTestId('ingestion-api-key')).toHaveTextContent( + 'ingestion_key_xyz', + ); + expect(screen.getByTestId('personal-access-key')).toHaveTextContent( + 'personal_key_abc', + ); + }); + + it('asks for a danger confirmation naming what the old key breaks', async () => { + const user = userEvent.setup(); + renderWithMantine(); + + await user.click(screen.getByTestId('rotate-access-key-button')); + + expect(confirmSpy).toHaveBeenCalledTimes(1); + expect(confirmSpy.mock.calls[0][1]).toBe('Rotate key'); + expect(confirmSpy.mock.calls[0][2]).toEqual({ variant: 'danger' }); + + const { container } = renderConfirmMessage(); + expect(container).toHaveTextContent(/not reversible/); + expect(container).toHaveTextContent(/MCP \/ AI agent configs/); + expect(container).toHaveTextContent(/stay signed in/); + }); + + it('does not rotate when the confirmation is declined', async () => { + confirmAccepts = false; + const user = userEvent.setup(); + renderWithMantine(); + + await user.click(screen.getByTestId('rotate-access-key-button')); + + await waitFor(() => expect(confirmSpy).toHaveBeenCalled()); + expect(rotatePersonalMutate).not.toHaveBeenCalled(); + }); + + it('rotates once when the confirmation is accepted', async () => { + const user = userEvent.setup(); + renderWithMantine(); + + await user.click(screen.getByTestId('rotate-access-key-button')); + + await waitFor(() => expect(rotatePersonalMutate).toHaveBeenCalledTimes(1)); + }); + + it('notifies on a successful rotation', async () => { + const user = userEvent.setup(); + renderWithMantine(); + + await user.click(screen.getByTestId('rotate-access-key-button')); + await waitFor(() => expect(rotatePersonalMutate).toHaveBeenCalled()); + capturedPersonalOptions?.onSuccess?.(); + + expect( + await screen.findByText(/Revoked your old personal access key/), + ).toBeInTheDocument(); + }); + + it('surfaces the error message when rotation fails', async () => { + const user = userEvent.setup(); + renderWithMantine(); + + await user.click(screen.getByTestId('rotate-access-key-button')); + await waitFor(() => expect(rotatePersonalMutate).toHaveBeenCalled()); + capturedPersonalOptions?.onError?.(new Error('rotate blew up')); + + expect(await screen.findByText('rotate blew up')).toBeInTheDocument(); + }); + + it('confirms separately before rotating the ingestion key', async () => { + const user = userEvent.setup(); + renderWithMantine(); + + await user.click(screen.getByTestId('rotate-api-key-button')); + + await waitFor(() => expect(rotateTeamMutate).toHaveBeenCalledTimes(1)); + expect(rotatePersonalMutate).not.toHaveBeenCalled(); + const { container } = renderConfirmMessage(); + expect(container).toHaveTextContent(/invalidate your existing API key/); + }); + + it('renders neither the personal key nor its rotate button when me is null', () => { + setMe(null); + + renderWithMantine(); + + expect(screen.queryByTestId('personal-access-key')).not.toBeInTheDocument(); + expect( + screen.queryByTestId('rotate-access-key-button'), + ).not.toBeInTheDocument(); + // The ingestion key is unaffected by the me payload. + expect(screen.getByTestId('ingestion-api-key')).toBeInTheDocument(); + }); +}); diff --git a/packages/app/tests/e2e/features/team.spec.ts b/packages/app/tests/e2e/features/team.spec.ts index e7dc837e52..1afece9e29 100644 --- a/packages/app/tests/e2e/features/team.spec.ts +++ b/packages/app/tests/e2e/features/team.spec.ts @@ -72,7 +72,7 @@ test.describe('Team Settings Page', { tag: ['@team', '@full-stack'] }, () => { await test.step('Verify API & Agents tab headings exist', async () => { await expect( - teamPage.apiKeys.getByText('API Keys', { exact: true }), + teamPage.apiKeys.getByText('API keys', { exact: true }), ).toBeVisible(); await expect( teamPage.mcpServer.getByText('Connect your AI Agents', { exact: true }), @@ -160,31 +160,64 @@ test.describe('Team Settings Page', { tag: ['@team', '@full-stack'] }, () => { await test.step('Verify API key labels are visible', async () => { await expect( - teamPage.apiKeys.getByText('Ingestion API Key'), + teamPage.apiKeys.getByText('Ingestion API key'), ).toBeVisible(); await expect( - teamPage.apiKeys.getByText('Personal API Access Key'), + teamPage.apiKeys.getByText('Personal API access key'), ).toBeVisible(); }); - await test.step('Verify rotate button is visible', async () => { + await test.step('Verify rotate buttons are visible', async () => { await expect(teamPage.rotateButton).toBeVisible(); + await expect(teamPage.rotateAccessKeyTrigger).toBeVisible(); }); }); + // Both rotate flows are only opened and cancelled here, never confirmed. + // Every spec shares one account via a single storageState, and + // dashboard-external-api-*.spec.ts read the personal access key then fire + // bearer requests — a confirmed rotation in a parallel worker would 401 them. + // The confirm path is covered by packages/api me.int.test.ts. test('should open and cancel rotate API key modal', async () => { await test.step('Open rotate API key modal', async () => { await teamPage.openApiAndAgentsTab(); await teamPage.clickRotateApiKey(); }); - await test.step('Verify modal shows irreversible warning', async () => { - await expect(teamPage.page.getByText('not reversible')).toBeVisible(); + await test.step('Verify dialog shows irreversible warning', async () => { + await expect( + teamPage.confirmDialogBox.getByText(/invalidate your existing API key/), + ).toBeVisible(); + }); + + await test.step('Cancel and verify dialog closes', async () => { + await teamPage.cancelConfirmDialog(); + await expect( + teamPage.confirmDialogBox.getByText(/invalidate your existing API key/), + ).toBeHidden(); + }); + }); + + test('should open and cancel rotate personal access key dialog', async () => { + await test.step('Open rotate personal access key dialog', async () => { + await teamPage.openApiAndAgentsTab(); + await teamPage.clickRotateAccessKey(); + }); + + await test.step('Verify dialog warns about irreversibility and agent configs', async () => { + await expect( + teamPage.confirmDialogBox.getByText(/not reversible/), + ).toBeVisible(); + await expect( + teamPage.confirmDialogBox.getByText(/MCP \/ AI agent configs/), + ).toBeVisible(); }); - await test.step('Cancel and verify modal closes', async () => { - await teamPage.cancelRotateApiKey(); - await expect(teamPage.page.getByText('not reversible')).toBeHidden(); + await test.step('Cancel and verify dialog closes', async () => { + await teamPage.cancelConfirmDialog(); + await expect( + teamPage.confirmDialogBox.getByText(/MCP \/ AI agent configs/), + ).toBeHidden(); }); }); diff --git a/packages/app/tests/e2e/page-objects/TeamPage.ts b/packages/app/tests/e2e/page-objects/TeamPage.ts index 07acbf0ef3..a859da1e9f 100644 --- a/packages/app/tests/e2e/page-objects/TeamPage.ts +++ b/packages/app/tests/e2e/page-objects/TeamPage.ts @@ -36,9 +36,10 @@ export class TeamPage { private readonly teamNameCancelButton: Locator; // API Keys elements + // Both rotate flows go through the shared `useConfirm` dialog, so only the + // triggers are per-key. Its locators are confirmDialog* below. private readonly rotateApiKeyButton: Locator; - private readonly rotateApiKeyConfirm: Locator; - private readonly rotateApiKeyCancel: Locator; + private readonly rotateAccessKeyButton: Locator; // Connections elements private readonly addConnectionButton: Locator; @@ -54,6 +55,8 @@ export class TeamPage { private readonly confirmDeleteMemberButton: Locator; private readonly cancelDeleteMemberButton: Locator; private readonly confirmDialogConfirmBtn: Locator; + private readonly confirmDialogCancelBtn: Locator; + private readonly confirmDialogModal: Locator; constructor(page: Page) { this.page = page; @@ -101,8 +104,7 @@ export class TeamPage { this.teamNameCancelButton = page.getByTestId('team-name-cancel-button'); this.rotateApiKeyButton = page.getByTestId('rotate-api-key-button'); - this.rotateApiKeyConfirm = page.getByTestId('rotate-api-key-confirm'); - this.rotateApiKeyCancel = page.getByTestId('rotate-api-key-cancel'); + this.rotateAccessKeyButton = page.getByTestId('rotate-access-key-button'); this.addConnectionButton = page.getByTestId('add-connection-button'); @@ -115,6 +117,8 @@ export class TeamPage { this.confirmDeleteMemberButton = page.getByTestId('confirm-delete-member'); this.cancelDeleteMemberButton = page.getByTestId('cancel-delete-member'); this.confirmDialogConfirmBtn = page.getByTestId('confirm-confirm-button'); + this.confirmDialogCancelBtn = page.getByTestId('confirm-cancel-button'); + this.confirmDialogModal = page.getByTestId('confirm-modal'); } async goto() { @@ -191,12 +195,16 @@ export class TeamPage { await this.rotateApiKeyButton.click(); } - async confirmRotateApiKey() { - await this.rotateApiKeyConfirm.click(); + async clickRotateAccessKey() { + await this.rotateAccessKeyButton.click(); } - async cancelRotateApiKey() { - await this.rotateApiKeyCancel.click(); + // Confirming rotates the shared E2E account's keys. For the personal key that + // 401s the bearer requests in dashboard-external-api-*.spec.ts running in + // parallel workers, so the rotate specs only cancel. See the note in + // team.spec.ts; the confirm path is covered by me.int.test.ts. + async cancelConfirmDialog() { + await this.confirmDialogCancelBtn.click(); } // --- Connections --- @@ -396,6 +404,15 @@ export class TeamPage { return this.rotateApiKeyButton; } + get rotateAccessKeyTrigger() { + return this.rotateAccessKeyButton; + } + + /** Shared `useConfirm` dialog, used by both rotate flows. */ + get confirmDialogBox() { + return this.confirmDialogModal; + } + get members() { return this.teamMembersSection; } diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index 8f38bf7f5a..5e60bd0276 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -2501,6 +2501,20 @@ export const MeApiResponseSchema = z.object({ export type MeApiResponse = z.infer; +// Response for `PATCH /me/accessKey`. +// +// Deliberately not RotateApiKeyApiResponseSchema (`{ newApiKey }`): `team.apiKey` +// is the shared ingestion key, while `user.accessKey` is the per-user bearer +// token for the external API v2 and the MCP server. The two are rendered side by +// side in Team Settings, so the wire names must not blur together. +export const RotateAccessKeyApiResponseSchema = z.object({ + newAccessKey: z.string(), +}); + +export type RotateAccessKeyApiResponse = z.infer< + typeof RotateAccessKeyApiResponseSchema +>; + // IaC (Terraform) export // // Shared so `GET /iac/import-manifest` and the generators in