diff --git a/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs b/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs new file mode 100644 index 0000000000..73e3e19619 --- /dev/null +++ b/src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs @@ -0,0 +1,205 @@ +using Bit.Core; +using Bit.Core.Context; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Core.Utilities; +using Microsoft.AspNetCore.Authorization; + +namespace Bit.Api.Vault.AuthorizationHandlers.Collections; + +/// +/// Handles authorization logic for Collection objects, including access permissions for users and groups. +/// This uses new logic implemented in the Flexible Collections initiative. +/// +public class BulkCollectionAuthorizationHandler : BulkAuthorizationHandler +{ + private readonly ICurrentContext _currentContext; + private readonly ICollectionRepository _collectionRepository; + private readonly IFeatureService _featureService; + + private bool FlexibleCollectionsIsEnabled => _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext); + + public BulkCollectionAuthorizationHandler( + ICurrentContext currentContext, + ICollectionRepository collectionRepository, + IFeatureService featureService) + { + _currentContext = currentContext; + _collectionRepository = collectionRepository; + _featureService = featureService; + } + + protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, + CollectionOperationRequirement requirement, ICollection resources) + { + if (!FlexibleCollectionsIsEnabled) + { + // Flexible collections is OFF, should not be using this handler + throw new FeatureUnavailableException("Flexible collections is OFF when it should be ON."); + } + + // Establish pattern of authorization handler null checking passed resources + if (resources == null) + { + context.Fail(); + return; + } + + if (!_currentContext.UserId.HasValue) + { + context.Fail(); + return; + } + + var targetOrganizationId = requirement.OrganizationId != default + ? requirement.OrganizationId : resources.FirstOrDefault()?.OrganizationId ?? default; + + if (targetOrganizationId == default) + { + context.Fail(); + return; + } + + // 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.Read: + await CanReadAsync(context, requirement, resources, org); + break; + + case not null when requirement == CollectionOperations.Delete: + await CanDeleteAsync(context, requirement, resources, org); + break; + + case not null when requirement == CollectionOperations.Update: + 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.LimitCollectionCreationDeletion) + { + context.Succeed(requirement); + return; + } + + // Owners, Admins, Providers, and users with CreateNewCollections permission can always create collections + if ( + org.Type is OrganizationUserType.Owner or OrganizationUserType.Admin || + org.Permissions is { CreateNewCollections: true } || + await _currentContext.ProviderUserForOrgAsync(org.Id)) + { + context.Succeed(requirement); + return; + } + + context.Fail(); + } + + private async Task CanReadAsync(AuthorizationHandlerContext context, CollectionOperationRequirement requirement, + ICollection targetCollections, CurrentContextOrganization org) + { + if (org.Type is OrganizationUserType.Owner or OrganizationUserType.Admin || + await _currentContext.ProviderUserForOrgAsync(org.Id)) + { + context.Succeed(requirement); + return; + } + + await CheckCollectionPermissionsAsync(context, requirement, targetCollections, org, requireManagePermission: false); + } + + private async Task CanDeleteAsync(AuthorizationHandlerContext context, CollectionOperationRequirement requirement, + ICollection targetCollections, CurrentContextOrganization org) + { + // Owners, Admins, Providers, and users with DeleteAnyCollection permission can always delete collections + if ( + org.Type is OrganizationUserType.Owner or OrganizationUserType.Admin || + org.Permissions is { DeleteAnyCollection: true } || + 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.LimitCollectionCreationDeletion) + { + context.Fail(); + return; + } + + await CheckCollectionPermissionsAsync(context, requirement, targetCollections, org, requireManagePermission: true); + } + + /// + /// Ensures the acting user is allowed to manage access permissions for the target collections. + /// + private async Task CanManageCollectionAccessAsync(AuthorizationHandlerContext context, + IAuthorizationRequirement requirement, ICollection 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; + } + + await CheckCollectionPermissionsAsync(context, requirement, targetCollections, org, requireManagePermission: true); + } + + private async Task CheckCollectionPermissionsAsync( + AuthorizationHandlerContext context, + IAuthorizationRequirement requirement, + ICollection targetCollections, + CurrentContextOrganization org, + bool requireManagePermission) + { + // List of collection Ids the acting user has access to + var manageableCollectionIds = + (await _collectionRepository.GetManyByUserIdAsync(_currentContext.UserId!.Value)) + .Where(c => + // If requireManagePermission is true, check Collections with Manage permission + (!requireManagePermission || c.Manage) + && c.OrganizationId == org.Id) + .Select(c => c.Id) + .ToHashSet(); + + // The acting user does not have permissions for all target collections, fail + if (targetCollections.Any(tc => !manageableCollectionIds.Contains(tc.Id))) + { + context.Fail(); + return; + } + + context.Succeed(requirement); + } +} diff --git a/src/Api/Vault/AuthorizationHandlers/Collections/CollectionAuthorizationHandler.cs b/src/Api/Vault/AuthorizationHandlers/Collections/CollectionAuthorizationHandler.cs index 509d2fe433..5cde10d0c1 100644 --- a/src/Api/Vault/AuthorizationHandlers/Collections/CollectionAuthorizationHandler.cs +++ b/src/Api/Vault/AuthorizationHandlers/Collections/CollectionAuthorizationHandler.cs @@ -1,11 +1,8 @@ using Bit.Core; using Bit.Core.Context; -using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; -using Bit.Core.Repositories; using Bit.Core.Services; -using Bit.Core.Utilities; using Microsoft.AspNetCore.Authorization; namespace Bit.Api.Vault.AuthorizationHandlers.Collections; @@ -51,13 +48,7 @@ public class CollectionAuthorizationHandler : AuthorizationHandler -/// Handles authorization logic for Collection objects, including access permissions for users and groups. -/// This uses new logic implemented in the Flexible Collections initiative. -/// -public class BulkCollectionAuthorizationHandler : BulkAuthorizationHandler -{ - private readonly ICurrentContext _currentContext; - private readonly ICollectionRepository _collectionRepository; - private readonly IFeatureService _featureService; - - private bool FlexibleCollectionsIsEnabled => _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext); - - public BulkCollectionAuthorizationHandler( - ICurrentContext currentContext, - ICollectionRepository collectionRepository, - IFeatureService featureService) - { - _currentContext = currentContext; - _collectionRepository = collectionRepository; - _featureService = featureService; - } - - protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, - CollectionOperationRequirement requirement, ICollection resources) - { - if (!FlexibleCollectionsIsEnabled) + else { - // Flexible collections is OFF, should not be using this handler - throw new FeatureUnavailableException("Flexible collections is OFF when it should be ON."); - } - - // Establish pattern of authorization handler null checking passed resources - if (resources == null) - { - context.Fail(); - return; - } - - if (!_currentContext.UserId.HasValue) - { - context.Fail(); - return; - } - - var targetOrganizationId = requirement.OrganizationId != default - ? requirement.OrganizationId : resources.FirstOrDefault()?.OrganizationId ?? default; - - if (targetOrganizationId == default) - { - context.Fail(); - return; - } - - // 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.Read: - await CanReadAsync(context, requirement, resources, org); - break; - - case not null when requirement == CollectionOperations.Delete: - await CanDeleteAsync(context, requirement, resources, org); - break; - - case not null when requirement == CollectionOperations.Update: - 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.LimitCollectionCreationDeletion) - { - context.Succeed(requirement); - return; - } - - // Owners, Admins, Providers, and users with CreateNewCollections permission can always create collections - if ( - org.Type is OrganizationUserType.Owner or OrganizationUserType.Admin || - org.Permissions is { CreateNewCollections: true } || - await _currentContext.ProviderUserForOrgAsync(org.Id)) - { - context.Succeed(requirement); - return; + // Check if acting user is a provider user for the target organization + if (await _currentContext.ProviderUserForOrgAsync(requirement.OrganizationId)) + { + context.Succeed(requirement); + return; + } } + // Acting user is neither a member of the target organization or a provider user, fail context.Fail(); } - - private async Task CanReadAsync(AuthorizationHandlerContext context, CollectionOperationRequirement requirement, - ICollection targetCollections, CurrentContextOrganization org) - { - if (org.Type is OrganizationUserType.Owner or OrganizationUserType.Admin || - await _currentContext.ProviderUserForOrgAsync(org.Id)) - { - context.Succeed(requirement); - return; - } - - await CheckCollectionPermissionsAsync(context, requirement, targetCollections, org, requireManagePermission: false); - } - - private async Task CanDeleteAsync(AuthorizationHandlerContext context, CollectionOperationRequirement requirement, - ICollection targetCollections, CurrentContextOrganization org) - { - // Owners, Admins, Providers, and users with DeleteAnyCollection permission can always delete collections - if ( - org.Type is OrganizationUserType.Owner or OrganizationUserType.Admin || - org.Permissions is { DeleteAnyCollection: true } || - 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.LimitCollectionCreationDeletion) - { - context.Fail(); - return; - } - - await CheckCollectionPermissionsAsync(context, requirement, targetCollections, org, requireManagePermission: true); - } - - /// - /// Ensures the acting user is allowed to manage access permissions for the target collections. - /// - private async Task CanManageCollectionAccessAsync(AuthorizationHandlerContext context, - IAuthorizationRequirement requirement, ICollection 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; - } - - await CheckCollectionPermissionsAsync(context, requirement, targetCollections, org, requireManagePermission: true); - } - - private async Task CheckCollectionPermissionsAsync( - AuthorizationHandlerContext context, - IAuthorizationRequirement requirement, - ICollection targetCollections, - CurrentContextOrganization org, - bool requireManagePermission) - { - // List of collection Ids the acting user has access to - var manageableCollectionIds = - (await _collectionRepository.GetManyByUserIdAsync(_currentContext.UserId!.Value)) - .Where(c => - // If requireManagePermission is true, check Collections with Manage permission - (!requireManagePermission || c.Manage) - && c.OrganizationId == org.Id) - .Select(c => c.Id) - .ToHashSet(); - - // The acting user does not have permissions for all target collections, fail - if (targetCollections.Any(tc => !manageableCollectionIds.Contains(tc.Id))) - { - context.Fail(); - return; - } - - context.Succeed(requirement); - } } diff --git a/test/Api.Test/Controllers/CollectionsControllerTests.cs b/test/Api.Test/Controllers/CollectionsControllerTests.cs index 5bd5bfcb4c..04a31a7eb4 100644 --- a/test/Api.Test/Controllers/CollectionsControllerTests.cs +++ b/test/Api.Test/Controllers/CollectionsControllerTests.cs @@ -47,101 +47,90 @@ public class CollectionsControllerTests } [Theory, BitAutoData] - public async Task Put_Success(Guid orgId, Guid collectionId, Guid userId, CollectionRequestModel collectionRequest, + public async Task Put_Success(Collection collection, CollectionRequestModel collectionRequest, SutProvider sutProvider) { - sutProvider.GetDependency() - .ViewAssignedCollections(orgId) - .Returns(true); - - sutProvider.GetDependency() - .EditAssignedCollections(orgId) - .Returns(true); - - sutProvider.GetDependency() - .UserId - .Returns(userId); + Collection ExpectedCollection() => Arg.Is(c => c.Id == collection.Id && + c.Name == collectionRequest.Name && c.ExternalId == collectionRequest.ExternalId && + c.OrganizationId == collection.OrganizationId); sutProvider.GetDependency() - .GetByIdAsync(collectionId, userId) - .Returns(new CollectionDetails - { - OrganizationId = orgId, - }); + .GetByIdAsync(collection.Id) + .Returns(collection); - _ = await sutProvider.Sut.Put(orgId, collectionId, collectionRequest); + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), + collection, + Arg.Is>(r => r.Contains(CollectionOperations.Update))) + .Returns(AuthorizationResult.Success()); + + _ = await sutProvider.Sut.Put(collection.OrganizationId, collection.Id, collectionRequest); + + await sutProvider.GetDependency() + .Received(1) + .SaveAsync(ExpectedCollection(), Arg.Any>(), + Arg.Any>()); } [Theory, BitAutoData] - public async Task Put_CanNotEditAssignedCollection_ThrowsNotFound(Guid orgId, Guid collectionId, Guid userId, CollectionRequestModel collectionRequest, + public async Task Put_WithNoCollectionPermission_ThrowsNotFound(Collection collection, CollectionRequestModel collectionRequest, SutProvider sutProvider) { - sutProvider.GetDependency() - .EditAssignedCollections(orgId) - .Returns(true); - - sutProvider.GetDependency() - .UserId - .Returns(userId); + sutProvider.GetDependency() + .AuthorizeAsync(Arg.Any(), + collection, + Arg.Is>(r => r.Contains(CollectionOperations.Update))) + .Returns(AuthorizationResult.Failed()); sutProvider.GetDependency() - .GetByIdAsync(collectionId, userId) - .Returns(Task.FromResult(null)); + .GetByIdAsync(collection.Id) + .Returns(collection); - _ = await Assert.ThrowsAsync(async () => await sutProvider.Sut.Put(orgId, collectionId, collectionRequest)); + _ = await Assert.ThrowsAsync(async () => await sutProvider.Sut.Put(collection.OrganizationId, collection.Id, collectionRequest)); } [Theory, BitAutoData] - public async Task GetOrganizationCollectionsWithGroups_NoManagerPermissions_ThrowsNotFound(Organization organization, SutProvider sutProvider) + public async Task GetOrganizationCollectionsWithGroups_WithReadAllPermissions_GetsAllCollections(Organization organization, Guid userId, SutProvider sutProvider) { - sutProvider.GetDependency().ViewAssignedCollections(organization.Id).Returns(false); + sutProvider.GetDependency().UserId.Returns(userId); - await Assert.ThrowsAsync(() => sutProvider.Sut.GetManyWithDetails(organization.Id)); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().GetManyByOrganizationIdWithAccessAsync(default); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().GetManyByUserIdWithAccessAsync(default, default); - } - - [Theory, BitAutoData] - public async Task GetOrganizationCollectionsWithGroups_AdminPermissions_GetsAllCollections(Organization organization, User user, SutProvider sutProvider) - { - sutProvider.GetDependency().UserId.Returns(user.Id); - sutProvider.GetDependency().ViewAllCollections(organization.Id).Returns(true); - sutProvider.GetDependency().OrganizationAdmin(organization.Id).Returns(true); + sutProvider.GetDependency() + .AuthorizeAsync( + Arg.Any(), + Arg.Any(), + Arg.Is>(requirements => + requirements.Cast().All(operation => + operation.Name == nameof(CollectionOperations.ReadAll) + && operation.OrganizationId == organization.Id))) + .Returns(AuthorizationResult.Success()); await sutProvider.Sut.GetManyWithDetails(organization.Id); - await sutProvider.GetDependency().Received().GetManyByOrganizationIdWithAccessAsync(organization.Id); - await sutProvider.GetDependency().Received().GetManyByUserIdWithAccessAsync(user.Id, organization.Id); + await sutProvider.GetDependency().Received(1).GetManyByUserIdWithAccessAsync(userId, organization.Id); + await sutProvider.GetDependency().Received(1).GetManyByOrganizationIdWithAccessAsync(organization.Id); } [Theory, BitAutoData] - public async Task GetOrganizationCollectionsWithGroups_MissingViewAllPermissions_GetsAssignedCollections(Organization organization, User user, SutProvider sutProvider) + public async Task GetOrganizationCollectionsWithGroups_MissingReadAllPermissions_GetsAssignedCollections(Organization organization, Guid userId, SutProvider sutProvider) { - sutProvider.GetDependency().UserId.Returns(user.Id); - sutProvider.GetDependency().ViewAssignedCollections(organization.Id).Returns(true); - sutProvider.GetDependency().OrganizationManager(organization.Id).Returns(true); + sutProvider.GetDependency().UserId.Returns(userId); + + sutProvider.GetDependency() + .AuthorizeAsync( + Arg.Any(), + Arg.Any(), + Arg.Is>(requirements => + requirements.Cast().All(operation => + operation.Name == nameof(CollectionOperations.ReadAll) + && operation.OrganizationId == organization.Id))) + .Returns(AuthorizationResult.Failed()); await sutProvider.Sut.GetManyWithDetails(organization.Id); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().GetManyByOrganizationIdWithAccessAsync(default); - await sutProvider.GetDependency().Received().GetManyByUserIdWithAccessAsync(user.Id, organization.Id); + await sutProvider.GetDependency().Received(1).GetManyByUserIdWithAccessAsync(userId, organization.Id); + await sutProvider.GetDependency().DidNotReceive().GetManyByOrganizationIdWithAccessAsync(organization.Id); } - [Theory, BitAutoData] - public async Task GetOrganizationCollectionsWithGroups_CustomUserWithManagerPermissions_GetsAssignedCollections(Organization organization, User user, SutProvider sutProvider) - { - sutProvider.GetDependency().UserId.Returns(user.Id); - sutProvider.GetDependency().ViewAssignedCollections(organization.Id).Returns(true); - sutProvider.GetDependency().EditAssignedCollections(organization.Id).Returns(true); - - - await sutProvider.Sut.GetManyWithDetails(organization.Id); - - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().GetManyByOrganizationIdWithAccessAsync(default); - await sutProvider.GetDependency().Received().GetManyByUserIdWithAccessAsync(user.Id, organization.Id); - } - - [Theory, BitAutoData] public async Task DeleteMany_Success(Guid orgId, Collection collection1, Collection collection2, SutProvider sutProvider) { diff --git a/test/Api.Test/Vault/AuthorizationHandlers/BulkCollectionAuthorizationHandlerTests.cs b/test/Api.Test/Vault/AuthorizationHandlers/BulkCollectionAuthorizationHandlerTests.cs new file mode 100644 index 0000000000..b08a3be0bb --- /dev/null +++ b/test/Api.Test/Vault/AuthorizationHandlers/BulkCollectionAuthorizationHandlerTests.cs @@ -0,0 +1,224 @@ +using System.Security.Claims; +using Bit.Api.Vault.AuthorizationHandlers.Collections; +using Bit.Core; +using Bit.Core.Context; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.Models.Data; +using Bit.Core.Repositories; +using Bit.Core.Test.AutoFixture; +using Bit.Core.Test.Vault.AutoFixture; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Microsoft.AspNetCore.Authorization; +using NSubstitute; +using Xunit; + +namespace Bit.Api.Test.Vault.AuthorizationHandlers; + +[SutProviderCustomize] +[FeatureServiceCustomize(FeatureFlagKeys.FlexibleCollections)] +public class BulkCollectionAuthorizationHandlerTests +{ + [Theory, CollectionCustomization] + [BitAutoData(OrganizationUserType.User, false, true)] + [BitAutoData(OrganizationUserType.Admin, false, false)] + [BitAutoData(OrganizationUserType.Owner, false, false)] + [BitAutoData(OrganizationUserType.Custom, true, false)] + [BitAutoData(OrganizationUserType.Owner, true, true)] + public async Task CanManageCollectionAccessAsync_Success( + OrganizationUserType userType, bool editAnyCollection, bool manageCollections, + SutProvider sutProvider, + ICollection collections, + ICollection collectionDetails, + CurrentContextOrganization organization) + { + var actingUserId = Guid.NewGuid(); + foreach (var collectionDetail in collectionDetails) + { + collectionDetail.Manage = manageCollections; + } + + organization.Type = userType; + organization.Permissions.EditAnyCollection = editAnyCollection; + + var context = new AuthorizationHandlerContext( + new[] { CollectionOperations.ModifyAccess }, + new ClaimsPrincipal(), + collections); + + sutProvider.GetDependency().UserId.Returns(actingUserId); + sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); + sutProvider.GetDependency().GetManyByUserIdAsync(actingUserId).Returns(collectionDetails); + + await sutProvider.Sut.HandleAsync(context); + + 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 sutProvider, + ICollection collections, + CurrentContextOrganization organization) + { + var actingUserId = Guid.NewGuid(); + + organization.Type = userType; + organization.Permissions.CreateNewCollections = createNewCollection; + organization.LimitCollectionCreationDeletion = limitCollectionCreateDelete; + + var context = new AuthorizationHandlerContext( + new[] { CollectionOperations.Create }, + new ClaimsPrincipal(), + collections); + + sutProvider.GetDependency().UserId.Returns(actingUserId); + sutProvider.GetDependency().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 sutProvider, + ICollection collections, + ICollection collectionDetails, + CurrentContextOrganization organization) + { + var actingUserId = Guid.NewGuid(); + foreach (var collectionDetail in collectionDetails) + { + collectionDetail.Manage = manageCollections; + } + + organization.Type = userType; + organization.Permissions.DeleteAnyCollection = deleteAnyCollection; + organization.LimitCollectionCreationDeletion = limitCollectionCreateDelete; + + var context = new AuthorizationHandlerContext( + new[] { CollectionOperations.Delete }, + new ClaimsPrincipal(), + collections); + + sutProvider.GetDependency().UserId.Returns(actingUserId); + sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); + sutProvider.GetDependency().GetManyByUserIdAsync(actingUserId).Returns(collectionDetails); + + await sutProvider.Sut.HandleAsync(context); + + Assert.True(context.HasSucceeded); + } + + [Theory, BitAutoData, CollectionCustomization] + public async Task HandleRequirementAsync_MissingUserId_Failure( + SutProvider sutProvider, + ICollection collections) + { + var context = new AuthorizationHandlerContext( + new[] { CollectionOperations.Create }, + new ClaimsPrincipal(), + collections + ); + + // Simulate missing user id + sutProvider.GetDependency().UserId.Returns((Guid?)null); + + await sutProvider.Sut.HandleAsync(context); + Assert.True(context.HasFailed); + sutProvider.GetDependency().DidNotReceiveWithAnyArgs(); + } + + [Theory, BitAutoData, CollectionCustomization] + public async Task HandleRequirementAsync_TargetCollectionsMultipleOrgs_Failure( + SutProvider sutProvider, + IList collections) + { + var actingUserId = Guid.NewGuid(); + + // Simulate a collection in a different organization + collections.First().OrganizationId = Guid.NewGuid(); + + var context = new AuthorizationHandlerContext( + new[] { CollectionOperations.Create }, + new ClaimsPrincipal(), + collections + ); + + sutProvider.GetDependency().UserId.Returns(actingUserId); + + var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.HandleAsync(context)); + Assert.Equal("Requested collections must belong to the same organization.", exception.Message); + sutProvider.GetDependency().DidNotReceiveWithAnyArgs().GetOrganization(default); + } + + [Theory, BitAutoData, CollectionCustomization] + public async Task HandleRequirementAsync_MissingOrg_Failure( + SutProvider sutProvider, + ICollection collections) + { + var actingUserId = Guid.NewGuid(); + + var context = new AuthorizationHandlerContext( + new[] { CollectionOperations.Create }, + new ClaimsPrincipal(), + collections + ); + + sutProvider.GetDependency().UserId.Returns(actingUserId); + sutProvider.GetDependency().GetOrganization(Arg.Any()).Returns((CurrentContextOrganization)null); + + await sutProvider.Sut.HandleAsync(context); + Assert.True(context.HasFailed); + } + + [Theory, BitAutoData, CollectionCustomization] + public async Task CanManageCollectionAccessAsync_MissingManageCollectionPermission_Failure( + SutProvider sutProvider, + ICollection collections, + ICollection collectionDetails, + CurrentContextOrganization organization) + { + var actingUserId = Guid.NewGuid(); + + foreach (var collectionDetail in collectionDetails) + { + collectionDetail.Manage = true; + } + // Simulate one collection missing the manage permission + collectionDetails.First().Manage = false; + + // Ensure the user is not an owner/admin and does not have edit any collection permission + organization.Type = OrganizationUserType.User; + organization.Permissions.EditAnyCollection = false; + + var context = new AuthorizationHandlerContext( + new[] { CollectionOperations.ModifyAccess }, + new ClaimsPrincipal(), + collections + ); + + sutProvider.GetDependency().UserId.Returns(actingUserId); + sutProvider.GetDependency().GetOrganization(Arg.Any()).Returns(organization); + sutProvider.GetDependency().GetManyByUserIdAsync(actingUserId).Returns(collectionDetails); + + await sutProvider.Sut.HandleAsync(context); + Assert.True(context.HasFailed); + sutProvider.GetDependency().ReceivedWithAnyArgs().GetOrganization(default); + await sutProvider.GetDependency().ReceivedWithAnyArgs() + .GetManyByUserIdAsync(default); + } +} diff --git a/test/Api.Test/Vault/AuthorizationHandlers/CollectionAuthorizationHandlerTests.cs b/test/Api.Test/Vault/AuthorizationHandlers/CollectionAuthorizationHandlerTests.cs index aaaa3076b0..1ab0ee3ac0 100644 --- a/test/Api.Test/Vault/AuthorizationHandlers/CollectionAuthorizationHandlerTests.cs +++ b/test/Api.Test/Vault/AuthorizationHandlers/CollectionAuthorizationHandlerTests.cs @@ -2,13 +2,8 @@ using Bit.Api.Vault.AuthorizationHandlers.Collections; using Bit.Core; using Bit.Core.Context; -using Bit.Core.Entities; using Bit.Core.Enums; -using Bit.Core.Exceptions; -using Bit.Core.Models.Data; -using Bit.Core.Repositories; using Bit.Core.Test.AutoFixture; -using Bit.Core.Test.Vault.AutoFixture; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; using Microsoft.AspNetCore.Authorization; @@ -21,117 +16,75 @@ namespace Bit.Api.Test.Vault.AuthorizationHandlers; [FeatureServiceCustomize(FeatureFlagKeys.FlexibleCollections)] public class CollectionAuthorizationHandlerTests { - [Theory, CollectionCustomization] + [Theory] [BitAutoData(OrganizationUserType.User, false, true)] [BitAutoData(OrganizationUserType.Admin, false, false)] [BitAutoData(OrganizationUserType.Owner, false, false)] [BitAutoData(OrganizationUserType.Custom, true, false)] [BitAutoData(OrganizationUserType.Owner, true, true)] - public async Task CanManageCollectionAccessAsync_Success( - OrganizationUserType userType, bool editAnyCollection, bool manageCollections, - SutProvider sutProvider, - ICollection collections, - ICollection collectionDetails, + public async Task CanReadAllAccessAsync_Success( + OrganizationUserType userType, bool editAnyCollection, bool deleteAnyCollection, + Guid userId, SutProvider sutProvider, CurrentContextOrganization organization) { - var actingUserId = Guid.NewGuid(); - foreach (var collectionDetail in collectionDetails) - { - collectionDetail.Manage = manageCollections; - } + // if (org.Type is OrganizationUserType.Owner or OrganizationUserType.Admin || + // org.Permissions.ManageGroups || + // org.Permissions.ManageUsers || + // org.Permissions.EditAnyCollection || + // org.Permissions.DeleteAnyCollection || + // org.Permissions.AccessImportExport || + // await _currentContext.ProviderUserForOrgAsync(org.Id)) + // { + // context.Succeed(requirement); + // } organization.Type = userType; organization.Permissions.EditAnyCollection = editAnyCollection; - - var context = new AuthorizationHandlerContext( - new[] { CollectionOperations.ModifyAccess }, - new ClaimsPrincipal(), - collections); - - sutProvider.GetDependency().UserId.Returns(actingUserId); - sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); - sutProvider.GetDependency().GetManyByUserIdAsync(actingUserId).Returns(collectionDetails); - - await sutProvider.Sut.HandleAsync(context); - - 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 sutProvider, - ICollection collections, - CurrentContextOrganization organization) - { - var actingUserId = Guid.NewGuid(); - - organization.Type = userType; - organization.Permissions.CreateNewCollections = createNewCollection; - organization.LimitCollectionCreationDeletion = limitCollectionCreateDelete; - - var context = new AuthorizationHandlerContext( - new[] { CollectionOperations.Create }, - new ClaimsPrincipal(), - collections); - - sutProvider.GetDependency().UserId.Returns(actingUserId); - sutProvider.GetDependency().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 sutProvider, - ICollection collections, - ICollection collectionDetails, - CurrentContextOrganization organization) - { - var actingUserId = Guid.NewGuid(); - foreach (var collectionDetail in collectionDetails) - { - collectionDetail.Manage = manageCollections; - } - - organization.Type = userType; organization.Permissions.DeleteAnyCollection = deleteAnyCollection; - organization.LimitCollectionCreationDeletion = limitCollectionCreateDelete; var context = new AuthorizationHandlerContext( - new[] { CollectionOperations.Delete }, + new[] { CollectionOperations.ReadAll(organization.Id) }, new ClaimsPrincipal(), - collections); + null); - sutProvider.GetDependency().UserId.Returns(actingUserId); + sutProvider.GetDependency().UserId.Returns(userId); sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); - sutProvider.GetDependency().GetManyByUserIdAsync(actingUserId).Returns(collectionDetails); await sutProvider.Sut.HandleAsync(context); Assert.True(context.HasSucceeded); } - [Theory, BitAutoData, CollectionCustomization] + [Theory, BitAutoData] + public async Task CanReadAllAccessAsync_WithProviderUser_Success( + Guid userId, + SutProvider sutProvider, CurrentContextOrganization organization) + { + var context = new AuthorizationHandlerContext( + new[] { CollectionOperations.ReadAll(organization.Id) }, + new ClaimsPrincipal(), + null); + + sutProvider.GetDependency() + .UserId + .Returns(userId); + sutProvider.GetDependency() + .ProviderUserForOrgAsync(organization.Id) + .Returns(true); + + await sutProvider.Sut.HandleAsync(context); + + Assert.True(context.HasSucceeded); + } + + [Theory, BitAutoData] public async Task HandleRequirementAsync_MissingUserId_Failure( - SutProvider sutProvider, - ICollection collections) + SutProvider sutProvider) { var context = new AuthorizationHandlerContext( new[] { CollectionOperations.Create }, new ClaimsPrincipal(), - collections + null ); // Simulate missing user id @@ -139,86 +92,24 @@ public class CollectionAuthorizationHandlerTests await sutProvider.Sut.HandleAsync(context); Assert.True(context.HasFailed); - sutProvider.GetDependency().DidNotReceiveWithAnyArgs(); } - [Theory, BitAutoData, CollectionCustomization] - public async Task HandleRequirementAsync_TargetCollectionsMultipleOrgs_Failure( - SutProvider sutProvider, - IList collections) - { - var actingUserId = Guid.NewGuid(); - - // Simulate a collection in a different organization - collections.First().OrganizationId = Guid.NewGuid(); - - var context = new AuthorizationHandlerContext( - new[] { CollectionOperations.Create }, - new ClaimsPrincipal(), - collections - ); - - sutProvider.GetDependency().UserId.Returns(actingUserId); - - var exception = await Assert.ThrowsAsync(() => sutProvider.Sut.HandleAsync(context)); - Assert.Equal("Requested collections must belong to the same organization.", exception.Message); - sutProvider.GetDependency().DidNotReceiveWithAnyArgs().GetOrganization(default); - } - - [Theory, BitAutoData, CollectionCustomization] + [Theory, BitAutoData] public async Task HandleRequirementAsync_MissingOrg_Failure( - SutProvider sutProvider, - ICollection collections) + Guid userId, + Guid organizationId, + SutProvider sutProvider) { - var actingUserId = Guid.NewGuid(); - var context = new AuthorizationHandlerContext( - new[] { CollectionOperations.Create }, + new[] { CollectionOperations.ReadAll(organizationId) }, new ClaimsPrincipal(), - collections + null ); - sutProvider.GetDependency().UserId.Returns(actingUserId); + sutProvider.GetDependency().UserId.Returns(userId); sutProvider.GetDependency().GetOrganization(Arg.Any()).Returns((CurrentContextOrganization)null); await sutProvider.Sut.HandleAsync(context); Assert.True(context.HasFailed); } - - [Theory, BitAutoData, CollectionCustomization] - public async Task CanManageCollectionAccessAsync_MissingManageCollectionPermission_Failure( - SutProvider sutProvider, - ICollection collections, - ICollection collectionDetails, - CurrentContextOrganization organization) - { - var actingUserId = Guid.NewGuid(); - - foreach (var collectionDetail in collectionDetails) - { - collectionDetail.Manage = true; - } - // Simulate one collection missing the manage permission - collectionDetails.First().Manage = false; - - // Ensure the user is not an owner/admin and does not have edit any collection permission - organization.Type = OrganizationUserType.User; - organization.Permissions.EditAnyCollection = false; - - var context = new AuthorizationHandlerContext( - new[] { CollectionOperations.ModifyAccess }, - new ClaimsPrincipal(), - collections - ); - - sutProvider.GetDependency().UserId.Returns(actingUserId); - sutProvider.GetDependency().GetOrganization(Arg.Any()).Returns(organization); - sutProvider.GetDependency().GetManyByUserIdAsync(actingUserId).Returns(collectionDetails); - - await sutProvider.Sut.HandleAsync(context); - Assert.True(context.HasFailed); - sutProvider.GetDependency().ReceivedWithAnyArgs().GetOrganization(default); - await sutProvider.GetDependency().ReceivedWithAnyArgs() - .GetManyByUserIdAsync(default); - } } diff --git a/test/Core.Test/Services/OrganizationServiceTests.cs b/test/Core.Test/Services/OrganizationServiceTests.cs index e31989f37e..62b6dda35b 100644 --- a/test/Core.Test/Services/OrganizationServiceTests.cs +++ b/test/Core.Test/Services/OrganizationServiceTests.cs @@ -75,8 +75,11 @@ public class OrganizationServiceTests .CreateAsync(default); // Create new users - await sutProvider.GetDependency().Received(1) - .CreateManyAsync(Arg.Is>(users => users.Count() == expectedNewUsersCount)); + await sutProvider.GetDependency().Received(expectedNewUsersCount) + .CreateAsync( + Arg.Is(user => user.Status == OrganizationUserStatusType.Invited + && newUsers.Any(u => string.Equals(u.Email, user.Email, StringComparison.InvariantCultureIgnoreCase))), + Arg.Any>()); await sutProvider.GetDependency().Received(1) .BulkSendOrganizationInviteEmailAsync(org.Name, Arg.Is>(messages => messages.Count() == expectedNewUsersCount), org.PlanType == PlanType.Free); @@ -125,16 +128,17 @@ public class OrganizationServiceTests .UpsertAsync(default); await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() .CreateAsync(default); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() - .CreateAsync(default, default); // Upserted existing user await sutProvider.GetDependency().Received(1) .UpsertManyAsync(Arg.Is>(users => users.Count() == 1)); // Created and invited new users - await sutProvider.GetDependency().Received(1) - .CreateManyAsync(Arg.Is>(users => users.Count() == expectedNewUsersCount)); + await sutProvider.GetDependency().Received(expectedNewUsersCount) + .CreateAsync( + Arg.Is(user => user.Status == OrganizationUserStatusType.Invited + && newUsers.Any(u => string.Equals(u.Email, user.Email, StringComparison.InvariantCultureIgnoreCase))), + Arg.Any>()); await sutProvider.GetDependency().Received(1) .BulkSendOrganizationInviteEmailAsync(org.Name, Arg.Is>(messages => messages.Count() == expectedNewUsersCount), org.PlanType == PlanType.Free);