205 lines
11 KiB
Markdown
205 lines
11 KiB
Markdown
# Implementation Plan: 083-required-permissions-hardening
|
|
|
|
**Branch**: `083-required-permissions-hardening` | **Date**: 2026-02-08 | **Spec**: [spec.md](spec.md)
|
|
**Input**: Feature specification from [spec.md](spec.md)
|
|
|
|
**Note**: This template is filled in by the `/speckit.plan` command. See `.specify/scripts/` for helper scripts.
|
|
|
|
## Summary
|
|
|
|
Harden the canonical Required Permissions manage surface so it is only accessible via `GET /admin/tenants/{tenant}/required-permissions`, enforces deny-as-not-found (404) when the actor is not workspace-member or not tenant-entitled, removes any cross-plane tenant-context fallback, and presents issues-first UX using **stored DB data only** (no provider calls on render).
|
|
|
|
Research decisions are captured in [research.md](research.md).
|
|
|
|
## Technical Context
|
|
|
|
<!--
|
|
ACTION REQUIRED: Replace the content in this section with the technical details
|
|
for the project. The structure here is presented in advisory capacity to guide
|
|
the iteration process.
|
|
-->
|
|
|
|
**Language/Version**: PHP 8.4.15 (Laravel 12)
|
|
**Primary Dependencies**: Filament v5 + Livewire v4, PostgreSQL, Tailwind CSS v4
|
|
**Storage**: PostgreSQL (Sail)
|
|
**Testing**: Pest v4 (run via Sail)
|
|
**Target Platform**: Web app (Laravel) running in Docker via Sail
|
|
**Project Type**: Web application (Laravel + Filament admin panel)
|
|
**Performance Goals**: Fast, DB-only page render (no outbound HTTP / Graph calls)
|
|
**Constraints**: Strict 404 vs 403 semantics (deny-as-not-found), no cross-plane tenant fallback
|
|
**Scale/Scope**: Single page hardening + view-model/UX changes + targeted tests
|
|
|
|
## Constitution Check
|
|
|
|
*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.*
|
|
|
|
- Inventory-first: clarify what is “last observed” vs snapshots/backups
|
|
- Read/write separation: any writes require preview + confirmation + audit + tests
|
|
- Graph contract path: Graph calls only via `GraphClientInterface` + `config/graph_contracts.php`
|
|
- Deterministic capabilities: capability derivation is testable (snapshot/golden tests)
|
|
- RBAC-UX: manage surface (`/admin/tenants/...`), tenant plane (`/admin/t/{tenant}/...`), and platform plane (`/system/...`) remain clearly separated; cross-plane access is 404; non-member tenant access is 404; member-but-missing-capability is 403; authorization checks use Gates/Policies + capability registries (no raw strings, no role-string checks)
|
|
- RBAC-UX: destructive-like actions require `->requiresConfirmation()` and clear warning text
|
|
- RBAC-UX: global search is tenant-scoped; non-members get no hints; inaccessible results are treated as not found (404 semantics)
|
|
- Tenant isolation: all reads/writes tenant-scoped; cross-tenant views are explicit and access-checked
|
|
- Run observability: long-running/remote/queued work creates/reuses `OperationRun`; start surfaces enqueue-only; Monitoring is DB-only; DB-only <2s actions may skip runs but security-relevant ones still audit-log; auth handshake exception OPS-EX-AUTH-001 allows synchronous outbound HTTP on `/auth/*` without `OperationRun`
|
|
- Automation: queued/scheduled ops use locks + idempotency; handle 429/503 with backoff+jitter
|
|
- Data minimization: Inventory stores metadata + whitelisted meta; logs contain no secrets/tokens
|
|
- Badge semantics (BADGE-001): status-like badges use `BadgeCatalog` / `BadgeRenderer`; no ad-hoc mappings; new values include tests
|
|
- Filament UI Action Surface Contract: for any new/modified Filament Resource/RelationManager/Page, define Header/Row/Bulk/Empty-State actions, ensure every List/Table has a record inspection affordance (prefer `recordUrl()` clickable rows; do not render a lone View row action), keep max 2 visible row actions with the rest in “More”, group bulk actions, require confirmations for destructive actions (typed confirmation for large/bulk where applicable), write audit logs for mutations, enforce RBAC via central helpers (non-member 404, member missing capability 403), and ensure CI blocks merges if the contract is violated or not explicitly exempted
|
|
|
|
### Gate evaluation (pre-design)
|
|
|
|
- **Inventory-first / DB-only**: PASS. This surface renders from stored `tenant_permissions` only.
|
|
- **Read/write separation**: PASS. The page is read-only; it only links to mutation surfaces.
|
|
- **Graph contract path**: PASS. No Graph calls on render; any verification runs remain elsewhere.
|
|
- **Deterministic capabilities**: PASS. Access is entitlement-based via tenant membership; capability checks remain on mutation surfaces.
|
|
- **RBAC-UX semantics**: PASS (planned). Implement explicit 404 denial for non-members/non-entitled and remove implicit tenant fallback.
|
|
- **BADGE-001**: PASS (planned). Use existing overall status enum values (`Blocked`, `NeedsAttention`, `Ready`) and render via existing badge mechanisms.
|
|
- **Filament Action Surface Contract**: PASS (exempt-by-design). This is a Filament Page (not a List/Table CRUD surface). It has no row/bulk actions; it is read-only and link-only.
|
|
|
|
## Project Structure
|
|
|
|
### Documentation (this feature)
|
|
|
|
```text
|
|
specs/[###-feature]/
|
|
├── plan.md # This file (/speckit.plan command output)
|
|
├── research.md # Phase 0 output (/speckit.plan command)
|
|
├── data-model.md # Phase 1 output (/speckit.plan command)
|
|
├── quickstart.md # Phase 1 output (/speckit.plan command)
|
|
├── contracts/ # Phase 1 output (/speckit.plan command)
|
|
└── tasks.md # Phase 2 output (/speckit.tasks command - NOT created by /speckit.plan)
|
|
```
|
|
|
|
### Source Code (repository root)
|
|
<!--
|
|
ACTION REQUIRED: Replace the placeholder tree below with the concrete layout
|
|
for this feature. Delete unused options and expand the chosen structure with
|
|
real paths (e.g., apps/admin, packages/something). The delivered plan must
|
|
not include Option labels.
|
|
-->
|
|
|
|
```text
|
|
app/
|
|
├── Filament/
|
|
│ ├── Pages/
|
|
│ │ └── TenantRequiredPermissions.php
|
|
│ └── Pages/Workspaces/
|
|
│ └── ManagedTenantOnboardingWizard.php # Start verification surface (CTA target)
|
|
├── Models/
|
|
│ ├── Tenant.php
|
|
│ ├── TenantPermission.php
|
|
│ ├── TenantMembership.php
|
|
│ ├── WorkspaceMembership.php
|
|
│ └── User.php
|
|
└── Services/
|
|
├── Auth/CapabilityResolver.php
|
|
└── Intune/
|
|
├── TenantPermissionService.php
|
|
└── TenantRequiredPermissionsViewModelBuilder.php
|
|
|
|
resources/
|
|
└── views/
|
|
└── filament/pages/tenant-required-permissions.blade.php
|
|
|
|
tests/
|
|
├── Feature/
|
|
│ ├── RequiredPermissions/ # to be created in Phase 2
|
|
│ │ ├── RequiredPermissionsAccessTest.php
|
|
│ │ ├── RequiredPermissionsDbOnlyRenderTest.php
|
|
│ │ ├── RequiredPermissionsEmptyStateTest.php
|
|
│ │ ├── RequiredPermissionsLegacyRouteTest.php
|
|
│ │ └── RequiredPermissionsLinksTest.php
|
|
└── Unit/
|
|
├── TenantRequiredPermissionsFreshnessTest.php
|
|
└── TenantRequiredPermissionsOverallStatusTest.php
|
|
```
|
|
|
|
**Structure Decision**: Web application (Laravel + Filament admin panel). Changes are localized to the Filament Page, its view-model builder, Blade view, and new targeted tests.
|
|
|
|
## Complexity Tracking
|
|
|
|
> **Fill ONLY if Constitution Check has violations that must be justified**
|
|
|
|
| Violation | Why Needed | Simpler Alternative Rejected Because |
|
|
|-----------|------------|-------------------------------------|
|
|
| [e.g., 4th project] | [current need] | [why 3 projects insufficient] |
|
|
| [e.g., Repository pattern] | [specific problem] | [why direct DB access insufficient] |
|
|
|
|
## Phase 0 — Outline & Research (complete)
|
|
|
|
- Consolidated repo reality (existing canonical route, current tenant resolution fallback, current view-model behavior) and made explicit decisions in [research.md](research.md).
|
|
- No remaining NEEDS CLARIFICATION items for Spec 083.
|
|
|
|
## Phase 1 — Design & Contracts (complete)
|
|
|
|
- Data model notes captured in [data-model.md](data-model.md).
|
|
- Route/semantics contract captured in [contracts/routes.md](contracts/routes.md).
|
|
- Developer quickstart captured in [quickstart.md](quickstart.md).
|
|
|
|
## Constitution Check (post-design re-check)
|
|
|
|
- **Tenant isolation / deny-as-not-found**: PASS (design enforces explicit 404 for non-member/non-entitled).
|
|
- **Cross-plane separation**: PASS (design removes `Tenant::current()` fallback on this surface).
|
|
- **Read/write separation**: PASS (read-only page; mutation remains capability-gated on other surfaces).
|
|
- **DB-only render**: PASS (stored `tenant_permissions` + derived freshness).
|
|
- **Filament action contract**: PASS (page is read-only; no list/table actions introduced).
|
|
|
|
## Phase 1 — Agent context update (required)
|
|
|
|
Run:
|
|
|
|
```bash
|
|
.specify/scripts/bash/update-agent-context.sh copilot
|
|
```
|
|
|
|
## Phase 2 — Implementation plan (input for tasks.md)
|
|
|
|
1. **Authorization + 404 semantics (page entry)**
|
|
- Update `App\Filament\Pages\TenantRequiredPermissions` to enforce deny-as-not-found (404) when:
|
|
- workspace not selected / tenant not found / tenant not in workspace
|
|
- actor not workspace member
|
|
- actor not tenant-entitled (`User::canAccessTenant($tenant)` false)
|
|
- Ensure the checks run on initial page mount, not only in navigation gating.
|
|
|
|
2. **Remove cross-plane tenant fallback**
|
|
- Make `resolveScopedTenant()` strict: only resolve from route `{tenant}` (bound model or `external_id` lookup). If absent/invalid → treat as not found.
|
|
|
|
3. **DB-only render guarantees**
|
|
- Confirm the view-model builder continues to call `TenantPermissionService::compare(... liveCheck:false ...)`.
|
|
- Add tests to ensure no outbound HTTP is performed during render.
|
|
|
|
4. **Issues-first UX + canonical CTAs**
|
|
- Update the Blade view to present:
|
|
- Summary (overall, counts, freshness)
|
|
- Issues (Blockers + Warnings only; no separate “Error” category)
|
|
- Passed / Technical details (de-emphasized, Technical collapsed by default)
|
|
- Add a dedicated empty-data state (“Keine Daten verfügbar”) with a links-only CTA to start verification.
|
|
- Update “Re-run verification” / “Start verification” link-only CTA to point canonical to `/admin/onboarding` via route helper generation.
|
|
|
|
5. **Freshness / stale detection**
|
|
- Extend the view-model to include:
|
|
- `last_refreshed_at` derived from stored `tenant_permissions.last_checked_at` (max)
|
|
- `is_stale` (missing OR > 30 days)
|
|
- Update overall status derivation to include stale as a warning.
|
|
|
|
6. **Tests (Pest) — minimum set**
|
|
- Feature tests for:
|
|
- 404 for non-workspace-member
|
|
- 404 for workspace-member but not tenant-entitled
|
|
- 200 for tenant-entitled read-only
|
|
- empty-data state (“Keine Daten verfügbar”) with canonical start-verification CTA
|
|
- 404 for legacy route `/admin/t/{tenant}/required-permissions`
|
|
- 404 when route tenant missing/invalid (no fallback)
|
|
- Summary status mapping + stale threshold
|
|
- Technical details rendered after Issues/Passed and collapsed by default
|
|
- “Re-run verification” links to `/admin/onboarding`
|
|
|
|
7. **Scope boundary for FR-083-009**
|
|
- This feature does not modify mutation endpoints.
|
|
- Capability-based 403 enforcement remains on the linked target surfaces and is treated as an explicit dependency, not newly implemented behavior in Spec 083.
|
|
|
|
8. **Formatting + verification**
|
|
- Run `vendor/bin/sail bin pint --dirty`.
|
|
- Run the targeted tests via `vendor/bin/sail artisan test --compact ...`.
|