From c271e2dae214583cb449f58aa67340a135432305 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rui=20Tom=C3=A9?= <108268980+r-tome@users.noreply.github.com> Date: Fri, 1 Sep 2023 09:10:02 +0100 Subject: [PATCH] [PM-3177] Extract IOrganizationService.UpdateUserGroupsAsync to a command (#3131) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [AC-1423] Add AddonProduct and BitwardenProduct properties to BillingSubscriptionItem (#3037) * [AC-1423] Add AddonProduct and BitwardenProduct properties to BillingSubscriptionItem - Add a helper method to determine the appropriate addon type based on the subscription items StripeId * [AC-1423] Add helper to StaticStore.cs to find a Plan by StripePlanId * [AC-1423] Use the helper method to set SubscriptionInfo.BitwardenProduct * Add SecretsManagerBilling feature flag to Constants * [AC 1409] Secrets Manager Subscription Stripe Integration (#3019) * Adding the Secret manager to the Plan List * Adding the unit test for the StaticStoreTests class * Fix whitespace formatting * Fix whitespace formatting * Price update * Resolving the PR comments * Resolving PR comments * Fixing the whitespace * only password manager plans are return for now * format whitespace * Resolve the test issue * Fixing the failing test * Refactoring the Plan separation * add a unit test for SingleOrDefault * Fix the whitespace format * Separate the PM and SM plans * Fixing the whitespace * Remove unnecessary directive * Fix imports ordering * Fix imports ordering * Resolve imports ordering * Fixing imports ordering * Fix response model, add MaxProjects * Fix filename * Fix format * Fix: seat price should match annual/monthly * Fix service account annual pricing * Changes for secret manager signup and upgradeplan * Changes for secrets manager signup and upgrade * refactoring the code * Format whitespace * remove unnecessary using directive * Resolve the PR comment on Subscription creation * Resolve PR comment * Add password manager to the error message * Add UseSecretsManager to the event log * Resolve PR comment on plan validation * Resolving pr comments for service account count * Resolving pr comments for service account count * Resolve the pr comments * Remove the store procedure that is no-longer needed * Rename a property properly * Resolving the PR comment * Resolve PR comments * Resolving PR comments * Resolving the Pr comments * Resolving some PR comments * Resolving the PR comments * Resolving the build identity build * Add additional Validation * Resolve the Lint issues * remove unnecessary using directive * Remove the white spaces * Adding unit test for the stripe payment * Remove the incomplete test * Fixing the failing test * Fix the failing test * Fix the fail test on organization service * Fix the failing unit test * Fix the whitespace format * Fix the failing test * Fix the whitespace format * resolve pr comments * Fix the lint message * Resolve the PR comments * resolve pr comments * Resolve pr comments * Resolve the pr comments * remove unused code * Added for sm validation test * Fix the whitespace format issues --------- Co-authored-by: Thomas Rittson Co-authored-by: Thomas Rittson <31796059+eliykat@users.noreply.github.com> * SM-802: Add SecretsManagerBetaColumn SQL migration and Org table update * SM-802: Run EF Migrations for SecretsManagerBeta * SM-802: Update the two Org procs and View, and move data migration to a separate file * SM-802: Add missing comma to Organization_Create * [AC-1418] Add missing SecretsManagerPlan property to OrganizationResponseModel (#3055) * SM-802: Remove extra GO statement from data migration script * [AC 1460] Update Stripe Configuration (#3070) * change the stripeseat id * change service accountId to align with new product * make all the Id name for consistent * SM-802: Add SecretsManagerBeta to OrganizationResponseModel * SM-802: Move SecretsManagerBeta from OrganizationResponseModel to OrganizationSubscriptionResponseModel. Use sp_refreshview instead of sp_refreshsqlmodule in the migration script. * SM-802: Remove OrganizationUserOrganizationDetailsView.sql changes * [AC 1410] Secrets Manager subscription adjustment back-end changes (#3036) * Create UpgradeSecretsManagerSubscription command --------- Co-authored-by: Thomas Rittson * SM-802: Remove SecretsManagerBetaColumn migration * SM-802: Add SecretsManagerBetaColumn migration * SM-802: Remove OrganizationUserOrganizationDetailsView update * [AC-1495] Extract UpgradePlanAsync into a command (#3081) * This is a pure lift & shift with no refactors * Only register subscription commands in Api --------- Co-authored-by: cyprain-okeke * [AC-1503] Fix Stripe integration on organization upgrade (#3084) * Fix SM parameters not being passed to Stripe * Fix flaky test * Fix error message * [AC-1504] Allow SM max autoscale limits to be disabled (#3085) * [AC-1488] Changed SM Signup and Upgrade paths to set SmServiceAccounts to include the plan BaseServiceAccount (#3086) * [AC-1510] Enable access to Secrets Manager to Organization owner for new Subscription (#3089) * Revert changes to ReferenceEvent code (#3091) * Revert changes to ReferenceEvent code This will be done in AC-1481 * Revert ReferenceEventType change * Move NoopServiceAccountRepository to SM and update namespace * [AC-1462] Add secrets manager service accounts autoscaling commands (#3059) * Adding the Secret manager to the Plan List * Adding the unit test for the StaticStoreTests class * Fix whitespace formatting * Fix whitespace formatting * Price update * Resolving the PR comments * Resolving PR comments * Fixing the whitespace * only password manager plans are return for now * format whitespace * Resolve the test issue * Fixing the failing test * Refactoring the Plan separation * add a unit test for SingleOrDefault * Fix the whitespace format * Separate the PM and SM plans * Fixing the whitespace * Remove unnecessary directive * Fix imports ordering * Fix imports ordering * Resolve imports ordering * Fixing imports ordering * Fix response model, add MaxProjects * Fix filename * Fix format * Fix: seat price should match annual/monthly * Fix service account annual pricing * Changes for secret manager signup and upgradeplan * Changes for secrets manager signup and upgrade * refactoring the code * Format whitespace * remove unnecessary using directive * Changes for subscription Update * Update the seatAdjustment and update * Resolve the PR comment on Subscription creation * Resolve PR comment * Add password manager to the error message * Add UseSecretsManager to the event log * Resolve PR comment on plan validation * Resolving pr comments for service account count * Resolving pr comments for service account count * Resolve the pr comments * Remove the store procedure that is no-longer needed * Add a new class for update subscription * Modify the Update subscription for sm * Add the missing property * Rename a property properly * Resolving the PR comment * Resolve PR comments * Resolving PR comments * Resolving the Pr comments * Resolving some PR comments * Resolving the PR comments * Resolving the build identity build * Add additional Validation * Resolve the Lint issues * remove unnecessary using directive * Remove the white spaces * Adding unit test for the stripe payment * Remove the incomplete test * Fixing the failing test * Fix the failing test * Fix the fail test on organization service * Fix the failing unit test * Fix the whitespace format * Fix the failing test * Fix the whitespace format * resolve pr comments * Fix the lint message * refactor the code * Fix the failing Test * adding a new endpoint * Remove the unwanted code * Changes for Command and Queries * changes for command and queries * Fix the Lint issues * Fix imports ordering * Resolve the PR comments * resolve pr comments * Resolve pr comments * Fix the failing test on adjustSeatscommandtests * Fix the failing test * Fix the whitespaces * resolve failing test * rename a property * Resolve the pr comments * refactoring the existing implementation * Resolve the whitespaces format issue * Resolve the pr comments * [AC-1462] Created IAvailableServiceAccountsQuery along its implementation and with unit tests * [AC-1462] Renamed ICountNewServiceAccountSlotsRequiredQuery * [AC-1462] Added IAutoscaleServiceAccountsCommand and implementation * Add more unit testing * fix the whitespaces issues * [AC-1462] Added unit tests for AutoscaleServiceAccountsCommand * Add more unit test * Remove unnecessary directive * Resolve some pr comments * Adding more unit test * adding more test * add more test * Resolving some pr comments * Resolving some pr comments * Resolving some pr comments * resolve some pr comments * Resolving pr comments * remove whitespaces * remove white spaces * Resolving pr comments * resolving pr comments and fixing white spaces * resolving the lint error * Run dotnet format * resolving the pr comments * Add a missing properties to plan response model * Add the email sender for sm seat and service acct * Add the email sender for sm seat and service acct * Fix the failing test after email sender changes * Add staticstorewrapper to properly test the plans * Add more test and validate the existing test * Fix the white spaces issues * Remove staticstorewrapper and fix the test * fix a null issue on autoscaling * Suggestion: do all seat calculations in update model * Resolve some pr comments * resolving some pr comments * Return value is unnecessary * Resolve the failing test * resolve pr comments * Resolve the pr comments * Resolving admin api failure and adding more test * Resolve the issue failing admin project * Fixing the failed test * Clarify naming and add comments * Clarify naming conventions * Dotnet format * Fix the failing dependency * remove similar test * [AC-1462] Rewrote AutoscaleServiceAccountsCommand to use UpdateSecretsManagerSubscriptionCommand which has the same logic * [AC-1462] Deleted IAutoscaleServiceAccountsCommand as the logic will be moved to UpdateSecretsManagerSubscriptionCommand * [AC-1462] Created method AdjustSecretsManagerServiceAccountsAsync * [AC-1462] Changed SecretsManagerSubscriptionUpdate to only be set by its constructor * [AC-1462] Added check to CountNewServiceAccountSlotsRequiredQuery and revised unit tests * [AC-1462] Revised logic for CountNewServiceAccountSlotsRequiredQuery and fixed unit tests * [AC-1462] Changed SecretsManagerSubscriptionUpdate to receive Organization as a parameter and fixed the unit tests * [AC-1462] Renamed IUpdateSecretsManagerSubscriptionCommand methods UpdateSubscriptionAsync and AdjustServiceAccountsAsync * [AC-1462] Rewrote unit test UpdateSubscriptionAsync_ValidInput_Passes * [AC-1462] Registered CountNewServiceAccountSlotsRequiredQuery for dependency injection * [AC-1462] Added parameter names to SecretsManagerSubscriptionUpdateRequestModel * [AC-1462] Updated SecretsManagerSubscriptionUpdate logic to handle null parameters. Revised the unit tests to test null values --------- Co-authored-by: cyprain-okeke Co-authored-by: Thomas Rittson Co-authored-by: Thomas Rittson <31796059+eliykat@users.noreply.github.com> * Add UsePasswordManager to sync data (#3114) * [AC-1522] Fix service account check on upgrading (#3111) * Create new query and add to save orgUser flow * Add tests * Resolved the checkmarx issues * [AC-1521] Address checkmarx security feedback (#3124) * Reinstate target attribute but add noopener noreferrer * Make same updates to service account adjustment code * Wire up to BulkEnableSecretsManager, delete separate command * Register new query * WIP: autoscaling in invite user flow * Resolve dependency issues * circular dependency between OrganizationService and UpdateSecretsManagerSubscriptionCommand - fixed by temporarily duplicating ReplaceAndUpdateCache * Unresolvable dependencies in other services - fixed by temporarily registering noop services and moving around some DI code All should be resolved in PM-1880 * fix using refs * Update date on migration script * Remove unused constant * Revert "Remove unused constant" This reverts commit 4fcb9da4d62af815c01579ab265d0ce11b47a9bb. This is required to make feature flags work on the client * Fix tests * Refactor: fix the update object and use it to adjust values * [PM-3177] Created OrganizationUserCommand and UpdateOrganizationUserGroupsCommand * [PM-3177] Added unit tests for UpdateOrganizationUserGroupsCommand * [PM-3177] Replaced IOrganizationService.UpdateUserGroupsAsync with IUpdateOrganizationUserGroupsCommand * [AC-1458] Add Endpoint And Service Logic for secrets manager to existing subscription (#3087) --------- Co-authored-by: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Co-authored-by: Thomas Rittson * Handle autoscaling-specific errors * Revert name change to method * Fix typo * Exclude beta users from seat limits * Fix inaccurate comment * Update nullable properties to use .Value accessor * Fix tests * Add missing awaits * Move early return up * Revert based on currentOrganization * Remove duplicate migrations from incorrectly resolved merge * Add tests * [PM-3177] Remove Admin project referencing Commercial.Infrastructure.EntityFramework * [PM-3177] Removed abstract class OrganizationUserCommand. Added ValidateOrganizationUserUpdatePermissions to IOrganizationService --------- Co-authored-by: Shane Melton Co-authored-by: Thomas Rittson Co-authored-by: cyprain-okeke <108260115+cyprain-okeke@users.noreply.github.com> Co-authored-by: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Co-authored-by: Colton Hurst Co-authored-by: cyprain-okeke Co-authored-by: Conner Turnbull --- .../ScimServiceCollectionExtensions.cs | 5 +- .../OrganizationUsersController.cs | 7 +- .../Public/Controllers/MembersController.cs | 8 ++- ...OrganizationServiceCollectionExtensions.cs | 7 ++ .../IUpdateOrganizationUserGroupsCommand.cs | 8 +++ .../UpdateOrganizationUserGroupsCommand.cs | 34 +++++++++ src/Core/Services/IOrganizationService.cs | 3 +- .../Implementations/OrganizationService.cs | 13 +--- ...pdateOrganizationUserGroupsCommandTests.cs | 50 +++++++++++++ .../Services/OrganizationServiceTests.cs | 70 +++++++++++++++++++ 10 files changed, 184 insertions(+), 21 deletions(-) create mode 100644 src/Core/OrganizationFeatures/OrganizationUsers/Interfaces/IUpdateOrganizationUserGroupsCommand.cs create mode 100644 src/Core/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserGroupsCommand.cs create mode 100644 test/Core.Test/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserGroupsCommandTests.cs diff --git a/bitwarden_license/src/Scim/Utilities/ScimServiceCollectionExtensions.cs b/bitwarden_license/src/Scim/Utilities/ScimServiceCollectionExtensions.cs index fff70760d4ee..75b60a71fc58 100644 --- a/bitwarden_license/src/Scim/Utilities/ScimServiceCollectionExtensions.cs +++ b/bitwarden_license/src/Scim/Utilities/ScimServiceCollectionExtensions.cs @@ -1,6 +1,4 @@ -using Bit.Core.OrganizationFeatures.OrganizationUsers; -using Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces; -using Bit.Scim.Groups; +using Bit.Scim.Groups; using Bit.Scim.Groups.Interfaces; using Bit.Scim.Users; using Bit.Scim.Users.Interfaces; @@ -23,7 +21,6 @@ public static void AddScimGroupQueries(this IServiceCollection services) public static void AddScimUserCommands(this IServiceCollection services) { - services.AddScoped(); services.AddScoped(); services.AddScoped(); } diff --git a/src/Api/Controllers/OrganizationUsersController.cs b/src/Api/Controllers/OrganizationUsersController.cs index 3dbaea3ff90e..d1c4ebacd601 100644 --- a/src/Api/Controllers/OrganizationUsersController.cs +++ b/src/Api/Controllers/OrganizationUsersController.cs @@ -30,6 +30,7 @@ public class OrganizationUsersController : Controller private readonly ICurrentContext _currentContext; private readonly ICountNewSmSeatsRequiredQuery _countNewSmSeatsRequiredQuery; private readonly IUpdateSecretsManagerSubscriptionCommand _updateSecretsManagerSubscriptionCommand; + private readonly IUpdateOrganizationUserGroupsCommand _updateOrganizationUserGroupsCommand; public OrganizationUsersController( IOrganizationRepository organizationRepository, @@ -41,7 +42,8 @@ public OrganizationUsersController( IPolicyRepository policyRepository, ICurrentContext currentContext, ICountNewSmSeatsRequiredQuery countNewSmSeatsRequiredQuery, - IUpdateSecretsManagerSubscriptionCommand updateSecretsManagerSubscriptionCommand) + IUpdateSecretsManagerSubscriptionCommand updateSecretsManagerSubscriptionCommand, + IUpdateOrganizationUserGroupsCommand updateOrganizationUserGroupsCommand) { _organizationRepository = organizationRepository; _organizationUserRepository = organizationUserRepository; @@ -53,6 +55,7 @@ public OrganizationUsersController( _currentContext = currentContext; _countNewSmSeatsRequiredQuery = countNewSmSeatsRequiredQuery; _updateSecretsManagerSubscriptionCommand = updateSecretsManagerSubscriptionCommand; + _updateOrganizationUserGroupsCommand = updateOrganizationUserGroupsCommand; } [HttpGet("{id}")] @@ -308,7 +311,7 @@ public async Task PutGroups(string orgId, string id, [FromBody] OrganizationUser } var loggedInUserId = _userService.GetProperUserId(User); - await _organizationService.UpdateUserGroupsAsync(organizationUser, model.GroupIds.Select(g => new Guid(g)), loggedInUserId); + await _updateOrganizationUserGroupsCommand.UpdateUserGroupsAsync(organizationUser, model.GroupIds.Select(g => new Guid(g)), loggedInUserId); } [HttpPut("{userId}/reset-password-enrollment")] diff --git a/src/Api/Public/Controllers/MembersController.cs b/src/Api/Public/Controllers/MembersController.cs index 9d7ca86d3b9a..74dd4b83e10b 100644 --- a/src/Api/Public/Controllers/MembersController.cs +++ b/src/Api/Public/Controllers/MembersController.cs @@ -3,6 +3,7 @@ using Bit.Api.Models.Public.Response; using Bit.Core.Context; using Bit.Core.Models.Business; +using Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.Repositories; using Bit.Core.Services; using Microsoft.AspNetCore.Authorization; @@ -19,19 +20,22 @@ public class MembersController : Controller private readonly IOrganizationService _organizationService; private readonly IUserService _userService; private readonly ICurrentContext _currentContext; + private readonly IUpdateOrganizationUserGroupsCommand _updateOrganizationUserGroupsCommand; public MembersController( IOrganizationUserRepository organizationUserRepository, IGroupRepository groupRepository, IOrganizationService organizationService, IUserService userService, - ICurrentContext currentContext) + ICurrentContext currentContext, + IUpdateOrganizationUserGroupsCommand updateOrganizationUserGroupsCommand) { _organizationUserRepository = organizationUserRepository; _groupRepository = groupRepository; _organizationService = organizationService; _userService = userService; _currentContext = currentContext; + _updateOrganizationUserGroupsCommand = updateOrganizationUserGroupsCommand; } /// @@ -183,7 +187,7 @@ public async Task PutGroupIds(Guid id, [FromBody] UpdateGroupIdsR { return new NotFoundResult(); } - await _organizationService.UpdateUserGroupsAsync(existingUser, model.GroupIds, null); + await _updateOrganizationUserGroupsCommand.UpdateUserGroupsAsync(existingUser, model.GroupIds, null); return new OkResult(); } diff --git a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs index a534ed5bea2d..1756149236a3 100644 --- a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs +++ b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs @@ -45,6 +45,7 @@ public static void AddOrganizationServices(this IServiceCollection services, IGl services.AddOrganizationLicenseCommandsQueries(); services.AddOrganizationDomainCommandsQueries(); services.AddOrganizationAuthCommands(); + services.AddOrganizationUserCommands(); services.AddOrganizationUserCommandsQueries(); services.AddBaseOrganizationSubscriptionCommandsQueries(); } @@ -81,6 +82,12 @@ private static void AddOrganizationSponsorshipCommands(this IServiceCollection s } } + private static void AddOrganizationUserCommands(this IServiceCollection services) + { + services.AddScoped(); + services.AddScoped(); + } + private static void AddOrganizationApiKeyCommandsQueries(this IServiceCollection services) { services.AddScoped(); diff --git a/src/Core/OrganizationFeatures/OrganizationUsers/Interfaces/IUpdateOrganizationUserGroupsCommand.cs b/src/Core/OrganizationFeatures/OrganizationUsers/Interfaces/IUpdateOrganizationUserGroupsCommand.cs new file mode 100644 index 000000000000..9dae9323349c --- /dev/null +++ b/src/Core/OrganizationFeatures/OrganizationUsers/Interfaces/IUpdateOrganizationUserGroupsCommand.cs @@ -0,0 +1,8 @@ +using Bit.Core.Entities; + +namespace Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces; + +public interface IUpdateOrganizationUserGroupsCommand +{ + Task UpdateUserGroupsAsync(OrganizationUser organizationUser, IEnumerable groupIds, Guid? loggedInUserId); +} diff --git a/src/Core/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserGroupsCommand.cs b/src/Core/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserGroupsCommand.cs new file mode 100644 index 000000000000..7bb5814d6e1f --- /dev/null +++ b/src/Core/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserGroupsCommand.cs @@ -0,0 +1,34 @@ +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.OrganizationFeatures.OrganizationUsers.Interfaces; +using Bit.Core.Repositories; +using Bit.Core.Services; + +namespace Bit.Core.OrganizationFeatures.OrganizationUsers; + +public class UpdateOrganizationUserGroupsCommand : IUpdateOrganizationUserGroupsCommand +{ + private readonly IEventService _eventService; + private readonly IOrganizationService _organizationService; + private readonly IOrganizationUserRepository _organizationUserRepository; + + public UpdateOrganizationUserGroupsCommand( + IEventService eventService, + IOrganizationService organizationService, + IOrganizationUserRepository organizationUserRepository) + { + _eventService = eventService; + _organizationService = organizationService; + _organizationUserRepository = organizationUserRepository; + } + + public async Task UpdateUserGroupsAsync(OrganizationUser organizationUser, IEnumerable groupIds, Guid? loggedInUserId) + { + if (loggedInUserId.HasValue) + { + await _organizationService.ValidateOrganizationUserUpdatePermissions(organizationUser.OrganizationId, organizationUser.Type, null, organizationUser.GetPermissions()); + } + await _organizationUserRepository.UpdateGroupsAsync(organizationUser.Id, groupIds); + await _eventService.LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_UpdatedGroups); + } +} diff --git a/src/Core/Services/IOrganizationService.cs b/src/Core/Services/IOrganizationService.cs index 832aa9de4c1f..e085d0453105 100644 --- a/src/Core/Services/IOrganizationService.cs +++ b/src/Core/Services/IOrganizationService.cs @@ -54,7 +54,6 @@ Task>> ConfirmUsersAsync(Guid organizationI Task DeleteUserAsync(Guid organizationId, Guid userId); Task>> DeleteUsersAsync(Guid organizationId, IEnumerable organizationUserIds, Guid? deletingUserId); - Task UpdateUserGroupsAsync(OrganizationUser organizationUser, IEnumerable groupIds, Guid? loggedInUserId); Task UpdateUserResetPasswordEnrollmentAsync(Guid organizationId, Guid userId, string resetPasswordKey, Guid? callingUserId); Task ImportAsync(Guid organizationId, Guid? importingUserId, IEnumerable groups, IEnumerable newUsers, IEnumerable removeUserExternalIds, @@ -82,4 +81,6 @@ Task>> RestoreUsersAsync(Guid organizationI void ValidatePasswordManagerPlan(Models.StaticStore.Plan plan, OrganizationUpgrade upgrade); void ValidateSecretsManagerPlan(Models.StaticStore.Plan plan, OrganizationUpgrade upgrade); + Task ValidateOrganizationUserUpdatePermissions(Guid organizationId, OrganizationUserType newType, + OrganizationUserType? oldType, Permissions permissions); } diff --git a/src/Core/Services/Implementations/OrganizationService.cs b/src/Core/Services/Implementations/OrganizationService.cs index b7424a53e101..d9d12d1f9fdd 100644 --- a/src/Core/Services/Implementations/OrganizationService.cs +++ b/src/Core/Services/Implementations/OrganizationService.cs @@ -1582,17 +1582,6 @@ public async Task HasConfirmedOwnersExceptAsync(Guid organizationId, IEnum return hasOtherOwner; } - public async Task UpdateUserGroupsAsync(OrganizationUser organizationUser, IEnumerable groupIds, Guid? loggedInUserId) - { - if (loggedInUserId.HasValue) - { - await ValidateOrganizationUserUpdatePermissions(organizationUser.OrganizationId, organizationUser.Type, null, organizationUser.GetPermissions()); - } - await _organizationUserRepository.UpdateGroupsAsync(organizationUser.Id, groupIds); - await _eventService.LogOrganizationUserEventAsync(organizationUser, - EventType.OrganizationUser_UpdatedGroups); - } - public async Task UpdateUserResetPasswordEnrollmentAsync(Guid organizationId, Guid userId, string resetPasswordKey, Guid? callingUserId) { // Org User must be the same as the calling user and the organization ID associated with the user must match passed org ID @@ -2032,7 +2021,7 @@ public void ValidateSecretsManagerPlan(Models.StaticStore.Plan plan, Organizatio } } - private async Task ValidateOrganizationUserUpdatePermissions(Guid organizationId, OrganizationUserType newType, OrganizationUserType? oldType, Permissions permissions) + public async Task ValidateOrganizationUserUpdatePermissions(Guid organizationId, OrganizationUserType newType, OrganizationUserType? oldType, Permissions permissions) { if (await _currentContext.OrganizationOwner(organizationId)) { diff --git a/test/Core.Test/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserGroupsCommandTests.cs b/test/Core.Test/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserGroupsCommandTests.cs new file mode 100644 index 000000000000..542220560ca6 --- /dev/null +++ b/test/Core.Test/OrganizationFeatures/OrganizationUsers/UpdateOrganizationUserGroupsCommandTests.cs @@ -0,0 +1,50 @@ +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.OrganizationFeatures.OrganizationUsers; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using Xunit; + +namespace Bit.Core.Test.OrganizationFeatures.OrganizationUsers; + +[SutProviderCustomize] +public class UpdateOrganizationUserGroupsCommandTests +{ + [Theory, BitAutoData] + public async Task UpdateUserGroups_Passes( + OrganizationUser organizationUser, + IEnumerable groupIds, + SutProvider sutProvider) + { + await sutProvider.Sut.UpdateUserGroupsAsync(organizationUser, groupIds, null); + + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .ValidateOrganizationUserUpdatePermissions(default, default, default, default); + await sutProvider.GetDependency().Received(1) + .UpdateGroupsAsync(organizationUser.Id, groupIds); + await sutProvider.GetDependency().Received(1) + .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_UpdatedGroups); + } + + [Theory, BitAutoData] + public async Task UpdateUserGroups_WithSavingUserId_Passes( + OrganizationUser organizationUser, + IEnumerable groupIds, + Guid savingUserId, + SutProvider sutProvider) + { + organizationUser.Permissions = null; + + await sutProvider.Sut.UpdateUserGroupsAsync(organizationUser, groupIds, savingUserId); + + await sutProvider.GetDependency().Received(1) + .ValidateOrganizationUserUpdatePermissions(organizationUser.OrganizationId, organizationUser.Type, null, organizationUser.GetPermissions()); + await sutProvider.GetDependency().Received(1) + .UpdateGroupsAsync(organizationUser.Id, groupIds); + await sutProvider.GetDependency().Received(1) + .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_UpdatedGroups); + } +} diff --git a/test/Core.Test/Services/OrganizationServiceTests.cs b/test/Core.Test/Services/OrganizationServiceTests.cs index 01073f4d2e4a..0389407da119 100644 --- a/test/Core.Test/Services/OrganizationServiceTests.cs +++ b/test/Core.Test/Services/OrganizationServiceTests.cs @@ -1832,4 +1832,74 @@ public void ValidateSecretsManagerPlan_ValidPlan_NoExceptionThrown( sutProvider.Sut.ValidateSecretsManagerPlan(plan, signup); } + + [Theory] + [OrganizationInviteCustomize( + InviteeUserType = OrganizationUserType.Owner, + InvitorUserType = OrganizationUserType.Admin + ), BitAutoData] + public async Task ValidateOrganizationUserUpdatePermissions_WithAdminAddingOwner_Throws( + Guid organizationId, + OrganizationUserInvite organizationUserInvite, + SutProvider sutProvider) + { + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.ValidateOrganizationUserUpdatePermissions(organizationId, organizationUserInvite.Type.Value, null, organizationUserInvite.Permissions)); + + Assert.Contains("only an owner can configure another owner's account.", exception.Message.ToLowerInvariant()); + } + + [Theory] + [OrganizationInviteCustomize( + InviteeUserType = OrganizationUserType.Admin, + InvitorUserType = OrganizationUserType.Owner + ), BitAutoData] + public async Task ValidateOrganizationUserUpdatePermissions_WithoutManageUsersPermission_Throws( + Guid organizationId, + OrganizationUserInvite organizationUserInvite, + SutProvider sutProvider) + { + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.ValidateOrganizationUserUpdatePermissions(organizationId, organizationUserInvite.Type.Value, null, organizationUserInvite.Permissions)); + + Assert.Contains("your account does not have permission to manage users.", exception.Message.ToLowerInvariant()); + } + + [Theory] + [OrganizationInviteCustomize( + InviteeUserType = OrganizationUserType.Admin, + InvitorUserType = OrganizationUserType.Custom + ), BitAutoData] + public async Task ValidateOrganizationUserUpdatePermissions_WithCustomAddingAdmin_Throws( + Guid organizationId, + OrganizationUserInvite organizationUserInvite, + SutProvider sutProvider) + { + sutProvider.GetDependency().ManageUsers(organizationId).Returns(true); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.ValidateOrganizationUserUpdatePermissions(organizationId, organizationUserInvite.Type.Value, null, organizationUserInvite.Permissions)); + + Assert.Contains("custom users can not manage admins or owners.", exception.Message.ToLowerInvariant()); + } + + [Theory] + [OrganizationInviteCustomize( + InviteeUserType = OrganizationUserType.Custom, + InvitorUserType = OrganizationUserType.Custom + ), BitAutoData] + public async Task ValidateOrganizationUserUpdatePermissions_WithCustomAddingUser_WithoutPermissions_Throws( + Guid organizationId, + OrganizationUserInvite organizationUserInvite, + SutProvider sutProvider) + { + var invitePermissions = new Permissions { AccessReports = true }; + sutProvider.GetDependency().ManageUsers(organizationId).Returns(true); + sutProvider.GetDependency().AccessReports(organizationId).Returns(false); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.ValidateOrganizationUserUpdatePermissions(organizationId, organizationUserInvite.Type.Value, null, invitePermissions)); + + Assert.Contains("custom users can only grant the same custom permissions that they have.", exception.Message.ToLowerInvariant()); + } }