feat(integration-registry): contract + registry + external router (umbrella PR 1/N)

[rubenvdlinde]

Closes part of #1307.

First slice of the pluggable-integration-registry umbrella. Lands the foundation that every leaf change (W1-W3) depends on: the provider contract, the registry, external-routing plumbing, and the query-time storage-strategy helper. 6 of 69 umbrella tasks complete (Backend — Provider Contract & Registry section).

What's in

File	Role
lib/Service/Integration/IntegrationProvider.php	15-method interface per design.md normative contract
lib/Service/Integration/AbstractIntegrationProvider.php	Base class — group=null, requiresPermission=null, auth=['type'=>'none'], no OpenConnector source, get/create/update/delete default to NotImplementedException
lib/Service/Integration/IntegrationRegistry.php	Explicit addProvider() registration with collision detection (AD-13) + external-source rejection (AD-4). list() / listIds() / get() / getEnabled(); withProviders() test seam
lib/Service/Integration/ExternalIntegrationRouter.php	Dispatches storage='external' CRUD through OpenConnector. Surfaces failures via ProviderUnavailableException with 3-way cause classification per AD-23. Includes probe() for admin/OCS health
lib/Service/Integration/QueryTimeContract.php	Codifies the 2 s render timeout (AD-22) + HTTP 501 envelope builder for ObjectsController to consume in tasks 7-11
lib/Exception/NotImplementedException.php	Thrown by query-time / list-only providers' unsupported CRUD
lib/Exception/ProviderUnavailableException.php	Carries the 3-way cause (openconnector-down / openconnector-source-missing / upstream-service-down) + getDetails()
lib/AppInfo/Application.php	New registerIntegrationRegistry() phase wires both as shared per-request singletons
tests/Unit/Service/Integration/*Test.php	25 unit tests across 4 classes (registry, abstract provider, external router, query-time contract)

Spec deviation flagged

Tasks.md said "DI-tag-based discovery" but modern Nextcloud doesn't expose IAppContainer::queryAll(<tag>) as a public API. Switched to explicit addProvider() at app bootstrap — identical semantics, NC-compatible. Documented in:

• IntegrationRegistry.php class docblock
• openspec/changes/pluggable-integration-registry/tasks.md (inline note on the checked task)
• openspec/changes/pluggable-integration-registry/plan.json (status: done, description annotated)

What's still pending in the umbrella

• Schema validator refactor (tasks 7-11): Schema::validateLinkedTypesValue consumes IntegrationRegistry::listIds; deprecate VALID_LINKED_TYPES + LinkedEntityService::TYPE_COLUMN_MAP
• 5 Built-in providers (tasks 12-17): files/notes/tasks/tags/audit-trail wrappers
• Routes, controller, capabilities (tasks 18-22): IntegrationsController, ObjectsController sub-resource dispatch, OCS capabilities block
• Admin UI + frontend registry + 3 widgets + Cn refactor* (tasks 23-46)
• Tests / CI parity gate / scaffold / ADR / translations (tasks 47-67)
• E2E acceptance (tasks 68-71)

Each follow-up PR will reference the same issue #1307 and tick its plan.json + tasks.md as it goes.

Test plan

• All new files pass php -l syntax check
• vendor/bin/phpunit -c phpunit-unit.xml tests/Unit/Service/Integration/ — 25 tests, 42 assertions, all pass (run inside the openregister-postgres dev container)
• composer check:strict — deferred to PR 2 once the strict-quality gates are wired against the new lib/Service/Integration/ subdir
• Frontend changes — none in this PR; first frontend touchpoint lands in PR 3 of the umbrella

🤖 Generated with Claude Code

[github-actions]

Quality Report — ConductionNL/openregister @ 34d7f32

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	❌
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		❌
stylelint		✅
composer			✅	❌ 1/153 denied
npm			✅	✅ 598/598
PHPUnit					⏭️
Newman					⏭️
Playwright					⏭️

❌ Denied composer licenses

Package	Version	License
dompdf/dompdf	v3.1.5	LGPL-2.1

---

Quality workflow — 2026-05-11 12:07 UTC

Download the full PDF report from the workflow artifacts.

[WilcoLouwerse]

[BLOCKER] No caller-authorization check before dispatching to provider (IDOR class)

ExternalIntegrationRouter::call() receives a provider and immediately dispatches an HTTP call through OpenConnector. There is zero check that the calling user is allowed to invoke this provider: no requiresPermission() enforcement, no tenant-scope guard, no object-ownership verification. The IntegrationProvider interface declares requiresPermission(): ?string and the docblock states 'the registry calls AuthorizationService with the current user and target object to gate visibility (AD-16)' — but that call is never made. Fix: ExternalIntegrationRouter::call() must accept an IUser (or string $userId) and enforce requiresPermission() via AuthorizationService before invoking.

[WilcoLouwerse]

[BLOCKER] Gate 7 — silent null from get() collapses auth decision

IntegrationRegistry::get() returns null for an unknown id. Any consumer that calls get($id) to decide whether to proceed will silently skip the dispatch when the registry is misconfigured. No distinction between 'provider does not exist on this instance' (valid no-op) and 'provider exists but failed to register' (should surface an error). Fix: distinguish the two null cases (introduce a ProviderNotFoundException or return a typed result object).

[WilcoLouwerse]

[BLOCKER] Gate 3 — abstract base ships NotImplementedException stubs as trapdoors for CRUD operations

AbstractIntegrationProvider implements get(), create(), update(), delete() as throw-by-default stubs. Every concrete provider that forgets to override a mutation method will silently inherit a NotImplementedException-throwing body. A provider declaring storage='link-table' but forgetting to override create() will throw at runtime — translated to HTTP 501, which controllers may interpret as 'query-time provider'. Remove default implementations; keep them in a separate AbstractQueryTimeProvider sub-class so non-query-time providers must implement them.

[WilcoLouwerse]

[BLOCKER] No caller-authorization check before dispatching to provider (IDOR class)

ExternalIntegrationRouter::call() receives a provider and immediately dispatches an HTTP call through OpenConnector. There is zero check that the calling user is allowed to invoke this provider: no requiresPermission() enforcement, no tenant-scope guard, no object-ownership verification. The IntegrationProvider interface declares requiresPermission(): ?string and the docblock states 'the registry calls AuthorizationService with the current user and target object to gate visibility (AD-16)' — but that call is never made. Fix: ExternalIntegrationRouter::call() must accept an IUser (or string $userId) and enforce requiresPermission() via AuthorizationService before invoking.

[WilcoLouwerse]

[BLOCKER] Gate 7 — silent null from get() collapses auth decision

IntegrationRegistry::get() returns null for an unknown id. Any consumer that calls get($id) to decide whether to proceed will silently skip the dispatch when the registry is misconfigured. No distinction between 'provider does not exist on this instance' (valid no-op) and 'provider exists but failed to register' (should surface an error). Fix: distinguish the two null cases (introduce a ProviderNotFoundException or return a typed result object).

[WilcoLouwerse]

[BLOCKER] Gate 3 — abstract base ships NotImplementedException stubs as trapdoors for CRUD operations

AbstractIntegrationProvider implements get(), create(), update(), delete() as throw-by-default stubs. Every concrete provider that forgets to override a mutation method will silently inherit a NotImplementedException-throwing body. A provider declaring storage='link-table' but forgetting to override create() will throw at runtime — translated to HTTP 501, which controllers may interpret as 'query-time provider'. Remove default implementations; keep them in a separate AbstractQueryTimeProvider sub-class so non-query-time providers must implement them.

[WilcoLouwerse]

[BLOCKER] Gate 7 — broad Throwable catch in loadSource() reclassifies all exceptions as source-missing

loadSource() catches \Throwable (line 691) and unconditionally rethrows as ProviderUnavailableException with cause CAUSE_OPENCONNECTOR_SOURCE_MISSING. A DI container resolution failure, DB connection error, or OOM during SourceMapper::find() is reported as 'OpenConnector source missing — Reconfigure connector'. Callers catching ProviderUnavailableException will silently treat infrastructure failures as configuration problems. Only catch specific mapper-not-found exceptions; let infrastructure failures propagate.

[WilcoLouwerse]

[BLOCKER] Gate 1 — SPDX machine-readable identifiers missing from all 6 new lib/ files

All 6 new lib/ files (IntegrationProvider.php, AbstractIntegrationProvider.php, ExternalIntegrationRouter.php, IntegrationRegistry.php, QueryTimeContract.php, NotImplementedException.php, ProviderUnavailableException.php) contain only the human-readable @license PHPDoc tag, not the SPDX identifiers. CI License composer check is already failing. Add SPDX-License-Identifier: EUPL-1.2 and SPDX-FileCopyrightText: 2026 Conduction B.V. lines.

[WilcoLouwerse]

[BLOCKER] withProviders() is public on the shared singleton

withProviders() is declared public on the per-request singleton. Any code calling IntegrationRegistry::get()->withProviders([]) will destroy every registered provider for the rest of the request. The docblock says 'Production code SHOULD use addProvider()' — non-binding. An attacker-controlled provider reaching the DI container can flush all other providers. Make withProviders() @internal, throw LogicException unless defined('PHPUNIT_RUN'), or expose only via a test-seam interface not bound in production DI.

[WilcoLouwerse]

[CONCERN] getStorageStrategy() returns unvalidated string — typo-prone

getStorageStrategy() is typed string with the valid set documented only in a docblock. A typo like 'link_table', 'external ', 'External' passes registry validation and fails only at dispatch time deep inside assertProviderIsExternal() with a misleading \LogicException. Introduce a StorageStrategy enum (PHP 8.1) or const STORAGE_* constants on the interface, validated in addProvider().

[WilcoLouwerse]

[CONCERN] No test for provider registration order or duplicate-id determinism

Multiple apps call addProvider() from Application::register() hooks during bootstrap. PHP is single-threaded per request but bootstrap order is non-deterministic across NC versions. Duplicate-id 'first wins' combined with unordered bootstrap means the winner varies between deployments. No test asserts bootstrap-order determinism. Add such a test and document priority ordering in addProvider().

[WilcoLouwerse]

[CONCERN] isEnabledForUser('openconnector') called without a user — result cached per-request

isOpenConnectorAvailable() calls appManager->isEnabledForUser('openconnector') without a user parameter and caches the result. NC's isEnabledForUser() without second arg checks global state, not per-user. For cli/batch/system jobs spanning multiple users the result may be wrong for some. Accept ?IUser in the constructor or document instance-level scope only.

[WilcoLouwerse]

[CONCERN] invoke() uses dynamic method_exists dispatch against an untrusted service

invoke() resolves OpenConnector's CallService from DI then checks method_exists for call and request. Any object registered under that DI key is accepted without type verification. A different version registering a different CallService (e.g. void-returning, different exception hierarchy) is silently invoked. Declare a local ICallService interface, verify implementation, throw ProviderUnavailableException(CAUSE_OPENCONNECTOR_DOWN) if mismatched.

[WilcoLouwerse]

[CONCERN] QueryTimeContract timeout is advisory only — no enforcement

RENDER_TIMEOUT_SECONDS = 2.0 defines the timeout but no enforcement mechanism exists: no pcntl_alarm, no set_time_limit, no wrapper measuring elapsed time. The constant is advisory; tests only assert the value, not enforcement. Downstream consumers may skip wrapping. Add a static helper QueryTimeContract::wrapWithTimeout(callable $fn): array that enforces the 2s limit so consumers have a ready-made enforcer.

[WilcoLouwerse]

[CONCERN] No happy-path test for the router — upstream call path completely untested

The test file states 'actual upstream call path is exercised in integration tests' but no such integration tests exist in this PR. All 4 tests only verify failure modes. invoke() and decodeResponse() have non-trivial branching (3 response-shape branches, 2 CallService method signatures) with zero coverage. Add at least one happy-path test with a mocked ContainerInterface returning a CallService stub with call() returning an array.

[WilcoLouwerse]

[CONCERN] getEnabled() iterates all providers calling isEnabled() — no caching

On instances with 20+ providers, getEnabled() becomes O(n) IAppManager calls per invocation, and is called on every API request (capabilities, schema validation). Interface docblock mentions per-request cache but registry provides none. Memoize the result within the request, or document isEnabled() MUST be O(1) (no IO) and enforce via test.

[WilcoLouwerse]

Review

🔴 Blockers (6)

• No caller-authorization check before dispatching to provider (IDOR class) — lib/Service/Integration/ExternalIntegrationRouter.php:501
• Gate 7 — silent null from get() collapses auth decision — lib/Service/Integration/IntegrationRegistry.php:1232
• Gate 3 — abstract base ships NotImplementedException stubs as trapdoors for CRUD operations — lib/Service/Integration/AbstractIntegrationProvider.php:244
• Gate 7 — broad Throwable catch in loadSource() reclassifies all exceptions as source-missing — lib/Service/Integration/ExternalIntegrationRouter.php:671
• Gate 1 — SPDX machine-readable identifiers missing from all 6 new lib/ files — lib/Service/Integration/IntegrationProvider.php:1
• withProviders() is public on the shared singleton — lib/Service/Integration/IntegrationRegistry.php:1193

🟡 Concerns (7)

• getStorageStrategy() returns unvalidated string — typo-prone — lib/Service/Integration/IntegrationProvider.php:900
• No test for provider registration order or duplicate-id determinism — lib/Service/Integration/IntegrationRegistry.php:1151
• isEnabledForUser('openconnector') called without a user — result cached per-request — lib/Service/Integration/ExternalIntegrationRouter.php:648
• invoke() uses dynamic method_exists dispatch against an untrusted service — lib/Service/Integration/ExternalIntegrationRouter.php:721
• QueryTimeContract timeout is advisory only — no enforcement — lib/Service/Integration/QueryTimeContract.php:1303
• No happy-path test for the router — upstream call path completely untested — tests/Unit/Service/Integration/ExternalIntegrationRouterTest.php:1607
• getEnabled() iterates all providers calling isEnabled() — no caching — lib/Service/Integration/IntegrationRegistry.php:1246

🟢 Minor (2)

• Interface has 16 methods but plan.json says 15 (lib/Service/Integration/IntegrationProvider.php:839)
  Counting: getId, getLabel, getIcon, getGroup, getRequiredApp, getStorageStrategy, getOpenConnectorSource, isEnabled, requiresPermission, authRequirements, list, get, create, update, delete, health = 16. Plan.json task 1.1 says 15. Spec and implementation diverged during authoring. Verify against design.md and update task description / plan.json count.
• IntegrationRegistry::list() shadows IntegrationProvider::list() (lib/Service/Integration/IntegrationRegistry.php:1206)
  Both interfaces declare a list() method with completely different semantics. Loosely-typed variables can call the wrong one. Rename the registry method to listProviders() or all() to disambiguate, especially since the registry's list() is 'irrespective of isEnabled()' while the provider's list() is the data-fetch operation.

Reviewed by WilcoLouwerse via automated batch review.