From c94a084c86f9e2a653e3e1562906a2ca8db321c9 Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Tue, 1 Oct 2024 07:14:16 +1000 Subject: [PATCH] [PM-3478] Refactor OrganizationUser api (#4752) * Add OrganizationUserMiniDetails endpoint, models and authorization * Restrict access to current OrganizationUserUserDetails endpoint Both are behind feature flags --- .../OrganizationUsersController.cs | 23 ++- .../OrganizationUserResponseModel.cs | 23 +++ .../Utilities/ServiceCollectionExtensions.cs | 2 - .../OrganizationUserAuthorizationHandler.cs | 60 ------ .../OrganizationUserOperations.cs | 22 --- .../Authorization/OrganizationScope.cs | 23 +++ ...tionUserUserDetailsAuthorizationHandler.cs | 77 ++++++++ .../OrganizationUserUserDetailsOperations.cs | 10 + ...UserUserMiniDetailsAuthorizationHandler.cs | 51 +++++ ...ganizationUserUserMiniDetailsOperations.cs | 10 + src/Core/Constants.cs | 1 + ...OrganizationServiceCollectionExtensions.cs | 5 + ...ganizationUserAuthorizationHandlerTests.cs | 122 ------------ ...serUserDetailsAuthorizationHandlerTests.cs | 176 ++++++++++++++++++ ...serMiniDetailsAuthorizationHandlerTests.cs | 80 ++++++++ .../CurrentContextOrganizationFixtures.cs | 40 ++++ 16 files changed, 514 insertions(+), 211 deletions(-) delete mode 100644 src/Api/Vault/AuthorizationHandlers/OrganizationUsers/OrganizationUserAuthorizationHandler.cs delete mode 100644 src/Api/Vault/AuthorizationHandlers/OrganizationUsers/OrganizationUserOperations.cs create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationScope.cs create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserUserDetailsAuthorizationHandler.cs create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserUserDetailsOperations.cs create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserUserMiniDetailsAuthorizationHandler.cs create mode 100644 src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserUserMiniDetailsOperations.cs delete mode 100644 test/Api.Test/Vault/AuthorizationHandlers/OrganizationUserAuthorizationHandlerTests.cs create mode 100644 test/Core.Test/AdminConsole/Authorization/OrganizationUserUserDetailsAuthorizationHandlerTests.cs create mode 100644 test/Core.Test/AdminConsole/Authorization/OrganizationUserUserMiniDetailsAuthorizationHandlerTests.cs create mode 100644 test/Core.Test/AdminConsole/AutoFixture/CurrentContextOrganizationFixtures.cs diff --git a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs index c9a414316..990a9d51c 100644 --- a/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs +++ b/src/Api/AdminConsole/Controllers/OrganizationUsersController.cs @@ -3,10 +3,10 @@ using Bit.Api.AdminConsole.Models.Response.Organizations; using Bit.Api.Models.Request.Organizations; using Bit.Api.Models.Response; using Bit.Api.Vault.AuthorizationHandlers.Collections; -using Bit.Api.Vault.AuthorizationHandlers.OrganizationUsers; using Bit.Core; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.AdminConsole.Repositories; using Bit.Core.Auth.Enums; @@ -44,7 +44,6 @@ public class OrganizationUsersController : Controller private readonly ICountNewSmSeatsRequiredQuery _countNewSmSeatsRequiredQuery; private readonly IUpdateSecretsManagerSubscriptionCommand _updateSecretsManagerSubscriptionCommand; private readonly IUpdateOrganizationUserCommand _updateOrganizationUserCommand; - private readonly IUpdateOrganizationUserGroupsCommand _updateOrganizationUserGroupsCommand; private readonly IAcceptOrgUserCommand _acceptOrgUserCommand; private readonly IAuthorizationService _authorizationService; private readonly IApplicationCacheService _applicationCacheService; @@ -66,7 +65,6 @@ public class OrganizationUsersController : Controller ICountNewSmSeatsRequiredQuery countNewSmSeatsRequiredQuery, IUpdateSecretsManagerSubscriptionCommand updateSecretsManagerSubscriptionCommand, IUpdateOrganizationUserCommand updateOrganizationUserCommand, - IUpdateOrganizationUserGroupsCommand updateOrganizationUserGroupsCommand, IAcceptOrgUserCommand acceptOrgUserCommand, IAuthorizationService authorizationService, IApplicationCacheService applicationCacheService, @@ -86,7 +84,6 @@ public class OrganizationUsersController : Controller _countNewSmSeatsRequiredQuery = countNewSmSeatsRequiredQuery; _updateSecretsManagerSubscriptionCommand = updateSecretsManagerSubscriptionCommand; _updateOrganizationUserCommand = updateOrganizationUserCommand; - _updateOrganizationUserGroupsCommand = updateOrganizationUserGroupsCommand; _acceptOrgUserCommand = acceptOrgUserCommand; _authorizationService = authorizationService; _applicationCacheService = applicationCacheService; @@ -115,11 +112,27 @@ public class OrganizationUsersController : Controller return response; } + [HttpGet("mini-details")] + [RequireFeature(FeatureFlagKeys.Pm3478RefactorOrganizationUserApi)] + public async Task> GetMiniDetails(Guid orgId) + { + var authorizationResult = await _authorizationService.AuthorizeAsync(User, new OrganizationScope(orgId), + OrganizationUserUserMiniDetailsOperations.ReadAll); + if (!authorizationResult.Succeeded) + { + throw new NotFoundException(); + } + + var organizationUserUserDetails = await _organizationUserRepository.GetManyDetailsByOrganizationAsync(orgId); + return new ListResponseModel( + organizationUserUserDetails.Select(ou => new OrganizationUserUserMiniDetailsResponseModel(ou))); + } + [HttpGet("")] public async Task> Get(Guid orgId, bool includeGroups = false, bool includeCollections = false) { var authorized = (await _authorizationService.AuthorizeAsync( - User, OrganizationUserOperations.ReadAll(orgId))).Succeeded; + User, new OrganizationScope(orgId), OrganizationUserUserDetailsOperations.ReadAll)).Succeeded; if (!authorized) { throw new NotFoundException(); diff --git a/src/Api/AdminConsole/Models/Response/Organizations/OrganizationUserResponseModel.cs b/src/Api/AdminConsole/Models/Response/Organizations/OrganizationUserResponseModel.cs index dcf5119d2..874169486 100644 --- a/src/Api/AdminConsole/Models/Response/Organizations/OrganizationUserResponseModel.cs +++ b/src/Api/AdminConsole/Models/Response/Organizations/OrganizationUserResponseModel.cs @@ -84,6 +84,29 @@ public class OrganizationUserDetailsResponseModel : OrganizationUserResponseMode public IEnumerable Groups { get; set; } } +#nullable enable +public class OrganizationUserUserMiniDetailsResponseModel : ResponseModel +{ + public OrganizationUserUserMiniDetailsResponseModel(OrganizationUserUserDetails organizationUser) + : base("organizationUserUserMiniDetails") + { + Id = organizationUser.Id; + UserId = organizationUser.UserId; + Type = organizationUser.Type; + Status = organizationUser.Status; + Name = organizationUser.Name; + Email = organizationUser.Email; + } + + public Guid Id { get; } + public Guid? UserId { get; } + public OrganizationUserType Type { get; } + public OrganizationUserStatusType Status { get; } + public string? Name { get; } + public string Email { get; } +} +#nullable disable + public class OrganizationUserUserDetailsResponseModel : OrganizationUserResponseModel { public OrganizationUserUserDetailsResponseModel(OrganizationUserUserDetails organizationUser, diff --git a/src/Api/Utilities/ServiceCollectionExtensions.cs b/src/Api/Utilities/ServiceCollectionExtensions.cs index 46f2d272b..a7fbddbaa 100644 --- a/src/Api/Utilities/ServiceCollectionExtensions.cs +++ b/src/Api/Utilities/ServiceCollectionExtensions.cs @@ -1,6 +1,5 @@ using Bit.Api.Vault.AuthorizationHandlers.Collections; using Bit.Api.Vault.AuthorizationHandlers.Groups; -using Bit.Api.Vault.AuthorizationHandlers.OrganizationUsers; using Bit.Core.IdentityServer; using Bit.Core.Settings; using Bit.Core.Utilities; @@ -100,6 +99,5 @@ public static class ServiceCollectionExtensions services.AddScoped(); services.AddScoped(); services.AddScoped(); - services.AddScoped(); } } diff --git a/src/Api/Vault/AuthorizationHandlers/OrganizationUsers/OrganizationUserAuthorizationHandler.cs b/src/Api/Vault/AuthorizationHandlers/OrganizationUsers/OrganizationUserAuthorizationHandler.cs deleted file mode 100644 index 4b267242a..000000000 --- a/src/Api/Vault/AuthorizationHandlers/OrganizationUsers/OrganizationUserAuthorizationHandler.cs +++ /dev/null @@ -1,60 +0,0 @@ -#nullable enable -using Bit.Core.Context; -using Microsoft.AspNetCore.Authorization; - -namespace Bit.Api.Vault.AuthorizationHandlers.OrganizationUsers; - -/// -/// Handles authorization logic for OrganizationUser objects. -/// This uses new logic implemented in the Flexible Collections initiative. -/// -public class OrganizationUserAuthorizationHandler : AuthorizationHandler -{ - private readonly ICurrentContext _currentContext; - - public OrganizationUserAuthorizationHandler(ICurrentContext currentContext) - { - _currentContext = currentContext; - } - - protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, - OrganizationUserOperationRequirement requirement) - { - if (!_currentContext.UserId.HasValue) - { - context.Fail(); - return; - } - - if (requirement.OrganizationId == default) - { - context.Fail(); - return; - } - - var org = _currentContext.GetOrganization(requirement.OrganizationId); - - switch (requirement) - { - case not null when requirement.Name == nameof(OrganizationUserOperations.ReadAll): - await CanReadAllAsync(context, requirement, org); - break; - } - } - - private async Task CanReadAllAsync(AuthorizationHandlerContext context, OrganizationUserOperationRequirement requirement, - CurrentContextOrganization? org) - { - // All users of an organization can read all other users of that organization for collection access management - if (org is not null) - { - context.Succeed(requirement); - } - - // Allow provider users to read all organization users if they are a provider for the target organization - if (await _currentContext.ProviderUserForOrgAsync(requirement.OrganizationId)) - { - context.Succeed(requirement); - } - } -} diff --git a/src/Api/Vault/AuthorizationHandlers/OrganizationUsers/OrganizationUserOperations.cs b/src/Api/Vault/AuthorizationHandlers/OrganizationUsers/OrganizationUserOperations.cs deleted file mode 100644 index c085083c3..000000000 --- a/src/Api/Vault/AuthorizationHandlers/OrganizationUsers/OrganizationUserOperations.cs +++ /dev/null @@ -1,22 +0,0 @@ -using Microsoft.AspNetCore.Authorization.Infrastructure; - -namespace Bit.Api.Vault.AuthorizationHandlers.OrganizationUsers; - -public class OrganizationUserOperationRequirement : OperationAuthorizationRequirement -{ - public Guid OrganizationId { get; } - - public OrganizationUserOperationRequirement(string name, Guid organizationId) - { - Name = name; - OrganizationId = organizationId; - } -} - -public static class OrganizationUserOperations -{ - public static OrganizationUserOperationRequirement ReadAll(Guid organizationId) - { - return new OrganizationUserOperationRequirement(nameof(ReadAll), organizationId); - } -} diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationScope.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationScope.cs new file mode 100644 index 000000000..57856e702 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationScope.cs @@ -0,0 +1,23 @@ +#nullable enable + +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization; + +/// +/// A typed wrapper for an organization Guid. This is used for authorization checks +/// scoped to an organization's resources (e.g. all users for an organization). +/// In these cases, AuthorizationService needs more than just a Guid, but we also don't want to fetch the +/// Organization object from the database each time when it's usually not needed. +/// This should not be used for operations on the organization itself. +/// It implicitly converts to a regular Guid. +/// +public record OrganizationScope +{ + public OrganizationScope(Guid id) + { + Id = id; + } + private Guid Id { get; } + public static implicit operator Guid(OrganizationScope organizationScope) => + organizationScope.Id; + public override string ToString() => Id.ToString(); +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserUserDetailsAuthorizationHandler.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserUserDetailsAuthorizationHandler.cs new file mode 100644 index 000000000..dcfe630e3 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserUserDetailsAuthorizationHandler.cs @@ -0,0 +1,77 @@ +#nullable enable +using Bit.Core.Context; +using Bit.Core.Enums; +using Bit.Core.Services; +using Microsoft.AspNetCore.Authorization; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization; + +public class OrganizationUserUserDetailsAuthorizationHandler + : AuthorizationHandler +{ + private readonly ICurrentContext _currentContext; + private readonly IFeatureService _featureService; + + public OrganizationUserUserDetailsAuthorizationHandler(ICurrentContext currentContext, IFeatureService featureService) + { + _currentContext = currentContext; + _featureService = featureService; + } + + protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, + OrganizationUserUserDetailsOperationRequirement requirement, OrganizationScope organizationScope) + { + var authorized = false; + + switch (requirement) + { + case not null when requirement.Name == nameof(OrganizationUserUserDetailsOperations.ReadAll): + authorized = await CanReadAllAsync(organizationScope); + break; + } + + if (authorized) + { + context.Succeed(requirement!); + } + } + + private async Task CanReadAllAsync(Guid organizationId) + { + if (_featureService.IsEnabled(FeatureFlagKeys.Pm3478RefactorOrganizationUserApi)) + { + return await CanReadAllAsync_vNext(organizationId); + } + + return await CanReadAllAsync_vCurrent(organizationId); + } + + private async Task CanReadAllAsync_vCurrent(Guid organizationId) + { + // All users of an organization can read all other users of that organization for collection access management + var org = _currentContext.GetOrganization(organizationId); + if (org is not null) + { + return true; + } + + // Allow provider users to read all organization users if they are a provider for the target organization + return await _currentContext.ProviderUserForOrgAsync(organizationId); + } + + private async Task CanReadAllAsync_vNext(Guid organizationId) + { + // Admins can access this for general user management + var organization = _currentContext.GetOrganization(organizationId); + if (organization is + { Type: OrganizationUserType.Owner } or + { Type: OrganizationUserType.Admin } or + { Permissions.ManageUsers: true }) + { + return true; + } + + // Allow provider users to read all organization users if they are a provider for the target organization + return await _currentContext.ProviderUserForOrgAsync(organizationId); + } +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserUserDetailsOperations.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserUserDetailsOperations.cs new file mode 100644 index 000000000..51778dc25 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserUserDetailsOperations.cs @@ -0,0 +1,10 @@ +using Microsoft.AspNetCore.Authorization.Infrastructure; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization; + +public class OrganizationUserUserDetailsOperationRequirement : OperationAuthorizationRequirement; + +public static class OrganizationUserUserDetailsOperations +{ + public static OrganizationUserUserDetailsOperationRequirement ReadAll = new() { Name = nameof(ReadAll) }; +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserUserMiniDetailsAuthorizationHandler.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserUserMiniDetailsAuthorizationHandler.cs new file mode 100644 index 000000000..24d9d0c26 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserUserMiniDetailsAuthorizationHandler.cs @@ -0,0 +1,51 @@ +using Bit.Core.Context; +using Bit.Core.Services; +using Microsoft.AspNetCore.Authorization; + +namespace Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization; + +public class OrganizationUserUserMiniDetailsAuthorizationHandler : + AuthorizationHandler +{ + private readonly IApplicationCacheService _applicationCacheService; + private readonly ICurrentContext _currentContext; + + public OrganizationUserUserMiniDetailsAuthorizationHandler( + IApplicationCacheService applicationCacheService, + ICurrentContext currentContext) + { + _applicationCacheService = applicationCacheService; + _currentContext = currentContext; + } + + protected override async Task HandleRequirementAsync(AuthorizationHandlerContext context, + OrganizationUserUserMiniDetailsOperationRequirement requirement, OrganizationScope organizationScope) + { + var authorized = false; + + switch (requirement) + { + case not null when requirement.Name == nameof(OrganizationUserUserMiniDetailsOperations.ReadAll): + authorized = await CanReadAllAsync(organizationScope); + break; + } + + if (authorized) + { + context.Succeed(requirement); + } + } + + private async Task CanReadAllAsync(Guid organizationId) + { + // All organization users can access this data to manage collection access + var organization = _currentContext.GetOrganization(organizationId); + if (organization != null) + { + return true; + } + + // Providers can also access this to manage the organization generally + return await _currentContext.ProviderUserForOrgAsync(organizationId); + } +} diff --git a/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserUserMiniDetailsOperations.cs b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserUserMiniDetailsOperations.cs new file mode 100644 index 000000000..285bf44f4 --- /dev/null +++ b/src/Core/AdminConsole/OrganizationFeatures/OrganizationUsers/Authorization/OrganizationUserUserMiniDetailsOperations.cs @@ -0,0 +1,10 @@ +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/src/Core/Constants.cs b/src/Core/Constants.cs index 65c83da50..3f54f7d42 100644 --- a/src/Core/Constants.cs +++ b/src/Core/Constants.cs @@ -143,6 +143,7 @@ public static class FeatureFlagKeys public const string EnableNewCardCombinedExpiryAutofill = "enable-new-card-combined-expiry-autofill"; public const string StorageReseedRefactor = "storage-reseed-refactor"; public const string TrialPayment = "PM-8163-trial-payment"; + public const string Pm3478RefactorOrganizationUserApi = "pm-3478-refactor-organizationuser-api"; public static List GetAllKeys() { diff --git a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs index dac1268dc..3e2946248 100644 --- a/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs +++ b/src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs @@ -9,6 +9,7 @@ using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationConnections.Interfa using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationDomains.Interfaces; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization; using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; using Bit.Core.Models.Business.Tokenables; using Bit.Core.OrganizationFeatures.OrganizationCollections; @@ -28,6 +29,7 @@ using Bit.Core.Settings; using Bit.Core.Tokens; using Core.AdminConsole.OrganizationFeatures.OrganizationUsers; using Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Interfaces; +using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.DataProtection; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; @@ -141,6 +143,9 @@ public static class OrganizationServiceCollectionExtensions services.AddScoped(); services.AddScoped(); services.AddScoped(); + + services.AddScoped(); + services.AddScoped(); } // TODO: move to OrganizationSubscriptionServiceCollectionExtensions when OrganizationUser methods are moved out of diff --git a/test/Api.Test/Vault/AuthorizationHandlers/OrganizationUserAuthorizationHandlerTests.cs b/test/Api.Test/Vault/AuthorizationHandlers/OrganizationUserAuthorizationHandlerTests.cs deleted file mode 100644 index 0d7090e68..000000000 --- a/test/Api.Test/Vault/AuthorizationHandlers/OrganizationUserAuthorizationHandlerTests.cs +++ /dev/null @@ -1,122 +0,0 @@ -using System.Security.Claims; -using Bit.Api.Vault.AuthorizationHandlers.OrganizationUsers; -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.Vault.AuthorizationHandlers; - -[SutProviderCustomize] -public class OrganizationUserAuthorizationHandlerTests -{ - [Theory] - [BitAutoData(OrganizationUserType.Admin)] - [BitAutoData(OrganizationUserType.Owner)] - [BitAutoData(OrganizationUserType.User)] - [BitAutoData(OrganizationUserType.Custom)] - public async Task CanReadAllAsync_WhenMemberOfOrg_Success( - OrganizationUserType userType, - Guid userId, SutProvider sutProvider, - CurrentContextOrganization organization) - { - organization.Type = userType; - organization.Permissions = new Permissions(); - - var context = new AuthorizationHandlerContext( - new[] { OrganizationUserOperations.ReadAll(organization.Id) }, - new ClaimsPrincipal(), - null); - - sutProvider.GetDependency().UserId.Returns(userId); - sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); - - await sutProvider.Sut.HandleAsync(context); - - Assert.True(context.HasSucceeded); - } - - [Theory, BitAutoData] - public async Task CanReadAllAsync_WithProviderUser_Success( - Guid userId, - SutProvider sutProvider, CurrentContextOrganization organization) - { - organization.Type = OrganizationUserType.User; - organization.Permissions = new Permissions(); - - var context = new AuthorizationHandlerContext( - new[] { OrganizationUserOperations.ReadAll(organization.Id) }, - new ClaimsPrincipal(), - null); - - sutProvider.GetDependency() - .UserId - .Returns(userId); - sutProvider.GetDependency() - .ProviderUserForOrgAsync(organization.Id) - .Returns(true); - - await sutProvider.Sut.HandleAsync(context); - - Assert.True(context.HasSucceeded); - } - - [Theory, BitAutoData] - public async Task HandleRequirementAsync_WhenMissingOrgAccess_NoSuccess( - Guid userId, - CurrentContextOrganization organization, - SutProvider sutProvider) - { - var context = new AuthorizationHandlerContext( - new[] { OrganizationUserOperations.ReadAll(organization.Id) }, - new ClaimsPrincipal(), - null - ); - - 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] - public async Task HandleRequirementAsync_MissingUserId_Failure( - Guid organizationId, - SutProvider sutProvider) - { - var context = new AuthorizationHandlerContext( - new[] { OrganizationUserOperations.ReadAll(organizationId) }, - new ClaimsPrincipal(), - null - ); - - // Simulate missing user id - sutProvider.GetDependency().UserId.Returns((Guid?)null); - - await sutProvider.Sut.HandleAsync(context); - Assert.True(context.HasFailed); - } - - [Theory, BitAutoData] - public async Task HandleRequirementAsync_NoSpecifiedOrgId_Failure( - SutProvider sutProvider) - { - var context = new AuthorizationHandlerContext( - new[] { OrganizationUserOperations.ReadAll(default) }, - new ClaimsPrincipal(), - null - ); - - sutProvider.GetDependency().UserId.Returns(new Guid()); - - await sutProvider.Sut.HandleAsync(context); - - Assert.True(context.HasFailed); - } -} diff --git a/test/Core.Test/AdminConsole/Authorization/OrganizationUserUserDetailsAuthorizationHandlerTests.cs b/test/Core.Test/AdminConsole/Authorization/OrganizationUserUserDetailsAuthorizationHandlerTests.cs new file mode 100644 index 000000000..4d9208a2b --- /dev/null +++ b/test/Core.Test/AdminConsole/Authorization/OrganizationUserUserDetailsAuthorizationHandlerTests.cs @@ -0,0 +1,176 @@ +using System.Security.Claims; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.Authorization; +using Bit.Core.Context; +using Bit.Core.Enums; +using Bit.Core.Services; +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 OrganizationUserUserDetailsAuthorizationHandlerTests +{ + [Theory, CurrentContextOrganizationCustomize] + [BitAutoData(OrganizationUserType.Admin)] + [BitAutoData(OrganizationUserType.Owner)] + [BitAutoData(OrganizationUserType.Custom)] + public async Task ReadAll_Admins_Success( + OrganizationUserType userType, + CurrentContextOrganization organization, + SutProvider sutProvider) + { + EnableFeatureFlag(sutProvider); + organization.Type = userType; + sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); + + if (userType == OrganizationUserType.Custom) + { + organization.Permissions.ManageUsers = true; + } + + var context = new AuthorizationHandlerContext( + new[] { OrganizationUserUserDetailsOperations.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 sutProvider) + { + EnableFeatureFlag(sutProvider); + organization.Type = OrganizationUserType.User; + sutProvider.GetDependency() + .ProviderUserForOrgAsync(organization.Id) + .Returns(true); + + var context = new AuthorizationHandlerContext( + new[] { OrganizationUserUserDetailsOperations.ReadAll }, + new ClaimsPrincipal(), + new OrganizationScope(organization.Id)); + + await sutProvider.Sut.HandleAsync(context); + + Assert.True(context.HasSucceeded); + } + + [Theory, BitAutoData, CurrentContextOrganizationCustomize] + public async Task ReadAll_User_NoSuccess( + CurrentContextOrganization organization, + SutProvider sutProvider) + { + EnableFeatureFlag(sutProvider); + organization.Type = OrganizationUserType.User; + sutProvider.GetDependency().GetOrganization(Arg.Any()).Returns(organization); + sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); + + var context = new AuthorizationHandlerContext( + new[] { OrganizationUserUserDetailsOperations.ReadAll }, + new ClaimsPrincipal(), + new OrganizationScope(organization.Id) + ); + + await sutProvider.Sut.HandleAsync(context); + Assert.False(context.HasSucceeded); + } + + [Theory, BitAutoData] + public async Task ReadAll_NotMember_NoSuccess( + CurrentContextOrganization organization, + SutProvider sutProvider) + { + EnableFeatureFlag(sutProvider); + var context = new AuthorizationHandlerContext( + new[] { OrganizationUserUserDetailsOperations.ReadAll }, + new ClaimsPrincipal(), + new OrganizationScope(organization.Id) + ); + + sutProvider.GetDependency().GetOrganization(Arg.Any()).Returns((CurrentContextOrganization)null); + sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); + + await sutProvider.Sut.HandleAsync(context); + Assert.False(context.HasSucceeded); + } + + private void EnableFeatureFlag(SutProvider sutProvider) + { + sutProvider.GetDependency().IsEnabled(FeatureFlagKeys.Pm3478RefactorOrganizationUserApi) + .Returns(true); + } + + // TESTS WITH FLAG DISABLED - TO BE DELETED IN FLAG CLEANUP + + [Theory, CurrentContextOrganizationCustomize] + [BitAutoData(OrganizationUserType.Admin)] + [BitAutoData(OrganizationUserType.Owner)] + [BitAutoData(OrganizationUserType.User)] + [BitAutoData(OrganizationUserType.Custom)] + public async Task FlagDisabled_ReadAll_AnyMemberOfOrg_Success( + OrganizationUserType userType, + Guid userId, SutProvider sutProvider, + CurrentContextOrganization organization) + { + organization.Type = userType; + + var context = new AuthorizationHandlerContext( + new[] { OrganizationUserUserDetailsOperations.ReadAll }, + new ClaimsPrincipal(), + new OrganizationScope(organization.Id)); + + sutProvider.GetDependency().UserId.Returns(userId); + sutProvider.GetDependency().GetOrganization(organization.Id).Returns(organization); + + await sutProvider.Sut.HandleAsync(context); + + Assert.True(context.HasSucceeded); + } + + [Theory, BitAutoData, CurrentContextOrganizationCustomize] + public async Task FlagDisabled_ReadAll_ProviderUser_Success( + CurrentContextOrganization organization, + SutProvider sutProvider) + { + organization.Type = OrganizationUserType.User; + sutProvider.GetDependency() + .ProviderUserForOrgAsync(organization.Id) + .Returns(true); + + var context = new AuthorizationHandlerContext( + new[] { OrganizationUserUserDetailsOperations.ReadAll }, + new ClaimsPrincipal(), + new OrganizationScope(organization.Id)); + + await sutProvider.Sut.HandleAsync(context); + + Assert.True(context.HasSucceeded); + } + + [Theory, BitAutoData] + public async Task FlagDisabled_ReadAll_NotMember_NoSuccess( + CurrentContextOrganization organization, + SutProvider sutProvider) + { + var context = new AuthorizationHandlerContext( + new[] { OrganizationUserUserDetailsOperations.ReadAll }, + new ClaimsPrincipal(), + new OrganizationScope(organization.Id) + ); + + sutProvider.GetDependency().GetOrganization(Arg.Any()).Returns((CurrentContextOrganization)null); + sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); + + await sutProvider.Sut.HandleAsync(context); + Assert.False(context.HasSucceeded); + } +} diff --git a/test/Core.Test/AdminConsole/Authorization/OrganizationUserUserMiniDetailsAuthorizationHandlerTests.cs b/test/Core.Test/AdminConsole/Authorization/OrganizationUserUserMiniDetailsAuthorizationHandlerTests.cs new file mode 100644 index 000000000..c74cb2d5c --- /dev/null +++ b/test/Core.Test/AdminConsole/Authorization/OrganizationUserUserMiniDetailsAuthorizationHandlerTests.cs @@ -0,0 +1,80 @@ +using System.Security.Claims; +using Bit.Core.AdminConsole.OrganizationFeatures.OrganizationUsers.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 sutProvider) + { + organization.Type = userType; + sutProvider.GetDependency().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 sutProvider) + { + organization.Type = OrganizationUserType.User; + sutProvider.GetDependency() + .GetOrganization(organization.Id) + .Returns((CurrentContextOrganization)null); + sutProvider.GetDependency() + .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 sutProvider) + { + var context = new AuthorizationHandlerContext( + new[] { OrganizationUserUserMiniDetailsOperations.ReadAll }, + new ClaimsPrincipal(), + new OrganizationScope(organization.Id) + ); + + sutProvider.GetDependency().GetOrganization(Arg.Any()).Returns((CurrentContextOrganization)null); + sutProvider.GetDependency().ProviderUserForOrgAsync(Arg.Any()).Returns(false); + + await sutProvider.Sut.HandleAsync(context); + Assert.False(context.HasSucceeded); + } +} diff --git a/test/Core.Test/AdminConsole/AutoFixture/CurrentContextOrganizationFixtures.cs b/test/Core.Test/AdminConsole/AutoFixture/CurrentContextOrganizationFixtures.cs new file mode 100644 index 000000000..080b8ec62 --- /dev/null +++ b/test/Core.Test/AdminConsole/AutoFixture/CurrentContextOrganizationFixtures.cs @@ -0,0 +1,40 @@ +using AutoFixture; +using Bit.Core.Context; +using Bit.Core.Enums; +using Bit.Core.Models.Data; +using Bit.Test.Common.AutoFixture.Attributes; + +namespace Bit.Core.Test.AdminConsole.AutoFixture; + +public class CurrentContextOrganizationCustomization : ICustomization +{ + public Guid Id { get; set; } + public OrganizationUserType Type { get; set; } + public Permissions Permissions { get; set; } = new(); + public bool AccessSecretsManager { get; set; } + + public void Customize(IFixture fixture) + { + fixture.Customize(composer => composer + .With(o => o.Id, Id) + .With(o => o.Type, Type) + .With(o => o.Permissions, Permissions) + .With(o => o.AccessSecretsManager, AccessSecretsManager)); + } +} + +public class CurrentContextOrganizationCustomizeAttribute : BitCustomizeAttribute +{ + public Guid Id { get; set; } + public OrganizationUserType Type { get; set; } = OrganizationUserType.User; + public Permissions Permissions { get; set; } = new(); + public bool AccessSecretsManager { get; set; } = false; + + public override ICustomization GetCustomization() => new CurrentContextOrganizationCustomization() + { + Id = Id, + Type = Type, + Permissions = Permissions, + AccessSecretsManager = AccessSecretsManager + }; +}