From f00dbf00528832e33589f52c581be7d377d5a2b3 Mon Sep 17 00:00:00 2001 From: Rui Tome Date: Tue, 24 Oct 2023 12:48:02 +0100 Subject: [PATCH] [AC-1139] Added permission checks for GroupsController.Get if FC feature flag is enabled --- src/Api/Controllers/GroupsController.cs | 46 +++++++---- .../Utilities/ServiceCollectionExtensions.cs | 3 + .../Groups/GroupAuthorizationHandler.cs | 81 +++++++++++++++++++ .../Groups/GroupOperations.cs | 22 +++++ 4 files changed, 135 insertions(+), 17 deletions(-) create mode 100644 src/Api/Vault/AuthorizationHandlers/Groups/GroupAuthorizationHandler.cs create mode 100644 src/Api/Vault/AuthorizationHandlers/Groups/GroupOperations.cs diff --git a/src/Api/Controllers/GroupsController.cs b/src/Api/Controllers/GroupsController.cs index 1dba0efc24..2c75bd3741 100644 --- a/src/Api/Controllers/GroupsController.cs +++ b/src/Api/Controllers/GroupsController.cs @@ -1,10 +1,9 @@ using Bit.Api.Models.Request; using Bit.Api.Models.Response; +using Bit.Api.Vault.AuthorizationHandlers.Groups; using Bit.Core; using Bit.Core.Context; -using Bit.Core.Entities; using Bit.Core.Exceptions; -using Bit.Core.Models.Data; using Bit.Core.OrganizationFeatures.Groups.Interfaces; using Bit.Core.Repositories; using Bit.Core.Services; @@ -25,6 +24,7 @@ public class GroupsController : Controller private readonly ICreateGroupCommand _createGroupCommand; private readonly IUpdateGroupCommand _updateGroupCommand; private readonly IFeatureService _featureService; + private readonly IAuthorizationService _authorizationService; private bool FlexibleCollectionsIsEnabled => _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext); @@ -47,6 +47,7 @@ public class GroupsController : Controller _updateGroupCommand = updateGroupCommand; _deleteGroupCommand = deleteGroupCommand; _featureService = featureService; + _authorizationService = authorizationService; } [HttpGet("{id}")] @@ -76,27 +77,24 @@ public class GroupsController : Controller [HttpGet("")] public async Task> Get(Guid orgId) { - ICollection>> groups; - if (FlexibleCollectionsIsEnabled) { - groups = await _groupRepository.GetManyWithCollectionsByOrganizationIdAsync(orgId); + // New flexible collections logic + return await Get_FC(orgId); } - else + + // Old pre-flexible collections logic follows + var canAccess = await _currentContext.ManageGroups(orgId) || + await _currentContext.ViewAssignedCollections(orgId) || + await _currentContext.ViewAllCollections(orgId) || + await _currentContext.ManageUsers(orgId); + + if (!canAccess) { - var canAccess = await _currentContext.ManageGroups(orgId) || - await _currentContext.ViewAssignedCollections(orgId) || - await _currentContext.ViewAllCollections(orgId) || - await _currentContext.ManageUsers(orgId); - - if (!canAccess) - { - throw new NotFoundException(); - } - - groups = await _groupRepository.GetManyWithCollectionsByOrganizationIdAsync(orgId); + throw new NotFoundException(); } + var groups = await _groupRepository.GetManyWithCollectionsByOrganizationIdAsync(orgId); var responses = groups.Select(g => new GroupDetailsResponseModel(g.Item1, g.Item2)); return new ListResponseModel(responses); } @@ -201,4 +199,18 @@ public class GroupsController : Controller await _groupService.DeleteUserAsync(group, new Guid(orgUserId)); } + + private async Task> Get_FC(Guid orgId) + { + var authorized = + (await _authorizationService.AuthorizeAsync(User, null, GroupOperations.ReadAll(orgId))).Succeeded; + if (!authorized) + { + throw new NotFoundException(); + } + + var groups = await _groupRepository.GetManyWithCollectionsByOrganizationIdAsync(orgId); + var responses = groups.Select(g => new GroupDetailsResponseModel(g.Item1, g.Item2)); + return new ListResponseModel(responses); + } } diff --git a/src/Api/Utilities/ServiceCollectionExtensions.cs b/src/Api/Utilities/ServiceCollectionExtensions.cs index 2cf7ca3a2e..7afa9315d9 100644 --- a/src/Api/Utilities/ServiceCollectionExtensions.cs +++ b/src/Api/Utilities/ServiceCollectionExtensions.cs @@ -1,4 +1,5 @@ using Bit.Api.Vault.AuthorizationHandlers.Collections; +using Bit.Api.Vault.AuthorizationHandlers.Groups; using Bit.Api.Vault.AuthorizationHandlers.OrganizationUsers; using Bit.Core.IdentityServer; using Bit.Core.Settings; @@ -122,6 +123,8 @@ public static class ServiceCollectionExtensions public static void AddAuthorizationHandlers(this IServiceCollection services) { services.AddScoped(); + services.AddScoped(); + services.AddScoped(); services.AddScoped(); } } diff --git a/src/Api/Vault/AuthorizationHandlers/Groups/GroupAuthorizationHandler.cs b/src/Api/Vault/AuthorizationHandlers/Groups/GroupAuthorizationHandler.cs new file mode 100644 index 0000000000..0b1808791e --- /dev/null +++ b/src/Api/Vault/AuthorizationHandlers/Groups/GroupAuthorizationHandler.cs @@ -0,0 +1,81 @@ +using Bit.Core; +using Bit.Core.Context; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.Services; +using Microsoft.AspNetCore.Authorization; + +namespace Bit.Api.Vault.AuthorizationHandlers.Groups; + +/// +/// Handles authorization logic for Group operations. +/// This uses new logic implemented in the Flexible Collections initiative. +/// +public class GroupAuthorizationHandler : AuthorizationHandler +{ + private readonly ICurrentContext _currentContext; + private readonly IFeatureService _featureService; + + private bool FlexibleCollectionsIsEnabled => _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext); + + public GroupAuthorizationHandler( + ICurrentContext currentContext, + IFeatureService featureService) + { + _currentContext = currentContext; + _featureService = featureService; + } + + protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, + GroupOperationRequirement requirement) + { + if (!FlexibleCollectionsIsEnabled) + { + // Flexible collections is OFF, should not be using this handler + throw new FeatureUnavailableException("Flexible collections is OFF when it should be ON."); + } + + if (!_currentContext.UserId.HasValue) + { + context.Fail(); + return; + } + + var targetOrganizationId = requirement.OrganizationId; + if (targetOrganizationId == default) + { + context.Fail(); + return; + } + + // 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.Name == nameof(GroupOperations.ReadAll): + await CanReadAllAsync(context, requirement, org); + break; + } + } + + private async Task CanReadAllAsync(AuthorizationHandlerContext context, GroupOperationRequirement requirement, + CurrentContextOrganization org) + { + 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); + } + } +} diff --git a/src/Api/Vault/AuthorizationHandlers/Groups/GroupOperations.cs b/src/Api/Vault/AuthorizationHandlers/Groups/GroupOperations.cs new file mode 100644 index 0000000000..7735bd52a1 --- /dev/null +++ b/src/Api/Vault/AuthorizationHandlers/Groups/GroupOperations.cs @@ -0,0 +1,22 @@ +using Microsoft.AspNetCore.Authorization.Infrastructure; + +namespace Bit.Api.Vault.AuthorizationHandlers.Groups; + +public class GroupOperationRequirement : OperationAuthorizationRequirement +{ + public Guid OrganizationId { get; init; } + + public GroupOperationRequirement(string name, Guid organizationId) + { + Name = name; + OrganizationId = organizationId; + } +} + +public static class GroupOperations +{ + public static GroupOperationRequirement ReadAll(Guid organizationId) + { + return new GroupOperationRequirement(nameof(ReadAll), organizationId); + } +}