diff --git a/src/Api/Controllers/OrganizationsController.cs b/src/Api/Controllers/OrganizationsController.cs index 3259fd50c4..0c02a929fb 100644 --- a/src/Api/Controllers/OrganizationsController.cs +++ b/src/Api/Controllers/OrganizationsController.cs @@ -552,7 +552,7 @@ namespace Bit.Api.Controllers throw new UnauthorizedAccessException(); } - var org = await _organizationService.UpdateOrganizationKeysAsync(user.Id, new Guid(id), model.PublicKey, model.EncryptedPrivateKey); + var org = await _organizationService.UpdateOrganizationKeysAsync(new Guid(id), model.PublicKey, model.EncryptedPrivateKey); return new OrganizationKeysResponseModel(org); } } diff --git a/src/Core/Services/IOrganizationService.cs b/src/Core/Services/IOrganizationService.cs index 7a5b908224..e8b8419ed1 100644 --- a/src/Core/Services/IOrganizationService.cs +++ b/src/Core/Services/IOrganizationService.cs @@ -58,6 +58,6 @@ namespace Bit.Core.Services bool overwriteExisting); Task RotateApiKeyAsync(Organization organization); Task DeleteSsoUserAsync(Guid userId, Guid? organizationId); - Task UpdateOrganizationKeysAsync(Guid userId, Guid orgId, string publicKey, string privateKey); + Task UpdateOrganizationKeysAsync(Guid orgId, string publicKey, string privateKey); } } diff --git a/src/Core/Services/Implementations/OrganizationService.cs b/src/Core/Services/Implementations/OrganizationService.cs index 070c665757..23d7fe214e 100644 --- a/src/Core/Services/Implementations/OrganizationService.cs +++ b/src/Core/Services/Implementations/OrganizationService.cs @@ -15,7 +15,7 @@ using Bit.Core.Settings; using System.IO; using Newtonsoft.Json; using System.Text.Json; -using Bit.Core.Models.Api; +using Bit.Core.Context; namespace Bit.Core.Services { @@ -42,6 +42,7 @@ namespace Bit.Core.Services private readonly IReferenceEventService _referenceEventService; private readonly GlobalSettings _globalSettings; private readonly ITaxRateRepository _taxRateRepository; + private readonly ICurrentContext _currentContext; public OrganizationService( IOrganizationRepository organizationRepository, @@ -64,7 +65,8 @@ namespace Bit.Core.Services ISsoUserRepository ssoUserRepository, IReferenceEventService referenceEventService, GlobalSettings globalSettings, - ITaxRateRepository taxRateRepository) + ITaxRateRepository taxRateRepository, + ICurrentContext currentContext) { _organizationRepository = organizationRepository; _organizationUserRepository = organizationUserRepository; @@ -87,6 +89,7 @@ namespace Bit.Core.Services _referenceEventService = referenceEventService; _globalSettings = globalSettings; _taxRateRepository = taxRateRepository; + _currentContext = currentContext; } public async Task ReplacePaymentMethodAsync(Guid organizationId, string paymentToken, @@ -1048,7 +1051,7 @@ namespace Bit.Core.Services { foreach (var type in inviteTypes) { - await ValidateOrganizationUserUpdatePermissionsAsync(invitingUserId.Value, organizationId, type, null); + ValidateOrganizationUserUpdatePermissions(organizationId, type, null); } } @@ -1155,7 +1158,7 @@ namespace Bit.Core.Services if (invitingUserId.HasValue && invite.Type.HasValue) { - await ValidateOrganizationUserUpdatePermissionsAsync(invitingUserId.Value, organizationId, invite.Type.Value, null); + ValidateOrganizationUserUpdatePermissions(organizationId, invite.Type.Value, null); } if (organization.Seats.HasValue) @@ -1529,7 +1532,7 @@ namespace Bit.Core.Services if (savingUserId.HasValue) { - await ValidateOrganizationUserUpdatePermissionsAsync(savingUserId.Value, user.OrganizationId, user.Type, originalUser.Type); + ValidateOrganizationUserUpdatePermissions(user.OrganizationId, user.Type, originalUser.Type); } if (user.Type != OrganizationUserType.Owner && @@ -1561,7 +1564,7 @@ namespace Bit.Core.Services } if (orgUser.Type == OrganizationUserType.Owner && deletingUserId.HasValue && - !await UserIsOwnerAsync(organizationId, deletingUserId.Value)) + !_currentContext.OrganizationOwner(organizationId)) { throw new BadRequestException("Only owners can delete other owners."); } @@ -1623,7 +1626,7 @@ namespace Bit.Core.Services var deletingUserIsOwner = false; if (deletingUserId.HasValue) { - deletingUserIsOwner = await UserIsOwnerAsync(organizationId, deletingUserId.Value); + deletingUserIsOwner = _currentContext.OrganizationOwner(organizationId); } var result = new List>(); @@ -1669,17 +1672,11 @@ namespace Bit.Core.Services return confirmedOwnersIds.Except(organizationUsersId).Any(); } - private async Task UserIsOwnerAsync(Guid organizationId, Guid deletingUserId) - { - var deletingUserOrgs = await _organizationUserRepository.GetManyByUserAsync(deletingUserId); - return deletingUserOrgs.Any(u => u.OrganizationId == organizationId && u.Type == OrganizationUserType.Owner); - } - public async Task UpdateUserGroupsAsync(OrganizationUser organizationUser, IEnumerable groupIds, Guid? loggedInUserId) { if (loggedInUserId.HasValue) { - await ValidateOrganizationUserUpdatePermissionsAsync(loggedInUserId.Value, organizationUser.OrganizationId, organizationUser.Type, null); + ValidateOrganizationUserUpdatePermissions(organizationUser.OrganizationId, organizationUser.Type, null); } await _organizationUserRepository.UpdateGroupsAsync(organizationUser.Id, groupIds); await _eventService.LogOrganizationUserEventAsync(organizationUser, @@ -1962,25 +1959,13 @@ namespace Bit.Core.Services } } - public async Task UpdateOrganizationKeysAsync(Guid userId, Guid orgId, string publicKey, string privateKey) + public async Task UpdateOrganizationKeysAsync(Guid orgId, string publicKey, string privateKey) { - // Only Owners/Admins/Custom (w/ ManageResetPassword) can create org keys - var orgUser = await _organizationUserRepository.GetDetailsByUserAsync(userId, orgId); - if (orgUser == null || orgUser.Type != OrganizationUserType.Admin && - orgUser.Type != OrganizationUserType.Owner && orgUser.Type != OrganizationUserType.Custom) + if (_currentContext.ManageResetPassword(orgId)) { throw new UnauthorizedAccessException(); } - if (orgUser.Type == OrganizationUserType.Custom) - { - var permissions = CoreHelpers.LoadClassFromJsonData(orgUser.Permissions); - if (permissions == null || !permissions.ManageResetPassword) - { - throw new UnauthorizedAccessException(); - } - } - // If the keys already exist, error out var org = await _organizationRepository.GetByIdAsync(orgId); if (org.PublicKey != null && org.PrivateKey != null) @@ -2087,56 +2072,35 @@ namespace Bit.Core.Services } } - private async Task ValidateOrganizationUserUpdatePermissionsAsync(Guid loggedInUserId, Guid organizationId, - OrganizationUserType newType, OrganizationUserType? oldType) + private void ValidateOrganizationUserUpdatePermissions(Guid organizationId, OrganizationUserType newType, + OrganizationUserType? oldType) { - var loggedInUserOrgs = await _organizationUserRepository.GetManyByUserAsync(loggedInUserId); - var loggedInAsOrgOwner = loggedInUserOrgs - .Any(u => u.OrganizationId == organizationId && u.Type == OrganizationUserType.Owner); - if (loggedInAsOrgOwner) + if (_currentContext.OrganizationOwner(organizationId)) { return; } - var isOwner = oldType == OrganizationUserType.Owner; - var nowOwner = newType == OrganizationUserType.Owner; - var ownerUserConfigurationAttempt = (isOwner && nowOwner) || !(isOwner.Equals(nowOwner)); - if (ownerUserConfigurationAttempt) + if (oldType == OrganizationUserType.Owner || newType == OrganizationUserType.Owner) { throw new BadRequestException("Only an Owner can configure another Owner's account."); } - var loggedInAsOrgAdmin = loggedInUserOrgs.Any(u => u.OrganizationId == organizationId && u.Type == OrganizationUserType.Admin); - if (loggedInAsOrgAdmin) + if (_currentContext.OrganizationAdmin(organizationId)) { return; } - var isCustom = oldType == OrganizationUserType.Custom; - var nowCustom = newType == OrganizationUserType.Custom; - var customUserConfigurationAttempt = (isCustom && nowCustom) || !(isCustom.Equals(nowCustom)); - if (customUserConfigurationAttempt) + if (oldType == OrganizationUserType.Custom || newType == OrganizationUserType.Custom) { throw new BadRequestException("Only Owners and Admins can configure Custom accounts."); } - var loggedInAsOrgCustom = loggedInUserOrgs.Any(u => u.OrganizationId == organizationId && u.Type == OrganizationUserType.Custom); - if (!loggedInAsOrgCustom) - { - return; - } - - var loggedInCustomOrgUser = loggedInUserOrgs.First(u => u.OrganizationId == organizationId && u.Type == OrganizationUserType.Custom); - var loggedInUserPermissions = CoreHelpers.LoadClassFromJsonData(loggedInCustomOrgUser.Permissions); - if (!loggedInUserPermissions.ManageUsers) + if (!_currentContext.ManageUsers(organizationId)) { throw new BadRequestException("Your account does not have permission to manage users."); } - var isAdmin = oldType == OrganizationUserType.Admin; - var nowAdmin = newType == OrganizationUserType.Admin; - var adminUserConfigurationAttempt = (isAdmin && nowAdmin) || !(isAdmin.Equals(nowAdmin)); - if (adminUserConfigurationAttempt) + if (oldType == OrganizationUserType.Admin || newType == OrganizationUserType.Admin) { throw new BadRequestException("Custom users can not manage Admins or Owners."); } diff --git a/test/Core.Test/Services/OrganizationServiceTests.cs b/test/Core.Test/Services/OrganizationServiceTests.cs index 9576f576ff..bbb93af51a 100644 --- a/test/Core.Test/Services/OrganizationServiceTests.cs +++ b/test/Core.Test/Services/OrganizationServiceTests.cs @@ -15,6 +15,7 @@ using Bit.Core.Enums; using Bit.Core.Test.AutoFixture.Attributes; using Bit.Core.Test.AutoFixture.OrganizationFixtures; using System.Text.Json; +using Bit.Core.Context; using Bit.Core.Test.AutoFixture.OrganizationUserFixtures; using Organization = Bit.Core.Models.Table.Organization; using OrganizationUser = Bit.Core.Models.Table.OrganizationUser; @@ -43,6 +44,7 @@ namespace Bit.Core.Test.Services .Returns(existingUsers); sutProvider.GetDependency().GetCountByOrganizationIdAsync(org.Id) .Returns(existingUsers.Count); + sutProvider.GetDependency().ManageUsers(org.Id).Returns(true); await sutProvider.Sut.ImportAsync(org.Id, userId, null, newUsers, null, false); @@ -92,6 +94,8 @@ namespace Bit.Core.Test.Services .Returns(existingUsers.Count); sutProvider.GetDependency().GetByIdAsync(reInvitedUser.Id) .Returns(new OrganizationUser { Id = reInvitedUser.Id }); + var currentContext = sutProvider.GetDependency(); + currentContext.ManageUsers(org.Id).Returns(true); await sutProvider.Sut.ImportAsync(org.Id, userId, null, newUsers, null, false); @@ -194,10 +198,10 @@ namespace Bit.Core.Test.Services OrganizationUser invitor, SutProvider sutProvider) { var organizationRepository = sutProvider.GetDependency(); - var organizationUserRepository = sutProvider.GetDependency(); + var currentContext = sutProvider.GetDependency(); organizationRepository.GetByIdAsync(organization.Id).Returns(organization); - organizationUserRepository.GetManyByUserAsync(invitor.Id).Returns(new List { invitor }); + currentContext.OrganizationAdmin(organization.Id).Returns(true); var exception = await Assert.ThrowsAsync( () => sutProvider.Sut.InviteUserAsync(organization.Id, invitor.UserId, null, invite)); @@ -207,16 +211,16 @@ namespace Bit.Core.Test.Services [Theory] [OrganizationInviteAutoData( inviteeUserType: (int)OrganizationUserType.Custom, - invitorUserType: (int)OrganizationUserType.Admin + invitorUserType: (int)OrganizationUserType.User )] public async Task InviteUser_NonAdminConfiguringAdmin_Throws(Organization organization, OrganizationUserInvite invite, OrganizationUser invitor, SutProvider sutProvider) { var organizationRepository = sutProvider.GetDependency(); - var organizationUserRepository = sutProvider.GetDependency(); + var currentContext = sutProvider.GetDependency(); organizationRepository.GetByIdAsync(organization.Id).Returns(organization); - organizationUserRepository.GetManyByUserAsync(invitor.Id).Returns(new List { invitor }); + currentContext.OrganizationUser(organization.Id).Returns(true); var exception = await Assert.ThrowsAsync( () => sutProvider.Sut.InviteUserAsync(organization.Id, invitor.UserId, null, invite)); @@ -238,10 +242,11 @@ namespace Bit.Core.Test.Services }); var organizationRepository = sutProvider.GetDependency(); - var organizationUserRepository = sutProvider.GetDependency(); + var currentContext = sutProvider.GetDependency(); organizationRepository.GetByIdAsync(organization.Id).Returns(organization); - organizationUserRepository.GetManyByUserAsync(invitor.UserId.Value).Returns(new List { invitor }); + currentContext.OrganizationCustom(organization.Id).Returns(true); + currentContext.ManageUsers(organization.Id).Returns(false); var exception = await Assert.ThrowsAsync( () => sutProvider.Sut.InviteUserAsync(organization.Id, invitor.UserId, null, invite)); @@ -263,10 +268,11 @@ namespace Bit.Core.Test.Services }); var organizationRepository = sutProvider.GetDependency(); - var organizationUserRepository = sutProvider.GetDependency(); + var currentContext = sutProvider.GetDependency(); organizationRepository.GetByIdAsync(organization.Id).Returns(organization); - organizationUserRepository.GetManyByUserAsync(invitor.UserId.Value).Returns(new List { invitor }); + currentContext.OrganizationCustom(organization.Id).Returns(true); + currentContext.ManageUsers(organization.Id).Returns(true); var exception = await Assert.ThrowsAsync( () => sutProvider.Sut.InviteUserAsync(organization.Id, invitor.UserId, null, invite)); @@ -283,11 +289,10 @@ namespace Bit.Core.Test.Services { invite.Permissions = null; var organizationRepository = sutProvider.GetDependency(); - var organizationUserRepository = sutProvider.GetDependency(); - var eventService = sutProvider.GetDependency(); + var currentContext = sutProvider.GetDependency(); organizationRepository.GetByIdAsync(organization.Id).Returns(organization); - organizationUserRepository.GetManyByUserAsync(invitor.UserId.Value).Returns(new List { invitor }); + currentContext.OrganizationOwner(organization.Id).Returns(true); await sutProvider.Sut.InviteUserAsync(organization.Id, invitor.UserId, null, invite); } @@ -307,11 +312,10 @@ namespace Bit.Core.Test.Services }); var organizationRepository = sutProvider.GetDependency(); - var organizationUserRepository = sutProvider.GetDependency(); - var eventService = sutProvider.GetDependency(); + var currentContext = sutProvider.GetDependency(); organizationRepository.GetByIdAsync(organization.Id).Returns(organization); - organizationUserRepository.GetManyByUserAsync(invitor.UserId.Value).Returns(new List { invitor }); + currentContext.ManageUsers(organization.Id).Returns(true); await sutProvider.Sut.InviteUserAsync(organization.Id, invitor.UserId, null, invite); } @@ -346,6 +350,7 @@ namespace Bit.Core.Test.Services SutProvider sutProvider) { var organizationUserRepository = sutProvider.GetDependency(); + var currentContext = sutProvider.GetDependency(); newUserData.Id = oldUserData.Id; newUserData.UserId = oldUserData.UserId; @@ -353,7 +358,7 @@ namespace Bit.Core.Test.Services organizationUserRepository.GetByIdAsync(oldUserData.Id).Returns(oldUserData); organizationUserRepository.GetManyByOrganizationAsync(savingUser.OrganizationId, OrganizationUserType.Owner) .Returns(new List { savingUser }); - organizationUserRepository.GetManyByUserAsync(savingUser.UserId.Value).Returns(new List { savingUser }); + currentContext.OrganizationOwner(savingUser.OrganizationId).Returns(true); await sutProvider.Sut.SaveUserAsync(newUserData, savingUser.UserId, collections); } @@ -390,10 +395,11 @@ namespace Bit.Core.Test.Services SutProvider sutProvider) { var organizationUserRepository = sutProvider.GetDependency(); + var currentContext = sutProvider.GetDependency(); organizationUser.OrganizationId = deletingUser.OrganizationId; organizationUserRepository.GetByIdAsync(organizationUser.Id).Returns(organizationUser); - organizationUserRepository.GetManyByUserAsync(deletingUser.UserId.Value).Returns(new[] { deletingUser }); + currentContext.OrganizationAdmin(deletingUser.OrganizationId).Returns(true); var exception = await Assert.ThrowsAsync( () => sutProvider.Sut.DeleteUserAsync(deletingUser.OrganizationId, organizationUser.Id, deletingUser.UserId)); @@ -425,13 +431,14 @@ namespace Bit.Core.Test.Services SutProvider sutProvider) { var organizationUserRepository = sutProvider.GetDependency(); + var currentContext = sutProvider.GetDependency(); organizationUser.OrganizationId = deletingUser.OrganizationId; organizationUserRepository.GetByIdAsync(organizationUser.Id).Returns(organizationUser); organizationUserRepository.GetByIdAsync(deletingUser.Id).Returns(deletingUser); - organizationUserRepository.GetManyByUserAsync(deletingUser.UserId.Value).Returns(new[] { deletingUser }); organizationUserRepository.GetManyByOrganizationAsync(deletingUser.OrganizationId, OrganizationUserType.Owner) .Returns(new[] {deletingUser, organizationUser}); + currentContext.OrganizationOwner(deletingUser.OrganizationId).Returns(true); await sutProvider.Sut.DeleteUserAsync(deletingUser.OrganizationId, organizationUser.Id, deletingUser.UserId); } @@ -509,15 +516,16 @@ namespace Bit.Core.Test.Services SutProvider sutProvider) { var organizationUserRepository = sutProvider.GetDependency(); - + var currentContext = sutProvider.GetDependency(); + orgUser1.OrganizationId = orgUser2.OrganizationId = deletingUser.OrganizationId; var organizationUsers = new[] { orgUser1, orgUser2 }; var organizationUserIds = organizationUsers.Select(u => u.Id); organizationUserRepository.GetManyAsync(default).ReturnsForAnyArgs(organizationUsers); organizationUserRepository.GetByIdAsync(deletingUser.Id).Returns(deletingUser); - organizationUserRepository.GetManyByUserAsync(deletingUser.UserId.Value).Returns(new[] { deletingUser }); organizationUserRepository.GetManyByOrganizationAsync(deletingUser.OrganizationId, OrganizationUserType.Owner) .Returns(new[] {deletingUser, orgUser1}); + currentContext.OrganizationOwner(deletingUser.OrganizationId).Returns(true); await sutProvider.Sut.DeleteUsersAsync(deletingUser.OrganizationId, organizationUserIds, deletingUser.UserId); }