From 91d61396d0d1a8646e4b2aacf1e72fb1909d8d60 Mon Sep 17 00:00:00 2001 From: Brad Deibert Date: Tue, 8 Sep 2026 16:08:57 -0700 Subject: [PATCH 1/6] use httpclient with ssrf protection in teams connector client --- src/Core/Dirt/Services/Implementations/TeamsService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Core/Dirt/Services/Implementations/TeamsService.cs b/src/Core/Dirt/Services/Implementations/TeamsService.cs index edb43bf85ef8..d1583e18a3c9 100644 --- a/src/Core/Dirt/Services/Implementations/TeamsService.cs +++ b/src/Core/Dirt/Services/Implementations/TeamsService.cs @@ -114,7 +114,7 @@ public async Task> GetJoinedTeamsAsync(string accessToke public async Task SendMessageToChannelAsync(Uri serviceUri, string channelId, string message) { var credentials = new MicrosoftAppCredentials(_clientId, _clientSecret); - using var connectorClient = new ConnectorClient(serviceUri, credentials); + using var connectorClient = new ConnectorClient(serviceUri, credentials, _httpClient); var activity = new Activity { From 250e6131eb3dcb5ee39dd108487dee1b0497ef33 Mon Sep 17 00:00:00 2001 From: Brad Deibert Date: Wed, 9 Sep 2026 11:15:10 -0700 Subject: [PATCH 2/6] check validation state in put controller --- .../Dirt/Controllers/OrganizationIntegrationController.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Api/Dirt/Controllers/OrganizationIntegrationController.cs b/src/Api/Dirt/Controllers/OrganizationIntegrationController.cs index 31acbddd5237..956444ab861f 100644 --- a/src/Api/Dirt/Controllers/OrganizationIntegrationController.cs +++ b/src/Api/Dirt/Controllers/OrganizationIntegrationController.cs @@ -68,6 +68,11 @@ public async Task> CreateAsyn [HttpPut("{integrationId:guid}")] public async Task> UpdateAsync(Guid organizationId, Guid integrationId, [FromBody] OrganizationIntegrationRequestModel model) { + if (!ModelState.IsValid) + { + return BadRequest(ModelState); + } + if (!await HasPermission(organizationId)) { return NotFound(); From 008102d64095df545477858c252f16546639c589 Mon Sep 17 00:00:00 2001 From: Brad Deibert Date: Wed, 9 Sep 2026 14:27:57 -0700 Subject: [PATCH 3/6] add tests to ensure injected httpclient is used --- .../Services/Implementations/TeamsService.cs | 2 +- .../Dirt/Services/TeamsServiceTests.cs | 60 +++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/src/Core/Dirt/Services/Implementations/TeamsService.cs b/src/Core/Dirt/Services/Implementations/TeamsService.cs index d1583e18a3c9..ec3d0bb55502 100644 --- a/src/Core/Dirt/Services/Implementations/TeamsService.cs +++ b/src/Core/Dirt/Services/Implementations/TeamsService.cs @@ -114,7 +114,7 @@ public async Task> GetJoinedTeamsAsync(string accessToke public async Task SendMessageToChannelAsync(Uri serviceUri, string channelId, string message) { var credentials = new MicrosoftAppCredentials(_clientId, _clientSecret); - using var connectorClient = new ConnectorClient(serviceUri, credentials, _httpClient); + using var connectorClient = new ConnectorClient(serviceUri, credentials, _httpClient, disposeHttpClient: false); var activity = new Activity { diff --git a/test/Core.Test/Dirt/Services/TeamsServiceTests.cs b/test/Core.Test/Dirt/Services/TeamsServiceTests.cs index 61d20cc0af68..26a47c61fb23 100644 --- a/test/Core.Test/Dirt/Services/TeamsServiceTests.cs +++ b/test/Core.Test/Dirt/Services/TeamsServiceTests.cs @@ -11,6 +11,7 @@ using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using Bit.Test.Common.MockedHttpClient; +using Microsoft.Bot.Schema; using NSubstitute; using Xunit; using GlobalSettings = Bit.Core.Settings.GlobalSettings; @@ -179,6 +180,65 @@ public async Task GetJoinedTeamsAsync_ServerErrorCode_ReturnsEmptyList() Assert.Empty(result); } + [Fact] + public async Task SendMessageToChannelAsync_SendsViaInjectedHttpClient() + { + var sutProvider = GetSutProvider(); + var serviceUri = new Uri("https://smba.example.com/amer/"); + var channelId = "19:channel-id@thread.tacv2"; + + string? capturedBody = null; + var matcher = _handler + .When(request => + { + capturedBody = request.Content?.ReadAsStringAsync().GetAwaiter().GetResult(); + return true; + }) + .RespondWith(HttpStatusCode.OK) + .WithContent("application/json", JsonSerializer.Serialize(new { id = "activity-id" })); + + await sutProvider.Sut.SendMessageToChannelAsync(serviceUri, channelId, "test message"); + + // The ConnectorClient only reaches this handler if it was constructed with the injected + // (SSRF-protected) HttpClient rather than creating its own. + Assert.Single(_handler.CapturedRequests); + } + + [Fact] + public async Task SendMessageToChannelAsync_CalledMultipleTimes_DoesNotDisposeInjectedHttpClient() + { + var sutProvider = GetSutProvider(); + var serviceUri = new Uri("https://smba.example.com/amer/"); + + _handler + .When(_ => true) + .RespondWith(HttpStatusCode.OK) + .WithContent("application/json", JsonSerializer.Serialize(new { id = "activity-id" })); + + await sutProvider.Sut.SendMessageToChannelAsync(serviceUri, "channel-id", "first message"); + await sutProvider.Sut.SendMessageToChannelAsync(serviceUri, "channel-id", "second message"); + + // Disposing the ConnectorClient must not dispose the shared, factory-provided HttpClient. + Assert.Equal(2, _handler.CapturedRequests.Count); + } + + [Fact] + public async Task SendMessageToChannelAsync_ServerErrorCode_Throws() + { + var sutProvider = GetSutProvider(); + var serviceUri = new Uri("https://smba.example.com/amer/"); + + _handler + .When(_ => true) + .RespondWith(HttpStatusCode.Forbidden) + .WithContent("application/json", JsonSerializer.Serialize(new { error = new { code = "Forbidden" } })); + + await Assert.ThrowsAsync(() => + sutProvider.Sut.SendMessageToChannelAsync(serviceUri, "channel-id", "test message")); + + Assert.Single(_handler.CapturedRequests); + } + [Theory, BitAutoData] public async Task HandleIncomingAppInstall_Success_UpdatesTeamsIntegration( OrganizationIntegration integration) From fb6c32908f7aa7ecd075c8824e9339bd0cca4993 Mon Sep 17 00:00:00 2001 From: Brad Deibert Date: Wed, 9 Sep 2026 15:08:40 -0700 Subject: [PATCH 4/6] remove unused test code --- test/Core.Test/Dirt/Services/TeamsServiceTests.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/test/Core.Test/Dirt/Services/TeamsServiceTests.cs b/test/Core.Test/Dirt/Services/TeamsServiceTests.cs index 26a47c61fb23..376b0f23a3cc 100644 --- a/test/Core.Test/Dirt/Services/TeamsServiceTests.cs +++ b/test/Core.Test/Dirt/Services/TeamsServiceTests.cs @@ -187,13 +187,8 @@ public async Task SendMessageToChannelAsync_SendsViaInjectedHttpClient() var serviceUri = new Uri("https://smba.example.com/amer/"); var channelId = "19:channel-id@thread.tacv2"; - string? capturedBody = null; var matcher = _handler - .When(request => - { - capturedBody = request.Content?.ReadAsStringAsync().GetAwaiter().GetResult(); - return true; - }) + .When(_ => true) .RespondWith(HttpStatusCode.OK) .WithContent("application/json", JsonSerializer.Serialize(new { id = "activity-id" })); From 8df3a92ffdcf18a28afb0503d46482efb0f1a772 Mon Sep 17 00:00:00 2001 From: Brad Deibert Date: Wed, 9 Sep 2026 16:34:17 -0700 Subject: [PATCH 5/6] remove unnecessary model state checks from controllers, update error msg --- .../Controllers/OrganizationIntegrationController.cs | 10 ---------- .../Request/OrganizationIntegrationRequestModel.cs | 5 ++++- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/src/Api/Dirt/Controllers/OrganizationIntegrationController.cs b/src/Api/Dirt/Controllers/OrganizationIntegrationController.cs index 956444ab861f..c0f9ee910bfb 100644 --- a/src/Api/Dirt/Controllers/OrganizationIntegrationController.cs +++ b/src/Api/Dirt/Controllers/OrganizationIntegrationController.cs @@ -42,11 +42,6 @@ public async Task>> GetA [HttpPost("")] public async Task> CreateAsync(Guid organizationId, [FromBody] OrganizationIntegrationRequestModel model) { - if (!ModelState.IsValid) - { - return BadRequest(ModelState); - } - if (!await HasPermission(organizationId)) { return NotFound(); @@ -68,11 +63,6 @@ public async Task> CreateAsyn [HttpPut("{integrationId:guid}")] public async Task> UpdateAsync(Guid organizationId, Guid integrationId, [FromBody] OrganizationIntegrationRequestModel model) { - if (!ModelState.IsValid) - { - return BadRequest(ModelState); - } - if (!await HasPermission(organizationId)) { return NotFound(); diff --git a/src/Api/Dirt/Models/Request/OrganizationIntegrationRequestModel.cs b/src/Api/Dirt/Models/Request/OrganizationIntegrationRequestModel.cs index 259671bd66a7..f925478f2784 100644 --- a/src/Api/Dirt/Models/Request/OrganizationIntegrationRequestModel.cs +++ b/src/Api/Dirt/Models/Request/OrganizationIntegrationRequestModel.cs @@ -28,6 +28,9 @@ public OrganizationIntegration ToOrganizationIntegration(OrganizationIntegration return currentIntegration; } + /// + /// Validates the request model based on the integration type. Runs automatically when the model is bound to a controller action parameter. + /// public IEnumerable Validate(ValidationContext validationContext) { switch (Type) @@ -36,7 +39,7 @@ public IEnumerable Validate(ValidationContext validationContex yield return new ValidationResult($"{nameof(Type)} integrations are not yet supported.", [nameof(Type)]); break; case IntegrationType.Slack or IntegrationType.Teams: - yield return new ValidationResult($"{nameof(Type)} integrations cannot be created directly.", [nameof(Type)]); + yield return new ValidationResult($"{nameof(Type)} integrations cannot be created or updated directly.", [nameof(Type)]); break; case IntegrationType.Webhook: foreach (var r in ValidateConfiguration(allowNullOrEmpty: true)) From db73b41f4f07bb56a5a6818fdb4ca95fd70ca100 Mon Sep 17 00:00:00 2001 From: Brad Deibert Date: Wed, 9 Sep 2026 16:35:40 -0700 Subject: [PATCH 6/6] update tests --- .../Request/OrganizationIntegrationRequestModelTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Api.Test/Dirt/Models/Request/OrganizationIntegrationRequestModelTests.cs b/test/Api.Test/Dirt/Models/Request/OrganizationIntegrationRequestModelTests.cs index 190eae260c8e..fa8a79398537 100644 --- a/test/Api.Test/Dirt/Models/Request/OrganizationIntegrationRequestModelTests.cs +++ b/test/Api.Test/Dirt/Models/Request/OrganizationIntegrationRequestModelTests.cs @@ -87,7 +87,7 @@ public void Validate_Slack_ReturnsCannotBeCreatedDirectlyError() Assert.Single(results); Assert.Contains(nameof(model.Type), results[0].MemberNames); - Assert.Contains("cannot be created directly", results[0].ErrorMessage); + Assert.Contains("cannot be created or updated directly", results[0].ErrorMessage); } [Fact] @@ -103,7 +103,7 @@ public void Validate_Teams_ReturnsCannotBeCreatedDirectlyError() Assert.Single(results); Assert.Contains(nameof(model.Type), results[0].MemberNames); - Assert.Contains("cannot be created directly", results[0].ErrorMessage); + Assert.Contains("cannot be created or updated directly", results[0].ErrorMessage); } [Fact]