From cb73056c42aacfd2e7280f186d625e7527e77e57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Wed, 18 Oct 2023 11:39:00 +0100 Subject: [PATCH] =?UTF-8?q?[AC-1654]=C2=A0idor=20allow=20the=20attacker=20?= =?UTF-8?q?to=20disable=20any=20one=20scim=20provising=20(#3325)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [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 --- .../OrganizationConnectionsController.cs | 7 ++++++- .../IOrganizationConnectionRepository.cs | 1 + .../OrganizationConnectionRepository.cs | 17 +++++++++++++++++ .../OrganizationConnectionRepository.cs | 11 +++++++++++ ...izationConnection_ReadByIdOrganizationId.sql | 15 +++++++++++++++ .../OrganizationConnectionsControllerTests.cs | 16 +++++++++------- ...023-10-05_00_OrgConnectionsReadByIdOrgId.sql | 16 ++++++++++++++++ 7 files changed, 75 insertions(+), 8 deletions(-) create mode 100644 src/Sql/dbo/Stored Procedures/OrganizationConnection_ReadByIdOrganizationId.sql create mode 100644 util/Migrator/DbScripts/2023-10-05_00_OrgConnectionsReadByIdOrgId.sql diff --git a/src/Api/Controllers/OrganizationConnectionsController.cs b/src/Api/Controllers/OrganizationConnectionsController.cs index 4c04a2497..e9ea31c8f 100644 --- a/src/Api/Controllers/OrganizationConnectionsController.cs +++ b/src/Api/Controllers/OrganizationConnectionsController.cs @@ -78,7 +78,12 @@ public class OrganizationConnectionsController : Controller [HttpPut("{organizationConnectionId}")] public async Task 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(); diff --git a/src/Core/Repositories/IOrganizationConnectionRepository.cs b/src/Core/Repositories/IOrganizationConnectionRepository.cs index a3bdbb037..b480333f6 100644 --- a/src/Core/Repositories/IOrganizationConnectionRepository.cs +++ b/src/Core/Repositories/IOrganizationConnectionRepository.cs @@ -5,6 +5,7 @@ namespace Bit.Core.Repositories; public interface IOrganizationConnectionRepository : IRepository { + Task GetByIdOrganizationIdAsync(Guid id, Guid organizationId); Task> GetByOrganizationIdTypeAsync(Guid organizationId, OrganizationConnectionType type); Task> GetEnabledByOrganizationIdTypeAsync(Guid organizationId, OrganizationConnectionType type); } diff --git a/src/Infrastructure.Dapper/Repositories/OrganizationConnectionRepository.cs b/src/Infrastructure.Dapper/Repositories/OrganizationConnectionRepository.cs index 7ea1c7227..0168f57e3 100644 --- a/src/Infrastructure.Dapper/Repositories/OrganizationConnectionRepository.cs +++ b/src/Infrastructure.Dapper/Repositories/OrganizationConnectionRepository.cs @@ -14,6 +14,23 @@ public class OrganizationConnectionRepository : Repository GetByIdOrganizationIdAsync(Guid id, Guid organizationId) + { + using (var connection = new SqlConnection(ConnectionString)) + { + var results = await connection.QueryAsync( + $"[{Schema}].[OrganizationConnection_ReadByIdOrganizationId]", + new + { + Id = id, + OrganizationId = organizationId + }, + commandType: CommandType.StoredProcedure); + + return results.FirstOrDefault(); + } + } + public async Task> GetByOrganizationIdTypeAsync(Guid organizationId, OrganizationConnectionType type) { using (var connection = new SqlConnection(ConnectionString)) diff --git a/src/Infrastructure.EntityFramework/Repositories/OrganizationConnectionRepository.cs b/src/Infrastructure.EntityFramework/Repositories/OrganizationConnectionRepository.cs index 298e28e02..ad8359758 100644 --- a/src/Infrastructure.EntityFramework/Repositories/OrganizationConnectionRepository.cs +++ b/src/Infrastructure.EntityFramework/Repositories/OrganizationConnectionRepository.cs @@ -15,6 +15,17 @@ public class OrganizationConnectionRepository : Repository 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(connection); + } + } + public async Task> GetByOrganizationIdTypeAsync(Guid organizationId, OrganizationConnectionType type) { using (var scope = ServiceScopeFactory.CreateScope()) diff --git a/src/Sql/dbo/Stored Procedures/OrganizationConnection_ReadByIdOrganizationId.sql b/src/Sql/dbo/Stored Procedures/OrganizationConnection_ReadByIdOrganizationId.sql new file mode 100644 index 000000000..42c41f7e1 --- /dev/null +++ b/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 diff --git a/test/Api.Test/Controllers/OrganizationConnectionsControllerTests.cs b/test/Api.Test/Controllers/OrganizationConnectionsControllerTests.cs index c95d6861f..bbeb50b18 100644 --- a/test/Api.Test/Controllers/OrganizationConnectionsControllerTests.cs +++ b/test/Api.Test/Controllers/OrganizationConnectionsControllerTests.cs @@ -143,10 +143,10 @@ public class OrganizationConnectionsControllerTests public async Task UpdateConnection_RequiresOwnerPermissions(SutProvider sutProvider) { sutProvider.GetDependency() - .GetByIdAsync(Arg.Any()) + .GetByIdOrganizationIdAsync(Arg.Any(), Arg.Any()) .Returns(new OrganizationConnection()); - var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.UpdateConnection(default, null)); + var exception = await Assert.ThrowsAsync(() => 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().OrganizationOwner(typedModel.OrganizationId).Returns(true); var orgConnectionRepository = sutProvider.GetDependency(); - 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(() => sutProvider.Sut.UpdateConnection(existing1.Id, typedModel)); @@ -186,7 +186,7 @@ public class OrganizationConnectionsControllerTests sutProvider.GetDependency().OrganizationOwner(typedModel.OrganizationId).Returns(true); sutProvider.GetDependency() - .GetByIdAsync(existing1.Id) + .GetByIdOrganizationIdAsync(existing1.Id, existing1.OrganizationId) .Returns(existing1); sutProvider.GetDependency().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(updated); @@ -224,7 +225,7 @@ public class OrganizationConnectionsControllerTests .UpdateAsync(default) .ReturnsForAnyArgs(updated); sutProvider.GetDependency() - .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(updated); sutProvider.GetDependency().SelfHosted.Returns(true); @@ -275,7 +277,7 @@ public class OrganizationConnectionsControllerTests .UpdateAsync(default) .ReturnsForAnyArgs(updated); sutProvider.GetDependency() - .GetByIdAsync(existing.Id) + .GetByIdOrganizationIdAsync(existing.Id, existing.OrganizationId) .Returns(existing); OrganizationLicense organizationLicense = new OrganizationLicense(); diff --git a/util/Migrator/DbScripts/2023-10-05_00_OrgConnectionsReadByIdOrgId.sql b/util/Migrator/DbScripts/2023-10-05_00_OrgConnectionsReadByIdOrgId.sql new file mode 100644 index 000000000..34c950308 --- /dev/null +++ b/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