From 7c3637c8ba78307c9dfe0039f3ff2ac83e6e8ce0 Mon Sep 17 00:00:00 2001
From: Thomas Rittson <31796059+eliykat@users.noreply.github.com>
Date: Fri, 23 Sep 2022 14:30:39 +1000
Subject: [PATCH] [EC-387] Don't count revoked users towards occupied seat
count (#2256)
Also autoscale seats when restoring user if required
---
.../src/Sso/Controllers/AccountController.cs | 4 +-
src/Admin/Models/OrganizationViewModel.cs | 4 +-
.../Organizations/_ViewInformation.cshtml | 2 +-
.../OrganizationUserUserDetails.cs | 8 +++
src/Core/Services/IOrganizationService.cs | 1 +
.../Implementations/OrganizationService.cs | 49 +++++++++++++------
.../Services/OrganizationServiceTests.cs | 2 +-
7 files changed, 49 insertions(+), 21 deletions(-)
diff --git a/bitwarden_license/src/Sso/Controllers/AccountController.cs b/bitwarden_license/src/Sso/Controllers/AccountController.cs
index 4ab9d7ef03..fdbcf0f0d1 100644
--- a/bitwarden_license/src/Sso/Controllers/AccountController.cs
+++ b/bitwarden_license/src/Sso/Controllers/AccountController.cs
@@ -483,9 +483,9 @@ public class AccountController : Controller
// Before any user creation - if Org User doesn't exist at this point - make sure there are enough seats to add one
if (orgUser == null && organization.Seats.HasValue)
{
- var userCount = await _organizationUserRepository.GetCountByOrganizationIdAsync(orgId);
+ var occupiedSeats = await _organizationService.GetOccupiedSeatCount(organization);
var initialSeatCount = organization.Seats.Value;
- var availableSeats = initialSeatCount - userCount;
+ var availableSeats = initialSeatCount - occupiedSeats;
var prorationDate = DateTime.UtcNow;
if (availableSeats < 1)
{
diff --git a/src/Admin/Models/OrganizationViewModel.cs b/src/Admin/Models/OrganizationViewModel.cs
index 5a487cd03e..31504f9371 100644
--- a/src/Admin/Models/OrganizationViewModel.cs
+++ b/src/Admin/Models/OrganizationViewModel.cs
@@ -18,7 +18,7 @@ public class OrganizationViewModel
UserInvitedCount = orgUsers.Count(u => u.Status == OrganizationUserStatusType.Invited);
UserAcceptedCount = orgUsers.Count(u => u.Status == OrganizationUserStatusType.Accepted);
UserConfirmedCount = orgUsers.Count(u => u.Status == OrganizationUserStatusType.Confirmed);
- UserCount = orgUsers.Count();
+ OccupiedSeatCount = orgUsers.Count(u => u.OccupiesOrganizationSeat);
CipherCount = ciphers.Count();
CollectionCount = collections.Count();
GroupCount = groups?.Count() ?? 0;
@@ -40,7 +40,7 @@ public class OrganizationViewModel
public int UserInvitedCount { get; set; }
public int UserConfirmedCount { get; set; }
public int UserAcceptedCount { get; set; }
- public int UserCount { get; set; }
+ public int OccupiedSeatCount { get; set; }
public int CipherCount { get; set; }
public int CollectionCount { get; set; }
public int GroupCount { get; set; }
diff --git a/src/Admin/Views/Organizations/_ViewInformation.cshtml b/src/Admin/Views/Organizations/_ViewInformation.cshtml
index 3c5a5ca8a6..24e69d298b 100644
--- a/src/Admin/Views/Organizations/_ViewInformation.cshtml
+++ b/src/Admin/Views/Organizations/_ViewInformation.cshtml
@@ -11,7 +11,7 @@
Users
- @Model.UserCount / @(Model.Organization.Seats?.ToString() ?? "-")
+ @Model.OccupiedSeatCount / @(Model.Organization.Seats?.ToString() ?? "-")
(@Model.UserInvitedCount /
@Model.UserAcceptedCount /
@Model.UserConfirmedCount)
diff --git a/src/Core/Models/Data/Organizations/OrganizationUsers/OrganizationUserUserDetails.cs b/src/Core/Models/Data/Organizations/OrganizationUsers/OrganizationUserUserDetails.cs
index ff28d1f3cd..ce9cf64a5c 100644
--- a/src/Core/Models/Data/Organizations/OrganizationUsers/OrganizationUserUserDetails.cs
+++ b/src/Core/Models/Data/Organizations/OrganizationUsers/OrganizationUserUserDetails.cs
@@ -56,4 +56,12 @@ public class OrganizationUserUserDetails : IExternal, ITwoFactorProvidersUser
{
return Premium.GetValueOrDefault(false);
}
+
+ public bool OccupiesOrganizationSeat
+ {
+ get
+ {
+ return Status != OrganizationUserStatusType.Revoked;
+ }
+ }
}
diff --git a/src/Core/Services/IOrganizationService.cs b/src/Core/Services/IOrganizationService.cs
index 3bd3e1f6eb..45f91c8bbc 100644
--- a/src/Core/Services/IOrganizationService.cs
+++ b/src/Core/Services/IOrganizationService.cs
@@ -64,4 +64,5 @@ public interface IOrganizationService
Task RestoreUserAsync(OrganizationUser organizationUser, Guid? restoringUserId, IUserService userService);
Task>> RestoreUsersAsync(Guid organizationId,
IEnumerable organizationUserIds, Guid? restoringUserId, IUserService userService);
+ Task GetOccupiedSeatCount(Organization organization);
}
diff --git a/src/Core/Services/Implementations/OrganizationService.cs b/src/Core/Services/Implementations/OrganizationService.cs
index d9551811bb..ce2b33ec58 100644
--- a/src/Core/Services/Implementations/OrganizationService.cs
+++ b/src/Core/Services/Implementations/OrganizationService.cs
@@ -44,7 +44,6 @@ public class OrganizationService : IOrganizationService
private readonly ICurrentContext _currentContext;
private readonly ILogger _logger;
-
public OrganizationService(
IOrganizationRepository organizationRepository,
IOrganizationUserRepository organizationUserRepository,
@@ -199,10 +198,10 @@ public class OrganizationService : IOrganizationService
(newPlan.HasAdditionalSeatsOption ? upgrade.AdditionalSeats : 0));
if (!organization.Seats.HasValue || organization.Seats.Value > newPlanSeats)
{
- var userCount = await _organizationUserRepository.GetCountByOrganizationIdAsync(organization.Id);
- if (userCount > newPlanSeats)
+ var occupiedSeats = await GetOccupiedSeatCount(organization);
+ if (occupiedSeats > newPlanSeats)
{
- throw new BadRequestException($"Your organization currently has {userCount} seats filled. " +
+ throw new BadRequestException($"Your organization currently has {occupiedSeats} seats filled. " +
$"Your new plan only has ({newPlanSeats}) seats. Remove some users.");
}
}
@@ -494,10 +493,10 @@ public class OrganizationService : IOrganizationService
if (!organization.Seats.HasValue || organization.Seats.Value > newSeatTotal)
{
- var userCount = await _organizationUserRepository.GetCountByOrganizationIdAsync(organization.Id);
- if (userCount > newSeatTotal)
+ var occupiedSeats = await GetOccupiedSeatCount(organization);
+ if (occupiedSeats > newSeatTotal)
{
- throw new BadRequestException($"Your organization currently has {userCount} seats filled. " +
+ throw new BadRequestException($"Your organization currently has {occupiedSeats} seats filled. " +
$"Your new plan only has ({newSeatTotal}) seats. Remove some users.");
}
}
@@ -861,10 +860,10 @@ public class OrganizationService : IOrganizationService
if (license.Seats.HasValue &&
(!organization.Seats.HasValue || organization.Seats.Value > license.Seats.Value))
{
- var userCount = await _organizationUserRepository.GetCountByOrganizationIdAsync(organization.Id);
- if (userCount > license.Seats.Value)
+ var occupiedSeats = await GetOccupiedSeatCount(organization);
+ if (occupiedSeats > license.Seats.Value)
{
- throw new BadRequestException($"Your organization currently has {userCount} seats filled. " +
+ throw new BadRequestException($"Your organization currently has {occupiedSeats} seats filled. " +
$"Your new license only has ({license.Seats.Value}) seats. Remove some users.");
}
}
@@ -1138,8 +1137,8 @@ public class OrganizationService : IOrganizationService
organizationId, invites.SelectMany(i => i.invite.Emails), false), StringComparer.InvariantCultureIgnoreCase);
if (organization.Seats.HasValue)
{
- var userCount = await _organizationUserRepository.GetCountByOrganizationIdAsync(organizationId);
- var availableSeats = organization.Seats.Value - userCount;
+ var occupiedSeats = await GetOccupiedSeatCount(organization);
+ var availableSeats = organization.Seats.Value - occupiedSeats;
newSeatsRequired = invites.Sum(i => i.invite.Emails.Count()) - existingEmails.Count() - availableSeats;
}
@@ -1559,7 +1558,7 @@ public class OrganizationService : IOrganizationService
organization.MaxAutoscaleSeats.HasValue &&
organization.MaxAutoscaleSeats.Value < organization.Seats.Value + seatsToAdd)
{
- return (false, $"Cannot invite new users. Seat limit has been reached.");
+ return (false, $"Seat limit has been reached.");
}
return (true, failureReason);
@@ -1951,8 +1950,8 @@ public class OrganizationService : IOrganizationService
var enoughSeatsAvailable = true;
if (organization.Seats.HasValue)
{
- var userCount = await _organizationUserRepository.GetCountByOrganizationIdAsync(organizationId);
- seatsAvailable = organization.Seats.Value - userCount;
+ var occupiedSeats = await GetOccupiedSeatCount(organization);
+ seatsAvailable = organization.Seats.Value - occupiedSeats;
enoughSeatsAvailable = seatsAvailable >= usersToAdd.Count;
}
@@ -2324,6 +2323,14 @@ public class OrganizationService : IOrganizationService
throw new BadRequestException("Only owners can restore other owners.");
}
+ var organization = await _organizationRepository.GetByIdAsync(organizationUser.OrganizationId);
+ var occupiedSeats = await GetOccupiedSeatCount(organization);
+ var availableSeats = organization.Seats.GetValueOrDefault(0) - occupiedSeats;
+ if (availableSeats < 1)
+ {
+ await AutoAddSeatsAsync(organization, 1, DateTime.UtcNow);
+ }
+
await CheckPoliciesBeforeRestoreAsync(organizationUser, userService);
var status = GetPriorActiveOrganizationUserStatusType(organizationUser);
@@ -2345,6 +2352,12 @@ public class OrganizationService : IOrganizationService
throw new BadRequestException("Users invalid.");
}
+ var organization = await _organizationRepository.GetByIdAsync(organizationId);
+ var occupiedSeats = await GetOccupiedSeatCount(organization);
+ var availableSeats = organization.Seats.GetValueOrDefault(0) - occupiedSeats;
+ var newSeatsRequired = organizationUserIds.Count() - availableSeats;
+ await AutoAddSeatsAsync(organization, newSeatsRequired, DateTime.UtcNow);
+
var deletingUserIsOwner = false;
if (restoringUserId.HasValue)
{
@@ -2455,4 +2468,10 @@ public class OrganizationService : IOrganizationService
return status;
}
+
+ public async Task GetOccupiedSeatCount(Organization organization)
+ {
+ var orgUsers = await _organizationUserRepository.GetManyDetailsByOrganizationAsync(organization.Id);
+ return orgUsers.Count(ou => ou.OccupiesOrganizationSeat);
+ }
}
diff --git a/test/Core.Test/Services/OrganizationServiceTests.cs b/test/Core.Test/Services/OrganizationServiceTests.cs
index c07a30184d..7e5e4474a1 100644
--- a/test/Core.Test/Services/OrganizationServiceTests.cs
+++ b/test/Core.Test/Services/OrganizationServiceTests.cs
@@ -897,7 +897,7 @@ public class OrganizationServiceTests
[BitAutoData(0, 100, 100, true, "")]
[BitAutoData(0, null, 100, true, "")]
[BitAutoData(1, 100, null, true, "")]
- [BitAutoData(1, 100, 100, false, "Cannot invite new users. Seat limit has been reached")]
+ [BitAutoData(1, 100, 100, false, "Seat limit has been reached")]
public void CanScale(int seatsToAdd, int? currentSeats, int? maxAutoscaleSeats,
bool expectedResult, string expectedFailureMessage, Organization organization,
SutProvider sutProvider)