From 22a90dee77ef30cebac0463b4287eaf91188bc12 Mon Sep 17 00:00:00 2001
From: Rui Tome <rtome@bitwarden.com>
Date: Fri, 24 Nov 2023 13:17:18 +0000
Subject: [PATCH] [AC-1139] Changed GroupAuthorizationHandler and
 OrganizationUserAuthorizationHandler to fail if no OrganizationId is passed
 as a parameter

---
 .../Groups/GroupAuthorizationHandler.cs       |  1 +
 .../OrganizationUserAuthorizationHandler.cs   |  1 +
 .../GroupAuthorizationHandlerTests.cs         | 45 ++++++++++---------
 ...ganizationUserAuthorizationHandlerTests.cs | 45 +++++++++----------
 4 files changed, 47 insertions(+), 45 deletions(-)

diff --git a/src/Api/Vault/AuthorizationHandlers/Groups/GroupAuthorizationHandler.cs b/src/Api/Vault/AuthorizationHandlers/Groups/GroupAuthorizationHandler.cs
index a80b3db928..8c0fc24395 100644
--- a/src/Api/Vault/AuthorizationHandlers/Groups/GroupAuthorizationHandler.cs
+++ b/src/Api/Vault/AuthorizationHandlers/Groups/GroupAuthorizationHandler.cs
@@ -45,6 +45,7 @@ public class GroupAuthorizationHandler : AuthorizationHandler<GroupOperationRequ
 
         if (requirement.OrganizationId == default)
         {
+            context.Fail();
             return;
         }
 
diff --git a/src/Api/Vault/AuthorizationHandlers/OrganizationUsers/OrganizationUserAuthorizationHandler.cs b/src/Api/Vault/AuthorizationHandlers/OrganizationUsers/OrganizationUserAuthorizationHandler.cs
index e7c4f64927..6bfc38f8a4 100644
--- a/src/Api/Vault/AuthorizationHandlers/OrganizationUsers/OrganizationUserAuthorizationHandler.cs
+++ b/src/Api/Vault/AuthorizationHandlers/OrganizationUsers/OrganizationUserAuthorizationHandler.cs
@@ -44,6 +44,7 @@ public class OrganizationUserAuthorizationHandler : AuthorizationHandler<Organiz
 
         if (requirement.OrganizationId == default)
         {
+            context.Fail();
             return;
         }
 
diff --git a/test/Api.Test/Vault/AuthorizationHandlers/GroupAuthorizationHandlerTests.cs b/test/Api.Test/Vault/AuthorizationHandlers/GroupAuthorizationHandlerTests.cs
index 36e4ff5fae..74711c80b8 100644
--- a/test/Api.Test/Vault/AuthorizationHandlers/GroupAuthorizationHandlerTests.cs
+++ b/test/Api.Test/Vault/AuthorizationHandlers/GroupAuthorizationHandlerTests.cs
@@ -108,7 +108,7 @@ public class GroupAuthorizationHandlerTests
     [Theory]
     [BitAutoData(OrganizationUserType.User)]
     [BitAutoData(OrganizationUserType.Custom)]
-    public async Task CanReadAllAsync_WhenMissingAccess_Failure(
+    public async Task CanReadAllAsync_WhenMissingPermissions_NoSuccess(
         OrganizationUserType userType,
         SutProvider<GroupAuthorizationHandler> sutProvider,
         CurrentContextOrganization organization)
@@ -140,25 +140,7 @@ public class GroupAuthorizationHandlerTests
     }
 
     [Theory, BitAutoData]
-    public async Task HandleRequirementAsync_MissingUserId_Failure(
-        Guid organizationId,
-        SutProvider<GroupAuthorizationHandler> sutProvider)
-    {
-        var context = new AuthorizationHandlerContext(
-            new[] { GroupOperations.ReadAll(organizationId) },
-            new ClaimsPrincipal(),
-            null
-        );
-
-        // Simulate missing user id
-        sutProvider.GetDependency<ICurrentContext>().UserId.Returns((Guid?)null);
-
-        await sutProvider.Sut.HandleAsync(context);
-        Assert.False(context.HasSucceeded);
-    }
-
-    [Theory, BitAutoData]
-    public async Task HandleRequirementAsync_MissingOrg_Failure(
+    public async Task CanReadAllAsync_WhenMissingOrgAccess_NoSuccess(
         Guid userId,
         Guid organizationId,
         SutProvider<GroupAuthorizationHandler> sutProvider)
@@ -177,7 +159,26 @@ public class GroupAuthorizationHandlerTests
     }
 
     [Theory, BitAutoData]
-    public async Task HandleRequirementAsync_NoSpecifiedOrgId_NoSuccessOrFailure(
+    public async Task HandleRequirementAsync_MissingUserId_Failure(
+        Guid organizationId,
+        SutProvider<GroupAuthorizationHandler> sutProvider)
+    {
+        var context = new AuthorizationHandlerContext(
+            new[] { GroupOperations.ReadAll(organizationId) },
+            new ClaimsPrincipal(),
+            null
+        );
+
+        // Simulate missing user id
+        sutProvider.GetDependency<ICurrentContext>().UserId.Returns((Guid?)null);
+
+        await sutProvider.Sut.HandleAsync(context);
+        Assert.False(context.HasSucceeded);
+        Assert.True(context.HasFailed);
+    }
+
+    [Theory, BitAutoData]
+    public async Task HandleRequirementAsync_NoSpecifiedOrgId_Failure(
         SutProvider<GroupAuthorizationHandler> sutProvider)
     {
         var context = new AuthorizationHandlerContext(
@@ -191,6 +192,6 @@ public class GroupAuthorizationHandlerTests
         await sutProvider.Sut.HandleAsync(context);
 
         Assert.False(context.HasSucceeded);
-        Assert.False(context.HasFailed);
+        Assert.True(context.HasFailed);
     }
 }
diff --git a/test/Api.Test/Vault/AuthorizationHandlers/OrganizationUserAuthorizationHandlerTests.cs b/test/Api.Test/Vault/AuthorizationHandlers/OrganizationUserAuthorizationHandlerTests.cs
index 3fe15b24c1..1f6916faf3 100644
--- a/test/Api.Test/Vault/AuthorizationHandlers/OrganizationUserAuthorizationHandlerTests.cs
+++ b/test/Api.Test/Vault/AuthorizationHandlers/OrganizationUserAuthorizationHandlerTests.cs
@@ -108,7 +108,7 @@ public class OrganizationUserAuthorizationHandlerTests
     [Theory]
     [BitAutoData(OrganizationUserType.User)]
     [BitAutoData(OrganizationUserType.Custom)]
-    public async Task CanReadAllAsync_WhenMissingAccess_Failure(
+    public async Task CanReadAllAsync_WhenMissingPermissions_NoSuccess(
         OrganizationUserType userType,
         SutProvider<OrganizationUserAuthorizationHandler> sutProvider,
         CurrentContextOrganization organization)
@@ -139,25 +139,7 @@ public class OrganizationUserAuthorizationHandlerTests
     }
 
     [Theory, BitAutoData]
-    public async Task HandleRequirementAsync_MissingUserId_Failure(
-        Guid organizationId,
-        SutProvider<OrganizationUserAuthorizationHandler> sutProvider)
-    {
-        var context = new AuthorizationHandlerContext(
-            new[] { OrganizationUserOperations.ReadAll(organizationId) },
-            new ClaimsPrincipal(),
-            null
-        );
-
-        // Simulate missing user id
-        sutProvider.GetDependency<ICurrentContext>().UserId.Returns((Guid?)null);
-
-        await sutProvider.Sut.HandleAsync(context);
-        Assert.False(context.HasSucceeded);
-    }
-
-    [Theory, BitAutoData]
-    public async Task HandleRequirementAsync_MissingOrg_Failure(
+    public async Task HandleRequirementAsync_WhenMissingOrgAccess_NoSuccess(
         Guid userId,
         Guid organizationId,
         SutProvider<OrganizationUserAuthorizationHandler> sutProvider)
@@ -176,7 +158,25 @@ public class OrganizationUserAuthorizationHandlerTests
     }
 
     [Theory, BitAutoData]
-    public async Task HandleRequirementAsync_NoSpecifiedOrgId_NoSuccessOrFailure(
+    public async Task HandleRequirementAsync_MissingUserId_Failure(
+        Guid organizationId,
+        SutProvider<OrganizationUserAuthorizationHandler> sutProvider)
+    {
+        var context = new AuthorizationHandlerContext(
+            new[] { OrganizationUserOperations.ReadAll(organizationId) },
+            new ClaimsPrincipal(),
+            null
+        );
+
+        // Simulate missing user id
+        sutProvider.GetDependency<ICurrentContext>().UserId.Returns((Guid?)null);
+
+        await sutProvider.Sut.HandleAsync(context);
+        Assert.True(context.HasFailed);
+    }
+
+    [Theory, BitAutoData]
+    public async Task HandleRequirementAsync_NoSpecifiedOrgId_Failure(
         SutProvider<OrganizationUserAuthorizationHandler> sutProvider)
     {
         var context = new AuthorizationHandlerContext(
@@ -189,7 +189,6 @@ public class OrganizationUserAuthorizationHandlerTests
 
         await sutProvider.Sut.HandleAsync(context);
 
-        Assert.False(context.HasSucceeded);
-        Assert.False(context.HasFailed);
+        Assert.True(context.HasFailed);
     }
 }