From 5c56b012c9843dddc9e093458aadbff5df6d91a7 Mon Sep 17 00:00:00 2001
From: Rui Tome <rtome@bitwarden.com>
Date: Wed, 25 Oct 2023 16:20:13 +0100
Subject: [PATCH] [AC-1139] Fixed Provider AuthorizationHandler logic for
 Groups and OrganizationUsers

---
 .../Groups/GroupAuthorizationHandler.cs       | 38 +++++++++++-------
 .../OrganizationUserAuthorizationHandler.cs   | 40 +++++++++++++------
 2 files changed, 51 insertions(+), 27 deletions(-)

diff --git a/src/Api/Vault/AuthorizationHandlers/Groups/GroupAuthorizationHandler.cs b/src/Api/Vault/AuthorizationHandlers/Groups/GroupAuthorizationHandler.cs
index 0b1808791e..7f7805ef17 100644
--- a/src/Api/Vault/AuthorizationHandlers/Groups/GroupAuthorizationHandler.cs
+++ b/src/Api/Vault/AuthorizationHandlers/Groups/GroupAuthorizationHandler.cs
@@ -48,13 +48,7 @@ public class GroupAuthorizationHandler : AuthorizationHandler<GroupOperationRequ
             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)
         {
@@ -67,15 +61,31 @@ public class GroupAuthorizationHandler : AuthorizationHandler<GroupOperationRequ
     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))
+        if (org != null)
         {
-            context.Succeed(requirement);
+            // Acting user is a member of the target organization, check permissions
+            if (org.Type is OrganizationUserType.Owner or OrganizationUserType.Admin ||
+                org.Permissions.ManageGroups ||
+                org.Permissions.ManageUsers ||
+                org.Permissions.EditAnyCollection ||
+                org.Permissions.DeleteAnyCollection ||
+                org.Permissions.AccessImportExport)
+            {
+                context.Succeed(requirement);
+                return;
+            }
         }
+        else
+        {
+            // 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();
     }
 }
diff --git a/src/Api/Vault/AuthorizationHandlers/OrganizationUsers/OrganizationUserAuthorizationHandler.cs b/src/Api/Vault/AuthorizationHandlers/OrganizationUsers/OrganizationUserAuthorizationHandler.cs
index 7ccee5aa58..2c74de5d47 100644
--- a/src/Api/Vault/AuthorizationHandlers/OrganizationUsers/OrganizationUserAuthorizationHandler.cs
+++ b/src/Api/Vault/AuthorizationHandlers/OrganizationUsers/OrganizationUserAuthorizationHandler.cs
@@ -16,6 +16,8 @@ public class OrganizationUserAuthorizationHandler : AuthorizationHandler<Organiz
     private readonly ICurrentContext _currentContext;
     private readonly IFeatureService _featureService;
 
+    private bool FlexibleCollectionsIsEnabled => _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext);
+
     public OrganizationUserAuthorizationHandler(
         ICurrentContext currentContext,
         IFeatureService featureService)
@@ -27,7 +29,7 @@ public class OrganizationUserAuthorizationHandler : AuthorizationHandler<Organiz
     protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context,
         OrganizationUserOperationRequirement requirement)
     {
-        if (!_featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext))
+        if (!FlexibleCollectionsIsEnabled)
         {
             // Flexible collections is OFF, should not be using this handler
             throw new FeatureUnavailableException("Flexible collections is OFF when it should be ON.");
@@ -40,15 +42,14 @@ public class OrganizationUserAuthorizationHandler : AuthorizationHandler<Organiz
         }
 
         var targetOrganizationId = requirement.OrganizationId;
-
-        // Acting user is not a member of the target organization, fail
-        var org = _currentContext.GetOrganization(targetOrganizationId);
-        if (org == null)
+        if (targetOrganizationId == default)
         {
             context.Fail();
             return;
         }
 
+        var org = _currentContext.GetOrganization(targetOrganizationId);
+
         switch (requirement)
         {
             case not null when requirement.Name == nameof(OrganizationUserOperations.ReadAll):
@@ -60,17 +61,30 @@ public class OrganizationUserAuthorizationHandler : AuthorizationHandler<Organiz
     private async Task CanReadAllAsync(AuthorizationHandlerContext context, OrganizationUserOperationRequirement requirement,
         CurrentContextOrganization org)
     {
-        if (org.Type is OrganizationUserType.Owner or OrganizationUserType.Admin ||
-            org.Permissions.ManageGroups ||
-            org.Permissions.ManageUsers ||
-            org.Permissions.EditAnyCollection ||
-            org.Permissions.DeleteAnyCollection ||
-            await _currentContext.ProviderUserForOrgAsync(org.Id))
+        if (org != null)
         {
-            context.Succeed(requirement);
-            return;
+            // Acting user is a member of the target organization, check permissions
+            if (org.Type is OrganizationUserType.Owner or OrganizationUserType.Admin ||
+                org.Permissions.ManageGroups ||
+                org.Permissions.ManageUsers ||
+                org.Permissions.EditAnyCollection ||
+                org.Permissions.DeleteAnyCollection)
+            {
+                context.Succeed(requirement);
+                return;
+            }
+        }
+        else
+        {
+            // 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();
     }
 }