From 87c181b6621de77d1cc47513b523b6f03e9d28a3 Mon Sep 17 00:00:00 2001 From: jrmccannon Date: Fri, 21 Mar 2025 15:18:05 -0500 Subject: [PATCH] Refactor validation parameter to improve clarity and consistency. Added XML doc --- .../IInviteOrganizationUsersCommand.cs | 16 ++++- .../InviteOrganizationUsersCommand.cs | 58 +++++++++---------- ...asswordManagerInviteUserValidationTests.cs | 4 +- 3 files changed, 46 insertions(+), 32 deletions(-) diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/IInviteOrganizationUsersCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/IInviteOrganizationUsersCommand.cs index 81eabc8d30..332991db49 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/IInviteOrganizationUsersCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/IInviteOrganizationUsersCommand.cs @@ -3,7 +3,21 @@ using Bit.Core.Models.Commands; namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.InviteUsers; +/// +/// Defines the contract for inviting organization users via SCIM (System for Cross-domain Identity Management). +/// Provides functionality for handling single email invitation requests within an organization context. +/// public interface IInviteOrganizationUsersCommand { - Task> InviteScimOrganizationUserAsync(OrganizationUserSingleEmailInvite request); + /// + /// Sends an invitation to add an organization user via SCIM (System for Cross-domain Identity Management) system. + /// This can be a Success or a Failure. Failure will contain the Error along with a representation of the errored value. + /// Success will be the successful return object. + /// + /// + /// Contains the details for inviting a single organization user via email. + /// + /// Response from InviteScimOrganiation + Task> InviteScimOrganizationUserAsync( + OrganizationUserSingleEmailInvite request); } diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/InviteOrganizationUsersCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/InviteOrganizationUsersCommand.cs index b15face7a4..4a1a0cdb2a 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/InviteOrganizationUsersCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/InviteOrganizationUsersCommand.cs @@ -139,42 +139,42 @@ public class InviteOrganizationUsersCommand(IEventService eventService, return new Success>(organizationUserCollection.Select(x => x.OrganizationUser)); } - private async Task RevertPasswordManagerChangesAsync(Valid valid, Organization organization) + private async Task RevertPasswordManagerChangesAsync(Valid validatedResult, Organization organization) { - if (valid.Value.PasswordManagerSubscriptionUpdate.SeatsRequiredToAdd > 0) + if (validatedResult.Value.PasswordManagerSubscriptionUpdate.SeatsRequiredToAdd > 0) { // When reverting seats, we have to tell payments service that the seats are going back down by what we attempted to add. // However, this might lead to a problem if we don't actually update stripe but throw any ways. // stripe could not be updated, and then we would decrement the number of seats in stripe accidentally. - var seatsToRemove = -valid.Value.PasswordManagerSubscriptionUpdate.SeatsRequiredToAdd; - await paymentService.AdjustSeatsAsync(organization, valid.Value.InviteOrganization.Plan, -seatsToRemove); + var seatsToRemove = -validatedResult.Value.PasswordManagerSubscriptionUpdate.SeatsRequiredToAdd; + await paymentService.AdjustSeatsAsync(organization, validatedResult.Value.InviteOrganization.Plan, -seatsToRemove); - organization.Seats = (short?)valid.Value.PasswordManagerSubscriptionUpdate.Seats; + organization.Seats = (short?)validatedResult.Value.PasswordManagerSubscriptionUpdate.Seats; await organizationRepository.ReplaceAsync(organization); await applicationCacheService.UpsertOrganizationAbilityAsync(organization); } } - private async Task RevertSecretsManagerChangesAsync(Valid valid, Organization organization) + private async Task RevertSecretsManagerChangesAsync(Valid validatedResult, Organization organization) { - if (valid.Value.SecretsManagerSubscriptionUpdate.SeatsRequiredToAdd < 0) + if (validatedResult.Value.SecretsManagerSubscriptionUpdate.SeatsRequiredToAdd < 0) { - var updateRevert = new SecretsManagerSubscriptionUpdate(organization, valid.Value.InviteOrganization.Plan, false) + var updateRevert = new SecretsManagerSubscriptionUpdate(organization, validatedResult.Value.InviteOrganization.Plan, false) { - SmSeats = valid.Value.SecretsManagerSubscriptionUpdate.Seats + SmSeats = validatedResult.Value.SecretsManagerSubscriptionUpdate.Seats }; await updateSecretsManagerSubscriptionCommand.UpdateSubscriptionAsync(updateRevert); } } - private async Task PublishReferenceEventAsync(Valid valid, + private async Task PublishReferenceEventAsync(Valid validatedResult, Organization organization) => await referenceEventService.RaiseEventAsync( new ReferenceEvent(ReferenceEventType.InvitedUsers, organization, currentContext) { - Users = valid.Value.Invites.Length + Users = validatedResult.Value.Invites.Length }); private async Task SendInvitesAsync(IEnumerable users, Organization organization) => @@ -183,14 +183,14 @@ public class InviteOrganizationUsersCommand(IEventService eventService, users.Select(x => x.OrganizationUser), organization)); - private async Task SendAdditionalEmailsAsync(Valid valid, Organization organization) + private async Task SendAdditionalEmailsAsync(Valid validatedResult, Organization organization) { - await SendPasswordManagerMaxSeatLimitEmailsAsync(valid, organization); + await SendPasswordManagerMaxSeatLimitEmailsAsync(validatedResult, organization); } - private async Task SendPasswordManagerMaxSeatLimitEmailsAsync(Valid valid, Organization organization) + private async Task SendPasswordManagerMaxSeatLimitEmailsAsync(Valid validatedResult, Organization organization) { - if (!valid.Value.PasswordManagerSubscriptionUpdate.MaxSeatsReached) + if (!validatedResult.Value.PasswordManagerSubscriptionUpdate.MaxSeatsReached) { return; } @@ -198,12 +198,12 @@ public class InviteOrganizationUsersCommand(IEventService eventService, try { var ownerEmails = (await organizationUserRepository - .GetManyByMinimumRoleAsync(valid.Value.InviteOrganization.OrganizationId, OrganizationUserType.Owner)) + .GetManyByMinimumRoleAsync(validatedResult.Value.InviteOrganization.OrganizationId, OrganizationUserType.Owner)) .Select(x => x.Email) .Distinct(); await mailService.SendOrganizationMaxSeatLimitReachedEmailAsync(organization, - valid.Value.PasswordManagerSubscriptionUpdate.MaxAutoScaleSeats!.Value, ownerEmails); + validatedResult.Value.PasswordManagerSubscriptionUpdate.MaxAutoScaleSeats!.Value, ownerEmails); } catch (Exception ex) { @@ -211,29 +211,29 @@ public class InviteOrganizationUsersCommand(IEventService eventService, } } - private async Task AdjustSecretsManagerSeatsAsync(Valid valid, Organization organization) + private async Task AdjustSecretsManagerSeatsAsync(Valid validatedResult, Organization organization) { - if (valid.Value.SecretsManagerSubscriptionUpdate.SeatsRequiredToAdd <= 0) + if (validatedResult.Value.SecretsManagerSubscriptionUpdate.SeatsRequiredToAdd <= 0) { return; } - var subscriptionUpdate = new SecretsManagerSubscriptionUpdate(organization, valid.Value.InviteOrganization.Plan, true) - .AdjustSeats(valid.Value.SecretsManagerSubscriptionUpdate.SeatsRequiredToAdd); + var subscriptionUpdate = new SecretsManagerSubscriptionUpdate(organization, validatedResult.Value.InviteOrganization.Plan, true) + .AdjustSeats(validatedResult.Value.SecretsManagerSubscriptionUpdate.SeatsRequiredToAdd); await updateSecretsManagerSubscriptionCommand.UpdateSubscriptionAsync(subscriptionUpdate); } - private async Task AdjustPasswordManagerSeatsAsync(Valid valid, Organization organization) + private async Task AdjustPasswordManagerSeatsAsync(Valid validatedResult, Organization organization) { - if (valid.Value.PasswordManagerSubscriptionUpdate.SeatsRequiredToAdd <= 0) + if (validatedResult.Value.PasswordManagerSubscriptionUpdate.SeatsRequiredToAdd <= 0) { return; } - await paymentService.AdjustSeatsAsync(organization, valid.Value.InviteOrganization.Plan, valid.Value.PasswordManagerSubscriptionUpdate.SeatsRequiredToAdd); + await paymentService.AdjustSeatsAsync(organization, validatedResult.Value.InviteOrganization.Plan, validatedResult.Value.PasswordManagerSubscriptionUpdate.SeatsRequiredToAdd); - organization.Seats = (short?)valid.Value.PasswordManagerSubscriptionUpdate.UpdatedSeatTotal; + organization.Seats = (short?)validatedResult.Value.PasswordManagerSubscriptionUpdate.UpdatedSeatTotal; await organizationRepository.ReplaceAsync(organization); // could optimize this with only a property update await applicationCacheService.UpsertOrganizationAbilityAsync(organization); @@ -241,10 +241,10 @@ public class InviteOrganizationUsersCommand(IEventService eventService, await referenceEventService.RaiseEventAsync( new ReferenceEvent(ReferenceEventType.AdjustSeats, organization, currentContext) { - PlanName = valid.Value.InviteOrganization.Plan.Name, - PlanType = valid.Value.InviteOrganization.Plan.Type, - Seats = valid.Value.PasswordManagerSubscriptionUpdate.UpdatedSeatTotal, - PreviousSeats = valid.Value.PasswordManagerSubscriptionUpdate.Seats + PlanName = validatedResult.Value.InviteOrganization.Plan.Name, + PlanType = validatedResult.Value.InviteOrganization.Plan.Type, + Seats = validatedResult.Value.PasswordManagerSubscriptionUpdate.UpdatedSeatTotal, + PreviousSeats = validatedResult.Value.PasswordManagerSubscriptionUpdate.Seats }); } } diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/Validation/PasswordManagerInviteUserValidationTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/Validation/PasswordManagerInviteUserValidationTests.cs index 8064e9e37f..c76ed258ff 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/Validation/PasswordManagerInviteUserValidationTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/Validation/PasswordManagerInviteUserValidationTests.cs @@ -61,7 +61,7 @@ public class PasswordManagerInviteUserValidationTests var result = PasswordManagerInviteUserValidation.Validate(subscriptionUpdate); Assert.IsType>(result); - Assert.Equal(PasswordManagerSeatLimitHasBeenReachedError.Code, (result as Invalid).ErrorMessageString); + Assert.Equal(PasswordManagerSeatLimitHasBeenReachedError.Code, (result as Invalid)!.ErrorMessageString); } [Theory] @@ -80,6 +80,6 @@ public class PasswordManagerInviteUserValidationTests var result = PasswordManagerInviteUserValidation.Validate(subscriptionUpdate); Assert.IsType>(result); - Assert.Equal(PasswordManagerPlanDoesNotAllowAdditionalSeatsError.Code, (result as Invalid).ErrorMessageString); + Assert.Equal(PasswordManagerPlanDoesNotAllowAdditionalSeatsError.Code, (result as Invalid)!.ErrorMessageString); } }