diff --git a/src/Api/Startup.cs b/src/Api/Startup.cs index a642dcb10f..ffdfba9c14 100644 --- a/src/Api/Startup.cs +++ b/src/Api/Startup.cs @@ -137,6 +137,9 @@ public class Startup services.AddOrganizationSubscriptionServices(); services.AddCoreLocalizationServices(); + // Authorization Handlers + services.AddAuthorizationHandlers(); + //health check if (!globalSettings.SelfHosted) { diff --git a/src/Api/Utilities/ServiceCollectionExtensions.cs b/src/Api/Utilities/ServiceCollectionExtensions.cs index af7fa0116f..ef77a32c6a 100644 --- a/src/Api/Utilities/ServiceCollectionExtensions.cs +++ b/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(); + } } diff --git a/src/Api/Vault/AuthorizationHandlers/CollectionAuthorizationHandler.cs b/src/Api/Vault/AuthorizationHandlers/CollectionAuthorizationHandler.cs new file mode 100644 index 0000000000..5434332a6d --- /dev/null +++ b/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 +{ + 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 resources) + { + switch (requirement) + { + case not null when requirement == CollectionOperation.ModifyAccess: + await CanManageCollectionAccessAsync(context, requirement, resources); + break; + } + } + + /// + /// Ensures the acting user is allowed to manage access permissions for the target collections. + /// + private async Task CanManageCollectionAccessAsync(AuthorizationHandlerContext context, IAuthorizationRequirement requirement, ICollection 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); + } +} diff --git a/src/Api/Vault/AuthorizationHandlers/CollectionOperation.cs b/src/Api/Vault/AuthorizationHandlers/CollectionOperation.cs new file mode 100644 index 0000000000..f8b0008240 --- /dev/null +++ b/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 +{ + /// + /// 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. + /// + public static readonly CollectionOperationRequirement ModifyAccess = new() { Name = nameof(ModifyAccess) }; +} diff --git a/src/Core/Utilities/BulkAuthorizationHandler.cs b/src/Core/Utilities/BulkAuthorizationHandler.cs new file mode 100644 index 0000000000..c427a426e0 --- /dev/null +++ b/src/Core/Utilities/BulkAuthorizationHandler.cs @@ -0,0 +1,40 @@ +using Microsoft.AspNetCore.Authorization; + +namespace Bit.Core.Utilities; + +/// +/// Allows a single authorization handler implementation to handle requirements for +/// both singular or bulk operations on single or multiple resources. +/// +/// The type of the requirement to evaluate. +/// The type of the resource(s) that will be evaluated. +public abstract class BulkAuthorizationHandler : AuthorizationHandler + 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 GetBulkResourceFromContext(AuthorizationHandlerContext context) + { + return context.Resource switch + { + TResource resource => new List { resource }, + IEnumerable resources => resources.ToList(), + _ => null + }; + } + + protected abstract Task HandleRequirementAsync(AuthorizationHandlerContext context, TRequirement requirement, + ICollection resources); +} diff --git a/test/Api.Test/Vault/AuthorizationHandlers/CollectionAuthorizationHandlerTests.cs b/test/Api.Test/Vault/AuthorizationHandlers/CollectionAuthorizationHandlerTests.cs new file mode 100644 index 0000000000..34fead7ac0 --- /dev/null +++ b/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 sutProvider, + ICollection collections, + ICollection 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().UserId.Returns(actingUserId); + sutProvider.GetDependency().OrganizationMembershipAsync(Arg.Any(), actingUserId).Returns(new[] { organization }); + sutProvider.GetDependency().GetManyByUserIdAsync(actingUserId).Returns(collectionDetails); + + await sutProvider.Sut.HandleAsync(context); + + Assert.True(context.HasSucceeded); + } + + + [Theory, BitAutoData, CollectionCustomization] + public async Task CanManageCollectionAccessAsync_MissingUserId_Failure( + SutProvider sutProvider, + ICollection collections) + { + var context = new AuthorizationHandlerContext( + new[] { CollectionOperation.ModifyAccess }, + 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 CanManageCollectionAccessAsync_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[] { CollectionOperation.ModifyAccess }, + 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); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .OrganizationMembershipAsync(default, default); + } + + [Theory, BitAutoData, CollectionCustomization] + public async Task CanManageCollectionAccessAsync_MissingOrgMembership_Failure( + SutProvider sutProvider, + ICollection 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().UserId.Returns(actingUserId); + sutProvider.GetDependency().OrganizationMembershipAsync(Arg.Any(), actingUserId).Returns(new[] { organization }); + + await sutProvider.Sut.HandleAsync(context); + Assert.True(context.HasFailed); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .GetManyByUserIdAsync(default); + } + + [Theory, BitAutoData, CollectionCustomization] + public async Task CanManageCollectionAccessAsync_MissingManageCollectionPermission_Failure( + SutProvider sutProvider, + ICollection collections, + ICollection 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().UserId.Returns(actingUserId); + sutProvider.GetDependency().OrganizationMembershipAsync(Arg.Any(), actingUserId).Returns(new[] { organization }); + sutProvider.GetDependency().GetManyByUserIdAsync(actingUserId).Returns(collectionDetails); + + await sutProvider.Sut.HandleAsync(context); + Assert.True(context.HasFailed); + await sutProvider.GetDependency().ReceivedWithAnyArgs().OrganizationMembershipAsync(default, default); + await sutProvider.GetDependency().ReceivedWithAnyArgs() + .GetManyByUserIdAsync(default); + } +} diff --git a/test/Core.Test/AutoFixture/AutoFixtureExtensions.cs b/test/Core.Test/AutoFixture/AutoFixtureExtensions.cs new file mode 100644 index 0000000000..8657b2ff53 --- /dev/null +++ b/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 +{ + /// + /// Registers that a writable Guid property should be assigned a random value that is derived from the given seed. + /// + /// + /// 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. + /// + /// + /// The Guid property to register + /// The random seed to use for random Guid generation + public static IPostprocessComposer WithGuidFromSeed(this IPostprocessComposer composer, Expression> 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); + }); + } + + /// + /// Registers that a writable property should be assigned a value from the given list. + /// + /// + /// 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. + /// + /// + /// + /// + public static IPostprocessComposer WithValueFromList( + this IPostprocessComposer composer, + Expression> propertyPicker, + ICollection values) + { + var index = 0; + return composer.With(propertyPicker, () => + { + var value = values.ElementAt(index); + index = (index + 1) % values.Count; + return value; + }); + } +} diff --git a/test/Core.Test/Utilities/BulkAuthorizationHandlerTests.cs b/test/Core.Test/Utilities/BulkAuthorizationHandlerTests.cs new file mode 100644 index 0000000000..88efbd3d95 --- /dev/null +++ b/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 + { + protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, + TestOperationRequirement requirement, + ICollection resources) + { + context.Succeed(requirement); + } + } +} diff --git a/test/Core.Test/Vault/AutoFixture/CollectionFixture.cs b/test/Core.Test/Vault/AutoFixture/CollectionFixture.cs new file mode 100644 index 0000000000..541f43b35d --- /dev/null +++ b/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(composer => composer + .With(o => o.Id, orgId)); + + fixture.Customize(composer => composer + .With(o => o.OrganizationId, orgId) + .WithGuidFromSeed(o => o.Id, _userIdSeed)); + + fixture.Customize(composer => composer + .With(o => o.OrganizationId, orgId) + .WithGuidFromSeed(c => c.Id, _collectionIdSeed)); + + fixture.Customize(composer => composer + .With(o => o.OrganizationId, orgId) + .WithGuidFromSeed(cd => cd.Id, _collectionIdSeed)); + + fixture.Customize(c => c + .WithGuidFromSeed(cu => cu.OrganizationUserId, _userIdSeed) + .WithGuidFromSeed(cu => cu.CollectionId, _collectionIdSeed)); + + fixture.Customize(composer => composer + .With(o => o.OrganizationId, orgId) + .WithGuidFromSeed(o => o.Id, _groupIdSeed)); + + fixture.Customize(c => c + .WithGuidFromSeed(cu => cu.GroupId, _groupIdSeed) + .WithGuidFromSeed(cu => cu.CollectionId, _collectionIdSeed)); + } +} + +public class CollectionCustomizationAttribute : BitCustomizeAttribute +{ + public override ICustomization GetCustomization() => new CollectionCustomization(); +}