nhyatt/bitwarden
1
0
Fork 0
mirror of https://github.com/bitwarden/server.git synced 2025-07-01 08:02:49 -05:00
Code Activity

[AC-1125] Enforce org setting for creating/deleting collections (#3241)

Browse Source 
* [AC-1117] Add manage permission (#3126)

* Update sql files to add Manage permission

* Add migration script

* Rename collection manage migration file to remove duplicate migration date

* Migrations

* Add manage to models

* Add manage to repository

* Add constraint to Manage columns

* Migration lint fixes

* Add manage to OrganizationUserUserDetails_ReadWithCollectionsById

* Add missing manage fields

* Add 'Manage' to UserCollectionDetails

* Use CREATE OR ALTER where possible

* [AC-1374] Limit collection creation/deletion to Owner/Admin (#3145)

* feat: update org table with new column, write migration, refs AC-1374

* feat: update views with new column, refs AC-1374

* feat: Alter sprocs (org create/update) to include new column, refs AC-1374

* feat: update entity/data/request/response models to handle new column, refs AC-1374

* feat: update necessary Provider related views during migration, refs AC-1374

* fix: update org create to default new column to false, refs AC-1374

* feat: added new API/request model for collection management and removed property from update request model, refs AC-1374

* fix: renamed migration script to be after secrets manage beta column changes, refs AC-1374

* fix: dotnet format, refs AC-1374

* feat: add ef migrations to reflect mssql changes, refs AC-1374

* fix: dotnet format, refs AC-1374

* feat: update API signature to accept Guid and explain Cd verbiage, refs AC-1374

* feat: created collection auth handler/operations, added LimitCollectionCdOwnerAdmin to CurrentContentOrganization, refs AC-1125

* feat: create vault service collection extensions and register with base services, refs AC-1125

* feat: deprecated CurrentContext.CreateNewCollections, refs AC-1125

* feat: deprecate DeleteAnyCollection for single resource usages, refs AC-1125

* feat: move service registration to api, update references, refs AC-1125

* feat: add bulk delete authorization handler, refs AC-1125

* feat: always assign user and give manage access on create, refs AC-1125

* fix: updated CurrentContextOrganization type, refs AC-1125

* feat: combined existing collection authorization handlers/operations, refs AC-1125

* fix: OrganizationServiceTests -> CurrentContentOrganization typo, refs AC-1125

* fix: format, refs AC-1125

* fix: update collection controller tests, refs AC-1125

* fix: dotnet format, refs AC-1125

* feat: removed extra BulkAuthorizationHandler, refs AC-1125

* fix: dotnet format, refs AC-1125

* fix: change string to guid for org id, update bulk delete request model, refs AC-1125

* fix: remove delete many collection check, refs AC-1125

* fix: clean up collection auth handler, refs AC-1125

* fix: format fix for CollectionOperations, refs AC-1125

* fix: removed unnecessary owner check, add org null check to custom permission validation, refs AC-1125

* fix: remove unused methods in CurrentContext, refs AC-1125

* fix: removed obsolete test, fixed failling delete many test, refs AC-1125

* fix: CollectionAuthorizationHandlerTests fixes, refs AC-1125

* fix: OrganizationServiceTests fix broken test by mocking GetOrganization, refs AC-1125

* fix: CollectionAuthorizationHandler - remove unused repository, refs AC-1125

* feat: moved UserId null check to common method, refs AC-1125

* fix: updated auth handler tests to remove dependency on requirement for common code checks, refs AC-1125

* feat: updated conditionals/comments for create/delete methods within colleciton auth handler, refs AC-1125

* feat: added create/delete collection auth handler success methods, refs AC-1125

* fix: new up permissions to prevent excessive null checks, refs AC-1125

* fix: remove old reference to CreateNewCollections, refs AC-1125

* fix: typo within ViewAssignedCollections method, refs AC-1125

---------

Co-authored-by: Robyn MacCallum <robyntmaccallum@gmail.com>
This commit is contained in:
Vincent Salucci
2023-09-18 17:02:53 -05:00
committed by GitHub
parent acd3997133
commit 34dfdc53aa
14 changed files with 386 additions and 285 deletions
Download Patch File Download Diff File Expand all files Collapse all files

82
src/Api/Controllers/CollectionsController.cs
View File

@ -1,5 +1,6 @@
using Bit.Api.Models.Request;
using Bit.Api.Models.Response;
using Bit.Api.Vault.AuthorizationHandlers.Collections;
using Bit.Core.Context;
using Bit.Core.Entities;
using Bit.Core.Exceptions;
@ -20,19 +21,22 @@ public class CollectionsController : Controller
private readonly IDeleteCollectionCommand _deleteCollectionCommand;
private readonly IUserService _userService;
private readonly ICurrentContext _currentContext;
private readonly IAuthorizationService _authorizationService;
public CollectionsController(
ICollectionRepository collectionRepository,
ICollectionService collectionService,
IDeleteCollectionCommand deleteCollectionCommand,
IUserService userService,
ICurrentContext currentContext)
ICurrentContext currentContext,
IAuthorizationService authorizationService)
{
_collectionRepository = collectionRepository;
_collectionService = collectionService;
_deleteCollectionCommand = deleteCollectionCommand;
_userService = userService;
_currentContext = currentContext;
_authorizationService = authorizationService;
}
[HttpGet("{id}")]
@ -62,6 +66,7 @@ public class CollectionsController : Controller
{
throw new NotFoundException();
}
return new CollectionAccessDetailsResponseModel(collection, access.Groups, access.Users);
}
else
@ -72,6 +77,7 @@ public class CollectionsController : Controller
{
throw new NotFoundException();
}
return new CollectionAccessDetailsResponseModel(collection, access.Groups, access.Users);
}
}
@ -79,13 +85,15 @@ public class CollectionsController : Controller
[HttpGet("details")]
public async Task<ListResponseModel<CollectionAccessDetailsResponseModel>> GetManyWithDetails(Guid orgId)
{
if (!await ViewAtLeastOneCollectionAsync(orgId) && !await _currentContext.ManageUsers(orgId) && !await _currentContext.ManageGroups(orgId))
if (!await ViewAtLeastOneCollectionAsync(orgId) && !await _currentContext.ManageUsers(orgId) &&
!await _currentContext.ManageGroups(orgId))
{
throw new NotFoundException();
}
// We always need to know which collections the current user is assigned to
var assignedOrgCollections = await _collectionRepository.GetManyByUserIdWithAccessAsync(_currentContext.UserId.Value, orgId);
var assignedOrgCollections =
await _collectionRepository.GetManyByUserIdWithAccessAsync(_currentContext.UserId.Value, orgId);
if (await _currentContext.ViewAllCollections(orgId) || await _currentContext.ManageUsers(orgId))
{
@ -141,8 +149,8 @@ public class CollectionsController : Controller
{
var collection = model.ToCollection(orgId);
if (!await CanCreateCollection(orgId, collection.Id) &&
!await CanEditCollectionAsync(orgId, collection.Id))
var result = await _authorizationService.AuthorizeAsync(User, collection, CollectionOperations.Create);
if (!result.Succeeded)
{
throw new NotFoundException();
}
@ -150,10 +158,7 @@ public class CollectionsController : Controller
var groups = model.Groups?.Select(g => g.ToSelectionReadOnly());
var users = model.Users?.Select(g => g.ToSelectionReadOnly());
var assignUserToCollection = !(await _currentContext.EditAnyCollection(orgId)) &&
await _currentContext.EditAssignedCollections(orgId);
await _collectionService.SaveAsync(collection, groups, users, assignUserToCollection ? _currentContext.UserId : null);
await _collectionService.SaveAsync(collection, groups, users, _currentContext.UserId);
return new CollectionResponseModel(collection);
}
@ -189,12 +194,13 @@ public class CollectionsController : Controller
[HttpPost("{id}/delete")]
public async Task Delete(Guid orgId, Guid id)
{
if (!await CanDeleteCollectionAsync(orgId, id))
var collection = await GetCollectionAsync(id, orgId);
var result = await _authorizationService.AuthorizeAsync(User, collection, CollectionOperations.Delete);
if (!result.Succeeded)
{
throw new NotFoundException();
}
var collection = await GetCollectionAsync(id, orgId);
await _deleteCollectionCommand.DeleteAsync(collection);
}
@ -202,22 +208,14 @@ public class CollectionsController : Controller
[HttpPost("delete")]
public async Task DeleteMany([FromBody] CollectionBulkDeleteRequestModel model)
{
var orgId = new Guid(model.OrganizationId);
var collectionIds = model.Ids.Select(i => new Guid(i));
if (!await _currentContext.DeleteAssignedCollections(orgId) && !await _currentContext.DeleteAnyCollection(orgId))
var collections = await _collectionRepository.GetManyByManyIdsAsync(model.Ids);
var result = await _authorizationService.AuthorizeAsync(User, collections, CollectionOperations.Delete);
if (!result.Succeeded)
{
throw new NotFoundException();
}
var userCollections = await _collectionService.GetOrganizationCollectionsAsync(orgId);
var filteredCollections = userCollections.Where(c => collectionIds.Contains(c.Id) && c.OrganizationId == orgId);
if (!filteredCollections.Any())
{
throw new BadRequestException("No collections found.");
}
await _deleteCollectionCommand.DeleteManyAsync(filteredCollections);
await _deleteCollectionCommand.DeleteManyAsync(collections);
}
[HttpDelete("{id}/user/{orgUserId}")]
@ -248,17 +246,6 @@ public class CollectionsController : Controller
return collection;
}
private async Task<bool> CanCreateCollection(Guid orgId, Guid collectionId)
{
if (collectionId != default)
{
return false;
}
return await _currentContext.CreateNewCollections(orgId);
}
private async Task<bool> CanEditCollectionAsync(Guid orgId, Guid collectionId)
{
if (collectionId == default)
@ -273,28 +260,8 @@ public class CollectionsController : Controller
if (await _currentContext.EditAssignedCollections(orgId))
{
var collectionDetails = await _collectionRepository.GetByIdAsync(collectionId, _currentContext.UserId.Value);
return collectionDetails != null;
}
return false;
}
private async Task<bool> CanDeleteCollectionAsync(Guid orgId, Guid collectionId)
{
if (collectionId == default)
{
return false;
}
if (await _currentContext.DeleteAnyCollection(orgId))
{
return true;
}
if (await _currentContext.DeleteAssignedCollections(orgId))
{
var collectionDetails = await _collectionRepository.GetByIdAsync(collectionId, _currentContext.UserId.Value);
var collectionDetails =
await _collectionRepository.GetByIdAsync(collectionId, _currentContext.UserId.Value);
return collectionDetails != null;
}
@ -315,7 +282,8 @@ public class CollectionsController : Controller
if (await _currentContext.ViewAssignedCollections(orgId))
{
var collectionDetails = await _collectionRepository.GetByIdAsync(collectionId, _currentContext.UserId.Value);
var collectionDetails =
await _collectionRepository.GetByIdAsync(collectionId, _currentContext.UserId.Value);
return collectionDetails != null;
}

3
src/Api/Models/Request/CollectionRequestModel.cs
View File

@ -34,7 +34,8 @@ public class CollectionRequestModel
public class CollectionBulkDeleteRequestModel
{
[Required]
public IEnumerable<string> Ids { get; set; }
public IEnumerable<Guid> Ids { get; set; }
[Obsolete("OrganizationId is no longer required and will be removed in a future release")]
public string OrganizationId { get; set; }
}

2
src/Api/Utilities/ServiceCollectionExtensions.cs
View File

@ -1,4 +1,4 @@
using Bit.Api.Vault.AuthorizationHandlers;
using Bit.Api.Vault.AuthorizationHandlers.Collections;
using Bit.Core.IdentityServer;
using Bit.Core.Settings;
using Bit.Core.Utilities;

90
src/Api/Vault/AuthorizationHandlers/CollectionAuthorizationHandler.cs
View File

@ -1,90 +0,0 @@
using Bit.Core.Context;
using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Exceptions;
using Bit.Core.Repositories;
using Bit.Core.Utilities;
using Microsoft.AspNetCore.Authorization;
namespace Bit.Api.Vault.AuthorizationHandlers;
public class CollectionAuthorizationHandler : BulkAuthorizationHandler<CollectionOperationRequirement, Collection>
{
private readonly ICurrentContext _currentContext;
private readonly ICollectionRepository _collectionRepository;
private readonly IOrganizationUserRepository _organizationUserRepository;
public CollectionAuthorizationHandler(ICurrentContext currentContext, ICollectionRepository collectionRepository, IOrganizationUserRepository organizationUserRepository)
{
_currentContext = currentContext;
_collectionRepository = collectionRepository;
_organizationUserRepository = organizationUserRepository;
}
protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, CollectionOperationRequirement requirement,
ICollection<Collection> resources)
{
switch (requirement)
{
case not null when requirement == CollectionOperation.ModifyAccess:
await CanManageCollectionAccessAsync(context, requirement, resources);
break;
}
}
/// <summary>
/// Ensures the acting user is allowed to manage access permissions for the target collections.
/// </summary>
private async Task CanManageCollectionAccessAsync(AuthorizationHandlerContext context, IAuthorizationRequirement requirement, ICollection<Collection> targetCollections)
{
if (!_currentContext.UserId.HasValue)
{
context.Fail();
return;
}
var targetOrganizationId = targetCollections.First().OrganizationId;
// Ensure all target collections belong to the same organization
if (targetCollections.Any(tc => tc.OrganizationId != targetOrganizationId))
{
throw new BadRequestException("Requested collections must belong to the same organization.");
}
var org = (await _currentContext.OrganizationMembershipAsync(_organizationUserRepository, _currentContext.UserId.Value))
.FirstOrDefault(o => targetOrganizationId == o.Id);
// Acting user is not a member of the target organization, fail
if (org == null)
{
context.Fail();
return;
}
// Owners, Admins, Providers, and users with EditAnyCollection permission can always manage collection access
if (
org.Permissions is { EditAnyCollection: true } ||
org.Type is OrganizationUserType.Admin or OrganizationUserType.Owner ||
await _currentContext.ProviderUserForOrgAsync(org.Id))
{
context.Succeed(requirement);
return;
}
// List of collection Ids the acting user is allowed to manage
var manageableCollectionIds =
(await _collectionRepository.GetManyByUserIdAsync(_currentContext.UserId.Value))
.Where(c => c.Manage && c.OrganizationId == targetOrganizationId)
.Select(c => c.Id)
.ToHashSet();
// The acting user does not have permission to manage all target collections, fail
if (targetCollections.Any(tc => !manageableCollectionIds.Contains(tc.Id)))
{
context.Fail();
return;
}
context.Succeed(requirement);
}
}

162
src/Api/Vault/AuthorizationHandlers/Collections/CollectionAuthorizationHandler.cs Normal file
View File

@ -0,0 +1,162 @@
using Bit.Core.Context;
using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Exceptions;
using Bit.Core.Repositories;
using Bit.Core.Utilities;
using Microsoft.AspNetCore.Authorization;
namespace Bit.Api.Vault.AuthorizationHandlers.Collections;
public class CollectionAuthorizationHandler : BulkAuthorizationHandler<CollectionOperationRequirement, Collection>
{
private readonly ICurrentContext _currentContext;
private readonly ICollectionRepository _collectionRepository;
public CollectionAuthorizationHandler(ICurrentContext currentContext, ICollectionRepository collectionRepository)
{
_currentContext = currentContext;
_collectionRepository = collectionRepository;
}
protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context,
CollectionOperationRequirement requirement, ICollection<Collection> resources)
{
// Establish pattern of authorization handler null checking passed resources
if (resources == null || !resources.Any())
{
context.Fail();
return;
}
if (!_currentContext.UserId.HasValue)
{
context.Fail();
return;
}
var targetOrganizationId = resources.First().OrganizationId;
// Ensure all target collections belong to the same organization
if (resources.Any(tc => tc.OrganizationId != targetOrganizationId))
{
throw new BadRequestException("Requested collections must belong to the same organization.");
}
// Acting user is not a member of the target organization, fail
var org = _currentContext.GetOrganization(targetOrganizationId);
if (org == null)
{
context.Fail();
return;
}
switch (requirement)
{
case not null when requirement == CollectionOperations.Create:
await CanCreateAsync(context, requirement, org);
break;
case not null when requirement == CollectionOperations.Delete:
await CanDeleteAsync(context, requirement, resources, org);
break;
case not null when requirement == CollectionOperations.ModifyAccess:
await CanManageCollectionAccessAsync(context, requirement, resources, org);
break;
}
}
private async Task CanCreateAsync(AuthorizationHandlerContext context, CollectionOperationRequirement requirement,
CurrentContextOrganization org)
{
// If false, all organization members are allowed to create collections
if (!org.LimitCollectionCdOwnerAdmin)
{
context.Succeed(requirement);
return;
}
// Owners, Admins, Providers, and users with CreateNewCollections or EditAnyCollection permission can always create collections
if (
org.Type is OrganizationUserType.Owner or OrganizationUserType.Admin ||
org.Permissions.CreateNewCollections || org.Permissions.EditAnyCollection ||
await _currentContext.ProviderUserForOrgAsync(org.Id))
{
context.Succeed(requirement);
return;
}
context.Fail();
}
private async Task CanDeleteAsync(AuthorizationHandlerContext context, CollectionOperationRequirement requirement,
ICollection<Collection> resources, CurrentContextOrganization org)
{
// Owners, Admins, Providers, and users with DeleteAnyCollection or EditAnyCollection permission can always delete collections
if (
org.Type is OrganizationUserType.Owner or OrganizationUserType.Admin ||
org.Permissions.DeleteAnyCollection || org.Permissions.EditAnyCollection ||
await _currentContext.ProviderUserForOrgAsync(org.Id))
{
context.Succeed(requirement);
return;
}
// The limit collection management setting is enabled and we are not an Admin (above condition), fail
if (org.LimitCollectionCdOwnerAdmin)
{
context.Fail();
return;
}
// Other members types should have the Manage capability for all collections being deleted
var manageableCollectionIds =
(await _collectionRepository.GetManyByUserIdAsync(_currentContext.UserId!.Value))
.Where(c => c.Manage && c.OrganizationId == org.Id)
.Select(c => c.Id)
.ToHashSet();
// The acting user does not have permission to manage all target collections, fail
if (resources.Any(c => !manageableCollectionIds.Contains(c.Id)))
{
context.Fail();
return;
}
context.Succeed(requirement);
}
/// <summary>
/// Ensures the acting user is allowed to manage access permissions for the target collections.
/// </summary>
private async Task CanManageCollectionAccessAsync(AuthorizationHandlerContext context,
IAuthorizationRequirement requirement, ICollection<Collection> targetCollections, CurrentContextOrganization org)
{
// Owners, Admins, Providers, and users with EditAnyCollection permission can always manage collection access
if (
org.Permissions is { EditAnyCollection: true } ||
org.Type is OrganizationUserType.Owner or OrganizationUserType.Admin ||
await _currentContext.ProviderUserForOrgAsync(org.Id))
{
context.Succeed(requirement);
return;
}
// List of collection Ids the acting user is allowed to manage
var manageableCollectionIds =
(await _collectionRepository.GetManyByUserIdAsync(_currentContext.UserId!.Value))
.Where(c => c.Manage && c.OrganizationId == org.Id)
.Select(c => c.Id)
.ToHashSet();
// The acting user does not have permission to manage all target collections, fail
if (targetCollections.Any(tc => !manageableCollectionIds.Contains(tc.Id)))
{
context.Fail();
return;
}
context.Succeed(requirement);
}
}

6
src/Api/Vault/AuthorizationHandlers/CollectionOperation.cs → src/Api/Vault/AuthorizationHandlers/Collections/CollectionOperations.cs
View File

@ -1,11 +1,13 @@
using Microsoft.AspNetCore.Authorization.Infrastructure;
namespace Bit.Api.Vault.AuthorizationHandlers;
namespace Bit.Api.Vault.AuthorizationHandlers.Collections;
public class CollectionOperationRequirement : OperationAuthorizationRequirement { }
public static class CollectionOperation
public static class CollectionOperations
{
public static readonly CollectionOperationRequirement Create = new() { Name = nameof(Create) };
public static readonly CollectionOperationRequirement Delete = new() { Name = nameof(Delete) };
/// <summary>
/// The operation that represents creating, updating, or removing collection access.
/// Combined together to allow for a single requirement to be used for each operation

37
src/Core/Context/CurrentContext.cs
View File

@ -321,27 +321,16 @@ public class CurrentContext : ICurrentContext
&& (o.Permissions?.AccessReports ?? false)) ?? false);
}
public async Task<bool> CreateNewCollections(Guid orgId)
{
return await OrganizationManager(orgId) || (Organizations?.Any(o => o.Id == orgId
&& (o.Permissions?.CreateNewCollections ?? false)) ?? false);
}
public async Task<bool> EditAnyCollection(Guid orgId)
{
return await OrganizationAdmin(orgId) || (Organizations?.Any(o => o.Id == orgId
&& (o.Permissions?.EditAnyCollection ?? false)) ?? false);
}
public async Task<bool> DeleteAnyCollection(Guid orgId)
{
return await OrganizationAdmin(orgId) || (Organizations?.Any(o => o.Id == orgId
&& (o.Permissions?.DeleteAnyCollection ?? false)) ?? false);
}
public async Task<bool> ViewAllCollections(Guid orgId)
{
return await EditAnyCollection(orgId) || await DeleteAnyCollection(orgId);
var org = GetOrganization(orgId);
return await EditAnyCollection(orgId) || (org != null && org.Permissions.DeleteAnyCollection);
}
public async Task<bool> EditAssignedCollections(Guid orgId)
@ -358,9 +347,20 @@ public class CurrentContext : ICurrentContext
public async Task<bool> ViewAssignedCollections(Guid orgId)
{
return await CreateNewCollections(orgId) // Required to display the existing collections under which the new collection can be nested
|| await EditAssignedCollections(orgId)
|| await DeleteAssignedCollections(orgId);
/*
* Required to display the existing collections under which the new collection can be nested.
* Owner, Admin, Manager, and Provider checks are handled via the EditAssigned/DeleteAssigned context calls.
* This entire method will be moved to the CollectionAuthorizationHandler in the future
*/
var canCreateNewCollections = false;
var org = GetOrganization(orgId);
if (org != null)
{
canCreateNewCollections = !org.LimitCollectionCdOwnerAdmin || org.Permissions.CreateNewCollections;
}
return await EditAssignedCollections(orgId)
|| await DeleteAssignedCollections(orgId)
|| canCreateNewCollections;
}
public async Task<bool> ManageGroups(Guid orgId)
@ -509,6 +509,11 @@ public class CurrentContext : ICurrentContext
return Providers;
}
public CurrentContextOrganization GetOrganization(Guid orgId)
{
return Organizations?.Find(o => o.Id == orgId);
}
private string GetClaimValue(Dictionary<string, IEnumerable<Claim>> claims, string type)
{
if (!claims.ContainsKey(type))

4
src/Core/Context/CurrentContextOrganization.cs
View File

@ -15,10 +15,12 @@ public class CurrentContextOrganization
Type = orgUser.Type;
Permissions = CoreHelpers.LoadClassFromJsonData<Permissions>(orgUser.Permissions);
AccessSecretsManager = orgUser.AccessSecretsManager && orgUser.UseSecretsManager;
LimitCollectionCdOwnerAdmin = orgUser.LimitCollectionCdOwnerAdmin;
}
public Guid Id { get; set; }
public OrganizationUserType Type { get; set; }
public Permissions Permissions { get; set; }
public Permissions Permissions { get; set; } = new();
public bool AccessSecretsManager { get; set; }
public bool LimitCollectionCdOwnerAdmin { get; set; }
}

7
src/Core/Context/ICurrentContext.cs
View File

@ -1,4 +1,6 @@
using System.Security.Claims;
#nullable enable
using System.Security.Claims;
using Bit.Core.Entities;
using Bit.Core.Enums;
using Bit.Core.Identity;
@ -39,9 +41,7 @@ public interface ICurrentContext
Task<bool> AccessEventLogs(Guid orgId);
Task<bool> AccessImportExport(Guid orgId);
Task<bool> AccessReports(Guid orgId);
Task<bool> CreateNewCollections(Guid orgId);
Task<bool> EditAnyCollection(Guid orgId);
Task<bool> DeleteAnyCollection(Guid orgId);
Task<bool> ViewAllCollections(Guid orgId);
Task<bool> EditAssignedCollections(Guid orgId);
Task<bool> DeleteAssignedCollections(Guid orgId);
@ -72,4 +72,5 @@ public interface ICurrentContext
Task<Guid?> ProviderIdForOrg(Guid orgId);
bool AccessSecretsManager(Guid organizationId);
CurrentContextOrganization? GetOrganization(Guid orgId);
}

2
src/Core/Services/Implementations/CollectionService.cs
View File

@ -71,7 +71,7 @@ public class CollectionService : ICollectionService
{
await _collectionRepository.UpdateUsersAsync(collection.Id,
new List<CollectionAccessSelection> {
new CollectionAccessSelection { Id = orgUser.Id, ReadOnly = false } });
new CollectionAccessSelection { Id = orgUser.Id, Manage = true} });
}
}

28
src/Core/Services/Implementations/OrganizationService.cs
View File

@ -2083,7 +2083,7 @@ public class OrganizationService : IOrganizationService
private async Task<bool> ValidateCustomPermissionsGrant(Guid organizationId, Permissions permissions)
{
if (permissions == null || await _currentContext.OrganizationOwner(organizationId) || await _currentContext.OrganizationAdmin(organizationId))
if (permissions == null || await _currentContext.OrganizationAdmin(organizationId))
{
return true;
}
@ -2128,16 +2128,6 @@ public class OrganizationService : IOrganizationService
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;
@ -2158,6 +2148,22 @@ public class OrganizationService : IOrganizationService
return false;
}
var org = _currentContext.GetOrganization(organizationId);
if (org == null)
{
return false;
}
if (permissions.CreateNewCollections && !org.Permissions.CreateNewCollections)
{
return false;
}
if (permissions.DeleteAnyCollection && !org.Permissions.DeleteAnyCollection)
{
return false;
}
return true;
}

127
test/Api.Test/Controllers/CollectionsControllerTests.cs
View File

@ -1,5 +1,7 @@
using Bit.Api.Controllers;
using System.Security.Claims;
using Bit.Api.Controllers;
using Bit.Api.Models.Request;
using Bit.Api.Vault.AuthorizationHandlers.Collections;
using Bit.Core.Context;
using Bit.Core.Entities;
using Bit.Core.Exceptions;
@ -9,6 +11,7 @@ using Bit.Core.Repositories;
using Bit.Core.Services;
using Bit.Test.Common.AutoFixture;
using Bit.Test.Common.AutoFixture.Attributes;
using Microsoft.AspNetCore.Authorization;
using NSubstitute;
using Xunit;
@ -19,27 +22,25 @@ namespace Bit.Api.Test.Controllers;
public class CollectionsControllerTests
{
[Theory, BitAutoData]
public async Task Post_Success(Guid orgId, SutProvider<CollectionsController> sutProvider)
public async Task Post_Success(Guid orgId, CollectionRequestModel collectionRequest,
SutProvider<CollectionsController> sutProvider)
{
sutProvider.GetDependency<ICurrentContext>()
.CreateNewCollections(orgId)
.Returns(true);
Collection ExpectedCollection() => Arg.Is<Collection>(c =>
c.Name == collectionRequest.Name && c.ExternalId == collectionRequest.ExternalId &&
c.OrganizationId == orgId);
sutProvider.GetDependency<ICurrentContext>()
.EditAnyCollection(orgId)
.Returns(false);
var collectionRequest = new CollectionRequestModel
{
Name = "encrypted_string",
ExternalId = "my_external_id"
};
sutProvider.GetDependency<IAuthorizationService>()
.AuthorizeAsync(Arg.Any<ClaimsPrincipal>(),
ExpectedCollection(),
Arg.Is<IEnumerable<IAuthorizationRequirement>>(r => r.Contains(CollectionOperations.Create)))
.Returns(AuthorizationResult.Success());
_ = await sutProvider.Sut.Post(orgId, collectionRequest);
await sutProvider.GetDependency<ICollectionService>()
.Received(1)
.SaveAsync(Arg.Any<Collection>(), Arg.Any<IEnumerable<CollectionAccessSelection>>(), null);
.SaveAsync(Arg.Any<Collection>(), Arg.Any<IEnumerable<CollectionAccessSelection>>(),
Arg.Any<IEnumerable<CollectionAccessSelection>>(), null);
}
[Theory, BitAutoData]
@ -139,13 +140,12 @@ public class CollectionsControllerTests
[Theory, BitAutoData]
public async Task DeleteMany_Success(Guid orgId, User user, Collection collection1, Collection collection2, SutProvider<CollectionsController> sutProvider)
public async Task DeleteMany_Success(Guid orgId, Collection collection1, Collection collection2, SutProvider<CollectionsController> sutProvider)
{
// Arrange
var model = new CollectionBulkDeleteRequestModel
{
Ids = new[] { collection1.Id.ToString(), collection2.Id.ToString() },
OrganizationId = orgId.ToString()
Ids = new[] { collection1.Id, collection2.Id }
};
var collections = new List<Collection>
@ -162,18 +162,15 @@ public class CollectionsControllerTests
},
};
sutProvider.GetDependency<ICurrentContext>()
.DeleteAssignedCollections(orgId)
.Returns(true);
sutProvider.GetDependency<ICurrentContext>()
.UserId
.Returns(user.Id);
sutProvider.GetDependency<ICollectionService>()
.GetOrganizationCollectionsAsync(orgId)
sutProvider.GetDependency<ICollectionRepository>().GetManyByManyIdsAsync(Arg.Any<IEnumerable<Guid>>())
.Returns(collections);
sutProvider.GetDependency<IAuthorizationService>()
.AuthorizeAsync(Arg.Any<ClaimsPrincipal>(),
collections,
Arg.Is<IEnumerable<IAuthorizationRequirement>>(r => r.Contains(CollectionOperations.Delete)))
.Returns(AuthorizationResult.Success());
// Act
await sutProvider.Sut.DeleteMany(model);
@ -185,18 +182,36 @@ public class CollectionsControllerTests
}
[Theory, BitAutoData]
public async Task DeleteMany_CanNotDeleteAssignedCollection_ThrowsNotFound(Guid orgId, Collection collection1, Collection collection2, SutProvider<CollectionsController> sutProvider)
public async Task DeleteMany_PermissionDenied_ThrowsNotFound(Guid orgId, Collection collection1, Collection collection2, SutProvider<CollectionsController> sutProvider)
{
// Arrange
var model = new CollectionBulkDeleteRequestModel
{
Ids = new[] { collection1.Id.ToString(), collection2.Id.ToString() },
OrganizationId = orgId.ToString()
Ids = new[] { collection1.Id, collection2.Id }
};
sutProvider.GetDependency<ICurrentContext>()
.DeleteAssignedCollections(orgId)
.Returns(false);
var collections = new List<Collection>
{
new CollectionDetails
{
Id = collection1.Id,
OrganizationId = orgId,
},
new CollectionDetails
{
Id = collection2.Id,
OrganizationId = orgId,
},
};
sutProvider.GetDependency<ICollectionRepository>().GetManyByManyIdsAsync(Arg.Any<IEnumerable<Guid>>())
.Returns(collections);
sutProvider.GetDependency<IAuthorizationService>()
.AuthorizeAsync(Arg.Any<ClaimsPrincipal>(),
collections,
Arg.Is<IEnumerable<IAuthorizationRequirement>>(r => r.Contains(CollectionOperations.Delete)))
.Returns(AuthorizationResult.Failed());
// Assert
await Assert.ThrowsAsync<NotFoundException>(() =>
@ -205,49 +220,5 @@ public class CollectionsControllerTests
await sutProvider.GetDependency<IDeleteCollectionCommand>()
.DidNotReceiveWithAnyArgs()
.DeleteManyAsync((IEnumerable<Collection>)default);
}
[Theory, BitAutoData]
public async Task DeleteMany_UserCanNotAccessCollections_FiltersOutInvalid(Guid orgId, User user, Collection collection1, Collection collection2, SutProvider<CollectionsController> sutProvider)
{
// Arrange
var model = new CollectionBulkDeleteRequestModel
{
Ids = new[] { collection1.Id.ToString(), collection2.Id.ToString() },
OrganizationId = orgId.ToString()
};
var collections = new List<Collection>
{
new CollectionDetails
{
Id = collection2.Id,
OrganizationId = orgId,
},
};
sutProvider.GetDependency<ICurrentContext>()
.DeleteAssignedCollections(orgId)
.Returns(true);
sutProvider.GetDependency<ICurrentContext>()
.UserId
.Returns(user.Id);
sutProvider.GetDependency<ICollectionService>()
.GetOrganizationCollectionsAsync(orgId)
.Returns(collections);
// Act
await sutProvider.Sut.DeleteMany(model);
// Assert
await sutProvider.GetDependency<IDeleteCollectionCommand>()
.Received(1)
.DeleteManyAsync(Arg.Is<IEnumerable<Collection>>(coll => coll.Select(c => c.Id).SequenceEqual(collections.Select(c => c.Id))));
}
}

102
test/Api.Test/Vault/AuthorizationHandlers/CollectionAuthorizationHandlerTests.cs
View File

@ -1,5 +1,5 @@
using System.Security.Claims;
using Bit.Api.Vault.AuthorizationHandlers;
using Bit.Api.Vault.AuthorizationHandlers.Collections;
using Bit.Core.Context;
using Bit.Core.Entities;
using Bit.Core.Enums;
@ -41,12 +41,12 @@ public class CollectionAuthorizationHandlerTests
organization.Permissions.EditAnyCollection = editAnyCollection;
var context = new AuthorizationHandlerContext(
new[] { CollectionOperation.ModifyAccess },
new[] { CollectionOperations.ModifyAccess },
new ClaimsPrincipal(),
collections);
sutProvider.GetDependency<ICurrentContext>().UserId.Returns(actingUserId);
sutProvider.GetDependency<ICurrentContext>().OrganizationMembershipAsync(Arg.Any<IOrganizationUserRepository>(), actingUserId).Returns(new[] { organization });
sutProvider.GetDependency<ICurrentContext>().GetOrganization(organization.Id).Returns(organization);
sutProvider.GetDependency<ICollectionRepository>().GetManyByUserIdAsync(actingUserId).Returns(collectionDetails);
await sutProvider.Sut.HandleAsync(context);
@ -54,14 +54,79 @@ public class CollectionAuthorizationHandlerTests
Assert.True(context.HasSucceeded);
}
[Theory, CollectionCustomization]
[BitAutoData(OrganizationUserType.User, false, false)]
[BitAutoData(OrganizationUserType.Admin, false, true)]
[BitAutoData(OrganizationUserType.Owner, false, true)]
[BitAutoData(OrganizationUserType.Custom, true, true)]
public async Task CanCreateAsync_Success(
OrganizationUserType userType, bool createNewCollection, bool limitCollectionCreateDelete,
SutProvider<CollectionAuthorizationHandler> sutProvider,
ICollection<Collection> collections,
CurrentContextOrganization organization)
{
var actingUserId = Guid.NewGuid();
organization.Type = userType;
organization.Permissions.CreateNewCollections = createNewCollection;
organization.LimitCollectionCdOwnerAdmin = limitCollectionCreateDelete;
var context = new AuthorizationHandlerContext(
new[] { CollectionOperations.Create },
new ClaimsPrincipal(),
collections);
sutProvider.GetDependency<ICurrentContext>().UserId.Returns(actingUserId);
sutProvider.GetDependency<ICurrentContext>().GetOrganization(organization.Id).Returns(organization);
await sutProvider.Sut.HandleAsync(context);
Assert.True(context.HasSucceeded);
}
[Theory, CollectionCustomization]
[BitAutoData(OrganizationUserType.User, false, false, true)]
[BitAutoData(OrganizationUserType.Admin, false, true, false)]
[BitAutoData(OrganizationUserType.Owner, false, true, false)]
[BitAutoData(OrganizationUserType.Custom, true, true, false)]
public async Task CanDeleteAsync_Success(
OrganizationUserType userType, bool deleteAnyCollection, bool limitCollectionCreateDelete, bool manageCollections,
SutProvider<CollectionAuthorizationHandler> sutProvider,
ICollection<Collection> collections,
ICollection<CollectionDetails> collectionDetails,
CurrentContextOrganization organization)
{
var actingUserId = Guid.NewGuid();
foreach (var collectionDetail in collectionDetails)
{
collectionDetail.Manage = manageCollections;
}
organization.Type = userType;
organization.Permissions.DeleteAnyCollection = deleteAnyCollection;
organization.LimitCollectionCdOwnerAdmin = limitCollectionCreateDelete;
var context = new AuthorizationHandlerContext(
new[] { CollectionOperations.Delete },
new ClaimsPrincipal(),
collections);
sutProvider.GetDependency<ICurrentContext>().UserId.Returns(actingUserId);
sutProvider.GetDependency<ICurrentContext>().GetOrganization(organization.Id).Returns(organization);
sutProvider.GetDependency<ICollectionRepository>().GetManyByUserIdAsync(actingUserId).Returns(collectionDetails);
await sutProvider.Sut.HandleAsync(context);
Assert.True(context.HasSucceeded);
}
[Theory, BitAutoData, CollectionCustomization]
public async Task CanManageCollectionAccessAsync_MissingUserId_Failure(
public async Task HandleRequirementAsync_MissingUserId_Failure(
SutProvider<CollectionAuthorizationHandler> sutProvider,
ICollection<Collection> collections)
{
var context = new AuthorizationHandlerContext(
new[] { CollectionOperation.ModifyAccess },
new[] { CollectionOperations.Create },
new ClaimsPrincipal(),
collections
);
@ -75,7 +140,7 @@ public class CollectionAuthorizationHandlerTests
}
[Theory, BitAutoData, CollectionCustomization]
public async Task CanManageCollectionAccessAsync_TargetCollectionsMultipleOrgs_Failure(
public async Task HandleRequirementAsync_TargetCollectionsMultipleOrgs_Failure(
SutProvider<CollectionAuthorizationHandler> sutProvider,
IList<Collection> collections)
{
@ -85,7 +150,7 @@ public class CollectionAuthorizationHandlerTests
collections.First().OrganizationId = Guid.NewGuid();
var context = new AuthorizationHandlerContext(
new[] { CollectionOperation.ModifyAccess },
new[] { CollectionOperations.Create },
new ClaimsPrincipal(),
collections
);
@ -94,34 +159,27 @@ public class CollectionAuthorizationHandlerTests
var exception = await Assert.ThrowsAsync<BadRequestException>(() => sutProvider.Sut.HandleAsync(context));
Assert.Equal("Requested collections must belong to the same organization.", exception.Message);
await sutProvider.GetDependency<ICurrentContext>().DidNotReceiveWithAnyArgs()
.OrganizationMembershipAsync(default, default);
sutProvider.GetDependency<ICurrentContext>().DidNotReceiveWithAnyArgs().GetOrganization(default);
}
[Theory, BitAutoData, CollectionCustomization]
public async Task CanManageCollectionAccessAsync_MissingOrgMembership_Failure(
public async Task HandleRequirementAsync_MissingOrg_Failure(
SutProvider<CollectionAuthorizationHandler> sutProvider,
ICollection<Collection> collections,
CurrentContextOrganization organization)
ICollection<Collection> collections)
{
var actingUserId = Guid.NewGuid();
var context = new AuthorizationHandlerContext(
new[] { CollectionOperation.ModifyAccess },
new[] { CollectionOperations.Create },
new ClaimsPrincipal(),
collections
);
// Simulate a missing org membership
organization.Id = Guid.NewGuid();
sutProvider.GetDependency<ICurrentContext>().UserId.Returns(actingUserId);
sutProvider.GetDependency<ICurrentContext>().OrganizationMembershipAsync(Arg.Any<IOrganizationUserRepository>(), actingUserId).Returns(new[] { organization });
sutProvider.GetDependency<ICurrentContext>().GetOrganization(Arg.Any<Guid>()).Returns((CurrentContextOrganization)null);
await sutProvider.Sut.HandleAsync(context);
Assert.True(context.HasFailed);
await sutProvider.GetDependency<ICollectionRepository>().DidNotReceiveWithAnyArgs()
.GetManyByUserIdAsync(default);
}
[Theory, BitAutoData, CollectionCustomization]
@ -145,18 +203,18 @@ public class CollectionAuthorizationHandlerTests
organization.Permissions.EditAnyCollection = false;
var context = new AuthorizationHandlerContext(
new[] { CollectionOperation.ModifyAccess },
new[] { CollectionOperations.ModifyAccess },
new ClaimsPrincipal(),
collections
);
sutProvider.GetDependency<ICurrentContext>().UserId.Returns(actingUserId);
sutProvider.GetDependency<ICurrentContext>().OrganizationMembershipAsync(Arg.Any<IOrganizationUserRepository>(), actingUserId).Returns(new[] { organization });
sutProvider.GetDependency<ICurrentContext>().GetOrganization(Arg.Any<Guid>()).Returns(organization);
sutProvider.GetDependency<ICollectionRepository>().GetManyByUserIdAsync(actingUserId).Returns(collectionDetails);
await sutProvider.Sut.HandleAsync(context);
Assert.True(context.HasFailed);
await sutProvider.GetDependency<ICurrentContext>().ReceivedWithAnyArgs().OrganizationMembershipAsync(default, default);
sutProvider.GetDependency<ICurrentContext>().ReceivedWithAnyArgs().GetOrganization(default);
await sutProvider.GetDependency<ICollectionRepository>().ReceivedWithAnyArgs()
.GetManyByUserIdAsync(default);
}

19
test/Core.Test/Services/OrganizationServiceTests.cs
View File

@ -593,12 +593,19 @@ public class OrganizationServiceTests
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);
currentContext.GetOrganization(organization.Id)
.Returns(new CurrentContextOrganization()
{
Permissions = new Permissions
{
CreateNewCollections = true,
DeleteAnyCollection = true
}
});
await sutProvider.Sut.InviteUsersAsync(organization.Id, invitor.UserId, invites);
@ -928,6 +935,14 @@ public class OrganizationServiceTests
currentContext.OrganizationCustom(savingUser.OrganizationId).Returns(true);
currentContext.ManageUsers(savingUser.OrganizationId).Returns(true);
currentContext.AccessReports(savingUser.OrganizationId).Returns(true);
currentContext.GetOrganization(savingUser.OrganizationId).Returns(
new CurrentContextOrganization()
{
Permissions = new Permissions
{
AccessReports = true
}
});
await sutProvider.Sut.SaveUserAsync(newUserData, savingUser.UserId, collections, groups);
}