feat/048-backup-restore-ui-graph-safety #55

Merged
ahmido merged 4 commits from feat/048-backup-restore-ui-graph-safety into dev 2026-01-11 00:14:35 +00:00
8 changed files with 580 additions and 0 deletions
Showing only changes of commit e9994aa5cc - Show all commits

View File

@ -0,0 +1,34 @@
# Specification Quality Checklist: Backup/Restore UI Graph-Safety (Phase 1)
**Purpose**: Validate specification completeness and quality before proceeding to planning
**Created**: 2026-01-10
**Feature**: specs/048-backup-restore-ui-graph-safety/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
- Phase 1 intentionally does not refactor action handlers (covered in Spec 049).

View File

@ -0,0 +1,42 @@
openapi: 3.0.3
info:
title: TenantPilot Admin Page Render Contracts (Feature 048)
version: 0.1.0
description: |
Minimal HTTP contracts for the Filament pages that must render without touching Microsoft Graph.
servers:
- url: /
paths:
/admin/t/{tenantExternalId}/backup-sets:
get:
operationId: backupSetsIndex
summary: Backup Sets index (tenant-scoped)
parameters:
- name: tenantExternalId
in: path
required: true
schema:
type: string
responses:
'200':
description: Page renders successfully.
'302':
description: Redirect to login when unauthenticated.
/admin/t/{tenantExternalId}/restore-runs/create:
get:
operationId: restoreRunWizardCreate
summary: Restore Run wizard (create) page (tenant-scoped)
parameters:
- name: tenantExternalId
in: path
required: true
schema:
type: string
responses:
'200':
description: Page renders successfully.
'302':
description: Redirect to login when unauthenticated.

View File

@ -0,0 +1,59 @@
# Data Model: Backup/Restore UI Graph-Safety (048)
This phase introduces **no schema changes**. It hardens UI render boundaries and adds guard tests.
## Existing Entities (used by this feature)
### Tenant
- Purpose: Tenancy boundary for all reads/writes.
- Notes:
- Filament panel is tenant-scoped via `external_id` slug.
### User
- Purpose: Authenticated actor; has tenant memberships/roles.
### BackupSet (`backup_sets`)
- Fields (selected):
- `id`, `tenant_id`
- `name`, `created_by`, `status`, `item_count`, `completed_at`
- `metadata` (JSON)
- `deleted_at` (soft deletes, via housekeeping migration)
- Indexes: `tenant_id,status`, `completed_at`
- Used in UI:
- Backup Sets index (table rendering must be DB-only)
### BackupItem (`backup_items`)
- Fields (selected):
- `id`, `tenant_id`, `backup_set_id`
- `policy_id` (nullable), `policy_version_id` (nullable)
- `policy_identifier`, `policy_type`, `platform`
- `captured_at`
- `payload` (JSON), `metadata` (JSON), `assignments` (JSON)
- `deleted_at` (soft deletes)
- Constraints:
- Unique: `(backup_set_id, policy_identifier, policy_type)`
- Used in UI:
- Restore wizard item selection + metadata display (must not resolve external names via Graph)
### RestoreRun (`restore_runs`)
- Fields (selected):
- `id`, `tenant_id`, `backup_set_id`
- `requested_by`, `is_dry_run`, `status`
- `requested_items` (JSON), `preview` (JSON), `results` (JSON)
- `group_mapping` (JSON)
- `failure_reason`, `started_at`, `completed_at`, `metadata` (JSON)
- `deleted_at` (soft deletes)
- Used in UI:
- Restore wizard render (Create page) including group mapping controls
## Relationships (high-level)
- `Tenant` has many `BackupSet`, `BackupItem`, `RestoreRun`.
- `BackupSet` has many `BackupItem`.
- `RestoreRun` belongs to `BackupSet`.
## Validation & State Transitions (relevant to this phase)
- No new state transitions introduced.
- The key invariant enforced by tests:
- Rendering Backup/Restore UI pages must not invoke `GraphClientInterface`.

View File

@ -0,0 +1,82 @@
# Implementation Plan: Backup/Restore UI Graph-Safety (Phase 1)
**Branch**: `feat/048-backup-restore-ui-graph-safety` | **Date**: 2026-01-11 | **Spec**: specs/048-backup-restore-ui-graph-safety/spec.md
**Input**: Feature specification from `specs/048-backup-restore-ui-graph-safety/spec.md`
## Summary
Ensure Backup/Restore Filament UI is Graph-safe by construction:
- No Microsoft Graph calls during render/mount/options/typeahead.
- Restore wizard group mapping UI shows DB-only fallback labels (`…<last8>`) instead of resolving names via Graph.
- Add fail-hard Pest feature tests that bind `GraphClientInterface` to throw and still allow:
- Backup Sets index to render (HTTP 200 + stable marker)
- Restore wizard to render (HTTP 200 + stable marker)
## Technical Context
**Language/Version**: PHP 8.2+ (repo guidance targets PHP 8.4.x)
**Primary Dependencies**: Laravel 12, Filament 4, Livewire 3
**Storage**: PostgreSQL (JSON columns used for snapshots/preview/results/mappings)
**Testing**: Pest 4 (via `php artisan test`), PHPUnit 12 under the hood
**Target Platform**: Sail-first local dev (Docker), Dokploy-first staging/prod (containers)
**Project Type**: Laravel monolith (Filament admin UI + jobs/services)
**Performance Goals**: N/A (UI correctness + safety)
**Constraints**:
- UI render must be DB-only (no Graph in request lifecycle)
- No new tables/migrations in Phase 1
- Keep surface area minimal and low-risk
**Scale/Scope**: Multi-tenant admin app; tests must enforce tenant scoping and guardrails
## Constitution Check
*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.*
- Inventory-first: PASS (this phase only hardens UI render boundaries; no changes to SoT semantics)
- Read/write separation: PASS (no restore execution changes; tests cover render only)
- Single contract path to Graph: PASS (goal is “no Graph in UI render”; Graph stays behind `GraphClientInterface`)
- Deterministic capabilities: PASS (no capability derivation changes)
- Tenant isolation: PASS (tests use tenant-scoped URLs and seeded tenant data)
- Automation idempotent/observable: PASS (no job/scheduler changes in Phase 1)
- Data minimization & safe logging: PASS (no new stored data or logs)
## Project Structure
### Documentation (this feature)
```text
specs/048-backup-restore-ui-graph-safety/
├── plan.md # This file (/speckit.plan output)
├── research.md # Phase 0 output
├── data-model.md # Phase 1 output
├── quickstart.md # Phase 1 output
├── contracts/ # Phase 1 output
└── tasks.md # Task breakdown (already present)
```
### Source Code (repository root)
```text
app/
├── Filament/
│ ├── Resources/
│ │ ├── BackupSetResource.php
│ │ └── RestoreRunResource.php
│ └── Resources/*/Pages/
├── Providers/
│ ├── AppServiceProvider.php # GraphClientInterface binding
│ └── Filament/AdminPanelProvider.php # panel path + tenant routing
├── Services/
│ └── Graph/
│ ├── GraphClientInterface.php
│ └── NullGraphClient.php
database/migrations/
tests/Feature/
```
**Structure Decision**: Laravel monolith (Filament admin UI + services). No new top-level folders.
## Complexity Tracking
None.

View File

@ -0,0 +1,35 @@
# Quickstart: Backup/Restore UI Graph-Safety (048)
This quickstart is for validating the **UI render Graph-safety** guardrails locally.
## Prerequisites
- Laravel Sail running (recommended)
- Database migrated
## Local setup (Sail)
1) Start Sail
- `./vendor/bin/sail up -d`
2) Run migrations
- `./vendor/bin/sail artisan migrate`
## Run the targeted tests (once implemented)
- `./vendor/bin/sail artisan test --filter=graph\-safety`
Or run specific files (names TBD when tests land):
- `./vendor/bin/sail artisan test tests/Feature/Filament/*GraphSafety*Test.php`
## Formatting
- `./vendor/bin/pint --dirty`
## Notes
- These guard tests intentionally fail if any Backup/Restore page render path touches `App\\Services\\Graph\\GraphClientInterface`.
- Graph credentials are not required for this phase; tests should pass with Graph effectively disabled.

View File

@ -0,0 +1,45 @@
# Research: Backup/Restore UI Graph-Safety (048)
## Decision: Enforce Graph-safety via fail-hard feature renders
- **Decision**: Add Pest *feature* tests that `actingAs(...)` and `GET` Filament page URLs while binding `App\Services\Graph\GraphClientInterface` to a fail-hard implementation (throws on any call).
- **Rationale**: A full HTTP render exercises the real Filament request lifecycle (middleware, tenancy, resource/page boot) and will fail immediately if any UI render path touches Graph.
- **Alternatives considered**:
- Livewire component tests only → can miss route/middleware/tenancy boot and wont reflect “real render” regressions as reliably.
- Binding Graph to `NullGraphClient` → would allow silent Graph usage to slip through.
## Decision: Use `Resource::getUrl()` for stable Filament routes (tenant-scoped)
- **Decision**: Use Filaments URL helpers in tests:
- `BackupSetResource::getUrl('index', tenant: $tenant)`
- `RestoreRunResource::getUrl('create', tenant: $tenant)`
- **Rationale**: Avoids hardcoding route paths and keeps tests resilient to panel path / tenancy prefix changes.
- **Repo evidence**:
- `tests/Feature/Filament/InventorySyncRunResourceTest.php` uses `->get(InventorySyncRunResource::getUrl('index', tenant: $tenant))`.
- **Alternatives considered**:
- Hardcoded `/admin/t/{tenant}/...` paths → brittle if the panel path or tenant prefix changes.
## Decision: Assert HTTP 200 + stable marker
- **Decision**: Guard tests assert `->assertOk()` plus a stable marker string per page.
- **Rationale**: Reduces false positives (e.g., a redirect to login) and makes failures easier to diagnose.
- **Alternatives considered**:
- Status-only (200) → may still pass with empty/partial output or wrong page.
## Decision: Fallback label masking format
- **Decision**: Mask unresolved external IDs as `…<last8>` (last 8 characters, prefixed with ellipsis).
- **Rationale**: Keeps UI readable while avoiding full identifier disclosure.
- **Alternatives considered**:
- Full ID → increases accidental disclosure.
- Hash → less readable for operators.
## Decision: Filament panel + tenancy routing assumptions
- **Decision**: Treat the Filament admin panel as tenant-scoped under the configured path/prefix:
- Panel path: `admin`
- Tenant route prefix: `t`
- Tenant slug attribute: `external_id`
- **Rationale**: This is the repos current canonical setup (`App\Providers\Filament\AdminPanelProvider`).
- **Alternatives considered**:
- Non-tenant-scoped pages → not applicable (TenantPilot is tenant-first).

View File

@ -0,0 +1,127 @@
# Feature Specification: Backup/Restore UI Graph-Safety (Phase 1)
**Feature Branch**: `feat/048-backup-restore-ui-graph-safety`
**Created**: 2026-01-10
**Status**: Draft
## Purpose
Ensure Backup/Restore UI follows the same guardrails as Inventory:
- UI renders DB-only (no Microsoft Graph calls during render/mount/options/typeahead)
- UI still remains usable with safe fallbacks for unresolved external identifiers
- Add programmatic guard tests that fail if Graph is touched while rendering
This phase is intentionally minimal-change and high-safety.
## Clarifications
### Session 2026-01-10
- Q: Which pages must the fail-hard `GraphClientInterface` guard tests render? → A: Backup Sets index + Restore wizard.
- Q: How should `<masked-id>` be formatted in fallback labels? → A: `…<last8>`
### Session 2026-01-11
- Q: How should the fail-hard Graph guard tests be structured? → A: Feature tests that `actingAs(...)` then `GET` the Filament pages routes.
- Q: For the feature tests, should we assert only HTTP 200, or also a stable UI marker? → A: Assert HTTP 200 + a stable page marker string.
## Users
- Tenant Admin
- MSP Operator (within authorized tenant scope)
## User Stories
### US1 (P1): Backup Sets Index is Graph-safe
As a tenant admin, I can open the Backup Sets index page and it renders DB-only (no Graph calls during request/render/mount/options/typeahead).
**Maps to**: Scenario 1
### US2 (P1): Restore Wizard is Graph-safe incl. Group Mapping UI
As a tenant admin, I can open the Restore wizard page and the group mapping UI remains usable without any Graph calls (including typeahead/search/label resolution).
**Maps to**: Scenario 2 + Scenario 3
## User Scenarios & Testing
### Scenario 1: Open Backup pages without Graph access
- Given a user is authenticated and has access to a tenant
- When they open the Backup Sets index page
- Then the page renders successfully (HTTP 200) without any Graph calls
### Scenario 2: Open Restore Wizard without Graph access
- Given a user is authenticated and has access to a tenant
- When they open the Restore Run wizard page
- Then the wizard renders successfully (HTTP 200) without any Graph calls
### Scenario 3: Group mapping shows safe fallback labels
- Given the UI displays group IDs (e.g., in mapping fields)
- When the UI cannot resolve group names without Graph
- Then it shows safe fallback labels like:
- `Unresolved (…<last8>)`
- `Group (external): …<last8>`
## Stable Page Marker Rules (for Guard Tests)
Guard tests MUST assert a stable, static marker string in addition to HTTP 200.
Marker selection rules:
- MUST be static UI text that is not tenant data (avoid names, IDs, timestamps, counts)
- SHOULD be a column label, section heading, or primary action label rendered by Filament
- MUST be present on the initial GET without requiring any Livewire interaction
- MUST NOT depend on Microsoft Graph availability
Markers MUST be recorded in quickstart.md once chosen.
## Functional Requirements
- FR-001: Backup/Restore UI MUST NOT call Microsoft Graph during render/mount/options/typeahead.
- FR-002: The Restore Wizard group mapping controls MUST NOT call Graph for search results or label resolution.
### Group Mapping UX (DB-only)
When group mapping is required, the UI MUST remain Graph-free and MUST provide a DB-only safe input.
- For each unresolved source group, the UI shows a mapping row with:
- Source label: `<displayName or "Security group"> (…<last8>)`
- Target input:
- Allowed values:
- `SKIP` (skip assignment)
- A manually entered Entra ID group objectId (GUID/UUID string)
- Validation rules:
- If not `SKIP`, the value MUST be a syntactically valid UUID
- No network lookup/verification is performed in Phase 1 (Graph-free). Existence is validated later during preview/restore execution paths.
- Helper text MUST explain:
- “Paste the target Entra ID group Object ID (GUID). Names are not resolved in this phase.”
- “Use SKIP to omit the assignment.”
- FR-003: When names cannot be resolved DB-only, the UI MUST show safe fallback labels using masked identifiers.
- `<masked-id>` format: `…<last8>` (last 8 characters of the external identifier, prefixed with an ellipsis)
- FR-004: Add fail-hard Pest feature tests binding `GraphClientInterface` to throw on any invocation and verify:
- Backup Sets index renders OK (HTTP 200 + stable page marker)
- Restore wizard renders OK (HTTP 200 + stable page marker)
## Non-Functional Requirements
- NFR-001: No new Graph calls are introduced in UI code paths.
- NFR-002: No new tables are added in this phase.
- NFR-003: Changes should be low-risk and minimal surface area.
## Out of Scope
- Moving action handlers (snapshot capture, backup create, restore rerun, restore dry-run) to jobs.
- Creating new cache/inventory tables to support group search.
## Success Criteria
- SC-001: Guard tests reliably fail if any Backup/Restore UI render path touches Graph.
- SC-002: Backup and Restore wizard pages render successfully with Graph disabled.
- SC-003: Group mapping UI remains usable with DB-only fallback labels.
## Related Specs
- Follow-up (Phase 2): Backup/Restore job orchestration (TBD)

View File

@ -0,0 +1,156 @@
# Tasks: Backup/Restore UI Graph-Safety (Phase 1)
**Input**: Design documents from `/specs/048-backup-restore-ui-graph-safety/`
**Prerequisites**: plan.md (required), spec.md (required), research.md, data-model.md, contracts/, quickstart.md
**Tests**: REQUIRED (Pest). This feature explicitly adds guard tests (FR-004).
**Organization**: Tasks are grouped by user story to enable independent implementation and testing.
## Format: `- [ ] T### [P?] [US#?] Description with file path`
- **[P]**: Can run in parallel (different files, no dependencies)
- **[US#]**: Which user story this task belongs to (US1, US2)
- Include exact file paths in descriptions
---
## Phase 1: Setup (Shared Infrastructure)
**Purpose**: Confirm scope, lock stable UI markers as concrete strings, and ensure the contracts/quickstart reflect the intended test approach.
- [ ] T001 Confirm tenant-scoped admin URLs for target pages in specs/048-backup-restore-ui-graph-safety/contracts/admin-pages.openapi.yaml
- [ ] T002 Lock stable marker strings and record them in specs/048-backup-restore-ui-graph-safety/quickstart.md:
- Backup Sets index marker: `Created by`
- Restore wizard create marker: `Create restore run` (and wizard step: `Select Backup Set`)
---
## Phase 2: Foundational (Blocking Prerequisites)
**Purpose**: Shared test helpers and a clear boundary that fails-hard when Graph is touched.
**⚠️ CRITICAL**: No user story work should be considered “done” unless the fail-hard Graph binding is used in the storys feature tests.
- [ ] T003 [P] Create a fail-hard Graph client test double in tests/Support/FailHardGraphClient.php (implements App\\Services\\Graph\\GraphClientInterface and throws on any method)
- [ ] T004 Add a reusable binding helper for tests (either a helper function in tests/Pest.php or a trait in tests/Support/) that binds GraphClientInterface to FailHardGraphClient
**Checkpoint**: Foundation ready — both page-render tests can now be implemented.
---
## Phase 3: User Story 1 — Backup Sets index renders Graph-free (Priority: P1) 🎯 MVP
**Goal**: As a tenant admin, I can open the Backup Sets index page even when Graph is disabled.
**Independent Test**: A Pest feature test does an HTTP GET to the tenant-scoped Filament Backup Sets index route and asserts assertOk() + assertSee('Created by') — with Graph bound to fail-hard.
### Tests
- [ ] T005 [P] [US1] Add Pest feature test in tests/Feature/Filament/BackupSetGraphSafetyTest.php:
- bind GraphClientInterface to FailHardGraphClient (fail-hard on ANY invocation)
- HTTP GET App\\Filament\\Resources\\BackupSetResource::getUrl('index', tenant: $tenant)
- assertOk() + assertSee('Created by')
- [ ] T006 [US1] In tests/Feature/Filament/BackupSetGraphSafetyTest.php, add tenant isolation assertions (second tenant data must not render) while still using fail-hard Graph binding
### Implementation
- [ ] T007 [US1] Audit Backup Sets render path for any Graph usage and refactor to DB-only if needed in app/Filament/Resources/BackupSetResource.php and app/Filament/Resources/BackupSetResource/Pages/ListBackupSets.php
**Checkpoint**: Backup Sets index renders (assertOk() + assertSee('Created by')) with fail-hard Graph binding.
---
## Phase 4: User Story 2 — Restore wizard renders Graph-free + DB-only group mapping (Priority: P1)
**Goal**: As a tenant admin, I can open the Restore wizard page and interact with group mapping without any Graph calls in render/mount/options/typeahead.
**Independent Test**: Pest feature tests that (a) render the Restore wizard create page without Graph, and (b) render the group mapping section (using query params to preselect a backup set with group assignments) and verify fallback labels use `…<last8>`.
### Tests
- [ ] T008 [P] [US2] Add Pest feature test in tests/Feature/Filament/RestoreWizardGraphSafetyTest.php:
- bind GraphClientInterface to FailHardGraphClient (fail-hard on ANY invocation)
- HTTP GET App\\Filament\\Resources\\RestoreRunResource::getUrl('create', tenant: $tenant)
- assertOk() + assertSee('Create restore run') + assertSee('Select Backup Set')
- [ ] T009 [P] [US2] Extend tests/Feature/Filament/RestoreWizardGraphSafetyTest.php (or a second file):
- seed a BackupSet + BackupItem with group assignment targets (groupId present)
- HTTP GET create URL with `?backup_set_id=` (and optional `backup_item_ids`/`scope_mode`) to force group mapping render
- keep fail-hard Graph binding (no Graph/typeahead/label resolution allowed)
- [ ] T010 [US2] In the group mapping render test, assert all DB-only UX requirements:
- assertSee('…<last8>') masked fallback appears for source labels
- assertSee('Paste the target Entra ID group Object ID') helper text appears
- assertSee('Use SKIP to omit the assignment.') helper text appears
### Implementation
- [ ] T011 [US2] Remove Graph-dependent typeahead/search from group mapping controls in app/Filament/Resources/RestoreRunResource.php (no Graph/typeahead; remove getSearchResultsUsing paths)
- [ ] T012 [US2] Remove Graph-dependent option label resolution in app/Filament/Resources/RestoreRunResource.php (no Graph label resolution; remove getOptionLabelUsing paths)
- [ ] T013 [US2] Implement DB-only group mapping UX in app/Filament/Resources/RestoreRunResource.php:
- manual target group objectId input (GUID/UUID)
- GUID validation (if not SKIP)
- helper text: “Paste the target Entra ID group Object ID (GUID). Names are not resolved in this phase.” + “Use SKIP to omit the assignment.”
- no Graph/typeahead
- [ ] T014 [US2] Make unresolved group detection DB-only in app/Filament/Resources/RestoreRunResource.php (remove GroupResolver usage from unresolvedGroups() and any other render-time helpers)
- [ ] T015 [US2] Implement masked fallback label formatting `…<last8>` in app/Filament/Resources/RestoreRunResource.php (update formatGroupLabel() and ensure all source labels route through it)
- [ ] T016 [US2] Remove now-unused methods/imports after refactor (e.g., targetGroupOptions(), resolveTargetGroupLabel(), GroupResolver import) in app/Filament/Resources/RestoreRunResource.php
**Checkpoint**: Restore wizard renders (assertOk() + assertSee('Create restore run') + assertSee('Select Backup Set')) and group mapping renders DB-only; tests pass with fail-hard Graph binding.
---
## Phase 5: Polish & Cross-Cutting Concerns
**Purpose**: Keep docs and tooling aligned with the guardrail.
- [ ] T017 [P] Update specs/048-backup-restore-ui-graph-safety/quickstart.md with the final test file names and the exact `artisan test --filter=...` / file commands
- [ ] T018 [P] Update specs/048-backup-restore-ui-graph-safety/contracts/admin-pages.openapi.yaml if any page paths/markers changed during implementation
- [ ] T019 Run formatting (./vendor/bin/pint --dirty) and targeted tests (./vendor/bin/sail artisan test --filter=graph\-safety or the exact files)
---
## Dependencies & Execution Order
### Phase Dependencies
- **Setup (Phase 1)**: No dependencies
- **Foundational (Phase 2)**: Depends on Setup completion — BLOCKS both user stories
- **US1 + US2 (Phases 34)**: Depend on Foundational completion; can proceed in parallel
- **Polish (Phase 5)**: Depends on desired user stories being complete
### User Story Dependencies
- **US1 (P1)**: Depends on T003T004; otherwise independent
- **US2 (P1)**: Depends on T003T004; otherwise independent
### Parallel Opportunities
- T003 and T004 can be developed in parallel if split across files.
- US1 test work (T005T006) and US2 test work (T008T010) can be developed in parallel.
---
## Parallel Example
```bash
# Parallelizable work after Phase 2:
# - US1: tests/Feature/Filament/BackupSetGraphSafetyTest.php
# - US2: tests/Feature/Filament/RestoreWizardGraphSafetyTest.php
# - Shared helper: tests/Support/FailHardGraphClient.php
```
---
## Implementation Strategy
### MVP Scope (Recommended)
1. Phase 12 (setup + fail-hard Graph test binding)
2. Phase 3 (US1: Backup Sets index renders Graph-free)
3. Validate tests + stop
### Then
4. Phase 4 (US2: Restore wizard + group mapping Graph-free)
5. Phase 5 polish