[AC-1139] Added permission checks for GroupsController.Get if FC feature flag is enabled

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<ListResponseModel<GroupDetailsResponseModel>> Get(Guid orgId)
     {
-        ICollection<Tuple<Group, ICollection<CollectionAccessSelection>>> 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<GroupDetailsResponseModel>(responses);
     }
@@ -201,4 +199,18 @@ public class GroupsController : Controller
 
         await _groupService.DeleteUserAsync(group, new Guid(orgUserId));
     }
+
+    private async Task<ListResponseModel<GroupDetailsResponseModel>> 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<GroupDetailsResponseModel>(responses);
+    }
 }

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<IAuthorizationHandler, CollectionAuthorizationHandler>();
+        services.AddScoped<IAuthorizationHandler, BulkCollectionAuthorizationHandler>();
+        services.AddScoped<IAuthorizationHandler, GroupAuthorizationHandler>();
         services.AddScoped<IAuthorizationHandler, OrganizationUserAuthorizationHandler>();
     }
 }

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;
+
+/// <summary>
+/// Handles authorization logic for Group operations.
+/// This uses new logic implemented in the Flexible Collections initiative.
+/// </summary>
+public class GroupAuthorizationHandler : AuthorizationHandler<GroupOperationRequirement>
+{
+    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);
+        }
+    }
+}

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);
+    }
+}