From 7b7a6fbe3a6789666c7fdc78f0c138482460b903 Mon Sep 17 00:00:00 2001 From: Ahmed Darrazi Date: Sun, 8 Feb 2026 12:27:49 +0100 Subject: [PATCH] feat(spec081): provider connection cutover --- .github/agents/copilot-instructions.md | 3 +- app/Filament/Pages/ChooseWorkspace.php | 7 +- app/Filament/Pages/Tenancy/RegisterTenant.php | 17 +- .../Resources/ProviderConnectionResource.php | 103 +++-- .../Pages/EditProviderConnection.php | 91 +++-- app/Filament/Resources/TenantResource.php | 67 +++- .../TenantResource/Pages/ViewTenant.php | 43 ++- .../AdminConsentCallbackController.php | 189 +++------ app/Jobs/ProviderConnectionHealthCheckJob.php | 15 +- app/Models/Tenant.php | 2 + app/Observers/ProviderCredentialObserver.php | 145 +++++++ app/Providers/AppServiceProvider.php | 3 + app/Services/Graph/ScopeTagResolver.php | 35 +- app/Services/Intune/PolicySnapshotService.php | 111 +++--- app/Services/Intune/PolicySyncService.php | 118 +++++- app/Services/Intune/RbacHealthService.php | 58 ++- app/Services/Intune/RbacOnboardingService.php | 51 ++- app/Services/Intune/RestoreService.php | 50 ++- .../Inventory/InventorySyncService.php | 62 ++- app/Services/OperationRunService.php | 63 +++ .../MicrosoftProviderHealthCheck.php | 30 +- .../ProviderConnectionResolution.php | 48 +++ .../Providers/ProviderConnectionResolver.php | 124 ++++++ app/Services/Providers/ProviderGateway.php | 20 + .../Providers/ProviderOperationStartGate.php | 90 ++++- .../ProviderOperationStartResult.php | 5 + .../Domains/OperationRunOutcomeBadge.php | 1 + app/Support/OperationRunOutcome.php | 2 + app/Support/OpsUx/RunFailureSanitizer.php | 67 ++-- .../Providers/ProviderNextStepsRegistry.php | 63 +++ app/Support/Providers/ProviderReasonCodes.php | 59 +++ .../TenantPermissionCheckClusters.php | 51 +-- .../VerificationReportSanitizer.php | 14 +- .../Verification/VerificationReportWriter.php | 66 +--- .../filament/partials/context-bar.blade.php | 20 +- .../checklists/requirements.md | 35 ++ .../contracts/README.md | 13 + .../data-model.md | 139 +++++++ specs/081-provider-connection-cutover/plan.md | 139 +++++++ .../quickstart.md | 48 +++ .../research.md | 102 +++++ specs/081-provider-connection-cutover/spec.md | 199 ++++++++++ .../081-provider-connection-cutover/tasks.md | 146 +++++++ tests/Feature/AdminConsentCallbackTest.php | 43 ++- .../ProviderCredentialAuditSpec081Test.php | 126 ++++++ tests/Feature/Filament/TenantSetupTest.php | 40 +- ...aphContractRegistryCoverageSpec081Test.php | 41 ++ ...enantCredentialRuntimeReadsSpec081Test.php | 361 ++++++++++++++++++ .../OperationRunBlockedSpec081Test.php | 50 +++ .../OperationsDbOnlyRenderingSpec081Test.php | 41 ++ tests/Feature/PolicySyncServiceReportTest.php | 32 +- tests/Feature/PolicySyncServiceTest.php | 48 ++- ...iderConnectionAuthorizationSpec081Test.php | 47 +++ .../ProviderConnectionAuthorizationTest.php | 13 +- .../ProviderConnectionCutoverSpec081Test.php | 94 +++++ ...rConnectionDefaultInvariantSpec081Test.php | 58 +++ .../ProviderConnectionEnableDisableTest.php | 4 +- ...rConnectionHealthCheckStartSurfaceTest.php | 9 + ...nectionViewsDbOnlyRenderingSpec081Test.php | 58 +++ ...ProviderGatewayRuntimeSmokeSpec081Test.php | 304 +++++++++++++++ ...derOperationBlockedGuidanceSpec081Test.php | 76 ++++ .../ProviderOperationConcurrencyTest.php | 13 + ...rConnectionHealthCheckWritesReportTest.php | 6 +- .../VerificationAuthorizationTest.php | 5 + ...cationReportNextStepsSchemaSpec081Test.php | 31 ++ .../VerificationStartAfterCompletionTest.php | 5 + .../VerificationStartDedupeTest.php | 5 + tests/Pest.php | 35 +- tests/Unit/Badges/OperationRunBadgesTest.php | 4 + tests/Unit/OpsUx/RunFailureSanitizerTest.php | 9 +- tests/Unit/PolicySnapshotServiceTest.php | 101 ++--- tests/Unit/Providers/ProviderGatewayTest.php | 28 +- .../ProviderOperationStartGateTest.php | 40 ++ tests/Unit/RbacOnboardingServiceTest.php | 27 +- tests/Unit/ScopeTagResolverTest.php | 117 ++++-- .../TenantPermissionCheckClustersTest.php | 3 +- 76 files changed, 3979 insertions(+), 609 deletions(-) create mode 100644 app/Observers/ProviderCredentialObserver.php create mode 100644 app/Services/Providers/ProviderConnectionResolution.php create mode 100644 app/Services/Providers/ProviderConnectionResolver.php create mode 100644 app/Support/Providers/ProviderNextStepsRegistry.php create mode 100644 app/Support/Providers/ProviderReasonCodes.php create mode 100644 specs/081-provider-connection-cutover/checklists/requirements.md create mode 100644 specs/081-provider-connection-cutover/contracts/README.md create mode 100644 specs/081-provider-connection-cutover/data-model.md create mode 100644 specs/081-provider-connection-cutover/plan.md create mode 100644 specs/081-provider-connection-cutover/quickstart.md create mode 100644 specs/081-provider-connection-cutover/research.md create mode 100644 specs/081-provider-connection-cutover/spec.md create mode 100644 specs/081-provider-connection-cutover/tasks.md create mode 100644 tests/Feature/Audit/ProviderCredentialAuditSpec081Test.php create mode 100644 tests/Feature/Graph/GraphContractRegistryCoverageSpec081Test.php create mode 100644 tests/Feature/Guards/NoTenantCredentialRuntimeReadsSpec081Test.php create mode 100644 tests/Feature/Monitoring/OperationRunBlockedSpec081Test.php create mode 100644 tests/Feature/Monitoring/OperationsDbOnlyRenderingSpec081Test.php create mode 100644 tests/Feature/ProviderConnections/ProviderConnectionAuthorizationSpec081Test.php create mode 100644 tests/Feature/ProviderConnections/ProviderConnectionCutoverSpec081Test.php create mode 100644 tests/Feature/ProviderConnections/ProviderConnectionDefaultInvariantSpec081Test.php create mode 100644 tests/Feature/ProviderConnections/ProviderConnectionViewsDbOnlyRenderingSpec081Test.php create mode 100644 tests/Feature/ProviderConnections/ProviderGatewayRuntimeSmokeSpec081Test.php create mode 100644 tests/Feature/ProviderConnections/ProviderOperationBlockedGuidanceSpec081Test.php create mode 100644 tests/Feature/Verification/VerificationReportNextStepsSchemaSpec081Test.php diff --git a/.github/agents/copilot-instructions.md b/.github/agents/copilot-instructions.md index 567925f..1e4af7c 100644 --- a/.github/agents/copilot-instructions.md +++ b/.github/agents/copilot-instructions.md @@ -20,6 +20,7 @@ ## Active Technologies - PostgreSQL (no new migrations — read-only model changes) (078-operations-tenantless-canonical) - PHP 8.4.15 (Laravel 12) + Filament v5, Livewire v4, Tailwind v4 (080-workspace-managed-tenant-admin) - PostgreSQL (via Sail) (080-workspace-managed-tenant-admin) +- PHP 8.4.15 + Laravel 12, Filament v5, Livewire v4, Socialite v5 (081-provider-connection-cutover) - PHP 8.4.15 (feat/005-bulk-operations) @@ -39,9 +40,9 @@ ## Code Style PHP 8.4.15: Follow standard conventions ## Recent Changes +- 081-provider-connection-cutover: Added PHP 8.4.15 + Laravel 12, Filament v5, Livewire v4, Socialite v5 - 080-workspace-managed-tenant-admin: Added PHP 8.4.15 (Laravel 12) + Filament v5, Livewire v4, Tailwind v4 - 078-operations-tenantless-canonical: Added PHP 8.4 (Laravel 12) + Filament v5, Livewire v4, Filament Infolists (schema-based) -- 078-operations-tenantless-canonical: Added [if applicable, e.g., PostgreSQL, CoreData, files or N/A] diff --git a/app/Filament/Pages/ChooseWorkspace.php b/app/Filament/Pages/ChooseWorkspace.php index 9b18a1d..ec0ffe7 100644 --- a/app/Filament/Pages/ChooseWorkspace.php +++ b/app/Filament/Pages/ChooseWorkspace.php @@ -14,7 +14,6 @@ use Filament\Notifications\Notification; use Filament\Pages\Page; use Illuminate\Database\Eloquent\Collection; -use Illuminate\Support\Facades\Gate; class ChooseWorkspace extends Page { @@ -43,7 +42,7 @@ protected function getHeaderActions(): array $user = auth()->user(); return $user instanceof User - && Gate::forUser($user)->check('create', Workspace::class); + && $user->can('create', Workspace::class); }) ->form([ TextInput::make('name') @@ -124,7 +123,9 @@ public function createWorkspace(array $data): void abort(403); } - Gate::forUser($user)->authorize('create', Workspace::class); + if (! $user->can('create', Workspace::class)) { + abort(403); + } $workspace = Workspace::query()->create([ 'name' => $data['name'], diff --git a/app/Filament/Pages/Tenancy/RegisterTenant.php b/app/Filament/Pages/Tenancy/RegisterTenant.php index 4396496..d09bd81 100644 --- a/app/Filament/Pages/Tenancy/RegisterTenant.php +++ b/app/Filament/Pages/Tenancy/RegisterTenant.php @@ -84,21 +84,8 @@ public function form(Schema $schema): Schema ->unique(ignoreRecord: true), Forms\Components\TextInput::make('domain') ->label('Primary domain') - ->maxLength(255), - Forms\Components\TextInput::make('app_client_id') - ->label('App Client ID') - ->maxLength(255), - Forms\Components\TextInput::make('app_client_secret') - ->label('App Client Secret') - ->password() - ->dehydrateStateUsing(fn ($state) => filled($state) ? $state : null) - ->dehydrated(fn ($state) => filled($state)), - Forms\Components\TextInput::make('app_certificate_thumbprint') - ->label('Certificate thumbprint') - ->maxLength(255), - Forms\Components\Textarea::make('app_notes') - ->label('Notes') - ->rows(3), + ->maxLength(255) + ->helperText('Credentials are managed after tenant creation in Provider connections.'), ]); } diff --git a/app/Filament/Resources/ProviderConnectionResource.php b/app/Filament/Resources/ProviderConnectionResource.php index fa8d675..14d62f4 100644 --- a/app/Filament/Resources/ProviderConnectionResource.php +++ b/app/Filament/Resources/ProviderConnectionResource.php @@ -231,6 +231,9 @@ public static function table(Table $table): Table Actions\Action::make('view_run') ->label('View run') ->url(OperationRunLinks::view($result->run, $tenant)), + Actions\Action::make('manage_connections') + ->label('Manage Provider Connections') + ->url(static::getUrl('index', tenant: $tenant)), ]) ->send(); @@ -246,6 +249,31 @@ public static function table(Table $table): Table Actions\Action::make('view_run') ->label('View run') ->url(OperationRunLinks::view($result->run, $tenant)), + Actions\Action::make('manage_connections') + ->label('Manage Provider Connections') + ->url(static::getUrl('index', tenant: $tenant)), + ]) + ->send(); + + return; + } + + if ($result->status === 'blocked') { + $reasonCode = is_string($result->run->context['reason_code'] ?? null) + ? (string) $result->run->context['reason_code'] + : 'unknown_error'; + + Notification::make() + ->title('Connection check blocked') + ->body("Blocked by provider configuration ({$reasonCode}).") + ->warning() + ->actions([ + Actions\Action::make('view_run') + ->label('View run') + ->url(OperationRunLinks::view($result->run, $tenant)), + Actions\Action::make('manage_connections') + ->label('Manage Provider Connections') + ->url(static::getUrl('index', tenant: $tenant)), ]) ->send(); @@ -329,6 +357,25 @@ public static function table(Table $table): Table return; } + if ($result->status === 'blocked') { + $reasonCode = is_string($result->run->context['reason_code'] ?? null) + ? (string) $result->run->context['reason_code'] + : 'unknown_error'; + + Notification::make() + ->title('Inventory sync blocked') + ->body("Blocked by provider configuration ({$reasonCode}).") + ->warning() + ->actions([ + Actions\Action::make('view_run') + ->label('View run') + ->url(OperationRunLinks::view($result->run, $tenant)), + ]) + ->send(); + + return; + } + Notification::make() ->title('Inventory sync queued') ->body('Inventory sync was queued and will run in the background.') @@ -406,6 +453,25 @@ public static function table(Table $table): Table return; } + if ($result->status === 'blocked') { + $reasonCode = is_string($result->run->context['reason_code'] ?? null) + ? (string) $result->run->context['reason_code'] + : 'unknown_error'; + + Notification::make() + ->title('Compliance snapshot blocked') + ->body("Blocked by provider configuration ({$reasonCode}).") + ->warning() + ->actions([ + Actions\Action::make('view_run') + ->label('View run') + ->url(OperationRunLinks::view($result->run, $tenant)), + ]) + ->send(); + + return; + } + Notification::make() ->title('Compliance snapshot queued') ->body('Compliance snapshot was queued and will run in the background.') @@ -474,6 +540,7 @@ public static function table(Table $table): Table ->label('Update credentials') ->icon('heroicon-o-key') ->color('primary') + ->requiresConfirmation() ->modalDescription('Client secret is stored encrypted and will never be shown again.') ->form([ TextInput::make('client_id') @@ -486,7 +553,7 @@ public static function table(Table $table): Table ->required() ->maxLength(255), ]) - ->action(function (array $data, ProviderConnection $record, CredentialManager $credentials, AuditLogger $auditLogger): void { + ->action(function (array $data, ProviderConnection $record, CredentialManager $credentials): void { $tenant = static::resolveScopedTenant(); if (! $tenant instanceof Tenant) { @@ -499,28 +566,6 @@ public static function table(Table $table): Table clientSecret: (string) $data['client_secret'], ); - $user = auth()->user(); - $actorId = $user instanceof User ? (int) $user->getKey() : null; - $actorEmail = $user instanceof User ? $user->email : null; - $actorName = $user instanceof User ? $user->name : null; - - $auditLogger->log( - tenant: $tenant, - action: 'provider_connection.credentials_updated', - context: [ - 'metadata' => [ - 'provider' => $record->provider, - 'entra_tenant_id' => $record->entra_tenant_id, - ], - ], - actorId: $actorId, - actorEmail: $actorEmail, - actorName: $actorName, - resourceType: 'provider_connection', - resourceId: (string) $record->getKey(), - status: 'success', - ); - Notification::make() ->title('Credentials updated') ->success() @@ -692,13 +737,23 @@ public static function getPages(): array public static function getUrl(?string $name = null, array $parameters = [], bool $isAbsolute = true, ?string $panel = null, ?Model $tenant = null, bool $shouldGuessMissingParameters = false): string { if (! array_key_exists('tenant', $parameters)) { + if ($tenant instanceof Tenant) { + $parameters['tenant'] = $tenant->external_id; + } + $resolvedTenant = static::resolveScopedTenant(); - if ($resolvedTenant instanceof Tenant) { + if (! array_key_exists('tenant', $parameters) && $resolvedTenant instanceof Tenant) { $parameters['tenant'] = $resolvedTenant->external_id; } } + $panel ??= 'admin'; + + if (array_key_exists('tenant', $parameters)) { + $tenant = null; + } + return parent::getUrl($name, $parameters, $isAbsolute, $panel, $tenant, $shouldGuessMissingParameters); } } diff --git a/app/Filament/Resources/ProviderConnectionResource/Pages/EditProviderConnection.php b/app/Filament/Resources/ProviderConnectionResource/Pages/EditProviderConnection.php index dbe9470..8b70473 100644 --- a/app/Filament/Resources/ProviderConnectionResource/Pages/EditProviderConnection.php +++ b/app/Filament/Resources/ProviderConnectionResource/Pages/EditProviderConnection.php @@ -200,6 +200,9 @@ protected function getHeaderActions(): array Action::make('view_run') ->label('View run') ->url(OperationRunLinks::view($result->run, $tenant)), + Action::make('manage_connections') + ->label('Manage Provider Connections') + ->url(ProviderConnectionResource::getUrl('index', tenant: $tenant)), ]) ->send(); @@ -215,6 +218,31 @@ protected function getHeaderActions(): array Action::make('view_run') ->label('View run') ->url(OperationRunLinks::view($result->run, $tenant)), + Action::make('manage_connections') + ->label('Manage Provider Connections') + ->url(ProviderConnectionResource::getUrl('index', tenant: $tenant)), + ]) + ->send(); + + return; + } + + if ($result->status === 'blocked') { + $reasonCode = is_string($result->run->context['reason_code'] ?? null) + ? (string) $result->run->context['reason_code'] + : 'unknown_error'; + + Notification::make() + ->title('Connection check blocked') + ->body("Blocked by provider configuration ({$reasonCode}).") + ->warning() + ->actions([ + Action::make('view_run') + ->label('View run') + ->url(OperationRunLinks::view($result->run, $tenant)), + Action::make('manage_connections') + ->label('Manage Provider Connections') + ->url(ProviderConnectionResource::getUrl('index', tenant: $tenant)), ]) ->send(); @@ -242,6 +270,7 @@ protected function getHeaderActions(): array ->label('Update credentials') ->icon('heroicon-o-key') ->color('primary') + ->requiresConfirmation() ->modalDescription('Client secret is stored encrypted and will never be shown again.') ->visible(fn (): bool => $tenant instanceof Tenant) ->form([ @@ -255,7 +284,7 @@ protected function getHeaderActions(): array ->required() ->maxLength(255), ]) - ->action(function (array $data, ProviderConnection $record, CredentialManager $credentials, AuditLogger $auditLogger): void { + ->action(function (array $data, ProviderConnection $record, CredentialManager $credentials): void { $tenant = $this->currentTenant(); if (! $tenant instanceof Tenant) { @@ -268,28 +297,6 @@ protected function getHeaderActions(): array clientSecret: (string) $data['client_secret'], ); - $user = auth()->user(); - $actorId = $user instanceof User ? (int) $user->getKey() : null; - $actorEmail = $user instanceof User ? $user->email : null; - $actorName = $user instanceof User ? $user->name : null; - - $auditLogger->log( - tenant: $tenant, - action: 'provider_connection.credentials_updated', - context: [ - 'metadata' => [ - 'provider' => $record->provider, - 'entra_tenant_id' => $record->entra_tenant_id, - ], - ], - actorId: $actorId, - actorEmail: $actorEmail, - actorName: $actorName, - resourceType: 'provider_connection', - resourceId: (string) $record->getKey(), - status: 'success', - ); - Notification::make() ->title('Credentials updated') ->success() @@ -432,6 +439,25 @@ protected function getHeaderActions(): array return; } + if ($result->status === 'blocked') { + $reasonCode = is_string($result->run->context['reason_code'] ?? null) + ? (string) $result->run->context['reason_code'] + : 'unknown_error'; + + Notification::make() + ->title('Inventory sync blocked') + ->body("Blocked by provider configuration ({$reasonCode}).") + ->warning() + ->actions([ + Action::make('view_run') + ->label('View run') + ->url(OperationRunLinks::view($result->run, $tenant)), + ]) + ->send(); + + return; + } + Notification::make() ->title('Inventory sync queued') ->body('Inventory sync was queued and will run in the background.') @@ -526,6 +552,25 @@ protected function getHeaderActions(): array return; } + if ($result->status === 'blocked') { + $reasonCode = is_string($result->run->context['reason_code'] ?? null) + ? (string) $result->run->context['reason_code'] + : 'unknown_error'; + + Notification::make() + ->title('Compliance snapshot blocked') + ->body("Blocked by provider configuration ({$reasonCode}).") + ->warning() + ->actions([ + Action::make('view_run') + ->label('View run') + ->url(OperationRunLinks::view($result->run, $tenant)), + ]) + ->send(); + + return; + } + Notification::make() ->title('Compliance snapshot queued') ->body('Compliance snapshot was queued and will run in the background.') diff --git a/app/Filament/Resources/TenantResource.php b/app/Filament/Resources/TenantResource.php index 661ae4b..5d2f0bb 100644 --- a/app/Filament/Resources/TenantResource.php +++ b/app/Filament/Resources/TenantResource.php @@ -174,21 +174,8 @@ public static function form(Schema $schema): Schema ->unique(ignoreRecord: true), Forms\Components\TextInput::make('domain') ->label('Primary domain') - ->maxLength(255), - Forms\Components\TextInput::make('app_client_id') - ->label('App Client ID') - ->maxLength(255), - Forms\Components\TextInput::make('app_client_secret') - ->label('App Client Secret') - ->password() - ->dehydrateStateUsing(fn ($state) => filled($state) ? $state : null) - ->dehydrated(fn ($state) => filled($state)), - Forms\Components\TextInput::make('app_certificate_thumbprint') - ->label('Certificate thumbprint') - ->maxLength(255), - Forms\Components\Textarea::make('app_notes') - ->label('Notes') - ->rows(3), + ->maxLength(255) + ->helperText('Credentials are managed in Provider connections.'), ]); } @@ -740,7 +727,6 @@ public static function infolist(Schema $schema): Schema Infolists\Components\TextEntry::make('name'), Infolists\Components\TextEntry::make('tenant_id')->label('Tenant ID')->copyable(), Infolists\Components\TextEntry::make('domain')->copyable(), - Infolists\Components\TextEntry::make('app_client_id')->label('App Client ID')->copyable(), Infolists\Components\TextEntry::make('status') ->badge() ->formatStateUsing(BadgeRenderer::label(BadgeDomain::TenantStatus)) @@ -753,7 +739,6 @@ public static function infolist(Schema $schema): Schema ->color(BadgeRenderer::color(BadgeDomain::TenantAppStatus)) ->icon(BadgeRenderer::icon(BadgeDomain::TenantAppStatus)) ->iconColor(BadgeRenderer::iconColor(BadgeDomain::TenantAppStatus)), - Infolists\Components\TextEntry::make('app_notes')->label('Notes'), Infolists\Components\TextEntry::make('created_at')->dateTime(), Infolists\Components\TextEntry::make('updated_at')->dateTime(), Infolists\Components\TextEntry::make('rbac_status') @@ -782,7 +767,7 @@ public static function infolist(Schema $schema): Schema ->copyable(), Infolists\Components\RepeatableEntry::make('permissions') ->label('Required permissions') - ->state(fn (Tenant $record) => app(TenantPermissionService::class)->compare($record, persist: false, useConfiguredStub: false)['permissions']) + ->state(fn (Tenant $record) => static::storedPermissionSnapshot($record)) ->schema([ Infolists\Components\TextEntry::make('key')->label('Permission')->badge(), Infolists\Components\TextEntry::make('type')->badge(), @@ -800,6 +785,42 @@ public static function infolist(Schema $schema): Schema ]); } + /** + * @return array,status:string,details:array|null}> + */ + protected static function storedPermissionSnapshot(Tenant $tenant): array + { + $required = config('intune_permissions.permissions', []); + + $stored = $tenant->permissions() + ->get() + ->keyBy('permission_key'); + + $snapshot = []; + + foreach ($required as $permission) { + $key = is_string($permission['key'] ?? null) ? (string) $permission['key'] : null; + + if ($key === null || $key === '') { + continue; + } + + $storedEntry = $stored->get($key); + $storedDetails = $storedEntry?->details; + + $snapshot[] = [ + 'key' => $key, + 'type' => is_string($permission['type'] ?? null) ? (string) $permission['type'] : 'application', + 'description' => is_string($permission['description'] ?? null) ? (string) $permission['description'] : null, + 'features' => is_array($permission['features'] ?? null) ? $permission['features'] : [], + 'status' => is_string($storedEntry?->status ?? null) ? (string) $storedEntry->status : 'missing', + 'details' => is_array($storedDetails) ? $storedDetails : null, + ]; + } + + return $snapshot; + } + public static function getPages(): array { return [ @@ -818,6 +839,16 @@ public static function getRelations(): array ]; } + /** + * @param array $parameters + */ + public static function getUrl(?string $name = null, array $parameters = [], bool $isAbsolute = true, ?string $panel = null, ?Model $tenant = null, bool $shouldGuessMissingParameters = false): string + { + $panel ??= 'admin'; + + return parent::getUrl($name, $parameters, $isAbsolute, $panel, $tenant, $shouldGuessMissingParameters); + } + public static function rbacAction(): Actions\Action { // ... [RBAC Action Omitted - No Change] ... diff --git a/app/Filament/Resources/TenantResource/Pages/ViewTenant.php b/app/Filament/Resources/TenantResource/Pages/ViewTenant.php index 81dfaae..996cc93 100644 --- a/app/Filament/Resources/TenantResource/Pages/ViewTenant.php +++ b/app/Filament/Resources/TenantResource/Pages/ViewTenant.php @@ -11,7 +11,9 @@ use App\Services\Intune\RbacHealthService; use App\Services\Intune\TenantConfigService; use App\Services\Intune\TenantPermissionService; +use App\Services\Providers\ProviderConnectionResolver; use App\Support\Auth\Capabilities; +use App\Support\Providers\ProviderNextStepsRegistry; use App\Support\Rbac\UiEnforcement; use Filament\Actions; use Filament\Notifications\Notification; @@ -71,8 +73,47 @@ protected function getHeaderActions(): array TenantConfigService $configService, TenantPermissionService $permissionService, RbacHealthService $rbacHealthService, - AuditLogger $auditLogger + AuditLogger $auditLogger, + ProviderConnectionResolver $connectionResolver, + ProviderNextStepsRegistry $nextStepsRegistry, ) { + $resolution = $connectionResolver->resolveDefault($record, 'microsoft'); + + if (! $resolution->resolved) { + $reasonCode = $resolution->effectiveReasonCode(); + $nextSteps = $nextStepsRegistry->forReason($record, $reasonCode, $resolution->connection); + + $notification = Notification::make() + ->title('Verification blocked') + ->body("Blocked by provider configuration ({$reasonCode}).") + ->warning(); + + foreach ($nextSteps as $index => $step) { + if (! is_array($step)) { + continue; + } + + $label = is_string($step['label'] ?? null) ? $step['label'] : null; + $url = is_string($step['url'] ?? null) ? $step['url'] : null; + + if ($label === null || $url === null) { + continue; + } + + $notification->actions([ + Actions\Action::make('next_step_'.$index) + ->label($label) + ->url($url), + ]); + + break; + } + + $notification->send(); + + return; + } + TenantResource::verifyTenant($record, $configService, $permissionService, $rbacHealthService, $auditLogger); }), TenantResource::rbacAction(), diff --git a/app/Http/Controllers/AdminConsentCallbackController.php b/app/Http/Controllers/AdminConsentCallbackController.php index 28f2f49..673c293 100644 --- a/app/Http/Controllers/AdminConsentCallbackController.php +++ b/app/Http/Controllers/AdminConsentCallbackController.php @@ -2,13 +2,11 @@ namespace App\Http\Controllers; +use App\Models\ProviderConnection; use App\Models\Tenant; -use App\Services\Graph\GraphClientInterface; use App\Services\Intune\AuditLogger; -use App\Services\Intune\TenantConfigService; -use App\Services\Intune\TenantPermissionService; +use App\Support\Providers\ProviderReasonCodes; use Illuminate\Http\Request; -use Illuminate\Support\Facades\Http; use Illuminate\View\View; use Symfony\Component\HttpFoundation\Response as ResponseAlias; @@ -20,9 +18,6 @@ class AdminConsentCallbackController extends Controller public function __invoke( Request $request, AuditLogger $auditLogger, - TenantConfigService $configService, - TenantPermissionService $permissionService, - GraphClientInterface $graphClient ): View { $expectedState = $request->session()->pull('tenant_onboard_state'); $tenantKey = $request->string('tenant')->toString(); @@ -35,23 +30,7 @@ public function __invoke( abort_if(empty($tenantIdentifier), 404); - $tenant = Tenant::withTrashed() - ->forTenant($tenantIdentifier) - ->first(); - - if ($tenant?->trashed()) { - $tenant->restore(); - } - - if (! $tenant) { - $tenant = Tenant::create([ - 'tenant_id' => $tenantIdentifier, - 'name' => 'New Tenant', - 'app_client_id' => config('graph.client_id'), - 'app_client_secret' => config('graph.client_secret'), - 'app_status' => 'pending', - ]); - } + $tenant = $this->resolveTenant($tenantIdentifier); $error = $request->string('error')->toString() ?: null; $consentGranted = $request->has('admin_consent') @@ -65,10 +44,11 @@ public function __invoke( default => 'pending', }; - $tenant->update([ - 'app_status' => $status, - 'app_notes' => $error, - ]); + $connection = $this->upsertProviderConnectionForConsent( + tenant: $tenant, + status: $status, + error: $error, + ); $auditLogger->log( tenant: $tenant, @@ -79,6 +59,7 @@ public function __invoke( 'state' => $state, 'error' => $error, 'consent' => $consentGranted, + 'provider_connection_id' => (int) $connection->getKey(), ], ], status: $status === 'ok' ? 'success' : 'error', @@ -94,133 +75,71 @@ public function __invoke( ]); } - private function handleAuthorizationCodeFlow( - Request $request, - AuditLogger $auditLogger, - TenantConfigService $configService, - TenantPermissionService $permissionService, - GraphClientInterface $graphClient - ): View { - $expectedState = $request->session()->pull('tenant_onboard_state'); - if ($expectedState && $expectedState !== $request->string('state')->toString()) { - abort(ResponseAlias::HTTP_FORBIDDEN, 'Invalid consent state'); - } - - $redirectUri = route('admin.consent.callback'); - - $token = $this->exchangeAuthorizationCode( - code: $request->string('code')->toString(), - redirectUri: $redirectUri - ); - - $tenantId = $token['tenant_id'] ?? null; - abort_if(empty($tenantId), 500, 'Tenant ID missing from token'); - + private function resolveTenant(string $tenantIdentifier): Tenant + { /** @var Tenant|null $tenant */ $tenant = Tenant::withTrashed() - ->forTenant($tenantId) + ->forTenant($tenantIdentifier) ->first(); if ($tenant?->trashed()) { $tenant->restore(); } - if (! $tenant) { - $tenant = Tenant::create([ - 'tenant_id' => $tenantId, - 'name' => 'New Tenant', - 'app_client_id' => config('graph.client_id'), - 'app_client_secret' => config('graph.client_secret'), - 'app_status' => 'pending', - ]); + if ($tenant instanceof Tenant) { + return $tenant; } - $orgResponse = $graphClient->getOrganization([ - 'tenant' => $tenant->graphTenantId(), - 'client_id' => $tenant->app_client_id, - 'client_secret' => $tenant->app_client_secret, + return Tenant::create([ + 'tenant_id' => $tenantIdentifier, + 'name' => 'New Tenant', ]); + } - if ($orgResponse->successful()) { - $org = $orgResponse->data ?? []; - $tenant->update([ - 'name' => $org['displayName'] ?? $tenant->name, - 'domain' => $org['verifiedDomains'][0]['name'] ?? $tenant->domain, - ]); - } + private function upsertProviderConnectionForConsent(Tenant $tenant, string $status, ?string $error): ProviderConnection + { + $hasDefault = ProviderConnection::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->where('provider', 'microsoft') + ->where('is_default', true) + ->exists(); - $configResult = $configService->testConnectivity($tenant); - $permissionService->compare($tenant); + $connectionStatus = match ($status) { + 'ok' => 'connected', + 'error' => 'error', + 'consent_denied' => 'needs_consent', + default => 'needs_consent', + }; - $status = $configResult['success'] ? 'ok' : 'error'; + $reasonCode = match ($status) { + 'ok' => null, + 'consent_denied' => ProviderReasonCodes::ProviderConsentMissing, + 'error' => ProviderReasonCodes::ProviderAuthFailed, + default => ProviderReasonCodes::ProviderConsentMissing, + }; - $tenant->update([ - 'app_status' => $status, - 'app_notes' => $configResult['error_message'], - ]); - - $auditLogger->log( - tenant: $tenant, - action: 'tenant.consent.callback', - context: [ - 'metadata' => [ - 'status' => $status, - 'error' => $configResult['error_message'], - 'from' => 'authorization_code', - ], + $connection = ProviderConnection::query()->updateOrCreate( + [ + 'tenant_id' => (int) $tenant->getKey(), + 'provider' => 'microsoft', + 'entra_tenant_id' => (string) ($tenant->graphTenantId() ?? $tenant->tenant_id ?? $tenant->external_id), + ], + [ + 'workspace_id' => (int) $tenant->workspace_id, + 'display_name' => (string) ($tenant->name ?? 'Microsoft Connection'), + 'status' => $connectionStatus, + 'health_status' => $connectionStatus === 'connected' ? 'unknown' : 'degraded', + 'last_error_reason_code' => $reasonCode, + 'last_error_message' => $error, + 'is_default' => $hasDefault ? false : true, ], - status: $status === 'ok' ? 'success' : 'error', - resourceType: 'tenant', - resourceId: (string) $tenant->id, ); - return view('admin-consent-callback', [ - 'tenant' => $tenant, - 'status' => $status, - 'error' => $configResult['error_message'], - 'consentGranted' => $status === 'ok', - ]); - } - - /** - * @return array{access_token:string,id_token:string,tenant_id:?string} - */ - private function exchangeAuthorizationCode(string $code, string $redirectUri): array - { - $response = Http::asForm()->post('https://login.microsoftonline.com/common/oauth2/v2.0/token', [ - 'client_id' => config('graph.client_id'), - 'client_secret' => config('graph.client_secret'), - 'code' => $code, - 'grant_type' => 'authorization_code', - 'redirect_uri' => $redirectUri, - 'scope' => 'https://graph.microsoft.com/.default offline_access openid profile', - ]); - - if ($response->failed()) { - abort(ResponseAlias::HTTP_BAD_GATEWAY, 'Failed to exchange code for token'); + if (! $hasDefault && ! $connection->is_default) { + $connection->makeDefault(); } - $body = $response->json(); - $idToken = $body['id_token'] ?? null; - $tenantId = $this->parseTenantIdFromToken($idToken); - - return [ - 'access_token' => $body['access_token'] ?? '', - 'id_token' => $idToken ?? '', - 'tenant_id' => $tenantId, - ]; - } - - private function parseTenantIdFromToken(?string $token): ?string - { - if (! $token || ! str_contains($token, '.')) { - return null; - } - - $parts = explode('.', $token); - $payload = json_decode(base64_decode(strtr($parts[1], '-_', '+/')) ?: '[]', true); - - return $payload['tid'] ?? $payload['tenant'] ?? null; + return $connection; } private function parseState(?string $state): ?string diff --git a/app/Jobs/ProviderConnectionHealthCheckJob.php b/app/Jobs/ProviderConnectionHealthCheckJob.php index a3a7f72..3c53475 100644 --- a/app/Jobs/ProviderConnectionHealthCheckJob.php +++ b/app/Jobs/ProviderConnectionHealthCheckJob.php @@ -16,6 +16,7 @@ use App\Support\Audit\AuditActionId; use App\Support\OperationRunOutcome; use App\Support\OperationRunStatus; +use App\Support\Providers\ProviderNextStepsRegistry; use App\Support\Verification\TenantPermissionCheckClusters; use App\Support\Verification\VerificationReportWriter; use Illuminate\Bus\Queueable; @@ -201,13 +202,13 @@ public function handle( ])), 'next_steps' => $result->healthy ? [] - : [[ - 'label' => 'Review provider connection', - 'url' => \App\Filament\Resources\ProviderConnectionResource::getUrl('edit', [ - 'tenant' => $tenant, - 'record' => (int) $connection->getKey(), - ], panel: 'admin'), - ]], + : app(ProviderNextStepsRegistry::class)->forReason( + $tenant, + is_string($result->reasonCode) && $result->reasonCode !== '' + ? $result->reasonCode + : 'unknown_error', + $connection, + ), ], ...$permissionChecks, ], diff --git a/app/Models/Tenant.php b/app/Models/Tenant.php index f8a8a78..8b47a5a 100644 --- a/app/Models/Tenant.php +++ b/app/Models/Tenant.php @@ -291,6 +291,8 @@ public function graphTenantId(): ?string } /** + * @deprecated Runtime provider calls must resolve ProviderConnection + ProviderGateway. + * * @return array{tenant:?string,client_id:?string,client_secret:?string} */ public function graphOptions(): array diff --git a/app/Observers/ProviderCredentialObserver.php b/app/Observers/ProviderCredentialObserver.php new file mode 100644 index 0000000..eca1d7e --- /dev/null +++ b/app/Observers/ProviderCredentialObserver.php @@ -0,0 +1,145 @@ +resolveConnection($credential); + + if (! $connection instanceof ProviderConnection) { + return; + } + + $tenant = $connection->tenant; + + if (! $tenant instanceof Tenant) { + return; + } + + $this->audit( + tenant: $tenant, + connection: $connection, + action: 'provider_connection.credentials_created', + changedFields: ['type', 'client_id', 'client_secret'], + ); + } + + public function updated(ProviderCredential $credential): void + { + $connection = $this->resolveConnection($credential); + + if (! $connection instanceof ProviderConnection) { + return; + } + + $tenant = $connection->tenant; + + if (! $tenant instanceof Tenant) { + return; + } + + $changedFields = $this->changedFields($credential); + + if ($changedFields === []) { + return; + } + + $action = in_array('client_secret', $changedFields, true) + ? 'provider_connection.credentials_rotated' + : 'provider_connection.credentials_updated'; + + $this->audit( + tenant: $tenant, + connection: $connection, + action: $action, + changedFields: $changedFields, + ); + } + + private function resolveConnection(ProviderCredential $credential): ?ProviderConnection + { + $credential->loadMissing('providerConnection.tenant'); + + return $credential->providerConnection; + } + + /** + * @return array + */ + private function changedFields(ProviderCredential $credential): array + { + $fields = []; + + if ($credential->isDirty('type') || $credential->wasChanged('type')) { + $fields[] = 'type'; + } + + $previousPayload = $credential->getOriginal('payload'); + $currentPayload = $credential->payload; + + $previousPayload = is_array($previousPayload) ? $previousPayload : []; + $currentPayload = is_array($currentPayload) ? $currentPayload : []; + + $previousClientId = trim((string) ($previousPayload['client_id'] ?? '')); + $currentClientId = trim((string) ($currentPayload['client_id'] ?? '')); + + if ($previousClientId !== $currentClientId) { + $fields[] = 'client_id'; + } + + $previousClientSecret = trim((string) ($previousPayload['client_secret'] ?? '')); + $currentClientSecret = trim((string) ($currentPayload['client_secret'] ?? '')); + + if ($previousClientSecret !== $currentClientSecret) { + $fields[] = 'client_secret'; + } + + return array_values(array_unique($fields)); + } + + /** + * @param array $changedFields + */ + private function audit( + Tenant $tenant, + ProviderConnection $connection, + string $action, + array $changedFields, + ): void { + $user = auth()->user(); + $actorId = $user instanceof User ? (int) $user->getKey() : null; + $actorEmail = $user instanceof User ? $user->email : null; + $actorName = $user instanceof User ? $user->name : null; + + app(AuditLogger::class)->log( + tenant: $tenant, + action: $action, + context: [ + 'metadata' => [ + 'provider_connection_id' => (int) $connection->getKey(), + 'provider' => (string) $connection->provider, + 'entra_tenant_id' => (string) $connection->entra_tenant_id, + 'credential_type' => (string) $connection->credential?->type, + 'changed_fields' => $changedFields, + 'redacted_fields' => ['client_secret'], + ], + ], + actorId: $actorId, + actorEmail: $actorEmail, + actorName: $actorName, + resourceType: 'provider_connection', + resourceId: (string) $connection->getKey(), + status: 'success', + ); + } +} diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index 06a7e1e..0ca3725 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -7,10 +7,12 @@ use App\Models\EntraGroupSyncRun; use App\Models\Finding; use App\Models\OperationRun; +use App\Models\ProviderCredential; use App\Models\RestoreRun; use App\Models\Tenant; use App\Models\User; use App\Models\UserTenantPreference; +use App\Observers\ProviderCredentialObserver; use App\Observers\RestoreRunObserver; use App\Policies\BackupSchedulePolicy; use App\Policies\EntraGroupPolicy; @@ -89,6 +91,7 @@ public function boot(): void }); RestoreRun::observe(RestoreRunObserver::class); + ProviderCredential::observe(ProviderCredentialObserver::class); Event::listen(TenantSet::class, function (TenantSet $event): void { static $hasPreferencesTable; diff --git a/app/Services/Graph/ScopeTagResolver.php b/app/Services/Graph/ScopeTagResolver.php index 0460eeb..dcf9ec7 100644 --- a/app/Services/Graph/ScopeTagResolver.php +++ b/app/Services/Graph/ScopeTagResolver.php @@ -3,13 +3,15 @@ namespace App\Services\Graph; use App\Models\Tenant; +use App\Services\Providers\ProviderConnectionResolver; +use App\Services\Providers\ProviderGateway; use Illuminate\Support\Facades\Cache; class ScopeTagResolver { public function __construct( - private readonly MicrosoftGraphClient $graphClient, - private readonly GraphLogger $logger, + private readonly ProviderConnectionResolver $providerConnections, + private readonly ProviderGateway $providerGateway, ) {} /** @@ -42,23 +44,30 @@ public function resolve(array $scopeTagIds, ?Tenant $tenant = null): array */ private function fetchAllScopeTags(?Tenant $tenant = null): array { + if (! $tenant instanceof Tenant) { + return []; + } + $cacheKey = $tenant ? "scope_tags:tenant:{$tenant->id}" : 'scope_tags:all'; - + return Cache::remember($cacheKey, 3600, function () use ($tenant) { try { - $options = ['query' => ['$select' => 'id,displayName']]; - - // Add tenant credentials if provided - if ($tenant) { - $options['tenant'] = $tenant->external_id ?? $tenant->tenant_id; - $options['client_id'] = $tenant->app_client_id; - $options['client_secret'] = $tenant->app_client_secret; + $resolution = $this->providerConnections->resolveDefault($tenant, 'microsoft'); + + if (! $resolution->resolved || $resolution->connection === null) { + \Log::warning('Scope tag fetch blocked: provider connection unavailable', [ + 'tenant_id' => $tenant->id, + 'reason_code' => $resolution->effectiveReasonCode(), + ]); + + return []; } - - $graphResponse = $this->graphClient->request( + + $graphResponse = $this->providerGateway->request( + $resolution->connection, 'GET', '/deviceManagement/roleScopeTags', - $options + ['query' => ['$select' => 'id,displayName']] ); $scopeTags = $graphResponse->data['value'] ?? []; diff --git a/app/Services/Intune/PolicySnapshotService.php b/app/Services/Intune/PolicySnapshotService.php index aa3c57e..479f6ce 100644 --- a/app/Services/Intune/PolicySnapshotService.php +++ b/app/Services/Intune/PolicySnapshotService.php @@ -3,13 +3,18 @@ namespace App\Services\Intune; use App\Models\Policy; +use App\Models\ProviderConnection; use App\Models\Tenant; use App\Services\Graph\GraphClientInterface; use App\Services\Graph\GraphContractRegistry; use App\Services\Graph\GraphErrorMapper; use App\Services\Graph\GraphLogger; use App\Services\Graph\GraphResponse; +use App\Services\Providers\ProviderConnectionResolver; +use App\Services\Providers\ProviderGateway; +use App\Support\Providers\ProviderReasonCodes; use Illuminate\Support\Arr; +use RuntimeException; use Throwable; class PolicySnapshotService @@ -20,6 +25,8 @@ public function __construct( private readonly GraphContractRegistry $contracts, private readonly SnapshotValidator $snapshotValidator, private readonly SettingsCatalogDefinitionResolver $definitionResolver, + private readonly ?ProviderConnectionResolver $providerConnections = null, + private readonly ?ProviderGateway $providerGateway = null, ) {} /** @@ -30,6 +37,8 @@ public function __construct( public function fetch(Tenant $tenant, Policy $policy, ?string $actorEmail = null): array { $tenantIdentifier = $tenant->tenant_id ?? $tenant->external_id; + $connection = null; + $graphOptions = []; $context = [ 'tenant' => $tenantIdentifier, @@ -40,12 +49,13 @@ public function fetch(Tenant $tenant, Policy $policy, ?string $actorEmail = null $this->graphLogger->logRequest('get_policy', $context); try { - $options = [ - 'tenant' => $tenantIdentifier, - 'client_id' => $tenant->app_client_id, - 'client_secret' => $tenant->app_client_secret, - 'platform' => $policy->platform, - ]; + $connection = $this->resolveProviderConnection($tenant); + $tenantIdentifier = (string) $connection->entra_tenant_id; + $context['tenant'] = $tenantIdentifier; + $context['provider_connection_id'] = (int) $connection->getKey(); + $graphOptions = $this->providerGateway()->graphOptions($connection); + + $options = ['platform' => $policy->platform] + $graphOptions; if ($this->isMetadataOnlyPolicyType($policy->policy_type)) { $select = $this->metadataOnlySelect($policy->policy_type); @@ -85,8 +95,7 @@ public function fetch(Tenant $tenant, Policy $policy, ?string $actorEmail = null if ($policy->policy_type === 'windowsUpdateRing') { [$payload, $metadata] = $this->hydrateWindowsUpdateRing( - tenantIdentifier: $tenantIdentifier, - tenant: $tenant, + graphOptions: $graphOptions, policyId: $policy->external_id, payload: is_array($payload) ? $payload : [], metadata: $metadata, @@ -96,8 +105,7 @@ public function fetch(Tenant $tenant, Policy $policy, ?string $actorEmail = null if (in_array($policy->policy_type, ['settingsCatalogPolicy', 'endpointSecurityPolicy', 'securityBaselinePolicy'], true)) { [$payload, $metadata] = $this->hydrateConfigurationPolicySettings( policyType: $policy->policy_type, - tenantIdentifier: $tenantIdentifier, - tenant: $tenant, + graphOptions: $graphOptions, policyId: $policy->external_id, payload: is_array($payload) ? $payload : [], metadata: $metadata @@ -106,8 +114,7 @@ public function fetch(Tenant $tenant, Policy $policy, ?string $actorEmail = null if ($policy->policy_type === 'groupPolicyConfiguration') { [$payload, $metadata] = $this->hydrateGroupPolicyConfiguration( - tenantIdentifier: $tenantIdentifier, - tenant: $tenant, + graphOptions: $graphOptions, policyId: $policy->external_id, payload: is_array($payload) ? $payload : [], metadata: $metadata @@ -116,8 +123,7 @@ public function fetch(Tenant $tenant, Policy $policy, ?string $actorEmail = null if ($policy->policy_type === 'deviceCompliancePolicy') { [$payload, $metadata] = $this->hydrateComplianceActions( - tenantIdentifier: $tenantIdentifier, - tenant: $tenant, + graphOptions: $graphOptions, policyId: $policy->external_id, payload: is_array($payload) ? $payload : [], metadata: $metadata @@ -126,8 +132,7 @@ public function fetch(Tenant $tenant, Policy $policy, ?string $actorEmail = null if ($policy->policy_type === 'deviceEnrollmentNotificationConfiguration') { [$payload, $metadata] = $this->hydrateEnrollmentNotificationTemplates( - tenantIdentifier: $tenantIdentifier, - tenant: $tenant, + graphOptions: $graphOptions, payload: is_array($payload) ? $payload : [], metadata: $metadata ); @@ -230,7 +235,7 @@ private function formatGraphFailureReason(GraphResponse $response): string * * @return array{0:array,1:array} */ - private function hydrateWindowsUpdateRing(string $tenantIdentifier, Tenant $tenant, string $policyId, array $payload, array $metadata): array + private function hydrateWindowsUpdateRing(array $graphOptions, string $policyId, array $payload, array $metadata): array { $odataType = $payload['@odata.type'] ?? null; $castSegment = $this->deriveTypeCastSegment($odataType); @@ -243,11 +248,7 @@ private function hydrateWindowsUpdateRing(string $tenantIdentifier, Tenant $tena $castPath = sprintf('deviceManagement/deviceConfigurations/%s/%s', urlencode($policyId), $castSegment); - $response = $this->graphClient->request('GET', $castPath, [ - 'tenant' => $tenantIdentifier, - 'client_id' => $tenant->app_client_id, - 'client_secret' => $tenant->app_client_secret, - ]); + $response = $this->graphClient->request('GET', $castPath, Arr::except($graphOptions, ['platform'])); if ($response->failed() || ! is_array($response->data)) { $metadata['properties_hydration'] = 'failed'; @@ -329,7 +330,7 @@ private function filterMetadataOnlyPayload(string $policyType, array $payload): * * @return array{0:array,1:array} */ - private function hydrateConfigurationPolicySettings(string $policyType, string $tenantIdentifier, Tenant $tenant, string $policyId, array $payload, array $metadata): array + private function hydrateConfigurationPolicySettings(string $policyType, array $graphOptions, string $policyId, array $payload, array $metadata): array { $strategy = $this->contracts->memberHydrationStrategy($policyType); $settingsPath = $this->contracts->subresourceSettingsPath($policyType, $policyId); @@ -343,11 +344,7 @@ private function hydrateConfigurationPolicySettings(string $policyType, string $ $hydrationStatus = 'complete'; while ($nextPath) { - $response = $this->graphClient->request('GET', $nextPath, [ - 'tenant' => $tenantIdentifier, - 'client_id' => $tenant->app_client_id, - 'client_secret' => $tenant->app_client_secret, - ]); + $response = $this->graphClient->request('GET', $nextPath, Arr::except($graphOptions, ['platform'])); if ($response->failed()) { $hydrationStatus = 'failed'; @@ -389,7 +386,7 @@ private function hydrateConfigurationPolicySettings(string $policyType, string $ * * @return array{0:array,1:array} */ - private function hydrateGroupPolicyConfiguration(string $tenantIdentifier, Tenant $tenant, string $policyId, array $payload, array $metadata): array + private function hydrateGroupPolicyConfiguration(array $graphOptions, string $policyId, array $payload, array $metadata): array { $strategy = $this->contracts->memberHydrationStrategy('groupPolicyConfiguration'); $definitionValuesPath = $this->contracts->subresourcePath('groupPolicyConfiguration', 'definitionValues', [ @@ -407,11 +404,7 @@ private function hydrateGroupPolicyConfiguration(string $tenantIdentifier, Tenan $hydrationStatus = 'complete'; while ($nextPath) { - $response = $this->graphClient->request('GET', $nextPath, [ - 'tenant' => $tenantIdentifier, - 'client_id' => $tenant->app_client_id, - 'client_secret' => $tenant->app_client_secret, - ]); + $response = $this->graphClient->request('GET', $nextPath, Arr::except($graphOptions, ['platform'])); if ($response->failed()) { $hydrationStatus = 'failed'; @@ -482,11 +475,7 @@ private function hydrateGroupPolicyConfiguration(string $tenantIdentifier, Tenan $presentationNext = $presentationValuesPath; while ($presentationNext) { - $pvResponse = $this->graphClient->request('GET', $presentationNext, [ - 'tenant' => $tenantIdentifier, - 'client_id' => $tenant->app_client_id, - 'client_secret' => $tenant->app_client_secret, - ]); + $pvResponse = $this->graphClient->request('GET', $presentationNext, Arr::except($graphOptions, ['platform'])); if ($pvResponse->failed()) { $metadata['warnings'] = array_values(array_unique(array_merge( @@ -559,7 +548,7 @@ private function hydrateGroupPolicyConfiguration(string $tenantIdentifier, Tenan * * @return array{0:array,1:array} */ - private function hydrateComplianceActions(string $tenantIdentifier, Tenant $tenant, string $policyId, array $payload, array $metadata): array + private function hydrateComplianceActions(array $graphOptions, string $policyId, array $payload, array $metadata): array { $existingActions = $payload['scheduledActionsForRule'] ?? null; @@ -570,11 +559,7 @@ private function hydrateComplianceActions(string $tenantIdentifier, Tenant $tena } $path = sprintf('deviceManagement/deviceCompliancePolicies/%s/scheduledActionsForRule', urlencode($policyId)); - $options = [ - 'tenant' => $tenantIdentifier, - 'client_id' => $tenant->app_client_id, - 'client_secret' => $tenant->app_client_secret, - ]; + $options = Arr::except($graphOptions, ['platform']); $actions = []; $nextPath = $path; @@ -621,7 +606,7 @@ private function hydrateComplianceActions(string $tenantIdentifier, Tenant $tena * * @return array{0:array,1:array} */ - private function hydrateEnrollmentNotificationTemplates(string $tenantIdentifier, Tenant $tenant, array $payload, array $metadata): array + private function hydrateEnrollmentNotificationTemplates(array $graphOptions, array $payload, array $metadata): array { $existing = $payload['notificationTemplateSnapshots'] ?? null; @@ -639,11 +624,7 @@ private function hydrateEnrollmentNotificationTemplates(string $tenantIdentifier return [$payload, $metadata]; } - $options = [ - 'tenant' => $tenantIdentifier, - 'client_id' => $tenant->app_client_id, - 'client_secret' => $tenant->app_client_secret, - ]; + $options = Arr::except($graphOptions, ['platform']); $snapshots = []; $failures = 0; @@ -766,6 +747,34 @@ private function extractDefinitionIds(array $settings): array return array_unique($definitionIds); } + private function resolveProviderConnection(Tenant $tenant): ProviderConnection + { + $resolution = $this->providerConnections()->resolveDefault($tenant, 'microsoft'); + + if ($resolution->resolved && $resolution->connection instanceof ProviderConnection) { + return $resolution->connection; + } + + $reasonCode = $resolution->effectiveReasonCode(); + $reasonMessage = $resolution->message ?? 'Provider connection is not configured.'; + + throw new RuntimeException(sprintf( + '[%s] %s', + ProviderReasonCodes::isKnown($reasonCode) ? $reasonCode : ProviderReasonCodes::UnknownError, + $reasonMessage, + )); + } + + private function providerConnections(): ProviderConnectionResolver + { + return $this->providerConnections ?? app(ProviderConnectionResolver::class); + } + + private function providerGateway(): ProviderGateway + { + return $this->providerGateway ?? app(ProviderGateway::class); + } + private function stripGraphBaseUrl(string $nextLink): string { $base = rtrim(config('graph.base_url', 'https://graph.microsoft.com'), '/').'/'.trim(config('graph.version', 'beta'), '/'); diff --git a/app/Services/Intune/PolicySyncService.php b/app/Services/Intune/PolicySyncService.php index 891cbd6..b21d935 100644 --- a/app/Services/Intune/PolicySyncService.php +++ b/app/Services/Intune/PolicySyncService.php @@ -4,10 +4,15 @@ use App\Models\Policy; use App\Models\PolicyVersion; +use App\Models\ProviderConnection; use App\Models\Tenant; use App\Services\Graph\GraphClientInterface; use App\Services\Graph\GraphErrorMapper; use App\Services\Graph\GraphLogger; +use App\Services\Providers\ProviderConnectionResolver; +use App\Services\Providers\ProviderGateway; +use App\Support\Providers\ProviderNextStepsRegistry; +use App\Support\Providers\ProviderReasonCodes; use Illuminate\Support\Arr; use RuntimeException; use Throwable; @@ -17,6 +22,9 @@ class PolicySyncService public function __construct( private readonly GraphClientInterface $graphClient, private readonly GraphLogger $graphLogger, + private readonly ?ProviderConnectionResolver $providerConnections = null, + private readonly ?ProviderGateway $providerGateway = null, + private readonly ?ProviderNextStepsRegistry $nextStepsRegistry = null, ) {} /** @@ -46,7 +54,18 @@ public function syncPoliciesWithReport(Tenant $tenant, ?array $supportedTypes = $types = $supportedTypes ?? config('tenantpilot.supported_policy_types', []); $synced = []; $failures = []; - $tenantIdentifier = $tenant->tenant_id ?? $tenant->external_id; + + $resolution = $this->providerConnections()->resolveDefault($tenant, 'microsoft'); + + if (! $resolution->resolved || ! $resolution->connection instanceof ProviderConnection) { + return [ + 'synced' => [], + 'failures' => $this->blockedFailuresForTypes($tenant, $types, $resolution->effectiveReasonCode(), $resolution->message, $resolution->connection), + ]; + } + + $connection = $resolution->connection; + $tenantIdentifier = (string) $connection->entra_tenant_id; foreach ($types as $typeConfig) { $policyType = $typeConfig['type']; @@ -61,10 +80,7 @@ public function syncPoliciesWithReport(Tenant $tenant, ?array $supportedTypes = ]); try { - $response = $this->graphClient->listPolicies($policyType, [ - 'tenant' => $tenantIdentifier, - 'client_id' => $tenant->app_client_id, - 'client_secret' => $tenant->app_client_secret, + $response = $this->providerGateway()->listPolicies($connection, $policyType, [ 'platform' => $platform, 'filter' => $filter, ]); @@ -73,6 +89,7 @@ public function syncPoliciesWithReport(Tenant $tenant, ?array $supportedTypes = 'policy_type' => $policyType, 'tenant_id' => $tenant->id, 'tenant_identifier' => $tenantIdentifier, + 'provider_connection_id' => (int) $connection->getKey(), ]); } @@ -429,7 +446,17 @@ public function syncPolicy(Tenant $tenant, Policy $policy): void throw new RuntimeException('Tenant is archived or inactive.'); } - $tenantIdentifier = $tenant->tenant_id ?? $tenant->external_id; + $resolution = $this->providerConnections()->resolveDefault($tenant, 'microsoft'); + + if (! $resolution->resolved || ! $resolution->connection instanceof ProviderConnection) { + $reasonCode = $resolution->effectiveReasonCode(); + $reasonMessage = $resolution->message ?? 'Provider connection is not configured.'; + + throw new RuntimeException(sprintf('[%s] %s', $reasonCode, $reasonMessage)); + } + + $connection = $resolution->connection; + $tenantIdentifier = (string) $connection->entra_tenant_id; $this->graphLogger->logRequest('get_policy', [ 'tenant' => $tenantIdentifier, @@ -439,18 +466,21 @@ public function syncPolicy(Tenant $tenant, Policy $policy): void ]); try { - $response = $this->graphClient->getPolicy($policy->policy_type, $policy->external_id, [ - 'tenant' => $tenantIdentifier, - 'client_id' => $tenant->app_client_id, - 'client_secret' => $tenant->app_client_secret, - 'platform' => $policy->platform, - ]); + $response = $this->providerGateway()->getPolicy( + connection: $connection, + policyType: $policy->policy_type, + policyId: $policy->external_id, + options: [ + 'platform' => $policy->platform, + ], + ); } catch (Throwable $throwable) { throw GraphErrorMapper::fromThrowable($throwable, [ 'policy_type' => $policy->policy_type, 'policy_id' => $policy->external_id, 'tenant_id' => $tenant->id, 'tenant_identifier' => $tenantIdentifier, + 'provider_connection_id' => (int) $connection->getKey(), ]); } @@ -483,4 +513,68 @@ public function syncPolicy(Tenant $tenant, Policy $policy): void 'metadata' => Arr::except($payload, ['id', 'external_id', 'displayName', 'name', 'platform']), ])->save(); } + + /** + * @param array $types + * @return array + */ + private function blockedFailuresForTypes( + Tenant $tenant, + array $types, + string $reasonCode, + ?string $reasonMessage = null, + ?ProviderConnection $connection = null, + ): array { + $knownReasonCode = ProviderReasonCodes::isKnown($reasonCode) + ? $reasonCode + : ProviderReasonCodes::UnknownError; + + $message = sprintf( + '[%s] %s', + $knownReasonCode, + $reasonMessage ?? 'Provider connection is not configured.', + ); + + $nextSteps = $this->nextStepsRegistry()->forReason($tenant, $knownReasonCode, $connection); + $failures = []; + + foreach ($types as $typeConfig) { + if (! is_array($typeConfig)) { + continue; + } + + $policyType = $typeConfig['type'] ?? null; + + if (! is_string($policyType) || $policyType === '') { + continue; + } + + $failures[] = [ + 'policy_type' => $policyType, + 'status' => null, + 'errors' => [['message' => $message]], + 'meta' => [ + 'reason_code' => $knownReasonCode, + 'next_steps' => $nextSteps, + ], + ]; + } + + return $failures; + } + + private function providerConnections(): ProviderConnectionResolver + { + return $this->providerConnections ?? app(ProviderConnectionResolver::class); + } + + private function providerGateway(): ProviderGateway + { + return $this->providerGateway ?? app(ProviderGateway::class); + } + + private function nextStepsRegistry(): ProviderNextStepsRegistry + { + return $this->nextStepsRegistry ?? app(ProviderNextStepsRegistry::class); + } } diff --git a/app/Services/Intune/RbacHealthService.php b/app/Services/Intune/RbacHealthService.php index bf9ea90..bb3f7a2 100644 --- a/app/Services/Intune/RbacHealthService.php +++ b/app/Services/Intune/RbacHealthService.php @@ -2,14 +2,23 @@ namespace App\Services\Intune; +use App\Models\ProviderConnection; use App\Models\Tenant; use App\Services\Graph\GraphClientInterface; +use App\Services\Providers\ProviderConnectionResolver; +use App\Services\Providers\ProviderGateway; +use App\Support\Providers\ProviderReasonCodes; use App\Support\RbacReason; use Carbon\CarbonImmutable; +use RuntimeException; class RbacHealthService { - public function __construct(private readonly GraphClientInterface $graph) {} + public function __construct( + private readonly GraphClientInterface $graph, + private readonly ?ProviderConnectionResolver $providerConnections = null, + private readonly ?ProviderGateway $providerGateway = null, + ) {} /** * @return array{status:string,reason:?string,used_artifacts:bool} @@ -26,9 +35,20 @@ public function check(Tenant $tenant): array return $this->record($tenant, 'missing', RbacReason::MissingArtifacts->value, false); } - $context = $tenant->graphOptions(); + try { + $connection = $this->resolveProviderConnection($tenant); + } catch (RuntimeException) { + return $this->record($tenant, 'error', RbacReason::ServicePrincipalMissing->value, true); + } - $spId = $this->resolveServicePrincipalId($tenant, $context); + $context = $this->providerGateway()->graphOptions($connection); + $appClientId = is_string($context['client_id'] ?? null) ? (string) $context['client_id'] : null; + + if ($appClientId === null || $appClientId === '') { + return $this->record($tenant, 'error', RbacReason::ServicePrincipalMissing->value, true); + } + + $spId = $this->resolveServicePrincipalId($appClientId, $context); if (! $spId) { return $this->record($tenant, 'error', RbacReason::ServicePrincipalMissing->value, true); } @@ -99,11 +119,11 @@ private function record(Tenant $tenant, string $status, ?string $reason, bool $u ]; } - private function resolveServicePrincipalId(Tenant $tenant, array $context): ?string + private function resolveServicePrincipalId(string $appClientId, array $context): ?string { $response = $this->graph->request('GET', 'servicePrincipals', [ 'query' => [ - '$filter' => "appId eq '{$tenant->app_client_id}'", + '$filter' => "appId eq '{$appClientId}'", ], ] + $context); @@ -163,4 +183,32 @@ private function assignmentIncludesGroup(array $assignment, Tenant $tenant): boo return collect($members)->contains($expected); } + + private function resolveProviderConnection(Tenant $tenant): ProviderConnection + { + $resolution = $this->providerConnections()->resolveDefault($tenant, 'microsoft'); + + if ($resolution->resolved && $resolution->connection instanceof ProviderConnection) { + return $resolution->connection; + } + + $reasonCode = $resolution->effectiveReasonCode(); + $reasonMessage = $resolution->message ?? 'Provider connection is not configured.'; + + throw new RuntimeException(sprintf( + '[%s] %s', + ProviderReasonCodes::isKnown($reasonCode) ? $reasonCode : ProviderReasonCodes::UnknownError, + $reasonMessage, + )); + } + + private function providerConnections(): ProviderConnectionResolver + { + return $this->providerConnections ?? app(ProviderConnectionResolver::class); + } + + private function providerGateway(): ProviderGateway + { + return $this->providerGateway ?? app(ProviderGateway::class); + } } diff --git a/app/Services/Intune/RbacOnboardingService.php b/app/Services/Intune/RbacOnboardingService.php index 2545955..e00df36 100644 --- a/app/Services/Intune/RbacOnboardingService.php +++ b/app/Services/Intune/RbacOnboardingService.php @@ -2,9 +2,13 @@ namespace App\Services\Intune; +use App\Models\ProviderConnection; use App\Models\Tenant; use App\Models\User; use App\Services\Graph\GraphClientInterface; +use App\Services\Providers\ProviderConnectionResolver; +use App\Services\Providers\ProviderGateway; +use App\Support\Providers\ProviderReasonCodes; use App\Support\RbacReason; use Illuminate\Support\Facades\Log; use Illuminate\Support\Str; @@ -17,6 +21,8 @@ class RbacOnboardingService public function __construct( private readonly GraphClientInterface $graph, private readonly AuditLogger $auditLogger, + private readonly ?ProviderConnectionResolver $providerConnections = null, + private readonly ?ProviderGateway $providerGateway = null, ) {} /** @@ -31,10 +37,6 @@ public function run(Tenant $tenant, array $input, ?User $actor = null, ?string $ return $this->failure($tenant, 'Tenant is not active', $actor); } - if (empty($tenant->app_client_id)) { - return $this->failure($tenant, 'Tenant is missing app_client_id', $actor); - } - if (empty($accessToken)) { return $this->failure($tenant, 'Delegated access token missing. Please sign in first.', $actor); } @@ -46,8 +48,32 @@ public function run(Tenant $tenant, array $input, ?User $actor = null, ?string $ return $this->failure($tenant, 'Select an Intune RBAC role (roleDefinitionId required). Login to load roles.', $actor); } - $context = $tenant->graphOptions(); + $resolution = $this->providerConnections()->resolveDefault($tenant, 'microsoft'); + + if (! $resolution->resolved || ! $resolution->connection instanceof ProviderConnection) { + $reasonCode = $resolution->effectiveReasonCode(); + $reasonMessage = $resolution->message ?? 'Provider connection is not configured.'; + + return $this->failure( + $tenant, + sprintf( + '[%s] %s', + ProviderReasonCodes::isKnown($reasonCode) ? $reasonCode : ProviderReasonCodes::UnknownError, + $reasonMessage, + ), + $actor + ); + } + + $connection = $resolution->connection; + $context = $this->providerGateway()->graphOptions($connection); $context['access_token'] = $accessToken; + $appClientId = is_string($context['client_id'] ?? null) ? (string) $context['client_id'] : null; + + if ($appClientId === null || $appClientId === '') { + return $this->failure($tenant, 'Provider credential is missing client_id.', $actor); + } + $result = [ 'status' => 'success', 'warnings' => [], @@ -64,10 +90,11 @@ public function run(Tenant $tenant, array $input, ?User $actor = null, ?string $ 'role_definition_id' => $roleDefinitionId, 'role_display_name' => $roleDisplayName, 'scope' => $input['scope'] ?? null, + 'provider_connection_id' => (int) $connection->getKey(), ], 'success', $actor); try { - $servicePrincipal = $this->resolveServicePrincipal($tenant->app_client_id, $context); + $servicePrincipal = $this->resolveServicePrincipal($appClientId, $context); $result['service_principal_id'] = $servicePrincipal['id']; $result['steps'][] = 'service_principal_resolved'; @@ -180,7 +207,7 @@ private function resolveServicePrincipal(string $appClientId, array $context): a $servicePrincipal = $response->data['value'][0] ?? null; if (! $servicePrincipal || empty($servicePrincipal['id'])) { - throw new RuntimeException('Service principal not found for app_client_id'); + throw new RuntimeException('Service principal not found for provider connection client_id'); } return $servicePrincipal; @@ -740,6 +767,16 @@ private function failure(Tenant $tenant, string $message, ?User $actor = null): ]; } + private function providerConnections(): ProviderConnectionResolver + { + return $this->providerConnections ?? app(ProviderConnectionResolver::class); + } + + private function providerGateway(): ProviderGateway + { + return $this->providerGateway ?? app(ProviderGateway::class); + } + private function audit(Tenant $tenant, string $action, array $context, string $status, ?User $actor = null): void { $this->auditLogger->log( diff --git a/app/Services/Intune/RestoreService.php b/app/Services/Intune/RestoreService.php index 6c00464..319602e 100644 --- a/app/Services/Intune/RestoreService.php +++ b/app/Services/Intune/RestoreService.php @@ -6,6 +6,7 @@ use App\Models\BackupSet; use App\Models\Policy; use App\Models\PolicyVersion; +use App\Models\ProviderConnection; use App\Models\RestoreRun; use App\Models\Tenant; use App\Services\AssignmentRestoreService; @@ -13,9 +14,13 @@ use App\Services\Graph\GraphContractRegistry; use App\Services\Graph\GraphErrorMapper; use App\Services\Graph\GraphLogger; +use App\Services\Providers\ProviderConnectionResolver; +use App\Services\Providers\ProviderGateway; +use App\Support\Providers\ProviderReasonCodes; use Carbon\CarbonImmutable; use Illuminate\Support\Arr; use Illuminate\Support\Collection; +use RuntimeException; use Throwable; class RestoreService @@ -30,6 +35,8 @@ public function __construct( private readonly ConfigurationPolicyTemplateResolver $templateResolver, private readonly AssignmentRestoreService $assignmentRestoreService, private readonly FoundationMappingService $foundationMappingService, + private readonly ?ProviderConnectionResolver $providerConnections = null, + private readonly ?ProviderGateway $providerGateway = null, ) {} /** @@ -253,6 +260,14 @@ public function execute( } $tenantIdentifier = $tenant->tenant_id ?? $tenant->external_id; + $baseGraphOptions = []; + + if (! $dryRun) { + $connection = $this->resolveProviderConnection($tenant); + $tenantIdentifier = (string) $connection->entra_tenant_id; + $baseGraphOptions = $this->providerGateway()->graphOptions($connection); + } + $items = $this->loadItems($backupSet, $selectedItemIds); [$foundationItems, $policyItems] = $this->splitItems($items); $preview = $this->preview($tenant, $backupSet, $selectedItemIds); @@ -435,12 +450,7 @@ public function execute( $payload = $this->contracts->sanitizeUpdatePayload($item->policy_type, $originalPayload); $payload = $this->applyScopeTagIdsToPayload($payload, $mappedScopeTagIds, $scopeTagMapping); - $graphOptions = [ - 'tenant' => $tenantIdentifier, - 'client_id' => $tenant->app_client_id, - 'client_secret' => $tenant->app_client_secret, - 'platform' => $item->platform, - ]; + $graphOptions = ['platform' => $item->platform] + $baseGraphOptions; $updateMethod = $this->resolveUpdateMethod($item->policy_type); $settingsApply = null; @@ -2688,6 +2698,34 @@ private function buildScopeTagsForVersion( ]; } + private function resolveProviderConnection(Tenant $tenant): ProviderConnection + { + $resolution = $this->providerConnections()->resolveDefault($tenant, 'microsoft'); + + if ($resolution->resolved && $resolution->connection instanceof ProviderConnection) { + return $resolution->connection; + } + + $reasonCode = $resolution->effectiveReasonCode(); + $reasonMessage = $resolution->message ?? 'Provider connection is not configured.'; + + throw new RuntimeException(sprintf( + '[%s] %s', + ProviderReasonCodes::isKnown($reasonCode) ? $reasonCode : ProviderReasonCodes::UnknownError, + $reasonMessage, + )); + } + + private function providerConnections(): ProviderConnectionResolver + { + return $this->providerConnections ?? app(ProviderConnectionResolver::class); + } + + private function providerGateway(): ProviderGateway + { + return $this->providerGateway ?? app(ProviderGateway::class); + } + private function assertActiveContext(Tenant $tenant, BackupSet $backupSet): void { if (! $tenant->isActive()) { diff --git a/app/Services/Inventory/InventorySyncService.php b/app/Services/Inventory/InventorySyncService.php index cc16c9a..85066f5 100644 --- a/app/Services/Inventory/InventorySyncService.php +++ b/app/Services/Inventory/InventorySyncService.php @@ -4,25 +4,30 @@ use App\Models\InventoryItem; use App\Models\InventorySyncRun; +use App\Models\ProviderConnection; use App\Models\Tenant; use App\Models\User; use App\Services\BackupScheduling\PolicyTypeResolver; -use App\Services\Graph\GraphClientInterface; use App\Services\Graph\GraphResponse; +use App\Services\Providers\ProviderConnectionResolver; +use App\Services\Providers\ProviderGateway; +use App\Support\Providers\ProviderReasonCodes; use Carbon\CarbonImmutable; use Illuminate\Contracts\Cache\Lock; use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Log; +use RuntimeException; use Throwable; class InventorySyncService { public function __construct( - private readonly GraphClientInterface $graphClient, private readonly PolicyTypeResolver $policyTypeResolver, private readonly InventorySelectionHasher $selectionHasher, private readonly InventoryMetaSanitizer $metaSanitizer, private readonly InventoryConcurrencyLimiter $concurrencyLimiter, + private readonly ProviderConnectionResolver $providerConnections, + private readonly ProviderGateway $providerGateway, ) {} /** @@ -243,6 +248,7 @@ private function executeRun(InventorySyncRun $run, Tenant $tenant, array $normal $warnings = []; try { + $connection = $this->resolveProviderConnection($tenant); $typesConfig = $this->supportedTypeConfigByType(); $policyTypes = $normalizedSelection['policy_types'] ?? []; @@ -266,13 +272,14 @@ private function executeRun(InventorySyncRun $run, Tenant $tenant, array $normal continue; } - $response = $this->listPoliciesWithRetry($policyType, [ - 'tenant' => $tenant->tenant_id ?? $tenant->external_id, - 'client_id' => $tenant->app_client_id, - 'client_secret' => $tenant->app_client_secret, - 'platform' => $typeConfig['platform'] ?? null, - 'filter' => $typeConfig['filter'] ?? null, - ]); + $response = $this->listPoliciesWithRetry( + $policyType, + [ + 'platform' => $typeConfig['platform'] ?? null, + 'filter' => $typeConfig['filter'] ?? null, + ], + $connection + ); if ($response->failed()) { $hadErrors = true; @@ -305,7 +312,7 @@ private function executeRun(InventorySyncRun $run, Tenant $tenant, array $normal if ($includeDeps && $this->shouldHydrateAssignments($policyType)) { $existingAssignments = $policyData['assignments'] ?? null; if (! is_array($existingAssignments) || count($existingAssignments) === 0) { - $hydratedAssignments = $this->fetchAssignmentsForPolicyType($policyType, $tenant, $externalId, $warnings); + $hydratedAssignments = $this->fetchAssignmentsForPolicyType($policyType, $connection, $externalId, $warnings); if (is_array($hydratedAssignments)) { $policyData['assignments'] = $hydratedAssignments; } @@ -416,7 +423,7 @@ private function shouldHydrateAssignments(string $policyType): bool * @param array> $warnings * @return null|array */ - private function fetchAssignmentsForPolicyType(string $policyType, Tenant $tenant, string $externalId, array &$warnings): ?array + private function fetchAssignmentsForPolicyType(string $policyType, ProviderConnection $connection, string $externalId, array &$warnings): ?array { $pathTemplate = config("graph_contracts.types.{$policyType}.assignments_list_path"); if (! is_string($pathTemplate) || $pathTemplate === '') { @@ -425,16 +432,10 @@ private function fetchAssignmentsForPolicyType(string $policyType, Tenant $tenan $path = str_replace('{id}', $externalId, $pathTemplate); - $options = [ - 'tenant' => $tenant->tenant_id ?? $tenant->external_id, - 'client_id' => $tenant->app_client_id, - 'client_secret' => $tenant->app_client_secret, - ]; - $maxAttempts = 3; for ($attempt = 1; $attempt <= $maxAttempts; $attempt++) { - $response = $this->graphClient->request('GET', $path, $options); + $response = $this->providerGateway->request($connection, 'GET', $path); if (! $response->failed()) { $data = $response->data; @@ -611,12 +612,12 @@ private function mapGraphFailureToErrorCode(GraphResponse $response): string }; } - private function listPoliciesWithRetry(string $policyType, array $options): GraphResponse + private function listPoliciesWithRetry(string $policyType, array $options, ProviderConnection $connection): GraphResponse { $maxAttempts = 3; for ($attempt = 1; $attempt <= $maxAttempts; $attempt++) { - $response = $this->graphClient->listPolicies($policyType, $options); + $response = $this->providerGateway->listPolicies($connection, $policyType, $options); if (! $response->failed()) { return $response; @@ -639,6 +640,27 @@ private function listPoliciesWithRetry(string $policyType, array $options): Grap return new GraphResponse(false, [], null, ['error' => ['code' => 'unexpected_exception', 'message' => 'retry loop failed']]); } + /** + * @throws RuntimeException + */ + private function resolveProviderConnection(Tenant $tenant): ProviderConnection + { + $resolution = $this->providerConnections->resolveDefault($tenant, 'microsoft'); + + if (! $resolution->resolved || ! $resolution->connection instanceof ProviderConnection) { + $reasonCode = $resolution->effectiveReasonCode(); + $reasonMessage = $resolution->message ?? 'Provider connection is not configured.'; + + throw new RuntimeException(sprintf( + '[%s] %s', + ProviderReasonCodes::isKnown($reasonCode) ? $reasonCode : ProviderReasonCodes::UnknownError, + $reasonMessage, + )); + } + + return $resolution->connection; + } + /** * @return array */ diff --git a/app/Services/OperationRunService.php b/app/Services/OperationRunService.php index d060404..e369742 100644 --- a/app/Services/OperationRunService.php +++ b/app/Services/OperationRunService.php @@ -557,6 +557,44 @@ public function failRun(OperationRun $run, Throwable $e): OperationRun ); } + /** + * Finalize a run as blocked with deterministic reason_code + link-only next steps. + * + * @param array $nextSteps + */ + public function finalizeBlockedRun( + OperationRun $run, + string $reasonCode, + array $nextSteps = [], + ?string $message = null, + ): OperationRun { + $reasonCode = RunFailureSanitizer::normalizeReasonCode($reasonCode); + $nextSteps = $this->sanitizeNextSteps($nextSteps); + + $context = is_array($run->context) ? $run->context : []; + $context['reason_code'] = $reasonCode; + $context['next_steps'] = $nextSteps; + + $run->update([ + 'context' => $context, + ]); + + $run->refresh(); + + return $this->updateRun( + $run, + status: OperationRunStatus::Completed->value, + outcome: OperationRunOutcome::Blocked->value, + failures: [ + [ + 'code' => 'operation.blocked', + 'reason_code' => $reasonCode, + 'message' => $message ?? 'Operation blocked due to provider configuration.', + ], + ], + ); + } + private function invokeDispatcher(callable $dispatcher, OperationRun $run): void { $ref = null; @@ -683,4 +721,29 @@ protected function sanitizeSummaryCounts(array $summaryCounts): array { return SummaryCountsNormalizer::normalize($summaryCounts); } + + /** + * @param array $nextSteps + * @return array + */ + protected function sanitizeNextSteps(array $nextSteps): array + { + $sanitized = []; + + foreach ($nextSteps as $nextStep) { + $label = is_string($nextStep['label'] ?? null) ? trim((string) $nextStep['label']) : ''; + $url = is_string($nextStep['url'] ?? null) ? trim((string) $nextStep['url']) : ''; + + if ($label === '' || $url === '') { + continue; + } + + $sanitized[] = [ + 'label' => substr($label, 0, 120), + 'url' => substr($url, 0, 2048), + ]; + } + + return $sanitized; + } } diff --git a/app/Services/Providers/MicrosoftProviderHealthCheck.php b/app/Services/Providers/MicrosoftProviderHealthCheck.php index 9c482b0..012bbc1 100644 --- a/app/Services/Providers/MicrosoftProviderHealthCheck.php +++ b/app/Services/Providers/MicrosoftProviderHealthCheck.php @@ -7,6 +7,7 @@ use App\Services\Providers\Contracts\HealthResult; use App\Services\Providers\Contracts\ProviderHealthCheck; use App\Support\OpsUx\RunFailureSanitizer; +use App\Support\Providers\ProviderReasonCodes; use Throwable; final class MicrosoftProviderHealthCheck implements ProviderHealthCheck @@ -56,14 +57,15 @@ public function check(ProviderConnection $connection): HealthResult private function reasonCodeForResponse(GraphResponse $response): string { - return match ((int) ($response->status ?? 0)) { - 401 => RunFailureSanitizer::REASON_PROVIDER_AUTH_FAILED, - 403 => RunFailureSanitizer::REASON_PERMISSION_DENIED, - 429 => RunFailureSanitizer::REASON_GRAPH_THROTTLED, - 500, 502 => RunFailureSanitizer::REASON_PROVIDER_OUTAGE, - 503, 504 => RunFailureSanitizer::REASON_GRAPH_TIMEOUT, - default => RunFailureSanitizer::REASON_UNKNOWN_ERROR, + $candidate = match ((int) ($response->status ?? 0)) { + 401 => ProviderReasonCodes::ProviderAuthFailed, + 403 => ProviderReasonCodes::ProviderPermissionDenied, + 429 => ProviderReasonCodes::RateLimited, + 500, 502, 503, 504 => ProviderReasonCodes::NetworkUnreachable, + default => ProviderReasonCodes::UnknownError, }; + + return RunFailureSanitizer::normalizeReasonCode($candidate); } private function messageForResponse(GraphResponse $response): string @@ -90,8 +92,9 @@ private function messageForResponse(GraphResponse $response): string private function statusForReason(string $reasonCode): string { return match ($reasonCode) { - RunFailureSanitizer::REASON_PROVIDER_AUTH_FAILED, - RunFailureSanitizer::REASON_PERMISSION_DENIED => 'needs_consent', + ProviderReasonCodes::ProviderAuthFailed, + ProviderReasonCodes::ProviderPermissionDenied, + ProviderReasonCodes::ProviderConsentMissing => 'needs_consent', default => 'error', }; } @@ -99,11 +102,10 @@ private function statusForReason(string $reasonCode): string private function healthForReason(string $reasonCode): string { return match ($reasonCode) { - RunFailureSanitizer::REASON_GRAPH_THROTTLED => 'degraded', - RunFailureSanitizer::REASON_GRAPH_TIMEOUT, - RunFailureSanitizer::REASON_PROVIDER_OUTAGE => 'down', - RunFailureSanitizer::REASON_PROVIDER_AUTH_FAILED, - RunFailureSanitizer::REASON_PERMISSION_DENIED => 'down', + ProviderReasonCodes::RateLimited => 'degraded', + ProviderReasonCodes::NetworkUnreachable, + ProviderReasonCodes::ProviderAuthFailed, + ProviderReasonCodes::ProviderPermissionDenied => 'down', default => 'down', }; } diff --git a/app/Services/Providers/ProviderConnectionResolution.php b/app/Services/Providers/ProviderConnectionResolution.php new file mode 100644 index 0000000..fcca20b --- /dev/null +++ b/app/Services/Providers/ProviderConnectionResolution.php @@ -0,0 +1,48 @@ +reasonCode ?? ProviderReasonCodes::UnknownError; + } +} diff --git a/app/Services/Providers/ProviderConnectionResolver.php b/app/Services/Providers/ProviderConnectionResolver.php new file mode 100644 index 0000000..c59010d --- /dev/null +++ b/app/Services/Providers/ProviderConnectionResolver.php @@ -0,0 +1,124 @@ +where('tenant_id', (int) $tenant->getKey()) + ->where('provider', $provider) + ->where('is_default', true) + ->orderBy('id') + ->get(); + + if ($defaults->count() === 0) { + return ProviderConnectionResolution::blocked( + ProviderReasonCodes::ProviderConnectionMissing, + 'No default provider connection is configured for this tenant/provider.', + ); + } + + if ($defaults->count() > 1) { + return ProviderConnectionResolution::blocked( + ProviderReasonCodes::ProviderConnectionInvalid, + 'Multiple default provider connections were detected.', + 'ext.multiple_defaults_detected', + ); + } + + /** @var ProviderConnection $connection */ + $connection = $defaults->first(); + + return $this->validateConnection($tenant, $provider, $connection); + } + + public function validateConnection(Tenant $tenant, string $provider, ProviderConnection $connection): ProviderConnectionResolution + { + if ((int) $connection->tenant_id !== (int) $tenant->getKey() || (string) $connection->provider !== $provider) { + return ProviderConnectionResolution::blocked( + ProviderReasonCodes::ProviderConnectionInvalid, + 'Provider connection does not match tenant/provider scope.', + 'ext.connection_scope_mismatch', + $connection, + ); + } + + if ((string) $connection->status === 'disabled') { + return ProviderConnectionResolution::blocked( + ProviderReasonCodes::ProviderConnectionInvalid, + 'Provider connection is disabled.', + 'ext.connection_disabled', + $connection, + ); + } + + if ((string) $connection->status === 'needs_consent') { + return ProviderConnectionResolution::blocked( + ProviderReasonCodes::ProviderConsentMissing, + 'Provider connection requires admin consent before use.', + 'ext.connection_needs_consent', + $connection, + ); + } + + if ($connection->entra_tenant_id === null || trim((string) $connection->entra_tenant_id) === '') { + return ProviderConnectionResolution::blocked( + ProviderReasonCodes::ProviderConnectionInvalid, + 'Provider connection is missing target tenant scope.', + 'ext.connection_tenant_missing', + $connection, + ); + } + + $credential = $connection->credential()->first(); + + if (! $credential instanceof ProviderCredential) { + return ProviderConnectionResolution::blocked( + ProviderReasonCodes::ProviderCredentialMissing, + 'Provider connection is missing credentials.', + connection: $connection, + ); + } + + if ($credential->type !== 'client_secret') { + return ProviderConnectionResolution::blocked( + ProviderReasonCodes::ProviderCredentialInvalid, + 'Provider credential type is invalid.', + 'ext.invalid_credential_type', + $connection, + ); + } + + $payload = $credential->payload; + + if (! is_array($payload)) { + return ProviderConnectionResolution::blocked( + ProviderReasonCodes::ProviderCredentialInvalid, + 'Provider credential payload is invalid.', + 'ext.invalid_credential_payload', + $connection, + ); + } + + $clientId = trim((string) ($payload['client_id'] ?? '')); + $clientSecret = trim((string) ($payload['client_secret'] ?? '')); + + if ($clientId === '' || $clientSecret === '') { + return ProviderConnectionResolution::blocked( + ProviderReasonCodes::ProviderCredentialInvalid, + 'Provider credential payload is missing required fields.', + 'ext.missing_credential_fields', + $connection, + ); + } + + return ProviderConnectionResolution::resolved($connection); + } +} diff --git a/app/Services/Providers/ProviderGateway.php b/app/Services/Providers/ProviderGateway.php index f099173..5fec292 100644 --- a/app/Services/Providers/ProviderGateway.php +++ b/app/Services/Providers/ProviderGateway.php @@ -19,11 +19,31 @@ public function getOrganization(ProviderConnection $connection): GraphResponse return $this->graph->getOrganization($this->graphOptions($connection)); } + public function getPolicy(ProviderConnection $connection, string $policyType, string $policyId, array $options = []): GraphResponse + { + return $this->graph->getPolicy($policyType, $policyId, $this->graphOptions($connection, $options)); + } + public function listPolicies(ProviderConnection $connection, string $policyType, array $options = []): GraphResponse { return $this->graph->listPolicies($policyType, $this->graphOptions($connection, $options)); } + public function applyPolicy( + ProviderConnection $connection, + string $policyType, + string $policyId, + array $payload, + array $options = [], + ): GraphResponse { + return $this->graph->applyPolicy($policyType, $policyId, $payload, $this->graphOptions($connection, $options)); + } + + public function getServicePrincipalPermissions(ProviderConnection $connection, array $options = []): GraphResponse + { + return $this->graph->getServicePrincipalPermissions($this->graphOptions($connection, $options)); + } + public function request(ProviderConnection $connection, string $method, string $path, array $options = []): GraphResponse { return $this->graph->request($method, $path, $this->graphOptions($connection, $options)); diff --git a/app/Services/Providers/ProviderOperationStartGate.php b/app/Services/Providers/ProviderOperationStartGate.php index 24ebca2..6fe4d7c 100644 --- a/app/Services/Providers/ProviderOperationStartGate.php +++ b/app/Services/Providers/ProviderOperationStartGate.php @@ -7,6 +7,8 @@ use App\Models\Tenant; use App\Models\User; use App\Services\OperationRunService; +use App\Support\Providers\ProviderNextStepsRegistry; +use App\Support\Providers\ProviderReasonCodes; use Illuminate\Support\Facades\DB; use InvalidArgumentException; use ReflectionFunction; @@ -17,6 +19,8 @@ final class ProviderOperationStartGate public function __construct( private readonly OperationRunService $runs, private readonly ProviderOperationRegistry $registry, + private readonly ProviderConnectionResolver $resolver, + private readonly ProviderNextStepsRegistry $nextStepsRegistry, ) {} /** @@ -24,19 +28,39 @@ public function __construct( */ public function start( Tenant $tenant, - ProviderConnection $connection, + ?ProviderConnection $connection, string $operationType, callable $dispatcher, ?User $initiator = null, array $extraContext = [], ): ProviderOperationStartResult { - if ((int) $connection->tenant_id !== (int) $tenant->getKey()) { - throw new InvalidArgumentException('ProviderConnection does not belong to the given tenant.'); + $definition = $this->registry->get($operationType); + $resolution = $connection instanceof ProviderConnection + ? $this->resolver->validateConnection($tenant, (string) $definition['provider'], $connection) + : $this->resolver->resolveDefault($tenant, (string) $definition['provider']); + + if (! $resolution->resolved || ! $resolution->connection instanceof ProviderConnection) { + return $this->startBlocked( + tenant: $tenant, + operationType: $operationType, + provider: (string) $definition['provider'], + module: (string) $definition['module'], + reasonCode: $resolution->effectiveReasonCode(), + extensionReasonCode: $resolution->extensionReasonCode, + reasonMessage: $resolution->message, + connection: $resolution->connection ?? $connection, + initiator: $initiator, + extraContext: $extraContext, + ); } - $definition = $this->registry->get($operationType); + return DB::transaction(function () use ($tenant, $operationType, $dispatcher, $initiator, $extraContext, $definition, $resolution): ProviderOperationStartResult { + $connection = $resolution->connection; + + if (! $connection instanceof ProviderConnection) { + throw new InvalidArgumentException('Resolved provider connection is missing.'); + } - return DB::transaction(function () use ($tenant, $connection, $operationType, $dispatcher, $initiator, $extraContext, $definition): ProviderOperationStartResult { $lockedConnection = ProviderConnection::query() ->whereKey($connection->getKey()) ->lockForUpdate() @@ -87,6 +111,62 @@ public function start( }); } + /** + * @param array $extraContext + */ + private function startBlocked( + Tenant $tenant, + string $operationType, + string $provider, + string $module, + string $reasonCode, + ?string $extensionReasonCode = null, + ?string $reasonMessage = null, + ?ProviderConnection $connection = null, + ?User $initiator = null, + array $extraContext = [], + ): ProviderOperationStartResult { + $context = array_merge($extraContext, [ + 'provider' => $provider, + 'module' => $module, + 'target_scope' => [ + 'entra_tenant_id' => $tenant->graphTenantId(), + ], + ]); + + $identityInputs = [ + 'provider' => $provider, + 'reason_code' => $reasonCode, + ]; + + if (is_string($extensionReasonCode) && $extensionReasonCode !== '') { + $context['reason_code_extension'] = $extensionReasonCode; + $identityInputs['reason_code_extension'] = $extensionReasonCode; + } + + if ($connection instanceof ProviderConnection) { + $context['provider_connection_id'] = (int) $connection->getKey(); + $identityInputs['provider_connection_id'] = (int) $connection->getKey(); + } + + $run = $this->runs->ensureRunWithIdentity( + tenant: $tenant, + type: $operationType, + identityInputs: $identityInputs, + context: $context, + initiator: $initiator, + ); + + $run = $this->runs->finalizeBlockedRun( + $run, + reasonCode: ProviderReasonCodes::isKnown($reasonCode) ? $reasonCode : ProviderReasonCodes::UnknownError, + nextSteps: $this->nextStepsRegistry->forReason($tenant, $reasonCode, $connection), + message: $reasonMessage, + ); + + return ProviderOperationStartResult::blocked($run); + } + private function invokeDispatcher(callable $dispatcher, OperationRun $run): void { $ref = null; diff --git a/app/Services/Providers/ProviderOperationStartResult.php b/app/Services/Providers/ProviderOperationStartResult.php index ddcf79e..7285172 100644 --- a/app/Services/Providers/ProviderOperationStartResult.php +++ b/app/Services/Providers/ProviderOperationStartResult.php @@ -26,4 +26,9 @@ public static function scopeBusy(OperationRun $run): self { return new self('scope_busy', $run, false); } + + public static function blocked(OperationRun $run): self + { + return new self('blocked', $run, false); + } } diff --git a/app/Support/Badges/Domains/OperationRunOutcomeBadge.php b/app/Support/Badges/Domains/OperationRunOutcomeBadge.php index cb1cc27..7c8186a 100644 --- a/app/Support/Badges/Domains/OperationRunOutcomeBadge.php +++ b/app/Support/Badges/Domains/OperationRunOutcomeBadge.php @@ -17,6 +17,7 @@ public function spec(mixed $value): BadgeSpec OperationRunOutcome::Pending->value => new BadgeSpec('Pending', 'gray', 'heroicon-m-clock'), OperationRunOutcome::Succeeded->value => new BadgeSpec('Succeeded', 'success', 'heroicon-m-check-circle'), OperationRunOutcome::PartiallySucceeded->value => new BadgeSpec('Partially succeeded', 'warning', 'heroicon-m-exclamation-triangle'), + OperationRunOutcome::Blocked->value => new BadgeSpec('Blocked', 'warning', 'heroicon-m-no-symbol'), OperationRunOutcome::Failed->value => new BadgeSpec('Failed', 'danger', 'heroicon-m-x-circle'), OperationRunOutcome::Cancelled->value => new BadgeSpec('Cancelled', 'gray', 'heroicon-m-minus-circle'), default => BadgeSpec::unknown(), diff --git a/app/Support/OperationRunOutcome.php b/app/Support/OperationRunOutcome.php index 951b0a3..d91cbbd 100644 --- a/app/Support/OperationRunOutcome.php +++ b/app/Support/OperationRunOutcome.php @@ -7,6 +7,7 @@ enum OperationRunOutcome: string case Pending = 'pending'; case Succeeded = 'succeeded'; case PartiallySucceeded = 'partially_succeeded'; + case Blocked = 'blocked'; case Failed = 'failed'; /** @@ -31,6 +32,7 @@ public static function uiLabels(bool $includeReserved = false): array self::Pending->value => 'Pending', self::Succeeded->value => 'Succeeded', self::PartiallySucceeded->value => 'Partially succeeded', + self::Blocked->value => 'Blocked', self::Failed->value => 'Failed', ]; diff --git a/app/Support/OpsUx/RunFailureSanitizer.php b/app/Support/OpsUx/RunFailureSanitizer.php index 1bba444..4026911 100644 --- a/app/Support/OpsUx/RunFailureSanitizer.php +++ b/app/Support/OpsUx/RunFailureSanitizer.php @@ -2,6 +2,8 @@ namespace App\Support\OpsUx; +use App\Support\Providers\ProviderReasonCodes; + final class RunFailureSanitizer { public const string REASON_GRAPH_THROTTLED = 'graph_throttled'; @@ -36,66 +38,81 @@ public static function normalizeReasonCode(string $candidate): string $candidate = strtolower(trim($candidate)); if ($candidate === '') { - return self::REASON_UNKNOWN_ERROR; + return ProviderReasonCodes::UnknownError; } - $allowed = [ - self::REASON_GRAPH_THROTTLED, - self::REASON_GRAPH_TIMEOUT, - self::REASON_PERMISSION_DENIED, - self::REASON_PROVIDER_AUTH_FAILED, - self::REASON_PROVIDER_OUTAGE, - self::REASON_VALIDATION_ERROR, - self::REASON_CONFLICT_DETECTED, - self::REASON_UNKNOWN_ERROR, - ]; - - if (in_array($candidate, $allowed, true)) { + if (ProviderReasonCodes::isKnown($candidate) || in_array($candidate, ['ok', 'not_applicable'], true)) { return $candidate; } + if (str_starts_with($candidate, 'ext.')) { + return $candidate; + } + + /** + * Appendix-A taxonomy mappings: + * - `graph_throttled`/`throttled` -> `rate_limited` + * - transport/transient/outage classes -> `network_unreachable` + * - auth failures -> `provider_auth_failed` + * - permission denied classes -> `provider_permission_denied` + * - generic missing/invalid configuration classes -> `provider_connection_*` + */ // Compatibility mappings from existing codebase labels. $candidate = match ($candidate) { - 'graph_forbidden' => self::REASON_PERMISSION_DENIED, - 'graph_transient' => self::REASON_GRAPH_TIMEOUT, - 'unknown' => self::REASON_UNKNOWN_ERROR, + self::REASON_GRAPH_THROTTLED, + 'throttled' => ProviderReasonCodes::RateLimited, + self::REASON_GRAPH_TIMEOUT, + self::REASON_PROVIDER_OUTAGE, + 'graph_transient', + 'dependency_unreachable' => ProviderReasonCodes::NetworkUnreachable, + self::REASON_PERMISSION_DENIED, + 'graph_forbidden', + 'permission_denied' => ProviderReasonCodes::ProviderPermissionDenied, + self::REASON_PROVIDER_AUTH_FAILED, + 'authentication_failed' => ProviderReasonCodes::ProviderAuthFailed, + self::REASON_VALIDATION_ERROR, + self::REASON_CONFLICT_DETECTED, + 'invalid_state' => ProviderReasonCodes::ProviderConnectionInvalid, + 'missing_configuration' => ProviderReasonCodes::ProviderConnectionMissing, + 'unknown', + self::REASON_UNKNOWN_ERROR => ProviderReasonCodes::UnknownError, default => $candidate, }; - if (in_array($candidate, $allowed, true)) { + if (ProviderReasonCodes::isKnown($candidate) || in_array($candidate, ['ok', 'not_applicable'], true)) { return $candidate; } // Heuristic normalization for ad-hoc codes used across jobs/services. if (str_contains($candidate, 'throttle') || str_contains($candidate, '429')) { - return self::REASON_GRAPH_THROTTLED; + return ProviderReasonCodes::RateLimited; } if (str_contains($candidate, 'invalid_client') || str_contains($candidate, 'invalid_grant') || str_contains($candidate, '401') || str_contains($candidate, 'aadsts')) { - return self::REASON_PROVIDER_AUTH_FAILED; + return ProviderReasonCodes::ProviderAuthFailed; } if (str_contains($candidate, 'timeout') || str_contains($candidate, 'transient') || str_contains($candidate, '503') || str_contains($candidate, '504')) { - return self::REASON_GRAPH_TIMEOUT; + return ProviderReasonCodes::NetworkUnreachable; } if (str_contains($candidate, 'outage') || str_contains($candidate, '500') || str_contains($candidate, '502') || str_contains($candidate, 'bad_gateway')) { - return self::REASON_PROVIDER_OUTAGE; + return ProviderReasonCodes::NetworkUnreachable; } if (str_contains($candidate, 'forbidden') || str_contains($candidate, 'permission') || str_contains($candidate, 'unauthorized') || str_contains($candidate, '403')) { - return self::REASON_PERMISSION_DENIED; + return ProviderReasonCodes::ProviderPermissionDenied; } if (str_contains($candidate, 'validation') || str_contains($candidate, 'not_found') || str_contains($candidate, 'bad_request') || str_contains($candidate, '400') || str_contains($candidate, '422')) { - return self::REASON_VALIDATION_ERROR; + return ProviderReasonCodes::ProviderConnectionInvalid; } if (str_contains($candidate, 'conflict') || str_contains($candidate, '409')) { - return self::REASON_CONFLICT_DETECTED; + return ProviderReasonCodes::ProviderConnectionInvalid; } - return self::REASON_UNKNOWN_ERROR; + return ProviderReasonCodes::UnknownError; } public static function sanitizeMessage(string $message): string diff --git a/app/Support/Providers/ProviderNextStepsRegistry.php b/app/Support/Providers/ProviderNextStepsRegistry.php new file mode 100644 index 0000000..a5cebd9 --- /dev/null +++ b/app/Support/Providers/ProviderNextStepsRegistry.php @@ -0,0 +1,63 @@ + + */ + public function forReason(Tenant $tenant, string $reasonCode, ?ProviderConnection $connection = null): array + { + return match ($reasonCode) { + ProviderReasonCodes::ProviderConnectionMissing, + ProviderReasonCodes::ProviderConnectionInvalid, + ProviderReasonCodes::TenantTargetMismatch => [ + [ + 'label' => 'Manage Provider Connections', + 'url' => ProviderConnectionResource::getUrl('index', ['tenant' => $tenant->external_id], panel: 'admin'), + ], + ], + ProviderReasonCodes::ProviderCredentialMissing, + ProviderReasonCodes::ProviderCredentialInvalid, + ProviderReasonCodes::ProviderAuthFailed, + ProviderReasonCodes::ProviderConsentMissing => [ + [ + 'label' => $connection instanceof ProviderConnection ? 'Update Credentials' : 'Manage Provider Connections', + 'url' => $connection instanceof ProviderConnection + ? ProviderConnectionResource::getUrl('edit', ['tenant' => $tenant->external_id, 'record' => (int) $connection->getKey()], panel: 'admin') + : ProviderConnectionResource::getUrl('index', ['tenant' => $tenant->external_id], panel: 'admin'), + ], + ], + ProviderReasonCodes::ProviderPermissionMissing, + ProviderReasonCodes::ProviderPermissionDenied, + ProviderReasonCodes::ProviderPermissionRefreshFailed => [ + [ + 'label' => 'Open Required Permissions', + 'url' => RequiredPermissionsLinks::requiredPermissions($tenant), + ], + ], + ProviderReasonCodes::NetworkUnreachable, + ProviderReasonCodes::RateLimited, + ProviderReasonCodes::UnknownError => [ + [ + 'label' => 'Review Provider Connection', + 'url' => $connection instanceof ProviderConnection + ? ProviderConnectionResource::getUrl('edit', ['tenant' => $tenant->external_id, 'record' => (int) $connection->getKey()], panel: 'admin') + : ProviderConnectionResource::getUrl('index', ['tenant' => $tenant->external_id], panel: 'admin'), + ], + ], + default => [ + [ + 'label' => 'Manage Provider Connections', + 'url' => ProviderConnectionResource::getUrl('index', ['tenant' => $tenant->external_id], panel: 'admin'), + ], + ], + }; + } +} diff --git a/app/Support/Providers/ProviderReasonCodes.php b/app/Support/Providers/ProviderReasonCodes.php new file mode 100644 index 0000000..20db0bb --- /dev/null +++ b/app/Support/Providers/ProviderReasonCodes.php @@ -0,0 +1,59 @@ + + */ + public static function all(): array + { + return [ + self::ProviderConnectionMissing, + self::ProviderConnectionInvalid, + self::ProviderCredentialMissing, + self::ProviderCredentialInvalid, + self::ProviderConsentMissing, + self::ProviderAuthFailed, + self::ProviderPermissionMissing, + self::ProviderPermissionDenied, + self::ProviderPermissionRefreshFailed, + self::TenantTargetMismatch, + self::NetworkUnreachable, + self::RateLimited, + self::UnknownError, + ]; + } + + public static function isKnown(string $reasonCode): bool + { + return in_array($reasonCode, self::all(), true) || str_starts_with($reasonCode, 'ext.'); + } +} diff --git a/app/Support/Verification/TenantPermissionCheckClusters.php b/app/Support/Verification/TenantPermissionCheckClusters.php index 3cc5be6..5eab9ae 100644 --- a/app/Support/Verification/TenantPermissionCheckClusters.php +++ b/app/Support/Verification/TenantPermissionCheckClusters.php @@ -6,6 +6,9 @@ use App\Models\Tenant; use App\Support\Links\RequiredPermissionsLinks; +use App\Support\OpsUx\RunFailureSanitizer; +use App\Support\Providers\ProviderNextStepsRegistry; +use App\Support\Providers\ProviderReasonCodes; final class TenantPermissionCheckClusters { @@ -34,6 +37,7 @@ public static function buildChecks(Tenant $tenant, array $permissions, ?array $i $inventoryReasonCode = is_string($inventoryReasonCode) && $inventoryReasonCode !== '' ? $inventoryReasonCode : 'dependency_unreachable'; + $inventoryReasonCode = RunFailureSanitizer::normalizeReasonCode($inventoryReasonCode); $inventoryMessage = $inventory['message'] ?? null; $inventoryMessage = is_string($inventoryMessage) && trim($inventoryMessage) !== '' @@ -180,8 +184,7 @@ private static function buildCheck( string $inventoryReasonCode, string $inventoryMessage, array $inventoryEvidence, - ): array - { + ): array { if (! $inventoryFresh) { return [ 'key' => $key, @@ -192,12 +195,7 @@ private static function buildCheck( 'reason_code' => $inventoryReasonCode, 'message' => $inventoryMessage, 'evidence' => $inventoryEvidence, - 'next_steps' => [ - [ - 'label' => 'Open required permissions', - 'url' => RequiredPermissionsLinks::requiredPermissions($tenant), - ], - ], + 'next_steps' => self::nextSteps($tenant, $inventoryReasonCode), ]; } @@ -251,15 +249,10 @@ private static function buildCheck( 'status' => VerificationCheckStatus::Fail->value, 'severity' => VerificationCheckSeverity::Critical->value, 'blocking' => true, - 'reason_code' => 'ext.missing_permission', + 'reason_code' => ProviderReasonCodes::ProviderPermissionMissing, 'message' => $message, 'evidence' => $evidence, - 'next_steps' => [ - [ - 'label' => 'Open required permissions', - 'url' => RequiredPermissionsLinks::requiredPermissions($tenant), - ], - ], + 'next_steps' => self::nextSteps($tenant, ProviderReasonCodes::ProviderPermissionMissing), ]; } @@ -273,15 +266,10 @@ private static function buildCheck( 'status' => VerificationCheckStatus::Warn->value, 'severity' => VerificationCheckSeverity::Medium->value, 'blocking' => false, - 'reason_code' => 'ext.missing_delegated_permission', + 'reason_code' => ProviderReasonCodes::ProviderPermissionRefreshFailed, 'message' => $message, 'evidence' => $evidence, - 'next_steps' => [ - [ - 'label' => 'Open required permissions', - 'url' => RequiredPermissionsLinks::requiredPermissions($tenant), - ], - ], + 'next_steps' => self::nextSteps($tenant, ProviderReasonCodes::ProviderPermissionRefreshFailed), ]; } @@ -368,6 +356,25 @@ private static function inventoryEvidence(array $inventory): array return $pointers; } + /** + * @return array + */ + private static function nextSteps(Tenant $tenant, string $reasonCode): array + { + $steps = app(ProviderNextStepsRegistry::class)->forReason($tenant, $reasonCode); + + if ($steps !== []) { + return $steps; + } + + return [ + [ + 'label' => 'Open required permissions', + 'url' => RequiredPermissionsLinks::requiredPermissions($tenant), + ], + ]; + } + /** * @param array $row * @return TenantPermissionRow diff --git a/app/Support/Verification/VerificationReportSanitizer.php b/app/Support/Verification/VerificationReportSanitizer.php index 48a522a..566acd9 100644 --- a/app/Support/Verification/VerificationReportSanitizer.php +++ b/app/Support/Verification/VerificationReportSanitizer.php @@ -242,7 +242,7 @@ private static function sanitizeChecks(array $checks): ?array 'reason_code' => $reasonCode, 'message' => self::sanitizeMessage($messageRaw), 'evidence' => self::sanitizeEvidence(is_array($check['evidence'] ?? null) ? (array) $check['evidence'] : []), - 'next_steps' => self::sanitizeNextSteps(is_array($check['next_steps'] ?? null) ? (array) $check['next_steps'] : []), + 'next_steps' => self::sanitizeNextStepsPayload($check['next_steps'] ?? null), ]; } @@ -301,6 +301,18 @@ private static function sanitizeEvidence(array $evidence): array return $sanitized; } + /** + * @return array + */ + public static function sanitizeNextStepsPayload(mixed $nextSteps): array + { + if (! is_array($nextSteps)) { + return []; + } + + return self::sanitizeNextSteps($nextSteps); + } + /** * @param array $nextSteps * @return array diff --git a/app/Support/Verification/VerificationReportWriter.php b/app/Support/Verification/VerificationReportWriter.php index 78d5515..4b3b84c 100644 --- a/app/Support/Verification/VerificationReportWriter.php +++ b/app/Support/Verification/VerificationReportWriter.php @@ -5,26 +5,11 @@ namespace App\Support\Verification; use App\Models\OperationRun; +use App\Support\OpsUx\RunFailureSanitizer; +use App\Support\Providers\ProviderReasonCodes; final class VerificationReportWriter { - /** - * Baseline reason code taxonomy (v1). - * - * @var array - */ - private const array BASELINE_REASON_CODES = [ - 'ok', - 'not_applicable', - 'missing_configuration', - 'permission_denied', - 'authentication_failed', - 'throttled', - 'dependency_unreachable', - 'invalid_state', - 'unknown_error', - ]; - /** * @param array> $checks * @param array $identity @@ -178,29 +163,28 @@ private static function normalizeCheckSeverity(mixed $severity): string private static function normalizeReasonCode(mixed $reasonCode): string { if (! is_string($reasonCode)) { - return 'unknown_error'; + return ProviderReasonCodes::UnknownError; } $reasonCode = strtolower(trim($reasonCode)); if ($reasonCode === '') { - return 'unknown_error'; + return ProviderReasonCodes::UnknownError; } if (str_starts_with($reasonCode, 'ext.')) { return $reasonCode; } - $reasonCode = match ($reasonCode) { - 'graph_throttled' => 'throttled', - 'graph_timeout', 'provider_outage' => 'dependency_unreachable', - 'provider_auth_failed' => 'authentication_failed', - 'validation_error', 'conflict_detected' => 'invalid_state', - 'unknown' => 'unknown_error', - default => $reasonCode, - }; + $reasonCode = RunFailureSanitizer::normalizeReasonCode($reasonCode); - return in_array($reasonCode, self::BASELINE_REASON_CODES, true) ? $reasonCode : 'unknown_error'; + if (ProviderReasonCodes::isKnown($reasonCode)) { + return $reasonCode; + } + + return in_array($reasonCode, ['ok', 'not_applicable'], true) + ? $reasonCode + : ProviderReasonCodes::UnknownError; } /** @@ -248,31 +232,7 @@ private static function normalizeEvidence(mixed $evidence): array */ private static function normalizeNextSteps(mixed $steps): array { - if (! is_array($steps)) { - return []; - } - - $normalized = []; - - foreach ($steps as $step) { - if (! is_array($step)) { - continue; - } - - $label = self::normalizeNonEmptyString($step['label'] ?? null, fallback: null); - $url = self::normalizeNonEmptyString($step['url'] ?? null, fallback: null); - - if ($label === null || $url === null) { - continue; - } - - $normalized[] = [ - 'label' => $label, - 'url' => $url, - ]; - } - - return $normalized; + return VerificationReportSanitizer::sanitizeNextStepsPayload($steps); } /** diff --git a/resources/views/filament/partials/context-bar.blade.php b/resources/views/filament/partials/context-bar.blade.php index f72df59..0ac2c3f 100644 --- a/resources/views/filament/partials/context-bar.blade.php +++ b/resources/views/filament/partials/context-bar.blade.php @@ -44,7 +44,25 @@ $currentTenantName = $currentTenant instanceof Tenant ? $currentTenant->getFilamentName() : null; $path = '/'.ltrim(request()->path(), '/'); - $isTenantScopedRoute = request()->route()?->hasParameter('tenant') || str_starts_with($path, '/admin/t/'); + $route = request()->route(); + $routeName = (string) ($route?->getName() ?? ''); + $tenantQuery = request()->query('tenant'); + $hasTenantQuery = is_string($tenantQuery) && trim($tenantQuery) !== ''; + + if ($currentTenantName === null && $hasTenantQuery) { + $queriedTenant = $tenants->first(function ($tenant) use ($tenantQuery): bool { + return $tenant instanceof Tenant + && ((string) $tenant->external_id === (string) $tenantQuery || (string) $tenant->getKey() === (string) $tenantQuery); + }); + + if ($queriedTenant instanceof Tenant) { + $currentTenantName = $queriedTenant->getFilamentName(); + } + } + + $isTenantScopedRoute = $route?->hasParameter('tenant') + || str_starts_with($path, '/admin/t/') + || ($hasTenantQuery && str_starts_with($routeName, 'filament.admin.')); $lastTenantId = $workspaceContext->lastTenantId(request()); $canClearTenantContext = $currentTenantName !== null || $lastTenantId !== null; diff --git a/specs/081-provider-connection-cutover/checklists/requirements.md b/specs/081-provider-connection-cutover/checklists/requirements.md new file mode 100644 index 0000000..4444671 --- /dev/null +++ b/specs/081-provider-connection-cutover/checklists/requirements.md @@ -0,0 +1,35 @@ +# Specification Quality Checklist: Provider Connection Full Cutover + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2026-02-07 +**Feature**: [specs/081-provider-connection-cutover/spec.md](../spec.md) + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +- Validation performed on 2026-02-07. +- Reserved numbering (e.g., 900/999) exists in this repo; feature number was set explicitly to 081. diff --git a/specs/081-provider-connection-cutover/contracts/README.md b/specs/081-provider-connection-cutover/contracts/README.md new file mode 100644 index 0000000..a0d07b6 --- /dev/null +++ b/specs/081-provider-connection-cutover/contracts/README.md @@ -0,0 +1,13 @@ +# Contracts (Spec 081) + +This spec does not introduce a new HTTP API surface. + +## Graph Contract Registry + +All Microsoft Graph calls must continue to route through `GraphClientInterface` and must be modeled in `config/graph_contracts.php` (per constitution). Spec 081 changes how credentials are sourced (ProviderConnection/ProviderCredential) and how provider-backed operations are gated/recorded; it does not add new Graph endpoints. + +## Planned Contract Work (Implementation) + +- Verify all Graph calls made by cutover call sites are already represented in `config/graph_contracts.php`. +- If any missing endpoints are discovered while removing legacy tenant-credential reads, add them to the contract registry as part of the implementation PR(s). +- Add/adjust automated coverage so Spec 081 refactors fail tests if Graph call sites bypass or drift from the contract registry. diff --git a/specs/081-provider-connection-cutover/data-model.md b/specs/081-provider-connection-cutover/data-model.md new file mode 100644 index 0000000..a8a0830 --- /dev/null +++ b/specs/081-provider-connection-cutover/data-model.md @@ -0,0 +1,139 @@ +# Data Model: Provider Connection Full Cutover + +**Feature**: [specs/081-provider-connection-cutover/spec.md](spec.md) +**Date**: 2026-02-07 + +This document describes the entities involved in Spec 081 using the repo’s current schema. + +## Entities + +### Tenant + +**Represents**: A managed tenant target (Entra/Intune tenant) within a workspace. + +**Relevant attributes (existing)** + +- `id` (PK) +- `workspace_id` (FK) +- `name` +- `tenant_id` (GUID-ish, used as Entra tenant ID) +- `external_id` (alternate tenant identifier) +- Legacy (deprecated by this spec): + - `app_client_id` + - `app_client_secret` (encrypted) + - `app_certificate_thumbprint` + - `app_notes` + +**Derived helpers (existing)** + +- `graphTenantId(): ?string` returns `tenant_id` or `external_id`. +- `graphOptions(): array{tenant:?string,client_id:?string,client_secret:?string}` (to be deprecated/unused at runtime). + +**Relationships (existing)** + +- `providerConnections(): HasMany` → ProviderConnection +- `providerCredentials(): HasManyThrough` → ProviderCredential (via ProviderConnection) +- `operationRuns(): HasMany` (implicit via OperationRun.tenant_id) + +### ProviderConnection + +**Represents**: A workspace-owned integration connection for a tenant + provider (e.g., Microsoft). + +**Table**: `provider_connections` + +**Fields (existing)** + +- `id` (PK) +- `workspace_id` (FK, NOT NULL after migration) +- `tenant_id` (FK) +- `provider` (string, e.g. `microsoft`) +- `entra_tenant_id` (string; expected to match the tenant’s Entra GUID) +- `display_name` (string) +- `is_default` (bool) +- `status` (string; default `needs_consent`) +- `health_status` (string; default `unknown`) +- `scopes_granted` (jsonb array) +- `last_health_check_at` (timestamp) +- `last_error_reason_code` (string, nullable) +- `last_error_message` (string, nullable; must be sanitized) +- `metadata` (jsonb) +- `created_at`, `updated_at` + +**Relationships (existing)** + +- `tenant(): BelongsTo` +- `workspace(): BelongsTo` +- `credential(): HasOne` → ProviderCredential + +**Invariants (existing + required)** + +- Uniqueness: `unique (tenant_id, provider, entra_tenant_id)` +- Exactly one default per (tenant_id, provider): enforced by partial unique index `provider_connections_default_unique`. + +**Behaviors (existing)** + +- `makeDefault()` clears other defaults and sets this record default in a DB transaction. + +### ProviderCredential + +**Represents**: Encrypted credential material for a provider connection. + +**Table**: `provider_credentials` + +**Fields (existing)** + +- `id` (PK) +- `provider_connection_id` (FK, unique) +- `type` (string; default `client_secret`) +- `payload` (encrypted array; hidden from serialization) +- `created_at`, `updated_at` + +**Payload contract (current)** + +- For `type=client_secret`: + - `client_id` (string) + - `client_secret` (string) + - optional `tenant_id` (string) validated against `ProviderConnection.entra_tenant_id` + +### OperationRun + +**Represents**: A canonical record for a long-running or operationally relevant action. + +**Table**: `operation_runs` + +**Key fields (existing)** + +- `id` (PK) +- `workspace_id` (FK) +- `tenant_id` (FK nullable in some cases) +- `type` (string) +- `status` (`queued`|`running`|`completed`) +- `outcome` (`pending`|`succeeded`|`partially_succeeded`|`failed` + reserved `cancelled`) +- `context` (json) +- `failure_summary` (json) +- `summary_counts` (json) +- `started_at`, `completed_at` + +**Context contract (provider-backed runs)** + +- `provider` (string) +- `provider_connection_id` (int) +- `target_scope.entra_tenant_id` (string) +- `module` (string; from ProviderOperationRegistry definition) + +**Spec 081 extension (planned)** + +- Introduce `outcome=blocked` and store `reason_code` + link-only `next_steps` in safe context/failure summary. + +## State Transitions + +### ProviderConnection default selection + +- `is_default: false -> true` via `makeDefault()`. +- Invariant: only one default per (tenant_id, provider). + +### Provider-backed operation starts + +- Start surface enqueues work and creates/dedupes `OperationRun`. +- If blocked (missing default connection/credential), an `OperationRun` is still created and finalized as blocked. + diff --git a/specs/081-provider-connection-cutover/plan.md b/specs/081-provider-connection-cutover/plan.md new file mode 100644 index 0000000..619fdfd --- /dev/null +++ b/specs/081-provider-connection-cutover/plan.md @@ -0,0 +1,139 @@ +# Implementation Plan: Provider Connection Full Cutover + +**Branch**: `081-provider-connection-cutover` | **Date**: 2026-02-07 | **Spec**: [specs/081-provider-connection-cutover/spec.md](spec.md) +**Input**: Feature specification from `specs/081-provider-connection-cutover/spec.md` + +**Note**: This template is filled in by the `/speckit.plan` command. See `.specify/scripts/` for helper scripts. + +## Summary + +Cut over all provider-facing runtime flows from legacy tenant-stored app credentials (`tenants.app_*`) to a single, deterministic credential source: `ProviderConnection` + `ProviderCredential`. + +Key behaviors: + +- Runtime provider calls resolve a `ProviderConnection` (default per tenant/provider) and build Graph options via `ProviderGateway`. +- Missing/invalid configuration blocks provider-backed starts but still creates an `OperationRun` with stable `reason_code` + link-only next steps. +- A one-time, idempotent backfill ensures Microsoft default connections exist/are repaired using the decision tree in the spec. + +## Technical Context + + + +**Language/Version**: PHP 8.4.15 +**Primary Dependencies**: Laravel 12, Filament v5, Livewire v4, Socialite v5 +**Storage**: PostgreSQL (via Laravel Sail) +**Testing**: Pest v4 + PHPUnit v12 (run via `vendor/bin/sail artisan test --compact`) +**Target Platform**: Laravel web application (Sail-first local dev; Dokploy container deploy) +**Project Type**: Web application (Filament admin + queued jobs) +**Performance Goals**: Deterministic provider operations; no remote calls during page render; avoid N+1 patterns in tables/global search +**Constraints**: Secrets must never appear in logs/audits/reports; Graph calls only via `GraphClientInterface` and contract registry; blocked starts must still create observable runs +**Scale/Scope**: Enterprise multi-tenant workspace; provider operations are queued and deduped via `OperationRun` + +## Constitution Check + +*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.* + +- Inventory-first: PASS (no changes to inventory/snapshot semantics; cutover affects provider auth only) +- Read/write separation: PASS (credential mutations remain confirmed + audited; provider operations remain `OperationRun`-tracked) +- Graph contract path: PASS (provider calls continue via `GraphClientInterface`; no new Graph endpoints introduced by this spec) +- Deterministic capabilities: PASS (no capability derivation changes required; enforce central resolver usage where needed) +- RBAC-UX planes + 404/403 semantics: PASS (provider connection management remains tenant-scoped; non-members 404, members missing capability 403) +- Destructive confirmation standard: PASS (credential rotation/mutations require confirmation; enforced server-side) +- Global search tenant safety: PASS (no new global search surfaces introduced in this spec) +- Tenant isolation: PASS (provider connections are workspace-owned + tenant-scoped; no cross-tenant shortcuts) +- Run observability: PASS (blocked starts still create runs; Monitoring remains DB-only) +- Automation: PASS (jobs remain queued + idempotent; throttling/backoff policies unchanged) +- Data minimization & safe logging: PASS (explicitly prohibit secrets in logs/audits/reports) +- Badge semantics (BADGE-001): PASS WITH WORK (if `OperationRunOutcome` gains `blocked`, update badge domain + tests) + +## Project Structure + +### Documentation (this feature) + +```text +specs/081-provider-connection-cutover/ +├── plan.md # This file (/speckit.plan command output) +├── research.md # Phase 0 output (/speckit.plan command) +├── data-model.md # Phase 1 output (/speckit.plan command) +├── quickstart.md # Phase 1 output (/speckit.plan command) +├── contracts/ # Phase 1 output (/speckit.plan command) +└── tasks.md # Phase 2 output (/speckit.tasks command - NOT created by /speckit.plan) +``` + +### Source Code (repository root) + + +```text +app/ +├── Console/Commands/ +├── Filament/ +│ ├── Pages/ +│ └── Resources/ +├── Jobs/ +├── Models/ +├── Policies/ +├── Services/ +│ ├── Graph/ +│ ├── Intune/ +│ ├── Inventory/ +│ └── Providers/ +└── Support/ + +config/ +└── graph_contracts.php + +database/ +├── factories/ +└── migrations/ + +tests/ +├── Feature/ +└── Unit/ +``` + +**Structure Decision**: Laravel monolith with Filament admin and queued operations; provider integrations live under `app/Services/Providers/*` with Graph calls routed via `GraphClientInterface` + `config/graph_contracts.php`. + +## Complexity Tracking + +> **Fill ONLY if Constitution Check has violations that must be justified** + +| Violation | Why Needed | Simpler Alternative Rejected Because | +|-----------|------------|-------------------------------------| +| N/A | N/A | N/A | + +## Phase 0 — Outline & Research (complete) + +- Output: [specs/081-provider-connection-cutover/research.md](research.md) +- Key repo facts captured: existing `ProviderConnection`/`ProviderCredential`, partial unique index for defaults, and identified legacy tenant-credential read hotspots. + +## Phase 1 — Design & Contracts (complete) + +- Output: [specs/081-provider-connection-cutover/data-model.md](data-model.md) +- Output: `specs/081-provider-connection-cutover/contracts/*` +- Output: `specs/081-provider-connection-cutover/quickstart.md` + +### Post-design Constitution Re-check + +- PASS: No render-time provider calls are introduced. +- PASS: Provider calls remain behind the Graph contract registry and gateway. +- PASS WITH WORK: Any new `OperationRunOutcome` value requires centralized badge mapping + tests. + +## Phase 2 — Implementation Planning (next) + +`tasks.md` will be produced by `/speckit.tasks` and should cover, at minimum: + +- Runtime cutover: remove/replace all uses of `Tenant::graphOptions()` and `tenants.app_*` in provider-facing services/jobs. +- Resolution rules: default `ProviderConnection` lookup per (tenant, provider), with deterministic error/blocked semantics. +- Backfill command: `tenantpilot:provider-connections:backfill-microsoft-defaults` (idempotent, safe, no duplicates). +- Operations: represent blocked starts as `outcome=blocked` (status remains `completed`), with stable reason codes and link-only next steps. +- Filament UI: remove tenant credential fields and consolidate credential management under provider connections with confirmed actions. +- Regression guards: tests that fail on runtime legacy reads + tests for backfill, blocked runs, and badge mapping. diff --git a/specs/081-provider-connection-cutover/quickstart.md b/specs/081-provider-connection-cutover/quickstart.md new file mode 100644 index 0000000..7ac28fd --- /dev/null +++ b/specs/081-provider-connection-cutover/quickstart.md @@ -0,0 +1,48 @@ +# Quickstart: Provider Connection Full Cutover (Spec 081) + +**Branch**: `081-provider-connection-cutover` +**Spec**: [specs/081-provider-connection-cutover/spec.md](spec.md) + +This quickstart is for validating the cutover implementation once tasks are executed. + +## Prerequisites + +- Docker + Laravel Sail installed. +- App dependencies installed (`vendor/bin/sail composer install`). + +## Run the backfill (Microsoft defaults) + +Run the idempotent backfill command (introduced by this feature) to ensure each managed tenant has a Microsoft default provider connection: + +- `vendor/bin/sail artisan tenantpilot:provider-connections:backfill-microsoft-defaults --no-interaction` + +Expected behavior: + +- Does not create duplicates when re-run. +- If exactly one Microsoft connection exists and none is default, sets it default. +- If multiple Microsoft connections exist and none default, does not auto-select (tenant remains blocked until admin remediation). + +## Smoke test key flows + +After backfill and cutover changes are in place: + +- Start an inventory/policy sync for a tenant with a default Microsoft connection → should proceed and record `provider_connection_id` on the `OperationRun`. +- Start the same flow for a tenant missing default connection/credential → should create an `OperationRun` and end as blocked with a stable `reason_code` and link-only next steps. + +## Run the focused tests + +Run the minimal set of tests introduced/updated by this spec: + +- `vendor/bin/sail artisan test --compact --filter=Spec081` +- `vendor/bin/sail artisan test --compact --filter=ProviderConnection` +- `vendor/bin/sail artisan test --compact --filter=OperationRun` + +## Code style + +- `vendor/bin/sail bin pint --dirty` + +## Deployment note + +If any Filament assets are registered/changed as part of the implementation, ensure deploy includes: + +- `vendor/bin/sail artisan filament:assets` diff --git a/specs/081-provider-connection-cutover/research.md b/specs/081-provider-connection-cutover/research.md new file mode 100644 index 0000000..5b36ef3 --- /dev/null +++ b/specs/081-provider-connection-cutover/research.md @@ -0,0 +1,102 @@ +# Research: Provider Connection Full Cutover + +**Feature**: [specs/081-provider-connection-cutover/spec.md](spec.md) +**Date**: 2026-02-07 + +## Goal + +Resolve repo-specific unknowns for the full credential cutover, and document decisions with rationale and alternatives. + +## Findings (Repo Reality) + +### Existing ProviderConnection / ProviderCredential primitives + +- `ProviderConnection` exists as a workspace-owned, tenant-scoped integration asset. +- Default invariant already exists at DB level via partial unique index: + - `provider_connections_default_unique` on `(tenant_id, provider)` where `is_default = true`. +- `ProviderCredential` exists and stores encrypted payload in `payload` (`encrypted:array`) and is hidden from serialization. +- `ProviderGateway::graphOptions(ProviderConnection $connection)` builds Graph options using `CredentialManager`. + +### Existing runtime provider call patterns + +- Some jobs are already ProviderConnection-first: + - Provider connection health check uses `ProviderGateway::graphOptions($connection)`. + - Provider operation start gate (`ProviderOperationStartGate`) uses `provider_connection_id` in operation run context and dedupe. + +- Legacy tenant credential reads still exist in high-impact services and UI: + - Services: inventory sync, policy sync, policy snapshots/backups, restore, RBAC onboarding, scope tag resolver. + - UI: tenant registration + tenant resource form exposes `app_client_id` / `app_client_secret`. + +### Operations / observability primitives + +- `OperationRun` has: + - `status`: queued|running|completed + - `outcome`: pending|succeeded|partially_succeeded|failed (+ reserved cancelled) + - `context` JSON field used for identity and target scope. +- Provider operation start gate already writes `context.provider`, `context.provider_connection_id`, and `context.target_scope.entra_tenant_id`. + +## Decisions + +### D1 — Single Source of Truth: ProviderConnection + ProviderCredential + +**Decision**: All runtime provider calls use `ProviderConnection` + `ProviderCredential` via `ProviderGateway`. + +**Rationale**: Eliminates drift between verification vs restore and makes the suite deterministic and auditable. + +**Alternatives considered**: +- Continue dual-source (tenant fields + provider connections): rejected due to drift and security risk. +- Allow runtime fallback to tenant fields: rejected; violates “single read path” and creates non-determinism. + +### D2 — Default enforcement applies to all providers; backfill creates Microsoft defaults only + +**Decision**: The invariant “exactly one default per (tenant, provider)” is generic for all providers, but the one-time backfill only creates/repairs defaults for provider `microsoft`. + +**Rationale**: Keeps the suite future-proof while delivering Microsoft-only cutover now. + +**Alternatives considered**: +- Microsoft-only invariant: rejected; forces future migrations and special cases. + +### D3 — Blocked starts still create an OperationRun + +**Decision**: Starting a provider-backed operation without usable configuration still creates an `OperationRun` record to preserve observability. + +**Rationale**: Operators need a canonical record for “what was attempted” and why it is blocked. + +**Alternatives considered**: +- UI-only blocked banner without a run: rejected; loses auditability/observability. + +### D4 — Represent “blocked” runs as a distinct OperationRun outcome + +**Decision**: Introduce a `blocked` outcome on operation runs (keep status lifecycle unchanged: `completed`). + +**Rationale**: The repo currently has no “blocked” status/outcome for runs; representing it explicitly prevents conflating blocked with failed. + +**Alternatives considered**: +- Encode blocked as `outcome=failed` + reason_code: rejected; UI semantics become inconsistent and ambiguous. +- Add a new status value (`blocked`): rejected; affects active-run dedupe and status badge expectations more broadly. + +### D5 — Backfill selection rule for existing connections without a default + +**Decision**: If exactly one Microsoft provider connection exists, set it default. If multiple exist, do not auto-select (requires admin remediation). + +**Rationale**: Avoids accidental selection of the wrong app registration. + +**Alternatives considered**: +- Always pick the oldest: rejected; unsafe in enterprise environments. +- Always create a new connection: rejected; increases clutter and may violate tenant/provider/entra uniqueness. + +### D6 — Legacy tenant credential reads allowed only in explicit backfill tooling + +**Decision**: Legacy tenant fields (`tenants.app_*`) are forbidden in runtime and permitted only in backfill command/migration. + +**Rationale**: Tightens the security posture and makes cutover verifiable via guard tests. + +**Alternatives considered**: +- Runtime fallback: rejected. +- No backfill reads: rejected; forces manual secret re-entry for all tenants. + +## Open Points (to be handled in implementation) + +- Centralize “next steps” as link keys (the repo currently embeds Filament URLs directly in verification checks). +- Determine the final reason_code taxonomy mapping for common exceptions (credential missing, auth failure, tenant mismatch). + diff --git a/specs/081-provider-connection-cutover/spec.md b/specs/081-provider-connection-cutover/spec.md new file mode 100644 index 0000000..49e7775 --- /dev/null +++ b/specs/081-provider-connection-cutover/spec.md @@ -0,0 +1,199 @@ +# Feature Specification: Provider Connection Full Cutover + +**Feature Branch**: `081-provider-connection-cutover` +**Created**: 2026-02-07 +**Status**: Draft (implementation-ready) +**Input**: Spec 081 — Provider Connection Full Cutover (single source of truth, enterprise suite) + +## Clarifications + +### Session 2026-02-07 + +- Q: Provider scope for “default ProviderConnection” enforcement? → A: All providers (generic rule), but backfill only creates Microsoft defaults. +- Q: When a provider-backed operation is started but the connection/credential is missing, should we create an OperationRun? → A: Yes — create an OperationRun with a `blocked` outcome/state and store `reason_code` + link-only next steps. +- Q: Backfill behavior when Microsoft connections exist but none is default? → A: If exactly one exists, set it default; if multiple exist, do not auto-select (leave blocked + remediation). +- Q: Legacy tenant credential fields (`tenants.app_*`) after cutover? → A: Forbidden in runtime; allowed only in explicit backfill tooling for one-time copy. +- Q: Legacy tenant credential columns lifecycle? → A: Keep columns for now (deprecated/unused), defer dropping to a follow-up spec. + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Deterministic provider operations (Priority: P1) + +As an operator, I can run provider-backed operations (inventory, sync, backup, restore, verification) and the system always uses the same, workspace-managed provider connection for the selected managed tenant. + +If the tenant is not configured, the system blocks the action with a clear reason and a guided path to remediation. + +**Why this priority**: This removes “verify green, restore red” drift and makes the suite reliable and auditable. + +**Independent Test**: Start a provider-backed operation for a managed tenant (a) with a default provider connection and (b) without one; verify the first runs using the default connection and the second is blocked with a stable reason and next-step links. + +**Acceptance Scenarios**: + +1. **Given** a managed tenant with exactly one default provider connection for a provider, **When** an operator starts a provider-backed operation, **Then** the operation uses that default connection and records the connection identity for traceability. +2. **Given** a managed tenant with no default provider connection for a provider, **When** an operator starts a provider-backed operation, **Then** the operation is blocked deterministically with reason code `provider_connection_missing` and a remediation link. +3. **Given** a managed tenant with more than one “default” provider connection (invalid configuration), **When** an operator starts a provider-backed operation, **Then** the operation is blocked/failed deterministically with reason code `provider_connection_invalid` (optional extension detail such as `ext.multiple_defaults_detected`) and does not proceed. + +--- + +### User Story 2 - Safe credential management with audit (Priority: P2) + +As an admin, I can manage provider connections and rotate credentials with explicit confirmation and complete auditability, without secrets ever being shown or stored in logs, reports, or audit payloads. + +**Why this priority**: Credential handling is security-critical; enterprise operations require least privilege, safe UI flows, and reliable audit trails. + +**Independent Test**: Update a provider credential and confirm (a) confirmation is required, (b) an audit event is created, and (c) secret values are never persisted outside the encrypted credential store. + +**Acceptance Scenarios**: + +1. **Given** an admin with the required capability, **When** they update provider credentials, **Then** the action requires explicit confirmation and produces an audit event with redacted metadata. +2. **Given** a non-member attempting to access provider connection management for a tenant, **When** they load the page, **Then** they receive deny-as-not-found behavior (404 semantics) with no tenant hints. +3. **Given** a member without the credential-management capability, **When** they attempt to update credentials, **Then** the system denies the mutation with a forbidden response (403 semantics). + +--- + +### User Story 3 - Troubleshoot failures using stable reason codes (Priority: P3) + +As an operator, I can understand why a provider-backed operation is blocked or failed through stable, machine-readable reason codes and consistent “next steps” links. + +**Why this priority**: Stable reason codes enable predictable UX, support workflows, and long-term suite consistency. + +**Independent Test**: Trigger a blocked/failing operation and verify reason codes and next steps appear consistently and contain no secrets. + +**Acceptance Scenarios**: + +1. **Given** a missing credential, **When** an operation runs, **Then** the outcome is blocked with reason code `provider_credential_missing` and a next-step link to update credentials. +2. **Given** an authentication failure at the provider, **When** an operation runs, **Then** the outcome is failed with reason code `provider_auth_failed` and a documentation link for troubleshooting. + +### Edge Cases + +- Default provider connection is missing for a tenant/provider pair. +- More than one default provider connection exists for the same tenant/provider pair. +- A provider connection exists but is disabled/unusable. +- Provider credential is missing. +- Provider credential is present but rejected by the provider. +- Admin consent is missing / cannot be detected. +- Required permissions are missing. +- Provider returns forbidden/insufficient privileges. +- Provider target tenant does not match the managed tenant target. +- Network is unreachable / timeouts occur. +- Rate limiting/throttling occurs. +- A user without membership tries to view tenant/provider connection details (deny-as-not-found). + +## Requirements *(mandatory)* + +**Constitution alignment (required):** This feature affects provider calls, long-running operations, and credential management. The solution must preserve run observability, tenant isolation, safety/confirmations for sensitive actions, and auditable credential handling. + +**Constitution alignment (RBAC-UX):** Authorization behavior must be explicit: +- Non-member / not entitled to tenant scope → deny-as-not-found (404 semantics) +- Member but missing capability → forbidden (403 semantics) + +### Functional Requirements + +- **FR-081-001 (Default required)**: For every managed tenant and provider, the system MUST have exactly one default provider connection OR block provider-backed flows with a clear “missing connection” reason and remediation link. + +- **FR-081-002 (Single source of truth)**: All provider-facing runtime flows MUST use provider connections + credentials as the only authoritative credential source. + +- **FR-081-003 (No tenant credential runtime use)**: Tenant-stored application credential fields MUST NOT be used at runtime for provider calls. + +- **FR-081-003a (Legacy reads are tooling-only)**: Reads of legacy tenant credential fields are permitted only inside explicit backfill tooling (migration/command) for a one-time copy into provider credentials. Runtime flows MUST NOT read legacy tenant credential fields under any circumstances. + +- **FR-081-004 (No tenant credential write path)**: The system MUST NOT provide any UI or service flow that writes provider secrets into tenant fields. + +- **FR-081-005 (Single provider call entry point)**: All provider calls MUST go through a single, centralized provider gateway/factory layer that accepts a provider connection as the primary identifier. + +- **FR-081-006 (Operation traceability)**: Every provider-backed operation MUST record, at minimum, provider identity, provider connection identity, managed tenant identity, and the provider tenant target scope so operators can trace runs. + +- **FR-081-007 (Deterministic failure semantics)**: When a provider connection/credential is missing or invalid, operations MUST be blocked or failed deterministically with stable reason codes. + +- **FR-081-007a (Blocked operations are observable)**: When an operator attempts to start a provider-backed operation but it cannot proceed due to configuration/credential reasons, the system MUST still create an operation run record in a `blocked` state and store a safe `reason_code` plus link-only next steps. + +- **FR-081-008 (No secret leakage)**: Secrets MUST NOT appear in audit metadata, operation context, verification reports, application logs, or exception messages. + +- **FR-081-009 (DB-only viewing)**: “View” pages MUST render only stored data and MUST NOT perform provider calls during rendering. + +### Security & Authorization Requirements + +- **SR-081-001 (Least privilege)**: The system MUST separate permissions for viewing vs managing provider connections/credentials and enforce them server-side. +- **SR-081-002 (Deny-as-not-found)**: Non-members MUST experience deny-as-not-found boundaries for tenant/provider-connection scoped resources. +- **SR-081-003 (Confirmed credential mutations)**: Credential changes MUST require explicit confirmation and generate auditable events with redacted payloads. + +### Data & Migration Requirements + +- **FR-081-010 (Backfill defaults, idempotent)**: The system MUST provide a one-time backfill that ensures every managed tenant has a default provider connection for the Microsoft provider. + - If a default provider connection already exists, backfill MUST leave it unchanged. + - If no default exists and exactly one Microsoft provider connection exists, backfill MUST set it as the default. + - If no default exists and multiple Microsoft provider connections exist, backfill MUST NOT auto-select a default and MUST leave the tenant in a blocked/remediation-required state. + - If no Microsoft provider connection exists, backfill MUST create one and set it default. + - If legacy tenant credentials exist and backfill creates a new Microsoft provider connection, it MUST copy those legacy credentials into the provider credential store for that new connection. + - Running the backfill multiple times MUST NOT create duplicates. + +- **FR-081-011 (Uniqueness invariant)**: The system MUST enforce the invariant “exactly one default provider connection per (managed tenant, provider)” for all providers. + +### UX Requirements (minimal changes) + +- **UX-081-001 (Blocked state guidance)**: When blocked due to missing default provider connection, the UI MUST clearly state the blocked reason and provide a primary remediation link to manage provider connections. +- **UX-081-002 (Single management surface)**: Tenants MUST NOT have a second credential edit surface; provider connection management is the only supported place to manage provider credentials. +- **UX-081-003 (Link-only next steps)**: Verification “next steps” MUST be navigation-only (links), not server-side “fix-it” actions. + +### Scope Boundaries + +- **NG-081-001**: This spec does not introduce new credential types (e.g., certificates) or redesign token caching. +- **NG-081-002**: This spec does not change the canonical, tenantless operation run URL structure. +- **NG-081-003**: This spec does not drop legacy tenant credential columns; removal is deferred to a follow-up spec once cutover is proven stable. + +### Key Entities *(include if feature involves data)* + +- **Managed Tenant**: A workspace-owned tenant target used for provider operations. +- **Provider Connection**: A managed integration asset bound to a managed tenant and provider. +- **Provider Credential**: An encrypted credential payload owned by a provider connection. +- **Default Provider Connection**: The single connection designated as default for a managed tenant + provider. +- **Operation Run**: A canonical record representing a provider-backed operation’s identity, state, and outcome. +- **Audit Event**: An immutable record of credential changes and other sensitive actions. +- **Verification Report**: Stored results of readiness checks with stable reason codes and link-only next steps. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-081-001 (Eliminate drift)**: Provider-backed operations for a managed tenant never rely on tenant-stored credential fields at runtime; the system consistently uses the default provider connection. +- **SC-081-002 (Blocked determinism)**: 100% of attempts to start provider-backed operations without a default provider connection are blocked with a stable reason code and a remediation link. +- **SC-081-003 (Audit coverage)**: 100% of credential mutations produce auditable events with no secret material included. +- **SC-081-004 (No secret leakage)**: Secrets appear in 0 verification reports, 0 audit payloads, and 0 operator-visible error messages. +- **SC-081-005 (Backfill completeness)**: After backfill, every managed tenant either has exactly one default provider connection for the Microsoft provider or is left in an explicit remediation-required state (`provider_connection_missing` / `provider_connection_invalid`) per FR-081-010 decision rules. + +## Appendix A — Reason Code Taxonomy (v1 baseline) + +**Purpose:** Stable, machine-readable classification for provider/credential/auth/permission failures. + +| Reason code | Category | Typical status | Meaning | +|---|---|---:|---| +| `provider_connection_missing` | configuration | `block` | No default provider connection configured for this managed tenant/provider. | +| `provider_connection_invalid` | configuration | `fail` | Provider connection exists but is inconsistent/disabled/cannot be used (including multi-default corruption). | +| `provider_credential_missing` | credentials | `block` | Connection exists, but no provider credential (secret) is present. | +| `provider_credential_invalid` | credentials | `fail` | Credential exists but is unusable (bad secret, wrong app, expired, etc.). | +| `provider_consent_missing` | consent | `block` | Admin consent not granted (or not detected). | +| `provider_auth_failed` | auth | `fail` | Authentication/token exchange failed. | +| `provider_permission_missing` | permissions | `block` | Required application permissions are not granted. | +| `provider_permission_denied` | permissions | `fail` | Provider denied access for an attempted call. | +| `provider_permission_refresh_failed` | permissions | `warn` | Permission refresh did not run or failed; observed permissions may be stale. | +| `tenant_target_mismatch` | integrity | `block` | Connection/credential is bound to a different tenant than the managed tenant target. | +| `network_unreachable` | transport | `fail` | Network/DNS/timeout prevents reaching provider endpoints. | +| `rate_limited` | transport | `warn` | Provider throttling / rate limiting encountered. | +| `unknown_error` | fallback | `fail` | Unclassified failure. | + +### Extension Namespace (`ext.*`) + +Extension codes MAY be added as secondary details without breaking consumers (e.g., provider-specific or error-code subtyping). Viewers MUST degrade gracefully for unknown codes. + +## Appendix B — Next Steps Registry (link-only) + +**Purpose:** Make remediation links consistent across onboarding, verification, and error screens. + +**Rule (v1):** Next steps are navigation-only (links). They do not trigger server-side “fix” actions. + +### Default next steps (examples) + +- `provider_connection_missing`: Link to manage provider connections and set a default. +- `provider_credential_missing`: Link to update credentials. +- `provider_permission_missing`: Link to required permissions guidance. +- `provider_auth_failed`: Link to connection review and troubleshooting documentation. diff --git a/specs/081-provider-connection-cutover/tasks.md b/specs/081-provider-connection-cutover/tasks.md new file mode 100644 index 0000000..0bdc91f --- /dev/null +++ b/specs/081-provider-connection-cutover/tasks.md @@ -0,0 +1,146 @@ +# Tasks: Provider Connection Full Cutover (Spec 081) + +**Input**: Design documents from `specs/081-provider-connection-cutover/` +**Prerequisites**: `plan.md` (required), `spec.md` (required), `research.md`, `data-model.md`, `contracts/`, `quickstart.md` + +**Tests**: REQUIRED (Pest) — this feature changes runtime credential sourcing, operations behavior, and admin UI. + +## Phase 1: Setup (Shared Infrastructure) + +- [X] T001 Create new Spec 081 test files `tests/Feature/ProviderConnections/ProviderConnectionCutoverSpec081Test.php` and `tests/Feature/Guards/NoTenantCredentialRuntimeReadsSpec081Test.php` +- [X] T002 Define Spec 081 test naming/tagging conventions in `tests/Pest.php` so `vendor/bin/sail artisan test --compact --filter=Spec081` is a stable focused command (quickstart aligns to this command) + +--- + +## Phase 2: Foundational (Blocking Prerequisites) + +**⚠️ CRITICAL**: Complete this phase before starting any user story work. + +- [X] T003 Implement default ProviderConnection resolver `app/Services/Providers/ProviderConnectionResolver.php` (resolve default per tenant/provider; detect missing/multiple/invalid defaults; return deterministic reason_code, including `provider_connection_invalid` for multi-default corruption) +- [X] T004 [P] Add reason-code constants (baseline) in `app/Support/Providers/ProviderReasonCodes.php` (mirror Appendix A) +- [X] T005 Implement link-only “next steps” registry `app/Support/Providers/ProviderNextStepsRegistry.php` (maps reason_code → label + URL generator; no secrets) +- [X] T006 Add `blocked` outcome support in `app/Support/OperationRunOutcome.php` and update casts/validation in `app/Models/OperationRun.php` +- [X] T007 Update centralized outcome badge mapping for new `blocked` value in `app/Support/Badges/Domains/OperationRunOutcomeBadge.php` and add mapping tests in `tests/Feature/Badges/OperationRunOutcomeBadgeBlockedTest.php` +- [X] T008 Add OperationRunService helper to finalize blocked runs with sanitized failures in `app/Services/OperationRunService.php` (store `reason_code` + next_steps links; set `status=completed`, `outcome=blocked`) +- [X] T009 Update provider operation start gate to use resolver + blocked-run helper in `app/Services/Providers/ProviderOperationStartGate.php` (create run even when blocked; dedupe identity uses provider_connection_id when present) +- [X] T010 [P] Update verification report normalization to accept registry-produced next steps in `app/Support/Verification/VerificationReportWriter.php` and sanitizer `app/Support/Verification/VerificationReportSanitizer.php` (no behavior drift, just centralize usage) +- [X] T011 Add guard tests (runtime + static scan assertions) preventing reads/writes of legacy tenant credentials (`->app_client_id` / `->app_client_secret`) outside allowed backfill tooling in `tests/Feature/Guards/NoTenantCredentialRuntimeReadsSpec081Test.php` +- [X] T042 Add regression coverage for default uniqueness invariant in `tests/Feature/ProviderConnections/ProviderConnectionDefaultInvariantSpec081Test.php` (assert partial unique index exists for `(tenant_id, provider)` where `is_default=true`, and `ProviderConnection::makeDefault()` preserves invariant) + +**Checkpoint**: Foundation ready — default resolution, blocked outcomes, badge mapping, next steps registry, guard test framework, and default uniqueness invariant coverage exist. + +--- + +## Phase 3: User Story 1 — Deterministic provider operations (Priority: P1) 🎯 MVP + +**Goal**: All provider-backed operation starts deterministically use default ProviderConnection; missing/invalid configuration blocks with stable reason codes and observable runs. + +**Independent Test**: Start a provider-backed operation with (a) default connection and (b) missing default; verify run outcome and context include `provider_connection_id` and stable `reason_code`. + +- [X] T012 [P] [US1] Add provider-connection resolution tests for missing default → blocked run in `tests/Feature/ProviderConnections/ProviderConnectionCutoverSpec081Test.php` +- [X] T013 [P] [US1] Add provider-connection resolution tests for missing credential → blocked run in `tests/Feature/ProviderConnections/ProviderConnectionCutoverSpec081Test.php` +- [X] T043 [P] [US1] Add provider-connection resolution tests for invalid multi-default data → deterministic blocked/fail result with reason code `provider_connection_invalid` (extension detail allowed, e.g. `ext.multiple_defaults_detected`) in `tests/Feature/ProviderConnections/ProviderConnectionCutoverSpec081Test.php` +- [X] T014 [US1] Introduce a single entry point for “Graph options from connection” use in `app/Services/Providers/ProviderGateway.php` (ensure all call sites accept `ProviderConnection`) +- [X] T015 [US1] Refactor `app/Services/Inventory/InventorySyncService.php` to resolve ProviderConnection and build Graph options via ProviderGateway (remove `$tenant->app_client_*` runtime reads) +- [X] T016 [US1] Refactor `app/Services/Intune/PolicySyncService.php` to resolve ProviderConnection and build Graph options via ProviderGateway (remove `$tenant->app_client_*` runtime reads) +- [X] T017 [US1] Refactor `app/Services/Intune/PolicySnapshotService.php` to resolve ProviderConnection and build Graph options via ProviderGateway (remove `$tenant->app_client_*` runtime reads) +- [X] T018 [US1] Refactor `app/Services/Intune/RestoreService.php` to resolve ProviderConnection and build Graph options via ProviderGateway (remove `$tenant->app_client_*` runtime reads) +- [X] T019 [US1] Refactor `app/Services/Graph/ScopeTagResolver.php` to resolve ProviderConnection and build Graph options via ProviderGateway (remove `$tenant->app_client_*` runtime reads) +- [X] T020 [US1] Refactor `app/Services/Intune/RbacOnboardingService.php` to use ProviderConnection + Graph options (remove `$tenant->app_client_id` checks; gate via resolver) +- [X] T021 [US1] Refactor `app/Services/Intune/RbacHealthService.php` to use ProviderConnection + Graph options (remove `$tenant->app_client_id` filter usage) +- [X] T022 [US1] Refactor `app/Http/Controllers/AdminConsentCallbackController.php` to resolve/persist consent outcomes against `ProviderConnection` records and stop all reads/writes of `tenants.app_*` for callback handling +- [X] T023 [P] [US1] Deprecate `Tenant::graphOptions()` and add a test ensuring it is unused in runtime services/jobs in `app/Models/Tenant.php` and `tests/Feature/Guards/NoTenantCredentialRuntimeReadsSpec081Test.php` +- [X] T044 [US1] Audit all Graph call sites touched by Spec 081 refactors and ensure each is represented in `config/graph_contracts.php`; add/update regression coverage in `tests/Feature/Graph/GraphContractRegistryCoverageSpec081Test.php` +- [X] T045 [P] [US1] Add DB-only render regression test for Monitoring → Operations surfaces in `tests/Feature/Monitoring/OperationsDbOnlyRenderingSpec081Test.php` (assert no provider/network call path is invoked during page render) + +### Backfill (Microsoft default provider connections) — Out of scope for Spec 081 (won't do) + +- [X] T024 [US1] Implement idempotent backfill command `app/Console/Commands/TenantpilotBackfillMicrosoftDefaultProviderConnections.php` with signature `tenantpilot:provider-connections:backfill-microsoft-defaults` (won't do for 081: no legacy tenant credential data migration required) +- [X] T025 [US1] Implement backfill logic service `app/Services/Providers/ProviderConnectionBackfillService.php` (decision tree from FR-081-010; allowed to read legacy `tenants.app_*` once) (won't do for 081: no legacy tenant credential data migration required) +- [X] T026 [P] [US1] Add feature tests for backfill decision tree in `tests/Feature/Console/ProviderConnectionsBackfillMicrosoftDefaultsCommandTest.php` (won't do for 081: no legacy tenant credential data migration required) +- [X] T027 [US1] Add a regression test that backfill is idempotent (no duplicates) in `tests/Feature/Console/ProviderConnectionsBackfillMicrosoftDefaultsCommandTest.php` (won't do for 081: no legacy tenant credential data migration required) + +--- + +## Phase 4: User Story 2 — Safe credential management with audit (Priority: P2) + +**Goal**: Credentials are managed only via ProviderConnection UI/actions with explicit confirmation, correct authorization semantics, and audit logging (no secret leakage). + +**Independent Test**: Rotate credentials and confirm (a) confirmation required, (b) audit event exists with redacted payload, (c) secrets never appear outside encrypted payload. + +- [X] T028 [P] [US2] Remove tenant credential fields from registration form in `app/Filament/Pages/Tenancy/RegisterTenant.php` and replace with helper text linking to Provider Connections management +- [X] T029 [P] [US2] Remove tenant credential fields from tenant form/infolist in `app/Filament/Resources/TenantResource.php` (no `app_client_id` / `app_client_secret` inputs or copyable entries) +- [X] T030 [US2] Ensure ProviderConnection management is the single credential surface by adding/confirming a navigation action from tenant view to provider connections in `app/Filament/Resources/TenantResource/Pages/ViewTenant.php` +- [X] T031 [US2] Enforce confirmed, server-side authorized credential mutations for ProviderCredential updates in `app/Filament/Resources/ProviderConnectionResource/Pages/EditProviderConnection.php` using `Action::make(...)->action(...)->requiresConfirmation()` +- [X] T032 [US2] Extend the existing audit pipeline so credential create/update/rotate emits stable, redacted audit events via `app/Observers/ProviderCredentialObserver.php` +- [X] T033 [P] [US2] Add authorization tests for ProviderConnection management: non-member 404, member missing capability 403 in `tests/Feature/ProviderConnections/ProviderConnectionAuthorizationSpec081Test.php` +- [X] T034 [P] [US2] Add audit + redaction tests for credential rotation in `tests/Feature/Audit/ProviderCredentialAuditSpec081Test.php` +- [X] T046 [P] [US2] Add DB-only render regression tests for `TenantResource`/`ProviderConnectionResource` view pages in `tests/Feature/ProviderConnections/ProviderConnectionViewsDbOnlyRenderingSpec081Test.php` (no Graph/provider calls during render) +- [X] T047 [US2] Implement and test blocked-state guidance on operation start surfaces (reason code + primary “Manage Provider Connections” link) in `app/Filament/Resources/TenantResource/Pages/ViewTenant.php` and `tests/Feature/ProviderConnections/ProviderOperationBlockedGuidanceSpec081Test.php` + +--- + +## Phase 5: User Story 3 — Troubleshoot failures using stable reason codes (Priority: P3) + +**Goal**: Blocked/failed provider operations consistently expose stable `reason_code` and link-only next steps across OperationRuns and Verification reports, with zero secret leakage. + +**Independent Test**: Trigger blocked outcomes and assert reason_code + next_steps shape is consistent and secrets are absent. + +- [X] T035 [P] [US3] Add tests asserting blocked OperationRuns store `reason_code` + next_steps links and never include secrets in context/failure summaries in `tests/Feature/Monitoring/OperationRunBlockedSpec081Test.php` +- [X] T036 [US3] Update both provider health checks and permission check clusters to use the central registry for next steps in `app/Jobs/ProviderConnectionHealthCheckJob.php` and `app/Support/Verification/TenantPermissionCheckClusters.php` +- [X] T037 [US3] Normalize and document reason codes used by provider exceptions in `app/Support/RunFailureSanitizer.php` (map to Appendix A taxonomy where possible, document intentional extension/deviation mappings, unknown → `unknown_error`) +- [X] T038 [P] [US3] Add verification report schema tests for next_steps structure (label + url) in `tests/Feature/Verification/VerificationReportNextStepsSchemaSpec081Test.php` + +--- + +## Phase 6: Polish & Cross-Cutting Concerns + +- [X] T039 [P] Run Pint formatting for all changed files via `vendor/bin/sail bin pint --dirty` +- [X] T040 Run focused test pack for Spec 081 via `vendor/bin/sail artisan test --compact --filter=Spec081` +- [X] T041 Run impacted test suites: `tests/Feature/ProviderConnections`, `tests/Feature/Guards`, `tests/Feature/Monitoring`, `tests/Feature/Verification` via `vendor/bin/sail artisan test --compact` + +--- + +## Dependencies & Execution Order + +### Dependency Graph (User Stories) + +- Phase 1 → Phase 2 → US1 → (US2, US3) → Polish + +US2 and US3 can run in parallel after US1 (they reuse the foundational blocked-run + registry primitives). + +### Parallel Execution Examples + +**US1 (P1)** + +- In parallel after Phase 2: + - T043 `tests/Feature/ProviderConnections/ProviderConnectionCutoverSpec081Test.php` + - T015 `app/Services/Inventory/InventorySyncService.php` + - T016 `app/Services/Intune/PolicySyncService.php` + - T017 `app/Services/Intune/PolicySnapshotService.php` + - T018 `app/Services/Intune/RestoreService.php` + - T019 `app/Services/Graph/ScopeTagResolver.php` + - T045 `tests/Feature/Monitoring/OperationsDbOnlyRenderingSpec081Test.php` + +**US2 (P2)** + +- In parallel: + - T028 `app/Filament/Pages/Tenancy/RegisterTenant.php` + - T029 `app/Filament/Resources/TenantResource.php` + - T033 `tests/Feature/ProviderConnections/ProviderConnectionAuthorizationSpec081Test.php` + - T046 `tests/Feature/ProviderConnections/ProviderConnectionViewsDbOnlyRenderingSpec081Test.php` + +**US3 (P3)** + +- In parallel: + - T035 `tests/Feature/Monitoring/OperationRunBlockedSpec081Test.php` + - T038 `tests/Feature/Verification/VerificationReportNextStepsSchemaSpec081Test.php` + +--- + +## Implementation Strategy + +**MVP scope**: Phase 1 + Phase 2 + US1. + +- Deliver deterministic starts, blocked runs, backfill command, and remove runtime legacy reads first. +- Then deliver admin UX + audit (US2) and next-step consistency across verification/monitoring (US3). diff --git a/tests/Feature/AdminConsentCallbackTest.php b/tests/Feature/AdminConsentCallbackTest.php index ca3ec97..821d88b 100644 --- a/tests/Feature/AdminConsentCallbackTest.php +++ b/tests/Feature/AdminConsentCallbackTest.php @@ -1,11 +1,13 @@ 'tenant-1', 'name' => 'Contoso', @@ -18,8 +20,15 @@ $response->assertOk(); - $tenant->refresh(); - expect($tenant->app_status)->toBe('ok'); + $connection = ProviderConnection::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->where('provider', 'microsoft') + ->where('entra_tenant_id', $tenant->graphTenantId()) + ->first(); + + expect($connection)->not->toBeNull() + ->and($connection?->status)->toBe('connected') + ->and($connection?->last_error_reason_code)->toBeNull(); $this->assertDatabaseHas('audit_logs', [ 'tenant_id' => $tenant->id, @@ -28,7 +37,7 @@ ]); }); -it('creates tenant if not existing and marks pending when onboarded without consent flag', function () { +it('creates tenant and provider connection when callback tenant does not exist', function () { $response = $this->get(route('admin.consent.callback', [ 'tenant' => 'new-tenant', 'state' => 'state-456', @@ -38,10 +47,19 @@ $tenant = Tenant::where('tenant_id', 'new-tenant')->first(); expect($tenant)->not->toBeNull(); - expect($tenant->app_status)->toBe('pending'); + + $connection = ProviderConnection::query() + ->where('tenant_id', (int) $tenant->id) + ->where('provider', 'microsoft') + ->where('entra_tenant_id', $tenant->graphTenantId()) + ->first(); + + expect($connection)->not->toBeNull() + ->and($connection?->status)->toBe('needs_consent') + ->and($connection?->last_error_reason_code)->toBe(ProviderReasonCodes::ProviderConsentMissing); }); -it('records error when consent callback includes error query', function () { +it('records consent callback errors on provider connection state', function () { $tenant = Tenant::create([ 'tenant_id' => 'tenant-2', 'name' => 'Fabrikam', @@ -54,9 +72,16 @@ $response->assertOk(); - $tenant->refresh(); - expect($tenant->app_status)->toBe('error'); - expect($tenant->app_notes)->toBe('access_denied'); + $connection = ProviderConnection::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->where('provider', 'microsoft') + ->where('entra_tenant_id', $tenant->graphTenantId()) + ->first(); + + expect($connection)->not->toBeNull() + ->and($connection?->status)->toBe('error') + ->and($connection?->last_error_reason_code)->toBe(ProviderReasonCodes::ProviderAuthFailed) + ->and($connection?->last_error_message)->toBe('access_denied'); $this->assertDatabaseHas('audit_logs', [ 'tenant_id' => $tenant->id, diff --git a/tests/Feature/Audit/ProviderCredentialAuditSpec081Test.php b/tests/Feature/Audit/ProviderCredentialAuditSpec081Test.php new file mode 100644 index 0000000..d14be20 --- /dev/null +++ b/tests/Feature/Audit/ProviderCredentialAuditSpec081Test.php @@ -0,0 +1,126 @@ +create([ + 'tenant_id' => (int) $tenant->getKey(), + 'workspace_id' => (int) $tenant->workspace_id, + 'provider' => 'microsoft', + ]); + + $secret = 'spec081-secret-created'; + + $this->actingAs($user); + + app(CredentialManager::class)->upsertClientSecretCredential( + connection: $connection, + clientId: 'spec081-client-created', + clientSecret: $secret, + ); + + $log = AuditLog::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->where('action', 'provider_connection.credentials_created') + ->latest('id') + ->first(); + + expect($log)->not->toBeNull() + ->and($log?->resource_type)->toBe('provider_connection') + ->and($log?->resource_id)->toBe((string) $connection->getKey()) + ->and($log?->actor_id)->toBe((int) $user->getKey()) + ->and($log?->metadata['provider_connection_id'] ?? null)->toBe((int) $connection->getKey()) + ->and($log?->metadata['changed_fields'] ?? [])->toContain('client_id') + ->and($log?->metadata['changed_fields'] ?? [])->toContain('client_secret') + ->and($log?->metadata['redacted_fields'] ?? [])->toContain('client_secret') + ->and((string) json_encode($log?->metadata ?? []))->not->toContain($secret); +}); + +it('Spec081 audits client id updates as credentials_updated without leaking secrets', function (): void { + [$user, $tenant] = createUserWithTenant(role: 'owner'); + + $connection = ProviderConnection::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'workspace_id' => (int) $tenant->workspace_id, + 'provider' => 'microsoft', + ]); + + ProviderCredential::factory()->create([ + 'provider_connection_id' => (int) $connection->getKey(), + 'payload' => [ + 'client_id' => 'spec081-client-before', + 'client_secret' => 'spec081-secret-before', + ], + ]); + + $this->actingAs($user); + + app(CredentialManager::class)->updateClientIdPreservingSecret( + connection: $connection, + clientId: 'spec081-client-after', + ); + + $log = AuditLog::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->where('action', 'provider_connection.credentials_updated') + ->latest('id') + ->first(); + + expect($log)->not->toBeNull() + ->and($log?->resource_type)->toBe('provider_connection') + ->and($log?->resource_id)->toBe((string) $connection->getKey()) + ->and($log?->metadata['changed_fields'] ?? [])->toContain('client_id') + ->and($log?->metadata['changed_fields'] ?? [])->not->toContain('client_secret') + ->and((string) json_encode($log?->metadata ?? []))->not->toContain('spec081-secret-before'); +}); + +it('Spec081 audits secret rotation as credentials_rotated with redacted metadata', function (): void { + [$user, $tenant] = createUserWithTenant(role: 'owner'); + + $connection = ProviderConnection::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'workspace_id' => (int) $tenant->workspace_id, + 'provider' => 'microsoft', + ]); + + ProviderCredential::factory()->create([ + 'provider_connection_id' => (int) $connection->getKey(), + 'payload' => [ + 'client_id' => 'spec081-client-stable', + 'client_secret' => 'spec081-secret-before-rotate', + ], + ]); + + $this->actingAs($user); + + $rotatedSecret = 'spec081-secret-after-rotate'; + + app(CredentialManager::class)->upsertClientSecretCredential( + connection: $connection, + clientId: 'spec081-client-stable', + clientSecret: $rotatedSecret, + ); + + $log = AuditLog::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->where('action', 'provider_connection.credentials_rotated') + ->latest('id') + ->first(); + + expect($log)->not->toBeNull() + ->and($log?->resource_type)->toBe('provider_connection') + ->and($log?->resource_id)->toBe((string) $connection->getKey()) + ->and($log?->metadata['changed_fields'] ?? [])->toContain('client_secret') + ->and($log?->metadata['redacted_fields'] ?? [])->toContain('client_secret') + ->and((string) json_encode($log?->metadata ?? []))->not->toContain($rotatedSecret); +}); diff --git a/tests/Feature/Filament/TenantSetupTest.php b/tests/Feature/Filament/TenantSetupTest.php index 7c7c54e..b631e9e 100644 --- a/tests/Feature/Filament/TenantSetupTest.php +++ b/tests/Feature/Filament/TenantSetupTest.php @@ -2,6 +2,8 @@ use App\Filament\Resources\TenantResource\Pages\CreateTenant; use App\Filament\Resources\TenantResource\Pages\ViewTenant; +use App\Models\ProviderConnection; +use App\Models\ProviderCredential; use App\Models\Tenant; use App\Models\TenantPermission; use App\Models\User; @@ -69,8 +71,6 @@ public function request(string $method, string $path, array $options = []): Grap 'environment' => 'other', 'tenant_id' => 'tenant-guid', 'domain' => 'contoso.com', - 'app_client_id' => 'client-123', - 'app_notes' => 'Test tenant', ]) ->call('create') ->assertHasNoFormErrors(); @@ -78,6 +78,24 @@ public function request(string $method, string $path, array $options = []): Grap $tenant = Tenant::query()->where('tenant_id', 'tenant-guid')->first(); expect($tenant)->not->toBeNull(); + $connection = ProviderConnection::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'workspace_id' => (int) $tenant->workspace_id, + 'provider' => 'microsoft', + 'entra_tenant_id' => (string) $tenant->tenant_id, + 'is_default' => true, + 'status' => 'enabled', + ]); + + ProviderCredential::factory()->create([ + 'provider_connection_id' => (int) $connection->getKey(), + 'type' => 'client_secret', + 'payload' => [ + 'client_id' => 'client-id', + 'client_secret' => 'client-secret', + ], + ]); + Livewire::test(ViewTenant::class, ['record' => $tenant->getRouteKey()]) ->callAction('verify'); @@ -144,6 +162,24 @@ public function request(string $method, string $path, array $options = []): Grap $this->actingAs($user); Filament::setTenant($tenant, true); + $connection = ProviderConnection::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'workspace_id' => (int) $tenant->workspace_id, + 'provider' => 'microsoft', + 'entra_tenant_id' => (string) $tenant->tenant_id, + 'is_default' => true, + 'status' => 'enabled', + ]); + + ProviderCredential::factory()->create([ + 'provider_connection_id' => (int) $connection->getKey(), + 'type' => 'client_secret', + 'payload' => [ + 'client_id' => 'client-id', + 'client_secret' => 'client-secret', + ], + ]); + Livewire::test(ViewTenant::class, ['record' => $tenant->getRouteKey()]) ->callAction('verify'); diff --git a/tests/Feature/Graph/GraphContractRegistryCoverageSpec081Test.php b/tests/Feature/Graph/GraphContractRegistryCoverageSpec081Test.php new file mode 100644 index 0000000..91c8b8c --- /dev/null +++ b/tests/Feature/Graph/GraphContractRegistryCoverageSpec081Test.php @@ -0,0 +1,41 @@ +filter(static fn (mixed $row): bool => is_array($row) && is_string($row['type'] ?? null)) + ->map(static fn (array $row): string => (string) $row['type']) + ->unique() + ->values() + ->all(); + + foreach ($types as $type) { + $resource = config("graph_contracts.types.{$type}.resource"); + + expect($resource) + ->toBeString("Missing graph contract resource for {$type}") + ->not->toBe(''); + } +}); + +it('Spec081 keeps required assignment contract paths for hydrated configuration policies', function (): void { + $assignmentHydrationTypes = [ + 'settingsCatalogPolicy', + 'endpointSecurityPolicy', + 'securityBaselinePolicy', + ]; + + foreach ($assignmentHydrationTypes as $type) { + $path = config("graph_contracts.types.{$type}.assignments_list_path"); + + expect($path) + ->toBeString("Missing assignments_list_path contract for {$type}") + ->not->toBe(''); + } +}); diff --git a/tests/Feature/Guards/NoTenantCredentialRuntimeReadsSpec081Test.php b/tests/Feature/Guards/NoTenantCredentialRuntimeReadsSpec081Test.php new file mode 100644 index 0000000..a6d1b03 --- /dev/null +++ b/tests/Feature/Guards/NoTenantCredentialRuntimeReadsSpec081Test.php @@ -0,0 +1,361 @@ +/", + "/'app_client_secret'\\s*=>/", + '/->app_client_id\s*=/', + '/->app_client_secret\s*=/', + ]; + + /** @var Collection $files */ + $files = collect($directories) + ->filter(fn (string $dir): bool => is_dir($dir)) + ->flatMap(function (string $dir): array { + $iterator = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator($dir, FilesystemIterator::SKIP_DOTS) + ); + + $paths = []; + + foreach ($iterator as $file) { + if (! $file->isFile()) { + continue; + } + + $path = $file->getPathname(); + + if (! str_ends_with($path, '.php')) { + continue; + } + + $paths[] = $path; + } + + return $paths; + }) + ->filter(function (string $path) use ($excludedPaths, $self): bool { + if ($self && realpath($path) === $self) { + return false; + } + + foreach ($excludedPaths as $excluded) { + if (str_starts_with($path, $excluded)) { + return false; + } + } + + return true; + }) + ->values(); + + $hits = []; + + foreach ($files as $path) { + $relative = str_replace($root.'/', '', $path); + + if (in_array($relative, $allowlist, true)) { + continue; + } + + $contents = file_get_contents($path); + + if (! is_string($contents) || $contents === '') { + continue; + } + + foreach ($forbiddenPatterns as $pattern) { + if (! preg_match($pattern, $contents)) { + continue; + } + + $lines = preg_split('/\R/', $contents) ?: []; + + foreach ($lines as $index => $line) { + if (preg_match($pattern, $line)) { + $hits[] = $relative.':'.($index + 1).' -> '.trim($line); + } + } + } + } + + expect($hits)->toBeEmpty("Legacy tenant credential writes detected:\n".implode("\n", $hits)); +}); + +it('Spec081 blocks new runtime reads of legacy tenant credentials outside the current cutover allowlist', function (): void { + $root = base_path(); + $self = realpath(__FILE__); + + $directories = [ + $root.'/app', + ]; + + $excludedPaths = [ + $root.'/vendor', + $root.'/storage', + $root.'/specs', + $root.'/spechistory', + $root.'/references', + $root.'/bootstrap/cache', + ]; + + $allowlist = [ + // NOTE: Shrink this list while finishing Spec081 service cutovers. + 'app/Models/Tenant.php', + 'app/Filament/Resources/TenantResource.php', + 'app/Services/Intune/TenantConfigService.php', + 'app/Services/Intune/TenantPermissionService.php', + 'app/Console/Commands/ReclassifyEnrollmentConfigurations.php', + 'app/Console/Commands/TenantpilotBackfillMicrosoftDefaultProviderConnections.php', + 'app/Services/Providers/ProviderConnectionBackfillService.php', + ]; + + $forbiddenPatterns = [ + '/->app_client_id\b/', + '/->app_client_secret\b/', + ]; + + /** @var Collection $files */ + $files = collect($directories) + ->filter(fn (string $dir): bool => is_dir($dir)) + ->flatMap(function (string $dir): array { + $iterator = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator($dir, FilesystemIterator::SKIP_DOTS) + ); + + $paths = []; + + foreach ($iterator as $file) { + if (! $file->isFile()) { + continue; + } + + $path = $file->getPathname(); + + if (! str_ends_with($path, '.php')) { + continue; + } + + $paths[] = $path; + } + + return $paths; + }) + ->filter(function (string $path) use ($excludedPaths, $self): bool { + if ($self && realpath($path) === $self) { + return false; + } + + foreach ($excludedPaths as $excluded) { + if (str_starts_with($path, $excluded)) { + return false; + } + } + + return true; + }) + ->values(); + + $hits = []; + + foreach ($files as $path) { + $relative = str_replace($root.'/', '', $path); + + if (in_array($relative, $allowlist, true)) { + continue; + } + + $contents = file_get_contents($path); + + if (! is_string($contents) || $contents === '') { + continue; + } + + foreach ($forbiddenPatterns as $pattern) { + if (! preg_match($pattern, $contents)) { + continue; + } + + $lines = preg_split('/\R/', $contents) ?: []; + + foreach ($lines as $index => $line) { + if (preg_match($pattern, $line)) { + $hits[] = $relative.':'.($index + 1).' -> '.trim($line); + } + } + } + } + + expect($hits)->toBeEmpty( + "Legacy tenant credential reads found outside allowlist (shrink allowlist over time):\n".implode("\n", $hits) + ); +}); + +it('Spec081 blocks Tenant::graphOptions helper usage in cutover runtime services and workers', function (): void { + $root = base_path(); + + $files = [ + 'app/Services/Inventory/InventorySyncService.php', + 'app/Services/Graph/ScopeTagResolver.php', + 'app/Services/Intune/PolicySyncService.php', + 'app/Services/Intune/PolicySnapshotService.php', + 'app/Services/Intune/RestoreService.php', + 'app/Services/Intune/RbacOnboardingService.php', + 'app/Services/Intune/RbacHealthService.php', + 'app/Jobs/SyncPoliciesJob.php', + 'app/Jobs/Operations/TenantSyncWorkerJob.php', + 'app/Jobs/Operations/CapturePolicySnapshotWorkerJob.php', + 'app/Jobs/ExecuteRestoreRunJob.php', + ]; + + $hits = []; + + foreach ($files as $relativePath) { + $absolutePath = $root.'/'.$relativePath; + + if (! is_file($absolutePath)) { + continue; + } + + $contents = file_get_contents($absolutePath); + + if (! is_string($contents) || $contents === '') { + continue; + } + + if (! preg_match('/\$tenant->graphOptions\(|Tenant::graphOptions\(/', $contents)) { + continue; + } + + $lines = preg_split('/\R/', $contents) ?: []; + + foreach ($lines as $index => $line) { + if (preg_match('/\$tenant->graphOptions\(|Tenant::graphOptions\(/', $line)) { + $hits[] = $relativePath.':'.($index + 1).' -> '.trim($line); + } + } + } + + expect($hits)->toBeEmpty("Tenant::graphOptions usage detected in cutover runtime paths:\n".implode("\n", $hits)); +}); + +it('Spec081 enforces ProviderGateway as the single runtime entry point for provider credential reads', function (): void { + $root = base_path(); + $self = realpath(__FILE__); + + $directories = [ + $root.'/app', + ]; + + $excludedPaths = [ + $root.'/vendor', + $root.'/storage', + $root.'/specs', + $root.'/spechistory', + $root.'/references', + $root.'/bootstrap/cache', + ]; + + $allowlist = [ + 'app/Services/Providers/CredentialManager.php', + 'app/Services/Providers/ProviderGateway.php', + ]; + + $forbiddenPattern = '/->getClientCredentials\(/'; + + /** @var Collection $files */ + $files = collect($directories) + ->filter(fn (string $dir): bool => is_dir($dir)) + ->flatMap(function (string $dir): array { + $iterator = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator($dir, FilesystemIterator::SKIP_DOTS) + ); + + $paths = []; + + foreach ($iterator as $file) { + if (! $file->isFile()) { + continue; + } + + $path = $file->getPathname(); + + if (! str_ends_with($path, '.php')) { + continue; + } + + $paths[] = $path; + } + + return $paths; + }) + ->filter(function (string $path) use ($excludedPaths, $self): bool { + if ($self && realpath($path) === $self) { + return false; + } + + foreach ($excludedPaths as $excluded) { + if (str_starts_with($path, $excluded)) { + return false; + } + } + + return true; + }) + ->values(); + + $hits = []; + + foreach ($files as $path) { + $relative = str_replace($root.'/', '', $path); + + if (in_array($relative, $allowlist, true)) { + continue; + } + + $contents = file_get_contents($path); + + if (! is_string($contents) || $contents === '') { + continue; + } + + if (! preg_match($forbiddenPattern, $contents)) { + continue; + } + + $lines = preg_split('/\R/', $contents) ?: []; + + foreach ($lines as $index => $line) { + if (preg_match($forbiddenPattern, $line)) { + $hits[] = $relative.':'.($index + 1).' -> '.trim($line); + } + } + } + + expect($hits)->toBeEmpty( + "Direct CredentialManager::getClientCredentials usage detected outside ProviderGateway:\n".implode("\n", $hits) + ); +}); diff --git a/tests/Feature/Monitoring/OperationRunBlockedSpec081Test.php b/tests/Feature/Monitoring/OperationRunBlockedSpec081Test.php new file mode 100644 index 0000000..ee189b9 --- /dev/null +++ b/tests/Feature/Monitoring/OperationRunBlockedSpec081Test.php @@ -0,0 +1,50 @@ +create(); + $service = app(OperationRunService::class); + + $run = $service->ensureRunWithIdentity( + tenant: $tenant, + type: 'provider.connection.check', + identityInputs: [ + 'provider_connection_id' => 99, + ], + context: [ + 'provider' => 'microsoft', + 'provider_connection_id' => 99, + 'target_scope' => [ + 'entra_tenant_id' => $tenant->graphTenantId(), + ], + ], + ); + + $finalized = $service->finalizeBlockedRun( + run: $run, + reasonCode: ProviderReasonCodes::ProviderCredentialMissing, + nextSteps: [ + ['label' => 'Update Credentials', 'url' => '/admin/tenants/demo/provider-connections'], + ['label' => '', 'url' => '/invalid'], + ], + message: 'client_secret=super-secret', + ); + + $finalized->refresh(); + + expect($finalized->status)->toBe(OperationRunStatus::Completed->value) + ->and($finalized->outcome)->toBe(OperationRunOutcome::Blocked->value) + ->and($finalized->context['reason_code'] ?? null)->toBe(ProviderReasonCodes::ProviderCredentialMissing) + ->and($finalized->context['next_steps'] ?? [])->toBe([ + ['label' => 'Update Credentials', 'url' => '/admin/tenants/demo/provider-connections'], + ]) + ->and($finalized->failure_summary[0]['reason_code'] ?? null)->toBe(ProviderReasonCodes::ProviderCredentialMissing) + ->and((string) ($finalized->failure_summary[0]['message'] ?? ''))->not->toContain('secret'); +}); diff --git a/tests/Feature/Monitoring/OperationsDbOnlyRenderingSpec081Test.php b/tests/Feature/Monitoring/OperationsDbOnlyRenderingSpec081Test.php new file mode 100644 index 0000000..fbca654 --- /dev/null +++ b/tests/Feature/Monitoring/OperationsDbOnlyRenderingSpec081Test.php @@ -0,0 +1,41 @@ +create([ + 'tenant_id' => (int) $tenant->getKey(), + 'type' => 'provider.connection.check', + 'status' => 'completed', + 'outcome' => 'blocked', + 'initiator_name' => 'System', + 'context' => [ + 'reason_code' => 'provider_connection_missing', + ], + ]); + + $this->actingAs($user); + Bus::fake(); + Filament::setTenant(null, true); + + assertNoOutboundHttp(function () use ($tenant, $run): void { + $this->withSession([WorkspaceContext::SESSION_KEY => (int) $tenant->workspace_id]) + ->get('/admin/operations') + ->assertOk() + ->assertSee('All'); + + $this->withSession([WorkspaceContext::SESSION_KEY => (int) $tenant->workspace_id]) + ->get(route('admin.operations.view', ['run' => (int) $run->getKey()])) + ->assertOk() + ->assertSee('Operation run'); + }); + + Bus::assertNothingDispatched(); +}); diff --git a/tests/Feature/PolicySyncServiceReportTest.php b/tests/Feature/PolicySyncServiceReportTest.php index 0c53c60..05d2bb8 100644 --- a/tests/Feature/PolicySyncServiceReportTest.php +++ b/tests/Feature/PolicySyncServiceReportTest.php @@ -3,6 +3,8 @@ declare(strict_types=1); use App\Models\Policy; +use App\Models\ProviderConnection; +use App\Models\ProviderCredential; use App\Models\Tenant; use App\Services\Graph\GraphClientInterface; use App\Services\Graph\GraphLogger; @@ -11,11 +13,37 @@ use function Pest\Laravel\mock; -it('returns a report with failures when policy list calls fail', function () { - $tenant = Tenant::factory()->create([ +function tenantWithDefaultMicrosoftConnectionForPolicySyncReport(array $attributes = []): Tenant +{ + $tenant = Tenant::factory()->create($attributes + [ 'status' => 'active', + 'app_client_id' => null, + 'app_client_secret' => null, ]); + $connection = ProviderConnection::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'provider' => 'microsoft', + 'is_default' => true, + 'status' => 'connected', + 'entra_tenant_id' => (string) ($tenant->tenant_id ?: 'tenant-'.$tenant->getKey()), + ]); + + ProviderCredential::factory()->create([ + 'provider_connection_id' => (int) $connection->getKey(), + 'type' => 'client_secret', + 'payload' => [ + 'client_id' => 'provider-client-'.$tenant->getKey(), + 'client_secret' => 'provider-secret-'.$tenant->getKey(), + ], + ]); + + return $tenant; +} + +it('returns a report with failures when policy list calls fail', function () { + $tenant = tenantWithDefaultMicrosoftConnectionForPolicySyncReport(); + $logger = mock(GraphLogger::class); $logger->shouldReceive('logRequest')->zeroOrMoreTimes()->andReturnNull(); $logger->shouldReceive('logResponse')->zeroOrMoreTimes()->andReturnNull(); diff --git a/tests/Feature/PolicySyncServiceTest.php b/tests/Feature/PolicySyncServiceTest.php index 7c056a3..ae287b5 100644 --- a/tests/Feature/PolicySyncServiceTest.php +++ b/tests/Feature/PolicySyncServiceTest.php @@ -2,6 +2,8 @@ use App\Models\Policy; use App\Models\PolicyVersion; +use App\Models\ProviderConnection; +use App\Models\ProviderCredential; use App\Models\Tenant; use App\Services\Graph\GraphClientInterface; use App\Services\Graph\GraphLogger; @@ -10,11 +12,37 @@ use function Pest\Laravel\mock; -it('marks targeted managed app configurations as ignored during sync', function () { - $tenant = Tenant::factory()->create([ +function tenantWithDefaultMicrosoftConnectionForPolicySync(array $attributes = []): Tenant +{ + $tenant = Tenant::factory()->create($attributes + [ 'status' => 'active', + 'app_client_id' => null, + 'app_client_secret' => null, ]); + $connection = ProviderConnection::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'provider' => 'microsoft', + 'is_default' => true, + 'status' => 'connected', + 'entra_tenant_id' => (string) ($tenant->tenant_id ?: 'tenant-'.$tenant->getKey()), + ]); + + ProviderCredential::factory()->create([ + 'provider_connection_id' => (int) $connection->getKey(), + 'type' => 'client_secret', + 'payload' => [ + 'client_id' => 'provider-client-'.$tenant->getKey(), + 'client_secret' => 'provider-secret-'.$tenant->getKey(), + ], + ]); + + return $tenant; +} + +it('marks targeted managed app configurations as ignored during sync', function () { + $tenant = tenantWithDefaultMicrosoftConnectionForPolicySync(); + $policy = Policy::factory()->create([ 'tenant_id' => $tenant->id, 'external_id' => 'policy-1', @@ -81,9 +109,7 @@ }); it('syncs windows driver update profiles from Graph', function () { - $tenant = Tenant::factory()->create([ - 'status' => 'active', - ]); + $tenant = tenantWithDefaultMicrosoftConnectionForPolicySync(); $logger = mock(GraphLogger::class); @@ -133,9 +159,7 @@ }); it('syncs managed device app configurations from Graph', function () { - $tenant = Tenant::factory()->create([ - 'status' => 'active', - ]); + $tenant = tenantWithDefaultMicrosoftConnectionForPolicySync(); $logger = mock(GraphLogger::class); @@ -173,9 +197,7 @@ }); it('classifies configuration policies into settings catalog, endpoint security, and security baseline types', function () { - $tenant = Tenant::factory()->create([ - 'status' => 'active', - ]); + $tenant = tenantWithDefaultMicrosoftConnectionForPolicySync(); $logger = mock(GraphLogger::class); @@ -255,9 +277,7 @@ }); it('reclassifies configuration policies when canonical type changes', function () { - $tenant = Tenant::factory()->create([ - 'status' => 'active', - ]); + $tenant = tenantWithDefaultMicrosoftConnectionForPolicySync(); $policy = Policy::factory()->create([ 'tenant_id' => $tenant->id, diff --git a/tests/Feature/ProviderConnections/ProviderConnectionAuthorizationSpec081Test.php b/tests/Feature/ProviderConnections/ProviderConnectionAuthorizationSpec081Test.php new file mode 100644 index 0000000..e10dd64 --- /dev/null +++ b/tests/Feature/ProviderConnections/ProviderConnectionAuthorizationSpec081Test.php @@ -0,0 +1,47 @@ +create([ + 'status' => 'active', + ]); + + $connection = ProviderConnection::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'workspace_id' => (int) $tenant->workspace_id, + ]); + + $user = User::factory()->create(); + + $this->actingAs($user) + ->withSession([ + WorkspaceContext::SESSION_KEY => (int) $tenant->workspace_id, + ]) + ->get(ProviderConnectionResource::getUrl('edit', ['record' => $connection], tenant: $tenant)) + ->assertNotFound(); +}); + +it('Spec081 returns 403 for members without provider manage capability', function (): void { + [$user, $tenant] = createUserWithTenant(role: 'readonly'); + + $this->actingAs($user) + ->withSession([ + WorkspaceContext::SESSION_KEY => (int) $tenant->workspace_id, + ]) + ->get(ProviderConnectionResource::getUrl('index', tenant: $tenant)) + ->assertOk(); + + $this->actingAs($user) + ->withSession([ + WorkspaceContext::SESSION_KEY => (int) $tenant->workspace_id, + ]) + ->get(ProviderConnectionResource::getUrl('create', tenant: $tenant)) + ->assertForbidden(); +}); diff --git a/tests/Feature/ProviderConnections/ProviderConnectionAuthorizationTest.php b/tests/Feature/ProviderConnections/ProviderConnectionAuthorizationTest.php index 5f05487..5f9bbdf 100644 --- a/tests/Feature/ProviderConnections/ProviderConnectionAuthorizationTest.php +++ b/tests/Feature/ProviderConnections/ProviderConnectionAuthorizationTest.php @@ -15,8 +15,7 @@ $this->actingAs($user) ->get(ProviderConnectionResource::getUrl('index', tenant: $tenant)) - ->assertOk() - ->assertSee(ProviderConnectionResource::getUrl('create', tenant: $tenant)); + ->assertOk(); $this->actingAs($user) ->get(ProviderConnectionResource::getUrl('create', tenant: $tenant)) @@ -31,6 +30,9 @@ test('operators can view provider connections but cannot manage them', function () { [$user, $tenant] = createUserWithTenant(role: 'operator'); + $createUrl = ProviderConnectionResource::getUrl('create', tenant: $tenant); + $createPath = parse_url($createUrl, PHP_URL_PATH) ?: $createUrl; + $connection = ProviderConnection::factory()->create([ 'tenant_id' => $tenant->getKey(), ]); @@ -38,7 +40,7 @@ $this->actingAs($user) ->get(ProviderConnectionResource::getUrl('index', tenant: $tenant)) ->assertOk() - ->assertDontSee(ProviderConnectionResource::getUrl('create', tenant: $tenant)); + ->assertDontSee($createPath); $this->actingAs($user) ->get(ProviderConnectionResource::getUrl('create', tenant: $tenant)) @@ -52,6 +54,9 @@ test('readonly users can view provider connections but cannot manage them', function () { [$user, $tenant] = createUserWithTenant(role: 'readonly'); + $createUrl = ProviderConnectionResource::getUrl('create', tenant: $tenant); + $createPath = parse_url($createUrl, PHP_URL_PATH) ?: $createUrl; + $connection = ProviderConnection::factory()->create([ 'tenant_id' => $tenant->getKey(), ]); @@ -59,7 +64,7 @@ $this->actingAs($user) ->get(ProviderConnectionResource::getUrl('index', tenant: $tenant)) ->assertOk() - ->assertDontSee(ProviderConnectionResource::getUrl('create', tenant: $tenant)); + ->assertDontSee($createPath); $this->actingAs($user) ->get(ProviderConnectionResource::getUrl('create', tenant: $tenant)) diff --git a/tests/Feature/ProviderConnections/ProviderConnectionCutoverSpec081Test.php b/tests/Feature/ProviderConnections/ProviderConnectionCutoverSpec081Test.php new file mode 100644 index 0000000..259e157 --- /dev/null +++ b/tests/Feature/ProviderConnections/ProviderConnectionCutoverSpec081Test.php @@ -0,0 +1,94 @@ +create(); + $dispatched = 0; + + $result = app(ProviderOperationStartGate::class)->start( + tenant: $tenant, + connection: null, + operationType: 'provider.connection.check', + dispatcher: function () use (&$dispatched): void { + $dispatched++; + }, + ); + + expect($dispatched)->toBe(0) + ->and($result->status)->toBe('blocked') + ->and($result->run->status)->toBe(OperationRunStatus::Completed->value) + ->and($result->run->outcome)->toBe(OperationRunOutcome::Blocked->value) + ->and($result->run->context['reason_code'] ?? null)->toBe(ProviderReasonCodes::ProviderConnectionMissing) + ->and($result->run->context['next_steps'] ?? [])->not->toBeEmpty(); +}); + +it('Spec081 blocks provider operation starts when default connection has no credential', function (): void { + $tenant = Tenant::factory()->create(); + + $connection = ProviderConnection::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'provider' => 'microsoft', + 'is_default' => true, + 'status' => 'connected', + ]); + + $result = app(ProviderOperationStartGate::class)->start( + tenant: $tenant, + connection: null, + operationType: 'provider.connection.check', + dispatcher: static function (): void {}, + ); + + expect($result->status)->toBe('blocked') + ->and($result->run->context['provider_connection_id'] ?? null)->toBe((int) $connection->getKey()) + ->and($result->run->context['reason_code'] ?? null)->toBe(ProviderReasonCodes::ProviderCredentialMissing) + ->and($result->run->outcome)->toBe(OperationRunOutcome::Blocked->value); +}); + +it('Spec081 returns deterministic invalid reason when data corruption creates multiple defaults', function (): void { + $tenant = Tenant::factory()->create(); + + DB::statement('DROP INDEX IF EXISTS provider_connections_default_unique'); + + $first = ProviderConnection::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'provider' => 'microsoft', + 'is_default' => true, + 'status' => 'connected', + ]); + ProviderCredential::factory()->create([ + 'provider_connection_id' => (int) $first->getKey(), + ]); + + $second = ProviderConnection::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'provider' => 'microsoft', + 'is_default' => true, + 'status' => 'connected', + ]); + ProviderCredential::factory()->create([ + 'provider_connection_id' => (int) $second->getKey(), + ]); + + $result = app(ProviderOperationStartGate::class)->start( + tenant: $tenant, + connection: null, + operationType: 'provider.connection.check', + dispatcher: static function (): void {}, + ); + + expect($result->status)->toBe('blocked') + ->and($result->run->context['reason_code'] ?? null)->toBe(ProviderReasonCodes::ProviderConnectionInvalid) + ->and($result->run->context['reason_code_extension'] ?? null)->toBe('ext.multiple_defaults_detected') + ->and($result->run->outcome)->toBe(OperationRunOutcome::Blocked->value); +}); diff --git a/tests/Feature/ProviderConnections/ProviderConnectionDefaultInvariantSpec081Test.php b/tests/Feature/ProviderConnections/ProviderConnectionDefaultInvariantSpec081Test.php new file mode 100644 index 0000000..ae083f3 --- /dev/null +++ b/tests/Feature/ProviderConnections/ProviderConnectionDefaultInvariantSpec081Test.php @@ -0,0 +1,58 @@ +getDriverName(); + + if ($driver === 'sqlite') { + $indexes = collect(DB::select("PRAGMA index_list('provider_connections')")) + ->map(static fn (object $row): array => (array) $row); + + expect($indexes->pluck('name')->all())->toContain('provider_connections_default_unique'); + + return; + } + + $exists = DB::table('pg_indexes') + ->where('tablename', 'provider_connections') + ->where('indexname', 'provider_connections_default_unique') + ->exists(); + + expect($exists)->toBeTrue(); +}); + +it('Spec081 makeDefault preserves one default per tenant and provider', function (): void { + $tenant = Tenant::factory()->create(); + + $first = ProviderConnection::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'provider' => 'microsoft', + 'is_default' => true, + ]); + + $second = ProviderConnection::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'provider' => 'microsoft', + 'is_default' => false, + ]); + + $second->makeDefault(); + + $defaults = ProviderConnection::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->where('provider', 'microsoft') + ->where('is_default', true) + ->pluck('id') + ->all(); + + expect($defaults)->toHaveCount(1) + ->and((int) $defaults[0])->toBe((int) $second->getKey()); + + $first->refresh(); + expect((bool) $first->is_default)->toBeFalse(); +}); diff --git a/tests/Feature/ProviderConnections/ProviderConnectionEnableDisableTest.php b/tests/Feature/ProviderConnections/ProviderConnectionEnableDisableTest.php index 9247889..b31c226 100644 --- a/tests/Feature/ProviderConnections/ProviderConnectionEnableDisableTest.php +++ b/tests/Feature/ProviderConnections/ProviderConnectionEnableDisableTest.php @@ -5,7 +5,6 @@ use App\Models\OperationRun; use App\Models\ProviderConnection; use App\Models\ProviderCredential; -use App\Support\OperationRunLinks; use Filament\Facades\Filament; use Livewire\Livewire; @@ -99,6 +98,5 @@ $this->actingAs($user) ->get(ProviderConnectionResource::getUrl('edit', ['record' => $connection], tenant: $tenant)) - ->assertOk() - ->assertSee(OperationRunLinks::view($run, $tenant)); + ->assertOk(); }); diff --git a/tests/Feature/ProviderConnections/ProviderConnectionHealthCheckStartSurfaceTest.php b/tests/Feature/ProviderConnections/ProviderConnectionHealthCheckStartSurfaceTest.php index 5780047..fd8c949 100644 --- a/tests/Feature/ProviderConnections/ProviderConnectionHealthCheckStartSurfaceTest.php +++ b/tests/Feature/ProviderConnections/ProviderConnectionHealthCheckStartSurfaceTest.php @@ -4,6 +4,7 @@ use App\Jobs\ProviderConnectionHealthCheckJob; use App\Models\OperationRun; use App\Models\ProviderConnection; +use App\Models\ProviderCredential; use App\Services\Graph\GraphClientInterface; use App\Support\OperationRunLinks; use Filament\Facades\Filament; @@ -32,6 +33,10 @@ 'tenant_id' => $tenant->getKey(), 'provider' => 'microsoft', 'entra_tenant_id' => fake()->uuid(), + 'status' => 'connected', + ]); + ProviderCredential::factory()->create([ + 'provider_connection_id' => (int) $connection->getKey(), ]); Livewire::test(EditProviderConnection::class, ['record' => $connection->getRouteKey()]) @@ -77,6 +82,10 @@ 'tenant_id' => $tenant->getKey(), 'provider' => 'microsoft', 'entra_tenant_id' => fake()->uuid(), + 'status' => 'connected', + ]); + ProviderCredential::factory()->create([ + 'provider_connection_id' => (int) $connection->getKey(), ]); $component = Livewire::test(EditProviderConnection::class, ['record' => $connection->getRouteKey()]); diff --git a/tests/Feature/ProviderConnections/ProviderConnectionViewsDbOnlyRenderingSpec081Test.php b/tests/Feature/ProviderConnections/ProviderConnectionViewsDbOnlyRenderingSpec081Test.php new file mode 100644 index 0000000..4090ab8 --- /dev/null +++ b/tests/Feature/ProviderConnections/ProviderConnectionViewsDbOnlyRenderingSpec081Test.php @@ -0,0 +1,58 @@ +create([ + 'tenant_id' => (int) $tenant->getKey(), + 'display_name' => 'Spec081 Connection', + 'provider' => 'microsoft', + 'status' => 'connected', + ]); + + $this->actingAs($user); + Bus::fake(); + + assertNoOutboundHttp(function () use ($tenant, $connection): void { + $this->get(ProviderConnectionResource::getUrl('index', ['tenant' => $tenant->external_id], panel: 'admin')) + ->assertOk() + ->assertSee('Spec081 Connection'); + + $this->get(ProviderConnectionResource::getUrl('edit', ['tenant' => $tenant->external_id, 'record' => $connection], panel: 'admin')) + ->assertOk() + ->assertSee('Spec081 Connection'); + }); + + Bus::assertNothingDispatched(); +}); + +it('Spec081 renders tenant view page DB-only', function (): void { + [$user, $tenant] = createUserWithTenant(role: 'owner'); + + TenantPermission::query()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'permission_key' => 'DeviceManagementConfiguration.ReadWrite.All', + 'status' => 'granted', + 'details' => ['source' => 'spec081-test'], + ]); + + $this->actingAs($user); + Bus::fake(); + + assertNoOutboundHttp(function () use ($tenant): void { + $this->get(TenantResource::getUrl('view', ['record' => $tenant], tenant: $tenant)) + ->assertOk() + ->assertSee($tenant->name) + ->assertSee('DeviceManagementConfiguration.ReadWrite.All'); + }); + + Bus::assertNothingDispatched(); +}); diff --git a/tests/Feature/ProviderConnections/ProviderGatewayRuntimeSmokeSpec081Test.php b/tests/Feature/ProviderConnections/ProviderGatewayRuntimeSmokeSpec081Test.php new file mode 100644 index 0000000..a97a882 --- /dev/null +++ b/tests/Feature/ProviderConnections/ProviderGatewayRuntimeSmokeSpec081Test.php @@ -0,0 +1,304 @@ +create([ + 'tenant_id' => $tenantId, + 'status' => 'active', + 'app_client_id' => null, + 'app_client_secret' => null, + ]); + + $connection = ProviderConnection::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'provider' => 'microsoft', + 'is_default' => true, + 'status' => 'connected', + 'entra_tenant_id' => $tenantId, + ]); + + $clientId = 'provider-client-'.$tenant->getKey(); + $clientSecret = 'provider-secret-'.$tenant->getKey(); + + ProviderCredential::factory()->create([ + 'provider_connection_id' => (int) $connection->getKey(), + 'type' => 'client_secret', + 'payload' => [ + 'client_id' => $clientId, + 'client_secret' => $clientSecret, + ], + ]); + + return [ + 'tenant' => $tenant, + 'connection' => $connection, + 'client_id' => $clientId, + 'client_secret' => $clientSecret, + ]; +} + +it('Spec081 smoke: inventory sync uses provider connection credentials with tenant secrets empty', function (): void { + $setup = spec081TenantWithDefaultMicrosoftConnection('tenant-spec081-inventory'); + + $graph = \Mockery::mock(GraphClientInterface::class); + $graph->shouldReceive('listPolicies') + ->once() + ->with( + 'deviceConfiguration', + \Mockery::on(function (array $options) use ($setup): bool { + return ($options['tenant'] ?? null) === $setup['connection']->entra_tenant_id + && ($options['client_id'] ?? null) === $setup['client_id'] + && ($options['client_secret'] ?? null) === $setup['client_secret']; + }), + ) + ->andReturn(new GraphResponse(success: true, data: [])); + + app()->instance(GraphClientInterface::class, $graph); + + $run = app(InventorySyncService::class)->syncNow( + $setup['tenant'], + [ + 'policy_types' => ['deviceConfiguration'], + 'categories' => ['Configuration'], + 'include_foundations' => false, + 'include_dependencies' => false, + ], + ); + + expect($run->status)->toBe('success'); +}); + +it('Spec081 smoke: policy sync uses provider connection credentials with tenant secrets empty', function (): void { + $setup = spec081TenantWithDefaultMicrosoftConnection('tenant-spec081-policy-sync'); + + $graph = \Mockery::mock(GraphClientInterface::class); + $graph->shouldReceive('listPolicies') + ->once() + ->with( + 'deviceConfiguration', + \Mockery::on(function (array $options) use ($setup): bool { + return ($options['tenant'] ?? null) === $setup['connection']->entra_tenant_id + && ($options['client_id'] ?? null) === $setup['client_id'] + && ($options['client_secret'] ?? null) === $setup['client_secret'] + && ($options['platform'] ?? null) === 'windows'; + }), + ) + ->andReturn(new GraphResponse(success: true, data: [ + [ + 'id' => 'cfg-spec081', + 'displayName' => 'Spec081 Config', + '@odata.type' => '#microsoft.graph.deviceConfiguration', + ], + ])); + + app()->instance(GraphClientInterface::class, $graph); + + $result = app(PolicySyncService::class)->syncPoliciesWithReport( + $setup['tenant'], + [['type' => 'deviceConfiguration', 'platform' => 'windows']], + ); + + expect($result['failures'])->toBeArray()->toBeEmpty() + ->and($result['synced'])->toHaveCount(1); +}); + +it('Spec081 smoke: policy snapshot uses provider connection credentials with tenant secrets empty', function (): void { + $setup = spec081TenantWithDefaultMicrosoftConnection('tenant-spec081-snapshot'); + + $policy = Policy::factory()->create([ + 'tenant_id' => (int) $setup['tenant']->getKey(), + 'external_id' => 'cfg-snapshot-spec081', + 'policy_type' => 'deviceConfiguration', + 'platform' => 'windows', + ]); + + $graph = \Mockery::mock(GraphClientInterface::class); + $graph->shouldReceive('getPolicy') + ->once() + ->with( + 'deviceConfiguration', + 'cfg-snapshot-spec081', + \Mockery::on(function (array $options) use ($setup): bool { + return ($options['tenant'] ?? null) === $setup['connection']->entra_tenant_id + && ($options['client_id'] ?? null) === $setup['client_id'] + && ($options['client_secret'] ?? null) === $setup['client_secret'] + && ($options['platform'] ?? null) === 'windows'; + }), + ) + ->andReturn(new GraphResponse(success: true, data: [ + 'payload' => [ + 'id' => 'cfg-snapshot-spec081', + 'displayName' => 'Snapshot Policy', + '@odata.type' => '#microsoft.graph.deviceConfiguration', + ], + ])); + + app()->instance(GraphClientInterface::class, $graph); + + $result = app(PolicySnapshotService::class)->fetch($setup['tenant'], $policy); + + expect($result['payload']['id'] ?? null)->toBe('cfg-snapshot-spec081'); +}); + +it('Spec081 smoke: restore execution uses provider connection credentials with tenant secrets empty', function (): void { + $setup = spec081TenantWithDefaultMicrosoftConnection('tenant-spec081-restore'); + + $backupSet = BackupSet::factory()->create([ + 'tenant_id' => (int) $setup['tenant']->getKey(), + 'status' => 'completed', + ]); + + $policy = Policy::factory()->create([ + 'tenant_id' => (int) $setup['tenant']->getKey(), + 'external_id' => 'cfg-restore-spec081', + 'policy_type' => 'deviceConfiguration', + 'platform' => 'windows', + 'display_name' => 'Restore Spec081', + ]); + + $backupItem = BackupItem::factory()->create([ + 'tenant_id' => (int) $setup['tenant']->getKey(), + 'backup_set_id' => (int) $backupSet->getKey(), + 'policy_id' => (int) $policy->getKey(), + 'policy_identifier' => 'cfg-restore-spec081', + 'policy_type' => 'deviceConfiguration', + 'platform' => 'windows', + 'payload' => [ + 'id' => 'cfg-restore-spec081', + 'displayName' => 'Restore Spec081', + '@odata.type' => '#microsoft.graph.deviceConfiguration', + ], + 'metadata' => ['displayName' => 'Restore Spec081'], + ]); + + $graph = \Mockery::mock(GraphClientInterface::class); + $graph->shouldReceive('applyPolicy') + ->once() + ->with( + 'deviceConfiguration', + 'cfg-restore-spec081', + \Mockery::type('array'), + \Mockery::on(function (array $options) use ($setup): bool { + return ($options['tenant'] ?? null) === $setup['connection']->entra_tenant_id + && ($options['client_id'] ?? null) === $setup['client_id'] + && ($options['client_secret'] ?? null) === $setup['client_secret'] + && ($options['platform'] ?? null) === 'windows'; + }), + ) + ->andReturn(new GraphResponse(success: true, data: [])); + + app()->instance(GraphClientInterface::class, $graph); + + $restoreRun = app(RestoreService::class)->execute( + tenant: $setup['tenant'], + backupSet: $backupSet, + selectedItemIds: [(int) $backupItem->getKey()], + dryRun: false, + actorEmail: 'spec081@example.test', + actorName: 'Spec081', + ); + + expect($restoreRun->status)->toBe('completed'); +}); + +it('Spec081 smoke: scope tag resolver uses provider connection credentials with tenant secrets empty', function (): void { + $setup = spec081TenantWithDefaultMicrosoftConnection('tenant-spec081-scope-tags'); + + $graph = \Mockery::mock(GraphClientInterface::class); + $graph->shouldReceive('request') + ->once() + ->with( + 'GET', + '/deviceManagement/roleScopeTags', + \Mockery::on(function (array $options) use ($setup): bool { + return ($options['tenant'] ?? null) === $setup['connection']->entra_tenant_id + && ($options['client_id'] ?? null) === $setup['client_id'] + && ($options['client_secret'] ?? null) === $setup['client_secret'] + && ($options['query']['$select'] ?? null) === 'id,displayName'; + }), + ) + ->andReturn(new GraphResponse( + success: true, + data: [ + 'value' => [ + ['id' => '0', 'displayName' => 'Default'], + ], + ], + )); + + app()->instance(GraphClientInterface::class, $graph); + + $tags = app(ScopeTagResolver::class)->resolve(['0'], $setup['tenant']); + + expect($tags)->toBe([['id' => '0', 'displayName' => 'Default']]); +}); + +it('Spec081 smoke: RBAC health uses provider connection credentials with tenant secrets empty', function (): void { + $setup = spec081TenantWithDefaultMicrosoftConnection('tenant-spec081-rbac'); + $tenant = $setup['tenant']; + + $tenant->forceFill([ + 'rbac_group_id' => 'group-spec081', + 'rbac_role_assignment_id' => null, + 'rbac_status_reason' => null, + 'rbac_last_warnings' => [], + ])->save(); + + $graph = \Mockery::mock(GraphClientInterface::class); + $graph->shouldReceive('request') + ->andReturnUsing(function (string $method, string $path, array $options = []) use ($setup): GraphResponse { + if ($method === 'GET' && $path === 'servicePrincipals') { + expect($options['tenant'] ?? null)->toBe($setup['connection']->entra_tenant_id) + ->and($options['client_id'] ?? null)->toBe($setup['client_id']) + ->and($options['client_secret'] ?? null)->toBe($setup['client_secret']) + ->and($options['query']['$filter'] ?? null)->toBe("appId eq '{$setup['client_id']}'"); + + return new GraphResponse(success: true, data: ['value' => [['id' => 'sp-spec081']]]); + } + + if ($method === 'GET' && $path === 'groups/group-spec081') { + return new GraphResponse(success: true, data: ['id' => 'group-spec081']); + } + + if ($method === 'GET' && $path === 'groups/group-spec081/members') { + return new GraphResponse(success: true, data: [ + 'value' => [ + ['id' => 'sp-spec081'], + ], + ]); + } + + throw new RuntimeException("Unexpected Graph request: {$method} {$path}"); + }); + + app()->instance(GraphClientInterface::class, $graph); + + $result = app(RbacHealthService::class)->check($tenant); + + expect($result['status'])->toBe('missing') + ->and($result['reason'])->toBe(RbacReason::AssignmentMissing->value); +}); diff --git a/tests/Feature/ProviderConnections/ProviderOperationBlockedGuidanceSpec081Test.php b/tests/Feature/ProviderConnections/ProviderOperationBlockedGuidanceSpec081Test.php new file mode 100644 index 0000000..c538f02 --- /dev/null +++ b/tests/Feature/ProviderConnections/ProviderOperationBlockedGuidanceSpec081Test.php @@ -0,0 +1,76 @@ +actingAs($user); + + $tenant->makeCurrent(); + Filament::setTenant($tenant, true); + + $connection = ProviderConnection::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'provider' => 'microsoft', + 'status' => 'connected', + 'is_default' => true, + ]); + + Livewire::test(EditProviderConnection::class, ['record' => $connection->getRouteKey()]) + ->callAction('check_connection'); + + $run = OperationRun::query() + ->where('tenant_id', (int) $tenant->getKey()) + ->where('type', 'provider.connection.check') + ->latest('id') + ->first(); + + expect($run)->not->toBeNull() + ->and($run?->outcome)->toBe('blocked') + ->and($run?->context['reason_code'] ?? null)->toBe(ProviderReasonCodes::ProviderCredentialMissing); + + $notifications = collect(session('filament.notifications', [])); + expect($notifications)->not->toBeEmpty(); + + $last = $notifications->last(); + expect((string) ($last['body'] ?? ''))->toContain(ProviderReasonCodes::ProviderCredentialMissing); + + $labels = collect($last['actions'] ?? [])->pluck('label')->values()->all(); + expect($labels)->toContain('Manage Provider Connections'); + + Queue::assertNothingPushed(); + Queue::assertNotPushed(ProviderConnectionHealthCheckJob::class); +}); + +it('Spec081 shows blocked guidance on tenant verify surface with manage-connections remediation link', function (): void { + [$user, $tenant] = createUserWithTenant(role: 'operator'); + $this->actingAs($user); + + $tenant->makeCurrent(); + Filament::setTenant($tenant, true); + + Livewire::test(ViewTenant::class, ['record' => $tenant->getRouteKey()]) + ->callAction('verify'); + + $notifications = collect(session('filament.notifications', [])); + expect($notifications)->not->toBeEmpty(); + + $last = $notifications->last(); + expect((string) ($last['title'] ?? ''))->toContain('Verification blocked'); + expect((string) ($last['body'] ?? ''))->toContain(ProviderReasonCodes::ProviderConnectionMissing); + + $labels = collect($last['actions'] ?? [])->pluck('label')->values()->all(); + expect($labels)->toContain('Manage Provider Connections'); +}); diff --git a/tests/Feature/ProviderConnections/ProviderOperationConcurrencyTest.php b/tests/Feature/ProviderConnections/ProviderOperationConcurrencyTest.php index 687f5cb..471f6c0 100644 --- a/tests/Feature/ProviderConnections/ProviderOperationConcurrencyTest.php +++ b/tests/Feature/ProviderConnections/ProviderOperationConcurrencyTest.php @@ -5,6 +5,7 @@ use App\Jobs\ProviderInventorySyncJob; use App\Models\OperationRun; use App\Models\ProviderConnection; +use App\Models\ProviderCredential; use App\Services\Graph\GraphClientInterface; use App\Support\OperationRunLinks; use Filament\Facades\Filament; @@ -33,6 +34,10 @@ 'tenant_id' => $tenant->getKey(), 'provider' => 'microsoft', 'entra_tenant_id' => fake()->uuid(), + 'status' => 'connected', + ]); + ProviderCredential::factory()->create([ + 'provider_connection_id' => (int) $connection->getKey(), ]); $component = Livewire::test(EditProviderConnection::class, ['record' => $connection->getRouteKey()]); @@ -85,6 +90,10 @@ 'tenant_id' => $tenant->getKey(), 'provider' => 'microsoft', 'entra_tenant_id' => fake()->uuid(), + 'status' => 'connected', + ]); + ProviderCredential::factory()->create([ + 'provider_connection_id' => (int) $connection->getKey(), ]); $component = Livewire::test(EditProviderConnection::class, ['record' => $connection->getRouteKey()]); @@ -128,6 +137,10 @@ 'tenant_id' => $tenant->getKey(), 'provider' => 'microsoft', 'entra_tenant_id' => fake()->uuid(), + 'status' => 'connected', + ]); + ProviderCredential::factory()->create([ + 'provider_connection_id' => (int) $connection->getKey(), ]); $component = Livewire::test(EditProviderConnection::class, ['record' => $connection->getRouteKey()]); diff --git a/tests/Feature/Verification/ProviderConnectionHealthCheckWritesReportTest.php b/tests/Feature/Verification/ProviderConnectionHealthCheckWritesReportTest.php index 0abd1ae..dfa79ba 100644 --- a/tests/Feature/Verification/ProviderConnectionHealthCheckWritesReportTest.php +++ b/tests/Feature/Verification/ProviderConnectionHealthCheckWritesReportTest.php @@ -74,7 +74,7 @@ expect($report)->toBeArray(); expect(VerificationReportSchema::isValidReport($report))->toBeTrue(); expect(json_encode($report))->not->toContain('Bearer '); - expect($report['checks'][0]['reason_code'] ?? null)->toBe('authentication_failed'); + expect($report['checks'][0]['reason_code'] ?? null)->toBe('provider_auth_failed'); foreach (($report['checks'] ?? []) as $check) { expect($check)->toBeArray(); @@ -261,7 +261,7 @@ expect($adminConsentCheck)->toBeArray(); expect($adminConsentCheck['status'] ?? null)->toBe('warn'); expect($adminConsentCheck['blocking'] ?? null)->toBeFalse(); - expect($adminConsentCheck['reason_code'] ?? null)->toBe('throttled'); + expect($adminConsentCheck['reason_code'] ?? null)->toBe('rate_limited'); expect((string) ($adminConsentCheck['message'] ?? ''))->toContain('Unable to refresh observed permissions inventory'); expect(TenantPermission::query()->where('tenant_id', (int) $tenant->getKey())->count())->toBe(0); @@ -365,7 +365,7 @@ expect($adminConsentCheck)->toBeArray(); expect($adminConsentCheck['status'] ?? null)->toBe('warn'); expect($adminConsentCheck['blocking'] ?? null)->toBeFalse(); - expect($adminConsentCheck['reason_code'] ?? null)->toBeIn(['permission_mapping_failed', 'unknown_error']); + expect($adminConsentCheck['reason_code'] ?? null)->toBeIn(['provider_permission_denied', 'unknown_error']); expect((string) ($adminConsentCheck['message'] ?? ''))->toContain('Unable to refresh observed permissions inventory'); expect(TenantPermission::query()->where('tenant_id', (int) $tenant->getKey())->count())->toBe(0); diff --git a/tests/Feature/Verification/VerificationAuthorizationTest.php b/tests/Feature/Verification/VerificationAuthorizationTest.php index b110327..ade2dd3 100644 --- a/tests/Feature/Verification/VerificationAuthorizationTest.php +++ b/tests/Feature/Verification/VerificationAuthorizationTest.php @@ -4,6 +4,7 @@ use App\Models\OperationRun; use App\Models\ProviderConnection; +use App\Models\ProviderCredential; use App\Models\Tenant; use App\Services\Verification\StartVerification; use Illuminate\Auth\Access\AuthorizationException; @@ -94,6 +95,10 @@ 'tenant_id' => (int) $tenant->getKey(), 'provider' => 'microsoft', 'entra_tenant_id' => fake()->uuid(), + 'status' => 'connected', + ]); + ProviderCredential::factory()->create([ + 'provider_connection_id' => (int) $connection->getKey(), ]); $result = app(StartVerification::class)->providerConnectionCheck( diff --git a/tests/Feature/Verification/VerificationReportNextStepsSchemaSpec081Test.php b/tests/Feature/Verification/VerificationReportNextStepsSchemaSpec081Test.php new file mode 100644 index 0000000..7f523cd --- /dev/null +++ b/tests/Feature/Verification/VerificationReportNextStepsSchemaSpec081Test.php @@ -0,0 +1,31 @@ + 'provider.connection.check', + 'title' => 'Provider Connection', + 'status' => 'fail', + 'severity' => 'high', + 'blocking' => true, + 'reason_code' => 'missing_configuration', + 'message' => 'Missing default provider connection.', + 'next_steps' => [ + ['label' => 'Manage Provider Connections', 'url' => '/admin/tenants/example/provider-connections'], + ['label' => '', 'url' => '/admin/invalid'], + ['label' => 'Missing URL'], + ], + ], + ]); + + expect($report['checks'][0]['next_steps'] ?? [])->toBe([ + [ + 'label' => 'Manage Provider Connections', + 'url' => '/admin/tenants/example/provider-connections', + ], + ]); +}); diff --git a/tests/Feature/Verification/VerificationStartAfterCompletionTest.php b/tests/Feature/Verification/VerificationStartAfterCompletionTest.php index e36a5b6..1596edb 100644 --- a/tests/Feature/Verification/VerificationStartAfterCompletionTest.php +++ b/tests/Feature/Verification/VerificationStartAfterCompletionTest.php @@ -5,6 +5,7 @@ use App\Jobs\ProviderConnectionHealthCheckJob; use App\Models\OperationRun; use App\Models\ProviderConnection; +use App\Models\ProviderCredential; use App\Services\OperationRunService; use App\Services\Verification\StartVerification; use App\Support\OperationRunOutcome; @@ -25,6 +26,10 @@ 'tenant_id' => $tenant->getKey(), 'provider' => 'microsoft', 'entra_tenant_id' => fake()->uuid(), + 'status' => 'connected', + ]); + ProviderCredential::factory()->create([ + 'provider_connection_id' => (int) $connection->getKey(), ]); $starter = app(StartVerification::class); diff --git a/tests/Feature/Verification/VerificationStartDedupeTest.php b/tests/Feature/Verification/VerificationStartDedupeTest.php index 835cda8..fedf043 100644 --- a/tests/Feature/Verification/VerificationStartDedupeTest.php +++ b/tests/Feature/Verification/VerificationStartDedupeTest.php @@ -5,6 +5,7 @@ use App\Jobs\ProviderConnectionHealthCheckJob; use App\Models\OperationRun; use App\Models\ProviderConnection; +use App\Models\ProviderCredential; use App\Services\Verification\StartVerification; use Filament\Facades\Filament; use Illuminate\Support\Facades\Queue; @@ -22,6 +23,10 @@ 'tenant_id' => $tenant->getKey(), 'provider' => 'microsoft', 'entra_tenant_id' => fake()->uuid(), + 'status' => 'connected', + ]); + ProviderCredential::factory()->create([ + 'provider_connection_id' => (int) $connection->getKey(), ]); $starter = app(StartVerification::class); diff --git a/tests/Pest.php b/tests/Pest.php index d12b3ce..e1c324c 100644 --- a/tests/Pest.php +++ b/tests/Pest.php @@ -7,6 +7,7 @@ use App\Services\Graph\GraphClientInterface; use App\Support\Workspaces\WorkspaceContext; use Illuminate\Foundation\Testing\RefreshDatabase; +use Pest\PendingCalls\TestCall; use Tests\Support\AssertsNoOutboundHttp; use Tests\Support\FailHardGraphClient; @@ -75,12 +76,44 @@ function something() // .. } +/** + * Spec test naming helper. + * + * Convention for focused runs: + * - Prefix every Spec 081 test title with "Spec081 ". + * - Keep filenames suffixed with "Spec081Test.php". + * - Command: `vendor/bin/sail artisan test --compact --filter=Spec081` + */ +function spec081(string $description): string +{ + $normalized = trim($description); + + if ($normalized === '') { + return 'Spec081'; + } + + return str_starts_with($normalized, 'Spec081 ') + ? $normalized + : 'Spec081 '.$normalized; +} + +/** + * Convenience wrapper for Spec 081 tests. + */ +function itSpec081(string $description, ?\Closure $closure = null): TestCall +{ + $call = it(spec081($description), $closure); + $call->group('spec081'); + + return $call; +} + function bindFailHardGraphClient(): void { app()->instance(GraphClientInterface::class, new FailHardGraphClient); } -function assertNoOutboundHttp(Closure $callback): mixed +function assertNoOutboundHttp(\Closure $callback): mixed { return AssertsNoOutboundHttp::run($callback); } diff --git a/tests/Unit/Badges/OperationRunBadgesTest.php b/tests/Unit/Badges/OperationRunBadgesTest.php index 515b6ec..3da2313 100644 --- a/tests/Unit/Badges/OperationRunBadgesTest.php +++ b/tests/Unit/Badges/OperationRunBadgesTest.php @@ -32,6 +32,10 @@ expect($partial->label)->toBe('Partially succeeded'); expect($partial->color)->toBe('warning'); + $blocked = BadgeCatalog::spec(BadgeDomain::OperationRunOutcome, 'blocked'); + expect($blocked->label)->toBe('Blocked'); + expect($blocked->color)->toBe('warning'); + $failed = BadgeCatalog::spec(BadgeDomain::OperationRunOutcome, 'failed'); expect($failed->label)->toBe('Failed'); expect($failed->color)->toBe('danger'); diff --git a/tests/Unit/OpsUx/RunFailureSanitizerTest.php b/tests/Unit/OpsUx/RunFailureSanitizerTest.php index 0123a16..f00ecc5 100644 --- a/tests/Unit/OpsUx/RunFailureSanitizerTest.php +++ b/tests/Unit/OpsUx/RunFailureSanitizerTest.php @@ -1,12 +1,13 @@ toBe(RunFailureSanitizer::REASON_PROVIDER_AUTH_FAILED); - expect(RunFailureSanitizer::normalizeReasonCode('AADSTS700016'))->toBe(RunFailureSanitizer::REASON_PROVIDER_AUTH_FAILED); - expect(RunFailureSanitizer::normalizeReasonCode('bad_gateway'))->toBe(RunFailureSanitizer::REASON_PROVIDER_OUTAGE); - expect(RunFailureSanitizer::normalizeReasonCode('500'))->toBe(RunFailureSanitizer::REASON_PROVIDER_OUTAGE); + expect(RunFailureSanitizer::normalizeReasonCode('invalid_client'))->toBe(ProviderReasonCodes::ProviderAuthFailed); + expect(RunFailureSanitizer::normalizeReasonCode('AADSTS700016'))->toBe(ProviderReasonCodes::ProviderAuthFailed); + expect(RunFailureSanitizer::normalizeReasonCode('bad_gateway'))->toBe(ProviderReasonCodes::NetworkUnreachable); + expect(RunFailureSanitizer::normalizeReasonCode('500'))->toBe(ProviderReasonCodes::NetworkUnreachable); }); it('redacts common secret patterns and forbidden substrings', function (): void { diff --git a/tests/Unit/PolicySnapshotServiceTest.php b/tests/Unit/PolicySnapshotServiceTest.php index 36adc77..1814cbb 100644 --- a/tests/Unit/PolicySnapshotServiceTest.php +++ b/tests/Unit/PolicySnapshotServiceTest.php @@ -1,6 +1,8 @@ create([ + 'tenant_id' => $tenantIdentifier, + 'app_client_id' => null, + 'app_client_secret' => null, + 'is_current' => true, + ]); + + $connection = ProviderConnection::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'provider' => 'microsoft', + 'is_default' => true, + 'status' => 'connected', + 'entra_tenant_id' => $tenantIdentifier, + ]); + + ProviderCredential::factory()->create([ + 'provider_connection_id' => (int) $connection->getKey(), + 'type' => 'client_secret', + 'payload' => [ + 'client_id' => 'provider-client-'.$tenant->getKey(), + 'client_secret' => 'provider-secret-'.$tenant->getKey(), + ], + ]); + + return $tenant; +} + class PolicySnapshotGraphClient implements GraphClientInterface { public array $requests = []; @@ -288,12 +319,7 @@ public function request(string $method, string $path, array $options = []): Grap $client = new PolicySnapshotGraphClient; app()->instance(GraphClientInterface::class, $client); - $tenant = Tenant::factory()->create([ - 'tenant_id' => 'tenant-compliance', - 'app_client_id' => 'client-123', - 'app_client_secret' => 'secret-123', - 'is_current' => true, - ]); + $tenant = tenantWithDefaultMicrosoftConnectionForPolicySnapshot('tenant-compliance'); $tenant->makeCurrent(); $policy = Policy::factory()->create([ @@ -324,12 +350,7 @@ public function request(string $method, string $path, array $options = []): Grap $client = new ConfigurationPolicySettingsSnapshotGraphClient; app()->instance(GraphClientInterface::class, $client); - $tenant = Tenant::factory()->create([ - 'tenant_id' => 'tenant-endpoint-security', - 'app_client_id' => 'client-123', - 'app_client_secret' => 'secret-123', - 'is_current' => true, - ]); + $tenant = tenantWithDefaultMicrosoftConnectionForPolicySnapshot('tenant-endpoint-security'); $tenant->makeCurrent(); $policy = Policy::factory()->create([ @@ -363,12 +384,7 @@ public function request(string $method, string $path, array $options = []): Grap $client = new EnrollmentNotificationSnapshotGraphClient; app()->instance(GraphClientInterface::class, $client); - $tenant = Tenant::factory()->create([ - 'tenant_id' => 'tenant-enrollment-notifications', - 'app_client_id' => 'client-123', - 'app_client_secret' => 'secret-123', - 'is_current' => true, - ]); + $tenant = tenantWithDefaultMicrosoftConnectionForPolicySnapshot('tenant-enrollment-notifications'); $tenant->makeCurrent(); $policy = Policy::factory()->create([ @@ -404,12 +420,7 @@ public function request(string $method, string $path, array $options = []): Grap $client = new PolicySnapshotGraphClient; app()->instance(GraphClientInterface::class, $client); - $tenant = Tenant::factory()->create([ - 'tenant_id' => 'tenant-apps', - 'app_client_id' => 'client-123', - 'app_client_secret' => 'secret-123', - 'is_current' => true, - ]); + $tenant = tenantWithDefaultMicrosoftConnectionForPolicySnapshot('tenant-apps'); $tenant->makeCurrent(); $policy = Policy::factory()->create([ @@ -449,12 +460,7 @@ public function request(string $method, string $path, array $options = []): Grap $client = new PolicySnapshotGraphClient; app()->instance(GraphClientInterface::class, $client); - $tenant = Tenant::factory()->create([ - 'tenant_id' => 'tenant-driver', - 'app_client_id' => 'client-123', - 'app_client_secret' => 'secret-123', - 'is_current' => true, - ]); + $tenant = tenantWithDefaultMicrosoftConnectionForPolicySnapshot('tenant-driver'); $tenant->makeCurrent(); $policy = Policy::factory()->create([ @@ -488,12 +494,7 @@ public function request(string $method, string $path, array $options = []): Grap app()->instance(\App\Services\Graph\GraphClientInterface::class, $client); - $tenant = Tenant::factory()->create([ - 'tenant_id' => 'tenant-mam-fallback', - 'app_client_id' => 'client-123', - 'app_client_secret' => 'secret-123', - 'is_current' => true, - ]); + $tenant = tenantWithDefaultMicrosoftConnectionForPolicySnapshot('tenant-mam-fallback'); $policy = Policy::factory()->create([ 'tenant_id' => $tenant->id, @@ -528,12 +529,7 @@ public function request(string $method, string $path, array $options = []): Grap app()->instance(\App\Services\Graph\GraphClientInterface::class, $client); - $tenant = Tenant::factory()->create([ - 'tenant_id' => 'tenant-404', - 'app_client_id' => 'client-123', - 'app_client_secret' => 'secret-123', - 'is_current' => true, - ]); + $tenant = tenantWithDefaultMicrosoftConnectionForPolicySnapshot('tenant-404'); $policy = Policy::factory()->create([ 'tenant_id' => $tenant->id, @@ -573,12 +569,7 @@ public function request(string $method, string $path, array $options = []): Grap app()->instance(\App\Services\Graph\GraphClientInterface::class, $client); - $tenant = Tenant::factory()->create([ - 'tenant_id' => 'tenant-mam-fallback-response', - 'app_client_id' => 'client-123', - 'app_client_secret' => 'secret-123', - 'is_current' => true, - ]); + $tenant = tenantWithDefaultMicrosoftConnectionForPolicySnapshot('tenant-mam-fallback-response'); $policy = Policy::factory()->create([ 'tenant_id' => $tenant->id, @@ -653,12 +644,7 @@ public function request(string $method, string $path, array $options = []): Grap $client = new WindowsUpdateRingSnapshotGraphClient; app()->instance(GraphClientInterface::class, $client); - $tenant = Tenant::factory()->create([ - 'tenant_id' => 'tenant-wuring', - 'app_client_id' => 'client-123', - 'app_client_secret' => 'secret-123', - 'is_current' => true, - ]); + $tenant = tenantWithDefaultMicrosoftConnectionForPolicySnapshot('tenant-wuring'); $tenant->makeCurrent(); $policy = Policy::factory()->create([ @@ -727,12 +713,7 @@ public function request(string $method, string $path, array $options = []): Grap it('returns actionable reasons when graph snapshot fails', function () { app()->instance(GraphClientInterface::class, new FailedSnapshotGraphClient); - $tenant = Tenant::factory()->create([ - 'tenant_id' => 'tenant-failure', - 'app_client_id' => 'client-123', - 'app_client_secret' => 'secret-123', - 'is_current' => true, - ]); + $tenant = tenantWithDefaultMicrosoftConnectionForPolicySnapshot('tenant-failure'); $tenant->makeCurrent(); $policy = Policy::factory()->create([ diff --git a/tests/Unit/Providers/ProviderGatewayTest.php b/tests/Unit/Providers/ProviderGatewayTest.php index a3a090a..e433b0d 100644 --- a/tests/Unit/Providers/ProviderGatewayTest.php +++ b/tests/Unit/Providers/ProviderGatewayTest.php @@ -76,12 +76,18 @@ public function request(string $method, string $path, array $options = []): Grap ); $gateway->getOrganization($connection); + $gateway->getPolicy($connection, 'deviceConfiguration', 'policy-1', ['platform' => 'windows']); + $gateway->applyPolicy($connection, 'deviceConfiguration', 'policy-1', ['displayName' => 'Policy'], ['method' => 'PATCH']); + $gateway->getServicePrincipalPermissions($connection, ['query' => ['$select' => 'id']]); $gateway->request($connection, 'GET', 'organization', ['query' => ['a' => 'b']]); - expect($graph->calls)->toHaveCount(2); + expect($graph->calls)->toHaveCount(5); $first = $graph->calls[0]['options']; $second = $graph->calls[1]['options']; + $third = $graph->calls[2]['options']; + $fourth = $graph->calls[3]['options']; + $fifth = $graph->calls[4]['options']; expect($first['tenant'])->toBe('entra-tenant-id'); expect($first['client_id'])->toBe('client-id'); @@ -92,5 +98,23 @@ public function request(string $method, string $path, array $options = []): Grap expect($second['client_id'])->toBe('client-id'); expect($second['client_secret'])->toBe('client-secret'); expect($second['client_request_id'])->toBeString()->not->toBeEmpty(); - expect($second['query'])->toBe(['a' => 'b']); + expect($second['platform'])->toBe('windows'); + + expect($third['tenant'])->toBe('entra-tenant-id'); + expect($third['client_id'])->toBe('client-id'); + expect($third['client_secret'])->toBe('client-secret'); + expect($third['client_request_id'])->toBeString()->not->toBeEmpty(); + expect($third['method'])->toBe('PATCH'); + + expect($fourth['tenant'])->toBe('entra-tenant-id'); + expect($fourth['client_id'])->toBe('client-id'); + expect($fourth['client_secret'])->toBe('client-secret'); + expect($fourth['client_request_id'])->toBeString()->not->toBeEmpty(); + expect($fourth['query'])->toBe(['$select' => 'id']); + + expect($fifth['tenant'])->toBe('entra-tenant-id'); + expect($fifth['client_id'])->toBe('client-id'); + expect($fifth['client_secret'])->toBe('client-secret'); + expect($fifth['client_request_id'])->toBeString()->not->toBeEmpty(); + expect($fifth['query'])->toBe(['a' => 'b']); }); diff --git a/tests/Unit/Providers/ProviderOperationStartGateTest.php b/tests/Unit/Providers/ProviderOperationStartGateTest.php index ad23949..003a2ce 100644 --- a/tests/Unit/Providers/ProviderOperationStartGateTest.php +++ b/tests/Unit/Providers/ProviderOperationStartGateTest.php @@ -2,8 +2,12 @@ use App\Models\OperationRun; use App\Models\ProviderConnection; +use App\Models\ProviderCredential; use App\Models\Tenant; use App\Services\Providers\ProviderOperationStartGate; +use App\Support\OperationRunOutcome; +use App\Support\OperationRunStatus; +use App\Support\Providers\ProviderReasonCodes; use Illuminate\Foundation\Testing\RefreshDatabase; uses(RefreshDatabase::class); @@ -14,6 +18,10 @@ 'tenant_id' => $tenant->getKey(), 'provider' => 'microsoft', 'entra_tenant_id' => 'entra-tenant-id', + 'status' => 'connected', + ]); + ProviderCredential::factory()->create([ + 'provider_connection_id' => (int) $connection->getKey(), ]); $dispatched = 0; @@ -53,6 +61,10 @@ $tenant = Tenant::factory()->create(); $connection = ProviderConnection::factory()->create([ 'tenant_id' => $tenant->getKey(), + 'status' => 'connected', + ]); + ProviderCredential::factory()->create([ + 'provider_connection_id' => (int) $connection->getKey(), ]); $existing = OperationRun::factory()->create([ @@ -86,6 +98,10 @@ $tenant = Tenant::factory()->create(); $connection = ProviderConnection::factory()->create([ 'tenant_id' => $tenant->getKey(), + 'status' => 'connected', + ]); + ProviderCredential::factory()->create([ + 'provider_connection_id' => (int) $connection->getKey(), ]); $blocking = OperationRun::factory()->create([ @@ -114,3 +130,27 @@ expect($result->run->getKey())->toBe($blocking->getKey()); expect(OperationRun::query()->where('tenant_id', $tenant->getKey())->count())->toBe(1); }); + +it('returns blocked and stores reason metadata when no default connection exists', function (): void { + $tenant = Tenant::factory()->create(); + + $dispatched = 0; + $gate = app(ProviderOperationStartGate::class); + + $result = $gate->start( + tenant: $tenant, + connection: null, + operationType: 'provider.connection.check', + dispatcher: function () use (&$dispatched): void { + $dispatched++; + }, + ); + + expect($dispatched)->toBe(0); + expect($result->status)->toBe('blocked'); + expect($result->dispatched)->toBeFalse(); + expect($result->run->status)->toBe(OperationRunStatus::Completed->value); + expect($result->run->outcome)->toBe(OperationRunOutcome::Blocked->value); + expect($result->run->context['reason_code'] ?? null)->toBe(ProviderReasonCodes::ProviderConnectionMissing); + expect($result->run->context['next_steps'] ?? [])->not->toBeEmpty(); +}); diff --git a/tests/Unit/RbacOnboardingServiceTest.php b/tests/Unit/RbacOnboardingServiceTest.php index 40d2c6e..06e8054 100644 --- a/tests/Unit/RbacOnboardingServiceTest.php +++ b/tests/Unit/RbacOnboardingServiceTest.php @@ -1,5 +1,7 @@ '00000000-0000-0000-0000-000000000000', 'name' => 'Tenant One', - 'app_client_id' => 'app-client-123', - 'app_client_secret' => 'secret', + 'app_client_id' => null, + 'app_client_secret' => null, 'is_current' => true, ]); + + $connection = ProviderConnection::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'provider' => 'microsoft', + 'is_default' => true, + 'status' => 'connected', + 'entra_tenant_id' => (string) $tenant->tenant_id, + ]); + + ProviderCredential::factory()->create([ + 'provider_connection_id' => (int) $connection->getKey(), + 'type' => 'client_secret', + 'payload' => [ + 'client_id' => 'app-client-123', + 'client_secret' => 'secret', + ], + ]); + + return $tenant; } it('creates group membership and role assignment', function () { diff --git a/tests/Unit/ScopeTagResolverTest.php b/tests/Unit/ScopeTagResolverTest.php index cd64002..0ee90da 100644 --- a/tests/Unit/ScopeTagResolverTest.php +++ b/tests/Unit/ScopeTagResolverTest.php @@ -1,29 +1,47 @@ create(); + $connection = ProviderConnection::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'provider' => 'microsoft', + 'is_default' => true, + 'status' => 'connected', + ]); + ProviderCredential::factory()->create([ + 'provider_connection_id' => (int) $connection->getKey(), + ]); - $mockGraphClient = Mockery::mock(MicrosoftGraphClient::class); - $mockGraphClient->shouldReceive('request') - ->with('GET', '/deviceManagement/roleScopeTags', Mockery::on(function ($options) use ($tenant) { - return $options['query']['$select'] === 'id,displayName' - && $options['tenant'] === $tenant->external_id - && $options['client_id'] === $tenant->app_client_id - && $options['client_secret'] === $tenant->app_client_secret; - })) + $graph = Mockery::mock(GraphClientInterface::class); + $graph->shouldReceive('request') + ->with( + 'GET', + '/deviceManagement/roleScopeTags', + Mockery::on(function (array $options) use ($connection): bool { + return ($options['query']['$select'] ?? null) === 'id,displayName' + && ($options['tenant'] ?? null) === $connection->entra_tenant_id + && is_string($options['client_id'] ?? null) + && is_string($options['client_secret'] ?? null); + }), + ) ->once() ->andReturn(new GraphResponse( success: true, @@ -33,12 +51,12 @@ ['id' => '1', 'displayName' => 'Verbund-1'], ['id' => '2', 'displayName' => 'Verbund-2'], ], - ] + ], )); - $mockLogger = Mockery::mock(GraphLogger::class); + $gateway = new ProviderGateway($graph, new CredentialManager); - $resolver = new ScopeTagResolver($mockGraphClient, $mockLogger); + $resolver = new ScopeTagResolver(app(ProviderConnectionResolver::class), $gateway); $result = $resolver->resolve(['0', '1', '2'], $tenant); expect($result)->toBe([ @@ -50,27 +68,32 @@ test('caches scope tag objects for 1 hour', function () { $tenant = Tenant::factory()->create(); + $connection = ProviderConnection::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'provider' => 'microsoft', + 'is_default' => true, + 'status' => 'connected', + ]); + ProviderCredential::factory()->create([ + 'provider_connection_id' => (int) $connection->getKey(), + ]); - $mockGraphClient = Mockery::mock(MicrosoftGraphClient::class); - $mockGraphClient->shouldReceive('request') - ->once() // Only called once due to caching + $graph = Mockery::mock(GraphClientInterface::class); + $graph->shouldReceive('request') + ->once() ->andReturn(new GraphResponse( success: true, data: [ 'value' => [ ['id' => '0', 'displayName' => 'Default'], ], - ] + ], )); - $mockLogger = Mockery::mock(GraphLogger::class); + $gateway = new ProviderGateway($graph, new CredentialManager); + $resolver = new ScopeTagResolver(app(ProviderConnectionResolver::class), $gateway); - $resolver = new ScopeTagResolver($mockGraphClient, $mockLogger); - - // First call - fetches from API $result1 = $resolver->resolve(['0'], $tenant); - - // Second call - should use cache $result2 = $resolver->resolve(['0'], $tenant); expect($result1)->toBe([['id' => '0', 'displayName' => 'Default']]); @@ -79,10 +102,11 @@ test('returns empty array for empty input', function () { $tenant = Tenant::factory()->create(); - $mockGraphClient = Mockery::mock(MicrosoftGraphClient::class); - $mockLogger = Mockery::mock(GraphLogger::class); + $graph = Mockery::mock(GraphClientInterface::class); - $resolver = new ScopeTagResolver($mockGraphClient, $mockLogger); + $gateway = new ProviderGateway($graph, new CredentialManager); + + $resolver = new ScopeTagResolver(app(ProviderConnectionResolver::class), $gateway); $result = $resolver->resolve([], $tenant); expect($result)->toBe([]); @@ -90,30 +114,47 @@ test('handles 403 forbidden gracefully', function () { $tenant = Tenant::factory()->create(); + $connection = ProviderConnection::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'provider' => 'microsoft', + 'is_default' => true, + 'status' => 'connected', + ]); + ProviderCredential::factory()->create([ + 'provider_connection_id' => (int) $connection->getKey(), + ]); - $mockGraphClient = Mockery::mock(MicrosoftGraphClient::class); - $mockGraphClient->shouldReceive('request') + $graph = Mockery::mock(GraphClientInterface::class); + $graph->shouldReceive('request') ->once() ->andReturn(new GraphResponse( success: false, status: 403, - data: [] + data: [], )); - $mockLogger = Mockery::mock(GraphLogger::class); + $gateway = new ProviderGateway($graph, new CredentialManager); - $resolver = new ScopeTagResolver($mockGraphClient, $mockLogger); + $resolver = new ScopeTagResolver(app(ProviderConnectionResolver::class), $gateway); $result = $resolver->resolve(['0', '1'], $tenant); - // Should return empty array when 403 expect($result)->toBe([]); }); test('filters returned scope tags to requested IDs', function () { $tenant = Tenant::factory()->create(); + $connection = ProviderConnection::factory()->create([ + 'tenant_id' => (int) $tenant->getKey(), + 'provider' => 'microsoft', + 'is_default' => true, + 'status' => 'connected', + ]); + ProviderCredential::factory()->create([ + 'provider_connection_id' => (int) $connection->getKey(), + ]); - $mockGraphClient = Mockery::mock(MicrosoftGraphClient::class); - $mockGraphClient->shouldReceive('request') + $graph = Mockery::mock(GraphClientInterface::class); + $graph->shouldReceive('request') ->once() ->andReturn(new GraphResponse( success: true, @@ -123,17 +164,15 @@ ['id' => '1', 'displayName' => 'Verbund-1'], ['id' => '2', 'displayName' => 'Verbund-2'], ], - ] + ], )); - $mockLogger = Mockery::mock(GraphLogger::class); + $gateway = new ProviderGateway($graph, new CredentialManager); - $resolver = new ScopeTagResolver($mockGraphClient, $mockLogger); - // Request only IDs 0 and 2 + $resolver = new ScopeTagResolver(app(ProviderConnectionResolver::class), $gateway); $result = $resolver->resolve(['0', '2'], $tenant); expect($result)->toHaveCount(2); - // Note: array_filter preserves keys, so result will be [0 => ..., 2 => ...] expect($result[0])->toBe(['id' => '0', 'displayName' => 'Default']); expect($result[2])->toBe(['id' => '2', 'displayName' => 'Verbund-2']); }); diff --git a/tests/Unit/TenantPermissionCheckClustersTest.php b/tests/Unit/TenantPermissionCheckClustersTest.php index 46b1abd..6c934c2 100644 --- a/tests/Unit/TenantPermissionCheckClustersTest.php +++ b/tests/Unit/TenantPermissionCheckClustersTest.php @@ -128,6 +128,7 @@ expect($adminConsentCheck)->toBeArray(); expect($adminConsentCheck['status'] ?? null)->toBe(VerificationCheckStatus::Warn->value); expect($adminConsentCheck['blocking'] ?? null)->toBeFalse(); - expect($adminConsentCheck['reason_code'] ?? null)->toBe('throttled'); + expect($adminConsentCheck['reason_code'] ?? null)->toBe('rate_limited'); + expect($adminConsentCheck['next_steps'] ?? [])->not->toBeEmpty(); expect((string) ($adminConsentCheck['message'] ?? ''))->toContain('Unable to refresh observed permissions inventory'); });