From bd1745a50d7bb24060b45f57a7604c6cd17066d7 Mon Sep 17 00:00:00 2001 From: Thomas Rittson <31796059+eliykat@users.noreply.github.com> Date: Thu, 11 Sep 2025 07:37:45 +1000 Subject: [PATCH] =?UTF-8?q?[PM-24192]=20Add=20OrganizationContext=20in=20A?= =?UTF-8?q?PI=C2=A0project=20(#6291)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...uthorizationHandlerCollectionExtensions.cs | 21 +++ .../Authorization/HttpContextExtensions.cs | 4 +- .../OrganizationClaimsExtensions.cs | 4 +- .../Authorization/OrganizationContext.cs | 84 ++++++++++++ .../Utilities/ServiceCollectionExtensions.cs | 8 +- .../Context/CurrentContextOrganization.cs | 4 + .../Authorization/OrganizationContextTests.cs | 125 ++++++++++++++++++ 7 files changed, 238 insertions(+), 12 deletions(-) create mode 100644 src/Api/AdminConsole/Authorization/AuthorizationHandlerCollectionExtensions.cs create mode 100644 src/Api/AdminConsole/Authorization/OrganizationContext.cs create mode 100644 test/Api.Test/AdminConsole/Authorization/OrganizationContextTests.cs diff --git a/src/Api/AdminConsole/Authorization/AuthorizationHandlerCollectionExtensions.cs b/src/Api/AdminConsole/Authorization/AuthorizationHandlerCollectionExtensions.cs new file mode 100644 index 0000000000..70cbc0d1a4 --- /dev/null +++ b/src/Api/AdminConsole/Authorization/AuthorizationHandlerCollectionExtensions.cs @@ -0,0 +1,21 @@ +using Bit.Api.Vault.AuthorizationHandlers.Collections; +using Bit.Core.AdminConsole.OrganizationFeatures.Groups.Authorization; +using Microsoft.AspNetCore.Authorization; +using Microsoft.Extensions.DependencyInjection.Extensions; + +namespace Bit.Api.AdminConsole.Authorization; + +public static class AuthorizationHandlerCollectionExtensions +{ + public static void AddAdminConsoleAuthorizationHandlers(this IServiceCollection services) + { + services.TryAddScoped(); + + services.TryAddEnumerable([ + ServiceDescriptor.Scoped(), + ServiceDescriptor.Scoped(), + ServiceDescriptor.Scoped(), + ServiceDescriptor.Scoped(), + ]); + } +} diff --git a/src/Api/AdminConsole/Authorization/HttpContextExtensions.cs b/src/Api/AdminConsole/Authorization/HttpContextExtensions.cs index accb9539fa..5cb261b41d 100644 --- a/src/Api/AdminConsole/Authorization/HttpContextExtensions.cs +++ b/src/Api/AdminConsole/Authorization/HttpContextExtensions.cs @@ -1,6 +1,4 @@ -#nullable enable - -using Bit.Core.AdminConsole.Enums.Provider; +using Bit.Core.AdminConsole.Enums.Provider; using Bit.Core.AdminConsole.Models.Data.Provider; using Bit.Core.AdminConsole.Repositories; diff --git a/src/Api/AdminConsole/Authorization/OrganizationClaimsExtensions.cs b/src/Api/AdminConsole/Authorization/OrganizationClaimsExtensions.cs index a3af3669ac..9ea01bd21b 100644 --- a/src/Api/AdminConsole/Authorization/OrganizationClaimsExtensions.cs +++ b/src/Api/AdminConsole/Authorization/OrganizationClaimsExtensions.cs @@ -1,6 +1,4 @@ -#nullable enable - -using System.Security.Claims; +using System.Security.Claims; using Bit.Core.Auth.Identity; using Bit.Core.Context; using Bit.Core.Enums; diff --git a/src/Api/AdminConsole/Authorization/OrganizationContext.cs b/src/Api/AdminConsole/Authorization/OrganizationContext.cs new file mode 100644 index 0000000000..7b06e33dfd --- /dev/null +++ b/src/Api/AdminConsole/Authorization/OrganizationContext.cs @@ -0,0 +1,84 @@ +using System.Security.Claims; +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.Services; + +// Note: do not move this into Core! See remarks below. +namespace Bit.Api.AdminConsole.Authorization; + +/// +/// Provides information about a user's membership or provider relationship with an organization. +/// Used for authorization decisions in the API layer, usually called by a controller or authorization handler or attribute. +/// +/// +/// This is intended to deprecate organization-related methods in . +/// It should remain in the API layer (not Core) because it is closely tied to user claims and authentication. +/// +public interface IOrganizationContext +{ + /// + /// Parses the provided for claims relating to the specified organization. + /// A user will have organization claims if they are a confirmed member of the organization. + /// + /// The claims for the user. + /// The organization to extract claims for. + /// + /// A representing the user's claims for the organization, + /// or null if the user has no claims. + /// + public CurrentContextOrganization? GetOrganizationClaims(ClaimsPrincipal user, Guid organizationId); + /// + /// Used to determine whether the user is a ProviderUser for the specified organization. + /// + /// The claims for the user. + /// The organization to check the provider relationship for. + /// True if the user is a ProviderUser for the specified organization, otherwise false. + /// + /// This requires a database call, but the results are cached for the lifetime of the service instance. + /// Try to check purely claims-based sources of authorization first (such as organization membership with + /// ) to avoid unnecessary database calls. + /// + public Task IsProviderUserForOrganization(ClaimsPrincipal user, Guid organizationId); +} + +public class OrganizationContext( + IUserService userService, + IProviderUserRepository providerUserRepository) : IOrganizationContext +{ + public const string NoUserIdError = "This method should only be called on the private api with a logged in user."; + + /// + /// Caches provider relationships by UserId. + /// In practice this should only have 1 entry (for the current user), but this approach ensures that a mix-up + /// between users cannot occur if is called with a different + /// ClaimsPrincipal for any reason. + /// + private readonly Dictionary> _providerUserOrganizationsCache = new(); + + public CurrentContextOrganization? GetOrganizationClaims(ClaimsPrincipal user, Guid organizationId) + { + return user.GetCurrentContextOrganization(organizationId); + } + + public async Task IsProviderUserForOrganization(ClaimsPrincipal user, Guid organizationId) + { + var userId = userService.GetProperUserId(user); + if (!userId.HasValue) + { + throw new InvalidOperationException(NoUserIdError); + } + + if (!_providerUserOrganizationsCache.TryGetValue(userId.Value, out var providerUserOrganizations)) + { + providerUserOrganizations = + await providerUserRepository.GetManyOrganizationDetailsByUserAsync(userId.Value, + ProviderUserStatusType.Confirmed); + providerUserOrganizations = providerUserOrganizations.ToList(); + _providerUserOrganizationsCache[userId.Value] = providerUserOrganizations; + } + + return providerUserOrganizations.Any(o => o.OrganizationId == organizationId); + } +} diff --git a/src/Api/Utilities/ServiceCollectionExtensions.cs b/src/Api/Utilities/ServiceCollectionExtensions.cs index b956fc73bb..6af688f548 100644 --- a/src/Api/Utilities/ServiceCollectionExtensions.cs +++ b/src/Api/Utilities/ServiceCollectionExtensions.cs @@ -1,7 +1,5 @@ using Bit.Api.AdminConsole.Authorization; using Bit.Api.Tools.Authorization; -using Bit.Api.Vault.AuthorizationHandlers.Collections; -using Bit.Core.AdminConsole.OrganizationFeatures.Groups.Authorization; using Bit.Core.Auth.IdentityServer; using Bit.Core.PhishingDomainFeatures; using Bit.Core.PhishingDomainFeatures.Interfaces; @@ -109,14 +107,12 @@ public static class ServiceCollectionExtensions public static void AddAuthorizationHandlers(this IServiceCollection services) { - services.AddScoped(); - services.AddScoped(); - services.AddScoped(); services.AddScoped(); services.AddScoped(); services.AddScoped(); - services.AddScoped(); + // Admin Console authorization handlers + services.AddAdminConsoleAuthorizationHandlers(); } public static void AddPhishingDomainServices(this IServiceCollection services, GlobalSettings globalSettings) diff --git a/src/Core/AdminConsole/Context/CurrentContextOrganization.cs b/src/Core/AdminConsole/Context/CurrentContextOrganization.cs index 3c9dc10cc0..e154a5a25f 100644 --- a/src/Core/AdminConsole/Context/CurrentContextOrganization.cs +++ b/src/Core/AdminConsole/Context/CurrentContextOrganization.cs @@ -5,6 +5,10 @@ using Bit.Core.Utilities; namespace Bit.Core.Context; +/// +/// Represents the claims for a user in relation to a particular organization. +/// These claims will only be present for users in the status. +/// public class CurrentContextOrganization { public CurrentContextOrganization() { } diff --git a/test/Api.Test/AdminConsole/Authorization/OrganizationContextTests.cs b/test/Api.Test/AdminConsole/Authorization/OrganizationContextTests.cs new file mode 100644 index 0000000000..92109cea93 --- /dev/null +++ b/test/Api.Test/AdminConsole/Authorization/OrganizationContextTests.cs @@ -0,0 +1,125 @@ +using System.Security.Claims; +using AutoFixture; +using Bit.Api.AdminConsole.Authorization; +using Bit.Core.AdminConsole.Enums.Provider; +using Bit.Core.AdminConsole.Models.Data.Provider; +using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Services; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using Xunit; + +namespace Bit.Api.Test.AdminConsole.Authorization; + +[SutProviderCustomize] +public class OrganizationContextTests +{ + [Theory, BitAutoData] + public async Task IsProviderUserForOrganization_UserIsProviderUser_ReturnsTrue( + Guid userId, Guid organizationId, Guid otherOrganizationId, + SutProvider sutProvider) + { + var claimsPrincipal = new ClaimsPrincipal(); + var providerUserOrganizations = new List + { + new() { OrganizationId = organizationId }, + new() { OrganizationId = otherOrganizationId } + }; + + sutProvider.GetDependency() + .GetProperUserId(claimsPrincipal) + .Returns(userId); + + sutProvider.GetDependency() + .GetManyOrganizationDetailsByUserAsync(userId, ProviderUserStatusType.Confirmed) + .Returns(providerUserOrganizations); + + var result = await sutProvider.Sut.IsProviderUserForOrganization(claimsPrincipal, organizationId); + + Assert.True(result); + await sutProvider.GetDependency() + .Received(1) + .GetManyOrganizationDetailsByUserAsync(userId, ProviderUserStatusType.Confirmed); + } + + public static IEnumerable UserIsNotProviderUserData() + { + // User has provider organizations, but not for the target organization + yield return + [ + new List + { + new Fixture().Create() + } + ]; + + // User has no provider organizations + yield return [Array.Empty()]; + } + + [Theory, BitMemberAutoData(nameof(UserIsNotProviderUserData))] + public async Task IsProviderUserForOrganization_UserIsNotProviderUser_ReturnsFalse( + IEnumerable providerUserOrganizations, + Guid userId, Guid organizationId, + SutProvider sutProvider) + { + var claimsPrincipal = new ClaimsPrincipal(); + + sutProvider.GetDependency() + .GetProperUserId(claimsPrincipal) + .Returns(userId); + + sutProvider.GetDependency() + .GetManyOrganizationDetailsByUserAsync(userId, ProviderUserStatusType.Confirmed) + .Returns(providerUserOrganizations); + + var result = await sutProvider.Sut.IsProviderUserForOrganization(claimsPrincipal, organizationId); + + Assert.False(result); + } + + [Theory, BitAutoData] + public async Task IsProviderUserForOrganization_UserIdIsNull_ThrowsException( + Guid organizationId, + SutProvider sutProvider) + { + var claimsPrincipal = new ClaimsPrincipal(); + + sutProvider.GetDependency() + .GetProperUserId(claimsPrincipal) + .Returns((Guid?)null); + + var exception = await Assert.ThrowsAsync(() => + sutProvider.Sut.IsProviderUserForOrganization(claimsPrincipal, organizationId)); + + Assert.Equal(OrganizationContext.NoUserIdError, exception.Message); + } + + [Theory, BitAutoData] + public async Task IsProviderUserForOrganization_UsesCaching( + Guid userId, Guid organizationId, + SutProvider sutProvider) + { + var claimsPrincipal = new ClaimsPrincipal(); + var providerUserOrganizations = new List + { + new() { OrganizationId = organizationId } + }; + + sutProvider.GetDependency() + .GetProperUserId(claimsPrincipal) + .Returns(userId); + + sutProvider.GetDependency() + .GetManyOrganizationDetailsByUserAsync(userId, ProviderUserStatusType.Confirmed) + .Returns(providerUserOrganizations); + + await sutProvider.Sut.IsProviderUserForOrganization(claimsPrincipal, organizationId); + await sutProvider.Sut.IsProviderUserForOrganization(claimsPrincipal, organizationId); + + await sutProvider.GetDependency() + .Received(1) + .GetManyOrganizationDetailsByUserAsync(userId, ProviderUserStatusType.Confirmed); + } +}