[backend] feat(multitenancy): adding builtin connectors for new tenants (#3505)#5633
Draft
Dimfacion wants to merge 14 commits intoissue/4864-url-fallbackfrom
Draft
[backend] feat(multitenancy): adding builtin connectors for new tenants (#3505)#5633Dimfacion wants to merge 14 commits intoissue/4864-url-fallbackfrom
Dimfacion wants to merge 14 commits intoissue/4864-url-fallbackfrom
Conversation
github-actions
Bot
added
the
filigran team
Dimfacion
changed the title
Codecov Report❌ Patch coverage is ❌ Your project check has failed because the head coverage (1.71%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## release/current #5633 +/- ##
=====================================================
+ Coverage 40.74% 40.78% +0.04%
- Complexity 5960 5970 +10
=====================================================
Files 2099 2102 +3
Lines 55583 55714 +131
Branches 6963 6968 +5
=====================================================
+ Hits 22645 22724 +79
- Misses 31605 31658 +53
+ Partials 1333 1332 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Dimfacion
force-pushed
the
issue/3505_add_builtin_connectors_2
branch
from
fba515a to
eb9bc4f
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates connector provisioning to support multi-tenancy where built-in connectors (collectors/injectors/executors) can exist with the same static ID across different tenants, and introduces startup/tenant-provisioning hooks to auto-register those built-ins.
Changes:
- Add a Flyway migration to move connector tables (collectors/injectors/executors) to composite primary keys
(id, tenant_id)and update related FKs. - Introduce a built-in connector registration mechanism (
BuiltinTenantRegistrable/BuiltinIntegrationFactory) and invoke it fromManagerFactoryfor all tenants. - Update repositories/services/tests to use tenant-scoped lookups (
findByIdAndTenantId) and adjust integration startup patterns accordingly.
Reviewed changes
Copilot reviewed 70 out of 70 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| openaev-model/src/main/java/io/openaev/database/repository/InjectorRepository.java | Add tenant-scoped lookup for injectors by id. |
| openaev-model/src/main/java/io/openaev/database/repository/ExecutorRepository.java | Add tenant-scoped lookup for executors by id. |
| openaev-model/src/main/java/io/openaev/database/repository/CollectorRepository.java | Add tenant-scoped lookup for collectors by id. |
| openaev-model/src/main/java/io/openaev/database/model/InjectorContract.java | Guard against null injectors/ids/names when building derived fields. |
| openaev-model/src/main/java/io/openaev/database/model/Injector.java | Adjust join table mapping to reference explicit columns under composite PK migration. |
| openaev-model/src/main/java/io/openaev/database/model/Inject.java | Adjust FK column mapping to injector id for composite PK migration. |
| openaev-model/src/main/java/io/openaev/database/model/BaseConnectorEntity.java | Add Persistable handling (isNew=false) for static-id connectors. |
| openaev-model/src/main/java/io/openaev/database/model/Agent.java | Adjust executor FK mapping to reference executor id for composite PK migration. |
| openaev-model/src/main/java/io/openaev/database/audit/TenantBaseListener.java | Allow suppressing tenant immutability assertion via thread-local flag. |
| openaev-model/src/main/java/io/openaev/database/audit/TenantAssertionControl.java | New thread-local suppression utility for tenant assertions. |
| openaev-api/src/test/java/io/openaev/utils/fixtures/InjectorFixture.java | Switch tests from Manager.monitorIntegrations() to per-tenant builtin registration. |
| openaev-api/src/test/java/io/openaev/utils/fixtures/InjectorContractFixture.java | Switch tests to per-tenant builtin registration for contracts. |
| openaev-api/src/test/java/io/openaev/service/tenants/TenantServiceTest.java | Add flush/clear in test to avoid persistence-context side effects. |
| openaev-api/src/test/java/io/openaev/security/OpenCTIJwtAuthenticationTest.java | Switch test setup to per-tenant builtin registration. |
| openaev-api/src/test/java/io/openaev/rest/threat_arsenal/ThreatArsenalApiTest.java | Switch test setup to per-tenant builtin registration. |
| openaev-api/src/test/java/io/openaev/rest/scenario/ScenarioInjectApiTest.java | Switch test setup to per-tenant builtin registration (multi-factory). |
| openaev-api/src/test/java/io/openaev/rest/payload/PayloadApiTest.java | Switch test setup to per-tenant builtin registration; remove redundant manager calls. |
| openaev-api/src/test/java/io/openaev/rest/inject/InjectImportTest.java | Switch test setup to per-tenant builtin registration (multi-factory). |
| openaev-api/src/test/java/io/openaev/rest/inject/InjectExportTest.java | Switch test setup to per-tenant builtin registration (multi-factory). |
| openaev-api/src/test/java/io/openaev/rest/inject/InjectApiTest.java | Switch test setup to per-tenant builtin registration (multi-factory). |
| openaev-api/src/test/java/io/openaev/rest/exercise/imports/ExerciseApiImportWithoutExistingItemsTest.java | Switch test setup to per-tenant builtin registration. |
| openaev-api/src/test/java/io/openaev/rest/exercise/imports/ExerciseApiImportWithExistingItemsTest.java | Switch test setup to per-tenant builtin registration (multi-factory). |
| openaev-api/src/test/java/io/openaev/rest/exercise/ExerciseApiExportTest.java | Switch test setup to per-tenant builtin registration (multi-factory). |
| openaev-api/src/test/java/io/openaev/rest/InjectorApiTest.java | Update assertions to use tenant-scoped injector lookup. |
| openaev-api/src/test/java/io/openaev/rest/ExpectationApiTest.java | Use entity deletes for collectors; switch setup to per-tenant builtin registration. |
| openaev-api/src/test/java/io/openaev/rest/ExerciseLessonsApiTest.java | Switch test setup to per-tenant builtin registration. |
| openaev-api/src/test/java/io/openaev/rest/CveApiTest.java | Update assertions to use tenant-scoped collector lookup. |
| openaev-api/src/test/java/io/openaev/rest/ConnectorInstanceApiTest.java | Update assertions to use tenant-scoped injector/executor lookup. |
| openaev-api/src/test/java/io/openaev/rest/CollectorApiTest.java | Update assertions to use tenant-scoped collector lookup. |
| openaev-api/src/test/java/io/openaev/rest/ChallengeApiTest.java | Switch test setup to per-tenant builtin registration. |
| openaev-api/src/test/java/io/openaev/injects/technical_inject/OpenAEVImplantExecutorTest.java | Replace Manager executor resolution with direct executor construction + builtin registration. |
| openaev-api/src/test/java/io/openaev/injects/email/EmailExecutorTest.java | Replace Manager executor resolution with direct executor construction + builtin registration. |
| openaev-api/src/test/java/io/openaev/injector_contract/InjectorContratApiTest.java | Switch test setup to per-tenant builtin registration (multi-factory). |
| openaev-api/src/test/java/io/openaev/importer/V1_DataImporterTest.java | Switch test setup to per-tenant builtin registration. |
| openaev-api/src/test/java/io/openaev/api/payload/PayloadApiImporterTest.java | Switch test setup to per-tenant builtin registration. |
| openaev-api/src/main/java/io/openaev/utils/InjectUtils.java | Use tenant-scoped injector lookup in inject resolution. |
| openaev-api/src/main/java/io/openaev/service/InjectorService.java | Tenant-scoped lookups; persist-based insert path for per-tenant builtin injector creation. |
| openaev-api/src/main/java/io/openaev/service/InjectSearchService.java | Include injector type in select tuples (query/projection alignment). |
| openaev-api/src/main/java/io/openaev/service/InjectExpectationTraceService.java | Use tenant-scoped collector lookup for bulk insert source resolution. |
| openaev-api/src/main/java/io/openaev/service/EndpointService.java | Use tenant-scoped executor lookup when building agent registration input. |
| openaev-api/src/main/java/io/openaev/scheduler/jobs/InjectsExecutionJob.java | Disable tenant filter for cross-tenant execution job path. |
| openaev-api/src/main/java/io/openaev/rest/injector_contract/InjectorContractService.java | Extend GROUP BY to include selected injector fields. |
| openaev-api/src/main/java/io/openaev/rest/injector/InjectorApi.java | Switch injector lookups to tenant-scoped; route optionsById via service method. |
| openaev-api/src/main/java/io/openaev/rest/inject_expectation_trace/InjectExpectationTraceApi.java | Use tenant-scoped collector lookup for trace creation and retrieval. |
| openaev-api/src/main/java/io/openaev/rest/executor/ExecutorApi.java | Use tenant-scoped executor lookup for update/register flows. |
| openaev-api/src/main/java/io/openaev/rest/document/DocumentApi.java | Use tenant-scoped injector/collector lookup for image downloads. |
| openaev-api/src/main/java/io/openaev/rest/collector/service/CollectorService.java | Tenant-scoped lookups; persist-based insert path for per-tenant collector creation. |
| openaev-api/src/main/java/io/openaev/migration/V5_02__Composite_pk_connectors.java | New migration to convert connector PKs and update dependent FKs to composite keys. |
| openaev-api/src/main/java/io/openaev/integration/impl/injectors/ovh/OvhInjectorIntegration.java | Remove builtin injector registration at integration start. |
| openaev-api/src/main/java/io/openaev/integration/impl/injectors/opencti/OpenCTIInjectorIntegration.java | Remove builtin injector registration at integration start. |
| openaev-api/src/main/java/io/openaev/integration/impl/injectors/openaev/OpenaevInjectorIntegrationFactory.java | Convert to builtin registrable factory; add per-tenant registration hook. |
| openaev-api/src/main/java/io/openaev/integration/impl/injectors/openaev/OpenaevInjectorIntegration.java | Remove builtin registration logic; leave executor wiring only. |
| openaev-api/src/main/java/io/openaev/integration/impl/injectors/openaev/OpenaevImplantCommandBuilder.java | New utility to centralize implant command generation. |
| openaev-api/src/main/java/io/openaev/integration/impl/injectors/manual/ManualInjectorIntegrationFactory.java | Convert to builtin registrable factory; add per-tenant registration hook. |
| openaev-api/src/main/java/io/openaev/integration/impl/injectors/manual/ManualInjectorIntegration.java | Remove builtin registration logic; leave executor wiring only. |
| openaev-api/src/main/java/io/openaev/integration/impl/injectors/email/EmailInjectorIntegrationFactory.java | Convert to builtin registrable factory; add per-tenant registration hook. |
| openaev-api/src/main/java/io/openaev/integration/impl/injectors/email/EmailInjectorIntegration.java | Remove builtin registration logic; leave executor wiring only. |
| openaev-api/src/main/java/io/openaev/integration/impl/injectors/channel/ChannelInjectorIntegrationFactory.java | Convert to builtin registrable factory; add per-tenant registration hook. |
| openaev-api/src/main/java/io/openaev/integration/impl/injectors/channel/ChannelInjectorIntegration.java | Remove builtin registration logic; leave executor wiring only. |
| openaev-api/src/main/java/io/openaev/integration/impl/injectors/challenge/ChallengeInjectorIntegrationFactory.java | Convert to builtin registrable factory; add per-tenant registration hook. |
| openaev-api/src/main/java/io/openaev/integration/impl/injectors/challenge/ChallengeInjectorIntegration.java | Remove builtin registration logic; leave executor wiring only. |
| openaev-api/src/main/java/io/openaev/integration/impl/executors/openaev/OpenAEVExecutorIntegrationFactory.java | Convert to builtin registrable factory; add per-tenant registration hook. |
| openaev-api/src/main/java/io/openaev/integration/impl/executors/openaev/OpenAEVExecutorIntegration.java | Remove executor registration logic; keep runtime integration logic only. |
| openaev-api/src/main/java/io/openaev/integration/ManagerFactory.java | Register built-ins for all tenants + implement DependenciesManager for tenant provisioning. |
| openaev-api/src/main/java/io/openaev/integration/BuiltinTenantRegistrable.java | New marker interface for tenant-scoped builtin registration. |
| openaev-api/src/main/java/io/openaev/integration/BuiltinIntegrationFactory.java | New base class to unify builtin factory registration for tenants. |
| openaev-api/src/main/java/io/openaev/helper/InjectHelper.java | Disable tenant filter for cross-tenant inject execution queries. |
| openaev-api/src/main/java/io/openaev/executors/ExecutorService.java | Tenant-scoped lookups; persist-based insert path for per-tenant executor creation. |
| openaev-api/src/main/java/io/openaev/collectors/expectations_vulnerability_manager/ExpectationsVulnerabilityManagerCollector.java | Implement per-tenant builtin registration for this collector. |
| openaev-api/src/main/java/io/openaev/collectors/expectations_expiration_manager/ExpectationsExpirationManagerJob.java | Add per-tenant builtin registration hook for collector creation. |
Comments suppressed due to low confidence (4)
openaev-model/src/main/java/io/openaev/database/model/Inject.java:192
- issue (blocking):
Inject.injectorjoins only oninject_injector→injectors.injector_id, but the DB FK is now composite(inject_injector, tenant_id)and this codebase disables the tenant filter for cross-tenant execution. Without joining ontenant_id(e.g., via@JoinColumnsOrFormulaslikeinjectorContractabove), cross-tenant queries can associate an inject with an injector from another tenant (or produce duplicates).
openaev-model/src/main/java/io/openaev/database/model/Injector.java:96 - issue (blocking): The join table
injectors_injector_contractsnow has a composite FK toinjectors(injector_id, tenant_id)(seeV5_02__Composite_pk_connectors), but this@ManyToManymapping only declaresinjector_idon the owning side. This can lead to ambiguous joins once the sameinjector_idexists in multiple tenants (especially when the tenant filter is disabled). Suggestion: includetenant_idin the join columns (and handle the sharedtenant_idcolumn carefully, e.g., withinsertable=false/updatable=falseon one side if needed).
openaev-model/src/main/java/io/openaev/database/model/Agent.java:91 - issue (blocking):
Agent.executoris mapped with a single@JoinColumn(agent_executor)but the DB FK is now composite(agent_executor, tenant_id)(see migration). If multiple tenants can have the same executor_id, this association is ambiguous when the tenant filter is disabled and doesn’t reflect the actual FK. Suggestion: map the association with both columns (e.g.,@JoinColumnsOrFormulasaddingtenant_id), similar toInject.injectorContract.
openaev-api/src/main/java/io/openaev/integration/impl/injectors/ovh/OvhInjectorIntegration.java:72 - issue (blocking):
innerStart()no longer registers the built-in OVH SMS injector/contract in the DB. SinceOvhInjectorIntegrationFactorydoes not implementBuiltinTenantRegistrable/BuiltinIntegrationFactory, there’s currently no code path left that guarantees this injector exists for a tenant before the executor runs. Suggestion: move the registration logic into aregisterConnectorForTenant()implementation (factory-side) or restore a tenant-scoped registration step here.
@Override
protected void innerStart() throws Exception {
OvhSmsService ovhSmsService = new OvhSmsService(this.config);
this.ovhSmsExecutor =
new OvhSmsExecutor(injectorContext, ovhSmsService, injectExpectationService);
}
Comment on lines
+12
to
+15
| * <p>The DB primary key is composite {@code (id, tenant_id)} for multi-tenant isolation, but JPA | ||
| * maps only {@code id} as {@code @Id}. The Hibernate tenant filter scopes all queries to the | ||
| * current tenant, and services use {@code findByIdAndTenantId()} for explicit lookups. | ||
| * |
Comment on lines
59
to
+86
| @Transactional | ||
| @Lock(type = MANAGER_FACTORY, key = "manager-factory") | ||
| public Manager getManager() { | ||
| if (manager == null) { | ||
| try { | ||
| registerBuiltinsForAllTenants(); | ||
| this.manager = new Manager(factories); | ||
| this.manager.monitorIntegrations(); | ||
| } catch (Exception e) { | ||
| throw new RuntimeException("Failed to initialize Manager", e); | ||
| } | ||
| } | ||
| return this.manager; | ||
| } | ||
|
|
||
| /** | ||
| * Ensures built-in connectors are registered for every existing tenant. This covers tenants | ||
| * created before the builtin registration mechanism was introduced (e.g. the default tenant | ||
| * created by Flyway migration) and is idempotent — safe to run on every startup. | ||
| */ | ||
| private void registerBuiltinsForAllTenants() { | ||
| List<Tenant> tenants = fromIterable(tenantRepository.findAll()); | ||
| TenantAssertionControl.suppress(); | ||
| try { | ||
| for (Tenant tenant : tenants) { | ||
| try { | ||
| createDependencyForTenant(tenant); | ||
| } catch (DependenciesManagerException e) { |
Comment on lines
+105
to
+130
| String previousTenant = TenantContext.getCurrentTenant(); | ||
| try { | ||
| TenantContext.setCurrentTenant(tenant.getId()); | ||
| // Re-enable the Hibernate filter with the correct tenant — the AOP aspect already activated | ||
| // it with the previous tenant value before this method body runs. | ||
| entityManager | ||
| .unwrap(Session.class) | ||
| .enableFilter("tenantFilter") | ||
| .setParameter("tenantId", tenant.getId()); | ||
| for (BuiltinTenantRegistrable registrable : builtinRegistrables) { | ||
| try { | ||
| registrable.registerForTenant(); | ||
| } catch (Exception e) { | ||
| throw new DependenciesManagerException( | ||
| "Failed to register built-in connector %s for tenant %s" | ||
| .formatted(registrable.getClass().getSimpleName(), tenant.getName()), | ||
| e); | ||
| } | ||
| } | ||
| log.info( | ||
| "Successfully registered {} built-in connector(s) for tenant '{}'", | ||
| builtinRegistrables.size(), | ||
| tenant.getName()); | ||
| } finally { | ||
| TenantContext.setCurrentTenant(previousTenant); | ||
| } |
| null, | ||
| null, | ||
| false, | ||
| List.of(ExternalServiceDependency.SMTP, ExternalServiceDependency.SMTP)); |
Comment on lines
22
to
40
| @@ -34,6 +40,22 @@ public ExpectationsExpirationManagerJob( | |||
| } | |||
Comment on lines
+15
to
18
| import io.openaev.context.TenantContext; | ||
| import io.openaev.database.model.*; | ||
| import io.openaev.database.model.Inject; | ||
| import io.openaev.database.repository.InjectorRepository; |
Comment on lines
69
to
73
| @Override | ||
| protected void innerStart() throws Exception { | ||
| String injectorId = | ||
| connectorInstanceService.getConnectorInstanceConfigurationsByIdAndKey( | ||
| connectorInstance.getId(), ConnectorType.INJECTOR.getIdKeyName()); | ||
|
|
||
| injectorService.registerBuiltinInjector( | ||
| injectorId, | ||
| OPENCTI_INJECTOR_NAME, | ||
| openCTIContract, | ||
| true, | ||
| "incident-response", | ||
| null, | ||
| null, | ||
| false, | ||
| new ArrayList<>()); | ||
| this.openCTIExecutor = | ||
| new OpenCTIExecutor(injectorContext, openCTIService, injectExpectationService); | ||
| } |
Comment on lines
+29
to
31
| import jakarta.persistence.EntityManager; | ||
| import jakarta.transaction.Transactional; | ||
| import jakarta.validation.constraints.NotBlank; |
Dimfacion
force-pushed
the
issue/3505_add_builtin_connectors_2
branch
from
87a0e55 to
f42cce7
Compare
Dimfacion
force-pushed
the
issue/3505_add_builtin_connectors_2
branch
from
4ac58e4 to
823fbc8
Compare
Dimfacion
changed the base branch from
release/current
to
issue/4864-url-fallback
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
Testing Instructions
Related issues
Checklist
Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...