From 294e4977b28ec2e06e78ec274d6d8f939e2e1f4a Mon Sep 17 00:00:00 2001 From: Thomas Rittson Date: Fri, 19 Jun 2026 15:18:09 +1000 Subject: [PATCH 1/3] Add test coverage --- .../OrganizationConnectionResponseModel.cs | 4 + .../OrganizationConnectionsControllerTests.cs | 177 ++++++++++++++++++ .../OrganizationConnectionsControllerTests.cs | 100 ++++++++++ 3 files changed, 281 insertions(+) create mode 100644 test/Api.IntegrationTest/AdminConsole/Controllers/OrganizationConnectionsControllerTests.cs diff --git a/src/Api/AdminConsole/Models/Response/Organizations/OrganizationConnectionResponseModel.cs b/src/Api/AdminConsole/Models/Response/Organizations/OrganizationConnectionResponseModel.cs index f365080b73bc..d3211d511052 100644 --- a/src/Api/AdminConsole/Models/Response/Organizations/OrganizationConnectionResponseModel.cs +++ b/src/Api/AdminConsole/Models/Response/Organizations/OrganizationConnectionResponseModel.cs @@ -2,6 +2,7 @@ #nullable disable using System.Text.Json; +using System.Text.Json.Serialization; using Bit.Core.Entities; using Bit.Core.Enums; @@ -15,6 +16,9 @@ public class OrganizationConnectionResponseModel public bool Enabled { get; set; } public JsonDocument Config { get; set; } + [JsonConstructor] + public OrganizationConnectionResponseModel() { } + public OrganizationConnectionResponseModel(OrganizationConnection connection, Type configType) { if (connection == null) diff --git a/test/Api.IntegrationTest/AdminConsole/Controllers/OrganizationConnectionsControllerTests.cs b/test/Api.IntegrationTest/AdminConsole/Controllers/OrganizationConnectionsControllerTests.cs new file mode 100644 index 000000000000..b5b74c06f7e6 --- /dev/null +++ b/test/Api.IntegrationTest/AdminConsole/Controllers/OrganizationConnectionsControllerTests.cs @@ -0,0 +1,177 @@ +using System.Net; +using System.Net.Http.Json; +using Bit.Api.AdminConsole.Models.Response.Organizations; +using Bit.Api.IntegrationTest.Factories; +using Bit.Api.IntegrationTest.Helpers; +using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.Models.OrganizationConnectionConfigs; +using Bit.Core.Billing.Enums; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Repositories; +using Xunit; + +namespace Bit.Api.IntegrationTest.AdminConsole.Controllers; + +public class OrganizationConnectionsControllerTests : IClassFixture, IAsyncLifetime +{ + private readonly HttpClient _client; + private readonly ApiApplicationFactory _factory; + private readonly LoginHelper _loginHelper; + + private Organization _organization = null!; + private string _ownerEmail = null!; + + public OrganizationConnectionsControllerTests(ApiApplicationFactory apiFactory) + { + _factory = apiFactory; + _client = _factory.CreateClient(); + _loginHelper = new LoginHelper(_factory, _client); + } + + public async Task InitializeAsync() + { + _ownerEmail = $"org-connections-test-{Guid.NewGuid()}@example.com"; + await _factory.LoginWithNewAccount(_ownerEmail); + + (_organization, _) = await OrganizationTestHelpers.SignUpAsync(_factory, plan: PlanType.EnterpriseAnnually, + ownerEmail: _ownerEmail, passwordManagerSeats: 5, paymentMethod: PaymentMethodType.Card); + } + + public Task DisposeAsync() + { + _client.Dispose(); + return Task.CompletedTask; + } + + [Fact] + public async Task CreateConnection_AsOwner_Succeeds() + { + await _loginHelper.LoginAsync(_ownerEmail); + + var response = await _client.PostAsJsonAsync( + "organizations/connections", + BuildScimRequest(_organization.Id, enabled: true)); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var responseBody = await response.Content.ReadFromJsonAsync(); + Assert.NotNull(responseBody); + Assert.Equal(OrganizationConnectionType.Scim, responseBody!.Type); + Assert.Equal(_organization.Id, responseBody.OrganizationId); + + var connectionRepository = _factory.GetService(); + var persisted = await connectionRepository.GetByOrganizationIdTypeAsync( + _organization.Id, OrganizationConnectionType.Scim); + var single = Assert.Single(persisted); + Assert.True(single.GetConfig()!.Enabled); + } + + [Fact] + public async Task CreateConnection_AsRegularUser_Forbidden() + { + var (regularUserEmail, _) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync( + _factory, _organization.Id, OrganizationUserType.User); + await _loginHelper.LoginAsync(regularUserEmail); + + var response = await _client.PostAsJsonAsync( + "organizations/connections", + BuildScimRequest(_organization.Id, enabled: true)); + + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + var body = await response.Content.ReadAsStringAsync(); + Assert.Contains("You do not have permission to create a connection of type", body); + } + + [Fact] + public async Task UpdateConnection_AsOwner_Succeeds() + { + var existing = await SeedScimConnectionAsync(enabled: false); + + await _loginHelper.LoginAsync(_ownerEmail); + + var response = await _client.PutAsJsonAsync( + $"organizations/connections/{existing.Id}", + BuildScimRequest(_organization.Id, enabled: true)); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + var connectionRepository = _factory.GetService(); + var persisted = await connectionRepository.GetByIdAsync(existing.Id); + Assert.NotNull(persisted); + Assert.True(persisted!.Enabled); + Assert.True(persisted.GetConfig()!.Enabled); + } + + [Fact] + public async Task UpdateConnection_AsRegularUser_Forbidden() + { + var existing = await SeedScimConnectionAsync(); + + var (regularUserEmail, _) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync( + _factory, _organization.Id, OrganizationUserType.User); + await _loginHelper.LoginAsync(regularUserEmail); + + var response = await _client.PutAsJsonAsync( + $"organizations/connections/{existing.Id}", + BuildScimRequest(_organization.Id, enabled: false)); + + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + var body = await response.Content.ReadAsStringAsync(); + Assert.Contains("You do not have permission to update this connection.", body); + } + + [Fact] + public async Task GetConnection_AsOwner_Succeeds() + { + var existing = await SeedScimConnectionAsync(); + + await _loginHelper.LoginAsync(_ownerEmail); + + var response = await _client.GetAsync( + $"organizations/connections/{_organization.Id}/{(int)OrganizationConnectionType.Scim}"); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var responseBody = await response.Content.ReadFromJsonAsync(); + Assert.NotNull(responseBody); + Assert.Equal(existing.Id, responseBody!.Id); + Assert.Equal(OrganizationConnectionType.Scim, responseBody.Type); + } + + [Fact] + public async Task DeleteConnection_AsOwner_Succeeds() + { + var existing = await SeedScimConnectionAsync(); + + await _loginHelper.LoginAsync(_ownerEmail); + + var response = await _client.DeleteAsync($"organizations/connections/{existing.Id}"); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + var connectionRepository = _factory.GetService(); + var persisted = await connectionRepository.GetByIdAsync(existing.Id); + Assert.Null(persisted); + } + + private async Task SeedScimConnectionAsync(bool enabled = true) + { + var connectionRepository = _factory.GetService(); + var connection = new OrganizationConnection + { + OrganizationId = _organization.Id, + Type = OrganizationConnectionType.Scim, + Enabled = enabled, + }; + connection.SetConfig(new ScimConfig { Enabled = enabled }); + return await connectionRepository.CreateAsync(connection); + } + + private static object BuildScimRequest(Guid organizationId, bool enabled) => + new + { + type = OrganizationConnectionType.Scim, + organizationId, + enabled, + config = new ScimConfig { Enabled = enabled }, + }; +} diff --git a/test/Api.Test/AdminConsole/Controllers/OrganizationConnectionsControllerTests.cs b/test/Api.Test/AdminConsole/Controllers/OrganizationConnectionsControllerTests.cs index 363f86884ebe..2a49287740ea 100644 --- a/test/Api.Test/AdminConsole/Controllers/OrganizationConnectionsControllerTests.cs +++ b/test/Api.Test/AdminConsole/Controllers/OrganizationConnectionsControllerTests.cs @@ -10,6 +10,7 @@ using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; +using Bit.Core.Models.Data.Organizations.OrganizationConnections; using Bit.Core.Models.OrganizationConnectionConfigs; using Bit.Core.Repositories; using Bit.Core.Settings; @@ -408,6 +409,105 @@ public async Task DeleteConnection_Success(OrganizationConnection connection, await sutProvider.GetDependency().DeleteAsync(connection); } + [Theory] + [BitAutoData] + public async Task UpdateConnection_PermissionCheckUsesPersistedType_NotRequestType( + Guid connectionId, Guid organizationId, BillingSyncConfig config, + SutProvider sutProvider) + { + var existing = new OrganizationConnection + { + Id = connectionId, + Type = OrganizationConnectionType.CloudBillingSync, + OrganizationId = organizationId, + Config = JsonSerializer.Serialize(config), + }; + + var request = new OrganizationConnectionRequestModel + { + Type = OrganizationConnectionType.Scim, + OrganizationId = organizationId, + Enabled = true, + Config = JsonDocumentFromObject(new ScimConfig()), + }; + + sutProvider.GetDependency() + .GetByIdOrganizationIdAsync(connectionId, organizationId) + .Returns(existing); + + // The user can update a Scim connection but not a Cloud Billing Sync connection + sutProvider.GetDependency().ManageScim(organizationId).Returns(true); + sutProvider.GetDependency().OrganizationOwner(organizationId).Returns(false); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.UpdateConnection(connectionId, request)); + + Assert.Contains("You do not have permission to update this connection.", exception.Message); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .UpdateAsync(default); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .UpdateAsync(default); + } + + [Theory] + [BitAutoData] + public async Task UpdateConnection_IgnoresChangeToType( + Guid connectionId, Guid organizationId, + BillingSyncConfig config, OrganizationLicense organizationLicense, + SutProvider sutProvider) + { + organizationLicense.Id = config.CloudOrganizationId; + organizationLicense.Issued = DateTime.UtcNow.AddDays(-1); + organizationLicense.Expires = DateTime.UtcNow.AddDays(1); + organizationLicense.Version = 1; + + var existing = new OrganizationConnection + { + Id = connectionId, + Type = OrganizationConnectionType.CloudBillingSync, + OrganizationId = organizationId, + Config = JsonSerializer.Serialize(config), + }; + + var request = new OrganizationConnectionRequestModel + { + Type = OrganizationConnectionType.Scim, + OrganizationId = organizationId, + Enabled = true, + Config = JsonDocumentFromObject(new ScimConfig()), + }; + + sutProvider.GetDependency() + .GetByIdOrganizationIdAsync(connectionId, organizationId) + .Returns(existing); + sutProvider.GetDependency().OrganizationOwner(organizationId).Returns(true); + sutProvider.GetDependency().ManageScim(organizationId).Returns(true); + sutProvider.GetDependency().SelfHosted.Returns(true); + sutProvider.GetDependency() + .ReadOrganizationLicenseAsync(Arg.Any()) + .Returns(organizationLicense); + sutProvider.GetDependency() + .VerifyLicense(organizationLicense) + .Returns(true); + sutProvider.GetDependency() + .UpdateAsync(default) + .ReturnsForAnyArgs(existing); + + await sutProvider.Sut.UpdateConnection(connectionId, request); + + // Expect: connection type has not changed + await sutProvider.GetDependency().Received(1) + .UpdateAsync(Arg.Is>(d => + d.Type == OrganizationConnectionType.CloudBillingSync + && d.OrganizationId == organizationId + && d.Id == connectionId)); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .UpdateAsync(default); + } + private static OrganizationConnectionRequestModel RequestModelFromEntity(OrganizationConnection entity) where T : IConnectionConfig { From 599cc072394a59bb8c9cc820f73da7ef0d23790c Mon Sep 17 00:00:00 2001 From: Thomas Rittson Date: Sat, 20 Jun 2026 08:44:18 +1000 Subject: [PATCH 2/3] Fix authz --- .../OrganizationConnectionsController.cs | 16 ++++++++- .../OrganizationConnectionsControllerTests.cs | 34 +++++-------------- 2 files changed, 23 insertions(+), 27 deletions(-) diff --git a/src/Api/AdminConsole/Controllers/OrganizationConnectionsController.cs b/src/Api/AdminConsole/Controllers/OrganizationConnectionsController.cs index 18389755c57b..40fee169e086 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationConnectionsController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationConnectionsController.cs @@ -92,11 +92,16 @@ public async Task UpdateConnection(Guid org throw new NotFoundException(); } - if (!await HasPermissionAsync(model?.OrganizationId, model?.Type)) + if (!await HasPermissionAsync(existingOrganizationConnection.OrganizationId, existingOrganizationConnection.Type)) { throw new BadRequestException("You do not have permission to update this connection."); } + if (model.Type != existingOrganizationConnection.Type) + { + throw new BadRequestException("The connection type cannot be changed."); + } + if (await HasConnectionTypeAsync(model, organizationConnectionId, model.Type)) { throw new BadRequestException($"The requested organization already has a connection of type {model.Type}. Only one of each connection type may exist per organization."); @@ -175,6 +180,15 @@ private async Task HasConnectionTypeAsync(OrganizationConnectionRequestMod return existingConnections.Any(c => c.Type == model.Type && (!connectionId.HasValue || c.Id != connectionId.Value)); } + /// + /// Returns whether the current user has permission to manage a connection of the given + /// in the given organization. The required permission varies by connection type (e.g. Scim requires Manage SCIM, + /// while CloudBillingSync requires Organization Owner). + /// + /// + /// When authorizing an update or delete against an existing connection, MUST be sourced + /// from the persisted connection — never from the request body. + /// private async Task HasPermissionAsync(Guid? organizationId, OrganizationConnectionType? type = null) { if (!organizationId.HasValue) diff --git a/test/Api.Test/AdminConsole/Controllers/OrganizationConnectionsControllerTests.cs b/test/Api.Test/AdminConsole/Controllers/OrganizationConnectionsControllerTests.cs index 2a49287740ea..b9e12857838d 100644 --- a/test/Api.Test/AdminConsole/Controllers/OrganizationConnectionsControllerTests.cs +++ b/test/Api.Test/AdminConsole/Controllers/OrganizationConnectionsControllerTests.cs @@ -10,7 +10,6 @@ using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; -using Bit.Core.Models.Data.Organizations.OrganizationConnections; using Bit.Core.Models.OrganizationConnectionConfigs; using Bit.Core.Repositories; using Bit.Core.Settings; @@ -453,16 +452,10 @@ await sutProvider.GetDependency() [Theory] [BitAutoData] - public async Task UpdateConnection_IgnoresChangeToType( - Guid connectionId, Guid organizationId, - BillingSyncConfig config, OrganizationLicense organizationLicense, + public async Task UpdateConnection_TypeCannotBeChanged( + Guid connectionId, Guid organizationId, BillingSyncConfig config, SutProvider sutProvider) { - organizationLicense.Id = config.CloudOrganizationId; - organizationLicense.Issued = DateTime.UtcNow.AddDays(-1); - organizationLicense.Expires = DateTime.UtcNow.AddDays(1); - organizationLicense.Version = 1; - var existing = new OrganizationConnection { Id = connectionId, @@ -484,25 +477,14 @@ public async Task UpdateConnection_IgnoresChangeToType( .Returns(existing); sutProvider.GetDependency().OrganizationOwner(organizationId).Returns(true); sutProvider.GetDependency().ManageScim(organizationId).Returns(true); - sutProvider.GetDependency().SelfHosted.Returns(true); - sutProvider.GetDependency() - .ReadOrganizationLicenseAsync(Arg.Any()) - .Returns(organizationLicense); - sutProvider.GetDependency() - .VerifyLicense(organizationLicense) - .Returns(true); - sutProvider.GetDependency() - .UpdateAsync(default) - .ReturnsForAnyArgs(existing); - await sutProvider.Sut.UpdateConnection(connectionId, request); + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.UpdateConnection(connectionId, request)); - // Expect: connection type has not changed - await sutProvider.GetDependency().Received(1) - .UpdateAsync(Arg.Is>(d => - d.Type == OrganizationConnectionType.CloudBillingSync - && d.OrganizationId == organizationId - && d.Id == connectionId)); + Assert.Contains("The connection type cannot be changed.", exception.Message); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .UpdateAsync(default); await sutProvider.GetDependency() .DidNotReceiveWithAnyArgs() .UpdateAsync(default); From e7b4ac72ed1dd87960d0bf5b0753a70802cc5627 Mon Sep 17 00:00:00 2001 From: Thomas Rittson Date: Sat, 20 Jun 2026 08:51:35 +1000 Subject: [PATCH 3/3] Remove dead endpoint and nullable directive --- .../OrganizationConnectionsController.cs | 19 +++++-------------- .../OrganizationConnectionsControllerTests.cs | 3 +-- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/src/Api/AdminConsole/Controllers/OrganizationConnectionsController.cs b/src/Api/AdminConsole/Controllers/OrganizationConnectionsController.cs index 40fee169e086..ff72430d4567 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationConnectionsController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationConnectionsController.cs @@ -1,7 +1,4 @@ -// FIXME: Update this file to be null safe and then delete the line below -#nullable disable - -using Bit.Api.AdminConsole.Models.Request.Organizations; +using Bit.Api.AdminConsole.Models.Request.Organizations; using Bit.Api.AdminConsole.Models.Response.Organizations; using Bit.Core.AdminConsole.Models.OrganizationConnectionConfigs; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationConnections.Interfaces; @@ -57,7 +54,7 @@ public bool ConnectionsEnabled() [HttpPost] public async Task CreateConnection([FromBody] OrganizationConnectionRequestModel model) { - if (!await HasPermissionAsync(model?.OrganizationId, model?.Type)) + if (!await HasPermissionAsync(model.OrganizationId, model.Type)) { throw new BadRequestException($"You do not have permission to create a connection of type {model.Type}."); } @@ -162,13 +159,6 @@ public async Task DeleteConnection(Guid organizationConnectionId) await _deleteOrganizationConnectionCommand.DeleteAsync(connection); } - [HttpPost("{organizationConnectionId}/delete")] - [Obsolete("This endpoint is deprecated. Use DELETE method instead")] - public async Task PostDeleteConnection(Guid organizationConnectionId) - { - await DeleteConnection(organizationConnectionId); - } - private async Task> GetConnectionsAsync(Guid organizationId, OrganizationConnectionType type) => await _organizationConnectionRepository.GetByOrganizationIdTypeAsync(organizationId, type); @@ -209,7 +199,8 @@ private async Task ValidateBillingSyncConfig(OrganizationConnectionRequestModel< throw new BadRequestException($"Cannot create a {typedModel.Type} connection outside of a self-hosted instance."); } var license = await _licensingService.ReadOrganizationLicenseAsync(typedModel.OrganizationId); - if (!_licensingService.VerifyLicense(license)) + + if (license == null || !_licensingService.VerifyLicense(license)) { throw new BadRequestException("Cannot verify license file."); } @@ -219,7 +210,7 @@ private async Task ValidateBillingSyncConfig(OrganizationConnectionRequestModel< private async Task CreateOrUpdateOrganizationConnectionAsync( Guid? organizationConnectionId, OrganizationConnectionRequestModel model, - Func, Task> validateAction = null) + Func, Task>? validateAction = null) where T : IConnectionConfig { var typedModel = new OrganizationConnectionRequestModel(model); diff --git a/test/Api.IntegrationTest/AdminConsole/Controllers/OrganizationConnectionsControllerTests.cs b/test/Api.IntegrationTest/AdminConsole/Controllers/OrganizationConnectionsControllerTests.cs index b5b74c06f7e6..2e731d42d08b 100644 --- a/test/Api.IntegrationTest/AdminConsole/Controllers/OrganizationConnectionsControllerTests.cs +++ b/test/Api.IntegrationTest/AdminConsole/Controllers/OrganizationConnectionsControllerTests.cs @@ -1,5 +1,4 @@ -using System.Net; -using System.Net.Http.Json; +using System.Net; using Bit.Api.AdminConsole.Models.Response.Organizations; using Bit.Api.IntegrationTest.Factories; using Bit.Api.IntegrationTest.Helpers;