diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IUpdateOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IUpdateOrganizationUserCommand.cs index c7298e1cd9..0cd5a3295f 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IUpdateOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Interfaces/IUpdateOrganizationUserCommand.cs @@ -6,6 +6,6 @@ namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interface public interface IUpdateOrganizationUserCommand { - Task UpdateUserAsync(OrganizationUser user, Guid? savingUserId, + Task UpdateUserAsync(OrganizationUser organizationUser, Guid? savingUserId, List? collectionAccess, IEnumerable? groupAccess); } diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs index c5a4b3da1d..3dd55f9893 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommand.cs @@ -1,6 +1,7 @@ #nullable enable using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Billing.Enums; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; @@ -49,48 +50,64 @@ public class UpdateOrganizationUserCommand : IUpdateOrganizationUserCommand /// /// Update an organization user. /// - /// The modified user to save. + /// The modified organization user to save. /// The userId of the currently logged in user who is making the change. /// The user's updated collection access. If set to null, this removes all collection access. /// The user's updated group access. If set to null, groups are not updated. /// - public async Task UpdateUserAsync(OrganizationUser user, Guid? savingUserId, + public async Task UpdateUserAsync(OrganizationUser organizationUser, Guid? savingUserId, List? collectionAccess, IEnumerable? groupAccess) { // Avoid multiple enumeration collectionAccess = collectionAccess?.ToList(); groupAccess = groupAccess?.ToList(); - if (user.Id.Equals(default(Guid))) + if (organizationUser.Id.Equals(default(Guid))) { throw new BadRequestException("Invite the user first."); } - var originalUser = await _organizationUserRepository.GetByIdAsync(user.Id); - if (originalUser == null || user.OrganizationId != originalUser.OrganizationId) + var originalOrganizationUser = await _organizationUserRepository.GetByIdAsync(organizationUser.Id); + if (originalOrganizationUser == null || organizationUser.OrganizationId != originalOrganizationUser.OrganizationId) { throw new NotFoundException(); } + var organization = await _organizationRepository.GetByIdAsync(organizationUser.OrganizationId); + if (organization == null) + { + throw new NotFoundException(); + } + + if (organizationUser.UserId.HasValue && organization.PlanType == PlanType.Free && organizationUser.Type is OrganizationUserType.Admin or OrganizationUserType.Owner) + { + // Since free organizations only supports a few users there is not much point in avoiding N+1 queries for this. + var adminCount = await _organizationUserRepository.GetCountByFreeOrganizationAdminUserAsync(organizationUser.UserId.Value); + if (adminCount > 0) + { + throw new BadRequestException("User can only be an admin of one free organization."); + } + } + if (collectionAccess?.Any() == true) { - await ValidateCollectionAccessAsync(originalUser, collectionAccess.ToList()); + await ValidateCollectionAccessAsync(originalOrganizationUser, collectionAccess.ToList()); } if (groupAccess?.Any() == true) { - await ValidateGroupAccessAsync(originalUser, groupAccess.ToList()); + await ValidateGroupAccessAsync(originalOrganizationUser, groupAccess.ToList()); } if (savingUserId.HasValue) { - await _organizationService.ValidateOrganizationUserUpdatePermissions(user.OrganizationId, user.Type, originalUser.Type, user.GetPermissions()); + await _organizationService.ValidateOrganizationUserUpdatePermissions(organizationUser.OrganizationId, organizationUser.Type, originalOrganizationUser.Type, organizationUser.GetPermissions()); } - await _organizationService.ValidateOrganizationCustomPermissionsEnabledAsync(user.OrganizationId, user.Type); + await _organizationService.ValidateOrganizationCustomPermissionsEnabledAsync(organizationUser.OrganizationId, organizationUser.Type); - if (user.Type != OrganizationUserType.Owner && - !await _hasConfirmedOwnersExceptQuery.HasConfirmedOwnersExceptAsync(user.OrganizationId, new[] { user.Id })) + if (organizationUser.Type != OrganizationUserType.Owner && + !await _hasConfirmedOwnersExceptQuery.HasConfirmedOwnersExceptAsync(organizationUser.OrganizationId, new[] { organizationUser.Id })) { throw new BadRequestException("Organization must have at least one confirmed owner."); } @@ -106,26 +123,25 @@ public class UpdateOrganizationUserCommand : IUpdateOrganizationUserCommand // Only autoscale (if required) after all validation has passed so that we know it's a valid request before // updating Stripe - if (!originalUser.AccessSecretsManager && user.AccessSecretsManager) + if (!originalOrganizationUser.AccessSecretsManager && organizationUser.AccessSecretsManager) { - var additionalSmSeatsRequired = await _countNewSmSeatsRequiredQuery.CountNewSmSeatsRequiredAsync(user.OrganizationId, 1); + var additionalSmSeatsRequired = await _countNewSmSeatsRequiredQuery.CountNewSmSeatsRequiredAsync(organizationUser.OrganizationId, 1); if (additionalSmSeatsRequired > 0) { - var organization = await _organizationRepository.GetByIdAsync(user.OrganizationId); var update = new SecretsManagerSubscriptionUpdate(organization, true) - .AdjustSeats(additionalSmSeatsRequired); + .AdjustSeats(additionalSmSeatsRequired); await _updateSecretsManagerSubscriptionCommand.UpdateSubscriptionAsync(update); } } - await _organizationUserRepository.ReplaceAsync(user, collectionAccess); + await _organizationUserRepository.ReplaceAsync(organizationUser, collectionAccess); if (groupAccess != null) { - await _organizationUserRepository.UpdateGroupsAsync(user.Id, groupAccess); + await _organizationUserRepository.UpdateGroupsAsync(organizationUser.Id, groupAccess); } - await _eventService.LogOrganizationUserEventAsync(user, EventType.OrganizationUser_Updated); + await _eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_Updated); } private async Task ValidateCollectionAccessAsync(OrganizationUser originalUser, diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs index eca4f449b0..2dda23481a 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/AcceptOrgUserCommandTests.cs @@ -2,6 +2,7 @@ using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Services; using Bit.Core.Auth.Models.Business.Tokenables; +using Bit.Core.Billing.Enums; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; @@ -182,6 +183,29 @@ public class AcceptOrgUserCommandTests exception.Message); } + [Theory] + [BitAutoData(OrganizationUserType.Admin)] + [BitAutoData(OrganizationUserType.Owner)] + public async Task AcceptOrgUser_AdminOfFreePlanTryingToJoinSecondFreeOrg_ThrowsBadRequest( + OrganizationUserType userType, + SutProvider sutProvider, + User user, Organization org, OrganizationUser orgUser, OrganizationUserUserDetails adminUserDetails) + { + // Arrange + SetupCommonAcceptOrgUserMocks(sutProvider, user, org, orgUser, adminUserDetails); + org.PlanType = PlanType.Free; + orgUser.Type = userType; + + sutProvider.GetDependency() + .GetCountByFreeOrganizationAdminUserAsync(user.Id) + .Returns(1); + + // Act & Assert + var exception = await Assert.ThrowsAsync(() => + sutProvider.Sut.AcceptOrgUserAsync(orgUser, user, _userService)); + + Assert.Equal("You can only be an admin of one free organization.", exception.Message); + } // AcceptOrgUserByOrgIdAsync tests -------------------------------------------------------------------------------- diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommandTests.cs index 73bf00474b..cd03f9583b 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserCommandTests.cs @@ -3,6 +3,7 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Billing.Enums; using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; @@ -144,6 +145,7 @@ public class UpdateOrganizationUserCommandTests newUserData.Id = oldUserData.Id; newUserData.UserId = oldUserData.UserId; newUserData.OrganizationId = savingUser.OrganizationId = oldUserData.OrganizationId = organization.Id; + newUserData.Type = OrganizationUserType.Admin; newUserData.Permissions = JsonSerializer.Serialize(permissions, new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase, @@ -159,6 +161,10 @@ public class UpdateOrganizationUserCommandTests .Returns(callInfo => callInfo.Arg>() .Select(guid => new Group { Id = guid, OrganizationId = oldUserData.OrganizationId }).ToList()); + sutProvider.GetDependency() + .GetCountByFreeOrganizationAdminUserAsync(newUserData.Id) + .Returns(0); + await sutProvider.Sut.UpdateUserAsync(newUserData, savingUser.UserId, collections, groups); var organizationService = sutProvider.GetDependency(); @@ -175,6 +181,31 @@ public class UpdateOrganizationUserCommandTests Arg.Is>(i => i.Contains(newUserData.Id))); } + [Theory] + [BitAutoData(OrganizationUserType.Admin)] + [BitAutoData(OrganizationUserType.Owner)] + public async Task UpdateUserAsync_WhenUpdatingUserToAdminOrOwner_WithUserAlreadyAdminOfAnotherFreeOrganization_Throws( + OrganizationUserType userType, + OrganizationUser oldUserData, + OrganizationUser newUserData, + Organization organization, + SutProvider sutProvider) + { + organization.PlanType = PlanType.Free; + newUserData.Type = userType; + + Setup(sutProvider, organization, newUserData, oldUserData); + + sutProvider.GetDependency() + .GetCountByFreeOrganizationAdminUserAsync(newUserData.UserId!.Value) + .Returns(1); + + // Assert + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.UpdateUserAsync(newUserData, null, null, null)); + Assert.Contains("User can only be an admin of one free organization.", exception.Message); + } + private void Setup(SutProvider sutProvider, Organization organization, OrganizationUser newUser, OrganizationUser oldUser) { diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/Organizations/OrganizationSignUp/CloudOrganizationSignUpCommandTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/Organizations/OrganizationSignUp/CloudOrganizationSignUpCommandTests.cs index a16b48240c..46b4f0b334 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/Organizations/OrganizationSignUp/CloudOrganizationSignUpCommandTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Organizations/OrganizationSignUp/CloudOrganizationSignUpCommandTests.cs @@ -242,4 +242,27 @@ public class CloudICloudOrganizationSignUpCommandTests () => sutProvider.Sut.SignUpOrganizationAsync(signup)); Assert.Contains("You can't subtract Machine Accounts!", exception.Message); } + + [Theory] + [BitAutoData] + public async Task SignUpAsync_Free_ExistingFreeOrgAdmin_ThrowsBadRequest( + SutProvider sutProvider) + { + // Arrange + var signup = new OrganizationSignup + { + Plan = PlanType.Free, + IsFromProvider = false, + Owner = new User { Id = Guid.NewGuid() } + }; + + sutProvider.GetDependency() + .GetCountByFreeOrganizationAdminUserAsync(signup.Owner.Id) + .Returns(1); + + // Act & Assert + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.SignUpOrganizationAsync(signup)); + Assert.Contains("You can only be an admin of one free organization.", exception.Message); + } }