From 44b817ad03c0d774bb36fb5ac1e2fecd65ede2b8 Mon Sep 17 00:00:00 2001 From: jrmccannon Date: Mon, 24 Mar 2025 12:07:28 -0500 Subject: [PATCH] Separated old and new code explicitly. Moved old code checks down into new code as well. Added error and mapper to Failure --- .../src/Scim/Models/ScimUserRequestModel.cs | 22 +++- .../src/Scim/Users/PostUserCommand.cs | 105 ++++++++++-------- .../Scim.Test/Users/PostUserCommandTests.cs | 9 +- .../InviteUsers/Errors/ErrorMapper.cs | 37 ++++++ .../Errors/UserAlreadyExistsError.cs | 9 ++ .../InviteOrganizationUsersCommand.cs | 12 ++ .../Models/InviteOrganizationUsersResponse.cs | 13 +++ .../CannotAutoScaleOnSelfHostError.cs | 2 +- src/Core/Models/Commands/CommandResult.cs | 10 +- 9 files changed, 163 insertions(+), 56 deletions(-) create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/Errors/ErrorMapper.cs create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/Errors/UserAlreadyExistsError.cs diff --git a/bitwarden_license/src/Scim/Models/ScimUserRequestModel.cs b/bitwarden_license/src/Scim/Models/ScimUserRequestModel.cs index 1ba66983b8..16911d9078 100644 --- a/bitwarden_license/src/Scim/Models/ScimUserRequestModel.cs +++ b/bitwarden_license/src/Scim/Models/ScimUserRequestModel.cs @@ -2,6 +2,7 @@ using Bit.Core.AdminConsole.Models.Business; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.InviteUsers.Models; using Bit.Core.Enums; +using Bit.Core.Exceptions; using Bit.Core.Models.Data; using Bit.Core.Utilities; using OrganizationUserInvite = Bit.Core.Models.Business.OrganizationUserInvite; @@ -30,12 +31,21 @@ public class ScimUserRequestModel : BaseScimUserModel public OrganizationUserSingleEmailInvite ToRequest( ScimProviderType scimProvider, InviteOrganization inviteOrganization, - DateTimeOffset performedAt) => - new( - email: EmailForInvite(scimProvider), - inviteOrganization: inviteOrganization, - performedAt: performedAt, - externalId: ExternalIdForInvite()); + DateTimeOffset performedAt) + { + var email = EmailForInvite(scimProvider); + + if (string.IsNullOrWhiteSpace(email) || !Active) + { + throw new BadRequestException(); + } + + return new( + email: email, + inviteOrganization: inviteOrganization, + performedAt: performedAt, + externalId: ExternalIdForInvite()); + } private string EmailForInvite(ScimProviderType scimProvider) { diff --git a/bitwarden_license/src/Scim/Users/PostUserCommand.cs b/bitwarden_license/src/Scim/Users/PostUserCommand.cs index b947797516..7faf7096c9 100644 --- a/bitwarden_license/src/Scim/Users/PostUserCommand.cs +++ b/bitwarden_license/src/Scim/Users/PostUserCommand.cs @@ -1,7 +1,6 @@ #nullable enable using Bit.Core; -using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Business; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.InviteUsers; @@ -16,6 +15,7 @@ using Bit.Core.Services; using Bit.Scim.Context; using Bit.Scim.Models; using Bit.Scim.Users.Interfaces; +using static Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.InviteUsers.Errors.ErrorMapper; namespace Bit.Scim.Users; @@ -34,7 +34,63 @@ public class PostUserCommand( { public async Task PostUserAsync(Guid organizationId, ScimUserRequestModel model) { - var scimProvider = scimContext.RequestScimProvider; + if (featureService.IsEnabled(FeatureFlagKeys.ScimInviteUserOptimization) is false) + { + return await InviteScimOrganizationUserAsync(model, organizationId, scimContext.RequestScimProvider); + } + + return await InviteScimOrganizationUserAsync_vNext(model, organizationId, scimContext.RequestScimProvider); + } + + private async Task InviteScimOrganizationUserAsync_vNext( + ScimUserRequestModel model, + Guid organizationId, + ScimProviderType scimProvider) + { + var organization = await organizationRepository.GetByIdAsync(organizationId); + + if (organization is null) + { + throw new NotFoundException(); + } + + var plan = await pricingClient.GetPlan(organization.PlanType); + + if (plan == null) + { + logger.LogError("Plan {planType} not found for organization {organizationId}", + organization.PlanType, organization.Id); + return null; + } + + var request = model.ToRequest( + scimProvider: scimProvider, + inviteOrganization: new InviteOrganization(organization, plan), + performedAt: timeProvider.GetUtcNow()); + + var result = await inviteOrganizationUsersCommand.InviteScimOrganizationUserAsync(request); + + var invitedOrganizationUserId = result switch + { + Success success => success.Value.InvitedUser.Id, + Failure { ErrorMessage: InviteOrganizationUsersCommand.NoUsersToInvite } => (Guid?)null, + Failure failure when failure.Errors.Length != 0 => throw MapToBitException(failure.Errors), + Failure failure when failure.ErrorMessages.Count != 0 => throw new BadRequestException(failure.ErrorMessage), + _ => throw new InvalidOperationException() + }; + + var organizationUser = invitedOrganizationUserId.HasValue + ? await organizationUserRepository.GetDetailsByIdAsync(invitedOrganizationUserId.Value) + : null; + + return organizationUser; + } + + private async Task InviteScimOrganizationUserAsync( + ScimUserRequestModel model, + Guid organizationId, + ScimProviderType scimProvider) + { var invite = model.ToOrganizationUserInvite(scimProvider); var email = invite.Emails.Single(); @@ -65,56 +121,15 @@ public class PostUserCommand( throw new NotFoundException(); } - - if (featureService.IsEnabled(FeatureFlagKeys.ScimInviteUserOptimization)) - { - return await InviteScimOrganizationUserAsync(model, organization, scimProvider); - } - var hasStandaloneSecretsManager = await paymentService.HasSecretsManagerStandalone(organization); invite.AccessSecretsManager = hasStandaloneSecretsManager; var invitedOrgUser = await organizationService.InviteUserAsync(organizationId, invitingUserId: null, EventSystemUser.SCIM, - invite, externalId); + invite, + externalId); var orgUser = await organizationUserRepository.GetDetailsByIdAsync(invitedOrgUser.Id); return orgUser; } - - private async Task InviteScimOrganizationUserAsync(ScimUserRequestModel model, - Organization organization, ScimProviderType scimProvider) - { - var plan = await pricingClient.GetPlan(organization.PlanType); - - if (plan == null) - { - logger.LogError("Plan {planType} not found for organization {organizationId}", organization.PlanType, - organization.Id); - return null; - } - - var request = model.ToRequest( - scimProvider: scimProvider, - inviteOrganization: new InviteOrganization(organization, plan), - performedAt: timeProvider.GetUtcNow()); - - var result = await inviteOrganizationUsersCommand.InviteScimOrganizationUserAsync(request); - - var invitedOrganizationUserId = result switch - { - Success success => success.Value.InvitedUser.Id, - Failure { ErrorMessage: InviteOrganizationUsersCommand.NoUsersToInvite } => (Guid?)null, - Failure failure when failure.ErrorMessages.Count != 0 => throw new BadRequestException(failure.ErrorMessage), - _ => throw new InvalidOperationException() - }; - - var organizationUser = invitedOrganizationUserId.HasValue - ? await organizationUserRepository.GetDetailsByIdAsync(invitedOrganizationUserId.Value) - : null; - - return organizationUser; - } - - } diff --git a/bitwarden_license/test/Scim.Test/Users/PostUserCommandTests.cs b/bitwarden_license/test/Scim.Test/Users/PostUserCommandTests.cs index 71ad6361bd..ac23e7ecc1 100644 --- a/bitwarden_license/test/Scim.Test/Users/PostUserCommandTests.cs +++ b/bitwarden_license/test/Scim.Test/Users/PostUserCommandTests.cs @@ -27,7 +27,7 @@ public class PostUserCommandTests ExternalId = externalId, Emails = emails, Active = true, - Schemas = new List { ScimConstants.Scim2SchemaUser } + Schemas = [ScimConstants.Scim2SchemaUser] }; sutProvider.GetDependency() @@ -39,13 +39,16 @@ public class PostUserCommandTests sutProvider.GetDependency().HasSecretsManagerStandalone(organization).Returns(true); sutProvider.GetDependency() - .InviteUserAsync(organizationId, invitingUserId: null, EventSystemUser.SCIM, + .InviteUserAsync(organizationId, + invitingUserId: null, + EventSystemUser.SCIM, Arg.Is(i => i.Emails.Single().Equals(scimUserRequestModel.PrimaryEmail.ToLowerInvariant()) && i.Type == OrganizationUserType.User && !i.Collections.Any() && !i.Groups.Any() && - i.AccessSecretsManager), externalId) + i.AccessSecretsManager), + externalId) .Returns(newUser); var user = await sutProvider.Sut.PostUserAsync(organizationId, scimUserRequestModel); diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/Errors/ErrorMapper.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/Errors/ErrorMapper.cs new file mode 100644 index 0000000000..45df8b22f1 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/Errors/ErrorMapper.cs @@ -0,0 +1,37 @@ +using Bit.Core.AdminConsole.Errors; +using Bit.Core.Exceptions; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.InviteUsers.Errors; + +public static class ErrorMapper +{ + + /// + /// Maps the Error to a Bit.Exception class. + /// + /// + /// + /// + public static Exception MapToBitException(Error error) => + error switch + { + UserAlreadyExistsError alreadyExistsError => new ConflictException(alreadyExistsError.Message), + _ => new BadRequestException(error.Message) + }; + + /// + /// This maps the Error object to the Bit.Exception class. + /// + /// This should be replaced by an IActionResult mapper when possible. + /// + /// + /// + /// + public static Exception MapToBitException(IEnumerable> errors) => + errors switch + { + not null when errors.Count() == 1 => MapToBitException(errors.First()), + not null when errors.Count() > 1 => new BadRequestException(string.Join(' ', errors.Select(e => e.Message))), + _ => new BadRequestException() + }; +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/Errors/UserAlreadyExistsError.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/Errors/UserAlreadyExistsError.cs new file mode 100644 index 0000000000..475ad4a886 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/Errors/UserAlreadyExistsError.cs @@ -0,0 +1,9 @@ +using Bit.Core.AdminConsole.Errors; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.InviteUsers.Models; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.InviteUsers.Errors; + +public record UserAlreadyExistsError(ScimInviteOrganizationUsersResponse Response) : Error(Code, Response) +{ + public const string Code = "User already exists"; +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/InviteOrganizationUsersCommand.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/InviteOrganizationUsersCommand.cs index 4a1a0cdb2a..97bdf9445c 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/InviteOrganizationUsersCommand.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/InviteOrganizationUsersCommand.cs @@ -1,5 +1,6 @@ using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Interfaces; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.InviteUsers.Errors; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.InviteUsers.Models; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.InviteUsers.Validation; using Bit.Core.AdminConsole.Shared.Validation; @@ -39,10 +40,21 @@ public class InviteOrganizationUsersCommand(IEventService eventService, public const string NoUsersToInvite = "No users to invite."; public const string InvalidResultType = "Invalid result type."; + public async Task> InviteScimOrganizationUserAsync(OrganizationUserSingleEmailInvite request) { var hasSecretsManager = await paymentService.HasSecretsManagerStandalone(request.InviteOrganization); + var orgUsers = await organizationUserRepository.GetManyDetailsByOrganizationAsync(request.InviteOrganization.OrganizationId); + + if (orgUsers.Any(existingUser => + request.Email.Equals(existingUser.Email, StringComparison.InvariantCultureIgnoreCase) || + request.ExternalId.Equals(existingUser.ExternalId, StringComparison.InvariantCultureIgnoreCase))) + { + return new Failure( + new UserAlreadyExistsError(new ScimInviteOrganizationUsersResponse(request))); + } + var result = await InviteOrganizationUsersAsync(new InviteOrganizationUsersRequest(request, hasSecretsManager)); switch (result) diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/Models/InviteOrganizationUsersResponse.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/Models/InviteOrganizationUsersResponse.cs index 76bb5e4399..bbfa855ac8 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/Models/InviteOrganizationUsersResponse.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/Models/InviteOrganizationUsersResponse.cs @@ -11,4 +11,17 @@ public class ScimInviteOrganizationUsersResponse { public OrganizationUser InvitedUser { get; init; } + public ScimInviteOrganizationUsersResponse() + { + + } + + public ScimInviteOrganizationUsersResponse(OrganizationUserSingleEmailInvite request) + { + InvitedUser = new OrganizationUser + { + Email = request.Email, + ExternalId = request.ExternalId + }; + } } diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/Validation/GlobalSettings/CannotAutoScaleOnSelfHostError.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/Validation/GlobalSettings/CannotAutoScaleOnSelfHostError.cs index 3ea620d60d..f1a4e5d6d3 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/Validation/GlobalSettings/CannotAutoScaleOnSelfHostError.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/InviteUsers/Validation/GlobalSettings/CannotAutoScaleOnSelfHostError.cs @@ -5,5 +5,5 @@ namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.InviteUse public record CannotAutoScaleOnSelfHostError(IGlobalSettings InvalidSettings) : Error(Code, InvalidSettings) { - public const string Code = "CannotAutoScaleOnSelfHost"; + public const string Code = "Cannot auto scale self-host."; } diff --git a/src/Core/Models/Commands/CommandResult.cs b/src/Core/Models/Commands/CommandResult.cs index a8ec772fc1..26aac9504b 100644 --- a/src/Core/Models/Commands/CommandResult.cs +++ b/src/Core/Models/Commands/CommandResult.cs @@ -40,10 +40,18 @@ public class Success(T value) : CommandResult public class Failure(IEnumerable errorMessages) : CommandResult { public List ErrorMessages { get; } = errorMessages.ToList(); + public Error[] Errors { get; set; } = []; public string ErrorMessage => string.Join(" ", ErrorMessages); - public Failure(string error) : this([error]) { } + public Failure(string error) : this([error]) + { + } + + public Failure(Error error) : this([error.Message]) + { + Errors = [error]; + } } public class Partial : CommandResult