diff --git a/bitwarden_license/src/Scim/Utilities/ScimServiceCollectionExtensions.cs b/bitwarden_license/src/Scim/Utilities/ScimServiceCollectionExtensions.cs index fff70760d..75b60a71f 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 class ScimServiceCollectionExtensions 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 3dbaea3ff..d1c4ebacd 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 class OrganizationUsersController : Controller IPolicyRepository policyRepository, ICurrentContext currentContext, ICountNewSmSeatsRequiredQuery countNewSmSeatsRequiredQuery, - IUpdateSecretsManagerSubscriptionCommand updateSecretsManagerSubscriptionCommand) + IUpdateSecretsManagerSubscriptionCommand updateSecretsManagerSubscriptionCommand, + IUpdateOrganizationUserGroupsCommand updateOrganizationUserGroupsCommand) { _organizationRepository = organizationRepository; _organizationUserRepository = organizationUserRepository; @@ -53,6 +55,7 @@ public class OrganizationUsersController : Controller _currentContext = currentContext; _countNewSmSeatsRequiredQuery = countNewSmSeatsRequiredQuery; _updateSecretsManagerSubscriptionCommand = updateSecretsManagerSubscriptionCommand; + _updateOrganizationUserGroupsCommand = updateOrganizationUserGroupsCommand; } [HttpGet("{id}")] @@ -308,7 +311,7 @@ public class OrganizationUsersController : Controller } 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 9d7ca86d3..74dd4b83e 100644 --- a/src/Api/Public/Controllers/MembersController.cs +++ b/src/Api/Public/Controllers/MembersController.cs @@ -3,6 +3,7 @@ using Bit.Api.Models.Public.Request; 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 class MembersController : Controller { 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 a534ed5be..175614923 100644 --- a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs +++ b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs @@ -45,6 +45,7 @@ public static class OrganizationServiceCollectionExtensions services.AddOrganizationLicenseCommandsQueries(); services.AddOrganizationDomainCommandsQueries(); services.AddOrganizationAuthCommands(); + services.AddOrganizationUserCommands(); services.AddOrganizationUserCommandsQueries(); services.AddBaseOrganizationSubscriptionCommandsQueries(); } @@ -81,6 +82,12 @@ public static class OrganizationServiceCollectionExtensions } } + 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 000000000..9dae93233 --- /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 000000000..7bb5814d6 --- /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 832aa9de4..e085d0453 100644 --- a/src/Core/Services/IOrganizationService.cs +++ b/src/Core/Services/IOrganizationService.cs @@ -54,7 +54,6 @@ public interface IOrganizationService 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 @@ public interface IOrganizationService 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 b7424a53e..d9d12d1f9 100644 --- a/src/Core/Services/Implementations/OrganizationService.cs +++ b/src/Core/Services/Implementations/OrganizationService.cs @@ -1582,17 +1582,6 @@ public class OrganizationService : IOrganizationService 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 class OrganizationService : IOrganizationService } } - 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 000000000..542220560 --- /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 01073f4d2..0389407da 100644 --- a/test/Core.Test/Services/OrganizationServiceTests.cs +++ b/test/Core.Test/Services/OrganizationServiceTests.cs @@ -1832,4 +1832,74 @@ public class OrganizationServiceTests 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()); + } }