Fix OrganizationConnection Update (#2071)

* Force CloudOrganizationId to be read only

* Fix tests

src/Api/Controllers/OrganizationConnectionsController.cs

@@ -89,6 +89,12 @@ namespace Bit.Api.Controllers
         [HttpPut("{organizationConnectionId}")]
         public async Task<OrganizationConnectionResponseModel> UpdateConnection(Guid organizationConnectionId, [FromBody] OrganizationConnectionRequestModel model)
         {
+            var existingOrganizationConnection = await _organizationConnectionRepository.GetByIdAsync(organizationConnectionId);
+            if (existingOrganizationConnection == null)
+            {
+                throw new NotFoundException();
+            }
+
             if (!await HasPermissionAsync(model?.OrganizationId))
             {
                 throw new BadRequestException("Only the owner of an organization can update a connection.");
@@ -103,6 +109,8 @@ namespace Bit.Api.Controllers
             {
                 case OrganizationConnectionType.CloudBillingSync:
                     var typedModel = new OrganizationConnectionRequestModel<BillingSyncConfig>(model);
+                    // We don't allow overwriting or changing the CloudOrganizationId so save it from the existing connection
+                    typedModel.ParsedConfig.CloudOrganizationId = existingOrganizationConnection.GetConfig<BillingSyncConfig>().CloudOrganizationId;
                     var connection = await _updateOrganizationConnectionCommand.UpdateAsync(typedModel.ToData(organizationConnectionId));
                     return new OrganizationConnectionResponseModel(connection, typeof(BillingSyncConfig));
                 default:

test/Api.Test/Controllers/OrganizationConnectionsControllerTests.cs

@@ -141,6 +141,10 @@ namespace Bit.Api.Test.Controllers
         [BitAutoData]
         public async Task UpdateConnection_RequiresOwnerPermissions(SutProvider<OrganizationConnectionsController> sutProvider)
         {
+            sutProvider.GetDependency<IOrganizationConnectionRepository>()
+                .GetByIdAsync(Arg.Any<Guid>())
+                .Returns(new OrganizationConnection());
+
             var exception = await Assert.ThrowsAsync<BadRequestException>(() => sutProvider.Sut.UpdateConnection(default, null));
 
             Assert.Contains("Only the owner of an organization can update a connection.", exception.Message);
@@ -157,6 +161,10 @@ namespace Bit.Api.Test.Controllers
 
             sutProvider.GetDependency<ICurrentContext>().OrganizationOwner(typedModel.OrganizationId).Returns(true);
 
+            sutProvider.GetDependency<IOrganizationConnectionRepository>()
+                .GetByIdAsync(existing1.Id)
+                .Returns(existing1);
+
             sutProvider.GetDependency<IOrganizationConnectionRepository>().GetByOrganizationIdTypeAsync(typedModel.OrganizationId, type).Returns(new[] { existing1, existing2 });
 
             var exception = await Assert.ThrowsAsync<BadRequestException>(() => sutProvider.Sut.UpdateConnection(existing1.Id, typedModel));
@@ -170,6 +178,10 @@ namespace Bit.Api.Test.Controllers
             OrganizationConnection updated,
             SutProvider<OrganizationConnectionsController> sutProvider)
         {
+            existing.SetConfig(new BillingSyncConfig
+            {
+                CloudOrganizationId = config.CloudOrganizationId,
+            });
             updated.Config = JsonSerializer.Serialize(config);
             updated.Id = existing.Id;
             var model = RequestModelFromEntity(updated);
@@ -177,6 +189,9 @@ namespace Bit.Api.Test.Controllers
             sutProvider.GetDependency<ICurrentContext>().OrganizationOwner(model.OrganizationId).Returns(true);
             sutProvider.GetDependency<IOrganizationConnectionRepository>().GetByOrganizationIdTypeAsync(model.OrganizationId, model.Type).Returns(new[] { existing });
             sutProvider.GetDependency<IUpdateOrganizationConnectionCommand>().UpdateAsync<BillingSyncConfig>(default).ReturnsForAnyArgs(updated);
+            sutProvider.GetDependency<IOrganizationConnectionRepository>()
+                .GetByIdAsync(existing.Id)
+                .Returns(existing);
 
             var expected = new OrganizationConnectionResponseModel(updated, typeof(BillingSyncConfig));
             var result = await sutProvider.Sut.UpdateConnection(existing.Id, model);
@@ -186,6 +201,13 @@ namespace Bit.Api.Test.Controllers
                 .UpdateAsync(Arg.Is(AssertHelper.AssertPropertyEqual(model.ToData(updated.Id))));
         }
 
+        [Theory]
+        [BitAutoData]
+        public async Task UpdateConnection_DoesNotExist_ThrowsNotFound(SutProvider<OrganizationConnectionsController> sutProvider)
+        {
+            await Assert.ThrowsAsync<NotFoundException>(() => sutProvider.Sut.UpdateConnection(Guid.NewGuid(), null));
+        }
+
         [Theory]
         [BitAutoData]
         public async Task GetConnection_RequiresOwnerPermissions(Guid connectionId, SutProvider<OrganizationConnectionsController> sutProvider)