From 66b46955811e25d9c32b8de33f1877b10bd07476 Mon Sep 17 00:00:00 2001 From: Ahmed Darrazi Date: Thu, 15 Jan 2026 00:12:55 +0100 Subject: [PATCH] feat(044): drift findings UI + bulk acknowledge --- app/Filament/Pages/DriftLanding.php | 152 ++++++++ app/Filament/Resources/FindingResource.php | 329 +++++++++++++++++- .../FindingResource/Pages/ListFindings.php | 160 +++++++++ app/Jobs/GenerateDriftFindingsJob.php | 64 +++- app/Models/Finding.php | 2 + .../Drift/DriftFindingDiffBuilder.php | 304 ++++++++++++++++ app/Services/Drift/DriftFindingGenerator.php | 250 +++++++++++-- app/Services/Drift/DriftHasher.php | 68 ++++ .../Normalizers/AssignmentsNormalizer.php | 113 ++++++ .../Drift/Normalizers/ScopeTagsNormalizer.php | 136 ++++++++ .../Drift/Normalizers/SettingsNormalizer.php | 19 + config/intune_permissions.php | 6 +- .../entries/assignments-diff.blade.php | 114 ++++++ .../entries/scope-tags-diff.blade.php | 111 ++++++ .../filament/pages/drift-landing.blade.php | 92 +++++ .../044-drift-mvp/checklists/requirements.md | 26 +- .../contracts/admin-findings.openapi.yaml | 21 ++ specs/044-drift-mvp/plan.md | 2 + specs/044-drift-mvp/quickstart.md | 8 +- specs/044-drift-mvp/spec.md | 185 +++------- specs/044-drift-mvp/tasks.md | 113 +++--- .../DriftAcknowledgeAuthorizationTest.php | 31 ++ tests/Feature/Drift/DriftAcknowledgeTest.php | 28 ++ .../DriftAssignmentDriftDetectionTest.php | 2 - ...AcknowledgeAllMatchingConfirmationTest.php | 34 ++ .../DriftBulkAcknowledgeAllMatchingTest.php | 51 +++ .../DriftBulkAcknowledgeAuthorizationTest.php | 60 ++++ .../Drift/DriftBulkAcknowledgeTest.php | 34 ++ .../DriftCompletedRunWithZeroFindingsTest.php | 71 ++++ .../Drift/DriftEvidenceMinimizationTest.php | 24 ++ ...tFindingDetailShowsAssignmentsDiffTest.php | 151 ++++++++ ...iftFindingDetailShowsScopeTagsDiffTest.php | 100 ++++++ ...riftFindingDetailShowsSettingsDiffTest.php | 100 ++++++ .../Feature/Drift/DriftFindingDetailTest.php | 48 +++ .../Drift/DriftGenerationDeterminismTest.php | 76 ++++ .../Drift/DriftGenerationDispatchTest.php | 73 +++- tests/Feature/Drift/DriftHasherTest.php | 39 +++ .../DriftLandingShowsComparisonInfoTest.php | 33 ++ .../DriftPolicySnapshotDriftDetectionTest.php | 73 ++++ ...otMetadataOnlyDoesNotCreateFindingTest.php | 62 ++++ .../Drift/DriftScopeTagDriftDetectionTest.php | 84 +++++ ...gLegacyDefaultDoesNotCreateFindingTest.php | 69 ++++ 42 files changed, 3276 insertions(+), 242 deletions(-) create mode 100644 app/Services/Drift/DriftFindingDiffBuilder.php create mode 100644 app/Services/Drift/Normalizers/AssignmentsNormalizer.php create mode 100644 app/Services/Drift/Normalizers/ScopeTagsNormalizer.php create mode 100644 app/Services/Drift/Normalizers/SettingsNormalizer.php create mode 100644 resources/views/filament/infolists/entries/assignments-diff.blade.php create mode 100644 resources/views/filament/infolists/entries/scope-tags-diff.blade.php create mode 100644 tests/Feature/Drift/DriftAcknowledgeAuthorizationTest.php create mode 100644 tests/Feature/Drift/DriftAcknowledgeTest.php create mode 100644 tests/Feature/Drift/DriftBulkAcknowledgeAllMatchingConfirmationTest.php create mode 100644 tests/Feature/Drift/DriftBulkAcknowledgeAllMatchingTest.php create mode 100644 tests/Feature/Drift/DriftBulkAcknowledgeAuthorizationTest.php create mode 100644 tests/Feature/Drift/DriftBulkAcknowledgeTest.php create mode 100644 tests/Feature/Drift/DriftCompletedRunWithZeroFindingsTest.php create mode 100644 tests/Feature/Drift/DriftEvidenceMinimizationTest.php create mode 100644 tests/Feature/Drift/DriftFindingDetailShowsAssignmentsDiffTest.php create mode 100644 tests/Feature/Drift/DriftFindingDetailShowsScopeTagsDiffTest.php create mode 100644 tests/Feature/Drift/DriftFindingDetailShowsSettingsDiffTest.php create mode 100644 tests/Feature/Drift/DriftFindingDetailTest.php create mode 100644 tests/Feature/Drift/DriftGenerationDeterminismTest.php create mode 100644 tests/Feature/Drift/DriftHasherTest.php create mode 100644 tests/Feature/Drift/DriftLandingShowsComparisonInfoTest.php create mode 100644 tests/Feature/Drift/DriftPolicySnapshotDriftDetectionTest.php create mode 100644 tests/Feature/Drift/DriftPolicySnapshotMetadataOnlyDoesNotCreateFindingTest.php create mode 100644 tests/Feature/Drift/DriftScopeTagDriftDetectionTest.php create mode 100644 tests/Feature/Drift/DriftScopeTagLegacyDefaultDoesNotCreateFindingTest.php diff --git a/app/Filament/Pages/DriftLanding.php b/app/Filament/Pages/DriftLanding.php index cfdcb44..3b7c1d6 100644 --- a/app/Filament/Pages/DriftLanding.php +++ b/app/Filament/Pages/DriftLanding.php @@ -2,13 +2,18 @@ namespace App\Filament\Pages; +use App\Filament\Resources\BulkOperationRunResource; use App\Filament\Resources\FindingResource; +use App\Filament\Resources\InventorySyncRunResource; use App\Jobs\GenerateDriftFindingsJob; +use App\Models\BulkOperationRun; use App\Models\Finding; use App\Models\InventorySyncRun; use App\Models\Tenant; use App\Models\User; +use App\Services\BulkOperationService; use App\Services\Drift\DriftRunSelector; +use App\Support\RunIdempotency; use BackedEnum; use Filament\Pages\Page; use UnitEnum; @@ -23,6 +28,30 @@ class DriftLanding extends Page protected string $view = 'filament.pages.drift-landing'; + public ?string $state = null; + + public ?string $message = null; + + public ?string $scopeKey = null; + + public ?int $baselineRunId = null; + + public ?int $currentRunId = null; + + public ?string $baselineFinishedAt = null; + + public ?string $currentFinishedAt = null; + + public ?int $bulkOperationRunId = null; + + /** @var array|null */ + public ?array $statusCounts = null; + + public static function canAccess(): bool + { + return FindingResource::canAccess(); + } + public function mount(): void { $tenant = Tenant::current(); @@ -40,21 +69,45 @@ public function mount(): void ->first(); if (! $latestSuccessful instanceof InventorySyncRun) { + $this->state = 'blocked'; + $this->message = 'No successful inventory runs found yet.'; + return; } $scopeKey = (string) $latestSuccessful->selection_hash; + $this->scopeKey = $scopeKey; $selector = app(DriftRunSelector::class); $comparison = $selector->selectBaselineAndCurrent($tenant, $scopeKey); if ($comparison === null) { + $this->state = 'blocked'; + $this->message = 'Need at least 2 successful runs for this scope to calculate drift.'; + return; } $baseline = $comparison['baseline']; $current = $comparison['current']; + $this->baselineRunId = (int) $baseline->getKey(); + $this->currentRunId = (int) $current->getKey(); + + $this->baselineFinishedAt = $baseline->finished_at?->toDateTimeString(); + $this->currentFinishedAt = $current->finished_at?->toDateTimeString(); + + $idempotencyKey = RunIdempotency::buildKey( + tenantId: (int) $tenant->getKey(), + operationType: 'drift.generate', + targetId: $scopeKey, + context: [ + 'scope_key' => $scopeKey, + 'baseline_run_id' => (int) $baseline->getKey(), + 'current_run_id' => (int) $current->getKey(), + ], + ); + $exists = Finding::query() ->where('tenant_id', $tenant->getKey()) ->where('finding_type', Finding::FINDING_TYPE_DRIFT) @@ -64,15 +117,87 @@ public function mount(): void ->exists(); if ($exists) { + $this->state = 'ready'; + $newCount = (int) Finding::query() + ->where('tenant_id', $tenant->getKey()) + ->where('finding_type', Finding::FINDING_TYPE_DRIFT) + ->where('scope_key', $scopeKey) + ->where('baseline_run_id', $baseline->getKey()) + ->where('current_run_id', $current->getKey()) + ->where('status', Finding::STATUS_NEW) + ->count(); + + $this->statusCounts = [Finding::STATUS_NEW => $newCount]; + return; } + $latestRun = BulkOperationRun::query() + ->where('tenant_id', $tenant->getKey()) + ->where('idempotency_key', $idempotencyKey) + ->latest('id') + ->first(); + + $activeRun = RunIdempotency::findActiveBulkOperationRun((int) $tenant->getKey(), $idempotencyKey); + if ($activeRun instanceof BulkOperationRun) { + $this->state = 'generating'; + $this->bulkOperationRunId = (int) $activeRun->getKey(); + + return; + } + + if ($latestRun instanceof BulkOperationRun && $latestRun->status === 'completed') { + $this->state = 'ready'; + $this->bulkOperationRunId = (int) $latestRun->getKey(); + + $newCount = (int) Finding::query() + ->where('tenant_id', $tenant->getKey()) + ->where('finding_type', Finding::FINDING_TYPE_DRIFT) + ->where('scope_key', $scopeKey) + ->where('baseline_run_id', $baseline->getKey()) + ->where('current_run_id', $current->getKey()) + ->where('status', Finding::STATUS_NEW) + ->count(); + + $this->statusCounts = [Finding::STATUS_NEW => $newCount]; + + if ($newCount === 0) { + $this->message = 'No drift findings for this comparison. If you changed settings after the current run, run Inventory Sync again to capture a newer snapshot.'; + } + + return; + } + + if ($latestRun instanceof BulkOperationRun && in_array($latestRun->status, ['failed', 'aborted'], true)) { + $this->state = 'error'; + $this->message = 'Drift generation failed for this comparison. See the run for details.'; + $this->bulkOperationRunId = (int) $latestRun->getKey(); + + return; + } + + $bulkOperationService = app(BulkOperationService::class); + $run = $bulkOperationService->createRun( + tenant: $tenant, + user: $user, + resource: 'drift', + action: 'generate', + itemIds: [$scopeKey], + totalItems: 1, + ); + + $run->update(['idempotency_key' => $idempotencyKey]); + + $this->state = 'generating'; + $this->bulkOperationRunId = (int) $run->getKey(); + GenerateDriftFindingsJob::dispatch( tenantId: (int) $tenant->getKey(), userId: (int) $user->getKey(), baselineRunId: (int) $baseline->getKey(), currentRunId: (int) $current->getKey(), scopeKey: $scopeKey, + bulkOperationRunId: (int) $run->getKey(), ); } @@ -80,4 +205,31 @@ public function getFindingsUrl(): string { return FindingResource::getUrl('index', tenant: Tenant::current()); } + + public function getBaselineRunUrl(): ?string + { + if (! is_int($this->baselineRunId)) { + return null; + } + + return InventorySyncRunResource::getUrl('view', ['record' => $this->baselineRunId], tenant: Tenant::current()); + } + + public function getCurrentRunUrl(): ?string + { + if (! is_int($this->currentRunId)) { + return null; + } + + return InventorySyncRunResource::getUrl('view', ['record' => $this->currentRunId], tenant: Tenant::current()); + } + + public function getBulkRunUrl(): ?string + { + if (! is_int($this->bulkOperationRunId)) { + return null; + } + + return BulkOperationRunResource::getUrl('view', ['record' => $this->bulkOperationRunId], tenant: Tenant::current()); + } } diff --git a/app/Filament/Resources/FindingResource.php b/app/Filament/Resources/FindingResource.php index bfd097e..a78c03a 100644 --- a/app/Filament/Resources/FindingResource.php +++ b/app/Filament/Resources/FindingResource.php @@ -4,14 +4,28 @@ use App\Filament\Resources\FindingResource\Pages; use App\Models\Finding; +use App\Models\InventoryItem; +use App\Models\PolicyVersion; use App\Models\Tenant; +use App\Models\User; +use App\Services\Drift\DriftFindingDiffBuilder; use BackedEnum; use Filament\Actions; +use Filament\Actions\BulkAction; +use Filament\Actions\BulkActionGroup; +use Filament\Forms\Components\TextInput; +use Filament\Infolists\Components\TextEntry; +use Filament\Infolists\Components\ViewEntry; +use Filament\Notifications\Notification; use Filament\Resources\Resource; +use Filament\Schemas\Components\Section; use Filament\Schemas\Schema; use Filament\Tables; use Filament\Tables\Table; use Illuminate\Database\Eloquent\Builder; +use Illuminate\Support\Arr; +use Illuminate\Support\Collection; +use Illuminate\Support\Facades\Gate; use UnitEnum; class FindingResource extends Resource @@ -29,22 +43,326 @@ public static function form(Schema $schema): Schema return $schema; } + public static function infolist(Schema $schema): Schema + { + return $schema + ->schema([ + Section::make('Finding') + ->schema([ + TextEntry::make('finding_type')->badge()->label('Type'), + TextEntry::make('status')->badge(), + TextEntry::make('severity')->badge(), + TextEntry::make('fingerprint')->label('Fingerprint')->copyable(), + TextEntry::make('scope_key')->label('Scope')->copyable(), + TextEntry::make('subject_display_name')->label('Subject')->placeholder('—'), + TextEntry::make('subject_type')->label('Subject type'), + TextEntry::make('subject_external_id')->label('External ID')->copyable(), + TextEntry::make('baseline_run_id') + ->label('Baseline run') + ->url(fn (Finding $record): ?string => $record->baseline_run_id + ? InventorySyncRunResource::getUrl('view', ['record' => $record->baseline_run_id], tenant: Tenant::current()) + : null) + ->openUrlInNewTab(), + TextEntry::make('current_run_id') + ->label('Current run') + ->url(fn (Finding $record): ?string => $record->current_run_id + ? InventorySyncRunResource::getUrl('view', ['record' => $record->current_run_id], tenant: Tenant::current()) + : null) + ->openUrlInNewTab(), + TextEntry::make('acknowledged_at')->dateTime()->placeholder('—'), + TextEntry::make('acknowledged_by_user_id')->label('Acknowledged by')->placeholder('—'), + TextEntry::make('created_at')->label('Created')->dateTime(), + ]) + ->columns(2) + ->columnSpanFull(), + + Section::make('Diff') + ->schema([ + ViewEntry::make('settings_diff') + ->label('') + ->view('filament.infolists.entries.normalized-diff') + ->state(function (Finding $record): array { + $tenant = Tenant::current(); + if (! $tenant) { + return ['summary' => ['message' => 'No tenant context'], 'added' => [], 'removed' => [], 'changed' => []]; + } + + $baselineId = Arr::get($record->evidence_jsonb ?? [], 'baseline.policy_version_id'); + $currentId = Arr::get($record->evidence_jsonb ?? [], 'current.policy_version_id'); + + $baselineVersion = is_numeric($baselineId) + ? PolicyVersion::query()->where('tenant_id', $tenant->getKey())->find((int) $baselineId) + : null; + + $currentVersion = is_numeric($currentId) + ? PolicyVersion::query()->where('tenant_id', $tenant->getKey())->find((int) $currentId) + : null; + + $diff = app(DriftFindingDiffBuilder::class)->buildSettingsDiff($baselineVersion, $currentVersion); + + $addedCount = (int) Arr::get($diff, 'summary.added', 0); + $removedCount = (int) Arr::get($diff, 'summary.removed', 0); + $changedCount = (int) Arr::get($diff, 'summary.changed', 0); + + if (($addedCount + $removedCount + $changedCount) === 0) { + Arr::set( + $diff, + 'summary.message', + 'No normalized changes were found. This drift finding may be based on fields that are intentionally excluded from normalization.' + ); + } + + return $diff; + }) + ->visible(fn (Finding $record): bool => Arr::get($record->evidence_jsonb ?? [], 'summary.kind') === 'policy_snapshot') + ->columnSpanFull(), + + ViewEntry::make('scope_tags_diff') + ->label('') + ->view('filament.infolists.entries.scope-tags-diff') + ->state(function (Finding $record): array { + $tenant = Tenant::current(); + if (! $tenant) { + return ['summary' => ['message' => 'No tenant context'], 'added' => [], 'removed' => [], 'changed' => []]; + } + + $baselineId = Arr::get($record->evidence_jsonb ?? [], 'baseline.policy_version_id'); + $currentId = Arr::get($record->evidence_jsonb ?? [], 'current.policy_version_id'); + + $baselineVersion = is_numeric($baselineId) + ? PolicyVersion::query()->where('tenant_id', $tenant->getKey())->find((int) $baselineId) + : null; + + $currentVersion = is_numeric($currentId) + ? PolicyVersion::query()->where('tenant_id', $tenant->getKey())->find((int) $currentId) + : null; + + return app(DriftFindingDiffBuilder::class)->buildScopeTagsDiff($baselineVersion, $currentVersion); + }) + ->visible(fn (Finding $record): bool => Arr::get($record->evidence_jsonb ?? [], 'summary.kind') === 'policy_scope_tags') + ->columnSpanFull(), + + ViewEntry::make('assignments_diff') + ->label('') + ->view('filament.infolists.entries.assignments-diff') + ->state(function (Finding $record): array { + $tenant = Tenant::current(); + if (! $tenant) { + return ['summary' => ['message' => 'No tenant context'], 'added' => [], 'removed' => [], 'changed' => []]; + } + + $baselineId = Arr::get($record->evidence_jsonb ?? [], 'baseline.policy_version_id'); + $currentId = Arr::get($record->evidence_jsonb ?? [], 'current.policy_version_id'); + + $baselineVersion = is_numeric($baselineId) + ? PolicyVersion::query()->where('tenant_id', $tenant->getKey())->find((int) $baselineId) + : null; + + $currentVersion = is_numeric($currentId) + ? PolicyVersion::query()->where('tenant_id', $tenant->getKey())->find((int) $currentId) + : null; + + return app(DriftFindingDiffBuilder::class)->buildAssignmentsDiff($tenant, $baselineVersion, $currentVersion); + }) + ->visible(fn (Finding $record): bool => Arr::get($record->evidence_jsonb ?? [], 'summary.kind') === 'policy_assignments') + ->columnSpanFull(), + ]) + ->collapsed() + ->columnSpanFull(), + + Section::make('Evidence (Sanitized)') + ->schema([ + ViewEntry::make('evidence_jsonb') + ->label('') + ->view('filament.infolists.entries.snapshot-json') + ->state(fn (Finding $record) => $record->evidence_jsonb ?? []) + ->columnSpanFull(), + ]) + ->columnSpanFull(), + ]); + } + public static function table(Table $table): Table { return $table + ->defaultSort('created_at', 'desc') ->columns([ Tables\Columns\TextColumn::make('finding_type')->badge()->label('Type'), Tables\Columns\TextColumn::make('status')->badge(), Tables\Columns\TextColumn::make('severity')->badge(), - Tables\Columns\TextColumn::make('subject_type')->label('Subject')->searchable(), + Tables\Columns\TextColumn::make('subject_display_name')->label('Subject')->placeholder('—'), + Tables\Columns\TextColumn::make('subject_type')->label('Subject type')->searchable(), Tables\Columns\TextColumn::make('subject_external_id')->label('External ID')->toggleable(isToggledHiddenByDefault: true), Tables\Columns\TextColumn::make('scope_key')->label('Scope')->toggleable(isToggledHiddenByDefault: true), Tables\Columns\TextColumn::make('created_at')->since()->label('Created'), ]) + ->filters([ + Tables\Filters\SelectFilter::make('status') + ->options([ + Finding::STATUS_NEW => 'New', + Finding::STATUS_ACKNOWLEDGED => 'Acknowledged', + ]) + ->default(Finding::STATUS_NEW), + Tables\Filters\SelectFilter::make('finding_type') + ->options([ + Finding::FINDING_TYPE_DRIFT => 'Drift', + ]) + ->default(Finding::FINDING_TYPE_DRIFT), + Tables\Filters\Filter::make('scope_key') + ->form([ + TextInput::make('scope_key') + ->label('Scope key') + ->placeholder('Inventory selection hash') + ->maxLength(255), + ]) + ->query(function (Builder $query, array $data): Builder { + $scopeKey = $data['scope_key'] ?? null; + + if (! is_string($scopeKey) || $scopeKey === '') { + return $query; + } + + return $query->where('scope_key', $scopeKey); + }), + Tables\Filters\Filter::make('run_ids') + ->label('Run IDs') + ->form([ + TextInput::make('baseline_run_id') + ->label('Baseline run id') + ->numeric(), + TextInput::make('current_run_id') + ->label('Current run id') + ->numeric(), + ]) + ->query(function (Builder $query, array $data): Builder { + $baselineRunId = $data['baseline_run_id'] ?? null; + if (is_numeric($baselineRunId)) { + $query->where('baseline_run_id', (int) $baselineRunId); + } + + $currentRunId = $data['current_run_id'] ?? null; + if (is_numeric($currentRunId)) { + $query->where('current_run_id', (int) $currentRunId); + } + + return $query; + }), + ]) ->actions([ + Actions\Action::make('acknowledge') + ->label('Acknowledge') + ->icon('heroicon-o-check') + ->color('gray') + ->requiresConfirmation() + ->visible(fn (Finding $record): bool => $record->status === Finding::STATUS_NEW) + ->authorize(function (Finding $record): bool { + $user = auth()->user(); + + if (! $user instanceof User) { + return false; + } + + return $user->can('update', $record); + }) + ->action(function (Finding $record): void { + $tenant = Tenant::current(); + $user = auth()->user(); + + if (! $tenant || ! $user instanceof User) { + return; + } + + if ((int) $record->tenant_id !== (int) $tenant->getKey()) { + Notification::make() + ->title('Finding belongs to a different tenant') + ->danger() + ->send(); + + return; + } + + $record->acknowledge($user); + + Notification::make() + ->title('Finding acknowledged') + ->success() + ->send(); + }), Actions\ViewAction::make(), ]) - ->bulkActions([]); + ->bulkActions([ + BulkActionGroup::make([ + BulkAction::make('acknowledge_selected') + ->label('Acknowledge selected') + ->icon('heroicon-o-check') + ->color('gray') + ->authorize(function (): bool { + $tenant = Tenant::current(); + $user = auth()->user(); + + if (! $tenant || ! $user instanceof User) { + return false; + } + + $probe = new Finding(['tenant_id' => $tenant->getKey()]); + + return $user->can('update', $probe); + }) + ->authorizeIndividualRecords('update') + ->requiresConfirmation() + ->action(function (Collection $records): void { + $tenant = Tenant::current(); + $user = auth()->user(); + + if (! $tenant || ! $user instanceof User) { + return; + } + + $firstRecord = $records->first(); + if ($firstRecord instanceof Finding) { + Gate::authorize('update', $firstRecord); + } + + $acknowledgedCount = 0; + $skippedCount = 0; + + foreach ($records as $record) { + if (! $record instanceof Finding) { + $skippedCount++; + + continue; + } + + if ((int) $record->tenant_id !== (int) $tenant->getKey()) { + $skippedCount++; + + continue; + } + + if ($record->status !== Finding::STATUS_NEW) { + $skippedCount++; + + continue; + } + + $record->acknowledge($user); + $acknowledgedCount++; + } + + $body = "Acknowledged {$acknowledgedCount} finding".($acknowledgedCount === 1 ? '' : 's').'.'; + if ($skippedCount > 0) { + $body .= " Skipped {$skippedCount}."; + } + + Notification::make() + ->title('Bulk acknowledge completed') + ->body($body) + ->success() + ->send(); + }) + ->deselectRecordsAfterCompletion(), + ]), + ]); } public static function getEloquentQuery(): Builder @@ -52,6 +370,13 @@ public static function getEloquentQuery(): Builder $tenantId = Tenant::current()->getKey(); return parent::getEloquentQuery() + ->addSelect([ + 'subject_display_name' => InventoryItem::query() + ->select('display_name') + ->whereColumn('inventory_items.tenant_id', 'findings.tenant_id') + ->whereColumn('inventory_items.external_id', 'findings.subject_external_id') + ->limit(1), + ]) ->when($tenantId, fn (Builder $query) => $query->where('tenant_id', $tenantId)); } diff --git a/app/Filament/Resources/FindingResource/Pages/ListFindings.php b/app/Filament/Resources/FindingResource/Pages/ListFindings.php index cb872dd..0322a04 100644 --- a/app/Filament/Resources/FindingResource/Pages/ListFindings.php +++ b/app/Filament/Resources/FindingResource/Pages/ListFindings.php @@ -3,9 +3,169 @@ namespace App\Filament\Resources\FindingResource\Pages; use App\Filament\Resources\FindingResource; +use App\Models\Finding; +use App\Models\Tenant; +use App\Models\User; +use Filament\Actions; +use Filament\Forms\Components\TextInput; +use Filament\Notifications\Notification; use Filament\Resources\Pages\ListRecords; +use Illuminate\Database\Eloquent\Builder; +use Illuminate\Support\Arr; +use Illuminate\Support\Facades\Gate; class ListFindings extends ListRecords { protected static string $resource = FindingResource::class; + + protected function getHeaderActions(): array + { + return [ + Actions\Action::make('acknowledge_all_matching') + ->label('Acknowledge all matching') + ->icon('heroicon-o-check') + ->color('gray') + ->requiresConfirmation() + ->authorize(function (): bool { + $tenant = Tenant::current(); + $user = auth()->user(); + + if (! $tenant || ! $user instanceof User) { + return false; + } + + $probe = new Finding(['tenant_id' => $tenant->getKey()]); + + return $user->can('update', $probe); + }) + ->visible(fn (): bool => $this->getStatusFilterValue() === Finding::STATUS_NEW) + ->modalDescription(function (): string { + $count = $this->getAllMatchingCount(); + + return "You are about to acknowledge {$count} finding".($count === 1 ? '' : 's').' matching the current filters.'; + }) + ->form(function (): array { + $count = $this->getAllMatchingCount(); + + if ($count <= 100) { + return []; + } + + return [ + TextInput::make('confirmation') + ->label('Type ACKNOWLEDGE to confirm') + ->required() + ->in(['ACKNOWLEDGE']) + ->validationMessages([ + 'in' => 'Please type ACKNOWLEDGE to confirm.', + ]), + ]; + }) + ->action(function (array $data): void { + $tenant = Tenant::current(); + $user = auth()->user(); + + if (! $tenant || ! $user instanceof User) { + return; + } + + $query = $this->buildAllMatchingQuery(); + $count = (clone $query)->count(); + + if ($count === 0) { + Notification::make() + ->title('No matching findings') + ->body('There are no new findings matching the current filters.') + ->warning() + ->send(); + + return; + } + + $firstRecord = (clone $query)->first(); + if ($firstRecord instanceof Finding) { + Gate::authorize('update', $firstRecord); + } + + $updated = $query->update([ + 'status' => Finding::STATUS_ACKNOWLEDGED, + 'acknowledged_at' => now(), + 'acknowledged_by_user_id' => $user->getKey(), + ]); + + $this->deselectAllTableRecords(); + $this->resetPage(); + + Notification::make() + ->title('Bulk acknowledge completed') + ->body("Acknowledged {$updated} finding".($updated === 1 ? '' : 's').'.') + ->success() + ->send(); + }), + ]; + } + + protected function buildAllMatchingQuery(): Builder + { + $tenant = Tenant::current(); + + $query = Finding::query(); + + if (! $tenant) { + return $query->whereRaw('1 = 0'); + } + + $query->where('tenant_id', $tenant->getKey()); + + $query->where('status', Finding::STATUS_NEW); + + $findingType = $this->getFindingTypeFilterValue(); + if (is_string($findingType) && $findingType !== '') { + $query->where('finding_type', $findingType); + } + + $scopeKeyState = $this->getTableFilterState('scope_key') ?? []; + $scopeKey = Arr::get($scopeKeyState, 'scope_key'); + if (is_string($scopeKey) && $scopeKey !== '') { + $query->where('scope_key', $scopeKey); + } + + $runIdsState = $this->getTableFilterState('run_ids') ?? []; + $baselineRunId = Arr::get($runIdsState, 'baseline_run_id'); + if (is_numeric($baselineRunId)) { + $query->where('baseline_run_id', (int) $baselineRunId); + } + + $currentRunId = Arr::get($runIdsState, 'current_run_id'); + if (is_numeric($currentRunId)) { + $query->where('current_run_id', (int) $currentRunId); + } + + return $query; + } + + protected function getAllMatchingCount(): int + { + return (int) $this->buildAllMatchingQuery()->count(); + } + + protected function getStatusFilterValue(): string + { + $state = $this->getTableFilterState('status') ?? []; + $value = Arr::get($state, 'value'); + + return is_string($value) && $value !== '' + ? $value + : Finding::STATUS_NEW; + } + + protected function getFindingTypeFilterValue(): string + { + $state = $this->getTableFilterState('finding_type') ?? []; + $value = Arr::get($state, 'value'); + + return is_string($value) && $value !== '' + ? $value + : Finding::FINDING_TYPE_DRIFT; + } } diff --git a/app/Jobs/GenerateDriftFindingsJob.php b/app/Jobs/GenerateDriftFindingsJob.php index 645dae4..353cb77 100644 --- a/app/Jobs/GenerateDriftFindingsJob.php +++ b/app/Jobs/GenerateDriftFindingsJob.php @@ -2,15 +2,19 @@ namespace App\Jobs; +use App\Models\BulkOperationRun; use App\Models\InventorySyncRun; use App\Models\Tenant; +use App\Services\BulkOperationService; use App\Services\Drift\DriftFindingGenerator; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; +use Illuminate\Support\Facades\Log; use RuntimeException; +use Throwable; class GenerateDriftFindingsJob implements ShouldQueue { @@ -22,13 +26,22 @@ public function __construct( public int $baselineRunId, public int $currentRunId, public string $scopeKey, + public int $bulkOperationRunId, ) {} /** * Execute the job. */ - public function handle(DriftFindingGenerator $generator): void + public function handle(DriftFindingGenerator $generator, BulkOperationService $bulkOperationService): void { + Log::info('GenerateDriftFindingsJob: started', [ + 'tenant_id' => $this->tenantId, + 'baseline_run_id' => $this->baselineRunId, + 'current_run_id' => $this->currentRunId, + 'scope_key' => $this->scopeKey, + 'bulk_operation_run_id' => $this->bulkOperationRunId, + ]); + $tenant = Tenant::query()->find($this->tenantId); if (! $tenant instanceof Tenant) { throw new RuntimeException('Tenant not found.'); @@ -44,11 +57,48 @@ public function handle(DriftFindingGenerator $generator): void throw new RuntimeException('Current run not found.'); } - $generator->generate( - tenant: $tenant, - baseline: $baseline, - current: $current, - scopeKey: $this->scopeKey, - ); + $run = BulkOperationRun::query() + ->where('tenant_id', $tenant->getKey()) + ->find($this->bulkOperationRunId); + + if (! $run instanceof BulkOperationRun) { + throw new RuntimeException('Bulk operation run not found.'); + } + + $bulkOperationService->start($run); + + try { + $created = $generator->generate( + tenant: $tenant, + baseline: $baseline, + current: $current, + scopeKey: $this->scopeKey, + ); + + Log::info('GenerateDriftFindingsJob: completed', [ + 'tenant_id' => $this->tenantId, + 'baseline_run_id' => $this->baselineRunId, + 'current_run_id' => $this->currentRunId, + 'scope_key' => $this->scopeKey, + 'bulk_operation_run_id' => $this->bulkOperationRunId, + 'created_findings_count' => $created, + ]); + + $bulkOperationService->recordSuccess($run); + $bulkOperationService->complete($run); + } catch (Throwable $e) { + Log::error('GenerateDriftFindingsJob: failed', [ + 'tenant_id' => $this->tenantId, + 'baseline_run_id' => $this->baselineRunId, + 'current_run_id' => $this->currentRunId, + 'scope_key' => $this->scopeKey, + 'bulk_operation_run_id' => $this->bulkOperationRunId, + 'error' => $e->getMessage(), + ]); + + $bulkOperationService->fail($run, $e->getMessage()); + + throw $e; + } } } diff --git a/app/Models/Finding.php b/app/Models/Finding.php index c0e8173..5f94e31 100644 --- a/app/Models/Finding.php +++ b/app/Models/Finding.php @@ -61,5 +61,7 @@ public function acknowledge(User $user): void 'acknowledged_at' => now(), 'acknowledged_by_user_id' => $user->getKey(), ]); + + $this->save(); } } diff --git a/app/Services/Drift/DriftFindingDiffBuilder.php b/app/Services/Drift/DriftFindingDiffBuilder.php new file mode 100644 index 0000000..1196947 --- /dev/null +++ b/app/Services/Drift/DriftFindingDiffBuilder.php @@ -0,0 +1,304 @@ + + */ + public function buildSettingsDiff(?PolicyVersion $baselineVersion, ?PolicyVersion $currentVersion): array + { + $policyType = $currentVersion?->policy_type ?? $baselineVersion?->policy_type ?? ''; + $platform = $currentVersion?->platform ?? $baselineVersion?->platform; + + $from = $baselineVersion + ? $this->settingsNormalizer->normalizeForDiff(is_array($baselineVersion->snapshot) ? $baselineVersion->snapshot : [], (string) $policyType, $platform) + : []; + + $to = $currentVersion + ? $this->settingsNormalizer->normalizeForDiff(is_array($currentVersion->snapshot) ? $currentVersion->snapshot : [], (string) $policyType, $platform) + : []; + + $result = $this->versionDiff->compare($from, $to); + $result['policy_type'] = $policyType; + + return $result; + } + + /** + * @return array + */ + public function buildAssignmentsDiff(Tenant $tenant, ?PolicyVersion $baselineVersion, ?PolicyVersion $currentVersion, int $limit = 200): array + { + $baseline = $baselineVersion ? $this->assignmentsNormalizer->normalizeForDiff($baselineVersion->assignments) : []; + $current = $currentVersion ? $this->assignmentsNormalizer->normalizeForDiff($currentVersion->assignments) : []; + + $baselineMap = []; + foreach ($baseline as $row) { + $baselineMap[$row['key']] = $row; + } + + $currentMap = []; + foreach ($current as $row) { + $currentMap[$row['key']] = $row; + } + + $allKeys = array_values(array_unique(array_merge(array_keys($baselineMap), array_keys($currentMap)))); + sort($allKeys); + + $added = []; + $removed = []; + $changed = []; + + foreach ($allKeys as $key) { + $from = $baselineMap[$key] ?? null; + $to = $currentMap[$key] ?? null; + + if ($from === null && is_array($to)) { + $added[] = $to; + + continue; + } + + if ($to === null && is_array($from)) { + $removed[] = $from; + + continue; + } + + if (! is_array($from) || ! is_array($to)) { + continue; + } + + $diffFields = [ + 'filter_type', + 'filter_id', + 'intent', + 'mode', + ]; + + $fieldChanges = []; + + foreach ($diffFields as $field) { + $fromValue = $from[$field] ?? null; + $toValue = $to[$field] ?? null; + + if ($fromValue !== $toValue) { + $fieldChanges[$field] = [ + 'from' => $fromValue, + 'to' => $toValue, + ]; + } + } + + if ($fieldChanges !== []) { + $changed[] = [ + 'key' => $key, + 'include_exclude' => $to['include_exclude'], + 'target_type' => $to['target_type'], + 'target_id' => $to['target_id'], + 'from' => $from, + 'to' => $to, + 'changes' => $fieldChanges, + ]; + } + } + + $truncated = false; + + $total = count($added) + count($removed) + count($changed); + if ($total > $limit) { + $truncated = true; + + $budget = $limit; + + $changed = array_slice($changed, 0, min(count($changed), $budget)); + $budget -= count($changed); + + $added = array_slice($added, 0, min(count($added), $budget)); + $budget -= count($added); + + $removed = array_slice($removed, 0, min(count($removed), $budget)); + } + + $labels = $this->groupLabelsForDiff($tenant, $added, $removed, $changed); + + $decorateAssignment = function (array $row) use ($labels): array { + $row['target_label'] = $this->targetLabel($row, $labels); + + return $row; + }; + + $decorateChanged = function (array $row) use ($decorateAssignment): array { + $row['from'] = is_array($row['from'] ?? null) ? $decorateAssignment($row['from']) : $row['from']; + $row['to'] = is_array($row['to'] ?? null) ? $decorateAssignment($row['to']) : $row['to']; + $row['target_label'] = is_array($row['to'] ?? null) ? ($row['to']['target_label'] ?? null) : null; + + return $row; + }; + + return [ + 'summary' => [ + 'added' => count($added), + 'removed' => count($removed), + 'changed' => count($changed), + 'message' => sprintf('%d added, %d removed, %d changed', count($added), count($removed), count($changed)), + 'truncated' => $truncated, + 'limit' => $limit, + ], + 'added' => array_map($decorateAssignment, $added), + 'removed' => array_map($decorateAssignment, $removed), + 'changed' => array_map($decorateChanged, $changed), + ]; + } + + /** + * @return array + */ + public function buildScopeTagsDiff(?PolicyVersion $baselineVersion, ?PolicyVersion $currentVersion): array + { + $baselineIds = $baselineVersion ? ($this->scopeTagsNormalizer->normalizeIdsForHash($baselineVersion->scope_tags) ?? []) : []; + $currentIds = $currentVersion ? ($this->scopeTagsNormalizer->normalizeIdsForHash($currentVersion->scope_tags) ?? []) : []; + + $baselineLabels = $baselineVersion ? $this->scopeTagsNormalizer->labelsById($baselineVersion->scope_tags) : []; + $currentLabels = $currentVersion ? $this->scopeTagsNormalizer->labelsById($currentVersion->scope_tags) : []; + + $baselineSet = array_fill_keys($baselineIds, true); + $currentSet = array_fill_keys($currentIds, true); + + $addedIds = array_values(array_diff($currentIds, $baselineIds)); + $removedIds = array_values(array_diff($baselineIds, $currentIds)); + + sort($addedIds); + sort($removedIds); + + $decorate = static function (array $ids, array $labels): array { + $rows = []; + + foreach ($ids as $id) { + if (! is_string($id) || $id === '') { + continue; + } + + $rows[] = [ + 'id' => $id, + 'name' => $labels[$id] ?? ($id === '0' ? 'Default' : $id), + ]; + } + + return $rows; + }; + + return [ + 'summary' => [ + 'added' => count($addedIds), + 'removed' => count($removedIds), + 'changed' => 0, + 'message' => sprintf('%d added, %d removed', count($addedIds), count($removedIds)), + 'baseline_count' => count($baselineSet), + 'current_count' => count($currentSet), + ], + 'added' => $decorate($addedIds, $currentLabels), + 'removed' => $decorate($removedIds, $baselineLabels), + 'baseline' => $decorate($baselineIds, $baselineLabels), + 'current' => $decorate($currentIds, $currentLabels), + 'changed' => [], + ]; + } + + /** + * @param array> $added + * @param array> $removed + * @param array> $changed + * @return array + */ + private function groupLabelsForDiff(Tenant $tenant, array $added, array $removed, array $changed): array + { + $groupIds = []; + + foreach ([$added, $removed] as $items) { + foreach ($items as $row) { + $targetType = $row['target_type'] ?? null; + $targetId = $row['target_id'] ?? null; + + if (! is_string($targetType) || ! is_string($targetId)) { + continue; + } + + if (! str_contains($targetType, 'groupassignmenttarget')) { + continue; + } + + $groupIds[] = $targetId; + } + } + + foreach ($changed as $row) { + $targetType = $row['target_type'] ?? null; + $targetId = $row['target_id'] ?? null; + + if (! is_string($targetType) || ! is_string($targetId)) { + continue; + } + + if (! str_contains($targetType, 'groupassignmenttarget')) { + continue; + } + + $groupIds[] = $targetId; + } + + $groupIds = array_values(array_unique($groupIds)); + + if ($groupIds === []) { + return []; + } + + return $this->groupLabelResolver->resolveMany($tenant, $groupIds); + } + + /** + * @param array $assignment + * @param array $groupLabels + */ + private function targetLabel(array $assignment, array $groupLabels): string + { + $targetType = $assignment['target_type'] ?? null; + $targetId = $assignment['target_id'] ?? null; + + if (! is_string($targetType) || ! is_string($targetId)) { + return 'Unknown target'; + } + + if (str_contains($targetType, 'alldevicesassignmenttarget')) { + return 'All devices'; + } + + if (str_contains($targetType, 'allusersassignmenttarget')) { + return 'All users'; + } + + if (str_contains($targetType, 'groupassignmenttarget')) { + return $groupLabels[$targetId] ?? EntraGroupLabelResolver::formatLabel(null, $targetId); + } + + return sprintf('%s (%s)', $targetType, $targetId); + } +} diff --git a/app/Services/Drift/DriftFindingGenerator.php b/app/Services/Drift/DriftFindingGenerator.php index 1fc88fa..1944925 100644 --- a/app/Services/Drift/DriftFindingGenerator.php +++ b/app/Services/Drift/DriftFindingGenerator.php @@ -7,6 +7,8 @@ use App\Models\Policy; use App\Models\PolicyVersion; use App\Models\Tenant; +use App\Services\Drift\Normalizers\ScopeTagsNormalizer; +use App\Services\Drift\Normalizers\SettingsNormalizer; use Illuminate\Support\Arr; use RuntimeException; @@ -15,6 +17,8 @@ class DriftFindingGenerator public function __construct( private readonly DriftHasher $hasher, private readonly DriftEvidence $evidence, + private readonly SettingsNormalizer $settingsNormalizer, + private readonly ScopeTagsNormalizer $scopeTagsNormalizer, ) {} public function generate(Tenant $tenant, InventorySyncRun $baseline, InventorySyncRun $current, string $scopeKey): int @@ -48,48 +52,139 @@ public function generate(Tenant $tenant, InventorySyncRun $baseline, InventorySy $baselineVersion = $this->versionForRun($policy, $baseline); $currentVersion = $this->versionForRun($policy, $current); + if ($baselineVersion instanceof PolicyVersion || $currentVersion instanceof PolicyVersion) { + $policyType = (string) ($policy->policy_type ?? ''); + $platform = is_string($policy->platform ?? null) ? $policy->platform : null; + + $baselineSnapshot = $baselineVersion instanceof PolicyVersion && is_array($baselineVersion->snapshot) + ? $baselineVersion->snapshot + : []; + $currentSnapshot = $currentVersion instanceof PolicyVersion && is_array($currentVersion->snapshot) + ? $currentVersion->snapshot + : []; + + $baselineNormalized = $this->settingsNormalizer->normalizeForDiff($baselineSnapshot, $policyType, $platform); + $currentNormalized = $this->settingsNormalizer->normalizeForDiff($currentSnapshot, $policyType, $platform); + + $baselineSnapshotHash = $this->hasher->hashNormalized($baselineNormalized); + $currentSnapshotHash = $this->hasher->hashNormalized($currentNormalized); + + if ($baselineSnapshotHash !== $currentSnapshotHash) { + $changeType = match (true) { + $baselineVersion instanceof PolicyVersion && ! $currentVersion instanceof PolicyVersion => 'removed', + ! $baselineVersion instanceof PolicyVersion && $currentVersion instanceof PolicyVersion => 'added', + default => 'modified', + }; + + $fingerprint = $this->hasher->fingerprint( + tenantId: (int) $tenant->getKey(), + scopeKey: $scopeKey, + subjectType: 'policy', + subjectExternalId: (string) $policy->external_id, + changeType: $changeType, + baselineHash: $baselineSnapshotHash, + currentHash: $currentSnapshotHash, + ); + + $rawEvidence = [ + 'change_type' => $changeType, + 'summary' => [ + 'kind' => 'policy_snapshot', + 'changed_fields' => ['snapshot_hash'], + ], + 'baseline' => [ + 'policy_id' => $policy->external_id, + 'policy_version_id' => $baselineVersion?->getKey(), + 'snapshot_hash' => $baselineSnapshotHash, + ], + 'current' => [ + 'policy_id' => $policy->external_id, + 'policy_version_id' => $currentVersion?->getKey(), + 'snapshot_hash' => $currentSnapshotHash, + ], + ]; + + $finding = Finding::query()->firstOrNew([ + 'tenant_id' => $tenant->getKey(), + 'fingerprint' => $fingerprint, + ]); + + $wasNew = ! $finding->exists; + + $finding->forceFill([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'scope_key' => $scopeKey, + 'baseline_run_id' => $baseline->getKey(), + 'current_run_id' => $current->getKey(), + 'subject_type' => 'policy', + 'subject_external_id' => (string) $policy->external_id, + 'severity' => Finding::SEVERITY_MEDIUM, + 'evidence_jsonb' => $this->evidence->sanitize($rawEvidence), + ]); + + if ($wasNew) { + $finding->forceFill([ + 'status' => Finding::STATUS_NEW, + 'acknowledged_at' => null, + 'acknowledged_by_user_id' => null, + ]); + } + + $finding->save(); + + if ($wasNew) { + $created++; + } + } + } + if (! $baselineVersion instanceof PolicyVersion || ! $currentVersion instanceof PolicyVersion) { continue; } - $baselineAssignmentsHash = $baselineVersion->assignments_hash ?? null; - $currentAssignmentsHash = $currentVersion->assignments_hash ?? null; + $baselineAssignments = is_array($baselineVersion->assignments) ? $baselineVersion->assignments : []; + $currentAssignments = is_array($currentVersion->assignments) ? $currentVersion->assignments : []; - if ($baselineAssignmentsHash === $currentAssignmentsHash) { - continue; - } + $baselineAssignmentsHash = $this->hasher->hashNormalized($baselineAssignments); + $currentAssignmentsHash = $this->hasher->hashNormalized($currentAssignments); - $fingerprint = $this->hasher->fingerprint( - tenantId: (int) $tenant->getKey(), - scopeKey: $scopeKey, - subjectType: 'assignment', - subjectExternalId: (string) $policy->external_id, - changeType: 'modified', - baselineHash: (string) ($baselineAssignmentsHash ?? ''), - currentHash: (string) ($currentAssignmentsHash ?? ''), - ); + if ($baselineAssignmentsHash !== $currentAssignmentsHash) { + $fingerprint = $this->hasher->fingerprint( + tenantId: (int) $tenant->getKey(), + scopeKey: $scopeKey, + subjectType: 'assignment', + subjectExternalId: (string) $policy->external_id, + changeType: 'modified', + baselineHash: (string) ($baselineAssignmentsHash ?? ''), + currentHash: (string) ($currentAssignmentsHash ?? ''), + ); - $rawEvidence = [ - 'change_type' => 'modified', - 'summary' => 'Policy assignments changed', - 'baseline' => [ - 'policy_id' => $policy->external_id, - 'policy_version_id' => $baselineVersion->getKey(), - 'assignments_hash' => $baselineAssignmentsHash, - ], - 'current' => [ - 'policy_id' => $policy->external_id, - 'policy_version_id' => $currentVersion->getKey(), - 'assignments_hash' => $currentAssignmentsHash, - ], - ]; + $rawEvidence = [ + 'change_type' => 'modified', + 'summary' => [ + 'kind' => 'policy_assignments', + 'changed_fields' => ['assignments_hash'], + ], + 'baseline' => [ + 'policy_id' => $policy->external_id, + 'policy_version_id' => $baselineVersion->getKey(), + 'assignments_hash' => $baselineAssignmentsHash, + ], + 'current' => [ + 'policy_id' => $policy->external_id, + 'policy_version_id' => $currentVersion->getKey(), + 'assignments_hash' => $currentAssignmentsHash, + ], + ]; - Finding::query()->updateOrCreate( - [ + $finding = Finding::query()->firstOrNew([ 'tenant_id' => $tenant->getKey(), 'fingerprint' => $fingerprint, - ], - [ + ]); + + $wasNew = ! $finding->exists; + + $finding->forceFill([ 'finding_type' => Finding::FINDING_TYPE_DRIFT, 'scope_key' => $scopeKey, 'baseline_run_id' => $baseline->getKey(), @@ -97,14 +192,97 @@ public function generate(Tenant $tenant, InventorySyncRun $baseline, InventorySy 'subject_type' => 'assignment', 'subject_external_id' => (string) $policy->external_id, 'severity' => Finding::SEVERITY_MEDIUM, + 'evidence_jsonb' => $this->evidence->sanitize($rawEvidence), + ]); + + if ($wasNew) { + $finding->forceFill([ + 'status' => Finding::STATUS_NEW, + 'acknowledged_at' => null, + 'acknowledged_by_user_id' => null, + ]); + } + + $finding->save(); + + if ($wasNew) { + $created++; + } + } + + $baselineScopeTagIds = $this->scopeTagsNormalizer->normalizeIdsForHash($baselineVersion->scope_tags); + $currentScopeTagIds = $this->scopeTagsNormalizer->normalizeIdsForHash($currentVersion->scope_tags); + + if ($baselineScopeTagIds === null || $currentScopeTagIds === null) { + continue; + } + + $baselineScopeTagsHash = $this->hasher->hashNormalized($baselineScopeTagIds); + $currentScopeTagsHash = $this->hasher->hashNormalized($currentScopeTagIds); + + if ($baselineScopeTagsHash === $currentScopeTagsHash) { + continue; + } + + $fingerprint = $this->hasher->fingerprint( + tenantId: (int) $tenant->getKey(), + scopeKey: $scopeKey, + subjectType: 'scope_tag', + subjectExternalId: (string) $policy->external_id, + changeType: 'modified', + baselineHash: $baselineScopeTagsHash, + currentHash: $currentScopeTagsHash, + ); + + $rawEvidence = [ + 'change_type' => 'modified', + 'summary' => [ + 'kind' => 'policy_scope_tags', + 'changed_fields' => ['scope_tags_hash'], + ], + 'baseline' => [ + 'policy_id' => $policy->external_id, + 'policy_version_id' => $baselineVersion->getKey(), + 'scope_tags_hash' => $baselineScopeTagsHash, + ], + 'current' => [ + 'policy_id' => $policy->external_id, + 'policy_version_id' => $currentVersion->getKey(), + 'scope_tags_hash' => $currentScopeTagsHash, + ], + ]; + + $finding = Finding::query()->firstOrNew([ + 'tenant_id' => $tenant->getKey(), + 'fingerprint' => $fingerprint, + ]); + + $wasNew = ! $finding->exists; + + $finding->forceFill([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'scope_key' => $scopeKey, + 'baseline_run_id' => $baseline->getKey(), + 'current_run_id' => $current->getKey(), + 'subject_type' => 'scope_tag', + 'subject_external_id' => (string) $policy->external_id, + 'severity' => Finding::SEVERITY_MEDIUM, + 'evidence_jsonb' => $this->evidence->sanitize($rawEvidence), + ]); + + if ($wasNew) { + $finding->forceFill([ 'status' => Finding::STATUS_NEW, 'acknowledged_at' => null, 'acknowledged_by_user_id' => null, - 'evidence_jsonb' => $this->evidence->sanitize($rawEvidence), - ], - ); + ]); + } - $created++; + $finding->save(); + + if ($wasNew) { + $created++; + } } }); diff --git a/app/Services/Drift/DriftHasher.php b/app/Services/Drift/DriftHasher.php index 6ca3aee..6f9ec9d 100644 --- a/app/Services/Drift/DriftHasher.php +++ b/app/Services/Drift/DriftHasher.php @@ -4,6 +4,24 @@ class DriftHasher { + /** + * @param array $volatileKeys + */ + public function hashNormalized(mixed $value, array $volatileKeys = [ + '@odata.context', + '@odata.etag', + 'createdDateTime', + 'lastModifiedDateTime', + 'modifiedDateTime', + 'createdAt', + 'updatedAt', + ]): string + { + $normalized = $this->normalizeValue($value, $volatileKeys); + + return hash('sha256', json_encode($normalized, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE)); + } + public function fingerprint( int $tenantId, string $scopeKey, @@ -30,4 +48,54 @@ private function normalize(string $value): string { return trim(mb_strtolower($value)); } + + /** + * @param array $volatileKeys + */ + private function normalizeValue(mixed $value, array $volatileKeys): mixed + { + if (is_array($value)) { + if ($this->isList($value)) { + $items = array_map(fn ($item) => $this->normalizeValue($item, $volatileKeys), $value); + + usort($items, function ($a, $b): int { + return strcmp( + json_encode($a, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE), + json_encode($b, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE) + ); + }); + + return $items; + } + + $result = []; + + foreach ($value as $key => $item) { + if (is_string($key) && in_array($key, $volatileKeys, true)) { + continue; + } + + $result[$key] = $this->normalizeValue($item, $volatileKeys); + } + + ksort($result); + + return $result; + } + + if (is_string($value)) { + return trim($value); + } + + return $value; + } + + private function isList(array $value): bool + { + if ($value === []) { + return true; + } + + return array_keys($value) === range(0, count($value) - 1); + } } diff --git a/app/Services/Drift/Normalizers/AssignmentsNormalizer.php b/app/Services/Drift/Normalizers/AssignmentsNormalizer.php new file mode 100644 index 0000000..77b85c6 --- /dev/null +++ b/app/Services/Drift/Normalizers/AssignmentsNormalizer.php @@ -0,0 +1,113 @@ + + */ + public function normalizeForDiff(mixed $assignments): array + { + if (! is_array($assignments)) { + return []; + } + + $rows = []; + + foreach ($assignments as $assignment) { + if (! is_array($assignment)) { + continue; + } + + $target = $assignment['target'] ?? null; + if (! is_array($target)) { + continue; + } + + $rawType = $target['@odata.type'] ?? null; + $targetType = $this->normalizeOdataType(is_string($rawType) ? $rawType : ''); + + $includeExclude = str_contains($targetType, 'exclusion') ? 'exclude' : 'include'; + $targetId = $this->extractTargetId($targetType, $target); + + if ($targetId === '') { + continue; + } + + $filterId = $target['deviceAndAppManagementAssignmentFilterId'] ?? null; + $filterType = $target['deviceAndAppManagementAssignmentFilterType'] ?? 'none'; + + $intent = $assignment['intent'] ?? null; + $mode = $assignment['mode'] ?? null; + + $row = [ + 'key' => implode('|', [ + $includeExclude, + $targetType, + $targetId, + ]), + 'include_exclude' => $includeExclude, + 'target_type' => $targetType, + 'target_id' => $targetId, + 'filter_type' => is_string($filterType) && $filterType !== '' ? strtolower(trim($filterType)) : 'none', + 'filter_id' => is_string($filterId) && $filterId !== '' ? $filterId : null, + 'intent' => is_string($intent) && $intent !== '' ? strtolower(trim($intent)) : null, + 'mode' => is_string($mode) && $mode !== '' ? strtolower(trim($mode)) : null, + ]; + + $rows[] = $row; + } + + usort($rows, function (array $a, array $b): int { + return strcmp($a['key'], $b['key']); + }); + + return array_values($rows); + } + + private function normalizeOdataType(string $odataType): string + { + $value = trim($odataType); + $value = ltrim($value, '#'); + + if ($value === '') { + return 'unknown'; + } + + if (str_contains($value, '.')) { + $value = (string) strrchr($value, '.'); + $value = ltrim($value, '.'); + } + + return strtolower(trim($value)); + } + + /** + * @param array $target + */ + private function extractTargetId(string $targetType, array $target): string + { + if (str_contains($targetType, 'alldevicesassignmenttarget')) { + return 'all_devices'; + } + + if (str_contains($targetType, 'allusersassignmenttarget')) { + return 'all_users'; + } + + $groupId = Arr::get($target, 'groupId'); + if (is_string($groupId) && $groupId !== '') { + return $groupId; + } + + $collectionId = Arr::get($target, 'collectionId'); + if (is_string($collectionId) && $collectionId !== '') { + return $collectionId; + } + + return ''; + } +} diff --git a/app/Services/Drift/Normalizers/ScopeTagsNormalizer.php b/app/Services/Drift/Normalizers/ScopeTagsNormalizer.php new file mode 100644 index 0000000..a543d15 --- /dev/null +++ b/app/Services/Drift/Normalizers/ScopeTagsNormalizer.php @@ -0,0 +1,136 @@ + + */ + public function normalizeIds(mixed $scopeTags): array + { + return $this->normalizeIdsForHash($scopeTags) ?? []; + } + + /** + * For drift hashing/comparison we need stable, reliable IDs. + * + * Legacy policy versions may have only `names` without `ids`. In that case we: + * - infer `Default` as id `0` + * - otherwise return null (unknown/unreliable; should not create drift) + * + * @return array|null + */ + public function normalizeIdsForHash(mixed $scopeTags): ?array + { + if (! is_array($scopeTags)) { + return []; + } + + $ids = $scopeTags['ids'] ?? null; + if (is_array($ids)) { + $normalized = []; + + foreach ($ids as $id) { + if (! is_string($id)) { + continue; + } + + $id = trim($id); + + if ($id === '') { + continue; + } + + $normalized[] = $id; + } + + $normalized = array_values(array_unique($normalized)); + sort($normalized); + + return $normalized; + } + + $names = $scopeTags['names'] ?? null; + if (! is_array($names) || $names === []) { + return []; + } + + $normalizedNames = []; + + foreach ($names as $name) { + if (! is_string($name)) { + continue; + } + + $name = trim($name); + + if ($name === '') { + continue; + } + + $normalizedNames[] = strtolower($name); + } + + $normalizedNames = array_values(array_unique($normalizedNames)); + sort($normalizedNames); + + if ($normalizedNames === ['default']) { + return ['0']; + } + + return null; + } + + /** + * @return array + */ + public function labelsById(mixed $scopeTags): array + { + if (! is_array($scopeTags)) { + return []; + } + + $ids = is_array($scopeTags['ids'] ?? null) ? $scopeTags['ids'] : null; + $names = is_array($scopeTags['names'] ?? null) ? $scopeTags['names'] : []; + + if (! is_array($ids)) { + $inferred = $this->normalizeIdsForHash($scopeTags); + + if ($inferred === ['0'] && $names !== []) { + return ['0' => 'Default']; + } + + return []; + } + + $labels = []; + + foreach ($ids as $index => $id) { + if (! is_string($id)) { + continue; + } + + $id = trim($id); + + if ($id === '') { + continue; + } + + $name = $names[$index] ?? ''; + $name = is_string($name) ? trim($name) : ''; + + if ($name === '') { + $name = $id === '0' ? 'Default' : $id; + } + + if (! array_key_exists($id, $labels) || $labels[$id] === $id) { + $labels[$id] = $name; + } + } + + ksort($labels); + + return $labels; + } +} diff --git a/app/Services/Drift/Normalizers/SettingsNormalizer.php b/app/Services/Drift/Normalizers/SettingsNormalizer.php new file mode 100644 index 0000000..38b352e --- /dev/null +++ b/app/Services/Drift/Normalizers/SettingsNormalizer.php @@ -0,0 +1,19 @@ +|null $snapshot + * @return array + */ + public function normalizeForDiff(?array $snapshot, string $policyType, ?string $platform = null): array + { + return $this->policyNormalizer->flattenForDiff($snapshot ?? [], $policyType, $platform); + } +} diff --git a/config/intune_permissions.php b/config/intune_permissions.php index 609aedc..96677d8 100644 --- a/config/intune_permissions.php +++ b/config/intune_permissions.php @@ -6,13 +6,13 @@ 'key' => 'DeviceManagementConfiguration.ReadWrite.All', 'type' => 'application', 'description' => 'Read and write Intune device configuration policies.', - 'features' => ['policy-sync', 'backup', 'restore', 'settings-normalization'], + 'features' => ['policy-sync', 'backup', 'restore', 'settings-normalization', 'drift'], ], [ 'key' => 'DeviceManagementConfiguration.Read.All', 'type' => 'application', 'description' => 'Read Intune device configuration policies (least-privilege for inventory).', - 'features' => ['policy-sync', 'backup', 'settings-normalization'], + 'features' => ['policy-sync', 'backup', 'settings-normalization', 'drift'], ], [ 'key' => 'DeviceManagementApps.ReadWrite.All', @@ -72,7 +72,7 @@ 'key' => 'Group.Read.All', 'type' => 'application', 'description' => 'Read group information for resolving assignment group names and cross-tenant group mapping.', - 'features' => ['assignments', 'group-mapping', 'backup-metadata', 'directory-groups', 'group-directory-cache'], + 'features' => ['assignments', 'group-mapping', 'backup-metadata', 'directory-groups', 'group-directory-cache', 'drift'], ], [ 'key' => 'DeviceManagementScripts.ReadWrite.All', diff --git a/resources/views/filament/infolists/entries/assignments-diff.blade.php b/resources/views/filament/infolists/entries/assignments-diff.blade.php new file mode 100644 index 0000000..1118937 --- /dev/null +++ b/resources/views/filament/infolists/entries/assignments-diff.blade.php @@ -0,0 +1,114 @@ +@php + $diff = $getState() ?? []; + $summary = $diff['summary'] ?? []; + + $added = is_array($diff['added'] ?? null) ? $diff['added'] : []; + $removed = is_array($diff['removed'] ?? null) ? $diff['removed'] : []; + $changed = is_array($diff['changed'] ?? null) ? $diff['changed'] : []; + + $renderRow = static function (array $row): array { + return [ + 'include_exclude' => (string) ($row['include_exclude'] ?? 'include'), + 'target_label' => (string) ($row['target_label'] ?? 'Unknown target'), + 'filter_type' => (string) ($row['filter_type'] ?? 'none'), + 'filter_id' => $row['filter_id'] ?? null, + 'intent' => $row['intent'] ?? null, + 'mode' => $row['mode'] ?? null, + ]; + }; +@endphp + +
+ +
+ + {{ (int) ($summary['added'] ?? 0) }} added + + + {{ (int) ($summary['removed'] ?? 0) }} removed + + + {{ (int) ($summary['changed'] ?? 0) }} changed + + + @if (($summary['truncated'] ?? false) === true) + + Truncated to {{ (int) ($summary['limit'] ?? 0) }} items + + @endif +
+ + + @if ($changed !== []) + +
+ @foreach ($changed as $row) + @php + $to = is_array($row['to'] ?? null) ? $renderRow($row['to']) : $renderRow([]); + $from = is_array($row['from'] ?? null) ? $renderRow($row['from']) : $renderRow([]); + @endphp + +
+
+ {{ $to['target_label'] }} +
+ +
+
+
From
+
+
Type: {{ $from['include_exclude'] }}
+
Filter: {{ $from['filter_type'] }}@if($from['filter_id']) ({{ $from['filter_id'] }})@endif
+
+
+
+
To
+
+
Type: {{ $to['include_exclude'] }}
+
Filter: {{ $to['filter_type'] }}@if($to['filter_id']) ({{ $to['filter_id'] }})@endif
+
+
+
+
+ @endforeach +
+ + @endif + + @if ($added !== []) + +
+ @foreach ($added as $row) + @php $row = $renderRow(is_array($row) ? $row : []); @endphp + +
+
{{ $row['target_label'] }}
+
+ {{ $row['include_exclude'] }} · filter: {{ $row['filter_type'] }} +
+
+ @endforeach +
+ + @endif + + @if ($removed !== []) + +
+ @foreach ($removed as $row) + @php $row = $renderRow(is_array($row) ? $row : []); @endphp + +
+
{{ $row['target_label'] }}
+
+ {{ $row['include_exclude'] }} · filter: {{ $row['filter_type'] }} +
+
+ @endforeach +
+ + @endif +
diff --git a/resources/views/filament/infolists/entries/scope-tags-diff.blade.php b/resources/views/filament/infolists/entries/scope-tags-diff.blade.php new file mode 100644 index 0000000..324f574 --- /dev/null +++ b/resources/views/filament/infolists/entries/scope-tags-diff.blade.php @@ -0,0 +1,111 @@ +@php + $diff = $getState() ?? []; + $summary = $diff['summary'] ?? []; + + $added = is_array($diff['added'] ?? null) ? $diff['added'] : []; + $removed = is_array($diff['removed'] ?? null) ? $diff['removed'] : []; + $baseline = is_array($diff['baseline'] ?? null) ? $diff['baseline'] : []; + $current = is_array($diff['current'] ?? null) ? $diff['current'] : []; +@endphp + +
+ +
+ + {{ (int) ($summary['added'] ?? 0) }} added + + + {{ (int) ($summary['removed'] ?? 0) }} removed + + + Baseline: {{ (int) ($summary['baseline_count'] ?? 0) }} + + + Current: {{ (int) ($summary['current_count'] ?? 0) }} + +
+ + + @if ($added !== []) + +
+ @foreach ($added as $row) + @php + $name = (string) ($row['name'] ?? 'Unknown'); + $id = (string) ($row['id'] ?? ''); + @endphp + +
+
{{ $name }}
+ @if ($id !== '') +
{{ $id }}
+ @endif +
+ @endforeach +
+ + @endif + + @if ($removed !== []) + +
+ @foreach ($removed as $row) + @php + $name = (string) ($row['name'] ?? 'Unknown'); + $id = (string) ($row['id'] ?? ''); + @endphp + +
+
{{ $name }}
+ @if ($id !== '') +
{{ $id }}
+ @endif +
+ @endforeach +
+ + @endif + + @if ($current !== []) + +
+ @foreach ($current as $row) + @php + $name = (string) ($row['name'] ?? 'Unknown'); + $id = (string) ($row['id'] ?? ''); + @endphp + +
+
{{ $name }}
+ @if ($id !== '') +
{{ $id }}
+ @endif +
+ @endforeach +
+ + @endif + + @if ($baseline !== []) + +
+ @foreach ($baseline as $row) + @php + $name = (string) ($row['name'] ?? 'Unknown'); + $id = (string) ($row['id'] ?? ''); + @endphp + +
+
{{ $name }}
+ @if ($id !== '') +
{{ $id }}
+ @endif +
+ @endforeach +
+ + @endif +
diff --git a/resources/views/filament/pages/drift-landing.blade.php b/resources/views/filament/pages/drift-landing.blade.php index 9b631d5..d1703ed 100644 --- a/resources/views/filament/pages/drift-landing.blade.php +++ b/resources/views/filament/pages/drift-landing.blade.php @@ -5,6 +5,98 @@ Review new drift findings between the last two inventory sync runs for the current scope. + @if (filled($scopeKey)) +
+ Scope: {{ $scopeKey }} + @if ($baselineRunId && $currentRunId) + · Baseline + @if ($this->getBaselineRunUrl()) + + #{{ $baselineRunId }} + + @else + #{{ $baselineRunId }} + @endif + @if (filled($baselineFinishedAt)) + ({{ $baselineFinishedAt }}) + @endif + · Current + @if ($this->getCurrentRunUrl()) + + #{{ $currentRunId }} + + @else + #{{ $currentRunId }} + @endif + @if (filled($currentFinishedAt)) + ({{ $currentFinishedAt }}) + @endif + @endif +
+ @endif + + @if ($state === 'blocked') + + Blocked + + + @if (filled($message)) +
+ {{ $message }} +
+ @endif + @elseif ($state === 'generating') + + Generating + + +
+ Drift generation has been queued. Refresh this page once it finishes. +
+ + @if ($this->getBulkRunUrl()) +
+ + View run #{{ $bulkOperationRunId }} + +
+ @endif + @elseif ($state === 'error') + + Error + + + @if (filled($message)) +
+ {{ $message }} +
+ @endif + + @if ($this->getBulkRunUrl()) +
+ + View run #{{ $bulkOperationRunId }} + +
+ @endif + @elseif ($state === 'ready') +
+ + New: {{ (int) ($statusCounts['new'] ?? 0) }} + +
+ + @if (filled($message)) +
+ {{ $message }} +
+ @endif + @else + + Ready + + @endif +
Findings diff --git a/specs/044-drift-mvp/checklists/requirements.md b/specs/044-drift-mvp/checklists/requirements.md index 04dab55..ceb096c 100644 --- a/specs/044-drift-mvp/checklists/requirements.md +++ b/specs/044-drift-mvp/checklists/requirements.md @@ -6,28 +6,28 @@ # Specification Quality Checklist: Drift MVP (044) ## Content Quality -- [ ] No implementation details (languages, frameworks, APIs) (T002) -- [x] Focused on user value and business needs (spec.md: Purpose, User Scenarios & Testing, Success Criteria) -- [ ] Written for non-technical stakeholders (T002) -- [x] All mandatory sections completed (spec.md includes Purpose, Scenarios, FR/NFR, Success Criteria, Out of Scope) +- [x] No implementation details (languages, frameworks, APIs) (spec.md contains scenarios/rules/states/acceptance only) +- [x] Focused on user value and business needs (spec.md: Purpose, User Scenarios, Acceptance Criteria) +- [x] Written for non-technical stakeholders (spec.md uses plain language; avoids code/framework terms) +- [x] All mandatory sections completed (spec.md includes Purpose, User Scenarios, Rules, Acceptance Criteria) ## Requirement Completeness - [x] No [NEEDS CLARIFICATION] markers remain (spec.md: no "[NEEDS CLARIFICATION]" markers) -- [x] Requirements are testable and unambiguous (spec.md: FR1–FR4; tasks.md defines tests for key behaviors T015–T018, T024–T025, T029–T030, T035, T038) -- [x] Success criteria are measurable (spec.md: SC1 "under 3 minutes", SC2 deterministic consistency) -- [x] Success criteria are technology-agnostic (no implementation details) (spec.md: SC1–SC2) +- [x] Requirements are testable and unambiguous (spec.md: Rules + Acceptance Criteria) +- [x] Success criteria are measurable (spec.md: Acceptance Criteria) +- [x] Success criteria are technology-agnostic (no implementation details) (spec.md: Acceptance Criteria) - [x] All acceptance scenarios are defined (spec.md: Scenario 1/2/3) -- [x] Edge cases are identified (spec.md: <2 runs blocked state; generation failure explicit error state; acknowledgement per comparison) -- [x] Scope is clearly bounded (spec.md: FR2b + Out of Scope) -- [x] Dependencies and assumptions identified (spec.md: Dependencies / Name Resolution; NFR2; "No render-time Graph calls") +- [x] Edge cases are identified (spec.md: blocked state; error state; acknowledgement per comparison) +- [x] Scope is clearly bounded (spec.md: Rules → Coverage (MVP)) +- [x] Dependencies and assumptions identified (spec.md: Rules → UI states; Run tracking) ## Feature Readiness -- [x] All functional requirements have clear acceptance criteria (spec.md: FR1–FR4 + Scenario 1/2/3) +- [x] All functional requirements have clear acceptance criteria (spec.md: Rules + Acceptance Criteria) - [x] User scenarios cover primary flows (spec.md: Scenario 1/2/3) -- [ ] Feature meets measurable outcomes defined in Success Criteria (T022, T023, T026, T027, T031, T033, T035) -- [ ] No implementation details leak into specification (T002) +- [x] Feature meets measurable outcomes defined in Success Criteria (spec.md: Acceptance Criteria are measurable and testable) +- [x] No implementation details leak into specification (spec.md avoids implementation and names a generic “persisted run record” only) ## Notes diff --git a/specs/044-drift-mvp/contracts/admin-findings.openapi.yaml b/specs/044-drift-mvp/contracts/admin-findings.openapi.yaml index 3c742bd..7d20e3c 100644 --- a/specs/044-drift-mvp/contracts/admin-findings.openapi.yaml +++ b/specs/044-drift-mvp/contracts/admin-findings.openapi.yaml @@ -102,9 +102,30 @@ paths: responses: '202': description: Accepted + content: + application/json: + schema: + type: object + properties: + data: + $ref: '#/components/schemas/DriftGenerateAccepted' components: schemas: + DriftGenerateAccepted: + type: object + properties: + bulk_operation_run_id: + type: integer + description: Canonical async run record (status/errors/idempotency) + scope_key: + type: string + baseline_run_id: + type: integer + current_run_id: + type: integer + required: [bulk_operation_run_id, scope_key, baseline_run_id, current_run_id] + Finding: type: object properties: diff --git a/specs/044-drift-mvp/plan.md b/specs/044-drift-mvp/plan.md index 81fe07e..a448d66 100644 --- a/specs/044-drift-mvp/plan.md +++ b/specs/044-drift-mvp/plan.md @@ -11,6 +11,7 @@ ## Summary - Baseline run = previous successful run for the same scope; comparison run = latest successful run. - Findings are persisted with deterministic fingerprints and support MVP triage (`new` → `acknowledged`). - UI is DB-only for label/name resolution (no render-time Graph calls). +- Drift generation is tracked via `BulkOperationRun` for status/errors across refresh and idempotency. ## Technical Context @@ -28,6 +29,7 @@ ## Technical Context - Tenant isolation for all reads/writes - No render-time Graph calls; labels resolved from DB caches - Evidence minimization (sanitized allowlist; no raw payload dumps) +- Drift generation is job-only and uses `BulkOperationRun` (`resource=drift`, `action=generate`) as the canonical run record **Scale/Scope**: - Tenants may have large inventories; findings must be indexed for typical filtering diff --git a/specs/044-drift-mvp/quickstart.md b/specs/044-drift-mvp/quickstart.md index 2e709a3..b38df8b 100644 --- a/specs/044-drift-mvp/quickstart.md +++ b/specs/044-drift-mvp/quickstart.md @@ -15,14 +15,16 @@ ## Prepare data ## Use Drift 1. Navigate to the new Drift area. -2. On first open, Drift will dispatch a background job to generate findings for: +2. On first open, Drift will queue background generation and record status in a persisted run record. +3. Generation produces findings for: - baseline = previous successful run for the same `scope_key` - current = latest successful run for the same `scope_key` -3. Refresh the page once the job finishes. +4. Refresh the page once generation finishes. ## Triage -- Acknowledge a finding; it should move out of the “new” view but remain visible/auditable. +- Acknowledge a finding; it moves out of the default “new” view but remains visible/auditable. +- Use the status filter to include acknowledged findings. ## Notes diff --git a/specs/044-drift-mvp/spec.md b/specs/044-drift-mvp/spec.md index a3a2a61..e6c04e0 100644 --- a/specs/044-drift-mvp/spec.md +++ b/specs/044-drift-mvp/spec.md @@ -1,166 +1,81 @@ -# Feature Specification: Drift MVP - -**Feature Branch**: `feat/044-drift-mvp` -**Created**: 2026-01-07 -**Status**: Draft +# Feature Specification: Drift MVP (044) ## Purpose -Detect and report drift between expected and observed states using inventory and run metadata. +Help admins quickly spot and triage configuration “drift”: what changed between two inventory snapshots. -This MVP focuses on reporting and triage, not automatic remediation. +This MVP is about visibility and acknowledgement (triage), not automatic fixes. -## Clarifications - -### Session 2026-01-12 - -- Q: How should Drift pick the baseline run for a given tenant + scope? → A: Baseline = previous successful inventory run for the same scope; compare against the latest successful run. -- Q: Should Drift findings be persisted or computed on demand? → A: Persist findings in DB per comparison (baseline_run_id + current_run_id), including a deterministic fingerprint for stable identity + triage. -- Q: How define the fingerprint (Stable ID) for a drift finding? → A: `sha256(tenant_id + scope_key + subject_type + subject_external_id + change_type + baseline_hash + current_hash)` (normalized; excludes volatile fields). -- Q: Which inventory entities/types are in scope for Drift MVP? → A: Policies + Assignments. -- Q: When should drift findings be generated? → A: On-demand when opening Drift: if findings for (baseline,current,scope) don’t exist yet, dispatch an async job to generate them. - -### Session 2026-01-13 - -- Q: What should Drift do if there are fewer than two successful inventory runs for the same `scope_key`? → A: Show a blocked/empty state (“Need at least 2 successful runs for this scope to calculate drift”) and do not dispatch drift generation. -- Q: Should acknowledgement carry forward across comparisons? → A: No; acknowledgement is per comparison (`baseline_run_id` + `current_run_id` + `scope_key`). The same drift may re-appear as `new` in later comparisons. -- Q: Which `change_type` values are supported in Drift MVP? → A: `added`, `removed`, `modified` (assignment target/intent changes are covered under `modified`). -- Q: What is the default UI behavior for `new` vs `acknowledged` findings? → A: Default UI shows only `new`; `acknowledged` is accessible via an explicit filter. -- Q: What should the UI do if drift generation fails for a comparison? → A: Show an explicit error state (safe message + reference/run ids) and do not show findings for that comparison until a successful generation exists. - -### Session 2026-01-14 - -- Q: How should Drift track generation status/errors/idempotency for a comparison? → A: Use `BulkOperationRun` as the canonical run container (status, failures, idempotency_key, and consistent UI/ops patterns). - -## Pinned Decisions (MVP defaults) - -- Drift is implemented as a generator that writes persisted Finding rows (not only an in-memory/on-demand diff). -- Baseline selection: baseline = previous successful inventory run for the same scope_key; comparison = latest successful inventory run for the same scope_key. -- Scope is first-class via `scope_key` and must be deterministic to support future pinned baselines and compare workflows. -- Fingerprints are deterministic and stable for triage/audit workflows. -- Drift MVP only uses `finding_type=drift` and `status` in {`new`, `acknowledged`}. -- Default severity: `medium` (until a rule engine exists). -- UI must not perform render-time Graph calls. Graph access (if any) is limited to background sync/jobs. -- Drift generation is tracked via `BulkOperationRun` to persist status/errors across refresh and to enforce idempotency per (tenant, scope_key, baseline_run_id, current_run_id). - -## Key Entities / Generic Findings (Future-proof) - -### Finding (generic) - -We want Drift MVP to remain MVP-sized, while making it easy to add future generators (Security Suite Audits, Cross-tenant Compare) without inventing a new model. - -Rationale: -- Drift = delta engine over runs. -- Audit = rule engine over inventory. -- Both write Findings with the same semantics: deterministic fingerprint + triage + minimized evidence. - -- `finding_type` (enum): `drift` (MVP), later `audit`, `compare` -- `tenant_id` -- `scope_key` (string): deterministic scope identifier (see Scope Definition / FR1) -- `baseline_run_id` (nullable; e.g. audit/compare) -- `current_run_id` (nullable; e.g. audit) -- `fingerprint` (string): deterministic; unique per tenant+scope+subject+change -- `subject_type` (string): e.g. policy type (or other inventory entity type) -- `subject_external_id` (string): Graph external id -- `severity` (enum): `low` / `medium` / `high` (MVP default: `medium`) -- `status` (enum): `new` / `acknowledged` (later: `snoozed` / `assigned` / `commented`) -- `acknowledged_at` (nullable) -- `acknowledged_by_user_id` (nullable) -- `evidence_jsonb` (jsonb): sanitized, small, secrets-free (no raw payload dumps) -- Optional/nullable for later (prepared; out of MVP): `rule_id`, `control_id`, `expected_value`, `source` - -MVP implementation scope: only `finding_type=drift`, statuses `new/acknowledged`, and no rule engine. - -## User Scenarios & Testing +## User Scenarios ### Scenario 1: View drift summary -- Given inventory sync has run at least twice -- When the admin opens Drift -- Then they see a summary of changes since the last baseline -- If there are fewer than two successful runs for the same `scope_key`, Drift shows a blocked/empty state and does not start drift generation. +- Given the system has at least two successful inventory snapshots for the same selection/scope +- When an admin opens Drift +- Then they see a summary of what was added, removed, or changed since the previous snapshot ### Scenario 2: Drill into a drift finding + +- Given drift findings exist for a comparison +- When an admin opens a specific finding +- Then they can see what changed and which two snapshots were compared + +### Scenario 3: Acknowledge / triage + - Given a drift finding exists -- When the admin opens the finding -- Then they see what changed, when, and which run observed it +- When an admin acknowledges it +- Then it no longer appears in “new” views, but remains available for audit/history -### Scenario 3: Acknowledge/triage -- Given a drift finding exists -- When the admin marks it acknowledged -- Then it is hidden from “new” lists but remains auditable +## Rules -- Acknowledgement is per comparison; later comparisons may still surface the same drift as `new`. +### Coverage (MVP) -## Functional Requirements +- Drift findings cover **policies, their assignments, and scope tags** for the selected scope. -- FR1: Baseline + scope - - Define `scope_key` as the deterministic Inventory selection identifier. - - MVP definition: `scope_key = InventorySyncRun.selection_hash`. - - Rationale: selection hashing already normalizes equivalent selections; reusing it keeps drift scope stable and consistent across the product. - - Baseline run (MVP) = previous successful inventory run for the same `scope_key`. - - Comparison run (MVP) = latest successful inventory run for the same `scope_key`. +### Baseline and comparison selection -- FR2: Finding generation (Drift MVP) - - Findings are persisted per (`baseline_run_id`, `current_run_id`, `scope_key`). - - Findings cover adds, removals, and changes for supported entities (Policies + Assignments). - - MVP `change_type` values: `added`, `removed`, `modified`. - - Findings are deterministic: same baseline/current + scope_key ⇒ same set of fingerprints. - - Drift generation must be tracked via `BulkOperationRun` with an idempotency key derived from (tenant_id, scope_key, baseline_run_id, current_run_id). - - If fewer than two successful inventory runs exist for a given `scope_key`, Drift does not generate findings and must surface a clear blocked/empty state in the UI. +- Drift always compares two successful inventory snapshots for the same selection/scope. +- The “current” snapshot is the latest successful snapshot for that scope. +- The “baseline” snapshot is the previous successful snapshot for that scope. -- FR2a: Fingerprint definition (MVP) - - Fingerprint = `sha256(tenant_id + scope_key + subject_type + subject_external_id + change_type + baseline_hash + current_hash)`. - - `baseline_hash` / `current_hash` are hashes over normalized, sanitized comparison data (exclude volatile fields like timestamps). - - Goal: stable identity for triage + audit compatibility. +### Change types -- FR2b: Drift MVP scope includes Policies and their Assignments. - - Assignment drift includes target changes (e.g., groupId) and intent changes. +Each drift finding must be categorized as one of: -- FR3: Provide Drift UI with summary and details. - - Default lists and the Drift landing summary show only `status=new` by default. - - The UI must provide a filter to include `acknowledged` findings. - - If drift generation fails for a comparison, the UI must surface an explicit error state (no secrets), including reference identifiers (e.g., run ids and the `BulkOperationRun` id), and must not fall back to stale/previous results. +- **added**: the item exists in current but not in baseline +- **removed**: the item exists in baseline but not in current +- **modified**: the item exists in both but differs (including assignment target and/or intent changes) -- FR4: Triage (MVP) - - Admin can acknowledge a finding; record `acknowledged_by_user_id` + `acknowledged_at`. - - Acknowledgement does not carry forward across comparisons in the MVP. - - Findings are never deleted in the MVP. +### Acknowledgement -## Non-Functional Requirements +- Acknowledgement is **per comparison** (baseline + current within a scope). +- Acknowledgement does **not** carry forward to later comparisons. -- NFR1: Drift generation must be deterministic for the same baseline and scope. -- NFR2: Drift must remain tenant-scoped and safe to display. -- NFR3: Evidence minimization - - `evidence_jsonb` must be sanitized (no tokens/secrets) and kept small. - - MVP drift evidence should include only: - - `change_type` - - changed_fields / metadata summary (counts, field list) - - run refs (baseline_run_id/current_run_id, timestamps) - - No raw payload dumps. +### UI states -## Dependencies / Name Resolution +- **blocked**: If fewer than two successful snapshots exist for the same scope, Drift shows a clear blocked state and does not attempt generation. +- **error**: If drift generation fails for a comparison, Drift shows a clear error state with safe information and reference identifiers to the recorded run. -- Drift/Audit UI should resolve labels via Inventory + Foundations (047) + Groups Cache (051) where applicable. -- No render-time Graph calls (Graph only in background sync/jobs, never in UI render). +### Default views -## Success Criteria +- Default Drift summary and default finding lists show **new** findings only. +- Acknowledged findings are accessible via an explicit filter. -- SC1: Admins can identify drift across supported types (Policies + Assignments) in under 3 minutes. -- SC2: Drift results are consistent across repeated generation for the same baseline. +### Run tracking (status, errors, idempotency) -## Out of Scope +- Drift generation status and errors must be recorded in a **persisted run record** so that progress/failure survives refresh and can be inspected later. +- Re-opening Drift for the same comparison must be idempotent (it should not create duplicate work for the same comparison). -- Automatic revert/promotion. -- Rule engine in MVP (Audit later), but the data model is prepared via `rule_id` / `control_id` / `expected_value`. +### Determinism and stable identity -## Future Work (non-MVP) +- For the same scope + baseline + current, Drift must produce the same set of findings. +- Each finding must have a stable identifier (“fingerprint”) so triage actions can reliably reference the same drift item within a comparison. -- Security Suite Audits: add rule-based generators that write Findings (no new Finding model). -- Cross-tenant Compare: may write Findings (`finding_type=compare`) or emit a compatible format that can be stored as Findings. +## Acceptance Criteria -## Related Specs - -- Program: `specs/039-inventory-program/spec.md` -- Core: `specs/040-inventory-core/spec.md` -- Compare: `specs/043-cross-tenant-compare-and-promotion/spec.md` +- With two successful snapshots for the same scope, Drift shows a summary of **added/removed/modified** items for that comparison. +- With fewer than two successful snapshots for the same scope, Drift shows **blocked** and does not start generation. +- If generation fails, Drift shows **error** and provides reference identifiers to the persisted run record. +- Default views exclude acknowledged findings, and acknowledged findings remain available via filter. +- Acknowledging a finding records who/when acknowledged and hides it from “new” views. +- Re-running generation for the same comparison does not create duplicate work and produces consistent results. diff --git a/specs/044-drift-mvp/tasks.md b/specs/044-drift-mvp/tasks.md index 481f990..f6dd823 100644 --- a/specs/044-drift-mvp/tasks.md +++ b/specs/044-drift-mvp/tasks.md @@ -25,10 +25,10 @@ ## Phase 1: Setup (Shared Infrastructure) **Purpose**: Project wiring for Drift MVP. -- [ ] T001 Create/update constitution gate checklist in `specs/044-drift-mvp/checklists/requirements.md` -- [ ] T002 Confirm spec/plan artifacts are current in `specs/044-drift-mvp/{plan.md,spec.md,research.md,data-model.md,quickstart.md,contracts/admin-findings.openapi.yaml}` -- [ ] T003 Add Drift landing page shell in `app/Filament/Pages/DriftLanding.php` -- [ ] T004 [P] Add Finding resource shells in `app/Filament/Resources/FindingResource.php` and `app/Filament/Resources/FindingResource/Pages/` +- [x] T001 Create/update constitution gate checklist in `specs/044-drift-mvp/checklists/requirements.md` +- [x] T002 Confirm spec/plan artifacts are current in `specs/044-drift-mvp/{plan.md,spec.md,research.md,data-model.md,quickstart.md,contracts/admin-findings.openapi.yaml}` +- [x] T003 Add Drift landing page shell in `app/Filament/Pages/DriftLanding.php` +- [x] T004 [P] Add Finding resource shells in `app/Filament/Resources/FindingResource.php` and `app/Filament/Resources/FindingResource/Pages/` --- @@ -38,17 +38,17 @@ ## Phase 2: Foundational (Blocking Prerequisites) **Checkpoint**: DB schema exists, tenant scoping enforced, and tests can create Finding rows. -- [ ] T005 Create `findings` migration in `database/migrations/*_create_findings_table.php` with Finding fields aligned to `specs/044-drift-mvp/spec.md`: +- [x] T005 Create `findings` migration in `database/migrations/*_create_findings_table.php` with Finding fields aligned to `specs/044-drift-mvp/spec.md`: (tenant_id, finding_type, scope_key, baseline_run_id, current_run_id, subject_type, subject_external_id, severity, status, fingerprint unique, evidence_jsonb, acknowledged_at, acknowledged_by_user_id) -- [ ] T006 Create `Finding` model in `app/Models/Finding.php` (casts for `evidence_jsonb`, enums/constants for `finding_type`/`severity`/`status`, `acknowledged_at` handling) -- [ ] T007 [P] Add `FindingFactory` in `database/factories/FindingFactory.php` -- [ ] T008 Ensure `InventorySyncRunFactory` exists (or add it) in `database/factories/InventorySyncRunFactory.php` -- [ ] T009 Add authorization policy in `app/Policies/FindingPolicy.php` and wire it in `app/Providers/AuthServiceProvider.php` (or project equivalent) -- [ ] T010 Add Drift permissions in `config/intune_permissions.php` (view + acknowledge) and wire them into Filament navigation/actions -- [ ] T011 Enforce tenant scoping for Finding queries in `app/Models/Finding.php` and/or `app/Filament/Resources/FindingResource.php` -- [ ] T012 Implement fingerprint helper in `app/Services/Drift/DriftHasher.php` (sha256 per spec) -- [ ] T013 Pin `scope_key = InventorySyncRun.selection_hash` in `app/Services/Drift/DriftScopeKey.php` (single canonical definition) -- [ ] T014 Implement evidence allowlist builder in `app/Services/Drift/DriftEvidence.php` (new file) +- [x] T006 Create `Finding` model in `app/Models/Finding.php` (casts for `evidence_jsonb`, enums/constants for `finding_type`/`severity`/`status`, `acknowledged_at` handling) +- [x] T007 [P] Add `FindingFactory` in `database/factories/FindingFactory.php` +- [x] T008 Ensure `InventorySyncRunFactory` exists (or add it) in `database/factories/InventorySyncRunFactory.php` +- [x] T009 Add authorization policy in `app/Policies/FindingPolicy.php` and wire it in `app/Providers/AuthServiceProvider.php` (or project equivalent) +- [x] T010 Add Drift permissions in `config/intune_permissions.php` (view + acknowledge) and wire them into Filament navigation/actions +- [x] T011 Enforce tenant scoping for Finding queries in `app/Models/Finding.php` and/or `app/Filament/Resources/FindingResource.php` +- [x] T012 Implement fingerprint helper in `app/Services/Drift/DriftHasher.php` (sha256 per spec) +- [x] T013 Pin `scope_key = InventorySyncRun.selection_hash` in `app/Services/Drift/DriftScopeKey.php` (single canonical definition) +- [x] T014 Implement evidence allowlist builder in `app/Services/Drift/DriftEvidence.php` (new file) --- @@ -60,18 +60,18 @@ ## Phase 3: User Story 1 - View drift summary (Priority: P1) MVP ### Tests (write first) -- [ ] T015 [P] [US1] Baseline selection tests in `tests/Feature/Drift/DriftBaselineSelectionTest.php` -- [ ] T016 [P] [US1] Generation dispatch tests in `tests/Feature/Drift/DriftGenerationDispatchTest.php` -- [ ] T017 [P] [US1] Tenant isolation tests in `tests/Feature/Drift/DriftTenantIsolationTest.php` -- [ ] T018 [P] [US1] Assignment drift detection test (targets + intent changes per FR2b) in `tests/Feature/Drift/DriftAssignmentDriftDetectionTest.php` +- [x] T015 [P] [US1] Baseline selection tests in `tests/Feature/Drift/DriftBaselineSelectionTest.php` +- [x] T016 [P] [US1] Generation dispatch tests in `tests/Feature/Drift/DriftGenerationDispatchTest.php` +- [x] T017 [P] [US1] Tenant isolation tests in `tests/Feature/Drift/DriftTenantIsolationTest.php` +- [x] T018 [P] [US1] Assignment drift detection test (targets + intent changes per FR2b) in `tests/Feature/Drift/DriftAssignmentDriftDetectionTest.php` ### Implementation -- [ ] T019 [US1] Implement run selection service in `app/Services/Drift/DriftRunSelector.php` -- [ ] T020 [US1] Implement generator job in `app/Jobs/GenerateDriftFindingsJob.php` (dedupe/lock by tenant+scope+baseline+current) -- [ ] T021 [US1] Implement generator service in `app/Services/Drift/DriftFindingGenerator.php` (idempotent) -- [ ] T022 [US1] Implement landing behavior (dispatch + status UI) in `app/Filament/Pages/DriftLanding.php` -- [ ] T023 [US1] Implement summary queries/widgets in `app/Filament/Pages/DriftLanding.php` +- [x] T019 [US1] Implement run selection service in `app/Services/Drift/DriftRunSelector.php` +- [x] T021 [US1] Implement generator service in `app/Services/Drift/DriftFindingGenerator.php` (idempotent) +- [x] T020 [US1] Implement generator job in `app/Jobs/GenerateDriftFindingsJob.php` (dedupe/lock by tenant+scope+baseline+current) +- [x] T022 [US1] Implement landing behavior (dispatch + status UI) in `app/Filament/Pages/DriftLanding.php` +- [x] T023 [US1] Implement summary queries/widgets in `app/Filament/Pages/DriftLanding.php` --- @@ -83,14 +83,17 @@ ## Phase 4: User Story 2 - Drill into a drift finding (Priority: P2) ### Tests (write first) -- [ ] T024 [P] [US2] Finding detail test in `tests/Feature/Drift/DriftFindingDetailTest.php` -- [ ] T025 [P] [US2] Evidence minimization test in `tests/Feature/Drift/DriftEvidenceMinimizationTest.php` +- [x] T024 [P] [US2] Finding detail test in `tests/Feature/Drift/DriftFindingDetailTest.php` +- [x] T025 [P] [US2] Evidence minimization test in `tests/Feature/Drift/DriftEvidenceMinimizationTest.php` +- [x] T041 [P] [US2] Finding detail shows normalized settings diff (DB-only) in `tests/Feature/Drift/DriftFindingDetailShowsSettingsDiffTest.php` +- [x] T042 [P] [US2] Finding detail shows assignments diff + cached group labels (DB-only) in `tests/Feature/Drift/DriftFindingDetailShowsAssignmentsDiffTest.php` ### Implementation -- [ ] T026 [US2] Implement list UI in `app/Filament/Resources/FindingResource.php` (filters: status, scope_key, run) -- [ ] T027 [US2] Implement detail UI in `app/Filament/Resources/FindingResource/Pages/ViewFinding.php` -- [ ] T028 [US2] Implement DB-only name resolution in `app/Filament/Resources/FindingResource.php` (inventory/foundations caches) +- [x] T026 [US2] Implement list UI in `app/Filament/Resources/FindingResource.php` (filters: status, scope_key, run) +- [x] T027 [US2] Implement detail UI in `app/Filament/Resources/FindingResource/Pages/ViewFinding.php` +- [x] T028 [US2] Implement DB-only name resolution in `app/Filament/Resources/FindingResource.php` (inventory/foundations caches) +- [x] T043 [US2] Add real diffs to Finding detail (settings + assignments) in `app/Filament/Resources/FindingResource.php`, `app/Services/Drift/*`, and `resources/views/filament/infolists/entries/assignments-diff.blade.php` --- @@ -102,25 +105,57 @@ ## Phase 5: User Story 3 - Acknowledge/triage (Priority: P3) ### Tests (write first) -- [ ] T029 [P] [US3] Acknowledge action test in `tests/Feature/Drift/DriftAcknowledgeTest.php` -- [ ] T030 [P] [US3] Acknowledge authorization test in `tests/Feature/Drift/DriftAcknowledgeAuthorizationTest.php` +- [x] T029 [P] [US3] Acknowledge action test in `tests/Feature/Drift/DriftAcknowledgeTest.php` +- [x] T030 [P] [US3] Acknowledge authorization test in `tests/Feature/Drift/DriftAcknowledgeAuthorizationTest.php` ### Implementation -- [ ] T031 [US3] Add acknowledge actions in `app/Filament/Resources/FindingResource.php` -- [ ] T032 [US3] Implement `acknowledge()` domain method in `app/Models/Finding.php` -- [ ] T033 [US3] Ensure Drift summary excludes acknowledged by default in `app/Filament/Pages/DriftLanding.php` +- [x] T031 [US3] Add acknowledge actions in `app/Filament/Resources/FindingResource.php` +- [x] T032 [US3] Implement `acknowledge()` domain method in `app/Models/Finding.php` +- [x] T033 [US3] Ensure Drift summary excludes acknowledged by default in `app/Filament/Pages/DriftLanding.php` + +### Bulk triage (post-MVP UX) + +**Goal**: Admin can acknowledge many findings safely and quickly. + +### Tests (write first) + +- [x] T048 [P] [US3] Bulk acknowledge selected test in `tests/Feature/Drift/DriftBulkAcknowledgeTest.php` +- [x] T049 [P] [US3] Acknowledge all matching current filters test in `tests/Feature/Drift/DriftBulkAcknowledgeAllMatchingTest.php` +- [x] T050 [P] [US3] Acknowledge all matching requires type-to-confirm when >100 in `tests/Feature/Drift/DriftBulkAcknowledgeAllMatchingConfirmationTest.php` +- [x] T051 [P] [US3] Bulk acknowledge authorization test in `tests/Feature/Drift/DriftBulkAcknowledgeAuthorizationTest.php` + +### Implementation + +- [x] T052 [US3] Add bulk triage actions to Findings list in `app/Filament/Resources/FindingResource.php` and `app/Filament/Resources/FindingResource/Pages/ListFindings.php` --- ## Phase 6: Polish & Cross-Cutting Concerns -- [ ] T034 Add DB indexes in `database/migrations/*_create_findings_table.php` (tenant_id+status, tenant_id+scope_key, tenant_id+baseline_run_id, tenant_id+current_run_id) -- [ ] T035 [P] Add determinism test in `tests/Feature/Drift/DriftGenerationDeterminismTest.php` (same baseline/current => same fingerprints) -- [ ] T036 Add job observability logs in `app/Jobs/GenerateDriftFindingsJob.php` (no secrets) -- [ ] T037 Add idempotency/upsert strategy in `app/Services/Drift/DriftFindingGenerator.php` -- [ ] T038 Ensure volatile fields excluded from hashing in `app/Services/Drift/DriftHasher.php` and cover in `tests/Feature/Drift/DriftHasherTest.php` -- [ ] T039 Validate and update `specs/044-drift-mvp/quickstart.md` after implementation +- [x] T034 Add DB indexes in `database/migrations/*_create_findings_table.php` (tenant_id+status, tenant_id+scope_key, tenant_id+baseline_run_id, tenant_id+current_run_id) +- [x] T035 [P] Add determinism test in `tests/Feature/Drift/DriftGenerationDeterminismTest.php` (same baseline/current => same fingerprints) +- [x] T036 Add job observability logs in `app/Jobs/GenerateDriftFindingsJob.php` (no secrets) +- [x] T037 Add idempotency/upsert strategy in `app/Services/Drift/DriftFindingGenerator.php` +- [x] T038 Ensure volatile fields excluded from hashing in `app/Services/Drift/DriftHasher.php` and cover in `tests/Feature/Drift/DriftHasherTest.php` +- [x] T039 Validate and update `specs/044-drift-mvp/quickstart.md` after implementation +- [x] T040 Fix drift evidence summary shape in `app/Services/Drift/DriftFindingGenerator.php` and cover in `tests/Feature/Drift/DriftPolicySnapshotDriftDetectionTest.php` (snapshot_hash vs assignments_hash) + +--- + +## Phase 7: Scope Tags Drift (Post-MVP Fix) + +**Goal**: Detect and display drift caused by `roleScopeTagIds` changes on a policy version. + +### Tests (write first) + +- [x] T044 [P] [US1] Scope-tag drift detection test in `tests/Feature/Drift/DriftScopeTagDriftDetectionTest.php` +- [x] T045 [P] [US2] Finding detail shows scope-tags diff (DB-only) in `tests/Feature/Drift/DriftFindingDetailShowsScopeTagsDiffTest.php` + +### Implementation + +- [x] T046 [US1] Implement scope-tag drift detection in `app/Services/Drift/DriftFindingGenerator.php` (deterministic hashing; kind=`policy_scope_tags`) +- [x] T047 [US2] Implement scope-tags diff builder + UI in `app/Services/Drift/DriftFindingDiffBuilder.php`, `app/Filament/Resources/FindingResource.php`, and `resources/views/filament/infolists/entries/scope-tags-diff.blade.php` --- diff --git a/tests/Feature/Drift/DriftAcknowledgeAuthorizationTest.php b/tests/Feature/Drift/DriftAcknowledgeAuthorizationTest.php new file mode 100644 index 0000000..c687664 --- /dev/null +++ b/tests/Feature/Drift/DriftAcknowledgeAuthorizationTest.php @@ -0,0 +1,31 @@ +actingAs($user); + Filament::setTenant($tenant, true); + + $finding = Finding::factory()->for($tenant)->create([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'status' => Finding::STATUS_NEW, + ]); + + $thrown = null; + + try { + Livewire::test(ListFindings::class) + ->callTableAction('acknowledge', $finding); + } catch (Throwable $exception) { + $thrown = $exception; + } + + expect($thrown)->not->toBeNull(); + + $finding->refresh(); + expect($finding->status)->toBe(Finding::STATUS_NEW); +}); diff --git a/tests/Feature/Drift/DriftAcknowledgeTest.php b/tests/Feature/Drift/DriftAcknowledgeTest.php new file mode 100644 index 0000000..05f8784 --- /dev/null +++ b/tests/Feature/Drift/DriftAcknowledgeTest.php @@ -0,0 +1,28 @@ +actingAs($user); + Filament::setTenant($tenant, true); + + $finding = Finding::factory()->for($tenant)->create([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'status' => Finding::STATUS_NEW, + 'acknowledged_at' => null, + 'acknowledged_by_user_id' => null, + ]); + + Livewire::test(ListFindings::class) + ->callTableAction('acknowledge', $finding); + + $finding->refresh(); + + expect($finding->status)->toBe(Finding::STATUS_ACKNOWLEDGED); + expect($finding->acknowledged_at)->not->toBeNull(); + expect((int) $finding->acknowledged_by_user_id)->toBe((int) $user->getKey()); +}); diff --git a/tests/Feature/Drift/DriftAssignmentDriftDetectionTest.php b/tests/Feature/Drift/DriftAssignmentDriftDetectionTest.php index 5a186ea..dee9bc6 100644 --- a/tests/Feature/Drift/DriftAssignmentDriftDetectionTest.php +++ b/tests/Feature/Drift/DriftAssignmentDriftDetectionTest.php @@ -52,7 +52,6 @@ 'policy_type' => $policy->policy_type, 'captured_at' => $baseline->finished_at->copy()->subMinute(), 'assignments' => $baselineAssignments, - 'assignments_hash' => hash('sha256', json_encode($baselineAssignments)), ]); PolicyVersion::factory()->for($tenant)->for($policy)->create([ @@ -60,7 +59,6 @@ 'policy_type' => $policy->policy_type, 'captured_at' => $current->finished_at->copy()->subMinute(), 'assignments' => $currentAssignments, - 'assignments_hash' => hash('sha256', json_encode($currentAssignments)), ]); $generator = app(DriftFindingGenerator::class); diff --git a/tests/Feature/Drift/DriftBulkAcknowledgeAllMatchingConfirmationTest.php b/tests/Feature/Drift/DriftBulkAcknowledgeAllMatchingConfirmationTest.php new file mode 100644 index 0000000..936ff40 --- /dev/null +++ b/tests/Feature/Drift/DriftBulkAcknowledgeAllMatchingConfirmationTest.php @@ -0,0 +1,34 @@ +actingAs($user); + Filament::setTenant($tenant, true); + + $findings = Finding::factory() + ->count(101) + ->for($tenant) + ->create([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'status' => Finding::STATUS_NEW, + ]); + + Livewire::test(ListFindings::class) + ->mountAction('acknowledge_all_matching') + ->callMountedAction(); + + $findings->each(fn (Finding $finding) => expect($finding->refresh()->status)->toBe(Finding::STATUS_NEW)); + + Livewire::test(ListFindings::class) + ->mountAction('acknowledge_all_matching') + ->setActionData(['confirmation' => 'ACKNOWLEDGE']) + ->callMountedAction() + ->assertHasNoActionErrors(); + + $findings->each(fn (Finding $finding) => expect($finding->refresh()->status)->toBe(Finding::STATUS_ACKNOWLEDGED)); +}); diff --git a/tests/Feature/Drift/DriftBulkAcknowledgeAllMatchingTest.php b/tests/Feature/Drift/DriftBulkAcknowledgeAllMatchingTest.php new file mode 100644 index 0000000..b283d35 --- /dev/null +++ b/tests/Feature/Drift/DriftBulkAcknowledgeAllMatchingTest.php @@ -0,0 +1,51 @@ +actingAs($user); + Filament::setTenant($tenant, true); + + $scopeA = 'scope-a'; + $scopeB = 'scope-b'; + + $matching = Finding::factory() + ->count(2) + ->for($tenant) + ->create([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'status' => Finding::STATUS_NEW, + 'scope_key' => $scopeA, + ]); + + $nonMatching = Finding::factory() + ->for($tenant) + ->create([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'status' => Finding::STATUS_NEW, + 'scope_key' => $scopeB, + ]); + + Livewire::test(ListFindings::class) + ->set('tableFilters', [ + 'status' => ['value' => Finding::STATUS_NEW], + 'finding_type' => ['value' => Finding::FINDING_TYPE_DRIFT], + 'scope_key' => ['scope_key' => $scopeA], + ]) + ->callAction('acknowledge_all_matching'); + + $matching->each(function (Finding $finding) use ($user): void { + $finding->refresh(); + + expect($finding->status)->toBe(Finding::STATUS_ACKNOWLEDGED); + expect((int) $finding->acknowledged_by_user_id)->toBe((int) $user->getKey()); + }); + + $nonMatching->refresh(); + expect($nonMatching->status)->toBe(Finding::STATUS_NEW); + expect($nonMatching->acknowledged_at)->toBeNull(); +}); diff --git a/tests/Feature/Drift/DriftBulkAcknowledgeAuthorizationTest.php b/tests/Feature/Drift/DriftBulkAcknowledgeAuthorizationTest.php new file mode 100644 index 0000000..4903049 --- /dev/null +++ b/tests/Feature/Drift/DriftBulkAcknowledgeAuthorizationTest.php @@ -0,0 +1,60 @@ +actingAs($user); + Filament::setTenant($tenant, true); + + $findings = Finding::factory() + ->count(2) + ->for($tenant) + ->create([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'status' => Finding::STATUS_NEW, + ]); + + $thrown = null; + + try { + Livewire::test(ListFindings::class) + ->callTableBulkAction('acknowledge_selected', $findings); + } catch (Throwable $exception) { + $thrown = $exception; + } + + expect($thrown)->not->toBeNull(); + + $findings->each(fn (Finding $finding) => expect($finding->refresh()->status)->toBe(Finding::STATUS_NEW)); +}); + +test('readonly users cannot acknowledge all matching findings', function () { + [$user, $tenant] = createUserWithTenant(role: 'readonly'); + $this->actingAs($user); + Filament::setTenant($tenant, true); + + $findings = Finding::factory() + ->count(2) + ->for($tenant) + ->create([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'status' => Finding::STATUS_NEW, + ]); + + $thrown = null; + + try { + Livewire::test(ListFindings::class) + ->callAction('acknowledge_all_matching'); + } catch (Throwable $exception) { + $thrown = $exception; + } + + expect($thrown)->not->toBeNull(); + + $findings->each(fn (Finding $finding) => expect($finding->refresh()->status)->toBe(Finding::STATUS_NEW)); +}); diff --git a/tests/Feature/Drift/DriftBulkAcknowledgeTest.php b/tests/Feature/Drift/DriftBulkAcknowledgeTest.php new file mode 100644 index 0000000..ea613bf --- /dev/null +++ b/tests/Feature/Drift/DriftBulkAcknowledgeTest.php @@ -0,0 +1,34 @@ +actingAs($user); + Filament::setTenant($tenant, true); + + $findings = Finding::factory() + ->count(3) + ->for($tenant) + ->create([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'status' => Finding::STATUS_NEW, + 'acknowledged_at' => null, + 'acknowledged_by_user_id' => null, + ]); + + Livewire::test(ListFindings::class) + ->callTableBulkAction('acknowledge_selected', $findings) + ->assertHasNoTableBulkActionErrors(); + + $findings->each(function (Finding $finding) use ($user): void { + $finding->refresh(); + + expect($finding->status)->toBe(Finding::STATUS_ACKNOWLEDGED); + expect($finding->acknowledged_at)->not->toBeNull(); + expect((int) $finding->acknowledged_by_user_id)->toBe((int) $user->getKey()); + }); +}); diff --git a/tests/Feature/Drift/DriftCompletedRunWithZeroFindingsTest.php b/tests/Feature/Drift/DriftCompletedRunWithZeroFindingsTest.php new file mode 100644 index 0000000..f8832aa --- /dev/null +++ b/tests/Feature/Drift/DriftCompletedRunWithZeroFindingsTest.php @@ -0,0 +1,71 @@ +actingAs($user); + Filament::setTenant($tenant, true); + + $scopeKey = hash('sha256', 'scope-zero-findings'); + + $baseline = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['settingsCatalogPolicy']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['settingsCatalogPolicy']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + $idempotencyKey = RunIdempotency::buildKey( + tenantId: (int) $tenant->getKey(), + operationType: 'drift.generate', + targetId: $scopeKey, + context: [ + 'scope_key' => $scopeKey, + 'baseline_run_id' => (int) $baseline->getKey(), + 'current_run_id' => (int) $current->getKey(), + ], + ); + + BulkOperationRun::factory()->for($tenant)->for($user)->create([ + 'resource' => 'drift', + 'action' => 'generate', + 'status' => 'completed', + 'idempotency_key' => $idempotencyKey, + 'item_ids' => [$scopeKey], + 'total_items' => 1, + 'processed_items' => 1, + 'succeeded' => 1, + 'failed' => 0, + 'skipped' => 0, + ]); + + Livewire::test(DriftLanding::class) + ->assertSet('state', 'ready') + ->assertSet('scopeKey', $scopeKey); + + Queue::assertNothingPushed(); + + expect(BulkOperationRun::query() + ->where('tenant_id', $tenant->getKey()) + ->where('idempotency_key', $idempotencyKey) + ->count())->toBe(1); + + Queue::assertNotPushed(GenerateDriftFindingsJob::class); +}); diff --git a/tests/Feature/Drift/DriftEvidenceMinimizationTest.php b/tests/Feature/Drift/DriftEvidenceMinimizationTest.php new file mode 100644 index 0000000..82bf47b --- /dev/null +++ b/tests/Feature/Drift/DriftEvidenceMinimizationTest.php @@ -0,0 +1,24 @@ + 'modified', + 'summary' => ['changed_fields' => ['assignments_hash']], + 'baseline' => ['hash' => 'a'], + 'current' => ['hash' => 'b'], + 'diff' => ['a' => 'b'], + 'notes' => 'ok', + 'access_token' => 'should-not-leak', + 'client_secret' => 'should-not-leak', + 'raw_payload' => ['big' => 'blob'], + ]; + + $safe = app(DriftEvidence::class)->sanitize($payload); + + expect($safe)->toHaveKeys(['change_type', 'summary', 'baseline', 'current', 'diff', 'notes']); + expect($safe)->not->toHaveKey('access_token'); + expect($safe)->not->toHaveKey('client_secret'); + expect($safe)->not->toHaveKey('raw_payload'); +}); diff --git a/tests/Feature/Drift/DriftFindingDetailShowsAssignmentsDiffTest.php b/tests/Feature/Drift/DriftFindingDetailShowsAssignmentsDiffTest.php new file mode 100644 index 0000000..605617c --- /dev/null +++ b/tests/Feature/Drift/DriftFindingDetailShowsAssignmentsDiffTest.php @@ -0,0 +1,151 @@ +for($tenant)->create([ + 'selection_hash' => hash('sha256', 'scope-assignments-diff'), + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $baseline->selection_hash, + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + $policy = Policy::factory()->for($tenant)->create([ + 'external_id' => 'policy-456', + 'policy_type' => 'deviceConfiguration', + 'platform' => 'windows10', + ]); + + $group1 = '76b787af-cae9-4a8e-89e9-b8cc67f81779'; + $group2 = '6b0bc3d7-91f3-4e4b-8181-8236d908d2dd'; + $group3 = 'cbd8d685-0d95-4de0-8fce-140a5cad8ddc'; + + EntraGroup::factory()->for($tenant)->create([ + 'entra_id' => strtolower($group1), + 'display_name' => 'Group One', + ]); + + EntraGroup::factory()->for($tenant)->create([ + 'entra_id' => strtolower($group3), + 'display_name' => 'Group Three', + ]); + + $baselineVersion = PolicyVersion::factory()->for($tenant)->create([ + 'policy_id' => $policy->getKey(), + 'version_number' => 1, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $baseline->finished_at->copy()->subHour(), + 'assignments' => [ + [ + 'intent' => 'apply', + 'target' => [ + '@odata.type' => '#microsoft.graph.groupAssignmentTarget', + 'groupId' => $group1, + 'deviceAndAppManagementAssignmentFilterId' => null, + 'deviceAndAppManagementAssignmentFilterType' => 'none', + ], + ], + [ + 'target' => [ + '@odata.type' => '#microsoft.graph.exclusionGroupAssignmentTarget', + 'groupId' => $group2, + 'deviceAndAppManagementAssignmentFilterId' => null, + 'deviceAndAppManagementAssignmentFilterType' => 'none', + ], + ], + ], + ]); + + $currentVersion = PolicyVersion::factory()->for($tenant)->create([ + 'policy_id' => $policy->getKey(), + 'version_number' => 2, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $current->finished_at->copy()->subHour(), + 'assignments' => [ + [ + 'intent' => 'apply', + 'target' => [ + '@odata.type' => '#microsoft.graph.groupAssignmentTarget', + 'groupId' => $group1, + 'deviceAndAppManagementAssignmentFilterId' => '62fb77d0-8f85-4ba0-a1c7-fd71d418521d', + 'deviceAndAppManagementAssignmentFilterType' => 'include', + ], + ], + [ + 'intent' => 'apply', + 'target' => [ + '@odata.type' => '#microsoft.graph.groupAssignmentTarget', + 'groupId' => $group3, + 'deviceAndAppManagementAssignmentFilterId' => null, + 'deviceAndAppManagementAssignmentFilterType' => 'none', + ], + ], + ], + ]); + + $finding = Finding::factory()->for($tenant)->create([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'scope_key' => (string) $current->selection_hash, + 'baseline_run_id' => $baseline->getKey(), + 'current_run_id' => $current->getKey(), + 'subject_type' => 'assignment', + 'subject_external_id' => $policy->external_id, + 'evidence_jsonb' => [ + 'change_type' => 'modified', + 'summary' => [ + 'kind' => 'policy_assignments', + 'changed_fields' => ['assignments_hash'], + ], + 'baseline' => [ + 'policy_id' => $policy->external_id, + 'policy_version_id' => $baselineVersion->getKey(), + 'assignments_hash' => 'baseline-assignments-hash', + ], + 'current' => [ + 'policy_id' => $policy->external_id, + 'policy_version_id' => $currentVersion->getKey(), + 'assignments_hash' => 'current-assignments-hash', + ], + ], + ]); + + InventoryItem::factory()->for($tenant)->create([ + 'external_id' => $finding->subject_external_id, + 'display_name' => 'My Policy 456', + ]); + + $expectedGroup1 = EntraGroupLabelResolver::formatLabel('Group One', $group1); + $expectedGroup2 = EntraGroupLabelResolver::formatLabel(null, $group2); + $expectedGroup3 = EntraGroupLabelResolver::formatLabel('Group Three', $group3); + + $this->actingAs($user) + ->get(FindingResource::getUrl('view', ['record' => $finding], tenant: $tenant)) + ->assertOk() + ->assertSee('Assignments diff') + ->assertSee('1 added') + ->assertSee('1 removed') + ->assertSee('1 changed') + ->assertSee($expectedGroup1) + ->assertSee($expectedGroup2) + ->assertSee($expectedGroup3) + ->assertSee('include') + ->assertSee('none'); +}); diff --git a/tests/Feature/Drift/DriftFindingDetailShowsScopeTagsDiffTest.php b/tests/Feature/Drift/DriftFindingDetailShowsScopeTagsDiffTest.php new file mode 100644 index 0000000..3d905d3 --- /dev/null +++ b/tests/Feature/Drift/DriftFindingDetailShowsScopeTagsDiffTest.php @@ -0,0 +1,100 @@ +for($tenant)->create([ + 'selection_hash' => hash('sha256', 'scope-scope-tags-diff'), + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $baseline->selection_hash, + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + $policy = Policy::factory()->for($tenant)->create([ + 'external_id' => 'policy-scope-tags-1', + 'policy_type' => 'deviceConfiguration', + 'platform' => 'windows10', + ]); + + $baselineVersion = PolicyVersion::factory()->for($tenant)->create([ + 'policy_id' => $policy->getKey(), + 'version_number' => 17, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $baseline->finished_at->copy()->subHour(), + 'snapshot' => ['customSettingFoo' => 'Same value'], + 'assignments' => [], + 'scope_tags' => [ + 'ids' => ['0'], + 'names' => ['Default'], + ], + ]); + + $currentVersion = PolicyVersion::factory()->for($tenant)->create([ + 'policy_id' => $policy->getKey(), + 'version_number' => 18, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $current->finished_at->copy()->subHour(), + 'snapshot' => ['customSettingFoo' => 'Same value'], + 'assignments' => [], + 'scope_tags' => [ + 'ids' => ['0', 'a1b2c3'], + 'names' => ['Default', 'Verbund-1'], + ], + ]); + + $finding = Finding::factory()->for($tenant)->create([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'scope_key' => (string) $current->selection_hash, + 'baseline_run_id' => $baseline->getKey(), + 'current_run_id' => $current->getKey(), + 'subject_type' => 'scope_tag', + 'subject_external_id' => $policy->external_id, + 'evidence_jsonb' => [ + 'change_type' => 'modified', + 'summary' => [ + 'kind' => 'policy_scope_tags', + 'changed_fields' => ['scope_tags_hash'], + ], + 'baseline' => [ + 'policy_id' => $policy->external_id, + 'policy_version_id' => $baselineVersion->getKey(), + 'scope_tags_hash' => 'baseline-scope-tags-hash', + ], + 'current' => [ + 'policy_id' => $policy->external_id, + 'policy_version_id' => $currentVersion->getKey(), + 'scope_tags_hash' => 'current-scope-tags-hash', + ], + ], + ]); + + InventoryItem::factory()->for($tenant)->create([ + 'external_id' => $finding->subject_external_id, + 'display_name' => 'My Policy Scope Tags', + ]); + + $this->actingAs($user) + ->get(FindingResource::getUrl('view', ['record' => $finding], tenant: $tenant)) + ->assertOk() + ->assertSee('Scope tags diff') + ->assertSee('1 added') + ->assertSee('0 removed') + ->assertSee('Verbund-1') + ->assertSee('Default'); +}); diff --git a/tests/Feature/Drift/DriftFindingDetailShowsSettingsDiffTest.php b/tests/Feature/Drift/DriftFindingDetailShowsSettingsDiffTest.php new file mode 100644 index 0000000..ffc4136 --- /dev/null +++ b/tests/Feature/Drift/DriftFindingDetailShowsSettingsDiffTest.php @@ -0,0 +1,100 @@ +for($tenant)->create([ + 'selection_hash' => hash('sha256', 'scope-settings-diff'), + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $baseline->selection_hash, + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + $policy = Policy::factory()->for($tenant)->create([ + 'external_id' => 'policy-123', + 'policy_type' => 'deviceConfiguration', + 'platform' => 'windows10', + ]); + + $baselineVersion = PolicyVersion::factory()->for($tenant)->create([ + 'policy_id' => $policy->getKey(), + 'version_number' => 1, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $baseline->finished_at->copy()->subHour(), + 'snapshot' => [ + 'displayName' => 'My Policy', + 'description' => 'Old description', + 'customSettingFoo' => 'Old value', + ], + ]); + + $currentVersion = PolicyVersion::factory()->for($tenant)->create([ + 'policy_id' => $policy->getKey(), + 'version_number' => 2, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $current->finished_at->copy()->subHour(), + 'snapshot' => [ + 'displayName' => 'My Policy', + 'description' => 'New description', + 'customSettingFoo' => 'New value', + ], + ]); + + $finding = Finding::factory()->for($tenant)->create([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'scope_key' => (string) $current->selection_hash, + 'baseline_run_id' => $baseline->getKey(), + 'current_run_id' => $current->getKey(), + 'subject_type' => 'policy', + 'subject_external_id' => $policy->external_id, + 'evidence_jsonb' => [ + 'change_type' => 'modified', + 'summary' => [ + 'kind' => 'policy_snapshot', + 'changed_fields' => ['snapshot_hash'], + ], + 'baseline' => [ + 'policy_id' => $policy->external_id, + 'policy_version_id' => $baselineVersion->getKey(), + 'snapshot_hash' => 'baseline-hash', + ], + 'current' => [ + 'policy_id' => $policy->external_id, + 'policy_version_id' => $currentVersion->getKey(), + 'snapshot_hash' => 'current-hash', + ], + ], + ]); + + InventoryItem::factory()->for($tenant)->create([ + 'external_id' => $finding->subject_external_id, + 'display_name' => 'My Policy 123', + ]); + + $this->actingAs($user) + ->get(FindingResource::getUrl('view', ['record' => $finding], tenant: $tenant)) + ->assertOk() + ->assertSee('Normalized diff') + ->assertSee('1 changed') + ->assertSee('Custom Setting Foo') + ->assertSee('From') + ->assertSee('To') + ->assertSee('Old value') + ->assertSee('New value'); +}); diff --git a/tests/Feature/Drift/DriftFindingDetailTest.php b/tests/Feature/Drift/DriftFindingDetailTest.php new file mode 100644 index 0000000..b216dc4 --- /dev/null +++ b/tests/Feature/Drift/DriftFindingDetailTest.php @@ -0,0 +1,48 @@ +for($tenant)->create([ + 'selection_hash' => hash('sha256', 'scope-detail'), + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $baseline->selection_hash, + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + $finding = Finding::factory()->for($tenant)->create([ + 'finding_type' => Finding::FINDING_TYPE_DRIFT, + 'scope_key' => (string) $current->selection_hash, + 'baseline_run_id' => $baseline->getKey(), + 'current_run_id' => $current->getKey(), + 'subject_type' => 'deviceConfiguration', + 'subject_external_id' => 'policy-123', + 'evidence_jsonb' => [ + 'change_type' => 'modified', + 'summary' => ['changed_fields' => ['assignments_hash']], + ], + ]); + + $inventoryItem = InventoryItem::factory()->for($tenant)->create([ + 'external_id' => $finding->subject_external_id, + 'display_name' => 'My Policy 123', + ]); + + $this->actingAs($user) + ->get(FindingResource::getUrl('view', ['record' => $finding], tenant: $tenant)) + ->assertOk() + ->assertSee($finding->fingerprint) + ->assertSee($inventoryItem->display_name); +}); diff --git a/tests/Feature/Drift/DriftGenerationDeterminismTest.php b/tests/Feature/Drift/DriftGenerationDeterminismTest.php new file mode 100644 index 0000000..c5ea172 --- /dev/null +++ b/tests/Feature/Drift/DriftGenerationDeterminismTest.php @@ -0,0 +1,76 @@ +for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['settingsCatalogPolicy']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['settingsCatalogPolicy']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + $policy = Policy::factory()->for($tenant)->create([ + 'policy_type' => 'settingsCatalogPolicy', + ]); + + $baselineAssignments = [ + ['target' => ['@odata.type' => '#microsoft.graph.groupAssignmentTarget', 'groupId' => 'group-a']], + ['target' => ['@odata.type' => '#microsoft.graph.groupAssignmentTarget', 'groupId' => 'group-b']], + ]; + + $currentAssignments = [ + ['target' => ['@odata.type' => '#microsoft.graph.groupAssignmentTarget', 'groupId' => 'group-c']], + ]; + + PolicyVersion::factory()->for($tenant)->for($policy)->create([ + 'version_number' => 1, + 'policy_type' => $policy->policy_type, + 'captured_at' => $baseline->finished_at->copy()->subMinute(), + 'assignments' => $baselineAssignments, + ]); + + PolicyVersion::factory()->for($tenant)->for($policy)->create([ + 'version_number' => 2, + 'policy_type' => $policy->policy_type, + 'captured_at' => $current->finished_at->copy()->subMinute(), + 'assignments' => $currentAssignments, + ]); + + $generator = app(DriftFindingGenerator::class); + + $created1 = $generator->generate($tenant, $baseline, $current, $scopeKey); + $fingerprints1 = Finding::query() + ->where('tenant_id', $tenant->getKey()) + ->pluck('fingerprint') + ->sort() + ->values() + ->all(); + + $created2 = $generator->generate($tenant, $baseline, $current, $scopeKey); + $fingerprints2 = Finding::query() + ->where('tenant_id', $tenant->getKey()) + ->pluck('fingerprint') + ->sort() + ->values() + ->all(); + + expect($created1)->toBeGreaterThan(0); + expect($created2)->toBe(0); + expect($fingerprints2)->toBe($fingerprints1); +}); diff --git a/tests/Feature/Drift/DriftGenerationDispatchTest.php b/tests/Feature/Drift/DriftGenerationDispatchTest.php index 3e54f9a..09fc6fa 100644 --- a/tests/Feature/Drift/DriftGenerationDispatchTest.php +++ b/tests/Feature/Drift/DriftGenerationDispatchTest.php @@ -2,7 +2,9 @@ use App\Filament\Pages\DriftLanding; use App\Jobs\GenerateDriftFindingsJob; +use App\Models\BulkOperationRun; use App\Models\InventorySyncRun; +use App\Support\RunIdempotency; use Filament\Facades\Filament; use Illuminate\Support\Facades\Queue; use Livewire\Livewire; @@ -30,15 +32,81 @@ Livewire::test(DriftLanding::class); - Queue::assertPushed(GenerateDriftFindingsJob::class, function (GenerateDriftFindingsJob $job) use ($tenant, $user, $baseline, $current, $scopeKey): bool { + $idempotencyKey = RunIdempotency::buildKey( + tenantId: (int) $tenant->getKey(), + operationType: 'drift.generate', + targetId: $scopeKey, + context: [ + 'scope_key' => $scopeKey, + 'baseline_run_id' => (int) $baseline->getKey(), + 'current_run_id' => (int) $current->getKey(), + ], + ); + + $bulkRun = BulkOperationRun::query() + ->where('tenant_id', $tenant->getKey()) + ->where('idempotency_key', $idempotencyKey) + ->latest('id') + ->first(); + + expect($bulkRun)->not->toBeNull(); + expect($bulkRun->resource)->toBe('drift'); + expect($bulkRun->action)->toBe('generate'); + expect($bulkRun->status)->toBe('pending'); + + Queue::assertPushed(GenerateDriftFindingsJob::class, function (GenerateDriftFindingsJob $job) use ($tenant, $user, $baseline, $current, $scopeKey, $bulkRun): bool { return $job->tenantId === (int) $tenant->getKey() && $job->userId === (int) $user->getKey() && $job->baselineRunId === (int) $baseline->getKey() && $job->currentRunId === (int) $current->getKey() - && $job->scopeKey === $scopeKey; + && $job->scopeKey === $scopeKey + && $job->bulkOperationRunId === (int) $bulkRun->getKey(); }); }); +test('opening Drift is idempotent while a run is pending', function () { + Queue::fake(); + + [$user, $tenant] = createUserWithTenant(role: 'manager'); + $this->actingAs($user); + Filament::setTenant($tenant, true); + + $scopeKey = hash('sha256', 'scope-idempotent'); + + $baseline = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + Livewire::test(DriftLanding::class); + Livewire::test(DriftLanding::class); + + Queue::assertPushed(GenerateDriftFindingsJob::class, 1); + + $idempotencyKey = RunIdempotency::buildKey( + tenantId: (int) $tenant->getKey(), + operationType: 'drift.generate', + targetId: $scopeKey, + context: [ + 'scope_key' => $scopeKey, + 'baseline_run_id' => (int) $baseline->getKey(), + 'current_run_id' => (int) $current->getKey(), + ], + ); + + expect(BulkOperationRun::query() + ->where('tenant_id', $tenant->getKey()) + ->where('idempotency_key', $idempotencyKey) + ->count())->toBe(1); +}); + test('opening Drift does not dispatch generation when fewer than two successful runs exist', function () { Queue::fake(); @@ -57,4 +125,5 @@ Livewire::test(DriftLanding::class); Queue::assertNothingPushed(); + expect(BulkOperationRun::query()->where('tenant_id', $tenant->getKey())->count())->toBe(0); }); diff --git a/tests/Feature/Drift/DriftHasherTest.php b/tests/Feature/Drift/DriftHasherTest.php new file mode 100644 index 0000000..2d39eeb --- /dev/null +++ b/tests/Feature/Drift/DriftHasherTest.php @@ -0,0 +1,39 @@ + 'abc', + 'createdDateTime' => '2020-01-01T00:00:00Z', + 'lastModifiedDateTime' => '2020-01-02T00:00:00Z', + 'target' => ['groupId' => 'group-a'], + ]; + + $b = [ + 'id' => 'abc', + 'createdDateTime' => '2025-01-01T00:00:00Z', + 'lastModifiedDateTime' => '2026-01-02T00:00:00Z', + 'target' => ['groupId' => 'group-a'], + ]; + + expect($hasher->hashNormalized($a))->toBe($hasher->hashNormalized($b)); +}); + +test('normalized hashing is order-insensitive for lists', function () { + $hasher = app(DriftHasher::class); + + $listA = [ + ['target' => ['groupId' => 'group-a']], + ['target' => ['groupId' => 'group-b']], + ]; + + $listB = [ + ['target' => ['groupId' => 'group-b']], + ['target' => ['groupId' => 'group-a']], + ]; + + expect($hasher->hashNormalized($listA))->toBe($hasher->hashNormalized($listB)); +}); diff --git a/tests/Feature/Drift/DriftLandingShowsComparisonInfoTest.php b/tests/Feature/Drift/DriftLandingShowsComparisonInfoTest.php new file mode 100644 index 0000000..3927771 --- /dev/null +++ b/tests/Feature/Drift/DriftLandingShowsComparisonInfoTest.php @@ -0,0 +1,33 @@ +actingAs($user); + Filament::setTenant($tenant, true); + + $scopeKey = hash('sha256', 'scope-landing-comparison-info'); + + $baseline = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + Livewire::test(DriftLanding::class) + ->assertSet('scopeKey', $scopeKey) + ->assertSet('baselineRunId', (int) $baseline->getKey()) + ->assertSet('currentRunId', (int) $current->getKey()) + ->assertSet('baselineFinishedAt', $baseline->finished_at->toDateTimeString()) + ->assertSet('currentFinishedAt', $current->finished_at->toDateTimeString()); +}); diff --git a/tests/Feature/Drift/DriftPolicySnapshotDriftDetectionTest.php b/tests/Feature/Drift/DriftPolicySnapshotDriftDetectionTest.php new file mode 100644 index 0000000..34ecbf3 --- /dev/null +++ b/tests/Feature/Drift/DriftPolicySnapshotDriftDetectionTest.php @@ -0,0 +1,73 @@ +for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['deviceConfiguration']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['deviceConfiguration']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + $policy = Policy::factory()->for($tenant)->create([ + 'policy_type' => 'deviceConfiguration', + 'platform' => 'windows10', + ]); + + PolicyVersion::factory()->for($tenant)->for($policy)->create([ + 'version_number' => 1, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $baseline->finished_at->copy()->subMinute(), + 'snapshot' => ['customSettingFoo' => 'Old value'], + 'assignments' => [], + ]); + + PolicyVersion::factory()->for($tenant)->for($policy)->create([ + 'version_number' => 2, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $current->finished_at->copy()->subMinute(), + 'snapshot' => ['customSettingFoo' => 'New value'], + 'assignments' => [], + ]); + + $generator = app(DriftFindingGenerator::class); + $created = $generator->generate($tenant, $baseline, $current, $scopeKey); + + expect($created)->toBe(1); + + $finding = Finding::query() + ->where('tenant_id', $tenant->getKey()) + ->where('finding_type', Finding::FINDING_TYPE_DRIFT) + ->where('scope_key', $scopeKey) + ->where('subject_type', 'policy') + ->first(); + + expect($finding)->not->toBeNull(); + expect($finding->subject_external_id)->toBe($policy->external_id); + expect($finding->evidence_jsonb)->toHaveKey('change_type', 'modified'); + expect($finding->evidence_jsonb) + ->toHaveKey('summary.changed_fields') + ->and($finding->evidence_jsonb['summary']['changed_fields'])->toContain('snapshot_hash') + ->and($finding->evidence_jsonb)->toHaveKey('baseline.snapshot_hash') + ->and($finding->evidence_jsonb)->toHaveKey('current.snapshot_hash') + ->and($finding->evidence_jsonb)->not->toHaveKey('baseline.assignments_hash') + ->and($finding->evidence_jsonb)->not->toHaveKey('current.assignments_hash'); +}); diff --git a/tests/Feature/Drift/DriftPolicySnapshotMetadataOnlyDoesNotCreateFindingTest.php b/tests/Feature/Drift/DriftPolicySnapshotMetadataOnlyDoesNotCreateFindingTest.php new file mode 100644 index 0000000..539a343 --- /dev/null +++ b/tests/Feature/Drift/DriftPolicySnapshotMetadataOnlyDoesNotCreateFindingTest.php @@ -0,0 +1,62 @@ +for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['deviceConfiguration']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['deviceConfiguration']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + $policy = Policy::factory()->for($tenant)->create([ + 'policy_type' => 'deviceConfiguration', + 'platform' => 'windows10', + ]); + + PolicyVersion::factory()->for($tenant)->for($policy)->create([ + 'version_number' => 1, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $baseline->finished_at->copy()->subMinute(), + 'snapshot' => [ + 'displayName' => 'My Policy', + 'description' => 'Old description', + ], + 'assignments' => [], + ]); + + PolicyVersion::factory()->for($tenant)->for($policy)->create([ + 'version_number' => 2, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $current->finished_at->copy()->subMinute(), + 'snapshot' => [ + 'displayName' => 'My Policy', + 'description' => 'New description', + ], + 'assignments' => [], + ]); + + $generator = app(DriftFindingGenerator::class); + $created = $generator->generate($tenant, $baseline, $current, $scopeKey); + + expect($created)->toBe(0); + expect(Finding::query()->where('tenant_id', $tenant->getKey())->count())->toBe(0); +}); diff --git a/tests/Feature/Drift/DriftScopeTagDriftDetectionTest.php b/tests/Feature/Drift/DriftScopeTagDriftDetectionTest.php new file mode 100644 index 0000000..15b8b8f --- /dev/null +++ b/tests/Feature/Drift/DriftScopeTagDriftDetectionTest.php @@ -0,0 +1,84 @@ +for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['deviceConfiguration']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['deviceConfiguration']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + $policy = Policy::factory()->for($tenant)->create([ + 'policy_type' => 'deviceConfiguration', + 'platform' => 'windows10', + ]); + + PolicyVersion::factory()->for($tenant)->for($policy)->create([ + 'version_number' => 1, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $baseline->finished_at->copy()->subMinute(), + 'snapshot' => ['customSettingFoo' => 'Same value'], + 'assignments' => [], + 'scope_tags' => [ + 'ids' => ['0'], + 'names' => ['Default'], + ], + ]); + + PolicyVersion::factory()->for($tenant)->for($policy)->create([ + 'version_number' => 2, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $current->finished_at->copy()->subMinute(), + 'snapshot' => ['customSettingFoo' => 'Same value'], + 'assignments' => [], + 'scope_tags' => [ + 'ids' => ['0', 'a1b2c3'], + 'names' => ['Default', 'Verbund-1'], + ], + ]); + + $generator = app(DriftFindingGenerator::class); + $created = $generator->generate($tenant, $baseline, $current, $scopeKey); + + expect($created)->toBe(1); + + $finding = Finding::query() + ->where('tenant_id', $tenant->getKey()) + ->where('finding_type', Finding::FINDING_TYPE_DRIFT) + ->where('scope_key', $scopeKey) + ->where('subject_type', 'scope_tag') + ->first(); + + expect($finding)->not->toBeNull(); + expect($finding->subject_external_id)->toBe($policy->external_id); + expect($finding->evidence_jsonb)->toHaveKey('change_type', 'modified'); + expect($finding->evidence_jsonb) + ->toHaveKey('summary.kind', 'policy_scope_tags') + ->toHaveKey('summary.changed_fields') + ->and($finding->evidence_jsonb['summary']['changed_fields'])->toContain('scope_tags_hash') + ->and($finding->evidence_jsonb)->toHaveKey('baseline.scope_tags_hash') + ->and($finding->evidence_jsonb)->toHaveKey('current.scope_tags_hash') + ->and($finding->evidence_jsonb)->not->toHaveKey('baseline.snapshot_hash') + ->and($finding->evidence_jsonb)->not->toHaveKey('current.snapshot_hash') + ->and($finding->evidence_jsonb)->not->toHaveKey('baseline.assignments_hash') + ->and($finding->evidence_jsonb)->not->toHaveKey('current.assignments_hash'); +}); diff --git a/tests/Feature/Drift/DriftScopeTagLegacyDefaultDoesNotCreateFindingTest.php b/tests/Feature/Drift/DriftScopeTagLegacyDefaultDoesNotCreateFindingTest.php new file mode 100644 index 0000000..be29a5a --- /dev/null +++ b/tests/Feature/Drift/DriftScopeTagLegacyDefaultDoesNotCreateFindingTest.php @@ -0,0 +1,69 @@ +for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['deviceConfiguration']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDays(2), + ]); + + $current = InventorySyncRun::factory()->for($tenant)->create([ + 'selection_hash' => $scopeKey, + 'selection_payload' => ['policy_types' => ['deviceConfiguration']], + 'status' => InventorySyncRun::STATUS_SUCCESS, + 'finished_at' => now()->subDay(), + ]); + + $policy = Policy::factory()->for($tenant)->create([ + 'policy_type' => 'deviceConfiguration', + 'platform' => 'windows10', + ]); + + PolicyVersion::factory()->for($tenant)->for($policy)->create([ + 'version_number' => 1, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $baseline->finished_at->copy()->subMinute(), + 'snapshot' => ['customSettingFoo' => 'Same value'], + 'assignments' => [], + 'scope_tags' => [ + // legacy data shape (missing ids) + 'names' => ['Default'], + ], + ]); + + PolicyVersion::factory()->for($tenant)->for($policy)->create([ + 'version_number' => 2, + 'policy_type' => $policy->policy_type, + 'platform' => $policy->platform, + 'captured_at' => $current->finished_at->copy()->subMinute(), + 'snapshot' => ['customSettingFoo' => 'Same value'], + 'assignments' => [], + 'scope_tags' => [ + 'ids' => ['0'], + 'names' => ['Default'], + ], + ]); + + $generator = app(DriftFindingGenerator::class); + $created = $generator->generate($tenant, $baseline, $current, $scopeKey); + + expect($created)->toBe(0); + + expect(Finding::query() + ->where('tenant_id', $tenant->getKey()) + ->where('finding_type', Finding::FINDING_TYPE_DRIFT) + ->where('scope_key', $scopeKey) + ->count())->toBe(0); +});