From b24c25ff144d63afca742da4e66265b6c2d4c574 Mon Sep 17 00:00:00 2001 From: Jonas Hendrickx Date: Wed, 26 Mar 2025 15:01:32 +0100 Subject: [PATCH] Wrong business logic checking for invalid permissions. --- .../CreateAdminInitiatedSponsorshipHandler.cs | 6 +- .../CreateSponsorshipCommandTests.cs | 12 +--- ...teAdminInitiatedSponsorshipHandlerTests.cs | 56 ++++++++++++++++--- 3 files changed, 53 insertions(+), 21 deletions(-) diff --git a/src/Core/OrganizationFeatures/OrganizationSponsorships/FamiliesForEnterprise/SponsorshipCreation/CreateAdminInitiatedSponsorshipHandler.cs b/src/Core/OrganizationFeatures/OrganizationSponsorships/FamiliesForEnterprise/SponsorshipCreation/CreateAdminInitiatedSponsorshipHandler.cs index f36a3719f1..2ae1132b16 100644 --- a/src/Core/OrganizationFeatures/OrganizationSponsorships/FamiliesForEnterprise/SponsorshipCreation/CreateAdminInitiatedSponsorshipHandler.cs +++ b/src/Core/OrganizationFeatures/OrganizationSponsorships/FamiliesForEnterprise/SponsorshipCreation/CreateAdminInitiatedSponsorshipHandler.cs @@ -28,10 +28,10 @@ public class CreateAdminInitiatedSponsorshipHandler( OrganizationUserType[] allowedUserTypes = [ OrganizationUserType.Admin, - OrganizationUserType.Owner, - OrganizationUserType.Custom + OrganizationUserType.Owner ]; - if (!organization.Permissions.ManageUsers || allowedUserTypes.All(x => x != organization.Type)) + + if (!organization.Permissions.ManageUsers && allowedUserTypes.All(x => x != organization.Type)) { throw new UnauthorizedAccessException("You do not have permissions to send sponsorships on behalf of the organization."); } diff --git a/test/Core.Test/OrganizationFeatures/OrganizationSponsorships/FamiliesForEnterprise/CreateSponsorshipCommandTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationSponsorships/FamiliesForEnterprise/CreateSponsorshipCommandTests.cs index d45de832ff..1e16564eee 100644 --- a/test/Core.Test/OrganizationFeatures/OrganizationSponsorships/FamiliesForEnterprise/CreateSponsorshipCommandTests.cs +++ b/test/Core.Test/OrganizationFeatures/OrganizationSponsorships/FamiliesForEnterprise/CreateSponsorshipCommandTests.cs @@ -211,7 +211,7 @@ public class CreateSponsorshipCommandTests : FamiliesForEnterpriseTestsBase { Id = sponsoringOrg.Id, Permissions = new Permissions(), - Type = OrganizationUserType.Admin + Type = OrganizationUserType.Custom } ]); @@ -225,6 +225,7 @@ public class CreateSponsorshipCommandTests : FamiliesForEnterpriseTestsBase [Theory] [BitAutoData(OrganizationUserType.User)] + [BitAutoData(OrganizationUserType.Custom)] public async Task CreateSponsorship_InvalidUserType_ThrowsUnauthorizedException( OrganizationUserType organizationUserType, Organization sponsoringOrg, OrganizationUser sponsoringOrgUser, User user, string sponsoredEmail, @@ -248,10 +249,6 @@ public class CreateSponsorshipCommandTests : FamiliesForEnterpriseTestsBase new() { Id = sponsoringOrg.Id, - Permissions = new Permissions - { - ManageUsers = true, - }, Type = organizationUserType } ]); @@ -266,7 +263,6 @@ public class CreateSponsorshipCommandTests : FamiliesForEnterpriseTestsBase [Theory] [BitAutoData(OrganizationUserType.Admin)] - [BitAutoData(OrganizationUserType.Custom)] [BitAutoData(OrganizationUserType.Owner)] public async Task CreateSponsorship_CreatesAdminInitiatedSponsorship( OrganizationUserType organizationUserType, @@ -291,10 +287,6 @@ public class CreateSponsorshipCommandTests : FamiliesForEnterpriseTestsBase new() { Id = sponsoringOrg.Id, - Permissions = new Permissions - { - ManageUsers = true, - }, Type = organizationUserType } ]); diff --git a/test/Core.Test/OrganizationFeatures/OrganizationSponsorships/FamiliesForEnterprise/SponsorshipCreation/CreateAdminInitiatedSponsorshipHandlerTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationSponsorships/FamiliesForEnterprise/SponsorshipCreation/CreateAdminInitiatedSponsorshipHandlerTests.cs index ee6e64535a..4a921bf652 100644 --- a/test/Core.Test/OrganizationFeatures/OrganizationSponsorships/FamiliesForEnterprise/SponsorshipCreation/CreateAdminInitiatedSponsorshipHandlerTests.cs +++ b/test/Core.Test/OrganizationFeatures/OrganizationSponsorships/FamiliesForEnterprise/SponsorshipCreation/CreateAdminInitiatedSponsorshipHandlerTests.cs @@ -19,8 +19,10 @@ namespace Bit.Core.Test.OrganizationFeatures.OrganizationSponsorships.FamiliesFo public class CreateAdminInitiatedSponsorshipHandlerTests : FamiliesForEnterpriseTestsBase { [Theory] - [BitAutoData] + [BitAutoData(OrganizationUserType.User)] + [BitAutoData(OrganizationUserType.Custom)] public async Task HandleAsync_MissingManageUsersPermission_ThrowsUnauthorizedException( + OrganizationUserType organizationUserType, Organization sponsoringOrg, OrganizationUser sponsoringOrgUser, string sponsoredEmail, string friendlyName, Guid currentUserId, SutProvider sutProvider) { @@ -37,7 +39,7 @@ public class CreateAdminInitiatedSponsorshipHandlerTests : FamiliesForEnterprise { Id = sponsoringOrg.Id, Permissions = new Permissions(), - Type = OrganizationUserType.Admin + Type = organizationUserType } ]); @@ -52,6 +54,7 @@ public class CreateAdminInitiatedSponsorshipHandlerTests : FamiliesForEnterprise [Theory] [BitAutoData(OrganizationUserType.User)] + [BitAutoData(OrganizationUserType.Custom)] public async Task HandleAsync_InvalidUserType_ThrowsUnauthorizedException( OrganizationUserType organizationUserType, Organization sponsoringOrg, OrganizationUser sponsoringOrgUser, string sponsoredEmail, @@ -72,7 +75,7 @@ public class CreateAdminInitiatedSponsorshipHandlerTests : FamiliesForEnterprise Id = sponsoringOrg.Id, Permissions = new Permissions { - ManageUsers = true, + ManageUsers = false, }, Type = organizationUserType } @@ -89,7 +92,6 @@ public class CreateAdminInitiatedSponsorshipHandlerTests : FamiliesForEnterprise [Theory] [BitAutoData(OrganizationUserType.Admin)] - [BitAutoData(OrganizationUserType.Custom)] [BitAutoData(OrganizationUserType.Owner)] public async Task HandleAsync_CreatesAdminInitiatedSponsorship( OrganizationUserType organizationUserType, Organization sponsoringOrg, OrganizationUser sponsoringOrgUser, @@ -108,10 +110,6 @@ public class CreateAdminInitiatedSponsorshipHandlerTests : FamiliesForEnterprise new() { Id = sponsoringOrg.Id, - Permissions = new Permissions - { - ManageUsers = true, - }, Type = organizationUserType } ]); @@ -130,6 +128,48 @@ public class CreateAdminInitiatedSponsorshipHandlerTests : FamiliesForEnterprise AssertHelper.AssertPropertyEqual(expectedSponsorship, actual); } + [Theory] + [BitAutoData(OrganizationUserType.User)] + [BitAutoData(OrganizationUserType.Custom)] + public async Task HandleAsync_CreatesAdminInitiatedSponsorshipWithValidPermissionsButInvalidOrganizationUserType( + OrganizationUserType organizationUserType, Organization sponsoringOrg, OrganizationUser sponsoringOrgUser, + string sponsoredEmail, string friendlyName, Guid currentUserId, string notes, + SutProvider sutProvider) + { + sponsoringOrg.PlanType = PlanType.EnterpriseAnnually; + sponsoringOrgUser.Status = OrganizationUserStatusType.Confirmed; + + sutProvider.GetDependency() + .IsEnabled(Arg.Is(p => p == FeatureFlagKeys.PM17772_AdminInitiatedSponsorships)) + .Returns(true); + + sutProvider.GetDependency().UserId.Returns(currentUserId); + sutProvider.GetDependency().Organizations.Returns([ + new() + { + Id = sponsoringOrg.Id, + Type = organizationUserType, + Permissions = + { + ManageUsers = true + } + } + ]); + + var request = new CreateSponsorshipRequest(sponsoringOrg, sponsoringOrgUser, + PlanSponsorshipType.FamiliesForEnterprise, sponsoredEmail, friendlyName, notes); + + var actual = await sutProvider.Sut.HandleAsync(request); + + var expectedSponsorship = new OrganizationSponsorship + { + IsAdminInitiated = true, + Notes = notes + }; + + AssertHelper.AssertPropertyEqual(expectedSponsorship, actual); + } + [Theory] [BitAutoData] public async Task HandleAsync_ThrowsBadRequestException_WhenFeatureFlagIsDisabled(