Add CQRS and caching support for OrganizationIntegrationConfigurations

[brant-livefront]

📔 Objective

This PR extends what was built in #6650 and #6689 to extend the CQRS pattern and the support for clearing the cache to OrganizationIntegrationConfigurations.

Changes:

• Remove repository from OrganizationIntegrationConfigurationController
• Replace with specific Commands / Queries
• Add Commands / Queries to handle OrganizationIntegrationConfiguration
• Add cache-clearing to the Create, Update and Delete Commands
• Adds a new OrganizationIntegrationConfigurationValidator to replace the validation done in OrganizationIntegrationConfigurationRequestModel. Use the validator in the commands to make sure we're only storing valid configurations, templates, and filters.
• Add DI and tests for all these pieces

This PR (along with #6689 ) is a replacement for #6675 to break up the changes into smaller, more easily reviewed pieces.

⏰ Reminders before review

• Contributor guidelines followed
• All formatters and local linters executed and passed
• Written new unit and / or integration tests where applicable
• Protected functional changes with optionality (feature flags)
• Used internationalization (i18n) for all UI strings
• CI builds passed
• Communicated to DevOps any deployment requirements
• Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

• 👍 (:+1:) or similar for great changes
• 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
• ❓ (:question:) for questions
• 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
• 🎨 (:art:) for suggestions / improvements
• ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
• 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
• ⛏ (:pick:) for minor or nitpick changes

[claude]

Claude finished @brant-livefront's task —— View job

---

Code Review Complete

Tasks

• Review PR changes for CQRS and caching implementation
• Analyze architectural patterns and consistency
• Check for security concerns and best practices
• Verify test coverage and quality
• Provide feedback and recommendations

---

Overall Assessment

This is a well-structured PR that follows established CQRS patterns from #6689 with excellent test coverage. However, I've identified 2 critical issues and 2 important issues that should be addressed before merging.

Critical Issues

❌ Missing OrganizationIntegrationId Assignment in Update Command

File: src/Core/AdminConsole/EventIntegrations/OrganizationIntegrationConfigurations/UpdateOrganizationIntegrationConfigurationCommand.cs:43

The UpdateAsync method preserves Id and CreationDate from the existing configuration but does NOT preserve OrganizationIntegrationId. This could break foreign key constraints or associate the configuration with the wrong integration.

Fix needed:

updatedConfiguration.Id = configuration.Id;
updatedConfiguration.CreationDate = configuration.CreationDate;
updatedConfiguration.OrganizationIntegrationId = configuration.OrganizationIntegrationId; // ADD THIS LINE
await configurationRepository.ReplaceAsync(updatedConfiguration);

❌ Generic Exception Swallowing in Validator

File: src/Core/AdminConsole/Services/OrganizationIntegrationConfigurationValidator.cs:54,72

The validator uses generic catch blocks without examining exception types, which masks actual JSON parsing errors, type mismatches, and potentially security-relevant failures.

Fix needed:

try
{
    var config = JsonSerializer.Deserialize<T>(configuration);
    return config is not null;
}
catch (JsonException)
{
    return false;
}
catch (NotSupportedException)
{
    return false;
}
// Allow other exceptions (OOM, StackOverflow) to propagate

Important Issues

⚠️ Race Condition in Cache Invalidation

File: src/Core/AdminConsole/EventIntegrations/OrganizationIntegrationConfigurations/UpdateOrganizationIntegrationConfigurationCommand.cs:69-78

When EventType changes, cache invalidation happens AFTER database update. A concurrent read could cache the new data between the update and the second invalidation.

Fix: Invalidate both old and new event type caches BEFORE calling ReplaceAsync:

// Invalidate both caches BEFORE updating
await cache.RemoveAsync(/* old event type key */);
if (configuration.EventType != updatedConfiguration.EventType)
{
    await cache.RemoveAsync(/* new event type key */);
}
await configurationRepository.ReplaceAsync(updatedConfiguration);

⚠️ Configuration Validation May Not Match Test Expectations

File: src/Core/AdminConsole/Services/OrganizationIntegrationConfigurationValidator.cs:35

For Hec, Datadog, and Teams integration types, the validator checks configuration.Configuration is null but doesn't reject empty strings. However, test ValidateConfiguration_EmptyNonNullConfiguration_ReturnsFalse expects empty strings to return false.

Verify: Do the validator tests actually pass? If not, change line 35 to:

return string.IsNullOrWhiteSpace(configuration.Configuration);

Suggestions for Improvement

🎨 No Size Limits on Configuration/Filter JSON

File: src/Core/AdminConsole/Services/OrganizationIntegrationConfigurationValidator.cs

The validator accepts any size JSON payload. Consider adding size validation:

if (configuration.Length > 10_240) // 10KB limit
{
    return false;
}

❓ Query Does Not Implement Caching

File: src/Core/AdminConsole/EventIntegrations/OrganizationIntegrationConfigurations/GetOrganizationIntegrationConfigurationsQuery.cs

The Create/Update/Delete commands invalidate cache entries, but the Query doesn't read from or populate cache. Is this intentional? If configurations are read frequently, consider implementing cache-aside pattern in the Query.

❓ Owner-Only Authorization

File: src/Api/AdminConsole/Controllers/OrganizationIntegrationConfigurationController.cs:91

The controller only allows Organization Owners to manage configurations. Should Organization Admins also have access? Please confirm this is the intended permission level.

---

Positive Observations

👍 Excellent pattern consistency - Follows the OrganizationIntegration CQRS structure from #6689 precisely

👍 Comprehensive test coverage - Every command, query, and validator has thorough unit tests

👍 Well-reasoned cache invalidation - Distinction between wildcard and specific event types is handled correctly

👍 Good separation of concerns - Moving validation from RequestModel to dedicated Validator improves testability

👍 Clear documentation - XML comments on interfaces and cache constants are helpful

👍 Idempotent DI registration - Proper use of TryAddScoped and TryAddSingleton

---

Security Assessment

✅ Zero-knowledge principles: Configuration data stored as opaque JSON strings

✅ No sensitive data logging: Error messages don't expose internal details

✅ Authorization checks: NotFoundException prevents information leakage

⚠️ Input validation: Could be strengthened with size/depth limits

---

Recommendation

Request Changes - Please address the 2 critical issues before merging. The important issues and suggestions can be addressed now or in follow-up work depending on your team's preferences.

[github-actions]

Checkmarx One – Scan Summary & Details – 477fd3a6-5da9-46ca-a757-63084b059caa

Fixed Issues (1)

Great job! The following issues were fixed in this Pull Request

Severity	Issue	Source File / Package
	CSRF	/src/Api/AdminConsole/Public/Controllers/MembersController.cs: 207

[codecov]

Codecov Report

❌ Patch coverage is 98.30508% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 53.68%. Comparing base (3ff5902) to head (de91439).
⚠️ Report is 25 commits behind head on main.

Files with missing lines	Patch %	Lines
...s/OrganizationIntegrationConfigurationValidator.cs	91.66%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6690      +/-   ##
==========================================
+ Coverage   53.60%   53.68%   +0.08%     
==========================================
  Files        1921     1931      +10     
  Lines       85650    85808     +158     
  Branches     7687     7688       +1     
==========================================
+ Hits        45911    46066     +155     
- Misses      37967    37969       +2     
- Partials     1772     1773       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[withinfocus]

Big change but having the central commands most importantly allows for the centralization of cache management.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

The base branch was changed.

[withinfocus]

@JimmyVo16 we had a rebase here -- can you approve again?