diff --git a/app/Filament/Pages/TenantRequiredPermissions.php b/app/Filament/Pages/TenantRequiredPermissions.php index 1c874b8..26a8a88 100644 --- a/app/Filament/Pages/TenantRequiredPermissions.php +++ b/app/Filament/Pages/TenantRequiredPermissions.php @@ -5,7 +5,6 @@ namespace App\Filament\Pages; use App\Filament\Resources\ProviderConnectionResource; -use App\Models\ProviderConnection; use App\Models\Tenant; use App\Models\User; use App\Models\WorkspaceMembership; @@ -41,34 +40,28 @@ class TenantRequiredPermissions extends Page */ public array $viewModel = []; + public ?Tenant $scopedTenant = null; + public static function canAccess(): bool { - $tenant = static::resolveScopedTenant(); - $user = auth()->user(); - - if (! $tenant instanceof Tenant || ! $user instanceof User) { - return false; - } - - $workspaceId = app(WorkspaceContext::class)->currentWorkspaceId(request()); - - if ($workspaceId === null || (int) $tenant->workspace_id !== (int) $workspaceId) { - return false; - } - - return WorkspaceMembership::query() - ->where('workspace_id', (int) $workspaceId) - ->where('user_id', (int) $user->getKey()) - ->exists(); + return static::hasScopedTenantAccess(static::resolveScopedTenant()); } public function currentTenant(): ?Tenant { - return static::resolveScopedTenant(); + return $this->scopedTenant; } public function mount(): void { + $tenant = static::resolveScopedTenant(); + + if (! $tenant instanceof Tenant || ! static::hasScopedTenantAccess($tenant)) { + abort(404); + } + + $this->scopedTenant = $tenant; + $queryFeatures = request()->query('features', $this->features); $state = TenantRequiredPermissionsViewModelBuilder::normalizeFilterState([ @@ -147,7 +140,7 @@ public function resetFilters(): void private function refreshViewModel(): void { - $tenant = static::resolveScopedTenant(); + $tenant = $this->scopedTenant; if (! $tenant instanceof Tenant) { $this->viewModel = []; @@ -174,25 +167,20 @@ private function refreshViewModel(): void } } - public function reRunVerificationUrl(): ?string + public function reRunVerificationUrl(): string { - $tenant = static::resolveScopedTenant(); + return route('admin.onboarding'); + } + + public function manageProviderConnectionUrl(): ?string + { + $tenant = $this->scopedTenant; if (! $tenant instanceof Tenant) { return null; } - $connectionId = ProviderConnection::query() - ->where('tenant_id', (int) $tenant->getKey()) - ->orderByDesc('is_default') - ->orderByDesc('id') - ->value('id'); - - if (! is_int($connectionId)) { - return ProviderConnectionResource::getUrl('index', ['tenant' => $tenant], panel: 'admin'); - } - - return ProviderConnectionResource::getUrl('edit', ['tenant' => $tenant, 'record' => $connectionId], panel: 'admin'); + return ProviderConnectionResource::getUrl('index', ['tenant' => $tenant], panel: 'admin'); } protected static function resolveScopedTenant(): ?Tenant @@ -209,6 +197,32 @@ protected static function resolveScopedTenant(): ?Tenant ->first(); } - return Tenant::current(); + return null; + } + + private static function hasScopedTenantAccess(?Tenant $tenant): bool + { + $user = auth()->user(); + + if (! $tenant instanceof Tenant || ! $user instanceof User) { + return false; + } + + $workspaceId = app(WorkspaceContext::class)->currentWorkspaceId(request()); + + if ($workspaceId === null || (int) $tenant->workspace_id !== (int) $workspaceId) { + return false; + } + + $isWorkspaceMember = WorkspaceMembership::query() + ->where('workspace_id', (int) $workspaceId) + ->where('user_id', (int) $user->getKey()) + ->exists(); + + if (! $isWorkspaceMember) { + return false; + } + + return $user->canAccessTenant($tenant); } } diff --git a/app/Services/Intune/TenantPermissionService.php b/app/Services/Intune/TenantPermissionService.php index 31a47de..ec72650 100644 --- a/app/Services/Intune/TenantPermissionService.php +++ b/app/Services/Intune/TenantPermissionService.php @@ -5,6 +5,8 @@ use App\Models\Tenant; use App\Models\TenantPermission; use App\Services\Graph\GraphClientInterface; +use DateTimeInterface; +use Illuminate\Support\Carbon; class TenantPermissionService { @@ -44,6 +46,7 @@ public function getGrantedPermissions(Tenant $tenant): array * @return array{ * overall_status:string, * permissions:array,status:string,details:array|null}>, + * last_refreshed_at:?string, * live_check?: array{attempted:bool,succeeded:bool,http_status:?int,reason_code:?string} * } */ @@ -210,6 +213,7 @@ public function compare( $payload = [ 'overall_status' => $overall, 'permissions' => $results, + 'last_refreshed_at' => $this->lastRefreshedAtIso($tenant), ]; if ($liveCheckMeta['attempted'] === true) { @@ -389,4 +393,25 @@ private function fetchLivePermissions(Tenant $tenant, ?array $graphOptions = nul ]; } } + + private function lastRefreshedAtIso(Tenant $tenant): ?string + { + $lastCheckedAt = TenantPermission::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->max('last_checked_at'); + + if ($lastCheckedAt instanceof DateTimeInterface) { + return Carbon::instance($lastCheckedAt)->toIso8601String(); + } + + if (is_string($lastCheckedAt) && $lastCheckedAt !== '') { + try { + return Carbon::parse($lastCheckedAt)->toIso8601String(); + } catch (\Throwable) { + return null; + } + } + + return null; + } } diff --git a/app/Services/Intune/TenantRequiredPermissionsViewModelBuilder.php b/app/Services/Intune/TenantRequiredPermissionsViewModelBuilder.php index 98d7aaa..a6d9a5a 100644 --- a/app/Services/Intune/TenantRequiredPermissionsViewModelBuilder.php +++ b/app/Services/Intune/TenantRequiredPermissionsViewModelBuilder.php @@ -4,6 +4,8 @@ use App\Models\Tenant; use App\Support\Verification\VerificationReportOverall; +use Carbon\CarbonInterface; +use Illuminate\Support\Carbon; class TenantRequiredPermissionsViewModelBuilder { @@ -16,7 +18,8 @@ class TenantRequiredPermissionsViewModelBuilder * overview: array{ * overall: string, * counts: array{missing_application:int,missing_delegated:int,present:int,error:int}, - * feature_impacts: array + * feature_impacts: array, + * freshness: array{last_refreshed_at:?string,is_stale:bool} * }, * permissions: array, * filters: FilterState, @@ -48,6 +51,7 @@ public function build(Tenant $tenant, array $filters = []): array $state = self::normalizeFilterState($filters); $filteredPermissions = self::applyFilterState($allPermissions, $state); + $freshness = self::deriveFreshness(self::parseLastRefreshedAt($comparison['last_refreshed_at'] ?? null)); return [ 'tenant' => [ @@ -56,9 +60,10 @@ public function build(Tenant $tenant, array $filters = []): array 'name' => (string) $tenant->name, ], 'overview' => [ - 'overall' => self::deriveOverallStatus($allPermissions), + 'overall' => self::deriveOverallStatus($allPermissions, (bool) ($freshness['is_stale'] ?? true)), 'counts' => self::deriveCounts($allPermissions), 'feature_impacts' => self::deriveFeatureImpacts($allPermissions), + 'freshness' => $freshness, ], 'permissions' => $filteredPermissions, 'filters' => $state, @@ -72,7 +77,7 @@ public function build(Tenant $tenant, array $filters = []): array /** * @param array $permissions */ - public static function deriveOverallStatus(array $permissions): string + public static function deriveOverallStatus(array $permissions, bool $hasStaleFreshness = false): string { $hasMissingApplication = collect($permissions)->contains( fn (array $row): bool => $row['status'] === 'missing' && $row['type'] === 'application', @@ -90,13 +95,35 @@ public static function deriveOverallStatus(array $permissions): string fn (array $row): bool => $row['status'] === 'missing' && $row['type'] === 'delegated', ); - if ($hasErrors || $hasMissingDelegated) { + if ($hasErrors || $hasMissingDelegated || $hasStaleFreshness) { return VerificationReportOverall::NeedsAttention->value; } return VerificationReportOverall::Ready->value; } + /** + * @return array{last_refreshed_at:?string,is_stale:bool} + */ + public static function deriveFreshness(?CarbonInterface $lastRefreshedAt, ?CarbonInterface $referenceTime = null): array + { + $reference = $referenceTime instanceof Carbon + ? $referenceTime->copy() + : ($referenceTime !== null ? Carbon::instance($referenceTime) : now()); + + $lastRefreshed = $lastRefreshedAt instanceof Carbon + ? $lastRefreshedAt + : ($lastRefreshedAt !== null ? Carbon::instance($lastRefreshedAt) : null); + + $isStale = $lastRefreshed === null + || $lastRefreshed->lt($reference->copy()->subDays(30)); + + return [ + 'last_refreshed_at' => $lastRefreshed?->toIso8601String(), + 'is_stale' => $isStale, + ]; + } + /** * @param array $permissions * @return array{missing_application:int,missing_delegated:int,present:int,error:int} @@ -386,4 +413,25 @@ private static function normalizePermissionRow(array $row): array 'details' => $details, ]; } + + private static function parseLastRefreshedAt(mixed $value): ?Carbon + { + if ($value instanceof Carbon) { + return $value; + } + + if ($value instanceof CarbonInterface) { + return Carbon::instance($value); + } + + if (is_string($value) && $value !== '') { + try { + return Carbon::parse($value); + } catch (\Throwable) { + return null; + } + } + + return null; + } } diff --git a/resources/views/filament/pages/tenant-required-permissions.blade.php b/resources/views/filament/pages/tenant-required-permissions.blade.php index 68f5cb8..c02d51b 100644 --- a/resources/views/filament/pages/tenant-required-permissions.blade.php +++ b/resources/views/filament/pages/tenant-required-permissions.blade.php @@ -2,6 +2,7 @@ use App\Support\Badges\BadgeDomain; use App\Support\Badges\BadgeRenderer; use App\Support\Links\RequiredPermissionsLinks; + use Illuminate\Support\Carbon; $tenant = $this->currentTenant(); @@ -9,6 +10,7 @@ $overview = is_array($vm['overview'] ?? null) ? $vm['overview'] : []; $counts = is_array($overview['counts'] ?? null) ? $overview['counts'] : []; $featureImpacts = is_array($overview['feature_impacts'] ?? null) ? $overview['feature_impacts'] : []; + $freshness = is_array($overview['freshness'] ?? null) ? $overview['freshness'] : []; $filters = is_array($vm['filters'] ?? null) ? $vm['filters'] : []; $selectedFeatures = is_array($filters['features'] ?? null) ? $filters['features'] : []; @@ -47,17 +49,77 @@ $adminConsentLabel = $adminConsentUrl ? 'Open admin consent' : 'Admin consent guide'; $reRunUrl = $this->reRunVerificationUrl(); + $manageProviderConnectionUrl = $this->manageProviderConnectionUrl(); + $lastRefreshedAt = is_string($freshness['last_refreshed_at'] ?? null) ? (string) $freshness['last_refreshed_at'] : null; + $lastRefreshedLabel = $lastRefreshedAt ? Carbon::parse($lastRefreshedAt)->diffForHumans() : 'Unknown'; + $isStale = (bool) ($freshness['is_stale'] ?? true); + $hasStoredPermissionData = $lastRefreshedAt !== null; + + $issues = []; + + if ($missingApplication > 0) { + $issues[] = [ + 'severity' => 'Blocker', + 'title' => 'Missing application permissions', + 'description' => "{$missingApplication} required application permission(s) are missing.", + 'links' => array_values(array_filter([ + ['label' => $adminConsentLabel, 'url' => $adminConsentPrimaryUrl, 'external' => true], + $manageProviderConnectionUrl ? ['label' => 'Manage provider connection', 'url' => $manageProviderConnectionUrl, 'external' => false] : null, + ['label' => 'Re-run verification', 'url' => $reRunUrl, 'external' => false], + ])), + ]; + } + + if ($missingDelegated > 0) { + $issues[] = [ + 'severity' => 'Warning', + 'title' => 'Missing delegated permissions', + 'description' => "{$missingDelegated} delegated permission(s) are missing.", + 'links' => [ + ['label' => $adminConsentLabel, 'url' => $adminConsentPrimaryUrl, 'external' => true], + ['label' => 'Re-run verification', 'url' => $reRunUrl, 'external' => false], + ], + ]; + } + + if ($errorCount > 0) { + $issues[] = [ + 'severity' => 'Warning', + 'title' => 'Verification results need review', + 'description' => "{$errorCount} permission row(s) are in an unknown/error state and require follow-up.", + 'links' => [ + ['label' => 'Re-run verification', 'url' => $reRunUrl, 'external' => false], + $manageProviderConnectionUrl ? ['label' => 'Manage provider connection', 'url' => $manageProviderConnectionUrl, 'external' => false] : ['label' => 'Admin consent guide', 'url' => RequiredPermissionsLinks::adminConsentGuideUrl(), 'external' => true], + ], + ]; + } + + if ($isStale) { + $issues[] = [ + 'severity' => 'Warning', + 'title' => 'Freshness warning', + 'description' => $hasStoredPermissionData + ? "Permission data is older than 30 days (last refresh {$lastRefreshedLabel})." + : 'No stored verification data is available yet.', + 'links' => [ + ['label' => 'Start verification', 'url' => $reRunUrl, 'external' => false], + ], + ]; + } @endphp
- +
Review what’s missing for this tenant and copy the missing permissions for admin consent.
+
+ Stored-data view only. Last refreshed: {{ $lastRefreshedLabel }}{{ $isStale ? ' (stale)' : '' }}. +
@if ($overallSpec) @@ -86,6 +148,16 @@
+ @if (! $hasStoredPermissionData) +
+
Keine Daten verfügbar
+
+ Für diesen Tenant liegen noch keine gespeicherten Verifikationsdaten vor. + Start verification. +
+
+ @endif +
Guidance
@@ -322,7 +394,75 @@ class="mt-4 space-y-2"
- + + @if ($issues === []) +
+ No blockers or warnings detected from stored data. +
+ @else +
+ @foreach ($issues as $issue) + @php + $severity = (string) ($issue['severity'] ?? 'Warning'); + $severityColor = $severity === 'Blocker' ? 'danger' : 'warning'; + $title = (string) ($issue['title'] ?? 'Issue'); + $description = (string) ($issue['description'] ?? ''); + $links = is_array($issue['links'] ?? null) ? $issue['links'] : []; + @endphp + +
+
+ {{ $severity }} +
{{ $title }}
+
+
{{ $description }}
+ @if ($links !== []) +
+ @foreach ($links as $link) + @php + $label = is_array($link) ? (string) ($link['label'] ?? '') : ''; + $url = is_array($link) ? (string) ($link['url'] ?? '') : ''; + $external = is_array($link) ? (bool) ($link['external'] ?? false) : false; + @endphp + @if ($label !== '' && $url !== '') + + {{ $label }} + + @endif + @endforeach +
+ @endif +
+ @endforeach +
+ @endif + + + +
+
+ {{ $presentCount }} permission(s) currently pass. +
+
+ {{ $requiredTotal > 0 ? "Out of {$requiredTotal} required permissions, {$presentCount} are currently granted." : 'No required permissions are configured yet.' }} +
+
+ + + +
+ + Expand technical details + + +
@if (! $tenant)
No tenant selected. @@ -507,6 +647,8 @@ class="align-top" @endif
@endif +
+
diff --git a/specs/083-required-permissions-hardening/checklists/requirements.md b/specs/083-required-permissions-hardening/checklists/requirements.md new file mode 100644 index 0000000..8afbb33 --- /dev/null +++ b/specs/083-required-permissions-hardening/checklists/requirements.md @@ -0,0 +1,39 @@ +# Specification Quality Checklist: Canonical Required Permissions (Manage) Hardening & Enterprise UX + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2026-02-08 +**Feature**: [specs/083-required-permissions-hardening/spec.md](../spec.md) + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +- Items marked incomplete require spec updates before `/speckit.clarify` or `/speckit.plan` + +Validation run (2026-02-08): +- Spec includes explicit 404 vs 403 semantics (deny-as-not-found for non-entitlement). +- Legacy URL non-existence is explicitly required and covered by test requirements. +- DB-only rendering constraint is explicitly required and test-covered. diff --git a/specs/083-required-permissions-hardening/contracts/routes.md b/specs/083-required-permissions-hardening/contracts/routes.md new file mode 100644 index 0000000..5e6749d --- /dev/null +++ b/specs/083-required-permissions-hardening/contracts/routes.md @@ -0,0 +1,27 @@ +# Route Contract — Spec 083 + +This contract defines the **Required Permissions** routes and their **404/403 semantics**. + +## Canonical management surface (must exist) + +- `GET /admin/tenants/{tenant}/required-permissions` + +Identifier contract: +- `{tenant}` is `Tenant.external_id` (Entra tenant GUID) + +Authorization contract: +- Not authenticated → handled by Filament auth middleware +- Workspace not selected → 404 (deny-as-not-found) +- Not a workspace member → 404 +- Workspace member but **not tenant-entitled** (no `tenant_memberships` row) → 404 +- Tenant-entitled (including read-only) → 200 + +Action contract: +- This page is read-only. Any mutations are only linked to and executed on other surfaces. +- Mutations on other surfaces must enforce capability checks server-side (missing capability → 403). +- "Re-run verification" links canonical to the start-verification surface: `GET /admin/onboarding` (generated via route helper, not hardcoded legacy paths). + +## Removed tenant-plane route (must 404) + +The following route MUST NOT exist and MUST return 404 (no redirects, no aliases): +- `GET /admin/t/{tenant}/required-permissions` diff --git a/specs/083-required-permissions-hardening/data-model.md b/specs/083-required-permissions-hardening/data-model.md new file mode 100644 index 0000000..2f67e01 --- /dev/null +++ b/specs/083-required-permissions-hardening/data-model.md @@ -0,0 +1,40 @@ +# Data Model — Spec 083 + +This feature is primarily **read-only UX + authorization hardening**. No new tables are required. + +## Existing entities (relevant) + +### Workspace +- **Purpose**: Isolation boundary for tenant management surfaces. +- **Key fields**: `id`. + +### WorkspaceMembership +- **Purpose**: Establishes user membership in a workspace. +- **Key fields**: `workspace_id`, `user_id`, `role`. + +### Tenant +- **Purpose**: Managed Entra tenant (scoped to a workspace). +- **Key fields**: `id`, `external_id` (Entra tenant GUID), `workspace_id`, `status`, `name`. + +### TenantMembership +- **Purpose**: Tenant entitlement (read-only access at minimum). +- **Key fields**: `tenant_id`, `user_id`, `role`, `source`, `source_ref`. + +### TenantPermission +- **Purpose**: Stored permission inventory used by Required Permissions page. +- **Key fields**: `tenant_id`, `permission_key`, `status` (`granted|missing|error`), `details` (JSON), `last_checked_at`. + +## Derived / computed values + +### "Last refreshed" +- **Definition**: `max(tenant_permissions.last_checked_at)` for the tenant. +- **Stale rule** (Spec 083): stale if missing OR older than 30 days. + +### Summary overall status +Derived from stored permission rows (and freshness): +- **Blocked**: any missing `application` permission. +- **Needs attention**: any warning exists (missing delegated OR error rows folded into warning OR stale freshness). +- **Ready**: no blockers, no warnings. + +## State transitions +- None introduced here (page remains read-only). Mutations happen on other surfaces (verification start, provider connection management) and must enforce capability checks there. diff --git a/specs/083-required-permissions-hardening/plan.md b/specs/083-required-permissions-hardening/plan.md new file mode 100644 index 0000000..0a9e218 --- /dev/null +++ b/specs/083-required-permissions-hardening/plan.md @@ -0,0 +1,204 @@ +# 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 + + + +**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) + + +```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 ...`. diff --git a/specs/083-required-permissions-hardening/quickstart.md b/specs/083-required-permissions-hardening/quickstart.md new file mode 100644 index 0000000..938fb3a --- /dev/null +++ b/specs/083-required-permissions-hardening/quickstart.md @@ -0,0 +1,31 @@ +# Quickstart — Spec 083 + +## Dev prerequisites +- Run via Sail (local): Docker + `vendor/bin/sail` available. + +## What to validate + +### Route semantics +- Canonical (must exist): `GET /admin/tenants/{tenant}/required-permissions` +- Legacy (must 404): `GET /admin/t/{tenant}/required-permissions` + +### Authorization semantics +- Non-workspace-member → 404 +- Workspace-member but not tenant-entitled → 404 +- Tenant-entitled (including read-only) → 200 + +### Render behavior +- Page render uses stored DB data only (no Graph / no outbound HTTP). +- If no stored permission data exists, page shows "Keine Daten verfügbar" with a canonical CTA to `/admin/onboarding`. +- "Technical details" appears after Issues/Passed and is collapsed by default. + +## Run targeted tests +- `vendor/bin/sail artisan test --compact tests/Feature/RequiredPermissions/*` + - (Exact filenames to be created in Phase 2 tasks.) + +## Manual smoke test +1. Log in to admin panel. +2. Select a workspace. +3. Open `/admin/tenants/{external_id}/required-permissions` for a tenant you are a member of. +4. Confirm Summary + Issues-first layout and that "Re-run verification" links to `/admin/onboarding`. +5. As a user without tenant entitlement, confirm the same URL returns 404. diff --git a/specs/083-required-permissions-hardening/research.md b/specs/083-required-permissions-hardening/research.md new file mode 100644 index 0000000..6830888 --- /dev/null +++ b/specs/083-required-permissions-hardening/research.md @@ -0,0 +1,63 @@ +# Research — Spec 083 (Required Permissions hardening) + +## Context recap +- Canonical manage-plane surface already exists as a Filament Page: `App\Filament\Pages\TenantRequiredPermissions` with slug `tenants/{tenant}/required-permissions` (admin panel). +- A legacy tenant-plane path prefix exists (`/admin/t/...`) via the tenant panel; spec requires `/admin/t/{tenant}/required-permissions` to remain non-existent and return 404. + +## Decisions + +### Decision 1 — Canonical route stays `/admin/tenants/{tenant}/required-permissions` +- **Chosen**: Keep the canonical manage URL exactly as specified in Spec 083. +- **Rationale**: Already aligned with the existing page slug and with the established route contract in Spec 080. +- **Alternatives considered**: + - Redirect from `/admin/t/...` → rejected (spec requires 404, no redirect). + +### Decision 2 — Deny-as-not-found is implemented explicitly (404), not via “canAccess() only” +- **Chosen**: Enforce 404 using explicit `abort(404)` checks on request entry (e.g., `mount()`), instead of relying solely on Filament’s `canAccess()` return value. +- **Rationale**: Filament’s `canAccess()` may produce behavior that is not guaranteed to be a 404. Spec 083 requires strict 404 semantics for non-members / non-entitled. +- **Alternatives considered**: + - Only `canAccess()` returning false → rejected (status code semantics uncertain). + - Route-level middleware on just this page → possible, but still needs explicit entitlement checks; can be added later if desired. + +### Decision 3 — Tenant entitlement is checked via `User::canAccessTenant($tenant)` +- **Chosen**: Use `User::canAccessTenant()` for tenant entitlement (no workspace-wide “view all tenants” override). +- **Rationale**: This matches existing patterns across the codebase, uses `tenant_memberships`, and aligns with the clarification outcome. +- **Alternatives considered**: + - Workspace membership only → rejected (Spec 083 requires tenant entitlement). + - Capability checks for read-only view → rejected (read-only access is entitlement-only; mutations are capability-gated elsewhere). + +### Decision 4 — Remove cross-plane tenant fallback (`Tenant::current()`) from this surface +- **Chosen**: `TenantRequiredPermissions::resolveScopedTenant()` must be strict: resolve only from route parameter `{tenant}` (external_id or bound model). If absent/invalid → 404. +- **Rationale**: `Tenant::current()` can leak cross-plane context and violates FR-083-007. +- **Alternatives considered**: + - Keep fallback for convenience → rejected (security hardening goal). + +### Decision 5 — DB-only render is guaranteed by using stored `tenant_permissions` +- **Chosen**: Continue using `TenantPermissionService::compare(... liveCheck:false ...)` for this page (no Graph calls). +- **Rationale**: With `liveCheck=false`, compare reads stored `tenant_permissions` only. +- **Alternatives considered**: + - Allow live-check on render → rejected (violates FR-083-010). + +### Decision 6 — Freshness (“Last refreshed”) comes from `tenant_permissions.last_checked_at` +- **Chosen**: Define page “Last refreshed” as the max timestamp of stored permission checks for the tenant. Stale if missing or older than 30 days. +- **Rationale**: This is already stored in the database and does not require provider calls. +- **Alternatives considered**: + - Use latest verification run timestamps → possible, but increases coupling; not necessary for Spec 083. + +### Decision 7 — Summary status logic is centralized in the view-model builder +- **Chosen**: Update `TenantRequiredPermissionsViewModelBuilder::deriveOverallStatus()` so: + - Blocked if any missing application permission (blocker) + - Else Needs attention if any warning exists (missing delegated, error rows folded into warning, or stale freshness) + - Else Ready +- **Rationale**: Aligns with Spec 083 summary rules and keeps mapping centralized. +- **Alternatives considered**: + - Compute in Blade view → rejected (harder to test, risks drift). + +### Decision 8 — “Re-run verification” CTA links to `/admin/onboarding` (“Start verification” surface) +- **Chosen**: Link-only CTA points to the existing onboarding wizard page (admin panel slug `onboarding`). +- **Rationale**: Clarification outcome; capability gating occurs on the start/execute surface, not on this read-only page. +- **Alternatives considered**: + - Link to provider connection edit → rejected (not the requested primary action). + +## Open questions +None remaining for Spec 083 (clarifications already settled). diff --git a/specs/083-required-permissions-hardening/spec.md b/specs/083-required-permissions-hardening/spec.md new file mode 100644 index 0000000..81f65b8 --- /dev/null +++ b/specs/083-required-permissions-hardening/spec.md @@ -0,0 +1,214 @@ +# Feature Specification: Canonical Required Permissions (Manage) Hardening & Enterprise UX + +**Feature Branch**: `083-required-permissions-hardening` +**Created**: 2026-02-08 +**Status**: Ready for implementation +**Input**: User description: "Harden the canonical Required Permissions manage surface: enforce tenant entitlement, keep legacy URL non-existent (404), remove cross-plane fallbacks, and improve issues-first UX without any provider calls." + +## Clarifications + +### Session 2026-02-08 + +- Q: Soll die optionale Workspace-weite Ausnahme „alle Tenants ansehen“ (ohne TenantMembership) Teil von Spec 083 sein? → A: Nein. Spec 083 basiert ausschließlich auf Tenant-Entitlement; kein „view all tenants“ Override. +- Q: Wie genau soll die Summary-Status-Logik (Blocked / Needs attention / Ready) definiert werden? → A: Blocked wenn mind. 1 Blocker; sonst Needs attention wenn mind. 1 Warning (inkl. stale); sonst Ready. +- Q: Ab wann gilt „Freshness“ als „stale“ (Warning)? → A: Warnung, wenn „Last refreshed“ fehlt oder älter als 30 Tage ist. +- Q: Soll die Seite einen expliziten „Error“-Issue-Typ anzeigen, oder nur Blocker/Warnings basierend auf gespeicherten Permission-Daten? → A: Kein „Error“-Issue-Typ in Spec 083. Nur Blocker (missing application) + Warnings (delegated/stale/unknown). +- Q: Wohin soll der links-only CTA „Re-run verification“ canonical führen? → A: Zur „Start verification“ Surface (Wizard/Startseite), damit ein neuer Run gestartet werden kann (capability-gated dort). + +## User Scenarios & Testing *(mandatory)* + + + +### User Story 1 - Required Permissions sicher ansehen (Priority: P1) + +Als Workspace-Mitglied mit Tenant-Entitlement möchte ich die "Required Permissions" Seite eines Tenants öffnen, um sofort zu erkennen, ob administrative Berechtigungen fehlen (Blocker) oder ob nur Hinweise/Warnings bestehen — ohne dass dadurch externe Provider-Aufrufe ausgelöst werden. + +**Why this priority**: Das ist die primäre, risikorelevante Enterprise-UX: Security- und Operations-Teams müssen schnell und sicher einschätzen können, ob Handlungsbedarf besteht. + +**Independent Test**: Kann vollständig über einen einzelnen GET-Aufruf auf die Canonical-URL getestet werden, inklusive 200/404 Semantik, UI-Sektionen und „keine externen Calls“. + +**Acceptance Scenarios**: + +1. **Given** ein User ist Workspace-Mitglied und tenant-entitled, **When** er die Canonical-URL für den Tenant öffnet, **Then** erhält er 200 und sieht eine issues-first Zusammenfassung (Summary → Issues → Passed → Technical). +2. **Given** die Seite wird aufgerufen, **When** sie gerendert wird, **Then** werden keine externen Provider-Anfragen ausgelöst (nur gespeicherte Daten werden verwendet). + +--- + +### User Story 2 - Next steps finden, ohne Mutationsrechte zu benötigen (Priority: P2) + +Als tenant-entitled User möchte ich auf der Seite klare "Next steps" sehen (links-only), um fehlende Berechtigungen zu beheben oder eine erneute Verifikation anzustoßen, ohne dass ich selbst zwingend Mutationsrechte habe. + +**Why this priority**: In Enterprise-Umgebungen sind Rollen getrennt: Viewer müssen Probleme erkennen und korrekt eskalieren können, ohne selbst Änderungen durchführen zu dürfen. + +**Independent Test**: Kann über Render-Assertions getestet werden: Issue-Karten enthalten ausschließlich Links zu passenden Folgeseiten, und die Links sind canonical. + +**Acceptance Scenarios**: + +1. **Given** es existieren Blocker/Warnings, **When** die Seite gerendert wird, **Then** enthält jede Issue eine klare, links-only Handlungsempfehlung (z.B. „Admin consent dokumentieren“, „Verifikation erneut starten“, „Provider-Verbindung verwalten“). +2. **Given** Next-step Links werden angezeigt, **When** die URLs geprüft werden, **Then** verweisen sie auf die canonical Manage-Surfaces und nicht auf Legacy-Tenant-Plane URLs. + +--- + +### User Story 3 - Tenant-Discovery verhindern (Deny-as-not-found) (Priority: P3) + +Als Security Owner möchte ich, dass Workspace-Mitglieder ohne Tenant-Entitlement weder über URL-Varianten noch über Fehlermeldungen Hinweise auf die Existenz eines Tenants oder dessen Security-Posture erhalten. + +**Why this priority**: Verhindert Tenant-Leakage und erzwingt eine konsistente Enterprise-Sicherheitsposition. + +**Independent Test**: Kann isoliert über negative Access-Tests (404 Semantik) für verschiedene Benutzerzustände getestet werden. + +**Acceptance Scenarios**: + +1. **Given** ein User ist Workspace-Mitglied ohne Tenant-Entitlement, **When** er die canonical Required-Permissions URL eines Tenants aufruft, **Then** erhält er 404 (deny-as-not-found). +2. **Given** ein User ruft eine Legacy-Tenant-Plane URL-Variante auf, **When** der Request verarbeitet wird, **Then** ist das Ergebnis 404 (keine Redirects, keine Aliases). + +--- + +### Edge Cases + +- Tenant-ID ist syntaktisch ungültig oder verweist auf keinen Tenant → 404. +- Tenant gehört nicht zum aktuell selektierten Workspace → 404. +- Workspace ist nicht selektiert / User ist kein Workspace-Mitglied → 404. +- Es existieren keine gespeicherten Daten (noch nie verifiziert / gelöscht) → Seite erklärt „keine Daten verfügbar“ und verlinkt zur Verifikation. +- Daten sind alt (stale) → Warning + Link zu „erneut verifizieren“. +- Freshness ist unbekannt (kein „Last refreshed“) → Warning + Link zu „erneut verifizieren“. + +## Requirements *(mandatory)* + +**Constitution alignment (required):** If this feature introduces any Microsoft Graph calls, any write/change behavior, +or any long-running/queued/scheduled work, the spec MUST describe contract registry updates, safety gates +(preview/confirmation/audit), tenant isolation, run observability (`OperationRun` type/identity/visibility), and tests. +If security-relevant DB-only actions intentionally skip `OperationRun`, the spec MUST describe `AuditLog` entries. + +**Constitution alignment (RBAC-UX):** If this feature introduces or changes authorization behavior, the spec MUST: +- state which authorization plane(s) are involved (tenant `/admin/t/{tenant}` vs platform `/system`), +- ensure any cross-plane access is deny-as-not-found (404), +- explicitly define 404 vs 403 semantics: + - non-member / not entitled to tenant scope → 404 (deny-as-not-found) + - member but missing capability → 403 +- describe how authorization is enforced server-side (Gates/Policies) for every mutation/operation-start/credential change, +- reference the canonical capability registry (no raw capability strings; no role-string checks in feature code), +- ensure global search is tenant-scoped and non-member-safe (no hints; inaccessible results treated as 404 semantics), +- ensure destructive-like actions require confirmation (`->requiresConfirmation()`), +- include at least one positive and one negative authorization test, and note any RBAC regression tests added/updated. + +**Constitution alignment (OPS-EX-AUTH-001):** OIDC/SAML login handshakes may perform synchronous outbound HTTP (e.g., token exchange) +on `/auth/*` endpoints without an `OperationRun`. This MUST NOT be used for Monitoring/Operations pages. + +**Constitution alignment (BADGE-001):** If this feature changes status-like badges (status/outcome/severity/risk/availability/boolean), +the spec MUST describe how badge semantics stay centralized (no ad-hoc mappings) and which tests cover any new/changed values. + +**Constitution alignment (Filament Action Surfaces):** If this feature adds or modifies any Filament Resource / RelationManager / Page, +the spec MUST include a “UI Action Matrix” (see below) and explicitly state whether the Action Surface Contract is satisfied. +If the contract is not satisfied, the spec MUST include an explicit exemption with rationale. + + + +### Functional Requirements + +#### Surfaces & Routing + +- **FR-083-001**: Die Required-Permissions Oberfläche MUSS ausschließlich auf der canonical Manage-URL verfügbar sein: `GET /admin/tenants/{tenant}/required-permissions`. +- **FR-083-002**: Eine Legacy-Tenant-Plane Variante MUSS nicht existieren und MUSS 404 liefern: `GET /admin/t/{tenant}/required-permissions` (keine Redirects, keine Aliases). + +#### Authorization (Enterprise Hardening) + +- **FR-083-003**: Die Seite MUSS deny-as-not-found (404) verwenden, wenn der User kein Workspace-Mitglied ist. +- **FR-083-004**: Die Seite MUSS deny-as-not-found (404) verwenden, wenn der User Workspace-Mitglied ist, aber kein Tenant-Entitlement besitzt. +- **FR-083-005**: Die Seite MUSS 200 liefern, wenn der User Workspace-Mitglied ist und Tenant-Entitlement besitzt (inkl. Readonly-Entitlement). +- **FR-083-006**: Der Route-Parameter `{tenant}` MUSS vorhanden sein und einem Tenant im aktuell selektierten Workspace entsprechen; fehlt der Parameter oder ist er ungültig, MUSS 404 zurückgegeben werden. +- **FR-083-007**: Die Seite MUSS strikt an den URL-Tenant gebunden sein; es darf keinen impliziten Fallback auf einen „aktuellen“ Tenant-Kontext geben. + +#### 404 vs 403 Semantik (RBAC-UX) + +- **FR-083-008**: 404-Antworten bei Membership-/Entitlement-Denial MÜSSEN generisch bleiben und dürfen keinen Ablehnungsgrund offenlegen (kein Tenant-Leakage). +- **FR-083-009**: Falls auf der Seite Aktionen/Mutations verlinkt werden (z.B. „Verifikation starten“), MUSS die eigentliche Mutation server-seitig capability-gated sein und bei fehlender Fähigkeit 403 liefern. Die Required-Permissions Seite selbst bleibt read-only; die 403-Durchsetzung wird auf den Ziel-Surfaces umgesetzt (kein zusätzlicher Mutations-Endpunkt in Spec 083). + +#### Data Source & External Calls + +- **FR-083-010**: Das Anzeigen der Seite MUSS ausschließlich gespeicherte Daten verwenden und darf keine externen Provider-Aufrufe auslösen. + +#### UX (Issues-first) + +- **FR-083-011**: Die Seite MUSS oben eine Summary zeigen, die die Gesamtlage verständlich einordnet (z.B. „Blocked / Needs attention / Ready“) und die wichtigsten Counts enthält. +- **FR-083-011a**: Die Summary-Status-Logik MUSS eindeutig sein: **Blocked** wenn mindestens ein Blocker vorliegt; sonst **Needs attention** wenn mindestens ein Warning vorliegt (inkl. „stale“); sonst **Ready**. +- **FR-083-012**: Die Seite MUSS prominent eine Issues-Sektion bereitstellen, die Blocker (fehlende Application-Berechtigungen) und Warnings (z.B. delegated gaps, stale data) priorisiert. +- **FR-083-012a**: Die Issues-Sektion MUSS sich in Spec 083 auf **Blocker** und **Warnings** beschränken; ein separater „Error“-Issue-Typ ist nicht Teil des Umfangs. +- **FR-083-013**: Jede Issue MUSS links-only Next steps enthalten (keine eingebetteten Mutations) und klar zwischen „Beheben“ und „erneut verifizieren“ unterscheiden. +- **FR-083-013a**: Der links-only CTA „Re-run verification“ MUSS canonical zur „Start verification“ Surface `/admin/onboarding` führen und über zentrale Route-Generierung erstellt werden (kein hardcodierter Legacy-Pfad). Die capability-basierte Durchsetzung (403) erfolgt dort, nicht auf der Required-Permissions Seite. +- **FR-083-014**: Die Seite MUSS einen Hinweis enthalten, dass die Anzeige auf gespeicherten Daten basiert, inkl. Freshness/Last refreshed Information, sofern aus gespeicherten Daten ableitbar. +- **FR-083-014a**: Freshness MUSS als Warning gelten, wenn „Last refreshed“ fehlt oder älter als 30 Tage ist. +- **FR-083-014b**: Wenn keine gespeicherten Permission-Daten vorhanden sind, MUSS die Seite einen klaren Empty State („Keine Daten verfügbar“) rendern und einen links-only CTA zur Start-verification Surface anzeigen. +- **FR-083-015**: „Technical details“ MUSS verfügbar sein, aber nachrangig: die Sektion MUSS nach „Issues“ und „Passed“ erscheinen und standardmäßig eingeklappt sein. + +#### Link Consistency + +- **FR-083-016**: In-App Links zur Required-Permissions Oberfläche MÜSSEN canonical sein und konsistent generiert werden (keine hardcodierten Legacy-Pfade). + +#### Dependencies & Assumptions + +- **FR-083-017**: Die Seite baut auf existierenden Manage-Surfaces für Tenants, Verifikation und Provider-Verbindungen auf (nur Verlinkung; keine neue Surface wird dadurch eingeführt). +- **FR-083-018**: Es existiert ein Konzept von Workspace-Mitgliedschaft und Tenant-Entitlement; Entitlement ist die Voraussetzung für read-only Zugriff. + +#### Test Requirements (Mandatory) + +- **T-083-001**: Kein Workspace-Mitglied → 404. +- **T-083-002**: Workspace-Mitglied ohne Tenant-Entitlement → 404. +- **T-083-003**: Tenant-entitled User (Readonly) → 200. +- **T-083-004**: Keine gespeicherten Daten → Seite zeigt „Keine Daten verfügbar“ und einen canonical CTA zur Start-verification Surface. +- **T-083-005**: DB-only Render: canonical URL rendert ohne externe Provider-Requests und ohne Hintergrundarbeit auszulösen. +- **T-083-006**: Legacy URL bleibt 404: `/admin/t/{tenant}/required-permissions`. +- **T-083-007**: Link canonicalization: Next steps enthalten ausschließlich canonical Manage-Links. +- **T-083-008**: Cross-plane fallback Regression: Aufruf ohne gültigen Route-Tenant darf keinen impliziten „aktuellen Tenant“ nutzen → 404. +- **T-083-009**: Summary-Status-Logik: Blocker → „Blocked“; nur Warnings/Stale → „Needs attention“; keine Issues → „Ready“. +- **T-083-010**: Stale-Threshold: „Last refreshed“ fehlt oder älter als 30 Tage → Warning; jünger/gleich 30 Tage → kein Freshness-Warning. +- **T-083-011**: Issues-Typen: Seite zeigt keine separate „Error“-Issue-Kategorie (nur Blocker + Warnings). +- **T-083-012**: „Re-run verification“ Link führt canonical zur „Start verification“ Surface (kein Link auf „latest report“ als Primärziel). +- **T-083-013**: „Technical details“ ist standardmäßig eingeklappt und erscheint nach „Issues“ und „Passed“. + +## UI Action Matrix *(mandatory when Filament is changed)* + +Für jede betroffene UI-Oberfläche: liste die sichtbaren Actions/CTAs, ob sie destruktiv sind (Bestätigung erforderlich), +welche Autorisierung gilt (Entitlement vs. Fähigkeit für Mutationen), und ob ein Audit-Eintrag erwartet wird. + +| Surface | Location | Header Actions | Inspect Affordance (List/Table) | Row Actions (max 2 visible) | Bulk Actions (grouped) | Empty-State CTA(s) | View Header Actions | Create/Edit Save+Cancel | Audit log? | Notes / Exemptions | +|---|---|---|---|---|---|---|---|---|---| +| Admin Page: Required Permissions | Admin → Tenants → Required permissions | None (read-only) | N/A | None | None | Links-only: “Start verification”, “Manage provider connection” | N/A | N/A | No (view-only) | Verlinkte Mutations/Aktionen liegen auf anderen Surfaces und müssen dort 403/capability-gated sein | + +### Key Entities *(include if feature involves data)* + +- **Workspace**: Sicherheits- und Sichtbarkeitsgrenze; ein User muss Mitglied sein, um Tenant-Surfaces überhaupt sehen zu können. +- **Tenant**: Mandant im Workspace; Required Permissions sind tenant-spezifisch. +- **Workspace Membership**: Belegt, dass ein User zum Workspace gehört. +- **Tenant Entitlement (Tenant Membership)**: Belegt, dass ein User in diesem Tenant lesen darf (inkl. Readonly). +- **Permission Inventory Snapshot**: Gespeicherte Datenbasis, aus der Required-Permissions Status/Issues abgeleitet werden. +- **Verification Evidence / Report**: Gespeicherte Ergebnisse, die Freshness/Last refreshed und Issues erklären und auf „erneut verifizieren“ verlinken. + +## Success Criteria *(mandatory)* + + + +### Measurable Outcomes + +- **SC-083-001**: 100% der Requests von nicht-entitled Workspace-Mitgliedern auf die Required-Permissions Seite enden in 404 (kein Tenant-Leakage über Statuscodes). +- **SC-083-002**: 100% der Requests auf die Legacy-URL-Variante `/admin/t/{tenant}/required-permissions` enden in 404 (keine Redirects). +- **SC-083-003**: Beim Anzeigen der Seite werden 0 externe Provider-Anfragen ausgelöst (verifizierbar über Tests/Instrumentation). +- **SC-083-004**: Tenant-entitled Nutzer können in ≤ 30 Sekunden mindestens einen Blocker identifizieren und den passenden Next-step Link finden (Usability/UX-Verifikation). +- **SC-083-005**: In Staging liegt für `GET /admin/tenants/{tenant}/required-permissions` bei einer typischen Tenant-Datenmenge (mindestens 200 gespeicherte Permission-Zeilen) die p95-Server-Antwortzeit bei ≤ 500 ms (DB-only, ohne externe Provider-Calls). diff --git a/specs/083-required-permissions-hardening/tasks.md b/specs/083-required-permissions-hardening/tasks.md new file mode 100644 index 0000000..0355410 --- /dev/null +++ b/specs/083-required-permissions-hardening/tasks.md @@ -0,0 +1,162 @@ +--- + +description: "Task list for Spec 083-required-permissions-hardening" + +--- + +# Tasks: 083-required-permissions-hardening + +**Input**: Design documents from `/specs/083-required-permissions-hardening/` + +- Spec: [spec.md](spec.md) +- Plan: [plan.md](plan.md) +- Research: [research.md](research.md) +- Data model: [data-model.md](data-model.md) +- Contracts: [contracts/routes.md](contracts/routes.md) +- Quickstart: [quickstart.md](quickstart.md) + +**Tests**: REQUIRED (Pest) — runtime behavior changes. + +## Phase 1: Setup (Shared Infrastructure) + +- [X] T001 Run prerequisites check via .specify/scripts/bash/check-prerequisites.sh --json +- [X] T002 Ensure agent context is up to date via .specify/scripts/bash/update-agent-context.sh copilot +- [X] T003 [P] Create feature test directory tests/Feature/RequiredPermissions/ (add .gitkeep if needed) + +--- + +## Phase 2: Foundational (Blocking Prerequisites) + +- [X] T004 Review current canonical page implementation in app/Filament/Pages/TenantRequiredPermissions.php (identify tenant fallback + current access checks) +- [X] T005 [P] Review existing DB-only render guard patterns in tests/Feature/Auth/DbOnlyPagesDoNotMakeHttpRequestsTest.php (copy the Http::preventStrayRequests() approach) +- [X] T006 [P] Review existing cross-plane 404 patterns in tests/Feature/Auth/CrossScopeAccessTest.php (align with 404 semantics) +- [X] T007 [P] Confirm factories exist for required models (Workspace, WorkspaceMembership, Tenant, TenantMembership, TenantPermission, User) under database/factories/ + +**Checkpoint**: Foundational ready — implement US1/US2/US3. + +--- + +## Phase 3: User Story 1 — Required Permissions sicher ansehen (Priority: P1) 🎯 MVP + +**Goal**: Canonical manage surface renders issues-first from DB-only state with correct 200/404 semantics. + +**Independent Test**: A single GET to `/admin/tenants/{external_id}/required-permissions` returns 200 for tenant-entitled users and triggers no outbound HTTP. + +### Tests (US1) + +- [X] T008 [P] [US1] Add DB-only render test in tests/Feature/RequiredPermissions/RequiredPermissionsDbOnlyRenderTest.php +- [X] T009 [P] [US1] Add happy-path entitlement test (tenant-entitled → 200) in tests/Feature/RequiredPermissions/RequiredPermissionsAccessTest.php +- [X] T030 [P] [US1] Add empty-data state test ("Keine Daten verfügbar" + Start verification CTA) in tests/Feature/RequiredPermissions/RequiredPermissionsEmptyStateTest.php +- [X] T031 [P] [US1] Add test that "Technical details" is rendered after Issues/Passed and is collapsed by default in tests/Feature/RequiredPermissions/RequiredPermissionsLinksTest.php + +### Implementation (US1) + +- [X] T010 [US1] Enforce explicit 404 denial rules on page entry in app/Filament/Pages/TenantRequiredPermissions.php (workspace selected, tenant in workspace, workspace member, tenant-entitled) +- [X] T011 [US1] Remove cross-plane fallback by making resolveScopedTenant() strict (no Tenant::current()) in app/Filament/Pages/TenantRequiredPermissions.php +- [X] T012 [US1] Add freshness derivation (last_refreshed_at, is_stale) based on tenant_permissions.last_checked_at in app/Services/Intune/TenantRequiredPermissionsViewModelBuilder.php +- [X] T013 [US1] Update summary overall status derivation to treat stale freshness as a warning (Blocked > Needs attention > Ready) in app/Services/Intune/TenantRequiredPermissionsViewModelBuilder.php +- [X] T014 [US1] Render Summary → Issues → Passed → Technical layout (issues-first) using viewModel fields in resources/views/filament/pages/tenant-required-permissions.blade.php +- [X] T032 [US1] Render explicit empty-data state and keep "Technical details" collapsed by default in resources/views/filament/pages/tenant-required-permissions.blade.php + +--- + +## Phase 4: User Story 2 — Next steps finden, ohne Mutationsrechte zu benötigen (Priority: P2) + +**Goal**: Each issue includes link-only next steps that point to canonical manage surfaces; re-run verification links to Start verification. + +**Independent Test**: Page renders next-step links that are canonical and the “Re-run verification” CTA points to `/admin/onboarding`. + +### Tests (US2) + +- [X] T015 [P] [US2] Add CTA/link assertion test for re-run verification pointing to /admin/onboarding in tests/Feature/RequiredPermissions/RequiredPermissionsLinksTest.php +- [X] T016 [P] [US2] Add test asserting no legacy tenant-plane links are emitted (no /admin/t/...) in tests/Feature/RequiredPermissions/RequiredPermissionsLinksTest.php + +### Implementation (US2) + +- [X] T017 [US2] Change reRunVerificationUrl() to return the canonical Start verification surface via route helper (target: /admin/onboarding) in app/Filament/Pages/TenantRequiredPermissions.php +- [X] T018 [US2] Ensure issue cards only contain link-only next steps and canonical manage URLs in resources/views/filament/pages/tenant-required-permissions.blade.php + +--- + +## Phase 5: User Story 3 — Tenant-Discovery verhindern (Deny-as-not-found) (Priority: P3) + +**Goal**: Non-entitled users cannot discover tenant existence/posture via status codes or legacy routes. + +**Independent Test**: Requests for non-members/non-entitled return 404, and legacy `/admin/t/{tenant}/required-permissions` is 404. + +### Tests (US3) + +- [X] T019 [P] [US3] Add test: workspace-member but not tenant-entitled → 404 in tests/Feature/RequiredPermissions/RequiredPermissionsAccessTest.php +- [X] T020 [P] [US3] Add test: not a workspace member → 404 in tests/Feature/RequiredPermissions/RequiredPermissionsAccessTest.php +- [X] T021 [P] [US3] Add test: legacy /admin/t/{tenant}/required-permissions returns 404 in tests/Feature/RequiredPermissions/RequiredPermissionsLegacyRouteTest.php +- [X] T022 [P] [US3] Add regression test: route tenant invalid does not fall back to a current tenant context (still 404) in tests/Feature/RequiredPermissions/RequiredPermissionsAccessTest.php + +### Implementation (US3) + +- [X] T023 [US3] Ensure all deny-as-not-found conditions abort(404) (not 403) in app/Filament/Pages/TenantRequiredPermissions.php + +--- + +## Phase 6: Polish & Cross-Cutting Concerns + +- [X] T024 [P] Update existing unit coverage for overall status if signature/logic changed in tests/Unit/TenantRequiredPermissionsOverallStatusTest.php +- [X] T025 [P] Add new unit tests for freshness/stale threshold (missing or >30 days) in tests/Unit/TenantRequiredPermissionsFreshnessTest.php +- [X] T026 Run formatting via vendor/bin/sail bin pint --dirty +- [X] T027 Run targeted tests via vendor/bin/sail artisan test --compact tests/Feature/RequiredPermissions +- [X] T028 Run targeted unit tests via vendor/bin/sail artisan test --compact tests/Unit/TenantRequiredPermissions +- [X] T029 Validate quickstart steps remain accurate in specs/083-required-permissions-hardening/quickstart.md + +--- + +## Dependencies & Execution Order + +### User Story completion order +```mermaid +graph TD + P1[US1: View canonical page safely] --> P2[US2: Canonical next steps links] + P1 --> P3[US3: Deny-as-not-found + legacy 404] + P2 --> Polish[Polish & regression coverage] + P3 --> Polish +``` + +- Setup (T001–T003) → Foundational (T004–T007) → US1 (T008–T014, T030–T032) → US2 (T015–T018) + US3 (T019–T023) → Polish (T024–T029) + +### Parallel opportunities +- Phase 1: T003 can run in parallel. +- Phase 2: T005–T007 are parallel. +- US1 tests (T008–T009, T030–T031) can be written in parallel. +- US2 tests (T015–T016) can be written in parallel. +- US3 tests (T019–T022) can be written in parallel. +- Polish: T024–T025 are parallel; T026–T028 are sequential validation. + +--- + +## Parallel execution examples (per story) + +### US1 +- Run in parallel: + - T008: tests/Feature/RequiredPermissions/RequiredPermissionsDbOnlyRenderTest.php + - T009: tests/Feature/RequiredPermissions/RequiredPermissionsAccessTest.php + - T030: tests/Feature/RequiredPermissions/RequiredPermissionsEmptyStateTest.php + - T031: tests/Feature/RequiredPermissions/RequiredPermissionsLinksTest.php + +### US2 +- Run in parallel: + - T015: tests/Feature/RequiredPermissions/RequiredPermissionsLinksTest.php (CTA) + - T016: tests/Feature/RequiredPermissions/RequiredPermissionsLinksTest.php (no legacy links) + +### US3 +- Run in parallel: + - T019: tests/Feature/RequiredPermissions/RequiredPermissionsAccessTest.php (non-entitled 404) + - T020: tests/Feature/RequiredPermissions/RequiredPermissionsAccessTest.php (non-member 404) + - T021: tests/Feature/RequiredPermissions/RequiredPermissionsLegacyRouteTest.php + - T022: tests/Feature/RequiredPermissions/RequiredPermissionsAccessTest.php (no fallback) + +--- + +## Task completeness validation + +- Every user story has: + - At least one independently runnable verification test task + - Implementation tasks with concrete file paths + - A clear checkpoint goal and independent test criteria diff --git a/tests/Feature/RequiredPermissions/RequiredPermissionsAccessTest.php b/tests/Feature/RequiredPermissions/RequiredPermissionsAccessTest.php new file mode 100644 index 0000000..c11861c --- /dev/null +++ b/tests/Feature/RequiredPermissions/RequiredPermissionsAccessTest.php @@ -0,0 +1,63 @@ +actingAs($user) + ->get("/admin/tenants/{$tenant->external_id}/required-permissions") + ->assertOk(); +}); + +it('returns 404 for workspace members without tenant entitlement on the canonical route', function (): void { + $user = User::factory()->create(); + $workspace = Workspace::factory()->create(); + $tenant = Tenant::factory()->create([ + 'workspace_id' => (int) $workspace->getKey(), + ]); + + WorkspaceMembership::factory()->create([ + 'workspace_id' => (int) $workspace->getKey(), + 'user_id' => (int) $user->getKey(), + 'role' => 'owner', + ]); + + $this->actingAs($user) + ->withSession([ + WorkspaceContext::SESSION_KEY => (int) $workspace->getKey(), + ]) + ->get("/admin/tenants/{$tenant->external_id}/required-permissions") + ->assertNotFound(); +}); + +it('returns 404 for users who are not workspace members', function (): void { + $user = User::factory()->create(); + $workspace = Workspace::factory()->create(); + $tenant = Tenant::factory()->create([ + 'workspace_id' => (int) $workspace->getKey(), + ]); + + $this->actingAs($user) + ->withSession([ + WorkspaceContext::SESSION_KEY => (int) $workspace->getKey(), + ]) + ->get("/admin/tenants/{$tenant->external_id}/required-permissions") + ->assertNotFound(); +}); + +it('returns 404 when the route tenant is invalid instead of falling back to the current tenant context', function (): void { + [$user, $tenant] = createUserWithTenant(role: 'readonly'); + + Tenant::query()->whereKey((int) $tenant->getKey())->update(['is_current' => true]); + + $this->actingAs($user) + ->get('/admin/tenants/invalid-tenant-id/required-permissions') + ->assertNotFound(); +}); diff --git a/tests/Feature/RequiredPermissions/RequiredPermissionsCopyActionsTest.php b/tests/Feature/RequiredPermissions/RequiredPermissionsCopyActionsTest.php index 175ed2e..842072c 100644 --- a/tests/Feature/RequiredPermissions/RequiredPermissionsCopyActionsTest.php +++ b/tests/Feature/RequiredPermissions/RequiredPermissionsCopyActionsTest.php @@ -13,7 +13,7 @@ [$user, $tenant] = createUserWithTenant(tenant: $tenant, role: 'readonly'); $this->actingAs($user) - ->get("/admin/t/{$tenant->external_id}/required-permissions") + ->get("/admin/tenants/{$tenant->external_id}/required-permissions") ->assertSuccessful() ->assertSee('Guidance') ->assertSee('Who can fix this?', false) diff --git a/tests/Feature/RequiredPermissions/RequiredPermissionsDbOnlyRenderTest.php b/tests/Feature/RequiredPermissions/RequiredPermissionsDbOnlyRenderTest.php index 7600534..3188f60 100644 --- a/tests/Feature/RequiredPermissions/RequiredPermissionsDbOnlyRenderTest.php +++ b/tests/Feature/RequiredPermissions/RequiredPermissionsDbOnlyRenderTest.php @@ -1,13 +1,21 @@ actingAs($user) - ->get("/admin/t/{$tenant->external_id}/required-permissions") + ->get("/admin/tenants/{$tenant->external_id}/required-permissions") ->assertSuccessful(); }); + + Queue::assertNothingPushed(); }); diff --git a/tests/Feature/RequiredPermissions/RequiredPermissionsEmptyStateTest.php b/tests/Feature/RequiredPermissions/RequiredPermissionsEmptyStateTest.php new file mode 100644 index 0000000..d7b735a --- /dev/null +++ b/tests/Feature/RequiredPermissions/RequiredPermissionsEmptyStateTest.php @@ -0,0 +1,14 @@ +actingAs($user) + ->get("/admin/tenants/{$tenant->external_id}/required-permissions") + ->assertSuccessful() + ->assertSee('Keine Daten verfügbar') + ->assertSee('/admin/onboarding', false) + ->assertSee('Start verification'); +}); diff --git a/tests/Feature/RequiredPermissions/RequiredPermissionsFiltersTest.php b/tests/Feature/RequiredPermissions/RequiredPermissionsFiltersTest.php index b95eb6c..b3b6b6a 100644 --- a/tests/Feature/RequiredPermissions/RequiredPermissionsFiltersTest.php +++ b/tests/Feature/RequiredPermissions/RequiredPermissionsFiltersTest.php @@ -51,7 +51,7 @@ ]); $missingResponse = $this->actingAs($user) - ->get("/admin/t/{$tenant->external_id}/required-permissions") + ->get("/admin/tenants/{$tenant->external_id}/required-permissions") ->assertSuccessful() ->assertSee('All required permissions are present', false); @@ -61,7 +61,7 @@ ->assertDontSee('data-permission-key="Gamma.Manage.All"', false); $presentResponse = $this->actingAs($user) - ->get("/admin/t/{$tenant->external_id}/required-permissions?status=present") + ->get("/admin/tenants/{$tenant->external_id}/required-permissions?status=present") ->assertSuccessful() ->assertSee('wire:model.live="status"', false); @@ -71,7 +71,7 @@ ->assertSee('data-permission-key="Gamma.Manage.All"', false); $delegatedResponse = $this->actingAs($user) - ->get("/admin/t/{$tenant->external_id}/required-permissions?status=present&type=delegated") + ->get("/admin/tenants/{$tenant->external_id}/required-permissions?status=present&type=delegated") ->assertSuccessful(); $delegatedResponse @@ -85,7 +85,7 @@ ]); $featureResponse = $this->actingAs($user) - ->get("/admin/t/{$tenant->external_id}/required-permissions?{$featureQuery}") + ->get("/admin/tenants/{$tenant->external_id}/required-permissions?{$featureQuery}") ->assertSuccessful(); $featureResponse @@ -94,7 +94,7 @@ ->assertDontSee('data-permission-key="Beta.Read.All"', false); $searchResponse = $this->actingAs($user) - ->get("/admin/t/{$tenant->external_id}/required-permissions?status=all&search=delegated") + ->get("/admin/tenants/{$tenant->external_id}/required-permissions?status=all&search=delegated") ->assertSuccessful(); $searchResponse diff --git a/tests/Feature/RequiredPermissions/RequiredPermissionsLegacyRouteTest.php b/tests/Feature/RequiredPermissions/RequiredPermissionsLegacyRouteTest.php new file mode 100644 index 0000000..ed59e05 --- /dev/null +++ b/tests/Feature/RequiredPermissions/RequiredPermissionsLegacyRouteTest.php @@ -0,0 +1,11 @@ +actingAs($user) + ->get("/admin/t/{$tenant->external_id}/required-permissions") + ->assertNotFound(); +}); diff --git a/tests/Feature/RequiredPermissions/RequiredPermissionsLinksTest.php b/tests/Feature/RequiredPermissions/RequiredPermissionsLinksTest.php new file mode 100644 index 0000000..2ef7921 --- /dev/null +++ b/tests/Feature/RequiredPermissions/RequiredPermissionsLinksTest.php @@ -0,0 +1,25 @@ +actingAs($user) + ->get("/admin/tenants/{$tenant->external_id}/required-permissions") + ->assertSuccessful() + ->assertSee('Re-run verification') + ->assertSee('/admin/onboarding', false) + ->assertDontSee('/admin/t/', false); +}); + +it('renders sections in summary-issues-passed-technical order and keeps technical details collapsed by default', function (): void { + [$user, $tenant] = createUserWithTenant(role: 'readonly'); + + $this->actingAs($user) + ->get("/admin/tenants/{$tenant->external_id}/required-permissions") + ->assertSuccessful() + ->assertSeeInOrder(['Summary', 'Issues', 'Passed', 'Technical details']) + ->assertSee('
assertDontSee('data-testid="technical-details" open', false); +}); diff --git a/tests/Feature/RequiredPermissions/RequiredPermissionsOverviewTest.php b/tests/Feature/RequiredPermissions/RequiredPermissionsOverviewTest.php index b837ffb..3ffba20 100644 --- a/tests/Feature/RequiredPermissions/RequiredPermissionsOverviewTest.php +++ b/tests/Feature/RequiredPermissions/RequiredPermissionsOverviewTest.php @@ -26,7 +26,7 @@ ]); $this->actingAs($user) - ->get("/admin/t/{$tenant->external_id}/required-permissions") + ->get("/admin/tenants/{$tenant->external_id}/required-permissions") ->assertSuccessful() ->assertSee('Blocked', false) ->assertSee('applyFeatureFilter', false) diff --git a/tests/Feature/RequiredPermissions/RequiredPermissionsRbacTest.php b/tests/Feature/RequiredPermissions/RequiredPermissionsRbacTest.php deleted file mode 100644 index 6d04504..0000000 --- a/tests/Feature/RequiredPermissions/RequiredPermissionsRbacTest.php +++ /dev/null @@ -1,33 +0,0 @@ -create([ - 'workspace_id' => (int) $tenant->workspace_id, - ]); - - $this->actingAs($user) - ->get("/admin/t/{$otherTenant->external_id}/required-permissions") - ->assertNotFound(); -}); - -it('returns 403 for members without tenant.view capability accessing required permissions', function (): void { - [$user, $tenant] = createUserWithTenant(role: 'readonly'); - - $this->mock(CapabilityResolver::class, function ($mock): void { - $mock->shouldReceive('isMember') - ->andReturn(true); - - $mock->shouldReceive('can') - ->andReturnUsing(fn ($user, $tenant, $capability): bool => $capability !== Capabilities::TENANT_VIEW); - }); - - $this->actingAs($user) - ->get("/admin/t/{$tenant->external_id}/required-permissions") - ->assertForbidden(); -}); diff --git a/tests/Unit/TenantRequiredPermissionsFreshnessTest.php b/tests/Unit/TenantRequiredPermissionsFreshnessTest.php new file mode 100644 index 0000000..250dbd3 --- /dev/null +++ b/tests/Unit/TenantRequiredPermissionsFreshnessTest.php @@ -0,0 +1,34 @@ +toBeNull() + ->and($freshness['is_stale'])->toBeTrue(); +}); + +it('marks freshness as stale when last refreshed is older than 30 days', function (): void { + $freshness = TenantRequiredPermissionsViewModelBuilder::deriveFreshness( + CarbonImmutable::parse('2026-01-08 11:59:59'), + CarbonImmutable::parse('2026-02-08 12:00:00'), + ); + + expect($freshness['is_stale'])->toBeTrue(); +}); + +it('marks freshness as not stale when last refreshed is exactly 30 days old', function (): void { + $freshness = TenantRequiredPermissionsViewModelBuilder::deriveFreshness( + CarbonImmutable::parse('2026-01-09 12:00:00'), + CarbonImmutable::parse('2026-02-08 12:00:00'), + ); + + expect($freshness['is_stale'])->toBeFalse(); +}); diff --git a/tests/Unit/TenantRequiredPermissionsOverallStatusTest.php b/tests/Unit/TenantRequiredPermissionsOverallStatusTest.php index 1807efc..9bf9291 100644 --- a/tests/Unit/TenantRequiredPermissionsOverallStatusTest.php +++ b/tests/Unit/TenantRequiredPermissionsOverallStatusTest.php @@ -98,3 +98,27 @@ expect(TenantRequiredPermissionsViewModelBuilder::deriveOverallStatus($rows)) ->toBe(VerificationReportOverall::Ready->value); }); + +it('maps overall to needs_attention when freshness is stale without explicit permission gaps', function (): void { + $rows = [ + [ + 'key' => 'A', + 'type' => 'application', + 'description' => null, + 'features' => ['backup'], + 'status' => 'granted', + 'details' => null, + ], + [ + 'key' => 'B', + 'type' => 'delegated', + 'description' => null, + 'features' => ['backup'], + 'status' => 'granted', + 'details' => null, + ], + ]; + + expect(TenantRequiredPermissionsViewModelBuilder::deriveOverallStatus($rows, true)) + ->toBe(VerificationReportOverall::NeedsAttention->value); +});