bitwarden/server
1
0
Fork 0
mirror of https://github.com/bitwarden/server synced 2025-12-10 13:23:27 +00:00
Code Issues Releases Wiki Activity

[PM-22692] Fix Secrets Manager Seat and ServiceAccount Limit Bug (#6138)

Browse Source 
* test: add new test harnesses

* feat: update autoscale limit logic for SM Subscription Command

* fix: remove redundant helper methods

* fix: add periods to second sentence of templates
This commit is contained in:
Stephon Brown
2025-08-01 14:40:43 -04:00
committed by GitHub
parent 5485c12445
commit 2908ddb759
5 changed files with 193 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/Core/MailTemplates/Handlebars/OrganizationSmServiceAccountsMaxReached.html.hbs
View File

@@ -6,7 +6,7 @@
<td class="content-block"
style="font-family: 'Helvetica Neue', Helvetica, Arial, sans-serif; box-sizing: border-box; font-size: 16px; color: #333; line-height: 25px; margin: 0; -webkit-font-smoothing: antialiased; padding: 0 0 10px; -webkit-text-size-adjust: none;"
valign="top">
Your organization has reached the Secrets Manager machine accounts limit of {{MaxServiceAccountsCount}}. New machine accounts cannot be created
Your organization has reached the Secrets Manager machine accounts limit of {{MaxServiceAccountsCount}}. New machine accounts cannot be created.
</td>
</tr>
<tr

2
src/Core/MailTemplates/Handlebars/OrganizationSmServiceAccountsMaxReached.text.hbs
View File

@@ -1,5 +1,5 @@
{{#>BasicTextLayout}}
Your organization has reached the Secrets Manager machine accounts limit of {{MaxServiceAccountsCount}}. New machine accounts cannot be created
Your organization has reached the Secrets Manager machine accounts limit of {{MaxServiceAccountsCount}}. New machine accounts cannot be created.
For more information, please refer to the following help article: https://bitwarden.com/help/managing-users
{{/BasicTextLayout}}

4
src/Core/Models/Business/SecretsManagerSubscriptionUpdate.cs
View File

@@ -50,11 +50,7 @@ public class SecretsManagerSubscriptionUpdate
public bool MaxAutoscaleSmSeatsChanged => MaxAutoscaleSmSeats != Organization.MaxAutoscaleSmSeats;
public bool MaxAutoscaleSmServiceAccountsChanged =>
MaxAutoscaleSmServiceAccounts != Organization.MaxAutoscaleSmServiceAccounts;
public bool SmSeatAutoscaleLimitReached => SmSeats.HasValue && MaxAutoscaleSmSeats.HasValue && SmSeats == MaxAutoscaleSmSeats;
public bool SmServiceAccountAutoscaleLimitReached => SmServiceAccounts.HasValue &&
MaxAutoscaleSmServiceAccounts.HasValue &&
SmServiceAccounts == MaxAutoscaleSmServiceAccounts;
public SecretsManagerSubscriptionUpdate(Organization organization, Plan plan, bool autoscaling)
{

77
src/Core/OrganizationFeatures/OrganizationSubscriptions/UpdateSecretsManagerSubscriptionCommand.cs
View File

@@ -55,15 +55,7 @@ public class UpdateSecretsManagerSubscriptionCommand : IUpdateSecretsManagerSubs
await FinalizeSubscriptionAdjustmentAsync(update);
if (update.SmSeatAutoscaleLimitReached)
{
await SendSeatLimitEmailAsync(update.Organization);
}
if (update.SmServiceAccountAutoscaleLimitReached)
{
await SendServiceAccountLimitEmailAsync(update.Organization);
}
await ValidateAutoScaleLimitsAsync(update);
}
private async Task FinalizeSubscriptionAdjustmentAsync(SecretsManagerSubscriptionUpdate update)
@@ -100,7 +92,7 @@ public class UpdateSecretsManagerSubscriptionCommand : IUpdateSecretsManagerSubs
OrganizationUserType.Owner))
.Select(u => u.Email).Distinct();
await _mailService.SendSecretsManagerMaxSeatLimitReachedEmailAsync(organization, organization.MaxAutoscaleSmSeats.Value, ownerEmails);
await _mailService.SendSecretsManagerMaxSeatLimitReachedEmailAsync(organization, organization.MaxAutoscaleSmSeats!.Value, ownerEmails);
}
catch (Exception e)
@@ -117,7 +109,7 @@ public class UpdateSecretsManagerSubscriptionCommand : IUpdateSecretsManagerSubs
OrganizationUserType.Owner))
.Select(u => u.Email).Distinct();
await _mailService.SendSecretsManagerMaxServiceAccountLimitReachedEmailAsync(organization, organization.MaxAutoscaleSmServiceAccounts.Value, ownerEmails);
await _mailService.SendSecretsManagerMaxServiceAccountLimitReachedEmailAsync(organization, organization.MaxAutoscaleSmServiceAccounts!.Value, ownerEmails);
}
catch (Exception e)
@@ -197,7 +189,7 @@ public class UpdateSecretsManagerSubscriptionCommand : IUpdateSecretsManagerSubs
throw new BadRequestException("Organization has no Secrets Manager seat limit, no need to adjust seats");
}
if (update.Autoscaling && update.SmSeats.Value < organization.SmSeats.Value)
if (update.Autoscaling && update.SmSeats!.Value < organization.SmSeats.Value)
{
throw new BadRequestException("Cannot use autoscaling to subtract seats.");
}
@@ -211,7 +203,7 @@ public class UpdateSecretsManagerSubscriptionCommand : IUpdateSecretsManagerSubs
}
// Check autoscale maximum seats
if (update.MaxAutoscaleSmSeats.HasValue && update.SmSeats.Value > update.MaxAutoscaleSmSeats.Value)
if (update.MaxAutoscaleSmSeats.HasValue && update.SmSeats!.Value > update.MaxAutoscaleSmSeats.Value)
{
var message = update.Autoscaling
? "Secrets Manager seat limit has been reached."
@@ -220,7 +212,7 @@ public class UpdateSecretsManagerSubscriptionCommand : IUpdateSecretsManagerSubs
}
// Check minimum seats included with plan
if (plan.SecretsManager.BaseSeats > update.SmSeats.Value)
if (plan.SecretsManager.BaseSeats > update.SmSeats!.Value)
{
throw new BadRequestException($"Plan has a minimum of {plan.SecretsManager.BaseSeats} Secrets Manager seats.");
}
@@ -260,7 +252,7 @@ public class UpdateSecretsManagerSubscriptionCommand : IUpdateSecretsManagerSubs
throw new BadRequestException("Organization has no machine accounts limit, no need to adjust machine accounts");
}
if (update.Autoscaling && update.SmServiceAccounts.Value < organization.SmServiceAccounts.Value)
if (update.Autoscaling && update.SmServiceAccounts!.Value < organization.SmServiceAccounts.Value)
{
throw new BadRequestException("Cannot use autoscaling to subtract machine accounts.");
}
@@ -276,7 +268,7 @@ public class UpdateSecretsManagerSubscriptionCommand : IUpdateSecretsManagerSubs
// Check autoscale maximum service accounts
if (update.MaxAutoscaleSmServiceAccounts.HasValue &&
update.SmServiceAccounts.Value > update.MaxAutoscaleSmServiceAccounts.Value)
update.SmServiceAccounts!.Value > update.MaxAutoscaleSmServiceAccounts.Value)
{
var message = update.Autoscaling
? "Secrets Manager machine account limit has been reached."
@@ -285,7 +277,7 @@ public class UpdateSecretsManagerSubscriptionCommand : IUpdateSecretsManagerSubs
}
// Check minimum service accounts included with plan
if (plan.SecretsManager.BaseServiceAccount > update.SmServiceAccounts.Value)
if (plan.SecretsManager.BaseServiceAccount > update.SmServiceAccounts!.Value)
{
throw new BadRequestException($"Plan has a minimum of {plan.SecretsManager.BaseServiceAccount} machine accounts.");
}
@@ -379,4 +371,55 @@ public class UpdateSecretsManagerSubscriptionCommand : IUpdateSecretsManagerSubs
await _eventService.LogOrganizationEventAsync(org, orgEvent.Value);
}
}
private async Task ValidateAutoScaleLimitsAsync(SecretsManagerSubscriptionUpdate update)
{
var (smSeatAutoScaleLimitReached, smServiceAccountsLimitReached) = await AreAutoscaleLimitsReachedAsync(update);
if (smSeatAutoScaleLimitReached)
{
await SendSeatLimitEmailAsync(update.Organization);
}
if (smServiceAccountsLimitReached)
{
await SendServiceAccountLimitEmailAsync(update.Organization);
}
}
private async Task<(bool, bool)> AreAutoscaleLimitsReachedAsync(SecretsManagerSubscriptionUpdate update)
{
var smSeatAutoScaleLimitReached = false;
var smServiceAccountsLimitReached = false;
var (occupiedSmSeats, occupiedSmServiceAccounts) = await GetOccupiedSmSeatsAndServiceAccountsAsync(update.Organization.Id);
if (occupiedSmSeats > 0
&& update.MaxAutoscaleSmSeats is not null
&& occupiedSmSeats == update.MaxAutoscaleSmSeats!.Value)
{
smSeatAutoScaleLimitReached = true;
}
if (occupiedSmServiceAccounts > 0
&& update.MaxAutoscaleSmServiceAccounts is not null
&& occupiedSmServiceAccounts == update.MaxAutoscaleSmServiceAccounts!.Value)
{
smServiceAccountsLimitReached = true;
}
return (smSeatAutoScaleLimitReached, smServiceAccountsLimitReached);
}
/// <summary>
/// Requests the number of Secret Manager seats and service accounts are currently used by the organization
/// </summary>
/// <param name="organizationId"> The id of the organization</param>
/// <returns > A tuple containing the occupied seats and the occupied service account counts</returns>
private async Task<(int, int)> GetOccupiedSmSeatsAndServiceAccountsAsync(Guid organizationId)
{
var occupiedSmSeatsTask = _organizationUserRepository.GetOccupiedSmSeatCountByOrganizationIdAsync(organizationId);
var occupiedServiceAccountsTask = _serviceAccountRepository.GetServiceAccountCountByOrganizationIdAsync(organizationId);
return (await occupiedSmSeatsTask, await occupiedServiceAccountsTask);
}
}

141
test/Core.Test/OrganizationFeatures/OrganizationSubscriptionUpdate/UpdateSecretsManagerSubscriptionCommandTests.cs
View File

@@ -1,7 +1,9 @@
using Bit.Core.AdminConsole.Entities;
using Bit.Core.Billing.Enums;
using Bit.Core.Enums;
using Bit.Core.Exceptions;
using Bit.Core.Models.Business;
using Bit.Core.Models.Data.Organizations.OrganizationUsers;
using Bit.Core.Models.StaticStore;
using Bit.Core.OrganizationFeatures.OrganizationSubscriptions;
using Bit.Core.Repositories;
@@ -99,8 +101,13 @@ public class UpdateSecretsManagerSubscriptionCommandTests
org.MaxAutoscaleSmServiceAccounts == updateMaxAutoscaleSmServiceAccounts),
sutProvider);
await sutProvider.GetDependency<IMailService>().DidNotReceiveWithAnyArgs().SendSecretsManagerMaxSeatLimitReachedEmailAsync(default, default, default);
await sutProvider.GetDependency<IMailService>().DidNotReceiveWithAnyArgs().SendSecretsManagerMaxServiceAccountLimitReachedEmailAsync(default, default, default);
await sutProvider
.GetDependency<IMailService>()
.DidNotReceiveWithAnyArgs()
.SendSecretsManagerMaxSeatLimitReachedEmailAsync(default, default, default);
await sutProvider.GetDependency<IMailService>()
.DidNotReceiveWithAnyArgs()
.SendSecretsManagerMaxServiceAccountLimitReachedEmailAsync(default, default, default);
}
[Theory]
@@ -266,11 +273,13 @@ public class UpdateSecretsManagerSubscriptionCommandTests
[Theory]
[BitAutoData]
public async Task UpdateSubscriptionAsync_UpdateSeatsToAutoscaleLimit_EmailsOwners(
public async Task UpdateSubscriptionAsync_UpdateSeatCount_AndExistingSeatsDoNotReachAutoscaleLimit_NoEmailSent(
Organization organization,
SutProvider<UpdateSecretsManagerSubscriptionCommand> sutProvider)
{
// Arrange
const int seatCount = 10;
var existingSeatCount = 9;
// Make sure Password Manager seats is greater or equal to Secrets Manager seats
organization.Seats = seatCount;
@@ -281,11 +290,66 @@ public class UpdateSecretsManagerSubscriptionCommandTests
SmSeats = seatCount,
MaxAutoscaleSmSeats = seatCount
};
sutProvider.GetDependency<IOrganizationUserRepository>()
.GetOccupiedSmSeatCountByOrganizationIdAsync(organization.Id)
.Returns(existingSeatCount);
// Act
await sutProvider.Sut.UpdateSubscriptionAsync(update);
await sutProvider.GetDependency<IMailService>().Received(1).SendSecretsManagerMaxSeatLimitReachedEmailAsync(
organization, organization.MaxAutoscaleSmSeats.Value, Arg.Any<IEnumerable<string>>());
// Assert
// Currently being called once each for different validation methods
await sutProvider.GetDependency<IOrganizationUserRepository>()
.Received(2)
.GetOccupiedSmSeatCountByOrganizationIdAsync(organization.Id);
await sutProvider.GetDependency<IMailService>()
.DidNotReceiveWithAnyArgs()
.SendSecretsManagerMaxSeatLimitReachedEmailAsync(Arg.Any<Organization>(), Arg.Any<int>(), Arg.Any<IEnumerable<string>>());
}
[Theory]
[BitAutoData]
public async Task UpdateSubscriptionAsync_ExistingSeatsReachAutoscaleLimit_EmailOwners(
Organization organization,
SutProvider<UpdateSecretsManagerSubscriptionCommand> sutProvider)
{
// Arrange
const int seatCount = 10;
const int existingSeatCount = 10;
var ownerDetailsList = new List<OrganizationUserUserDetails> { new() { Email = "owner@example.com" } };
// The amount of seats for users in an organization
var plan = StaticStore.GetPlan(organization.PlanType);
var update = new SecretsManagerSubscriptionUpdate(organization, plan, false)
{
SmSeats = seatCount,
MaxAutoscaleSmSeats = seatCount
};
sutProvider.GetDependency<IOrganizationUserRepository>()
.GetOccupiedSmSeatCountByOrganizationIdAsync(organization.Id)
.Returns(existingSeatCount);
sutProvider.GetDependency<IOrganizationUserRepository>()
.GetManyByMinimumRoleAsync(organization.Id, OrganizationUserType.Owner)
.Returns(ownerDetailsList);
// Act
await sutProvider.Sut.UpdateSubscriptionAsync(update);
// Assert
// Currently being called once each for different validation methods
await sutProvider.GetDependency<IOrganizationUserRepository>()
.Received(2)
.GetOccupiedSmSeatCountByOrganizationIdAsync(organization.Id);
await sutProvider.GetDependency<IMailService>()
.Received(1)
.SendSecretsManagerMaxSeatLimitReachedEmailAsync(Arg.Is(organization),
Arg.Is(seatCount),
Arg.Is<IEnumerable<string>>(emails => emails.Contains(ownerDetailsList[0].Email)));
}
[Theory]
@@ -413,21 +477,78 @@ public class UpdateSecretsManagerSubscriptionCommandTests
[Theory]
[BitAutoData]
public async Task UpdateSubscriptionAsync_UpdateServiceAccountsToAutoscaleLimit_EmailsOwners(
public async Task UpdateSubscriptionAsync_UpdateServiceAccounts_AndExistingServiceAccountsCountDoesNotReachAutoscaleLimit_NoEmailSent(
Organization organization,
SutProvider<UpdateSecretsManagerSubscriptionCommand> sutProvider)
{
// Arrange
var smServiceAccounts = 300;
var existingServiceAccountCount = 299;
var plan = StaticStore.GetPlan(organization.PlanType);
var update = new SecretsManagerSubscriptionUpdate(organization, plan, false)
{
SmServiceAccounts = 300,
MaxAutoscaleSmServiceAccounts = 300
SmServiceAccounts = smServiceAccounts,
MaxAutoscaleSmServiceAccounts = smServiceAccounts
};
sutProvider.GetDependency<IServiceAccountRepository>()
.GetServiceAccountCountByOrganizationIdAsync(organization.Id)
.Returns(existingServiceAccountCount);
// Act
await sutProvider.Sut.UpdateSubscriptionAsync(update);
await sutProvider.GetDependency<IMailService>().Received(1).SendSecretsManagerMaxServiceAccountLimitReachedEmailAsync(
organization, organization.MaxAutoscaleSmServiceAccounts.Value, Arg.Any<IEnumerable<string>>());
// Assert
await sutProvider.GetDependency<IServiceAccountRepository>()
.Received(1)
.GetServiceAccountCountByOrganizationIdAsync(organization.Id);
await sutProvider.GetDependency<IMailService>()
.DidNotReceiveWithAnyArgs()
.SendSecretsManagerMaxServiceAccountLimitReachedEmailAsync(
Arg.Any<Organization>(),
Arg.Any<int>(),
Arg.Any<IEnumerable<string>>());
}
[Theory]
[BitAutoData]
public async Task UpdateSubscriptionAsync_ExistingServiceAccountsReachAutoscaleLimit_EmailOwners(
Organization organization,
SutProvider<UpdateSecretsManagerSubscriptionCommand> sutProvider)
{
var smServiceAccounts = 300;
var plan = StaticStore.GetPlan(organization.PlanType);
var update = new SecretsManagerSubscriptionUpdate(organization, plan, false)
{
SmServiceAccounts = smServiceAccounts,
MaxAutoscaleSmServiceAccounts = smServiceAccounts
};
var ownerDetailsList = new List<OrganizationUserUserDetails> { new() { Email = "owner@example.com" } };
sutProvider.GetDependency<IServiceAccountRepository>()
.GetServiceAccountCountByOrganizationIdAsync(organization.Id)
.Returns(smServiceAccounts);
sutProvider.GetDependency<IOrganizationUserRepository>()
.GetManyByMinimumRoleAsync(organization.Id, OrganizationUserType.Owner)
.Returns(ownerDetailsList);
// Act
await sutProvider.Sut.UpdateSubscriptionAsync(update);
// Assert
await sutProvider.GetDependency<IServiceAccountRepository>()
.Received(1)
.GetServiceAccountCountByOrganizationIdAsync(organization.Id);
await sutProvider.GetDependency<IMailService>()
.Received(1)
.SendSecretsManagerMaxServiceAccountLimitReachedEmailAsync(Arg.Is(organization),
Arg.Is(smServiceAccounts),
Arg.Is<IEnumerable<string>>(emails => emails.Contains(ownerDetailsList[0].Email)));
}
[Theory]