From f985e616d63968707defaa44198485e5035f474e Mon Sep 17 00:00:00 2001 From: "courtmanr@gmail.com" Date: Thu, 30 Jul 2026 12:06:19 +0100 Subject: [PATCH] Allow in-place API token scope edits (#1629) --- docs/API.md | 11 + .../v6/internal/subsystems/agent-lifecycle.md | 13 +- .../v6/internal/subsystems/api-contracts.md | 17 +- .../subsystems/frontend-primitives.md | 5 +- .../internal/subsystems/security-privacy.md | 13 + .../internal/subsystems/storage-recovery.md | 6 + .../src/api/__tests__/security.test.ts | 25 ++ frontend-modern/src/api/security.ts | 11 + .../components/Settings/APITokenManager.tsx | 176 ++++++++- .../__tests__/APITokenManager.test.tsx | 112 ++++++ .../Settings/useAPITokenManagerState.ts | 26 ++ .../__tests__/apiTokenPresentation.test.ts | 16 + .../src/utils/apiTokenPresentation.ts | 15 + ..._token_scope_transport_integration_test.go | 8 + internal/api/router_routes_auth_security.go | 4 + internal/api/security_tokens.go | 117 ++++++ internal/api/security_tokens_update_test.go | 341 ++++++++++++++++++ 17 files changed, 899 insertions(+), 17 deletions(-) create mode 100644 internal/api/security_tokens_update_test.go diff --git a/docs/API.md b/docs/API.md index 77f3fe0bb..2cd861424 100644 --- a/docs/API.md +++ b/docs/API.md @@ -636,6 +636,17 @@ In Docker installs, the response includes a restart notice. { "name": "ansible-script", "scopes": ["monitoring:read"] } ``` +### Edit API Token Scopes +`PATCH /api/security/tokens/` +```json +{ "scopes": ["monitoring:read", "settings:read"] } +``` + +The `scopes` field is required and must contain at least one known scope. +Wildcard access (`"*"`) cannot be combined with other scopes. The update takes +effect on the token's next request without changing its ID, secret, expiry, or +organization bindings. + ### Revoke Token `DELETE /api/security/tokens/` diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 8b3c7ab74..7ae5f3392 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -273,10 +273,15 @@ instead of reporting drift against a token that cannot execute. Install tokens for the generic host agent flow are minted server-side through the agent install command endpoint. The server decides the token's scopes from the operator's command-execution choice at mint time, because scopes are never -upgraded on an existing token, and stamps the install-type and issuance -metadata that make the token eligible for one first-use command-channel -binding. Frontend surfaces may choose whether commands are requested but must -not compose install-token scope lists themselves. +upgraded implicitly by an install or repair flow, and stamps the install-type +and issuance metadata that make the token eligible for one first-use +command-channel binding. The operator-owned +`PATCH /api/security/tokens/` scope editor is a separate API/security +mutation: it may deliberately change an installed agent token's live +authority, but agent lifecycle code must not call it to auto-upgrade, repair, +or reconcile command permissions. Frontend install surfaces may choose whether +commands are requested for a newly minted credential but must not compose +install-token scope lists themselves. PVE node setup shared boundaries that render or copy `PulseMonitor` permissions must treat `VM.GuestAgent.Audit` plus `VM.GuestAgent.FileRead` as diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index c12d0670a..0ae59d23f 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -7583,7 +7583,7 @@ top-level preset array that can reintroduce settings-chunk initialization-order failures in production bundles. That same boundary now also includes `frontend-modern/src/utils/apiTokenPresentation.ts`, so token load/create/ -revoke errors keep one governed customer-facing message source instead of +update/revoke errors keep one governed customer-facing message source instead of reappearing as hook-local strings. That same token surface, together with `frontend-modern/src/api/security.ts`, `internal/api/security.go`, `internal/api/security_tokens.go`, and @@ -7611,9 +7611,18 @@ shared setup helpers must not point API responses or runtime guidance at GitHub `main` for security instructions that the running build already serves locally; those references belong on the shipped `/docs/SECURITY.md` path. That same governed token contract must fail closed on mutation. Limited-scope -API tokens may only create, rotate, or delete tokens whose effective scopes -are a subset of the caller's own scopes; token-management routes must not let a -settings-capable but narrower token revoke or replace a broader credential. +API tokens may only create, update, rotate, or delete tokens whose effective +scopes are a subset of the caller's own scopes; token-management routes must +not let a settings-capable but narrower token revoke, narrow, or replace a +broader credential. `PATCH /api/security/tokens/` requires an explicit +non-empty canonical `scopes` array and checks both the target's existing scope +set and the requested scope set against token-authenticated caller authority. +A successful patch preserves token identity, secret hash, expiry, metadata, +and organization bindings, takes effect on the next request, and returns the +updated metadata record without exposing or rotating the raw secret. The +mutation must emit a `token_updated` audit event containing the old and new +scope sets, and a persistence failure must restore the complete in-memory +record before returning failure. Those owner-bound credentials now also define the effective authenticated principal on governed API routes: when token metadata carries `ownerUserId`, RBAC and audit-facing auth resolution must use that bound user identity rather diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index 1ca9c62db..c30f49f22 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -3332,7 +3332,10 @@ pressed button shell and `aria-pressed` wiring, while `frontend-modern/src/components/shared/selectablePillModel.ts` owns the active and inactive pill class catalog. API token scope surfaces may own the security scope labels and click handlers, but must not recreate rounded-full selector -pill class strings locally. +pill class strings locally. A token's in-place scope editor may use semantic +native checkboxes because it is a multi-select form checklist rather than a +pressed pill selector; it must not imitate or fork the selectable-pill class +catalog. Filter-toolbar segmented controls must delegate to this primitive rather than calling `segmentedButtonClass` directly, and icon+text labels must render as one inline-flex button label so compact bars keep the v5 single-line control diff --git a/docs/release-control/v6/internal/subsystems/security-privacy.md b/docs/release-control/v6/internal/subsystems/security-privacy.md index ccf739dd4..6036f76ad 100644 --- a/docs/release-control/v6/internal/subsystems/security-privacy.md +++ b/docs/release-control/v6/internal/subsystems/security-privacy.md @@ -119,6 +119,11 @@ with missing, unknown, or unrelated scopes fail closed. Full access is a deliberate wildcard choice, not the default empty selection. The token creation form must require an explicit scoped preset, custom scope, or Full access selection before a credential can be minted. + Existing token rows may open an explicit checkbox checklist for in-place + scope editing. That editor must initialize from the token's effective live + scopes, preserve wildcard exclusivity, reject empty or unchanged + submissions, state that the token value does not rotate, and keep the + dialog open when the backend rejects the transition. Stable in-page anchors for sibling API Access onboarding panels are allowed only as navigation into the token creation section; those sibling panels do not own token scope derivation or preset contents. @@ -1297,6 +1302,14 @@ That same trust boundary also governs API token scope identity: legacy migration boundaries, where they must be rewritten immediately into canonical `agent:*` scopes. Live token records and runtime scope checks may not keep the legacy scope names as an active second contract. +In-place token scope mutation belongs to that same boundary. +`PATCH /api/security/tokens/` must require `settings:write`, normalize and +validate the complete replacement scope set, and prevent token-authenticated +callers from controlling a target whose existing authority exceeds their own +or granting authority they do not hold. Successful edits preserve the token +secret and all identity/binding metadata, apply to the next request, and +record both sides of the transition in a `token_updated` audit event without +including a raw secret or hash. Update-readiness checks may inspect loaded API token metadata to determine whether agent reporting scope exists or has expired, but they must not expose raw token values, token hashes, or owner metadata in the update plan payload. diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index bac035507..d21504240 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -4183,6 +4183,12 @@ token-store write that fails after the nodes write would otherwise leave a persisted source next to a still-live one-shot grant, so a persistently failing token store becomes a repeatable source-creation primitive. Either both stores advanced or neither did. +The API-token store also owns atomic in-place scope replacement. +`PATCH /api/security/tokens/` may change only the canonical scope slice; it +must preserve the stored hash, ID, timestamps, expiry, metadata, and +organization bindings. If `SaveAPITokens` fails, the complete prior in-memory +record must be restored so runtime authorization cannot diverge from the +durable token store. That same shared dependency also assumes generated setup scripts preserve setup-token auth guidance, so adjacent setup flows do not regress back to stale API-token instructions after the backend has already standardized on the diff --git a/frontend-modern/src/api/__tests__/security.test.ts b/frontend-modern/src/api/__tests__/security.test.ts index ed7d3a19e..f391cb320 100644 --- a/frontend-modern/src/api/__tests__/security.test.ts +++ b/frontend-modern/src/api/__tests__/security.test.ts @@ -209,6 +209,31 @@ describe('SecurityAPI', () => { }); }); + describe('updateTokenScopes', () => { + it('patches an encoded token id and returns the updated record', async () => { + const updated: APITokenRecord = { + id: 'token/1', + name: 'Automation', + prefix: 'pulse', + suffix: '1234', + createdAt: '2026-07-30T10:00:00Z', + scopes: ['monitoring:read', 'settings:read'], + }; + vi.mocked(apiFetchJSON).mockResolvedValueOnce({ record: updated }); + + const result = await SecurityAPI.updateTokenScopes('token/1', [ + 'monitoring:read', + 'settings:read', + ]); + + expect(apiFetchJSON).toHaveBeenCalledWith('/api/security/tokens/token%2F1', { + method: 'PATCH', + body: JSON.stringify({ scopes: ['monitoring:read', 'settings:read'] }), + }); + expect(result).toEqual(updated); + }); + }); + describe('deleteToken', () => { it('deletes a token', async () => { vi.mocked(apiFetchJSON).mockResolvedValueOnce(undefined); diff --git a/frontend-modern/src/api/security.ts b/frontend-modern/src/api/security.ts index fa35cc630..922926cd7 100644 --- a/frontend-modern/src/api/security.ts +++ b/frontend-modern/src/api/security.ts @@ -49,6 +49,17 @@ export class SecurityAPI { }); } + static async updateTokenScopes(id: string, scopes: string[]): Promise { + const response = await apiFetchJSON<{ record: APITokenRecord }>( + `/api/security/tokens/${encodeURIComponent(id)}`, + { + method: 'PATCH', + body: JSON.stringify({ scopes }), + }, + ); + return response.record; + } + static async deleteToken(id: string): Promise { await apiFetchJSON(`/api/security/tokens/${encodeURIComponent(id)}`, { method: 'DELETE', diff --git a/frontend-modern/src/components/Settings/APITokenManager.tsx b/frontend-modern/src/components/Settings/APITokenManager.tsx index 91fbf8eb2..7206d6e3d 100644 --- a/frontend-modern/src/components/Settings/APITokenManager.tsx +++ b/frontend-modern/src/components/Settings/APITokenManager.tsx @@ -26,6 +26,8 @@ interface APITokenManagerProps { export const APITokenManager: Component = (props) => { const [tokenToRevoke, setTokenToRevoke] = createSignal(null); + const [tokenToEdit, setTokenToEdit] = createSignal(null); + const [editScopes, setEditScopes] = createSignal([]); const { API_SCOPE_LABELS, API_TOKEN_SCOPES_DOC_URL, @@ -43,6 +45,7 @@ export const APITokenManager: Component = (props) => { formatRelativeTime, handleDelete, handleGenerate, + handleUpdateScopes, hasWildcardTokens, hasScopeSelection, isFullAccessSelected, @@ -64,10 +67,56 @@ export const APITokenManager: Component = (props) => { tokenHint, toggleScope, totalTokens, + updatingTokenId, wildcardCount, presetMatchesSelection, } = useAPITokenManagerState(props); + const effectiveScopes = (token: APITokenRecord) => + token.scopes && token.scopes.length > 0 ? token.scopes : ['*']; + + const openScopeEditor = (token: APITokenRecord) => { + setEditScopes([...effectiveScopes(token)]); + setTokenToEdit(token); + }; + + const closeScopeEditor = () => { + if (updatingTokenId() !== null) return; + setTokenToEdit(null); + setEditScopes([]); + }; + + const toggleEditScope = (scope: string) => { + setEditScopes((previous) => { + if (scope === '*') { + return previous.includes('*') ? [] : ['*']; + } + const scoped = previous.filter((value) => value !== '*'); + if (scoped.includes(scope)) { + return scoped.filter((value) => value !== scope); + } + return [...scoped, scope]; + }); + }; + + const scopeSelectionKey = (scopes: string[]) => Array.from(new Set(scopes)).sort().join('\u0000'); + + const editScopesChanged = () => { + const token = tokenToEdit(); + return token + ? scopeSelectionKey(editScopes()) !== scopeSelectionKey(effectiveScopes(token)) + : false; + }; + + const saveEditedScopes = async () => { + const token = tokenToEdit(); + if (!token || editScopes().length === 0 || !editScopesChanged()) return; + if (await handleUpdateScopes(token, editScopes())) { + setTokenToEdit(null); + setEditScopes([]); + } + }; + return (
@@ -389,16 +438,27 @@ export const APITokenManager: Component = (props) => { }, { key: 'action', - label: 'Action', + label: 'Actions', align: 'right', render: (token) => ( - +
+ + +
), }, ]} @@ -559,6 +619,106 @@ export const APITokenManager: Component = (props) => { + + +
+
+

Edit token scopes

+

+ Changes to{' '} + + {tokenToEdit()!.name || tokenToEdit()!.id} + {' '} + take effect on its next request. The token value and expiry do not change. +

+
+ + + +
+ + {([group, options]) => ( +
+ + {group} + +
+ + {(option) => ( + + )} + +
+
+ )} + +
+ + +

+ Select at least one scope before saving. +

+ + +
+ + +
+
+ + + {/* Revoke confirmation modal — token deletion is irreversible and breaks any agents/integrations relying on the token, so guard the action behind an explicit confirm. */} diff --git a/frontend-modern/src/components/Settings/__tests__/APITokenManager.test.tsx b/frontend-modern/src/components/Settings/__tests__/APITokenManager.test.tsx index dab06325a..916d511f2 100644 --- a/frontend-modern/src/components/Settings/__tests__/APITokenManager.test.tsx +++ b/frontend-modern/src/components/Settings/__tests__/APITokenManager.test.tsx @@ -23,6 +23,7 @@ import { APITokenManager } from '../APITokenManager'; const listTokensMock = vi.fn(); const createTokenMock = vi.fn(); +const updateTokenScopesMock = vi.fn(); const deleteTokenMock = vi.fn(); const fetchAgentCapabilitiesManifestMock = vi.fn(); const notificationSuccessMock = vi.fn(); @@ -39,6 +40,7 @@ vi.mock('@/api/security', () => ({ SecurityAPI: { listTokens: (...args: unknown[]) => listTokensMock(...args), createToken: (...args: unknown[]) => createTokenMock(...args), + updateTokenScopes: (...args: unknown[]) => updateTokenScopesMock(...args), deleteToken: (...args: unknown[]) => deleteTokenMock(...args), }, })); @@ -148,6 +150,7 @@ describe('APITokenManager', () => { beforeEach(() => { listTokensMock.mockReset(); createTokenMock.mockReset(); + updateTokenScopesMock.mockReset(); deleteTokenMock.mockReset(); notificationSuccessMock.mockReset(); notificationErrorMock.mockReset(); @@ -195,6 +198,9 @@ describe('APITokenManager', () => { }), }); deleteTokenMock.mockResolvedValue(undefined); + updateTokenScopesMock.mockImplementation( + async (id: string, scopes: string[]): Promise => makeToken({ id, scopes }), + ); }); afterEach(() => { @@ -528,6 +534,112 @@ describe('APITokenManager', () => { }); }); + it('edits a token scope set in place without rotating or revoking it', async () => { + const onTokensChanged = vi.fn(); + listTokensMock.mockResolvedValue([ + makeToken({ + id: 'token-edit', + name: 'Editable token', + scopes: [DOCKER_REPORT_SCOPE], + }), + ]); + updateTokenScopesMock.mockResolvedValue( + makeToken({ + id: 'token-edit', + name: 'Editable token', + scopes: [MONITORING_READ_SCOPE], + }), + ); + + render(() => ); + + const tokenName = await screen.findByText('Editable token'); + const row = tokenName.closest('tr'); + expect(row).toBeTruthy(); + fireEvent.click( + within(row as HTMLTableRowElement).getByRole('button', { name: 'Edit scopes' }), + ); + + const dialog = await screen.findByRole('dialog', { name: 'Edit API token scopes' }); + expect(within(dialog).getByText(/take effect on its next request/i)).toBeInTheDocument(); + const dockerReport = within(dialog).getByRole('checkbox', { + name: /Docker \/ Podman reporting/i, + }); + const monitoringRead = within(dialog).getByRole('checkbox', { + name: /Monitoring & alerts \(read\)/i, + }); + expect(dockerReport).toBeChecked(); + expect(monitoringRead).not.toBeChecked(); + + fireEvent.click(dockerReport); + fireEvent.click(monitoringRead); + fireEvent.click(within(dialog).getByRole('button', { name: 'Save scopes' })); + + await waitFor(() => { + expect(updateTokenScopesMock).toHaveBeenCalledWith('token-edit', [MONITORING_READ_SCOPE]); + }); + expect(deleteTokenMock).not.toHaveBeenCalled(); + expect(createTokenMock).not.toHaveBeenCalled(); + expect(onTokensChanged).toHaveBeenCalledTimes(1); + expect(notificationSuccessMock).toHaveBeenCalledWith('Scopes updated for Editable token.'); + + await waitFor(() => { + expect( + screen.queryByRole('dialog', { name: 'Edit API token scopes' }), + ).not.toBeInTheDocument(); + }); + const updatedRow = screen.getByText('Editable token').closest('tr'); + expect( + within(updatedRow as HTMLTableRowElement).getByText('Monitoring & alerts (read)'), + ).toBeInTheDocument(); + expect( + within(updatedRow as HTMLTableRowElement).queryByText('Docker / Podman reporting'), + ).not.toBeInTheDocument(); + }); + + it('requires at least one changed scope and keeps failed edits open', async () => { + listTokensMock.mockResolvedValue([ + makeToken({ + id: 'token-edit-failure', + name: 'Protected token', + scopes: [DOCKER_REPORT_SCOPE], + }), + ]); + updateTokenScopesMock.mockRejectedValueOnce(new Error('Cannot grant scope "monitoring:read"')); + + render(() => ); + + const tokenName = await screen.findByText('Protected token'); + const row = tokenName.closest('tr'); + fireEvent.click( + within(row as HTMLTableRowElement).getByRole('button', { name: 'Edit scopes' }), + ); + + const dialog = await screen.findByRole('dialog', { name: 'Edit API token scopes' }); + const save = within(dialog).getByRole('button', { name: 'Save scopes' }); + expect(save).toBeDisabled(); + + fireEvent.click(within(dialog).getByRole('checkbox', { name: /Docker \/ Podman reporting/i })); + expect( + within(dialog).getByText('Select at least one scope before saving.'), + ).toBeInTheDocument(); + expect(save).toBeDisabled(); + + fireEvent.click( + within(dialog).getByRole('checkbox', { name: /Monitoring & alerts \(read\)/i }), + ); + expect(save).not.toBeDisabled(); + fireEvent.click(save); + + await waitFor(() => { + expect(updateTokenScopesMock).toHaveBeenCalledWith('token-edit-failure', [ + MONITORING_READ_SCOPE, + ]); + }); + expect(notificationErrorMock).toHaveBeenCalledWith('Unable to update API token scopes.'); + expect(screen.getByRole('dialog', { name: 'Edit API token scopes' })).toBeInTheDocument(); + }); + it('keeps governed infrastructure token usage labels on local operator identity', async () => { listTokensMock.mockResolvedValue([ makeToken({ diff --git a/frontend-modern/src/components/Settings/useAPITokenManagerState.ts b/frontend-modern/src/components/Settings/useAPITokenManagerState.ts index 51b6355b6..81c5c4e9b 100644 --- a/frontend-modern/src/components/Settings/useAPITokenManagerState.ts +++ b/frontend-modern/src/components/Settings/useAPITokenManagerState.ts @@ -26,6 +26,7 @@ import { getAPITokenRevealSettingsNote, getAPITokensLoadErrorMessage, getAPITokenRevokeErrorMessage, + getAPITokenUpdateErrorMessage, } from '@/utils/apiTokenPresentation'; import { logger } from '@/utils/logger'; import { getPulseBaseUrl } from '@/utils/url'; @@ -73,6 +74,7 @@ export const useAPITokenManagerState = (props: APITokenManagerProps) => { const [tokensLoaded, setTokensLoaded] = createSignal(false); const [loading, setLoading] = createSignal(true); const [isGenerating, setIsGenerating] = createSignal(false); + const [updatingTokenId, setUpdatingTokenId] = createSignal(null); const [newTokenValue, setNewTokenValue] = createSignal(null); const [newTokenRecord, setNewTokenRecord] = createSignal(null); const [nameInput, setNameInput] = createSignal(''); @@ -391,6 +393,28 @@ export const useAPITokenManagerState = (props: APITokenManagerProps) => { } }; + const handleUpdateScopes = async (record: APITokenRecord, scopes: string[]) => { + if (!canManage() || scopes.length === 0 || updatingTokenId() !== null) return false; + + setUpdatingTokenId(record.id); + try { + const updated = await SecurityAPI.updateTokenScopes(record.id, [...scopes].sort()); + setTokens((previous) => previous.map((token) => (token.id === updated.id ? updated : token))); + if (newTokenRecord()?.id === updated.id) { + setNewTokenRecord(updated); + } + notificationStore.success(`Scopes updated for ${getAPITokenDialogName(updated)}.`); + props.onTokensChanged?.(); + return true; + } catch (err) { + logger.error('Failed to update API token scopes', err); + notificationStore.error(getAPITokenUpdateErrorMessage(err)); + return false; + } finally { + setUpdatingTokenId(null); + } + }; + const isRevealActiveForCurrentToken = () => { const active = tokenRevealState(); return newTokenValue() !== null && Boolean(active && active.token === newTokenValue()); @@ -447,10 +471,12 @@ export const useAPITokenManagerState = (props: APITokenManagerProps) => { formatRelativeTime, handleDelete, handleGenerate, + handleUpdateScopes, hasWildcardTokens, hasScopeSelection, isFullAccessSelected, isGenerating, + updatingTokenId, isRevealActiveForCurrentToken, loading, nameInput, diff --git a/frontend-modern/src/utils/__tests__/apiTokenPresentation.test.ts b/frontend-modern/src/utils/__tests__/apiTokenPresentation.test.ts index 6b26aafe0..5d1b19b3c 100644 --- a/frontend-modern/src/utils/__tests__/apiTokenPresentation.test.ts +++ b/frontend-modern/src/utils/__tests__/apiTokenPresentation.test.ts @@ -17,6 +17,7 @@ import { getAPITokenRevealSettingsNote, getAPITokensLoadErrorMessage, getAPITokenRevokeErrorMessage, + getAPITokenUpdateErrorMessage, } from '@/utils/apiTokenPresentation'; describe('apiTokenPresentation', () => { @@ -24,6 +25,7 @@ describe('apiTokenPresentation', () => { expect(getAPITokensLoadErrorMessage()).toBe('Unable to load API tokens.'); expect(getAPITokenGenerateErrorMessage()).toBe('Unable to generate the API token.'); expect(getAPITokenRevokeErrorMessage()).toBe('Unable to revoke the API token.'); + expect(getAPITokenUpdateErrorMessage()).toBe('Unable to update API token scopes.'); }); it('returns canonical API token settings location copy', () => { @@ -82,6 +84,20 @@ describe('apiTokenPresentation', () => { ); }); + it('surfaces token scope denial copy for update failures', () => { + const grantError = Object.assign( + new Error('Cannot grant scope "monitoring:write": your token does not have this scope'), + { status: 403 }, + ); + const targetError = Object.assign( + new Error('Cannot update token with scope "*": your token does not have this scope'), + { status: 403 }, + ); + + expect(getAPITokenUpdateErrorMessage(grantError)).toBe(grantError.message); + expect(getAPITokenUpdateErrorMessage(targetError)).toBe(targetError.message); + }); + it('surfaces required scope when middleware returns missing_scope', () => { const error = Object.assign(new Error('missing_scope'), { status: 403, diff --git a/frontend-modern/src/utils/apiTokenPresentation.ts b/frontend-modern/src/utils/apiTokenPresentation.ts index 3d3bf8057..21f4df91c 100644 --- a/frontend-modern/src/utils/apiTokenPresentation.ts +++ b/frontend-modern/src/utils/apiTokenPresentation.ts @@ -64,6 +64,21 @@ export function getAPITokenRevokeErrorMessage(): string { return 'Unable to revoke the API token.'; } +export function getAPITokenUpdateErrorMessage(error?: unknown): string { + if (error && typeof error === 'object') { + const typedError = error as APITokenErrorShape; + const message = typedError.message?.trim(); + if ( + typedError.status === 403 && + message && + (message.startsWith('Cannot grant scope') || message.startsWith('Cannot update token')) + ) { + return message; + } + } + return 'Unable to update API token scopes.'; +} + export function getAPITokenDockerPodmanUsageCountLabel(count: number): string { return count === 1 ? API_TOKEN_DOCKER_PODMAN_RUNTIME_LABEL diff --git a/internal/api/api_token_scope_transport_integration_test.go b/internal/api/api_token_scope_transport_integration_test.go index 2a0a71052..ab63c1c6c 100644 --- a/internal/api/api_token_scope_transport_integration_test.go +++ b/internal/api/api_token_scope_transport_integration_test.go @@ -43,6 +43,14 @@ func TestBearerAPITokenScopesDenyReadWriteAndExecRoutes(t *testing.T) { token: "bearer-write-token-123.12345678", wantScopeHint: config.ScopeSettingsWrite, }, + { + name: "scope update route missing settings write", + method: http.MethodPatch, + path: "/api/security/tokens/token-1", + body: `{"scopes":["monitoring:read"]}`, + token: "bearer-write-token-123.12345678", + wantScopeHint: config.ScopeSettingsWrite, + }, { name: "exec route missing ai execute", method: http.MethodPost, diff --git a/internal/api/router_routes_auth_security.go b/internal/api/router_routes_auth_security.go index 918318fdf..eda444967 100644 --- a/internal/api/router_routes_auth_security.go +++ b/internal/api/router_routes_auth_security.go @@ -292,6 +292,10 @@ func (r *Router) registerAuthSecurityInstallRoutes() { if !ensureSettingsWriteScope(r.config, w, req) { return } + if req.Method == http.MethodPatch { + r.handleUpdateAPIToken(w, req) + return + } if strings.HasSuffix(req.URL.Path, "/rotate") && req.Method == http.MethodPost { r.handleRotateAPIToken(w, req) return diff --git a/internal/api/security_tokens.go b/internal/api/security_tokens.go index d57283061..5f781bce4 100644 --- a/internal/api/security_tokens.go +++ b/internal/api/security_tokens.go @@ -293,6 +293,10 @@ type createTokenRequest struct { ExpiresIn *string `json:"expiresIn,omitempty"` // e.g. "24h", "720h", "8760h" } +type updateTokenRequest struct { + Scopes *[]string `json:"scopes"` +} + func (r *Router) auditTokenEvent(req *http.Request, event string, success bool, details string) { user := internalauth.GetUser(req.Context()) if user == "" && r != nil && r.config != nil { @@ -369,6 +373,119 @@ func (r *Router) handleCreateAPIToken(w http.ResponseWriter, req *http.Request) }) } +// handleUpdateAPIToken changes an API token's scopes without rotating its secret. +func (r *Router) handleUpdateAPIToken(w http.ResponseWriter, req *http.Request) { + if req.Method != http.MethodPatch { + http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) + return + } + + tokenID := strings.TrimSpace(strings.TrimPrefix(req.URL.Path, "/api/security/tokens/")) + if tokenID == "" || strings.Contains(tokenID, "/") { + r.auditTokenEvent(req, "token_updated", false, "Missing or invalid API token ID in update request") + http.Error(w, "Token ID required", http.StatusBadRequest) + return + } + + var payload updateTokenRequest + if err := json.NewDecoder(req.Body).Decode(&payload); err != nil { + r.auditTokenEvent(req, "token_updated", false, fmt.Sprintf("Failed to decode scope update for token id=%s", tokenID)) + log.Warn().Err(err).Str("token_id", tokenID).Msg("Failed to decode API token update request") + http.Error(w, "Invalid request body", http.StatusBadRequest) + return + } + if payload.Scopes == nil { + r.auditTokenEvent(req, "token_updated", false, fmt.Sprintf("Missing scopes field for token id=%s", tokenID)) + http.Error(w, "Scopes field required", http.StatusBadRequest) + return + } + + scopes, err := normalizeRequestedScopes(payload.Scopes) + if err != nil { + r.auditTokenEvent(req, "token_updated", false, fmt.Sprintf("Invalid scope update for token id=%s", tokenID)) + log.Warn().Err(err).Str("token_id", tokenID).Msg("Invalid scopes provided for API token update") + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + + config.Mu.Lock() + defer config.Mu.Unlock() + + tokenIndex := -1 + for idx := range r.config.APITokens { + if r.config.APITokens[idx].ID == tokenID { + tokenIndex = idx + break + } + } + if tokenIndex == -1 { + r.auditTokenEvent(req, "token_updated", false, fmt.Sprintf("API token id=%s not found for update", tokenID)) + http.Error(w, "Token not found", http.StatusNotFound) + return + } + + storedRecord := r.config.APITokens[tokenIndex] + oldRecord := storedRecord.Clone() + + // A token-authenticated caller must be able to control both sides of the + // transition. Checking the existing scopes prevents a limited caller from + // narrowing or otherwise mutating a more privileged credential; checking + // the requested scopes prevents widening a token beyond the caller's own + // authority, including when editing itself. + if callerToken := getAPITokenRecordFromRequest(req); callerToken != nil { + for _, scope := range oldRecord.Scopes { + if !callerToken.HasScope(scope) { + r.auditTokenEvent(req, "token_updated", false, + fmt.Sprintf("Scope update denied: caller missing existing scope %q on target token id=%s", scope, tokenID)) + http.Error(w, fmt.Sprintf("Cannot update token with scope %q: your token does not have this scope", scope), http.StatusForbidden) + return + } + } + for _, scope := range scopes { + if !callerToken.HasScope(scope) { + r.auditTokenEvent(req, "token_updated", false, + fmt.Sprintf("Scope update denied: caller cannot grant scope %q on target token id=%s", scope, tokenID)) + http.Error(w, fmt.Sprintf("Cannot grant scope %q: your token does not have this scope", scope), http.StatusForbidden) + return + } + } + } + + r.config.APITokens[tokenIndex].Scopes = append([]string(nil), scopes...) + if r.persistence != nil { + if err := r.persistence.SaveAPITokens(r.config.APITokens); err != nil { + r.config.APITokens[tokenIndex] = storedRecord + r.auditTokenEvent(req, "token_updated", false, fmt.Sprintf("Failed to persist scope update for token id=%s", tokenID)) + log.Error().Err(err).Str("token_id", tokenID).Msg("Failed to persist API token scope update") + http.Error(w, "Failed to save token", http.StatusInternalServerError) + return + } + } + + updatedRecord := r.config.APITokens[tokenIndex] + r.auditTokenEvent(req, "token_updated", true, + fmt.Sprintf( + "Updated API token id=%s old_scopes=%s new_scopes=%s", + tokenID, + strings.Join(oldRecord.Scopes, ","), + strings.Join(updatedRecord.Scopes, ","), + )) + + log.Info(). + Str("audit_event", "token_updated"). + Str("token_id", tokenID). + Str("token_name", updatedRecord.Name). + Strs("old_scopes", oldRecord.Scopes). + Strs("new_scopes", updatedRecord.Scopes). + Str("client_ip", req.RemoteAddr). + Msg("API token scopes updated") + + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{ + "record": toAPITokenDTO(updatedRecord), + }) +} + // handleCreateRelayMobileAccessToken generates the canonical Pulse Mobile relay runtime token. func (r *Router) handleCreateRelayMobileAccessToken(w http.ResponseWriter, req *http.Request) { if req.Method != http.MethodPost { diff --git a/internal/api/security_tokens_update_test.go b/internal/api/security_tokens_update_test.go new file mode 100644 index 000000000..56c07c41a --- /dev/null +++ b/internal/api/security_tokens_update_test.go @@ -0,0 +1,341 @@ +package api + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/rcourtman/pulse-go-rewrite/internal/config" + "github.com/rcourtman/pulse-go-rewrite/pkg/audit" + authpkg "github.com/rcourtman/pulse-go-rewrite/pkg/auth" +) + +func TestSecurityTokens_UpdateScopesPreservesCredentialAndAuditsTransition(t *testing.T) { + capture := &auditCaptureLogger{} + prevLogger := audit.GetLogger() + prevManager := GetTenantAuditManager() + audit.SetLogger(capture) + SetTenantAuditManager(nil) + t.Cleanup(func() { + audit.SetLogger(prevLogger) + SetTenantAuditManager(prevManager) + }) + + record := newTokenRecord(t, "update-target-token-123.12345678", []string{config.ScopeSettingsRead}, map[string]string{ + "bound_agent_id": "agent-1", + }) + record.Name = "automation" + record.OrgID = "acme" + record.OrgIDs = []string{"acme", "beta"} + originalHash := record.Hash + originalCreatedAt := record.CreatedAt + + cfg := newTestConfigWithTokens(t, record) + router := &Router{ + config: cfg, + persistence: config.NewConfigPersistence(t.TempDir()), + } + + req := httptest.NewRequest( + http.MethodPatch, + "/api/security/tokens/"+record.ID, + strings.NewReader(`{"scopes":["monitoring:write","monitoring:read","monitoring:write"]}`), + ) + req = req.WithContext(authpkg.WithUser(req.Context(), "alice")) + rec := httptest.NewRecorder() + router.handleUpdateAPIToken(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d (body=%q)", rec.Code, http.StatusOK, rec.Body.String()) + } + if len(cfg.APITokens) != 1 { + t.Fatalf("APITokens len = %d, want 1", len(cfg.APITokens)) + } + + updated := cfg.APITokens[0] + if updated.ID != record.ID || updated.Hash != originalHash || !updated.CreatedAt.Equal(originalCreatedAt) { + t.Fatalf("credential identity changed during scope update: %+v", updated) + } + if updated.Name != "automation" || updated.OrgID != "acme" || len(updated.OrgIDs) != 2 { + t.Fatalf("token metadata or org bindings changed during scope update: %+v", updated) + } + if updated.Metadata["bound_agent_id"] != "agent-1" { + t.Fatalf("token metadata changed during scope update: %+v", updated.Metadata) + } + if got := strings.Join(updated.Scopes, ","); got != "monitoring:read,monitoring:write" { + t.Fatalf("stored scopes = %q, want %q", got, "monitoring:read,monitoring:write") + } + + var response struct { + Record apiTokenDTO `json:"record"` + } + if err := json.NewDecoder(rec.Body).Decode(&response); err != nil { + t.Fatalf("decode update response: %v", err) + } + if response.Record.ID != record.ID { + t.Fatalf("response token ID = %q, want %q", response.Record.ID, record.ID) + } + if got := strings.Join(response.Record.Scopes, ","); got != "monitoring:read,monitoring:write" { + t.Fatalf("response scopes = %q, want canonical scopes", got) + } + + events, err := capture.Query(audit.QueryFilter{}) + if err != nil { + t.Fatalf("query audit events: %v", err) + } + var updateEvent *audit.Event + for i := range events { + if events[i].EventType == "token_updated" && events[i].Success { + updateEvent = &events[i] + break + } + } + if updateEvent == nil { + t.Fatal("expected successful token_updated audit event") + } + if updateEvent.User != "alice" { + t.Fatalf("audit user = %q, want alice", updateEvent.User) + } + if !strings.Contains(updateEvent.Details, "old_scopes=settings:read") || + !strings.Contains(updateEvent.Details, "new_scopes=monitoring:read,monitoring:write") { + t.Fatalf("audit details do not record scope transition: %q", updateEvent.Details) + } + if strings.Contains(updateEvent.Details, originalHash) { + t.Fatal("audit details leaked token hash") + } +} + +func TestSecurityTokens_UpdateScopesValidatesRequest(t *testing.T) { + tests := []struct { + name string + method string + path string + body string + wantStatus int + wantBody string + }{ + { + name: "wrong method", + method: http.MethodPost, + path: "/api/security/tokens/token-1", + body: `{"scopes":["monitoring:read"]}`, + wantStatus: http.StatusMethodNotAllowed, + wantBody: "Method not allowed", + }, + { + name: "missing token id", + method: http.MethodPatch, + path: "/api/security/tokens/", + body: `{"scopes":["monitoring:read"]}`, + wantStatus: http.StatusBadRequest, + wantBody: "Token ID required", + }, + { + name: "invalid body", + method: http.MethodPatch, + path: "/api/security/tokens/token-1", + body: `{bad`, + wantStatus: http.StatusBadRequest, + wantBody: "Invalid request body", + }, + { + name: "missing scopes", + method: http.MethodPatch, + path: "/api/security/tokens/token-1", + body: `{}`, + wantStatus: http.StatusBadRequest, + wantBody: "Scopes field required", + }, + { + name: "empty scopes", + method: http.MethodPatch, + path: "/api/security/tokens/token-1", + body: `{"scopes":[]}`, + wantStatus: http.StatusBadRequest, + wantBody: "select at least one scope", + }, + { + name: "unknown scope", + method: http.MethodPatch, + path: "/api/security/tokens/token-1", + body: `{"scopes":["unknown:scope"]}`, + wantStatus: http.StatusBadRequest, + wantBody: `unknown scope "unknown:scope"`, + }, + { + name: "wildcard combination", + method: http.MethodPatch, + path: "/api/security/tokens/token-1", + body: `{"scopes":["*","monitoring:read"]}`, + wantStatus: http.StatusBadRequest, + wantBody: "wildcard '*' cannot be combined", + }, + { + name: "missing token", + method: http.MethodPatch, + path: "/api/security/tokens/missing", + body: `{"scopes":["monitoring:read"]}`, + wantStatus: http.StatusNotFound, + wantBody: "Token not found", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + cfg := &config.Config{ + APITokens: []config.APITokenRecord{{ + ID: "token-1", + Name: "target", + Hash: "hash", + Scopes: []string{config.ScopeSettingsRead}, + OrgID: "default", + }}, + } + router := &Router{config: cfg} + req := httptest.NewRequest(test.method, test.path, strings.NewReader(test.body)) + rec := httptest.NewRecorder() + + router.handleUpdateAPIToken(rec, req) + + if rec.Code != test.wantStatus { + t.Fatalf("status = %d, want %d (body=%q)", rec.Code, test.wantStatus, rec.Body.String()) + } + if !strings.Contains(rec.Body.String(), test.wantBody) { + t.Fatalf("body = %q, want fragment %q", rec.Body.String(), test.wantBody) + } + if got := strings.Join(cfg.APITokens[0].Scopes, ","); got != config.ScopeSettingsRead { + t.Fatalf("scopes changed after invalid request: %q", got) + } + }) + } +} + +func TestSecurityTokens_UpdateScopesRejectsScopeEscalationForTokenCaller(t *testing.T) { + t.Run("caller cannot mutate broader existing token", func(t *testing.T) { + target := newTokenRecord(t, "update-broad-target-123.12345678", []string{config.ScopeWildcard}, nil) + caller := newTokenRecord(t, "update-limited-caller-123.12345678", []string{ + config.ScopeSettingsWrite, + config.ScopeMonitoringRead, + }, nil) + cfg := newTestConfigWithTokens(t, target, caller) + router := &Router{config: cfg} + + req := httptest.NewRequest( + http.MethodPatch, + "/api/security/tokens/"+target.ID, + strings.NewReader(`{"scopes":["monitoring:read"]}`), + ) + req = req.WithContext(authpkg.WithAPIToken(req.Context(), &caller)) + rec := httptest.NewRecorder() + router.handleUpdateAPIToken(rec, req) + + if rec.Code != http.StatusForbidden { + t.Fatalf("status = %d, want %d (body=%q)", rec.Code, http.StatusForbidden, rec.Body.String()) + } + if !strings.Contains(rec.Body.String(), `Cannot update token with scope "*"`) { + t.Fatalf("unexpected response: %q", rec.Body.String()) + } + if got := strings.Join(findAPITokenByID(t, cfg.APITokens, target.ID).Scopes, ","); got != config.ScopeWildcard { + t.Fatalf("target scopes changed after denied update: %q", got) + } + }) + + t.Run("caller cannot grant scope it does not hold", func(t *testing.T) { + target := newTokenRecord(t, "update-target-123456.12345678", []string{config.ScopeMonitoringRead}, nil) + caller := newTokenRecord(t, "update-caller-123456.12345678", []string{ + config.ScopeSettingsWrite, + config.ScopeMonitoringRead, + }, nil) + cfg := newTestConfigWithTokens(t, target, caller) + router := &Router{config: cfg} + + req := httptest.NewRequest( + http.MethodPatch, + "/api/security/tokens/"+target.ID, + strings.NewReader(`{"scopes":["monitoring:read","monitoring:write"]}`), + ) + req = req.WithContext(authpkg.WithAPIToken(req.Context(), &caller)) + rec := httptest.NewRecorder() + router.handleUpdateAPIToken(rec, req) + + if rec.Code != http.StatusForbidden { + t.Fatalf("status = %d, want %d (body=%q)", rec.Code, http.StatusForbidden, rec.Body.String()) + } + if !strings.Contains(rec.Body.String(), `Cannot grant scope "monitoring:write"`) { + t.Fatalf("unexpected response: %q", rec.Body.String()) + } + if got := strings.Join(findAPITokenByID(t, cfg.APITokens, target.ID).Scopes, ","); got != config.ScopeMonitoringRead { + t.Fatalf("target scopes changed after denied update: %q", got) + } + }) + + t.Run("caller can narrow itself", func(t *testing.T) { + target := newTokenRecord(t, "update-self-token-123.12345678", []string{ + config.ScopeSettingsWrite, + config.ScopeMonitoringRead, + }, nil) + cfg := newTestConfigWithTokens(t, target) + router := &Router{config: cfg} + + req := httptest.NewRequest( + http.MethodPatch, + "/api/security/tokens/"+target.ID, + strings.NewReader(`{"scopes":["settings:write"]}`), + ) + req = req.WithContext(authpkg.WithAPIToken(req.Context(), &target)) + rec := httptest.NewRecorder() + router.handleUpdateAPIToken(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d (body=%q)", rec.Code, http.StatusOK, rec.Body.String()) + } + if got := strings.Join(cfg.APITokens[0].Scopes, ","); got != config.ScopeSettingsWrite { + t.Fatalf("self-narrowed scopes = %q, want %q", got, config.ScopeSettingsWrite) + } + }) +} + +func TestSecurityTokens_UpdateScopesRollsBackWhenPersistenceFails(t *testing.T) { + record := newTokenRecord(t, "update-rollback-token-123.12345678", []string{config.ScopeSettingsRead}, nil) + cfg := newTestConfigWithTokens(t, record) + + stateDir := filepath.Join(t.TempDir(), "state") + persistence := config.NewConfigPersistence(stateDir) + if err := os.RemoveAll(stateDir); err != nil { + t.Fatalf("remove persistence directory: %v", err) + } + if err := os.WriteFile(stateDir, []byte("not a directory"), 0o600); err != nil { + t.Fatalf("create persistence blocker: %v", err) + } + router := &Router{config: cfg, persistence: persistence} + + req := httptest.NewRequest( + http.MethodPatch, + "/api/security/tokens/"+record.ID, + strings.NewReader(`{"scopes":["monitoring:read"]}`), + ) + rec := httptest.NewRecorder() + router.handleUpdateAPIToken(rec, req) + + if rec.Code != http.StatusInternalServerError { + t.Fatalf("status = %d, want %d (body=%q)", rec.Code, http.StatusInternalServerError, rec.Body.String()) + } + if got := strings.Join(cfg.APITokens[0].Scopes, ","); got != config.ScopeSettingsRead { + t.Fatalf("scopes after failed persistence = %q, want rollback to %q", got, config.ScopeSettingsRead) + } +} + +func findAPITokenByID(t *testing.T, tokens []config.APITokenRecord, tokenID string) config.APITokenRecord { + t.Helper() + for _, token := range tokens { + if token.ID == tokenID { + return token + } + } + t.Fatalf("token %q not found", tokenID) + return config.APITokenRecord{} +}