[AC-1139] Modified authorization handlers to not fail in case the resource is null

src/Api/Vault/AuthorizationHandlers/Collections/BulkCollectionAuthorizationHandler.cs

@@ -41,8 +41,15 @@ public class BulkCollectionAuthorizationHandler : BulkAuthorizationHandler<Colle
             throw new FeatureUnavailableException("Flexible collections is OFF when it should be ON.");
         }
 
+        // Acting user is not authenticated, fail
+        if (!_currentContext.UserId.HasValue)
+        {
+            context.Fail();
+            return;
+        }
+
         // Establish pattern of authorization handler null checking passed resources
-        if (resources == null || !resources.Any() || !_currentContext.UserId.HasValue)
+        if (resources == null || !resources.Any())
         {
             return;
         }

src/Api/Vault/AuthorizationHandlers/Collections/CollectionAuthorizationHandler.cs

@@ -35,7 +35,14 @@ public class CollectionAuthorizationHandler : AuthorizationHandler<CollectionOpe
             throw new FeatureUnavailableException("Flexible collections is OFF when it should be ON.");
         }
 
-        if (!_currentContext.UserId.HasValue || requirement.OrganizationId == default)
+        // Acting user is not authenticated, fail
+        if (!_currentContext.UserId.HasValue)
+        {
+            context.Fail();
+            return;
+        }
+
+        if (requirement.OrganizationId == default)
         {
             return;
         }

src/Api/Vault/AuthorizationHandlers/Groups/GroupAuthorizationHandler.cs

@@ -35,20 +35,19 @@ public class GroupAuthorizationHandler : AuthorizationHandler<GroupOperationRequ
             throw new FeatureUnavailableException("Flexible collections is OFF when it should be ON.");
         }
 
+        // Acting user is not authenticated, fail
         if (!_currentContext.UserId.HasValue)
         {
             context.Fail();
             return;
         }
 
-        var targetOrganizationId = requirement.OrganizationId;
+        if (requirement.OrganizationId == default)
-        if (targetOrganizationId == default)
         {
-            context.Fail();
             return;
         }
 
-        var org = _currentContext.GetOrganization(targetOrganizationId);
+        var org = _currentContext.GetOrganization(requirement.OrganizationId);
 
         switch (requirement)
         {
@@ -72,7 +71,6 @@ public class GroupAuthorizationHandler : AuthorizationHandler<GroupOperationRequ
                 org.Permissions.AccessImportExport)
             {
                 context.Succeed(requirement);
-                return;
             }
         }
         else
@@ -81,11 +79,7 @@ public class GroupAuthorizationHandler : AuthorizationHandler<GroupOperationRequ
             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();
     }
 }

src/Api/Vault/AuthorizationHandlers/OrganizationUsers/OrganizationUserAuthorizationHandler.cs

@@ -41,14 +41,12 @@ public class OrganizationUserAuthorizationHandler : AuthorizationHandler<Organiz
             return;
         }
 
-        var targetOrganizationId = requirement.OrganizationId;
+        if (requirement.OrganizationId == default)
-        if (targetOrganizationId == default)
         {
-            context.Fail();
             return;
         }
 
-        var org = _currentContext.GetOrganization(targetOrganizationId);
+        var org = _currentContext.GetOrganization(requirement.OrganizationId);
 
         switch (requirement)
         {
@@ -71,7 +69,6 @@ public class OrganizationUserAuthorizationHandler : AuthorizationHandler<Organiz
                 org.Permissions.DeleteAnyCollection)
             {
                 context.Succeed(requirement);
-                return;
             }
         }
         else
@@ -80,11 +77,7 @@ public class OrganizationUserAuthorizationHandler : AuthorizationHandler<Organiz
             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();
     }
 }

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

@@ -750,6 +750,8 @@ public class BulkCollectionAuthorizationHandlerTests
             new List<Collection>()
         );
 
+        sutProvider.GetDependency<ICurrentContext>().UserId.Returns(new Guid());
+
         await sutProvider.Sut.HandleAsync(context);
 
         Assert.False(context.HasSucceeded);
@@ -757,7 +759,7 @@ public class BulkCollectionAuthorizationHandlerTests
     }
 
     [Theory, BitAutoData, CollectionCustomization]
-    public async Task HandleRequirementAsync_MissingUserId_NoSuccessOrFailure(
+    public async Task HandleRequirementAsync_MissingUserId_Failure(
         SutProvider<BulkCollectionAuthorizationHandler> sutProvider,
         ICollection<Collection> collections)
     {
@@ -771,8 +773,7 @@ public class BulkCollectionAuthorizationHandlerTests
         sutProvider.GetDependency<ICurrentContext>().UserId.Returns((Guid?)null);
 
         await sutProvider.Sut.HandleAsync(context);
-        Assert.False(context.HasSucceeded);
+        Assert.True(context.HasFailed);
-        Assert.False(context.HasFailed);
         sutProvider.GetDependency<ICollectionRepository>().DidNotReceiveWithAnyArgs();
     }
 

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

@@ -18,32 +18,15 @@ namespace Bit.Api.Test.Vault.AuthorizationHandlers;
 public class GroupAuthorizationHandlerTests
 {
     [Theory]
-    [BitAutoData(OrganizationUserType.Admin, false, false, false, false, false, true)]
+    [BitAutoData(OrganizationUserType.Admin)]
-    [BitAutoData(OrganizationUserType.Owner, false, false, false, false, false, true)]
+    [BitAutoData(OrganizationUserType.Owner)]
-    [BitAutoData(OrganizationUserType.User, false, false, false, false, false, false)]
+    public async Task CanReadAllAsync_WhenAdminOrOwner_Success(
-    [BitAutoData(OrganizationUserType.Custom, true, false, false, false, false, true)]
+        OrganizationUserType userType,
-    [BitAutoData(OrganizationUserType.Custom, false, true, false, false, false, true)]
-    [BitAutoData(OrganizationUserType.Custom, false, false, true, false, false, true)]
-    [BitAutoData(OrganizationUserType.Custom, false, false, false, true, false, true)]
-    [BitAutoData(OrganizationUserType.Custom, false, false, false, false, true, true)]
-    [BitAutoData(OrganizationUserType.Custom, false, false, false, false, false, false)]
-    public async Task CanReadAllAccessAsync_ReturnsExpectedResult(
-        OrganizationUserType userType, bool editAnyCollection, bool deleteAnyCollection,
-        bool manageGroups, bool manageUsers, bool accessImportExport, bool expectedSuccess,
         Guid userId, SutProvider<GroupAuthorizationHandler> sutProvider,
         CurrentContextOrganization organization)
     {
-        var permissions = new Permissions
-        {
-            EditAnyCollection = editAnyCollection,
-            DeleteAnyCollection = deleteAnyCollection,
-            ManageGroups = manageGroups,
-            ManageUsers = manageUsers,
-            AccessImportExport = accessImportExport
-        };
-
         organization.Type = userType;
-        organization.Permissions = permissions;
+        organization.Permissions = new Permissions();
 
         var context = new AuthorizationHandlerContext(
             new[] { GroupOperations.ReadAll(organization.Id) },
@@ -55,11 +38,11 @@ public class GroupAuthorizationHandlerTests
 
         await sutProvider.Sut.HandleAsync(context);
 
-        Assert.True(expectedSuccess ? context.HasSucceeded : context.HasFailed);
+        Assert.True(context.HasSucceeded);
     }
 
     [Theory, BitAutoData]
-    public async Task CanReadAllAccessAsync_WithProviderUser_Success(
+    public async Task CanReadAllAsync_WithProviderUser_Success(
         Guid userId,
         SutProvider<GroupAuthorizationHandler> sutProvider, CurrentContextOrganization organization)
     {
@@ -80,6 +63,75 @@ public class GroupAuthorizationHandlerTests
         Assert.True(context.HasSucceeded);
     }
 
+    [Theory]
+    [BitAutoData(true, false, false, false, false)]
+    [BitAutoData(false, true, false, false, false)]
+    [BitAutoData(false, false, true, false, false)]
+    [BitAutoData(false, false, false, true, false)]
+    [BitAutoData(false, false, false, false, true)]
+    public async Task CanReadAllAsync_WhenCustomUserWithRequiredPermissions_Success(
+        bool editAnyCollection, bool deleteAnyCollection, bool manageGroups, bool manageUsers, bool accessImportExport,
+        SutProvider<GroupAuthorizationHandler> sutProvider,
+        CurrentContextOrganization organization)
+    {
+        var actingUserId = Guid.NewGuid();
+
+        organization.Type = OrganizationUserType.Custom;
+        organization.Permissions = new Permissions
+        {
+            EditAnyCollection = editAnyCollection,
+            DeleteAnyCollection = deleteAnyCollection,
+            ManageGroups = manageGroups,
+            ManageUsers = manageUsers,
+            AccessImportExport = accessImportExport
+        };
+
+        var context = new AuthorizationHandlerContext(
+            new[] { GroupOperations.ReadAll(organization.Id) },
+            new ClaimsPrincipal(),
+            null);
+
+        sutProvider.GetDependency<ICurrentContext>().UserId.Returns(actingUserId);
+        sutProvider.GetDependency<ICurrentContext>().GetOrganization(organization.Id).Returns(organization);
+
+        await sutProvider.Sut.HandleAsync(context);
+
+        Assert.True(context.HasSucceeded);
+    }
+
+    [Theory]
+    [BitAutoData(OrganizationUserType.User)]
+    [BitAutoData(OrganizationUserType.Custom)]
+    public async Task CanReadAllAsync_WhenMissingAccess_Failure(
+        OrganizationUserType userType,
+        SutProvider<GroupAuthorizationHandler> sutProvider,
+        CurrentContextOrganization organization)
+    {
+        var actingUserId = Guid.NewGuid();
+
+        organization.Type = userType;
+        organization.Permissions = new Permissions
+        {
+            EditAnyCollection = false,
+            DeleteAnyCollection = false,
+            ManageGroups = false,
+            ManageUsers = false,
+            AccessImportExport = false
+        };
+
+        var context = new AuthorizationHandlerContext(
+            new[] { GroupOperations.ReadAll(organization.Id) },
+            new ClaimsPrincipal(),
+            null);
+
+        sutProvider.GetDependency<ICurrentContext>().UserId.Returns(actingUserId);
+        sutProvider.GetDependency<ICurrentContext>().GetOrganization(organization.Id).Returns(organization);
+
+        await sutProvider.Sut.HandleAsync(context);
+
+        Assert.False(context.HasSucceeded);
+    }
+
     [Theory, BitAutoData]
     public async Task HandleRequirementAsync_MissingUserId_Failure(
         Guid organizationId,
@@ -95,7 +147,7 @@ public class GroupAuthorizationHandlerTests
         sutProvider.GetDependency<ICurrentContext>().UserId.Returns((Guid?)null);
 
         await sutProvider.Sut.HandleAsync(context);
-        Assert.True(context.HasFailed);
+        Assert.False(context.HasSucceeded);
     }
 
     [Theory, BitAutoData]
@@ -114,6 +166,24 @@ public class GroupAuthorizationHandlerTests
         sutProvider.GetDependency<ICurrentContext>().GetOrganization(Arg.Any<Guid>()).Returns((CurrentContextOrganization)null);
 
         await sutProvider.Sut.HandleAsync(context);
-        Assert.True(context.HasFailed);
+        Assert.False(context.HasSucceeded);
+    }
+
+    [Theory, BitAutoData]
+    public async Task HandleRequirementAsync_NoSpecifiedOrgId_NoSuccessOrFailure(
+        SutProvider<GroupAuthorizationHandler> sutProvider)
+    {
+        var context = new AuthorizationHandlerContext(
+            new[] { GroupOperations.ReadAll(default) },
+            new ClaimsPrincipal(),
+            null
+        );
+
+        sutProvider.GetDependency<ICurrentContext>().UserId.Returns(new Guid());
+
+        await sutProvider.Sut.HandleAsync(context);
+
+        Assert.False(context.HasSucceeded);
+        Assert.False(context.HasFailed);
     }
 }

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

@@ -18,30 +18,15 @@ namespace Bit.Api.Test.Vault.AuthorizationHandlers;
 public class OrganizationUserAuthorizationHandlerTests
 {
     [Theory]
-    [BitAutoData(OrganizationUserType.Admin, false, false, false, false, true)]
+    [BitAutoData(OrganizationUserType.Admin)]
-    [BitAutoData(OrganizationUserType.Owner, false, false, false, false, true)]
+    [BitAutoData(OrganizationUserType.Owner)]
-    [BitAutoData(OrganizationUserType.User, false, false, false, false, false)]
+    public async Task CanReadAllAsync_WhenAdminOrOwner_Success(
-    [BitAutoData(OrganizationUserType.Custom, true, false, false, false, true)]
+        OrganizationUserType userType,
-    [BitAutoData(OrganizationUserType.Custom, false, true, false, false, true)]
-    [BitAutoData(OrganizationUserType.Custom, false, false, true, false, true)]
-    [BitAutoData(OrganizationUserType.Custom, false, false, false, true, true)]
-    [BitAutoData(OrganizationUserType.Custom, false, false, false, false, false)]
-    public async Task CanReadAllAccessAsync_ReturnsExpectedResult(
-        OrganizationUserType userType, bool editAnyCollection, bool deleteAnyCollection,
-        bool manageGroups, bool manageUsers, bool expectedSuccess,
         Guid userId, SutProvider<OrganizationUserAuthorizationHandler> sutProvider,
         CurrentContextOrganization organization)
     {
-        var permissions = new Permissions
-        {
-            EditAnyCollection = editAnyCollection,
-            DeleteAnyCollection = deleteAnyCollection,
-            ManageGroups = manageGroups,
-            ManageUsers = manageUsers
-        };
-
         organization.Type = userType;
-        organization.Permissions = permissions;
+        organization.Permissions = new Permissions();
 
         var context = new AuthorizationHandlerContext(
             new[] { OrganizationUserOperations.ReadAll(organization.Id) },
@@ -53,11 +38,11 @@ public class OrganizationUserAuthorizationHandlerTests
 
         await sutProvider.Sut.HandleAsync(context);
 
-        Assert.True(expectedSuccess ? context.HasSucceeded : context.HasFailed);
+        Assert.True(context.HasSucceeded);
     }
 
     [Theory, BitAutoData]
-    public async Task CanReadAllAccessAsync_WithProviderUser_Success(
+    public async Task CanReadAllAsync_WithProviderUser_Success(
         Guid userId,
         SutProvider<OrganizationUserAuthorizationHandler> sutProvider, CurrentContextOrganization organization)
     {
@@ -78,6 +63,73 @@ public class OrganizationUserAuthorizationHandlerTests
         Assert.True(context.HasSucceeded);
     }
 
+    [Theory]
+    [BitAutoData(true, false, false, false)]
+    [BitAutoData(false, true, false, false)]
+    [BitAutoData(false, false, true, false)]
+    [BitAutoData(false, false, false, true)]
+    public async Task CanReadAllAsync_WhenCustomUserWithRequiredPermissions_Success(
+        bool editAnyCollection, bool deleteAnyCollection, bool manageGroups, bool manageUsers,
+        SutProvider<OrganizationUserAuthorizationHandler> sutProvider,
+        CurrentContextOrganization organization)
+    {
+        var actingUserId = Guid.NewGuid();
+
+        organization.Type = OrganizationUserType.Custom;
+        organization.Permissions = new Permissions
+        {
+            EditAnyCollection = editAnyCollection,
+            DeleteAnyCollection = deleteAnyCollection,
+            ManageGroups = manageGroups,
+            ManageUsers = manageUsers
+        };
+
+        var context = new AuthorizationHandlerContext(
+            new[] { OrganizationUserOperations.ReadAll(organization.Id) },
+            new ClaimsPrincipal(),
+            null);
+
+        sutProvider.GetDependency<ICurrentContext>().UserId.Returns(actingUserId);
+        sutProvider.GetDependency<ICurrentContext>().GetOrganization(organization.Id).Returns(organization);
+
+        await sutProvider.Sut.HandleAsync(context);
+
+        Assert.True(context.HasSucceeded);
+    }
+
+    [Theory]
+    [BitAutoData(OrganizationUserType.User)]
+    [BitAutoData(OrganizationUserType.Custom)]
+    public async Task CanReadAllAsync_WhenMissingAccess_Failure(
+        OrganizationUserType userType,
+        SutProvider<OrganizationUserAuthorizationHandler> sutProvider,
+        CurrentContextOrganization organization)
+    {
+        var actingUserId = Guid.NewGuid();
+
+        organization.Type = userType;
+        organization.Permissions = new Permissions
+        {
+            EditAnyCollection = false,
+            DeleteAnyCollection = false,
+            ManageGroups = false,
+            ManageUsers = false,
+            AccessImportExport = false
+        };
+
+        var context = new AuthorizationHandlerContext(
+            new[] { OrganizationUserOperations.ReadAll(organization.Id) },
+            new ClaimsPrincipal(),
+            null);
+
+        sutProvider.GetDependency<ICurrentContext>().UserId.Returns(actingUserId);
+        sutProvider.GetDependency<ICurrentContext>().GetOrganization(organization.Id).Returns(organization);
+
+        await sutProvider.Sut.HandleAsync(context);
+
+        Assert.False(context.HasSucceeded);
+    }
+
     [Theory, BitAutoData]
     public async Task HandleRequirementAsync_MissingUserId_Failure(
         Guid organizationId,
@@ -93,7 +145,7 @@ public class OrganizationUserAuthorizationHandlerTests
         sutProvider.GetDependency<ICurrentContext>().UserId.Returns((Guid?)null);
 
         await sutProvider.Sut.HandleAsync(context);
-        Assert.True(context.HasFailed);
+        Assert.False(context.HasSucceeded);
     }
 
     [Theory, BitAutoData]
@@ -112,6 +164,24 @@ public class OrganizationUserAuthorizationHandlerTests
         sutProvider.GetDependency<ICurrentContext>().GetOrganization(Arg.Any<Guid>()).Returns((CurrentContextOrganization)null);
 
         await sutProvider.Sut.HandleAsync(context);
-        Assert.True(context.HasFailed);
+        Assert.False(context.HasSucceeded);
+    }
+
+    [Theory, BitAutoData]
+    public async Task HandleRequirementAsync_NoSpecifiedOrgId_NoSuccessOrFailure(
+        SutProvider<OrganizationUserAuthorizationHandler> sutProvider)
+    {
+        var context = new AuthorizationHandlerContext(
+            new[] { OrganizationUserOperations.ReadAll(default) },
+            new ClaimsPrincipal(),
+            null
+        );
+
+        sutProvider.GetDependency<ICurrentContext>().UserId.Returns(new Guid());
+
+        await sutProvider.Sut.HandleAsync(context);
+
+        Assert.False(context.HasSucceeded);
+        Assert.False(context.HasFailed);
     }
 }