1
0
mirror of https://github.com/bitwarden/server.git synced 2025-02-08 00:31:27 +01:00

Refactor permission checks in OrganizationsService to use currentContext (#1420)

This commit is contained in:
Oscar Hinton 2021-07-01 14:31:05 +02:00 committed by GitHub
parent 43f7271147
commit a733257bc6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 51 additions and 79 deletions

View File

@ -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);
}
}

View File

@ -58,6 +58,6 @@ namespace Bit.Core.Services
bool overwriteExisting);
Task RotateApiKeyAsync(Organization organization);
Task DeleteSsoUserAsync(Guid userId, Guid? organizationId);
Task<Organization> UpdateOrganizationKeysAsync(Guid userId, Guid orgId, string publicKey, string privateKey);
Task<Organization> UpdateOrganizationKeysAsync(Guid orgId, string publicKey, string privateKey);
}
}

View File

@ -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<Tuple<OrganizationUser, string>>();
@ -1669,17 +1672,11 @@ namespace Bit.Core.Services
return confirmedOwnersIds.Except(organizationUsersId).Any();
}
private async Task<bool> 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<Guid> 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<Organization> UpdateOrganizationKeysAsync(Guid userId, Guid orgId, string publicKey, string privateKey)
public async Task<Organization> 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<Permissions>(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<Permissions>(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.");
}

View File

@ -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<IOrganizationUserRepository>().GetCountByOrganizationIdAsync(org.Id)
.Returns(existingUsers.Count);
sutProvider.GetDependency<ICurrentContext>().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<IOrganizationUserRepository>().GetByIdAsync(reInvitedUser.Id)
.Returns(new OrganizationUser { Id = reInvitedUser.Id });
var currentContext = sutProvider.GetDependency<ICurrentContext>();
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<OrganizationService> sutProvider)
{
var organizationRepository = sutProvider.GetDependency<IOrganizationRepository>();
var organizationUserRepository = sutProvider.GetDependency<IOrganizationUserRepository>();
var currentContext = sutProvider.GetDependency<ICurrentContext>();
organizationRepository.GetByIdAsync(organization.Id).Returns(organization);
organizationUserRepository.GetManyByUserAsync(invitor.Id).Returns(new List<OrganizationUser> { invitor });
currentContext.OrganizationAdmin(organization.Id).Returns(true);
var exception = await Assert.ThrowsAsync<BadRequestException>(
() => 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<OrganizationService> sutProvider)
{
var organizationRepository = sutProvider.GetDependency<IOrganizationRepository>();
var organizationUserRepository = sutProvider.GetDependency<IOrganizationUserRepository>();
var currentContext = sutProvider.GetDependency<ICurrentContext>();
organizationRepository.GetByIdAsync(organization.Id).Returns(organization);
organizationUserRepository.GetManyByUserAsync(invitor.Id).Returns(new List<OrganizationUser> { invitor });
currentContext.OrganizationUser(organization.Id).Returns(true);
var exception = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.InviteUserAsync(organization.Id, invitor.UserId, null, invite));
@ -238,10 +242,11 @@ namespace Bit.Core.Test.Services
});
var organizationRepository = sutProvider.GetDependency<IOrganizationRepository>();
var organizationUserRepository = sutProvider.GetDependency<IOrganizationUserRepository>();
var currentContext = sutProvider.GetDependency<ICurrentContext>();
organizationRepository.GetByIdAsync(organization.Id).Returns(organization);
organizationUserRepository.GetManyByUserAsync(invitor.UserId.Value).Returns(new List<OrganizationUser> { invitor });
currentContext.OrganizationCustom(organization.Id).Returns(true);
currentContext.ManageUsers(organization.Id).Returns(false);
var exception = await Assert.ThrowsAsync<BadRequestException>(
() => sutProvider.Sut.InviteUserAsync(organization.Id, invitor.UserId, null, invite));
@ -263,10 +268,11 @@ namespace Bit.Core.Test.Services
});
var organizationRepository = sutProvider.GetDependency<IOrganizationRepository>();
var organizationUserRepository = sutProvider.GetDependency<IOrganizationUserRepository>();
var currentContext = sutProvider.GetDependency<ICurrentContext>();
organizationRepository.GetByIdAsync(organization.Id).Returns(organization);
organizationUserRepository.GetManyByUserAsync(invitor.UserId.Value).Returns(new List<OrganizationUser> { invitor });
currentContext.OrganizationCustom(organization.Id).Returns(true);
currentContext.ManageUsers(organization.Id).Returns(true);
var exception = await Assert.ThrowsAsync<BadRequestException>(
() => 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<IOrganizationRepository>();
var organizationUserRepository = sutProvider.GetDependency<IOrganizationUserRepository>();
var eventService = sutProvider.GetDependency<IEventService>();
var currentContext = sutProvider.GetDependency<ICurrentContext>();
organizationRepository.GetByIdAsync(organization.Id).Returns(organization);
organizationUserRepository.GetManyByUserAsync(invitor.UserId.Value).Returns(new List<OrganizationUser> { 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<IOrganizationRepository>();
var organizationUserRepository = sutProvider.GetDependency<IOrganizationUserRepository>();
var eventService = sutProvider.GetDependency<IEventService>();
var currentContext = sutProvider.GetDependency<ICurrentContext>();
organizationRepository.GetByIdAsync(organization.Id).Returns(organization);
organizationUserRepository.GetManyByUserAsync(invitor.UserId.Value).Returns(new List<OrganizationUser> { 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<OrganizationService> sutProvider)
{
var organizationUserRepository = sutProvider.GetDependency<IOrganizationUserRepository>();
var currentContext = sutProvider.GetDependency<ICurrentContext>();
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<OrganizationUser> { savingUser });
organizationUserRepository.GetManyByUserAsync(savingUser.UserId.Value).Returns(new List<OrganizationUser> { 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<OrganizationService> sutProvider)
{
var organizationUserRepository = sutProvider.GetDependency<IOrganizationUserRepository>();
var currentContext = sutProvider.GetDependency<ICurrentContext>();
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<BadRequestException>(
() => sutProvider.Sut.DeleteUserAsync(deletingUser.OrganizationId, organizationUser.Id, deletingUser.UserId));
@ -425,13 +431,14 @@ namespace Bit.Core.Test.Services
SutProvider<OrganizationService> sutProvider)
{
var organizationUserRepository = sutProvider.GetDependency<IOrganizationUserRepository>();
var currentContext = sutProvider.GetDependency<ICurrentContext>();
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<OrganizationService> sutProvider)
{
var organizationUserRepository = sutProvider.GetDependency<IOrganizationUserRepository>();
var currentContext = sutProvider.GetDependency<ICurrentContext>();
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);
}