diff --git a/src/Api/AdminConsole/Authorization/AuthorizationHandlerCollectionExtensions.cs b/src/Api/AdminConsole/Authorization/AuthorizationHandlerCollectionExtensions.cs index 02f6c1fcc3c7..2149043e54d7 100644 --- a/src/Api/AdminConsole/Authorization/AuthorizationHandlerCollectionExtensions.cs +++ b/src/Api/AdminConsole/Authorization/AuthorizationHandlerCollectionExtensions.cs @@ -1,6 +1,5 @@ using Bit.Api.AdminConsole.Authorization.Collections; using Bit.Api.AdminConsole.Authorization.Providers; -using Bit.Core.AdminConsole.OrganizationFeatures.Groups.Authorization; using Microsoft.AspNetCore.Authorization; using Microsoft.Extensions.DependencyInjection.Extensions; @@ -15,7 +14,7 @@ public static void AddAdminConsoleAuthorizationHandlers(this IServiceCollection services.TryAddEnumerable([ ServiceDescriptor.Scoped(), ServiceDescriptor.Scoped(), - ServiceDescriptor.Scoped(), + ServiceDescriptor.Scoped(), ServiceDescriptor.Scoped(), ServiceDescriptor.Scoped(), ServiceDescriptor.Scoped(), diff --git a/src/Api/AdminConsole/Authorization/Collections/BulkCollectionAuthorizationHandler.cs b/src/Api/AdminConsole/Authorization/Collections/BulkCollectionAuthorizationHandler.cs index 287d5dffe769..67db4430710b 100644 --- a/src/Api/AdminConsole/Authorization/Collections/BulkCollectionAuthorizationHandler.cs +++ b/src/Api/AdminConsole/Authorization/Collections/BulkCollectionAuthorizationHandler.cs @@ -116,21 +116,7 @@ protected override async Task HandleRequirementAsync(AuthorizationHandlerContext private async Task CanCreateAsync(CurrentContextOrganization? org) { - // Owners, Admins, and users with CreateNewCollections permission can always create collections - if (org is - { Type: OrganizationUserType.Owner or OrganizationUserType.Admin } or - { Permissions.CreateNewCollections: true }) - { - return true; - } - - var organizationAbility = await GetOrganizationAbilityAsync(org); - - var userIsMemberOfOrg = org is not null; - var limitCollectionCreationEnabled = await GetOrganizationAbilityAsync(org) is { LimitCollectionCreation: true }; - var userIsOrgOwnerOrAdmin = org is { Type: OrganizationUserType.Owner or OrganizationUserType.Admin }; - // If the limit collection management setting is disabled, allow any user to create collections - if (userIsMemberOfOrg && (!limitCollectionCreationEnabled || userIsOrgOwnerOrAdmin)) + if (CollectionPermissions.CanCreate(org, await GetOrganizationAbilityAsync(org))) { return true; } diff --git a/src/Api/AdminConsole/Authorization/Collections/CollectionPermissions.cs b/src/Api/AdminConsole/Authorization/Collections/CollectionPermissions.cs new file mode 100644 index 000000000000..d2691efbe1d9 --- /dev/null +++ b/src/Api/AdminConsole/Authorization/Collections/CollectionPermissions.cs @@ -0,0 +1,31 @@ +#nullable enable +using Bit.Core.Context; +using Bit.Core.Enums; +using Bit.Core.Models.Data.Organizations; + +namespace Bit.Api.AdminConsole.Authorization.Collections; + +/// +/// Reusable, pure permission checks for Collections that other authorization requirements/handlers can depend on +/// without needing to invoke directly. +/// +public static class CollectionPermissions +{ + /// + /// Returns true if the user is allowed to create a new collection in the organization. + /// This does not account for Provider users - callers must check that separately (it requires a database call). + /// + public static bool CanCreate(CurrentContextOrganization? organizationClaims, OrganizationAbility? organizationAbility) + { + // Owners, Admins, and users with CreateNewCollections permission can always create collections + if (organizationClaims is + { Type: OrganizationUserType.Owner or OrganizationUserType.Admin } or + { Permissions.CreateNewCollections: true }) + { + return true; + } + + // If the limit collection creation setting is disabled, allow any member to create collections + return organizationClaims is not null && organizationAbility is not { LimitCollectionCreation: true }; + } +} diff --git a/src/Api/AdminConsole/Authorization/Organizations/Handlers/OrganizationCollectionManagementAccessRequirement.cs b/src/Api/AdminConsole/Authorization/Organizations/Handlers/OrganizationCollectionManagementAccessRequirement.cs new file mode 100644 index 000000000000..c60470c9f333 --- /dev/null +++ b/src/Api/AdminConsole/Authorization/Organizations/Handlers/OrganizationCollectionManagementAccessRequirement.cs @@ -0,0 +1,92 @@ +#nullable enable + +using Bit.Api.AdminConsole.Authorization.Collections; +using Bit.Core.AdminConsole.AbilitiesCache; +using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Enums; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Microsoft.AspNetCore.Authorization; + +namespace Bit.Api.AdminConsole.Authorization; + +/// +/// Requires that the user is allowed to create collections, has Can Manage permissions for at least one +/// collection in the organization, or has a custom permission (ManageUsers, ManageGroups, or AccessReports) that +/// depends on this same basic organization member/group information for an unrelated reason (e.g. the Members +/// page, Groups page, and Member Access Report). This data is not privileged - it contains as little information +/// as possible and no cryptographic keys or other sensitive data - but if an organization has restricted +/// collection management to a subset of users, there's no reason to expose it more broadly than the users who +/// actually have some legitimate need for it. +/// +/// +/// This intentionally does not implement because it needs more than JWT +/// claims and a provider check to answer the question - it needs the Organization's ability settings and, in the +/// less common case where collection creation is restricted, a database call to check the user's collection +/// access. Pulls the organization ID from the route itself, following the same shape as +/// . +/// +public class OrganizationCollectionManagementAccessRequirement : IAuthorizationRequirement; + +public class OrganizationCollectionManagementAccessHandler( + IHttpContextAccessor httpContextAccessor, + IUserService userService, + IProviderUserRepository providerUserRepository, + IOrganizationAbilityCacheService organizationAbilityCacheService, + ICollectionRepository collectionRepository) + : AuthorizationHandler +{ + protected override async Task HandleRequirementAsync( + AuthorizationHandlerContext context, + OrganizationCollectionManagementAccessRequirement requirement) + { + var httpContext = httpContextAccessor.HttpContext + ?? throw new InvalidOperationException("This handler requires an HTTP context."); + + var userId = userService.GetProperUserId(httpContext.User); + if (userId is null) + { + return; + } + + var orgId = httpContext.GetOrganizationId(); + var organizationClaims = httpContext.User.GetCurrentContextOrganization(orgId); + var organizationAbility = await organizationAbilityCacheService.GetOrganizationAbilityAsync(orgId); + + if (CollectionPermissions.CanCreate(organizationClaims, organizationAbility)) + { + context.Succeed(requirement); + return; + } + + // Custom users who manage org members/groups or view reports have their own legitimate need for this + // basic directory data, independent of their collection permissions. + if (organizationClaims is { Type: OrganizationUserType.Custom } && + (organizationClaims.Permissions.ManageUsers || + organizationClaims.Permissions.ManageGroups || + organizationClaims.Permissions.AccessReports)) + { + context.Succeed(requirement); + return; + } + + // The user is a confirmed member of the organization but cannot create collections - check whether they + // have Can Manage permissions on at least one collection instead. + if (organizationClaims is not null) + { + var collections = await collectionRepository.GetManySharedByOrganizationIdWithPermissionsAsync( + orgId, userId.Value, includeAccessRelationships: false); + if (collections.Any(c => c.Manage)) + { + context.Succeed(requirement); + return; + } + } + + // Allow provider users to access this information if they are a provider for the target organization + if (await httpContext.IsProviderUserForOrgAsync(providerUserRepository, userId.Value, orgId)) + { + context.Succeed(requirement); + } + } +} diff --git a/src/Api/AdminConsole/Controllers/GroupsController.cs b/src/Api/AdminConsole/Controllers/GroupsController.cs index 6bc78347292c..b1331b9b0b9b 100644 --- a/src/Api/AdminConsole/Controllers/GroupsController.cs +++ b/src/Api/AdminConsole/Controllers/GroupsController.cs @@ -88,7 +88,7 @@ public async Task GetDetails(Guid orgId, Guid id) } [HttpGet("")] - [Authorize] + [Authorize] public async Task> GetOrganizationGroups(Guid orgId) { var groups = await _groupRepository.GetManyByOrganizationIdAsync(orgId); diff --git a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs index 62e07987d155..2ad675bb5174 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs @@ -190,9 +190,11 @@ public async Task Get(Guid orgId, Guid id, } /// - /// Returns a set of basic information about all members of the organization. This is available to all members of - /// the organization to manage collections. For this reason, it contains as little information as possible and no - /// cryptographic keys or other sensitive data. + /// Returns a set of basic information about all members of the organization. This is available to all members + /// of the organization, since a broad range of features across the app depend on basic member lookups + /// (collection management, group management, event logs, sponsorship, etc.) that are not specific to any one + /// permission. For this reason, it contains as little information as possible and no cryptographic keys or + /// other sensitive data. /// /// Organization identifier /// List of users for the organization. diff --git a/src/Core/AdminConsole/OrganizationFeatures/Groups/Authorization/GroupAuthorizationHandler.cs b/src/Core/AdminConsole/OrganizationFeatures/Groups/Authorization/GroupAuthorizationHandler.cs deleted file mode 100644 index d531c1175d38..000000000000 --- a/src/Core/AdminConsole/OrganizationFeatures/Groups/Authorization/GroupAuthorizationHandler.cs +++ /dev/null @@ -1,43 +0,0 @@ -#nullable enable -using Bit.Core.AdminConsole.OrganizationFeatures.Shared.Authorization; -using Bit.Core.Context; -using Bit.Core.Enums; -using Microsoft.AspNetCore.Authorization; - -namespace Bit.Core.AdminConsole.OrganizationFeatures.Groups.Authorization; - -public class GroupAuthorizationHandler(ICurrentContext currentContext) - : AuthorizationHandler -{ - protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, - GroupOperationRequirement requirement, OrganizationScope organizationScope) - { - var authorized = requirement switch - { - not null when requirement.Name == nameof(GroupOperations.ReadAll) => - await CanReadAllAsync(organizationScope), - not null when requirement.Name == nameof(GroupOperations.ReadAllDetails) => - await CanViewGroupDetailsAsync(organizationScope), - _ => false - }; - - if (requirement is not null && authorized) - { - context.Succeed(requirement); - } - } - - private async Task CanReadAllAsync(OrganizationScope organizationScope) => - currentContext.GetOrganization(organizationScope) is not null - || await currentContext.ProviderUserForOrgAsync(organizationScope); - - private async Task CanViewGroupDetailsAsync(OrganizationScope organizationScope) => - currentContext.GetOrganization(organizationScope) is - { Type: OrganizationUserType.Owner } or - { Type: OrganizationUserType.Admin } or - { - Permissions: { ManageGroups: true } or - { ManageUsers: true } - } || - await currentContext.ProviderUserForOrgAsync(organizationScope); -} diff --git a/src/Core/AdminConsole/OrganizationFeatures/Groups/Authorization/GroupOperations.cs b/src/Core/AdminConsole/OrganizationFeatures/Groups/Authorization/GroupOperations.cs deleted file mode 100644 index f5b3b975f9e9..000000000000 --- a/src/Core/AdminConsole/OrganizationFeatures/Groups/Authorization/GroupOperations.cs +++ /dev/null @@ -1,17 +0,0 @@ -using Microsoft.AspNetCore.Authorization.Infrastructure; - -namespace Bit.Core.AdminConsole.OrganizationFeatures.Groups.Authorization; - -public class GroupOperationRequirement : OperationAuthorizationRequirement -{ - public GroupOperationRequirement(string name) - { - Name = name; - } -} - -public static class GroupOperations -{ - public static readonly GroupOperationRequirement ReadAll = new(nameof(ReadAll)); - public static readonly GroupOperationRequirement ReadAllDetails = new(nameof(ReadAllDetails)); -} diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserUserMiniDetailsOperations.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserUserMiniDetailsOperations.cs deleted file mode 100644 index 285bf44f429a..000000000000 --- a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserUserMiniDetailsOperations.cs +++ /dev/null @@ -1,10 +0,0 @@ -using Microsoft.AspNetCore.Authorization.Infrastructure; - -namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization; - -public class OrganizationUserUserMiniDetailsOperationRequirement : OperationAuthorizationRequirement; - -public static class OrganizationUserUserMiniDetailsOperations -{ - public static readonly OrganizationUserUserMiniDetailsOperationRequirement ReadAll = new() { Name = nameof(ReadAll) }; -} diff --git a/test/Api.IntegrationTest/AdminConsole/Controllers/GroupsControllerTests.cs b/test/Api.IntegrationTest/AdminConsole/Controllers/GroupsControllerTests.cs index e8120c80d53c..0b67ba467ae5 100644 --- a/test/Api.IntegrationTest/AdminConsole/Controllers/GroupsControllerTests.cs +++ b/test/Api.IntegrationTest/AdminConsole/Controllers/GroupsControllerTests.cs @@ -1,5 +1,6 @@ using System.Net; using Bit.Api.AdminConsole.Models.Request; +using Bit.Api.AdminConsole.Models.Request.Organizations; using Bit.Api.IntegrationTest.Factories; using Bit.Api.IntegrationTest.Helpers; using Bit.Core.AdminConsole.Entities; @@ -177,6 +178,97 @@ public async Task GetOrganizationGroups_NotAMember_ReturnsForbidden() Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); } + [Fact] + public async Task GetOrganizationGroups_AsUserWithLimitCollectionCreationEnabledAndNoCollectionAccess_ReturnsForbidden() + { + var (email, _) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync( + _factory, _organization.Id, OrganizationUserType.User); + + await EnableLimitCollectionCreationAsync(); + + await _loginHelper.LoginAsync(email); + + var response = await _client.GetAsync($"/organizations/{_organization.Id}/groups"); + + Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); + } + + [Fact] + public async Task GetOrganizationGroups_AsUserWithLimitCollectionCreationEnabledButManagesACollection_ReturnsSuccess() + { + var (email, orgUser) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync( + _factory, _organization.Id, OrganizationUserType.User); + + await OrganizationTestHelpers.CreateCollectionAsync( + _factory, + _organization.Id, + "Managed Collection", + users: [new CollectionAccessSelection { Id = orgUser.Id, ReadOnly = false, HidePasswords = false, Manage = true }]); + + await EnableLimitCollectionCreationAsync(); + + await _loginHelper.LoginAsync(email); + + var response = await _client.GetAsync($"/organizations/{_organization.Id}/groups"); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + + [Theory] + [InlineData(true, false, false)] + [InlineData(false, true, false)] + [InlineData(false, false, true)] + public async Task GetOrganizationGroups_AsCustomWithManageUsersOrGroupsOrAccessReports_ReturnsSuccess( + bool manageUsers, bool manageGroups, bool accessReports) + { + var (email, _) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync( + _factory, _organization.Id, OrganizationUserType.Custom, + permissions: new Permissions + { + ManageUsers = manageUsers, + ManageGroups = manageGroups, + AccessReports = accessReports, + }); + + await EnableLimitCollectionCreationAsync(); + + await _loginHelper.LoginAsync(email); + + var response = await _client.GetAsync($"/organizations/{_organization.Id}/groups"); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + + [Fact] + public async Task GetOrganizationGroups_AsCustomWithUnrelatedPermissionAndNoCollectionAccess_ReturnsForbidden() + { + var (email, _) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync( + _factory, _organization.Id, OrganizationUserType.Custom, + permissions: new Permissions { AccessEventLogs = true, ManagePolicies = true }); + + await EnableLimitCollectionCreationAsync(); + + await _loginHelper.LoginAsync(email); + + var response = await _client.GetAsync($"/organizations/{_organization.Id}/groups"); + + Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); + } + + /// + /// Enables the "Limit collection creation" collection management setting for the test organization via the + /// public API, ensuring the OrganizationAbility cache is updated as it would be in production. + /// + private async Task EnableLimitCollectionCreationAsync() + { + await _loginHelper.LoginAsync(_ownerEmail); + + var response = await _client.PutAsJsonAsync($"/organizations/{_organization.Id}/collection-management", + new OrganizationCollectionManagementUpdateRequestModel { LimitCollectionCreation = true }); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + [Fact] public async Task GetOrganizationGroupDetails_AsOwner_ReturnsSuccess() { diff --git a/test/Api.IntegrationTest/AdminConsole/Controllers/OrganizationUsersControllerGetMiniDetailsTests.cs b/test/Api.IntegrationTest/AdminConsole/Controllers/OrganizationUsersControllerGetMiniDetailsTests.cs new file mode 100644 index 000000000000..a1038c55a330 --- /dev/null +++ b/test/Api.IntegrationTest/AdminConsole/Controllers/OrganizationUsersControllerGetMiniDetailsTests.cs @@ -0,0 +1,108 @@ +using System.Net; +using Bit.Api.IntegrationTest.Factories; +using Bit.Api.IntegrationTest.Helpers; +using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.Enums.Provider; +using Bit.Core.Billing.Enums; +using Bit.Core.Enums; +using Xunit; + +namespace Bit.Api.IntegrationTest.AdminConsole.Controllers; + +public class OrganizationUsersControllerGetMiniDetailsTests : IClassFixture, IAsyncLifetime +{ + private readonly HttpClient _client; + private readonly ApiApplicationFactory _factory; + private readonly LoginHelper _loginHelper; + + private string _ownerEmail = null!; + private Organization _organization = null!; + + public OrganizationUsersControllerGetMiniDetailsTests(ApiApplicationFactory factory) + { + _factory = factory; + _client = factory.CreateClient(); + _loginHelper = new LoginHelper(_factory, _client); + } + + public async Task InitializeAsync() + { + _ownerEmail = $"integration-test{Guid.NewGuid()}@bitwarden.com"; + await _factory.LoginWithNewAccount(_ownerEmail); + + (_organization, _) = await OrganizationTestHelpers.SignUpAsync( + _factory, + plan: PlanType.EnterpriseAnnually, + ownerEmail: _ownerEmail, + passwordManagerSeats: 10, + paymentMethod: PaymentMethodType.Card); + } + + public Task DisposeAsync() + { + _client.Dispose(); + return Task.CompletedTask; + } + + [Fact] + public async Task GetMiniDetails_AsOwner_ReturnsSuccess() + { + await _loginHelper.LoginAsync(_ownerEmail); + + var response = await _client.GetAsync($"/organizations/{_organization.Id}/users/mini-details"); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + + [Fact] + public async Task GetMiniDetails_AsUser_ReturnsSuccess() + { + var (email, _) = await OrganizationTestHelpers.CreateNewUserWithAccountAsync( + _factory, _organization.Id, OrganizationUserType.User); + await _loginHelper.LoginAsync(email); + + var response = await _client.GetAsync($"/organizations/{_organization.Id}/users/mini-details"); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + + [Fact] + public async Task GetMiniDetails_AsProviderUser_ReturnsSuccess() + { + var email = await CreateProviderUserForOrganizationAsync(); + await _loginHelper.LoginAsync(email); + + var response = await _client.GetAsync($"/organizations/{_organization.Id}/users/mini-details"); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + + [Fact] + public async Task GetMiniDetails_NotAMember_ReturnsForbidden() + { + var email = $"integration-test{Guid.NewGuid()}@bitwarden.com"; + await _factory.LoginWithNewAccount(email); + await _loginHelper.LoginAsync(email); + + var response = await _client.GetAsync($"/organizations/{_organization.Id}/users/mini-details"); + + Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); + } + + /// + /// Creates a provider linked to the test organization and returns a provider user's credentials. + /// + private async Task CreateProviderUserForOrganizationAsync() + { + var email = $"integration-test{Guid.NewGuid()}@bitwarden.com"; + await _factory.LoginWithNewAccount(email); + + var provider = await ProviderTestHelpers.CreateProviderAndLinkToOrganizationAsync( + _factory, _organization.Id, ProviderType.Msp); + + await ProviderTestHelpers.CreateProviderUserAsync( + _factory, provider.Id, email, ProviderUserType.ServiceUser); + + return email; + } +} diff --git a/test/Api.Test/AdminConsole/Authorization/CollectionPermissionsTests.cs b/test/Api.Test/AdminConsole/Authorization/CollectionPermissionsTests.cs new file mode 100644 index 000000000000..36b6c8a654c3 --- /dev/null +++ b/test/Api.Test/AdminConsole/Authorization/CollectionPermissionsTests.cs @@ -0,0 +1,75 @@ +#nullable enable +using Bit.Api.AdminConsole.Authorization.Collections; +using Bit.Core.Context; +using Bit.Core.Enums; +using Bit.Core.Models.Data; +using Bit.Core.Models.Data.Organizations; +using Bit.Test.Common.AutoFixture.Attributes; +using Xunit; + +namespace Bit.Api.Test.AdminConsole.Authorization; + +public class CollectionPermissionsTests +{ + [Theory] + [BitAutoData(OrganizationUserType.Owner)] + [BitAutoData(OrganizationUserType.Admin)] + public void CanCreate_WhenOwnerOrAdmin_ReturnsTrue(OrganizationUserType type, Guid orgId) + { + var organizationClaims = new CurrentContextOrganization { Id = orgId, Type = type }; + var organizationAbility = new OrganizationAbility { Id = orgId, LimitCollectionCreation = true }; + + Assert.True(CollectionPermissions.CanCreate(organizationClaims, organizationAbility)); + } + + [Theory, BitAutoData] + public void CanCreate_WhenCustomUserWithCreateNewCollectionsPermission_ReturnsTrue(Guid orgId) + { + var organizationClaims = new CurrentContextOrganization + { + Id = orgId, + Type = OrganizationUserType.Custom, + Permissions = new Permissions { CreateNewCollections = true } + }; + var organizationAbility = new OrganizationAbility { Id = orgId, LimitCollectionCreation = true }; + + Assert.True(CollectionPermissions.CanCreate(organizationClaims, organizationAbility)); + } + + [Theory] + [BitAutoData(OrganizationUserType.User)] + [BitAutoData(OrganizationUserType.Custom)] + public void CanCreate_WhenMemberAndLimitCollectionCreationDisabled_ReturnsTrue(OrganizationUserType type, Guid orgId) + { + var organizationClaims = new CurrentContextOrganization { Id = orgId, Type = type }; + var organizationAbility = new OrganizationAbility { Id = orgId, LimitCollectionCreation = false }; + + Assert.True(CollectionPermissions.CanCreate(organizationClaims, organizationAbility)); + } + + [Theory] + [BitAutoData(OrganizationUserType.User)] + [BitAutoData(OrganizationUserType.Custom)] + public void CanCreate_WhenMemberAndLimitCollectionCreationEnabledWithoutPermission_ReturnsFalse(OrganizationUserType type, Guid orgId) + { + var organizationClaims = new CurrentContextOrganization { Id = orgId, Type = type, Permissions = new Permissions() }; + var organizationAbility = new OrganizationAbility { Id = orgId, LimitCollectionCreation = true }; + + Assert.False(CollectionPermissions.CanCreate(organizationClaims, organizationAbility)); + } + + [Fact] + public void CanCreate_WhenNotAMember_ReturnsFalse() + { + Assert.False(CollectionPermissions.CanCreate(null, null)); + } + + [Theory, BitAutoData] + public void CanCreate_WhenMemberAndNoOrganizationAbility_ReturnsTrue(Guid orgId) + { + // A null OrganizationAbility means LimitCollectionCreation is treated as disabled + var organizationClaims = new CurrentContextOrganization { Id = orgId, Type = OrganizationUserType.User }; + + Assert.True(CollectionPermissions.CanCreate(organizationClaims, null)); + } +} diff --git a/test/Api.Test/AdminConsole/Authorization/Organizations/Handlers/OrganizationCollectionManagementAccessHandlerTests.cs b/test/Api.Test/AdminConsole/Authorization/Organizations/Handlers/OrganizationCollectionManagementAccessHandlerTests.cs new file mode 100644 index 000000000000..a0088ab1dfc5 --- /dev/null +++ b/test/Api.Test/AdminConsole/Authorization/Organizations/Handlers/OrganizationCollectionManagementAccessHandlerTests.cs @@ -0,0 +1,232 @@ +#nullable enable +using System.Security.Claims; +using Bit.Api.AdminConsole.Authorization; +using Bit.Core.AdminConsole.AbilitiesCache; +using Bit.Core.AdminConsole.Enums.Provider; +using Bit.Core.AdminConsole.Models.Data.Provider; +using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Context; +using Bit.Core.Entities; +using Bit.Core.Enums; +using Bit.Core.Models.Data; +using Bit.Core.Models.Data.Organizations; +using Bit.Core.Repositories; +using Bit.Core.Services; +using Bit.Core.Utilities; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Http; +using NSubstitute; +using Xunit; + +namespace Bit.Api.Test.AdminConsole.Authorization; + +[SutProviderCustomize] +public class OrganizationCollectionManagementAccessHandlerTests +{ + [Theory, BitAutoData] + public async Task HandleRequirementAsync_WhenUserIdIsNull_NotAuthorized( + Guid orgId, SutProvider sutProvider) + { + ArrangeRoute(sutProvider, orgId, null); + + var context = Authorize(sutProvider); + + Assert.False(context.HasSucceeded); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .GetManySharedByOrganizationIdWithPermissionsAsync(default, default, default); + } + + [Theory] + [BitAutoData(OrganizationUserType.Owner)] + [BitAutoData(OrganizationUserType.Admin)] + public async Task HandleRequirementAsync_WhenOwnerOrAdmin_Authorized_WithoutQueryingCollections( + OrganizationUserType type, Guid orgId, Guid userId, User user, + SutProvider sutProvider) + { + ArrangeRoute(sutProvider, orgId, userId); + ArrangeOrganizationAbility(sutProvider, orgId, limitCollectionCreation: true); + var claimsPrincipal = BuildClaimsPrincipal(user, new CurrentContextOrganization { Id = orgId, Type = type }); + + var context = Authorize(sutProvider, claimsPrincipal); + + Assert.True(context.HasSucceeded); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .GetManySharedByOrganizationIdWithPermissionsAsync(default, default, default); + } + + [Theory] + [BitAutoData(true, false, false)] + [BitAutoData(false, true, false)] + [BitAutoData(false, false, true)] + public async Task HandleRequirementAsync_WhenCustomUserManagesUsersOrGroupsOrAccessesReports_Authorized_WithoutQueryingCollections( + bool manageUsers, bool manageGroups, bool accessReports, Guid orgId, Guid userId, User user, + SutProvider sutProvider) + { + ArrangeRoute(sutProvider, orgId, userId); + ArrangeOrganizationAbility(sutProvider, orgId, limitCollectionCreation: true); + var claimsPrincipal = BuildClaimsPrincipal(user, new CurrentContextOrganization + { + Id = orgId, + Type = OrganizationUserType.Custom, + Permissions = new Permissions + { + ManageUsers = manageUsers, + ManageGroups = manageGroups, + AccessReports = accessReports, + } + }); + + var context = Authorize(sutProvider, claimsPrincipal); + + Assert.True(context.HasSucceeded); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .GetManySharedByOrganizationIdWithPermissionsAsync(default, default, default); + } + + [Theory, BitAutoData] + public async Task HandleRequirementAsync_WhenCustomUserHasUnrelatedPermissionAndDoesNotManageAnyCollection_NotAuthorized( + Guid orgId, Guid userId, User user, List collections, + SutProvider sutProvider) + { + ArrangeRoute(sutProvider, orgId, userId); + ArrangeOrganizationAbility(sutProvider, orgId, limitCollectionCreation: true); + var claimsPrincipal = BuildClaimsPrincipal(user, new CurrentContextOrganization + { + Id = orgId, + Type = OrganizationUserType.Custom, + Permissions = new Permissions { AccessEventLogs = true, ManagePolicies = true }, + }); + + collections.ForEach(c => c.Manage = false); + sutProvider.GetDependency() + .GetManySharedByOrganizationIdWithPermissionsAsync(orgId, userId, false) + .Returns(collections); + ArrangeProvider(sutProvider, userId, orgId, isProvider: false); + + var context = Authorize(sutProvider, claimsPrincipal); + + Assert.False(context.HasSucceeded); + } + + [Theory, BitAutoData] + public async Task HandleRequirementAsync_WhenMemberCannotCreateButManagesACollection_Authorized( + Guid orgId, Guid userId, User user, List collections, + SutProvider sutProvider) + { + ArrangeRoute(sutProvider, orgId, userId); + ArrangeOrganizationAbility(sutProvider, orgId, limitCollectionCreation: true); + var claimsPrincipal = BuildClaimsPrincipal(user, + new CurrentContextOrganization { Id = orgId, Type = OrganizationUserType.User }); + + collections[0].Manage = true; + sutProvider.GetDependency() + .GetManySharedByOrganizationIdWithPermissionsAsync(orgId, userId, false) + .Returns(collections); + + var context = Authorize(sutProvider, claimsPrincipal); + + Assert.True(context.HasSucceeded); + } + + [Theory, BitAutoData] + public async Task HandleRequirementAsync_WhenMemberCannotCreateAndDoesNotManageAnyCollection_NotAuthorized( + Guid orgId, Guid userId, User user, List collections, + SutProvider sutProvider) + { + ArrangeRoute(sutProvider, orgId, userId); + ArrangeOrganizationAbility(sutProvider, orgId, limitCollectionCreation: true); + var claimsPrincipal = BuildClaimsPrincipal(user, + new CurrentContextOrganization { Id = orgId, Type = OrganizationUserType.User }); + + collections.ForEach(c => c.Manage = false); + sutProvider.GetDependency() + .GetManySharedByOrganizationIdWithPermissionsAsync(orgId, userId, false) + .Returns(collections); + ArrangeProvider(sutProvider, userId, orgId, isProvider: false); + + var context = Authorize(sutProvider, claimsPrincipal); + + Assert.False(context.HasSucceeded); + } + + [Theory, BitAutoData] + public async Task HandleRequirementAsync_WhenNotMemberButProvider_Authorized_WithoutQueryingCollections( + Guid orgId, Guid userId, SutProvider sutProvider) + { + ArrangeRoute(sutProvider, orgId, userId); + ArrangeOrganizationAbility(sutProvider, orgId, limitCollectionCreation: true); + ArrangeProvider(sutProvider, userId, orgId, isProvider: true); + + var context = Authorize(sutProvider, new ClaimsPrincipal(new ClaimsIdentity())); + + Assert.True(context.HasSucceeded); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .GetManySharedByOrganizationIdWithPermissionsAsync(default, default, default); + } + + [Theory, BitAutoData] + public async Task HandleRequirementAsync_WhenNotMemberOrProvider_NotAuthorized( + Guid orgId, Guid userId, SutProvider sutProvider) + { + ArrangeRoute(sutProvider, orgId, userId); + ArrangeOrganizationAbility(sutProvider, orgId, limitCollectionCreation: true); + ArrangeProvider(sutProvider, userId, orgId, isProvider: false); + + var context = Authorize(sutProvider, new ClaimsPrincipal(new ClaimsIdentity())); + + Assert.False(context.HasSucceeded); + } + + private static AuthorizationHandlerContext Authorize( + SutProvider sutProvider, ClaimsPrincipal? claimsPrincipal = null) + { + var httpContext = sutProvider.GetDependency().HttpContext!; + httpContext.User = claimsPrincipal ?? new ClaimsPrincipal(new ClaimsIdentity()); + + var requirement = new OrganizationCollectionManagementAccessRequirement(); + var context = new AuthorizationHandlerContext([requirement], httpContext.User, null); + + sutProvider.Sut.HandleAsync(context).GetAwaiter().GetResult(); + return context; + } + + private static void ArrangeRoute( + SutProvider sutProvider, Guid orgId, Guid? userId) + { + var httpContext = new DefaultHttpContext(); + httpContext.Request.RouteValues["orgId"] = orgId.ToString(); + sutProvider.GetDependency().HttpContext = httpContext; + sutProvider.GetDependency().GetProperUserId(Arg.Any()).Returns(userId); + } + + private static void ArrangeOrganizationAbility( + SutProvider sutProvider, Guid orgId, bool limitCollectionCreation) + { + sutProvider.GetDependency().GetOrganizationAbilityAsync(orgId) + .Returns(new OrganizationAbility { Id = orgId, LimitCollectionCreation = limitCollectionCreation }); + } + + private static void ArrangeProvider( + SutProvider sutProvider, Guid userId, Guid orgId, bool isProvider) + { + var organizations = isProvider + ? [new ProviderUserOrganizationDetails { OrganizationId = orgId }] + : Array.Empty(); + + sutProvider.GetDependency() + .GetManyOrganizationDetailsByUserAsync(userId, ProviderUserStatusType.Confirmed) + .Returns(organizations); + } + + private static ClaimsPrincipal BuildClaimsPrincipal(User user, CurrentContextOrganization organization) + { + var claims = CoreHelpers.BuildIdentityClaims(user, [organization], [], false) + .Select(c => new Claim(c.Key, c.Value)); + + var claimsPrincipal = new ClaimsPrincipal(); + claimsPrincipal.AddIdentities([new ClaimsIdentity(claims)]); + return claimsPrincipal; + } +} diff --git a/test/Api.Test/AdminConsole/AuthorizationHandlers/GroupAuthorizationHandlerTests.cs b/test/Api.Test/AdminConsole/AuthorizationHandlers/GroupAuthorizationHandlerTests.cs deleted file mode 100644 index a1b63c4fd047..000000000000 --- a/test/Api.Test/AdminConsole/AuthorizationHandlers/GroupAuthorizationHandlerTests.cs +++ /dev/null @@ -1,223 +0,0 @@ -using System.Security.Claims; -using Bit.Core.AdminConsole.OrganizationFeatures.Groups.Authorization; -using Bit.Core.AdminConsole.OrganizationFeatures.Shared.Authorization; -using Bit.Core.Context; -using Bit.Core.Enums; -using Bit.Core.Models.Data; -using Bit.Test.Common.AutoFixture; -using Bit.Test.Common.AutoFixture.Attributes; -using Microsoft.AspNetCore.Authorization; -using NSubstitute; -using Xunit; - -namespace Bit.Api.Test.AdminConsole.AuthorizationHandlers; - -[SutProviderCustomize] -public class GroupAuthorizationHandlerTests -{ - [Theory] - [BitAutoData(OrganizationUserType.Admin)] - [BitAutoData(OrganizationUserType.Owner)] - [BitAutoData(OrganizationUserType.User)] - [BitAutoData(OrganizationUserType.Custom)] - public async Task CanReadAllAsync_WhenMemberOfOrg_Success( - OrganizationUserType userType, - OrganizationScope scope, - Guid userId, SutProvider sutProvider, - CurrentContextOrganization organization) - { - organization.Type = userType; - organization.Permissions = new Permissions(); - - var context = new AuthorizationHandlerContext( - new[] { GroupOperations.ReadAll }, - new ClaimsPrincipal(), - scope); - - sutProvider.GetDependency().UserId.Returns(userId); - sutProvider.GetDependency().GetOrganization(scope).Returns(organization); - - await sutProvider.Sut.HandleAsync(context); - - Assert.True(context.HasSucceeded); - } - - [Theory, BitAutoData] - public async Task CanReadAllAsync_WithProviderUser_Success( - Guid userId, - OrganizationScope scope, - SutProvider sutProvider, CurrentContextOrganization organization) - { - organization.Type = OrganizationUserType.User; - organization.Permissions = new Permissions(); - - var context = new AuthorizationHandlerContext( - new[] { GroupOperations.ReadAll }, - new ClaimsPrincipal(), - scope); - - sutProvider.GetDependency() - .UserId - .Returns(userId); - sutProvider.GetDependency() - .ProviderUserForOrgAsync(scope) - .Returns(true); - - await sutProvider.Sut.HandleAsync(context); - - Assert.True(context.HasSucceeded); - } - - [Theory, BitAutoData] - public async Task CanReadAllAsync_WhenMissingOrgAccess_NoSuccess( - Guid userId, - OrganizationScope scope, - SutProvider sutProvider) - { - - var context = new AuthorizationHandlerContext( - new[] { GroupOperations.ReadAll }, - new ClaimsPrincipal(), - scope - ); - - sutProvider.GetDependency().UserId.Returns(userId); - sutProvider.GetDependency().GetOrganization(Arg.Any()).Returns((CurrentContextOrganization)null); - sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); - - await sutProvider.Sut.HandleAsync(context); - Assert.False(context.HasSucceeded); - } - - [Theory] - [BitAutoData(OrganizationUserType.Admin)] - [BitAutoData(OrganizationUserType.Owner)] - public async Task HandleRequirementsAsync_GivenViewDetailsOperation_WhenUserIsAdminOwner_ThenShouldSucceed(OrganizationUserType userType, - OrganizationScope scope, - CurrentContextOrganization organization, SutProvider sutProvider) - { - organization.Type = userType; - organization.Permissions = new Permissions(); - - var context = new AuthorizationHandlerContext( - [GroupOperations.ReadAllDetails], - new ClaimsPrincipal(), - scope - ); - - sutProvider.GetDependency().GetOrganization(scope).Returns(organization); - - await sutProvider.Sut.HandleAsync(context); - Assert.True(context.HasSucceeded); - } - - [Theory] - [BitAutoData(OrganizationUserType.User)] - [BitAutoData(OrganizationUserType.Custom)] - public async Task HandleRequirementsAsync_GivenViewDetailsOperation_WhenUserIsNotOwnerOrAdmin_ThenShouldFail(OrganizationUserType userType, - OrganizationScope scope, - CurrentContextOrganization organization, SutProvider sutProvider) - { - organization.Type = userType; - organization.Permissions = new Permissions(); - - var context = new AuthorizationHandlerContext( - [GroupOperations.ReadAllDetails], - new ClaimsPrincipal(), - scope - ); - - sutProvider.GetDependency().GetOrganization(scope).Returns(organization); - - await sutProvider.Sut.HandleAsync(context); - Assert.False(context.HasSucceeded); - } - - [Theory, BitAutoData] - public async Task HandleRequirementsAsync_GivenViewDetailsOperation_WhenUserHasManageGroupPermission_ThenShouldSucceed( - OrganizationScope scope, - CurrentContextOrganization organization, SutProvider sutProvider) - { - organization.Type = OrganizationUserType.Custom; - organization.Permissions = new Permissions - { - ManageGroups = true - }; - - var context = new AuthorizationHandlerContext( - [GroupOperations.ReadAllDetails], - new ClaimsPrincipal(), - scope - ); - - sutProvider.GetDependency().GetOrganization(scope).Returns(organization); - - await sutProvider.Sut.HandleAsync(context); - Assert.True(context.HasSucceeded); - } - - [Theory, BitAutoData] - public async Task HandleRequirementsAsync_GivenViewDetailsOperation_WhenUserHasManageUserPermission_ThenShouldSucceed( - OrganizationScope scope, - CurrentContextOrganization organization, SutProvider sutProvider) - { - organization.Type = OrganizationUserType.Custom; - organization.Permissions = new Permissions - { - ManageUsers = true - }; - - var context = new AuthorizationHandlerContext( - [GroupOperations.ReadAllDetails], - new ClaimsPrincipal(), - scope - ); - - sutProvider.GetDependency().GetOrganization(scope).Returns(organization); - - await sutProvider.Sut.HandleAsync(context); - Assert.True(context.HasSucceeded); - } - - [Theory, BitAutoData] - public async Task HandleRequirementsAsync_GivenViewDetailsOperation_WhenUserIsStandardUserTypeWithoutElevatedPermissions_ThenShouldFail( - OrganizationScope scope, - CurrentContextOrganization organization, SutProvider sutProvider) - { - organization.Type = OrganizationUserType.User; - organization.Permissions = new Permissions(); - - var context = new AuthorizationHandlerContext( - [GroupOperations.ReadAllDetails], - new ClaimsPrincipal(), - scope - ); - - sutProvider.GetDependency().GetOrganization(scope).Returns(organization); - sutProvider.GetDependency().ProviderUserForOrgAsync(scope).Returns(false); - - await sutProvider.Sut.HandleAsync(context); - Assert.False(context.HasSucceeded); - } - - [Theory, BitAutoData] - public async Task HandleRequirementsAsync_GivenViewDetailsOperation_WhenIsProviderUser_ThenShouldSucceed( - OrganizationScope scope, - SutProvider sutProvider, CurrentContextOrganization organization) - { - organization.Type = OrganizationUserType.User; - organization.Permissions = new Permissions(); - - var context = new AuthorizationHandlerContext( - new[] { GroupOperations.ReadAll }, - new ClaimsPrincipal(), - scope); - - sutProvider.GetDependency().GetOrganization(scope).Returns(organization); - sutProvider.GetDependency().ProviderUserForOrgAsync(scope).Returns(true); - - await sutProvider.Sut.HandleAsync(context); - - Assert.True(context.HasSucceeded); - } -}