diff --git a/src/Api/AdminConsole/Controllers/OrganizationConnectionsController.cs b/src/Api/AdminConsole/Controllers/OrganizationConnectionsController.cs index 18389755c57b..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}."); } @@ -92,11 +89,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."); @@ -157,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); @@ -175,6 +170,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) @@ -195,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."); } @@ -205,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/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..2e731d42d08b --- /dev/null +++ b/test/Api.IntegrationTest/AdminConsole/Controllers/OrganizationConnectionsControllerTests.cs @@ -0,0 +1,176 @@ +using System.Net; +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..b9e12857838d 100644 --- a/test/Api.Test/AdminConsole/Controllers/OrganizationConnectionsControllerTests.cs +++ b/test/Api.Test/AdminConsole/Controllers/OrganizationConnectionsControllerTests.cs @@ -408,6 +408,88 @@ 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_TypeCannotBeChanged( + 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); + sutProvider.GetDependency().OrganizationOwner(organizationId).Returns(true); + sutProvider.GetDependency().ManageScim(organizationId).Returns(true); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.UpdateConnection(connectionId, request)); + + Assert.Contains("The connection type cannot be changed.", exception.Message); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .UpdateAsync(default); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .UpdateAsync(default); + } + private static OrganizationConnectionRequestModel RequestModelFromEntity(OrganizationConnection entity) where T : IConnectionConfig {