TenantAtlas/specs/309-rbac-role-matrix-access-boundary-audit/plan.md
ahmido dd175c16a1 fix: tighten workspace RBAC access boundaries (#364)
## Summary
- tighten workspace RBAC and panel access boundaries
- remove non-owner workspace membership management capability from workspace role mapping
- add focused boundary coverage for admin panel, managed environments, providers, review packs, operation runs, finding exceptions, and workspace role capabilities
- include spec artifacts for feature 309

## Testing
- cd apps/platform && ./vendor/bin/sail artisan test --compact tests/Feature/Auth/WorkspaceFirstManagedEnvironmentAccessTest.php tests/Feature/Rbac/RoleMatrix/ManagerAccessTest.php tests/Feature/Rbac/WorkspaceMembershipsRelationManagerUiEnforcementTest.php tests/Feature/Rbac/AdminPanelAccessBoundaryTest.php tests/Feature/Rbac/FindingExceptionLifecycleAccessBoundaryTest.php tests/Feature/Rbac/ManagedEnvironmentAccessBoundaryTest.php tests/Feature/Rbac/OperationRunAccessBoundaryTest.php tests/Feature/Rbac/ProviderConnectionAccessBoundaryTest.php tests/Feature/Rbac/ReviewPackAccessBoundaryTest.php tests/Feature/Rbac/SystemPanelAccessBoundaryTest.php tests/Feature/Rbac/WorkspaceRoleCapabilityBoundaryTest.php tests/Unit/Auth/CapabilityResolverTest.php tests/Unit/Auth/WorkspaceRoleCapabilityMapTest.php
- cd apps/platform && ./vendor/bin/sail bin pint --dirty --format agent

Co-authored-by: Ahmed Darrazi <ahmed.darrazi@live.de>
Reviewed-on: #364
2026-05-15 14:00:21 +00:00

349 lines
20 KiB
Markdown

# Implementation Plan: RBAC Role Matrix & Access Boundary Audit
**Branch**: `309-rbac-role-matrix-access-boundary-audit` | **Date**: 2026-05-15 | **Spec**: [spec.md](spec.md)
**Input**: Feature specification from `/specs/309-rbac-role-matrix-access-boundary-audit/spec.md`
## Summary
Audit and harden TenantPilot's RBAC and panel-access boundaries across workspace, managed-environment, provider connection, review, review pack, evidence, finding exception, OperationRun, Admin panel, and System panel surfaces.
The implementation is audit-first:
1. Inventory repo-real role/capability grants and direct access boundaries.
2. Classify Constitution/runtime contradictions.
3. Write focused tests for confirmed contradictions and boundary bugs.
4. Apply only minimal runtime fixes.
5. Re-run focused regression lanes and document remaining product decisions.
## Technical Context
**Language/Version**: PHP 8.4.15, Laravel 12.52.0
**Primary Dependencies**: Filament 5.2.1, Livewire 4.1.4, Pest 4.3.1, Laravel Sail
**Storage**: PostgreSQL; no schema change expected
**Testing**: Pest 4 Unit and Feature tests; Browser only if Feature/Filament tests cannot prove a panel/action boundary
**Validation Lanes**: confidence; optional browser; formatting/diff-check
**Target Platform**: Laravel Sail locally; Dokploy container deployment for staging/production
**Project Type**: Laravel/Filament app under `apps/platform`
**Performance Goals**: No broad query redesign; authorization checks should reuse existing resolvers and avoid new global scans
**Constraints**: no new RBAC architecture, no migrations by default, no new roles, no new assets, no UI redesign, no broad route cleanup
**Scale/Scope**: focused security-boundary audit and minimal fixes over existing role maps/policies/panels/tests
## UI / Surface Guardrail Plan
- **Guardrail scope**: no new operator-facing surface; direct access/action authorization and possible minor visibility alignment only.
- **Native vs custom classification summary**: existing Filament v5 panels/resources/pages/actions.
- **Shared-family relevance**: panel access, workspace/environment context, resource authorization, direct action execution, global search safety.
- **State layers in scope**: panel guard, middleware, route model binding, policies, Filament page/action access, Livewire action execution.
- **Audience modes in scope**: workspace users, customer-safe readers, platform/system users.
- **Decision/diagnostic/raw hierarchy plan**: no presentation hierarchy change.
- **Raw/support gating plan**: verify raw/provider/support details remain gated by existing policies.
- **One-primary-action / duplicate-truth control**: no new actions.
- **Handling modes by drift class or surface**: security blocker and high-risk contradiction fix in 309; product-decision-needed stays deferred.
- **Repository-signal treatment**: review-mandatory for role-map, panel, policy, and sensitive action changes.
- **Special surface test profiles**: standard-native-filament plus direct Feature/Livewire action tests.
- **Required tests or manual smoke**: focused Unit/Feature tests; browser not required unless direct behavior cannot be proven otherwise.
- **Exception path and spread control**: no exceptions planned.
- **Active feature PR close-out entry**: RBAC Inventory / Boundary Proof / Remaining Decisions.
## Shared Pattern & System Fit
- **Cross-cutting feature marker**: yes.
- **Systems touched**: role maps, capability resolvers, policies, gates, panels, workspace context, Filament resources/pages/actions, existing RBAC tests.
- **Shared abstractions reused**: `Capabilities`, `PlatformCapabilities`, `WorkspaceRoleCapabilityMap`, `RoleCapabilityMap`, `WorkspaceCapabilityResolver`, `CapabilityResolver`, `ManagedEnvironmentAccessScopeResolver`, model policies, Gate definitions, Filament access methods.
- **New abstraction introduced? why?**: none planned.
- **Why the existing abstraction was sufficient or insufficient**: the repo already has canonical maps and resolvers; the issue is suspected incorrect grants or missing enforcement, not absent infrastructure.
- **Bounded deviation / spread control**: any fix must remain local to existing maps/policies/panel checks and must not add a public permission framework.
## OperationRun UX Impact
- **Touches OperationRun start/completion/link UX?**: authorization only.
- **Central contract reused**: `OperationRunPolicy`, `OperationRunCapabilityResolver`, `OperationRunLinks`.
- **Delegated UX behaviors**: all existing start/completion/link feedback remains unchanged.
- **Surface-owned behavior kept local**: no new operation UI or notifications.
- **Queued DB-notification policy**: N/A.
- **Terminal notification path**: existing central lifecycle.
- **Exception path**: none.
## Provider Boundary & Portability Fit
- **Shared provider/platform boundary touched?**: provider credential authorization is audited.
- **Provider-owned seams**: credential rotation/delete/dedicated credential management.
- **Platform-core seams**: ProviderConnection resource/policy and workspace/environment access boundaries.
- **Neutral platform terms / contracts preserved**: provider connection, provider credential, workspace, managed environment, operation.
- **Retained provider-specific semantics and why**: existing Microsoft/Intune details stay in provider-owned logic.
- **Bounded extraction or follow-up path**: no provider registry redesign; provider-specific follow-up only if audit finds structural drift.
## Constitution Check
- Inventory-first: PASS. No inventory or Graph sync behavior changes are planned.
- Read/write separation: PASS. Mutations remain behind existing action flows; this spec audits and fixes authorization.
- Graph contract path: PASS. No Graph endpoint or contract change.
- Deterministic capabilities: NEEDS IMPLEMENTATION PROOF. Role/capability grants must become deterministic and tested.
- RBAC-UX: NEEDS IMPLEMENTATION PROOF. Constitution says Manager must not manage tenant memberships; current maps appear to grant Manager membership-management authority.
- Workspace isolation: NEEDS IMPLEMENTATION PROOF. Cross-workspace direct URLs must be tested for representative sensitive resources.
- Tenant/managed-environment isolation: NEEDS IMPLEMENTATION PROOF. Same-workspace wrong-environment direct URLs must be tested.
- Cross-plane access: PARTIAL PASS. Existing `/system` tests prove several tenant-session denials; Spec 309 should keep/extend focused proof.
- Run observability: PASS. No new run behavior; OperationRun access must remain policy-gated.
- TEST-GOV-001: PASS if tests stay focused and no broad helper defaults are introduced.
- PROP-001 / BLOAT-001: PASS. No new tables, roles, capability framework, enum, or UI taxonomy planned.
- PERSIST-001: PASS. No new persisted truth.
- STATE-001: PASS. No new lifecycle/status family.
- UI-SEM-001: PASS. No presentation framework.
- XCUT-001: PASS if implementation reuses existing auth/policy/gate paths.
- PROV-001: PASS. Provider credential authorization is audited without expanding provider semantics.
- UI-FIL-001: PASS if any visibility alignment stays Filament-native and avoids assets/ad-hoc styling.
## Initial Read-Only Repo Findings
### RBAC Inventory
| Role | Current inspected sensitive capabilities | Initial match assessment | Action |
|---|---|---|---|
| Workspace Owner | `workspace_membership.manage`, `tenant_membership.manage`, provider manage/dedicated, review/evidence manage, finding exception approve | Mostly expected | Keep unless tests reveal bypass |
| Workspace Manager | `workspace_membership.manage`, appended `tenant_membership.manage`, provider manage, review/evidence manage, finding exception approve | High-risk contradiction for membership; other manage grants need classification | Add tests, fix membership if Owner-only confirmed |
| Workspace Operator | membership view, provider view/run, review/evidence view, findings triage, audit view | Mostly expected | Prove no owner-only mutation |
| Workspace Readonly | workspace/settings/alerts/baselines/audit view plus tenant/review/evidence/provider view via merged tenant role | Mostly expected but verify customer-safe download/mutation boundaries | Prove read-only cannot mutate |
| Platform/System | `PlatformUser` + `PlatformCapabilities`, separate `platform` guard | Expected separate plane | Prove no ordinary workspace access to `/system` and no implicit customer data access |
### Constitution vs Runtime
- `apps/platform/app/Services/Auth/WorkspaceRoleCapabilityMap.php` currently grants Manager `Capabilities::WORKSPACE_MEMBERSHIP_MANAGE`.
- `WorkspaceRoleCapabilityMap::getCapabilities()` appends `Capabilities::TENANT_MEMBERSHIP_MANAGE` for Manager.
- `apps/platform/app/Services/Auth/RoleCapabilityMap.php` does not grant tenant Manager `TENANT_MEMBERSHIP_MANAGE`, which makes the WorkspaceRoleCapabilityMap append the direct source of the tenant-membership Manager grant.
- `.specify/memory/constitution.md` states: "Manager ... MUST NOT manage tenant memberships (Owner-only)."
- Existing tests currently assert Manager can manage tenant membership:
- `apps/platform/tests/Unit/Auth/CapabilityResolverTest.php`
- `apps/platform/tests/Feature/Rbac/RoleMatrix/ManagerAccessTest.php`
### Panel Boundary Findings
- `apps/platform/app/Models/User.php::canAccessPanel()` currently returns `true`.
- Admin panel uses `web` guard plus `ensure-correct-guard:web`, `ensure-workspace-selected`, and `ensure-filament-tenant-selected`.
- System panel uses `authGuard('platform')`, `ensure-correct-guard:platform`, and `ensure-platform-capability:platform.access_system_panel`.
- Existing tests already cover several system-panel cross-plane denials:
- `apps/platform/tests/Feature/Auth/CrossScopeAccessTest.php`
- `apps/platform/tests/Feature/System/Spec113/AuthorizationSemanticsTest.php`
- `apps/platform/tests/Feature/System/Spec114/SystemConsoleAccessSemanticsTest.php`
- Spec 309 still needs a dedicated boundary test family tying those semantics to the RBAC matrix and current `canAccessPanel()` posture.
## Proposed Minimal Fix Strategy
1. Start with static tests for `WorkspaceRoleCapabilityMap` and `RoleCapabilityMap`.
2. If Constitution Owner-only semantics are confirmed, remove Manager's `WORKSPACE_MEMBERSHIP_MANAGE` grant and the Manager-specific appended `TENANT_MEMBERSHIP_MANAGE`.
3. Update existing tests that assert Manager membership management so they become denial tests.
4. Verify whether `User::canAccessPanel()` can be tightened without breaking admin login/workspace chooser flow. If not, document the middleware/policy proof and add direct route tests.
5. Add or adjust policy/action tests for confirmed direct-access bypasses only.
6. Align Filament visibility only after server-side policy behavior is correct.
7. Do not change provider/review/evidence/finding-operation grants unless the audit classifies them as confirmed security bugs rather than product decisions.
## Project Structure
### Documentation (this feature)
```text
specs/309-rbac-role-matrix-access-boundary-audit/
|-- spec.md
|-- plan.md
|-- tasks.md
`-- checklists/
`-- requirements.md
```
### Likely Source Code Surfaces For Later Implementation
```text
apps/platform/app/Support/Auth/
|-- Capabilities.php
|-- PlatformCapabilities.php
`-- WorkspaceRole.php
apps/platform/app/Support/
`-- TenantRole.php
apps/platform/app/Services/Auth/
|-- WorkspaceRoleCapabilityMap.php
`-- RoleCapabilityMap.php
apps/platform/app/Models/
|-- User.php
`-- PlatformUser.php
apps/platform/app/Providers/
|-- AuthServiceProvider.php
`-- Filament/
|-- AdminPanelProvider.php
`-- SystemPanelProvider.php
apps/platform/app/Policies/
|-- OperationRunPolicy.php
|-- ProviderConnectionPolicy.php
|-- EnvironmentReviewPolicy.php
|-- ReviewPackPolicy.php
|-- EvidenceSnapshotPolicy.php
`-- FindingExceptionPolicy.php
apps/platform/tests/Unit/Auth/
apps/platform/tests/Feature/Rbac/
apps/platform/tests/Feature/Auth/
apps/platform/tests/Feature/System/
apps/platform/tests/Feature/ProviderConnections/
apps/platform/tests/Feature/ReviewPack/
apps/platform/tests/Feature/Reviews/
apps/platform/tests/Feature/Findings/
apps/platform/tests/Feature/Operations/
```
**Structure Decision**: Use existing Laravel/Filament/Pest test families where equivalent coverage exists. Create new `tests/Feature/Rbac/*BoundaryTest.php` files only when no repo-real focused equivalent exists.
## Technical Approach
1. Inventory capabilities from `Capabilities::all()` and `PlatformCapabilities::all()`.
2. Inventory role grants from `WorkspaceRoleCapabilityMap::getCapabilities()` and `RoleCapabilityMap::getCapabilities()`.
3. Categorize capabilities into workspace administration, managed-environment administration, provider connections, findings/exceptions, reviews, review packs, evidence, OperationRuns, reports, support requests, and system panel.
4. Compare membership, credential, lifecycle, review/export, and operation actions against Constitution RBAC-UX statements.
5. Add focused tests for confirmed mismatches and direct route/action boundaries.
6. Apply only the smallest map/policy/panel/resource changes needed for confirmed bugs.
7. Run focused regressions for affected review, review-pack, provider, finding, and operation lanes.
8. Close out with role inventory, fixed contradictions, deferred product decisions, and validation results.
## Data / Model Implications
- No migration expected.
- No new model expected.
- No new capability persistence expected.
- Static role-to-capability maps may change.
- Existing `workspace_memberships` and `managed_environment_memberships` data is not migrated by default.
## Policy / Authorization Implications
- `WorkspaceMembershipPolicy` and `WorkspaceRoleCapabilityMap` are the primary owner-only membership-management proof targets.
- Provider credential actions should continue to rely on `ProviderConnectionPolicy`.
- Review and Review Pack actions should continue to rely on `EnvironmentReviewPolicy` and `ReviewPackPolicy`.
- FindingException lifecycle actions should continue to rely on `FindingExceptionPolicy`.
- OperationRun visibility/action access should continue to rely on `OperationRunPolicy` and `OperationRunCapabilityResolver`.
- Service-level mutations must call Gate/Policy where UI actions execute state changes.
## Filament v5 / Livewire v4 Compliance Notes
- Filament is v5.2.1 and Livewire is v4.1.4.
- Panel providers are registered in `apps/platform/bootstrap/providers.php`, not `bootstrap/app.php`.
- This spec does not add a new panel provider.
- Any changed globally searchable resource must either keep an Edit/View page or disable global search.
- Destructive actions must execute via `Action::make(...)->action(...)`, include `->requiresConfirmation()`, and still authorize server-side.
- No new assets are planned. Existing panel theme assets remain resolved through `PanelThemeAsset`; deployment should keep the existing `cd apps/platform && php artisan filament:assets` step where registered Filament assets are published.
## Test Strategy
### New Or Updated Focused Tests
- `apps/platform/tests/Unit/Auth/WorkspaceRoleCapabilityMapTest.php`
- `apps/platform/tests/Feature/Rbac/WorkspaceRoleCapabilityBoundaryTest.php`
- `apps/platform/tests/Feature/Rbac/AdminPanelAccessBoundaryTest.php`
- `apps/platform/tests/Feature/Rbac/SystemPanelAccessBoundaryTest.php`
- `apps/platform/tests/Feature/Rbac/ManagedEnvironmentAccessBoundaryTest.php`
- `apps/platform/tests/Feature/Rbac/ProviderConnectionAccessBoundaryTest.php`
- `apps/platform/tests/Feature/Rbac/ReviewPackAccessBoundaryTest.php`
- `apps/platform/tests/Feature/Rbac/OperationRunAccessBoundaryTest.php`
- `apps/platform/tests/Feature/Rbac/FindingExceptionLifecycleAccessBoundaryTest.php`
Use existing tests instead when they already provide the exact proof and update names only if a new focused file improves reviewability.
### Regression Lanes
- Review/customer workspace:
- `apps/platform/tests/Feature/Reviews/CustomerReviewWorkspaceAuthorizationTest.php`
- `apps/platform/tests/Feature/Reviews/CustomerReviewWorkspacePackAccessTest.php`
- ReviewPack:
- `apps/platform/tests/Feature/ReviewPack/ReviewPackRbacTest.php`
- `apps/platform/tests/Feature/ReviewPack/ReviewPackDownloadTest.php`
- ProviderConnection:
- existing `apps/platform/tests/Feature/ProviderConnections/*Authorization*` and credential security tests if provider boundaries change.
- OperationRun:
- `apps/platform/tests/Feature/Rbac/OperationRunWorkspaceFirstAuthorizationTest.php`
- `apps/platform/tests/Feature/Operations/TenantlessOperationRunViewerTest.php`
- existing operation link/legacy route guard tests if policy or links change.
### Validation Commands
```bash
cd apps/platform && ./vendor/bin/sail artisan test --compact \
tests/Unit/Auth/WorkspaceRoleCapabilityMapTest.php \
tests/Feature/Rbac/WorkspaceRoleCapabilityBoundaryTest.php \
tests/Feature/Rbac/AdminPanelAccessBoundaryTest.php \
tests/Feature/Rbac/SystemPanelAccessBoundaryTest.php \
tests/Feature/Rbac/ManagedEnvironmentAccessBoundaryTest.php \
tests/Feature/Rbac/ProviderConnectionAccessBoundaryTest.php \
tests/Feature/Rbac/ReviewPackAccessBoundaryTest.php \
tests/Feature/Rbac/OperationRunAccessBoundaryTest.php \
tests/Feature/Rbac/FindingExceptionLifecycleAccessBoundaryTest.php
```
```bash
cd apps/platform && ./vendor/bin/sail artisan test --compact \
tests/Feature/Reviews/CustomerReviewWorkspaceAuthorizationTest.php \
tests/Feature/Reviews/CustomerReviewWorkspacePackAccessTest.php \
tests/Feature/ReviewPack/ReviewPackRbacTest.php \
tests/Feature/ReviewPack/ReviewPackDownloadTest.php
```
If OperationRun policy or links are touched:
```bash
cd apps/platform && ./vendor/bin/sail artisan test --compact \
tests/Feature/Rbac/OperationRunWorkspaceFirstAuthorizationTest.php \
tests/Feature/Operations/TenantlessOperationRunViewerTest.php \
tests/Feature/Guards/OperationRunLinkContractGuardTest.php
```
Final:
```bash
cd apps/platform && ./vendor/bin/sail bin pint --dirty --format agent
git diff --check
```
## Implementation Phases
1. **Read-only inventory**: confirm role maps, panel guards, policies, routes, resources, and existing tests.
2. **Classification**: distinguish confirmed security bugs, product decisions, docs drift, UI-only mismatches, and out-of-scope findings.
3. **Tests first**: add failing tests for confirmed contradictions.
4. **Minimal fixes**: adjust maps/policies/panel/resource access only where confirmed.
5. **Regression validation**: run focused affected lanes.
6. **Close-out**: record inventory, fixed contradictions, deferred decisions, tests, and remaining risks.
## Risk Controls
- Do not remove Manager capabilities unrelated to membership unless classified as confirmed bugs.
- Do not tighten `canAccessPanel()` until direct route/middleware behavior and workspace chooser flow are understood.
- Do not implement support impersonation, break-glass redesign, or system-to-workspace implicit access.
- Do not add new capability aliases.
- Do not create migrations unless the existing model cannot represent the confirmed fix.
- Do not broaden test helpers or factories globally.
## Rollout / Deployment Considerations
- No migration expected.
- No env vars expected.
- No queue/cron changes expected.
- No storage volume changes expected.
- No frontend build expected.
- Any changed authorization behavior must be validated in Staging before Production promotion.
- Existing Dokploy deployment should keep current asset publication and cache-clearing steps; no new asset step is introduced by this spec.
## Complexity Tracking
| Violation | Why Needed | Simpler Alternative Rejected Because |
|---|---|---|
| None planned | N/A | N/A |
## Open Product Decisions To Preserve If Unclear
- Manager membership-management authority.
- Manager managed-environment access-scope authority.
- Manager provider credential rotation/delete authority.
- Manager accepted-risk approval authority.
- Readonly Review Pack download authority.
- System/platform support access to customer workspaces.
Unclear items must be recorded as product-decision-needed rather than fixed blindly.