Extend Unit Test Coverage of Event Integrations (#6517)

* Extend Unit Test Coverage of Event Integrations

* Expanded SlackService error handling and tests

* Cleaned up a few issues noted by Claude

Author: brant-livefront

src/Api/AdminConsole/Models/Response/Organizations/OrganizationIntegrationConfigurationResponseModel.cs

@@ -2,17 +2,13 @@
 using Bit.Core.Enums;
 using Bit.Core.Models.Api;
 
-#nullable enable
-
 namespace Bit.Api.AdminConsole.Models.Response.Organizations;
 
 public class OrganizationIntegrationConfigurationResponseModel : ResponseModel
 {
     public OrganizationIntegrationConfigurationResponseModel(OrganizationIntegrationConfiguration organizationIntegrationConfiguration, string obj = "organizationIntegrationConfiguration")
         : base(obj)
     {
-        ArgumentNullException.ThrowIfNull(organizationIntegrationConfiguration);
-
         Id = organizationIntegrationConfiguration.Id;
         Configuration = organizationIntegrationConfiguration.Configuration;
         CreationDate = organizationIntegrationConfiguration.CreationDate;

src/Core/AdminConsole/Models/Slack/SlackApiResponse.cs

@@ -33,6 +33,12 @@ public class SlackOAuthResponse : SlackApiResponse
     public SlackTeam Team { get; set; } = new();
 }
 
+public class SlackSendMessageResponse : SlackApiResponse
+{
+    [JsonPropertyName("channel")]
+    public string Channel { get; set; } = string.Empty;
+}
+
 public class SlackTeam
 {
     public string Id { get; set; } = string.Empty;

src/Core/AdminConsole/Services/ISlackService.cs

@@ -1,4 +1,6 @@
-namespace Bit.Core.Services;
+using Bit.Core.Models.Slack;
+
+namespace Bit.Core.Services;
 
 /// <summary>Defines operations for interacting with Slack, including OAuth authentication, channel discovery,
 /// and sending messages.</summary>
@@ -54,6 +56,6 @@ public interface ISlackService
     /// <param name="token">A valid Slack OAuth access token.</param>
     /// <param name="message">The message text to send.</param>
     /// <param name="channelId">The channel ID to send the message to.</param>
-    /// <returns>A task that completes when the message has been sent.</returns>
-    Task SendSlackMessageByChannelIdAsync(string token, string message, string channelId);
+    /// <returns>The response from Slack after sending the message.</returns>
+    Task<SlackSendMessageResponse?> SendSlackMessageByChannelIdAsync(string token, string message, string channelId);
 }

src/Core/AdminConsole/Services/Implementations/EventIntegrations/SlackIntegrationHandler.cs

@@ -6,14 +6,43 @@ public class SlackIntegrationHandler(
     ISlackService slackService)
     : IntegrationHandlerBase<SlackIntegrationConfigurationDetails>
 {
+    private static readonly HashSet<string> _retryableErrors = new(StringComparer.Ordinal)
+        {
+            "internal_error",
+            "message_limit_exceeded",
+            "rate_limited",
+            "ratelimited",
+            "service_unavailable"
+        };
+
     public override async Task<IntegrationHandlerResult> HandleAsync(IntegrationMessage<SlackIntegrationConfigurationDetails> message)
     {
-        await slackService.SendSlackMessageByChannelIdAsync(
+        var slackResponse = await slackService.SendSlackMessageByChannelIdAsync(
             message.Configuration.Token,
             message.RenderedTemplate,
             message.Configuration.ChannelId
         );
 
-        return new IntegrationHandlerResult(success: true, message: message);
+        if (slackResponse is null)
+        {
+            return new IntegrationHandlerResult(success: false, message: message)
+            {
+                FailureReason = "Slack response was null"
+            };
+        }
+
+        if (slackResponse.Ok)
+        {
+            return new IntegrationHandlerResult(success: true, message: message);
+        }
+
+        var result = new IntegrationHandlerResult(success: false, message: message) { FailureReason = slackResponse.Error };
+
+        if (_retryableErrors.Contains(slackResponse.Error))
+        {
+            result.Retryable = true;
+        }
+
+        return result;
     }
 }

src/Core/AdminConsole/Services/Implementations/EventIntegrations/SlackService.cs

@@ -1,5 +1,6 @@
 using System.Net.Http.Headers;
 using System.Net.Http.Json;
+using System.Text.Json;
 using System.Web;
 using Bit.Core.Models.Slack;
 using Bit.Core.Settings;
@@ -71,7 +72,7 @@ public async Task<List<string>> GetChannelIdsAsync(string token, List<string> ch
     public async Task<string> GetDmChannelByEmailAsync(string token, string email)
     {
         var userId = await GetUserIdByEmailAsync(token, email);
-        return await OpenDmChannel(token, userId);
+        return await OpenDmChannelAsync(token, userId);
     }
 
     public string GetRedirectUrl(string callbackUrl, string state)
@@ -97,21 +98,21 @@ public async Task<string> ObtainTokenViaOAuth(string code, string redirectUrl)
         }
 
         var tokenResponse = await _httpClient.PostAsync($"{_slackApiBaseUrl}/oauth.v2.access",
-            new FormUrlEncodedContent(new[]
-            {
+            new FormUrlEncodedContent([
                 new KeyValuePair<string, string>("client_id", _clientId),
                 new KeyValuePair<string, string>("client_secret", _clientSecret),
                 new KeyValuePair<string, string>("code", code),
                 new KeyValuePair<string, string>("redirect_uri", redirectUrl)
-            }));
+            ]));
 
         SlackOAuthResponse? result;
         try
         {
             result = await tokenResponse.Content.ReadFromJsonAsync<SlackOAuthResponse>();
         }
-        catch
+        catch (JsonException ex)
         {
+            logger.LogError(ex, "Error parsing SlackOAuthResponse: invalid JSON");
             result = null;
         }
 
@@ -129,22 +130,42 @@ public async Task<string> ObtainTokenViaOAuth(string code, string redirectUrl)
         return result.AccessToken;
     }
 
-    public async Task SendSlackMessageByChannelIdAsync(string token, string message, string channelId)
+    public async Task<SlackSendMessageResponse?> SendSlackMessageByChannelIdAsync(string token, string message,
+        string channelId)
     {
         var payload = JsonContent.Create(new { channel = channelId, text = message });
         var request = new HttpRequestMessage(HttpMethod.Post, $"{_slackApiBaseUrl}/chat.postMessage");
         request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token);
         request.Content = payload;
 
-        await _httpClient.SendAsync(request);
+        var response = await _httpClient.SendAsync(request);
+
+        try
+        {
+            return await response.Content.ReadFromJsonAsync<SlackSendMessageResponse>();
+        }
+        catch (JsonException ex)
+        {
+            logger.LogError(ex, "Error parsing Slack message response: invalid JSON");
+            return null;
+        }
     }
 
     private async Task<string> GetUserIdByEmailAsync(string token, string email)
     {
         var request = new HttpRequestMessage(HttpMethod.Get, $"{_slackApiBaseUrl}/users.lookupByEmail?email={email}");
         request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token);
         var response = await _httpClient.SendAsync(request);
-        var result = await response.Content.ReadFromJsonAsync<SlackUserResponse>();
+        SlackUserResponse? result;
+        try
+        {
+            result = await response.Content.ReadFromJsonAsync<SlackUserResponse>();
+        }
+        catch (JsonException ex)
+        {
+            logger.LogError(ex, "Error parsing SlackUserResponse: invalid JSON");
+            result = null;
+        }
 
         if (result is null)
         {
@@ -160,7 +181,7 @@ private async Task<string> GetUserIdByEmailAsync(string token, string email)
         return result.User.Id;
     }
 
-    private async Task<string> OpenDmChannel(string token, string userId)
+    private async Task<string> OpenDmChannelAsync(string token, string userId)
     {
         if (string.IsNullOrEmpty(userId))
             return string.Empty;
@@ -170,7 +191,16 @@ private async Task<string> OpenDmChannel(string token, string userId)
         request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token);
         request.Content = payload;
         var response = await _httpClient.SendAsync(request);
-        var result = await response.Content.ReadFromJsonAsync<SlackDmResponse>();
+        SlackDmResponse? result;
+        try
+        {
+            result = await response.Content.ReadFromJsonAsync<SlackDmResponse>();
+        }
+        catch (JsonException ex)
+        {
+            logger.LogError(ex, "Error parsing SlackDmResponse: invalid JSON");
+            result = null;
+        }
 
         if (result is null)
         {

src/Core/AdminConsole/Services/NoopImplementations/NoopSlackService.cs

@@ -1,4 +1,5 @@
-using Bit.Core.Services;
+using Bit.Core.Models.Slack;
+using Bit.Core.Services;
 
 namespace Bit.Core.AdminConsole.Services.NoopImplementations;
 
@@ -24,9 +25,10 @@ public string GetRedirectUrl(string callbackUrl, string state)
         return string.Empty;
     }
 
-    public Task SendSlackMessageByChannelIdAsync(string token, string message, string channelId)
+    public Task<SlackSendMessageResponse?> SendSlackMessageByChannelIdAsync(string token, string message,
+        string channelId)
     {
-        return Task.FromResult(0);
+        return Task.FromResult<SlackSendMessageResponse?>(null);
     }
 
     public Task<string> ObtainTokenViaOAuth(string code, string redirectUrl)

test/Api.Test/AdminConsole/Controllers/OrganizationIntegrationControllerTests.cs

@@ -133,6 +133,29 @@ await sutProvider.GetDependency<IOrganizationIntegrationRepository>().Received(1
             .DeleteAsync(organizationIntegration);
     }
 
+    [Theory, BitAutoData]
+    public async Task PostDeleteAsync_AllParamsProvided_Succeeds(
+        SutProvider<OrganizationIntegrationController> sutProvider,
+        Guid organizationId,
+        OrganizationIntegration organizationIntegration)
+    {
+        organizationIntegration.OrganizationId = organizationId;
+        sutProvider.Sut.Url = Substitute.For<IUrlHelper>();
+        sutProvider.GetDependency<ICurrentContext>()
+            .OrganizationOwner(organizationId)
+            .Returns(true);
+        sutProvider.GetDependency<IOrganizationIntegrationRepository>()
+            .GetByIdAsync(Arg.Any<Guid>())
+            .Returns(organizationIntegration);
+
+        await sutProvider.Sut.PostDeleteAsync(organizationId, organizationIntegration.Id);
+
+        await sutProvider.GetDependency<IOrganizationIntegrationRepository>().Received(1)
+            .GetByIdAsync(organizationIntegration.Id);
+        await sutProvider.GetDependency<IOrganizationIntegrationRepository>().Received(1)
+            .DeleteAsync(organizationIntegration);
+    }
+
     [Theory, BitAutoData]
     public async Task DeleteAsync_IntegrationDoesNotBelongToOrganization_ThrowsNotFound(
         SutProvider<OrganizationIntegrationController> sutProvider,