[AC-1174] CollectionUser and CollectionGroup authorization handlers (#3194)

* [AC-1174] Introduce BulkAuthorizationHandler.cs

* [AC-1174] Introduce CollectionUserAuthorizationHandler

* [AC-1174] Add CreateForNewCollection CollectionUser requirement

* [AC-1174] Add some more details to CollectionCustomization

* [AC-1174] Formatting

* [AC-1174] Add CollectionGroupOperation.cs

* [AC-1174] Introduce CollectionGroupAuthorizationHandler.cs

* [AC-1174] Cleanup CollectionFixture customization

Implement and use re-usable extension method to support seeded Guids

* [AC-1174] Introduce WithValueFromList AutoFixtureExtensions

Modify CollectionCustomization to use multiple organization Ids for auto generated test data

* [AC-1174] Simplify CollectionUserAuthorizationHandler.cs

Modify the authorization handler to only perform authorization logic. Validation logic will need to be handled by any calling commands/controllers instead.

* [AC-1174] Introduce shared CollectionAccessAuthorizationHandlerBase

A shared base authorization handler was created for both CollectionUser and CollectionGroup resources, as they share the same underlying management authorization logic.

* [AC-1174] Update CollectionUserAuthorizationHandler and CollectionGroupAuthorizationHandler to use the new CollectionAccessAuthorizationHandlerBase class

* [AC-1174] Formatting

* [AC-1174] Cleanup typo and redundant ToList() call

* [AC-1174] Add check for provider users

* [AC-1174] Reduce nested loops

* [AC-1174] Introduce ICollectionAccess.cs

* [AC-1174] Remove individual CollectionGroup and CollectionUser auth handlers and use base class instead

* [AC-1174] Tweak unit test to fail minimally

* [AC-1174] Reorganize authorization handlers in Core project

* [AC-1174] Introduce new AddCoreAuthorizationHandlers() extension method

* [AC-1174] Move CollectionAccessAuthorizationHandler into Api project

* [AC-1174] Move CollectionFixture to Vault folder

* [AC-1174] Rename operation to CreateUpdateDelete

* [AC-1174] Require single organization for collection access authorization handler

- Add requirement that all target collections must belong to the same organization
- Simplify logic related to multiple organizations
- Update tests and helpers
- Use ToHashSet to improve lookup time

* [AC-1174] Fix null reference exception

* [AC-1174] Throw bad request exception when collections belong to different organizations

* [AC-1174] Switch to CollectionAuthorizationHandler instead of CollectionAccessAuthorizationHandler to reduce complexity

src/Api/Startup.cs

@@ -137,6 +137,9 @@ public class Startup
         services.AddOrganizationSubscriptionServices();
         services.AddCoreLocalizationServices();
 
+        // Authorization Handlers
+        services.AddAuthorizationHandlers();
+
         //health check
         if (!globalSettings.SelfHosted)
         {

src/Api/Utilities/ServiceCollectionExtensions.cs

@@ -1,7 +1,9 @@
-using Bit.Core.IdentityServer;
+using Bit.Api.Vault.AuthorizationHandlers;
+using Bit.Core.IdentityServer;
 using Bit.Core.Settings;
 using Bit.Core.Utilities;
 using Bit.SharedWeb.Health;
+using Microsoft.AspNetCore.Authorization;
 using Microsoft.OpenApi.Models;
 
 namespace Bit.Api.Utilities;
@@ -115,4 +117,9 @@ public static class ServiceCollectionExtensions
             }
         });
     }
+
+    public static void AddAuthorizationHandlers(this IServiceCollection services)
+    {
+        services.AddScoped<IAuthorizationHandler, CollectionAuthorizationHandler>();
+    }
 }

src/Api/Vault/AuthorizationHandlers/CollectionAuthorizationHandler.cs

@@ -0,0 +1,90 @@
+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);
+    }
+}

src/Api/Vault/AuthorizationHandlers/CollectionOperation.cs

@@ -0,0 +1,15 @@
+using Microsoft.AspNetCore.Authorization.Infrastructure;
+
+namespace Bit.Api.Vault.AuthorizationHandlers;
+
+public class CollectionOperationRequirement : OperationAuthorizationRequirement { }
+
+public static class CollectionOperation
+{
+    /// <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
+    /// as they all currently share the same underlying authorization logic.
+    /// </summary>
+    public static readonly CollectionOperationRequirement ModifyAccess = new() { Name = nameof(ModifyAccess) };
+}

src/Core/Utilities/BulkAuthorizationHandler.cs

@@ -0,0 +1,40 @@
+using Microsoft.AspNetCore.Authorization;
+
+namespace Bit.Core.Utilities;
+
+/// <summary>
+/// Allows a single authorization handler implementation to handle requirements for
+/// both singular or bulk operations on single or multiple resources.
+/// </summary>
+/// <typeparam name="TRequirement">The type of the requirement to evaluate.</typeparam>
+/// <typeparam name="TResource">The type of the resource(s) that will be evaluated.</typeparam>
+public abstract class BulkAuthorizationHandler<TRequirement, TResource> : AuthorizationHandler<TRequirement>
+    where TRequirement : IAuthorizationRequirement
+{
+    protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, TRequirement requirement)
+    {
+        // Attempt to get the resource(s) from the context
+        var bulkResources = GetBulkResourceFromContext(context);
+
+        // No resources of the expected type were found in the context, nothing to evaluate
+        if (bulkResources == null)
+        {
+            return;
+        }
+
+        await HandleRequirementAsync(context, requirement, bulkResources);
+    }
+
+    private static ICollection<TResource> GetBulkResourceFromContext(AuthorizationHandlerContext context)
+    {
+        return context.Resource switch
+        {
+            TResource resource => new List<TResource> { resource },
+            IEnumerable<TResource> resources => resources.ToList(),
+            _ => null
+        };
+    }
+
+    protected abstract Task HandleRequirementAsync(AuthorizationHandlerContext context, TRequirement requirement,
+        ICollection<TResource> resources);
+}

test/Api.Test/Vault/AuthorizationHandlers/CollectionAuthorizationHandlerTests.cs

@@ -0,0 +1,163 @@
+using System.Security.Claims;
+using Bit.Api.Vault.AuthorizationHandlers;
+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.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]
+public class CollectionAuthorizationHandlerTests
+{
+    [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<CollectionAuthorizationHandler> sutProvider,
+        ICollection<Collection> collections,
+        ICollection<CollectionDetails> collectionDetails,
+        CurrentContentOrganization 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[] { CollectionOperation.ModifyAccess },
+            new ClaimsPrincipal(),
+            collections);
+
+        sutProvider.GetDependency<ICurrentContext>().UserId.Returns(actingUserId);
+        sutProvider.GetDependency<ICurrentContext>().OrganizationMembershipAsync(Arg.Any<IOrganizationUserRepository>(), actingUserId).Returns(new[] { 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(
+        SutProvider<CollectionAuthorizationHandler> sutProvider,
+        ICollection<Collection> collections)
+    {
+        var context = new AuthorizationHandlerContext(
+            new[] { CollectionOperation.ModifyAccess },
+            new ClaimsPrincipal(),
+            collections
+        );
+
+        // Simulate missing user id
+        sutProvider.GetDependency<ICurrentContext>().UserId.Returns((Guid?)null);
+
+        await sutProvider.Sut.HandleAsync(context);
+        Assert.True(context.HasFailed);
+        sutProvider.GetDependency<ICollectionRepository>().DidNotReceiveWithAnyArgs();
+    }
+
+    [Theory, BitAutoData, CollectionCustomization]
+    public async Task CanManageCollectionAccessAsync_TargetCollectionsMultipleOrgs_Failure(
+        SutProvider<CollectionAuthorizationHandler> sutProvider,
+        IList<Collection> collections)
+    {
+        var actingUserId = Guid.NewGuid();
+
+        // Simulate a collection in a different organization
+        collections.First().OrganizationId = Guid.NewGuid();
+
+        var context = new AuthorizationHandlerContext(
+            new[] { CollectionOperation.ModifyAccess },
+            new ClaimsPrincipal(),
+            collections
+        );
+
+        sutProvider.GetDependency<ICurrentContext>().UserId.Returns(actingUserId);
+
+        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);
+    }
+
+    [Theory, BitAutoData, CollectionCustomization]
+    public async Task CanManageCollectionAccessAsync_MissingOrgMembership_Failure(
+        SutProvider<CollectionAuthorizationHandler> sutProvider,
+        ICollection<Collection> collections,
+        CurrentContentOrganization organization)
+    {
+        var actingUserId = Guid.NewGuid();
+
+        var context = new AuthorizationHandlerContext(
+            new[] { CollectionOperation.ModifyAccess },
+            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 });
+
+        await sutProvider.Sut.HandleAsync(context);
+        Assert.True(context.HasFailed);
+        await sutProvider.GetDependency<ICollectionRepository>().DidNotReceiveWithAnyArgs()
+            .GetManyByUserIdAsync(default);
+    }
+
+    [Theory, BitAutoData, CollectionCustomization]
+    public async Task CanManageCollectionAccessAsync_MissingManageCollectionPermission_Failure(
+        SutProvider<CollectionAuthorizationHandler> sutProvider,
+        ICollection<Collection> collections,
+        ICollection<CollectionDetails> collectionDetails,
+        CurrentContentOrganization 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[] { CollectionOperation.ModifyAccess },
+            new ClaimsPrincipal(),
+            collections
+        );
+
+        sutProvider.GetDependency<ICurrentContext>().UserId.Returns(actingUserId);
+        sutProvider.GetDependency<ICurrentContext>().OrganizationMembershipAsync(Arg.Any<IOrganizationUserRepository>(), actingUserId).Returns(new[] { 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);
+        await sutProvider.GetDependency<ICollectionRepository>().ReceivedWithAnyArgs()
+            .GetManyByUserIdAsync(default);
+    }
+}

test/Core.Test/AutoFixture/AutoFixtureExtensions.cs

@@ -0,0 +1,54 @@
+using System.Linq.Expressions;
+using AutoFixture.Dsl;
+
+namespace Bit.Core.Test.AutoFixture;
+
+public static class AutoFixtureExtensions
+{
+    /// <summary>
+    /// Registers that a writable Guid property should be assigned a random value that is derived from the given seed.
+    /// </summary>
+    /// <remarks>
+    /// This can be used to generate random Guids that are deterministic based on the seed and thus can be re-used for
+    /// different entities that share the same identifiers. e.g. Collections, CollectionUsers, and CollectionGroups can
+    /// all have the same Guids generate for their "collection id" properties.
+    /// </remarks>
+    /// <param name="composer"></param>
+    /// <param name="propertyPicker">The Guid property to register</param>
+    /// <param name="seed">The random seed to use for random Guid generation</param>
+    public static IPostprocessComposer<T> WithGuidFromSeed<T>(this IPostprocessComposer<T> composer, Expression<Func<T, Guid>> propertyPicker, int seed)
+    {
+        var rnd = new Random(seed);
+        return composer.With(propertyPicker, () =>
+        {
+            // While not as random/unique as Guid.NewGuid(), this is works well enough for testing purposes.
+            var bytes = new byte[16];
+            rnd.NextBytes(bytes);
+            return new Guid(bytes);
+        });
+    }
+
+    /// <summary>
+    /// Registers that a writable property should be assigned a value from the given list.
+    /// </summary>
+    /// <remarks>
+    /// The value will be assigned in the order that the list is enumerated. Values will wrap around to the beginning
+    /// should the end of the list be reached.
+    /// </remarks>
+    /// <param name="composer"></param>
+    /// <param name="propertyPicker"></param>
+    /// <param name="values"></param>
+    public static IPostprocessComposer<T> WithValueFromList<T, TValue>(
+        this IPostprocessComposer<T> composer,
+        Expression<Func<T, TValue>> propertyPicker,
+        ICollection<TValue> values)
+    {
+        var index = 0;
+        return composer.With(propertyPicker, () =>
+        {
+            var value = values.ElementAt(index);
+            index = (index + 1) % values.Count;
+            return value;
+        });
+    }
+}

test/Core.Test/Utilities/BulkAuthorizationHandlerTests.cs

@@ -0,0 +1,72 @@
+using System.Security.Claims;
+using Bit.Core.Utilities;
+using Microsoft.AspNetCore.Authorization;
+using Microsoft.AspNetCore.Authorization.Infrastructure;
+using Xunit;
+
+namespace Bit.Core.Test.Utilities;
+
+public class BulkAuthorizationHandlerTests
+{
+    [Fact]
+    public async Task HandleRequirementAsync_SingleResource_Success()
+    {
+        var handler = new TestBulkAuthorizationHandler();
+        var context = new AuthorizationHandlerContext(
+            new[] { new TestOperationRequirement() },
+            new ClaimsPrincipal(),
+            new TestResource());
+        await handler.HandleAsync(context);
+        Assert.True(context.HasSucceeded);
+    }
+
+    [Fact]
+    public async Task HandleRequirementAsync_BulkResource_Success()
+    {
+        var handler = new TestBulkAuthorizationHandler();
+        var context = new AuthorizationHandlerContext(
+            new[] { new TestOperationRequirement() },
+            new ClaimsPrincipal(),
+            new[] { new TestResource(), new TestResource() });
+        await handler.HandleAsync(context);
+        Assert.True(context.HasSucceeded);
+    }
+
+    [Fact]
+    public async Task HandleRequirementAsync_NoResources_Failure()
+    {
+        var handler = new TestBulkAuthorizationHandler();
+        var context = new AuthorizationHandlerContext(
+            new[] { new TestOperationRequirement() },
+            new ClaimsPrincipal(),
+            null);
+        await handler.HandleAsync(context);
+        Assert.False(context.HasSucceeded);
+    }
+
+    [Fact]
+    public async Task HandleRequirementAsync_WrongResourceType_Failure()
+    {
+        var handler = new TestBulkAuthorizationHandler();
+        var context = new AuthorizationHandlerContext(
+            new[] { new TestOperationRequirement() },
+            new ClaimsPrincipal(),
+            new object());
+        await handler.HandleAsync(context);
+        Assert.False(context.HasSucceeded);
+    }
+
+    private class TestOperationRequirement : OperationAuthorizationRequirement { }
+
+    private class TestResource { }
+
+    private class TestBulkAuthorizationHandler : BulkAuthorizationHandler<TestOperationRequirement, TestResource>
+    {
+        protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context,
+            TestOperationRequirement requirement,
+            ICollection<TestResource> resources)
+        {
+            context.Succeed(requirement);
+        }
+    }
+}

test/Core.Test/Vault/AutoFixture/CollectionFixture.cs

@@ -0,0 +1,52 @@
+using AutoFixture;
+using Bit.Core.Context;
+using Bit.Core.Entities;
+using Bit.Core.Models.Data;
+using Bit.Core.Test.AutoFixture;
+using Bit.Test.Common.AutoFixture.Attributes;
+
+namespace Bit.Core.Test.Vault.AutoFixture;
+
+public class CollectionCustomization : ICustomization
+{
+    private const int _collectionIdSeed = 1;
+    private const int _userIdSeed = 2;
+    private const int _groupIdSeed = 3;
+
+    public void Customize(IFixture fixture)
+    {
+        var orgId = Guid.NewGuid();
+
+        fixture.Customize<CurrentContentOrganization>(composer => composer
+            .With(o => o.Id, orgId));
+
+        fixture.Customize<OrganizationUser>(composer => composer
+            .With(o => o.OrganizationId, orgId)
+            .WithGuidFromSeed(o => o.Id, _userIdSeed));
+
+        fixture.Customize<Collection>(composer => composer
+            .With(o => o.OrganizationId, orgId)
+            .WithGuidFromSeed(c => c.Id, _collectionIdSeed));
+
+        fixture.Customize<CollectionDetails>(composer => composer
+            .With(o => o.OrganizationId, orgId)
+            .WithGuidFromSeed(cd => cd.Id, _collectionIdSeed));
+
+        fixture.Customize<CollectionUser>(c => c
+            .WithGuidFromSeed(cu => cu.OrganizationUserId, _userIdSeed)
+            .WithGuidFromSeed(cu => cu.CollectionId, _collectionIdSeed));
+
+        fixture.Customize<Group>(composer => composer
+            .With(o => o.OrganizationId, orgId)
+            .WithGuidFromSeed(o => o.Id, _groupIdSeed));
+
+        fixture.Customize<CollectionGroup>(c => c
+            .WithGuidFromSeed(cu => cu.GroupId, _groupIdSeed)
+            .WithGuidFromSeed(cu => cu.CollectionId, _collectionIdSeed));
+    }
+}
+
+public class CollectionCustomizationAttribute : BitCustomizeAttribute
+{
+    public override ICustomization GetCustomization() => new CollectionCustomization();
+}