From 0fbbb6a984a84463e2912178e815a6b07528c9df Mon Sep 17 00:00:00 2001 From: Brant DeBow <125889545+brant-livefront@users.noreply.github.com> Date: Mon, 8 Sep 2025 10:54:43 -0400 Subject: [PATCH] Event integration updates and cleanups (#6288) * Event integration updates and cleanups * Fix empty message on ArgumentException * Adjust exception message Co-authored-by: Matt Bishop --------- Co-authored-by: Matt Bishop --- .../Services/EventLoggingListenerService.cs | 4 +- .../Services/IEventMessageHandler.cs | 5 +- .../Services/IIntegrationHandler.cs | 55 +++++++++++++++++-- .../EventIntegrations/README.md | 12 +++- .../WebhookIntegrationHandler.cs | 46 ++-------------- .../Models/OrganizationIntegration.cs | 7 +-- .../OrganizationIntegrationConfiguration.cs | 7 +-- .../WebhookIntegrationHandlerTests.cs | 4 ++ 8 files changed, 76 insertions(+), 64 deletions(-) diff --git a/src/Core/AdminConsole/Services/EventLoggingListenerService.cs b/src/Core/AdminConsole/Services/EventLoggingListenerService.cs index 53ff3d4d0a..84a862ce94 100644 --- a/src/Core/AdminConsole/Services/EventLoggingListenerService.cs +++ b/src/Core/AdminConsole/Services/EventLoggingListenerService.cs @@ -28,12 +28,12 @@ public abstract class EventLoggingListenerService : BackgroundService if (root.ValueKind == JsonValueKind.Array) { var eventMessages = root.Deserialize>(); - await _handler.HandleManyEventsAsync(eventMessages); + await _handler.HandleManyEventsAsync(eventMessages ?? throw new JsonException("Deserialize returned null")); } else if (root.ValueKind == JsonValueKind.Object) { var eventMessage = root.Deserialize(); - await _handler.HandleEventAsync(eventMessage); + await _handler.HandleEventAsync(eventMessage ?? throw new JsonException("Deserialize returned null")); } else { diff --git a/src/Core/AdminConsole/Services/IEventMessageHandler.cs b/src/Core/AdminConsole/Services/IEventMessageHandler.cs index fcffb56c65..83c5e33ecb 100644 --- a/src/Core/AdminConsole/Services/IEventMessageHandler.cs +++ b/src/Core/AdminConsole/Services/IEventMessageHandler.cs @@ -1,7 +1,4 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using Bit.Core.Models.Data; +using Bit.Core.Models.Data; namespace Bit.Core.Services; diff --git a/src/Core/AdminConsole/Services/IIntegrationHandler.cs b/src/Core/AdminConsole/Services/IIntegrationHandler.cs index 9a3edac9ec..bb10dc01b9 100644 --- a/src/Core/AdminConsole/Services/IIntegrationHandler.cs +++ b/src/Core/AdminConsole/Services/IIntegrationHandler.cs @@ -1,6 +1,5 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - +using System.Globalization; +using System.Net; using Bit.Core.AdminConsole.Models.Data.EventIntegrations; namespace Bit.Core.Services; @@ -20,8 +19,56 @@ public abstract class IntegrationHandlerBase : IIntegrationHandler public async Task HandleAsync(string json) { var message = IntegrationMessage.FromJson(json); - return await HandleAsync(message); + return await HandleAsync(message ?? throw new ArgumentException("IntegrationMessage was null when created from the provided JSON")); } public abstract Task HandleAsync(IntegrationMessage message); + + protected IntegrationHandlerResult ResultFromHttpResponse( + HttpResponseMessage response, + IntegrationMessage message, + TimeProvider timeProvider) + { + var result = new IntegrationHandlerResult(success: response.IsSuccessStatusCode, message); + + if (response.IsSuccessStatusCode) return result; + + switch (response.StatusCode) + { + case HttpStatusCode.TooManyRequests: + case HttpStatusCode.RequestTimeout: + case HttpStatusCode.InternalServerError: + case HttpStatusCode.BadGateway: + case HttpStatusCode.ServiceUnavailable: + case HttpStatusCode.GatewayTimeout: + result.Retryable = true; + result.FailureReason = response.ReasonPhrase ?? $"Failure with status code: {(int)response.StatusCode}"; + + if (response.Headers.TryGetValues("Retry-After", out var values)) + { + var value = values.FirstOrDefault(); + if (int.TryParse(value, out var seconds)) + { + // Retry-after was specified in seconds. Adjust DelayUntilDate by the requested number of seconds. + result.DelayUntilDate = timeProvider.GetUtcNow().AddSeconds(seconds).UtcDateTime; + } + else if (DateTimeOffset.TryParseExact(value, + "r", // "r" is the round-trip format: RFC1123 + CultureInfo.InvariantCulture, + DateTimeStyles.AssumeUniversal | DateTimeStyles.AdjustToUniversal, + out var retryDate)) + { + // Retry-after was specified as a date. Adjust DelayUntilDate to the specified date. + result.DelayUntilDate = retryDate.UtcDateTime; + } + } + break; + default: + result.Retryable = false; + result.FailureReason = response.ReasonPhrase ?? $"Failure with status code {(int)response.StatusCode}"; + break; + } + + return result; + } } diff --git a/src/Core/AdminConsole/Services/Implementations/EventIntegrations/README.md b/src/Core/AdminConsole/Services/Implementations/EventIntegrations/README.md index 83b59cdec1..4092cc20ad 100644 --- a/src/Core/AdminConsole/Services/Implementations/EventIntegrations/README.md +++ b/src/Core/AdminConsole/Services/Implementations/EventIntegrations/README.md @@ -418,13 +418,21 @@ dependencies and integrations. For instance, `SlackIntegrationHandler` needs a ` `AddEventIntegrationServices` has a call to `AddSlackService`. Same thing for webhooks when it comes to defining a custom HttpClient by name. -1. In `AddEventIntegrationServices` create the listener configuration: +In `AddEventIntegrationServices`: + +1. Create the singleton for the handler: + +``` csharp + services.TryAddSingleton, ExampleIntegrationHandler>(); +``` + +2. Create the listener configuration: ``` csharp var exampleConfiguration = new ExampleListenerConfiguration(globalSettings); ``` -2. Add the integration to both the RabbitMQ and ASB specific declarations: +3. Add the integration to both the RabbitMQ and ASB specific declarations: ``` csharp services.AddRabbitMqIntegration(exampleConfiguration); diff --git a/src/Core/AdminConsole/Services/Implementations/EventIntegrations/WebhookIntegrationHandler.cs b/src/Core/AdminConsole/Services/Implementations/EventIntegrations/WebhookIntegrationHandler.cs index 99cad65efa..e0c2b66a90 100644 --- a/src/Core/AdminConsole/Services/Implementations/EventIntegrations/WebhookIntegrationHandler.cs +++ b/src/Core/AdminConsole/Services/Implementations/EventIntegrations/WebhookIntegrationHandler.cs @@ -1,7 +1,5 @@ #nullable enable -using System.Globalization; -using System.Net; using System.Net.Http.Headers; using System.Text; using Bit.Core.AdminConsole.Models.Data.EventIntegrations; @@ -17,7 +15,8 @@ public class WebhookIntegrationHandler( public const string HttpClientName = "WebhookIntegrationHandlerHttpClient"; - public override async Task HandleAsync(IntegrationMessage message) + public override async Task HandleAsync( + IntegrationMessage message) { var request = new HttpRequestMessage(HttpMethod.Post, message.Configuration.Uri); request.Content = new StringContent(message.RenderedTemplate, Encoding.UTF8, "application/json"); @@ -28,45 +27,8 @@ public class WebhookIntegrationHandler( parameter: message.Configuration.Token ); } + var response = await _httpClient.SendAsync(request); - var result = new IntegrationHandlerResult(success: response.IsSuccessStatusCode, message); - - switch (response.StatusCode) - { - case HttpStatusCode.TooManyRequests: - case HttpStatusCode.RequestTimeout: - case HttpStatusCode.InternalServerError: - case HttpStatusCode.BadGateway: - case HttpStatusCode.ServiceUnavailable: - case HttpStatusCode.GatewayTimeout: - result.Retryable = true; - result.FailureReason = response.ReasonPhrase ?? $"Failure with status code: {(int)response.StatusCode}"; - - if (response.Headers.TryGetValues("Retry-After", out var values)) - { - var value = values.FirstOrDefault(); - if (int.TryParse(value, out var seconds)) - { - // Retry-after was specified in seconds. Adjust DelayUntilDate by the requested number of seconds. - result.DelayUntilDate = timeProvider.GetUtcNow().AddSeconds(seconds).UtcDateTime; - } - else if (DateTimeOffset.TryParseExact(value, - "r", // "r" is the round-trip format: RFC1123 - CultureInfo.InvariantCulture, - DateTimeStyles.AssumeUniversal | DateTimeStyles.AdjustToUniversal, - out var retryDate)) - { - // Retry-after was specified as a date. Adjust DelayUntilDate to the specified date. - result.DelayUntilDate = retryDate.UtcDateTime; - } - } - break; - default: - result.Retryable = false; - result.FailureReason = response.ReasonPhrase ?? $"Failure with status code {(int)response.StatusCode}"; - break; - } - - return result; + return ResultFromHttpResponse(response, message, timeProvider); } } diff --git a/src/Infrastructure.EntityFramework/AdminConsole/Models/OrganizationIntegration.cs b/src/Infrastructure.EntityFramework/AdminConsole/Models/OrganizationIntegration.cs index 5e5f7d4802..0f47d5947b 100644 --- a/src/Infrastructure.EntityFramework/AdminConsole/Models/OrganizationIntegration.cs +++ b/src/Infrastructure.EntityFramework/AdminConsole/Models/OrganizationIntegration.cs @@ -1,13 +1,10 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using AutoMapper; +using AutoMapper; namespace Bit.Infrastructure.EntityFramework.AdminConsole.Models; public class OrganizationIntegration : Core.AdminConsole.Entities.OrganizationIntegration { - public virtual Organization Organization { get; set; } + public virtual required Organization Organization { get; set; } } public class OrganizationIntegrationMapperProfile : Profile diff --git a/src/Infrastructure.EntityFramework/AdminConsole/Models/OrganizationIntegrationConfiguration.cs b/src/Infrastructure.EntityFramework/AdminConsole/Models/OrganizationIntegrationConfiguration.cs index 52b8783fcf..21b282f767 100644 --- a/src/Infrastructure.EntityFramework/AdminConsole/Models/OrganizationIntegrationConfiguration.cs +++ b/src/Infrastructure.EntityFramework/AdminConsole/Models/OrganizationIntegrationConfiguration.cs @@ -1,13 +1,10 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using AutoMapper; +using AutoMapper; namespace Bit.Infrastructure.EntityFramework.AdminConsole.Models; public class OrganizationIntegrationConfiguration : Core.AdminConsole.Entities.OrganizationIntegrationConfiguration { - public virtual OrganizationIntegration OrganizationIntegration { get; set; } + public virtual required OrganizationIntegration OrganizationIntegration { get; set; } } public class OrganizationIntegrationConfigurationMapperProfile : Profile diff --git a/test/Core.Test/AdminConsole/Services/WebhookIntegrationHandlerTests.cs b/test/Core.Test/AdminConsole/Services/WebhookIntegrationHandlerTests.cs index bf4283243c..53a3598d47 100644 --- a/test/Core.Test/AdminConsole/Services/WebhookIntegrationHandlerTests.cs +++ b/test/Core.Test/AdminConsole/Services/WebhookIntegrationHandlerTests.cs @@ -51,6 +51,7 @@ public class WebhookIntegrationHandlerTests Assert.True(result.Success); Assert.Equal(result.Message, message); + Assert.Empty(result.FailureReason); sutProvider.GetDependency().Received(1).CreateClient( Arg.Is(AssertHelper.AssertPropertyEqual(WebhookIntegrationHandler.HttpClientName)) @@ -59,6 +60,7 @@ public class WebhookIntegrationHandlerTests Assert.Single(_handler.CapturedRequests); var request = _handler.CapturedRequests[0]; Assert.NotNull(request); + Assert.NotNull(request.Content); var returned = await request.Content.ReadAsStringAsync(); Assert.Equal(HttpMethod.Post, request.Method); @@ -77,6 +79,7 @@ public class WebhookIntegrationHandlerTests Assert.True(result.Success); Assert.Equal(result.Message, message); + Assert.Empty(result.FailureReason); sutProvider.GetDependency().Received(1).CreateClient( Arg.Is(AssertHelper.AssertPropertyEqual(WebhookIntegrationHandler.HttpClientName)) @@ -85,6 +88,7 @@ public class WebhookIntegrationHandlerTests Assert.Single(_handler.CapturedRequests); var request = _handler.CapturedRequests[0]; Assert.NotNull(request); + Assert.NotNull(request.Content); var returned = await request.Content.ReadAsStringAsync(); Assert.Equal(HttpMethod.Post, request.Method);