From 8f0115e62f3987ecd90a7ed5318e43a9d6e8643b Mon Sep 17 00:00:00 2001 From: Matt Gibson Date: Mon, 25 Oct 2021 10:19:37 -0500 Subject: [PATCH] Check canScale when scaling for sso (#1661) * Check canScale when scaling for sso * PR review Use AutoAddSeats to add seats in a consistent way. This requires moving user check out of that method. * User logic moved out of method --- .../src/Sso/Controllers/AccountController.cs | 7 +------ src/Core/Services/IOrganizationService.cs | 1 + .../Implementations/OrganizationService.cs | 20 +++++++++---------- .../Services/OrganizationServiceTests.cs | 17 ++-------------- 4 files changed, 14 insertions(+), 31 deletions(-) diff --git a/bitwarden_license/src/Sso/Controllers/AccountController.cs b/bitwarden_license/src/Sso/Controllers/AccountController.cs index 4fc77bd4c..d39a1d930 100644 --- a/bitwarden_license/src/Sso/Controllers/AccountController.cs +++ b/bitwarden_license/src/Sso/Controllers/AccountController.cs @@ -483,12 +483,7 @@ namespace Bit.Sso.Controllers throw new Exception("Cannot autoscale on self-hosted instance."); } - var paymentIntentClientSecret = await _organizationService.AdjustSeatsAsync(orgId, 1, prorationDate); - organization = await _organizationRepository.GetByIdAsync(orgId); - if (!string.IsNullOrEmpty(paymentIntentClientSecret)) - { - throw new Exception("Stripe payment required client-side confirmation."); - } + await _organizationService.AutoAddSeatsAsync(organization, 1, prorationDate); } catch (Exception e) { diff --git a/src/Core/Services/IOrganizationService.cs b/src/Core/Services/IOrganizationService.cs index de1185456..c5705bc57 100644 --- a/src/Core/Services/IOrganizationService.cs +++ b/src/Core/Services/IOrganizationService.cs @@ -18,6 +18,7 @@ namespace Bit.Core.Services Task> UpgradePlanAsync(Guid organizationId, OrganizationUpgrade upgrade); Task AdjustStorageAsync(Guid organizationId, short storageAdjustmentGb); Task UpdateSubscription(Guid organizationId, int seatAdjustment, int? maxAutoscaleSeats); + Task AutoAddSeatsAsync(Organization organization, int seatsToAdd, DateTime? prorationDate = null); Task AdjustSeatsAsync(Guid organizationId, int seatAdjustment, DateTime? prorationDate = null); Task VerifyBankAsync(Guid organizationId, int amount1, int amount2); Task> SignUpAsync(OrganizationSignup organizationSignup, bool provider = false); diff --git a/src/Core/Services/Implementations/OrganizationService.cs b/src/Core/Services/Implementations/OrganizationService.cs index 7da711539..aeba331c8 100644 --- a/src/Core/Services/Implementations/OrganizationService.cs +++ b/src/Core/Services/Implementations/OrganizationService.cs @@ -1074,7 +1074,7 @@ namespace Bit.Core.Services if (newSeatsRequired > 0) { - var (canScale, failureReason) = await CanScaleAsync(organization, newSeatsRequired); + var (canScale, failureReason) = CanScale(organization, newSeatsRequired); if (!canScale) { throw new BadRequestException(failureReason); @@ -1160,6 +1160,11 @@ namespace Bit.Core.Services await _organizationUserRepository.CreateAsync(orgUser, collections); } + if (!await _currentContext.ManageUsers(organization.Id)) + { + throw new BadRequestException("Cannot add seats. Cannot manage organization users."); + } + await AutoAddSeatsAsync(organization, newSeatsRequired, prorationDate); await SendInvitesAsync(orgUsers, organization); await _eventService.LogOrganizationUserEventsAsync(events); @@ -1454,7 +1459,8 @@ namespace Bit.Core.Services return result; } - internal async Task<(bool canScale, string failureReason)> CanScaleAsync(Organization organization, int seatsToAdd) + internal (bool canScale, string failureReason) CanScale(Organization organization, + int seatsToAdd) { var failureReason = ""; if (_globalSettings.SelfHosted) @@ -1463,12 +1469,6 @@ namespace Bit.Core.Services return (false, failureReason); } - if (!await _currentContext.ManageUsers(organization.Id)) - { - failureReason = "Cannot manage organization users."; - return (false, failureReason); - } - if (seatsToAdd < 1) { return (true, failureReason); @@ -1484,14 +1484,14 @@ namespace Bit.Core.Services return (true, failureReason); } - private async Task AutoAddSeatsAsync(Organization organization, int seatsToAdd, DateTime? prorationDate = null) + public async Task AutoAddSeatsAsync(Organization organization, int seatsToAdd, DateTime? prorationDate = null) { if (seatsToAdd < 1 || !organization.Seats.HasValue) { return; } - var (canScale, failureMessage) = await CanScaleAsync(organization, seatsToAdd); + var (canScale, failureMessage) = CanScale(organization, seatsToAdd); if (!canScale) { throw new BadRequestException(failureMessage); diff --git a/test/Core.Test/Services/OrganizationServiceTests.cs b/test/Core.Test/Services/OrganizationServiceTests.cs index 1aa369c47..c9e74b93f 100644 --- a/test/Core.Test/Services/OrganizationServiceTests.cs +++ b/test/Core.Test/Services/OrganizationServiceTests.cs @@ -868,7 +868,7 @@ namespace Bit.Core.Test.Services organization.MaxAutoscaleSeats = maxAutoscaleSeats; sutProvider.GetDependency().ManageUsers(organization.Id).Returns(true); - var (result, failureMessage) = await sutProvider.Sut.CanScaleAsync(organization, seatsToAdd); + var (result, failureMessage) = sutProvider.Sut.CanScale(organization, seatsToAdd); if (expectedFailureMessage == string.Empty) { @@ -886,23 +886,10 @@ namespace Bit.Core.Test.Services SutProvider sutProvider) { sutProvider.GetDependency().SelfHosted.Returns(true); - var (result, failureMessage) = await sutProvider.Sut.CanScaleAsync(organization, 10); + var (result, failureMessage) = sutProvider.Sut.CanScale(organization, 10); Assert.False(result); Assert.Contains("Cannot autoscale on self-hosted instance", failureMessage); } - - [Theory, PaidOrganizationAutoData] - public async Task CanScale_FailsIfCannotManageUsers(Organization organization, - SutProvider sutProvider) - { - organization.MaxAutoscaleSeats = null; - sutProvider.GetDependency().ManageUsers(organization.Id).Returns(false); - - var (result, failureMessage) = await sutProvider.Sut.CanScaleAsync(organization, 10); - - Assert.False(result); - Assert.Contains("Cannot manage organization users", failureMessage); - } } }