[AC-1654] idor allow the attacker to disable any one scim provising (#3325)

* [AC-1654] Added IOrganizationConnectionRepository.GetByIdOrganizationIdAsync and modified OrganizationConnectionsController to use it to get a connection matching both Id and OrganizationId

* [AC-1654] Fixed unit tests

src/Api/Controllers/OrganizationConnectionsController.cs

@@ -78,7 +78,12 @@ public class OrganizationConnectionsController : Controller
     [HttpPut("{organizationConnectionId}")]
     public async Task<OrganizationConnectionResponseModel> UpdateConnection(Guid organizationConnectionId, [FromBody] OrganizationConnectionRequestModel model)
     {
-        var existingOrganizationConnection = await _organizationConnectionRepository.GetByIdAsync(organizationConnectionId);
+        if (model == null)
+        {
+            throw new NotFoundException();
+        }
+
+        var existingOrganizationConnection = await _organizationConnectionRepository.GetByIdOrganizationIdAsync(organizationConnectionId, model.OrganizationId);
         if (existingOrganizationConnection == null)
         {
             throw new NotFoundException();

src/Core/Repositories/IOrganizationConnectionRepository.cs

@@ -5,6 +5,7 @@ namespace Bit.Core.Repositories;
 
 public interface IOrganizationConnectionRepository : IRepository<OrganizationConnection, Guid>
 {
+    Task<OrganizationConnection> GetByIdOrganizationIdAsync(Guid id, Guid organizationId);
     Task<ICollection<OrganizationConnection>> GetByOrganizationIdTypeAsync(Guid organizationId, OrganizationConnectionType type);
     Task<ICollection<OrganizationConnection>> GetEnabledByOrganizationIdTypeAsync(Guid organizationId, OrganizationConnectionType type);
 }

src/Infrastructure.Dapper/Repositories/OrganizationConnectionRepository.cs

@@ -14,6 +14,23 @@ public class OrganizationConnectionRepository : Repository<OrganizationConnectio
         : base(globalSettings.SqlServer.ConnectionString, globalSettings.SqlServer.ReadOnlyConnectionString)
     { }
 
+    public async Task<OrganizationConnection> GetByIdOrganizationIdAsync(Guid id, Guid organizationId)
+    {
+        using (var connection = new SqlConnection(ConnectionString))
+        {
+            var results = await connection.QueryAsync<OrganizationConnection>(
+                $"[{Schema}].[OrganizationConnection_ReadByIdOrganizationId]",
+                new
+                {
+                    Id = id,
+                    OrganizationId = organizationId
+                },
+                commandType: CommandType.StoredProcedure);
+
+            return results.FirstOrDefault();
+        }
+    }
+
     public async Task<ICollection<OrganizationConnection>> GetByOrganizationIdTypeAsync(Guid organizationId, OrganizationConnectionType type)
     {
         using (var connection = new SqlConnection(ConnectionString))

src/Infrastructure.EntityFramework/Repositories/OrganizationConnectionRepository.cs

@@ -15,6 +15,17 @@ public class OrganizationConnectionRepository : Repository<OrganizationConnectio
     {
     }
 
+    public async Task<OrganizationConnection> GetByIdOrganizationIdAsync(Guid id, Guid organizationId)
+    {
+        using (var scope = ServiceScopeFactory.CreateScope())
+        {
+            var dbContext = GetDatabaseContext(scope);
+            var connection = await dbContext.OrganizationConnections
+                .FirstOrDefaultAsync(oc => oc.Id == id && oc.OrganizationId == organizationId);
+            return Mapper.Map<OrganizationConnection>(connection);
+        }
+    }
+
     public async Task<ICollection<OrganizationConnection>> GetByOrganizationIdTypeAsync(Guid organizationId, OrganizationConnectionType type)
     {
         using (var scope = ServiceScopeFactory.CreateScope())

src/Sql/dbo/Stored Procedures/OrganizationConnection_ReadByIdOrganizationId.sql

@@ -0,0 +1,15 @@
+CREATE PROCEDURE [dbo].[OrganizationConnection_ReadByIdOrganizationId]
+    @Id UNIQUEIDENTIFIER,
+    @OrganizationId UNIQUEIDENTIFIER
+AS
+BEGIN
+    SET NOCOUNT ON
+
+    SELECT
+        *
+    FROM
+        [dbo].[OrganizationConnectionView]
+    WHERE
+        [Id] = @Id AND
+        [OrganizationId] = @OrganizationId
+END

test/Api.Test/Controllers/OrganizationConnectionsControllerTests.cs

@@ -143,10 +143,10 @@ public class OrganizationConnectionsControllerTests
     public async Task UpdateConnection_RequiresOwnerPermissions(SutProvider<OrganizationConnectionsController> sutProvider)
     {
         sutProvider.GetDependency<IOrganizationConnectionRepository>()
-            .GetByIdAsync(Arg.Any<Guid>())
+            .GetByIdOrganizationIdAsync(Arg.Any<Guid>(), Arg.Any<Guid>())
             .Returns(new OrganizationConnection());
 
-        var exception = await Assert.ThrowsAsync<BadRequestException>(() => sutProvider.Sut.UpdateConnection(default, null));
+        var exception = await Assert.ThrowsAsync<BadRequestException>(() => sutProvider.Sut.UpdateConnection(default, new OrganizationConnectionRequestModel()));
 
         Assert.Contains("You do not have permission to update this connection.", exception.Message);
     }
@@ -164,8 +164,8 @@ public class OrganizationConnectionsControllerTests
         sutProvider.GetDependency<ICurrentContext>().OrganizationOwner(typedModel.OrganizationId).Returns(true);
 
         var orgConnectionRepository = sutProvider.GetDependency<IOrganizationConnectionRepository>();
-        orgConnectionRepository.GetByIdAsync(existing1.Id).Returns(existing1);
-        orgConnectionRepository.GetByIdAsync(existing2.Id).Returns(existing2);
+        orgConnectionRepository.GetByIdOrganizationIdAsync(existing1.Id, existing1.OrganizationId).Returns(existing1);
+        orgConnectionRepository.GetByIdOrganizationIdAsync(existing2.Id, existing2.OrganizationId).Returns(existing2);
         orgConnectionRepository.GetByOrganizationIdTypeAsync(typedModel.OrganizationId, type).Returns(new[] { existing1, existing2 });
 
         var exception = await Assert.ThrowsAsync<BadRequestException>(() => sutProvider.Sut.UpdateConnection(existing1.Id, typedModel));
@@ -186,7 +186,7 @@ public class OrganizationConnectionsControllerTests
         sutProvider.GetDependency<ICurrentContext>().OrganizationOwner(typedModel.OrganizationId).Returns(true);
 
         sutProvider.GetDependency<IOrganizationConnectionRepository>()
-            .GetByIdAsync(existing1.Id)
+            .GetByIdOrganizationIdAsync(existing1.Id, existing1.OrganizationId)
             .Returns(existing1);
 
         sutProvider.GetDependency<ICurrentContext>().ManageScim(typedModel.OrganizationId).Returns(true);
@@ -212,6 +212,7 @@ public class OrganizationConnectionsControllerTests
         });
         updated.Config = JsonSerializer.Serialize(config);
         updated.Id = existing.Id;
+        updated.OrganizationId = existing.OrganizationId;
         updated.Type = OrganizationConnectionType.CloudBillingSync;
         var model = RequestModelFromEntity<BillingSyncConfig>(updated);
 
@@ -224,7 +225,7 @@ public class OrganizationConnectionsControllerTests
             .UpdateAsync<BillingSyncConfig>(default)
             .ReturnsForAnyArgs(updated);
         sutProvider.GetDependency<IOrganizationConnectionRepository>()
-            .GetByIdAsync(existing.Id)
+            .GetByIdOrganizationIdAsync(existing.Id, existing.OrganizationId)
             .Returns(existing);
 
         OrganizationLicense organizationLicense = new OrganizationLicense();
@@ -264,6 +265,7 @@ public class OrganizationConnectionsControllerTests
         });
         updated.Config = JsonSerializer.Serialize(config);
         updated.Id = existing.Id;
+        updated.OrganizationId = existing.OrganizationId;
         updated.Type = OrganizationConnectionType.CloudBillingSync;
         var model = RequestModelFromEntity<BillingSyncConfig>(updated);
         sutProvider.GetDependency<IGlobalSettings>().SelfHosted.Returns(true);
@@ -275,7 +277,7 @@ public class OrganizationConnectionsControllerTests
             .UpdateAsync<BillingSyncConfig>(default)
             .ReturnsForAnyArgs(updated);
         sutProvider.GetDependency<IOrganizationConnectionRepository>()
-            .GetByIdAsync(existing.Id)
+            .GetByIdOrganizationIdAsync(existing.Id, existing.OrganizationId)
             .Returns(existing);
 
         OrganizationLicense organizationLicense = new OrganizationLicense();

util/Migrator/DbScripts/2023-10-05_00_OrgConnectionsReadByIdOrgId.sql

@@ -0,0 +1,16 @@
+CREATE OR ALTER PROCEDURE [dbo].[OrganizationConnection_ReadByIdOrganizationId]
+    @Id UNIQUEIDENTIFIER,
+    @OrganizationId UNIQUEIDENTIFIER
+AS
+BEGIN
+    SET NOCOUNT ON
+
+SELECT
+    *
+FROM
+    [dbo].[OrganizationConnectionView]
+WHERE
+    [Id] = @Id AND
+    [OrganizationId] = @OrganizationId
+END
+GO