diff --git a/src/Api/Models/Request/Organizations/OrganizationUserRequestModels.cs b/src/Api/Models/Request/Organizations/OrganizationUserRequestModels.cs index cef7721b1..92044c455 100644 --- a/src/Api/Models/Request/Organizations/OrganizationUserRequestModels.cs +++ b/src/Api/Models/Request/Organizations/OrganizationUserRequestModels.cs @@ -1,5 +1,4 @@ using System.ComponentModel.DataAnnotations; -using System.Text.Json; using Bit.Api.Auth.Models.Request.Accounts; using Bit.Core.Entities; using Bit.Core.Enums; @@ -96,10 +95,7 @@ public class OrganizationUserUpdateRequestModel public OrganizationUser ToOrganizationUser(OrganizationUser existingUser) { existingUser.Type = Type.Value; - existingUser.Permissions = JsonSerializer.Serialize(Permissions, new JsonSerializerOptions - { - PropertyNamingPolicy = JsonNamingPolicy.CamelCase, - }); + existingUser.Permissions = CoreHelpers.ClassToJsonData(Permissions); existingUser.AccessAll = AccessAll; existingUser.AccessSecretsManager = AccessSecretsManager; return existingUser; diff --git a/src/Core/Entities/OrganizationUser.cs b/src/Core/Entities/OrganizationUser.cs index 9e2efb262..c5bd39658 100644 --- a/src/Core/Entities/OrganizationUser.cs +++ b/src/Core/Entities/OrganizationUser.cs @@ -1,6 +1,7 @@ using System.ComponentModel.DataAnnotations; using Bit.Core.Enums; using Bit.Core.Models; +using Bit.Core.Models.Data; using Bit.Core.Utilities; namespace Bit.Core.Entities; @@ -28,4 +29,10 @@ public class OrganizationUser : ITableObject, IExternal { Id = CoreHelpers.GenerateComb(); } + + public Permissions GetPermissions() + { + return string.IsNullOrWhiteSpace(Permissions) ? null + : CoreHelpers.LoadClassFromJsonData(Permissions); + } } diff --git a/src/Core/Services/Implementations/OrganizationService.cs b/src/Core/Services/Implementations/OrganizationService.cs index e707df1be..547050bc4 100644 --- a/src/Core/Services/Implementations/OrganizationService.cs +++ b/src/Core/Services/Implementations/OrganizationService.cs @@ -986,10 +986,10 @@ public class OrganizationService : IOrganizationService .Select(i => i.invite.Type.Value)); if (invitingUserId.HasValue && inviteTypes.Count > 0) { - foreach (var type in inviteTypes) + foreach (var (invite, _) in invites) { - await ValidateOrganizationUserUpdatePermissions(organizationId, type, null); - await ValidateOrganizationCustomPermissionsEnabledAsync(organizationId, type); + await ValidateOrganizationUserUpdatePermissions(organizationId, invite.Type.Value, null, invite.Permissions); + await ValidateOrganizationCustomPermissionsEnabledAsync(organizationId, invite.Type.Value); } } @@ -1536,7 +1536,7 @@ public class OrganizationService : IOrganizationService if (savingUserId.HasValue) { - await ValidateOrganizationUserUpdatePermissions(user.OrganizationId, user.Type, originalUser.Type); + await ValidateOrganizationUserUpdatePermissions(user.OrganizationId, user.Type, originalUser.Type, user.GetPermissions()); } await ValidateOrganizationCustomPermissionsEnabledAsync(user.OrganizationId, user.Type); @@ -1709,7 +1709,7 @@ public class OrganizationService : IOrganizationService { if (loggedInUserId.HasValue) { - await ValidateOrganizationUserUpdatePermissions(organizationUser.OrganizationId, organizationUser.Type, null); + await ValidateOrganizationUserUpdatePermissions(organizationUser.OrganizationId, organizationUser.Type, null, organizationUser.GetPermissions()); } await _organizationUserRepository.UpdateGroupsAsync(organizationUser.Id, groupIds); await _eventService.LogOrganizationUserEventAsync(organizationUser, @@ -2100,8 +2100,7 @@ public class OrganizationService : IOrganizationService } } - private async Task ValidateOrganizationUserUpdatePermissions(Guid organizationId, OrganizationUserType newType, - OrganizationUserType? oldType) + private async Task ValidateOrganizationUserUpdatePermissions(Guid organizationId, OrganizationUserType newType, OrganizationUserType? oldType, Permissions permissions) { if (await _currentContext.OrganizationOwner(organizationId)) { @@ -2118,11 +2117,6 @@ public class OrganizationService : IOrganizationService return; } - if (oldType == OrganizationUserType.Custom || newType == OrganizationUserType.Custom) - { - throw new BadRequestException("Only Owners and Admins can configure Custom accounts."); - } - if (!await _currentContext.ManageUsers(organizationId)) { throw new BadRequestException("Your account does not have permission to manage users."); @@ -2132,6 +2126,11 @@ public class OrganizationService : IOrganizationService { throw new BadRequestException("Custom users can not manage Admins or Owners."); } + + if (newType == OrganizationUserType.Custom && !await ValidateCustomPermissionsGrant(organizationId, permissions)) + { + throw new BadRequestException("Custom users can only grant the same custom permissions that they have."); + } } private async Task ValidateOrganizationCustomPermissionsEnabledAsync(Guid organizationId, OrganizationUserType newType) @@ -2153,6 +2152,86 @@ public class OrganizationService : IOrganizationService } } + private async Task ValidateCustomPermissionsGrant(Guid organizationId, Permissions permissions) + { + if (permissions == null || await _currentContext.OrganizationOwner(organizationId) || await _currentContext.OrganizationAdmin(organizationId)) + { + return true; + } + + if (permissions.ManageUsers && !await _currentContext.ManageUsers(organizationId)) + { + return false; + } + + if (permissions.AccessReports && !await _currentContext.AccessReports(organizationId)) + { + return false; + } + + if (permissions.ManageGroups && !await _currentContext.ManageGroups(organizationId)) + { + return false; + } + + if (permissions.ManagePolicies && !await _currentContext.ManagePolicies(organizationId)) + { + return false; + } + + if (permissions.ManageScim && !await _currentContext.ManageScim(organizationId)) + { + return false; + } + + if (permissions.ManageSso && !await _currentContext.ManageSso(organizationId)) + { + return false; + } + + if (permissions.AccessEventLogs && !await _currentContext.AccessEventLogs(organizationId)) + { + return false; + } + + if (permissions.AccessImportExport && !await _currentContext.AccessImportExport(organizationId)) + { + return false; + } + + if (permissions.CreateNewCollections && !await _currentContext.CreateNewCollections(organizationId)) + { + return false; + } + + if (permissions.DeleteAnyCollection && !await _currentContext.DeleteAnyCollection(organizationId)) + { + return false; + } + + if (permissions.DeleteAssignedCollections && !await _currentContext.DeleteAssignedCollections(organizationId)) + { + return false; + } + + if (permissions.EditAnyCollection && !await _currentContext.EditAnyCollection(organizationId)) + { + return false; + } + + if (permissions.EditAssignedCollections && !await _currentContext.EditAssignedCollections(organizationId)) + { + return false; + } + + if (permissions.ManageResetPassword && !await _currentContext.ManageResetPassword(organizationId)) + { + return false; + } + + return true; + } + private async Task ValidateDeleteOrganizationAsync(Organization organization) { var ssoConfig = await _ssoConfigRepository.GetByOrganizationIdAsync(organization.Id); diff --git a/test/Core.Test/Services/OrganizationServiceTests.cs b/test/Core.Test/Services/OrganizationServiceTests.cs index 5a92bdcdf..9275da54a 100644 --- a/test/Core.Test/Services/OrganizationServiceTests.cs +++ b/test/Core.Test/Services/OrganizationServiceTests.cs @@ -33,7 +33,6 @@ namespace Bit.Core.Test.Services; [SutProviderCustomize] public class OrganizationServiceTests { - // [Fact] [Theory, PaidOrganizationCustomize, BitAutoData] public async Task OrgImportCreateNewUsers(SutProvider sutProvider, Guid userId, Organization org, List existingUsers, List newUsers) @@ -200,6 +199,7 @@ public class OrganizationServiceTests OrganizationUserInvite invite, SutProvider sutProvider) { invite.Emails = null; + sutProvider.GetDependency().OrganizationOwner(organization.Id).Returns(true); sutProvider.GetDependency().ManageUsers(organization.Id).Returns(true); sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); await Assert.ThrowsAsync( @@ -281,7 +281,7 @@ public class OrganizationServiceTests var exception = await Assert.ThrowsAsync( () => sutProvider.Sut.InviteUsersAsync(organization.Id, invitor.UserId, new (OrganizationUserInvite, string)[] { (invite, null) })); - Assert.Contains("only owners and admins", exception.Message.ToLowerInvariant()); + Assert.Contains("your account does not have permission to manage users", exception.Message.ToLowerInvariant()); } [Theory] @@ -463,6 +463,19 @@ public class OrganizationServiceTests organizationUserRepository.GetManyByOrganizationAsync(organization.Id, OrganizationUserType.Owner) .Returns(new[] { owner }); currentContext.ManageUsers(organization.Id).Returns(true); + currentContext.AccessReports(organization.Id).Returns(true); + currentContext.ManageGroups(organization.Id).Returns(true); + currentContext.ManagePolicies(organization.Id).Returns(true); + currentContext.ManageScim(organization.Id).Returns(true); + currentContext.ManageSso(organization.Id).Returns(true); + currentContext.AccessEventLogs(organization.Id).Returns(true); + currentContext.AccessImportExport(organization.Id).Returns(true); + currentContext.CreateNewCollections(organization.Id).Returns(true); + currentContext.DeleteAnyCollection(organization.Id).Returns(true); + currentContext.DeleteAssignedCollections(organization.Id).Returns(true); + currentContext.EditAnyCollection(organization.Id).Returns(true); + currentContext.EditAssignedCollections(organization.Id).Returns(true); + currentContext.ManageResetPassword(organization.Id).Returns(true); await sutProvider.Sut.InviteUsersAsync(organization.Id, invitor.UserId, invites); @@ -535,6 +548,7 @@ public class OrganizationServiceTests OrganizationUser newUserData, IEnumerable collections, IEnumerable groups, + Permissions permissions, [OrganizationUser(type: OrganizationUserType.Owner)] OrganizationUser savingUser, SutProvider sutProvider) { @@ -547,6 +561,10 @@ public class OrganizationServiceTests newUserData.Id = oldUserData.Id; newUserData.UserId = oldUserData.UserId; newUserData.OrganizationId = savingUser.OrganizationId = oldUserData.OrganizationId = organization.Id; + newUserData.Permissions = JsonSerializer.Serialize(permissions, new JsonSerializerOptions + { + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + }); organizationUserRepository.GetByIdAsync(oldUserData.Id).Returns(oldUserData); organizationUserRepository.GetManyByOrganizationAsync(savingUser.OrganizationId, OrganizationUserType.Owner) .Returns(new List { savingUser }); @@ -576,6 +594,7 @@ public class OrganizationServiceTests newUserData.Id = oldUserData.Id; newUserData.UserId = oldUserData.UserId; newUserData.OrganizationId = savingUser.OrganizationId = oldUserData.OrganizationId = organization.Id; + newUserData.Permissions = null; organizationUserRepository.GetByIdAsync(oldUserData.Id).Returns(oldUserData); organizationUserRepository.GetManyByOrganizationAsync(savingUser.OrganizationId, OrganizationUserType.Owner) .Returns(new List { savingUser }); @@ -598,6 +617,7 @@ public class OrganizationServiceTests OrganizationUser newUserData, IEnumerable collections, IEnumerable groups, + Permissions permissions, [OrganizationUser(type: OrganizationUserType.Owner)] OrganizationUser savingUser, SutProvider sutProvider) { @@ -613,6 +633,10 @@ public class OrganizationServiceTests newUserData.UserId = oldUserData.UserId; newUserData.OrganizationId = savingUser.OrganizationId = oldUserData.OrganizationId = organization.Id; newUserData.Type = newUserType; + newUserData.Permissions = JsonSerializer.Serialize(permissions, new JsonSerializerOptions + { + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + }); organizationUserRepository.GetByIdAsync(oldUserData.Id).Returns(oldUserData); organizationUserRepository.GetManyByOrganizationAsync(savingUser.OrganizationId, OrganizationUserType.Owner) .Returns(new List { savingUser }); @@ -628,6 +652,7 @@ public class OrganizationServiceTests [OrganizationUser(type: OrganizationUserType.Custom)] OrganizationUser newUserData, IEnumerable collections, IEnumerable groups, + Permissions permissions, [OrganizationUser(type: OrganizationUserType.Owner)] OrganizationUser savingUser, SutProvider sutProvider) { @@ -642,6 +667,10 @@ public class OrganizationServiceTests newUserData.Id = oldUserData.Id; newUserData.UserId = oldUserData.UserId; newUserData.OrganizationId = savingUser.OrganizationId = oldUserData.OrganizationId = organization.Id; + newUserData.Permissions = JsonSerializer.Serialize(permissions, new JsonSerializerOptions + { + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + }); organizationUserRepository.GetByIdAsync(oldUserData.Id).Returns(oldUserData); organizationUserRepository.GetManyByOrganizationAsync(savingUser.OrganizationId, OrganizationUserType.Owner) .Returns(new List { savingUser }); @@ -650,6 +679,111 @@ public class OrganizationServiceTests await sutProvider.Sut.SaveUserAsync(newUserData, savingUser.UserId, collections, groups); } + [Theory, BitAutoData] + public async Task SaveUser_WithCustomPermission_WhenSavingUserHasCustomPermission_Passes( + Organization organization, + [OrganizationUser(type: OrganizationUserType.User)] OrganizationUser oldUserData, + [OrganizationUser(type: OrganizationUserType.Custom)] OrganizationUser newUserData, + IEnumerable collections, + IEnumerable groups, + [OrganizationUser(type: OrganizationUserType.Custom)] OrganizationUser savingUser, + [OrganizationUser(type: OrganizationUserType.Owner)] OrganizationUser organizationOwner, + SutProvider sutProvider) + { + organization.UseCustomPermissions = true; + + var organizationRepository = sutProvider.GetDependency(); + var organizationUserRepository = sutProvider.GetDependency(); + var currentContext = sutProvider.GetDependency(); + + organizationRepository.GetByIdAsync(organization.Id).Returns(organization); + + newUserData.Id = oldUserData.Id; + newUserData.UserId = oldUserData.UserId; + newUserData.OrganizationId = savingUser.OrganizationId = oldUserData.OrganizationId = organizationOwner.OrganizationId = organization.Id; + newUserData.Permissions = JsonSerializer.Serialize(new Permissions { AccessReports = true }, new JsonSerializerOptions + { + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + }); + organizationUserRepository.GetByIdAsync(oldUserData.Id).Returns(oldUserData); + organizationUserRepository.GetManyByOrganizationAsync(savingUser.OrganizationId, OrganizationUserType.Owner) + .Returns(new List { organizationOwner }); + currentContext.OrganizationCustom(savingUser.OrganizationId).Returns(true); + currentContext.ManageUsers(savingUser.OrganizationId).Returns(true); + currentContext.AccessReports(savingUser.OrganizationId).Returns(true); + + await sutProvider.Sut.SaveUserAsync(newUserData, savingUser.UserId, collections, groups); + } + + [Theory, BitAutoData] + public async Task SaveUser_WithCustomPermission_WhenSavingUserDoesNotHaveCustomPermission_Throws( + Organization organization, + [OrganizationUser(type: OrganizationUserType.User)] OrganizationUser oldUserData, + [OrganizationUser(type: OrganizationUserType.Custom)] OrganizationUser newUserData, + IEnumerable collections, + IEnumerable groups, + [OrganizationUser(type: OrganizationUserType.Custom)] OrganizationUser savingUser, + SutProvider sutProvider) + { + organization.UseCustomPermissions = true; + + var organizationRepository = sutProvider.GetDependency(); + var organizationUserRepository = sutProvider.GetDependency(); + var currentContext = sutProvider.GetDependency(); + + organizationRepository.GetByIdAsync(organization.Id).Returns(organization); + + newUserData.Id = oldUserData.Id; + newUserData.UserId = oldUserData.UserId; + newUserData.OrganizationId = savingUser.OrganizationId = oldUserData.OrganizationId = organization.Id; + newUserData.Permissions = JsonSerializer.Serialize(new Permissions { AccessReports = true }, new JsonSerializerOptions + { + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + }); + organizationUserRepository.GetByIdAsync(oldUserData.Id).Returns(oldUserData); + currentContext.OrganizationCustom(savingUser.OrganizationId).Returns(true); + currentContext.ManageUsers(savingUser.OrganizationId).Returns(true); + currentContext.AccessReports(savingUser.OrganizationId).Returns(false); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.SaveUserAsync(newUserData, savingUser.UserId, collections, groups)); + Assert.Contains("custom users can only grant the same custom permissions that they have", exception.Message.ToLowerInvariant()); + } + + [Theory, BitAutoData] + public async Task SaveUser_WithCustomPermission_WhenUpgradingToAdmin_Throws( + Organization organization, + [OrganizationUser(type: OrganizationUserType.Custom)] OrganizationUser oldUserData, + [OrganizationUser(type: OrganizationUserType.Admin)] OrganizationUser newUserData, + IEnumerable collections, + IEnumerable groups, + SutProvider sutProvider) + { + organization.UseCustomPermissions = true; + + var organizationRepository = sutProvider.GetDependency(); + var organizationUserRepository = sutProvider.GetDependency(); + var currentContext = sutProvider.GetDependency(); + + organizationRepository.GetByIdAsync(organization.Id).Returns(organization); + + newUserData.Id = oldUserData.Id; + newUserData.UserId = oldUserData.UserId; + newUserData.OrganizationId = oldUserData.OrganizationId = organization.Id; + newUserData.Permissions = JsonSerializer.Serialize(new Permissions { AccessReports = true }, new JsonSerializerOptions + { + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + }); + organizationUserRepository.GetByIdAsync(oldUserData.Id).Returns(oldUserData); + currentContext.OrganizationCustom(oldUserData.OrganizationId).Returns(true); + currentContext.ManageUsers(oldUserData.OrganizationId).Returns(true); + currentContext.AccessReports(oldUserData.OrganizationId).Returns(false); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.SaveUserAsync(newUserData, oldUserData.UserId, collections, groups)); + Assert.Contains("custom users can not manage admins or owners", exception.Message.ToLowerInvariant()); + } + [Theory, BitAutoData] public async Task DeleteUser_InvalidUser(OrganizationUser organizationUser, OrganizationUser deletingUser, SutProvider sutProvider)