From ffda25608cd681d8e3b2c84f71e4c85d08a95eac Mon Sep 17 00:00:00 2001 From: Brant DeBow Date: Mon, 24 Mar 2025 11:13:23 -0400 Subject: [PATCH] Moved Slack OAuth to take into account the Organization it's being stored for. Added methods to store the top level integration for Slack --- .../Controllers/SlackOAuthController.cs | 51 +++++--- src/Api/Startup.cs | 5 + .../Data/Integrations/SlackIntegration.cs | 3 + ...ationIntegrationConfigurationRepository.cs | 13 +- ...tionIntegrationConfigurationRepository.cs} | 25 ++-- .../AdminConsole/Services/ISlackService.cs | 2 +- .../Implementations/SlackEventHandler.cs | 5 +- .../Services/Implementations/SlackService.cs | 80 ++++++------ .../Implementations/WebhookEventHandler.cs | 7 +- src/Events/Startup.cs | 2 +- .../Controllers/SlackOAuthControllerTests.cs | 118 +++++++++++++----- .../Services/SlackEventHandlerTests.cs | 6 +- .../Services/WebhookEventHandlerTests.cs | 7 +- 13 files changed, 200 insertions(+), 124 deletions(-) create mode 100644 src/Core/AdminConsole/Models/Data/Integrations/SlackIntegration.cs rename src/Core/AdminConsole/Repositories/{OrganizationIntegrationConfigurationRepository.cs => LocalOrganizationIntegrationConfigurationRepository.cs} (65%) diff --git a/src/Api/AdminConsole/Controllers/SlackOAuthController.cs b/src/Api/AdminConsole/Controllers/SlackOAuthController.cs index dd8902b32b..15a9fe3300 100644 --- a/src/Api/AdminConsole/Controllers/SlackOAuthController.cs +++ b/src/Api/AdminConsole/Controllers/SlackOAuthController.cs @@ -1,31 +1,52 @@ -using Bit.Core.Services; +using Bit.Core.Context; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.Models.Data.Integrations; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; namespace Bit.Api.AdminConsole.Controllers; [Route("slack/oauth")] -public class SlackOAuthController(ISlackService slackService) : Controller +[Authorize("Application")] +public class SlackOAuthController( + ICurrentContext currentContext, + IOrganizationIntegrationConfigurationRepository integrationConfigurationRepository, + ISlackService slackService) : Controller { - [HttpGet("redirect")] - public IActionResult RedirectToSlack() + [HttpGet("redirect/{id}")] + public async Task RedirectToSlack(string id) { - string callbackUrl = Url.RouteUrl(nameof(OAuthCallback)); + var orgIdGuid = new Guid(id); + if (!await currentContext.OrganizationOwner(orgIdGuid)) + { + throw new NotFoundException(); + } + string callbackUrl = Url.RouteUrl(nameof(OAuthCallback), new { id = id }, currentContext.HttpContext.Request.Scheme); var redirectUrl = slackService.GetRedirectUrl(callbackUrl); if (string.IsNullOrEmpty(redirectUrl)) { - return BadRequest("Slack not currently supported."); + throw new NotFoundException(); } return Redirect(redirectUrl); } - [HttpGet("callback")] - public async Task OAuthCallback([FromQuery] string code) + [HttpGet("callback/{id}", Name = nameof(OAuthCallback))] + public async Task OAuthCallback(string id, [FromQuery] string code) { + var orgIdGuid = new Guid(id); + if (!await currentContext.OrganizationOwner(orgIdGuid)) + { + throw new NotFoundException(); + } + if (string.IsNullOrEmpty(code)) { - return BadRequest("Missing code from Slack."); + throw new BadRequestException("Missing code from Slack."); } string callbackUrl = Url.RouteUrl(nameof(OAuthCallback)); @@ -33,15 +54,13 @@ public class SlackOAuthController(ISlackService slackService) : Controller if (string.IsNullOrEmpty(token)) { - return BadRequest("Invalid response from Slack."); + throw new BadRequestException("Invalid response from Slack."); } - SaveTokenToDatabase(token); + await integrationConfigurationRepository.CreateOrganizationIntegrationAsync( + orgIdGuid, + IntegrationType.Slack, + new SlackIntegration(token)); return Ok("Slack OAuth successful. Your bot is now installed."); } - - private void SaveTokenToDatabase(string botToken) - { - Console.WriteLine($"Stored bot token for team: {botToken}"); - } } diff --git a/src/Api/Startup.cs b/src/Api/Startup.cs index e4c8ed7db7..eb386c538a 100644 --- a/src/Api/Startup.cs +++ b/src/Api/Startup.cs @@ -30,6 +30,7 @@ using Bit.Api.Auth.Models.Request.WebAuthn; using Bit.Core.AdminConsole.Services.NoopImplementations; using Bit.Core.Auth.Models.Data; using Bit.Core.Auth.Identity.TokenProviders; +using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Core.Tools.ImportFeatures; using Bit.Core.Tools.ReportFeatures; @@ -222,10 +223,14 @@ public class Startup { services.AddHttpClient(SlackService.HttpClientName); services.AddSingleton(); + services.AddSingleton(); } else { services.AddSingleton(); + services.AddSingleton(); } } diff --git a/src/Core/AdminConsole/Models/Data/Integrations/SlackIntegration.cs b/src/Core/AdminConsole/Models/Data/Integrations/SlackIntegration.cs new file mode 100644 index 0000000000..e6fc1440ea --- /dev/null +++ b/src/Core/AdminConsole/Models/Data/Integrations/SlackIntegration.cs @@ -0,0 +1,3 @@ +namespace Bit.Core.Models.Data.Integrations; + +public record SlackIntegration(string token); diff --git a/src/Core/AdminConsole/Repositories/IOrganizationIntegrationConfigurationRepository.cs b/src/Core/AdminConsole/Repositories/IOrganizationIntegrationConfigurationRepository.cs index 28a5b31391..6d780ed00f 100644 --- a/src/Core/AdminConsole/Repositories/IOrganizationIntegrationConfigurationRepository.cs +++ b/src/Core/AdminConsole/Repositories/IOrganizationIntegrationConfigurationRepository.cs @@ -1,12 +1,17 @@ using Bit.Core.Enums; using Bit.Core.Models.Data.Integrations; -#nullable enable - namespace Bit.Core.Repositories; public interface IOrganizationIntegrationConfigurationRepository { - Task>> GetConfigurationsAsync(IntegrationType integrationType, - Guid organizationId, EventType eventType); + Task>> GetConfigurationsAsync( + Guid organizationId, + IntegrationType integrationType, + EventType eventType); + + Task CreateOrganizationIntegrationAsync( + Guid organizationId, + IntegrationType integrationType, + T configuration); } diff --git a/src/Core/AdminConsole/Repositories/OrganizationIntegrationConfigurationRepository.cs b/src/Core/AdminConsole/Repositories/LocalOrganizationIntegrationConfigurationRepository.cs similarity index 65% rename from src/Core/AdminConsole/Repositories/OrganizationIntegrationConfigurationRepository.cs rename to src/Core/AdminConsole/Repositories/LocalOrganizationIntegrationConfigurationRepository.cs index 88542c6c5c..cbe96213a9 100644 --- a/src/Core/AdminConsole/Repositories/OrganizationIntegrationConfigurationRepository.cs +++ b/src/Core/AdminConsole/Repositories/LocalOrganizationIntegrationConfigurationRepository.cs @@ -1,14 +1,15 @@ -using Bit.Core.Enums; +using System.Text.Json; +using Bit.Core.Enums; using Bit.Core.Models.Data.Integrations; using Bit.Core.Settings; namespace Bit.Core.Repositories; -public class OrganizationIntegrationConfigurationRepository(GlobalSettings globalSettings) +public class LocalOrganizationIntegrationConfigurationRepository(GlobalSettings globalSettings) : IOrganizationIntegrationConfigurationRepository { - public async Task>> GetConfigurationsAsync(IntegrationType integrationType, - Guid organizationId, + public async Task>> GetConfigurationsAsync(Guid organizationId, + IntegrationType integrationType, EventType eventType) { var configurations = new List>(); @@ -39,13 +40,13 @@ public class OrganizationIntegrationConfigurationRepository(GlobalSettings globa return configurations; } - public async Task>> GetAllConfigurationsAsync(Guid organizationId) => throw new NotImplementedException(); + public async Task CreateOrganizationIntegrationAsync( + Guid organizationId, + IntegrationType integrationType, + T configuration) + { + var json = JsonSerializer.Serialize(configuration); - public async Task AddConfigurationAsync(Guid organizationId, IntegrationType integrationType, EventType eventType, - IntegrationConfiguration configuration) => - throw new NotImplementedException(); - - public async Task UpdateConfigurationAsync(IntegrationConfiguration configuration) => throw new NotImplementedException(); - - public async Task DeleteConfigurationAsync(Guid id) => throw new NotImplementedException(); + Console.WriteLine($"Organization: {organizationId}, IntegrationType: {integrationType}, Configuration: {json}"); + } } diff --git a/src/Core/AdminConsole/Services/ISlackService.cs b/src/Core/AdminConsole/Services/ISlackService.cs index 2209084bc6..6c6a846f0d 100644 --- a/src/Core/AdminConsole/Services/ISlackService.cs +++ b/src/Core/AdminConsole/Services/ISlackService.cs @@ -6,6 +6,6 @@ public interface ISlackService Task> GetChannelIdsAsync(string token, List channelNames); Task GetDmChannelByEmailAsync(string token, string email); string GetRedirectUrl(string redirectUrl); - Task SendSlackMessageByChannelIdAsync(string token, string message, string channelId); Task ObtainTokenViaOAuth(string code, string redirectUrl); + Task SendSlackMessageByChannelIdAsync(string token, string message, string channelId); } diff --git a/src/Core/AdminConsole/Services/Implementations/SlackEventHandler.cs b/src/Core/AdminConsole/Services/Implementations/SlackEventHandler.cs index e3ef43c77f..c63a12e628 100644 --- a/src/Core/AdminConsole/Services/Implementations/SlackEventHandler.cs +++ b/src/Core/AdminConsole/Services/Implementations/SlackEventHandler.cs @@ -13,10 +13,7 @@ public class SlackEventHandler( public async Task HandleEventAsync(EventMessage eventMessage) { var organizationId = eventMessage.OrganizationId ?? Guid.NewGuid(); - var configurations = await configurationRepository.GetConfigurationsAsync( - IntegrationType.Slack, - organizationId, eventMessage.Type - ); + var configurations = await configurationRepository.GetConfigurationsAsync(organizationId, IntegrationType.Slack, eventMessage.Type); foreach (var configuration in configurations) { diff --git a/src/Core/AdminConsole/Services/Implementations/SlackService.cs b/src/Core/AdminConsole/Services/Implementations/SlackService.cs index ff812d633e..46cb1e1d01 100644 --- a/src/Core/AdminConsole/Services/Implementations/SlackService.cs +++ b/src/Core/AdminConsole/Services/Implementations/SlackService.cs @@ -24,46 +24,6 @@ public class SlackService( return (await GetChannelIdsAsync(token, new List { channelName })).FirstOrDefault(); } - public string GetRedirectUrl(string redirectUrl) - { - return $"https://slack.com/oauth/v2/authorize?client_id={_clientId}&scope={_scopes}&redirect_uri={redirectUrl}"; - } - - public async Task ObtainTokenViaOAuth(string code, string redirectUrl) - { - var tokenResponse = await _httpClient.PostAsync("https://slack.com/api/oauth.v2.access", - new FormUrlEncodedContent(new[] - { - new KeyValuePair("client_id", _clientId), - new KeyValuePair("client_secret", _clientSecret), - new KeyValuePair("code", code), - new KeyValuePair("redirect_uri", redirectUrl) - })); - - SlackOAuthResponse result; - try - { - result = await tokenResponse.Content.ReadFromJsonAsync(); - } - catch - { - result = null; - } - - if (result == null) - { - logger.LogError("Error obtaining token via OAuth: Unknown error"); - return string.Empty; - } - if (!result.Ok) - { - logger.LogError("Error obtaining token via OAuth: " + result.Error); - return string.Empty; - } - - return result.AccessToken; - } - public async Task> GetChannelIdsAsync(string token, List channelNames) { var matchingChannelIds = new List(); @@ -111,6 +71,46 @@ public class SlackService( return await OpenDmChannel(token, userId); } + public string GetRedirectUrl(string redirectUrl) + { + return $"https://slack.com/oauth/v2/authorize?client_id={_clientId}&scope={_scopes}&redirect_uri={redirectUrl}"; + } + + public async Task ObtainTokenViaOAuth(string code, string redirectUrl) + { + var tokenResponse = await _httpClient.PostAsync("https://slack.com/api/oauth.v2.access", + new FormUrlEncodedContent(new[] + { + new KeyValuePair("client_id", _clientId), + new KeyValuePair("client_secret", _clientSecret), + new KeyValuePair("code", code), + new KeyValuePair("redirect_uri", redirectUrl) + })); + + SlackOAuthResponse result; + try + { + result = await tokenResponse.Content.ReadFromJsonAsync(); + } + catch + { + result = null; + } + + if (result == null) + { + logger.LogError("Error obtaining token via OAuth: Unknown error"); + return string.Empty; + } + if (!result.Ok) + { + logger.LogError("Error obtaining token via OAuth: " + result.Error); + return string.Empty; + } + + return result.AccessToken; + } + public async Task SendSlackMessageByChannelIdAsync(string token, string message, string channelId) { var payload = JsonContent.Create(new { channel = channelId, text = message }); diff --git a/src/Core/AdminConsole/Services/Implementations/WebhookEventHandler.cs b/src/Core/AdminConsole/Services/Implementations/WebhookEventHandler.cs index 8edc11ba4f..933d9c2eae 100644 --- a/src/Core/AdminConsole/Services/Implementations/WebhookEventHandler.cs +++ b/src/Core/AdminConsole/Services/Implementations/WebhookEventHandler.cs @@ -21,11 +21,8 @@ public class WebhookEventHandler( { Guid organizationId = eventMessage.OrganizationId ?? Guid.NewGuid(); - var configurations = await configurationRepository.GetConfigurationsAsync( - IntegrationType.Webhook, - organizationId, - eventMessage.Type - ); + var configurations = await configurationRepository.GetConfigurationsAsync(organizationId, + IntegrationType.Webhook, eventMessage.Type); foreach (var configuration in configurations) { diff --git a/src/Events/Startup.cs b/src/Events/Startup.cs index 9e505b1de4..5d9f2ee86e 100644 --- a/src/Events/Startup.cs +++ b/src/Events/Startup.cs @@ -119,7 +119,7 @@ public class Startup globalSettings, globalSettings.EventLogging.RabbitMq.EventRepositoryQueueName)); - services.AddSingleton(); + services.AddSingleton(); if (CoreHelpers.SettingHasValue(globalSettings.Slack.ClientId) && CoreHelpers.SettingHasValue(globalSettings.Slack.ClientSecret) && diff --git a/test/Api.Test/AdminConsole/Controllers/SlackOAuthControllerTests.cs b/test/Api.Test/AdminConsole/Controllers/SlackOAuthControllerTests.cs index 6d9e4c166c..dfebd8396e 100644 --- a/test/Api.Test/AdminConsole/Controllers/SlackOAuthControllerTests.cs +++ b/test/Api.Test/AdminConsole/Controllers/SlackOAuthControllerTests.cs @@ -1,4 +1,9 @@ using Bit.Api.AdminConsole.Controllers; +using Bit.Core.Context; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.Models.Data.Integrations; +using Bit.Core.Repositories; using Bit.Core.Services; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; @@ -13,63 +18,112 @@ namespace Bit.Api.Test.AdminConsole.Controllers; public class SlackOAuthControllerTests { [Theory, BitAutoData] - public async Task OAuthCallback_ThrowsBadResultWhenCodeIsEmpty(SutProvider sutProvider) - { - sutProvider.Sut.Url = Substitute.For(); - - var requestAction = await sutProvider.Sut.OAuthCallback(string.Empty); - - Assert.IsType(requestAction); - } - - [Theory, BitAutoData] - public async Task OAuthCallback_ThrowsBadResultWhenSlackServiceReturnsEmpty(SutProvider sutProvider) + public async Task OAuthCallback_CompletesSuccessfully(SutProvider sutProvider, Guid organizationId) { + var token = "xoxb-test-token"; sutProvider.Sut.Url = Substitute.For(); + sutProvider.GetDependency() + .OrganizationOwner(organizationId) + .Returns(true); sutProvider.GetDependency() .ObtainTokenViaOAuth(Arg.Any(), Arg.Any()) - .Returns(string.Empty); + .Returns(token); - var requestAction = await sutProvider.Sut.OAuthCallback("A_test_code"); - - Assert.IsType(requestAction); - } - - [Theory, BitAutoData] - public async Task OAuthCallback_CompletesSuccessfully(SutProvider sutProvider) - { - sutProvider.Sut.Url = Substitute.For(); - sutProvider.GetDependency() - .ObtainTokenViaOAuth(Arg.Any(), Arg.Any()) - .Returns("xoxb-test-token"); - - var requestAction = await sutProvider.Sut.OAuthCallback("A_test_code"); + var requestAction = await sutProvider.Sut.OAuthCallback(organizationId.ToString(), "A_test_code"); + await sutProvider.GetDependency().Received(1) + .CreateOrganizationIntegrationAsync(organizationId, IntegrationType.Slack, new SlackIntegration(token)); Assert.IsType(requestAction); } [Theory, BitAutoData] - public void Redirect_ShouldRedirectToSlack(SutProvider sutProvider) + public async Task OAuthCallback_ThrowsBadResultWhenCodeIsEmpty(SutProvider sutProvider, Guid organizationId) { - var expectedUrl = "https://localhost/"; + sutProvider.Sut.Url = Substitute.For(); + sutProvider.GetDependency() + .OrganizationOwner(organizationId) + .Returns(true); + + await Assert.ThrowsAsync(async () => await sutProvider.Sut.OAuthCallback(organizationId.ToString(), string.Empty)); + } + + [Theory, BitAutoData] + public async Task OAuthCallback_ThrowsBadResultWhenSlackServiceReturnsEmpty(SutProvider sutProvider, Guid organizationId) + { + sutProvider.Sut.Url = Substitute.For(); + sutProvider.GetDependency() + .OrganizationOwner(organizationId) + .Returns(true); + sutProvider.GetDependency() + .ObtainTokenViaOAuth(Arg.Any(), Arg.Any()) + .Returns(string.Empty); + + await Assert.ThrowsAsync(async () => await sutProvider.Sut.OAuthCallback(organizationId.ToString(), "A_test_code")); + } + + [Theory, BitAutoData] + public async Task OAuthCallback_ThrowsNotFoundIfUserIsNotOrganizationAdmin(SutProvider sutProvider, Guid organizationId) + { + var token = "xoxb-test-token"; + sutProvider.Sut.Url = Substitute.For(); + sutProvider.GetDependency() + .OrganizationOwner(organizationId) + .Returns(false); + sutProvider.GetDependency() + .ObtainTokenViaOAuth(Arg.Any(), Arg.Any()) + .Returns(token); + + await Assert.ThrowsAsync(async () => await sutProvider.Sut.OAuthCallback(organizationId.ToString(), "A_test_code")); + } + + [Theory, BitAutoData] + public async Task Redirect_ShouldRedirectToSlack(SutProvider sutProvider, Guid organizationId) + { + var expectedUrl = $"https://localhost/{organizationId.ToString()}"; sutProvider.Sut.Url = Substitute.For(); sutProvider.GetDependency().GetRedirectUrl(Arg.Any()).Returns(expectedUrl); + sutProvider.GetDependency() + .OrganizationOwner(organizationId) + .Returns(true); + sutProvider.GetDependency() + .HttpContext.Request.Scheme + .Returns("https"); - var requestAction = sutProvider.Sut.RedirectToSlack(); + var requestAction = await sutProvider.Sut.RedirectToSlack(organizationId.ToString()); var redirectResult = Assert.IsType(requestAction); Assert.Equal(expectedUrl, redirectResult.Url); } [Theory, BitAutoData] - public void Redirect_ThrowsBadResultWhenSlackServiceReturnsEmpty(SutProvider sutProvider) + public async Task Redirect_ThrowsNotFoundWhenSlackServiceReturnsEmpty(SutProvider sutProvider, Guid organizationId) { sutProvider.Sut.Url = Substitute.For(); sutProvider.GetDependency().GetRedirectUrl(Arg.Any()).Returns(string.Empty); + sutProvider.GetDependency() + .OrganizationOwner(organizationId) + .Returns(true); + sutProvider.GetDependency() + .HttpContext.Request.Scheme + .Returns("https"); - var requestAction = sutProvider.Sut.RedirectToSlack(); + await Assert.ThrowsAsync(async () => await sutProvider.Sut.RedirectToSlack(organizationId.ToString())); + } - Assert.IsType(requestAction); + [Theory, BitAutoData] + public async Task Redirect_ThrowsNotFoundWhenUserIsNotOrganizationAdmin(SutProvider sutProvider, + Guid organizationId) + { + sutProvider.Sut.Url = Substitute.For(); + sutProvider.GetDependency().GetRedirectUrl(Arg.Any()).Returns(string.Empty); + sutProvider.GetDependency() + .OrganizationOwner(organizationId) + .Returns(false); + sutProvider.GetDependency() + .HttpContext.Request.Scheme + .Returns("https"); + + await Assert.ThrowsAsync(async () => await sutProvider.Sut.RedirectToSlack(organizationId.ToString())); } } diff --git a/test/Core.Test/AdminConsole/Services/SlackEventHandlerTests.cs b/test/Core.Test/AdminConsole/Services/SlackEventHandlerTests.cs index 3048088215..24977f240b 100644 --- a/test/Core.Test/AdminConsole/Services/SlackEventHandlerTests.cs +++ b/test/Core.Test/AdminConsole/Services/SlackEventHandlerTests.cs @@ -24,10 +24,8 @@ public class SlackEventHandlerTests private SutProvider GetSutProvider( List> integrationConfigurations) { - _repository.GetConfigurationsAsync( - IntegrationType.Slack, - Arg.Any(), - Arg.Any()) + _repository.GetConfigurationsAsync(Arg.Any(), + IntegrationType.Slack, Arg.Any()) .Returns(integrationConfigurations); return new SutProvider() diff --git a/test/Core.Test/AdminConsole/Services/WebhookEventHandlerTests.cs b/test/Core.Test/AdminConsole/Services/WebhookEventHandlerTests.cs index 4d23ec8989..59f3b1a99c 100644 --- a/test/Core.Test/AdminConsole/Services/WebhookEventHandlerTests.cs +++ b/test/Core.Test/AdminConsole/Services/WebhookEventHandlerTests.cs @@ -47,11 +47,8 @@ public class WebhookEventHandlerTests clientFactory.CreateClient(WebhookEventHandler.HttpClientName).Returns(_httpClient); var repository = Substitute.For(); - repository.GetConfigurationsAsync( - IntegrationType.Webhook, - Arg.Any(), - Arg.Any() - ).Returns(configurations); + repository.GetConfigurationsAsync(Arg.Any(), + IntegrationType.Webhook, Arg.Any()).Returns(configurations); return new SutProvider() .SetDependency(repository)