From 9a11bc0905a6ae615a875409d5a85b9babe1feab Mon Sep 17 00:00:00 2001 From: Cy Okeke Date: Wed, 15 Jul 2026 17:40:49 +0100 Subject: [PATCH] [PM-39894] fix: Exclude provider-linked organizations from add-existing-organization flow The Provider Portal's "Add existing organization" flow never checked whether the target organization already had a ProviderOrganization row, so a reseller-linked org (Status stays Created) could be absorbed into an MSP's consolidated billing, leaving the org linked to two providers. Two layers: - Organization_ReadAddableToProviderByUserId (Dapper sproc + EF twin) now excludes any org with an existing ProviderOrganization row, so linked orgs never appear in the picker and the endpoint's eligibility re-check returns 404 for them. - ProviderBillingService.AddExistingOrganization guards first with the same check as the legacy ProviderService.AddOrganization path and throws ConflictException (409) before any Stripe or repository side effects, covering the check-then-act race and direct service callers. --- .../Services/ProviderBillingService.cs | 8 + .../Services/ProviderBillingServiceTests.cs | 41 +++++ .../Repositories/OrganizationRepository.cs | 3 +- ...nization_ReadAddableToProviderByUserId.sql | 8 +- .../OrganizationRepositoryTests.cs | 155 ++++++++++++++++++ ...AlreadyLinkedOrgsFromAddableToProvider.sql | 30 ++++ 6 files changed, 243 insertions(+), 2 deletions(-) create mode 100644 util/Migrator/DbScripts/2026-07-15_00_ExcludeAlreadyLinkedOrgsFromAddableToProvider.sql diff --git a/bitwarden_license/src/Commercial.Core/Billing/Providers/Services/ProviderBillingService.cs b/bitwarden_license/src/Commercial.Core/Billing/Providers/Services/ProviderBillingService.cs index 4b3e194b5c06..b949c67495a1 100644 --- a/bitwarden_license/src/Commercial.Core/Billing/Providers/Services/ProviderBillingService.cs +++ b/bitwarden_license/src/Commercial.Core/Billing/Providers/Services/ProviderBillingService.cs @@ -59,6 +59,14 @@ public async Task AddExistingOrganization( Organization organization, string key) { + var existingProviderOrganization = + await providerOrganizationRepository.GetByOrganizationId(organization.Id); + + if (existingProviderOrganization != null) + { + throw new ConflictException("Organization already belongs to a provider."); + } + await priceIncreaseScheduler.Release( organization.GatewayCustomerId, organization.GatewaySubscriptionId, diff --git a/bitwarden_license/test/Commercial.Core.Test/Billing/Providers/Services/ProviderBillingServiceTests.cs b/bitwarden_license/test/Commercial.Core.Test/Billing/Providers/Services/ProviderBillingServiceTests.cs index 303c28aa9a11..633529cac73e 100644 --- a/bitwarden_license/test/Commercial.Core.Test/Billing/Providers/Services/ProviderBillingServiceTests.cs +++ b/bitwarden_license/test/Commercial.Core.Test/Billing/Providers/Services/ProviderBillingServiceTests.cs @@ -2679,6 +2679,47 @@ await eventService.Received(0).LogProviderOrganizationEventAsync( Arg.Any(), Arg.Any()); } + [Theory, BitAutoData] + public async Task AddExistingOrganization_OrgAlreadyBelongsToProvider_ThrowsAndShortCircuits( + Provider provider, + Organization organization, + string key, + SutProvider sutProvider) + { + // Arrange — the organization already has a ProviderOrganization row (PM-39894). + // The guard must fire before any billing side effects (Release/Stripe) or repo writes. + provider.Type = ProviderType.Msp; + + var providerOrganizationRepository = sutProvider.GetDependency(); + providerOrganizationRepository + .GetByOrganizationId(organization.Id) + .Returns(new ProviderOrganization { OrganizationId = organization.Id }); + + var priceIncreaseScheduler = sutProvider.GetDependency(); + var stripeAdapter = sutProvider.GetDependency(); + var organizationRepository = sutProvider.GetDependency(); + var eventService = sutProvider.GetDependency(); + + // Act + var exception = await Assert.ThrowsAsync(() => + sutProvider.Sut.AddExistingOrganization(provider, organization, key)); + + // Assert — exact message and nothing downstream of the guard fired (representative calls asserted). + Assert.Equal("Organization already belongs to a provider.", exception.Message); + await providerOrganizationRepository.Received(1) + .GetByOrganizationId(organization.Id); + await priceIncreaseScheduler.Received(0) + .Release(Arg.Any(), Arg.Any(), Arg.Any()); + await stripeAdapter.Received(0) + .UpdateSubscriptionAsync(Arg.Any(), Arg.Any()); + await stripeAdapter.Received(0) + .CancelSubscriptionAsync(Arg.Any(), Arg.Any()); + await organizationRepository.Received(0).ReplaceAsync(Arg.Any()); + await providerOrganizationRepository.Received(0).CreateAsync(Arg.Any()); + await eventService.Received(0).LogProviderOrganizationEventAsync( + Arg.Any(), Arg.Any()); + } + [Theory, BitAutoData] public async Task AddExistingOrganization_StripeCancelSucceeds_OnlyWhenReleasePrecedesIt( Provider provider, diff --git a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs index 44d3efc0f4d2..79e814fb10a1 100644 --- a/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs +++ b/src/Infrastructure.EntityFramework/AdminConsole/Repositories/OrganizationRepository.cs @@ -423,7 +423,8 @@ join organization in dbContext.Organizations on organizationUser.OrganizationId organization.Seats > 0 && organization.Status == OrganizationStatusType.Created && !organization.UseSecretsManager && - planTypes.Contains(organization.PlanType) + planTypes.Contains(organization.PlanType) && + !dbContext.ProviderOrganizations.Any(po => po.OrganizationId == organization.Id) select organization; return await query.ToArrayAsync(); diff --git a/src/Sql/dbo/Stored Procedures/Organization_ReadAddableToProviderByUserId.sql b/src/Sql/dbo/Stored Procedures/Organization_ReadAddableToProviderByUserId.sql index e11109ae1041..af02a6a93970 100644 --- a/src/Sql/dbo/Stored Procedures/Organization_ReadAddableToProviderByUserId.sql +++ b/src/Sql/dbo/Stored Procedures/Organization_ReadAddableToProviderByUserId.sql @@ -19,5 +19,11 @@ BEGIN -- All Teams & Enterprise for MSP (@ProviderType = 0 AND O.[PlanType] IN (2, 3, 4, 5, 8, 9, 10, 11, 12, 13, 14, 15, 17, 18, 19, 20) OR -- All Enterprise for MOE - @ProviderType = 2 AND O.[PlanType] IN (4, 5, 10, 11, 14, 15, 19, 20)); + @ProviderType = 2 AND O.[PlanType] IN (4, 5, 10, 11, 14, 15, 19, 20)) AND + -- Exclude organizations that already belong to a provider (PM-39894) + NOT EXISTS ( + SELECT 1 + FROM [dbo].[ProviderOrganization] PO + WHERE PO.[OrganizationId] = O.[Id] + ); END diff --git a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationRepositoryTests.cs b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationRepositoryTests.cs index d1c807789106..dcc41b990833 100644 --- a/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationRepositoryTests.cs +++ b/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/OrganizationRepositoryTests.cs @@ -1,5 +1,9 @@ using System.Data.Common; using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.Entities.Provider; +using Bit.Core.AdminConsole.Enums.Provider; +using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Billing.Enums; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Repositories; @@ -453,4 +457,155 @@ public async Task GetAbilityAsync_WithNonExistentOrganization_ReturnsNull( return (user, organization, organizationUser); } + + [Theory, DatabaseData] + public async Task GetAddableToProviderByUserIdAsync_StandaloneOrg_Included( + IUserRepository userRepository, + IOrganizationRepository organizationRepository, + IOrganizationUserRepository organizationUserRepository) + { + // Arrange — an org meeting all addable criteria with no ProviderOrganization link. + var user = await userRepository.CreateTestUserAsync(); + var organization = await CreateAddableOrganizationAsync(organizationRepository); + await organizationUserRepository.CreateTestOrganizationUserAsync(organization, user); + + // Act + var result = await organizationRepository.GetAddableToProviderByUserIdAsync(user.Id, ProviderType.Msp); + + // Assert + Assert.Contains(result, o => o.Id == organization.Id); + } + + [Theory, DatabaseData] + public async Task GetAddableToProviderByUserIdAsync_ResellerLinkedOrg_Excluded( + IUserRepository userRepository, + IOrganizationRepository organizationRepository, + IOrganizationUserRepository organizationUserRepository, + IProviderRepository providerRepository, + IProviderOrganizationRepository providerOrganizationRepository) + { + // Arrange — the regression PM-39894 addresses: a reseller-linked org keeps + // Status = Created, so it looks addable, but it already has a ProviderOrganization row. + var user = await userRepository.CreateTestUserAsync(); + var organization = await CreateAddableOrganizationAsync(organizationRepository); + await organizationUserRepository.CreateTestOrganizationUserAsync(organization, user); + + var provider = await providerRepository.CreateAsync(new Provider + { + Name = $"Reseller {CombGuid.Generate()}", + Type = ProviderType.Reseller, + Status = ProviderStatusType.Created, + Enabled = true, + BillingEmail = "reseller@example.com" + }); + await providerOrganizationRepository.CreateAsync(new ProviderOrganization + { + ProviderId = provider.Id, + OrganizationId = organization.Id + }); + + // Act + var result = await organizationRepository.GetAddableToProviderByUserIdAsync(user.Id, ProviderType.Msp); + + // Assert + Assert.DoesNotContain(result, o => o.Id == organization.Id); + } + + [Theory, DatabaseData] + public async Task GetAddableToProviderByUserIdAsync_MspLinkedOrg_Excluded( + IUserRepository userRepository, + IOrganizationRepository organizationRepository, + IOrganizationUserRepository organizationUserRepository, + IProviderRepository providerRepository, + IProviderOrganizationRepository providerOrganizationRepository) + { + // Arrange — an org already linked to an MSP provider must not be addable again. + var user = await userRepository.CreateTestUserAsync(); + var organization = await CreateAddableOrganizationAsync(organizationRepository); + await organizationUserRepository.CreateTestOrganizationUserAsync(organization, user); + + var provider = await providerRepository.CreateAsync(new Provider + { + Name = $"MSP {CombGuid.Generate()}", + Type = ProviderType.Msp, + Status = ProviderStatusType.Created, + Enabled = true, + BillingEmail = "msp@example.com" + }); + await providerOrganizationRepository.CreateAsync(new ProviderOrganization + { + ProviderId = provider.Id, + OrganizationId = organization.Id + }); + + // Act + var result = await organizationRepository.GetAddableToProviderByUserIdAsync(user.Id, ProviderType.Msp); + + // Assert + Assert.DoesNotContain(result, o => o.Id == organization.Id); + } + + [Theory, DatabaseData] + public async Task GetAddableToProviderByUserIdAsync_LinkedOrg_ExcludedForBusinessUnit( + IUserRepository userRepository, + IOrganizationRepository organizationRepository, + IOrganizationUserRepository organizationUserRepository, + IProviderRepository providerRepository, + IProviderOrganizationRepository providerOrganizationRepository) + { + // Arrange — the provider-link exclusion must apply to the BusinessUnit branch of the + // query too, not just MSP; this pins the predicate's placement outside the OR'd + // plan-type group. + var user = await userRepository.CreateTestUserAsync(); + var organization = await CreateAddableOrganizationAsync(organizationRepository); + await organizationUserRepository.CreateTestOrganizationUserAsync(organization, user); + + var provider = await providerRepository.CreateAsync(new Provider + { + Name = $"Reseller {CombGuid.Generate()}", + Type = ProviderType.Reseller, + Status = ProviderStatusType.Created, + Enabled = true, + BillingEmail = "reseller@example.com" + }); + await providerOrganizationRepository.CreateAsync(new ProviderOrganization + { + ProviderId = provider.Id, + OrganizationId = organization.Id + }); + + // Act + var result = await organizationRepository.GetAddableToProviderByUserIdAsync( + user.Id, ProviderType.BusinessUnit); + + // Assert + Assert.DoesNotContain(result, o => o.Id == organization.Id); + } + + /// + /// Builds an Organization inline that satisfies every predicate of the addable-to-provider query. + /// The shared CreateTestOrganizationAsync helper defaults to Status = Managed and + /// UseSecretsManager = true, both of which the query excludes, so we construct the org directly. + /// + private static Task CreateAddableOrganizationAsync( + IOrganizationRepository organizationRepository) + { + var id = CombGuid.Generate(); + return organizationRepository.CreateAsync(new Organization + { + Name = $"addable-{id}", + BillingEmail = $"billing-{id}@example.com", + Plan = "Enterprise (Annually)", + PlanType = PlanType.EnterpriseAnnually, + Status = OrganizationStatusType.Created, + Enabled = true, + UseSecretsManager = false, + Seats = 5, + Gateway = GatewayType.Stripe, + GatewayCustomerId = $"cus_{id}", + GatewaySubscriptionId = $"sub_{id}", + PublicKey = "test-public-key", + PrivateKey = "test-private-key" + }); + } } diff --git a/util/Migrator/DbScripts/2026-07-15_00_ExcludeAlreadyLinkedOrgsFromAddableToProvider.sql b/util/Migrator/DbScripts/2026-07-15_00_ExcludeAlreadyLinkedOrgsFromAddableToProvider.sql new file mode 100644 index 000000000000..bc202ad92b33 --- /dev/null +++ b/util/Migrator/DbScripts/2026-07-15_00_ExcludeAlreadyLinkedOrgsFromAddableToProvider.sql @@ -0,0 +1,30 @@ +CREATE OR ALTER PROCEDURE [dbo].[Organization_ReadAddableToProviderByUserId] + @UserId UNIQUEIDENTIFIER, + @ProviderType TINYINT +AS +BEGIN + SET NOCOUNT ON + SELECT O.* FROM [dbo].[OrganizationUser] AS OU + JOIN [dbo].[Organization] AS O ON O.[Id] = OU.[OrganizationId] + WHERE + OU.[UserId] = @UserId AND + OU.[Type] = 0 AND + OU.[Status] = 2 AND + O.[Enabled] = 1 AND + O.[GatewayCustomerId] IS NOT NULL AND + O.[GatewaySubscriptionId] IS NOT NULL AND + O.[Seats] > 0 AND + O.[Status] = 1 AND + O.[UseSecretsManager] = 0 AND + -- All Teams & Enterprise for MSP + (@ProviderType = 0 AND O.[PlanType] IN (2, 3, 4, 5, 8, 9, 10, 11, 12, 13, 14, 15, 17, 18, 19, 20) OR + -- All Enterprise for MOE + @ProviderType = 2 AND O.[PlanType] IN (4, 5, 10, 11, 14, 15, 19, 20)) AND + -- Exclude organizations that already belong to a provider (PM-39894) + NOT EXISTS ( + SELECT 1 + FROM [dbo].[ProviderOrganization] PO + WHERE PO.[OrganizationId] = O.[Id] + ); +END +GO