From 0e06f2b38baf4a7a7550b07bf49aa85b15454540 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Thu, 1 Oct 2026 08:23:23 +0100 Subject: [PATCH 1/3] Make administration guides safe and executable Keep RBAC, audit and organisation tokens out of process arguments. Use signed-in organisation mutations, actual schemas and acceptance rules, a custom role ID, and private audit exports. Exercise copied commands and handler lifecycles without changing runtime authority. Contract-Neutral: Documentation and documentation tests only; authentication, RBAC, tenant isolation, licensing and API runtime contracts are unchanged. Change-source: pulse-maintainer --- docs/API.md | 4 +- docs/AUDIT_LOGGING.md | 42 +++-- docs/MULTI_TENANT.md | 126 ++++++++----- docs/RBAC.md | 89 +++++++--- frontend-modern/public/docs/API.md | 4 +- frontend-modern/public/docs/AUDIT_LOGGING.md | 42 +++-- frontend-modern/public/docs/MULTI_TENANT.md | 126 ++++++++----- frontend-modern/public/docs/RBAC.md | 89 +++++++--- internal/api/admin_docs_test.go | 175 +++++++++++++++++++ scripts/tests/test_admin_docs.py | 162 +++++++++++++++++ scripts/tests/test_api_auth_docs.py | 86 ++++----- 11 files changed, 731 insertions(+), 214 deletions(-) create mode 100644 internal/api/admin_docs_test.go create mode 100644 scripts/tests/test_admin_docs.py diff --git a/docs/API.md b/docs/API.md index 1b6696de7..285d945cc 100644 --- a/docs/API.md +++ b/docs/API.md @@ -1204,7 +1204,7 @@ Returns all members with their roles. User must be a member of the org. ```json { "userId": "jane", "role": "editor" } ``` -Roles: `owner`, `admin`, `editor`, `viewer`. Admin or owner role required. Setting role to `owner` transfers ownership (only current owner can do this). Default org members cannot be managed. +Roles: `owner`, `admin`, `editor`, `viewer`. Admin or owner role required. A new user receives a pending invitation (`202`) and must accept it in Pulse before gaining membership. Posting an existing member's `userId` updates their role; there is no member PATCH endpoint. Setting role to `owner` transfers ownership only to an existing member, by the current owner after fresh sign-in. Default org members cannot be managed. ### Remove Member `DELETE /api/orgs/{id}/members/{userId}` (requires `settings:write`, session auth only) @@ -1229,7 +1229,7 @@ Returns resources shared inbound to this organization from other organizations. "accessRole": "viewer" } ``` -Share a resource with another organization. Valid resource types: `vm`, `container`, `agent`, `storage`, `pbs`, `pmg`. Access roles: `viewer`, `editor`, `admin`. Admin or owner role required on the source org. +Share a resource with another organization. Valid resource types: `vm`, `container`, `agent`, `storage`, `pbs`, `pmg` (not `host`). Access roles: `viewer`, `editor`, `admin`. Admin or owner role required on the source org. The share is pending until a target-org admin or owner accepts it in Pulse; changing its access role requires acceptance again. ### Delete Share `DELETE /api/orgs/{id}/shares/{shareId}` (requires `settings:write`, session auth only) diff --git a/docs/AUDIT_LOGGING.md b/docs/AUDIT_LOGGING.md index b6d397f85..403e91800 100644 --- a/docs/AUDIT_LOGGING.md +++ b/docs/AUDIT_LOGGING.md @@ -55,18 +55,31 @@ The audit log panel shows events in reverse chronological order with filtering b ### API +Use a token with `audit:read`, bound to a user permitted to read audit logs, +and the licensed `audit_logging` capability. Prepare the private header file +in [API authentication](API.md#-authentication); never paste the token or a +session cookie into a command. For a one-off read, you can instead open the +API path in your signed-in Pulse browser. + +The loopback URLs below apply on the Pulse host. For remote access, use your +Pulse HTTPS URL with certificate verification enabled. Use curl 7.76 or later, +keeping `--disable` first to ignore local trace/verbose defaults. +`--fail-with-body` returns a non-zero exit on HTTP failures, including 401, +402 and 403. Share only the relevant redacted error, not the header file or +whole audit response: events can contain usernames, client addresses and paths. + ```bash # List recent events -curl http://localhost:7655/api/audit?limit=50 \ - -H "Authorization: Bearer $TOKEN" +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + 'http://127.0.0.1:7655/api/audit?limit=50' # Filter by event type and date range -curl "http://localhost:7655/api/audit?event=login&startTime=2026-01-01T00:00:00Z&endTime=2026-01-31T23:59:59Z&success=false" \ - -H "Authorization: Bearer $TOKEN" +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + 'http://127.0.0.1:7655/api/audit?event=login&startTime=2026-01-01T00:00:00Z&endTime=2026-01-31T23:59:59Z&success=false' # Get audit summary -curl http://localhost:7655/api/audit/summary \ - -H "Authorization: Bearer $TOKEN" +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + http://127.0.0.1:7655/api/audit/summary ``` ### Query Parameters @@ -87,12 +100,17 @@ curl http://localhost:7655/api/audit/summary \ Export the audit log for external analysis or compliance archival: ```bash -curl http://localhost:7655/api/audit/export \ - -H "Authorization: Bearer $TOKEN" \ - -o audit-export.json +umask 077 +export_dir="$(mktemp -d)" && +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + --output "$export_dir/audit-export.json" \ + http://127.0.0.1:7655/api/audit/export && +printf 'Saved private export to %s\n' "$export_dir/audit-export.json" ``` -The export includes all events matching the current filter criteria. +This creates a new private directory and prints the file's location only on +success. It does not reuse filters selected in the UI. Treat the export as +sensitive data; keep it outside shared repositories and issue attachments. --- @@ -101,8 +119,8 @@ The export includes all events matching the current filter criteria. Every audit event is cryptographically signed at creation time. You can verify that an event has not been modified: ```bash -curl http://localhost:7655/api/audit/6b3c9c3c-9a2f-4b3c-9a3b-3d0e8c5c5d45/verify \ - -H "Authorization: Bearer $TOKEN" +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + http://127.0.0.1:7655/api/audit/6b3c9c3c-9a2f-4b3c-9a3b-3d0e8c5c5d45/verify ``` Response: diff --git a/docs/MULTI_TENANT.md b/docs/MULTI_TENANT.md index 6213f7c39..88276d41f 100644 --- a/docs/MULTI_TENANT.md +++ b/docs/MULTI_TENANT.md @@ -51,9 +51,30 @@ Each member has a role within an organization: Organizations can share specific resources with other organizations: -- Share a VM, container, host, or storage resource with another org. +- Share a VM, container, machine agent, storage, PBS, or PMG resource with another org. - Assign an access role (`viewer`, `editor`, or `admin`) to the share. -- The receiving org sees shared resources alongside their own, with a share badge. +- The receiving org must accept the share before it sees the resource alongside its own. + +## Before Using the API + +Create organizations and manage members and shares in the signed-in Pulse UI. +These changes require **session-based user authentication** and the relevant +organization role; API tokens are rejected with `403 session_required`, even +when they have `settings:write`. The UI handles the session and CSRF protection. +Do not extract a session cookie or CSRF token into a shell command. + +The read-only curl examples use an org-bound token with `settings:read` and +the private header file from [API authentication](API.md#-authentication). +Keep the token out of command lines, URLs and reports. A token's organization +binding is an access boundary: changing a URL or `X-Pulse-Org-ID` header does +not grant it access to another organization. + +Use curl 7.76 or later. Keep `--disable` first to ignore local trace/verbose +defaults; `--fail-with-body` makes HTTP failures return a non-zero exit. The +loopback URLs apply on the Pulse host; remotely, use your Pulse HTTPS URL and +keep certificate verification enabled. Replace the example organization ID +`production-datacenter` with your own. Run requests separately, and share only +the relevant redacted error, not whole member or infrastructure responses. ## Managing Organizations @@ -61,14 +82,15 @@ Organizations can share specific resources with other organizations: **UI:** Settings → Organization → Create Organization -**API:** -```bash -curl -X POST http://localhost:7655/api/orgs \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d '{"name": "Production Datacenter", "description": "EU production infrastructure"}' +**API contract:** `POST /api/orgs`, session authentication only: +```json +{"id": "production-datacenter", "displayName": "Production Datacenter"} ``` +The creator becomes the owner. `id` is a lowercase alphanumeric/hyphen ID +(3–64 characters); `displayName` is the name shown in Pulse. `name` and +`description` are not the creation fields. + ### Switching Organizations Use the **Org Switcher** dropdown in the header. When you switch: @@ -81,45 +103,57 @@ Use the **Org Switcher** dropdown in the header. When you switch: **UI:** Settings → Organization → Access -**API:** +**Read-only API:** ```bash -# List members -curl http://localhost:7655/api/orgs/{orgId}/members \ - -H "Authorization: Bearer $TOKEN" - -# Add a member -curl -X POST http://localhost:7655/api/orgs/{orgId}/members \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d '{"userId": "user-id", "role": "editor"}' - -# Update role -curl -X PATCH http://localhost:7655/api/orgs/{orgId}/members/{userId} \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d '{"role": "admin"}' +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + http://127.0.0.1:7655/api/orgs/production-datacenter/members ``` +**Invite or update a member:** use the Access panel as an owner or admin. +Its API contract is `POST /api/orgs/{id}/members`, session authentication only: +```json +{"userId": "user-id", "role": "editor"} +``` + +For a new member, the response is `202` with a pending invitation; the user +must accept it in Pulse before gaining access. An existing member's role is +updated by posting their `userId` and new role to the same endpoint, not by +PATCHing a member URL: +```json +{"userId": "user-id", "role": "admin"} +``` + +Only the current owner can transfer ownership, and only to an existing member +after fresh sign-in. The owner cannot be demoted or removed as an ordinary +member update. Default-organization members cannot be managed here. + ### Sharing Resources **UI:** Settings → Organization → Sharing -**API:** -```bash -# Create a share -curl -X POST http://localhost:7655/api/orgs/{orgId}/shares \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d '{ - "targetOrgId": "other-org-id", - "resourceType": "host", - "resourceId": "resource-id", - "role": "viewer" - }' +Create the share as an owner or admin of the source organization. An owner or +admin of the target organization then accepts it in **Sharing → Incoming**. +Until acceptance, the share is pending and does not grant access. -# View incoming shares -curl http://localhost:7655/api/orgs/{orgId}/shares/incoming \ - -H "Authorization: Bearer $TOKEN" +**API contract:** `POST /api/orgs/{id}/shares`, session authentication only: +```json +{ + "targetOrgId": "other-org-id", + "resourceType": "vm", + "resourceId": "vm:101", + "accessRole": "viewer" +} +``` + +Use `accessRole`, not `role`. Supported resource types are `vm`, `container`, +`agent`, `storage`, `pbs` and `pmg`; `host` is not supported. Use the resource ID +returned by Pulse, not a display name. Changing a share's access role makes it +pending again, so the target must accept the new grant. + +**Read-only incoming shares:** +```bash +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + http://127.0.0.1:7655/api/orgs/production-datacenter/shares/incoming ``` ## Monitoring Multiple Internal Estates @@ -158,15 +192,16 @@ When multi-tenant is enabled, **Settings → Organization** shows: | `GET` | `/api/orgs` | List organizations the current user can access | | `POST` | `/api/orgs` | Create a new organization | | `GET` | `/api/orgs/{id}` | Get organization details | -| `PATCH` | `/api/orgs/{id}` | Update organization | +| `PUT` | `/api/orgs/{id}` | Update organization (session only) | | `DELETE` | `/api/orgs/{id}` | Delete organization | | `GET` | `/api/orgs/{id}/members` | List members | -| `POST` | `/api/orgs/{id}/members` | Add a member | -| `PATCH` | `/api/orgs/{id}/members/{userId}` | Update member role | +| `POST` | `/api/orgs/{id}/members` | Invite a member or update an existing member's role (session only) | | `DELETE` | `/api/orgs/{id}/members/{userId}` | Remove a member | +| `POST` | `/api/org-invitations/{id}/accept` | Accept your pending invitation (session only) | | `GET` | `/api/orgs/{id}/shares` | List outgoing shares | | `GET` | `/api/orgs/{id}/shares/incoming` | List incoming shares | -| `POST` | `/api/orgs/{id}/shares` | Create a share | +| `POST` | `/api/orgs/{id}/shares` | Create or update a share (session only) | +| `POST` | `/api/orgs/{id}/shares/incoming/{shareId}/accept` | Accept an incoming share (session only) | | `DELETE` | `/api/orgs/{id}/shares/{shareId}` | Remove a share | ### Tenant Context @@ -177,6 +212,9 @@ All data-fetching endpoints respect the active organization context. The active 2. Session cookie (browser) 3. Falls back to the `default` organization +Neither the active context nor a token scope overrides organization membership +or a token's organization binding. + ## Storage - The **default** org uses the root data directory (backward compatible). @@ -202,7 +240,7 @@ Activate an Enterprise license with the `multi_tenant` capability in **Settings ### Shared resources not appearing -1. Verify the share exists: **Settings → Organization → Sharing → Incoming**. +1. Verify the share exists and has been accepted: **Settings → Organization → Sharing → Incoming**. 2. Confirm the share role grants sufficient access. 3. Check that the source org's resources are online. diff --git a/docs/RBAC.md b/docs/RBAC.md index 3162eaa43..7b0f1d69c 100644 --- a/docs/RBAC.md +++ b/docs/RBAC.md @@ -46,42 +46,65 @@ user merely because no local administrator is configured. ## Managing Roles +### Before Using the API + +Prefer **Settings → Security → Access Control** for one-off administration. +The API examples require the licensed `rbac` capability and permission to +administer users. With RBAC enabled, role and user administration requires a +full-access (`*`) API token bound to an authorised administrator; a monitoring +token is not enough. Do not widen an agent's token for this job. + +If you already use an administration token, prepare its private header file +as described in [API authentication](API.md#-authentication). The examples read +`$HOME/.config/pulse/api-header`; never paste the token or a session cookie into +a command. Keep this file outside shared repositories and diagnostics. + +The loopback URLs work on the Pulse host. For remote use, substitute your +Pulse HTTPS URL and keep certificate verification enabled. Use curl 7.76 or +later; `--disable` must remain first to ignore local trace/verbose defaults, +and `--fail-with-body` makes HTTP failures return a non-zero exit. Run each +change separately and check its response before continuing. + ### Creating a Role **UI:** Settings → Security → Access Control → Create Role +Use a new custom role ID. Built-in roles (`admin`, `operator`, `viewer`, +`auditor`) cannot be modified or deleted. These examples use `alert-manager`. + **API:** ```bash -curl -X POST http://localhost:7655/api/admin/roles \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d '{ - "id": "operator", - "name": "Operator", +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + --header 'Content-Type: application/json' --request POST \ + --data-binary @- http://127.0.0.1:7655/api/admin/roles <<'JSON' + { + "id": "alert-manager", + "name": "Alert Manager", "description": "Can view and manage alerts", "permissions": [ {"action": "read", "resource": "alerts"}, {"action": "write", "resource": "alerts"}, {"action": "read", "resource": "nodes"} ] - }' + } +JSON ``` ### Listing Roles ```bash -curl http://localhost:7655/api/admin/roles \ - -H "Authorization: Bearer $TOKEN" +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + http://127.0.0.1:7655/api/admin/roles ``` ### Updating a Role ```bash -curl -X PUT http://localhost:7655/api/admin/roles/operator \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d '{ - "name": "Operator", +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + --header 'Content-Type: application/json' --request PUT \ + --data-binary @- http://127.0.0.1:7655/api/admin/roles/alert-manager <<'JSON' + { + "name": "Alert Manager", "description": "Updated description", "permissions": [ {"action": "read", "resource": "alerts"}, @@ -89,14 +112,15 @@ curl -X PUT http://localhost:7655/api/admin/roles/operator \ {"action": "read", "resource": "nodes"}, {"action": "read", "resource": "ai"} ] - }' + } +JSON ``` ### Deleting a Role ```bash -curl -X DELETE http://localhost:7655/api/admin/roles/operator \ - -H "Authorization: Bearer $TOKEN" +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + --request DELETE http://127.0.0.1:7655/api/admin/roles/alert-manager ``` --- @@ -106,8 +130,8 @@ curl -X DELETE http://localhost:7655/api/admin/roles/operator \ ### Listing Users and Their Roles ```bash -curl http://localhost:7655/api/admin/users \ - -H "Authorization: Bearer $TOKEN" +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + http://127.0.0.1:7655/api/admin/users ``` SSO users are displayed using the latest configured username claim and email @@ -118,20 +142,27 @@ principal used for authorization. Role assignments are set as a complete list — the user's roles are replaced with the provided set: +Use the stable `username` returned by the user list, not the display name or +email. URL-encode it as one path segment (for example, `:` becomes `%3A` for an +SSO principal). `jane` below is an example local username. Create the custom +role before assigning it; do not run the deletion example first. + ```bash -curl -X PUT http://localhost:7655/api/admin/users/jane/roles \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d '{"roleIds": ["operator", "viewer"]}' +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + --header 'Content-Type: application/json' --request PUT \ + --data-binary @- http://127.0.0.1:7655/api/admin/users/jane/roles <<'JSON' +{"roleIds": ["alert-manager", "viewer"]} +JSON ``` To remove all custom roles from a user, send an empty list: ```bash -curl -X PUT http://localhost:7655/api/admin/users/jane/roles \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d '{"roleIds": []}' +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + --header 'Content-Type: application/json' --request PUT \ + --data-binary @- http://127.0.0.1:7655/api/admin/users/jane/roles <<'JSON' +{"roleIds": []} +JSON ``` Note: Users cannot modify their own role assignments (self-escalation prevention). @@ -139,8 +170,8 @@ Note: Users cannot modify their own role assignments (self-escalation prevention ### Removing User Access ```bash -curl -X DELETE http://localhost:7655/api/admin/users/jane \ - -H "Authorization: Bearer $TOKEN" +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + --request DELETE http://127.0.0.1:7655/api/admin/users/jane ``` This removes the Pulse identity and all role assignments and revokes its active diff --git a/frontend-modern/public/docs/API.md b/frontend-modern/public/docs/API.md index 1b6696de7..285d945cc 100644 --- a/frontend-modern/public/docs/API.md +++ b/frontend-modern/public/docs/API.md @@ -1204,7 +1204,7 @@ Returns all members with their roles. User must be a member of the org. ```json { "userId": "jane", "role": "editor" } ``` -Roles: `owner`, `admin`, `editor`, `viewer`. Admin or owner role required. Setting role to `owner` transfers ownership (only current owner can do this). Default org members cannot be managed. +Roles: `owner`, `admin`, `editor`, `viewer`. Admin or owner role required. A new user receives a pending invitation (`202`) and must accept it in Pulse before gaining membership. Posting an existing member's `userId` updates their role; there is no member PATCH endpoint. Setting role to `owner` transfers ownership only to an existing member, by the current owner after fresh sign-in. Default org members cannot be managed. ### Remove Member `DELETE /api/orgs/{id}/members/{userId}` (requires `settings:write`, session auth only) @@ -1229,7 +1229,7 @@ Returns resources shared inbound to this organization from other organizations. "accessRole": "viewer" } ``` -Share a resource with another organization. Valid resource types: `vm`, `container`, `agent`, `storage`, `pbs`, `pmg`. Access roles: `viewer`, `editor`, `admin`. Admin or owner role required on the source org. +Share a resource with another organization. Valid resource types: `vm`, `container`, `agent`, `storage`, `pbs`, `pmg` (not `host`). Access roles: `viewer`, `editor`, `admin`. Admin or owner role required on the source org. The share is pending until a target-org admin or owner accepts it in Pulse; changing its access role requires acceptance again. ### Delete Share `DELETE /api/orgs/{id}/shares/{shareId}` (requires `settings:write`, session auth only) diff --git a/frontend-modern/public/docs/AUDIT_LOGGING.md b/frontend-modern/public/docs/AUDIT_LOGGING.md index b6d397f85..403e91800 100644 --- a/frontend-modern/public/docs/AUDIT_LOGGING.md +++ b/frontend-modern/public/docs/AUDIT_LOGGING.md @@ -55,18 +55,31 @@ The audit log panel shows events in reverse chronological order with filtering b ### API +Use a token with `audit:read`, bound to a user permitted to read audit logs, +and the licensed `audit_logging` capability. Prepare the private header file +in [API authentication](API.md#-authentication); never paste the token or a +session cookie into a command. For a one-off read, you can instead open the +API path in your signed-in Pulse browser. + +The loopback URLs below apply on the Pulse host. For remote access, use your +Pulse HTTPS URL with certificate verification enabled. Use curl 7.76 or later, +keeping `--disable` first to ignore local trace/verbose defaults. +`--fail-with-body` returns a non-zero exit on HTTP failures, including 401, +402 and 403. Share only the relevant redacted error, not the header file or +whole audit response: events can contain usernames, client addresses and paths. + ```bash # List recent events -curl http://localhost:7655/api/audit?limit=50 \ - -H "Authorization: Bearer $TOKEN" +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + 'http://127.0.0.1:7655/api/audit?limit=50' # Filter by event type and date range -curl "http://localhost:7655/api/audit?event=login&startTime=2026-01-01T00:00:00Z&endTime=2026-01-31T23:59:59Z&success=false" \ - -H "Authorization: Bearer $TOKEN" +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + 'http://127.0.0.1:7655/api/audit?event=login&startTime=2026-01-01T00:00:00Z&endTime=2026-01-31T23:59:59Z&success=false' # Get audit summary -curl http://localhost:7655/api/audit/summary \ - -H "Authorization: Bearer $TOKEN" +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + http://127.0.0.1:7655/api/audit/summary ``` ### Query Parameters @@ -87,12 +100,17 @@ curl http://localhost:7655/api/audit/summary \ Export the audit log for external analysis or compliance archival: ```bash -curl http://localhost:7655/api/audit/export \ - -H "Authorization: Bearer $TOKEN" \ - -o audit-export.json +umask 077 +export_dir="$(mktemp -d)" && +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + --output "$export_dir/audit-export.json" \ + http://127.0.0.1:7655/api/audit/export && +printf 'Saved private export to %s\n' "$export_dir/audit-export.json" ``` -The export includes all events matching the current filter criteria. +This creates a new private directory and prints the file's location only on +success. It does not reuse filters selected in the UI. Treat the export as +sensitive data; keep it outside shared repositories and issue attachments. --- @@ -101,8 +119,8 @@ The export includes all events matching the current filter criteria. Every audit event is cryptographically signed at creation time. You can verify that an event has not been modified: ```bash -curl http://localhost:7655/api/audit/6b3c9c3c-9a2f-4b3c-9a3b-3d0e8c5c5d45/verify \ - -H "Authorization: Bearer $TOKEN" +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + http://127.0.0.1:7655/api/audit/6b3c9c3c-9a2f-4b3c-9a3b-3d0e8c5c5d45/verify ``` Response: diff --git a/frontend-modern/public/docs/MULTI_TENANT.md b/frontend-modern/public/docs/MULTI_TENANT.md index 6213f7c39..88276d41f 100644 --- a/frontend-modern/public/docs/MULTI_TENANT.md +++ b/frontend-modern/public/docs/MULTI_TENANT.md @@ -51,9 +51,30 @@ Each member has a role within an organization: Organizations can share specific resources with other organizations: -- Share a VM, container, host, or storage resource with another org. +- Share a VM, container, machine agent, storage, PBS, or PMG resource with another org. - Assign an access role (`viewer`, `editor`, or `admin`) to the share. -- The receiving org sees shared resources alongside their own, with a share badge. +- The receiving org must accept the share before it sees the resource alongside its own. + +## Before Using the API + +Create organizations and manage members and shares in the signed-in Pulse UI. +These changes require **session-based user authentication** and the relevant +organization role; API tokens are rejected with `403 session_required`, even +when they have `settings:write`. The UI handles the session and CSRF protection. +Do not extract a session cookie or CSRF token into a shell command. + +The read-only curl examples use an org-bound token with `settings:read` and +the private header file from [API authentication](API.md#-authentication). +Keep the token out of command lines, URLs and reports. A token's organization +binding is an access boundary: changing a URL or `X-Pulse-Org-ID` header does +not grant it access to another organization. + +Use curl 7.76 or later. Keep `--disable` first to ignore local trace/verbose +defaults; `--fail-with-body` makes HTTP failures return a non-zero exit. The +loopback URLs apply on the Pulse host; remotely, use your Pulse HTTPS URL and +keep certificate verification enabled. Replace the example organization ID +`production-datacenter` with your own. Run requests separately, and share only +the relevant redacted error, not whole member or infrastructure responses. ## Managing Organizations @@ -61,14 +82,15 @@ Organizations can share specific resources with other organizations: **UI:** Settings → Organization → Create Organization -**API:** -```bash -curl -X POST http://localhost:7655/api/orgs \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d '{"name": "Production Datacenter", "description": "EU production infrastructure"}' +**API contract:** `POST /api/orgs`, session authentication only: +```json +{"id": "production-datacenter", "displayName": "Production Datacenter"} ``` +The creator becomes the owner. `id` is a lowercase alphanumeric/hyphen ID +(3–64 characters); `displayName` is the name shown in Pulse. `name` and +`description` are not the creation fields. + ### Switching Organizations Use the **Org Switcher** dropdown in the header. When you switch: @@ -81,45 +103,57 @@ Use the **Org Switcher** dropdown in the header. When you switch: **UI:** Settings → Organization → Access -**API:** +**Read-only API:** ```bash -# List members -curl http://localhost:7655/api/orgs/{orgId}/members \ - -H "Authorization: Bearer $TOKEN" - -# Add a member -curl -X POST http://localhost:7655/api/orgs/{orgId}/members \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d '{"userId": "user-id", "role": "editor"}' - -# Update role -curl -X PATCH http://localhost:7655/api/orgs/{orgId}/members/{userId} \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d '{"role": "admin"}' +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + http://127.0.0.1:7655/api/orgs/production-datacenter/members ``` +**Invite or update a member:** use the Access panel as an owner or admin. +Its API contract is `POST /api/orgs/{id}/members`, session authentication only: +```json +{"userId": "user-id", "role": "editor"} +``` + +For a new member, the response is `202` with a pending invitation; the user +must accept it in Pulse before gaining access. An existing member's role is +updated by posting their `userId` and new role to the same endpoint, not by +PATCHing a member URL: +```json +{"userId": "user-id", "role": "admin"} +``` + +Only the current owner can transfer ownership, and only to an existing member +after fresh sign-in. The owner cannot be demoted or removed as an ordinary +member update. Default-organization members cannot be managed here. + ### Sharing Resources **UI:** Settings → Organization → Sharing -**API:** -```bash -# Create a share -curl -X POST http://localhost:7655/api/orgs/{orgId}/shares \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d '{ - "targetOrgId": "other-org-id", - "resourceType": "host", - "resourceId": "resource-id", - "role": "viewer" - }' +Create the share as an owner or admin of the source organization. An owner or +admin of the target organization then accepts it in **Sharing → Incoming**. +Until acceptance, the share is pending and does not grant access. -# View incoming shares -curl http://localhost:7655/api/orgs/{orgId}/shares/incoming \ - -H "Authorization: Bearer $TOKEN" +**API contract:** `POST /api/orgs/{id}/shares`, session authentication only: +```json +{ + "targetOrgId": "other-org-id", + "resourceType": "vm", + "resourceId": "vm:101", + "accessRole": "viewer" +} +``` + +Use `accessRole`, not `role`. Supported resource types are `vm`, `container`, +`agent`, `storage`, `pbs` and `pmg`; `host` is not supported. Use the resource ID +returned by Pulse, not a display name. Changing a share's access role makes it +pending again, so the target must accept the new grant. + +**Read-only incoming shares:** +```bash +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + http://127.0.0.1:7655/api/orgs/production-datacenter/shares/incoming ``` ## Monitoring Multiple Internal Estates @@ -158,15 +192,16 @@ When multi-tenant is enabled, **Settings → Organization** shows: | `GET` | `/api/orgs` | List organizations the current user can access | | `POST` | `/api/orgs` | Create a new organization | | `GET` | `/api/orgs/{id}` | Get organization details | -| `PATCH` | `/api/orgs/{id}` | Update organization | +| `PUT` | `/api/orgs/{id}` | Update organization (session only) | | `DELETE` | `/api/orgs/{id}` | Delete organization | | `GET` | `/api/orgs/{id}/members` | List members | -| `POST` | `/api/orgs/{id}/members` | Add a member | -| `PATCH` | `/api/orgs/{id}/members/{userId}` | Update member role | +| `POST` | `/api/orgs/{id}/members` | Invite a member or update an existing member's role (session only) | | `DELETE` | `/api/orgs/{id}/members/{userId}` | Remove a member | +| `POST` | `/api/org-invitations/{id}/accept` | Accept your pending invitation (session only) | | `GET` | `/api/orgs/{id}/shares` | List outgoing shares | | `GET` | `/api/orgs/{id}/shares/incoming` | List incoming shares | -| `POST` | `/api/orgs/{id}/shares` | Create a share | +| `POST` | `/api/orgs/{id}/shares` | Create or update a share (session only) | +| `POST` | `/api/orgs/{id}/shares/incoming/{shareId}/accept` | Accept an incoming share (session only) | | `DELETE` | `/api/orgs/{id}/shares/{shareId}` | Remove a share | ### Tenant Context @@ -177,6 +212,9 @@ All data-fetching endpoints respect the active organization context. The active 2. Session cookie (browser) 3. Falls back to the `default` organization +Neither the active context nor a token scope overrides organization membership +or a token's organization binding. + ## Storage - The **default** org uses the root data directory (backward compatible). @@ -202,7 +240,7 @@ Activate an Enterprise license with the `multi_tenant` capability in **Settings ### Shared resources not appearing -1. Verify the share exists: **Settings → Organization → Sharing → Incoming**. +1. Verify the share exists and has been accepted: **Settings → Organization → Sharing → Incoming**. 2. Confirm the share role grants sufficient access. 3. Check that the source org's resources are online. diff --git a/frontend-modern/public/docs/RBAC.md b/frontend-modern/public/docs/RBAC.md index 3162eaa43..7b0f1d69c 100644 --- a/frontend-modern/public/docs/RBAC.md +++ b/frontend-modern/public/docs/RBAC.md @@ -46,42 +46,65 @@ user merely because no local administrator is configured. ## Managing Roles +### Before Using the API + +Prefer **Settings → Security → Access Control** for one-off administration. +The API examples require the licensed `rbac` capability and permission to +administer users. With RBAC enabled, role and user administration requires a +full-access (`*`) API token bound to an authorised administrator; a monitoring +token is not enough. Do not widen an agent's token for this job. + +If you already use an administration token, prepare its private header file +as described in [API authentication](API.md#-authentication). The examples read +`$HOME/.config/pulse/api-header`; never paste the token or a session cookie into +a command. Keep this file outside shared repositories and diagnostics. + +The loopback URLs work on the Pulse host. For remote use, substitute your +Pulse HTTPS URL and keep certificate verification enabled. Use curl 7.76 or +later; `--disable` must remain first to ignore local trace/verbose defaults, +and `--fail-with-body` makes HTTP failures return a non-zero exit. Run each +change separately and check its response before continuing. + ### Creating a Role **UI:** Settings → Security → Access Control → Create Role +Use a new custom role ID. Built-in roles (`admin`, `operator`, `viewer`, +`auditor`) cannot be modified or deleted. These examples use `alert-manager`. + **API:** ```bash -curl -X POST http://localhost:7655/api/admin/roles \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d '{ - "id": "operator", - "name": "Operator", +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + --header 'Content-Type: application/json' --request POST \ + --data-binary @- http://127.0.0.1:7655/api/admin/roles <<'JSON' + { + "id": "alert-manager", + "name": "Alert Manager", "description": "Can view and manage alerts", "permissions": [ {"action": "read", "resource": "alerts"}, {"action": "write", "resource": "alerts"}, {"action": "read", "resource": "nodes"} ] - }' + } +JSON ``` ### Listing Roles ```bash -curl http://localhost:7655/api/admin/roles \ - -H "Authorization: Bearer $TOKEN" +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + http://127.0.0.1:7655/api/admin/roles ``` ### Updating a Role ```bash -curl -X PUT http://localhost:7655/api/admin/roles/operator \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d '{ - "name": "Operator", +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + --header 'Content-Type: application/json' --request PUT \ + --data-binary @- http://127.0.0.1:7655/api/admin/roles/alert-manager <<'JSON' + { + "name": "Alert Manager", "description": "Updated description", "permissions": [ {"action": "read", "resource": "alerts"}, @@ -89,14 +112,15 @@ curl -X PUT http://localhost:7655/api/admin/roles/operator \ {"action": "read", "resource": "nodes"}, {"action": "read", "resource": "ai"} ] - }' + } +JSON ``` ### Deleting a Role ```bash -curl -X DELETE http://localhost:7655/api/admin/roles/operator \ - -H "Authorization: Bearer $TOKEN" +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + --request DELETE http://127.0.0.1:7655/api/admin/roles/alert-manager ``` --- @@ -106,8 +130,8 @@ curl -X DELETE http://localhost:7655/api/admin/roles/operator \ ### Listing Users and Their Roles ```bash -curl http://localhost:7655/api/admin/users \ - -H "Authorization: Bearer $TOKEN" +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + http://127.0.0.1:7655/api/admin/users ``` SSO users are displayed using the latest configured username claim and email @@ -118,20 +142,27 @@ principal used for authorization. Role assignments are set as a complete list — the user's roles are replaced with the provided set: +Use the stable `username` returned by the user list, not the display name or +email. URL-encode it as one path segment (for example, `:` becomes `%3A` for an +SSO principal). `jane` below is an example local username. Create the custom +role before assigning it; do not run the deletion example first. + ```bash -curl -X PUT http://localhost:7655/api/admin/users/jane/roles \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d '{"roleIds": ["operator", "viewer"]}' +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + --header 'Content-Type: application/json' --request PUT \ + --data-binary @- http://127.0.0.1:7655/api/admin/users/jane/roles <<'JSON' +{"roleIds": ["alert-manager", "viewer"]} +JSON ``` To remove all custom roles from a user, send an empty list: ```bash -curl -X PUT http://localhost:7655/api/admin/users/jane/roles \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d '{"roleIds": []}' +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + --header 'Content-Type: application/json' --request PUT \ + --data-binary @- http://127.0.0.1:7655/api/admin/users/jane/roles <<'JSON' +{"roleIds": []} +JSON ``` Note: Users cannot modify their own role assignments (self-escalation prevention). @@ -139,8 +170,8 @@ Note: Users cannot modify their own role assignments (self-escalation prevention ### Removing User Access ```bash -curl -X DELETE http://localhost:7655/api/admin/users/jane \ - -H "Authorization: Bearer $TOKEN" +curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ + --request DELETE http://127.0.0.1:7655/api/admin/users/jane ``` This removes the Pulse identity and all role assignments and revokes its active diff --git a/internal/api/admin_docs_test.go b/internal/api/admin_docs_test.go new file mode 100644 index 000000000..15e2ed1a0 --- /dev/null +++ b/internal/api/admin_docs_test.go @@ -0,0 +1,175 @@ +package api + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "regexp" + "strings" + "testing" + + "github.com/rcourtman/pulse-go-rewrite/internal/config" + "github.com/rcourtman/pulse-go-rewrite/internal/models" + "github.com/rcourtman/pulse-go-rewrite/pkg/auth" +) + +// Read what the guide actually sends, rather than maintaining a second client. +// Authentication, licensing and CSRF are tested by the existing route tests; +// these documentation tests exercise the real handlers and persistent values. +func adminDocBodies(t *testing.T, name, heading string) [][]byte { + t.Helper() + contents, err := os.ReadFile(filepath.Join("..", "..", "docs", name+".md")) + if err != nil { + t.Fatal(err) + } + _, section, found := strings.Cut(string(contents), "### "+heading+"\n") + if !found { + t.Fatalf("missing %s heading %q", name, heading) + } + if next := strings.Index(section, "\n##"); next >= 0 { + section = section[:next] + } + // Both shell heredocs and API schema blocks contain standalone JSON. + re := regexp.MustCompile("(?s)(?:<<'JSON'\\n|```json\\n)(.*?)(?:\\nJSON|\\n```)") + var bodies [][]byte + for _, match := range re.FindAllStringSubmatch(section, -1) { + if !json.Valid([]byte(match[1])) { + t.Fatalf("invalid JSON in %s/%s", name, heading) + } + bodies = append(bodies, []byte(match[1])) + } + if len(bodies) == 0 { + t.Fatalf("missing request JSON in %s/%s", name, heading) + } + return bodies +} + +func TestAdminDocsCustomRoleLifecycle(t *testing.T) { + manager, err := auth.NewFileManager(t.TempDir()) + if err != nil { + t.Fatal(err) + } + previous := auth.GetManager() + auth.SetManager(manager) + t.Cleanup(func() { auth.SetManager(previous) }) + h := NewRBACHandlers(&config.Config{AuthUser: "alice"}) + + // Exact-base guide control: creating/updating the built-in operator fails. + legacy := httptest.NewRecorder() + h.HandleRoles(legacy, withUser(httptest.NewRequest(http.MethodPost, "/api/admin/roles", + strings.NewReader(`{"id":"operator","name":"Operator"}`)), "alice")) + if legacy.Code != http.StatusInternalServerError { + t.Fatalf("legacy built-in mutation was not rejected: %d", legacy.Code) + } + + assignments := adminDocBodies(t, "RBAC", "Setting Roles for a User") + if len(assignments) != 2 { + t.Fatalf("want assign and clear examples, got %d", len(assignments)) + } + steps := []struct { + method, path string + body []byte + handler http.HandlerFunc + status int + }{ + {http.MethodPost, "/api/admin/roles", adminDocBodies(t, "RBAC", "Creating a Role")[0], h.HandleRoles, http.StatusOK}, + {http.MethodPut, "/api/admin/roles/alert-manager", adminDocBodies(t, "RBAC", "Updating a Role")[0], h.HandleRoles, http.StatusOK}, + {http.MethodPut, "/api/admin/users/jane/roles", assignments[0], h.HandleUserRoleActions, http.StatusOK}, + {http.MethodPut, "/api/admin/users/jane/roles", assignments[1], h.HandleUserRoleActions, http.StatusOK}, + {http.MethodDelete, "/api/admin/users/jane", nil, h.HandleUserRoleActions, http.StatusNoContent}, + {http.MethodDelete, "/api/admin/roles/alert-manager", nil, h.HandleRoles, http.StatusNoContent}, + } + for _, step := range steps { + rec := httptest.NewRecorder() + step.handler(rec, withUser(httptest.NewRequest(step.method, step.path, bytes.NewReader(step.body)), "alice")) + if rec.Code != step.status { + t.Fatalf("documented %s %s: got %d: %s", step.method, step.path, rec.Code, rec.Body.String()) + } + if step.method == http.MethodPut && step.path == "/api/admin/roles/alert-manager" { + role, exists := manager.GetRole("alert-manager") + if !exists || role.IsBuiltIn || len(role.Permissions) != 4 { + t.Fatalf("custom role update did not persist its permissions: %+v", role) + } + } + if step.method == http.MethodPut && strings.HasSuffix(step.path, "/jane/roles") { + assignment, exists := manager.GetUserAssignment("jane") + var want struct { + RoleIDs []string `json:"roleIds"` + } + if err := json.Unmarshal(step.body, &want); err != nil { + t.Fatal(err) + } + if !exists || strings.Join(assignment.RoleIDs, ",") != strings.Join(want.RoleIDs, ",") { + t.Fatalf("assignment did not persist the complete role list: %+v", assignment) + } + } + } + if _, exists := manager.GetRole("alert-manager"); exists { + t.Fatal("custom role deletion did not persist") + } + if role, exists := manager.GetRole("operator"); !exists || !role.IsBuiltIn { + t.Fatal("examples altered the built-in role") + } +} + +func TestAdminDocsOrganizationSchemasAndAcceptance(t *testing.T) { + t.Setenv("PULSE_DEV", "true") + wasEnabled := IsMultiTenantEnabled() + SetMultiTenantEnabled(true) + t.Cleanup(func() { SetMultiTenantEnabled(wasEnabled) }) + persistence := config.NewMultiTenantPersistence(t.TempDir()) + h := NewOrgHandlers(persistence, nil) + call := func(method, path, user, orgID string, body []byte, handler http.HandlerFunc) *httptest.ResponseRecorder { + req := withUser(httptest.NewRequest(method, path, bytes.NewReader(body)), user) + req.SetPathValue("id", orgID) + rec := httptest.NewRecorder() + handler(rec, req) + return rec + } + check := func(rec *httptest.ResponseRecorder, status int) { + t.Helper() + if rec.Code != status { + t.Fatalf("documented request: got %d, want %d: %s", rec.Code, status, rec.Body.String()) + } + } + + // Exact-base guide controls: wrong creation fields and member PATCH fail. + check(call(http.MethodPost, "/api/orgs", "alice", "", []byte(`{"name":"Production Datacenter","description":"EU production infrastructure"}`), h.HandleCreateOrg), http.StatusBadRequest) + check(call(http.MethodPatch, "/api/orgs/production-datacenter/members/user-id", "alice", "production-datacenter", []byte(`{"role":"admin"}`), h.HandleInviteMember), http.StatusMethodNotAllowed) + + creation := adminDocBodies(t, "MULTI_TENANT", "Creating an Organization")[0] + check(call(http.MethodPost, "/api/orgs", "alice", "", creation, h.HandleCreateOrg), http.StatusCreated) + check(call(http.MethodPost, "/api/orgs", "bob", "", []byte(`{"id":"other-org-id","displayName":"Other estate"}`), h.HandleCreateOrg), http.StatusCreated) + members := adminDocBodies(t, "MULTI_TENANT", "Managing Members") + if len(members) != 2 { + t.Fatalf("want invitation and existing-member update, got %d", len(members)) + } + invitation := call(http.MethodPost, "/api/orgs/production-datacenter/members", "alice", "production-datacenter", members[0], h.HandleInviteMember) + check(invitation, http.StatusAccepted) + var invited organizationAccessMutationResponse + if err := json.Unmarshal(invitation.Body.Bytes(), &invited); err != nil || invited.Kind != "invitation" || invited.Member != nil { + t.Fatalf("new member was not pending: %s", invitation.Body.String()) + } + acceptInvitationForTest(t, h, "production-datacenter", "user-id") + check(call(http.MethodPost, "/api/orgs/production-datacenter/members", "alice", "production-datacenter", members[1], h.HandleInviteMember), http.StatusOK) + org, err := persistence.LoadOrganization("production-datacenter") + if err != nil || organizationRoleForUser(org, "user-id") != models.OrgRoleAdmin { + t.Fatalf("documented role update did not persist: %v", err) + } + + check(call(http.MethodPost, "/api/orgs/production-datacenter/shares", "alice", "production-datacenter", []byte(`{"targetOrgId":"other-org-id","resourceType":"host","resourceId":"resource-id","role":"viewer"}`), h.HandleCreateShare), http.StatusBadRequest) + shareBody := adminDocBodies(t, "MULTI_TENANT", "Sharing Resources")[0] + created := call(http.MethodPost, "/api/orgs/production-datacenter/shares", "alice", "production-datacenter", shareBody, h.HandleCreateShare) + check(created, http.StatusCreated) + var share models.OrganizationShare + if err := json.Unmarshal(created.Body.Bytes(), &share); err != nil || share.Status != models.OrganizationShareStatusPending || share.AccessRole != models.OrgRoleViewer { + t.Fatalf("documented share grant was not pending/viewer: %s", created.Body.String()) + } + accepted := acceptIncomingShareForTest(t, h, "other-org-id", "bob", share.ID) + if accepted.Status != models.OrganizationShareStatusAccepted { + t.Fatal("share acceptance did not persist") + } +} diff --git a/scripts/tests/test_admin_docs.py b/scripts/tests/test_admin_docs.py new file mode 100644 index 000000000..ac59f2f57 --- /dev/null +++ b/scripts/tests/test_admin_docs.py @@ -0,0 +1,162 @@ +#!/usr/bin/env python3 +"""Execute the administration guides with synthetic credentials and loopback. + +Run with pulse-worker-source-proof; these fixtures do not perform real role, +tenant or audit operations. The Go documentation tests check the actual handlers. +""" + +from __future__ import annotations + +import json +from pathlib import Path +import re +import stat +import tempfile +import unittest + +from test_api_auth_docs import ROOT, TEST_TOKEN, exercise_curl, recording_server + + +DOCS = ROOT / "docs" +GUIDES = ("RBAC", "AUDIT_LOGGING", "MULTI_TENANT") +EXPECTED = { + "RBAC": ( + ("POST", "/api/admin/roles", { + "id": "alert-manager", "name": "Alert Manager", + "description": "Can view and manage alerts", + "permissions": [{"action": "read", "resource": "alerts"}, + {"action": "write", "resource": "alerts"}, + {"action": "read", "resource": "nodes"}], + }), + ("GET", "/api/admin/roles", None), + ("PUT", "/api/admin/roles/alert-manager", { + "name": "Alert Manager", "description": "Updated description", + "permissions": [{"action": "read", "resource": "alerts"}, + {"action": "write", "resource": "alerts"}, + {"action": "read", "resource": "nodes"}, + {"action": "read", "resource": "ai"}], + }), + ("DELETE", "/api/admin/roles/alert-manager", None), + ("GET", "/api/admin/users", None), + ("PUT", "/api/admin/users/jane/roles", {"roleIds": ["alert-manager", "viewer"]}), + ("PUT", "/api/admin/users/jane/roles", {"roleIds": []}), + ("DELETE", "/api/admin/users/jane", None), + ), + "AUDIT_LOGGING": ( + ("GET", "/api/audit?limit=50", None), + ("GET", "/api/audit?event=login&startTime=2026-01-01T00:00:00Z&endTime=2026-01-31T23:59:59Z&success=false", None), + ("GET", "/api/audit/summary", None), + ("GET", "/api/audit/export", None), + ("GET", "/api/audit/6b3c9c3c-9a2f-4b3c-9a3b-3d0e8c5c5d45/verify", None), + ), + "MULTI_TENANT": ( + ("GET", "/api/orgs/production-datacenter/members", None), + ("GET", "/api/orgs/production-datacenter/shares/incoming", None), + ), +} + + +def recipes(name: str) -> list[str]: + requests = [] + for block in re.findall(r"```bash\n(.*?)```", (DOCS / f"{name}.md").read_text(), re.DOTALL): + # The audit read examples share a fence but are independent operations. + for step in re.split(r"\n(?=# )", block): + if "curl " in step: + requests.append(step) + return requests + + +class AdminDocsTest(unittest.TestCase): + def test_shell_recipes_never_expose_credentials_or_override_tls(self): + for name in GUIDES: + with self.subTest(guide=name): + steps = recipes(name) + self.assertEqual(len(steps), len(EXPECTED[name])) + for step in steps: + self.assertNotRegex(step, r"(?:\b\w*TOKEN=|Authorization:|X-API-Token:|Bearer\s|--cookie\b)") + self.assertNotRegex(step, r"(?:--insecure|--verbose|--trace\S*|--location|\s-k\b)") + self.assertIn('curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header"', step) + if re.search(r"--request (POST|PUT)", step): + self.assertIn("--data-binary @-", step) + self.assertIn("<<'JSON'", step) + + def test_guides_explain_authority_and_sensitive_output(self): + for name in GUIDES: + guide = (DOCS / f"{name}.md").read_text() + with self.subTest(guide=name): + self.assertIn("API.md#-authentication", guide) + self.assertIn("HTTPS", guide) + self.assertIn("certificate verification", guide) + rbac = (DOCS / "RBAC.md").read_text() + for boundary in ("full-access (`*`)", "bound to an authorised administrator", "cannot be modified or deleted", "self-escalation", "complete list", "URL-encode"): + self.assertIn(boundary, rbac) + audit = (DOCS / "AUDIT_LOGGING.md").read_text() + for boundary in ("audit:read", "audit_logging", "private directory", "redacted error", "does not reuse filters"): + self.assertIn(boundary, audit) + org = (DOCS / "MULTI_TENANT.md").read_text() + for boundary in ("session-based user authentication", "403 session_required", "CSRF", "settings:read", "organization binding", "pending invitation", "accept the share", "accessRole"): + self.assertIn(boundary, org) + self.assertNotRegex(org, r"\| `PATCH` \|") + + def test_shipped_copies_are_identical(self): + for name in (*GUIDES, "API"): + with self.subTest(guide=name): + self.assertEqual((DOCS / f"{name}.md").read_bytes(), + (ROOT / "frontend-modern/public/docs" / f"{name}.md").read_bytes()) + + def test_all_recipes_send_exact_methods_paths_and_bodies_with_each_header(self): + with recording_server() as (port, requests): + for name, operations in EXPECTED.items(): + steps = recipes(name) + self.assertEqual(len(steps), len(operations)) + for key, value in (("X-API-Token", TEST_TOKEN), ("Authorization", "Bearer " + TEST_TOKEN)): + for step, (method, path, body) in zip(steps, operations): + with self.subTest(guide=name, method=method, path=path, header=key), tempfile.TemporaryDirectory(prefix="admin docs ") as temporary: + before = len(requests) + result = exercise_curl(self, Path(temporary), f"{key}: {value}", port, step) + self.assertEqual(result.returncode, 0, result.stderr.decode()) + self.assertEqual(len(requests), before + 1, "a copied step sends exactly one request") + sent_path, headers, sent_method, sent_body = requests[-1] + self.assertEqual((sent_method, sent_path), (method, path)) + self.assertEqual(headers[key], value) + self.assertNotIn("X-Curlrc-Injected", headers) + self.assertNotIn("Authorization" if key == "X-API-Token" else "X-API-Token", headers) + if body is None: + self.assertEqual(sent_body, b"") + else: + self.assertEqual(headers["Content-Type"], "application/json") + self.assertEqual(json.loads(sent_body), body) + + def test_every_recipe_surfaces_auth_and_license_errors_without_next_steps(self): + for status in (401, 402, 403): + with recording_server(status) as (port, requests): + for name in GUIDES: + for step in recipes(name): + with self.subTest(status=status, guide=name), tempfile.TemporaryDirectory() as temporary: + before = len(requests) + result = exercise_curl(self, Path(temporary), f"X-API-Token: {TEST_TOKEN}", port, step) + self.assertEqual(result.returncode, 22, result.stderr.decode()) + self.assertEqual(len(requests), before + 1) + self.assertNotIn(b"Saved private export", result.stdout) + # curl preserves an error body, in stdout or the requested file. + exports = list(Path(temporary).glob("*/audit-export.json")) + output = exports[0].read_bytes() if exports else result.stdout + self.assertIn(b'{"fixture":true}', output) + + def test_audit_export_creates_a_private_new_directory_and_file(self): + step = next(step for step in recipes("AUDIT_LOGGING") if "/api/audit/export" in step) + with recording_server() as (port, _), tempfile.TemporaryDirectory() as temporary: + home = Path(temporary) + for _ in range(2): + result = exercise_curl(self, home, f"X-API-Token: {TEST_TOKEN}", port, step) + self.assertEqual(result.returncode, 0, result.stderr.decode()) + output = Path(result.stdout.decode().strip().removeprefix("Saved private export to ")) + self.assertEqual(output.parent.parent, home) + self.assertEqual(stat.S_IMODE(output.parent.stat().st_mode), 0o700) + self.assertEqual(stat.S_IMODE(output.stat().st_mode), 0o600) + self.assertEqual(json.loads(output.read_bytes()), {"fixture": True}) + self.assertEqual(len(list(home.glob("*/audit-export.json"))), 2) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/tests/test_api_auth_docs.py b/scripts/tests/test_api_auth_docs.py index b085fc5d5..4f18ff5fd 100644 --- a/scripts/tests/test_api_auth_docs.py +++ b/scripts/tests/test_api_auth_docs.py @@ -80,6 +80,9 @@ def recording_server(status: int = 200): do_GET = record_request do_POST = record_request + do_PUT = record_request + do_PATCH = record_request + do_DELETE = record_request def log_message(self, *_args): pass @@ -95,6 +98,48 @@ def recording_server(status: int = 200): thread.join(timeout=5) +def exercise_curl(case: unittest.TestCase, home: Path, header_text: str, port: int, request: str): + """Execute the exact copied recipe; capture argv and hostile curl defaults.""" + header = home / ".config/pulse/api-header" + header.parent.mkdir(parents=True, exist_ok=True) + header.write_text(header_text + "\n") + header.chmod(0o600) + real_curl = shutil.which("curl") + case.assertIsNotNone(real_curl, "curl is required to exercise the documented command") + tools = home / "tools" + tools.mkdir(exist_ok=True) + recorder = tools / "curl" + recorder.write_text( + "#!/usr/bin/env python3\nimport json, os, sys\n" + "from pathlib import Path\n" + "Path(os.environ['ARGV_RECEIPT']).write_text(json.dumps(sys.argv[1:]))\n" + "os.execv(os.environ['REAL_CURL'], [os.environ['REAL_CURL'], *sys.argv[1:]])\n" + ) + recorder.chmod(0o700) + receipt = home / "argv.json" + trace = home / "curl-trace.txt" + # An existing curl configuration must not turn safe argv into a trace + # containing the header file's credential, or inject another header. + (home / ".curlrc").write_text( + f'header = "X-Curlrc-Injected: yes"\nverbose\ntrace-ascii = "{trace}"\n' + ) + env = dict(os.environ, HOME=str(home), PATH=f"{tools}:{os.environ['PATH']}", + CURL_HOME=str(home), XDG_CONFIG_HOME=str(home / ".config"), TMPDIR=str(home), + REAL_CURL=real_curl, ARGV_RECEIPT=str(receipt)) + for key in list(env): + if key.lower().endswith("_proxy"): + del env[key] + request = request.replace("http://127.0.0.1:7655", f"http://127.0.0.1:{port}") + result = subprocess.run(["bash", "-eu", "-c", request], env=env, capture_output=True, timeout=10) + argv = json.loads(receipt.read_text()) + case.assertEqual(argv[0], "--disable", "curl defaults must be disabled by the first option") + case.assertNotIn(TEST_TOKEN, " ".join(argv)) + case.assertIn("@" + str(header), argv) + case.assertNotIn(b"synthetic-doc-test-token", result.stdout + result.stderr) + case.assertFalse(trace.exists(), "local curl configuration must not create a credential trace") + return result + + class APIAuthDocsTest(unittest.TestCase): def test_credentials_are_not_documented_as_command_arguments(self): section = auth_section() @@ -155,46 +200,7 @@ class APIAuthDocsTest(unittest.TestCase): self.assertEqual(header.read_text(), f"X-API-Token: {TEST_TOKEN}\n" if existing else "") def run_documented_request(self, home: Path, header_text: str, port: int, request=None): - if request is None: - _, request = commands() - header = home / ".config/pulse/api-header" - header.parent.mkdir(parents=True, exist_ok=True) - header.write_text(header_text + "\n") - header.chmod(0o600) - real_curl = shutil.which("curl") - self.assertIsNotNone(real_curl, "curl is required to exercise the documented command") - tools = home / "tools" - tools.mkdir(exist_ok=True) - recorder = tools / "curl" - recorder.write_text( - "#!/usr/bin/env python3\nimport json, os, sys\n" - "from pathlib import Path\n" - "Path(os.environ['ARGV_RECEIPT']).write_text(json.dumps(sys.argv[1:]))\n" - "os.execv(os.environ['REAL_CURL'], [os.environ['REAL_CURL'], *sys.argv[1:]])\n" - ) - recorder.chmod(0o700) - receipt = home / "argv.json" - trace = home / "curl-trace.txt" - # An existing curl configuration must not turn safe argv into a trace - # containing the header file's credential, or inject another header. - (home / ".curlrc").write_text( - f'header = "X-Curlrc-Injected: yes"\nverbose\ntrace-ascii = "{trace}"\n' - ) - env = dict(os.environ, HOME=str(home), PATH=f"{tools}:{os.environ['PATH']}", - CURL_HOME=str(home), XDG_CONFIG_HOME=str(home / ".config"), - REAL_CURL=real_curl, ARGV_RECEIPT=str(receipt)) - for key in list(env): - if key.lower().endswith("_proxy"): - del env[key] - request = request.replace("http://127.0.0.1:7655", f"http://127.0.0.1:{port}") - result = subprocess.run(["bash", "-eu", "-c", request], env=env, capture_output=True, timeout=10) - argv = json.loads(receipt.read_text()) - self.assertEqual(argv[0], "--disable", "curl defaults must be disabled by the first option") - self.assertNotIn(TEST_TOKEN, " ".join(argv)) - self.assertIn("@" + str(header), argv) - self.assertNotIn(b"synthetic-doc-test-token", result.stdout + result.stderr) - self.assertFalse(trace.exists(), "local curl configuration must not create a credential trace") - return result + return exercise_curl(self, home, header_text, port, request if request is not None else commands()[1]) def test_header_file_sends_each_supported_header_without_exposing_argv(self): with tempfile.TemporaryDirectory() as temporary, recording_server() as (port, requests): From 2c98baabeac49673c9ed0ad2e9ef91fb8795beb8 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Thu, 1 Oct 2026 08:44:10 +0100 Subject: [PATCH 2/3] Align administration guide details with current handlers Use the accepted organisation ID alphabet and canonical resource types, and expect the documented role assignments to return 204. Keep the guide lifecycle checks at the existing API contract. Contract-Neutral: Documentation and test expectations only; API methods, validation, authentication and tenant isolation are unchanged. Change-source: pulse-maintainer --- docs/API.md | 4 ++-- docs/MULTI_TENANT.md | 16 +++++++++------- frontend-modern/public/docs/API.md | 4 ++-- frontend-modern/public/docs/MULTI_TENANT.md | 16 +++++++++------- internal/api/admin_docs_test.go | 4 ++-- 5 files changed, 24 insertions(+), 20 deletions(-) diff --git a/docs/API.md b/docs/API.md index 285d945cc..dde843211 100644 --- a/docs/API.md +++ b/docs/API.md @@ -1178,7 +1178,7 @@ Returns organizations accessible to the authenticated user. ```json { "id": "acme-corp", "displayName": "Acme Corporation" } ``` -The creator becomes the owner and first member. Organization IDs must be lowercase alphanumeric with hyphens, 3-64 characters. +The creator becomes the owner and first member. Organization IDs use letters, digits, periods, underscores or hyphens, 1–64 characters, but cannot be `.` or `..`. Prefer a simple lowercase/hyphen ID such as the example. ### Get Organization `GET /api/orgs/{id}` (requires `settings:read`) @@ -1229,7 +1229,7 @@ Returns resources shared inbound to this organization from other organizations. "accessRole": "viewer" } ``` -Share a resource with another organization. Valid resource types: `vm`, `container`, `agent`, `storage`, `pbs`, `pmg` (not `host`). Access roles: `viewer`, `editor`, `admin`. Admin or owner role required on the source org. The share is pending until a target-org admin or owner accepts it in Pulse; changing its access role requires acceptance again. +Share a resource with another organization, using the resource type and ID returned by Pulse. Supported types include `vm`, `system-container`, `agent`, `node`, `docker-host`, `storage`, `pbs` and `pmg`; generic `host` and `container` types are not supported. Access roles: `viewer`, `editor`, `admin`. Admin or owner role required on the source org. The share is pending until a target-org admin or owner accepts it in Pulse; changing its access role requires acceptance again. ### Delete Share `DELETE /api/orgs/{id}/shares/{shareId}` (requires `settings:write`, session auth only) diff --git a/docs/MULTI_TENANT.md b/docs/MULTI_TENANT.md index 88276d41f..4e7e4d070 100644 --- a/docs/MULTI_TENANT.md +++ b/docs/MULTI_TENANT.md @@ -87,9 +87,10 @@ the relevant redacted error, not whole member or infrastructure responses. {"id": "production-datacenter", "displayName": "Production Datacenter"} ``` -The creator becomes the owner. `id` is a lowercase alphanumeric/hyphen ID -(3–64 characters); `displayName` is the name shown in Pulse. `name` and -`description` are not the creation fields. +The creator becomes the owner. `id` is a stable ID of 1–64 characters using +letters, digits, periods, underscores or hyphens, but not `.` or `..`; use a +simple lowercase/hyphen ID such as the example. `displayName` is the name +shown in Pulse. `name` and `description` are not the creation fields. ### Switching Organizations @@ -145,10 +146,11 @@ Until acceptance, the share is pending and does not grant access. } ``` -Use `accessRole`, not `role`. Supported resource types are `vm`, `container`, -`agent`, `storage`, `pbs` and `pmg`; `host` is not supported. Use the resource ID -returned by Pulse, not a display name. Changing a share's access role makes it -pending again, so the target must accept the new grant. +Use `accessRole`, not `role`, and the resource type and ID returned by Pulse, +not a display name. Supported types include `vm`, `system-container`, `agent`, +`node`, `docker-host`, `storage`, `pbs` and `pmg`; the generic types `host` and +`container` are not supported. Changing a share's access role makes it pending +again, so the target must accept the new grant. **Read-only incoming shares:** ```bash diff --git a/frontend-modern/public/docs/API.md b/frontend-modern/public/docs/API.md index 285d945cc..dde843211 100644 --- a/frontend-modern/public/docs/API.md +++ b/frontend-modern/public/docs/API.md @@ -1178,7 +1178,7 @@ Returns organizations accessible to the authenticated user. ```json { "id": "acme-corp", "displayName": "Acme Corporation" } ``` -The creator becomes the owner and first member. Organization IDs must be lowercase alphanumeric with hyphens, 3-64 characters. +The creator becomes the owner and first member. Organization IDs use letters, digits, periods, underscores or hyphens, 1–64 characters, but cannot be `.` or `..`. Prefer a simple lowercase/hyphen ID such as the example. ### Get Organization `GET /api/orgs/{id}` (requires `settings:read`) @@ -1229,7 +1229,7 @@ Returns resources shared inbound to this organization from other organizations. "accessRole": "viewer" } ``` -Share a resource with another organization. Valid resource types: `vm`, `container`, `agent`, `storage`, `pbs`, `pmg` (not `host`). Access roles: `viewer`, `editor`, `admin`. Admin or owner role required on the source org. The share is pending until a target-org admin or owner accepts it in Pulse; changing its access role requires acceptance again. +Share a resource with another organization, using the resource type and ID returned by Pulse. Supported types include `vm`, `system-container`, `agent`, `node`, `docker-host`, `storage`, `pbs` and `pmg`; generic `host` and `container` types are not supported. Access roles: `viewer`, `editor`, `admin`. Admin or owner role required on the source org. The share is pending until a target-org admin or owner accepts it in Pulse; changing its access role requires acceptance again. ### Delete Share `DELETE /api/orgs/{id}/shares/{shareId}` (requires `settings:write`, session auth only) diff --git a/frontend-modern/public/docs/MULTI_TENANT.md b/frontend-modern/public/docs/MULTI_TENANT.md index 88276d41f..4e7e4d070 100644 --- a/frontend-modern/public/docs/MULTI_TENANT.md +++ b/frontend-modern/public/docs/MULTI_TENANT.md @@ -87,9 +87,10 @@ the relevant redacted error, not whole member or infrastructure responses. {"id": "production-datacenter", "displayName": "Production Datacenter"} ``` -The creator becomes the owner. `id` is a lowercase alphanumeric/hyphen ID -(3–64 characters); `displayName` is the name shown in Pulse. `name` and -`description` are not the creation fields. +The creator becomes the owner. `id` is a stable ID of 1–64 characters using +letters, digits, periods, underscores or hyphens, but not `.` or `..`; use a +simple lowercase/hyphen ID such as the example. `displayName` is the name +shown in Pulse. `name` and `description` are not the creation fields. ### Switching Organizations @@ -145,10 +146,11 @@ Until acceptance, the share is pending and does not grant access. } ``` -Use `accessRole`, not `role`. Supported resource types are `vm`, `container`, -`agent`, `storage`, `pbs` and `pmg`; `host` is not supported. Use the resource ID -returned by Pulse, not a display name. Changing a share's access role makes it -pending again, so the target must accept the new grant. +Use `accessRole`, not `role`, and the resource type and ID returned by Pulse, +not a display name. Supported types include `vm`, `system-container`, `agent`, +`node`, `docker-host`, `storage`, `pbs` and `pmg`; the generic types `host` and +`container` are not supported. Changing a share's access role makes it pending +again, so the target must accept the new grant. **Read-only incoming shares:** ```bash diff --git a/internal/api/admin_docs_test.go b/internal/api/admin_docs_test.go index 15e2ed1a0..e625e449d 100644 --- a/internal/api/admin_docs_test.go +++ b/internal/api/admin_docs_test.go @@ -77,8 +77,8 @@ func TestAdminDocsCustomRoleLifecycle(t *testing.T) { }{ {http.MethodPost, "/api/admin/roles", adminDocBodies(t, "RBAC", "Creating a Role")[0], h.HandleRoles, http.StatusOK}, {http.MethodPut, "/api/admin/roles/alert-manager", adminDocBodies(t, "RBAC", "Updating a Role")[0], h.HandleRoles, http.StatusOK}, - {http.MethodPut, "/api/admin/users/jane/roles", assignments[0], h.HandleUserRoleActions, http.StatusOK}, - {http.MethodPut, "/api/admin/users/jane/roles", assignments[1], h.HandleUserRoleActions, http.StatusOK}, + {http.MethodPut, "/api/admin/users/jane/roles", assignments[0], h.HandleUserRoleActions, http.StatusNoContent}, + {http.MethodPut, "/api/admin/users/jane/roles", assignments[1], h.HandleUserRoleActions, http.StatusNoContent}, {http.MethodDelete, "/api/admin/users/jane", nil, h.HandleUserRoleActions, http.StatusNoContent}, {http.MethodDelete, "/api/admin/roles/alert-manager", nil, h.HandleRoles, http.StatusNoContent}, } From 0c3cd4fb3ad1a32cb7ddb00b0992a53991594b79 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Thu, 1 Oct 2026 08:55:56 +0100 Subject: [PATCH 3/3] Isolate administration guide session-revocation proof Initialise the real persistent auth stores for the guide lifecycle and prove user removal revokes its session and CSRF token. Clarify that an empty role list clears built-in assignments too; no runtime authority changes. Contract-Neutral: Documentation and test-fixture correction only; session revocation, role assignment and authorization runtime contracts are unchanged. Change-source: pulse-maintainer --- docs/RBAC.md | 2 +- frontend-modern/public/docs/RBAC.md | 2 +- internal/api/admin_docs_test.go | 18 +++++++++++++++++- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/docs/RBAC.md b/docs/RBAC.md index 7b0f1d69c..ca9b5d1cc 100644 --- a/docs/RBAC.md +++ b/docs/RBAC.md @@ -155,7 +155,7 @@ curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ JSON ``` -To remove all custom roles from a user, send an empty list: +To clear a user's entire role assignment, including built-in roles, send an empty list: ```bash curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ diff --git a/frontend-modern/public/docs/RBAC.md b/frontend-modern/public/docs/RBAC.md index 7b0f1d69c..ca9b5d1cc 100644 --- a/frontend-modern/public/docs/RBAC.md +++ b/frontend-modern/public/docs/RBAC.md @@ -155,7 +155,7 @@ curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ JSON ``` -To remove all custom roles from a user, send an empty list: +To clear a user's entire role assignment, including built-in roles, send an empty list: ```bash curl --disable --fail-with-body --header "@$HOME/.config/pulse/api-header" \ diff --git a/internal/api/admin_docs_test.go b/internal/api/admin_docs_test.go index e625e449d..331bf05c2 100644 --- a/internal/api/admin_docs_test.go +++ b/internal/api/admin_docs_test.go @@ -10,6 +10,7 @@ import ( "regexp" "strings" "testing" + "time" "github.com/rcourtman/pulse-go-rewrite/internal/config" "github.com/rcourtman/pulse-go-rewrite/internal/models" @@ -48,7 +49,14 @@ func adminDocBodies(t *testing.T, name, heading string) [][]byte { } func TestAdminDocsCustomRoleLifecycle(t *testing.T) { - manager, err := auth.NewFileManager(t.TempDir()) + dataPath := t.TempDir() + resetPersistentAuthStoresForTests() + t.Cleanup(resetPersistentAuthStoresForTests) + InitPersistentAuthStores(dataPath) + const fixtureSession = "admin-docs-jane-session-fixture" + GetSessionStore().CreateSession(fixtureSession, time.Hour, "fixture", "127.0.0.1", "jane") + fixtureCSRF := GetCSRFStore().GenerateCSRFToken(fixtureSession) + manager, err := auth.NewFileManager(dataPath) if err != nil { t.Fatal(err) } @@ -106,6 +114,14 @@ func TestAdminDocsCustomRoleLifecycle(t *testing.T) { t.Fatalf("assignment did not persist the complete role list: %+v", assignment) } } + if step.method == http.MethodDelete && step.path == "/api/admin/users/jane" { + if _, exists := manager.GetUserAssignment("jane"); exists { + t.Fatal("user removal retained the role assignment") + } + if GetSessionStore().GetSession(fixtureSession) != nil || GetCSRFStore().ValidateCSRFToken(fixtureSession, fixtureCSRF) { + t.Fatal("user removal retained the session or its CSRF token") + } + } } if _, exists := manager.GetRole("alert-manager"); exists { t.Fatal("custom role deletion did not persist")