feat(spec-088): remove tenant graphOptions legacy path (#105)

## Summary
- remove tenant-based Graph options access from runtime service paths and enforce provider-only resolution
- add `MicrosoftGraphOptionsResolver` and `ProviderConfigurationRequiredException` for centralized, actionable provider-config errors
- turn `Tenant::graphOptions()` into a fail-fast kill switch to prevent legacy runtime usage
- add and update tests (including guardrail) to enforce no reintroduction in `app/`
- update Spec 088 artifacts (`spec`, `plan`, `research`, `tasks`, checklist)

## Validation
- `vendor/bin/sail bin pint --dirty`
- `vendor/bin/sail artisan test --compact --filter=NoLegacyTenantGraphOptions`
- `vendor/bin/sail artisan test --compact tests/Feature/Filament`
- `CI=1 vendor/bin/sail artisan test --compact`

## Notes
- Branch includes the guardrail test for legacy callsite detection in `app/`.
- Full suite currently green: 1227 passed, 5 skipped.

Co-authored-by: Ahmed Darrazi <ahmeddarrazi@MacBookPro.fritz.box>
Reviewed-on: #105
This commit is contained in:
ahmido 2026-02-12 10:14:44 +00:00
parent 57f3e3934c
commit 1acbf8cc54
56 changed files with 1767 additions and 87 deletions

View File

@ -46,10 +46,8 @@ ## Code Style
PHP 8.4.15: Follow standard conventions
## Recent Changes
- 088-remove-tenant-graphoptions-legacy: Added PHP 8.4.15 (Laravel 12) + Filament v5, Livewire v4, Pest v4
- 086-retire-legacy-runs-into-operation-runs: Spec docs updated (PHP 8.4.15 + Laravel 12, Filament v5, Livewire v4)
- 085-tenant-operate-hub: Added PHP 8.4 (Laravel 12) + Filament v5, Livewire v4, Laravel Sail, Tailwind CSS v4
- 085-tenant-operate-hub: Added PHP 8.4.15 (Laravel 12) + Filament v5, Livewire v4, Pest v4, Tailwind CSS v4, Laravel Sail
- 084-verification-surfaces-unification: Added PHP 8.4 (Laravel 12) + Filament v5 (Livewire v4), Queue/Jobs (Laravel), Microsoft Graph via `GraphClientInterface`
- 082-action-surface-contract: Added PHP 8.4.x + Laravel 12, Filament v5, Livewire v4
<!-- MANUAL ADDITIONS START -->
<!-- MANUAL ADDITIONS END -->

View File

@ -297,11 +297,9 @@ public function graphTenantId(): ?string
*/
public function graphOptions(): array
{
return [
'tenant' => $this->graphTenantId(),
'client_id' => $this->app_client_id,
'client_secret' => $this->app_client_secret,
];
throw new \BadMethodCallException(
'Tenant::graphOptions() has been removed. Resolve a ProviderConnection via ProviderConnectionResolver and build options via ProviderGateway (or use MicrosoftGraphOptionsResolver).'
);
}
public function scopeForTenant(Builder $query, self|int|string $tenant): Builder

View File

@ -35,6 +35,9 @@
use App\Services\Intune\WindowsFeatureUpdateProfileNormalizer;
use App\Services\Intune\WindowsQualityUpdateProfileNormalizer;
use App\Services\Intune\WindowsUpdateRingNormalizer;
use App\Services\Providers\MicrosoftGraphOptionsResolver;
use App\Services\Providers\ProviderConnectionResolver;
use App\Services\Providers\ProviderGateway;
use Filament\Events\TenantSet;
use Illuminate\Cache\RateLimiting\Limit;
use Illuminate\Http\Request;
@ -61,6 +64,13 @@ public function register(): void
return $app->make(NullGraphClient::class);
});
$this->app->singleton(MicrosoftGraphOptionsResolver::class, function ($app): MicrosoftGraphOptionsResolver {
return new MicrosoftGraphOptionsResolver(
connections: $app->make(ProviderConnectionResolver::class),
gateway: $app->make(ProviderGateway::class),
);
});
$this->app->tag(
[
AppProtectionPolicyNormalizer::class,

View File

@ -8,6 +8,7 @@
use App\Services\Graph\AssignmentFilterResolver;
use App\Services\Graph\GroupResolver;
use App\Services\Graph\ScopeTagResolver;
use App\Services\Providers\MicrosoftGraphOptionsResolver;
use Illuminate\Support\Facades\Log;
class AssignmentBackupService
@ -17,6 +18,7 @@ public function __construct(
private readonly GroupResolver $groupResolver,
private readonly AssignmentFilterResolver $assignmentFilterResolver,
private readonly ScopeTagResolver $scopeTagResolver,
private readonly MicrosoftGraphOptionsResolver $graphOptionsResolver,
) {}
/**
@ -58,8 +60,8 @@ public function enrichWithAssignments(
}
// Fetch assignments from Graph API
$graphOptions = $tenant->graphOptions();
$tenantId = $graphOptions['tenant'] ?? $tenant->external_id ?? $tenant->tenant_id;
$graphOptions = $this->graphOptionsResolver->resolveForTenant($tenant);
$tenantId = (string) ($graphOptions['tenant'] ?? '');
$assignments = $this->assignmentFetcher->fetch(
$policyType,
$tenantId,

View File

@ -10,6 +10,7 @@
use App\Services\Graph\GraphLogger;
use App\Services\Graph\GraphResponse;
use App\Services\Intune\AuditLogger;
use App\Services\Providers\MicrosoftGraphOptionsResolver;
use Illuminate\Support\Arr;
use Illuminate\Support\Facades\Log;
@ -21,6 +22,7 @@ public function __construct(
private readonly GraphLogger $graphLogger,
private readonly AuditLogger $auditLogger,
private readonly AssignmentFilterResolver $assignmentFilterResolver,
private readonly MicrosoftGraphOptionsResolver $graphOptionsResolver,
) {}
/**
@ -86,8 +88,8 @@ public function restore(
];
}
$graphOptions = $tenant->graphOptions();
$tenantIdentifier = $graphOptions['tenant'] ?? $tenant->graphTenantId() ?? (string) $tenant->getKey();
$graphOptions = $this->graphOptionsResolver->resolveForTenant($tenant);
$tenantIdentifier = (string) ($graphOptions['tenant'] ?? '');
$context = [
'tenant' => $tenantIdentifier,

View File

@ -2,15 +2,16 @@
namespace App\Services\Directory;
use App\Models\EntraGroup;
use App\Jobs\EntraGroupSyncJob;
use App\Models\EntraGroup;
use App\Models\OperationRun;
use App\Models\Tenant;
use App\Models\User;
use App\Services\OperationRunService;
use App\Services\Graph\GraphClientInterface;
use App\Services\Graph\GraphContractRegistry;
use App\Services\Graph\GraphResponse;
use App\Services\OperationRunService;
use App\Services\Providers\MicrosoftGraphOptionsResolver;
use Carbon\CarbonImmutable;
class EntraGroupSyncService
@ -18,6 +19,7 @@ class EntraGroupSyncService
public function __construct(
private readonly GraphClientInterface $graph,
private readonly GraphContractRegistry $contracts,
private readonly MicrosoftGraphOptionsResolver $graphOptionsResolver,
) {}
public function startManualSync(Tenant $tenant, User $user): OperationRun
@ -103,7 +105,7 @@ public function sync(Tenant $tenant, string $selectionKey): array
$errorSummary = null;
$errorCount = 0;
$options = $tenant->graphOptions();
$options = $this->graphOptionsResolver->resolveForTenant($tenant);
$useQuery = $query;
$nextPath = $path;

View File

@ -11,6 +11,7 @@
use App\Services\Graph\GraphContractRegistry;
use App\Services\Graph\GraphResponse;
use App\Services\OperationRunService;
use App\Services\Providers\MicrosoftGraphOptionsResolver;
use Carbon\CarbonImmutable;
class RoleDefinitionsSyncService
@ -18,6 +19,7 @@ class RoleDefinitionsSyncService
public function __construct(
private readonly GraphClientInterface $graph,
private readonly GraphContractRegistry $contracts,
private readonly MicrosoftGraphOptionsResolver $graphOptionsResolver,
) {}
public function startManualSync(Tenant $tenant, User $user): OperationRun
@ -101,7 +103,7 @@ public function sync(Tenant $tenant): array
$errorSummary = null;
$errorCount = 0;
$options = $tenant->graphOptions();
$options = $this->graphOptionsResolver->resolveForTenant($tenant);
$useQuery = $query;
$nextPath = $path;

View File

@ -3,12 +3,14 @@
namespace App\Services\Graph;
use App\Models\Tenant;
use App\Services\Providers\MicrosoftGraphOptionsResolver;
use Illuminate\Support\Facades\Cache;
class AssignmentFilterResolver
{
public function __construct(
private readonly MicrosoftGraphClient $graphClient,
private readonly MicrosoftGraphOptionsResolver $graphOptionsResolver,
) {}
/**
@ -39,7 +41,7 @@ private function fetchAllFilters(?Tenant $tenant = null): array
$options = ['query' => ['$select' => 'id,displayName']];
if ($tenant) {
$options = array_merge($options, $tenant->graphOptions());
$options = array_merge($options, $this->graphOptionsResolver->resolveForTenant($tenant));
}
$response = $this->graphClient->request(

View File

@ -4,6 +4,7 @@
use App\Models\Tenant;
use App\Services\Graph\GraphClientInterface;
use App\Services\Providers\MicrosoftGraphOptionsResolver;
use Illuminate\Support\Arr;
class ConfigurationPolicyTemplateResolver
@ -25,6 +26,7 @@ class ConfigurationPolicyTemplateResolver
public function __construct(
private readonly GraphClientInterface $graphClient,
private readonly MicrosoftGraphOptionsResolver $graphOptionsResolver,
) {}
/**
@ -155,7 +157,10 @@ public function getTemplate(Tenant $tenant, string $templateId, array $graphOpti
return $this->templateCache[$tenantKey][$templateId];
}
$context = array_merge($tenant->graphOptions(), Arr::except($graphOptions, ['platform']));
$context = array_merge(
$this->graphOptionsResolver->resolveForTenant($tenant),
Arr::except($graphOptions, ['platform']),
);
$path = sprintf('/deviceManagement/configurationPolicyTemplates/%s', urlencode($templateId));
$response = $this->graphClient->request('GET', $path, $context);
@ -189,7 +194,7 @@ public function listTemplatesByFamily(Tenant $tenant, string $templateFamily, ar
$escapedFamily = str_replace("'", "''", $templateFamily);
$context = array_merge($tenant->graphOptions(), Arr::except($graphOptions, ['platform']), [
$context = array_merge($this->graphOptionsResolver->resolveForTenant($tenant), Arr::except($graphOptions, ['platform']), [
'query' => [
'$filter' => "templateFamily eq '{$escapedFamily}'",
'$top' => 999,
@ -228,7 +233,7 @@ public function fetchTemplateSettingDefinitionIds(Tenant $tenant, string $templa
return $this->templateDefinitionCache[$tenantKey][$templateId];
}
$context = array_merge($tenant->graphOptions(), Arr::except($graphOptions, ['platform']), [
$context = array_merge($this->graphOptionsResolver->resolveForTenant($tenant), Arr::except($graphOptions, ['platform']), [
'query' => [
'$expand' => 'settingDefinitions',
'$top' => 999,

View File

@ -6,6 +6,7 @@
use App\Models\Tenant;
use App\Services\Graph\GraphClientInterface;
use App\Services\Graph\GraphContractRegistry;
use App\Services\Providers\MicrosoftGraphOptionsResolver;
use Illuminate\Support\Collection;
class FoundationMappingService
@ -14,6 +15,7 @@ public function __construct(
private readonly GraphClientInterface $graphClient,
private readonly GraphContractRegistry $contracts,
private readonly FoundationSnapshotService $snapshotService,
private readonly MicrosoftGraphOptionsResolver $graphOptionsResolver,
) {}
/**
@ -271,7 +273,7 @@ private function createFoundation(
$response = $this->graphClient->request(
$method,
$resource,
['json' => $payload] + $tenant->graphOptions()
['json' => $payload] + $this->graphOptionsResolver->resolveForTenant($tenant)
);
if ($response->failed()) {

View File

@ -5,12 +5,14 @@
use App\Models\Tenant;
use App\Services\Graph\GraphClientInterface;
use App\Services\Graph\GraphContractRegistry;
use App\Services\Providers\MicrosoftGraphOptionsResolver;
class FoundationSnapshotService
{
public function __construct(
private readonly GraphClientInterface $graphClient,
private readonly GraphContractRegistry $contracts,
private readonly MicrosoftGraphOptionsResolver $graphOptionsResolver,
) {}
/**
@ -39,7 +41,7 @@ public function fetchAll(Tenant $tenant, string $foundationType): array
}
$sanitized = $this->contracts->sanitizeQuery($foundationType, $query);
$options = $tenant->graphOptions();
$options = $this->graphOptionsResolver->resolveForTenant($tenant);
$items = [];
$failures = [];
$nextPath = $resource;

View File

@ -9,6 +9,7 @@
use App\Services\Graph\AssignmentFilterResolver;
use App\Services\Graph\GroupResolver;
use App\Services\Graph\ScopeTagResolver;
use App\Services\Providers\MicrosoftGraphOptionsResolver;
use Illuminate\Support\Facades\Log;
/**
@ -25,6 +26,7 @@ public function __construct(
private readonly GroupResolver $groupResolver,
private readonly AssignmentFilterResolver $assignmentFilterResolver,
private readonly ScopeTagResolver $scopeTagResolver,
private readonly MicrosoftGraphOptionsResolver $graphOptionsResolver,
) {}
/**
@ -40,8 +42,8 @@ public function capture(
?string $createdBy = null,
array $metadata = []
): array {
$graphOptions = $tenant->graphOptions();
$tenantIdentifier = $graphOptions['tenant'] ?? $tenant->graphTenantId() ?? (string) $tenant->getKey();
$graphOptions = $this->graphOptionsResolver->resolveForTenant($tenant);
$tenantIdentifier = (string) ($graphOptions['tenant'] ?? '');
// 1. Fetch policy snapshot
$snapshot = $this->snapshotService->fetch($tenant, $policy, $createdBy);
@ -231,8 +233,8 @@ public function ensureVersionHasAssignments(
bool $includeAssignments = false,
bool $includeScopeTags = false
): PolicyVersion {
$graphOptions = $tenant->graphOptions();
$tenantIdentifier = $graphOptions['tenant'] ?? $tenant->graphTenantId() ?? (string) $tenant->getKey();
$graphOptions = $this->graphOptionsResolver->resolveForTenant($tenant);
$tenantIdentifier = (string) ($graphOptions['tenant'] ?? '');
if ($version->assignments !== null && $version->scope_tags !== null) {
Log::debug('Version already has assignments, skipping', [

View File

@ -8,6 +8,7 @@
use App\Models\PolicyVersion;
use App\Models\Tenant;
use App\Services\Graph\GroupResolver;
use App\Services\Providers\MicrosoftGraphOptionsResolver;
use Carbon\CarbonImmutable;
use Illuminate\Support\Collection;
use Illuminate\Support\Str;
@ -17,6 +18,7 @@ class RestoreRiskChecker
public function __construct(
private readonly GroupResolver $groupResolver,
private readonly ConfigurationPolicyTemplateResolver $templateResolver,
private readonly MicrosoftGraphOptionsResolver $graphOptionsResolver,
) {}
/**
@ -109,8 +111,8 @@ private function checkOrphanedGroups(Tenant $tenant, Collection $policyItems, ar
];
}
$graphOptions = $tenant->graphOptions();
$tenantIdentifier = $graphOptions['tenant'] ?? $tenant->graphTenantId() ?? (string) $tenant->getKey();
$graphOptions = $this->graphOptionsResolver->resolveForTenant($tenant);
$tenantIdentifier = (string) ($graphOptions['tenant'] ?? '');
$resolved = $this->groupResolver->resolveGroupIds($groupIds, $tenantIdentifier, $graphOptions);
$orphaned = [];
@ -241,7 +243,7 @@ private function checkEndpointSecurityTemplates(Tenant $tenant, Collection $poli
{
$issues = [];
$hasRestoreEnabled = false;
$graphOptions = $tenant->graphOptions();
$graphOptions = $this->graphOptionsResolver->resolveForTenant($tenant);
foreach ($policyItems as $item) {
if ($item->policy_type !== 'endpointSecurityPolicy') {

View File

@ -5,18 +5,32 @@
use App\Models\Tenant;
use App\Services\Graph\GraphClientInterface;
use App\Services\Graph\GraphErrorMapper;
use App\Services\Providers\MicrosoftGraphOptionsResolver;
use App\Services\Providers\ProviderConfigurationRequiredException;
use App\Support\Providers\ProviderReasonCodes;
use Throwable;
class TenantConfigService
{
public function __construct(private readonly GraphClientInterface $graphClient) {}
public function __construct(
private readonly GraphClientInterface $graphClient,
private readonly MicrosoftGraphOptionsResolver $graphOptionsResolver,
) {}
/**
* @return array{success:bool,error_message:?string,requires_consent:bool}
*/
public function testConnectivity(Tenant $tenant): array
{
$options = $this->graphOptions($tenant);
try {
$options = $this->graphOptions($tenant);
} catch (ProviderConfigurationRequiredException $exception) {
return [
'success' => false,
'error_message' => $exception->getMessage(),
'requires_consent' => $exception->reasonCode === ProviderReasonCodes::ProviderConsentMissing,
];
}
if ($options['tenant'] === null) {
return [
@ -52,11 +66,11 @@ public function testConnectivity(Tenant $tenant): array
}
/**
* @return array{tenant:?string,client_id:?string,client_secret:?string}
* @return array<string, mixed>
*/
public function graphOptions(Tenant $tenant): array
{
return $tenant->graphOptions();
return $this->graphOptionsResolver->resolveForTenant($tenant);
}
private function requiresConsent(string $message): bool

View File

@ -5,12 +5,16 @@
use App\Models\Tenant;
use App\Models\TenantPermission;
use App\Services\Graph\GraphClientInterface;
use App\Services\Providers\MicrosoftGraphOptionsResolver;
use DateTimeInterface;
use Illuminate\Support\Carbon;
class TenantPermissionService
{
public function __construct(private readonly GraphClientInterface $graphClient) {}
public function __construct(
private readonly GraphClientInterface $graphClient,
private readonly MicrosoftGraphOptionsResolver $graphOptionsResolver,
) {}
/**
* @return array<int, array{key:string,type:string,description:?string,features:array<int,string>}>
@ -73,11 +77,14 @@ public function compare(
if ($liveCheck && $grantedStatuses === null) {
$liveCheckMeta['attempted'] = true;
if (! is_array($graphOptions)) {
$graphOptions = $this->graphOptionsResolver->resolveForTenant($tenant);
}
$appId = null;
if (is_array($graphOptions) && is_string($graphOptions['client_id'] ?? null) && $graphOptions['client_id'] !== '') {
if (is_string($graphOptions['client_id'] ?? null) && $graphOptions['client_id'] !== '') {
$appId = (string) $graphOptions['client_id'];
} elseif (is_string($tenant->graphOptions()['client_id'] ?? null) && $tenant->graphOptions()['client_id'] !== '') {
$appId = (string) $tenant->graphOptions()['client_id'];
}
if ($appId !== null) {
@ -328,8 +335,10 @@ private function configuredGrantedKeys(): array
private function fetchLivePermissions(Tenant $tenant, ?array $graphOptions = null): array
{
try {
$graphOptions ??= $this->graphOptionsResolver->resolveForTenant($tenant);
$response = $this->graphClient->getServicePrincipalPermissions(
$graphOptions ?? $tenant->graphOptions()
$graphOptions
);
if (! $response->success) {

View File

@ -9,6 +9,7 @@
use App\Services\Graph\AssignmentFilterResolver;
use App\Services\Graph\GroupResolver;
use App\Services\Graph\ScopeTagResolver;
use App\Services\Providers\MicrosoftGraphOptionsResolver;
use Carbon\CarbonImmutable;
use Illuminate\Database\QueryException;
use Illuminate\Database\UniqueConstraintViolationException;
@ -23,6 +24,7 @@ public function __construct(
private readonly GroupResolver $groupResolver,
private readonly AssignmentFilterResolver $assignmentFilterResolver,
private readonly ScopeTagResolver $scopeTagResolver,
private readonly MicrosoftGraphOptionsResolver $graphOptionsResolver,
) {}
public function captureVersion(
@ -119,8 +121,8 @@ public function captureFromGraph(
bool $includeAssignments = true,
bool $includeScopeTags = true,
): PolicyVersion {
$graphOptions = $tenant->graphOptions();
$tenantIdentifier = $graphOptions['tenant'] ?? $tenant->graphTenantId() ?? (string) $tenant->getKey();
$graphOptions = $this->graphOptionsResolver->resolveForTenant($tenant);
$tenantIdentifier = (string) ($graphOptions['tenant'] ?? '');
$snapshot = $this->snapshotService->fetch($tenant, $policy, $createdBy);

View File

@ -0,0 +1,31 @@
<?php
namespace App\Services\Providers;
use App\Models\Tenant;
final class MicrosoftGraphOptionsResolver
{
public function __construct(
private readonly ProviderConnectionResolver $connections,
private readonly ProviderGateway $gateway,
) {}
/**
* @return array<string, mixed>
*/
public function resolveForTenant(Tenant $tenant, array $overrides = []): array
{
$resolution = $this->connections->resolveDefault($tenant, 'microsoft');
if (! $resolution->resolved || $resolution->connection === null) {
throw ProviderConfigurationRequiredException::forTenant(
$tenant,
provider: 'microsoft',
resolution: $resolution,
);
}
return $this->gateway->graphOptions($resolution->connection, $overrides);
}
}

View File

@ -0,0 +1,30 @@
<?php
namespace App\Services\Providers;
use App\Models\Tenant;
use RuntimeException;
final class ProviderConfigurationRequiredException extends RuntimeException
{
public function __construct(
public readonly int $tenantId,
public readonly string $provider,
public readonly string $reasonCode,
public readonly ?string $extensionReasonCode = null,
string $message = 'Provider configuration is required.',
) {
parent::__construct($message);
}
public static function forTenant(Tenant $tenant, string $provider, ProviderConnectionResolution $resolution): self
{
return new self(
tenantId: (int) $tenant->getKey(),
provider: $provider,
reasonCode: $resolution->effectiveReasonCode(),
extensionReasonCode: $resolution->extensionReasonCode,
message: $resolution->message ?? 'Provider configuration is required.',
);
}
}

View File

@ -0,0 +1,770 @@
# Spec 087 — Legacy Runs Removal: Analysis Report
> **Report Date:** 2026-02-11
> **Scope:** Analyse → Backfill → Cleanup → Drop for all non-canonical run tracking tables.
> **Canonical Run System:** `operation_runs` table + `OperationRun` model + `/admin/operations/{run}`
---
## 1) Executive Summary
### Legacy Sources Identified: **4** (plus 1 already dropped)
| # | Source Table | Classification | Current State | Risk |
|---|---|---|---|---|
| 1 | `inventory_sync_runs` | **Legacy run tracking** | Active writes, has FK `operation_run_id` (nullable) | Medium — Drift depends on this as entity |
| 2 | `entra_group_sync_runs` | **Legacy run tracking** | Active writes, has FK `operation_run_id` (nullable) | Low — hidden nav, small footprint |
| 3 | `backup_schedule_runs` | **Legacy run tracking** | Active writes, has FK `operation_run_id` (nullable) | High — scheduler writes here every minute |
| 4 | `restore_runs` | **Domain entity** (NOT purely legacy tracking) | Active writes, has FK `operation_run_id` (nullable), SoftDeletes | High — 1,980-line resource, wizard, 20+ tests |
| 5 | `bulk_operation_runs` | **Already dropped** | Migration `2026_01_18` drops table; guard test prevents references | None — done |
### Biggest Risks
1. **`restore_runs` is a domain entity**, not just run tracking — it holds `requested_items`, `preview`, `results`, `group_mapping`, `metadata`. Its execution tracking (status, timestamps, counts) should come from `operation_runs`, but the entity itself must survive. **It cannot be dropped.**
2. **Drift detection** depends on `InventorySyncRun` as a domain entity for baseline/current run references (`Finding.baseline_run_id`, `Finding.current_run_id`, `InventoryItem.last_seen_run_id`). Dropping this table requires migrating those FKs to `operation_run_id`.
3. **BackupScheduleRun** is written by `RunBackupScheduleJob` on every scheduled execution — the scheduler produces ~N rows/minute where N = active schedules. Backfill must handle high volume.
4. **Graph API calls in UI** are limited to `TenantResource.php` (RBAC setup modal, not in run rendering) — **no run-related UI makes Graph calls**.
### Quick Wins
- `EntraGroupSyncRunResource` (`$shouldRegisterNavigation = false`) — lowest friction to remove.
- `BulkOperationRun` — already dropped, guard test exists.
- Both `InventorySyncRunResource.ViewInventorySyncRun` and `EntraGroupSyncRunResource.ViewEntraGroupSyncRun` already redirect to canonical `TenantlessOperationRunViewer` when `operation_run_id` is set.
---
## 2) Inventory: Table-by-Table
### 2a) `inventory_sync_runs`
| Aspect | Detail |
|---|---|
| **Migration** | `2026_01_07_142719_create_inventory_sync_runs_table.php` |
| **Columns** | `id`, `tenant_id` (FK), `user_id` (FK, nullable, added `2026_01_09`), `selection_hash` (64), `selection_payload` (jsonb, nullable), `status` (string), `had_errors` (bool), `error_codes` (jsonb), `error_context` (jsonb), `started_at` (timestampTz), `finished_at` (timestampTz), `items_observed_count`, `items_upserted_count`, `errors_count`, `operation_run_id` (FK, nullable, added `2026_02_10`), `created_at`, `updated_at` |
| **FK Relationships** | `tenant_id → tenants`, `user_id → users`, `operation_run_id → operation_runs` |
| **Indexes** | `(tenant_id, selection_hash)`, `(tenant_id, status)`, `(tenant_id, finished_at)`, `(tenant_id, user_id)`, `operation_run_id` |
| **Model** | `App\Models\InventorySyncRun` (59 lines) |
| **Status constants** | `STATUS_PENDING`, `STATUS_RUNNING`, `STATUS_SUCCESS`, `STATUS_PARTIAL`, `STATUS_FAILED`, `STATUS_SKIPPED` |
| **Factory** | `InventorySyncRunFactory` ✓ |
| **Row count** | unknown (SQL: `SELECT COUNT(*) FROM inventory_sync_runs;`) |
| **Classification** | **Hybrid** — legacy run tracking BUT also used as domain entity by Drift (`Finding.baseline_run_id`, `Finding.current_run_id`, `InventoryItem.last_seen_run_id`) |
| **Inbound FKs** | `findings.baseline_run_id`, `findings.current_run_id`, `inventory_items.last_seen_run_id` (all → `inventory_sync_runs.id`), `operation_runs` ← via `inventory_sync_runs.operation_run_id` |
**Start surfaces that write to this table:**
| Surface | File | Line(s) |
|---|---|---|
| `InventorySyncService::startSync()` | `app/Services/Inventory/InventorySyncService.php` | L43, L62, L68 |
**Filament UI:**
| Component | File | Lines |
|---|---|---|
| `InventorySyncRunResource` | `app/Filament/Resources/InventorySyncRunResource.php` | 231 |
| `ListInventorySyncRuns` | `…/Pages/ListInventorySyncRuns.php` | 19 |
| `ViewInventorySyncRun` (redirects to OpRun) | `…/Pages/ViewInventorySyncRun.php` | 24 |
| `InventoryKpiHeader` widget (reads last sync) | `app/Filament/Widgets/Inventory/InventoryKpiHeader.php` | 163 |
| Nav: registered in Inventory cluster, sort=2, label="Sync History" | — | — |
---
### 2b) `entra_group_sync_runs`
| Aspect | Detail |
|---|---|
| **Migration** | `2026_01_11_120004_create_entra_group_sync_runs_table.php` |
| **Columns** | `id`, `tenant_id` (FK, cascadeOnDelete), `selection_key`, `slot_key` (nullable), `status`, `error_code` (nullable), `error_category` (nullable), `error_summary` (text, nullable), `safety_stop_triggered` (bool), `safety_stop_reason` (nullable), `pages_fetched`, `items_observed_count`, `items_upserted_count`, `error_count`, `initiator_user_id` (FK users, nullable), `started_at` (timestampTz), `finished_at` (timestampTz), `operation_run_id` (FK, nullable, added `2026_02_10`), `created_at`, `updated_at` |
| **FK Relationships** | `tenant_id → tenants`, `initiator_user_id → users`, `operation_run_id → operation_runs` |
| **Indexes** | `(tenant_id, selection_key)`, `(tenant_id, status)`, `(tenant_id, finished_at)`, `UNIQUE(tenant_id, selection_key, slot_key)`, `operation_run_id` |
| **Model** | `App\Models\EntraGroupSyncRun` (40 lines) |
| **Status constants** | `STATUS_PENDING`, `STATUS_RUNNING`, `STATUS_SUCCEEDED`, `STATUS_FAILED`, `STATUS_PARTIAL` |
| **Factory** | `EntraGroupSyncRunFactory` ✓ |
| **Row count** | unknown (SQL: `SELECT COUNT(*) FROM entra_group_sync_runs;`) |
| **Classification** | **Pure legacy run tracking** — no inbound FKs from other domain tables |
| **Inbound FKs** | None |
**Start surfaces that write to this table:**
| Surface | File | Line(s) |
|---|---|---|
| `EntraGroupSyncJob::handle()` | `app/Jobs/EntraGroupSyncJob.php` | L56-L227 |
**Filament UI:**
| Component | File | Lines |
|---|---|---|
| `EntraGroupSyncRunResource` | `app/Filament/Resources/EntraGroupSyncRunResource.php` | 168 |
| `ListEntraGroupSyncRuns` | `…/Pages/ListEntraGroupSyncRuns.php` | 11 |
| `ViewEntraGroupSyncRun` (redirects to OpRun) | `…/Pages/ViewEntraGroupSyncRun.php` | 24 |
| Nav: **hidden** (`$shouldRegisterNavigation = false`) | — | — |
---
### 2c) `backup_schedule_runs`
| Aspect | Detail |
|---|---|
| **Migration** | `2026_01_05_011034_create_backup_schedule_runs_table.php` |
| **Columns** | `id`, `backup_schedule_id` (FK, cascadeOnDelete), `tenant_id` (FK, cascadeOnDelete), `scheduled_for` (datetime), `started_at` (datetime, nullable), `finished_at` (datetime, nullable), `status` (enum: running/success/partial/failed/canceled/skipped), `summary` (json, nullable), `error_code` (nullable), `error_message` (text, nullable), `backup_set_id` (FK, nullable), `user_id` (FK, added `2026_01_06`), `operation_run_id` (FK, nullable, added `2026_02_10`), `created_at`, `updated_at` |
| **FK Relationships** | `backup_schedule_id → backup_schedules`, `tenant_id → tenants`, `backup_set_id → backup_sets`, `user_id → users`, `operation_run_id → operation_runs` |
| **Indexes** | `UNIQUE(backup_schedule_id, scheduled_for)`, `(backup_schedule_id, scheduled_for)`, `(tenant_id, created_at)`, `operation_run_id` |
| **Model** | `App\Models\BackupScheduleRun` (53 lines) |
| **Status constants** | `STATUS_RUNNING`, `STATUS_SUCCESS`, `STATUS_PARTIAL`, `STATUS_FAILED`, `STATUS_CANCELED`, `STATUS_SKIPPED` |
| **Factory** | **None** |
| **Row count** | unknown (SQL: `SELECT COUNT(*) FROM backup_schedule_runs;`) |
| **Classification** | **Legacy run tracking** — but references `backup_set_id` (produced artifact) |
| **Inbound FKs** | None |
**Start surfaces that write to this table:**
| Surface | File | Line(s) |
|---|---|---|
| `RunBackupScheduleJob::handle()` | `app/Jobs/RunBackupScheduleJob.php` | L40, L80, L119-L1035 |
**Filament UI:**
| Component | File | Lines |
|---|---|---|
| `BackupScheduleRunsRelationManager` | `app/Filament/Resources/BackupScheduleResource/RelationManagers/BackupScheduleRunsRelationManager.php` | 107 |
| `BackupScheduleOperationRunsRelationManager` (canonical) | `…/RelationManagers/BackupScheduleOperationRunsRelationManager.php` | 96 |
| Blade view: `backup-schedule-run-view.blade.php` (modal) | `resources/views/filament/modals/backup-schedule-run-view.blade.php` | 48 |
| Nav: no dedicated nav; shown as RelationManager tab on BackupSchedule | — | — |
---
### 2d) `restore_runs`
| Aspect | Detail |
|---|---|
| **Migration** | `2025_12_10_000150_create_restore_runs_table.php` + several amendments |
| **Columns** | `id`, `tenant_id` (FK, cascadeOnDelete), `backup_set_id` (FK, cascadeOnDelete), `requested_by` (nullable), `is_dry_run` (bool), `status` (string, uses `RestoreRunStatus` enum), `requested_items` (json), `preview` (json), `results` (json), `failure_reason` (text, nullable), `metadata` (json), `group_mapping` (json), `idempotency_key` (string, nullable), `started_at`, `completed_at`, `operation_run_id` (FK, nullable, added `2026_02_10`), `deleted_at` (soft deletes), `created_at`, `updated_at` |
| **FK Relationships** | `tenant_id → tenants`, `backup_set_id → backup_sets`, `operation_run_id → operation_runs` |
| **Indexes** | `(tenant_id, status)`, `started_at`, `completed_at`, `operation_run_id` |
| **Model** | `App\Models\RestoreRun` (157 lines, `SoftDeletes`) |
| **Status enum** | `RestoreRunStatus` (13 cases: Draft→Scoped→Checked→Previewed→Pending→Queued→Running→Completed→Partial→Failed→Cancelled→Aborted→CompletedWithErrors) |
| **Factory** | `RestoreRunFactory` ✓ |
| **Row count** | unknown (SQL: `SELECT COUNT(*) FROM restore_runs;`) |
| **Classification** | **Domain entity** — NOT droppable. Holds wizard state, preview diffs, group mappings, results. Execution tracking portion migrates to `operation_runs`. |
| **Inbound FKs** | None from other tables (but `BackupSet.restoreRuns()` hasMany) |
**Start surfaces that write to this table:**
| Surface | File | Line(s) |
|---|---|---|
| `RestoreService::startRestore()` | `app/Services/Intune/RestoreService.php` | L120, L208, L213 |
| `RestoreRunResource` wizard | `app/Filament/Resources/RestoreRunResource.php` | ~1,980 lines total |
| `RestoreRunObserver``SyncRestoreRunToOperationRun` | `app/Observers/RestoreRunObserver.php` | L13-L30 |
| `ExecuteRestoreRunJob` | `app/Jobs/ExecuteRestoreRunJob.php` | L29-L155 |
**Filament UI:**
| Component | File | Lines |
|---|---|---|
| `RestoreRunResource` (largest resource) | `app/Filament/Resources/RestoreRunResource.php` | 1,980 |
| `ListRestoreRuns` | `…/Pages/ListRestoreRuns.php` | 21 |
| `CreateRestoreRun` (wizard) | `…/Pages/CreateRestoreRun.php` | 167 |
| `ViewRestoreRun` | `…/Pages/ViewRestoreRun.php` | 11 |
| Blade: `restore-run-preview.blade.php` | `resources/views/filament/forms/components/restore-run-preview.blade.php` | 180 |
| Blade: `restore-run-checks.blade.php` | `resources/views/filament/forms/components/restore-run-checks.blade.php` | 121 |
| Nav: registered in tenant panel under "Backups & Restore" | — | — |
---
### 2e) `bulk_operation_runs` — **ALREADY DROPPED**
| Aspect | Detail |
|---|---|
| **Drop migration** | `2026_01_18_000001_drop_bulk_operation_runs_table.php` |
| **Model** | Deleted — `BulkOperationRun` class does not exist |
| **Guard test** | `tests/Feature/Guards/NoLegacyBulkOperationsTest.php` — scans codebase for `\bBulkOperationRun\b` |
| **Status** | ✅ Complete. No action needed. |
---
## 3) Backfill Plan per Source
### Existing Infrastructure
All 4 legacy tables already have `operation_run_id` FK (nullable, added `2026_02_10_*`). This means:
- **New runs** may already link to `operation_runs` (depends on whether the write path sets the FK).
- **Historical runs** (before 2026-02-10) have `operation_run_id = NULL` and need backfill.
### 3a) `inventory_sync_runs``operation_runs` Backfill
```
OperationRun mapping:
├── type = 'inventory.sync' (OperationRunType::InventorySync)
├── status = legacy.status mapped:
│ ├── pending → queued
│ ├── running → running
│ ├── success → completed
│ ├── partial → completed
│ ├── failed → completed
│ └── skipped → completed
├── outcome = legacy.status mapped:
│ ├── pending → pending
│ ├── running → pending
│ ├── success → succeeded
│ ├── partial → partially_succeeded
│ ├── failed → failed
│ └── skipped → cancelled
├── run_identity_hash = sha256("{tenant_id}|inventory.sync|{selection_hash}")
├── summary_counts = {
│ total: legacy.items_observed_count,
│ succeeded: legacy.items_upserted_count,
│ failed: legacy.errors_count
│ }
├── failure_summary = legacy.error_codes?.map(c => {code: c, reason_code: 'unknown', message: ''}) ?? []
├── context = {
│ inputs: { selection_hash: legacy.selection_hash },
│ selection_payload: legacy.selection_payload,
│ legacy: { source: 'inventory_sync_runs', id: legacy.id }
│ }
├── tenant_id = legacy.tenant_id
├── user_id = legacy.user_id
├── initiator_name = User.name ?? 'system'
├── started_at = legacy.started_at
├── completed_at = legacy.finished_at
├── created_at = legacy.created_at
└── updated_at = legacy.updated_at
```
**Missing data:** None significant. `user_id` may be NULL for system-initiated syncs.
**Post-backfill:** Set `inventory_sync_runs.operation_run_id = created_operation_run.id`.
**Drift FK migration:** After backfill, update:
- `findings.baseline_run_id` → new column `findings.baseline_operation_run_id`
- `findings.current_run_id` → new column `findings.current_operation_run_id`
- `inventory_items.last_seen_run_id` → new column `inventory_items.last_seen_operation_run_id`
---
### 3b) `entra_group_sync_runs``operation_runs` Backfill
```
OperationRun mapping:
├── type = 'directory_groups.sync' (OperationRunType::DirectoryGroupsSync)
├── status = legacy.status mapped:
│ ├── pending → queued
│ ├── running → running
│ ├── succeeded → completed
│ ├── failed → completed
│ └── partial → completed
├── outcome = legacy.status mapped:
│ ├── pending → pending
│ ├── running → pending
│ ├── succeeded → succeeded
│ ├── failed → failed
│ └── partial → partially_succeeded
├── run_identity_hash = sha256("{tenant_id}|directory_groups.sync|{selection_key}:{slot_key}")
├── summary_counts = {
│ total: legacy.items_observed_count,
│ succeeded: legacy.items_upserted_count,
│ failed: legacy.error_count,
│ items: legacy.pages_fetched
│ }
├── failure_summary = legacy.error_code ?
│ [{code: legacy.error_code, reason_code: legacy.error_category ?? 'unknown',
│ message: substr(legacy.error_summary, 0, 120)}] : []
├── context = {
│ inputs: { selection_key: legacy.selection_key, slot_key: legacy.slot_key },
│ safety_stop: { triggered: legacy.safety_stop_triggered, reason: legacy.safety_stop_reason },
│ legacy: { source: 'entra_group_sync_runs', id: legacy.id }
│ }
├── tenant_id = legacy.tenant_id
├── user_id = legacy.initiator_user_id
├── initiator_name = User.name ?? 'scheduler'
├── started_at = legacy.started_at
├── completed_at = legacy.finished_at
├── created_at = legacy.created_at
└── updated_at = legacy.updated_at
```
**Missing data:** None — all fields map cleanly.
**Post-backfill:** Set `entra_group_sync_runs.operation_run_id = created_operation_run.id`.
---
### 3c) `backup_schedule_runs``operation_runs` Backfill
```
OperationRun mapping:
├── type = legacy→type mapping:
│ └── All historical: 'backup_schedule.scheduled' (OperationRunType::BackupScheduleScheduled)
│ NOTE: run_now/retry distinction is lost in historical data
├── status = legacy.status mapped:
│ ├── running → running
│ ├── success → completed
│ ├── partial → completed
│ ├── failed → completed
│ ├── canceled → completed
│ └── skipped → completed
├── outcome = legacy.status mapped:
│ ├── running → pending
│ ├── success → succeeded
│ ├── partial → partially_succeeded
│ ├── failed → failed
│ ├── canceled → cancelled
│ └── skipped → cancelled
├── run_identity_hash = sha256("{tenant_id}|backup_schedule.scheduled|{schedule_id}:{scheduled_for_iso}")
├── summary_counts = legacy.summary (JSON) — extract from structure:
│ { total: summary.total_policies ?? 0, succeeded: summary.backed_up ?? 0,
│ failed: summary.errors ?? 0 }
├── failure_summary = legacy.error_code ?
│ [{code: legacy.error_code, reason_code: 'unknown', message: substr(legacy.error_message, 0, 120)}] : []
├── context = {
│ inputs: {
│ backup_schedule_id: legacy.backup_schedule_id,
│ scheduled_for: legacy.scheduled_for
│ },
│ backup_set_id: legacy.backup_set_id,
│ legacy: { source: 'backup_schedule_runs', id: legacy.id }
│ }
├── tenant_id = legacy.tenant_id
├── user_id = legacy.user_id
├── initiator_name = User.name ?? 'scheduler'
├── started_at = legacy.started_at
├── completed_at = legacy.finished_at
├── created_at = legacy.created_at
└── updated_at = legacy.updated_at
```
**Missing data:**
- `type` distinction (run_now vs retry vs scheduled) is lost for historical rows — all backfilled as `backup_schedule.scheduled`.
- `summary` JSON structure varies — needs defensive parsing.
**Post-backfill:** Set `backup_schedule_runs.operation_run_id = created_operation_run.id`.
---
### 3d) `restore_runs``operation_runs` Backfill
**NOTE:** `restore_runs` is a domain entity. Only the execution-tracking portion (status, timestamps, counts) is backfilled to `operation_runs`. The entity itself persists.
```
OperationRun mapping:
├── type = 'restore.execute' (OperationRunType::RestoreExecute)
│ NOTE: only Queued→Running→Completed states get an OpRun.
│ Draft/Scoped/Checked/Previewed are wizard states, NOT execution runs.
├── SKIP condition = legacy.status IN (Draft, Scoped, Checked, Previewed, Pending) → do NOT backfill
├── status = legacy.status mapped:
│ ├── Queued → queued
│ ├── Running → running
│ ├── Completed → completed
│ ├── Partial → completed
│ ├── Failed → completed
│ ├── Cancelled → completed
│ ├── Aborted → completed
│ └── CompletedWithErrors → completed
├── outcome = legacy.status mapped:
│ ├── Queued → pending
│ ├── Running → pending
│ ├── Completed → succeeded
│ ├── Partial → partially_succeeded
│ ├── Failed → failed
│ ├── Cancelled → cancelled
│ ├── Aborted → failed
│ └── CompletedWithErrors → partially_succeeded
├── run_identity_hash = sha256("{tenant_id}|restore.execute|{restore_run_id}")
├── summary_counts = derived from legacy.results JSON:
│ { total: count(results.items), succeeded: count(results where ok), failed: count(results where !ok) }
├── failure_summary = legacy.failure_reason ?
│ [{code: 'restore.failed', reason_code: 'unknown', message: substr(legacy.failure_reason, 0, 120)}] : []
├── context = {
│ inputs: { restore_run_id: legacy.id, backup_set_id: legacy.backup_set_id },
│ is_dry_run: legacy.is_dry_run,
│ legacy: { source: 'restore_runs', id: legacy.id }
│ }
├── tenant_id = legacy.tenant_id
├── user_id = NULL (legacy has requested_by as string, not FK)
├── initiator_name = legacy.requested_by ?? 'system'
├── started_at = legacy.started_at
├── completed_at = legacy.completed_at
├── created_at = legacy.created_at
└── updated_at = legacy.updated_at
```
**Missing data:**
- `user_id``requested_by` is a string name, not an FK. Backfill can attempt `User::where('name', requested_by)->first()` but may not resolve.
- `summary_counts``results` JSON structure varies; needs defensive parsing.
**Post-backfill:** Set `restore_runs.operation_run_id = created_operation_run.id`.
**Existing sync infrastructure:**
- `RestoreRunObserver` already syncs RestoreRun → OperationRun on create/update via `SyncRestoreRunToOperationRun`.
- `AdapterRunReconciler` reconciles RestoreRun status → OperationRun via scheduled `ReconcileAdapterRunsJob` (every 30 min).
---
### 3e) Existing `operation_run_id` Link Check
All 4 legacy tables already have `operation_run_id` FK (nullable):
| Table | Migration | FK constraint | On delete |
|---|---|---|---|
| `inventory_sync_runs` | `2026_02_10_090213` | `constrained('operation_runs')` | `nullOnDelete` |
| `entra_group_sync_runs` | `2026_02_10_090214` | `constrained('operation_runs')` | `nullOnDelete` |
| `backup_schedule_runs` | `2026_02_10_090215` | `constrained('operation_runs')` | `nullOnDelete` |
| `restore_runs` | `2026_02_10_115908` | `constrained('operation_runs')` | `nullOnDelete` |
**Current behavior:** New write paths (post 2026-02-10) *should* be populating this FK, but needs verification per table:
- `InventorySyncService` — needs audit (may not set FK yet)
- `EntraGroupSyncJob` — needs audit
- `RunBackupScheduleJob` — needs audit (reconcile command exists for this)
- `RestoreRunObserver``SyncRestoreRunToOperationRun`**actively syncs** (confirmed)
SQL to verify:
```sql
SELECT 'inventory_sync_runs' AS tbl, COUNT(*) AS total,
COUNT(operation_run_id) AS linked, COUNT(*) - COUNT(operation_run_id) AS unlinked
FROM inventory_sync_runs
UNION ALL
SELECT 'entra_group_sync_runs', COUNT(*), COUNT(operation_run_id), COUNT(*) - COUNT(operation_run_id)
FROM entra_group_sync_runs
UNION ALL
SELECT 'backup_schedule_runs', COUNT(*), COUNT(operation_run_id), COUNT(*) - COUNT(operation_run_id)
FROM backup_schedule_runs
UNION ALL
SELECT 'restore_runs', COUNT(*), COUNT(operation_run_id), COUNT(*) - COUNT(operation_run_id)
FROM restore_runs;
```
---
## 4) Cleanup Plan
### 4a) Code Removal
#### Phase 1: `EntraGroupSyncRunResource` (Quick Win)
| What | File | Action |
|---|---|---|
| Resource | `app/Filament/Resources/EntraGroupSyncRunResource.php` (168 lines) | Delete |
| List page | `…/Pages/ListEntraGroupSyncRuns.php` (11 lines) | Delete |
| View page | `…/Pages/ViewEntraGroupSyncRun.php` (24 lines) | Delete |
| Policy | `app/Policies/EntraGroupSyncRunPolicy.php` | Delete |
| Badge class | `app/Support/Badges/Domains/EntraGroupSyncRunStatusBadge.php` | Delete |
| Badge enum case | `BadgeDomain::EntraGroupSyncRun` → remove case | Edit |
| Badge catalog mapping | `BadgeCatalog` → remove mapping | Edit |
| Notification link | `RunStatusChangedNotification.php` → remove entra branch | Edit |
#### Phase 2: `InventorySyncRunResource` (Medium — drift dependency)
| What | File | Action |
|---|---|---|
| Resource | `app/Filament/Resources/InventorySyncRunResource.php` (231 lines) | Delete |
| List page | `…/Pages/ListInventorySyncRuns.php` (19 lines) | Delete |
| View page | `…/Pages/ViewInventorySyncRun.php` (24 lines) | Delete |
| Badge class | `app/Support/Badges/Domains/InventorySyncRunStatusBadge.php` | Delete |
| Badge enum case | `BadgeDomain::InventorySyncRun` → remove case | Edit |
| Badge catalog mapping | `BadgeCatalog` → remove mapping | Edit |
| KPI widget refs | `InventoryKpiHeader.php` → update to query `operation_runs` only | Edit |
| Nav item | Remove "Sync History" from Inventory cluster | — |
**Prerequisite:** Drift tables must migrate FKs from `inventory_sync_runs.id``operation_runs.id`:
- `findings.baseline_run_id` + `findings.current_run_id``findings.baseline_operation_run_id` + `findings.current_operation_run_id`
- `inventory_items.last_seen_run_id``inventory_items.last_seen_operation_run_id`
#### Phase 3: `BackupScheduleRunsRelationManager` (Medium)
| What | File | Action |
|---|---|---|
| Legacy RM | `BackupScheduleRunsRelationManager.php` (107 lines) | Delete |
| Blade modal | `backup-schedule-run-view.blade.php` (48 lines) | Delete |
| Badge class | `app/Support/Badges/Domains/BackupScheduleRunStatusBadge.php` | Delete |
| Badge enum case | `BadgeDomain::BackupScheduleRun` → remove case | Edit |
| Badge catalog mapping | `BadgeCatalog` → remove mapping | Edit |
| BackupSchedule resource | Remove legacy RM, keep `BackupScheduleOperationRunsRelationManager` | Edit |
| Reconcile command | `TenantpilotReconcileBackupScheduleOperationRuns` → keep for transition, then archive | — |
#### Phase 4: `RestoreRun` (Largest — domain entity)
**`RestoreRun` is NOT removed** — it is a domain entity. The cleanup targets:
| What | File | Action |
|---|---|---|
| Status tracking columns | Remove `status` duplication → read from `operationRun.status` | Deprecate, keep column |
| `started_at`, `completed_at` | Redundant with `operationRun.started_at/completed_at` | Deprecate, keep column |
| `failure_reason` | Move to `operationRun.failure_summary` | Deprecate, keep column |
| `RestoreRunObserver` sync | Keep (ensures OpRun is created on RestoreRun lifecycle) | Keep |
| `AdapterRunReconciler` | Eventually remove once all status reads go through OpRun | Keep for transition |
| Badge class | `app/Support/Badges/Domains/RestoreRunBadges` → derive from OpRun status | Edit |
### 4b) Service/Job Cleanup
| File | Action |
|---|---|
| `InventorySyncService.php` | Stop writing `InventorySyncRun::create()`, create only `OperationRun` |
| `EntraGroupSyncJob.php` | Stop writing `EntraGroupSyncRun::create()`, use only `OperationRun` |
| `RunBackupScheduleJob.php` | Stop writing `BackupScheduleRun::create()`, use only `OperationRun` |
| `ApplyBackupScheduleRetentionJob.php` | Update to read retention data from `OperationRun` context |
| `TenantpilotPurgeNonPersistentData.php` | Remove legacy table references |
| DriftRunSelector, DriftScopeKey, DriftFindingGenerator | Update to use `OperationRun` (after FK migration) |
### 4c) Model Cleanup (after drop)
| Model | Action |
|---|---|
| `InventorySyncRun` | Delete (after drift FK migration + table drop) |
| `EntraGroupSyncRun` | Delete (after table drop) |
| `BackupScheduleRun` | Delete (after table drop) |
| `RestoreRun` | **Keep** — domain entity |
| Factories for above | Delete respective factories |
---
## 5) Redirect Strategy
### Existing Routes for Legacy Run Views
| Pattern | Name | Current Handler |
|---|---|---|
| `GET /admin/t/{tenant}/inventory-sync-runs/{record}` | `filament.tenant.resources.inventory-sync-runs.view` | `ViewInventorySyncRun` |
| `GET /admin/t/{tenant}/inventory-sync-runs` | `filament.tenant.resources.inventory-sync-runs.index` | `ListInventorySyncRuns` |
| `GET /admin/entra-group-sync-runs/{record}` | `filament.admin.resources.entra-group-sync-runs.view` | `ViewEntraGroupSyncRun` |
| `GET /admin/entra-group-sync-runs` | `filament.admin.resources.entra-group-sync-runs.index` | `ListEntraGroupSyncRuns` |
| `GET /admin/t/{tenant}/restore-runs` | `filament.tenant.resources.restore-runs.index` | `ListRestoreRuns` |
| `GET /admin/t/{tenant}/restore-runs/create` | `filament.tenant.resources.restore-runs.create` | `CreateRestoreRun` |
| `GET /admin/t/{tenant}/restore-runs/{record}` | `filament.tenant.resources.restore-runs.view` | `ViewRestoreRun` |
### Redirect Logic
For **dropped** resources (inventory sync, entra group sync):
```php
// Register in routes/web.php after Resources are deleted
Route::get('/admin/t/{tenant}/inventory-sync-runs/{record}', function ($tenant, $record) {
$opRunId = DB::table('inventory_sync_runs')
->where('id', $record)->value('operation_run_id');
if ($opRunId) {
return redirect()->route('admin.operations.view', ['run' => $opRunId]);
}
abort(404);
});
// List pages → redirect to Operations index
Route::get('/admin/t/{tenant}/inventory-sync-runs', fn() =>
redirect()->route('admin.operations.index'));
// Same pattern for entra-group-sync-runs
```
**Constraint:** Redirect must NEVER create new `OperationRun` rows — only lookup + redirect.
For `backup_schedule_runs`: No direct URL exists (only RelationManager modal), so no redirect needed.
For `restore_runs`: **Not dropped** — resource stays. No redirect needed.
---
## 6) Drop Plan
### Tables to Drop (in order)
| # | Table | Preconditions | Migration name |
|---|---|---|---|
| 1 | `entra_group_sync_runs` | All rows have `operation_run_id` ≠ NULL; no inbound FKs | `drop_entra_group_sync_runs_table` |
| 2 | `backup_schedule_runs` | All rows have `operation_run_id` ≠ NULL; no inbound FKs | `drop_backup_schedule_runs_table` |
| 3 | `inventory_sync_runs` | All rows have `operation_run_id` ≠ NULL; drift FKs migrated to `operation_run_id` | `drop_inventory_sync_runs_table` |
**NOT dropped:** `restore_runs` (domain entity), `operation_runs` (canonical).
### Preconditions per Table
#### `entra_group_sync_runs`
- [ ] Backfill complete: `SELECT COUNT(*) FROM entra_group_sync_runs WHERE operation_run_id IS NULL` = 0
- [ ] Verify count matches: `SELECT COUNT(*) FROM entra_group_sync_runs``SELECT COUNT(*) FROM operation_runs WHERE type = 'directory_groups.sync' AND context->>'legacy'->>'source' = 'entra_group_sync_runs'`
- [ ] No code writes to table: arch guard test passes (`grep -r 'EntraGroupSyncRun' app/ | wc -l` = 0, excluding model file)
- [ ] Filament resource deleted
- [ ] Redirect route registered
- [ ] DB snapshot taken
#### `backup_schedule_runs`
- [ ] Backfill complete: `SELECT COUNT(*) FROM backup_schedule_runs WHERE operation_run_id IS NULL` = 0
- [ ] `RunBackupScheduleJob` writes only to `operation_runs`
- [ ] `TenantpilotReconcileBackupScheduleOperationRuns` command removed/archived
- [ ] `ApplyBackupScheduleRetentionJob` reads from `operation_runs`
- [ ] DB snapshot taken
#### `inventory_sync_runs`
- [ ] Backfill complete: all rows linked
- [ ] Drift FK migration complete: `findings.*_run_id` + `inventory_items.last_seen_run_id``operation_run_id` columns
- [ ] `InventorySyncService` writes only to `operation_runs`
- [ ] All Drift services read from `OperationRun`
- [ ] DB snapshot taken
### Release Gates
Each drop migration should:
1. Check precondition (backfill count) in `up()` — abort if unlinked rows exist
2. Create backup table `_archive_{table}` before dropping (for rollback)
3. Drop FK constraints before dropping table
4. Be reversible: `down()` recreates table from archive
---
## 7) Hotspot Files (Top 15)
| # | File | Lines | Impact | Why it's hot |
|---|---|---|---|---|
| 1 | `app/Filament/Resources/RestoreRunResource.php` | 1,980 | High | Largest resource; full wizard, execution tracking, must decouple status reads |
| 2 | `app/Jobs/RunBackupScheduleJob.php` | ~1,035 | High | Heavy schedule→BackupScheduleRun writer; must rewrite to OpRun-only |
| 3 | `app/Services/OperationRunService.php` | 808 | Medium | Canonical service; backfill must use this; no breaking changes allowed |
| 4 | `app/Filament/Resources/OperationRunResource.php` | 483 | Medium | Canonical viewer; must absorb data previously shown in legacy resources |
| 5 | `app/Services/Inventory/InventorySyncService.php` | ~100 | Medium | Creates InventorySyncRun; must rewrite to OpRun-only |
| 6 | `app/Jobs/EntraGroupSyncJob.php` | ~227 | Medium | Creates EntraGroupSyncRun; must rewrite to OpRun-only |
| 7 | `app/Filament/Resources/InventorySyncRunResource.php` | 231 | Low | Will be deleted entirely |
| 8 | `app/Jobs/ExecuteRestoreRunJob.php` | ~155 | Medium | Bridges RestoreRun↔OpRun; sync logic must be verified |
| 9 | `app/Filament/Resources/EntraGroupSyncRunResource.php` | 168 | Low | Will be deleted entirely |
| 10 | `app/Filament/Pages/Monitoring/Operations.php` | 149 | Low | Must ensure all legacy run data is visible here after migration |
| 11 | `app/Filament/Pages/Operations/TenantlessOperationRunViewer.php` | 142 | Low | Canonical viewer page; needs "related links" for all legacy sources |
| 12 | `app/Filament/Widgets/Inventory/InventoryKpiHeader.php` | 163 | Medium | Reads InventorySyncRun; must switch to OpRun queries |
| 13 | `app/Listeners/SyncRestoreRunToOperationRun.php` | ~21 | Low | Existing bridge; must be verified/kept during transition |
| 14 | `app/Services/AdapterRunReconciler.php` | ~253 | Medium | Reconciles RestoreRun→OpRun; may be simplified or removed |
| 15 | `app/Console/Commands/TenantpilotReconcileBackupScheduleOperationRuns.php` | ~130 | Low | Reconcile command; kept during transition, then archived |
---
## 8) Test Impact Map
### Tests That WILL Break (must be updated/deleted)
| # | Test File | Legacy Model | Action Required |
|---|---|---|---|
| 1 | `tests/Feature/Filament/InventorySyncRunResourceTest.php` | InventorySyncRun | Delete |
| 2 | `tests/Feature/Filament/EntraGroupSyncRunResourceTest.php` | EntraGroupSyncRun | Delete |
| 3 | `tests/Feature/Filament/InventoryPagesTest.php` | InventorySyncRunResource URL | Update URLs |
| 4 | `tests/Feature/Filament/InventoryHubDbOnlyTest.php` | InventorySyncRunResource URL | Update URLs |
| 5 | `tests/Feature/Guards/ActionSurfaceContractTest.php` | InventorySyncRunResource | Remove references |
| 6 | `tests/Feature/Operations/LegacyRunRedirectTest.php` | Both sync resources | Update to test redirect routes |
| 7 | `tests/Feature/Console/ReconcileBackupScheduleOperationRunsCommandTest.php` | BackupScheduleRun | Delete or convert |
| 8 | `tests/Feature/Console/PurgeNonPersistentDataCommandTest.php` | BackupScheduleRun + RestoreRun | Update to remove legacy refs |
| 9 | `tests/Feature/Rbac/EntraGroupSyncRunsUiEnforcementTest.php` | EntraGroupSyncRunResource | Delete |
| 10 | `tests/Feature/DirectoryGroups/StartSyncFromGroupsPageTest.php` | EntraGroupSyncRun counts | Update assertions |
| 11 | `tests/Feature/DirectoryGroups/ScheduledSyncDispatchTest.php` | EntraGroupSyncRun counts | Update assertions |
| 12 | `tests/Feature/RunStartAuthorizationTest.php` | All legacy models | Update to assert OpRun creation |
### Tests That STAY (RestoreRun is domain entity):
All 20 RestoreRun test files survive because RestoreRun persists. But tests that check `RestoreRun.status` directly may need to verify via `OperationRun` instead.
### Tests That STAY (Drift uses InventorySyncRun):
15 Drift test files — until the Drift FK migration is complete, these stay. After migration, they update to use `OperationRun` factories.
### 3 New Tests Proposed
#### Test 1: Backfill Idempotency
```php
it('backfill is idempotent — running twice produces the same operation_run links', function () {
// Create legacy rows, run backfill command, verify links
// Run backfill again, verify no duplicate OperationRuns created
// Assert operation_run_id unchanged on legacy rows
});
```
#### Test 2: Verify Counts Match
```php
it('all legacy runs are linked to exactly one operation run after backfill', function () {
// For each legacy table:
// Assert COUNT(*) WHERE operation_run_id IS NULL = 0
// Assert COUNT(DISTINCT operation_run_id) = COUNT(*)
// Assert OpRun count for type matches legacy count
});
```
#### Test 3: Redirect Resolution
```php
it('legacy run view URLs redirect to canonical operation run viewer', function () {
$syncRun = InventorySyncRun::factory()->create(['operation_run_id' => $opRun->id]);
$this->get("/admin/t/{$tenant->external_id}/inventory-sync-runs/{$syncRun->id}")
->assertRedirect(route('admin.operations.view', ['run' => $opRun->id]));
});
it('returns 404 when legacy run has no operation_run_id', function () {
$syncRun = InventorySyncRun::factory()->create(['operation_run_id' => null]);
$this->get("/admin/t/{$tenant->external_id}/inventory-sync-runs/{$syncRun->id}")
->assertNotFound();
});
```
---
## 9) Graph/HTTP Call Sites in UI (Guardrail Audit)
**Run-related rendering has ZERO Graph/HTTP calls.** ✅
The only Graph call sites in Filament UI are in `TenantResource.php` (RBAC setup modal):
| Call Site | File | Lines | Risk |
|---|---|---|---|
| `searchRoleDefinitionsDelegated()` | `app/Filament/Resources/TenantResource.php` | L1267-L1271 | HIGH (keystroke-triggered) |
| `groupSearchOptions()` (scope_group_id) | `app/Filament/Resources/TenantResource.php` | L1387-L1395 | HIGH (keystroke-triggered) |
| `groupSearchOptions()` (existing_group_id) | `app/Filament/Resources/TenantResource.php` | L1387-L1395 | HIGH (keystroke-triggered) |
These are **not run-related** and are not in scope for Spec 087, but documented for completeness.
---
## 10) Search Queries Used
Key ripgrep/search queries used to produce this report:
```bash
rg 'create_.*runs.*_table|_runs_table' database/migrations/
rg 'operation_run' database/migrations/
rg 'InventorySyncRun' app/ tests/ --files-with-matches
rg 'EntraGroupSyncRun' app/ tests/ --files-with-matches
rg 'BackupScheduleRun' app/ tests/ --files-with-matches
rg 'RestoreRun' app/ tests/ --files-with-matches
rg 'BulkOperationRun' app/ tests/ --files-with-matches
rg 'getSearchResultsUsing|getOptionLabelUsing' app/Filament/
rg 'GraphClient|Http::' app/Filament/
rg 'OperationRunType' app/ --files-with-matches
rg '::create\(' app/Services/Inventory/ app/Jobs/EntraGroupSyncJob.php app/Jobs/RunBackupScheduleJob.php
find app/Models -name '*Run*.php'
find app/Filament -name '*Run*' -o -name '*Operation*'
find tests/ -name '*Run*' -o -name '*run*'
```
---
## 11) Implementation Sequence (Recommended)
```
Phase 0: Audit & verify current operation_run_id fill rates (SQL queries above)
Phase 1: Backfill command (artisan tenantpilot:backfill-legacy-runs)
- Process each table independently, idempotent
- Write operation_run_id back to legacy rows
- Verify counts
Phase 2: Cleanup Code — Quick Wins
2a) Delete EntraGroupSyncRunResource + tests + badge + policy
2b) Register redirect routes
2c) Update guard tests
Phase 3: Cleanup Code — Medium
3a) Rewrite EntraGroupSyncJob to use OperationRun only
3b) Delete InventorySyncRunResource + tests + badge
3c) Migrate Drift FKs (new migration)
3d) Rewrite InventorySyncService to use OperationRun only
3e) Update InventoryKpiHeader widget
Phase 4: Cleanup Code — Heavy
4a) Rewrite RunBackupScheduleJob to use OperationRun only
4b) Delete BackupScheduleRunsRelationManager + blade modal + badge
4c) Delete reconcile command
Phase 5: RestoreRun Decoupling
5a) Deprecate status/timestamp reads from RestoreRun → read from OpRun
5b) Keep RestoreRun entity (wizard state, results, group_mapping)
5c) Simplify/remove AdapterRunReconciler
Phase 6: Drop Migrations (with gates)
6a) Drop entra_group_sync_runs (quick — no inbound FKs)
6b) Drop backup_schedule_runs (medium — verify retention logic)
6c) Drop inventory_sync_runs (last — after drift FK migration)
Phase 7: Final Cleanup
7a) Delete legacy models (InventorySyncRun, EntraGroupSyncRun, BackupScheduleRun)
7b) Delete factories
7c) Remove BadgeDomain enum cases
7d) Update NoLegacyBulkOperationsTest guard scope to cover all dropped models
```

View File

@ -0,0 +1,35 @@
# Specification Quality Checklist: Remove Legacy Graph Options
**Purpose**: Validate specification completeness and quality before proceeding to planning
**Created**: 2026-02-11
**Feature**: specs/088-remove-tenant-graphoptions-legacy/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
- Validation pass (2026-02-11): Spec is ready for planning.
- Optional DB cleanup is explicitly gated (only after zero usage confirmed).

View File

@ -0,0 +1,11 @@
# Contracts — Remove Legacy Tenant Graph Options
This feature introduces **no new HTTP/API endpoints** and does not change the Microsoft Graph contract registry.
## Why `contracts/` exists anyway
The SpecKit workflow expects a `contracts/` directory for features that introduce or modify external interfaces.
## Contract impact for this feature
- **No changes** to `config/graph_contracts.php` are required.
- **No new** application routes/controllers are introduced.
- The change is limited to **internal configuration sourcing** (provider connection resolution + guardrail).

View File

@ -0,0 +1,40 @@
# Data Model — Remove Legacy Tenant Graph Options
## Summary
This feature is a behavioral refactor only. It changes **how Graph credentials/options are sourced** (provider connection only) and adds a CI guardrail. No schema changes are included.
## Entities (existing)
### Tenant (`app/Models/Tenant.php`)
- **Relevant fields (legacy)**: `app_client_id`, `app_client_secret`, `tenant_id`, `external_id`
- **Relevant method (deprecated)**: `graphOptions(): array`
- **Planned behavior**: `graphOptions()` remains but throws (kill-switch) to prevent legacy use.
### ProviderConnection (`app/Models/ProviderConnection.php`)
- **Used by**: `ProviderConnectionResolver::resolveDefault($tenant, 'microsoft')`
- **Key fields**: `tenant_id`, `provider`, `is_default`, `status`, `entra_tenant_id`
### ProviderCredential (`app/Models/ProviderCredential.php`)
- **Used by**: `CredentialManager::getClientCredentials($connection)` via `ProviderGateway::graphOptions()`
- **Expected payload**: `['client_id' => string, 'client_secret' => string]`
## Relationships (existing)
- `Tenant::providerConnections()` → hasMany `ProviderConnection`
- `ProviderConnection::credential()` → hasOne/hasMany `ProviderCredential` (via relationship method in model)
## Validation / Constraints
- Provider connection resolution must fail deterministically when:
- No default connection exists for tenant/provider
- Multiple defaults exist
- Connection is disabled / needs consent
- Missing `entra_tenant_id`
- Missing/invalid credential payload
(These rules are currently enforced by `ProviderConnectionResolver`.)
## State Transitions
- None added by this feature.
## Out of Scope
- Dropping / migrating tenant credential columns.
- Changing provider resolution semantics.

View File

@ -0,0 +1,105 @@
# Implementation Plan: Remove Legacy Tenant Graph Options
**Branch**: `088-remove-tenant-graphoptions-legacy` | **Date**: 2026-02-11 | **Spec**: [spec.md](spec.md)
**Input**: Feature specification in [spec.md](spec.md)
## Summary
Hard cut removal of the deprecated tenant-based Graph options accessor. All Microsoft Graph configuration must be derived exclusively from a resolved `ProviderConnection` via `ProviderConnectionResolver::resolveDefault()` and `ProviderGateway::graphOptions()`. Add a CI guardrail scanning `app/` for `$tenant->graphOptions()` / `Tenant::graphOptions()` and implement a kill-switch by making `Tenant::graphOptions()` throw.
## Technical Context
**Language/Version**: PHP 8.4.15 (Laravel 12)
**Primary Dependencies**: Filament v5, Livewire v4, Pest v4
**Storage**: PostgreSQL (Sail locally)
**Testing**: Pest via `vendor/bin/sail artisan test --compact`
**Target Platform**: Containerized Linux deployment (Sail local, Dokploy staging/prod)
**Project Type**: Laravel web application (monolith)
**Performance Goals**: N/A (internal refactor + CI guard)
**Constraints**:
- No fallback to tenant-stored credentials.
- Guardrail scans `app/` only (do not fail due to `tests/`).
- No new Graph endpoints; Graph calls remain routed through `GraphClientInterface`.
**Scale/Scope**: Repo-wide refactor of call sites in `app/Services/**`.
## Constitution Check
*GATE: Must pass before implementation. Re-check after design decisions.*
- Inventory-first / snapshots: **N/A** (no inventory/snapshot semantics change)
- Read/write separation: **PASS** (no new write surfaces; existing flows only refactored)
- Single contract path to Graph: **PASS** (Graph calls remain through `GraphClientInterface`; only options sourcing changes)
- Deterministic capabilities: **N/A** (no capability mapping changes)
- RBAC-UX / workspace isolation / tenant isolation: **N/A** (no routing or authorization semantic changes)
- Run observability: **N/A** (no new operations or run tracking changes)
- Data minimization / safe logging: **PASS** (no secrets introduced; provider credentials remain in credential store)
- Filament UI Action Surface Contract: **N/A** (no UI resources/pages are added or modified)
## Project Structure
### Documentation (this feature)
```text
specs/088-remove-tenant-graphoptions-legacy/
├── plan.md
├── research.md
├── data-model.md
├── quickstart.md
├── contracts/
└── tasks.md # created by /speckit.tasks (later)
```
### Source Code (repository root)
```text
app/
├── Models/
│ └── Tenant.php
├── Services/
│ ├── Providers/
│ │ ├── ProviderConnectionResolver.php
│ │ └── ProviderGateway.php
│ └── ... (multiple Graph-enabled services)
tests/
└── Feature/
└── Guards/
```
**Structure Decision**: Laravel monolith; refactor stays within existing services/models.
## Implementation Plan
### Phase 0 — Outline & Research (completed)
- Documented decisions and call sites in [research.md](research.md).
### Phase 1 — Design & Implementation (planned)
1. **Kill-switch the deprecated accessor**
- Update `Tenant::graphOptions()` in `app/Models/Tenant.php` to throw a clear exception explaining that provider connections are required.
2. **Refactor all `app/` call sites off `$tenant->graphOptions()`**
- For each call site, resolve the default Microsoft provider connection via `ProviderConnectionResolver::resolveDefault($tenant, 'microsoft')`.
- If resolution fails, fail fast with a clear, actionable error (include the effective reason code/message).
- Replace option building with `ProviderGateway::graphOptions($connection, $overrides)`.
- Update `app/Services/Intune/TenantConfigService.php` (currently returns `$tenant->graphOptions()`), so it no longer provides a tenant-based path.
3. **Add CI guardrail (Pest)**
- Add a new guard test under `tests/Feature/Guards/` that scans `app/` for forbidden patterns:
- `\$tenant->graphOptions(`
- `Tenant::graphOptions(`
- Ensure failure message explains how to fix (use provider connection resolution).
4. **Verification**
- Run the guard test and the most relevant subsets described in [quickstart.md](quickstart.md).
- Run `vendor/bin/sail bin pint --dirty`.
### Phase 2 — Tasks (next step)
- Completed: `tasks.md` has been generated and Phase 1 is broken into smaller, reviewable steps.
## Complexity Tracking
No constitution violations are required for this feature.

View File

@ -0,0 +1,22 @@
# Quickstart — Remove Legacy Tenant Graph Options
## Preconditions
- Sail is running: `vendor/bin/sail up -d`
## What to run (during implementation)
### 1) Run the guardrail test (fast)
Once implemented, run the guard that enforces no deprecated accessor usage:
- `vendor/bin/sail artisan test --compact --filter=NoLegacyTenantGraphOptions`
### 2) Run the most relevant test subset
After refactoring Graph option sourcing, run tests touching provider resolution / Graph-enabled flows:
- `vendor/bin/sail artisan test --compact tests/Feature/Guards`
## Manual sanity checks (optional)
- If a tenant has **no default provider connection**, Graph-enabled flows should fail with a clear “provider connection required” style error.
- If a tenant has a configured default provider connection, flows should continue to work using `ProviderGateway::graphOptions($connection)`.
## Formatting
Before finalizing the implementation PR:
- `vendor/bin/sail bin pint --dirty`

View File

@ -0,0 +1,71 @@
# Research — Remove Legacy Tenant Graph Options
## Goal
Remove usage of the deprecated tenant-based Graph options accessor (`$tenant->graphOptions()` / `Tenant::graphOptions()`) and make provider-based resolution (`ProviderConnectionResolver` + `ProviderGateway`) the single source of truth for Microsoft Graph configuration.
## Decisions
### Decision: Provider connection is canonical for Graph options
- **Chosen**: Resolve `ProviderConnection` via `App\Services\Providers\ProviderConnectionResolver::resolveDefault($tenant, 'microsoft')`, then derive options via `App\Services\Providers\ProviderGateway::graphOptions($connection, $overrides)`.
- **Rationale**: Existing services already follow this path (e.g. onboarding/health); it centralizes credential storage and validation.
- **Alternatives considered**:
- Keep reading tenant columns (`Tenant.app_client_id` / `Tenant.app_client_secret`) as fallback — rejected (explicitly forbidden; introduces mixed credential sources).
### Decision: Kill-switch behavior for `Tenant::graphOptions()`
- **Chosen**: Keep method for now, but make it throw a clear exception (fail-fast).
- **Rationale**: Ensures any missed call site fails deterministically in CI/runtime.
- **Alternatives considered**:
- Delete method outright — rejected (higher risk / more disruptive API break).
### Decision: Guardrail scope and matching rules
- **Chosen**: Add a CI guard test that scans **`app/` only** for these forbidden patterns:
- `\$tenant->graphOptions(`
- `Tenant::graphOptions(`
- **Rationale**: Matches the agreed scope and avoids false positives on unrelated `graphOptions(...)` methods (e.g. `ProviderGateway::graphOptions`).
- **Alternatives considered**:
- Scan for all `->graphOptions(` — rejected (would flag many legitimate methods).
- Scan `tests/` too — rejected (would create noisy failures during refactors).
## Current Code Findings (Call Sites)
### Deprecated tenant accessor call sites (must be removed)
These are current `app/` references to `$tenant->graphOptions()`:
- `app/Services/AssignmentRestoreService.php`
- `app/Services/AssignmentBackupService.php`
- `app/Services/Intune/RestoreRiskChecker.php`
- `app/Services/Intune/TenantPermissionService.php`
- `app/Services/Intune/VersionService.php`
- `app/Services/Intune/FoundationMappingService.php`
- `app/Services/Intune/PolicyCaptureOrchestrator.php`
- `app/Services/Intune/FoundationSnapshotService.php`
- `app/Services/Intune/TenantConfigService.php` (wraps + returns `$tenant->graphOptions()`)
- `app/Services/Intune/ConfigurationPolicyTemplateResolver.php`
- `app/Services/Directory/EntraGroupSyncService.php`
- `app/Services/Directory/RoleDefinitionsSyncService.php`
- `app/Services/Graph/AssignmentFilterResolver.php`
### Canonical provider-based examples to copy
- `app/Services/Intune/RbacOnboardingService.php`
- `app/Services/Intune/RbacHealthService.php`
- `app/Services/Intune/RestoreService.php`
- `app/Services/Intune/PolicySnapshotService.php`
### Source of legacy accessor
- `app/Models/Tenant.php` defines the deprecated `graphOptions()` method.
## Recommended Refactor Pattern
```php
$resolution = $this->providerConnections()->resolveDefault($tenant, 'microsoft');
if (! $resolution->resolved || ! $resolution->connection instanceof ProviderConnection) {
// Fail fast with actionable reason.
}
$connection = $resolution->connection;
$graphOptions = $this->providerGateway()->graphOptions($connection, $overrides);
```
## Notes / Non-goals
- No database schema changes (tenant credential columns stay for now).
- No changes to Graph contract registry (`config/graph_contracts.php`) are required for this feature; this work changes configuration sourcing only.

View File

@ -0,0 +1,88 @@
# Feature Specification: Remove Legacy Graph Options
**Feature Branch**: `088-remove-tenant-graphoptions-legacy`
**Created**: 2026-02-11
**Status**: Draft
**Input**: Remove the legacy tenant-based Graph options path and make provider-based resolution the only source of Graph configuration.
## Clarifications
### Session 2026-02-11
- Q: How should `Tenant::graphOptions()` be made unusable (kill-switch)? → A: Keep the method, but make it throw a clear exception so any remaining call sites fail fast in CI/runtime.
- Q: Should we drop legacy tenant credential DB fields as part of this feature? → A: No — defer DB schema cleanup to a follow-up feature.
- Q: Which pattern should the CI guardrail block? → A: Block any call to the deprecated tenant accessor (`$tenant->graphOptions()` and `Tenant::graphOptions()`), but do not block other unrelated `graphOptions(...)` methods.
- Q: Should the CI guardrail scan only production code, or also tests? → A: Scan `app/` only.
## User Scenarios & Testing *(mandatory)*
### User Story 1 - Safe Refactors via Single Credential Source (Priority: P1)
As a maintainer, I want all Microsoft Graph configuration to come from a single, canonical provider-based source, so refactors do not accidentally mix credential sources or introduce inconsistent authentication behavior.
**Why this priority**: This removes a major source of drift and breakage, making changes safer and predictable.
**Independent Test**: Can be fully tested by running the automated test suite and verifying that Graph configuration is resolved only via the canonical provider connection, with no tenant-stored credential fallback.
**Acceptance Scenarios**:
1. **Given** a tenant with a valid provider connection configured, **When** any Graph-enabled flow starts, **Then** it uses only the provider connection-derived configuration.
2. **Given** a tenant without a configured provider connection, **When** a Graph-enabled flow starts, **Then** the flow fails deterministically without using any tenant-stored credentials.
---
### User Story 2 - Prevent Reintroduction of Legacy Paths (Priority: P2)
As a maintainer, I want an automated guardrail that fails CI if the deprecated tenant-based Graph options accessor is reintroduced, so the codebase cannot regress silently.
**Why this priority**: Prevents recurring tech debt and ensures the hard cut remains enforced.
**Independent Test**: Can be fully tested by introducing a deliberate reference in a sandbox change and confirming CI fails.
**Acceptance Scenarios**:
1. **Given** a change that reintroduces a usage of the deprecated tenant-based Graph options accessor, **When** tests run in CI, **Then** the pipeline fails with a clear message describing the forbidden pattern.
---
### Edge Cases
- Tenants that previously relied on tenant-stored credentials must not silently keep working; they should fail clearly until a provider connection is configured.
- Flows that run non-interactively (jobs/scheduled work) must behave consistently with interactive flows: no alternate credential source.
## Requirements *(mandatory)*
### Assumptions
- The canonical source of truth for Microsoft Graph configuration is the provider connection resolved for the tenant (default provider key: `microsoft`).
- No user-facing navigation or UI changes are required.
- Provider connection resolution semantics remain unchanged.
- Dropping/altering deprecated tenant credential fields in the database is out of scope for this feature.
### Definitions
- **Provider configuration is “missing or invalid”** when `ProviderConnectionResolver::resolveDefault($tenant, 'microsoft')` results in a blocked resolution (e.g. missing default connection, disabled/needs consent, missing/invalid credentials).
- Downstream Microsoft Graph authentication / HTTP failures are still treated as runtime Graph errors; this feature only removes the legacy tenant-credential configuration path and standardizes option sourcing.
### Out of Scope
- Any database schema changes to remove deprecated tenant credential fields (follow-up feature).
### Functional Requirements
- **FR-001**: The system MUST not use the deprecated tenant-based Graph options accessor anywhere under `app/`.
- **FR-002**: The system MUST derive Microsoft Graph configuration exclusively from the canonical provider-based resolution for the tenant.
- **FR-003**: The system MUST NOT fallback to tenant-stored credentials if provider-based configuration is missing or invalid.
- **FR-004**: If provider-based configuration is missing or invalid (see Definitions), the system MUST fail fast with a clear, actionable error indicating that provider configuration is required.
- **FR-005**: The system MUST include an automated guardrail that fails CI if the deprecated tenant-based Graph options accessor is referenced in application code (instance or static): `$tenant->graphOptions()` or `Tenant::graphOptions()`.
- **FR-005a**: The guardrail MUST scan `app/` (production code) and SHOULD NOT fail due to references in `tests/`.
- **FR-006**: The deprecated tenant-based Graph options accessor MUST throw a clear exception if called (kill-switch), and MUST NOT attempt to resolve credentials from tenant-stored fields.
## Success Criteria *(mandatory)*
### Measurable Outcomes
- **SC-001**: CI includes a guardrail that reports 0 allowed references to the deprecated tenant-based Graph options accessor.
- **SC-002**: 100% of Graph-enabled flows use provider-based resolution as their configuration source.
- **SC-003**: The existing automated test suite passes without disabling tests or reducing coverage.
- **SC-004**: When provider configuration is missing, affected flows fail with a consistent, actionable error (no silent fallback behavior).

View File

@ -0,0 +1,128 @@
---
description: "Task list for implementing Spec 088"
---
# Tasks: Remove Legacy Tenant Graph Options
**Input**: Design documents from `/specs/088-remove-tenant-graphoptions-legacy/`
**Prerequisites**: plan.md (required), spec.md (required), research.md, data-model.md, contracts/, quickstart.md
**Tests**: REQUIRED (Pest) — this feature changes runtime behavior and introduces a CI guardrail.
## Phase 1: Setup (Shared Infrastructure)
- [x] T001 Verify test + format tooling paths (composer.json, phpunit.xml, specs/088-remove-tenant-graphoptions-legacy/quickstart.md)
- [x] T002 [P] Add a focused test runner note to specs/088-remove-tenant-graphoptions-legacy/quickstart.md (include `--filter=NoLegacyTenantGraphOptions`)
---
## Phase 2: Foundational (Blocking Prerequisites)
**⚠️ CRITICAL**: No user story work can begin until this phase is complete.
- [x] T003 Create provider-only Graph options resolver in app/Services/Providers/MicrosoftGraphOptionsResolver.php
- [x] T004 [P] Create exception for missing/invalid provider config in app/Services/Providers/ProviderConfigurationRequiredException.php
- [x] T005 Wire resolver dependencies (ProviderConnectionResolver + ProviderGateway) in app/Providers/AppServiceProvider.php
- [x] T006 Add tests for resolver failure/success paths in tests/Feature/Providers/MicrosoftGraphOptionsResolverTest.php
- [x] T007 Implement kill-switch: make Tenant::graphOptions() throw in app/Models/Tenant.php
- [x] T008 Add test asserting kill-switch behavior in tests/Feature/Models/TenantGraphOptionsKillSwitchTest.php
**Checkpoint**: A single reusable provider-only resolver exists, with tests, and the legacy accessor fails fast.
---
## Phase 3: User Story 1 — Safe Refactors via Single Credential Source (Priority: P1) 🎯 MVP
**Goal**: All Graph-enabled flows use provider connection-derived configuration only.
**Independent Test**: `vendor/bin/sail artisan test --compact --filter=MicrosoftGraphOptionsResolverTest` + run a few refactored service tests (as applicable) and ensure no `app/` references remain.
### Implementation
- [x] T009 [P] [US1] Refactor graph options usage in app/Services/AssignmentRestoreService.php (replace `$tenant->graphOptions()` with resolver)
- [x] T010 [P] [US1] Refactor graph options usage in app/Services/AssignmentBackupService.php (replace `$tenant->graphOptions()` with resolver)
- [x] T011 [P] [US1] Refactor graph options usage in app/Services/Intune/RestoreRiskChecker.php (replace `$tenant->graphOptions()` with resolver)
- [x] T012 [P] [US1] Refactor graph options usage in app/Services/Intune/TenantPermissionService.php (derive `client_id` from provider-based options; no tenant fallback)
- [x] T013 [P] [US1] Refactor graph options usage in app/Services/Intune/VersionService.php (replace `$tenant->graphOptions()` with resolver)
- [x] T014 [P] [US1] Refactor graph options usage in app/Services/Intune/FoundationMappingService.php (replace array merge with provider-based options)
- [x] T015 [P] [US1] Refactor graph options usage in app/Services/Intune/PolicyCaptureOrchestrator.php (replace `$tenant->graphOptions()` with resolver)
- [x] T016 [P] [US1] Refactor graph options usage in app/Services/Intune/FoundationSnapshotService.php (replace `$tenant->graphOptions()` with resolver)
- [x] T017 [P] [US1] Refactor/remove legacy wrapper in app/Services/Intune/TenantConfigService.php (must not return `$tenant->graphOptions()`; use resolver)
- [x] T018 [P] [US1] Refactor context building in app/Services/Intune/ConfigurationPolicyTemplateResolver.php (replace `array_merge($tenant->graphOptions(), ...)` with provider-based context)
- [x] T019 [P] [US1] Refactor graph options usage in app/Services/Directory/EntraGroupSyncService.php (replace `$tenant->graphOptions()` with resolver)
- [x] T020 [P] [US1] Refactor graph options usage in app/Services/Directory/RoleDefinitionsSyncService.php (replace `$tenant->graphOptions()` with resolver)
- [x] T021 [P] [US1] Refactor options merge in app/Services/Graph/AssignmentFilterResolver.php (replace `array_merge($options, $tenant->graphOptions())` with provider-based options)
### Verification
- [x] T022 [US1] Run focused tests for this story: `vendor/bin/sail artisan test --compact --filter=MicrosoftGraphOptionsResolver`
- [x] T023 [US1] Run a repo scan to ensure no `app/` usage remains (confirm via tests/Feature/Guards/NoLegacyTenantGraphOptionsTest.php once added)
**Checkpoint**: `app/` contains zero tenant-based graph options usages, and runtime failures are provider-config actionable.
---
## Phase 4: User Story 2 — Prevent Reintroduction of Legacy Paths (Priority: P2)
**Goal**: CI fails if `$tenant->graphOptions()` or `Tenant::graphOptions()` appears anywhere under `app/`.
**Independent Test**: `vendor/bin/sail artisan test --compact --filter=NoLegacyTenantGraphOptions`.
### Tests (Guardrail)
- [x] T024 [P] [US2] Create guard test scanning `app/` in tests/Feature/Guards/NoLegacyTenantGraphOptionsTest.php
- [x] T025 [US2] Ensure guard failure message is actionable (points to ProviderConnectionResolver + ProviderGateway) in tests/Feature/Guards/NoLegacyTenantGraphOptionsTest.php
### Verification
- [x] T026 [US2] Run guard test: `vendor/bin/sail artisan test --compact --filter=NoLegacyTenantGraphOptions`
---
## Phase 5: Polish & Cross-Cutting Concerns
- [x] T027 [P] Run formatter: `vendor/bin/sail bin pint --dirty` (app/**, tests/**)
- [x] T028 Run full affected test folders: `vendor/bin/sail artisan test --compact tests/Feature/Guards`
- [x] T029 Run a broader suite if needed: `vendor/bin/sail artisan test --compact`
---
## Dependencies & Execution Order
### User Story Dependencies
- **US1 (P1)** depends on Phase 2 foundational tasks (resolver + kill-switch).
- **US2 (P2)** can be implemented after Phase 2, but should run after US1 refactor so the guard passes.
### Dependency Graph (high level)
- Phase 2 (T003T008)
-→ US1 refactors (T009T021)
-→ US1 verification (T022T023)
-→ US2 guardrail (T024T026)
-→ Polish (T027T029)
## Parallel Execution Examples
### After Phase 2 is complete
These refactors are parallelizable because they touch different files:
- T009T016, T018T021 can run in parallel ([P]).
### US2 guardrail
- T024 can be done in parallel with late US1 refactors, but it will only pass once all US1 call sites are removed.
## Implementation Strategy
### MVP (US1)
1. Complete Phase 2 (resolver + kill-switch + tests).
2. Refactor all `app/` call sites (T009T021).
3. Run focused tests (T022).
### Then lock it in (US2)
1. Add guardrail test (T024T026).
2. Finish with formatting + regression checks (T027T029).

View File

@ -16,6 +16,8 @@
beforeEach(function () {
$this->tenant = Tenant::factory()->create(['status' => 'active']);
ensureDefaultProviderConnection($this->tenant, 'microsoft');
$this->policy = Policy::factory()->create([
'tenant_id' => $this->tenant->id,
'external_id' => 'test-policy-123',

View File

@ -231,6 +231,8 @@ public function request(string $method, string $path, array $options = []): Grap
app()->instance(GraphClientInterface::class, $client);
$tenant = Tenant::factory()->create();
ensureDefaultProviderConnection($tenant);
$backupSet = BackupSet::factory()->for($tenant)->create([
'status' => 'completed',
'item_count' => 1,

View File

@ -78,6 +78,8 @@ public function request(string $method, string $path, array $options = []): Grap
'metadata' => [],
]);
ensureDefaultProviderConnection($tenant);
$tenant->makeCurrent();
$policyA = Policy::create([
@ -169,6 +171,8 @@ public function request(string $method, string $path, array $options = []): Grap
'metadata' => [],
]);
ensureDefaultProviderConnection($tenant);
$tenant->makeCurrent();
$policyA = Policy::create([

View File

@ -24,6 +24,7 @@
$tenant = Tenant::factory()->create();
$tenant->makeCurrent();
ensureDefaultProviderConnection($tenant, 'microsoft');
$policy = Policy::factory()->for($tenant)->create([
'external_id' => 'policy-123',
]);

View File

@ -50,6 +50,7 @@ public function request(string $method, string $path, array $options = []): Grap
'name' => 'Tenant One',
'metadata' => [],
]);
ensureDefaultProviderConnection($tenant, 'microsoft');
$policy = Policy::create([
'tenant_id' => $tenant->id,
@ -143,6 +144,7 @@ public function request(string $method, string $path, array $options = []): Grap
'name' => 'Tenant Preview',
'metadata' => [],
]);
ensureDefaultProviderConnection($tenant, 'microsoft');
$backupSet = BackupSet::create([
'tenant_id' => $tenant->id,
@ -214,6 +216,7 @@ public function request(string $method, string $path, array $options = []): Grap
'name' => 'Tenant Two',
'metadata' => [],
]);
ensureDefaultProviderConnection($tenant, 'microsoft');
$policy = Policy::create([
'tenant_id' => $tenant->id,

View File

@ -108,7 +108,7 @@
'status' => 'active',
]);
[$user, $tenant] = createUserWithTenant(tenant: $tenant, user: $user, role: 'owner');
[$user, $tenant] = createUserWithTenant(tenant: $tenant, user: $user, role: 'owner', ensureDefaultMicrosoftProviderConnection: false);
$this->actingAs($user);
Filament::setTenant($tenant, true);

View File

@ -21,7 +21,7 @@
it('starts tenant verification from header action and links to the canonical run viewer', function (): void {
Queue::fake();
[$user, $tenant] = createUserWithTenant(role: 'operator');
[$user, $tenant] = createUserWithTenant(role: 'operator', ensureDefaultMicrosoftProviderConnection: false);
$this->actingAs($user);
$tenant->makeCurrent();
@ -147,7 +147,7 @@
it('starts verification from the embedded widget CTA and uses canonical view-run links', function (): void {
Queue::fake();
[$user, $tenant] = createUserWithTenant(role: 'operator');
[$user, $tenant] = createUserWithTenant(role: 'operator', ensureDefaultMicrosoftProviderConnection: false);
$this->actingAs($user);
$connection = ProviderConnection::factory()->create([
@ -194,7 +194,7 @@
it('starts tenant verification from the tenant list row action via the unified run path', function (): void {
Queue::fake();
[$user, $tenant] = createUserWithTenant(role: 'operator');
[$user, $tenant] = createUserWithTenant(role: 'operator', ensureDefaultMicrosoftProviderConnection: false);
$this->actingAs($user);
Filament::setTenant($tenant, true);

View File

@ -11,6 +11,8 @@
beforeEach(function () {
$this->tenant = Tenant::factory()->create(['status' => 'active']);
ensureDefaultProviderConnection($this->tenant);
$this->policy = Policy::factory()->create([
'tenant_id' => $this->tenant->id,
'policy_type' => 'deviceConfiguration',

View File

@ -0,0 +1,108 @@
<?php
use Illuminate\Support\Collection;
it('Spec088 blocks legacy Tenant::graphOptions call sites in app/', function (): void {
$root = base_path();
$self = realpath(__FILE__);
$directories = [
$root.'/app',
];
$excludedPaths = [
$root.'/vendor',
$root.'/storage',
$root.'/specs',
$root.'/spechistory',
$root.'/references',
$root.'/bootstrap/cache',
];
$forbiddenPatterns = [
'/\$tenant->graphOptions\s*\(/',
// Avoid failing on the kill-switch error message string in app/Models/Tenant.php.
'/(?<![\'"`])Tenant::graphOptions\s*\(/',
];
/** @var Collection<int, string> $files */
$files = collect($directories)
->filter(fn (string $dir): bool => is_dir($dir))
->flatMap(function (string $dir): array {
$iterator = new RecursiveIteratorIterator(
new RecursiveDirectoryIterator($dir, FilesystemIterator::SKIP_DOTS)
);
$paths = [];
foreach ($iterator as $file) {
if (! $file->isFile()) {
continue;
}
$path = $file->getPathname();
if (! str_ends_with($path, '.php')) {
continue;
}
$paths[] = $path;
}
return $paths;
})
->filter(function (string $path) use ($excludedPaths, $self): bool {
if ($self && realpath($path) === $self) {
return false;
}
foreach ($excludedPaths as $excluded) {
if (str_starts_with($path, $excluded)) {
return false;
}
}
return true;
})
->values();
$hits = [];
foreach ($files as $path) {
$contents = file_get_contents($path);
if (! is_string($contents) || $contents === '') {
continue;
}
foreach ($forbiddenPatterns as $pattern) {
if (! preg_match($pattern, $contents)) {
continue;
}
$lines = preg_split('/\R/', $contents) ?: [];
foreach ($lines as $index => $line) {
if (preg_match($pattern, $line)) {
$relative = str_replace($root.'/', '', $path);
$hits[] = $relative.':'.($index + 1).' -> '.trim($line);
}
}
}
}
$howToFix = <<<'TXT'
Legacy tenant Graph options accessor usage detected.
Do not call `$tenant->graphOptions()` or `Tenant::graphOptions()` anywhere under `app/`.
Use provider connection resolution instead:
- `ProviderConnectionResolver::resolveDefault($tenant, 'microsoft')`
- `ProviderGateway::graphOptions($connection, $overrides)`
Or prefer the canonical helper: `MicrosoftGraphOptionsResolver::resolveForTenant($tenant, $overrides)`.
TXT;
expect($hits)->toBeEmpty($howToFix."\n\nMatches:\n".implode("\n", $hits));
});

View File

@ -0,0 +1,17 @@
<?php
use App\Models\Tenant;
use Illuminate\Foundation\Testing\RefreshDatabase;
uses(RefreshDatabase::class);
it('throws when Tenant::graphOptions is called', function (): void {
$tenant = Tenant::factory()->create([
'app_client_id' => 'legacy-client-id',
'app_client_secret' => 'legacy-client-secret',
]);
$call = fn (): array => $tenant->graphOptions();
expect($call)->toThrow(BadMethodCallException::class);
});

View File

@ -16,7 +16,7 @@
it('Spec081 shows blocked guidance with reason and manage-connections link on start surfaces', function (): void {
Queue::fake();
[$user, $tenant] = createUserWithTenant(role: 'operator');
[$user, $tenant] = createUserWithTenant(role: 'operator', ensureDefaultMicrosoftProviderConnection: false);
$this->actingAs($user);
$tenant->makeCurrent();
@ -60,7 +60,7 @@
});
it('Spec081 shows blocked guidance on tenant verify surface with manage-connections remediation link', function (): void {
[$user, $tenant] = createUserWithTenant(role: 'operator');
[$user, $tenant] = createUserWithTenant(role: 'operator', ensureDefaultMicrosoftProviderConnection: false);
$this->actingAs($user);
$tenant->makeCurrent();

View File

@ -0,0 +1,85 @@
<?php
use App\Models\ProviderConnection;
use App\Models\ProviderCredential;
use App\Models\Tenant;
use App\Services\Providers\MicrosoftGraphOptionsResolver;
use App\Services\Providers\ProviderConfigurationRequiredException;
use App\Support\Providers\ProviderReasonCodes;
use Illuminate\Foundation\Testing\RefreshDatabase;
uses(RefreshDatabase::class);
it('throws when no default provider connection exists', function (): void {
$tenant = Tenant::factory()->create();
$resolver = app(MicrosoftGraphOptionsResolver::class);
$call = fn (): array => $resolver->resolveForTenant($tenant);
expect($call)->toThrow(ProviderConfigurationRequiredException::class);
try {
$call();
} catch (ProviderConfigurationRequiredException $exception) {
expect($exception->reasonCode)->toBe(ProviderReasonCodes::ProviderConnectionMissing);
expect($exception->provider)->toBe('microsoft');
expect($exception->tenantId)->toBe((int) $tenant->getKey());
}
});
it('throws when default provider connection needs consent', function (): void {
$tenant = Tenant::factory()->create();
ProviderConnection::factory()->create([
'tenant_id' => $tenant->getKey(),
'workspace_id' => $tenant->workspace_id,
'provider' => 'microsoft',
'is_default' => true,
'status' => 'needs_consent',
]);
$resolver = app(MicrosoftGraphOptionsResolver::class);
$call = fn (): array => $resolver->resolveForTenant($tenant);
expect($call)->toThrow(ProviderConfigurationRequiredException::class);
try {
$call();
} catch (ProviderConfigurationRequiredException $exception) {
expect($exception->reasonCode)->toBe(ProviderReasonCodes::ProviderConsentMissing);
}
});
it('builds graph options from default provider connection credentials', function (): void {
$tenant = Tenant::factory()->create();
$connection = ProviderConnection::factory()->create([
'tenant_id' => $tenant->getKey(),
'workspace_id' => $tenant->workspace_id,
'provider' => 'microsoft',
'entra_tenant_id' => 'entra-tenant-id',
'is_default' => true,
'status' => 'connected',
]);
ProviderCredential::factory()->create([
'provider_connection_id' => $connection->getKey(),
'type' => 'client_secret',
'payload' => [
'client_id' => 'client-id',
'client_secret' => 'client-secret',
],
]);
$resolver = app(MicrosoftGraphOptionsResolver::class);
$options = $resolver->resolveForTenant($tenant, ['query' => ['a' => 'b']]);
expect($options['tenant'])->toBe('entra-tenant-id');
expect($options['client_id'])->toBe('client-id');
expect($options['client_secret'])->toBe('client-secret');
expect($options['client_request_id'])->toBeString()->not->toBeEmpty();
expect($options['query'])->toBe(['a' => 'b']);
});

View File

@ -1,5 +1,6 @@
<?php
use App\Filament\Resources\PolicyResource;
use App\Filament\Resources\PolicyResource\Pages\ListPolicies;
use App\Models\OperationRun;
use App\Models\Tenant;
@ -32,17 +33,15 @@
});
it('hides sync action for users who are not members of the tenant', function () {
// Create user without membership to the tenant
$user = User::factory()->create();
$tenant = Tenant::factory()->create();
// No membership created
$otherTenant = Tenant::factory()->create();
$this->actingAs($user);
$tenant->makeCurrent();
Filament::setTenant($tenant, true);
// Create user with a valid workspace context, but without membership to $tenant
[$user] = createUserWithTenant(tenant: $otherTenant, role: 'owner');
Livewire::test(ListPolicies::class)
->assertActionHidden('sync');
$this->actingAs($user)
->get(PolicyResource::getUrl('index', tenant: $tenant))
->assertNotFound();
Queue::assertNothingPushed();
});
@ -53,12 +52,9 @@
$tenantB = Tenant::factory()->create();
// User has no membership to tenantB
$this->actingAs($user);
$tenantB->makeCurrent();
Filament::setTenant($tenantB, true);
Livewire::test(ListPolicies::class)
->assertActionHidden('sync');
$this->actingAs($user)
->get(PolicyResource::getUrl('index', tenant: $tenantB))
->assertNotFound();
Queue::assertNothingPushed();
});
@ -70,20 +66,16 @@
});
it('blocks action execution for non-members (no side effects)', function () {
$user = User::factory()->create();
$tenant = Tenant::factory()->create();
$otherTenant = Tenant::factory()->create();
// Create user with a valid workspace context, but without membership to $tenant
[$user] = createUserWithTenant(tenant: $otherTenant, role: 'owner');
// No membership
$this->actingAs($user);
$tenant->makeCurrent();
Filament::setTenant($tenant, true);
// Hidden actions are treated as disabled by Filament
// The action call returns 200 but no execution occurs
Livewire::test(ListPolicies::class)
->mountAction('sync')
->callMountedAction()
->assertSuccessful();
$this->actingAs($user)
->get(PolicyResource::getUrl('index', tenant: $tenant))
->assertNotFound();
// Verify no side effects
Queue::assertNothingPushed();
@ -101,12 +93,12 @@
[$user, $tenant] = createUserWithTenant(role: 'owner');
$this->actingAs($user);
$tenant->makeCurrent();
Filament::setTenant($tenant, true);
// Start the test - action should be visible for member
$component = Livewire::test(ListPolicies::class)
$component = Livewire::actingAs($user)
->test(ListPolicies::class)
->assertActionVisible('sync')
->assertActionEnabled('sync');
@ -131,21 +123,22 @@
it('hides action in UI after membership revocation on re-render', function () {
[$user, $tenant] = createUserWithTenant(role: 'owner');
$this->actingAs($user);
$tenant->makeCurrent();
Filament::setTenant($tenant, true);
// Initial state - action visible
Livewire::test(ListPolicies::class)
Livewire::actingAs($user)
->test(ListPolicies::class)
->assertActionVisible('sync');
// Revoke membership
$user->tenants()->detach($tenant->getKey());
app(\App\Services\Auth\CapabilityResolver::class)->clearCache();
// New component instance (simulates page refresh)
Livewire::test(ListPolicies::class)
->assertActionHidden('sync');
// New request (simulates page refresh) should now be tenant-denied
$this->actingAs($user)
->get(PolicyResource::getUrl('index', tenant: $tenant))
->assertNotFound();
Queue::assertNothingPushed();
});

View File

@ -10,7 +10,7 @@
'app_client_id' => null,
]);
[$user, $tenant] = createUserWithTenant(tenant: $tenant, role: 'readonly');
[$user, $tenant] = createUserWithTenant(tenant: $tenant, role: 'readonly', ensureDefaultMicrosoftProviderConnection: false);
$this->actingAs($user)
->get("/admin/tenants/{$tenant->external_id}/required-permissions")

View File

@ -28,6 +28,7 @@
]);
$tenant->makeCurrent();
ensureDefaultProviderConnection($tenant, 'microsoft');
$policy = Policy::create([
'tenant_id' => $tenant->id,
@ -148,6 +149,7 @@
]);
$tenant->makeCurrent();
ensureDefaultProviderConnection($tenant, 'microsoft');
$policy = Policy::create([
'tenant_id' => $tenant->id,
@ -244,6 +246,7 @@
]);
$tenant->makeCurrent();
ensureDefaultProviderConnection($tenant, 'microsoft');
$policy = Policy::create([
'tenant_id' => $tenant->id,

View File

@ -30,6 +30,7 @@
]);
$tenant->makeCurrent();
ensureDefaultProviderConnection($tenant, 'microsoft');
$policy = Policy::create([
'tenant_id' => $tenant->id,
@ -103,6 +104,7 @@
]);
$tenant->makeCurrent();
ensureDefaultProviderConnection($tenant, 'microsoft');
$policy = Policy::create([
'tenant_id' => $tenant->id,

View File

@ -25,6 +25,7 @@
]);
$tenant->makeCurrent();
ensureDefaultProviderConnection($tenant, 'microsoft');
$backupSet = BackupSet::create([
'tenant_id' => $tenant->id,

View File

@ -13,7 +13,7 @@
it('dedupes verification starts while a run is active', function (): void {
Queue::fake();
[$user, $tenant] = createUserWithTenant(role: 'operator');
[$user, $tenant] = createUserWithTenant(role: 'operator', ensureDefaultMicrosoftProviderConnection: false);
$this->actingAs($user);
$tenant->makeCurrent();
@ -60,7 +60,7 @@
it('dedupes tenant-default verification starts while a run is active', function (): void {
Queue::fake();
[$user, $tenant] = createUserWithTenant(role: 'operator');
[$user, $tenant] = createUserWithTenant(role: 'operator', ensureDefaultMicrosoftProviderConnection: false);
$this->actingAs($user);
$tenant->makeCurrent();

View File

@ -12,6 +12,7 @@
it('persists metadata-only snapshot metadata on captured versions', function () {
$tenant = Tenant::factory()->create();
ensureDefaultProviderConnection($tenant, 'microsoft');
$policy = Policy::factory()->for($tenant)->create([
'policy_type' => 'mamAppConfiguration',
'platform' => 'mobile',

View File

@ -11,6 +11,7 @@
beforeEach(function () {
$this->tenant = Tenant::factory()->create();
ensureDefaultProviderConnection($this->tenant, 'microsoft');
$this->policy = Policy::factory()->create([
'tenant_id' => $this->tenant->id,
'external_id' => 'test-policy-id',

View File

@ -128,6 +128,7 @@ function createUserWithTenant(
?User $user = null,
string $role = 'owner',
?string $workspaceRole = null,
bool $ensureDefaultMicrosoftProviderConnection = true,
): array {
$user ??= User::factory()->create();
$tenant ??= Tenant::factory()->create();
@ -171,6 +172,10 @@ function createUserWithTenant(
$tenant->getKey() => ['role' => $role],
]);
if ($ensureDefaultMicrosoftProviderConnection) {
ensureDefaultProviderConnection($tenant, 'microsoft');
}
return [$user, $tenant];
}
@ -187,7 +192,7 @@ function ensureDefaultProviderConnection(Tenant $tenant, string $provider = 'mic
$connection = ProviderConnection::query()
->where('tenant_id', (int) $tenant->getKey())
->where('provider', $provider)
->where('is_default', true)
->orderByDesc('is_default')
->orderBy('id')
->first();
@ -200,6 +205,30 @@ function ensureDefaultProviderConnection(Tenant $tenant, string $provider = 'mic
'health_status' => 'ok',
'is_default' => true,
]);
} else {
$entraTenantId = trim((string) $connection->entra_tenant_id);
$updates = [];
if (! $connection->is_default) {
$updates['is_default'] = true;
}
if ($connection->status !== 'connected') {
$updates['status'] = 'connected';
}
if ($connection->health_status !== 'ok') {
$updates['health_status'] = 'ok';
}
if ($entraTenantId === '') {
$updates['entra_tenant_id'] = (string) ($tenant->tenant_id ?? fake()->uuid());
}
if ($updates !== []) {
$connection->forceFill($updates)->save();
$connection->refresh();
}
}
$credential = $connection->credential()->first();

View File

@ -17,6 +17,8 @@
'external_id' => 'tenant-123',
]);
ensureDefaultProviderConnection($tenant, 'microsoft');
$backupItem = BackupItem::factory()->create([
'tenant_id' => $tenant->id,
'metadata' => [],

View File

@ -3,6 +3,7 @@
use App\Services\Graph\AssignmentFilterResolver;
use App\Services\Graph\GraphResponse;
use App\Services\Graph\MicrosoftGraphClient;
use App\Services\Providers\MicrosoftGraphOptionsResolver;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Support\Facades\Cache;
@ -10,7 +11,7 @@
beforeEach(function () {
Cache::flush();
$this->graphClient = Mockery::mock(MicrosoftGraphClient::class);
$this->resolver = new AssignmentFilterResolver($this->graphClient);
$this->resolver = new AssignmentFilterResolver($this->graphClient, app(MicrosoftGraphOptionsResolver::class));
});
test('resolves assignment filters by id', function () {

View File

@ -9,6 +9,10 @@
use App\Services\Graph\GraphLogger;
use App\Services\Graph\GraphResponse;
use App\Services\Intune\AuditLogger;
use App\Services\Providers\MicrosoftGraphOptionsResolver;
use Illuminate\Foundation\Testing\RefreshDatabase;
uses(RefreshDatabase::class);
beforeEach(function () {
config()->set('graph_contracts.types.deviceManagementScript', [
@ -37,15 +41,18 @@
app(GraphLogger::class),
$this->auditLogger,
$this->filterResolver,
app(MicrosoftGraphOptionsResolver::class),
);
});
it('uses the contract assignment payload key for assign actions', function () {
$tenant = Tenant::factory()->make([
$tenant = Tenant::factory()->create([
'tenant_id' => 'tenant-123',
'app_client_id' => null,
'app_client_secret' => null,
]);
ensureDefaultProviderConnection($tenant, 'microsoft');
$policyId = 'policy-123';
$assignments = [
[
@ -92,11 +99,13 @@
});
it('uses derived assign endpoints for app protection policies', function () {
$tenant = Tenant::factory()->make([
$tenant = Tenant::factory()->create([
'tenant_id' => 'tenant-123',
'app_client_id' => null,
'app_client_secret' => null,
]);
ensureDefaultProviderConnection($tenant, 'microsoft');
$policyId = 'policy-123';
$assignments = [
[
@ -140,11 +149,13 @@
});
it('maps assignment filter ids stored at the root of assignments', function () {
$tenant = Tenant::factory()->make([
$tenant = Tenant::factory()->create([
'tenant_id' => 'tenant-123',
'app_client_id' => null,
'app_client_secret' => null,
]);
ensureDefaultProviderConnection($tenant, 'microsoft');
$policyId = 'policy-789';
$assignments = [
[
@ -200,11 +211,13 @@
});
it('keeps assignment filters when mapping is missing but filter exists in target', function () {
$tenant = Tenant::factory()->make([
$tenant = Tenant::factory()->create([
'tenant_id' => 'tenant-123',
'app_client_id' => null,
'app_client_secret' => null,
]);
ensureDefaultProviderConnection($tenant, 'microsoft');
$policyId = 'policy-999';
$assignments = [
[

View File

@ -11,6 +11,7 @@
use Mockery\MockInterface;
uses(RefreshDatabase::class);
class FoundationMappingGraphClient implements GraphClientInterface
{
public array $requests = [];
@ -59,6 +60,8 @@ public function request(string $method, string $path, array $options = []): Grap
it('maps existing foundations by display name', function () {
$tenant = Tenant::factory()->create();
ensureDefaultProviderConnection($tenant, 'microsoft');
$backupSet = BackupSet::factory()->for($tenant)->create();
$item = BackupItem::factory()
->for($tenant)
@ -121,6 +124,8 @@ public function request(string $method, string $path, array $options = []): Grap
'app_client_id' => 'client-1',
'app_client_secret' => 'secret-1',
]);
ensureDefaultProviderConnection($tenant, 'microsoft');
$backupSet = BackupSet::factory()->for($tenant)->create();
$item = BackupItem::factory()
->for($tenant)
@ -191,6 +196,8 @@ public function request(string $method, string $path, array $options = []): Grap
it('skips built-in scope tags', function () {
$tenant = Tenant::factory()->create();
ensureDefaultProviderConnection($tenant, 'microsoft');
$backupSet = BackupSet::factory()->for($tenant)->create();
$item = BackupItem::factory()
->for($tenant)

View File

@ -57,6 +57,8 @@ public function request(string $method, string $path, array $options = []): Grap
it('returns a failure when the foundation contract is missing', function () {
$tenant = Tenant::factory()->create();
ensureDefaultProviderConnection($tenant, 'microsoft');
$client = new FoundationSnapshotGraphClient([]);
app()->instance(GraphClientInterface::class, $client);
@ -80,6 +82,8 @@ public function request(string $method, string $path, array $options = []): Grap
'app_client_secret' => 'secret-123',
]);
ensureDefaultProviderConnection($tenant, 'microsoft');
$responses = [
new GraphResponse(
success: true,

View File

@ -10,6 +10,7 @@
use App\Services\Intune\AuditLogger;
use App\Services\Intune\PolicySnapshotService;
use App\Services\Intune\VersionService;
use App\Services\Providers\MicrosoftGraphOptionsResolver;
use Illuminate\Foundation\Testing\RefreshDatabase;
uses(RefreshDatabase::class);
@ -26,6 +27,7 @@
groupResolver: Mockery::mock(GroupResolver::class),
assignmentFilterResolver: Mockery::mock(AssignmentFilterResolver::class),
scopeTagResolver: Mockery::mock(ScopeTagResolver::class),
graphOptionsResolver: app(MicrosoftGraphOptionsResolver::class),
);
$fired = false;

View File

@ -9,6 +9,7 @@
use App\Services\Intune\PolicyCaptureOrchestrator;
use App\Services\Intune\PolicySnapshotService;
use App\Services\Intune\VersionService;
use App\Services\Providers\MicrosoftGraphOptionsResolver;
use Illuminate\Foundation\Testing\RefreshDatabase;
uses(RefreshDatabase::class);
@ -20,6 +21,8 @@
'is_current' => true,
]);
ensureDefaultProviderConnection($tenant, 'microsoft');
$policy = Policy::factory()->create([
'tenant_id' => $tenant->id,
'policy_type' => 'mamAppConfiguration',
@ -46,6 +49,7 @@
groupResolver: Mockery::mock(GroupResolver::class),
assignmentFilterResolver: Mockery::mock(AssignmentFilterResolver::class),
scopeTagResolver: Mockery::mock(ScopeTagResolver::class),
graphOptionsResolver: app(MicrosoftGraphOptionsResolver::class),
);
$result = $orchestrator->capture(

View File

@ -36,6 +36,8 @@ function requiredPermissions(): array
'name' => 'Tenant OK',
]);
ensureDefaultProviderConnection($tenant, 'microsoft');
foreach (requiredPermissions() as $permission) {
TenantPermission::create([
'tenant_id' => $tenant->id,
@ -67,6 +69,8 @@ function requiredPermissions(): array
'name' => 'Tenant Missing',
]);
ensureDefaultProviderConnection($tenant, 'microsoft');
$first = $permissions[0]['key'];
TenantPermission::create([
'tenant_id' => $tenant->id,
@ -101,6 +105,8 @@ function requiredPermissions(): array
'name' => 'Tenant Error',
]);
ensureDefaultProviderConnection($tenant, 'microsoft');
$permissions = requiredPermissions();
$first = $permissions[0]['key'];
@ -136,6 +142,8 @@ function requiredPermissions(): array
$tenant = Tenant::factory()->create();
ensureDefaultProviderConnection($tenant, 'microsoft');
TenantPermission::create([
'tenant_id' => $tenant->id,
'permission_key' => 'DeviceManagementRBAC.ReadWrite.All',