fix(04-rbac): revise plans based on checker feedback
This commit is contained in:
@@ -28,6 +28,7 @@ must_haves:
|
||||
- "Every tenant-scoped endpoint returns 403 for customer_admin accessing a different tenant"
|
||||
- "Platform admin gets 200 on any tenant's endpoints regardless of membership"
|
||||
- "Customer operator gets 200 on read-only endpoints (GET agents, GET usage) for their tenant"
|
||||
- "Customer operator can POST a test message to an agent (POST /tenants/{tid}/agents/{aid}/test) and get 200"
|
||||
- "Impersonation actions are logged in audit_events with platform admin user_id"
|
||||
- "Full invite flow works end-to-end: create invitation -> accept -> login -> correct role"
|
||||
artifacts:
|
||||
@@ -40,8 +41,8 @@ must_haves:
|
||||
key_links:
|
||||
- from: "packages/shared/shared/api/portal.py"
|
||||
to: "packages/shared/shared/api/rbac.py"
|
||||
via: "Depends(require_tenant_admin) on mutating endpoints"
|
||||
pattern: "Depends\\(require_tenant_admin\\)|Depends\\(require_platform_admin\\)"
|
||||
via: "Depends(require_tenant_admin) on mutating endpoints, Depends(require_tenant_member) on test-message endpoint"
|
||||
pattern: "Depends\\(require_tenant_admin\\)|Depends\\(require_platform_admin\\)|Depends\\(require_tenant_member\\)"
|
||||
- from: "packages/shared/shared/api/billing.py"
|
||||
to: "packages/shared/shared/api/rbac.py"
|
||||
via: "Depends(require_tenant_admin) on billing endpoints"
|
||||
@@ -53,10 +54,10 @@ must_haves:
|
||||
---
|
||||
|
||||
<objective>
|
||||
Wire RBAC guards to ALL existing portal API endpoints, add impersonation audit logging, add user listing endpoints, and create comprehensive integration tests proving every endpoint enforces role-based authorization.
|
||||
Wire RBAC guards to ALL existing portal API endpoints, add test-message endpoint for operators, add impersonation audit logging, add user listing endpoints, and create comprehensive integration tests proving every endpoint enforces role-based authorization.
|
||||
|
||||
Purpose: Defense in depth — the UI hides things, but the API MUST enforce authorization. This plan completes the server-side enforcement layer and validates the entire RBAC system end-to-end.
|
||||
Output: All portal endpoints guarded, impersonation logged, integration tests for RBAC + invite flow.
|
||||
Output: All portal endpoints guarded, test-message endpoint for operators, impersonation logged, integration tests for RBAC + invite flow.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@@ -104,6 +105,9 @@ POST /api/portal/tenants/{tid}/agents # require_tenant_admin
|
||||
GET /api/portal/tenants/{tid}/agents/{aid} # require_tenant_member
|
||||
PUT /api/portal/tenants/{tid}/agents/{aid} # require_tenant_admin
|
||||
DELETE /api/portal/tenants/{tid}/agents/{aid} # require_tenant_admin
|
||||
|
||||
# NEW — Test message (per locked decision: operators can send test messages)
|
||||
POST /api/portal/tenants/{tid}/agents/{aid}/test # require_tenant_member (operators included)
|
||||
```
|
||||
|
||||
From packages/shared/shared/api/billing.py, channels.py, llm_keys.py, usage.py:
|
||||
@@ -126,7 +130,7 @@ class AuditEvent(Base):
|
||||
<tasks>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 1: Wire RBAC guards to all existing API endpoints + impersonation + user listing</name>
|
||||
<name>Task 1: Wire RBAC guards to all existing API endpoints + test-message endpoint + impersonation + user listing</name>
|
||||
<files>
|
||||
packages/shared/shared/api/portal.py,
|
||||
packages/shared/shared/api/billing.py,
|
||||
@@ -149,6 +153,7 @@ class AuditEvent(Base):
|
||||
- `GET /tenants/{tenant_id}/agents/{agent_id}` — add `Depends(require_tenant_member)`.
|
||||
- `PUT /tenants/{tenant_id}/agents/{agent_id}` — add `Depends(require_tenant_admin)`.
|
||||
- `DELETE /tenants/{tenant_id}/agents/{agent_id}` — add `Depends(require_tenant_admin)`.
|
||||
- ADD new endpoint: `POST /tenants/{tenant_id}/agents/{agent_id}/test` — requires `Depends(require_tenant_member)` (NOT require_tenant_admin). This is the test-message endpoint per locked decision: "operators can send test messages to agents." Accepts `{message: str}` body. Dispatches the message through the agent orchestrator pipeline (or a lightweight test handler) for the specified agent, returns the agent's response. This allows operators to QA agent behavior without agent CRUD access.
|
||||
- ADD new endpoint: `GET /tenants/{tenant_id}/users` — requires require_tenant_admin. Queries UserTenantRole JOIN PortalUser WHERE tenant_id matches. Returns list of {id, name, email, role, created_at}. Also queries PortalInvitation WHERE tenant_id AND status='pending' to include pending invites.
|
||||
- ADD new endpoint: `GET /admin/users` — requires require_platform_admin. Queries ALL PortalUser with their UserTenantRole associations. Supports optional query params: tenant_id filter, role filter. Returns list with tenant membership info.
|
||||
|
||||
@@ -189,8 +194,11 @@ from shared.api.billing import billing_router
|
||||
from shared.api.channels import channels_router
|
||||
routes = [r.path for r in portal_router.routes]
|
||||
print(f'Portal routes: {len(routes)}')
|
||||
# Verify test-message endpoint exists
|
||||
test_routes = [r.path for r in portal_router.routes if 'test' in r.path]
|
||||
assert test_routes, 'Missing /test endpoint for agent test messages'
|
||||
print(f'Test-message route: {test_routes}')
|
||||
# Verify at least one route has dependencies
|
||||
import inspect
|
||||
for r in portal_router.routes:
|
||||
if hasattr(r, 'dependant') and r.dependant.dependencies:
|
||||
print(f' {r.path} has {len(r.dependant.dependencies)} dependencies')
|
||||
@@ -199,7 +207,7 @@ else:
|
||||
print('WARNING: No routes have dependencies')
|
||||
"</automated>
|
||||
</verify>
|
||||
<done>Every portal API endpoint has an RBAC guard. Mutating endpoints require tenant_admin or platform_admin. Read-only tenant endpoints allow tenant_member. Global endpoints require platform_admin. Impersonation endpoint logs to audit trail. User listing endpoints exist for both per-tenant and global views.</done>
|
||||
<done>Every portal API endpoint has an RBAC guard. Mutating endpoints require tenant_admin or platform_admin. Read-only tenant endpoints allow tenant_member. Test-message endpoint (POST /tenants/{tid}/agents/{aid}/test) allows tenant_member including operators. Global endpoints require platform_admin. Impersonation endpoint logs to audit trail. User listing endpoints exist for both per-tenant and global views.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto" tdd="true">
|
||||
@@ -212,6 +220,7 @@ else:
|
||||
- Platform admin with correct headers gets 200 on all endpoints
|
||||
- Customer admin gets 200 on own-tenant endpoints, 403 on other tenants
|
||||
- Customer operator gets 200 on GET endpoints, 403 on POST/PUT/DELETE
|
||||
- Customer operator gets 200 on POST /tenants/{tid}/agents/{aid}/test (test message — exception to POST restriction)
|
||||
- Missing role headers return 401/422 (FastAPI Header() validation)
|
||||
- Impersonation endpoint logs AuditEvent row
|
||||
- Full invite flow: admin creates invite -> token generated -> accept with password -> new user can login -> new user has correct role and tenant membership
|
||||
@@ -237,6 +246,7 @@ else:
|
||||
| POST /tenants/{tid}/agents | 201 | 201 | 403 | 403 |
|
||||
| PUT /tenants/{tid}/agents/{aid} | 200 | 200 | 403 | 403 |
|
||||
| DELETE /tenants/{tid}/agents/{aid} | 204 | 204 | 403 | 403 |
|
||||
| POST /tenants/{tid}/agents/{aid}/test | 200 | 200 | 403 | 200 |
|
||||
| GET /tenants/{tid}/users | 200 | 200 | 403 | 403 |
|
||||
| GET /admin/users | 200 | 403 | 403 | 403 |
|
||||
|
||||
@@ -244,6 +254,7 @@ else:
|
||||
- Request with NO role headers -> 422 (missing required header)
|
||||
- Impersonation endpoint creates AuditEvent row
|
||||
- Billing, channels, llm_keys, usage endpoints follow same pattern (at least one representative test per router)
|
||||
- Specific test: customer_operator can POST to /test endpoint but NOT to agent CRUD POST
|
||||
|
||||
Create `tests/integration/test_invite_flow.py`:
|
||||
- Set up: create a tenant, create a customer_admin user with membership
|
||||
@@ -263,7 +274,7 @@ else:
|
||||
<verify>
|
||||
<automated>cd /home/adelorenzo/repos/konstruct && pytest tests/integration/test_portal_rbac.py tests/integration/test_invite_flow.py -x -v</automated>
|
||||
</verify>
|
||||
<done>All RBAC integration tests pass — every endpoint returns correct status code for each role. Full invite flow works end-to-end. Expired invites are rejected. Resend works. Double-accept prevented.</done>
|
||||
<done>All RBAC integration tests pass — every endpoint returns correct status code for each role. Operator test-message endpoint returns 200. Full invite flow works end-to-end. Expired invites are rejected. Resend works. Double-accept prevented.</done>
|
||||
</task>
|
||||
|
||||
<task type="checkpoint:human-verify" gate="blocking">
|
||||
@@ -279,8 +290,9 @@ else:
|
||||
- Verify: can access /admin/users (global user management)
|
||||
- Verify: can impersonate a tenant (banner appears, can exit)
|
||||
4. Create a customer_admin invite from the Users page
|
||||
5. Open the invite link in an incognito window
|
||||
- Verify: activation page shows, can set password
|
||||
5. Open the invite link in an incognito window (URL will be /invite/{token} — NOT inside dashboard)
|
||||
- Verify: activation page shows without requiring login
|
||||
- Verify: can set password and complete account creation
|
||||
- Verify: after activation, redirected to login
|
||||
6. Login as the new customer admin:
|
||||
- Verify: sees Dashboard, Employees, Usage, Billing, API Keys, Users (no Tenants, no Platform)
|
||||
@@ -290,11 +302,12 @@ else:
|
||||
- Verify: sees only Employees and Usage in nav
|
||||
- Verify: navigating to /billing redirects to /agents
|
||||
- Verify: cannot see Billing, API Keys, Users in sidebar
|
||||
- Verify: can click "Test" on an agent to send a test message (per locked decision)
|
||||
9. If user has multiple tenants, verify tenant switcher appears and switches context
|
||||
10. Run: `pytest tests/ -x` — all tests pass
|
||||
</action>
|
||||
<verify>Human confirms all verification steps pass or reports issues</verify>
|
||||
<done>All three roles behave correctly in portal UI and API. Invitation flow works end-to-end. Full test suite green.</done>
|
||||
<done>All three roles behave correctly in portal UI and API. Operators can send test messages but not edit agents. Invitation flow works end-to-end. Full test suite green.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
@@ -305,16 +318,18 @@ else:
|
||||
- `pytest tests/ -x` — entire test suite green (no regressions)
|
||||
- Every mutating endpoint returns 403 without proper role headers
|
||||
- Platform admin bypasses all tenant membership checks
|
||||
- Operator gets 200 on POST /tenants/{tid}/agents/{aid}/test
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- All portal API endpoints enforce role-based authorization via Depends() guards
|
||||
- Customer operators cannot mutate any data via API (403 on POST/PUT/DELETE)
|
||||
- Customer operators cannot mutate any data via API (403 on POST/PUT/DELETE) EXCEPT test-message endpoint
|
||||
- Customer operators CAN send test messages to agents (POST /tenants/{tid}/agents/{aid}/test returns 200)
|
||||
- Customer admins can only access their own tenant's data (403 on other tenants)
|
||||
- Platform admin has unrestricted access to all endpoints
|
||||
- Impersonation actions logged in audit_events table
|
||||
- User listing endpoints exist for per-tenant and global views
|
||||
- Integration tests comprehensively cover the RBAC matrix
|
||||
- Integration tests comprehensively cover the RBAC matrix including test-message operator access
|
||||
- Full invite flow works end-to-end in integration tests
|
||||
- Human verification confirms visual role-based behavior in portal
|
||||
</success_criteria>
|
||||
|
||||
Reference in New Issue
Block a user