[PM-12474] Move to authorization to attibutes/handlers/requirements (#6001)

* Created ReadAllOrganizationUsersBasicInformationRequirement for use with Authorize attribute. * Removed unused req and Handler and tests. Moved to new auth attribute * Moved tests to integration tests with new response. * Removed tests that were migrated to integration tests. * Made string params Guids instead of parsing them manually in methods. * Admin and Owner added to requirement. * Added XML docs for basic get endpoint. Removed unused. Added another auth check. Inverted if check. * Removed unused endpoint * Added tests for requirement * Added checks for both User and Custom * Added org id check to validate the user being requested belongs to the org in the route. * typo
2025-12-06 00:03:34 +00:00 · 2025-07-15 07:52:47 -05:00
parent 93a00373d2
commit c4965350d1
8 changed files with 253 additions and 307 deletions
--- a/test/Api.IntegrationTest/AdminConsole/Controllers/OrganizationUserControllerTests.cs
+++ b/test/Api.IntegrationTest/AdminConsole/Controllers/OrganizationUserControllerTests.cs
@@ -0,0 +1,101 @@
+using System.Net;
+using Bit.Api.AdminConsole.Models.Request.Organizations;
+using Bit.Api.IntegrationTest.Factories;
+using Bit.Api.IntegrationTest.Helpers;
+using Bit.Core.AdminConsole.Entities;
+using Bit.Core.Billing.Enums;
+using Bit.Core.Enums;
+using Bit.Core.Models.Data;
+using Xunit;
+
+namespace Bit.Api.IntegrationTest.AdminConsole.Controllers;
+
+public class OrganizationUserControllerTests : IClassFixture<ApiApplicationFactory>, IAsyncLifetime
+{
+    public OrganizationUserControllerTests(ApiApplicationFactory apiFactory)
+    {
+        _factory = apiFactory;
+        _client = _factory.CreateClient();
+        _loginHelper = new LoginHelper(_factory, _client);
+    }
+
+    private readonly HttpClient _client;
+    private readonly ApiApplicationFactory _factory;
+    private readonly LoginHelper _loginHelper;
+
+    private Organization _organization = null!;
+    private string _ownerEmail = null!;
+
+    [Theory]
+    [InlineData(OrganizationUserType.User)]
+    [InlineData(OrganizationUserType.Custom)]
+    public async Task BulkDeleteAccount_WhenUserCannotManageUsers_ReturnsForbiddenResponse(OrganizationUserType organizationUserType)
+    {
+        var (userEmail, _) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync(_factory,
+            _organization.Id, organizationUserType, new Permissions { ManageUsers = false });
+
+        await _loginHelper.LoginAsync(userEmail);
+
+        var request = new OrganizationUserBulkRequestModel
+        {
+            Ids = new List<Guid> { Guid.NewGuid() }
+        };
+
+        var httpResponse = await _client.PostAsJsonAsync($"organizations/{_organization.Id}/users/remove", request);
+
+        Assert.Equal(HttpStatusCode.Forbidden, httpResponse.StatusCode);
+    }
+
+    [Theory]
+    [InlineData(OrganizationUserType.User)]
+    [InlineData(OrganizationUserType.Custom)]
+    public async Task DeleteAccount_WhenUserCannotManageUsers_ReturnsForbiddenResponse(OrganizationUserType organizationUserType)
+    {
+        var (userEmail, _) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync(_factory,
+            _organization.Id, organizationUserType, new Permissions { ManageUsers = false });
+
+        await _loginHelper.LoginAsync(userEmail);
+
+        var userToRemove = Guid.NewGuid();
+
+        var httpResponse = await _client.DeleteAsync($"organizations/{_organization.Id}/users/{userToRemove}");
+
+        Assert.Equal(HttpStatusCode.Forbidden, httpResponse.StatusCode);
+    }
+
+    [Theory]
+    [InlineData(OrganizationUserType.User)]
+    [InlineData(OrganizationUserType.Custom)]
+    public async Task GetAccountRecoveryDetails_WithoutManageResetPasswordPermission_ReturnsForbiddenResponse(OrganizationUserType organizationUserType)
+    {
+        var (userEmail, _) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync(_factory,
+            _organization.Id, organizationUserType, new Permissions { ManageUsers = false });
+
+        await _loginHelper.LoginAsync(userEmail);
+
+        var request = new OrganizationUserBulkRequestModel
+        {
+            Ids = []
+        };
+
+        var httpResponse =
+            await _client.PostAsJsonAsync($"organizations/{_organization.Id}/users/account-recovery-details", request);
+
+        Assert.Equal(HttpStatusCode.Forbidden, httpResponse.StatusCode);
+    }
+
+    public async Task InitializeAsync()
+    {
+        _ownerEmail = $"org-user-integration-test-{Guid.NewGuid()}@bitwarden.com";
+        await _factory.LoginWithNewAccount(_ownerEmail);
+
+        (_organization, _) = await OrganizationTestHelpers.SignUpAsync(_factory, plan: PlanType.EnterpriseAnnually2023,
+            ownerEmail: _ownerEmail, passwordManagerSeats: 5, paymentMethod: PaymentMethodType.Card);
+    }
+
+    public Task DisposeAsync()
+    {
+        _client.Dispose();
+        return Task.CompletedTask;
+    }
+}
--- a/test/Api.Test/AdminConsole/Authorization/Requirements/ManageGroupsOrUsersRequirementTests.cs
+++ b/test/Api.Test/AdminConsole/Authorization/Requirements/ManageGroupsOrUsersRequirementTests.cs
@@ -0,0 +1,84 @@
+using Bit.Api.AdminConsole.Authorization.Requirements;
+using Bit.Core.Context;
+using Bit.Core.Enums;
+using Bit.Core.Models.Data;
+using Bit.Core.Test.AdminConsole.AutoFixture;
+using Bit.Test.Common.AutoFixture;
+using Bit.Test.Common.AutoFixture.Attributes;
+using Xunit;
+
+namespace Bit.Api.Test.AdminConsole.Authorization.Requirements;
+
+[SutProviderCustomize]
+public class ManageGroupsOrUsersRequirementTests
+{
+    [Theory]
+    [CurrentContextOrganizationCustomize]
+    [BitAutoData(OrganizationUserType.Admin)]
+    [BitAutoData(OrganizationUserType.Owner)]
+    public async Task AuthorizeAsync_WhenUserTypeCanManageUsers_ThenRequestShouldBeAuthorized(
+        OrganizationUserType type,
+        CurrentContextOrganization organization,
+        SutProvider<ManageGroupsOrUsersRequirement> sutProvider)
+    {
+        organization.Type = type;
+
+        var actual = await sutProvider.Sut.AuthorizeAsync(organization, () => Task.FromResult(false));
+
+        Assert.True(actual);
+    }
+
+    [Theory]
+    [CurrentContextOrganizationCustomize]
+    [BitAutoData(OrganizationUserType.Custom, true, false)]
+    [BitAutoData(OrganizationUserType.Custom, false, true)]
+    public async Task AuthorizeAsync_WhenCustomUserThatCanManageUsersOrGroups_ThenRequestShouldBeAuthorized(
+        OrganizationUserType type,
+        bool canManageUsers,
+        bool canManageGroups,
+        CurrentContextOrganization organization,
+        SutProvider<ManageGroupsOrUsersRequirement> sutProvider)
+    {
+        organization.Type = type;
+        organization.Permissions = new Permissions { ManageUsers = canManageUsers, ManageGroups = canManageGroups };
+
+        var actual = await sutProvider.Sut.AuthorizeAsync(organization, () => Task.FromResult(false));
+
+        Assert.True(actual);
+    }
+
+    [Theory]
+    [CurrentContextOrganizationCustomize]
+    [BitAutoData]
+    public async Task AuthorizeAsync_WhenProviderUserForAnOrganization_ThenRequestShouldBeAuthorized(
+        CurrentContextOrganization organization,
+        SutProvider<ManageGroupsOrUsersRequirement> sutProvider)
+    {
+        var actual = await sutProvider.Sut.AuthorizeAsync(organization, IsProviderUserForOrg);
+
+        Assert.True(actual);
+        return;
+
+        Task<bool> IsProviderUserForOrg() => Task.FromResult(true);
+    }
+
+    [Theory]
+    [CurrentContextOrganizationCustomize]
+    [BitAutoData(OrganizationUserType.User)]
+    [BitAutoData(OrganizationUserType.Custom)]
+    public async Task AuthorizeAsync_WhenUserCannotManageUsersOrGroupsAndIsNotAProviderUser_ThenRequestShouldBeDenied(
+        OrganizationUserType type,
+        CurrentContextOrganization organization,
+        SutProvider<ManageGroupsOrUsersRequirement> sutProvider)
+    {
+        organization.Type = type;
+        organization.Permissions = new Permissions { ManageUsers = false, ManageGroups = false }; // When Type is User, the canManage permissions don't matter
+
+        var actual = await sutProvider.Sut.AuthorizeAsync(organization, IsNotProviderUserForOrg);
+
+        Assert.False(actual);
+        return;
+
+        Task<bool> IsNotProviderUserForOrg() => Task.FromResult(false);
+    }
+}
--- a/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs
+++ b/test/Api.Test/AdminConsole/Controllers/OrganizationUsersControllerTests.cs
@@ -257,7 +257,7 @@ public class OrganizationUsersControllerTests
            .GetUsersOrganizationClaimedStatusAsync(organizationUser.OrganizationId, Arg.Is<IEnumerable<Guid>>(ids => ids.Contains(organizationUser.Id)))
            .Returns(new Dictionary<Guid, bool> { { organizationUser.Id, true } });

-        var response = await sutProvider.Sut.Get(organizationUser.Id, false);
+        var response = await sutProvider.Sut.Get(organizationUser.OrganizationId, organizationUser.Id, false);

        Assert.Equal(organizationUser.Id, response.Id);
        Assert.True(response.ManagedByOrganization);
@@ -303,18 +303,6 @@ public class OrganizationUsersControllerTests
                ou.EncryptedPrivateKey == r.EncryptedPrivateKey)));
    }

-    [Theory]
-    [BitAutoData]
-    public async Task GetAccountRecoveryDetails_WithoutManageResetPasswordPermission_Throws(
-        Guid organizationId,
-        OrganizationUserBulkRequestModel bulkRequestModel,
-        SutProvider<OrganizationUsersController> sutProvider)
-    {
-        sutProvider.GetDependency<ICurrentContext>().ManageResetPassword(organizationId).Returns(false);
-
-        await Assert.ThrowsAsync<NotFoundException>(async () => await sutProvider.Sut.GetAccountRecoveryDetails(organizationId, bulkRequestModel));
-    }
-
    [Theory]
    [BitAutoData]
    public async Task DeleteAccount_WhenUserCanManageUsers_Success(
@@ -330,17 +318,6 @@ public class OrganizationUsersControllerTests
            .DeleteUserAsync(orgId, id, currentUser.Id);
    }

-    [Theory]
-    [BitAutoData]
-    public async Task DeleteAccount_WhenUserCannotManageUsers_ThrowsNotFoundException(
-        Guid orgId, Guid id, SutProvider<OrganizationUsersController> sutProvider)
-    {
-        sutProvider.GetDependency<ICurrentContext>().ManageUsers(orgId).Returns(false);
-
-        await Assert.ThrowsAsync<NotFoundException>(() =>
-            sutProvider.Sut.DeleteAccount(orgId, id));
-    }
-
    [Theory]
    [BitAutoData]
    public async Task DeleteAccount_WhenCurrentUserNotFound_ThrowsUnauthorizedAccessException(
@@ -374,17 +351,6 @@ public class OrganizationUsersControllerTests
            .DeleteManyUsersAsync(orgId, model.Ids, currentUser.Id);
    }

-    [Theory]
-    [BitAutoData]
-    public async Task BulkDeleteAccount_WhenUserCannotManageUsers_ThrowsNotFoundException(
-        Guid orgId, OrganizationUserBulkRequestModel model, SutProvider<OrganizationUsersController> sutProvider)
-    {
-        sutProvider.GetDependency<ICurrentContext>().ManageUsers(orgId).Returns(false);
-
-        await Assert.ThrowsAsync<NotFoundException>(() =>
-            sutProvider.Sut.BulkDeleteAccount(orgId, model));
-    }
-
    [Theory]
    [BitAutoData]
    public async Task BulkDeleteAccount_WhenCurrentUserNotFound_ThrowsUnauthorizedAccessException(
--- a/test/Core.Test/AdminConsole/Authorization/OrganizationUserUserMiniDetailsAuthorizationHandlerTests.cs
+++ b/test/Core.Test/AdminConsole/Authorization/OrganizationUserUserMiniDetailsAuthorizationHandlerTests.cs
@@ -1,81 +0,0 @@
-using System.Security.Claims;
-using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization;
-using Bit.Core.AdminConsole.OrganizationFeatures.Shared.Authorization;
-using Bit.Core.Context;
-using Bit.Core.Enums;
-using Bit.Core.Test.AdminConsole.AutoFixture;
-using Bit.Test.Common.AutoFixture;
-using Bit.Test.Common.AutoFixture.Attributes;
-using Microsoft.AspNetCore.Authorization;
-using NSubstitute;
-using Xunit;
-
-namespace Bit.Core.Test.AdminConsole.Authorization;
-
-[SutProviderCustomize]
-public class OrganizationUserUserMiniDetailsAuthorizationHandlerTests
-{
-    [Theory, CurrentContextOrganizationCustomize]
-    [BitAutoData(OrganizationUserType.Admin)]
-    [BitAutoData(OrganizationUserType.Owner)]
-    [BitAutoData(OrganizationUserType.Custom)]
-    [BitAutoData(OrganizationUserType.User)]
-    public async Task ReadAll_AnyOrganizationMember_Success(
-        OrganizationUserType userType,
-        CurrentContextOrganization organization,
-        SutProvider<OrganizationUserUserMiniDetailsAuthorizationHandler> sutProvider)
-    {
-        organization.Type = userType;
-        sutProvider.GetDependency<ICurrentContext>().GetOrganization(organization.Id).Returns(organization);
-
-        var context = new AuthorizationHandlerContext(
-            new[] { OrganizationUserUserMiniDetailsOperations.ReadAll },
-            new ClaimsPrincipal(),
-            new OrganizationScope(organization.Id));
-
-        await sutProvider.Sut.HandleAsync(context);
-
-        Assert.True(context.HasSucceeded);
-    }
-
-    [Theory, BitAutoData, CurrentContextOrganizationCustomize]
-    public async Task ReadAll_ProviderUser_Success(
-        CurrentContextOrganization organization,
-        SutProvider<OrganizationUserUserMiniDetailsAuthorizationHandler> sutProvider)
-    {
-        organization.Type = OrganizationUserType.User;
-        sutProvider.GetDependency<ICurrentContext>()
-            .GetOrganization(organization.Id)
-            .Returns((CurrentContextOrganization)null);
-        sutProvider.GetDependency<ICurrentContext>()
-            .ProviderUserForOrgAsync(organization.Id)
-            .Returns(true);
-
-        var context = new AuthorizationHandlerContext(
-            new[] { OrganizationUserUserMiniDetailsOperations.ReadAll },
-            new ClaimsPrincipal(),
-            new OrganizationScope(organization.Id));
-
-        await sutProvider.Sut.HandleAsync(context);
-
-        Assert.True(context.HasSucceeded);
-    }
-
-    [Theory, BitAutoData, CurrentContextOrganizationCustomize]
-    public async Task ReadAll_NotMember_NoSuccess(
-        CurrentContextOrganization organization,
-        SutProvider<OrganizationUserUserMiniDetailsAuthorizationHandler> sutProvider)
-    {
-        var context = new AuthorizationHandlerContext(
-            new[] { OrganizationUserUserMiniDetailsOperations.ReadAll },
-            new ClaimsPrincipal(),
-            new OrganizationScope(organization.Id)
-        );
-
-        sutProvider.GetDependency<ICurrentContext>().GetOrganization(Arg.Any<Guid>()).Returns((CurrentContextOrganization)null);
-        sutProvider.GetDependency<ICurrentContext>().ProviderUserForOrgAsync(Arg.Any<Guid>()).Returns(false);
-
-        await sutProvider.Sut.HandleAsync(context);
-        Assert.False(context.HasSucceeded);
-    }
-}