ahmido
05a604cfb6
Implements Spec 076 enterprise remediation UX for tenant required permissions. Highlights - Above-the-fold overview (impact + counts) with missing-first experience - Feature-based grouping, filters/search, copy-to-clipboard for missing app/delegated permissions - Tenant-scoped deny-as-not-found semantics; DB-only viewing - Centralized badge semantics (no ad-hoc status mapping) Testing - Feature tests for default filters, grouping, copy output, and non-member 404 behavior. Integration - Adds deep links from verification checks to the Required permissions page. Co-authored-by: Ahmed Darrazi <ahmeddarrazi@MacBookPro.fritz.box> Reviewed-on: #92
99 lines
6.3 KiB
Markdown
99 lines
6.3 KiB
Markdown
# Research — Spec 076 (Permissions Enterprise UI)
|
||
|
||
## Decisions
|
||
|
||
### 1) Build a dedicated tenant-scoped Filament Page
|
||
- Decision: Implement a new Filament Page under the tenant panel route (`/admin/t/{tenant}/...`) for the “Required permissions” enterprise remediation UX.
|
||
- Rationale:
|
||
- The existing `TenantResource` infolist entry is a raw list; Spec 076 requires a two-layer remediation layout (overview + matrix) and feature grouping.
|
||
- A dedicated Page can provide operator-first UX without bloating the tenant detail resource.
|
||
- Alternatives considered:
|
||
- Extending the existing `TenantResource` view: rejected because it couples a complex remediation UI to a general-purpose resource view and makes verification deep-linking/clustering harder.
|
||
|
||
### 2) Use stored + config-based data only at render time (DB-only render)
|
||
- Decision: The page loads required permissions from `config('intune_permissions.permissions')` and granted/missing statuses from the `tenant_permissions` table via `TenantPermissionService::compare($tenant, persist: false, liveCheck: false, useConfiguredStub: false)`.
|
||
- Rationale:
|
||
- Satisfies FR-076-008 (no external network calls during page view).
|
||
- Reuses existing data normalization and status modeling.
|
||
- Alternatives considered:
|
||
- Calling Graph on page view (`liveCheck: true`): rejected (explicitly out of scope and violates DB-only render).
|
||
|
||
### 3) Authorization semantics: non-member 404, member missing capability 403
|
||
- Decision:
|
||
- Non-member tenant access remains deny-as-not-found (404) via the Admin panel middleware (`DenyNonMemberTenantAccess`).
|
||
- Page access is capability-gated via `Page::canAccess()` using `Capabilities::TENANT_VIEW`.
|
||
- Rationale:
|
||
- Matches Constitution RBAC-UX-002 and RBAC-UX-003.
|
||
- Ensures correct semantics for both initial request and Livewire requests.
|
||
- Alternatives considered:
|
||
- Enforcing capability only in `mount()` with custom `abort(...)`: rejected because `canAccess()` is the consistent Filament entry-point gate and keeps nav hiding in sync.
|
||
|
||
### 4) Badge semantics: use centralized domains only
|
||
- Decision:
|
||
- Per-permission badges: `BadgeDomain::TenantPermissionStatus`.
|
||
- Overall status badge: `BadgeDomain::VerificationReportOverall` with values from `VerificationReportOverall`.
|
||
- Rationale:
|
||
- Constitution BADGE-001 requires centralized semantic mapping.
|
||
|
||
### 5) Filters/search implementation: server-side Livewire state, in-memory filtering
|
||
- Decision: Represent filter/search state as Livewire properties on the Page and filter the already-loaded permission array in-memory.
|
||
- Rationale:
|
||
- Dataset size is small (config-defined permissions), so in-memory filtering is fast and stable.
|
||
- Enables programmatic tests for filtering/copy payload generation without relying on browser JS.
|
||
- Alternatives considered:
|
||
- Filament `Tables` with query-backed filters: rejected as unnecessary complexity for a config-driven list.
|
||
- Pure client-side Alpine filtering: rejected due to weaker automated testability.
|
||
|
||
### 6) Copy-to-clipboard: “copy payload modal” + robust clipboard fallback
|
||
- Decision: Implement copy actions that open a modal (or inline panel) containing the exact newline-separated payload; a “Copy” button uses the existing robust Alpine clipboard fallback pattern.
|
||
- Rationale:
|
||
- Clipboard APIs are browser-only; Livewire actions cannot write directly to clipboard.
|
||
- Reuses proven fallback approach in `resources/views/filament/partials/json-viewer.blade.php`.
|
||
- Makes copy output auditable/visible before copying (enterprise-friendly).
|
||
- Alternatives considered:
|
||
- Attempting server-side copy: not possible.
|
||
|
||
### 7) Verify-step clustering: emit clustered checks in `verification_report`
|
||
- Decision:
|
||
- Extend the queued verification job (`ProviderConnectionHealthCheckJob`) to write a verification report that includes the existing connection check plus 5–6 permission cluster checks.
|
||
- Update the onboarding wizard Verify step to render `OperationRun.context.verification_report` (via `VerificationReportViewer`) and show checks issues-first.
|
||
- Rationale:
|
||
- The project already has a report schema (`VerificationReportSchema`) and writer (`VerificationReportWriter`).
|
||
- Clustering in the report keeps the experience consistent across the wizard and operation-run detail views.
|
||
- Alternatives considered:
|
||
- Cluster in Blade only: rejected because it does not affect summary/overall and can drift between views.
|
||
|
||
### 8) Enterprise correctness: refresh Observed permissions during the verification run
|
||
- Decision:
|
||
- The queued verification run (Operation Run) attempts a live Graph refresh for Observed permissions and persists it to `tenant_permissions`.
|
||
- Viewer surfaces (Required Permissions page, onboarding Verify step, operation run viewer) remain DB-only at render time.
|
||
- If the refresh fails (429/network), permission clusters degrade to warnings with retry guidance and MUST NOT assert “missing permissions” based on stale/empty inventory.
|
||
- Rationale:
|
||
- Prevents false “missing” findings when the stored inventory is empty/stale.
|
||
- Keeps all external calls in the queued run, maintaining the DB-only render rule.
|
||
|
||
## Open questions resolved (NEEDS CLARIFICATION → decision)
|
||
|
||
### “Enabled features” for impact summary
|
||
- Decision: In Spec 076 scope, treat “enabled features” as the feature tags present in `config('intune_permissions.permissions')`.
|
||
- Rationale: There is no current per-tenant feature-enable registry in the codebase; feature tags already exist and are deterministic.
|
||
- Future upgrade path: If/when tenant-specific enablement exists, compute relevance by intersecting enabled features with permission feature tags.
|
||
|
||
## Check cluster proposal (stable keys)
|
||
|
||
Target: 5–7 checks; issues-first.
|
||
|
||
- `provider.connection.check` (existing)
|
||
- `permissions.admin_consent` (overall admin consent / application permissions missing)
|
||
- `permissions.directory_groups`
|
||
- `permissions.intune_configuration`
|
||
- `permissions.intune_apps`
|
||
- `permissions.intune_rbac_assignments`
|
||
- `permissions.scripts_remediations` (optional / skip when irrelevant)
|
||
|
||
Each permission-derived check:
|
||
- Pass: no missing permissions in its mapped set
|
||
- Fail/Blocked: any missing required permission in its set
|
||
- Skip: cluster mapped permissions set is empty (or feature not relevant)
|
||
- Next step: “Open required permissions” deep link to the new page (optionally pre-filtered by Feature).
|