From 57252a7ee93eb42dbb9e3aca8fe4ec44fe995724 Mon Sep 17 00:00:00 2001 From: Rui Tome Date: Fri, 5 Dec 2025 15:51:28 +0000 Subject: [PATCH] Refactor IPremiumAccessQuery and PremiumAccessQuery to remove the overloaded CanAccessPremiumAsync method. Update related methods to streamline premium access checks using the User object directly. Enhance test coverage by removing obsolete tests and ensuring proper functionality with the new method signatures. --- .../PremiumAccess/IPremiumAccessQuery.cs | 9 -- .../PremiumAccess/PremiumAccessQuery.cs | 9 +- .../Interfaces/ITwoFactorIsEnabledQuery.cs | 3 +- .../TwoFactorAuth/TwoFactorIsEnabledQuery.cs | 32 +---- .../PremiumAccess/PremiumAccessQueryTests.cs | 74 ------------ .../TwoFactorIsEnabledQueryTests.cs | 114 ++---------------- 6 files changed, 15 insertions(+), 226 deletions(-) diff --git a/src/Core/Auth/UserFeatures/PremiumAccess/IPremiumAccessQuery.cs b/src/Core/Auth/UserFeatures/PremiumAccess/IPremiumAccessQuery.cs index ebd7aec402..dfea0cb659 100644 --- a/src/Core/Auth/UserFeatures/PremiumAccess/IPremiumAccessQuery.cs +++ b/src/Core/Auth/UserFeatures/PremiumAccess/IPremiumAccessQuery.cs @@ -22,15 +22,6 @@ public interface IPremiumAccessQuery /// True if user can access premium features; false otherwise Task CanAccessPremiumAsync(User user); - /// - /// Checks if a user has access to premium features (personal subscription or organization). - /// Use this overload when you already know the personal premium status and only need to check organization premium. - /// - /// The user ID to check for premium access - /// Whether the user has a personal premium subscription - /// True if user can access premium features; false otherwise - Task CanAccessPremiumAsync(Guid userId, bool hasPersonalPremium); - /// /// Checks if a user has access to premium features through organization membership only. /// This is useful for determining the source of premium access (personal vs organization). diff --git a/src/Core/Auth/UserFeatures/PremiumAccess/PremiumAccessQuery.cs b/src/Core/Auth/UserFeatures/PremiumAccess/PremiumAccessQuery.cs index 8da0f120e0..1489c4b3a4 100644 --- a/src/Core/Auth/UserFeatures/PremiumAccess/PremiumAccessQuery.cs +++ b/src/Core/Auth/UserFeatures/PremiumAccess/PremiumAccessQuery.cs @@ -22,17 +22,12 @@ public class PremiumAccessQuery : IPremiumAccessQuery public async Task CanAccessPremiumAsync(User user) { - return await CanAccessPremiumAsync(user.Id, user.Premium); - } - - public async Task CanAccessPremiumAsync(Guid userId, bool hasPersonalPremium) - { - if (hasPersonalPremium) + if (user.Premium) { return true; } - return await HasPremiumFromOrganizationAsync(userId); + return await HasPremiumFromOrganizationAsync(user.Id); } public async Task HasPremiumFromOrganizationAsync(Guid userId) diff --git a/src/Core/Auth/UserFeatures/TwoFactorAuth/Interfaces/ITwoFactorIsEnabledQuery.cs b/src/Core/Auth/UserFeatures/TwoFactorAuth/Interfaces/ITwoFactorIsEnabledQuery.cs index 9beb658a32..453249343c 100644 --- a/src/Core/Auth/UserFeatures/TwoFactorAuth/Interfaces/ITwoFactorIsEnabledQuery.cs +++ b/src/Core/Auth/UserFeatures/TwoFactorAuth/Interfaces/ITwoFactorIsEnabledQuery.cs @@ -1,4 +1,5 @@ using Bit.Core.Auth.Models; +using Bit.Core.Entities; namespace Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; @@ -42,5 +43,5 @@ public interface ITwoFactorIsEnabledQuery /// This version uses PremiumAccessQuery with cached organization abilities for better performance. /// /// The user to check. - Task TwoFactorIsEnabledVNextAsync(ITwoFactorProvidersUser user); + Task TwoFactorIsEnabledVNextAsync(User user); } diff --git a/src/Core/Auth/UserFeatures/TwoFactorAuth/TwoFactorIsEnabledQuery.cs b/src/Core/Auth/UserFeatures/TwoFactorAuth/TwoFactorIsEnabledQuery.cs index 9ed022184f..3809e01e68 100644 --- a/src/Core/Auth/UserFeatures/TwoFactorAuth/TwoFactorIsEnabledQuery.cs +++ b/src/Core/Auth/UserFeatures/TwoFactorAuth/TwoFactorIsEnabledQuery.cs @@ -6,7 +6,6 @@ using Bit.Core.Auth.Models; using Bit.Core.Auth.UserFeatures.PremiumAccess; using Bit.Core.Auth.UserFeatures.TwoFactorAuth.Interfaces; using Bit.Core.Entities; -using Bit.Core.Models.Data.Organizations.OrganizationUsers; using Bit.Core.Repositories; namespace Bit.Core.Auth.UserFeatures.TwoFactorAuth; @@ -147,38 +146,11 @@ public class TwoFactorIsEnabledQuery : ITwoFactorIsEnabledQuery return result; } - public async Task TwoFactorIsEnabledVNextAsync(ITwoFactorProvidersUser user) + public async Task TwoFactorIsEnabledVNextAsync(User user) { - var userId = user.GetUserId(); - if (!userId.HasValue) - { - return false; - } - - // Try to get premium status without fetching User entity if possible - bool hasPersonalPremium; - if (user is User userEntity) - { - hasPersonalPremium = userEntity.Premium; - } - else if (user is OrganizationUserUserDetails orgUserDetails) - { - hasPersonalPremium = orgUserDetails.Premium.GetValueOrDefault(false); - } - else - { - // Fallback: fetch the User entity - var fetchedUser = await _userRepository.GetByIdAsync(userId.Value); - if (fetchedUser == null) - { - return false; - } - hasPersonalPremium = fetchedUser.Premium; - } - return await TwoFactorEnabledAsync( user.GetTwoFactorProviders(), - async () => await _premiumAccessQuery.CanAccessPremiumAsync(userId.Value, hasPersonalPremium)); + async () => await _premiumAccessQuery.CanAccessPremiumAsync(user)); } /// diff --git a/test/Core.Test/Auth/UserFeatures/PremiumAccess/PremiumAccessQueryTests.cs b/test/Core.Test/Auth/UserFeatures/PremiumAccess/PremiumAccessQueryTests.cs index b30fec46e6..43b57ac29f 100644 --- a/test/Core.Test/Auth/UserFeatures/PremiumAccess/PremiumAccessQueryTests.cs +++ b/test/Core.Test/Auth/UserFeatures/PremiumAccess/PremiumAccessQueryTests.cs @@ -93,80 +93,6 @@ public class PremiumAccessQueryTests Assert.False(result); } - [Theory, BitAutoData] - public async Task CanAccessPremiumAsync_WithGuidAndPremiumFlag_WhenHasPersonalPremium_ReturnsTrue( - Guid userId, - SutProvider sutProvider) - { - // Act - var result = await sutProvider.Sut.CanAccessPremiumAsync(userId, hasPersonalPremium: true); - - // Assert - Assert.True(result); - - // Should not call repository since personal premium is enough - await sutProvider.GetDependency() - .DidNotReceive() - .GetManyByUserAsync(Arg.Any()); - } - - [Theory, BitAutoData] - public async Task CanAccessPremiumAsync_WithGuidAndPremiumFlag_WhenNoPersonalPremiumButHasOrgPremium_ReturnsTrue( - Guid userId, - OrganizationUser orgUser, - SutProvider sutProvider) - { - // Arrange - orgUser.UserId = userId; - - var orgAbilities = new Dictionary - { - { - orgUser.OrganizationId, new OrganizationAbility - { - Id = orgUser.OrganizationId, - UsersGetPremium = true, - Enabled = true - } - } - }; - - sutProvider.GetDependency() - .GetManyByUserAsync(userId) - .Returns(new List { orgUser }); - - sutProvider.GetDependency() - .GetOrganizationAbilitiesAsync() - .Returns(orgAbilities); - - // Act - var result = await sutProvider.Sut.CanAccessPremiumAsync(userId, hasPersonalPremium: false); - - // Assert - Assert.True(result); - } - - [Theory, BitAutoData] - public async Task CanAccessPremiumAsync_WithGuidAndPremiumFlag_WhenNoPersonalPremiumAndNoOrgPremium_ReturnsFalse( - Guid userId, - SutProvider sutProvider) - { - // Arrange - sutProvider.GetDependency() - .GetManyByUserAsync(userId) - .Returns(new List()); - - sutProvider.GetDependency() - .GetOrganizationAbilitiesAsync() - .Returns(new Dictionary()); - - // Act - var result = await sutProvider.Sut.CanAccessPremiumAsync(userId, hasPersonalPremium: false); - - // Assert - Assert.False(result); - } - [Theory, BitAutoData] public async Task HasPremiumFromOrganizationAsync_WhenUserHasNoOrganizations_ReturnsFalse( Guid userId, diff --git a/test/Core.Test/Auth/UserFeatures/TwoFactorAuth/TwoFactorIsEnabledQueryTests.cs b/test/Core.Test/Auth/UserFeatures/TwoFactorAuth/TwoFactorIsEnabledQueryTests.cs index 7619331009..098e6ab952 100644 --- a/test/Core.Test/Auth/UserFeatures/TwoFactorAuth/TwoFactorIsEnabledQueryTests.cs +++ b/test/Core.Test/Auth/UserFeatures/TwoFactorAuth/TwoFactorIsEnabledQueryTests.cs @@ -634,24 +634,6 @@ public class TwoFactorIsEnabledQueryTests .GetManyAsync(default); } - [Theory] - [BitAutoData] - public async Task TwoFactorIsEnabledVNextAsync_SingleUser_UserIdNull_ReturnsFalse( - SutProvider sutProvider) - { - // Arrange - var user = new TestTwoFactorProviderUser - { - Id = null - }; - - // Act - var result = await sutProvider.Sut.TwoFactorIsEnabledVNextAsync(user); - - // Assert - Assert.False(result); - } - [Theory] [BitAutoData(TwoFactorProviderType.Authenticator)] [BitAutoData(TwoFactorProviderType.Email)] @@ -681,7 +663,7 @@ public class TwoFactorIsEnabledQueryTests // Should not need to check premium access for free providers await sutProvider.GetDependency() .DidNotReceiveWithAnyArgs() - .CanAccessPremiumAsync(default(Guid), default); + .CanAccessPremiumAsync(Arg.Any()); } [Theory] @@ -706,7 +688,7 @@ public class TwoFactorIsEnabledQueryTests await sutProvider.GetDependency() .DidNotReceiveWithAnyArgs() - .CanAccessPremiumAsync(default(Guid), default); + .CanAccessPremiumAsync(Arg.Any()); } [Theory] @@ -727,7 +709,7 @@ public class TwoFactorIsEnabledQueryTests user.SetTwoFactorProviders(twoFactorProviders); sutProvider.GetDependency() - .CanAccessPremiumAsync(user.Id, false) + .CanAccessPremiumAsync(user) .Returns(false); // Act @@ -738,7 +720,7 @@ public class TwoFactorIsEnabledQueryTests await sutProvider.GetDependency() .Received(1) - .CanAccessPremiumAsync(user.Id, false); + .CanAccessPremiumAsync(user); } [Theory] @@ -759,7 +741,7 @@ public class TwoFactorIsEnabledQueryTests user.SetTwoFactorProviders(twoFactorProviders); sutProvider.GetDependency() - .CanAccessPremiumAsync(user.Id, true) + .CanAccessPremiumAsync(user) .Returns(true); // Act @@ -770,7 +752,7 @@ public class TwoFactorIsEnabledQueryTests await sutProvider.GetDependency() .Received(1) - .CanAccessPremiumAsync(user.Id, true); + .CanAccessPremiumAsync(user); } [Theory] @@ -791,7 +773,7 @@ public class TwoFactorIsEnabledQueryTests user.SetTwoFactorProviders(twoFactorProviders); sutProvider.GetDependency() - .CanAccessPremiumAsync(user.Id, false) + .CanAccessPremiumAsync(user) .Returns(true); // Has premium from org // Act @@ -802,7 +784,7 @@ public class TwoFactorIsEnabledQueryTests await sutProvider.GetDependency() .Received(1) - .CanAccessPremiumAsync(user.Id, false); + .CanAccessPremiumAsync(user); } [Theory] @@ -821,85 +803,7 @@ public class TwoFactorIsEnabledQueryTests Assert.False(result); await sutProvider.GetDependency() .DidNotReceiveWithAnyArgs() - .CanAccessPremiumAsync(default(Guid), default); - } - - [Theory] - [BitAutoData(TwoFactorProviderType.Duo)] - [BitAutoData(TwoFactorProviderType.YubiKey)] - public async Task TwoFactorIsEnabledVNextAsync_SingleUser_OrganizationUserUserDetails_WithPremium_ReturnsTrue( - TwoFactorProviderType premiumProviderType, - SutProvider sutProvider, - OrganizationUserUserDetails orgUserDetails) - { - // Arrange - var twoFactorProviders = new Dictionary - { - { premiumProviderType, new TwoFactorProvider { Enabled = true } } - }; - - orgUserDetails.Premium = false; - orgUserDetails.TwoFactorProviders = JsonHelpers.LegacySerialize(twoFactorProviders, JsonHelpers.LegacyEnumKeyResolver); - - sutProvider.GetDependency() - .CanAccessPremiumAsync(orgUserDetails.UserId!.Value, false) - .Returns(true); - - // Act - var result = await sutProvider.Sut.TwoFactorIsEnabledVNextAsync(orgUserDetails); - - // Assert - Assert.True(result); - - await sutProvider.GetDependency() - .Received(1) - .CanAccessPremiumAsync(orgUserDetails.UserId.Value, false); - } - - [Theory] - [BitAutoData(TwoFactorProviderType.Duo)] - [BitAutoData(TwoFactorProviderType.YubiKey)] - public async Task TwoFactorIsEnabledVNextAsync_SingleUser_UnknownType_FetchesUser( - TwoFactorProviderType premiumProviderType, - SutProvider sutProvider, - User fetchedUser) - { - // Arrange - var twoFactorProviders = new Dictionary - { - { premiumProviderType, new TwoFactorProvider { Enabled = true } } - }; - - var testUser = new TestTwoFactorProviderUser - { - Id = fetchedUser.Id, - Premium = false, - TwoFactorProviders = JsonHelpers.LegacySerialize(twoFactorProviders, JsonHelpers.LegacyEnumKeyResolver) - }; - - fetchedUser.Premium = false; - - sutProvider.GetDependency() - .GetByIdAsync(fetchedUser.Id) - .Returns(fetchedUser); - - sutProvider.GetDependency() - .CanAccessPremiumAsync(fetchedUser.Id, false) - .Returns(true); - - // Act - var result = await sutProvider.Sut.TwoFactorIsEnabledVNextAsync(testUser); - - // Assert - Assert.True(result); - - await sutProvider.GetDependency() - .Received(1) - .GetByIdAsync(fetchedUser.Id); - - await sutProvider.GetDependency() - .Received(1) - .CanAccessPremiumAsync(fetchedUser.Id, false); + .CanAccessPremiumAsync(Arg.Any()); } private class TestTwoFactorProviderUser : ITwoFactorProvidersUser